Only 1 arrow at a time can collide

no-reply@allegro.cc (DJLad16) — Wed, 13 Mar 2013 23:25:12 +0000

So I decided to put my arrows into a class array discarding the idea of limited arrows. But when I did that, only 1 arrow would collide with the enemy at once. I.e if I shot several arrows with none of them hitting an enemy, then I go to hit an enemy, it would go straight through without colliding.
When I run the program I get something saying "warnig C4175 : 'collideArrow' : not all control paths return a value". Has this got something to do with it?
(I've removed some code)

#SelectExpand
   1void initPlayer(Player &player);

   3void initEnemy(Enemy &enemy, int x, int y, int speedR, int speedL, int speed);
   4void enemyAnimtaion(Enemy &enemy, ALLEGRO_BITMAP *image);
   5void drawEnemy(Enemy &enemy, ALLEGRO_BITMAP *image);
   6void updateEnemy(Enemy &enemy, int posX, int posX2);
   7bool collideArrow(Enemy &enemy, Arrow arrow[], int size);
   8
   9
  10void initArrow(Arrow arrow[], int size);
  11void drawArrow(Arrow arrow[], int size, ALLEGRO_BITMAP *image, ALLEGRO_BITMAP *image2, Player &player);
  12void fireArrow(Arrow arrow[], int size, Player &player, ALLEGRO_SAMPLE *bowShot);
  13void updateArrow(Arrow arrow[], int size, Player &player);
  14
  15
  16bool collision(Player &player, int ex, int ey, int eWidth, int eHeight, int pWidth, int pHeight);
  17
  18void handleArrowCollision(Enemy &enemy, Arrow arrow[], Player &player, int size);
  19
  20void cameraUpdate(float *cameraPosition,Player &player, float width, float height);
  21
  22//Global Variables
  23
  24
  25int main()
  26{
  27
  28
  29

al_reserve_samples(5); //Respresents how many audio clips I have in the game
al_set_display_icon(display, icon);
  33
al_set_sample_instance_playmode(songInstance, ALLEGRO_PLAYMODE_LOOP);
al_attach_sample_instance_to_mixer(songInstance, al_get_default_mixer());
al_set_window_title(display, "Australian Outback");
al_register_event_source(event_queue, al_get_keyboard_event_source());
al_register_event_source(event_queue, al_get_timer_event_source(timer));
al_register_event_source(event_queue, al_get_timer_event_source(frameTimer));
al_register_event_source(event_queue, al_get_timer_event_source(enemyTimer));
al_register_event_source(event_queue, al_get_display_event_source(display));

al_play_sample_instance(songInstance);
  44
al_start_timer(timer);
al_start_timer(frameTimer);
al_start_timer(enemyTimer);
while(!done)
{
  ALLEGRO_EVENT event;
  al_wait_for_event(event_queue, &event);
  al_get_keyboard_state(&keystate);
    
  if(al_key_down(&keystate, ALLEGRO_KEY_ESCAPE))
  {
    done = true;
  }
  else if(event.type == ALLEGRO_EVENT_DISPLAY_CLOSE)
  {
    done = true;
  }
  
  
  else if(event.type == ALLEGRO_EVENT_TIMER)
  {
    if(event.timer.source == timer)
    {
      active = true;
      updateArrow(arrow, NUM_ARROW ,player);
      updateEnemy(enemy, 238, 78);
      updateEnemy(enemy2, 1236, 1167);
      updateEnemy(enemy3, 660, 378);
      updateEnemy(enemy4, 660, 378);
      updateEnemy(enemy5, 2163, 1599);
      updateEnemy(enemy6, 2163, 1599);
      updateEnemy(enemy7, 2163, 1599);
      updateEnemy(enemy8, 2163, 1599);
      updateEnemy(enemy9, 2163, 1599);
      updateEnemy(enemy10, 1506, 1306);
      frames++;
      if(al_current_time() - gameTime >= 1)
      {
        gameTime = al_current_time();
        gameFPS = frames;
        frames = 0;
      }
  87
      if(al_key_down(&keystate, ALLEGRO_KEY_SPACE))
      {
        fireArrow(arrow, NUM_ARROW, player, bowShot);
      }
      else
        fired = false;
      if(al_key_down(&keystate, ALLEGRO_KEY_D))
      {
        velX = player.speed;
        player.dir = RIGHT;
      
      }
      else if(al_key_down(&keystate, ALLEGRO_KEY_A))
      {
        velX = -player.speed;
        player.dir = LEFT;
      }
      else 
      {
        velX = 0;
        active = false;
      }
      if(al_key_down(&keystate, ALLEGRO_KEY_W) && jump)
      {
        velY = -jumpSpeed;
        jump = false;
      }
      
    }
      if(active)
      {
        sourceX += al_get_bitmap_width(character) / 3;
      }
      else 
      {
        sourceX = 32; 
      }
      if(sourceX >= al_get_bitmap_width(character))
      {
        sourceX = 0;
      }
        sourceY = player.dir;

      
    
      if(!jump)
        velY += gravity;
      else
        velY = 0;
        player.x += velX;
        player.y += velY;
    
        jump = (player.y >= groundHeight);
 141
        if(jump)
        {
              player.y = groundHeight;
        }  draw = true;
 146
        if(event.timer.source == enemyTimer)
        {
          enemyAnimtaion(enemy, enemyImg2);
          enemyAnimtaion(enemy2, enemyImg2);
          enemyAnimtaion(enemy3, enemyImg2);
          enemyAnimtaion(enemy4, enemyImg2);
          enemyAnimtaion(enemy5, enemyImg2);
          enemyAnimtaion(enemy6, enemyImg2);
          enemyAnimtaion(enemy7, enemyImg2);
          enemyAnimtaion(enemy8, enemyImg2);
          enemyAnimtaion(enemy9, enemyImg2);
          enemyAnimtaion(enemy10, enemyImg2);
        }
        
    
 162
    cameraUpdate(cameraPosition, player, 32, 32);
    al_identity_transform(&camera);
    al_translate_transform(&camera, -cameraPosition[0], -cameraPosition[1]);
    al_use_transform(&camera);
 167
  }
  
 170
  if(collideArrow(enemy, arrow, NUM_ARROW))//Arrow Collision with enemy
  {
    handleArrowCollision(enemy, arrow, player, NUM_ARROW);
  }
  if(collideArrow(enemy2, arrow, NUM_ARROW))//Arrow Collision with enemy
  {
    handleArrowCollision(enemy2, arrow, player, NUM_ARROW);
  }
  if(collideArrow(enemy3, arrow, NUM_ARROW))//Arrow Collision with enemy
  {
    handleArrowCollision(enemy3, arrow, player, NUM_ARROW);
  } 
  if(collideArrow(enemy4, arrow, NUM_ARROW))//Arrow Collision with enemy
  {
    handleArrowCollision(enemy4, arrow, player, NUM_ARROW);
  } 
  if(collideArrow(enemy5, arrow, NUM_ARROW))//Arrow Collision with enemy
  {
    handleArrowCollision(enemy5, arrow, player, NUM_ARROW);
  } 
  if(collideArrow(enemy6, arrow, NUM_ARROW))//Arrow Collision with enemy
  {
    handleArrowCollision(enemy6, arrow, player, NUM_ARROW);
  } 
  if(collideArrow(enemy7, arrow, NUM_ARROW))//Arrow Collision with enemy
  {
    handleArrowCollision(enemy7, arrow, player, NUM_ARROW);
  }
  if(collideArrow(enemy8, arrow, NUM_ARROW))//Arrow Collision with enemy
  {
    handleArrowCollision(enemy8, arrow, player, NUM_ARROW);
  } 
  if(collideArrow(enemy9, arrow, NUM_ARROW))//Arrow Collision with enemy
  {
    handleArrowCollision(enemy9, arrow, player, NUM_ARROW);
  } 
  if(collideArrow(enemy10, arrow, NUM_ARROW))//Arrow Collision with enemy
  {
    handleArrowCollision(enemy10, arrow, player, NUM_ARROW);
  } 
  if(collision(player, 1480, 295, 692, 305, 32, 32))//Mountain collision
  {
    player.x -= player.speed;
  }
  if(collision(player, 1505, 300, 580, 300, 32, 32))//Mountain collision 2
  {
    player.x = 10;
    player.y = 535;
    --player.lives;
  }
  if(collision(player, 2135, 535, 530, 50, 32, 32))//Quicksand colllision
  {
    player.x = 10;
    player.y = 535;
    --player.lives;
  }
 227
if(draw && al_is_event_queue_empty(event_queue))
{
  draw = false;
 231
  al_draw_bitmap(background2, 0, 0, 0);
  al_draw_bitmap_region(character, sourceX, sourceY * al_get_bitmap_height(character) / 2, 32, 32, player.x, player.y, 0);

  al_identity_transform(&identity);
  al_use_transform(&identity);
  al_draw_textf(font, al_map_rgb(252, 209, 22), 10, 10, 0, "Coins: %i", coinCount);
  al_draw_textf(font, al_map_rgb(252, 209, 22), 110, 10, 0, "Lives: %i", player.lives);
  al_draw_textf(font, al_map_rgb(252, 209, 22), 10, 45, 0, "FPS: %i", gameFPS);
  al_flip_display();
  al_clear_to_color(al_map_rgb(255, 255, 255));
 242
}
}
 245
 246}
 247void initPlayer(Player &player)
 248{
player.dir = LEFT;
player.x = 10;
player.y = 535;
player.lives = 3;
player.speed = 5;
player.score = 0;
player.dir = RIGHT;
 256}
 257
 258void initEnemy(Enemy &enemy, int x, int y, int speedR, int speedL, int speed)
 259{
enemy.live = true;
enemy.speedR = speedR;
enemy.speedL = speedL;
enemy.speed = speed;
enemy.x = x;
enemy.y = y;
enemy.dir = eLEFT;
enemy.eSourceX = 32;
enemy.eSourceY = 0;
enemy.acitve = true;
enemy.width = 29;
enemy.height = 29;
 272}
 273void drawEnemy(Enemy &enemy, ALLEGRO_BITMAP *image)
 274{
  if(enemy.live)
  {
    al_draw_bitmap_region(image, enemy.eSourceX, enemy.eSourceY * al_get_bitmap_height(image) / 2, 32, 32, enemy.x, enemy.y, 0);
  }    
 279}
 280void enemyAnimtaion(Enemy &enemy, ALLEGRO_BITMAP *image)
 281{
  if(enemy.acitve)
  {
    enemy.eSourceX += al_get_bitmap_width(image) / 3;
  }
  else
  {
    enemy.eSourceX = 0;  
  }
  if(enemy.eSourceX >= al_get_bitmap_width(image))
  {
    enemy.eSourceX = 0;
  }
    enemy.eSourceY = enemy.dir;
 295}
 296void updateEnemy(Enemy &enemy, int posX, int posX2)
 297{
if(enemy.live)
{
  enemy.x += enemy.speed;
    if(enemy.x >= posX)
    {
      enemy.dir = eLEFT;
      enemy.x = posX;
      enemy.speed = enemy.speedL;
    }
    if(enemy.x <= posX2)
    {
      enemy.dir = eRIGHT;
      enemy.x = posX2;
      enemy.speed = enemy.speedR;
    }
}
 314}
 315bool collideArrow(Enemy &enemy, Arrow arrow[], int size)
 316{
for(int i = 0; i < size; i++)
{
  if((arrow[i].x > enemy.x + enemy.width) || (arrow[i].y > enemy.y + enemy.height) ||
  (enemy.x > arrow[i].x + arrow[i].height) || (enemy.y > arrow[i].y + arrow[i].height))
  {
    return false;
  }
  return true;
}

 327}
 328
 329
 330void initArrow(Arrow arrow[], int size)
 331{
for(int i = 0; i < size; i++)
{
  arrow[i].speed = 5.0;
  arrow[i].live = false;
  arrow[i].width = 16;
  arrow[i].height = 5;
  arrow[i].fired = false;
  arrow[i].aDir = aRIGHT;
}
  
 342}
 343void drawArrow(Arrow arrow[], int size, ALLEGRO_BITMAP *image, ALLEGRO_BITMAP *image2, Player &player)
 344{
for(int i = 0; i < size; i++)
{
  if(arrow[i].live && arrow[i].aDir == aRIGHT && arrow[i].fired)
  {
      al_draw_bitmap(image, arrow[i].x, arrow[i].y, 0);
  }
  if(arrow[i].live && arrow[i].aDir == aLEFT && arrow[i].fired)
  {
    al_draw_bitmap(image2, arrow[i].x, arrow[i].y, 0);
  }
}  
 356}
 357void fireArrow(Arrow arrow[], int size, Player &player, ALLEGRO_SAMPLE *bowShot)
 358{
for(int i = 0; i < size; i++)
{
  if(!arrow[i].live && !fired)
  {
    --arrowCount;
    arrow[i].live = true;
    arrow[i].x = player.x + 17;
    arrow[i].y = player.y + 22;
    arrow[i].fired = true;
    fired = true;
    arrow[i].aDir = player.dir;    
    al_play_sample(bowShot, 1.0, 0.0, 1.0, ALLEGRO_PLAYMODE_ONCE, 0);
  }
}
 373}
 374void updateArrow(Arrow arrow[], int size, Player &player)
 375{
for(int i = 0; i < size; i++)
{
  if(arrow[i].live && arrow[i].fired)
  {
    if(arrow[i].aDir == aRIGHT)
    {
      arrow[i].x += arrow[i].speed;
      if(arrow[i].x >= 2990)
      {
        arrow[i].live = false;
      }
    }
    if(arrow[i].aDir == aLEFT)
    {
      arrow[i].x -= arrow[i].speed;
      if(arrow[i].x <= 5)
      {
        arrow[i].live = false;
      }
    }
  }
}    
 398}
 399
 400
 401bool collision(Player &player, int ex, int ey, int eWidth, int eHeight, int pWidth, int pHeight)
 402{
if((player.x > ex + eWidth - 10) || (player.y > ey + eHeight - 10) ||
  (ex > player.x + pWidth - 10) || (ey > player.y + pHeight - 10))
{
  return false;
}
return true;
 409}
 410void handlePlatCollision(Player &player, int ex, int ey, int ewidth, int eheight, int pwidth, int pheight, int &groundheight, float vely)
 411{
  if(player.y > ey)
  {
    player.y += vely;
    groundheight = ey;
    player.y = groundheight;
  }
 418}
 419void handleArrowCollision(Enemy &enemy, Arrow arrow[], Player &player, int size)
 420{
for(int i = 0; i < size; i++)
{
  enemy.live = false;
  enemy.x = -500;
  enemy.y = -500;
  arrow[i].live = false;
  ++player.score;
}
 429}
 430
 431}

no-reply@allegro.cc (Cassio Renan) — Wed, 13 Mar 2013 23:43:05 +0000

Look at your collideArrow function:

bool collideArrow(Enemy &enemy, Arrow arrow[], int size)
{
  for(int i = 0; i < size; i++)
  {
    if((arrow[i].x > enemy.x + enemy.width) || (arrow[i].y > enemy.y + enemy.height) ||
    (enemy.x > arrow[i].x + arrow[i].height) || (enemy.y > arrow[i].y + arrow[i].height))
    {
      return false;
    }
    return true;
  }
  
}

Let's pretend we are running it.
At the first loop, i == 0. At this loop, your code will test:

    if((arrow[0].x > enemy.x + enemy.width) || (arrow[0].y > enemy.y + enemy.height) ||
    (enemy.x > arrow[0].x + arrow[0].height) || (enemy.y > arrow[0].y + arrow[i].height))
    {
      return false;
    }
    return true;

If your first arrow is colliding, it will return true. If it is not, it will return false. There will never be a loop for i==1, or i >= 1, for that matter.

no-reply@allegro.cc (DJLad16) — Thu, 14 Mar 2013 00:03:09 +0000

Is there anyway to fix this because I cannot think of any solutions?

no-reply@allegro.cc (adamk kromm) — Thu, 14 Mar 2013 00:13:25 +0000

