5
\$\begingroup\$

This is the 2nd half of my Memory game. The first half can be found here. The repository can be found on Github.

I am using SFML 2.4.2. In this project I focused on a few key aspects of design and development.

  • Top-down Design
  • DRY & SRP
  • Encapsulation & Abstraction

In an effort to eliminate magic numbers I shoved all of my constexpr as well as my enums, a static logger and a static PRNG in a single header file. I don't know if this approach is best practice or not and would appreciate input. On one hand I could move the expressions to the appropriate headers where they are localized, but many of them are used repeatedly throughout the codebase. Should I localize the values that are used once and only keep a header file for the reused items (PRNG, Enums, etc...)?

note: in the previous answer it was recommended to me to use double over float. I chose not to do this as SFML takes floats for its parameters and the number of casts made the code unreadable. I could let the computer narrow implicitly but then I would need to turn of wX which I prefer to keep on. It would also lead to a significant number of warnings.

Expressions.h

#ifndef BRUGLESCO_MEMORY_EXPRESSIONS_H
#define BRUGLESCO_MEMORY_EXPRESSIONS_H

#include <fstream>
#include <random>

namespace bruglesco {

    static std::random_device rd;
    static std::mt19937 generator(rd());
    static std::ofstream debuggStream("DebugLog.txt"); // remove for release

    constexpr unsigned screen_width = 1500u;
    constexpr unsigned screen_height = 800u;

    constexpr float menu_button_width = 256.f;
    constexpr float menu_button_height = 256.f;

    constexpr float play_button_x = 622.f;
    constexpr float play_button_y = 96.f;
    constexpr float six_button_x = 95.f;
    constexpr float eight_button_x = 446.f;
    constexpr float twelve_button_x = 797.f;
    constexpr float sixteen_button_x = 1148.f;
    constexpr float pair_button_y = 448.f;

    constexpr float play_string_x_offset = 18.f;
    constexpr float play_string_y_offset = 28.f;
    constexpr float six_string_x_offset = 80.f;
    constexpr float eight_string_x_offset = 40.f;
    constexpr float twelve_string_x_offset = 15.f;
    constexpr float sixteen_string_x_offset = 8.f;
    constexpr float pair_string_x_offset = 40.f;
    constexpr float pair_string_y_offset = 100.f;

    constexpr float game_button_width = 128.f;
    constexpr float game_button_height = 64.f;

    constexpr float pause_x = 1244.f;
    constexpr float reset_x = 1372.f;
    constexpr float pause_offset = 12.f;
    constexpr float reset_offset = 5.f;

    constexpr float display_x = 1244.f;
    constexpr float player_one_display_y = 64.f;
    constexpr float player_two_display_y = 432.f;
    constexpr float display_offset = 20.f;

    constexpr float display_width = 256.f;
    constexpr float display_height = 368.f;

    constexpr float canvas_width = screen_width - display_width;
    constexpr float canvas_height = static_cast<float>(screen_height);

    constexpr float player_string_x = 1265.f;
    constexpr float player_one_string_y = 72.f;
    constexpr float player_two_string_y = 440.f;

    constexpr float player_card_x = 1265.f;
    constexpr float player_card_x_offset = 15.f;
    constexpr float player_one_card_y = 120.f;
    constexpr float player_two_card_y = 488.f;

    constexpr float card_width = 128.f;
    constexpr float card_height = 128.f;

    constexpr unsigned max_pairs = 16u;

    constexpr float win_width = screen_width - 300.f;
    constexpr float win_height = screen_height - 150.f;
    constexpr float win_x = 150.f;
    constexpr float win_y = 75.f;
    constexpr float winstring_x = 400.f;
    constexpr float winstring_y = 100.f;
    constexpr float player_winstring_x = winstring_x - 50.f;
    constexpr float player_winstring_y = winstring_y + 200.f;

    enum class DeckSize
    {
        six = 6,
        eight = 8,
        twelve = 12,
        sixteen = 16
    };

    enum class CardState
    {
        unmatched,
        checking,
        matched
    };

    enum class menuMouseIn
    {
        none,
        play,
        six,
        eight,
        twelve,
        sixteen
    };

    enum class gameMouseIn
    {
        none,
        pause,
        reset
    };

    enum class winState
    {
        none,
        draw,
        playerOne,
        playerTwo
    };
}

