remove updateEntitiesInternal(), make updateEntities() virtual

libraries/entities/src/EntitySimulation.cpp

@@ -21,6 +21,7 @@ void EntitySimulation::setEntityTree(EntityTreePointer tree) {
     if (_entityTree && _entityTree != tree) {
         _entitiesToSort.clear();
         _simpleKinematicEntities.clear();
+        _changedEntities.clear();
         _entitiesToUpdate.clear();
         _mortalEntities.clear();
         _nextExpiry = std::numeric_limits<uint64_t>::max();
@@ -29,15 +30,14 @@ void EntitySimulation::setEntityTree(EntityTreePointer tree) {
 }
 
 void EntitySimulation::updateEntities() {
+    PerformanceTimer perfTimer("EntitySimulation::updateEntities");
     QMutexLocker lock(&_mutex);
     uint64_t now = usecTimestampNow();
-    PerformanceTimer perfTimer("EntitySimulation::updateEntities");
 
     // these methods may accumulate entries in _entitiesToBeDeleted
     expireMortalEntities(now);
     callUpdateOnEntitiesThatNeedIt(now);
     moveSimpleKinematics(now);
-    updateEntitiesInternal(now);
     sortEntitiesThatMoved();
 }
 
@@ -220,14 +220,12 @@ void EntitySimulation::clearEntities() {
     QMutexLocker lock(&_mutex);
     _entitiesToSort.clear();
     _simpleKinematicEntities.clear();
+    _changedEntities.clear();
+    _allEntities.clear();
+    _deadEntities.clear();
     _entitiesToUpdate.clear();
     _mortalEntities.clear();
     _nextExpiry = std::numeric_limits<uint64_t>::max();
-
-    clearEntitiesInternal();
-
-    _allEntities.clear();
-    _deadEntities.clear();
 }
 
 void EntitySimulation::moveSimpleKinematics(uint64_t now) {

libraries/entities/src/EntitySimulation.h

@@ -46,8 +46,8 @@ const int DIRTY_SIMULATION_FLAGS =
 
 class EntitySimulation : public QObject, public std::enable_shared_from_this<EntitySimulation> {
 public:
-    EntitySimulation() : _mutex(QMutex::Recursive), _entityTree(NULL), _nextExpiry(std::numeric_limits<uint64_t>::max()) { }
-    virtual ~EntitySimulation() { setEntityTree(NULL); }
+    EntitySimulation() : _mutex(QMutex::Recursive), _nextExpiry(std::numeric_limits<uint64_t>::max()), _entityTree(nullptr) { }
+    virtual ~EntitySimulation() { setEntityTree(nullptr); }
 
     inline EntitySimulationPointer getThisPointer() const {
         return std::const_pointer_cast<EntitySimulation>(shared_from_this());
@@ -56,7 +56,7 @@ public:
     /// \param tree pointer to EntityTree which is stored internally
     void setEntityTree(EntityTreePointer tree);
 
-    void updateEntities();
+    virtual void updateEntities();
 
     // FIXME: remove these
     virtual void addDynamic(EntityDynamicPointer dynamic) {}
@@ -71,7 +71,7 @@ public:
     /// call this whenever an entity was changed from some EXTERNAL event (NOT by the EntitySimulation itself)
     void changeEntity(EntityItemPointer entity);
 
-    void clearEntities();
+    virtual void clearEntities();
 
     void moveSimpleKinematics(uint64_t now);
 
@@ -79,19 +79,14 @@ public:
 
     virtual void takeDeadEntities(SetOfEntities& entitiesToDelete);
 
-    /// \param entity pointer to EntityItem that needs to be put on the entitiesToDelete list and removed from others.
     virtual void prepareEntityForDelete(EntityItemPointer entity);
 
     void processChangedEntities();
 
 protected:
-    // These pure virtual methods are protected because they are not to be called will-nilly. The base class
-    // calls them in the right places.
-    virtual void updateEntitiesInternal(uint64_t now) = 0;
     virtual void addEntityToInternalLists(EntityItemPointer entity);
     virtual void removeEntityFromInternalLists(EntityItemPointer entity);
     virtual void processChangedEntity(const EntityItemPointer& entity);
-    virtual void clearEntitiesInternal() = 0;
 
     void expireMortalEntities(uint64_t now);
     void callUpdateOnEntitiesThatNeedIt(uint64_t now);
@@ -101,16 +96,11 @@ protected:
 
     SetOfEntities _entitiesToSort; // entities moved by simulation (and might need resort in EntityTree)
     SetOfEntities _simpleKinematicEntities; // entities undergoing non-colliding kinematic motion
-
-protected:
     SetOfEntities _deadEntities; // dead entities that might still be in the _entityTree
 
 private:
     void moveSimpleKinematics();
 
-    // back pointer to EntityTree structure
-    EntityTreePointer _entityTree;
-
     // We maintain multiple lists, each for its distinct purpose.
     // An entity may be in more than one list.
     std::unordered_set<EntityItemPointer> _changedEntities; // all changes this frame
@@ -118,6 +108,9 @@ private:
     SetOfEntities _entitiesToUpdate; // entities that need to call EntityItem::update()
     SetOfEntities _mortalEntities; // entities that have an expiry
     uint64_t _nextExpiry;
+
+    // back pointer to EntityTree structure
+    EntityTreePointer _entityTree;
 };
 
 #endif // hifi_EntitySimulation_h

libraries/entities/src/SimpleEntitySimulation.cpp

@@ -47,7 +47,10 @@ void SimpleEntitySimulation::clearOwnership(const QUuid& ownerID) {
     }
 }
 
-void SimpleEntitySimulation::updateEntitiesInternal(uint64_t now) {
+void SimpleEntitySimulation::updateEntities() {
+    EntitySimulation::updateEntities();
+    QMutexLocker lock(&_mutex);
+    uint64_t now = usecTimestampNow();
     expireStaleOwnerships(now);
     stopOwnerlessEntities(now);
 }
@@ -134,10 +137,11 @@ void SimpleEntitySimulation::processChangedEntity(const EntityItemPointer& entit
     entity->clearDirtyFlags();
 }
 
-void SimpleEntitySimulation::clearEntitiesInternal() {
+void SimpleEntitySimulation::clearEntities() {
     QMutexLocker lock(&_mutex);
     _entitiesWithSimulationOwner.clear();
     _entitiesThatNeedSimulationOwner.clear();
+    EntitySimulation::clearEntities();
 }
 
 void SimpleEntitySimulation::sortEntitiesThatMoved() {

libraries/entities/src/SimpleEntitySimulation.h

@@ -23,16 +23,16 @@ using SimpleEntitySimulationPointer = std::shared_ptr<SimpleEntitySimulation>;
 class SimpleEntitySimulation : public EntitySimulation {
 public:
     SimpleEntitySimulation() : EntitySimulation() { }
-    ~SimpleEntitySimulation() { clearEntitiesInternal(); }
+    ~SimpleEntitySimulation() { clearEntities(); }
 
     void clearOwnership(const QUuid& ownerID);
+    void clearEntities() override;
+    void updateEntities() override;
 
 protected:
-    void updateEntitiesInternal(uint64_t now) override;
     void addEntityToInternalLists(EntityItemPointer entity) override;
     void removeEntityFromInternalLists(EntityItemPointer entity) override;
     void processChangedEntity(const EntityItemPointer& entity) override;
-    void clearEntitiesInternal() override;
 
     void sortEntitiesThatMoved() override;
 

libraries/physics/src/PhysicalEntitySimulation.cpp

@@ -40,10 +40,6 @@ void PhysicalEntitySimulation::init(
 }
 
 // begin EntitySimulation overrides
-void PhysicalEntitySimulation::updateEntitiesInternal(uint64_t now) {
-    // Do nothing here because the "internal" update the PhysicsEngine::stepSimulation() which is done elsewhere.
-}
-
 void PhysicalEntitySimulation::addEntityToInternalLists(EntityItemPointer entity) {
     EntitySimulation::addEntityToInternalLists(entity);
     entity->deserializeActions(); // TODO: do this elsewhere
@@ -186,11 +182,12 @@ void PhysicalEntitySimulation::processChangedEntity(const EntityItemPointer& ent
     }
 }
 
-void PhysicalEntitySimulation::clearEntitiesInternal() {
+void PhysicalEntitySimulation::clearEntities() {
     // TODO: we should probably wait to lock the _physicsEngine so we don't mess up data structures
     // while it is in the middle of a simulation step.  As it is, we're probably in shutdown mode
     // anyway, so maybe the simulation was already properly shutdown?  Cross our fingers...
 
+    QMutexLocker lock(&_mutex);
     // remove the objects (aka MotionStates) from physics
     _physicsEngine->removeSetOfObjects(_physicalObjects);
 
@@ -212,6 +209,8 @@ void PhysicalEntitySimulation::clearEntitiesInternal() {
     _entitiesToAddToPhysics.clear();
     _incomingChanges.clear();
     _entitiesToDeleteLater.clear();
+
+    EntitySimulation::clearEntities();
 }
 
 // virtual

libraries/physics/src/PhysicalEntitySimulation.h

@@ -66,16 +66,16 @@ public:
     virtual void takeDeadEntities(SetOfEntities& deadEntities) override;
     void takeDeadAvatarEntities(SetOfEntities& deadEntities);
 
+    virtual void clearEntities() override;
+
 signals:
     void entityCollisionWithEntity(const EntityItemID& idA, const EntityItemID& idB, const Collision& collision);
 
 protected: // only called by EntitySimulation
     // overrides for EntitySimulation
-    virtual void updateEntitiesInternal(uint64_t now) override;
     virtual void addEntityToInternalLists(EntityItemPointer entity) override;
     virtual void removeEntityFromInternalLists(EntityItemPointer entity) override;
     void processChangedEntity(const EntityItemPointer& entity) override;
-    virtual void clearEntitiesInternal() override;
 
     void removeOwnershipData(EntityMotionState* motionState);
     void clearOwnershipData();