vn-tools / arc_unpacker

CLI tool for extracting images and sounds from visual novels.
GNU General Public License v3.0
562 stars 83 forks source link

Add common interface for archive and converter #14

Closed rr- closed 9 years ago

rr- commented 9 years ago

For #13 to work nicely, Archive and Converter should derive from a new class, Transformer, which has pure virtual methods such as add_cli_help and parse_cli_options.

rr- commented 9 years ago

BTW, the only reason why Converter is not an Archive that does file_saver.save(one_file) is to ensure that following

./arc_unpacker ~/file.dat ~/file2.dat

produces

~/file~.png
~/file2~.png

and not

~/file~/file.png
~/file2~/file2.png

Because of this, the code needs to be duplicated all over the place:

If Archive and Converter were to be merged into one Transformer, the original behaviour could be emulated with a use_folder_prefix public field inside each Transformer. This isn't exactly best solution ever, but it's probably better than having to deal with such a dual design thorough whole project.

rr- commented 9 years ago

I tried implementing this by making Converter derive from Archive ( == Transformer from my ponderings above) but it didn't end too well, since it didn't understood that nested archives should have name in form of parent file/inner file. It just kept naming everything in inner file form.

If I am to merge these classes indeed, use_folder_prefix or injecting file naming strategy on Transformer level is an absolute must. Additionally, the ideal solution would unify file naming logic between main executable and nested archives handling. Right now they're completely separate :-1:


The other option to detect how to name the file is to do it after the fact. If the transformer produced more than two files, it should create a folder for them, if not, place it aside the original file. Detecting this is difficult, because it would require file pooling to be able to tell how to proceed before saving, or renaming the files after the fact. Both variants can get a little mind-numbing when considering nested archives.

Needless to say, I'd prefer automatic way over manual, failure-prone micromanagement. I'm going to try file pools this weekend. Until then, I'll focus on making handling nested archives less pain in the ass (see 6a270eeaa45c583fd0696d436e21389cb8657e65 for what I mean).

rr- commented 9 years ago

Needless to say, I'd prefer automatic way over manual, failure-prone micromanagement. I'm going to try file pools this weekend. Until then, I'll focus on making handling nested archives less pain in the ass (see 6a270ee for what I mean).

Actually I just stumbled across a use case that makes manual file naming convention injecting seem much more reasonable. Imagine archive containing files, that, once extracted, make up for the final file system:

So forget about file pools for now, gonna go with use_folder_prefix (or similar, more enterprisey approach). The way I see it is that both Archive and Converter should derive from a Transformer and implement use_folder_prefix. By default Converter disables prefixes and Archive enables them, but at the same time, it's left public so it can be manually tweaked depending on current needs. After that, FileSaver proxy within Transformer will either add the folder prefix to the saved file or not, depending on configuration of active sub-transformer.

rr- commented 9 years ago

Fuck yeah :100:

The naming management is now hard-to-follow, but it works better than before, made project structure much cleaner and unit tests should convey why it needs to be like this. I'm quite satisfied with the results.