Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load assets before enabling editor plugins #52344

Merged
merged 1 commit into from
Sep 15, 2021

Conversation

TechnoPorg
Copy link
Contributor

Closes #36713 by moving the code for enabling plugins in EditorNode from NOTIFICATION_READY to after the first scan of the filesystem has been completed.

Moves the code for enabling plugins from NOTIFICATION_READY to after the first scan has been completed.
@Calinou Calinou added bug topic:editor topic:import topic:plugin cherrypick:3.3 cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Sep 2, 2021
@Calinou Calinou added this to the 4.0 milestone Sep 2, 2021
@YuriSizov YuriSizov requested review from a team September 9, 2021 10:11
Comment on lines +961 to +971
// Only enable addons once resources have been imported
_initializing_addons = true;
Vector<String> addons;
if (ProjectSettings::get_singleton()->has_setting("editor_plugins/enabled")) {
addons = ProjectSettings::get_singleton()->get("editor_plugins/enabled");
}

for (int i = 0; i < addons.size(); i++) {
set_addon_plugin_enabled(addons[i], true);
}
_initializing_addons = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this mean that it will re-initialize addons every time sources change, instead of only once at startup?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so, because this code will only run when waiting_for_first_scan is true.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah right, I misread as GH messes tab width for me between near and extended diff.

@akien-mga akien-mga merged commit a2b727e into godotengine:master Sep 15, 2021
@akien-mga
Copy link
Member

Thanks!

@TechnoPorg TechnoPorg deleted the import-assets-first branch September 15, 2021 18:48
@akien-mga
Copy link
Member

akien-mga commented Sep 21, 2021

Cherry-picked for 3.4.

Edit: Reverted due to regressions: #52968 and #52995.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 21, 2021
@akien-mga
Copy link
Member

akien-mga commented Sep 21, 2021

Cherry-picked for 3.3.4.

Edit: Reverted due to regressions.

@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 27, 2021
@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Nov 15, 2021
@akien-mga
Copy link
Member

In the end I also reverted this in master since we did not find a fix yet for the regressions.

@TechnoPorg
Copy link
Contributor Author

TechnoPorg commented Nov 16, 2021

I'm really sorry about this. I started high school in the fall and the workload has been harder to adjust to than I expected that it would be. Also, I had an issue to do with outdated graphics drivers that I only fixed yesterday, which was causing Godot to crash on launch. Neither of those excuses are preventing me from working on Godot now, though, so I will try and be more helpful in the future.

@akien-mga
Copy link
Member

No worries, I wasn't trying to blame you or put pressure on you to fix the regression. But indeed when a known regression couldn't be fixed fast, it's usually best to rollback the change, and re-attempt it at a later time to solve both the original issue and the regressions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Assets should be imported before running plugins and tool scripts
3 participants