-
-
Notifications
You must be signed in to change notification settings - Fork 909
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
Revise Capsule Length Calculation #2086
base: master
Are you sure you want to change the base?
Conversation
Resolved shape generation issues by correcting the length calculation for the capsule. This fix ensures accurate dimensions and enhances overall project stability.
@dotnet-policy-service agree |
Hey @svaldenegro thanks for the PR :) Looks like you didn't adhere to the template github provides you with for the text of the pull request, I'll have to go over a couple of points with you:
Besides that, the PR looks good :) |
@Eideren here's a video with the changes. Net8.0-windows7.0.2023.12.20.-.20.58.56.03.mp4 |
The breaking change regarding how length is calculated is problematic. I think it should remain to be the distance between the centers. The full length can always be calculated by adding two radii. I'd prefer to add a new FullLength calculated property for that purpose instead of breaking the existing one. Edit: the whole point of having the length be the distance between the two points, is that it makes it independent from the radius. Now, it means you could have invalid values serialized (even if fixup by the min calculation later on). Edit 2: if the issue is how the value id displayed in the editor. Then it's different and could be handled with a custom editor logic to display that total length (though I personally prefer when the displayed value matches the serialized one, less magic is better). But changing the underlying code feels wrong to me. It sure will have impact on other parts (such as the physics) and would require more careful testing. Or if the calculation is wrong on the physics part, then it should be fixed in that place. |
var position = radius * normal + new Vector3(0, deltaY, 0); | ||
var position = radius * normal; | ||
position.Y += deltaY; |
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.
Why making this change? It is equivalent. I'd rather keep the file history clean and avoid changes that don't bring anything of value.
Resolved shape generation issues by correcting the length calculation for the capsule. This fix ensures accurate dimensions and enhances overall project stability.
Related Issue
Misscalculation in primitive capsule model.
Types of changes
Small changes in code to correct the length of the capsule and to prevent a radius higher than half the length.
The changes were made in the class: GeometricPrimitive.Capsule