SDL2 Game of Fifteen / Sliding Puzzle

I have made a simple sliding puzzle/game of fifteen using SDL. I originally attempted this to learn about game states, but it turned into a general exercise in code organisation and SDL use. Therefore, I would especially appreciate comments regarding organisation and clarity, but general suggestions or comments are very welcome as well.

I should note that there is a bug that occasionally causes the program to remain open (despite being closed according to XCode and Activity Monitor) until you hard reboot the computer. I'm not sure what causes this bug (please tell me if you do), but I'd advise against running the code on your computer!

Posted below are Game.cpp, Gamestate_MainGame.cpp, Gamestate_Menu.cpp, Graphics.cpp and Tile.h. The full source of this project can be found here

Game.cpp

```
#include
#include
#include "Game.h"
#include "Tile.h"
#include "Gamestate.h"

Game::Game() {
quit = false;
}

bool Game::init(GameState* state) {
graphics.setup();
pushState(state);
return true;
}

void Game::loop()
{
while(quit == false)
{
update();
render();
}
quitGame();
}

void Game::update()
{
while(SDL_PollEvent(&event))
{
if(states.size() > 0){
states.back()->handleEvents(event);
}
if(states.size() > 0){
states.back()->update();
}
}
}

void Game::render()
{
if(states.size() > 0)
states.back()->render();
}

void Game::setQuit() {
quit = true;
}

void Game::toggleCatMode() {
if (catMode == false)
catMode = true;
else if (catMode == true)
catMode = false;
}

void Game::pushState(GameState* state) {
states.push_back(state);
if(state->init(&graphics, this) == false)
quit = true;
}

void Game::popState() {
delete states.back();
states.pop_back();

if(states.size() == 0)
quit = true;
}

void Game::quitGame() {
while(states.size() > 0)
{
delete states.back();
st

Overall it is good quality. I only have these complaints:

MainGame::~MainGame() {
    Mix_Quit();
    game     = NULL;
    graphics = NULL;
    click    = NULL;
    cat1     = NULL;
    cat2     = NULL;
    cat3     = NULL;
}

The indentation of these expressions can be dangerous. People may see the NULL value, but ignore the variable's name.

I use this indentation instead:

MainGame::~MainGame() {
    Mix_Quit();
    game = NULL;
    graphics = NULL;
    click = NULL;
    cat1 = NULL;
    cat2 = NULL;
    cat3 = NULL;
}

Variable's names stand out more now.

Speaking about NULL, C++11 introduces the nullptr. You should use that and not the NULL macro:

MainGame::~MainGame() {
        Mix_Quit();
        game = nullptr;
        graphics = nullptr;
        click = nullptr;
        cat1 = nullptr;
        cat2 = nullptr;
        cat3 = nullptr;
    }

Moving to another point:

bool MainGame::init(Graphics* graph, Game* g) {
    game     = g;
    graphics = graph;
    int shiftamount = 3;

    // Fill vector 'positions' with possible positions of n*n tiles & make shadow positions
    loadPositions(positions, graphics->gridSize());
    loadPositions(shadowPositions, graphics->gridSize(), shiftamount);
    // Assign these starting positions to n*n tiles in vector 'tiles' & make tile shadows
    makeTiles(tiles, positions, Tile::type::button);
    makeTiles(shadowTiles, shadowPositions, Tile::type::shadow);

    // Audio loading
    Mix_OpenAudio(44100, MIX_DEFAULT_FORMAT, 2, 2048);
    click = Mix_LoadWAV("assets/hover.wav");
    if( click == NULL ) {
        std::cerr << "Failed to load beat 'click', error:" << Mix_GetError() << std::endl;
        return false;
    }
    cat1 = Mix_LoadWAV("assets/meow1.wav");
    if( cat1 == NULL ) {
        std::cerr << "Failed to load beat 'meow', error:" << Mix_GetError() << std::endl;
        return false;
    }
    cat2 = Mix_LoadWAV("assets/meow2.wav");
    if( cat2 == NULL ) {
        std::cerr << "Failed to load beat 'meow', error:" << Mix_GetError() << std::endl;
        return false;
    }
    cat3 = Mix_LoadWAV("assets/meow3.wav");
    if( cat3 == NULL ) {
        std::cerr << "Failed to load beat 'meow', error:" << Mix_GetError() << std::endl;
        return false;
    }

    return true;
}

This function is really long and has duplicated code.

Let's start at the beginning: Refactoring the work in sub-functions:

bool MainGame::init(Graphics* graph, Game* g) {
        game     = g;
        graphics = graph;
        int shiftamount = 3;

        loadPositions();
        makeTiles();
        return loadAudio();
    }

    void MainGame::loadPositions()
    {
        loadPositions(positions, graphics->gridSize());
        loadPositions(shadowPositions, graphics->gridSize(), shiftamount);
    }

    void MainGame::makeTiles()
    {
        makeTiles(tiles, positions, Tile::type::button);
        makeTiles(shadowTiles, shadowPositions, Tile::type::shadow);
    }

    bool MainGame::loadAudio() {
        // Audio loading
        Mix_OpenAudio(44100, MIX_DEFAULT_FORMAT, 2, 2048);
        click = Mix_LoadWAV("assets/hover.wav");
        if( click == NULL ) {
            std::cerr << "Failed to load beat 'click', error:" << Mix_GetError() << std::endl;
            return false;
        }
        cat1 = Mix_LoadWAV("assets/meow1.wav");
        if( cat1 == NULL ) {
            std::cerr << "Failed to load beat 'meow', error:" << Mix_GetError() << std::endl;
            return false;
        }
        cat2 = Mix_LoadWAV("assets/meow2.wav");
        if( cat2 == NULL ) {
            std::cerr << "Failed to load beat 'meow', error:" << Mix_GetError() << std::endl;
            return false;
        }
        cat3 = Mix_LoadWAV("assets/meow3.wav");
        if( cat3 == NULL ) {
            std::cerr << "Failed to load beat 'meow', error:" << Mix_GetError() << std::endl;
            return false;
        }

        return true;
    }

loadAudio() has a lot duplicated code. Just write a function and call it:

bool MainGame::loadAudio() {
        // Audio loading
        Mix_OpenAudio(44100, MIX_DEFAULT_FORMAT, 2, 2048);

        if (!loadWav(click,"assets/hover.wav"))
           return false;
        if (!loadWav(cat1,",assets/meow1.wav"))
           return false;
        // And so on...

        return true;
    }

    //  I don't know what handler type is, so i will do a template
    template
    bool loadWav(T& handler,const char* route)
    {
        handler = Mix_LoadWAV("assets/hover.wav");
        if(handler == NULL ) {
           std::cerr << "Failed to load " << route <<, error:" << Mix_GetError() << std::endl;
        return false;
    }
        return true;
    }

You have some similar problems in Graphics and Gamestate_menu.

And in last:

```
void Graphics::setup() {
if (SDL_Init(SDL_INIT_EVERYTHING) < 0)
std::cerr << "Error: init" << SDL_GetError() << std::endl;

else {
window = SDL_Crea

StackExchange Code Review Q#145846, answer score: 3