MagicBeanDev/Hookshot
Archived
Watch
2
0
Fork
You've already forked Hookshot
 0
Code Issues 125 Pull requests 205 Activity

fix: switches that behave as players expect, and dead ones removed #2042

Merged
hckrman101 merged 21 commits from knobs-cleanup into main 2026-11-11 16:33:01 +00:00
Conversation 7 Commits 21 Files changed 136 +1860 -975
hckrman101 commented 2026-11-11 00:50:58 +00:00 (Migrated from github.com)

Several switches did things a player would not expect: a test run's RX_VK_PACE=1 stayed in hookshot_options.ini for good, a line in hookshot_env.txt beat a switch set for one launch, and switches that change the shader translation did nothing on a PC that already had a program cache. This PR fixes those, removes switches left over from mkw-recomp and TOTK, makes the AMD ASTC workaround a menu row, and makes =0 mean off for every on/off switch. The V-Sync row is #2041.

It is a series of commits, one per part, in this order.

1. Bug fixes

Environment overrides last for their run. RX_VK_FRAMEGEN_MULT, RX_VK_PACE, RX_VK_UPSCALE, HOS_REALTIME_STEP, HOS_DYNRES and HOS_OUTPUT_RES were copied into their rows in menu_init, and the next change of any row saved them. They now apply to the run only: save() writes the player's own value until the player changes that row (env_override, saved_value in vk_menu.cpp; a custom output size of the player's is kept too). tests/test_menu_env_overrides.py compiles the real save() and the overrides' part of menu_init.

Translation switches are in the program cache's stamp. RX_VK_NO_CBUF_LOAD_RULE, RX_VK_PTR_SPLIT_FALLBACK, RX_VK_ALL_GLOBAL_PROGS, RX_VK_NO_EXTREGS, RX_VK_NO_DRAW_PARAMS, and the miss log's RX_VK_MISS_LOG and RX_VK_BLOCK_PROBE, each change what the translator makes (checked at each read) but were not in disk_stamp's list. tests/test_program_stamp_knobs.py reads every RX_VK_ switch the translator reads and fails when one is missing (two that only log are listed as such). Nothing changes for a run that sets none of them.

Rows no preset sets keep the Quality preset. Changing Reflex, Anti-Lag 2, Output resolution, Anisotropic filtering, Ultrawide, Ultrawide menus, Robust buffers, Mesh shaders, Text speed, Text size, Controller in background, Layout preset, Descriptor reuse or Sampler cache turned the preset to Custom. They are preset_exempt now; tests/test_preset_exempt.py derives the rule from apply_preset. The Upscaler stays as it was, because changing it moves Render resolution to the upscaler's own steps.

The environment goes over hookshot_env.txt. The file used setenv(..., 1), so a switch set for one launch (HOS_FULLSCREEN=0 ./hookshot, a launcher's or a script's own) lost to the saved line. A name already set when the game starts now keeps its value, and the log says the file's line was left out; within the file a later line still replaces an earlier one. tests/test_env_file.py covers it. The readme and the development guide now give the order: the environment, then hookshot_env.txt, then the menu's saved rows.

2. Dead switches removed, duplicates merged

