ov063: give daTBasket_c and daTrsTrap_c their ROM names and real vtable slots - #1978
Merged
Conversation
…le slots The cartridge names both classes and never named the coined ones: config/arm9/overlays/ov063/symbols.txt carries _ZTI11daTBasket_c / _ZTS11daTBasket_c and _ZTI11daTrsTrap_c / _ZTS11daTrsTrap_c, but no _ZTI7BooCage, _ZTS7BooCage, _ZTI12MansionSteps or _ZTS12MansionSteps has ever existed. `BooCage` and `MansionSteps` were actor names standing in for class names, and `_ZTV7BooCage` / `_ZTV12MansionSteps` were aliases mapped onto the real vtables' own addresses (0x0211e930, 0x0211ea88). daTrsTrap_c settles it independently: Bookshelf, MerryGoRound, TrapDoor and MansionSteps all store that one vtable, so a one-actor spelling could never have been the class. Both headers also carried a fake derived class -- `struct BooCage : daTBasket_c` and `struct MansionSteps : daTrsTrap_c` -- declaring InitResources/CleanupResources/Behavior/Render (and OnPendingDestroy) NON-virtual. Overrides parked on a subclass fill no slot, so the emitted vtables kept fBase_c's generic entries in 0/3/6/9/12 while the ROM has ov063 code there. That is the whole of the reported difference: 4 slots = 16 bytes for daTBasket_c, 5 slots = 20 bytes for daTrsTrap_c. Dissolving the fake subclasses and declaring the overrides on the real classes is the fix; the destructor stays first-declared, because it is the ABI key function that picks the TU emitting _ZTV. Per the ov018 MotherPenguin/daPgMthr_c precedent, BooCage_Spawn, BooCage_SpawnInfo, MansionSteps_Spawn and MansionSteps_SpawnInfo keep the coined actor names -- those name the actor, not the class -- and no directories move. Also refreshes symbols/actor_renames.tsv, which cpp_rename.py loads as an addr -> current-name map and which was already stale for the four destructors (it still claimed _ZN7BooCageD1Ev where the tree has _ZN11daTBasket_cD1Ev). Content only; the file renames follow in the next commit so git records them as pure renames and attribution lineage survives.
Pure rename, no content change, so git records R100 and attribution lineage follows the files. Directories keep the coined actor names, as does ov018 for MotherPenguin/daPgMthr_c.
…t gate Every file that includes this header is .cpp -- the #else branch had no consumer. It did have a cost: once both preprocessor branches carried the same struct tag, check_header_offsets stopped resolving the whole struct and fell back to the 0xd4 prefix. Both headers now check 7 field offsets each against the ROM-observed values and span the sizes the spawn functions allocate: include/daTBasket_c.h 7 commented fields, 0 mismatched, spans 0x380 include/daTrsTrap_c.h 7 commented fields, 0 mismatched, spans 0x354 On main both reported 0 commented fields, so the gate was checking nothing. Byte-neutral: rombuild still 106/106 exact, 11,082 reproducing / 0 mismatching.
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Byte-verified means the range carries Warnings: 5 linkcheck result(s) have unresolved relocations. Per-file link-check detailAll 1148 changed file(s) compile to the ROM byte-for-byte with correct relocation targets.
The private worker commits a test merge, builds the stock ROM profile, compares every executable module, measures matched and source-built code, checks contributor lineage, and verifies affected relocations. The mod profile is opt-in and is not part of this merge gate. |
These were left behind when the RTTI recovery moved daTBasket_c and
daTrsTrap_c into their own headers. Nothing includes either file -- all
eleven source files under src/actors/BooCage/ and src/actors/MansionSteps/
include daTBasket_c.h and daTrsTrap_c.h instead -- so they were invisible
to every gate while still carrying the exact shape this PR exists to
disprove:
struct BooCage : daTBasket_c { int InitResources(); int Render(); ... };
struct MansionSteps : daTrsTrap_c { int Behavior(); ... };
Those are new non-virtual members on a fake subclass. They fill no slot in
the base's vtable, which is why _ZTV11daTBasket_c kept inherited entries
where the ROM has overlay code. They also each redefine the real class in
full, with a duplicate sizeof assert -- a second, divergent copy of a
layout that now has one authenticated home.
Deleting rather than renaming, per the ov018 MotherPenguin/daPgMthr_c
precedent: the actor's coined name survives only in *_Spawn/*_SpawnInfo
and the src/actors/<Coined>/ directory names, both of which are unchanged.
Byte-neutral: zero includers, so no translation unit changes.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ
One conflict, config/converted-baseline.json. main re-banked the whole
CONVERTED set (1957 -> 2567 paths) while this branch renamed BooCage ->
daTBasket_c and MansionSteps -> daTrsTrap_c. Resolved by taking main's
re-banked set and applying this branch's five renames to it, so the
banked paths name files that exist:
BooCage/_ZN7BooCage16CleanupResourcesEv.cpp
-> BooCage/_ZN11daTBasket_c16CleanupResourcesEv.cpp
BooCage/_ZN7BooCage6RenderEv.cpp
-> BooCage/_ZN11daTBasket_c6RenderEv.cpp
MansionSteps/_ZN12MansionSteps16CleanupResourcesEv.cpp
-> MansionSteps/_ZN11daTrsTrap_c16CleanupResourcesEv.cpp
MansionSteps/_ZN12MansionSteps16OnPendingDestroyEv.cpp
-> MansionSteps/_ZN11daTrsTrap_c16OnPendingDestroyEv.cpp
MansionSteps/_ZN12MansionSteps6RenderEv.cpp
-> MansionSteps/_ZN11daTrsTrap_c6RenderEv.cpp
Count stays 2567 -- the renames are one-for-one, nothing banked was
dropped and nothing new was banked. `tools/tiers_ratchet.py --check`
passes (baseline 2567, current 2567).
config/match_attempts.jsonl and config/match_provenance.jsonl merged
cleanly: main touched no line this branch edited, so the union driver
never fired and no row was duplicated. Verified the merged files carry
zero `_ZN12MansionSteps` names and the same duplicate-key counts as
both parents (6 in attempts, 21 in provenance, all pre-existing).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ
andrewboudreau
added a commit
that referenced
this pull request
Aug 30, 2026
…het on main (#2011) Main has been out of sync since #1996 merged. `config/converted-baseline.json` banked `src/_ZN12daObjAbuku_cD0Ev.cpp` and `src/_ZN12daObjAbuku_cD1Ev.cpp`, which that PR deleted when it promoted the ov002/daObjAbuku_c translation unit. Nobody's checks were wrong; they just never overlapped. #1996's green ran against a base predating #2007's re-bank, which is what first banked those two files. By the time #1996 merged, the paths it deletes were in the baseline it never re-read. The next unrelated PR to touch src/** -- #1978, which has nothing to do with ov002 -- is the one that went red. The removal is legitimate and is banked with a reason, not reverted: MOVED -- absorbed into src/actors/daObjAbuku_c.cpp by TU promotion (ov002/daObjAbuku_c), which fails: No raw offset arithmetic; No unk_<off> fields; Calls things by real names, not mangled _Z A reconstructed TU must spell vague-linkage symbols directly (_ZN7fBase_cnwEj, _ZN8dActor_cC2Ev, _ZN8dActor_cD2Ev) or its range will not link, so `no_mangled_refs` structurally cannot pass for an absorbing file. Byte-match outranks readability. No source changed and no byte moved. Also adds a `push: [main]` trigger. The workflow's design note claimed staleness "only ever runs one way" -- permissive, never falsely red. TU promotion is the exception, because it REMOVES banked paths, and the note now says so. The trigger gates nothing (no branch protection; a red main is advisory) but it attributes the breakage to the merge that caused it instead of to whoever opens the next PR. tiers_ratchet --check PASS baseline 2565 current 2565 pytest tools/test_tiers.py 29 passed check_dead_references no new dead references Claude-Session: https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 30, 2026
andrewboudreau
added a commit
that referenced
this pull request
Aug 30, 2026
Resolved: - attribution.json: key union of both overrides blocks (878 keys, no key or value lost from either side). - notes/cpp-tu-current-state.md: regenerated with tools/cpp_tu_state.py --write-note; it now names both main's daKpFr_c.cpp and this branch's ActorDerived.cpp as complete multi-function owners. Took main's naming for the #1978 ov063 RTTI renames (MansionSteps -> daTrsTrap_c, BooCage -> daTBasket_c), the #1991 daDsnBase_c slots and the #2000 daKpFr_c TU promotion. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ
andrewboudreau
added a commit
that referenced
this pull request
Aug 30, 2026
Resolved: - attribution.json: key union of both overrides blocks (903 keys; nothing lost from either side, no value disagreements). - notes/cpp-tu-current-state.md: regenerated with tools/cpp_tu_state.py --write-note. Main's naming wins throughout (#1978 ov063 RTTI renames, #1991 daDsnBase_c, #2000 daKpFr_c, #2002 daKrpa_c, the WingFeather move under src/game/actors/). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ
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
1 participant
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.
Clears 4 of the 15 symbols
tools/romdata_check.pyreports as differing, all in ov063.The defect
symbols.txtcarried two names for one vtable address, twice:include/BooCage.hdeclaredThose are new non-virtual members on a subclass, not overrides. They fill no slot in
daTBasket_c's vtable, so the emitted_ZTV11daTBasket_ckeptfBase_c's genericinherited entries at exactly the offsets where the cartridge has ov063 code.
MansionSteps.hhad the same shape. Same family as the undeclared-override defectfixed in #1970 — an override the ROM has that nothing declares on the real class.
Every byte gate stayed green throughout, because
objisolatediscards non-.textsections:
106/106 exactsays nothing about_ZTVbytes.romdata_check.pyis theonly gate that reads them.
Which name is real
RTTI is decisive.
_ZTI11daTBasket_c/_ZTS11daTBasket_cand_ZTI11daTrsTrap_c/_ZTS11daTrsTrap_cexist in ov063; no_ZTI7BooCageor_ZTI12MansionStepsever did.The cartridge names these classes, and the coined spellings were only ever the actors'
names.
Independent second proof for
daTrsTrap_c: four spawn functions store_ZTV12MansionSteps—Bookshelf_Spawn,MerryGoRound_Spawn,TrapDoor_Spawn,MansionSteps_Spawn. A one-actor spelling could never have been the class name.The differing byte counts are the slot counts, exactly: daTBasket_c 16 bytes = slots
0/3/6/9; daTrsTrap_c 20 bytes = slots 0/3/6/9/12, per
include/fBase_c.h:130-142.What changed
Both fake derived classes dissolved; the five slots redeclared as real virtual overrides
on the real class, with the destructor still declared first (it is the ABI key function
and picks the TU that emits
_ZTV). The two coined_ZTValiases deleted fromsymbols.txtanddecl_common.h— deleted, not renamed, since the real lines alreadyexisted at those addresses. Ten files moved onto their ROM class names as a pure
git mvin its own commit, so lineage records R100.BooCage_Spawn,MansionSteps_Spawn, both_SpawnInfo, and thesrc/actors/BooCage/+src/actors/MansionSteps/directory names are keptdeliberately: those name the actor, not the class, exactly as ov018 keeps
MotherPenguin_SpawnfordaPgMthr_c. No directories move.Two pre-existing defects fixed on the way
symbols/actor_renames.tsvwas already stale on main, claiming_ZN7BooCageD1Evand
_ZN12MansionStepsD1Evwhere the tree had theda*_cspellings. This file is notan audit log —
tools/cpp_rename.pyloads it as an addr → current-name map, so a stalenewcolumn is a live hazard for the next rename. 15 rows corrected.check_header_offsetswas reporting 0 commented fields on both headers, i.e.checking nothing.
daTrsTrap_c.h's dead#ifdef __cplusplusC view made the parserfall back to a 0xd4 prefix span. Every includer is
.cpp, so the dead view is gone andboth headers now check 7 fields each against the correct 0x380 / 0x354 spans.
Measured
port_refcheck405, all resolve.check_src_tu_compiles88/88. No new dead references.Exactly the four expected symbols cleared, zero new differing symbols introduced.
twin_align: FAILonWorkElevator.hreproduces on main'sinclude/— pre-existing,untouched here.
🤖 Generated with Claude Code
https://claude.ai/code/session_01WdCK1xgrJdiJzPCh3bAJfQ