Fix synchronization of global class name

[Hilderin]

This should fix the issues #76380, #75388, #81615 and #92054

This should manage these situations:

• At startup, the global_script_class_cache.cfg is missing or incomplete but the filesystem cache is present
• At startup, the global_script_class_cache.cfg and the system cache are missing
• At startup, the global_script_class_cache.cfg is missing classes but are present on disk
• At startup, the global_script_class_cache.cfg has a class that does not exists on disk
• At startup, the path for class name in global_script_class_cache.cfg has changed
• In the editor if the user duplicates a script with a class name and change the class name after
• In the editor if the user changes the class name for an existing class name and change it again
• In the editor if the user deletes a folder containing a script with a class name that is used by another script
• Exporting project from command line that uses global class names in addons without or with an invalid global_script_class_cache.cfg
• Updating scripts from outside the Editor that uses new global class name

The global class name are now loaded from the scripts at the editor startup, always recreating the global_script_class_cache.cfg. When not in the editor, the global_script_class_cache.cfg is used for performance when the project runs.

Bugsquad edit:

• Fix Class names stop working after a while #76380
• Fix Plugins & Autoloaded scripts breaks project if classes missing from global_script_class_cache.cfg #75388
• Fix Using other scripts by their class_name in an import plugin causes import to fail #81615
• Fix Pulling GDScript changes that introduce a new class_name results in errors #92054

Edited: Adding modifications for the issue #81615
Edited: Updating for the modifications loading class name at startup from scripts.
Edited: Adding modifications for the issue #92054
Edited: Adding fix for deletion of folders

[donn-xx]

If this works, you will be a God amongst ots! :D

[Hilderin]

I added some modifications to fix the issue #81615
I tested with the MRP project from this issue when starting the projet in the editor and exporting from the command line.

[Hilderin]

Found a problem in the is_ready condition in editor_node. Last commit should fix that.

[mihe]

While I'm sure this change will be of help to many people, since it does seem to resolve the issue of scripts ending up in a perpetual state of error and needing to be resaved one at a time, it obviously does not change the actual order of global_script_class_cache.cfg being populated after the initial load of the autoloads, so you'll still see errors from that initial broken compilation of any autoloads that reference any class_name, although it will (mostly) only output to the command-line (i.e. stderr) with this change, since EditorLog has yet to initialize when autoloads are first loaded.

This does however mean that importing from the command-line using --headless, through something like a CI job, either explicitly through the new --import option or implicitly through --export-debug/--export-release, will still emit errors. I'm able to reproduce this by exporting the repro project in #75388 for example, even with the latest commit as of this comment.

There's also the issue of the more or less official (and fairly common) plugin pattern where you have a @tool script that registers a potentially new autoload using add_autoload_singleton in its _enter_tree. In such a case add_autoload_singleton will trigger update_autoload, which kicks off another broken (assuming it depends on some other class_name) autoload compilation, which this time actually emits to the editor log, since EditorLog is up and running by that point. You'll see that in this repro that I just made: autoload-plugin-errors.zip

Both of these are of course largely "cosmetic" issues (as far as I can tell), since these errors seem to eventually get dealt with by your recompile_invalid_autoload_scripts, but any non-trivial project that uses class_name or a plugin autoload (like GodotLogger) in a widespread manner will still end up with hundreds/thousands of errors in these two scenarios if there's no valid global_script_class_cache.cfg.

Anyway, again, I'm not saying this change shouldn't be merged, and I wouldn't/shouldn't be the judge of that in the first place, but I figured I'd bring this up in case it hadn't been considered already, or if there's some discussion to be had about a more holistic solution.

[Hilderin]

Thanks @mihe for your detailed response and to take the time to test my modifications. I'll try your autoload-plugin-errors.zip and see if I can do something about the error messages.

If I understand correctly, with this fix, you have error messages in the console and output panel (and potentially a lot of them), but with the recompile_invalid_autoload_scripts, at some point they are working without having to resave them manualy?

[mihe]

If I understand correctly, with this fix, you have error messages in the console and output panel (and potentially a lot of them), but with the recompile_invalid_autoload_scripts, at some point they are working without having to resave them manualy?

Just to clarify, the issue with the errors in the two scenarios (--headless import and autoload plugins) I mentioned are not regressions caused by this PR. I mostly just wanted to highlight that they remain despite these changes, in case there's some discussion to be had about some more holistic solution/change to global_script_class_cache.cfg.

Those two scenarios also seem largely orthogonal to the issue of needing to resave files individually. I honestly haven't run into the issue of files needing to be resaved much myself, but when testing your PR on the repro in #75388 I seemed to observe that having been fixed. Don't quote me on that though. :)

