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

[3.x] Try loading the path specified in a preload(..) method call with the ResourceLoader if the script code completion cache is not yet available #69362

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

Maran23
Copy link
Contributor

@Maran23 Maran23 commented Nov 29, 2022

Fixes: #69360
Fixes: #78475
Maybe related to: #35248 (Not sure as I didn't used an external editor, but the issue there sounds very similar)

This fixes the problem that autoloaded nodes could not be loaded because they (or there children) preload other nodes in their script.
This only happened at the start of Godot. When the autoloaded nodes will be resolved, there is no ScriptCodeCompletionCache yet.

This scenario is also handled above, so we may as well handle it here in the same way.

res = ResourceLoader::load(path);

Why exactly is the ScriptCodeCompletionCache not yet resolved?
In editor_node:

  • Line 6348: ProjectSettingsEditor is instantiated -> EditorAutoloadSettings -> Autoloaded scenes will be created
  • Line 6985: ScriptEditorPlugin is instantiated -> ScriptCodeCompletionCache will be instantiated

ANOTHER POSSIBLE FIX

Initialize the ScriptCodeCompletionCache (ScriptCodeCompletionCache::get_singleton()) before resolving the autoloaded scenes.
Then the ScriptCodeCompletionCache cache can be reused. (Order needs be changed in editor_node)

Moving down the ProjectSettingsEditor in the instantiation order (after ScriptEditorPlugin) does work and everything looks normal. And the ScriptCodeCompletionCache is created before the autoloaded nodes are created. But this is a much more risky change.

Note: This bug does not happen in Godot 4.0

@Maran23 Maran23 requested a review from a team as a code owner November 29, 2022 23:14
@Calinou Calinou added bug topic:editor cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release labels Dec 1, 2022
@Calinou Calinou added this to the 3.6 milestone Dec 1, 2022
@akien-mga akien-mga changed the title Try loading the path specified in a preload(..) method call with the ResourceLoader if the script code completion cache is not yet available [3.x] Try loading the path specified in a preload(..) method call with the ResourceLoader if the script code completion cache is not yet available Jan 3, 2023
…ResourceLoader if the script code completion cache is not yet available

This fixes the problem that autoloaded nodes could not be loaded because they (or there children) preload other nodes in their script. This was only happening on the start of Godot, as when the autoloaded nodes are resolved there is no code completion cache yet.
Comment on lines +530 to +531
} else {
res = ResourceLoader::load(path);
Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with this code but I feel like the comment in the !validating branch:

//this can be too slow for just validating code

actually explains why this else branch does not do a load, which is expensive.

CC @vnen @adamscott (note: 3.x code)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Same thought, but better then failing. Since this only affects autoloaded Nodes that uses preload, it seems okay to me

Valla-Chan added a commit to Valla-Chan/Godot-3.5.1-Ghodost that referenced this pull request Jan 20, 2024
(Try loading the path specified in a preload(..) method call with the ResourceLoader if the script code completion cache is not yet available)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug cherrypick:3.5 Considered for cherry-picking into a future 3.5.x release topic:editor topic:gdscript
Projects
Status: PRs for 3.6 beta 1
Development

Successfully merging this pull request may close these issues.

None yet

3 participants