#endif // !BRUGLESCO_MEMORY_EXPRESSIONS_H

My main is intended to be small. Memory is the class that represents the entirety of the application.

Main.cpp

#include "Memory.h"

int main()
{
    Memory memory;
    memory.run();
}

The Memory class contains the ModelData and Viewport class. These classes represent the model logic and presentation logic respectively.

Memory.h

#ifndef BRUGLESCO_MEMORY_MEMORY_H
#define BRUGLESCO_MEMORY_MEMORY_H

#include "Expressions.h"
#include "ModelData.h"
#include "Viewport.h"

#include <SFML\Graphics.hpp>

class Memory
{
public:
    void run();
private:
    ModelData data;
    Viewport view{ data };
    bool playing{ true };

    void input();

    void update();

    void draw();
};

#endif // !BRUGLESCO_MEMORY_MEMORY_H

The ModelData class was the main focus of the last post. In this one I'd like to focus on the Vieport

This class contains the window for rendering, as well as the MenuScreen and GameScreen classes. These are the only two screens in the program.

First the MenuScreen. It is a simple arrangement of buttons that are highlighted dynamically both on selection and mouseover. The trackMouse method is to check for mouseover, the input method responds to user input (in this case clicks), and the update method dynamically maintains your selected game size and highlights the buttons according to that selection and mouseover.

MenuScreen.h

#ifndef BRUGLESCO_MEMORY_MENUSCREEN_H
#define BRUGLESCO_MEMORY_MENUSCREEN_H

#include "Expressions.h"
#include "GameScreen.h"
#include "ModelData.h"

#include <SFML\Graphics.hpp>
class MenuScreen
{
public:
    MenuScreen(ModelData& data, GameScreen& game, sf::Font& font);

    void trackMouse(const sf::Vector2f& mousePos);

    void input(const sf::Vector2f& mousePos);

    void update();

    void draw(sf::RenderWindow& window);
private:
    ModelData& data;
    GameScreen& game;
    sf::Font& font;
    sf::RectangleShape playButton{ sf::Vector2f(bruglesco::menu_button_width, bruglesco::menu_button_height) };
    sf::Text playString{ "Play", font };
    sf::RectangleShape sixPairs{ sf::Vector2f(bruglesco::menu_button_width, bruglesco::menu_button_height) };
    sf::Text sixString{ "Six", font };
    sf::RectangleShape eightPairs{ sf::Vector2f(bruglesco::menu_button_width, bruglesco::menu_button_height) };
    sf::Text eightString{ "Eight", font };
    sf::RectangleShape twelvePairs{ sf::Vector2f(bruglesco::menu_button_width, bruglesco::menu_button_height) };
    sf::Text twelveString{ "Twelve", font };
    sf::RectangleShape sixteenPairs{ sf::Vector2f(bruglesco::menu_button_width, bruglesco::menu_button_height) };
    sf::Text sixteenString{ "Sixteen", font };
    std::vector<sf::Text> pairString{ 4, { "Pairs", font } };
    bruglesco::menuMouseIn mouseIn{ bruglesco::menuMouseIn::none };
    bruglesco::DeckSize deckSize{ bruglesco::DeckSize::six };

    void highlightButton();
};
#endif // !BRUGLESCO_MEMORY_MENUSCREEN_H

MenuScreen.cpp

#include "MenuScreen.h"

