From 9557ba80c95873b4441c10d8c232c13c5192b8e1 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Wed, 25 Sep 2019 16:31:21 -0700
Subject: [PATCH] all delete pass through
 EntitySimulation::prepareEntityForDelete()

---
 libraries/entities/src/EntitySimulation.cpp      | 16 ++++++++--------
 libraries/entities/src/EntitySimulation.h        |  3 +--
 .../physics/src/PhysicalEntitySimulation.cpp     | 12 ++++++------
 3 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp
index b23bb4df30..8a38905567 100644
--- a/libraries/entities/src/EntitySimulation.cpp
+++ b/libraries/entities/src/EntitySimulation.cpp
@@ -19,11 +19,11 @@
 
 void EntitySimulation::setEntityTree(EntityTreePointer tree) {
     if (_entityTree && _entityTree != tree) {
-        _mortalEntities.clear();
-        _nextExpiry = std::numeric_limits<uint64_t>::max();
-        _entitiesToUpdate.clear();
         _entitiesToSort.clear();
         _simpleKinematicEntities.clear();
+        _entitiesToUpdate.clear();
+        _mortalEntities.clear();
+        _nextExpiry = std::numeric_limits<uint64_t>::max();
     }
     _entityTree = tree;
 }
@@ -49,11 +49,11 @@ void EntitySimulation::takeDeadEntities(SetOfEntities& entitiesToDelete) {
 
 void EntitySimulation::removeEntityFromInternalLists(EntityItemPointer entity) {
     // remove from all internal lists except _deadEntities
-    _mortalEntities.remove(entity);
-    _entitiesToUpdate.remove(entity);
     _entitiesToSort.remove(entity);
     _simpleKinematicEntities.remove(entity);
     _allEntities.remove(entity);
+    _entitiesToUpdate.remove(entity);
+    _mortalEntities.remove(entity);
     entity->setSimulated(false);
 }
 
@@ -218,11 +218,11 @@ void EntitySimulation::processChangedEntity(const EntityItemPointer& entity) {
 
 void EntitySimulation::clearEntities() {
     QMutexLocker lock(&_mutex);
-    _mortalEntities.clear();
-    _nextExpiry = std::numeric_limits<uint64_t>::max();
-    _entitiesToUpdate.clear();
     _entitiesToSort.clear();
     _simpleKinematicEntities.clear();
+    _entitiesToUpdate.clear();
+    _mortalEntities.clear();
+    _nextExpiry = std::numeric_limits<uint64_t>::max();
 
     clearEntitiesInternal();
 
diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h
index f80d78412a..22d098168f 100644
--- a/libraries/entities/src/EntitySimulation.h
+++ b/libraries/entities/src/EntitySimulation.h
@@ -115,10 +115,9 @@ private:
     // An entity may be in more than one list.
     std::unordered_set<EntityItemPointer> _changedEntities; // all changes this frame
     SetOfEntities _allEntities; // tracks all entities added the simulation
+    SetOfEntities _entitiesToUpdate; // entities that need to call EntityItem::update()
     SetOfEntities _mortalEntities; // entities that have an expiry
     uint64_t _nextExpiry;
-
-    SetOfEntities _entitiesToUpdate; // entities that need to call EntityItem::update()
 };
 
 #endif // hifi_EntitySimulation_h
diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp
index 7335f2b1ee..b3f4f28e29 100644
--- a/libraries/physics/src/PhysicalEntitySimulation.cpp
+++ b/libraries/physics/src/PhysicalEntitySimulation.cpp
@@ -71,7 +71,8 @@ void PhysicalEntitySimulation::removeEntityFromInternalLists(EntityItemPointer e
     if (motionState) {
         removeOwnershipData(motionState);
         _entitiesToRemoveFromPhysics.insert(entity);
-    } else if (entity->isDead() && entity->getElement()) {
+    }
+    if (entity->isDead() && entity->getElement()) {
         _deadEntities.insert(entity);
     }
     if (entity->isAvatarEntity()) {
@@ -215,7 +216,8 @@ void PhysicalEntitySimulation::clearEntitiesInternal() {
 
 // virtual
 void PhysicalEntitySimulation::prepareEntityForDelete(EntityItemPointer entity) {
-    // this can be called on any thread
+    // DANGER! this can be called on any thread
+    // do no dirty deeds here --> assemble list for later
     assert(entity);
     assert(entity->isDead());
     QMutexLocker lock(&_mutex);
@@ -223,13 +225,11 @@ void PhysicalEntitySimulation::prepareEntityForDelete(EntityItemPointer entity)
 }
 
 void PhysicalEntitySimulation::removeDeadEntities() {
-    // only ever call this on the main thread
+    // DANGER! only ever call this on the main thread
     QMutexLocker lock(&_mutex);
     for (auto& entity : _entitiesToDeleteLater) {
         entity->clearActions(getThisPointer());
-        if (entity->isSimulated()) {
-            removeEntityFromInternalLists(entity);
-        }
+        EntitySimulation::prepareEntityForDelete(entity);
     }
     _entitiesToDeleteLater.clear();
 }