OutpostUniverse / OP2Utility

C++ library for working with Outpost 2 related files and tasks.
MIT License
4 stars 0 forks source link

Fix decompression bug #151

Closed DanRStevens closed 6 years ago

DanRStevens commented 6 years ago

The mod field was being calculated after the offset value it relied on was modified. Moved the calculation of mod up to before offset was modified.

DanRStevens commented 6 years ago

I'm thinking we should merge this before merging PR #147. If it works for you, feel free to do the honours.

Brett208 commented 6 years ago

Thanks for fixing the bugs I introduced.

I think adding those variables makes the code a little easier to understand. Reading though the source code I think the mod variable or the subfunction to calculate it subfunction could use some explaining (coming from a programmer with no background in compression) to ease understanding when reading through the sourcecode.

I tested by extracting a compressed sheets.vol. No issues noted.

DanRStevens commented 6 years ago

Better comments would be nice. One problem though being, I don't really understand that method either. It was written using the disassembly from the game as a guide.

From a high level, I know that section of code calculates an offset into the repeat buffer, and I know it uses a variable length code to determine that offset. It's biased such that small offsets (backwards from the current output position) have shorter codes. That seems to make sense in that repeated data is often close to each other.

As for why particular constants were chosen, or why the offset is calculated as it is, I'm afraid I don't have answers there. I suspect it's related to Huffman codes, but using a fixed code table rather than adaptive codes for that section. Even that though, I'm uncertain of. I would need to review Huffman codes to see if the implementation actually matches.