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

feat: [Physics] Adding margins to convex hull colliders (Issue1577) #2257

Conversation

dloe
Copy link
Contributor

@dloe dloe commented May 20, 2024

PR Details

Allow specification of margin for collider shapes. Shows to be useful for ConvexHull colliders.

Description

Margin property now able to be set on ConvexHull collider shape. Added margin getter and setting vars onto ConvexHullColliderShape as well as the viewable field in the properties box for the user to adjust. The base class ColliderShape contains where margin is defined as recommened so all collider shapes can potentially benefit from the new variable.

Related Issue

Link to issue:
#1577

Link to initial discussion:
#1574

Motivation and Context

Originally reported as a missing field that solves an issue related to a test project not being able to adjust the corresponding collider. This margin not being able to be adjusted was causing unintended collisions and not allowing the user to be able to properly set up their ConvexHullCollider properties to fix the problem in the property box.

For additonal testing, I made a fork of the users orignal project that showcases the issue here:
https://github.com/dloe/StrideEngineIssue1577_RigidBodyPhysicsBugProject

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • [x ] New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My change requires a change to the documentation.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • [x ] I have built and run the editor to try this change out.

dloe added 2 commits May 20, 2024 11:35
Added margin field to parent class ColliderShape, therefore all collider shapes can have access.
@dloe
Copy link
Contributor Author

dloe commented May 21, 2024

@dotnet-policy-service agree

@dloe please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@dotnet-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@dotnet-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@dotnet-policy-service agree company="Microsoft"

Contributor License Agreement

/// The Margin of the generated convex hull.
/// </userdoc>
[DataMember(48)]
public float Margin { get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a small note, it looks like margins in bullet by default should be around 0.04 for dynamic and 0.0 for static objects. Convex hulls are likely used as replacement for the lack of support of non-static meshes, so best set it to the default value for dynamics, so 0.04 by default.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, adding that now

dloe added 2 commits May 22, 2024 12:00
Context: Margins in bullet by default should be around 0.04 for dynamic and 0.0 for static objects. Convex hulls are likely used as replacement for the lack of support of non-static meshes, so best set it to the default value for dynamics, so 0.04 by default.
@Eideren
Copy link
Collaborator

Eideren commented May 22, 2024

Looks good, thanks a bunch !

@Eideren Eideren merged commit da1d50c into stride3d:master May 22, 2024
2 checks passed
@Eideren Eideren changed the title [Physics] Adding margins to convex hull colliders (Issue1577) feat: [Physics] Adding margins to convex hull colliders (Issue1577) May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants