6
\$\begingroup\$

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 <= bottomRight.y; row++)
            {
                if(map.getType(row, topLeft.x) == Tile.SOLID) 
                { 
                    collision = true; 
                    break;
                }
            }

            if(collision)
            {
                dx = 0;

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

        // Right
        if(dx > 0)
        {
            boolean collision = false;
            for(int row = topLeft.y; row <= bottomRight.y; row++)
            {
                if(map.getType(row, bottomRight.x) == Tile.SOLID) 
                { 
                    collision = true;
                    break;
                }
            }

            if(collision)
            {
                dx = 0;

                // Move player to edge of solid tile
                int col = ((int)(xtemp + dx) + collisionWidth/2) / map.getTileSize();
                xtemp = (col + 1) * map.getTileSize() - collisionWidth / 2 - 1;
            }
            else
            {
                xtemp += dx;
            }
        }
    }
\$\endgroup\$
5
\$\begingroup\$
  • 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;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.