From 9ab643c41684000e526921d04c22cd5164b4cb15 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Fri, 5 May 2017 16:47:08 -0700 Subject: [PATCH 1/4] I think this is all that must be done. --- libraries/avatars/src/AvatarData.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index cb819c6b20..c4d72efa43 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1497,7 +1497,7 @@ QUrl AvatarData::cannonicalSkeletonModelURL(const QUrl& emptyURL) const { void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged) { - if (identity.updatedAt < _identityUpdatedAt) { + if (identity.updatedAt < _identityUpdatedAt && !(DependencyManager::get()->getRequestsDomainListData())) { qCDebug(avatars) << "Ignoring late identity packet for avatar " << getSessionUUID() << "identity.updatedAt:" << identity.updatedAt << "_identityUpdatedAt:" << _identityUpdatedAt; return; From d7e4f022911afa2460f8a164ba5859716caf56d9 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Tue, 9 May 2017 12:23:40 -0700 Subject: [PATCH 2/4] This is a better method. --- assignment-client/src/avatars/AvatarMixer.cpp | 2 +- libraries/avatars/src/AvatarData.cpp | 6 +++--- libraries/avatars/src/AvatarData.h | 2 +- libraries/avatars/src/AvatarHashMap.cpp | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 05dbfee912..b4d0864581 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -402,7 +402,7 @@ void AvatarMixer::handleAvatarIdentityPacket(QSharedPointer mes AvatarData::parseAvatarIdentityPacket(message->getMessage(), identity); bool identityChanged = false; bool displayNameChanged = false; - avatar.processAvatarIdentity(identity, identityChanged, displayNameChanged); + avatar.processAvatarIdentity(identity, identityChanged, displayNameChanged, 0); if (identityChanged) { QMutexLocker nodeDataLocker(&nodeData->getMutex()); nodeData->flagIdentityChange(); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index c4d72efa43..5d5455327c 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1495,11 +1495,11 @@ QUrl AvatarData::cannonicalSkeletonModelURL(const QUrl& emptyURL) const { return _skeletonModelURL.scheme() == "file" ? emptyURL : _skeletonModelURL; } -void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged) { +void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged, const qint64 clockSkew) { - if (identity.updatedAt < _identityUpdatedAt && !(DependencyManager::get()->getRequestsDomainListData())) { + if (identity.updatedAt < _identityUpdatedAt + clockSkew) { qCDebug(avatars) << "Ignoring late identity packet for avatar " << getSessionUUID() - << "identity.updatedAt:" << identity.updatedAt << "_identityUpdatedAt:" << _identityUpdatedAt; + << "identity.updatedAt:" << identity.updatedAt << "_identityUpdatedAt:" << _identityUpdatedAt << "clockSkew:" << clockSkew; return; } diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 6d801793b7..e6e0571878 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -538,7 +538,7 @@ public: // 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); + void processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged, const qint64 clockSkew); QByteArray identityByteArray() const; diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 0d341c684e..f4dc30a10c 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -148,7 +148,7 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer auto avatar = newOrExistingAvatar(identity.uuid, sendingNode); bool identityChanged = false; bool displayNameChanged = false; - avatar->processAvatarIdentity(identity, identityChanged, displayNameChanged); + avatar->processAvatarIdentity(identity, identityChanged, displayNameChanged, sendingNode->getClockSkewUsec()); } } From ae983658becf18702d6cd957016f84bda0ab3141 Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Wed, 10 May 2017 14:46:14 -0700 Subject: [PATCH 3/4] Finally, the actual fix? --- assignment-client/src/avatars/AvatarMixer.cpp | 2 +- libraries/avatars/src/AvatarData.cpp | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index b4d0864581..998799f5e6 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -402,7 +402,7 @@ void AvatarMixer::handleAvatarIdentityPacket(QSharedPointer mes AvatarData::parseAvatarIdentityPacket(message->getMessage(), identity); bool identityChanged = false; bool displayNameChanged = false; - avatar.processAvatarIdentity(identity, identityChanged, displayNameChanged, 0); + avatar.processAvatarIdentity(identity, identityChanged, displayNameChanged, senderNode->getClockSkewUsec()); if (identityChanged) { QMutexLocker nodeDataLocker(&nodeData->getMutex()); nodeData->flagIdentityChange(); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 5d5455327c..ec36a81a43 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1497,9 +1497,9 @@ QUrl AvatarData::cannonicalSkeletonModelURL(const QUrl& emptyURL) const { void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged, const qint64 clockSkew) { - if (identity.updatedAt < _identityUpdatedAt + clockSkew) { + if ((_identityUpdatedAt > identity.updatedAt - clockSkew) && (_identityUpdatedAt != 0)) { qCDebug(avatars) << "Ignoring late identity packet for avatar " << getSessionUUID() - << "identity.updatedAt:" << identity.updatedAt << "_identityUpdatedAt:" << _identityUpdatedAt << "clockSkew:" << clockSkew; + << "_identityUpdatedAt (" << _identityUpdatedAt << ") is greater than identity.updatedAt - clockSkew (" << identity.updatedAt << "-" << clockSkew << ")"; return; } @@ -1535,7 +1535,7 @@ void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityC // 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 = identity.updatedAt; + _identityUpdatedAt = identity.updatedAt - clockSkew; } QByteArray AvatarData::identityByteArray() const { From 15574f509edab5081afea71f415d8959b105a3ee Mon Sep 17 00:00:00 2001 From: Zach Fox Date: Thu, 11 May 2017 11:45:07 -0700 Subject: [PATCH 4/4] Clarifying comments --- libraries/avatars/src/AvatarData.cpp | 3 +++ libraries/avatars/src/AvatarHashMap.cpp | 1 + 2 files changed, 4 insertions(+) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index ec36a81a43..392d3752e8 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1497,6 +1497,9 @@ QUrl AvatarData::cannonicalSkeletonModelURL(const QUrl& emptyURL) const { void AvatarData::processAvatarIdentity(const Identity& identity, bool& identityChanged, bool& displayNameChanged, const qint64 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 > identity.updatedAt - clockSkew) && (_identityUpdatedAt != 0)) { qCDebug(avatars) << "Ignoring late identity packet for avatar " << getSessionUUID() << "_identityUpdatedAt (" << _identityUpdatedAt << ") is greater than identity.updatedAt - clockSkew (" << identity.updatedAt << "-" << clockSkew << ")"; diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index f4dc30a10c..155ef9a0a2 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -148,6 +148,7 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer auto avatar = newOrExistingAvatar(identity.uuid, sendingNode); bool identityChanged = false; bool displayNameChanged = false; + // In this case, the "sendingNode" is the Avatar Mixer. avatar->processAvatarIdentity(identity, identityChanged, displayNameChanged, sendingNode->getClockSkewUsec()); } }