Snake++ game (in C++ with SDL)

Question

Yes, It's called Snake++. I build it to learn C++. What features and techniques can be improved? I used SDL for some basic rendering, but my concern is more about the language use.

Things I'm very concerned about:

1. Generating a new food means trying random positions over and over again till one is free. This is going to become a problem very soon. What data structures can I use here?
2. Am I using references to their full potential and avoiding unnecessary copying?

Main.cpp

#include <iostream>
#include "Game.hpp"

using namespace std;

int main(int argc, char * argv[])
{
    Game game = Game();
    Game().Run();
    cout << "Game has terminated successfully, score: " << game.GetScore()
        << ", size: " << game.GetSize() << endl;
    return 0;
}

Game.hpp

#pragma once
#include <vector>
#include "SDL.h"
#include "SDL_image.h"

class Game
{

public:

    Game();
    void Run();
    int GetScore();
    int GetSize();

private:

    bool running = false;
    bool alive = false;
    int fps = 0;

    static const int FRAME_RATE     = 1000 / 60;
    static const int SCREEN_WIDTH   = 640;
    static const int SCREEN_HEIGHT  = 640;
    static const int GRID_WIDTH     = 32;
    static const int GRID_HEIGHT    = 32;

    SDL_Window * window = nullptr;
    SDL_Renderer * renderer = nullptr;

    enum class Block { head, body, food, empty };
    enum class Move { up, down, left, right };

    Move last_dir = Move::up;
    Move dir = Move::up;

    struct { float x = GRID_WIDTH / 2, y = GRID_HEIGHT / 2; } pos;

    SDL_Point head = { static_cast<int>(pos.x), static_cast<int>(pos.y) };
    SDL_Point food;
    std::vector<SDL_Point> body;

    Block grid[GRID_WIDTH][GRID_HEIGHT];

    float speed = 0.5f;
    int growing = 0;
    int score = 0;
    int size = 1;

    void ReplaceFood();
    void GrowBody(int quantity);
    void UpdateWindowTitle();
    void GameLoop();
    void Render();
    void Update();
    void PollEvents();
    void Close();

};

Game.cpp

#include <iostream>
#include <string>
#include <ctime>

#include "SDL.h"
#include "Game.hpp"

using namespace std;

Game::Game()
{
    for (int i = 0; i < GRID_WIDTH; ++i)
        for (int j = 0; j < GRID_HEIGHT; ++j)
        {
            grid[i][j] = Block::empty;
        }

    srand(static_cast<unsigned int>(time(0)));
}

void Game::Run()
{
    // Initialize SDL
    if (SDL_Init(SDL_INIT_VIDEO) < 0)
    {
        cerr << "SDL could not initialize! SDL_Error: " << SDL_GetError() << endl;
        exit(EXIT_FAILURE);
    }

    // Create Window
    window = SDL_CreateWindow("Snake Game", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED,
        SCREEN_WIDTH, SCREEN_HEIGHT, SDL_WINDOW_SHOWN);

    if (window == NULL)
    {
        cout << "Window could not be created! SDL_Error: " << SDL_GetError() << endl;
        exit(EXIT_FAILURE);
    }

    // Create renderer
    renderer = SDL_CreateRenderer(window, -1, SDL_RENDERER_ACCELERATED);
    if (renderer == NULL)
    {
        cout << "Renderer could not be created! SDL_Error: " << SDL_GetError() << endl;
        exit(EXIT_FAILURE);
    }

    alive = true;
    running = true;
    ReplaceFood();
    GameLoop();
}

void Game::ReplaceFood()
{
    int x, y;
    while (true)
    {
        x = rand() % GRID_WIDTH;
        y = rand() % GRID_HEIGHT;

        if (grid[x][y] == Block::empty)
        {
            grid[x][y] = Block::food;
            food.x = x;
            food.y = y;
            break;
        }
    }
}

void Game::GameLoop()
{
    Uint32 before, second = SDL_GetTicks(), after;
    int frame_time, frames = 0;

    while (running)
    {
        before = SDL_GetTicks();

        PollEvents();
        Update();
        Render();

        frames++;
        after = SDL_GetTicks();
        frame_time = after - before;

        if (after - second >= 1000)
        {
            fps = frames;
            frames = 0;
            second = after;
            UpdateWindowTitle();
        }

        if (FRAME_RATE > frame_time)
        {
            SDL_Delay(FRAME_RATE - frame_time);
        }
    }

}

