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

[Bugfix & Enchancement?] Clicking on the sound slider plays the sound at wrong volume #7065

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Tully-B
Copy link
Contributor

@Tully-B Tully-B commented Apr 10, 2024

This pull request does two things: the first commit is a bug-fix, the second one is a related adjustment.

The sound slider in the game menu plays the move sound at the previous volume level when clicked on, as clicking itself will play the same sound (gmenu.cpp) and because of that the duplicate in gamemenu.cpp/gmenu_left_mouse will never play again. This fix makes the left click not play a sound when clicked on the sound slider as an exception. If there's a better way to do it, let me know!

The second one changes the minimum volume from -100.0 dB to -60.0 dB. Initially I thought the sound slider was wrong, as moving it to the middle almost made the music go silent, but after checking the code, it seems to be working perfectly, logarithmic conversion et al, just the minimum value seems to be way too low and this caused the slider to seem off. -60.0 dB is arbitrary, but in my opinion, -100.0 dB seems too low in any case.


// Do not play a duplicate sound if we are clicking on the sound slider.
// CurrentMenuId == 5 is the options menu, i == 1 is the sound slider.
if ((sgCurrentMenuIdx != 5) || (i != 1)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if ((sgCurrentMenuIdx != 5) || (i != 1)) {
if (!(sgCurrentMenuIdx == 5 && i == 1)) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could also be ok with, but I can see how Qndel's suggestion makes logical sens

Suggested change
if ((sgCurrentMenuIdx != 5) || (i != 1)) {
if (sgCurrentMenuIdx != 5 || i != 1) {

@Trihedraf
Copy link
Collaborator

Why remove range from the volume? I can still hear sound till 4 notches from the bottom.

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.

None yet

4 participants