Second attempt at canvas spatial grid collision

Question

After posting my first question and my first attempt which you can see here:

Canvas spatial grid collision

I have re-written it to try and implement what the person gave as an answer. I didn't do the prototype method as it was overkill for this and besides I am saving that for my next project!

$(document).ready(function() {
 var canvas = $('canvas')[0]; // canvas object
 var ctx = canvas.getContext('2d'); // canvas context
 var engine = new engine(new grid(new block().getSize())); // run the game
 window.addEventListener('keydown', keyPressDown, true); // event listener for keydown
 function keyPressDown(evt) {
 if(engine.activeBlock) { // block is falling
 switch(evt.keyCode) {
 case 37: // left arrow
 engine.pressLeft();
 break;
 case 39: // right arrow
 engine.pressRight();
 break;
 case 40: // down arrow
 engine.activeBlock.pressDown();
 break;
 }
 }
 }
 window.addEventListener('keyup', keyPressUp, true); // event listener for keyup
 function keyPressUp(evt) {
 if(engine.activeBlock) { // block is falling
 switch(evt.keyCode) {
 case 40:
 engine.activeBlock.releaseDown();
 break;
 }
 }
 }
 // block object
 function block() {
 this.x = 500; // x coord
 this.y = 0; // y coord
 this.size = 50; // size of block squared
 this.getSize = function() { // returns size of the block
 return this.size;
 };
 this.baseVelocity = 1; // base speed
 this.velocity = this.baseVelocity; // current speed
 this.genColor = function() { // random block color
 var colors = ['yellow', 'red', 'blue', 'green', 'orange', 'fuchsia'];
 return colors[Math.floor(Math.random() * colors.length)];
 };
 this.color = this.genColor(); // blocks color
 this.update = function() { // update blocks position
 this.y += this.velocity;
 };
 this.pressDown = function() { // key press down
 this.velocity += 1;
 };
 this.releaseDown = function() { // key release down
 this.velocity = this.baseVelocity;
 };
 this.moveLeft = function() {
 this.x -= this.size;
 };
 this.moveRight = function() {
 this.x += this.size;
 };
 }
 // grid object
 function grid(blockSize) {
 this.cellSize = blockSize; // cell size of grid
 this.cellsX = canvas.width / this.cellSize; // cells along x axis
 this.cellsY = canvas.height / this.cellSize; // cells along y axis
 this.buildGrid = function() { // returns built grid
 var arr = new Array(this.cellsX);
 for(var i = 0; i < arr.length; ++i) {
 arr[i] = new Array(this.cellsY);
 }
 return arr;
 };
 this.arr = this.buildGrid(); // holds grid
 this.getCoords = function(i) { // get relevant grid coords
 return Math.floor(i / this.cellSize);
 };
 this.checkDown = function(block) { // check grid for collisions down
 if(this.getCoords(block.y) == (this.cellsY - 1)) { // block has hit bottom of canvas
 return true;
 } else if(this.arr[this.getCoords(block.x)][this.getCoords(block.y) + 1]) { // block has landed on top of another block
 return true;
 }
 };
 this.checkLeft = function(block) { // check grid for collisions left
 if(this.getCoords(block.x) <= 0) { // block has hit left edge
 return true;
 } else if(this.arr[this.getCoords(block.x) - 1][this.getCoords(block.y)]) { // block is on left
 return true;
 } else if(this.arr[this.getCoords(block.x) - 1][this.getCoords(block.y) - 1]) { // block is on left
 return true;
 } else if(this.arr[this.getCoords(block.x) - 1][this.getCoords(block.y) + 1]) { // block is on left
 return true;
 }
 return false;
 };
 this.checkRight = function(block) { // check grid for collisions right
 if(this.getCoords(block.x) >= (this.cellsX - 1)) {
 return true;
 } else if(this.arr[this.getCoords(block.x) + 1][this.getCoords(block.y)]) {
 return true;
 } else if(this.arr[this.getCoords(block.x) + 1][this.getCoords(block.y) - 1]) {
 return true;
 } else if(this.arr[this.getCoords(block.x) + 1][this.getCoords(block.y) + 1]) {
 return true;
 }
 return false;
 };
 this.storeBlock = function(block) { // store block in grid
 block.x = this.cellSize * Math.floor(block.x / this.cellSize); // floor x coord to nearest cellSize multiple
 block.y = this.cellSize * Math.floor(block.y / this.cellSize); // floor y coord to nearest cellSize multiple
 this.arr[this.getCoords(block.x)][this.getCoords(block.y)] = block;
 };
 }
 // engine object
 function engine(grid) {
 this.framerate = 20; // framerate
 this.activeBlock = new block(); // active block
 this.nextBlock = new block(); // next block
 this.drawBlock = function(block) { // draw block onto canvas
 ctx.fillStyle = block.color;
 ctx.fillRect(block.x, block.y, block.size, block.size);
 ctx.fillStyle = 'rgba(0, 0, 0, 0.2)';
 ctx.fillRect(block.x, block.y, block.size, block.size);
 ctx.fillStyle = block.color;
 ctx.fillRect(block.x + (block.size / 10), block.y + (block.size / 10), block.size - ((block.size / 10) * 2), block.size - ((block.size / 10) * 2));
 };
 this.drawGrid = function() { // draw existing blocks stored in grid
 for(var i = 0; i < grid.arr.length; ++i) {
 for(var i2 = 0; i2 < grid.arr[i].length; ++i2) {
 if(grid.arr[i][i2]) {
 this.drawBlock(grid.arr[i][i2]);
 }
 }
 }
 };
 this.pressLeft = function() { // press left
 if(!grid.checkLeft(this.activeBlock)) { // check grid collision
 this.activeBlock.moveLeft(); // move left
 }
 };
 this.pressRight = function() { // press right
 if(!grid.checkRight(this.activeBlock)) { // check grid collision
 this.activeBlock.moveRight(); // move right
 }
 };
 this.resetCanvas = function() { // reset canvas background
 ctx.fillStyle = '#eee';
 ctx.fillRect(0, 0, canvas.width, canvas.height);
 };
 this.loop = function() { // the loop
 this.resetCanvas();
 this.drawGrid();
 this.drawBlock(this.activeBlock);
 this.activeBlock.update();
 if(grid.checkDown(this.activeBlock)) { // check grid collision down
 grid.storeBlock(this.activeBlock); // store block in grid
 this.activeBlock = this.nextBlock;
 this.nextBlock = new block();
 }
 };
 this.run = function() { // set interval
 setInterval(this.loop.bind(this), this.framerate); // bind the loop
 };
 this.run(); // run on instance
 }
});