void Game::PollEvents()
{
    SDL_Event e;
    while (SDL_PollEvent(&e))
    {
        if (e.type == SDL_QUIT)
        {
            running = false;
        }
        else if (e.type == SDL_KEYDOWN)
        {
            switch (e.key.keysym.sym)
            {
                case SDLK_UP:
                    if (last_dir != Move::down || size == 1)
                        dir = Move::up;
                    break;

                case SDLK_DOWN:
                    if (last_dir != Move::up || size == 1)
                        dir = Move::down;
                    break;

                case SDLK_LEFT:
                    if (last_dir != Move::right || size == 1)
                        dir = Move::left;
                    break;

                case SDLK_RIGHT:
                    if (last_dir != Move::left || size == 1)
                        dir = Move::right;
                    break;
            }
        }
    }
}

int Game::GetSize()
{
    return size;
}

void Game::GrowBody(int quantity)
{
    growing += quantity;
}

void Game::Update()
{
    if (!alive)
        return;

    switch (dir)
    {
        case Move::up:
            pos.y -= speed;
            pos.x = floorf(pos.x);
            break;

        case Move::down:
            pos.y += speed;
            pos.x = floorf(pos.x);
            break;

        case Move::left:
            pos.x -= speed;
            pos.y = floorf(pos.y);
            break;

        case Move::right:
            pos.x += speed;
            pos.y = floorf(pos.y);
            break;
    }

    // Wrap
    if (pos.x < 0) pos.x = GRID_WIDTH - 1;
    else if (pos.x > GRID_WIDTH - 1) pos.x = 0;

    if (pos.y < 0) pos.y = GRID_HEIGHT - 1;
    else if (pos.y > GRID_HEIGHT - 1) pos.y = 0;

    int new_x = static_cast<int>(pos.x);
    int new_y = static_cast<int>(pos.y);

    // Check if head position has changed
    if (new_x != head.x || new_y != head.y)
    {
        last_dir = dir;

        // If we are growing, just make a new neck
        if (growing > 0)
        {
            size++;
            body.push_back(head);
            growing--;
            grid[head.x][head.y] = Block::body;
        }
        else
        {
            // We need to shift the body
            SDL_Point free = head;
            vector<SDL_Point>::reverse_iterator rit = body.rbegin();
            for ( ; rit != body.rend(); ++rit)
            {
                grid[free.x][free.y] = Block::body;
                swap(*rit, free);
            }

            grid[free.x][free.y] = Block::empty;
        }

    }

    head.x = new_x;
    head.y = new_y;

    Block & next = grid[head.x][head.y];
    // Check if there's food over here
    if (next == Block::food)
    {
        score++;
        ReplaceFood();
        GrowBody(1);
    }
    // Check if we're dead
    else if (next == Block::body)
    {
        alive = false;
    }

    next = Block::head;
}

int Game::GetScore()
{
    return score;
}

void Game::UpdateWindowTitle()
{
    string title = "Snakle++ Score: " + to_string(score) + " FPS: " + to_string(fps);
    SDL_SetWindowTitle(window, title.c_str());
}

void Game::Render()
{
    SDL_Rect block;
    block.w = SCREEN_WIDTH / GRID_WIDTH;
    block.h = SCREEN_WIDTH / GRID_HEIGHT;

    // Clear screen
    SDL_SetRenderDrawColor(renderer, 0x1E, 0x1E, 0x1E, 0xFF);
    SDL_RenderClear(renderer);

    // Render food
    SDL_SetRenderDrawColor(renderer, 0xFF, 0xCC, 0x00, 0xFF);
    block.x = food.x * block.w;
    block.y = food.y * block.h;
    SDL_RenderFillRect(renderer, &block);

    // Render snake's body
    SDL_SetRenderDrawColor(renderer, 0xFF, 0xFF, 0xFF, 0xFF);
    for (SDL_Point & point : body)
    {
        block.x = point.x * block.w;
        block.y = point.y * block.h;
        SDL_RenderFillRect(renderer, &block);
    }

    // Render snake's head
    block.x = head.x * block.w;
    block.y = head.y * block.h;
    if (alive) SDL_SetRenderDrawColor(renderer, 0x00, 0x7A, 0xCC, 0xFF);
    else       SDL_SetRenderDrawColor(renderer, 0xFF, 0x00, 0x00, 0xFF);
    SDL_RenderFillRect(renderer, &block);

    // Update Screen
    SDL_RenderPresent(renderer);
}

