From ec67b8ad56f3cddd8f491ea44d45b19d9a74631e Mon Sep 17 00:00:00 2001 From: Simon Walton Date: Tue, 3 Jul 2018 15:31:34 -0700 Subject: [PATCH] Address reviewer comments --- assignment-client/src/avatars/AvatarMixer.cpp | 43 ++++++------------- interface/src/avatar/AvatarManager.cpp | 6 ++- interface/src/avatar/AvatarManager.h | 4 +- libraries/avatars/src/AvatarHashMap.cpp | 2 +- libraries/avatars/src/AvatarHashMap.h | 3 +- .../networking/src/udt/PacketHeaders.cpp | 4 +- 6 files changed, 24 insertions(+), 38 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index b567119181..772c8643b3 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -612,38 +612,19 @@ void AvatarMixer::handleAvatarIdentityRequestPacket(QSharedPointergetMessage()) ); if (!avatarID.isNull()) { auto nodeList = DependencyManager::get(); - - nodeList->eachMatchingNode( - // Predicate: - [&](SharedNodePointer node) -> bool { - QMutexLocker lock(&node->getMutex()); - if (node->getType() == NodeType::Agent && node->getLinkedData()) { - AvatarMixerClientData* avatarClientData = dynamic_cast(node->getLinkedData()); - if (avatarClientData) { - const AvatarData& avatarData = avatarClientData->getAvatar(); - if (avatarData.getID() == avatarID) { - return true; - } - } - } - return false; - }, - // Action: - [&](SharedNodePointer node) { - QMutexLocker lock(&node->getMutex()); - AvatarMixerClientData* avatarClientData = dynamic_cast(node->getLinkedData()); - if (avatarClientData) { - const AvatarData& avatarData = avatarClientData->getAvatar(); - if (avatarData.getID() == avatarID) { - QByteArray serializedAvatar = avatarData.identityByteArray(); - auto identityPackets = NLPacketList::create(PacketType::AvatarIdentity, QByteArray(), true, true); - identityPackets->write(serializedAvatar); - nodeList->sendPacketList(std::move(identityPackets), *senderNode); - ++_sumIdentityPackets; - } - } + auto node = nodeList->nodeWithUUID(avatarID); + if (node) { + QMutexLocker lock(&node->getMutex()); + AvatarMixerClientData* avatarClientData = dynamic_cast(node->getLinkedData()); + if (avatarClientData) { + const AvatarData& avatarData = avatarClientData->getAvatar(); + QByteArray serializedAvatar = avatarData.identityByteArray(); + auto identityPackets = NLPacketList::create(PacketType::AvatarIdentity, QByteArray(), true, true); + identityPackets->write(serializedAvatar); + nodeList->sendPacketList(std::move(identityPackets), *senderNode); + ++_sumIdentityPackets; } - ); + } } } diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index b876551fad..b421a8ca42 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -57,7 +57,8 @@ static const quint64 MIN_TIME_BETWEEN_MY_AVATAR_DATA_SENDS = USECS_PER_SECOND / const QUuid MY_AVATAR_KEY; // NULL key // For an unknown avatar-data packet, wait this long before requesting the identity (in µs). -constexpr quint64 AvatarManager::REQUEST_UNKNOWN_IDENTITY_DELAY = 5 * 1000 * 1000; +constexpr std::chrono::milliseconds AvatarManager::REQUEST_UNKNOWN_IDENTITY_DELAY { 5 * 1000 }; +using std::chrono::steady_clock; AvatarManager::AvatarManager(QObject* parent) : _avatarsToFade(), @@ -283,7 +284,7 @@ void AvatarManager::updateOtherAvatars(float deltaTime) { simulateAvatarFades(deltaTime); // Check on avatars with pending identities: - const quint64 now = usecTimestampNow(); + steady_clock::time_point now = steady_clock::now(); QWriteLocker writeLock(&_hashLock); for (auto pendingAvatar = _pendingAvatars.begin(); pendingAvatar != _pendingAvatars.end(); ++pendingAvatar) { if (now - pendingAvatar->creationTime >= REQUEST_UNKNOWN_IDENTITY_DELAY) { @@ -319,6 +320,7 @@ void AvatarManager::sendIdentityRequest(const QUuid& avatarID) const { auto packet = NLPacket::create(PacketType::AvatarIdentityRequest, NUM_BYTES_RFC4122_UUID, true); packet->write(avatarID.toRfc4122()); nodeList->sendPacket(std::move(packet), *node); + ++_identityRequestsSent; }); } diff --git a/interface/src/avatar/AvatarManager.h b/interface/src/avatar/AvatarManager.h index 4d2f9c5002..9be748d59e 100644 --- a/interface/src/avatar/AvatarManager.h +++ b/interface/src/avatar/AvatarManager.h @@ -196,9 +196,9 @@ private: int _numAvatarsNotUpdated { 0 }; float _avatarSimulationTime { 0.0f }; bool _shouldRender { true }; - int _identityRequestsSent { 0 }; + mutable int _identityRequestsSent { 0 }; - static const quint64 REQUEST_UNKNOWN_IDENTITY_DELAY; + static const std::chrono::milliseconds REQUEST_UNKNOWN_IDENTITY_DELAY; }; #endif // hifi_AvatarManager_h diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 63028dd113..20308eccc6 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -134,7 +134,7 @@ AvatarSharedPointer AvatarHashMap::parseAvatarData(QSharedPointer #include +#include #include @@ -154,7 +155,7 @@ protected: AvatarHash _avatarHash; struct PendingAvatar { - quint64 creationTime; + std::chrono::steady_clock::time_point creationTime; AvatarSharedPointer avatar; }; using AvatarPendingHash = QHash; diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index 02d59674d6..51ef6de295 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -92,8 +92,10 @@ PacketVersion versionForPacketType(PacketType packetType) { return static_cast(PingVersion::IncludeConnectionID); case PacketType::AvatarQuery: return static_cast(AvatarQueryVersion::ConicalFrustums); - default: + case PacketType::AvatarIdentityRequest: return 22; + default: + return 21; } }