Refactoring my Pong clone

Back in February I made a Pong clone in C++ using SFML, and I documented the process of building it in a blog post. I'm now coming back to it — 6 months later! — to refactor it and make sure it runs smoother.

The first thing I'm going to do is document everything using Doxygen; back when I first made this thing I wanted a quick prototype, so there were barely any comments in the code and it looked like a mess. It was really hard to understand what the code was even doing in some places.

Paddle.hh

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37

#pragma once

#include <SFML/Graphics.hpp>


/**
 * @brief The Paddle class.
 * 
 * This class describes the Paddle object, which is what the user controls in
 * order to deflect the Pong and gain points. It inherits properties from 
 * SFML's RectangleShape class, as it is basically a rectangle.
 */
class Paddle: public sf::RectangleShape {

    public:
        /**
         * @brief The Paddle constructor method.
         *
         * @param[in]  width   The width of the Paddle.
         * @param[in]  height  The height of the Paddle.
         * @param[in]  speed   The speed of the Paddle.
         */
        Paddle (float width, float height, float speed);
        /**
         * @brief ++ operator override to move the Paddle based on its speed.
         */
        void operator++ ();
        /**
         * @brief -- operator override to move the Paddle based on its speed.
         */
        void operator-- ();


    private:
        /** The Paddle's speed. */
        float _speed;
};

Much better now!

Now that the documentation is done, the first clear improvement I can see would be to add a generic Menu class which we would then use for every menu screen in the game. Right now, all menus are drawn manually, and this is a very bad idea as I'm pretty sure it's all based on magic numbers in order to be centered, and if I ever want to change the style of the menus I would need to go and change them all one by one. Let's stick to the DRY principle.

Also, it is very clear to me that all menus contain three basic elements:

• The menu's title
• Some text (for example in the controls menu the text explains what the controls of the game are)
• Some "buttons" (in this case it's more like static text with a Pong cursor next to a choice, to indicate which one is currently chosen)

That means that there's a way to refactor all that code in a generic Menu class. Let me draw a quick sketch of what I want this "default menu" to look like:

A sketch of what a default Menu should look like.

The title and menu text would be string parameters to the constructor of the Menu class. The options would then be passed as a list of strings (maybe inside of a vector?), along with some way to describe what triggering them does. This last one could be accomplished by some kind of callback function that gets called when the user selects a specific menu item.

I've now gotten the basic functionality down. With a relatively short Menu class, I've managed to get the following example to show up on the screen:

An example of using the Menu class.

Nice! Now let's get around to how you actually use it in the "front end" (or in this case, the Game class).

Basically, what I want to do is create a Menu object for each menu — in my case I have three, ignoring the win screen: the Main menu, the Controls menu and the Credits menu — and then connect it to other menus or the actual game via callbacks. So, ideally, for the first menu I would do something like:

1
2
3
4
5
6
7

Menu mainMenu("p0ng",
              "",
              { "play", "controls", "credits", "quit" },
              { &play, &controlsMenu.show, &creditsMenu.show, [this]() { close(); } },
              _mainFont);

mainMenu.show(*this);

Let's see if I can explain what that does (or better yet, what it is supposed to do since I haven't fully tested it yet).

The first two arguments are pretty simple, as they're just the title and the text/body of the menu. In this case, the title is the name of the game (so that's p0ng with a 0 because we're l33t h4x0rz), and since there's no body text I'm just passing an empty string. Next, we need to pass in a list of options for our menu; this one has four, as you may remember (Play, Controls, Credits and Quit). Right after that we need to pass the callbacks that we want to be called when each option is chosen. The first one is the play() function that launches a game of Pong, so that's easy. The second and third callbacks are actually referring to the other two menu screens, presumably created just before (more on that later). The last one is just a lambda function that calls close() on our window instance (aka the Game class). As it turns out, this needs to be captured before entering the callback; I'm assuming that's because the callback tries to use its own this otherwise, which of course won't work.

