fix(menu): stop reporting guessed defaults in Bug reports #103

Merged
HumanBean merged 4 commits from mooki/Longshot:fix/bug-report-dynamic-defaults into main 2026-11-14 01:01:53 +00:00
Member

Why

Bug reports should not invent a default when a setting depends on the display, upscaler or Auto preset. The old report could say Aspect ratio: Monitor's shape (default: 32:9) even when the player left Monitor's shape selected.

This changes report text only, not the player's settings. HumanBean rebased the three original commits onto main fbd420241a and added 3078962ee62c268530a8e342c591768ea687c2d3. That is his commit, tested unchanged here. The four older game runs below had shared load, so they do not show a speed or cost difference.

What changed

  • gpu/vk/vk_menu.cpp: keep current PC-dependent values without inventing a default. report_default() restores defaults the menu can name. Writing a report does not change any option.
  • gpu/vk/vk_bug_report.h/.cpp: keep all settings in the form's Changes from the defaults field. A Depends on this PC line comes after known-default changes and before switches. There is no separate section to lose when filing the issue.
  • HumanBean's 3078962e adds the link fallbacks and known-default handling. The existing report test now checks the field, its ordering and the shorter links. The defaults test covers 16 authored cases, including a changed aspect ratio, Custom before and after Auto, Frame generation by, and saved game language. No existing check was removed.
  • The menu wording, translations, option values, startup defaults, setters and save format are unchanged. No new row, collector or attachment is added.

Current report layout

The README sentence this follows is:

Settings whose defaults depend on the display, upscaler, Auto preset or other run-time context are listed after the changes, in the same answer, under a Depends on this PC line, with their current value and “default depends on this PC”.

This example was printed by the real formatter and report writer at 3078962e, using an authored 3440x1440 monitor, a 16:9 output, Custom before Auto, NVIDIA DLSS, Frame generation by set to Built-in FG, and a 110% field of view. It is a game-free example, not a new game run. The monitor default is named by the code, not guessed:

### Changes from the defaults

