-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
(fix) Sound preferences: don't set m_settingsModified in update slots #13450
(fix) Sound preferences: don't set m_settingsModified in update slots #13450
Conversation
f53f991
to
c75a420
Compare
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.
I haven't had a chance to test it due to time, but reviewed the code, and I can see no missing logic. Thanks for the fix!
c75a420
to
5321f91
Compare
@mixxxdj/developers This fixes an annoying bug in main, please someone take a look. |
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.
LGTM and thank you for the control proxy refactoring
#ifdef __RUBBERBAND__ | ||
} | ||
|
||
void DlgPrefSound::updateKeylockDualThreadingCheckbox() { |
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.
This #ifdef
looks suspicious, does it work as intended? I'm getting some errors building for iOS (without Rubberband)...
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.
Seems to work, but the brace placement makes this confusing. I've fixed this and some related issues in
Currently the
m_settingsModified
is set inDlgPrefSound::slotUpdate()
which will cause a sound pause when pressing Apply in any pref page, becauseDlgPrefSound::slotApply()
would reapply the current (unchanged) sound config.#13070 (comment)
Fixed by moving the keylock dual-threading checkbox update to a dedicated method and call then when necessary.
@acolombier please check if I got everything right.