diff --git a/docs/M-24-QOL-REVIEW.md b/docs/M-24-QOL-REVIEW.md index 5bc41f0c..fcd71070 100644 --- a/docs/M-24-QOL-REVIEW.md +++ b/docs/M-24-QOL-REVIEW.md @@ -1,5 +1,7 @@ # M-24 — input rework + playtest QoL (review sheet) +> **Follow-up work: `docs/M-25-QOL-REVIEW.md`** (`[Video]` ini knobs, …). + Session M-24, 2026-08-30. Three commits on `master`: | Commit | Title | diff --git a/docs/M-25-QOL-REVIEW.md b/docs/M-25-QOL-REVIEW.md new file mode 100644 index 00000000..3e97b214 --- /dev/null +++ b/docs/M-25-QOL-REVIEW.md @@ -0,0 +1,104 @@ +# M-25 — port-layer QoL / playtest-experience polish (review sheet) + +Session M-25, 2026-08-30. Continues `docs/M-24-QOL-REVIEW.md`. + +**Scope guarantee:** every change is in `port/` (shim layer). **No `src/` +file touched** — no game-logic / ABI / asset-format change, no `#ifdef PORT` +in game code. N64 build untouched. Every new behavior defaults to the +pre-M-25 behavior — a fresh or partial `ge007.ini` changes nothing. + +| Commit | Title | +|---|---| +| _(this)_ | `[Video]` ge007.ini knobs + config auto-migration | + +--- + +## 1. `[Video]` ge007.ini knobs + config auto-migration + +### Rationale + +`video.c` hardcoded five display settings that testers reasonably want to +change per-machine (weak GPU → drop MSAA; capture work → nearest-filter for +crisp texels; high-refresh monitor → uncap fps). All were compile-time only. + +### Changes + +**`port/src/video.c`** — new `PD_CONSTRUCTOR videoConfigInit()` registers: + +| Key | Default | Effect | Applied | +|---|---|---|---| +| `Video.VSync` | 1 | swap interval 0/1 | `wmAPI->set_swap_interval` in `videoInit` | +| `Video.FpsCap` | 0 | frame cap in fps, 0 = uncapped | `gfx_set_target_fps` in `videoInit` | +| `Video.MSAA` | 1 | 1/2/4/8 samples (snapped down to a power of two) | `gfx_msaa_level` before `gfx_init` | +| `Video.TextureFilter` | 1 | 0 = nearest, 1 = bilinear | `gfx_set_texture_filter` + mipmap filter | +| `Video.Fullscreen` | 0 | 0 = windowed, 1 = borderless fullscreen | `window_settings.fullscreen` at window create | + +Every default reproduces the exact prior hardcoded value +(`set_swap_interval(1)`, no fps target, `gfx_msaa_level = 1`, +`FILTER_LINEAR` / `MIPMAP_LINEAR`, windowed). MSAA is clamped to +`{1,2,4,8}` by a snap-down expression so a bogus `MSAA = 3` can't reach the +GL layer. `gfx_pc.cpp` already self-disables MSAA if framebuffers are +unavailable. + +**`port/src/config.c`** — `configLoad()` now tracks, per registered option, +whether the on-disk file actually mentioned it (`seen` flag set in +`applyKV`). If any registered key was missing after the parse, it calls +`configSave()` once to rewrite the file with the new key at its default. + +- Without this, a new `[Video]` block would never appear in an existing + `ge007.ini` (M-24: `configLoad` only writes defaults when the file is + *absent*, and the clean-exit `configSave` is unreachable in practice) — + the knobs would exist but be undiscoverable. +- An **up-to-date** file (every registered key present) is **not** + rewritten — user comments and ordering are preserved, exactly as before. +- A file that *is* missing keys gets rewritten in the canonical grouped + form (comments dropped — same contract M-24 already documented for the + clean-exit save). Existing values are preserved (they're parsed into the + option storage before the re-save). + +### Review risk + +Low. `video.c` changes are all "read an int, pick a code path that already +existed". `config.c` change is one extra conditional `configSave()` on a +path that already round-trips through the same serializer. + +Failure modes: a corrupt ini value is clamped (ints) or ignored (unknown +key, already handled); the migration re-save uses `fopen(path,"w")` which +already had a `cannot write` warning path. + +### Verification + +- Build green (`./build-pc.sh ntsc-final`), no new warnings in `video.c` / + `config.c`. (Pre-existing `getenv` implicit-decl warning in the untouched + `GE_PCDUMP` block is not from this change.) +- Fresh ini (deleted): `[Video]` block written with all five defaults. +- Migration: hand-written ini with only `[Input] MouseAimSpeed = 77` + + `[Audio]` → after one launch, `[Video]` added, `MouseAimSpeed = 77` + preserved. +- No-op: ini with all keys present + a custom leading comment → launch + leaves the comment in place (no rewrite). +- `-level_09`: `MSAA=4 / TextureFilter=0 / VSync=0 / FpsCap=120` → + crash-free to frame 380+, **91.67% pixel coverage (unregressed)**. + Defaults → crash-free to frame 620+, 91.67%. +- `-level_20` (Silo): crash-free to frame 620+ (defaults). NB Silo has a + known pre-existing early freeze on the silo→Bond camera descent (~3 s + in) — reported M-25, unrelated to this change, tracked for a later + session. + +### What to check by playing + +1. Edit `data/ge007.ini` `[Video]` with the game closed; values take effect + next launch. +2. `TextureFilter = 0` → visibly crisp/pixelated textures. `MSAA = 4` → + smoother polygon edges. `Fullscreen = 1` → starts borderless-fullscreen + (Alt-Enter still toggles). +3. `FpsCap = 30` → title-bar fps pins near 30; `VSync = 0` + `FpsCap = 0` → + uncapped. + +## Still TODO (carried from M-24) + +Key rebinding, in-render on-screen FPS/pos overlay, in-game options menu, +backward weapon cycle, the `bondview.c` analog-aim hook (D118a), PNG (not +PPM) screenshots (needs stb_image_write vendored into `port/`), window +size/pos persistence (needs a reliable save-on-exit hook — `configSave` is +currently only on the unreachable clean-shutdown path). diff --git a/port/src/config.c b/port/src/config.c index be17469f..c28e7a1b 100644 --- a/port/src/config.c +++ b/port/src/config.c @@ -29,8 +29,8 @@ #define MAX_OPTIONS 256 #define INI_PATH "$S/ge007.ini" -struct IntOption { char key[64]; int *value; int min, max; }; -struct StrOption { char key[64]; char *value; int bufSize; }; +struct IntOption { char key[64]; int *value; int min, max; int seen; }; +struct StrOption { char key[64]; char *value; int bufSize; int seen; }; static struct IntOption intOpts[MAX_OPTIONS]; static int numIntOpts = 0; @@ -91,6 +91,7 @@ static void applyKV(const char *dottedKey, const char *val) { for (int i = 0; i < numIntOpts; i++) { if (ci_eq(intOpts[i].key, dottedKey)) { + intOpts[i].seen = 1; int v = parseInt(val); if (intOpts[i].min != intOpts[i].max) { if (v < intOpts[i].min) v = intOpts[i].min; @@ -102,6 +103,7 @@ static void applyKV(const char *dottedKey, const char *val) } for (int i = 0; i < numStrOpts; i++) { if (ci_eq(strOpts[i].key, dottedKey)) { + strOpts[i].seen = 1; strncpy(strOpts[i].value, val, strOpts[i].bufSize - 1); strOpts[i].value[strOpts[i].bufSize - 1] = 0; return; @@ -151,6 +153,19 @@ void configLoad(void) } fclose(f); sysLogPrintf(LOG_INFO, "config: loaded %s", path); + + /* Migration: if the running build registers options the on-disk file + * never mentioned (a new [Video] knob, say), rewrite the file once so the + * defaults are visible and editable. Only triggers when something is + * genuinely missing -- an up-to-date file is left untouched (comments and + * all). */ + int missing = 0; + for (int i = 0; i < numIntOpts && !missing; i++) missing = !intOpts[i].seen; + for (int i = 0; i < numStrOpts && !missing; i++) missing = !strOpts[i].seen; + if (missing) { + sysLogPrintf(LOG_INFO, "config: adding newly-registered keys to %s", path); + configSave(); + } } /* Section name = everything before the last '.'; "" if the key isn't dotted. */ diff --git a/port/src/video.c b/port/src/video.c index 47f161d0..7e3f56bc 100644 --- a/port/src/video.c +++ b/port/src/video.c @@ -25,6 +25,7 @@ #include "platform.h" #include "system.h" +#include "config.h" #include "video.h" #include "input.h" @@ -46,6 +47,25 @@ static struct GfxWindowManagerAPI *wmAPI; static struct GfxRenderingAPI *renderingAPI; static int initDone = 0; +/* + * [Video] ge007.ini knobs. Every default reproduces the previously-hardcoded + * behaviour, so a fresh config or a missing [Video] section changes nothing. + */ +static int cfgVSync = 1; /* swap interval: 0 = off, 1 = on */ +static int cfgFpsCap = 0; /* frame cap in fps; 0 = uncapped (vsync) */ +static int cfgMSAA = 1; /* 1/2/4/8 samples; 1 = off */ +static int cfgTexFilter = 1; /* 0 = nearest (crisp), 1 = bilinear */ +static int cfgFullscreen = 0; /* 0 = windowed, 1 = borderless fullscreen */ + +PD_CONSTRUCTOR static void videoConfigInit(void) +{ + configRegisterInt("Video.VSync", &cfgVSync, 0, 1); + configRegisterInt("Video.FpsCap", &cfgFpsCap, 0, 1000); + configRegisterInt("Video.MSAA", &cfgMSAA, 1, 8); + configRegisterInt("Video.TextureFilter", &cfgTexFilter, 0, 1); + configRegisterInt("Video.Fullscreen", &cfgFullscreen, 0, 1); +} + static u32 frames = 0; /* Set by the host event pump (F12), consumed on the render thread in * videoEndFrame where a GL context is current. */ @@ -64,7 +84,9 @@ int videoInit(void) gfx_current_native_aspect = (float)GE_NATIVE_W / (float)GE_NATIVE_H; gfx_framebuffers_enabled = true; gfx_detail_textures_enabled = false; - gfx_msaa_level = 1; + + /* MSAA: snap the requested sample count down to a supported power of two. */ + gfx_msaa_level = cfgMSAA >= 8 ? 8 : cfgMSAA >= 4 ? 4 : cfgMSAA >= 2 ? 2 : 1; struct GfxInitSettings set = { .wapi = wmAPI, @@ -75,7 +97,7 @@ int videoInit(void) .height = GE_NATIVE_H, .x = 100, .y = 100, - .fullscreen = false, + .fullscreen = cfgFullscreen != 0, .fullscreen_is_exclusive = false, .maximized = false, .centered = true, @@ -85,11 +107,17 @@ int videoInit(void) gfx_init(&set); - /* VSync on; fast3d paces the window itself. */ - wmAPI->set_swap_interval(1); + /* VSync + optional fps cap; fast3d paces the window itself. */ + wmAPI->set_swap_interval(cfgVSync ? 1 : 0); + gfx_set_target_fps(cfgFpsCap); /* 0 = uncapped */ - gfx_set_texture_filter(FILTER_LINEAR); - gfx_set_mipmap_filter(MIPMAP_LINEAR); + if (cfgTexFilter) { + gfx_set_texture_filter(FILTER_LINEAR); + gfx_set_mipmap_filter(MIPMAP_LINEAR); + } else { + gfx_set_texture_filter(FILTER_NONE); + gfx_set_mipmap_filter(MIPMAP_NEAREST); + } /* The GL context is currently current on this (host main) thread, but all * rendering happens on the game's scheduler thread. WGL only allows a