Closed RafaelVidaurre closed 1 year ago
Hey, thanks a lot for checking out the addon and for taking the time to report this! If it isn't too much to ask, could you maybe give a bit more details about your setup and how exactly the switch on next frame gets in the way? I would like to understand the problem a bit better before shooting a quick solution from the hip which may not be sufficient or even break existing setups.
which
Hey, thanks for the quick reply @derkork!
Absolutely, here's some more context:
We have a bunch of states in our weapon system which are part of a weapon's action's lifecycle. In our game, weapons define your moveset, and you can get upgrades that make even further modifications. For this reason, we need a lot of places we can hook onto and replace/extend logic (hence why we have all those Before/After).
By default, most of this states just jump to the next one. Meaning that for an action that has no overrides, you might get ~6 state jumps in a row. This would mean a delay of 6 frames, which in real milliseconds would depend on framerate.
Besides that, if we skip a frame when jumping states, we lose determinism in terms of when a given state would be resolved. This creates a couple of issues too that come to mind:
state_entered
signal. Or, programming gods forbid, set an arbitrary timeout and hope for the best.I think an ideal change would be to guarantee state resolution in the same frame, though if backwards compatibility is a concern, I understand this might not be attractive. Since the addon is pretty early still, it might be reasonable to take the hit early? I guess you could have a global project setting that people could turn on in order for the old behavior to kick in by default.
Implementation-wise, I'm not too familiar with the inner workings of the addon, so I might be saying something dumb:
Don't rely on process
for managing state transitions, instead, recursively evaluate if any transitions trigger when a state is entered until the state you are iterating doesn't trigger a transition.
Behavior is probably the trickiest: I think state_entered
state_exited
should still emit, but not the process functions? I guess you'd need to make sure that every time we enter a state, we don't consider it final until all connections to its relevant signals have ran. Userland coroutines should await
after any send_event
calls if they expect transitions to happen in the same frame of course.
Hopefully that makes sense or gives some ideas?
Loving what you have built so far by the way. I think the addon is pretty well written in general, and I like the way you are thinking about state charts compared to other solutions I've seen so far.
Cheers
I had a look at the implementation again. In general it should not be too hard to implement this. The tricky part is ensuring that all state changes have been fully processed before working on another state chart event, otherwise the state chart will end up in some inconsistent state. This means that when e.g. a state_entered
handler does another send_event
, this will not be processed until the state_entered
handler returns and the rest of the pending state transition has happened. Only doing state transitions in _process
sidestepped this issue but I can see now how this is really only a stopgap measure. I think with some extra code in the root state chart class both issues can be addressed. It may not be 100% the same behavior as before but it will be more correct.
I am currently on a trip and have no access to a computer so I will only be able to look at this in some more depth next week.
Understood, thanks for the help!
I have opened a branch for this: https://github.com/derkork/godot-statecharts/tree/feature/predictable-state-changes . The changes are indeed rather minor. Could you give this a spin and see if it works for you?
There is another edge case which is if you enter a state and have a transition there which should immediately switch to another state. Handling this will be a bit more tricky but I'll think of something.
I have added some handling for this now as well. Transitions now run sequentially in the order in which they are triggered. So when a state change triggers another transition immediately, then the current state change is first completed and then the other transition will run and so on. Neither of these changes seem to break the demos but since you have quite an elaborate setup it would be nice if you could give the branch a spin and report any issues you are having before I make a release, @RafaelVidaurre . Thanks a lot!
I have also added some improvements to the state chart debugger so you can now see in which frame stuff happens:
You can see that 0 delay transitions now happen in the same frame.
Even though I'm loving this addon so far, I recently came to the sad realization that 0-second transitions do not happen on the same frame.
This is a big problem depending on how your state machine is structured. Immediate transitions now take a variable amount of frames depending on the amount of state jumps that happen, while also making it hard to perform logic after all transitions are finished.
Is there a workaround for this or is this in your plans?