phetsims / natural-selection

"Natural Selection" is an educational simulation in HTML5, by PhET Interactive Simulations
GNU General Public License v3.0
3 stars 7 forks source link

is PlayButtonGroup using the wrong Timer? #227

Closed pixelzoom closed 4 years ago

pixelzoom commented 4 years ago

Related to the fix for https://github.com/phetsims/natural-selection/issues/217 (double-click on Start Over button)...

The fix (arrived at with the help of @jonathanolson) was to make the "Add a Mate" or "Play" button visible in the next frame, using a Timer. In PlayButtonGroup.js:

import timer from '../../../../axon/js/timer.js';
...
          timer.runOnNextFrame( () => {
            addAMateButton.visible = ( bunnyCount === 1 );
            playButton.visible = ( bunnyCount > 1 );
          } );

For https://github.com/phetsims/axon/issues/329, @samreid renamed timer.js to stepTimer.js, and added this documentation:

// Register and return a singleton timer. Only runs when the sim is active, during the step. For a timer that runs even
// when the sim is inactive, see animationFrameTimer

"Only runs when the sim is active" is a potential problem for the workaround in PlayButtonGroup.js. I expected this to run on the next next frame, not on the next step. And I think that's a valid assumption based on the name runOnNextFrame.

I think that I should therefore switch to animationFrameTimer.js, which also appear to be new. @samreid does that sound correct?

pixelzoom commented 4 years ago

Maybe it doesn't matter which timer NS is using? I'm not clear on what "Only runs when the sim is active" means, when the sim might be inactive, and whether it's an issue if for NS.

samreid commented 4 years ago

In order to have reproducible playbacks in the PhET-iO playback wrapper, Natural Selection should use the stepTimer so that the input event and step sequencing will be reproducible.

pixelzoom commented 4 years ago

Great, thanks for the info. No changes required, so closing.