6
\$\begingroup\$

In this game the pieces are trains and you move forward on the track (never backwards) in search of your goal. The players move based on a combination of resources (Soap and Water to make Bubbles) that are added each turn with a die roll. Players must choose a direction when faced with a branch. The game has not been fully implemented (no end game, no player interaction, no art) but I am not seeking advice on how to implement this. I am asking for a review now partly because if my architexture needs refactored Id like to do it before adding more features.

I approached this project with 2 goals in mind. The first was to apply Dependency Injection. I am very uncertain what I did qualifies at all as such. If this does not represent good IoC and DI but is a maintainable architexture please let me know. The second goal (and the reason I chose to go with a board game) was to learn how to traverse a 1D array for player movement. This quickly changed to a 2D Matrix when it became apparent that it would better suit the project. I have no frame of reference for how well this was implemented and would love feedback on how I implemented player movement along the matrix by following the current and previous position and by scanning the map.

Additionally I would like to ask if I should apply the Singleton pattern here. I avoided using it because the pattern is generally reviled in most of what I have read on the internet (but I have no real working knowledge base for why) but also because it is a small project maintained only by me (what could go wrong? hah!) However to have multiple instances of the data structs or handlers would probably break things (or would it? Hence why I ask.)

Main.cpp

#include "Expressions.h"
#include "GamePieceWrapper.h"
#include "GUIWrapper.h"
#include "InputHandler.h"
#include "SoundWrapper.h"
#include "SpriteWrapper.h"
#include "TextureWrapper.h"
#include "UpdateHandler.h"
#include "Viewport.h"

int main()
{
    sf::Vector2u resolution(screen_width, screen_height);
    sf::RenderWindow window(sf::VideoMode(resolution.x, resolution.y), "The Last Lost Bubble Shooter");
    window.setFramerateLimit(60);

    TextureWrapper textures;
    SoundWrapper sounds;

    GamePieceWrapper gamePieces(&textures);
    GUIWrapper graphicInterface(&textures);
    SpriteWrapper sprites(&textures);

    InputHandler inputHandler(gamePieces, graphicInterface, sprites, window);
    UpdateHandler updateHandler(gamePieces, graphicInterface, sprites);
    Viewport viewport(gamePieces, graphicInterface, sprites, window);

    while (window.isOpen())
    {
        inputHandler.input();
        updateHandler.update();
        viewport.draw();
    }
}

I am not sure if I am approaching this correctly. What is the best way to wrap your objects before passing them around to the managing classes? Here I've used structs that I feel represent good logical divisions of object classifications. Are there more or less classifications I could have used? Was a struct a good choice for this?

Here too are the structs so you can see them a little more in detail.

SpriteWrapper.h

#pragma once

#include "Expressions.h"
#include "TextureWrapper.h"

#include <SFML\Graphics.hpp>

struct SpriteWrapper
{
    SpriteWrapper(TextureWrapper* textureMaps);

    void update();

    TextureWrapper* textures;
    sf::Sprite background;
    sf::Sprite mainLogo;
    sf::Sprite smallLogo;
    sf::Sprite gameBoard;
    sf::Sprite water;
    sf::Sprite soap;
};

SpriteWrapper.cpp

#include "SpriteWrapper.h"

SpriteWrapper::SpriteWrapper(TextureWrapper* textureMaps) :
    textures(textureMaps),
    background(textures->textureMap),
    mainLogo(textures->textureMap),
    smallLogo(textures->textureMap),
    gameBoard(textures->textureMap),
    water(textures->dieMap),
    soap(textures->dieMap)
{
    background.setPosition(origin_x, origin_y);
    background.setTextureRect(sf::IntRect(0, 0, screen_width, screen_height));

    mainLogo.setPosition(main_logo_x, main_logo_y);
    mainLogo.setTextureRect(sf::IntRect(0, 0, main_logo_width, main_logo_height));

    smallLogo.setPosition(origin_x, origin_y);
    //smallLogo.setTextureRect(sf::IntRect());

    gameBoard.setPosition(origin_x, origin_y);
    gameBoard.setTextureRect(sf::IntRect(0, 0, screen_width, screen_height));

    water.setPosition(water_x, water_y);
    water.setTextureRect(sf::IntRect(0, 192, water_width, water_height));

    soap.setPosition(soap_x, soap_y);
    soap.setTextureRect(sf::IntRect(64, 192, soap_width, soap_height));
}

