SFML game using moving circles

Question

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 <iostream>
#include <SFML/Graphics.hpp>






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) && (event.key.code == sf::Keyboard::Left)){
            circle.move(-0.5, 0);





        }



        else if (sf::Keyboard::isKeyPressed(sf::Keyboard::W)){
            circle2.move(0, -0.5);


        }


        else if (sf::Keyboard::isKeyPressed(sf::Keyboard::S)){
            circle2.move(0, 0.5);


        }


        else if (sf::Keyboard::isKeyPressed(sf::Keyboard::D)){
            circle2.move(0.5, 0);


        }


        else if (sf::Keyboard::isKeyPressed(sf::Keyboard::A)){
            circle2.move(-0.5, 0);


        }







        if (intersects(circle, circle2)){
            circle2.move(0.5, 0);
        }
        if (intersects(circle2, circle)){
            circle.move(0.5, 0);

        }
        if (intersects(circle, circle3)){

            std::cout << "Red loses";
        }
        if (intersects(circle2, circle3)){

            std::cout << "Yellow loses";
        }
    }
}

Answer 1

I see some things that may help you improve your program.

Fix the formatting

There is a lot of vertical empty space. A little is good, but too much makes it hard to read and understand the program, so I'd advise removing most of the blank lines.

Eliminate "magic numbers"

This code is littered with "magic numbers," that is, unnamed constants such as -0.5, 20, 100, 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.

Use more descriptive variable names

All of the similar variable names make it difficult to follow the code. More meaningful variable names make the code easier to read and maintain, so instead of circle and circle2, you might call them redPlayer and yellowPlayer for example.

Make the game predictable

The game is a little confusing because no matter from which direction one circle "pushes" another, it always moves in the same direction. It would make things much more logical and less surprising if the pushed circle moved in the direction that the pushing circle was moving.

Use objects

The good news is that SFML already uses an object model, so making better use of ojbects for your own code is often just a matter of deriving from that code. For example, here's a simple adaptation of your game's primary object.

class Player : public sf::CircleShape {
public:
    Player(sf::Color color, float xpos, float ypos, float radius=20.0) 
    : sf::CircleShape(radius) 
    {
        setFillColor(color);
        setPosition(xpos, ypos);
    }
    bool intersects(const Player& other) const {
        sf::FloatRect box1 = getGlobalBounds();
        return box1.intersects(other.getGlobalBounds());
    }
    void setKeys(sf::Keyboard::Key upkey, sf::Keyboard::Key downkey, sf::Keyboard::Key leftkey, sf::Keyboard::Key rightkey) {
        K_up = upkey;
        K_down = downkey;
        K_left = leftkey;
        K_right = rightkey;
    }
    sf::Vector2<float> lastDirection() const {
        return lastdir;
    }
    bool handleKey() {
        if (sf::Keyboard::isKeyPressed(K_up)) {
            move(lastdir = up);
        } else if (sf::Keyboard::isKeyPressed(K_down)) {
            move(lastdir = down);
        } else if (sf::Keyboard::isKeyPressed(K_left)) {
            move(lastdir = left);
        } else if (sf::Keyboard::isKeyPressed(K_right)) {
            move(lastdir = right);
        } else {
            return false;
        }
        return true;
    }
private:
    sf::Keyboard::Key K_up, K_down, K_left, K_right;
    sf::Vector2<float> lastdir;
    const static sf::Vector2<float> up, down, left, right;
};
const sf::Vector2<float> Player::up{0, -0.5};
const sf::Vector2<float> Player::down{0, +0.5};
const sf::Vector2<float> Player::left{-0.5, 0};
const sf::Vector2<float> Player::right{+0.5, 0};

Now the main is much less cluttered and much easier to read.

int main()
{
    sf::VideoMode videomode(400, 400);
    sf::RenderWindow window(videomode, "GAME");
    Player player1{sf::Color::Red, 100, 100};
    player1.setKeys(sf::Keyboard::Up,
                    sf::Keyboard::Down,
                    sf::Keyboard::Left,
                    sf::Keyboard::Right);
    Player player2{sf::Color::Yellow, 200, 200};
    player2.setKeys(sf::Keyboard::W,
                    sf::Keyboard::S,
                    sf::Keyboard::A,
                    sf::Keyboard::D);
    Player blackHole{sf::Color::Black, 300, 300, 50};

    while (window.isOpen() && (!player1.intersects(blackHole))
           && (!player2.intersects(blackHole))) {
        window.clear(sf::Color::Blue);
        window.draw(player1);
        window.draw(player2);
        window.draw(blackHole);

        window.display();

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

        if (player1.handleKey() && player1.intersects(player2)) {
            player2.move(player1.lastDirection());
        }
        if (player2.handleKey() && player2.intersects(player1)) {
            player1.move(player2.lastDirection());
        }
        if (player1.intersects(blackHole)) {
            std::cout << "Red loses\n";
        }
        if (player2.intersects(blackHole)) {
            std::cout << "Yellow loses\n";
        }
    }
}

I didn't apply all of the suggestions I had made, but this is a start that may perhaps inspire you to go further.

Answer 2

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!