GDScript: Fix lambda hot reloading

[rune-scape]

fixes #62769

swaps out the function pointers used in LambdaCallables when a script is reloaded
if it cant find a good replacement it invalidates the callable, so no more 'Bad Address Index' and crashes, it tells you its called an invalid lambda
the logic it uses to swap lambdas is very simple right now, swapping only if the argument count vaguely matches, and no lambdas were added or removed, evaluated for each function the lambda was in
also should be safe across threads, because lambdas can be created on any thread. i took care to make sure it wouldnt slow lambda creation down

it may be a good idea to add some more logic to how old lamdas are matched to new ones, the data is there but i dont know what would be intuitive, what would break logic, im all ears for heuristics

MRP:
81628.zip

Production edit: added the MRP

[rune-scape]

umm, the failed check seems like an unrelated crash..

[ryanabx]

umm, the failed check seems like an unrelated crash..

You're absolutely right

[dalexeev]

I'm not qualified enough to review this, I just noticed a couple of minor flaws.

If hot reloading is only available in the editor, does it make sense to wrap some code in TOOLS_ENABLED or DEBUG_ENABLED?

[rune-scape]

If hot reloading is only available in the editor, does it make sense to wrap some code in TOOLS_ENABLED or DEBUG_ENABLED?

@dalexeev script.reload(true) is hot reloading, i believe. and thats available at runtime with tools disabled
placeholders, docs, and tests are whats disabled in gdscript when the editor is
except for old_static_variables and member_default_values? it seems inconsistent, but hot-reloading in concept is allowed without tools,, and this is a memory safety issue,, so i think a big chunk of it would need to stay, and it should probably be enabled wherever member function hot-reloading is enabled, which is all the time with or without tools right now

tbh i dont know why you would pass false to reload for that matter,,, it seems like that would just cause weird runtime behavior. is it even used anywhere?

[rune-scape]

should i click the resolve conversation button now that ive pushed the corrections?? i do not remember

[YuriSizov]

should i click the resolve conversation button now that ive pushed the corrections?? i do not remember

If you've addressed the comment and there are no open matters to discuss, then feel free to click resolve :)

[rune-scape]

@AThousandShips id appreciate another review on the thread safety, i changed some things in gdscript.cpp, and i remember your affinity on the subject

[AThousandShips]

Otherwise I don't see any issues with the threading, but I'm a bit rusty with the specifics so might be missing something, so someone else should take a look at that as well

[adamscott]

Thanks @AThousandShips, I integrated your suggestions into the PR as @rune-scape is unavailable for the moment.

[adamscott]

I created a MRP to make sure that it works. And it works flawlessly.
81628.zip

Just run the project, then change the inner value of the lambdas. It just works!

[adamscott]

It's fine for me. It works and the code is clean. I'm no hot-reloading-whiz, but I'm OK with merging this for 4.2.

[rune-scape]

while using it yesterday i just found a small bug related to thread safety checks. pushing that now before it gets merged

[vnen]

For a glance it looks good. It'll take some time if I dive deep on the logic, so I'll trust rune on this. It's already a big improvement over the current status even if some bug crops up.

[vnen]

Looks good. It's a huge improvement over the current status.

[akien-mga]

Thanks! Amazing work 🎉