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

Reload kinematic shapes when changing PhysicsBody mode to Kinematic #53118

Merged
merged 2 commits into from
Sep 27, 2021

Conversation

BimDav
Copy link
Contributor

@BimDav BimDav commented Sep 27, 2021

This complements the PR #51857, as there is still a crash when changing the mode to Kinematic.

Reload kinematic shapes when changing PhysicsBody mode to Kinematic to prevent a crash when calling test_body_motion.
Call reload_kinematic_shapes from init_kinematic_utilities as they are always called together.

Bugsquad edit: Fixes #53125.

…o prevent a crash when calling test_body_motion. Call reload_kinematic_shapes from init_kinematic_utilities as they are always called together.
@BimDav BimDav requested a review from a team as a code owner September 27, 2021 08:55
@BimDav
Copy link
Contributor Author

BimDav commented Sep 27, 2021

Fixes #53125

@akien-mga akien-mga added this to the 4.0 milestone Sep 27, 2021
@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 27, 2021
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.

Thanks for finding the issue and fixing it!

It looks great. I was just wondering, why did you decide to revert the change to avoid calling init_kinematic_utilities() directly when switching to Kinematic mode? Was it causing issues in some cases?

@BimDav
Copy link
Contributor Author

BimDav commented Sep 27, 2021

It looks great. I was just wondering, why did you decide to revert the change to avoid calling init_kinematic_utilities() directly when switching to Kinematic mode? Was it causing issues in some cases?

It don't think it causes issues either way but I think that the majority of cases for a PhysicsBody in Kinematic mode may be the KinematicBody nodes. So in that case, I think it makes more sense to initialize everything needed by kinematic methods at the body's initialization, not at the first call of a kinematic method. It was also one less change so I thought it was less risky. But either way is OK with me!

@pouleyKetchoupp
Copy link
Contributor

It looks great. I was just wondering, why did you decide to revert the change to avoid calling init_kinematic_utilities() directly when switching to Kinematic mode? Was it causing issues in some cases?

It don't think it causes issues either way but I think that the majority of cases for a PhysicsBody in Kinematic mode may be the KinematicBody nodes. So in that case, I think it makes more sense to initialize everything needed by kinematic methods at the body's initialization, not at the first call of a kinematic method. It was also one less change so I thought it was less risky. But either way is OK with me!

Alright, sounds good to me then :) It does make sense to just focus on fixing the crash in this PR.

@pouleyKetchoupp pouleyKetchoupp merged commit 27417c0 into godotengine:master Sep 27, 2021
@pouleyKetchoupp
Copy link
Contributor

Congrats on your first merged PR!

@akien-mga
Copy link
Member

Cherry-picked for 3.4.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 27, 2021
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.

[Bullet] body_test_motion causes crash when used with a Rigidbody in Kinematic mode and Bullet
3 participants