void SpriteWrapper::update()
{
    // remove if never filled
}

GUIWrapper.h

#pragma once

#include "Expressions.h"
#include "TextureWrapper.h"

#include <SFML\Graphics.hpp>

#include <vector>

struct GUIWrapper
{
    GUIWrapper(TextureWrapper* textureMaps);

    void update();

    TextureWrapper* textures;
    screenSelection currentScreen;
    gameMode currentMode;
    std::vector<sf::RectangleShape> modes;
    std::vector<sf::RectangleShape> players;
    sf::RectangleShape startGame;
    std::vector<sf::RectangleShape> gameMenu;
    sf::RectangleShape endTurnButton;
    sf::RectangleShape popUpBackground;
    sf::RectangleShape popUpCloseButton;
    bool popUpOpen;
};

GUIWrapper.cpp

#include "GUIWrapper.h"

GUIWrapper::GUIWrapper(TextureWrapper* textureMaps) :
    textures(textureMaps),
    currentScreen(screenSelection::main_menu),
    currentMode(gameMode::cooperative),
    popUpOpen(false)
{
    for (std::size_t i = 0; i < num_modes; ++i)
    {
        modes.push_back(sf::RectangleShape());
        modes[i].setSize(sf::Vector2f(mode_button_width, mode_button_height));
        modes[i].setPosition(base_mode_x + (mode_button_width * 2 + mode_button_spacer * 2) * i, base_mode_y);
        modes[i].setTexture(&textures->buttonMap);
        //modes[i].setTextureRect(sf::IntRect());
        modes[i].setFillColor(sf::Color::Magenta);
    }

    for (std::size_t i = 0; i < num_player_buttons; ++i)
    {
        players.push_back(sf::RectangleShape());
        players[i].setSize(sf::Vector2f(player_button_width, player_button_height));
        players[i].setPosition(base_player_x + (player_button_width + player_button_spacer) * i, base_player_y);
        players[i].setTexture(&textures->buttonMap);
        //players[i].setTextureRect(sf::IntRect());
        players[i].setFillColor(sf::Color::Magenta);
    }

    startGame.setSize(sf::Vector2f(play_button_width, play_button_height));
    startGame.setPosition(play_x, play_y);
    startGame.setTexture(&textures->buttonMap);
    //startGame.setTextureRect(sf::IntRect());
    startGame.setFillColor(sf::Color::Magenta);

    for (std::size_t i = 0; i < num_game_buttons; ++i)
    {
        gameMenu.push_back(sf::RectangleShape());
        gameMenu[i].setSize(sf::Vector2f(game_button_width, game_button_height));
        gameMenu[i].setPosition(base_game_x + (game_button_width +game_button_spacer) * i, base_game_y);
        gameMenu[i].setTexture(&textures->buttonMap);
        /*gameMenu[i].setTextureRect(sf::IntRect());*/
        gameMenu[i].setFillColor(sf::Color::Magenta);
    }

    popUpBackground.setSize(sf::Vector2f(pop_up_width, pop_up_height));
    popUpBackground.setPosition(pop_up_x, pop_up_y);
    popUpBackground.setFillColor(sf::Color(0x3d3d3d));

    popUpCloseButton.setSize(sf::Vector2f(pop_button_width, pop_button_height));
    popUpCloseButton.setPosition(pop_button_x, pop_button_y);
    popUpCloseButton.setTexture(&textures->buttonMap);
    //popUpCloseButton.setTextureRect(sf::IntRect());
    popUpCloseButton.setFillColor(sf::Color::Magenta);

    endTurnButton.setSize(sf::Vector2f(end_turn_button_width, end_turn_button_height));
    endTurnButton.setPosition(end_turn_button_x, end_turn_button_y);
    endTurnButton.setTexture(&textures->buttonMap);
    //endTurnButton.setTextureRect(sf::IntRect());
    endTurnButton.setFillColor(sf::Color::Magenta);
}

void GUIWrapper::update()
{
    // remove if never filled
}

The sprite struct is rather small because I have yet to do much of the artwork yet. I am also not happy with the name GUIWrapper. It implies that the GUI aspects in the game wrapper aren't GUI. Is there a name for the parts of a GUI that are not very game-related?

