Merge pull request #14627 from AndrewMeadows/fewer-avatar-entity-updates

case 20304: please don't flood the avatar-mixer with unnecessary AvatarEntity udpates

interface/src/AvatarBookmarks.cpp

@@ -60,7 +60,6 @@ void addAvatarEntities(const QVariantList& avatarEntities) {
         entityProperties.setParentID(myNodeID);
         entityProperties.setEntityHostType(entity::HostType::AVATAR);
         entityProperties.setOwningAvatarID(myNodeID);
-        entityProperties.setSimulationOwner(myNodeID, AVATAR_ENTITY_SIMULATION_PRIORITY);
         entityProperties.markAllChanged();
 
         EntityItemID id = EntityItemID(QUuid::createUuid());

interface/src/ui/overlays/Base3DOverlay.cpp

@@ -27,6 +27,9 @@ Base3DOverlay::Base3DOverlay() :
     _drawInFront(false),
     _drawHUDLayer(false)
 {
+    // HACK: queryAACube stuff not actually relevant for 3DOverlays, and by setting _queryAACubeSet true here
+    // we can avoid incorrect evaluation for sending updates for entities with 3DOverlays children.
+    _queryAACubeSet = true;
 }
 
 Base3DOverlay::Base3DOverlay(const Base3DOverlay* base3DOverlay) :
@@ -41,6 +44,9 @@ Base3DOverlay::Base3DOverlay(const Base3DOverlay* base3DOverlay) :
     _isVisibleInSecondaryCamera(base3DOverlay->_isVisibleInSecondaryCamera)
 {
     setTransform(base3DOverlay->getTransform());
+    // HACK: queryAACube stuff not actually relevant for 3DOverlays, and by setting _queryAACubeSet true here
+    // we can avoid incorrect evaluation for sending updates for entities with 3DOverlays children.
+    _queryAACubeSet = true;
 }
 
 QVariantMap convertOverlayLocationFromScriptSemantics(const QVariantMap& properties, bool scalesWithParent) {
@@ -209,6 +215,7 @@ void Base3DOverlay::setProperties(const QVariantMap& originalProperties) {
             transaction.updateItem(itemID);
             scene->enqueueTransaction(transaction);
         }
+        _queryAACubeSet = true; // HACK: just in case some SpatiallyNestable code accidentally set it false
     }
 }
 

interface/src/ui/overlays/Base3DOverlay.h

@@ -25,6 +25,7 @@ public:
     Base3DOverlay(const Base3DOverlay* base3DOverlay);
 
     void setVisible(bool visible) override;
+    bool queryAACubeNeedsUpdate() const override { return false; } // HACK: queryAACube not relevant for Overlays
 
     virtual OverlayID getOverlayID() const override { return OverlayID(getID().toString()); }
     void setOverlayID(OverlayID overlayID) override { setID(overlayID); }

libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp

