[WIP,DRAFT] Fix disabled Music & sound crash #1050
Draft
+23
−22
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
OK, I tested a bit more and I'm hearing music & laser shooting sounds, also voice communications. But the two separate settings in config file are now broken, if you enable music, you'll also have sounds. But at least the crash is gone... |
vincele
changed the title
royfalk
requested review from
royfalk,
stephengtuggy and
BenjamenMeyer
and removed request for
royfalk and
stephengtuggy
stephengtuggy
approved these changes
Mar 3, 2025
There was a problem hiding this comment.
This looks really good, @vincele . Thanks for the PR. Tentatively approving, pending further testing.
|
I'll keep this as WIP until I understand why the "music off" option also disable the sounds. Either we want a single option or both should be properly working independently. |
BenjamenMeyer
approved these changes
Mar 8, 2025
vincele
force-pushed
the
fix_nosound_crash
branch
3 times, most recently
from
72f2440 to
7e63edc
Compare
Issue is that audio system initialization order was not right and some audio processing was not properly guarded by checks. * `AUDInit()` returns a boolean value telling audio system initialization status, use that to set `g_game.sound_enabled` which is now used everywhere needed. This reflects either failures, or music & sound being disabled in the config file. Then only call the other initialization / close functions: * `initALRenderer()` * `Music::InitMuzak()` * `initSceneManager()` * `initScenes()` * `closeRenderer()` if the audio susbystem has been properly initialized Then the `initALRenderer()` function does not need to check for the sound system being enabled. Call `Music::InitMuzak()` after `initALRenderer()`, not sure if this is absolutely required, but seemed sensible anyways. Move the `Music::MuzakCycle()` function out of `bootstrap_draw()` into a more sensible place in `bootstrap_first_loop()`. Then guard: * `Music::MuzakCycle()` * `muzak->GotoSong()` with an `if (g_game.sound_enabled)` check. Remove the FIXME comment in OpenALRenderer.cpp `RendererData::suspend()` as the exception is not thrown there any more. Fixes issue: vegastrike#1040 Related issue: vegastrike#859 Signed-off-by: Vincent Legoll <[email protected]>
vincele
force-pushed
the
fix_nosound_crash
branch
from
7e63edc to
0f6ac8b
Compare
|
Still WIP, but slight progress:
|
vincele
changed the title
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue is that audio system initialization order was not right and some audio processing was not properly guarded by checks.
More details in the commit message.
Code Changes:
Issues:
On void-linux x86_64 musl libc, bare ALSA sound system (no pulseaudio, no pipewire, nor jack).
But music disabled & "All Sounds" enabled did not output anything, I don't know if this has been broken or if there are any sounds that should have been played...
Please test under windows, macos, and other linuxes...
Purpose:
What is this pull request trying to do?
What release is this for? Next one
Is there a project or milestone we should apply this to? Next one