GDScript: Enhance handling of cyclic dependencies#93346
GDScript: Enhance handling of cyclic dependencies#93346akien-mga merged 1 commit intogodotengine:masterfrom
Conversation
5f01479 to
1849948
Compare
GDScript: Enhance handling of cyclic dependencies|
I thought that your PR could solve what I stumbled upon, that I hoped to solve with my previous PR. Could you try to check if it could solve it? I got this crash when I tried to load this peculiar MRP (cyclic-scenes.zip): Crash logIn that MRP, I have |
1849948 to
2281f39
Compare
|
@adamscott, is that something we even want to support? I guess we can modify the loading machinery so that's possible, but, then, the two How was that scene setup made in the first place? Can one create that with normal usage of the editor? We're getting maybe a bit off-topic, though... |
|
Tested on a few projects (GDQuest TPS demo, W4 Planet Crashers), I confirm that it fixes the linked issues, and I didn't spot any obvious regression. That confirms #90362 fixed. I also confirm that the MRP in #92780 seems fixed by this PR. For #70985, I tested the MRP and it seems to solve the main issue (couldn't load the script at all), but the first load of the project still produces these errors: This sounds related to #92303 so I tested with this PR on top of #93346, but that doesn't solve it either. For #90954, with a dev build I'm actually triggering the |
2281f39 to
58b5443
Compare
|
Pushed a fix for #90954. There's still an issue there, but I think unrelated to the scope of this PR: in my testing, the global classname cache isn't updated at once; I have first to remove the use of some classes from a script that refers the other. Once the editor has been able to parse the scripts thanks to that manual aid, the project loads and works. Thoughts? |
58b5443 to
c4f3e12
Compare
This sounds like what #92303 aims at solving. This morning I've actually been testing both PRs together for best results (but also separately to make sure I'm not attributing a problem to a PR that's caused by the other). |
c4f3e12 to
c139148
Compare
|
Thanks! Awesome work to everyone involved here and on #92326 🎉 |
Since I wasn't very successful in getting my ideas across in #92326, due to the complex nature of the matter, I decided to get my own hands dirty with this to illustrate what I meant. It turned out that the resulting code is much more complicated than what I was trying to convey in my comments. Therefore, I'm submitting my complete idea in a new PR.
Fixes #70985.
Fixes #90362.
Fixes #90954.
Fixes #92780 (according to #92780 (comment)).
Disclaimers:
For reviewers, disable whitespace diffing because the changes are not a big as they seem.