1
\$\begingroup\$

I am making use of C++ and SFML to make a basic Pong game. I have made it so that there are two players who can control the paddle on each side and the ball will bounce off screen and paddles except the left and right sides of screen. Player can also pause and restart the game anytime.

My goal is to learn C++'s features such as pointers, references, polymorphism using virtual, inheritance and memory management by putting it in practice. Therefore, keeping that in mind, I tried to create the game.

I want to know how successful I have been so far and if I am in right track to achieving object oriented program. Any advice on how I can improve and criticism related to my code structure would be appreciated as I look to improve as a programmer.

My code:

GameObject.h

#include <SFML/Graphics.hpp>

using namespace sf;

class GameObject
{
protected:
Vector2f position;
float speed;
RenderWindow& m_window;

public:
    GameObject(float startX, float startY, RenderWindow& window);
    virtual void Draw() = 0;
    virtual void Update() = 0;
};

GameObject.cpp

#include "GameObject.h"

GameObject::GameObject(float startX, float startY, sf::RenderWindow& window) : m_window(window)
{
    position.x = startX;
    position.y = startY;
    speed = 0.3f;
}

Paddle.h

#include <iostream>

#include "GameObject.h"

class Paddle : public GameObject
{
private:
    RectangleShape paddleShape;
    const int shapeWidth = 10;
    const int shapeHeight = 50;

public:
    Paddle(float startX, float startY, RenderWindow& window);
    FloatRect getPosition();
    RectangleShape getShape();
    void moveUp();
    void moveDown();
    void HandleInput1();
    void HandleInput2();
    void Update() override;
    void Draw() override;
};

Paddle.cpp

#include "Paddle.h"

Paddle::Paddle(float startX, float startY, RenderWindow& window) : GameObject(startX, startY, window)
{
    paddleShape.setSize(sf::Vector2f(shapeWidth, shapeHeight));
    paddleShape.setPosition(position);
}

FloatRect Paddle::getPosition()
{
    return paddleShape.getGlobalBounds();
}

RectangleShape Paddle::getShape()
{
    return paddleShape;
}

void Paddle::moveUp()
{
    position.y -= speed;
}

void Paddle::moveDown()
{
    position.y += speed;
}

void Paddle::HandleInput1()
{
    if (Keyboard::isKeyPressed(Keyboard::W) && (paddleShape.getPosition().y - paddleShape.getSize().y / 2 > -20.0f))
    {
        paddleShape.move(0.f, -speed);
    }
    else if (Keyboard::isKeyPressed(Keyboard::S) && (paddleShape.getPosition().y + paddleShape.getSize().y / 2 < m_window.getSize().y - 35.f))
    {
        paddleShape.move(0.f, speed);
    }
}

void Paddle::HandleInput2()
{
    if (Keyboard::isKeyPressed(Keyboard::Up) && (paddleShape.getPosition().y - paddleShape.getSize().y / 2 > -20.0f))
    {
        paddleShape.move(0.f, -speed);
    }
    else if (Keyboard::isKeyPressed(Keyboard::Down) && (paddleShape.getPosition().y + paddleShape.getSize().y / 2 < m_window.getSize().y - 35.f))
    {
        paddleShape.move(0.f, speed);
    }
}

void Paddle::Update()
{

}

void Paddle::Draw()
{
    m_window.draw(paddleShape);
}

Ball.h

#include "GameObject.h"
#include "Paddle.h"

class Ball : public GameObject
{
private:
    CircleShape ballShape;
    const float radius = 10.0f;

public:
    float ballAngle = 0.0f;
    const float pi = 3.14159f;

public:
    Ball(float startX, float startY, RenderWindow& window);
    FloatRect getPosition();
    CircleShape getShape();
    float getRadius();
    void reboundTop();
    void reboundBottom();
    void reboundBat();
    void Update() override;
    void Draw() override;

Ball.cpp

#include "Ball.h"

Ball::Ball(float startX, float startY, RenderWindow& window) : GameObject(startX, startY, window)
{
    ballShape.setRadius(radius);
    ballShape.setPosition(position);

    do
    {
        ballAngle = (std::rand() % 360) * 2 * pi / 360;
    } while (std::abs(std::cos(ballAngle)) < 0.7f);
}

FloatRect Ball::getPosition()
{
    return ballShape.getGlobalBounds();
}

CircleShape Ball::getShape()
{
    return ballShape;
}

float Ball::getRadius()
{
    return radius;
}

void Ball::reboundTop()
{
    ballAngle = -ballAngle;
    ballShape.setPosition(ballShape.getPosition().x, radius + 0.1f);
}

void Ball::reboundBottom()
{
    ballAngle = -ballAngle;
    ballShape.setPosition(ballShape.getPosition().x, m_window.getSize().y - radius - 0.1f);
}

void Ball::reboundBat()
{

}

void Ball::Update()
{
    ballShape.move(std::cos(ballAngle) * (speed - 0.1), std::sin(ballAngle) * (speed - 0.1));
}

void Ball::Draw()
{
    m_window.draw(ballShape);
}

Game.h

#include <SFML/Graphics.hpp>
#include "Paddle.h"
#include "Ball.h"

using namespace sf;

class Game
{
private:
    std::unique_ptr<Paddle> player1;
    std::unique_ptr<Paddle> player2;
    std::unique_ptr<Ball> ball;

