C++ Game Development – Creating a Simple Snake Game in C++

cdesigndesign-patternsobject-orientedobject-oriented-design

I posted this question originally in Code Review, but then thought that I could possibly get more feedback about the design here.

I just finished writing a simple Snake clone in C++ with the goal of having an actual design this time and to adhere to the SOLID principles of OOP. So I made a class diagram of the game and wrote all the code, and it works fine, but there are some areas where I feel like I could have done better. For example, there are some hard coded elements which I wasn't able to elegantly remove, and there's one place where I use inheritance, but then I also have an enum to figure out which child a message is coming from when I would prefer not to have to care at all what type the child is.

Anyway, here's the class diagram:

class diagram

Engine is a simple engine I wrote to display 2d graphics easily, I only use it for rendering and some helper classes like v2di which is a 2d integer vector for storing positional information


Here's the game class which is responsible for starting and running the game.

OnCreate() is called once on startup,

OnUpdate() and OnRender() are called once per frame and should contain the game loop,

OnDestroy() is called when the inner game loop is exiting and the program is about to quit:

///////////////// .h

class SnakeGame : public rge::DXGraphicsEngine {
public:
    SnakeGame();
    bool OnCreate();
    bool OnUpdate();
    bool OnRender();
    void OnDestroy();

protected:
    Snake snake;
    FieldGrid field;
    int score;
    bool gameOver;
    int updateFreq;
    int updateCounter;
};

//////////////////////// .cpp

SnakeGame::SnakeGame() : DXGraphicsEngine(), field(10, 10), snake(3, rge::v2di(3, 1), Direction::RIGHT), score(0), gameOver(false), updateFreq(10), updateCounter(0){

}

bool SnakeGame::OnCreate() {
    field.AddSnake(snake.GetBody());
    field.GenerateFood();
    return true;
}

bool SnakeGame::OnUpdate() {

    //check user input
    if(GetKey(rge::W).pressed) {
        snake.SetDirection(Direction::UP);
    }
    if(GetKey(rge::S).pressed) {
        snake.SetDirection(Direction::DOWN);
    }
    if(GetKey(rge::A).pressed) {
        snake.SetDirection(Direction::LEFT);
    }
    if(GetKey(rge::D).pressed) {
        snake.SetDirection(Direction::RIGHT);
    }

    updateCounter++;
    if(!gameOver && updateCounter >= updateFreq) {
        updateCounter = 0;
        //clear snake body from field
        field.ClearSnake(snake.GetBody());
        //move
        snake.MoveSnake();
        //add snake body to field
        field.AddSnake(snake.GetBody());
        //testcollision
        CollisionMessage cm = field.CheckCollision(snake.GetHead());
        gameOver = cm.gameOver;
        score += cm.scoreChange ? snake.GetLength() * 10 : 0;
        if(cm.tileType == TileType::Food) {
            field.GenerateFood();
            snake.ExtendSnake();
        }
    }
    return true;
}

bool SnakeGame::OnRender() {
    std::cout << score << std::endl;
    field.Draw(&m_colorBuffer, 100, 20, 10);
    snake.DrawHead(&m_colorBuffer, 100, 20, 10);
    return true;
}

Next up is the Snake class that moves and extends the snake. There's also an enum for the Direction the snake can move in:

///////////// .h

enum class Direction {
    UP, DOWN, LEFT, RIGHT
};

class Snake {
public:
    Snake();
    Snake(int length, rge::v2di position, Direction direction);
    rge::v2di GetHead() { return head; }
    std::vector<rge::v2di> GetBody() { return body; }
    void MoveSnake();
    void ExtendSnake();
    Direction GetDirection() { return direction; }
    void SetDirection(Direction direction);
    int GetLength() { return body.size() + 1; }
    void DrawHead(rge::Buffer* buffer, int x, int y, int size);

protected:
    std::vector<rge::v2di> body;
    rge::v2di head;
    Direction direction;
    Direction oldDirection;
};

////////////// .cpp

Snake::Snake(): head(rge::v2di(0, 0)), direction(Direction::UP), oldDirection(Direction::UP), body(std::vector<rge::v2di>()){
    body.push_back(rge::v2di(head.x, head.y + 1));
}

Snake::Snake(int length, rge::v2di position, Direction direction) : head(position), direction(direction), oldDirection(direction), body(std::vector<rge::v2di>()) {
    for(int i = 0; i < length-1; ++i) {
        rge::v2di bodyTile;
        switch(direction) {
        case Direction::UP:{
            bodyTile.x = head.x;
            bodyTile.y = head.y + (i + 1);
            break;
        }
        case Direction::DOWN:{
            bodyTile.x = head.x;
            bodyTile.y = head.y - (i + 1);
            break;
        }
        case Direction::LEFT: {
            bodyTile.y = head.y;
            bodyTile.x = head.x + (i + 1);
            break;
        }
        case Direction::RIGHT: {
            bodyTile.y = head.y;
            bodyTile.x = head.x - (i + 1);
            break;
        }
        }
        body.push_back(bodyTile);
    }
}

