3
\$\begingroup\$

I am learning javascript and have developed a car game using :

  • HTML Canvas
  • javascript

The repository link is : https://github.com/lazycoder-007/car_race_html_javascript_game

The code is given below :

This is the GameArea object, with functions related to load, start and stop the game.

var myGameArea = {
    canvas : document.getElementById("myCanvas"),
    load : function(){
        this.canvas.width = canvasW;
        this.canvas.height = canvasH;
        this.context = this.canvas.getContext("2d");
        this.loadInterval = setInterval(loadGameArea, 20);
        this.frameNo = 0;
        window.addEventListener("keydown", function(e){
            myGameArea.keys = (myGameArea.keys || []);
            myGameArea.keys[e.keyCode] = true;
        });
        window.addEventListener("keyup", function(e){
            myGameArea.keys[e.keyCode] = false;
        });
    },
    start : function(){
        clearInterval(this.loadInterval);
        this.interval = setInterval(updateGameArea, 20);
    },
    clear : function(){
        this.context.clearRect(0, 0, this.canvas.width, this.canvas.height);
    },
    stop : function() {
        clearInterval(this.interval);
        myMusic.stop();
    }
}

This is the updateGameArea function, which is invoked in a certain interval defined above.

function updateGameArea()
{
    for (i = 0; i < myObstacles.length; i += 1) {
        if (redGamePiece.crashWith(myObstacles[i])) {
          myGameArea.stop();
          return;
        }
    }
    myGameArea.clear();
    backgroundRoad.update();
    myGameArea.frameNo += 1;
    if (myGameArea.frameNo == 1 || diveiderInterval(numberOfDividersPerFrame)) {
        roadDivider.push(new component(20, 80, "white", canvasW/2, 0));
    }
    for (i = 0; i < roadDivider.length; i += 1) {
        roadDivider[i].y += myObstacleSpeed;
        roadDivider[i].update();
    }
    if (myGameArea.frameNo == 1 || obstacleInterval(numberOfObstaclesPerFrame)) {
        myObstacleX = Math.floor(Math.random() * canvasW);
        myObstacleY = 0;
        myObstacles.push(new component(myObstacleW, myObstacleH, "redCarImage.png", myObstacleX, myObstacleY, "image"));
    }
    for (i = 0; i < myObstacles.length; i += 1) {
        myObstacles[i].y += myObstacleSpeed;
        myObstacles[i].update();
    }
    redGamePiece.speedX = 0;
    redGamePiece.speedY = 0;
    if (myGameArea.keys && myGameArea.keys[37] && redGamePiece.x > cornerGap) 
    {
        redGamePiece.speedX = -redGamePieceSpeedLeft; 
    }
    if (myGameArea.keys && myGameArea.keys[39] && redGamePiece.x < myGameArea.canvas.width - myObstacleW - cornerGap) 
    {
        redGamePiece.speedX = redGamePieceSpeedRight; 
    }
    if (myGameArea.keys && myGameArea.keys[38] && redGamePiece.y > cornerGap) 
    {
        redGamePiece.speedY = -redGamePieceSpeedUp; 
    }
    if (myGameArea.keys && myGameArea.keys[40] && redGamePiece.y < myGameArea.canvas.height - myObstacleH - cornerGap) 
    {
        redGamePiece.speedY = redGamePieceSpeedDown ; 
    }
    myScore.text = "SCORE: " + myGameArea.frameNo;
    myScore.update();
    redGamePiece.newPos();
    redGamePiece.update();

}

Finally, the functions to start the game:

function startTheGame()
{
    myMusic = new Sound("gameTheme.mp3", "true");
    redGamePieceX = canvasW/2;
    redGamePieceY = canvasH - redGamePieceX - cornerGap;
    //redGamePiece = new component(redGamePieceW, redGamePieceH, redGamePieceColor, redGamePieceX, redGamePieceY);
    redGamePiece = new component(redGamePieceW, redGamePieceH, "yellowCarImage.png", redGamePieceX, redGamePieceY, "image");
    backgroundRoad = new component(canvasW, canvasH, "roadImage.png", 0, 0, "image");
    myScore = new component("20px", "Consolas", "red", 10, 40, "text");
    myGameArea.stop();
    if(start)
    {
        myGameArea.start();
    }
    else
    {
        myGameArea.load();
    }
    //myMusic.play();
}

It is all working fine in this way. Please help improve this.

