search

pmndrs / use-cannon

👋💣 physics based hooks for @react-three/fiber
https://cannon.pmnd.rs
2.76k stars 154 forks source link

Fix sphere collisions #224

Closed David-Beale closed 3 years ago

David-Beale commented 3 years ago

On line 30, the shape is being passed an array instead of a number:

return new Sphere(...prepareSphere(args)) // radius = args

Cannon converts this into a string for some reason and screws up all the collision boundaries.

Changing this messes up the debug view so I added a quick fix to convert it back to a number

David-Beale commented 3 years ago

223

drcmda commented 3 years ago

this bit is strangely flipping ever since that props to body thing was made.

this "fixes" it by moving from array notation to a number https://github.com/pmndrs/use-cannon/commit/21b046c80574871ae5feeac398f1159824f9c87b#diff-a66e81f364e21ca212f52503975a9a7f2f2a14d021e7ce3b1a8285ed34c31043

this then "fixes" it by moving from number notation back to an array https://github.com/pmndrs/use-cannon/commit/699ab15d5e7a1cf9ecdf4f521a780ea062672520#diff-a66e81f364e21ca212f52503975a9a7f2f2a14d021e7ce3b1a8285ed34c31043

where does this come from? is it an array or a string? why does one break the other? there must be one of the three that's wrong: cannon-es, cannon-es-debugger and use-cannon. so no matter what we do here, it will always break.

it would be good to be very strict typing here or unify types into all-arrays like we have done in r3f for args no matter if something takes a single argument or multiple.

bjornstar commented 3 years ago

where does this come from? is it an array or a string? why does one break the other? there must be one of the three that's wrong: cannon-es, cannon-es-debugger and use-cannon. so no matter what we do here, it will always break.

I'm also curious to see where the string is coming from. It would be nice to have some failing code so we can make sure it stays fixed.

it would be good to be very strict typing here or unify types into all-arrays like we have done in r3f for args no matter if something takes a single argument or multiple.

I would prefer consistency, args always being an array would simplify the interface a bit but it will break a lot of existing code. The big issue here is that propsToBody and worker are not typed. I started #225 for propsToBody.

bjornstar commented 3 years ago

@David-Beale Can you check again with the latest commits? This branch is a bit behind master.

David-Beale commented 3 years ago

Sorry, I was passing in the args as an array instead of a num :man_facepalming: