diff --git a/docs/dev/D356-SETTINGS-REGROUP-PLAN.md b/docs/dev/D356-SETTINGS-REGROUP-PLAN.md new file mode 100644 index 00000000..9b4d779f --- /dev/null +++ b/docs/dev/D356-SETTINGS-REGROUP-PLAN.md @@ -0,0 +1,351 @@ +# D356 — Settings regroup: functional sections, save-file scoping, per-section reset + +Status: **PROPOSAL — awaiting sign-off** (2026-09-27). Supersedes the "BOND FILE" +section introduced in D353. Baselines: Turok PC port options (Nightdive, list +provided by the maintainer), the PD port's `port/src/optionsmenu.c` +(`C:\Users\james\Source\Repos\pd_port`), and plan §6 of +`docs/dev/OPTIONS-MENU-PLAN.md` (the original "align with Turok" note). + +## 1. Goal + +The D353 "BOND FILE" section is the one section named after *where a value is +saved* — an implementation detail — while every other section is functional +(DISPLAY, GRAPHICS, MOUSE/AIM, CONTROLLER, GAME). Its contents are also a +mishmash (aim/look toggles + HUD toggles + audio), and it exposes N64-watch +rows that **collide with or duplicate** the PC-native rows the port added +(watch look-invert vs `Input.MouseInvertY`/`Input.PadLookInvertY`; watch aim +control vs `Input.AimMode`). The baseline ports (Turok, PD) never name a +section after the save and never re-expose legacy overrides. + +D356 rewrites the row table so that: + +1. Every section is functional, using the baseline names: + **INPUT, GAMEPLAY, GRAPHICS, AUDIO, VIDEO**. +2. Save-file scope is communicated the PD way — *in the section title* + ("GAMEPLAY (File 1)"), generalized from the D353 BOND-FILE annotation — + plus a single **Save file: File N [change]** row at the top of the + options screen (PD "Player N" pattern; D354's default resolution applies). + "Bond file" appears nowhere in the UI. +3. Redundant/colliding watch rows are removed from the **menu surface only** + (the established D181/D216/D304 pattern): keys, in-game watch behavior, + and ini hand-editing all remain untouched. No game-code change (rule 2). +4. A **Reset to defaults** action row per section (Turok's per-category + reset; plan §6 Gate E, made concrete) with a **confirmation contract**: + edge-triggered activation (no auto-repeat), two-step arm→confirm, never + repeatable (WATCH-SETTINGS-PLAN §6 gate: "separate confirmation and + persistence policy before wiring a button" — §5.4 below is that policy). +5. A **context-aware file accessor** (front-end target vs active stage + target) with the "none" chooser position retired, so "Save file: File N" + is always a true statement (§5.5). +6. Section resets iterate the section's **declared** rows (including + conditionally hidden ones, §5.6) and reset verification exercises the UI + action itself, headlessly (§5.8). + +## 2. Non-goals + +- No `src/game` changes; the N64 watch menu keeps all its options and its + N64 wording (ground truth). +- No key rebinding (Phase 4) — but the section layout leaves room for a + "KEY BINDINGS" page exactly where Turok/PD have one. +- Master volume stays M3-backlog; the AUDIO section is shaped for it + (Turok's Master/Sound/Music trio). +- N64 build untouched. + +## 3. Exposure filter (menu surface only) + +| Row (key) | Verdict | Reason | +|---|---|---| +| `Bond.Look` ("Look up/down (watch; stacks)") | **Hide** | Duplicates `Input.MouseInvertY` + `Input.PadLookInvertY`; stacking both produces the double-negation trap. | +| `Bond.AimControl` ("Aim control") | **Hide** | Collides with `Input.AimMode` (N64 vs PC aim model) — two rows fighting over the same domain. | +| `Bond.AutoAim` | Keep | Unique gameplay assist; orthogonal to aim style/range. | +| `Bond.LookAhead` | Keep | Unique; no PC counterpart, no conflict. | +| `Bond.Sight`, `Bond.Ammo` | Keep | Unique HUD-visibility options; compose with `Game.HudScale`. | +| `Bond.Music`, `Bond.FX` | Keep | Core of D350–D355. | + +Hidden rows are commented out in `rows[]` with a D356 note (D181/D216 style: +config keys + watch path stay live; ini power users can still set them). + +## 4. New layout (5 pages + save-file row) + +`MAX_PAGES=8`, `MAX_PROWS=15` — all pages below the caps (D353 overflow log +still guards). + +Top row (drawn by `frontoptions.c` above the paged content, not paged): +**`Save file: File 1 [change]`** — cycles files via `watchSettingsChooseFile` +(D354 default resolution: explicit pick > viewed folder > first valid; +auto-init fallback). Hidden in F10 (existing `optionsRowIsBondChooser` +mechanism, generalized). F10 targets the active file, as today. + +| Page | Rows (key · kind) | Title annotation | +|---|---|---| +| **INPUT** | `Input.MouseSensitivity` · `Input.MouseInvertY` · `Input.AimMode` · `Input.AimRange` · `Input.PadLookInvertY` · `Input.PadDeadzone` · `Input.PadTriggerPct` · *Reset to defaults* | | +| **GAMEPLAY** (File N) | `Bond.AutoAim` · `Bond.LookAhead` · `Bond.Sight` · `Bond.Ammo` *(save, dim "(save)" tag)* · `Game.SkipIntro` · `Game.NoHitFlash` · `Game.AllUnlocked` · `Game.HudScale` · *Reset to defaults* · `__QuitToDesktop` | (File 1) | +| **GRAPHICS** | 11 rows unchanged from D353 · *Reset to defaults* | | +| **AUDIO** (File N) | `Bond.Music` · `Bond.FX` *(save; title annotation only — homogeneous section, PD-style)* · (master volume, M3) · *Reset to defaults* | (File 1) | +| **VIDEO** | `Video.Fullscreen` · `__Resolution` · `Video.VSync` · `Video.FpsCap` · `Video.DisplayFPS` · *Reset to defaults* | | + +Moves from D353 layout: mouse/aim + controller rows merge into **INPUT** +(the two old headers `__HdrMouse`/`__HdrPad` are dropped); DISPLAY → **VIDEO** +(rename, `Video.DisplayFPS` moves from GAME); GAME section is renamed +**GAMEPLAY** and absorbs the surviving watch toggles; the `__HdrBond` header +and `__BondFile` chooser row are retired (replaced by the top save-file row); +`Bond.Look`/`Bond.AimControl` rows are commented out (§3). + +Scope annotation: the D353 "(File N)"/"(no file)" title annotation — today +special-cased to the BOND FILE page via `optionsRowIsBondChooser()` — is +generalized: **any page containing per-file rows annotates**. Sections with +per-file rows: GAMEPLAY, AUDIO. + +F10: same table (F10 already shows all rows, chooser hidden). The +(Gameplay/Audio) annotation in-stage shows the active file number (the +context-aware accessor, §5.8 — *not* the raw chooser value). "Reset to +defaults" rows stay visible in F10 (O2) under the same confirmation +contract (§5.4). + +**(save) tag (O1):** the four mixed-scope GAMEPLAY save rows carry a subtle +dim "(save)" suffix — the only place a section mixes scopes. AUDIO is +homogeneously per-file, so its title annotation is sufficient (no tags). +Implementation: one `int saveScoped` field on `struct Row` + dim-ink suffix in +the row-draw path (frontoptions.c already draws the dim annotation). + +## 5. Code changes + +All in `port/` (D350-family); files: `port/src/optionsoverlay.c`, +`port/src/frontoptions.c`, `port/include/optionsoverlay.h`, +`port/src/watchsettings.c` (only if reset needs a new enqueue helper — +expected not to, see below). + +### 5.1 Row table (`optionsoverlay.c`) + +- Rename headers: `__HdrDisplay`→VIDEO (last page), add `__HdrInput` + (INPUT), `__HdrGameplay` (GAMEPLAY); delete `__HdrBond`, `__HdrMouse`, + `__HdrPad`, and rename `__HdrGame`→GAMEPLAY. New section order in the + table: INPUT, GAMEPLAY, GRAPHICS, AUDIO, VIDEO (Turok's order: Input, + Gameplay, Graphics, Audio, Video). +- Move the 7 mouse/aim + 3 controller rows under INPUT; move the 4 surviving + `Bond.*` toggles + music/FX into GAMEPLAY/AUDIO; comment out + `Bond.Look`/`Bond.AimControl` (§3). +- New `ROW_ACTION` rows, one per section: `.key="__Reset
"`, + `.label="Reset to defaults"`. All five use one shared handler + (`rowResetSection(section)`). + +### 5.2 Save-file top row (`frontoptions.c`) + +- New non-paged line above the page content: "Save file: File N" + a + "change" control (left/right or Enter cycles, reusing + `watchSettingsChooseFile(dir)`; D354 lazy default fills N). Dim ink, same + visual language as the D353 annotation. `selected_folder_num` unchanged — + the row never touches the game's active file (plan Gate B invariant). + **Display value reads through the context-aware accessor (§5.5), never + the raw chooser value.** +- F10 context: row suppressed (extend the D353 accessor to cover it). + +### 5.3 Reset to defaults (Gate E) + +- **Per-file rows** (GAMEPLAY/AUDIO save rows): defaults = **BLANKSAVEDATA** + (`src/game/file2.c` — the game's own factory save; per WATCH-SETTINGS-PLAN + §6, watch rows reset to BLANKSAVEDATA/`DEFAULT_OPTIONS` for the selected + file, *not* from a global ini snapshot; new saves start Music/FX 0xFF, + auto-aim/sight/look-ahead/ammo enabled — read the fields from + `BLANKSAVEDATA` at reset time, no hardcoded table). Reset re-applies live + via the existing `watchsettings` apply path **and** persists field-scoped + to the selected file (`watchSettingsPersistField`), exactly like a normal + commit. In-stage: enqueued through the D352 command queue (N field commits + with `commit=1`) — no new threading; game thread applies + persists. +- **Ini rows** (INPUT/GRAPHICS/VIDEO/GAMEPLAY-ini): `config.c` has no + central default table (first-write "defaults" = current values, + config.c:280), so the section handler uses an explicit + `static const struct { const char *key; double def; }` reset table in + `optionsoverlay.c`, mirroring the port's C initializers (each entry + commented with its source variable; **verified against the initializer at + implementation time** — e.g. `Game.AllUnlocked` = **0**, + `port/src/video.c:116,348`; the D257 comment in optionsoverlay.c that + says "default ON" is stale and is corrected as part of D356). Front-end: + write live + `configSave()` (existing). In-stage ini resets: applied + **immediately** (O3 — consistent with how every other ini row already + behaves when changed in F10; video.c applies most display changes on the + fly). +- **Resolution is excluded from the VIDEO reset (maintainer item 2).** + `__Resolution` is an action backed by `s_resSel` + + `videoRequestWindowSize()` — not a registered config row, so the numeric + default table cannot reset it, and a "default window size" is a + display-capability choice, not a tunable. The D356 findings entry + documents the exception explicitly: VIDEO reset covers Fullscreen, VSync, + FpsCap, DisplayFPS — Resolution keeps the player's current choice. +- **Scope-aware by construction:** a section's reset touches each row + **by its own scope** (file rows → that file's BLANKSAVEDATA values; + ini rows → ini defaults). No cross-scope surprise (Gate E: "do not make + a global reset wipe per-file watch settings" — a reset of the selected + file never touches other files; §5.8 verifies this). +- **Activation contract (§5.4) applies to every section's reset row.** + +### 5.4 Reset activation contract (maintainer item 1) + +A reset is a *destructive bulk write* — GAMEPLAY's in particular changes +both ini values **and** the selected save in one action. WATCH-SETTINGS-PLAN +§6 gates confirmation + persistence policy; this is that policy: + +- **Edge-triggered:** reset actions fire only on a *fresh* event. The + frontoptions left/right key repeat (held keys re-issue the action) is + suppressed for `ROW_ACTION` reset rows — the arming/confirm events must + carry a no-repeat flag (SDL keydown with `repeat==0` / a fresh click), + so a held key or held mouse button can never arm *and* confirm, nor + re-trigger. +- **Two-step confirm:** first activation **arms** the row (label swaps to + "Confirm reset…", armed highlight); a second activation within **3 s** + commits. Timeout, or navigating to another row, disarms (label + restores). Arm state is per-row, per-open; opening/closing F10 or the + options screen clears all armed resets. +- **One commit per confirmed activation.** No auto-repeat, no re-arming on + the same event. F10 and the options screen share the identical contract + (O2: rows stay visible in F10; confirmation makes accidental in-game + resets unlikely). +- The confirmed action then dispatches the section handler (§5.3) exactly + once. + +### 5.5 Context-aware file accessor; retire "none" (maintainer item 3) + +`watchSettingsFolder()` returns the raw `chosen`, which may be -1 before +`frontFolder()` lazily resolves, and in-stage `chosen` can differ from the +active file (`snapFolder`). The generalized title/top-row accessor must be +**context-aware**, not a repurposed chooser-value read: + +- New `watchSettingsActiveFolder(void)` (name tentative): + - **Front-end contexts** (options screen, F10-on-file-select, title): + the *resolved front target* — explicit `chosen` if set, else + `selected_folder_num` if valid, else first valid folder (i.e. the + `frontFolder()` resolution; the accessor is pure-read and does not + mutate `chosen` — lazy mutation stays in `frontFolder()`/the + auto-init fallback). + - **In-stage (F10):** the *active stage target* — `selected_folder_num` + (the file the stage loaded, `snapFolder` at apply time). + - The D353 title annotation and the §5.2 top row both read through this + accessor, in both surfaces. +- **The chooser's "none" position is retired.** `watchSettingsChooseFile` + cycles FOLDER1..MAX only (dropping the -1 stop). Rationale: D354's + default resolution + boot-time BLANKSAVEDATA slots + the auto-init + fallback guarantee a valid target, so "none" was only reachable by + deliberate cycling and serves no remaining purpose — and "Save file: + File N" (the §5.2 invariant) is not true when `chosen == -1`. + `watchSettingsFolder()`'s "-1 = none" contract is updated/superseded by + the new accessor. + +### 5.6 Reset iterates declared rows (maintainer item 4) + +The section handler iterates the section's **declared row range in +`rows[]`** (from its header to the next header), *not* `s_visIdx`/the +currently visible list — conditionally hidden rows (GRAPHICS draw/LOD +sliders while their auto-FOV toggles are on) must be reset too, or they +silently survive a purported section reset. Hidden-row defaults come from +the same default table; their *live* values are written directly (the +`ptr`/`rowSet` path), visibility is irrelevant. The `(save)` tag, hidden-row +resets, and the reset table are all keyed off `rows[]` indices, never the +visible list. + +### 5.7 F10 + +Unchanged machinery: shared table, save-file row suppressed (the D353 +accessor pattern extended to the §5.2 top row). Annotation shows the active +file via §5.5. "Reset to defaults" rows stay **visible** in F10 (O2) under +the §5.4 confirmation contract. + +### 5.8 Reset verification probe (maintainer item 5) + +New env-gated probe **`GE_WSPROBE_RESET=`** (GE_D314/GE_WSPROBE +pattern): at tick N of a *front-end* boot it drives the **UI action path** +— dispatches the GAMEPLAY and VIDEO section-reset handlers through the same +handler the confirm step calls (arm → confirm → dispatch, no bypass), then +logs/asserts on the following tick: + +1. **Selected-file scope:** the chosen file's `save_data` fields == + BLANKSAVEDATA (auto-aim/sight/look-ahead/ammo enabled, Music/FX 0xFF — + per WATCH-SETTINGS-PLAN §6). +2. **Scope isolation:** a *second* folder's `save_data` is byte-identical + to its pre-reset state (a GAMEPLAY reset never touches other files). +3. **Hidden rows:** GRAPHICS draw/LOD slider values (hidden while the + auto-FOV toggles are on) == their table defaults after a GRAPHICS reset, + even while hidden. +4. **Mixed scopes:** after a GAMEPLAY reset, the ini rows + (SkipIntro/NoHitFlash/AllUnlocked/HudScale) == the ini default table + while the save rows == BLANKSAVEDATA — the two scopes land + independently. + +In-stage dispatch is covered by a `GE_WSPROBE`-style queue check on a +`-level_XX` run (reset commits ride the D352 queue; the probe asserts the +enqueued field count and the post-apply save state). + +### 5.9 No changes + +- `buildPages` in `frontoptions.c` (generic; 5 pages < MAX_PAGES), + `watchSettingsGameTick`/queue (D352/D354/D355 logic untouched), the D355 + track-1 resync, `BLANKSAVEDATA` itself. + +## 6. Threading / persistence gates (unchanged, restated) + +- D350: no GE writes from the F10 input/render thread except the sanctioned + front-end commit path; in-stage edits queue to the game thread. +- D352: queue holds across transitions; per-field coalescing. +- D354: front contexts (including F10-on-file-select) route through the + chooser path; `frontFolder()` default resolution; auto-init fallback. +- D355: Music commit + level-entry resync of the BGM (track 1) player. +- D356 adds no new write path: reset = N existing commits (file rows) + + existing ini writes (ini rows). + +## 7. Docs + +- `docs/dev/findings.md` D356 entry + §F index row; regenerate + `findings-index.csv`. +- `docs/dev/WATCH-SETTINGS-PLAN.md`: status + D353/D354 "Bond File" section + references updated; Gate E marked landed (subset: per-section rows). +- `docs/dev/OPTIONS-MENU-PLAN.md` (tracked) + local notes: §6 "align with + Turok" annotated as landed via D356 (M3 master volume + key bindings + remain). + +## 8. Verification + +1. Clean NTSC build (`./build-pc.sh` / `cmake --build build-pc`). +2. Headless smoke: boots to front end; log page count (5, under MAX_PAGES), + no `buildPages` overflow warning, save-file row resolves to File 1 by + default (D354), annotation on GAMEPLAY/AUDIO pages, no "none" reachable + (chooser cycles files only, §5.5). +3. `GE_WSPROBE` / `GE_WSPROBE_FRONT` regression: identical pass as D355 + (queue/commit paths untouched). +4. **`GE_WSPROBE_RESET` (§5.8) exercises the UI action, not just direct + commits:** confirmed-reset dispatch → (a) selected file == BLANKSAVEDATA, + (b) second file byte-identical (scope isolation), (c) hidden GRAPHICS + draw/LOD values == table defaults, (d) mixed-scope GAMEPLAY lands both + scopes independently. In-stage dispatch: `GE_WSPROBE`-style queue check + on a `-level_XX` run. +5. **Confirmation contract check (§5.4):** a held key/click arms but never + confirms (edge-triggered); second activation within 3 s commits exactly + once; timeout/navigate disarms. Headlessly assertable via the probe + (arm-then-timeout leaves values untouched). +6. Manual visual pass (owed, same list as D353): arrows through 5 pages, + save-file cycling updates the annotation live, `(save)` tag on the four + GAMEPLAY save rows, F10 in-stage, reset in stage vs front (including the + armed/confirm visual state), rapid slider drag. + +## 9. Open questions — RESOLVED (maintainer sign-off, 2026-09-27) + +- **O1:** **yes** — subtle dim "(save)" tag on the four mixed-scope + GAMEPLAY rows; AUDIO's title annotation alone is sufficient (homogeneous + section). +- **O2:** **visible in F10**, provided resets require the §5.4 confirmation + step (which they do). +- **O3:** **immediate** in-stage ini resets, consistent with existing ini + edits in F10. +- **O4:** `__QuitToDesktop` stays in GAMEPLAY, **literally last — after the + Reset row** (section order: rows…, *Reset to defaults*, *Quit to desktop*; + the §4 page table is corrected accordingly). + +Layout itself is signed off by the maintainer (2026-09-27): five-page +regroup + menu-only exposure filter, with the §5.4–§5.8 contracts as +specified above. + +## 10. Estimate + +One commit. ~5 files (`optionsoverlay.c`, `frontoptions.c`, +`watchsettings.c`, `port/include/optionsoverlay.h`, +`port/include/watchsettings.h`), net +400/−140 (table reshuffle + reset +contract + context-aware accessor + `GE_WSPROBE_RESET` are the new code). +Findings/csv/plan updates ride along.