Fix resources extracted from pak_wii being unusable

[Belokuikuini]

We ran into problems trying to use resources that were extracted by pak_wii, causing RDS to raise exceptions

[Belokuikuini]

Here are several error logs that were found trying to run the following command :
python -m retro_data_structures areas --game CORRUPTION --input-iso "$env:PRIME3_ISO" list-areas

From @henriquegemignani :

(venv) PS C:\Users\henri\programming\retro-data-structures> python -m retro_data_structures areas --game CORRUPTION --input-iso "$env:PRIME3_ISO" list-areas                                                        
INFO:retro_data_structures.asset_manager:Reading FrontEnd.pak
~no name~ (0x8d8c431ab3fb1cc0) - !!front_end_universe_area: 0xFE9A0228BA72CFFD
~no name~ (0xf4a9f72c65fbae26) - !!front_end_universe_area: 0xFE9A0228BA72CFFD
~no name~ (0xf4a9f72c65fbae26) - !!front_end_world_area: 0xF6AF1EEDB63F5E73
~no name~ (0xf4a9f72c65fbae26) - !!front_end_world: 0xD5190403C6FCA59C
INFO:retro_data_structures.asset_manager:Reading Worlds.pak
Traceback (most recent call last):
  File "C:\Users\henri\AppData\Local\Programs\Python\Python310\lib\runpy.py", line 196, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "C:\Users\henri\AppData\Local\Programs\Python\Python310\lib\runpy.py", line 86, in _run_code
    exec(code, run_globals)
  File "C:\Users\henri\programming\retro-data-structures\src\retro_data_structures\__main__.py", line 6, in <module>
    cli.main()
  File "C:\Users\henri\programming\retro-data-structures\src\retro_data_structures\cli.py", line 450, in main
    handle_args(create_parser().parse_args())
  File "C:\Users\henri\programming\retro-data-structures\src\retro_data_structures\cli.py", line 443, in handle_args
    do_area_command(args)
  File "C:\Users\henri\programming\retro-data-structures\src\retro_data_structures\cli.py", line 406, in do_area_command
    _list_areas_command(args, asset_manager)
  File "C:\Users\henri\programming\retro-data-structures\src\retro_data_structures\cli.py", line 361, in _list_areas_command
    mlvl = asset_manager.get_file(mlvl_id, Mlvl)
  File "C:\Users\henri\programming\retro-data-structures\src\retro_data_structures\asset_manager.py", line 264, in get_file
    return self.get_parsed_asset(path, type_hint=type_hint)
  File "C:\Users\henri\programming\retro-data-structures\src\retro_data_structures\asset_manager.py", line 258, in get_parsed_asset
    return format_class.parse(self.get_raw_asset(asset_id).data, target_game=self.target_game, asset_manager=self)
  File "C:\Users\henri\programming\retro-data-structures\src\retro_data_structures\base_resource.py", line 55, in parse
    return cls(cls.construct_class(target_game).parse(data, target_game=target_game), target_game, asset_manager)
  File "C:\Users\henri\programming\retro-data-structures\venv\lib\site-packages\construct\core.py", line 404, in parse
    return self.parse_stream(io.BytesIO(data), **contextkw)
  File "C:\Users\henri\programming\retro-data-structures\venv\lib\site-packages\construct\core.py", line 416, in parse_stream
    return self._parsereport(stream, context, "(parsing)")
  File "C:\Users\henri\programming\retro-data-structures\venv\lib\site-packages\construct\core.py", line 428, in _parsereport
    obj = self._parse(stream, context, path)
  File "C:\Users\henri\programming\retro-data-structures\venv\lib\site-packages\construct\core.py", line 3233, in _parse
    parseret = sc._parsereport(stream, context, path)
  File "C:\Users\henri\programming\retro-data-structures\venv\lib\site-packages\construct\core.py", line 428, in _parsereport
    obj = self._parse(stream, context, path)
  File "C:\Users\henri\programming\retro-data-structures\venv\lib\site-packages\construct\core.py", line 2770, in _parse
    return self.subcon._parsereport(stream, context, path)
  File "C:\Users\henri\programming\retro-data-structures\venv\lib\site-packages\construct\core.py", line 428, in _parsereport
    obj = self._parse(stream, context, path)
  File "C:\Users\henri\programming\retro-data-structures\venv\lib\site-packages\construct\core.py", line 4036, in _parse
    return sc._parsereport(stream, context, path)
  File "C:\Users\henri\programming\retro-data-structures\venv\lib\site-packages\construct\core.py", line 428, in _parsereport
    obj = self._parse(stream, context, path)
  File "C:\Users\henri\programming\retro-data-structures\venv\lib\site-packages\construct\core.py", line 3145, in _parse
    raise ExplicitError("Error field was activated during parsing", path=path)
