25
\$\begingroup\$

I wrote a simple, 2-player Pong game for a Udacity Nanodegree in C++.

The program has the following class structure (relatively new to UML so any tips on here would be very much welcome too): UML class structure

Towards the end of the project, as I needed to add more features, my Game class started to do many many things (collision detection for board edges, physics for ball deflection, render scenes, keep track of scores etc).

Whilst writing the code, I couldn't see clearly how these things could be encapsulated in their own class and it felt much easier, quicker and less verbose to continue expanding the Game class.

The code works now but I know its OOP design could be improved. Where would you start? Your thoughts are very much appreciated!

EDIT: Going to be updating the code based off of the invaluable advice given below. Link to the repo is here for those interested in the progress.

Here is the source code (from original post):

main.cpp

#include "Game.h"

int main(int argc, char* argv[])
{
    Game game;  
    return game.execute();
}

GameObjects.h

#pragma once
#include <SDL.h>
#include <SDL_image.h>
#include <memory>
#include <iostream>
#include <random>

struct Vect_2D {

    int x;
    int y;
};

struct Circle {

    int x;
    int y;
    int r;
};

template <typename T>
class GameObjects{

protected:

    Vect_2D _velocity;
    int _speed;

    int _curX;
    int _curY;
    SDL_Texture* _texture;  // TODO need to destroy textures // SDL_DestroyTexture(t);
    T _boundingBox;

public:
    // Constructors
    GameObjects() = default;
    GameObjects(GameObjects& other) = delete;
    GameObjects& operator=(GameObjects& other) = delete;
    GameObjects(GameObjects&& other) = delete;
    GameObjects& operator=(GameObjects&& other) = delete;

    // Destructor
    ~GameObjects() { SDL_DestroyTexture(_texture); }

    // Accessors
    Vect_2D velocity() { return _velocity; }
    int curX() { return _curX; }
    int curY() { return _curY; }
    T boundingBox() { return _boundingBox;  }
    SDL_Texture* getTexture() { return _texture;  }

    // Modifiers
    void setTexture(SDL_Texture* t) { _texture = t; }
    void setVelocity(Vect_2D v) { _velocity = v;  }
    void setSpeed(int s) { _speed = s;  }
    void curX(int n);
    void curY(int n);
    
    // Special functions
    virtual void render(SDL_Renderer* renderer) = 0;
    void move();
    void updateBoundingBox();
};

class Ball : public GameObjects<Circle> {

private:
    std::mt19937 _mt;
    std::random_device _rdevice;
public:

    Ball() = default;
    Ball(const int& x, const int& y, const int& r);

    // Functions
    void setRandomVelocity();
    void render(SDL_Renderer* renderer) override;
    
};

class Platform : public GameObjects<SDL_Rect> {

public:

    Platform() = default;
    Platform(const int& x, const int& y, const int& w, const int& h);

    // Functions
    void moveUp();
    void moveDown();
    void stop();
    void render(SDL_Renderer * renderer) override;
};

template <typename T>
void GameObjects<T>::move() {
    _curX += _velocity.x;
    _curY += _velocity.y;
    updateBoundingBox();
}

template <typename T>
void GameObjects<T>::curX(int n) {
    _curX = n;
    updateBoundingBox();
}

template <typename T>
void GameObjects<T>::curY(int n) {
    _curY = n;
    updateBoundingBox();
}

GameObjects.cpp

#include "GameObjects.h"

Platform::Platform(const int& x, const int& y, const int& w, const int& h) {

    _velocity.x = 0;
    _velocity.y = 0;

    setSpeed(7);

    _curX = x;
    _curY = y;

    //_boundingBox = SDL_Rect();
    _boundingBox.x = x;
    _boundingBox.y = y;
    _boundingBox.w = w;
    _boundingBox.h = h;
}

Ball::Ball(const int& x, const int& y, const int& r) {

    setSpeed(8);

    //setRandomVelocity();

    _curX = x;
    _curY = y;

    _boundingBox.r = r;

    // Account for the fact that textures are drawn at top left, 
    // but circle x,y is in centre of circle.
    _boundingBox.x = x + r;
    _boundingBox.y = y + r;
    
}

