Xonix game in JavaScript

This is my very first program in JavaScript. I am trying to teach myself from JavaScript: The Definitive Guide, 6th Edition.

The code is also available on my bitbucket.

I decided to remake the classical game Xonix as an exercise. I took some concepts from this video.

I would like to ask you for advice in:

-
Style: I really have no idea how should good JavaScript look like. Please, could you point out the most serious flaws in my code?

My mentality is blocked in the C++ frame of reference and I do not truly get the entire prototype based inheritance. My code looks somewhat sick to me. There are too many uses of this, but I do not know how to cure this. Also I miss the separation of code to headers and sources and find it hard to organize my code.

-
Performance: The game seems to run good and fluently on Chrome, but on Firefox 35.0.1 for Ubuntu it

• runs 2 times slower

• feels jittery

• caps the CPU at 100%

• sometimes causes "too much recursion error"

I tried to profile the code on Firefox and 99.8% of time is spent on rendering. Also, I believe that the source of too much recursion is the Game.floodFill function. Is it possible, that a simple floodFill would cause this problem? How can I speed up the rendering?

I tried to refactor the code to be as small and as readable as possible, but I think I am just making it worse at this point. I do not expect you to read it all, but I would be very grateful if you pointed out the most serious flaws.

index.html:

main.js

```
;(function()
{
var scale = 10;
var nMonsters = 3;
var KEYS = {LEFT : 37, UP : 38, RIGHT : 39, DOWN : 40};

var randomInt = function(up, down)
{
return Math.round(down + Math.random() * (up - down));
};
var flip = function()
{
return Math.random() = this.width - 3 || j >= this.height - 3)
{
this.field[i][j] = this.FIELD.FULL;
}
else

Game class

First off, why is it a class? You are only using it once. It makes much more sense to just save the class's fields as normal variables.

However, if you want to keep the object oriented flow, you can make this an object, and access it's fields as like: Game.field

So instead of this:

var Game = function() {
    ...
}

You can have:

var Game = {
    ...
}

But, having it be a class provides benefits, such as being able to have a field/method be either public or private. It's up to you which one you go with.

Second of all, the Game class does a lot of work in it's constructor. A constructor is meant to declare and define different fields and methods; your Game constructor does not do this. To aid that, it would be better to put all that set up into a function that initializes everything, and starts the game.

To add to that, everything is very spread out. In your class constructor, you should have all of your fields at the top, and all the methods at the bottom. You seem to be throwing things all over the place.

That being said, you class constructor would now look like this:

var Game = function (canvasId)
{
    var canvas = document.getElementById(canvasId);
    this.c = canvas.getContext("2d");
    canvas.width  = this.width * scale;
    canvas.height = this.height * scale;
    this.player = new Player(this.width / 2, 0, this);
    this.keyBoard = new KeyboardController;
    this.field = new Array(this.width);
    this.FIELD = {EMPTY : 0, FULL : 1, INCONSTRUCTION : 2, RIGHT_EVALUATED : 3, LEFT_EVALUATED : 4};
    this.COLORS = ["black", "green", "yellow", "magenta", "blue"];
    this.monsters = [];
    var self = this;

    var tick = function()
    {
        if (!this.running)
        {
            return;
        }
        self.update();
        self.render();
        requestAnimationFrame(tick);
    };

    this.init = function() {
        for (var i = 0; i = this.width - 3 || j >= this.height - 3)
                {
                    this.field[i][j] = this.FIELD.FULL;
                }
                else
                {
                    this.field[i][j] = this.FIELD.EMPTY;
                }
            }
        }

        for (var i = 0; i < nMonsters; ++i)
        {
            var min = 10;
            this.monsters.push(new Monster(randomInt(min, this.width  - min),
                                           randomInt(min, this.height - min),
                                           flip() ? -0.5 : 0.5,
                                           flip() ? -0.5 : 0.5,
                                           this));
        }
        tick();
    }
};

I also made a small extra change; I moved the tick() function call so now, when the game is inited, the game will start after that extra set-up.
KeyboardController class

This class doesn't seem to serve it's full purpose. All it does it set up two event listeners that store some information from the event.

The main problem is that the Game class is doing this class's job. As in, the Game class is doing a lot of checking of the key presses.

To me, that seems like the KeyboardController's job.
Getting and setting

From my knowledge of Java, it is common practice to have getters and setters, rather than leaving the fields open for anything to touch and change.

You do do this in some places, but not in others.

Although, do not create too many getters and setters. For example, if you have a two dimensional array, you don't want to have a getter and setter for each individual array member.

Disclaimer: I'm not sure if the same practice applies to JavaScript.
Indentation

Judging by your post, you come from a C++ background. Also, I can tell by your indentation.

In JavaScript, it is common to put the { on the same line as the signature, if/else statement, etc.
Comments

You don't have any, and that can become a problem, especially with games where you do a bunch of crazy arithmetic on different numbers, coordinates, and other doo-hickeys.

You shouldn't over comment, but at least explain lines that aren't blatantly obvious.

StackExchange Code Review Q#80155, answer score: 5