From 2dab3069daee47774c308ed5015c2f0141fa52da Mon Sep 17 00:00:00 2001
From: Seth Alves <seth.alves@gmail.com>
Date: Fri, 14 Apr 2017 11:06:40 -0700
Subject: [PATCH] fix bug that was causing spring action to cause constant
 re-insertion of its rigidbody into bullet.  store UUIDs rather than shared
 pointers in _objectDynamicsByBody so that refcounting works right and actions
 get their destructors called.

---
 libraries/entities/src/EntityItem.cpp    |  1 -
 libraries/physics/src/ObjectAction.cpp   |  2 +-
 libraries/physics/src/ObjectAction.h     |  1 +
 libraries/physics/src/ObjectConstraint.h |  2 ++
 libraries/physics/src/PhysicsEngine.cpp  | 24 +++++++++++++++++-------
 libraries/physics/src/PhysicsEngine.h    |  2 +-
 6 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp
index 500e4c30fe..f5b2214273 100644
--- a/libraries/entities/src/EntityItem.cpp
+++ b/libraries/entities/src/EntityItem.cpp
@@ -2010,7 +2010,6 @@ bool EntityItem::updateAction(EntitySimulationPointer simulation, const QUuid& a
             action->setIsMine(true);
             serializeActions(success, _allActionsDataCache);
             _dirtyFlags |= Simulation::DIRTY_PHYSICS_ACTIVATION;
-            _dirtyFlags |= Simulation::DIRTY_COLLISION_GROUP; // may need to not collide with own avatar
         } else {
             qCDebug(entities) << "EntityItem::updateAction failed";
         }
diff --git a/libraries/physics/src/ObjectAction.cpp b/libraries/physics/src/ObjectAction.cpp
index f14070fde6..5f5f763ca6 100644
--- a/libraries/physics/src/ObjectAction.cpp
+++ b/libraries/physics/src/ObjectAction.cpp
@@ -32,7 +32,7 @@ void ObjectAction::updateAction(btCollisionWorld* collisionWorld, btScalar delta
     });
 
     if (!ownerEntity) {
-        qCDebug(physics) << "warning -- action with no entity removing self from btCollisionWorld.";
+        qCDebug(physics) << "warning -- action [" << _tag << "] with no entity removing self from btCollisionWorld.";
         btDynamicsWorld* dynamicsWorld = static_cast<btDynamicsWorld*>(collisionWorld);
         if (dynamicsWorld) {
             dynamicsWorld->removeAction(this);
diff --git a/libraries/physics/src/ObjectAction.h b/libraries/physics/src/ObjectAction.h
index 8e2f0e3383..fb141a4620 100644
--- a/libraries/physics/src/ObjectAction.h
+++ b/libraries/physics/src/ObjectAction.h
@@ -20,6 +20,7 @@
 class ObjectAction : public btActionInterface, public ObjectDynamic {
 public:
     ObjectAction(EntityDynamicType type, const QUuid& id, EntityItemPointer ownerEntity);
+    virtual ~ObjectAction() {}
 
     virtual bool isAction() const override { return true; }
 
diff --git a/libraries/physics/src/ObjectConstraint.h b/libraries/physics/src/ObjectConstraint.h
index 13aa871d98..711daea812 100644
--- a/libraries/physics/src/ObjectConstraint.h
+++ b/libraries/physics/src/ObjectConstraint.h
@@ -21,6 +21,8 @@ class ObjectConstraint : public ObjectDynamic
 {
 public:
     ObjectConstraint(EntityDynamicType type, const QUuid& id, EntityItemPointer ownerEntity);
+    virtual ~ObjectConstraint() {}
+
     virtual btTypedConstraint* getConstraint() = 0;
 
     virtual bool isConstraint() const override { return true; }
diff --git a/libraries/physics/src/PhysicsEngine.cpp b/libraries/physics/src/PhysicsEngine.cpp
index 432f4287ea..0c4290eff2 100644
--- a/libraries/physics/src/PhysicsEngine.cpp
+++ b/libraries/physics/src/PhysicsEngine.cpp
@@ -145,10 +145,18 @@ void PhysicsEngine::addObjectToDynamicsWorld(ObjectMotionState* motionState) {
 QList<EntityDynamicPointer> PhysicsEngine::removeDynamicsForBody(btRigidBody* body) {
     // remove dynamics that are attached to this body
     QList<EntityDynamicPointer> removedDynamics;
-    QMutableSetIterator<EntityDynamicPointer> i(_objectDynamicsByBody[body]);
+    QMutableSetIterator<QUuid> i(_objectDynamicsByBody[body]);
+
     while (i.hasNext()) {
-        EntityDynamicPointer dynamic = i.next();
-        removeDynamic(dynamic->getID());
+        QUuid dynamicID = i.next();
+        if (dynamicID.isNull()) {
+            continue;
+        }
+        EntityDynamicPointer dynamic = _objectDynamics[dynamicID];
+        if (!dynamic) {
+            continue;
+        }
+        removeDynamic(dynamicID);
         removedDynamics += dynamic;
     }
     return removedDynamics;
@@ -608,7 +616,7 @@ bool PhysicsEngine::addDynamic(EntityDynamicPointer dynamic) {
     if (success) {
         _objectDynamics[dynamicID] = dynamic;
         foreach(btRigidBody* rigidBody, std::static_pointer_cast<ObjectDynamic>(dynamic)->getRigidBodies()) {
-            _objectDynamicsByBody[rigidBody] += dynamic;
+            _objectDynamicsByBody[rigidBody] += dynamic->getID();
         }
     }
     return success;
@@ -632,16 +640,18 @@ void PhysicsEngine::removeDynamic(const QUuid dynamicID) {
         }
         _objectDynamics.remove(dynamicID);
         foreach(btRigidBody* rigidBody, rigidBodies) {
-            _objectDynamicsByBody[rigidBody].remove(dynamic);
+            _objectDynamicsByBody[rigidBody].remove(dynamic->getID());
         }
         dynamic->invalidate();
     }
 }
 
 void PhysicsEngine::forEachDynamic(std::function<void(EntityDynamicPointer)> actor) {
-    QHashIterator<QUuid, EntityDynamicPointer> iter(_objectDynamics);
+    QMutableHashIterator<QUuid, EntityDynamicPointer> iter(_objectDynamics);
     while (iter.hasNext()) {
         iter.next();
-        actor(iter.value());
+        if (iter.value()) {
+            actor(iter.value());
+        }
     }
 }
diff --git a/libraries/physics/src/PhysicsEngine.h b/libraries/physics/src/PhysicsEngine.h
index 33d60997a8..9f2f1aff5c 100644
--- a/libraries/physics/src/PhysicsEngine.h
+++ b/libraries/physics/src/PhysicsEngine.h
@@ -113,7 +113,7 @@ private:
     ContactMap _contactMap;
     CollisionEvents _collisionEvents;
     QHash<QUuid, EntityDynamicPointer> _objectDynamics;
-    QHash<btRigidBody*, QSet<EntityDynamicPointer>> _objectDynamicsByBody;
+    QHash<btRigidBody*, QSet<QUuid>> _objectDynamicsByBody;
     std::vector<btRigidBody*> _activeStaticBodies;
 
     glm::vec3 _originOffset;