Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure initial volume for CD music is properly set. #406

Merged

Conversation

b-kurczynski
Copy link
Contributor

First intention was to do a fix in a similar way like it has been fixed for effects volume. However, the "set-volume" request does not reach S3SetCDAVolume() function where caching of requested volume level could take place because it's stopped at if (c->active) >>here<<.

As the fix enforced a change in original code, I decided to put it in a place where it's easier to understand. Other option was to do a fix in mentioned spot of S3.c file together with an implementation of deferred volume set in S3SetCDAVolume(), but that would change two files compared to one in proposed solution.

@dethrace-labs
Copy link
Owner

When you say "initial volume" do you mean the volume as set when the game starts and loads the music volume from config file, or when changing the music volume from the sound options screen?

@b-kurczynski
Copy link
Contributor Author

b-kurczynski commented Sep 19, 2024

I meant the game can't set a proper volume to the music as it's stored in the config.
Let say you set the Music Volume to 0 in the Settings menu, quit the game, and then start again. Music will be at full volume even though in Settings menu 0 will be shown.

@b-kurczynski b-kurczynski force-pushed the cda-initial-volume-fix branch from e6f704f to f5c0948 Compare November 12, 2024 21:34
@b-kurczynski
Copy link
Contributor Author

Hi @dethrace-labs , this PR hangs for some time already. Should I do something more or there are objections to let it go to main?

@Serasul1984
Copy link

its crazy to me that this over 10 years old bug, you find in many carmageddon games that also happen during map loading can be fixed now.
This gives me hope that other very annoying bugs from the original game can be fixed too, like "sound of car engines gets silent when to many things explode or crash at the same time"

@dethrace-labs
Copy link
Owner

Thanks for the ping! LGTM, made a suggestion to more clearly document the original behavior bug

@dethrace-labs dethrace-labs merged commit 884f134 into dethrace-labs:main Dec 3, 2024
6 checks passed
@b-kurczynski b-kurczynski deleted the cda-initial-volume-fix branch December 4, 2024 17:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants