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

Simple JavaScript canvas game

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
javascriptgamesimplecanvas

Problem

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:



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);
c

Solution

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.

Code Snippets

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)

Context

StackExchange Code Review Q#3903, answer score: 7

Revisions (0)

No revisions yet.