phetsims / projectile-data-lab

"Projectile Data Lab" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 0 forks source link

Custom launch mechanism on measure screen does not reset #288

Closed KatieWoe closed 5 months ago

KatieWoe commented 5 months ago

Test device Samsung Operating System Win 11 Browser Chrome Problem description For https://github.com/phetsims/qa/issues/1068. Seen by @matthew-blackman On the Measure screen, the Custom Launcher setting does not reset with reset all. Steps to reproduce

  1. Go to the measure screen
  2. Select Custom Launcher
  3. Switch from spring to either air pressure or explosion
  4. Press reset all
  5. Select Custom Launcher again

Troubleshooting information:

!!!!! DO NOT EDIT !!!!! Name: ‪Projectile Data Lab‬ URL: https://phet-dev.colorado.edu/html/projectile-data-lab/1.0.0-rc.1/phet/projectile-data-lab_all_phet.html Version: 1.0.0-rc.1 2024-04-09 18:41:49 UTC Features missing: applicationcache, applicationcache, touch Flags: pixelRatioScaling User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/123.0.0.0 Safari/537.36 Language: en-US Window: 1536x695 Pixel Ratio: 1.25/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: 31 uniform: 4096 Texture: size: 8192 imageUnits: 32 (vertex: 32, combined: 64) Max viewport: 8192x8192 OES_texture_float: true Dependencies JSON: {}
matthew-blackman commented 5 months ago

The following patch appears to solve the problem. I'd like to review this with @samreid before committing, because I'm still not sure why the call to this.customLauncherMechanismProperty.reset(); in SMField.reset is not accomplishing the same thing.

``` diff Subject: [PATCH] Patch --- Index: js/common/model/Launcher.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/common/model/Launcher.ts b/js/common/model/Launcher.ts --- a/js/common/model/Launcher.ts (revision f6284ae547b92e59d1dbdb051152b65b49f62cad) +++ b/js/common/model/Launcher.ts (date 1712859154114) @@ -94,6 +94,10 @@ derive: launcherMechanism => launcherMechanism.speedStandardDeviationProperty } ); } + + public reset(): void { + this.launcherMechanismProperty.reset(); + } } const mysteryLaunchersTandem = Tandem.GLOBAL_MODEL.createTandem( 'mysteryLaunchers' ); Index: js/measures/model/MeasuresField.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/measures/model/MeasuresField.ts b/js/measures/model/MeasuresField.ts --- a/js/measures/model/MeasuresField.ts (revision f6284ae547b92e59d1dbdb051152b65b49f62cad) +++ b/js/measures/model/MeasuresField.ts (date 1712859158284) @@ -133,6 +133,9 @@ public override reset(): void { super.reset(); + //Reset all launchers + this.launchers.forEach( launcher => launcher.reset() ); + this.mysteryOrCustomProperty.reset(); this.mysteryLauncherProperty.reset(); }
samreid commented 5 months ago

Good find @KatieWoe. This also led me and @matthew-blackman to discover a similar problem with the angle stability not resetting (when a different launcher was selected). We corrected both and are ready for review in main.

KatieWoe commented 5 months ago

The issue with the custom launcher seems fixed, but I wasn't able to figure out what problem there was with the angle stability. I don't see a difference on main there.

samreid commented 5 months ago

but I wasn't able to figure out what problem there was with the angle stability

Testing with https://phet-dev.colorado.edu/html/projectile-data-lab/1.0.0-rc.1/phet/projectile-data-lab_all_phet.html

  1. Select Measures Screen
  2. Select Custom Launcher
  3. Change Angle Stability to max
  4. Switch to Mystery Launcher
  5. Press Reset All
  6. Switch to Custom Launcher

observe that it incorrectly remains at max angle stability (did not reset).

I confirmed this seems fixed in main. Reassigning to @KatieWoe to explore in this area.

KatieWoe commented 5 months ago

Thanks!

matthew-blackman commented 5 months ago

Please close after verifying.

KatieWoe commented 5 months ago

Looks good in rc.2