From 7652023407679cb5c9723ca006c1f88f847ae8db Mon Sep 17 00:00:00 2001 From: JosJuice Date: Mon, 17 Jun 2019 15:31:20 +0200 Subject: [PATCH 1/3] Android: Don't copy global INIs into game INIs See the source code comment in the next commit for why this is bad. --- .../features/settings/model/Settings.java | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java index d76d9ec688..3eb9e1d2f7 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java @@ -97,31 +97,23 @@ public class Settings { sections = new Settings.SettingsSectionMap(); - HashSet filesToExclude = new HashSet<>(); - if (!TextUtils.isEmpty(gameId)) + if (TextUtils.isEmpty(gameId)) { - // for per-game settings, don't load the WiiMoteNew.ini settings - filesToExclude.add(SettingsFile.FILE_NAME_WIIMOTE); + loadDolphinSettings(view); } - - loadDolphinSettings(view, filesToExclude); - - if (!TextUtils.isEmpty(gameId)) + else { loadGenericGameSettings(gameId, view); loadCustomGameSettings(gameId, view); } } - private void loadDolphinSettings(SettingsActivityView view, HashSet filesToExclude) + private void loadDolphinSettings(SettingsActivityView view) { for (Map.Entry> entry : configFileSectionsMap.entrySet()) { String fileName = entry.getKey(); - if (filesToExclude == null || !filesToExclude.contains(fileName)) - { - sections.putAll(SettingsFile.readFile(fileName, view)); - } + sections.putAll(SettingsFile.readFile(fileName, view)); } } From 7f841e9bfd6dad709335ab7937fa03279d47adb4 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Tue, 18 Jun 2019 12:42:00 +0200 Subject: [PATCH 2/3] Android: Suggest deleting game INIs if they contain global INI data --- .../dolphinemu/dolphinemu/NativeLibrary.java | 2 ++ .../features/settings/model/Settings.java | 32 +++++++++++++++++++ .../settings/ui/SettingsActivity.java | 13 ++++++++ .../ui/SettingsActivityPresenter.java | 11 +++++++ .../settings/ui/SettingsActivityView.java | 5 +++ .../features/settings/utils/SettingsFile.java | 3 +- .../app/src/main/res/values/strings.xml | 2 ++ Source/Android/jni/MainAndroid.cpp | 6 ++++ 8 files changed, 73 insertions(+), 1 deletion(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java index b901f2f691..174d2d1ff2 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/NativeLibrary.java @@ -253,6 +253,8 @@ public final class NativeLibrary Rumble.checkRumble(padID, state); } + public static native void NewGameIniFile(); + public static native void LoadGameIniFile(String gameId); public static native void SaveGameIniFile(String gameId); diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java index 3eb9e1d2f7..6b0d3acfd3 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java @@ -184,4 +184,36 @@ public class Settings SettingsFile.saveCustomGameSettings(gameId, sections); } } + + public void clearSettings() + { + sections.clear(); + } + + public boolean gameIniContainsJunk() + { + // Older versions of Android Dolphin would copy the entire contents of most of the global INIs + // into any game INI that got saved (with some of the sections renamed to match the game INI + // section names). The problems with this are twofold: + // + // 1. The user game INIs will contain entries that Dolphin doesn't support reading from + // game INIs. This is annoying when editing game INIs manually but shouldn't really be + // a problem for those who only use the GUI. + // + // 2. Global settings will stick around in user game INIs. For instance, if someone wants to + // change the texture cache accuracy to safe for all games, they have to edit not only the + // global settings but also every single game INI they have created, since the old value of + // the texture cache accuracy setting has been copied into every user game INI. + // + // These problems are serious enough that we should detect and delete such INI files. + // Problem 1 is easy to detect, but due to the nature of problem 2, it's unfortunately not + // possible to know which lines were added intentionally by the user and which lines were added + // unintentionally, which is why we have to delete the whole file in order to fix everything. + + if (TextUtils.isEmpty(gameId)) + return false; + + SettingSection interfaceSection = sections.get("Interface"); + return interfaceSection != null && interfaceSection.getSetting("ThemeName") != null; + } } diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivity.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivity.java index 78b8ef2d1f..56b407cb82 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivity.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivity.java @@ -1,5 +1,6 @@ package org.dolphinemu.dolphinemu.features.settings.ui; +import android.app.AlertDialog; import android.app.ProgressDialog; import android.content.Context; import android.content.Intent; @@ -184,6 +185,18 @@ public final class SettingsActivity extends AppCompatActivity implements Setting .show(); } + @Override + public void showGameIniJunkDeletionQuestion() + { + new AlertDialog.Builder(this) + .setTitle(getString(R.string.game_ini_junk_title)) + .setMessage(getString(R.string.game_ini_junk_question)) + .setPositiveButton(R.string.yes, (dialogInterface, i) -> mPresenter.clearSettings()) + .setNegativeButton(R.string.no, null) + .create() + .show(); + } + @Override public org.dolphinemu.dolphinemu.features.settings.model.Settings getSettings() { diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivityPresenter.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivityPresenter.java index 57bbe9d6cc..21115e464d 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivityPresenter.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivityPresenter.java @@ -58,6 +58,11 @@ public final class SettingsActivityPresenter if (!TextUtils.isEmpty(gameId)) { mSettings.loadSettings(gameId, mView); + + if (mSettings.gameIniContainsJunk()) + { + mView.showGameIniJunkDeletionQuestion(); + } } else { @@ -118,6 +123,12 @@ public final class SettingsActivityPresenter return mSettings; } + public void clearSettings() + { + mSettings.clearSettings(); + onSettingChanged(); + } + public void onStop(boolean finishing) { if (directoryStateReceiver != null) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivityView.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivityView.java index 910c9f0014..71583aa989 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivityView.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivityView.java @@ -119,6 +119,11 @@ public interface SettingsActivityView */ void showExternalStorageNotMountedHint(); + /** + * Tell the user that there is junk in the game INI and ask if they want to delete the whole file. + */ + void showGameIniJunkDeletionQuestion(); + /** * Start the DirectoryInitialization and listen for the result. * diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/utils/SettingsFile.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/utils/SettingsFile.java index 7ba9bcfd38..54640a3710 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/utils/SettingsFile.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/utils/SettingsFile.java @@ -452,7 +452,8 @@ public final class SettingsFile final HashMap sections) { Set sortedSections = new TreeSet<>(sections.keySet()); - NativeLibrary.LoadGameIniFile(gameId); + + NativeLibrary.NewGameIniFile(); for (String sectionKey : sortedSections) { SettingSection section = sections.get(sectionKey); diff --git a/Source/Android/app/src/main/res/values/strings.xml b/Source/Android/app/src/main/res/values/strings.xml index fae6455244..c64446fd8d 100644 --- a/Source/Android/app/src/main/res/values/strings.xml +++ b/Source/Android/app/src/main/res/values/strings.xml @@ -274,6 +274,8 @@ Settings Game Settings Extension Bindings + Junk Data Found + The settings file for this game contains junk data created by an old version of Dolphin. Would you like to fix this by deleting the settings file for this game? This cannot be undone. Take Screenshot diff --git a/Source/Android/jni/MainAndroid.cpp b/Source/Android/jni/MainAndroid.cpp index e0fce56fd9..3fcae32df8 100644 --- a/Source/Android/jni/MainAndroid.cpp +++ b/Source/Android/jni/MainAndroid.cpp @@ -357,6 +357,12 @@ JNIEXPORT jstring JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_GetUserSe return ToJString(env, value.c_str()); } +JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_NewGameIniFile(JNIEnv* env, + jobject obj) +{ + s_ini = IniFile(); +} + JNIEXPORT void JNICALL Java_org_dolphinemu_dolphinemu_NativeLibrary_LoadGameIniFile(JNIEnv* env, jobject obj, jstring jGameID) From 36166c9b4f7e72b5f9e9a00cb1326a442a4a171e Mon Sep 17 00:00:00 2001 From: JosJuice Date: Tue, 18 Jun 2019 14:37:49 +0200 Subject: [PATCH 3/3] Android: Don't copy default game INIs into user game INIs This isn't as serious as copying global INIs into user game INIs, but still not good. We want to be able to remove settings from default game INIs and have those removals apply. --- .../dolphinemu/dolphinemu/features/settings/model/Settings.java | 1 - 1 file changed, 1 deletion(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java index 6b0d3acfd3..ca34fc575b 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/model/Settings.java @@ -103,7 +103,6 @@ public class Settings } else { - loadGenericGameSettings(gameId, view); loadCustomGameSettings(gameId, view); } }