Declare dScMgBase_c slot 34 (Virtual88) across the minigame family - #2110
Conversation
Slot 34 is the first slot in this campaign that takes ARGUMENTS, the first whose job is legible from the body alone, and the first whose rename breaks a cross-file reference rather than just a name. IT IS THE BRUSH. ov004:0x020ae3b4 walks a size x size square centred on (cx, cy) and for each cell computes the address of one 4-bit pixel in BG character VRAM -- (x/8 + (y/8)*32)*32 + (y&7)*4, the standard DS 4bpp char layout -- reads the containing word, splices `colour` into the nibble at (x&7)*4, and writes it back. Sixteen colours, one palette index per pixel, clipping on all four sides. It reads exactly the two object fields slot 33 initialises. Virtual84 sets obj+0x68 to 0 and obj+0x6c to -1; here obj+0x6c selects which BG layer to draw into (0..3 index G2S::GetBG0CharPtr..GetBG3CharPtr, anything else returns without drawing) and obj+0x68 gates the wrapped region above the touch screen. So slot 33 leaves the brush disabled and a minigame arms it by picking a layer. Two slots, one mechanism. Signature measured, not assumed. The only in-family caller is ov004:0x020ae5c4, a line rasteriser sitting immediately after the brush in the image, which dispatches through +0x88 at seven sites as it steps along a segment. Every one of the seven sets up r1, r2, r3 AND one stack word -- this plus four. Two of them (0x020ae7e4, 0x020ae83c) overwrite r0 on the very next instruction with `ldr r0,[sp,#0xc]`, so `void` is what the ROM says rather than merely what it permits. Nothing to correct, as at slot 33: no `recovered name:` line exists on the base body or on any of the four overrides, so no borrowed label is being retired. Virtual88 is this tree's own no-name spelling after the +0x88 vtable offset. Four overrides, four addresses, nothing inherited and nothing shared: dScMgAmida_c (ov006:0x020d14c0), dScMgTeresa_c (0x021200dc), dScMgTrampoline_c (0x02120da8), dScMgTrampoline2_c (0x02122cb0). dScMgD3DBase_c does NOT override this slot, so its two trampoline children declare it themselves instead of inheriting a shared body the way they do at 26-31 and 33. dScMgAmida_c's override is the only one in the family that CALLS the base rather than replacing it -- it is the collision half of the ghost-leg rule and the base is the drawing half -- so it held an extern on the old func_ov004_020ae3b4 name at six call sites. Retargeted here; without that the link breaks. Its body reads only three of the four parameters because the fourth arrives on the stack and it supplies its own size (2 or 4) per case, which is why the reconstruction had nothing to name there. The call sites are unanimous and fix the signature. include/dScMgTeresa_c.h had independently predicted this slot -- "takes four extra parameters and barely touches `this` (draws a HUD digit/glyph) ... not reconstructed tree-wide yet". Both halves check out: that class's override stamps a stipple pattern from data_ov006_0213f9e4 and never reads its object pointer, because unlike the base it always draws into sub BG0. That prose and the matching guess in notes/minigame-provenance.md are re-counted here. Verified: rombuild 11,088/11,088 reproducing, 106/106 modules exact, 100.000000% of compared bytes. romdata_check verifiedBytes 35,424 -> 35,428 (+4) and partialBytes 11,996 -> 12,120 (+124) -- 128 bytes, 32 tables x 4, the campaign's per-slot signature -- with no key added or removed and every other counter and the differing set unchanged. dead-references, port_refcheck, duplicate-sources, header-offsets --changed, src_tu-compiles 97/97 and the langmode ratchet all pass. One slot left: 35. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QhhAeJwXBnfPp7B5DNjCwh
andrewboudreau
added
the
attribution-override
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Byte-verified means the range carries Per-file link-check detailAll 195 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-reviewOne account owns this repo, so nobody can press Approve here. This is the sign-off in writing. Merge-tree gate, not the branch. Every earlier PR in the stack was checked in the same run and is clean on all eight. Bytes. Two things worth flagging in this one specifically1. This is the first slot whose rename broke a link, not just a name. 2. What the slot turned out to be
It reads exactly the two object fields slot 33 initialises: A guess recorded independently in Ready to land from my side. Ordering is yours. |
andrewboudreau
left a comment
andrewboudreau
left a comment
There was a problem hiding this comment.
Reviewed at bdb2510ca600. Every vtable claim in this PR is correct, and the arity claim — the one that carries the whole thing — is right and better-evidenced than the PR realises. I read the bytes independently rather than agreeing with the description, and I came back with five corrections. Three of them make your claims stronger. Two are wrong in a way I have flagged once already on this stack, and one of those is now printed in a header. There is also one concrete ask, which I measured rather than guessed.
The arity claim is earned
On #2107 I struck MEASURED from the slot-32 evidence because the three sites there put the pointer in r1/r2/r3 and the mov r4,r1 park tracks caller liveness, not callee signature. Here the evidence is structurally decisive and I am not asking for anything on it. Site at 0x020ae690, as you cite it:
0x020ae684 str r7, [sp] <- arg 4
0x020ae688 ldr r4, [r0]
0x020ae68c ldr r3, [sp, #0x3c] <- arg 3
0x020ae690 ldr r4, [r4, #0x88]
0x020ae694 mov r1, sb <- arg 1
0x020ae698 mov r2, r8 <- arg 2
0x020ae69c blx r4
r0 = this, r1/r2/r3 = args 1-3, [sp,#0] = arg 4. That is AAPCS for a four-argument member call, it is unanimous across every site, and the pointer sits in r4 so it competes with nothing. virtual void Virtual88(int, int, int, int) is the right declaration.
I also confirmed the caller extent: func_ov004_020ae5c4, size 0x294, spanning 0x020ae5c4..0x020ae858, sitting immediately after the brush at 0x020ae3b4 (size 0x210) exactly as described.
Independent verification of the six vtable claims
Dumped from the cartridge, slot indices from each _ZTV's recorded address:
| table | addr | slot 33 | slot 34 | agrees? |
|---|---|---|---|---|
_ZTV11dScMgBase_c |
020bc0c0 (ov004) |
020b265c |
020ae3b4 |
yes |
_ZTV14dScMgD3DBase_c |
0213c62c |
020e7124 |
020ae3b4 |
inherits, does not override — yes |
_ZTV12dScMgAmida_c |
0213b918 |
020b265c |
020d14c0 |
yes |
_ZTV13dScMgTeresa_c |
0213fa0c |
020b265c |
021200dc |
yes |
_ZTV17dScMgTrampoline_c |
0213fb34 |
020e7124 |
02120da8 |
yes |
_ZTV18dScMgTrampoline2_c |
0213fc7c |
020e7124 |
02122cb0 |
yes |
The structural claim is the interesting one and it holds: dScMgD3DBase_c carries the base's 020ae3b4 at slot 34, so the two trampolines really do have to declare it themselves rather than inheriting a shared body the way they do at 26-31 and 33. Four tables, four declarations, nothing shared — as written.
Config half is clean: five renames, all address- and size-preserving function renames, zero coined data symbols. I check for that specifically after #2096 and #2104.
Three corrections that strengthen the PR
1. Eight dispatch sites in the rasteriser, not seven. The header says "seven separate sites" twice. I count eight:
ldr@0x020ae608 -> blx@0x020ae60c r4
ldr@0x020ae62c -> blx@0x020ae640 r5 <- not in your seven
ldr@0x020ae690 -> blx@0x020ae69c r4
ldr@0x020ae6e4 -> blx@0x020ae6f0 r12
ldr@0x020ae73c -> blx@0x020ae748 r12
ldr@0x020ae784 -> blx@0x020ae790 r4
ldr@0x020ae7d8 -> blx@0x020ae7e4 r12
ldr@0x020ae830 -> blx@0x020ae83c r12
The missed one is a genuine dispatch with the full four-argument setup:
0x020ae61c ldr r0, [sp, #4] <- this
0x020ae620 str r7, [sp] <- arg 4
0x020ae624 ldr r5, [r0]
0x020ae628 ldr r4, [sp, #0x38]
0x020ae62c ldr r5, [r5, #0x88]
0x020ae630 ldr r3, [sp, #0x3c] <- arg 3
0x020ae634 mov r1, sb <- arg 1
0x020ae638 mov r2, r8 <- arg 2
0x020ae63c str r4, [sp, #0xc] <- local spill, non-contiguous, not an argument
0x020ae640 blx r5
Four instructions separate the ldr from the blx, which is why a tight scan drops it. Worth saying because I made the same mistake first: my initial pass required the blx to follow immediately and found 8 image-wide against your 14, which would have "refuted" a correct claim. A dispatch scan needs a lookahead window with a break on redefinition of the loaded register, and the count is window-dependent — please state the window when you quote a census.
2. Four sites overwrite r0, not two. The header names 0x020ae7e4 and 0x020ae83c. Add 0x020ae6f0 and 0x020ae748:
after blx@0x020ae6f0: ldr r0, [sp, #8] OVERWRITES r0
after blx@0x020ae748: ldr r0, [sp, #8] OVERWRITES r0
after blx@0x020ae7e4: ldr r0, [sp, #0xc] OVERWRITES r0
after blx@0x020ae83c: ldr r0, [sp, #0xc] OVERWRITES r0
Half the sites discard the return value on the next instruction rather than two of seven. void is better supported than you claim.
3. Teresa's "barely touches this" is stronger than "barely". It never touches it. The first reference to r0 in 0x021200dc is add r0, sl, sl, lsr #31 at 0x021200ec — a write into a dead register. And ldr sl,[sp,#0x40] at 0x021200e4 is the incoming fourth argument (9 pushed registers + 0x1c = 0x40), so that body genuinely reads args 1-4 and not this. The five-parameter definition with an unused a0 is the correct shape.
Two corrections that cut against a claim, one of them repeated
The header says:
Image-wide the +0x88 dispatch pair appears 14 times, 7 in ov004/ov006 and 7 in ov064
ov006 contains zero +0x88 dispatches. All of the in-family sites are in ov004. Saying "ov004/ov006" implies the overriding classes dispatch through this slot; they do not — the call graph really is one function in ov004, which is the more striking claim and the one your own prose makes two lines earlier.
The ov064 seven are not this virtual, and this is the same misattribution I corrected on #2107 at +0x80. All seven are one shape:
0x02116828 mov r1, #0
0x0211682c mov r0, r5
0x02116830 strb r1, [r5, #0x3f9]
0x02116834 ldr r1, [r0]
0x02116838 ldr r1, [r1, #0x88] <- pointer occupies r1
0x0211683c blx r1
0x02116840 add sp, sp, #0x24
The loaded pointer is in r1, so r1 cannot also carry cx; r2, r3 and [sp] are never written. Zero explicit arguments. Virtual88(int,int,int,int) takes four. This is exactly the structural refutation your own slot-33 argument turned on at +0x84, running the other direction. Corroborating: config/arm9/overlays/ov064/symbols.txt contains zero dScMg symbols, and all seven sites live inside func_ov064_021166f0 and func_ov064_02116754 — a different hierarchy that happens to put something else at vtable offset 0x88.
So the honest census is 8 in ov004, 0 in ov006, and 7 unrelated in ov064 (15 raw at window 8; your 14 is the window-4 figure). The fix is one sentence, but it matters more here than on #2107 because on #2107 it lived in a PR description and here it is being written into a header that outlives the PR.
The one ask: close the arity split in Amida, and it is free
include/dScMgAmida_c.h declares four parameters. src/_ZN12dScMgAmida_c9Virtual88Eiiii.c defines three:
void _ZN12dScMgAmida_c9Virtual88Eiiii(Ctx* ctx, int y, int x, int arg3)under a symbol mangled Eiiii. Your justification for this is correct — I verified it. The prologue is push {r4,r5,r6,r7,r8,sb,lr} + sub sp,sp,#4, putting the incoming fourth argument at [sp,#32], and across the whole 0x498-byte body there is not one load from any positive sp offset. All six sp accesses are stores to [sp,#0], which is the outgoing fifth argument for the base-brush call. It really does ignore its fourth parameter and hardcode 2 or 4 instead.
But "the reconstruction had nothing to name" is a reason to name it anyway, not a reason to leave the surfaces disagreeing — and this is the same header-declares-X / definition-defines-Y shape I have changes-requested on five times in this stack, just in arity rather than return type. It bites concretely at promotion: an out-of-line void dScMgAmida_c::Virtual88(int,int,int) has no matching declaration, and this family is already being promoted (#2064, #2067, #2077, #2079).
I measured the fix rather than assuming it. Adding the unused parameter:
void _ZN12dScMgAmida_c9Virtual88Eiiii(Ctx* ctx, int y, int x, int arg3, int size)compiled under the file's own pin:
pin: 2004/b56
3-param (as-is): (True, '2004/b56')
4-param (unused): (True, '2004/b56')
Byte-identical to the ROM either way, which is what you would expect for a stack argument that is never loaded. It costs nothing and it removes the only surface disagreement this PR introduces. Please add it, and reword the closing paragraph — the interesting fact is that Amida ignores the size and picks 2 or 4 per case, which is a real observation about the game, not that the parameter is absent from the signature.
Note that tools/check_decl_return_types.py will not catch this: it compares return types only. Extending it to compare arity is on my list, not yours.
Return type: this PR follows #2108's answer, and that is the news
It declares void, all five bodies fall off the end, and it adds zero new header/definition splits. It is the second PR in the stack to do that and the first that had a real chance to get it wrong — a four-argument slot is exactly where a speculative int would have been tempting. This is what I asked for at the bottom of the stack on #2108, arriving on its own. Nothing to correct here.
Off-by-one, and it is smaller than I previously said
Correcting my own note from #2106/#2107: slot 30's line is right. The chain is 28 first, 29 second, 30 third, so only two lines are stale:
:607 slot 31 -- "the third consecutive slot no body pins." -> fourth
:694 slot 32 -- "and the FOURTH consecutive slot no body pins." -> fifth
Still fix them in whichever of #2106/#2107 lands second, not here — all three PRs touch those lines and would conflict.
Gates
vs origin/cpp/minigame-slot33 @a3d94efa1a0d merge tree 16b1db00dc58 8/8 pass -> pass
mergeStateStatus: CLEAN, PR validation SUCCESS. The branch is a clean 8-commit stack of this campaign's own slot declarations — no foreign ancestry, unlike #2109. Merge order stays #2099 → #2100 → #2102 → #2106 → #2107 → #2108 → #2110.
Summary
Approve on content, with one code ask and two prose fixes.
- Add the unused fourth parameter to Amida's definition. Measured byte-neutral above.
- Drop "ov006" from the image-wide census, and either drop the ov064 seven or say explicitly that they are a different hierarchy with zero arguments. Do not leave a header asserting they are the same virtual.
- Optionally take the three strengthening corrections: eight sites, four r0-overwrites, Teresa never reads
this.
The vtable work is right, the arity work is right and hard-won, and the return type follows the answer the stack needed. My only real objection is that a census I already corrected once has been promoted from a PR description into a header.
1 participant
Slot 34 of the 18-35 keystone range. One slot left after this one.
Stacked on #2108 (slot 33). Base is
cpp/minigame-slot33; review the top commit only.What slot 34 is
It is the brush.
ov004:0x020ae3b4walks asizexsizesquare centred on(cx, cy)and, for each cell inside it, computes the address of one 4-bit pixel in BG character VRAM —(x/8 + (y/8)*32)*32 + (y&7)*4, the standard DS 4bpp char layout — reads the containing word, splicescolourinto the nibble at(x&7)*4, and writes it back. Sixteen colours, one palette index per pixel. It clips on all four sides.The two object fields slot 33 initialises are the two this one reads. Slot 33 (
Virtual84, engine bring-up) setsobj+0x68to 0 andobj+0x6cto -1. Hereobj+0x6cselects which background layer to draw into — 0..3 indexG2S::GetBG0CharPtrthroughGetBG3CharPtr, anything else returns without drawing, which is what -1 buys — andobj+0x68gates the wrapped region above the touch screen, where the main engine'sG2::GetBG*CharPtrare used instead and y is folded by+ data_ov004_020beb6c + 0xc0. So slot 33 leaves the brush disabled and a minigame arms it by picking a layer. Two slots, one mechanism.Who calls it:
ov004:0x020ae5c4, a line rasteriser sitting immediately after the brush in the image, which dispatches through +0x88 at seven separate sites as it steps along a segment. That is the whole of the in-family call graph for this slot — all seven are inside that one function. A line-drawing minigame is exactly the set that overrides it.The signature is measured, not assumed
This is the first slot in the campaign that takes arguments, so the arity had to come from the image rather than from "no parameters, nothing to get wrong".
Arity — four explicit parameters, unanimous across all seven call sites. Each sets up r1, r2, r3 and one stack word before the call. At
0x020ae690, for instance:So the callee takes
thisplus four. Image-wide the +0x88 dispatch pair appears 14 times, 7 in ov004/ov006 and 7 in ov064.Return type —
void, and here the ROM says so rather than merely permitting it. Two of the seven sites (0x020ae7e4,0x020ae83c) overwrite r0 on the very next instruction withldr r0,[sp,#0xc]; the rest tail-return without reading it. All five bodies fall off the end without setting a result.The dispatch scanner is the one validated at slot 33 by re-running it at +0x80 and reproducing slot 32's known site: mask
insn & 0x0FF00FFF == 0x05900000 | off(not0x0FFF0FFF— bits 19-16 are Rn),Rn != 15to exclude pc-relative literal pools, and up to three intervening instructions before theblx/bx. Strict adjacency returns zero here and would have produced a false "never dispatched" claim.Census
dScMgAmida_c(ov006:0x020d14c0),dScMgTeresa_c(0x021200dc),dScMgTrampoline_c(0x02120da8),dScMgTrampoline2_c(0x02122cb0).dScMgD3DBase_cdoes not override this slot, so unlike slots 26-31 and 33 the two trampoline classes do not share an ancestor body here — each declares and defines its own.Nothing to correct, as at slot 33: no
recovered name:line exists on the base body or on any of the four overrides, so no borrowed label is being retired.Virtual88is this tree's own no-name spelling after the +0x88 vtable offset, not a ROM name.The one new hazard: a cross-file reference
dScMgAmida_c's override is the only one in the family that calls the base rather than replacing it. Amida is the ghost-leg game — you draw a line and it must not cross one already there — so this override is the collision half and the base is the drawing half. It probes the 0x158-stride occupancy grid at+0x4710(the cell under the point, plus for a diagonal step the two cells the diagonal would cut between), sets the foul flag at+0x4709if any is already>= 3, and then calls the brush.That means it held an
externon the oldfunc_ov004_020ae3b4name plus six call sites.apply_renamerenames the definition,symbols.txtanddelinks.txt— it does not touch callers, so without the retarget in this commit the link breaks. A post-renamegrepfor every old symbol is now a standing step in this campaign.Why that body decompiles with only three explicit parameters: the fourth arrives on the stack and it never reads it — it supplies its own size (2 or 4) per case. So the reconstruction had nothing to name there. The call sites are unanimous and fix the slot's signature; an ignored stack argument costs the callee nothing.
Independent corroboration
include/dScMgTeresa_c.hpredicted this slot before it was declared, and the comment is quoted and re-counted in this diff:Both halves check out. That class's override stamps a shape by testing
data_ov006_0213f9e4[row]one bit per column — a stipple pattern, i.e. a glyph, not a solid square — and it really does never read its object pointer, because unlike the base it always draws into sub BG0 instead of consulting the layer index at+0x6c. The same guess innotes/minigame-provenance.mdis re-counted too (it was also the one dead prose reference this rename created, caught bycheck_dead_references).Verification
rombuild.py -j 16 --no-rom: 11,088 / 11,088 reproducing, 106/106 modules exact, 100.000000% of compared bytes.romdata_check --json, diffed against slot 33 in both directions:verifiedBytes35,424 -> 35,428 (+4) andpartialBytes11,996 -> 12,120 (+124). 128 bytes = 32 tables x 4 — the campaign's per-slot signature. No key added or removed; every other counter and the wholedifferingset unchanged.check_dead_references,port_refcheck,check_duplicate_sources,check_header_offsets --changed origin/main,check_src_tu_compiles(97/97),langmode_audit --check langmode-baseline.json: all pass.premerge_check.py --base origin/main --fetchover the whole stack: nothing goes green -> red.What this does NOT establish
Virtual88is a placeholder, not a recovered ROM name. The ROM's RTTI carries class names only, never method names.cx, cy, colour, sizeare inferred from behaviour, not from the ROM. The arity and the return type are measured; the names are a reading of what the body does with them.dActor_c's names by index, remains the owner's call and is deliberately not touched here.src_tu/actors/unit_ov004_020b0a38.cppregeneration is still outstanding as its own follow-up PR (harmless today:text-verified, not enrolled,src_tu/is not in the ROM build).attribution-overrideapplied per the standing instruction.