patternjavaMinor
2D Platformer Collision Detection/Resolution
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
` // 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
which should be extracted to a method like
-
If a collision is found you have this
Do you on purpose set
-
You are also doing a lot of
-
you should extract the
-
in Java the convention is to place the opening brace
-
using a guard clause for
-
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
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.