From 65a7e15f5d1615044c1cc6157e3b66f3ad56253f Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Thu, 20 Dec 2018 14:49:03 -0800
Subject: [PATCH] don't send AvatarEntity updates when not necessary

---
 interface/src/AvatarBookmarks.cpp                |  1 -
 interface/src/ui/overlays/Base3DOverlay.cpp      |  7 +++++++
 interface/src/ui/overlays/Base3DOverlay.h        |  1 +
 .../src/avatars-renderer/Avatar.cpp              |  4 ----
 libraries/entities/src/EntityItemProperties.h    | 12 +++++++++++-
 libraries/entities/src/SimulationOwner.h         |  2 +-
 libraries/physics/src/EntityMotionState.cpp      | 16 +++++++++-------
 7 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/interface/src/AvatarBookmarks.cpp b/interface/src/AvatarBookmarks.cpp
index ee639f602d..1d003f19c1 100644
--- a/interface/src/AvatarBookmarks.cpp
+++ b/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());
diff --git a/interface/src/ui/overlays/Base3DOverlay.cpp b/interface/src/ui/overlays/Base3DOverlay.cpp
index eab7a96a4b..eb43e8cf45 100644
--- a/interface/src/ui/overlays/Base3DOverlay.cpp
+++ b/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
     }
 }
 
diff --git a/interface/src/ui/overlays/Base3DOverlay.h b/interface/src/ui/overlays/Base3DOverlay.h
index 6cc5182b56..daf15e676f 100644
--- a/interface/src/ui/overlays/Base3DOverlay.h
+++ b/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); }
diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp
index 50d3c568d9..598f552bcd 100644
--- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp
+++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp
@@ -393,10 +393,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());
             }
diff --git a/libraries/entities/src/EntityItemProperties.h b/libraries/entities/src/EntityItemProperties.h
index b741bb8ca4..9f635057d2 100644
--- a/libraries/entities/src/EntityItemProperties.h
+++ b/libraries/entities/src/EntityItemProperties.h
@@ -143,7 +143,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());
@@ -482,6 +482,16 @@ void EntityPropertyFlagsFromScriptValue(const QScriptValue& object, EntityProper
 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) {
diff --git a/libraries/entities/src/SimulationOwner.h b/libraries/entities/src/SimulationOwner.h
index 353255728c..bd444d28dd 100644
--- a/libraries/entities/src/SimulationOwner.h
+++ b/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
diff --git a/libraries/physics/src/EntityMotionState.cpp b/libraries/physics/src/EntityMotionState.cpp
index 4b635ef0be..cf098136d3 100644
--- a/libraries/physics/src/EntityMotionState.cpp
+++ b/libraries/physics/src/EntityMotionState.cpp
@@ -307,6 +307,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");
@@ -316,15 +318,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();
@@ -452,8 +449,13 @@ void EntityMotionState::updateSendVelocities() {
         if (!_body->isKinematicObject()) {
             clearObjectVelocities();
         }
-        // we pretend we sent the inactive update for this object
-        _numInactiveUpdates = 1;
+        if (_entity->getEntityHostType() == entity::HostType::AVATAR) {
+            // 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();