GDScript: Fix null deref when ensuring cached parsers

[vnen]

Fix regression reported in #94617 (comment)

Note: I don't have a way to reproduce the bug, but this should avoid crashing.

[AThousandShips]

LGTM

[vnen]

This is still raising a parser bug error message, even if not crashing.

[vnen]

@mihe @ydeltastar Changed the location of the error messages so they only are raised when relevant.

[mihe]

I still get Parse Error: Could not find script "..." (While resolving "..."). even with the latest changes from this PR.

Give me a little while and I'll have an MRP for you guys.

[ydeltastar]

This PR fixes the crash and parser errors after I deleted the .godot and reimported the project. Or so I thought but reopening the project again triggers the parser error.

PS: Deleting .godot still crashed with #94617

[mihe]

I uploaded an MRP here.

Stale

[ydeltastar]

I managed to isolate the issue in a few minimal scripts. MRP: issue-94617.zip

Open "PlayerController.gd" and it raises an error with this PR or crash when testing with #94617.
It will work if you change the variable to the base class Actor even though FPSActor is just an empty class extending Actor.

extends Control

- var _actor:FPSActor
+ var _actor:Actor

@export var disable_input = false:

[rune-scape]

please dont remove the push_error in ensure_cached_parser_for_class! it should always be able to find the parser and if it cant, the underlying issue should be solved, it will most likely just cause an error later down the line instead, when it needs a class from the result of a previous expression

that would make the error much harder to track

you also replaced the error messages with ones that have much less information ..

[akien-mga]

@rune-scape Feel free to make another PR to fix the regression. It's much harder for vnen to fix the bug than it should be for you who authored that code. There's a clear design issue in that code where a null argument is passed to a method that can't handle it.

I'm building 4.3 rc 1 today, so if we can't hotfix this now I'll have to revert #94617 for now as the regression is worse than the bug it fixed.

[akien-mga]

Superseded by #94723.

[vnen]

@rune-scape I was mostly trying to figure out where the error comes from. If something can be nullptr (as it is passed directly as a function argument) then it shouldn't just throw a "parser bug" error. Kind of a bandaid to avoid crashing but also avoid spurious messages.

This system has become way to complicated. After 4.3 we'll need to focus on rethinking it and cleaning it up.

[rune-scape]

ill get to this when i have my computer again in a few days.

i know exactly what the problem is now, dependant_parser and dependant_parser_ref should only cleared when p_from_class is null.
they should also be in the null check, that was my intention with the code, it wasnt meant to throw an error if p_from_class is null it was supposed to search on the current pasers depended parsers

This system has become way to complicated. After 4.3 we'll need to focus on rethinking it and cleaning it up.

@vnen completely agree, and understand if everything related to removing parsers from cache and refreshing on reload in editor got reverted before release

i think it maybe was you who said there should be a separate dependancy tree for every script and id definitely agree with you now, im having some ideas on how to do that