mirror of
https://github.com/jkdansereau/goldeneye-pc-port.git
synced 2026-10-23 15:52:42 +02:00
fix(D255): correct 32-bit pointer truncation in vtxstore_fix_refs
Root-caused via a live Steam Deck session over SSH: sub_GAME_7F09BAC4 (vtxstore_fix_refs) read an active OBJ record's Model* through ((Model*)((ChrRecord*)prop->chr)->chrflags) - a union pun onto ChrRecord.chrflags, which is typed CHRFLAG, a 4-byte enum. On 32-bit N64 this is bit-exact (the enum and a pointer are both 4 bytes, so casting the enum's value to Model* reproduces the real stored bytes). On 64-bit PC, Model* is 8 bytes but the enum read is still only 4 - the cast reads just the pointer's low 32 bits as an int and zero-extends it back to a 64-bit address, discarding the real high bits. This is the D3x pointer-width ABI class from AGENTS.md - a struct field misread across the 32-to-64-bit pointer-width transition. This was actually flagged as suspect (a) in the very first D255 investigation, then wrongly deprioritized in favor of a "destroy-path race" theory once a register capture happened to show Rdi=(nil) and looked like it confirmed a different, unrelated NULL-model theory. It didn't: (nil) is exactly what a 32-bit-truncated read produces whenever the real pointer's low 32 bits happen to be zero - a coincidence that pointed two sessions' worth of investigation at the wrong field. Proof: reading the same record through the correctly 64-bit-typed ObjectRecord.model field (this session's diagnostic probe) was always a valid, non-NULL pointer - the object's model was never actually missing. Fix: read the pointer through ObjectRecord.model (the same union memory, correctly typed) instead of through the mistyped ChrRecord.chrflags alias, at both use sites in the function. Identical bytes and identical result on N64; correct on 64-bit PC. Verified live on the user's own Steam Deck hardware over SSH, running their own prepared ROM assets: held through the entire Facility level including repeated terminal destruction (the most consistent prior repro) and through Caverns' radio-objective terminals (the second confirmed level), zero crashes. Windows rebuild + full 21-level verify.sh sweep also clean. The separate audio-thread crash (D285, caught during this same live session) is unrelated and unaffected by this fix.
This commit is contained in:
1 parent
49f18ad1e6
commit
7ef0395a3f
4 files changed
+49
-52
No files matched your search
+10
-13
@@ -140,19 +140,16 @@ Small hotfix batch from user playtest feedback on v0.2.0:
|
||||
behavior even after the D259 fix above. We recommend leaving it off unless
|
||||
you specifically want the unlock goodies and are willing to accept an
|
||||
experimental feature (D259/D257).
|
||||
- **Linux / Steam Deck: an intermittent SIGSEGV remains, with a reliable
|
||||
repro found this release** — Facility's computer terminals, the ones you
|
||||
activate to open a door for level progression, consistently crash the
|
||||
game when destroyed (D255; not Deck-hardware-specific, seen on Linux
|
||||
generally). Confirmed on real Steam Deck hardware across multiple levels:
|
||||
every terminal on Facility, terminals near the radio objective on
|
||||
Caverns, and a similar crash on Archives when a guard's grenade
|
||||
detonates — all point at the same underlying bug (an object record whose
|
||||
model reference goes NULL while the record is still active for one more
|
||||
tick), triggered by an object exploding/being destroyed rather than by
|
||||
simple interaction. Not (yet) reproduced by shooting a room of terminals
|
||||
on Frigate, so the trigger isn't purely "destroy any terminal" — still
|
||||
under investigation. Please report any other Deck/Linux-specific faults.
|
||||
- ~~Linux / Steam Deck: an intermittent SIGSEGV, reproducible by destroying
|
||||
Facility's computer terminals or other exploding objects~~ **FIXED
|
||||
(D255).** Root cause: a 32-bit pointer read through a mistyped field
|
||||
(`ChrRecord.chrflags`, a 4-byte enum aliased onto a real 8-bit pointer
|
||||
field) silently truncated the pointer on 64-bit builds — harmless on the
|
||||
original 32-bit N64, broken on PC. Verified live on real Steam Deck
|
||||
hardware: held through the entire Facility level (including repeated
|
||||
terminal destruction) and through Caverns' radio-objective terminals
|
||||
with zero crashes. Please still report any other Deck/Linux-specific
|
||||
faults.
|
||||
- **Steam Deck / gamepad: the right analog stick currently navigates the
|
||||
main menu, file select, and mission-select map** — not the left stick.
|
||||
This matches the N64 default but is unintuitive on a modern controller;
|
||||
|
||||
@@ -150,7 +150,7 @@ D251,FIXED,F10 options overlay QoL pass:,"FIXED (port-only, port/src/optionsover
|
||||
D252,?,"Particle effects cycle through a rainbow palette (M-124, user playtest of the v0.2.0-pre candidate).",
|
||||
D253,FIXED,Facility (and any level with a FORCECROUCH/LADDER linked special tile) SIGSEGV on Linux/Steam Deck —,FIXED — Deck-verified:
|
||||
D254,FIXED,Linux crash handler destroyed its own logs:,FIXED — field-verified.
|
||||
D255,OPEN,"Facility firefight SIGSEGV on Steam Deck (v0.2.0-pre CI bundle, D253 fix confirmed working —","OPEN — case identified (NULL model on an active OBJ record; destroy-path ordering); confirmed trigger is destruction-via-explosion (Facility terminals, Caverns terminals, Archives grenade —"
|
||||
D255,?,"Facility firefight SIGSEGV on Steam Deck (v0.2.0-pre CI bundle, D253 fix confirmed working —",
|
||||
D256,FIXED,Saves (and config) silently failed to persist on Linux whenever the game was not launched from the directory containing data/ —,FIXED (path half) — save-flow detail still owed from the user.
|
||||
D257,FIXED,New Game.AllUnlocked option (F10; default flipped OFF for v0.2.0 —,FIXED — verified by user playtest (PR #79 merged); default flipped OFF for v0.2.0 (D259).
|
||||
D259,FIXED,Game.AllUnlocked (D257) breaks audio on a fresh save:,FIXED in v0.2.1 — the deferred fix was applied:
|
||||
|
||||
|
@@ -546,7 +546,9 @@ covers D24–D69; the log continues in §H (D32 procedure, D70–D121).
|
||||
| D252 | **Particle effects cycle through a rainbow palette (M-124, user playtest of the v0.2.0-pre candidate).** Shooting repeatedly makes the bullet-impact "sparks" slowly progress through the full hue wheel (~10 shots to sweep it); other lingering particle effects — smoke / explosion residue — show the same rainbow colouration instead of their intended grey/orange palette. **Mechanism context (verified this session):** particle tris carry no per-primitive material; their colour comes entirely from the ROM-resident state records `globalDL_0x078 … globalDL_0xa50` (`assets/oddtextures.c`, 15 of them) replayed per particle batch (`explosion.c:894`). Each record binds a **distinct** texture pair — `IMAGE_SMOKE_N` (IA, tile 0 @ TMEM 0) × `IMAGE_FIRE_N` (RGBA16, tile 1 @ TMEM 0x188) under `G_CC_INTERFERENCE` (TEXEL0·TEXEL1), and `explosionRenderPart` uses a fixed all-white default vertex (`explosion.c:215`) — so the intended per-stage colour is whatever those 15 fire/smoke pairs encode, and particles **age through the records** (`var_s2` 14→0 at `frame/flareanimspeed`), so a per-texture decode error surfaces exactly as a hue sweep over time / across shots. **Ruled out this session:** (a) the M-82/D172 tile-offset fix is correctly configured for this path — `gsDPSetTextureLOD(G_TL_TILE)` does NOT set the `G_TL_LOD` bit, so `rdp.tex_lod` stays false (`gfx_pc.cpp:2860`) and `gfx_lod_tile_offset` returns `i`, i.e. TEXEL1 genuinely samples tile 1; (b) the RGBA16/5551 texel decode in `import_texture_rgba16` is correct (big-endian, R=bits15-11 G=10-6 B=5-1 A=bit0). **Prime suspect: the width/height derivation in `import_texture_rgba16`** — `width = rdp.texture_tile[tile].line_size_bytes / 2`, `height = size_bytes / line_size_bytes`. The fire tiles are 16 px wide with a MASK_16 TMEM line (64 bytes), so `width` computes 32 (true: 16) and, with `size_bytes` = the LOADBLOCK's 16×14×2 = 448, `height` computes 7 (true: 14) — the GL texture would be built from the right pixels at the wrong dimensions, scrambling each of the 15 fire sprites into a different wrong-hued smear; that fits "slow rainbow progression" precisely (each aging stage = a different scrambled sprite). **Not yet confirmed** — needs one imported fire tile dumped and compared against the raw `IMAGE_FIRE_N` bytes (`GE_TEXDUMP`/`GE_PCDUMP`, or a targeted probe at the `upload_texture` call). **Per user instruction: documented only, NOT fixed for v0.2.0-pre** — ships as a known issue; pick up afterwards (cheap if the suspect holds: derive width/height from the LOADBLOCK's dtx/dlh rather than the tile line size, or special-case non-32-texel-wide RGBA16 tiles). D172/M-82 lineage. |
|
||||
| D253 | **Facility (and any level with a FORCECROUCH/LADDER linked special tile) SIGSEGV on Linux/Steam Deck — `stanCheckLinkedSpecialTile` stores to a garbage `outFlags` pointer (D203 root cause).** The B-callback typedef `standTileLocusCallback_B_t` (`bondtypes.h:573`) declares the middle three params as `f32 f32 f32`, so on x86-64 SysV the caller (`sub_GAME_7F0B1DDC`, `stan.c:2228`) passes the 6th arg (`record`) in **`%rdx`** (3rd integer slot; the floats take xmm0–2). But the actual implementation `stanCheckLinkedSpecialTile` was decompiled with an all-`s32` prototype, so it reads `outFlags` from **`%r9`** (6th integer slot) — a stale scratch value left by earlier callees. On the N64 (MIPS o32) both prototypes agree: everything flows through GPRs a0–a5 in order, 6th arg = a5 either way, so the original is fine and the decompilation looked consistent. The mismatch stays dormant until a tile actually hits the FORCECROUCH or LADDER branch and *stores* through `outFlags` → SEGV; that's why it was level-specific (Facility's opening crouch tile) and input-dependent, and why idle repros were negative. The sibling B-callback `stanLocusEdgeIsAboveY` already had the correct float prototype, which is why only this path was booby-trapped. **Fix (ABI-class, semantics-preserving):** declare the three middle params of `stanCheckLinkedSpecialTile` as `f32` in both the forward decl and definition (`stan.c:152`, `stan.c:2407`) to match the typedef; the body never uses them, and on N64 o32 f32 args occupy the same GPRs, so N64 codegen is unaffected. **Verified:** gdb run on WSL (gcc 15) — pre-fix, SIGSEGV at `stan.c:2441` with `r9=0x3` and valid `record=0x409fea00` in the caller frame; post-fix, the same walk route completes with no crash. Side benefit: the FORCECROUCH/LADDER branches were effectively dead-on-arrival on PC (any hit = crash); they now function. **Also observed post-fix (separate, WSLg-only):** a segfault *inside libgallium* during `gfx_opengl_upload_texture` (32×48 RGBA + mipmaps) while walking further — GL-driver-side under WSLg's Mesa; not reproducible on Windows; park as a new finding if it recurs on real hardware. | FIXED — Deck-verified: user got past the crouch tile with the re-cut bundle (a *new*, later crash is D255). |
|
||||
| D254 | **Linux crash handler destroyed its own logs: `ge007.crash.log` only ever contained the SIGABRT block, never the original SEGV.** The POSIX `crashHandler` (`port/src/crash.c`) finished by calling `sysFatalError()` → `abort()` → SIGABRT, which re-entered the *same* handler (SIGABRT was one of the installed signals); the second pass reopened the log with `"wb"` and overwrote the first pass's SEGV block — faulting PC, signal, everything. The Deck firefight crash (D255) came back as a bare `SIGNAL: 6` abort record for exactly this reason, forcing offline disassembly of the CI binary to recover what the log should have said. **Fix (port-only):** re-entrancy guard (`inCrashHandler`, second pass `_exit(128+sig)` immediately) + terminate directly with `_exit()` instead of `sysFatalError()` (the Windows path already terminated directly for the same reason) + the Linux block now dumps the faulting context: `FAULT ADDR` (`si_addr`) and the full general register set (Rax/Rcx/Rdx/Rsi/Rdi/R8–R11, Rsp/Rbp) from the `ucontext`, so a field crash now names the bad pointer on its face. **Field-verified 2026-09-13:** the D255 Deck crash came back as `SIGNAL: 11` + `FAULT ADDR: 0x10` + full register set — exactly the format designed here; it directly identified the NULL-model case (see D255). | FIXED — field-verified. |
|
||||
| D255 | **Facility firefight SIGSEGV on Steam Deck (v0.2.0-pre CI bundle, D253 fix confirmed working — user got past the opening crouch tile).** Crash during gunplay; abort-unwind backtrace points at `sub_GAME_7F09BAC4` (`vtxstore.c:158`) ← `sub_GAME_7F09BBBC` (`:220`) ← `lvlManageMpGame` (`lv.c:1302`) ← `bossMainloop` (`boss.c:567`). Disassembling the exact CI binary at the fault PC (`0x200db135`) gives `mov 0x14(%rax),%edi; mov 0x10(%rdi),%rax`: the function walks the active PropRecord list keeping `type == 1` — which is **PROP_TYPE_OBJ, not CHR** (the decompiler cast to the union's first member and misnamed the field `ChrRecord.chrflags`; it is actually `ObjectRecord.model` at `0x14`) — then dereferences `model->obj`. Two port-relevant wrinkles: (a) the load is **32-bit zero-extended** because the decomp types the field through the `CHRFLAG` enum, so a model pointer ≥ 4 GB would truncate to garbage; (b) the loop skips records whose `model->obj` is NULL but *dereferences* `model` unconditionally, so an active object with a NULL/freed `model` faults — plausible in a firefight where destructibles (crates etc.) die. Not reproduced on WSL after equivalent play. **Register capture (D254 format, 2026-09-13):** two more Deck crashes — Bunker mid-firefight, then Frigate with ~3 NPCs alive; log: `SIGNAL: 11 / FAULT ADDR: 0x10 / Rax=0x701e5f54 Rdi=(nil) / PC 0x200db135`. `Rdi` = the record's model value = **exactly NULL** → case (b) confirmed: an active PROP_TYPE_OBJ record with a NULL `model`, dereferenced by the unconditional `mov 0x10(%rdi),%rax`; truncation (a) and stale/garbage pointers are ruled out. Faulting PC matches the recorded disassembly (`sub_GAME_7F09BAC4` = `vtxstore_fix_refs`). Grep-verified: nothing in game code ever writes NULL into `ObjectRecord.model` — `clear_model_obj()` nulls `model->obj` only, and `objInit`/spawn paths only activate a record once model ≠ NULL — so the NULL comes from either a fresh/uninitialized record slot being activated or a destroy-path ordering race (model freed/nulled while the record is still active this tick). **Fix constraint:** any guard lands in game code (`vtxstore.c`) → needs rule-#2 sign-off; `port/` cannot intercept this dereference. Next: (1) decision on whether a NULL-model skip in `vtxstore_fix_refs` qualifies as an ABI-class exception (semantics-preserving defensive guard, PD analogue check); (2) if yes, add it plus a `#ifdef PORT` trace at the deactivation/model-free sites to name the path that leaves the record model-less. **Corrected, deterministic repro (v0.2.1 user testing, 2026-09-15, superseding the initial report): the trigger is Facility's computer terminals — the ones you ACTIVATE (interact with, not shoot/destroy) to open a door for level progression.** User confirms this is consistent/reliably reproducible on those specific terminals, on Steam Deck **and** Linux generally (not Deck-hardware-specific). This is a much stronger lead than the original "sometimes during firefights" — a terminal activation deactivating/freeing its associated object model while the record briefly stays active this tick is fully consistent with the destroy-path-ordering theory above, just via a scripted trigger instead of combat destruction, and gives a deterministic repro instead of a probabilistic one. **Next session should reproduce via terminal activation specifically**, not generic combat — much cheaper to instrument. **M-140: added a diagnostic probe (`GE_D255`, off by default, `#ifdef PORT`) — `vtxstore.c`'s `sub_GAME_7F09BAC4` now skips-and-logs instead of dereferencing when it finds a NULL model on an active OBJ record (identity + state fields), and `propobj.c`'s `objFree` logs every free call for correlation. Attempted a live Windows repro this session (5 terminal activations across 3 Facility launches, one direct `-level_34` boot each time): zero probe hits, zero crashes — Windows does not reproduce this, consistent with the existing WSL-negative result. Two negatives (WSL, native Windows) against one positive (Linux/Steam Deck) points at something genuinely platform-specific (allocator/timing), not a Windows-reachable path — don't spend further Windows session time chasing a live repro.** The probes are harmless (env-gated, zero behavior change when unset) and committed so a Linux/Deck session can set `GE_D255=1` and get the trace directly. **User report, 2026-09-15 (same day, later, real Steam Deck hardware on the published v0.2.1 release — unverified by this session, logging as reported, not re-asserting the activation theory as wrong):** Facility crashes **consistently on terminals being destroyed** (shot), not (only) on activation. Frigate crashed once during this same testing pass, specifics not recorded. Shooting terminals elsewhere/other attempts did not reliably reproduce. This is closer to the *original* pre-M-139 framing (combat/destructibles triggering the NULL-model race) than the M-139 "corrected, acLine truncated
|
||||
| D255 | **Facility firefight SIGSEGV on Steam Deck (v0.2.0-pre CI bundle, D253 fix confirmed working — user got past the opening crouch tile).** Crash during gunplay; abort-unwind backtrace points at `sub_GAME_7F09BAC4` (`vtxstore.c:158`) ← `sub_GAME_7F09BBBC` (`:220`) ← `lvlManageMpGame` (`lv.c:1302`) ← `bossMainloop` (`boss.c:567`). Disassembling the exact CI binary at the fault PC (`0x200db135`) gives `mov 0x14(%rax),%edi; mov 0x10(%rdi),%rax`: the function walks the active PropRecord list keeping `type == 1` — which is **PROP_TYPE_OBJ, not CHR** (the decompiler cast to the union's first member and misnamed the field `ChrRecord.chrflags`; it is actually `ObjectRecord.model` at `0x14`) — then dereferences `model->obj`. Two port-relevant wrinkles: (a) the load is **32-bit zero-extended** because the decomp types the field through the `CHRFLAG` enum, so a model pointer ≥ 4 GB would truncate to garbage; (b) the loop skips records whose `model->obj` is NULL but *dereferences* `model` unconditionally, so an active object with a NULL/freed `model` faults — plausible in a firefight where destructibles (crates etc.) die. Not reproduced on WSL after equivalent play. **Register capture (D254 format, 2026-09-13):** two more Deck crashes — Bunker mid-firefight, then Frigate with ~3 NPCs alive; log: `SIGNAL: 11 / FAULT ADDR: 0x10 / Rax=0x701e5f54 Rdi=(nil) / PC 0x200db135`. `Rdi` = the record's model value = **exactly NULL** → case (b) confirmed: an active PROP_TYPE_OBJ record with a NULL `model`, dereferenced by the unconditional `mov 0x10(%rdi),%rax`; truncation (a) and stale/garbage pointers are ruled out. Faulting PC matches the recorded disassembly (`sub_GAME_7F09BAC4` = `vtxstore_fix_refs`). Grep-verified: nothing in game code ever writes NULL into `ObjectRecord.model` — `clear_model_obj()` nulls `model->obj` only, and `objInit`/spawn paths only activate a record once model ≠ NULL — so the NULL comes from either a fresh/uninitialized record slot being activated or a destroy-path ordering race (model freed/nulled while the record is still active this tick). **Fix constraint:** any guard lands in game code (`vtxstore.c`) → needs rule-#2 sign-off; `port/` cannot intercept this dereference. Next: (1) decision on whether a NULL-model skip in `vtxstore_fix_refs` qualifies as an ABI-class exception (semantics-preserving defensive guard, PD analogue check); (2) if yes, add it plus a `#ifdef PORT` trace at the deactivation/model-free sites to name the path that leaves the record model-less. **Corrected, deterministic repro (v0.2.1 user testing, 2026-09-15, superseding the initial report): the trigger is Facility's computer terminals — the ones you ACTIVATE (interact with, not shoot/destroy) to open a door for level progression.** User confirms this is consistent/reliably reproducible on those specific terminals, on Steam Deck **and** Linux generally (not Deck-hardware-specific). This is a much stronger lead than the original "sometimes during firefights" — a terminal activation deactivating/freeing its associated object model while the record briefly stays active this tick is fully consistent with the destroy-path-ordering theory above, just via a scripted trigger instead of combat destruction, and gives a deterministic repro instead of a probabilistic one. **Next session should reproduce via terminal activation specifically**, not generic combat — much cheaper to instrument. **M-140: added a diagnostic probe (`GE_D255`, off by default, `#ifdef PORT`) — `vtxstore.c`'s `sub_GAME_7F09BAC4` now skips-and-logs instead of dereferencing when it finds a NULL model on an active OBJ record (identity + state fields), and `propobj.c`'s `objFree` logs every free call for correlation. Attempted a live Windows repro this session (5 terminal activations across 3 Facility launches, one direct `-level_34` boot each time): zero probe hits, zero crashes — Windows does not reproduce this, consistent with the existing WSL-negative result. Two negatives (WSL, native Windows) against one positive (Linux/Steam Deck) points at something genuinely platform-specific (allocator/timing), not a Windows-reachable path — don't spend further Windows session time chasing a live repro.** The probes are harmless (env-gated, zero behavior change when unset) and committed so a Linux/Deck session can set `GE_D255=1` and get the trace directly. **User report, 2026-09-15 (same day, later, real Steam Deck hardware on the published v0.2.1 release — unverified by this session, logging as reported, not re-asserting the activation theory as wrong):** Facility crashes **consistently on terminals being destroyed** (shot), not (only) on activation. Frigate crashed once during this same testing pass, specifics not recorded. Shooting terminals elsewhere/other attempts did not reliably reproduce. This is closer to the *original* pre-M-139 framing (combat/destructibles triggering the NULL-model race) than the M-139 "corrected, acLine truncated
|
||||
|
||||
**ROOT-CAUSED AND FIXED, same day, live Deck session over SSH (M-140).** The `objFree`/destroy-path-race theory above was a red herring — set up a real Deck test (SSH access, key-based auth, WSL-cross-compiled local Linux build deployed and run live against the user's own prepared ROM assets), reproduced the crash directly, and symbolized it on-device against the build's own debug info. **First capture with the `GE_D255` probe active:** the probe's `ObjectRecord.model == NULL` guard did NOT fire, yet the game crashed anyway at `vtxstore.c:199` (`var_v1->RootNode`), `FAULT ADDR: 0x10`, `Rdi=(nil)` — a *different* pointer than the one the probe was checking. **Actual bug:** `sub_GAME_7F09BAC4` (`vtxstore_fix_refs`) reads the record's `Model*` through `((Model*)((ChrRecord*)prop->chr)->chrflags)` — a union pun onto `ChrRecord.chrflags`, which is typed `CHRFLAG`, a **4-byte enum**. On 32-bit N64 this is bit-exact (both the enum and a pointer are 4 bytes, so casting the enum's value to `Model*` reproduces the real stored bytes). On 64-bit PC, `Model*` is 8 bytes but the enum read is still only 4 — the cast reads just the pointer's low 32 bits as an `int` and zero-extends it back to a 64-bit address, discarding the real high bits entirely. This is the **exact D3x pointer-width ABI class** documented in AGENTS.md/porting-notes — a struct field misread across the 32→64-bit pointer-width transition — and it was *already flagged* as suspect (a) in this very row's first paragraph, back in the original v0.2.0-pre investigation, then wrongly deprioritized in favor of suspect (b) (the NULL-model theory) once a live register capture happened to show `Rdi=(nil)` and looked like it confirmed (b). It didn't: `(nil)` is exactly what a 32-bit-truncated read produces whenever the real pointer's low 32 bits happen to be zero — a coincidence that pointed the whole investigation at the wrong field for two sessions. **Proof it was the field, not the object:** this session's `GE_D255` probe read the SAME record through the *correctly*-typed `ObjectRecord.model` field (`Model*`, 8 bytes) and it was **always a valid, non-NULL pointer** — the object's model was never actually missing. **Fix (ABI-class, semantics-preserving, `#ifdef PORT`; `src/game/vtxstore.c`, `sub_GAME_7F09BAC4`):** read the pointer through `ObjectRecord.model` (the same union memory, correctly typed as `Model*`) instead of through the mistyped `ChrRecord.chrflags` alias, at both use sites in the function (the `RootNode` walk and the `modelGetNodeRwData` call). Identical bytes and identical result on N64 (both fields are 4 bytes there); correct on 64-bit PC. The earlier `GE_D255` diagnostic skip-guard (which checked the wrong field and would never have actually fired) is removed — it's no longer needed, the record's model was never really missing. **Verified live, in order:** (1) diagnostic build with the guard checking BOTH fields caught the exact moment of divergence — `ObjectRecord.model` valid, `ChrRecord.chrflags`-cast pointer producing the crash — nailing the mechanism beyond doubt; (2) the real fix (no guard, just the corrected field) held through the *entire* Facility level including multiple terminal destructions (the most consistent prior repro) and through Caverns' radio-objective terminals (the second confirmed level) with zero crashes, live on the user's own Steam Deck hardware over the ROM/assets they already had prepared. Windows rebuild + full 21-level `verify.sh sweep` also run this session as the standard regression gate. | **FIXED** — root-caused (D3x-class 32-bit pointer truncation via a mistyped enum-to-pointer cast, not a destroy-path race) and verified live on real Steam Deck hardware across both confirmed-repro levels (Facility, Caverns), zero crashes; Windows sweep clean. The separate audio-thread crash (D285) is untouched by this fix. |
|
||||
| D256 | **Saves (and config) silently failed to persist on Linux whenever the game was not launched from the directory containing `data/` — reported as "saves do not work at all" on Steam Deck.** `romdataInit` finds the ROM in `$S/` (CWD `data/`) *or* `$E/` (exe dir) *or* `./`, but `sysResolvePath`'s POSIX `$S` branch (used by `ge007.eep` and `ge007.ini`) only ever produced a CWD-relative path — its "fallback" was `./data/`, identical to the primary. Launch from any other CWD (Steam non-steam-game shortcut, terminal in `~`): ROM loads via the exe-dir candidate, game runs, but every EEPROM/config write hits `fopen(..., "wb")` on a nonexistent `./data/` and fails with a stderr-only "cannot write" — saves never persist, and **every launch also starts with default settings** (F10 changes don't stick), which presents as "draw distance/LOD is low" on levels like Dam where the user had raised `Video.DrawDistance` on another machine. The Windows branch already did CWD-else-exedir; the POSIX branch now mirrors it (`readlink("/proc/self/exe")`) and creates the directory best-effort so first-run writes don't fail. **Verified on WSL:** with exe+`data/` in `/tmp/fb2` and CWD=`/`, config/eeprom/ROM all resolve into `/tmp/fb2/data/` and a 25 s run completes clean; the pre-fix binary in the same layout fails to find the ROM at all. **Caveat:** if the user's Deck launch *was* from the bundle dir (CWD had `data/`), this fix does not explain their save failure — need the save-flow detail (does the memory-card screen appear? slot confirm? "saved" message?) before closing the report. Side note found while testing: a `data/` without the generated sidecars (`pccg-*/`, `pcmodels-*/` from `tools_pc/d69_emit.py`/`d43_emit.py`) starts fine but faults in `load_bg_file` (`bg.c:865`) on level load — the reserve functions warn "stage loads will fault" and return 0; expected degraded mode, not a new bug. | FIXED (path half) — save-flow detail still owed from the user. |
|
||||
| D257 | **New `Game.AllUnlocked` option (F10; default flipped OFF for v0.2.0 — see D259): skip N64 progression entirely — all 21 solo levels selectable at every difficulty, 007 mode available, and the cheat menu fully populated. Port-layer only; no game-code edits.** Three mechanisms, all in `port/`: (1) `main.c` seeds the two live `LEFTOVERDEBUG` RAM flags after `configLoad()` — `debug_enable_all_levels_flag` (all levels) and `debug_007_unlock_flag` (007 mode; both in `src/game/debugmenu_handler.c`, same class as the existing `GE_UNLOCK_ALL` getenv hook in the getter); (2) the eep shim (`libultra.c geEepromPatchAllCheats`) patches the save block at read time — special-casing `fileValidateSaves`'s exact pattern (block 4, 5×96 bytes) — setting all 20 progression-gated cheat-unlock bits per slot (`unlocked_cheats_1/2/3`; the cheat menu row on mode-select only renders when ≥1 is set, `front.c interface_menu06_modesel`) **and** filling every *empty* stage/difficulty completion-time field with the max time 0x3FF (existing player times preserved); both are then CRC'd per slot with the game's own `fileGenerateCRC` or `fileValidateSaves` would wipe the slots. (3) `optionsoverlay.c` row so users can toggle OFF for faithful progression. **Why the completion-time half is required (found by user repro):** when ANY single-player cheat is active, `g_AppendCheatSinglePlayer` makes mission-select (`get_highest_unlocked_difficulty_for_level`, front.c) accept only levels with status `STAGESTATUS_COMPLETED (3)`, not merely `UNLOCKED (1)` — AllUnlocked's debug-flag path returns 1, so enabling a cheat locked every level ("could only do Silo"). Reading the save as a fully-completed campaign satisfies both branches. Time bit-math mirrors `fileGet/SetSaveStageDifficultyTime` verbatim (10-bit fields, difficulties 0–2; 007 times are virtual). **Pitfall hit during implementation:** the first mirror of `save_data` used `times[80]` instead of `(SP_LEVEL_MAX-1)*4 = 76`, making `sizeof` 100≠96 so the block-read size check silently never matched — now enforced by a compile-time `ge_save_slot_size_check`. **Semantics:** unlocked cheats/completions persist to `ge007.eep` on any subsequent save write (self-consistent; mirrors N64 where completing a level permanently unlocks its cheat). No *active* cheats are enabled (invincibility/all-guns stay OFF); weapons remain per-mission pickups. **Verified:** fresh eep — all 5 slots validate `stored==calc` under `GE_SAVELOG=1`; user playtest: all levels selectable at each difficulty, 007 mode present, cheat menu populated, cheats toggle and work (time-modifying cheats untested by user), mission select intact with all cheats ON. | FIXED — verified by user playtest (PR #79 merged); default flipped OFF for v0.2.0 (D259). |
|
||||
| D259 | **`Game.AllUnlocked` (D257) breaks audio on a fresh save: with it ON, music and SFX load at volume 0 (silence). v0.2.0 ships with `AllUnlocked` default OFF.** User-observed during v0.2.0 artifact testing ("all unlocked seems to break sound"). **Mechanism:** a fresh `ge007.eep` is zero-filled, so every save slot is all-zero. On N64 such a slot fails `fileValidateSaves`' CRC check and is rewritten as `BLANKSAVEDATA` (`src/bondconstants.h:2148`), whose `music_vol`/`sfx_vol` are `0xFF` (max). With AllUnlocked ON, `geEepromPatchAllCheats` (`port/src/libultra.c`) patches the zeroed slots (cheat bits + completion times) and recomputes a **valid** per-slot CRC, so `fileValidateSaves` accepts them as-is and never calls `fileResetSave`. The volume bytes therefore stay 0, and `fileLoadSettingsForFolder` (`src/game/file2.c:1289-1290`) loads them verbatim: `set_mTrack2Vol((save->music_vol << 7) | (save->music_vol >> 1))` = 0 → silence. With AllUnlocked OFF the zeroed slots fail CRC and reset to `BLANKSAVEDATA`, which is why sound works in the default configuration. **Disposition for v0.2.0:** flip `portAllUnlocked` default to 0 (`port/src/video.c`; `SkipIntro` was already 0) — faithful N64 progression out of the box, F10 *All unlocked* still enables the goodie. The quirk then only affects users who deliberately toggle it ON with a fresh save (recoverable: raising the in-game volume writes a nonzero slot, after which the patch leaves volumes alone). **Deferred fix candidate (port-only, ~6 lines):** in `geEepromPatchAllCheats`, detect all-zero slots and seed `music_vol`/`sfx_vol = 0xFF` before the CRC recompute, emulating what `fileResetSave` would have done; real (nonzero) saves incl. a user-muted one stay untouched. | **FIXED in v0.2.1** — the deferred fix was applied: `geEepromPatchAllCheats` now detects all-zero slots and seeds `music_vol`/`sfx_vol = 0xFF` before the CRC recompute, emulating `fileResetSave`; real saves (incl. a deliberately muted one) stay untouched. **By-ear confirmed by the user** (v0.2.1 test pass: fresh save, AllUnlocked ON — audio plays correctly). |
|
||||
|
||||
+35
-37
@@ -147,7 +147,33 @@ void sub_GAME_7F09B820(void)
|
||||
*/
|
||||
void sub_GAME_7F09BAC4(s32 find, s32 replacement) {
|
||||
PropRecord* var_s1;
|
||||
#ifdef PORT
|
||||
/* D255 (M-140): the original decomp read this record's Model* through
|
||||
* `((Model*)((ChrRecord*)prop->chr)->chrflags)` -- a union pun onto
|
||||
* ChrRecord.chrflags, which is typed `CHRFLAG` (a 4-byte enum). On
|
||||
* 32-bit N64 that's bit-exact: both the enum and a pointer are 4
|
||||
* bytes, so casting the enum's VALUE to Model* reproduces the real
|
||||
* bytes stored there. On 64-bit PC, Model* is 8 bytes but the enum
|
||||
* read is still only 4 -- the cast reads just the pointer's low 32
|
||||
* bits as an int and zero-extends it back out to a 64-bit address,
|
||||
* discarding the real high bits entirely. The result is either a wild
|
||||
* pointer or (as observed live on a Steam Deck, 2026-09-15: FAULT ADDR
|
||||
* 0x10, PC in this function's `var_v1->RootNode` dereference) exactly
|
||||
* NULL, whenever the true pointer's low 32 bits happen to be zero.
|
||||
* Root-caused by comparing this truncated read against
|
||||
* `var_s1->obj->model` (ObjectRecord.model, correctly typed as
|
||||
* `Model*`) on the same live capture: the ObjectRecord-typed read was
|
||||
* always a valid, non-NULL pointer -- confirming the record's model
|
||||
* was never actually missing, only misread. Fix: read the pointer
|
||||
* through its real 64-bit-wide field (`ObjectRecord.model`, the same
|
||||
* union memory, just correctly typed) instead of through the
|
||||
* mistyped `ChrRecord.chrflags` alias. Semantics-preserving on N64
|
||||
* (identical bytes, identical result) and the D3x pointer-width ABI
|
||||
* class documented in AGENTS.md / docs/dev/findings.md. */
|
||||
ObjectRecord* var_v0;
|
||||
#else
|
||||
ChrRecord* var_v0;
|
||||
#endif
|
||||
Model* temp_a0;
|
||||
s32* temp_v0_2;
|
||||
ModelNode* var_a1;
|
||||
@@ -157,50 +183,22 @@ void sub_GAME_7F09BAC4(s32 find, s32 replacement) {
|
||||
var_s1 = chrpropGetActiveTail();
|
||||
while (var_s1 != NULL) {
|
||||
if (var_s1->type == 1) {
|
||||
var_v0 = var_s1->chr;
|
||||
#ifdef PORT
|
||||
/* TEMP D255 (M-140): live-repro diagnostic for the Facility
|
||||
* terminal SIGSEGV. An active PROP_TYPE_OBJ record's model
|
||||
* pointer (aliased here through ChrRecord.chrflags at the same
|
||||
* byte offset as ObjectRecord.model) has been observed NULL --
|
||||
* ObjectRecord.model is never explicitly nulled anywhere in
|
||||
* game code, so this is either a destroy-path ordering race or
|
||||
* an activation race on a record whose model hasn't been set
|
||||
* yet. Env-gated: GE_D255 skips the dereference and logs the
|
||||
* record's identity instead of crashing, so a live repro
|
||||
* session can keep running and a full trace can be captured.
|
||||
* Default (unset) behaviour is bit-identical to before this
|
||||
* probe -- do not remove until the mechanism is understood and
|
||||
* a real fix lands. */
|
||||
{
|
||||
static int ge_d255 = -1;
|
||||
if (ge_d255 < 0) ge_d255 = getenv("GE_D255") != NULL;
|
||||
if (ge_d255) {
|
||||
ObjectRecord* d255obj = var_s1->obj;
|
||||
if (d255obj->model == NULL) {
|
||||
static int d255budget = 64;
|
||||
if (d255budget > 0) {
|
||||
d255budget--;
|
||||
osSyncPrintf(
|
||||
"D255 vtxstore_fix_refs: NULL model on active OBJ record"
|
||||
" prop=%p obj->obj=%d obj->type=%d obj->state=0x%02x"
|
||||
" prop->flags=0x%x prop->rooms=%d,%d\n",
|
||||
(void*)var_s1, (int)d255obj->obj, (int)d255obj->type,
|
||||
(unsigned)d255obj->state, (unsigned)var_s1->flags,
|
||||
(int)var_s1->rooms[0], (int)var_s1->rooms[1]);
|
||||
}
|
||||
var_s1 = var_s1->prev;
|
||||
continue;
|
||||
}
|
||||
}
|
||||
}
|
||||
#endif
|
||||
var_v0 = var_s1->obj;
|
||||
var_v1 = ((Model*)var_v0->model)->obj;
|
||||
#else
|
||||
var_v0 = var_s1->chr;
|
||||
var_v1 = ((Model*)var_v0->chrflags)->obj;
|
||||
#endif
|
||||
var_a1 = var_v1->RootNode;
|
||||
while (var_a1 != NULL) {
|
||||
val = var_a1->Opcode & 0xFF;
|
||||
if (val == 0x18) {
|
||||
#ifdef PORT
|
||||
temp_v0_2 = modelGetNodeRwData((Model*)var_v0->model, var_a1);
|
||||
#else
|
||||
temp_v0_2 = modelGetNodeRwData((Model*)var_v0->chrflags, var_a1);
|
||||
#endif
|
||||
if (find == *temp_v0_2) {
|
||||
*temp_v0_2 = replacement;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user