11
\$\begingroup\$

My take on the Prebash Challenge, "Simon Says" game, in less than 300 lines of C++ (plus the header file), using SDL for window management and drawing.

simon_says.hpp:

#ifndef CR_SIMON_SAYS_HPP
#define CR_SIMON_SAYS_HPP

#include "SDL.h"
#include <vector>
#include <random>

// --------------------------------------------------------

// "cr" stands for Code-Review, not Coding-Rampage ;)
namespace cr
{

class SimonSaysGame
{
public:

    SimonSaysGame() = default;
    ~SimonSaysGame();

    SimonSaysGame(const SimonSaysGame &) = delete;
    SimonSaysGame & operator = (const SimonSaysGame &) = delete;

    bool initSDL();
    void runGameLoop();

private:

    // IDs of the four colored squares of "Simon Says".
    enum class ColorIndex
    {
        Green, Red, Yellow, Blue,

        // Number of enum entries / invalid dummy:
        Count, Invalid = Count
    };

    // Internal helpers:
    void checkAppEvents();
    void checkFailCondition();
    void renderGameBoard();
    void newRandomColors();
    void drawColoredSquare(ColorIndex colorIdx, bool halfTone);

    // Random colors drawn for the current turn.
    std::vector<ColorIndex> colorsThisTurn;

    // User input this turn.
    std::vector<ColorIndex> userInputColors;
    ColorIndex lastUserColor = ColorIndex::Invalid;

    // Number of colors the player will have to remember next time.
    // Start of with an easy 2 colors.
    unsigned int numColorsNextTurn = 2;

    // Number of colors we still draw before ending
    // the display part of the current turn.
    unsigned int numColorsLeftToDisplay = 0;

    // Pseudo-random engine:
    std::mt19937 randGenerator{ std::random_device()() };
    std::uniform_int_distribution<int> randDist{ 0, static_cast<int>(ColorIndex::Count) - 1 };

    // Misc helper data:
    SDL_Window   * window   = nullptr;
    SDL_Renderer * renderer = nullptr;
    bool           lostGame = false;
    bool           isDone   = false;
};

} // namespace cr {}

#endif // CR_SIMON_SAYS_HPP

simon_says.cpp:

#include "simon_says.hpp"
#include <array>
#include <cassert>
#include <iostream>

// --------------------------------------------------------

