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: Support of SAF #2699

Merged
merged 66 commits into from
Apr 1, 2022
Merged

Android: Support of SAF #2699

merged 66 commits into from
Apr 1, 2022

Conversation

BlisterB
Copy link
Member

@BlisterB BlisterB commented Nov 19, 2021

This PR add the support of Android SAF in order to be compatible with API 30 and the new rules about scoped storage permissions.

What has been done :

  • At the start of the app, it checks storage permissions and ask the right to access a Games Folder if necessary.
    • On API < 30, we just need to select the folder one time, on API >= 30, we have to ask for this permission at each start (permanent rights doesn't seems to work)
    • Drop the "multi games folder" feature, not suitable with this new paradigm
  • Rewrite most of the code referring to File class (deprecated and broken) and replaced with DocumentFile class.

Important notes :

  • Minimum API is now 21
    - SAF is really slow and using tricks to avoid this system would probably lead to future incompatibilities.

What remains to do :

  • Fast asynchronous recursive games scanning & result cache
  • Check/save of the encoding configuration in the .ini file
  • Support for soundfout files (waiting for the player to take into account --soundfont argument in the command line)
  • Rework inputs layouts
  • Fix the long loading times at the start of a game
  • Fix the crash during the start of some games (ex. Ara Fell)
  • Change the messages explaining how the app works (at the end)
  • Bug Report feature
  • Add a button to open soundfont folder

Fix #1041
Fix #1330
Fix #2434

@Ghabry
Copy link
Member

Ghabry commented Nov 19, 2021

I can already see the async loading: Searching for games please come back in 2 hours. Send hate to Google kthxbye

@BlisterB
Copy link
Member Author

It is now possible to use a custom soundfont by putting the .sf2 in the games folder (only on the android side).
The path is sent to the native code through command line with argument --soundfont.

As discussed with Ghabry, the idea is to also provide a default sounfont in the apk and drop Timidity.
A message will be placed in Settings to inform the player of this possibility and inform what is the current used soundfont.

@BlisterB
Copy link
Member Author

The browser now scans games asynchronously. I will later add a visual feedback explaining that it's processing.
The app is much more snappier, but games scanning remains pretty slow.

@fdelapena fdelapena added this to the 0.7.1 milestone Nov 21, 2021
@BlisterB
Copy link
Member Author

The games list is now cached and only refreshed when the player click the "refresh" button, or change the games folder in settings.
With the cache system and asynchronous scans, the only thing we can improve is the scan speed, this is the next things I will be working on.

@BlisterB
Copy link
Member Author

The speed of game scanning has been dramatically improved, it's not a problem anymore.
By design the scan can't be recursive without a huge performance loss (we use the heavy listFile() for the games folder to obtain various information about files/folder, and quick searches in subfolders with query() to obtains only file names).
It may be possible to use query() for all the search, but other things are more important for now.

@BlisterB
Copy link
Member Author

The user doesn't need anymore to choose the games folder at each startup, even on API 30.

@BlisterB
Copy link
Member Author

The favorite ("like") feature is now instant. It previously used to rescan the games folder. The app has never been so snappy.

@Ghabry
Copy link
Member

Ghabry commented Nov 23, 2021

@BlisterB

I researced the query stuff a bit more (good job btw)
As a second column in the query you can ask for DocumentsContract.Document.COLUMN_MIME_TYPE and then compare if the string is equal to DocumentsContract.Document.MIME_TYPE_DIR (currently defined as vnd.android.document/directory).

If yes it is a folder. Should make recursive scanning possible again :)

@Ghabry
Copy link
Member

Ghabry commented Nov 23, 2021

Here is a patch to make the native part faster and to fix some API warnings

diff --git a/builds/android/app/src/main/java/org/easyrpg/player/player/SafFile.java b/builds/android/app/src/main/java/org/easyrpg/player/player/SafFile.java
index f26e09f5..050d7ea6 100644
--- a/builds/android/app/src/main/java/org/easyrpg/player/player/SafFile.java
+++ b/builds/android/app/src/main/java/org/easyrpg/player/player/SafFile.java
@@ -1,14 +1,18 @@
 package org.easyrpg.player.player;
 
+import android.content.ContentResolver;
 import android.content.Context;
+import android.database.Cursor;
 import android.net.Uri;
 import android.os.ParcelFileDescriptor;