GamePieceWrapper.h

#pragma once

#include "Bubble.h"
#include "Die.h"
#include "Player.h"
#include "TextureWrapper.h"
#include "Tile.h"

#include <SFML\Graphics.hpp>

#include <fstream>
#include <vector>

struct GamePieceWrapper
{
    GamePieceWrapper(TextureWrapper* textureMaps);

    void positionPlayers(const unsigned numOfPlayers);

    void reset();

    void endTurn();

    void update();

    TextureWrapper* textures;
    std::vector<std::vector<int>> gameMap;
    std::vector<Tile> gameSpaces;
    std::vector<Tile*> adjacentSpaces;
    std::vector<Player> players;
    std::vector<Player*> reachablePlayers;
    Player* activePlayer;
    std::vector<Die> dice;
    std::vector<Bubble> bubblePool;
    std::vector<Bubble*> activeBubbles;
    sf::RectangleShape dicePad;
    sf::RectangleShape displayPad;
    numPlayers playerCount;
    rollState turnState;
    rollState battleState;
    unsigned value;
    std::vector<unsigned> reachableX;
    std::vector<unsigned> reachableY;
    std::vector<unsigned> chosenX;
    std::vector<unsigned> chosenY;
    bool isTunnelOpen;

private:
    std::ofstream debuggStream;

    void setMap();

    void setBoard();

    void evaluateTurn();

    void scanPlayers();

    void checkPath();

    void checkDice();

    void findAdjacents();

    void move();

    void checkLanding();
};

GamePieceWrapper.cpp

 #include "GamePieceWrapper.h"


GamePieceWrapper::GamePieceWrapper(TextureWrapper* textureMaps) :
    textures(textureMaps),
    activePlayer(nullptr),
    playerCount(numPlayers::two),
    turnState(rollState::unrolled),
    battleState(rollState::unrolled),
    value(0),
    isTunnelOpen(false),
    debuggStream("debugLog.txt")
{
    setMap();
    setBoard();

    dicePad.setSize(sf::Vector2f(dice_pad_width, dice_pad_height));
    dicePad.setPosition(dice_pad_x, dice_pad_y);
    dicePad.setFillColor(sf::Color(90, 90, 90, 255));

    displayPad.setSize(sf::Vector2f(display_pad_width, display_pad_height));
    displayPad.setPosition(display_pad_x, display_pad_y);
    displayPad.setFillColor(sf::Color(90, 90, 90, 255));

    for (std::size_t i = 0; i < num_dice; ++i)
    {
        dice.push_back(Die());
        dice[i].setTexture(&textures->dieMap);
    }

    dice[0].setPosition(die_x, die_y);
    dice[1].setPosition(die_x + die_spacer, die_y);
    dice[2].setPosition(battle_die_x, battle_die_y);
    dice[3].setPosition(battle_die_x + die_width + battle_die_spacer, battle_die_y);

    reset();

    for (std::size_t i = 0; i < num_bubbles; ++i)
    {
        bubblePool.push_back(Bubble());
        // set textures
    }
}

void GamePieceWrapper::positionPlayers(const unsigned numOfPlayers)
{
    players.clear();
    for (std::size_t i = 0; i < numOfPlayers; ++i)
    {
        players.push_back(Player(i));
    }
    activePlayer = &players[0];
    if (numOfPlayers == 2)
    {
        playerCount = numPlayers::two;
    }
    else if (numOfPlayers == 3)
    {
        playerCount = numPlayers::three;
    }
    else if (numOfPlayers == 4)
    {
        playerCount = numPlayers::four;
    }
}

void GamePieceWrapper::reset()
{
    if (playerCount == numPlayers::two)
    {
        positionPlayers(2);
    }
    else if (playerCount == numPlayers::three)
    {
        positionPlayers(3);
    }
    else if (playerCount == numPlayers::four)
    {
        positionPlayers(4);
    }
}

void GamePieceWrapper::endTurn()
{
    unsigned newIndex;
    if (activePlayer->m_index > players.size() - 1)
    {
        newIndex = activePlayer->m_index + 1;
    }
    else
    {
        newIndex = 0;
    }
    activePlayer = &players[newIndex];
    turnState = rollState::unrolled;
    battleState = rollState::unrolled;
}

