MagicBeanDev/Longshot

Watch

Fork 98

fix(website): the settings page takes the menu's text row, so the site builds again #533

Open

jarynclouatre wants to merge 1 commit from jarynclouatre/Longshot:fix/website-text-row into main

jarynclouatre commented

2026-11-14 18:07:57 +00:00

Contributor

Type: fix · Risk: low · Tested on: Linux x86-64 (game-free checks only) · Needs: none

Why

The website stopped building on main. npm run build exits 1 with README/menu graphics coverage mismatch at Custom output size, and node --test tests/*.test.ts in website/ gives 64 of 72. The hourly website job (.forgejo/workflows/website.yml) runs the build and npm run check before it deploys, so the site no longer updates. It stopped before #51: since #136 (the Mono choice) one website test has failed, and npm run check runs the website tests before wrangler deploy in that job.

readGraphicsSettings (website/src/repo-docs/graphics-settings.ts) takes every Graphics row that is not a heading only when its value starts with &opts.. The Custom output size row from #51 is a text field: its value is nullptr and the field lives in field_rows[] (gpu/vk/vk_menu.cpp), so the reader threw at it.

None filed. Found running the website's tests on main.

How to reproduce

Seen on: Linux x86-64, running the website's tests and build; the game was not run.
Settings and switches: defaults
Steps: In website/ after npm ci, run node --test tests/*.test.ts and npm run build on main.
Expected: The website tests pass and the settings page builds with the Custom output size row.
Actual: The tests give 64 of 72, and the build exits 1 with README/menu graphics coverage mismatch at Custom output size.

What changed

The website's settings reader now accepts text fields listed by the menu. The tests also use the current restart rows and Audio output wording, so the site builds again.

website (website/src/repo-docs/graphics-settings.ts): readMenuRows reads the names in the menu's field_rows[] and marks those valueless rows as fields. A field row is listed like the others, with README's choices and default in order. Its restart comes from its help's "Applies after a restart": the menu keeps the restart on the row the field sets (Output resolution), not on the field's own row. Any other valueless row that is not a heading still stops the build.
tests (website/tests/graphics-settings.test.ts): a new case reads a menu table with a text row named in field_rows[] (listed, restart from its help) and a valueless row that is not one (still refused), and checks that the real Custom output size takes README's choices and default. Two existing checks that the throw hid were out of date: the list of restart rows lacked Skip title screen (#399) and now Custom output size, and the Audio output restart drift case still replaced README's line from before Mono (#136), so it no longer changed anything and its assert.throws failed. That last one is the check that already failed before #51.

Measurements

Verdict: the website tests go from 64 of 72 to 73 of 73 and the site builds; the game-free suite has no new failures.

Hardware: none used (a Linux x86-64 PC, Node 24.22, Python 3.14, gcc 15.2)
Scene: none, the game was not run
Method: 2026-11-14. in website/ after npm ci: node --test tests/*.test.ts, npm run build, node scripts/check-site.ts; then python3 -I -B tests/run_all.py, two runs each, interleaved (main, PR, main, PR). The new website case also ran against main's graphics-settings.ts. Branch: fix/website-text-row.
How to reproduce: the commands above in a checkout of each commit

	Before: main @ `543846eb`	After: this PR @ `6102ddc3`
website tests	64 of 72	73 of 73
`npm run build`	exits 1: `coverage mismatch at Custom output size`	builds
`node scripts/check-site.ts`	not run (nothing built)	passed: 15 pages, 842 local links and assets
new website case, main's reader	fails	passes
`tests/run_all.py` (run 1, run 2)	485 passed, 2 failed, 84 skipped (both)	485 passed, 2 failed, 84 skipped (both)
failing checks	`test_bake_portability.py`, `test_walk_key.py`	the same two

test_bake_portability.py fails here for a missing Python module (zstandard). test_walk_key.py's fixture doesn't link on main (undefined reference to hos_oot_autoload_input).

Flags and settings

None.

Testing

Tested: the website's tests, build and site check, and the game-free suite, on a Linux PC, as in Measurements.

Not tested: the deploy itself (it needs the Cloudflare token). The built settings page was not looked at in a browser.

Must be tested: None.

Risks

What could break: A new text field row in field_rows[] is published from README's line like any other setting; one README does not list still stops the build. #236 (README split) rewords comments and error texts in the same file; whichever lands second needs a small rebase.

Turning it off: Reverting the commit brings the build failure back.

Compatibility: Nothing changes in the game. The website builds again and lists Custom output size with README's choices and default.

Checklist

Everyone

One topic.
The title is a Conventional Commit (type(scope): description) of the status line's type.
Why gives the reason plainly and links the issues that fit (the tracker searched).
Measurements has a verdict, or one line saying why there is nothing to measure.
Testing names what was tested, what wasn't, and what must still be tested, where and how.
Rebased on current main.
python -I -B tests/run_all.py passes (any failure that also fails on main is named).
No game files, dumps, build output or generated files.
third_party/ untouched (a new folder with a PROVENANCE.md instead).
The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled.

When it applies

Numbers from the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
A game-free test added for the fix, failing on main and passing here.
A check under tests/ that reads changed source updated in this PR, not weakened.

**Type:** fix · **Risk:** low · **Tested on:** Linux x86-64 (game-free checks only) · **Needs:** none ## Why The website stopped building on main. `npm run build` exits 1 with `README/menu graphics coverage mismatch at Custom output size`, and `node --test tests/*.test.ts` in `website/` gives 64 of 72. The hourly website job (`.forgejo/workflows/website.yml`) runs the build and `npm run check` before it deploys, so the site no longer updates. It stopped before #51: since #136 (the Mono choice) one website test has failed, and `npm run check` runs the website tests before `wrangler deploy` in that job. `readGraphicsSettings` (`website/src/repo-docs/graphics-settings.ts`) takes every Graphics row that is not a heading only when its value starts with `&opts.`. The Custom output size row from #51 is a text field: its value is `nullptr` and the field lives in `field_rows[]` (`gpu/vk/vk_menu.cpp`), so the reader threw at it. None filed. Found running the website's tests on main. ## How to reproduce **Seen on:** Linux x86-64, running the website's tests and build; the game was not run. **Settings and switches:** defaults **Steps:** In `website/` after `npm ci`, run `node --test tests/*.test.ts` and `npm run build` on main. **Expected:** The website tests pass and the settings page builds with the Custom output size row. **Actual:** The tests give 64 of 72, and the build exits 1 with `README/menu graphics coverage mismatch at Custom output size`. ## What changed The website's settings reader now accepts text fields listed by the menu. The tests also use the current restart rows and Audio output wording, so the site builds again. - website (`website/src/repo-docs/graphics-settings.ts`): `readMenuRows` reads the names in the menu's `field_rows[]` and marks those valueless rows as fields. A field row is listed like the others, with README's choices and default in order. Its restart comes from its help's "Applies after a restart": the menu keeps the restart on the row the field sets (Output resolution), not on the field's own row. Any other valueless row that is not a heading still stops the build. - tests (`website/tests/graphics-settings.test.ts`): a new case reads a menu table with a text row named in `field_rows[]` (listed, restart from its help) and a valueless row that is not one (still refused), and checks that the real Custom output size takes README's choices and default. Two existing checks that the throw hid were out of date: the list of restart rows lacked Skip title screen (#399) and now Custom output size, and the Audio output restart drift case still replaced README's line from before Mono (#136), so it no longer changed anything and its `assert.throws` failed. That last one is the check that already failed before #51. ## Measurements **Verdict:** the website tests go from 64 of 72 to 73 of 73 and the site builds; the game-free suite has no new failures. Hardware: none used (a Linux x86-64 PC, Node 24.22, Python 3.14, gcc 15.2) Scene: none, the game was not run Method: 2026-11-14. in `website/` after `npm ci`: `node --test tests/*.test.ts`, `npm run build`, `node scripts/check-site.ts`; then `python3 -I -B tests/run_all.py`, two runs each, interleaved (main, PR, main, PR). The new website case also ran against main's `graphics-settings.ts`. Branch: `fix/website-text-row`. How to reproduce: the commands above in a checkout of each commit | | Before: main @ 543846eb | After: this PR @ 6102ddc3 | |---|---|---| | website tests | 64 of 72 | 73 of 73 | | `npm run build` | exits 1: `coverage mismatch at Custom output size` | builds | | `node scripts/check-site.ts` | not run (nothing built) | passed: 15 pages, 842 local links and assets | | new website case, main's reader | fails | passes | | `tests/run_all.py` (run 1, run 2) | 485 passed, 2 failed, 84 skipped (both) | 485 passed, 2 failed, 84 skipped (both) | | failing checks | `test_bake_portability.py`, `test_walk_key.py` | the same two | `test_bake_portability.py` fails here for a missing Python module (zstandard). `test_walk_key.py`'s fixture doesn't link on main (`undefined reference to hos_oot_autoload_input`). ### Flags and settings None. ## Testing **Tested:** the website's tests, build and site check, and the game-free suite, on a Linux PC, as in Measurements. **Not tested:** the deploy itself (it needs the Cloudflare token). The built settings page was not looked at in a browser. **Must be tested:** None. ## Risks **What could break:** A new text field row in `field_rows[]` is published from README's line like any other setting; one README does not list still stops the build. #236 (README split) rewords comments and error texts in the same file; whichever lands second needs a small rebase. **Turning it off:** Reverting the commit brings the build failure back. **Compatibility:** Nothing changes in the game. The website builds again and lists Custom output size with README's choices and default. ## Checklist ### Everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`) of the status line's type. - [x] Why gives the reason plainly and links the issues that fit (the tracker searched). - [x] Measurements has a verdict, or one line saying why there is nothing to measure. - [x] Testing names what was tested, what wasn't, and what must still be tested, where and how. - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` passes (any failure that also fails on `main` is named). - [x] No game files, dumps, build output or generated files. - [x] `third_party/` untouched (a new folder with a `PROVENANCE.md` instead). - [x] The license is OK: GPL-3.0-or-later, nothing incompatible and nothing decompiled. ### When it applies - [x] Numbers from the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [x] A game-free test added for the fix, failing on `main` and passing here. - [x] A check under `tests/` that reads changed source updated in this PR, not weakened.

jarynclouatre added 1 commit

2026-11-14 18:07:57 +00:00

fix(website): the settings page takes the menu's text row, so the site builds again

CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions

Details

CI / Conventional Commit title (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions

Details

Third-party parts / As their notes say (pull_request) Blocked by required conditions

Details

Third-party parts / The same as upstream (pull_request) Blocked by required conditions

Details

Website / Website on Cloudflare (pull_request) Blocked by required conditions

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

PR description / Description follows the template (pull_request_target) Successful in 6s

Details

beanstalk/approval Test farm not requested

6102ddc32b

The menu's Custom output size row (#51) is a text field: it has no
&opts value, its field is in field_rows[]. readGraphicsSettings threw
"coverage mismatch" at it, so npm run build stopped and the website
was not deployed.

readMenuRows now reads field_rows[] names and marks those rows as
fields. A field's restart comes from its help's "Applies after a
restart", since the row it sets (Output resolution) holds the restart.
Any other valueless row that is not a heading still stops the build.

Two tests that the throw hid were out of date: the restart list lacks
Skip title screen (#399) and Custom output size, and the Audio output
restart drift case still replaced the line from before Mono (#136),
so it no longer changed anything.

jarynclouatre changed title from ~~WIP: fix(website): the settings page takes the menu's text row, so the site builds again~~ to fix(website): the settings page takes the menu's text row, so the site builds again

2026-11-14 18:13:09 +00:00

mooki added the

labels

2026-11-14 18:15:33 +00:00

futondestroyer approved these changes

2026-11-14 21:31:45 +00:00

futondestroyer left a comment

Reviewed head 6102ddc32bef2aba721fca82c142c3f53083354a; no correctness issue found in the complete two-file diff.

Independent Windows 11 / Node 24.22.0 validation with the pinned lockfile: the unchanged parent 543846eb website gives 64 passed / 8 failed (72 tests), including the Custom output size coverage error. This head gives 73 passed / 0 failed. The added field case also checks that an unrecognized valueless row still stops publication. The corrected restart/Mono assertions continue to enforce the source/README contract rather than bypassing it.

The reader accepts a null-valued, non-heading row only when field_rows[] names it, and keeps graphics ordering, coverage and restart checks. This covers the real Custom output size text row without accepting arbitrary valueless settings. No dependency, build-script or deployment change is present.

Independent full Astro build is not claimed: on Windows both parent and PR stop earlier in unchanged src/repo-docs/assets.ts, where the asset hook joins a file-URL pathname into a doubled-drive path (<drive>:\\<drive>:\\...\\public\\assets). That is a separate pre-existing Windows path issue, not introduced by this diff. The author's Linux build evidence is in the description; this review independently verifies the parser/test regression and source change. No deployment performed.

Reviewed head `6102ddc32bef2aba721fca82c142c3f53083354a`; no correctness issue found in the complete two-file diff. Independent Windows 11 / Node 24.22.0 validation with the pinned lockfile: the unchanged parent `543846eb` website gives 64 passed / 8 failed (72 tests), including the Custom output size coverage error. This head gives **73 passed / 0 failed**. The added field case also checks that an unrecognized valueless row still stops publication. The corrected restart/Mono assertions continue to enforce the source/README contract rather than bypassing it. The reader accepts a null-valued, non-heading row only when `field_rows[]` names it, and keeps graphics ordering, coverage and restart checks. This covers the real Custom output size text row without accepting arbitrary valueless settings. No dependency, build-script or deployment change is present. Independent full Astro build is **not claimed**: on Windows both parent and PR stop earlier in unchanged `src/repo-docs/assets.ts`, where the asset hook joins a file-URL pathname into a doubled-drive path (`<drive>:\\<drive>:\\...\\public\\assets`). That is a separate pre-existing Windows path issue, not introduced by this diff. The author's Linux build evidence is in the description; this review independently verifies the parser/test regression and source change. No deployment performed.

HuzZz referenced this pull request

2026-11-14 22:59:21 +00:00

Sun shadows on Link flicker/break dependant on camera view #601

mooki added the

area/menu

label

2026-11-15 13:55:10 +00:00

malon referenced this pull request

2026-11-15 15:11:03 +00:00

Game: Keeps on crashing in death mountain #817

CI / Workflow lint (actionlint) (pull_request) Blocked by required conditions

Details

CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Blocked by required conditions

Details

CI / Conventional Commit title (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Blocked by required conditions

Details

CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Blocked by required conditions

Details

Third-party parts / As their notes say (pull_request) Blocked by required conditions

Details

Third-party parts / The same as upstream (pull_request) Blocked by required conditions

Details

Website / Website on Cloudflare (pull_request) Blocked by required conditions

Details

Website / Website on Cloudflare (pull_request_target) Has been skipped

Details

PR description / Description follows the template (pull_request_target) Successful in 6s

Details

beanstalk/approval Test farm not requested

This pull request has changes conflicting with the target branch.

website/src/repo-docs/graphics-settings.ts
website/tests/graphics-settings.test.ts

Some workflows are waiting to be reviewed.

View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Merge commit title

fix(website): the settings page takes the menu's text row, so the site builds again (#533)

Merge commit body

Reviewed-on: https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/533

fix(website): the settings page takes the menu's text row, so the site builds again (#533)

Reviewed-on: https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/533