Simple Snake Game - followup

Question

Based on the previous question, I have implemented all suggestions. Also, I added new features to the game.

Summary of improvements:

• Implemented smooth movements to snake instead of tile movements to give realistic impression
• Added fixed timestep
• Game relied on radian and degrees instead of enum of directions for computing positions and directions compassion

I would like to know how I can improve it further.

#include <SFML/Graphics.hpp>

#include <array>
#include <vector>
#include <random>
#include <utility>
#include <iostream>
#include <algorithm>
#include <functional>

namespace
{
    constexpr auto BlockSize = 16.f;
    const sf::Vector2f WindowSize(640, 480);

    auto randomEngine()
    {
        std::array<std::mt19937::result_type, std::mt19937::state_size> seed_data;
        thread_local std::random_device source;
        std::generate(std::begin(seed_data), std::end(seed_data), std::ref(source));
        std::seed_seq seeds(std::begin(seed_data), std::end(seed_data));
        thread_local std::mt19937 seeded_engine(seeds);
        return seeded_engine;
    }

    auto random(std::uniform_real_distribution<float>& dist)
    {
        thread_local auto RandomEngine = randomEngine();
        return dist(RandomEngine);
    }
}

class Snake : public sf::Drawable, sf::NonCopyable
{
public:
    enum Direction
    {
        Up,
        Down,
        Left,
        Right,
        DirectionCount
    };


private:
    using SnakeContainer = std::vector<sf::RectangleShape>;
    using DirectionContainer = std::array<float, Direction::DirectionCount>;


public:
    explicit Snake(const sf::Font& font)
    {
        reset();

        mLivesText.setFont(font);
        mLivesText.setStyle(sf::Text::Bold);
        mLivesText.setCharacterSize(30);
        mLivesText.setColor(sf::Color::White);
        mLivesText.setPosition(WindowSize.x - 160.f, 0.f);
    }

    void reset()
    {
        mSnakeBody.clear();
        sf::RectangleShape shape({ BlockSize - 1, BlockSize - 1 });

        shape.setOrigin(shape.getSize() / 2.f);
        shape.setPosition(70 + BlockSize, 70 + 3 * BlockSize);
        shape.setFillColor(sf::Color::Yellow);
        mSnakeBody.push_back(shape);

        shape.setPosition(70 + BlockSize, 70 + 2 * BlockSize);
        shape.setFillColor(sf::Color::Green);
        mSnakeBody.push_back(shape);

        shape.setPosition(70 + BlockSize, 70 + BlockSize);
        mSnakeBody.push_back(shape);

        mDirection = Direction::Down;
        mLives = 3;
        mIsFirstRun = true;
        mLivesText.setString("Lives: " + std::to_string(mLives));
    }

    void setDirection(Direction dir)
    {
        if (std::abs(getDegree(mDirection) - getDegree(dir)) == 180) return;
        std::exchange(mIsFirstRun, false);
        mDirection = dir;
    }

    bool hasLost() const
    {
        return mLives == 0;
    }

    void lose()
    {
        mLives = 0;
    }

    void extend()
    {
        sf::RectangleShape shape({ BlockSize - 1, BlockSize - 1 });
        shape.setOrigin(shape.getSize() / 2.f);

        const auto& tail = mSnakeBody.back();

        auto x = tail.getPosition().x - std::sin(getRadian()) * BlockSize;
        auto y = tail.getPosition().y - std::cos(getRadian()) * BlockSize;;

        shape.setPosition(x, y);

        shape.setFillColor(sf::Color::Green);
        mSnakeBody.push_back(shape);
    }

    void update(sf::Time dt)
    {
        if (mIsFirstRun) return;

        move(dt);
        checkCollision();
    }

    sf::FloatRect getGlobalBounds() const
    {
        return mSnakeBody.front().getGlobalBounds();
    }

    bool hasCollideWithFruit(const sf::CircleShape& shape) const
    {
        return std::find_if(mSnakeBody.begin(), mSnakeBody.end(),
            [&shape](const auto& part)
        {
            return part.getGlobalBounds().intersects(shape.getGlobalBounds());
        }) != mSnakeBody.end();
    }


private:
    void draw(sf::RenderTarget& target, sf::RenderStates states) const override
    {
        for (const auto& part : mSnakeBody)
            target.draw(part, states);

        target.draw(mLivesText, states);
    }

    void checkCollision()
    {
        if (mSnakeBody.size() < 5) return;

        const auto& head = mSnakeBody.front();

        std::all_of(mSnakeBody.begin() + 1, mSnakeBody.end(), 
            [this, &head](const auto& part)
        {
            if (!part.getGlobalBounds().contains(head.getPosition())) return true;

            auto index = &*(mSnakeBody.end() - 1) - &part;
            shrink(index);
            return false;
        });
    }

    void move(sf::Time dt)
    {
        constexpr static auto SnakeSpeed = 80.f;
        auto vx = std::sin(getRadian()) * SnakeSpeed * dt.asSeconds();
        auto vy = std::cos(getRadian()) * SnakeSpeed * dt.asSeconds();
        mSnakeBody.front().move(vx, vy);

        std::for_each(mSnakeBody.begin() + 1, mSnakeBody.end(),
            [this](auto& current)
        {
            auto index = &current - &*(mSnakeBody.begin() + 1);
            const auto& previous = mSnakeBody[index];

            auto position = previous.getPosition() - current.getPosition();
            auto angle = std::atan2(position.y, position.x);

            auto x = previous.getPosition().x - std::cos(angle) * BlockSize;
            auto y = previous.getPosition().y - std::sin(angle) * BlockSize;
            current.setPosition(x, y);
        });
    }

    void shrink(std::size_t index)
    {
        for (auto i = 0u; i <= index; ++i)
            mSnakeBody.pop_back();

        --mLives;

        mLivesText.setString("Lives: " + std::to_string(mLives));
    }

    float getRadian() const
    {
        constexpr static auto Ratio = 0.0174532925f; //pi/180
        constexpr static DirectionContainer radians{ 180 * Ratio, 0, 270 * Ratio, 90 * Ratio };

        return radians[mDirection];
    }

    float getDegree(Direction dir) const
    {
        constexpr static DirectionContainer degrees{ 180, 0, 270, 90 };

        return degrees[dir];
    }


private:
    SnakeContainer mSnakeBody;
    Direction mDirection;
    std::size_t mLives;
    sf::Text mLivesText;
    bool mIsFirstRun;
};

class World : public sf::Drawable, sf::NonCopyable
{
    using BordersContainer = std::vector<sf::RectangleShape>;
    using DistType = std::uniform_real_distribution<float>;


public:
    explicit World(const sf::Font& font)
        : mSnake(font)
        , mBorders(4)
        , mDistX(2 * BlockSize, WindowSize.x - 2 * BlockSize)
        , mDistY(3 * BlockSize, WindowSize.y - 2 * BlockSize)
        , mScore()
    {
        mFruit.setFillColor(sf::Color::Red);
        mFruit.setRadius(BlockSize / 2.f);
        respawnFruit();

        // initial Wall borders
        for (auto& side : mBorders)
        {
            auto index = &side - &*mBorders.begin();

            if (index % 2 == 0)
                side.setSize({ WindowSize.x, BlockSize });
            else
                side.setSize({ BlockSize, WindowSize.y });

            if (index == 1)
                side.setPosition(WindowSize.x - BlockSize, 2 * BlockSize);
            else if (index == 2)
                side.setPosition(0, WindowSize.y - BlockSize);
            else
                side.setPosition(0, 2 * BlockSize);

            side.setFillColor(sf::Color(211, 211, 211));
        }

        mScoreText.setString("Score: " + std::to_string(mScore));
        mScoreText.setFont(font);
        mScoreText.setStyle(sf::Text::Bold);
        mScoreText.setCharacterSize(30);
        mScoreText.setColor(sf::Color::White);
        mScoreText.setPosition(30.f, 0.f);
    }

    void update(sf::Time dt)
    {
        // real time input
        if (sf::Keyboard::isKeyPressed(sf::Keyboard::Up))
            mSnake.setDirection(Snake::Up);

        else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Down))
            mSnake.setDirection(Snake::Down);

        else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Left))
            mSnake.setDirection(Snake::Left);

        else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Right))
            mSnake.setDirection(Snake::Right);

        if (mSnake.hasLost())
            mSnake.reset();

        if (mFruit.getGlobalBounds().intersects(mSnake.getGlobalBounds()))
        {
            mSnake.extend();
            mScoreText.setString("Score: " + std::to_string(mScore += 10));
            respawnFruit();
        }

        for (const auto& side : mBorders)
        {
            if (!side.getGlobalBounds().intersects(mSnake.getGlobalBounds())) continue;

            mScore = 0;
            mScoreText.setString("Score: " + std::to_string(mScore));
            mSnake.lose();
        }

        // update
        mSnake.update(dt);
    }