MenuScreen::MenuScreen(ModelData& data, GameScreen& game, sf::Font& font) :
    data{ data },
    game{ game },
    font{ font }
{
    playButton.setPosition(bruglesco::play_button_x, bruglesco::play_button_y);
    playButton.setFillColor(sf::Color(120, 120, 120, 255));
    playString.setPosition(bruglesco::play_button_x + bruglesco::play_string_x_offset, bruglesco::play_button_y + bruglesco::play_string_y_offset);
    playString.setCharacterSize(150);
    playString.setFillColor(sf::Color::Black);

    sixPairs.setPosition(bruglesco::six_button_x, bruglesco::pair_button_y);
    sixPairs.setFillColor(sf::Color(120, 120, 120, 255));
    sixString.setPosition(bruglesco::six_button_x + bruglesco::six_string_x_offset, bruglesco::pair_button_y);
    sixString.setCharacterSize(100);
    sixString.setFillColor(sf::Color::Red);
    pairString[0].setPosition(bruglesco::six_button_x + bruglesco::pair_string_x_offset, bruglesco::pair_button_y + bruglesco::pair_string_y_offset);
    pairString[0].setCharacterSize(100);
    pairString[0].setFillColor(sf::Color::Red);

    eightPairs.setPosition(bruglesco::eight_button_x, bruglesco::pair_button_y);
    eightPairs.setFillColor(sf::Color(120, 120, 120, 255));
    eightString.setPosition(bruglesco::eight_button_x + bruglesco::eight_string_x_offset, bruglesco::pair_button_y);
    eightString.setCharacterSize(100);
    eightString.setFillColor(sf::Color::Black);
    pairString[1].setPosition(bruglesco::eight_button_x + bruglesco::pair_string_x_offset, bruglesco::pair_button_y + bruglesco::pair_string_y_offset);
    pairString[1].setCharacterSize(100);
    pairString[1].setFillColor(sf::Color::Black);

    twelvePairs.setPosition(bruglesco::twelve_button_x, bruglesco::pair_button_y);
    twelvePairs.setFillColor(sf::Color(120, 120, 120, 255));
    twelveString.setPosition(bruglesco::twelve_button_x + bruglesco::twelve_string_x_offset, bruglesco::pair_button_y);
    twelveString.setCharacterSize(100);
    twelveString.setFillColor(sf::Color::Black);
    pairString[2].setPosition(bruglesco::twelve_button_x + bruglesco::pair_string_x_offset, bruglesco::pair_button_y + bruglesco::pair_string_y_offset);
    pairString[2].setCharacterSize(100);
    pairString[2].setFillColor(sf::Color::Black);

    sixteenPairs.setPosition(bruglesco::sixteen_button_x, bruglesco::pair_button_y);
    sixteenPairs.setFillColor(sf::Color(120, 120, 120, 255));
    sixteenString.setPosition(bruglesco::sixteen_button_x + bruglesco::sixteen_string_x_offset, bruglesco::pair_button_y);
    sixteenString.setCharacterSize(100);
    sixteenString.setFillColor(sf::Color::Black);
    pairString[3].setPosition(bruglesco::sixteen_button_x + bruglesco::pair_string_x_offset, bruglesco::pair_button_y + bruglesco::pair_string_y_offset);
    pairString[3].setCharacterSize(100);
    pairString[3].setFillColor(sf::Color::Black);
}

void MenuScreen::trackMouse(const sf::Vector2f& mousePos)
{
    if (playButton.getGlobalBounds().contains(mousePos))
    {
        mouseIn = bruglesco::menuMouseIn::play;
    }
    else if (sixPairs.getGlobalBounds().contains(mousePos))
    {
        mouseIn = bruglesco::menuMouseIn::six;
    }
    else if (eightPairs.getGlobalBounds().contains(mousePos))
    {
        mouseIn = bruglesco::menuMouseIn::eight;
    }
    else if (twelvePairs.getGlobalBounds().contains(mousePos))
    {
        mouseIn = bruglesco::menuMouseIn::twelve;
    }
    else if (sixteenPairs.getGlobalBounds().contains(mousePos))
    {
        mouseIn = bruglesco::menuMouseIn::sixteen;
    }
    else 
    {
        mouseIn = bruglesco::menuMouseIn::none;
    }
}

void MenuScreen::input(const sf::Vector2f& mousePos)
{
    if (playButton.getGlobalBounds().contains(mousePos))
    {
        data.setSize(deckSize);
        data.play();
        game.setGame();
        mouseIn = bruglesco::menuMouseIn::none;
    }
    else if (sixPairs.getGlobalBounds().contains(mousePos))
    {
        deckSize = bruglesco::DeckSize::six;
        data.setSize(deckSize);
    }
    else if (eightPairs.getGlobalBounds().contains(mousePos))
    {
        deckSize = bruglesco::DeckSize::eight;
        data.setSize(deckSize);
    }
    else if (twelvePairs.getGlobalBounds().contains(mousePos))
    {
        deckSize = bruglesco::DeckSize::twelve;
        data.setSize(deckSize);
    }
    else if (sixteenPairs.getGlobalBounds().contains(mousePos))
    {
        deckSize = bruglesco::DeckSize::sixteen;
        data.setSize(deckSize);
    }
}

void MenuScreen::update()
{
    highlightButton();
}

