[3.x] Make the project data directory customizable

[m4gr3d]

This PR makes the default project data directory (.import) customizable.

This is useful on platforms where certain directory patterns are disallowed (e.g: some Android build tools disallow hidden directories in the generated APK).

Bugsquad edit: 3.x version of #52548.

[akien-mga]

Approved, subject to changes from my review of the master PR: #52548 (review)

[m4gr3d]

@akien-mga I've addressed the feedback, and tested the changes, all good to go!

[akien-mga]

Thanks!

[Listwon]

@akien-mga sorry for catching right after you merged it, but isn't replacing == with begins_with() too broad in EditorFileSystem::_should_skip_directory, EditorFileDialog::_item_list_item_rmb_selected or FindInFiles::_scan_dir? If we set our custom path to let's say imported, it will skip (or include) all of the folders starting with imported, while previously it ignored those equal to imported.

[akien-mga]

Yeah I thought about this while reviewing but as it's a corner case on top of a corner case, I thought it's not worth the trouble to handle better. Can be done if there's a need, it just means more string wrangling for something that shouldn't even be needed in the first place if third-party tooling wasn't broken...

[Listwon]

It's not even an optimization in this case but can cause trouble, so I don't know if it's worth to replace == in those places. I'd say we are introducing a bug and gain nothing for that price.

It's not only the case of thirdparty tooling, we can name directories manually and cause problems to ourselves unaware of leaky solution above.

[akien-mga]

I don't think it's an optimization, it's needed if you change the project data path. Previous code worked because everything starting with . was ignored. But now if you change .godot to myfolder, this wouldn't be ignored with an exact check: myfolder/some.png. So if you want to do it right, you need to check for path == project_dir || path.starts_with(project_dir + "/"), after checking that project_dir doesn't end with a /... Maybe sprinkling some replacement for Windows backslashes too if that's not handled upstream.

It's not only the case of thirdparty tooling, we can name directories manually and cause problems to ourselves unaware of leaky solution above.

This leaky solution is made for third-party tooling. You shouldn't use it if you don't have tooling that requires it. It's an advanced option, provided as is, without guaranteed (for example it doesn't work with Mono which can't know about it).

[Listwon]

I don't think it's an optimization, it's needed if you change the project data path.

I'm talking only about the == to begins_with() change, sorry for the misunderstanding. == is not even faster than begins_with() in this case and doesn't contribute positively to the logic. The whole PR is a good solution by itself. Just changing == to begins_with() is leaky. I have to check if it's possible to set the dir to empty string because in that case begins_with() will always return true and break the whole project.

[Listwon]

== to begins_with() has nothing to do with the idea behind the PR (which I agree with, to fix problems with third party tooling), it just introduced possible bugs

[akien-mga]

I have to check if it's possible to set the dir to empty string because in that case begins_with() will always return true and break the whole project.

That seems possible indeed, should be fixed.

== to begins_with() has nothing to do with the idea behind the PR (which I agree with, to fix problems with third party tooling), it just introduced possible bugs

Well I assumed that it was changed for a reason to handle subpaths of the project directory. But if you can confirm that it's not needed, then yes, there's no reason to use a more computationally intensive String method over an equality check.

[Listwon]

I've just confirmed that:

• you can set the path to empty string (I'm not sure about all of the consequences, but I don't think it should be allowed)
• setting it to an empty string causes begins_with() to ignore all of the folders in project path (I've created Test folder and it wasn't shown in the FileSystem, moving files to folder makes them invisible to the editor) and import assets directly in the root path
• it will be the same if you set the path to imported (as mentioned above) and then name other folders imported_models or imported_from_psd etc.

TestPathPR.zip

[akien-mga]

If we want this to be fool-proof, it needs much better validation. The ProjectSettings constructor should check if the provided setting is a valid identifier that can be used as folder name, and if not, fall back to .godot/.import.

[m4gr3d]

@Listwon @akien-mga Thanks for the feedback!

I'm leaning toward restricting the option to just using (or not) a hidden directory:

boolean flag with the following:

• Godot 3.x: .import -> import
• Godot 4.x: .godot -> godot

That'd accomplish the initial goal while removing a lot of the raised concerns. In addition, it'd make support by tools easier as they would only have a couple options to cycle through.

@Listwon The use of begins_with() in EditorFileSystem::_should_skip_directory, FindInFiles::_scan_dir is to handle the edge case @akien-mga referenced.
For example, for a value of res://godot as the project data path, _should_skip_directory and _scan_dir fail when using == to check against values like res://godot/ or res://godot/<sub_file_or_dir>.

An improvement here would be to use begins_with() with a trailing slash added to the project data path, but I don't see a way around using begins_with().

The update to EditorFileDialog::_item_list_item_rmb_selected is to make the logic consistent with the master branch; the logic there had switched from == to begins_with() quite a while ago.

[m4gr3d]

@akien-mga @Listwon See #53779 for an implementation of my above suggestions.