Enemy/DebuTelesa, System/MarNameRefGen_Map: include InfectiousStrings - #156
Open
KakarottoCake wants to merge 1 commit into
Open
Enemy/DebuTelesa, System/MarNameRefGen_Map: include InfectiousStrings#156KakarottoCake wants to merge 1 commit into
KakarottoCake wants to merge 1 commit into
Conversation
Both units' retail objects carry the four `MActorMtxCalcType_*` strings and the `DummyStrings.hpp` pair ahead of them, byte for byte, and neither of our sources emitted them. `M3DUtil/InfectiousStrings.hpp` already exists for exactly this and pulls in `System/DummyStrings.hpp` itself, in that order. DebuTelesa's `.rodata` goes from 241 bytes of missing compiler-generated constants to 24. MarNameRefGen_Map's drops to none. **No change to matched functions or matched data** -- 8131 and 353,763 both before and after. This is a source-fidelity fix, not a scoring one. It closes the head of those two `.rodata` sections so that whatever is fixed there next is not sitting at the wrong offset. On picking the units: the retail MAP lists `MtxCalcTypeName` for 181 `.cpp` files, but that is the wrong list to work from. 164 of those are `UNUSED` with a `........` address -- dead-stripped, so absent from the extracted objects that objdiff actually compares against. Emitting the strings in those units would put bytes in our `.rodata` that the target can never have. The right list is the 142 units whose retail *object* still carries them, and the existing 87 carriers in our tree agree: 86 of them are in that set. That leaves 56 units still to do. 54 of them have pre-existing `check-changed-symbol-order.py` failures on unmodified main, so touching them would redden a PR over breakage they already had -- I have left those alone rather than bury this in unrelated red. These two are the ones that are clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two units whose retail objects carry the four
MActorMtxCalcType_*strings and theDummyStrings.hpppair ahead of them, byte for byte, where our sources emitted neither.M3DUtil/InfectiousStrings.hppexists for exactly this and pulls inSystem/DummyStrings.hppitself, in the right order.DebuTelesa's.rodatagoes from 241 bytes of missing compiler-generated constants to24.
MarNameRefGen_Map's drops to none.Matched functions and matched data are unchanged -- 8131 and 353,763 either way. This
is source fidelity, not a scoring change. It closes the head of those two
.rodatasections so the next thing fixed in them is not sitting at the wrong offset.
Why only two
The header's TODO asks which TUs should carry these. The retail MAP answers it for 181
.cppfiles -- but that is the wrong list to work from, and I want to write down whybefore someone else does the obvious thing:
164 of those 181 are
UNUSEDwith a........address. They were dead-stripped, sothey are absent from the extracted objects in
build/GMSJ01/obj/that objdiff actuallycompares against. Adding the include to those units puts bytes in our
.rodatathat thetarget can never contain. I tried it on 32 units first and measured it: every one of them
ended up emitting strings its target does not have.
The right list is the 142 units whose retail object still carries them. The tree
already agrees with that rule without anyone having written it down: of the 87 files that
currently carry these constants, 86 are in that set.
That leaves 56 units genuinely still to do, not 106.
Why not all 56
54 of them fail
check-changed-symbol-order.pyon unmodifiedmain. CI only runs thatcheck on changed
.cppfiles, so those failures are dormant until someone touches thefile -- and a PR adding one include line to 54 of them would come back red for breakage it
did not cause. I would rather not bury a one-line change in unrelated red, so this is the
two that are clean.
Happy to do the other 54 in whatever form you prefer -- all at once with the symbol-order
noise explained, in batches, or after the ordering issues are dealt with separately. There
are 331 units failing that check tree-wide at the moment, so it is not specific to these.
mario.dol: OK. Symbol order passes on both changed files. Whole-tree clang-format clean.