3
\$\begingroup\$

Edit 2: For anyone interested, you can play the game at buggysnake.com

Edit 1: I have typed up the code for making the body of the snake move. It's not perfect and there are small problems I need to fix. But I have updated the javascript code below to include that code.

Background: I recently started learning Javascript about 3 weeks ago. About a week ago I learnt about the Canvas API.

As for my first project, I decided to make a snake game using the Canvas API. As I am a beginner in both Javascript and the Canvas API, I am not sure if what I have typed up is good code. I would appreciate if someone could review it and provide some feedback regarding simplifying it and maybe generalising it to make the movements work with the entire body of the snake. As it is now, the only possible way I can think of moving the snake is by moving individual parts of it. There is still one piece missing in the code. That piece is the part where the body of the snake moves after the head has. I haven't been able to add that part in yet.

There are a total of three files. The HTML, CSS and Javascript files. I have typed them up in that order. Just in case if anyone wants to run the game on their own computer. But the code in review is the Javascript code.

index.html

<!DOCTYPE html>
<html lang="en-us">
  <head>
    <meta charset="utf-8">
    <meta name="viewport" content="width=device-width, initial-scale=1.0">
    <title>Canvas</title>
    <script src="script.js" defer></script>
    <link href="style.css" rel="stylesheet">
  </head>
  <body>
    <canvas class="myCanvas">
      <p>Something for now</p>
    </canvas>
  </body>
</html>

style.css

body {
  margin: 0;
  overflow: hidden;
}

script.js

const canvas = document.querySelector(".myCanvas");
const html = document.querySelector("html");
const width = canvas.width = window.innerWidth;
const height = canvas.height = window.innerHeight;
const ctx = canvas.getContext("2d");
let scoreCounter = 0;
let generalInstanceNumber = 0;
let current_x;
let current_y;

//An array for our snake body.
const snakes = [];

//An array for the keys pressed.
const keysPressed = [];

//The constructor for our snake.
function Snake(x, y,) {
    this.alreadyTurnedRight = true;
    this.alreadyTurnedLeft = true;
    this.alreadyTurnedUp = true;
    this.alreadyTurnedDown = true;
    this.x = x;
    this.y = y;
    this.velx = 0;
    this.vely = 0;
    this.position_x = 0;
    this.position_y = 0;
    this.instanceNumber;
    this.draw = function () {
        ctx.fillStyle = "green";
        ctx.fillRect(this.x, this.y, 50, 50);
    }
    this.updateRight = function () {
        if (this.x >= width) {
            gameStarted = false;
            gameOver();
        }
        else if (this.y === current_y) {
            this.velx = 0;
            this.velx += 10;
            this.x += this.velx;
            this.alreadyTurnedRight = false;
            this.alreadyTurnedLeft = true;
            this.alreadyTurnedUp = true;
            this.alreadyTurnedDown = true;
        }
        else {
            if (keysPressed[keysPressed.length - 2] === "ArrowUp" && this.alreadyTurnedRight === true) {
                this.y -= 10;
            }
            if (keysPressed[keysPressed.length -2] === "ArrowDown" && this.alreadyTurnedRight === true) {
                this.y += 10;
            }
        }
    }
    this.updateLeft = function () {
        if (this.x <= 0) {
            gameStarted = false;
            gameOver();
        }
        else if (this.y === current_y) {
            this.velx = 0;
            this.velx -= 10;
            this.x += this.velx;
            this.alreadyTurnedRight = true;
            this.alreadyTurnedLeft = false;
            this.alreadyTurnedUp = true;
            this.alreadyTurnedDown = true;
        }
        else {
            if (keysPressed[keysPressed.length - 2] === "ArrowUp" && this.alreadyTurnedLeft === true) {
                this.y -= 10;
            }
            if (keysPressed[keysPressed.length - 2] === "ArrowDown" && this.alreadyTurnedLeft === true) {
                this.y += 10;
            }
        }
    }
    this.updateUp = function () {
        if (this.y <= 0) {
            gameStarted = false;
            gameOver();
        }
        else if (this.x === current_x) {
            this.vely = 0;
            this.vely -= 10;
            this.y += this.vely;
            this.alreadyTurnedRight = true;
            this.alreadyTurnedLeft = true;
            this.alreadyTurnedUp = false;
            this.alreadyTurnedDown = true;
        }
        else {
            if (keysPressed[keysPressed.length - 2] === "ArrowRight" && this.alreadyTurnedUp === true) {
                this.x += 10;
            }
            if (keysPressed[keysPressed.length - 2] === "ArrowLeft" && this.alreadyTurnedUp === true) {
                this.x -= 10;
            }
        }
    }
    this.updateDown = function () {
        if (this.y >= height) {
            gameStarted = false;
            gameOver();
        }
        else if (this.x === current_x) {
            this.vely = 0;
            this.vely += 10;
            this.y += this.vely;
            this.alreadyTurnedRight = true;
            this.alreadyTurnedLeft = true;
            this.alreadyTurnedUp = true;
            this.alreadyTurnedDown = false;
        }
        else {
            if (keysPressed[keysPressed.length - 2] === "ArrowRight" && this.alreadyTurnedDown === true) {
                this.x += 10;
            }
            if (keysPressed[keysPressed.length - 2] === "ArrowLeft" && this.alreadyTurnedDown === true) {
                this.x -= 10;
            }
        }
    }
}

