From 718ecea404459d9eb779ada40c5b99e1103fc7b2 Mon Sep 17 00:00:00 2001
From: Zach Fox <fox@highfidelity.io>
Date: Mon, 27 Feb 2017 16:10:53 -0800
Subject: [PATCH] Potential non-spammy solution using pseudo-state

---
 .../src/avatars/AvatarMixerSlave.cpp          | 31 ++++++++++++-------
 .../src/avatars/AvatarMixerSlave.h            | 12 +++++++
 libraries/avatars/src/AvatarData.h            |  4 +--
 3 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp
index 88b363b73a..15cf89754e 100644
--- a/assignment-client/src/avatars/AvatarMixerSlave.cpp
+++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp
@@ -80,16 +80,6 @@ int AvatarMixerSlave::sendIdentityPacket(const AvatarMixerClientData* nodeData,
 
 static const int AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND = 45;
 
-// FIXME - There is some old logic (unchanged as of 2/17/17) that randomly decides to send an identity
-// packet. That logic had the following comment about the constants it uses...
-//
-//         An 80% chance of sending a identity packet within a 5 second interval.
-//         assuming 60 htz update rate.
-//
-// Assuming the calculation of the constant is in fact correct for 80% and 60hz and 5 seconds (an assumption
-// that I have not verified) then the constant is definitely wrong now, since we send at 45hz.
-const float IDENTITY_SEND_PROBABILITY = 1.0f / 187.0f;
-
 void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) {
     quint64 start = usecTimestampNow();
 
@@ -150,6 +140,23 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) {
                 sizeof(AvatarDataPacket::AudioLoudness);
         }
 
+        if (PALIsOpen) {
+            if (_identitySendProbability == DEFAULT_IDENTITY_SEND_PROBABILITY)
+            {
+                // The client has just opened the PAL. Force all identity packets to be sent to
+                // this client.
+                _identitySendProbability = 1.0f;
+            } else {
+                // The user recently opened the PAL, but we've already gone through the above conditional.
+                // We want to receive identity updates more often than default when the PAL is open
+                // to be more confident that the user will see the most up-to-date information in the PAL.
+                _identitySendProbability = DEFAULT_IDENTITY_SEND_PROBABILITY * 2;
+            }
+        } else {
+            // If the PAL is closed, reset the identitySendProbability to the default.
+            _identitySendProbability = DEFAULT_IDENTITY_SEND_PROBABILITY;
+        }
+
         // setup a PacketList for the avatarPackets
         auto avatarPacketList = NLPacketList::create(PacketType::BulkAvatarData);
 
@@ -318,9 +325,9 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) {
                 && otherNodeData->getIdentityChangeTimestamp().time_since_epoch().count() > 0
                 && (forceSend
                 || otherNodeData->getIdentityChangeTimestamp() > _lastFrameTimestamp
-                || distribution(generator) < IDENTITY_SEND_PROBABILITY)) ||
+                || distribution(generator) < _identitySendProbability)) ||
                 // Also make sure we send identity packets if the PAL is open.
-                (PALIsOpen || getsAnyIgnored)) {
+                ((PALIsOpen || getsAnyIgnored) && distribution(generator) < _identitySendProbability)) {
 
                 identityBytesSent += sendIdentityPacket(otherNodeData, node);
             }
diff --git a/assignment-client/src/avatars/AvatarMixerSlave.h b/assignment-client/src/avatars/AvatarMixerSlave.h
index 04141d9d72..2fa5fa4484 100644
--- a/assignment-client/src/avatars/AvatarMixerSlave.h
+++ b/assignment-client/src/avatars/AvatarMixerSlave.h
@@ -101,6 +101,18 @@ private:
     float _maxKbpsPerNode { 0.0f };
     float _throttlingRatio { 0.0f };
 
+
+    // FIXME - There is some old logic (unchanged as of 2/17/17) that randomly decides to send an identity
+    // packet. That logic had the following comment about the constants it uses...
+    //
+    //         An 80% chance of sending a identity packet within a 5 second interval.
+    //         assuming 60 htz update rate.
+    //
+    // Assuming the calculation of the constant is in fact correct for 80% and 60hz and 5 seconds (an assumption
+    // that I have not verified) then the constant is definitely wrong now, since we send at 45hz.
+    const float DEFAULT_IDENTITY_SEND_PROBABILITY = 1.0f / 187.0f;
+    float _identitySendProbability = DEFAULT_IDENTITY_SEND_PROBABILITY;
+
     AvatarMixerSlaveStats _stats;
 };
 
diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h
index ac6c2fcbe0..f0759aedbd 100644
--- a/libraries/avatars/src/AvatarData.h
+++ b/libraries/avatars/src/AvatarData.h
@@ -376,11 +376,11 @@ public:
 
     typedef enum { 
         NoData,
+        PALMinimum,
         MinimumData, 
         CullSmallData,
         IncludeSmallData,
-        SendAllData,
-        PALMinimum
+        SendAllData
     } AvatarDataDetail;
 
     virtual QByteArray toByteArrayStateful(AvatarDataDetail dataDetail);