3
\$\begingroup\$

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 <SDL.h>
#include <SDL_ttf.h>
#include <string>
#include <memory>
#include <random>

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<typename T>
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<typename T>
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,
    Down = 1
};

struct Paddle
{
    short x;
    short y;
    unsigned short width;
    unsigned short height;
    short destination = gPaddleDefaultDest;

    bool tryMoveY(const short value)
    {
        //Move only if inside window bounds
        if (y + value >= 0 && y + height + value <= gHeight)
        {
            y += value;
            return true;
        }
        return false;
    }
};

struct Ball
{
    short x;
    short y;
    unsigned short width;
    unsigned short height;

    void move(const Paddle& pad1, const Paddle& pad2, const short value, unsigned short& score, bool disable = false)
    {
        //Flip direction if applicable
        if (x + value <= 0 || x + value + width >= gWidth)
        {
            flipX *= -1;
            ++score;
        }
        if (y + value <= 0 || y + value + height >= gHeight)
            flipY *= -1;
        if (x > pad1.x && x < pad1.x + pad1.width && ((y > pad1.y && y < pad1.y + pad1.height) || (y + height > pad1.y && y + height < pad1.y + pad1.height)) && !disable)
            flipX *= -1;
        if (x + width > pad2.x && x + width < pad2.x + pad2.width && ((y > pad2.y && y < pad2.y + pad2.height) || (y + height > pad2.y && y + height < pad2.y + pad2.height)) && !disable)
            flipX *= -1;

        x += value * flipX;
        y += value * flipY;
    }

    DirectionX getDirectionX() const { return flipX == +1 ? DirectionX::Right : DirectionX::Left; }
    DirectionY getDirectionY() const { return flipY == +1 ? DirectionY::Up : DirectionY::Down; }
private:
    char flipX = 1;
    char flipY = 1;
};

//Moving AIs
void moveAIs(Paddle& pad1, Paddle& pad2, const Ball& ball);

//Helper function
void movePaddle(Paddle& pad1, Paddle& pad2, const Ball& ball);

int main(int, char**)
{
    if (SDL_Init(SDL_INIT_VIDEO | SDL_INIT_EVENTS) == -1)
        return -1;
    if (TTF_Init() == -1)
        return -1;

    std::unique_ptr<SDL_Window, decltype(&SDL_DestroyWindow)> window{ nullptr, &SDL_DestroyWindow };
    window.reset(SDL_CreateWindow("Pong", SDL_WINDOWPOS_UNDEFINED, SDL_WINDOWPOS_UNDEFINED, gWidth, gHeight, SDL_WINDOW_SHOWN));
    if (!check(window)) return -1;

    std::unique_ptr<SDL_Renderer, decltype(&SDL_DestroyRenderer)> render{ nullptr, &SDL_DestroyRenderer };
    render.reset(SDL_CreateRenderer(window.get(), -1, SDL_RENDERER_ACCELERATED | SDL_RENDERER_PRESENTVSYNC));
    if (!check(render)) return -1;

    Paddle pad1, pad2;
    loadObject(pad1, gDistance, gStartY, gPaddleWidth, gPaddleHeight);
    loadObject(pad2, gWidth - gDistance - gPaddleWidth, gStartY, gPaddleWidth, gPaddleHeight);

    //Random start position
    std::random_device re;
    std::mt19937 mt{ re() };
    std::uniform_int_distribution<int> uid{ 100, 300 };

    Ball ball;
    loadObject(ball, uid(mt), uid(mt), gBallWidth, gBallWidth);

    //Load textures
    std::unique_ptr<SDL_Surface, decltype(&SDL_FreeSurface)> surface{ SDL_LoadBMP("paddle.bmp"), &SDL_FreeSurface };
    std::unique_ptr<SDL_Texture, decltype(&SDL_DestroyTexture)> tex{ SDL_CreateTextureFromSurface(render.get(), surface.get()), &SDL_DestroyTexture };

    //Load font for text rendering
    std::unique_ptr<TTF_Font, decltype(&TTF_CloseFont)> cornerstone{ TTF_OpenFont("Cornerstone.ttf", 32), [](TTF_Font*) {} };
    SDL_Color White = { 255, 255, 255 };

    //Main loop
    SDL_Event event;
    bool run = true;

    unsigned short score = 0;

    while (run)
    {
        //Event loop
        while (SDL_PollEvent(&event))
        {
            if (event.type == SDL_QUIT)
                run = false;
        }

        //Move ball
        ball.move(pad1, pad2, +gBallVelocity, score);

        //Update AI positions
        moveAIs(pad1, pad2, ball);

        //Clear screen
        SDL_RenderClear(render.get());

        //Copy paddles
        SDL_Rect rect{ pad1.x, pad1.y, pad1.width, pad1.height };
        SDL_RenderCopy(render.get(), tex.get(), nullptr, &rect);

        rect = { pad2.x, pad2.y, pad2.width, pad2.height };
        SDL_RenderCopy(render.get(), tex.get(), nullptr, &rect);

        //Copy ball
        rect = { ball.x, ball.y, ball.width, ball.height };
        SDL_RenderCopy(render.get(), tex.get(), nullptr, &rect);

        //Copy text
        std::unique_ptr<SDL_Surface, decltype(&SDL_FreeSurface)> fontSurface{ TTF_RenderText_Solid(cornerstone.get(), std::to_string(score).c_str(), White), &SDL_FreeSurface };
        std::unique_ptr<SDL_Texture, decltype(&SDL_DestroyTexture)> fontTexture{ SDL_CreateTextureFromSurface(render.get(), fontSurface.get()), &SDL_DestroyTexture };
        rect = { gWidth / 2 - 30, 60, 80, 80 };
        SDL_RenderCopy(render.get(), fontTexture.get(), nullptr, &rect);

        //Present
        SDL_RenderPresent(render.get());
    }

    TTF_Quit();
    SDL_Quit();
    return 0;
}

