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

Max Call Stack exceeded for slip collisions #129

Closed brandonLi8 closed 4 years ago

brandonLi8 commented 4 years ago

With the last preset for the 'Inelastic' screen on "slip", there appears to be infinite recursion. Strangely, the problem goes away when the reset button is pressed. Looking into it.

brandonLi8 commented 4 years ago

Able to reproduce. Doesn't seem to happen when the step button is pressed with ?timeStepDuration=1

brandonLi8 commented 4 years ago

Fixed in the commits above. Closing

brandonLi8 commented 4 years ago

Reopening, the changes that I implemented above works for this scenario. However, now multiple balls colliding at the same time is not handled correctly.

brandonLi8 commented 4 years ago

I committed a workaround above. Essentially, I've figured out that the problem is that after the collision occurs, Ball2's velocity is tangential to Ball1's velocity (instead of "away" from) and the balls are tangentially touching. image

Our collision detection then thinks that the collision still is happening on each step call (in the next 0 seconds), causing a max call stack error. This problem is, thus, impossible for all other screen, since there are no perfectly inelastic collisions for Explore 2D and the other screens are 1D.

My first solution was to ignore collisions that happen in the next 0 seconds, but that broke the collision engine's handling of multiple collisions that truly occur at the same time.

The workaround I implemented was to step the balls by 1E-14 seconds after a slip inelastic collision occurs in 2D. This will move the balls forwards by an small quantity (around 1E-14), meaning the balls are not tangentially touching and collisions are no longer detected.

I think this workaround works really well for this problem and I'm happy with it. Closing.

brandonLi8 commented 4 years ago

Reopening due to #132.

brandonLi8 commented 4 years ago

132 was fixed. I confirmed that this scenario is no longer buggy. Closing again.