Merge pull request #10489 from davidkelly/dk/sequenceNumberForAvatarIdentity

Switch from timestamp to sequence id for avatar identity

assignment-client/src/avatars/AvatarMixer.cpp

@@ -402,7 +402,7 @@ void AvatarMixer::handleAvatarIdentityPacket(QSharedPointer<ReceivedMessage> mes
             AvatarData::parseAvatarIdentityPacket(message->getMessage(), identity);
             bool identityChanged = false;
             bool displayNameChanged = false;
-            avatar.processAvatarIdentity(identity, identityChanged, displayNameChanged, senderNode->getClockSkewUsec());
+            avatar.processAvatarIdentity(identity, identityChanged, displayNameChanged);
             if (identityChanged) {
                 QMutexLocker nodeDataLocker(&nodeData->getMutex());
                 nodeData->flagIdentityChange();

libraries/avatars/src/AvatarData.cpp

@@ -1479,7 +1479,7 @@ void AvatarData::parseAvatarIdentityPacket(const QByteArray& data, Identity& ide
                  >> identityOut.displayName
                  >> identityOut.sessionDisplayName
                  >> identityOut.avatarEntityData
-                 >> identityOut.updatedAt;
+                 >> identityOut.sequenceId;
 
 #ifdef WANT_DEBUG
     qCDebug(avatars) << __FUNCTION__
@@ -1501,21 +1501,15 @@ QUrl AvatarData::cannonicalSkeletonModelURL(const QUrl& emptyURL) const {
     return _skeletonModelURL.scheme() == "file" ? emptyURL : _skeletonModelURL;
 }
 
-void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged, const qint64 clockSkew) {
-    quint64 identityPacketUpdatedAt = identity.updatedAt;
+void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged) {
 
-    if (identityPacketUpdatedAt <= (uint64_t)(abs(clockSkew))) { // Incoming timestamp is bad - compute our own timestamp
-        identityPacketUpdatedAt = usecTimestampNow() + clockSkew;
-    }
-
-    // Consider the case where this packet is being processed on Client A, and Client A is connected to Sandbox B.
-    // If Client A's system clock is *ahead of* Sandbox B's system clock, "clockSkew" will be *negative*.
-    // If Client A's system clock is *behind* Sandbox B's system clock, "clockSkew" will be *positive*.
-    if ((_identityUpdatedAt > identityPacketUpdatedAt - clockSkew) && (_identityUpdatedAt != 0)) {
-        qCDebug(avatars) << "Ignoring late identity packet for avatar " << getSessionUUID() 
-            << "_identityUpdatedAt (" << _identityUpdatedAt << ") is greater than identityPacketUpdatedAt - clockSkew (" << identityPacketUpdatedAt << "-" << clockSkew << ")";
+    if (identity.sequenceId < _identitySequenceId) {
+        qCDebug(avatars) << "Ignoring older identity packet for avatar" << getSessionUUID()
+            << "_identitySequenceId (" << _identitySequenceId << ") is greater than" << identity.sequenceId;
         return;
     }
+    // otherwise, set the identitySequenceId to match the incoming identity
+    _identitySequenceId = identity.sequenceId;
 
     if (_firstSkeletonCheck || (identity.skeletonModelURL != cannonicalSkeletonModelURL(emptyURL))) {
         setSkeletonModelURL(identity.skeletonModelURL);
@@ -1547,9 +1541,6 @@ void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityC
         identityChanged = true;
     }
 
-    // use the timestamp from this identity, since we want to honor the updated times in "server clock"
-    // this will overwrite any changes we made locally to this AvatarData's _identityUpdatedAt
-    _identityUpdatedAt = identityPacketUpdatedAt - clockSkew;
 }
 
 QByteArray AvatarData::identityByteArray() const {
@@ -1564,7 +1555,7 @@ QByteArray AvatarData::identityByteArray() const {
                        << _displayName
                        << getSessionDisplayNameForTransport() // depends on _sessionDisplayName
                        << _avatarEntityData
-                       << _identityUpdatedAt;
+                       << _identitySequenceId;
     });
 
     return identityData;

libraries/avatars/src/AvatarData.h

@@ -531,14 +531,14 @@ public:
         QString displayName;
         QString sessionDisplayName;
         AvatarEntityMap avatarEntityData;
-        quint64 updatedAt;
+        quint64 sequenceId;
     };
 
     static void parseAvatarIdentityPacket(const QByteArray& data, Identity& identityOut);
 
     // identityChanged returns true if identity has changed, false otherwise.
     // displayNameChanged returns true if displayName has changed, false otherwise.
-    void processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged, const qint64 clockSkew);
+    void processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged);
 
     QByteArray identityByteArray() const;
 
@@ -626,7 +626,7 @@ public:
     bool getIdentityDataChanged() const { return _identityDataChanged; } // has the identity data changed since the last time sendIdentityPacket() was called
     void markIdentityDataChanged() {
         _identityDataChanged = true;
-        _identityUpdatedAt = usecTimestampNow();
+        _identitySequenceId++;
     }
 
     float getDensity() const { return _density; }
@@ -786,7 +786,7 @@ protected:
     float _audioAverageLoudness { 0.0f };
 
     bool _identityDataChanged { false };
-    quint64 _identityUpdatedAt { 0 };
+    quint64 _identitySequenceId { 0 };
     float _density;
 
 private:

libraries/avatars/src/AvatarHashMap.cpp

@@ -149,7 +149,7 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer<ReceivedMessage>
         bool identityChanged = false;
         bool displayNameChanged = false;
         // In this case, the "sendingNode" is the Avatar Mixer.
-        avatar->processAvatarIdentity(identity, identityChanged, displayNameChanged, sendingNode->getClockSkewUsec());
+        avatar->processAvatarIdentity(identity, identityChanged, displayNameChanged);
     }
 }
 

libraries/networking/src/udt/PacketHeaders.cpp

@@ -56,7 +56,7 @@ PacketVersion versionForPacketType(PacketType packetType) {
         case PacketType::AvatarData:
         case PacketType::BulkAvatarData:
         case PacketType::KillAvatar:
-            return static_cast<PacketVersion>(AvatarMixerPacketVersion::IdentityPacketsIncludeUpdateTime);
+            return static_cast<PacketVersion>(AvatarMixerPacketVersion::AvatarIdentitySequenceId);
         case PacketType::MessagesData:
             return static_cast<PacketVersion>(MessageDataVersion::TextOrBinaryData);
         case PacketType::ICEServerHeartbeat:

libraries/networking/src/udt/PacketHeaders.h

@@ -234,7 +234,8 @@ enum class AvatarMixerPacketVersion : PacketVersion {
     VariableAvatarData,
     AvatarAsChildFixes,
     StickAndBallDefaultAvatar,
-    IdentityPacketsIncludeUpdateTime
+    IdentityPacketsIncludeUpdateTime,
+    AvatarIdentitySequenceId
 };
 
 enum class DomainConnectRequestVersion : PacketVersion {