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

Custom launch mechanism on measure screen does not reset #288

Closed
Tracked by #1079
KatieWoe opened this issue Apr 11, 2024 · 7 comments
Closed
Tracked by #1079

Custom launch mechanism on measure screen does not reset #288

KatieWoe opened this issue Apr 11, 2024 · 7 comments

Comments

@KatieWoe
Copy link

Test device
Samsung
Operating System
Win 11
Browser
Chrome
Problem description
For phetsims/qa#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: {}

@KatieWoe KatieWoe added the type:bug Something isn't working label Apr 11, 2024
@matthew-blackman
Copy link
Contributor

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.

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();
   }

@matthew-blackman matthew-blackman added the dev:help-wanted Extra attention is needed label Apr 11, 2024
samreid added a commit that referenced this issue Apr 12, 2024
@samreid
Copy link
Member

samreid commented Apr 12, 2024

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.

@samreid samreid assigned KatieWoe and unassigned matthew-blackman and samreid Apr 12, 2024
@samreid samreid added status:ready-for-review and removed dev:help-wanted Extra attention is needed labels Apr 12, 2024
@KatieWoe
Copy link
Author

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

samreid commented Apr 15, 2024

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.

@samreid samreid removed their assignment Apr 15, 2024
@KatieWoe
Copy link
Author

Thanks!

@matthew-blackman
Copy link
Contributor

Please close after verifying.

@KatieWoe
Copy link
Author

Looks good in rc.2

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