private:
    void draw(sf::RenderTarget& target, sf::RenderStates states) const override
    {
        for (const auto& side : mBorders)
            target.draw(side, states);

        target.draw(mFruit, states);
        target.draw(mSnake, states);
        target.draw(mScoreText, states);
    }

    void respawnFruit()
    {
        do
            mFruit.setPosition(random(mDistX), random(mDistY));
        while (mSnake.hasCollideWithFruit(mFruit));
    }


private:
    Snake mSnake;
    sf::CircleShape mFruit;
    sf::Text mScoreText;
    BordersContainer mBorders;
    DistType mDistX;
    DistType mDistY;
    std::size_t mScore;
};

class Game : sf::NonCopyable
{
public:
    Game()
        : mWindow(sf::VideoMode(static_cast<int>(WindowSize.x), static_cast<int>(WindowSize.y)), "Snake")
        , mWorld(mFont)
    {
        if (!mFont.loadFromFile("arial.ttf"))
            throw "Can't load font file";
    }

    void run()
    {
        sf::Clock clock;
        sf::Time timeSinceLastUpdate = sf::Time::Zero;
        const auto TimePerFrame = sf::seconds(1.f / 60.f);

        while (mWindow.isOpen())
        {
            sf::Time dt = clock.restart();
            timeSinceLastUpdate += dt;

            while (timeSinceLastUpdate > TimePerFrame)
            {
                timeSinceLastUpdate -= TimePerFrame;

                processEvents();
                update(TimePerFrame);
            }

            render();
        }
    }


private:
    void processEvents()
    {
        sf::Event event;

        while (mWindow.pollEvent(event))
        {
            if (event.type == sf::Event::Closed)
                mWindow.close();
        }
    }

    void update(sf::Time dt)
    {
        mWorld.update(dt);
    }

    void render()
    {
        mWindow.clear();
        mWindow.draw(mWorld);
        mWindow.display();
    }


private:
    sf::RenderWindow mWindow;
    World mWorld;
    sf::Font mFont;
};

int main()
{
    try
    {
        Game game;
        game.run();
    }
    catch (std::runtime_error& e)
    {
        std::cout << "Exception: " << e.what() << std::endl;
        return 1;
    }
}

Answer 1

I don't know anything about SFML, but I'll offer some more general tips and observations.

Interfaces should be minimal but complete

It seems unnecessary for Snake::getRadian and Snake::getDegree to be (private) member functions of Snake. Instead, they should be free (non-member) functions, and not clutter the class interface. Similarly couldn't the Direction enum even exist outside of Snake?

Implementation details

There are some implementation details I think could be improved. Let me start with the ones I have a stronger opinion on. In Snake::setDirection, why do you use std::exchange? It seems unnecessary, as you don't do anything with the old value of mIsFirstRun. That is, why not simply assign false to the boolean? Also, I don't think it makes any difference here, but be aware of the difference between push_back and emplace_back.

In Snake::shrink, you don't need to do pop_back's "manually". Instead, you can replace the explicit for-loop by:

v.erase(v.end() - index - 1, v.end());

This is arguably more readable, but in general tends to be more efficient in my experience.

In World::update, I don't like this line:

mScoreText.setString("Score: " + std::to_string(mScore += 10));

I think it makes the reader wonder "what is happening to mScore here, and is it intended?" I'd prefer to increment mScore first, and then set the string. There is another thing in World::update I don't quite like: the if-else structure checking for different keyboard presses. Instead, why not consider a more data-driven approach? That is, you could set up e.g., a map, where the keys are possible recognized keyboard presses. The value is then the desired action, for instance. This way, you avoid the manual error prone approach of if-elses. (This idea could be taken further, and applied to some other places too).

Magic values

I think you are already using some constant values, but you could do more of this. BlockSize is a good start. In Snake, I see values like "70", "3", "2.f", "5", and so on. Especially if the values pop up in multiple places, it can be a nightmare to modify them. So make them constant values as well, as you lose nothing.