phetsims / number-line-integers

"Number Line: Integers" is an educational simulation in HTML5, by PhET Interactive Simulations.
GNU General Public License v3.0
0 stars 4 forks source link

Values don't match after pressing Restart #114

Closed Nancy-Salpepi closed 12 months ago

Nancy-Salpepi commented 1 year ago

Test device MacBook Air M1 chip and Dell

Operating System 14.0 and Win10

Browser Safari 17 and Chrome

Problem description For https://github.com/phetsims/qa/issues/1002, pressing the Restart button in the Elevation scene on the Explore screen will cause the absolute values to no longer match the actual values.

Steps to reproduce

  1. In the Elevation scene, check Absolute Value
  2. Add the character, bird and fish to the scene
  3. Press Restart
  4. Add the character, bird and fish to the scene

Visuals

Screenshot 2023-11-14 at 8 18 28 AM
Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Number Line: Integers‬ URL: https://phet-dev.colorado.edu/html/number-line-integers/1.2.0-dev.5/phet/number-line-integers_all_phet.html Version: 1.2.0-dev.5 2023-11-13 16:44:44 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/119.0.0.0 Safari/537.36 Language: en-US Window: 1386x728 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: {}
KatieWoe commented 1 year ago

Also happens with the reset scene button or even just adding a character, taking them off the scene, then adding them back in.

marlitas commented 1 year ago

I believe this is fixed above, but I would like to check in with @jbphet about it before closing. It relies on an assumption that a PointController's numberLinePoints never has more than one active numberLinePoint at a time. In some testing it seems like that's accurate, but I'm not convinced since the type of that property is an ObservableArray.

@Nancy-Salpepi or @KatieWoe, I think it's ready for you to check it on main but please leave open until I have a chance to meet with JB about it.

Nancy-Salpepi commented 1 year ago

@marlitas the text isn't correct this morning:

Screenshot 2023-11-15 at 9 13 27 AM
marlitas commented 1 year ago

Oh boy... How did that happen? I tested it last night... I'm mostly away today, but I'll be able to check on it this afternoon.

marlitas commented 1 year ago

Okay... let's try this again. To confirm, this is what I'm seeing:

image

@Nancy-Salpepi can you check it on main?

Nancy-Salpepi commented 1 year ago

This looks fixed now on main @marlitas! 🙂

marlitas commented 1 year ago

While we met @jbphet I noticed that I no longer needed to wait until the multi-link to create the absolute value text. The most recent commit refactors the code to not have to use that awkward work-around. Ready to go for your review. Thanks!

jbphet commented 1 year ago

@marlitas - The changes mostly look fine. I made some edits of my own, so I'm passing it back to you for final review and possible closure.

marlitas commented 12 months ago

@jbphet those are great edits. Thanks so much for cleaning that up. I believe this is ready to close!