Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Nancy-Salpepi opened this issue Mar 24, 2023 · 6 comments
Closed

Comments

@Nancy-Salpepi
Copy link

Test device
MacBook Air M1 chip

Operating System
13.2.1

Browser
Safari

Problem description
For phetsims/qa#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

graphdisappears.mp4

Screenshot 2023-03-23 at 7 51 41 PM

@veillette
Copy link
Contributor

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

@veillette
Copy link
Contributor

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
Copy link
Contributor

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 pixelzoom assigned pixelzoom and unassigned veillette Mar 24, 2023
@pixelzoom
Copy link
Contributor

Reproduced in master, using the steps in #316 (comment).

pixelzoom added a commit that referenced this issue Mar 24, 2023
pixelzoom added a commit that referenced this issue Mar 24, 2023
@pixelzoom
Copy link
Contributor

pixelzoom commented Mar 24, 2023

@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
Copy link
Author

This is fixed in master!
Closing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants