5
\$\begingroup\$

I've created a simple "Snake" game in C++, to practice using OOP and SDL2 (this is the first time I've used a graphics library) and it works fine.

The problem is, I do more C than C++ and I'm not happy with the object-orientedness (if that's even a word) of my code.

For example, I really hate my implementation of SDL renderer sharing between Game and Snake classes.

Also, the way I handle textures in this game is very ugly. Maybe I should have a separate class for it?

main.cpp:

#include "Game.hpp"

Game * game = nullptr;

int main(int argc, char * argv[]) {
    srand(time(NULL));
    const int frameRate  = 5;
    const int frameDelay = 1000 / frameRate;

    Uint32 frameTime;
    int frameElapsedTime;

    game = new Game();
    game->init("Snake in C++ with SDL2 help", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED, G_SIZE, G_SIZE, false);
    while (game->isRunning()) {
        frameTime = SDL_GetTicks();

        game->handleEvents();
        game->update();
        game->render();

        frameElapsedTime = SDL_GetTicks() - frameTime;
        if (frameDelay > frameElapsedTime)
            SDL_Delay(frameDelay - frameElapsedTime);
    }
    game->clean();
    return 0;
}

Game.hpp:

#ifndef GAME_HPP_INCLUDED
#define GAME_HPP_INCLUDED

#include "global_constants.hpp"
#include <SDL2/SDL.h>
#include <SDL2/SDL_image.h>
#include <iostream>
#include <ctime>
#include <cstdlib>
#include "Snake.hpp"

class Game {

public:
    Game();
    ~Game();
    void init(const char * title, int posx, int posy, int width, int height, bool maximised);
    void handleEvents();
    void update();
    void render();
    void clean();
    bool isRunning() { return running; }

private:
    SDL_Window * window;
    SDL_Renderer * renderer;
    bool running;
    Snake * snake;

};

#endif // GAME_HPP_INCLUDED

Game.cpp:

#include "Game.hpp"

Game::Game() {
    window   = nullptr;
    renderer = nullptr;
    running  = false;
    snake    = nullptr;
}

Game::~Game() {
    delete snake;
}

void Game::init(const char* title, int posx, int posy, int width, int height, bool maximised) {
    if (SDL_Init(SDL_INIT_VIDEO) < 0) {
        std::cout << "SDL2 initialisation failed: " << SDL_GetError() << std::endl;
        return;
    }
    window = SDL_CreateWindow(title, posx, posy, width, height, maximised == true ? SDL_WINDOW_MAXIMIZED : NULL);
    if (window == NULL) {
        std::cout << "Window creation failed: " << SDL_GetError() << std::endl;
        return;
    }
    renderer = SDL_CreateRenderer(window, -1, NULL);
    if (renderer == NULL) {
        std::cout << "Renderer creation failed: " << SDL_GetError() << std::endl;
        return;
    }
    SDL_SetRenderDrawColor(renderer, 170, 170, 170, 255);
    running = true;
    snake = new Snake(GRID / 2, GRID / 2, rand() % 4, renderer);
}

void Game::handleEvents() {
    SDL_Event event;
    while (SDL_PollEvent(&event)) {
        if (event.type == SDL_KEYDOWN) {
            switch (event.key.keysym.sym) {
                case SDLK_UP:
                    snake->setDirection(Snake::NORTH);
                    break;
                case SDLK_DOWN:
                    snake->setDirection(Snake::SOUTH);
                    break;
                case SDLK_LEFT:
                    snake->setDirection(Snake::WEST);
                    break;
                case SDLK_RIGHT:
                    snake->setDirection(Snake::EAST);
                    break;
                case SDLK_RETURN:
                    if (!snake->isAlive()) running = false;
                    break;
                default:
                    break;
            }
        }
        else if (event.type == SDL_QUIT) running = false;
    }
}

void Game::update() {
    if (snake->isAlive()) {
        snake->checkCollision();
        snake->checkAndEatFood();
        snake->move();
    }
}

void Game::render() {
    SDL_RenderClear(renderer);
    snake->render();
    if (!snake->isAlive()) {
        SDL_Rect r;
        r.w = 150;// Image width
        r.h = 105;// Image height
        r.x = (G_SIZE - r.w) / 2;
        r.y = (G_SIZE - r.h) / 2;
        SDL_RenderCopy(renderer, snake->gameOverTexture, NULL, &r);
    }
    SDL_RenderPresent(renderer);
}