// Sets a random y velocity going towards left of right. left/right speed remains the same as before.
void Ball::setRandomVelocity() {
    int a = _rdevice();
    std::cout << "random seed = " << a << "\n";
    _mt.seed(a);
    std::uniform_int_distribution<int> dist(0, 1);

    if (dist(_mt) == 1)
        _velocity.x = _speed;
    else
        _velocity.x = -_speed;

    std::uniform_int_distribution<int> dist2(-3, 3);
    _velocity.y = dist2(_mt);

}

void Ball::render(SDL_Renderer* renderer) {
    SDL_SetRenderDrawColor(renderer, 0xFF, 0xFF, 0XFF, 0XFF);

    SDL_Rect newPos = { _curX, _curY, 15, 15 };
    SDL_RenderCopy(renderer, _texture, NULL, &newPos);

    //SDL_RenderPresent(renderer);
}

void Platform::render(SDL_Renderer* renderer) {
    SDL_SetRenderDrawColor(renderer, 0xFF, 0xFF, 0XFF, 0XFF);

    SDL_Rect newPos = { _curX, _curY, 13, 73 };
    SDL_RenderCopy(renderer, _texture, NULL, &newPos);

    //SDL_RenderPresent(renderer);
}

void Platform::moveUp() {
    setVelocity(Vect_2D{ 0, -_speed });
}

void Platform::moveDown() {
    setVelocity(Vect_2D{ 0, _speed });
}

void Platform::stop() {
    setVelocity(Vect_2D{ 0, 0 });
}

template<>
void GameObjects<Circle>::updateBoundingBox() {
    _boundingBox.x = _curX + _boundingBox.r;
    _boundingBox.y = _curY + _boundingBox.r;
}

template<>
void GameObjects<SDL_Rect>::updateBoundingBox() {
    _boundingBox.x = _curX;
    _boundingBox.y = _curY;
}

Timer.h

// Thread safe timer class

#pragma once
#include <chrono>
#include <mutex>
#include <iostream>
#include <future>

using std::chrono::steady_clock;

class Timer
{

private:
    std::mutex mtx;
    std::future<void> _ftr;

    bool _isRunning;
    bool _completed;
    void delay(const std::chrono::milliseconds& ms);

public:
    Timer() : _isRunning(false), _completed(false) {};

    bool isRunning();
    bool isCompleted();
    bool start(const std::chrono::milliseconds& ms);
};

Timer.cpp

#include "Timer.h"

void Timer::delay(const std::chrono::milliseconds& ms) {
    std::unique_lock<std::mutex> lck(mtx);
    _completed = false;

    _isRunning = true;

    lck.unlock();
    auto time_started = steady_clock::now();

    std::this_thread::sleep_for(ms);

    lck.lock();
    _isRunning = false;
    _completed = true;
}

bool Timer::isRunning() {
    std::unique_lock<std::mutex> lck(mtx);
    return _isRunning;
}

bool Timer::isCompleted() {
    std::unique_lock<std::mutex> lck(mtx);
    return _completed;
}

bool Timer::start(const std::chrono::milliseconds& ms) {
    if (isRunning()) {
        return false;
    }
    else {
        _ftr = std::async(&Timer::delay, this, ms);
        return true;
    }
}

CollisionDetection.h

#pragma once
#include <cmath>
#include "GameObjects.h"

class CollisionDetection
{
public:
    static int square_of_distance(int x1, int y1, int x2, int y2);
    static void detectCollision(const Circle& item1, const SDL_Rect& item2, int& collisionX, int& collisionY);

};

CollisionDetection.cpp

#include "CollisionDetection.h"
#include <iostream>

