From 99d05790072932a32a204ba25ea5d861d274c913 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 26 Sep 2019 10:40:47 -0700 Subject: [PATCH] remove updateEntitiesInternal(), make updateEntities() virtual --- libraries/entities/src/EntitySimulation.cpp | 12 +++++------ libraries/entities/src/EntitySimulation.h | 21 +++++++------------ .../entities/src/SimpleEntitySimulation.cpp | 8 +++++-- .../entities/src/SimpleEntitySimulation.h | 6 +++--- .../physics/src/PhysicalEntitySimulation.cpp | 9 ++++---- .../physics/src/PhysicalEntitySimulation.h | 4 ++-- 6 files changed, 27 insertions(+), 33 deletions(-) diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 8a38905567..976cae9bd9 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/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::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::max(); - - clearEntitiesInternal(); - - _allEntities.clear(); - _deadEntities.clear(); } void EntitySimulation::moveSimpleKinematics(uint64_t now) { diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 22d098168f..e9ad7ab2b7 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -46,8 +46,8 @@ const int DIRTY_SIMULATION_FLAGS = class EntitySimulation : public QObject, public std::enable_shared_from_this { public: - EntitySimulation() : _mutex(QMutex::Recursive), _entityTree(NULL), _nextExpiry(std::numeric_limits::max()) { } - virtual ~EntitySimulation() { setEntityTree(NULL); } + EntitySimulation() : _mutex(QMutex::Recursive), _nextExpiry(std::numeric_limits::max()), _entityTree(nullptr) { } + virtual ~EntitySimulation() { setEntityTree(nullptr); } inline EntitySimulationPointer getThisPointer() const { return std::const_pointer_cast(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 _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 diff --git a/libraries/entities/src/SimpleEntitySimulation.cpp b/libraries/entities/src/SimpleEntitySimulation.cpp index f5e7810858..d64efdf87f 100644 --- a/libraries/entities/src/SimpleEntitySimulation.cpp +++ b/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() { diff --git a/libraries/entities/src/SimpleEntitySimulation.h b/libraries/entities/src/SimpleEntitySimulation.h index 6194193f5a..e984b72ed4 100644 --- a/libraries/entities/src/SimpleEntitySimulation.h +++ b/libraries/entities/src/SimpleEntitySimulation.h @@ -23,16 +23,16 @@ using SimpleEntitySimulationPointer = std::shared_ptr; 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; diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index b3f4f28e29..6356a439d3 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/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 diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index 044eb47dcb..7efb000050 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/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();