Improve handling when StateChart methods are called before _ready

[zibetnu]

My project has nodes that call set_expression_property in their _ready methods. 0.13.0 makes set_expression_property assume that _state is initialized, which isn't the case when another node's _ready is called before the chart's _ready.

I added this handling to my project and found it to work well:

func set_expression_property(name:StringName, value) -> void:
    if not _is_ready:  # Set to true at the end of _ready.
        await ready

    if not is_instance_valid(_state):
        push_error("StateChart must have a child State")
        return

    _expression_properties[name] = value
    # run a property change event through the state chart to run automatic transitions
    _state._process_transitions(&"", true)

Here's a minimal demo of the above handling: demo.zip.

The same thing would ideally be done in send_event and step as well to keep things consistent.

I'd be happy to put together a pull request if this sounds good to you. Thanks for the great plugin!

[derkork]

Thanks a lot for taking the time to report this! The problem is actually very similar to the situation when calling send_event. It can occur in two flavours:

• someone calls send_event / set_expression_property before the state chart is ready.
• someone calls send_event / set_expression_property when the state chart is ready but nodes further up the tree are not yet ready. That is why the initial state is only activated one frame after the state chart becomes ready so nodes further up the tree have a chance of initializing themselves before receiving callbacks from the state chart.

The state chart cannot reliably detect the second situation to handle this situation automatically and defer the call one frame. I'll add a check for the state chart itself to be ready in set_expression_property but I think the solution with await ready is only going to work in the first case (which is ok for your specific instance but not the general case). So I'll also update the documentation to include that calls to set_expression_property inside of a _ready function should also be called deferred. Would this work for you?

[derkork]

Turns our this is also true for the step function, so I'm going to add handling for this there as well. Always funny how every change has subtle side effects elsewhere.

[zibetnu]

It turns out that my prior solution wasn't enough to have _state._process_transitions do anything. _state.active is still false by the time await ready is done, so all it did was prevent a crash. I also hadn't considered the case where the state chart is ready and does callbacks before other nodes in the tree are ready.

I think it's best to have the state chart handle these two problems so that no other nodes interacting with the chart need to take them into consideration.

Here's something that seems to solve both problems! The idea is like _is_ready but for when the first process frame has started.

var _first_process_frame_started := false

func _ready() -> void:
    ...  # Current _ready code.

    get_tree().process_frame.connect(
            func(): _first_process_frame_started = true,
            CONNECT_ONE_SHOT
    )

func send_event(event: StringName) -> void:
    if not await _all_ready_and_state_valid():
        return

    ...  # Current send_event code.

func set_expression_property(name: StringName, value) -> void:
    if not await _all_ready_and_state_valid():
        return

    ...  # Current set_expression_property code.

func step():
    if not await _all_ready_and_state_valid():
        return

    ...  # Current step code.

func _all_ready_and_state_valid() -> bool:
    if not _first_process_frame_started:
        await get_tree().process_frame

    if not is_instance_valid(_state):
        push_error("Child State is invalid.")
        return false

    return true

Here's another demo with print statements that make it easy to tell in what order things are happening: demo.zip.

To be clear, this isn't an attempt at embedding a pull request into a comment. I made this solution for my own project and thought I'd share it here as well. I completely understand if you don't find it appealing.

[zibetnu]

This is tangential, but I somehow hadn't heard of is_node_ready() before. That was neat to learn from your recent commit.

[derkork]

Well I'm incorporating the part I am comfortable with in 0.13.1. There are too many edge cases when trying to guess what nodes outside of the state chart will do when (e.g. someone could instantiate nodes at runtime and connect them to state chart events or even instantiate state charts at runtime), so I rather play it safe here and let the user decide if they need a delay. Thanks again for reporting this and for discussing potential fixes, it really helped a lot!

[zibetnu]

Sounds good! I agree that the simpler approach you took is the safer one, and my more complicated solution was made pointless when I found out I can defer signals connected by the editor anyway.