I would suggest you change your logic around a bit.

Currently you are checking to see if any of the arrow coordinates are outside of the enemy bounds.

if((arrow[i].x > enemy.x + enemy.width) || (arrow[i].y > enemy.y + enemy.height) ||
    (enemy.x > arrow[i].x + arrow[i].height) || (enemy.y > arrow[i].y + arrow[i].height))

Try checking to see if all the coordinates are within the enemy bounds instead. If they all are then return true, otherwise don't do anything. If you reach the end of the function then return false.

no-reply@allegro.cc (Fishcake) — Thu, 14 Mar 2013 05:55:48 +0000

As Casio said, the function returns after the collision check with the first arrow, regardless of whether there is a collision or not. What you really want is to have the function return after detecting a collision with an arrow or none of the arrows. This can be achieved with some modifications to your current code:

#SelectExpand
  1bool collideArrow(Enemy &enemy, Arrow arrow[], int size)
  2{
for(int i = 0; i < size; i++)
{
  if((arrow[i].x > enemy.x + enemy.width) || (arrow[i].y > enemy.y + enemy.height) ||
  (enemy.x > arrow[i].x + arrow[i].height) || (enemy.y > arrow[i].y + arrow[i].height))
  {
    // do nothing, don't return!
  }
  else 
  { 
    // Immediately return after detecting a collision
    return true;
  }
}
 16
// You'll only reach here if none of the arrows are colliding
// with your enemy.
return false;
 20}

