Pong game in C++

Question

This is my second SFML game, which is Pong. Please tell me what you think.

  #include <iostream>
#include <SFML/Graphics.hpp>
int player1score = 0;
int player2score = 0;

bool intersects(sf::CircleShape& c1, sf::RectangleShape& rect2){
sf::FloatRect circle = c1.getGlobalBounds();
sf::FloatRect rectangle = rect2.getGlobalBounds();
return circle.intersects(rectangle);
}
    int main(){
sf::VideoMode videomode(400, 400);
sf::RenderWindow window(videomode, "PONG");
sf::CircleShape Ball;
Ball.setFillColor(sf::Color::Red);
Ball.setRadius(10);
Ball.setPosition(100, 200);
sf::RectangleShape firstPlayer;
firstPlayer.setFillColor(sf::Color::Black);
firstPlayer.setSize(sf::Vector2f(10, 50));
firstPlayer.setPosition(30, 200);

sf::RectangleShape secondPlayer;
secondPlayer.setFillColor(sf::Color::Black);
secondPlayer.setSize(sf::Vector2f(10, 50));
secondPlayer.setPosition(370, 200);

sf::RectangleShape outleft;
outleft.setFillColor(sf::Color::Black);
outleft.setSize(sf::Vector2f(10, 400));
outleft.setPosition(390, 0);

sf::RectangleShape outright;
outright.setFillColor(sf::Color::Black);
outright.setSize(sf::Vector2f(10, 400));
outright.setPosition(0,0);





sf::RectangleShape mid;
mid.setFillColor(sf::Color::Blue);
mid.setSize(sf::Vector2f(20, 400));
mid.setPosition(200, 0);

sf::RectangleShape up;
up.setFillColor(sf::Color::Blue);
up.setSize(sf::Vector2f(20, 400));
up.setPosition(400, 0);
up.rotate(90);

sf::RectangleShape down;
down.setFillColor(sf::Color::Blue);
down.setSize(sf::Vector2f(20, 400));
down.setPosition(400, 380);
down.rotate(90);
sf::RectangleShape left;
sf::RectangleShape right;
sf::Vector2<float>ballSpeed(0.1, 0.1);








while (window.isOpen()){
    window.clear(sf::Color::Yellow);
    window.draw(mid);
    window.draw(Ball);
    window.draw(secondPlayer);
    window.draw(down);
    window.draw(up);
    window.draw(firstPlayer);
    window.draw(outleft);
    window.draw(outright);


    window.display();

    sf::Event event;
    while (window.pollEvent(event))
    {
        if (event.type == sf::Event::Closed)
            window.close();
    }

if (sf::Keyboard::isKeyPressed(sf::Keyboard::Up)){
    firstPlayer.move(0, -0.1);
}
else if (sf::Keyboard::isKeyPressed(sf::Keyboard::Down)){
    firstPlayer.move(0, 0.1);
}
else if (sf::Keyboard::isKeyPressed(sf::Keyboard::W)){
    secondPlayer.move(0, -0.1);
}
else if (sf::Keyboard::isKeyPressed(sf::Keyboard::S)){
    secondPlayer.move(0, 0.1);
}
if (intersects(Ball, outleft)){
    player1score++;
    Ball.setPosition(100, 200);
    std::cout << "Player1=  " << player1score << std::endl;
}
if (intersects(Ball, outright)){
    player2score++;
    Ball.setPosition(100, 200);
    std::cout << "Player2 =  " << player2score << std::endl;
}
if (intersects(Ball, up) || (intersects(Ball, down))){
    ballSpeed.y = -ballSpeed.y;
}
if (intersects(Ball, firstPlayer) || (intersects(Ball, secondPlayer))){
    ballSpeed.x = -ballSpeed.x;
}
Ball.move(ballSpeed.x, ballSpeed.y);

}

}

Answer 1

I see a number of things that may help you improve your program.

Fix the formatting

It's not clear to me if the code as posted is what you see when you look at it, or if there was a problem pasting the code, but in either case, it's worthwhile to fix the formatting. In particular, consistent indentation and consistent use of a particular style is really more important than which particular style you may eventually choose. The key to readable code is to pick a style and apply it consistently.

Eliminate "magic numbers"

This code is littered with "magic numbers," that is, unnamed constants such as -0.1, 20, 400, etc. Generally it's better to avoid that and give such constants meaningful names. That way, if anything ever needs to be changed, you won't have to go hunting through the code for all instances of "20" and then trying to determine if this particular 20 means the radius of the player's circle or some other constant that happens to have the same value.

Be aware of object lifecycles

There really isn't any need for the event object to be created and destroyed each time through the main loop since it can easily be reused. It may be better to declare it once outside the loop and simply use it within.

Think of the user

The ball appears to consistently launch from midfield on the left side. When the ball is launched toward the right, that's reasonable, but when it's launched to the left, it means that the ball suddenly appears with very little time to react. It would improve the game play to have the ball launch such that it always traverses at least three quarters of the court before coming into the plane of a user paddle.

Check for reasonable ranges

The paddles can be moved entirely off the playing field, and further, there doesn't appear to be a limit at any distance. It would be nicer to limit the paddle to within the playing field. Otherwise, if the player wasn't paying attention, the paddle could be off the board with no visual indication of which way to send it back.

Eliminate global variables where practical

Having routines dependent on global variables makes it that much more difficult to understand the logic and introduces many opportunities for error. Eliminating global variables where practical is always a good idea, whether programming for desktop machines or for embedded systems. For global variables such as player1score, the simplest fix in this case is to simply move them to within main.

Consider scaling

Right now the board is asymmetric. This could more easily be fixed if the numbers were not all fixed but rather derived from one or two fixed values. This would allow scaling and enable easier symmetry.

Use objects

Because SFML already uses an object model, making better use of objects for your own code is often just a matter of deriving from that code. For example, in this game, each of the players and the ball could be an object with more sophisticated and game-specific behavior than simply a sf::RectangleShape.

class Paddle : public sf::RectangleShape {
public:
    Paddle(float horzPos) 
    : sf::RectangleShape(sf::Vector2f{10, 50}),
      horz{horzPos}
    {
        setFillColor(sf::Color::Black);
        setPosition(horz, 200);
    }
    void setKeys(sf::Keyboard::Key upkey, sf::Keyboard::Key downkey) {
        K_up = upkey;
        K_down = downkey;
    }
    bool handleKey() {
        if (sf::Keyboard::isKeyPressed(K_up)) {
            move(up);
        } else if (sf::Keyboard::isKeyPressed(K_down)) {
            move(down);
        } else {
            return false;
        }
        return true;
    }
private:
    float horz;
    sf::Keyboard::Key K_up, K_down;
    static const sf::Vector2f up, down;
};
const sf::Vector2f Paddle::up{0, -0.1};
const sf::Vector2f Paddle::down{0, +0.1};