But yes, the error messages show up in the console in both scenarios and the output panel for the plugin scenario, if by console you're referring to a command-line terminal.

[Hilderin]

Thanks for the clarifications :)

[donn-xx]

A holistic solution is vital. If class_name is not immediately and always respected, then what is the point of class_name? All class_names, no matter their origins, must be known at all times.

[Hilderin]

I totally agree with that. Actually, the way I see it, the only way to really do that would be to remove completely the global_script_class_cache.cfg and reload all the class on startup from the code. I don't why it was not done like that from the beginning, maybe for performance issue?

Just today I think I saw 2-3 news issues linked to global script class.

I can try to implement it that way if everyone thinks that's a good idea? I'm a pretty new contributor, I usually try to limit the amount of code I touch, but I'm welling to try!

[donn-xx]

I don't know why there's a cache file either, but I guess it was for speed.

Perhaps moving it out of the dot-godot directory could be an answer; but that would involve merging it with other class caches that come from plugins and general project churn. That sounds like hell.

The simplest way feels like, on startup, grepping all the resource and gdscript files for 'class_name' and thence into a hash map.

[Hilderin]

I took your advice @mihe and @donn-xx and I’m pretty happy with the result.

This commit removes the need for global_script_class_cache.cfg when starting the editor. It loads class_name from the scripts on disk at startup and rewrites the global_script_class_cache.cfg. I kept the global_script_class_cache.cfg for exported project and when running the project to be sure that the start time is not affected. Doing it like that, I was able to remove the recompile_invalid_autoload_scripts after the first full file scan, preventing potential double error messages.

I tested with a project having 3000 gdscripts of 22ko each. At first, it took 10 sec to parse every file at startup. After a bit of debugging I found out that the complete file was parsed just to find the class_name and the icon. I added a new parameter to the GDScriptParser.parse to only parse the script header. After this modification, it took in a debug build only around 400ms to read all the 3000 files on my Intel i5-11600 3.9Ghz on Nvme. It’s not extensive testing but I think it’s good enough, it will append only on startup of the editor.

With this modification the test project from @mihe loads without any error or warning event if the .godot folder is missing or the
global_script_class_cache.cfg missing or invalid.

[Hilderin]

I also tested the MRP from #75388 and #92054 with these news modifications, and everything seems to work fine, even on fresh git clone (no .godot folder).

[donn-xx]

I am holding my breath! So excite!

[AThousandShips]

That's not relevant to this so please stay on topic

[KoBeWi]

The changes look fine overall. I tested it in various scenarios and a complex project and the script cache works correctly.

Aside from the last comments I left, there code could get more consts for some pointers/Strings, but it's not really important.

[adamscott]

@Hilderin Don't hesitate to come chat in the developers chat! Especially for such a big PR! :)

[akien-mga]

I've made some test builds to help validate this PR, in hope that some Godot users could try it out on their projects before we include it in the next 4.3 beta.

I built the editor for Windows, macOS, and Linux, with:

• Current master branch (7907ef8) as point of comparison: https://downloads.tuxfamily.org/godotengine/testing/4.3-beta-7907ef8/
• This PR (Fix synchronization of global class name #92303) rebased on top of 7907ef8: https://downloads.tuxfamily.org/godotengine/testing/4.3-beta-pr92303/

To test and help validate this PR:

• Make a copy of your Godot project(s) you want to test with
• Delete .godot folder
• Open the project in the 7907ef8 build, take note of potential errors that may not be present in the version you were using (to be reported separately as general 4.3 bugs). Try usual workarounds like restarting the editor once after the initial import, and check if your game works as intended.
• Close editor
• Delete .godot folder
• Open the project in the Fix synchronization of global class name #92303 build and see if it performs better, using the same steps as with the 7907ef8 build. Ideally there should be a lot less errors about class names not being found, though some may still appear if using addons (this will be tackled later in another PR).

If your project generally works well in current 4.3-beta / 7907ef8, try to use the build from #92303 to do some development work like creating new global classes, renaming some, etc., to find potential bugs.

If your project doesn't work at all with either build (vanilla master and this PR), then I successfully tricked you into helping beta test 4.3 generally, so please report those problems in dedicated issues so we make sure to fix it for 4.3-stable ;)

[akien-mga]

Did some testing on various projects and it seems to work well for me.

I also got reports from various users testing it and they either saw improvements, or no new issue in their daily workflow, which is also a good sign.

So let's give it a spin and see how it fares in 4.3-beta3.

[akien-mga]

Thanks! Amazing work 🎉

[Hilderin]

Thanks for your help and support!!

[donn-xx]

Sorry for being away. Have just done my first few tests for this fix and it's looking real good. I'll poke at my addon project and keep notes.