void MenuScreen::draw(sf::RenderWindow& window)
{
    window.draw(playButton);
    window.draw(playString);
    window.draw(sixPairs);
    window.draw(sixString);
    window.draw(eightPairs);
    window.draw(eightString);
    window.draw(twelvePairs);
    window.draw(twelveString);
    window.draw(sixteenPairs);
    window.draw(sixteenString);
    for (std::vector<sf::Text>::iterator string = pairString.begin(); string != pairString.end(); ++string)
    {
        window.draw(*string);
    }
}

void MenuScreen::highlightButton()
{
    if (mouseIn == bruglesco::menuMouseIn::play)
    {
        playButton.setFillColor(sf::Color(60, 60, 60, 255));
        playString.setFillColor(sf::Color::Red);
    }
    else
    {
        playButton.setFillColor(sf::Color(120, 120, 120, 255));
        playString.setFillColor(sf::Color::Black);
    }

    if (mouseIn == bruglesco::menuMouseIn::six)
    {
        sixPairs.setFillColor(sf::Color(60, 60, 60, 255));
        sixString.setFillColor(sf::Color::Red);
        pairString[0].setFillColor(sf::Color::Red);
    }
    else if (deckSize == bruglesco::DeckSize::six)
    {
        sixPairs.setFillColor(sf::Color(120, 120, 120, 255));
        sixString.setFillColor(sf::Color::Red);
        pairString[0].setFillColor(sf::Color::Red);
    }
    else
    {
        sixPairs.setFillColor(sf::Color(120, 120, 120, 255));
        sixString.setFillColor(sf::Color::Black);
        pairString[0].setFillColor(sf::Color::Black);
    }

    if (mouseIn == bruglesco::menuMouseIn::eight)
    {
        eightPairs.setFillColor(sf::Color(60, 60, 60, 255));
        eightString.setFillColor(sf::Color::Red);
        pairString[1].setFillColor(sf::Color::Red);
    }
    else if (deckSize == bruglesco::DeckSize::eight)
    {
        eightPairs.setFillColor(sf::Color(120, 120, 120, 255));
        eightString.setFillColor(sf::Color::Red);
        pairString[1].setFillColor(sf::Color::Red);
    }
    else
    {
        eightPairs.setFillColor(sf::Color(120, 120, 120, 255));
        eightString.setFillColor(sf::Color::Black);
        pairString[1].setFillColor(sf::Color::Black);
    }

    if (mouseIn == bruglesco::menuMouseIn::twelve)
    {
        twelvePairs.setFillColor(sf::Color(60, 60, 60, 255));
        twelveString.setFillColor(sf::Color::Red);
        pairString[2].setFillColor(sf::Color::Red);
    }
    else if (deckSize == bruglesco::DeckSize::twelve)
    {
        twelvePairs.setFillColor(sf::Color(120, 120, 120, 255));
        twelveString.setFillColor(sf::Color::Red);
        pairString[2].setFillColor(sf::Color::Red);
    }
    else
    {
        twelvePairs.setFillColor(sf::Color(120, 120, 120, 255));
        twelveString.setFillColor(sf::Color::Black);
        pairString[2].setFillColor(sf::Color::Black);
    }

    if (mouseIn == bruglesco::menuMouseIn::sixteen)
    {
        sixteenPairs.setFillColor(sf::Color(60, 60, 60, 255));
        sixteenString.setFillColor(sf::Color::Red);
        pairString[3].setFillColor(sf::Color::Red);
    }
    else if (deckSize == bruglesco::DeckSize::sixteen)
    {
        sixteenPairs.setFillColor(sf::Color(120, 120, 120, 255));
        sixteenString.setFillColor(sf::Color::Red);
        pairString[3].setFillColor(sf::Color::Red);
    }
    else
    {
        sixteenPairs.setFillColor(sf::Color(120, 120, 120, 255));
        sixteenString.setFillColor(sf::Color::Black);
        pairString[3].setFillColor(sf::Color::Black);
    }
}

Next is the GameScreen class. Here is the meat of the presentation logic. The HUDisplay class is a subsection that updates the players score as strings to display them. It also maintains a space to draw matched cards in a stack in the players "hand".

HUDisplay.h

#ifndef BRUGLESCO_MEMORY_HUDISPLAY_H
#define BRUGLESCO_MEMORY_HUDISPLAY_H