void GamePieceWrapper::update()
{
    for (std::size_t i = 0; i < dice.size(); ++i)
    {
        dice[i].update();
    }

    evaluateTurn();

    for (std::size_t i = 0; i < players.size(); ++i)
    {
        players[i].update();
    }

    for (std::size_t i = 0; i < activeBubbles.size(); ++i)
    {
        activeBubbles[i]->update();
    }
}

void GamePieceWrapper::setMap()
{
    gameMap = {
        { 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 3 },
        { 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 1, 0, 1 },
        { 2, 1, 1, 1, 1, 1, 4, 1, 1, 1, 1, 1, 1, 0, 0, 1, 0, 1, 1, 6 },
        { 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 1, 0, 0, 0, 1 },
        { 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 6, 1, 1, 6, 1, 1, 1, 6 },
        { 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0, 1 },
        { 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 1, 1, 6 },
        { 1, 0, 0, 0, 0, 0, 1, 0, 3, 1, 1, 0, 6, 1, 1, 3, 0, 1, 0, 1 },
        { 1, 0, 0, 0, 0, 0, 1, 0, 1, 0, 6, 1, 6, 0, 0, 1, 0, 1, 0, 1 },
        { 1, 1, 1, 1, 1, 1, 1, 0, 1, 1, 1, 0, 3, 1, 1, 1, 0, 1, 3, 1 }
    };

    //0 = no tile
    //1 = normal tile
    //2 = start tile
    //3 = artifact / treasure tile
    //4 = tunnel tile
    //5 = card tile
    //6 = switch tile
}

void GamePieceWrapper::setBoard()
{
    std::size_t k = 0;
    for (std::size_t i = 0; i < num_tile_rows; ++i)
    {
        for (std::size_t j = 0; j < num_tile_collumns; ++j)
        {
            gameSpaces.push_back(Tile(gameMap[i][j], j, i));
            gameSpaces[k].setPosition(tile_base_x + tile_spacer * j, tile_base_y + tile_spacer * i);
            // set texture
            ++k;
        }
    }
}

void GamePieceWrapper::evaluateTurn()
{
    scanPlayers();
    checkPath();
    checkDice();
    move();
    checkLanding();
    // resolve landing events
}

void GamePieceWrapper::scanPlayers()
{
    reachablePlayers.clear();
    for (std::size_t i = 0; i < players.size(); ++i)
    {
        if (activePlayer != &players[i])
        {
            if (activePlayer->m_currentX == players[i].m_currentX
                && activePlayer->m_currentY == players[i].m_currentY)
            {
                reachablePlayers.push_back(&players[i]);
            }
            else if (activePlayer->m_currentX > 0
                && activePlayer->m_currentX - 1 == players[i].m_currentX
                && activePlayer->m_currentY == players[i].m_currentY)
            {
                reachablePlayers.push_back(&players[i]);
            }
            else if (activePlayer->m_currentX < 19
                && activePlayer->m_currentX + 1 == players[i].m_currentX
                && activePlayer->m_currentY == players[i].m_currentY)
            {
                reachablePlayers.push_back(&players[i]);
            }
            else if (activePlayer->m_currentY > 0
                && activePlayer->m_currentY - 1 == players[i].m_currentY
                && activePlayer->m_currentX == players[i].m_currentX)
            {
                reachablePlayers.push_back(&players[i]);
            }
            else if (activePlayer->m_currentY < 9
                && activePlayer->m_currentY + 1 == players[i].m_currentY
                && activePlayer->m_currentX == players[i].m_currentX)
            {
                reachablePlayers.push_back(&players[i]);
            }
        }
    }
}

void GamePieceWrapper::checkPath()
{
    reachableX.clear();
    reachableY.clear();
    if (activePlayer->m_currentX > 0
        && activePlayer->m_currentX - 1 != activePlayer->m_previousX
        && gameMap[activePlayer->m_currentY][activePlayer->m_currentX - 1] > 0)
    {
        reachableX.push_back(activePlayer->m_currentX - 1);
    }
    if (activePlayer->m_currentX < 19
        && activePlayer->m_currentX + 1 != activePlayer->m_previousX
        && gameMap[activePlayer->m_currentY][activePlayer->m_currentX + 1] > 0)
    {
        reachableX.push_back(activePlayer->m_currentX + 1);
    }
    if (activePlayer->m_currentY > 0
        && activePlayer->m_currentY - 1 != activePlayer->m_previousY
        && gameMap[activePlayer->m_currentY - 1][activePlayer->m_currentX] > 0)
    {
        reachableY.push_back(activePlayer->m_currentY - 1);
    }
    if (activePlayer->m_currentY < 9
        && activePlayer->m_currentY + 1 != activePlayer->m_previousY
        && gameMap[activePlayer->m_currentY + 1][activePlayer->m_currentX] > 0)
    {
        reachableY.push_back(activePlayer->m_currentY + 1);
    }

    findAdjacents();
}

