[Windows] Fix root and current folder in native file dialog. #99652
Conversation
scene/gui/file_dialog.cpp
Outdated
| @@ -174,6 +176,7 @@ void FileDialog::_native_dialog_cb_with_options(bool p_ok, const Vector<String> | |||
| file->set_text(f); | |||
| dir->set_text(f.get_base_dir()); | |||
| filter->select(p_filter); | |||
| update_dir(); | |||
There was a problem hiding this comment.
| update_dir(); | |
| _change_dir(f.get_base_dir()); |
current_dir would just be reset to the same value when using update_dir(). Calling _change_dir() properly updates the current_dir and also calls update_dir().
But this made me discover some more bugs:
- When listening to the
*_selectedsignals in GDScript, readingcurrent_*variables in the signal handlers gives outdated values, since the values are updated too late. User must wait 1 frame to get updated values. Fix: move the code that updates file and dir above the signal emission. - Reading the value of
current_dir(after fixing bug 1) does not include the drive letter. Fix: getter should returnfull_dirinstead I assume? - Reading the value of
current_path(after fixing bug 1) returns this:/Users/Public/Test/C:/Users/Public/Test/test123.json. Didn't read the code to figure out why.
Let me know which of these you'd fix in this PR, and I can make issues for the rest!
There was a problem hiding this comment.
Yes, probably moving it will work fine.
There was a problem hiding this comment.
I tried moving it and it works as expected.
Not sure if you saw but I updated my above comment since I found 2 more semi-related bugs, regarding the values of current_dir and current_path after selecting a file/dir in dialog.
There was a problem hiding this comment.
I fixed it like this, feel free to cherry-pick: RedMser@bf1d24b (The EditorFileDialog changes are untested!!)
This also fixes embedded file dialogs, which used to return paths like /Users/Public/... for current_dir and current_path, now also returning the drive letter as expected.
There was a problem hiding this comment.
Not sure what changing dir_access->get_current_dir() to full_path should do, it should always be the same value.
There was a problem hiding this comment.
Yes I guess the EditorFileDialog change is not strictly needed then.
But FileDialog uses dir->get_text(); for get_current_dir() which excludes drive letter.
Or could we even get rid of full_dir variable completely and just use FileDialog::get_current_dir() which would return dir_access->get_current_dir()?
There was a problem hiding this comment.
Yes I guess the EditorFileDialog change is not strictly needed then.
Missed that it is different in FileDialog (why do we have two almost identical dialogs?), this make sense for the FileDialog. Updated PR.
678d356
to
adba8b1
Compare
Co-authored-by: RedMser <redmser.jj2@gmail.com>
Supersede #98839
Fixes #98832
Fixes #99598