-
Notifications
You must be signed in to change notification settings - Fork 933
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
Prevent PlaySound overlapping #1692
Conversation
I'm OK with this, but it touches on sound so i'll defer to @kcat |
return; | ||
|
||
sndmgr->playSound(soundId, volume, pitch, MWSound::Type::Sfx, MWSound::PlayMode::NoEnv); | ||
MWBase::Environment::get().getSoundManager()->playSound(soundId, volume, pitch, MWSound::Type::Sfx, MWSound::PlayMode::NoEnv); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't this change now cause the UI sound to get cutoff if it's played again while already playing, rather than the apparently intended behavior of letting the original sound finish without interruption?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we treat GUI sounds differently from other ones?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How much work would it be to treat GUI sounds differently?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How much work would it be to treat GUI sounds differently?
Almost none. We just should decide which behaviour is intended:
- Allow to stack most of GUI sounds, with special case for journal wheel scrolling to prevent overlapping.
It is our current upstream implementation. - Play old copy to end and do not launch new copy to prevent overlapping.
- Take the PR implementation.
Honestly, I can not hear much difference between all three variants (excepts of loudness in first case), so I can not advice which behaviour is better.
Also keep in mind that too much playing sounds can lead to crash on Windows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(3)
My only other real complaint is that it basically does |
So you would rather have a temp variable to save another cycle? RAM's cheap, CPU is not. :) |
How do you suggest to avoid it? |
I added a new stopSound function, which accept Sound_Buffer. |
Should be fixed now. Any other issues? |
@kcat mergable? :) |
Looks alright at a glance. Assuming it works and no one else has objections, go for it. |
Merged, this should make Arktwend fans happy. :) |
If we play a new sound, we should stop the previous copy of this sound. This is easily to check: execute "playsound X" several times in console in vanilla game.
We already use similar approach for 3D sounds: we allow to play only one copy of given sound per object, so this PR just makes sound playing consistent.
As a side effect, this PR fixes a nightmare scene in Arktwend, so it should not crash now.