Address reviewer comments

This commit is contained in:
Simon Walton 2018-07-03 15:31:34 -07:00
parent 3775bdc335
commit ec67b8ad56
6 changed files with 24 additions and 38 deletions

View file

@ -612,38 +612,19 @@ void AvatarMixer::handleAvatarIdentityRequestPacket(QSharedPointer<ReceivedMessa
QUuid avatarID(QUuid::fromRfc4122(message->getMessage()) ); QUuid avatarID(QUuid::fromRfc4122(message->getMessage()) );
if (!avatarID.isNull()) { if (!avatarID.isNull()) {
auto nodeList = DependencyManager::get<NodeList>(); auto nodeList = DependencyManager::get<NodeList>();
auto node = nodeList->nodeWithUUID(avatarID);
nodeList->eachMatchingNode( if (node) {
// Predicate: QMutexLocker lock(&node->getMutex());
[&](SharedNodePointer node) -> bool { AvatarMixerClientData* avatarClientData = dynamic_cast<AvatarMixerClientData*>(node->getLinkedData());
QMutexLocker lock(&node->getMutex()); if (avatarClientData) {
if (node->getType() == NodeType::Agent && node->getLinkedData()) { const AvatarData& avatarData = avatarClientData->getAvatar();
AvatarMixerClientData* avatarClientData = dynamic_cast<AvatarMixerClientData*>(node->getLinkedData()); QByteArray serializedAvatar = avatarData.identityByteArray();
if (avatarClientData) { auto identityPackets = NLPacketList::create(PacketType::AvatarIdentity, QByteArray(), true, true);
const AvatarData& avatarData = avatarClientData->getAvatar(); identityPackets->write(serializedAvatar);
if (avatarData.getID() == avatarID) { nodeList->sendPacketList(std::move(identityPackets), *senderNode);
return true; ++_sumIdentityPackets;
}
}
}
return false;
},
// Action:
[&](SharedNodePointer node) {
QMutexLocker lock(&node->getMutex());
AvatarMixerClientData* avatarClientData = dynamic_cast<AvatarMixerClientData*>(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;
}
}
} }
); }
} }
} }

View file

@ -57,7 +57,8 @@ static const quint64 MIN_TIME_BETWEEN_MY_AVATAR_DATA_SENDS = USECS_PER_SECOND /
const QUuid MY_AVATAR_KEY; // NULL key const QUuid MY_AVATAR_KEY; // NULL key
// For an unknown avatar-data packet, wait this long before requesting the identity (in µs). // 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) : AvatarManager::AvatarManager(QObject* parent) :
_avatarsToFade(), _avatarsToFade(),
@ -283,7 +284,7 @@ void AvatarManager::updateOtherAvatars(float deltaTime) {
simulateAvatarFades(deltaTime); simulateAvatarFades(deltaTime);
// Check on avatars with pending identities: // Check on avatars with pending identities:
const quint64 now = usecTimestampNow(); steady_clock::time_point now = steady_clock::now();
QWriteLocker writeLock(&_hashLock); QWriteLocker writeLock(&_hashLock);
for (auto pendingAvatar = _pendingAvatars.begin(); pendingAvatar != _pendingAvatars.end(); ++pendingAvatar) { for (auto pendingAvatar = _pendingAvatars.begin(); pendingAvatar != _pendingAvatars.end(); ++pendingAvatar) {
if (now - pendingAvatar->creationTime >= REQUEST_UNKNOWN_IDENTITY_DELAY) { 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); auto packet = NLPacket::create(PacketType::AvatarIdentityRequest, NUM_BYTES_RFC4122_UUID, true);
packet->write(avatarID.toRfc4122()); packet->write(avatarID.toRfc4122());
nodeList->sendPacket(std::move(packet), *node); nodeList->sendPacket(std::move(packet), *node);
++_identityRequestsSent;
}); });
} }

View file

@ -196,9 +196,9 @@ private:
int _numAvatarsNotUpdated { 0 }; int _numAvatarsNotUpdated { 0 };
float _avatarSimulationTime { 0.0f }; float _avatarSimulationTime { 0.0f };
bool _shouldRender { true }; 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 #endif // hifi_AvatarManager_h

View file

@ -134,7 +134,7 @@ AvatarSharedPointer AvatarHashMap::parseAvatarData(QSharedPointer<ReceivedMessag
auto avatar = newOrExistingAvatar(sessionUUID, sendingNode, isNewAvatar); auto avatar = newOrExistingAvatar(sessionUUID, sendingNode, isNewAvatar);
if (isNewAvatar) { if (isNewAvatar) {
QWriteLocker locker(&_hashLock); QWriteLocker locker(&_hashLock);
_pendingAvatars.insert(sessionUUID, { usecTimestampNow(), avatar }); _pendingAvatars.insert(sessionUUID, { std::chrono::steady_clock::now(), avatar });
} }
// have the matching (or new) avatar parse the data from the packet // have the matching (or new) avatar parse the data from the packet

View file

@ -19,6 +19,7 @@
#include <functional> #include <functional>
#include <memory> #include <memory>
#include <chrono>
#include <glm/glm.hpp> #include <glm/glm.hpp>
@ -154,7 +155,7 @@ protected:
AvatarHash _avatarHash; AvatarHash _avatarHash;
struct PendingAvatar { struct PendingAvatar {
quint64 creationTime; std::chrono::steady_clock::time_point creationTime;
AvatarSharedPointer avatar; AvatarSharedPointer avatar;
}; };
using AvatarPendingHash = QHash<QUuid, PendingAvatar>; using AvatarPendingHash = QHash<QUuid, PendingAvatar>;

View file

@ -92,8 +92,10 @@ PacketVersion versionForPacketType(PacketType packetType) {
return static_cast<PacketVersion>(PingVersion::IncludeConnectionID); return static_cast<PacketVersion>(PingVersion::IncludeConnectionID);
case PacketType::AvatarQuery: case PacketType::AvatarQuery:
return static_cast<PacketVersion>(AvatarQueryVersion::ConicalFrustums); return static_cast<PacketVersion>(AvatarQueryVersion::ConicalFrustums);
default: case PacketType::AvatarIdentityRequest:
return 22; return 22;
default:
return 21;
} }
} }