#include "Expressions.h"
#include "ModelData.h"

#include <SFML\Graphics.hpp>

#include <string>

class HUDisplay
{
public:
    HUDisplay(ModelData& data, sf::Font& font);

    void update();

    void draw(sf::RenderWindow& window);
private:
    ModelData& data;
    sf::Font& font;
    sf::RectangleShape playerOneDisplay{ sf::Vector2f(bruglesco::display_width, bruglesco::display_height) };
    sf::RectangleShape playerTwoDisplay{ sf::Vector2f(bruglesco::display_width, bruglesco::display_height) };
    unsigned playerOneScore{ 0 };
    sf::Text playerOne{ "Player One: " + std::to_string(playerOneScore), font };
    unsigned playerTwoScore{ 0 };
    sf::Text playerTwo{ "Player Two: " + std::to_string(playerTwoScore), font };
};

#endif // !BRUGLESCO_MEMORY_HUDISPLAY_H

HUDisplay.cpp

#include "HUDisplay.h"

HUDisplay::HUDisplay(ModelData& data, sf::Font& font) :
    data{ data },
    font{ font }
{
    playerOneDisplay.setPosition(bruglesco::display_x, bruglesco::player_one_display_y);
    playerOneDisplay.setFillColor(sf::Color(120, 120, 120, 255));
    playerTwoDisplay.setPosition(bruglesco::display_x, bruglesco::player_two_display_y);
    playerTwoDisplay.setFillColor(sf::Color(120, 120, 120, 255));
    playerOne.setPosition(bruglesco::display_x + bruglesco::display_offset, bruglesco::player_one_display_y);
    playerOne.setCharacterSize(50);
    playerOne.setFillColor(sf::Color::Red);
    playerTwo.setPosition(bruglesco::display_x + bruglesco::display_offset, bruglesco::player_two_display_y);
    playerTwo.setCharacterSize(50);
    playerTwo.setFillColor(sf::Color::Black);
}

void HUDisplay::update()
{
    if (data.playerOneTurn())
    {
        playerOneScore = data.getPlayer().getScore();
        playerOne.setString("Player One: " + std::to_string(playerOneScore));
        playerOne.setFillColor(sf::Color::Red);
        playerTwo.setFillColor(sf::Color::Black);
    }
    else
    {
        playerTwoScore = data.getPlayer().getScore();
        playerTwo.setString("Player Two: " + std::to_string(playerTwoScore));
        playerOne.setFillColor(sf::Color::Black);
        playerTwo.setFillColor(sf::Color::Red);
    }
}

void HUDisplay::draw(sf::RenderWindow& window)
{
    window.draw(playerOneDisplay);
    window.draw(playerOne);
    window.draw(playerTwoDisplay);
    window.draw(playerTwo);
}

The trackMouse method here works the same as in the Menu. The input method is extended to registering clicks on card images. The logic is then passed over to the Model class which is maintained as a visitor to the View and passed in as a dependency. The positionCards function is able to position the cards dynamically based on the size of deck the player chose on the menu screen. The makeCards function creates the number of display objects based on deck size. The matchFailDelay is an attempt at keeping the images displayed for a short period of time after a failed match. endFailDelay updates the Model so the display can update accordingly. moveMatched sets the position of matched cards according to the player who made the match and how many they've made. adjustTexture is done this way to consistently vary the images used and to swap back and forth between face up and face down.

GameScreen.h

#ifndef BRUGLESCO_MEMORY_GAMESCREEN_H
#define BRUGLESCO_MEMORY_GAMESCREEN_H

#include "Card.h"
#include "Expressions.h"
#include "HUDisplay.h"
#include "ModelData.h"

#include <SFML\Graphics.hpp>

#include <vector>

class GameScreen
{
public:
    GameScreen(ModelData& data, sf::Font& font);

    void setGame();

    void trackMouse(const sf::Vector2f& mousePos);

    void input(const sf::Vector2f& mousePos);

    void update();

