From 6837d042ad52fc2fee8ab1efa2f5446a856eaec0 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 7 Mar 2018 14:24:13 -0800 Subject: [PATCH 01/30] move entityCollisionWithEntity to PhysicalEntitySimulation --- interface/src/Application.cpp | 2 +- libraries/entities/src/EntitySimulation.h | 7 ------- libraries/physics/src/PhysicalEntitySimulation.h | 4 ++++ 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 6209b828fc..0ab8c64b5a 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -4679,7 +4679,7 @@ void Application::init() { auto entityScriptingInterface = DependencyManager::get(); // connect the _entityCollisionSystem to our EntityTreeRenderer since that's what handles running entity scripts - connect(_entitySimulation.get(), &EntitySimulation::entityCollisionWithEntity, + connect(_entitySimulation.get(), &PhysicalEntitySimulation::entityCollisionWithEntity, getEntities().data(), &EntityTreeRenderer::entityCollisionWithEntity); // connect the _entities (EntityTreeRenderer) to our script engine's EntityScriptingInterface for firing diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 1c633aa9dc..8cfeb34af8 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -43,7 +43,6 @@ const int DIRTY_SIMULATION_FLAGS = Simulation::DIRTY_SIMULATOR_ID; class EntitySimulation : public QObject, public std::enable_shared_from_this { -Q_OBJECT public: EntitySimulation() : _mutex(QMutex::Recursive), _entityTree(NULL), _nextExpiry(quint64(-1)) { } virtual ~EntitySimulation() { setEntityTree(NULL); } @@ -57,8 +56,6 @@ public: void updateEntities(); -// friend class EntityTree; - virtual void addDynamic(EntityDynamicPointer dynamic); virtual void removeDynamic(const QUuid dynamicID); virtual void removeDynamics(QList dynamicIDsToRemove); @@ -83,9 +80,6 @@ public: /// \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); - 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. @@ -124,7 +118,6 @@ private: SetOfEntities _entitiesToUpdate; // entities that need to call EntityItem::update() - }; #endif // hifi_EntitySimulation_h diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index b9acf4cace..24d97cf509 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -28,6 +28,7 @@ using PhysicalEntitySimulationPointer = std::shared_ptr; class PhysicalEntitySimulation : public EntitySimulation { + Q_OBJECT public: PhysicalEntitySimulation(); ~PhysicalEntitySimulation(); @@ -39,6 +40,9 @@ public: virtual void takeEntitiesToDelete(VectorOfEntities& entitiesToDelete) override; +signals: + void entityCollisionWithEntity(const EntityItemID& idA, const EntityItemID& idB, const Collision& collision); + protected: // only called by EntitySimulation // overrides for EntitySimulation virtual void updateEntitiesInternal(const quint64& now) override; From bd434e3f188f1703c5b5ac8b310b9361c468568f Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 8 Mar 2018 10:19:01 -0800 Subject: [PATCH 02/30] make EntitySimulation::addEntityInternal() pure virtual --- libraries/entities/src/EntitySimulation.cpp | 8 -------- libraries/entities/src/EntitySimulation.h | 2 +- libraries/entities/src/SimpleEntitySimulation.cpp | 6 +++++- 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 36b0d8ab2d..0cd727c6bc 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -71,14 +71,6 @@ void EntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { } } -void EntitySimulation::addEntityInternal(EntityItemPointer entity) { - if (entity->isMovingRelativeToParent() && !entity->getPhysicsInfo()) { - QMutexLocker lock(&_mutex); - _simpleKinematicEntities.insert(entity); - entity->setLastSimulated(usecTimestampNow()); - } -} - void EntitySimulation::changeEntityInternal(EntityItemPointer entity) { QMutexLocker lock(&_mutex); if (entity->isMovingRelativeToParent() && !entity->getPhysicsInfo()) { diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 8cfeb34af8..1621a47996 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -84,7 +84,7 @@ 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(const quint64& now) = 0; - virtual void addEntityInternal(EntityItemPointer entity); + virtual void addEntityInternal(EntityItemPointer entity) = 0; virtual void removeEntityInternal(EntityItemPointer entity) = 0; virtual void changeEntityInternal(EntityItemPointer entity); virtual void clearEntitiesInternal() = 0; diff --git a/libraries/entities/src/SimpleEntitySimulation.cpp b/libraries/entities/src/SimpleEntitySimulation.cpp index 3203c2968c..3dbeff2a87 100644 --- a/libraries/entities/src/SimpleEntitySimulation.cpp +++ b/libraries/entities/src/SimpleEntitySimulation.cpp @@ -85,7 +85,11 @@ void SimpleEntitySimulation::updateEntitiesInternal(const quint64& now) { } void SimpleEntitySimulation::addEntityInternal(EntityItemPointer entity) { - EntitySimulation::addEntityInternal(entity); + if (entity->isMovingRelativeToParent() && !entity->getPhysicsInfo()) { + QMutexLocker lock(&_mutex); + _simpleKinematicEntities.insert(entity); + entity->setLastSimulated(usecTimestampNow()); + } if (!entity->getSimulatorID().isNull()) { QMutexLocker lock(&_mutex); _entitiesWithSimulationOwner.insert(entity); From 5649748b177685a4e6ff7d0bf78809dc3066c3a4 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 8 Mar 2018 10:21:13 -0800 Subject: [PATCH 03/30] make EntitySimulation::changeEntityInternal() pure virtual --- libraries/entities/src/EntitySimulation.cpp | 13 ------------- libraries/entities/src/EntitySimulation.h | 2 +- libraries/entities/src/SimpleEntitySimulation.cpp | 13 ++++++++++++- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 0cd727c6bc..442802fa00 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -71,19 +71,6 @@ void EntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { } } -void EntitySimulation::changeEntityInternal(EntityItemPointer entity) { - QMutexLocker lock(&_mutex); - if (entity->isMovingRelativeToParent() && !entity->getPhysicsInfo()) { - int numKinematicEntities = _simpleKinematicEntities.size(); - _simpleKinematicEntities.insert(entity); - if (numKinematicEntities != _simpleKinematicEntities.size()) { - entity->setLastSimulated(usecTimestampNow()); - } - } else { - _simpleKinematicEntities.remove(entity); - } -} - // protected void EntitySimulation::expireMortalEntities(const quint64& now) { if (now > _nextExpiry) { diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 1621a47996..82eed98b09 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -86,7 +86,7 @@ protected: virtual void updateEntitiesInternal(const quint64& now) = 0; virtual void addEntityInternal(EntityItemPointer entity) = 0; virtual void removeEntityInternal(EntityItemPointer entity) = 0; - virtual void changeEntityInternal(EntityItemPointer entity); + virtual void changeEntityInternal(EntityItemPointer entity) = 0; virtual void clearEntitiesInternal() = 0; void expireMortalEntities(const quint64& now); diff --git a/libraries/entities/src/SimpleEntitySimulation.cpp b/libraries/entities/src/SimpleEntitySimulation.cpp index 3dbeff2a87..c7353fb53a 100644 --- a/libraries/entities/src/SimpleEntitySimulation.cpp +++ b/libraries/entities/src/SimpleEntitySimulation.cpp @@ -111,7 +111,18 @@ void SimpleEntitySimulation::removeEntityInternal(EntityItemPointer entity) { } void SimpleEntitySimulation::changeEntityInternal(EntityItemPointer entity) { - EntitySimulation::changeEntityInternal(entity); + { + QMutexLocker lock(&_mutex); + if (entity->isMovingRelativeToParent() && !entity->getPhysicsInfo()) { + int numKinematicEntities = _simpleKinematicEntities.size(); + _simpleKinematicEntities.insert(entity); + if (numKinematicEntities != _simpleKinematicEntities.size()) { + entity->setLastSimulated(usecTimestampNow()); + } + } else { + _simpleKinematicEntities.remove(entity); + } + } if (entity->getSimulatorID().isNull()) { QMutexLocker lock(&_mutex); _entitiesWithSimulationOwner.remove(entity); From 0596497930cc407c9caaa894006a10ec9192ef55 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 8 Mar 2018 11:57:40 -0800 Subject: [PATCH 04/30] more correct comments --- libraries/physics/src/PhysicalEntitySimulation.cpp | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index e4ba47e205..8ca514fea5 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -80,8 +80,6 @@ void PhysicalEntitySimulation::takeEntitiesToDelete(VectorOfEntities& entitiesTo // this entity is still in its tree, so we insert into the external list entitiesToDelete.push_back(entity); - // Someday when we invert the entities/physics lib dependencies we can let EntityItem delete its own PhysicsInfo - // rather than do it here EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (motionState) { _entitiesToRemoveFromPhysics.insert(entity); @@ -141,6 +139,8 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (motionState) { entity->setPhysicsInfo(nullptr); + // someday when we invert the entities/physics lib dependencies we can let EntityItem delete its own PhysicsInfo + // until then we must do it here delete motionState; } } @@ -192,6 +192,8 @@ void PhysicalEntitySimulation::deleteObjectsRemovedFromPhysics() { EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); assert(motionState); entity->setPhysicsInfo(nullptr); + // someday when we invert the entities/physics lib dependencies we can let EntityItem delete its own PhysicsInfo + // until then we must do it here delete motionState; if (entity->isDead()) { From f20c9d3c5ba57ee3a015cb96daddc9c4ad375afc Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 8 Mar 2018 11:59:08 -0800 Subject: [PATCH 05/30] EntitySimulation::removeEntityInternal isn't pure virtual --- libraries/entities/src/EntitySimulation.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 82eed98b09..bdd205c11a 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -85,7 +85,7 @@ protected: // calls them in the right places. virtual void updateEntitiesInternal(const quint64& now) = 0; virtual void addEntityInternal(EntityItemPointer entity) = 0; - virtual void removeEntityInternal(EntityItemPointer entity) = 0; + virtual void removeEntityInternal(EntityItemPointer entity); virtual void changeEntityInternal(EntityItemPointer entity) = 0; virtual void clearEntitiesInternal() = 0; From 738e1267abc5e472706e1b9b757ed0a099dadd18 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 8 Mar 2018 17:15:09 -0800 Subject: [PATCH 06/30] small optimization in EntitySimulation::changeEntity() --- libraries/entities/src/EntitySimulation.cpp | 34 ++++++++++----------- 1 file changed, 16 insertions(+), 18 deletions(-) diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 442802fa00..24e139da61 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -186,7 +186,6 @@ void EntitySimulation::changeEntity(EntityItemPointer entity) { // 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 // 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(); if (dirtyFlags & Simulation::DIRTY_POSITION) { AACube domainBounds(glm::vec3((float)-HALF_TREE_SCALE), (float)TREE_SCALE); @@ -196,29 +195,28 @@ void EntitySimulation::changeEntity(EntityItemPointer entity) { qCDebug(entities) << "Entity " << entity->getEntityItemID() << " moved out of domain bounds."; entity->die(); prepareEntityForDelete(entity); - wasRemoved = true; + return; } } - if (!wasRemoved) { - if (dirtyFlags & Simulation::DIRTY_LIFETIME) { - if (entity->isMortal()) { - _mortalEntities.insert(entity); - quint64 expiry = entity->getExpiry(); - if (expiry < _nextExpiry) { - _nextExpiry = expiry; - } - } else { - _mortalEntities.remove(entity); + + if (dirtyFlags & Simulation::DIRTY_LIFETIME) { + if (entity->isMortal()) { + _mortalEntities.insert(entity); + quint64 expiry = entity->getExpiry(); + if (expiry < _nextExpiry) { + _nextExpiry = expiry; } - entity->clearDirtyFlags(Simulation::DIRTY_LIFETIME); - } - if (entity->needsToCallUpdate()) { - _entitiesToUpdate.insert(entity); } else { - _entitiesToUpdate.remove(entity); + _mortalEntities.remove(entity); } - changeEntityInternal(entity); + entity->clearDirtyFlags(Simulation::DIRTY_LIFETIME); } + if (entity->needsToCallUpdate()) { + _entitiesToUpdate.insert(entity); + } else { + _entitiesToUpdate.remove(entity); + } + changeEntityInternal(entity); } void EntitySimulation::clearEntities() { From a05c753e03911635b976d20da885c0f62f28a100 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 9 Mar 2018 15:26:33 -0800 Subject: [PATCH 07/30] only process changed entities when simulated --- libraries/entities/src/EntityTree.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 75f024d0b9..d728384468 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -453,12 +453,13 @@ bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperti uint32_t newFlags = entity->getDirtyFlags() & ~preFlags; if (newFlags) { - if (_simulation) { + if (entity->isSimulated()) { + assert((bool)_simulation); if (newFlags & DIRTY_SIMULATION_FLAGS) { _simulation->changeEntity(entity); } } else { - // normally the _simulation clears ALL updateFlags, but since there is none we do it explicitly + // normally the _simulation clears ALL dirtyFlags, but when not possible we do it explicitly entity->clearDirtyFlags(); } } @@ -469,7 +470,7 @@ bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperti if (entityScriptBefore != entityScriptAfter || reload) { emitEntityScriptChanging(entity->getEntityItemID(), reload); // the entity script has changed } - } + } // TODO: this final containingElement check should eventually be removed (or wrapped in an #ifdef DEBUG). if (!entity->getElement()) { From 5854a36ff775b86fec6c081231f535d13e2df202 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 12 Mar 2018 09:30:55 -0700 Subject: [PATCH 08/30] more correct handling of simulated entity --- libraries/entities/src/EntityTree.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index d728384468..1171b79bbc 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -260,7 +260,7 @@ void EntityTree::postAddEntity(EntityItemPointer entity) { return; } } - + // check to see if we need to simulate this entity.. if (_simulation) { _simulation->addEntity(entity); @@ -693,7 +693,7 @@ void EntityTree::processRemovedEntities(const DeleteEntityOperator& theOperator) trackDeletedEntity(theEntity->getEntityItemID()); } - if (_simulation) { + if (theEntity->isSimulated()) { _simulation->prepareEntityForDelete(theEntity); } } @@ -1689,7 +1689,7 @@ void EntityTree::releaseSceneEncodeData(OctreeElementExtraEncodeData* extraEncod } void EntityTree::entityChanged(EntityItemPointer entity) { - if (_simulation) { + if (entity->isSimulated()) { _simulation->changeEntity(entity); } } From e1d2a5e5f3477020956d77a663562a8a1574b70e Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 12 Mar 2018 15:26:11 -0700 Subject: [PATCH 09/30] more cleanup for deleting entities --- libraries/entities/src/EntityItem.cpp | 7 --- libraries/entities/src/EntitySimulation.cpp | 19 +++--- libraries/entities/src/EntitySimulation.h | 4 +- libraries/entities/src/EntityTree.cpp | 11 ++-- libraries/entities/src/MaterialEntityItem.cpp | 7 ++- .../entities/src/SimpleEntitySimulation.cpp | 1 - .../physics/src/PhysicalEntitySimulation.cpp | 62 ++++++++----------- .../physics/src/PhysicalEntitySimulation.h | 4 +- 8 files changed, 47 insertions(+), 68 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 5d056e17d8..901d7cf424 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -2962,13 +2962,6 @@ void EntityItem::retrieveMarketplacePublicKey() { } void EntityItem::preDelete() { - // clear out any left-over actions - EntityTreeElementPointer element = _element; // use local copy of _element for logic below - EntityTreePointer entityTree = element ? element->getTree() : nullptr; - EntitySimulationPointer simulation = entityTree ? entityTree->getSimulation() : nullptr; - if (simulation) { - clearActions(simulation); - } } void EntityItem::addMaterial(graphics::MaterialLayer material, const std::string& parentMaterialName) { diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 24e139da61..0edb7492d5 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -40,18 +40,17 @@ void EntitySimulation::updateEntities() { sortEntitiesThatMoved(); } -void EntitySimulation::takeEntitiesToDelete(VectorOfEntities& entitiesToDelete) { +void EntitySimulation::takeDeadEntities(VectorOfEntities& entitiesToDelete) { QMutexLocker lock(&_mutex); - for (auto entity : _entitiesToDelete) { + for (auto entity : _deadEntities) { // push this entity onto the external list entitiesToDelete.push_back(entity); } - _entitiesToDelete.clear(); + _deadEntities.clear(); } void EntitySimulation::removeEntityInternal(EntityItemPointer entity) { - QMutexLocker lock(&_mutex); - // remove from all internal lists except _entitiesToDelete + // remove from all internal lists except _deadEntities _mortalEntities.remove(entity); _entitiesToUpdate.remove(entity); _entitiesToSort.remove(entity); @@ -67,7 +66,9 @@ void EntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { QMutexLocker lock(&_mutex); entity->clearActions(getThisPointer()); removeEntityInternal(entity); - _entitiesToDelete.insert(entity); + if (entity->getElement()) { + _deadEntities.insert(entity); + } } } @@ -229,12 +230,8 @@ void EntitySimulation::clearEntities() { clearEntitiesInternal(); - for (auto entity : _allEntities) { - entity->setSimulated(false); - entity->die(); - } _allEntities.clear(); - _entitiesToDelete.clear(); + _deadEntities.clear(); } void EntitySimulation::moveSimpleKinematics(const quint64& now) { diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index bdd205c11a..92a3e3207b 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -75,7 +75,7 @@ public: EntityTreePointer getEntityTree() { return _entityTree; } - virtual void takeEntitiesToDelete(VectorOfEntities& entitiesToDelete); + virtual void takeDeadEntities(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); @@ -102,7 +102,7 @@ protected: QMutex _dynamicsMutex { QMutex::Recursive }; protected: - SetOfEntities _entitiesToDelete; // entities simulation decided needed to be deleted (EntityTree will actually delete) + SetOfEntities _deadEntities; private: void moveSimpleKinematics(); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 1171b79bbc..7a3ed91d51 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -94,7 +94,6 @@ OctreeElementPointer EntityTree::createNewElement(unsigned char* octalCode) { void EntityTree::eraseAllOctreeElements(bool createNewRoot) { emit clearingEntities(); - // this would be a good place to clean up our entities... if (_simulation) { _simulation->clearEntities(); } @@ -552,8 +551,6 @@ void EntityTree::setSimulation(EntitySimulationPointer simulation) { assert(simulation->getEntityTree().get() == this); } if (_simulation && _simulation != simulation) { - // It's important to clearEntities() on the simulation since taht will update each - // EntityItem::_simulationState correctly so as to not confuse the next _simulation. _simulation->clearEntities(); } _simulation = simulation; @@ -1803,13 +1800,13 @@ void EntityTree::update(bool simulate) { _simulation->updateEntities(); { PROFILE_RANGE(simulation_physics, "Deletes"); - VectorOfEntities pendingDeletes; - _simulation->takeEntitiesToDelete(pendingDeletes); - if (pendingDeletes.size() > 0) { + VectorOfEntities deadEntities; + _simulation->takeDeadEntities(deadEntities); + if (deadEntities.size() > 0) { // translate into list of ID's QSet idsToDelete; - for (auto entity : pendingDeletes) { + for (auto entity : deadEntities) { idsToDelete.insert(entity->getEntityItemID()); } diff --git a/libraries/entities/src/MaterialEntityItem.cpp b/libraries/entities/src/MaterialEntityItem.cpp index 137d6ef396..8b595bf69d 100644 --- a/libraries/entities/src/MaterialEntityItem.cpp +++ b/libraries/entities/src/MaterialEntityItem.cpp @@ -267,8 +267,11 @@ void MaterialEntityItem::setOwningAvatarID(const QUuid& owningAvatarID) { void MaterialEntityItem::removeMaterial() { graphics::MaterialPointer material = getMaterial(); + if (!material) { + return; + } QUuid parentID = getClientOnly() ? getOwningAvatarID() : getParentID(); - if (!material || parentID.isNull()) { + if (parentID.isNull()) { return; } @@ -336,4 +339,4 @@ void MaterialEntityItem::update(const quint64& now) { } EntityItem::update(now); -} \ No newline at end of file +} diff --git a/libraries/entities/src/SimpleEntitySimulation.cpp b/libraries/entities/src/SimpleEntitySimulation.cpp index c7353fb53a..583a310b41 100644 --- a/libraries/entities/src/SimpleEntitySimulation.cpp +++ b/libraries/entities/src/SimpleEntitySimulation.cpp @@ -105,7 +105,6 @@ void SimpleEntitySimulation::addEntityInternal(EntityItemPointer entity) { void SimpleEntitySimulation::removeEntityInternal(EntityItemPointer entity) { EntitySimulation::removeEntityInternal(entity); - QMutexLocker lock(&_mutex); _entitiesWithSimulationOwner.remove(entity); _entitiesThatNeedSimulationOwner.remove(entity); } diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 8ca514fea5..fb7123b7b6 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -61,31 +61,30 @@ void PhysicalEntitySimulation::addEntityInternal(EntityItemPointer entity) { void PhysicalEntitySimulation::removeEntityInternal(EntityItemPointer entity) { if (entity->isSimulated()) { EntitySimulation::removeEntityInternal(entity); - QMutexLocker lock(&_mutex); _entitiesToAddToPhysics.remove(entity); EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (motionState) { _outgoingChanges.remove(motionState); _entitiesToRemoveFromPhysics.insert(entity); - } else { - _entitiesToDelete.insert(entity); + } else if (entity->isDead() && entity->getElement()) { + _deadEntities.insert(entity); } } } -void PhysicalEntitySimulation::takeEntitiesToDelete(VectorOfEntities& entitiesToDelete) { +void PhysicalEntitySimulation::takeDeadEntities(VectorOfEntities& deadEntities) { QMutexLocker lock(&_mutex); - for (auto entity : _entitiesToDelete) { + for (auto entity : _deadEntities) { // this entity is still in its tree, so we insert into the external list - entitiesToDelete.push_back(entity); + deadEntities.push_back(entity); EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (motionState) { _entitiesToRemoveFromPhysics.insert(entity); } } - _entitiesToDelete.clear(); + _deadEntities.clear(); } void PhysicalEntitySimulation::changeEntityInternal(EntityItemPointer entity) { @@ -123,32 +122,24 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { // 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... - // copy everything into _entitiesToDelete - for (auto stateItr : _physicalObjects) { - EntityMotionState* motionState = static_cast(&(*stateItr)); - _entitiesToDelete.insert(motionState->getEntity()); - } - - // then remove the objects (aka MotionStates) from physics + // remove the objects (aka MotionStates) from physics _physicsEngine->removeSetOfObjects(_physicalObjects); // delete the MotionStates - // TODO: after we invert the entities/physics lib dependencies we will let EntityItem delete - // its own PhysicsInfo rather than do it here - for (auto entity : _entitiesToDelete) { - EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); - if (motionState) { - entity->setPhysicsInfo(nullptr); - // someday when we invert the entities/physics lib dependencies we can let EntityItem delete its own PhysicsInfo - // until then we must do it here - delete motionState; - } + for (auto stateItr : _physicalObjects) { + EntityMotionState* motionState = static_cast(&(*stateItr)); + assert(motionState); + EntityItemPointer entity = motionState->getEntity(); + entity->setPhysicsInfo(nullptr); + // TODO: someday when we invert the entities/physics lib dependencies we can let EntityItem delete its own PhysicsInfo + // until then we must do it here + delete motionState; } - - // finally clear all lists maintained by this class _physicalObjects.clear(); + + // clear all other lists specific to this derived class _entitiesToRemoveFromPhysics.clear(); - _entitiesToRelease.clear(); + _entitiesToForget.clear(); _entitiesToAddToPhysics.clear(); _pendingChanges.clear(); _outgoingChanges.clear(); @@ -158,6 +149,7 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { void PhysicalEntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { assert(entity); assert(entity->isDead()); + QMutexLocker lock(&_mutex); entity->clearActions(getThisPointer()); removeEntityInternal(entity); } @@ -176,11 +168,9 @@ void PhysicalEntitySimulation::getObjectsToRemoveFromPhysics(VectorOfMotionState _outgoingChanges.remove(motionState); _physicalObjects.remove(motionState); result.push_back(motionState); - _entitiesToRelease.insert(entity); - } - - if (entity->isDead()) { - _entitiesToDelete.insert(entity); + _entitiesToForget.insert(entity); + } else if (entity->isDead() && entity->getElement()) { + _deadEntities.insert(entity); } } _entitiesToRemoveFromPhysics.clear(); @@ -188,7 +178,7 @@ void PhysicalEntitySimulation::getObjectsToRemoveFromPhysics(VectorOfMotionState void PhysicalEntitySimulation::deleteObjectsRemovedFromPhysics() { QMutexLocker lock(&_mutex); - for (auto entity: _entitiesToRelease) { + for (auto entity: _entitiesToForget) { EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); assert(motionState); entity->setPhysicsInfo(nullptr); @@ -196,11 +186,11 @@ void PhysicalEntitySimulation::deleteObjectsRemovedFromPhysics() { // until then we must do it here delete motionState; - if (entity->isDead()) { - _entitiesToDelete.insert(entity); + if (entity->isDead() && entity->getElement()) { + _deadEntities.insert(entity); } } - _entitiesToRelease.clear(); + _entitiesToForget.clear(); } void PhysicalEntitySimulation::getObjectsToAddToPhysics(VectorOfMotionStates& result) { diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index 24d97cf509..0823198e2d 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -38,7 +38,7 @@ public: virtual void addDynamic(EntityDynamicPointer dynamic) override; virtual void applyDynamicChanges() override; - virtual void takeEntitiesToDelete(VectorOfEntities& entitiesToDelete) override; + virtual void takeDeadEntities(VectorOfEntities& deadEntities) override; signals: void entityCollisionWithEntity(const EntityItemID& idA, const EntityItemID& idB, const Collision& collision); @@ -68,7 +68,7 @@ public: private: SetOfEntities _entitiesToRemoveFromPhysics; - SetOfEntities _entitiesToRelease; + SetOfEntities _entitiesToForget; SetOfEntities _entitiesToAddToPhysics; SetOfEntityMotionStates _pendingChanges; // EntityMotionStates already in PhysicsEngine that need their physics changed From 6746e08206b2a10e009da1edf19224da1204d3a5 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 13 Mar 2018 11:38:50 -0700 Subject: [PATCH 10/30] faster EntitySimulation::takeDeadEntities() --- libraries/entities/src/EntitySimulation.cpp | 7 ++----- libraries/entities/src/EntitySimulation.h | 2 +- libraries/entities/src/EntityTree.cpp | 2 +- libraries/physics/src/PhysicalEntitySimulation.cpp | 6 ++---- libraries/physics/src/PhysicalEntitySimulation.h | 2 +- 5 files changed, 7 insertions(+), 12 deletions(-) diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 0edb7492d5..47e3e51fb9 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -40,12 +40,9 @@ void EntitySimulation::updateEntities() { sortEntitiesThatMoved(); } -void EntitySimulation::takeDeadEntities(VectorOfEntities& entitiesToDelete) { +void EntitySimulation::takeDeadEntities(SetOfEntities& entitiesToDelete) { QMutexLocker lock(&_mutex); - for (auto entity : _deadEntities) { - // push this entity onto the external list - entitiesToDelete.push_back(entity); - } + entitiesToDelete.swap(_deadEntities); _deadEntities.clear(); } diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 92a3e3207b..b5edb31907 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -75,7 +75,7 @@ public: EntityTreePointer getEntityTree() { return _entityTree; } - virtual void takeDeadEntities(VectorOfEntities& entitiesToDelete); + 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); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 7a3ed91d51..232880880e 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -1800,7 +1800,7 @@ void EntityTree::update(bool simulate) { _simulation->updateEntities(); { PROFILE_RANGE(simulation_physics, "Deletes"); - VectorOfEntities deadEntities; + SetOfEntities deadEntities; _simulation->takeDeadEntities(deadEntities); if (deadEntities.size() > 0) { // translate into list of ID's diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index fb7123b7b6..7161cff1da 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -73,17 +73,15 @@ void PhysicalEntitySimulation::removeEntityInternal(EntityItemPointer entity) { } } -void PhysicalEntitySimulation::takeDeadEntities(VectorOfEntities& deadEntities) { +void PhysicalEntitySimulation::takeDeadEntities(SetOfEntities& deadEntities) { QMutexLocker lock(&_mutex); for (auto entity : _deadEntities) { - // this entity is still in its tree, so we insert into the external list - deadEntities.push_back(entity); - EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (motionState) { _entitiesToRemoveFromPhysics.insert(entity); } } + _deadEntities.swap(deadEntities); _deadEntities.clear(); } diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index 0823198e2d..4425a73709 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -38,7 +38,7 @@ public: virtual void addDynamic(EntityDynamicPointer dynamic) override; virtual void applyDynamicChanges() override; - virtual void takeDeadEntities(VectorOfEntities& deadEntities) override; + virtual void takeDeadEntities(SetOfEntities& deadEntities) override; signals: void entityCollisionWithEntity(const EntityItemID& idA, const EntityItemID& idB, const Collision& collision); From 73fa6d3d2fb9f438a3b0ed42c11b68e7f4f510ba Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 13 Mar 2018 15:52:11 -0700 Subject: [PATCH 11/30] slightly simpler EntityMotionState delete logic --- interface/src/Application.cpp | 12 +++--- libraries/physics/src/EntityMotionState.cpp | 10 +++-- libraries/physics/src/EntityMotionState.h | 6 +-- .../physics/src/PhysicalEntitySimulation.cpp | 38 ++++++++----------- .../physics/src/PhysicalEntitySimulation.h | 8 ++-- 5 files changed, 36 insertions(+), 38 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 0ab8c64b5a..8c0dd12ae5 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5274,11 +5274,13 @@ void Application::update(float deltaTime) { { PROFILE_RANGE(simulation_physics, "PreStep"); PerformanceTimer perfTimer("preStep)"); - static VectorOfMotionStates motionStates; - _entitySimulation->getObjectsToRemoveFromPhysics(motionStates); - _physicsEngine->removeObjects(motionStates); - _entitySimulation->deleteObjectsRemovedFromPhysics(); + { + const VectorOfMotionStates& motionStates = _entitySimulation->getObjectsToRemoveFromPhysics(); + _physicsEngine->removeObjects(motionStates); + _entitySimulation->deleteObjectsRemovedFromPhysics(); + } + VectorOfMotionStates motionStates; getEntities()->getTree()->withReadLock([&] { _entitySimulation->getObjectsToAddToPhysics(motionStates); _physicsEngine->addObjects(motionStates); @@ -5292,7 +5294,7 @@ void Application::update(float deltaTime) { _entitySimulation->applyDynamicChanges(); - avatarManager->getObjectsToRemoveFromPhysics(motionStates); + avatarManager->getObjectsToRemoveFromPhysics(motionStates); _physicsEngine->removeObjects(motionStates); avatarManager->getObjectsToAddToPhysics(motionStates); _physicsEngine->addObjects(motionStates); diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 420da5a1e0..b512c2af98 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -49,8 +49,7 @@ bool entityTreeIsLocked() { EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer entity) : ObjectMotionState(nullptr), - _entityPtr(entity), - _entity(entity.get()), + _entity(entity), _serverPosition(0.0f), _serverRotation(), _serverVelocity(0.0f), @@ -80,8 +79,11 @@ EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer } EntityMotionState::~EntityMotionState() { - assert(_entity); - _entity = nullptr; + if (_entity) { + assert(_entity->getPhysicsInfo() == this); + _entity->setPhysicsInfo(nullptr); + _entity.reset(); + } } void EntityMotionState::updateServerPhysicsVariables() { diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index 784273d600..279772dd7e 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -70,7 +70,7 @@ public: virtual QUuid getSimulatorID() const override; virtual void bump(uint8_t priority) override; - EntityItemPointer getEntity() const { return _entityPtr.lock(); } + const EntityItemPointer& getEntity() const { return _entity; } void resetMeasuredBodyAcceleration(); void measureBodyAcceleration(); @@ -104,9 +104,7 @@ protected: // recursive dependency). Instead we keep a EntityItemWeakPointer to break that dependency while // still granting us the capability to generate EntityItemPointers as necessary (for external data // structures that use the MotionState to get to the EntityItem). - EntityItemWeakPointer _entityPtr; - // Meanwhile we also keep a raw EntityItem* for internal stuff where the pointer is guaranteed valid. - EntityItem* _entity; + EntityItemPointer _entity; bool _serverVariablesSet { false }; glm::vec3 _serverPosition; // in simulation-frame (not world-frame) diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 7161cff1da..40eb6c34e4 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -137,7 +137,6 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { // clear all other lists specific to this derived class _entitiesToRemoveFromPhysics.clear(); - _entitiesToForget.clear(); _entitiesToAddToPhysics.clear(); _pendingChanges.clear(); _outgoingChanges.clear(); @@ -153,42 +152,37 @@ void PhysicalEntitySimulation::prepareEntityForDelete(EntityItemPointer entity) } // end EntitySimulation overrides -void PhysicalEntitySimulation::getObjectsToRemoveFromPhysics(VectorOfMotionStates& result) { - result.clear(); +const VectorOfMotionStates& PhysicalEntitySimulation::getObjectsToRemoveFromPhysics() { QMutexLocker lock(&_mutex); for (auto entity: _entitiesToRemoveFromPhysics) { - // make sure it isn't on any side lists - _entitiesToAddToPhysics.remove(entity); - EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); - if (motionState) { - _pendingChanges.remove(motionState); - _outgoingChanges.remove(motionState); - _physicalObjects.remove(motionState); - result.push_back(motionState); - _entitiesToForget.insert(entity); - } else if (entity->isDead() && entity->getElement()) { + assert(motionState); + + _entitiesToAddToPhysics.remove(entity); + if (entity->isDead() && entity->getElement()) { _deadEntities.insert(entity); } + + _pendingChanges.remove(motionState); + _outgoingChanges.remove(motionState); + _physicalObjects.remove(motionState); + + // remember this motionState and delete it later (after removing its RigidBody from the PhysicsEngine) + _objectsToDelete.push_back(motionState); } _entitiesToRemoveFromPhysics.clear(); + return _objectsToDelete; } void PhysicalEntitySimulation::deleteObjectsRemovedFromPhysics() { QMutexLocker lock(&_mutex); - for (auto entity: _entitiesToForget) { - EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); - assert(motionState); - entity->setPhysicsInfo(nullptr); + for (auto motionState : _objectsToDelete) { // someday when we invert the entities/physics lib dependencies we can let EntityItem delete its own PhysicsInfo // until then we must do it here + // NOTE: a reference to the EntityItemPointer is released in the EntityMotionState::dtor delete motionState; - - if (entity->isDead() && entity->getElement()) { - _deadEntities.insert(entity); - } } - _entitiesToForget.clear(); + _objectsToDelete.clear(); } void PhysicalEntitySimulation::getObjectsToAddToPhysics(VectorOfMotionStates& result) { diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index 4425a73709..90e4a79a18 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -54,8 +54,9 @@ protected: // only called by EntitySimulation public: virtual void prepareEntityForDelete(EntityItemPointer entity) override; - void getObjectsToRemoveFromPhysics(VectorOfMotionStates& result); + const VectorOfMotionStates& getObjectsToRemoveFromPhysics(); void deleteObjectsRemovedFromPhysics(); + void getObjectsToAddToPhysics(VectorOfMotionStates& result); void setObjectsToChange(const VectorOfMotionStates& objectsToChange); void getObjectsToChange(VectorOfMotionStates& result); @@ -67,9 +68,10 @@ public: EntityEditPacketSender* getPacketSender() { return _entityPacketSender; } private: - SetOfEntities _entitiesToRemoveFromPhysics; - SetOfEntities _entitiesToForget; SetOfEntities _entitiesToAddToPhysics; + SetOfEntities _entitiesToRemoveFromPhysics; + + VectorOfMotionStates _objectsToDelete; SetOfEntityMotionStates _pendingChanges; // EntityMotionStates already in PhysicsEngine that need their physics changed SetOfEntityMotionStates _outgoingChanges; // EntityMotionStates for which we may need to send updates to entity-server From c61a226bb5f6386892f38ee853ac1447c716a772 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 14 Mar 2018 08:56:54 -0700 Subject: [PATCH 12/30] delete default EntityMotionState ctor --- libraries/physics/src/EntityMotionState.cpp | 6 ++++-- libraries/physics/src/EntityMotionState.h | 1 + 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index b512c2af98..6514e37cb7 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -67,6 +67,7 @@ EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer _accelerationNearlyGravityCount(0), _numInactiveUpdates(1) { + // EntityMotionState keeps a SharedPointer to its EntityItem which is only set in the CTOR _type = MOTIONSTATE_TYPE_ENTITY; assert(_entity); assert(entityTreeIsLocked()); @@ -80,6 +81,7 @@ EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer EntityMotionState::~EntityMotionState() { if (_entity) { + // EntityMotionState keeps a SharedPointer to its EntityItem which is only cleared in the DTOR assert(_entity->getPhysicsInfo() == this); _entity->setPhysicsInfo(nullptr); _entity.reset(); @@ -127,7 +129,7 @@ void EntityMotionState::handleEasyChanges(uint32_t& flags) { if (flags & Simulation::DIRTY_SIMULATOR_ID) { if (_entity->getSimulatorID().isNull()) { - // simulation ownership has been removed by an external simulator + // simulation ownership has been removed if (glm::length2(_entity->getWorldVelocity()) == 0.0f) { // this object is coming to rest --> clear the ACTIVATION flag and _outgoingPriority flags &= ~Simulation::DIRTY_PHYSICS_ACTIVATION; @@ -630,7 +632,7 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_ _entity->rememberHasSimulationOwnershipBid(); // ...then reset _outgoingPriority _outgoingPriority = 0; - // _outgoingPrioriuty will be re-computed before next bid, + // _outgoingPriority will be re-computed before next bid, // or will be set to agree with ownership priority should we win the bid } else if (_outgoingPriority != _entity->getSimulationPriority()) { // we own the simulation but our desired priority has changed diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index 279772dd7e..43529871a8 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -25,6 +25,7 @@ class EntityMotionState : public ObjectMotionState { public: + EntityMotionState() = delete; EntityMotionState(btCollisionShape* shape, EntityItemPointer item); virtual ~EntityMotionState(); From 2be0f0fa22613ff7041acbd274e1b8e21ef0b9ea Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 14 Mar 2018 09:14:30 -0700 Subject: [PATCH 13/30] better comments --- libraries/physics/src/EntityMotionState.cpp | 2 -- libraries/physics/src/EntityMotionState.h | 8 ++------ 2 files changed, 2 insertions(+), 8 deletions(-) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 6514e37cb7..2364dbdf81 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -67,7 +67,6 @@ EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer _accelerationNearlyGravityCount(0), _numInactiveUpdates(1) { - // EntityMotionState keeps a SharedPointer to its EntityItem which is only set in the CTOR _type = MOTIONSTATE_TYPE_ENTITY; assert(_entity); assert(entityTreeIsLocked()); @@ -81,7 +80,6 @@ EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer EntityMotionState::~EntityMotionState() { if (_entity) { - // EntityMotionState keeps a SharedPointer to its EntityItem which is only cleared in the DTOR assert(_entity->getPhysicsInfo() == this); _entity->setPhysicsInfo(nullptr); _entity.reset(); diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index 43529871a8..9722fe0a7a 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -99,12 +99,8 @@ protected: void setShape(const btCollisionShape* shape) override; void setMotionType(PhysicsMotionType motionType) override; - // In the glorious future (when entities lib depends on physics lib) the EntityMotionState will be - // properly "owned" by the EntityItem and will be deleted by it in the dtor. In pursuit of that - // state of affairs we can't keep a real EntityItemPointer as data member (it would produce a - // recursive dependency). Instead we keep a EntityItemWeakPointer to break that dependency while - // still granting us the capability to generate EntityItemPointers as necessary (for external data - // structures that use the MotionState to get to the EntityItem). + // EntityMotionState keeps a SharedPointer to its EntityItem which is only set in the CTOR + // and is only cleared in the DTOR EntityItemPointer _entity; bool _serverVariablesSet { false }; From 4501033861a44b2371bb4f00f210a752d1c22d37 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 14 Mar 2018 14:39:46 -0700 Subject: [PATCH 14/30] add description of simulation ownership rules --- libraries/entities/src/SimulationOwner.h | 73 +++++++++++++++++++++++- 1 file changed, 70 insertions(+), 3 deletions(-) diff --git a/libraries/entities/src/SimulationOwner.h b/libraries/entities/src/SimulationOwner.h index 94ed1d9a08..c10bb00fb2 100644 --- a/libraries/entities/src/SimulationOwner.h +++ b/libraries/entities/src/SimulationOwner.h @@ -18,9 +18,76 @@ #include #include -// Simulation observers will bid to simulate unowned active objects at the lowest possible priority -// which is VOLUNTEER. If the server accepts a VOLUNTEER bid it will automatically bump it -// to RECRUIT priority so that other volunteers don't accidentally take over. +// HighFidelity uses a distributed physics simulation where multiple "participants" simulate portions +// of the same world. When portions overlap only one participant is allowed to be the authority for any +// particular object. For a simulated entity the authoritative participant is called the simulation "owner" and +// their duty is to send transform/velocity updates for the entity to the central entity-server. +// The entity-server relays updates to other participants who apply them as "state synchronization" +// to their own simulation. +// +// Participants acquire ownership by sending a "bid" to the entity-server. The bid is a properties update: +// { +// "simulationOwner": { "ownerID" : sessionID, "priority" : priority }, +// transform/velocity properties +// } +// +// The entity-server is the authority as to who owns what and may reject a bid. +// The rules for handling a bid are as follows: +// +// (1) A bid may be refused for special ownership restrictions, but otherwise... +// +// (2) A bid at higher priority is accepted +// +// (3) A bid at equal priority is rejected if receieved within a grace-period (200msec) +// of the last ownership transition, otherwise it is accepted +// +// (4) The current owner is the only participant allowed to clear ownership (entity-server can override). +// +// (5) The current owner is the only participant allowed to adjust priority (entity-server can override). +// +// (6) If an owner does not update the transform or velocities of an owned entity within some period +// (5 seconds) then ownership is cleared and the entity's velocities are zeroed. This to handle +// the case when an owner drops off the network. +// +// The priority of a participant's bid depends on how "interested" it is in the entity's motion. The rules +// for bidding are as follows: +// +// (7) A participant (almost) never assumes that a bid is accepted by the entity-server. It packs the +// simulation owner and priority as if they really did change but doesn't actually modify them +// locally. Thus, if the bid packet is lost the participant will re-send after some period. +// The participant only updates its knowledge of who owns what when it recieves an update from the +// entity-server. An exception is when the participant creates a moving entity: it assumes it starts +// off owning any moving entities it creates. +// +// (8) When an unowned entity becomes active in the physics simulation the participant will +// start a timer and if the entity is still unowned after some period (0.5 seconds) +// it will bid at priority = VOLUNTEER (=2). The entity-server never grants ownership at VOLUNTEER +// priority: when a VOLUNTEER bid is accepted the entity-server always promotes the priority to +// RECRUIT (VOLUNTEER + 1); this to avoid a race-condition which might rapidly transition ownership +// when multiple participants (with variable ping-times to the server) bid simultaneously for a +// recently activated entity. +// +// (9) When a participant changes an entity's transform/velocity it will bid at priority = POKE (=127) +// +// (10) When an entity touches MyAvatar the participant it will bid at priority = POKE. +// +// (11) When a participant grabs an entity it will bid at priority = GRAB (=128). +// +// (12) When entityA, locally owned at priority = N, collides with an unowned entityB the owner will +// also bid for entityB at priority = N-1 (or VOLUNTEER, whichever is larger). +// +// (13) When an entity comes to rest and is deactivated in the physics simulation the owner will +// send an update to: clear their ownerhsip, set priority to zero, and set the object's +// velocities to be zero. As per a normal bid, the owner does NOT assume that its ownership +// has been cleared until it hears from the entity-server. This, if the packet is lost the +// owner will re-send after some period. +// +// (14) When an entity's ownership priority drops below VOLUNTEER other participants may bid for it +// immediately at priority = VOLUNTEER. +// +// (15) When an entity is still active but the owner no longer wants to own it, it will drop its priority +// to YIELD (=1, less than VOLUNTEER) thereby signalling to other participants to bid for it. +// const quint8 VOLUNTEER_SIMULATION_PRIORITY = 0x01; const quint8 RECRUIT_SIMULATION_PRIORITY = VOLUNTEER_SIMULATION_PRIORITY + 1; From d6ac294f39fb0b3c77221ea95f88ed808313ffea Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 14 Mar 2018 14:42:17 -0700 Subject: [PATCH 15/30] use uint8_t instead of quint8 --- libraries/entities/src/SimulationOwner.cpp | 20 +++++++------- libraries/entities/src/SimulationOwner.h | 32 +++++++++++----------- 2 files changed, 26 insertions(+), 26 deletions(-) diff --git a/libraries/entities/src/SimulationOwner.cpp b/libraries/entities/src/SimulationOwner.cpp index 4398582673..af69b7a166 100644 --- a/libraries/entities/src/SimulationOwner.cpp +++ b/libraries/entities/src/SimulationOwner.cpp @@ -16,9 +16,9 @@ #include -const quint8 PENDING_STATE_NOTHING = 0; -const quint8 PENDING_STATE_TAKE = 1; -const quint8 PENDING_STATE_RELEASE = 2; +const uint8_t PENDING_STATE_NOTHING = 0; +const uint8_t PENDING_STATE_TAKE = 1; +const uint8_t PENDING_STATE_RELEASE = 2; // static const int SimulationOwner::NUM_BYTES_ENCODED = NUM_BYTES_RFC4122_UUID + 1; @@ -33,7 +33,7 @@ SimulationOwner::SimulationOwner() : { } -SimulationOwner::SimulationOwner(const QUuid& id, quint8 priority) : +SimulationOwner::SimulationOwner(const QUuid& id, uint8_t priority) : _id(id), _expiry(0), _pendingBidTimestamp(0), @@ -67,11 +67,11 @@ void SimulationOwner::clear() { _pendingState = PENDING_STATE_NOTHING; } -void SimulationOwner::setPriority(quint8 priority) { +void SimulationOwner::setPriority(uint8_t priority) { _priority = priority; } -void SimulationOwner::promotePriority(quint8 priority) { +void SimulationOwner::promotePriority(uint8_t priority) { if (priority > _priority) { _priority = priority; } @@ -89,7 +89,7 @@ bool SimulationOwner::setID(const QUuid& id) { return false; } -bool SimulationOwner::set(const QUuid& id, quint8 priority) { +bool SimulationOwner::set(const QUuid& id, uint8_t priority) { uint8_t oldPriority = _priority; setPriority(priority); return setID(id) || oldPriority != _priority; @@ -101,7 +101,7 @@ bool SimulationOwner::set(const SimulationOwner& owner) { return setID(owner._id) || oldPriority != _priority; } -void SimulationOwner::setPendingPriority(quint8 priority, const quint64& timestamp) { +void SimulationOwner::setPendingPriority(uint8_t priority, const quint64& timestamp) { _pendingBidPriority = priority; _pendingBidTimestamp = timestamp; _pendingState = (_pendingBidPriority == 0) ? PENDING_STATE_RELEASE : PENDING_STATE_TAKE; @@ -142,7 +142,7 @@ void SimulationOwner::test() { { // test set constructor QUuid id = QUuid::createUuid(); - quint8 priority = 128; + uint8_t priority = 128; SimulationOwner simOwner(id, priority); if (simOwner.isNull()) { std::cout << __FILE__ << ":" << __LINE__ << " ERROR : SimulationOwner should NOT be NULL" << std::endl; @@ -164,7 +164,7 @@ void SimulationOwner::test() { { // test set() QUuid id = QUuid::createUuid(); - quint8 priority = 1; + uint8_t priority = 1; SimulationOwner simOwner; simOwner.set(id, priority); if (simOwner.isNull()) { diff --git a/libraries/entities/src/SimulationOwner.h b/libraries/entities/src/SimulationOwner.h index c10bb00fb2..5cfa0b2e1c 100644 --- a/libraries/entities/src/SimulationOwner.h +++ b/libraries/entities/src/SimulationOwner.h @@ -88,17 +88,17 @@ // (15) When an entity is still active but the owner no longer wants to own it, it will drop its priority // to YIELD (=1, less than VOLUNTEER) thereby signalling to other participants to bid for it. // -const quint8 VOLUNTEER_SIMULATION_PRIORITY = 0x01; -const quint8 RECRUIT_SIMULATION_PRIORITY = VOLUNTEER_SIMULATION_PRIORITY + 1; +const uint8_t VOLUNTEER_SIMULATION_PRIORITY = 0x01; +const uint8_t RECRUIT_SIMULATION_PRIORITY = VOLUNTEER_SIMULATION_PRIORITY + 1; // When poking objects with scripts an observer will bid at SCRIPT_EDIT priority. -const quint8 SCRIPT_GRAB_SIMULATION_PRIORITY = 0x80; -const quint8 SCRIPT_POKE_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY - 1; -const quint8 AVATAR_ENTITY_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY + 1; +const uint8_t SCRIPT_GRAB_SIMULATION_PRIORITY = 0x80; +const uint8_t SCRIPT_POKE_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY - 1; +const uint8_t AVATAR_ENTITY_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY + 1; // PERSONAL priority (needs a better name) is the level at which a simulation observer owns its own avatar // which really just means: things that collide with it will be bid at a priority level one lower -const quint8 PERSONAL_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY; +const uint8_t PERSONAL_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY; class SimulationOwner { @@ -106,25 +106,25 @@ public: static const int NUM_BYTES_ENCODED; SimulationOwner(); - SimulationOwner(const QUuid& id, quint8 priority); + SimulationOwner(const QUuid& id, uint8_t priority); const QUuid& getID() const { return _id; } const quint64& getExpiry() const { return _expiry; } - quint8 getPriority() const { return _priority; } + uint8_t getPriority() const { return _priority; } QByteArray toByteArray() const; bool fromByteArray(const QByteArray& data); void clear(); - void setPriority(quint8 priority); - void promotePriority(quint8 priority); + void setPriority(uint8_t priority); + void promotePriority(uint8_t priority); // return true if id is changed bool setID(const QUuid& id); - bool set(const QUuid& id, quint8 priority); + bool set(const QUuid& id, uint8_t priority); bool set(const SimulationOwner& owner); - void setPendingPriority(quint8 priority, const quint64& timestamp); + void setPendingPriority(uint8_t priority, const quint64& timestamp); bool isNull() const { return _id.isNull(); } bool matchesValidID(const QUuid& id) const { return _id == id && !_id.isNull(); } @@ -138,7 +138,7 @@ public: bool pendingTake(const quint64& timestamp); // return true if valid pending TAKE void clearCurrentOwner(); - bool operator>=(quint8 priority) const { return _priority >= priority; } + bool operator>=(uint8_t priority) const { return _priority >= priority; } bool operator==(const SimulationOwner& other) { return (_id == other._id && _priority == other._priority); } bool operator!=(const SimulationOwner& other); @@ -153,9 +153,9 @@ private: QUuid _id; // owner quint64 _expiry; // time when ownership can transition at equal priority quint64 _pendingBidTimestamp; // time when pending bid was set - quint8 _priority; // priority of current owner - quint8 _pendingBidPriority; // priority at which we'd like to own it - quint8 _pendingState; // NOTHING, TAKE, or RELEASE + uint8_t _priority; // priority of current owner + uint8_t _pendingBidPriority; // priority at which we'd like to own it + uint8_t _pendingState; // NOTHING, TAKE, or RELEASE }; From 4e0f307dc085378ad68a47f50e77732ce35d81f3 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 14 Mar 2018 16:29:20 -0700 Subject: [PATCH 16/30] quint8 --> uint8_t --- libraries/entities/src/EntityItem.cpp | 4 ++-- libraries/entities/src/EntityItem.h | 8 ++++---- libraries/entities/src/EntityItemProperties.h | 2 +- libraries/physics/src/EntityMotionState.cpp | 2 +- libraries/physics/src/ObjectMotionState.h | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 901d7cf424..aab8777862 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -1911,7 +1911,7 @@ void EntityItem::computeCollisionGroupAndFinalMask(int16_t& group, int16_t& mask } } -void EntityItem::setSimulationOwner(const QUuid& id, quint8 priority) { +void EntityItem::setSimulationOwner(const QUuid& id, uint8_t priority) { if (wantTerseEditLogging() && (id != _simulationOwner.getID() || priority != _simulationOwner.getPriority())) { qCDebug(entities) << "sim ownership for" << getDebugName() << "is now" << id << priority; } @@ -1942,7 +1942,7 @@ void EntityItem::clearSimulationOwnership() { } -void EntityItem::setPendingOwnershipPriority(quint8 priority, const quint64& timestamp) { +void EntityItem::setPendingOwnershipPriority(uint8_t priority, const quint64& timestamp) { _simulationOwner.setPendingPriority(priority, timestamp); } diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index 0303964e18..ebbeaaa254 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -304,14 +304,14 @@ public: // FIXME not thread safe? const SimulationOwner& getSimulationOwner() const { return _simulationOwner; } - void setSimulationOwner(const QUuid& id, quint8 priority); + void setSimulationOwner(const QUuid& id, uint8_t priority); void setSimulationOwner(const SimulationOwner& owner); - void promoteSimulationPriority(quint8 priority); + void promoteSimulationPriority(uint8_t priority); - quint8 getSimulationPriority() const { return _simulationOwner.getPriority(); } + uint8_t getSimulationPriority() const { return _simulationOwner.getPriority(); } QUuid getSimulatorID() const { return _simulationOwner.getID(); } void clearSimulationOwnership(); - void setPendingOwnershipPriority(quint8 priority, const quint64& timestamp); + void setPendingOwnershipPriority(uint8_t priority, const quint64& timestamp); uint8_t getPendingOwnershipPriority() const { return _simulationOwner.getPendingPriority(); } void rememberHasSimulationOwnershipBid() const; diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h index 9e5d6ddc79..0568a859ab 100644 --- a/libraries/entities/src/EntityItemProperties.h +++ b/libraries/entities/src/EntityItemProperties.h @@ -326,7 +326,7 @@ public: void clearSimulationOwner(); void setSimulationOwner(const QUuid& id, uint8_t priority); void setSimulationOwner(const QByteArray& data); - void promoteSimulationPriority(quint8 priority) { _simulationOwner.promotePriority(priority); } + void promoteSimulationPriority(uint8_t priority) { _simulationOwner.promotePriority(priority); } void setActionDataDirty() { _actionDataChanged = true; } diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 2364dbdf81..e0955fb2e1 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -621,7 +621,7 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_ _entity->setPendingOwnershipPriority(_outgoingPriority, now); } else if (!isLocallyOwned()) { // we don't own the simulation for this entity yet, but we're sending a bid for it - quint8 bidPriority = glm::max(_outgoingPriority, VOLUNTEER_SIMULATION_PRIORITY); + uint8_t bidPriority = glm::max(_outgoingPriority, VOLUNTEER_SIMULATION_PRIORITY); properties.setSimulationOwner(Physics::getSessionUUID(), bidPriority); _nextOwnershipBid = now + USECS_BETWEEN_OWNERSHIP_BIDS; // copy _outgoingPriority into pendingPriority... diff --git a/libraries/physics/src/ObjectMotionState.h b/libraries/physics/src/ObjectMotionState.h index 7f583ca9ca..8383c539f4 100644 --- a/libraries/physics/src/ObjectMotionState.h +++ b/libraries/physics/src/ObjectMotionState.h @@ -148,9 +148,9 @@ public: virtual const QUuid getObjectID() const = 0; - virtual quint8 getSimulationPriority() const { return 0; } + virtual uint8_t getSimulationPriority() const { return 0; } virtual QUuid getSimulatorID() const = 0; - virtual void bump(quint8 priority) {} + virtual void bump(uint8_t priority) {} virtual QString getName() const { return ""; } From dcf5110caad4e65665122cb34659c5168dd192ac Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 14 Mar 2018 16:31:00 -0700 Subject: [PATCH 17/30] clarify hardcoded constant --- libraries/entities/src/SimulationOwner.cpp | 2 +- libraries/entities/src/SimulationOwner.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/libraries/entities/src/SimulationOwner.cpp b/libraries/entities/src/SimulationOwner.cpp index af69b7a166..50cbeddef8 100644 --- a/libraries/entities/src/SimulationOwner.cpp +++ b/libraries/entities/src/SimulationOwner.cpp @@ -108,7 +108,7 @@ void SimulationOwner::setPendingPriority(uint8_t priority, const quint64& timest } void SimulationOwner::updateExpiry() { - const quint64 OWNERSHIP_LOCKOUT_EXPIRY = USECS_PER_SECOND / 5; + const quint64 OWNERSHIP_LOCKOUT_EXPIRY = 200 * USECS_PER_MSEC; _expiry = usecTimestampNow() + OWNERSHIP_LOCKOUT_EXPIRY; } diff --git a/libraries/entities/src/SimulationOwner.h b/libraries/entities/src/SimulationOwner.h index 5cfa0b2e1c..223232a4b6 100644 --- a/libraries/entities/src/SimulationOwner.h +++ b/libraries/entities/src/SimulationOwner.h @@ -88,11 +88,12 @@ // (15) When an entity is still active but the owner no longer wants to own it, it will drop its priority // to YIELD (=1, less than VOLUNTEER) thereby signalling to other participants to bid for it. // -const uint8_t VOLUNTEER_SIMULATION_PRIORITY = 0x01; +const uint8_t YIELD_SIMULATION_PRIORITY = 1; +const uint8_t VOLUNTEER_SIMULATION_PRIORITY = YIELD_SIMULATION_PRIORITY + 1; const uint8_t RECRUIT_SIMULATION_PRIORITY = VOLUNTEER_SIMULATION_PRIORITY + 1; // When poking objects with scripts an observer will bid at SCRIPT_EDIT priority. -const uint8_t SCRIPT_GRAB_SIMULATION_PRIORITY = 0x80; +const uint8_t SCRIPT_GRAB_SIMULATION_PRIORITY = 128; const uint8_t SCRIPT_POKE_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY - 1; const uint8_t AVATAR_ENTITY_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY + 1; From 49dce6fa1af6d7425e5316ba85d863dc89ca35cc Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 15 Mar 2018 15:21:43 -0700 Subject: [PATCH 18/30] remove some redundancy checks no need to assert(_entity) : guaranteed by ctor and dtor logic moved some checks around to avoid redundant work --- libraries/physics/src/EntityMotionState.cpp | 37 ++++++++------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index e0955fb2e1..0e901356f5 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -120,7 +120,6 @@ void EntityMotionState::handleDeactivation() { // virtual void EntityMotionState::handleEasyChanges(uint32_t& flags) { - assert(_entity); assert(entityTreeIsLocked()); updateServerPhysicsVariables(); ObjectMotionState::handleEasyChanges(flags); @@ -177,7 +176,6 @@ void EntityMotionState::handleEasyChanges(uint32_t& flags) { // virtual bool EntityMotionState::handleHardAndEasyChanges(uint32_t& flags, PhysicsEngine* engine) { - assert(_entity); updateServerPhysicsVariables(); return ObjectMotionState::handleHardAndEasyChanges(flags, engine); } @@ -255,7 +253,6 @@ void EntityMotionState::getWorldTransform(btTransform& worldTrans) const { // This callback is invoked by the physics simulation at the end of each simulation step... // iff the corresponding RigidBody is DYNAMIC and ACTIVE. void EntityMotionState::setWorldTransform(const btTransform& worldTrans) { - assert(_entity); assert(entityTreeIsLocked()); measureBodyAcceleration(); @@ -327,18 +324,25 @@ void EntityMotionState::setShape(const btCollisionShape* shape) { bool EntityMotionState::isCandidateForOwnership() const { assert(_body); - assert(_entity); assert(entityTreeIsLocked()); - return _outgoingPriority != 0 - || isLocallyOwned() - || _entity->dynamicDataNeedsTransmit(); + if (isLocallyOwned()) { + return true; + } else if (_entity->getClientOnly()) { + // don't send updates for someone else's avatarEntities + return false; + } + return _outgoingPriority != 0 || _entity->dynamicDataNeedsTransmit(); } bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { - DETAILED_PROFILE_RANGE(simulation_physics, "CheckOutOfSync"); // NOTE: we only get here if we think we own the simulation + DETAILED_PROFILE_RANGE(simulation_physics, "CheckOutOfSync"); assert(_body); + // Since we own the simulation: make sure _outgoingPriority is not less than current owned priority + // because: an _outgoingPriority of zero indicates that we should drop ownership when we have it. + upgradeOutgoingPriority(_entity->getSimulationPriority()); + bool parentTransformSuccess; Transform localToWorld = _entity->getParentTransform(parentTransformSuccess); Transform worldToLocal; @@ -417,10 +421,6 @@ bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { return true; } - if (_entity->shouldSuppressLocationEdits()) { - return false; - } - // Else we measure the error between current and extrapolated transform (according to expected behavior // of remote EntitySimulation) and return true if the error is significant. @@ -491,14 +491,11 @@ bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep) { DETAILED_PROFILE_RANGE(simulation_physics, "ShouldSend"); // NOTE: we expect _entity and _body to be valid in this context, since shouldSendUpdate() is only called // after doesNotNeedToSendUpdate() returns false and that call should return 'true' if _entity or _body are NULL. - assert(_entity); assert(_body); assert(entityTreeIsLocked()); - if (_entity->getClientOnly() && _entity->getOwningAvatarID() != Physics::getSessionUUID()) { - // don't send updates for someone else's avatarEntities - return false; - } + // this case prevented by isCandidateForOwnership() + assert(!(_entity->getClientOnly() && _entity->getOwningAvatarID() != Physics::getSessionUUID())); if (_entity->dynamicDataNeedsTransmit() || _entity->queryAACubeNeedsUpdate()) { return true; @@ -523,10 +520,6 @@ bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep) { _nextOwnershipBid = usecTimestampNow() + USECS_BETWEEN_OWNERSHIP_BIDS; } return shouldBid; - } else { - // When we own the simulation: make sure _outgoingPriority is not less than current owned priority - // because: an _outgoingPriority of zero indicates that we should drop ownership when we have it. - upgradeOutgoingPriority(_entity->getSimulationPriority()); } return remoteSimulationOutOfSync(simulationStep); @@ -534,7 +527,6 @@ bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep) { void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_t step) { DETAILED_PROFILE_RANGE(simulation_physics, "Send"); - assert(_entity); assert(entityTreeIsLocked()); if (!_body->isActive()) { @@ -807,7 +799,6 @@ QString EntityMotionState::getName() const { // virtual void EntityMotionState::computeCollisionGroupAndMask(int16_t& group, int16_t& mask) const { - assert(_entity); _entity->computeCollisionGroupAndFinalMask(group, mask); } From eb07f6732f33cb64ea69dd77c763098e3f4ac4bd Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 16 Mar 2018 11:45:53 -0700 Subject: [PATCH 19/30] add reminder comment --- libraries/physics/src/EntityMotionState.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 0e901356f5..093010c9e4 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -282,6 +282,7 @@ void EntityMotionState::setWorldTransform(const btTransform& worldTrans) { } } + // ADEBUG: move this _loopsWithoutOwner stuff out of EntityMotionState if (_entity->getSimulatorID().isNull()) { _loopsWithoutOwner++; if (_loopsWithoutOwner > LOOPS_FOR_SIMULATION_ORPHAN && usecTimestampNow() > _nextOwnershipBid) { From 170ec838701a17c0368f83e12a89e55c92d763b0 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 16 Mar 2018 11:46:04 -0700 Subject: [PATCH 20/30] remove unnecessary assert --- libraries/physics/src/PhysicalEntitySimulation.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 40eb6c34e4..103640e138 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -271,12 +271,10 @@ void PhysicalEntitySimulation::handleChangedMotionStates(const VectorOfMotionSta assert(state); if (state->getType() == MOTIONSTATE_TYPE_ENTITY) { EntityMotionState* entityState = static_cast(state); - EntityItemPointer entity = entityState->getEntity(); - assert(entity.get()); if (entityState->isCandidateForOwnership()) { _outgoingChanges.insert(entityState); } - _entitiesToSort.insert(entity); + _entitiesToSort.insert(entityState->getEntity()); } } } From 5a5376c3d5ddd2b3ff44f758015f0bd2fd6641d1 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 28 Mar 2018 13:40:16 -0700 Subject: [PATCH 21/30] untangle 'bidding' from 'owned' --- libraries/entities/src/EntitySimulation.cpp | 20 +- libraries/entities/src/EntitySimulation.h | 12 +- .../entities/src/SimpleEntitySimulation.cpp | 12 +- .../entities/src/SimpleEntitySimulation.h | 4 +- libraries/physics/src/EntityMotionState.cpp | 388 +++++++++--------- libraries/physics/src/EntityMotionState.h | 45 +- libraries/physics/src/ObjectMotionState.h | 2 +- .../physics/src/PhysicalEntitySimulation.cpp | 178 ++++++-- .../physics/src/PhysicalEntitySimulation.h | 17 +- libraries/physics/src/PhysicsEngine.cpp | 4 +- 10 files changed, 414 insertions(+), 268 deletions(-) diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 47e3e51fb9..82418e83e9 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -20,7 +20,7 @@ void EntitySimulation::setEntityTree(EntityTreePointer tree) { if (_entityTree && _entityTree != tree) { _mortalEntities.clear(); - _nextExpiry = quint64(-1); + _nextExpiry = uint64_t(-1); _entitiesToUpdate.clear(); _entitiesToSort.clear(); _simpleKinematicEntities.clear(); @@ -30,7 +30,7 @@ void EntitySimulation::setEntityTree(EntityTreePointer tree) { void EntitySimulation::updateEntities() { QMutexLocker lock(&_mutex); - quint64 now = usecTimestampNow(); + uint64_t now = usecTimestampNow(); // these methods may accumulate entries in _entitiesToBeDeleted expireMortalEntities(now); @@ -70,16 +70,16 @@ void EntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { } // protected -void EntitySimulation::expireMortalEntities(const quint64& now) { +void EntitySimulation::expireMortalEntities(const uint64_t& now) { if (now > _nextExpiry) { PROFILE_RANGE_EX(simulation_physics, "ExpireMortals", 0xffff00ff, (uint64_t)_mortalEntities.size()); // only search for expired entities if we expect to find one - _nextExpiry = quint64(-1); + _nextExpiry = uint64_t(-1); QMutexLocker lock(&_mutex); SetOfEntities::iterator itemItr = _mortalEntities.begin(); while (itemItr != _mortalEntities.end()) { EntityItemPointer entity = *itemItr; - quint64 expiry = entity->getExpiry(); + uint64_t expiry = entity->getExpiry(); if (expiry < now) { itemItr = _mortalEntities.erase(itemItr); entity->die(); @@ -99,7 +99,7 @@ void EntitySimulation::expireMortalEntities(const quint64& now) { } // protected -void EntitySimulation::callUpdateOnEntitiesThatNeedIt(const quint64& now) { +void EntitySimulation::callUpdateOnEntitiesThatNeedIt(const uint64_t& now) { PerformanceTimer perfTimer("updatingEntities"); QMutexLocker lock(&_mutex); SetOfEntities::iterator itemItr = _entitiesToUpdate.begin(); @@ -153,7 +153,7 @@ void EntitySimulation::addEntity(EntityItemPointer entity) { entity->deserializeActions(); if (entity->isMortal()) { _mortalEntities.insert(entity); - quint64 expiry = entity->getExpiry(); + uint64_t expiry = entity->getExpiry(); if (expiry < _nextExpiry) { _nextExpiry = expiry; } @@ -200,7 +200,7 @@ void EntitySimulation::changeEntity(EntityItemPointer entity) { if (dirtyFlags & Simulation::DIRTY_LIFETIME) { if (entity->isMortal()) { _mortalEntities.insert(entity); - quint64 expiry = entity->getExpiry(); + uint64_t expiry = entity->getExpiry(); if (expiry < _nextExpiry) { _nextExpiry = expiry; } @@ -220,7 +220,7 @@ void EntitySimulation::changeEntity(EntityItemPointer entity) { void EntitySimulation::clearEntities() { QMutexLocker lock(&_mutex); _mortalEntities.clear(); - _nextExpiry = quint64(-1); + _nextExpiry = uint64_t(-1); _entitiesToUpdate.clear(); _entitiesToSort.clear(); _simpleKinematicEntities.clear(); @@ -231,7 +231,7 @@ void EntitySimulation::clearEntities() { _deadEntities.clear(); } -void EntitySimulation::moveSimpleKinematics(const quint64& now) { +void EntitySimulation::moveSimpleKinematics(const uint64_t& now) { PROFILE_RANGE_EX(simulation_physics, "MoveSimples", 0xffff00ff, (uint64_t)_simpleKinematicEntities.size()); SetOfEntities::iterator itemItr = _simpleKinematicEntities.begin(); while (itemItr != _simpleKinematicEntities.end()) { diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index b5edb31907..19ab170ce5 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -44,7 +44,7 @@ const int DIRTY_SIMULATION_FLAGS = class EntitySimulation : public QObject, public std::enable_shared_from_this { public: - EntitySimulation() : _mutex(QMutex::Recursive), _entityTree(NULL), _nextExpiry(quint64(-1)) { } + EntitySimulation() : _mutex(QMutex::Recursive), _entityTree(NULL), _nextExpiry(uint64_t(-1)) { } virtual ~EntitySimulation() { setEntityTree(NULL); } inline EntitySimulationPointer getThisPointer() const { @@ -71,7 +71,7 @@ public: void clearEntities(); - void moveSimpleKinematics(const quint64& now); + void moveSimpleKinematics(const uint64_t& now); EntityTreePointer getEntityTree() { return _entityTree; } @@ -83,14 +83,14 @@ public: 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(const quint64& now) = 0; + virtual void updateEntitiesInternal(const uint64_t& now) = 0; virtual void addEntityInternal(EntityItemPointer entity) = 0; virtual void removeEntityInternal(EntityItemPointer entity); virtual void changeEntityInternal(EntityItemPointer entity) = 0; virtual void clearEntitiesInternal() = 0; - void expireMortalEntities(const quint64& now); - void callUpdateOnEntitiesThatNeedIt(const quint64& now); + void expireMortalEntities(const uint64_t& now); + void callUpdateOnEntitiesThatNeedIt(const uint64_t& now); virtual void sortEntitiesThatMoved(); QMutex _mutex{ QMutex::Recursive }; @@ -114,7 +114,7 @@ private: // An entity may be in more than one list. SetOfEntities _allEntities; // tracks all entities added the simulation SetOfEntities _mortalEntities; // entities that have an expiry - quint64 _nextExpiry; + uint64_t _nextExpiry; SetOfEntities _entitiesToUpdate; // entities that need to call EntityItem::update() diff --git a/libraries/entities/src/SimpleEntitySimulation.cpp b/libraries/entities/src/SimpleEntitySimulation.cpp index 583a310b41..266f10cf91 100644 --- a/libraries/entities/src/SimpleEntitySimulation.cpp +++ b/libraries/entities/src/SimpleEntitySimulation.cpp @@ -18,7 +18,7 @@ #include "EntityItem.h" #include "EntitiesLogging.h" -const quint64 MAX_OWNERLESS_PERIOD = 2 * USECS_PER_SECOND; +const uint64_t MAX_OWNERLESS_PERIOD = 2 * USECS_PER_SECOND; void SimpleEntitySimulation::clearOwnership(const QUuid& ownerID) { QMutexLocker lock(&_mutex); @@ -33,7 +33,7 @@ void SimpleEntitySimulation::clearOwnership(const QUuid& ownerID) { if (entity->getDynamic() && entity->hasLocalVelocity()) { // it is still moving dynamically --> add to orphaned list _entitiesThatNeedSimulationOwner.insert(entity); - quint64 expiry = entity->getLastChangedOnServer() + MAX_OWNERLESS_PERIOD; + uint64_t expiry = entity->getLastChangedOnServer() + MAX_OWNERLESS_PERIOD; if (expiry < _nextOwnerlessExpiry) { _nextOwnerlessExpiry = expiry; } @@ -50,7 +50,7 @@ void SimpleEntitySimulation::clearOwnership(const QUuid& ownerID) { } } -void SimpleEntitySimulation::updateEntitiesInternal(const quint64& now) { +void SimpleEntitySimulation::updateEntitiesInternal(const uint64_t& now) { if (now > _nextOwnerlessExpiry) { // search for ownerless objects that have expired QMutexLocker lock(&_mutex); @@ -58,7 +58,7 @@ void SimpleEntitySimulation::updateEntitiesInternal(const quint64& now) { SetOfEntities::iterator itemItr = _entitiesThatNeedSimulationOwner.begin(); while (itemItr != _entitiesThatNeedSimulationOwner.end()) { EntityItemPointer entity = *itemItr; - quint64 expiry = entity->getLastChangedOnServer() + MAX_OWNERLESS_PERIOD; + uint64_t expiry = entity->getLastChangedOnServer() + MAX_OWNERLESS_PERIOD; if (expiry < now) { // no simulators have volunteered ownership --> remove from list itemItr = _entitiesThatNeedSimulationOwner.erase(itemItr); @@ -96,7 +96,7 @@ void SimpleEntitySimulation::addEntityInternal(EntityItemPointer entity) { } else if (entity->getDynamic() && entity->hasLocalVelocity()) { QMutexLocker lock(&_mutex); _entitiesThatNeedSimulationOwner.insert(entity); - quint64 expiry = entity->getLastChangedOnServer() + MAX_OWNERLESS_PERIOD; + uint64_t expiry = entity->getLastChangedOnServer() + MAX_OWNERLESS_PERIOD; if (expiry < _nextOwnerlessExpiry) { _nextOwnerlessExpiry = expiry; } @@ -127,7 +127,7 @@ void SimpleEntitySimulation::changeEntityInternal(EntityItemPointer entity) { _entitiesWithSimulationOwner.remove(entity); if (entity->getDynamic() && entity->hasLocalVelocity()) { _entitiesThatNeedSimulationOwner.insert(entity); - quint64 expiry = entity->getLastChangedOnServer() + MAX_OWNERLESS_PERIOD; + uint64_t expiry = entity->getLastChangedOnServer() + MAX_OWNERLESS_PERIOD; if (expiry < _nextOwnerlessExpiry) { _nextOwnerlessExpiry = expiry; } diff --git a/libraries/entities/src/SimpleEntitySimulation.h b/libraries/entities/src/SimpleEntitySimulation.h index 9c7c9a374e..10714be299 100644 --- a/libraries/entities/src/SimpleEntitySimulation.h +++ b/libraries/entities/src/SimpleEntitySimulation.h @@ -28,7 +28,7 @@ public: void clearOwnership(const QUuid& ownerID); protected: - virtual void updateEntitiesInternal(const quint64& now) override; + virtual void updateEntitiesInternal(const uint64_t& now) override; virtual void addEntityInternal(EntityItemPointer entity) override; virtual void removeEntityInternal(EntityItemPointer entity) override; virtual void changeEntityInternal(EntityItemPointer entity) override; @@ -38,7 +38,7 @@ protected: SetOfEntities _entitiesWithSimulationOwner; SetOfEntities _entitiesThatNeedSimulationOwner; - quint64 _nextOwnerlessExpiry { 0 }; + uint64_t _nextOwnerlessExpiry { 0 }; }; #endif // hifi_SimpleEntitySimulation_h diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 093010c9e4..9e891806f2 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -26,12 +26,7 @@ #ifdef WANT_DEBUG_ENTITY_TREE_LOCKS #include "EntityTree.h" -#endif -const uint8_t LOOPS_FOR_SIMULATION_ORPHAN = 50; -const quint64 USECS_BETWEEN_OWNERSHIP_BIDS = USECS_PER_SECOND / 5; - -#ifdef WANT_DEBUG_ENTITY_TREE_LOCKS bool EntityMotionState::entityTreeIsLocked() const { EntityTreeElementPointer element = _entity->getElement(); EntityTreePointer tree = element ? element->getTree() : nullptr; @@ -46,6 +41,9 @@ bool entityTreeIsLocked() { } #endif +const uint8_t LOOPS_FOR_SIMULATION_ORPHAN = 50; +const quint64 USECS_BETWEEN_OWNERSHIP_BIDS = USECS_PER_SECOND / 5; + EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer entity) : ObjectMotionState(nullptr), @@ -59,7 +57,7 @@ EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer _serverActionData(QByteArray()), _lastVelocity(0.0f), _measuredAcceleration(0.0f), - _nextOwnershipBid(0), + _nextBidExpiry(0), _measuredDeltaTime(0.0f), _lastMeasureStep(0), _lastStep(0), @@ -67,6 +65,12 @@ EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer _accelerationNearlyGravityCount(0), _numInactiveUpdates(1) { + // Why is _numInactiveUpdates initialied to 1? + // Because: when an entity is first created by a LOCAL operatioin its local simulation ownership is assumed, + // which causes it to be immediately placed on the 'owned' list, but in this case an "update" already just + // went out for the object's creation and there is no need to send another. By initializing _numInactiveUpdates + // to 1 here we trick remoteSimulationOutOfSync() to return "false" the first time through for this case. + _type = MOTIONSTATE_TYPE_ENTITY; assert(_entity); assert(entityTreeIsLocked()); @@ -75,7 +79,19 @@ EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer // rather than pass the legit shape pointer to the ObjectMotionState ctor above. setShape(shape); - _outgoingPriority = _entity->getPendingOwnershipPriority(); + _bidPriority = _entity->getPendingOwnershipPriority(); + if (_entity->getClientOnly() && _entity->getOwningAvatarID() != Physics::getSessionUUID()) { + // client-only entities are always thus, so we cache this fact in _ownershipState + _ownershipState = EntityMotionState::OwnershipState::Unownable; + } + + Transform localTransform; + _entity->getLocalTransformAndVelocities(localTransform, _serverVelocity, _serverAngularVelocity); + _serverPosition = localTransform.getTranslation(); + _serverRotation = localTransform.getRotation(); + _serverAcceleration = _entity->getAcceleration(); + _serverActionData = _entity->getDynamicData(); + } EntityMotionState::~EntityMotionState() { @@ -87,35 +103,30 @@ EntityMotionState::~EntityMotionState() { } void EntityMotionState::updateServerPhysicsVariables() { - assert(entityTreeIsLocked()); - if (isLocallyOwned()) { - // don't slam these values if we are the simulation owner - return; + if (_ownershipState != EntityMotionState::OwnershipState::LocallyOwned) { + // only slam these values if we are NOT the simulation owner + Transform localTransform; + _entity->getLocalTransformAndVelocities(localTransform, _serverVelocity, _serverAngularVelocity); + _serverPosition = localTransform.getTranslation(); + _serverRotation = localTransform.getRotation(); + _serverAcceleration = _entity->getAcceleration(); + _serverActionData = _entity->getDynamicData(); + _lastStep = ObjectMotionState::getWorldSimulationStep(); } - - Transform localTransform; - _entity->getLocalTransformAndVelocities(localTransform, _serverVelocity, _serverAngularVelocity); - _serverVariablesSet = true; - _serverPosition = localTransform.getTranslation(); - _serverRotation = localTransform.getRotation(); - _serverAcceleration = _entity->getAcceleration(); - _serverActionData = _entity->getDynamicData(); } void EntityMotionState::handleDeactivation() { - if (_serverVariablesSet) { - // copy _server data to entity - Transform localTransform = _entity->getLocalTransform(); - localTransform.setTranslation(_serverPosition); - localTransform.setRotation(_serverRotation); - _entity->setLocalTransformAndVelocities(localTransform, ENTITY_ITEM_ZERO_VEC3, ENTITY_ITEM_ZERO_VEC3); - // and also to RigidBody - btTransform worldTrans; - worldTrans.setOrigin(glmToBullet(_entity->getWorldPosition())); - worldTrans.setRotation(glmToBullet(_entity->getWorldOrientation())); - _body->setWorldTransform(worldTrans); - // no need to update velocities... should already be zero - } + // copy _server data to entity + Transform localTransform = _entity->getLocalTransform(); + localTransform.setTranslation(_serverPosition); + localTransform.setRotation(_serverRotation); + _entity->setLocalTransformAndVelocities(localTransform, ENTITY_ITEM_ZERO_VEC3, ENTITY_ITEM_ZERO_VEC3); + // and also to RigidBody + btTransform worldTrans; + worldTrans.setOrigin(glmToBullet(_entity->getWorldPosition())); + worldTrans.setRotation(glmToBullet(_entity->getWorldOrientation())); + _body->setWorldTransform(worldTrans); + // no need to update velocities... should already be zero } // virtual @@ -128,26 +139,29 @@ void EntityMotionState::handleEasyChanges(uint32_t& flags) { if (_entity->getSimulatorID().isNull()) { // simulation ownership has been removed if (glm::length2(_entity->getWorldVelocity()) == 0.0f) { - // this object is coming to rest --> clear the ACTIVATION flag and _outgoingPriority + // this object is coming to rest --> clear the ACTIVATION flag and _bidPriority flags &= ~Simulation::DIRTY_PHYSICS_ACTIVATION; _body->setActivationState(WANTS_DEACTIVATION); - _outgoingPriority = 0; + _bidPriority = 0; const float ACTIVATION_EXPIRY = 3.0f; // something larger than the 2.0 hard coded in Bullet _body->setDeactivationTime(ACTIVATION_EXPIRY); } else { // disowned object is still moving --> start timer for ownership bid // TODO? put a delay in here proportional to distance from object? - upgradeOutgoingPriority(VOLUNTEER_SIMULATION_PRIORITY); - _nextOwnershipBid = usecTimestampNow() + USECS_BETWEEN_OWNERSHIP_BIDS; + upgradeBidPriority(VOLUNTEER_SIMULATION_PRIORITY); + _nextBidExpiry = usecTimestampNow() + USECS_BETWEEN_OWNERSHIP_BIDS; } _loopsWithoutOwner = 0; _numInactiveUpdates = 0; } else if (isLocallyOwned()) { // we just inherited ownership, make sure our desired priority matches what we have - upgradeOutgoingPriority(_entity->getSimulationPriority()); + upgradeBidPriority(_entity->getSimulationPriority()); } else { - _outgoingPriority = 0; - _nextOwnershipBid = usecTimestampNow() + USECS_BETWEEN_OWNERSHIP_BIDS; + // the entity is owned by someone else, so we clear _bidPriority here + // but _bidPriority may be updated to non-zero value if this object interacts with locally owned simulation + // in which case we may try to bid again + _bidPriority = 0; + _nextBidExpiry = usecTimestampNow() + USECS_BETWEEN_OWNERSHIP_BIDS; _numInactiveUpdates = 0; } } @@ -156,10 +170,10 @@ void EntityMotionState::handleEasyChanges(uint32_t& flags) { // (1) we own it but may need to change the priority OR... // (2) we don't own it but should bid (because a local script has been changing physics properties) uint8_t newPriority = isLocallyOwned() ? _entity->getSimulationOwner().getPriority() : _entity->getSimulationOwner().getPendingPriority(); - upgradeOutgoingPriority(newPriority); + upgradeBidPriority(newPriority); // reset bid expiry so that we bid ASAP - _nextOwnershipBid = 0; + _nextBidExpiry = 0; } if ((flags & Simulation::DIRTY_PHYSICS_ACTIVATION) && !_body->isActive()) { if (_body->isKinematicObject()) { @@ -282,24 +296,12 @@ void EntityMotionState::setWorldTransform(const btTransform& worldTrans) { } } - // ADEBUG: move this _loopsWithoutOwner stuff out of EntityMotionState if (_entity->getSimulatorID().isNull()) { _loopsWithoutOwner++; - if (_loopsWithoutOwner > LOOPS_FOR_SIMULATION_ORPHAN && usecTimestampNow() > _nextOwnershipBid) { - upgradeOutgoingPriority(VOLUNTEER_SIMULATION_PRIORITY); + if (_loopsWithoutOwner > LOOPS_FOR_SIMULATION_ORPHAN && usecTimestampNow() > _nextBidExpiry) { + upgradeBidPriority(VOLUNTEER_SIMULATION_PRIORITY); } } - - #ifdef WANT_DEBUG - quint64 now = usecTimestampNow(); - qCDebug(physics) << "EntityMotionState::setWorldTransform()... changed entity:" << _entity->getEntityItemID(); - qCDebug(physics) << " last edited:" << _entity->getLastEdited() - << formatUsecTime(now - _entity->getLastEdited()) << "ago"; - qCDebug(physics) << " last simulated:" << _entity->getLastSimulated() - << formatUsecTime(now - _entity->getLastSimulated()) << "ago"; - qCDebug(physics) << " last updated:" << _entity->getLastUpdated() - << formatUsecTime(now - _entity->getLastUpdated()) << "ago"; - #endif } @@ -323,26 +325,13 @@ void EntityMotionState::setShape(const btCollisionShape* shape) { } } -bool EntityMotionState::isCandidateForOwnership() const { - assert(_body); - assert(entityTreeIsLocked()); - if (isLocallyOwned()) { - return true; - } else if (_entity->getClientOnly()) { - // don't send updates for someone else's avatarEntities - return false; - } - return _outgoingPriority != 0 || _entity->dynamicDataNeedsTransmit(); -} - bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { // NOTE: we only get here if we think we own the simulation DETAILED_PROFILE_RANGE(simulation_physics, "CheckOutOfSync"); - assert(_body); - // Since we own the simulation: make sure _outgoingPriority is not less than current owned priority - // because: an _outgoingPriority of zero indicates that we should drop ownership when we have it. - upgradeOutgoingPriority(_entity->getSimulationPriority()); + // Since we own the simulation: make sure _bidPriority is not less than current owned priority + // because: an _bidPriority of zero indicates that we should drop ownership when we have it. + upgradeBidPriority(_entity->getSimulationPriority()); bool parentTransformSuccess; Transform localToWorld = _entity->getParentTransform(parentTransformSuccess); @@ -354,27 +343,6 @@ bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { worldVelocityToLocal.setTranslation(glm::vec3(0.0f)); } - // if we've never checked before, our _lastStep will be 0, and we need to initialize our state - if (_lastStep == 0) { - btTransform xform = _body->getWorldTransform(); - _serverVariablesSet = true; - _serverPosition = worldToLocal.transform(bulletToGLM(xform.getOrigin())); - _serverRotation = worldToLocal.getRotation() * bulletToGLM(xform.getRotation()); - _serverVelocity = worldVelocityToLocal.transform(getBodyLinearVelocityGTSigma()); - _serverAcceleration = Vectors::ZERO; - _serverAngularVelocity = worldVelocityToLocal.transform(bulletToGLM(_body->getAngularVelocity())); - _lastStep = simulationStep; - _serverActionData = _entity->getDynamicData(); - _numInactiveUpdates = 1; - return false; - } - - #ifdef WANT_DEBUG - glm::vec3 wasPosition = _serverPosition; - glm::quat wasRotation = _serverRotation; - glm::vec3 wasAngularVelocity = _serverAngularVelocity; - #endif - int numSteps = simulationStep - _lastStep; float dt = (float)(numSteps) * PHYSICS_ENGINE_FIXED_SUBSTEP; @@ -385,9 +353,9 @@ bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { _entity->clearSimulationOwnership(); return false; } - // we resend the inactive update every INACTIVE_UPDATE_PERIOD - // until it is removed from the outgoing updates - // (which happens when we don't own the simulation and it isn't touching our simulation) + // we resend the inactive update with a growing delay: every INACTIVE_UPDATE_PERIOD * _numInactiveUpdates + // until it is removed from the owned list + // (which happens when we no longer own the simulation) const float INACTIVE_UPDATE_PERIOD = 0.5f; return (dt > INACTIVE_UPDATE_PERIOD * (float)_numInactiveUpdates); } @@ -418,7 +386,7 @@ bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { if (_entity->dynamicDataNeedsTransmit()) { uint8_t priority = _entity->hasActions() ? SCRIPT_GRAB_SIMULATION_PRIORITY : SCRIPT_POKE_SIMULATION_PRIORITY; - upgradeOutgoingPriority(priority); + upgradeBidPriority(priority); return true; } @@ -443,13 +411,6 @@ bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { const float MIN_ERROR_RATIO_SQUARED = 0.0025f; // corresponds to 5% error in 1 second const float MIN_SPEED_SQUARED = 1.0e-6f; // corresponds to 1mm/sec if (speed2 < MIN_SPEED_SQUARED || dx2 / speed2 > MIN_ERROR_RATIO_SQUARED) { - #ifdef WANT_DEBUG - qCDebug(physics) << ".... (dx2 > MAX_POSITION_ERROR_SQUARED) ...."; - qCDebug(physics) << "wasPosition:" << wasPosition; - qCDebug(physics) << "bullet position:" << position; - qCDebug(physics) << "_serverPosition:" << _serverPosition; - qCDebug(physics) << "dx2:" << dx2; - #endif return true; } } @@ -469,22 +430,6 @@ bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) { const float MIN_ROTATION_DOT = 0.99999f; // This corresponds to about 0.5 degrees of rotation glm::quat actualRotation = worldToLocal.getRotation() * bulletToGLM(worldTrans.getRotation()); - #ifdef WANT_DEBUG - if ((fabsf(glm::dot(actualRotation, _serverRotation)) < MIN_ROTATION_DOT)) { - qCDebug(physics) << ".... ((fabsf(glm::dot(actualRotation, _serverRotation)) < MIN_ROTATION_DOT)) ...."; - - qCDebug(physics) << "wasAngularVelocity:" << wasAngularVelocity; - qCDebug(physics) << "_serverAngularVelocity:" << _serverAngularVelocity; - - qCDebug(physics) << "length wasAngularVelocity:" << glm::length(wasAngularVelocity); - qCDebug(physics) << "length _serverAngularVelocity:" << glm::length(_serverAngularVelocity); - - qCDebug(physics) << "wasRotation:" << wasRotation; - qCDebug(physics) << "bullet actualRotation:" << actualRotation; - qCDebug(physics) << "_serverRotation:" << _serverRotation; - } - #endif - return (fabsf(glm::dot(actualRotation, _serverRotation)) < MIN_ROTATION_DOT); } @@ -492,10 +437,9 @@ bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep) { DETAILED_PROFILE_RANGE(simulation_physics, "ShouldSend"); // NOTE: we expect _entity and _body to be valid in this context, since shouldSendUpdate() is only called // after doesNotNeedToSendUpdate() returns false and that call should return 'true' if _entity or _body are NULL. - assert(_body); assert(entityTreeIsLocked()); - // this case prevented by isCandidateForOwnership() + // this case is prevented by setting _ownershipState to UNOWNABLE in EntityMotionState::ctor assert(!(_entity->getClientOnly() && _entity->getOwningAvatarID() != Physics::getSessionUUID())); if (_entity->dynamicDataNeedsTransmit() || _entity->queryAACubeNeedsUpdate()) { @@ -506,33 +450,111 @@ bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep) { return false; } - if (!isLocallyOwned()) { - // we don't own the simulation + return remoteSimulationOutOfSync(simulationStep); +} - // NOTE: we do not volunteer to own kinematic or static objects - uint8_t volunteerPriority = _body->isStaticOrKinematicObject() ? VOLUNTEER_SIMULATION_PRIORITY : 0; +void EntityMotionState::sendBid(OctreeEditPacketSender* packetSender, uint32_t step) { + DETAILED_PROFILE_RANGE(simulation_physics, "Bid"); + assert(entityTreeIsLocked()); - bool shouldBid = _outgoingPriority > volunteerPriority && // but we would like to own it AND - usecTimestampNow() > _nextOwnershipBid; // it is time to bid again - if (shouldBid && _outgoingPriority < _entity->getSimulationPriority()) { - // we are insufficiently interested so clear _outgoingPriority - // and reset the bid expiry - _outgoingPriority = 0; - _nextOwnershipBid = usecTimestampNow() + USECS_BETWEEN_OWNERSHIP_BIDS; + if (!_body->isActive()) { + // make sure all derivatives are zero + clearObjectVelocities(); + _numInactiveUpdates = 1; + } else { + glm::vec3 gravity = _entity->getGravity(); + + // if this entity has been accelerated at close to gravity for a certain number of simulation-steps + // let the entity server's estimates include gravity. + const uint8_t STEPS_TO_DECIDE_BALLISTIC = 4; + if (_accelerationNearlyGravityCount >= STEPS_TO_DECIDE_BALLISTIC) { + _entity->setAcceleration(gravity); + } else { + _entity->setAcceleration(Vectors::ZERO); } - return shouldBid; + + if (!_body->isStaticOrKinematicObject()) { + const float DYNAMIC_LINEAR_VELOCITY_THRESHOLD = 0.05f; // 5 cm/sec + const float DYNAMIC_ANGULAR_VELOCITY_THRESHOLD = 0.087266f; // ~5 deg/sec + + bool movingSlowlyLinear = + glm::length2(_entity->getWorldVelocity()) < (DYNAMIC_LINEAR_VELOCITY_THRESHOLD * DYNAMIC_LINEAR_VELOCITY_THRESHOLD); + bool movingSlowlyAngular = glm::length2(_entity->getWorldAngularVelocity()) < + (DYNAMIC_ANGULAR_VELOCITY_THRESHOLD * DYNAMIC_ANGULAR_VELOCITY_THRESHOLD); + bool movingSlowly = movingSlowlyLinear && movingSlowlyAngular && _entity->getAcceleration() == Vectors::ZERO; + + if (movingSlowly) { + // velocities might not be zero, but we'll fake them as such, which will hopefully help convince + // other simulating observers to deactivate their own copies + clearObjectVelocities(); + } + } + _numInactiveUpdates = 0; } - return remoteSimulationOutOfSync(simulationStep); + EntityItemProperties properties; + Transform localTransform; + glm::vec3 linearVelocity; + glm::vec3 angularVelocity; + _entity->getLocalTransformAndVelocities(localTransform, linearVelocity, angularVelocity); + properties.setPosition(localTransform.getTranslation()); + properties.setRotation(localTransform.getRotation()); + properties.setVelocity(linearVelocity); + properties.setAcceleration(_entity->getAcceleration()); + properties.setAngularVelocity(angularVelocity); + if (_entity->dynamicDataNeedsTransmit()) { + _entity->setDynamicDataNeedsTransmit(false); + properties.setActionData(_entity->getDynamicData()); + } + + if (_entity->updateQueryAACube()) { + // due to parenting, the server may not know where something is in world-space, so include the bounding cube. + properties.setQueryAACube(_entity->getQueryAACube()); + } + + // set the LastEdited of the properties but NOT the entity itself + quint64 now = usecTimestampNow(); + properties.setLastEdited(now); + + // we don't own the simulation for this entity yet, but we're sending a bid for it + uint8_t bidPriority = glm::max(_bidPriority, VOLUNTEER_SIMULATION_PRIORITY); + properties.setSimulationOwner(Physics::getSessionUUID(), bidPriority); + // copy _bidPriority into pendingPriority... + _entity->setPendingOwnershipPriority(_bidPriority, now); + // don't forget to remember that we have made a bid + _entity->rememberHasSimulationOwnershipBid(); + + EntityTreeElementPointer element = _entity->getElement(); + EntityTreePointer tree = element ? element->getTree() : nullptr; + + properties.setClientOnly(_entity->getClientOnly()); + properties.setOwningAvatarID(_entity->getOwningAvatarID()); + + EntityItemID id(_entity->getID()); + EntityEditPacketSender* entityPacketSender = static_cast(packetSender); + entityPacketSender->queueEditEntityMessage(PacketType::EntityPhysics, tree, id, properties); + _entity->setLastBroadcast(now); // ffor debug/physics status icons + + // NOTE: we don't descend to children for ownership bid. Instead, if we win ownership of the parent + // then in sendUpdate() we'll walk descendents and send updates for their QueryAACubes if necessary. + + _lastStep = step; + _nextBidExpiry = now + USECS_BETWEEN_OWNERSHIP_BIDS; + + // finally: clear _bidPriority + // which will may get promoted before next bid + // or maybe we'll win simulation ownership + _bidPriority = 0; } void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_t step) { DETAILED_PROFILE_RANGE(simulation_physics, "Send"); assert(entityTreeIsLocked()); + assert(_entity->isLocallyOwned()); if (!_body->isActive()) { // make sure all derivatives are zero - zeroCleanObjectVelocities(); + clearObjectVelocities(); _numInactiveUpdates++; } else { glm::vec3 gravity = _entity->getGravity(); @@ -559,13 +581,13 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_ if (movingSlowly) { // velocities might not be zero, but we'll fake them as such, which will hopefully help convince // other simulating observers to deactivate their own copies - zeroCleanObjectVelocities(); + clearObjectVelocities(); } } _numInactiveUpdates = 0; } - // remember properties for local server prediction + // remember _serverFoo data for local prediction of server state Transform localTransform; _entity->getLocalTransformAndVelocities(localTransform, _serverVelocity, _serverAngularVelocity); _serverPosition = localTransform.getTranslation(); @@ -574,11 +596,8 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_ _serverActionData = _entity->getDynamicData(); EntityItemProperties properties; - - // explicitly set the properties that changed so that they will be packed properties.setPosition(_entity->getLocalPosition()); properties.setRotation(_entity->getLocalOrientation()); - properties.setVelocity(_serverVelocity); properties.setAcceleration(_serverAcceleration); properties.setAngularVelocity(_serverAngularVelocity); @@ -587,61 +606,35 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_ properties.setActionData(_serverActionData); } - if (properties.transformChanged()) { - if (_entity->updateQueryAACube()) { - // due to parenting, the server may not know where something is in world-space, so include the bounding cube. - properties.setQueryAACube(_entity->getQueryAACube()); - } + if (_entity->updateQueryAACube()) { + // due to parenting, the server may not know where something is in world-space, so include the bounding cube. + properties.setQueryAACube(_entity->getQueryAACube()); } // set the LastEdited of the properties but NOT the entity itself quint64 now = usecTimestampNow(); properties.setLastEdited(now); - #ifdef WANT_DEBUG - quint64 lastSimulated = _entity->getLastSimulated(); - qCDebug(physics) << "EntityMotionState::sendUpdate()"; - qCDebug(physics) << " EntityItemId:" << _entity->getEntityItemID() - << "---------------------------------------------"; - qCDebug(physics) << " lastSimulated:" << debugTime(lastSimulated, now); - #endif //def WANT_DEBUG - if (_numInactiveUpdates > 0) { - // we own the simulation but the entity has stopped so we tell the server we're clearing simulatorID + // the entity is stopped and inactive so we tell the server we're clearing simulatorID // but we remember we do still own it... and rely on the server to tell us we don't properties.clearSimulationOwner(); - _outgoingPriority = 0; - _entity->setPendingOwnershipPriority(_outgoingPriority, now); - } else if (!isLocallyOwned()) { - // we don't own the simulation for this entity yet, but we're sending a bid for it - uint8_t bidPriority = glm::max(_outgoingPriority, VOLUNTEER_SIMULATION_PRIORITY); - properties.setSimulationOwner(Physics::getSessionUUID(), bidPriority); - _nextOwnershipBid = now + USECS_BETWEEN_OWNERSHIP_BIDS; - // copy _outgoingPriority into pendingPriority... - _entity->setPendingOwnershipPriority(_outgoingPriority, now); - // don't forget to remember that we have made a bid - _entity->rememberHasSimulationOwnershipBid(); - // ...then reset _outgoingPriority - _outgoingPriority = 0; - // _outgoingPriority will be re-computed before next bid, - // or will be set to agree with ownership priority should we win the bid - } else if (_outgoingPriority != _entity->getSimulationPriority()) { - // we own the simulation but our desired priority has changed - if (_outgoingPriority == 0) { + _bidPriority = 0; + _entity->setPendingOwnershipPriority(_bidPriority, now); + } else if (_bidPriority != _entity->getSimulationPriority()) { + // our desired priority has changed + if (_bidPriority == 0) { // we should release ownership properties.clearSimulationOwner(); } else { // we just need to change the priority - properties.setSimulationOwner(Physics::getSessionUUID(), _outgoingPriority); + properties.setSimulationOwner(Physics::getSessionUUID(), _bidPriority); } - _entity->setPendingOwnershipPriority(_outgoingPriority, now); + _entity->setPendingOwnershipPriority(_bidPriority, now); } EntityItemID id(_entity->getID()); EntityEditPacketSender* entityPacketSender = static_cast(packetSender); - #ifdef WANT_DEBUG - qCDebug(physics) << "EntityMotionState::sendUpdate()... calling queueEditEntityMessage()..."; - #endif EntityTreeElementPointer element = _entity->getElement(); EntityTreePointer tree = element ? element->getTree() : nullptr; @@ -650,7 +643,7 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_ properties.setOwningAvatarID(_entity->getOwningAvatarID()); entityPacketSender->queueEditEntityMessage(PacketType::EntityPhysics, tree, id, properties); - _entity->setLastBroadcast(now); + _entity->setLastBroadcast(now); // for debug/physics status icons // if we've moved an entity with children, check/update the queryAACube of all descendents and tell the server // if they've changed. @@ -661,13 +654,12 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_ EntityItemProperties newQueryCubeProperties; newQueryCubeProperties.setQueryAACube(descendant->getQueryAACube()); newQueryCubeProperties.setLastEdited(properties.getLastEdited()); - newQueryCubeProperties.setClientOnly(entityDescendant->getClientOnly()); newQueryCubeProperties.setOwningAvatarID(entityDescendant->getOwningAvatarID()); entityPacketSender->queueEditEntityMessage(PacketType::EntityPhysics, tree, descendant->getID(), newQueryCubeProperties); - entityDescendant->setLastBroadcast(now); + entityDescendant->setLastBroadcast(now); // for debug/physics status icons } } }); @@ -726,7 +718,7 @@ QUuid EntityMotionState::getSimulatorID() const { void EntityMotionState::bump(uint8_t priority) { assert(priority != 0); - upgradeOutgoingPriority(glm::max(VOLUNTEER_SIMULATION_PRIORITY, --priority)); + upgradeBidPriority(glm::max(VOLUNTEER_SIMULATION_PRIORITY, --priority)); } void EntityMotionState::resetMeasuredBodyAcceleration() { @@ -750,13 +742,14 @@ void EntityMotionState::measureBodyAcceleration() { _lastMeasureStep = thisStep; _measuredDeltaTime = dt; - // Note: the integration equation for velocity uses damping: v1 = (v0 + a * dt) * (1 - D)^dt + // Note: the integration equation for velocity uses damping (D): v1 = (v0 + a * dt) * (1 - D)^dt // hence the equation for acceleration is: a = (v1 / (1 - D)^dt - v0) / dt glm::vec3 velocity = getBodyLinearVelocityGTSigma(); _measuredAcceleration = (velocity / powf(1.0f - _body->getLinearDamping(), dt) - _lastVelocity) * invDt; _lastVelocity = velocity; if (numSubsteps > PHYSICS_ENGINE_MAX_NUM_SUBSTEPS) { + // we fall in here when _lastMeasureStep is old: the body has just become active _loopsWithoutOwner = 0; _lastStep = ObjectMotionState::getWorldSimulationStep(); _numInactiveUpdates = 0; @@ -803,20 +796,41 @@ void EntityMotionState::computeCollisionGroupAndMask(int16_t& group, int16_t& ma _entity->computeCollisionGroupAndFinalMask(group, mask); } +bool EntityMotionState::shouldSendBid() { + if (_bidPriority >= glm::max(_entity->getSimulationPriority(), VOLUNTEER_SIMULATION_PRIORITY)) { + return true; + } else { + // NOTE: this 'else' case has a side-effect: it clears _bidPriority + // which may be updated next simulation step (via collision or script event) + _bidPriority = 0; + return false; + } +} + bool EntityMotionState::isLocallyOwned() const { return _entity->getSimulatorID() == Physics::getSessionUUID(); } -bool EntityMotionState::shouldBeLocallyOwned() const { - return (_outgoingPriority > VOLUNTEER_SIMULATION_PRIORITY && _outgoingPriority > _entity->getSimulationPriority()) || +bool EntityMotionState::isLocallyOwnedOrShouldBe() const { + return (_bidPriority > VOLUNTEER_SIMULATION_PRIORITY && _bidPriority > _entity->getSimulationPriority()) || _entity->getSimulatorID() == Physics::getSessionUUID(); } -void EntityMotionState::upgradeOutgoingPriority(uint8_t priority) { - _outgoingPriority = glm::max(_outgoingPriority, priority); +void EntityMotionState::initForBid() { + assert(_ownershipState != EntityMotionState::OwnershipState::Unownable); + _ownershipState = EntityMotionState::OwnershipState::PendingBid; } -void EntityMotionState::zeroCleanObjectVelocities() const { +void EntityMotionState::initForOwned() { + assert(_ownershipState != EntityMotionState::OwnershipState::Unownable); + _ownershipState = EntityMotionState::OwnershipState::LocallyOwned; +} + +void EntityMotionState::upgradeBidPriority(uint8_t priority) { + _bidPriority = glm::max(_bidPriority, priority); +} + +void EntityMotionState::clearObjectVelocities() const { // If transform or velocities are flagged as dirty it means a network or scripted change // occured between the beginning and end of the stepSimulation() and we DON'T want to apply // these physics simulation results. diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index 9722fe0a7a..b37fa9df68 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -24,12 +24,17 @@ class EntityMotionState : public ObjectMotionState { public: + enum class OwnershipState { + NotLocallyOwned = 0, + PendingBid, + LocallyOwned, + Unownable + }; EntityMotionState() = delete; EntityMotionState(btCollisionShape* shape, EntityItemPointer item); virtual ~EntityMotionState(); - void updateServerPhysicsVariables(); void handleDeactivation(); virtual void handleEasyChanges(uint32_t& flags) override; virtual bool handleHardAndEasyChanges(uint32_t& flags, PhysicsEngine* engine) override; @@ -45,9 +50,8 @@ public: // this relays outgoing position/rotation to the EntityItem virtual void setWorldTransform(const btTransform& worldTrans) override; - bool isCandidateForOwnership() const; - bool remoteSimulationOutOfSync(uint32_t simulationStep); bool shouldSendUpdate(uint32_t simulationStep); + void sendBid(OctreeEditPacketSender* packetSender, uint32_t step); void sendUpdate(OctreeEditPacketSender* packetSender, uint32_t step); virtual uint32_t getIncomingDirtyFlags() override; @@ -80,15 +84,24 @@ public: virtual void computeCollisionGroupAndMask(int16_t& group, int16_t& mask) const override; + bool shouldSendBid(); bool isLocallyOwned() const override; - bool shouldBeLocallyOwned() const override; + bool isLocallyOwnedOrShouldBe() const override; // aka shouldEmitCollisionEvents() friend class PhysicalEntitySimulation; + OwnershipState getOwnershipState() const { return _ownershipState; } protected: - // changes _outgoingPriority only if priority is larger - void upgradeOutgoingPriority(uint8_t priority); - void zeroCleanObjectVelocities() const; + uint64_t getNextBidExpiry() const { return _nextBidExpiry; } + void initForBid(); + void initForOwned(); + void clearOwnershipState() { _ownershipState = OwnershipState::NotLocallyOwned; } + void updateServerPhysicsVariables(); + bool remoteSimulationOutOfSync(uint32_t simulationStep); + + // changes _bidPriority only if priority is larger + void upgradeBidPriority(uint8_t priority); + void clearObjectVelocities() const; #ifdef WANT_DEBUG_ENTITY_TREE_LOCKS bool entityTreeIsLocked() const; @@ -103,7 +116,17 @@ protected: // and is only cleared in the DTOR EntityItemPointer _entity; - bool _serverVariablesSet { false }; + // These "_serverFoo" variables represent what we think the server knows. + // They are used in two different modes: + // + // (1) For remotely owned simulation: we store the last values recieved from the server. + // When the body comes to rest and goes inactive we slam its final transforms to agree with the last server + // update. This to reduce state synchronization errors when the local simulation deviated from remote. + // + // (2) For locally owned simulation: we store the last values sent to the server, integrated forward over time + // according to how we think the server doing it. We calculate the error between the true local transform + // and the remote to decide when to send another update. + // glm::vec3 _serverPosition; // in simulation-frame (not world-frame) glm::quat _serverRotation; glm::vec3 _serverVelocity; @@ -114,16 +137,18 @@ protected: glm::vec3 _lastVelocity; glm::vec3 _measuredAcceleration; - quint64 _nextOwnershipBid { 0 }; + quint64 _nextBidExpiry { 0 }; float _measuredDeltaTime; uint32_t _lastMeasureStep; uint32_t _lastStep; // last step of server extrapolation + OwnershipState _ownershipState { OwnershipState::NotLocallyOwned }; uint8_t _loopsWithoutOwner; mutable uint8_t _accelerationNearlyGravityCount; uint8_t _numInactiveUpdates { 1 }; - uint8_t _outgoingPriority { 0 }; + uint8_t _bidPriority { 0 }; + bool _serverVariablesSet { false }; }; #endif // hifi_EntityMotionState_h diff --git a/libraries/physics/src/ObjectMotionState.h b/libraries/physics/src/ObjectMotionState.h index 8383c539f4..fbda9366fc 100644 --- a/libraries/physics/src/ObjectMotionState.h +++ b/libraries/physics/src/ObjectMotionState.h @@ -164,7 +164,7 @@ public: void clearInternalKinematicChanges() { _hasInternalKinematicChanges = false; } virtual bool isLocallyOwned() const { return false; } - virtual bool shouldBeLocallyOwned() const { return false; } + virtual bool isLocallyOwnedOrShouldBe() const { return false; } // aka shouldEmitCollisionEvents() friend class PhysicsEngine; diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 103640e138..9129e29671 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -40,7 +40,7 @@ void PhysicalEntitySimulation::init( } // begin EntitySimulation overrides -void PhysicalEntitySimulation::updateEntitiesInternal(const quint64& now) { +void PhysicalEntitySimulation::updateEntitiesInternal(const uint64_t& now) { // Do nothing here because the "internal" update the PhysicsEngine::stepSimualtion() which is done elsewhere. } @@ -65,7 +65,7 @@ void PhysicalEntitySimulation::removeEntityInternal(EntityItemPointer entity) { EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); if (motionState) { - _outgoingChanges.remove(motionState); + removeOwnershipData(motionState); _entitiesToRemoveFromPhysics.insert(entity); } else if (entity->isDead() && entity->getElement()) { _deadEntities.insert(entity); @@ -73,6 +73,42 @@ void PhysicalEntitySimulation::removeEntityInternal(EntityItemPointer entity) { } } +void PhysicalEntitySimulation::removeOwnershipData(EntityMotionState* motionState) { + assert(motionState); + if (motionState->getOwnershipState() == EntityMotionState::OwnershipState::LocallyOwned) { + for (uint32_t i = 0; i < _owned.size(); ++i) { + if (_owned[i] == motionState) { + _owned[i]->clearOwnershipState(); + if (i < _owned.size() - 1) { + _owned[i] = _owned.back(); + } + _owned.pop_back(); + } + } + } else if (motionState->getOwnershipState() == EntityMotionState::OwnershipState::PendingBid) { + for (uint32_t i = 0; i < _bids.size(); ++i) { + if (_bids[i] == motionState) { + _bids[i]->clearOwnershipState(); + if (i < _bids.size() - 1) { + _bids[i] = _bids.back(); + } + _bids.pop_back(); + } + } + } +} + +void PhysicalEntitySimulation::clearOwnershipData() { + for (uint32_t i = 0; i < _owned.size(); ++i) { + _owned[i]->clearOwnershipState(); + } + _owned.clear(); + for (uint32_t i = 0; i < _bids.size(); ++i) { + _bids[i]->clearOwnershipState(); + } + _bids.clear(); +} + void PhysicalEntitySimulation::takeDeadEntities(SetOfEntities& deadEntities) { QMutexLocker lock(&_mutex); for (auto entity : _deadEntities) { @@ -93,15 +129,15 @@ void PhysicalEntitySimulation::changeEntityInternal(EntityItemPointer entity) { if (motionState) { if (!entity->shouldBePhysical()) { // the entity should be removed from the physical simulation - _pendingChanges.remove(motionState); + _incomingChanges.remove(motionState); _physicalObjects.remove(motionState); - _outgoingChanges.remove(motionState); + removeOwnershipData(motionState); _entitiesToRemoveFromPhysics.insert(entity); if (entity->isMovingRelativeToParent()) { _simpleKinematicEntities.insert(entity); } } else { - _pendingChanges.insert(motionState); + _incomingChanges.insert(motionState); } } else if (entity->shouldBePhysical()) { // The intent is for this object to be in the PhysicsEngine, but it has no MotionState yet. @@ -136,10 +172,10 @@ void PhysicalEntitySimulation::clearEntitiesInternal() { _physicalObjects.clear(); // clear all other lists specific to this derived class + clearOwnershipData(); _entitiesToRemoveFromPhysics.clear(); _entitiesToAddToPhysics.clear(); - _pendingChanges.clear(); - _outgoingChanges.clear(); + _incomingChanges.clear(); } // virtual @@ -163,8 +199,8 @@ const VectorOfMotionStates& PhysicalEntitySimulation::getObjectsToRemoveFromPhys _deadEntities.insert(entity); } - _pendingChanges.remove(motionState); - _outgoingChanges.remove(motionState); + _incomingChanges.remove(motionState); + removeOwnershipData(motionState); _physicalObjects.remove(motionState); // remember this motionState and delete it later (after removing its RigidBody from the PhysicsEngine) @@ -232,18 +268,18 @@ void PhysicalEntitySimulation::getObjectsToAddToPhysics(VectorOfMotionStates& re void PhysicalEntitySimulation::setObjectsToChange(const VectorOfMotionStates& objectsToChange) { QMutexLocker lock(&_mutex); for (auto object : objectsToChange) { - _pendingChanges.insert(static_cast(object)); + _incomingChanges.insert(static_cast(object)); } } void PhysicalEntitySimulation::getObjectsToChange(VectorOfMotionStates& result) { result.clear(); QMutexLocker lock(&_mutex); - for (auto stateItr : _pendingChanges) { + for (auto stateItr : _incomingChanges) { EntityMotionState* motionState = &(*stateItr); result.push_back(motionState); } - _pendingChanges.clear(); + _incomingChanges.clear(); } void PhysicalEntitySimulation::handleDeactivatedMotionStates(const VectorOfMotionStates& motionStates) { @@ -263,18 +299,22 @@ void PhysicalEntitySimulation::handleChangedMotionStates(const VectorOfMotionSta PROFILE_RANGE_EX(simulation_physics, "ChangedEntities", 0x00000000, (uint64_t)motionStates.size()); QMutexLocker lock(&_mutex); - // walk the motionStates looking for those that correspond to entities - { - PROFILE_RANGE_EX(simulation_physics, "Filter", 0x00000000, (uint64_t)motionStates.size()); - for (auto stateItr : motionStates) { - ObjectMotionState* state = &(*stateItr); - assert(state); - if (state->getType() == MOTIONSTATE_TYPE_ENTITY) { - EntityMotionState* entityState = static_cast(state); - if (entityState->isCandidateForOwnership()) { - _outgoingChanges.insert(entityState); + for (auto stateItr : motionStates) { + ObjectMotionState* state = &(*stateItr); + assert(state); + if (state->getType() == MOTIONSTATE_TYPE_ENTITY) { + EntityMotionState* entityState = static_cast(state); + _entitiesToSort.insert(entityState->getEntity()); + if (entityState->getOwnershipState() == EntityMotionState::OwnershipState::NotLocallyOwned) { + // NOTE: entityState->getOwnershipState() reflects what ownership list (_bids or _owned) it is in + // and is distinct from entityState->isLocallyOwned() which checks the simulation ownership + // properties of the corresponding EntityItem. It is possible for the two states to be out + // of sync. In fact, we're trying to put them back into sync here. + if (entityState->isLocallyOwned()) { + addOwnership(entityState); + } else if (entityState->shouldSendBid()) { + addOwnershipBid(entityState); } - _entitiesToSort.insert(entityState->getEntity()); } } } @@ -284,26 +324,83 @@ void PhysicalEntitySimulation::handleChangedMotionStates(const VectorOfMotionSta _lastStepSendPackets = numSubsteps; if (Physics::getSessionUUID().isNull()) { - // usually don't get here, but if so --> nothing to do - _outgoingChanges.clear(); - return; + // usually don't get here, but if so clear all ownership + clearOwnershipData(); } + // send updates before bids, because this simplifies the logic thasuccessful bids will immediately send an update when added to the 'owned' list + sendOwnedUpdates(numSubsteps); + sendOwnershipBids(numSubsteps); + } +} - // look for entities to prune or update - PROFILE_RANGE_EX(simulation_physics, "Prune/Send", 0x00000000, (uint64_t)_outgoingChanges.size()); - QSet::iterator stateItr = _outgoingChanges.begin(); - while (stateItr != _outgoingChanges.end()) { - EntityMotionState* state = *stateItr; - if (!state->isCandidateForOwnership()) { - // prune - stateItr = _outgoingChanges.erase(stateItr); - } else if (state->shouldSendUpdate(numSubsteps)) { - // update - state->sendUpdate(_entityPacketSender, numSubsteps); - ++stateItr; - } else { - ++stateItr; +void PhysicalEntitySimulation::addOwnershipBid(EntityMotionState* motionState) { + motionState->initForBid(); + motionState->sendBid(_entityPacketSender, _physicsEngine->getNumSubsteps()); + _bids.push_back(motionState); + _nextBidExpiry = glm::min(_nextBidExpiry, motionState->getNextBidExpiry()); +} + +void PhysicalEntitySimulation::addOwnership(EntityMotionState* motionState) { + motionState->initForOwned(); + _owned.push_back(motionState); +} + +void PhysicalEntitySimulation::sendOwnershipBids(uint32_t numSubsteps) { + uint64_t now = usecTimestampNow(); + if (now > _nextBidExpiry) { + PROFILE_RANGE_EX(simulation_physics, "Bid", 0x00000000, (uint64_t)_bids.size()); + _nextBidExpiry = (uint64_t)(-1); + uint32_t i = 0; + while (i < _bids.size()) { + bool removeBid = false; + if (_bids[i]->isLocallyOwned()) { + // when an object transitions from 'bid' to 'owned' we are changing the "mode" of data stored + // in the EntityMotionState::_serverFoo variables (please see comments in EntityMotionState.h) + // therefore we need to immediately send an update so that the values stored are what we're + // "telling" the server rather than what we've been "hearing" from the server. + _bids[i]->sendUpdate(_entityPacketSender, numSubsteps); + + addOwnership(_bids[i]); + removeBid = true; + } else if (!_bids[i]->shouldSendBid()) { + removeBid = true; + _bids[i]->clearOwnershipState(); } + if (removeBid) { + if (i < _bids.size() - 1) { + _bids[i] = _bids.back(); + } + _bids.pop_back(); + } else { + if (now > _bids[i]->getNextBidExpiry()) { + _bids[i]->sendBid(_entityPacketSender, numSubsteps); + _nextBidExpiry = glm::min(_nextBidExpiry, _bids[i]->getNextBidExpiry()); + } + ++i; + } + } + } +} + +void PhysicalEntitySimulation::sendOwnedUpdates(uint32_t numSubsteps) { + PROFILE_RANGE_EX(simulation_physics, "Update", 0x00000000, (uint64_t)_owned.size()); + uint32_t i = 0; + while (i < _owned.size()) { + if (!_owned[i]->isLocallyOwned()) { + if (_owned[i]->shouldSendBid()) { + addOwnershipBid(_owned[i]); + } else { + _owned[i]->clearOwnershipState(); + } + if (i < _owned.size() - 1) { + _owned[i] = _owned.back(); + } + _owned.pop_back(); + } else { + if (_owned[i]->shouldSendUpdate(numSubsteps)) { + _owned[i]->sendUpdate(_entityPacketSender, numSubsteps); + } + ++i; } } } @@ -318,7 +415,6 @@ void PhysicalEntitySimulation::handleCollisionEvents(const CollisionEvents& coll } } - void PhysicalEntitySimulation::addDynamic(EntityDynamicPointer dynamic) { if (_physicsEngine) { // FIXME put fine grain locking into _physicsEngine diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index 90e4a79a18..72107fa278 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -45,12 +45,15 @@ signals: protected: // only called by EntitySimulation // overrides for EntitySimulation - virtual void updateEntitiesInternal(const quint64& now) override; + virtual void updateEntitiesInternal(const uint64_t& now) override; virtual void addEntityInternal(EntityItemPointer entity) override; virtual void removeEntityInternal(EntityItemPointer entity) override; virtual void changeEntityInternal(EntityItemPointer entity) override; virtual void clearEntitiesInternal() override; + void removeOwnershipData(EntityMotionState* motionState); + void clearOwnershipData(); + public: virtual void prepareEntityForDelete(EntityItemPointer entity) override; @@ -67,20 +70,28 @@ public: EntityEditPacketSender* getPacketSender() { return _entityPacketSender; } + void addOwnershipBid(EntityMotionState* motionState); + void addOwnership(EntityMotionState* motionState); + void sendOwnershipBids(uint32_t numSubsteps); + void sendOwnedUpdates(uint32_t numSubsteps); + private: SetOfEntities _entitiesToAddToPhysics; SetOfEntities _entitiesToRemoveFromPhysics; VectorOfMotionStates _objectsToDelete; - SetOfEntityMotionStates _pendingChanges; // EntityMotionStates already in PhysicsEngine that need their physics changed - SetOfEntityMotionStates _outgoingChanges; // EntityMotionStates for which we may need to send updates to entity-server + SetOfEntityMotionStates _incomingChanges; // EntityMotionStates that have changed from external sources + // and need their RigidBodies updated SetOfMotionStates _physicalObjects; // MotionStates of entities in PhysicsEngine PhysicsEnginePointer _physicsEngine = nullptr; EntityEditPacketSender* _entityPacketSender = nullptr; + std::vector _owned; + std::vector _bids; + uint64_t _nextBidExpiry; uint32_t _lastStepSendPackets { 0 }; }; diff --git a/libraries/physics/src/PhysicsEngine.cpp b/libraries/physics/src/PhysicsEngine.cpp index 2d84f0cef1..1d4c385f07 100644 --- a/libraries/physics/src/PhysicsEngine.cpp +++ b/libraries/physics/src/PhysicsEngine.cpp @@ -571,7 +571,7 @@ const CollisionEvents& PhysicsEngine::getCollisionEvents() { // modify the logic below. // // We only create events when at least one of the objects is (or should be) owned in the local simulation. - if (motionStateA && (motionStateA->shouldBeLocallyOwned())) { + if (motionStateA && (motionStateA->isLocallyOwnedOrShouldBe())) { QUuid idA = motionStateA->getObjectID(); QUuid idB; if (motionStateB) { @@ -582,7 +582,7 @@ const CollisionEvents& PhysicsEngine::getCollisionEvents() { (motionStateB ? motionStateB->getObjectLinearVelocityChange() : glm::vec3(0.0f)); glm::vec3 penetration = bulletToGLM(contact.distance * contact.normalWorldOnB); _collisionEvents.push_back(Collision(type, idA, idB, position, penetration, velocityChange)); - } else if (motionStateB && (motionStateB->shouldBeLocallyOwned())) { + } else if (motionStateB && (motionStateB->isLocallyOwnedOrShouldBe())) { QUuid idB = motionStateB->getObjectID(); QUuid idA; if (motionStateA) { From a924bd4eb07f0c67ce0389c9db0ed854450c22e7 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Thu, 29 Mar 2018 13:59:42 -0700 Subject: [PATCH 22/30] set correct _bidPriority on first ownership update --- libraries/physics/src/EntityMotionState.cpp | 4 ++++ libraries/physics/src/EntityMotionState.h | 4 ++++ libraries/physics/src/PhysicalEntitySimulation.cpp | 1 + 3 files changed, 9 insertions(+) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 9e891806f2..5285ec9a2e 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -710,6 +710,10 @@ uint8_t EntityMotionState::getSimulationPriority() const { return _entity->getSimulationPriority(); } +void EntityMotionState::slaveBidPriority() { + upgradeBidPriority(_entity->getSimulationPriority()); +} + // virtual QUuid EntityMotionState::getSimulatorID() const { assert(entityTreeIsLocked()); diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index b37fa9df68..978bafd539 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -101,6 +101,10 @@ protected: // changes _bidPriority only if priority is larger void upgradeBidPriority(uint8_t priority); + + // upgradeBidPriority to value stored in _entity + void slaveBidPriority(); + void clearObjectVelocities() const; #ifdef WANT_DEBUG_ENTITY_TREE_LOCKS diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 9129e29671..ddfd079362 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -358,6 +358,7 @@ void PhysicalEntitySimulation::sendOwnershipBids(uint32_t numSubsteps) { // in the EntityMotionState::_serverFoo variables (please see comments in EntityMotionState.h) // therefore we need to immediately send an update so that the values stored are what we're // "telling" the server rather than what we've been "hearing" from the server. + _bids[i]->slaveBidPriority(); _bids[i]->sendUpdate(_entityPacketSender, numSubsteps); addOwnership(_bids[i]); From 766077204ef7d317c5e95e1e5116224f20bc8e85 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 30 Mar 2018 16:35:16 -0700 Subject: [PATCH 23/30] fix typo inside assert() --- libraries/physics/src/EntityMotionState.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 5285ec9a2e..42c1213c41 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -550,7 +550,7 @@ void EntityMotionState::sendBid(OctreeEditPacketSender* packetSender, uint32_t s void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_t step) { DETAILED_PROFILE_RANGE(simulation_physics, "Send"); assert(entityTreeIsLocked()); - assert(_entity->isLocallyOwned()); + assert(isLocallyOwned()); if (!_body->isActive()) { // make sure all derivatives are zero From 59abc1d5b5ac2363990f6d74da7e2e9a36c23160 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Fri, 30 Mar 2018 17:04:31 -0700 Subject: [PATCH 24/30] DRY sendBid() and sendUpdate() --- libraries/physics/src/EntityMotionState.cpp | 47 ++++----------------- libraries/physics/src/EntityMotionState.h | 1 + 2 files changed, 10 insertions(+), 38 deletions(-) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 42c1213c41..3ca69080ec 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -453,10 +453,7 @@ bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep) { return remoteSimulationOutOfSync(simulationStep); } -void EntityMotionState::sendBid(OctreeEditPacketSender* packetSender, uint32_t step) { - DETAILED_PROFILE_RANGE(simulation_physics, "Bid"); - assert(entityTreeIsLocked()); - +void EntityMotionState::updateSendVelocities() { if (!_body->isActive()) { // make sure all derivatives are zero clearObjectVelocities(); @@ -491,6 +488,13 @@ void EntityMotionState::sendBid(OctreeEditPacketSender* packetSender, uint32_t s } _numInactiveUpdates = 0; } +} + +void EntityMotionState::sendBid(OctreeEditPacketSender* packetSender, uint32_t step) { + DETAILED_PROFILE_RANGE(simulation_physics, "Bid"); + assert(entityTreeIsLocked()); + + updateSendVelocities(); EntityItemProperties properties; Transform localTransform; @@ -552,40 +556,7 @@ void EntityMotionState::sendUpdate(OctreeEditPacketSender* packetSender, uint32_ assert(entityTreeIsLocked()); assert(isLocallyOwned()); - if (!_body->isActive()) { - // make sure all derivatives are zero - clearObjectVelocities(); - _numInactiveUpdates++; - } else { - glm::vec3 gravity = _entity->getGravity(); - - // if this entity has been accelerated at close to gravity for a certain number of simulation-steps, let - // the entity server's estimates include gravity. - const uint8_t STEPS_TO_DECIDE_BALLISTIC = 4; - if (_accelerationNearlyGravityCount >= STEPS_TO_DECIDE_BALLISTIC) { - _entity->setAcceleration(gravity); - } else { - _entity->setAcceleration(Vectors::ZERO); - } - - if (!_body->isStaticOrKinematicObject()) { - const float DYNAMIC_LINEAR_VELOCITY_THRESHOLD = 0.05f; // 5 cm/sec - const float DYNAMIC_ANGULAR_VELOCITY_THRESHOLD = 0.087266f; // ~5 deg/sec - - bool movingSlowlyLinear = - glm::length2(_entity->getWorldVelocity()) < (DYNAMIC_LINEAR_VELOCITY_THRESHOLD * DYNAMIC_LINEAR_VELOCITY_THRESHOLD); - bool movingSlowlyAngular = glm::length2(_entity->getWorldAngularVelocity()) < - (DYNAMIC_ANGULAR_VELOCITY_THRESHOLD * DYNAMIC_ANGULAR_VELOCITY_THRESHOLD); - bool movingSlowly = movingSlowlyLinear && movingSlowlyAngular && _entity->getAcceleration() == Vectors::ZERO; - - if (movingSlowly) { - // velocities might not be zero, but we'll fake them as such, which will hopefully help convince - // other simulating observers to deactivate their own copies - clearObjectVelocities(); - } - } - _numInactiveUpdates = 0; - } + updateSendVelocities(); // remember _serverFoo data for local prediction of server state Transform localTransform; diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index 978bafd539..e94b84572c 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -92,6 +92,7 @@ public: OwnershipState getOwnershipState() const { return _ownershipState; } protected: + void updateSendVelocities(); uint64_t getNextBidExpiry() const { return _nextBidExpiry; } void initForBid(); void initForOwned(); From 0a2b4a8d1da305bc7160d766947dcde5b444009d Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 3 Apr 2018 18:26:43 -0700 Subject: [PATCH 25/30] add comment about returning smart pointer by ref --- libraries/physics/src/EntityMotionState.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/libraries/physics/src/EntityMotionState.h b/libraries/physics/src/EntityMotionState.h index e94b84572c..807acbfe80 100644 --- a/libraries/physics/src/EntityMotionState.h +++ b/libraries/physics/src/EntityMotionState.h @@ -75,6 +75,8 @@ public: virtual QUuid getSimulatorID() const override; virtual void bump(uint8_t priority) override; + // getEntity() returns a smart-pointer by reference because it is only ever used + // to insert into lists of smart pointers, and the lists will make their own copies const EntityItemPointer& getEntity() const { return _entity; } void resetMeasuredBodyAcceleration(); From bd2bfb68076857e0cbbca8dd23726fc6ead109a1 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 3 Apr 2018 18:28:07 -0700 Subject: [PATCH 26/30] DRY: simpler removal of element from middle of std::vector --- .../physics/src/PhysicalEntitySimulation.cpp | 20 ++++--------------- .../physics/src/PhysicalEntitySimulation.h | 15 ++++++++++++-- 2 files changed, 17 insertions(+), 18 deletions(-) diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index ddfd079362..5b63a82ee2 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -79,20 +79,14 @@ void PhysicalEntitySimulation::removeOwnershipData(EntityMotionState* motionStat for (uint32_t i = 0; i < _owned.size(); ++i) { if (_owned[i] == motionState) { _owned[i]->clearOwnershipState(); - if (i < _owned.size() - 1) { - _owned[i] = _owned.back(); - } - _owned.pop_back(); + _owned.remove(i); } } } else if (motionState->getOwnershipState() == EntityMotionState::OwnershipState::PendingBid) { for (uint32_t i = 0; i < _bids.size(); ++i) { if (_bids[i] == motionState) { _bids[i]->clearOwnershipState(); - if (i < _bids.size() - 1) { - _bids[i] = _bids.back(); - } - _bids.pop_back(); + _bids.remove(i); } } } @@ -368,10 +362,7 @@ void PhysicalEntitySimulation::sendOwnershipBids(uint32_t numSubsteps) { _bids[i]->clearOwnershipState(); } if (removeBid) { - if (i < _bids.size() - 1) { - _bids[i] = _bids.back(); - } - _bids.pop_back(); + _bids.remove(i); } else { if (now > _bids[i]->getNextBidExpiry()) { _bids[i]->sendBid(_entityPacketSender, numSubsteps); @@ -393,10 +384,7 @@ void PhysicalEntitySimulation::sendOwnedUpdates(uint32_t numSubsteps) { } else { _owned[i]->clearOwnershipState(); } - if (i < _owned.size() - 1) { - _owned[i] = _owned.back(); - } - _owned.pop_back(); + _owned.remove(i); } else { if (_owned[i]->shouldSendUpdate(numSubsteps)) { _owned[i]->sendUpdate(_entityPacketSender, numSubsteps); diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index 72107fa278..bfb151bf23 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -27,6 +27,17 @@ class PhysicalEntitySimulation; using PhysicalEntitySimulationPointer = std::shared_ptr; using SetOfEntityMotionStates = QSet; +class VectorOfEntityMotionStates: public std::vector { +public: + void remove(uint32_t index) { + assert(index < size()); + if (index < size() - 1) { + (*this)[index] = back(); + } + pop_back(); + } +}; + class PhysicalEntitySimulation : public EntitySimulation { Q_OBJECT public: @@ -89,8 +100,8 @@ private: PhysicsEnginePointer _physicsEngine = nullptr; EntityEditPacketSender* _entityPacketSender = nullptr; - std::vector _owned; - std::vector _bids; + VectorOfEntityMotionStates _owned; + VectorOfEntityMotionStates _bids; uint64_t _nextBidExpiry; uint32_t _lastStepSendPackets { 0 }; }; From 70d6aa99e8282a5f69a35304333e02b5036db2b2 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 3 Apr 2018 21:55:31 -0700 Subject: [PATCH 27/30] more 'const uint64_t&' purges --- .../src/avatars/AvatarMixerClientData.cpp | 2 +- .../src/avatars/AvatarMixerClientData.h | 2 +- libraries/entities/src/EntitySimulation.cpp | 6 +++--- libraries/entities/src/EntitySimulation.h | 8 ++++---- libraries/entities/src/SimpleEntitySimulation.cpp | 2 +- libraries/entities/src/SimpleEntitySimulation.h | 2 +- libraries/entities/src/SimulationOwner.cpp | 8 ++++---- libraries/entities/src/SimulationOwner.h | 12 ++++++------ libraries/physics/src/PhysicalEntitySimulation.cpp | 2 +- libraries/physics/src/PhysicalEntitySimulation.h | 2 +- 10 files changed, 23 insertions(+), 23 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 288652715a..268aba62d6 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -33,7 +33,7 @@ uint64_t AvatarMixerClientData::getLastOtherAvatarEncodeTime(QUuid otherAvatar) return 0; } -void AvatarMixerClientData::setLastOtherAvatarEncodeTime(const QUuid& otherAvatar, const uint64_t& time) { +void AvatarMixerClientData::setLastOtherAvatarEncodeTime(const QUuid& otherAvatar, uint64_t time) { std::unordered_map::iterator itr = _lastOtherAvatarEncodeTime.find(otherAvatar); if (itr != _lastOtherAvatarEncodeTime.end()) { itr->second = time; diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 7a7210a0e8..3c2e660cbc 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -113,7 +113,7 @@ public: ViewFrustum getViewFrustum() const { return _currentViewFrustum; } uint64_t getLastOtherAvatarEncodeTime(QUuid otherAvatar) const; - void setLastOtherAvatarEncodeTime(const QUuid& otherAvatar, const uint64_t& time); + void setLastOtherAvatarEncodeTime(const QUuid& otherAvatar, uint64_t time); QVector& getLastOtherAvatarSentJoints(QUuid otherAvatar) { auto& lastOtherAvatarSentJoints = _lastOtherAvatarSentJoints[otherAvatar]; diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 82418e83e9..b850cee405 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -70,7 +70,7 @@ void EntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { } // protected -void EntitySimulation::expireMortalEntities(const uint64_t& now) { +void EntitySimulation::expireMortalEntities(uint64_t now) { if (now > _nextExpiry) { PROFILE_RANGE_EX(simulation_physics, "ExpireMortals", 0xffff00ff, (uint64_t)_mortalEntities.size()); // only search for expired entities if we expect to find one @@ -99,7 +99,7 @@ void EntitySimulation::expireMortalEntities(const uint64_t& now) { } // protected -void EntitySimulation::callUpdateOnEntitiesThatNeedIt(const uint64_t& now) { +void EntitySimulation::callUpdateOnEntitiesThatNeedIt(uint64_t now) { PerformanceTimer perfTimer("updatingEntities"); QMutexLocker lock(&_mutex); SetOfEntities::iterator itemItr = _entitiesToUpdate.begin(); @@ -231,7 +231,7 @@ void EntitySimulation::clearEntities() { _deadEntities.clear(); } -void EntitySimulation::moveSimpleKinematics(const uint64_t& now) { +void EntitySimulation::moveSimpleKinematics(uint64_t now) { PROFILE_RANGE_EX(simulation_physics, "MoveSimples", 0xffff00ff, (uint64_t)_simpleKinematicEntities.size()); SetOfEntities::iterator itemItr = _simpleKinematicEntities.begin(); while (itemItr != _simpleKinematicEntities.end()) { diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 19ab170ce5..1e4b5069be 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -71,7 +71,7 @@ public: void clearEntities(); - void moveSimpleKinematics(const uint64_t& now); + void moveSimpleKinematics(uint64_t now); EntityTreePointer getEntityTree() { return _entityTree; } @@ -83,14 +83,14 @@ public: 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(const uint64_t& now) = 0; + virtual void updateEntitiesInternal(uint64_t now) = 0; virtual void addEntityInternal(EntityItemPointer entity) = 0; virtual void removeEntityInternal(EntityItemPointer entity); virtual void changeEntityInternal(EntityItemPointer entity) = 0; virtual void clearEntitiesInternal() = 0; - void expireMortalEntities(const uint64_t& now); - void callUpdateOnEntitiesThatNeedIt(const uint64_t& now); + void expireMortalEntities(uint64_t now); + void callUpdateOnEntitiesThatNeedIt(uint64_t now); virtual void sortEntitiesThatMoved(); QMutex _mutex{ QMutex::Recursive }; diff --git a/libraries/entities/src/SimpleEntitySimulation.cpp b/libraries/entities/src/SimpleEntitySimulation.cpp index 266f10cf91..5d82d5ea40 100644 --- a/libraries/entities/src/SimpleEntitySimulation.cpp +++ b/libraries/entities/src/SimpleEntitySimulation.cpp @@ -50,7 +50,7 @@ void SimpleEntitySimulation::clearOwnership(const QUuid& ownerID) { } } -void SimpleEntitySimulation::updateEntitiesInternal(const uint64_t& now) { +void SimpleEntitySimulation::updateEntitiesInternal(uint64_t now) { if (now > _nextOwnerlessExpiry) { // search for ownerless objects that have expired QMutexLocker lock(&_mutex); diff --git a/libraries/entities/src/SimpleEntitySimulation.h b/libraries/entities/src/SimpleEntitySimulation.h index 10714be299..95c996a920 100644 --- a/libraries/entities/src/SimpleEntitySimulation.h +++ b/libraries/entities/src/SimpleEntitySimulation.h @@ -28,7 +28,7 @@ public: void clearOwnership(const QUuid& ownerID); protected: - virtual void updateEntitiesInternal(const uint64_t& now) override; + virtual void updateEntitiesInternal(uint64_t now) override; virtual void addEntityInternal(EntityItemPointer entity) override; virtual void removeEntityInternal(EntityItemPointer entity) override; virtual void changeEntityInternal(EntityItemPointer entity) override; diff --git a/libraries/entities/src/SimulationOwner.cpp b/libraries/entities/src/SimulationOwner.cpp index 50cbeddef8..b312c6deb9 100644 --- a/libraries/entities/src/SimulationOwner.cpp +++ b/libraries/entities/src/SimulationOwner.cpp @@ -101,22 +101,22 @@ bool SimulationOwner::set(const SimulationOwner& owner) { return setID(owner._id) || oldPriority != _priority; } -void SimulationOwner::setPendingPriority(uint8_t priority, const quint64& timestamp) { +void SimulationOwner::setPendingPriority(uint8_t priority, uint64_t timestamp) { _pendingBidPriority = priority; _pendingBidTimestamp = timestamp; _pendingState = (_pendingBidPriority == 0) ? PENDING_STATE_RELEASE : PENDING_STATE_TAKE; } void SimulationOwner::updateExpiry() { - const quint64 OWNERSHIP_LOCKOUT_EXPIRY = 200 * USECS_PER_MSEC; + const uint64_t OWNERSHIP_LOCKOUT_EXPIRY = 200 * USECS_PER_MSEC; _expiry = usecTimestampNow() + OWNERSHIP_LOCKOUT_EXPIRY; } -bool SimulationOwner::pendingRelease(const quint64& timestamp) { +bool SimulationOwner::pendingRelease(uint64_t timestamp) { return _pendingBidPriority == 0 && _pendingState == PENDING_STATE_RELEASE && _pendingBidTimestamp >= timestamp; } -bool SimulationOwner::pendingTake(const quint64& timestamp) { +bool SimulationOwner::pendingTake(uint64_t timestamp) { return _pendingBidPriority > 0 && _pendingState == PENDING_STATE_TAKE && _pendingBidTimestamp >= timestamp; } diff --git a/libraries/entities/src/SimulationOwner.h b/libraries/entities/src/SimulationOwner.h index 223232a4b6..cc2069dcc8 100644 --- a/libraries/entities/src/SimulationOwner.h +++ b/libraries/entities/src/SimulationOwner.h @@ -110,7 +110,7 @@ public: SimulationOwner(const QUuid& id, uint8_t priority); const QUuid& getID() const { return _id; } - const quint64& getExpiry() const { return _expiry; } + const uint64_t& getExpiry() const { return _expiry; } uint8_t getPriority() const { return _priority; } QByteArray toByteArray() const; @@ -125,7 +125,7 @@ public: bool setID(const QUuid& id); bool set(const QUuid& id, uint8_t priority); bool set(const SimulationOwner& owner); - void setPendingPriority(uint8_t priority, const quint64& timestamp); + void setPendingPriority(uint8_t priority, uint64_t timestamp); bool isNull() const { return _id.isNull(); } bool matchesValidID(const QUuid& id) const { return _id == id && !_id.isNull(); } @@ -135,8 +135,8 @@ public: bool hasExpired() const { return usecTimestampNow() > _expiry; } uint8_t getPendingPriority() const { return _pendingBidPriority; } - bool pendingRelease(const quint64& timestamp); // return true if valid pending RELEASE - bool pendingTake(const quint64& timestamp); // return true if valid pending TAKE + bool pendingRelease(uint64_t timestamp); // return true if valid pending RELEASE + bool pendingTake(uint64_t timestamp); // return true if valid pending TAKE void clearCurrentOwner(); bool operator>=(uint8_t priority) const { return _priority >= priority; } @@ -152,8 +152,8 @@ public: private: QUuid _id; // owner - quint64 _expiry; // time when ownership can transition at equal priority - quint64 _pendingBidTimestamp; // time when pending bid was set + uint64_t _expiry; // time when ownership can transition at equal priority + uint64_t _pendingBidTimestamp; // time when pending bid was set uint8_t _priority; // priority of current owner uint8_t _pendingBidPriority; // priority at which we'd like to own it uint8_t _pendingState; // NOTHING, TAKE, or RELEASE diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 5b63a82ee2..3883054e1e 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -40,7 +40,7 @@ void PhysicalEntitySimulation::init( } // begin EntitySimulation overrides -void PhysicalEntitySimulation::updateEntitiesInternal(const uint64_t& now) { +void PhysicalEntitySimulation::updateEntitiesInternal(uint64_t now) { // Do nothing here because the "internal" update the PhysicsEngine::stepSimualtion() which is done elsewhere. } diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index bfb151bf23..7b6fe221fb 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -56,7 +56,7 @@ signals: protected: // only called by EntitySimulation // overrides for EntitySimulation - virtual void updateEntitiesInternal(const uint64_t& now) override; + virtual void updateEntitiesInternal(uint64_t now) override; virtual void addEntityInternal(EntityItemPointer entity) override; virtual void removeEntityInternal(EntityItemPointer entity) override; virtual void changeEntityInternal(EntityItemPointer entity) override; From 6d7574cab9fef6e2faacde4d33ccfff96b53f475 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 3 Apr 2018 22:13:25 -0700 Subject: [PATCH 28/30] use std::numeric_limtis::max() instead of -1 --- libraries/entities/src/EntitySimulation.cpp | 6 +++--- libraries/entities/src/EntitySimulation.h | 4 +++- libraries/entities/src/SimpleEntitySimulation.cpp | 2 +- libraries/physics/src/PhysicalEntitySimulation.cpp | 2 +- 4 files changed, 8 insertions(+), 6 deletions(-) diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index b850cee405..d034ddedbe 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -20,7 +20,7 @@ void EntitySimulation::setEntityTree(EntityTreePointer tree) { if (_entityTree && _entityTree != tree) { _mortalEntities.clear(); - _nextExpiry = uint64_t(-1); + _nextExpiry = std::numeric_limits::max(); _entitiesToUpdate.clear(); _entitiesToSort.clear(); _simpleKinematicEntities.clear(); @@ -74,7 +74,7 @@ void EntitySimulation::expireMortalEntities(uint64_t now) { if (now > _nextExpiry) { PROFILE_RANGE_EX(simulation_physics, "ExpireMortals", 0xffff00ff, (uint64_t)_mortalEntities.size()); // only search for expired entities if we expect to find one - _nextExpiry = uint64_t(-1); + _nextExpiry = std::numeric_limits::max(); QMutexLocker lock(&_mutex); SetOfEntities::iterator itemItr = _mortalEntities.begin(); while (itemItr != _mortalEntities.end()) { @@ -220,7 +220,7 @@ void EntitySimulation::changeEntity(EntityItemPointer entity) { void EntitySimulation::clearEntities() { QMutexLocker lock(&_mutex); _mortalEntities.clear(); - _nextExpiry = uint64_t(-1); + _nextExpiry = std::numeric_limits::max(); _entitiesToUpdate.clear(); _entitiesToSort.clear(); _simpleKinematicEntities.clear(); diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 1e4b5069be..b19e1c33d3 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -12,6 +12,8 @@ #ifndef hifi_EntitySimulation_h #define hifi_EntitySimulation_h +#include + #include #include #include @@ -44,7 +46,7 @@ const int DIRTY_SIMULATION_FLAGS = class EntitySimulation : public QObject, public std::enable_shared_from_this { public: - EntitySimulation() : _mutex(QMutex::Recursive), _entityTree(NULL), _nextExpiry(uint64_t(-1)) { } + EntitySimulation() : _mutex(QMutex::Recursive), _entityTree(NULL), _nextExpiry(std::numeric_limits::max()) { } virtual ~EntitySimulation() { setEntityTree(NULL); } inline EntitySimulationPointer getThisPointer() const { diff --git a/libraries/entities/src/SimpleEntitySimulation.cpp b/libraries/entities/src/SimpleEntitySimulation.cpp index 5d82d5ea40..a2aba0169d 100644 --- a/libraries/entities/src/SimpleEntitySimulation.cpp +++ b/libraries/entities/src/SimpleEntitySimulation.cpp @@ -54,7 +54,7 @@ void SimpleEntitySimulation::updateEntitiesInternal(uint64_t now) { if (now > _nextOwnerlessExpiry) { // search for ownerless objects that have expired QMutexLocker lock(&_mutex); - _nextOwnerlessExpiry = -1; + _nextOwnerlessExpiry = std::numeric_limits::max(); SetOfEntities::iterator itemItr = _entitiesThatNeedSimulationOwner.begin(); while (itemItr != _entitiesThatNeedSimulationOwner.end()) { EntityItemPointer entity = *itemItr; diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 3883054e1e..d799577fc2 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -343,7 +343,7 @@ void PhysicalEntitySimulation::sendOwnershipBids(uint32_t numSubsteps) { uint64_t now = usecTimestampNow(); if (now > _nextBidExpiry) { PROFILE_RANGE_EX(simulation_physics, "Bid", 0x00000000, (uint64_t)_bids.size()); - _nextBidExpiry = (uint64_t)(-1); + _nextBidExpiry = std::numeric_limits::max(); uint32_t i = 0; while (i < _bids.size()) { bool removeBid = false; From d57cb0b5d7ad0366b58cb1e75d47935d7ddb16c4 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 3 Apr 2018 22:15:57 -0700 Subject: [PATCH 29/30] use list.empty() instead of comparing list.size() --- libraries/entities/src/EntityTree.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 232880880e..15dacf0d9c 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -648,7 +648,7 @@ void EntityTree::deleteEntities(QSet entityIDs, bool force, bool i emit deletingEntityPointer(existingEntity.get()); } - if (theOperator.getEntities().size() > 0) { + if (!theOperator.getEntities().empty()) { recurseTreeWithOperator(&theOperator); processRemovedEntities(theOperator); _isDirty = true; @@ -1802,7 +1802,7 @@ void EntityTree::update(bool simulate) { PROFILE_RANGE(simulation_physics, "Deletes"); SetOfEntities deadEntities; _simulation->takeDeadEntities(deadEntities); - if (deadEntities.size() > 0) { + if (!deadEntities.empty()) { // translate into list of ID's QSet idsToDelete; From f0fcfc432c9dbc3bd9870fd24a8523a8a22c24a0 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Tue, 3 Apr 2018 22:16:10 -0700 Subject: [PATCH 30/30] fix typo in comment --- libraries/physics/src/EntityMotionState.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 3ca69080ec..6a25700ce8 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -537,7 +537,7 @@ void EntityMotionState::sendBid(OctreeEditPacketSender* packetSender, uint32_t s EntityItemID id(_entity->getID()); EntityEditPacketSender* entityPacketSender = static_cast(packetSender); entityPacketSender->queueEditEntityMessage(PacketType::EntityPhysics, tree, id, properties); - _entity->setLastBroadcast(now); // ffor debug/physics status icons + _entity->setLastBroadcast(now); // for debug/physics status icons // NOTE: we don't descend to children for ownership bid. Instead, if we win ownership of the parent // then in sendUpdate() we'll walk descendents and send updates for their QueryAACubes if necessary.