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

The FileDialog could not get the path to the file correctly #45822

Open
HapeniaLans opened this issue Feb 8, 2021 · 5 comments
Open

The FileDialog could not get the path to the file correctly #45822

HapeniaLans opened this issue Feb 8, 2021 · 5 comments

Comments

@HapeniaLans
Copy link

Godot version:
3.2.3 and also 3.2.4 rc1

OS/device including version:
at least windows 10

Issue description:
When you select a file in a FileDialog in file system mode, it won't return a path with current driver.

For example:

actual path: D:\foo\bar.png
expected result:
  current_path = D:/foo/bar.png or D://foo/bar.png
actual result:
  current_path = /foo/bar.png

When I open the file on the default drive, this is no problem. But this can cause confusion when I open files on other drives.

@Calinou
Copy link
Member

Calinou commented Feb 8, 2021

@HapeniaLans Please upload a minimal reproduction project to make this easier to troubleshoot.

@HapeniaLans
Copy link
Author

Here it is.
test.zip

@Bauxitedev
Copy link

This is quite confusing! This issue caused a bug in my program Bitmapflow when it tries to load images from another drive, since the drive letter is missing from FileDialog.current_path and FileDialog.current_dir.

I've found a workaround though: if you yield for the "file_selected" signal emitted by FileDialog, you will get the current path correctly, with drive letter and all. E.g.

var dialog = FileDialog.new()
dialog.access = FileDialog.ACCESS_FILESYSTEM
add_child(dialog)
dialog.popup_centered_ratio()

var filename = yield(dialog, "file_selected")

# filename now holds the correct filename

@yiyuezhuo
Copy link

Current FileDialog::get_current_path and FileDialog::get_current_dir just take text value from two TextEdit so it just ignore the driver:

String FileDialog::get_current_dir() const {
	return dir->get_text();
}

String FileDialog::get_current_file() const {
	return file->get_text();
}

String FileDialog::get_current_path() const {
	return dir->get_text().plus_file(file->get_text());
}

It makes sense to expose methods to access those two raw UI values, but I guess users usually want the value of dir_access->get_current_dir(true) for those method names, just like the value from mentioned workaround:

	String file_text = file->get_text();
	String f = file_text.is_absolute_path() ? file_text : dir_access->get_current_dir().plus_file(file_text);

	if ((mode == FILE_MODE_OPEN_ANY || mode == FILE_MODE_OPEN_FILE) && dir_access->file_exists(f)) {
		emit_signal(SNAME("file_selected"), f);
		hide();

I will send a PR once I figure out all of contributing rules.

@yiyuezhuo
Copy link

yiyuezhuo commented Aug 9, 2022

After some research, I found that current_dir and current_path are somewhat ill-defined. The current implementation is not useful but has its own advantage of simplicity. Any methods other than replicating signal behavior would introduce more complicated subtle variance... So it may be better to just remove these properties or follow the style to expose the driver as well and rename them to prevent misunderstanding.

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

Successfully merging a pull request may close this issue.

4 participants