Repository navigation
Conversation
|
The two failing checks are |
hckrman101
left a comment
There was a problem hiding this comment.
Nice follow-up, and the host side looks right: the ini is read once inside au_speakers, HOS_AUDIO_CHANNELS wins 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 in run_all.py with --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 Row now has a hint function pointer before the restart and preset_exempt flags (see the Output resolution and Microphone rows), so the trailing true lands 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.
| "each page at once, as pressing A does."}, | ||
| {TAB_GAMEPLAY, "Audio output", &opts.audio_output, 0, 2, 1, tx_audio_output, | ||
| "The speakers the game mixes its sound for. Auto is 5.1 when your default output device has 5.1 or more speakers, " | ||
| "else stereo. Stereo or 5.1 chooses one whatever the device has. Applies after a restart.", true}, |
There was a problem hiding this comment.
On current main the fields after help are hint, then restart, then preset_exempt, so this needs to be ..., nullptr, true, true} (restart and preset_exempt), or a small hn_audio_output with MENU_HINTS if you want a per-value line. Setting restart = true is 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.
There was a problem hiding this comment.
Fixed, the row now ends hn_audio_output, true, true} and matches Row.
| const char* tx_rumble(int v) { return v ? tx_percent(v) : "Off"; } | ||
| // (hookshot) Audio output: the speakers the game mixes for, read once at its start (host/nn_audio.c au_menu_output) | ||
| const char* tx_audio_output(int v) { | ||
| static const char* t[] = {"Auto (restart)", "Stereo (restart)", "5.1 (restart)"}; |
There was a problem hiding this comment.
Once restart is 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's tx_audio_output expectations adjusted to match; otherwise it would read "Auto (restart) (after restart)".
There was a problem hiding this comment.
Plain labels now, thanks.
A follow-up to NotRexed#1182, as suggested in its review: the speakers the game mixes for can be chosen in the Insert menu (Gameplay), not only with HOS_AUDIO_CHANNELS. Auto keeps NotRexed#1182's behaviour (5.1 when the default output has 5.1 or more, else stereo); Stereo or 5.1 chooses one whatever the device has. The choice is saved as audio_output= in hookshot_options.ini and read by host/nn_audio.c from the file, as nn_applet.c reads the Handheld profile: the game asks for its speakers once, at its start. The row is a restart row (Row::restart), so the menu marks a change "(after restart)" and offers a restart on closing; each value has a hint line. HOS_AUDIO_CHANNELS still wins over the menu. tests/test_audio_output_menu.py compiles the real au_speakers with the menu's ini_key, value texts and hints, so the key the menu saves is the one the host reads, and checks the row is a preset-exempt restart row. Assisted-by: Claude Opus 5.5 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5e8c1d6 to
4856258
Compare
|
Thanks. Rebased onto main (ac3bd44) as 4856258:
|
hckrman101
left a comment
There was a problem hiding this comment.
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.
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 Mix for 5.1 when the default output has 5.1 speakers #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