SFML Collision Detection :/

[ascii]

Hey guys, I was wondering if someone would be willing to look over my code and see if they can tell what's screwed up with my collision detection. For some reason 0, 0 in the game is being set to wherever I draw my first rectangle, and my code is checking for collision in the wrong place. I started learning SFML recently, so please help me figure out what's wrong! I apologize if it's messy, I've just started learning SFML and I'm not too great at C++ as well :/ There's a reasonable amount of code and I don't want to post it all here, but please just send me a message if you're experienced with SFML and can look it over, I'm really baffled here. Thank you!

[Disch]

Post a link to the source and I'll check it out. No guarantees that I'll be able to spot it though.

[ascii]

Where might I be able to upload it?

[Disch]

http://sfmluploads.org/

[ascii]

That was frustrating... kept cutting off the code so I made it two parts:
http://www.sfmluploads.org/index.php?page=view_code&isparent=yes&id=14
http://www.sfmluploads.org/index.php?page=view_code&id=37

Thank you Disch!

[Disch]

er... I just assumed you would zip up the files and upload that so I could just unzip it. That'd be easier for everyone. =x

This will work though, I guess.


* proceeds to copy/paste code to generate the files on his own *

EDIT: no, that won't work. Pasting retains the line numbers. I'm not about to delete all of them by hand.

Please zip it

[ascii]

Done: http://www.sfmluploads.org/index.php?page=view_file&file_id=18
Sorry about the inconvenience, I was trying to zip it before but sfmluploads doesn't work on my laptop for some reason, so I had to e-mail all the files to my netbook, and my security stuff doesn't like .exe and .cpp files so I had to go in an disable some stuff, so that's why I put in the straight source. Anyhow all's well that ends well :)

[Disch]

OK I finally figured out part of it.

SFML shapes have a position that's independent of where the actual shape is. That position is 0,0 by default.

You seem to be under the assumption that the position will be equal to the upper-left corner of the rectangle, which isn't true.

I don't know if this will help... but think of the shape as a sort of piece of transparent paper. The position of that piece of paper is 0,0 (top left of the screen). If you draw a rectangle at 50,50, it will be placed at 50,50 on the paper, but the paper itself is still at 0,0. Moving the paper around with SetPosition() / Move() would of course move the piece of paper (and therefore move the visible shape)


So really, the problem is, GetPosition is giving you the position of the paper, not the position of the shape drawn on the paper. If you want these to be the same, then the shape must be drawn at 0,0, and the paper moved to the desired position.

Change your CObject ctor to something like this:

CObject::CObject(float s, float w, float h, sf::Shape& obj, colRegion col, float x, float y)
    : speed(s), width(w), height(h), object(&obj), collision(col), startx(x), starty(y)
{
    // note we put it at 0,0,w,h
    *object = sf::Shape::Rectangle(0,0,w,h, sf::Color(100, 100, 100));
    // then move it to the desired position
    object->SetPosition(startx,starty);
}

(of course you'll need to update both ctors)


After that it appears to work for me.

Although you have some design issues =x for such a small program it's very messy and confusing.

[ascii]

Thank you! The paper anaology made everything make sense :) I didn't think the top left corner of the rectangle would be at 0, 0 however, but when I was looking at the coordinates of the shape in the console it said that the rectangle was at position 0, 0 when it was in the middle of the screen. This of course now makes sense thanks to your explanation :-)

As for the code being messy and confusing, I know it could be fewer lines, but I was trying to make it easier later to add in objects. Anyhow I apologize, I'm still pretty sucky at C++ :(
How do you recommend I make it less obfuscated?

[Disch]

Anyhow I apologize, I'm still pretty sucky at C++ :(

No need to apologize. We were all there at one time. You should've seen some of my early programs. They were pretty terrible.

How do you recommend I make it less obfuscated?

some things off the top of my head:


- Why does main own rectangle and rectangle2? Why does it even need them? Wouldn't it make more sense if CObject owned its own shape instead of just having a pointer to an outside shape?

- I was scratching my head as to what colRegion was supposed to do and why you needed to construct it for every collision check. If it represents a region of an object that is collidable (seems logical), then shouldn't each CObject own one to indicate their collision region?

- handleMovement(Input, pMove); instead of the more intuitive pMove->handleInput(Input);?


You're making the effort to do OOP, which is good, but you haven't quite gotten the idea yet. The idea is that classes should be self contained (ie: encapsulated). If a CObject needs to do something, then the CObject class should take care of it internally. If you have functions to manipulate CObject members outside of CObject it's a sign of poor encapsulation. That makes the code harder to follow.

[ascii]

Thanks so much for the advice and all the help, you've been awesome man! I've improved the OO design of the whole program (at least I think) in the ways that you've recommended so hopefully the whole thing is clear now (at least I think xD) I'll try to re upload next time I can, if you want to critique it more :/ CObject still has a lot of friends... I'm not sure how to fix it though

[Disch]

CObject still has a lot of friends... I'm not sure how to fix it though

Your problem here is that other classes are trying to do stuff with CObject members. You shouldn't do that. Instead you should have CObject do it.

Example:

// ill formed
void SomeOtherClass::foo(CObject& obj) // needs to be a friend
{
  DoSomething( obj.member );
}

// better
void CObject::foo()  // just have CObject do it
{
  DoSomething( member );
}

[ascii]

Well CMove needs some way to get access to the speed and object members. I was thinking I could either make CMove derive from CObject and make all of CObjects members protected (but I feel like it would be wasting a lot of it's inherited members), or I could just make accessor functions for CObject (but this would somewhat destroy its encapsulation).

[Disch]

I was thinking I could either make CMove derive from CObject and make all of CObjects members protected

My first instinct would be to derive CObject from CMove (although I would rename it to something like CMovableObject).

But this begs the question of what exactly CObject represents. What information do you need in CObject that wouldn't be part of CMove?

[ascii]

I'm working on it, but it's turning out to be a major pain in the ass -_- 99 stupid errors
EDIT: Yay it works!