//Function for clearing the canvas for a new drawing.
function updateCanvas() {
    ctx.fillStyle = "black";
    ctx.fillRect(0, 0, width, height);
}

//Function for Game Over.
function gameOver() {
    rPressed = true;
    updateCanvas();
    ctx.fillStyle = "red";
    ctx.font = "70px helvetica";
    ctx.fillText("Game Over", width/2 - 200, height/2);
    ctx.fillStyle = "yellow";
    ctx.font = "45px helvetica";
    ctx.fillText("Press r to restart the game", width/2 - 280, height/2 + 100);
}

//Function for drawing the score.
function score() {
    ctx.fillStyle = "blue";
    ctx.font = "45px helvetica";
    ctx.fillText(`Score: ${scoreCounter}`, 10, 50);
}

//Code for setting up the starting screen.
ctx.fillStyle = "black";
ctx.fillRect(0, 0, width, height);
ctx.fillStyle = "Green";
ctx.font = "70px helvetica";
ctx.fillText("Snake Game", width/2 - 220, height/2);
ctx.fillStyle = "yellow";
ctx.font = "45px helvetica";
ctx.fillText("Press s to start the game", width/2 - 260, height/2 + 100);
let x = width/2;
let y = height/2;
const snake = new Snake(x, y);
snake.instanceNumber = 0;
snakes.push(snake);
let apple_x = randomNumber(0, width);
let apple_y = randomNumber(0, height);
const apple = new Apple(apple_x, apple_y);

//Function for starting the game.
let gameStarted = false;
addEventListener("keydown", setUpScreen);
function setUpScreen(event) {
    if (event.key === "s") {
        updateCanvas();
        snake.draw();
        apple.draw();
        score();
        gameStarted = true;
        removeEventListener("keydown", setUpScreen);
    }
    
}

//Code for restarting the game.
let rPressed = false;
addEventListener("keydown", restartGame);
function restartGame(event) {
    if (event.key === "r" && rPressed === true) {
        location.reload();
    }
}

//Code for performing animations based on user input.
let rightKeyPressed = true;
let leftKeyPressed = true;
let upKeyPressed = true;
let downKeyPressed = true;

