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

Fix unable to change directory in user access mode for 3.x #59838

Merged

Conversation

keptsecret
Copy link
Contributor

Fix for #59828
Uses the solution from #58772

@keptsecret keptsecret requested a review from a team as a code owner April 3, 2022 15:23
@Calinou Calinou added this to the 3.5 milestone Apr 3, 2022
@akien-mga
Copy link
Member

I know we already merged the same fix in master, but I think we should also make sure that we understand where the regression stems from. Both for master and 3.5 this was reported as a regression so we should be able to find which PR caused the behavior change and was backported to 3.x, and whether this fix is the proper follow-up or if something else should be done.

@keptsecret
Copy link
Contributor Author

On master I believe it was the introduction of OS specific stuff (for Windows) that meant the directories FileDialog was in and what the game was actually in were different. I haven't looked too closely into the 3.x branch yet though.

@akien-mga
Copy link
Member

Oh I see, so that might be a consequence of #56012 (4.0) and #55987 (3.x).

CC @bruvzg for additional context on your review of #58772.

@bruvzg
Copy link
Member

bruvzg commented Apr 4, 2022

Oh I see, so that might be a consequence of #56012 (4.0) and #55987 (3.x).

I guess it can affect the current working directory of the editor process on Windows, but --path should reset in on the start, and it should only affect res:// in this case. Also, #59828 is reported on Linux, so I'm not sure if it might cause it.

@bruvzg
Copy link
Member

bruvzg commented Apr 4, 2022

It's possible it was always broken, but editor was overwriting file dialog settings with some valid (on the developer machine, it's not necessery valid for the end user) value before #54399, so it was not noticed.

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.

Fix should be valid, regardless of the case of the issue, CWD is not appropriate default path for USER or RESOURCE file systems.

@akien-mga
Copy link
Member

Sounds great, thanks for looking into it.

@akien-mga akien-mga merged commit 20e4fa2 into godotengine:3.x Apr 4, 2022
@akien-mga
Copy link
Member

Thanks!

@keptsecret keptsecret deleted the 3x_fix_filedialog_user_data_access branch April 4, 2022 20:57
@keptsecret
Copy link
Contributor Author

Ah, it might be a different problem on Linux then because I'm working on Windows and I found that get_user_data_dir() from

String DirAccess::_get_root_path() const {
switch (_access_type) {
case ACCESS_RESOURCES:
return ProjectSettings::get_singleton()->get_resource_path();
case ACCESS_USERDATA:
return OS::get_singleton()->get_user_data_dir();
default:
return "";
}
}

was returning a value (somewhere in %appdata%) that led to preventing changing of directories (coming from os_windows.cpp instead of os.cpp). It seems to be the same bits on both master and 3.x. And in both, it does return CWD on Linux, so I guess the fix remains valid.

@akien-mga
Copy link
Member

Cherry-picked for 3.4.5.

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.

4 participants