0
\$\begingroup\$

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

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

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);
    }
}
\$\endgroup\$
1
  • 1
    \$\begingroup\$ Hello, I made the changes that you suggested, and the moving states are more clear now. Thanks for that. Also, about the hashset override, tbh, now I realize that it wasn't making sense overriding the StateTransitions, and I fixed that. But I didn't make it public like you suggested; instead, I marked it as protected and created a public function to get the readonly collection, like this: public IReadOnlyCollection(State, Funcbool>)> GetTransitions() { return StateTransitions; } I'll also make another change, but for now, that's all. Thank you :D \$\endgroup\$
    – Thomas
    Apr 15, 2023 at 18:46

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.