Square grid collision detection

Question

    var s = player.getSurrounding();
    if (vY > 0) {
        if ((s[0] || (s[7] && xLocal < LOCAL_LEFT) || (s[4] && xLocal > LOCAL_RIGHT)) && (yLocal + vY * dt) > LOCAL_RIGHT) {
            vY = (LOCAL_RIGHT - yLocal) / dt;
        }
    } else if (vY < 0) {
        if ((s[2] || (s[6] && xLocal < LOCAL_LEFT) || (s[5] && xLocal > LOCAL_RIGHT)) && (yLocal + vY * dt) < LOCAL_LEFT) {
            vY = (LOCAL_LEFT - yLocal) / dt;
        }
    }
    if (vX < 0) {
        if ((s[3] || (s[7] && yLocal > LOCAL_RIGHT) || (s[6] && yLocal < LOCAL_LEFT)) && (xLocal + vX * dt) < LOCAL_LEFT) {
            vX = (LOCAL_LEFT - xLocal) / dt;
        }
    } else if (vX > 0) {
        if ((s[1] || (s[4] && yLocal > LOCAL_RIGHT) || (s[5] && yLocal < LOCAL_LEFT)) && (xLocal + vX * dt) > 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.

Answer 1

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_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;
    }
}

(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_LEFT) || (surroundingTiles[BOTTOMRIGHT_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_LEFT);
    var hittingBottomRightTile = (surroundingTiles[BOTTOMRIGHT_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 bug is obvious.

I'll leave the location of variable definitions and the (yLocal + vY * dt) > LOCAL_RIGHT to other answerers.

Answer 2

I'll let someone smarter than myself help you with your actual problem. I'm going to talk about naming a little bit. Naming, you see, it's important (and hard). If your variables had more descriptive names, you might not have had to write several paragraphs explaining what they are. Poor Mr. Maintainer will not have the benefit of your explanation either. By time he sees this code, you've already moved on and he's left figuring out what's what; mapping short and cryptic names to meanings in his mind.

• vY & vX become vectorY & vectorX
• dt should be deltaTime
• xLocal & yLocal would be easier understood as playerX and playerY. Maybe even a little more verbosely as playerXCoord.
• I can't think of anything better for your array of booleans, but s should be replaced as well.