Keep GDScript functions in stack while yielding #28884
Conversation
This prevents GDScript functions from leaving the stack too soon when they are resuming from yield, allowing the ones expecting it to finish to know the caller. Helps debugging cases when you use: `yield(function_which_yields(), "completed")` since now it shows the call that resumed that function.
Also show script and line when the instance is gone when resuming from yield.
|
This is a remake of #28815 (I thought I could reopen but it's not possible after force-pushing). |
|
This will be great, these types of errors have been so difficult to track down. From time to time I have desired a graceful way to check / ignore them, in situations where things are shutting down or the game is closing and yields were out and resuming as resources are vanishing. |
|
Related to #24199. |
| @@ -1583,15 +1583,26 @@ Variant GDScriptFunction::call(GDScriptInstance *p_instance, const Variant **p_a | |||
| GDScriptLanguage::get_singleton()->script_frame_time += time_taken - function_call_time; | |||
| } | |||
|
|
|||
| if (ScriptDebugger::get_singleton()) | |||
| GDScriptLanguage::get_singleton()->exit_function(); | |||
| bool yielded = retvalue.is_ref() && Object::cast_to<GDScriptFunctionState>(retvalue); | |||
There was a problem hiding this comment.
I'm not sure that's the best way to check if the function yielded. This might break in cases such as:
func a():
yield()
func b():
var x = a()
# x.resume()
return x # Not yielding, yet returning a GDScriptFunctionStateThere was a problem hiding this comment.
I see, but then the completed signal from function also relies on this, so we may need to check it.
I guess we can find a way to check if the state correspond to the same function. Not sure how to do this.
There was a problem hiding this comment.
I guess we can find a way to check if the state correspond to the same function. Not sure how to do this.
Still gameable:
func a():
var x = b()
return b(x)
func b(z=null):
if z != null:
return z
else:
yield()
There was a problem hiding this comment.
So is any further change needed?
There was a problem hiding this comment.
Guess it could be fine as-is.
ghost
mentioned this pull request
|
Thanks! |
|
Now that Godot 3.1 has been released, we don't plan to cherry-pick new features and enhancements to the 3.0 branch, unless there is very strong support in favor of it. Removing cherrypick label for 3.0. |
This prevents GDScript functions from leaving the stack too soon when they are resuming from yield, allowing the ones expecting it to finish to know the caller.
Helps debugging cases when you use:
yield(function_which_yields(), "completed")since now it shows the call that resumed that function.Also show the function names in the debugger stack, which helps identify the stack (not sure why that was commented out, it's been like this since the first commit). And show function/script/line when resuming from yield after instance is gone, to help identify the issues.