Aspect ratio: 16:9 (default: 21:9 (monitor's))
Frame generation by: Built-in FG (default: Auto)
Field of view: 110% (default: 100% (original))
Depends on this PC (the values now, not counted as changes):
Quality preset: Custom (default depends on this PC)
Output resolution: 2048x1152 (game's) (default depends on this PC)
Upscaler: NVIDIA DLSS (default depends on this PC)
Render resolution: Auto (dynamic) (default depends on this PC)
Window scaling filter: Not needed (window = output) (default depends on this PC)
Switches set (the environment, longshot_env.txt; a value only when it is a number or on/off, and never for a name with TOKEN, KEY, ROOM, SECRET, PASS, SERVER or USER in it):
RX_VK_SHOT_EVERY=60

Rows that still say default depends on this PC:

Rows Why
Output resolution, Upscaler, Render resolution, Window scaling filter Their displayed text reads the current display, engine or picture sizes. Formatting a constructor value would describe the live state, not a reliable default.
Quality preset A first start chooses Auto (this PC), not the constructor's Custom value.
DLSS model, while RX_VK_DLSS_PRESET applies The override determines the model for this run. Without it, the built-in model default is retained.
Sharpness, Frame pacing, Game speed, Profile, VRAM budget, with a preset selected or after Auto has chosen for the options file Their defaults depend on those choices. The report does not run apply_auto on options the renderer is using.

Rows with a known default now state it when changed:

  • Aspect ratio: the monitor's shape, as shown in the example.
  • Frame generation by: plain Auto, not the name of the engine currently making frames.
  • Game language: the language saved in language.txt, otherwise the first language. Leaving that menu row normally saves the choice, so this difference rarely remains in a report.
  • On Custom before Auto has ever chosen, Sharpness, Frame pacing, Game speed, Profile and VRAM budget use their built-in defaults. They move to the PC-dependent list after Auto has chosen, or with a preset selected.
  • GPU Auto, the system-default microphone and other fixed defaults keep their existing behavior. A named microphone still appears only as “a microphone by its name”.

The link keeps the same field. It first includes known-default changes, PC rows and switch names, never switch values. If that will not fit, it tries these three shorter forms in order. These are actual decoded field:settings values printed by the report writer at 3078962e. The authored fixture adds switches and then rows to reach each length limit; those padding rows are not new menu options.

Switch names become a count; the setting rows stay:

Aspect ratio: 16:9 (default: 21:9 (monitor's))
Frame generation by: Built-in FG (default: Auto)
Field of view: 110% (default: 100% (original))
Depends on this PC (the values now, not counted as changes):
Quality preset: Custom (default depends on this PC)
Output resolution: 2048x1152 (game's) (default depends on this PC)
Upscaler: NVIDIA DLSS (default depends on this PC)
Render resolution: Auto (dynamic) (default depends on this PC)
Window scaling filter: Not needed (window = output) (default depends on this PC)
Switches set: 81, listed in the attached report.txt.

PC rows become a count; the known-default changes stay:

Aspect ratio: 16:9 (default: 21:9 (monitor's))
Frame generation by: Built-in FG (default: Auto)
Field of view: 110% (default: 100% (original))
Depends on this PC: 80, listed in the attached report.txt.
Switches set: 81, listed in the attached report.txt.

Counts only:

83 Insert menu options with fixed defaults changed, 80 whose defaults depend on this PC and 81 switches set, listed in the attached report.txt.

Flags and settings

Flag What it does Default now Recommendation Helps on Doesn't help on Untested on
Gameplay → Bug report Same action; reports PC-dependent values without an invented default Manual press, unchanged; no setting value or startup default changes On by default: accurate report labels should need no opt-in Display/upscaler/Auto-dependent diagnostic rows Changing rendering settings or resetting them Native Windows and other monitor configurations

Measurements

Branch state: on main fbd420241afdba1d5ba2936971eb48549edb85ae; head 3078962ee62c268530a8e342c591768ea687c2d3; whole game-free suite on this head: 515 passed, 0 failed, 15 skipped (main fbd420241afdba1d5ba2936971eb48549edb85ae: 514 passed, 0 failed, 15 skipped). The rebase onto this main and the last commit of the branch are HumanBean's; the suite was run on his head unchanged, on Arch Linux. Main has gained 10 commits since; the branch still merges cleanly.

Review checks at 3078962e: both report checks and 57 other source-reading checks exited 0 on Linux. The defaults check printed report defaults: 16 cases, 0 failed. The recorder's Windows exit case and the Windows UTF-8 compile probe were skipped. This is targeted coverage, not the whole-suite result in the branch-state block.

Earlier native runs

The four runs below keep their original commits and results. Their separate PC section is historical, not the current report layout.

Rows for docs/tested-configurations.md, kept here instead of in the branch (see #45)
| Date | Build | OS / desktop | GPU / driver | CPU / RAM | Settings | Where | Result |
|---|---|---|---|---|---|---|---|
| 2026-11-12 | Longshot main d4a69456c (source 33703252) | Arch/Omarchy Linux 7.2.15-1-omarchy, windowless | RTX 4090, NVIDIA 615.78.08 | Core i9-13900K, 94 GB reported | Custom, 1920x1080, Game's own 50%, FG off, cap 60, precompile off, Aspect ratio Monitor's shape, Text speed Instant; ordinary shared load | Child Hyrule Field, Gameplay → Bug report; first interleaved run | 92.2s, exit 0. Report falsely says Monitor's shape has default 32:9. Exactly report.txt, log and picture; all 61 option values/labels unchanged. Text files omit the user name, the home folder and the host name. No fatal/device/kernel/core fault; complete journal. Not a speed measurement. |
| 2026-11-12 | Longshot report-default fix 93462ee71 (source f0c7f0e0) | Arch/Omarchy Linux 7.2.15-1-omarchy, windowless | RTX 4090, NVIDIA 615.78.08 | Core i9-13900K, 94 GB reported | Same copied save and settings; second interleaved run | Same child Hyrule Field spot and Bug report row | 92.7s, exit 0. No invented 32:9 default; 13 PC-dependent rows in their own section. Real Text speed difference retained; all 61 option values/labels unchanged. Same three files, text privacy checks pass. No fatal/device/kernel/core fault; complete journal. Not a speed measurement. |
| 2026-11-12 | Longshot main d4a69456c (source 33703252) | Arch/Omarchy Linux 7.2.15-1-omarchy, windowless | RTX 4090, NVIDIA 615.78.08 | Core i9-13900K, 94 GB reported | Same copied save and settings; third interleaved run | Same child Hyrule Field spot and Bug report row | 92.5s, exit 0. Wrong Monitor's shape/default 32:9 line reproduced again; same three files and unchanged 61 option values/labels. Text privacy checks pass; no fatal/device/kernel/core fault; complete journal. Ordinary shared load, no speed claim. |
| 2026-11-12 | Longshot report-default fix 93462ee71 (source f0c7f0e0) | Arch/Omarchy Linux 7.2.15-1-omarchy, windowless | RTX 4090, NVIDIA 615.78.08 | Core i9-13900K, 94 GB reported | Same copied save and settings; fourth interleaved run | Same child Hyrule Field spot and Bug report row | 93.2s, exit 0. Wrong default absent again; 13 PC-dependent rows separate, real Text speed difference retained. Same three files and unchanged 61 values/labels; text privacy checks pass; no fatal/device/kernel/core fault; complete journal. Microphone list bounds changed during enumeration in all four runs, not its selected value. No speed claim. |

Historical main: d4a69456c9 (2026-11-12)
Historical PR: 93462ee71d16f1f77284cf120efbaf737ef8f49e
main: d4a69456c9 (2026-11-12)
this PR: fix/bug-report-dynamic-defaults @ 93462ee71d16f1f77284cf120efbaf737ef8f49e (the head the four game runs used; the current head is in the branch-state line above)
Hardware: RTX 4090, NVIDIA 615.78.08; Core i9-13900K, 96 GB installed (the generated report says 94 GB); Arch/Omarchy Linux 7.2.15-1-omarchy
Scene: same copied child Hyrule Field save, 1920x1080, Custom, Game's own upscaler 50%, frame generation off, cap 60, precompile off, Aspect ratio Monitor's shape, Text speed Instant; windowless
Method: two interleaved pairs, main / branch / main / branch, 92.2–93.2 s each under shared load. Press the actual Bug report row once per run; inspect its generated text, file inventory, privacy and before/after menu values. Also run the real-source nine-context fixture twice each way in the same order. These are correctness observations, not timing benchmarks.
How to reproduce: on the historical main, leave Aspect ratio on Monitor's shape, open Gameplay → Bug report and save once to see the wrong default. With the new code, a row at the monitor default is omitted; a different shape names the monitor's shape as its default. Fixed-default differences such as Text speed remain in Changes from the defaults. Run python -I -B tests/test_bug_report_defaults.py; --source selects another checkout for the same authored cases.

main @ d4a69456 this PR @ 93462ee7
False Aspect ratio default lines, run1 / run2 1 / 1 0 / 0
Median false Aspect ratio default lines 1 0
PC-dependent rows in the old separate section, run1 / run2 0 / 0 13 / 13
Selected option values/labels unchanged by report, both runs 61 of61 61 of61
Native report files, each run 3 3
Authored contexts failing, two interleaved checks 7 / 7 of9 0 / 0 of9
Median authored failures 7 0

Historical main report excerpt:

### Changes from the defaults
Aspect ratio: Monitor's shape (default: 32:9)
Text speed: Instant (default: Normal)

That is the wrong default from the historical main runs. The earlier fix put PC rows in their own section. HumanBean's commit replaces that layout with the single field shown above.

The fixture's negative control uses d725b363 runtime source, identical to the measured main for this code, plus only the new authored test. Its repeated output is:

report defaults: 9 cases, 7 failed: aspect-default, reordered-shapes, output-context, render-context, upscaler-context, auto-profile, model-env

Both fixed invocations passed; the first also passed the full report-writer fixture. The stable GPU/microphone and fixed-model cases already pass on main and remain covered.

Order Build Wall time Load start → end Report result
1 main d4a69456 92.2s 69.16 →34.88 Wrong32:9 default reproduced
2 branch93462ee7 92.7s 67.03 →80.48 Current value separated; no guessed default
3 main d4a69456 92.5s 135.05 →137.20 Wrong default reproduced again
4 branch93462ee7 93.2s 68.47 →85.68 Correct separation again

All four exited0, with no fatal, device-loss, virtual-core escape or kernel fault and complete journal coverage. Each folder contains exactly report.txt, longshot_log.txt and picture.png. Both text files omit the user name, home path and host name. The microphone list's upper bound changed18→2 during enumeration in every run; its selected value and label did not change, so the61-value invariant deliberately does not claim identical dynamic device-list bounds.

The older builds use matching release profiles. Their results keep the measured commits named above. They do not exercise HumanBean's new report layout or known-default changes. The source checks at 3078962e cover those changes, not a new game run.

Testing

Tested: at HumanBean's 3078962e, unchanged: python -I -B tests/test_bug_report.py passed. python -I -B tests/test_bug_report_defaults.py printed report defaults: 16 cases, 0 failed. Another 57 checks that read either changed C++ file exited 0 on Linux. The recorder's Windows exit case and the Windows UTF-8 compile probe were skipped, not passed. The whole-suite result and any failing test names are recorded once in the branch-state block under Measurements. These targeted checks are not a whole-suite result.

Earlier game evidence: the four native Linux runs and the two real-source fixture pairs in Measurements keep their original commit names. They show the original wrong default and unchanged option values. They do not exercise the new report layout.

Built for Windows and Linux at the current head, not run: At 3078962ee62c268530a8e342c591768ea687c2d3, the Windows x86-64 Vulkan renderer DLL and all 44 selected host translation units were cross-built on Linux with llvm-mingw 20261104 (Clang 23.1.5), using the project's production flags and pinned dependencies (renderer: 35 newly compiled / 170 reused; host: 1 newly compiled / 43 reused; lifted C: 0 newly compiled / 80 reused; all Windows units: 36 newly compiled / 607 reused from matching object-cache entries), the renderer DLL linked and the full game executable linked; nothing was run on Windows or under Wine, and this is neither the project's hosted builder run nor a Windows UTF-8 run-time path test. The same head also compiled and linked its full native Linux executable and renderer with Clang 23.1.3 through the documented resume path (201 newly compiled units, 284 matching Linux-cache units, source stamp c52a6bc9), using the verified target-neutral C from the fresh fbd420241a lift rather than carrying the patch onto another commit; neither Linux binary was run. An earlier Windows link at 9291b06f, before the last commit, gave the same result.

Not tested: at 3078962e: a full game build, the revised report in a running game, Windows gameplay or C++ fixture execution, other GPUs or real monitor shapes, physical microphone devices, real-window rendering, and a signed-in form submission. Authored tests cover the shape and engine cases, not those physical configurations. No rendering, default-setting or performance improvement is claimed.

Needs other testers:

  • Windows or a different monitor shape: leave Aspect ratio at the monitor's shape and save a Bug report. It should not appear as a changed setting. Choose another shape and check that the report names the monitor's shape as the default. Genuine fixed-default changes must remain listed: test request T-VNZ6

Screenshots

These pictures are from the older native runs at the commits in their captions. The report illustration shows the old separate-section layout, replaced by the single field above.

Selected verbatim lines from the two generated native reports, clearly labelled as a text illustration rather than a menu screenshot: main @ d4a69456 invents 32:9; this PR @ 93462ee7 keeps the current value and admits the context-dependent default. Sources and JPG inspected.

Actual main @ d4a69456 and this PR @ 93462ee7 menus at the same saved location, both confirming a local report. The action and translated helper text remain unchanged; the corrected content is in the saved report. Sources and JPG inspected.

Risk and rollback

The report-only classification must stay aligned with rows that Auto or the display/engine controls. The test derives Auto-written members from the real setters and checks that they are included. A wrong classification would hide or misdescribe a diagnostic difference, not change the player's setting. Reverting this fix restores the old report format; no save or renderer interface migration is needed.

Reviewer notes

Start with report_default_depends_on_pc, report_default and changed_rows_text in gpu/vk/vk_menu.cpp, then the settings-field fallbacks in gpu/vk/vk_bug_report.cpp. HumanBean's 3078962e supplies the review changes. Auto's choices are not recomputed while writing a report: calling apply_auto would alter the live options the renderer reads. The richer collector is a separate draft and is not part of this review.

Issues

See #44

Checklist

For everyone

  • One topic.
  • The title is a Conventional Commit (type(scope): description).
  • Rebased on current main.
  • python -I -B tests/run_all.py run and passing (any failure that also fails on main is named above).
  • Tested in the game, or what wasn't is named under Testing.
  • Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests).
  • Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported.
  • 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.

If applicable

  • AI assistance recorded with Assisted-by: / Co-Authored-By: trailers.
  • Flags noted, and the switch reference updated (docs/development.md, the README).
  • Screenshots for a change that shows on screen.
  • 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.
  • Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. At 3078962e: the Linux executable and renderer built and linked; the Windows renderer and game executable cross-built and linked on Linux (see Testing); neither was run.
  • Docs updated.

New on this forge; not opened on GitHub.

Assisted-by: GPT-6 Astra
Co-Authored-By: GPT-6 Astra [email protected]

## Why Bug reports should not invent a default when a setting depends on the display, upscaler or Auto preset. The old report could say `Aspect ratio: Monitor's shape (default: 32:9)` even when the player left Monitor's shape selected. This changes report text only, not the player's settings. HumanBean rebased the three original commits onto main `fbd420241a` and added `3078962ee62c268530a8e342c591768ea687c2d3`. That is his commit, tested unchanged here. The four older game runs below had shared load, so they do not show a speed or cost difference. ## What changed - `gpu/vk/vk_menu.cpp`: keep current PC-dependent values without inventing a default. `report_default()` restores defaults the menu can name. Writing a report does not change any option. - `gpu/vk/vk_bug_report.h/.cpp`: keep all settings in the form's **Changes from the defaults** field. A **Depends on this PC** line comes after known-default changes and before switches. There is no separate section to lose when filing the issue. - HumanBean's `3078962e` adds the link fallbacks and known-default handling. The existing report test now checks the field, its ordering and the shorter links. The defaults test covers 16 authored cases, including a changed aspect ratio, Custom before and after Auto, Frame generation by, and saved game language. No existing check was removed. - The menu wording, translations, option values, startup defaults, setters and save format are unchanged. No new row, collector or attachment is added. ### Current report layout The README sentence this follows is: > Settings whose defaults depend on the display, upscaler, Auto preset or other run-time context are listed after the changes, in the same answer, under a **Depends on this PC** line, with their current value and “default depends on this PC”. This example was printed by the real formatter and report writer at `3078962e`, using an authored 3440x1440 monitor, a 16:9 output, Custom before Auto, NVIDIA DLSS, Frame generation by set to Built-in FG, and a 110% field of view. It is a game-free example, not a new game run. The monitor default is named by the code, not guessed: ```text ### Changes from the defaults Aspect ratio: 16:9 (default: 21:9 (monitor's)) Frame generation by: Built-in FG (default: Auto) Field of view: 110% (default: 100% (original)) Depends on this PC (the values now, not counted as changes): Quality preset: Custom (default depends on this PC) Output resolution: 2048x1152 (game's) (default depends on this PC) Upscaler: NVIDIA DLSS (default depends on this PC) Render resolution: Auto (dynamic) (default depends on this PC) Window scaling filter: Not needed (window = output) (default depends on this PC) Switches set (the environment, longshot_env.txt; a value only when it is a number or on/off, and never for a name with TOKEN, KEY, ROOM, SECRET, PASS, SERVER or USER in it): RX_VK_SHOT_EVERY=60 ``` Rows that still say **default depends on this PC**: | Rows | Why | |---|---| | Output resolution, Upscaler, Render resolution, Window scaling filter | Their displayed text reads the current display, engine or picture sizes. Formatting a constructor value would describe the live state, not a reliable default. | | Quality preset | A first start chooses Auto (this PC), not the constructor's Custom value. | | DLSS model, while `RX_VK_DLSS_PRESET` applies | The override determines the model for this run. Without it, the built-in model default is retained. | | Sharpness, Frame pacing, Game speed, Profile, VRAM budget, with a preset selected or after Auto has chosen for the options file | Their defaults depend on those choices. The report does not run `apply_auto` on options the renderer is using. | Rows with a known default now state it when changed: - **Aspect ratio:** the monitor's shape, as shown in the example. - **Frame generation by:** plain Auto, not the name of the engine currently making frames. - **Game language:** the language saved in `language.txt`, otherwise the first language. Leaving that menu row normally saves the choice, so this difference rarely remains in a report. - On **Custom before Auto has ever chosen**, **Sharpness**, **Frame pacing**, **Game speed**, **Profile** and **VRAM budget** use their built-in defaults. They move to the PC-dependent list after Auto has chosen, or with a preset selected. - GPU Auto, the system-default microphone and other fixed defaults keep their existing behavior. A named microphone still appears only as “a microphone by its name”. ### Shorter links The link keeps the same field. It first includes known-default changes, PC rows and switch names, never switch values. If that will not fit, it tries these three shorter forms in order. These are actual decoded `field:settings` values printed by the report writer at `3078962e`. The authored fixture adds switches and then rows to reach each length limit; those padding rows are not new menu options. Switch names become a count; the setting rows stay: ```text Aspect ratio: 16:9 (default: 21:9 (monitor's)) Frame generation by: Built-in FG (default: Auto) Field of view: 110% (default: 100% (original)) Depends on this PC (the values now, not counted as changes): Quality preset: Custom (default depends on this PC) Output resolution: 2048x1152 (game's) (default depends on this PC) Upscaler: NVIDIA DLSS (default depends on this PC) Render resolution: Auto (dynamic) (default depends on this PC) Window scaling filter: Not needed (window = output) (default depends on this PC) Switches set: 81, listed in the attached report.txt. ``` PC rows become a count; the known-default changes stay: ```text Aspect ratio: 16:9 (default: 21:9 (monitor's)) Frame generation by: Built-in FG (default: Auto) Field of view: 110% (default: 100% (original)) Depends on this PC: 80, listed in the attached report.txt. Switches set: 81, listed in the attached report.txt. ``` Counts only: ```text 83 Insert menu options with fixed defaults changed, 80 whose defaults depend on this PC and 81 switches set, listed in the attached report.txt. ``` ## Flags and settings | Flag | What it does | Default now | Recommendation | Helps on | Doesn't help on | Untested on | |---|---|---|---|---|---|---| | Gameplay → Bug report | Same action; reports PC-dependent values without an invented default | Manual press, unchanged; no setting value or startup default changes | On by default: accurate report labels should need no opt-in | Display/upscaler/Auto-dependent diagnostic rows | Changing rendering settings or resetting them | Native Windows and other monitor configurations | ## Measurements <!-- branch-state --> Branch state: on main `fbd420241afdba1d5ba2936971eb48549edb85ae`; head `3078962ee62c268530a8e342c591768ea687c2d3`; whole game-free suite on this head: `515 passed, 0 failed, 15 skipped` (main `fbd420241afdba1d5ba2936971eb48549edb85ae`: `514 passed, 0 failed, 15 skipped`). The rebase onto this main and the last commit of the branch are HumanBean's; the suite was run on his head unchanged, on Arch Linux. Main has gained 10 commits since; the branch still merges cleanly. <!-- /branch-state --> **Review checks at `3078962e`:** both report checks and 57 other source-reading checks exited 0 on Linux. The defaults check printed `report defaults: 16 cases, 0 failed`. The recorder's Windows exit case and the Windows UTF-8 compile probe were skipped. This is targeted coverage, not the whole-suite result in the branch-state block. ### Earlier native runs The four runs below keep their original commits and results. Their separate PC section is historical, not the current report layout. <details> <summary>Rows for docs/tested-configurations.md, kept here instead of in the branch (see #45)</summary> ```markdown | Date | Build | OS / desktop | GPU / driver | CPU / RAM | Settings | Where | Result | |---|---|---|---|---|---|---|---| | 2026-11-12 | Longshot main d4a69456c (source 33703252) | Arch/Omarchy Linux 7.2.15-1-omarchy, windowless | RTX 4090, NVIDIA 615.78.08 | Core i9-13900K, 94 GB reported | Custom, 1920x1080, Game's own 50%, FG off, cap 60, precompile off, Aspect ratio Monitor's shape, Text speed Instant; ordinary shared load | Child Hyrule Field, Gameplay → Bug report; first interleaved run | 92.2s, exit 0. Report falsely says Monitor's shape has default 32:9. Exactly report.txt, log and picture; all 61 option values/labels unchanged. Text files omit the user name, the home folder and the host name. No fatal/device/kernel/core fault; complete journal. Not a speed measurement. | | 2026-11-12 | Longshot report-default fix 93462ee71 (source f0c7f0e0) | Arch/Omarchy Linux 7.2.15-1-omarchy, windowless | RTX 4090, NVIDIA 615.78.08 | Core i9-13900K, 94 GB reported | Same copied save and settings; second interleaved run | Same child Hyrule Field spot and Bug report row | 92.7s, exit 0. No invented 32:9 default; 13 PC-dependent rows in their own section. Real Text speed difference retained; all 61 option values/labels unchanged. Same three files, text privacy checks pass. No fatal/device/kernel/core fault; complete journal. Not a speed measurement. | | 2026-11-12 | Longshot main d4a69456c (source 33703252) | Arch/Omarchy Linux 7.2.15-1-omarchy, windowless | RTX 4090, NVIDIA 615.78.08 | Core i9-13900K, 94 GB reported | Same copied save and settings; third interleaved run | Same child Hyrule Field spot and Bug report row | 92.5s, exit 0. Wrong Monitor's shape/default 32:9 line reproduced again; same three files and unchanged 61 option values/labels. Text privacy checks pass; no fatal/device/kernel/core fault; complete journal. Ordinary shared load, no speed claim. | | 2026-11-12 | Longshot report-default fix 93462ee71 (source f0c7f0e0) | Arch/Omarchy Linux 7.2.15-1-omarchy, windowless | RTX 4090, NVIDIA 615.78.08 | Core i9-13900K, 94 GB reported | Same copied save and settings; fourth interleaved run | Same child Hyrule Field spot and Bug report row | 93.2s, exit 0. Wrong default absent again; 13 PC-dependent rows separate, real Text speed difference retained. Same three files and unchanged 61 values/labels; text privacy checks pass; no fatal/device/kernel/core fault; complete journal. Microphone list bounds changed during enumeration in all four runs, not its selected value. No speed claim. | ``` </details> Historical main: d4a69456c9de5131589af1fcf9a09adba4c403d1 (2026-11-12) Historical PR: 93462ee71d16f1f77284cf120efbaf737ef8f49e main: d4a69456c9de5131589af1fcf9a09adba4c403d1 (2026-11-12) this PR: fix/bug-report-dynamic-defaults @ 93462ee71d16f1f77284cf120efbaf737ef8f49e (the head the four game runs used; the current head is in the branch-state line above) Hardware: RTX 4090, NVIDIA 615.78.08; Core i9-13900K, 96 GB installed (the generated report says 94 GB); Arch/Omarchy Linux 7.2.15-1-omarchy Scene: same copied child Hyrule Field save, 1920x1080, Custom, Game's own upscaler 50%, frame generation off, cap 60, precompile off, Aspect ratio Monitor's shape, Text speed Instant; windowless Method: two interleaved pairs, main / branch / main / branch, 92.2–93.2 s each under shared load. Press the actual Bug report row once per run; inspect its generated text, file inventory, privacy and before/after menu values. Also run the real-source nine-context fixture twice each way in the same order. These are correctness observations, not timing benchmarks. How to reproduce: on the historical main, leave Aspect ratio on Monitor's shape, open Gameplay → Bug report and save once to see the wrong default. With the new code, a row at the monitor default is omitted; a different shape names the monitor's shape as its default. Fixed-default differences such as Text speed remain in Changes from the defaults. Run `python -I -B tests/test_bug_report_defaults.py`; `--source` selects another checkout for the same authored cases. | | main @ d4a69456 | this PR @ 93462ee7 | |---|---|---| | False Aspect ratio default lines, run1 / run2 | 1 / 1 | 0 / 0 | | Median false Aspect ratio default lines | 1 | 0 | | PC-dependent rows in the old separate section, run1 / run2 | 0 / 0 | 13 / 13 | | Selected option values/labels unchanged by report, both runs | 61 of61 | 61 of61 | | Native report files, each run | 3 | 3 | | Authored contexts failing, two interleaved checks | 7 / 7 of9 | 0 / 0 of9 | | Median authored failures | 7 | 0 | Historical main report excerpt: ```text ### Changes from the defaults Aspect ratio: Monitor's shape (default: 32:9) Text speed: Instant (default: Normal) ``` That is the wrong default from the historical main runs. The earlier fix put PC rows in their own section. HumanBean's commit replaces that layout with the single field shown above. The fixture's negative control uses d725b363 runtime source, identical to the measured main for this code, plus only the new authored test. Its repeated output is: ```text report defaults: 9 cases, 7 failed: aspect-default, reordered-shapes, output-context, render-context, upscaler-context, auto-profile, model-env ``` Both fixed invocations passed; the first also passed the full report-writer fixture. The stable GPU/microphone and fixed-model cases already pass on main and remain covered. | Order | Build | Wall time | Load start → end | Report result | |---|---|---:|---|---| |1|main d4a69456|92.2s|69.16 →34.88|Wrong32:9 default reproduced| |2|branch93462ee7|92.7s|67.03 →80.48|Current value separated; no guessed default| |3|main d4a69456|92.5s|135.05 →137.20|Wrong default reproduced again| |4|branch93462ee7|93.2s|68.47 →85.68|Correct separation again| All four exited0, with no fatal, device-loss, virtual-core escape or kernel fault and complete journal coverage. Each folder contains exactly report.txt, longshot_log.txt and picture.png. Both text files omit the user name, home path and host name. The microphone list's upper bound changed18→2 during enumeration in every run; its selected value and label did not change, so the61-value invariant deliberately does not claim identical dynamic device-list bounds. The older builds use matching release profiles. Their results keep the measured commits named above. They do not exercise HumanBean's new report layout or known-default changes. The source checks at `3078962e` cover those changes, not a new game run. ## Testing **Tested:** at HumanBean's `3078962e`, unchanged: `python -I -B tests/test_bug_report.py` passed. `python -I -B tests/test_bug_report_defaults.py` printed `report defaults: 16 cases, 0 failed`. Another 57 checks that read either changed C++ file exited 0 on Linux. The recorder's Windows exit case and the Windows UTF-8 compile probe were skipped, not passed. The whole-suite result and any failing test names are recorded once in the branch-state block under Measurements. These targeted checks are not a whole-suite result. **Earlier game evidence:** the four native Linux runs and the two real-source fixture pairs in Measurements keep their original commit names. They show the original wrong default and unchanged option values. They do not exercise the new report layout. **Built for Windows and Linux at the current head, not run:** At `3078962ee62c268530a8e342c591768ea687c2d3`, the Windows x86-64 Vulkan renderer DLL and all 44 selected host translation units were cross-built on Linux with llvm-mingw 20261104 (Clang 23.1.5), using the project's production flags and pinned dependencies (renderer: 35 newly compiled / 170 reused; host: 1 newly compiled / 43 reused; lifted C: 0 newly compiled / 80 reused; all Windows units: 36 newly compiled / 607 reused from matching object-cache entries), the renderer DLL linked and the full game executable linked; nothing was run on Windows or under Wine, and this is neither the project's hosted builder run nor a Windows UTF-8 run-time path test. The same head also compiled and linked its full native Linux executable and renderer with Clang 23.1.3 through the documented resume path (201 newly compiled units, 284 matching Linux-cache units, source stamp `c52a6bc9`), using the verified target-neutral C from the fresh fbd420241a lift rather than carrying the patch onto another commit; neither Linux binary was run. An earlier Windows link at `9291b06f`, before the last commit, gave the same result. **Not tested:** at `3078962e`: a full game build, the revised report in a running game, Windows gameplay or C++ fixture execution, other GPUs or real monitor shapes, physical microphone devices, real-window rendering, and a signed-in form submission. Authored tests cover the shape and engine cases, not those physical configurations. No rendering, default-setting or performance improvement is claimed. **Needs other testers:** - Windows or a different monitor shape: leave Aspect ratio at the monitor's shape and save a Bug report. It should not appear as a changed setting. Choose another shape and check that the report names the monitor's shape as the default. Genuine fixed-default changes must remain listed: test request T-VNZ6 ## Screenshots These pictures are from the older native runs at the commits in their captions. The report illustration shows the old separate-section layout, replaced by the single field above. ![Selected verbatim lines from the two generated native reports, clearly labelled as a text illustration rather than a menu screenshot: main @ d4a69456 invents 32:9; this PR @ 93462ee7 keeps the current value and admits the context-dependent default. Sources and JPG inspected.](https://git.magicbean.dev/attachments/98e5dd25-5c45-4fc6-82e4-1b17d41ef54e) ![Actual main @ d4a69456 and this PR @ 93462ee7 menus at the same saved location, both confirming a local report. The action and translated helper text remain unchanged; the corrected content is in the saved report. Sources and JPG inspected.](https://git.magicbean.dev/attachments/db1efdec-2e30-43ab-8998-478b25bec94c) ## Risk and rollback The report-only classification must stay aligned with rows that Auto or the display/engine controls. The test derives Auto-written members from the real setters and checks that they are included. A wrong classification would hide or misdescribe a diagnostic difference, not change the player's setting. Reverting this fix restores the old report format; no save or renderer interface migration is needed. ## Reviewer notes Start with `report_default_depends_on_pc`, `report_default` and `changed_rows_text` in `gpu/vk/vk_menu.cpp`, then the settings-field fallbacks in `gpu/vk/vk_bug_report.cpp`. HumanBean's `3078962e` supplies the review changes. Auto's choices are not recomputed while writing a report: calling `apply_auto` would alter the live options the renderer reads. The richer collector is a separate draft and is not part of this review. ## Issues See #44 ## Checklist ### For everyone - [x] One topic. - [x] The title is a Conventional Commit (`type(scope): description`). - [x] Rebased on current `main`. - [x] `python -I -B tests/run_all.py` run and passing (any failure that also fails on `main` is named above). - [x] Tested in the game, or what wasn't is named under Testing. - [x] Testing filled in: what was tested, what wasn't, and what needs other testers (with test requests). - [x] Measured: main and this PR at named commits, on the same PC, scene and settings, at least 2 interleaved runs each, the median reported. - [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. ### If applicable - [x] AI assistance recorded with `Assisted-by:` / `Co-Authored-By:` trailers. - [x] Flags noted, and the switch reference updated (docs/development.md, the README). - [x] Screenshots for a change that shows on screen. - [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. - [x] Host code builds on both Windows (llvm-mingw) and Linux, with UTF-8 paths. At `3078962e`: the Linux executable and renderer built and linked; the Windows renderer and game executable cross-built and linked on Linux (see Testing); neither was run. - [x] Docs updated. --- New on this forge; not opened on GitHub. Assisted-by: GPT-6 Astra Co-Authored-By: GPT-6 Astra <[email protected]>
mooki force-pushed fix/bug-report-dynamic-defaults from b3851373c0
All checks were successful
PR description / Description follows the template (pull_request_target) Successful in 2s
to 9291b06f6c
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 5s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
2026-11-13 14:17:29 +00:00
Compare
Author
Member

Carried onto main 0c74253d4 (head 9291b06f6) without its rows for the results table, which are kept in the description instead (see #45); nothing else changed. The whole game-free suite on this head: 506 passed, 2 failed, 15 skipped. Test request T-VNZ6 is open (Windows, or a monitor that is not 16:9). One box is still open, the Windows half of the two-platform build: a cross-build of this head for Windows is being prepared, so this stays WIP until that is settled.

Carried onto main `0c74253d4` (head `9291b06f6`) without its rows for the results table, which are kept in the description instead (see #45); nothing else changed. The whole game-free suite on this head: 506 passed, 2 failed, 15 skipped. Test request T-VNZ6 is open (Windows, or a monitor that is not 16:9). One box is still open, the Windows half of the two-platform build: a cross-build of this head for Windows is being prepared, so this stays WIP until that is settled.
Author
Member

The last open box is settled: at 9291b06f the renderer DLL and the full game executable were cross-built and linked for Windows on Linux with the project's llvm-mingw and its own flags (36 files newly compiled, 607 reused from matching objects); nothing was run on Windows, which test request T-VNZ6 asks for. Carried onto main 0edc13771 (head c139f9b4e) with the change unchanged; the whole game-free suite on this head: 510 passed, 2 failed, 15 skipped. Ready for review.

The last open box is settled: at `9291b06f` the renderer DLL and the full game executable were cross-built and linked for Windows on Linux with the project's llvm-mingw and its own flags (36 files newly compiled, 607 reused from matching objects); nothing was run on Windows, which test request T-VNZ6 asks for. Carried onto main `0edc13771` (head `c139f9b4e`) with the change unchanged; the whole game-free suite on this head: 510 passed, 2 failed, 15 skipped. Ready for review.
mooki force-pushed fix/bug-report-dynamic-defaults from 9291b06f6c
Some checks failed
PR description / Description follows the template (pull_request_target) Successful in 5s
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
Website / Website on Cloudflare (pull_request) Has been cancelled
to c139f9b4e5
Some checks failed
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Has been cancelled
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
2026-11-13 16:32:55 +00:00
Compare
mooki changed title from WIP: fix(menu): stop reporting guessed defaults in Bug reports to fix(menu): stop reporting guessed defaults in Bug reports 2026-11-13 16:32:56 +00:00
HumanBean added this to the v3.0.0 milestone 2026-11-13 17:53:22 +00:00
HumanBean left a comment

Thanks: the bug is real, and the report-writer tests are stronger than before. On main, tx_upscaler ignores its argument and tx_resolution/tx_filter/tx_framegen_by format the live state, so filed issues carry lines like Upscaler: AMD FSR 4 (default: AMD FSR 4) and Render resolution: Native (FSR 4, 2560x1440) (default: Native (FSR 4, 2560x1440)) (#226), and the Aspect ratio's Options{} index means nothing once outputs_sync has run. Other good parts:

  • The report doesn't change any option value, the microphone stays "a microphone by its name", and the new text adds no paths or names (only row names and the texts they already had).
  • test_bug_report.py now checks the new call, and nothing in it was weakened.
  • The new fixture takes Auto's rows from apply_auto itself, so when the Steam Deck profile (#225) lands, its opts.framegen = 2 will fail the check until Frame generation is classified too.

Must change

  1. The PC-dependent rows no longer reach the issue. field:settings and its two fallbacks (vk_bug_report.cpp:525) are built from in.options only. The issue form has no field called "Depends on this PC", yet report.txt tells players to copy each answer "into the fields of the same names". So for a report filed through the link, Upscaler, Render resolution, Output resolution, Aspect ratio, Quality preset, Window scaling filter, Sharpness, Frame pacing, the VRAM budget, Frame generation by and Game language all drop out of the issue. In #226 (an upscaler bug) and #112, those were most of the lines. The fix hides settings the player did change. Put the PC-dependent lines in the settings field too, for example under a Depends on this PC: line inside "Changes from the defaults", and in the link and its short forms (a count at least).
  2. "Rebased on current main" is no longer true. The base is 0edc1377. Main has since merged eight commits, among them #55, which changes tx_resolution in vk_menu.cpp (v = ur::snap_render(up, v)). The new fixture copies tx_resolution out of vk_menu.cpp. A trial merge has no conflicts, but neither check has run on the merged code. Rebase onto main, run the suite again and update the description.

Should change

  1. Several reclassified rows have a known default, and listing them as "default depends on this PC" erases real changes. The rule ignores the preset and whether the value changed, so all 13 rows are always listed with no default:

    • Frame generation by: the default is Auto. Only its text is live.
    • Game language: language.txt, else index 0. No reason for moving it is given.
    • Aspect ratio: the default is the monitor's shape (aspect_monitor_value()). Your own example prints Monitor's shape (default depends on this PC), which is at its default.
    • Frame pacing, Sharpness, Window scaling filter, handheld and real time on a Custom player whose options were never Auto's.

    Lines like Frame pacing: On (default: Off) and Frame generation by: Built-in FG (default: Auto) (from #112) were right. Now nobody can tell whether the player changed them. Where the default is known, report it. For the rows Auto writes, the real default on this PC is what auto_choice picks, which the menu already has (it is the source of the log's "Auto (this PC) would be" line). Compute it, for example with apply_auto on a saved and restored opts, or at least say "(set by Auto (this PC))" or "(changed)". "depends on this PC" alone doesn't say which.

  2. Description.

    • It still calls this "a separate WIP fix".
    • Testing names test_updater_asset_digest.py as the "sole failure", but Measurements names two others (test_display_pointer, test_texture_staging_publication).

Not checked: no build and no tests were run for this review (code read only), including whether the fixture compiles against main's tx_resolution after #55. #170 is already in the base, and LongshotLauncher PR 42 writes no settings section, so neither overlaps.

Thanks: the bug is real, and the report-writer tests are stronger than before. On main, `tx_upscaler` ignores its argument and `tx_resolution`/`tx_filter`/`tx_framegen_by` format the live state, so filed issues carry lines like `Upscaler: AMD FSR 4 (default: AMD FSR 4)` and `Render resolution: Native (FSR 4, 2560x1440) (default: Native (FSR 4, 2560x1440))` (https://git.magicbean.dev/MagicBeanDev/Longshot/issues/226), and the Aspect ratio's `Options{}` index means nothing once `outputs_sync` has run. Other good parts: - The report doesn't change any option value, the microphone stays "a microphone by its name", and the new text adds no paths or names (only row names and the texts they already had). - `test_bug_report.py` now checks the new call, and nothing in it was weakened. - The new fixture takes Auto's rows from `apply_auto` itself, so when the Steam Deck profile (https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/225) lands, its `opts.framegen = 2` will fail the check until Frame generation is classified too. ## Must change 1. **The PC-dependent rows no longer reach the issue.** `field:settings` and its two fallbacks (vk_bug_report.cpp:525) are built from `in.options` only. The issue form has no field called "Depends on this PC", yet report.txt tells players to copy each answer "into the fields of the same names". So for a report filed through the link, Upscaler, Render resolution, Output resolution, Aspect ratio, Quality preset, Window scaling filter, Sharpness, Frame pacing, the VRAM budget, Frame generation by and Game language all drop out of the issue. In https://git.magicbean.dev/MagicBeanDev/Longshot/issues/226 (an upscaler bug) and https://git.magicbean.dev/MagicBeanDev/Longshot/issues/112, those were most of the lines. The fix hides settings the player did change. Put the PC-dependent lines in the settings field too, for example under a `Depends on this PC:` line inside "Changes from the defaults", and in the link and its short forms (a count at least). 2. **"Rebased on current main" is no longer true.** The base is 0edc1377. Main has since merged eight commits, among them https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/55, which changes `tx_resolution` in vk_menu.cpp (`v = ur::snap_render(up, v)`). The new fixture copies `tx_resolution` out of vk_menu.cpp. A trial merge has no conflicts, but neither check has run on the merged code. Rebase onto main, run the suite again and update the description. ## Should change 3. **Several reclassified rows have a known default, and listing them as "default depends on this PC" erases real changes.** The rule ignores the preset and whether the value changed, so all 13 rows are always listed with no default: - Frame generation by: the default is Auto. Only its text is live. - Game language: language.txt, else index 0. No reason for moving it is given. - Aspect ratio: the default is the monitor's shape (`aspect_monitor_value()`). Your own example prints `Monitor's shape (default depends on this PC)`, which is at its default. - Frame pacing, Sharpness, Window scaling filter, handheld and real time on a Custom player whose options were never Auto's. Lines like `Frame pacing: On (default: Off)` and `Frame generation by: Built-in FG (default: Auto)` (from https://git.magicbean.dev/MagicBeanDev/Longshot/issues/112) were right. Now nobody can tell whether the player changed them. Where the default is known, report it. For the rows Auto writes, the real default on this PC is what `auto_choice` picks, which the menu already has (it is the source of the log's "Auto (this PC) would be" line). Compute it, for example with `apply_auto` on a saved and restored `opts`, or at least say "(set by Auto (this PC))" or "(changed)". "depends on this PC" alone doesn't say which. 4. **Description.** - It still calls this "a separate WIP fix". - Testing names `test_updater_asset_digest.py` as the "sole failure", but Measurements names two others (`test_display_pointer`, `test_texture_staging_publication`). Not checked: no build and no tests were run for this review (code read only), including whether the fixture compiles against main's `tx_resolution` after https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/55. https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/170 is already in the base, and LongshotLauncher PR 42 writes no settings section, so neither overlaps.
@ -521,4 +522,4 @@
const std::string named = sw.empty() ? "Switches set: none.\n" : "Switches set (their values are in the attached report.txt): " + names + ".\n";
const std::string listed = sw.empty() ? "" : "Switches set: " + count(sw) + ", listed in the attached report.txt.\n";
bool link_short = false;
for (const std::string& s : {options + named, options + listed,
Owner

The link fills field:settings from in.options only, and its count fallback leaves pc_options out too. The form has no field called "Depends on this PC", so a report filed through this link loses Upscaler, Render resolution, Output resolution, Aspect ratio, Sharpness, Frame pacing, the VRAM budget and the other rows the player changed. Please put these lines in the settings field as well (under a Depends on this PC: line), at least as a count in the short forms.

The link fills `field:settings` from `in.options` only, and its count fallback leaves `pc_options` out too. The form has no field called "Depends on this PC", so a report filed through this link loses Upscaler, Render resolution, Output resolution, Aspect ratio, Sharpness, Frame pacing, the VRAM budget and the other rows the player changed. Please put these lines in the settings field as well (under a `Depends on this PC:` line), at least as a count in the short forms.
@ -542,6 +544,7 @@ std::string write(const Inputs& in) {
text += "### Graphics card and driver version\n\n" + gpu + "\n\n";
text += "### CPU, RAM and operating system\n\n" + in.system + "\n\n";
text += "### Changes from the defaults\n\n" + settings + "\n";
if (!in.pc_options.empty()) text += "### Depends on this PC\n\n" + in.pc_options + "\n";
Owner

This heading isn't one of the issue form's fields, yet the report's first lines say to copy each answer "into the fields of the same names". It would be safer as a sub-list inside "Changes from the defaults", so it reaches the issue whichever way the player files it.

This heading isn't one of the issue form's fields, yet the report's first lines say to copy each answer "into the fields of the same names". It would be safer as a sub-list inside "Changes from the defaults", so it reaches the issue whichever way the player files it.
@ -2910,0 +2915,4 @@
return r.value == &opts.ultrawide || r.value == &opts.output_res || r.value == &opts.resolution ||
r.value == &opts.dlss || r.value == &opts.preset || r.value == &opts.upscale ||
r.value == &opts.sharpness || r.value == &opts.pace || r.value == &opts.realtime ||
r.value == &opts.handheld || r.value == &opts.vram_budget || r.value == &opts.framegen_by ||
Owner

framegen_by's default is Auto, a fixed value: only tx_framegen_by(0) formats the live state. Main's Frame generation by: Built-in FG (default: Auto) (issue 112) was right. Format the default as plain "Auto" and keep the row in the changes. pace, sharpness, upscale, realtime and handheld are also listed unconditionally, so a Custom player's real Frame pacing: On (default: Off) loses its default too.

`framegen_by`'s default is Auto, a fixed value: only `tx_framegen_by(0)` formats the live state. Main's `Frame generation by: Built-in FG (default: Auto)` (issue 112) was right. Format the default as plain "Auto" and keep the row in the changes. `pace`, `sharpness`, `upscale`, `realtime` and `handheld` are also listed unconditionally, so a Custom player's real `Frame pacing: On (default: Off)` loses its default too.
@ -2910,0 +2916,4 @@
r.value == &opts.dlss || r.value == &opts.preset || r.value == &opts.upscale ||
r.value == &opts.sharpness || r.value == &opts.pace || r.value == &opts.realtime ||
r.value == &opts.handheld || r.value == &opts.vram_budget || r.value == &opts.framegen_by ||
r.value == &opts.game_lang || (r.value == &opts.dlss_model && dlss_preset_env());
Owner

Why is Game language here? Its value is language.txt's, else index 0 (game_lang_index), which doesn't depend on the display, the upscaler or Auto.

Why is Game language here? Its value is language.txt's, else index 0 (`game_lang_index`), which doesn't depend on the display, the upscaler or Auto.
@ -2922,2 +2937,2 @@
out += std::string(r.name) + ": " + now + " (default: " + r.text(was) +
(restart_changed(r) ? "; takes effect after a restart" : "") + ")\n";
to += r.value == &opts.mic && *r.value >= 2 ? "a microphone by its name" : r.text(*r.value);
to += depends ? " (default depends on this PC" : " (default: ";
Owner

The default is often known: the monitor's shape for Aspect ratio (aspect_monitor_value()), and what auto_choice picks for the rows Auto writes. Saying "default depends on this PC" for every row, changed or not, means a reader can't tell what the player changed (your own example prints Aspect ratio: Monitor's shape (default depends on this PC), which is at its default). Report the real default where it is known, or at least say "set by Auto (this PC)" or "changed".

The default is often known: the monitor's shape for Aspect ratio (`aspect_monitor_value()`), and what `auto_choice` picks for the rows Auto writes. Saying "default depends on this PC" for every row, changed or not, means a reader can't tell what the player changed (your own example prints `Aspect ratio: Monitor's shape (default depends on this PC)`, which is at its default). Report the real default where it is known, or at least say "set by Auto (this PC)" or "changed".
HumanBean force-pushed fix/bug-report-dynamic-defaults from c139f9b4e5
Some checks failed
Website / Website on Cloudflare (pull_request) Blocked by required conditions
PR description / Description follows the template (pull_request_target) Has been cancelled
CI / Conventional Commit title (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been cancelled
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been cancelled
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been cancelled
CI / Workflow lint (actionlint) (pull_request) Has been cancelled
Third-party parts / As their notes say (pull_request) Has been cancelled
Third-party parts / The same as upstream (pull_request) Has been cancelled
to 3078962ee6
All checks were successful
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Third-party parts / The same as upstream (pull_request) Has been skipped
CI / Conventional Commit title (pull_request) Successful in 3s
CI / Workflow lint (actionlint) (pull_request) Successful in 5s
Third-party parts / As their notes say (pull_request) Successful in 6s
PR description / Description follows the template (pull_request_target) Successful in 3s
Website / Website on Cloudflare (pull_request_target) Has been skipped
2026-11-13 18:14:38 +00:00
Compare
Owner

I rebased your three commits onto main @ fbd42024 (no conflicts), which includes #55. Your test's cuts still match main's tx_resolution and the other functions. I addressed my review in one commit on top, pushed as 3078962e.

Nothing here was compiled or run. The C++ in vk_menu.cpp, vk_bug_report.cpp and the test fixture has only been read. Please run the suite at this head before relying on it.

My commit:

  1. The PC rows now reach the issue. The "Depends on this PC" rows now sit inside "Changes from the defaults" (the form's settings field), under their own line, after the fixed-default changes and before the switches. That's true both in report.txt and in the link. When the link is too long it falls back in order:

    • the changes, the PC rows and the switch names;
    • the same with a switch count instead of names;
    • the changes, then Depends on this PC: N, listed in the attached report.txt., then the switch count;
    • counts only.
  2. Known defaults are stated. A new report_default() gives:

    • Aspect ratio: the monitor's shape, aspect_of_choice(defaults.output_res, defaults.ultrawide);
    • Game language: i18n::game_lang_index();
    • Frame generation by: "Auto" by name.

    Sharpness, Frame pacing, Game speed, Profile and VRAM budget use the built-in defaults for a Custom player that Auto never chose for (auto_for == 0). These stay "default depends on this PC":

    • Output resolution, Upscaler, Render resolution and the Window scaling filter, whose texts read the live display or engine;
    • the Quality preset;
    • the DLSS model under RX_VK_DLSS_PRESET;
    • the preset rows once Auto has chosen.

    Auto's picks aren't computed as defaults: that would mean running apply_auto on the live options the renderer reads.

  3. Tests, only added to:

    • test_bug_report.py: the line and its order, the link's settings field carrying the rows, the count sentence, and a new long case where they don't fit.
    • test_bug_report_defaults.py: cases for a changed aspect ratio, Custom before and after Auto, Frame generation by, and the game language.

Please update the description:

  • the new layout (one field, not a section), with its example and the README quote;
  • the rows that still say "depends", and why;
  • drop "a separate WIP fix";
  • make Testing and Measurements agree on which tests fail;
  • post the suite at this head.

Since leaving the Game language row writes language.txt, that row will rarely show up in a report now.

I rebased your three commits onto main @ fbd42024 (no conflicts), which includes https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/55. Your test's cuts still match main's `tx_resolution` and the other functions. I addressed my review in one commit on top, pushed as 3078962e. **Nothing here was compiled or run.** The C++ in `vk_menu.cpp`, `vk_bug_report.cpp` and the test fixture has only been read. Please run the suite at this head before relying on it. **My commit:** 1. **The PC rows now reach the issue.** The "Depends on this PC" rows now sit inside "Changes from the defaults" (the form's `settings` field), under their own line, after the fixed-default changes and before the switches. That's true both in report.txt and in the link. When the link is too long it falls back in order: - the changes, the PC rows and the switch names; - the same with a switch count instead of names; - the changes, then `Depends on this PC: N, listed in the attached report.txt.`, then the switch count; - counts only. 2. **Known defaults are stated.** A new `report_default()` gives: - Aspect ratio: the monitor's shape, `aspect_of_choice(defaults.output_res, defaults.ultrawide)`; - Game language: `i18n::game_lang_index()`; - Frame generation by: "Auto" by name. Sharpness, Frame pacing, Game speed, Profile and VRAM budget use the built-in defaults for a Custom player that Auto never chose for (`auto_for == 0`). These stay "default depends on this PC": - Output resolution, Upscaler, Render resolution and the Window scaling filter, whose texts read the live display or engine; - the Quality preset; - the DLSS model under `RX_VK_DLSS_PRESET`; - the preset rows once Auto has chosen. Auto's picks aren't computed as defaults: that would mean running `apply_auto` on the live options the renderer reads. 3. **Tests, only added to:** - `test_bug_report.py`: the line and its order, the link's `settings` field carrying the rows, the count sentence, and a new long case where they don't fit. - `test_bug_report_defaults.py`: cases for a changed aspect ratio, Custom before and after Auto, Frame generation by, and the game language. **Please update the description:** - the new layout (one field, not a section), with its example and the README quote; - the rows that still say "depends", and why; - drop "a separate WIP fix"; - make Testing and Measurements agree on which tests fail; - post the suite at this head. Since leaving the Game language row writes language.txt, that row will rarely show up in a report now.
Author
Member

Thank you for the rebase and for 3078962e. I tested your commit unchanged. No follow-up code commit was needed.

On Linux at 3078962e:

  • python -I -B tests/test_bug_report.py: bug report: ok (report.txt as the issue form, both logs redacted, the picture, the long log, the terminal start, switch values, NUL bytes, the folder's reason).
  • python -I -B tests/test_bug_report_defaults.py: report defaults: 16 cases, 0 failed.
  • Another 57 checks reading the affected C++ files exited 0. The Windows-only recorder exit case and UTF-8 compile probe were skipped.
  • Whole game-free suite at 3078962e: 515 passed, 0 failed, 15 skipped (main fbd42024: 514 passed, 0 failed, 15 skipped).
  1. The PC rows now stay inside Changes from the defaults, after known-default changes and before switches, in both report.txt and the link. vk_bug_report.cpp retains the rows when switch names become a count, then counts PC rows, then uses counts only. The description includes output printed by the real formatter and report writer, including those shorter forms.
  2. Your rebase onto fbd420241a is kept, including #55. Both report tests compiled and ran against that code. The 16 defaults cases include the updated render-resolution formatter. The whole-suite result is above.
  3. report_default() now states the monitor's shape for Aspect ratio, the saved language for Game language, and plain Auto for Frame generation by. Sharpness, Frame pacing, Game speed, Profile and VRAM budget keep built-in defaults on Custom before Auto has chosen. After that, or with a preset, they remain PC-dependent. Output resolution, Upscaler, Render resolution, Window scaling filter and Quality preset still depend on context; DLSS model does too under its override. The description gives these reasons. I kept your choice not to call apply_auto on the live options.
  4. The description now calls this a report-only fix. It shows one field and quotes the README sentence it follows. The stale sole-failure claim is removed; Testing points to the same exact-head whole-suite result and failure list in Measurements. Older game and Windows-build evidence keeps its original commits and is not presented as a run of your new code.

Windows execution and a game run of the revised layout were not done in this check.

One box is unticked again on purpose: the Windows link and the Linux build in the description are from 9291b06f, and your commit changes two renderer files since. I am repeating the Windows link at 3078962e and tick the box when it has linked; the result goes in a comment here.

Thank you for the rebase and for `3078962e`. I tested your commit unchanged. No follow-up code commit was needed. On Linux at `3078962e`: - `python -I -B tests/test_bug_report.py`: `bug report: ok (report.txt as the issue form, both logs redacted, the picture, the long log, the terminal start, switch values, NUL bytes, the folder's reason)`. - `python -I -B tests/test_bug_report_defaults.py`: `report defaults: 16 cases, 0 failed`. - Another 57 checks reading the affected C++ files exited 0. The Windows-only recorder exit case and UTF-8 compile probe were skipped. - Whole game-free suite at `3078962e`: 515 passed, 0 failed, 15 skipped (main `fbd42024`: 514 passed, 0 failed, 15 skipped). 1. The PC rows now stay inside **Changes from the defaults**, after known-default changes and before switches, in both `report.txt` and the link. `vk_bug_report.cpp` retains the rows when switch names become a count, then counts PC rows, then uses counts only. The description includes output printed by the real formatter and report writer, including those shorter forms. 2. Your rebase onto `fbd420241a` is kept, including https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/55. Both report tests compiled and ran against that code. The 16 defaults cases include the updated render-resolution formatter. The whole-suite result is above. 3. `report_default()` now states the monitor's shape for Aspect ratio, the saved language for Game language, and plain Auto for Frame generation by. Sharpness, Frame pacing, Game speed, Profile and VRAM budget keep built-in defaults on Custom before Auto has chosen. After that, or with a preset, they remain PC-dependent. Output resolution, Upscaler, Render resolution, Window scaling filter and Quality preset still depend on context; DLSS model does too under its override. The description gives these reasons. I kept your choice not to call `apply_auto` on the live options. 4. The description now calls this a report-only fix. It shows one field and quotes the README sentence it follows. The stale sole-failure claim is removed; Testing points to the same exact-head whole-suite result and failure list in Measurements. Older game and Windows-build evidence keeps its original commits and is not presented as a run of your new code. Windows execution and a game run of the revised layout were not done in this check. One box is unticked again on purpose: the Windows link and the Linux build in the description are from `9291b06f`, and your commit changes two renderer files since. I am repeating the Windows link at `3078962e` and tick the box when it has linked; the result goes in a comment here.
Author
Member

A correction to my description update of a few minutes ago: it had dropped the two lines that name the measured commits and renamed two column heads and two Testing labels, so the description check failed on them. They are restored; nothing else changed. The check now reports one thing only, the Windows-and-Linux build box, which stays unticked until your commit has been linked for Windows, as said above.

A correction to my description update of a few minutes ago: it had dropped the two lines that name the measured commits and renamed two column heads and two Testing labels, so the description check failed on them. They are restored; nothing else changed. The check now reports one thing only, the Windows-and-Linux build box, which stays unticked until your commit has been linked for Windows, as said above.
Author
Member

Your commit builds on both platforms. At 3078962e the Windows renderer and the full game executable cross-built and linked on Linux with llvm-mingw 20261104 (Clang 23.1.5), and the native Linux executable and renderer built and linked with Clang 23.1.3; neither compiler rejected anything. Nothing was run from either build, and nothing ran on Windows. The Windows-and-Linux build box is ticked again with that scope, so the description check passes; the exact counts are under Testing.

Your commit builds on both platforms. At `3078962e` the Windows renderer and the full game executable cross-built and linked on Linux with llvm-mingw 20261104 (Clang 23.1.5), and the native Linux executable and renderer built and linked with Clang 23.1.3; neither compiler rejected anything. Nothing was run from either build, and nothing ran on Windows. The Windows-and-Linux build box is ticked again with that scope, so the description check passes; the exact counts are under Testing.
HumanBean left a comment

Thanks for testing 3078962e and for the rewritten description. All four points from my last review are addressed:

  1. The PC rows now reach the issue. They sit inside "Changes from the defaults" under their own line, in report.txt and in field:settings, and the three fallbacks keep the changes longest. test_bug_report.py covers each form.
  2. The PR was rebased onto fbd42024 and the suite was run at that head.
  3. Known defaults are stated again: Aspect ratio, Frame generation by, and the preset rows on a Custom player that Auto never chose for. The remaining "depends on this PC" rows each have a reason in the description.
  4. The description no longer calls this a "separate WIP fix", and Testing and Measurements now agree on the suite result.

I read the code against the surrounding menu code:

  • report_default agrees with outputs_sync for the Aspect ratio, because both call aspect_of_choice(…, 1) on choice 0.
  • auto_for and dlss_preset_env are declared before they are used.
  • The defaults fixture's auto-profile case collects every member that apply_auto/apply_preset write, so a new Auto-controlled row can't slip into the fixed list unnoticed.
  • No paths or names are added, and the report changes no option value.

Must change

  1. No game has run the code at the current head. All four game runs were at 93462ee7. 3078962e rewrote changed_rows_text, report_default and the settings field, and the new layout has only been exercised through the authored fixture. AGENTS.md asks for a game run before a PR is marked ready, and the PR otherwise stays WIP until someone has run it. On your RTX 4090 setup, one Bug report press at the current head is enough: paste its "Changes from the defaults" answer and the decoded field:settings. T-VNZ6 (Windows, other monitor shapes) can stay open alongside it.
  2. Rebase onto current main while you're at it. It is 10 commits past fbd42024, so "Rebased on current main" no longer holds. I checked those commits: none touches the report path. The only overlap is damage_taken = 1 in Options (#138), which the fixture's regexes handle. The game run in point 1 should then be at the rebased head.

Suggestion (not blocking)

  • Game language (inline): the PR now uses language.txt's value as the default. That matches the README's table, but the row writes language.txt when the player leaves it, so the language never appears in a report or an issue any more. On main, a German player's report said Game language: Deutsch (default: English (US)). The game log's oe: language line still has it, but only in the attachment.

Not checked: code read only. No build or test was run for this review. CI's Python checks run on workflow_dispatch only, so they were skipped on this PR's run 2009, as on every PR. The suite result is the author's own.

Thanks for testing `3078962e` and for the rewritten description. All four points from my last review are addressed: 1. The PC rows now reach the issue. They sit inside "Changes from the defaults" under their own line, in report.txt and in `field:settings`, and the three fallbacks keep the changes longest. `test_bug_report.py` covers each form. 2. The PR was rebased onto `fbd42024` and the suite was run at that head. 3. Known defaults are stated again: Aspect ratio, Frame generation by, and the preset rows on a Custom player that Auto never chose for. The remaining "depends on this PC" rows each have a reason in the description. 4. The description no longer calls this a "separate WIP fix", and Testing and Measurements now agree on the suite result. I read the code against the surrounding menu code: - `report_default` agrees with `outputs_sync` for the Aspect ratio, because both call `aspect_of_choice(…, 1)` on choice 0. - `auto_for` and `dlss_preset_env` are declared before they are used. - The defaults fixture's `auto-profile` case collects every member that `apply_auto`/`apply_preset` write, so a new Auto-controlled row can't slip into the fixed list unnoticed. - No paths or names are added, and the report changes no option value. ## Must change 1. **No game has run the code at the current head.** All four game runs were at `93462ee7`. `3078962e` rewrote `changed_rows_text`, `report_default` and the settings field, and the new layout has only been exercised through the authored fixture. AGENTS.md asks for a game run before a PR is marked ready, and the PR otherwise stays WIP until someone has run it. On your RTX 4090 setup, one Bug report press at the current head is enough: paste its "Changes from the defaults" answer and the decoded `field:settings`. T-VNZ6 (Windows, other monitor shapes) can stay open alongside it. 2. **Rebase onto current `main` while you're at it.** It is 10 commits past `fbd42024`, so "Rebased on current main" no longer holds. I checked those commits: none touches the report path. The only overlap is `damage_taken = 1` in `Options` (https://git.magicbean.dev/MagicBeanDev/Longshot/pulls/138), which the fixture's regexes handle. The game run in point 1 should then be at the rebased head. ## Suggestion (not blocking) - **Game language** (inline): the PR now uses language.txt's value as the default. That matches the README's table, but the row writes language.txt when the player leaves it, so the language never appears in a report or an issue any more. On `main`, a German player's report said `Game language: Deutsch (default: English (US))`. The game log's `oe: language` line still has it, but only in the attachment. Not checked: code read only. No build or test was run for this review. CI's Python checks run on `workflow_dispatch` only, so they were skipped on this PR's run 2009, as on every PR. The suite result is the author's own.
@ -2906,0 +2930,4 @@
static const Options defaults{};
*named = nullptr;
if (r.value == &opts.ultrawide) return aspect_of_choice(defaults.output_res, defaults.ultrawide);
if (r.value == &opts.game_lang) return i18n::game_lang_index();
Owner

language.txt starts as en-US (host/nn_applet.c writes it on the first start), and game_lang_tick rewrites it whenever the player leaves this row. Taking it as the default therefore means the row is always "at its default", so the game language no longer reaches the issue at all; on main, Game language: Deutsch (default: English (US)) did. This is consistent with the README table ("language.txt's, else English (US)"), so it isn't blocking. Still, the language is useful context for text and font bugs. Consider keeping the fixed default (index 0, English (US)), or always listing the row with its value (for example among the PC rows). The lang-saved case would then change with it.

language.txt starts as `en-US` (host/nn_applet.c writes it on the first start), and `game_lang_tick` rewrites it whenever the player leaves this row. Taking it as the default therefore means the row is always "at its default", so the game language no longer reaches the issue at all; on `main`, `Game language: Deutsch (default: English (US))` did. This is consistent with the README table ("`language.txt`'s, else English (US)"), so it isn't blocking. Still, the language is useful context for text and font bugs. Consider keeping the fixed default (index 0, English (US)), or always listing the row with its value (for example among the PC rows). The `lang-saved` case would then change with it.
HumanBean deleted branch fix/bug-report-dynamic-defaults 2026-11-14 01:01:54 +00:00
All checks were successful
CI / Python source checks (ubuntu-24.04, part 1/2) (pull_request) Has been skipped
CI / Python source checks (ubuntu-24.04, part 2/2) (pull_request) Has been skipped
CI / SPIR-V fixtures (global narrow load/store, direct surface atomics, F2I immediate) (pull_request) Has been skipped
Third-party parts / The same as upstream (pull_request) Has been skipped
CI / Conventional Commit title (pull_request) Successful in 3s
CI / Workflow lint (actionlint) (pull_request) Successful in 5s
Third-party parts / As their notes say (pull_request) Successful in 6s
PR description / Description follows the template (pull_request_target) Successful in 3s
Website / Website on Cloudflare (pull_request_target) Has been skipped
Some workflows are waiting to be reviewed.
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
MagicBeanDev/Longshot!103
No description provided.