feat: Audio tab in the Insert menu with Volume and Mute in background #1270
No reviewers
Labels
No labels
accessibility
autorelease: pending
autorelease: tagged
bug
dependencies
documentation
duplicate
enhancement
github_actions
good first issue
help wanted
invalid
question
testing wanted
wontfix
No milestone
No assignees
1 participant
Due date
No due date set.
Dependencies
No dependencies set
Reference
MagicBeanDev/Hookshot!1270
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/audio-settings"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What
The Insert menu gets a fourth tab, Audio, between Graphics and Gameplay, with two rows:
host/nn_audio.c) scales each frame before the output, on every output channel (stereo or #1182's 5.1). A change ramps over one 5 ms frame, so it does not click. Gains stay at or below 1, so nothing clips. At 100% the frame is left alone.host/nn_hid.c'sgame_in_fronttreats Steam's or Discord's) still counts as the game's window.The host reads both through the renderer's option table every 100 ms, as new ids
HOS_OPT_VOLUME = 27andHOS_OPT_MUTE_BACKGROUND = 28(gpu/vk/vk_api.h; 18 to 22 are the motion rows, 24 to 26 DLSS). Without the Vulkan renderer's menu the game plays at full volume. Both are saved inhookshot_options.ini(volume=,mute_in_background=).menu_hit::tab_slot_wnow takes the tab count instead of assuming 3, so the tab strip and its clicks fit four tabs.Why
There was no way to set the game's volume apart from the Windows mixer, and no way to silence it while alt-tabbed. Both are common requests for a PC game.
#1240's Audio output row (Auto, Stereo, 5.1) would fit in this tab too. Whichever of the two lands second can move it here.
How it was tested
tests/test_audio_volume.pycompilesaudio_apply_gainfrom the source and checks the ramp, the end gain, no clipping and equal gain on 2 and 6 channels. It also checks the option ids against the renderer's table and the menu's rows and defaults.tests/test_menu_hit.cppgains checks for a four-tab strip.tests/run_all.pyshows no new failures on Windows: the 3 that fail locally fail the same way onmain.🤖 Generated with Claude Code
Nice, tidy change. Option ids 27/28 are free on main and in the open PRs, no API version bump is needed (no struct change), and the 4-tab hit-testing, Q/E cycling (all through TAB_COUNT) and the rows' flags (slider, stops_at_ends, preset_exempt) are consistent. Ini keys come out as
volumeandmute_in_background, and an old ini without them keeps the 100 / Off defaults. The gain ramp is correct (0..1 so nothing clips, same gain on every channel, applied after the mix in stereo and 5.1), and the tests are registered with their compiler.One real thing, plus nits:
game_in_frontin nn_hid.c only does that while the last normal foreground window was ours. Here, with the game in the background, a notification toast or a tray flyout takes the foreground, the check says "front", and the sound comes back until it goes away. Worth sharing that logic withgame_in_front(or copying itslast_normal_was_usstate).Nit: the ids are 27 and 28 (HOS_OPT_VOLUME / HOS_OPT_MUTE_BACKGROUND), not 18 and 19 (those are the gyro's). Same on the next line.
Any TOOLWINDOW / NOACTIVATE foreground counts as ours, whatever was in front before. nn_hid.c's game_in_front only allows that while the last normal foreground window was the game's (last_normal_was_us), so with the game in the background a toast or tray flyout would unmute it. Same state would fix it here.
Thanks for the careful review. All points are in 5976ed6 (the branch is also rebased on current main):
last_normal_was_us, asgame_in_frontdoes: a tool or no-activate window counts as the game's only while the last normal window in front was the game's. The mixer thread keeps its own copy of the state rather than sharinggame_in_front's statics across threads. The decision is a small pure function (audio_front_decide), andtests/test_audio_volume.pychecks the toast-behind-the-game case.kTabSideis now 70 (the[Q / LB]/[E / RB]hints are 49 / 47 px from 16 in), so the slots are 85 px with ~14 px between the highlight and the next title.test_menu_hit.cpphas the new edges. Checked in game at scale 2 (1440p).Thanks, the review points are all addressed:
One blocker left: the PR no longer merges. GitHub reports it as dirty, since #1240 (Audio output) has merged into main after your rebase base (
7dbdcdc). It touches the same hunks in vk_menu.cpp, vk_menu.h, nn_audio.c, readme.md and tests/run_all.py. Please rebase again.While you are in there, #1240 put Audio output in the Gameplay tab. It would fit better in your new Audio tab, but that is optional. If you move it, tests/test_audio_output_menu.py asserts the row's tab, so update that too. The ids 27/28 do not collide with anything on main.
No checks are reported on the branch, so I could not see CI. I did not build or run it. Once it merges cleanly this looks good to me.
Fixed in 5976ed6: it keeps its own last_normal_was_us and the decision is tested.
Fixed in 5976ed6, now 27/28.
#1459 rewrites the entire menu, I can assist rebasing this PR on top of the other (trying to merge ASAP)
Heads-up: this conflicts with main now (a lot landed today). Could you merge main into the branch (or rebase) and resolve the conflicts? Thanks!
Rebased onto the ImGui menu (#1459) and #1240. Thanks for the offer. Since #1459 replaced the menu, I redid the menu part on top of current main (
9edb3a5) as one commit,df2d84c, instead of carrying the old GDI tab-strip changes:TAB_COUNT5). The tab widths come from the titles now, so thevk_menu_hit.handtest_menu_hit.cppchanges are gone.tests/test_audio_output_menu.pyexpectsTAB_AUDIO.host/nn_audio.c): unchanged from the reviewed version. It has the squared curve and the overlay rule with its ownlast_normal_was_us, applied onout_channels. The ids are still 27/28, and nothing on main uses them.test_audio_volume.pydeclaresRUN_ALL_ARGSlike the other tests now.run_all.py: 258 passed. The 3 that fail locally on Windows (test_nup_out_of_space,test_fpx_sticky,test_precompile_compute_guard) fail the same way on main.484cfd1plus this change and played on Windows 11 with an RX 7900 XTX. The Audio tab shows its three rows in the new style, the volume slider and Mute in background work, and Audio output works as before. The rebase onto9edb3a5merged cleanly (the only conflict wasrun_all.py's move to per-test declarations), so I did not rebuild for it.Thanks for redoing this on top of the ImGui menu. It looks good to me.
last_normal_was_us, and the ramp onout_channels. On Linux the foreground calls go through the posix windows.h shim, the same way nn_hid.c's do.TAB_COUNTis 5. Everything that uses tab numbers goes through the enum: Q/E cycling,tab_selected,TAB_MOTION/TAB_PAGE, and the tests that parseenum MenuTab. Audio output keeps its ini key because the row name did not change, andtest_audio_output_menu.pynow expectsAUDIO.test_audio_volume.pydeclaresRUN_ALL_ARGS, and run_all.py is untouched. That matches #1822.Heads-up, not a blocker: #1737 (Volume slider, open) adds the same
opts.volume/HOS_OPT_VOLUME/is_sliderchanges and its own gain in nn_audio.c, with id 44. #1560 also uses 44. Whichever of #1270 and #1737 lands second has to drop its own copy instead of resolving the conflict line by line. Otherwise you'd end up with two volume rows or two gains applied.No CI checks are reported on the branch, and GitHub shows it as mergeable with main. I didn't build or run it.