PAK support for Corruption

[Belokuikuini]

What it says on the tin :)
Mostly a repeat from pak_gc.py, still with a few changes to reflect the Corruption differences with Echoes or Prime. Not yet tested, has plenty of comments regarding some questions I have on specific parts of the code.
Also regarding tests, I'm not entirely sure which tests I should run and how I should setup the testing environment
It's unfinished for now, so I'm just creating this pull request to submit my code for review and hopefully refine it a little better

[Miepee]

Some style guidelines regarding comments.
Note, that in most cases, we will not block a PR due to comment style.

[henriquegemignani]

Some style guidelines regarding comments. Note, that in most cases, we will not block a PR due to comment style.

pre-commit is broken here rn, these are addressed automatically

no

[codecov]

Codecov Report

Attention: Patch coverage is 92.77108% with 6 lines in your changes missing coverage. Please review.

Project coverage is 68.57%. Comparing base (02fc115) to head (afa3ad0).
Report is 17 commits behind head on main.

Files	Patch %	Lines
src/retro_data_structures/formats/pak_wii.py	92.68%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #129      +/-   ##
==========================================
+ Coverage   68.08%   68.57%   +0.49%     
==========================================
  Files          81       82       +1     
  Lines        5091     5194     +103     
==========================================
+ Hits         3466     3562      +96     
- Misses       1625     1632       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

go away

[Belokuikuini]

Apparently I still need to change something, but I can't seem to find what it is. I believe I've resolved every discussion in the initial review, did I miss something ?

[Miepee]

If you're talking about the big red "Changes requested", that just means that no one has rereviewed it yet.

[Belokuikuini]

Bumping this and also taking the chance to ask another question :
Once this is merged, what should I do next ? Are there some file formats that should be prioritized over others, or more complex that I should avoid while I get better at handling Construct ?

[henriquegemignani]

Did you play around with decoding all PAKs? A bit tricky with being able to determine the contents are correct, but you can try comparing what you get with PakTool or PWE.

If you recreate a PAK, can you parse what you just created? What if you feed what you created to PakTool, does it still work? What about booting the game in Dolphin? You can boot extracted games in Dolphin, so you don't even need to create an ISO (though you can just use nod for that)

[Belokuikuini]

So I tested all of the pak files from the game, and all pass the tests but one : Worlds.pak fails test_identical_when_keep_data (but still passes test_compare_header_keep_data).
The test failure stemmed from an incorrect length for the named resources section ; The parsed section was shorter than the original and some named resources were missing.
I went ahead and analyzed what went wrong and I think I've found the problem : in Worlds.pak, some resources share the same name despite having a different ID and asset type. Since the named_resources dict for the PakBody class is keyed by names, the resources that share a name get overwritten when parsed into PakBody (the data parsed by Construct seems accurate to the contents of World.pak, it's at line 143 where the dict is being built that things go wrong).
I'd suggest changing the keys to be the asset IDs since those should (tm) be unique to each resource, but this is quite a big change compared to how PakGC works. I'm not sure whether this will break things or not, so if I'm to use this solution, are there other things that I should be aware of? And if I'm not to use that solution, then what should I do?

[henriquegemignani]

Ah yes, game data that feels like bugs. Would be great to know how the game itself handles this case: does it search for all ids with a name, only the first one or only the last one? My guess is one of the later two.

Using the asset id as a key is a bit weird, I'd imagine you're interested in finding the value for a given name. Though checking uses, we never actually use these named resources right now (more because we don't touch these).

My suggestion is to change these named resources to just be a list.

[Belokuikuini]

So I did all of the recommended verifications, and everything seemed in order :

• I can arbitrarily build PAKs into files that I can then decode
• Feeding these arbitrarily built PAK files into PakTool has only caused one issue : declaring a resource as compressed causes extraction to fail, but I belive this is to be expected considering the arbitrary data I wrote isn't following the LZO norm and PakTool's output displays embedded compression handling as disabled. Besides that everything else works fine
• I can rebuild a PAK that was extracted via PakTool and boot the game on Dolphin with it just fine. Some duplicates were removed due to how PakTool extracts resources, so that may lead to even slower load times, but again I believe this is to be expected. Extracting the same rebuilt PAK with PakTool also caused no issue.
  I left the scripts I used to make my verifications as commented out functions in test_pak_wii.py so that they may be reviewed if I did something wrong