GameLoop with requestAnimationFrame

Question

I am creating a HTML game where you fight monsters and stuff and this is part of the game loop for dealing damage:

var hp = 1000000000;
var speed = 10;
var power = 5;   

var lastCalledTime;
var fps;

var GameLoop = function() {
    if(!lastCalledTime) {
           lastCalledTime = new Date().getTime();
           fps = 0;
           return;
    }
    delta = (new Date().getTime() - lastCalledTime)/1000;
    lastCalledTime = new Date().getTime();
    fps = 1/delta;

    hp = hp - (5 * speed * 0.1) / fps;
};

var animFrame = window.requestAnimationFrame ||
                window.webkitRequestAnimationFrame ||
                window.mozRequestAnimationFrame    ||
                window.oRequestAnimationFrame      ||
                window.msRequestAnimationFrame     ||
                null ;

if ( animFrame !== null ) {
    var recursiveAnim = function() {
        GameLoop();
        animFrame( recursiveAnim );
    };
    animFrame( recursiveAnim );
} else {
    setInterval( GameLoop, 33.33 );
}

http://jsfiddle.net/u0k7tbv4/2/

Power is how much you deal with each attack and each level of speed gives 0.1 attack / second. With 5 Power and 10 Speed it should deal 5 damage every second and that seems to be the case in the fiddle.

But I'm not sure, I only used requestAnimationFrame because I heard setInterval is unstable and so it would be nice if someone could validate the code.

Answer 1

Instead of var GameLoop = function() { ... };, you could just define a function the "natural" way. Calling it gameLoop() would respect capitalization conventions better.

You neglected to use the var keyword when defining delta, so it becomes a global variable.

The GameLoop() function sets fps as a side-effect. If you don't actually need fps anywhere else, then it should be local. Furthermore, if you don't need fps, then there is no point in setting it to the reciprocal of delta.

I don't see much harm in initializing lastCalledTime right away, to eliminate the one-time-use special case inside GameLoop() — even if you technically didn't call GameLoop() at the time.

Calling getTime() twice is ugly. If might also cause a tiny bit of clock skew.

var lastCalledTime = new Date().getTime();

function gameLoop() {
    var now = new Date().getTime();
    var delta = (now - lastCalledTime) / 1000;
    lastCalledTime = now;

    hp -= 0.5 * speed * delta;
};

Alternatively, if you would like lastCalledTime neatly tucked inside the gameLoop() function rather than dangling outside, you could write:

function gameLoop() {
    var now = new Date().getTime();
    if (!arguments.callee.lastCalledTime) {
        arguments.callee.lastCalledTime = now;
    }
    var delta = (now - arguments.callee.lastCalledTime) / 1000;
    arguments.callee.lastCalledTime = now;

    hp -= 0.5 * speed * delta;
};

Answer 2

• Use Classes : Most easy way to have a clean start is to build a class, and start adding properties (in the constructor) and methods (on the prototype).
I used the very common convention for private properties to have them prefixed with '_' (expl : this._delta ).
Even if they won't be private (Javascript requires more work than Dart to have properties private), it is a good convention.

• Don't use

var time = (new Date().getTime());

Because you are creating an object + invoking a function when :

var time = Date.now();

Does the same job, just faster.

• requestAnimationFrame : I'm not sure you need the requestAnimationFrame stuff : It has been unprefixed since quite some time, and i doubt someone playing on his browser can have a > 2 years old version.
Anyway, it's better written like that :

if (!window.requestAnimationFrame) {
  (function() {
      var w=windows, raf="RequestAnimationFrame";
      window.requestAnimationFrame = w['webkit'+raf] || w['moz'+raf] || 
                                     w['o'+raf]      || ['ms'+raf]   ;
  }());
}

So now the class.
Notice that when providing a method to, say, rAF, for instance with :

 requestAnimationFrame(this.someMethod);

The function that you will send will just be the plain method you wrote, it will loose its context ( this === undefined ).
To say 'hey, when i talk about this function, i mean it as a method of 'this'', you have to bind the function. Binding the function creates another function that i prefer to store once to avoid creating garbage.

• refactored code :

function Game ( initialHP, initialSpeed, initialPower ) {
   this.hp    = intialHP;
   this.speed = initialSpeed;
   this.power = initialPower;
   this._lastCallTime = 0;
   this._delta        = 0;
   this._boundRecursiveAnim = null;
}

Game.Prototype = {
   startGame : function() {  
        if (window.requestAnimationFrame) {
            this._boundRecursiveAnim = this._recursiveAnim.bind(this);
            this._lastCallTime = Date.now();
            this._recursiveAnim(); 
         }
        else  setInterval( this.gameLoop.bind(this), 33.33 );
   },
   _recursiveAnim : function() { 
        requestAnimationFrame(this._boundRecursiveAnim);
        this._gameLoop();
   },
   _gameLoop : function () {  
        var currentTime = Date.now();         
        this._delta = 1e-3*(currentTime - this._lastCallTime);
        this._lastCallTime = currentTime;

        this.hp = this.hp - this._delta *(this.speed );
   },      
   getFPS() : function() { return  ( 1/this._lastCallTime ); }
}

use with

 var myGame = new Game(1000000, 5, 5);
 myGame.startGame();