// Checks if circle and rectangle have collided. Returns 2 ints representing where on x and y they collided. Both will be -1, -1 if no collision.
void CollisionDetection::detectCollision(const Circle& circle, const SDL_Rect& rectangle, int& collision_x, int& collision_y) {

    collision_x = -1;
    collision_y = -1;

    int rectCollidePointY = 0;
    int rectCollidePointX = 0;

    // Check where on the y axis the circle is in relation to the rectangle
    if (circle.y > rectangle.y + rectangle.h)  rectCollidePointY = rectangle.y + rectangle.h;   // circle below rectangle
    else if (circle.y < rectangle.y) rectCollidePointY = rectangle.y;                           // circle above rectangle
    else rectCollidePointY = circle.y;                                                          // circle somewhere in the middle of rectangle in y axis

    // Check where on the x axis the circle is in relation to the rectangle
    if (circle.x > rectangle.x + rectangle.w)  rectCollidePointX = rectangle.x + rectangle.w;   // circle to the right of whole rectangle
    else if (circle.x < rectangle.x) rectCollidePointX = rectangle.x;                           // circle to the left of whole rectangle
    else rectCollidePointX = circle.x;                                                          // circle somewhere in the middle of rectangle in x axis

    int d = square_of_distance(circle.x, circle.y, rectCollidePointX, rectCollidePointY);

    if (d < pow(circle.r, 2)) {
        collision_x = rectCollidePointX;
        collision_y = rectCollidePointY;
        return;
    }
}

int CollisionDetection::square_of_distance(int x1, int y1, int x2, int y2) {
    return static_cast<int>(pow(x1 - x2, 2) + pow(y1 - y2, 2));
}

Game.h

#pragma once
#include <SDL.h>
#include <SDL_image.h>
#include <iostream>
#include <vector>
#include <string>
#include <SDL_ttf.h>
#include <chrono>
#include <mutex>
#include <future>
#include <sstream>
#include <iomanip>

#include "GameObjects.h"
#include "CollisionDetection.h"
#include "Timer.h"


enum class GameState {
    kMainMenu,
    kPreStart,
    kStart,
    kScoreScreen,
};

class Game{

private:
    bool _running;
    int _frames;
    uint32_t _timeAtLaunch;

    Timer _threadSafeTimer;

    std::vector<int> _scoresVector;

    GameState _state;
    bool _gameStarted;

    SDL_Window* _mainWindow;
    SDL_Renderer* _renderer;

    const int GAME_WIDTH = 600;
    const int GAME_HEIGHT = 400;

    std::unique_ptr<Ball> _ball;
    std::unique_ptr<Platform> _leftPlatform;
    std::unique_ptr<Platform> _rightPlatform;

    std::vector<TTF_Font*> _fonts;      // global font
    void renderText(SDL_Texture* text_texture, int xpos, int ypos);
    void updateScoreTextTure();

    SDL_Texture* _countdownTimer;
    SDL_Texture* _scoresTexture;
    SDL_Texture* _controlsTexture;

    SDL_Texture* loadTexture(std::string path);
    bool loadMedia();
    SDL_Texture* loadFromRenderedText(std::string textureTex, SDL_Color textColor, TTF_Font* font);

    void checkAndReactToBallCollisions(int& winner);
    void checkAndReactToPlatformCollisions();

    bool init();
    void onEvents(SDL_Event* event);
    void gameLoop();
    void render();
    void cleanUp();
    void start();
    
public:
    Game();
    int execute(); // Launch game

    
};

Game.cpp

#include "Game.h"
#include "CollisionDetection.h"

using std::cout;
using std::endl;


Game::Game() {
    cout << "Game object initialized." << endl;

    _state = GameState::kMainMenu;
    _scoresVector = { 0, 0 };

    _ball = std::make_unique<Ball>(GAME_WIDTH / 2, GAME_HEIGHT / 2, 8);
    _leftPlatform = std::make_unique<Platform>(7, 150, 13, 73);
    _rightPlatform = std::make_unique<Platform>(580, 150, 13, 73);

    _running = true;
    _gameStarted = false;
    _frames = 0;
}

int Game::execute() {
    cout << "Launching game." << endl;

    init();

    SDL_Event e;
    cout << "Starting game..." << endl;
    while (_running) {
        while (SDL_PollEvent(&e)) {
            onEvents(&e);
        }
        gameLoop();
        render();
    }

    cleanUp();
    return 0;
}