Each removal was checked against the code, tests, scripts, workflows, the builders and the docs first.

  • RX_BACKEND=eden: it loaded mkw_eden.dll, which nothing builds, then fell back to WebGPU. gpu/vk/VULKAN.md and vk_api.h now name hookshot_vk.dll.
  • HOS_FIX_RES and HOS_SCENE_NATIVE, with nx2/oot_native_resolution.py, the lift patch they switched (off since #849). The lift changes, so the next build lifts again.
  • RX_VK_FOLLOW_MESH ("tried for the foliage, which it did not bring back").
  • TOTK_MINIMAL_LAYOUT, and TOTK_ in the names hookshot_env.txt accepts.
  • RX_VK_OOT_VISUAL, RX_VK_OOT_FOG, RX_VK_OOT_EXPOSURE and the Orchid fog and exposure workaround (off since 11-07; its factors were tuned against bugs since fixed). The NaN guard that shared its header is now in ir_opt/fragment_output_guard.h. The readme's credits say what Orchid's workaround was used for, and the license line no longer lists Orchid's code.
  • nx2/build.py --lib, --only, --opt and --no-polls: nothing passes them. --dll stays (nx2/test_dll.py and nx2/dllmap.py use it).
  • Merged: RX_VK_DESC_SLOTGEN, RX_VK_NO_DESC_REUSE and RX_VK_DESC_REUSE_DGEN are now one switch, RX_VK_DESC_REUSE=0/1/never (the row's values: 0 is its Off, the global check; 1 is its On, by slot, the default; never turns reuse off), which goes over the Descriptor reuse row. The DGEN switch only turned off issue 1126's fix and is gone.
  • Merged: RX_VK_NAN_GUARD=0 turns off both NaN guards; RX_VK_NO_NAN_GUARD is gone.
  • nx2/bake_ds's copy of the stamp's list matches the live one again, and the stamp check now compares them.
  • Kept, documented side by side in the development guide: RX_VK_FRAMELOG (pictures) and RX_VK_FRAME_LOG (frame times), RX_VK_PRESENT_AHEAD (frame generation's copies) and RX_VK_PRESENTS_AHEAD (the replay's queue depth). Renaming them would break scripts that use them for little gain.

Looked at and left in place:

  • HOS_FPS_MAX is not a duplicate of the Game frame rate cap row: it takes any rate (75, 100, 180 Hz screens) where the row has steps, and tests/test_frame_rate_cap_step.py drives the cap through it. Its comment no longer calls it a test.
  • RX_JOBS is no longer in make_game.py; the Linux builder still honours it deliberately (tests/test_linux_builder_jobs.py), and third_party/a64-kit reads it.
  • RX_VK_SPARSE_IDLE turns on the stricter ordering of sparse binds, which was left off for MKW's sake. Whether OoT should have it on is a measurement, not a removal.

3. Menu rows

The Debug tab's Developer section, with Robust buffers and then ASTC as RGBA8 selected (headless capture, 1600x900, RTX 3090, Linux):

Robust buffers in the Developer section
ASTC as RGBA8 in the Developer section

  • Debug, Developer, ASTC as RGBA8 (after a restart): the AMD texture workaround the readme asked players to type into hookshot_env.txt (#57, #1006). Its help says what it is for and that it takes far more video memory. RX_VK_ASTC_RGBA8 still goes over it for a run. It does not turn itself on for any GPU: #993's report says it took effect on a 9070 XT and did not help, so there is no clear set of cards to turn it on for.
  • Robust buffers moves from Graphics to the Developer section. The Debug tab now draws a Developer row with more than two values as a choice with arrows.
  • The readme's menu table lists the Debug tab and the new row, and its AMD section points at the row.
  • #1946 adds its Shader compile row just above Robust buffers in the Graphics list. Whichever merges second has a one-line conflict there and in the readme's table.

4. One rule for on/off switches

About 230 switches read as on whenever they were set, so RX_VK_NO_PROGRAM_CACHE=0 or HOS_HEADLESS=0 turned the feature on. env_on(name) (gpu/vk/compat/hookshot_env.h, for both host/ and gpu/vk) is off when the variable is unset, empty, 0, false, off or no, and on otherwise. Every check that only asked whether a switch was set now uses it; =1 works as before.

  • 230 switches in 39 files under host/ and gpu/vk (the translator's own switches included) now use it. A switch that some other place reads for a value is left exactly as it was: 25 of them, such as RX_VK_WARP64=0, HOS_FULLSCREEN=0, the lists and the sizes, where 0 already means something.
  • nx2/kit.py has the same rule as env_on() for the build's RX_VK_ONLY. nx2/build.py counts the header in the runtime's object keys, as it does dlss_math.h. The AppImage now packs headers host/ includes from below gpu/vk/.
  • The development guide says what "set" means now.
  • tests/test_env_on.py checks the rule in C, C++ and Python, and fails if a switch is read the old way again. Checks that compile code cut out of the changed files now force-include the header (their getenv stand-ins pass env_on through them). The checks that look for a switch's read by its text now look for env_on. test_display_startup_failure's HOS_HEADLESS=0 case and test_program_answer_domains' RX_VK_REGISTER_AT_ONCE=0 case now expect off.
  • RX_VK_FIFO and RX_VK_IMMEDIATE keep their old reads here because #2041 (the V-Sync row) replaces them.
  • Nothing in tests/, scripts/, .github/, the builders or the docs set any of the converted switches to 0 or empty to turn it on (checked by grep).

This part touches many files that other PRs also edit (vk_draw.cpp above all). Most of it is a mechanical rewrite, done by a script that only rewrites presence checks whose value is not read on the same line, so on a conflict it is simplest to drop this commit and run the rewrite again on the new base.

Testing

  • python -I -B tests/run_all.py on the branch, rebased on main 667ba79: 342 passed, 13 skipped. The three failures are the same as on main: nx2/test_nup_read_name.py, tests/test_builder_log_read_only.py and tests/test_wayland_libdecor_hint.py, all failing on this machine without these changes. New checks: test_menu_env_overrides, test_program_stamp_knobs, test_preset_exempt, test_env_on; extended: test_env_file, test_menu_keys.
  • A full Linux build of the branch before the rebase (game and hookshot_vk.so, its exported-symbol check clean, the shaders prepared), and after it a build of hookshot_vk.so (exported-symbol check clean); a headless run with the Insert menu drawn into the shots: the Debug tab's Developer rows show and change. The log has rx: renderer: Vulkan and robust buffer access on (Auto), as before.
  • Not tested in a run: ASTC as RGBA8 turned on (the log should then lack the BC3 line), an AMD card, Windows.

For the maintainer to decide (not changed here)

  • The WebGPU fallback renderer (gpu/render, about 226 switches). It is reached only when hookshot_vk fails to load or start, and the builders still ship wgpu-native. It could be dropped, or frozen with its switches cut to a handful.
  • The A/B revert switches (49 switches whose own comment says "A/B" or "as before", for defaults that shipped and held: RX_VK_NO_PARTIAL, RX_VK_NO_STATE_LIST, RX_VK_PUSH_ALL, RX_VK_PUSH_STATE_OLD, RX_VK_UB_CUR_ONLY, RX_VK_POOL_END_IMPORTED, RX_VK_MIRROR_KEEP, RX_VK_READBACK_NO_CATCHUP, RX_VK_FORCE_LINEAR, RX_VK_UPLOAD_ALL, RX_VK_NO_TABLE_SHRINK and others). One policy that would work: an A/B switch lives for one release after its default flips, then goes.
Several switches did things a player would not expect: a test run's `RX_VK_PACE=1` stayed in `hookshot_options.ini` for good, a line in `hookshot_env.txt` beat a switch set for one launch, and switches that change the shader translation did nothing on a PC that already had a program cache. This PR fixes those, removes switches left over from mkw-recomp and TOTK, makes the AMD ASTC workaround a menu row, and makes `=0` mean off for every on/off switch. The V-Sync row is #2041. It is a series of commits, one per part, in this order. ## 1. Bug fixes **Environment overrides last for their run.** `RX_VK_FRAMEGEN_MULT`, `RX_VK_PACE`, `RX_VK_UPSCALE`, `HOS_REALTIME_STEP`, `HOS_DYNRES` and `HOS_OUTPUT_RES` were copied into their rows in `menu_init`, and the next change of any row saved them. They now apply to the run only: `save()` writes the player's own value until the player changes that row (`env_override`, `saved_value` in `vk_menu.cpp`; a custom output size of the player's is kept too). `tests/test_menu_env_overrides.py` compiles the real `save()` and the overrides' part of `menu_init`. **Translation switches are in the program cache's stamp.** `RX_VK_NO_CBUF_LOAD_RULE`, `RX_VK_PTR_SPLIT_FALLBACK`, `RX_VK_ALL_GLOBAL_PROGS`, `RX_VK_NO_EXTREGS`, `RX_VK_NO_DRAW_PARAMS`, and the miss log's `RX_VK_MISS_LOG` and `RX_VK_BLOCK_PROBE`, each change what the translator makes (checked at each read) but were not in `disk_stamp`'s list. `tests/test_program_stamp_knobs.py` reads every `RX_VK_` switch the translator reads and fails when one is missing (two that only log are listed as such). Nothing changes for a run that sets none of them. **Rows no preset sets keep the Quality preset.** Changing Reflex, Anti-Lag 2, Output resolution, Anisotropic filtering, Ultrawide, Ultrawide menus, Robust buffers, Mesh shaders, Text speed, Text size, Controller in background, Layout preset, Descriptor reuse or Sampler cache turned the preset to Custom. They are `preset_exempt` now; `tests/test_preset_exempt.py` derives the rule from `apply_preset`. The Upscaler stays as it was, because changing it moves Render resolution to the upscaler's own steps. **The environment goes over `hookshot_env.txt`.** The file used `setenv(..., 1)`, so a switch set for one launch (`HOS_FULLSCREEN=0 ./hookshot`, a launcher's or a script's own) lost to the saved line. A name already set when the game starts now keeps its value, and the log says the file's line was left out; within the file a later line still replaces an earlier one. `tests/test_env_file.py` covers it. The readme and the development guide now give the order: the environment, then `hookshot_env.txt`, then the menu's saved rows. ## 2. Dead switches removed, duplicates merged Each removal was checked against the code, tests, scripts, workflows, the builders and the docs first. - `RX_BACKEND=eden`: it loaded `mkw_eden.dll`, which nothing builds, then fell back to WebGPU. `gpu/vk/VULKAN.md` and `vk_api.h` now name `hookshot_vk.dll`. - `HOS_FIX_RES` and `HOS_SCENE_NATIVE`, with `nx2/oot_native_resolution.py`, the lift patch they switched (off since #849). The lift changes, so the next build lifts again. - `RX_VK_FOLLOW_MESH` ("tried for the foliage, which it did not bring back"). - `TOTK_MINIMAL_LAYOUT`, and `TOTK_` in the names `hookshot_env.txt` accepts. - `RX_VK_OOT_VISUAL`, `RX_VK_OOT_FOG`, `RX_VK_OOT_EXPOSURE` and the Orchid fog and exposure workaround (off since 11-07; its factors were tuned against bugs since fixed). The NaN guard that shared its header is now in `ir_opt/fragment_output_guard.h`. The readme's credits say what Orchid's workaround was used for, and the license line no longer lists Orchid's code. - `nx2/build.py --lib`, `--only`, `--opt` and `--no-polls`: nothing passes them. `--dll` stays (`nx2/test_dll.py` and `nx2/dllmap.py` use it). - Merged: `RX_VK_DESC_SLOTGEN`, `RX_VK_NO_DESC_REUSE` and `RX_VK_DESC_REUSE_DGEN` are now one switch, `RX_VK_DESC_REUSE=0/1/never` (the row's values: `0` is its Off, the global check; `1` is its On, by slot, the default; `never` turns reuse off), which goes over the Descriptor reuse row. The DGEN switch only turned off issue 1126's fix and is gone. - Merged: `RX_VK_NAN_GUARD=0` turns off both NaN guards; `RX_VK_NO_NAN_GUARD` is gone. - `nx2/bake_ds`'s copy of the stamp's list matches the live one again, and the stamp check now compares them. - Kept, documented side by side in the development guide: `RX_VK_FRAMELOG` (pictures) and `RX_VK_FRAME_LOG` (frame times), `RX_VK_PRESENT_AHEAD` (frame generation's copies) and `RX_VK_PRESENTS_AHEAD` (the replay's queue depth). Renaming them would break scripts that use them for little gain. Looked at and left in place: - `HOS_FPS_MAX` is not a duplicate of the Game frame rate cap row: it takes any rate (75, 100, 180 Hz screens) where the row has steps, and `tests/test_frame_rate_cap_step.py` drives the cap through it. Its comment no longer calls it a test. - `RX_JOBS` is no longer in `make_game.py`; the Linux builder still honours it deliberately (`tests/test_linux_builder_jobs.py`), and `third_party/a64-kit` reads it. - `RX_VK_SPARSE_IDLE` turns on the stricter ordering of sparse binds, which was left off for MKW's sake. Whether OoT should have it on is a measurement, not a removal. ## 3. Menu rows The Debug tab's Developer section, with Robust buffers and then ASTC as RGBA8 selected (headless capture, 1600x900, RTX 3090, Linux): ![Robust buffers in the Developer section](https://github.com/hckrman101/Hookshot/raw/pr-assets/knobs/debug-robust-buffers.png) ![ASTC as RGBA8 in the Developer section](https://github.com/hckrman101/Hookshot/raw/pr-assets/knobs/debug-astc-rgba8.png) - **Debug, Developer, ASTC as RGBA8** (after a restart): the AMD texture workaround the readme asked players to type into `hookshot_env.txt` (#57, #1006). Its help says what it is for and that it takes far more video memory. `RX_VK_ASTC_RGBA8` still goes over it for a run. It does not turn itself on for any GPU: #993's report says it took effect on a 9070 XT and did not help, so there is no clear set of cards to turn it on for. - **Robust buffers** moves from Graphics to the Developer section. The Debug tab now draws a Developer row with more than two values as a choice with arrows. - The readme's menu table lists the Debug tab and the new row, and its AMD section points at the row. - #1946 adds its Shader compile row just above Robust buffers in the Graphics list. Whichever merges second has a one-line conflict there and in the readme's table. ## 4. One rule for on/off switches About 230 switches read as on whenever they were set, so `RX_VK_NO_PROGRAM_CACHE=0` or `HOS_HEADLESS=0` turned the feature on. `env_on(name)` (`gpu/vk/compat/hookshot_env.h`, for both `host/` and `gpu/vk`) is off when the variable is unset, empty, `0`, `false`, `off` or `no`, and on otherwise. Every check that only asked whether a switch was set now uses it; `=1` works as before. - 230 switches in 39 files under `host/` and `gpu/vk` (the translator's own switches included) now use it. A switch that some other place reads for a value is left exactly as it was: 25 of them, such as `RX_VK_WARP64=0`, `HOS_FULLSCREEN=0`, the lists and the sizes, where `0` already means something. - `nx2/kit.py` has the same rule as `env_on()` for the build's `RX_VK_ONLY`. `nx2/build.py` counts the header in the runtime's object keys, as it does `dlss_math.h`. The AppImage now packs headers `host/` includes from below `gpu/vk/`. - The development guide says what "set" means now. - `tests/test_env_on.py` checks the rule in C, C++ and Python, and fails if a switch is read the old way again. Checks that compile code cut out of the changed files now force-include the header (their `getenv` stand-ins pass `env_on` through them). The checks that look for a switch's read by its text now look for `env_on`. `test_display_startup_failure`'s `HOS_HEADLESS=0` case and `test_program_answer_domains`' `RX_VK_REGISTER_AT_ONCE=0` case now expect off. - `RX_VK_FIFO` and `RX_VK_IMMEDIATE` keep their old reads here because #2041 (the V-Sync row) replaces them. - Nothing in `tests/`, `scripts/`, `.github/`, the builders or the docs set any of the converted switches to `0` or empty to turn it on (checked by grep). This part touches many files that other PRs also edit (`vk_draw.cpp` above all). Most of it is a mechanical rewrite, done by a script that only rewrites presence checks whose value is not read on the same line, so on a conflict it is simplest to drop this commit and run the rewrite again on the new base. ## Testing - `python -I -B tests/run_all.py` on the branch, rebased on `main` `667ba79`: 342 passed, 13 skipped. The three failures are the same as on `main`: `nx2/test_nup_read_name.py`, `tests/test_builder_log_read_only.py` and `tests/test_wayland_libdecor_hint.py`, all failing on this machine without these changes. New checks: `test_menu_env_overrides`, `test_program_stamp_knobs`, `test_preset_exempt`, `test_env_on`; extended: `test_env_file`, `test_menu_keys`. - A full Linux build of the branch before the rebase (game and `hookshot_vk.so`, its exported-symbol check clean, the shaders prepared), and after it a build of `hookshot_vk.so` (exported-symbol check clean); a headless run with the Insert menu drawn into the shots: the Debug tab's Developer rows show and change. The log has `rx: renderer: Vulkan` and robust buffer access on (Auto), as before. - Not tested in a run: ASTC as RGBA8 turned on (the log should then lack the BC3 line), an AMD card, Windows. ## For the maintainer to decide (not changed here) - **The WebGPU fallback renderer** (`gpu/render`, about 226 switches). It is reached only when `hookshot_vk` fails to load or start, and the builders still ship `wgpu-native`. It could be dropped, or frozen with its switches cut to a handful. - **The A/B revert switches** (49 switches whose own comment says "A/B" or "as before", for defaults that shipped and held: `RX_VK_NO_PARTIAL`, `RX_VK_NO_STATE_LIST`, `RX_VK_PUSH_ALL`, `RX_VK_PUSH_STATE_OLD`, `RX_VK_UB_CUR_ONLY`, `RX_VK_POOL_END_IMPORTED`, `RX_VK_MIRROR_KEEP`, `RX_VK_READBACK_NO_CATCHUP`, `RX_VK_FORCE_LINEAR`, `RX_VK_UPLOAD_ALL`, `RX_VK_NO_TABLE_SHRINK` and others). One policy that would work: an A/B switch lives for one release after its default flips, then goes.
nullifyshot commented 2026-11-11 01:30:24 +00:00 (Migrated from github.com)

I read through this one and found nothing that blocks it. Two things need fixing before it can merge, plus a few small ones. I only read the code and haven't run the tests locally. CI hasn't run them either, since GitHub skips the test workflows while the PR has conflicts.

Before merge

  1. Conflicts with main in gpu/vk/vk_menu.cpp and gpu/vk/vk_menu.h. All of them are small:

    • #1971's Mesh shaders row sits right next to the moved Robust buffers row
    • #2000 renamed ur::snap to ur::snap_render
    • the descriptor_reuse comment changed on both sides

    After the rebase, Mesh shaders probably needs preset_exempt as well (apply_preset doesn't set it). test_preset_exempt should flag it.

  2. Restart on Windows ignores changes to hookshot_env.txt. relaunch() in host/hos_relaunch.c passes NULL for the environment to CreateProcessW. The new process therefore inherits this one's environment, which already holds every value _putenv set from the file. With the environment now going first, the restarted game skips each line of the file, so edits made while the game runs are lost. The log also reports every line as "set in the environment, which goes first". Linux doesn't have this problem, because it execs with /proc/self/environ, the environment the game started with. A fix: save GetEnvironmentStringsW() in main before load_hookshot_env(), then pass that block, with HOS_RESTART_OF added and CREATE_UNICODE_ENVIRONMENT.

Minor

  • host/posix/display_sdl.c: SDL_VIDEODRIVER and SDL_VIDEO_DRIVER are now read with env_on. They're SDL's own variables and hold a driver name, not an on/off value, so getenv fits them better. These are the only two converted names outside RX_, HOS_, HOOKSHOT_, EDEN_ and NVN_.
  • Removed switches are now ignored silently. Older issues tell players to use RX_VK_NO_DESC_REUSE=1 or RX_VK_NO_NAN_GUARD=1. If those are in someone's hookshot_env.txt, they now do nothing, and nothing says so. A log line that names the replacement (RX_VK_DESC_REUSE=0, RX_VK_NAN_GUARD=0) would help bug reports.
  • load_hookshot_env tracks at most 64 names, each under 96 characters. Past that, a later line for an untracked name is skipped as if it came from the environment. Harmless in practice, but worth a comment or a higher limit.

Checked and looks right

  • The on/off rewrite. I checked that every name now read with env_on isn't read for its value anywhere else. I also checked that nothing in scripts/, builder/, .github/ or the tests sets one to 0 to turn it on, apart from the RX_VK_REGISTER_AT_ONCE=0 case this PR updates.
  • env_override and saved_value: a run's override stays out of hookshot_options.ini until the player changes the row, and a custom Output resolution size is kept.
  • The new stamp names only change the hash when they're set, so existing program caches survive.
  • RX_VK_DESC_REUSE levels match the old switches: row Off is 1, row On is 2, and 0 is the old RX_VK_NO_DESC_REUSE. The removed RX_VK_OOT_*, HOS_FIX_RES and HOS_SCENE_NATIVE were already off by default.

Suggestion: part 4 (the env_on rule) carries most of the risk, and the description already says it may need redoing after conflicts. It could go in a PR of its own, so the bug fixes in parts 1 to 3 don't wait on it.

I read through this one and found nothing that blocks it. Two things need fixing before it can merge, plus a few small ones. I only read the code and haven't run the tests locally. CI hasn't run them either, since GitHub skips the test workflows while the PR has conflicts. **Before merge** 1. **Conflicts with main** in `gpu/vk/vk_menu.cpp` and `gpu/vk/vk_menu.h`. All of them are small: - #1971's Mesh shaders row sits right next to the moved Robust buffers row - #2000 renamed `ur::snap` to `ur::snap_render` - the `descriptor_reuse` comment changed on both sides After the rebase, Mesh shaders probably needs `preset_exempt` as well (`apply_preset` doesn't set it). `test_preset_exempt` should flag it. 2. **Restart on Windows ignores changes to `hookshot_env.txt`.** `relaunch()` in `host/hos_relaunch.c` passes `NULL` for the environment to `CreateProcessW`. The new process therefore inherits this one's environment, which already holds every value `_putenv` set from the file. With the environment now going first, the restarted game skips each line of the file, so edits made while the game runs are lost. The log also reports every line as "set in the environment, which goes first". Linux doesn't have this problem, because it execs with `/proc/self/environ`, the environment the game started with. A fix: save `GetEnvironmentStringsW()` in `main` before `load_hookshot_env()`, then pass that block, with `HOS_RESTART_OF` added and `CREATE_UNICODE_ENVIRONMENT`. **Minor** - `host/posix/display_sdl.c`: `SDL_VIDEODRIVER` and `SDL_VIDEO_DRIVER` are now read with `env_on`. They're SDL's own variables and hold a driver name, not an on/off value, so `getenv` fits them better. These are the only two converted names outside `RX_`, `HOS_`, `HOOKSHOT_`, `EDEN_` and `NVN_`. - Removed switches are now ignored silently. Older issues tell players to use `RX_VK_NO_DESC_REUSE=1` or `RX_VK_NO_NAN_GUARD=1`. If those are in someone's `hookshot_env.txt`, they now do nothing, and nothing says so. A log line that names the replacement (`RX_VK_DESC_REUSE=0`, `RX_VK_NAN_GUARD=0`) would help bug reports. - `load_hookshot_env` tracks at most 64 names, each under 96 characters. Past that, a later line for an untracked name is skipped as if it came from the environment. Harmless in practice, but worth a comment or a higher limit. **Checked and looks right** - The on/off rewrite. I checked that every name now read with `env_on` isn't read for its value anywhere else. I also checked that nothing in `scripts/`, `builder/`, `.github/` or the tests sets one to `0` to turn it on, apart from the `RX_VK_REGISTER_AT_ONCE=0` case this PR updates. - `env_override` and `saved_value`: a run's override stays out of `hookshot_options.ini` until the player changes the row, and a custom Output resolution size is kept. - The new stamp names only change the hash when they're set, so existing program caches survive. - `RX_VK_DESC_REUSE` levels match the old switches: row Off is 1, row On is 2, and 0 is the old `RX_VK_NO_DESC_REUSE`. The removed `RX_VK_OOT_*`, `HOS_FIX_RES` and `HOS_SCENE_NATIVE` were already off by default. **Suggestion:** part 4 (the `env_on` rule) carries most of the risk, and the description already says it may need redoing after conflicts. It could go in a PR of its own, so the bug fixes in parts 1 to 3 don't wait on it.
hckrman101 (Migrated from github.com) reviewed 2026-11-11 01:36:24 +00:00
hckrman101 (Migrated from github.com) left a comment

I went through all six commits against the description. Most of it holds up. The removed switches really are gone: nothing in head reads RX_BACKEND=eden/mkw_eden, HOS_FIX_RES, HOS_SCENE_NATIVE, oot_native_resolution, RX_VK_FOLLOW_MESH, TOTK_*, RX_VK_OOT_*, RX_VK_DESC_SLOTGEN/NO_DESC_REUSE/DESC_REUSE_DGEN or RX_VK_NO_NAN_GUARD, and that includes scripts, workflows, builders, docs, the readme and tests. Nothing calls the removed build.py flags either. The Eden changes are only switch plumbing. With nothing set, the translator does what it did before: FOLLOW_MESH was off by default, GuardFragmentOutputs moved over byte for byte, and both NaN guards are still on by default. MkwVkHost/MkwVkRenderer are untouched (only comments in vk_api.h), so no API bump is needed. No new option ids. The edited tests aren't weakened: the needle changes follow the rename, HOS_HEADLESS=0 and RX_VK_REGISTER_AT_ONCE=0 now expect off, and every ENV_ON_H addition has Path imported first. No Windows macro clashes in hookshot_env.h, and every file that calls env_on includes it.

Blocking

  • The branch conflicts with main and CI hasn't run. It is 19 commits behind (base c09e0ac): #2020 Movement speed, #1971 Mesh shaders row, #2000, #2046 and others touch vk_menu.cpp, vk_draw.cpp, vk_shader.cpp, vk_api.h, hos_main.c and 7 of the edited tests. Only the title check ran. The head's vk_api.h still lacks HOS_OPT_CHEAT_SPEED because #2020 isn't in the base yet. After the rebase, the new test_preset_exempt will flag main's Mesh shaders row: apply_preset doesn't set mesh_mode, and the row isn't marked exempt. That's the right call, it just needs the true.
  • The screenshots don't render. The description links debug-robust-buffers.png and debug-astc-rgba8.png as relative paths, so GitHub serves broken images. They need to be uploaded into the description.

Should fix (inline)

  • On Windows, the environment-goes-first rule makes the menu's Restart ignore edits to hookshot_env.txt, and the log then says every line was "left out".
  • RX_VK_DESC_REUSE=1 means the row's Off in desc_reuse_level. test_menu_env_on still checks the env through menu_env_on, where 1 means on, and the real code never takes that path.
  • A few on/off switches still read set-means-on (HOS_NO_HANDOFF, RX_VK_NO_F16, RX_VK_NO_DMABUF, RX_VK_PROF_ALL, half of RX_VK_LOW_SPEC). test_env_on only catches the != NULL form.
  • Players who followed the old readme still have RX_VK_ASTC_RGBA8=1 in hookshot_env.txt. For them, the new row can't turn the workaround off.

What a player with an existing ini / hookshot_env.txt sees after updating. No saved choice flips, so no options_version bump or migration is needed:

  • Any on/off switch set to 0, false, off, no or empty is now off, where it was on before. A leftover RX_VK_NO_NAN_GUARD=1 or RX_VK_DESC_SLOTGEN=1 line now does nothing.
  • A launch's own environment (Steam launch options, a shell) now beats the same name in hookshot_env.txt.
  • RX_VK_PACE, HOS_DYNRES and the other menu-row switches no longer write themselves into the ini. Values saved by earlier runs stay.
  • Robust buffers moves to Debug, under Developer. Its ini key is the same (robust_buffers), so the saved value carries over. ASTC as RGBA8 is a new row, Off by default.
  • The listed rows no longer turn the preset to Custom. Saved presets are unchanged.
  • The first start re-translates the shaders, as with any change under gpu/vk/eden, compat or vk_shader.cpp. The stamp's new prefixes change nothing when they aren't set.

Nits

  • saved_value: if a preset or the upscaler's snap sets an overridden row (resolution, realtime, pace) to the same value the environment forced, save() writes the player's old value instead of the preset's.
  • from_file[64][96]: past 64 names, or for a name of 96 characters or more, a later line no longer replaces an earlier one. On Windows the file is read twice, so the second read also logs "left out" for those names.
  • Stale --no-polls mentions remain in comments at nx2/oot_patches.py:315 and tests/test_text_speed.py:24.

The two maintainer questions at the end (the WebGPU fallback, the A/B switch policy) are NotRexed's call, so I've only noted them here.

I went through all six commits against the description. Most of it holds up. The removed switches really are gone: nothing in head reads `RX_BACKEND=eden`/`mkw_eden`, `HOS_FIX_RES`, `HOS_SCENE_NATIVE`, `oot_native_resolution`, `RX_VK_FOLLOW_MESH`, `TOTK_*`, `RX_VK_OOT_*`, `RX_VK_DESC_SLOTGEN`/`NO_DESC_REUSE`/`DESC_REUSE_DGEN` or `RX_VK_NO_NAN_GUARD`, and that includes scripts, workflows, builders, docs, the readme and tests. Nothing calls the removed `build.py` flags either. The Eden changes are only switch plumbing. With nothing set, the translator does what it did before: `FOLLOW_MESH` was off by default, `GuardFragmentOutputs` moved over byte for byte, and both NaN guards are still on by default. `MkwVkHost`/`MkwVkRenderer` are untouched (only comments in `vk_api.h`), so no API bump is needed. No new option ids. The edited tests aren't weakened: the needle changes follow the rename, `HOS_HEADLESS=0` and `RX_VK_REGISTER_AT_ONCE=0` now expect off, and every `ENV_ON_H` addition has `Path` imported first. No Windows macro clashes in `hookshot_env.h`, and every file that calls `env_on` includes it. **Blocking** - **The branch conflicts with main and CI hasn't run.** It is 19 commits behind (base `c09e0ac`): #2020 Movement speed, #1971 Mesh shaders row, #2000, #2046 and others touch `vk_menu.cpp`, `vk_draw.cpp`, `vk_shader.cpp`, `vk_api.h`, `hos_main.c` and 7 of the edited tests. Only the title check ran. The head's `vk_api.h` still lacks `HOS_OPT_CHEAT_SPEED` because #2020 isn't in the base yet. After the rebase, the new `test_preset_exempt` will flag main's **Mesh shaders** row: `apply_preset` doesn't set `mesh_mode`, and the row isn't marked exempt. That's the right call, it just needs the `true`. - **The screenshots don't render.** The description links `debug-robust-buffers.png` and `debug-astc-rgba8.png` as relative paths, so GitHub serves broken images. They need to be uploaded into the description. **Should fix** (inline) - On Windows, the environment-goes-first rule makes the menu's **Restart** ignore edits to `hookshot_env.txt`, and the log then says every line was "left out". - `RX_VK_DESC_REUSE=1` means the row's Off in `desc_reuse_level`. `test_menu_env_on` still checks the env through `menu_env_on`, where 1 means on, and the real code never takes that path. - A few on/off switches still read set-means-on (`HOS_NO_HANDOFF`, `RX_VK_NO_F16`, `RX_VK_NO_DMABUF`, `RX_VK_PROF_ALL`, half of `RX_VK_LOW_SPEC`). `test_env_on` only catches the `!= NULL` form. - Players who followed the old readme still have `RX_VK_ASTC_RGBA8=1` in `hookshot_env.txt`. For them, the new row can't turn the workaround off. **What a player with an existing ini / `hookshot_env.txt` sees after updating.** No saved choice flips, so no `options_version` bump or migration is needed: - Any on/off switch set to `0`, `false`, `off`, `no` or empty is now off, where it was on before. A leftover `RX_VK_NO_NAN_GUARD=1` or `RX_VK_DESC_SLOTGEN=1` line now does nothing. - A launch's own environment (Steam launch options, a shell) now beats the same name in `hookshot_env.txt`. - `RX_VK_PACE`, `HOS_DYNRES` and the other menu-row switches no longer write themselves into the ini. Values saved by earlier runs stay. - Robust buffers moves to Debug, under Developer. Its ini key is the same (`robust_buffers`), so the saved value carries over. ASTC as RGBA8 is a new row, Off by default. - The listed rows no longer turn the preset to Custom. Saved presets are unchanged. - The first start re-translates the shaders, as with any change under `gpu/vk/eden`, `compat` or `vk_shader.cpp`. The stamp's new prefixes change nothing when they aren't set. **Nits** - `saved_value`: if a preset or the upscaler's snap sets an overridden row (`resolution`, `realtime`, `pace`) to the same value the environment forced, `save()` writes the player's old value instead of the preset's. - `from_file[64][96]`: past 64 names, or for a name of 96 characters or more, a later line no longer replaces an earlier one. On Windows the file is read twice, so the second read also logs "left out" for those names. - Stale `--no-polls` mentions remain in comments at `nx2/oot_patches.py:315` and `tests/test_text_speed.py:24`. The two maintainer questions at the end (the WebGPU fallback, the A/B switch policy) are NotRexed's call, so I've only noted them here.
host/hos_main.c
hckrman101 (Migrated from github.com) commented 2026-11-11 01:36:24 +00:00

On Windows this breaks the menu's Restart. hos_relaunch.c starts the new process with CreateProcessW(..., NULL /* env */, ...), so it inherits this process's environment, and _putenv has already put every line from the file there. In the new process from_file starts out empty, so each of the file's names is "set in the environment" and gets left out. If the player edits hookshot_env.txt and then picks Restart, the edit is ignored, and the log lists every line as left out. Linux is fine, because it re-execs with /proc/self/environ, the startup environment.

One fix: save the environment block before load_hookshot_env (GetEnvironmentStringsW) and give that to CreateProcessW with CREATE_UNICODE_ENVIRONMENT, plus HOS_RESTART_OF. Another: pass the file's names to the child, e.g. a HOS_ENV_FROM_FILE list that pre-fills from_file.

On Windows this breaks the menu's **Restart**. `hos_relaunch.c` starts the new process with `CreateProcessW(..., NULL /* env */, ...)`, so it inherits this process's environment, and `_putenv` has already put every line from the file there. In the new process `from_file` starts out empty, so each of the file's names is "set in the environment" and gets left out. If the player edits `hookshot_env.txt` and then picks Restart, the edit is ignored, and the log lists every line as left out. Linux is fine, because it re-execs with `/proc/self/environ`, the startup environment. One fix: save the environment block before `load_hookshot_env` (`GetEnvironmentStringsW`) and give that to `CreateProcessW` with `CREATE_UNICODE_ENVIRONMENT`, plus `HOS_RESTART_OF`. Another: pass the file's names to the child, e.g. a `HOS_ENV_FROM_FILE` list that pre-fills `from_file`.
readme.md
hckrman101 (Migrated from github.com) commented 2026-11-11 01:36:24 +00:00

Players who followed the old instructions still have RX_VK_ASTC_RGBA8=1 in hookshot_env.txt. That line goes over the row, so turning the row off does nothing for them, and the row shows Off while the workaround is on. Could this say "turn the row off (and remove any RX_VK_ASTC_RGBA8 line from hookshot_env.txt)"? Even better, the row could show as fixed while the variable is set, the way row_fixed does for other env-held rows.

Players who followed the old instructions still have `RX_VK_ASTC_RGBA8=1` in `hookshot_env.txt`. That line goes over the row, so turning the row off does nothing for them, and the row shows Off while the workaround is on. Could this say "turn the row off (and remove any `RX_VK_ASTC_RGBA8` line from `hookshot_env.txt`)"? Even better, the row could show as fixed while the variable is set, the way `row_fixed` does for other env-held rows.
tests/test_env_on.py
@ -0,0 +1,127 @@
"""One rule for on/off switches: env_on (gpu/vk/compat/hookshot_env.h) in C and C++, kit.env_on in Python.
hckrman101 (Migrated from github.com) commented 2026-11-11 01:36:24 +00:00

This only matches getenv("X") != NULL and == NULL. The forms the rewrite skipped still read set-means-on, so =0 turns them on:

  • host/nn_os_sync.c:446 getenv("HOS_NO_HANDOFF") ? 0 : 1
  • gpu/vk/vk_shader.cpp:572 !getenv("RX_VK_NO_F16") (also :1559 and vk_draw.cpp:6747; it's a translation switch)
  • gpu/vk/vk_memory.cpp:276 getenv("RX_VK_NO_DMABUF") ? nullptr : ...
  • gpu/vk/vk_internal.h:358 getenv("RX_VK_PROF_ALL") || ...
  • RX_VK_LOW_SPEC: vk_menu.cpp reads atoi != 0, but vk_memory.cpp:1410 treats any value as set, so =0 is half on.

Converting these and widening the pattern (a bare getenv("X") followed by ), ?, &&, ||, or preceded by !) would make "every check that only asked whether a switch was set now uses it" true, and keep it true.

This only matches `getenv("X") != NULL` and `== NULL`. The forms the rewrite skipped still read set-means-on, so `=0` turns them on: - `host/nn_os_sync.c:446` `getenv("HOS_NO_HANDOFF") ? 0 : 1` - `gpu/vk/vk_shader.cpp:572` `!getenv("RX_VK_NO_F16")` (also `:1559` and `vk_draw.cpp:6747`; it's a translation switch) - `gpu/vk/vk_memory.cpp:276` `getenv("RX_VK_NO_DMABUF") ? nullptr : ...` - `gpu/vk/vk_internal.h:358` `getenv("RX_VK_PROF_ALL") || ...` - `RX_VK_LOW_SPEC`: `vk_menu.cpp` reads `atoi != 0`, but `vk_memory.cpp:1410` treats any value as set, so `=0` is half on. Converting these and widening the pattern (a bare `getenv("X")` followed by `)`, `?`, `&&`, `||`, or preceded by `!`) would make "every check that only asked whether a switch was set now uses it" true, and keep it true.
tests/test_menu_env_on.py
@ -38,3 +39,4 @@
#include <string>
#include "hookshot_env.h"
using DWORD = unsigned long;
constexpr int MAX_PATH = 260;
hckrman101 (Migrated from github.com) commented 2026-11-11 01:36:24 +00:00

This now checks behaviour the real code doesn't have. desc_reuse_level() reads RX_VK_DESC_REUSE itself, as clamp(atoi, 0, 2), and only calls menu_env_on when the variable is unset. So the env cases here ("1" meaning reuse on, "0" meaning the row's Off) go through a path the game never takes. In the game, RX_VK_DESC_REUSE=1 gives level 1, which is the row's Off (no slotgen), and =0 turns reuse off completely. That's also a trap for anyone used to RX_VK_DESC_SLOTGEN=1 meaning on. Could the test cut desc_reuse_level too and check 0/1/2 plus the unset case? Making the env values match the row (0/1) with a separate value for "never" would also avoid the surprise.

This now checks behaviour the real code doesn't have. `desc_reuse_level()` reads `RX_VK_DESC_REUSE` itself, as `clamp(atoi, 0, 2)`, and only calls `menu_env_on` when the variable is unset. So the env cases here (`"1"` meaning reuse on, `"0"` meaning the row's Off) go through a path the game never takes. In the game, `RX_VK_DESC_REUSE=1` gives level 1, which is the row's **Off** (no slotgen), and `=0` turns reuse off completely. That's also a trap for anyone used to `RX_VK_DESC_SLOTGEN=1` meaning on. Could the test cut `desc_reuse_level` too and check 0/1/2 plus the unset case? Making the env values match the row (0/1) with a separate value for "never" would also avoid the surprise.
jarynclouatre commented 2026-11-11 02:45:32 +00:00 (Migrated from github.com)

Two more dead ones in the same area, in case you want them in this pass: RX_VK_DUMP_AT and RX_VK_DUMP_TEX are named in gpu/vk/VULKAN.md (lines 72-73 on f915af9) and in the comment at gpu/vk/vk_texture.h:211, but nothing in the source reads either one any more.

Two more dead ones in the same area, in case you want them in this pass: `RX_VK_DUMP_AT` and `RX_VK_DUMP_TEX` are named in `gpu/vk/VULKAN.md` (lines 72-73 on f915af9) and in the comment at `gpu/vk/vk_texture.h:211`, but nothing in the source reads either one any more.
hckrman101 commented 2026-11-11 11:08:41 +00:00 (Migrated from github.com)

nullifyshot, thanks for the careful read.

Here is where each point ended up:

Before merge

  1. Conflicts: main is merged in and CI runs again. Mesh shaders (#1971) has preset_exempt set (and the restart flag); test_preset_exempt passes on it.
  2. Windows Restart: fixed in fix(restart): Windows' Restart reads hookshot_env.txt again, the way you suggested. main keeps GetEnvironmentStringsW() before load_hookshot_env() (hos_relaunch_keep_env). relaunch() passes that block with CREATE_UNICODE_ENVIRONMENT. HOS_RESTART_OF replaces any older value and is placed in the block's sorted order (hos_relaunch_env_block in host/hos_relaunch.h). test_menu_restart checks the block-building on its own and checks that main and relaunch() wire it up. If saving the block failed, the restart falls back to inheriting the current environment, as before.

Minor

  • SDL_VIDEODRIVER / SDL_VIDEO_DRIVER: back to getenv. You're right, they hold a driver name.
  • Removed switches: the log now has one line per removed name that is still set, with its replacement where there is one: RX_VK_NO_DESC_REUSE → RX_VK_DESC_REUSE=never, RX_VK_DESC_SLOTGEN → RX_VK_DESC_REUSE (same 0/1), RX_VK_NO_NAN_GUARD → RX_VK_NAN_GUARD=0. The rest (RX_VK_OOT*, HOS_FIX_RES, HOS_SCENE_NATIVE, RX_VK_FOLLOW_MESH, RX_VK_DESC_REUSE_DGEN, TOTK_MINIMAL_LAYOUT) are logged as doing nothing.
  • from_file limits: raised to 256 names of up to 127 characters, with a comment saying what happens past that.

Suggestion (splitting out the env_on rule): I kept it in this PR. The merges and CI fixes have already been done on top of it. Splitting it now would mean re-resolving the same files twice. If a maintainer would still rather have it separate, I can split it.

jarynclouatre:

Thanks, both removed. gpu/vk/VULKAN.md now shows the current RX_VK_DUMP=<present> form and no longer names RX_VK_DUMP_TEX. The vk_texture.h comment now describes texture_read_texels itself (docs: drop the dead RX_VK_DUMP_AT and RX_VK_DUMP_TEX, ...).

**nullifyshot**, thanks for the careful read. Here is where each point ended up: **Before merge** 1. Conflicts: main is merged in and CI runs again. Mesh shaders (#1971) has `preset_exempt` set (and the restart flag); `test_preset_exempt` passes on it. 2. Windows Restart: fixed in `fix(restart): Windows' Restart reads hookshot_env.txt again`, the way you suggested. `main` keeps `GetEnvironmentStringsW()` before `load_hookshot_env()` (`hos_relaunch_keep_env`). `relaunch()` passes that block with `CREATE_UNICODE_ENVIRONMENT`. `HOS_RESTART_OF` replaces any older value and is placed in the block's sorted order (`hos_relaunch_env_block` in `host/hos_relaunch.h`). `test_menu_restart` checks the block-building on its own and checks that `main` and `relaunch()` wire it up. If saving the block failed, the restart falls back to inheriting the current environment, as before. **Minor** - `SDL_VIDEODRIVER` / `SDL_VIDEO_DRIVER`: back to `getenv`. You're right, they hold a driver name. - Removed switches: the log now has one line per removed name that is still set, with its replacement where there is one: `RX_VK_NO_DESC_REUSE` → `RX_VK_DESC_REUSE=never`, `RX_VK_DESC_SLOTGEN` → `RX_VK_DESC_REUSE` (same 0/1), `RX_VK_NO_NAN_GUARD` → `RX_VK_NAN_GUARD=0`. The rest (`RX_VK_OOT*`, `HOS_FIX_RES`, `HOS_SCENE_NATIVE`, `RX_VK_FOLLOW_MESH`, `RX_VK_DESC_REUSE_DGEN`, `TOTK_MINIMAL_LAYOUT`) are logged as doing nothing. - `from_file` limits: raised to 256 names of up to 127 characters, with a comment saying what happens past that. **Suggestion (splitting out the env_on rule):** I kept it in this PR. The merges and CI fixes have already been done on top of it. Splitting it now would mean re-resolving the same files twice. If a maintainer would still rather have it separate, I can split it. **jarynclouatre**: Thanks, both removed. `gpu/vk/VULKAN.md` now shows the current `RX_VK_DUMP=<present>` form and no longer names `RX_VK_DUMP_TEX`. The `vk_texture.h` comment now describes `texture_read_texels` itself (`docs: drop the dead RX_VK_DUMP_AT and RX_VK_DUMP_TEX, ...`).
nullifyshot commented 2026-11-11 16:23:29 +00:00 (Migrated from github.com)

Thanks, hckrman101. I checked the follow-ups and everything from my review is handled:

  • Restart: the kept start-up block, with HOS_RESTART_OF put in at its sorted place, is exactly what I had in mind, and the fallback to inheriting is a nice touch.
  • The smaller points: the SDL variables, the removed-switch log lines and the raised from_file limits all look good.

Two small things are left:

  1. A new one-line conflict with main in host/hos_main.c. #926 (the control bridge) added hos_control_start(HOOKSHOT_VERSION); right where this PR adds removed_switch_notes();, just after the version line. Keeping both resolves it. #926's new variables (HOS_CONTROL, HOS_CONTROL_MEM) hold a path and a value rather than on/off, so test_env_on shouldn't flag them after the merge. Main's other new commit (#2251) doesn't touch this PR's files.

  2. The PR description still has the old RX_VK_DESC_REUSE values. It says RX_VK_DESC_REUSE=0/1/2 (never, the global check, by slot). Since 641f516, the code and docs/development.md use the row's values instead:

    • 0 is the row's Off (the global check)
    • 1 is On (by slot)
    • never turns reuse off

    So anyone following the description and setting =0 to turn reuse off would get the global check. The docs are right; only the description needs the new values.

I only read the code this time and haven't run the tests on a merge with main. CI's full pass on 13bb15c covers everything except main's two newest commits.

Thanks, hckrman101. I checked the follow-ups and everything from my review is handled: - **Restart:** the kept start-up block, with `HOS_RESTART_OF` put in at its sorted place, is exactly what I had in mind, and the fallback to inheriting is a nice touch. - **The smaller points:** the SDL variables, the removed-switch log lines and the raised `from_file` limits all look good. Two small things are left: 1. **A new one-line conflict with main** in `host/hos_main.c`. #926 (the control bridge) added `hos_control_start(HOOKSHOT_VERSION);` right where this PR adds `removed_switch_notes();`, just after the version line. Keeping both resolves it. #926's new variables (`HOS_CONTROL`, `HOS_CONTROL_MEM`) hold a path and a value rather than on/off, so `test_env_on` shouldn't flag them after the merge. Main's other new commit (#2251) doesn't touch this PR's files. 2. **The PR description still has the old `RX_VK_DESC_REUSE` values.** It says `RX_VK_DESC_REUSE=0/1/2 (never, the global check, by slot)`. Since 641f516, the code and `docs/development.md` use the row's values instead: - `0` is the row's Off (the global check) - `1` is On (by slot) - `never` turns reuse off So anyone following the description and setting `=0` to turn reuse off would get the global check. The docs are right; only the description needs the new values. I only read the code this time and haven't run the tests on a merge with main. CI's full pass on 13bb15c covers everything except main's two newest commits.
nullifyshot commented 2026-11-11 16:27:33 +00:00 (Migrated from github.com)

Quick correction: you'd already merged main (b62dcc0) a couple of minutes before my comment went up, and it keeps both lines, so ignore point 1. Point 2, the RX_VK_DESC_REUSE values in the description, still applies.

The failed appimage check is the runner, not the PR: it couldn't resolve archive.ubuntu.com, so apt failed before anything was built. Re-running the job should clear it.

Quick correction: you'd already merged main (b62dcc0) a couple of minutes before my comment went up, and it keeps both lines, so ignore point 1. Point 2, the `RX_VK_DESC_REUSE` values in the description, still applies. The failed `appimage` check is the runner, not the PR: it couldn't resolve `archive.ubuntu.com`, so apt failed before anything was built. Re-running the job should clear it.
hckrman101 commented 2026-11-11 16:30:59 +00:00 (Migrated from github.com)

Thanks for checking again. The description now has the row values: RX_VK_DESC_REUSE=0 is the row's Off (the global check), 1 is On (by slot, the default), and never turns reuse off. Same as docs/development.md.

The merge with main (b62dcc0) keeps both lines in hos_main.c, as you said. It also needed one test change: save() now takes a lock since #926, so test_menu_env_overrides.py's harness includes <mutex>. The game-free suite passes on the merge, and a full build of it is running.

Thanks for checking again. The description now has the row values: `RX_VK_DESC_REUSE=0` is the row's Off (the global check), `1` is On (by slot, the default), and `never` turns reuse off. Same as `docs/development.md`. The merge with main (b62dcc0) keeps both lines in `hos_main.c`, as you said. It also needed one test change: `save()` now takes a lock since #926, so `test_menu_env_overrides.py`'s harness includes `<mutex>`. The game-free suite passes on the merge, and a full build of it is running.
Commenting is not possible because the repository is archived.
Reviewers
No reviewers
Labels
Clear labels   
accessibility
Barrier affecting people with disabilities

  
autorelease: pending

   
autorelease: tagged

   
bug
Something isn't working

  
dependencies
Pull requests that update a dependency file

  
documentation
Improvements or additions to documentation

  
duplicate
This issue or pull request already exists

  
enhancement
New feature or request

  
github_actions
Pull requests that update GitHub Actions code

  
good first issue
Good for newcomers

  
help wanted
Extra attention is needed

  
invalid
This doesn't seem right

  
question
Further information is requested

  
testing wanted

   
wontfix
This will not be worked on

No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
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/Hookshot!2042
No description provided.