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

Add Browse... button to Android game browser #13898

Merged
merged 2 commits into from
Jan 9, 2021

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Jan 9, 2021

Should allow you to pick the SD card, working around #13827 until proper Storage Access Framework support is added.

Does some highly unsupported path mangling to convert a storage uri to a local folder...

…a gross hack.

Should help #13827

Not yet using storage framework properly, just stealing the URI.
@hrydgard hrydgard added this to the v1.11.0 milestone Jan 9, 2021
@hrydgard hrydgard changed the title Add Browse... button to Android file browser Add Browse... button to Android game browser Jan 9, 2021
@hrydgard hrydgard merged commit e00f894 into master Jan 9, 2021
@hrydgard hrydgard deleted the android-storage-file-picker branch January 9, 2021 11:41
Copy link
Collaborator

@unknownbrackets unknownbrackets left a comment

Choose a reason for hiding this comment

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

How well does this local-only flag work? Does it restrict well?

The "right" way ought to be this: https://developer.android.com/reference/android/content/ContentResolver.html#openAssetFileDescriptor(android.net.Uri,%20java.lang.String,%20android.os.CancellationSignal)

I'd imagine something like (psuedo):

// Java land - with null checks...
parcel = openAssetFileDescriptor(uri, "r").getParcelFileDescriptor();
int desc = parcel.getFd();

// C++ land
std::string old = getcwd();
fchdir(desc);
std::string path = getcwd();
chdir(old);

// Back to Java
parcel.close();

// In C++ land: use path.

Haven't tested it. But that's what I'd hope for. Maybe with the local flag that could make #11997 pretty okay?

Edit: The above is, by the way, a technique that some have used to escape sandboxes which you have to be careful about when forking a privileged process to a less privileged one. So there might be restrictions (to mitigate security bugs) on it for Android, I don't know.

-[Unknown]

Intent intent = new Intent(Intent.ACTION_OPEN_DOCUMENT);
intent.addCategory(Intent.CATEGORY_OPENABLE);
intent.setType("application/octet-stream");
//intent.putExtra(DocumentsContract.EXTRA_INITIAL_URI, pickerInitialUri);
Copy link
Collaborator

Choose a reason for hiding this comment

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

We probably want EXTRA_LOCAL_ONLY here too in most cases (we could add another browse_foo for if we didn't.) I realize this isn't used yet.

-[Unknown]

@hrydgard
Copy link
Owner Author

hrydgard commented Jan 9, 2021

Yes, something like that could work. Apparently we can even preserve the permission to the folder (although not across some events...).

The local restriction - when choosing folders, it seems to already be limited to local paths. Only when you choose a single or multiple files do you get to select stuff from Google Drive.

So it seems to be maybe more workable than we thought...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants