Defer addons loading after the first file scan/import #86453

adamscott · 2023-12-22T18:41:00Z

This fixes the issue where the active plugins that are enabled refer to not yet imported textures or files.

What it does

It defers the load of plugin scripts after the first file scan. So, no plugin will be loaded before all the files would be imported.

What happened before is that Godot would complain if loaded addons would try to load not yet imported files, but it was only a race condition because the engine would have been done so a few seconds (if not milliseconds) later.

These errors were common for projects were the addon would be installed before opening Godot. The only fix was to restart Godot, which is unfortunate.

Fixes

Fixes #77037
Fixes #23937

adamscott · 2023-12-22T18:53:26Z

editor/editor_node.cpp

-			{
-				started_timestamp = Time::get_singleton()->get_unix_time_from_system();
-				_initializing_plugins = true;
-				Vector<String> addons;
-				if (ProjectSettings::get_singleton()->has_setting("editor_plugins/enabled")) {
-					addons = GLOBAL_GET("editor_plugins/enabled");
-				}
-
-				for (int i = 0; i < addons.size(); i++) {
-					set_addon_plugin_enabled(addons[i], true);
-				}
-				_initializing_plugins = false;
-
-				if (!pending_addons.is_empty()) {
-					EditorFileSystem::get_singleton()->connect("script_classes_updated", callable_mp(this, &EditorNode::_enable_pending_addons));
-				}
-			}
-


I'm currently thinking if we should still import the addons here when were not in the "editor". We cannot import resources outside the editor, so we should import plugins faster for games.

Not sure what do you mean by "editor", but this code never executes at runtime.

adamscott · 2023-12-22T20:42:06Z

editor/editor_node.cpp

+	if (!waiting_for_first_scan) {
+		return;
+	}


Here, I just early exited instead of wrapping the whole contents of the function inside an if block.

KoBeWi · 2023-12-22T21:42:30Z

Looks similar to what #70668 tried to do.

EDIT:
Tested with my plugin and it seems to load properly on first launch 🤔 (it needs some modifications before testing, because I force reload anyway).

KoBeWi · 2023-12-22T23:10:55Z

editor/editor_node.cpp

+		if (!pending_addons.is_empty()) {
+			EditorFileSystem::get_singleton()->connect("script_classes_updated", callable_mp(this, &EditorNode::_enable_pending_addons));
 		}


This part and related code can be removed now (basically reverting #70668), as it's no longer necessary.

KoBeWi

IIRC the original reason why plugins were loaded before scanning was that there can be EditorImportPlugin (or similar) among them, which is relevant for the scan. However I tested some custom EditorImportPlugin and it seems to import stuff as soon as it's enabled.

But that was a simple case; maybe a problem arises for more advanced import plugins that customize assets which are already handled. Needs a check from @godotengine/import

adamscott · 2023-12-23T11:32:23Z

IIRC the original reason why plugins were loaded before scanning was that there can be EditorImportPlugin (or similar) among them, which is relevant for the scan. However I tested some custom EditorImportPlugin and it seems to import stuff as soon as it's enabled.

Where I think it would fail is when you clear the .godot directory, the first load import will not include the addons EditorImportPlugin. But again, it would have a chance to fail to do so because the plugin could have preloaded a (yet) non-imported file beforehand.

Zylann · 2024-01-01T17:36:15Z

What if a user clones a project with import plugins in it, and then opens the editor? It sounds like it will now fail to load properly (in particular if the main scene uses such resources), as importing will happen before the import plugins are added (in that situation, the plugin won't be transitionning from disabled to enabled while the editor has already been running).

Maybe a second scan is necessary if import plugins are added (but still before the main scene loads because it could otherwise fail).

Ideally, an alternative would be the ability to somehow make sure all import plugins (core and custom) are loaded before plugins start to add UIs. That would allow to do a single import scan after that, and guarantees load can be used after it. Because this isn't just happening with icons, plugins can add a bunch of controls which could reference more textures. Though not sure what changes that would require, the API doesnt seem to have been thought for this.

So far in my own plugin I've been working around this by loading icons with something like ImageTexture.create_from_image(load_image(...))

akien-mga

This is just moving code around to run later / on each source change, so it seems good to me.

Edit: Just saw Zylann's question, should be answered before merging.

YuriSizov · 2024-01-08T10:15:46Z

IIRC the original reason why plugins were loaded before scanning was that there can be EditorImportPlugin (or similar) among them, which is relevant for the scan. However I tested some custom EditorImportPlugin and it seems to import stuff as soon as it's enabled.

Well, that is still the main problem. Resources may need a plugin to be loaded/imported, and plugins may need a resource to work, just like Zylann explains.

Just moving code around is not the proper solution here. We should instead add more steps to the plugin initialization instead. Either some hard defined hooks, similar to how extensions are initialized with different "levels", or an automatic solution which can detect failed imports and retry everything that fails until all issues are resolved (which seems like a complicated task, so probably not).

On top of all, I'd wait with modifications to the editor load order until I can do the refactor. There is no point making ad hoc patches for something that is going to be reworked in the same version.

YuriSizov

See above.

KoBeWi · 2024-01-20T21:57:48Z

Found some project to test: #81615
Opening the project (after you follow the steps in the OP) will cause example.csv fallback to CSV Translation due to missing importer. Though it happens regardless if you use this PR or not. With this PR you are at least able to reimport without restarting the editor.

noidexe · 2024-01-30T20:26:39Z

FYI tool scripts set as autoloads exhibit the same issue. If they preload any resource that has not yet been imported parsing and execution will fail

adamscott · 2024-02-05T18:56:08Z

FYI tool scripts set as autoloads exhibit the same issue. If they preload any resource that has not yet been imported parsing and execution will fail

You mean that this PR fixes this issue?

noidexe · 2024-02-05T19:27:17Z

You mean that this PR fixes this issue?

No. I mean that tool autoloads have exactly the same problem for exactly the same reason so you may want to fix it here or on a seperate PR.

This introduces new issues, there's a chicken-and-egg situation that requires more work to untangle.

KoBeWi · 2024-07-26T12:07:00Z

Seems like #92667 fixes a very similar (if not the same) issue.

akien-mga · 2024-07-26T12:19:51Z

@Hilderin Could you check if #92667 supersedes this, and also fixes #77037 and #23937?

Hilderin · 2024-07-26T15:16:49Z

I tested #77037 and #23937 with the PR #92667.

For #77037: the PR does not fix the problem and I don't think the original problem was caused by the order in which the plugins are loaded. I created an alternative solution to this PR with less modifications: #94802

For #23937: the PR does fix the problem of having a preload resource in tool script when starting the editor without the .godot folder. The PR #92303 already fixes the other problems linked to global class names not being registered correctly on startup with the .godot folder missing or when adding the addon files while the editor is running.

Also, I'm worry about this PR which seems to change the order of loading the plugins. It will creates problems with custom loaders that could be registered in plugins.

In conclusion, I suggest #92667 and #94802 to fix these issues.

akien-mga · 2024-07-26T15:43:58Z

Superseded by #92667 and #94802.

adamscott added enhancement usability topic:plugin bug and removed enhancement labels Dec 22, 2023

adamscott self-assigned this Dec 22, 2023

adamscott added this to the 4.x milestone Dec 22, 2023

adamscott added the cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release label Dec 22, 2023

adamscott commented Dec 22, 2023

View reviewed changes

adamscott requested a review from KoBeWi December 22, 2023 20:41

adamscott commented Dec 22, 2023

View reviewed changes

adamscott force-pushed the load-addons-after-files-import branch from 1ec8760 to f135b02 Compare December 22, 2023 20:43

KoBeWi reviewed Dec 22, 2023

View reviewed changes

KoBeWi approved these changes Dec 22, 2023

View reviewed changes

akien-mga modified the milestones: 4.x, 4.3 Jan 2, 2024

akien-mga previously approved these changes Jan 2, 2024

View reviewed changes

akien-mga requested review from vnen and YuriSizov January 2, 2024 15:21

bitbrain mentioned this pull request Jan 3, 2024

Addon errors on first load bitbrain/beehave#270

Closed

YuriSizov suggested changes Jan 8, 2024

View reviewed changes

Hilderin mentioned this pull request May 28, 2024

Fix synchronization of global class name #92303

Merged

akien-mga modified the milestones: 4.3, 4.4 May 28, 2024

Load only addons after first file scan/import

81d577d

adamscott force-pushed the load-addons-after-files-import branch from f135b02 to 81d577d Compare June 24, 2024 15:11

AThousandShips added the cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release label Jul 25, 2024

Hilderin mentioned this pull request Jul 26, 2024

Fix autoload node cannot be accessed by plugin on start-up #94802

Merged

akien-mga closed this Jul 26, 2024

akien-mga added archived and removed cherrypick:4.2 Considered for cherry-picking into a future 4.2.x release cherrypick:4.3 Considered for cherry-picking into a future 4.3.x release labels Jul 26, 2024

akien-mga removed this from the 4.4 milestone Jul 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Defer addons loading after the first file scan/import #86453

Defer addons loading after the first file scan/import #86453

adamscott commented Dec 22, 2023 •

edited by KoBeWi

Loading

adamscott Dec 22, 2023

KoBeWi Dec 22, 2023

adamscott Dec 22, 2023

KoBeWi commented Dec 22, 2023 •

edited

Loading

KoBeWi Dec 22, 2023

KoBeWi left a comment

adamscott commented Dec 23, 2023

Zylann commented Jan 1, 2024 •

edited

Loading

akien-mga left a comment •

edited

Loading

YuriSizov commented Jan 8, 2024

YuriSizov left a comment

KoBeWi commented Jan 20, 2024

noidexe commented Jan 30, 2024

adamscott commented Feb 5, 2024

noidexe commented Feb 5, 2024

KoBeWi commented Jul 26, 2024

akien-mga commented Jul 26, 2024

Hilderin commented Jul 26, 2024

akien-mga commented Jul 26, 2024

Defer addons loading after the first file scan/import #86453

Defer addons loading after the first file scan/import #86453

Conversation

adamscott commented Dec 22, 2023 • edited by KoBeWi Loading

What it does

Fixes

adamscott Dec 22, 2023

Choose a reason for hiding this comment

KoBeWi Dec 22, 2023

Choose a reason for hiding this comment

adamscott Dec 22, 2023

Choose a reason for hiding this comment

KoBeWi commented Dec 22, 2023 • edited Loading

KoBeWi Dec 22, 2023

Choose a reason for hiding this comment

KoBeWi left a comment

Choose a reason for hiding this comment

adamscott commented Dec 23, 2023

Zylann commented Jan 1, 2024 • edited Loading

akien-mga left a comment • edited Loading

Choose a reason for hiding this comment

YuriSizov commented Jan 8, 2024

YuriSizov left a comment

Choose a reason for hiding this comment

KoBeWi commented Jan 20, 2024

noidexe commented Jan 30, 2024

adamscott commented Feb 5, 2024

noidexe commented Feb 5, 2024

KoBeWi commented Jul 26, 2024

akien-mga commented Jul 26, 2024

Hilderin commented Jul 26, 2024

akien-mga commented Jul 26, 2024

adamscott commented Dec 22, 2023 •

edited by KoBeWi

Loading

KoBeWi commented Dec 22, 2023 •

edited

Loading

Zylann commented Jan 1, 2024 •

edited

Loading

akien-mga left a comment •

edited

Loading