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

Android: Use config changed callback for tracking recursive scan setting #11663

Merged

Conversation

JosJuice
Copy link
Member

This way the Settings class doesn't contain a hardcoded reference to a specific setting. And Settings.loadSettings no longer calls getBoolean, which is a step towards fixing the crash when recreating EmulationActivity after process death.

@JosJuice JosJuice force-pushed the android-config-change-callback branch 2 times, most recently from 7a8a169 to 30e4968 Compare March 16, 2023 09:29
@shuffle2
Copy link
Contributor

what thread does that execute on? it's not fault of this pr, but keep in mind config callbacks are currently totally broken/not thread safe: https://bugs.dolphin-emu.org/issues/13141

@JosJuice
Copy link
Member Author

I'm using new Handler(Looper.getMainLooper()).post in the callback, which ensures that the code will run in the UI thread regardless of what thread the callback was called from.

@JosJuice JosJuice force-pushed the android-config-change-callback branch from 30e4968 to 4f0132f Compare March 19, 2023 16:26
@lioncash
Copy link
Member

lioncash commented Dec 7, 2023

Just want to make sure if this is still relevant or needs any changes due to other merged android changes before merging (given it's been 9 months)

@JosJuice
Copy link
Member Author

JosJuice commented Dec 7, 2023

Yes, it's still relevant. Fixing the fact that config callbacks technically aren't thread safe would be nice, but it's very complex and I consider it out of scope for this PR. This PR doesn't interact with any merged Android PRs as far as I know, but I'll rebase it so we get a check that it still builds and all.

@JosJuice JosJuice force-pushed the android-config-change-callback branch from 4f0132f to d09d256 Compare December 7, 2023 19:29
This way the Settings class doesn't contain a hardcoded reference to
a specific setting. And Settings.loadSettings no longer calls
getBoolean, which is a step towards fixing the crash when recreating
EmulationActivity after process death.
@JosJuice
Copy link
Member Author

JosJuice commented Dec 7, 2023

The declaration of the callback ID needed changing because of the merge of 7197e3a. Should be good to go now.

@JosJuice JosJuice force-pushed the android-config-change-callback branch from d09d256 to 4203632 Compare December 7, 2023 20:11
@lioncash lioncash merged commit e0f4111 into dolphin-emu:master Dec 7, 2023
11 checks passed
@JosJuice JosJuice deleted the android-config-change-callback branch December 8, 2023 16:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants