Idea for a potential change to the FSM system

[KybernetikGames]

Use Case

@trueh is using the Animancer.FSM and wants to choose between multiple transitions when entering a state depending on the previous state it was in.

My current solution would be to use the CanEnterState method to implement something like the approach used in the Interrupt Management example. But that isn't really ideal though since you should really be playing animations and doing your state entry stuff in OnEnterState (or OnEnable if you're using a StateBehaviour).

So I had an idea for a different way the FSM system could work and I want to see what people think about it before deciding whether or not to implement it in the next version. There's a download link at the bottom of this post.

Solution

• Currently (in Animancer v5.2) the IState methods CanEnterState and CanExitState methods have a parameter that specifies what state it is trying to enter/exit respectively.
• My idea is to remove those parameters and add a StateChange script which gives static access to both those states and the StateMachine itself during a state change.

Example

The CreatureState class in the Interrupt Management example would have its CanExitState method would change from this:

public override bool CanExitState(CreatureState nextState)
{
    return nextState._Priority >= _Priority;
}

To any of the following options (they could all be available for you to choose from):

public override bool CanExitState()
{
    // Access the StateChange directly using its static property.
    return StateChange<CreatureState>.NextState._Priority >= _Priority;

    // Avoid needing to specify the <CreatureState> parameter by accessing it via the StateMachine.
    return _Creature.StateMachine.NextState._Priority >= _Priority;

    // Or even with an IState extension method.
    return this.GetNextState()._Priority >= _Priority;
}

Advantages

• The StateMachine, PreviousState, and NextState can all be accessed from anywhere (such as in an IState.OnEnterState or StateBehaviour.OnEnable method) without needing to manually pass the values around from CanStateEnter and CanStateExit.
• Cleaner implementation of the IState interface because it isn't generic any more so you no longer need a circular declaration (class MyState : IState<MyState> is now just class MyState : IState).
  • Same for StateBehaviour and DelegateState.

Disadvantages

• Accessing the previous/next state is a bit less straightforward than directly receiving them as parameters.
  • That means it would be a bit harder to learn to use the system, which would be somewhat mitigated by putting code examples in the IState documentation comments.
• The StateMachine, PreviousState, and NextState properties are visible at all times rather than only while a state change is occurring. They would be null at other times, but that would only lead to runtime exceptions rather than giving compile-time safety.

Try It

1. Download Animancer.FSM Experimental StateChange v3.zip.
2. Import it over the top of Animancer v5.2 (the current version, both Lite and Pro will work).
3. Let me know what you think.

[KybernetikGames]

Added a few more improvements. StateBehaviour and DelegateState no longer require generic arguments either.

New Package: Animancer.FSM Experimental StateChange v2.zip replaced by v3 for Animancer v5.2.

[dastillero]

Hi. I am @trueh.

I am trying the second version and it is working for me. As you can imagine, I agree with your idea as it is opening the possibility of choosing transitions based on the previous state in OnEnterState() or OnEnable(). Anyway, I would not want to break other user's code, so I thing that it would better to get feedback from other people.

The only thing that is not working in my tests is the access to the previous state by means of the extension methods. GetPreviousState() is always returning null for me. I am using StateBehaviours implementing IOwnedState. I did not try with regular States.

The solution I was using before these modifications was capturing the previous state in CanEnterState(), then using it in OnEnable() to activate the correct transition. It is not that bad but your solution would be better from a semantics point of view. I do not feel that I should be using CanEnterState() to perform that kind of logic. It is counterintuitive and that is the reason I was asking in the forums.

Thank you.

[KybernetikGames]

I'll keep this as a separate package until the next major version update to avoid breaking people's code in a minor update, but I'd much rather break some code than be stuck with an old system once I've found a better way.

I'm not seeing any issues with accessing the previous state using any of the approaches and IOwnedState shouldn't matter. What do your tests look like?

[dastillero]

I am using this as a base class for all states:

    public abstract class CharacterState : StateBehaviour, IOwnedState<CharacterState>
    {
        // State machine
        private StateMachine<CharacterState> _stateMachine;
        public StateMachine<CharacterState> OwnerStateMachine { get => _stateMachine; }

        // Character controller
        protected TpfCharacterController CharacterController { get; private set; }

        // Animancer
        protected AnimancerComponent Animancer { get; private set; }

        private void Awake()
        {
            // Find the character controller
            CharacterController = GetComponent<TpfCharacterController>();
            if (CharacterController == null)
            {
                Debug.LogError("No character controller was found");
                return;
            }

            // Get the state machine
            _stateMachine = CharacterController.StateMachine;

            // Get the Animancer component
            Animancer = GetComponent<AnimancerComponent>();
        }
    }

