HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavaMinor

Collision detection method

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
collisiondetectionmethod

Problem

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 Aster

Solution

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.

Code Snippets

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 ..
    }
}
@Override
void collisionHandling (Ship ship) {
    Game.ship.dead = false;
    if (Ship.lives < 5) Game.ship.incLives();
    this.hit();
}
@Override
void collisionHandling (Ship ship) {
    Game.ship.dead = false;
    Ship.invul = true;
    Game.shipInvulCounter = 400;
    this.hit();
}

Context

StackExchange Code Review Q#163091, answer score: 2

Revisions (0)

No revisions yet.