Strategy Pattern Inside State Pattern

Question

I'm making a fps game where I created a hierarchical state machine; the problem is the two types of movement (standing movement and crouching movement). They are very similar; the only difference is the velocity variable to apply the movement speed, so I decided to apply the strategy pattern and came up with this solution:

using UnityEngine;
using static PlayerStateMachine;

public abstract class MovementState : MovementType
{
    protected readonly PlayerCharacterController _playerCharacterController;

    protected readonly PlayerInputsManager _playerInputsManager;

    protected readonly PlayerStateParameters _playerStateParameters;

    protected readonly Transform _playerCameraTransform;

    protected Vector3 _playerDesiredDirectionVelocity;

    public MovementState(PlayerCharacterController playerCharacterController, PlayerInputsManager playerInputsManager, PlayerStateParameters playerStateParameters, Transform playerCameraTransform)
    {
        _playerInputsManager = playerInputsManager;

        _playerCharacterController = playerCharacterController;

        _playerCameraTransform = playerCameraTransform;

        _playerStateParameters = playerStateParameters;
    }
}

public abstract class MovementType
{
    public abstract void Move();
}

public class StandingMovement : MovementState
{
    public StandingMovement(PlayerCharacterController playerCharacterController, PlayerInputsManager playerInputsManager, PlayerStateParameters playerStateParameters, Transform playerCameraTransform) : base(playerCharacterController, playerInputsManager, playerStateParameters, playerCameraTransform)
    {

    }

    public override void Move()
    {
        _playerDesiredDirectionVelocity = (_playerCameraTransform.forward * _playerInputsManager.PlayerMovementInputValue.z + _playerCameraTransform.right * _playerInputsManager.PlayerMovementInputValue.x).normalized * _playerStateParameters.PlayerStandingMovementSpeed;

        _playerCharacterController.ChangePlayerMovementDirection(_playerDesiredDirectionVelocity);
    }
}

public class CrouchedMovement : MovementState
{
    public CrouchedMovement(PlayerCharacterController playerCharacterController, PlayerInputsManager playerInputsManager, PlayerStateParameters playerStateParameters, Transform playerCameraTransform) : base(playerCharacterController, playerInputsManager, playerStateParameters, playerCameraTransform)
    {

    }

    public override void Move()
    {
        _playerDesiredDirectionVelocity = (_playerCameraTransform.forward * _playerInputsManager.PlayerMovementInputValue.z + _playerCameraTransform.right * _playerInputsManager.PlayerMovementInputValue.x).normalized * _playerStateParameters.PlayerCrouchedMovementSpeed;

        _playerCharacterController.ChangePlayerMovementDirection(_playerDesiredDirectionVelocity);
    }
}

Then, inside the movement states I just Tick like this:

using System;

using System.Collections.Generic;

using UnityEngine;

public class PlayerStandingMovingState : State
{
    private readonly PlayerStandingRootState _playerStateMachine;

    private readonly PlayerTransitionsFactory _playerTransitionsFactory;

    private readonly MovementType _movementType;

    protected override HashSet<(State, Func<bool>)> StateTransitions { get; set; }

    public PlayerStandingMovingState(PlayerStandingRootState playerStateMachine, PlayerCharacterController playerCharacterController, PlayerInputsManager playerInputsManager, PlayerStateMachine.PlayerStateParameters playerStateParameters, PlayerTransitionsFactory playerTransitionsFactory, Transform playerCameraTransform)
    {
        _playerStateMachine = playerStateMachine;

        _playerTransitionsFactory = playerTransitionsFactory;

        _movementType = new StandingMovement(playerCharacterController, playerInputsManager, playerStateParameters, playerCameraTransform);
    }

    public override void InitializeState()
    {
        base.InitializeState();

        AddTransition(_playerStateMachine.PlayerIdleState, _playerTransitionsFactory.PlayerIsNotMoving);
    }

    public override void Tick()
    {
        _movementType.Move();
    }
}

(Crouched Movement State on The Crouched Root State)

using System;

using System.Collections.Generic;

using UnityEngine;

public class PlayerCrouchedMovingState : State
{
    private readonly PlayerCrouchedRootState _playerStateMachine;

    private readonly PlayerTransitionsFactory _playerTransitionsFactory;

    private readonly MovementType _movementType;

    protected override HashSet<(State, Func<bool>)> StateTransitions { get; set; }

    public PlayerCrouchedMovingState(PlayerCrouchedRootState playerCrouchedRootState, PlayerCharacterController playerCharacterController, PlayerInputsManager playerInputsManager, PlayerStateMachine.PlayerStateParameters playerStateParameters, PlayerTransitionsFactory playerTransitionsFactory, Transform playerCameraTransform)
    {
        _playerStateMachine = playerCrouchedRootState;

        _playerTransitionsFactory = playerTransitionsFactory;

        _movementType = new CrouchedMovement(playerCharacterController, playerInputsManager, playerStateParameters, playerCameraTransform);
    }

    public override void InitializeState()
    {
        base.InitializeState();

        AddTransition(_playerStateMachine.PlayerCrouchedIdleState, _playerTransitionsFactory.PlayerIsNotMoving);
    }

    public override void Tick()
    {
        _movementType.Move();
    }
}

I would like to know if I`m correctly implementing the strategy pattern using the state pattern, as well as if you guys have any tips to improve the code architecture. If you guys want to see the full project and the full state machine code I'll leave the link to my project, it's public repository: https://github.com/ThomasAllenSilva/Asylum-Horror

Thanks

Answer 1

Ideally you would create an intermediate abstract class PlayerMovingState between State and the different other player moving states, so that you could pull up all the common members up to it. The private members would have to be protected instead.

public abstract class PlayerMovingState : State
{
    protected readonly PlayerMovingState _playerStateMachine;
    protected readonly PlayerTransitionsFactory _playerTransitionsFactory;
    protected readonly MovementType _movementType;

    public PlayerMovingState(PlayerMovingState playerStateMachine, PlayerTransitionsFactory playerTransitionsFactory, MovementType movementType)
    {
        _playerStateMachine = playerStateMachine;
        _playerTransitionsFactory = playerTransitionsFactory;
        _movementType = movementType;
    }

    public override void Tick()
    {
        _movementType?.Move();
    }
}

Also, it is not clear to me why you override the StateTransitions HashSet. It could be implemented in State right away (even if State is abstract):

// In class State
public HashSet<(State, Func<bool>)> StateTransitions { get; } =
    new HashSet<(State, Func<bool>)>();

Then e.g. PlayerCrouchedMovingState becomes

public class PlayerCrouchedMovingState : PlayerMovingState
{
    public PlayerCrouchedMovingState(
            PlayerCrouchedRootState playerCrouchedRootState,
            PlayerCharacterController playerCharacterController, 
            PlayerInputsManager playerInputsManager,
            PlayerStateParameters playerStateParameters,
            PlayerTransitionsFactory playerTransitionsFactory,
            Transform playerCameraTransform
        ) : base(
            playerCrouchedRootState,
            playerTransitionsFactory,
            new CrouchedMovement(playerCharacterController, playerInputsManager,
                playerStateParameters, playerCameraTransform)
        )
    {
    }

    public override void InitializeState()
    {
        base.InitializeState();
        AddTransition(_playerStateMachine.PlayerCrouchedIdleState,
           _playerTransitionsFactory.PlayerIsNotMoving);
    }
}