Delay.onceComplete can fire "after" stop()

ronkorving commented 7 years ago

Expected output:

Stopping myDelay
Stopped myDelay

Actual output (50% of the time):

myDelay.onceComplete called
Stopping myDelay
Stopped myDelay

Sample code:

<!doctype html>
<html>
<head>
  <script type="text/javascript" src="./tina.js"></script>
</head>
<body>

<script>
  var duration = 3;

  var myDelay = new TINA.Delay(duration);
  myDelay.onceComplete(function () {
    console.log('myDelay.onceComplete called');
  });
  myDelay.start();

  var d2 = new TINA.Delay(0); // bug also appears with a delay of 1
  d2.onceComplete(function () {
    console.log('Stopping myDelay');
    myDelay.stop();
    console.log('Stopped myDelay');
  });
  d2.start();
</script>

</body>
</html>

ronkorving commented 7 years ago

Discussed this yesterday with @cainmartin and @jrouault and I believe the consensus on this at the moment is (correct me if I'm wrong) the following.

The resolution of event emission order is no smaller than a single tick. If both onceComplete events land in the same tick, the order is likely to be consistently first-registered followed by 2nd-registered (so we see "myDelay.onceComplete called" first). But sometimes, the two events will be in separate ticks, in which case the longer delay will always be in the 2nd tick and thus be stopped correctly.

This is probably as-good-as-it-gets, and I personally don't consider this an issue with TINA.

ronkorving commented 7 years ago

After further investigation, we decided that an elegant solution would be to allow a tween to be destroyed, even if active, avoiding its events being emitted from that moment on. @jrouault will submit a pull request to expose such functionality.

Wizcorp / tina.js

Delay.onceComplete can fire "after" stop() #63