However, IMO this is not a properly structured code, because having multiple return statements in a function is generally bad! (in regard to readability, especially when your function has a complex structure) And as adam said, you should really check if the arrow is within your enemy's bound instead, and not the otherwise. Something like this (pseudo code):

bool isColliding = false;
for (int i = 0; i < arrowCount; i++) {
   if (arrowIsCollidingWithEnemy) {
      isColliding = true;
      break;
   }
}

return isColliding;

no-reply@allegro.cc (Schyfis) — Thu, 14 Mar 2013 07:26:05 +0000

adamk kromm said:

I would suggest you change your logic around a bit.

Currently you are checking to see if any of the arrow coordinates are outside of the enemy bounds.

There's nothing wrong with this- checking if two bounding boxes overlap achieves the same result as checking if they do not overlap, just inverted.

Edit: misread some code.
I suggest at the very least that you rearrange the order of some things. For consistency, always start the condition with either arrow or enemy. It makes it more readable.

if((arrow[i].x > enemy.x + enemy.width) || (arrow[i].y > enemy.y + enemy.height) ||
    (arrow[i].x + arrow[i].height < enemy.x) || (arrow[i].y + arrow[i].height < enemy.y))

no-reply@allegro.cc (pkrcel) — Thu, 14 Mar 2013 12:53:25 +0000

Schyfis said:

There's nothing wrong with this- checking if two bounding boxes overlap achieves the same result as checking if they do not overlap, just inverted.

I noticed this is recurrent in the forum these days ...seems to generate a fair bit of confusion.

BTW, while I generally agree that "spamming" return statements in a function is BAD, I think that in a yes/no scenario is indeed BETTER to have a separate return statement with explicit true/false value passed in....'cause it's more readable.