bool Game::init() {
    cout << "Initializing game." << "\n" << std::flush;

    // Init SDL
    if (SDL_Init(SDL_INIT_EVERYTHING) < 0) {
        std::cout << "SDL couldn't initialize! SDL_Error: " << SDL_GetError() << "\n";
        return false;
    }

    // Create Window
    if ((_mainWindow = SDL_CreateWindow("Pong by Can", SDL_WINDOWPOS_CENTERED, SDL_WINDOWPOS_CENTERED,
        GAME_WIDTH, GAME_HEIGHT, SDL_WINDOW_SHOWN)) == NULL) {
        return false;
    }

    // Create renderer for window
    _renderer = SDL_CreateRenderer(_mainWindow, -1, SDL_RENDERER_ACCELERATED);
    if (_renderer == NULL) {
        std::cout << "Renderer could not be created.  SDL_Error: " << SDL_GetError() << "\n";
        return false;
    }

    //Initialize PNG loading
    int imgFlags = IMG_INIT_PNG;
    if (!(IMG_Init(imgFlags) & imgFlags))
    {
        std::cout << "SDL_image could not initialize! SDL_image Error: " << IMG_GetError() << "\n";
        return false;
    }

    // Initialize SDL TTF (text render)
    if (TTF_Init() == -1) {
        std::cout << "Failed to initialise SDL_ttf. SDL_ttf error: " << TTF_GetError() << "\n";
    }

    // Load media
    if (loadMedia() == false) {
        return false;
    }

    _timeAtLaunch = SDL_GetTicks();
    
    return true;
}

//  Load textures from image and text
bool Game::loadMedia() {
    
    _ball->setTexture(loadTexture("Resources/ball.png"));
    _leftPlatform->setTexture(loadTexture("Resources/plank.bmp"));
    _rightPlatform->setTexture(loadTexture("Resources/plank2.bmp"));

    _fonts.push_back(TTF_OpenFont("Resources/ARLRDBD.TTF", 28));
    if (_fonts[0] == NULL) {
        std::cout << "Failed to load 28 ARIAL ROUNDED font. SDL_ttf error: " << TTF_GetError() << "\n";
        return false;
    }

    _fonts.push_back(TTF_OpenFont("Resources/ARLRDBD.TTF", 14));
    if (_fonts[1] == NULL) {
        std::cout << "Failed to load 14 ARIAL ROUNDED font. SDL_ttf error: " << TTF_GetError() << "\n";
        return false;
    }

    return true;
}

SDL_Texture* Game::loadFromRenderedText(std::string textureText, SDL_Color textColor, TTF_Font* font) {

    SDL_Texture* newTexture = NULL;

    SDL_Surface* textSurface = TTF_RenderText_Blended(font, textureText.c_str(), textColor);
    if (textSurface == NULL) {
        std::cout << "Unable to render text surface. SDL_ttf error: " << TTF_GetError() << "\n";
    }
    else {
        newTexture = SDL_CreateTextureFromSurface(_renderer, textSurface);
        if (newTexture == NULL) {
            std::cout << "Unable to create texture from rendered text. SDL_ttf error: " << TTF_GetError() << "\n";
        }
        else {
        }
        SDL_FreeSurface(textSurface);
    }
    return newTexture;
}

SDL_Texture* Game::loadTexture(std::string path) {
    SDL_Texture* newTexture = NULL;

    SDL_Surface* loadedSurface = IMG_Load(path.c_str());
    if (loadedSurface == NULL) {
        std::cout << "Unable to load image " << path << ". SDL_image error: " << IMG_GetError() << "\n";
    }
    else {
        newTexture = SDL_CreateTextureFromSurface(_renderer, loadedSurface);
        if (newTexture == NULL) {
            std::cout << "Unable to create texture from " << path << ". SDL Error: " << SDL_GetError() << "\n";
        }
        SDL_FreeSurface(loadedSurface);
    }
    return newTexture;
}

void Game::onEvents(SDL_Event* event) {
    if (event->type == SDL_QUIT) {
        _running = false;
    }
    else if (event->type == SDL_KEYDOWN) {
        switch (event->key.keysym.sym)
        {
        case SDLK_UP:
            _rightPlatform->moveUp();
            break;
        case SDLK_DOWN:
            _rightPlatform->moveDown();
            break;
        case SDLK_w:
            _leftPlatform->moveUp();
            break;
        case SDLK_s:
            _leftPlatform->moveDown();
            break;
        case SDLK_SPACE:
            if (_state == GameState::kScoreScreen || _state == GameState::kMainMenu) _state = GameState::kPreStart;
            break;
        default:
            break;
        }
    }
    else if (event->type == SDL_KEYUP) {
        switch (event->key.keysym.sym)
        {
        case SDLK_UP:
            _rightPlatform->stop();
            break;
        case SDLK_DOWN:
            _rightPlatform->stop();
            break;
        case SDLK_w:
            _leftPlatform->stop();
            break;
        case SDLK_s:
            _leftPlatform->stop();
            break;
        default:
            break;
        }
    }
}