addEventListener("keydown", moveSnake);
function moveSnake(event) {
    if (event.key === "ArrowRight" && gameStarted === true && rightKeyPressed === true) {
        keysPressed.push("ArrowRight");
        current_x = snake.x;
        current_y = snake.y;
        rightKeyPressed = false;
        leftKeyPressed = true;
        upKeyPressed = true;
        downKeyPressed = true;
        cancelAnimationFrame(requestLoopLeft);
        cancelAnimationFrame(requestLoopUp);
        cancelAnimationFrame(requestLoopDown);
        loopRight();
    }
    if (event.key === "ArrowLeft" && gameStarted === true && leftKeyPressed === true) {
        keysPressed.push("ArrowLeft");
        current_x = snake.x;
        current_y = snake.y;
        rightKeyPressed = true;
        leftKeyPressed = false;
        upKeyPressed = true;
        downKeyPressed = true;
        cancelAnimationFrame(requestLoopRight);
        cancelAnimationFrame(requestLoopUp);
        cancelAnimationFrame(requestLoopDown);
        loopLeft();
    }
    if (event.key === "ArrowUp" && gameStarted === true && upKeyPressed === true) {
        keysPressed.push("ArrowUp");
        current_x = snake.x;
        current_y = snake.y;
        rightKeyPressed = true;
        leftKeyPressed = true;
        upKeyPressed = false;
        downKeyPressed = true;
        cancelAnimationFrame(requestLoopLeft);
        cancelAnimationFrame(requestLoopRight);
        cancelAnimationFrame(requestLoopDown);
        loopUp();
    }
    if (event.key === "ArrowDown" && gameStarted === true && downKeyPressed === true) {
        keysPressed.push("ArrowDown");
        current_x = snake.x;
        current_y = snake.y;
        rightKeyPressed = true;
        leftKeyPressed = true;
        upKeyPressed = true;
        downKeyPressed = false;
        cancelAnimationFrame(requestLoopLeft);
        cancelAnimationFrame(requestLoopRight);
        cancelAnimationFrame(requestLoopUp);
        loopDown();
    }
} 

//Functions for animating the direction of the snake.
let requestLoopRight;
function loopRight(number) {
    updateCanvas();
    score();
    apple.draw();
    for (const element of snakes) {
        element.draw();
        element.updateRight();
    }
    apple.update();
    //console.log(`from loopRight ${snake.x}`);

    requestLoopRight = requestAnimationFrame(loopRight);
}

let requestLoopLeft;
function loopLeft() {
    updateCanvas();
    score();
    apple.draw();
    for (const element of snakes) {
        element.draw();
        element.updateLeft();
    }
    apple.update();
    //console.log(`From loopLeft ${snake.x}`);  

    requestLoopLeft = requestAnimationFrame(loopLeft);
}

let requestLoopUp;
function loopUp(name) {
    updateCanvas();
    score();
    apple.draw();
    for (const element of snakes) {
        element.draw();
        element.updateUp();
    }
    apple.update();
    //console.log(`From loopUp ${snake.y}`);

    requestLoopUp = requestAnimationFrame(loopUp);
}

let requestLoopDown;
function loopDown() {
    updateCanvas();
    score();
    apple.draw();
    for (const element of snakes) {
        element.draw();
        element.updateDown();
    }
    apple.update();
    //console.log(`From loopDown ${snake.y}`);

    requestLoopDown = requestAnimationFrame(loopDown);
}

//Function for generating a random integer.
function randomNumber(min, max) {
    return Math.floor(min + Math.random()*(max - min));
}

