Opened 5 months ago

Last modified 10 hours ago

#12727 new defect

ANDROID: Allow Save location in SDcard (Write permission on SD)

Reported by: LukasThyWalls Owned by:
Priority: normal Component: Port: Android
Version: Keywords: Android, SDCard, Write permission, Savegames, Saves, Overwrite
Cc: Game:

Description

Hello.

I'm testing ScummVM 2.3.0git20431-g032f9e3857 Android.

It's something that should have been discussed in the past, but there i didn't find any ticket related to it.

The point is you can have the games and extra files anywhere in the SDcard, but not the savegames. The reason is because ScummVM doesn't have write access to the SDcard.

In my phone (Xiaomi Mi 9 Lite, Android 10/MIUI 12.0.7, non-rooted), when i go choose any directory, and go up all it allows, it appears five entries:

  • /storage/9016-4EF8--> (real SDcard root directory)
  • ScummVM data/ --> /data (Empty, no access)
  • ScummVM data (Ext)/ --> /storage/emulated/0/Android/data/org.scummvm.scummvm/files (Internal storage, empty)
  • ScummVM data (Int)/ --> /data/user/0/org.scummvm.scummvm/files (It has the other ScummVM files)
  • sdCard/ --> /mnt/sdcard (not the sdcard, is the internal storage root directory)