// Helper function to assist with the correct bounce physics of the ball when in contact with the platforms
void bounceBall(int x, int y, Platform* platform, Ball* ball) {
    int platformLeft;
    int platformRight;
    int platformTop;
    int platformBottom;

    int rectCenterX;
    int rectCenterY;

    // Move ball back one step
    int newX = ball->curX() - ball->velocity().x;
    int newY = ball->curY() - ball->velocity().y;
    
    ball->curX(newX);
    ball->curY(newY);

    // Figure out where from the centre point of rectangle the collision occured
    // Reflect ball away at this angle but keep its y velocity the same (only change y velocity if top/bottom of platform was hit)
    //  *       * O
    //  *        *
    //  * centre *
    //  *        *
    //  *        *
    platformLeft = platform->boundingBox().x;
    platformRight = platform->boundingBox().x + platform->boundingBox().w;
    platformTop = platform->boundingBox().y;
    platformBottom = platform->boundingBox().y + platform->boundingBox().h;

    rectCenterX = (platformLeft + platformRight) / 2;
    rectCenterY = (platformBottom + platformTop) / 2;

    int diffX = x - rectCenterX;
    int diffY = y - rectCenterY;
    int y_magnitude = abs(diffY / diffX);

    int y_dir_ball = ball->velocity().y < 0 ? -1 : 1;

    int direction_multiplierY = 1;

    if (y - ball->velocity().y >= platformBottom || y - ball->velocity().y <= platformTop) direction_multiplierY = -1;  // check if bottom or top of platform was hit

    // Calculate new y velocity
    int yVel = y_magnitude * y_dir_ball * direction_multiplierY;

    // Calculate new x velocity
    int xVel = ball->velocity().x * -1;

    ball->setVelocity({ xVel, yVel });
}

void Game::checkAndReactToPlatformCollisions() {
    // Left platform on boundary
    if (_leftPlatform->curY() < 0) {
        _leftPlatform->curY(0);
        _leftPlatform->stop();
    }
    if ((_leftPlatform->curY() + _leftPlatform->boundingBox().h) > (GAME_HEIGHT)) {
        _leftPlatform->curY(GAME_HEIGHT - _leftPlatform->boundingBox().h);
        _leftPlatform->stop();
    }

    // Right platform on boundary
    if (_rightPlatform->curY() < 0) {
        _rightPlatform->curY(0);
        _rightPlatform->stop();
    }
    if ((_rightPlatform->curY() + _rightPlatform->boundingBox().h) > (GAME_HEIGHT)) {
        _rightPlatform->curY(GAME_HEIGHT - _rightPlatform->boundingBox().h);
        _rightPlatform->stop();
    }

}

void Game::checkAndReactToBallCollisions(int& winner) {

    winner = -1;

    // Ball on boundary
    int ballDiameter = 2 * _ball->boundingBox().r;

    //LEFT
    if (_ball->curX() < 0) {                        // PLAYER 2 WINS
        /*_scoresVector[1]++;
        _ball->setVelocity({ 0, 0 });
        _state = GameState::kScoreScreen;
        updateScoreText();*/
        winner = 1;
    }
    //RIGHT
    else if (_ball->curX() > GAME_WIDTH - ballDiameter) { // PLAYER 1 WINS
        /*_scoresVector[0]++;
        _ball->setVelocity({ 0, 0 });
        _state = GameState::kScoreScreen;
        updateScoreText();*/
        winner = 0;
    }
    //TOP
    else if (_ball->curY() < 0) {
        int yVel = _ball->velocity().y;
        int xVel = _ball->velocity().x;

        _ball->setVelocity({ xVel, -yVel });
        _ball->curY(0);
    }
    //BOTTOM
    else if (_ball->curY() > GAME_HEIGHT - ballDiameter) {

        int yVel = _ball->velocity().y;
        int xVel = _ball->velocity().x;

        _ball->setVelocity({ xVel, -yVel });
        _ball->curY(GAME_HEIGHT - ballDiameter);
    }

    // Ball collision on platforms
    int x = -1;
    int y = -1;
    CollisionDetection::detectCollision(_ball->boundingBox(), _leftPlatform->boundingBox(), x, y);
    if (x != -1 && y != -1) {
        bounceBall(x, y, _leftPlatform.get(), _ball.get());
    }

    CollisionDetection::detectCollision(_ball->boundingBox(), _rightPlatform->boundingBox(), x, y);
    if (x != -1 && y != -1) {
        bounceBall(x, y, _rightPlatform.get(), _ball.get());
    }
}

