Skip to content

Fix plLocation encoding/decoding to match Plasma, and add tests.#311

Merged
zrax merged 2 commits into
H-uru:masterfrom
zrax:fix_location_encoding
Nov 1, 2025
Merged

Fix plLocation encoding/decoding to match Plasma, and add tests.#311
zrax merged 2 commits into
H-uru:masterfrom
zrax:fix_location_encoding

Conversation

@zrax
Copy link
Copy Markdown
Member

@zrax zrax commented Oct 23, 2025

No description provided.

@dgelessus
Copy link
Copy Markdown
Contributor

I looked into this a couple of years ago when implementing sequence number parsing in my NAGUS project, and also came to the conclusion that libHSPlasma was wrong :D If you want to compare notes (only for the MOUL format): these are my test cases (all taken from real ages currently in MOULa) and here's the text explanation of how I understand the encoding.

At first glance, your code looks right to me. I'll try to take a detailed look at the logic soon.

Comment thread core/PRP/KeyedObject/plLocation.cpp Outdated
Comment thread core/PRP/KeyedObject/plLocation.cpp Outdated
@zrax zrax force-pushed the fix_location_encoding branch from e34d801 to b438f39 Compare October 31, 2025 22:54
@zrax
Copy link
Copy Markdown
Member Author

zrax commented Oct 31, 2025

Good suggestion about using real PRP locations as test values... Despite modeling the tests after what Plasma's plAgeDescription::CalcPageLocation() does, this would have been even more wrong than before. Clearly, that function is not used in the way I think it is with negative prefixes.

@zrax zrax force-pushed the fix_location_encoding branch from b438f39 to 3347d0a Compare October 31, 2025 23:07
@zrax zrax force-pushed the fix_location_encoding branch from 3347d0a to 24a6b8e Compare October 31, 2025 23:22
Copy link
Copy Markdown
Member

@dpogue dpogue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

I guess Plasma itself is now the thing lacking unit tests for plLocation compatibility...

@zrax zrax merged commit 083742b into H-uru:master Nov 1, 2025
8 checks passed
@zrax zrax deleted the fix_location_encoding branch November 1, 2025 17:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants