Correct way of managing player states?

Question

I'm a relatively new programmer attempting to create a simple 2d fighter for fun using the SFML 2 library. As a game with interaction dependent on what the character is doing, using a state system seems to be the way to go. I've been trying to incorporate a finite state machine. My question is, what is the best way to go about this? Currently I have something like this:

switch (playerstate)
{
case idle:
    if (!onGround)
    {
        playerstate = fall;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Up))
    {
        moveUp();
        playerstate = air;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Down))
    {
        playerstate = crouch;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Right))
    {
        moveRight();
        playerstate = dash;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Left))
    {
        moveLeft();
        playerstate = dash;
    }
    else
    {
        decelOnGround();
    }
    break;
case dash:
    if (!onGround)
    {
        playerstate = fall;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Up))
    {
        moveUp();
        playerstate = air;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Down))
    {
        playerstate = crouch;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Right))
    {
        moveRight();
    }
    else if (sf::Keyboard::isKeyPressed(controls.Left))
    {
        moveLeft();
    }
    else
    {
        decelOnGround();
        playerstate = idle;
    }
    break;
case air:
    if (onGround)
    {
        playerstate = idle;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Up))
    {
        // if numOfJumps > 0, moveUp();
    }
    else if (sf::Keyboard::isKeyPressed(controls.Down))
    {
        fastFall();
    }
    else if (sf::Keyboard::isKeyPressed(controls.Right))
    {
        moveRight();
    }
    else if (sf::Keyboard::isKeyPressed(controls.Left))
    {
        moveLeft();
    }
    break;
case fall:
    if (onGround)
    {
        playerstate = idle;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Down))
    {
        fastFall();
    }
    break;
case crouch:
    if (sf::Keyboard::isKeyPressed(controls.Up))
    {
        moveUp();
        playerstate = air;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Down))
    {
        decelOnGround();
        playerstate = crouch;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Right))
    {
        moveRight();
        playerstate = dash;
    }
    else if (sf::Keyboard::isKeyPressed(controls.Left))
    {
        moveLeft();
        playerstate = dash;
    }
    else
    {
        playerstate = idle;
    }
    break;
default:
    std::cout << "ERROR\n";
}

Which as you can see, there is a lot of repetition and a lot of code. I haven't even incorporated using different attacks yet (I'm going to have a variety of these). Am I approaching this the wrong way? Should I instead have each control be a case instead and branch from there? Any help is appreciated!

Answer 1

You should not have if-statements nor case-statements; at least not nearly so many.

The immediate problem is that you're hard-coding a bunch of logic where a data-driven approach would work better. What you have here on first glance is a state graph (or a finite state machine). Rebuilding this as a state graph with inputs triggering transitions would be a first pass "cleanup".

The second pass then is to move all the specifics into separate logic or data. e.g., once you have a state graph runtime, even your hardcoded logic becomes much simpler:

state idle = machine.createState();
state falling = machine.createState();
state moving_right = machine.createState();
state moving_left = machine.createState();

idle.when([]{ !player.isGrounded(); }, falling);
idle.when([]{ isKeyDown(KEY_LEFT); }, moving_left);
idle.when([]{ isKeyDown(KEY_RIGHT); }, moving_right);

moving_right.update([]{ player.moveRight(); });
moving_right.when([]{ !isKeyDown(KEY_RIGHT); }, idle);
moving_right.when([]{ !player.isGrounded(); }, falling);

// and so on

You can also support hierarchical graphs so e.g. your attacks can be their own state graphs independent of moving.

With scripting or some other binding system, this can also easily become a data file. Data files are nice because you don't have to recompile to test changes. If you have hot reload support you can even tweak your data while playing the game, which really speeds up the development process.

The data then can map to your animation system, giving you controls on animation loops for states or for animations to play on transitions. A more "production grade" approach would also allow the animation system to feed back into the game logic, letting you tie attach hit boxes and such into the animations, or triggering audio and particles from animation frames, or even controlling physics (e.g., you wouldn't hardcode the movement logic into your movement state, but rather give your movement animations the "apply X force" action, making moving tied to animation state, and blendable if you have multiple animations that might affect movement).