Collsions not working

[mattr]

I've got this code:

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

class Player : public sf::Transformable, public sf::Drawable {
    public:
        Player(int x, int y);
        ~Player() {};

        sf::RectangleShape p_rect;

        void doMovement(const sf::RenderWindow& window);
        sf::FloatRect getGlobalBounds() const;
    private:
        virtual void draw(sf::RenderTarget& target, sf::RenderStates states) const {
            states.transform *= getTransform();
            target.draw(p_rect, states);
        }

};

class Ball : public sf::Transformable, public sf::Drawable {
    public:
        Ball(int r, int x, int y);
        ~Ball() {};

        sf::CircleShape b_circle;

        void doXMovement();
        void doYMovement();
        bool doXCollisions(const Player& player);
        bool doYCollisions(const Player& player);
        sf::FloatRect getGlobalBounds() const;
    private:
        virtual void draw(sf::RenderTarget& target, sf::RenderStates states) const {
            states.transform *= getTransform();
            target.draw(b_circle, states);
        }

        bool right;
        bool up;
};

Player::Player(int x, int y) {
    p_rect = sf::RectangleShape(sf::Vector2f(x, y));
}

void Player::doMovement(const sf::RenderWindow& window) {
    setPosition(sf::Mouse::getPosition(window).x, 500);
    if (getPosition().x < 0)
        setPosition(0, 500);
    else if (getPosition().x > 720)
        setPosition(720, 500);
}

sf::FloatRect Player::getGlobalBounds() const {
    return getTransform().transformRect(p_rect.getGlobalBounds());
}

Ball::Ball(int r, int x, int y) {
    b_circle = sf::CircleShape(r);
    b_circle.setPosition(x, y);
    right = true;
    up = false;
}

void Ball::doXMovement() {
    if (right)
        move(1, 0);
    else
        move(-1, 0);
}

void Ball::doYMovement() {
    if (up)
        move(0, -1);
    else
        move(0, 1);
}

bool Ball::doXCollisions(const Player& player) {
    bool coll;
    if (getGlobalBounds().intersects(player.getGlobalBounds())) {
        right = !right;
        coll = true;
    } else
        coll = false;

    if (getPosition().x >= 800 - b_circle.getRadius())
        right = false;
    else if (getPosition().x <= 0)
        right = true;
    return coll;
}

bool Ball::doYCollisions(const Player& player) {
    bool coll;
    if (getGlobalBounds().intersects(player.getGlobalBounds())) {
        up = !up;
        coll = true;
    } else
        coll = false;
    if (getPosition().x <= 0)
        up = false;
    return coll;
}

sf::FloatRect Ball::getGlobalBounds() const {
    return getTransform().transformRect(b_circle.getGlobalBounds());
}

int main() {
    sf::RenderWindow window(sf::VideoMode(800, 600), "Breakout");
    window.setMouseCursorVisible(false);
    Player player(80, 10);
    Ball ball(3, 100, 100);
    sf::Clock clock;
    while (window.isOpen()) {
        sf::Event event;
        while (window.pollEvent(event)) {
            if (event.type == sf::Event::Closed)
                window.close();
        }
        player.doMovement(window);
        if (clock.getElapsedTime().asMilliseconds() >= 3) {
            clock.restart();
            if (!ball.doYCollisions(player))
                ball.doXCollisions(player);
            ball.doYMovement();
            ball.doXMovement();
        }
        window.clear(sf::Color::Black);
        window.draw(player);
        window.draw(ball);
        window.display();
    }
    return 0;
}
 

And for some reason my collisions aren't working. It's a simple breakout game with just the ball and player brick. When the ball collides with the player it should change direction. What am I doing wrong here? Also, any other coding criticisms would be welcome, I'm new to C++.

EDIT: Also, if something needs to be explained more, or the code commented, don't hesitate to say something.

[gostron]

One error I see is checking x positions in DoYCollision(), also no issue when y goes up ?. Another thing it the checking of the clock. You check if 3ms have passed in both X and Y movement, but why using the same clock and restarting it inside it ? you should check time passed in your main loop and if it has passed, restart it and do all your movements, because you might never move Y since X just restarted your clock

[mattr]

One error I see is checking x positions in DoYCollision(), also no issue when y goes up ?.

