chained tweens cannot be stopped in `onComplete` callback

[gera2ld]

Create two tweens and chain them:

const tween1 = new TWEEN.Tween({})
.to({}, 500)
.onComplete(() => {
  console.log('complete');
  tween1.stop();
  tween2.stop();
});
const tween2 = new TWEEN.Tween({})
.to({}, 500);
tween1.chain(tween2);
tween2.chain(tween1);

In onComplete callback, I try to stop both of them, but it does not work and just loops forever.

Looking into the code, I find that, in onComplete callback tween2._isPlaying is set to false, after that, the chained tween2 is started no matter if it should be stopped, so the stop does not make a difference to the result. So a workaround is to wrap it in setTimeout:

.onComplete(() => {
  setTimeout(() => {
    tween1.stop();
    tween2.stop();
  });
})

Anyway, it's ugly and doesn't make sense.

This has worked before, I think the refactor has broken a lot of things. 😞

[gera2ld]

I think onComplete should be called when the tween is really completed, which means its state has been changed and removed from the group, so that we can choose whether to add it back.

But actually it is called before all the cleaning work. As a result, the tween cannot be added back to the group in its onComplete callback, because it will be removed after the callback no matter what is done in the callback. That's why chaining the tween itself (tween.chain(tween)) worked before and does not work now, because starting the chained tweens is like a part of onComplete, and the tween is removed immediately after being started, even though it should not. I think this does not make sense either.

[mikebolt]

The chain function has been problematic for a long time. I'm in favor of making a breaking change to the chain function so that it is more intuitive. So now the question is "How should the chain function work?" I would like to read some suggestions.

[trusktr]

@mikebolt Hello! This issue is from 2017. Besides the TypeScript change and refactorings in general, there has since been new changes to chained tweens (including updates to the tests), as well as some new discussion here in the issues.

One example is https://github.com/tweenjs/tween.js/issues/406#issuecomment-636268598

Does this particular issue still exist? Is there a live code example?

[trusktr]

The reason for this is because onComplete fires, then the next update() call is what starts a chained tween (which seems prone to errors with long periods of time between frames (such as when returning from another tab, see https://github.com/tweenjs/tween.js/pull/564 which is solving that problem)).

A couple solutions:

1. first why do you want to stop a chained tween in onComplete? Seems to defeat the purpose. Perhaps instead of chaining you can use a different pattern, if you chain a tween but don't want to run it maybe it can be confusing. Would like to see a use case example.
2. Wait until the onStart of the chained tween, then you can stop it.

   let stopChained = false
   
   tween.start()
   tween.chain(tween2)
   tween.onComplete(() => stopChained = true)
   
   tween2.onStart(() => { if (stopChained) tween2.stop() })
   

[trusktr]

This has worked before, I think the refactor has broken a lot of things. 😞

Unfortunately some things can break in the name of progress. Really sorry about that! We try to have good coverage with out tests, and this edge case managed to sneak by.

I will close this as a workaround has been described above.

Another workaround is that chained tweens can also be removed by calling .chain() without any args (it will set the chained tweens to an empty list, so the next update will not have any chained tweens to start).

const tween1 = new TWEEN.Tween({})
.to({}, 500)
.onComplete(() => {
  console.log('complete');
  tween1.stop();
  tween2.stop();
  tween1.chain(); // <-------------- Remove chained tweens
});
const tween2 = new TWEEN.Tween({})
.to({}, 500);
tween1.chain(tween2);
tween2.chain(tween1);