From 8ab70225d93aff8d6bddbc9f90bfe5058e2bc3cf Mon Sep 17 00:00:00 2001 From: Andrew Meadows Date: Mon, 7 Oct 2019 14:34:47 -0700 Subject: [PATCH] apply delete rules down linked hierarchy --- interface/src/avatar/AvatarManager.cpp | 8 +- interface/src/avatar/MyAvatar.cpp | 45 +-- interface/src/avatar/OtherAvatar.cpp | 8 +- interface/src/ui/AvatarCertifyBanner.cpp | 11 +- .../src/avatars-renderer/Avatar.cpp | 12 +- libraries/avatars/src/AvatarData.cpp | 5 +- libraries/avatars/src/AvatarData.h | 2 +- .../src/EntityTreeRenderer.cpp | 4 + .../src/EntityTreeRenderer.h | 1 + .../entities/src/DeleteEntityOperator.cpp | 8 + libraries/entities/src/DeleteEntityOperator.h | 1 + libraries/entities/src/EntityEditFilters.cpp | 2 +- libraries/entities/src/EntityEditFilters.h | 2 +- libraries/entities/src/EntityItem.cpp | 23 +- libraries/entities/src/EntityItem.h | 4 + .../entities/src/EntityItemProperties.cpp | 2 +- .../entities/src/EntityScriptingInterface.cpp | 78 ++-- libraries/entities/src/EntitySimulation.cpp | 24 +- libraries/entities/src/EntitySimulation.h | 6 +- libraries/entities/src/EntityTree.cpp | 353 ++++++++++-------- libraries/entities/src/EntityTree.h | 19 +- libraries/physics/src/EntityMotionState.cpp | 15 +- .../physics/src/PhysicalEntitySimulation.cpp | 53 ++- .../physics/src/PhysicalEntitySimulation.h | 7 +- tests/octree/src/ModelTests.cpp | 7 +- 25 files changed, 411 insertions(+), 289 deletions(-) diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 553033f394..97a24e97f1 100755 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -543,10 +543,13 @@ void AvatarManager::removeDeadAvatarEntities(const SetOfEntities& deadEntities) for (auto entity : deadEntities) { QUuid entityOwnerID = entity->getOwningAvatarID(); AvatarSharedPointer avatar = getAvatarBySessionID(entityOwnerID); - const bool REQUIRES_REMOVAL_FROM_TREE = false; if (avatar) { - avatar->clearAvatarEntity(entity->getID(), REQUIRES_REMOVAL_FROM_TREE); + avatar->clearAvatarEntity(entity->getID()); } + } +} + +/* ADEBUG: I don't think the code below is necessary because any dead entities will have already been removed from tree and simulation if (entityTree && entity->isMyAvatarEntity()) { entityTree->withWriteLock([&] { // We only need to delete the direct children (rather than the descendants) because @@ -566,6 +569,7 @@ void AvatarManager::removeDeadAvatarEntities(const SetOfEntities& deadEntities) } } } +*/ void AvatarManager::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason) { auto avatar = std::static_pointer_cast(removedAvatar); diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 7aeb0aaa64..a46b2f46c1 100644 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1577,7 +1577,8 @@ void MyAvatar::storeAvatarEntityDataPayload(const QUuid& entityID, const QByteAr } void MyAvatar::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) { - AvatarData::clearAvatarEntity(entityID, requiresRemovalFromTree); + // NOTE: the requiresRemovalFromTree argument is unused + AvatarData::clearAvatarEntity(entityID); _avatarEntitiesLock.withWriteLock([&] { _cachedAvatarEntityBlobsToDelete.push_back(entityID); }); @@ -1647,13 +1648,16 @@ void MyAvatar::handleChangedAvatarEntityData() { // move the lists to minimize lock time std::vector cachedBlobsToDelete; std::vector cachedBlobsToUpdate; - std::vector entitiesToDelete; + QSet idsToDelete; std::vector entitiesToAdd; std::vector entitiesToUpdate; _avatarEntitiesLock.withWriteLock([&] { cachedBlobsToDelete = std::move(_cachedAvatarEntityBlobsToDelete); cachedBlobsToUpdate = std::move(_cachedAvatarEntityBlobsToAddOrUpdate); - entitiesToDelete = std::move(_entitiesToDelete); + foreach (auto id, _entitiesToDelete) { + idsToDelete.insert(id); + } + _entitiesToDelete.clear(); entitiesToAdd = std::move(_entitiesToAdd); entitiesToUpdate = std::move(_entitiesToUpdate); }); @@ -1671,7 +1675,7 @@ void MyAvatar::handleChangedAvatarEntityData() { }; // remove delete-add and delete-update overlap - for (const auto& id : entitiesToDelete) { + for (const auto& id : idsToDelete) { removeAllInstancesHelper(id, cachedBlobsToUpdate); removeAllInstancesHelper(id, entitiesToAdd); removeAllInstancesHelper(id, entitiesToUpdate); @@ -1685,11 +1689,9 @@ void MyAvatar::handleChangedAvatarEntityData() { } // DELETE real entities - for (const auto& id : entitiesToDelete) { - entityTree->withWriteLock([&] { - entityTree->deleteEntity(id); - }); - } + entityTree->withWriteLock([&] { + entityTree->deleteEntitiesByID(idsToDelete); + }); // ADD real entities EntityEditPacketSender* packetSender = qApp->getEntityEditPacketSender(); @@ -1802,7 +1804,7 @@ void MyAvatar::handleChangedAvatarEntityData() { // we have a client traits handler // flag removed entities as deleted so that changes are sent next frame _avatarEntitiesLock.withWriteLock([&] { - for (const auto& id : entitiesToDelete) { + for (const auto& id : idsToDelete) { if (_packedAvatarEntityData.find(id) != _packedAvatarEntityData.end()) { _clientTraitsHandler->markInstancedTraitDeleted(AvatarTraits::AvatarEntity, id); } @@ -2531,18 +2533,11 @@ bool isWearableEntity(const EntityItemPointer& entity) { void MyAvatar::removeWornAvatarEntity(const EntityItemID& entityID) { auto treeRenderer = DependencyManager::get(); EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - if (entityTree) { auto entity = entityTree->findEntityByID(entityID); if (entity && isWearableEntity(entity)) { - entityTree->withWriteLock([&entityID, &entityTree] { - // remove this entity first from the entity tree - entityTree->deleteEntity(entityID, true, true); - }); - - // remove the avatar entity from our internal list - // (but indicate it doesn't need to be pulled from the tree) - clearAvatarEntity(entityID, false); + treeRenderer->deleteEntity(entityID); + clearAvatarEntity(entityID); } } } @@ -2552,8 +2547,16 @@ void MyAvatar::clearWornAvatarEntities() { _avatarEntitiesLock.withReadLock([&] { avatarEntityIDs = _packedAvatarEntityData.keys(); }); - for (auto entityID : avatarEntityIDs) { - removeWornAvatarEntity(entityID); + auto treeRenderer = DependencyManager::get(); + EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; + if (entityTree) { + for (auto entityID : avatarEntityIDs) { + auto entity = entityTree->findEntityByID(entityID); + if (entity && isWearableEntity(entity)) { + treeRenderer->deleteEntity(entityID); + clearAvatarEntity(entityID); + } + } } } diff --git a/interface/src/avatar/OtherAvatar.cpp b/interface/src/avatar/OtherAvatar.cpp index 5673c2443f..5a9d20bd81 100755 --- a/interface/src/avatar/OtherAvatar.cpp +++ b/interface/src/avatar/OtherAvatar.cpp @@ -558,11 +558,17 @@ void OtherAvatar::handleChangedAvatarEntityData() { _avatarEntitiesLock.withReadLock([&] { packedAvatarEntityData = _packedAvatarEntityData; }); + QSet idsToDelete; foreach (auto entityID, recentlyRemovedAvatarEntities) { if (!packedAvatarEntityData.contains(entityID)) { - entityTree->deleteEntity(entityID, true, true); + idsToDelete.insert(entityID); } } + if (!idsToDelete.empty()) { + bool force = true; + bool ignoreWarnings = true; + entityTree->deleteEntitiesByID(idsToDelete, force, ignoreWarnings); + } // TODO: move this outside of tree lock // remove stale data hashes diff --git a/interface/src/ui/AvatarCertifyBanner.cpp b/interface/src/ui/AvatarCertifyBanner.cpp index 5101188885..3fe2ed2027 100644 --- a/interface/src/ui/AvatarCertifyBanner.cpp +++ b/interface/src/ui/AvatarCertifyBanner.cpp @@ -62,16 +62,7 @@ void AvatarCertifyBanner::show(const QUuid& avatarID) { void AvatarCertifyBanner::clear() { if (_active) { - auto entityTreeRenderer = DependencyManager::get(); - EntityTreePointer entityTree = entityTreeRenderer->getTree(); - if (!entityTree) { - return; - } - - entityTree->withWriteLock([&] { - entityTree->deleteEntity(_bannerID); - }); - + DependencyManager::get()->deleteEntity(_bannerID); _active = false; } } diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 75a7693de8..a9e9ca3dc6 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -357,11 +357,13 @@ void Avatar::removeAvatarEntitiesFromTree() { _avatarEntitiesLock.withReadLock([&] { avatarEntityIDs = _packedAvatarEntityData.keys(); }); - entityTree->withWriteLock([&] { - for (const auto& entityID : avatarEntityIDs) { - entityTree->deleteEntity(entityID, true, true); - } - }); + QSet ids; + foreach (auto id, avatarEntityIDs) { + ids.insert(id); + } + bool force = true; + bool ignoreWarnings = true; + entityTree->deleteEntitiesByID(ids, force, ignoreWarnings); // locks tree } } diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index a91154ff15..8a7d17fb12 100755 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2985,15 +2985,12 @@ void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& ent } void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) { - + // NOTE: requiresRemovalFromTree is unused bool removedEntity = false; - _avatarEntitiesLock.withWriteLock([this, &removedEntity, &entityID] { removedEntity = _packedAvatarEntityData.remove(entityID); }); - insertRemovedEntityID(entityID); - if (removedEntity && _clientTraitsHandler) { // we have a client traits handler, so we need to mark this removed instance trait as deleted // so that changes are sent next frame diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 59a2e2a53e..a66c7d897d 100755 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -1158,7 +1158,7 @@ public: /**jsdoc * @function Avatar.clearAvatarEntity * @param {Uuid} entityID - The entity ID. - * @param {boolean} [requiresRemovalFromTree=true] - Requires removal from tree. + * @param {boolean} [requiresRemovalFromTree=true] - unused * @deprecated This function is deprecated and will be removed. */ Q_INVOKABLE virtual void clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree = true); diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.cpp b/libraries/entities-renderer/src/EntityTreeRenderer.cpp index 6533d462a4..f6c02c5285 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.cpp +++ b/libraries/entities-renderer/src/EntityTreeRenderer.cpp @@ -1357,6 +1357,10 @@ EntityItemPointer EntityTreeRenderer::getEntity(const EntityItemID& id) { return result; } +void EntityTreeRenderer::deleteEntity(const EntityItemID& id) const { + DependencyManager::get()->deleteEntity(id); +} + void EntityTreeRenderer::onEntityChanged(const EntityItemID& id) { _changedEntitiesGuard.withWriteLock([&] { _changedEntities.insert(id); diff --git a/libraries/entities-renderer/src/EntityTreeRenderer.h b/libraries/entities-renderer/src/EntityTreeRenderer.h index b9fda690dd..3c69618813 100644 --- a/libraries/entities-renderer/src/EntityTreeRenderer.h +++ b/libraries/entities-renderer/src/EntityTreeRenderer.h @@ -118,6 +118,7 @@ public: void setProxyWindow(const EntityItemID& id, QWindow* proxyWindow); void setCollisionSound(const EntityItemID& id, const SharedSoundPointer& sound); EntityItemPointer getEntity(const EntityItemID& id); + void deleteEntity(const EntityItemID& id) const; void onEntityChanged(const EntityItemID& id); // Access the workload Space diff --git a/libraries/entities/src/DeleteEntityOperator.cpp b/libraries/entities/src/DeleteEntityOperator.cpp index 1dca171ae3..6750db50d2 100644 --- a/libraries/entities/src/DeleteEntityOperator.cpp +++ b/libraries/entities/src/DeleteEntityOperator.cpp @@ -53,6 +53,14 @@ void DeleteEntityOperator::addEntityIDToDeleteList(const EntityItemID& searchEnt } } +void DeleteEntityOperator::addEntityToDeleteList(const EntityItemPointer& entity) { + EntityToDeleteDetails details; + details.entity = entity; + details.containingElement = entity->getElement(); + details.cube = details.containingElement->getAACube(); + _entitiesToDelete << details; + _lookingCount++; +} // does this entity tree element contain the old entity bool DeleteEntityOperator::subTreeContainsSomeEntitiesToDelete(const OctreeElementPointer& element) { diff --git a/libraries/entities/src/DeleteEntityOperator.h b/libraries/entities/src/DeleteEntityOperator.h index 3b3ee2a868..1449e2caad 100644 --- a/libraries/entities/src/DeleteEntityOperator.h +++ b/libraries/entities/src/DeleteEntityOperator.h @@ -42,6 +42,7 @@ public: ~DeleteEntityOperator(); void addEntityIDToDeleteList(const EntityItemID& searchEntityID); + void addEntityToDeleteList(const EntityItemPointer& entity); virtual bool preRecursion(const OctreeElementPointer& element) override; virtual bool postRecursion(const OctreeElementPointer& element) override; diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index a222ca8216..16dace0fc8 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -43,7 +43,7 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { } bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, - bool& wasChanged, EntityTree::FilterType filterType, EntityItemID& itemID, EntityItemPointer& existingEntity) { + bool& wasChanged, EntityTree::FilterType filterType, EntityItemID& itemID, const EntityItemPointer& existingEntity) { // get the ids of all the zones (plus the global entity edit filter) that the position // lies within diff --git a/libraries/entities/src/EntityEditFilters.h b/libraries/entities/src/EntityEditFilters.h index cb99c97762..69fd920998 100644 --- a/libraries/entities/src/EntityEditFilters.h +++ b/libraries/entities/src/EntityEditFilters.h @@ -55,7 +55,7 @@ public: void removeFilter(EntityItemID entityID); bool filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, - EntityTree::FilterType filterType, EntityItemID& entityID, EntityItemPointer& existingEntity); + EntityTree::FilterType filterType, EntityItemID& entityID, const EntityItemPointer& existingEntity); signals: void filterAdded(EntityItemID id, bool success); diff --git a/libraries/entities/src/EntityItem.cpp b/libraries/entities/src/EntityItem.cpp index 60a8cadf6c..15aacd934c 100644 --- a/libraries/entities/src/EntityItem.cpp +++ b/libraries/entities/src/EntityItem.cpp @@ -3203,6 +3203,7 @@ void EntityItem::somethingChangedNotification() { }); } +// static void EntityItem::retrieveMarketplacePublicKey() { QNetworkAccessManager& networkAccessManager = NetworkAccessManager::getInstance(); QNetworkRequest networkRequest; @@ -3234,6 +3235,26 @@ void EntityItem::retrieveMarketplacePublicKey() { }); } +void EntityItem::collectChildrenForDelete(SetOfEntities& entitiesToDelete, SetOfEntities& domainEntities, 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()) { + if (child && child->getNestableType() == NestableType::Entity) { + EntityItemPointer childEntity = std::static_pointer_cast(child); + // NOTE: null sessionID means "collect ALL known entities", else we only collect: local-entities and authorized avatar-entities + if (sessionID.isNull() || childEntity->isLocalEntity() || (childEntity->isAvatarEntity() && + (childEntity->isMyAvatarEntity() || childEntity->getOwningAvatarID() == sessionID))) { + if (entitiesToDelete.find(childEntity) == entitiesToDelete.end()) { + entitiesToDelete.insert(childEntity); + childEntity->collectChildrenForDelete(entitiesToDelete, domainEntities, sessionID); + } + } else if (childEntity->isDomainEntity()) { + domainEntities.insert(childEntity); + } + } + } +} + void EntityItem::setSpaceIndex(int32_t index) { assert(_spaceIndex == -1); _spaceIndex = index; @@ -3398,7 +3419,7 @@ void EntityItem::prepareForSimulationOwnershipBid(EntityItemProperties& properti properties.setSimulationOwner(Physics::getSessionUUID(), priority); setPendingOwnershipPriority(priority); - // ANDREW TODO: figure out if it would be OK to NOT bother set these properties + // ANDREW TODO: figure out if it would be OK to NOT bother set these properties here properties.setEntityHostType(getEntityHostType()); properties.setOwningAvatarID(getOwningAvatarID()); setLastBroadcast(now); // for debug/physics status icons diff --git a/libraries/entities/src/EntityItem.h b/libraries/entities/src/EntityItem.h index bdccd466cb..fef198c006 100644 --- a/libraries/entities/src/EntityItem.h +++ b/libraries/entities/src/EntityItem.h @@ -18,6 +18,7 @@ #include #include +#include #include // for EncodeBitstreamParams class #include // for OctreeElement::AppendState @@ -49,6 +50,7 @@ typedef std::shared_ptr EntityTreePointer; typedef std::shared_ptr EntityDynamicPointer; typedef std::shared_ptr EntityTreeElementPointer; using EntityTreeElementExtraEncodeDataPointer = std::shared_ptr; +using SetOfEntities = QSet; #define DONT_ALLOW_INSTANTIATION virtual void pureVirtualFunctionPlaceHolder() = 0; #define ALLOW_INSTANTIATION virtual void pureVirtualFunctionPlaceHolder() override { }; @@ -541,6 +543,8 @@ public: static QString _marketplacePublicKey; static void retrieveMarketplacePublicKey(); + void collectChildrenForDelete(SetOfEntities& entitiesToDelete, SetOfEntities& domainEntities, const QUuid& sessionID) const; + float getBoundingRadius() const { return _boundingRadius; } void setSpaceIndex(int32_t index); int32_t getSpaceIndex() const { return _spaceIndex; } diff --git a/libraries/entities/src/EntityItemProperties.cpp b/libraries/entities/src/EntityItemProperties.cpp index 5cb5ddbbfa..02272a1bfa 100644 --- a/libraries/entities/src/EntityItemProperties.cpp +++ b/libraries/entities/src/EntityItemProperties.cpp @@ -3936,7 +3936,7 @@ bool EntityItemProperties::decodeCloneEntityMessage(const QByteArray& buffer, in processedBytes = 0; if (NUM_BYTES_RFC4122_UUID * 2 > packetLength) { - qCDebug(entities) << "EntityItemProperties::processEraseMessageDetails().... bailing because not enough bytes in buffer"; + qCDebug(entities) << "EntityItemProperties::decodeCloneEntityMessage().... bailing because not enough bytes in buffer"; return false; // bail to prevent buffer overflow } diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index 803c1ac4ca..c550a7b38b 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -966,42 +966,52 @@ void EntityScriptingInterface::deleteEntity(const QUuid& id) { _activityTracking.deletedEntityCount++; - EntityItemID entityID(id); - bool shouldSendDeleteToServer = true; - - // If we have a local entity tree set, then also update it. - if (_entityTree) { - _entityTree->withWriteLock([&] { - EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID); - if (entity) { - auto nodeList = DependencyManager::get(); - const QUuid myNodeID = nodeList->getSessionUUID(); - if (entity->isAvatarEntity() && !entity->isMyAvatarEntity()) { - // don't delete other avatar's avatarEntities - shouldSendDeleteToServer = false; - return; - } - - if (entity->getLocked()) { - shouldSendDeleteToServer = false; - } else { - // only delete non-domain entities, domain entities will round trip through the server filters - if (!entity->isDomainEntity() || _entityTree->isServerlessMode()) { - shouldSendDeleteToServer = false; - _entityTree->deleteEntity(entityID); - - if (entity->isAvatarEntity() && getEntityPacketSender()->getMyAvatar()) { - getEntityPacketSender()->getMyAvatar()->clearAvatarEntity(entityID, false); - } - } - } - } - }); + if (!_entityTree) { + return; } - // if at this point, we know the id, and we should still delete the entity, send the update to the entity server - if (shouldSendDeleteToServer) { - getEntityPacketSender()->queueEraseEntityMessage(entityID); + EntityItemID entityID(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) { + if (entity->isAvatarEntity() && !entity->isMyAvatarEntity()) { + // don't delete other avatar's avatarEntities + return; + } + if (entity->getLocked()) { + return; + } + + // 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. + if (entity->isDomainEntity()) { + domainEntities.insert(entity); + } else { + entitiesToDeleteImmediately.insert(entity); + const auto sessionID = DependencyManager::get()->getSessionUUID(); + entity->collectChildrenForDelete(entitiesToDeleteImmediately, domainEntities, sessionID); + } + if (!entitiesToDeleteImmediately.empty()) { + _entityTree->deleteEntitiesByPointer(entitiesToDeleteImmediately); + } + } + }); + + foreach (auto entity, entitiesToDeleteImmediately) { + if (entity->isMyAvatarEntity()) { + getEntityPacketSender()->getMyAvatar()->clearAvatarEntity(entityID, false); + } + } + // finally ask entity-server to delete domainEntities + foreach (auto entity, domainEntities) { + getEntityPacketSender()->queueEraseEntityMessage(entity->getID()); } } diff --git a/libraries/entities/src/EntitySimulation.cpp b/libraries/entities/src/EntitySimulation.cpp index 976cae9bd9..48de811be5 100644 --- a/libraries/entities/src/EntitySimulation.cpp +++ b/libraries/entities/src/EntitySimulation.cpp @@ -39,12 +39,7 @@ void EntitySimulation::updateEntities() { callUpdateOnEntitiesThatNeedIt(now); moveSimpleKinematics(now); sortEntitiesThatMoved(); -} - -void EntitySimulation::takeDeadEntities(SetOfEntities& entitiesToDelete) { - QMutexLocker lock(&_mutex); - entitiesToDelete.swap(_deadEntities); - _deadEntities.clear(); + processDeadEntities(); } void EntitySimulation::removeEntityFromInternalLists(EntityItemPointer entity) { @@ -260,3 +255,20 @@ void EntitySimulation::moveSimpleKinematics(uint64_t now) { } } } + +void EntitySimulation::processDeadEntities() { + if (_deadEntities.empty()) { + return; + } + SetOfEntities entitiesToDeleteImmediately; + SetOfEntities dummyList; // ignore this list: it will be empty + foreach (auto entity, _deadEntities) { + QUuid nullSessionID; + entitiesToDeleteImmediately.insert(entity); + entity->collectChildrenForDelete(entitiesToDeleteImmediately, dummyList, nullSessionID); + } + if (_entityTree) { + _entityTree->deleteEntitiesByPointer(entitiesToDeleteImmediately); + } + _deadEntities.clear(); +} diff --git a/libraries/entities/src/EntitySimulation.h b/libraries/entities/src/EntitySimulation.h index e9ad7ab2b7..6c32a7526f 100644 --- a/libraries/entities/src/EntitySimulation.h +++ b/libraries/entities/src/EntitySimulation.h @@ -16,7 +16,6 @@ #include #include -#include #include #include @@ -25,7 +24,6 @@ #include "EntityTree.h" using EntitySimulationPointer = std::shared_ptr; -using SetOfEntities = QSet; using VectorOfEntities = QVector; // the EntitySimulation needs to know when these things change on an entity, @@ -77,16 +75,16 @@ public: EntityTreePointer getEntityTree() { return _entityTree; } - virtual void takeDeadEntities(SetOfEntities& entitiesToDelete); - virtual void prepareEntityForDelete(EntityItemPointer entity); void processChangedEntities(); + virtual void queueEraseDomainEntities(const SetOfEntities& domainEntities) const { } protected: virtual void addEntityToInternalLists(EntityItemPointer entity); virtual void removeEntityFromInternalLists(EntityItemPointer entity); virtual void processChangedEntity(const EntityItemPointer& entity); + virtual void processDeadEntities(); void expireMortalEntities(uint64_t now); void callUpdateOnEntitiesThatNeedIt(uint64_t now); diff --git a/libraries/entities/src/EntityTree.cpp b/libraries/entities/src/EntityTree.cpp index 6440f39f52..75a805559f 100644 --- a/libraries/entities/src/EntityTree.cpp +++ b/libraries/entities/src/EntityTree.cpp @@ -98,14 +98,15 @@ void EntityTree::eraseDomainAndNonOwnedEntities() { if (element) { element->cleanupDomainAndNonOwnedEntities(); } - - if (entity->isLocalEntity() || entity->isMyAvatarEntity()) { - savedEntities[entity->getEntityItemID()] = entity; - } else { - int32_t spaceIndex = entity->getSpaceIndex(); - if (spaceIndex != -1) { - // stale spaceIndices will be freed later - _staleProxies.push_back(spaceIndex); + if (!getIsServer()) { + if (entity->isLocalEntity() || entity->isMyAvatarEntity()) { + savedEntities[entity->getEntityItemID()] = entity; + } else { + int32_t spaceIndex = entity->getSpaceIndex(); + if (spaceIndex != -1) { + // stale spaceIndices will be freed later + _staleProxies.push_back(spaceIndex); + } } } } @@ -144,10 +145,12 @@ void EntityTree::eraseAllOctreeElements(bool createNewRoot) { if (element) { element->cleanupEntities(); } - int32_t spaceIndex = entity->getSpaceIndex(); - if (spaceIndex != -1) { - // assume stale spaceIndices will be freed later - _staleProxies.push_back(spaceIndex); + if (!getIsServer()) { + int32_t spaceIndex = entity->getSpaceIndex(); + if (spaceIndex != -1) { + // assume stale spaceIndices will be freed later + _staleProxies.push_back(spaceIndex); + } } } }); @@ -605,61 +608,21 @@ void EntityTree::setSimulation(EntitySimulationPointer simulation) { } void EntityTree::deleteEntity(const EntityItemID& entityID, bool force, bool ignoreWarnings) { - EntityTreeElementPointer containingElement = getContainingElement(entityID); - if (!containingElement) { - if (!ignoreWarnings) { - qCWarning(entities) << "EntityTree::deleteEntity() on non-existent entityID=" << entityID; - } - return; - } - - EntityItemPointer existingEntity = containingElement->getEntityWithEntityItemID(entityID); - if (!existingEntity) { - if (!ignoreWarnings) { - qCWarning(entities) << "EntityTree::deleteEntity() on non-existant entity item with entityID=" << entityID; - } - return; - } - - if (existingEntity->getLocked() && !force) { - if (!ignoreWarnings) { - qCDebug(entities) << "ERROR! EntityTree::deleteEntity() trying to delete locked entity. entityID=" << entityID; - } - return; - } - - cleanupCloneIDs(entityID); - unhookChildAvatar(entityID); - emit deletingEntity(entityID); - emit deletingEntityPointer(existingEntity.get()); - - // NOTE: callers must lock the tree before using this method - DeleteEntityOperator theOperator(getThisPointer(), entityID); - - existingEntity->forEachDescendant([&](SpatiallyNestablePointer descendant) { - auto descendantID = descendant->getID(); - theOperator.addEntityIDToDeleteList(descendantID); - emit deletingEntity(descendantID); - EntityItemPointer descendantEntity = std::dynamic_pointer_cast(descendant); - if (descendantEntity) { - emit deletingEntityPointer(descendantEntity.get()); - } - }); - - recurseTreeWithOperator(&theOperator); - processRemovedEntities(theOperator); - _isDirty = true; + // NOTE: can be called without lock because deleteEntitiesByID() will lock + QSet ids; + ids << entityID; + deleteEntitiesByID(ids, force, ignoreWarnings); } void EntityTree::unhookChildAvatar(const EntityItemID entityID) { - - EntityItemPointer entity = findEntityByEntityItemID(entityID); - - entity->forEachDescendant([&](SpatiallyNestablePointer child) { - if (child->getNestableType() == NestableType::Avatar) { - child->setParentID(nullptr); - } - }); + if (!getIsServer()) { + EntityItemPointer entity = findEntityByEntityItemID(entityID); + entity->forEachDescendant([&](SpatiallyNestablePointer child) { + if (child->getNestableType() == NestableType::Avatar) { + child->setParentID(nullptr); + } + }); + } } void EntityTree::cleanupCloneIDs(const EntityItemID& entityID) { @@ -684,40 +647,98 @@ void EntityTree::cleanupCloneIDs(const EntityItemID& entityID) { } } -void EntityTree::deleteEntities(QSet entityIDs, bool force, bool ignoreWarnings) { - // NOTE: callers must lock the tree before using this method +void EntityTree::recursivelyFilterAndCollectForDelete(const EntityItemPointer& entity, SetOfEntities& entitiesToDelete, bool force) const { + // tree must be read-locked before calling this method + //TODO: assert(treeIsLocked); + assert(entity); + if (entity->getElement() && (entitiesToDelete.find(entity) == entitiesToDelete.end())) { + // filter + bool allowed = force; + if (!allowed) { + bool wasChanged = false; + auto startFilter = usecTimestampNow(); + EntityItemProperties dummyProperties; + allowed = force || filterProperties(entity, dummyProperties, dummyProperties, wasChanged, FilterType::Delete); + auto endFilter = usecTimestampNow(); + _totalFilterTime += endFilter - startFilter; + } + if (allowed) { + entitiesToDelete.insert(entity); + for (SpatiallyNestablePointer child : entity->getChildren()) { + if (child && child->getNestableType() == NestableType::Entity) { + EntityItemPointer childEntity = std::static_pointer_cast(child); + recursivelyFilterAndCollectForDelete(childEntity, entitiesToDelete, force); + } + } + } + } +} + +void EntityTree::deleteEntitiesByID(const QSet& ids, bool force, bool ignoreWarnings) { + // this method has two paths: + // (a) entity-server: applies delete filter + // (b) interface-client: deletes local- and my-avatar-entities immediately, submits domainEntity deletes to the entity-server + if (getIsServer()) { + withWriteLock([&] { + SetOfEntities entitiesToDelete; + for (auto id : ids) { + EntityItemPointer entity; + { + QReadLocker locker(&_entityMapLock); + entity = _entityMap.value(id); + } + if (entity) { + recursivelyFilterAndCollectForDelete(entity, entitiesToDelete, force); + } + } + if (!entitiesToDelete.empty()) { + deleteEntitiesByPointer(entitiesToDelete); + } + }); + } else { + SetOfEntities entitiesToDelete; + SetOfEntities domainEntities; + QUuid sessionID = DependencyManager::get()->getSessionUUID(); + withWriteLock([&] { + for (auto id : ids) { + EntityItemPointer entity; + { + QReadLocker locker(&_entityMapLock); + entity = _entityMap.value(id); + } + if (entity) { + if (entity->isDomainEntity()) { + domainEntities.insert(entity); + } else if (entity->isLocalEntity() || entity->isMyAvatarEntity()) { + entitiesToDelete.insert(entity); + entity->collectChildrenForDelete(entitiesToDelete, domainEntities, sessionID); + } + } + } + if (!entitiesToDelete.empty()) { + deleteEntitiesByPointer(entitiesToDelete); + } + }); + if (!domainEntities.empty() && _simulation) { + // interface-client can't delete domainEntities outright, they must roundtrip through the entity-server + _simulation->queueEraseDomainEntities(domainEntities); + } + } +} + +void EntityTree::deleteEntitiesByPointer(const SetOfEntities& entities) { + // tree must be write-locked before calling this method + //TODO: assert(treeIsLocked); + // NOTE: there is no entity validation (i.e. is entity in tree?) nor snarfing of children beyond this point. + // Get those done BEFORE calling this method. + for (auto entity : entities) { + cleanupCloneIDs(entity->getID()); + } DeleteEntityOperator theOperator(getThisPointer()); - foreach(const EntityItemID& entityID, entityIDs) { - // ANDREW TODO: rejigger this logic to FIRST get entity THEN get element - EntityTreeElementPointer containingElement = getContainingElement(entityID); - if (!containingElement) { - if (!ignoreWarnings) { - qCWarning(entities) << "EntityTree::deleteEntities() on non-existent entityID=" << entityID; - } - continue; - } - - EntityItemPointer existingEntity = containingElement->getEntityWithEntityItemID(entityID); - if (!existingEntity) { - if (!ignoreWarnings) { - qCWarning(entities) << "EntityTree::deleteEntities() on non-existent entity item with entityID=" << entityID; - } - continue; - } - - if (existingEntity->getLocked() && !force) { - if (!ignoreWarnings) { - qCDebug(entities) << "ERROR! EntityTree::deleteEntities() trying to delete locked entity. entityID=" << entityID; - } - continue; - } - - // tell our delete operator about this entityID - cleanupCloneIDs(entityID); - unhookChildAvatar(entityID); - theOperator.addEntityIDToDeleteList(entityID); - emit deletingEntity(entityID); - emit deletingEntityPointer(existingEntity.get()); + for (auto entity : entities) { + theOperator.addEntityToDeleteList(entity); + emit deletingEntity(entity->getID()); + emit deletingEntityPointer(entity.get()); } if (!theOperator.getEntities().empty()) { @@ -728,23 +749,11 @@ void EntityTree::deleteEntities(QSet entityIDs, bool force, bool i } void EntityTree::processRemovedEntities(const DeleteEntityOperator& theOperator) { + // NOTE: assume tree already write-locked because this method only called in deleteEntitiesByPointer() quint64 deletedAt = usecTimestampNow(); const RemovedEntities& entities = theOperator.getEntities(); foreach(const EntityToDeleteDetails& details, entities) { EntityItemPointer theEntity = details.entity; - - if (getIsServer()) { - QSet childrenIDs; - theEntity->forEachChild([&](SpatiallyNestablePointer child) { - if (child->getNestableType() == NestableType::Entity) { - childrenIDs += child->getID(); - } - }); - deleteEntities(childrenIDs, true, true); - } - - theEntity->die(); - if (getIsServer()) { removeCertifiedEntityOnServer(theEntity); @@ -752,19 +761,24 @@ void EntityTree::processRemovedEntities(const DeleteEntityOperator& theOperator) QWriteLocker recentlyDeletedEntitiesLocker(&_recentlyDeletedEntitiesLock); _recentlyDeletedEntityItemIDs.insert(deletedAt, theEntity->getEntityItemID()); } else { + theEntity->forEachDescendant([&](SpatiallyNestablePointer child) { + if (child->getNestableType() == NestableType::Avatar) { + child->setParentID(nullptr); + } + }); + // on the client side, we also remember that we deleted this entity, we don't care about the time trackDeletedEntity(theEntity->getEntityItemID()); - } + int32_t spaceIndex = theEntity->getSpaceIndex(); + if (spaceIndex != -1) { + // stale spaceIndices will be freed later + _staleProxies.push_back(spaceIndex); + } + } if (theEntity->isSimulated()) { _simulation->prepareEntityForDelete(theEntity); } - - int32_t spaceIndex = theEntity->getSpaceIndex(); - if (spaceIndex != -1) { - // stale spaceIndices will be freed later - _staleProxies.push_back(spaceIndex); - } } } @@ -1370,7 +1384,7 @@ void EntityTree::fixupTerseEditLogging(EntityItemProperties& properties, QList(); if (entityEditFilters) { @@ -1750,9 +1764,9 @@ void EntityTree::processChallengeOwnershipPacket(ReceivedMessage& message, const } } +// NOTE: Caller must lock the tree before calling this. int EntityTree::processEditPacketData(ReceivedMessage& message, const unsigned char* editData, int maxLength, const SharedNodePointer& senderNode) { - if (!getIsServer()) { qCWarning(entities) << "EntityTree::processEditPacketData() should only be called on a server tree."; return 0; @@ -2218,7 +2232,9 @@ void EntityTree::fixupNeedsParentFixups() { void EntityTree::deleteDescendantsOfAvatar(QUuid avatarID) { if (_childrenOfAvatars.contains(avatarID)) { - deleteEntities(_childrenOfAvatars[avatarID]); + bool force = true; + bool ignoreWarnings = true; + deleteEntitiesByID(_childrenOfAvatars[avatarID], force, ignoreWarnings); _childrenOfAvatars.remove(avatarID); } } @@ -2250,22 +2266,6 @@ void EntityTree::update(bool simulate) { if (simulate && _simulation) { withWriteLock([&] { _simulation->updateEntities(); - { - PROFILE_RANGE(simulation_physics, "Deletes"); - SetOfEntities deadEntities; - _simulation->takeDeadEntities(deadEntities); - if (!deadEntities.empty()) { - // translate into list of ID's - QSet idsToDelete; - - for (auto entity : deadEntities) { - idsToDelete.insert(entity->getEntityItemID()); - } - - // delete these things the roundabout way - deleteEntities(idsToDelete, true); - } - } }); } } @@ -2354,12 +2354,17 @@ bool EntityTree::shouldEraseEntity(EntityItemID entityID, const SharedNodePointe return allowed; } - -// TODO: consider consolidating processEraseMessageDetails() and processEraseMessage() int EntityTree::processEraseMessage(ReceivedMessage& message, const SharedNodePointer& sourceNode) { + // NOTE: this is only called by the interface-client on receipt of deleteEntity message from entity-server. + // Which means this is a state synchronization message from the the entity-server. It is saying + // "The following domain-entities have already been deleted". While need to perform sanity checking + // (e.g. verify these are domain entities) permissions need NOT checked for the domain-entities. + assert(!getIsServer()); + // TODO: remove this stuff out of EntityTree:: and into interface-client code. #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)); @@ -2367,10 +2372,9 @@ int EntityTree::processEraseMessage(ReceivedMessage& message, const SharedNodePo message.readPrimitive(&numberOfIDs); if (numberOfIDs > 0) { - QSet entityItemIDsToDelete; + QSet idsToDelete; for (size_t i = 0; i < numberOfIDs; i++) { - if (NUM_BYTES_RFC4122_UUID > message.getBytesLeftToRead()) { qCDebug(entities) << "EntityTree::processEraseMessage().... bailing because not enough bytes in buffer"; break; // bail to prevent buffer overflow @@ -2382,64 +2386,85 @@ int EntityTree::processEraseMessage(ReceivedMessage& message, const SharedNodePo #endif EntityItemID entityItemID(entityID); + idsToDelete << entityItemID; + } - if (shouldEraseEntity(entityID, sourceNode)) { - entityItemIDsToDelete << entityItemID; - cleanupCloneIDs(entityItemID); + // domain-entity deletion can trigger deletion of other entities the entity-server doesn't know about + // so we must recurse down the children and collect consequential deletes however + // we must first identify all domain-entities in idsToDelete so as to not overstep entity-server's authority + SetOfEntities domainEntities; + for (auto id : idsToDelete) { + EntityItemPointer entity = _entityMap.value(id); + if (entity && entity->isDomainEntity()) { + domainEntities.insert(entity); } } - deleteEntities(entityItemIDsToDelete, true, true); + // now we recurse domain-entities children and snarf consequential entities + auto nodeList = DependencyManager::get(); + QUuid sessionID = nodeList->getSessionUUID(); + // NOTE: normally a null sessionID would be bad, as that would cause the collectDhildrenForDelete() method below + // to snarf domain entities for which the interface-client is not authorized to delete without explicit instructions + // from the entity-server, however it is ok here because that would mean: + // (a) interface-client is not connected to a domain which means... + // (b) we should never get here (since this would correspond to a message from the entity-server) but... + // (c) who cares? When not connected to a domain the interface-client can do whatever it wants. + SetOfEntities entitiesToDelete; + for (auto entity : domainEntities) { + entitiesToDelete.insert(entity); + entity->collectChildrenForDelete(entitiesToDelete, consequentialDomainEntities, sessionID); + } + + if (!entitiesToDelete.empty()) { + deleteEntitiesByPointer(entitiesToDelete); + } } }); + if (!consequentialDomainEntities.empty() && _simulation) { + _simulation->queueEraseDomainEntities(consequentialDomainEntities); + } return message.getPosition(); } // This version skips over the header -// NOTE: Caller must lock the tree before calling this. -// TODO: consider consolidating processEraseMessageDetails() and processEraseMessage() +// NOTE: Caller must write-lock the tree before calling this. int EntityTree::processEraseMessageDetails(const QByteArray& dataByteArray, const SharedNodePointer& sourceNode) { + //TODO: assert(treeIsLocked); + assert(getIsServer()); #ifdef EXTRA_ERASE_DEBUGGING qCDebug(entities) << "EntityTree::processEraseMessageDetails()"; #endif - const unsigned char* packetData = (const unsigned char*)dataByteArray.constData(); - const unsigned char* dataAt = packetData; size_t packetLength = dataByteArray.size(); size_t processedBytes = 0; uint16_t numberOfIds = 0; // placeholder for now - memcpy(&numberOfIds, dataAt, sizeof(numberOfIds)); - dataAt += sizeof(numberOfIds); + memcpy(&numberOfIds, dataByteArray.constData(), sizeof(numberOfIds)); processedBytes += sizeof(numberOfIds); if (numberOfIds > 0) { - QSet entityItemIDsToDelete; + QSet ids; + // extract ids from packet for (size_t i = 0; i < numberOfIds; i++) { - - if (processedBytes + NUM_BYTES_RFC4122_UUID > packetLength) { qCDebug(entities) << "EntityTree::processEraseMessageDetails().... bailing because not enough bytes in buffer"; break; // bail to prevent buffer overflow } QByteArray encodedID = dataByteArray.mid((int)processedBytes, NUM_BYTES_RFC4122_UUID); - QUuid entityID = QUuid::fromRfc4122(encodedID); - dataAt += encodedID.size(); + QUuid id = QUuid::fromRfc4122(encodedID); processedBytes += encodedID.size(); #ifdef EXTRA_ERASE_DEBUGGING - qCDebug(entities) << " ---- EntityTree::processEraseMessageDetails() contains id:" << entityID; + qCDebug(entities) << " ---- EntityTree::processEraseMessageDetails() contains id:" << id; #endif - EntityItemID entityItemID(entityID); - - if (shouldEraseEntity(entityID, sourceNode)) { - entityItemIDsToDelete << entityItemID; - cleanupCloneIDs(entityItemID); - } - + EntityItemID entityID(id); + ids << entityID; } - deleteEntities(entityItemIDsToDelete, true, true); + + bool force = sourceNode->isAllowedEditor(); + bool ignoreWarnings = true; + deleteEntitiesByID(ids, force, ignoreWarnings); } return (int)processedBytes; } diff --git a/libraries/entities/src/EntityTree.h b/libraries/entities/src/EntityTree.h index 9108f8d8d2..6574b9d601 100644 --- a/libraries/entities/src/EntityTree.h +++ b/libraries/entities/src/EntityTree.h @@ -126,7 +126,9 @@ public: void unhookChildAvatar(const EntityItemID entityID); void cleanupCloneIDs(const EntityItemID& entityID); void deleteEntity(const EntityItemID& entityID, bool force = false, bool ignoreWarnings = true); - void deleteEntities(QSet entityIDs, bool force = false, bool ignoreWarnings = true); + + void deleteEntitiesByID(const QSet& entityIDs, bool force = false, bool ignoreWarnings = true); + void deleteEntitiesByPointer(const SetOfEntities& entities); EntityItemPointer findEntityByID(const QUuid& id) const; EntityItemPointer findEntityByEntityItemID(const EntityItemID& entityID) const; @@ -291,6 +293,7 @@ signals: protected: + void recursivelyFilterAndCollectForDelete(const EntityItemPointer& entity, SetOfEntities& entitiesToDelete, bool force) const; void processRemovedEntities(const DeleteEntityOperator& theOperator); bool updateEntity(EntityItemPointer entity, const EntityItemProperties& properties, const SharedNodePointer& senderNode = SharedNodePointer(nullptr)); @@ -339,12 +342,12 @@ protected: int _totalEditMessages = 0; int _totalUpdates = 0; int _totalCreates = 0; - quint64 _totalDecodeTime = 0; - quint64 _totalLookupTime = 0; - quint64 _totalUpdateTime = 0; - quint64 _totalCreateTime = 0; - quint64 _totalLoggingTime = 0; - quint64 _totalFilterTime = 0; + mutable quint64 _totalDecodeTime = 0; + mutable quint64 _totalLookupTime = 0; + mutable quint64 _totalUpdateTime = 0; + mutable quint64 _totalCreateTime = 0; + mutable quint64 _totalLoggingTime = 0; + mutable quint64 _totalFilterTime = 0; // these performance statistics are only used in the client void resetClientEditStats(); @@ -364,7 +367,7 @@ protected: float _maxTmpEntityLifetime { DEFAULT_MAX_TMP_ENTITY_LIFETIME }; - bool filterProperties(EntityItemPointer& existingEntity, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, FilterType filterType); + bool filterProperties(const EntityItemPointer& existingEntity, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, FilterType filterType) const; bool _hasEntityEditFilter{ false }; QStringList _entityScriptSourceWhitelist; diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp index 2ec2eff7f9..f3d129871f 100644 --- a/libraries/physics/src/EntityMotionState.cpp +++ b/libraries/physics/src/EntityMotionState.cpp @@ -62,7 +62,6 @@ EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer // rather than pass the legit shape pointer to the ObjectMotionState ctor above. setShape(shape); - // ANDREW TODO: maybe all non-domain entities should be unownable? or our avatar entities should have a special OwnershipState if (_entity->isAvatarEntity() && !_entity->isMyAvatarEntity()) { // avatar entities are always thus, so we cache this fact in _ownershipState _ownershipState = EntityMotionState::OwnershipState::Unownable; @@ -748,7 +747,6 @@ bool EntityMotionState::shouldSendBid() const { // NOTE: this method is only ever called when the entity's simulation is NOT locally owned return _body->isActive() && (_region == workload::Region::R1) - // ANDREW TODO: make sure _ownershipState to Unownable on creation when applicable && _ownershipState != EntityMotionState::OwnershipState::Unownable && glm::max(glm::max(VOLUNTEER_SIMULATION_PRIORITY, _bumpedPriority), _entity->getScriptSimulationPriority()) >= _entity->getSimulationPriority() && !_entity->getLocked() @@ -770,8 +768,7 @@ uint8_t EntityMotionState::computeFinalBidPriority() const { } bool EntityMotionState::isLocallyOwned() const { - // ANDREW TODO: maybe also always return true for MyAvatar's avatar entities? - return _entity->getSimulatorID() == Physics::getSessionUUID(); + return _entity->getSimulatorID() == Physics::getSessionUUID() || _entity->isMyAvatarEntity(); } bool EntityMotionState::isLocallyOwnedOrShouldBe() const { @@ -789,13 +786,15 @@ void EntityMotionState::setRegion(uint8_t region) { } void EntityMotionState::initForBid() { - assert(_ownershipState != EntityMotionState::OwnershipState::Unownable); - _ownershipState = EntityMotionState::OwnershipState::PendingBid; + if (_ownershipState != EntityMotionState::OwnershipState::Unownable) { + _ownershipState = EntityMotionState::OwnershipState::PendingBid; + } } void EntityMotionState::initForOwned() { - assert(_ownershipState != EntityMotionState::OwnershipState::Unownable); - _ownershipState = EntityMotionState::OwnershipState::LocallyOwned; + if (_ownershipState != EntityMotionState::OwnershipState::Unownable) { + _ownershipState = EntityMotionState::OwnershipState::LocallyOwned; + } } void EntityMotionState::clearOwnershipState() { diff --git a/libraries/physics/src/PhysicalEntitySimulation.cpp b/libraries/physics/src/PhysicalEntitySimulation.cpp index 6356a439d3..a7f66929ed 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.cpp +++ b/libraries/physics/src/PhysicalEntitySimulation.cpp @@ -107,18 +107,6 @@ void PhysicalEntitySimulation::clearOwnershipData() { _bids.clear(); } -void PhysicalEntitySimulation::takeDeadEntities(SetOfEntities& deadEntities) { - QMutexLocker lock(&_mutex); - for (auto entity : _deadEntities) { - EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); - if (motionState) { - _entitiesToRemoveFromPhysics.insert(entity); - } - } - _deadEntities.swap(deadEntities); - _deadEntities.clear(); -} - void PhysicalEntitySimulation::takeDeadAvatarEntities(SetOfEntities& deadEntities) { _deadAvatarEntities.swap(deadEntities); _deadAvatarEntities.clear(); @@ -182,6 +170,38 @@ void PhysicalEntitySimulation::processChangedEntity(const EntityItemPointer& ent } } +void PhysicalEntitySimulation::processDeadEntities() { + if (_deadEntities.empty()) { + return; + } + PROFILE_RANGE(simulation_physics, "Deletes"); + SetOfEntities entitiesToDeleteImmediately; + SetOfEntities domainEntities; + QUuid sessionID = Physics::getSessionUUID(); + QMutexLocker lock(&_mutex); + for (auto entity : _deadEntities) { + EntityMotionState* motionState = static_cast(entity->getPhysicsInfo()); + if (motionState) { + _entitiesToRemoveFromPhysics.insert(entity); + } + if (entity->isDomainEntity()) { + domainEntities.insert(entity); + } else if (entity->isLocalEntity() || entity->isMyAvatarEntity()) { + entitiesToDeleteImmediately.insert(entity); + entity->collectChildrenForDelete(entitiesToDeleteImmediately, domainEntities, sessionID); + } + } + _deadEntities.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); + } +} + void PhysicalEntitySimulation::clearEntities() { // TODO: we should probably wait to lock the _physicsEngine so we don't mess up data structures // while it is in the middle of a simulation step. As it is, we're probably in shutdown mode @@ -213,6 +233,15 @@ void PhysicalEntitySimulation::clearEntities() { EntitySimulation::clearEntities(); } +void PhysicalEntitySimulation::queueEraseDomainEntities(const SetOfEntities& domainEntities) const { + if (_entityPacketSender) { + for (auto domainEntity : domainEntities) { + assert(domainEntity->isDomainEntity()); + _entityPacketSender->queueEraseEntityMessage(domainEntity->getID()); + } + } +} + // virtual void PhysicalEntitySimulation::prepareEntityForDelete(EntityItemPointer entity) { // DANGER! this can be called on any thread diff --git a/libraries/physics/src/PhysicalEntitySimulation.h b/libraries/physics/src/PhysicalEntitySimulation.h index 7efb000050..869ad1f108 100644 --- a/libraries/physics/src/PhysicalEntitySimulation.h +++ b/libraries/physics/src/PhysicalEntitySimulation.h @@ -63,19 +63,20 @@ public: void removeDynamic(const QUuid dynamicID) override; void applyDynamicChanges() override; - virtual void takeDeadEntities(SetOfEntities& deadEntities) override; void takeDeadAvatarEntities(SetOfEntities& deadEntities); virtual void clearEntities() override; + void queueEraseDomainEntities(const SetOfEntities& domainEntities) const override; signals: void entityCollisionWithEntity(const EntityItemID& idA, const EntityItemID& idB, const Collision& collision); protected: // only called by EntitySimulation // overrides for EntitySimulation - virtual void addEntityToInternalLists(EntityItemPointer entity) override; - virtual void removeEntityFromInternalLists(EntityItemPointer entity) override; + void addEntityToInternalLists(EntityItemPointer entity) override; + void removeEntityFromInternalLists(EntityItemPointer entity) override; void processChangedEntity(const EntityItemPointer& entity) override; + void processDeadEntities() override; void removeOwnershipData(EntityMotionState* motionState); void clearOwnershipData(); diff --git a/tests/octree/src/ModelTests.cpp b/tests/octree/src/ModelTests.cpp index f3e17a56a5..1ab32217d9 100644 --- a/tests/octree/src/ModelTests.cpp +++ b/tests/octree/src/ModelTests.cpp @@ -363,7 +363,8 @@ void EntityTests::entityTreeTests(bool verbose) { } quint64 startDelete = usecTimestampNow(); - tree.deleteEntity(entityID); + bool force = true; + tree.deleteEntity(entityID, force); quint64 endDelete = usecTimestampNow(); totalElapsedDelete += (endDelete - startDelete); @@ -446,7 +447,9 @@ void EntityTests::entityTreeTests(bool verbose) { } quint64 startDelete = usecTimestampNow(); - tree.deleteEntities(entitiesToDelete); + bool force = true; + bool ignoreWarnings = true; + tree.deleteEntitiesByID(entitiesToDelete, force, ignoreWarnings); quint64 endDelete = usecTimestampNow(); totalElapsedDelete += (endDelete - startDelete);