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

Square grid collision detection

Submitted by: @import:stackexchange-codereview··
0
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

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.