@@ -394,10 +394,6 @@ void Avatar::updateAvatarEntities() {
             properties.setEntityHostType(entity::HostType::AVATAR);
             properties.setOwningAvatarID(getID());
 
-            // 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.
-            properties.setSimulationOwner(getID(), AVATAR_ENTITY_SIMULATION_PRIORITY);
-
             if (properties.getParentID() == AVATAR_SELF_ID) {
                 properties.setParentID(getID());
             }

libraries/entities/src/EntityItemProperties.h

@@ -157,7 +157,7 @@ public:
     DEFINE_PROPERTY(PROP_CREATED, Created, created, quint64, UNKNOWN_CREATED_TIME);
     DEFINE_PROPERTY_REF(PROP_LAST_EDITED_BY, LastEditedBy, lastEditedBy, QUuid, ENTITY_ITEM_DEFAULT_LAST_EDITED_BY);
     DEFINE_PROPERTY_REF_ENUM(PROP_ENTITY_HOST_TYPE, EntityHostType, entityHostType, entity::HostType, entity::HostType::DOMAIN);
-    DEFINE_PROPERTY_REF(PROP_OWNING_AVATAR_ID, OwningAvatarID, owningAvatarID, QUuid, UNKNOWN_ENTITY_ID);
+    DEFINE_PROPERTY_REF_WITH_SETTER(PROP_OWNING_AVATAR_ID, OwningAvatarID, owningAvatarID, QUuid, UNKNOWN_ENTITY_ID);
     DEFINE_PROPERTY_REF(PROP_PARENT_ID, ParentID, parentID, QUuid, UNKNOWN_ENTITY_ID);
     DEFINE_PROPERTY_REF(PROP_PARENT_JOINT_INDEX, ParentJointIndex, parentJointIndex, quint16, -1);
     DEFINE_PROPERTY_REF(PROP_QUERY_AA_CUBE, QueryAACube, queryAACube, AACube, AACube());
@@ -499,6 +499,16 @@ void EntityPropertyInfoFromScriptValue(const QScriptValue& object, EntityPropert
 inline void EntityItemProperties::setPosition(const glm::vec3& value)
                     { _position = glm::clamp(value, (float)-HALF_TREE_SCALE, (float)HALF_TREE_SCALE); _positionChanged = true; }
 
+inline void EntityItemProperties::setOwningAvatarID(const QUuid& id) {
+    _owningAvatarID = id;
+    if (!_owningAvatarID.isNull()) {
+        // for AvatarEntities 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.
+        setSimulationOwner(_owningAvatarID, AVATAR_ENTITY_SIMULATION_PRIORITY);
+    }
+    _owningAvatarIDChanged = true;
+}
+
 QDebug& operator<<(QDebug& dbg, const EntityPropertyFlags& f);
 
 inline QDebug operator<<(QDebug debug, const EntityItemProperties& properties) {

libraries/entities/src/SimulationOwner.h

@@ -96,7 +96,7 @@ const uint8_t RECRUIT_SIMULATION_PRIORITY = VOLUNTEER_SIMULATION_PRIORITY + 1;
 // When poking objects with scripts an observer will bid at SCRIPT_EDIT priority.
 const uint8_t SCRIPT_GRAB_SIMULATION_PRIORITY = 128;
 const uint8_t SCRIPT_POKE_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY - 1;
-const uint8_t AVATAR_ENTITY_SIMULATION_PRIORITY = SCRIPT_GRAB_SIMULATION_PRIORITY + 1;
+const uint8_t AVATAR_ENTITY_SIMULATION_PRIORITY = 255;
 
 // PERSONAL priority (needs a better name) is the level at which a simulation observer owns its own avatar
 // which really just means: things that collide with it will be bid at a priority level one lower

libraries/physics/src/EntityMotionState.cpp

@@ -306,6 +306,8 @@ const btCollisionShape* EntityMotionState::computeNewShape() {
     return getShapeManager()->getShape(shapeInfo);
 }
 
+const uint8_t MAX_NUM_INACTIVE_UPDATES = 20;
+
 bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) {
     // NOTE: this method is only ever called when the entity simulation is locally owned
     DETAILED_PROFILE_RANGE(simulation_physics, "CheckOutOfSync");
@@ -315,15 +317,10 @@ bool EntityMotionState::remoteSimulationOutOfSync(uint32_t simulationStep) {
     // TODO: need to be able to detect when logic dictates we *decrease* priority
     // WIP: print info whenever _bidPriority mismatches what is known to the entity
 
-    if (_entity->dynamicDataNeedsTransmit()) {
-        return true;
-    }
-
     int numSteps = simulationStep - _lastStep;
     float dt = (float)(numSteps) * PHYSICS_ENGINE_FIXED_SUBSTEP;
 
     if (_numInactiveUpdates > 0) {
-        const uint8_t MAX_NUM_INACTIVE_UPDATES = 20;
         if (_numInactiveUpdates > MAX_NUM_INACTIVE_UPDATES) {
             // clear local ownership (stop sending updates) and let the server clear itself
             _entity->clearSimulationOwnership();
@@ -451,8 +448,13 @@ void EntityMotionState::updateSendVelocities() {
         if (!_body->isKinematicObject()) {
             clearObjectVelocities();
         }
-        // we pretend we sent the inactive update for this object
+        if (_entity->getEntityHostType() == entity::HostType::AVATAR) {
-        _numInactiveUpdates = 1;
+            // AvatarEntities only ever need to send one update (their updates are sent over a lossless protocol)
+            // so we set the count to the max to prevent resends
+            _numInactiveUpdates = MAX_NUM_INACTIVE_UPDATES;
+        } else {
+            ++_numInactiveUpdates;
+        }
     } else {
         glm::vec3 gravity = _entity->getGravity();