phetsims / twixt

Animation library for interactive HTML5 graphics
MIT License
1 stars 3 forks source link

Animation fails with duration:0 #27

Closed pixelzoom closed 4 years ago

pixelzoom commented 4 years ago

If an Animation is created and run with duration: 0, it fails with NaN due to a divide-by-zero error.

This was the cause of https://github.com/phetsims/unit-rates/issues/215. In Unit Rates, there's an animation that moves a marker along the number line, based on the number that's entered into a keypad.

Here's the Animation definition in DoubleNumberLineAccordionBox:

markerEditorAnimation = new Animation( {
  duration: 0.002 * Math.abs( destinationX - markerEditorNode.x ), // 2ms per 1 unit of distance
  easing: Easing.QUADRATIC_IN_OUT,
  object: markerEditorNode,
  attribute: 'x',
  to: destinationX
} );

If a value that is larger than the number line's range is entered into the keypad, then the marker moves to the far right end of the number line. If you enter 2 such large values in a row, then the distance that the marker needs to move is zero, and the computed value of Animation option duration will also be zero.

For example, here's the marker located at "333", and I'm about to enter "334". This requires no movement of the marker, so duration is 0.

screenshot_52

When you try to run an Animation with duration: 0, Animation step will have a divide-by-zero at line 312, where this.length is zero and ratio is therefore NaN.

312 const ratio = Utils.clamp( ( this.length - this.remainingAnimation ) / this.length, 0, 1 );

This causes failure further along in the code. In the case of Unit Rates, it failed down in Easing.polynomialEaseInOutValue, where an assertion noticed that the ratio was invalid.

In Unit Rates, I'm simply not going to create an Animation if duration is zero. But this should be handled in Animation by either:

(a) an immediate assertion failure if options.duration is zero, or (b) the animation is a no-op and completes immediately when it's started.

I don't know which option is better, but option (b) would have prevented having to add logic to Unit Rates.

jonathanolson commented 4 years ago

I'll lean towards (b) unless it seems unworkable in the code. Thanks for spotting!

jonathanolson commented 4 years ago

Implemented (b) above.

pixelzoom commented 4 years ago

I tested this by removing the workaround in phetsims/unit-rates#215. Seems to work great. Thanks, closing.