search

phetsims / gravity-and-orbits

"Gravity And Orbits" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
12 stars 6 forks source link

Inconsistent availability of buttons #459

Closed Nancy-Salpepi closed 1 year ago

Nancy-Salpepi commented 2 years ago

Test device

Operating System MacBook Air (m1 chip)

Browser safari 16

Problem description For https://github.com/phetsims/qa/issues/838, in both Studio and State wrappers: In the launched sim, both the Restart button and Clear button are enabled. Pressing the Reset button next to the scene disables them.

Steps to reproduce

  1. In the State wrapper set state rate= 0
  2. On either screen, press Play
  3. Pause after some time has passed
  4. Launch sim--buttons are enabled
  5. Press Reset button--buttons are disabled
  6. Press Reset All button--buttons are enabled

Should both the Clear button and Restart buttons be disabled when the sim is launched? This would prevent issues like the one reported by @KatieWoe in https://github.com/phetsims/gravity-and-orbits/issues/457

@arouinfar @samreid thoughts?

Visuals

https://user-images.githubusercontent.com/87318828/193315481-0dd235ac-ad25-48d0-a3d0-10d9f838d340.mp4

arouinfar commented 2 years ago

Typically the Restart Button restores one of two possible states

  1. The default configuration of the scene (if no custom configuration exists). This is identical in function to the scene reset button.
  2. A custom configuration created while the sim was paused

With PhET-iO this becomes a bit more complicated. When the Standard PhET-iO Wrapper is created, the current state becomes the state restored by the ResetAllButton, the scene reset button, and the Restart Button (at least until the user creates a new custom configuration).

I think there's a problem with how the Restart Button restores the saved configuration. It seems to always reset the time to zero, regardless of the time displayed when the saved configuration was created (this is true even in the 1.5 version). This seems like a bug to me.

Should both the Clear button and Restart buttons be disabled when the sim is launched?

The Clear button should be enabled if the displayed time is non-zero. In the video, pressing the scene reset button should have re-enabled the Clear button because it restores the state to 20 Earth Minutes.

If the sim is launched while paused, the Restart button should definitely be disabled, since the initial state should be identical to the what is restored by the Restart button. If this sim is playing, I would expect the button to only be disabled during the first frame, but once the objects start moving, the button would be enabled.

samreid commented 2 years ago

@marlitas and I reviewed the issue and summarized the recommendations:

samreid commented 2 years ago

@marlitas and I made good progress, and this issue is ready for cherry-picking.

Nancy-Salpepi commented 2 years ago

Things look good in master and 1.6.0-rc.3. For:

The Restart Button should set time back to stored time (the time saved in state), not zero

In a phet-io saved state, the Restart button shouldn't set the time readout back to 0. It should be the time when the state was captured.

If I launch the sim after a certain amount of time has passed (or open saved version), press the clear button so time = 0, press Play and then press Restart, the time returns to 0 and not the time of the original. @samreid based on your https://github.com/phetsims/gravity-and-orbits/issues/455#issuecomment-1270005253 that sounds correct so I'm going to close this issue. If I'm wrong....I'm sorry and please reopen!

samreid commented 2 years ago

Yes, Restart is supposed to go to the beginning of the experiment, and not the stored PhET-iO initial state, so that sounds reasonable. Thanks!

zepumph commented 1 year ago

There is one more TODO for this issue, see https://github.com/phetsims/gravity-and-orbits/blob/407291864f66b7e30b99202d0e3ee32c18ebead5/js/common/model/Body.ts#L232.

From https://github.com/phetsims/chipper/issues/1372

samreid commented 1 year ago

Fixed as @zepumph prescribed, it seemed straightforward. Closing.