98teg / NativeDialogs

Native Dialogs is a plugin for Godot that allows you to interact with OS-specific dialogs, such as notifications, messages and file dialogs.
MIT License
156 stars 8 forks source link

NativeFileDialog - setting root_subfolder before access results in an error #11

Closed tonebacas closed 1 year ago

tonebacas commented 1 year ago

Sample code:

func _ready() -> void:
    var f := NativeFileDialog.new()
    add_child(f)
    f.title = "My File Dialog title"
    f.file_mode = NativeFileDialog.FILE_MODE_SAVE_FILE
    f.root_subfolder = "res://"
    f.access = NativeFileDialog.ACCESS_FILESYSTEM
    f.show()

Error at runtime (line 11 is f.root_subfolder = "res://")

E 0:00:00:0748   native_dialog_test.gd:11 @ _ready(): root_subfolder must be an existing sub-directory.
  <C++ Error>    Condition "!DirAccess::dir_exists_absolute(get_root_string() + p_root)" is true.
  <C++ Source>   src\native_file_dialog.cpp:202 @ godot::NativeFileDialog::set_root_subfolder()
  <Stack Trace>  native_dialog_test.gd:11 @ _ready()

If I swap the two lines, the sample code runs without errors as expected, opening a file dialog at the specified path.

I've looked at the code in the following line and noticed you're calling set_root_subfolder("") at the end of the NativeFileDialog::set_access function. I suppose this is done as a way to handle Godot's paths starting with res:// or user://.

https://github.com/98teg/NativeDialogs/blob/a6acb2d4449f4017b7d9907122c0970782f4dc4a/src/native_file_dialog.cpp#L194

As a workaround, setting root_subfolder after setting access solves the problem.

98teg commented 1 year ago

Resetting the root_subfolder value when changing the access is something that also happens in Godot's FileDialog:

https://github.com/godotengine/godot/blob/9b9bb418cb1137e69b5131ec9fa7b41c0396db28/scene/gui/file_dialog.cpp#L858

This is done in order to offer a better UX when using the editor: changing the access would reset the root_subfolder since it no longer makes sense.

Do you think this issue may be solved by adding a comment to the documentation? A comment that warns users that changing the access would also reset the value of the root_subfolder.