Allegro.cc Forums » Programming Questions » Only 1 arrow at a time can collide

 Cassio Renan Member #14,189 April 2012 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.
 adamk kromm Member #5,432 January 2005 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. -----------Adam Kromm
 Fishcake Member #8,704 June 2007 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{ 3 for(int i = 0; i < size; i++) 4 { 5 if((arrow[i].x > enemy.x + enemy.width) || (arrow[i].y > enemy.y + enemy.height) || 6 (enemy.x > arrow[i].x + arrow[i].height) || (enemy.y > arrow[i].y + arrow[i].height)) 7 { 8 // do nothing, don't return! 9 } 10 else 11 { 12 // Immediately return after detecting a collision 13 return true; 14 } 15 } 16 17 // You'll only reach here if none of the arrows are colliding 18 // with your enemy. 19 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; ```
 Schyfis Member #9,752 May 2008 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)) ``` ________________________________________________________________________________________________________[freedwill.us][unTied Games]