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 native file picker support #98350

Merged

Conversation

syntaxerror247
Copy link
Member

@syntaxerror247 syntaxerror247 commented Oct 20, 2024

  1. First commit, creates a new DisplayServer Flag FEATURE_NATIVE_DIALOG_FILE_EXTRA, for access to USERDATA and RESOURCES directory and OPTIONS feature. Native File Dialog will fallback to custom one if not supported.

  2. Second commit, implements native file picker support for android

This Partially addresses

Test App (arm64): Test.zip

Demo Video

VID_20241021_215329.mp4

Task List
  • Every FileMode is supported (except OPEN_ANY).
  • Filters are supported (MIME types).
  • Current directory(initial directory) supported.
  • Filename for SAVE mode supported.
  • callback supported.
  • Options support Not possible with system file picker.
  • show hidden It can be set manually with system file picker.
  • selected filter index callback Not possible with multi select, that's why omited this feature entirely.
  • access mode RESOURCES and USERDATA support Not possible with native picker, will fallback to custom one.

@syntaxerror247 syntaxerror247 requested a review from a team as a code owner October 20, 2024 00:27
@syntaxerror247 syntaxerror247 marked this pull request as draft October 20, 2024 00:27
@syntaxerror247 syntaxerror247 force-pushed the android-native-filepicker branch from 603853c to 21ba4d9 Compare October 20, 2024 00:36
@syntaxerror247

This comment was marked as outdated.

@syntaxerror247

This comment was marked as outdated.

@syntaxerror247 syntaxerror247 force-pushed the android-native-filepicker branch 2 times, most recently from c303544 to ed8e2f9 Compare October 20, 2024 07:54
@Chaosus Chaosus added this to the 4.x milestone Oct 20, 2024
@syntaxerror247
Copy link
Member Author

Now, Current_directory (initial directory) is also supported. It should be an external storage directory, users can use OS.get_system_dir(OS.SYSTEM_DIR_DESKTOP) to get root path.
example:

func _show_native_file_picker() -> void:
	var filters = PackedStringArray(["image/*","text/plain","application/pdf"])
	var current_directory = OS.get_system_dir(OS.SYSTEM_DIR_DOCUMENTS)
	DisplayServer.file_dialog_show("title", current_directory, "filename", false, DisplayServer.FILE_DIALOG_MODE_OPEN_FILE, filters, _picker_callback)

func _picker_callback(status: bool, selected_paths: PackedStringArray, selected_filter_index: int):
	$Label.text += str("Status: ",status,"\n")
	$Label.text += str("Selected Paths: ",selected_paths,"\n\n")

@syntaxerror247
Copy link
Member Author

This is now ready for review, and all features are functioning as expected in my tests.

Regarding the two unchecked tasks:

  • I considered implementing the selected_filter_index callback for single file selection, but it won't be feasible for multi-select scenarios. Therefore, I decided to omit this feature entirely.
  • As for supporting RESOURCES and USERDATA access modes, this is not possible with the android native file picker.

Please let me know if any additional adjustments are needed!

Copy link
Member

@bruvzg bruvzg left a comment

Choose a reason for hiding this comment

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

