[PATCH] ".DELETED" suffix only works on files, not directories

[fabio]

It would be useful for modding if a ".DELETED" suffixed file (and/or directory?) could be able to also disable a directory. Actually it only works for files.

Note that the directory removal should be done before adding mod directory (or file) with the same name.

Example: the mod ship a maps.DELETED file and a maps directory with some files. maps.DELETED should first applied removing public maps directory, then mod maps directory should be added.

[leper]

For loose files the patch is trivial, but to properly handle .DELETED files in archives we need to explicitly order those first. Note that this will likely break if someone tries to build an archive by just zipping it up and not using the archive builder. (At least if they use .DELETED on folders and expect all their files to be there).

A possible enhancement to the patch would be checking if we find .DELETED files after having encountered a normal (not .DELETED) file in an archive. And even this might cause issues with having a .zip in side a .zip, where the first one removes a folder in the second, after the first one already started adding normal files. (Not unsolvable but quite some added complexity)

[fabio]

A possible way to improve this would be to change the file extension of the mods from .zip to .pyr or something, maybe also requiring inside the .pyr a specific file (containing an hash of some file of the package) that needs to be available to properly load the mod. This way modders would be forced to generate the proper archive with the autobuilder which would then be sorted the right way.

In a future if/when we'll have a mod download server the modder will provide the zip and the server will automatically prepare the .pyr file.

Not the best solution, I agree, but if the sort order is important this way it could be forced.

The .DELETED dir would be useful for example to delete 0ad civs directory in pony mod, currently ponies can fight against human being, this is probably not wanted (since this mod already has its own 3 pony civs): ​http://youtu.be/TERel5Obie4

[leper]

Removing review due to some possible issues that need some more investigation.

Mounting a mod with foo.DELETED (file or folder doesn't matter) with a lower priority than a mod with foo, could result in foo not being present.

How to nicely handle .DELETED files when encountering loose files or loose files and archives is also something I need to think about.

Requiring the specific order of .DELETED files in mods is something that will be kept in other iterations of this patch.

[leper]

We should just handle .DELETED in the same way we handle replacing files. That is if the .DELETED file has a higher priority than the normal file we remove the normal file.

So in case someone tries to add a loose .DELETED file that should remove the normal file in an archive the timestamp of both files is a deciding factor (but this is consistent with file replacing).

For .DELETED folders we should just perform the replacement check for each file in that folder. This way we do not need the sorting of .DELETED files at all, since a normal file and a .DELETED file in the same archive should not result in no file, but the normal file. (Or it might depend on the timestamps, which means it is a bit broken)

[leper]

Support .DELETED, fix .DELETED when mounting a lower priority mod.

[leper]

This patch adds some tests (only work with the rest of the patch but with some small changes one can verify that the current .DELETED handling is broken as it ignores priority), and adds support for .DELETED for folders.

If a .DELETED file and a folder occur in the same mod the folder is created. .DELETED files only apply to lower priority mods. Since we have one mod in a user-writable folder for each enabled mod (mod selector) which is mounted with higher priority than the (probably archived) mod supporting .DELETED for the same priority sounds like a source of confusion and possibly bugs.

CCing Jan since he might use the VFS in some other project and some input would be nice too. (I'm not really sure about tne naming of the new functions.)

[wraitii]

On windows 7 here. Seems to work correctly and passes the tests you wrote - which also look correct.

I think it's a little odd how you need a .deleted file and then you can add the folder instead of just naming the directory .deleted altogether but I guess it's far simpler that way.

Function names area little odd if you read the function itself but make sense in context of when they are used so it seems fine.

Think it can be committed.

[elexis]

Thanks for the review, but I'll just keep it in the queue until it's committed, otherwise it might become forgotton.

[Itms]

Move tickets from the review queue to the rfc one.

[elexis]

Feature freeze in 2 days.

[wraitii]

In 18916:

Allow .Deleted to work on directories. Fixes #2641. Patch by leper.