Fix Addon requires editor restart to become functional

[Hilderin]

This is a possible fix for #92493

The idea is that some resources are needed on startup before the reimportation of missing resources in cache but we cannot move the addon loading after the reimportation because addons can have custom importers, but what if the ResourceLoader could in that situation import on the fly the missing resources while loading the addons?

I tried that idea with the MRP from https://github.com/KoBeWi/Metroidvania-System/tree/bug_branch (thanks @KoBeWi) and it seems to work. IMPORTANT: to load this project without any errors, you need https://github.com/godotengine/godot/pull/92303. To make it work only with this PR, the global_script_class_cache.cfg must be present in the .godot folder.

Note: This is more a potential workaround from the current circular problem where addons need resources but resource needs addons.

One limitation of this solution is if an addon needs a resource from a custom importer, it will fail or use the default importer. Another limitation is that there is no progress bar or loading indicator while importing resources before the first scan, so if an addon needs a lot of resources, the editor could be unresponsive for quite some time.

To see it in action:

https://github.com/godotengine/godot/assets/81109165/250191c3-03c0-4465-863c-adf9c9982df9

[Hilderin]

Rebased following merge conflicts.

[KoBeWi]

Looks like the addon is functional even without #92303 EDIT: Ah I see you mentioned "without any errors". I don't think it's that important, as long as stuff works correctly.

[dsnopek]

I tested this with a project that has node types from a GDExtension in its main scene. I deleted the .godot directory, and then opened in an editor built with this PR and it seems to work! All the nodes were present in the scene, and seemed to work normally (which isn't the case without this PR - I usually need to open the Godot editor twice).

However, I'm still getting a bunch of errors about the GDExtension classes to the console as the editor starts up:

ERROR: Cannot get class 'OpenXRFbPassthroughExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: Cannot get class 'OpenXRFbRenderModelExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: Cannot get class 'OpenXRFbSceneCaptureExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: Cannot get class 'OpenXRFbSpatialEntityExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: Cannot get class 'OpenXRFbSpatialEntityQueryExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: Cannot get class 'OpenXRFbSpatialEntityContainerExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: Cannot get class 'OpenXRFbSceneExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: Cannot get class 'OpenXRFbHandTrackingAimExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: Cannot get class 'OpenXRFbHandTrackingCapsulesExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: Cannot get class 'OpenXRFbCompositionLayerSettingsExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: Cannot get class 'OpenXRHtcFacialTrackingExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: Cannot get class 'OpenXRHtcPassthroughExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: GDScript bug: Native class "OpenXRFbRenderModel" not found.
   at: _gdtype_from_datatype (modules/gdscript/gdscript_compiler.cpp:123)
ERROR: GDScript bug: Native class "OpenXRFbRenderModel" not found.
   at: _gdtype_from_datatype (modules/gdscript/gdscript_compiler.cpp:123)
ERROR: GDScript bug: Native class "OpenXRFbRenderModel" not found.
   at: _gdtype_from_datatype (modules/gdscript/gdscript_compiler.cpp:123)
ERROR: GDScript bug: Native class "OpenXRFbRenderModel" not found.
   at: _gdtype_from_datatype (modules/gdscript/gdscript_compiler.cpp:123)
ERROR: GDScript bug: Native class "OpenXRFbRenderModel" not found.
   at: _gdtype_from_datatype (modules/gdscript/gdscript_compiler.cpp:123)
ERROR: GDScript bug: Native class "OpenXRFbRenderModel" not found.
   at: _gdtype_from_datatype (modules/gdscript/gdscript_compiler.cpp:123)
ERROR: GDScript bug: Native class "OpenXRFbRenderModel" not found.
   at: _gdtype_from_datatype (modules/gdscript/gdscript_compiler.cpp:123)
ERROR: Cannot get class 'OpenXRFbHandTrackingMeshExtensionWrapper'.
   at: set_object_extension_instance (core/object/class_db.cpp:639)
ERROR: In Object of type 'OpenXRExtensionWrapperExtension': Attempt to connect nonexistent signal 'openxr_fb_hand_tracking_mesh_data_fetched' to callable '<CallableCustom>'.
   at: connect (core/object/object.cpp:1423)
ERROR: In Object of type 'OpenXRExtensionWrapperExtension': Attempt to connect nonexistent signal 'openxr_fb_hand_tracking_mesh_data_fetched' to callable '<CallableCustom>'.
   at: connect (core/object/object.cpp:1423)
ERROR: In Object of type 'OpenXRExtensionWrapperExtension': Attempt to connect nonexistent signal 'openxr_fb_hand_tracking_mesh_data_fetched' to callable '<CallableCustom>'.
   at: connect (core/object/object.cpp:1423)

Everything does still seem to work, though.

[Hilderin]

I don't think this PR should fix that, this PR is fixing a problem when an autoload or a plugin tries to access a resource before the first scan. Were you talking about: #93972 ? In the MRP a linked with the PR, there is a node of the type Example from a GDExtension in the main scene and I'm not able to reproduce the problem you mention with the code from #93972.

[dsnopek]

Oops, sorry, I was on the wrong PR!

[Hilderin]

Rebased from master to fix a merge conflict.

[akien-mga]

Could use a rebase for good measure as we've merged a lot of changes lately. I'll try to test soon myself.

[Hilderin]

Rebased done! I retested with the MRP project from the 2 issues linked in the PR descriptions. The project loaded perfectly without any error and I saw that the added code was executed as expected.

[akien-mga]

I tested on a few projects and confirmed that this seems to work well :tada:

I'm slightly concerned that the implementation looks a bit hacky though, with silencing error messages, and retry/loop shenanigans in editor only code. I'm not very familiar with that code so it's hard to assess whether there's a good approach, though. Could be good to have some extra input from @reduz and @RandomShaper.

Edit: In particular, outright silencing errors like this might mean that we'll miss actual errors which aren't just "I tried to load a file that wasn't imported yet" (e.g. originated from a different thread, or actual bugs in the import pipeline). If we need a way to do "try to load a dependency, but don't error yet, it's valid if it's missing", maybe we should add a dedicated function for that that doesn't print errors on cases of expected failure.

[reduz]

I think on the general level this PR is ok, but I don't feel very comfortable having import logic at core level. Maybe what we could do is have a function pointer that is set to ResourceImporter to do this logic and implement it effectively in EditorFileSystem so we have it a bit more centralized and don't have actual import code in core.

[Hilderin]

I made significant refactoring following the suggestions of @reduz. I hope I understood correctly your suggestion.

The logic of the import and retry is now in EditorFileSystem. I kept the modifications in ResourceFormatImporter::load at the minimum so the code for the retry is executed only on startup during the first scan. I think that way the code is better encapsulated and easier to understand while maintaining the functionality.

[akien-mga]

Thanks!

[LucaVazz]

Thank you all for working on this fix 🙂

Would it be possible to backport this fix for a 4.3 patch release? The underlying issue currently is a blocker to use the 4.3 release in a CI workflow when using addons.

[GreenCrowDev]

As already requested earlier, is it posible to cherry pick this for a 4.3 patch? Has this already been done? I'm not sure how to check.

[matheusmdx]

@LucaVazz @GreenCrowDev Generally when the pr can be cherry-picked a tag is add to sign that but in any moment this label was aded so probably this pr is too risky to be cherry-picked, require extra work for do it or simply is incompatible with old versions. From the fact a lot of people worked on that and no one cited nothing about cherry-pick probably this can't be cherry-picked (or everyone just forgot that).