Snake game in JavaScript

Question

This is my first work in JS. It works but I would like to hear criticism of this code.

var canvas = document.getElementById('canvas'); //taking canvas by id
ctx = canvas.getContext('2d');
var pText = document.getElementById('score');
document.addEventListener("keydown", keyPush);
document.addEventListener("keydownCreate", keyPush);
ctx.fillStyle = "DarkGreen";
ctx.fillRect( 0, 0, canvas.width, canvas.height);
//-----------------------------------------------------------
var step = 35; //cells size
var score = 0;					 //really score
var maxX = canvas.width-step; //border for creating meal
var maxY = canvas.height-step;
var min = 0;
var meal = 0; //flag(meal existing) ...(in future meal count)
mealx = 0; //meal x pos
mealy = 0;
var key = "";					 //keycode
var brd = 5;					 //border to make snake cells smaller than another objects
var tail = 3;					 //tail size
var pos = []; 					 //tail position (pos[0] - head)
var px = 0, py = 0;				 //new head position
var bigHead = 0;				 //flag to make another head after eating 
var time = 90;					 //timer delay
for(var i = 0; i < tail; i++){	 //pushing start tail
	pos.push([i*step, 0]);
}
function game(){
	//clear canvas
	ctx.fillStyle = "DarkGreen";
	ctx.fillRect( 0, 0, canvas.width, canvas.height);
	key = "right";
	//start timer
	var timerId = setInterval(function() {
		if(key == "esc"){//exit			
			clearInterval(timerId);
			ctx.fillStyle = "red";
			ctx.fillRect( 0, 0, canvas.width, canvas.height);
			alert("game over");
		}
		if(key == "left" || key == "up" || key == "down" || key == "right"){//newt move
			newMeal();
			changePos();
			draw();
		}
	}, time);
}
function changePos(){//changing elements position
	for(var i = 1; i < tail; i++){
		if(pos[i][0]==pos[0][0] && pos[i][1] == pos[0][1]){
			for(var j = 3; j < tail; j++){
				pos[i].pop();
			} 
			tail = 3;
			score = 0;
		}
	}
	if(pos[0][0] == mealx && pos[0][1] == mealy){//eating 
		score++;
		meal = 0;
		tail++;
		bigHead = 1;
		pos.push([0, 0]);
	}
	for(var i = tail-1; i >= 1; i--){//new tail position
		pos[i][0] = pos[i-1][0];
		pos[i][1] = pos[i-1][1];
	}
	if(px == pos[0][0] && py == pos[0][1]){//if no changes
		if(key == "left")px-=step;
		if(key == "right")px+=step;
		if(key == "up")py-=step;
		if(key == "down")py+=step;
	}
	if(px<0)px = canvas.width-step;//new head position
	if(py<0)py = canvas.height-step;
	if(px>canvas.width-step)px = 0;
	if(py>canvas.height-step)py = 0;
	pos[0][0]=px;
	pos[0][1]=py;
}
function newMeal(){
	if(meal==0){//if no meal
		mealx = Math.floor(Math.random() * (maxX - min) + min);//new position
		mealy = Math.floor(Math.random() * (maxY - min) + min);
		
		while(mealx%step){//center
			mealx+=1;
		}
		while(mealy%step){
			mealy+=1;
		}
		meal = 1;//flag(there is meal)
	}
}
function draw(){//drawing canvas
	pText.innerHTML = "Score: " + score;
	ctx.fillStyle = "DarkGreen";//claer canvas
	ctx.fillRect( 0, 0, canvas.width, canvas.height);
	if(meal != 0){//draw meal		
		ctx.fillStyle = "red";
		ctx.fillRect( mealx, mealy, step-brd, step-brd);
	}
	
	if(bigHead == 0){//drawing head
		ctx.fillStyle = "lime";
		ctx.fillRect(pos[0][0], pos[0][1], step-brd, step-brd);
		ctx.fillStyle = "DarkGreen";
		ctx.fillRect(pos[0][0]+step/7, pos[0][1]+step/7, step/1.8, step/1.8	);	
	}else{
		ctx.fillStyle = "yellow";
		ctx.fillRect(pos[0][0], pos[0][1], step-brd, step-brd);
		ctx.fillStyle = "red";
		ctx.fillRect(pos[0][0]+step/7, pos[0][1]+step/7, step/1.8, step/1.8	);
		bigHead++;
		if(bigHead == 5){
			bigHead = 0;
		}
	}
	//drawing tail
	var clrG = 255;
	var clrR = 0;
	var str = "rgb(0, ";
	for(var i = 1; i < tail; i++){
		if(clrG<=0){
			ctx.fillStyle = "rgb("+clrR+", 0, 0)";
			clrR+=20;
		}
		else{
			ctx.fillStyle = "rgb(0, "+clrG+", 0)";
			clrG -= 20;
		}
		
		ctx.fillRect(pos[i][0], pos[i][1], step-brd, step-brd);
	}	
}
function keyPush(evt){//key codes
	if(evt.keyCode == 37){
		if(key != "right" && key != "left"){//to avoid turn around and to avoid *jumping* through cells
			key = "left";
			px-=step;//changing head position
		}		
	}
	if(evt.keyCode == 38){
		if(key != "down" && key != "up"){
			key = "up";
			py-=step;
		}
	}
	if(evt.keyCode == 39){
		if(key != "left" && key != "right"){
			key = "right";
			px+=step;
		}
	}
	if(evt.keyCode == 40){
		if(key != 'up' && key != "down"){
			key = "down";
			py+=step;
		}
	} 
	if(evt.keyCode == 27){
		key = "esc";
	}
	if(evt.keyCode == 13){
		key = "enter";
	}
}
function upButton(){
	if(key != "down" && key != "up"){
		key = "up";
		py-=step;
	}
}
function downButton(){
	if(key != 'up' && key != "down"){
		key = "down";
		py+=step;
	}
}
function leftButton(){
	if(key != "right" && key != "left"){
		key = "left";
		px-=step;
	}	
}
function rightButton(){
	if(key != "left" && key != "right"){
		key = "right";
		px+=step;
	}
}