namespace cr
{

namespace {

// Dimensions of our game window:
constexpr int WinWidth  = 400;
constexpr int WinHeight = 400;

// Small border (in pixels) on each side of a square:
constexpr int RectOffset = 4;

// The four colored rectangles:
const std::array<SDL_Rect, 4> rects = {{
    { RectOffset,                  RectOffset,                   (WinWidth / 2) - (RectOffset * 2), (WinHeight / 2) - (RectOffset * 2) },
    { (WinWidth / 2) + RectOffset, RectOffset,                   (WinWidth / 2) - (RectOffset * 2), (WinHeight / 2) - (RectOffset * 2) },
    { RectOffset,                  (WinHeight / 2) + RectOffset, (WinWidth / 2) - (RectOffset * 2), (WinHeight / 2) - (RectOffset * 2) },
    { (WinWidth / 2) + RectOffset, (WinHeight / 2) + RectOffset, (WinWidth / 2) - (RectOffset * 2), (WinHeight / 2) - (RectOffset * 2) }
}};

// And their RGB colors:
const std::array<unsigned char[3], 4> colors = {{
    { 0,   255, 0   }, // green
    { 255, 0,   0   }, // red
    { 255, 255, 0   }, // yellow
    { 0,   0,   255 }  // blue
}};

// Test if the (x,y) point is inside the SDL_Rect.
inline bool rectContainsPoint(const SDL_Rect & rect, const int x, const int y)
{
    if (x < rect.x) { return false; }
    if (y < rect.y) { return false; }
    if (x > (rect.x + rect.w)) { return false; }
    if (y > (rect.y + rect.h)) { return false; }
    return true;
}

} // namespace {}

SimonSaysGame::~SimonSaysGame()
{
    if (renderer != nullptr)
    {
        SDL_DestroyRenderer(renderer);
    }

    if (window != nullptr)
    {
        SDL_DestroyWindow(window);
    }

    SDL_Quit();
}

bool SimonSaysGame::initSDL()
{
    if (SDL_Init(SDL_INIT_VIDEO | SDL_INIT_EVENTS) != 0)
    {
        std::cerr << "Failed to initialize SDL! " << SDL_GetError() << std::endl;
        return false;
    }

    if ((window = SDL_CreateWindow(nullptr, 0, 0, WinWidth, WinHeight, SDL_WINDOW_OPENGL)) == nullptr)
    {
        std::cerr << "Could not initialize Window! " << SDL_GetError() << std::endl;
        return false;
    }

    if ((renderer = SDL_CreateRenderer(window, -1, 0)) == nullptr)
    {
        std::cerr << "Could not create Renderer! " << SDL_GetError() << std::endl;
        return false;
    }

    newRandomColors();
    return true;
}

void SimonSaysGame::checkAppEvents()
{
    SDL_Event event;
    while (SDL_PollEvent(&event))
    {
        if (event.type == SDL_QUIT)
        {
            isDone = true;
        }
        else if (event.type == SDL_MOUSEBUTTONDOWN)
        {
            for (std::size_t i = 0; i < rects.size(); ++i)
            {
                if (!rectContainsPoint(rects[i], event.motion.x, event.motion.y))
                {
                    continue;
                }

                lastUserColor = static_cast<ColorIndex>(i);
                userInputColors.push_back(lastUserColor);
                std::cout << "Clicked square #" << i << "\n";
            }
        }
    }
}

void SimonSaysGame::checkFailCondition()
{
    if (lastUserColor == ColorIndex::Invalid)
    {
        return; // Didn't start playing yet.
    }

    if (!std::equal(std::begin(userInputColors), std::end(userInputColors), colorsThisTurn.rbegin()))
    {
        lostGame = true;
        std::cout << "You loose!\n";
    }
}

void SimonSaysGame::newRandomColors()
{
    colorsThisTurn.clear();
    userInputColors.clear();
    lastUserColor = ColorIndex::Invalid;

    std::cout << "Generating #" << numColorsNextTurn << " colors this turn.\n";

    for (unsigned int i = 0; i < numColorsNextTurn; ++i)
    {
        colorsThisTurn.push_back(static_cast<ColorIndex>(randDist(randGenerator)));
    }

    numColorsLeftToDisplay = static_cast<unsigned int>(colorsThisTurn.size());

    // One more color next turn.
    ++numColorsNextTurn;
}

void SimonSaysGame::renderGameBoard()
{
    if (lostGame)
    {
        // Draw a blood red screen and stall.
        // User will have to reset the app to play again :(
        SDL_SetRenderDrawColor(renderer, 180, 0, 0, 255);
        SDL_RenderClear(renderer);
        SDL_RenderPresent(renderer);
        return;
    }

    SDL_SetRenderDrawColor(renderer, 0, 0, 0, 255);
    SDL_RenderClear(renderer);

    ColorIndex colorIdx;

    // Draw the random color set of current turn once:
    if (numColorsLeftToDisplay != 0)
    {
        colorIdx = colorsThisTurn[--numColorsLeftToDisplay];
    }
    else
    {
        // Time to draw the user input.
        colorIdx = lastUserColor;
    }

    if (colorIdx != ColorIndex::Invalid)
    {
        drawColoredSquare(colorIdx, false);
    }

    // We need to draw the selected colored square (if any) with
    // full brightness. The other three with a half-tone/faded color.
    for (int i = 0; i < static_cast<int>(ColorIndex::Count); ++i)
    {
        if (static_cast<ColorIndex>(i) != colorIdx)
        {
            drawColoredSquare(static_cast<ColorIndex>(i), true);
        }
    }

    SDL_RenderPresent(renderer);
}

void SimonSaysGame::drawColoredSquare(const ColorIndex colorIdx, const bool halfTone)
{
    const std::size_t index = static_cast<std::size_t>(colorIdx);
    assert(index < rects.size() && index < colors.size());

    const unsigned char colorScale = (halfTone ? 4 : 1);
    SDL_SetRenderDrawColor(renderer,
        colors[index][0] / colorScale,
        colors[index][1] / colorScale,
        colors[index][2] / colorScale, 255);

    SDL_RenderFillRect(renderer, &rects[index]);
}

void SimonSaysGame::runGameLoop()
{
    while (!isDone)
    {
        checkAppEvents();
        renderGameBoard();
        checkFailCondition();

        if (userInputColors.size() == colorsThisTurn.size() && !lostGame)
        {
            // Shuffle for a new turn.
            newRandomColors();
        }

        // We handle the "speed" of the game with a simple delay.
        // A rather crude way, but works...
        SDL_Delay(700);
    }
}

} // namespace cr {}

// --------------------------------------------------------

int main(int /* argc */, const char ** /* argv */)
{
    cr::SimonSaysGame game;

    if (!game.initSDL())
    {
        return -1;
    }

    game.runGameLoop();
}

And this is how it looks:

enter image description here

All reviews and comments are welcome.

