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

Balls can escape reflecting border if mass is changed when near the edge #167

Closed arouinfar closed 3 years ago

arouinfar commented 4 years ago

For https://github.com/phetsims/QA/issues/537

One way to reproduce:

  1. Explore 1D
  2. Slow motion (optional, just makes things a bit easier)
  3. Press play
  4. Pause sim just before ball 1 reaches the left wall (optional, but makes the next step easier to see)
  5. Increase mass of ball 1 to the max. Its radius will expand and part of the ball will now be out-of-bounds image
  6. Press play. Ball 1 will travel off-screen.

I've lost a ball a few times this way, and I find it to be pretty problematic. Is there a way we could limit the range of acceptable masses based on the position? We already limit the range of positions based on the ball diameter. Another option would be to bump the ball away from the reflecting border if its diameter is too large, but that might have other unintended consequences.

jonathanolson commented 4 years ago

Is that worth considering a "partially-enabled range" on sliders? Can we consider just adjusting balls' positions if a mass change causes it to press against the wall (so that it stays in-bounds?) It looks like similar logic is in place to handle ball-ball overlap.

arouinfar commented 4 years ago

@jonathanolson I would be fine with using the enabledRange on the slider (which would also need to be applied to the keypad entry values) or with adjusting the ball's position. I think the former would probably look cleaner, but either would work for me. Is there a solution you prefer?

jonathanolson commented 3 years ago

I think it is much-less-complicated to handle this by adjusting the ball's position. The enabledRange would also decrease performance, as now we're animating sliders anytime a ball is next to a border, and it seems like a visual distraction.

Ok to proceed with the described approach?

arouinfar commented 3 years ago

Sounds good, go for it @jonathanolson.

jonathanolson commented 3 years ago

I've combined it with the currently-existing "bump away from other balls" behavior with the above commit. I believe it should handle degenerate cases too (putting a ball between another ball and a wall, etc.)

Can you try out master?

arouinfar commented 3 years ago

@jonathanolson this is working nicely in 1D, but something's not quite right in 2D.

  1. Explore 2D, 4 Balls
  2. Drag balls 2 and 4 to the right wall image
  3. Set ball 4 mass to max, it bumps away as expected. image
  4. Set ball 2 mass to max, and the sim freezes. When trying to refresh the page, the browser tab will crash. (Maybe we're stuck in an infinite loop?) image
jonathanolson commented 3 years ago

Figuring out the best fix, but I can reproduce and understand why this is happening.

jonathanolson commented 3 years ago

I believe the above patch should fix this, can you verify?

arouinfar commented 3 years ago

@jonathanolson the original issue has been addressed.

I do wonder if the "bump away" behavior can be fined tuned a bit further, though.

For example, if Ball 4 is dropped in this position, it gets bumped quite far away. I would expect it to get bump up a bit so it no longer overlaps Ball 2.

Screen Shot 2020-09-30 at 3 45 30 PM
arouinfar commented 3 years ago

@jonathanolson and I talked about this on 10/2 and decided not to make further changes to the bump away algorithm.

The situation in the comment above is definitely an edge case. Ball 4 has both a neighboring ball and wall. Since there is a wall to the right, it needs to bump to the left. The algorithm checks both sides of the other balls, but if there isn't a space large enough, the ball will bump away at some random angle.

I should add a note about the bumping algorithm to the Teacher Tips, see #156.