void Snake::MoveSnake() {
    oldDirection = direction;
    for(int i = body.size()-1; i > 0; --i) {
        body[i] = body[i - 1];
    }
    body[0] = head;

    switch(direction) {
    case Direction::UP: {
        head.y--;
        break;
    }
    case Direction::DOWN: {
        head.y++;
        break;
    }
    case Direction::LEFT: {
        head.x--;
        break;
    }
    case Direction::RIGHT: {
        head.x++;
        break;
    }
    }
}

void Snake::ExtendSnake() {
    body.push_back(body[body.size() - 1]);
}

void Snake::SetDirection(Direction direction) {
    switch(this->oldDirection) {
    case Direction::UP:
    case Direction::DOWN: {
        if(direction != Direction::UP && direction != Direction::DOWN) {
            this->direction = direction;
        }
        break;
    }
    case Direction::LEFT:
    case Direction::RIGHT: {
        if(direction != Direction::LEFT && direction != Direction::RIGHT) {
            this->direction = direction;
        }
        break;
    }
    }
}

void Snake::DrawHead(rge::Buffer* buffer, int x, int y, int size) {
    rge::Color c(100, 100, 200);
    buffer->DrawRegion(x + head.x * size, y + head.y * size, x + head.x * size + size, y + head.y * size + size, c.GetHex());
}

Then there's the FieldGrid class responsible for collision detection, food generation and storing the state of the map:

//////////// .h

class FieldGrid {
public:
    FieldGrid();
    FieldGrid(int width, int height);
    ~FieldGrid();
    void GenerateFood();
    CollisionMessage CheckCollision(rge::v2di head);
    void ClearSnake(std::vector<rge::v2di> body);
    void AddSnake(std::vector<rge::v2di> body);
    void Draw(rge::Buffer* buffer, int x, int y, int size);
protected:
    std::vector<std::vector<Tile*>> field;
    int width;
    int height;
};

//////////// .cpp

FieldGrid::FieldGrid() : width(10), height(10), field(std::vector<std::vector<Tile*>>()) {
    for(int i = 0; i < width; ++i) {
        field.push_back(std::vector<Tile*>());
        for(int j = 0; j < height; ++j) {
            field[i].push_back(new EmptyTile());
        }
    }
}

FieldGrid::FieldGrid(int width, int height): width(width), height(height), field(std::vector<std::vector<Tile*>>()) {
    for(int i = 0; i < width; ++i) {
        field.push_back(std::vector<Tile*>());
        for(int j = 0; j < height; ++j) {
            field[i].push_back(new EmptyTile());
        }
    }
}

FieldGrid::~FieldGrid() {
    for(int i = 0; i < field.size(); ++i) {
        for(int j = 0; j < field[i].size(); ++j) {
            delete field[i][j];
        }
        field[i].clear();
    }
    field.clear();
}

void FieldGrid::GenerateFood() {
    int x = rand() % width;
    int y = rand() % height;
    while(!field[x][y]->IsFree()) {
        x = rand() % width;
        y = rand() % height;
    }
    delete field[x][y];
    field[x][y] = new FoodTile();
}

CollisionMessage FieldGrid::CheckCollision(rge::v2di head) {
    if(head.x < 0 || head.x >= width || head.y < 0 || head.y >= height) {
        CollisionMessage cm;
        cm.scoreChange = false;
        cm.gameOver = true;
        return cm;
    }
    return field[head.x][head.y]->OnCollide();
}

void FieldGrid::ClearSnake(std::vector<rge::v2di> body) {
    for(int i = 0; i < body.size(); ++i) {
        delete field[body[i].x][body[i].y];
        field[body[i].x][body[i].y] = new EmptyTile();
    }
}

void FieldGrid::AddSnake(std::vector<rge::v2di> body) {
    for(int i = 0; i < body.size(); ++i) {
        delete field[body[i].x][body[i].y];
        field[body[i].x][body[i].y] = new SnakeTile();
    }
}

void FieldGrid::Draw(rge::Buffer* buffer, int x, int y, int size) {
    for(int xi = 0; xi < width; ++xi) {
        for(int yi = 0; yi < height; ++yi) {
            int xp = x + xi * size;
            int yp = y + yi * size;
            field[xi][yi]->Draw(buffer, xp, yp, size);
        }
    }
}

Tile class used in FieldGrid:

class Tile {
public:
    virtual CollisionMessage OnCollide() = 0;
    virtual bool IsFree() = 0;
    void Draw(rge::Buffer* buffer, int x, int y, int size) {
        buffer->DrawRegion(x, y, x + size, y + size, color.GetHex());
    }

protected:
    rge::Color color;
};

class EmptyTile : public Tile {
public:
    EmptyTile() {
        this->color = rge::Color(50, 50, 50);
    }

    CollisionMessage OnCollide() {
        CollisionMessage cm;
        cm.scoreChange = false;
        cm.gameOver = false;
        cm.tileType = TileType::Empty;
        return cm;
    }

    bool IsFree() { return true; }
};

class FoodTile : public Tile {
public:
    FoodTile() {
        this->color = rge::Color(50, 200, 70);
    }
    CollisionMessage OnCollide() {
        CollisionMessage cm;
        cm.scoreChange = true;
        cm.gameOver = false;
        cm.tileType = TileType::Food;
        return cm;
    }

