Better and more concise code

Hi guys,

I am trying to get this finite state machine toghether for a game I am currently working on. For some reason I find myself repeating a lot of code when creating the different states. Does anyone have any tips or advice on what I could do to avoid the code repetition?

Here’s the interface being implemented by the different states:

using UnityEngine;
using System.Collections;

public interface IRunnerState
{
    //Monobehaviour Callbacks
    void UpdateState();
    void OnTriggerEnter(Collider other);

    //State Change Functions
    void ToDodgingState();
    void ToRunningState();
    void ToJumpingState();

}

Here’s a couple of states to illustrate the repeated code:

Idle State:

using UnityEngine;
using System.Collections;

public class IdleState : IRunnerState
{

    private readonly StatePatternRunner runner;

    public IdleState(StatePatternRunner statePatternRunner)
    {
        runner = statePatternRunner;
    }

    public void UpdateState()
    {
    }

    public void OnTriggerEnter(Collider other)
    {
    }

    public void ToDodgingState()
    {
    }

    public void ToRunningState()
    {
    }

    public void ToJumpingState()
    {
    }
}

Running State:

public class RunningState : IRunnerState {

    private readonly StatePatternRunner runner;

    public RunningState(StatePatternRunner statePatternRunner)
    {
        runner = statePatternRunner;
    }

    public void UpdateState()
    {
    }

    public void OnTriggerEnter(Collider other)
    {
    }

    public void ToDodgingState()
    {
    }

    public void ToRunningState()
    {
    }

    public void ToJumpingState()
    {
    }
}

This is how the states are currently being implemented:

using UnityEngine;
using System.Collections;

public class StatePatternRunner : MonoBehaviour
{
    public float walkingSpeed = 2;
    public float runningSpeed = 4;
    public float dashSpeed = 8;

    [HideInInspector] public Transform grabTarget;

    [HideInInspector] public IRunnerState currentState;

    [HideInInspector] public IdleState idleState;
    [HideInInspector] public WalkingState walkingState;
    [HideInInspector] public DashingState dashingState;
    [HideInInspector] public RunningState runningState;
    [HideInInspector] public JumpingState jumpingState;
    [HideInInspector] public DodgingState dodgingState;
    [HideInInspector] public ClimbingState climbingState;
    [HideInInspector] public FallingState fallingState;

    void Awake()
    {
        idleState = new IdleState(this);
        walkingState = new WalkingState(this);
        runningState = new RunningState(this);
        dashingState = new DashingState(this);
        jumpingState = new JumpingState(this);
        dodgingState = new DodgingState(this);
        climbingState = new ClimbingState(this);
        fallingState = new FallingState(this);

    }
    void Start ()
    {
        currentState = idleState;
    }
    void Update ()
    {
        currentState.UpdateState();
    }
    private void OnTriggerEnter(Collider other)
    {
        currentState.OnTriggerEnter(other);
    }

}

Thank you!

R.

I would create a base class for the states and have every specific state inherit from the base class, that way you can place all the code that’s repeated in the base class.

So the structure would look something more like:
public class BaseState {}
public class WalkingState : BaseState, IRunnerState{} (Or without the interface if it is no longer necessary. )

Then instead of assigning currentState as an IRunnerState, assign a BaseState :slight_smile:

I use a state machine that uses Reflection to dynamically retrieve the various state transition methods. This allows me to omit certain methods (like Enter or Exit if they’re not relevant, or event handlers like OnTriggerEnter). The state machine is here, and here is an example of it’s use.

What are ‘ToRunningState’, ‘ToDodgingState’ and what not used for? Why does each state have them? Are they just events that each state can listen for? How about just have a ‘OnStateChanged’ method and pass in a handle to the state that is being transitioned to?

Or are they actions so you can change the state? In that case, why aren’t that on the state machine rather than on the states?

Also, why have the stateMachine control the events? Why not let the states do that? You could do that by having them also be MonoBehaviours. Furthermore, you then get to control what states some StateMachine has by adding or removing them as components on the gameobject.

Something like this (very basic code written in notepad real fast… not tested specifically)

public interface IState
{

    void OnEnterState(IState previousState);
    void OnExitState(IState nextState);

}

public class StateMachine : MonoBehaviour, IEnumerable<IState>
{

    private IState[] _states;
    private IState _currentState;
 
    void Awake()
    {
        _states = this.gameObject.GetComponents<IState>();
    }
 
    void Start()
    {
        this.ChangeState(0); //first state
    }
 
 
    public void ChangeState(int i)
    {
        //note, I jsut use an index for example. You could key them however you want
        var state = _states[i];
        if(state == _currentState) return;
     
        var oldState = _currentState;
        if(oldState != null) oldState.OnExitState(state);
        _currentState = state;
        if(state != null) state.OnEnterState(oldState);
    }
 
    public void ChangeState<T>() where T : IState
    {
        //hell, you could key them by type
        var state = this.GetComponent<T>();
        if(state == null || state == _currentState || _states.IndexOf(state) < 0) return;
     
        var oldState = _currentState;
        if(oldState != null) oldState.OnExitState(state);
        _currentState = state;
        if(state != null) state.OnEnterState(oldState);
    }
 
}

public class StateA : MonoBehaviour, IState
{

    void Awake()
    {
        //don't receive events
        this.enabled = false;
    }

    public void OnEnterState(IState previousState)
    {
        this.enabled = true;
     
        if(previousState is StateB)
        {
            //do something in the case that this is special
        }
    }
 
    public void OnExitState(IState nextState)
    {
        this.enabled = false;
    }
 
    void Update()
    {
        //this only fires if enabled
    }

}

public class StateB : MonoBehaviour, IState
{

    void Awake()
    {
        //don't receive events
        this.enabled = false;
    }

    public void OnEnterState(IState previousState)
    {
        this.enabled = true;
    }
 
    public void OnExitState(IState nextState)
    {
        this.enabled = false;
    }
 
    //This StateB doesn't care about update, so we don't necessarily need to have it
 
    //This state does care about OnTriggerEnter though...
    void OnTriggerEnter(Collider other)
    {
        //do stuff for this event
    }

}

You could even have a base class for your states to inherit from that handles the enabling and disabling on OnENter and OnExit, and just leave them virtual so each state could do some extra stuff if it needs to.

Note, I allowed the state to control its enabled/disabled, so that some states could stay enabled if they wanted even when it wasn’t the active state. And just tick a bool saying its inactive so it knows not to act like its the current state.

This way a given state could receive the ‘OnTriggerEnter’ and force itself the active state.