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.

Sign up for GitHub

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

Jump to bottom

Implement CollisionPolygon3D margin #45855

Merged
merged 1 commit into from
Feb 15, 2021
Merged

Implement CollisionPolygon3D margin #45855

merged 1 commit into from
Feb 15, 2021

Conversation

hoontee
Copy link
Contributor

@hoontee hoontee commented Feb 9, 2021 •
edited
Loading

Closes #45718.

@hoontee hoontee requested a review from a team as a code owner February 9, 2021 19:38
@hoontee
Copy link
Contributor Author

hoontee commented Feb 9, 2021

Please let me know if there are any issues with formatting or documentation.

@hoontee hoontee force-pushed the fix-45718 branch 4 times, most recently from 5ef4139 to 5867524 Compare February 9, 2021 20:03
@Calinou Calinou added topic:physics enhancement cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Feb 9, 2021
@Calinou Calinou added this to the 4.0 milestone Feb 9, 2021
@pouleyKetchoupp pouleyKetchoupp requested a review from a team February 10, 2021 00:56
pouleyKetchoupp
pouleyKetchoupp reviewed Feb 11, 2021
View reviewed changes
Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp left a comment

Choose a reason for hiding this comment

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

For info, the current plan is for collision margins to be removed in the future as Godot Physics 3D will become the default and Bullet Physics will be used as a separate plugin.

It's still fine to improve the API and documentation in the meantime, and it would make sense to backport this change to 3.2 as well.

doc/classes/CollisionPolygon3D.xml Outdated Show resolved Hide resolved
pouleyKetchoupp
pouleyKetchoupp approved these changes Feb 11, 2021
View reviewed changes
Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp left a comment

Choose a reason for hiding this comment

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

Great! Looks good to me.

Technically we don't need to rebuild the polygon when the margin is changed, but it would be more involved to improve that, and changing margins at runtime is very unlikely so it's fine to keep it simple.

@akien-mga akien-mga merged commit 3f6295a into godotengine:master Feb 15, 2021
@akien-mga
Copy link
Member

Thanks!

@hoontee hoontee deleted the fix-45718 branch February 15, 2021 15:28
@akien-mga
Copy link
Member

Cherry-picked for 3.2.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pouleyKetchoupp pouleyKetchoupp pouleyKetchoupp approved these changes

@akien-mga akien-mga akien-mga approved these changes

Assignees
No one assigned
Labels
enhancement topic:physics
Projects
None yet
Milestone
4.0
Development

Successfully merging this pull request may close these issues.

Unable to set margin on CollisionPolygon resulting in noticeable gaps when using Bullet engine
4 participants
@hoontee @akien-mga @pouleyKetchoupp @Calinou