\$\endgroup\$
  • 1
    \$\begingroup\$ i'm not quite comfortable with static_cast for every enum class ColorIndex underlays. it would be better if you used regular enum \$\endgroup\$ – MORTAL Dec 5 '14 at 6:11
  • 1
    \$\begingroup\$ after digging on scope enum and std, i found that there is std::underlying_type<>. it's typedef. please consider using the std::underlying_type<ColorIndex> to a void expensive call of static_cast \$\endgroup\$ – MORTAL Dec 6 '14 at 15:49
  • 1
    \$\begingroup\$ @MORTAL - Humm, good point about underlying_type, it seems to have been designed specifically for such cases. I don't think static_cast is expensive though. It is probably converted to a no-op when used with an enum. It just exists to relax the type-system, otherwise, should have no runtime cost. \$\endgroup\$ – glampert Dec 6 '14 at 16:58
7
\$\begingroup\$

Since this is a small project, it doesn't matter much, but if you use SDL with modern C++, don't hesitate to create this kind of custom deleters for SDL objects to use with std::unique_ptr:

struct renderer_delete
{
    void operator()(SDL_Renderer* renderer) const
    {
        if (renderer != nullptr)
        {
            SDL_DestroyRenderer(renderer);
        }
    }
};

using renderer_ptr = std::unique_ptr<SDL_Renderer, renderer_delete>;

Note that I am not sure whether the nullptr is needed or not, I coudn't find anything about it in less than 10 minutes. You can also create an equivalent deleter/pointer pair for SDL_Window and replace the raw pointers for window and renderer in SimonSaysGame by instances of windows_ptr and renderer_ptr.

Here, using function objects instead of function pointers for the custom deleter of std::unique_ptr has two advantages (besides all the benefits of automatic destruction thanks to RAII):

  • A function pointer type does not tell which function has to be used while renderer_delete knows it has to use SDL_DestroyRenderer. With an std::unique_ptr<SDL_Renderer, void(*)(SDL_Rerderer*)>, you would have to give SDL_DestroyRenderer at construction.

  • If the implementation of std::unique_ptr allows empty base class optimization, then using an empty function object (which can be derived from) will result in a lighter std::unique_ptr. Since function pointers cannot be derived from, the empty base class optimization does not work with them.

To sum up: you write ten lines of code, then you get an automatic destruction of an SDL object with std::unique_ptr without any overhead, be it memory or performance overhead. That's awesome.

\$\endgroup\$
  • \$\begingroup\$ Nice idea! BTW, the const in your example somehow ended up in the if clause :P, also the operator is missing a pair or parenthesis. E.g.: void operator() (SDL_Renderer* renderer) const \$\endgroup\$ – glampert Dec 6 '14 at 13:51
  • 1
    \$\begingroup\$ @glampert Haha, right. I will fix this (how did I manage to place const here?), thanks :) \$\endgroup\$ – Morwenn Dec 6 '14 at 13:55
5
\$\begingroup\$

I notice a few things that might allow for improvements in the code.

Make each square an object

By encapsulating the position and color of each square in an object (which could be private to the SimonSaysGame class), much of your code would be simpler. It would make sense to have have a std::array of them, initialize them once and then delegate much of the behavior to them. It would consolidate most of the items currently in the anonymous namespace.

Eliminate the ColorIndex enum

It seems that the ColorIndex enum is not really simplifying things. In fact, it may be making things more complex: all 7 of the static_casts in the program are due to ColorIndex. For that reason, I'd eliminate it preferring to either have a Color class or have this information subsumed under the proposed Square class I mentioned above.

Fix loss handling

When I lose a game, the console simply repeats "You loose!" forever until I quit the game. First, it's probably sufficient just to tell me once. Second, I think you mean "You lose!"

Fix memory leaks if possible

Using valgrind, on my Linux box, I see that memory is leaking, but it's not clear to me whether it's deep inside SDL or within your own code. I suspect the former, but you might want to investigate to make sure it's not your code and to see if you get the same thing on your machine.

Add sound

The ancient electronic version played sounds. Adding those to this program would be a nice addition.

\$\endgroup\$
  • \$\begingroup\$ Haha, "You loose!" - didn't notice that one :P \$\endgroup\$ – glampert Dec 6 '14 at 15:05
  • \$\begingroup\$ what about std::underlying_type<>::type for enums. if i'm not mistaken, it will solve the casting issues \$\endgroup\$ – MORTAL Dec 6 '14 at 16:12
  • 1
    \$\begingroup\$ That's not really clean either. If you need a scoped enum convertible to an integer, use a plain enum. If you really feel like it should be scoped more, put it in a namespace. Enum classes are great since they solve to problems at once (scoping and type safety), but there should be a way to get only one of these two :/ \$\endgroup\$ – Morwenn Dec 6 '14 at 21:04
  • \$\begingroup\$ Or better yet, make the entire problem go away by having each Square handle its own color. \$\endgroup\$ – Edward Dec 6 '14 at 21:16

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.