7
\$\begingroup\$

I'm relatively new to programming but my most familiar languages are Java, C# and Haskell and this is my first javascript program. All the concepts used in here were just the first thing that came to mind or the first thing that I stumbled across on google.

I tried my best to follow come scoping rules but I think I fell a bit short there and I'm mostly listening to my editor about what rules to follow. I would just like to ask some more experienced people what I could improve on/what things I should avoid moving further on into javascript. Thanks!

let xVelocity = 1;
let yVelocity = 0;
let isRunning = false;
//Iunno how to include these variables in the scope of playGame
document.onkeydown = keyPush;

function playGame() {
    if (!isRunning) {
        isRunning = true;
        let scalefact = 20;
        let canvas = document.getElementById("snakes-canvas");
        let ctx = canvas.getContext("2d");
        let itemWidth = canvas.height / scalefact;

        let snake = [];
        newSnake();
        let apple = new AppleObject();

//Updates the snake/apple in some time
        window.setInterval(pGame, 1000/15);

        function SnakeObject(x, y) {
            this.xPos = x;
            this.yPos = y;
        }

        function AppleObject() {
            this.xPos = ((Math.round(Math.random() * scalefact)) * itemWidth);
            this.yPos = ((Math.round(Math.random() * scalefact)) * itemWidth);
        }

        function pGame() {
            let consumedApple = false;

//Make sure apple lies within bounds
            if (apple.xPos > canvas.width - itemWidth
                || apple.yPos > canvas.height - itemWidth) {
                apple = new AppleObject();
            }

//Clear canvas at start of every iteration
            ctx.clearRect(0, 0, canvas.width, canvas.height);

//Make background black
            ctx.fillStyle = "#000000";
            ctx.fillRect(0, 0, canvas.width, canvas.height);

            ctx.fillStyle = "#FF0000";
            ctx.fillRect(apple.xPos, apple.yPos, itemWidth, itemWidth);

//Draw the snakes pieces
            ctx.fillStyle = "#1FFF1F";
            for (let i = 0; i < snake.length; i+=1) {
                ctx.fillRect(snake[i].xPos, snake[i].yPos, itemWidth, itemWidth);
            }
            ctx.fillStyle = "#0000FF";
            ctx.fillRect(snake[0].xPos, snake[0].yPos, itemWidth, itemWidth);

//Determine the new position for the snake head segment
            let newX = snake[0].xPos + (xVelocity * itemWidth);
            let newY = snake[0].yPos + (yVelocity * itemWidth);

//If head consumes apple
            if (newX === apple.xPos && newY === apple.yPos) {
                consumedApple = true;
            }

//Check if snake exceeds bounds of the canvas
            if (newX > canvas.width-1) newX = 0;
            if (newY > canvas.height-1) newY = 0;
            if (newX < -1) newX = canvas.width - itemWidth;
            if (newY < -1) newY = canvas.height - itemWidth;

            let restart = false;
//if snake hit its own body pieces
            for (let i = 0; i < snake.length; i++) {
                if (snake[i].xPos === newX && snake[i].yPos === newY) {
                    newSnake();
                    xVelocity = 1;
                    yVelocity = 0;
                    restart = true;
                }
            }

            if (!restart) {
                //Move head piece forward and delete last body piece
                snake.unshift(new SnakeObject(newX, newY));
                if (!consumedApple) {
                    snake.splice(-1, 1);
                }
                else {
                    apple = new AppleObject();
                }
            }
        }

//Generates an array with the starting snake of len 5
        function newSnake() {
            snake = [];
            for (let i = (itemWidth * 5); i <= (itemWidth*5) + (itemWidth * 5); i += itemWidth) {
                snake.push(new SnakeObject((itemWidth * 10), i));
            }
        }
    }
}

function keyPush(key) {
    switch (key.keyCode) {
        case 37: // Left arrow
            if (xVelocity !==1) {
                xVelocity = -1;
                yVelocity = 0;
            }
            break;
        case 38: // Up arrow
            if (yVelocity!==1) {
                xVelocity = 0;
                yVelocity = -1;
            }
            break;
        case 39: // Right arrow
            if (xVelocity!==-1) {
                xVelocity = 1;
                yVelocity = 0;
            }
            break;
        case 40: // Down arrow
            if (yVelocity !==-1) {
                xVelocity = 0;
                yVelocity = 1;
            }
            break;
        case 116:
            location.reload();
            break;
        default:
            break;
    }
    key.preventDefault();
}

playGame()
<canvas id="snakes-canvas"></canvas>

\$\endgroup\$
2
  • \$\begingroup\$ this is neat! using a class is one way you could handle scoping issues, and you could also pass the canvas object into the function to make it re-usable. \$\endgroup\$ Commented Jan 12, 2018 at 14:44
  • \$\begingroup\$ @Iwrestledabearonce. Please add an answer instead of a comment. Refer to the section When shouldn't I comment? on Comment everywhere. \$\endgroup\$ Commented Oct 2, 2019 at 15:50

1 Answer 1

Reset to default
1
\$\begingroup\$

This code looks like a good start.

Some variable could be declared with const because they aren't re-assigned - e.g. canvas, snake if the first line of newSnake() was changed to snake.length = 0;. This helps avoid accidental re-assignment in the future.

The function name pGame could be improved for readability to be more descriptive of what it does - perhaps a name like processGame or something along those lines.

It would be wise to use requestAnimationFrame() instead of setInterval(). As this article explains, setInterval() isn't as good for performance:

The problem with using setTmeout/setInterval for executing code that changes something on the screen is twofold.

  • What we specify as the delay (ie: 50 milliseconds) inside these functions are often times not honoured due to changes in user system resources at the time, leading to inconsistent delay intervals between animation frames.

  • Even worse, using setTimeout() or setInterval() to continuously make changes to the user's screen often induces "layout thrashing", the browser version of cardiac arrest where it is forced to perform unnecessary reflows of the page before the user's screen is physically able to display the changes. This is bad -very bad- due to the taxing nature of page reflows, especially on mobile devices where the problem is most apparent, with janky page loads and battery drains. An iPhone or two have even caught fire as a result (just a joke Apple, no law suits please)!

\$\endgroup\$

You must log in to answer this question.