Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Truncate arrays if they are longer in recomp #79

Merged
merged 3 commits into from
Jan 27, 2025

Conversation

disinvite
Copy link
Collaborator

@disinvite disinvite commented Jan 25, 2025

_match_array_elements adds offset entities for each array variable. We need this if array entries are referenced by pointers in other variables because datacmp only checks the database for a pointer match. (e.g. g_skBMap and g_skeletonKickPhases)

This function assumes the arrays are the same size in orig as in recomp. This is not always true for BETA10 and so the offsets we generate will clash with the ones from the following variable. We can detect when this occurs by looking at the next orig address. If recomp is larger, we add the extra offsets on the recomp side only and not link them to orig.

You can use roadmap --verbose to see what changed. I'll attach the BETA10 diff here. No change to LEGO1.
truncate-array-roadmap-beta10.txt

IIRC the ghidra scripts do not reference these offsets and use the cvdump data directly to build out the types. Is that right @jonschz?

@disinvite disinvite requested a review from jonschz January 25, 2025 21:59
Copy link
Collaborator

@jonschz jonschz left a comment

Choose a reason for hiding this comment

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

Looks good! You are correct, these array elements are not used by the Ghidra import. I originally added them because datacmp would raise errors on correct data.

reccmp/isledecomp/compare/core.py Outdated Show resolved Hide resolved
reccmp/isledecomp/compare/core.py Outdated Show resolved Hide resolved
@disinvite
Copy link
Collaborator Author

Thanks! Let me know how it looks now. I had to add the dreaded # pylint: disable=too-many-lines to the file, but I have some upcoming changes planned that will shrink this back down.

@jonschz
Copy link
Collaborator

jonschz commented Jan 27, 2025

looks good!

@disinvite disinvite merged commit 5db929e into isledecomp:master Jan 27, 2025
11 checks passed
@disinvite disinvite deleted the truncate-array branch January 27, 2025 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants