SFML snake game in C++

Question

I made a snake game in sfml and i'm kind of proud of the code's structure, but proud doesn't mean it's good, so i'm putting it here so that if there is something that could be bettered, you would know.

main.cpp

#ifndef UNICODE
#define UNICODE
#endif

#include "app.h"

int main() {

    app game(800, 600, L"Test");
    game.start();
    game.end();
}

app.h

#pragma once

#include <SFML/Graphics.hpp>

#include "Snake.h"
#include "Board.h"

class app {
public:

    app(int windowWidth, int windowHeight, const wchar_t* name);
    ~app() = default;

    // Runs the app
    void start();
    void end();

private:

    // MEMBER VARIABLES

    const int winWidth, winHeight;
    int score;
    bool play;
    sf::RenderWindow window;
    Snake snake;
    Board board;

    // MEMBER FUNCTIONS

    // Draws the objects
    void drawWindow();

    // Handles events
    void handleEvents();

    // Updates the window
    void updateWindow();
};

app.cpp

#include "app.h"

#include <iostream>
#include <thread>   
#include <chrono>

app::app(int windowWidth, int windowHeight, const wchar_t* name)
    : winWidth{ windowWidth }, winHeight{ windowHeight }, score{ 0 },
    play{ false } {

    while (true) {
        int choice;

        std::wcout << L"Choose: " << std::endl;
        std::wcout << L"1: Play " << std::endl;
        std::wcout << L"2: Quit " << std::endl;

        std::cin >> choice;

        if (choice == 1) {
            play = true;
            break;
        }
        else break;
    }

    // Clears screen
    for (size_t i = 0; i < 10; ++i)
        std::wcout << L"\n\n\n\n\n\n\n\n\n\n\n\n" << std::endl;

    if (play) {
        window.create(sf::VideoMode(winWidth, winHeight), name);
        window.setFramerateLimit(5);
    }
}

// Handles any game event
void app::handleEvents() {

    sf::Event event;

    while (window.pollEvent(event)) {
        switch (event.type) {

        case sf::Event::Closed:
            window.close();
            break;

        case sf::Event::TextEntered:
            snake.changeDirection(static_cast<char>(event.text.unicode));
        }
    }
}

// Draws all game objects
void app::drawWindow() {

    for (size_t i = 0, h = board.height(); i < h; ++i) {
        for (size_t j = 0, w = board.width(); j < w; ++j) {

            // Draws walls
            if (board[i * w + j] == 2) {
                sf::RectangleShape rect;
                rect.setSize({ static_cast<float>(board.divisor()), static_cast<float>(board.divisor()) });

                rect.setPosition({ static_cast<float>(board.divisor() * j), static_cast<float>(board.divisor() * i)});
                window.draw(rect);
            }

            // Draws snake
            else if (board[i * w + j] == 3) {

                sf::RectangleShape rect;
                rect.setFillColor(sf::Color::Green);
                rect.setSize({ static_cast<float>(board.divisor()), static_cast<float>(board.divisor()) });

                rect.setPosition({ static_cast<float>(board.divisor() * j), static_cast<float>(board.divisor() * i) });
                window.draw(rect);
            }

            // Draws food
            else if (board[i * w + j] == 4) {

                sf::RectangleShape rect;
                rect.setFillColor(sf::Color::Red);
                rect.setSize({ static_cast<float>(board.divisor()), static_cast<float>(board.divisor()) });

                rect.setPosition({ static_cast<float>(board.divisor() * j), static_cast<float>(board.divisor() * i) });
                window.draw(rect);
            }

        }
    }
}

// Updates the render window
void app::updateWindow() {
    window.clear(sf::Color::Black);

    drawWindow();

    window.display();
}

// Starts the app
void app::start() {

    while (window.isOpen()) {

        handleEvents();

        snake.move();
        board.update(window, snake, &score);

        updateWindow();
    }
}

void app::end() {

    if (play) {

        std::wcout << L"You lose!" << std::endl;
        std::wcout << L"Score: " << score << std::endl;

        std::this_thread::sleep_for((std::chrono::milliseconds)3000);
    }
}

Snake.h

#pragma once

#include <SFML/Graphics.hpp>
#include <vector>

class Snake {
public:

    Snake();
    ~Snake() = default;

    // Changes the dir value based on the input
    void changeDirection(char input);

    // Adds a piece to the snake
    void add();

    // Returns the size of snakeContainer
    size_t getSnakeSize();

    // Moves the snake
    void move();

private:

    // MEMBER VARIABLES 
    struct Snake_segment
    {
        int xPos, yPos, prevxPos, prevyPos;
    };

    const enum direction {
        UP = 0,
        RIGHT,
        DOWN,
        LEFT
    };

    std::vector<Snake_segment> snakeContainer;
    direction dir;

    // MEMBER FUNCTIONS

    // Makes the segments follow the head
    void follow();

    // Moves the snake's head
    void moveHead();

public:

    // Operator overloading (i wasn't able to declare it in .cpp file)
    Snake_segment operator[](int i) { return snakeContainer[i]; }

};

Snake.cpp

#include "Snake.h"

// Initializes a two-piece snake
Snake::Snake()
    : dir { RIGHT } {

    Snake_segment head { 10, 7, 9, 7 };
    snakeContainer.push_back(head);

    --head.xPos;
    snakeContainer.push_back(head);
}

void Snake::add() {

    Snake_segment newSegment;
    newSegment.xPos = snakeContainer[snakeContainer.size() - 1].prevxPos;
    newSegment.yPos = snakeContainer[snakeContainer.size() - 1].prevyPos;

    snakeContainer.push_back(newSegment);
}

size_t Snake::getSnakeSize() {
    return snakeContainer.size();
}

// Changes the direction based on input
void Snake::changeDirection(char input) {
    switch (input) {
    case 'w':
        if (dir != DOWN) dir = UP;
        break;

    case 'd':
        if (dir != LEFT) dir = RIGHT;
        break;

    case 's':
        if (dir != UP) dir = DOWN;
        break;

    case 'a':
        if (dir != RIGHT) dir = LEFT;

    }
}

// All the pieces follow the head
void Snake::follow() {

    for (size_t i = 1, n = snakeContainer.size(); i < n; ++i) {
        snakeContainer[i].prevxPos = snakeContainer[i].xPos;
        snakeContainer[i].prevyPos = snakeContainer[i].yPos;

        snakeContainer[i].xPos = snakeContainer[i - 1].prevxPos;
        snakeContainer[i].yPos = snakeContainer[i - 1].prevyPos;
    }

}

// Moves the snake's head
void Snake::moveHead() {

    snakeContainer[0].prevxPos = snakeContainer[0].xPos;
    snakeContainer[0].prevyPos = snakeContainer[0].yPos;

    switch (dir) {
    case UP:
        --snakeContainer[0].yPos;
        break;

    case RIGHT:
        ++snakeContainer[0].xPos;
        break;

    case DOWN:
        ++snakeContainer[0].yPos;
        break;

    case LEFT:
        --snakeContainer[0].xPos;

    }
}

// Moves the snake
void Snake::move() {
    moveHead();
    follow();
}

Board.h

#pragma once

#include <SFML/Graphics.hpp>

#include "Snake.h"

class Board {
public:

    Board();
    ~Board() = default;

    void update(sf::RenderWindow& win, Snake& snek, int* scor);

    int width()            const;
    int height()           const;
    int divisor()          const;
    char operator[](int i) const;

private:

    // MEMBER VARIABLES
    std::string map;
    const size_t mapWidth = 20;
    const size_t mapHeight = 15;

    // Is used to divide the screen in a grid
    const int common_divisor = 40;

    // MEMBER FUNCTIONS

    // Checks if snek has collided with something
    void genFood();
    void checkCollisions(sf::RenderWindow& win, Snake& snek, int* scor);
};

Board.cpp

#include "Board.h"

#include <random>

Board::Board() {

    // Creates a 20x15 grid
    map = {
        2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 2,
        2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2, 2
    };

    /*

    REMINDER:
    1 = FREE SPACE
    2 = WALL
    3 = SNAKE
    4 = FOOD

    */

    genFood();
}

void Board::genFood() {

    int fx, fy;

    do {
        std::random_device gen;
        std::uniform_int_distribution<int> disX(1, mapWidth - 1);
        std::uniform_int_distribution<int> disY(1, mapHeight - 1);

        fx = disX(gen);
        fy = disY(gen);
    } while (map[fy * mapWidth + fx] != 1);

    map[fy * mapWidth + fx] = 4;
}

void Board::update(sf::RenderWindow& win, Snake& snek, int* scor) {

    checkCollisions(win, snek, scor);

    // Iterates through the whole map
    for (size_t i = 0; i < mapHeight; ++i) {
        for (size_t j = 0; j < mapWidth; ++j) {

            // Makes walls
            if (i == 0 || i == mapHeight - 1) map[i * mapWidth + j] = 2;
            else if (j == 0 || j == mapWidth - 1) map[i * mapWidth + j] = 2;

            // Sets free space
            else if (map[i * mapWidth + j] != 4) map[i * mapWidth + j] = 1;

            // Sets snek
            for (size_t k = 0, n = snek.getSnakeSize(); k < n; ++k) {
                if (snek[k].yPos == i && snek[k].xPos == j)
                    map[i * mapWidth + j] = 3;
            }

        }
    }
}

void Board::checkCollisions(sf::RenderWindow& win, Snake& snek, int* scor) {

    for (size_t i = 0; i < mapHeight; ++i) {
        for (size_t j = 0; j < mapWidth; ++j) {

            // Checks snek and wall collisions
            if (map[snek[0].yPos * mapWidth + snek[0].xPos] == 2 ||
                map[snek[0].yPos * mapWidth + snek[0].xPos] == 3    ) win.close();

            // Checks snek and food collisions
            else if (map[snek[0].yPos * mapWidth + snek[0].xPos] == 4) {

                map[snek[0].yPos * mapWidth + snek[0].xPos] = 1;
                snek.add();             
                *scor += 100;

                genFood();
            }
        }
    }

}

int  Board::width()             const { return mapWidth;       }
int  Board::height()            const { return mapHeight;      }
int  Board::divisor()           const { return common_divisor; }
char Board::operator[](int i)   const { return map[i];         }

Edward · Accepted Answer · 2019-05-16 12:45:51Z

Here are some things that may help you improve your code.

Don't declare `enum` `const`

In snake.h, the direction enum is declared as const but this is an error, since only functions and objects can be declared const.

Use `const` where practical

The Snake::getSnakeSize() doesn't alter the underlying Snake and so it should be declared const. Additionally, I'd name it size() to be consistent with standard library functions.

Simplify your code

The current Snake::add() code is this:

void Snake::add() {

    Snake_segment newSegment;
    newSegment.xPos = snakeContainer[snakeContainer.size() - 1].prevxPos;
    newSegment.yPos = snakeContainer[snakeContainer.size() - 1].prevyPos;

    snakeContainer.push_back(newSegment);
}

However, it could be simplified into a single line:

void Snake::add() {
    snakeContainer.push_back({
            snakeContainer.back().prevxPos, 
            snakeContainer.back().prevyPos, 
            snakeContainer.back().prevxPos, 
            snakeContainer.back().prevyPos, 
            });
}

Similarly, the follow code could be simplified by using iterators.

void Snake::follow() {
    auto it = snakeContainer.begin();
    for (auto prev = it++; it != snakeContainer.end(); ++it, ++prev) {
        it->prevxPos = it->xPos;
        it->prevyPos = it->yPos;
        it->xPos = prev->prevxPos;
        it->yPos = prev->prevyPos;
    }
}

In both of these case, futher simplification could be obtained by introducing a struct Coord { unsigned x, y; };

void Snake::follow() {
    auto it = snakeContainer.begin();
    for (auto prev = it++; it != snakeContainer.end(); ++it, ++prev) {
        it->prev = it->curr;
        it->curr = prev->prev;
    }
}

Use `static constexpr` for class constants

The current code has internal const variables for the width and height and then wrapper accessors, but this is much simplified by simply using public static constexpr variables and no wrapper. That's assuming you have a C++11 compiler. If not, the next best thing would be plain const and no wrapper.

Reconsider the class interfaces

Mostly the classes make sense to me, but it seems that the score should actually be maintained by the Board class and then returned to a caller on request via a const method. Also, it seems that divisor should be calculated and stored by the app class as a float. This would remove a lot of ugly static_casts as well. Also, it may make sense for the Board to own the Snake.

Add helper functions for clarity

I would advise converting from a comment to an enum or an enum class and then using that.

enum Tile { Open = 1, Wall, Body, Food };

Next, I'd suggest using helper functions to make the code easier to read and understand. For example:

bool Board::isEmpty(Coord coord) const {
    return at(coord) == Open;
}

bool Board::place(Coord coord, int item) {
    if (item != Open && !isEmpty(coord)) {
        return false;
    }
    map[coord.y * width + coord.x] = item;
    return true;
}

int Board::at(Coord coord) const {
    return map[coord.y * width + coord.x];
}

Here's the corresponding update function.

void Board::update(sf::RenderWindow& win) {
    auto newHead{snake.moveHead()};
    place(snake.follow(), Open);
    switch (at(snake.headLocation())) {
        case Wall: 
        case Body: 
            win.close();
            break;
        case Food: 
            place(snake.headLocation(), Open);
            place(snake.add(), Body);
            m_score += 100;
            genFood();
    }
    place(newHead, Body);
}

With this, note that there is no longer any need to loop through all coordinates and no need for a separate collision detection routine. Also, move is eliminated in favor of the two distinct calls that were in it. In this rewrite, moveHead() returns the location of the new head, and follow() returns the old location of the last segment. Since those are the only two nodes of the snake that change from one iteration to the next, those are the only two cells that need updating.

Don't use `std::endl` if `'\n'` will do

Using std::endl emits a \n and flushes the stream. Unless you really need the stream flushed, you can improve the performance of the code by simply emitting '\n' instead of using the potentially more computationally costly std::endl. Also, you can make things a little neater. Instead of this:

        std::wcout << L"You lose!" << std::endl;
        std::wcout << L"Score: " << score << std::endl;

I would recommend writing it like this:       

        std::wcout << L"You lose!\nScore: " << score << '\n';

Don't overuse `std::random_device`

For some implementations, std::random_device is actually driven by a hardware-based generator and the quality of the generated random numbers may actually drop precipitously if too many random numbers are drawn too quickly. For that reason, it's better not to overuse std::random_device. Instead, seed a pseudorandom generator (PRG) once from std::random_device and then use the PRG. Here's a rewrite of the genFood() routine that does just that:

void Board::genFood() {
    static std::random_device rd;
    static std::mt19937 gen(rd());
    static std::uniform_int_distribution<unsigned> disX(1, width - 2);
    static std::uniform_int_distribution<unsigned> disY(1, height - 2);

    while (!place({disX(gen), disY(gen)}, Food))
    {  /* keep attempting until it works */ }
}

Think of the user

How often does it happen that the user starts a game, only to immediately ask to quit? It seems unlikely to me, so I would eliminate the Play/Quit prompt entirely. Future enhancements that might be nice would be to display the score and length of snake as the game is being played.

