Stop caching packed scenes in GDScript cache (on preload)#85501
Merged
akien-mga merged 1 commit intogodotengine:masterfrom Feb 25, 2024
Jordyfel:remove-packed-scene-cache
Merged
Stop caching packed scenes in GDScript cache (on preload)#85501akien-mga merged 1 commit intogodotengine:masterfrom Jordyfel:remove-packed-scene-cache
akien-mga merged 1 commit intogodotengine:masterfrom
Jordyfel:remove-packed-scene-cache
Conversation
Member
|
Oh, sorry, I did not see your ping! I'll take the time to investigate. |
Member
|
I just pinged you 3 min ago so you're not too late :P |
adamscott
approved these changes
Feb 24, 2024
Member
There was a problem hiding this comment.
To be honest, if I remember correctly, this was legacy code from when I tried to enable PackedScenes cyclic dependencies. Unfortunately, this idea was shut down, but the code stayed.
The all the tests pass and I even tested a small project. Everything seems to still work.
Great job! Thanks!
Member
|
Thanks! |
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
3 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.
Adresses #85081
This code was added in #67714, with the goal of resolving problems with cyclic references is
preload().However, cyclic scene references still don't work, as illustrated by #70985 and other preload related issues.
Being resources, scenes don't support cyclic references, and caching seems to just make the problem appear a little bit later, and make it harder to debug (unlike with scripts, where this should be fine, because
get_shallow_script()is designed to deal with the cyclic reference problem).IMO this problem cannot be solved from outside core resource functionality and the GDScript module should not try. Pinging @adamscott as the original implementer, please do correct me if I have misunderstood the intent of this code.
Most posted issues regarding preload have no reproduction steps and just include already corrupted scenes, which makes it very hard to tell which this change fixes, however it should improve recovery and consistency of behavior as scenes will for sure be reloaded when the script is recompiled.
Bugsquad edit: Fixes #79545 Fixes #84981 Fixes #79840