Are there any more improvements that I could change to this? Any best practices that I haven't done?

You can view what the code does here.

Answer 1

Just as a general point to consider, you're making everything public. Consider your Block

function block() {
 this.x = 500; // x coord
 this.y = 0; // y coord
 this.size = 50; // size of block squared
 this.getSize = function() { // returns size of the block
 return this.size;
 };
}

Do you want someone to be able to change the size of your block? It doesn't look like you do.

var b = new block();
b.size = -5 // I'm pretty sure you don't want this to be possible.

Consider only making things public if you want other parts of your script to be able to change it.

function Block() {
 this.x = 500;
 this.y = 0; 
 var size = 50; 
 this.getSize = function() { 
 return size;
 };
}
var b = new Block();
b.size // doesn't exist -> people have to use getSize function.

Thinking about it fully, you'd probably want your x and y coordinates to be private too - it should only be possible to update it via your update function. If you're interested, check out this page: http://javascript.crockford.com/private.html

• ShaShads: Thanks my first game I coded like this was in java for android and was pretty basic, guess I didn't think about the differences coding in javascript which is available client side.
• RobH: @ShaShads it's a pretty good effort :). Another small point would be your keyPressUp function, there's no point in using a switch statement if you only care about one possible value. Also, you need to check evt.which as well as evt.keyCode as some browsers use one and not the other.
• ShaShads: Thanks for the info. I didn't even notice that about the keyPressUp function, I can guarantee that I copy/pasted it in a hurry to see if it worked from the keyDown functions!! Thanks buddy you have been a great help :)
• RobH: @ShaShads looks like I was getting confused with jQuery about the event.which: stackoverflow.com/a/4471691/1402923. No problem, have fun!