    void draw(sf::RenderWindow& window);
private:
    ModelData& data;
    sf::Font& font;
    sf::Texture cardMap;
    sf::RectangleShape pauseButton{ sf::Vector2f(bruglesco::game_button_width, bruglesco::game_button_height) };
    sf::Text pauseString{ "Pause", font };
    sf::RectangleShape returnToMain{ sf::Vector2f(bruglesco::game_button_width, bruglesco::game_button_height) };
    sf::Text resetString{ "Return", font };
    HUDisplay hud{ data, font };
    sf::RectangleShape pauseForeground{ sf::Vector2f(static_cast<float>(bruglesco::screen_width), static_cast<float>(bruglesco::screen_height)) };
    sf::RectangleShape endGameForeground{ sf::Vector2f(bruglesco::win_width, bruglesco::win_height) };
    sf::Text winString{ "Winner", font };
    sf::Text playerWinString{ "", font };
    std::vector<sf::RectangleShape> deck;
    bruglesco::gameMouseIn mouseIn{ bruglesco::gameMouseIn::none };
    bool paused{ false };
    std::vector<unsigned> imageIdentifiers;
    unsigned delay{ 0 };

    void makeCards();

    void positionCards();

    void highlightButtons();

    void matchFailDelay();

    void endFailDelay();

    void moveMatched();

    void adjustTexture();
};

#endif // !BRUGLESCO_MEMORY_GAMESCREEN_H

GameScreen.cpp

#include "GameScreen.h"

GameScreen::GameScreen(ModelData& data, sf::Font& font) :
    data{ data },
    font{ font }
{
    if (!cardMap.loadFromFile("spritesheet.png"))
    {
        bruglesco::debuggStream << "you aren't loading your textures"; // remove for release
    }
    pauseButton.setPosition(bruglesco::pause_x, 0);
    pauseButton.setFillColor(sf::Color(120, 120, 120, 255));
    pauseString.setPosition(bruglesco::pause_x + bruglesco::pause_offset, 0);
    pauseString.setCharacterSize(50);
    pauseString.setFillColor(sf::Color::Black);
    returnToMain.setPosition(bruglesco::reset_x, 0);
    returnToMain.setFillColor(sf::Color(120, 120, 120, 255));
    resetString.setPosition(bruglesco::reset_x + bruglesco::reset_offset, 0);
    resetString.setCharacterSize(50);
    resetString.setFillColor(sf::Color::Black);
    pauseForeground.setFillColor(sf::Color(120, 120, 120, 120));
    endGameForeground.setPosition(bruglesco::win_x, bruglesco::win_y);
    endGameForeground.setFillColor(sf::Color(90, 90, 90, 180));
    winString.setPosition(bruglesco::winstring_x, bruglesco::winstring_y);
    winString.setCharacterSize(250);
    winString.setFillColor(sf::Color::Green);
    playerWinString.setPosition(bruglesco::player_winstring_x, bruglesco::player_winstring_y);
    playerWinString.setCharacterSize(250);
    playerWinString.setFillColor(sf::Color::Green);
    for (unsigned i = 0; i < bruglesco::max_pairs; ++i)
    {
        imageIdentifiers.push_back(i);
    }
}

void GameScreen::setGame()
{
    std::shuffle(std::begin(imageIdentifiers), std::end(imageIdentifiers), bruglesco::generator);
    makeCards();
    positionCards();
}

void GameScreen::trackMouse(const sf::Vector2f& mousePos)
{
    if (pauseButton.getGlobalBounds().contains(mousePos))
    {
        mouseIn = bruglesco::gameMouseIn::pause;
    }
    else if (returnToMain.getGlobalBounds().contains(mousePos))
    {
        mouseIn = bruglesco::gameMouseIn::reset;
    }
    else 
    {
        mouseIn = bruglesco::gameMouseIn::none;
    }
}

void GameScreen::input(const sf::Vector2f& mousePos)
{
    if (pauseButton.getGlobalBounds().contains(mousePos))
    {
        if (!data.isOver())
        {
            paused = !paused;
        }
    }
    else if (!paused && returnToMain.getGlobalBounds().contains(mousePos))
    {
        data.quit();
    }
    else if (!paused && delay == 0)
    {
        for (std::pair<std::vector<Card>::iterator, std::vector<sf::RectangleShape>::iterator> card(data.getDeck().begin(), deck.begin());
            card.first != data.getDeck().end() && card.second != deck.end(); ++card.first, ++card.second)
        {
            if (card.second->getGlobalBounds().contains(mousePos))
            {
                card.first->flip();
            }
        }
    }
}

