diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 37f71103d2..1da592b640 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/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); - if (sentAvatarTraitVersions != _pendingTraitVersions.end()) { - // Note, this is not a simple move of the pending traits - // to the acked traits. Instead, it's a copy where existing - // trait versions in the acked hash are retained for traits not - // included in the pending hash - for (auto& nodeTraitVersions : sentAvatarTraitVersions->second) { - auto& nodeId = nodeTraitVersions.first; - auto& versions = nodeTraitVersions.second; - auto simpleReceivedIt = versions.simpleCBegin(); - while (simpleReceivedIt != versions.simpleCEnd()) { - auto traitType = static_cast(std::distance(versions.simpleCBegin(), simpleReceivedIt)); - _ackedTraitVersions[nodeId][traitType] = *simpleReceivedIt; + auto sentAvatarTraitVersions = _perNodePendingTraitVersions.find(seq); + if (sentAvatarTraitVersions != _perNodePendingTraitVersions.end()) { + for (auto& perNodeTraitVersions : sentAvatarTraitVersions->second) { + auto& nodeId = perNodeTraitVersions.first; + auto& traitVersions = perNodeTraitVersions.second; + // For each trait that was sent in the traits packet, + // update the 'acked' trait version. Traits not + // sent in the traits packet keep their version. + + // process simple traits + auto simpleReceivedIt = traitVersions.simpleCBegin(); + while (simpleReceivedIt != traitVersions.simpleCEnd()) { + auto traitType = static_cast(std::distance(traitVersions.simpleCBegin(), simpleReceivedIt)); + _perNodeAckedTraitVersions[nodeId][traitType] = *simpleReceivedIt; simpleReceivedIt++; } - // enumerate the sent instanced trait versions - auto instancedSentIt = versions.instancedCBegin(); - while (instancedSentIt != versions.instancedCEnd()) { + // process instanced traits + auto instancedSentIt = traitVersions.instancedCBegin(); + 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); } diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index f26ce8504b..16054e3458 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/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; + using PerNodeTraitVersions = std::unordered_map; 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& getLastAckedTraitVersions(Node::LocalID otherAvatar) { return _ackedTraitVersions[otherAvatar]; } + AvatarTraits::TraitVersions& getLastSentTraitVersions(Node::LocalID otherAvatar) { return _perNodeSentTraitVersions[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 _pendingTraitVersions; + std::unordered_map _perNodePendingTraitVersions; std::unordered_map _lastSentTraitsTimestamps; - NodeTraitVersions _sentTraitVersions; - NodeTraitVersions _ackedTraitVersions; + PerNodeTraitVersions _perNodeSentTraitVersions; + PerNodeTraitVersions _perNodeAckedTraitVersions; std::atomic_bool _isIgnoreRadiusEnabled { false }; }; diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 5ed10dad98..5f24ae1b64 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/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 diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index ff902945bc..6a67ef6638 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -332,13 +332,15 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer 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(); 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