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: Android INI section and make platform tab selection an INI setting #8803

Merged
merged 2 commits into from Jun 21, 2020

Conversation

Ebola16
Copy link
Member

@Ebola16 Ebola16 commented May 9, 2020

Implements https://bugs.dolphin-emu.org/issues/10793
Prerequisite for https://bugs.dolphin-emu.org/issues/10957

Android-specific settings should be moved away from SharedPreferences and use INI settings. This allows these settings to be retained across multiple builds of Dolphin. I added an Android section to Dolphin.ini to make these settings easy to find. Most of the setting changes will happen in future PRs.

Android: Make last platform tab selection an INI setting was a test case. Now Dolphin will remember which platform tab was selected when (re)installing or starting Dolphin.

@JosJuice
Copy link
Member

JosJuice commented May 9, 2020

Adding a new INI section and putting the current tab selection in it makes sense, but why are you moving those two existing settings to the new INI section? For the savestates enabled setting, it is true that that setting currently only exists on Android, so it is debatable, but moving it resets the setting and it might make sense to make that setting available on non-Android in the future. For the DSP setting, I don't see how it would work correctly after the move.

@Ebola16
Copy link
Member Author

Ebola16 commented May 9, 2020

I can see the reasoning to leave the savestate setting alone.
I moved the DSP setting since the "master" setting is Android-specific. The logic based on how that "master" setting changes the universal DSP settings still works from my testing. Is this a bad idea?

@JosJuice
Copy link
Member

JosJuice commented May 9, 2020

I moved the DSP setting since the "master" setting is Android-specific. The logic based on how that "master" setting changes the universal DSP settings still works from my testing. Is this a bad idea?

Oh, sorry, I thought it was a normal DSP setting and not a special Android one. I suppose it makes sense then (though maybe this setting actually shouldn't exist, if possible... But that's out of scope for this PR.)

@Ebola16
Copy link
Member Author

Ebola16 commented May 9, 2020

Reverted the savestate change and kept the DSP Android-specific changes. I didn't want to make changes to the way Dolphin handles DSP settings in a PR that made Android UI changes so that's for a future PR. Unifying the settings will likely involve some INI clutter regardless of this PR.

@JosJuice JosJuice merged commit 8ce2576 into dolphin-emu:master Jun 21, 2020
@Ebola16 Ebola16 deleted the ANDROID branch June 21, 2020 16:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants