Audio output in the Insert menu: Auto, Stereo or 5.1 #1240
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!1240
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/audio-output-menu"
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?
A follow-up to #1182, as suggested in its review: an Audio output row in the Insert menu (Gameplay) with Auto, Stereo and 5.1.
What
audio_output=inhookshot_options.ini.host/nn_audio.creads it from the file, the waynn_applet.creads the Handheld profile, because the game asks for its speakers only once, at its start. The row says "(restart)", like Output resolution.HOS_AUDIO_CHANNELS=2or6still wins over the menu.How it was tested
Machine: Linux (CachyOS), RTX 4070 SUPER, default output stereo.
In game: a game build with this host change, on the v2.0.8-rc.5 source plus #1182, with headless runs. In each run the log line and the dumped audio's channel count match:
audio_outputHOS_AUDIO_CHANNELSNot tested: the menu row itself wasn't seen in game. This PC has no Vulkan toolchain, so the build used the release's prebuilt Vulkan back end.
New test:
tests/test_audio_output_menu.pycompiles the realau_speakersandau_menu_outputtogether with the menu'sini_keyandtx_audio_output. It writes the row's choice under the key the menu would save, so a renamed row or key fails it. It covers Auto with a 2-, 6- and 8-channel device, Stereo, 5.1, hand-edited 3 and -1 (clamped as the menu reads them), andHOS_AUDIO_CHANNELSover the menu. It's registered inrun_all.pywith--cxx g++. I checked that it fails when the host ignores the menu, when the key is misspelled or when the row is renamed.Windows:
host/nn_audio.cpasses a syntax check with mingw-w64 and has the same warnings as main's.Suite: on this branch (on
09e4eb5),python -I -B tests/run_all.pygives 144 passed, 2 failed, 17 skipped on Linux. Both failures are the same on unchanged main here:nx2/test_nup_read_name.pyneedszstandard, which isn't installed here.tests/test_output_resolution.pyaborts on its first case.🤖 Generated with Claude Code
The two failing checks are
tests/test_output_resolution.py, which also fails on main's own CI (fe611e4, Linux and Windows).tests/test_audio_output_menu.pypasses on both, and both Vulkan back-end builds pass.Nice follow-up, and the host side looks right: the ini is read once inside
au_speakers,HOS_AUDIO_CHANNELSwins over the menu, Auto falls back to #1182's device check, old ini files without the key give Auto, and the value is clamped to 0..2. 5.1 on a stereo device just gets mixed down like before. The test is thorough and is registered inrun_all.pywith--cxx g++. The readme matches the behaviour.It needs a rebase though: this PR is currently conflicting with main, and the row will not compile as written on current main. Main's
Rownow has ahintfunction pointer before therestartandpreset_exemptflags (see the Output resolution and Microphone rows), so the trailingtruelands in the hint slot. Two comments below on what to change.The Python source checks failures on this PR are the known ones on main (
test_output_resolution.py,test_program_cache_cleanup.py), not yours.Once
restartis set on the row, main appends " (after restart)" to the label itself when the value differs from the one at start, and the other restart rows (Output resolution) have plain value text. So this can be plain "Auto", "Stereo", "5.1" and the test'stx_audio_outputexpectations adjusted to match; otherwise it would read "Auto (restart) (after restart)".On current main the fields after
helparehint, thenrestart, thenpreset_exempt, so this needs to be..., nullptr, true, true}(restart and preset_exempt), or a smallhn_audio_outputwithMENU_HINTSif you want a per-value line. Settingrestart = trueis also what puts the row in the "Restart needed" list and makes closing the menu offer a restart, which this row doesn't get otherwise.Thanks. Rebased onto main (
ac3bd44) as4856258:hn_audio_output, true, true}: a restart row, still preset-exempt, so a change shows "(after restart)", joins the Restart needed list, and closing the menu offers the restart.Auto,Stereo,5.1, with a hint line each (hn_audio_outputviaMENU_HINTS).menu_restart.h's comment now counts six restart rows (kMaxis 8).Row.Fixed, the row now ends
hn_audio_output, true, true}and matches Row.Plain labels now, thanks.
Thanks, all points are fixed: the row ends
hn_audio_output, true, true}(hint, restart, preset_exempt, in main's Row order), so it shows "(after restart)", joins the Restart needed list and the close offers the restart; the labels are plain; the test checks the row fields and hints; no conflicts. The two failing checks are main's known ones.One non-blocking thing: #1270 adds an Audio tab to the Insert menu (Volume, Mute in background), and this row would fit naturally there. Worth coordinating with its author on which PR moves the row, whichever lands second.