phetsims / vegas

Reusable game components for PhET simulations.
MIT License
1 stars 4 forks source link

ProgressIndicator may create more stars than desired #30

Closed pixelzoom closed 10 years ago

pixelzoom commented 10 years ago

I ran into this when adding some 'dev' mode features to balancing-chemical-equations.

ProgressIndicator's constructor looks like:

24 function ProgressIndicator( numStars, scoreProperty, perfectScore, options )

It observes scoreProperty, and computes the number of 'filled' stars like this:

44 var proportion = score / perfectScore; 45 var numFilledStars = Math.floor( proportion * numStars );

The problem here is that there's no check to verify that the score is <= perfectScore. If it's not, then it will create more stars than numStars.

pixelzoom commented 10 years ago

I added this assertion:

40 assert && assert( score <= perfectScore );

Assigning to @jbphet to review.

jbphet commented 10 years ago

The assert looks reasonable to me. Tested a sim that uses the progress indicator with assertions turned on and saw no spurious asserts. Closing.