search

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

Inconsistency when setting state while fast forwarding #312

Closed KatieWoe closed 2 years ago

KatieWoe commented 2 years ago

Test device Dell Operating System Win 11 Browser Chrome Problem description For https://github.com/phetsims/qa/issues/818. In the state wrapper, if you hold down the fast forward button in the top sim while the sim is paused, one of the states set in the bottom sim will be un paused. While holding down the button, the bottom sim will behave like the top one. Once you let go, the next state will have the bottom sim playing, instead of paused. The state after that will be correct. This may not be worth addressing, but recording here. Steps to reproduce

  1. Go to the state wrapper and make sure state is automatically set every second or so (default)
  2. In the top sim go to the first screen
  3. Add a bunny
  4. Pause the sim
  5. Hold down the forward button and observe bottom sim
  6. Let go of forward button and observe bottom sim

Visuals stateoff

pixelzoom commented 2 years ago

I discussed with @KatieWoe on Zoom, because I didn't quite understand what I was looking for.

In the above movie, note the visual changes to the PlayPauseButton in the Downstream sim, immediately after releasing the PlayPauseButton in the Upstream sim. The button becomes enabled before the icon changes from "pause" (2 vertical bars) to the "play" (right arrow).

My guess is that this is due to the order that the sim's isPlayingProperty and the button's enabledProperty are being set in the Downstream sim. I don't think the Downstream sim is actually playing after the fast-forward button is released Upstream. I think it's just the Downstream PlayPauseButton that looks a little odd until its state has been restored.

The final state of the Downstream sim looks OK to me. So if my above hypothesis is correct, I would close this as "won't fix". But I'd like another opinion.

@samreid or @zepumph can you take a look?

pixelzoom commented 2 years ago

Dev test https://github.com/phetsims/qa/issues/818 is completed. Noting that this is the only issue that has not been addressed. I'm not clear on whether we're in a hurry (or even ready) to create a release branch.

zepumph commented 2 years ago

@samreid and I just finished our triage meeting. I'll take a look this afternoon.

samreid commented 2 years ago

I committed a proposed fix, @KatieWoe can you please review on phettest?

KatieWoe commented 2 years ago

Looks fixed on master

samreid commented 2 years ago

Thanks, since this was reported in a dev test, it seems ready for the next version. Closing.

pixelzoom commented 2 years ago

Thanks for handling this. Looks good.