pmndrs / use-cannon

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

Trying to set rotation from a quaternion leads to unpredictable results #259

Closed doglet23 closed 2 years ago

doglet23 commented 2 years ago

/worker.js line 118

else if (value instanceof Quaternion) {
    value.toEuler(state.tempVector)
    value = state.tempVector.toArray()
}

/worker.js line 195

case 'setQuaternion':
    state.bodies[uuid].quaternion.setFromEuler(props[0], props[1], props[2])
    break

Why is quaternion being converted to euler just to be converted back to a quaternion again when that causes loss of data? It should be the other way around. Eulers should be converted to quaternions since there's no loss of data. This is pretty annoying because i've set up cannon-es on a server and im broadcasting the quaternion of a body to the client and this conversion is most likely causing the client body to rotate unpredictably.

stockhuman commented 2 years ago

Hi @doglet23, I see this issue has made you join GitHub! Welcome :)

You're right, it is a little silly, and whilst I did not write those particular lines, we can see from the rest of the API that we've settled on passing information in Euler angles for human readability and consistency.

To fix this, I'd propose checking for a fourth argument passed to setQuaternion, and setting the rotation directly in the format provided, falling back to .setFromEuler(props[0], props[1], props[2]) as necessary. Note however that propsToBody, types within setup.ts, and hooks all expect and operate with the assumption that they will be provided rotations in Euler angles.

doglet23 commented 2 years ago

hey @stockHuman. thanks for the response. to be honest i've figured out a work around for this by converting the quaternion to a threejs euler with 'XYZ' order which only causes some minor tremors. I think the issue was that the toEuler() method used in worker is default 'ZYX' order. so for now i guess im content.