diff --git a/docs/dev/findings-index.csv b/docs/dev/findings-index.csv index 271e5b04..88fe7062 100644 --- a/docs/dev/findings-index.csv +++ b/docs/dev/findings-index.csv @@ -185,7 +185,7 @@ D303,OPEN,"Some muzzle flashes (M16 named specifically, ""and some others"") dra D304,FIXED,"Mouse sensitivity menu overhaul (user request, 2026-09-18):","FIXED — see full entry for both the range/parity pass and the same-day follow-up (collapsed to a single Input.MouseSensitivity menu row + fixed overlayUpdateScroll()'s unconditional bottom-anchor, th…" D305,FIXED,"Steam Deck SIGSEGV on the v0.3.0 pre-release build (Cavern, early in the level, Game.AllUnlocked previously enabled on the session/save) —",FIXED (defensive guard) — D306,CLOSED,"Dam intro/truck wheels render with odd transparent-looking areas, possibly z-fighting (user QA, 2026-09-18).","CLOSED — faithful, confirmed on 1964 (M-201):" -D307,OPEN,"In-level security camera entities can be flipped facing the wrong direction (backwards) on some levels, e.g.","OPEN — CONFIRMED PORT BUG (2026-09-26, user vs 1964:" +D307,FIXED,"In-level security camera entities can be flipped facing the wrong direction (backwards) on some levels, e.g.","FIXED (M-201, 2026-09-26, user-verified):" D308,CLOSED,"Occasional z-fighting confirmed on some levels' geometry, general/broader than the D306 truck-wheel instance (user QA, 2026-09-18).","CLOSED — faithful (2026-09-26, user:" D309,FIXED,Complete non-self-recovering freeze (Windows) returning from Caverns to the intro screen —,FIXED via D310 — user-playtest-confirmed 2026-09-18 (Caverns→intro clean; the spin record itself observed resuming correctly in the post-fix probe log). D310,FIXED,"RULE-2-SIGNOFF: chraiitemsize() mis-sizes 1-byte AI_PRINT records (NUL-scans forward), corrupting AI execution for any list containing an active PRINT —",FIXED — user-playtest-confirmed 2026-09-18 (v2: diff --git a/docs/dev/findings.md b/docs/dev/findings.md index f8f2eef0..35d5819a 100644 --- a/docs/dev/findings.md +++ b/docs/dev/findings.md @@ -593,7 +593,7 @@ covers D24–D69; the log continues in §H (D32 procedure, D70–D121). | D304 | **Mouse sensitivity menu overhaul (user request, 2026-09-18): aim speed/turn speed sliders weren't "1:1", had too little range, and (follow-up) risked decoupling into a bad state; F10 menu also had a click-jump bug.** — full `## D304` entry at file tail | FIXED — see full entry for both the range/parity pass and the same-day follow-up (collapsed to a single `Input.MouseSensitivity` menu row + fixed `overlayUpdateScroll()`'s unconditional bottom-anchor, the actual cause of the reported "menu jumps to the setting below on click"). Build-verified (Windows) both passes; headless overlay smoke test clean after each. **Live F10 playtest done (2026-09-18): user confirmed the collapsed single-sensitivity menu works, the click-jump is gone, and the D251 bottom-row duplication no longer occurs.** | | D305 | **Steam Deck SIGSEGV on the v0.3.0 pre-release build (Cavern, early in the level, `Game.AllUnlocked` previously enabled on the session/save) — same exact FAULT ADDR 0x62 / `sndHandleEvent` signature as D285, reproduced AFTER D285's own lock fix was already live-verified crash-free on real hardware.** — full `## D305` entry at file tail | FIXED (defensive guard) — `sndHandleEvent`'s preempt-scan (`src/snd.c`) dereferenced its list-walk pointer before ever checking it for NULL (a do-while always runs its body once); D285's `osSetIntMask` lock only prevents another thread from emptying the list mid-scan, not a scan that starts with an already-empty list, so the crash could still happen even with that fix in place. Added the missing `iterState != NULL` guard before the loop, matching D255's established skip-instead-of-dereference precedent. Root desync mechanism (why pool-exhaustion accounting and the list can disagree) still not identified; `Game.AllUnlocked` correlation not proven, only reported. **Live-verified on the Deck (2026-09-18):** user ran two test sessions on Cavern with the updated build, plus toggled `Game.AllUnlocked` off then back on and relaunched — no crash in any of them. | | D306 | **Dam intro/truck wheels render with odd transparent-looking areas, possibly z-fighting (user QA, 2026-09-18).** — full `## D306` entry at file tail | CLOSED — faithful, confirmed on 1964 (M-201): all PC-specific leads refuted (FOV, depth precision, depth state, near plane, wheel transforms); it is the authored truck geometry seen from intro camera 5 under the truck. Absorbs D291. Earlier GE_ZF "real fighting" was a probe-quantisation artefact. | -| D307 | **In-level security camera entities can be flipped facing the wrong direction (backwards) on some levels, e.g. Bunker (user QA, 2026-09-18) — NOT the player's view camera.** — full `## D307` entry at file tail | OPEN — CONFIRMED PORT BUG (2026-09-26, user vs 1964: ALL Bunker 1 security cameras face backwards on PC, correct on N64). Earlier: OPEN (investigated 2026-09-21) — no port-side bug found: orientation path traced, every input verified N64-faithful vs ROM (`PcctvZ` switch data, room-scaled boundpads, config words), all math byte-identical `src/game`. Blocked on user repro detail; `GE_D307=1` diagnostic in-tree. | +| D307 | **In-level security camera entities can be flipped facing the wrong direction (backwards) on some levels, e.g. Bunker (user QA, 2026-09-18) — NOT the player's view camera.** — full `## D307` entry at file tail | FIXED (M-201, 2026-09-26, user-verified): renamed duplicate member `CCTVRecord.pad`→`lookpad` made setupCctv read the MOUNT pad as the look-at pad; prop.c reads lookpad on PC. Earlier: OPEN — CONFIRMED PORT BUG (2026-09-26, user vs 1964: ALL Bunker 1 security cameras face backwards on PC, correct on N64). Earlier: OPEN (investigated 2026-09-21) — no port-side bug found: orientation path traced, every input verified N64-faithful vs ROM (`PcctvZ` switch data, room-scaled boundpads, config words), all math byte-identical `src/game`. Blocked on user repro detail; `GE_D307=1` diagnostic in-tree. | | D308 | **Occasional z-fighting confirmed on some levels' geometry, general/broader than the D306 truck-wheel instance (user QA, 2026-09-18).** — full `## D308` entry at file tail | CLOSED — faithful (2026-09-26, user: Statue ground z-fighting reproduces on 1964 GEPD bundle). Earlier: OPEN (user, 2026-09-26: possibly faithful; validate Statue ground z-fighting on 1964 vs port). Earlier: OPEN — investigated: PC depth precision == N64 (24-bit both sides; RSP writes full 16.16), so level-authored coincident geometry is the prime suspect; GE_ZF flicker probe in-tree. | | D309 | **Complete non-self-recovering freeze (Windows) returning from Caverns to the intro screen — boss thread spins forever in `ai()` on `m_RunToBondPersistent` via a phantom `GotoNext(lblZero)` created by `chraiitemsize`'s `AI_PRINT` NUL-scan mis-sizing a stringless PRINT record (user report + live GDB diagnosis, 2026-09-18).** — full `## D309` entry at file tail | **FIXED via D310 — user-playtest-confirmed 2026-09-18** (Caverns→intro clean; the spin record itself observed resuming correctly in the post-fix probe log). The teardown-ordering open question (why PC presents the unreachable-Bond state) remains documented but is now harmless. | | D310 | **RULE-2-SIGNOFF: `chraiitemsize()` mis-sizes 1-byte `AI_PRINT` records (NUL-scans forward), corrupting AI execution for any list containing an active PRINT — fixes D309's freeze mechanism and the Caverns standing-still guards (`Stop`+`PRINT("wait")` mis-lands on EndList, re-executed every tick; user report 2026-09-18).** — full `## D310` entry at file tail | **FIXED — user-playtest-confirmed 2026-09-18** (v2: global-list-only sizing via `d310ItemSize`; guards resume, Trevelyan pathing intact, Caverns→intro clean; probe log verified both list classes). PD precedent: PRINT removed, slot 0xAD reassigned. | @@ -12389,6 +12389,17 @@ do { **Status:** OPEN (investigated — no port-side bug found; awaiting repro detail). +**M-201 (2026-09-26, v0.4.0): ROOT CAUSE FOUND + FIXED — a renamed duplicate struct member re-pointed `setupCctv`'s look-at pad to the mount pad. User-verified.** + +- **Confirmed port bug first:** user side-by-side with 1964: every Bunker 1 camera faces backwards on PC, correct on N64. +- **Measured (TEMP render dump, since removed):** the CCTV model is fine (group `mtx 0` = base, group at the switch origin (0,70.4,32.0) = `mtx 1` = lens). The lens matrix's forward axis pointed back into the mount: camera 48's base maps model +Z to world +X, its lens matrix mapped +Z to about (-0.80,-0.60,0). +- **Cause:** `CCTVRecord` inherits `ObjectRecord` (s16 `pad` @0x08 = mount pad) and declares its own s32 `pad` @0x80 = the **look-at** pad. IDO accepts the duplicate and `arg1->pad` in `setupCctv` means the CCTV's own look-at pad. GCC rejects duplicate members, so the PC struct renamed it `lookpad`, and a comment asserted game code "never" means this member. So on PC `arg1->pad` silently became the mount pad: the look vector was just the lens's offset from its own mount (`target` ≈ (9.4, 7.0, 0.9)) and `matrix_4x4_set_basis_and_position` (Z = -look) aimed every lens back at its wall. The load dump showed it: `pad=10018 bound=1` (mount) while the record's look-at pad is 0x58 = 88. +- **Cross-check:** Perfect Dark's same function (`pd_port/src/game/setup.c:795`, `setupCreateCctv`) uses `cctv->lookatpadnum`, the CCTV's own field. +- **Fix (`src/game/prop.c`, `#ifdef PORT`, A1 class: read the correctly named field):** `CCTV_LOOKPAD` = `arg1->lookpad` on PC, `arg1->pad` on N64, for the 4 look-pad reads in `setupCctv`. `bondtypes.h`'s misleading comment corrected. After the fix the look vectors run from the floor look-at pads (y ≈ 185) to the lenses (~700 units, ~240 up), and the lens forward axis points at the watched spot. +- **Verified:** user playtest ("that fixed it"); golden gate 3/3. Only `CCTVRecord` had this rename (grep for the duplicate-member note). + +**Status: FIXED (M-201, user-verified 2026-09-26).** + ## D308 — Occasional z-fighting confirmed on some levels' geometry, general/broader than the D306 truck-wheel instance (user QA, 2026-09-18). **Reported/confirmed by the user, not investigated, explicitly not for v0.3.0.** User: "Confirmed occasional z-fighting in some levels due to the level geometry and current state of code" — framed as a general, known-class issue (overlapping/coincident geometry at the current renderer's precision) rather than a single specific bug site. D306 (Dam truck wheels) may be one visible instance of this broader class, not confirmed as the same mechanism. diff --git a/docs/porting-notes.md b/docs/porting-notes.md index e7a4be54..f15306d2 100644 --- a/docs/porting-notes.md +++ b/docs/porting-notes.md @@ -269,6 +269,10 @@ framing — it never looked like game logic to begin with. Diagnostic tell: a hardware watchpoint on the "poisoned" field sees *no writes at all*. +### A-dup. Renaming an IDO duplicate member re-points every read of it (D307) + +IDO accepts a struct that redeclares an inherited member name (`inherits ObjectRecord;` then its own `pad`); GCC rejects it, so the port renames one. **The rename changes which field existing code reads**: on N64 the later declaration shadows the inherited one, so `rec->pad` meant the derived record's own field. After renaming it (`lookpad`), unchanged game code silently binds to the inherited field instead. D307: `setupCctv` read the mount pad as the look-at pad and every security camera faced its wall. When a port rename resolves a duplicate, grep every `->name` on that struct type and decide per site which field N64 meant (PD's named-field sources are the tie-breaker). + ## B. 16-byte PC `Gfx` / `Vtx` vs 8-byte N64 Any buffer reservation, `memcpy` size, slot stride, or pool budget diff --git a/src/bondtypes.h b/src/bondtypes.h index 1ab71800..c8283e37 100644 --- a/src/bondtypes.h +++ b/src/bondtypes.h @@ -3189,10 +3189,11 @@ typedef union * compiler (GCC) resolves by inlining ObjectRecord's members. ObjectRecord * already has a `pad` member (s16 @0x08, the pad index set by * New_CCTVRecord), so this `pad` (s32 @0x80) is a duplicate name that GCC - * rejects as a hard error (IDO tolerates it). The game code's `->pad` - * (e.g. setupCctv) refers to the INHERITED pad index @0x08, never to this - * member, so renaming it keeps the layout byte-identical. See - * docs/dev/findings.md. */ + * rejects as a hard error (IDO tolerates it). Renaming keeps the layout + * byte-identical, BUT on N64 this member shadows the inherited one: + * setupCctv's `arg1->pad` is THIS look-at pad (@0x80), not the mount + * pad (@0x08). Code that means the look-at pad must read `lookpad` on + * PC (D307, M-201: cameras faced backwards). See docs/dev/findings.md. */ s32 lookpad; // lookpad #else s32 pad; // lookpad diff --git a/src/game/prop.c b/src/game/prop.c index 84c38df1..551bab45 100644 --- a/src/game/prop.c +++ b/src/game/prop.c @@ -646,17 +646,30 @@ void setupCctv(s32 arg0, CCTVRecord *arg1, s32 cmdindex) domakedefaultobj(arg0, (struct ObjectRecord*)arg1, cmdindex); - if (arg1->pad >= 0) +#ifdef PORT + /* D307 (M-201): on N64, CCTVRecord's own s32 `pad` @0x80 (the LOOK-AT + * pad) shadows the inherited ObjectRecord s16 `pad` @0x08 (the mount pad), + * so `arg1->pad` here is the look-at pad. The PC struct renames the + * duplicate to `lookpad` (GCC rejects duplicate members), which silently + * re-pointed these reads at the MOUNT pad: the look vector became the + * lens's own offset from its mount and every camera faced backwards. + * Read the correctly named field (A1 class: layout/naming only; PD's + * setupCreateCctv uses cctv->lookatpadnum for the same step). */ +#define CCTV_LOOKPAD (arg1->lookpad) +#else +#define CCTV_LOOKPAD (arg1->pad) +#endif + if (CCTV_LOOKPAD >= 0) { temp_a2 = (struct coord3d*)arg1->model->obj->Switches[0]->Data; - if (isNotBoundPad(arg1->pad)) + if (isNotBoundPad(CCTV_LOOKPAD)) { - sp50 = &g_CurrentSetup.pads[arg1->pad]; + sp50 = &g_CurrentSetup.pads[CCTV_LOOKPAD]; } else { - sp50 = (struct PadRecord *)&g_CurrentSetup.boundpads[getBoundPadNum(arg1->pad)]; + sp50 = (struct PadRecord *)&g_CurrentSetup.boundpads[getBoundPadNum(CCTV_LOOKPAD)]; } sp44.f[0] = temp_a2->f[0];