Collision detection method

Question

I think everyone has some code they are embarrassed and not proud of and today I have decided to show mine. I'm not sure how to go about making this more efficient. At the time I was just happy it did what I wanted it to do and it's the first game I ever made. So basically, this is a collision detection method for my game and it describes which objects should collide and if they do, what should happen?

Any ideas on how to improve would be much appreciated :)

void collisionHandling(GameObject other) { //the following method details the outcome whenever one class comes into contact with another if ((this instanceof Heart && other instanceof Ship) && this.overlap(other)) { //increments ship lives if heart is hit by ship unless the ship has 5 lives Game.ship.dead = false; if (Ship.lives < 5) Game.ship.incLives(); this.hit(); } if ((this instanceof ShieldSprite && other instanceof Ship) && this.overlap(other)) { //makes the ship invulnerable if the sprite is hit and sets a counter till it runs out Game.ship.dead = false; Ship.invul = true; Game.shipInvulCounter = 400; this.hit(); } if ((this instanceof Enemy && other instanceof Bullet) && this.overlap(other)) { //if the enemy gets it by a bullet it gets hit this.hit(); } if ((this instanceof Enemy && other instanceof Ship) && this.overlap(other)) { //if the enemy gets it by a bullet it gets hit this.hit(); other.hit(); } if((this instanceof Enemy && other instanceof BigBullet) && this.overlap(other)){ //if the enemy gets it by a big bullet it dies instantly this.dead = true; other.hit(); } if((this instanceof BigBullet && other instanceof Asteroid) && this.overlap(other)){ //if a big bullet hits an asteroid they both get hit other.hit(); this.hit(); } if((this instanceof Bullet && other instanceof Asteroid) && this.overlap(other)){ //if a bullet hits an asteroid they both get hit other.hit(); this.hit(); } if((this instanceof EnemBigBullet && other instanceof Ship) && this.overlap(other)){ //if an enemy big bullet hits the ship it dies instantly other.dead = true; this.hit(); } //the following 'if' statements are so that if those two objects meet, they will simply glide over eachother if (!Ship.invul) { //if the ship is invulnerable everything glides over it if (!((this instanceof EnemBullet && other instanceof Asteroid) || (this instanceof Asteroid && other instanceof EnemBullet))) { //enemy bullets should not kill asteroids if (!(this instanceof Enemy || other instanceof Enemy)) { //enemies do not come into contact with anything other than what is mentioned above if(!(this instanceof EnemBigBullet || other instanceof EnemBigBullet)) {//enemy big bullets do not come into contact with anything other than what is mentioned above if (!((this instanceof Bullet && other instanceof Ship) || (this instanceof Ship && other instanceof Bullet))) { //stops the bullets killing the ship when they spawn if (!((this instanceof BigBullet && other instanceof Ship) || (this instanceof Ship && other instanceof BigBullet))) { //likewise with big bullets if (!(this instanceof ShieldSprite || other instanceof ShieldSprite)) { //nothing collides with shield sprite except ship as mentioned before if (!(this instanceof Heart || other instanceof Heart)) { //likewise for heart if (this.getClass() != other.getClass() && this.overlap(other)) { //otherwise if the classes are different then they will hit this.hit(); } } } } } } } } } } 

Answer 1

I think everyone has some code they are embarrassed and not proud of and today I have decided to show mine.

A pattern to get out of this mess ..

That's an impressive amount of nested if-statements. As you can see, the behavior of collision handling depends both on the current instance's type and the provided instance's type. There is a pattern suited for this kind of complexity: the Visitor Pattern.

GameObject

You first declare a base method in GameObject that redirects the generic collision handling to a specific method based on the type of other.

void collisionHandling (GameObject other) { if (this.overlap(other)) { if ((other instanceof Ship)) { collisionHandling((Ship)other); } else if ((other instanceof Bullet)) { collisionHandling((Bullet)other); } // and so on .. } } 

Derived Classes

Class Heart could then override any such method.

@Override void collisionHandling (Ship ship) { Game.ship.dead = false; if (Ship.lives < 5) Game.ship.incLives(); this.hit(); } 

Class ShieldSprite would override that method with different behavior.

@Override void collisionHandling (Ship ship) { Game.ship.dead = false; Ship.invul = true; Game.shipInvulCounter = 400; this.hit(); } 

By implementing this pattern, the cyclomatic-complexity of collision handling method gets reduced drastically and all logic sits at the right place, which makes the design object-oriented.