Declare dScMgBase_c slot 31 (Virtual7C) across the minigame family - #2106
Conversation
Slot 31 of the 18-35 keystone range. Thirty-two vtables gain their
thirty-second word; seven override it, three of those need a declaration
(dScMgD3DBase_c, dScMgAmida_c, dScMgSmartball_c) and the other four --
dScMgJump_c, dScMgJump2_c, dScMgTrampoline_c, dScMgTrampoline2_c -- all
inherit dScMgD3DBase_c's body. No occupied-slot trap.
romdata_check vs slot 30: verifiedBytes 35412 -> 35416 (+4)
partialBytes 11624 -> 11748 (+124)
128 bytes = 32 tables x one word, split the usual way: _ZTV14dScMgD3DBase_c
is the one table already VERIFIED so its word lands in the verified pool.
Every count -- differs 6, partial 253, verified 465, symbols 1246,
totalRecords 7560 -- is unchanged, and no symbol changed verdict.
rombuild -j16 --no-rom: 11,088/11,088 reproducing, 106/106 exact,
100.000000% of compared bytes, ROM-build analysis PASS.
THE NAME IS DELIBERATELY A NON-NAME, AND REACHING THIS SLOT IS WHY
Slots 18-30 carry include/dActor_c.h's names because they sit at dActor_c's
indices. That was the whole of the argument, and at 31 it runs out:
dActor_c.h stops at slot 30. Reaching the end of the borrowed table is what
finally makes the borrowing legible.
fBase_c -> dBase_c -> dActor_c (adds slots 18-30)
fBase_c -> dBase_c -> dScene_c -> dScMgBase_c (adds slots 18-35)
include/fBase_c.h:130-146 declares slots 0-15 plus the destructor pair at
16/17. dBase_c adds no virtual of its own. dScene_c declares nothing above
slot 11. So the two branches are SIBLINGS that each begin appending at 18 --
dScMgBase_c is a scene, not an actor, and it is not a dActor_c descendant at
any depth. Two independent extensions of the same 18-slot base land on the
same indices for the same reason two books have a page 19. Nothing was
inherited; the indices merely coincide.
That is the single root cause behind three findings this campaign recorded
separately: the ROM contradicting the name at slot 26, at 29, and at both
halves of it at 30. It applies to all eighteen, not to those three.
Whether to retro-rename 18-30 is not this commit's call. What this commit
will not do is add a nineteenth borrowed name on top of the explanation.
WHERE `Kill` CAME FROM
The bodies said `Kill`, and ov002 really does carry _ZN10dBgActor_c4KillEv
at 0x020ee55c -- a genuine mangled ROM symbol. But dBgActor_c derives FROM
dActor_c: it is a nephew of this branch, one fork further away still, and
Kill is the one new virtual it appends at ITS slot 31. The name was carried
across a fork, to an index that does not even exist on the branch it was
taken from. Nothing in the cartridge names this function -- its RTTI carries
class names only.
Virtual7C is what fBase_c already spells for exactly this situation
(Virtual34, Virtual38), what the family spells at Virtual50/08/10/18, and
what slots 33, 34 and 35 are already recorded under in
notes/dScMgBase_c-slots-18-35.md. It costs no new convention and it is free
tree-wide. `_ZN11dScMgBase_c9Virtual7CEv`.
WHAT THE SLOT ACTUALLY DOES
All four bodies are one shape. Three read-modify-writes on the sub engine's
BG1CNT at 0x0400100a (`& 0x43`, `| X`, `& ~0x40`, `& ~3`) leave the register
holding exactly X -- priority 0, no mosaic, and the class's own base bits:
0x10 in the base, 0x800 in dScMgD3DBase_c, 4 in both dScMgAmida_c and
dScMgSmartball_c. Then SetSubBg1Offset(0, 0) resets the layer's scroll,
`data_0209d454 &= ~2` clears BG1's bit in the sub BG-enable shadow that slot
30 restores the sub DISPCNT from, and two LoadCompressedFileAt calls install
a language-indexed character file at G2S::GetBG1CharPtr() and the shared
screen map, file 0x5b, at G2S::GetBG1ScrPtr(). dScMgD3DBase_c wraps that in
the VRAM bank juggling only the 3D minigames need.
It builds this minigame's touch-screen background. It destroys nothing, which
is the second reason not to keep the name.
BeforeInitResources (ov004:0x020b0930) calls it last and returns a literal 1
without touching the result. Setup, at scene construction.
ARITY AND RETURN TYPE, MEASURED
No explicit parameters. Scanning arm9 and all 103 overlays for the dispatch
PAIR -- `ldr rN,[rM,#0x7c]` immediately followed by `blx rN`, not any load at
+0x7c -- finds 39 sites, of which exactly one is in ov004 or ov006:
ov004:0x020b0a0c, inside BeforeInitResources. It reads
`mov r0,r4; ldr r1,[r0]; ldr r1,[r1,#0x7c]; blx r1`, so r1 is the loaded
pointer and cannot also be a second argument. The other thirty-eight are in
ov002, ov015, ov018, ov027, ov064, ov079, ov081 and ov098 -- dBgActor_c's
Kill, on the other branch, which is a further check on the fork above.
Return type `int` is a HINT, the fourth consecutive slot no body pins. The
base sets r0 to #0 on its second instruction -- as SetSubBg1Offset's first
argument, before anything reads it -- and never assigns a result. The one
caller discards it.
ALSO IN THIS CHANGE
* Ninth `recovered name:` correction, eighth on dScMgD3DBase_c, and the
first wrong in BOTH halves: ov006:0x020e72c0 was labelled
dScMgTrampoline2_c_Kill and included that header. Five vtables reference
the address, so the class is dScMgD3DBase_c's; the include and the `this`
cast move with it. Each wrong line is kept visible with the correction
beside it.
* Prose re-counted in dScMgBase_c.h (tables now stop at 31),
dScMgD3DBase_c.h (one undeclared slot left, 33), dScMgSingle3DBase_c.h
(32-slot prefix) and dScMgAmida_c.h, where Unk36 moves from 31 to 32 --
one index closer to its true slot 36. _ZTV12dScMgAmida_c stays DIFFERS
until slot 35 lands; that is expected and unchanged.
* A deferred cleanup is recorded rather than done. The base bodies for
slots 31, 32 and 33 all live in one src_tu candidate unit,
src_tu/actors/unit_ov004_020b0a38.cpp, which still names them
func_ov004_*. Nothing reads it today -- the unit is "text-verified", not
enrolled, src_tu/ is not in the ROM build, check_src_tu_compiles is 97/97
green and tiers_ratchet only follows "promoted" entries -- and it will go
stale again at 32 and at 33. Regenerate it once, through tubuild, after
slot 33.
Gates: check_dead_references clean, port_refcheck 405/405,
check_header_offsets --changed origin/main 7 headers 0 mismatched,
check_src_tu_compiles 97/97, check_duplicate_sources 11038 stems none
doubled, langmode ratchet PASS.
Four slots left: 32-35.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh
✅ PR validation — PassedCommitted merge passes; 1 contributor credit reassignment(s) noted, not a blocker. Full merge validation
Contributor credit moved (1)
Byte-verified means the range carries Warnings: contributor attribution changed, not a blocker (1 changed, 0 lost -- src/_ZN11dScMgBase_c9Virtual7CEv.c: tangosdev -> andrewboudreau). Per-file link-check detailAll 193 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. |
|
Self-review. I wrote this, and this repo has one shared account so no PR here can be APPROVED by a second identity — signing off in a comment instead, with what I actually checked and what I could not. Verified on the merge tree, not just the branch. Byte evidence. What I checked hardest, because the naming claim is the load-bearing part. The sibling-branch argument is five independent header reads, not an inference from one: One correction to my own earlier work, stated plainly: slots 26, 29 and 30 each shipped with a note saying "the ROM contradicts this name." Those notes are right but they undersold it — the names do not apply to any of slots 18-30, for a structural reason, not a per-slot one. The prose in those headers stands; the general conclusion is new here. What this PR does NOT establish, and I want it on the record:
A gate defect I found while checking this one, and did not act on. The langmode ratchet's baseline is supposed to live on I am not deleting the root file in this PR — doing so would turn this stack red on a technicality unrelated to it, and the file appears to be regenerated by a bot, so the real fix is in that job, in its own tools-only PR. Flagging it so it is not discovered later as a surprise. Landing stays with you. |
andrewboudreau
left a comment
There was a problem hiding this comment.
Reviewed at 723cec8ec. Two asks, one of them the same systemic issue that already holds #2099, #2100 and #2102, plus one thing I want to say plainly in your favour.
The Virtual7C name is right, and I am not going to argue with it
I want this on the record because a placeholder name usually draws a block from me. This one should not.
Virtual7C is a deliberate non-name, the header says so, and the reasoning is correct: _ZN11dScMgBase_c9Virtual50Ev is already in tracked config on origin/main, and fBase_c declares Virtual34 / Virtual38 there too. Slot 31 is vtable+0x7C, so the spelling follows the landed convention exactly and invents no new one.
It is also the honest option. "Nothing in the cartridge names this function" is the sentence that makes this safe — a placeholder that admits it is a placeholder cannot later be mistaken for a ROM-proven identity, which is the failure mode I keep having to chase. The Kill-came-from-a-nephew derivation (dBgActor_c is under dActor_c, so Kill crossed a fork rather than descending a chain) is the good kind of argument: it explains the wrong answer instead of just deleting it.
The arity measurement is real work and it holds up — 39 ldr rN,[rM,#0x7c] + blx rN dispatch pairs across arm9 and 103 overlays, exactly one in ov004/ov006, and r1 is the loaded pointer so it cannot also be a second argument.
1. Four new header/definition return-type disagreements
The four headers declare:
include/dScMgBase_c.h:648 virtual int Virtual7C(); /* slot 31 */
include/dScMgD3DBase_c.h:100 virtual int Virtual7C();
include/dScMgAmida_c.h:58 virtual int Virtual7C();
include/dScMgSmartball_c.h:77 virtual int Virtual7C();
All four definitions return void:
src/_ZN11dScMgBase_c9Virtual7CEv.c:28 void _ZN11dScMgBase_c9Virtual7CEv(void)
src/_ZN14dScMgD3DBase_c9Virtual7CEv.c:44 void _ZN14dScMgD3DBase_c9Virtual7CEv(char *c)
src/_ZN12dScMgAmida_c9Virtual7CEv.cpp:20 extern "C" void _ZN12dScMgAmida_c9Virtual7CEv(void)
src/_ZN16dScMgSmartball_c9Virtual7CEv.c:17 void _ZN16dScMgSmartball_c9Virtual7CEv(void)
Nothing in the tree compares those two surfaces, so this compiles and links clean. A C++ caller reaching the method through the class header reads r0 from a body that never wrote it.
This is not new with you, and I am not treating it as if it were. I audited every dScMgBase_c virtual against its src/_ZN11dScMgBase_c* definition across the stack:
| ref | header/definition mismatches |
|---|---|
origin/main |
2 — OnYoshiTryEat, Virtual50 |
origin/cpp/minigame-slot30 |
5 — + OnHitFromUnderneath, OnAimedAtWithEgg, OnAimedAtWithEggReturnVec |
origin/cpp/minigame-slot31 |
6 — + Virtual7C |
Every one is header int, src def void. The stack adds exactly one per PR, monotonically, and this would be the fourth.
The ask is consistency, not a particular type. These bodies genuinely do not set r0, and your own header says int is A HINT, so I am not claiming the ROM says void. I measured the neutrality directly on slot 29 earlier in this campaign: int returning the value already in r0 byte-matches, and so does void. The ROM does not care. What it cannot tolerate is the two spellings disagreeing.
So either:
- declare
voidin all four headers — smallest change, and it matches what the bodies actually do; or - keep
intand return the free value in the definitions (for slot 31 that isLoadCompressedFileAt's return, which is already in r0 — do not writereturn 0;, that manufactures amov r0,#0the ROM does not have and reads like a false refutation).
Either is fine. Split is not.
Note this is a third declaration surface. Virtual7C is absent from include/decl_common.h, so the checker I opened in #2105 reports no new disagreement on this branch — it joins decl_common.h against class headers and does not read src/ definitions. That is a real gap in my tool, measured on your PR, and I will extend it. On this tree the union of both surfaces is 8 distinct symbols with a return-type disagreement somewhere.
2. Off-by-one: this is the fourth consecutive non-pin, not the third
Inside include/dScMgBase_c.h on this branch:
:366 slot 28 -- "return type: int, A HINT -- and slot 28 is the first in this campaign"
:425 slot 29 -- "A HINT, the second consecutive slot that no body pins"
:510 slot 30 -- "A HINT, and the THIRD consecutive slot no body pins"
:606 slot 31 -- "A HINT, the third consecutive slot no body pins"
Slots 30 and 31 both claim third. Slot 31 is the fourth. One word.
Where this sits
Stack is #2099 → #2100 → #2102 → #2106, and the three below all carry CHANGES REQUESTED from me on this same disagreement. I would rather that be settled once at the base than patched four times, so I am not asking you to fix the five inherited rows here — just to not add the sixth without the decision being made.
Everything else in this PR I checked and it holds: the Virtual7C symbol renames in config/arm9/overlays/{ov004,ov006}/symbols.txt are address-preserving and size-preserving (0x8c/0x104 unchanged), and the ninth name correction — 0x020e72c0 was carrying recovered name: dScMgTrampoline2_c_Kill while five tables reference it, so it is dScMgD3DBase_c's — is the right call for the right reason.
Slot 31 of the 18-35 keystone range, stacked on #2102 (slot 30). Review only the top commit; the rest is the stack below.
What lands
Thirty-two vtables gain their thirty-second word. Seven override the slot; three need a declaration (
dScMgD3DBase_c,dScMgAmida_c,dScMgSmartball_c) and the other four —dScMgJump_c,dScMgJump2_c,dScMgTrampoline_c,dScMgTrampoline2_c— inheritdScMgD3DBase_c's body. No occupied-slot trap.128 bytes = 32 tables x one word.
_ZTV14dScMgD3DBase_cis the one table already VERIFIED, so its word lands in the verified pool and the other thirty-one land in the partial one.differs6,partial253,verified465,symbols1246,totalRecords7560 — all unchanged, and no symbol changed verdict.rombuild -j16 --no-rom: 11,088/11,088 reproducing, 106/106 exact, 100.000000%, ROM-build analysis PASS.The part worth reading: the borrowed names never applied
Slots 18-30 carry
include/dActor_c.h's names because they sit at dActor_c's indices. At 31 that argument runs out —dActor_c.hstops at slot 30 — and reaching the end of the borrowed table is what makes the borrowing legible:include/fBase_c.h:130-146declares slots 0-15 plus the destructor pair at 16/17;dBase_cadds no virtual;dScene_cdeclares nothing above slot 11. The two branches are siblings that each begin appending at 18.dScMgBase_cis a scene, not an actor, and is not adActor_cdescendant at any depth. The indices coincide; nothing was inherited.That is one root cause for three findings this campaign recorded separately — the ROM contradicting the name at slot 26, at 29, and at both halves of it at 30. It applies to all eighteen, not to those three.
Killspecifically came from_ZN10dBgActor_c4KillEv(ov002:0x020ee55c) — a genuine mangled ROM symbol, butdBgActor_cderives fromdActor_c. It is a nephew branch, one fork further away still, andKillis the one new virtual it appends at its slot 31. The name was carried across a fork to an index that does not exist on the branch it was taken from.So this slot takes
Virtual7C— the spellingfBase_calready uses (Virtual34,Virtual38), the family uses (Virtual50/08/10/18), and slots 33/34/35 are already recorded under. No new convention, and verified free tree-wide.Whether to retro-rename 18-30 is yours to call, not this PR's. What this PR declines to do is add a nineteenth borrowed name on top of the explanation.
What the slot does
Three read-modify-writes on the sub engine's BG1CNT at
0x0400100aleave it holding exactly the class's own base bits — 0x10 base, 0x800 D3DBase, 4 Amida/Smartball — then the layer scroll is reset, BG1's bit is cleared from the sub BG-enable shadow that slot 30 restores the sub DISPCNT from, and a language-indexed character file plus the shared screen map (file0x5b) are installed.dScMgD3DBase_cwraps that in the VRAM bank juggling only the 3D minigames need.It builds the touch-screen background, from
BeforeInitResources, which calls it last and discards the result. It destroys nothing — the second reason not to keep the name.Arity measured at the only in-family dispatch site (
ov004:0x020b0a0c):mov r0,r4; ldr r1,[r0]; ldr r1,[r1,#0x7c]; blx r1, so r1 is the loaded pointer and cannot also be an argument. Scanning arm9 + all 103 overlays for theldr/blxpair at +0x7c finds 39 sites; the other 38 are dBgActor_c'sKillon the other branch. Return typeintis a hint, not a pin — the fourth consecutive slot no body pins.Also in here
recovered name:correction, eighth ondScMgD3DBase_c, and the first wrong in both halves:ov006:0x020e72c0was labelleddScMgTrampoline2_c_Killand included that header. Five vtables reference the address, so the class isdScMgD3DBase_c's; include andthiscast move with it. Each wrong line is kept visible with the correction beside it.dScMgAmida_c.h,Unk36moves from 31 to 32 — one index closer to its true slot 36._ZTV12dScMgAmida_cstays DIFFERS until slot 35 lands; expected and unchanged.src_tu/actors/unit_ov004_020b0a38.cpp, which still names themfunc_ov004_*. Nothing reads it today —"status": "text-verified", not enrolled,src_tu/is not in the ROM build,check_src_tu_compiles97/97 green, andtiers_ratchetonly follows"promoted"entries — and it goes stale again at 32 and 33. Regenerate once, throughtubuild, after slot 33. Noted innotes/dScMgBase_c-slots-18-35.md.Gates
check_dead_referencesclean ·port_refcheck405/405 ·check_header_offsets --changed origin/main7 headers, 0 mismatched ·check_src_tu_compiles97/97 ·check_duplicate_sources11038 stems, none doubled · langmode ratchet PASS.Four slots left: 32-35.
🤖 Generated with Claude Code
https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh