derkork / godot-statecharts

A state charts extension for Godot 4
MIT License
679 stars 33 forks source link

StateIsActiveGuard not triggering on delay complete #103

Closed mrmwiebe closed 2 months ago

mrmwiebe commented 3 months ago

It seems that I cannot get a parallel state transition to fire using a StateIsActiveGuard when the state it's tracking only uses a delay to trigger the next state.

Here I've set up a test state chart using the parallel node.

Screen Shot 2024-04-13 at 1 03 18 AM

Compound Chart A is using delays to transition back and forth between A1 and A2. Compound Chart B is using StateIsActiveGuards to follow Chart A. Example of "To B2" transition below:

Screen Shot 2024-04-13 at 1 06 11 AM

However, this does not work!

It seems to only be the delay that does not work, because if i use a script to fire an event to change A1 to A2, "To B2" will trigger the StateIsActiveGuard as expected. I would expect that "To B2" should trigger whenever A2 is active whether that is from a delay or event trigger.

Please help! I am loving this plugin and it's my favourite way to build state machines in any game engine.

derkork commented 3 months ago

A guard does not trigger transitions it is merely a check that is done when a transition would trigger. The guard can then prevent this transition from happening when it evaluates to false (hence the name "guard"). These actions will potentially trigger transitions:

Could you give me a bit more details about the use case you have? Maybe this can be modeled differently.

mrmwiebe commented 3 months ago

Ok thank you for further clarification! I guess I'm still a bit confused as to why a transition delay is not included in that list since isn't a state moving from one to the other using a delay still count as a state being entered?

This is how I had it set up to give you a bit more context for what I was expecting:

Screen Shot 2024-04-13 at 12 48 23 AM

I was hoping that the Move state would always be active when the Controllable and Stunned states were active, and the Hold state would be active when CoyoteTime was active. Basically, CoyoteTime is a grace period given to the player when a collision is detected to let the player make a split second last adjustment to avoid the collision, and was trying to use a delay transition to count down that time for me. If the player doesn't make an input, the delay ends and the Stunned state is triggered. If they do, an event fires and the player is Controllable again.

Hopefully that helps give you some extra context. I could definitely be over complicating things and have since simplified it to just one compound state and calling the movement via a function instead.

derkork commented 3 months ago

Thanks a lot for taking the time to write up a clarification!

I guess I'm still a bit confused as to why a transition delay is not included in that list since isn't a state moving from one to the other using a delay still count as a state being entered?

A delay delays the actual execution of the transition. So while the delay is running the transition is pending. But you will still be in the old state until the delay has fully elapsed. During that time other transitions will still be evaluated and could potentially supersede the pending transition. For example lets assume this setup:

image

The state chart starts by entering A which will evaluate all transitions leaving A. There is an automatic transition going to B which is delayed by 5 seconds and another one going to C when some_event is sent. This is similar to your Coyote Time setup. So the automatic transition will be triggered and is marked as pending. But we are still in A until the transition actually runs. If during the delay some_event is sent, the transition going to C will be triggered and we leave A. This will abort the pending transition to B. So during this whole operation a StateIsActive guard checking for B being active would return false because we never entered B because the transition was only pending but never executed.

I think in your particular case you could indeed remodel this and do away with the parallel state as it isn't really parallel. Parallel states are intended to model aspects that are mostly independent of each other but in your case the Move and Hold states basically just translate to "if i am in Stunned then Hold is active, otherwise Move is active". So one way to model this could be:

image

Then then you can just use the callbacks of the Moving and Holding states and you don't need any guards or custom code. This is because if a child state is active, its parent state is also active, so this setup exactly expresses "if i am in Stunned then Hold is active, otherwise Move is active". I hope this helps!

mrmwiebe commented 3 months ago

A delay delays the actual execution of the transition. So while the delay is running the transition is pending. But you will still be in the old state until the delay has fully elapsed.

Yep, this all makes sense to me, and tracks with my understanding of how the states work.

So during this whole operation a StateIsActive guard checking for B being active would return false because we never entered B because the transition was only pending but never executed.

But this still doesn't really clarify why a StateIsActive guard in a parallel state checking for B being active would continue to not return true if the delay transition to B completes, but does when B is transitioned to with an event transition. I feel like StateIsActive should not care how a parallel state was transitioned to, only if that state is active as the guard is described.

Anyway, I won't push further on this topic unless you'd like further details. I might be misunderstanding how this works or explaining it badly, but I love your suggestion on the new setup and it makes way more sense! I'll implement something like that moving forward. Thanks!

derkork commented 3 months ago

So am I getting this right, you have a situation where B is active and the StateIsActive guard for B still returns false and prevents the transition? That would indeed be a bug. Would you happen to have a small example project that reproduces this?

mrmwiebe commented 3 months ago

Sure! Yeah I just created a fresh project with the example.

statetest.zip

Hopefully that helps clear it up, sorry if i wasn't explaining it right 😅

derkork commented 2 months ago

Well I checked the example and it's pretty much what I described. State changes need to be triggered by something, a guard can only prevent a state change but not trigger it. So if you send an event by pressing 1 or 2, then is event is evaluated against all transitions and hence the B-branch transitions as there is a trigger. But if A1 auto-transitions to A2 (or vice versa) there is nothing that would trigger anything in the B-branch, so any guard in the B branch will not even be evaluated. For that to work, the state chart would need to re-evaluate all automatic transitions whenever a state change somewhere occurs. That is probably doable but I'm not quite sure if this would have unwanted side effects or prevent certain use cases.

mrmwiebe commented 2 months ago

Ah ok! Gotcha. Thank you for clarifying! Since refactoring the state machine as per your amazing suggestion, I probably won't be using it like this for a long time anyway. Thanks for your patience and support! 🙏