First pass at implementing Brad's simplification ideas

assignment-client/src/avatars/AvatarMixerClientData.cpp

@@ -65,15 +65,6 @@ int AvatarMixerClientData::parseData(ReceivedMessage& message) {
     // compute the offset to the data payload
     return _avatar->parseDataFromBuffer(message.readWithoutCopy(message.getBytesLeftToRead()));
 }
-
-bool AvatarMixerClientData::checkAndSetHasReceivedFirstPacketsFrom(const QUuid& uuid) {
-    if (_hasReceivedFirstPacketsFrom.find(uuid) == _hasReceivedFirstPacketsFrom.end()) {
-        _hasReceivedFirstPacketsFrom.insert(uuid);
-        return false;
-    }
-    return true;
-}
-
 uint64_t AvatarMixerClientData::getLastBroadcastTime(const QUuid& nodeUUID) const {
     // return the matching PacketSequenceNumber, or the default if we don't have it
     auto nodeMatch = _lastBroadcastTimes.find(nodeUUID);
@@ -103,7 +94,6 @@ void AvatarMixerClientData::ignoreOther(SharedNodePointer self, SharedNodePointe
             killPacket->writePrimitive(KillAvatarReason::YourAvatarEnteredTheirBubble);
         }
         DependencyManager::get<NodeList>()->sendUnreliablePacket(*killPacket, *self);
-        _hasReceivedFirstPacketsFrom.erase(other->getUUID());
     }
 }
 

assignment-client/src/avatars/AvatarMixerClientData.h

@@ -45,8 +45,6 @@ public:
     const AvatarData* getConstAvatarData() const { return _avatar.get(); }
     AvatarSharedPointer getAvatarSharedPointer() const { return _avatar; }
 
-    bool checkAndSetHasReceivedFirstPacketsFrom(const QUuid& uuid);
-
     uint16_t getLastBroadcastSequenceNumber(const QUuid& nodeUUID) const;
     void setLastBroadcastSequenceNumber(const QUuid& nodeUUID, uint16_t sequenceNumber)
         { _lastBroadcastSequenceNumbers[nodeUUID] = sequenceNumber; }
@@ -63,8 +61,8 @@ public:
 
     uint16_t getLastReceivedSequenceNumber() const { return _lastReceivedSequenceNumber; }
 
-    HRCTime getIdentityChangeTimestamp() const { return _identityChangeTimestamp; }
+    uint64_t getIdentityChangeTimestamp() const { return _identityChangeTimestamp; }
-    void flagIdentityChange() { _identityChangeTimestamp = p_high_resolution_clock::now(); }
+    void flagIdentityChange() { _identityChangeTimestamp = usecTimestampNow(); }
     bool getAvatarSessionDisplayNameMustChange() const { return _avatarSessionDisplayNameMustChange; }
     void setAvatarSessionDisplayNameMustChange(bool set = true) { _avatarSessionDisplayNameMustChange = set; }
 
@@ -139,7 +137,6 @@ private:
 
     uint16_t _lastReceivedSequenceNumber { 0 };
     std::unordered_map<QUuid, uint16_t> _lastBroadcastSequenceNumbers;
-    std::unordered_set<QUuid> _hasReceivedFirstPacketsFrom;
     std::unordered_map<QUuid, uint64_t> _lastBroadcastTimes;
 
     // this is a map of the last time we encoded an "other" avatar for
@@ -147,7 +144,7 @@ private:
     std::unordered_map<QUuid, quint64> _lastOtherAvatarEncodeTime;
     std::unordered_map<QUuid, QVector<JointData>> _lastOtherAvatarSentJoints;
 
-    HRCTime _identityChangeTimestamp;
+    uint64_t _identityChangeTimestamp;
     bool _avatarSessionDisplayNameMustChange{ false };
 
     int _numAvatarsSentLastFrame = 0;

assignment-client/src/avatars/AvatarMixerSlave.cpp

@@ -138,19 +138,6 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) {
             // Increase minimumBytesPerAvatar if the PAL is open
             minimumBytesPerAvatar += sizeof(AvatarDataPacket::AvatarGlobalPosition) +
                 sizeof(AvatarDataPacket::AudioLoudness);
-            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
@@ -313,18 +300,9 @@ void AvatarMixerSlave::broadcastAvatarData(const SharedNodePointer& node) {
 
             const AvatarMixerClientData* otherNodeData = reinterpret_cast<const AvatarMixerClientData*>(otherNode->getLinkedData());
 
-            // make sure we send out identity packets to and from new arrivals.
+            // If the time that the mixer sent AVATAR DATA about Avatar B to Avatar A is BEFORE OR EQUAL TO
-            bool forceSend = !nodeData->checkAndSetHasReceivedFirstPacketsFrom(otherNode->getUUID());
+            // the time that Avatar B flagged an IDENTITY DATA change, send IDENTITY DATA about Avatar B to Avatar A.
-
+            if (nodeData->getLastBroadcastTime(otherNode->getUUID()) <= otherNodeData->getIdentityChangeTimestamp()) {
-            // FIXME - this clause seems suspicious "... || otherNodeData->getIdentityChangeTimestamp() > _lastFrameTimestamp ..."
-            if ((!overBudget
-                && otherNodeData->getIdentityChangeTimestamp().time_since_epoch().count() > 0
-                && (forceSend
-                || otherNodeData->getIdentityChangeTimestamp() > _lastFrameTimestamp
-                || distribution(generator) < _identitySendProbability)) ||
-                // Also make sure we send identity packets if the PAL is open.
-                (PALIsOpen && distribution(generator) < _identitySendProbability)) {
-
                 identityBytesSent += sendIdentityPacket(otherNodeData, node);
             }
 

assignment-client/src/avatars/AvatarMixerSlave.h

@@ -101,18 +101,6 @@ 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;
 };