Android: Add Paths to UI #7920

Ebola16 · 2019-03-22T16:27:52Z

Implements https://bugs.dolphin-emu.org/issues/10781

Android: Add Set as Default ISO to UI
Android: Add Paths to UI
Very useful for loading game mods
Removed some unnecessary import statements
Renamed some things now that FilePicker is more versatile
Added ConfirmRunnable to confirm running arbitrary runnables (so they're not accidentally clicked). It will also be useful for future PRs, especially when modifying multiple settings (i.e. toggling all log types).
Fixed a bug where launchOpenFileActivity() could select multiple files. We don't want to be able to load multiple games at once!
Review requested: See the TODO added to SettingsActivity. Comment out the finish() to see the weirdness. My workaround works but it probably isn't the prettiest!

JosJuice · 2019-03-22T16:32:55Z

I don't think we should have this without having the default ISO exposed in the settings, since there otherwise would be no way to clear it without editing Dolphin.ini.

Source/Android/app/src/main/res/values/arrays.xml

Ebola16 · 2019-03-22T19:43:10Z

There's no play button on Android so is there a reason to clear the default ISO setting? Other than the play button, I think only mod launchers care about this setting and it can now be easily changed from Dolphin's UI. But we could wait on this PR until the Paths submenu is implemented.

leoetlino · 2019-05-11T12:35:16Z

Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/dialogs/GameSettingsDialog.java

@@ -64,14 +68,24 @@ public Dialog onCreateDialog(Bundle savedInstanceState)
                    SettingsActivity.launch(getActivity(), MenuTag.WIIMOTE, gameId);
                  break;
                case 4:
-                  clearGameSettings(gameId);
+                  // Set as default ISO for GC, Wii clear game settings for else


This seems very error prone to me... Is there no other way to tell what option was selected? Not a big fan of those unnamed indices which are essentially magic numbers that can even change depending on the game file platform (!)

I moved my changes to earlier in the array to reduce ugliness. The cases aren't named because they change depending upon platform. I think the goal was to conditionally include Wiimote settings while keeping Clear Game Settings at the end of the array. I could separate the Game Properties .setItems by platform but then I'd need to duplicate the switch. There's only one case that differs between platforms so I'm thinking I should keep this as-is. Let me know if you disagree.

Ebola16 · 2020-01-17T11:26:46Z

I don't think we should have this without having the default ISO exposed in the settings, since there otherwise would be no way to clear it without editing Dolphin.ini.

I took care of @JosJuice's concerns. I expanded the scope of this PR to include the Paths submenu. See updated original description for noteworthy changes. Requesting a review!

Ebola16 · 2020-01-23T18:23:09Z

Slight comment clarification. Although the changes in SettingsActivity may be somewhat wonky, I don't think there's any drawbacks to this PR so ready for review!

Ebola16 · 2020-01-31T02:27:26Z

I realize the few Android devs may be busy but ...anyone?

JosJuice

There is still no way to clear the Default ISO setting as far as I can tell.

The behavior of going back to the game list once you've selected OK or Cancel in the file picker feels quite weird to me. I'm assuming this is related to the TODO comments in SettingsActivity?

.../Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/CustomFilePickerFragment.java

Source/Android/app/src/main/res/values/strings.xml

Ebola16 · 2020-02-13T01:05:46Z

I implemented clearing the default ISO as a runnable that asks the user to confirm the action (see third screenshot). This will be used in future PRs too (i.e. toggling all log types).

I agree that the behavior in SettingsActivity is weird but it's the best I can come up with at the moment. That covers two of the TODOs I added. With the most recent comment, I've added a third TODO in SettingsFragment for a minor graphical glitch. I'm open to suggestions for improvement, but I think this can be merged and I'll do follow-up PRs if I can address the TODOs. This PR is getting fairly large.

I've done things in Java that I haven't tried before in this PR so it should probably be given a thorough look.

Ebola16 · 2020-03-17T16:52:16Z

...it seems nobody has any other comments on this PR so can it be merged?

JosJuice

Sorry for the delay.

.../src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsActivityPresenter.java

...ndroid/app/src/main/java/org/dolphinemu/dolphinemu/features/settings/ui/SettingsAdapter.java

JosJuice · 2020-03-21T21:37:27Z

I found a bug: If you first go to the paths settings and bring up the file picker for the SD card path (and press either Cancel or OK), and then go to add a game directory to the game list, the directory picker will only show .raw files instead of only game files.

The logic for which set of extensions to use seems quite fragile, with CustomFilePickerFragment (which is not part of the settings code) relying on the value of a static variable in SettingsAdapter. If it's possible to do it without static variables like that, that would be preferable.

Ebola16 · 2020-03-22T10:24:07Z

The bug you described has been fixed by clearing the static variables in SettingsActivity. Unfortunately I think the static variables are necessary here to avoid multiple inheritance problems.
The latest version of this PR:

Adds some static variables I accidentally removed in the previous version in SettingsAdapter. FilePicker should now properly interact with directory selection.
Repurposed clearDefaultISO to resetPaths (see second image in original post). It's silly to just reset one path. If users are messing around with these settings we might as well easily restore them all.

Hopefully the latest version addresses everything. I think this is good to go and I received feedback so I'm removing my [Feedback Wanted] label.

JosJuice · 2020-03-22T19:08:56Z

I have made a commit that makes CustomFilePickerFragment not rely on the static state in SettingsAdapter. If you could include it, or something similar to it, I think I would be happy with merging the PR.

I've checked that it works, but I haven't lint checked it, so maybe you'll need to edit the formatting of one or two lines.

JosJuice@779be10

Ebola16 · 2020-03-24T02:05:37Z

@JosJuice I pushed one more commit to fix onSettingChanged could still clobber changes made by ConfirmRunnable. The main functional changes are ConfirmRunnable now calls finish upon completion and yet another static variable has been added to resetPaths. This also allows me to remove the now unnecessary reloadSubMenu and associated changes.

I would prefer to not squash these commits in case I need to partially revert them in the future.

Ebola16 · 2020-03-25T21:40:34Z

@JosJuice can we get this merged? I have a feeling nobody else is going to review this PR.

ds22x · 2020-04-06T15:27:44Z

Pardon my intrusion, but this PR made me wonder if it were possible to set a custom path for the complete dolphin-emu, rather than just certain chunks.
Alternatively, the option for more folders to be given a custom path (like the GC folder).

Ebola16 · 2020-04-11T12:49:16Z

@ds22x That's likely possible, although using variables in paths may confuse users. I don't know if I'll get around to implementing that but you can file a feature request at https://bugs.dolphin-emu.org/projects/emulator/issues so people can better see your suggestion.

ds22x · 2020-04-11T13:18:34Z

And done.
https://bugs.dolphin-emu.org/issues/12046

JosJuice reviewed Mar 22, 2019

View reviewed changes

Source/Android/app/src/main/res/values/arrays.xml Outdated Show resolved Hide resolved

Ebola16 changed the title ~~Android: Add Set as default ISO to UI~~ Android: Add Set As Default ISO to UI Mar 22, 2019

Ebola16 force-pushed the DISO branch from c3052fd to 37f026c Compare March 22, 2019 16:42

Ebola16 changed the title ~~Android: Add Set As Default ISO to UI~~ Android: Add Set as Default ISO to UI Mar 22, 2019

Ebola16 force-pushed the DISO branch 2 times, most recently from a5a696b to 462cf6c Compare March 22, 2019 18:05

leoetlino reviewed May 11, 2019

View reviewed changes

Ebola16 force-pushed the DISO branch from 462cf6c to 2e2a5b1 Compare August 21, 2019 17:15

Ebola16 changed the title ~~Android: Add Set as Default ISO to UI~~ [WIP] Android: Add Set as Default ISO to UI Aug 30, 2019

Ebola16 force-pushed the DISO branch 2 times, most recently from 9695c2b to aa802eb Compare September 8, 2019 06:14

Ebola16 force-pushed the DISO branch from aa802eb to a48a2df Compare October 2, 2019 19:29

Ebola16 force-pushed the DISO branch 2 times, most recently from 4ff54a0 to fe8d8f0 Compare January 5, 2020 22:14

Ebola16 force-pushed the DISO branch 2 times, most recently from 6a4923a to d0c006a Compare January 17, 2020 10:47

Ebola16 changed the title ~~[WIP] Android: Add Set as Default ISO to UI~~ [Feedback Wanted] Android: Add Paths to UI Jan 17, 2020

Ebola16 force-pushed the DISO branch from d0c006a to 8a3fda0 Compare January 23, 2020 18:20

Ebola16 force-pushed the DISO branch from 8a3fda0 to da0a5be Compare January 24, 2020 04:14

JosJuice reviewed Feb 1, 2020

View reviewed changes

.../Android/app/src/main/java/org/dolphinemu/dolphinemu/fragments/CustomFilePickerFragment.java Outdated Show resolved Hide resolved

Source/Android/app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved

Ebola16 force-pushed the DISO branch 3 times, most recently from f2b9665 to 44129b1 Compare February 12, 2020 23:58

Ebola16 force-pushed the DISO branch from 44129b1 to 13e1808 Compare February 13, 2020 03:17

Ebola16 force-pushed the DISO branch from 240bec8 to 6f64afa Compare March 10, 2020 18:37

Android: Add Set as Default ISO to UI

38fa38d

Ebola16 force-pushed the DISO branch from 6f64afa to e2f14db Compare March 11, 2020 05:07

leoetlino requested a review from JosJuice March 18, 2020 11:06

JosJuice reviewed Mar 21, 2020

View reviewed changes

Ebola16 force-pushed the DISO branch 2 times, most recently from 0258f19 to 679b35e Compare March 22, 2020 10:01

Ebola16 changed the title ~~[Feedback Wanted] Android: Add Paths to UI~~ Android: Add Paths to UI Mar 22, 2020

Android: Add Paths to UI

ccda75f

Ebola16 force-pushed the DISO branch from 679b35e to ccda75f Compare March 22, 2020 10:43

Ebola16 force-pushed the DISO branch 2 times, most recently from a9ffeac to c25b4c5 Compare March 23, 2020 04:48

Android: Don't use SettingsAdapter's static state in file picker

de5430b

Ebola16 force-pushed the DISO branch from c25b4c5 to de5430b Compare March 23, 2020 04:53

JosJuice approved these changes Mar 23, 2020

View reviewed changes

Android: Don't let onSettingChanged clobber ConfirmRunnable changes

d15d6e7

Ebola16 force-pushed the DISO branch from 1195c10 to d15d6e7 Compare March 24, 2020 21:09

JosJuice merged commit 1b97f08 into dolphin-emu:master Mar 29, 2020

Ebola16 deleted the DISO branch March 29, 2020 19:02

Ebola16 mentioned this pull request Mar 30, 2020

Android: Add Insert SD Card and update the description #8708

Merged

Ebola16 mentioned this pull request Aug 7, 2020

Android: Hook up the new config system #8975

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Android: Add Paths to UI #7920

Android: Add Paths to UI #7920

Ebola16 commented Mar 22, 2019 •

edited

Loading

JosJuice commented Mar 22, 2019 •

edited

Loading

Ebola16 commented Mar 22, 2019

leoetlino May 11, 2019

Ebola16 Jan 5, 2020

Ebola16 commented Jan 17, 2020

Ebola16 commented Jan 23, 2020

Ebola16 commented Jan 31, 2020

JosJuice left a comment

Ebola16 commented Feb 13, 2020 •

edited

Loading

Ebola16 commented Mar 17, 2020

JosJuice left a comment

JosJuice commented Mar 21, 2020 •

edited

Loading

Ebola16 commented Mar 22, 2020

JosJuice commented Mar 22, 2020

Ebola16 commented Mar 24, 2020 •

edited

Loading

Ebola16 commented Mar 25, 2020

ds22x commented Apr 6, 2020

Ebola16 commented Apr 11, 2020

ds22x commented Apr 11, 2020

Android: Add Paths to UI #7920

Android: Add Paths to UI #7920

Conversation

Ebola16 commented Mar 22, 2019 • edited Loading

JosJuice commented Mar 22, 2019 • edited Loading

Ebola16 commented Mar 22, 2019

leoetlino May 11, 2019

Choose a reason for hiding this comment

Ebola16 Jan 5, 2020

Choose a reason for hiding this comment

Ebola16 commented Jan 17, 2020

Ebola16 commented Jan 23, 2020

Ebola16 commented Jan 31, 2020

JosJuice left a comment

Choose a reason for hiding this comment

Ebola16 commented Feb 13, 2020 • edited Loading

Ebola16 commented Mar 17, 2020

JosJuice left a comment

Choose a reason for hiding this comment

JosJuice commented Mar 21, 2020 • edited Loading

Ebola16 commented Mar 22, 2020

JosJuice commented Mar 22, 2020

Ebola16 commented Mar 24, 2020 • edited Loading

Ebola16 commented Mar 25, 2020

ds22x commented Apr 6, 2020

Ebola16 commented Apr 11, 2020

ds22x commented Apr 11, 2020

Ebola16 commented Mar 22, 2019 •

edited

Loading

JosJuice commented Mar 22, 2019 •

edited

Loading

Ebola16 commented Feb 13, 2020 •

edited

Loading

JosJuice commented Mar 21, 2020 •

edited

Loading

Ebola16 commented Mar 24, 2020 •

edited

Loading