\$\endgroup\$
  • \$\begingroup\$ Mandatory reading: developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/… It should be well-suited to you being from a Java background. Be also aware that there are people who do not think class syntax is necessary, and claim that it is backwards, only meant for people who don't really know how to use JavaScript \$\endgroup\$ – user120242 May 26 at 12:52
  • 1
    \$\begingroup\$ Thanks @user120242 for the reference. Be also aware that there are people who do not think class syntax is necessary, and claim that it is backwards, only meant for people who don't really know how to use JavaScript : isn't Angular and other frameworks of js, Object Oriented in nature. Just curious to know?? \$\endgroup\$ – Lazycoder_007 May 26 at 13:02
  • \$\begingroup\$ It's in reference to classes. Additional mandatory reading: developer.mozilla.org/en-US/docs/Learn/JavaScript/Objects/… \$\endgroup\$ – user120242 May 26 at 13:19
  • 2
    \$\begingroup\$ Welcome to code review, where we review working code and provide suggestions on how to improve the code. While asking for reviews of working code is on topic, asking how to change the code to object oriented design is off-topic. If you remove that part of the question the question becomes on-topic. In addition to the -3 you see, there is one vote to close the question as off topic (neither the down votes or the vote to close is mine). Our guidelines. \$\endgroup\$ – pacmaninbw May 26 at 15:29
2
\$\begingroup\$

Initial thoughts

The code is not bad - has most consistent indentation using tabs. The game plays fine in Chrome on my macbook pro. I know you initially asked how to convert it to "object Oriented Design". The current code is somewhat object-oriented already using JavaScripts objects.

Suggestions

Game play

The score does not appear to be reset after subsequent starts. This is typically a feature in many games.

For mobile users an option to detect motion might be considered for controlling the direction of the car - e.g. with the DeviceMotionEvent API.

Code

Variable scope

Limit the scope of variables to blocks and functions by using const as a default when declaring variables and functions. If re-assignment is needed then use let (e.g. in a loop). This can help avoid potential bugs where variables get over-written accidentally.

In a larger application you would want to limit the scope of all the variables declared at the top - e.g. redGamePiece, backgroundRoad, etc. An IIFE or revealing module can assist with doing this.

Braces

Some braces are placed on a new line:

function updateGameArea()
{

Many style guides disallow this - e.g. the Google JS Style guide

4.1.2 Nonempty blocks: K&R style

Braces follow the Kernighan and Ritchie style ("Egyptian brackets") for nonempty blocks and block-like constructs:

  • No line break before the opening brace.
  • Line break after the opening brace.
  • Line break before the closing brace.
  • Line break after the closing brace if that brace terminates a statement or the body of a function or class statement, or a class method. Specifically, there is no line break after the brace if it is followed by else, catch, while, or a comma, semicolon, or right-parenthesis.

Animation frames

SetInterval() is acceptable for many machines but for machines with lower resources (e.g. older machines, mobile devices) it would be wise to use requestAnimationFrame() instead. For more information on this matter, refer to this blog post: Animating with javascript: from setInterval to requestAnimationFrame.

Prototypal inheritance

The methods are added to each instance of the classes - e.g. myGameArea has methods load, start, clear, and stop while each component instance has its own update, newPos, and crashWith method. For better performance1 these should be added to the prototypes.

For example:

function myGameArea() {
    this.canvas = document.getElementById("myCanvas");
}

myGameArea.prototype.load = function() {
    this.canvas.width = canvasW;   
    // etc...
}
myGameArea.prototype.start = function() {
    clearInterval(this.loadInterval);
    this.interval = setInterval(updateGameArea, 20);
}

Class syntax

You could convert the code to the newer ES6 class syntax - bear in mind that it is "primarily syntactical sugar over JavaScript's existing prototype-based inheritance". If there were many subclasses then it would help simplify setting up the prototypal inheritance.

Looping

Some loops can be simplified - e.g. instead of the first for loop in updateGameArea():

 for (i = 0; i < myObstacles.length; i += 1) {
    if (redGamePiece.crashWith(myObstacles[i])) {
      myGameArea.stop();
      return;
    }
}

A for...of loop could eliminate the need to dereference the obstacle at the current index:

for (const obstacle of myObstacles) {
    if (redGamePiece.crashWith(obstacle)) {
      myGameArea.stop();
      return;
    }
}

That could be simplified even further using Array.some():

if (myObstacles.some(obstacle => redGamePiece.crashWith(obstacle))) {
    myGameArea.stop();
    return;
}

1https://stackoverflow.com/a/4508498/1575353)

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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