void GameScreen::update()
{
    highlightButtons();
    matchFailDelay();
    moveMatched();
    adjustTexture();
    hud.update();

    if (delay == 1)
    {
        endFailDelay();
    }
    if (delay > 0)
    {
        --delay;
    }

    if (data.isOver())
    {
        playerWinString.setString("Player " + std::to_string(data.getPlayer().getIdentity()));
    }
}

void GameScreen::draw(sf::RenderWindow& window)
{
    window.draw(pauseButton);
    window.draw(pauseString);
    window.draw(returnToMain);
    window.draw(resetString);
    hud.draw(window);
    for (std::vector<sf::RectangleShape>::iterator card = deck.begin(); card != deck.end(); ++card)
    {
        window.draw(*card);
    }

    if (paused)
    {
        window.draw(pauseForeground);
    }

    if (data.isOver())
    {
        window.draw(endGameForeground);
        window.draw(winString);
        window.draw(playerWinString);
    }
}

void GameScreen::makeCards()
{
    deck.clear();
    unsigned k = 0;
    for (std::vector<Card>::iterator card = data.getDeck().begin(); card != data.getDeck().end(); ++card)
    {
        deck.push_back(sf::RectangleShape(sf::Vector2f(bruglesco::card_width, bruglesco::card_height)));
        deck[k].setTexture(&cardMap);
        deck[k].setTextureRect(sf::IntRect(2048, 0, static_cast<int>(bruglesco::card_width), static_cast<int>(bruglesco::card_height)));
        ++k;
    }
}

void GameScreen::positionCards()
{
    unsigned rows = 0;
    for (unsigned i = 2; i * i <= deck.size(); ++i)
    {
        if (deck.size() % i == 0)
        {
            rows = i;
        }
    }
    unsigned columns = deck.size() / rows;

    float padding_x = (bruglesco::canvas_width - columns * 128.f) / (columns + 1);
    float padding_y = (bruglesco::canvas_height - rows * 128.f) / (rows + 1);
    float x = padding_x;
    float y = padding_y;
    unsigned counter = 1;
    for (std::vector<sf::RectangleShape>::iterator card = deck.begin(); card != deck.end(); ++card)
    {
        card->setPosition(x, y);
        if (counter < columns)
        {
            ++counter;
            x += padding_x + bruglesco::card_width;
        }
        else
        {
            counter = 1;
            x = padding_x;
            y += padding_y + bruglesco::card_height;
        }
    }
}

void GameScreen::highlightButtons()
{
    if (paused || mouseIn == bruglesco::gameMouseIn::pause)
    {
        pauseButton.setFillColor(sf::Color(60, 60, 60, 255));
        pauseString.setFillColor(sf::Color::Red);
    }
    else
    {
        pauseButton.setFillColor(sf::Color(120, 120, 120, 255));
        pauseString.setFillColor(sf::Color::Black);
    }

    if (mouseIn == bruglesco::gameMouseIn::reset)
    {
        returnToMain.setFillColor(sf::Color(60, 60, 60, 255));
        resetString.setFillColor(sf::Color::Red);
    }
    else
    {
        returnToMain.setFillColor(sf::Color(120, 120, 120, 255));
        resetString.setFillColor(sf::Color::Black);
    }
}

void GameScreen::matchFailDelay()
{
    if (delay == 0 && data.getFailedCards().size() == 2)
    {
        delay = 240;
    }
}

void GameScreen::endFailDelay()
{
    data.resetDeck();
}

void GameScreen::moveMatched()
{
    if (data.getMatchedCards().size() == 2)
    {
        float x = bruglesco::player_card_x + bruglesco::player_card_x_offset * data.getPlayer().getScore();
        float y;
        if (data.playerOneTurn())
        {
            y = bruglesco::player_one_card_y;
        }
        else
        {
            y = bruglesco::player_two_card_y;
        }

        for (unsigned i = 0; i < data.getMatchedCards().size(); ++i)
        {
            deck[data.getMatchedCards()[i]].setPosition(x, y);
        }
        data.resetDeck();
    }
}