construct.core.ExplicitError: Error in path (parsing) -> mlvl

From me :

PS C:\Users\belok\retro-data-structures> python -m retro_data_structures areas --game CORRUPTION --input-iso "$env:PRIME3_ISO" list-areas           
INFO:retro_data_structures.asset_manager:Reading FrontEnd.pak
~no name~ (0x8d8c431ab3fb1cc0) - !!front_end_universe_area: 0xFE9A0228BA72CFFD
~no name~ (0xf4a9f72c65fbae26) - !!front_end_universe_area: 0xFE9A0228BA72CFFD
~no name~ (0xf4a9f72c65fbae26) - !!front_end_world_area: 0xF6AF1EEDB63F5E73
~no name~ (0xf4a9f72c65fbae26) - !!front_end_world: 0xD5190403C6FCA59C
INFO:retro_data_structures.asset_manager:Reading Worlds.pak
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\__main__.py", line 6, in <module>
    cli.main()
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\cli.py", line 450, in main
    handle_args(create_parser().parse_args())
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\cli.py", line 443, in handle_args
    do_area_command(args)
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\cli.py", line 406, in do_area_command
    _list_areas_command(args, asset_manager)
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\cli.py", line 363, in _list_areas_command
    world_name = mlvl.world_name
                 ^^^^^^^^^^^^^^^
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\formats\mlvl.py", line 343, in world_name
    return self._name_strg.strings[0]
           ^^^^^^^^^^^^^^^
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\formats\mlvl.py", line 330, in _name_strg
    self._name_strg_cached = self.asset_manager.get_file(self._raw.world_name_id, type_hint=Strg)
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\asset_manager.py", line 261, in get_file
    return self.get_parsed_asset(path, type_hint=type_hint)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\asset_manager.py", line 255, in get_parsed_asset
    return format_class.parse(self.get_raw_asset(asset_id).data, target_game=self.target_game, asset_manager=self)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\base_resource.py", line 55, in parse
    return cls(cls.construct_class(target_game).parse(data, target_game=target_game), target_game, asset_manager)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 404, in parse
    return self.parse_stream(io.BytesIO(data), **contextkw)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 416, in parse_stream
    return self._parsereport(stream, context, "(parsing)")
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 428, in _parsereport
    obj = self._parse(stream, context, path)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 2236, in _parse
    subobj = sc._parsereport(stream, context, path)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 428, in _parsereport
    obj = self._parse(stream, context, path)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 2770, in _parse
    return self.subcon._parsereport(stream, context, path)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 428, in _parsereport
    obj = self._parse(stream, context, path)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 2843, in _parse
    obj = self.subcon._parsereport(stream, context, path)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 428, in _parsereport
    obj = self._parse(stream, context, path)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 1157, in _parse
    data = stream_read(stream, self.length, path)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 178, in stream_read
    raise StreamError("stream read less than specified amount, expected %d, found %d" % (length, len(data)), path=path)
construct.core.StreamError: Error in path (parsing) -> magic
stream read less than specified amount, expected 4, found 0

Strange thing is, we are not getting the same errors despite running the same command. I made sure to pull from upstream before running it.

As for my case, I've looked into why I'm getting such an error, so I've written test_resource_extraction to check whether the results from PakTool aligned with what I was getting.
So far, it seems like uncompressed resources work just fine, haven't gotten a single error on those yet, but resources that are compressed do have issues : running the get_decompressed method only yields empty bytes, which explains why parsing can't read anything from the stream. What remains unexplained though is why does that method return empty bytes when it shouldn't ?

[Belokuikuini]

By the looks of it, it seems like LZO is handled differently between Echoes and Corruption ; Corruption has some extra metadata ;

• All compressed resources start with the CMPD fourCC and the amount of compressed blocks
• Each block has a header with a 1 byte flag, a 3 bytes compressed size and a 4 bytes decompressed size

So far we've been using LZO blocks that were catered towards Echoes' compression, and not Corruption's, which would explain why the get_decompressed function returns odd results

[codecov]

Codecov Report

Attention: Patch coverage is 96.42857% with 1 line in your changes missing coverage. Please review.

Project coverage is 69.68%. Comparing base (a38a56a) to head (edbd45a).
Report is 41 commits behind head on main.