//The constructor for our apple.
function Apple(x, y) {
    this.x = x;
    this.y = y;
    this.draw = function () {
        ctx.fillStyle = "red";
        ctx.beginPath();
        ctx.arc(this.x, this.y, 28, 0, 2*Math.PI, false);
        ctx.fill();
    }
    //This function includes the code for collision detection and adding a square to the snakes body.
    this.update = function () {
        let x = 0;
        let y = 0;
        if (Math.sqrt(((snake.x + 25) - this.x)*((snake.x + 25) - this.x) + ((snake.y + 25) - this.y)*((snake.y + 25) - this.y)) <= 28) {
            this.x = randomNumber(0, width);
            this.y = randomNumber(0, height);
            scoreCounter += 1;
            generalInstanceNumber += 1;
            if (rightKeyPressed === false) {
                let arrayLength = snakes.length - 1;
                x = snakes[arrayLength].x - 50;
                y = snakes[arrayLength].y;
                const newSnake = new Snake(x, y);
                newSnake.instanceNumber = generalInstanceNumber;
                snakes.push(newSnake);
            }
            if (leftKeyPressed === false) {
                let arrayLength = snakes.length - 1;
                x = snakes[arrayLength].x + 50;
                y = snakes[arrayLength].y;
                const newSnake = new Snake(x, y);
                newSnake.instanceNumber = generalInstanceNumber;
                snakes.push(newSnake);
            }
            if (upKeyPressed === false) {
                let arrayLength = snakes.length - 1;
                x = snakes[arrayLength].x;
                y = snakes[arrayLength].y + 50;
                const newSnake = new Snake(x, y);
                newSnake.instanceNumber = generalInstanceNumber;
                snakes.push(newSnake);
            }
            if (downKeyPressed === false) {
                let arrayLength = snakes.length - 1;
                x = snakes[arrayLength].x;
                y = snakes[arrayLength].y - 50;
                const newSnake = new Snake(x, y);
                newSnake.instanceNumber = generalInstanceNumber;
                snakes.push(newSnake);
            }
        }
    }
}
Improve this question
\$\endgroup\$

1 Answer 1

Reset to default
1
\$\begingroup\$

Old school

The code looks very old school. No use of modern JS syntax (could be 8 year old code).

The comments in your code that contains pronoun "our" gives away the fact that you have copied an example, alway check the date of example code, and always use the most recent examples (within a year).

Stay up to date, especially when you are learning.

Writing a snake game

A snake game is has deceptively simple code complexity, however as the length of the snakes body grows the amount of work needed to update each frame of the animation quickly becomes overwhelming for the CPU.

Looking at your code the runtime complexity is \$O(n^2)\$ where \$n\$ is the number of snake body parts plus the number of apples. At worst, \$n\$ can be the number of playfield cells.

The naïve design starts with the snakes head and an apple, adding moving searchable snake segments, and searchable apples as the game progresses.

The better design considers a playfield (grid) of snake body parts and apples. Only the location of the snakes head and tail are tracked.

The best time complexity for a snake game is \$O(1)\$ (generally all classic games are time \$O(1)\$ and space \$O(n)\$)

Keep it D.R.Y.

