5

I have a vector holding 10 items (all of the same class for simplicity call it 'a'). What I want to do is to check that 'A' isn't either a) hiding the walls or b) hiding another 'A'. I have a collisions function that does this.

The idea is simply to have this looping class go though and move 'A' to the next position, if that potion is causing a collision then it needs to give itself a new random position on the screen. Because the screen is small, there is a good chance that the element will be put onto of another one (or on top of the wall etc). The logic of the code works well in my head - but debugging the code the object just gets stuck in the loop, and stay in the same position. 'A' is supposed to move about the screen, but it stays still!

When I comment out the Do while loop, and move the 'MoveObject()' Function up the code works perfectly the 'A's are moving about the screen. It is just when I try and add the extra functionality to it is when it doesn't work. 

    void Board::Loop(void){


        //Display the postion of that Element. 
        for (unsigned int i = 0; i <= 10; ++i){


            do {

                if (checkCollisions(i)==true){
                moveObject(i); 
                }
                else{
                    objects[i]->ResetPostion();

                }

            }
            while (checkCollisions(i) == false);
            objects[i]->SetPosition(objects[i]->getXDir(),objects[i]->getYDir());
        }

}

The class below is the collision detection. This I will expand later. 

    bool Board::checkCollisions(int index){

    char boundry = map[objects[index]->getXDir()][objects[index]->getYDir()];

    //There has been no collisions - therefore don't change anything 
    if(boundry == SYMBOL_EMPTY){
        return false;
    }
    else{
        return true;

    }

}

Any help would be much appreciated. I will buy you a virtual beer :-)

Thanks

Edit:

ResetPostion -> this will give the element A a random position on the screen moveObject -> this will look at the direction of the object and adjust the x and Y cord's appropriately.

Saro Taşciyan
  • 5,210
  • 5
  • 31
  • 50
KingJohnno
  • 602
  • 2
  • 12
  • 31
  • 3
    +1 for the virtual beer. – Maroun Apr 15 '13 at 18:41
  • doesn't `->SetPosition()` pretty much do the same thing as `moveObject()`? Perhaps remove one or the other from the code sample to avoid confusion? Also, why the infinite `while()**;**`? and even without the semicolon, why move `i` twice in one loop? –  Apr 15 '13 at 18:53
  • The 10 elements are in an array. The first index starts from 0 not from 1, doesn't it? Anyway what ResetPosition does? Nothing? – TrueY Apr 15 '13 at 18:54
  • Good point. Take your point on board. I will merge the two into one function. – KingJohnno Apr 15 '13 at 18:56

3 Answers3

2

I guess you need: 

do { ...
... } while (checkCollisions(i));

Also, if you have 10 elements, then i = 0; i < 10; i++

And btw. don't write if (something == true), simply if (something) or if (!something)

gcvt
  • 1,482
  • 13
  • 15
  • @gcvt You've inverted the while condition, `while (!checkCollisions(i));` would be the same as the original code. – john Apr 15 '13 at 19:02
  • I did that on purpose, since he suggested his code doesn't work, I assumed he wants to loop while there is a collision... – gcvt Apr 15 '13 at 19:08
  • Yup - while there is a collision, reset the postion of that element and test again. – KingJohnno Apr 15 '13 at 19:11
0
for (unsigned int i = 0; i <= 10; ++i){

is wrong because that's a loop for eleven items, use

for (unsigned int i = 0; i < 10; ++i){

instead.

You don't define what 'doesn't work' means, so that's all the help I can give for now.

john
  • 85,011
  • 4
  • 57
  • 81
  • When I run the code, the objects are displayed on screen, and then once it goes through the collision detection the screen remains blank. VERY odd.. I have narrowed it down to this code, as when I remove it from Main It works but without coliding. Added another paragraph! Thanks for the quick reply! – KingJohnno Apr 15 '13 at 18:57
  • Sounds like an infinite loop to me. Are you sure your do while loop terminates? – john Apr 15 '13 at 19:01
  • yes the loop goes through all 10, I have just put a break point in and tracked the value of (i) – KingJohnno Apr 15 '13 at 19:03
0

There seems to be a lot of confusion here over basic language structure and logic flow. Writing a few very simple test apps that exercise different language features will probably help you a lot. (So will a step-thru debugger, if you have one)

do/while() is a fairly advanced feature that some people spend whole careers never using, see: do...while vs while

I recommend getting a solid foundation with while and if/else before even using for. Your first look at do should be when you've just finished a while or for loop and realize you could save a mountain of duplicate initialization code if you just changed the order of execution a bit. (Personally I don't even use do for that any more, I just use an iterator with while(true)/break since it lets me pre and post code all within a single loop)

I think this simplifies what you're trying to accomplish:

void Board::Loop(void) {
    //Display the postion of that Element. 
    for (unsigned int i = 0; i < 10; ++i) {
        while(IsGoingToCollide(i))  //check is first, do while doesn't make sense
            objects[i]->ResetPosition();
        moveObject(i);   //same as ->SetPosition(XDir, YDir)?
                         //either explain difference or remove one or the other
    }
}

This function name seems ambiguous to me:

bool Board::checkCollisions(int index) {

I'd recommend changing it to:

// returns true if moving to next position (based on inertia) will 
// cause overlap with any other object's or structure's current location
bool Board::IsGoingToCollide(int index) {

In contrast checkCollisions() could also mean: 

// returns true if there is no overlap between this object's
// current location and any other object's or structure's current location
bool Board::DidntCollide(int index) {

Final note: Double check that ->ResetPosition() puts things inside the boundaries.

Community
  • 1
  • 1