Files
BT412/context/reconstruction-gotchas.md
T
arcattackandClaude Fable 5 a110041a70 Fix "can't turn in MP": pilotArray[1] overrun stomped controlMode
User report from the first live 2-window session: throttle worked, A/D turn
did nothing.  [mppr] showed the input REACHING the mapper (stickX=-1) but
turnDemand=0 and controlMode = pointer garbage (73583232 / 217478632 --
different per instance), from frame 1, MP only (solo mode=0).

Hunted with a cdb hardware write-breakpoint (ba w4 armed at ctor time on
&controlMode, compiled offset +0x128 from the [mode-ctor] probe).  Caught the
writers red-handed: MechControlsMapper::BuildPilotArray / FillPilotArray.

ROOT CAUSE -- a shrunk-span array: the binary reserves a FIXED 10-slot pilot
block @0x15C..0x183 ((pilotArrayBuilt@0x184 - 0x15C)/4 = 10); the recon
declared `Pilot *pilotArray[1]` ("variable length"), so every write past
slot 0 stomps the members declared after it.  In MP, FillPilotArray wrote the
PEER's Player pointer into slot 1 = over controlMode -> the turn shaping
dispatched on pointer garbage -> turnDemand 0.  MASKED in solo: the zero-loop
overrun wrote 0 there, and 0 == BasicMode.  This was ALSO the real cause of
task #48's "turnDemand zeroes out in -net" observation (worked around then
with the direct goto-turn; attribution corrected in the KB).

Fix: pilotArray sized to the binary's own 10-slot span; ctor zeroes all
slots; BuildPilotArray clamps pilotCount to capacity (local + 9 remotes; an
8-pod game fits).  Diagnostics kept (BT_MODE_LOG ctor probe; [mppr] line now
prints mapper/&mode).

VERIFIED: MP session holds mode=0 throughout, turnDemand flows nonzero, the
mech turns (heading -1.33) and closes on the peer; solo kill chain intact.
KB: new "shrunk-span array" gotcha (reconstruction-gotchas 5) with the cdb
ba-w4 recipe; multiplayer.md task-#48 note corrected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 18:16:46 -05:00