void Game::clean() {
    SDL_DestroyWindow(window);
    SDL_DestroyRenderer(renderer);
    SDL_Quit();
}

Snake.hpp:

#ifndef SNAKE_HPP_INCLUDED
#define SNAKE_HPP_INCLUDED

#include <iostream>
#include <vector>
#include <SDL2/SDL.h>
#include <SDL2/SDL_image.h>
#include <cstdlib>
#include "global_constants.hpp"

struct Segment {
    int x;
    int y;
    Segment(int posx, int posy) {
        x = posx;
        y = posy;
    }
};

class Snake {

public:
    Snake(int startx, int starty, char dir, SDL_Renderer * ren);
    ~Snake();
    void addSegment(int x, int y);
    void move();
    SDL_Texture * loadTexture(const char * filename);
    void setDirection(char dir);
    void checkCollision();
    void render();
    bool isAlive() { return alive; }
    void checkAndEatFood();

    SDL_Texture * gameOverTexture;

    static const char NORTH = 0;
    static const char SOUTH = 1;
    static const char EAST  = 2;
    static const char WEST  = 3;

private:
    std::vector<Segment *> body;
    char direction;
    SDL_Texture * headTexture;
    SDL_Texture * bodyTexture;
    SDL_Renderer * renderer;
    SDL_Texture * foodTexture;
    bool alive;
    Segment * food;
};


#endif // SNAKE_HPP_INCLUDED

Snake.cpp:

#include "Snake.hpp"

Snake::Snake(int startx, int starty, char dir, SDL_Renderer* ren) {
    direction       = dir;
    renderer        = ren;
    alive           = true;
    headTexture     = loadTexture("img/head.png");
    bodyTexture     = loadTexture("img/body.png");
    gameOverTexture = loadTexture("img/gameover.png");
    foodTexture     = loadTexture("img/food.png");
    food = new Segment(0, 0);
    do {
        food->x = rand() % GRID;
        food->y = rand() % GRID;
    }
    while (food->x != startx && food->y != starty);
    addSegment(startx, starty);
}

Snake::~Snake() {
    for (int i = 0; i < body.size(); i++)
        delete body[i];
}

void Snake::addSegment(int x, int y) {
    Segment * seg = new Segment(x, y);
    body.push_back(seg);
}

void Snake::move() {
    if (!alive) return;
    int dx, dy;
    switch (direction) {
        case Snake::NORTH:
            dx = 0;
            dy = -1;
            break;
        case Snake::SOUTH:
            dx = 0;
            dy = 1;
            break;
        case Snake::EAST:
            dx = 1;
            dy = 0;
            break;
        case Snake::WEST:
            dx = -1;
            dy = 0;
            break;
        default:
            break;
    }
    for (int i = body.size() - 1; i > 0; i--) {
        body[i]->x = body[i - 1]->x;
        body[i]->y = body[i - 1]->y;
    }
    Segment * snakeHead = *( body.begin() );
    snakeHead->x += dx;
    snakeHead->y += dy;
}

SDL_Texture* Snake::loadTexture(const char* filename) {
    SDL_Surface * tSurface = IMG_Load(filename);
    if (!tSurface)
        std::cout << "Failed to load image " << filename << ": " << IMG_GetError() << std::endl;
    SDL_Texture * texture  = SDL_CreateTextureFromSurface(renderer, tSurface);
    SDL_FreeSurface(tSurface);
    return texture;
}

void Snake::setDirection(char dir) {
    switch (direction) {
        case Snake::NORTH:
            if (dir == Snake::SOUTH) return;
            if (body.size() > 1 && body[0]->y == body[1]->y) return;
            break;
        case Snake::SOUTH:
            if (dir == Snake::NORTH) return;
            if (body.size() > 1 && body[0]->y == body[1]->y) return;
            break;
        case Snake::EAST:
            if (dir == Snake::WEST) return;
            if (body.size() > 1 && body[0]->x == body[1]->x) return;
            break;
        case Snake::WEST:
            if (dir == Snake::EAST) return;
            if (body.size() > 1 && body[0]->x == body[1]->x) return;
            break;
    }
    direction = dir;
}