void Game::gameLoop() {

    int winner = -1;

    switch (_state)
    {
        case GameState::kMainMenu:{
            _scoresVector = { 0, 0 };
            _leftPlatform->move();
            _rightPlatform->move();
            checkAndReactToPlatformCollisions();

        }break;
        case GameState::kPreStart:{
            _gameStarted = false;
            _ball->setVelocity({ 0,0 });
            _ball->curX(GAME_WIDTH / 2);
            _ball->curY(GAME_HEIGHT / 2);
            _threadSafeTimer.start(std::chrono::milliseconds(1500));
            _state = GameState::kStart;

        }break;
        case GameState::kStart:{
            if (_threadSafeTimer.isCompleted() == true && _gameStarted == false) {
                _ball->setRandomVelocity();
                _gameStarted = true;
            }

            _leftPlatform->move();
            _rightPlatform->move();
            checkAndReactToPlatformCollisions();
            _ball->move();
            checkAndReactToBallCollisions(winner);

            if (winner != -1) {
                _scoresVector[winner]++;
                _state = GameState::kScoreScreen;
            }
        }break;
        case GameState::kScoreScreen:{
            int a = 1;

        }break;
    }
}

void Game::updateScoreTextTure() {
    
    std::ostringstream oss;
    oss << "Score: " << std::setw(5) << std::right  << _scoresVector[0] << " - " << _scoresVector[1];
    SDL_Color white = { 255,255,255 };

    if (_scoresTexture != NULL) SDL_DestroyTexture(_scoresTexture);
    _scoresTexture = loadFromRenderedText(oss.str().c_str(), white, _fonts[0]);

    if (_scoresTexture == NULL) {
        std::cout << "Failed to change _scoresTexture texture \n";
    }
}


void Game::renderText(SDL_Texture* tt, int xpos, int ypos) {
    
    if (tt == NULL) return;

    int w=140;
    int h=40;

    SDL_QueryTexture(tt, NULL, NULL, &w, &h);

    SDL_Rect newPos = { xpos, ypos , w, h };
    SDL_RenderCopy(_renderer, tt, NULL, &newPos);
}

void print_FPS(uint32_t time_since_start, int frames) {
    int t = SDL_GetTicks();
    float fps = (static_cast<float>(frames)*1000) / (t - time_since_start);
    std::cout << "Avg FPS: " << std::setprecision(2) << fps << "\n";
}

