Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Physics methods should validate input #4504

Closed
echo-bravo-yahoo opened this issue Aug 3, 2022 · 4 comments
Closed

Physics methods should validate input #4504

echo-bravo-yahoo opened this issue Aug 3, 2022 · 4 comments
Labels
area: physics Physics related issue docs Documentation related feature request

Comments

@echo-bravo-yahoo
Copy link

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
Copy link
Contributor

LeXXik commented Aug 3, 2022

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
Copy link
Author

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.

@yaustar yaustar added feature request docs Documentation related area: physics Physics related issue labels Aug 3, 2022
@mvaligursky
Copy link
Contributor

we have an issue on this, but have not had a time to work on it yet
#2529

@mvaligursky
Copy link
Contributor

Over the last few years, the types the engine exports for the APIs has improved a lot. Also the new script editor we use in the Editor is based on Monaco (same code base used by VS Code), and this highlights issues if an incorrect type of the parameter is used. We also have plugin for VS Code that can be used to edit the scripts, highlighting these issues.

At the moment, we're not planning to add runtime checking.

There is also a community contributed tool that can help with this: #5817

And so I'll close it as Won't do at this stage.

@mvaligursky mvaligursky closed this as not planned Won't fix, can't repro, duplicate, stale Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: physics Physics related issue docs Documentation related feature request
Projects
None yet
Development

No branches or pull requests

4 participants