Review signal connections in the StateMachine

[NathanLovato]

We have some signals connected to e..g the Move state, that will trigger even if the Move state isn't active. For instance, if the player is staggered and tries to hook onto something, they'll instantly transition to Hook, even if this transition shouldn't exist.

In general, with an FSM, only the active state (and its parents in the HFSM) should have code running. The way we connect signals now breaks that. Even if we can work around individual signals, this is a general issue for the course, to me, as we wouldn't be teaching the pattern right.

A possible solution is to funnel signals through the state machine: connect everything to the StateMachine node, and delegate the callback to the current state, if it has a corresponding callback method. Another option, suggested by @henriiquecampos, is to connect and disconnect signals in each state on enter/exit. This could also work.

@razcore-art could you share your thoughts on this?

[razcore-rad]

I'll review the signals code, but as it stands the emit_signal methods should only trigger during call of the state function that's directly connected to State override-able methods: unhandled_input & physics_process, they should never be emitted anywhere else so I'm kind of surprised we got here.

edit: I just realized that I might have misunderstood the problem, in any case I'll check out the code

[razcore-rad]

Yeah, I think all of the above suggestions are good, about connecting to the state machine node for example or to connect/disconnect signals on enter/exit and I'll add another one: to check inside the callbacks if the current state is the active state. Eg. if _state_machine.state == self: return.

From all of these I prefer what @henriiquecampos suggested: connect/disconnect signals on enter/exit since it has multiple benefits. It means each state is responsible for itself (decentralization/decoupling) & we don't have unnecessary signal methods called by Godot on each frame just for a check.

This approach brings another benefit. As it stands we created a _setup virtual-like function in the base State.gd script which we only use for setting up signal connections. With @henriiquecampos' approach we could eliminate _setup altogether and just have the states manage their own signal connections as needed in enter/exit functions.

[NathanLovato]

to check inside the callbacks if the current state is the active state. Eg. if _state_machine.state == self: return.

@henriiquecampos also suggested it, I forgot to mention it.

I just tested the code with the two other approaches and I was going to say the same, connect/disconnect in each state. To funnel signals from the state machine with a single function elegantly, you'd need something like Python's **kwargs, which gdscript doesn't have.

Agreed, we should use Henrique's solution here.

[oganm]

Was wondering if you would consider adding this bit to the state machine tutorial. Even the raw discussion here is helpful as is