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

FileDialog current_* values should not be saved to scene files #29674

Closed
aaronfranke opened this issue Jun 11, 2019 · 6 comments · Fixed by #54399
Closed

FileDialog current_* values should not be saved to scene files #29674

aaronfranke opened this issue Jun 11, 2019 · 6 comments · Fixed by #54399

Comments

@aaronfranke
Copy link
Member

aaronfranke commented Jun 11, 2019

Godot version: 3.1.1

Issue description:

The FileDialog node has a few string properties called current_dir, current_file, and current_path. These are fine to generate on the fly and use while the game is running, but they should not be saved to .tscn or similar files. Every system that this project is opened on will change this path to be something else. If we really need to save it, then save it inside the .godot folder.

Minimal reproduction project:

The problem is visible here: godotengine/godot-demo-projects#89 (comment)

@Xrayez
Copy link
Contributor

Xrayez commented Aug 22, 2020

Likely yes, because when you switch mode, those properties are reset automatically AFAIK (current_dir becomes res:// if you switch mode to Resource, for instance). Could also be problematic for tool scripts.

@MatteoPiovanelli-Laser
Copy link

Hello. I think this is still an issue now in Godot 3.2.3
See this other issue I opened on a Godot project before finding this thread: #29674

Having those properties automatically save in the .tscn file is troublesome for VCS.

@andrea-calligaris
Copy link
Contributor

andrea-calligaris commented Oct 29, 2021

This is a security/privacy concerning bug.
Someone please delete the "enhancement" tag and put e.g. "high priority".

@Calinou
Copy link
Member

Calinou commented Oct 29, 2021

This is a security/privacy concerning bug. Someone please delete the "enhancement" tag and put e.g. "high priority".

If you consider your real name to be sensitive information, don't use your real name as your PC username. I know it requires a fair amount of work to move everything, but it's worth it in the long run 🙂

This way, you also won't have to edit screenshots and logs when posting them online.

@Zireael07
Copy link
Contributor

@Calinou: Counterargument: it's fairly easy to edit screenshots and logs when posting. On the other hand, you can't edit these fields without breaking stuff.

Also, many companies require user name to be your real name, end of story - they won't let you change.

@andrea-calligaris
Copy link
Contributor

If you consider your real name to be sensitive information, don't use your real name as your PC username. I know it requires a fair amount of work to move everything, but it's worth it in the long run slightly_smiling_face

As you can see I have my real name in this GitHub account, so it's not about me: it's about the fact that the bug described in this issue saves an entire path in the scene file. It's not about the name of the PC account, it's about any other folder, any other content that you can browse on your computer. Let's say that for whatever reason, for testing your game you traverse a path on your PC named /home/JohnSmith/Documents/MyVerySecretProjectAboutSomething/secret_information_about_XYZ.txt, that path will be saved in the scene file. That, as I pointed out, is a security/privacy concerning bug.

@akien-mga akien-mga added this to the 3.5 milestone Mar 12, 2022
Poobslag added a commit to Poobslag/turbofat that referenced this issue Jan 3, 2024
Our dialog popups used to default to game directories, but this was
broken in Godot 3.5 by the fix for Godot #29674
(godotengine/godot#29674)

I've restored these paths.
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.

8 participants