Tween on* methods don't set 'this'

[girasquid]

I noticed that when you define an onUpdate method for your Tween, the this object is null: https://github.com/Gamua/Starling-Framework/blob/master/starling/src/starling/animation/Tween.as#L187

This pattern is repeated for onStart, onRepeat, and onComplete as well. This is how I've been able to work around it, for onUpdate methods that need to refer to the tween they're being called on (usually to alter onUpdateArgs):

var myTween:Tween = new Tween(gridBackground, TWEEN_DURATION, Transitions.EASE_IN_OUT);
myTween.onUpdateArgs = [myTween, 0, 0];
myTween.onUpdate = function(parentTween:Tween, importantValue:int):void {
  // Do some things to update importantValue
  myTween.onUpdateArgs = [parentTween, importantValue];
}

But this got me wondering: does it make sense for the this object in on* calls from Tweens to be either:

1) The Tween calling the function, OR 2) The Tween's target?

I lean towards the first one (because you can go from it TO the target), but I figured I'd open an issue before submitting a PR. Thoughts?

[PrimaryFeather]

Hey Luke! I'm actually not entirely sure myself about what would be the best way to go here. The reasoning behind using "null" was that it makes sure that people who are not aware of AS3's changing of "this" get a null reference exception right away. E.g. if somebody thinks that he can do this:

var tween:Tween = new Tween(object, 1.0);
tween.onComplete = function():void
{
    this.hide(); // 'hide' declared on the class we're currently in
}

When I started with AS3, I actually thought I could do that. ;-) (And some languages are allowing it.) Granted, he would get an exception anyway if "hide" is not defined on the object we choose as "this"; but that object might actually declare such a method, and in that case, the bug will be not very obvious.

If "this" is null here, people learn immediately that "this" is not available in a callback, and will work around it.

What do you think? And are there other opinions on this?

Thanks in advance! :smile:

[girasquid]

I think your code sample points towards a decent argument for the this object to be something - when I first wrote my onUpdate code, I was expecting this to be the tween that was being updated. The fact that I need to pass around the tween in order to maintain a reference to it in my code that it calls seemed like a bug (which is why I opened the issue).

I put this inside an onComplete function:

        myTween.onComplete = function():void {
          trace(this);
          trace(typeof(this));
        }

And when it fires, this is my output:

[object global]
object

So even passing in null, it's never ... actually null. I think that's a good argument for setting it to something the user expects, rather than something they're not expecting.

[PrimaryFeather]

Ah, you're right! Indeed, I could reproduce that: this is never null, but the global object. Which is not a help to anybody.

I still must say that — as a user of Starling — I wouldn't actually know what to expect in the this pointer. I never use it in anonymous functions because it simply never works as I'd expect it to — which is that it would point to the same object as outside of the function's scope.

But since 'null' doesn't work the way I expected, I can easily change it to point to the tween — as you suggested. That way, you have a reference to the tween, without passing it around, which is of course an advantage.

Thanks for making me aware of this, Luke!

[girasquid]

Thanks!

[PrimaryFeather]

You're welcome! :smile:

[joshtynjala]

This change is going to break some of my code. If you pass a reference to a member function from an object (rather than an anonymous function), this is automatically bound to that object when that function is called.

[commovere]

joshtynjala +1 if someone need access to "this"-jbject then just add it to arguments of on*-methods

[PrimaryFeather]

Are you sure? I thought I had tried that ... but I might be mistaken.

[PrimaryFeather]

Hm, when I try this, everything seems to be happening as designed:

private function testTweens():void
{    
    _juggler.tween(card, ANIM_TIME, { 
        x: 100,
        onStart: onStart,
        onComplete: onComplete
    });

    function onComplete():void
    {
        trace(this); // prints the tween
    }
}

private function onStart():void
{
    trace(this); // prints the object the "onStart" method belongs to
}

(Could it be that this changed back when ASC 2.0 was introduced?)

[joshtynjala]

I guess you're right. I tested with ASC 2.0, Apache Flex, and Adobe Flex 4.6, and they all behave the same way. The fact that apply() sometimes completely ignores the argument meant to change the function's scope seems very strange to me. It doesn't seem to be documented either. Regardless, I guess this change is safe, afterall. Sorry for the noise.

[PrimaryFeather]

No problem, Josh! Better safe than sorry.

BTW, as I said above, I'm not entirely sure myself what's the best way to go here. So any comments and ideas are welcome! If there is some problem or argument against this change, I can easily revert it.

[PrimaryFeather]

BTW, I also had a look at "TweenLite" and "GTween", and the first passes "null" as I did in the past, while the latter uses the tween object. So there doesn't seem to be a consensus about what to do with "this" for such callbacks.