GDScript: Fix get_dependencies by deferring resolution#113791
Closed
cpl-s-matsuyama wants to merge 2 commits intogodotengine:masterfrom
Closed
GDScript: Fix get_dependencies by deferring resolution#113791cpl-s-matsuyama wants to merge 2 commits intogodotengine:masterfrom
cpl-s-matsuyama wants to merge 2 commits intogodotengine:masterfrom
Conversation
The parser and analyzer now track the dependencies of the script and return the list when the resource loader ask for them. What is considered a dependency: - Any `preload()` call. - The base script this one extends. - Any identifier, including types, that refers to global scripts. - Any autoload singleton reference.
Delay the resolution of script dependencies until after the pending updates are processed to avoid issues with initialization order.
Member
|
Thanks for the PR! However, I'm not sure this is even possible if we want to provide a full dependency graph. The resource system doesn't support circular dependencies, unlike GDScript. The concern is that this will cause issues on the resource system side, not to implement on the GDScript side. |
Member
|
Even when putting concerns about cyclic dependency support aside, I don't think this is an approach that's compatible with current established behaviour. See #116673 (comment) |
Contributor
Author
|
Thanks for checking. I looked into #116673 and agree — having a dedicated method like get_cyclic_dependencies seems like the right approach. Closing this PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes the issue where get_dependencies() would return an empty list in certain cases, as reported in #90643.
This is a re-implementation of the fix attempted in #90860, which was subsequently reverted in #91908 due to a regression reported in #91726 (import errors).
Unlike the previous attempt, this implementation avoids the regression by deferring the dependency resolution. The previous attempt resolved dependencies immediately, which led to the reported import errors. This approach ensures that dependencies are handled safely without interfering with the parsing process.
I have verified that this solution fixes the original issue (#90643) without causing the regression reported in #91726.
Fixes #90643