template<typename T>
void loadObject(T& pad, short x, short y, unsigned short w, unsigned short h)
{
    pad.x = x;
    pad.y = y;
    pad.width = w;
    pad.height = h;
}

//Paddle should move to the ball
void moveAIs(Paddle& pad1, Paddle& pad2, const Ball& ball)
{
    if (ball.getDirectionX() == DirectionX::Left)
    {
        movePaddle(pad1, pad2, ball);
        pad2.destination = gPaddleDefaultDest;
    }
    else
    {
        movePaddle(pad2, pad1, ball);
        pad1.destination = gPaddleDefaultDest;
    }
}

void movePaddle(Paddle& pad1, Paddle& pad2, const Ball& ball)
{
    //Stop if already in correct position
    if (pad1.destination > pad1.y && pad1.destination < pad1.y + pad1.height
        && pad1.destination + ball.height > pad1.y && pad1.destination + ball.height < pad1.y + pad1.height)
        return;

    if (pad1.destination != gPaddleDefaultDest)
    {
        pad1.tryMoveY(pad1.destination - pad1.y > 0 ? +gPaddleVelocity : -gPaddleVelocity);
        return;
    }

    //Move virtual ball until final position is known
    Ball copy = ball;
    unsigned short fakeScore = 0;

    if (copy.getDirectionX() == DirectionX::Right)
    {
        while (copy.x < pad1.x)
            copy.move(pad2, pad1, +gPaddleVelocity, fakeScore, true);
    }
    else
    {
        while (copy.x > pad1.x)
            copy.move(pad1, pad2, +gPaddleVelocity, fakeScore, true);
    }

    pad1.destination = copy.y;
}
\$\endgroup\$
3
\$\begingroup\$

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<SDL_Surface, decltype(&SDL_FreeSurface)> 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<typename T>
    struct SdlUniquePtrDeleter
    {
        void (*deleteFunc)(T *);
        void operator()(T * ptr) const
        {
            deleteFunc(ptr);
        }
    };
    
    template<typename T>
    std::unique_ptr<T, SdlUniquePtrDeleter<T>> 
    makeSdlUniquePtr(T * sdlObj, void (*deleteFunc)(T *))
    {
        return { sdlObj, SdlUniquePtrDeleter<T>{ 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.

\$\endgroup\$
  • \$\begingroup\$ Thanks! Actually I can't change the signature of main because SDL defines main as SDL_Main, and there is no prototype for SDL_Main without arguments, but thanks though :) \$\endgroup\$ – Rakete1111 May 27 '16 at 4:42

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.