-1

I have a function that returns a boolean. this function when compiled seems to contain nothing, and will always return true while also skipping over all the calls to cout or cin that i put in it. to see what it's actually doing. What is going on and how do i fix this issue.

In my process of troubleshooting, i have,

  • used GDB with a breakpoint at object::collides, this resulted in the function being called but not outputting anything to the console
  • Numbered my objects to and compared what objects the program thinks are colliding to the objects that are colliding. if it passes the proximity test, the program thinks the objects are colliding, evidence that it is always returning true.
  • tried various other methods to try to figure out what is going on, but all have left my without answers

in object.cpp:

bool object::collides(object * other)
{
   std::vector<point> a_pnt = getBounds();
   std::vector<point> b_pnt = other->getBounds();
   for (int i = 0; i < a_pnt.size(); i++)
   {
       for (int j = 0; j < b_pnt.size(); j++)
       {
          point v1 = a_pnt[i];
          point v2 = a_pnt[(i+1)%a_pnt.size()];
          point v3 = b_pnt[j];
          //edit: fixed typo
          point v4 = b_pnt[(j+1)%b_pnt.size()];

          double num_1 = ((v3.x - v1.x) * -(v4.y - v3.y)) - (-(v4.x - v3.x) * (v3.y - v1.y));
          double num_2 = ((v2.x - v1.x) * (v3.y - v1.y)) - ((v3.x - v1.x) * (v2.y - v1.y));
          double den =((v2.x - v1.x) * -(v4.y - v3.y)) - (-(v4.x - v3.x) * (v2.y - v1.y));
          double frac_1 = num_1 / den;
          double frac_2 = num_2 / den;

          //debug code start
          std::cout << num_1 << "/" << den << "=" << frac_1 << std::endl;
          std::cout << num_2 << "/" << den << "=" << frac_2 << std::endl;
          std::cout << (frac_1 > 0.0) << " " << (frac_1 < 1.0) << " " << (frac_2 > 0.0) << " " << (frac_2 < 1.0) << std::endl;
          std::cout << std::endl;

          std::string hahah;
          std::cin >> hahah;
          //end debug code

          //edit: fixed conditional
          if((frac_1>0.0)&&(frac_1<1.0)&&(frac_2>0.0)&&(frac_2<1.0));
             return true;
       }
   }
   //edit: fixed conditional
   return false;
}

in mode.cpp in function mode::step():

for (int i = 0; i<onScreen.size(); i++)
{

    object * o1 = onScreen[i];
    for(int j = i+1; j<onScreen.size(); j++)
    {
        object * o2 = onScreen[j];
        if(o1->getVectorLength(o2)<50){

            std::cout << "Checking collisions for objects " << i << " and " << j << std::endl;

            if(o1->collides(o2))
            {
                 std::cout << "somthing collided\n";

            }
        }
    }
}

output:

Checking for Collisions

Checking collisions for objects 0 and 11
somthing collided
Checking collisions for objects 1 and 8
somthing collided
Checking collisions for objects 1 and 18
somthing collided
Checking collisions for objects 1 and 26
somthing collided

Expected results is for the "collides" function to output to the screen or request the input for that string, this will show that it is actually going through that section of code properly. however it doesn't do this. the "collides" function returns true regardless of whether or not the actual intersect section is true or false, while skipping over all of my debug code, as shown in the output.

edits:

  • fixed the return in collides
  • fixed a typo
  • still doesn't work.

  • does go thought loops with bullet/bullet combinations not bullet/asteroid or asteroid/asteroid

  • checking getBounds has me scratching my head...

    std::vector asteroid::getBounds() { //my issue was here, check your functions a bit more closely :P //wasn't returning a vector with anything in it. std::vector t; //now it's std::vector t = lyrs[0].pnts;

    for (int i = 0; i < t.size(); i++)
{
    double x = t[i].x+location.x;
    double y = t[i].y+location.y;
    t[i] = point{x, y, t[i].z};
}
return t;

    }

  • i thought that was implemented properly

TheWired
  • 1
  • 3
  • 2
    You unconditionally return in your inner `for` loop. That means you might as well not have a loop at all, you only check the first pair of values. You likely meant to conditionally return instead. – François Andrieux Jan 07 '19 at 18:23
  • Your code has _undefined behavior_ and the compiler should have warned you that not all paths of `object::collides()` return a value. – πάντα ῥεῖ Jan 07 '19 at 18:24
  • @πάνταῥεῖ It always returns provided the bounding boxes each have at least one point, which may be guaranteed. – François Andrieux Jan 07 '19 at 18:25
  • Asking without trying to read through your math: does your algorithm care about the order of your bounding box points (clockwise/counter-clockwise) and if so, is that requirement respected? – François Andrieux Jan 07 '19 at 18:32
  • @FrançoisAndrieux yes i did, fixed that, and still doesn't enter into the for loop at all. which is the main issue. – TheWired Jan 07 '19 at 18:32
  • @TheWired If the `for` loop is never entered, one of your bounding boxes must necessarily have no elements in it. Check how you define them and check that `getBoundingBox` works. – François Andrieux Jan 07 '19 at 18:33
  • `point v4 = b_pnt[(i+1)%b_pnt.size()];` probably meant to use `j` instead of `i`. – François Andrieux Jan 07 '19 at 18:36
  • @FrançoisAndrieux yeah getBoundingBox is actually implemented on all the objects so it shouldn't be returning nothing. one thing i did notice is it particularly hates comparing asteroids to other asteroids. i can get it to go through the loops when it's a bullet/asteroid, or bullet/bullet but not asteroid/asteroid – TheWired Jan 07 '19 at 18:37
  • Another tip for clarity, `-(x-y)` can just be written `y-x`. – François Andrieux Jan 07 '19 at 18:39
  • @FrançoisAndrieux you were right, my getBounds was returning an empty vector. which makes sense because i was messing with that function trying to fix something, so yeah. – TheWired Jan 07 '19 at 19:08
  • You should remove the ';' before `return true;`. – Werner Henze Jan 07 '19 at 19:51

2 Answers2

0

The problem occurs on this line:

if((frac_1>0.0)&&(frac_1<1.0)&&(frac_2>0.0)&&(frac_2<1.0)); //This semicolon here
    return true;

Putting a semicolon at the end of the if statement basically ends the if statement. What you wrote is equivalent to 

if((frac_1>0.0)&&(frac_1<1.0)&&(frac_2>0.0)&&(frac_2<1.0))
{
}
return true;

Fixing it is pretty simple. Just remove the semicolon:

if((frac_1>0.0)&&(frac_1<1.0)&&(frac_2>0.0)&&(frac_2<1.0))
    return true;
Alecto Irene Perez
  • 10,321
  • 23
  • 46
  • That was an edit after i figured out part of my real issue, It's a typo, thanks though, that would be an issue if it was the actual code (and it's not to far from what I originally did), sadly what my real issue was as I described at the end of my question, (and i really should post an answer) was that I wasn't actually returning a vector with anything in it for one of my objects. – TheWired Jan 15 '19 at 15:21
0

So my issue was simple, and a bit of a "doh" moment. First the unrealtated issue I had to fix, was a return true, regardless of whether or not my math was actually done correctly, but as this section wasn't being hit, this wasn't the real issue. Thanks for those that noticed it anyway.

Issue one (no if on return true):

In collides.cpp

for (int i = 0; i < a_pnt.size(); i++)
{
   for (int j = 0; j < b_pnt.size(); j++)
   {
       ...
       return true;
    }
 }

Fixed to:

for (int i = 0; i < a_pnt.size(); i++)
{
   for (int j = 0; j < b_pnt.size(); j++)
   {
       ...
      if((frac_1>0.0)&&(frac_1<1.0)&&(frac_2>0.0)&&(frac_2<1.0))
         return true;
    }
 }

The second issue, and the main one, after a suggestion from a commenter, his name is up in the question above, I double checked my getter for the bounding boxes. Low and behold, that was my issue. While at first I discounted his advice, because I thought I had fully implemented that getter, it was my issue and learning valuable lessons is always a good thing.

Issue two(incomplete implementation of GetBounds, resulted in empty vector getting returned.):

in asteroids.cpp:

std::vector asteroid::getBounds() 
{ 
    //my issue was here, check your functions a bit more closely :P 
    //wasn't returning a vector with anything in it. 
    std::vector<point> t; 
    //now it's 
    std::vector<point> t = lyrs[0].pnts;

    for (int i = 0; i < t.size(); i++)
    {
       double x = t[i].x+location.x;
       double y = t[i].y+location.y;
       t[i] = point{x, y, t[i].z};
    }
return t;
}

Lesson to learn: Even when you think you have everything working correctly there are times when you don't and you should check and double check EVERY function you are calling just incase one of those functions you think is working isn't actually working as it should.

TheWired
  • 1
  • 3