-
-
Notifications
You must be signed in to change notification settings - Fork 21.1k
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
Checking if there is a collider when calling SoftBody::remove_collision_exception_with
#46704
Conversation
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.
Thanks for your contribution! Although instead of raising an error, this case could be handled properly in the call to remove_collision_exception
the same way it already does for add_collision_exception
.
This is the code for add_collision_exception
:
godot/modules/bullet/collision_object_bullet.cpp
Lines 138 to 147 in 17e6638
void CollisionObjectBullet::add_collision_exception(const CollisionObjectBullet *p_ignoreCollisionObject) { | |
exceptions.insert(p_ignoreCollisionObject->get_self()); | |
if (!bt_collision_object) { | |
return; | |
} | |
bt_collision_object->setIgnoreCollisionCheck(p_ignoreCollisionObject->bt_collision_object, true); | |
if (space) { | |
space->get_broadphase()->getOverlappingPairCache()->cleanProxyFromPairs(bt_collision_object->getBroadphaseHandle(), space->get_dispatcher()); | |
} | |
} |
The same check on bt_collision_object
could be done here:
godot/modules/bullet/collision_object_bullet.cpp
Lines 149 to 155 in 17e6638
void CollisionObjectBullet::remove_collision_exception(const CollisionObjectBullet *p_ignoreCollisionObject) { | |
exceptions.erase(p_ignoreCollisionObject->get_self()); | |
bt_collision_object->setIgnoreCollisionCheck(p_ignoreCollisionObject->bt_collision_object, false); | |
if (space) { | |
space->get_broadphase()->getOverlappingPairCache()->cleanProxyFromPairs(bt_collision_object->getBroadphaseHandle(), space->get_dispatcher()); | |
} | |
} |
And this method could also have a similar check:
godot/modules/bullet/collision_object_bullet.cpp
Lines 161 to 168 in 17e6638
void CollisionObjectBullet::set_collision_enabled(bool p_enabled) { | |
collisionsEnabled = p_enabled; | |
if (collisionsEnabled) { | |
bt_collision_object->setCollisionFlags(bt_collision_object->getCollisionFlags() & (~btCollisionObject::CF_NO_CONTACT_RESPONSE)); | |
} else { | |
bt_collision_object->setCollisionFlags(bt_collision_object->getCollisionFlags() | btCollisionObject::CF_NO_CONTACT_RESPONSE); | |
} | |
} |
And finally this one could benefit from a change to make it safe:
godot/modules/bullet/collision_object_bullet.cpp
Lines 157 to 160 in 17e6638
bool CollisionObjectBullet::has_collision_exception(const CollisionObjectBullet *p_otherCollisionObject) const { | |
return !bt_collision_object->checkCollideWith(p_otherCollisionObject->bt_collision_object); | |
} | |
It could be replaced with:
bool CollisionObjectBullet::has_collision_exception(const CollisionObjectBullet *p_otherCollisionObject) const {
return exceptions.has(p_otherCollisionObject->get_self());
}
It should be the exact same result but this way it wouldn't rely on bt_collision_object
being valid.
One last piece of advice is, since Bullet is disabled on master at the moment (it's not compiled at all), make sure to test on the 3.2 branch locally (this PR can still be on master though).
Previously godot would try to access `CollisionObjectBullet::bt_collision_object` even if it was null. Fixes godotengine#46651
Thanks! |
Cherry-picked for 3.2.4. |
fixes #46651