Files with missing lines	Patch %	Lines
src/retro_data_structures/compression.py	92.30%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #139      +/-   ##
==========================================
+ Coverage   68.57%   69.68%   +1.10%     
==========================================
  Files          82       88       +6     
  Lines        5194     5564     +370     
==========================================
+ Hits         3562     3877     +315     
- Misses       1632     1687      +55     

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

[Belokuikuini]

So I belive I've fixed parsing the compressed resources from paks, by creating a new LZOSegment class specifically for corruption ;
Parsing said resources always seems to yield coherent results to what was in the file. However, this did not fix decompression as many errors still persist.

Now, this wouldn't be as bad if it always was the same error, but no, that would be too easy :)
As a matter of fact, there are several types of exceptions raised, all when attempting decompression of a segment, IF an exception is raised, as sometimes it will succeed for some reason.

So I've run some statistics and tried decompressing every single compressed resource in the game to see what error would present itself most often. The results are compiled within this csv :
decompression_analysis.csv

Reading the LZO documentation itself wasn't of much help for me, seeing as most of these exceptions just mean "data corrupted", it doesn't tell me what's wrong with it from an outsider perspective. Besides, if my data was truly corrupted, I don't think my game would even run with only 1% file integrity.
Seeing as there's nowhere I intentionally modify the data aside from isolating it (and again cropping seems about right as the segment length seemed to be properly parsed), I'm simply fresh out of ideas as to what I'd be doing wrong ?

Maybe we're not using the right compression level ? The retro modding wiki page for Prime 3 Paks specifies Corruption's files "are compressed with segmented LZO1X, just like Prime 2". While it is specified Echoes uses LZO1X-999, Corruption's compression level isn't explicitly stated, but not only does the phrasing imply it's also 999, I see no reason why Retro woul go for a lower compression level ? It's very unlikely that's actually the problem, but I don't really see any other way to explain the errors I've been getting.

TLDR : I've hit a roadblock with decompressing some segments and cannot continue working on this PR if I can't find a way to explain the decompression errors.

[henriquegemignani]

Where exactly are you getting errors? When parsing a pak?

[Belokuikuini]

Parsing and Building as I've done them on the previous PR should still work as I've not modified them (the tests still pass)

This would initially happen on PakFile.get_decompressed() calls, at first because the compressed resources weren't parsed properly.
These errors still originate from there even after the parse fix, but this time it's lzokay exceptions that get raised : the csv I've put in my previous comment should list all of them and how many resources raise the same exception type. The stack traces would always end on CompressedLZO._decode() in compression.py, on return self.lib.decompress(data, length), at line 19

[Belokuikuini]

So the adapter class that I was recommended to implement does seem to function as expected, as test_corruption_resource_encode_decode now passes. However, there is still trouble with test_corruption_resource_decode.

Initially looking at the stack trace, we can see that decoded is only of length 12 instead of the expected 172, which is odd : why would only 12 bytes be parsed out of all the rest ?
As it turns out, those 12 bytes are exactly the contents of the 1st segment of the compressed resource. This segment is uncompressed, and as such there can't really be any errors when decompressing it because it isn't getting decompressed.

The real problem arises when parsing gets to the 2nd segment of the compressed resource ; parsing it yields empty bytes, which explains why only the 1st segment is returned, but also made me wonder why on earth was nothing returned ?
So I dug a little deeper and found out that a ValueError gets raised from LZOCompressedBlock._actual_segment_size, but seems to be handled elsewhere so that instead of stopping execution and printing the stack trace, empty bytes get returned instead.

Besides that, debugging revealed that decompressed_size was 12, but uncompressed seemed to be the right value ; I could find its entierety whithin the uncompressed resource used for testing. Coïncidentally, 12 was also the length of the 1st segment, but why would such a mismatch occur ?
Turns out that context._index is used both for the compressed resource segment index AND the segment block index. This works fine when the resource only has 1 segment, but in our case, it breaks everything ; since context._index is 0 when passed to _actual_segment_size (as it should, since it is beginning to parse the segment), the construct evaluation will go look for the uncompressed size of index 0, which is the size of the 1st segment instead of the intended 2nd.

