Datsik
Datsik

Reputation: 14824

Why is 'this' not what it should be in this context?

I have a function that I use on canvas, I'm trying to clear an interval that is called with the .animate function, but when I call .unbind(); it logs undefined still, when it should log the timeout, I'm not sure why it is not working, maybe you guys can help

function Character(model, srcX, srcY, srcW, srcH, destX, destY, destW, destH) {
    this.model  = model;
    this.srcX  = srcX;
    this.srcY  = srcY;
    this.srcW  = srcW;
    this.srcH  = srcH;
    this.destX = destX;
    this.destY = destY;
    this.destW = destW;
    this.destH = destH;
    this.timeout = undefined;

}

Character.prototype = {
    draw: function() {
        return ctx.drawImage(this.model, this.srcX, this.srcY, this.srcW, this.srcH,
                    this.destX, this.destY, this.destW, this.destH);
    },

    animate: function(claymation) {
        var top = this; <<<<<--------Set the this variable
        var queue = (function() {
            var that = this;
            var active = false;
            if (!this.active) {
                (function runQueue(i) {
                    that.active = true;
                    var length = claymation.length -1;      
    >>>>-Set the timeout    top.timeout = setTimeout(function() {
                        claymation[i].action();
                        if ( i < length ) {
                            runQueue(i + 1);
                            if (i === length - 1) {
                                that.active = false;
                            }
                        }
                    }, claymation[i].time);
                })(0);
            }
        })();
        return queue;
    },

    update: function(callback) {
        callback();
    },
    unbind: function() {
        console.log(this.timeout); < Logs undefined
        clearTimeout(this.timeout);
        console.log(this.timeout); < Also logs undefined?
    }
}

Update:

I'm calling unbind on:

player = new Character(playerModel, 0, 130, 100, 100, 150, 150, 100, 100)
        if (e.which === 39) {
            player.unbind();
            key = undefined;
        }

Full Source Code: https://github.com/Gacnt/FirstGame/blob/master/public/javascripts/superGame.js#L50-L77

Upvotes: 0

Views: 84

Answers (2)

Bergi
Bergi

Reputation: 665574

Your animate function is messed up. You already have seen the need to store the this reference in an extra variable (that, top, whatever) as it changes from call to call and from function to function, but you have failed to do it correctly.

var top = this;
var queue = (function() {
    var that = this;
    var active = false;
    if (!this.active) {
        // use
        that.active = true;
        // or
        top.timeout = …;
        // or
        that.active = false;
    }
})();

While top is correct and will reference the Character instance on which you called the method, that is definitely not - it will reference the global context (window), which is the default this value in normal (immediately) invoked function (expression)s. Therefore, this.active will hardly have a value as well, and your timeout property doesn't get set. Also notice that the IIFE doesn't return anything, so queue will be undefined.

Instead, you seem to want to use that local active variable. Then just do it! You don't have to use some Java-this-like keyword to refer to the "local" one - the variable is just the next in the scope chain, so it will be used.

I'm not entirely sure, but it looks like you want

Character.prototype.animate = function(claymation) {
    var that = this; // variable pointing to context
    var active = false; // again, simple local variable declaration
    if (!active) {
       (function runQueue(i) {
            active = true; // use the variable
            var length = claymation.length -1;      
            that.timeout = setTimeout(function() { // use property of "outer" context
                claymation[i].action();
                if ( i < length ) {
                    runQueue(i + 1);
                    if (i + 1 === length) {
                        active = false; // variable, again!
                    }
                }
            }, claymation[i].time);
        })(0);
    }
};

Upvotes: 3

RobG
RobG

Reputation: 147553

What Bergi is saying is that:

animate: function(claymation) {
    var top = this;

Here you set top to reference this, which is the instance (I'd rather call it character so you know it's an instance of Character). Then you have an IIFE that has it's own execution context and a new value of this:

    var queue = (function() {
        var that = this;

Here that is set the the this of the IIFE, which is not set so will default to the global/window object, or if in strict mode, will remain undefined.

        var active = false;
        if (!this.active) {

So here you are getting window.active, which is likely undefined the first time, so the test is true. Later you do:

            (function runQueue(i) {
                that.active = true;

Setting a window.active to true. Also, you are doing:

            (function runQueue(i) {
               ...
            })(0);

There isn't any need for the IIFE if you are just passing in a fixed value, just use 0 everywhere you have i and remove the IIFE, just use the function body, you shouldn't need that extra object on the scope chain.

Finally, neither of the IIFEs returns anything, so queue remains undefined, so:

    })();
    return queue;

returns the udefined value.

Upvotes: 0

Related Questions