Since implementation is limited (which is expected for Android), limitations (no options, no usr:// and res:// access) should be mentioned in the class reference documentation for FileDialog and DisplayServer:

DisplayServer:

<method name="file_dialog_show">

<method name="file_dialog_with_options_show">

FileDialog:

<member name="use_native_dialog" type="bool" setter="set_use_native_dialog" getter="get_use_native_dialog" default="false">

<method name="add_option">

Also, it might be worth adding new DisplayServer feature flags for options and res/usr access, and check these flags in the FileDialog to automatically switch to embedded dialog when incompatible options are used.

platform/android/java_godot_lib_jni.cpp Outdated Show resolved Hide resolved
@syntaxerror247

This comment was marked as outdated.

@syntaxerror247
Copy link
Member Author

@bruvzg how about only one new feature tag FEATURE_NATIVE_DIALOG_FILE_EXTRA ?

@bruvzg
Copy link
Member

bruvzg commented Oct 20, 2024

how about only one new feature tag FEATURE_NATIVE_DIALOG_FILE_EXTRA ?

One flag should be OK (iOS will have the same limitations).

@syntaxerror247 syntaxerror247 marked this pull request as draft October 20, 2024 16:50
@syntaxerror247 syntaxerror247 marked this pull request as draft October 20, 2024 16:50
@syntaxerror247 syntaxerror247 marked this pull request as draft October 20, 2024 16:50
@syntaxerror247 syntaxerror247 marked this pull request as draft October 20, 2024 16:50
@syntaxerror247 syntaxerror247 marked this pull request as draft October 20, 2024 16:50
@syntaxerror247 syntaxerror247 marked this pull request as draft October 20, 2024 16:50
@syntaxerror247 syntaxerror247 force-pushed the android-native-filepicker branch 2 times, most recently from 8cd0a88 to fd58b7e Compare October 20, 2024 17:55
@syntaxerror247 syntaxerror247 marked this pull request as ready for review October 20, 2024 18:00
@syntaxerror247 syntaxerror247 requested a review from a team as a code owner October 20, 2024 18:00
platform/android/display_server_android.cpp Outdated Show resolved Hide resolved
platform/android/display_server_android.h Outdated Show resolved Hide resolved
platform/android/java_godot_lib_jni.cpp Outdated Show resolved Hide resolved
platform/android/java_godot_wrapper.cpp Show resolved Hide resolved
@syntaxerror247 syntaxerror247 force-pushed the android-native-filepicker branch from 40a8199 to cc0ad6a Compare October 25, 2024 20:43
@syntaxerror247 syntaxerror247 requested a review from m4gr3d October 25, 2024 20:48
@syntaxerror247 syntaxerror247 force-pushed the android-native-filepicker branch from cc0ad6a to 872f6c7 Compare October 26, 2024 01:42
@syntaxerror247 syntaxerror247 requested a review from m4gr3d October 26, 2024 02:03
@llama-nl
Copy link

@syntaxerror247 can I have the APK to test?

@syntaxerror247
Copy link
Member Author

@UnderGamer05 I have uploaded the apk, check in PR description.

@syntaxerror247
Copy link
Member Author

Hi @m4gr3d , just checking in to see if the updates I made in the PR align with what you had in mind. Let me know if there's anything else you'd like me to tweak.

Copy link
Contributor

@m4gr3d m4gr3d left a comment

Choose a reason for hiding this comment

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

Looks good!

Added some minor comments to address.

@m4gr3d m4gr3d modified the milestones: 4.x, 4.4 Oct 28, 2024
@syntaxerror247 syntaxerror247 force-pushed the android-native-filepicker branch from 872f6c7 to 6c7ef40 Compare October 28, 2024 14:28
@syntaxerror247 syntaxerror247 force-pushed the android-native-filepicker branch from 6c7ef40 to 025a538 Compare October 28, 2024 15:37
@syntaxerror247 syntaxerror247 force-pushed the android-native-filepicker branch 3 times, most recently from 7dcfa62 to 64a4c2b Compare October 30, 2024 09:15
@syntaxerror247 syntaxerror247 force-pushed the android-native-filepicker branch from 64a4c2b to b2130ef Compare October 30, 2024 09:24
@syntaxerror247
Copy link
Member Author

Rebased and resolved conflicts.

@Repiteo Repiteo merged commit be70c2f into godotengine:master Nov 1, 2024
20 checks passed
@Repiteo
Copy link
Contributor

Repiteo commented Nov 1, 2024

Thanks!

@syntaxerror247 syntaxerror247 deleted the android-native-filepicker branch November 1, 2024 03:55
@ThatSimpleDev
Copy link

@syntaxerror247 Just tested the test apk provided here and when opening the resources it doesn't open the native file picker is that intentional?
Also it says you don't have enough permissions as you can see in the screenshot below 👇

Screenshot_2024-11-23-03-07-05-35_b62382514e18f27f1d9e6a66d3a6598c

I also tested this on v4.4 Dev 5 and when I try to use the native file dialog (not through code) and setting the access as ACCESS_FILESYSTEM the app immediately closes when I press the button. I believe I have checked the needed read user directory permissions (It asked to enable the same permission as the test apk you provided). Did you do this through code?

@syntaxerror247
Copy link
Member Author

syntaxerror247 commented Nov 23, 2024

Just tested the test apk provided here and when opening the resources it doesn't open the native file picker is that intentional?

@ThatSimpleDev Yes, that intentional. Android doesn't allow access to user:// and res:// directory, that's why it fallbacks to custom Filedialog.

I also tested this on v4.4 Dev 5 and when I try to use the native file dialog (not through code) and setting the access as ACCESS_FILESYSTEM the app immediately closes when I press the button.

Yes, This is a regression due an update to FileDialog. This issue wasn't present in dev4. This will be solved after these two PRs #99350 and #99385 are merged.

For now, you can use it via code.

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.

8 participants