### Author Topic: [Code Review] Simple Breakout/Arkanoid Clone + Asking for some help  (Read 2227 times)

0 Members and 1 Guest are viewing this topic.

#### aaammsw

• Newbie
• Posts: 2
##### [Code Review] Simple Breakout/Arkanoid Clone + Asking for some help
« on: November 15, 2015, 11:53:22 am »
Hi, Im doing this small breakout clone for school, I've looked at heaps of people's takes on Breakout and tried to combine bits and pieces that I liked. Only problem that I'm having is getting the game to end when the ball hits the bottom window edge + sounds.. You should be able to copy/paste the code into a project, only issues being with the sound and text. Thanks a lot

#include <SFML/Graphics.hpp>
#include <SFML/Window.hpp>
#include <SFML/Audio.hpp>
#include <iostream>
using namespace std;
using namespace sf;
int x = 5;
constexpr int windowWidth ( 800 ), windowHeight( 600 );
constexpr float ballRadius( 10.f ), ballVelocity( 6.f );
constexpr float blockWidth( 60.f ), blockHeight( 20.f );
constexpr int countBlocksX( 11 ), countBlocksY( 6 );
constexpr int countBlocks2X(11), countBlocks2Y(3);
bool isPlaying = true;

struct Ball
{
CircleShape shape;
Vector2f velocity{ -ballVelocity, -ballVelocity };

Ball(float mX, float mY)
{
shape.setPosition(mX, mY);
shape.setFillColor(Color::Yellow);
}

void update()
{
//Need to make the ball bounce of the window edges
shape.move(velocity);
//If it's leaving on the left edge, we set a positive horizontal value.
if (left() < 0)
velocity.x = ballVelocity;
//Same for the right
else if (right() > windowWidth)
velocity.x = -ballVelocity;
//Top
if (top() < 0)
velocity.y = ballVelocity;
//And bottom
else if (bottom() > windowHeight)
velocity.y = -ballVelocity;

}

float x() { return shape.getPosition().x; }
float y() { return shape.getPosition().y; }
float left() { return x() - shape.getRadius(); }
float right() { return x() + shape.getRadius(); }
float top() { return y() - shape.getRadius(); }
float bottom() { return y() + shape.getRadius(); }
};

//Create the Rectangle shape class for the brick
struct Rectangle
{
RectangleShape shape;
float x()               { return shape.getPosition().x; }
float y()               { return shape.getPosition().y; }
float left()    { return x() - shape.getSize().x / 2.f; }
float right()   { return x() + shape.getSize().x / 2.f; }
float top()             { return y() - shape.getSize().y / 2.f; }
float bottom()  { return y() + shape.getSize().y / 2.f; }
};

{
//Create a variable for speed.
Vector2f velocity;
//Set the variables for the paddle rectangle shape.
{
shape.setPosition(mX, mY);
shape.setFillColor(Color::Red);
}
// Within the update function we check if the player is moving the paddle
void update()
{
shape.move(velocity);
//To ensure that the paddle stays inside the window we only change the Velocity when it's inside the boundaries
//Making it impossible to move outside when the initial velocity is set to zero
if (Keyboard::isKeyPressed(Keyboard::Key::Left) && left() > 0)

else if (Keyboard::isKeyPressed(Keyboard::Key::Right) && right() < windowWidth)
//If the player isn't pressing a buttom (legt/right) the velocity is set to zero.
else
velocity.x = 0;
}
};
//Another class for the bricks
struct Brick : public Rectangle
{
bool destroyed{ false };

Brick(float mX, float mY)
{
shape.setPosition(mX, mY);
shape.setSize({ blockWidth, blockHeight });
shape.setFillColor(Color::Black);
shape.setOrigin(blockWidth / 2.f, blockHeight / 2.f);
}
};

//C++ Feature template allows us to create a generic funtion to check if two shapes are intersecting or colliding.
template <class T1, class T2>
bool isIntersecting(T1& mA, T2& mB)
{
return mA.right() >= mB.left() && mA.left() <= mB.right() &&
mA.bottom() >= mB.top() && mA.top() <= mB.bottom();
}

{

mBall.velocity.y = -ballVelocity;
mBall.velocity.x = -ballVelocity;
else
mBall.velocity.x = ballVelocity;
}

