Simple JavaScript canvas game

Question

Here is a link to the code on JSFiddle.

This is my first attempt at playing with canvas. Before I move on doing anything else, it would be nice to have insight from somebody who knows canvas and JavaScript better than me.

Things I am looking for:

• Ways to optimize animation

• Ways to optimize the lazer drawing (I know I need to clear the lazers from the array every once in awhile when they are no longer within the drawing area, just haven't gotten around to it yet.)

• Ways to optimize the code in general and have good code re-use.

HTML:

<canvas id="world" style="height: 300px; width: 300px;" />

JavaScript:

console.log("Game starting...");
var ship = new Object();
ship.name = "Enterprise";
ship.x = 0;
ship.y = 0;
ship.width = 50;
ship.left = false;
ship.right = false;
ship.up = false;
ship.down = false;
ship.fire = false;
ship.firerate = 5;
ship.cfirerate = 0;
var lazers = new Array();
var world = document.getElementById('world');
var cxt = world.getContext("2d");
$(document).bind('keydown', function(e) {
 if(e.keyCode==37){
 ship.left = true;
 }
 if(e.keyCode==38){
 ship.up = true;
 }
 if(e.keyCode==39){
 ship.right = true;
 }
 if(e.keyCode==40){
 ship.down = true;
 }
 if(e.keyCode==90){ //Z
 console.log("pew pew");
 ship.fire = true;
 } 
});
$(document).bind('keyup', function(e) {
 if(e.keyCode==37){
 ship.left = false;
 }
 if(e.keyCode==38){
 ship.up = false;
 }
 if(e.keyCode==39){
 ship.right = false;
 }
 if(e.keyCode==40){
 ship.down = false;
 }
 if(e.keyCode==90){ //Z
 ship.fire = false;
 }
});
function createLazer(type) {
if (type == 1) {//LEFT LAZER
 cxt.beginPath();
 cxt.moveTo(125+ship.x,140+ship.y);
 cxt.lineTo(125+ship.x,130+ship.y);
 var l = new Object();
 l.type = type;
 l.x = ship.x;
 l.y = ship.y;
 return l;
}
else if (type == 2) {//RIGHT LAZER
 cxt.beginPath();
 cxt.moveTo(125+ship.x+ship.width,140+ship.y);
 cxt.lineTo(125+ship.x+ship.width,130+ship.y);
 var l = new Object();
 l.type = type;
 l.x = ship.x;
 l.y = ship.y;
 return l; 
}
}
function drawWorld() {
cxt.fillStyle="#808080";
cxt.fillRect(0,0,300,300);
}
function drawLazers() {
for (x = 0; x < lazers.length; x++)
{
 cxt.beginPath();
 cxt.strokeStyle="#FF0000";
 if (lazers[x].type == 1) {
 cxt.moveTo(125+lazers[x].x,140+lazers[x].y);
 cxt.lineTo(125+lazers[x].x,120+lazers[x].y);
 }
 else if (lazers[x].type == 2) {
 cxt.moveTo(125+lazers[x].x+ship.width,140+lazers[x].y);
 cxt.lineTo(125+lazers[x].x+ship.width,120+lazers[x].y);
 }
 cxt.stroke();
 lazers[x].y = lazers[x].y - 6;
 //console.log("drawing lazer" + lazers[x].x + lazers[x].y);
}
}
function drawShip() {
if (ship.left) { ship.x = ship.x -5; }
if (ship.right) { ship.x = ship.x +5; }
if (ship.up) { ship.y = ship.y -5; }
if (ship.down) { ship.y = ship.y +5; }
if (ship.fire) {
 if (ship.cfirerate == 0) {
 lazers.push(createLazer(1));
 lazers.push(createLazer(2));
 ship.cfirerate = ship.firerate;
 }
}
if (ship.cfirerate != 0) {
 ship.cfirerate = ship.cfirerate - 1;
}
cxt.beginPath();
cxt.strokeStyle="#000000";
cxt.moveTo(125+ship.x,140+ship.y);
cxt.lineTo(150+ship.x,120+ship.y);
cxt.lineTo(175+ship.x,140+ship.y);
cxt.stroke();
}
function clear() {
cxt.clearRect(0, 0, 300, 300);
}
function gameLoop() {
drawWorld();
drawShip();
drawLazers();
}
setInterval(function() {
clear();
gameLoop();
}, 30);

Answer 1

Cool program!

I have put my review of the code on JsFiddle.

A basic synopsis of what I thought to improve:

• Everything constant about the map, ship, lasers, and keycodes is all in one place to improve scalability.

• I used object literals and array literals instead of new Object() and new Array() because using them is shorter and and makes things easier to manipulate.

• The keydown and keyup event handlers were refactored to eliminate duplicate code.

• The createLaser and drawLasers methods were refactored. I removed some drawing code from createLaser because it didn't seem to do anything, and I removed calculations in drawLasers that were redundant with createLaser.

• I added code in drawLasers to remove lasers from the array that are no longer on the map. I also removed or rearranged drawing code that didn't do anything or was being called too many times. I removed the clear() function because it didn't seem to do anything.

• I changed statements of form x = x + y, x = x - y and x = x + 1 to x += y, x -= y, and x++, respectively.

• I changed one instance of the form array.push(x); array.push(y); to array.push(x,y);

• I renamed lazer to laser because I kept typing laser and it caused bugs that here hard to find. You can rename it back, if you are accustomed to typing lazer.

Here is a copy of the revised code:

console.log("Game starting...");
var ship = {
 name: "Enterprise",
 x: 125,
 y: 120,
 width: 50,
 height: 40,
 left: false,
 right: false,
 up: false,
 down: false,
 fire: false,
 firerate: 5,
 cfirerate: 0,
 moveInterval: 5,
 color: "#000000"
},
 map = {
 width: 300,
 height: 300,
 color: "#808080",
 drawInterval: 30
 },
 laser = {
 height: 20,
 moveInterval: 6,
 color: "#FF0000"
 },
 lasers = [],
 keys = {
 left: 37,
 up: 38,
 right: 39,
 down: 40,
 fire: 90 //Z
 },
 getKey = function(key) {
 for (var i in keys) {
 if (keys.hasOwnProperty(i)) {
 if (keys[i] === key) {
 return i
 };
 }
 }
 },
 eventValues = {
 keyup: false,
 keydown: true
 },
 types = {
 right: 1,
 left: 2
 };
var world = document.getElementById('world');
var cxt = world.getContext("2d");
$(document).bind('keydown keyup', function(e) {
 var key = getKey(e.keyCode);
 ship[key] = eventValues[e.type];
});
function createLaser(type) {
 var x = ship.x;
 if (type === types.right) {
 x += ship.width;
 }
 var y = laser.height + ship.y;
 return {
 type: type,
 x: x,
 y: y,
 }
}
function drawWorld() {
 cxt.fillStyle = map.color;
 cxt.fillRect(0, 0, map.width, map.height);
}
function drawLasers() {
 cxt.beginPath();
 cxt.strokeStyle = laser.color;
 for (var i = 0; i < lasers.length; i++) {
 var lsr = lasers[i];
 if (lsr.y < -laser.height) {
 lasers.splice(i, 1);
 continue;
 }
 cxt.moveTo(lsr.x, lsr.y);
 cxt.lineTo(lsr.x, lsr.y - laser.height);
 cxt.stroke();
 lsr.y -= laser.moveInterval;
 }
}
function drawShip() {
 if (ship.left) {
 ship.x -= ship.moveInterval;
 }
 if (ship.right) {
 ship.x += ship.moveInterval;
 }
 if (ship.up) {
 ship.y -= ship.moveInterval;
 }
 if (ship.down) {
 ship.y += ship.moveInterval;
 }
 if (ship.fire) {
 if (ship.cfirerate === 0) {
 lasers.push(createLaser(types.left), createLaser(types.right));
 ship.cfirerate = ship.firerate;
 }
 }
 if (ship.cfirerate !== 0) {
 ship.cfirerate--;
 }
 cxt.beginPath();
 cxt.strokeStyle = ship.color;
 cxt.moveTo(ship.x, ship.y + (ship.height / 2));
 cxt.lineTo(ship.x + (ship.width / 2), ship.y);
 cxt.lineTo(ship.x + ship.width, ship.y + (ship.height / 2));
 cxt.stroke();
}
function gameLoop() {
 drawWorld();
 drawShip();
 drawLasers();
}
setInterval(gameLoop, map.drawInterval)

If you see anything that you think is weird, or have a question about what I did, just ask me about it.

• Caimen: Theres something a little weird when shooting a single lazer sometimes. Sometimes the left lazer lags behind the right lazer. This codes looks 10 times better. Do you happen to know if this is the best way to produce this type of animation? Sometimes I experience just a little bit of flashing. Before I award you the points I am going to keep the question open so I can post an update later this weekend.
• Peter Olson: To be honest, I haven't played much with canvas (though it looks like something that would be really fun to mess around with) so I don't really know the fastest ways to do things. But I'm pretty sure the reason one laser lags behind the other is because one is created before the other. Try modifying the code to create them at the same time, instead of separately.
• Rune FS: first of all nicely done Due to the (weird) scoping rules of JavaSCript it's generally recommended to declare all variables in a function at the top. E.g. your lsr variable is not limited to the for loop` but actually has drawLasers as it's scope. In this particular function it's doesn't create any problems but it's a good practice because when it does create a bug it's usually extremely hard to track down and btw the same goes for the iteration variable (i.e. i)

Answer 2

It's more personal preference than any hard requirement, but I always prefer object literal syntax to individual value assignments. That would turn your initial declarations into this:

var ship = {
 name: "Enterprise",
 x: 0,
 y: 0.
 width: 50,
 left: false,
 right: false,
 up: false,
 down: false,
 fire: false,
 firerate: 5,
 cfirerate: 0
},
lazers = [];

Additionally, multiple variables can be declared with a single var statement if separated by commas, also as demonstrated.

You may also benefit from adding a boundary that prevents the Ship from moving off of the World. As it stands now you can lose it.