map EntityItemID to EntityItemPointer

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 {

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++;
             }
         }

libraries/entities/src/EntityTree.cpp

@@ -91,11 +91,14 @@ void EntityTree::eraseAllOctreeElements(bool createNewRoot) {
         _simulation->clearEntities();
     }
     {
-        QWriteLocker locker(&_entityToElementLock);
+        QWriteLocker locker(&_entityMapLock);
-        foreach(EntityTreeElementPointer element, _entityToElementMap) {
+        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 (!entity->getElement()) {
-    if (!containingElement) {
         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);
+    QReadLocker locker(&_entityMapLock);
-    EntityTreeElementPointer element = _entityToElementMap.value(entityItemID);
+    EntityItemPointer entity = _entityMap.value(entityItemID);
-    return element;
+    if (entity) {
+        return entity->getElement();
+    }
+    return EntityTreeElementPointer(nullptr);
 }
 
-void EntityTree::setContainingElement(const EntityItemID& entityItemID, EntityTreeElementPointer element) {
+void EntityTree::addEntityMapEntry(EntityItemPointer entity) {
-    QWriteLocker locker(&_entityToElementLock);
+    EntityItemID id = entity->getEntityItemID();
-    if (element) {
+    QWriteLocker locker(&_entityMapLock);
-        _entityToElementMap[entityItemID] = element;
+    EntityItemPointer otherEntity = _entityMap.value(id);
-    } else {
+    if (otherEntity) {
-        _entityToElementMap.remove(entityItemID);
+        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);
+    QReadLocker locker(&_entityMapLock);
-    QHashIterator<EntityItemID, EntityTreeElementPointer> i(_entityToElementMap);
+    QHashIterator<EntityItemID, EntityItemPointer> i(_entityMap);
     while (i.hasNext()) {
         i.next();
-        qCDebug(entities) << i.key() << ": " << i.value().get();
+        qCDebug(entities) << i.key() << ": " << i.value()->getElement().get();
     }
     qCDebug(entities) << "-----------------------------------------------------";
 }

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,8 +273,7 @@ signals:
 protected:
 
     void processRemovedEntities(const DeleteEntityOperator& theOperator);
-    bool updateEntityWithElement(EntityItemPointer entity, const EntityItemProperties& properties,
+    bool updateEntity(EntityItemPointer entity, const EntityItemProperties& properties,
-                                 EntityTreeElementPointer containingElement,
             const SharedNodePointer& senderNode = SharedNodePointer(nullptr));
     static bool findNearPointOperation(const OctreeElementPointer& element, void* extraData);
     static bool findInSphereOperation(const OctreeElementPointer& element, void* extraData);
@@ -309,8 +306,8 @@ protected:
         _deletedEntityItemIDs << id;
     }
 
-    mutable QReadWriteLock _entityToElementLock;
+    mutable QReadWriteLock _entityMapLock;
-    QHash<EntityItemID, EntityTreeElementPointer> _entityToElementMap;
+    QHash<EntityItemID, EntityItemPointer> _entityMap;
 
     EntitySimulationPointer _simulation;
 

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();

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

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
+    // If we haven't yet found the old element, and this subTreeContains our old element,
-    // entity, then we need to keep searching.
+    // 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
+    // If we haven't yet found the new element, and this subTreeContains our new element,
-    // entity, then we need to keep searching.
+    // 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