    Font font;
    Text pause;
    Text gameOver;
    Text restart;

    bool isPaused;
    bool isGameOver;
    bool isRestart;

public:
    RenderWindow& m_window;
    const unsigned int m_windowWidth;
    const unsigned int m_windowHeight;

public:
    Game(RenderWindow& window, const unsigned int& windowWidth, const unsigned int& windowHeight);
    RenderWindow& GetWindow();
    void RestartGame();
    void HandleCollision();
    void HandleInput();
    void Update();
    void Draw();
    void Run();
};

Game.cpp

#include "Game.h"

Game::Game(RenderWindow& window, const unsigned int& windowWidth, const unsigned int& windowHeight) : 
    m_window(window), m_windowWidth(windowWidth), m_windowHeight(windowHeight)
{
    RestartGame();

    font.loadFromFile("resources/American Captain.ttf");
    gameOver.setFont(font);
    gameOver.setCharacterSize(42);
    gameOver.setFillColor(sf::Color::White);
    gameOver.setStyle(sf::Text::Bold);
    gameOver.setPosition((m_windowWidth / 2) - 100, m_windowHeight / 2);

    pause.setFont(font);
    pause.setCharacterSize(42);
    pause.setFillColor(sf::Color::White);
    pause.setStyle(sf::Text::Regular);
    pause.setPosition((m_windowWidth / 2) - 80, m_windowHeight / 2);

    restart.setFont(font);
    restart.setCharacterSize(25);
    restart.setFillColor(sf::Color::White);
    restart.setStyle(sf::Text::Regular);
    restart.setPosition((m_windowWidth / 2) - 100, (m_windowHeight / 2) + 50);
}

RenderWindow& Game::GetWindow()
{
    return m_window;
}

void Game::RestartGame()
{
    isPaused = false;
    isGameOver = false;
    isRestart = true;
    gameOver.setString("");
    pause.setString("");
    restart.setString("");
    player1 = std::make_unique<Paddle>(40, m_windowHeight / 2, m_window);
    player2 = std::make_unique<Paddle>(m_windowWidth - 50, m_windowHeight / 2, m_window);
    ball = std::make_unique<Ball>(m_windowWidth / 2, m_windowHeight / 2, m_window);
}

void Game::HandleCollision()
{
    if (ball->getShape().getPosition().x - ball->getRadius() < 0.0f)
    {
        isGameOver = true;
        gameOver.setString("PLAYER 2 WINS!");
        restart.setString("Press R to restart\nPress Esc to Quit");
    }
    else if (ball->getShape().getPosition().x + ball->getRadius() > m_windowWidth)
    {
        isGameOver = true;
        gameOver.setString("PLAYER 1 WINS!");
        restart.setString("Press R to restart\nPress Esc to Quit");
    }
    else if (ball->getShape().getPosition().y - ball->getRadius() < 0.f)
    {
        ball->reboundTop();
    }
    else if (ball->getShape().getPosition().y + ball->getRadius() > m_windowHeight)
    {
        ball->reboundBottom();
    }
    else if (ball->getPosition().intersects(player2->getPosition()))
    {
        if (ball->getShape().getPosition().y > player2->getShape().getPosition().y)
            ball->ballAngle = ball->pi - ball->ballAngle + (std::rand() % 20) * ball->pi / 180;
        else
            ball->ballAngle = ball->pi - ball->ballAngle - (std::rand() % 20) * ball->pi / 180;

        ball->getShape().setPosition(player2->getShape().getPosition().x - ball->getRadius() - player2->getShape().getSize().x / 2 - 0.1f, ball->getShape().getPosition().y);
    }

    else if (ball->getPosition().intersects(player1->getPosition()))
    {
        if (ball->getShape().getPosition().y > player1->getShape().getPosition().y)
            ball->ballAngle = ball->pi - ball->ballAngle + (std::rand() % 20) * ball->pi / 180;
        else
            ball->ballAngle = ball->pi - ball->ballAngle - (std::rand() % 20) * ball->pi / 180;

        ball->getShape().setPosition(player1->getShape().getPosition().x - ball->getRadius() - player1->getShape().getSize().x / 2 - 0.1f, ball->getShape().getPosition().y);
    }
}

void Game::HandleInput()
{
    player1->HandleInput1();
    player2->HandleInput2();
}

void Game::Update()
{
    player1->Update();
    player2->Update();
    ball->Update();
}

void Game::Draw()
{
    player1->Draw();
    player2->Draw();
    ball->Draw();

    m_window.draw(pause);
    m_window.draw(gameOver);
    m_window.draw(restart);
}

void Game::Run()
{
    //Game Loop
    while (m_window.isOpen())
    {
        sf::Event event;
        while (m_window.pollEvent(event))
        {
            if (event.type == sf::Event::Closed)
                m_window.close();

            if (event.type == Event::KeyPressed && event.key.code == sf::Keyboard::P)
            {
                if(!isGameOver)
                    isPaused = !isPaused;
            }

            if (event.type == Event::KeyPressed && event.key.code == sf::Keyboard::R)
                RestartGame();
        }

        m_window.clear();

        HandleCollision();

        if (!isPaused)
        {
            HandleInput();
            Update();
        }

        if (isPaused)
            pause.setString("Game Paused");
        else
            pause.setString("");

        Draw();

        m_window.display();
    }
}

Main.cpp

#include <iostream>
#include <memory>
#include <SFML/Graphics.hpp>

#include "Paddle.h"
#include "Game.h"

int main()
{
    const unsigned int windowWidth = 800;
    const unsigned int windowHeight = 600;
    RenderWindow window(VideoMode(windowWidth, windowHeight), "PONG GAME");

    std::unique_ptr<Game> game = std::make_unique<Game>(window, windowWidth, windowHeight);
    game->Run();

    return 0;
}
\$\endgroup\$
2
  • 1
    \$\begingroup\$ It appears that you have updated the description and code. Does the code work correctly to the best of your knowledge now? \$\endgroup\$ Sep 18, 2019 at 21:52
  • \$\begingroup\$ @SᴀᴍOnᴇᴌᴀ Yes, it does. \$\endgroup\$
    – AaySquare
    Sep 19, 2019 at 13:17

1 Answer 1

1
\$\begingroup\$

I am glad to see that you separated the components of the program into different source files. Here's my suggestions:

Game Object

Avoid using namespace sf;. Especially in a header. The problem of using-directives are especially apparent with third-party libraries — the source of the names is obfuscated and there's the risk of name clashing. For someone who doesn't know a lot about SFML (like me), I can see immediately what sf::RenderWindow is, but RenderWindow makes me wonder.

The virtual destructor is missing.

What's wrong with the indentation here?

protected:
Vector2f position;
float speed;
RenderWindow& m_window;

You should use member initializer clauses here:

GameObject::GameObject(float startX, float startY, sf::RenderWindow& window)
    : position{startX, startY}
    , speed{0.3}
    , m_window{window}
{
}

I don't see why the constructor shouldn't be taking a 2D vector directly.

Protected members in the base class are like "global variables" to the inheritance hierarchy. I much prefer a pure interface:

class GameObject {
public:
    virtual ~GameObject() {}
    virtual void Draw() = 0;
    virtual void Update() = 0;
};

It is not common to capitalize function names, but that's purely a matter of style.

Paddle

You don't use the I/O library, so omit #include <iostream>.

shapeWidth and shapeHeight should be static constexpr. The constructor can be inherited.

Now the downside of having protected members in the base class is evident: paddleShape stores the position, resulting in data duplication. You have to make sure the two sets of data are synchronized, which adds clutter to the code. The code looks better like this:

Paddle::Paddle(sf::Vector2f position, RenderWindow& window)
{
    paddleShape.setSize(sf::Vector2f(shapeWidth, shapeHeight));
    paddleShape.setPosition(position);
}

And if RectangleShape has constructors, you should use them.

The move functions should be defined in class.

Ball

Similarly, radius should be a static constexpr. pi should probably be global (possibly in a namespace) until we have C++20 std::numbers::pi.

The calculation of ballAngle can be written like this: (where engine is a suitable random number engine)

static const auto angle_min = std::acos(0.7);
static const auto angle_max = std::acos(-0.7);

std::uniform_real_distribution<float>{angle_min, angle_max} rdist;
ballAngle = rdist(engine);
if (std::bernoulli_distribution bdist{0.5}; bdist(engine))
    ballAngle = 2 * pi - ballAngle;

Game (logic)

You can use std::optional instead of std::unique_ptr:

std::optional<Paddle> player1;
std::optional<Paddle> player2;
std::optional<Ball> ball;

and then, in RestartGame,

player1.emplace(40, m_windowHeight / 2, m_window);
player2.emplace(m_windowWidth - 50, m_windowHeight / 2, m_window);
ball.emplace(m_windowWidth / 2, m_windowHeight / 2, m_window);

This also makes your code exception safe. Not particularly important in this case, I guess, but consider readability and semantic accuracy.

Main

The main code has zero reason to use dynamic memory. Use a local variable:

Game game(window, windowWidth, windowHeight);
game.Run();
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.