<!DOCTYPE html>
<html>
<head>
	<title>SnakeQQ</title>
	<style type="text/css">
		input{
			margin:10px;
			width:50px;
			height:40px;
		}
		.control{
			width:100px;
			height:60px;
		}
		p{
			text-align: -webkit-center;
			font-size:40px;
			margin: 0px;
		}
	</style>
</head>
<body>
	<p id="score">
		Score: 0
	</p>
	<div style="display: -webkit-inline-box">
		<canvas id="canvas" width="1050" height="560" ></canvas>
		<div>
			<div style="text-align: -webkit-center">
				<input type="button" id="play" value="play" onClick="game()"/>
				<!--<input type="button" id="map" value="create map" onClick="construct()"/>-->
			</div>
			<div style="text-align: -webkit-center">
				<div>
					<input class="control" type="button" id="up" value="up" onClick="upButton()"/>
				</div>
				<div>
					<input class="control" type="button" id="left" value="left" onClick="leftButton()"/>
					<input class="control" type="button" id="right" value="right" onClick="rightButton()"/>
				</div>
				<div>
					<input class="control" type="button" id="down" value="down" onClick="downButton()"/>
				</div>
				
			</div>
		</div>
	</div>
	<script type="text/javascript" src="script.js" ></script>
</body>
</html>

• user190754: KarthikNedunchezhiyan/MultiLevel-Snake-Game you can use this link for reference, i have made snake game using js

Answer 1

Besides what Janos wrote in his answer already, a few thoughts from me:

Inconsistent use of var

Some variables are declared using the var keyword, but others aren't:

var canvas = document.getElementById('canvas');
ctx = canvas.getContext('2d');

Note: You can use a , to separate multiple declarations, like:

var canvas = document.getElementById('canvas'),
 ctx = canvas.getContext('2d');

Configuration

I found it a bit hard to follow, which keys were used for what action and you often re-use color values.

To improve readability of your code and to make it more DRY you can create some sort of configuration.

Another advantage is that you can change the whole appearance and control of the game now. You can even expose this configuration to the user, making it possible to create own game settings.

Color and control configuration

var theme = {
 control: {
 up: 39,
 down: 40,
 left: 37,
 right: 38 
 },
 colors = {
 canvas: {
 background: 'darkGreen',
 gameOverBackground: 'red'
 },
 meal: {
 background: 'red'
 }
 }
}

Simply access the values where needed:

ctx.fillStyle = theme.colors.meal.background;
if (key === theme.control.left) {}

It is clear which key or color you're referring to and there's only one place to change it.

Answer 2

Mutually exclusive conditions

Here, the second condition will be evaluated too, even when the first one is true. To avoid that, the second if should be else if.

if(key == "esc"){//exit 
 // ...
}
if(key == "left" || key == "up" || key == "down" || key == "right"){//newt move
 // ...
}

The same goes for this sequence of conditionals:

if(key == "left")px-=step;
if(key == "right")px+=step;
if(key == "up")py-=step;
if(key == "down")py+=step;

And these, and so on, you get the idea:

if(px<0)px = canvas.width-step;//new head position
if(py<0)py = canvas.height-step;
if(px>canvas.width-step)px = 0;
if(py>canvas.height-step)py = 0;

Variable scope

This writing style of var i may lead to confusions in JavaScript:

for(var i = 1; i < tail; i++){
 // ...
}

Because this writing style suggests that i will be visible only within the body of the for statement, when in fact it's visible in the body of the entire function. The recommended practice is to declare variables at the top of the function.

Writing style

For better readability, it's generally recommended to put spaces around operators and parentheses in conditionals. For example instead of this:

while(mealy%step){
 mealy+=1;
}

Write like this:

while (mealy % step) {
 mealy += 1;
}

And instead of mealy += 1, you can write ++mealy or mealy++. Note that the ++ prefix and postfix operators are exempt from the recommendation to put spaces around operators.

Use boolean values for false and true meanings

Instead of using 0 and 1 values for meal to indicate false and true, it would be better to use false and true. And then write conditions on the value of meal as if (meal) or if (!meal).

Duplicated logic

The if and else branches execute the same non-trivial fillRect operations. It would be good to extract the 4 lines called on ctx into a helper function that takes two style parameters:

if(bigHead == 0){//drawing head
 ctx.fillStyle = "lime";
 ctx.fillRect(pos[0][0], pos[0][1], step-brd, step-brd);
 ctx.fillStyle = "DarkGreen";
 ctx.fillRect(pos[0][0]+step/7, pos[0][1]+step/7, step/1.8, step/1.8 ); 
}else{
 ctx.fillStyle = "yellow";
 ctx.fillRect(pos[0][0], pos[0][1], step-brd, step-brd);
 ctx.fillStyle = "red";
 ctx.fillRect(pos[0][0]+step/7, pos[0][1]+step/7, step/1.8, step/1.8 );
 // ...
}