This is my 3rd JavaScript canvas game. It's basically just snake right now but I am planning on adding some more aspects to it soon. I was wondering if I could get some feedback especially on how I'm checking if the snake crosses itself in the game function.
class Block {
constructor(x, y, w, h, col) {
this.x = x;
this.y = y;
this.w = w;
this.h = h;
this.col = col;
}
draw() {
ctx.fillStyle = "rgb" + this.col;
ctx.fillRect(this.x, this.y, this.w, this.h);
}
}
class Snake {
constructor(x, y, w, h, col) {
this.bod = [];
this.h = h;
this.w = w;
this.x = x;
this.killed = false;
this.y = y;
this.spd = 25;
this.vel = [0, 0];
this.bod.push(new Block(x, y, w, h, col))
}
draw() {
for (var x = 0; x < this.bod.length; x++) {
this.bod[x].draw();
}
}
move(tx, ty) {
this.bod[0].x += tx
this.bod[0].y += ty;
}
grow(pos_x, pos_y) {
this.bod.push(new Block(pos_x, pos_y, this.w, this.h, "(0, 255, 0)"));
}
update() {
for (var i = this.bod.length - 1; i > 0; i--) {
this.bod[i].x = this.bod[i - 1].x;
this.bod[i].y = this.bod[i - 1].y;
}
}
}
function init() {
start = false;
canvas = document.getElementById("display");
ctx = canvas.getContext("2d");
width = canvas.width;
height = canvas.height;
player = new Snake(width / 2, height / 2, 25, 25, "(25, 150, 25)")
food = new Block(Math.floor(Math.random() * 19 + 1) * 25, Math.floor(Math.random() * 19 + 1) * 25, 25, 25, "(255, 0, 0)");
addEventListener('keydown', keyDown, false);
menu_loop = setInterval(menu, 10)
}
function menu() {
ctx.clearRect(0, 0, width, height);
ctx.font = "75px Oswald";
ctx.textAlign = "center";
ctx.fillStyle = "rgb(0, 255, 0)";
ctx.fillText("Almost Snake!", width / 2, height - (height * 3 / 4));
ctx.font = "25px Oswald";
ctx.fillText("space to start", width / 2, height - height / 4);
if (start) {
clearInterval(menu_loop);
game_loop = setInterval(game, 100);
}
}
function eat() {
player.grow(food.x, food.y);
food.x = Math.floor(Math.random() * 19 + 1) * 25;
food.y = Math.floor(Math.random() * 19 + 1) * 25;
}
function die() {
clearInterval(game_loop);
ctx.clearRect(0, 0, width, height);
ctx.fillStyle = "rgb(255, 0, 0)";
ctx.textAlign = "center"
ctx.font = "75px Oswald";
ctx.fillText("you lose...", width / 2, height / 2);
setTimeout(function() {
location.reload();
}, 2000)
}
function keyDown(e) {
switch (e.keyCode) {
case 65:
case 37:
player.vel = [-player.spd, 0];
break;
case 87:
case 38:
player.vel = [0, -player.spd];
break;
case 68:
case 39:
player.vel = [player.spd, 0];
break;
case 83:
case 40:
player.vel = [0, player.spd];
break;
case 32:
start = true;
break;
}
}
function drawAll() {
player.draw();
food.draw();
ctx.textAlign = "left";
ctx.font = "25px Oswald";
ctx.fillText("Length: " + player.bod.length.toString(), 25, 25);
}
function game() {
player.update();
player.move(player.vel[0], player.vel[1]);
ctx.clearRect(0, 0, width, height);
for (var i = 1; i < player.bod.length; i++) {
if (player.bod[0].x == player.bod[i].x && player.bod[0].y == player.bod[i].y) {
player.killed = true;
}
}
if (player.bod[0].x > width - player.bod[0].w || player.bod[0].x < 0 || player.bod[0].y > height - player.bod[0].h || player.bod[0].y < 0) {
player.killed = true;
}
if (player.bod[0].x == food.x && player.bod[0].y == food.y) {
eat();
}
drawAll();
if (player.killed) {
clearInterval(game_loop);
player.update();
player.move(player.vel[0], player.vel[1]);
drawAll();
setTimeout(die, 1000);
}
}
window.onload = init;@import url('https://fonts.googleapis.com/css?family=Oswald');
html {
height: 100%;
display: grid;
}
body {
margin: auto;
background-color: black
}
#display {
border: 3px solid white;
}<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width">
<title>Almost Snake!</title>
<link href="style.css" rel="stylesheet" type="text/css" />
<link rel="icon" href="AS_ICO.png">
<script src="script.js"></script>
</head>
<body>
<canvas id="display" width="500" height="500"></canvas>
</body>
</html>1 Answer 1
I see Block and Snake have constructors that both accept five identical parameters. You could consider using inheritance to make a parent-class, which the two child classes inherit from via extends. This would be congruent with the Don't Repeat Yourself principle
Some of the variable are used globally - e.g. start, canvas, food, etc. You could at least limit the scope to an IIFE or a DOM ready callback.
I also see a few places where a for...of loop could be used instead of a traditional for loop. For instance, instead of this in Snake::draw():
for (var x = 0; x < this.bod.length; x++) { this.bod[x].draw(); }
A for...of loop could be used to simplify the array indexing:
for (const segment of this.bod) {
segment.draw();
}
I see that game_loop is assigned a timer from calling setInterval() in menu(), but it is cleared both in die() and game() - does it really need to be cleared in both places?
Explore related questions
See similar questions with these tags.