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: Implement DocumentProvider #11524

Merged
merged 1 commit into from Mar 11, 2023

Conversation

K0bin
Copy link
Contributor

@K0bin K0bin commented Feb 1, 2023

Apparently, being unable to access the Dolphin user folder (to install texture mods for example) is one thing that drives users to forks that shall not be named because those revert the scoped storage changes.

The Play Store and newer Android versions force us to implement scoped storage, so we place Dolphins user files in /sdcard/Android/data/org.dolphinemu.dolphinemu. On Android 13, this folder is not accessible to users.

So we just adopt the same solution as Skyline and RetroArch, we implement a custom DocumentProvier. This makes Dolphin show up as a file provider in the Android file manager and allows users to do things like editing the ini files or install texture mods.

cc @t895

@mbc07
Copy link
Contributor

mbc07 commented Feb 1, 2023

So, I'm currently working on a different approach for this problem: moving the user folder from /sdcard/Android/data/org.dolphinemu.dolphinemu to /sdcard/Android/media/org.dolphinemu.dolphinemu. That folder still complies with the Scoped Storage requirements and is freely accessible via any file manager app, even on Android 13.

The actual directory change is fairly simple and I had it sitting on a local repo since early December at least. Hadn't opened a public PR yet because I'm currently struggling a bit with providing migration logic to users coming from previous Dolphin versions, which still used Android/data.

About the DocumentProvider approach, it's also valid, but AFAICT it would still limit users to the system file manager (DocumentsUI), which is a bit barebones and doesn't seem to accept gamepad input, meaning it would be a bit troublesome to use on the Shield TV (yep, it does provide a DocumentsUI system app, unlikely other Android TV devices)...

@K0bin
Copy link
Contributor Author

K0bin commented Feb 3, 2023

@mbc07 That sounds like another way to do it. I personally prefer my approach (putting it into Media kinda feels wrong because it's not media) but I'd be okay with either way.

@mbc07
Copy link
Contributor

mbc07 commented Feb 3, 2023

Although it feels wrong, I went with the Android/media route because it's the closest to what we had before Scoped Storage got enforced (any app can access it without workarounds). Also, a couple of apps I have went with this route, so I doubt Google will be removing that anytime soon.

Anyway, maybe we can have both options? I pulled your changes locally and your DocumentProvider just worked with Android/media, without further changes. That way we have a backup solution in place if Google decides Android/media should get restricted in future...

@K0bin
Copy link
Contributor Author

K0bin commented Feb 3, 2023

Yeah, the DocumentProvider is very self contained and not that much code, so there isn't really any reason not to do that too.

@Starlightbotanist
Copy link

Starlightbotanist commented Feb 7, 2023

I tried to break this on android 10 and 13, couldn't find any issues. Everything seems to just work.

Tested:
INI edits
copying files into and out of the dolphin document provider such as:
texture packs
Riivolution mods
save game files

The android/media solution is interesting for those without documentsUI. Document provider is faster/simpler on devices that do have documentsUI because you don't have to navigate down into the org.dolphinemu.dolphinemu folder which can be intimidating for some users, due to the large number of other app folders in that directory.

@K0bin
Copy link
Contributor Author

K0bin commented Feb 7, 2023

Thanks for testing @Starlightbotanist.

@K0bin K0bin force-pushed the document-provider branch 2 times, most recently from cced8be to ff604d0 Compare February 13, 2023 18:57
@K0bin
Copy link
Contributor Author

K0bin commented Feb 13, 2023

I changed it to replace files that aren't save files or save states instead of creating duplicate files. That fits the use case for this a lot better.

@K0bin K0bin marked this pull request as ready for review February 13, 2023 18:58
Copy link
Member

@delroth delroth left a comment

Choose a reason for hiding this comment

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

Just a quick drive-by comment...

@K0bin K0bin force-pushed the document-provider branch 2 times, most recently from dda32d1 to 1b612b3 Compare March 8, 2023 18:43
@K0bin K0bin force-pushed the document-provider branch 2 times, most recently from bc09535 to 3f5bd65 Compare March 8, 2023 20:06
Copy link
Contributor

@t895 t895 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@JosJuice JosJuice left a comment

Choose a reason for hiding this comment

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

Thanks for implementing this. I considered implementing it back when scoped storage was new, but considered it not worth the effort... but since then the situation around being able to access the folder has gotten much worse, so it's good to have this now.

This allows users to access the Dolphin user directory.
@JosJuice JosJuice merged commit 804b94e into dolphin-emu:master Mar 11, 2023
@K0bin K0bin deleted the document-provider branch March 11, 2023 18:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
6 participants