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

Fire/Ice sputters while changing Particle amount #283

Closed Nancy-Salpepi closed 3 months ago

Nancy-Salpepi commented 4 months ago

Test device MacBook Air M1 chip

Operating System 14.5

Browser Chrome

Problem description For https://github.com/phetsims/qa/issues/1107 on the Ideal Screen with PressureT radio button selected, as I change the amount of particles the fire/ice will sputter the whole time I hold down an increment/decrement button. This doesn't happen in Published.

Steps to reproduce

  1. On the Ideal Screen add some particles
  2. Select the PressureT radio button
  3. Press and hold any increment/decrement button for red/blue particles

Visuals

https://github.com/user-attachments/assets/fe0aa463-930d-41fc-abb3-802c0ec4e798

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Gas Properties‬ URL: https://phet-dev.colorado.edu/html/gas-properties/1.1.0-rc.1/phet/gas-properties_all_phet.html Version: 1.1.0-rc.1 2024-07-11 00:18:26 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/126.0.0.0 Safari/537.36 Language: en-US Window: 1279x730 Pixel Ratio: 2/1 WebGL: WebGL 1.0 (OpenGL ES 2.0 Chromium) GLSL: WebGL GLSL ES 1.0 (OpenGL ES GLSL ES 1.0 Chromium) Vendor: WebKit (WebKit WebGL) Vertex: attribs: 16 varying: 30 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}
pixelzoom commented 4 months ago

Reproduced in main.

What you're observing is the heater/cooler animation starting over, which happens whenever the number of particles changes. Relevant code is in GasPropertiesHeaterCoolerNode.ts startAnimation.

pixelzoom commented 4 months ago

Looks like this was introduced in https://github.com/phetsims/gas-properties/commit/df4dcfb6c9b370eb37767df6152085c039414c51 for https://github.com/phetsims/gas-properties/issues/227, where animation changes were made for PhET-iO compatibility.

pixelzoom commented 4 months ago

@Nancy-Salpepi please review in main. Leave open for verification in 1.1.0-rc.2.

pixelzoom commented 4 months ago

Reminder to self that I need to cherry-pick https://github.com/phetsims/gas-properties/commit/97f65d111c7ef547a3cf5b21a930c1ea5d0b7565 into gases-intro. It is irrelevant for diffusion.

pixelzoom commented 4 months ago

I decided that it's going to be easier to patch any changes to gas-properties into all 3 release branches, regardless of whether they are relevant to all 3 sims. So that's what I did in the commits above.

Nancy-Salpepi commented 4 months ago

Working nicely in main.

pixelzoom commented 4 months ago

Ready for verification in 1.1.0-rc.2.

pixelzoom commented 4 months ago

Please verify for https://github.com/phetsims/qa/issues/1123 and https://github.com/phetsims/qa/issues/1124. (This issue is irrelevant for the Diffusion sim).

To verify, follow "Steps to reproduce" in https://github.com/phetsims/gas-properties/issues/283#issue-2408696594.

If everything looks OK, please close this issue.

Nancy-Salpepi commented 3 months ago

Looks good in rc.2 for Gas Properties and Gases Intro. Closing