From 2f3cf82202b3418456916ae22d8616242770cc8e Mon Sep 17 00:00:00 2001 From: Brad Hefta-Gaub Date: Wed, 18 Nov 2015 14:22:02 -0800 Subject: [PATCH] don't apply out of order edits to entities that have been deleted --- libraries/entities/src/EntityItem.cpp | 9 +++++++ libraries/entities/src/EntityTree.cpp | 4 ++++ libraries/entities/src/EntityTree.h | 25 +++++++++++++++++--- libraries/entities/src/EntityTreeElement.cpp | 19 ++++++++++----- 4 files changed, 48 insertions(+), 9 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 100f6dfe22..75bc26a560 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -500,6 +500,15 @@ int EntityItem::readEntityDataFromBuffer(const unsigned char* data, int bytesLef } } + // before proceeding, check to see if this is an entity that we know has been deleted, which + // might happen in the case of out-of-order and/or recorvered packets, if we've deleted the entity + // we can confidently ignore this packet + EntityTreePointer tree = getTree(); + if (tree && tree->isDeletedEntity(_id)) { + qDebug() << "Recieved packet for previously deleted entity [" << _id << "] ignoring. (inside " << __FUNCTION__ << ")"; + ignoreServerPacket = true; + } + if (ignoreServerPacket) { overwriteLocalData = false; #ifdef WANT_DEBUG diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index dc7c19056a..9eac14e4b4 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -68,6 +68,7 @@ void EntityTree::eraseAllOctreeElements(bool createNewRoot) { Octree::eraseAllOctreeElements(createNewRoot); resetClientEditStats(); + clearDeletedEntities(); } bool EntityTree::handlesEditPacketType(PacketType packetType) const { @@ -398,6 +399,9 @@ void EntityTree::processRemovedEntities(const DeleteEntityOperator& theOperator) // set up the deleted entities ID QWriteLocker locker(&_recentlyDeletedEntitiesLock); _recentlyDeletedEntityItemIDs.insert(deletedAt, theEntity->getEntityItemID()); + } else { + // on the client side, we also remember that we deleted this entity, we don't care about the time + trackDeletedEntity(theEntity->getEntityItemID()); } if (_simulation) { diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 645e3f4f76..d1e0462f64 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -228,6 +228,11 @@ public: EntityTreePointer getThisPointer() { return std::static_pointer_cast(shared_from_this()); } + bool isDeletedEntity(const QUuid& id) { + QReadLocker locker(&_deletedEntitiesLock); + return _deletedEntityItemIDs.contains(id); + } + signals: void deletingEntity(const EntityItemID& entityID); void addingEntity(const EntityItemID& entityID); @@ -235,7 +240,7 @@ signals: void newCollisionSoundURL(const QUrl& url); void clearingEntities(); -private: +protected: void processRemovedEntities(const DeleteEntityOperator& theOperator); bool updateEntityWithElement(EntityItemPointer entity, const EntityItemProperties& properties, @@ -252,8 +257,22 @@ private: QReadWriteLock _newlyCreatedHooksLock; QVector _newlyCreatedHooks; - mutable QReadWriteLock _recentlyDeletedEntitiesLock; - QMultiMap _recentlyDeletedEntityItemIDs; + mutable QReadWriteLock _recentlyDeletedEntitiesLock; /// lock of server side recent deletes + QMultiMap _recentlyDeletedEntityItemIDs; /// server side recent deletes + + mutable QReadWriteLock _deletedEntitiesLock; /// lock of client side recent deletes + QSet _deletedEntityItemIDs; /// client side recent deletes + + void clearDeletedEntities() { + QWriteLocker locker(&_deletedEntitiesLock); + _deletedEntityItemIDs.clear(); + } + + void trackDeletedEntity(const QUuid& id) { + QWriteLocker locker(&_deletedEntitiesLock); + _deletedEntityItemIDs << id; + } + EntityItemFBXService* _fbxService; QHash _entityToElementMap; diff --git a/libraries/entities/src/EntityTreeElement.cpp b/libraries/entities/src/EntityTreeElement.cpp index 7ada138d02..02552ef488 100644 --- a/libraries/entities/src/EntityTreeElement.cpp +++ b/libraries/entities/src/EntityTreeElement.cpp @@ -894,12 +894,19 @@ int EntityTreeElement::readElementDataFromBuffer(const unsigned char* data, int entityItem = EntityTypes::constructEntityItem(dataAt, bytesLeftToRead, args); if (entityItem) { bytesForThisEntity = entityItem->readEntityDataFromBuffer(dataAt, bytesLeftToRead, args); - 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(); + + // don't add if we've recently deleted.... + if (!_myTree->isDeletedEntity(entityItem->getID())) { + 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(); + } + } else { + qDebug() << "Recieved packet for previously deleted entity [" << + entityItem->getID() << "] ignoring. (inside " << __FUNCTION__ << ")"; } } }