I point this out because maybe is something with my phone, but i don't think so. But those redirection doesn't seems right also (and maybe is topic for another ticket, but if it's only this, it seems something with low priority).

Thanks.

Attachments (1)

logcat_scummvm_chooseSavesPathInSD.txt (58.5 KB ) - added by LukasThyWalls 5 months ago.

Download all attachments as: .zip

Change History (15)

comment:1 by antoniou79, 5 months ago

Summary: Allow Save location in SDcard (Write permission on SD)ANDROID: Allow Save location in SDcard (Write permission on SD)

The first and primary issue of the ticket, ie. writing permission on the SD Card, is an issue on "newer" Android OS. The request has to go through Android's Storage Access Framework.

I can reproduce a bug on this, but at the very least, when trying to gain storage access ScummVM should throw you to a file system "browser" to set a folder that can be accessed (typically you set this to the folder or parent or ancestor folder of the folder --or if you don't care that much, then the SD card root folder-- where you intend to save your games). Access would work recursively beyond the folder you set; that is, it will include access to its children folders.

Can you confirm that you got this redirection to an interface in order to set a folder to allow access?

I think that setting the SD card root folder should work, because I know I've tested this originally. There could be a regression to this though caused by another commit. In my testing today I chose a folder other than the SD card root and I can see that there's a bug there (the folder is 'retrieved' with the path "encoded" which means path slashes and colons are converted to their "%2A" "%3A" encoded versions. I will attempt to fix this and report back. Edit: No, the issue is elsewhere and look like it could be tricky to resolve. That code also needs some additions, more testing and optimizations, so that may take a while.

The second issue you're reporting it's the intended behavior. We put it there so that people would not navigate to a "/" or an empty path "" and couldn't recover from there. Some of those shortcuts are expected to not be useful (to most users anyway), or even redirect to the same thing, but other than that I don't see a bug in the feature. If you still think there's an issue there, please elaborate further.

Last edited 5 months ago by antoniou79 (previous) (diff)

comment:2 by LukasThyWalls, 5 months ago

Can you confirm that you got this redirection to an interface in order to set a folder to allow access?

No, i am not getting it. I tried to choose the (real) sdcard directory root (/storage/9016-4EF8/), the directory i want (/Juegos/ScummVM/Saves/) but i'm not getting a "system" browser to allow writing. The only thing i have is a message saying "The chosen directory cannot be written to. Please select another one".
Other apps like SyncMe Wireless or NewPipe do the thing you described and they do it and then they can use the SD to write, but ScummVM is not doing this.

... There could be a regression...

It's something i wanted to report, because is happening for a while. I have this phone since more than two years and it never do this if i recall.
I didn't report earlier because i remember reading something about this issue and people from ScummVM team saying they don't wanna fix it because is an issue of Android itself for changing the framework again and again and they don't wanna spend time in their constant changes... but it has been a while since then, and now i remembered this and i don't find any of that messages here, in open or closed tickets. Maybe they are in the forums, or i have dreamed them...

The second issue you're reporting it's the intended behavior. We put it there so that people would not navigate to a "/" or an empty path "" and couldn't recover from there. Some of those shortcuts are expected to not be useful (to most users anyway), or even redirect to the same thing, but other than that I don't see a bug in the feature. If you still think there's an issue there, please elaborate further.

I point that out because maybe is related to the other issue, but if you can access to all the directories needed it's not an issue, and looking where go all the shorcuts, you can go where you need.
I'm only saying the names doesn't match where they go and they are a little confussing: At least in my phone, the sdcard is not the real sdcard, is the internal storage, the data (ext) is the ScummVM data in internal storage, and the data (int), is the /data directory in the "system"? storage (or whatever is called, the root linux partition what have all the OS), what also is empty for me because is unnaccessible without root.

in reply to:  2 comment:3 by antoniou79, 5 months ago

Replying to LukasThyWalls:

No, i am not getting it. I tried to choose the (real) sdcard directory root (/storage/9016-4EF8/), the directory i want (/Juegos/ScummVM/Saves/) but i'm not getting a "system" browser to allow writing. The only thing i have is a message saying "The chosen directory cannot be written to. Please select another one".
Other apps like SyncMe Wireless or NewPipe do the thing you described and they do it and then they can use the SD to write, but ScummVM is not doing this.

Then a lot more testing is needed. SAF is supposed to bring up the "System file picker" (unless you've already set a permission with it that should satisfy the specific file access request, at some point in the apps lifetime). I'm on a Redmi Note 9 pro (Android 10), MUI 12.0.4 stable and am getting that picker interface. (There's a new system update pending though, so this is subject to change I guess).

It's just as well, because we need to:

  • add further checks for when a permission is granted not at root level but at folder level (most tests where done with permission at root level)
  • support possible UTF-8 characters in paths
  • test for save game functionality specifically (the feature was mostly tested for the ScummVM LAN server)
  • probably implement a way to revoke permissions granted with this browser or
  • recycle old ones as iirc there's a limit on how many of those permissions we can have for ScummVM

It's something i wanted to report, because is happening for a while. I have this phone since more than two years and it never do this if i recall.
I didn't report earlier because i remember reading something about this issue and people from ScummVM team saying they don't wanna fix it because is an issue of Android itself for changing the framework again and again and they don't wanna spend time in their constant changes... but it has been a while since then, and now i remembered this and i don't find any of that messages here, in open or closed tickets. Maybe they are in the forums, or i have dreamed them...

That would probably be me, complaining about Google API updates. It's not that I don't want to fix the issues that come up. It's that I'm not an Android expert, not a dedicated Android porter and that the solutions are rarely straightforward. And testing requires a lot of time on multiple emulator configurations and devices, and the constant and serious API updates keep requiring more maintenance or updating of the code -- a time that I prefer to spend elsewhere (eg. like the Blade Runner engine).

I do not express the sentiment of the whole team, though. Just my own thoughts on the matter. We all do work on a voluntary basis and each has their own priority on tasks to be worked on. I have a few Android devices and I would like ScummVM to run smoothly on them, so that's my main motivation on working on it.

Also keep in mind, in the last 2 years the Android port got a major update, to bring it up-to-date with the ScummVM core features and new game engines. And we keep hacking at bugs and improving existing features when possible and when time and life allows.

Anyway here's my summary of it. Android (Google) has radically tightened security wrt what an app can do with storage (and other stuff, but here we're mostly interested in storage). Unfortunately the ScummVM paradigm is not really the standard app they have in mind making these changes. And they do keep changing stuff which makes maintenance a chore for developers of apps outside the self-contained app norm, like ScummVM.

Ideally we will keep/fix supporting storing on an SD card in the newer Android versions, which seems like the proper thing to do, but jumping through hoops to achieve that is diminishing for any enthusiasm and motivation.

Also, the listing of directories (the second issue addressed in this ticket) is (again) going to have to change, since as of sometime this year, and in Android 11 and above, some of the functions we use to find the available storage options will cease to work (as deprecated). And this is yet another potentially huge thing to deal with, since Android vendors are very inconsistent in following a standard for external storage, so you cannot have hardcoded paths, and the code for accommodating all cases becomes a spaghetti nightmare.

I point that out because maybe is related to the other issue, but if you can access to all the directories needed it's not an issue, and looking where go all the shorcuts, you can go where you need.
I'm only saying the names doesn't match where they go and they are a little confussing: At least in my phone, the sdcard is not the real sdcard, is the internal storage, the data (ext) is the ScummVM data in internal storage, and the data (int), is the /data directory in the "system"? storage (or whatever is called, the root linux partition what have all the OS), what also is empty for me because is unnaccessible without root.

Again, that's Android for you. And its onomatopoeia.
They have a system "sdcard" folder, but it does not correspond to the user's external sd card (I think it never did).
Then, an Android app has an internal storage and an external storage, but both of those are actually allocated in "internal" storage space. And both will get deleted (wiped) when uninstalling the app. App external storage differs from App internal storage only in that external storage can be accessed from other apps, but internal cannot. Both of these storages are guaranteed to exist -- they don't depend on external physical storage, and the app has access to these spaces always without requiring permissions to read/write. These are the ONLY spaces a modern Android OS app is allowed to write without asking for explicit permissions.

I think the other shortcut ("ScummVM data") is there for compatibility with older versions of Android OS (and ScummVM). So is the "sdcard" shortcut for that matter.

Anyway...
ScummVM has implemented LAN server and Cloud Saving, so those could be used to work around the storage nightmares, at least until we (hopefully) sort out this new API stuff.

Last edited 5 months ago by antoniou79 (previous) (diff)

by LukasThyWalls, 5 months ago

comment:4 by LukasThyWalls, 5 months ago

I added a logcat (Created with Logs, filtered with keyword "ScummVM"), which i start ScummVM, went to the options screen to choose a new saves path, get the error and close. The point is i don't see anything useful for the moment when i pick the directory, but it's a start.

The directory i want to choose is /storage/9016-4EF8/Juegos/ScummVM/Saves/ (Juegos is Games in spanish). I tried every directory down (ScummvM, Juegos, and sdCard root) and anything is bringing the system picker. I also used a Download directory and a Musica ("Music") directory used by other app that SAF worked (SyncMe Wireless).

I tried to clear the storage permission to ScummVM in the permissions manager, but when i start it again ask to take it again and nothing more, anything has changed.

That would probably be me, complaining about Google API updates...
... I do not express the sentiment of the whole team, ...

Don't worry, i complete understand the topic and that point of view. I didn't report that earlier because i want to see if that API or way-to-access-the-SD or whatever, settles down and it's the way to go and there aren't more changes near.
However, i didn't know it was fixed some time ago and i didn't notice earlier because it doesn't worked for me.

About the other "issue", don't worry, as i said i only point that out because is related, but it's ok like it is.

comment:5 by antoniou79, 5 months ago

I should clarify:
The way ScummVM currently works:

  • You will *not* see the System File Picker upon setting the Save Game path.
  • You should see the System File Picker, if you had previously set a Save Game path that requires SAF permission, and you are currently trying to actually Save a game there.
  • You will *not* see the System File Picker, if at some point during the app's lifetime, you saw it and set a permission using it which covers the current access request (to save a game).
  • You should see the System File Picker again, if ScummVM tries to save / write something to a path that is not covered by previously allowed permissions.

I am also not sure if clearing the permissions for the app, also clears the SAF permissions (I think it might not).

Currently, the most certain way to clear those (SAF tokens/permissions) is to either uninstall the app and then reinstall it, or to delete its data (from Android's App Management) -- which is very similar to uninstalling and reinstalling and wipes Added games, configuration and any saved games from internal memory).

But to get to the important point: Are you saying that you were getting the error message upon setting the path in the Path Options tab and not upon actually trying to save a game? If so, this is very helpful, because I was looking elsewhere -- and while there's certainly a bug elsewhere, I didn't understand that the problem reported was occurring earlier.

Last edited 5 months ago by antoniou79 (previous) (diff)

in reply to:  5 comment:6 by LukasThyWalls, 5 months ago

Ok, i have started to see something:

  • You will *not* see the System File Picker upon setting the Save Game path.

OK, i get it. But If i try to set a game path in SDcard, it throws me the error it can't write, and after that it still says "Default" as save game path. I can't choose anything in the SDcard to set any directory...

  • You should see the System File Picker, if you had previously set a Save Game path that requires SAF permission, and you are currently trying to actually Save a game there.

.. and then try to save a game because it will going to save it to "default"!

  • You will *not* see the System File Picker, if at some point during the app's lifetime, you saw it and set a permission using it which covers the current access request (to save a game).

But then it will work. But i can't choose the directory anyway.

  • You should see the System File Picker again, if ScummVM tries to save / write something to a path that is not covered by previously allowed permissions.

That's why i tried to pick the sdcard root, but if ScummVM need to try to save to bring the system picker, first you need to choose that directory in options, and i can't.

I am also not sure if clearing the permissions for the app, also clears the SAF permissions (I think it might not).

Currently, the most certain way to clear those (SAF tokens/permissions) is to either uninstall the app and then reinstall it, or to delete its data (from Android's App Management) -- which is very similar to uninstalling and reinstalling and wipes Added games, configuration and any saved games from internal memory).

I tried, and it asks for storage permissions. I can't confirm the other because i think it didn't take the SAF permissions at any moment.

But to get to the important point: Are you saying that you were getting the error message upon setting the path in the Misc Options tab and not upon actually trying to save a game? If so, this is very helpful, because I was looking elsewhere -- and while there's certainly a bug elsewhere, I didn't understand that the problem reported was occurring earlier.

Summarizing: The error appears when i try to set the save game path to anywhere in the SD, i pick it in the ScummVM file browser, and then it pop up. The second part of the issue is after try to set that and it throw the error, the save game is still "default", so i can't choose any path in the SD and later try to save a game there.


I want to point out the examples of two apps using SAF in my phone:

  • SyncMe Wireless: It have two options in options, "Obtain sd card permissions" and "Revoke sd card permissions". If you choose Obtain, it brings the system file picker and it want to choose the sdcard root, it didn't allow anything else. Since then, you can choose any sdcard directory inside the app.
  • NewPipe: To download videos/audios in the SDcard, you need to choose the directory in options, when you try to choose the directory, it brings the system picker and you can choose where you want, and with the picked directory you have the SAF permissions altogether.

What i was expecting is when you choose the save game path with the ScummVM file browser and press "Choose", if it has write permissions nothing everything is ok, but it hasn't bring the system picker to choose that directory to take permissions.

Last edited 5 months ago by LukasThyWalls (previous) (diff)

comment:7 by antoniou79, 5 months ago

I see. I don't get this error on my phone (I will do more testing and try more path cases though). I can set a path on the SD card (eg. within a path like eg. selecting something under the shortcut for my SD card
/storage/5D9C-C55C/games/Blade Runner/saves).
There is a warning popup prompt there; "Saved games sync doesn't work with non-default directories", and that is necessary, but it won't revert my custom path to default. It's just that later on, the save process will still fail after granting SAF permissions (the bug that I can reproduce)

Yes, I have one or two applications that use SAF. The ones I've seen both urge the user to pick the root folder of the SD card but I am unsure if they allow picking something else (a child folder in the tree).

ScummVM will popup a message recommending the user does this too (pick the root) but it does allow picking a folder in the tree. Problem is, now that I have checked again, both of these cases do have issues, at least with save games (tested in Blade Runner).

ScummVM acquires the permission and keeps the token, but for some reason writing the save game still fails. On the first case (root folder level permission, an empty (awry) save game is created, and on the second one no file is created whatsoever. So, like I've said, this also needs more testing and fixing.

ScummVM has more cases of writing to storage, than just save games. Like I wrote, this feature was primarily tested for the LAN server feature. The idea is whenever ScummVM tries to access something that it has no write permissions on, SAF will fire up and allow the user to grant the permission -- pretty much as SAF is intended to be used. Of course the user can always choose the root folder and be done with this, but that is typically considered not a good security practice. Probably not with ScummVM in mind, but still.

Last edited 5 months ago by antoniou79 (previous) (diff)

comment:8 by LukasThyWalls, 5 months ago

I need to point this:

You will *not* see the System File Picker upon setting the Save Game path.

OK, i get it. But If i try to set a game path in SDcard, it throws me the error it can't write, and after that it still says "Default" as save game path. I can't choose anything in the SDcard to set any directory...

You should see the System File Picker, if you had previously set a Save Game path that requires SAF permission, and you are currently trying to actually Save a game there.

... and then try to save a game because it will going to save it to "default"!

When it shows the popup with the it can't write, the path is not set to "default", it simply is not changed and it shows the same used. I need to said that to clarify, i was testing some paths (ScummVM's Android/Data in internal storage and SDcard, in ScummVM's /data/user/, anywhere in the internal storage) and as it should they work.

The idea is whenever ScummVM tries to access something that it has no write permissions on, SAF will fire up and allow the user to grant the permission -- pretty much as SAF is intended to be used. Of course the user can always choose the root folder and be done with this, but that is typically considered not a good security practice. Probably not with ScummVM in mind, but still.

I understand the SAF needs to be fired up when ScummVM is trying to create/moving something where it hasn't the permissions, and it's a good idea, but i saw not firing it up when you choose the save folder strange, because it will pop out in the middle of the game when you are saving in it (although it only happens once). It's seems more natural in that way, although it does again when is going to do something.
And then it can also be coded the revoke of the permission if you change the folder in settings.
Also choose only the folder for the SAF is the safe measure, for sure.


If a can do anything more to test, tell me.


There is a warning popup prompt there; "Saved games sync doesn't work with non-default directories", and that is necessary

Well... i was thinking to use too the sync save games but with the directory i want to choose this i think i could not.
Thinking out how the LAN Server works, i suppose that is because ScummVM syncs everything in that folder whatever is there, so is a bit concerning about security.
I know this is pretty off-topic, but both ScummVM cloud sync and ScummVM LAN Server work for sure but both seems like it could be more and with need of a bit of further work, because the LAN server is just a server that you can share everything, and entire disc of unrelated ScummVM data if you want, you can see the potential but also is a very generic...
I'm not criticizing ScummVM devs, if i'm not contributing to the code i haven't any to say here as anyone can do things here (it they know), only i'm guessing it's something maybe planned but for now there is a lot of other things to deal (ResidualVM merge, new themes, new engines, etc.).

comment:9 by antoniou79, 5 months ago

Yes, the plan ideally is to grant the SAF access at the point of setting the save games path. I wasn't aware that we were checking write-ability at that point and apparently I was half-right.

The check is done when setting the path from the Global "Options" -> "Paths" (Tab).
But it wasn't done when setting it for a selected Game via "Edit Game" -> "Paths" (Tab).

I've pushed a fix for this now (it should be available in tomorrow's daily development build).
https://github.com/scummvm/scummvm/commit/d62202801a0ea851157b412e48900a478d799d56

So, this is small progress as I can now reproduce the issue (and previously I was not checking global options settings so that's why I couldn't see it). And ideally I'll be able to add code to trigger the SAF File System Picker at this point. We're still some ways before fully resolving this.

comment:10 by LukasThyWalls, 4 months ago

I've tested ScummVM Git#1da70b92 (what is beyond you d622028 commit) and there is no change in the behaviour commented above. No SAF dialog and path reverts back when say it's not writtable.

I know it is still WIP, but only for mentioning it.

comment:11 by LukasThyWalls, 5 weeks ago

Hello.

I just tested the 2.5.0 version (via Google Play) and the behavior is different. In fact, now the SAF appears. And with a certain selection i can make work the saves in the SDCard, but other things are wrong. But to explain it, i need to make context:

In my phone, the SDcard is /storage/9016-4EF8/, to cut: <SDCARD>.
What i want is a subfolder in SDCard like this: in <SDCARD>/Juegos/ScummVM/, with two or more subfolders inside, but the ones important here are: Juegos (where the games are) and Saves (where i want the saves).
So, the save folder i trying to pick and use is <SDCARD>/Juegos/ScummVM/Saves/.

First, when i pick the save folder (<SDCARD>/Juegos/ScummVM/Saves/) the first time, now the SAF appears to select a folder to bring the permissions. However, with anything i pick with the SAF (SDCard root, <SDCARD>/Juegos/ScummVM/ or <SDCARD>/Juegos/ScummVM/Saves/) brings other message "The chosen directory cannot be written to. Please select other one".
BUT, if i try to pick again the same folder (<SDCARD>/Juegos/ScummVM/Saves/), there is no SAF and it seems it takes the setting.

But then i detected ScummVM is creating the /Juegos/ScummVM/Saves/ subfolders, from wherever i picked from the SAF, for example:

  • If i picked with SAF <SDCARD>/Juegos/ScummVM/, it creates <SDCARD>/Juegos/ScummVM/Juegos/ScummVM/Saves/.
  • If i picked with SAF <SDCARD>/Juegos/ScummVM/Saves, it creates <SDCARD>/Juegos/ScummVM/Saves/Juegos/ScummVM/Saves.

In both cases, the save game fails ("Failed to save game to file: ..."), but it creates the file with 0 bytes alongside a file called "timestamps" with 0 bytes inside the created subfolder.

However, if i pick the SDcard root, it doesn't create the subfolders because they are already there and match where i choose the save game folder, and then save a game works.
BUT, I noticed also that the first time you save, the message with the "Successfully save the game" doesn't pop-up (In consecutive saves it does), AND you can't overwrite an old save (it shows the "Failed to save game to file: ..."), so this thing isn't fully working.

So, there are 4 things what doesn't work like it should:

  • The first time you select a SDcard subfolder and use the SAF, it should be stick the folder and no prompting that the folder is not writable, when the second time works.
  • It shouldn't be using the path from SDcard root to re-creates those subfolders from wherever you have picked with the SAF, it should be have the permission if i picked any parent of the save folder, and use that one to the save files. In fact, if you don't pick the SDcard root, they doesn't match and creates new sub-folders that you don't want and the saves don't work.
  • If everything is right with the subfolders, the first you save, the "save successfully" message should pop-up like the successive saves (This could be pretty minor, but maybe it is symptom/consequence of the others issues).
  • If you can save once, you should be able to overwrite the old saves.

I didn't test this with a devel version, but i will be doing it soon. If it's needed to create a new bug, i will do.

Thanks!

comment:12 by antoniou79, 5 weeks ago

Unfortunately this (buggy) behaviour is known, and I'm working on it, albeit very slowly.

There are no changes or improvements as of yet on the development builds, since I'm working on a local fork of the repository.

So no need to create a new ticket, and I will re-check the issues when I implement the fix (or maybe someone else does, who knows).

comment:13 by LukasThyWalls, 5 weeks ago

Keywords: Saves Overwrite added; Feature request removed
Type: feature requestdefect

Thanks!

I changed the type to "Defect" as is somehow implemented and working but not completely as it should.

I wish i could do something more, tag me if i can help you with more test.

comment:14 by LukasThyWalls, 10 hours ago

Only to add to the topic something i just tested: In 2.5.0, the cloud service actually works. it copies all the saves to the SD, although, it can't overwrite saves that exist in that folder.

Also, it's noticiable slow the first time (it's 2 MB and it took several minutes) and always the clould logo appears in the corner, like it's always trying to update the saves (but that can be the issue above, it can't overwrite them so it tries, and tries, and tries...).

Note: See TracTickets for help on using tickets.