Page MenuHome
Differential D1205

BGE: Add recursive dupli group deletion
ClosedPublic
Actions

Authored by Thomas Szepe (hg1) on Mar 28 2015, 9:32 PM.

Details

Reviewers
Sybren A. Stüvel (sybren)
Jorge Bernal (lordloki)
Mitchell Stokes (moguri)
Inês Almeida (brita_)
Maniphest Tasks
T36388: Delete recursive DupliGroup objects.
Commits
rBS2247ebe16ee8: BGE: Add recursive dupli group deletion
rB2247ebe16ee8: BGE: Add recursive dupli group deletion
Summary

This patch will delete all associated objects from a group instance (dupli group) if the are deleted.

Diff Detail

Event Timeline

Thomas Szepe (hg1) updated this revision to Diff 3844.Mar 28 2015, 9:32 PM
Thomas Szepe (hg1) retitled this revision from to Add recursive dupli group deletion..
Thomas Szepe (hg1) updated this object.
Thomas Szepe (hg1) updated this object.Mar 28 2015, 9:43 PM
Thomas Szepe (hg1) added reviewers: Sybren A. Stüvel (sybren), Jorge Bernal (lordloki).Mar 28 2015, 9:47 PM

This patch is useful if you want to delete a ragdoll.

Test files can be found in T36388.

Sergey Sharybin (sergey) added reviewers: Mitchell Stokes (moguri), Inês Almeida (brita_).Apr 3 2015, 11:27 AM
Jorge Bernal (lordloki) accepted this revision.Apr 5 2015, 12:59 PM
Jorge Bernal (lordloki) edited edge metadata.

Looks good to me.

I also tested and it worked ok.

This revision is now accepted and ready to land.Apr 5 2015, 12:59 PM
Sybren A. Stüvel (sybren) edited edge metadata.Apr 6 2015, 12:02 AM

I don't see how this patch relates to the blend file attached to T36388. Can you create a blend file that shows just this feature? We'll need it anyway for the release, since Ton asked for a demo blend file for each new feature.

Thomas Szepe (hg1) added a comment.Apr 6 2015, 8:10 AM

I don't see how this patch relates to the blend file attached to T36388.

The blend file in T36388 is related to this patch, and shows exactly the new feature. The complexity of the test file was necessary to test also adding and multiple dupli groups.

I reduced the test file to the minimum that is necessary to show the new feature.

Dupligroup_deletion.blend111 KBDownload

Sybren A. Stüvel (sybren) added a comment.Apr 20 2015, 2:50 PM

When I open the attached blend file and start the BGE, Blender crashes:

/home/sybren/workspace-local/blender-git/build_debug/bin/blender(BLI_system_backtrace+0x35) [0x2a9e161]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender() [0x19124b4]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender() [0x1912692]
/lib/x86_64-linux-gnu/libc.so.6(+0x36eb0) [0x7fc8576cdeb0]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZNK24CcdOverlapFilterCallBack23needBroadphaseCollisionEP17btBroadphaseProxyS1_+0x4d) [0x2ca8a49]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZNK28btHashedOverlappingPairCache24needsBroadphaseCollisionEP17btBroadphaseProxyS1_+0x48) [0x2e60f8c]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZN28btHashedOverlappingPairCache18addOverlappingPairEP17btBroadphaseProxyS1_+0x40) [0x2e6101c]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZN18btDbvtTreeCollider7ProcessEPK10btDbvtNodeS2_+0x6e) [0x2e5e1d8]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZN18btDbvtTreeCollider7ProcessEPK10btDbvtNode+0x37) [0x2e5e229]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZNK6btDbvt9collideTVEPK10btDbvtNodeRK12btDbvtAabbMmRNS_8ICollideE+0x16c) [0x2e5da74]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZN16btDbvtBroadphase11createProxyERK9btVector3S2_iPvssP12btDispatcherS3_+0x1ad) [0x2e5b1b9]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZN16btCollisionWorld18addCollisionObjectEP17btCollisionObjectss+0x1b1) [0x2e63b09]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZN23btDiscreteDynamicsWorld18addCollisionObjectEP17btCollisionObjectss+0x32) [0x2ecb934]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZN23btDiscreteDynamicsWorld12addRigidBodyEP11btRigidBodyss+0xe0) [0x2ecbbf2]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZN21CcdPhysicsEnvironment26UpdateCcdPhysicsControllerEP20CcdPhysicsControllerfiss+0x169) [0x2ca4799]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZN20CcdPhysicsController17RefreshCollisionsEv+0x104) [0x2cc8518]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZN13KX_GameObject21SetUserCollisionGroupEt+0x58) [0x2d2730a]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender() [0x2c51fcd]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_Z24BL_ConvertBlenderObjectsP4MainP8KX_SceneP15KX_KetsjiEngine15e_PhysicsEngineP15RAS_IRasterizerP11RAS_ICanvasP24KX_BlenderSceneConverterbb+0x1738) [0x2c55b41]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(_ZN24KX_BlenderSceneConverter12ConvertSceneEP8KX_SceneP15RAS_IRasterizerP11RAS_ICanvasb+0x13d) [0x2c7598f]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(StartKetsjiShell+0x1134) [0x2c4838e]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender() [0x1a696d0]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender() [0x191f096]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender() [0x1920642]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender() [0x1920f16]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender() [0x19212d1]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(wm_event_do_handlers+0x75d) [0x1922144]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(WM_main+0x24) [0x1915ed7]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender(main+0x49a) [0x1914fea]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5) [0x7fc8576b8ec5]
/home/sybren/workspace-local/blender-git/build_debug/bin/blender() [0x1911a2a]