Thanks. I never used constexpr, since i really don't understand how it works. anyways i prompt the user play/quit at the start in case the run of the program was a mistake/ something the user didn't want to do, but now that i think about it yeah, you're right, it's pretty useless. — Nadpher
– Nadpher, Commented May 16, 2019 at 13:08
Anyways, i'd like to know why i should make the variables public, without getters and setters. I've always known public variables are a bad practice. Just curious — Nadpher
– Nadpher, Commented May 16, 2019 at 14:30
Public variables are only bad practice when external access to them is not required or when unrestricted external write access could allow an invariant violation. Because external read-only access is required and because const enforces read-only access, making these public is the best way to write it. — Edward
– Edward, Commented May 16, 2019 at 14:47

Stephen Rauch · Accepted Answer · 2019-05-25 02:23:37Z

3

Some observations:

Comments:
```
// Draws the objects
void drawWindow();

// Handles events
void handleEvents();

// Updates the window
void updateWindow();
```
These comments don't tell you anything that the code doesn't already tell you, so they should be removed. Ideally the code is well named, like in that example, so you don't feel the need to even write a comment.

Annother advice with that is. If you feel the need, in a long function, to comment several parts because they do something different it's probably a good idea to extract the parts into its own functions. Erase the Comments and makes the code self documentary.
Namespaces:

You should always put your functions and classes into namespaces to avoid name clashes.
unicode
```
#ifndef UNICODE
#define UNICODE
#endif
```
is this still needed?
std::endl:

if you only want a newline you should replace std::endl with '\n'.

std::endl also does a expensive flush operation which is rarely desired.

edited May 25, 2019 at 2:23

Stephen Rauch

4,31412 gold badges24 silver badges36 bronze badges

answered May 14, 2019 at 16:07

Sandro4912

3,1522 gold badges23 silver badges50 bronze badges

\$\begingroup\$ Really i defined unicode since i've seen on multiple occasions that unicode is now the standard and writing unicode literals is good practice, on Windows docs too. That's why i always define unicode \$\endgroup\$

Nadpher
– Nadpher

2019-05-14 18:57:07 +00:00
Commented May 14, 2019 at 18:57
1

\$\begingroup\$ Pretty sure all compilers today define it for you, you don't need to define it yourself. \$\endgroup\$

Snowbody
– Snowbody

2019-05-25 12:15:23 +00:00
Commented May 25, 2019 at 12:15

Add a comment |

Snowbody · Accepted Answer · 2019-05-25 12:16:43Z

while (true) {

Some advise using for (;;) instead, but this loop really isn't necessary. But you don't even have a loop here; you'll exit the loop immediately no matter what is entered. Also this throws an exception if you enter a non-int.

Clearing the terminal should be done in a separate routine in case you want to switch to a terminal library like curses. (Also terminal is not guaranteed to be 24x80)

Try not to loop over the whole grid. See if you can avoid clearing and redrawing everything. Just make the necessary changes. When drawing the walls, think of a different encoding so you don't have to loop over the whole grid.

"divisor" is a bad name for the size of a grid cell.

snake color should be a parameter rather than a literal. Also the snake should draw itself, and drawWindow should call it.

In SnakeContainer types, use a bit more encapsulation, include xpos and ypos together in a struct pos to cut down on duplicated code..

Shouldn't the snake be a member of the board, not a parameter?

Stack Exchange Network

SFML snake game in C++

3 Answers 3

Don't declare `enum` `const`

Use `const` where practical

Simplify your code

Use `static constexpr` for class constants

Reconsider the class interfaces

Add helper functions for clarity

Don't use `std::endl` if `'\n'` will do

Don't overuse `std::random_device`

Think of the user

You must log in to answer this question.

Linked

Hot Network Questions

SFML snake game in C++

3 Answers 3

Don't declare enum const

Use const where practical

Simplify your code

Use static constexpr for class constants

Reconsider the class interfaces

Add helper functions for clarity

Don't use std::endl if '\n' will do

Don't overuse std::random_device

Think of the user

You must log in to answer this question.

Linked

Related

Hot Network Questions

Don't declare `enum` `const`

Use `const` where practical

Use `static constexpr` for class constants

Don't use `std::endl` if `'\n'` will do

Don't overuse `std::random_device`