void collisionTest(Brick& mBrick, Ball& mBall)
{
if (!isIntersecting(mBrick, mBall)) return;
mBrick.destroyed = true;

float overlapLeft{ mBall.right() - mBrick.left() };
float overlapRight{ mBrick.right() - mBall.left() };
float overlapTop{ mBall.bottom() - mBrick.top() };
float overlapBottom{ mBrick.bottom() - mBall.top() };

bool ballFromLeft(abs(overlapLeft) < abs(overlapRight));
bool ballFromTop(abs(overlapTop) < abs(overlapBottom));

float minOverlapX{ ballFromLeft ? overlapLeft : overlapRight };
float minOverlapY{ ballFromTop ? overlapTop : overlapBottom };

if (abs(minOverlapX) < abs(minOverlapY))
mBall.velocity.x = ballFromLeft ? -ballVelocity : ballVelocity;
else
mBall.velocity.y = ballFromTop ? -ballVelocity : ballVelocity;
}
int main()
{

//We render/create the window
RenderWindow window(VideoMode(windowWidth, windowHeight ), "Breakout Game" );
window.setFramerateLimit(60);

int x = 5;
//Here we use an unconditiional goto statement to allow the user to restart the game.
restart:
//We reference the Ball, Paddle and Bricks
Ball ball{ windowWidth / 2, windowHeight / 2 };
vector<Brick> bricks;
//vector<Brick2> bricks2;

for (int iX{ 0 }; iX < countBlocksX; ++iX)
for (int iY{ 0 }; iY < countBlocksY; ++iY)
bricks.emplace_back(
(iX + 1) * (blockWidth + 3) + 22, (iY + 2) * (blockHeight + 3));

sf::Font font;
return EXIT_FAILURE;

// Initialize the pause message
sf::Text loseGame;
loseGame.setFont(font);
loseGame.setCharacterSize(40);
loseGame.setPosition(80.f, 150.f);
loseGame.setColor(sf::Color::White);
loseGame.setString("You lost, press 'Space' to play again.");

// Load the sounds used in the game
sf::SoundBuffer ballSoundBuffer;
return EXIT_FAILURE;
sf::Sound loseGameSound(ballSoundBuffer);

while (true)
{
window.clear(Color::Color(49, 79, 79));

if (Keyboard::isKeyPressed(Keyboard::Key::Space))
goto restart;
if (Keyboard::isKeyPressed(Keyboard::Key::Escape))
break;

ball.update();
for (auto& brick : bricks) collisionTest(brick, ball);
bricks.erase(remove_if(begin(bricks), end(bricks),
[](const Brick& mBrick)
{
return mBrick.destroyed;
}),
end(bricks));

if (isPlaying)
{
window.draw(ball.shape);
for (auto& brick : bricks) window.draw(brick.shape);
}

else
{
window.clear(Color::Black);
// Draw the pause message
loseGameSound.play();
window.draw(loseGame);
if (Keyboard::isKeyPressed(Keyboard::Key::Space))
goto restart;

}

window.display();

}

return 0;
}

#### GraphicsWhale

• Full Member
• Posts: 131
##### Re: [Code Review] Simple Breakout/Arkanoid Clone + Asking for some help
« Reply #1 on: November 15, 2015, 06:27:16 pm »
using namespace std;
using namespace sf;

No.

//C++ Feature template allows us to create a generic funtion to check if two shapes are intersecting or colliding.
template <class T1, class T2>
bool isIntersecting(T1& mA, T2& mB)
{
return mA.right() >= mB.left() && mA.left() <= mB.right() &&
mA.bottom() >= mB.top() && mA.top() <= mB.bottom();
}

Using templates like this can be problematic. Aside from the fact that you'll probably end up with cryptic compiler errors at one point or another, you could just do something like this:

struct CollisionShape
{
CollisionShape(float left, float right, float top, float bottom) : left(left), right(right), top(top), bottom(bottom) { }
CollisionShape() { }
float left, right, top, bottom;
};

{
}

bool isIntersecting(const CollisionShape& l, const CollisionShape& r)
{
return l.right >= r.left && l.left <= r.right &&
l.bottom >= r.top && l.top <= r.bottom;
}

//Here we use an unconditiional goto statement to allow the user to restart the game.
restart:

Using a goto to restart the game is just bad program structure.

#### Elias Daler

• Hero Member
• Posts: 574
##### Re: [Code Review] Simple Breakout/Arkanoid Clone + Asking for some help
« Reply #2 on: November 15, 2015, 11:15:12 pm »
Your code is very good for beginner! Congrats. I like that you use some neat modern C++ stuff and most of your code is pretty readable.

But I can add more stuff to what GraphicsWhale said to improve your code.

1) Don't write code like this:
if(...)
expression
This style leads to some nasty bugs when you try to add another expression after this one and you end up like this:
if(...)
expression
expression2
This compiles fine, but it doesn't behave like it should.
So, always use braces for if, while and for statements even if they have a single expression.
if(...) {
expression
}

2) Your Ball struct should really be a class. You already defined some getters for most variables, why not make the rest private and define getters for them as well? The same goes for other structs.
And make member functions const, if you're not changing anything inside them.

3) Don't use magic numbers in your code. You've already defined a lot of constants, which is good, why not define constants for the rest of the numbers you use?
(iX + 1) * (blockWidth + 3) + 22, (iY + 2) * (blockHeight + 3));

4) Don't implicitly convert from int to float (or from any type!) as you do in this line, for example:
velocity.x = 0;
This line is not harmful, but implicit casting is a source of bugs in more complex code.

5) And, as GraphicsWhale noted, using goto is bad most of the time.
Define init() function instead. It will set all main variables to initial state and reset the game.
If you use goto, not only you make your code confusing, you do lots of unneccessary work reloading resources  and recreating objects where you can do less work.

6) Separate input, update and drawing. Your main loop should look like this:
while(isRunning) {
auto dt = getDelta();
handleInput();
update(dt);
draw(window);
}

This may eliminate some bugs and will give much cleaner structure to the whole program.

I highly recommend you to check out Effective C++ and Code Complete. This books are very important and will help you become a better programmer.

Good luck! Hope I wasn't being too harsh. Feel free to ask any questions about stuff you're not sure about/disagree with.
Tomb Painter, Re:creation dev | eliasdaler.github.io | @EliasDaler | Tomb Painter dev log