GDScript: Fix compilation of expressions compiling other classes #81577
Conversation
This file contains 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
anvilfolk
commented
Sep 12, 2023
Comment on lines
405
to
406
| // Should not need to pass p_owner since analyzer will already have done it. | ||
| res = GDScriptCache::get_shallow_script(global_class_path, err, parser->script_path); |
There was a problem hiding this comment.
For those curious, this is the fix. The other stuff is just trying to ensure dependencies are properly added in the GDScriptCache.
This PR is part of ongoing work on fixing cyclic dependencies in the GDScript compiler.
anvilfolk
force-pushed
the
thecycleeeeeeeeeeeeesaaaaaaaaaaaaah
branch
from
6b29a06 to
d330f56
Compare
ryanabx
approved these changes
Sep 12, 2023
akien-mga
approved these changes
Sep 16, 2023
There was a problem hiding this comment.
The changes are pretty self-contained and I trust @anvilfolk's work and @ryanabx's review, so this should be good to merge and test in dev5. If there are regressions, we'll spot them there :)
|
I'll be around to fix any more regressions! I did test this on my other cyclic dependencies tests and it worked properly. It might just be ongoing work to squash all these issues that only crop with with specific cyclic relationships and loading orders which exist in large projects. |
adamscott
approved these changes
Sep 16, 2023
|
Thanks! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 is part of ongoing work on fixing cyclic dependency errors in the GDScript compiler. This removes another instance of the compiler triggering compilation of an external class during compilation of the current class, with bad side-effects.
Since more of the GDScript codebase now more fully depends on
GDScriptCache::finish_compiling(), as it should, I also added dependencies to a couple ofGDScriptCache::get_shallow_script()calls that did not set those dependencies.Fixes #81548. Fixes #81447. Fixes #81799.