Simple Snake Game

I was working on this game for while and I have made it for education purposes by using SFML. The game works fine but I would like to know how I can improve it.

```
#include

#include
#include
#include
#include
#include
#include
#include

namespace
{
const sf::Vector2f WindowSize(640, 480);
constexpr auto BlockSize = 16.f;
constexpr auto SnakeSpeed = 5.f;
const auto TimeStep = sf::seconds(1 / SnakeSpeed);

auto randomEngine()
{
std::array 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(const std::uniform_real_distribution& dist)
{
static auto& RandomEngine = randomEngine();
return dist(RandomEngine);
}
}

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

private:
using SnakeContainer = std::vector;

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.setPosition(70 + BlockSize, 70 + 3 * BlockSize);
shape.setFillColor(sf::Color(211, 211, 211));
mSnakeBody.push_back(shape);

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

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

mDirection = Direction::None;
mLives

I don't know SFML at all, so very partial review.

auto random(const std::uniform_real_distribution& dist)
{
    static auto& RandomEngine = randomEngine();
    return dist(RandomEngine);
}

There are two portability bugs here:

• randomEngine() returns a temporary, but auto& will deduce a non-const reference type here - that's an error (MSVC has an extension that accepts this though, unfortunately). But you don't need a reference here though, just a plain static object, so drop the &

• the distribution's operator()(Generator &) member is not const according to the standard. You should remove the const from parameter for this to be portable.

auto randomEngine()
{
    // ...
    thread_local std::mt19937 seeded_engine(seeds);
    // ...
}

This thread_local doesn't make sense here, at least not with your random() function's usage. (Even without that it's fishy, your seed manipulations would be executed on every call, but the actual engine would be constructed only once and copied out of the function.) Remove it.

If threads are involved, you need to change the static to thread_local in random, you'll get data races in there otherwise.

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() {...}
// ...
std::size_t mScore;
// ...
DistType mDistX;
DistType mDistY;

The order in which you declared your variables is the order in which they will be initialized at runtime. But the order you have them in the constructor's initializer list is different. This can lead to subtle bugs when initialization of some members depends on others. You should make both orders match.

StackExchange Code Review Q#115544, answer score: 7