So I feel it has become quite clear that this is an indexation issue, but now I wonder how I should fix it ? First thing that comes to mind is manually putting a different index for the segment index directly into the context, but this feels a bit hacky, and I'm not 100% certain this wouldn't break Echoes decompression as well.
I found out when debugging _actual_segment_size that manually setting context._index to 1 when evaluating decompressed_size and segment_size, then setting it back to 0 when computing previous_segments on the first pass (second pass doesn't seem to change much, but third and last pass will break things if tampered with, it is supposed to raise construct.StopFieldError as this is effectively the first and last block)

TL;DR : There is a problem with LZOCompressedBlock._actual_segment_size where the same index is used for evaluating decompressed_size and previous_segments while the former should be using the segment index and the latter, the index for the block within that segment, but I'm not sure how to fix it without breaking what's already working for Echoes

[Belokuikuini]

So after digging a little deeper into construct's code I've finally figured out why the context's index gets set back to 0 when parsing the second segment from the compressed resource put in for testing.

That happens because of GreedyRange : when GreedyRange gets parsed (in core.py:2596), context._index gets overwitten from its original value of 1 (which is how IfThenElse evaluated LZOCompressedBlock as the construct to choose in the 1st place) back to 0, as after all, it is the first LZOSegment to be parsed.

What I would do to solve this is implement another class inheriting from GreedyRange that memorises the previous context._index value and uses it to distinguish the segment index and the block index.

On another note, is this an issue worth bringing up on the Construct library's github ? I feel like GreedyRange swallowing the previous index and overwriting it is probably not intended behavior, but maybe that's just because it's inconvenient for me at the moment that I feel that way ?

[duncathan]

does context._._index not work? if not, you can just assign the parent index to a Computed field and access that field

[Belokuikuini]

I had already tried context._._index beforehand, but the field wasn't defined, so I quickly shrugged it off. However, for some reason, when trying to implement the Computed field for the index, context._._index was not only defined, but also at the right value. I have no idea why that is but I'll take whatever I can get at this point lol.

Thanks a bunch, I'll push the fix once I make sure all the tests pass, though the code might be a little messy... but that's what reviews are for :)

[Belokuikuini]

Oh I completely forgot to mention that I was wrapping the Computed field along with the LZOCompressedBlock in a Struct that's probably helpful to know oops.

[Belokuikuini]

So thanks to Dunc's advice, I believe I've managed to fix the decompression issue I've been having : the resources now seem to properly decompress regardless of whether they have 1 or 2 segments.

However, trying to run the command that started this PR (python -m retro_data_structures areas --game CORRUPTION --input-iso "$env:PRIME3_ISO" list-areas) still yields a stack trace, though this one is much less mysterious :

Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\__main__.py", line 6, in <module>
    cli.main()
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\cli.py", line 450, in main
    handle_args(create_parser().parse_args())
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\cli.py", line 443, in handle_args
    do_area_command(args)
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\cli.py", line 406, in do_area_command
    _list_areas_command(args, asset_manager)
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\cli.py", line 363, in _list_areas_command
    world_name = mlvl.world_name
                 ^^^^^^^^^^^^^^^
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\formats\mlvl.py", line 343, in world_name
    return self._name_strg.strings[0]
           ^^^^^^^^^^^^^^^
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\formats\mlvl.py", line 330, in _name_strg
    self._name_strg_cached = self.asset_manager.get_file(self._raw.world_name_id, type_hint=Strg)
                             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\asset_manager.py", line 261, in get_file
    return self.get_parsed_asset(path, type_hint=type_hint)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\asset_manager.py", line 255, in get_parsed_asset
    return format_class.parse(self.get_raw_asset(asset_id).data, target_game=self.target_game, asset_manager=self)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\belok\retro-data-structures\src\retro_data_structures\base_resource.py", line 55, in parse
    return cls(cls.construct_class(target_game).parse(data, target_game=target_game), target_game, asset_manager)
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "c:\Users\belok\retro-data-structures\.venv\Lib\site-packages\construct\core.py", line 404, in parse
    return self.parse_stream(io.BytesIO(data), **contextkw)
                             ^^^^^^^^^^^^^^^^
TypeError: a bytes-like object is required, not 'Container'

I'm fairly certain this is caused by the fact I had to wrap the LZOCompressedBlockCorruption into a Struct so that GreedyRange doesn't eat the segment index and instead keeps it in the parent context.
Though I'm not too sure how could have Construct convert that to a bytes-like object instead of a Container without impacting parsing ?

[duncathan]

I'm fairly certain this is caused by the fact I had to wrap the LZOCompressedBlockCorruption into a Struct so that GreedyRange doesn't eat the segment index and instead keeps it in the parent context. Though I'm not too sure how could have Construct convert that to a bytes-like object instead of a Container without impacting parsing ?

try wrapping it in a FocusedSeq instead of a Struct. that way you should still get the context nesting you need, but the value will just end up being a bytes object

[Belokuikuini]

Just gonna bump this real quick. I've moved CMPD and CMPDAdapter to cmpd.py