void Game::Close()
{
    SDL_DestroyWindow(window);
    SDL_Quit();
}

Answer 1

Object Usage

This code:

Game game = Game();
Game().Run();
cout << "Game has terminated successfully, score: " << game.GetScore()
    << ", size: " << game.GetSize() << endl;

...isn't doing what I'm pretty sure you think it is. This part: Game game = Game(); creates an object named game which is of type Game. But, I'd prefer to use just Game game;, which accomplishes the same thing more easily.

Then you do: Game().Run();. This creates another (temporary) Game object, and invokes the Run member function on that temporary Game object (so the Game object named game that you just creates sits idly by, doing nothing).

Then you do:

cout << "Game has terminated successfully, score: " << game.GetScore()
    << ", size: " << game.GetSize() << endl;

...which tries to print the score accumulated in the object named game--but game hasn't run. Only the temporary object has run (so by rights, the score you display should always be 0).

If I were doing this, I'd probably do something more like:

Game game;
game.run();
cout << "Game has terminated successfully, score: " << game.GetScore()
    << ", size: " << game.GetSize() << endl;

using namespace std; isn't just using; it's abusing!

I'd (strongly) advise against using namespace std;. A using directive for another namespace can be all right, but std:: contains a huge amount of stuff, some of it with very common names that are likely to conflict with other code. Worse, every new release of the C++ standard adds still more "stuff" to std. It's generally preferable to just qualify names when you use them, so (for example) the cout shown above would be more like:

std::cout << "Game has terminated successfully, score: " << game.GetScore()
    << ", size: " << game.GetSize() << std::endl;

Avoid std::endl

I'd advise avoiding std::endl in general. Along with writing a new-line to the stream, it flushes the stream. You want the new-line, but almost never want to flush the stream, so it's generally better to just write a \n. On the rare occasion that you actually want the flush, do it explicitly: std::cout << '\n' << std::flush;.

Avoid the C random number generation routines

