Jump to content

  • Log In with Google      Sign In   
  • Create Account

Whats wrong with my Collision function?


Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.

  • You cannot reply to this topic
3 replies to this topic

#1 Crusable   Members   -  Reputation: 594

Like
1Likes
Like

Posted 27 December 2012 - 09:17 PM

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.


Edited by Mathew Bergen, 27 December 2012 - 09:22 PM.


Sponsor:

#2 Bacterius   Crossbones+   -  Reputation: 8945

Like
3Likes
Like

Posted 27 December 2012 - 10:07 PM

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.


The slowsort algorithm is a perfect illustration of the multiply and surrender paradigm, which is perhaps the single most important paradigm in the development of reluctant algorithms. The basic multiply and surrender strategy consists in replacing the problem at hand by two or more subproblems, each slightly simpler than the original, and continue multiplying subproblems and subsubproblems recursively in this fashion as long as possible. At some point the subproblems will all become so simple that their solution can no longer be postponed, and we will have to surrender. Experience shows that, in most cases, by the time this point is reached the total work will be substantially higher than what could have been wasted by a more direct approach.

 

- Pessimal Algorithms and Simplexity Analysis


#3 Servant of the Lord   Crossbones+   -  Reputation: 19665

Like
4Likes
Like

Posted 27 December 2012 - 11:15 PM

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.

 

 

 

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


Edited by Servant of the Lord, 27 December 2012 - 11:18 PM.

It's perfectly fine to abbreviate my username to 'Servant' rather than copy+pasting it all the time.
All glory be to the Man at the right hand... On David's throne the King will reign, and the Government will rest upon His shoulders. All the earth will see the salvation of God.
Of Stranger Flames - [indie turn-based rpg set in a para-historical French colony] | Indie RPG development journal

[Fly with me on Twitter] [Google+] [My broken website]

[Need web hosting? I personally like A Small Orange]


#4 KnolanCross   Members   -  Reputation: 1317

Like
1Likes
Like

Posted 28 December 2012 - 07:41 AM

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).





Old topic!
Guest, the last post of this topic is over 60 days old and at this point you may not reply in this topic. If you wish to continue this conversation start a new topic.



PARTNERS