From 7ea1e7285f0e6697309d34db73e37e48a6fb6af1 Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 28 Jun 2017 14:59:46 -0700 Subject: [PATCH] map EntityItemID to EntityItemPointer --- libraries/entities/src/AddEntityOperator.cpp | 4 +- .../entities/src/DeleteEntityOperator.cpp | 2 +- libraries/entities/src/EntityTree.cpp | 82 ++++++++++--------- libraries/entities/src/EntityTree.h | 15 ++-- libraries/entities/src/EntityTreeElement.cpp | 3 +- .../entities/src/MovingEntitiesOperator.cpp | 3 +- .../entities/src/UpdateEntityOperator.cpp | 10 +-- 7 files changed, 59 insertions(+), 60 deletions(-) diff --git a/libraries/entities/src/AddEntityOperator.cpp b/libraries/entities/src/AddEntityOperator.cpp index e86e70dd80..78d986f538 100644 --- a/libraries/entities/src/AddEntityOperator.cpp +++ b/libraries/entities/src/AddEntityOperator.cpp @@ -46,10 +46,8 @@ bool AddEntityOperator::preRecursion(const OctreeElementPointer& element) { // If this element is the best fit for the new entity properties, then add/or update it if (entityTreeElement->bestFitBounds(_newEntityBox)) { - + _tree->addEntityMapEntry(_newEntity); entityTreeElement->addEntityItem(_newEntity); - _tree->setContainingElement(_newEntity->getEntityItemID(), entityTreeElement); - _foundNew = true; keepSearching = false; } else { diff --git a/libraries/entities/src/DeleteEntityOperator.cpp b/libraries/entities/src/DeleteEntityOperator.cpp index 709c281341..cbd0ad7839 100644 --- a/libraries/entities/src/DeleteEntityOperator.cpp +++ b/libraries/entities/src/DeleteEntityOperator.cpp @@ -96,7 +96,7 @@ bool DeleteEntityOperator::preRecursion(const OctreeElementPointer& element) { bool entityDeleted = entityTreeElement->removeEntityItem(theEntity); // remove it from the element assert(entityDeleted); (void)entityDeleted; // quite warning - _tree->setContainingElement(details.entity->getEntityItemID(), NULL); // update or id to element lookup + _tree->clearEntityMapEntry(details.entity->getEntityItemID()); _foundCount++; } } diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 4773f45af7..31b96ebfc4 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -91,11 +91,14 @@ void EntityTree::eraseAllOctreeElements(bool createNewRoot) { _simulation->clearEntities(); } { - QWriteLocker locker(&_entityToElementLock); - foreach(EntityTreeElementPointer element, _entityToElementMap) { - element->cleanupEntities(); + QWriteLocker locker(&_entityMapLock); + foreach(EntityItemPointer entity, _entityMap) { + EntityTreeElementPointer element = entity->getElement(); + if (element) { + element->cleanupEntities(); + } } - _entityToElementMap.clear(); + _entityMap.clear(); } Octree::eraseAllOctreeElements(createNewRoot); @@ -136,29 +139,24 @@ void EntityTree::postAddEntity(EntityItemPointer entity) { } bool EntityTree::updateEntity(const EntityItemID& entityID, const EntityItemProperties& properties, const SharedNodePointer& senderNode) { - EntityTreeElementPointer containingElement = getContainingElement(entityID); + EntityItemPointer entity; + { + QReadLocker locker(&_entityMapLock); + entity = _entityMap.value(entityID); + } + if (!entity) { + return false; + } + return updateEntity(entity, properties, senderNode); +} + +bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperties& origProperties, + const SharedNodePointer& senderNode) { + EntityTreeElementPointer containingElement = entity->getElement(); if (!containingElement) { return false; } - EntityItemPointer existingEntity = containingElement->getEntityWithEntityItemID(entityID); - if (!existingEntity) { - return false; - } - - return updateEntityWithElement(existingEntity, properties, containingElement, senderNode); -} - -bool EntityTree::updateEntity(EntityItemPointer entity, const EntityItemProperties& properties, const SharedNodePointer& senderNode) { - EntityTreeElementPointer containingElement = getContainingElement(entity->getEntityItemID()); - if (!containingElement) { - return false; - } - return updateEntityWithElement(entity, properties, containingElement, senderNode); -} - -bool EntityTree::updateEntityWithElement(EntityItemPointer entity, const EntityItemProperties& origProperties, - EntityTreeElementPointer containingElement, const SharedNodePointer& senderNode) { EntityItemProperties properties = origProperties; bool allowLockChange; @@ -331,8 +329,7 @@ bool EntityTree::updateEntityWithElement(EntityItemPointer entity, const EntityI } // TODO: this final containingElement check should eventually be removed (or wrapped in an #ifdef DEBUG). - containingElement = getContainingElement(entity->getEntityItemID()); - if (!containingElement) { + if (!entity->getElement()) { qCDebug(entities) << "UNEXPECTED!!!! after updateEntity() we no longer have a containing element??? entityID=" << entity->getEntityItemID(); return false; @@ -1502,27 +1499,38 @@ int EntityTree::processEraseMessageDetails(const QByteArray& dataByteArray, cons } EntityTreeElementPointer EntityTree::getContainingElement(const EntityItemID& entityItemID) /*const*/ { - QReadLocker locker(&_entityToElementLock); - EntityTreeElementPointer element = _entityToElementMap.value(entityItemID); - return element; + QReadLocker locker(&_entityMapLock); + EntityItemPointer entity = _entityMap.value(entityItemID); + if (entity) { + return entity->getElement(); + } + return EntityTreeElementPointer(nullptr); } -void EntityTree::setContainingElement(const EntityItemID& entityItemID, EntityTreeElementPointer element) { - QWriteLocker locker(&_entityToElementLock); - if (element) { - _entityToElementMap[entityItemID] = element; - } else { - _entityToElementMap.remove(entityItemID); +void EntityTree::addEntityMapEntry(EntityItemPointer entity) { + EntityItemID id = entity->getEntityItemID(); + QWriteLocker locker(&_entityMapLock); + EntityItemPointer otherEntity = _entityMap.value(id); + if (otherEntity) { + qCDebug(entities) << "EntityTree::addEntityMapEntry() found pre-existing id " << id; + assert(false); + return; } + _entityMap.insert(id, entity); +} + +void EntityTree::clearEntityMapEntry(const EntityItemID& id) { + QWriteLocker locker(&_entityMapLock); + _entityMap.remove(id); } void EntityTree::debugDumpMap() { qCDebug(entities) << "EntityTree::debugDumpMap() --------------------------"; - QReadLocker locker(&_entityToElementLock); - QHashIterator i(_entityToElementMap); + QReadLocker locker(&_entityMapLock); + QHashIterator i(_entityMap); while (i.hasNext()) { i.next(); - qCDebug(entities) << i.key() << ": " << i.value().get(); + qCDebug(entities) << i.key() << ": " << i.value()->getElement().get(); } qCDebug(entities) << "-----------------------------------------------------"; } diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 24e6c364b1..efb8cf81ba 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -119,9 +119,6 @@ public: // use this method if you only know the entityID bool updateEntity(const EntityItemID& entityID, const EntityItemProperties& properties, const SharedNodePointer& senderNode = SharedNodePointer(nullptr)); - // use this method if you have a pointer to the entity (avoid an extra entity lookup) - bool updateEntity(EntityItemPointer entity, const EntityItemProperties& properties, const SharedNodePointer& senderNode = SharedNodePointer(nullptr)); - // check if the avatar is a child of this entity, If so set the avatar parentID to null void unhookChildAvatar(const EntityItemID entityID); void deleteEntity(const EntityItemID& entityID, bool force = false, bool ignoreWarnings = true); @@ -183,7 +180,8 @@ public: int processEraseMessageDetails(const QByteArray& buffer, const SharedNodePointer& sourceNode); EntityTreeElementPointer getContainingElement(const EntityItemID& entityItemID) /*const*/; - void setContainingElement(const EntityItemID& entityItemID, EntityTreeElementPointer element); + void addEntityMapEntry(EntityItemPointer entity); + void clearEntityMapEntry(const EntityItemID& id); void debugDumpMap(); virtual void dumpTree() override; virtual void pruneTree() override; @@ -275,9 +273,8 @@ signals: protected: void processRemovedEntities(const DeleteEntityOperator& theOperator); - bool updateEntityWithElement(EntityItemPointer entity, const EntityItemProperties& properties, - EntityTreeElementPointer containingElement, - const SharedNodePointer& senderNode = SharedNodePointer(nullptr)); + bool updateEntity(EntityItemPointer entity, const EntityItemProperties& properties, + const SharedNodePointer& senderNode = SharedNodePointer(nullptr)); static bool findNearPointOperation(const OctreeElementPointer& element, void* extraData); static bool findInSphereOperation(const OctreeElementPointer& element, void* extraData); static bool findInCubeOperation(const OctreeElementPointer& element, void* extraData); @@ -309,8 +306,8 @@ protected: _deletedEntityItemIDs << id; } - mutable QReadWriteLock _entityToElementLock; - QHash _entityToElementMap; + mutable QReadWriteLock _entityMapLock; + QHash _entityMap; EntitySimulationPointer _simulation; diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index cce7ee006f..4177724d1e 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -1001,7 +1001,6 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int if (currentContainingElement.get() != this) { currentContainingElement->removeEntityItem(entityItem); addEntityItem(entityItem); - _myTree->setContainingElement(entityItemID, getThisPointer()); } } } @@ -1032,9 +1031,9 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int // don't add if we've recently deleted.... if (!_myTree->isDeletedEntity(entityItem->getID())) { + _myTree->addEntityMapEntry(entityItem); addEntityItem(entityItem); // add this new entity to this elements entities entityItemID = entityItem->getEntityItemID(); - _myTree->setContainingElement(entityItemID, getThisPointer()); _myTree->postAddEntity(entityItem); if (entityItem->getCreated() == UNKNOWN_CREATED_TIME) { entityItem->recordCreationTime(); diff --git a/libraries/entities/src/MovingEntitiesOperator.cpp b/libraries/entities/src/MovingEntitiesOperator.cpp index ab97c67aa2..b9d5b681f6 100644 --- a/libraries/entities/src/MovingEntitiesOperator.cpp +++ b/libraries/entities/src/MovingEntitiesOperator.cpp @@ -51,7 +51,7 @@ MovingEntitiesOperator::~MovingEntitiesOperator() { void MovingEntitiesOperator::addEntityToMoveList(EntityItemPointer entity, const AACube& newCube) { - EntityTreeElementPointer oldContainingElement = _tree->getContainingElement(entity->getEntityItemID()); + EntityTreeElementPointer oldContainingElement = entity->getElement(); AABox newCubeClamped = newCube.clamp((float)-HALF_TREE_SCALE, (float)HALF_TREE_SCALE); if (_wantDebug) { @@ -201,7 +201,6 @@ bool MovingEntitiesOperator::preRecursion(const OctreeElementPointer& element) { oldElement->removeEntityItem(details.entity); } entityTreeElement->addEntityItem(details.entity); - _tree->setContainingElement(entityItemID, entityTreeElement); } _foundNewCount++; //details.newFound = true; // TODO: would be nice to add this optimization diff --git a/libraries/entities/src/UpdateEntityOperator.cpp b/libraries/entities/src/UpdateEntityOperator.cpp index ec6051af04..7a5c87187a 100644 --- a/libraries/entities/src/UpdateEntityOperator.cpp +++ b/libraries/entities/src/UpdateEntityOperator.cpp @@ -138,8 +138,8 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) { qCDebug(entities) << " _foundNew=" << _foundNew; } - // If we haven't yet found the old entity, and this subTreeContains our old - // entity, then we need to keep searching. + // If we haven't yet found the old element, and this subTreeContains our old element, + // then we need to keep searching. if (!_foundOld && subtreeContainsOld) { if (_wantDebug) { @@ -169,7 +169,6 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) { // NOTE: we know we haven't yet added it to its new element because _removeOld is true EntityTreeElementPointer oldElement = _existingEntity->getElement(); oldElement->removeEntityItem(_existingEntity); - _tree->setContainingElement(_entityItemID, NULL); if (oldElement != _containingElement) { qCDebug(entities) << "WARNING entity moved during UpdateEntityOperator recursion"; @@ -187,8 +186,8 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) { } } - // If we haven't yet found the new entity, and this subTreeContains our new - // entity, then we need to keep searching. + // If we haven't yet found the new element, and this subTreeContains our new element, + // then we need to keep searching. if (!_foundNew && subtreeContainsNew) { if (_wantDebug) { @@ -221,7 +220,6 @@ bool UpdateEntityOperator::preRecursion(const OctreeElementPointer& element) { } } entityTreeElement->addEntityItem(_existingEntity); - _tree->setContainingElement(_entityItemID, entityTreeElement); } _foundNew = true; // we found the new element _removeOld = false; // and it has already been removed from the old