Oh man, that's pretty bad. Can't believe I missed that. Though that still doesn't explain why the collision with the player aren't working. That was simply for when the ball hit the top of the screen. The ball continues to fly straight through the player, as if they were never hit at all. Maybe I'll put some print messages in functions for debugging.

Another thing it the checking of the clock. You check if 3ms have passed in both X and Y movement, but why using the same clock and restarting it inside it ? you should check time passed in your main loop and if it has passed, restart it and do all your movements, because you might never move Y since X just restarted your clock

Yeah that was a typo, the restart call should only have been in the Y movement. I've moved it to the main loop like you suggested, as it's probably better off out  there anyway.

EDIT: After some more testing, the positions of the bounding boxes don't seem to be changing. The left and top values for both are sitting at 0, 0 and 100.016, 100 for the entire running time of the program. Am I not understanding something correctly? These should change as the move.

EDIT 2: Is it possible that I've implemented Transformable incorrectly? Do I need to somehow update the bounding box when the object is moved?

[gostron]

I'm pretty sure globalBounds is not your responsibility to update. In the tutorial http://sfml-dev.org/tutorials/2.0/graphics-transform.php It is said the globalBounds takes into consideration the position (as well as everything else) so it should change with your ball positions. It is very unlikely that this method doesn't work properly so I guess you should try to make a minimal code that uses getGlobalBounds() to be sure of its behaviour.

EDIT : why would you reimplement draw in ball ? You never use transformations. I'd say you should try and remove your reimplementation of the method draw because doing without it should work without any issue since you never use renderStates, you just use move(). I'm not sure about why you would multiply the renderstates with your ball transformation when you never actually use any of them.

[mattr]

I'm pretty sure globalBounds is not your responsibility to update. In the tutorial http://sfml-dev.org/tutorials/2.0/graphics-transform.php It is said the globalBounds takes into consideration the position (as well as everything else) so it should change with your ball positions. It is very unlikely that this method doesn't work properly so I guess you should try to make a minimal code that uses getGlobalBounds() to be sure of its behaviour.

EDIT : why would you reimplement draw in ball ? You never use transformations. I'd say you should try and remove your reimplementation of the method draw because doing without it should work without any issue since you never use renderStates, you just use move(). I'm not sure about why you would multiply the renderstates with your ball transformation when you never actually use any of them.

I thought it would be best to make my class transformable and drawable because I need it to do both of those things. I need it to move and be drawn on the screen. Drawable makes you implement draw, which I assume is called by window.draw(). What's the standard way to make classes that do what I want them to? It seems pretty straightforward in the documentation, and it doesn't seem reasonable to make wrappers around a Transformable for no reason.

http://www.sfml-dev.org/documentation/2.0/classsf_1_1Transformable.php
The detailed description show what kind of thing I'm going for here.

[gostron]

I'm not saying you shouldn't generally do that, I'm just saying you should remove that part for now so that we can see if this is the cause of the issue. If it is, it is probably because you are not using it correctly. I'm afraid I am not expert enough to help you using this however.

[mattr]

I'm not saying you shouldn't generally do that, I'm just saying you should remove that part for now so that we can see if this is the cause of the issue. If it is, it is probably because you are not using it correctly. I'm afraid I am not expert enough to help you using this however.

So I updated to code to do transforms directly on the shape members and that seems to have fixed it. Before I was applying them to a transform and then drawing the object with the transform applied to it, so the actual object never was updated, it was just drawn with respect to the continually updated transform. I still would like to do it the other way I had initially (and was described in the documentation), so is the best way to implement that to make my own getGlobalBounds function on my objects?

[gostron]

Can't help you with that sorry, let's hope someone will step in to give you the answers you seek :p

[mattr]

Can't help you with that sorry, let's hope someone will step in to give you the answers you seek :p

Ok, well thanks a lot for your help.

EDIT: I implemented my own simple getGlobalBounds functions for each of my objects. It looks like this:

Code: [Select]

sf::FloatRect Player::getGlobalBounds() const {
    return getTransform().transformRect(p_rect.getGlobalBounds());
}

It works pretty well, but I'm running into other problems. The getPosition function seems to be returning the incorrect value. It seems to be shifted over to the right and down. So the origin for the ball is somewhere in the middle of the screen. Any reason this might be happening? I've updated my code.

[mattr]

I stopped using getPosition (for some reason it was messed up). I just use getGlobalBounds everywhere and it seems to work.