void Game::render() {
    SDL_Color white = { 255,255,255 };
    int t1 = SDL_GetTicks();
    int w = 0;
    int h = 0;

    SDL_RenderClear(_renderer);

    if (_state == GameState::kMainMenu) {
        
        _controlsTexture = loadFromRenderedText("W/S", white, _fonts[1]);
        renderText(_controlsTexture, 20, 20);

        _controlsTexture = loadFromRenderedText("UP/DOWN", white, _fonts[1]);
        w = 0;
        SDL_QueryTexture(_controlsTexture, NULL, NULL, &w, NULL);
        renderText(_controlsTexture, 580-w, 20);

        _countdownTimer = loadFromRenderedText("Press SPACE to Start", white, _fonts[0]);
        SDL_QueryTexture(_countdownTimer, NULL, NULL, &w, NULL);
        renderText(_countdownTimer, GAME_WIDTH / 2 - (w / 2), 350);

        updateScoreTextTure();
    }
    else if (_state == GameState::kScoreScreen) {
        updateScoreTextTure();
        _countdownTimer = loadFromRenderedText("Press SPACE to re-match", white, _fonts[1]);
        SDL_QueryTexture(_countdownTimer, NULL, NULL, &w, NULL);
        renderText(_countdownTimer, GAME_WIDTH / 2 - (w / 2), 350);
    }

    _leftPlatform->render(_renderer);
    _rightPlatform->render(_renderer);
    _ball->render(_renderer);

    // Render scores
    SDL_QueryTexture(_scoresTexture, NULL, NULL, &w, NULL);
    renderText(_scoresTexture, GAME_WIDTH/2 - (w/2), 20);

    SDL_SetRenderDrawColor(_renderer, 0x30, 0x30, 0x30, 0xFF);
    SDL_RenderPresent(_renderer);
    _frames++;

    // Delay to keep FPS consistent
    int t2 = SDL_GetTicks() - t1;
    int ticks_per_frame = 1000 / 60;
    if (t2 < ticks_per_frame) SDL_Delay(ticks_per_frame - t2);
    print_FPS(_timeAtLaunch, _frames);
}

void Game::cleanUp() {
    cout << "End. Cleaning up..." << endl;

    for (auto f : _fonts) {
        TTF_CloseFont(f);
        f = NULL;
    }

    SDL_DestroyRenderer(_renderer);
    SDL_DestroyWindow(_mainWindow);
    //SDL_FreeSurface(_gameSurface);

    _renderer = NULL;
    _mainWindow = NULL;

    IMG_Quit();
    SDL_Quit();
    TTF_Quit();

}
\$\endgroup\$
3
  • \$\begingroup\$ did you tried introduce ECS? \$\endgroup\$
    – Sugar
    Sep 22, 2020 at 12:49
  • \$\begingroup\$ @Sugar I know briefly what ECS is and from that I felt like it would be an overkill for this simple game concept? I could be wrong- I'm happy to investigate in to this. Thanks \$\endgroup\$
    – Eon
    Sep 22, 2020 at 15:53
  • 3
    \$\begingroup\$ "it felt much easier, quicker and less verbose to continue expanding the Game class" this is absolutely true. The problem is that it doesn't scale. Once you get above a certain LoC count it just doesn't work anymore and you spend more time trying to find the part of the code you need to fix than fixing the actual problem. \$\endgroup\$ Sep 23, 2020 at 18:41

1 Answer 1

16
\$\begingroup\$

Overall Observations

If I were a teacher I would give you an A+ for effort and about a B- for implementation.

From a design point of view try to separate the logic of the game as much as possible from the display of the game. Real gaming companies will do this to be able to distribute the same game for multiple platforms. This would also allow the use of the same game core using different graphic packages. While I doubt that Model View Control (MVC) or Model View View Model (MVVM) are the exact design patterns that games are built on, it is the kind of concept you want to use.

When you design object oriented programs you want to try to follow the SOLID design principles. SOLID is a mnemonic acronym for five design principles intended to make software designs more understandable, flexible and maintainable. This will help you design your objects and classes better.

  1. The Single Responsibility Principle - A class should only have a single responsibility, that is, only changes to one part of the software's specification should be able to affect the specification of the class.
  2. The Open–closed Principle - states software entities (classes, modules, functions, etc.) should be open for extension, but closed for modification.
  3. The Liskov Substitution Principle - Objects in a program should be replaceable with instances of their subtypes without altering the correctness of that program.
  4. The Interface segregation principle - states that no client should be forced to depend on methods it does not use.
  5. The Dependency Inversion Principle - is a specific form of decoupling software modules. When following this principle, the conventional dependency relationships established from high-level, policy-setting modules to low-level, dependency modules are reversed, thus rendering high-level modules independent of the low-level module implementation details.

Class Declaration Organization

You will very rarely be the only person working on a project in the industry, one or more people may be implementing the logic of the program and one or more other people may be implementing the display of the program. Over time a programming convention has emerged that public properties and methods should be at the top of the class declaration so that the programmers working with you can easily find them. This is generally the organization within the implementation of the class as well.

