phetsims / balloons-and-static-electricity

"Balloons and Static Electricity" is an educational simulation in HTML5, by PhET Interactive Simulations.
http://phet.colorado.edu/en/simulation/balloons-and-static-electricity
GNU General Public License v3.0
6 stars 10 forks source link

When balloon is moved very slowly over sweater, negative charges are not picked up #526

Closed Nancy-Salpepi closed 2 years ago

Nancy-Salpepi commented 3 years ago

Test device MacBook Air (M1 chip)

Operating System 11.4

Browser Seen on both safari and chrome. Can also be reproduced using iPad + safari.

Problem description https://github.com/phetsims/qa/issues/689

When the balloon is moved slowly over the sweater, negative charges are not picked up.

Steps to reproduce

  1. Either "show all charges" or "show charge differences" radio button should be selected
  2. Slowly move balloon over sweater using trackpad (or finger with iPad).

I do notice that this occurs in published sim as well.

Visuals https://drive.google.com/file/d/1Fw5_RPW3rhOOH5vt6MwWwVfKX19_sZ5A/view?usp=sharing https://drive.google.com/file/d/1jFPf06UuHFnqILTu912oP2vxbm5501lP/view?usp=sharing

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Balloons and Static Electricity‬ URL: https://phet-dev.colorado.edu/html/balloons-and-static-electricity/1.5.0-dev.35/phet/balloons-and-static-electricity_all_phet.html Version: 1.5.0-dev.35 2021-08-06 21:46:41 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/92.0.4515.131 Safari/537.36 Language: en-US Window: 1440x693 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: 31 uniform: 1024 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 80) Max viewport: 16384x16384 OES_texture_float: true Dependencies JSON: {}

brooklynlash commented 3 years ago

I also reproduced this on Chromebook.

jbphet commented 3 years ago

I think this should be fixed, and I suspect that it won't be trivial. Even though this exists in the published sim (as @Nancy-Salpepi noted above), I think we should fix it, primarily because the behavior occurs if you pick up the balloon using keyboard nav and then move it using the shift+arrow keys. Not good.

jbphet commented 3 years ago

I have a fix that seems to work reasonably well for this, which is to reduce the threshold for the minimum speed at which the balloon picks up changes to any value above zero. This make me a little nervous though, because I'm not sure why a threshold was used in the first place. I went through the history of the file, and as far as I can tell, the threshold was pretty much always there and was not added to address any particular issue.

Here's a patch with the proposed change:

``` Index: js/balloons-and-static-electricity/model/BalloonModel.js IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/js/balloons-and-static-electricity/model/BalloonModel.js b/js/balloons-and-static-electricity/model/BalloonModel.js --- a/js/balloons-and-static-electricity/model/BalloonModel.js (revision 02a386fe06b37795056afe67129c3d74a378c84d) +++ b/js/balloons-and-static-electricity/model/BalloonModel.js (date 1629829717231) @@ -27,7 +27,6 @@ // constants, most if not all of which were empirically determined to elicit the desired appearance and behavior const VELOCITY_ARRAY_LENGTH = 5; -const THRESHOLD_SPEED = 0.0125; const BALLOON_WIDTH = 134; const BALLOON_HEIGHT = 222; @@ -772,7 +771,7 @@ this.dragVelocityProperty.set( new Vector2( vx, vy ) ); let chargeFound = false; - if ( speed >= THRESHOLD_SPEED ) { + if ( speed > 0 ) { chargeFound = model.sweater.checkAndTransferCharges( this ); } ```
jbphet commented 3 years ago

I've decided to go ahead and commit the change described in the previous comment. I did a fair bit of local testing and didn't see any regressions, and also fuzz tested for a few minutes.

@Nancy-Salpepi - Can you please test this on master and verify that it's fixed and that you don't notice any odd side effects? I'd just like to get another pair of eyes on this before I decide for sure whether this is a reasonable fix.

jbphet commented 3 years ago

For the record, it was bothering me that the charges are being picked up based on the rectangular bounds of the balloon image and not something shaped more like the balloon, such as an ellipse, so I looked at the code that tests this. There was an explanation in a comment that said the rectangular bounds were used for performance reasons, and it referenced https://github.com/phetsims/balloons-and-static-electricity/issues/409.

This somewhat odd behavior is pretty noticeable when moving the balloon really slowly, so I found it very apparent when testing changes that were made for this issue, but I have to say that I had never really noticed it before working on this. In the end, that probably implies that it's not really much of a problem.

Nancy-Salpepi commented 3 years ago

Works great @jbphet!

jbphet commented 3 years ago

Cool. Unassigning, and will have this verified in the upcoming RC.

Nancy-Salpepi commented 3 years ago

Tested on the iPad and on the MacBook air. Looks good.

@jbphet I do see what you are saying in the above comment https://github.com/phetsims/balloons-and-static-electricity/issues/526#issuecomment-904923198

It was bothering me and then I remembered that you had also commented on it. In the video below it seems odd to me that the electrons closest to the balloon are not being picked up first.

https://user-images.githubusercontent.com/87318828/132747556-41f79150-13dc-4bb7-83bf-0841036e3a7a.mov

jbphet commented 2 years ago

@Nancy-Salpepi said:

@jbphet I do see what you are saying in the above comment #526 (comment)

It was bothering me and then I remembered that you had also commented on it. In the video below it seems odd to me that the electrons closest to the balloon are not being picked up first.

I looked at the code, and it would be a big lift to change this at this point in time. It's a bit more noticeable now that very slow motion will pick up electrons, but the balloons in this sim have always picked up charges in this way, and we haven't yet found it necessary to change it so far. In my opinion it doesn't seem too problematic, and in the vast majority of cases people won't be moving the balloon this slowly. So, I'm not going to pursue changes to this behavior, and I don't think it's even worth asking the design team at this point.

@Nancy-Salpepi also said:

Works great @jbphet!

So, based on this, I'm going to close.