Changing scene after three yields closes the game.

[EdwardAngeles]

Operating system or device - Godot version:
Ubuntu Linux / 2.1 alpha official

Issue description:
Having three or more yield within a function (it also happens if the yields are in different scripts) and reloading or changing the scene after those yields are already over closes the game. It only happens with three yields or more.

Steps to reproduce:
Set three yields in "_ready": yield(get_tree(), "fixed_frame") and then reload the current scene: get_tree().reload_current_scene() when pressing a key.

Link to minimal example project:
YieldDemo.zip

[kubecz3k]

It seems to be very similar to #4658, if any of those will be fixed we need to check other one.

[vnen]

Using yield inside _ready() does not seems like a good idea to me. Nevertheless, it shouldn't crash.

[vnen]

This is pretty much the same as #4658. Checking the test project there results in the same exception in the same spot.

I'm not sure how to debug it though, it seems there's some corruption in the heap (if I understand it right). So I guess it's needed to evaluate with Valgrind or something.

[Warlaan]

I ran some tests and it turns out that it doesn't matter whether the yield command is used directly in the _ready-method or in a method of its own. I changed the sample code above by moving everything after line 3 into a new method so that it looks like the yield()-sample-code from the official documentation (http://docs.godotengine.org/en/latest/reference/gdscript.html?highlight=yield#coroutines).

What does matter though is whether reload_current_scene is called from within _fixed_process() / _process() or not. If I just move it to the end of the new function it works:

extends Sprite

func _ready():
    set_process(true)

    print("ready")
    test()

func test():
    print("test 1")
    yield(get_tree(), "fixed_frame")
    print("test 2")
    yield(get_tree(), "fixed_frame")
    print("test 3")
    yield(get_tree(), "fixed_frame")
    print("test 4")
    get_tree().reload_current_scene()

Another thing that also works (and would be a workaround for the case above) is using another yield-call to delay the reload_current_scene()-call like this:

extends Sprite

var keysignal

func _ready():
    set_process(true)

    print("ready")
    test()

func test():
    print("test 1")
    yield(get_tree(), "fixed_frame")
    print("test 2")
    yield(get_tree(), "fixed_frame")
    print("test 3")
    yield(get_tree(), "fixed_frame")
    print("test 4")

    keysignal = KeySignal.new()
    add_child(keysignal)
    yield(keysignal, "keypress")
    get_tree().reload_current_scene()

class KeySignal:
    extends Node
    signal keypress

    func _ready():
        set_process(true)

    func _process(delta):
        if (Input.is_key_pressed(KEY_SPACE)):
            emit_signal("keypress")

Maybe this helps eliminate some theories about where to find the bug.

[Warlaan]

Ok, I had another idea and I think I found something important. This code works:

extends Sprite

func _ready():

    print("ready")
    test()

func test():
    print("test 1")
    yield(get_tree(), "fixed_frame")
    print("test 2")
    yield(get_tree(), "fixed_frame")
    print("test 3")
    yield(get_tree(), "fixed_frame")
    print("test 4")
    set_process(true)

func _process(delta):
    if (Input.is_key_pressed(KEY_SPACE)):
        get_tree().reload_current_scene()

I think that the actual problem is a race condition. By reloading the current scene the node is placed in the scene tree again and the _ready()-method is called again. But since the _process()-method was reactivated immediately and the space bar was still pressed reload_current_scene() was run again. This isn't a problem if loading the scene was finished, but if there are too many yield()-calls the engine tries to continue a yielded method in an object that was already destroyed.

Putting the "set_process(true)"-line behind print("test 2") also crashes but putting it behind print("test 3") works, so to me it looks like a race condition.

So to fix it yields would have to check whether the object they are referring to is still valid. And imho it shows that Godot is in dire need of better error messages. If there had been anything about an invalid reference this would have been a lot easier to debug (assuming that my theory is correct).

[vnen]

Godot is in dire need of better error messages

While I can't deny that some error messages are cryptic, this issue is about a crash, and since it's crashing there's no time to send any message, it just fails.

Thanks for looking into this. The problem is likely what you said: when it reloads the scene it loses the reference but the yield is still waiting to be called and when it does there's an invalid reference, so it crashes.

[Warlaan]

I am not talking about a popup or something, just an assert() resp. an ERR_FAIL_COND() so the debugger is stopped with a message (which imho should have a second parameter so you can specify a message that rephrases the condition in human readable form, but that's a discussion of its own and doesn't belong in this thread).