patternjavascriptModerate
Square grid collision detection
Viewed 0 times
collisiongriddetectionsquare
Problem
var s = player.getSurrounding();
if (vY > 0) {
if ((s[0] || (s[7] && xLocal LOCAL_RIGHT)) && (yLocal + vY * dt) > LOCAL_RIGHT) {
vY = (LOCAL_RIGHT - yLocal) / dt;
}
} else if (vY LOCAL_RIGHT)) && (yLocal + vY * dt) LOCAL_RIGHT) || (s[6] && yLocal 0) {
if ((s[1] || (s[4] && yLocal > LOCAL_RIGHT) || (s[5] && yLocal LOCAL_RIGHT) {
vX = (LOCAL_RIGHT - xLocal) / dt;
}
}This might be a long shot, but I threw this together. It seems very inefficient and repetitive. It might be hard to explain, but here I go.
The world is made up of a grid of blocks that is saved in a 40x600 boolean array (true if filled, false if not).
The red block represents the player, a simple block that can fly in all directions. The position is defined by it's center point (orange).
The numbers represent the index of the boolean array
s, true if the block is filled and false if not. Filled blocks should be interacted with.xLocal and yLocal represents the position of the player in it's current grid position. If the player is all the way to the left:
xLocal = (playersize / 2) AKA LOCAL_LEFT. If the player is all the way to the right:
xLocal = blocksize - (playersize / 2) AKA LOCAL_RIGHT.Same applies for
yLocal but for up and down.dt represents the game's delta time.If the if-statements go through, the velocity is set so that
xLocal will equal LOCAL_LEFT or LOCAL_RIGHT by the next time the position is updated.Solution
Naming
Array ordering
Additionally, the order of the tiles seems strange to me. It's ...
Magic numbers
Continuing from my previous point, this can be solved by naming the directions in constants:
I'm calling it Magic Numbers because without the image, I'd be left wondering which index is which. They're magic in that if they change, the code stops working. But why it has to be those numbers exactly is unknown... to the reader, at least.
Difference so far
(Ckuhn also posted some naming changes, I suggest you add them in here as well.)
But Pim, if I do this, my if statements become very long!
Encapsulate Conditionals
Let's take a look at one of these paths.
if the vectorY is greater than 0... I guess that means if you're moving ... down... then...
if the bottom tile is solid or if either the bottom left tile is solid and the ... x something? or the bottom right tile is solid and x something but different...
It's way too confusing!
I can understand it, obviously, as I recognize the pattern (and you've given me an image for explanation). However, if I were to explain this code to someone else, I'd mentally recheck the code before continuing my explanation. I'd even manage to get it wrong at least once during my explanation ("and this checks for the upward movement, ... er, I mean the downward movement").
So, how can we fix that? Let's use English!
It's still not clear enough. That's because I have no idea what
Almost.
Nice. It just got wondrously clean.
Sadly, computers are grumpy things that want everything specified, so we'll have to explain our human-readable English to them.
At this point, you could consider shifting certain parts of the logic towards a function which does simple calculations. Your call. I'd prefer that to this inline logic, because if you use the functions, you can write this function on a different level of abstraction. That is, you can write English code consisting entirely of function calls. At that point, if you made a mistake, you'll spot it instantly. Because if you have a function that looks like this...
The
s is a poor name for a boolean array. I have no idea what s stands for. How about surroundingTiles?Array ordering
Additionally, the order of the tiles seems strange to me. It's ...
bottom, right, top, left, bottom right, top right, top left, bottom left. Without the image, I don't think I'd have gotten it. (For those wondering, it's non-diagonal, starting bottom, counter clockwise, then diagonal.) Any particular reason you have this layout within your array?Magic numbers
Continuing from my previous point, this can be solved by naming the directions in constants:
var BOTTOM_INDEX = 0;
var RIGHT_INDEX = 1;
var TOP_INDEX = 2;
var LEFT_INDEX = 3;
var BOTTOMRIGHT_INDEX = 4;
var TOPRIGHT_INDEX = 5;
var TOPLEFT_INDEX = 6;
var BOTTOMLEFT_INDEX = 7;I'm calling it Magic Numbers because without the image, I'd be left wondering which index is which. They're magic in that if they change, the code stops working. But why it has to be those numbers exactly is unknown... to the reader, at least.
Difference so far
var surroundingTiles = player.getSurrounding();
if (vY > 0) {
if ((surroundingTiles[BOTTOM_INDEX] || (surroundingTiles[BOTTOMLEFT_INDEX] && xLocal LOCAL_RIGHT)) && (yLocal + vY * dt) > LOCAL_RIGHT) {
vY = (LOCAL_RIGHT - yLocal) / dt;
}
} else if (vY LOCAL_RIGHT)) && (yLocal + vY * dt) LOCAL_RIGHT) || (surroundingTiles[TOPLEFT_INDEX] && yLocal 0) {
if ((surroundingTiles[RIGHT_INDEX] || (surroundingTiles[BOTTOMRIGHT_INDEX] && yLocal > LOCAL_RIGHT) || (surroundingTiles[TOPRIGHT_INDEX] && yLocal LOCAL_RIGHT) {
vX = (LOCAL_RIGHT - xLocal) / dt;
}
}(Ckuhn also posted some naming changes, I suggest you add them in here as well.)
But Pim, if I do this, my if statements become very long!
Encapsulate Conditionals
Let's take a look at one of these paths.
var surroundingTiles = player.getSurrounding();
if (vY > 0) {
if ((surroundingTiles[BOTTOM_INDEX] || (surroundingTiles[BOTTOMLEFT_INDEX] && xLocal LOCAL_RIGHT)) && (yLocal + vY * dt) > LOCAL_RIGHT) {
vY = (LOCAL_RIGHT - yLocal) / dt;
}if the vectorY is greater than 0... I guess that means if you're moving ... down... then...
if the bottom tile is solid or if either the bottom left tile is solid and the ... x something? or the bottom right tile is solid and x something but different...
It's way too confusing!
I can understand it, obviously, as I recognize the pattern (and you've given me an image for explanation). However, if I were to explain this code to someone else, I'd mentally recheck the code before continuing my explanation. I'd even manage to get it wrong at least once during my explanation ("and this checks for the upward movement, ... er, I mean the downward movement").
So, how can we fix that? Let's use English!
var surroundingTiles = player.getSurrounding();
if (movingDown) {
if ((surroundingTiles[BOTTOM_INDEX] || (surroundingTiles[BOTTOMLEFT_INDEX] && passingLeftTileBorder) || (surroundingTiles[BOTTOMRIGHT_INDEX] && passingRightTileBorder)) && (yLocal + vY * dt) > LOCAL_RIGHT) {
vY = (LOCAL_RIGHT - yLocal) / dt;
}It's still not clear enough. That's because I have no idea what
(yLocal + vY * dt) > LOCAL_RIGHT means! Additionally, we still didn't get it clear enough.var surroundingTiles = player.getSurrounding();
if (movingDown) {
if ((hittingBottomTile || hittingBottomLeftTile || hittingBottomRightTile) && (yLocal + vY * dt) > LOCAL_RIGHT) {
vY = (LOCAL_RIGHT - yLocal) / dt;
}Almost.
var surroundingTiles = player.getSurrounding();
if (movingDown) {
if (hittingTileInBottomDirection && (yLocal + vY * dt) > LOCAL_RIGHT) {
vY = (LOCAL_RIGHT - yLocal) / dt;
}Nice. It just got wondrously clean.
Sadly, computers are grumpy things that want everything specified, so we'll have to explain our human-readable English to them.
var movingDown = vY > 0;
if (movingDown) {
var hittingBottomLeftTile = (surroundingTiles[BOTTOMLEFT_INDEX] && xLocal LOCAL_RIGHT);
var hittingTileInBottomDirection = (surroundingTiles[BOTTOM_INDEX] || hittingBottomLeftTile || hittingBottomRightTile);
if (hittingTileInBottomDirection && (yLocal + vY * dt) > LOCAL_RIGHT) {
vY = (LOCAL_RIGHT - yLocal) / dt;
}At this point, you could consider shifting certain parts of the logic towards a function which does simple calculations. Your call. I'd prefer that to this inline logic, because if you use the functions, you can write this function on a different level of abstraction. That is, you can write English code consisting entirely of function calls. At that point, if you made a mistake, you'll spot it instantly. Because if you have a function that looks like this...
function checkForCollision(/*arguments omitted*/) {
checkCollisionInDirection(LEFT);
checkCollisionInDirection(RIGHT);
checkCollisionInDirection(TOP);
checkCollisionInDirection(LEFT);
}The
Code Snippets
var BOTTOM_INDEX = 0;
var RIGHT_INDEX = 1;
var TOP_INDEX = 2;
var LEFT_INDEX = 3;
var BOTTOMRIGHT_INDEX = 4;
var TOPRIGHT_INDEX = 5;
var TOPLEFT_INDEX = 6;
var BOTTOMLEFT_INDEX = 7;var surroundingTiles = player.getSurrounding();
if (vY > 0) {
if ((surroundingTiles[BOTTOM_INDEX] || (surroundingTiles[BOTTOMLEFT_INDEX] && xLocal < LOCAL_LEFT) || (surroundingTiles[BOTTOMRIGHT_INDEX] && xLocal > LOCAL_RIGHT)) && (yLocal + vY * dt) > LOCAL_RIGHT) {
vY = (LOCAL_RIGHT - yLocal) / dt;
}
} else if (vY < 0) {
if ((surroundingTiles[TOP_INDEX] || (surroundingTiles[TOPLEFT_INDEX] && xLocal < LOCAL_LEFT) || (surroundingTiles[TOPRIGHT_INDEX] && xLocal > LOCAL_RIGHT)) && (yLocal + vY * dt) < LOCAL_LEFT) {
vY = (LOCAL_LEFT - yLocal) / dt;
}
}
if (vX < 0) {
if ((surroundingTiles[LEFT_INDEX] || (surroundingTiles[BOTTOMLEFT_INDEX] && yLocal > LOCAL_RIGHT) || (surroundingTiles[TOPLEFT_INDEX] && yLocal < LOCAL_LEFT)) && (xLocal + vX * dt) < LOCAL_LEFT) {
vX = (LOCAL_LEFT - xLocal) / dt;
}
} else if (vX > 0) {
if ((surroundingTiles[RIGHT_INDEX] || (surroundingTiles[BOTTOMRIGHT_INDEX] && yLocal > LOCAL_RIGHT) || (surroundingTiles[TOPRIGHT_INDEX] && yLocal < LOCAL_LEFT)) && (xLocal + vX * dt) > LOCAL_RIGHT) {
vX = (LOCAL_RIGHT - xLocal) / dt;
}
}var surroundingTiles = player.getSurrounding();
if (vY > 0) {
if ((surroundingTiles[BOTTOM_INDEX] || (surroundingTiles[BOTTOMLEFT_INDEX] && xLocal < LOCAL_LEFT) || (surroundingTiles[BOTTOMRIGHT_INDEX] && xLocal > LOCAL_RIGHT)) && (yLocal + vY * dt) > LOCAL_RIGHT) {
vY = (LOCAL_RIGHT - yLocal) / dt;
}var surroundingTiles = player.getSurrounding();
if (movingDown) {
if ((surroundingTiles[BOTTOM_INDEX] || (surroundingTiles[BOTTOMLEFT_INDEX] && passingLeftTileBorder) || (surroundingTiles[BOTTOMRIGHT_INDEX] && passingRightTileBorder)) && (yLocal + vY * dt) > LOCAL_RIGHT) {
vY = (LOCAL_RIGHT - yLocal) / dt;
}var surroundingTiles = player.getSurrounding();
if (movingDown) {
if ((hittingBottomTile || hittingBottomLeftTile || hittingBottomRightTile) && (yLocal + vY * dt) > LOCAL_RIGHT) {
vY = (LOCAL_RIGHT - yLocal) / dt;
}Context
StackExchange Code Review Q#58739, answer score: 10
Revisions (0)
No revisions yet.