HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Ping Pong game in SFML

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
gamepongsfmlping

Problem

I am learning C++ and SFML so I want to know if I am laying my code out neatly and if my code is efficient. I know this is not a full game yet but I want to know if I am on the right tracks.

```
#include
#include "ResourcePath.hpp"

#include

class Paddel {
int speed = 3;
sf::Vector2f border;
sf::Vector2f size;
sf::Vector2f position;
sf::Vector2f screenSize;
public:
sf::RectangleShape shape;

Paddel(sf::Vector2f screenSize,int player)
{
this->screenSize = screenSize;
this->size = sf::Vector2f(20,100);
this->border = sf::Vector2f(8,6);

if(player == 1)
this->position = border;
else if(player == 2) {
position.x = (screenSize.x - size.x) - border.x;
position.y = border.y;
}

this->shape.setSize(this->size);
this->shape.setPosition(this->position);
}

void moveUp()
{
this->position.y -= this->speed;
if(this->position.y border.y)
this->position.y += this->speed;
this->shape.setPosition(this->position);
}

void moveDown()
{
this->position.y += this->speed;
//100 - (600 + 5)
if(this->position.y > (this->screenSize.y - this->border.y) - this->size.y)
this->position.y -= this->speed;
this->shape.setPosition(this->position);
}
};

using std::cout;
using std::endl;

int main()
{
sf::Vector2f screenSize(800,600);
sf::RenderWindow Window(sf::VideoMode(screenSize.x,screenSize.y),"Ping Pong");
Window.setFramerateLimit(60);
Window.setVerticalSyncEnabled(true);

Paddel playerOne(screenSize,1);
Paddel playerTwo(screenSize,2);

const sf::Time TimePerFrame = sf::seconds(1.f/60.f);
sf::Clock clock;
sf::Time timeSinceLastUpdate = sf::Time::Zero;
sf::Time elapsedTime;

bool play = true;

while(Window.isOpen())
{
Window.clear();
sf::Event Event;

while(Window.pollEvent(Event))

Solution

your code is fine but I would go with some thing like this, i have commented the changes

#include 
#include 

// for eliminating magic numbers
enum class Players
{
    PlayerOne,
    PlayerTow
};

enum class ScreenSize
{
    Width = 800,
    Height = 600
};

// SFML has tiny class for rendering purpose, it has only a one virtual fucntion 
class Paddel : public sf::Drawable
{
    // for member data perfered tobe started with m prefix 
    float mSpeed; // here is an optional you may use integer value
    sf::Vector2f mBorder;
    sf::Vector2f mPosition;
    sf::Vector2u mScreenSize;
    sf::RectangleShape mShape; // (mScreenSize.y - mBorder.y) - mShape.getSize().y)
            mPosition.y -= mSpeed;
        mShape.setPosition(mPosition);
    }

    // sf::Drawable has singe virtual fucntion for rendering, override it 
    void draw(sf::RenderTarget& target, sf::RenderStates states) const override
    {
        // sf::RectangleShape has its own defualt RenderStates
        target.draw(mShape, states);
    }
};

int main()
{
    // unfortunately strong type enumeration doesn't decay to integer value, so that we need to static cast it to integer type 
    // here we don't want to have a negative value for size, we should use unsigned int or use std::uint32_t which is declared 
    // in  header file if you considered C++11 features.
    sf::Vector2u screenSize(static_cast(ScreenSize::Width), static_cast(ScreenSize::Height));
    sf::RenderWindow Window(sf::VideoMode(screenSize.x, screenSize.y), "Ping Pong");

    Window.setFramerateLimit(60);
    Window.setVerticalSyncEnabled(true);

    Paddel playerOne(screenSize, Players::PlayerOne); 
    Paddel playerTwo(screenSize, Players::PlayerTow);

    const sf::Time TimePerFrame = sf::seconds(1.f / 60.f);
    sf::Clock clock;
    sf::Time timeSinceLastUpdate = sf::Time::Zero;
    sf::Time elapsedTime;

    bool play = true;

    while (Window.isOpen())
    {
        Window.clear();
        sf::Event Event;

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

        elapsedTime = clock.restart();
        timeSinceLastUpdate += elapsedTime;

        if (play)
        {
            if (timeSinceLastUpdate > TimePerFrame)
            {
                if (sf::Keyboard::isKeyPressed(sf::Keyboard::W))
                {
                    playerOne.moveUp();
                }
                if (sf::Keyboard::isKeyPressed(sf::Keyboard::S))
                {
                    playerOne.moveDown();
                }

                if (sf::Keyboard::isKeyPressed(sf::Keyboard::Up))
                {
                    playerTwo.moveUp();
                }
                if (sf::Keyboard::isKeyPressed(sf::Keyboard::Down))
                {
                    playerTwo.moveDown();
                }

                if (sf::Keyboard::isKeyPressed(sf::Keyboard::P))
                    play = false;
            }

            Window.draw(playerOne); // <-- we can draw Paddel object directly
            Window.draw(playerTwo);

            Window.display();
        }
    }
}

Code Snippets

#include <SFML/Graphics.hpp>
#include <cstdint>

// for eliminating magic numbers
enum class Players
{
    PlayerOne,
    PlayerTow
};

enum class ScreenSize
{
    Width = 800,
    Height = 600
};

// SFML has tiny class for rendering purpose, it has only a one virtual fucntion 
class Paddel : public sf::Drawable
{
    // for member data perfered tobe started with m prefix 
    float mSpeed; // here is an optional you may use integer value
    sf::Vector2f mBorder;
    sf::Vector2f mPosition;
    sf::Vector2u mScreenSize;
    sf::RectangleShape mShape; // <-- now it's private
public:

    Paddel(sf::Vector2u screenSize, Players player)
        // member data perfered be initilaized as contructor's initilaized list
        : mScreenSize(screenSize)
        , mBorder(8, 6)
        , mSpeed(3.f)
    {
        sf::Vector2f size = sf::Vector2f(20, 100);


        switch (player)
        {
        case Players::PlayerOne:
            mPosition = mBorder;
            break;
        default:
            mPosition.x = (mScreenSize.x - size.x) - mBorder.x;
            mPosition.y = mBorder.y;
        }

        mShape.setSize(size);
        mShape.setPosition(mPosition);
    }

    void moveUp()
    {
        mPosition.y -= mSpeed;
        if (mPosition.y < mBorder.y)
            mPosition.y += mSpeed;
        mShape.setPosition(mPosition);
    }

    void moveDown()
    {
        mPosition.y += mSpeed;
        //100 - (600 + 5)
        if (mPosition.y >(mScreenSize.y - mBorder.y) - mShape.getSize().y)
            mPosition.y -= mSpeed;
        mShape.setPosition(mPosition);
    }

    // sf::Drawable has singe virtual fucntion for rendering, override it 
    void draw(sf::RenderTarget& target, sf::RenderStates states) const override
    {
        // sf::RectangleShape has its own defualt RenderStates
        target.draw(mShape, states);
    }
};

int main()
{
    // unfortunately strong type enumeration doesn't decay to integer value, so that we need to static cast it to integer type 
    // here we don't want to have a negative value for size, we should use unsigned int or use std::uint32_t which is declared 
    // in <cstdint> header file if you considered C++11 features.
    sf::Vector2u screenSize(static_cast<std::uint32_t>(ScreenSize::Width), static_cast<std::uint32_t>(ScreenSize::Height));
    sf::RenderWindow Window(sf::VideoMode(screenSize.x, screenSize.y), "Ping Pong");

    Window.setFramerateLimit(60);
    Window.setVerticalSyncEnabled(true);

    Paddel playerOne(screenSize, Players::PlayerOne); 
    Paddel playerTwo(screenSize, Players::PlayerTow);

    const sf::Time TimePerFrame = sf::seconds(1.f / 60.f);
    sf::Clock clock;
    sf::Time timeSinceLastUpdate = sf::Time::Zero;
    sf::Time elapsedTime;

    bool play = true;

    while (Window.isOpen())
    {
        Window.clear();
        sf::Event Event;

        while (Window.pollEvent(Event))
        {
            if (Event.type == sf::Event::Closed)
                Window.close();
   

Context

StackExchange Code Review Q#80697, answer score: 2

Revisions (0)

No revisions yet.