cleanup API for removal from EntitySimulation

libraries/entities/src/EntitySimulation.cpp

@@ -47,6 +47,24 @@ void EntitySimulation::getEntitiesToDelete(VectorOfEntities& entitiesToDelete) {
     _entitiesToDelete.clear();
 }
 
+void EntitySimulation::removeEntityInternal(EntityItemPointer entity) {
+    _mortalEntities.remove(entity);
+    _entitiesToUpdate.remove(entity);
+    _entitiesToSort.remove(entity);
+    _simpleKinematicEntities.remove(entity);
+    _allEntities.remove(entity);
+    entity->_simulated = false;
+}
+
+void EntitySimulation::prepareEntityForDelete(EntityItemPointer entity) {
+    assert(entity);
+    if (entity->_simulated) {
+        entity->clearActions(this);
+        _entitiesToDelete.insert(entity);
+        removeEntityInternal(entity);
+    }
+}
+
 void EntitySimulation::addEntityInternal(EntityItemPointer entity) {
     if (entity->isMoving() && !entity->getPhysicsInfo()) {
         _simpleKinematicEntities.insert(entity);
@@ -71,15 +89,8 @@ void EntitySimulation::expireMortalEntities(const quint64& now) {
             EntityItemPointer entity = *itemItr;
             quint64 expiry = entity->getExpiry();
             if (expiry < now) {
-                _entitiesToDelete.insert(entity);
                 itemItr = _mortalEntities.erase(itemItr);
-                _entitiesToUpdate.remove(entity);
+                prepareEntityForDelete(entity);
-                _entitiesToSort.remove(entity);
-                _simpleKinematicEntities.remove(entity);
-                removeEntityInternal(entity);
-
-                _allEntities.remove(entity);
-                entity->_simulated = false;
             } else {
                 if (expiry < _nextExpiry) {
                     // remeber the smallest _nextExpiry so we know when to start the next search
@@ -97,7 +108,7 @@ void EntitySimulation::callUpdateOnEntitiesThatNeedIt(const quint64& now) {
     SetOfEntities::iterator itemItr = _entitiesToUpdate.begin();
     while (itemItr != _entitiesToUpdate.end()) {
         EntityItemPointer entity = *itemItr;
-        // TODO: catch transition from needing update to not as a "change" 
+        // TODO: catch transition from needing update to not as a "change"
         // so we don't have to scan for it here.
         if (!entity->needsToCallUpdate()) {
             itemItr = _entitiesToUpdate.erase(itemItr);
@@ -123,16 +134,8 @@ void EntitySimulation::sortEntitiesThatMoved() {
         AACube newCube = entity->getQueryAACube(success);
         if (success && !domainBounds.touches(newCube)) {
             qCDebug(entities) << "Entity " << entity->getEntityItemID() << " moved out of domain bounds.";
-            _entitiesToDelete.insert(entity);
-            _mortalEntities.remove(entity);
-            _entitiesToUpdate.remove(entity);
-            _simpleKinematicEntities.remove(entity);
-            removeEntityInternal(entity);
-
-            _allEntities.remove(entity);
-            entity->_simulated = false;
-
             itemItr = _entitiesToSort.erase(itemItr);
+            prepareEntityForDelete(entity);
         } else {
             moveOperator.addEntityToMoveList(entity, newCube);
             ++itemItr;
@@ -165,37 +168,23 @@ void EntitySimulation::addEntity(EntityItemPointer entity) {
     _allEntities.insert(entity);
     entity->_simulated = true;
 
-    // DirtyFlags are used to signal changes to entities that have already been added, 
+    // DirtyFlags are used to signal changes to entities that have already been added,
     // so we can clear them for this entity which has just been added.
     entity->clearDirtyFlags();
 }
 
-void EntitySimulation::removeEntity(EntityItemPointer entity) {
-    QMutexLocker lock(&_mutex);
-    assert(entity);
-    _entitiesToUpdate.remove(entity);
-    _mortalEntities.remove(entity);
-    _entitiesToSort.remove(entity);
-    _simpleKinematicEntities.remove(entity);
-    _entitiesToDelete.remove(entity);
-    removeEntityInternal(entity);
-
-    _allEntities.remove(entity);
-    entity->_simulated = false;
-}
-
 void EntitySimulation::changeEntity(EntityItemPointer entity) {
     QMutexLocker lock(&_mutex);
     assert(entity);
     if (!entity->_simulated) {
         // This entity was either never added to the simulation or has been removed
-        // (probably for pending delete), so we don't want to keep a pointer to it 
+        // (probably for pending delete), so we don't want to keep a pointer to it
         // on any internal lists.
         return;
     }
 
     // Although it is not the responsibility of the EntitySimulation to sort the tree for EXTERNAL changes
-    // it IS responsibile for triggering deletes for entities that leave the bounds of the domain, hence 
+    // it IS responsibile for triggering deletes for entities that leave the bounds of the domain, hence
     // we must check for that case here, however we rely on the change event to have set DIRTY_POSITION flag.
     bool wasRemoved = false;
     uint32_t dirtyFlags = entity->getDirtyFlags();
@@ -205,13 +194,7 @@ void EntitySimulation::changeEntity(EntityItemPointer entity) {
         AACube newCube = entity->getQueryAACube(success);
         if (success && !domainBounds.touches(newCube)) {
             qCDebug(entities) << "Entity " << entity->getEntityItemID() << " moved out of domain bounds.";
-            _entitiesToDelete.insert(entity);
+            prepareEntityForDelete(entity);
-            _mortalEntities.remove(entity);
-            _entitiesToUpdate.remove(entity);
-            _entitiesToSort.remove(entity);
-            _simpleKinematicEntities.remove(entity);
-            removeEntityInternal(entity);
-            entity->_simulated = false;
             wasRemoved = true;
         }
     }

libraries/entities/src/EntitySimulation.h

@@ -63,26 +63,21 @@ public:
     /// \sideeffect sets relevant backpointers in entity, but maybe later when appropriate data structures are locked
     void addEntity(EntityItemPointer entity);
 
-    /// \param entity pointer to EntityItem to be removed
+    /// \param entity pointer to EntityItem that may have changed in a way that would affect its simulation
-    /// \brief the actual removal may happen later when appropriate data structures are locked
-    /// \sideeffect nulls relevant backpointers in entity
-    void removeEntity(EntityItemPointer entity);
-
-    /// \param entity pointer to EntityItem to that may have changed in a way that would affect its simulation
     /// call this whenever an entity was changed from some EXTERNAL event (NOT by the EntitySimulation itself)
     void changeEntity(EntityItemPointer entity);
 
     void clearEntities();
 
     void moveSimpleKinematics(const quint64& now);
-protected: // these only called by the EntityTree?
-
-public:
 
     EntityTreePointer getEntityTree() { return _entityTree; }
 
     void getEntitiesToDelete(VectorOfEntities& entitiesToDelete);
 
+    /// \param entity pointer to EntityItem that needs to be put on the entitiesToDelete list and removed from others.
+    virtual void prepareEntityForDelete(EntityItemPointer entity);
+
 signals:
     void entityCollisionWithEntity(const EntityItemID& idA, const EntityItemID& idB, const Collision& collision);
 

libraries/entities/src/EntityTree.cpp

@@ -453,8 +453,7 @@ void EntityTree::processRemovedEntities(const DeleteEntityOperator& theOperator)
         }
 
         if (_simulation) {
-            theEntity->clearActions(_simulation);
+            _simulation->prepareEntityForDelete(theEntity);
-            _simulation->removeEntity(theEntity);
         }
     }
 }

libraries/entities/src/SimpleEntitySimulation.cpp

@@ -53,6 +53,7 @@ void SimpleEntitySimulation::addEntityInternal(EntityItemPointer entity) {
 }
 
 void SimpleEntitySimulation::removeEntityInternal(EntityItemPointer entity) {
+    EntitySimulation::removeEntityInternal(entity);
     _entitiesWithSimulator.remove(entity);
 }
 

libraries/entities/src/SimpleEntitySimulation.h

@@ -22,11 +22,11 @@ public:
     virtual ~SimpleEntitySimulation() { clearEntitiesInternal(); }
 
 protected:
-    virtual void updateEntitiesInternal(const quint64& now);
+    virtual void updateEntitiesInternal(const quint64& now) override;
-    virtual void addEntityInternal(EntityItemPointer entity);
+    virtual void addEntityInternal(EntityItemPointer entity) override;
-    virtual void removeEntityInternal(EntityItemPointer entity);
+    virtual void removeEntityInternal(EntityItemPointer entity) override;
-    virtual void changeEntityInternal(EntityItemPointer entity);
+    virtual void changeEntityInternal(EntityItemPointer entity) override;
-    virtual void clearEntitiesInternal();
+    virtual void clearEntitiesInternal() override;
 
     SetOfEntities _entitiesWithSimulator;
 };

libraries/physics/src/PhysicalEntitySimulation.cpp

@@ -55,6 +55,7 @@ void PhysicalEntitySimulation::addEntityInternal(EntityItemPointer entity) {
 }
 
 void PhysicalEntitySimulation::removeEntityInternal(EntityItemPointer entity) {
+    EntitySimulation::removeEntityInternal(entity);
     EntityMotionState* motionState = static_cast<EntityMotionState*>(entity->getPhysicsInfo());
     if (motionState) {
         motionState->clearObjectBackPointer();