A final state looks like this (this is the state with several transitions which I am testing):

    public sealed class IdleCharacterState : CharacterState
    {
        [SerializeField]
        private ClipState.Transition _defaultTransition;

        [SerializeField]
        private ClipState.Transition _fromWalkTransition;

        private void OnEnable()
        {
            var previousState = this.GetPreviousState();  // This one returns null
            // var previousState = CharacterController.StateMachine.PreviousState;  // This one works

            if (previousState == CharacterController.WalkState)
            {
                Debug.Log("Using fromWalkTransition");
                Animancer.Play(_fromWalkTransition);
            }
            else
            {
                Debug.Log("Using defaultTransition");
                Animancer.Play(_defaultTransition);
            }
        }
    }

WalkState:

    public sealed class WalkCharacterState : CharacterState
    {
        // Clip de animación para el estado
        [SerializeField]
        private ClipState.Transition _defaultTransition;

        private void OnEnable()
        {
            Animancer.Play(_defaultTransition);
        }
    }

TpfCharacterController is similar to this (I removed some code for Kinematic Character Controller):

    public sealed class TpfCharacterController : MonoBehaviour
    {

        // Input values updated each frame by TpfInputManager
        public TpfPlayerInput PlayerInput { get; } = new TpfPlayerInput();

        // State machine
        public StateMachine<CharacterState> StateMachine { get; private set; } = new StateMachine<CharacterState>();

        // States
        public CharacterState IdleState { get; private set; }
        public CharacterState WalkState { get; private set; }

        private void Start()
        {
            // Get states and set the initial state
            IdleState = gameObject.GetOrAddComponent<IdleCharacterState>();
            WalkState = gameObject.GetOrAddComponent<WalkCharacterState>();
            IdleState.TryEnterState();
        }

        private void Update()
        {
            if (PlayerInput.Horizontal > 0)
            {
                // Right
                // currentRotation = Quaternion.Euler(0f, Directions.Right, 0);
                WalkState.TryEnterState();
            } else if (PlayerInput.Horizontal < 0)
            {
                // Left
                // currentRotation = Quaternion.Euler(0f, Directions.Left, 0);
                WalkState.TryEnterState();
            }
            else
            {
                // Stop
                //currentRotation = Quaternion.Euler(0f, Directions.Front, 0);
                IdleState.TryEnterState();
            }
        }
    }

[KybernetikGames]

The StateMachine.CurrentState starts as null, so in Start when you call IdleState.TryEnterState(); there is no previous state. The old system would have done exactly the same thing.

Are you sure that CharacterController.StateMachine.PreviousState works? Because they both wrap the same property so it should do exactly the same thing.

[dastillero]

The StateMachine.CurrentState starts as null, so in Start when you call IdleState.TryEnterState(); there is no previous state. The old system would have done exactly the same thing.

Are you sure that CharacterController.StateMachine.PreviousState works? Because they both wrap the same property so it should do exactly the same thing.

Yes. When the scene starts IdleState.PreviousState is null (using both, the extension method and by means of the StateMachine in TpfCharacterController) which is correct. In this case, IdleState.OnEnable activates the default transition (which is also correct in my use case).

Then I press Left or Right key on my Keyboard and WalkCharacterState gets activated which is also the expected behavior.

The problem arises when I release the key. At that moment IdleCharacterState becomes the active state and its OnEnable method is called. It is at that point when trying to get the previous state using the extension method is null when it should be WalkCharacterState. However CharacterController.StateMachine.PreviousState is returning WalkCharacterState as expected.

Anyway, I will try to debug it this evening. It will probably be a problem with my own code as using an extension method should not be different from calling StateMachine.PreviousState.

[dastillero]

To be honest, I have been trying to debug the problem but I have not been able to detect it. I have to be running into some kind of race condition. I will try to reproduce it tomorrow with a simpler project or unit test.

[KybernetikGames]

Animancer v5.2 is now available on itch.io and the Asset Store so here's an updated package for it:

Animancer.FSM Experimental StateChange v3.zip

[dastillero]

