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

Qt/GeneralPane: Don't trigger config change events when populating GUI. #10484

Merged

Conversation

AdmiralCurtiss
Copy link
Contributor

I ran into this while implementing a 'reset all settings' feature. Turns out that blocking signals on QWidget doesn't block events from its children, so what happened is that I called reset settings, all settings were reset, then the GUI got the request to update itself, and during the update one of the GUI elements called its 'I have been changed' signal, which stored the current GUI back to the settings, reverting my reset. Not ideal!

Is there a better way to handle this? This is the best I could come up with, but maybe someone has a less boilerplate-y idea...

@AdmiralCurtiss
Copy link
Contributor Author

After some discussion on IRC we decided on this approach being the least boilerplate-y.

Copy link
Member

@lioncash lioncash left a comment

Choose a reason for hiding this comment

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

Is there a better way to handle this?

Unfortunately there's no other better way to do this. There's no way to disable all signals from children on a parent-level (as far as I know)

Source/Core/DolphinQt/Settings/GeneralPane.cpp Outdated Show resolved Hide resolved
@mbc07
Copy link
Contributor

mbc07 commented Feb 28, 2022

Wouldn't it be easier to just trigger an app relaunch after resetting the settings?

@AdmiralCurtiss
Copy link
Contributor Author

I mean this technically doesn't apply to just a settings reset, this can always happen if multiple options are changed at once (through, say, a GameINI being loaded) -- it's just that so far this situation has been pretty unlikely as the only option on this panel that really makes sense to adjust on a GameINI level is dual core.

Besides, isn't it much nicer UX if you can reset settings (or possibly load a settings preset) on the fly?

@leoetlino leoetlino merged commit ebfee3b into dolphin-emu:master Mar 2, 2022
10 checks passed
@AdmiralCurtiss AdmiralCurtiss deleted the general-pane-signalblocker branch April 18, 2022 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants