Skip to content

Use int as address for in-memory report - #493

Merged
disinvite merged 4 commits into
isledecomp:masterfrom
disinvite:report-int-addr
Jul 25, 2026
Merged

Use int as address for in-memory report#493
disinvite merged 4 commits into
isledecomp:masterfrom
disinvite:report-int-addr

Conversation

@disinvite

Copy link
Copy Markdown
Collaborator

JSON and HTML report files expect a hex string for each address. We also use this to print to the terminal.

Instead of converting the int address when the in-memory report is created, delay this until we need to write a file or display something. (In the spirit of #307.)

The sticking point is the magic string "various" that appears for the recomp addr when the entity is created by reccmp-aggregate. We have a new property for this to distinguish between the recomp addr being None because this is an unmatched entity or because it is matched but has no fixed recomp address. We don't currently use the report to hold unmatched entities, but I want to do this in a follow-up to fix some gaps with accuracy/progress calculation. (See: #444 and #447.)

We did not have any tests specific to the version 1 report format, so I added some.

@jonschz jonschz left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good to me. I also didn't find any regression when trying reccmp-reccmp --json and reccmp-reccmp --diff, though I didn't test a lot of scenarios.

Comment thread reccmp/compare/report.py Outdated
Comment thread reccmp/compare/report.py
Comment thread tests/test_report_version_1.py
Comment thread reccmp/compare/report.py Outdated
@disinvite
disinvite merged commit 40a6c67 into isledecomp:master Jul 25, 2026
18 checks passed
@disinvite
disinvite deleted the report-int-addr branch July 25, 2026 21:59
@disinvite disinvite mentioned this pull request Jul 25, 2026
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