Updated to 5.3. Thanks!

[KybernetikGames]

Animancer v6.0 is now available for testing and has this new system integrated into it with a few more minor improvements, including some debug assertions to make sure you are actually checking the correct state change type which I believe is what caused the issue @dastillero was having where one method was always giving null.

For example, if you have a StateMachine<CreatureState> then you obviously need to check StateChange<CreatureState>, but if you're in an IdleState and you use this.GetPreviousState() the compiler will infer the generic type as IdleState and try to access StateChange<IdleState> which will be empty. So the assertion will just see that StateChange<IdleState> is not currently active and throw an exception listing all the changes that actually are active.

Unfortunately you still need to fix it manually to target the correct type, which can make the code a bit uglier (this.GetPreviousState() would instead need to be this.GetPreviousState<CreatureState>()).

[dastillero]

Yes, that was the problem. It works now 👍 Thank you.

I have another feature request for the FSM which may be interesting. I will try to send it tomorrow.

[KybernetikGames]

Another question to consider:

How should null states be handled?

Currently, they are fully allowed and will give no warnings or anything, but I figure that trying to set the state to null will usually be a bug. I can think of a few possibilities:

1. Don't allow them at all. Remove the parameterless constructor so you always need to specify a starting state. This improves performance a tiny bit (extremely tiny) because it doesn't have to null check the states during changes. If you want a state that does nothing, you have to implement one (or use your base state type if it's not abstract).
2. Add a new Optional Warning for when a state is set to null so it can be globally disabled if you want to allow them.
3. Add a bool to all state machines in the Unity Editor and Development Builds only which indicates whether that machine should allow null states. True or false by default?

[dastillero]

Hi:

I have been reviewing my code and, in my specific case, I am using the void constructor. I have a controller class which is creating the state machine and loading the states in Awake(). Those states get a reference to the State Machine in their own Awake() method, so the state machine has to be instantiated previously. Once everything is loaded, the controller class sets the initial state in the Start() method. Once I have set the initial state, it never becomes null again.

I can probably do the same in another way, but that specific part of my controller was tricky to fine tune.

In my opinion, the second option is the best of the three as it is coherent with other kind of warning that Animancer is raising.

Regards.

El mar., 17 nov. 2020 a las 1:23, Kailas Dierk (notifications@github.com) escribió:

Another question to consider: How should null states be handled?

Currently, they are fully allowed and will give no warnings or anything, but I figure that trying to set the state to null will usually be a bug. I can think of a few possibilities:

1. Don't allow them at all. Remove the parameterless constructor so you always need to specify a starting state. This improves performance a tiny bit (extremely tiny) because it doesn't have to null check the states during changes. If you want a state that does nothing, you have to implement one (or use your base state type if it's not abstract).
2. Add a new Optional Warning https://kybernetik.com.au/animancer/api/Animancer/OptionalWarning/ for when a state is set to null so it can be globally disabled if you want to allow them.
3. Add a bool to all state machines in the Unity Editor and Development Builds only which indicates whether that machine should allow null states. True or false by default?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/KybernetikGames/animancer/issues/73#issuecomment-728614673, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGA74FHWDZCZGJDCCEBYRNTSQHGADANCNFSM4Q6BL55A .

[KybernetikGames]

Lucky I asked then, since I was leaning towards number 1. In that case yeah, number 2 seems best.

[KybernetikGames]

Actually, number 2 won't be possible because the FSM assembly doesn't reference the Animancer assembly to ensure that both systems are completely separate.

So it might have to be number 3:

• Should it be a static flag or one per state machine?
• Should it allow nulls by default or warn by default? I'm thinking it should warn by default, but not in the constructor, only when you actually try to set a state.

[dastillero]

I think that the most flexible solution is to have a flag per state machine. I find your FSM so useful that I am using it for other tasks apart from controlling character animations. I do not have the case of having null states at this moment but it may be useful in some specific cases.

I agree with you. It should warn when setting states, but not in the constructor.

Regards.

[KybernetikGames]

I've added an AllowNullStates property and SetAllowNullStates method (so it can use the Conditional attribute) at the bottom of the file:

``` // Animancer // https://kybernetik.com.au/animancer // Copyright 2020 Kybernetik // using System; using UnityEngine; namespace Animancer.FSM { ///

A simple keyless Finite State Machine system.

/// /// This class doesn't keep track of any states other than the currently active one. /// See for a system that allows states to be pre-registered and accessed /// using a separate key. /// /// Documentation: Finite State Machines /// /// https://kybernetik.com.au/animancer/api/Animancer.FSM/StateMachine_1 /// [HelpURL(StateExtensions.APIDocumentationURL + nameof(StateMachine) + "_1")] public partial class StateMachine where TState : class, IState { /************************************************************************************************************************/ ///

The currently active state.

public TState CurrentState { get; private set; } /************************************************************************************************************************/ ///

Returns the .

public TState PreviousState => StateChange.PreviousState; ///

Returns the .

public TState NextState => StateChange.NextState; /************************************************************************************************************************/ ///

Creates a new , leaving the null.

public StateMachine() { } ///

Creates a new and immediately enters the `defaultState`.

/// This calls but not . public StateMachine(TState defaultState) { StateChange.Begin(null, defaultState, out var previouslyActiveChange); try { CurrentState = defaultState; defaultState.OnEnterState(); } finally { StateChange.End(previouslyActiveChange); } } /************************************************************************************************************************/ ///

/// Checks if it is currently possible to enter the specified `state`. This requires /// on the and /// on the specified `state` to both return true. ///

public bool CanSetState(TState state) { #if UNITY_ASSERTIONS if (state == null) throw new ArgumentNullException(nameof(state), NullNotAllowed); #endif StateChange.Begin(CurrentState, state, out var previouslyActiveChange); try { if (CurrentState != null && !CurrentState.CanExitState) return false; if (state != null && !state.CanEnterState) return false; return true; } finally { StateChange.End(previouslyActiveChange); } } /************************************************************************************************************************/ ///

/// Attempts to enter the specified `state` and returns true if successful. /// /// This method returns true immediately if the specified `state` is already the . /// To allow directly re-entering the same state, use instead. ///

public bool TrySetState(TState state) { if (CurrentState == state) { #if UNITY_ASSERTIONS if (state == null) throw new ArgumentNullException(nameof(state), NullNotAllowed); #endif return true; } return TryResetState(state); } /************************************************************************************************************************/ ///

/// Attempts to enter the specified `state` and returns true if successful. /// /// This method does not check if the `state` is already the . To do so, use /// instead. ///

public bool TryResetState(TState state) { if (!CanSetState(state)) return false; StateChange.Begin(CurrentState, state, out var previouslyActiveChange); try { ForceSetState(state); return true; } finally { StateChange.End(previouslyActiveChange); } } /************************************************************************************************************************/ ///

/// Calls on the then changes to the /// specified `state` and calls on it. /// /// This method does not check or /// . To do that, you should use instead. ///

public void ForceSetState(TState state) { #if UNITY_ASSERTIONS if (state == null) throw new ArgumentNullException(nameof(state), NullNotAllowed); #endif StateChange.Begin(CurrentState, state, out var previouslyActiveChange); try { #if UNITY_EDITOR if (state is IOwnedState owned && owned.OwnerStateMachine != this) { throw new InvalidOperationException( $"Attempted to use a state in a machine that is not its owner." + $"\n State: {state}" + $"\n Machine: {this}"); } #endif CurrentState?.OnExitState(); CurrentState = state; state?.OnEnterState(); } finally { StateChange.End(previouslyActiveChange); } } /************************************************************************************************************************/ ///

Returns a string describing the type of this state machine and its .

public override string ToString() => $"{GetType().Name} -> {CurrentState}"; /************************************************************************************************************************/ #if UNITY_ASSERTIONS ///

[Assert-Only] Should the be allowed to be set to null? Default is false.

public bool AllowNullStates { get; private set; } ///

[Assert-Only] The error given when attempting to set the to null.

private const string NullNotAllowed = "This " + nameof(StateMachine) + " does not allow its state to be set to null." + " Use " + nameof(SetAllowNullStates) + " to allow it if this is intentional."; #endif ///

[Assert-Conditional] Sets .

[System.Diagnostics.Conditional("UNITY_ASSERTIONS")] public void SetAllowNullStates(bool allow = true) { #if UNITY_ASSERTIONS AllowNullStates = allow; #endif } /************************************************************************************************************************/ } } ```

[KybernetikGames]

Animancer v6.0 Beta 1 is now available with the new null checks in the FSM system.

[KybernetikGames]

Animancer v6.0 is now up on itch.io (preferred) and the Unity Asset Store.