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

2D Platformer Collision Detection/Resolution

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

Problem

I am working on a 2D tile based platformer and was hoping to get someone to review my collision detection. I've only included the X collision check since the Y is essentially the same. Any suggestions/criticisms are appreciated.

` // Corrects X movement if a collision occurs.
private void checkXCollision()
{
// Find the range of tiles the player intersects
Point topLeft = new Point(((int)(xtemp + dx) - collisionWidth/2) / map.getTileSize(), ((int)ytemp - collisionHeight/2) / map.getTileSize());
Point bottomRight = new Point(((int)(xtemp + dx) + collisionWidth/2) / map.getTileSize(), ((int)ytemp + collisionHeight/2) / map.getTileSize());

// Left
if(dx 0)
{
boolean collision = false;
for(int row = topLeft.y; row

Solution

-
You have some code duplication there

for(int row = topLeft.y; row <= bottomRight.y; row++)
{
    if(map.getType(row, topLeft.x) == Tile.SOLID) 
    { 
        collision = true; 
        break;
    }
}


which should be extracted to a method like

private boolean hasCollision(int start, int stop, int x){

    for(int row = start; row <= stop; row++)
    {
        if(map.getType(row, x) == Tile.SOLID) 
        { 
            return true;
        }
    }
    return false;
 }


-
If a collision is found you have this

dx = 0;

// Move player to edge of solid tile
int col = ((int)(xtemp + dx) - collisionWidth/2) / map.getTileSize();


Do you on purpose set dx = 0 and calling then (xtemp + dx) ?

-
You are also doing a lot of collisionWidth/2 which should be extracted to a variable.

-
you should extract the left and right checking to separate methods.

-
in Java the convention is to place the opening brace { on the same line.

-
using a guard clause for dx == 0 well remove the creation of the Point's

-
comments should describe why something is done. **What is done should be described by the code itself by using meaningful names.

Implementing most the points above leads to

// Corrects X movement if a collision occurs.
private void checkXCollision()
{
    if (dx == 0) {
        return;
    }

    Point topLeft = new Point(((int)(xtemp + dx) - collisionWidth/2) / map.getTileSize(), ((int)ytemp - collisionHeight/2) / map.getTileSize());
    Point bottomRight = new Point(((int)(xtemp + dx) + collisionWidth/2) / map.getTileSize(), ((int)ytemp + collisionHeight/2) / map.getTileSize());

    if(dx < 0){

        checkLeftXCollision(topLeft, bottomRight);

    } else {

        checkRightXCollision(topLeft, bottomRight);

    }
}

private void checkLeftXCollision(Point topLeft, Point bottomRight) {

    if(hasCollision(topLeft.y, bottomRight.y, topLeft.x)) {
        dx = 0;

        int col = ((int)(xtemp + dx) - collisionWidth/2) / map.getTileSize();
        xtemp = col * map.getTileSize() + collisionWidth / 2;
    } else {
        xtemp += dx;
    }
}

private void checkRightXCollision(Point topLeft, Point bottomRight) {

    if(hasCollision(topLeft.y, bottomRight.y, bottomRight.x)) {
        dx = 0;

        int col = ((int)(xtemp + dx) + collisionWidth/2) / map.getTileSize();
        xtemp = (col + 1) * map.getTileSize() - collisionWidth / 2 - 1;
    } else {
        xtemp += dx;
    }
}    

private boolean hasCollision(int start, int stop, int x){

    for(int row = start; row <= stop; row++)
    {
        if(map.getType(row, x) == Tile.SOLID) 
        { 
            return true;
        }
    }
    return false;
}

Code Snippets

for(int row = topLeft.y; row <= bottomRight.y; row++)
{
    if(map.getType(row, topLeft.x) == Tile.SOLID) 
    { 
        collision = true; 
        break;
    }
}
private boolean hasCollision(int start, int stop, int x){

    for(int row = start; row <= stop; row++)
    {
        if(map.getType(row, x) == Tile.SOLID) 
        { 
            return true;
        }
    }
    return false;
 }
dx = 0;

// Move player to edge of solid tile
int col = ((int)(xtemp + dx) - collisionWidth/2) / map.getTileSize();
// Corrects X movement if a collision occurs.
private void checkXCollision()
{
    if (dx == 0) {
        return;
    }

    Point topLeft = new Point(((int)(xtemp + dx) - collisionWidth/2) / map.getTileSize(), ((int)ytemp - collisionHeight/2) / map.getTileSize());
    Point bottomRight = new Point(((int)(xtemp + dx) + collisionWidth/2) / map.getTileSize(), ((int)ytemp + collisionHeight/2) / map.getTileSize());

    if(dx < 0){

        checkLeftXCollision(topLeft, bottomRight);

    } else {

        checkRightXCollision(topLeft, bottomRight);

    }
}

private void checkLeftXCollision(Point topLeft, Point bottomRight) {

    if(hasCollision(topLeft.y, bottomRight.y, topLeft.x)) {
        dx = 0;

        int col = ((int)(xtemp + dx) - collisionWidth/2) / map.getTileSize();
        xtemp = col * map.getTileSize() + collisionWidth / 2;
    } else {
        xtemp += dx;
    }
}

private void checkRightXCollision(Point topLeft, Point bottomRight) {

    if(hasCollision(topLeft.y, bottomRight.y, bottomRight.x)) {
        dx = 0;

        int col = ((int)(xtemp + dx) + collisionWidth/2) / map.getTileSize();
        xtemp = (col + 1) * map.getTileSize() - collisionWidth / 2 - 1;
    } else {
        xtemp += dx;
    }
}    

private boolean hasCollision(int start, int stop, int x){

    for(int row = start; row <= stop; row++)
    {
        if(map.getType(row, x) == Tile.SOLID) 
        { 
            return true;
        }
    }
    return false;
}

Context

StackExchange Code Review Q#77717, answer score: 6

Revisions (0)

No revisions yet.