void GamePieceWrapper::findAdjacents()
{
    adjacentSpaces.clear();
    if (reachableX.size() + reachableY.size() > 1)
    {
        for (std::size_t i = 0; i < gameSpaces.size(); ++i)
        {
            for (std::size_t j = 0; j < reachableX.size(); ++j)
            {
                if (gameSpaces[i].m_x == reachableX[j]
                    && gameSpaces[i].m_y == activePlayer->m_currentY)
                {
                    adjacentSpaces.push_back(&gameSpaces[i]);
                }
            }
            for (std::size_t j = 0; j < reachableY.size(); ++j)
            {
                if (gameSpaces[i].m_y == reachableY[j]
                    && gameSpaces[i].m_x == activePlayer->m_currentX)
                {
                    adjacentSpaces.push_back(&gameSpaces[i]);
                }
            }
        }
    }
}

void GamePieceWrapper::checkDice()
{
    if (dice[0].m_stopped && dice[1].m_stopped)
    {
        activePlayer->m_water = dice[0].m_face;
        dice[0].m_stopped = false;
        activePlayer->m_soap = dice[1].m_face;
        dice[1].m_stopped = false;
        turnState = rollState::moving;
    }

    if (dice[2].m_stopped && dice[3].m_stopped)
    {
        if (dice[2].m_face > dice[3].m_face)
        {
            value = dice[2].m_face > dice[3].m_face;
        }
        battleState = rollState::moving;
    }
}

void GamePieceWrapper::move()
{
    if (activePlayer->m_move > 0
        && chosenX.size() > 0
        && chosenY.size() > 0)
    {
        debuggStream << "but do I ever get here" << activePlayer->m_move << "\n";
        activePlayer->m_previousX = activePlayer->m_currentX;
        activePlayer->m_previousY = activePlayer->m_currentY;
        activePlayer->m_currentX = chosenX[0];
        activePlayer->m_currentY = chosenY[0];
        activePlayer->move();
        chosenX.clear();
        chosenY.clear();
    }
    else if (activePlayer->m_move > 0
        && reachableX.size() + reachableY.size() == 1)
    {
        if (reachableX.size() > reachableY.size())
        {
            activePlayer->m_previousX = activePlayer->m_currentX;
            activePlayer->m_previousY = activePlayer->m_currentY;
            activePlayer->m_currentX = reachableX[0];
            activePlayer->move();
        }
        else
        {
            activePlayer->m_previousX = activePlayer->m_currentX;
            activePlayer->m_previousY = activePlayer->m_currentY;
            activePlayer->m_currentY = reachableY[0];
            activePlayer->move();
        }
    }
}

void GamePieceWrapper::checkLanding()
{
    if (turnState == rollState::moving
        && activePlayer->m_move == 0
        && (activePlayer->m_soap == 0 || activePlayer->m_water == 0))
    {
        // determine landing events
        turnState = rollState::moved;
    }
}

I am additionally adding the Input Handler as well as the Player class for further context.

InputHandler.cpp

#include "InputHandler.h"

InputHandler::InputHandler(GamePieceWrapper & gamePieces, GUIWrapper & graphicInterface, SpriteWrapper & sprites, sf::RenderWindow & window) :
    m_gamePieces(gamePieces),
    m_graphicInterface(graphicInterface),
    m_sprites(sprites),
    m_window(window)
{
    // do nothing
}

