diff --git a/docs/BRIEF-D122-per-level-modelload.md b/docs/BRIEF-D122-per-level-modelload.md new file mode 100644 index 00000000..6dc98ff3 --- /dev/null +++ b/docs/BRIEF-D122-per-level-modelload.md @@ -0,0 +1,165 @@ +# BRIEF D122 — per-level prop/item model-load crash (blocks the level sweep) + +Repo: `REPO_ROOT` (git, branch `master`, Windows; +PowerShell + Bash tool available). + +## READ FIRST + +- `docs/HANDOFF.md` (top) — current state, environment, repro line. +- `docs/PORT-LEARNINGS.md` — recurring bug classes. **A / C are the + prime suspects here** (pointer-width struct growth; BE rodata on LE). +- `docs/PCPortResearch.md` §F index (top) + entries **D43, D53.2, D56, + D57, D67, D68, D39, D108–D112, D120** — model/asset conversion + + pointer-width history. Jump to each via the index; do NOT linear-read. +- `AGENTS.md` non-negotiables (esp. #2 game logic unmodified — narrow + `#ifdef PORT` ABI/layout/format edits only, each logged in §F). + +## CONTEXT (established this session, M-12) + +- **WS1 landed** (commit `02c12068`, D121): a bare + `./build-pc/ge007.x86_64.exe -level_XX` now auto-injects that stage's + `memallocstringtable[]` pool row. Every solo level boots with correct + pools now — no manual `-m*` needed. +- **D88.4 (propDefs converter) is effectively done for the 21 solo + levels.** `tools_pc/d88_emit.py` + `d88_propdefs.py` are committed + (`ff563812`, `4ccc0d3c`); `python tools_pc/d88_emit.py ntsc-final + --regen` reports `21/21 converted … ALL CHECKS PASSED`. The placeholder + `PROPDEF_PC_BYTES` for VEHICLE/AIRCRAFT/TANK/AMMO are **not exercised by + any solo level** (`tools_pc/d88_propdef_scan.py` histogram — no + VEHICLE/AIRCRAFT/TANK in any of the 21). So D88.4 is NOT the blocker. +- **The actual blocker: prop/item model loading crashes on most levels.** + Smoke test (each `-level_XX`, `GE_PCDUMP="60-240:60"`, ~25 s): + + | level | # | result | + |---|---|---| + | Bunker1 | 09 | renders, playable (known good) | + | Silo | 20 | renders ~91% frame content, no crash | + | Dam | 33 | **CRASH** `modelInitRwData` (`src/game/model.c:6249`) — `node->Opcode` deref, bad `node` in the ModelNode walk | + | Facility | 34 | **CRASH** `modelLoad` (`src/game/loadobjectmodel.c:393`) — `PitemZ_entries[modelid].header->RootNode`, bad `header` (or OOB `modelid`) | + | Runway | 35 | **CRASH** same as Facility (`loadobjectmodel.c:393`) | + + Crashes are **early and deterministic** (before frame 1 renders), same + PC every run — distinct from BUNKER1's rare intermittent + `loadobjectmodel.c:393` (D117 nondeterminism). `EXCEPTION 0xc0000005`, + `FAULT ADDR: ffffffffffffffff`. `Rax` at the Facility crash holds ASCII + bytes ("…o ng nol…") — looks like a model/texture name string landed in + a register, consistent with a mis-strided table read. + +## TASK + +**One question: why do prop/item models load fine for BUNKER1/Silo but +crash for Dam/Facility/Runway, and what is the narrow fix?** + +Almost certainly one of: +1. `PitemZ_entries[341]` (`struct ItemModelFileRecord { ModelFileHeader + *header; char *filename; float scale; }`, `src/bondtypes.h:1466`) — + table stride or the `header`/`filename` pointer values wrong on PC. + Find where this array is **defined** (it is `extern` everywhere I + looked — `src/game/prop.c:40`, `chrobjdata.h:20`; the definition is + likely a generated `.s`/rodata blob or an asset — `grep` harder, + check `port/src/romassets_*.s`, `scripts/gen_romassets.py`, + `scripts/gen_*`). If the `header` pointers come from BE ROM rodata + they need rebasing/byteswap (D39 Globalimagetable / D68 class). +2. `modelid` passed to `modelLoad` is out of range / unswapped — it comes + from a propDef `ObjectRecord`-family `modelid` field. If a record type + used by Dam/Facility props (but not BUNKER1) has its `modelid` at a + field offset the `d88_propdefs.py` converter byteswaps wrongly (or + not at all), `modelid` is garbage. Cross-check the per-type field + maps in `tools_pc/d88_propdefs.py` against the record structs. +3. The `pcmodels` sidecar (`tools_pc/d43_emit.py`, loaded at + `0x10C00000`, "512 sidecars") is missing/mis-converting a model + feature (ModelNode opcode, `PointUsage[]` chain — cf. D120, + `d43_emit.py` opcode-0x18 gap) that Dam/Facility prop models use. + `model.c:6249` already has a `GE_D86` env probe for the node walk — + use it. + +Determine which, root-cause, and apply the **narrowest** fix (converter +extension preferred over runtime fixup for a whole format — D43/D88 +pattern; or a narrow `#ifdef PORT` ABI edit with the N64 line kept under +`#else`). + +## FILES YOU MAY TOUCH + +- `src/game/model.c`, `src/game/loadobjectmodel.c`, `src/game/prop.c` + (narrow `#ifdef PORT` only). +- `tools_pc/d43_emit.py` (+ `tools_pc/d43_*.py` helpers, `d43_layoutprobe.c`). +- `tools_pc/d88_propdefs.py` / `tools_pc/d88_emit.py` (only if the cause + is a propDef `modelid` field). +- `port/src/` model/asset shims if the table is patched at runtime like + `pcmodelsPatchTable` (`grep pcmodelsPatchTable`, `port/src/romdata*.c`). +- New probe files: `tools_pc/d122_*.py` / env-gated `GE_D122` blocks. +- `docs/PORT-LEARNINGS.md` (append any new generalisable quirk), + `docs/PCPortResearch.md` §F (write up as **D122**, add to the index). + +Nothing else is running — no file collisions to worry about. + +## KNOWN-GOOD / RULED OUT — do not re-investigate + +- WS1 per-level pool injection (D121) — works, committed. +- D88.4 propDefs stream conversion for solo levels — works (21/21). +- BUNKER1 + Silo prop/model loading — works. +- The intermittent BUNKER1 `loadobjectmodel.c:393` crash — that is + D117-class nondeterminism, separate; do not chase it, use the + deterministic Dam/Facility/Runway repro. +- `d88_propdef_scan.py` "STOP … unmapped type NOTHING" — that is the + *scan tool's* limitation, not a converter bug (`d88_emit.py` completes). + +## PRE-FLIGHT (done — results above) + +- `./build-pc.sh ntsc-final` green; binary at `build-pc/ge007.x86_64.exe`. +- Sidecars regenerated this session (`d43_emit.py`, `d69_emit.py`, + `d88_emit.py --regen`) — all pass. +- Smoke-test table above. + +## ENVIRONMENT + +```sh +export PATH="/c/msys64/mingw64/bin:$PATH" +./build-pc.sh ntsc-final # ~5 s, from repo root +./build-pc/ge007.x86_64.exe -level_34 # Facility repro (bare — D121) +``` + +- `timeout` does NOT reliably kill the game on Windows. Launch with + `… &`, `sleep N`, then `kill $PID; pkill -f ge007`. +- Crash log: `ge007.crash.log` (repo root). Symbolicate: + `addr2line -e build-pc/ge007.x86_64.exe -f -C 0x` (image base + `0x140000000`; PC in the log is already absolute). +- Frame capture: `GE_PCDUMP="a-b:step"` → `./ppm/`; content check + `python tools_pc/pixcount.py ppm/frame_XXXX.ppm`. +- gdb: **attach** mode is fast (`gdb -batch -x cmds.txt -p `, + winpid = 4th col of `ps -p `; game must already be running). + Launch mode is too slow. A hardware watchpoint on `PitemZ_entries` or + the bad `node` catches the corrupting write fast. +- Standalone probe compiles: `-std=c11`. + +## BUDGET + +Structural bug → generous: **~12 build→run→inspect cycles or ~90 min**. +On expiry: revert all temp probes (leave only committed env-gated ones), +write up the spec progress + best hypothesis in `docs/PCPortResearch.md` +§F as **D122** with an explicit confidence rating, and list exactly which +files you changed. A good half-solved write-up is a deliverable. + +## CONSTRAINTS + +- No game-logic / control-flow changes. ABI/layout/format only, each + `#ifdef PORT` with the N64 line kept verbatim under `#else`, each + documented in §F/D122. +- Prefer an offline sidecar-converter extension over a runtime fixup for + a whole asset format. +- Revert your own temp probes before reporting. +- Regenerate sidecars after any converter change: + `python tools_pc/d43_emit.py ntsc-final && python tools_pc/d69_emit.py + ntsc-final && python tools_pc/d88_emit.py ntsc-final --regen`. + +## REPORT + +1. Root cause — `file:line` evidence, which of the 3 hypotheses (or + other), why Dam/Facility/Runway differ from BUNKER1/Silo. +2. Fix — the diff, or why-not if unresolved. +3. Verification — `-level_33`, `-level_34`, `-level_35` each boot to ≥1 + non-degenerate frame (`pixcount.py` > a few %), AND `-level_09` + (BUNKER1) still renders (no regression — `tools_pc/framediff.py`). +4. Probes left in the tree (env-gated, capped). +5. Confidence rating. +6. Append any generalisable quirk to `docs/PORT-LEARNINGS.md`. diff --git a/docs/PORT-LEARNINGS.md b/docs/PORT-LEARNINGS.md index 191d2a3a..bc341be8 100644 --- a/docs/PORT-LEARNINGS.md +++ b/docs/PORT-LEARNINGS.md @@ -53,6 +53,16 @@ through a converter or a runtime bswap fixup reads scrambled. (StandTile id/room, header mid/tail). - **Rule:** prefer an offline sidecar converter (D43, D69, D88 pattern, `tools_pc/d*_emit.py`) over a runtime fixup for a whole format. +- A polymorphic-record converter with a per-type handler table + a + "generic" fallback silently corrupts any type the table forgot: the + fallback's word-granular bswap is wrong for sub-word fields. D122 — + `d88_propdefs.py` had no arm for 6 `inherits ObjectRecord` propDef types + (TINTED_GLASS/VEHICHLE/AIRCRAFT/TANK/AUTOGUN/AMMO); the generic arm + bswap32'd the `[s16 obj][s16 pad]` word as one u32, putting the model id + in the wrong half → OOB table deref. When you add a converter, enumerate + *every* `type` byte a level can emit and assert the handler set is + total; `[u16|u16]` / `[s16|s16]` packed words need a half-swap + (`_hh_word`), never a 32-bit swap. ## C2. Port-layer / SDL shims diff --git a/src/game/loadobjectmodel.c b/src/game/loadobjectmodel.c index c6583a64..45566951 100644 --- a/src/game/loadobjectmodel.c +++ b/src/game/loadobjectmodel.c @@ -91,10 +91,10 @@ s32 sizepropdef(PropDefHeaderRecord *pdef) case PROPDEF_ARMOUR: return 38; case PROPDEF_COLLECTABLE: return 40; case PROPDEF_TINTED_GLASS: return 42; - case PROPDEF_VEHICHLE: return 44; - case PROPDEF_AIRCRAFT: return 45; + case PROPDEF_VEHICHLE: return 52; /* D122: was 44 (N64) */ + case PROPDEF_AIRCRAFT: return 52; /* D122: was 45 (N64) */ case PROPDEF_AMMO: return 50; - case PROPDEF_TANK: return 56; + case PROPDEF_TANK: return 62; /* D122: was 56 (N64) */ case PROPDEF_AUTOGUN: return 62; case PROPDEF_CCTV: return 68; case PROPDEF_MONITOR: return 72; diff --git a/tools_pc/d88_propdefs.py b/tools_pc/d88_propdefs.py index dcc5e049..60a0ccbb 100644 --- a/tools_pc/d88_propdefs.py +++ b/tools_pc/d88_propdefs.py @@ -83,6 +83,23 @@ OBJ_PTR_WORDS = (4, 5, 26, 27) OBJ_N64_WORDS = 32 OBJ_PC_BYTES = 144 +# D122: ObjectRecord-derived types whose tail (N64 words >= 32) the converter +# must lay out with pointer widening. {type: (ptr_word_set, hh_word_set)}. +# 47 TintedGlassRecord tail: TintDist,CullDist,calculatedopacity,portalnum,unk90 +# 39 VehichleRecord ailist*@w32, [u16 aioffset|s16 aireturnlist]@w33, path*@w41, Sound*@w43 +# 40 AircraftRecord ailist*@w32, [u16|s16]@w33, path*@w43, Sound*@w44 +# 45 TankRecord collision*@w32 (rest scalar; struct locs unconfirmed) +OBJ_TAIL_DESC = { + 47: (frozenset(), frozenset()), + 39: (frozenset((32, 41, 43)), frozenset((33,))), + 40: (frozenset((32, 43, 44)), frozenset((33,))), + 45: (frozenset((32,)), frozenset()), + # 13 AutogunRecord: unkC4*@w49 unkC8*@w50 beam*@w51 (is_active@w52 unkD4@w53) + 13: (frozenset((49, 50, 51)), frozenset()), + # 20 MultiAmmoCrateRecord: slots[13] of [u16 modelnum][u16 quantity] + 20: (frozenset(), frozenset(range(32, 45))), +} + # Per-type PC struct size (bytes) the converter emits and sizepropdef must # return / 4. For ObjectRecord-derived types this is 144 + tail growth. PROPDEF_PC_BYTES = { @@ -114,10 +131,10 @@ PROPDEF_PC_BYTES = { 35: 16, # WATCH_MENU_OBJ_TEXT (4w) 37: 48, # RenameObjectRecord 38: 32, # LockDoorRecord - 39: 176, # VehichleRecord (guess: 44w + tail; refine if a level needs it) - 40: 180, # AircraftRecord + 39: 208, # VehichleRecord (D122: 144 prefix + widened tail) + 40: 208, # AircraftRecord (D122) 44: 24, # SafeObjectRecord fragment (5w + next*), refine on use - 45: 224, # TankRecord + 45: 248, # TankRecord (D122: 144 prefix + 23-word scalar tail + collision*) 46: 28, # CutsceneRecord 47: 168, # TintedGlassRecord 48: 4, # END @@ -275,6 +292,29 @@ def convert_record(src, so, type_byte): # word 6 = Data* -> zero @24 (already; 20-byte u16 block then 8-align pad@20) return bytes(out) + if type_byte in OBJ_TAIL_DESC: # D122: ObjectRecord prefix + typed tail + # 47 TINTED_GLASS / 39 VEHICHLE / 40 AIRCRAFT / 45 TANK. These all + # `inherits ObjectRecord` but had no handler -> fell through to the + # generic arm, which bswap32'd N64 word 1 ([s16 obj][s16 pad]) as one + # 32-bit value. That put `obj` in the wrong half -> garbage modelid -> + # OOB PitemZ_entries[] deref crash (loadobjectmodel.c:393 / + # model.c:6249). Emit the real 144B PC ObjectRecord prefix, then the + # tail with pointer members widened 4->8B and 8-aligned. + _emit_object_prefix(out, src, so) + ptr_words, hh_words = OBJ_TAIL_DESC[type_byte] + pc = OBJ_PC_BYTES + for i in range(OBJ_N64_WORDS, n64w): + w = src[so + 4 * i: so + 4 * i + 4] + if i in ptr_words: + pc = (pc + 7) & ~7 + pc += 8 # 8 zero bytes already present + elif i in hh_words: + out[pc:pc + 4] = _hh_word(w); pc += 4 + else: + out[pc:pc + 4] = _bswap32(w); pc += 4 + assert pc <= pcb, (type_byte, pc, pcb) + return bytes(out) + # ---- generic: header-only records (objectives, END, DOOR_SCALE, ...) ---- # No serialized pointers. header word special, rest plain bswap32. out[0:4] = _hdr_word(src[so:so + 4])