C's srand()/rand() have quite a few problems. I'd generally advise using the new routines in <random> instead. This is kind of a pain (seeding the new generators well is particularly painful) but they generally produce much higher quality randomness, are much more friendly to multi-threading, and using them well will keep the cool C++ programmers (now there's an oxymoron) from calling you names.

avoid exit()

When writing C++, it's generally better to avoid using exit. Calling it generally prevents destructors for objects on the stack from running, so you can't get a clean shutdown.

As a general rule, I'd add a try/catch block in main, and where you're currently calling exit(), throw an object derived from std::exception. In your case, std::runtime_error probably make sense.

if (renderer == NULL)
{
    throw std::runtime_error("Renderer could not be created!");
}

In main:

try {
    game.Run();
    std::cout << "Game has terminated successfully, score: " << game.GetScore()
        << ", size: " << game.GetSize() << '\n';
} 
catch (std::exception const &e) { 
    std::cerr << e.what();
}

Prefer nullptr to NULL

Pretty much self-explanatory. In C++, NULL is required to be an integer constant with the value 0 (e.g., either 0 or 0L). nullptr is a bit more special--it can convert to any pointer type, but can't accidentally be converted to an integer type. So, anywhere you might consider using NULL, you're almost certainly better off using nullptr:

if (renderer == nullptr)

Some also prefer to reverse those (giving "Yoda conditions"):

if (nullptr == renderer)

This way, if you accidentally use = where you meant ==:

if (nullptr = renderer)

...the code won't compile, because you've attempted to assign to a constant (whereas if (renderer = nullptr) could compile and do the wrong thing, though most current compilers will at least give a warning about it).

Answer 2

using namespace std; is a bad practice. I'm glad to see you didn't use it in the header. Better still to not to use it at all. Please see this post for more information.

---

int main(int argc, char * argv[])

If you are not going to use the command line arguments anyway then use the empty parameter main: int main()

---

return 0 at the end of main is unnecessary and will be supplied by the compiler.

---

Bug

int main(int argc, char * argv[])
{
    Game game = Game();
    Game().Run();
    cout << "Game has terminated successfully, score: " << game.GetScore()
        << ", size: " << game.GetSize() << endl;
    return 0;
}

Game().Run() calls the Game constructor and creates a second instance of Game and then calls Run() on that instance. Your postgame stats likely aren't working correctly. No?

---

Don't use std::endl. Prefer '\n' instead. std::endl flushes the stream, which if you wanted to do you could do manually << '\n' << std::flush and it would be more explicit what you were trying to accomplish.

Read more here.

---

static const int FRAME_RATE     = 1000 / 60;
static const int SCREEN_WIDTH   = 640;
static const int SCREEN_HEIGHT  = 640;
static const int GRID_WIDTH     = 32;
static const int GRID_HEIGHT    = 32;

constexpr is better for global named constants that are known at compile time. They would need moved outside of the class but could still be in the Game header file.

ALL_CAPS names are also typically used for macros. Better use snake_case, camelCase, or PascalCase. (I prefer snake_case for global constants but that's just me.)

---

struct { float x = GRID_WIDTH / 2, y = GRID_HEIGHT / 2; } pos;

SDL_Point head = { static_cast<int>(pos.x), static_cast<int>(pos.y) };

Here you define a float that is the result of the division of two ints (which won't return a float) and then immediately cast the result to int. It's also mildly worth mentioning that your values divide cleanly (which is why you didn't notice any errors.) I see pos cast to int a few other times. Just make it an int

---

srand(static_cast<unsigned int>(time(0)));

srand is not a very good PRNG. Learn the <random> header.

---

bool running = false;
bool alive = false;

You define and initialize these to false only to make them true before they can be used properly. Just initialize them to true to begin with. Also brace initialization is more idiomatic.

bool running{ true };
bool alive{ true };

---

Use the C++ language supported nullptr over the C macro NULL. NULL will be silently converted to int at unintended times.

---

ReplaceFood() Once again avoid rand. But have you considered maintaining an std::vector<Point> of Points that are empty. Then you can randomly index from the vector to find the location of the next food. You will have to add the previous location of the tail back to the vector and remove the location of the head on each move.

---

Uint32 before, second = SDL_GetTicks(), after;
int frame_time, frames = 0;

Don't declare multiple variable on single lines. Use 1:1 lines to variables. Especially when some are assigned and some are not. This can get confusing to read. is frame_time assigned 0? I know the answer but its not obvious just by looking at it.

---

Don't take a parameter in your GrowBody() function. You only ever grow your snake by one. Just increment the size internally and move on. Only grow by a size provided by a parameter if there is a possibility of different sizes being passed in as arguments.

---

Your Update() function is a bit on the larger side. I would break it into two or three helper functions. Maybe the move + wrap in one function and the checks for the head in another.

Then you'd have

void Game::Update()
{
    MoveWithWrap();
    CheckHead();
}

I also see that you did indeed need floats for the position so I will return to that. I'm not sure I would change the way you do the pos struct after all but I would seriously think about it.

The way to get the result as a float requires casting before the division call:

struct { float x = static_cast<float>(GRID_WIDTH) / 2, y = static_cast<float>(GRID_HEIGHT) / 2; } pos;

The solution could be to have a helper function that abstracts the casts away to one location. Like this:

struct Position
{
    float x;
    float y;
};

struct Point
{
    int x;
    int y;
};

Point PositionToPoint(Position const& position)
{
    Point point{ static_cast<int>(position.x), static_cast<int>(position.y) };
    return point;
}

Position pos{ grid_width / 2, grid_height / 2 };

Answer 3

Some additional points:

• Avoid "God classes". Your Game class does absolutely everything. This makes it hard to see which member variables are used where, and is one step away from using global variables. The larger the program, the harder it is to understand. Classes should follow the Single Responsibility Principle (SRP), and be responsible for only one thing.

• Use the "Resource Acquisition Is Initialization" (RAII) approach for managing resource lifetimes. For example, the SDL context, window, and renderer can be encapsulated in an object, with initialization done in the constructor, and cleanup in the destructor.

• Don't use global constants. GRID_WIDTH etc. apply to all instances of the Game class, which is unnecessarily restrictive.

• Use unsigned types for variables that can never be negative (grid width / height, etc.). SDL_Point uses the wrong type (int), but we can easily define our own Point class to use instead. We can convert to the necessary SDL type only when we need to call an SDL function with it.

• Member functions that don't change member variables (e.g. GetScore(), GetSize()) should be marked const (e.g. int GetScore() const;).

• A better strategy for picking a new food position might be to add the positions of all the empty squares to a vector, and then pick one (by choosing an index less than the size of the vector).

---

Here's a (pseudo-code) example of how we could split up the Game class. Note that there's no reason for the Game class to know anything about SDL. If we ever wanted to change to a different platform for rendering / input, it's much neater to keep it separate. Don't be afraid to use free functions as well.

namespace Snake
{
    template<class T>
    struct Point
    {
        T x;
        T y;
    };

    struct SDLContext
    {
        SDLContext(std::size_t window_width, std::size_t window_height);
        ~SDLContext();

        SDL_Window * window = nullptr;
        SDL_Renderer * renderer = nullptr;
    };

    SDLContext::SDLContext()
    {
        // ... SDL init
    }

    SDLContext::~SDLContext()
    {
        // ... SDL shutdown
    }

    struct Board
    {
        Board(std::size_t width, std::size_t height);

        enum class Block { head, body, food, empty };

        std::size_t width;
        std::size_t height;

        std::vector<std::vector<Block>> grid;
    };

    Board::Board()
    {
        // ... init grid to "empty"
    }

    struct Food
    {
        Point<std::size_t> position = Point{ 0, 0 };
    };

    struct Snake
    {
        void Grow(int amount);
        void UpdatePosition(Board& board);

        enum class Move { up, down, left, right };

        Move last_dir = Move::up;
        Move dir = Move::up;

        Point<std::size_t> headPosition;
        std::vector<Point<std::size_t>> body;

        int size = 1;
        float speed = 0.5f;
        int growing = 0;
    };

    class Game
    {
        Game(std::size_t gridWidth, std::size_t gridHeight);

        int GetScore() const;
        int GetSize() const;

        void Update();

    private:

        void ReplaceFood();

        Board board;
        Food food;
        Snake snake;

        int score = 0;
        bool alive = true;
    };

    Game::Game(std::size_t gridWidth, std::size_t gridHeight):
        Board(gridWidth, gridHeight)
    {
        ReplaceFood();
    }

    void PollEvents(SDLContext&, bool& quit)
    {
        // ...
    }

    void Render(SDLContext&, Game const& game)
    {
        // ...
    }

    void UpdateWindowTitle(SDLContext&, Game const& game)
    {
        // ...
    }

    void Run(SDLContext& context, Game& game, int frame_rate)
    {
        Uint32 before, second = SDL_GetTicks(), after;
        int frame_time, frames = 0;

        while (true)
        {
            before = SDL_GetTicks();

            bool quit = false;
            PollEvents(sdlContext, quit);

            if (quit)
                break;

            game.Update();

            Render(sdlContext, game);

            frames++;
            after = SDL_GetTicks();
            frame_time = after - before;

            if (after - second >= 1000)
            {
                UpdateWindowTitle(sdlContext, game.GetScore(), frames);

                frames = 0;
                second = after;
            }

            if (frame_rate > frame_time)
            {
                SDL_Delay(frame_rate - frame_time);
            }
        }
    }

} // Snake


#include <SDL.h>

#include <iostream>
#include <cstddef>

int main(int argc, char * argv[])
{
    using namespace Snake;

    const std::size_t window_width = 640;
    const std::size_t window_height = 640;
    SDLContext sdlContext(window_width, window_height);

    const std::size_t grid_width = 32;
    const std::size_t grid_height = 32;
    Game game(grid_width, grid_height);

    const int frame_rate = 1000 / 60;
    Run(sdlContext, game, frame_rate);

    std::cout << "Game has terminated successfully, score: " << game.GetScore()
        << ", size: " << game.GetSize() << std::endl;

    return 0;
}

The functions taking the SDLContext& don't necessarily use it, since SDL works through global functions. However, this does prove that we have properly initialized SDL at the point the function is called.