Whats wrong with my Collision function?

Started by
2 comments, last by KnolanCross 11 years, 3 months ago

void Move(){
	
	//Check if there is collision
	if(!(box.x >= SCREEN_W) && box.y >= SCREEN_H) {

		xDir = '+';
		yDir = '-';
	}

	if(!(box.y >= SCREEN_H) && box.x >= SCREEN_W) {

		xDir = '-';
		yDir = '+';
	}

	if(!(box.x <= 0) && box.y <= 0){

		xDir = '-';
		yDir = '+';
	}

	if(!(box.y <= 0) && box.x <= 0){

		xDir = '-';
		yDir = '+';
	}

	//Move
	if(xDir == '+'){

		box.x++;
	}

	else if(xDir == '-'){

		box.x--;
	}

	if(yDir =='+'){

		box.y++;
	}

	else if(yDir == '-'){

		box.y--;
	}

}

This is supposed to make a square bounce around the window. The first time it hits the bottom it works, then it goes right through.

Advertisement

The conditionals you chose are pretty difficult to read and understand logically. I suggest you refactor your code like this:


if (box.x <= 0) xDir = '+'; // box colliding with left wall, make it bounce right
if (box.y <= 0) yDir = '+'; // box colliding with top wall, make it bounce down
if (box.x >= SCREEN_W) xDir = '-'; // box colliding with right wall, make it bounce left
if (box.y >= SCREEN_H) yDir = '-'; // box colliding with bottom wall, make it bounce up

// move...

You don't need to keep the two xDir and yDir variables together - they can be separated, which considerably reduces the number of cases you have to handle.

“If I understand the standard right it is legal and safe to do this but the resulting value could be anything.”

In addition to what Bacterius said, 'char' also isn't the appropriate variable type for deciding whether you are moving up and down. chars are for written characters, like written numbers and letters and things. Neither is int appropriate, which is for numbers for math.

A decent choice would be a bool.


bool movingLeft = true; //If false, you are moving right.
bool movingUp = true; //If false, you are moving down.

A better choice would be enums.


enum HorizontalDirection{Right, Left};
enum VerticalDirection{Up, Down};
 
//Or:
enum Direction{NorthWest, North, NorthEast, East, SouthEast, South, SouthWest, West};

A choice better still would be enums with a bit flag, though the syntax may seem really weird at first.


enum Direction{None = 0, Up = 1, Down = 2, Left = 4, Right = 8};
typdef unsigned int DirectionFlags;
 
DirectionFlags direction = None;
 
direction |= Down; //Add to 'direction' that we are heading down.
direction |= Left; //Add to 'direction' that we are *also* heading left.
 
if(direction & Down) //Check if 'direction' contains 'Down' in it.

Any of those three are generally acceptable, but char is would be a very unusual choice to use there (even though it works) unless you have a special reason to use it. smile.png

Another option is to give velocities, and make the velocities negative or positive. A negative "vertical velocity" means you head up, while a positive one means you head down (or vice-versa):


int verticalVelocity = 0; //Not moving vertically.
verticalVelocity = 1; //Moving up.
verticalVelocity = 3; //Moving up really fast.
verticalVelocity = -1; //Moving down.
 
 
box.x += verticalVelocity; //Using the velocity (without caring whether it's zero, negative, or postive) to move the box.

[hr]

As Bacterius hinted at, the problem is that your if() collision checks for the walls are checking whether the ball is colliding with the bottom wall AND the side wall at the same time when you want the wall to bounce back even if it's only touching one wall. You also want to swap x and y individually, not together. (Also, your fourth if() statements you copy+pasted the symbols and forgot to swap the plus and minus).

Let's reread your if() statements:


//IF the box is NOT past the right side of the screen AND is below the bottom.
if(!(box.x >= SCREEN_W) && box.y >= SCREEN_H)
What happens if the box is past the right side of the screen? Why would you not reverse the vertical direction then?
There are nine different possible "states": (assuming the ball is smaller than the screen and can't touch the top and bottom at the same time).
  1. Not colliding with any wall.
  2. Colliding with the top
  3. Colliding with the bottom
  4. Colliding with the left
  5. Colliding with the right
  6. Colliding with the top AND right
  7. Colliding with the top AND left
  8. Colliding with the bottom AND right
  9. Colliding with the bottom and left

You only need to explicitly check for four states...

  1. Colliding right
  2. Colliding bottom
  3. Colliding left
  4. Colliding top

...but make each check not block the other states from being checked - in this situation, you don't use 'else' or 'if else', just 'if', so zero, one, or more 'if's can be triggered on the same run through.

Instead your code is currently checking for:

  1. "Colliding NOT right AND colliding bottom"
  2. "Colliding NOT bottom AND colliding right"
  3. "Colliding NOT left AND colliding top"
  4. "Colliding NOT top AND colliding right"

...which is over-complicating the matter, and in that complication, accidentally hides/disguises the fact that it's not actually checking all the possibilities. wink.png

Another important note, you are not considering the size of the object.

If x is the coord of the bottom-left a box, to check for collisions with the right part or the top part of the box you need to add the box size to it.

Currently working on a scene editor for ORX (http://orx-project.org), using kivy (http://kivy.org).

This topic is closed to new replies.

Advertisement