197 lines
13 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
---
id: reconstruction-gotchas
title: "Reconstruction Gotchas — the systemic bug classes (check these FIRST)"
status: established
source_sections: "CLAUDE.md §5a, §10c; docs/HARD_PROBLEMS.md; docs/RESOURCE_AUDIT.md; gauge-wave notes"
related_topics: [reconstruction-method, decomp-reference, subsystems, combat-damage, gauges-hud]
key_terms: [shadow-field, databinding-trap, Wword-trap, FORCE-trap, dtor-epilogue, bridge, attribute-pointer]
open_questions:
- "Which reconstructed classes still carry un-audited raw-offset reads?"
---
# Reconstruction Gotchas
The reconstruction is a **layout + linkage** problem as much as a logic problem. Our compiled
classes are NOT byte-identical to the 1995 binary, and the BT link uses `/FORCE`, so a whole
family of bugs is **silent** — garbage that happens to be non-fatal, or a runtime AV with no
link error. When a reconstructed class misbehaves, walk this checklist FIRST; the answer is
usually here, not in the logic.
> **The core rule (RULE: no stand-ins):** the full game logic IS in the pseudocode — a "gap"
> is a reconstruction stub not yet filled, never a hole in the original. Never write
> placeholder logic for an apparent gap; read the decomp. (User: "there are no gaps, just
> work to be done.") Bring-up scaffolding (env-var paths, explosion-for-beam) is clearly
> marked and meant to be REPLACED, never to substitute for reading the decomp. [T2]
---
## 1. Shadow field — re-declaring an engine-base field (THE most common)
**Symptom:** a field reads `0xCDCDCDCD` (fresh-heap fill) even though the ctor "sets" it; or an
object over-sizes past its factory alloc. **Cause:** the reconstruction re-declared a field the
engine base already owns, at the *binary's* offset. Two failures: (a) the copy **shadows** the
base — the engine ctor writes the base field, the reconstruction reads its own uninitialised
copy; (b) it lands at a **different offset** than the binary assumed.
**Fix:** delete the re-declaration; use the inherited member/accessor. Examples: `Mech`
`damageZoneCount`/`damageZones` (shadowed `Entity`'s → zones never built); `Mech__DamageZone`
`structureLevel`→engine `damageLevel`; the whole `HeatableSubsystem` de-shadow
(`statusFlags``simulationFlags`, `destroyed``simulationState`, `statusBits``ForceUpdate()`).
[T2]
## 2. Wword(N) — an ABSORBER, not storage (state cached there VANISHES)
`mechrecon.hpp:226` defines `Wword(int i)` as `static BTVal bank[0x400]; return bank[i&0x3ff]`,
and `BTVal` is the recon **absorber** type: `operator=` stores NOTHING, every read converts to
`T()` (zero), and ALL comparisons (`==` and `!=`, vs BTVal or int) return **false**. Consequences: [T2]
- Any state CACHED via `Wword(N) = x` silently vanishes; the later read is always 0/null.
**Archetype:** the STEP-6 cylinder table was "cached" at `Wword(0x111)` → the unaimed
TakeDamage path was totally inert (every hit no-op'd, "can't kill the enemy") while the
ctor log looked fine. Fix = a real named member (`Mech::damageLookupTable`). If a Wword
slot must hold real state, PROMOTE it to a named member mapped to that binary offset —
check `mech.hpp`'s offset map first (the slot may already exist under a best-effort
mislabel; 0x111 was mislabeled `ammoExpended`).
- `if (Wword(a) != Wword(b))` and `if (Wword(a) == 2)` are BOTH always-false → the guarded
branch is dead code. Known dead sites: `mech.cpp:1511` + `mech.cpp:1613` (replicant
leg-state / stability-alarm sync in ReadUpdateRecord — multiplayer-only, deferred).
- Any `Wword(N)` used for OBJECT access reads a shared global, not `this+i*4`; e.g.
`(Mech*)Wword(3)` for a zone's owner → garbage; use `GetOwningSimulation()`.
Sweep recipe: `grep -nE "\((int|void|[A-Z]\w+) ?\*\)\s*Wword\(|if \(Wword\(|Wword\([^)]+\)\s*(!=|==)"`
every hit is either dead code or a vanished cache.
## 3. Databinding trap — raw offsets read garbage
Our compiled layout != the 1995 binary, so `*(T*)(obj+0xNN)` reads garbage for ANY object we
compile. This is WHY shadow fields fail and why raw subsystem reads (e.g. a gauge reading
`owner+0x438`) return junk. **Fix:** use compiled named members/accessors; for a cross-TU raw
op, use a **bridge** (§8). A `+0x128`-style owner offset in subsystem code is the
[[subsystem-roster]] (`subsystemArray`), NOT the segment table — check every `GetSegment(int)`
in a reconstructed subsystem ctor. [T2]
## 4. Resource-struct layout mismatch (sibling of the shadow bug)
**Symptom:** RESOURCE fields read garbage (silently — a `heatSinkIndex` reading `10.0f`).
**Cause:** `*__SubsystemResource` structs overlay pre-built 256-byte records loaded VERBATIM at
fixed offsets, so OUR struct must match the binary byte-for-byte. Breaks two ways: (a) wrong
inheritance base (the resource must mirror the CLASS hierarchy — `HeatableSubsystem`'s resource
inherits `MechSubsystem__SubsystemResource` 0xE4, not `Subsystem::SubsystemResource` 0x30); (b)
under-sized fields (a 12-byte record field typed as a 4-byte `ResourceID`). **Diagnose:** log
compiled offsets `(char*)&res->field - (char*)res` vs the binary's; dump raw record bytes as
int+float. **Lock:** `static_assert(offsetof(...)==0xNN)` + `static_assert(sizeof(...)==0xNN)`.
The 33-agent RESOURCE_AUDIT fixed 8 such bugs (docs/RESOURCE_AUDIT.md). [T2]
## 5. Alias / phantom / interior fields (object layout)
- **Alias field:** a subclass member re-declaring an inherited slot the ctor reuses under a new
name (Condenser `refrigerationOutput`==inherited `massScale@0x160`; Emitter
`outputVoltage`==`rechargeLevel@0x320`). → delete, use the inherited name.
- **Alarm-interior field:** a value the binary reads at `alarm+0x14` modeled as a separate
member (HeatSink `heatState@0x184` == `heatAlarm.GetLevel()`). → route to the accessor.
- **Phantom field:** a member at an offset PAST the object (Generator `shortFlag@0x25C` is
really `*(owner+0x190)+0x25c` the msg-manager). → remove; read the real source.
- **Shrunk-span array:** a binary FIXED-SPAN block declared as `member[1]` ("variable length"
comment) — every write past slot 0 stomps the members declared after it. Archetype:
`MechControlsMapper::pilotArray` — the binary reserves 0x15C..0x183 (10 slots); the `[1]`
declaration let MP's `FillPilotArray` write the PEER's `Player*` over `controlMode`
("can't turn in MP": turn shaping dispatched on pointer garbage → `turnDemand=0`), MASKED
in solo because the overrun wrote 0 == BasicMode. → size the array to the binary's
inter-member span ((next-member offset array offset) / stride) + clamp the fill loops.
Caught live with cdb `ba w4` on the compiled member address (log `&member` from the ctor,
offset delta gives the compiled position). [T2]
`GaugeAlarm54` = 0x54 (the real `AlarmIndicator`; STATUS level at +0x14, so
`subsystem+0x184 == heatAlarm+0x14 == GetLevel()`); `SubsystemConnection` = 0xC. [T1]
## 6. /FORCE trap — unresolved symbol → runtime AV, not a link error
`/FORCE` turns an unresolved external (or a prose-only vtable slot) into a **runtime AV near
`__ImageBase`**, NOT a link error. **When a /FORCE build crashes with a garbage call target
near the image base, grep the link log for "unresolved external" — the "successful" build is
lying.** Corollary: a bridge fn / a `.data` fn-ptr callback MUST have a real (stub) definition.
A `SetVideoPathPriority` defined in an anonymous namespace → internal linkage → unresolved in
another TU → stubbed by /FORCE → AV in `LoadMissionImplementation`. [T2]
## 7. Dtor-epilogue rule — do not reconstruct compiler glue
In a decompiled DESTRUCTOR, the trailing member-dtor calls (`FUN_xxx(this+N, 2)`), the base-dtor
call (`FUN_xxx(this, 0)`), and the `(flags&1) && operator delete(this)` tail are COMPILER GLUE.
Reconstruct only the body ABOVE them; C++ re-emits member+base destruction at the closing brace.
An explicit base-dtor call runs the whole `~JointedMover → ~Mover → ~Entity` chain **TWICE** =
the P5 double-free (re-`delete[]`s `collisionLists`, re-runs `DeletePlugs` over the freed segment
table). ONE bug = BOTH the death-row crash AND the app-exit crash. [T2]
## 8. Bridges — the databinding-safe escape hatch
When TU A needs a raw-offset-safe op but its local RECON stubs collide with the real class
headers, put the op in a **bridge**: a free function in a complete-type TU, `extern`-declared in
A. Examples: `BTResolveWeaponMuzzle` (mech4.cpp — a complete-Mech TU with the segment API),
`BTRecomputeCondenserValves` (heatfamily_reslice.cpp — sees Condenser), `BTResolveMessageBoard`
(btplayer.cpp — complete BTPlayer), `BTGetSubsystemAuxScreen` (powersub.cpp — casts through the
real PoweredSubsystem). Keep the alloc SIZE + special-cache when swapping a factory case. [T2]
## 9. Message-handler chaining + entity validity
- A reconstructed class's `MessageHandlers` set must be built **chained to the parent's**
(`Receiver::MessageHandlerSet(Entity::GetMessageHandlers())`). An empty default-ctor set has
no parent chain → `Receiver::Receive` finds no handler → every inherited message (TakeDamage!)
is **silently dropped**. [T2]
- **Entity validity gates message delivery on BOTH paths, and an unvalidated entity drops everything.**
`Entity::Dispatch` delivers synchronously only for a VALID master (invalid → `Post(EntityInvalidEventPriority)`,
which does re-fire); but a message that arrives as an EVENT — `Entity::Receive(Event*)`, ENTITY.cpp:165,
e.g. any Posted or **cross-pod-delivered** message — does `if(!IsValid()) event->Defer()`, and the
deferred queue never re-fires until the entity becomes valid. A manually-spawned OR network-created entity
(the port's MakeReady/CheckLoad handshake is a partial impl) must call `SetValidFlag()` itself — else EVERY
message defers forever. Force-validate at `Make` (the reconstructed ctor builds the entity synchronously).
Hit by: the spawned dummy, replicants, AND — task #47 — a peer's own **MASTER** mech: cross-pod TakeDamage
reached B, resolved to B's real mech, then `Entity::Receive` saw `valid=0` and deferred it forever → 0
damage. Fix = `Mech::Make` sets `ValidFlag` for the master too (mech.cpp), not just replicants. [T2]
## 10. Container-Execute must override (gauges)
The 2007 engine `Gauge::Execute` base is `Fail("not overridden")``abort()` (GAUGE.cpp:598);
`GuardedExecute`'s SEH **cannot catch abort()**. So a container/parent gauge MUST override
`Execute` (even as a no-op) AND override `BecameActive` with a **non-inactivating** body (the
default `GaugeBase::BecameActive` inactivates). A `GraphicGaugeBackground`-derived widget
(PrepEngrScreen/BackgroundBitmap) has NO Execute virtual → the hazard doesn't apply; there the
overridden slot is `BecameActive`. [T2]
## 11. Dense-table hazard (attribute publishing)
`AttributeIndexSet::Build` leaves gap slots uninitialized and `Find` strcmps EVERY slot → a
published attribute table MUST be a **dense prefix** from the parent's `NextAttributeID`; a gap
AVs. Fill gaps with a shared read-only pad member. Same for a class's `<Name>AttributeID` enum.
[T2]
## 12. Verification gotchas (don't fool yourself)
- **Lazy gauge build:** `GaugeRenderer::BuildConfigurationFile` runs LAZILY. A too-early process
kill shows `[gskip]=0` / "not built" even though the widget is fine — **wait for the gauge
window** before concluding. (Cost a long detour this session.) [T2]
- **ReconStream is a no-op:** `btl4gau3.cpp`'s `DebugStream` is the `ReconStream` whose
`operator<<` is `{ return *this; }` — it DISCARDS everything. Use the engine `DEBUG_STREAM`
(what heat.cpp uses) for a log that reaches the BT_LOG file. [T2]
- **Head-on repro hides intermittent bugs:** a straight ram gives 1 clean result; the bug shows
on GLANCING/sliding/rough-terrain contact. Reproduce with an angled/terrain-crossing approach.
[T2]
- **`static_assert` not runtime `Check`:** a runtime `Check(sizeof<=alloc)` in a factory bridge
does NOT fail the build (it's a runtime assert → heap overflow at construction). Use a
compile-time `static_assert` sizeof lock. [T2]
- **Engine-class new member:** a NEW member on a 2007 engine class (d3d_OBJECT, DPLRenderer) MUST
be initialized in EVERY ctor init-list (debug heap fills 0xCDCDCDCD → an uninit flag reads
TRUE); and any device state a special draw path sets must be save/restored exactly. Deleting
stale `.obj`s fixes layout-mismatch corruption when a base class grows. [T2]
---
## Diagnostic recipe (the standard loop)
1. Read the RAW decomp `reference/decomp/all/part_*.c` for the `FUN_xxxx`.
2. Map `FUN_`/`DAT_`/`this+0xNN` to engine symbols via BT headers + WinTesla MUNGA source +
`CLASSMAP.md` + RP's parallel code.
3. Write the REAL reconstruction; `static_assert`-lock the layout.
4. Build; run env-gated; read `btl4.log`; cdb on any crash (0xCDCDCDCD=uninit, 0xFEEEFEEE=freed).
5. For exhaustive multi-function analysis: a read-only Workflow (understand), then implement hands-on.
## Key Relationships
- Applies to: every topic that reconstructs a class ([[subsystems]], [[combat-damage]], [[gauges-hud]], [[locomotion]]).
- Uses: [[decomp-reference]] (offsets/ClassIDs), [[reconstruction-method]] (the loop).