Prozi / detect-collisions

Points, Lines, Boxes, Polygons (also hollow), Ellipses, Circles. RayCasting, offsets, rotation, scaling, bounding box padding, flags for static and ghost/trigger bodies
https://prozi.github.io/detect-collisions/
MIT License
207 stars 22 forks source link

forEach in checkAll() and asynchronous operations in callback #40

Closed trekze closed 2 years ago

trekze commented 2 years ago

We've upgraded from a much older version version of detect-collisions, and it's mostly working as intended, aside for some rare race condition-like symptoms. Our handlecollisions callback has async calls in it at times. Wondering if this might be caused by checkAll() using foreach internally instead of for...of. Is it possible this might cause several of the callbacks to be called in parralel?

Thanks for your help. Amazing library.

Prozi commented 2 years ago

Is it possible this might cause several of the callbacks to be called in parralel?

as far as I know javascript can't really execute something out of the box in paralel since its single threaded :)

I am not sure I know what you mean, can you provide me with some minimal sample of code?

trekze commented 2 years ago

Resolved by using our own loop and using getPotentials and checkCollision instead.

Javascript is single threaded, but asynchronous. So if you use an await in the checkAll() callback, you may get interleaved execution of multiple calls to the callback.

For example is the callback is something like:

{
A
B
await someAsyncFunction()
C
D
}

You could get the following execution path (if it's called twice):

A B A B C D C D

Which can cause race conditions. Hope that makes. Resolved with workaround anyway so no need for changes for us.

Prozi commented 2 years ago

Resolved by using our own loop and using getPotentials and checkCollision instead.

I am glad you got it to work!

Which can cause race conditions. Hope that makes. Resolved with workaround anyway so no need for changes for us.

Yeah that makes sense, thank you for the explanation

may I close the issue now?

trekze commented 2 years ago

Sure