Update filesystem dock on script, scene, and resource creation

[YuriSizov]

Supersedes #22686.

Fixes #22475 among other related issues. With this change the filesystem dock correctly updates on script creation, as well as any resource saving (so that we support resources and scenes, as mentioned by @KoBeWi). Unfortunately, proposed additions to _make_scene_confirm() and _resource_created() wouldn't work, as scenes and resources are saved after these calls. So I've moved to signals. Scenes don't have any that I can see, so I've reused the one for resources (after all, PackedScene is a resource, so it should be ok).

Note that when a folder is created, the whole dock shows a progress bar, but this is not the case for these new rescans. Not sure what's the problem here, as that code is supposed to be triggered by _rescan(). Either way this only results in the file list displaying the old order for a second and then quickly updating to display the correct names and order.

Also note, that this is unlikely Windows-specific (the initial issue with casing is, but not the entire situation).

@KoBeWi feel free to merge if you approve, consider me implementing the suggested changes as my own approval as well 🙂

[KoBeWi]

Seems like saving any file resource any time will now trigger _rescan(). It sometimes causes the filesystem dock to hang for a couple of seconds with the "Scanning" message, which is undesired when happening so often. Rescan should be limited only to cases where filesystem actually changes, i.e. there is new file or file was renamed, otherwise it's useless. Maybe as a solution you could temporarily connect the resource_saved signal as oneshot when New Resource dialog opens and disconnect if it's cancelled.

Also for whatever reason the editor will hang infinitely upon exit and spam this error:

ERROR: Disconnecting nonexistent signal 'resource_saved', callable: FileSystemDock::_rescan.
   at: (core\object\object.cpp:1406)

[YuriSizov]

Maybe as a solution you could temporarily connect the resource_saved signal as oneshot when New Resource dialog opens and disconnect if it's cancelled.

This won't work for scenes. Scenes can be saved at any point in future, and we don't actually have a reference to the scene's resource. And the file path can also change for reasons outside of the editor UI interaction. I guess, we can debounce the rescan. How does that sound?

[KoBeWi]

I guess, we can debounce the rescan.

It's not a matter of debouncing. If you are working on one scene for e.g. 30 minutes and you save every minute or so (including by running the game), even with debouncing the rescan would be triggered many times without any need.

This won't work for scenes.

Yeah scenes are a weird case. We could either check if file exists upon saving scene and if not, trigger rescan. Or give the "New Scene" dialog the deserved makeover, because the fact that it doesn't create the scene was complained about in the past.

Personally I don't mind the current behavior, so I'd go with the first option. The second one is out of scope of this PR, so we could also leave the scenes for later I guess.

See discussion

[YuriSizov]

Okay, I've added a new resource_created signal for the EditorNode which fires alongside resource_saved, but only if that file never existed before. Seems to work for both resources and scenes.

As for the disconnect issue, it seems to be some weird case with unbind. Replacing a callback with unbind with a dedicated method fixed it. I'll report it later.

[KoBeWi]

Seems like it's still possible to create fake folder using scenes. The reason is that the scene tab will remember the path you created the scene with. If file didn't exist when saving, the filesystem will rescan. However it exists on subsequent saves, which creates the folder and doesn't rescan. This can be fixed by updating the scene path in EditorData when the file is created.

[YuriSizov]

It seems that these issues with saving scenes boil down to much bigger issues within the file handling code, specifically on Windows where the file system is case-insensitive by default. There is also a troubling "feature" that allows users to enter directory names into the FileDialog file name, which should probably be disallowed.

It doesn't seem possible to solve these problems in this PR without reliance on tricks and hacks, or making changes to FileAccessWindows and FileDialog which could affect a lot of other places in the code base. As such, I'd prefer we keep this PR to what it currently is. It does solve a couple of problems and the one that is linked in particular. So it's an improvement and it doesn't contain hacks to achieve those.

I'll file separate reports on the aforementioned issues so we can look for a better, proper solution to those.

[KoBeWi]

Actually I'd leave it like this. The scene issue needs to be specifically triggered, it won't happen to regular users. #famouslastwords

[KoBeWi]

I'd update the commit message though. It no longer updates on saving, only on creation.

[YuriSizov]

Done. Also rebased for the heck of it. And reported those two other problems in the meantime.

[groud]

The changes look good to me but I am not very familiar to the impact this might have, as I am not familiar with how the editor handles files internally (I only worked on the UI part). It likely needs a review from reduz to be sure.

[groud]

You should probably protect the remaining of the function from crash with ERR_FAIL_COND(!da).

[groud]

Same here.

[YuriSizov]

After a discussion with reduz it was determined that the initial approach of this PR was excessive. EditorFileSystem controls everything here, and if there are ghost folders or broken order of files, it's EditorFileSystem that needs to be fixed. The ordering problem (scripts/resources created via the context menu in the FileSystem dock can appear out of order) can likely be fixed very easily with a proper sort, but the ghosting due to case-insensitivity requires extra work on the platform code.

The problem is close to what I've described in #52231. It seems that the check I've mentioned there only tests the name of the file, not the entire path, so that needs to be fixed so that we properly report the issue to the user. We also need to expose a method to validate/sanitize the path against the file system, so that EditorFileSystem can correct itself. This method can then be used when creating and saving resources/scenes/scripts/etc.

This method can be exposed on DirAccess or as a part of OS with platform-specific implementations for either. https://stackoverflow.com/questions/4763117/how-can-i-obtain-the-case-sensitive-path-on-windows was mentioned as a good reference for the implementation. It may need extra code to check for file existence and whatnot. Specifically on Windows it's also important to remember that it supports both forward and backward slashes in paths.

So this is what needs to be done to properly fix this. @mhilbrunner if you still want to work on #52231, I think we should probably do it together to close both issues at the same time.