void Snake::checkCollision() {
    Segment * head = body[0];
    if (head->x < 0 || head->x > GRID - 1 || head->y < 0 || head->y > GRID - 1) {
        alive = false;
        return;
    }
    for (int i = 1; i < body.size(); i++) {
        if (head->x == body[i]->x && head->y == body[i]->y) {
            alive = false;
            break;
        }
    }
}

void Snake::render() {
    SDL_Rect r;
    r.w = r.h = C_SIZE;
    r.x = food->x * C_SIZE;
    r.y = food->y * C_SIZE;
    SDL_RenderCopy(renderer, foodTexture, NULL, &r);

    r.x = body[0]->x * C_SIZE;
    r.y = body[0]->y * C_SIZE;
    SDL_RenderCopyEx(renderer, headTexture, NULL, &r,
        (direction == Snake::WEST || direction == Snake::EAST) ? 90.0 : 0.0,
        NULL, SDL_FLIP_NONE);

    for (int i = 1; i < body.size(); i++) {
        r.x = body[i]->x * C_SIZE;
        r.y = body[i]->y * C_SIZE;
        SDL_RenderCopy(renderer, bodyTexture, NULL, &r);
    }
}

void Snake::checkAndEatFood() {
    if (body[0]->x == food->x && body[0]->y == food->y)
        addSegment( body[ body.size() - 1 ]->x, body[ body.size() - 1 ]->y );
    else return;

    auto collides = [&]() {
        for (auto s : body) {
            if (food->x == s->x && food->y == s->y)
                return true;
        }
        return false;
    };

    do {
        food->x = rand() % GRID;
        food->y = rand() % GRID;
    }
    while (collides());
}

global_constants.hpp:

#ifndef GLOBAL_CONSTANTS_HPP_INCLUDED
#define GLOBAL_CONSTANTS_HPP_INCLUDED

const int G_SIZE = 768;
const int C_SIZE = 32;
const int GRID   = G_SIZE / C_SIZE;

#endif // GLOBAL_CONSTANTS_HPP_INCLUDED
\$\endgroup\$

1 Answer 1

3
\$\begingroup\$

This is pretty cool! Looks like a lot of fun. 🙂 I think your instincts are right about separating out the Snake from the Renderer. Here are some suggestions on that and other improvements.

Constructors & Destructors

First, why is game global? It's not a huge deal since there are no other functions in main.cpp, but it's kind of odd. It could just as easily be a local variable in main().

That point aside, you've separated out the constructor from initializing its data. The entire point of the constructor is to initialize its data. As a general rule, if possible, a constructor should never allow you to construct an invalid object. There are times when this isn't possible, but this isn't one of them. Every value passed in to init() is a constant. You should just make the constructor do what init() does.

Furthermore, your init() method can fail, but returns no error, leaving your application in an invalid state. If you put the code from init() into the constructor, you could have it throw an exception, which you could catch in main() and do something reasonable.

Likewise with the destructor. There's no point in making a caller call clean(). Just put that code in the destructor and let it be handled when the object goes out of scope or the caller deletes it. The way it is now, a caller might forget to call clean() first, and they'll leak a window and a renderer, and probably leave SDL in a bad state. Speaking of which, your code never deletes the game object, so it's leaking a Snake.

Separation of Concerns

It's important to keep "display logic" (such as rendering) separate from "business logic" (in this case the mechanics of the game). Doing so allows you to change one with minimal impact on the other. Your code tightly couples those things together.

I would arrange things differently. I would make an object which keeps track of just the game state. That would be the Snake object, and a separate food object. It wouldn't do any rendering, nor would the Snake or food objects. The Snake object would only hold the positions of the head and body segments, and could be used to test for a collision.

The Game object would update the state of the game based on user input. I would not actually have the Game object check for the events, but be passed events as appropriate.

I would then have a separate Renderer object which was not part of the Game state. I would pass the Game to the renderer and let it render the output, or better yet, just pass it a list of objects to render in the appropriate order.

If you do this, it allows you to change the game without affecting the snake or the renderer. Say you wanted to have 2 snakes (or player v. player). No problem, just pass the renderer the positions of the segments of both snakes. Say you want to change how the snake is rendered. No problem, just update the renderer to use cooler textures. You don't have to update the snake object just because you changed how it renders. And so on.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.