D.R.Y. (Don't Repeat Yourself)

Your handling of directions is very repetitive.

There is no need.

Think of the 4 direction moves, as one move in a given direction.

Now you have one function to move, rather than 4 functions to move up, left, right, down.

Example

const Vec2 = (x = 0, y = 0) => ({x, y});
const directions = { 
  up:    Vec2( 0, -1), 
  right: Vec2( 1, 0), 
  down:  Vec2( 0,  1), 
  left:  Vec2(-1, 0)
};
const keys = ((...keyNames) => {
    const keys = {};
    for (const keyName of keyNames) { keys[keyName] = false }
    const keyEvents = e => { 
        keys[e.key] !== undefined && (keys[e.key] = e.type === "keydown"); 
    }
    addEventListener("keydown", keyEvents);
    addEventListener("keyup", keyEvents);    
    return keys;
})("ArrowUp", "ArrowRight", "ArrowDown", "ArrowLeft", "s", "r"); 
var gameOver = false;
function Snake(x, y) {
    const dirs = directions; // local alias dirs
    const pos = Vec2(x, y);
    var currentDir = Vec2();
    var velocity = 10;
    return Object.freeze({
        draw() {
            ctx.rect(pos.x, pos.y, 50, 50);
        },
        isGameOver() {
            if (pos.x >= width || pos.x < 0 || pos.y < 0 || pos.y >= height) {
                gameOver = true;
            }
        },
        move() {
            if (keys.ArrowUp) {
                currentDir !== dirs.down && (currentDir = dirs.up);
            } else if (keys.ArrowRight) {
                currentDir !== dirs.left && (currentDir = dirs.right);
            } else if (keys.ArrowDown) {
                currentDir !== dirs.up && (currentDir = dirs.down);
            } else if (keys.ArrowLeft) {
                currentDir !== dirs.right && (currentDir = dirs.left);
            }
            pos.x += currentDir.x * velocity;
            pos.y += currentDir.y * velocity;
            isGameOver();
        }
    });
}

Poor event use

Your use of events is way to complex for such a simple game. Imagine as the game gets more complex, following what is going on, testing and debugging would get very hard.

Always aim to minimise the number of listeners waiting for events. The more events there are the more you obfuscate logic flow!!!

I suspect you have already lost any idea of what is happening as it is evident that you are incorrectly requesting and canceling animation frames. Parts of your snake will simply stop moving.

Generally animations and games use one game loop. eg

function mainLoop(time) {
    if (gameOver) {
       showGameOver();
    } else {
       playGame();
    }
    requestAnimationFrame(mainLoop);
}

Too much this

Avoid the token this..

JavaScripts' this was not thought out (it's like a with without a name). Reading only part of the source code you can not know for sure what this refers to. this makes code harder to read and much noisier.

  1. Use closure to create private variables.
  2. Use named objects and reference the name rather than this

Comments can be evil

Comments can give the reader (and you when you come back months or years later to read the code) the wrong idea of the author's intent.

Example from your code

  // Function for generating a random integer. function
  randomNumber(min, max) {
    return Math.floor(min + Math.random()*(max - min)); 
  }

A number in javascript is not an integer, your intent is confused...

  • Did you want an integer or a number?
  • Is the function a bug and the comment a copy paste remnant
  • What is your intent the function name or the comment?

The only way to know is to find a use case within your code, thus the comment makes the code much harder to read and understand, the antithesis of what comment are there for.

The better option is to avoid comments and write code that is easily understood.

const randomInt = (min, max) => Math.floor(min + Math.random() * (max - min));

What is goin on?

I found some very strange bits of code, you assign value 0 and then add 10

 velx = 0; 
 velx += 10;

Why not just assign 10

velx = 10;
Improve this answer
\$\endgroup\$
6
  • 1
    \$\begingroup\$ Thanks for your reply. I’ll throughly read through your answer and reply soon. As for the first part, the code is not a copied example. I wrote those comments as if I was explaining the project to someone. Therefore, the use of the word “our”. Secondly, I’ve mostly been learning from this course on MDN web docs. I, being a beginner, thought it was up to date. Could you recommend any modern references to learn from? \$\endgroup\$
    – Seeker
    Jun 27 at 2:31
  • 1
    \$\begingroup\$ As for the integer part. I study mathematics and most often we say integers. So that was written out of habit. The code for the function is copied from elsewhere. \$\endgroup\$
    – Seeker
    Jun 27 at 2:31
  • 1
    \$\begingroup\$ Could you explain how the beginning code is old school? What kind of modern syntax should I be using? \$\endgroup\$
    – Seeker
    Jun 27 at 2:48
  • \$\begingroup\$ I do agree with your other points. I have had several ideas of reducing my code and making it simpler since I made this post. There are a few variables that are defined but never used. Or unnecessarily used. I was gonna remove the reassigning of velx and incrementing it every time from the code but forgot about that. \$\endgroup\$
    – Seeker
    Jun 27 at 2:55
  • \$\begingroup\$ If you don't mind, could you answer the questions I asked? \$\endgroup\$
    – Seeker
    Jun 29 at 4:22

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.