void GameScreen::adjustTexture()
{
    for (unsigned i = 0; i < data.getDeck().size(); ++i)
    {
        if (data.getDeck()[i].checkState() == bruglesco::CardState::unmatched)
        {
            deck[i].setTextureRect(sf::IntRect(2048, 0, static_cast<int>(bruglesco::card_width), static_cast<int>(bruglesco::card_height)));
        }
        else
        {
            deck[i].setTextureRect(sf::IntRect(imageIdentifiers[data.getDeck()[i].getMatch()] * 128, 0, static_cast<int>(bruglesco::card_width), static_cast<int>(bruglesco::card_height)));
        }
    }
    for (unsigned i = 0; i < data.getFailedCards().size(); ++i)
    {
        deck[data.getFailedCards()[i]].setTextureRect(sf::IntRect(imageIdentifiers[data.getDeck()[data.getFailedCards()[i]].getMatch()] * 128, 0, static_cast<int>(bruglesco::card_width), static_cast<int>(bruglesco::card_height)));
    }
}

I tried to be a little more thorough in my description of my code this time around but that is not a skill I am great at yet. I hope that if you need any further explanation you will leave a comment and I will provide it.

If I have any particular concerns for the code it would be the GameScreen class. It just seems to be large enough to worry. Should I try to abstract it further? any suggestions how? Of course I want any suggestions on any aspect of the code.

\$\endgroup\$

1 Answer 1

Reset to default
2
\$\begingroup\$ 
static std::ofstream debuggStream("DebugLog.txt"); // remove for release

This could possibly be wrapped in #ifdef DEBUG. Alternatively you could use a logging library for finetuned logging needs. Personally I like spdlog.

I think the name Memory is a bit misleading (probably just me though). I'd prefer something like MemoryGame.

MenuScreen::MenuScreen(ModelData& data, GameScreen& game, sf::Font& font) :
data{ data },
game{ game },
font{ font }

Not a fan of your formatting for this. A better (IMO) way to format this would be:

MenuScreen::MenuScreen(ModelData& data, GameScreen& game, sf::Font& font) 
    : data{ data }
    , game{ game } 
    , font{ font }

This way the respective entries nicely line up and are easier to read and edit.

for (std::vector<sf::Text>::iterator string = pairString.begin(); string != pairString.end(); ++string)

Does SFML not play nice with ranged for loops or why are you using the clunky iterator version? You use this version more than once.
In this instance I'm also not happy about the name string. Although not wrong it looks strange to me and I'd choose a different name. Maybe something that also conveys a bit of info like menu_string perhaps?

The way you do things in trackMouse doesn't seem efficient to me. However I'm not totally sure how to do it better.
One idea I had was to use a map and then just iterate. I'm just not very sure this is better or even feasible but for what it's worth, here is what I'd try (very untested):

std::map<sf::RectangleShape, bruglesco::menuMouseIn> const static mapping
{
    {playButton, bruglesco::menuMouseIn::play},
    {sixPairs, bruglesco::menuMouseIn::six},
    {eightPairs, bruglesco::menuMouseIn::eight},
    {twelvePairs, bruglesco::menuMouseIn::twelve},
    {sixPairs, bruglesco::menuMouseIn::sixteen},
};

for (auto const& element : mapping)
{
    if (element.first.getGlobalBounds().contains(mousePos))
    {
        mouseIn = element.second;
    }
}

This way you save the code duplication in the countless if branches. As I said I'm not sure this will even work and it's probably quite a bit of overhead anyways.

\$\endgroup\$
7
  • \$\begingroup\$ Thanks for killing this old zombie. The only thing I don't follow is why leading with the operators in the initializer list is more readable than trailing. \$\endgroup\$
    – Summer
    Commented Jun 5, 2019 at 11:58
  • \$\begingroup\$ @bruglesco That way the operator clearly belongs to its line and you don't have to think about if you missed a comma or not. Plus it seems to be the official standard: en.cppreference.com/w/cpp/language/initializer_list \$\endgroup\$
    – yuri
    Commented Jun 5, 2019 at 12:02
  • \$\begingroup\$ The example toward the bottom of the page is different from the one at the top. The only thing they are consistent with is the colon operator. \$\endgroup\$
    – Summer
    Commented Jun 5, 2019 at 12:40
  • \$\begingroup\$ @bruglesco Do you mean the other one where they put everything on one line? That is probably just laziness or trying to keep it compact for example code. \$\endgroup\$
    – yuri
    Commented Jun 5, 2019 at 12:48
  • \$\begingroup\$ No. The struct named Class near the bottom under the header Example \$\endgroup\$
    – Summer
    Commented Jun 5, 2019 at 12:54

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.