Reduce the Includes Within the Header Files

Only include header files that are necessary for compilation to a header file, include the other header files in the C++ source file as necessary for compilation. There are multiple reasons for this, one is that a basic premise of object oriented design is encapsulation which means that the internals of the class are protected. Another reason for reducing the included files within a header file is how include files are implemented in C and C++, the code in the include header is actually copied physically into a temporary version of the C++ source file being compiled. This means that the simple main program below all 7 lines of it will actually contain possibly more than a 1000 lines of code, most of which it doesn't need to compile because in addition to the 60 lines of code Game.h there are 14 includes most of which are non-trivial.

#include "Game.h"

int main(int argc, char* argv[])
{
    Game game;
    return game.execute();
}

Each Class Declaration Should be in Its Own Header File

The file GameObjects.h contains 3 class declarations and multiple struct declarations, there should be 3 header files instead, GameObjects.h that declares the GameObject base class, ball.h that includes GameObjects.h and declares the ball class and platform.h that includes GameObjects.h. The file Game.h should include ball.h and platform.h and not GameObjects.h. It might also be better if you could find a way not to include those headers in Game.h, one way that comes to mind is to use class ball; and class platform; at the top top of the Game.h file, then the compiler knows that those are pointers to a class without knowing the details of the class. The ball.h file and the platform.h file can then be included prior to Game.h in Game.cpp.

I've modified CollisionDetection.h and CollisionDetection.cpp to demonstrate what I mean:

CollisionDetection.h

#pragma once

struct Circle;
struct SDL_Rect;

class CollisionDetection
{
public:
    static int square_of_distance(int x1, int y1, int x2, int y2);
    static void detectCollision(const Circle& item1, const SDL_Rect& item2, int& collisionX, int& collisionY);

};

CollisionDetection.cpp

#include <cmath>
#include "GameObjects.h"
#include "CollisionDetection.h"
#include <iostream>

// Checks if circle and rectangle have collided. Returns 2 ints representing where on x and y they collided. Both will be -1, -1 if no collision.
void CollisionDetection::detectCollision(const Circle& circle, const SDL_Rect& rectangle, int& collision_x, int& collision_y) {

    collision_x = -1;
    collision_y = -1;

    int rectCollidePointY = 0;
    int rectCollidePointX = 0;

    // Check where on the y axis the circle is in relation to the rectangle
    if (circle.y > rectangle.y + rectangle.h)  rectCollidePointY = rectangle.y + rectangle.h;   // circle below rectangle
    else if (circle.y < rectangle.y) rectCollidePointY = rectangle.y;                           // circle above rectangle
    else rectCollidePointY = circle.y;                                                          // circle somewhere in the middle of rectangle in y axis

    // Check where on the x axis the circle is in relation to the rectangle
    if (circle.x > rectangle.x + rectangle.w)  rectCollidePointX = rectangle.x + rectangle.w;   // circle to the right of whole rectangle
    else if (circle.x < rectangle.x) rectCollidePointX = rectangle.x;                           // circle to the left of whole rectangle
    else rectCollidePointX = circle.x;                                                          // circle somewhere in the middle of rectangle in x axis

    int d = square_of_distance(circle.x, circle.y, rectCollidePointX, rectCollidePointY);

    if (d < pow(circle.r, 2)) {
        collision_x = rectCollidePointX;
        collision_y = rectCollidePointY;
        return;
    }
}

int CollisionDetection::square_of_distance(int x1, int y1, int x2, int y2) {
    return static_cast<int>(pow(x1 - x2, 2) + pow(y1 - y2, 2));
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ would you say it is mostly enough for software projects to adhere to the SOLID design principles for them to have a good class structure? I need something I can rely on to write good software every time! \$\endgroup\$
    – Eon
    Sep 22, 2020 at 16:17
  • 1
    \$\begingroup\$ It is a very good start. Also continue on with using UML, when you see a class get too large by itself in UML that's a good indication it's not following the single responsibility principle. A large class can be an aggregate of smaller classes. Try to make your base classes abstract classes if possible, that won't always be necessary. Experience will make you better. \$\endgroup\$
    – pacmaninbw
    Sep 22, 2020 at 16:30

Your Answer

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

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