Page MenuHome

BGE : fix bugs with physics collision mask/group
ClosedPublic

Authored by Porteries Tristan (panzergame) on Apr 21 2015, 5:23 PM.

Details

Summary

Currently there are bugs with physics objects in inactive layers and character/softbody.
So i added a function in CcdPhysicsEnvironement to know if a physics controller is currently active, and for soft body i added the correct function in UpdateCcdPhysicsController to re-add a softbody in the dynamics world.

Diff Detail

Repository
rB Blender

Event Timeline

Porteries Tristan (panzergame) retitled this revision from to BGE : fix bugs with physics collision mask/group.
Porteries Tristan (panzergame) updated this object.

hi, only some minor notes and some questions (just for me, as i didnt deal with softbody codewise yet) but at all seems to look ok

source/gameengine/Converter/BL_BlenderDataConversion.cpp
1350 ↗(On Diff #4041)

is physics refresh not needed during conversion, probably not... hmm so this might be ok (using false as second parameter)
same for following lines

source/gameengine/Ketsji/KX_GameObject.h
532

using default parameters may let compilation fail on compilers supporting only C90 Standard, like msvc

538

again, same thing with default parameter (made some bad experience with it, during compilation) BUT this is C++... so this even might be valid...

source/gameengine/Physics/Bullet/CcdPhysicsController.cpp
126–127

why deactivate object instead of activating as in old behavior ?

source/gameengine/Physics/Bullet/CcdPhysicsController.h
743 ↗(On Diff #4041)

doesnt this get confused with the active blender layer ? as in, objects you want to add later are on inactive layers.. but seems this is just a check to suppress physics object refreshing, which is probably not necessary on inactive layers

source/gameengine/Physics/Bullet/CcdPhysicsEnvironment.cpp
586

why re-add softbody here ? what didnt work with it exactly (when left as is ?)

source/gameengine/Physics/Bullet/CcdPhysicsController.cpp
126–127

You talk about activate(true/false) ? if it is, the activate(true) make a static object active, so i use activate(false).

source/gameengine/Physics/Bullet/CcdPhysicsEnvironment.cpp
586

The softbody is removed line 577 but only add as collision object only in the previous version (line 590), So on the old version if we do a UpdateCcdPhysicsController the softbody was removed and only the BAAB stayed.

Porteries Tristan (panzergame) edited edge metadata.

Use an other way to check if a controller is active

Mitchell Stokes (moguri) added inline comments.
source/gameengine/Ketsji/KX_GameObject.cpp
569 ↗(On Diff #4045)

Shouldn't we always refresh the physics controller to be safe? Is there harm in refreshing it if we don't actually need to?

source/gameengine/Physics/Bullet/CcdPhysicsController.cpp
1121

Is it safe to use m_object->getCollisionFlags() instead of m_cci.m_collisionFlags? Is there a reason for this change? Is m_cci.m_collisionFlags something we should even bother keeping around?

source/gameengine/Physics/Bullet/CcdPhysicsEnvironment.cpp
570

You should use IsActiveCcdPhysicsController(ctrl) here instead of duplicating logic.

source/gameengine/Physics/Bullet/CcdPhysicsEnvironment.h
229

You should add some spacing here to line this up with the previous line.

source/gameengine/Ketsji/KX_GameObject.cpp
569 ↗(On Diff #4045)

It's useless to refresh the physics controller during the construction and it can cause problems.

source/gameengine/Physics/Bullet/CcdPhysicsController.cpp
1121

Yes it's safe only for character because the flags used for character are not in m_cci.m_collisionFlags. This is describe in the previous comment.

source/gameengine/Physics/Bullet/CcdPhysicsEnvironment.cpp
570

oops, i forgot to remove this line.

source/gameengine/Physics/Bullet/CcdPhysicsEnvironment.h
229

We have to keep the old typo ?

Remove useless check in Update*

source/gameengine/Ketsji/KX_GameObject.cpp
569 ↗(On Diff #4045)

What kind of problems?

source/gameengine/Physics/Bullet/CcdPhysicsController.cpp
1121

That raises the question, why does m_cci.m_collisionFlag not have the character flag?

source/gameengine/Physics/Bullet/CcdPhysicsEnvironment.h
229

It's not really a typo, just how someone chose to align things. It's better to stay consistent. If you want to fix the style, it's best to do it in another patch.

Remove the refresh argument

This patch is looking pretty good now. I'll let @Angus Hollands (agoose77) and @Sergej Reich (sergof) comment on the correctness of the physics/Bullet changes.

source/gameengine/Physics/Bullet/CcdPhysicsController.cpp
1120

I'd like to know why m_cci and m_object have different collision flags.

source/gameengine/Physics/Bullet/CcdPhysicsEnvironment.h
229

Unless it goes against the style guide, keep the style of the file consistent. In other words, just add the extra spacing to line this up.

source/gameengine/Physics/Bullet/CcdPhysicsController.cpp
1120

line 1207 :
object->setCollisionFlags(object->getCollisionFlags() | btCollisionObject::CF_KINEMATIC_OBJECT);

source/gameengine/Physics/Bullet/CcdPhysicsController.cpp
1120

Well, that doesn't explain why one would have the character flag and not the other. However, now I'm wondering why we are using m_cci.m_mass and not m_object->getMass()? Similar for the group and mask. Since m_object is the actual Bullet object that is being updated at runtime, but m_cci is mostly static information used to initially create m_object.

source/gameengine/Physics/Bullet/CcdPhysicsEnvironment.cpp
570

While we're fixing things, "collisionning" should be "collision" and "collistioning" should be "colliding"

If you can address my last three inline comments and get an okay from either @Angus Hollands (agoose77) or @Sergej Reich (sergof), then I think this patch is good to go.

source/gameengine/Physics/Bullet/CcdPhysicsController.cpp
1120

Is it the character flag or btCollisionObject::CF_KINEMATIC_OBJECT that we are looking for in m_object->getCollisionFlags(). I get that we want to get the flags off of the object since it has the current runtime flags, but the comment doesn't really seem to match this.

Provided no edge-cases have been discovered, I'm happy with this for now :)

Porteries Tristan (panzergame) edited edge metadata.

Use GetMass() instead of m_cc1.m_mass

This revision is now accepted and ready to land.Apr 23 2015, 11:30 PM
Porteries Tristan (panzergame) edited edge metadata.

Use handle->m_collisionFilterGroup instead of m_cci.m_collisionFilterGroup

This revision was automatically updated to reflect the committed changes.