OnlineCop / kq-fork

Fork of KQ r910. Just for fun.
GNU General Public License v2.0
15 stars 9 forks source link

Crash in mixer.cpp when game is loaded #89

Closed OnlineCop closed 2 years ago

OnlineCop commented 2 years ago

When I launch the game, I see "ERROR: Error with sound: MOD support not available" on the console output.

If I open the Config window from the main menu, "Sound System" is OFF and toggling it ON does nothing: I see "...Please wait..." briefly, but it does not change to ON.

  1. I can start a new game, and do not crash throughout the opening dialogue or on the world map.
  2. If I load a game, in Mix_Volume() the line prev_volume /= num_channels; has num_channels set to 0, causing a divide-by-zero error.
OnlineCop commented 2 years ago

In the original code, shutdown_music(), set_music_volume(), poll_music() all had if (is_sound != 0) guards; removing them appears to be where the bug originates.

KSaveGame::load_game() calls KGame::change_map() which indirectly calls Music.play_music() (which has one of these is_sound checks and exits if not initialized), and afterward calls Music.set_volume() and Music.set_music_volume(), which do not have that check.

It appears that new games don't set the volume whereas loading a new game does.

pedro-w commented 2 years ago

Agreed, I’m on the case with this.

On Fri, 17 Jun 2022 at 20:11, OnlineCop @.***> wrote:

In the original code, shutdown_music(), set_music_volume(), poll_music() all had if (is_sound != 0) guards; removing them appears to be where the bug originates.

KSaveGame::load_game() calls KGame::change_map() which indirectly calls Music.play_music() (which has one of these is_sound checks and exits if not initialized), and afterward calls Music.set_volume() and Music.set_music_volume(), which do not have that check.

It appears that new games don't set the volume whereas loading a new game does.

— Reply to this email directly, view it on GitHub https://github.com/OnlineCop/kq-fork/issues/89#issuecomment-1159160482, or unsubscribe https://github.com/notifications/unsubscribe-auth/AFJFGZ4O6FIVZJQFG6LDMLDVPTEXBANCNFSM5ZDBDC7A . You are receiving this because you were assigned.Message ID: @.***>

OnlineCop commented 2 years ago

It looks like simply adding if (is_sound == 0) { return; } guards to both set_music_volume() and set_volume() were enough to get rid of the crash for me.

I've got a PR I'm almost ready to push that changes is_sound to a longer name. If you want, I can add those if() checks into that so you don't need to do a separate PR for that fix.

pedro-w commented 2 years ago

Alright but I found more problems:

I will make a branch, and when you've pushed yours I will rebase and make it a PR, OK?