Finally, we can display the menu by just calling its show function; this will take care of positioning the text elements where they need to be and will also handle the callbacks based on the user's choice.

Okay, so I've just tested the game and I was kinda wrong. Calling the other menus that way causes many many different problems, but in the end I've found a solution which is, in my opinion, more organised anyway. Basically, each menu screen is contained within its own function, as follows:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44

void Game::mainMenu ()
{
    Menu mainMenu("p0ng",
                  "",
                  { "play", "controls", "credits", "quit" },
                  {
                      [this]() { play(); },
                      [this]() { controlsMenu(); },
                      [this]() { creditsMenu(); },
                      [this]() { close(); }
                  },
                  _mainFont);

    mainMenu.show(*this);
}

void Game::controlsMenu ()
{
    Menu controlsMenu("controls",
                      L"left paddle:\n"\
                          "<w> to go up, <s> to go down\n"\
                          "\n"\
                          "right paddle:\n"\
                          "<↑> to go up, <↓> to go down",
                      { "back" },
                      { [this]() { mainMenu(); } },
                      _mainFont);

    controlsMenu.show(*this);
}

void Game::creditsMenu ()
{
    Menu creditsMenu("credits",
                     "made by dimitri kokkonis\n"\
                         "(kokkonisd.github.io)\n"\
                         "using c++ and sfml\n"\
                         "february 2019",
                     { "back" },
                     { [this]() { mainMenu(); } },
                     _mainFont);

    creditsMenu.show(*this);
}

This makes it really simple to control your menu screens while also having fairly understandable code. You just need to pass the function you want to call for each option as a callback (while capturing this each time). It works like a charm! I have exactly the same menus as before but way less code and no more magic numbers.

Let's walk you through how it works. First of all, we have the constructor method that actually builds the Menu object. Here's its header:

1
2
3
4

Menu (sf::String title, sf::String text, vector<string> options,
      vector<function<void()>> callbacks, sf::Font font,
      int titleSize = 50, int textSize = 35, int optionSize = 40,
      int margin = 50, int optionMargin = 20);

Let's explain the arguments here:

• sf::String title: The title of the menu.
• sf::String text: The text/body of the menu.
• vector<string> options: The names of the options of the menu.
• vector<function<void()>> callbacks: The callback functions related to the options.
• sf::Font font: The font used in the menu.
• int titleSize = 50: The font size for the menu's title (50 by default).
• int textSize = 35: The font size for the menu's text (35 by default).
• int optionSize = 40: The font size for the menu's options (40 by default).
• int margin = 50: The margin between the three menu elements (50 by default).
• int optionMargin = 20: The margin between the options (20 by default).

The constructor takes those in and starts by copying over the callbacks, then the option names, and then setting up the sf::Text objects for the text that's going to be displayed in the menu.

Then, when show() is called, it starts positioning the elements in the window and then draws them one by one. It also starts a polling loop — much like the one in Game::play() — that keeps track of what buttons are pushed, which allows the player to navigate the menu. I've also implemented wrapping the cursor this time, which wasn't present in the original version. This means that the cursor will loop around the options, and you can jump from the first to the last one without having to go through the others:

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18

void Menu::moveCursorUp ()
{
    // If we're on the first element
    if (_selectedMenuItem == 0) {
        // Set the counter to the last element
        _selectedMenuItem = _options.size() - 1;
        // Move the cursor next to the last element
        _cursor.setPosition(_cursor.getPosition().x,
                            _cursor.getPosition().y +
                                (_options.size() - 1) * _margin);
    } else {
        // Decrease the counter
        _selectedMenuItem--;
        // Move the cursor up by one spot
        _cursor.setPosition(_cursor.getPosition().x,
                            _cursor.getPosition().y - _margin);
    }
}

So that's it! I hope you've enjoyed this "devlog" as much as I did. If you want to check out the project or use the same menu architecture in your game, you can find the game's code on GitHub. Have fun making modular menus!