    bool IsFree() { return false; }
};

class SnakeTile : public Tile {
public:
    SnakeTile() {
        this->color = rge::Color(120, 130, 250);
    }

    CollisionMessage OnCollide() {
        CollisionMessage cm;
        cm.scoreChange = false;
        cm.gameOver = true;
        cm.tileType = TileType::Snake;
        return cm;
    }

    bool IsFree() { return false; }
};

Finally here's the CollisionMessage class used to send messages to the game when the snake head collides with any Tile:

enum class TileType {
    Empty,
    Snake,
    Food
};

class CollisionMessage {
public:
    bool scoreChange;
    bool gameOver;
    TileType tileType;
};

I omitted all the includes and the main method, as they aren't relevant to the design and would just take up extra space.


I appreciate the time you take to read through all my code (or just look at the class diagram) and would really like to hear what you think about the overall design I chose.

Best Answer

You are at the right place for a review of your design. But this is not a code review site, so don't expect an in-depth inspection here.

Overview

What strikes me first looking at the classes is that:

  • there is no relation between the Snake and either FieldGrid or Tile: it feels hard to believe that both could be as independent as this diagram suggests, especially when considering that some Tile represent parts of the Snake.
  • CollisionMessage is associated to the Tile and not to SnakeGame that orchestrates the collaboration between the entities
  • TileType is associated to the message but not tho the tile itself
  • TileType seems redundant with the specialization of Tile into EmptyTile, FoodTile, SnakeTile.

CollisionMessage, TileType, and separation of concerns

The UML model:

  • In your code, no tile is associated with a CollisionMessage (which is in fact more an event than a message). The tiles create the message and returns it to the Game that owns it temporarily and discard when it's handled.

  • So the class diagram should not show an association between Tile and CollisionMessage.

  • It should instead show an association between CollisionMessage and Game, and could show a creation dependency between Tile and CollisionMessage.

The other weak point is a lack of separation concerns:

  • the calss reveals the tileType to the Game, and the game decides what to do based on its (leaked) knowledge of the tile. Shouldn't the game loop coordinate, and let the other classes decide about the details ?

  • Now imagine that you add some more kind of tiles: MountainTile (randomly fatal or not because sometimes rocks fall and kill the snake), SeeTile (some snakes swimm), MongooseTile (always fatal): would you really want the game loop to know what to do in each case ?

Fortunately, there is a solution to improve this design by adding separation of concerns and strengthening the opend/close principle:

  • Let the collision detector (here FieldGrid::CheckCollision() with the support of Tile::OnColide() ) determine the event. For example if the collision is with FoodTile create a FeedingEvent whereas a SnakeTile or MongooseTile would create a FatalEvent. All those even being derived from an abstract CollisionMessage
  • Each kind of Tile having already its own implementation of OnColide(), that'll be super easy: just return the right event for each tile.
  • The easiest implementation would be to change CollisionMessage in away to refer to a CollisionType instead of a TileType. At least, the actions in the game loop would be more logic to read (eg if type is FeedignEvent ..., if type is FatalEvent... etc...)
  • A more elaborate way would be to use polymorphism , and return sepcialized subtypes of CollisionMessage. But polymorphism to work safely on return type in C++ would need the use of shared_ptr. The advatage could be that the game loop could call some virtual functions of the CollisionMessage instead of examining all the case by itself.

Finally, I spotted a nasty bug:

  • Currently you also have a FieldGrid::CheckCollision() that creates a CollisionMessage when the snakes goes out of bounds.
  • Unfortunately you do not set tileType and by the way it's neither snake, nor empty nor food.
  • Furthermore, the constructor does not initialize this member. So its value could be anything: a legal value corresponding to the wrong tile type (which could cause the game loop to draw wrong conclusions) or an illegal value. See also this SO question about enum initialization.

Grids and snakes

About the UML:

  • you should use composition between Snake and v2di and Direction. These classes are used as or in a couple of Snake members, but by value. So it's really an exclusive ownership and lifecycle control.

  • despite my initial question, the model is ok. In your code, you really keep a snake separate from the grid: at each move, you clear the snake tiles in the diagram and you add the snake to change the tiles of the new position. You could perhaps show a usage dependency of the snake by the grid, to make the diagram more expressive.

  • this design limits of course the game to a single snake, since in the grid you do not know to which snake a tile belongs.

Then a code recommendation: forget about raw pointers: it's so easy to make errors in memory management. Let the library care for memory management for you:

So

std::vector<std::vector<Tile*>> field;

could be replaced with

std::vector<std::vector<shared_ptr<Tile>>> field;

and all the

    delete field[body[i].x][body[i].y];
    field[body[i].x][body[i].y] = new EmptyTile();  // or SnakeTile or...

would then be replaced by

    field[body[i].x][body[i].y] = make_shared<EmptyTile>();  // or SnakeTile or...

The shared pointers take care of deallocating tiles which are no longer used. It's much safer.

So, that's all folks ! Sorry for the long answer to the long question. But hope it helps.

Related Topic