Pong game with a random twist

I made a simple Pong game using SDL2, with a twist: You don't get to play :), only the 2 AIs get to. ;)

Because Pong is a very small game, I put everything in a single file (main.cpp). If that was the wrong choice, please tell me :).

I know that global variables are bad, but in my defense they are all constexpr (if that's not ok, please tell me).

I didn't put many comments, because I think the code is fairly self-documenting (I could be wrong though).

As far as I know, the 2 AIs are unbeatable, and won't ever let the ball pass (except if the paddle speed is so low and the ball speed is so high). The score in the middle shows how many times the AIs failed (should always be 0).

I am fairly advanced (maybe even only low intermediate) in C++, so if I did something wrong in terms of idioms, etc in C++11/C++14, please tell me :).

As a bonus, if you could maybe review the AI, that would be great!

Here's the code in question:

```
#include
#include
#include
#include
#include

constexpr unsigned short gWidth = 640U;
constexpr unsigned short gHeight = 480U;
constexpr unsigned short gDistance = 25U;
constexpr unsigned short gStartY = 50U;
constexpr unsigned short gPaddleWidth = 10U;
constexpr unsigned short gPaddleHeight = 45U;
constexpr unsigned short gPaddleVelocity = 8U;
constexpr unsigned short gBallWidth = 10U;
constexpr unsigned short gBallHeight = 10U;
constexpr unsigned short gBallVelocity = 8U;
constexpr short gPaddleDefaultDest = -999;

//Constructs object with supplied parameters (note that object must have variables 'x', 'y', 'width' and 'height')
template
void loadObject(T& pad, short x, short y, unsigned short w, unsigned short h);

//Checks if variable is in a valid state, cleanups resources if not
template
inline bool check(const T& t)
{
if (!t)
{
TTF_Quit();
SDL_Quit();
return false;
}
return true;
}

enum class DirectionX
{
Right = +1,
Left = -1
};
enum class DirectionY
{
Up = -1,

I don't have SDL installed right now, so apologies for not being able to test your program. I won't have much to say about the AI logic in account of that.


I know that global variables are bad, but in my defense they are all constexpr

Not an absolute fact that all globals are bad, but yes, global variables can get out of hand because they are shared mutable state, so it is harder for the programmer to create a mental model of something that can be changed in many places and in many different ways.

But what you have there are global constants, which is perfectly fine. A constant always has the same value, so once you look at the declaration, you know what value to expect, so there's no mental overhead for the programmer to track down the state of a constant.

Other remarks:

-
loadObject seems unnecessary. It would be nicer to give Paddle and Ball parameterized constructors to have the initialization of data members closer to the class declaration. Besides, loadObject is not actually loading anything (not from a file, at least).

-
main is a bit long and messy. There's initialization code and game loop code in there, so it might be the time to think about defining a PongGame class that main just instantiates and calls a runGameLoop() method. I leave this as a suggestion/exercise for a follow-up question.

-
Since you have everything in a single file, there's no need for function prototypes. Declaring everything before main suffices. That prevents having to update prototype and declaration if the function is changed.

-
I see that you like to use the unary + on values and literals, but that's a no-op, so it doesn't serve a purpose, besides adding boilerplate to the code.

-
When you're not using the command line in main, you can declare it as a function taking no arguments:

int main()

Instead of:

int main(int, char**)

-
You're using these custom deleters with unique_ptr in several places:

std::unique_ptr surface{ SDL_LoadBMP("paddle.bmp"), &SDL_FreeSurface };

And they are very verbose and unreadable. We should introduce a helper function and unique_ptr deleter struct for that:

template
struct SdlUniquePtrDeleter
{
    void (*deleteFunc)(T *);
    void operator()(T * ptr) const
    {
        deleteFunc(ptr);
    }
};

template
std::unique_ptr> 
makeSdlUniquePtr(T * sdlObj, void (*deleteFunc)(T *))
{
    return { sdlObj, SdlUniquePtrDeleter{ deleteFunc } };
}

Then you can simplify the usage to this:

auto surface = makeSdlUniquePtr(SDL_LoadBMP("paddle.bmp"), &SDL_FreeSurface);

A little boilerplate to define the deleter struct, but I think it pays of when you're going to use it several times.

StackExchange Code Review Q#129389, answer score: 3