+import android.provider.DocumentsContract;
 import android.util.Log;
 
 import androidx.documentfile.provider.DocumentFile;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.List;
 
 /**
  * A wrapper around SAF for use from JNI
@@ -82,7 +86,7 @@ public class SafFile {
         }
 
         // No difference between read mode and binary read mode
-        try (ParcelFileDescriptor fd = context.getContentResolver().openFile(root.getUri(), "r", null)) {
+        try (ParcelFileDescriptor fd = context.getContentResolver().openFileDescriptor(root.getUri(), "r")) {
             return fd.detachFd();
         } catch (IOException e) {
             return -1;
@@ -119,7 +123,7 @@ public class SafFile {
             actualFile = df.getUri();
         }
 
-        try (ParcelFileDescriptor fd = context.getContentResolver().openFile(actualFile, mode, null)) {
+        try (ParcelFileDescriptor fd = context.getContentResolver().openFileDescriptor(actualFile, mode)) {
             return fd.detachFd();
         } catch (IOException e) {
             return -1;
@@ -131,13 +135,33 @@ public class SafFile {
             return null;
         }
 
+        Uri root_uri = root.getUri();
+
         ArrayList<String> files = new ArrayList<>();
         ArrayList<Boolean> is_dir = new ArrayList<>();
 
-        for (DocumentFile file: root.listFiles()) {
-            files.add(file.getName());
-            is_dir.add(file.isDirectory());
+        final ContentResolver resolver = context.getContentResolver();
+        final Uri childrenUri = DocumentsContract.buildChildDocumentsUriUsingTree(root_uri, DocumentsContract.getDocumentId(root_uri));
+
+        Cursor c = resolver.query(childrenUri,new String[] {
+                DocumentsContract.Document.COLUMN_DOCUMENT_ID,
+                DocumentsContract.Document.COLUMN_MIME_TYPE },
+                null, null, null);
+        while (c.moveToNext()) {
+            // The name can be something like ``primary:path/of/the/folder/the_file.txt
+            // Get rid of all that junk
+            String file_path = c.getString(0);
+            int slash_pos = file_path.lastIndexOf('/');
+            if (slash_pos != -1) {
+                file_path = file_path.substring(slash_pos + 1);
+            }
+
+            String mime_type = c.getString(1);
+
+            files.add(file_path);
+            is_dir.add(mime_type.equals(DocumentsContract.Document.MIME_TYPE_DIR));
         }
+        c.close();
 
         return new DirectoryTree(files, is_dir);
     }

//String str = SettingsManager.getEasyRPGFolder() + "/rtp";
// Log.v("SDL", "getRtpPath " + str);
//return str;
return SettingsManager.getRtpFolder().toString();
Copy link
Member

Choose a reason for hiding this comment

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

will this return a valid SAF Uri? (content:// and stuff)

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep :)

Copy link
Member

Choose a reason for hiding this comment

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

I just tested it.
You must use return SettingsManager.getRtpFolder().getUri().toString();, then it will work.Yours gives back the class name + pointer (Standard toString impl)

@BlisterB
Copy link
Member Author

The recursive games search is back and is fast. The game browser is officially better than before. :)

I've also moved all the file's related operation to the Helper class in order to centralized them, in case Google decides to change all the rules again.

@Ghabry
Copy link
Member

Ghabry commented Nov 25, 2021

@BlisterB added my two commits as requested

@Ghabry
Copy link
Member

Ghabry commented Nov 25, 2021

@BlisterB You can now grab the APK from the PR builder in case you want to do some native code testing.

Finally fixed the build ^^

@BlisterB
Copy link
Member Author

The app can now retrieve the encoding of the game, it can also save another encoding settings.
I dropped the JSON preference file and use the .ini file directly.

Thank you @Ghabry ;)

@Ghabry
Copy link
Member

Ghabry commented Nov 27, 2021

Fix the crash during the start of some games (ex. Ara Fell)

That is due to missing Midi support. It still uses "get_timidity_path" JNI stuff on Android. Have to patch this out and hook it in with fluidsynth.

@BlisterB
Copy link
Member Author

The Gamebrowser activity now recovers from the crash of a child activity (a game crashed for example).
It used to don't display the game list after coming back from the crash of a child activity.
I discovered that after a crash, the app will display the previous activity, but static field of the SettingsActivity are set to null (it's probably linked to the cancel of the Context and the fact that DocumentFile are linked to this context).
This is extremely bizarre but I'll remember that it is not safe to store user's preferences in static fields.

@BlisterB
Copy link
Member Author

The input layouts feature has been reworked. You can continue to create multiple input layouts and choose an active layout.
It's no more possible to associate a specific layout to a game folder.
I've also done a lot of code cleaning.

@BlisterB
Copy link
Member Author

The Audio Settings activity now display a checkbox to enable/disable custom SoundFounts. There is also some text explaining the feature and the .sf2 currently used (the first .sf2 fount in the games folder).
An improvement of the system would be to display a list of all the .sf2 with a RadioButton system to select the one to use. Maybe for later.

@Ghabry
Copy link
Member

Ghabry commented Jan 5, 2022

@BlisterB
Sorry for the slow response time: Is there anything missing here except for the soundfont handling on Player side?

@BlisterB
Copy link
Member Author

BlisterB commented Jan 8, 2022

Hello @Ghabry
It need the soundfont handling in order to work. And a rework of information messages explaining how the app works.
I think that it would be a good idea to make the use of the RTP folder optional (to help improve the initial loading) but it can be done in a futur PR.

@Ghabry
Copy link
Member

Ghabry commented Jan 14, 2022

I guess "Report a bug" is also kinda broken? Because attaching the files does not really work.

On the good news: We are down to ca. 5 bug reports per month so looks a bit as if we do not really need it anymore (and can just link e.g. Github)

@Ghabry
Copy link
Member

Ghabry commented Mar 1, 2022

Dropped now Timidity support completely. When no soundfont is provided Player uses now FmMidi.
Wildmidi has no advantage on Android anymore (is mostly useful for low power devices that cannot handle soundfont or fmmidi).
This resolves also the crash on Android when playing Midi.

Also the --soundfont argument is now supported but this requires some testing.

@Ghabry
Copy link
Member

Ghabry commented Mar 1, 2022

About the "TODO: Convert to SAF" functions: They can be all removed. I expect that SAF makes the folder the user selects writable solving this permission mess.

@Ghabry
Copy link
Member

Ghabry commented Mar 1, 2022

@BlisterB Soundfont stuff works now properly. When a game ran before then after changing the setting the app must be restarted due to some global state in C++ Player but this is no big deal imo. Is not like this is changed alot ^^.


Btw is the "games" folder still created in easyrpg? I mean it is not really needed anymore but maybe some users want to stay organized, so just create it by default ^^


Some nits about the terms:

I suggest changing the following terms:

Settings -> Game Directories: Change to "EasyRPG Folder"

Then the wording in "EasyRPG folder":

[SELECT EASYRPG FOLDER] <-- Button

This folder may only be used for RPG Maker 2000/2003 games. Do not select 
folders that contain other things, such as the download folder.

[OPEN RTP FOLDER] <-- Launches the file browser of Android and shows RTP

The RTP provides shared assets used by some games. Put the RPG Maker 2000
RTP in "rtp/2000" and the RPG Maker 2003 RTP in "rtp/2003".

(No need to print the paths here imo, Open RTP Folder kinda solves this)

Audio:

[x] Use Custom Soundfont

You can provide a custom soundfont to alter how MIDI music sounds. Place a
.sf2 file in the EasyRPG folder.

Soundfont used: $SOUNDFONT_NAME / File not found

Also the path displayed for Soundfont should be trimmed so they do not contain garbage like primary:. The soundfont could be only the filename.

@BlisterB
Copy link
Member Author

BlisterB commented Mar 6, 2022

The soundfont feature works great! 👍

Copy link
Member

@carstene1ns carstene1ns left a comment

Choose a reason for hiding this comment

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

Just few nits.

We probably should document somewhere, that the android files have a different license.

@Ghabry
Copy link
Member

Ghabry commented Mar 31, 2022

The basic app functionality appears to work 👍

Can you quickly reformat the files IniFileManager.java and Game.java with your IDE? They look very messy indentationn wise.

@fdelapena fdelapena removed the Awaiting Rebase Pull requests with conflicting files due to former merge label Apr 1, 2022
- Bump dependencies
- Fix warnings and formating issues in AndroidManifest.xml
- Revert SDL changes
@Ghabry Ghabry merged commit f8f8254 into EasyRPG:master Apr 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4 participants