feature request: zip support

[liPillON]

I check the patch notes from time to time and 0.25 is shaping up to be a pretty major release... Would you consider adding zip archive support, too?

I think it would be a pretty handy feature: Woof has it and being able to load the archive downloaded from dsda/idgames directly has pampered me a little bit lately..

Thank you for all the work on this port!

[kraflab]

It's something I plan on doing but I'm not sure when - most likely not in 0.25

[liPillON]

Nice! I'll wait then, there's already enough stuff on the table..

[liPillON]

I've just had the chance to try this, thank you all!

Is there any plan to add zip support to the autoload routines, too? Sorry, I should have specified it earlier..

[kraflab]

Yes I will be expanding zip support to cover various other cases

[liPillON]

Great!

One last thing: I've noticed that the current implementation does not remove from %TEMP% (I'm on Windows 11) the directories created for each zip. This could become a problem, should a zip archive be modified by removing/renaming/updating its contents: the temporary folder would then end up having both old and new versions of each wad/lump - leading to unpredictable/unexpected behaviour.

Keep up the good work!

[kraflab]

@FtZPetruska maybe the unzipping code should remove the directory if it exists before unzipping, what do you think?

[FtZPetruska]

Thanks for the heads up, I think I have a patch ready to address that.

Finding a way to recursively delete directories was surprisingly tedious (M_remove only works on empty directories, I_Glob API does not list directories, the easiest WIN32 call to achieve that requires double-null terminated string). I'm going to run a few more tests before opening a PR.

[kraflab]

@FtZPetruska should we use the md5 checksum of a zip file instead of its name for the temp directory? I'm just realizing for instance -file my_zip.zip my_other_zip/my_zip.zip is going to have problems 😄

Does it take long to compute a checksum of a large file? I have no clue tbh. It will make the logs less decipherable - maybe for instance my_zip.zip-<checksum> could be the location.

[kraflab]

There are other ways to avoid overwrites, like we could just have a zip file counter and append it to the name, so you get first.zip-1 second.zip-2 etc.

[liPillON]

github notifications brought me here, so while I'm here I'll comment on this:

the unzipping code should remove the directory if it exists before unzipping

I think removing the temporary folders during program shutdown would be better (Woof does this, iirc)

Having tried one of the latest dsda builds with various wads over the course of a week, I've noticed that a lot of cruft has accumulated in my %TEMP% folder..

[fabiangreffrath]

There are other ways to avoid overwrites, like we could just have a zip file counter and append it to the name, so you get first.zip-1 second.zip-2 etc.

Done so here: https://github.com/fabiangreffrath/woof/commit/5156ef30e9eb1a49c9d7daaea82745383d669348

[kraflab]

@liPillON as of https://github.com/kraflab/dsda-doom/commit/9c93ea86ddf49c6862d958ad7e2ed65c20f27fe4 it should be possible to autoload zip files and another change since the initial implementation is things like -file something will load something.zip if it exists (and there is no something.wad). Let me know if you run into any issues or if you think something is still missing.

[liPillON]

Thank you both for refining this! I'll see what I can do to provide my feedback, probably after the weekend.

[liPillON]

hey, just chiming in to say sorry for failing to provide a final feedback I've messed up my build environment, so I'm unable to test on my win11 machine.. that's all :(