Skip to content

Use ProjectSettings path functions instead of hard-coded folder names in tests #108170

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aaronfranke
Copy link
Member

I was grepping through the codebase to find the helper function that returns res://.godot/editor was, and didn't find these parts of the codebase when I searched for ".godot" since the actual constant is "godot". Once I found it, I thought it would be nice to have comments here, so that people can grep for these strings to find it. Then, I realized that my search for ".godot" led me to two places in the tests where this is hard-coded, so I replaced those.

While making this PR, I also noticed that DirAccess::make_dir_recursive doesn't properly handle res:// paths, so I have to call ProjectSettings::globalize_path first, but I wonder if that's a bug and DirAccess should be fixed?

@aaronfranke aaronfranke added this to the 4.x milestone Jul 1, 2025
@aaronfranke aaronfranke requested a review from a team as a code owner July 1, 2025 18:46
@aaronfranke aaronfranke requested review from a team as code owners July 1, 2025 18:46
@aaronfranke aaronfranke force-pushed the test-ps-path-func branch 2 times, most recently from 8bb1516 to 2c6c583 Compare July 1, 2025 18:59
@aaronfranke aaronfranke requested a review from a team as a code owner July 1, 2025 18:59
@Calinou
Copy link
Member

Calinou commented Jul 7, 2025

While making this PR, I also noticed that DirAccess::make_dir_recursive() doesn't properly handle res:// paths, so I have to call ProjectSettings::globalize_path() first, but I wonder if that's a bug and DirAccess should be fixed?

I'm not 100% sure. It's a bit like using OS.shell_open() on res:// or user:// paths, where you also have to use ProjectSettings::globalize_path() first. (You could technically be trying to open a protocol named res:// or user://, it's just pretty unlikely.)

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.

2 participants