comment and naming cleanup

assignment-client/src/avatars/AvatarMixerClientData.cpp

@@ -184,42 +184,47 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message,
 }
 
 void AvatarMixerClientData::processBulkAvatarTraitsAckMessage(ReceivedMessage& message) {
+    // Avatar Traits flow control marks each outgoing avatar traits packet with a
+    // sequence number. The mixer caches the traits sent in the traits packet.
+    // Until an ack with the sequence number comes back, all updates to _traits
+    // in that packet_ are ignored.  Updates to traits not in that packet will
+    // be sent.
+
     // Look up the avatar/trait data associated with this ack and update the 'last ack' list
     // with it.
     AvatarTraits::TraitMessageSequence seq;
     message.readPrimitive(&seq);
-    auto sentAvatarTraitVersions = _pendingTraitVersions.find(seq);
+    auto sentAvatarTraitVersions = _perNodePendingTraitVersions.find(seq);
-    if (sentAvatarTraitVersions != _pendingTraitVersions.end()) {
+    if (sentAvatarTraitVersions != _perNodePendingTraitVersions.end()) {
-        // Note, this is not a simple move of the pending traits
+        for (auto& perNodeTraitVersions : sentAvatarTraitVersions->second) {
-        // to the acked traits.  Instead, it's a copy where existing
+            auto& nodeId = perNodeTraitVersions.first;
-        // trait versions in the acked hash are retained for traits not
+            auto& traitVersions = perNodeTraitVersions.second;
-        // included in the pending hash
+            // For each trait that was sent in the traits packet,
-        for (auto& nodeTraitVersions : sentAvatarTraitVersions->second) {
+            // update the 'acked' trait version.  Traits not
-            auto& nodeId = nodeTraitVersions.first;
+            // sent in the traits packet keep their version.
-            auto& versions = nodeTraitVersions.second;
+
-            auto simpleReceivedIt = versions.simpleCBegin();
+            // process simple traits
-            while (simpleReceivedIt != versions.simpleCEnd()) {
+            auto simpleReceivedIt = traitVersions.simpleCBegin();
-                auto traitType = static_cast<AvatarTraits::TraitType>(std::distance(versions.simpleCBegin(), simpleReceivedIt));
+            while (simpleReceivedIt != traitVersions.simpleCEnd()) {
-                _ackedTraitVersions[nodeId][traitType] = *simpleReceivedIt;
+                auto traitType = static_cast<AvatarTraits::TraitType>(std::distance(traitVersions.simpleCBegin(), simpleReceivedIt));
+                _perNodeAckedTraitVersions[nodeId][traitType] = *simpleReceivedIt;
                 simpleReceivedIt++;
             }
 
-            // enumerate the sent instanced trait versions
+            // process instanced traits
-            auto instancedSentIt = versions.instancedCBegin();
+            auto instancedSentIt = traitVersions.instancedCBegin();
-            while (instancedSentIt != versions.instancedCEnd()) {
+            while (instancedSentIt != traitVersions.instancedCEnd()) {
                 auto traitType = instancedSentIt->traitType;
-                // get or create the sent trait versions for this trait type
 
-                // enumerate each sent instance
                 for (auto& sentInstance : instancedSentIt->instances) {
                     auto instanceID = sentInstance.id;
                     const auto sentVersion = sentInstance.value;
-                    _ackedTraitVersions[nodeId].instanceInsert(traitType, instanceID, sentVersion);
+                    _perNodeAckedTraitVersions[nodeId].instanceInsert(traitType, instanceID, sentVersion);
                 }
                 instancedSentIt++;
             }
         }
-        _pendingTraitVersions.erase(sentAvatarTraitVersions);
+        _perNodePendingTraitVersions.erase(sentAvatarTraitVersions);
     } else {
         // This can happen either the BulkAvatarTraits was sent with no simple traits,
         // or if the avatar mixer restarts while there are pending
@@ -334,7 +339,7 @@ void AvatarMixerClientData::removeFromRadiusIgnoringSet(const QUuid& other) {
 
 void AvatarMixerClientData::resetSentTraitData(Node::LocalID nodeLocalID) {
     _lastSentTraitsTimestamps[nodeLocalID] = TraitsCheckTimestamp();
-    _sentTraitVersions[nodeLocalID].reset();
+    _perNodeSentTraitVersions[nodeLocalID].reset();
 }
 
 void AvatarMixerClientData::readViewFrustumPacket(const QByteArray& message) {
@@ -390,5 +395,5 @@ void AvatarMixerClientData::cleanupKilledNode(const QUuid&, Node::LocalID nodeLo
     removeLastBroadcastSequenceNumber(nodeLocalID);
     removeLastBroadcastTime(nodeLocalID);
     _lastSentTraitsTimestamps.erase(nodeLocalID);
-    _sentTraitVersions.erase(nodeLocalID);
+    _perNodeSentTraitVersions.erase(nodeLocalID);
 }

assignment-client/src/avatars/AvatarMixerClientData.h

@@ -42,7 +42,7 @@ public:
     AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID);
     virtual ~AvatarMixerClientData() {}
     using HRCTime = p_high_resolution_clock::time_point;
-    using NodeTraitVersions = std::unordered_map<Node::LocalID, AvatarTraits::TraitVersions>;
+    using PerNodeTraitVersions = std::unordered_map<Node::LocalID, AvatarTraits::TraitVersions>;
 
     int parseData(ReceivedMessage& message) override;
     AvatarData& getAvatar() { return *_avatar; }
@@ -143,11 +143,11 @@ public:
     AvatarTraits::TraitMessageSequence getTraitsMessageSequence() const { return _currentTraitsMessageSequence; }
     AvatarTraits::TraitMessageSequence nextTraitsMessageSequence() { return ++_currentTraitsMessageSequence; }
     AvatarTraits::TraitVersions& getPendingTraitVersions(AvatarTraits::TraitMessageSequence seq, Node::LocalID otherId) {
-        return _pendingTraitVersions[seq][otherId];
+        return _perNodePendingTraitVersions[seq][otherId];
     }
 
-    AvatarTraits::TraitVersions& getLastSentTraitVersions(Node::LocalID otherAvatar) { return _sentTraitVersions[otherAvatar]; }
+    AvatarTraits::TraitVersions& getLastSentTraitVersions(Node::LocalID otherAvatar) { return _perNodeSentTraitVersions[otherAvatar]; }
-    AvatarTraits::TraitVersions& getLastAckedTraitVersions(Node::LocalID otherAvatar) { return _ackedTraitVersions[otherAvatar]; }
+    AvatarTraits::TraitVersions& getLastAckedTraitVersions(Node::LocalID otherAvatar) { return _perNodeAckedTraitVersions[otherAvatar]; }
 
     void resetSentTraitData(Node::LocalID nodeID);
 
@@ -194,11 +194,11 @@ private:
 
     AvatarTraits::TraitMessageSequence _currentTraitsMessageSequence{ 0 };
 
-    std::unordered_map<AvatarTraits::TraitMessageSequence, NodeTraitVersions> _pendingTraitVersions;
+    std::unordered_map<AvatarTraits::TraitMessageSequence, PerNodeTraitVersions> _perNodePendingTraitVersions;
 
     std::unordered_map<Node::LocalID, TraitsCheckTimestamp> _lastSentTraitsTimestamps;
-    NodeTraitVersions _sentTraitVersions;
+    PerNodeTraitVersions _perNodeSentTraitVersions;
-    NodeTraitVersions _ackedTraitVersions;
+    PerNodeTraitVersions _perNodeAckedTraitVersions;
 
     std::atomic_bool _isIgnoreRadiusEnabled { false };
 };

assignment-client/src/avatars/AvatarMixerSlave.cpp

@@ -85,11 +85,11 @@ qint64 AvatarMixerSlave::addTraitsNodeHeader(AvatarMixerClientData* listeningNod
                                              NLPacketList& traitsPacketList,
                                              qint64 bytesWritten) {
     if (bytesWritten == 0) {
-
         if (traitsPacketList.getNumPackets() == 0) {
+            // This is the beginning of the traits packet, write out the sequence number.
             bytesWritten += traitsPacketList.writePrimitive(listeningNodeData->nextTraitsMessageSequence());
         }
-        // add the avatar ID to mark the beginning of traits for this avatar
+        // This is the beginning of the traits for a node, write out the node id
         bytesWritten += traitsPacketList.write(sendingNodeData->getNodeID().toRfc4122());
     }
     return bytesWritten;
@@ -99,6 +99,12 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis
                                                       const AvatarMixerClientData* sendingNodeData,
                                                       NLPacketList& traitsPacketList) {
 
+    // Avatar Traits flow control marks each outgoing avatar traits packet with a
+    // sequence number. The mixer caches the traits sent in the traits packet.
+    // Until an ack with the sequence number comes back, all updates to _traits
+    // in that packet_ are ignored.  Updates to traits not in that packet will
+    // be sent.
+
     auto otherNodeLocalID = sendingNodeData->getNodeLocalID();
 
     // Perform a simple check with two server clock time points

libraries/avatars/src/AvatarHashMap.cpp

@@ -332,13 +332,15 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer<ReceivedMessage> mess
 
     message->readPrimitive(&seq);
 
-    // we have a mixer to send to, setup our set traits packet
     auto traitsAckPacket = NLPacket::create(PacketType::BulkAvatarTraitsAck, sizeof(AvatarTraits::TraitMessageSequence), true);
     traitsAckPacket->writePrimitive(seq);
     auto nodeList = DependencyManager::get<LimitedNodeList>();
     SharedNodePointer avatarMixer = nodeList->soloNodeOfType(NodeType::AvatarMixer);
-    nodeList->sendPacket(std::move(traitsAckPacket), *avatarMixer);
+    if (!avatarMixer.isNull()) {
-
+        // we have a mixer to send to, acknowledge that we received these
+        // traits.
+        nodeList->sendPacket(std::move(traitsAckPacket), *avatarMixer);
+    }
 
     while (message->getBytesLeftToRead()) {
         // read the avatar ID to figure out which avatar this is for