search

phetsims / gas-properties

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

Erase doesn't remove particles outside of container while sim is paused #268

Closed Nancy-Salpepi closed 3 months ago

Nancy-Salpepi commented 3 months ago

Test device MacBook Air M1 chip

Operating System 14.5

Browser Safari 17.5

Problem description Seen while testing https://github.com/phetsims/gas-properties/issues/263#event-13193146270, but unsure if this is related to changes made there. After blowing the lid and pressing pause, pressing erase doesn't remove the particles outside of the container until the Play button is pressed. This is different behavior than in published.

Steps to reproduce It is easiest to reproduce in the PhET brand sim:

  1. On any of the first 3 screens, add max blue particles
  2. Heat container
  3. Press Pause once lids blows
  4. Press Return Lid button (or not)
  5. Press Erase button
  6. Press Play button

Visuals

Screenshot 2024-06-18 at 8 04 06 AM Screenshot 2024-06-18 at 8 03 32 AM
pixelzoom commented 3 months ago

Reproduced in main.

This problem was introduced sometime between 1.1.0-dev.3 (7/8/2020) and 1.1.0-dev.4 (11/17/22).

pixelzoom commented 3 months ago

The model was correctly updating, but the view was not. Fixed in the above commit.

@Nancy-Salpepi please review, close if OK.

Nancy-Salpepi commented 3 months ago

I'm not sure why my brain keeps thinking of these odd things to do but......

  1. On any of the first 3 screens, add max blue particles
  2. Heat container
  3. Press Pause once lids blows
  4. Press Return Lid button (or not)
  5. Decrease the number of blue particles to zero using the decrement Course button
  6. Try to press the Erase button to clear the particles outside the container (not possible to press)

https://github.com/phetsims/gas-properties/assets/87318828/1db6398e-47b8-4870-9ae6-21a75354fdd1

Nancy-Salpepi commented 3 months ago

Aside from that, the original problem is fixed.

pixelzoom commented 3 months ago

@Nancy-Salpepi and I looked at what she reported in https://github.com/phetsims/gas-properties/issues/268#issuecomment-2176888383.

Particles outside the container were always intended to be treated as "outside the system", and eraseParticlesButton applicable only to particles inside the container. There is unfortunately no model Property that tracks how many particles exist outside the container, so no way to include that information in the derivation of eraseParticlesButton.enabledProperty. We could simply make eraseParticlesButton be always enabled. But we discovered that the particles outside the container will be erased only if there is at least 1 particle inside the container - the model is updated, but the view is not.

I'm leaning towards closing this as "won't fix". But I'll investigate a bit more.

pixelzoom commented 3 months ago

I'm leaning towards closing this as "won't fix". But I'll investigate a bit more.

This would require introducing new Properties to track the number of particles outside the container, and it would be a few hours of work. It doesn't seem worth the effort, since this is a bit of a corner case, and doesn't impact usability or learning goals. So I'm going to treat https://github.com/phetsims/gas-properties/issues/268#issuecomment-2176888383 as "won't fix". @Nancy-Salpepi thanks for reporting.

Since the original problem was verified as fixed, closing this issue.