Cleanup ov063/daTrsTrap_c: BBH animated-furniture traps - #2709
Conversation
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Byte-verified means the range carries Per-file link-check detailAll 4 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. |
One class, four registry profiles (KAIDAN stairs, TERESAPIT trapdoor, BOOKSHELF, MERRYGOROUND): merge 9 shards into d_a_trs_trap.cpp (5 methods) and d_a_trs_trap_classinits.cpp (4 x return new). Split 3 ways because ROM interleaves state bodies/callbacks between the class's runs; dtors stay single-function. Header gains named fields + leaf operator new; delinks 9 rows -> 2; two new manifest entries license Vector3D1/Matrix4x3D1. linkcheck: 11 checked, 11 verified, 0 blocking. decl-agreement: 2 deliberate typed-vs-slop rows (data_ov063_0211e27c/28c SharedFilePtr*[] vs decl_common int[]), listed not banked. resync --check in sync x2. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
9cac5ee to
e406d2a
Compare
andrewboudreau
left a comment
andrewboudreau
left a comment
There was a problem hiding this comment.
Humanizer review of e406d2aa0d5f6cccd7e801dc356fd228d2e33c93.
TRAP-2709-01 (reconstruction; correct the associated provenance claim), src/game/actors/daTrsTrap_c/d_a_trs_trap.cpp:35-36,63-65,221: the opaque C PMF receiver is unnecessary. On the complete candidate TU, replace:
-struct C;
-typedef void (C::*PMF)();
+typedef void (daTrsTrap_c::*PMF)();
- (((C *)this)->*data_ov063_0211ef38[idx])();
+ (this->*data_ov063_0211ef38[idx])();Under 2004/b56, all 8 emitted function bodies and their relocation offset/type/target records remain identical, with no added or removed function. linkcheck.py on _ZN11daTrsTrap_c8BehaviorEv, ov063 0x0211ce74, size 0x8c, returns VERIFIED, diffs: [], blind: 0. Remove the fake receiver and the statement that the polymorphic class cannot use the ROM representation, then refresh candidate proof.
The exact head also passes port_refcheck.py: 423 checked, zero stale references. This is initial blocking source review, not full-ROM/composition acceptance. Retained compiler constraints need the pinned experiment evidence required by notes/agents/SOURCE-REVIEW-CUTOVER.md; any accepted deferred reconstruction needs its issue and next owner. Current-main composition and independent source acceptance remain required before landing.
Main folded FallBlockBbh into src/game/actors/d_a_obj_th_fall_block.cpp under its ROM RTTI name, while this branch folded daTrsTrap_c; each fold leaves the other's per-function paths stale in config/converted-baseline.json, and git merged that file silently into a set that was neither side's. Both generated baselines are regenerated rather than merged. The converted baseline is rebuilt as the union of both sides intersected with what the tool actually scores today, which drops exactly the seven dead paths (four FallBlockBbh, three per-function daTrsTrap_c) and keeps every live identity; the ratchet then reports 0 orphaned. config/decl-agreement-baseline.json is re-banked from main's copy: the two new contradictions are this branch's own d_a_trs_trap.cpp SharedFilePtr spellings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y4FxdKHG3a6hQRYRAbx29c
TRAP-2709-01. The file bound the four ROM dispatch records to an opaque
forward-declared `struct C` and cast `this` to it, on the stated theory that
a pointer-to-member of the polymorphic daTrsTrap_c does not share the ROM's
{ptr, adj} record shape.
That is false for this TU. Review reproduced the real-class spelling under
2004/b56: all 8 emitted function bodies and their relocation offset/type/target
records are identical, with no function added or removed, and linkcheck on
_ZN11daTrsTrap_c8BehaviorEv (ov063 0x0211ce74, size 0x8c) returns VERIFIED,
diffs: [], blind: 0.
Remove the fake receiver and the representation claim that justified it.
Reverified here: 106/106 exact, mismatching 0, ROM-build analysis PASS.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y4FxdKHG3a6hQRYRAbx29c
|
TRAP-2709-01: fixed in Applied your diff exactly: -struct C;
-typedef void (C::*PMF)();
+typedef void (daTrsTrap_c::*PMF)();
- (((C *)this)->*data_ov063_0211ef38[idx])();
+ (this->*data_ov063_0211ef38[idx])();The opaque receiver and the representation claim that justified it are both gone. The Reverified on this head — full build, not a scoped linkcheck: The Head also carries the merge of current Still open on this PR and not claimed as done: the retained compiler constraints still need the pinned experiment evidence 🤖 Generated with Claude Code |
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
andrewboudreau
left a comment
andrewboudreau
left a comment
There was a problem hiding this comment.
Independent recheck at 1477adc72dd9d2b59c0b9cca5f84524430b37a38: TRAP-2709-01 is fixed. The dispatch table is bound to daTrsTrap_c, the fake C receiver is gone, and the disproved representation claim has been corrected. The additional direct mStateTimer reset also passes the fresh check: all 5 configured methods in d_a_trs_trap.cpp are VERIFIED, with zero warnings/blocking results.
This resolves the specific finding. It is not a whole-candidate Source review pass or current-main composition acceptance. I am coordinating integration and retaining those independent gates before landing the source PR.
TRAP-2709-01 was already fixed on this branch: 395312c removed the opaque `struct C` PMF receiver and the false claim that a polymorphic class cannot use the ROM's representation, and 1477adc unified the mStateTimer spelling. The reviewer confirmed that at 1477adc. Nothing is re-applied here. What was still outstanding from the review was the evidence around what the candidate RETAINS, and two committed claims that had gone stale against it: - notes/experiments/trap-2709-statetimer.md still said the disproved dual spelling was "left in place" and that "the branch comment over-claims". 1477adc had already adopted the unified spelling, so the note contradicted the source it documents. It now records the variant as the shipped form. - d_a_trs_trap.cpp's InitResources comment cited the swapped strh at +0x29c/+0x2a0; the pinned experiment measured +0x2a0/+0x2a4. The comment now matches its own evidence, and trap-2709-v16order.md drops the stale "off by one slot" aside. Comment-only, so it moves no byte. - Every experiment note now states its disposition in the vocabulary notes/agents/SOURCE-REVIEW-CUTOVER.md defines, and the deferred entry names its durable thread (this PR, until a daTrsTrap_c class issue exists), its next owner and the concrete work that unblocks each of its three items. - notes/agents/handoffs/pr-2709-source-review-fixes.md collects the finding -> disposition -> artifact table and the re-proof below. Re-proved here, not taken from the review: - rombuild.py -j16 --no-rom --no-cache: all 8754 enrolled files compiled from source with no cache; 11,207 source-built functions reproducing, 0 mismatching; module fidelity 106/106 exact; ROM-build analysis PASS. Re-run cached after the comment correction: 8753 reused, 1 compiled, same verdicts. - linkcheck.py over all 11 of the class's configured functions, including _ZN11daTrsTrap_c8BehaviorEv (ov063 0x0211ce74, size 0x8c): every one VERIFIED, diffs [], blind 0. - No function added or removed: e406d2a's pre-fix source and this head's source compile, under the build's exact flags, to the byte-identical object (sha256 240001deaa87109e2bdc68ad52c44e805ad1391f7f0ae8c60037981aa20bd8c3, 3648 bytes). It defines exactly the five configured methods, 0x44 + 0x4 + 0x40 + 0x8c + 0x370 = 0x484, exactly the delinks range 0x0211cdec..0x0211d270. - check_dead_references.py, check_decl_agreement.py and check_src_tu.py clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1Gdj5fTjMsW2VjRLpXxYA
andrewboudreau
left a comment
andrewboudreau
left a comment
There was a problem hiding this comment.
TRAP-2709-01: resolved. Verified on the current head, not on the response text.
d_a_trs_trap.cpp:63 now reads typedef void (daTrsTrap_c::*PMF)(); and the
dispatch at line 228 is (this->*data_ov063_0211ef38[idx])(). The opaque
struct C forward declaration and the ((C *)this) receiver cast are gone, and
the comment now states the records are "bound to this trap's own class" — so the
claim that the polymorphic class could not use the ROM representation is
withdrawn along with the construct it justified. The one remaining "cannot" in
the file is about block-scope declarations in a C++-named member, which is a
different and correct statement.
PR validation is green. review and Source review are red, but both are
advisory: only PR validation is a required check on main, and the review
queue has been dormant since 2026-09-11, so those reds carry no information
about this PR.
Clearing my block. As my original review said, this was an initial blocking
source review rather than whole-PR acceptance — this approval resolves the
finding I raised and does not certify the remaining deferred items.
Resolved on each file's key structure, not textually. config/converted-baseline.json (the only textual conflict) Merged as a SET, not as lines. base=2812, ours=2812 (+3 -3), theirs=2813 (+6 -5); no identity is added by one side and removed by the other. Result 2813 = 2812 + 3 + 6 - 3 - 5, with count == list length == deduplicated length == 2813, re-emitted in tiers_ratchet.write_baseline()'s exact form (sorted, indent=2, LF). Main's copy had the four daObjCtKaitendai_c rows inserted out of order; sorting restores the canonical shape --update produces. config/decl-agreement-baseline.json (git auto-merged; audited anyway) Re-merged per key over known[symbol][file]: base=13413, ours=13413, theirs=13385 entries -> 13385 merged (13280 unchanged, 4 ours-only, 215 theirs-only), zero keys both sides moved differently, zero entries matching neither side. The 4 ours-only keys are exactly this PR's shard-to-TU move. Key-for-key identical to git's textual result (0 keys either-only, 0 value differences), so the textual merge was correct here; the file is re-serialized in write_baseline() form, which also re-sorts the 7 symbols main left unsorted. attribution.json (git auto-merged; audited anyway) This branch never touched it, so the merge is main's file verbatim. Proved as a key union regardless: base=3422, ours=3422, theirs=3437, merged=3437 == |ours union theirs|, with nothing lost from either side, no value changed, and no key from nowhere. config/arm9/overlays/ov063/delinks.txt (git auto-merged; audited anyway) Both sides edit this file in disjoint address ranges -- this PR folds daTrsTrap_c at 0x0211cdec.., main folds daTrs_c at 0x0211ae40... Audited as blocks: 131 = 141 base + 2 ours + 3 theirs - 9 ours - 6 theirs, every kept block's body equal to one side's, and the 127 .text ranges still tile 0x02115ee0-0x0211e1c0 with zero gaps and zero overlaps. symbols/actor_renames.tsv Append log, touched only by main (3620 -> 3626 rows); this branch's copy is identical to the merge base, so the merge is main's file byte for byte. No row lost, none duplicated. No modify/delete conflict arose, and none was hidden: for all 9 files this PR deletes, main's blob is identical to the merge base's, and no daTrsTrap_c path appears anywhere in main's diff, so accepting the deletions reverts nothing of main's. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1Gdj5fTjMsW2VjRLpXxYA
The promotion folds these functions into the surviving TU, and attribution.json was given one owner for all of them -- the surviving file's file-level owner -- so every function a different contributor matched lost its credit. Each row below restores the owner the attribution gate reports for that function at the merge base. Eight are tangosdev; OnPendingDestroy is andrewboudreau. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G1Gdj5fTjMsW2VjRLpXxYA
Restored the credit this promotion droppedFolding the per-function sources into the Measured with the repaired gate against the merge base:
The owners below are the gate's own prior-owner column, not
Every row is a member-level 🤖 Generated with Claude Code |
2 participants
Deslops the Big Boo's Haunt animated-furniture traps (one class, four registry profiles): 9 shards merged into 2 real files + 2 kept dtor shards, named fields, typed resource rows, 4x
return newfactories,deslop+Leftover:markers.Stack: on origin/main, standalone. Note: touches ov063/delinks.txt adjacent to #2705 (BBH) rows — rebase order may matter at merge time.
Made with Cursor