void InputHandler::input()
{
    if (sf::Keyboard::isKeyPressed(sf::Keyboard::Escape))
    {
        m_window.close();
    }
    sf::Event event;
    while (m_window.pollEvent(event))
    {
        if (event.type == sf::Event::Closed)
        {
            m_window.close();
        }

        if (m_graphicInterface.popUpOpen && event.type == sf::Event::MouseButtonPressed)
        {
            if (m_graphicInterface.popUpCloseButton.getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
            {
                m_graphicInterface.popUpOpen = false;
            }
        }
        else if (event.type == sf::Event::MouseButtonPressed)
        {
            if (m_graphicInterface.currentScreen == screenSelection::main_menu)
            {
                if (m_graphicInterface.modes[0].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                {
                    m_graphicInterface.currentMode = gameMode::cooperative;
                    //m_graphicInterface.modes[0].setTextureRect(sf::IntRect());
                    //m_graphicInterface.modes[1].setTextureRect(sf::IntRect());
                }
                if (m_graphicInterface.modes[1].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                {
                    m_graphicInterface.currentMode = gameMode::competetive;
                    //m_graphicInterface.modes[0].setTextureRect(sf::IntRect());
                    //m_graphicInterface.modes[1].setTextureRect(sf::IntRect());
                }
                if (m_graphicInterface.playerButtons[0].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                {
                    m_gamePieces.positionPlayers(2);
                    //m_graphicInterface.playerButtons[0].setTextureRect(sf::IntRect());
                    //m_graphicInterface.playerButtons[1].setTextureRect(sf::IntRect());
                    //m_graphicInterface.playerButtons[2].setTextureRect(sf::IntRect());
                }
                if (m_graphicInterface.playerButtons[1].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                {
                    m_gamePieces.positionPlayers(3);
                    //m_graphicInterface.playerButtons[0].setTextureRect(sf::IntRect());
                    //m_graphicInterface.playerButtons[1].setTextureRect(sf::IntRect());
                    //m_graphicInterface.playerButtons[2].setTextureRect(sf::IntRect());
                }
                if (m_graphicInterface.playerButtons[2].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                {
                    m_gamePieces.positionPlayers(4);
                    //m_graphicInterface.playerButtons[0].setTextureRect(sf::IntRect());
                    //m_graphicInterface.playerButtons[1].setTextureRect(sf::IntRect());
                    //m_graphicInterface.playerButtons[2].setTextureRect(sf::IntRect());
                }
                if (m_graphicInterface.startGame.getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                {
                    m_graphicInterface.currentScreen = screenSelection::game_screen;
                }
            }
            else if (m_graphicInterface.currentScreen == screenSelection::game_screen)
            {
                if (m_graphicInterface.gameMenu[0].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                {
                    m_gamePieces.reset();
                    m_graphicInterface.currentScreen = screenSelection::main_menu;
                }
                if (m_graphicInterface.gameMenu[1].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                {
                    m_graphicInterface.popUpOpen = true;
                    // rules
                }
                if (m_graphicInterface.gameMenu[2].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                {
                    m_graphicInterface.popUpOpen = true;
                    // credits
                }

                for (std::size_t i = 0; i < m_gamePieces.reachablePlayers.size(); ++i)
                {
                    if (m_graphicInterface.currentMode == gameMode::competetive
                        && m_gamePieces.turnState != rollState::rolling
                        && m_gamePieces.turnState != rollState::moving
                        && m_gamePieces.reachablePlayers[i]->getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                    {
                        m_gamePieces.battleState = rollState::rolling;
                        m_gamePieces.dice[2].roll();
                        m_gamePieces.dice[3].roll();
                    }
                    else if (m_graphicInterface.currentMode == gameMode::cooperative
                        && m_gamePieces.turnState != rollState::rolling
                        && m_gamePieces.turnState != rollState::moving
                        && m_gamePieces.reachablePlayers[i]->getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                    {
                        // allow trade
                    }
                }
                for (std::size_t i = 0; i < m_gamePieces.adjacentSpaces.size(); ++i)
                {
                    if (m_gamePieces.adjacentSpaces[i]->getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                    {
                        m_gamePieces.chosenX.push_back(m_gamePieces.adjacentSpaces[i]->getCoordinates().x);
                        m_gamePieces.chosenY.push_back(m_gamePieces.adjacentSpaces[i]->getCoordinates().y);
                    }
                }

                if (m_gamePieces.turnState == rollState::unrolled)
                {
                    if (m_gamePieces.dicePad.getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                    {
                        m_gamePieces.turnState = rollState::rolling;
                        m_gamePieces.dice[0].roll();
                        m_gamePieces.dice[1].roll();
                    }
                }
                if (m_gamePieces.turnState == rollState::moved)
                {
                    if (m_graphicInterface.endTurnButton.getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
                    {
                        m_gamePieces.endTurn();
                    }
                }
            }
        }
    }
}

Player.cpp

#include "Player.h"

Player::Player(unsigned index) :
    m_index(index),
    m_water(0),
    m_soap(0),
    m_move(0),
    m_animationFrameCount(0),
    m_currentX(0),
    m_currentY(9),
    m_previousX(0),
    m_previousY(0)
{
    setSize(sf::Vector2f(player_width, player_height));
    setFillColor(sf::Color::Black);
    setPosition(player_base_x + player_spacer * m_currentX, player_base_y + player_spacer * m_currentY);
}

void Player::move()
{
    --m_move;
    setPosition(player_base_x + player_spacer * m_currentX, player_base_y + player_spacer * m_currentY);
}

void Player::update()
{
    if (m_water > 0 && m_soap > 0 && m_move == 0)
    {
        if (m_animationFrameCount == 0)
        {
            --m_water;
            --m_soap;
            ++m_move;
            animate();
        }

        --m_animationFrameCount;
    }
}

void Player::animate()
{
    m_animationFrameCount = single_animation_step;
}
\$\endgroup\$
  • 1
    \$\begingroup\$ I don't quite follow the design of your wrappers. Why do you pass things by pointer instead of by reference? Maybe you should introduce states into your program and the various states can then only own the resources they need? \$\endgroup\$ – yuri Apr 14 '18 at 11:32
  • \$\begingroup\$ the only one i pass by pointer is the textures. Id have to look at sfml documentation again to remember why. I like the idea of breaking it up by state. Thank you. \$\endgroup\$ – bruglesco Apr 14 '18 at 12:24
  • \$\begingroup\$ @yuri unless you mean the pointers I use within the struct. The active player pointer is sometimes nullptr so I'm gonna keep that one but the others are unnecessary. \$\endgroup\$ – bruglesco Apr 14 '18 at 13:04
3
\$\begingroup\$
GUIWrapper::GUIWrapper(TextureWrapper* textureMaps) :
    textures(textureMaps),
    currentScreen(screenSelection::main_menu),
    currentMode(gameMode::cooperative),
    popUpOpen(false)
{
    for (std::size_t i = 0; i < num_modes; ++i)
    {
        modes.push_back(sf::RectangleShape());
        modes[i].setSize(sf::Vector2f(mode_button_width, mode_button_height));
        modes[i].setPosition(base_mode_x + (mode_button_width * 2 + mode_button_spacer * 2) * i, base_mode_y);
        modes[i].setTexture(&textures->buttonMap);
        //modes[i].setTextureRect(sf::IntRect());
        modes[i].setFillColor(sf::Color::Magenta);
    }

    for (std::size_t i = 0; i < num_player_buttons; ++i)
    {
        players.push_back(sf::RectangleShape());
        players[i].setSize(sf::Vector2f(player_button_width, player_button_height));
        players[i].setPosition(base_player_x + (player_button_width + player_button_spacer) * i, base_player_y);
        players[i].setTexture(&textures->buttonMap);
        //players[i].setTextureRect(sf::IntRect());
        players[i].setFillColor(sf::Color::Magenta);
    }

    startGame.setSize(sf::Vector2f(play_button_width, play_button_height));
    startGame.setPosition(play_x, play_y);
    startGame.setTexture(&textures->buttonMap);
    //startGame.setTextureRect(sf::IntRect());
    startGame.setFillColor(sf::Color::Magenta);

    for (std::size_t i = 0; i < num_game_buttons; ++i)
    {
        gameMenu.push_back(sf::RectangleShape());
        gameMenu[i].setSize(sf::Vector2f(game_button_width, game_button_height));
        gameMenu[i].setPosition(base_game_x + (game_button_width +game_button_spacer) * i, base_game_y);
        gameMenu[i].setTexture(&textures->buttonMap);
        /*gameMenu[i].setTextureRect(sf::IntRect());*/
        gameMenu[i].setFillColor(sf::Color::Magenta);
    }

    popUpBackground.setSize(sf::Vector2f(pop_up_width, pop_up_height));
    popUpBackground.setPosition(pop_up_x, pop_up_y);
    popUpBackground.setFillColor(sf::Color(0x3d3d3d));

    popUpCloseButton.setSize(sf::Vector2f(pop_button_width, pop_button_height));
    popUpCloseButton.setPosition(pop_button_x, pop_button_y);
    popUpCloseButton.setTexture(&textures->buttonMap);
    //popUpCloseButton.setTextureRect(sf::IntRect());
    popUpCloseButton.setFillColor(sf::Color::Magenta);

    endTurnButton.setSize(sf::Vector2f(end_turn_button_width, end_turn_button_height));
    endTurnButton.setPosition(end_turn_button_x, end_turn_button_y);
    endTurnButton.setTexture(&textures->buttonMap);
    //endTurnButton.setTextureRect(sf::IntRect());
    endTurnButton.setFillColor(sf::Color::Magenta);
}

This is a ton of duplicated code.

Even the commented-out lines match.

I'd recommend that you make a separate function for setting the size, position, texture and fill color, in an attempt to get the initializer of GUIWrapper to a higher level of abstraction.

Similarily, InputHandler could use a higher level of abstraction as well - when handling mousebuttonpressed, you're traversing so many objects:

        if (m_graphicInterface.currentScreen == screenSelection::main_menu)
        {
            if (m_graphicInterface.modes[0].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
            {
                m_graphicInterface.currentMode = gameMode::cooperative;
                //m_graphicInterface.modes[0].setTextureRect(sf::IntRect());
                //m_graphicInterface.modes[1].setTextureRect(sf::IntRect());
            }
            if (m_graphicInterface.modes[1].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
            {
                m_graphicInterface.currentMode = gameMode::competetive;
                //m_graphicInterface.modes[0].setTextureRect(sf::IntRect());
                //m_graphicInterface.modes[1].setTextureRect(sf::IntRect());
            }
            if (m_graphicInterface.playerButtons[0].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
            {
                m_gamePieces.positionPlayers(2);
                //m_graphicInterface.playerButtons[0].setTextureRect(sf::IntRect());
                //m_graphicInterface.playerButtons[1].setTextureRect(sf::IntRect());
                //m_graphicInterface.playerButtons[2].setTextureRect(sf::IntRect());
            }
            if (m_graphicInterface.playerButtons[1].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
            {
                m_gamePieces.positionPlayers(3);
                //m_graphicInterface.playerButtons[0].setTextureRect(sf::IntRect());
                //m_graphicInterface.playerButtons[1].setTextureRect(sf::IntRect());
                //m_graphicInterface.playerButtons[2].setTextureRect(sf::IntRect());
            }
            if (m_graphicInterface.playerButtons[2].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
            {
                m_gamePieces.positionPlayers(4);
                //m_graphicInterface.playerButtons[0].setTextureRect(sf::IntRect());
                //m_graphicInterface.playerButtons[1].setTextureRect(sf::IntRect());
                //m_graphicInterface.playerButtons[2].setTextureRect(sf::IntRect());
            }
            if (m_graphicInterface.startGame.getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
            {
                m_graphicInterface.currentScreen = screenSelection::game_screen;
            }
        }
        else if (m_graphicInterface.currentScreen == screenSelection::game_screen)
        {
            if (m_graphicInterface.gameMenu[0].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
            {
                m_gamePieces.reset();
                m_graphicInterface.currentScreen = screenSelection::main_menu;
            }
            if (m_graphicInterface.gameMenu[1].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
            {
                m_graphicInterface.popUpOpen = true;
                // rules
            }
            if (m_graphicInterface.gameMenu[2].getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y))
            {
                m_graphicInterface.popUpOpen = true;
                // credits
            }

Every instance of "getGlobalBounds().contains(event.mouseButton.x, event.mouseButton.y)" is duplication.

\$\endgroup\$
  • \$\begingroup\$ What is a reasonable way to address the repitition inherent to GUI? Each object needs to behave differently. Should one then abstract the elements into a class and have a method like activate() that wraps up the functionality? There will still be a repetive call to object.activate() or click() or whatever. \$\endgroup\$ – bruglesco May 28 '18 at 13:20
  • \$\begingroup\$ @bruglesco just deduplicate your code as you see it. I see a lot of getGlobalBounds, I put this in a separate thing. Eventually I have my own language and my methods read like recipes instead of code. \$\endgroup\$ – Pimgd May 28 '18 at 14:08

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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