Tweens should be composable with no side-effects

[ejmurra]

Hi, I want to preface this by saying this library is excellent. I just finished a medium-sized project using tweenjs and I have some feedback.

I found that it was pretty cumbersome to create complex tweens using this library because the chain method is not side-effect free. Here's a pseudocode example:

/* I want to use tweenjs to make a light blink three times and then pause for a beat */
let light = new Light();
/* Here's the code I would expect to work */
let lightOn = new TWEEN.Tween().to(0, 100).easing(TWEEN.Easing.Quartic.Out).onUpdate(x => light.intensity = x * 1);
let lightOff = new TWEEN.Tween().to(0, 100).easing(TWEEN.Easing.Quartic.Out).onUpdate(x => light.intensity = 1 - (x * 1));

let lightBlink = lightOn.chain(lightOff) // A chain should return a new tween and NOT change lightOn itself

/* This makes our final product. lightBlink returns a tween, repeat 3 returns a new tween, delay 200 returns a tween after that, repeat again inifitely and then start that final tween. */
lightBlink.repeat(3).delay(300).repeat(Infinity).start() 

/* Here's the code I actually had to write to get this to work */
let lightOnTime = 75;
let lightOffTime = 225;

let lightOn = new TWEEN.Tween().to(0, lightOnTime).easing(TWEEN.Easing.Quartic.Out).onUpdate(x => light.intensity = x * 1);
let lightOff = new TWEEN.Tween().to(0, lightOffTime).easing(TWEEN.Easing.Quartic.Out).onUpdate(x => light.intensity = 1 - (x * 1));
let lightOn2 = new TWEEN.Tween().to(0, lightOnTime).easing(TWEEN.Easing.Quartic.Out).onUpdate(x => light.intensity = x * 1);
let lightOff2 = new TWEEN.Tween().to(0, lightOffTime).easing(TWEEN.Easing.Quartic.Out).onUpdate(x => light.intensity = 1 - (x * 1));
let lightOn3 = new TWEEN.Tween().to(0, lightOnTime).easing(TWEEN.Easing.Quartic.Out).onUpdate(x => light.intensity = x * 1);
let lightOff3 = new TWEEN.Tween().to(0, lightOffTime).easing(TWEEN.Easing.Quartic.Out).onUpdate(x => light.intensity = 1 - (x * 1));

lightOn.chain(lightOff.chain(lightOn2.chain(lightOff2.chain(lightOn3.chain(lightOff3.chain(new TWEEN.Tween().delay(150).chain(lightOn))))))).start()

As you can see, the second approach is more verbose and messy and less straight forward. This isn't a particularly complex tween but you can see how it would grow unwieldy very quickly.

One other note, when you have tween chains producing side-effects it can lead to subtle bugs. Here's an example:

let lightOn = new TWEEN.Tween().to(0, 100).easing(TWEEN.Easing.Quartic.Out).onUpdate(x => light.intensity = x * 1);
let lightOff = new TWEEN.Tween().to(0, 100).easing(TWEEN.Easing.Quartic.Out).onUpdate(x => light.intensity = 1 - (x * 1));

let lightBlink = lightOn.chain(lightOff) // If you create a new tween and don't use it, it shouldn't affect the tweens it is composing

/* One would expect this to turn on the light quickly. Instead it turns the light on and off because of the chain above */
lightOn.start()

[sole]

Thank you! Glad you liked the library and it helped you!

Agreeing that the chain method is unwieldly and not clear at all. We've discussed it before, in https://github.com/tweenjs/tween.js/issues/176 and https://github.com/tweenjs/tween.js/issues/153 (linking for reference).

I have no immediate fix for the issue - I'm just acknowledging it. It requires better thought and probably a breaking change (or more).

[mikebolt]

Because the chain method was advertised in the user guide as having an effect even when the return value is ignored, this probably cannot be changed safely in the current version. The best thing we could do right now would be to make the user guide more clear about the behavior. It may also help if we fixed the chain/repeat bug, or if we made a clone method, so that you would not have to completely redefine the tweens an extra two times.

Another option would be to make a compose method that takes two tweens and creates a new tween that is the same as the two tweens chained together, but does not modify either of the two tweens. Although it wouldn't break the existing API, this approach is probably more difficult to implement.

[dalisoft]

Write bit more code, you made complex tweens. Tween.js is simple and FREE (to make it as you like). Or use already complex tween maker based on tween.js

[sole]

@mikebolt I think since the syntax is so terribad it would be just better to either make a 'breaking change' (bumping major version) or to use a better syntax for V2.

[mikebolt]

I think we can deprecate 'chain' and add an equivalent 'compose' method with no side-effects. This should be documented better.

[trusktr]

Tween is already getting complicated. Maybe it is better to pursue something like a Timeline feature where functionalities are abstracted and not all meshed into a single Tween class.