SFML game using moving circles

This is my first SFML game. There are 2 players: the red circle and the yellow circle. The red circle is controlled by Up,down,left and right keys, the yellow one is controlled by WASD keys. The players are supposed to keep on trying to get behind each other in order to move them and get them inside the black circle, the player who gets in the circle first loses.

```
#include
#include

bool intersects(const sf::CircleShape& c1, const sf::CircleShape& c2){
sf::FloatRect circ1 = c1.getGlobalBounds();
sf::FloatRect circ2 = c2.getGlobalBounds();

return circ1.intersects(circ2);
}

int main(){

sf::VideoMode videomode(400, 400);
sf::RenderWindow window(videomode, "GAME");
sf::CircleShape circle;
circle.setFillColor(sf::Color::Red);
circle.setPosition(100, 100);
circle.setRadius(20);
sf::CircleShape circle2;
circle2.setFillColor(sf::Color::Yellow);
circle2.setPosition(200, 200);
circle2.setRadius(20);
sf::CircleShape circle3;
circle3.setFillColor(sf::Color::Black);
circle3.setPosition(300, 300);
circle3.setRadius(50);

while (window.isOpen()&&(!intersects(circle,circle3))&&(!intersects(circle2,circle3))){
window.clear(sf::Color::Blue);
window.draw(circle);
window.draw(circle2);
window.draw(circle3);

window.display();

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

if ((event.type == sf::Event::KeyPressed) && (event.key.code == sf::Keyboard::Up)){
circle.move(0, -0.5);

}

else if ((event.type == sf::Event::KeyPressed) && (event.key.code == sf::Keyboard::Down)){
circle.move(0, 0.5);

}

else if ((event.type == sf::Event::KeyPressed) && (event.key.code == sf::Keyboard::Right)){
circle.move(0.5, 0);

}

else if ((event.type == sf::Event::KeyPressed) &&

I do not know SFML so here some non SFML comments.

First of all - codding style.

-
You need to indent the code. It is very hard to read it.

-
Be consistent you are mixing coding styles - you are open { on different "places"

int main(){ <== open here

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

-
main() function is very long.

try separate the function into several small functions,

for example init_game, play_game end_game and so on.

• DRY - don't repeat yourself.

consider this code:

sf::CircleShape circle;
circle.setFillColor(sf::Color::Red);
circle.setPosition(100, 100);
circle.setRadius(20);

sf::CircleShape circle2;
circle2.setFillColor(sf::Color::Yellow);
circle2.setPosition(200, 200);
circle2.setRadius(20);

sf::CircleShape circle3;
circle3.setFillColor(sf::Color::Black);
circle3.setPosition(300, 300);
circle3.setRadius(50);

it is basically same thing three times. I would do:

// to do fix color_type
sf::CircleShape createCircleShape(color_type color, int x, int y, int r ){
   sf::CircleShape c;

   c.setFillColor(color);
   c.setPosition(x, y);
   c.setRadius(r);

   return c;
}

// ...
sf::CircleShape circle  = createCircleShape(sf::Color::Red, 100, 100, 20);
sf::CircleShape circle2 = createCircleShape(sf::Color::Yellow, 200, 200, 20);
sf::CircleShape circle3 = createCircleShape(sf::Color::Black, 300, 300, 20);

this way it is much more readable.

You can do same for controlling the keyboard:

if ((event.type == sf::Event::KeyPressed) && (event.key.code == sf::Keyboard::Up)){
    circle.move(0, -0.5);
}

you can write a function that get four keyboard codes, event and circle and you can do something like this:

void controlPlayer(const event_key_code &keycode, sf::CircleShape &circle,
           const key_code_type up,
           const key_code_type down,
           const key_code_type left,
           const key_code_type right);
// then you do:

if (event.type == sf::Event::KeyPressed){
        controlPlayer(event.key.code, circle, sf::Keyboard::Up, sf::Keyboard::Down, sf::Keyboard::Left, sf::Keyboard::Right);
        controlPlayer(event.key.code, circle2, sf::Keyboard::W, sf::Keyboard::S, sf::Keyboard::A, sf::Keyboard::D);
}

Hope this helps for start!

StackExchange Code Review Q#123891, answer score: 3