-
-
Notifications
You must be signed in to change notification settings - Fork 180
Add edge and contour shape to physics engine #1113
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
Conversation
@@ -293,6 +293,40 @@ type [<ReferenceEquality>] PhysicsEngine3d = | |||
Log.info "Rounded box not yet implemented via PhysicsEngine3d; creating a normal box instead." | |||
let boxShape = { Size = boxRoundedShape.Size; TransformOpt = boxRoundedShape.TransformOpt; PropertiesOpt = boxRoundedShape.PropertiesOpt } | |||
PhysicsEngine3d.attachBoxShape bodyProperties boxShape scShapeSettings masses | |||
|
|||
/// Jolt does not support chain shapes natively, so each link is approximated with a capsule with tiny radius. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using untested AI code as a placeholder, maybe just use attachPointsShape for now with a Log.warn that it is degrading.
Nu/Nu/Physics/PhysicsEngine2d.fs
Outdated
let vertices' = Array.zeroCreate chainShape.Links.Length | ||
for i in 0 .. dec chainShape.Links.Length do | ||
vertices'.[i] <- PhysicsEngine2d.toPhysicsV2 (chainShape.Links.[i].Transform transform) | ||
let bodyShape = (if chainShape.Closed then body.CreateLoopShape else body.CreateChainShape) (Common.Vertices vertices') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's just use a normal if else expression with duplicated function names rather than this sort of trivial currying in order to preserve clarity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Address the one remaining comment and I'll squash merge and clean up any additional things.
Nu/Nu/Physics/PhysicsEngine3d.fs
Outdated
static member private attachChainShape (bodyProperties : BodyProperties) (chainShape : Nu.ChainShape) (scShapeSettings : StaticCompoundShapeSettings) masses = | ||
Log.warnOnce "3D chain shapes are currently unsupported. Degrading to a convex points shape." | ||
PhysicsEngine3d.attachPointsShape bodyProperties { Points = chainShape.Links; Profile = Convex; TransformOpt = chainShape.TransformOpt; PropertiesOpt = boxRoundedShape.PropertiesOpt } scShapeSettings masses | ||
(* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commenting out non-trivial untested AI gen'd code doesn't actually solve the violation of standard A.12 -
Never add AI-generated code that isn't tested! AI code often includes subtle landmines, and checking it in untested leaves landmines for other developers who may not even know it's AI-generated!
Rather it merely also ends up violating standard C.3 as well -
Avoid checking in dead / commented-out code. If unavoidable, leave a comment above the code explaining why it's commented out and / or when it will be useful again.
This code cannot come into the repository in any form, commented out or otherwise.
If you wish to preserve it for later use (such as when we have time to test and potentially fix it), its presence in this PR's history should be adequate enough.
Will squash this merge to keep untested AI code out of history. |
No description provided.