phetsims / calculus-grapher

"Calculus Grapher" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
4 stars 4 forks source link

Pressing Reset All in launched sim can result in graph disappearing #316

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 1 year ago

Test device MacBook Air M1 chip

Operating System 13.2.1

Browser Safari

Problem description For https://github.com/phetsims/qa/issues/921, in Studio on the Advanced and Lab screens, after switching which graphs are visible on the screen using the radio buttons, pressing Reset All in the launched sim results in the selected graph disappearing instead of the launched state being restored.

Steps to reproduce In Studio, on the Advanced Screen:

  1. Select the integralRadioButton
  2. Press Test
  3. Press Reset All

In Studio, on the Lab Screen:

  1. Select the secondDerivativeRadioButton
  2. Press Test
  3. Press Reset All

Visuals

https://user-images.githubusercontent.com/87318828/227392201-d0d3eb1a-7c27-4cf0-a4e2-e9a04ee23ea0.mp4

Screenshot 2023-03-23 at 7 51 41 PM
veillette commented 1 year ago

That's another good one. You are closing in on a hat trick today.

veillette commented 1 year ago

I was able to reproduce in https://phet-dev.colorado.edu/html/calculus-grapher/1.0.0-dev.26/phet-io/wrappers/studio/ as well. I'll note that the missing graph will reappear if one presses the graphSetRadioButton. So the visibility of the graph is wrong, but its listener is still present.

pixelzoom commented 1 year ago

I'll take this one.

I suspect that GraphSetsAnimator.ts needs to know whether we're doing a reset. That involves modifying this expression:

if ( !oldGraphNodes || this.activeAnimation || phet.joist.sim.isSettingPhetioStateProperty.value ) {
pixelzoom commented 1 year ago

Reproduced in master, using the steps in https://github.com/phetsims/calculus-grapher/issues/316#issue-1638524754.

pixelzoom commented 1 year ago

@samreid helped me understand the problem here. I had instrumented the Properties that power GraphSetsAnimator (a twixt Animation) and that was causing problems when state was restored. It was resulting in one of the graphs having zero opacity, making it effectively invisible. Twixt Animations are generally not stateful, and if the ID saves a wrapper while an animation is in progress, we're not expecting the animation to continue from that point in the wrapper. So that fixed the problem reported, which @Nancy-Salpepi can please verify in master, and close if OK.

Noting for @amanda-phet, one other change in behavior... In a custom wrapper, if you change the default graph set that the sim initially shows, it's impossible to animate back to that graph set when the Reset All button is pressed. @samreid thought this was fine, and not something we should attempt to address.

Nancy-Salpepi commented 1 year ago

This is fixed in master! Closing!