patterncppMinor
Ping Pong game in SFML
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))
```
#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.