phetsims / twixt

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

Easing: duplicated assertion with no message #22

Closed pixelzoom closed 4 years ago

pixelzoom commented 5 years ago

CT recently reported a transient failure of this assertion in Easing:

Easing.polynomialEaseInOutValue = function( n, t ) {
  assert && assert( typeof t === 'number' && isFinite( t ) && t >= 0 && t <= 1 );

There's no assertion message, so no idea what was wrong with t that made this fail. And this same assertion expression is repeated 9 times.

TODO: factor out the expression and add a message.

@jonathanolson FYI.

pixelzoom commented 5 years ago

Doc for the t parameter is also not helpful, does not describe what it is:

@param {number} t - Should be in the range [0,1].
pixelzoom commented 5 years ago

Doc at the top of the file says "These should be equivalent to the polynomial tweens that TWEEN.js uses." Plowing through TWEEN.js website, I finally found Ease documentation:

The Ease class provides a collection of easing functions for use with TweenJS. 
It does not use the standard 4 param easing signature. Instead it uses a single 
param which indicates the current linear ratio (0 to 1) of the tween.
pixelzoom commented 5 years ago

I factored out tIsValid:

  /**
   * Verifies that t is valid.
   * @param t - The linear ratio [0,1] of the animation
   * @returns {boolean}
   */
  function tIsValid( t ) {
    return typeof t === 'number' && isFinite( t ) && t >= 0 && t <= 1;
  }

And the 9 assertions now look like:

assert && assert( tIsValid( t ), `invalid t: ${t}` );

No unit tests (see #23) so I tested a few sims that use Easing.

@jonathanolson please review.

pixelzoom commented 4 years ago

It's been almost 1 year since a review was requested. So I'm going to just go ahead and close this.