Twelve more Enemy subclasses -- and two that turned out not to be - #1389
Merged
Conversation
Intended as fourteen. Bully and BigBully are not Enemy subclasses, and three
independent sources say so:
- their destructors store TWO vtables, not one. Deleting either store makes
the function shrink, so the ROM really has both -- that second store is an
inlined base destructor's vptr write, and Bully and BigBully store the SAME
second vtable, which only makes sense if it is their shared base's.
- data_ov064_0211b768 is that vtable. Its slots 16/17 are func_ov064_02115ee0
and func_ov064_02115f28, an unnamed D1/D0 pair differing by 0x14 -- exactly
a deallocation.
- the RTTI graph names the base: daDonketu_c and daBDonketu_c both derive
from daOts_c, never from dEnemyBase_c.
`struct Bully : Enemy` would emit one vptr store and could not match. The
intermediate wants recovering the way CapEnemy was; it is not this commit.
The twelve that are Enemy subclasses each have two witnesses that had to agree
before the class was touched: the destructor destroys the members, and the
factory constructs the same types at the same offsets. Every one agreed.
SIX OF THE FOURTEEN SIZES WERE WRONG, and the factory is why. Each _Spawn calls
ActorBase::operator new with a literal, and that literal IS the class's sizeof;
the span of the evidenced fields is only a lower bound. MantaRay's fields reach
0x38c and it allocates 0x404 -- 0x94 of trailing space no source reads. Same
correction as the seven in the previous commit, from the same cause.
The factory is also the only witness to a member with a trivial destructor:
RollingIronBall constructs a PathPtr at 0x3f4 that nothing destroys. 0x3f4 + 8
is 0x3fc, exactly the allocation literal -- the layout does not close without
it, and reading only the destructor left the class eight bytes short.
TWO SUBSTITUTIONS WERE SEMANTIC CHANGES, and the byte gate caught both. The
sources spelled `*(char**)&unk_30c + 0x20`, meaning 0x20 BYTES; typing the
member makes it `data.materials + 0x20`, which is pointer arithmetic scaled by
sizeof(BMD_Material). Rabbit's materials and Key's bones both had it.
Actor declares param1 u32, but Rabbit::InitResources shifts it with ASR, not
LSR -- the ROM reads it signed there. Without the casts the function came out
two words different. The flat header called 0x008 an s32, which is exactly why
the rebase exposed it and nothing before it could.
WaterBomb's InitResources cast `this` to a full shadow struct; that is gone,
and the tail fields it was the only record of (0x3a8..0x3b6) moved into the
header. Its `+ 0x300 + 0xb4` folds to 0x3b4 -- past mModel's end, so a field of
this class and not something inside the Model.
Whomp::InitResources now compiles and byte-matches; on main it did not compile
at all, because two of its local declarations collided with decl_common.h's.
It is still not ELIGIBLE, so the enrolled count does not move. I did not
establish why: eligible.py gates on whether the object can link in place rather
than on the bytes, but the compiled object has one .text of 0x424 -- exactly the
declared size -- one global FUNC whose st_size matches, and no rodata, which is
what that gate asks for. Two runs agree it is excluded. Unexplained, not
diagnosed.
79 of the 80 functions across the twelve classes reproduce. Twelve headers pass
check_header_offsets with 0 mismatched, each struct spanning exactly its
asserted size. 106/106 exact, 10,813 reproducing, 0 mismatching; the eligible
name list is unchanged from the baseline.
STILL BROKEN, AND WAS BEFORE THIS BRANCH: RollingIronBall::InitResources
reports a size mismatch, reproduced on 1b45f57 before any of this. Its body
still carries laundering hacks and a volatile read; it wants its own session.
NOTED, NOT FIXED: TextureSequence::Prepare is declared a non-static member, but
the ROM passes it two argument registers and no `this` -- LakituBro's call site
writes r2 AFTER the bl. All 23 call sites in the tree declare it with two
parameters. A mangled name does not encode staticness, so the symbol is equally
the mangling of a static member, which is what the evidence says it is. Its 0xc
body is a pure `ldr r12,[pc]; bx r12` tail jump that reproduces for any
signature, so the definition file's `this` is unconstrained by the ROM and
contradicted by every caller. Changing it touches 23 dependents and belongs in
its own commit.
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Warnings: 2 linkcheck result(s) have unresolved relocations. Per-file link-check detailAll 80 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. |
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.
Twelve Enemy subclasses become real C++: LavaBubble, WaterRing (ov064),
LakituBro, Rabbit, RabbitKey (ov085), MantaRay, Skeeter (ov090),
Whomp (ov079), Key (ov089), WaterBomb (ov098), RollingIronBall
(ov100), PowerStar (ov002).
It was meant to be fourteen. Bully and BigBully are not Enemy subclasses, and
three independent sources say so:
function shrink, so the ROM really has both — the second is an inlined base
destructor's vptr write, and both classes store the same second vtable,
which only makes sense if it is their shared base's.
data_ov064_0211b768; its slots 16/17 arefunc_ov064_02115ee0/func_ov064_02115f28, an unnamed D1/D0 pair differingby 0x14 — exactly a deallocation.
daDonketu_canddaBDonketu_cboth derivefrom
daOts_c, never fromdEnemyBase_c.struct Bully : Enemyemits one vptr store and cannot match. That intermediatewants recovering the way
CapEnemywas; it is not this PR.The factory is the second witness, and it outranks the field span
Every class here has two witnesses that had to agree before it was touched: the
destructor destroys the members, and
<Class>_Spawnconstructs the same types atthe same offsets. All twelve agreed.
Six of the fourteen sizes were wrong when taken from the span of the evidenced
fields. Each
_SpawncallsActorBase::operator newwith a literal, and thatliteral is the class's
sizeof; the field span is only a lower bound. MantaRay'sfields reach 0x38c and it allocates 0x404 — 0x94 of trailing space no source reads.
The factory is also the only witness to a member with a trivial destructor:
RollingIronBall constructs a
PathPtrat 0x3f4 that nothing destroys. 0x3f4 + 8is 0x3fc, exactly the allocation literal — the layout does not close without it.
Two substitutions were semantic changes, and the byte gate caught both
The sources spelled
*(char**)&unk_30c + 0x20, meaning 0x20 bytes. Typing themember turns the same text into
data.materials + 0x20— pointer arithmeticscaled by
sizeof(BMD_Material). Rabbit's materials and Key's bones both had it.Actor declares
param1u32, butRabbit::InitResourcesshifts it with ASR, notLSR — the ROM reads it signed there, and without the casts the function came out
two words different. The flat header called 0x008 an
s32, which is exactly whythe rebase exposed it and nothing before it could.
Results
check_header_offsetswith 0 mismatched, each struct spanningexactly its asserted size.
rombuild: 106/106 exact, 10,813 reproducing, 0 mismatching.prepush_linkcheck: 80 checked, 78 verified, 0 blocking.prepush_attribution: 0 changed, 0 lost.port_refcheck,check_data_definitions,check_duplicate_sources: clean. Eligible name list unchanged from baseline.WaterBomb::InitResourcesno longer caststhisto a shadow struct; the tailfields that shadow was the only record of (0x3a8..0x3b6) moved into the header.
Whomp::InitResourcesnow compiles and byte-matches — on main it did not compileat all. It is still not eligible, and I did not establish why: its object has one
.textof 0x424 (exactly the declared size), one global FUNC whosest_sizematches, and no rodata, which is what that gate asks for. Two runs agree it is
excluded. Unexplained, not diagnosed.
Known, and pre-existing
RollingIronBall::InitResourcesreports a size mismatch. This is not aregression — it fails identically on
1b45f57b, before this branch, where theclass was still flat. Its body still carries laundering hacks and a
volatileread; it wants its own matching session.
Noted, evidenced, not fixed
TextureSequence::Prepareis declared a non-static member, but the ROM passes ittwo argument registers and no
this— LakituBro's call site writes r2 after thebl. All 23 call sites in the tree declare it with two parameters. A mangled namedoes not encode staticness, so the symbol is equally a static member's, which
is what the evidence says. Its 0xc body is a pure
ldr r12,[pc]; bx r12tail jumpthat reproduces for any signature, so the definition file's
thisis unconstrainedby the ROM and contradicted by every caller. Changing it touches 23 dependents and
belongs in its own PR.