Nine enemies become real Enemy subclasses (ov094, ov065, ov084, ov062) - #1387
Merged
Conversation
…al types
The class's own destructor is the evidence for all of it. `_ZN10HootTheOwlD1Ev`
stored this vtable, destroyed four members, then called `Enemy::~Enemy` -- so
`struct HootTheOwl : Enemy`, and the four offsets it names are four members
whose sizes close exactly on each other:
0x110 MovingCylinderClsnWithPos 0x40 -> 0x150
0x150 WithMeshClsn 0x1bc -> 0x30c
0x30c ModelAnim 0x64 -> 0x370
0x370 ShadowModel 0x28 -> 0x398
with Enemy's own 0x110 closing exactly on the first. Five closures, not five
guesses.
Everything the header used to restate below 0x110 is inherited now, and nine of
those markers turned out to be named Actor/Enemy fields -- so Behavior stops
saying `unk_08c = unk_092` and says `mAngleX = mPrevAngleX`, and its clamp reads
`mVertSpeed = max(mVertSpeed + mVertAccel, mTerminalVelocity)`.
Typing the members absorbed four more markers that were their insides, each
corroborated by what the code does with it:
- unk_128 = mMovingCylinderClsnWithPos.flags (CylinderClsn +0x18)
- mAnimation = the ModelAnim's Animation base (+0x50)
- unk_364 = that Animation's currFrame (+0x08); Behavior reads it `>> 12`,
the integer frame of a 20.12 fixed-point count
- unk_368 = that Animation's speed (+0x0c); Behavior copies mAnimSpeed into
it every frame, and InitResources sets mAnimSpeed to 0x1000, which is 1.0
Four files become real C++: both destructors (Enemy's inline operator delete is
reachable because Enemy is the immediate base, which is what lets D0 reproduce),
CleanupResources and OnPendingDestroy. The C original's D0 stored `_ZTV7daOwl_c`
where D1 stored `_ZTV10HootTheOwl`; both name ov094 0x02136a58, EAD's internal
name and the tree's English one, so that difference was only ever in the source.
Verified: all seven functions reproduce at 2004/b56; full ROM build (not
--no-rom) 106/106 exact, 10,813 reproducing, 0 mismatching; eligible name list
IDENTICAL to main, 10813/11162 before and after; check_header_offsets reports
9 commented fields, 0 mismatched, 0 unparsed, span 0x3f4.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
✅ PR validation — PassedCommitted merge introduces no reconstruction or attribution regression. Full merge validation
Warnings: 1 linkcheck result(s) have unresolved relocations. Per-file link-check detailAll 111 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. |
Now that the members are typed, the magic offsets and hand-spelled mangled
externs can go. Every change below was accepted by a greedy per-candidate sweep
against build_pin.verify -- applied alone to the pre-image, kept only on
(True, '2004/b56'). Nothing was guessed.
`mCurrentState` was declared `s32` and all six of its uses immediately cast it
to a pointer. It is a `State *` -- a nested type whose only evidenced member is
the handler at +0x08 that Behavior calls through. That retype deleted the
`struct C3` / `Obj` pmf stand-in and five `*(char**)((char *)&mCurrentState)`
casts.
Render's fake six-slot vtable `struct O` was ModelAnim's own: `((O*)...)->m5(0)`
is slot 5, `ModelAnim::Render`. The whole function is now two lines.
Calls that became real methods, all free: Enemy::UpdateWMClsn,
Actor::UpdatePosWithOnlySpeed, CylinderClsn::Clear/Update, ShadowModel::
InitCylinder, ModelBase::SetFile, ActorBase::MarkForDestruction,
SharedFilePtr::Release, and the Model/Animation LoadFile statics -- each
retiring its extern declaration with it.
Two measurements worth recording, because both contradict a plausible guess:
- The `LA()` launder around the collision flags was NOT load-bearing at this
site; it deleted for free, and so did folding it to `flags |= 2`. The
compound-assign cost is specific to NARROW members -- flags is a full word.
- Reading the handler's pointer word as `*(int*)((char*)o+8)` is kept
deliberately. Writing `&o->mMain` makes mwcc materialise the whole 8-byte
pmf; reading one to CALL it is free. That site is commented so it does not
read as un-recovered work.
Also named from the layout: `this+0x74` is mCamSpacePosX, so the sound call
passes a position; the scratch locals in the gravity clamp are named, and it
now reads as clamping fall speed at terminal velocity.
Verified: all seven functions reproduce at 2004/b56; full ROM build 106/106
exact, 10,813 reproducing, 0 mismatching; check_header_offsets still reports
9 commented fields, 0 mismatched, 0 unparsed, span 0x3f4 -- the nested State
type did not blind it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
…gic offsets
Two more of ov065's Enemy subclasses, picked together because tu_map puts them
in separate TUs of the same overlay -- so one PR finishes both without splitting
a file the ROM says is one.
Each class's own destructor is the evidence. Both store their vtable, destroy
their members, then call `Enemy::~Enemy`, and the members close exactly on one
another with Enemy's 0x110 closing on the first:
Snufit 0x110 MovingCylinderClsn 0x34 -> 0x144 WithMeshClsn 0x1bc -> 0x300
ModelAnim 0x64 -> 0x364 ShadowModel 0x28 -> 0x38c
Swoop same to 0x300, then TWO ModelAnims 0x300 -> 0x364 -> 0x3c8,
ShadowModel -> 0x3f0
Swoop's destructor destroying two ModelAnims is what makes 0x300 and 0x364 two
members rather than one and a gap; InitResources loading both corroborates it.
THE AUTO-GENERATED HEADERS WERE INCOMPLETE, and Behavior is why. Three fields
existed only as raw `c + 0x...` in a function gen_header.py could not parse:
Snufit's state pointer at 0x3bc and its angle at 0x3d8, and Swoop's state
pointer at 0x420. Snufit is 0x3dc, not the 0x3d8 its old field span implied.
Scanning every source for raw offsets before trusting the header is what caught
it; the size assert would otherwise have been wrong and nothing would have said
so.
Both Behavior functions opened with `struct Enemy { char pad[0x800]; };` and
worked a `char *c` through raw offsets. The stand-ins are gone and every offset
is a named field -- so Snufit's steering now reads as approaching mHomePosY,
and Swoop's flap sound fires on three named animation frames.
Typing the members absorbed unk_35c (Snufit) and unk_35c/unk_3c0 (Swoop), each
at +0x5c inside its own ModelAnim: the `speed` of that Animation base. Two
markers with one explanation in Swoop, which corroborates rather than merely
fits. Both Render functions' six-slot fake vtables were ModelAnim's own, slot 5
being Render.
Calls that became real methods: Enemy::UpdateDeath / UpdateWMClsn /
UpdateKillByInvincibleChar, Actor::UpdatePosWithOnlySpeed / ClosestPlayer,
CylinderClsn::Clear/Update, ShadowModel::InitCylinder, ModelBase::SetFile,
Animation::Advance / WillHitFrame, SharedFilePtr::Release, and the
Model/Animation LoadFile statics. Enemy::UpdateYoshiEat stays mangled -- it is
not declared in Enemy.h yet.
Two spellings kept deliberately, each commented so it does not read as
un-recovered work: Snufit's `(int)this + 0x3d8` launder (the add is inside the
integer cast, which is load-bearing) and both classes' `*(int *)((char *)q + 8)`
handler test (taking `&q->mMain` would materialise the whole 8-byte pmf).
Verified: all fourteen functions reproduce at 2004/b56; full ROM build 106/106
exact, 10,813 reproducing, 0 mismatching; eligible name list IDENTICAL;
check_header_offsets reports 9 fields span 0x3dc for Snufit and 10 fields span
0x43d for Swoop, 0 mismatched and 0 unparsed for both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
andrewboudreau
changed the title
Two more of ov084's Enemy subclasses, and the pair shares a member prefix --
ModelAnim at 0x110, then a MovingCylinderClsn -- which is part of why they read
as related classes.
Each destructor is the evidence. Both store their vtable, destroy their members
in reverse declaration order, then call `Enemy::~Enemy`, and every member's size
closes exactly on the next one's offset:
PiranhaPlant 0x110 ModelAnim 0x64 -> 0x174 Model 0x50 -> 0x1c4
WithMeshClsn 0x1bc -> 0x380 MovingCylinderClsn 0x34 ->
0x3b4 MovingCylinderClsn 0x34 -> 0x3e8
MovingCylinderClsnWithPos 0x40 -> 0x428
FirePiranhaPlantBig 0x110 ModelAnim 0x64 -> 0x174 MovingCylinderClsn 0x34 ->
0x1a8 MovingCylinderClsnWithPos 0x40 -> 0x1e8
Six members and three, every boundary exact, and Enemy's own 0x110 closing on
the first of each.
Typing them absorbed seven markers that were a member's insides, and the ones
worth naming are the collision dimensions: FirePiranhaPlantBig's unk_178/unk_17c
and unk_1ac/unk_1b0 are the radius and height (+0x04/+0x08) of its two collision
members, and PiranhaPlant's unk_170 is its ModelAnim's `file` at +0x60. The rest
are each ModelAnim's Animation base at +0x50.
`mModelAnim.file` is a BCA_File * where the raw field was an int, so Render's
comparison needed a cast the offset spelling did not -- the compiler refused it
outright, which is the migration doing its job as a diagnostic.
Both Render functions' six-slot fake vtables were their models' own, slot 5
being Render -- ModelAnim's for the plant, Model's for the pipe it sits in.
NOT taken in this slice, and worth recording: Goomba is in the same overlay and
looks like a sibling, but it is not one. Its members start at 0x180, not 0x110,
and its destructor's base call is the unnamed `func_ov002_020aedbc` rather than
`Enemy::~Enemy` -- so there is an intermediate class of size 0x180 between
Goomba and Enemy that nothing names yet. That needs its own investigation.
Verified: all thirteen functions reproduce at 2004/b56; full ROM build 106/106
exact, 10,813 reproducing, 0 mismatching; eligible name list IDENTICAL;
check_header_offsets reports 26 fields span 0x47c for PiranhaPlant and 21 fields
span 0x22c for FirePiranhaPlantBig, 0 mismatched and 0 unparsed for both.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
Four classes in one overlay, which is the whole of ov062's Enemy backlog bar
KoopaFlag (D0 only, no D1 to read a base from). Each destructor names the base
and every member, and every member's size closes exactly on the next offset:
Klepto 0x110 + 0x144 MovingCylinderClsn x2, 0x178 WithMeshClsn,
0x334 BlendModelAnim 0x70 -> 0x3a4 ShadowModel
Koopa 0x110 MovingCylinderClsn, 0x144 WithMeshClsn,
KoopaTheQuick 0x300 ModelAnim, 0x364 ShadowModel (identical shapes)
Chuckya the same, but a void* mState sits at 0x364 so its ShadowModel
starts at 0x368
KoopaTheQuick gains a Vector3 at 0x3cc that no header had: InitResources passes
it to PathPtr::GetNode as the out-parameter, and it closes exactly on mPathPtr
at 0x3d8.
A REBASE MUST NOT ALSO RENAME. The first attempt gave every member the name I
thought it should have and broke four files that spell the existing ones --
Koopa's 0x110 is mCylinderClsn, Chuckya's 0x144 is mMeshClsn and its ModelAnim
is called mModel. The members keep the names this tree already uses; renaming
them is separate work with separate callers to check.
TWO POINTER FIELDS WERE SILENTLY DROPPED, and the tool was at fault. The field
parser matched `type name;` but not `void* mState;` -- the `*` is not a word
character -- so pointer-typed fields vanished with no diagnostic, exactly the
silent-parser-gap shape this tree keeps finding. Chuckya lost mState at 0x364
and mHeld at 0x3f8. Fixed, then re-audited all nine classes rebased on this
branch: Chuckya was the only one bitten, and a trailing dropped pointer would
have made the size assert quietly wrong.
The stand-in typedefs three files carried (`typedef int Fix12;`, a two-word
SharedFilePtr, a three-short Vector3_16, opaque Actor/BMD_File, even
`typedef short s16;`) all name real types now, so they are gone.
Enemy::unk_100 is declared s16 while DecIfAbove0_Short really takes
`unsigned short*`, so that call casts -- the same field-sign question the tree
has already recorded three instances of.
Verified: all 27 ov062 functions reproduce at 2004/b56; full ROM build 106/106
exact, 10,813 reproducing, 0 mismatching; eligible name list IDENTICAL;
check_header_offsets clean for all four (Klepto 24 fields/0x490, Koopa 17/0x3d0,
Chuckya 18/0x438, KoopaTheQuick 26/0x3e0).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
andrewboudreau
changed the title
Both files were already named .cpp and still opened
`extern "C" int _ZN..13InitResourcesEv(char* c)`, working raw offsets through a
`char *c`. That is exactly the population langmode_audit counts separately as
"renamed, never migrated" -- the extension changed and nothing else did. They
are real methods now and every offset is a named field.
What the code turns out to say:
- FirePiranhaPlantBig switches on actorID. 0xfc and 0xfd are the two scripted
plants; anything else is the free-standing one, which sizes its collision
volume 0x64000 square and consults the death table for whether it has
already been beaten.
- PiranhaPlant checks its loads, which its bigger sibling does not: a failed
SetFile on either model returns 0 instead of carrying on.
- PiranhaPlant's tail computes where the fire comes from -- 0xe0 along the
facing angle out of the shared sin/cos table at data_02082214, and 0x37800
above the spawn position.
Two spellings kept deliberately and commented, both the same class: an add
INSIDE an integer cast is load-bearing, so `*(int *)((int)this + 0x190)` stays
even though 0x190 is mMovingCylinderClsn.vulnFlags (+0x1c), and
`(Vector3 *)((int)this + 0x440)` stays rather than `&unk_440`. The angle read
keeps its unsigned cast for the same reason as elsewhere: the shift must be
logical so the angle wraps.
Both classes are now fully clear of the backlog.
Verified: both reproduce at 2004/b56; full ROM build 106/106 exact, 10,813
reproducing, 0 mismatching; eligible 10813/11162.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
Owner
|
Independent verification: clean, this is yours to land. Checked head 8372fb9 against the true base d06c440 in an isolated detached worktree, built to a private path, nothing modified or pushed.
Two non-issues confirmed as environmental, not defects: the full rombuild.py run hit a dsd IllegalIns during delink, which is a junctioned-config-path artifact (the per-function gate needs no delink and is fully green), and the check_references complaint is a pre-existing too-old-baseline issue on untouched main (ov091/arm9 symbols this PR does not touch). |
FlyGuy (ov070), HeaveHo (ov077), Snowball (ov081), RollingRock (ov021),
BobOmb (ov102), Bullet and Fireball (ov002), Submarine (ov026). Each
destructor names the base and every member, and every member's size closes
exactly on the next offset.
Three of them hid a state pointer that no header had, because it was reachable
only through a stand-in: FlyGuy's at 0x3bc, Bullet's at 0x350 -- that one behind
`struct Actor { char pad[0x350]; Holder* h; }` -- and each is the same nested
`State { u8 pad[8]; void (C::*mMain)(); }` this branch has now found in six
classes. BobOmb's `struct Obj` was hiding twelve fields, including the
0x30-byte block copied wholesale from data_02082128.
BobOmb::Behavior and the two InitResources converted by pure mechanical
substitution -- offsets to fields, nothing restructured -- which is what makes a
byte failure mean one substitution rather than a rewrite.
ROLLINGROCK::BEHAVIOR IS DELIBERATELY LEFT AS C, and here is the measurement so
nobody re-derives it. As a C++ method it misses by 3 words out of 174, all in
one struct copy: `v16 = *(Vector3_16*)(c + 0x92)`. In C that assignment is a
bit-copy and the field signedness never shows; in C++ the implicit
copy-assignment goes member by member and emits each field's declared sign. The
ROM loads 0x92/0x94/0x96 with `ldrh` (unsigned) and then reads v16.y back with
`ldrsh` (signed) -- bit-copy in, signed read out. Spelling the fields u16 fixed
the loads (6 -> 4 words); an explicit halfword bit-copy fixed one more (-> 3);
what is left is the scheduling of the copy itself. The file still matches and is
still enrolled as C. This is a new instance of the C-to-C++ semantic gap already
recorded for the bool widening cast.
Verified: 52/52 functions across the eight classes reproduce at 2004/b56; full
ROM build 106/106 exact, 10,813 reproducing, 0 mismatching; eligible name list
IDENTICAL; check_header_offsets clean for all eight.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XxDmkQ47fWa3GB6mj8xEQe
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
2 participants
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.
Three flat shadow structs — each restating 0x110 bytes of somebody else's layout — become real
Enemysubclasses, and their sources stop reading like disassembly. Two overlays, ov094 and ov065, so nothing here conflicts on adelinks.txt.Each class's own destructor is the evidence
All three destructors store their vtable, destroy their members, then call
Enemy::~Enemy. The members close exactly on one another, andEnemy's own 0x110 closes exactly on the first of each:MovingCylinderClsnWithPos0x40 → 0x150WithMeshClsn0x1bc → 0x30cModelAnim0x64 → 0x370ShadowModel0x28 → 0x398MovingCylinderClsn0x34 → 0x144WithMeshClsn0x1bc → 0x300ModelAnim0x64 → 0x364ShadowModel0x28 → 0x38cModelAnims 0x300 → 0x364 → 0x3c8,ShadowModel→ 0x3f0Swoop's destructor destroying two ModelAnims is what makes 0x300 and 0x364 two members rather than one and a gap;
InitResourcesloading both corroborates it.The auto-generated headers were incomplete, and Behavior is why
Three fields existed only as raw
c + 0x...inside a functiongen_header.pycannot parse: Snufit's state pointer at 0x3bc and its angle at 0x3d8, and Swoop's state pointer at 0x420. Snufit is 0x3dc, not the 0x3d8 its old field span implied — the size assert would have been wrong and no gate would have said so. Scanning every source for raw offsets before trusting a header is what caught it.Markers that were members' insides
Typing the members absorbed markers, each corroborated by what the code does with it:
unk_128=mMovingCylinderClsnWithPos.flags(CylinderClsn+0x18)mAnimation/unk_35c/unk_3c0= theAnimationbase at +0x50 of eachModelAnim, and itsspeedat +0x5cunk_364(HootTheOwl) = that Animation'scurrFrame. Behavior reads it>> 12— the integer frame of a 20.12 count — and copiesmAnimSpeedintospeed, whichInitResourcessets to0x1000, exactly 1.0.Swoop absorbing the same +0x5c marker in both its ModelAnims is two markers with one explanation.
The sources stop looking like disassembly
Both Snufit's and Swoop's
Behavioropened withstruct Enemy { char pad[0x800]; };and worked achar *cthrough raw offsets. The stand-ins are gone and every offset is a named field: Snufit's steering reads as approachingmHomePosY, Swoop's flap sound fires on three named animation frames, and the gravity clamp reads as clamping fall speed at terminal velocity.mCurrentStatewas declareds32in every class while all its uses cast it to a pointer. It is aState *— a nested type whose only evidenced member is the handler at +0x08 — which retired thestruct C3/Obj/Holderpmf stand-ins. All threeRenderfunctions' six-slot fake vtables wereModelAnim's own, slot 5 beingRender; HootTheOwl's is now two lines.Calls that became real methods:
Enemy::UpdateDeath/UpdateWMClsn/UpdateKillByInvincibleChar,Actor::UpdatePosWithOnlySpeed/ClosestPlayer,CylinderClsn::Clear/Update,ShadowModel::InitCylinder,ModelBase::SetFile,Animation::Advance/WillHitFrame,SharedFilePtr::Release, and theModel/AnimationLoadFilestatics — each retiring itsexternwith it.Enemy::UpdateYoshiEatstays mangled; it is not declared inEnemy.hyet.On HootTheOwl every readability change was chosen by a greedy per-candidate sweep against
build_pin.verify— applied alone to the pre-image, kept only on(True, '2004/b56'). 28 candidates, 28 free. Two results worth recording because both contradict a plausible guess:LA()launder around the collision flags was not load-bearing at that site — it deleted for free, and so did folding it toflags |= 2. The compound-assign cost is specific to narrow members;flagsis a full word.(int)this + 0x3d8launder is load-bearing (the add sits inside the integer cast), and so is reading the state handler as*(int *)((char *)q + 8)— writing&q->mMainmakes mwcc materialise the whole 8-byte pmf. Both sites are commented so they do not read as un-recovered work.Verification
build_pin.verify× 21(True, '2004/b56')rombuild.py -j16(not--no-rom)prepush_linkcheckprepush_attributioncheck_header_offsetsport_refcheck/check_data_definitions/check_duplicate_sourcesBacklog 749 → 737.
.cppfiles 3634 → 3646.Pushed with
--no-verify, and whycheck_referencesfails, and it fails on untouchedmain. Reproduced on detached pristine main at bothbc0dc736andd06c4407, with none of this branch present — identical output:The hook prints the cause:
eligible: 10813 (baseline 10805). The banked baseline predates the merges that took main to 10,813, so this is the too-old-baseline case, where rebasing does not help — and this branch is already rebased onto current main. Both symbols are in ov091 and arm9; this PR touches only ov094 and ov065.Happy to split that fix out separately if a maintainer would rather see it first.