phetsims / collision-lab

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

On 1D, screen crashes if ball has nowhere to go #191

Closed brooklynlash closed 3 years ago

brooklynlash commented 3 years ago

Test device Lenovo ThinkPad

Operating System Win10

Browser Chrome

Problem description Found when testing issues related to https://github.com/phetsims/collision-lab/issues/179, for https://github.com/phetsims/QA/issues/594. With 5 masses on the 1D collision screen, sometimes when the masses are increased they move around another mass instead of overlapping, but if there is not a space for one of the masses, the entire screen freezes. I don't get any warnings or errors on console besides the warning that the frame rate is slow.

Visuals This is a long gif because it can take a long time to reproduce. https://drive.google.com/file/d/1iZMWeCl22P1EtiyEFjSXmM_AqV8I1wud/view?usp=sharing

Troubleshooting information: !!!!! DO NOT EDIT !!!!! Name: ‪Collision Lab‬ URL: https://phet-dev.colorado.edu/html/collision-lab/1.1.0-dev.13/phet/collision-lab_en_phet.html Version: 1.1.0-dev.13 2021-01-06 18:20:45 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/87.0.4280.88 Safari/537.36 Language: en-US Window: 1536x722 Pixel Ratio: 2.5/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: 4095 Texture: size: 16384 imageUnits: 16 (vertex: 16, combined: 32) Max viewport: 32767x32767 OES_texture_float: true Dependencies JSON: {}

jonathanolson commented 3 years ago

@brooklynlash any idea what I need to do to be able to view the above link? I haven't been able to watch the video reproduction yet.

brooklynlash commented 3 years ago

@jonathanolson Oops, forgot to give access through the link. It should work now.

jonathanolson commented 3 years ago

Ahh, I see that it can be set up where four balls can be positioned so that there is no valid position for the 5th ball if it has a mass increase:

image

@arouinfar thoughts on how to tweak this?

In general, I've preferred methods of "handling overlap" where it iteratively bumps things apart (moving everything that overlaps), as this guarantees things will adjust to fit AND looks more natural instead of switching things to the "other" side. Do you think it's worth replacing the current "handle overlap" behavior fully? OR just detecting this case, and doing the more robust method there?

arouinfar commented 3 years ago

In general, I've preferred methods of "handling overlap" where it iteratively bumps things apart (moving everything that overlaps), as this guarantees things will adjust to fit AND looks more natural instead of switching things to the "other" side. Do you think it's worth replacing the current "handle overlap" behavior fully? OR just detecting this case, and doing the more robust method there?

I think the current method of "handling overlap" is working nicely, so I wouldn't want to fully change the behavior. Let's try detecting this specific case and employing a more robust method only as necessary.

jonathanolson commented 3 years ago

Added the robust handling after (it shouldn't usually need to kick in). @KatieWoe can you confirm?

KatieWoe commented 3 years ago

This fix looks good on master

arouinfar commented 3 years ago

Looks good @jonathanolson

loganbraywork commented 3 years ago

Seems to be fixed as of https://github.com/phetsims/QA/issues/599