That's with or without this patch, so probably has nothing to do with it, but it does make it hard to assert its correctness.

source/gameengine/Ketsji/KX_Scene.cpp
1006

A change to if (!newobj->IsDupliGroup()) return; simplifies the function, as it decreases the indentation level.

Sybren A. Stüvel (sybren) added a comment.Apr 20 2015, 3:38 PM

I've worked around the crash just to see how your patch works, and it looks good to me.

Thomas Szepe (hg1) mentioned this in D1222: BGE : addObject in python without reference object..Apr 27 2015, 12:25 PM
Mitchell Stokes (moguri) edited edge metadata.May 6 2015, 12:21 AM

I feel like this should be handled in KX_Scene::NewRemoveObject(), but then I don't think we'll properly handle physics. So, this approach is probably fine.

Address the inline comment (either make the member function private or explain why it should remain public), and make sure the patch has been tested. After that, the patch should be good to commit. What kind of test cases have you tried? I think the following would be good to check:

  • LibLoad/LibFree
  • Constraints
  • Memory leaks
source/gameengine/Ketsji/KX_Scene.h
341

This should be made private. I don't see much need for anything outside of KX_Scene calling this.

Thomas Szepe (hg1) added a comment.May 9 2015, 9:23 PM

I have tested:
normal objects (still working)
normal grouped objects (still working)
Dupligroup
Dupligroup in Dupligroup
Dupligroup with grouped objects
Dupligroup in Dupligroup with grouped objects
Dupligroup with Constraints
Dupligroup in Dupligroup with Constraints
Scene linked Dupligroup
Scene linked Dupligroup in Dupligroup
Scene linked Dupligroup with Constraints
Scene linked Dupligroup in Dupligroup with Constraints
File linked Dupligroup
File linked Dupligroup in Dupligroup
File linked Dupligroup with Constraints
File linked Dupligroup in Dupligroup with Constraints
LibLoad/LibFree Dupligroup
LibLoad/LibFree Dupligroup in Dupligroup
LibLoad/LibFree Dupligroup with Constraints (but due to a bug in LibLoad the constrains are not working)
LibLoad/LibFree file linked Dupligroup with Constraints (but due to a bug in LibLoad the constrains are not working)

I don't tested about Memory leaks. I don't know how to on Windows.

Jorge Bernal (lordloki) edited edge metadata.May 18 2015, 7:19 AM
Jorge Bernal (lordloki) added a project: Game Engine.
Porteries Tristan (panzergame) mentioned this in T36388: Delete recursive DupliGroup objects. .Jul 3 2015, 1:29 PM
Thomas Szepe (hg1) added a task: T36388: Delete recursive DupliGroup objects. .Jul 3 2015, 5:49 PM
Thomas Szepe (hg1) added a comment.Jul 5 2015, 2:38 PM

There is no new memory leaks with this patch on Windows. Tested with Visual Leak Detector.

Mitchell Stokes (moguri) accepted this revision.Jul 6 2015, 2:25 AM
Mitchell Stokes (moguri) edited edge metadata.
Thomas Szepe (hg1) updated this object.Oct 11 2015, 1:48 PM
Thomas Szepe (hg1) edited edge metadata.
Thomas Szepe (hg1) retitled this revision from Add recursive dupli group deletion. to BGE: Add recursive dupli group deletion.Oct 11 2015, 1:59 PM
Thomas Szepe (hg1) updated this revision to Diff 5149.Oct 11 2015, 2:03 PM

Update code style (asterisk).

Closed by commit rB2247ebe16ee8: BGE: Add recursive dupli group deletion (authored by Thomas Szepe (hg1)). · Explain WhyOct 11 2015, 2:10 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
source/
gameengine/
Ketsji/
1 line
19 lines

Diff 3844

View Options

source/gameengine/Ketsji/KX_Scene.h

Loading...
View Options

source/gameengine/Ketsji/KX_Scene.cpp

Loading...