playcanvas / engine

JavaScript game engine built on WebGL, WebGPU, WebXR and glTF
https://playcanvas.com
MIT License
9.37k stars 1.32k forks source link

Physics methods should validate input #4504

Open echo-bravo-yahoo opened 1 year ago

echo-bravo-yahoo commented 1 year ago

Description

Hi! Filing this bug report from my phone, so can't make a minimal reproduction now, but can if one would be helpful.

Many of the physics APIs (e.g., RigidBodyComponent.applyImpulseRotation) take either a Vec3 or the component numbers of a Vec3. However, they do not throw if they're provided an Array instead of a Vec3.

These methods (when erroneously called with an Array) often cause the object to disappear / end up somewhere very out of frame. I haven't spent much time investigating what happens once I realized what the issue was.

It seems to me that it would be better mannered for the RigidBodyComponent methods to throw on bad input.

Steps to Reproduce

  1. Make an Entity with a RigidBodyComponent and Script.
  2. In the script, call RigidBodyComponent.applyImpulseRotation, and provide an array with 3 components (e.g., [1, 1, 1]).
  3. Observe that the Entity is no longer in the expected location.
LeXXik commented 1 year ago

There is an API documentation for such cases. It specifies what attributes are accepted by various methods of the engine. Please, refer to it whenever in doubt.

For example: https://developer.playcanvas.com/api/pc.RigidBodyComponent.html#applyImpulse

echo-bravo-yahoo commented 1 year ago

Hey! I'm aware the documentation exists (and I refer to it frequently), but it should still validate input. Many of the non-physics APIs throw errors in similar cases.

mvaligursky commented 1 year ago

we have an issue on this, but have not had a time to work on it yet https://github.com/playcanvas/engine/issues/2529