mirror of
https://github.com/jkdansereau/goldeneye-pc-port.git
synced 2026-10-23 15:52:42 +02:00
fix(D299): Game.SkipIntro crash selecting most save slots
fileValidateSaves() -- the only place saves[] is populated from EEPROM -- is called exclusively from the legal screen's init, which Game.SkipIntro's boot-to-file-select shortcut skips entirely. saves[] was left all-zero, so fileGetSaveForFoldernum() returned NULL for every folder except whichever one a zeroed save_data happened to decode to, crashing file-select's unguarded fileGetIsCheatUnlocked() for the rest. Call fileValidateSaves() from the shortcut too (port-layer completeness fix, zero game-logic change). Live-verified by user: all 4 save slots now selectable with SkipIntro=1, intro still correctly skipped.
This commit is contained in:
1 parent
85a165d74b
commit
7b405abc63
4 files changed
+51
-1
No files matched your search
@@ -176,3 +176,4 @@ D295,FIXED,"Campaign-halting crash at the end of Control Center on 00 Agent diff
|
|||||||
D296,OPEN,"Control Center, 00 Agent:","OPEN — single live report, un-triaged; observed only under a gdb-attached debug-harness launch (tools_pc/debug.ps1), not the packaged release artifact —"
|
D296,OPEN,"Control Center, 00 Agent:","OPEN — single live report, un-triaged; observed only under a gdb-attached debug-harness launch (tools_pc/debug.ps1), not the packaged release artifact —"
|
||||||
D297,FIXED,v0.3.0 silently wipes ALL pre-fix player saves on first launch (D284 side effect; found during D295/M-148).,FIXED (M-150) — migration shim landed:
|
D297,FIXED,v0.3.0 silently wipes ALL pre-fix player saves on first launch (D284 side effect; found during D295/M-148).,FIXED (M-150) — migration shim landed:
|
||||||
D298,?,Full HUD (not just the aim-mode crosshair) missing in a v0.3.0 dev build —,"s — regression reported 2026-09-16 during v0.3.0 work (build ce8427cf); user says ""don't fix, just for context""; not yet investigated —"
|
D298,?,Full HUD (not just the aim-mode crosshair) missing in a v0.3.0 dev build —,"s — regression reported 2026-09-16 during v0.3.0 work (build ce8427cf); user says ""don't fix, just for context""; not yet investigated —"
|
||||||
|
D299,FIXED,Game.SkipIntro crashed selecting any save slot except one —,"FIXED — fileValidateSaves() (only called from the skipped legal screen) never ran under SkipIntro, leaving saves[] all-zero; added the missing call to the SkipIntro shortcut (src/game/lv.c, #ifdef PO…"
|
||||||
|
@@ -584,6 +584,7 @@ covers D24–D69; the log continues in §H (D32 procedure, D70–D121).
|
|||||||
| D296 | **Control Center, 00 Agent: noticeable stutter, user's own read is "maybe from the drone guns" (M-146, same D295 investigation session).** — full `## D296` entry at file tail | OPEN — single live report, un-triaged; observed only under a gdb-attached debug-harness launch (`tools_pc/debug.ps1`), not the packaged release artifact — user flagged that harness overhead has caused false stutter before. Needs a plain-exe / real-release repro check before any perf tracing.… |
|
| D296 | **Control Center, 00 Agent: noticeable stutter, user's own read is "maybe from the drone guns" (M-146, same D295 investigation session).** — full `## D296` entry at file tail | OPEN — single live report, un-triaged; observed only under a gdb-attached debug-harness launch (`tools_pc/debug.ps1`), not the packaged release artifact — user flagged that harness overhead has caused false stutter before. Needs a plain-exe / real-release repro check before any perf tracing.… |
|
||||||
| D297 | **v0.3.0 silently wipes ALL pre-fix player saves on first launch (D284 side effect; found during D295/M-148).** — full `## D297` entry at file tail | FIXED (M-150) — migration shim landed: `port/src/legacycrc.c` (isolated, deliberately-wrong pre-D284 PRNG, reviewer-verified bit-exact against `0de99b4d^`) + eeprom-shim read-path hook in `libultra.c` (block 4 only; persists pristine+migrated bytes before the read-time-only AllUnlocked patch). Verified: user's #87 artifact → `D297-migrated 5`, all slots OK, idempotent on relaunch; native post-fix save untouched; corrupt slot left to the game's own wipe path; fresh eep untouched; 21/21 sweep PASS. Block-0 header needed no migration (factory seal, verified). |
|
| D297 | **v0.3.0 silently wipes ALL pre-fix player saves on first launch (D284 side effect; found during D295/M-148).** — full `## D297` entry at file tail | FIXED (M-150) — migration shim landed: `port/src/legacycrc.c` (isolated, deliberately-wrong pre-D284 PRNG, reviewer-verified bit-exact against `0de99b4d^`) + eeprom-shim read-path hook in `libultra.c` (block 4 only; persists pristine+migrated bytes before the read-time-only AllUnlocked patch). Verified: user's #87 artifact → `D297-migrated 5`, all slots OK, idempotent on relaunch; native post-fix save untouched; corrupt slot left to the game's own wipe path; fresh eep untouched; 21/21 sweep PASS. Block-0 header needed no migration (factory seal, verified). |
|
||||||
| D298 | **Full HUD (not just the aim-mode crosshair) missing in a v0.3.0 dev build — user suspects a debug-launch artifact and asked NOT to raise it until seen in a regular release-targeted build; parked** — full `## D298` entry at file tail |s — regression reported 2026-09-16 during v0.3.0 work (build ce8427cf); user says "don't fix, just for context"; not yet investigated** — full `## D298` entry at file tail |
|
| D298 | **Full HUD (not just the aim-mode crosshair) missing in a v0.3.0 dev build — user suspects a debug-launch artifact and asked NOT to raise it until seen in a regular release-targeted build; parked** — full `## D298` entry at file tail |s — regression reported 2026-09-16 during v0.3.0 work (build ce8427cf); user says "don't fix, just for context"; not yet investigated** — full `## D298` entry at file tail |
|
||||||
|
| D299 | **Game.SkipIntro crashed selecting any save slot except one — reproduced on a fresh save and on a clean Release rebuild, ruling out stale save data (found + fixed 2026-09-17).** — full `## D299` entry at file tail | FIXED — `fileValidateSaves()` (only called from the skipped legal screen) never ran under `SkipIntro`, leaving `saves[]` all-zero; added the missing call to the `SkipIntro` shortcut (`src/game/lv.c`, `#ifdef PORT`, zero game-logic change). Build-verified both `RelWithDebInfo`/`Release`; live user re-test confirms all 4 slots now selectable with `SkipIntro=1`. Not yet run through a full 21-level sweep. |
|
||||||
|
|
||||||
Phase 2 replaced the Phase-1 demo loop with the real `mainproc()` on real OS
|
Phase 2 replaced the Phase-1 demo loop with the real `mainproc()` on real OS
|
||||||
threads, compiled GE's real `src/sched.c`, and brought in PD's fast3d software
|
threads, compiled GE's real `src/sched.c`, and brought in PD's fast3d software
|
||||||
@@ -11321,3 +11322,17 @@ This is a **perfect, smooth white-hot → yellow → orange → deep-red → fad
|
|||||||
**Next (only if it reproduces in a release-targeted build):** first re-check the launch path (debug vs release config, `data/ge007.ini` state, any env vars left set — e.g. a stray debug render flag), then ask for the last build where the HUD was confirmed working → `git bisect` with a 30-second in-level check; static-trace the HUD draw conditions (crosshair show/hide flag at `bondview.h:558` is the cheapest single suspect).
|
**Next (only if it reproduces in a release-targeted build):** first re-check the launch path (debug vs release config, `data/ge007.ini` state, any env vars left set — e.g. a stray debug render flag), then ask for the last build where the HUD was confirmed working → `git bisect` with a 30-second in-level check; static-trace the HUD draw conditions (crosshair show/hide flag at `bondview.h:558` is the cheapest single suspect).
|
||||||
|
|
||||||
**Status:** **OPEN — parked per user instruction: suspected debug-launch artifact; do not raise until seen in a regular release-targeted build. Not yet investigated.**
|
**Status:** **OPEN — parked per user instruction: suspected debug-launch artifact; do not raise until seen in a regular release-targeted build. Not yet investigated.**
|
||||||
|
|
||||||
|
## D299 — Game.SkipIntro crashed selecting any save slot except one (found + fixed this session, 2026-09-17)
|
||||||
|
|
||||||
|
**Symptom (user, live repro):** with `Game.SkipIntro=1` (D216), booting straight to file-select and selecting any of the 4 file slots except one crashed the game (`EXCEPTION: 0xc0000005`, fault addr `0x10`). Reproduced on both a clean `CMAKE_BUILD_TYPE=Release` rebuild and the normal `RelWithDebInfo` dev build, and with a completely fresh (no `data/ge007.eep`) save — ruling out stale/incompatible save data. With `SkipIntro=0` (normal intro/legal-screen boot), all 4 slots worked with no crash.
|
||||||
|
|
||||||
|
**Root cause:** `fileValidateSaves()` (`file2.c:499`) — the only place `saves[]` is populated from EEPROM (`joyGamePakLongRead`, CRC-validated/repaired) — is called exclusively from `init_menu00_legalscreen()` (`front.c:1450`), which only runs when the front-end state machine passes through `MENU_LEGAL_SCREEN`. `Game.SkipIntro`'s shortcut (`src/game/lv.c:417-430`, `#ifdef PORT`) jumps straight to `MENU_FILE_SELECT`, skipping the legal screen entirely, so `fileValidateSaves()` never ran — `saves[SAVESLOTMAX]` (`file2.c:29`, a plain zero-initialized global) stayed all-zero instead of loading real save data. `fileGetSaveForFoldernum()` (`file2.c:434`) then legitimately returned `NULL` for every folder except whichever one a zeroed `save_data` happens to decode its folder field as. File-select's `fileGetIsCheatUnlocked()` (`file2.c:397`) dereferences its `save` argument with no `NULL` guard (unlike sibling call sites, e.g. `file2.c:1190`, which do check) — crashing for every other folder.
|
||||||
|
|
||||||
|
**Fix (`src/game/lv.c`, `#ifdef PORT` block, D216's own shortcut):** call `fileValidateSaves()` from the `SkipIntro` shortcut, so `saves[]` gets populated exactly as the skipped legal screen would have done. Zero game-logic change — reuses the existing decomp-verbatim function; rule-2-clean (port-layer completeness fix for a port-only feature, not a `src/game` behavior change). Left `fileGetIsCheatUnlocked`'s missing `NULL` guard alone since the actual bug is the port shortcut not fully replicating the skipped screen's side effects, not a missing defensive check in game code.
|
||||||
|
|
||||||
|
**Verified:** build-clean (`ninja`, both `RelWithDebInfo` and `Release`); live user re-test with `SkipIntro=1` restored — intro correctly skipped, all 4 file slots selectable with no crash.
|
||||||
|
|
||||||
|
**Generalisable lesson (added to `porting-notes.md`):** a port-only "skip to state X" shortcut must replicate every side effect the skipped screens perform, not just the terminal menu-state flags — `SkipIntro`'s original comment claimed to fully reuse "the game's own post-intro route," but missed the legal screen's one-time `fileValidateSaves()` call.
|
||||||
|
|
||||||
|
**Status:** **FIXED** — verified live this session. Not yet run through a full 21-level sweep (docs-only + one `#ifdef PORT` line touching front-end boot, not level code) or committed.
|
||||||
@@ -1101,6 +1101,26 @@ output is stored on disk — not just the in-memory streams. A "port-only" fix t
|
|||||||
a checksum primitive is a save-file-format break unless old and new outputs are
|
a checksum primitive is a save-file-format break unless old and new outputs are
|
||||||
made mutually acceptable (dual validation / one-time migration).
|
made mutually acceptable (dual validation / one-time migration).
|
||||||
|
|
||||||
|
## D10. A "skip to state X" port shortcut must replicate every side effect of the screens it skips, not just the terminal flags
|
||||||
|
|
||||||
|
D216's `Game.SkipIntro` jumps straight from boot to `MENU_FILE_SELECT`,
|
||||||
|
skipping the legal screen + Nintendo/Rare/GoldenEye logo attract loop, by
|
||||||
|
directly setting the handful of state-machine flags (`is_first_time_on_main_menu`,
|
||||||
|
`prev_keypresses`, `maybe_is_in_menu`, `menu_update`) that the normal intro
|
||||||
|
path would have arrived at. Its original comment claimed this "reuses the
|
||||||
|
game's own post-intro route" — true for the menu *flags*, but the skipped
|
||||||
|
`init_menu00_legalscreen()` also has a one-time side effect with no flag of
|
||||||
|
its own: it calls `fileValidateSaves()`, the only place `saves[]` is ever
|
||||||
|
populated from EEPROM. Skipping it left `saves[]` all-zero, and file-select's
|
||||||
|
unguarded `fileGetIsCheatUnlocked()` crashed selecting any folder except the
|
||||||
|
one a zeroed `save_data` happened to decode to (D299). Generalizable rule:
|
||||||
|
when adding a port shortcut that jumps over N64 screens/states, enumerate
|
||||||
|
every side effect those screens' `init_menuXX_*()`/`update_menuXX_*()`
|
||||||
|
functions perform — not just the menu-transition flags — and replicate the
|
||||||
|
non-flag ones (data loads, one-time validation, subsystem init) explicitly.
|
||||||
|
Grepping the skipped `init_*`/`update_*` functions for calls with no
|
||||||
|
matching state flag is the concrete check.
|
||||||
|
|
||||||
## E. Process / method notes
|
## E. Process / method notes
|
||||||
|
|
||||||
- Investigation loop is: reproduce → env-gated capped probe → root-cause
|
- Investigation loop is: reproduce → env-gated capped probe → root-cause
|
||||||
|
|||||||
+15
-1
@@ -419,9 +419,23 @@ void lvlStageLoad(s32 stage)
|
|||||||
* logo attract loop. Reuses the game's own post-intro route:
|
* logo attract loop. Reuses the game's own post-intro route:
|
||||||
* every intro stage already jumps to MENU_FILE_SELECT once
|
* every intro stage already jumps to MENU_FILE_SELECT once
|
||||||
* is_first_time_on_main_menu is FALSE (front.c). Port config
|
* is_first_time_on_main_menu is FALSE (front.c). Port config
|
||||||
* only; default 0 = unchanged. */
|
* only; default 0 = unchanged.
|
||||||
|
*
|
||||||
|
* D3xx (found this session): the skipped legal screen's own
|
||||||
|
* init (front.c init_menu00_legalscreen) is what calls
|
||||||
|
* fileValidateSaves() -- the only place saves[] is ever
|
||||||
|
* populated from EEPROM. Skipping straight to file-select
|
||||||
|
* left saves[] all-zero, so fileGetSaveForFoldernum()
|
||||||
|
* returned NULL for every folder except whichever one a
|
||||||
|
* zeroed save_data happens to decode to, and file-select's
|
||||||
|
* unguarded fileGetIsCheatUnlocked() (file2.c:397) crashed
|
||||||
|
* dereferencing NULL for the other three. Call it here too
|
||||||
|
* so saves[] matches what the legal screen would have
|
||||||
|
* produced. */
|
||||||
extern s32 portSkipIntro;
|
extern s32 portSkipIntro;
|
||||||
if (portSkipIntro) {
|
if (portSkipIntro) {
|
||||||
|
extern void fileValidateSaves(void);
|
||||||
|
fileValidateSaves();
|
||||||
is_first_time_on_main_menu = FALSE;
|
is_first_time_on_main_menu = FALSE;
|
||||||
prev_keypresses = TRUE;
|
prev_keypresses = TRUE;
|
||||||
maybe_is_in_menu = TRUE;
|
maybe_is_in_menu = TRUE;
|
||||||
|
|||||||
Reference in new issue
Block a user