From 4159bc4862c5c9a165b546b0cb1fd972c281d37b Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Wed, 16 Oct 2019 08:55:02 -0700 Subject: [PATCH] abide by domain-entity hierarchy deletion rules --- libraries/entities/src/EntityItem.cpp | 6 ++---- libraries/entities/src/EntityItem.h | 2 +- .../entities/src/EntityScriptingInterface.cpp | 13 +++---------- libraries/entities/src/EntitySimulation.cpp | 6 +----- libraries/entities/src/EntitySimulation.h | 2 +- libraries/entities/src/EntityTree.cpp | 17 +++++------------ .../physics/src/PhysicalEntitySimulation.cpp | 17 +++++------------ .../physics/src/PhysicalEntitySimulation.h | 2 +- 8 files changed, 19 insertions(+), 46 deletions(-) diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 15aacd934c..58584cc72b 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -3235,7 +3235,7 @@ void EntityItem::retrieveMarketplacePublicKey() { }); } -void EntityItem::collectChildrenForDelete(SetOfEntities& entitiesToDelete, SetOfEntities& domainEntities, const QUuid& sessionID) const { +void EntityItem::collectChildrenForDelete(SetOfEntities& entitiesToDelete, const QUuid& sessionID) const { // Deleting an entity has consequences for its children, however there are rules dictating what can be deleted. // This method helps enforce those rules for the children of entity (not for this entity). for (SpatiallyNestablePointer child : getChildren()) { @@ -3246,10 +3246,8 @@ void EntityItem::collectChildrenForDelete(SetOfEntities& entitiesToDelete, SetOf (childEntity->isMyAvatarEntity() || childEntity->getOwningAvatarID() == sessionID))) { if (entitiesToDelete.find(childEntity) == entitiesToDelete.end()) { entitiesToDelete.insert(childEntity); - childEntity->collectChildrenForDelete(entitiesToDelete, domainEntities, sessionID); + childEntity->collectChildrenForDelete(entitiesToDelete, sessionID); } - } else if (childEntity->isDomainEntity()) { - domainEntities.insert(childEntity); } } } diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index fef198c006..01e63bbd3b 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -543,7 +543,7 @@ public: static QString _marketplacePublicKey; static void retrieveMarketplacePublicKey(); - void collectChildrenForDelete(SetOfEntities& entitiesToDelete, SetOfEntities& domainEntities, const QUuid& sessionID) const; + void collectChildrenForDelete(SetOfEntities& entitiesToDelete, const QUuid& sessionID) const; float getBoundingRadius() const { return _boundingRadius; } void setSpaceIndex(int32_t index); diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index 7bb7f65089..7299665bd2 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -974,7 +974,6 @@ void EntityScriptingInterface::deleteEntity(const QUuid& id) { // If we have a local entity tree set, then also update it. SetOfEntities entitiesToDeleteImmediately; - SetOfEntities domainEntities; _entityTree->withWriteLock([&] { EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID); if (entity) { @@ -988,15 +987,13 @@ void EntityScriptingInterface::deleteEntity(const QUuid& id) { // Deleting an entity has consequences for linked children: some can be deleted but others can't. // Local- and my-avatar-entities can be deleted immediately, but other-avatar-entities can't be deleted - // by this context, and domain-entity deletes must rountrip through the entity-server for authorization. - // So we recurse down the linked hierarchy and snarf children into two categories: - // (a) entitiesToDeleteImmediately and (b) domainEntntities. + // by this context, and a domain-entity must rountrip through the entity-server for authorization. if (entity->isDomainEntity()) { - domainEntities.insert(entity); + getEntityPacketSender()->queueEraseEntityMessage(entity->getID()); } else { entitiesToDeleteImmediately.insert(entity); const auto sessionID = DependencyManager::get()->getSessionUUID(); - entity->collectChildrenForDelete(entitiesToDeleteImmediately, domainEntities, sessionID); + entity->collectChildrenForDelete(entitiesToDeleteImmediately, sessionID); } if (!entitiesToDeleteImmediately.empty()) { _entityTree->deleteEntitiesByPointer(entitiesToDeleteImmediately); @@ -1009,10 +1006,6 @@ void EntityScriptingInterface::deleteEntity(const QUuid& id) { getEntityPacketSender()->getMyAvatar()->clearAvatarEntity(entityID, false); } } - // finally ask entity-server to delete domainEntities - foreach (auto entity, domainEntities) { - getEntityPacketSender()->queueEraseEntityMessage(entity->getID()); - } } QString EntityScriptingInterface::getEntityType(const QUuid& entityID) { diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 5576f21cc5..c272d16b4d 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -261,14 +261,10 @@ void EntitySimulation::processDeadEntities() { return; } SetOfEntities entitiesToDeleteImmediately; - // NOTE: dummyList will be empty because this base-class implementation is only used server-side - // for which ATM we only process domain-entities, and since we are passing nullSessionID for authorization - // EntityItem::collectChildrenForDelete() will not collect domain-entities into this side list. - SetOfEntities dummyList; QUuid nullSessionID; foreach (auto entity, _deadEntitiesToRemoveFromTree) { entitiesToDeleteImmediately.insert(entity); - entity->collectChildrenForDelete(entitiesToDeleteImmediately, dummyList, nullSessionID); + entity->collectChildrenForDelete(entitiesToDeleteImmediately, nullSessionID); } if (_entityTree) { _entityTree->deleteEntitiesByPointer(entitiesToDeleteImmediately); diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index 646e5a0f67..5b7b38e447 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -78,7 +78,7 @@ public: virtual void prepareEntityForDelete(EntityItemPointer entity); void processChangedEntities(); - virtual void queueEraseDomainEntities(const SetOfEntities& domainEntities) const { } + virtual void queueEraseDomainEntity(const QUuid& id) const { } protected: virtual void addEntityToInternalLists(EntityItemPointer entity); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index f44f2eb7c5..ad9e71be8f 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -697,7 +697,6 @@ void EntityTree::deleteEntitiesByID(const QSet& ids, bool force, b }); } else { SetOfEntities entitiesToDelete; - SetOfEntities domainEntities; QUuid sessionID = DependencyManager::get()->getSessionUUID(); withWriteLock([&] { for (auto id : ids) { @@ -708,10 +707,12 @@ void EntityTree::deleteEntitiesByID(const QSet& ids, bool force, b } if (entity) { if (entity->isDomainEntity()) { - domainEntities.insert(entity); + if (_simulation) { + _simulation->queueEraseDomainEntity(entity->getID()); + } } else if (entity->isLocalEntity() || entity->isMyAvatarEntity()) { entitiesToDelete.insert(entity); - entity->collectChildrenForDelete(entitiesToDelete, domainEntities, sessionID); + entity->collectChildrenForDelete(entitiesToDelete, sessionID); } } } @@ -719,10 +720,6 @@ void EntityTree::deleteEntitiesByID(const QSet& ids, bool force, b deleteEntitiesByPointer(entitiesToDelete); } }); - if (!domainEntities.empty() && _simulation) { - // interface-client can't delete domainEntities outright, they must roundtrip through the entity-server - _simulation->queueEraseDomainEntities(domainEntities); - } } } @@ -2366,7 +2363,6 @@ int EntityTree::processEraseMessage(ReceivedMessage& message, const SharedNodePo #ifdef EXTRA_ERASE_DEBUGGING qCDebug(entities) << "EntityTree::processEraseMessage()"; #endif - SetOfEntities consequentialDomainEntities; withWriteLock([&] { message.seek(sizeof(OCTREE_PACKET_FLAGS) + sizeof(OCTREE_PACKET_SEQUENCE) + sizeof(OCTREE_PACKET_SENT_TIME)); @@ -2413,7 +2409,7 @@ int EntityTree::processEraseMessage(ReceivedMessage& message, const SharedNodePo SetOfEntities entitiesToDelete; for (auto entity : domainEntities) { entitiesToDelete.insert(entity); - entity->collectChildrenForDelete(entitiesToDelete, consequentialDomainEntities, sessionID); + entity->collectChildrenForDelete(entitiesToDelete, sessionID); } if (!entitiesToDelete.empty()) { @@ -2421,9 +2417,6 @@ int EntityTree::processEraseMessage(ReceivedMessage& message, const SharedNodePo } } }); - if (!consequentialDomainEntities.empty() && _simulation) { - _simulation->queueEraseDomainEntities(consequentialDomainEntities); - } return message.getPosition(); } diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 0375e1dfd9..d83708b2a2 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -176,7 +176,6 @@ void PhysicalEntitySimulation::processDeadEntities() { } PROFILE_RANGE(simulation_physics, "Deletes"); SetOfEntities entitiesToDeleteImmediately; - SetOfEntities domainEntities; QUuid sessionID = Physics::getSessionUUID(); QMutexLocker lock(&_mutex); for (auto entity : _deadEntitiesToRemoveFromTree) { @@ -185,18 +184,15 @@ void PhysicalEntitySimulation::processDeadEntities() { _entitiesToRemoveFromPhysics.insert(entity); } if (entity->isDomainEntity()) { - domainEntities.insert(entity); + // interface-client can't delete domainEntities outright, they must roundtrip through the entity-server + _entityPacketSender->queueEraseEntityMessage(entity->getID()); } else if (entity->isLocalEntity() || entity->isMyAvatarEntity()) { entitiesToDeleteImmediately.insert(entity); - entity->collectChildrenForDelete(entitiesToDeleteImmediately, domainEntities, sessionID); + entity->collectChildrenForDelete(entitiesToDeleteImmediately, sessionID); } } _deadEntitiesToRemoveFromTree.clear(); - // interface-client can't delete domainEntities outright, they must roundtrip through the entity-server - for (auto entity : domainEntities) { - _entityPacketSender->queueEraseEntityMessage(entity->getID()); - } if (!entitiesToDeleteImmediately.empty()) { getEntityTree()->deleteEntitiesByPointer(entitiesToDeleteImmediately); } @@ -233,12 +229,9 @@ void PhysicalEntitySimulation::clearEntities() { EntitySimulation::clearEntities(); } -void PhysicalEntitySimulation::queueEraseDomainEntities(const SetOfEntities& domainEntities) const { +void PhysicalEntitySimulation::queueEraseDomainEntity(const QUuid& id) const { if (_entityPacketSender) { - for (auto domainEntity : domainEntities) { - assert(domainEntity->isDomainEntity()); - _entityPacketSender->queueEraseEntityMessage(domainEntity->getID()); - } + _entityPacketSender->queueEraseEntityMessage(id); } } diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index a3bd60b96e..0f0a8e9295 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -66,7 +66,7 @@ public: void takeDeadAvatarEntities(SetOfEntities& deadEntities); virtual void clearEntities() override; - void queueEraseDomainEntities(const SetOfEntities& domainEntities) const override; + void queueEraseDomainEntity(const QUuid& id) const override; signals: void entityCollisionWithEntity(const EntityItemID& idA, const EntityItemID& idB, const Collision& collision);