EntityItem::_owningAvatarID always AVATAR_SELF_ID for MyAvatar's entities

interface/src/avatar/MyAvatar.cpp

@@ -1585,7 +1585,12 @@ void MyAvatar::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFrom
 
 void MyAvatar::sanitizeAvatarEntityProperties(EntityItemProperties& properties) const {
     properties.setEntityHostType(entity::HostType::AVATAR);
-    properties.setOwningAvatarID(getID());
+
+    // Note: we store AVATAR_SELF_ID in EntityItem::_owningAvatarID and we usually
+    // store the actual sessionUUID in EntityItemProperties::_owningAvatarID (for JS
+    // consumption, for example).  However at this context we are preparing properties
+    // for outgoing packet, in which case we use AVATAR_SELF_ID.
+    properties.setOwningAvatarID(AVATAR_SELF_ID);
 
     // there's no entity-server to tell us we're the simulation owner, so always set the
     // simulationOwner to the owningAvatarID and a high priority.
@@ -4000,6 +4005,10 @@ float MyAvatar::getGravity() {
 void MyAvatar::setSessionUUID(const QUuid& sessionUUID) {
     QUuid oldSessionID = getSessionUUID();
     Avatar::setSessionUUID(sessionUUID);
+    bool sendPackets = !DependencyManager::get<NodeList>()->getSessionUUID().isNull();
+    if (!sendPackets) {
+        return;
+    }
     QUuid newSessionID = getSessionUUID();
     if (newSessionID != oldSessionID) {
         auto treeRenderer = DependencyManager::get<EntityTreeRenderer>();
@@ -4009,7 +4018,6 @@ void MyAvatar::setSessionUUID(const QUuid& sessionUUID) {
             _avatarEntitiesLock.withReadLock([&] {
                 avatarEntityIDs = _packedAvatarEntityData.keys();
             });
-            bool sendPackets = !DependencyManager::get<NodeList>()->getSessionUUID().isNull();
             EntityEditPacketSender* packetSender = qApp->getEntityEditPacketSender();
             entityTree->withWriteLock([&] {
                 for (const auto& entityID : avatarEntityIDs) {
@@ -4017,11 +4025,9 @@ void MyAvatar::setSessionUUID(const QUuid& sessionUUID) {
                     if (!entity) {
                         continue;
                     }
-                    // update OwningAvatarID so entity can be identified as "ours" later
-                    entity->setOwningAvatarID(newSessionID);
                     // NOTE: each attached AvatarEntity already have the correct updated parentID
                     // via magic in SpatiallyNestable, hence we check against newSessionID
-                    if (sendPackets && entity->getParentID() == newSessionID) {
+                    if (entity->getParentID() == newSessionID) {
                         // but when we have a real session and the AvatarEntity is parented to MyAvatar
                         // we need to update the "packedAvatarEntityData" sent to the avatar-mixer
                         // because it contains a stale parentID somewhere deep inside

libraries/entities-renderer/src/EntityTreeRenderer.cpp

@@ -221,7 +221,7 @@ void EntityTreeRenderer::stopDomainAndNonOwnedEntities() {
             EntityItemPointer entityItem = getTree()->findEntityByEntityItemID(entityID);
 
             if (entityItem && !entityItem->getScript().isEmpty()) {
-                if (!(entityItem->isLocalEntity() || (entityItem->isAvatarEntity() && entityItem->getOwningAvatarID() == getTree()->getMyAvatarSessionUUID()))) {
+                if (!(entityItem->isLocalEntity() || entityItem->isMyAvatarEntity())) {
                     if (_currentEntitiesInside.contains(entityID)) {
                         _entitiesScriptEngine->callEntityScriptMethod(entityID, "leaveEntity");
                     }
@@ -235,7 +235,6 @@ void EntityTreeRenderer::stopDomainAndNonOwnedEntities() {
 void EntityTreeRenderer::clearDomainAndNonOwnedEntities() {
     stopDomainAndNonOwnedEntities();
 
-    auto sessionUUID = getTree()->getMyAvatarSessionUUID();
     std::unordered_map<EntityItemID, EntityRendererPointer> savedEntities;
     std::unordered_set<EntityRendererPointer> savedRenderables;
     // remove all entities from the scene
@@ -244,7 +243,7 @@ void EntityTreeRenderer::clearDomainAndNonOwnedEntities() {
         for (const auto& entry :  _entitiesInScene) {
             const auto& renderer = entry.second;
             const EntityItemPointer& entityItem = renderer->getEntity();
-            if (!(entityItem->isLocalEntity() || (entityItem->isAvatarEntity() && entityItem->getOwningAvatarID() == sessionUUID))) {
+            if (!(entityItem->isLocalEntity() || entityItem->isMyAvatarEntity())) {
                 fadeOutRenderable(renderer);
             } else {
                 savedEntities[entry.first] = entry.second;
@@ -256,6 +255,7 @@ void EntityTreeRenderer::clearDomainAndNonOwnedEntities() {
     _renderablesToUpdate = savedRenderables;
     _entitiesInScene = savedEntities;
 
+    auto sessionUUID = getTree()->getMyAvatarSessionUUID();
     if (_layeredZones.clearDomainAndNonOwnedZones(sessionUUID)) {
         applyLayeredZones();
     }
@@ -678,7 +678,7 @@ void EntityTreeRenderer::leaveDomainAndNonOwnedEntities() {
         QSet<EntityItemID> currentEntitiesInsideToSave;
         foreach (const EntityItemID& entityID, _currentEntitiesInside) {
             EntityItemPointer entityItem = getTree()->findEntityByEntityItemID(entityID);
-            if (!(entityItem->isLocalEntity() || (entityItem->isAvatarEntity() && entityItem->getOwningAvatarID() == getTree()->getMyAvatarSessionUUID()))) {
+            if (!(entityItem->isLocalEntity() || entityItem->isMyAvatarEntity())) {
                 emit leaveEntity(entityID);
                 if (_entitiesScriptEngine) {
                     _entitiesScriptEngine->callEntityScriptMethod(entityID, "leaveEntity");
@@ -1216,7 +1216,7 @@ bool EntityTreeRenderer::LayeredZones::clearDomainAndNonOwnedZones(const QUuid&
     auto it = begin();
     while (it != end()) {
         auto zone = it->zone.lock();
-        if (!zone || !(zone->isLocalEntity() || (zone->isAvatarEntity() && zone->getOwningAvatarID() == sessionUUID))) {
+        if (!zone || !(zone->isLocalEntity() || zone->isMyAvatarEntity())) {
             zonesChanged = true;
             it = erase(it);
         } else {

libraries/entities-renderer/src/RenderableEntityItem.cpp

@@ -43,6 +43,7 @@ const Transform& EntityRenderer::getModelTransform() const {
 
 void EntityRenderer::makeStatusGetters(const EntityItemPointer& entity, Item::Status::Getters& statusGetters) {
     auto nodeList = DependencyManager::get<NodeList>();
+    // DANGER: nodeList->getSessionUUID() will return null id when not connected to domain.
     const QUuid& myNodeID = nodeList->getSessionUUID();
 
     statusGetters.push_back([entity]() -> render::Item::Status::Value {
@@ -103,9 +104,9 @@ void EntityRenderer::makeStatusGetters(const EntityItemPointer& entity, Item::St
             (unsigned char)render::Item::Status::Icon::HAS_ACTIONS);
     });
 
-    statusGetters.push_back([entity, myNodeID] () -> render::Item::Status::Value {
+    statusGetters.push_back([entity] () -> render::Item::Status::Value {
         if (entity->isAvatarEntity()) {
-            if (entity->getOwningAvatarID() == myNodeID) {
+            if (entity->isMyAvatarEntity()) {
                 return render::Item::Status::Value(1.0f, render::Item::Status::Value::GREEN,
                     (unsigned char)render::Item::Status::Icon::ENTITY_HOST_TYPE);
             } else {

libraries/entities/src/EntityEditPacketSender.cpp

@@ -73,8 +73,12 @@ void EntityEditPacketSender::queueEditEntityMessage(PacketType type,
     if (properties.getEntityHostType() == entity::HostType::AVATAR) {
         if (!_myAvatar) {
             qCWarning(entities) << "Suppressing entity edit message: cannot send avatar entity edit with no myAvatar";
-        } else if (properties.getOwningAvatarID() == _myAvatar->getID()) {
+        } else if (properties.getOwningAvatarID() == _myAvatar->getID() || properties.getOwningAvatarID() == AVATAR_SELF_ID) {
-            // this is an avatar-based entity --> update our avatar-data rather than sending to the entity-server
+            // this is a local avatar-entity --> update our avatar-data rather than sending to the entity-server
+            // Note: we store AVATAR_SELF_ID in EntityItem::_owningAvatarID and we usually
+            // store the actual sessionUUID in EntityItemProperties::_owningAvatarID.
+            // However at this context we check for both cases just in case.  Really we just want to know
+            // where to route the data: entity-server or avatar-mixer.
             queueEditAvatarEntityMessage(entityTree, entityItemID);
         } else {
             qCWarning(entities) << "Suppressing entity edit message: cannot send avatar entity edit for another avatar";

libraries/entities/src/EntityItem.cpp

@@ -1347,7 +1347,7 @@ EntityItemProperties EntityItem::getProperties(const EntityPropertyFlags& desire
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(created, getCreated);
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(lastEditedBy, getLastEditedBy);
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(entityHostType, getEntityHostType);
-    COPY_ENTITY_PROPERTY_TO_PROPERTIES(owningAvatarID, getOwningAvatarID);
+    COPY_ENTITY_PROPERTY_TO_PROPERTIES(owningAvatarID, getOwningAvatarIDForProperties);
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(queryAACube, getQueryAACube);
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(canCastShadow, getCanCastShadow);
     COPY_ENTITY_PROPERTY_TO_PROPERTIES(isVisibleInSecondaryCamera, isVisibleInSecondaryCamera);
@@ -3398,6 +3398,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
     properties.setEntityHostType(getEntityHostType());
     properties.setOwningAvatarID(getOwningAvatarID());
     setLastBroadcast(now); // for debug/physics status icons
@@ -3409,9 +3410,27 @@ bool EntityItem::isWearable() const {
 }
 
 bool EntityItem::isMyAvatarEntity() const {
-    return _hostType == entity::HostType::AVATAR && Physics::getSessionUUID() == _owningAvatarID;
+    return _hostType == entity::HostType::AVATAR && AVATAR_SELF_ID == _owningAvatarID;
 };
 
+QUuid EntityItem::getOwningAvatarIDForProperties() const {
+    if (isMyAvatarEntity()) {
+        // NOTE: we always store AVATAR_SELF_ID for MyAvatar's avatar entities,
+        // however for EntityItemProperties to be consumed by outside contexts (e.g. JS)
+        // we use the actual "sessionUUID" which is conveniently cached in the Physics namespace
+        return Physics::getSessionUUID();
+    }
+    return _owningAvatarID;
+}
+
+void EntityItem::setOwningAvatarID(const QUuid& owningAvatarID) {
+    if (!owningAvatarID.isNull() && owningAvatarID == Physics::getSessionUUID()) {
+        _owningAvatarID = AVATAR_SELF_ID;
+    } else {
+        _owningAvatarID = owningAvatarID;
+    }
+}
+
 void EntityItem::addGrab(GrabPointer grab) {
     enableNoBootstrap();
     SpatiallyNestable::addGrab(grab);

libraries/entities/src/EntityItem.h

@@ -514,7 +514,8 @@ public:
 
     // if this entity is an avatar entity, which avatar is it associated with?
     QUuid getOwningAvatarID() const { return _owningAvatarID; }
-    virtual void setOwningAvatarID(const QUuid& owningAvatarID) { _owningAvatarID = owningAvatarID; }
+    QUuid getOwningAvatarIDForProperties() const;
+    void setOwningAvatarID(const QUuid& owningAvatarID);
 
     virtual bool wantsHandControllerPointerEvents() const { return false; }
     virtual bool wantsKeyboardFocus() const { return false; }

libraries/entities/src/EntityScriptingInterface.cpp

@@ -480,17 +480,11 @@ QUuid EntityScriptingInterface::addEntityInternal(const EntityItemProperties& pr
 
     _activityTracking.addedEntityCount++;
 
-    auto nodeList = DependencyManager::get<NodeList>();
-    auto sessionID = nodeList->getSessionUUID();
-
     EntityItemProperties propertiesWithSimID = properties;
     propertiesWithSimID.setEntityHostType(entityHostType);
     if (entityHostType == entity::HostType::AVATAR) {
-        if (sessionID.isNull()) {
+        // only allow adding our own avatar entities from script
-            // null sessionID is unacceptable in this case
+        propertiesWithSimID.setOwningAvatarID(AVATAR_SELF_ID);
-            sessionID = AVATAR_SELF_ID;
-        }
-        propertiesWithSimID.setOwningAvatarID(sessionID);
     } else if (entityHostType == entity::HostType::LOCAL) {
         // For now, local entities are always collisionless
         // TODO: create a separate, local physics simulation that just handles local entities (and MyAvatar?)
@@ -498,6 +492,8 @@ QUuid EntityScriptingInterface::addEntityInternal(const EntityItemProperties& pr
     }
 
     // the created time will be set in EntityTree::addEntity by recordCreationTime()
+    auto nodeList = DependencyManager::get<NodeList>();
+    auto sessionID = nodeList->getSessionUUID();
     propertiesWithSimID.setLastEditedBy(sessionID);
 
     bool scalesWithParent = propertiesWithSimID.getScalesWithParent();
@@ -805,7 +801,7 @@ QUuid EntityScriptingInterface::editEntity(const QUuid& id, const EntityItemProp
             return;
         }
 
-        if (entity->isAvatarEntity() && entity->getOwningAvatarID() != sessionID && entity->getOwningAvatarID() != AVATAR_SELF_ID) {
+        if (entity->isAvatarEntity() && !entity->isMyAvatarEntity()) {
             // don't edit other avatar's avatarEntities
             properties = EntityItemProperties();
             return;
@@ -978,10 +974,9 @@ void EntityScriptingInterface::deleteEntity(const QUuid& id) {
         _entityTree->withWriteLock([&] {
             EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID);
             if (entity) {
-
                 auto nodeList = DependencyManager::get<NodeList>();
                 const QUuid myNodeID = nodeList->getSessionUUID();
-                if (entity->isAvatarEntity() && entity->getOwningAvatarID() != myNodeID) {
+                if (entity->isAvatarEntity() && !entity->isMyAvatarEntity()) {
                     // don't delete other avatar's avatarEntities
                     shouldSendDeleteToServer = false;
                     return;
@@ -990,7 +985,7 @@ void EntityScriptingInterface::deleteEntity(const QUuid& id) {
                 if (entity->getLocked()) {
                     shouldSendDeleteToServer = false;
                 } else {
-                    // only delete local entities, server entities will round trip through the server filters
+                    // only delete non-domain entities, domain entities will round trip through the server filters
                     if (!entity->isDomainEntity() || _entityTree->isServerlessMode()) {
                         shouldSendDeleteToServer = false;
                         _entityTree->deleteEntity(entityID);
@@ -1671,7 +1666,7 @@ bool EntityScriptingInterface::actionWorker(const QUuid& entityID,
             return;
         }
 
-        if (entity->isAvatarEntity() && entity->getOwningAvatarID() != myNodeID) {
+        if (entity->isAvatarEntity() && !entity->isMyAvatarEntity()) {
             return;
         }
 

libraries/entities/src/EntityTree.cpp

@@ -99,7 +99,7 @@ void EntityTree::eraseDomainAndNonOwnedEntities() {
                 element->cleanupDomainAndNonOwnedEntities();
             }
 
-            if (entity->isLocalEntity() || (entity->isAvatarEntity() && entity->getOwningAvatarID() == getMyAvatarSessionUUID())) {
+            if (entity->isLocalEntity() || entity->isMyAvatarEntity()) {
                 savedEntities[entity->getEntityItemID()] = entity;
             } else {
                 int32_t spaceIndex = entity->getSpaceIndex();
@@ -121,7 +121,7 @@ void EntityTree::eraseDomainAndNonOwnedEntities() {
 
         foreach (EntityItemWeakPointer entityItem, _needsParentFixup) {
             auto entity = entityItem.lock();
-            if (entity && (entity->isLocalEntity() || (entity->isAvatarEntity() && entity->getOwningAvatarID() == getMyAvatarSessionUUID()))) {
+            if (entity && (entity->isLocalEntity() || entity->isMyAvatarEntity())) {
                 needParentFixup.push_back(entityItem);
             }
         }
@@ -688,6 +688,7 @@ void EntityTree::deleteEntities(QSet<EntityItemID> entityIDs, bool force, bool i
     // NOTE: callers must lock the tree before using this method
     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) {

libraries/entities/src/EntityTreeElement.cpp

@@ -705,7 +705,7 @@ void EntityTreeElement::cleanupDomainAndNonOwnedEntities() {
     withWriteLock([&] {
         EntityItems savedEntities;
         foreach(EntityItemPointer entity, _entityItems) {
-            if (!(entity->isLocalEntity() || (entity->isAvatarEntity() && entity->getOwningAvatarID() == getTree()->getMyAvatarSessionUUID()))) {
+            if (!(entity->isLocalEntity() || entity->isMyAvatarEntity())) {
                 entity->preDelete();
                 entity->_element = NULL;
             } else {

libraries/physics/src/EntityMotionState.cpp

@@ -62,7 +62,8 @@ EntityMotionState::EntityMotionState(btCollisionShape* shape, EntityItemPointer
     // rather than pass the legit shape pointer to the ObjectMotionState ctor above.
     setShape(shape);
 
-    if (_entity->isAvatarEntity() && _entity->getOwningAvatarID() != Physics::getSessionUUID()) {
+    // 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;
     }
@@ -407,8 +408,8 @@ bool EntityMotionState::shouldSendUpdate(uint32_t simulationStep) {
     // NOTE: we expect _entity and _body to be valid in this context, since shouldSendUpdate() is only called
     // after doesNotNeedToSendUpdate() returns false and that call should return 'true' if _entity or _body are NULL.
 
-    // this case is prevented by setting _ownershipState to UNOWNABLE in EntityMotionState::ctor
+    // this case is prevented by setting _ownershipState to OwnershipState::Unownable in EntityMotionState::ctor
-    assert(!(_entity->isAvatarEntity() && _entity->getOwningAvatarID() != Physics::getSessionUUID()));
+    assert(!(_entity->isAvatarEntity() && !_entity->isMyAvatarEntity()));
 
     if (_entity->getTransitingWithAvatar()) {
         return false;
@@ -747,6 +748,7 @@ 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()
@@ -768,6 +770,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();
 }
 
@@ -795,6 +798,12 @@ void EntityMotionState::initForOwned() {
     _ownershipState = EntityMotionState::OwnershipState::LocallyOwned;
 }
 
+void EntityMotionState::clearOwnershipState() {
+    if (_ownershipState != OwnershipState::Unownable) {
+        _ownershipState = OwnershipState::NotLocallyOwned;
+    }
+}
+
 void EntityMotionState::clearObjectVelocities() const {
     // If transform or velocities are flagged as dirty it means a network or scripted change
     // occured between the beginning and end of the stepSimulation() and we DON'T want to apply

libraries/physics/src/EntityMotionState.h

@@ -107,7 +107,7 @@ protected:
     uint64_t getNextBidExpiry() const { return _nextBidExpiry; }
     void initForBid();
     void initForOwned();
-    void clearOwnershipState() { _ownershipState = OwnershipState::NotLocallyOwned; }
+    void clearOwnershipState();
     void updateServerPhysicsVariables();
     bool remoteSimulationOutOfSync(uint32_t simulationStep);