From 486557e28a7d35e635224e7849700b287e1c705c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 30 Jul 2018 16:29:28 -0700 Subject: [PATCH 01/31] initial trait packet sending on skeleton URL change --- assignment-client/src/avatars/AvatarMixer.cpp | 5 ++ assignment-client/src/avatars/AvatarMixer.h | 1 + interface/src/avatar/MyAvatar.cpp | 7 +- interface/src/avatar/MyAvatar.h | 20 +++-- libraries/avatars/src/AvatarTraits.h | 29 +++++++ libraries/avatars/src/ClientTraitsHandler.cpp | 84 +++++++++++++++++++ libraries/avatars/src/ClientTraitsHandler.h | 42 ++++++++++ libraries/networking/src/udt/PacketHeaders.h | 2 +- 8 files changed, 179 insertions(+), 11 deletions(-) create mode 100644 libraries/avatars/src/AvatarTraits.h create mode 100644 libraries/avatars/src/ClientTraitsHandler.cpp create mode 100644 libraries/avatars/src/ClientTraitsHandler.h diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 8f1df9c321..a5eba6ad11 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -54,6 +54,7 @@ AvatarMixer::AvatarMixer(ReceivedMessage& message) : packetReceiver.registerListener(PacketType::RadiusIgnoreRequest, this, "handleRadiusIgnoreRequestPacket"); packetReceiver.registerListener(PacketType::RequestsDomainListData, this, "handleRequestsDomainListDataPacket"); packetReceiver.registerListener(PacketType::AvatarIdentityRequest, this, "handleAvatarIdentityRequestPacket"); + packetReceiver.registerListener(PacketType::SetAvatarTraits, this, "handleSetAvatarTraitsMessage"); packetReceiver.registerListenerForTypes({ PacketType::ReplicatedAvatarIdentity, @@ -606,6 +607,10 @@ void AvatarMixer::handleAvatarIdentityPacket(QSharedPointer mes _handleAvatarIdentityPacketElapsedTime += (end - start); } +void AvatarMixer::handleSetAvatarTraitsMessage(QSharedPointer message, SharedNodePointer senderNode) { + qDebug() << "Got a traits packet of" << message->getSize() << "bytes from" << senderNode; +} + void AvatarMixer::handleAvatarIdentityRequestPacket(QSharedPointer message, SharedNodePointer senderNode) { if (message->getSize() < NUM_BYTES_RFC4122_UUID) { qCDebug(avatars) << "Malformed AvatarIdentityRequest received from" << message->getSenderSockAddr().toString(); diff --git a/assignment-client/src/avatars/AvatarMixer.h b/assignment-client/src/avatars/AvatarMixer.h index c27ca33729..b0405a0b6d 100644 --- a/assignment-client/src/avatars/AvatarMixer.h +++ b/assignment-client/src/avatars/AvatarMixer.h @@ -48,6 +48,7 @@ private slots: void handleAdjustAvatarSorting(QSharedPointer message, SharedNodePointer senderNode); void handleAvatarQueryPacket(QSharedPointer message, SharedNodePointer senderNode); void handleAvatarIdentityPacket(QSharedPointer message, SharedNodePointer senderNode); + void handleSetAvatarTraitsMessage(QSharedPointer message, SharedNodePointer senderNode); void handleKillAvatarPacket(QSharedPointer message, SharedNodePointer senderNode); void handleNodeIgnoreRequestPacket(QSharedPointer message, SharedNodePointer senderNode); void handleRadiusIgnoreRequestPacket(QSharedPointer packet, SharedNodePointer sendingNode); diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 41855e7973..3ba2cbfc1e 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -119,7 +119,8 @@ MyAvatar::MyAvatar(QThread* thread) : _goToOrientation(), _prevShouldDrawHead(true), _audioListenerMode(FROM_HEAD), - _hmdAtRestDetector(glm::vec3(0), glm::quat()) + _hmdAtRestDetector(glm::vec3(0), glm::quat()), + _clientTraitsHandler(this) { // give the pointer to our head to inherited _headData variable from AvatarData @@ -513,6 +514,8 @@ void MyAvatar::update(float deltaTime) { sendIdentityPacket(); } + _clientTraitsHandler.sendChangedTraitsToMixer(); + simulate(deltaTime); currentEnergy += energyChargeRate; @@ -1696,6 +1699,8 @@ void MyAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { saveAvatarUrl(); emit skeletonChanged(); emit skeletonModelURLChanged(); + + _clientTraitsHandler.markTraitChanged(AvatarTraits::SkeletonModelURL); } void MyAvatar::removeAvatarEntities(const std::function& condition) { diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index f2c52624d9..8fdc4ba4e3 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -18,22 +18,22 @@ #include -#include -#include -#include -#include - -#include -#include #include #include #include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include "AtRestDetector.h" #include "MyCharacterController.h" #include "RingBufferHistory.h" -#include -#include class AvatarActionHold; class ModelItemID; @@ -1776,6 +1776,8 @@ private: bool _haveReceivedHeightLimitsFromDomain { false }; int _disableHandTouchCount { 0 }; + + ClientTraitsHandler _clientTraitsHandler; }; QScriptValue audioListenModeToScriptValue(QScriptEngine* engine, const AudioListenerMode& audioListenerMode); diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h new file mode 100644 index 0000000000..ebeaeaea20 --- /dev/null +++ b/libraries/avatars/src/AvatarTraits.h @@ -0,0 +1,29 @@ +// +// AvatarTraits.h +// libraries/avatars/src +// +// Created by Stephen Birarda on 7/30/18. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#ifndef hifi_AvatarTraits_h +#define hifi_AvatarTraits_h + +#include + +namespace AvatarTraits { + enum Trait : uint8_t { + SkeletonModelURL, + TotalTraits + }; + + using TraitVersion = uint32_t; + const TraitVersion DEFAULT_TRAIT_VERSION = 0; + + using TraitVersions = std::vector; +} + +#endif // hifi_AvatarTraits_h diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp new file mode 100644 index 0000000000..c4685461a0 --- /dev/null +++ b/libraries/avatars/src/ClientTraitsHandler.cpp @@ -0,0 +1,84 @@ +// +// ClientTraitsHandler.cpp +// libraries/avatars/src +// +// Created by Stephen Birarda on 7/30/18. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#include "ClientTraitsHandler.h" + +#include + +#include +#include + +#include "AvatarData.h" + +ClientTraitsHandler::ClientTraitsHandler(AvatarData* owningAvatar) : + _owningAvatar(owningAvatar) +{ + auto nodeList = DependencyManager::get(); + QObject::connect(nodeList.data(), &NodeList::nodeAdded, [this](SharedNodePointer addedNode){ + if (addedNode->getType() == NodeType::AvatarMixer) { + resetForNewMixer(); + } + }); +} + +void ClientTraitsHandler::resetForNewMixer() { + // re-set the current version to 0 + _currentTraitVersion = AvatarTraits::DEFAULT_TRAIT_VERSION; + + // mark that all traits should be sent next time + _performInitialSend = true; +} + +void ClientTraitsHandler::sendChangedTraitsToMixer() { + if (hasChangedTraits() || _performInitialSend) { + // we have at least one changed trait to send + + auto nodeList = DependencyManager::get(); + auto avatarMixer = nodeList->soloNodeOfType(NodeType::AvatarMixer); + if (!avatarMixer || !avatarMixer->getActiveSocket()) { + // we don't have an avatar mixer with an active socket, we can't send changed traits at this time + return; + } + + // we have a mixer to send to, setup our set traits packet + + // bump and write the current trait version to an extended header + // the trait version is the same for all traits in this packet list + ++_currentTraitVersion; + QByteArray extendedHeader(reinterpret_cast(&_currentTraitVersion), sizeof(_currentTraitVersion)); + + auto traitsPacketList = NLPacketList::create(PacketType::SetAvatarTraits, extendedHeader, true); + + // take a copy of the set of changed traits and clear the stored set + auto changedTraitsCopy { _changedTraits }; + _changedTraits.clear(); + + if (_performInitialSend || changedTraitsCopy.count(AvatarTraits::SkeletonModelURL)) { + traitsPacketList->startSegment(); + + traitsPacketList->writePrimitive(AvatarTraits::SkeletonModelURL); + + auto encodedSkeletonURL = _owningAvatar->getSkeletonModelURL().toEncoded(); + + uint16_t encodedURLSize = encodedSkeletonURL.size(); + traitsPacketList->writePrimitive(encodedURLSize); + + traitsPacketList->write(encodedSkeletonURL); + + traitsPacketList->endSegment(); + } + + nodeList->sendPacketList(std::move(traitsPacketList), *avatarMixer); + + // if this was an initial send of all traits, consider it completed + _performInitialSend = false; + } +} diff --git a/libraries/avatars/src/ClientTraitsHandler.h b/libraries/avatars/src/ClientTraitsHandler.h new file mode 100644 index 0000000000..9fd3104e7e --- /dev/null +++ b/libraries/avatars/src/ClientTraitsHandler.h @@ -0,0 +1,42 @@ +// +// ClientTraitsHandler.h +// libraries/avatars/src +// +// Created by Stephen Birarda on 7/30/18. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#ifndef hifi_ClientTraitsHandler_h +#define hifi_ClientTraitsHandler_h + +#include + +#include "AvatarTraits.h" + +class AvatarData; + +class ClientTraitsHandler { +public: + ClientTraitsHandler(AvatarData* owningAvatar); + + void sendChangedTraitsToMixer(); + + bool hasChangedTraits() { return _changedTraits.size(); } + void markTraitChanged(AvatarTraits::Trait changedTrait) { _changedTraits.insert(changedTrait); } + + bool hasTraitChanged(AvatarTraits::Trait checkTrait) { return _changedTraits.count(checkTrait) > 0; } + + void resetForNewMixer(); + +private: + AvatarData* _owningAvatar; + + std::set _changedTraits; + AvatarTraits::TraitVersion _currentTraitVersion { AvatarTraits::DEFAULT_TRAIT_VERSION }; + bool _performInitialSend { false }; +}; + +#endif // hifi_ClientTraitsHandler_h diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 126dac7c8f..3990afa79a 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -56,7 +56,7 @@ public: ICEServerPeerInformation, ICEServerQuery, OctreeStats, - UNUSED_PACKET_TYPE_1, + SetAvatarTraits, AvatarIdentityRequest, AssignmentClientStatus, NoisyMute, From ac835650b74421be618aaef7bd362f6dd17c0e95 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 30 Jul 2018 17:33:01 -0700 Subject: [PATCH 02/31] add skeleton trait sending to Agent, queuing in AvatarMixer --- assignment-client/src/Agent.h | 1 + assignment-client/src/avatars/AvatarMixer.cpp | 6 +----- assignment-client/src/avatars/AvatarMixer.h | 3 --- assignment-client/src/avatars/AvatarMixerClientData.cpp | 8 ++++++++ assignment-client/src/avatars/AvatarMixerClientData.h | 2 ++ assignment-client/src/avatars/ScriptableAvatar.cpp | 5 +++++ assignment-client/src/avatars/ScriptableAvatar.h | 5 ++++- interface/src/avatar/MyAvatar.cpp | 9 ++++++++- libraries/avatars/src/AvatarTraits.h | 2 -- 9 files changed, 29 insertions(+), 12 deletions(-) diff --git a/assignment-client/src/Agent.h b/assignment-client/src/Agent.h index 7d47c8e713..2b5ff51b49 100644 --- a/assignment-client/src/Agent.h +++ b/assignment-client/src/Agent.h @@ -21,6 +21,7 @@ #include #include +#include #include #include #include diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index a5eba6ad11..56fb15c9f0 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -54,7 +54,7 @@ AvatarMixer::AvatarMixer(ReceivedMessage& message) : packetReceiver.registerListener(PacketType::RadiusIgnoreRequest, this, "handleRadiusIgnoreRequestPacket"); packetReceiver.registerListener(PacketType::RequestsDomainListData, this, "handleRequestsDomainListDataPacket"); packetReceiver.registerListener(PacketType::AvatarIdentityRequest, this, "handleAvatarIdentityRequestPacket"); - packetReceiver.registerListener(PacketType::SetAvatarTraits, this, "handleSetAvatarTraitsMessage"); + packetReceiver.registerListener(PacketType::SetAvatarTraits, this, "queueIncomingPacket"); packetReceiver.registerListenerForTypes({ PacketType::ReplicatedAvatarIdentity, @@ -607,10 +607,6 @@ void AvatarMixer::handleAvatarIdentityPacket(QSharedPointer mes _handleAvatarIdentityPacketElapsedTime += (end - start); } -void AvatarMixer::handleSetAvatarTraitsMessage(QSharedPointer message, SharedNodePointer senderNode) { - qDebug() << "Got a traits packet of" << message->getSize() << "bytes from" << senderNode; -} - void AvatarMixer::handleAvatarIdentityRequestPacket(QSharedPointer message, SharedNodePointer senderNode) { if (message->getSize() < NUM_BYTES_RFC4122_UUID) { qCDebug(avatars) << "Malformed AvatarIdentityRequest received from" << message->getSenderSockAddr().toString(); diff --git a/assignment-client/src/avatars/AvatarMixer.h b/assignment-client/src/avatars/AvatarMixer.h index b0405a0b6d..14f84c9e7b 100644 --- a/assignment-client/src/avatars/AvatarMixer.h +++ b/assignment-client/src/avatars/AvatarMixer.h @@ -48,7 +48,6 @@ private slots: void handleAdjustAvatarSorting(QSharedPointer message, SharedNodePointer senderNode); void handleAvatarQueryPacket(QSharedPointer message, SharedNodePointer senderNode); void handleAvatarIdentityPacket(QSharedPointer message, SharedNodePointer senderNode); - void handleSetAvatarTraitsMessage(QSharedPointer message, SharedNodePointer senderNode); void handleKillAvatarPacket(QSharedPointer message, SharedNodePointer senderNode); void handleNodeIgnoreRequestPacket(QSharedPointer message, SharedNodePointer senderNode); void handleRadiusIgnoreRequestPacket(QSharedPointer packet, SharedNodePointer sendingNode); @@ -127,9 +126,7 @@ private: RateCounter<> _loopRate; // this is the rate that the main thread tight loop runs - AvatarMixerSlavePool _slavePool; - }; #endif // hifi_AvatarMixer_h diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 9aa3e88b52..f6718b839a 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -62,6 +62,9 @@ int AvatarMixerClientData::processPackets() { case PacketType::AvatarData: parseData(*packet); break; + case PacketType::SetAvatarTraits: + processSetTraitsMessage(*packet); + break; default: Q_UNREACHABLE(); } @@ -87,6 +90,11 @@ int AvatarMixerClientData::parseData(ReceivedMessage& message) { // compute the offset to the data payload return _avatar->parseDataFromBuffer(message.readWithoutCopy(message.getBytesLeftToRead())); } + +void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message) { + qDebug() << "Pulling a traits message of" << message.getSize(); +} + uint64_t AvatarMixerClientData::getLastBroadcastTime(const QUuid& nodeUUID) const { // return the matching PacketSequenceNumber, or the default if we don't have it auto nodeMatch = _lastBroadcastTimes.find(nodeUUID); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index e038e81505..2e30b2c8a1 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -120,6 +120,8 @@ public: void queuePacket(QSharedPointer message, SharedNodePointer node); int processPackets(); // returns number of packets processed + void processSetTraitsMessage(ReceivedMessage& message); + private: struct PacketQueue : public std::queue> { QWeakPointer node; diff --git a/assignment-client/src/avatars/ScriptableAvatar.cpp b/assignment-client/src/avatars/ScriptableAvatar.cpp index e7210db83a..4e47cf96f1 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.cpp +++ b/assignment-client/src/avatars/ScriptableAvatar.cpp @@ -61,7 +61,10 @@ AnimationDetails ScriptableAvatar::getAnimationDetails() { void ScriptableAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { _bind.reset(); _animSkeleton.reset(); + AvatarData::setSkeletonModelURL(skeletonModelURL); + + _clientTraitsHandler.markTraitChanged(AvatarTraits::SkeletonModelURL); } static AnimPose composeAnimPose(const FBXJoint& fbxJoint, const glm::quat rotation, const glm::vec3 translation) { @@ -137,4 +140,6 @@ void ScriptableAvatar::update(float deltatime) { _animation.clear(); } } + + _clientTraitsHandler.sendChangedTraitsToMixer(); } diff --git a/assignment-client/src/avatars/ScriptableAvatar.h b/assignment-client/src/avatars/ScriptableAvatar.h index d34ad2d21e..8e3d779dda 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.h +++ b/assignment-client/src/avatars/ScriptableAvatar.h @@ -15,6 +15,7 @@ #include #include #include +#include #include /**jsdoc @@ -123,7 +124,7 @@ class ScriptableAvatar : public AvatarData, public Dependency { Q_OBJECT public: - + /**jsdoc * @function Avatar.startAnimation * @param {string} url @@ -164,6 +165,8 @@ private: QStringList _maskedJoints; AnimationPointer _bind; // a sleazy way to get the skeleton, given the various library/cmake dependencies std::shared_ptr _animSkeleton; + + ClientTraitsHandler _clientTraitsHandler { this }; }; #endif // hifi_ScriptableAvatar_h diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 3ba2cbfc1e..53ad8c0a0f 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1669,7 +1669,10 @@ void MyAvatar::clearJointsData() { void MyAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { _skeletonModelChangeCount++; int skeletonModelChangeCount = _skeletonModelChangeCount; + + auto previousSkeletonModelURL = _skeletonModelURL; Avatar::setSkeletonModelURL(skeletonModelURL); + _skeletonModel->setTagMask(render::hifi::TAG_NONE); _skeletonModel->setGroupCulled(true); _skeletonModel->setVisibleInScene(true, qApp->getMain3DScene()); @@ -1700,7 +1703,11 @@ void MyAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { emit skeletonChanged(); emit skeletonModelURLChanged(); - _clientTraitsHandler.markTraitChanged(AvatarTraits::SkeletonModelURL); + if (previousSkeletonModelURL != _skeletonModelURL) { + _clientTraitsHandler.markTraitChanged(AvatarTraits::SkeletonModelURL); + } else { + qDebug() << "Not marking skeleton model URL trait changed since the new value matches the previous"; + } } void MyAvatar::removeAvatarEntities(const std::function& condition) { diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h index ebeaeaea20..c35bfae95c 100644 --- a/libraries/avatars/src/AvatarTraits.h +++ b/libraries/avatars/src/AvatarTraits.h @@ -22,8 +22,6 @@ namespace AvatarTraits { using TraitVersion = uint32_t; const TraitVersion DEFAULT_TRAIT_VERSION = 0; - - using TraitVersions = std::vector; } #endif // hifi_AvatarTraits_h From f23a036f4aab08ab49a2cd49803e9dc0353076b6 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 7 Aug 2018 11:29:03 -0700 Subject: [PATCH 03/31] add node local ID retreivable from NodeData --- assignment-client/src/audio/AudioMixer.cpp | 2 +- assignment-client/src/audio/AudioMixerClientData.cpp | 4 ++-- assignment-client/src/audio/AudioMixerClientData.h | 2 +- assignment-client/src/avatars/AvatarMixer.cpp | 2 +- assignment-client/src/avatars/AvatarMixerClientData.cpp | 2 +- assignment-client/src/avatars/AvatarMixerClientData.h | 2 +- libraries/networking/src/NodeData.cpp | 9 ++++----- libraries/networking/src/NodeData.h | 5 ++++- 8 files changed, 15 insertions(+), 13 deletions(-) diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index d56b22466e..0d42cc83be 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/assignment-client/src/audio/AudioMixer.cpp @@ -366,7 +366,7 @@ AudioMixerClientData* AudioMixer::getOrCreateClientData(Node* node) { auto clientData = dynamic_cast(node->getLinkedData()); if (!clientData) { - node->setLinkedData(std::unique_ptr { new AudioMixerClientData(node->getUUID()) }); + node->setLinkedData(std::unique_ptr { new AudioMixerClientData(node->getUUID(), node->getLocalID()) }); clientData = dynamic_cast(node->getLinkedData()); connect(clientData, &AudioMixerClientData::injectorStreamFinished, this, &AudioMixer::removeHRTFsForFinishedInjector); } diff --git a/assignment-client/src/audio/AudioMixerClientData.cpp b/assignment-client/src/audio/AudioMixerClientData.cpp index bc08c6f24a..07cc5493b0 100644 --- a/assignment-client/src/audio/AudioMixerClientData.cpp +++ b/assignment-client/src/audio/AudioMixerClientData.cpp @@ -25,8 +25,8 @@ #include "AudioHelpers.h" #include "AudioMixer.h" -AudioMixerClientData::AudioMixerClientData(const QUuid& nodeID) : - NodeData(nodeID), +AudioMixerClientData::AudioMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID) : + NodeData(nodeID, nodeLocalID), audioLimiter(AudioConstants::SAMPLE_RATE, AudioConstants::STEREO), _ignoreZone(*this), _outgoingMixedAudioSequenceNumber(0), diff --git a/assignment-client/src/audio/AudioMixerClientData.h b/assignment-client/src/audio/AudioMixerClientData.h index 514e1c9756..82bdc0e5c5 100644 --- a/assignment-client/src/audio/AudioMixerClientData.h +++ b/assignment-client/src/audio/AudioMixerClientData.h @@ -30,7 +30,7 @@ class AudioMixerClientData : public NodeData { Q_OBJECT public: - AudioMixerClientData(const QUuid& nodeID); + AudioMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID); ~AudioMixerClientData(); using SharedStreamPointer = std::shared_ptr; diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 56fb15c9f0..b139870e6e 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -904,7 +904,7 @@ AvatarMixerClientData* AvatarMixer::getOrCreateClientData(SharedNodePointer node auto clientData = dynamic_cast(node->getLinkedData()); if (!clientData) { - node->setLinkedData(std::unique_ptr { new AvatarMixerClientData(node->getUUID()) }); + node->setLinkedData(std::unique_ptr { new AvatarMixerClientData(node->getUUID(), node->getLocalID()) }); clientData = dynamic_cast(node->getLinkedData()); auto& avatar = clientData->getAvatar(); avatar.setDomainMinimumHeight(_domainMinimumHeight); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index f6718b839a..19016ae80e 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -16,7 +16,7 @@ #include #include -AvatarMixerClientData::AvatarMixerClientData(const QUuid& nodeID) : +AvatarMixerClientData::AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID) : NodeData(nodeID) { // in case somebody calls getSessionUUID on the AvatarData instance, make sure it has the right ID diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 2e30b2c8a1..ceb66a9d22 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -36,7 +36,7 @@ const QString INBOUND_AVATAR_DATA_STATS_KEY = "inbound_av_data_kbps"; class AvatarMixerClientData : public NodeData { Q_OBJECT public: - AvatarMixerClientData(const QUuid& nodeID = QUuid()); + AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID); virtual ~AvatarMixerClientData() {} using HRCTime = p_high_resolution_clock::time_point; diff --git a/libraries/networking/src/NodeData.cpp b/libraries/networking/src/NodeData.cpp index 300c76ca28..dd926852cd 100644 --- a/libraries/networking/src/NodeData.cpp +++ b/libraries/networking/src/NodeData.cpp @@ -11,13 +11,12 @@ #include "NodeData.h" -NodeData::NodeData(const QUuid& nodeID) : +NodeData::NodeData(const QUuid& nodeID, NetworkPeer::LocalID nodeLocalID) : _mutex(), - _nodeID(nodeID) + _nodeID(nodeID), + _nodeLocalID(nodeLocalID) { } -NodeData::~NodeData() { - -} +NodeData::~NodeData() {} diff --git a/libraries/networking/src/NodeData.h b/libraries/networking/src/NodeData.h index 9aeac03837..ffbc0c2376 100644 --- a/libraries/networking/src/NodeData.h +++ b/libraries/networking/src/NodeData.h @@ -16,6 +16,7 @@ #include #include +#include "NetworkPeer.h" #include "NLPacket.h" #include "ReceivedMessage.h" @@ -24,17 +25,19 @@ class Node; class NodeData : public QObject { Q_OBJECT public: - NodeData(const QUuid& nodeID = QUuid()); + NodeData(const QUuid& nodeID = QUuid(), NetworkPeer::LocalID localID = NetworkPeer::NULL_LOCAL_ID); virtual ~NodeData() = 0; virtual int parseData(ReceivedMessage& message) { return 0; } const QUuid& getNodeID() const { return _nodeID; } + NetworkPeer::LocalID getNodeLocalID() const { return _nodeLocalID; } QMutex& getMutex() { return _mutex; } private: QMutex _mutex; QUuid _nodeID; + NetworkPeer::LocalID _nodeLocalID; }; #endif // hifi_NodeData_h From 26a1f0331478d15fbe0824542b14b0cf61db4a4b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 7 Aug 2018 14:02:07 -0700 Subject: [PATCH 04/31] send traits in bulk to avatar mixer client --- .../src/avatars/AvatarMixerClientData.cpp | 75 ++++++++++++++++++- .../src/avatars/AvatarMixerClientData.h | 21 ++++++ .../src/avatars/AvatarMixerSlave.cpp | 72 +++++++++++++++++- .../src/avatars/AvatarMixerSlave.h | 4 + libraries/avatars/src/AvatarTraits.h | 14 +++- libraries/avatars/src/ClientTraitsHandler.cpp | 4 +- libraries/avatars/src/ClientTraitsHandler.h | 6 +- libraries/networking/src/udt/PacketHeaders.h | 3 +- 8 files changed, 188 insertions(+), 11 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 19016ae80e..cf30aca1a7 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -17,7 +17,8 @@ #include AvatarMixerClientData::AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID) : - NodeData(nodeID) + NodeData(nodeID), + _receivedSimpleTraitVersions(AvatarTraits::SimpleTraitTypes.size()) { // in case somebody calls getSessionUUID on the AvatarData instance, make sure it has the right ID _avatar->setID(nodeID); @@ -92,7 +93,41 @@ int AvatarMixerClientData::parseData(ReceivedMessage& message) { } void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message) { - qDebug() << "Pulling a traits message of" << message.getSize(); + // pull the trait version from the message + AvatarTraits::TraitVersion packetTraitVersion; + message.readPrimitive(&packetTraitVersion); + + bool anyTraitsChanged = false; + + while (message.getBytesLeftToRead() > 0) { + // for each trait in the packet, apply it if the trait version is newer than what we have + + AvatarTraits::TraitType traitType; + message.readPrimitive(&traitType); + + AvatarTraits::TraitWireSize traitSize; + message.readPrimitive(&traitSize); + + if (packetTraitVersion > _receivedSimpleTraitVersions[traitType]) { + if (traitType == AvatarTraits::SkeletonModelURL) { + // get the URL from the binary data + auto skeletonModelURL = QUrl::fromEncoded(message.read(traitSize)); + _avatar->setSkeletonModelURL(skeletonModelURL); + + qDebug() << "Set skeleton URL to" << skeletonModelURL << "for trait packet version" << packetTraitVersion; + + _receivedSimpleTraitVersions[traitType] = packetTraitVersion; + + anyTraitsChanged = true; + } + } else { + message.seek(message.getPosition() + traitSize); + } + } + + if (anyTraitsChanged) { + _lastReceivedTraitsChange = std::chrono::steady_clock::now(); + } } uint64_t AvatarMixerClientData::getLastBroadcastTime(const QUuid& nodeUUID) const { @@ -172,3 +207,39 @@ void AvatarMixerClientData::loadJSONStats(QJsonObject& jsonObject) const { jsonObject["recent_other_av_in_view"] = _recentOtherAvatarsInView; jsonObject["recent_other_av_out_of_view"] = _recentOtherAvatarsOutOfView; } + +AvatarMixerClientData::TraitsCheckTimestamp AvatarMixerClientData::getLastOtherAvatarTraitsSendPoint(Node::LocalID otherAvatar) const { + auto it = _lastSentTraitsTimestamps.find(otherAvatar); + + if (it != _lastSentTraitsTimestamps.end()) { + return it->second; + } else { + return TraitsCheckTimestamp(); + } +} + +AvatarTraits::TraitVersion AvatarMixerClientData::getLastSentSimpleTraitVersion(Node::LocalID otherAvatar, + AvatarTraits::TraitType traitType) const { + auto it = _sentSimpleTraitVersions.find(otherAvatar); + + if (it != _sentSimpleTraitVersions.end()) { + return it->second[traitType]; + } + + return AvatarTraits::DEFAULT_TRAIT_VERSION; +} + +void AvatarMixerClientData::setLastSentSimpleTraitVersion(Node::LocalID otherAvatar, AvatarTraits::TraitType traitType, AvatarTraits::TraitVersion traitVersion) { + + auto it = _sentSimpleTraitVersions.find(otherAvatar); + + if (it == _sentSimpleTraitVersions.end()) { + auto pair = _sentSimpleTraitVersions.insert({ + otherAvatar, { AvatarTraits::TotalTraitTypes, AvatarTraits::DEFAULT_TRAIT_VERSION } + }); + + it = pair.first; + } + + it->second[traitType] = traitVersion; +} diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index ceb66a9d22..8792ecfa5d 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -22,6 +22,7 @@ #include #include +#include #include #include #include @@ -122,6 +123,20 @@ public: void processSetTraitsMessage(ReceivedMessage& message); + using TraitsCheckTimestamp = std::chrono::steady_clock::time_point; + + TraitsCheckTimestamp getLastReceivedTraitsChange() const { return _lastReceivedTraitsChange; } + AvatarTraits::TraitVersion getLastReceivedSimpleTraitVersion(AvatarTraits::TraitType traitType) const + { return _receivedSimpleTraitVersions[traitType]; } + + TraitsCheckTimestamp getLastOtherAvatarTraitsSendPoint(Node::LocalID otherAvatar) const; + void setLastOtherAvatarTraitsSendPoint(Node::LocalID otherAvatar, TraitsCheckTimestamp sendPoint) + { _lastSentTraitsTimestamps[otherAvatar] = sendPoint; } + + AvatarTraits::TraitVersion getLastSentSimpleTraitVersion(Node::LocalID otherAvatar, AvatarTraits::TraitType traitType) const; + void setLastSentSimpleTraitVersion(Node::LocalID otherAvatar, AvatarTraits::TraitType traitType, + AvatarTraits::TraitVersion traitVersion); + private: struct PacketQueue : public std::queue> { QWeakPointer node; @@ -158,6 +173,12 @@ private: int _recentOtherAvatarsOutOfView { 0 }; QString _baseDisplayName{}; // The santized key used in determinging unique sessionDisplayName, so that we can remove from dictionary. bool _requestsDomainListData { false }; + + AvatarTraits::SimpleTraitVersions _receivedSimpleTraitVersions; + TraitsCheckTimestamp _lastReceivedTraitsChange; + + std::unordered_map _lastSentTraitsTimestamps; + std::unordered_map _sentSimpleTraitVersions; }; #endif // hifi_AvatarMixerClientData_h diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 563aac879f..c996008a48 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -79,6 +79,61 @@ int AvatarMixerSlave::sendIdentityPacket(const AvatarMixerClientData* nodeData, } } +void AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* listeningNodeData, + const AvatarMixerClientData* sendingNodeData, + NLPacketList& traitsPacketList) { + + auto otherNodeLocalID = sendingNodeData->getNodeLocalID(); + + // Perform a simple check with two server clock time points + // to see if there is any new traits data for this avatar that we need to send + auto timeOfLastTraitsSent = listeningNodeData->getLastOtherAvatarTraitsSendPoint(otherNodeLocalID); + auto timeOfLastTraitsChange = sendingNodeData->getLastReceivedTraitsChange(); + + if (timeOfLastTraitsChange > timeOfLastTraitsSent) { + // there is definitely new traits data to send + + // add the avatar ID to mark the beginning of traits for this avatar + traitsPacketList.write(sendingNodeData->getNodeID().toRfc4122()); + + auto sendingAvatar = sendingNodeData->getAvatarSharedPointer(); + + // compare trait versions so we can see what exactly needs to go out + for (int i = 0; i < AvatarTraits::TotalTraitTypes; ++i) { + AvatarTraits::TraitType traitType = static_cast(i); + + auto lastSentVersion = listeningNodeData->getLastSentSimpleTraitVersion(otherNodeLocalID, traitType); + auto lastReceivedVersion = sendingNodeData->getLastReceivedSimpleTraitVersion(traitType); + + if (lastReceivedVersion > lastSentVersion) { + // there is an update to this trait, add it to the traits packet + + // write the trait type and the trait version + traitsPacketList.writePrimitive(traitType); + traitsPacketList.writePrimitive(lastReceivedVersion); + + // update the last sent version since we're adding this to the packet + listeningNodeData->setLastSentSimpleTraitVersion(otherNodeLocalID, traitType, lastReceivedVersion); + + if (traitType == AvatarTraits::SkeletonModelURL) { + // get an encoded version of the URL, write its size and then the data itself + auto encodedSkeletonURL = sendingAvatar->getSkeletonModelURL().toEncoded(); + + traitsPacketList.writePrimitive(uint16_t(encodedSkeletonURL.size())); + traitsPacketList.write(encodedSkeletonURL); + } + } + } + + // write a null trait type to mark the end of trait data for this avatar + traitsPacketList.writePrimitive(AvatarTraits::NullTrait); + + // since we send all traits for this other avatar, update the time of last traits sent + // to match the time of last traits change + listeningNodeData->setLastOtherAvatarTraitsSendPoint(otherNodeLocalID, timeOfLastTraitsChange); + } +} + int AvatarMixerSlave::sendReplicatedIdentityPacket(const Node& agentNode, const AvatarMixerClientData* nodeData, const Node& destinationNode) { if (AvatarMixer::shouldReplicateTo(agentNode, destinationNode)) { QByteArray individualData = nodeData->getConstAvatarData()->identityByteArray(true); @@ -326,6 +381,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // loop through our sorted avatars and allocate our bandwidth to them accordingly int remainingAvatars = (int)sortedAvatars.size(); + auto traitsPacketList = NLPacketList::create(PacketType::BulkAvatarTraits, QByteArray(), true, true); while (!sortedAvatars.empty()) { const auto avatarData = sortedAvatars.top().getAvatar(); sortedAvatars.pop(); @@ -392,11 +448,12 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) quint64 start = usecTimestampNow(); QByteArray bytes = otherAvatar->toByteArray(detail, lastEncodeForOther, lastSentJointsForOther, - hasFlagsOut, dropFaceTracking, distanceAdjust, viewerPosition, &lastSentJointsForOther); + hasFlagsOut, dropFaceTracking, distanceAdjust, viewerPosition, + &lastSentJointsForOther); quint64 end = usecTimestampNow(); _stats.toByteArrayElapsedTime += (end - start); - auto maxAvatarDataBytes = avatarPacketList->getMaxSegmentSize() - NUM_BYTES_RFC4122_UUID; + static auto maxAvatarDataBytes = avatarPacketList->getMaxSegmentSize() - NUM_BYTES_RFC4122_UUID; if (bytes.size() > maxAvatarDataBytes) { qCWarning(avatars) << "otherAvatar.toByteArray() for" << otherNode->getUUID() << "resulted in very large buffer of" << bytes.size() << "bytes - dropping facial data"; @@ -445,6 +502,9 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) quint64 endAvatarDataPacking = usecTimestampNow(); _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); + + // use helper to add any changed traits to our packet list + addChangedTraitsToBulkPacket(nodeData, otherNodeData, *traitsPacketList); } quint64 startPacketSending = usecTimestampNow(); @@ -461,6 +521,14 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // record the bytes sent for other avatar data in the AvatarMixerClientData nodeData->recordSentAvatarData(numAvatarDataBytes); + // close the current traits packet list + traitsPacketList->closeCurrentPacket(); + + if (traitsPacketList->getNumPackets() >= 1) { + // send the traits packet list + nodeList->sendPacketList(std::move(traitsPacketList), *node); + } + // record the number of avatars held back this frame nodeData->recordNumOtherAvatarStarves(numAvatarsHeldBack); nodeData->recordNumOtherAvatarSkips(numAvatarsWithSkippedFrames); diff --git a/assignment-client/src/avatars/AvatarMixerSlave.h b/assignment-client/src/avatars/AvatarMixerSlave.h index 7be119c4b2..ed27709e3e 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.h +++ b/assignment-client/src/avatars/AvatarMixerSlave.h @@ -99,6 +99,10 @@ private: int sendIdentityPacket(const AvatarMixerClientData* nodeData, const SharedNodePointer& destinationNode); int sendReplicatedIdentityPacket(const Node& agentNode, const AvatarMixerClientData* nodeData, const Node& destinationNode); + void addChangedTraitsToBulkPacket(AvatarMixerClientData* listeningNodeData, + const AvatarMixerClientData* sendingNodeData, + NLPacketList& traitsPacketList); + void broadcastAvatarDataToAgent(const SharedNodePointer& node); void broadcastAvatarDataToDownstreamMixer(const SharedNodePointer& node); diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h index c35bfae95c..90258982c3 100644 --- a/libraries/avatars/src/AvatarTraits.h +++ b/libraries/avatars/src/AvatarTraits.h @@ -13,15 +13,25 @@ #define hifi_AvatarTraits_h #include +#include +#include namespace AvatarTraits { - enum Trait : uint8_t { + enum TraitType : int8_t { + NullTrait = -1, SkeletonModelURL, - TotalTraits + TotalTraitTypes }; + using TraitTypeSet = std::set; + const TraitTypeSet SimpleTraitTypes = { SkeletonModelURL }; + using TraitVersion = uint32_t; const TraitVersion DEFAULT_TRAIT_VERSION = 0; + + using TraitWireSize = uint16_t; + + using SimpleTraitVersions = std::vector; } #endif // hifi_AvatarTraits_h diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp index c4685461a0..5d1f2e4926 100644 --- a/libraries/avatars/src/ClientTraitsHandler.cpp +++ b/libraries/avatars/src/ClientTraitsHandler.cpp @@ -68,9 +68,11 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() { auto encodedSkeletonURL = _owningAvatar->getSkeletonModelURL().toEncoded(); - uint16_t encodedURLSize = encodedSkeletonURL.size(); + AvatarTraits::TraitWireSize encodedURLSize = encodedSkeletonURL.size(); traitsPacketList->writePrimitive(encodedURLSize); + qDebug() << "Sending trait of size" << encodedURLSize; + traitsPacketList->write(encodedSkeletonURL); traitsPacketList->endSegment(); diff --git a/libraries/avatars/src/ClientTraitsHandler.h b/libraries/avatars/src/ClientTraitsHandler.h index 9fd3104e7e..4aea0bb433 100644 --- a/libraries/avatars/src/ClientTraitsHandler.h +++ b/libraries/avatars/src/ClientTraitsHandler.h @@ -25,16 +25,16 @@ public: void sendChangedTraitsToMixer(); bool hasChangedTraits() { return _changedTraits.size(); } - void markTraitChanged(AvatarTraits::Trait changedTrait) { _changedTraits.insert(changedTrait); } + void markTraitChanged(AvatarTraits::TraitType changedTrait) { _changedTraits.insert(changedTrait); } - bool hasTraitChanged(AvatarTraits::Trait checkTrait) { return _changedTraits.count(checkTrait) > 0; } + bool hasTraitChanged(AvatarTraits::TraitType checkTrait) { return _changedTraits.count(checkTrait) > 0; } void resetForNewMixer(); private: AvatarData* _owningAvatar; - std::set _changedTraits; + AvatarTraits::TraitTypeSet _changedTraits; AvatarTraits::TraitVersion _currentTraitVersion { AvatarTraits::DEFAULT_TRAIT_VERSION }; bool _performInitialSend { false }; }; diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 3990afa79a..616694c8da 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -133,7 +133,8 @@ public: EntityClone, EntityQueryInitialResultsComplete, - + BulkAvatarTraits, + NUM_PACKET_TYPE }; From a80d19a44a393c45e6855cd97c0a9d8bf49e72d2 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 7 Aug 2018 15:38:57 -0700 Subject: [PATCH 05/31] remove skeleton from identity, handle in clients --- assignment-client/src/Agent.cpp | 5 -- .../src/avatars/AvatarMixerClientData.cpp | 15 +--- .../src/scripts/EntityScriptServer.cpp | 3 - interface/src/avatar/AvatarManager.cpp | 4 - interface/src/avatar/MyAvatar.cpp | 2 - libraries/avatars/src/AvatarData.cpp | 27 +++---- libraries/avatars/src/AvatarData.h | 7 +- libraries/avatars/src/AvatarHashMap.cpp | 76 ++++++++++++++++++- libraries/avatars/src/AvatarHashMap.h | 7 ++ .../networking/src/udt/PacketHeaders.cpp | 2 +- libraries/networking/src/udt/PacketHeaders.h | 3 +- 11 files changed, 104 insertions(+), 47 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index f534be9346..4cc24e2110 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -447,11 +447,6 @@ void Agent::executeScript() { auto avatarHashMap = DependencyManager::set(); _scriptEngine->registerGlobalObject("AvatarList", avatarHashMap.data()); - auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); - packetReceiver.registerListener(PacketType::BulkAvatarData, avatarHashMap.data(), "processAvatarDataPacket"); - packetReceiver.registerListener(PacketType::KillAvatar, avatarHashMap.data(), "processKillAvatar"); - packetReceiver.registerListener(PacketType::AvatarIdentity, avatarHashMap.data(), "processAvatarIdentityPacket"); - // register ourselves to the script engine _scriptEngine->registerGlobalObject("Agent", new AgentScriptingInterface(this)); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index cf30aca1a7..f279d76450 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -109,17 +109,10 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message) { message.readPrimitive(&traitSize); if (packetTraitVersion > _receivedSimpleTraitVersions[traitType]) { - if (traitType == AvatarTraits::SkeletonModelURL) { - // get the URL from the binary data - auto skeletonModelURL = QUrl::fromEncoded(message.read(traitSize)); - _avatar->setSkeletonModelURL(skeletonModelURL); - - qDebug() << "Set skeleton URL to" << skeletonModelURL << "for trait packet version" << packetTraitVersion; - - _receivedSimpleTraitVersions[traitType] = packetTraitVersion; - - anyTraitsChanged = true; - } + _avatar->processTrait(traitType, message.readWithoutCopy(traitSize)); + _receivedSimpleTraitVersions[traitType] = packetTraitVersion; + + anyTraitsChanged = true; } else { message.seek(message.getPosition() + traitSize); } diff --git a/assignment-client/src/scripts/EntityScriptServer.cpp b/assignment-client/src/scripts/EntityScriptServer.cpp index 0e1126cebe..05002828f5 100644 --- a/assignment-client/src/scripts/EntityScriptServer.cpp +++ b/assignment-client/src/scripts/EntityScriptServer.cpp @@ -81,9 +81,6 @@ EntityScriptServer::EntityScriptServer(ReceivedMessage& message) : ThreadedAssig packetReceiver.registerListener(PacketType::SelectedAudioFormat, this, "handleSelectedAudioFormat"); auto avatarHashMap = DependencyManager::set(); - packetReceiver.registerListener(PacketType::BulkAvatarData, avatarHashMap.data(), "processAvatarDataPacket"); - packetReceiver.registerListener(PacketType::KillAvatar, avatarHashMap.data(), "processKillAvatar"); - packetReceiver.registerListener(PacketType::AvatarIdentity, avatarHashMap.data(), "processAvatarIdentityPacket"); packetReceiver.registerListener(PacketType::ReloadEntityServerScript, this, "handleReloadEntityServerScriptPacket"); packetReceiver.registerListener(PacketType::EntityScriptGetStatus, this, "handleEntityScriptGetStatusPacket"); diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 09fa6dc573..8569aaf05a 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -72,10 +72,6 @@ AvatarManager::AvatarManager(QObject* parent) : qRegisterMetaType >("NodeWeakPointer"); auto nodeList = DependencyManager::get(); - auto& packetReceiver = nodeList->getPacketReceiver(); - packetReceiver.registerListener(PacketType::BulkAvatarData, this, "processAvatarDataPacket"); - packetReceiver.registerListener(PacketType::KillAvatar, this, "processKillAvatar"); - packetReceiver.registerListener(PacketType::AvatarIdentity, this, "processAvatarIdentityPacket"); // when we hear that the user has ignored an avatar by session UUID // immediately remove that avatar instead of waiting for the absence of packets from avatar mixer diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 53ad8c0a0f..df9afeb92d 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1705,8 +1705,6 @@ void MyAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { if (previousSkeletonModelURL != _skeletonModelURL) { _clientTraitsHandler.markTraitChanged(AvatarTraits::SkeletonModelURL); - } else { - qDebug() << "Not marking skeleton model URL trait changed since the new value matches the previous"; } } diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 62c7a7053f..ddfeb4df24 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -42,6 +42,7 @@ #include #include "AvatarLogging.h" +#include "AvatarTraits.h" //#define WANT_DEBUG @@ -1756,7 +1757,7 @@ QUrl AvatarData::cannonicalSkeletonModelURL(const QUrl& emptyURL) const { } void AvatarData::processAvatarIdentity(const QByteArray& identityData, bool& identityChanged, - bool& displayNameChanged, bool& skeletonModelUrlChanged) { + bool& displayNameChanged) { QDataStream packetStream(identityData); @@ -1777,7 +1778,7 @@ void AvatarData::processAvatarIdentity(const QByteArray& identityData, bool& ide if (incomingSequenceNumber > _identitySequenceNumber) { Identity identity; - packetStream >> identity.skeletonModelURL + packetStream >> identity.attachmentData >> identity.displayName >> identity.sessionDisplayName @@ -1789,16 +1790,6 @@ void AvatarData::processAvatarIdentity(const QByteArray& identityData, bool& ide // set the store identity sequence number to match the incoming identity _identitySequenceNumber = incomingSequenceNumber; - if (_firstSkeletonCheck || (identity.skeletonModelURL != cannonicalSkeletonModelURL(emptyURL))) { - setSkeletonModelURL(identity.skeletonModelURL); - identityChanged = true; - skeletonModelUrlChanged = true; - if (_firstSkeletonCheck) { - displayNameChanged = true; - } - _firstSkeletonCheck = false; - } - if (identity.displayName != _displayName) { _displayName = identity.displayName; identityChanged = true; @@ -1834,7 +1825,6 @@ void AvatarData::processAvatarIdentity(const QByteArray& identityData, bool& ide #ifdef WANT_DEBUG qCDebug(avatars) << __FUNCTION__ << "identity.uuid:" << identity.uuid - << "identity.skeletonModelURL:" << identity.skeletonModelURL << "identity.displayName:" << identity.displayName << "identity.sessionDisplayName:" << identity.sessionDisplayName; } else { @@ -1846,10 +1836,18 @@ void AvatarData::processAvatarIdentity(const QByteArray& identityData, bool& ide } } +void AvatarData::processTrait(AvatarTraits::TraitType traitType, QByteArray traitBinaryData) { + if (traitType == AvatarTraits::SkeletonModelURL) { + // get the URL from the binary data + auto skeletonModelURL = QUrl::fromEncoded(traitBinaryData); + qDebug() << "Setting skeleton model URL from trait packet to" << skeletonModelURL; + setSkeletonModelURL(skeletonModelURL); + } +} + QByteArray AvatarData::identityByteArray(bool setIsReplicated) const { QByteArray identityData; QDataStream identityStream(&identityData, QIODevice::Append); - const QUrl& urlToSend = cannonicalSkeletonModelURL(emptyURL); // depends on _skeletonModelURL // when mixers send identity packets to agents, they simply forward along the last incoming sequence number they received // whereas agents send a fresh outgoing sequence number when identity data has changed @@ -1857,7 +1855,6 @@ QByteArray AvatarData::identityByteArray(bool setIsReplicated) const { _avatarEntitiesLock.withReadLock([&] { identityStream << getSessionUUID() << (udt::SequenceNumber::Type) _identitySequenceNumber - << urlToSend << _attachmentData << _displayName << getSessionDisplayNameForTransport() // depends on _sessionDisplayName diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index fcc63fdc98..12e8370b86 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -51,6 +51,7 @@ #include #include "AABox.h" +#include "AvatarTraits.h" #include "HeadData.h" #include "PathUtils.h" @@ -955,8 +956,9 @@ public: // identityChanged returns true if identity has changed, false otherwise. // identityChanged returns true if identity has changed, false otherwise. Similarly for displayNameChanged and skeletonModelUrlChange. - void processAvatarIdentity(const QByteArray& identityData, bool& identityChanged, - bool& displayNameChanged, bool& skeletonModelUrlChanged); + void processAvatarIdentity(const QByteArray& identityData, bool& identityChanged, bool& displayNameChanged); + + void processTrait(AvatarTraits::TraitType traitType, QByteArray traitBinaryData); QByteArray identityByteArray(bool setIsReplicated = false) const; @@ -1327,7 +1329,6 @@ protected: mutable HeadData* _headData { nullptr }; QUrl _skeletonModelURL; - bool _firstSkeletonCheck { true }; QUrl _skeletonFBXURL; QVector _attachmentData; QVector _oldAttachmentData; diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 174e81bb31..407e88e27c 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -19,10 +19,17 @@ #include #include "AvatarLogging.h" +#include "AvatarTraits.h" AvatarHashMap::AvatarHashMap() { auto nodeList = DependencyManager::get(); + auto& packetReceiver = nodeList->getPacketReceiver(); + packetReceiver.registerListener(PacketType::BulkAvatarData, this, "processAvatarDataPacket"); + packetReceiver.registerListener(PacketType::KillAvatar, this, "processKillAvatar"); + packetReceiver.registerListener(PacketType::AvatarIdentity, this, "processAvatarIdentityPacket"); + packetReceiver.registerListener(PacketType::BulkAvatarTraits, this, "processBulkAvatarTraits"); + connect(nodeList.data(), &NodeList::uuidChanged, this, &AvatarHashMap::sessionUUIDChanged); } @@ -182,9 +189,74 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer auto avatar = newOrExistingAvatar(identityUUID, sendingNode, isNewAvatar); bool identityChanged = false; bool displayNameChanged = false; - bool skeletonModelUrlChanged = false; // In this case, the "sendingNode" is the Avatar Mixer. - avatar->processAvatarIdentity(message->getMessage(), identityChanged, displayNameChanged, skeletonModelUrlChanged); + avatar->processAvatarIdentity(message->getMessage(), identityChanged, displayNameChanged); + } +} + +bool AvatarHashMap::checkLastProcessedTraitVersion(QUuid avatarID, + AvatarTraits::TraitType traitType, AvatarTraits::TraitVersion newVersion) { + auto it = _processedSimpleTraitVersions.find(avatarID); + if (it == _processedSimpleTraitVersions.end()) { + auto pair = _processedSimpleTraitVersions.insert({ + avatarID, + { AvatarTraits::TotalTraitTypes, AvatarTraits::DEFAULT_TRAIT_VERSION } + }); + + it = pair.first; + }; + + if (it->second[traitType] < newVersion) { + it->second[traitType] = newVersion; + return true; + } else { + return false; + } +} + +void AvatarHashMap::processBulkAvatarTraits(QSharedPointer message, SharedNodePointer sendingNode) { + while (message->getBytesLeftToRead()) { + // read the avatar ID to figure out which avatar this is for + auto avatarID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); + + // grab the avatar so we can ask it to process trait data + AvatarSharedPointer avatar; + + QReadLocker locker(&_hashLock); + auto it = _avatarHash.find(avatarID); + if (it != _avatarHash.end()) { + avatar = *it; + } + locker.unlock(); + + // read the first trait type for this avatar + AvatarTraits::TraitType traitType; + message->readPrimitive(&traitType); + + while (traitType != AvatarTraits::NullTrait) { + AvatarTraits::TraitVersion traitVersion; + message->readPrimitive(&traitVersion); + + AvatarTraits::TraitWireSize traitBinarySize; + message->readPrimitive(&traitBinarySize); + + if (avatar) { + // check if this trait version is newer than what we already have for this avatar + bool traitIsNewer = checkLastProcessedTraitVersion(avatarID, traitType, traitVersion); + if (traitIsNewer) { + avatar->processTrait(traitType, message->readWithoutCopy(traitBinarySize)); + } else { + message->seek(message->getPosition() + traitBinarySize); + } + } else { + // though we have no avatar pointer, we still hop through the packet in case there are + // traits for avatars we do have later in the packet + message->seek(message->getPosition() + traitBinarySize); + } + + // read the next trait type, which is null if there are no more traits for this avatar + message->readPrimitive(&traitType); + } } } diff --git a/libraries/avatars/src/AvatarHashMap.h b/libraries/avatars/src/AvatarHashMap.h index fd2cd76fbf..ed8440eb89 100644 --- a/libraries/avatars/src/AvatarHashMap.h +++ b/libraries/avatars/src/AvatarHashMap.h @@ -30,6 +30,7 @@ #include "ScriptAvatarData.h" #include "AvatarData.h" +#include "AvatarTraits.h" /**jsdoc * Note: An AvatarList API is also provided for Interface and client entity scripts: it is a @@ -133,6 +134,8 @@ protected slots: */ void processAvatarIdentityPacket(QSharedPointer message, SharedNodePointer sendingNode); + void processBulkAvatarTraits(QSharedPointer message, SharedNodePointer sendingNode); + /**jsdoc * @function AvatarList.processKillAvatar * @param {} message @@ -153,6 +156,9 @@ protected: virtual void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason = KillAvatarReason::NoReason); + bool checkLastProcessedTraitVersion(QUuid avatarID, + AvatarTraits::TraitType traitType, AvatarTraits::TraitVersion newVersion); + AvatarHash _avatarHash; struct PendingAvatar { std::chrono::steady_clock::time_point creationTime; @@ -163,6 +169,7 @@ protected: AvatarPendingHash _pendingAvatars; mutable QReadWriteLock _hashLock; + std::unordered_map _processedSimpleTraitVersions; private: QUuid _lastOwnerSessionUUID; }; diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index 8b50e37699..94137786cd 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -40,7 +40,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::AvatarData: case PacketType::BulkAvatarData: case PacketType::KillAvatar: - return static_cast(AvatarMixerPacketVersion::FarGrabJoints); + return static_cast(AvatarMixerPacketVersion::MigrateSkeletonURLToTraits); case PacketType::MessagesData: return static_cast(MessageDataVersion::TextOrBinaryData); // ICE packets diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 616694c8da..31724ab5dc 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -134,7 +134,7 @@ public: EntityClone, EntityQueryInitialResultsComplete, BulkAvatarTraits, - + NUM_PACKET_TYPE }; @@ -291,6 +291,7 @@ enum class AvatarMixerPacketVersion : PacketVersion { FixMannequinDefaultAvatarFeet, ProceduralFaceMovementFlagsAndBlendshapes, FarGrabJoints + MigrateSkeletonURLToTraits }; enum class DomainConnectRequestVersion : PacketVersion { From be7eb572058815eec125ee1d43880acaf6bd8b66 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 7 Aug 2018 18:08:10 -0700 Subject: [PATCH 06/31] handle whitelist avatar URL override via traits --- assignment-client/src/Agent.cpp | 23 ++++++-- assignment-client/src/AssignmentClient.cpp | 19 +------ assignment-client/src/avatars/AvatarMixer.cpp | 53 +++++-------------- assignment-client/src/avatars/AvatarMixer.h | 8 +-- .../src/avatars/AvatarMixerClientData.cpp | 53 +++++++++++++++++-- .../src/avatars/AvatarMixerClientData.h | 10 ++-- .../src/avatars/AvatarMixerSlave.cpp | 16 ++---- .../src/avatars/AvatarMixerSlave.h | 6 +++ .../src/avatars/AvatarMixerSlavePool.cpp | 2 +- .../src/avatars/AvatarMixerSlavePool.h | 8 ++- interface/src/Application.cpp | 4 +- libraries/avatars/src/AvatarData.cpp | 27 +++++++--- libraries/avatars/src/AvatarData.h | 2 +- libraries/avatars/src/AvatarTraits.h | 3 ++ libraries/avatars/src/ClientTraitsHandler.cpp | 48 ++++++++++++----- libraries/avatars/src/ClientTraitsHandler.h | 12 ++++- libraries/networking/src/ExtendedIODevice.h | 39 ++++++++++++++ libraries/networking/src/udt/BasePacket.cpp | 2 +- libraries/networking/src/udt/BasePacket.h | 22 +------- libraries/networking/src/udt/PacketList.h | 18 +------ 20 files changed, 224 insertions(+), 151 deletions(-) create mode 100644 libraries/networking/src/ExtendedIODevice.h diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index 4cc24e2110..049e7d0ede 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -25,16 +25,19 @@ #include #include #include +#include #include #include #include #include #include #include +#include #include #include #include #include +#include #include #include @@ -49,12 +52,12 @@ #include #include // TODO: consider moving to scriptengine.h +#include "AssignmentDynamicFactory.h" #include "entities/AssignmentParentFinder.h" #include "RecordingScriptingInterface.h" #include "AbstractAudioInterface.h" #include "AgentScriptingInterface.h" - static const int RECEIVED_AUDIO_STREAM_CAPACITY_FRAMES = 10; Agent::Agent(ReceivedMessage& message) : @@ -63,6 +66,18 @@ Agent::Agent(ReceivedMessage& message) : _audioGate(AudioConstants::SAMPLE_RATE, AudioConstants::MONO), _avatarAudioTimer(this) { + DependencyManager::set(); + + DependencyManager::set(); + DependencyManager::set(); + DependencyManager::set(false); + + DependencyManager::registerInheritance(); + DependencyManager::set(); + + DependencyManager::set(); + DependencyManager::set(); + _entityEditSender.setPacketsPerSecond(DEFAULT_ENTITY_PPS_PER_SCRIPT); DependencyManager::get()->setPacketSender(&_entityEditSender); @@ -99,7 +114,6 @@ Agent::Agent(ReceivedMessage& message) : this, "handleOctreePacket"); packetReceiver.registerListener(PacketType::SelectedAudioFormat, this, "handleSelectedAudioFormat"); - // 100Hz timer for audio const int TARGET_INTERVAL_MSEC = 10; // 10ms connect(&_avatarAudioTimer, &QTimer::timeout, this, &Agent::processAgentAvatarAudio); @@ -439,7 +453,7 @@ void Agent::executeScript() { encodedBuffer = audio; } - AbstractAudioInterface::emitAudioPacket(encodedBuffer.data(), encodedBuffer.size(), audioSequenceNumber, false, + AbstractAudioInterface::emitAudioPacket(encodedBuffer.data(), encodedBuffer.size(), audioSequenceNumber, false, audioTransform, scriptedAvatar->getWorldPosition(), glm::vec3(0), packetType, _selectedCodecName); }); @@ -842,6 +856,9 @@ void Agent::aboutToFinish() { DependencyManager::destroy(); DependencyManager::destroy(); DependencyManager::destroy(); + + DependencyManager::destroy(); + QMetaObject::invokeMethod(&_avatarAudioTimer, "stop"); // cleanup codec & encoder diff --git a/assignment-client/src/AssignmentClient.cpp b/assignment-client/src/AssignmentClient.cpp index d761699285..426f3ce6fc 100644 --- a/assignment-client/src/AssignmentClient.cpp +++ b/assignment-client/src/AssignmentClient.cpp @@ -21,10 +21,7 @@ #include #include #include -#include #include -#include -#include #include #include #include @@ -32,16 +29,12 @@ #include #include #include -#include -#include -#include + #include #include #include "AssignmentClientLogging.h" -#include "AssignmentDynamicFactory.h" #include "AssignmentFactory.h" -#include "avatars/ScriptableAvatar.h" const QString ASSIGNMENT_CLIENT_TARGET_NAME = "assignment-client"; const long long ASSIGNMENT_REQUEST_INTERVAL_MSECS = 1 * 1000; @@ -57,21 +50,11 @@ AssignmentClient::AssignmentClient(Assignment::Type requestAssignmentType, QStri DependencyManager::set(); DependencyManager::set(); - auto scriptableAvatar = DependencyManager::set(); auto addressManager = DependencyManager::set(); // create a NodeList as an unassigned client, must be after addressManager auto nodeList = DependencyManager::set(NodeType::Unassigned, listenPort); - auto animationCache = DependencyManager::set(); - DependencyManager::set(); - auto entityScriptingInterface = DependencyManager::set(false); - - DependencyManager::registerInheritance(); - auto dynamicFactory = DependencyManager::set(); - DependencyManager::set(); - DependencyManager::set(); - nodeList->startThread(); // set the logging target to the the CHILD_TARGET_NAME LogHandler::getInstance().setTargetName(ASSIGNMENT_CLIENT_TARGET_NAME); diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index b139870e6e..228102ee53 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -39,7 +39,8 @@ const QString AVATAR_MIXER_LOGGING_NAME = "avatar-mixer"; const int AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND = 45; AvatarMixer::AvatarMixer(ReceivedMessage& message) : - ThreadedAssignment(message) + ThreadedAssignment(message), + _slavePool(&_slaveSharedData) { // make sure we hear about node kills so we can tell the other nodes connect(DependencyManager::get().data(), &NodeList::nodeKilled, this, &AvatarMixer::handleAvatarKilled); @@ -338,17 +339,7 @@ void AvatarMixer::manageIdentityData(const SharedNodePointer& node) { sendIdentity = true; qCDebug(avatars) << "Giving session display name" << sessionDisplayName << "to node with ID" << node->getUUID(); } - if (nodeData && nodeData->getAvatarSkeletonModelUrlMustChange()) { // never true for an empty _avatarWhitelist - nodeData->setAvatarSkeletonModelUrlMustChange(false); - AvatarData& avatar = nodeData->getAvatar(); - static const QUrl emptyURL(""); - QUrl url = avatar.cannonicalSkeletonModelURL(emptyURL); - if (!isAvatarInWhitelist(url)) { - qCDebug(avatars) << "Forbidden avatar" << nodeData->getNodeID() << avatar.getSkeletonModelURL() << "replaced with" << (_replacementAvatar.isEmpty() ? "default" : _replacementAvatar); - avatar.setSkeletonModelURL(_replacementAvatar); - sendIdentity = true; - } - } + if (sendIdentity && !node->isUpstream()) { sendIdentityPacket(nodeData, node); // Tell node whose name changed about its new session display name or avatar. // since this packet includes a change to either the skeleton model URL or the display name @@ -360,22 +351,6 @@ void AvatarMixer::manageIdentityData(const SharedNodePointer& node) { } } -bool AvatarMixer::isAvatarInWhitelist(const QUrl& url) { - // The avatar is in the whitelist if: - // 1. The avatar's URL's host matches one of the hosts of the URLs in the whitelist AND - // 2. The avatar's URL's path starts with the path of that same URL in the whitelist - for (const auto& whiteListedPrefix : _avatarWhitelist) { - auto whiteListURL = QUrl::fromUserInput(whiteListedPrefix); - // check if this script URL matches the whitelist domain and, optionally, is beneath the path - if (url.host().compare(whiteListURL.host(), Qt::CaseInsensitive) == 0 && - url.path().startsWith(whiteListURL.path(), Qt::CaseInsensitive)) { - return true; - } - } - - return false; -} - void AvatarMixer::throttle(std::chrono::microseconds duration, int frame) { // throttle using a modified proportional-integral controller const float FRAME_TIME = USECS_PER_SECOND / AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND; @@ -588,8 +563,7 @@ void AvatarMixer::handleAvatarIdentityPacket(QSharedPointer mes // parse the identity packet and update the change timestamp if appropriate bool identityChanged = false; bool displayNameChanged = false; - bool skeletonModelUrlChanged = false; - avatar.processAvatarIdentity(message->getMessage(), identityChanged, displayNameChanged, skeletonModelUrlChanged); + avatar.processAvatarIdentity(message->getMessage(), identityChanged, displayNameChanged); if (identityChanged) { QMutexLocker nodeDataLocker(&nodeData->getMutex()); @@ -597,9 +571,6 @@ void AvatarMixer::handleAvatarIdentityPacket(QSharedPointer mes if (displayNameChanged) { nodeData->setAvatarSessionDisplayNameMustChange(true); } - if (skeletonModelUrlChanged && !_avatarWhitelist.isEmpty()) { - nodeData->setAvatarSkeletonModelUrlMustChange(true); - } } } } @@ -992,20 +963,22 @@ void AvatarMixer::parseDomainServerSettings(const QJsonObject& domainSettings) { qCDebug(avatars) << "This domain requires a minimum avatar height of" << _domainMinimumHeight << "and a maximum avatar height of" << _domainMaximumHeight; - const QString AVATAR_WHITELIST_DEFAULT{ "" }; static const QString AVATAR_WHITELIST_OPTION = "avatar_whitelist"; - _avatarWhitelist = domainSettings[AVATARS_SETTINGS_KEY].toObject()[AVATAR_WHITELIST_OPTION].toString(AVATAR_WHITELIST_DEFAULT).split(',', QString::KeepEmptyParts); + _slaveSharedData.skeletonURLWhitelist = domainSettings[AVATARS_SETTINGS_KEY].toObject()[AVATAR_WHITELIST_OPTION] + .toString().split(',', QString::KeepEmptyParts); static const QString REPLACEMENT_AVATAR_OPTION = "replacement_avatar"; - _replacementAvatar = domainSettings[AVATARS_SETTINGS_KEY].toObject()[REPLACEMENT_AVATAR_OPTION].toString(REPLACEMENT_AVATAR_DEFAULT); + _slaveSharedData.skeletonReplacementURL = domainSettings[AVATARS_SETTINGS_KEY].toObject()[REPLACEMENT_AVATAR_OPTION] + .toString(); - if ((_avatarWhitelist.count() == 1) && _avatarWhitelist[0].isEmpty()) { - _avatarWhitelist.clear(); // KeepEmptyParts above will parse "," as ["", ""] (which is ok), but "" as [""] (which is not ok). + if (_slaveSharedData.skeletonURLWhitelist.count() == 1 && _slaveSharedData.skeletonURLWhitelist[0].isEmpty()) { + // KeepEmptyParts above will parse "," as ["", ""] (which is ok), but "" as [""] (which is not ok). + _slaveSharedData.skeletonURLWhitelist.clear(); } - if (_avatarWhitelist.isEmpty()) { + if (_slaveSharedData.skeletonURLWhitelist.isEmpty()) { qCDebug(avatars) << "All avatars are allowed."; } else { - qCDebug(avatars) << "Avatars other than" << _avatarWhitelist << "will be replaced by" << (_replacementAvatar.isEmpty() ? "default" : _replacementAvatar); + qCDebug(avatars) << "Avatars other than" << _slaveSharedData.skeletonURLWhitelist << "will be replaced by" << (_slaveSharedData.skeletonReplacementURL.isEmpty() ? "default" : _slaveSharedData.skeletonReplacementURL.toString()); } } diff --git a/assignment-client/src/avatars/AvatarMixer.h b/assignment-client/src/avatars/AvatarMixer.h index 14f84c9e7b..8ae7fc9931 100644 --- a/assignment-client/src/avatars/AvatarMixer.h +++ b/assignment-client/src/avatars/AvatarMixer.h @@ -59,7 +59,6 @@ private slots: void handlePacketVersionMismatch(PacketType type, const HifiSockAddr& senderSockAddr, const QUuid& senderUUID); void start(); - private: AvatarMixerClientData* getOrCreateClientData(SharedNodePointer node); std::chrono::microseconds timeFrame(p_high_resolution_clock::time_point& timestamp); @@ -69,11 +68,6 @@ private: void sendIdentityPacket(AvatarMixerClientData* nodeData, const SharedNodePointer& destinationNode); void manageIdentityData(const SharedNodePointer& node); - bool isAvatarInWhitelist(const QUrl& url); - - const QString REPLACEMENT_AVATAR_DEFAULT{ "" }; - QStringList _avatarWhitelist { }; - QString _replacementAvatar { REPLACEMENT_AVATAR_DEFAULT }; void optionallyReplicatePacket(ReceivedMessage& message, const Node& node); @@ -83,7 +77,6 @@ private: float _trailingMixRatio { 0.0f }; float _throttlingRatio { 0.0f }; - int _sumListeners { 0 }; int _numStatFrames { 0 }; int _numTightLoopFrames { 0 }; @@ -127,6 +120,7 @@ private: RateCounter<> _loopRate; // this is the rate that the main thread tight loop runs AvatarMixerSlavePool _slavePool; + SlaveSharedData _slaveSharedData; }; #endif // hifi_AvatarMixer_h diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index f279d76450..552fe9a58b 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -16,6 +16,8 @@ #include #include +#include "AvatarMixerSlave.h" + AvatarMixerClientData::AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID) : NodeData(nodeID), _receivedSimpleTraitVersions(AvatarTraits::SimpleTraitTypes.size()) @@ -48,7 +50,7 @@ void AvatarMixerClientData::queuePacket(QSharedPointer message, _packetQueue.push(message); } -int AvatarMixerClientData::processPackets() { +int AvatarMixerClientData::processPackets(SlaveSharedData* slaveSharedData) { int packetsProcessed = 0; SharedNodePointer node = _packetQueue.node; assert(_packetQueue.empty() || node); @@ -64,7 +66,7 @@ int AvatarMixerClientData::processPackets() { parseData(*packet); break; case PacketType::SetAvatarTraits: - processSetTraitsMessage(*packet); + processSetTraitsMessage(*packet, slaveSharedData, *node); break; default: Q_UNREACHABLE(); @@ -92,7 +94,7 @@ int AvatarMixerClientData::parseData(ReceivedMessage& message) { return _avatar->parseDataFromBuffer(message.readWithoutCopy(message.getBytesLeftToRead())); } -void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message) { +void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, SlaveSharedData* slaveSharedData, Node& sendingNode) { // pull the trait version from the message AvatarTraits::TraitVersion packetTraitVersion; message.readPrimitive(&packetTraitVersion); @@ -111,6 +113,11 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message) { if (packetTraitVersion > _receivedSimpleTraitVersions[traitType]) { _avatar->processTrait(traitType, message.readWithoutCopy(traitSize)); _receivedSimpleTraitVersions[traitType] = packetTraitVersion; + + if (traitType == AvatarTraits::SkeletonModelURL) { + // special handling for skeleton model URL, since we need to make sure it is in the whitelist + checkSkeletonURLAgainstWhitelist(slaveSharedData, sendingNode, packetTraitVersion); + } anyTraitsChanged = true; } else { @@ -123,6 +130,46 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message) { } } +void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(SlaveSharedData *slaveSharedData, Node& sendingNode, + AvatarTraits::TraitVersion traitVersion) { + const auto& whitelist = slaveSharedData->skeletonURLWhitelist; + + if (!whitelist.isEmpty()) { + bool inWhitelist = false; + auto avatarURL = _avatar->getSkeletonModelURL(); + + // The avatar is in the whitelist if: + // 1. The avatar's URL's host matches one of the hosts of the URLs in the whitelist AND + // 2. The avatar's URL's path starts with the path of that same URL in the whitelist + for (const auto& whiteListedPrefix : whitelist) { + auto whiteListURL = QUrl::fromUserInput(whiteListedPrefix); + // check if this script URL matches the whitelist domain and, optionally, is beneath the path + if (avatarURL.host().compare(whiteListURL.host(), Qt::CaseInsensitive) == 0 && + avatarURL.path().startsWith(whiteListURL.path(), Qt::CaseInsensitive)) { + inWhitelist = true; + + break; + } + } + + if (!inWhitelist) { + // we need to change this avatar's skeleton URL, and send them a traits packet informing them of the change + _avatar->setSkeletonModelURL(slaveSharedData->skeletonReplacementURL); + + qDebug() << "Sending overwritten" << _avatar->getSkeletonModelURL() << "back to sending avatar"; + + auto packet = NLPacket::create(PacketType::SetAvatarTraits, -1, true); + + // the returned set traits packet uses the trait version from the incoming packet + // so the client knows they should not overwrite if they have since changed the trait + _avatar->packTrait(AvatarTraits::SkeletonModelURL, *packet, traitVersion); + + auto nodeList = DependencyManager::get(); + nodeList->sendPacket(std::move(packet), sendingNode); + } + } +} + uint64_t AvatarMixerClientData::getLastBroadcastTime(const QUuid& nodeUUID) const { // return the matching PacketSequenceNumber, or the default if we don't have it auto nodeMatch = _lastBroadcastTimes.find(nodeUUID); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 8792ecfa5d..96b420afc1 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -34,6 +34,8 @@ const QString OUTBOUND_AVATAR_DATA_STATS_KEY = "outbound_av_data_kbps"; const QString INBOUND_AVATAR_DATA_STATS_KEY = "inbound_av_data_kbps"; +struct SlaveSharedData; + class AvatarMixerClientData : public NodeData { Q_OBJECT public: @@ -66,8 +68,6 @@ public: void flagIdentityChange() { _identityChangeTimestamp = usecTimestampNow(); } bool getAvatarSessionDisplayNameMustChange() const { return _avatarSessionDisplayNameMustChange; } void setAvatarSessionDisplayNameMustChange(bool set = true) { _avatarSessionDisplayNameMustChange = set; } - bool getAvatarSkeletonModelUrlMustChange() const { return _avatarSkeletonModelUrlMustChange; } - void setAvatarSkeletonModelUrlMustChange(bool set = true) { _avatarSkeletonModelUrlMustChange = set; } void resetNumAvatarsSentLastFrame() { _numAvatarsSentLastFrame = 0; } void incrementNumAvatarsSentLastFrame() { ++_numAvatarsSentLastFrame; } @@ -119,9 +119,11 @@ public: QVector& getLastOtherAvatarSentJoints(QUuid otherAvatar) { return _lastOtherAvatarSentJoints[otherAvatar]; } void queuePacket(QSharedPointer message, SharedNodePointer node); - int processPackets(); // returns number of packets processed + int processPackets(SlaveSharedData* slaveSharedData); // returns number of packets processed - void processSetTraitsMessage(ReceivedMessage& message); + void processSetTraitsMessage(ReceivedMessage& message, SlaveSharedData* slaveSharedData, Node& sendingNode); + void checkSkeletonURLAgainstWhitelist(SlaveSharedData* slaveSharedData, Node& sendingNode, + AvatarTraits::TraitVersion traitVersion); using TraitsCheckTimestamp = std::chrono::steady_clock::time_point; diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index c996008a48..88e394bc95 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -59,7 +59,7 @@ void AvatarMixerSlave::processIncomingPackets(const SharedNodePointer& node) { auto nodeData = dynamic_cast(node->getLinkedData()); if (nodeData) { _stats.nodesProcessed++; - _stats.packetsProcessed += nodeData->processPackets(); + _stats.packetsProcessed += nodeData->processPackets(_sharedData); } auto end = usecTimestampNow(); _stats.processIncomingPacketsElapsedTime += (end - start); @@ -108,20 +108,10 @@ void AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* liste if (lastReceivedVersion > lastSentVersion) { // there is an update to this trait, add it to the traits packet - // write the trait type and the trait version - traitsPacketList.writePrimitive(traitType); - traitsPacketList.writePrimitive(lastReceivedVersion); - - // update the last sent version since we're adding this to the packet + // update the last sent version listeningNodeData->setLastSentSimpleTraitVersion(otherNodeLocalID, traitType, lastReceivedVersion); - if (traitType == AvatarTraits::SkeletonModelURL) { - // get an encoded version of the URL, write its size and then the data itself - auto encodedSkeletonURL = sendingAvatar->getSkeletonModelURL().toEncoded(); - - traitsPacketList.writePrimitive(uint16_t(encodedSkeletonURL.size())); - traitsPacketList.write(encodedSkeletonURL); - } + sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion); } } diff --git a/assignment-client/src/avatars/AvatarMixerSlave.h b/assignment-client/src/avatars/AvatarMixerSlave.h index ed27709e3e..5112faae29 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.h +++ b/assignment-client/src/avatars/AvatarMixerSlave.h @@ -78,11 +78,16 @@ public: jobElapsedTime += rhs.jobElapsedTime; return *this; } +}; +struct SlaveSharedData { + QStringList skeletonURLWhitelist; + QUrl skeletonReplacementURL; }; class AvatarMixerSlave { public: + AvatarMixerSlave(SlaveSharedData* sharedData) : _sharedData(sharedData) {}; using ConstIter = NodeList::const_iterator; void configure(ConstIter begin, ConstIter end); @@ -115,6 +120,7 @@ private: float _throttlingRatio { 0.0f }; AvatarMixerSlaveStats _stats; + SlaveSharedData* _sharedData; }; #endif // hifi_AvatarMixerSlave_h diff --git a/assignment-client/src/avatars/AvatarMixerSlavePool.cpp b/assignment-client/src/avatars/AvatarMixerSlavePool.cpp index 962bba21d2..cf842ac792 100644 --- a/assignment-client/src/avatars/AvatarMixerSlavePool.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlavePool.cpp @@ -168,7 +168,7 @@ void AvatarMixerSlavePool::resize(int numThreads) { if (numThreads > _numThreads) { // start new slaves for (int i = 0; i < numThreads - _numThreads; ++i) { - auto slave = new AvatarMixerSlaveThread(*this); + auto slave = new AvatarMixerSlaveThread(*this, _slaveSharedData); slave->start(); _slaves.emplace_back(slave); } diff --git a/assignment-client/src/avatars/AvatarMixerSlavePool.h b/assignment-client/src/avatars/AvatarMixerSlavePool.h index 15bd681b2c..71a9ace0d3 100644 --- a/assignment-client/src/avatars/AvatarMixerSlavePool.h +++ b/assignment-client/src/avatars/AvatarMixerSlavePool.h @@ -32,7 +32,8 @@ class AvatarMixerSlaveThread : public QThread, public AvatarMixerSlave { using Lock = std::unique_lock; public: - AvatarMixerSlaveThread(AvatarMixerSlavePool& pool) : _pool(pool) {} + AvatarMixerSlaveThread(AvatarMixerSlavePool& pool, SlaveSharedData* slaveSharedData) : + AvatarMixerSlave(slaveSharedData), _pool(pool) {}; void run() override final; @@ -59,7 +60,8 @@ class AvatarMixerSlavePool { public: using ConstIter = NodeList::const_iterator; - AvatarMixerSlavePool(int numThreads = QThread::idealThreadCount()) { setNumThreads(numThreads); } + AvatarMixerSlavePool(SlaveSharedData* slaveSharedData, int numThreads = QThread::idealThreadCount()) : + _slaveSharedData(slaveSharedData) { setNumThreads(numThreads); } ~AvatarMixerSlavePool() { resize(0); } // Jobs the slave pool can do... @@ -98,6 +100,8 @@ private: Queue _queue; ConstIter _begin; ConstIter _end; + + SlaveSharedData* _slaveSharedData; }; #endif // hifi_AvatarMixerSlavePool_h diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 396c6cbcac..e556fd734f 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -6393,8 +6393,8 @@ void Application::nodeActivated(SharedNodePointer node) { if (_avatarOverrideUrl.isValid()) { getMyAvatar()->useFullAvatarURL(_avatarOverrideUrl); } - static const QUrl empty{}; - if (getMyAvatar()->getFullAvatarURLFromPreferences() != getMyAvatar()->cannonicalSkeletonModelURL(empty)) { + + if (getMyAvatar()->getFullAvatarURLFromPreferences() != getMyAvatar()->getSkeletonModelURL()) { getMyAvatar()->resetFullAvatarURL(); } getMyAvatar()->markIdentityDataChanged(); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index ddfeb4df24..c9d8f7bb1e 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1750,12 +1750,6 @@ glm::quat AvatarData::getOrientationOutbound() const { return (getLocalOrientation()); } -static const QUrl emptyURL(""); -QUrl AvatarData::cannonicalSkeletonModelURL(const QUrl& emptyURL) const { - // We don't put file urls on the wire, but instead convert to empty. - return _skeletonModelURL.scheme() == "file" ? emptyURL : _skeletonModelURL; -} - void AvatarData::processAvatarIdentity(const QByteArray& identityData, bool& identityChanged, bool& displayNameChanged) { @@ -1836,6 +1830,27 @@ void AvatarData::processAvatarIdentity(const QByteArray& identityData, bool& ide } } +void AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, int64_t traitVersion) { + destination.writePrimitive(traitType); + + if (traitVersion > 0) { + AvatarTraits::TraitVersion typedVersion = traitVersion; + destination.writePrimitive(typedVersion); + } + + if (traitType == AvatarTraits::SkeletonModelURL) { + QByteArray encodedSkeletonURL; + if (_skeletonModelURL.scheme() != "file" && _skeletonModelURL.scheme() != "qrc") { + encodedSkeletonURL = _skeletonModelURL.toEncoded(); + } + + AvatarTraits::TraitWireSize encodedURLSize = encodedSkeletonURL.size(); + destination.writePrimitive(encodedURLSize); + + destination.write(encodedSkeletonURL); + } +} + void AvatarData::processTrait(AvatarTraits::TraitType traitType, QByteArray traitBinaryData) { if (traitType == AvatarTraits::SkeletonModelURL) { // get the URL from the binary data diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 12e8370b86..619f8e1722 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -426,7 +426,6 @@ public: virtual ~AvatarData(); static const QUrl& defaultFullAvatarModelUrl(); - QUrl cannonicalSkeletonModelURL(const QUrl& empty) const; virtual bool isMyAvatar() const { return false; } @@ -958,6 +957,7 @@ public: // identityChanged returns true if identity has changed, false otherwise. Similarly for displayNameChanged and skeletonModelUrlChange. void processAvatarIdentity(const QByteArray& identityData, bool& identityChanged, bool& displayNameChanged); + void packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, int64_t traitVersion = -1); void processTrait(AvatarTraits::TraitType traitType, QByteArray traitBinaryData); QByteArray identityByteArray(bool setIsReplicated = false) const; diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h index 90258982c3..121e1057c6 100644 --- a/libraries/avatars/src/AvatarTraits.h +++ b/libraries/avatars/src/AvatarTraits.h @@ -29,6 +29,9 @@ namespace AvatarTraits { using TraitVersion = uint32_t; const TraitVersion DEFAULT_TRAIT_VERSION = 0; + using NullableTraitVersion = int64_t; + const NullableTraitVersion NULL_TRAIT_VERSION = -1; + using TraitWireSize = uint16_t; using SimpleTraitVersions = std::vector; diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp index 5d1f2e4926..2518dedf37 100644 --- a/libraries/avatars/src/ClientTraitsHandler.cpp +++ b/libraries/avatars/src/ClientTraitsHandler.cpp @@ -27,6 +27,8 @@ ClientTraitsHandler::ClientTraitsHandler(AvatarData* owningAvatar) : resetForNewMixer(); } }); + + nodeList->getPacketReceiver().registerListener(PacketType::SetAvatarTraits, this, "processTraitOverride"); } void ClientTraitsHandler::resetForNewMixer() { @@ -63,19 +65,11 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() { if (_performInitialSend || changedTraitsCopy.count(AvatarTraits::SkeletonModelURL)) { traitsPacketList->startSegment(); - - traitsPacketList->writePrimitive(AvatarTraits::SkeletonModelURL); - - auto encodedSkeletonURL = _owningAvatar->getSkeletonModelURL().toEncoded(); - - AvatarTraits::TraitWireSize encodedURLSize = encodedSkeletonURL.size(); - traitsPacketList->writePrimitive(encodedURLSize); - - qDebug() << "Sending trait of size" << encodedURLSize; - - traitsPacketList->write(encodedSkeletonURL); - + _owningAvatar->packTrait(AvatarTraits::SkeletonModelURL, *traitsPacketList); traitsPacketList->endSegment(); + + // keep track of our skeleton version in case we get an override back + _currentSkeletonVersion = _currentTraitVersion; } nodeList->sendPacketList(std::move(traitsPacketList), *avatarMixer); @@ -84,3 +78,33 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() { _performInitialSend = false; } } + +void ClientTraitsHandler::processTraitOverride(QSharedPointer message, SharedNodePointer sendingNode) { + if (sendingNode->getType() == NodeType::AvatarMixer) { + while (message->getBytesLeftToRead()) { + AvatarTraits::TraitType traitType; + message->readPrimitive(&traitType); + + AvatarTraits::TraitVersion traitVersion; + message->readPrimitive(&traitVersion); + + AvatarTraits::TraitWireSize traitBinarySize; + message->readPrimitive(&traitBinarySize); + + // only accept an override if this is for a trait type we override + // and the version matches what we last sent for skeleton + if (traitType == AvatarTraits::SkeletonModelURL + && traitVersion == _currentSkeletonVersion + && !hasTraitChanged(AvatarTraits::SkeletonModelURL)) { + // override the skeleton URL but do not mark the trait as having changed + // so that we don't unecessarily sent a new trait packet to the mixer with the overriden URL + auto encodedSkeletonURL = QUrl::fromEncoded(message->readWithoutCopy(traitBinarySize)); + _owningAvatar->setSkeletonModelURL(encodedSkeletonURL); + + _changedTraits.erase(AvatarTraits::SkeletonModelURL); + } else { + message->seek(message->getPosition() + traitBinarySize); + } + } + } +} diff --git a/libraries/avatars/src/ClientTraitsHandler.h b/libraries/avatars/src/ClientTraitsHandler.h index 4aea0bb433..3a2b70776c 100644 --- a/libraries/avatars/src/ClientTraitsHandler.h +++ b/libraries/avatars/src/ClientTraitsHandler.h @@ -12,13 +12,15 @@ #ifndef hifi_ClientTraitsHandler_h #define hifi_ClientTraitsHandler_h -#include +#include #include "AvatarTraits.h" +#include "Node.h" class AvatarData; -class ClientTraitsHandler { +class ClientTraitsHandler : public QObject { + Q_OBJECT public: ClientTraitsHandler(AvatarData* owningAvatar); @@ -31,11 +33,17 @@ public: void resetForNewMixer(); +public slots: + void processTraitOverride(QSharedPointer message, SharedNodePointer sendingNode); + private: AvatarData* _owningAvatar; AvatarTraits::TraitTypeSet _changedTraits; AvatarTraits::TraitVersion _currentTraitVersion { AvatarTraits::DEFAULT_TRAIT_VERSION }; + + AvatarTraits::NullableTraitVersion _currentSkeletonVersion { AvatarTraits::NULL_TRAIT_VERSION }; + bool _performInitialSend { false }; }; diff --git a/libraries/networking/src/ExtendedIODevice.h b/libraries/networking/src/ExtendedIODevice.h new file mode 100644 index 0000000000..7df1af74b6 --- /dev/null +++ b/libraries/networking/src/ExtendedIODevice.h @@ -0,0 +1,39 @@ +// +// ExtendedIODevice.h +// libraries/networking/src +// +// Created by Stephen Birarda on 8/7/18. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#ifndef hifi_ExtendedIODevice_h +#define hifi_ExtendedIODevice_h + +#include + +class ExtendedIODevice : public QIODevice { +public: + ExtendedIODevice(QObject* parent = nullptr) : QIODevice(parent) {}; + + template qint64 peekPrimitive(T* data); + template qint64 readPrimitive(T* data); + template qint64 writePrimitive(const T& data); +}; + +template qint64 ExtendedIODevice::peekPrimitive(T* data) { + return peek(reinterpret_cast(data), sizeof(T)); +} + +template qint64 ExtendedIODevice::readPrimitive(T* data) { + return read(reinterpret_cast(data), sizeof(T)); +} + +template qint64 ExtendedIODevice::writePrimitive(const T& data) { + static_assert(!std::is_pointer::value, "T must not be a pointer"); + return write(reinterpret_cast(&data), sizeof(T)); +} + +#endif // hifi_ExtendedIODevice_h diff --git a/libraries/networking/src/udt/BasePacket.cpp b/libraries/networking/src/udt/BasePacket.cpp index 0540e60a0e..92ccdd6117 100644 --- a/libraries/networking/src/udt/BasePacket.cpp +++ b/libraries/networking/src/udt/BasePacket.cpp @@ -78,7 +78,7 @@ BasePacket::BasePacket(std::unique_ptr data, qint64 size, const HifiSock } BasePacket::BasePacket(const BasePacket& other) : - QIODevice() + ExtendedIODevice() { *this = other; } diff --git a/libraries/networking/src/udt/BasePacket.h b/libraries/networking/src/udt/BasePacket.h index d9b624b595..9c3244e08b 100644 --- a/libraries/networking/src/udt/BasePacket.h +++ b/libraries/networking/src/udt/BasePacket.h @@ -16,16 +16,15 @@ #include -#include - #include #include "../HifiSockAddr.h" #include "Constants.h" +#include "../ExtendedIODevice.h" namespace udt { -class BasePacket : public QIODevice { +class BasePacket : public ExtendedIODevice { Q_OBJECT public: static const qint64 PACKET_WRITE_ERROR; @@ -85,10 +84,6 @@ public: void setReceiveTime(p_high_resolution_clock::time_point receiveTime) { _receiveTime = receiveTime; } p_high_resolution_clock::time_point getReceiveTime() const { return _receiveTime; } - - template qint64 peekPrimitive(T* data); - template qint64 readPrimitive(T* data); - template qint64 writePrimitive(const T& data); protected: BasePacket(qint64 size); @@ -116,19 +111,6 @@ protected: p_high_resolution_clock::time_point _receiveTime; // captures the time the packet received (only used on receiving end) }; - -template qint64 BasePacket::peekPrimitive(T* data) { - return peek(reinterpret_cast(data), sizeof(T)); -} - -template qint64 BasePacket::readPrimitive(T* data) { - return read(reinterpret_cast(data), sizeof(T)); -} - -template qint64 BasePacket::writePrimitive(const T& data) { - static_assert(!std::is_pointer::value, "T must not be a pointer"); - return write(reinterpret_cast(&data), sizeof(T)); -} } // namespace udt diff --git a/libraries/networking/src/udt/PacketList.h b/libraries/networking/src/udt/PacketList.h index ff1860c3d1..b9bd6a8c15 100644 --- a/libraries/networking/src/udt/PacketList.h +++ b/libraries/networking/src/udt/PacketList.h @@ -14,8 +14,7 @@ #include -#include - +#include "../ExtendedIODevice.h" #include "Packet.h" #include "PacketHeaders.h" @@ -25,7 +24,7 @@ namespace udt { class Packet; -class PacketList : public QIODevice { +class PacketList : public ExtendedIODevice { Q_OBJECT public: using MessageNumber = uint32_t; @@ -59,9 +58,6 @@ public: virtual bool isSequential() const override { return false; } virtual qint64 size() const override { return getDataSize(); } - template qint64 readPrimitive(T* data); - template qint64 writePrimitive(const T& data); - qint64 writeString(const QString& string); protected: @@ -105,16 +101,6 @@ private: QByteArray _extendedHeader; }; -template qint64 PacketList::readPrimitive(T* data) { - static_assert(!std::is_pointer::value, "T must not be a pointer"); - return read(reinterpret_cast(data), sizeof(T)); -} - -template qint64 PacketList::writePrimitive(const T& data) { - static_assert(!std::is_pointer::value, "T must not be a pointer"); - return write(reinterpret_cast(&data), sizeof(T)); -} - template std::unique_ptr PacketList::takeFront() { static_assert(std::is_base_of::value, "T must derive from Packet."); From a0df68f32fda747afbc9d68c52524a0d4d810f9c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 8 Aug 2018 13:53:35 -0700 Subject: [PATCH 07/31] move skeleton model URL emit to AvatarData --- assignment-client/src/avatars/ScriptableAvatar.cpp | 2 +- assignment-client/src/avatars/ScriptableAvatar.h | 2 +- interface/src/avatar/MyAvatar.cpp | 4 +--- libraries/avatars/src/AvatarData.cpp | 4 +++- 4 files changed, 6 insertions(+), 6 deletions(-) diff --git a/assignment-client/src/avatars/ScriptableAvatar.cpp b/assignment-client/src/avatars/ScriptableAvatar.cpp index 4e47cf96f1..16a49a8999 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.cpp +++ b/assignment-client/src/avatars/ScriptableAvatar.cpp @@ -1,6 +1,6 @@ // // ScriptableAvatar.cpp -// +// assignment-client/src/avatars // // Created by Clement on 7/22/14. // Copyright 2014 High Fidelity, Inc. diff --git a/assignment-client/src/avatars/ScriptableAvatar.h b/assignment-client/src/avatars/ScriptableAvatar.h index 8e3d779dda..201bfe67e8 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.h +++ b/assignment-client/src/avatars/ScriptableAvatar.h @@ -1,6 +1,6 @@ // // ScriptableAvatar.h -// +// assignment-client/src/avatars // // Created by Clement on 7/22/14. // Copyright 2014 High Fidelity, Inc. diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index df9afeb92d..90d1ad257b 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1699,9 +1699,9 @@ void MyAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { } QObject::disconnect(*skeletonConnection); }); + saveAvatarUrl(); emit skeletonChanged(); - emit skeletonModelURLChanged(); if (previousSkeletonModelURL != _skeletonModelURL) { _clientTraitsHandler.markTraitChanged(AvatarTraits::SkeletonModelURL); @@ -1776,8 +1776,6 @@ void MyAvatar::useFullAvatarURL(const QUrl& fullAvatarURL, const QString& modelN setSkeletonModelURL(fullAvatarURL); UserActivityLogger::getInstance().changedModel("skeleton", urlString); } - - markIdentityDataChanged(); } glm::vec3 MyAvatar::getSkeletonPosition() const { diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index c9d8f7bb1e..c9d8049cd3 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1891,11 +1891,13 @@ void AvatarData::setSkeletonModelURL(const QUrl& skeletonModelURL) { if (expanded == _skeletonModelURL) { return; } + _skeletonModelURL = expanded; qCDebug(avatars) << "Changing skeleton model for avatar" << getSessionUUID() << "to" << _skeletonModelURL.toString(); updateJointMappings(); - markIdentityDataChanged(); + + emit skeletonModelURLChanged(); } void AvatarData::setDisplayName(const QString& displayName) { From ea7c0e923a9958d2fa21ba440d15bb48cd20075b Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 8 Aug 2018 13:59:21 -0700 Subject: [PATCH 08/31] make client traits handler a unique ptr in AvatarData --- .../src/avatars/ScriptableAvatar.cpp | 10 ++++--- .../src/avatars/ScriptableAvatar.h | 5 ++-- interface/src/avatar/MyAvatar.cpp | 10 +++---- interface/src/avatar/MyAvatar.h | 4 --- libraries/avatars/src/AvatarData.cpp | 5 ++++ libraries/avatars/src/AvatarData.h | 6 +++++ libraries/avatars/src/AvatarTraits.h | 27 ++++++++++++++++--- libraries/avatars/src/ClientTraitsHandler.cpp | 2 +- libraries/avatars/src/ClientTraitsHandler.h | 4 +-- 9 files changed, 49 insertions(+), 24 deletions(-) diff --git a/assignment-client/src/avatars/ScriptableAvatar.cpp b/assignment-client/src/avatars/ScriptableAvatar.cpp index 16a49a8999..6f04cfa196 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.cpp +++ b/assignment-client/src/avatars/ScriptableAvatar.cpp @@ -16,9 +16,13 @@ #include #include -#include #include +#include +#include +ScriptableAvatar::ScriptableAvatar() { + _clientTraitsHandler = std::unique_ptr(new ClientTraitsHandler(this)); +} QByteArray ScriptableAvatar::toByteArrayStateful(AvatarDataDetail dataDetail, bool dropFaceTracking) { _globalPosition = getWorldPosition(); @@ -63,8 +67,6 @@ void ScriptableAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { _animSkeleton.reset(); AvatarData::setSkeletonModelURL(skeletonModelURL); - - _clientTraitsHandler.markTraitChanged(AvatarTraits::SkeletonModelURL); } static AnimPose composeAnimPose(const FBXJoint& fbxJoint, const glm::quat rotation, const glm::vec3 translation) { @@ -141,5 +143,5 @@ void ScriptableAvatar::update(float deltatime) { } } - _clientTraitsHandler.sendChangedTraitsToMixer(); + _clientTraitsHandler->sendChangedTraitsToMixer(); } diff --git a/assignment-client/src/avatars/ScriptableAvatar.h b/assignment-client/src/avatars/ScriptableAvatar.h index 201bfe67e8..89f9369133 100644 --- a/assignment-client/src/avatars/ScriptableAvatar.h +++ b/assignment-client/src/avatars/ScriptableAvatar.h @@ -15,7 +15,6 @@ #include #include #include -#include #include /**jsdoc @@ -125,6 +124,8 @@ class ScriptableAvatar : public AvatarData, public Dependency { Q_OBJECT public: + ScriptableAvatar(); + /**jsdoc * @function Avatar.startAnimation * @param {string} url @@ -165,8 +166,6 @@ private: QStringList _maskedJoints; AnimationPointer _bind; // a sleazy way to get the skeleton, given the various library/cmake dependencies std::shared_ptr _animSkeleton; - - ClientTraitsHandler _clientTraitsHandler { this }; }; #endif // hifi_ScriptableAvatar_h diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 90d1ad257b..2c88f917a1 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -119,9 +119,9 @@ MyAvatar::MyAvatar(QThread* thread) : _goToOrientation(), _prevShouldDrawHead(true), _audioListenerMode(FROM_HEAD), - _hmdAtRestDetector(glm::vec3(0), glm::quat()), - _clientTraitsHandler(this) + _hmdAtRestDetector(glm::vec3(0), glm::quat()) { + _clientTraitsHandler = std::unique_ptr(new ClientTraitsHandler(this)); // give the pointer to our head to inherited _headData variable from AvatarData _headData = new MyHead(this); @@ -514,7 +514,7 @@ void MyAvatar::update(float deltaTime) { sendIdentityPacket(); } - _clientTraitsHandler.sendChangedTraitsToMixer(); + _clientTraitsHandler->sendChangedTraitsToMixer(); simulate(deltaTime); @@ -1702,10 +1702,6 @@ void MyAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { saveAvatarUrl(); emit skeletonChanged(); - - if (previousSkeletonModelURL != _skeletonModelURL) { - _clientTraitsHandler.markTraitChanged(AvatarTraits::SkeletonModelURL); - } } void MyAvatar::removeAvatarEntities(const std::function& condition) { diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 8fdc4ba4e3..ba6348cc22 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -1334,7 +1334,6 @@ public slots: */ void setAnimGraphUrl(const QUrl& url); // thread-safe - /**jsdoc * @function MyAvatar.getPositionForAudio * @returns {Vec3} @@ -1347,7 +1346,6 @@ public slots: */ glm::quat getOrientationForAudio(); - /**jsdoc * @function MyAvatar.setModelScale * @param {number} scale @@ -1776,8 +1774,6 @@ private: bool _haveReceivedHeightLimitsFromDomain { false }; int _disableHandTouchCount { 0 }; - - ClientTraitsHandler _clientTraitsHandler; }; QScriptValue audioListenModeToScriptValue(QScriptEngine* engine, const AudioListenerMode& audioListenerMode); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index c9d8049cd3..f32da39bba 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -43,6 +43,7 @@ #include "AvatarLogging.h" #include "AvatarTraits.h" +#include "ClientTraitsHandler.h" //#define WANT_DEBUG @@ -1897,6 +1898,10 @@ void AvatarData::setSkeletonModelURL(const QUrl& skeletonModelURL) { updateJointMappings(); + if (_clientTraitsHandler) { + _clientTraitsHandler->markTraitChanged(AvatarTraits::SkeletonModelURL); + } + emit skeletonModelURLChanged(); } diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 619f8e1722..a5d2d0749b 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -372,6 +372,8 @@ public: bool operator<(const AvatarPriority& other) const { return priority < other.priority; } }; +class ClientTraitsHandler; + class AvatarData : public QObject, public SpatiallyNestable { Q_OBJECT @@ -924,6 +926,7 @@ public: * @param {string} entityData */ Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData); + /**jsdoc * @function MyAvatar.clearAvatarEntity * @param {Uuid} entityID @@ -1435,6 +1438,9 @@ protected: bool _hasProcessedFirstIdentity { false }; float _density; + // null unless MyAvatar or ScriptableAvatar sending traits data to mixer + std::unique_ptr _clientTraitsHandler; + template T readLockWithNamedJointIndex(const QString& name, const T& defaultValue, F f) const { int index = getFauxJointIndex(name); diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h index 121e1057c6..d30da0e1af 100644 --- a/libraries/avatars/src/AvatarTraits.h +++ b/libraries/avatars/src/AvatarTraits.h @@ -12,8 +12,8 @@ #ifndef hifi_AvatarTraits_h #define hifi_AvatarTraits_h +#include #include -#include #include namespace AvatarTraits { @@ -23,7 +23,28 @@ namespace AvatarTraits { TotalTraitTypes }; - using TraitTypeSet = std::set; + class TraitTypeSet { + public: + TraitTypeSet() {}; + + TraitTypeSet(std::initializer_list types) { + for (auto type : types) { + _types[type] = true; + } + }; + + bool contains(TraitType type) const { return _types[type]; } + + bool hasAny() const { return std::find(_types.begin(), _types.end(), true) != _types.end(); } + int size() const { return std::count(_types.begin(), _types.end(), true); } + + void insert(TraitType type) { _types[type] = true; } + void erase(TraitType type) { _types[type] = false; } + void clear() { std::fill(_types.begin(), _types.end(), false); } + private: + std::vector _types = { AvatarTraits::TotalTraitTypes, false }; + }; + const TraitTypeSet SimpleTraitTypes = { SkeletonModelURL }; using TraitVersion = uint32_t; @@ -35,6 +56,6 @@ namespace AvatarTraits { using TraitWireSize = uint16_t; using SimpleTraitVersions = std::vector; -} +}; #endif // hifi_AvatarTraits_h diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp index 2518dedf37..8b3ded1e1c 100644 --- a/libraries/avatars/src/ClientTraitsHandler.cpp +++ b/libraries/avatars/src/ClientTraitsHandler.cpp @@ -63,7 +63,7 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() { auto changedTraitsCopy { _changedTraits }; _changedTraits.clear(); - if (_performInitialSend || changedTraitsCopy.count(AvatarTraits::SkeletonModelURL)) { + if (_performInitialSend || changedTraitsCopy.contains(AvatarTraits::SkeletonModelURL)) { traitsPacketList->startSegment(); _owningAvatar->packTrait(AvatarTraits::SkeletonModelURL, *traitsPacketList); traitsPacketList->endSegment(); diff --git a/libraries/avatars/src/ClientTraitsHandler.h b/libraries/avatars/src/ClientTraitsHandler.h index 3a2b70776c..1d4c67d0c4 100644 --- a/libraries/avatars/src/ClientTraitsHandler.h +++ b/libraries/avatars/src/ClientTraitsHandler.h @@ -26,10 +26,10 @@ public: void sendChangedTraitsToMixer(); - bool hasChangedTraits() { return _changedTraits.size(); } + bool hasChangedTraits() { return _changedTraits.hasAny(); } void markTraitChanged(AvatarTraits::TraitType changedTrait) { _changedTraits.insert(changedTrait); } - bool hasTraitChanged(AvatarTraits::TraitType checkTrait) { return _changedTraits.count(checkTrait) > 0; } + bool hasTraitChanged(AvatarTraits::TraitType checkTrait) { return _changedTraits.contains(checkTrait) > 0; } void resetForNewMixer(); From e6b419d283f49b13e75e5348121b0a638179dcec Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 9 Aug 2018 19:01:09 -0700 Subject: [PATCH 09/31] add instanced traits and migrate avatar entities --- .../src/avatars/AvatarMixerClientData.cpp | 77 +++++---- .../src/avatars/AvatarMixerClientData.h | 15 +- .../src/avatars/AvatarMixerSlave.cpp | 74 +++++++- libraries/avatars/src/AssociatedTraitValues.h | 158 ++++++++++++++++++ libraries/avatars/src/AvatarData.cpp | 124 ++++++++++---- libraries/avatars/src/AvatarData.h | 16 +- libraries/avatars/src/AvatarHashMap.cpp | 75 +++++---- libraries/avatars/src/AvatarHashMap.h | 9 +- libraries/avatars/src/AvatarTraits.h | 51 +++--- libraries/avatars/src/ClientTraitsHandler.cpp | 60 +++++-- libraries/avatars/src/ClientTraitsHandler.h | 25 ++- .../networking/src/udt/PacketHeaders.cpp | 2 +- libraries/networking/src/udt/PacketHeaders.h | 5 +- 13 files changed, 502 insertions(+), 189 deletions(-) create mode 100644 libraries/avatars/src/AssociatedTraitValues.h diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 552fe9a58b..34b7ec97ff 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -19,8 +19,7 @@ #include "AvatarMixerSlave.h" AvatarMixerClientData::AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID) : - NodeData(nodeID), - _receivedSimpleTraitVersions(AvatarTraits::SimpleTraitTypes.size()) + NodeData(nodeID) { // in case somebody calls getSessionUUID on the AvatarData instance, make sure it has the right ID _avatar->setID(nodeID); @@ -107,21 +106,47 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, Sl AvatarTraits::TraitType traitType; message.readPrimitive(&traitType); - AvatarTraits::TraitWireSize traitSize; - message.readPrimitive(&traitSize); + if (AvatarTraits::isSimpleTrait(traitType)) { + AvatarTraits::TraitWireSize traitSize; + message.readPrimitive(&traitSize); - if (packetTraitVersion > _receivedSimpleTraitVersions[traitType]) { - _avatar->processTrait(traitType, message.readWithoutCopy(traitSize)); - _receivedSimpleTraitVersions[traitType] = packetTraitVersion; + if (packetTraitVersion > _lastReceivedTraitVersions[traitType]) { + _avatar->processTrait(traitType, message.read(traitSize)); + _lastReceivedTraitVersions[traitType] = packetTraitVersion; - if (traitType == AvatarTraits::SkeletonModelURL) { - // special handling for skeleton model URL, since we need to make sure it is in the whitelist - checkSkeletonURLAgainstWhitelist(slaveSharedData, sendingNode, packetTraitVersion); + if (traitType == AvatarTraits::SkeletonModelURL) { + // special handling for skeleton model URL, since we need to make sure it is in the whitelist + checkSkeletonURLAgainstWhitelist(slaveSharedData, sendingNode, packetTraitVersion); + } + + anyTraitsChanged = true; + } else { + message.seek(message.getPosition() + traitSize); } - - anyTraitsChanged = true; } else { - message.seek(message.getPosition() + traitSize); + AvatarTraits::TraitInstanceID instanceID = QUuid::fromRfc4122(message.readWithoutCopy(NUM_BYTES_RFC4122_UUID)); + + AvatarTraits::TraitWireSize traitSize; + message.readPrimitive(&traitSize); + + auto& instanceVersionRef = _lastReceivedTraitVersions.getInstanceValueRef(traitType, instanceID); + + if (packetTraitVersion > instanceVersionRef) { + if (traitSize == AvatarTraits::DELETED_TRAIT_SIZE) { + _avatar->processDeletedTraitInstance(traitType, instanceID); + + // to track a deleted instance but keep version information + // the avatar mixer uses the negative value of the sent version + instanceVersionRef = -packetTraitVersion; + } else { + _avatar->processTraitInstance(traitType, instanceID, message.read(traitSize)); + instanceVersionRef = packetTraitVersion; + } + + anyTraitsChanged = true; + } else { + message.seek(message.getPosition() + traitSize); + } } } @@ -257,29 +282,3 @@ AvatarMixerClientData::TraitsCheckTimestamp AvatarMixerClientData::getLastOtherA return TraitsCheckTimestamp(); } } - -AvatarTraits::TraitVersion AvatarMixerClientData::getLastSentSimpleTraitVersion(Node::LocalID otherAvatar, - AvatarTraits::TraitType traitType) const { - auto it = _sentSimpleTraitVersions.find(otherAvatar); - - if (it != _sentSimpleTraitVersions.end()) { - return it->second[traitType]; - } - - return AvatarTraits::DEFAULT_TRAIT_VERSION; -} - -void AvatarMixerClientData::setLastSentSimpleTraitVersion(Node::LocalID otherAvatar, AvatarTraits::TraitType traitType, AvatarTraits::TraitVersion traitVersion) { - - auto it = _sentSimpleTraitVersions.find(otherAvatar); - - if (it == _sentSimpleTraitVersions.end()) { - auto pair = _sentSimpleTraitVersions.insert({ - otherAvatar, { AvatarTraits::TotalTraitTypes, AvatarTraits::DEFAULT_TRAIT_VERSION } - }); - - it = pair.first; - } - - it->second[traitType] = traitVersion; -} diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 96b420afc1..dcbf8a6dba 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -22,7 +22,7 @@ #include #include -#include +#include #include #include #include @@ -128,16 +128,15 @@ public: using TraitsCheckTimestamp = std::chrono::steady_clock::time_point; TraitsCheckTimestamp getLastReceivedTraitsChange() const { return _lastReceivedTraitsChange; } - AvatarTraits::TraitVersion getLastReceivedSimpleTraitVersion(AvatarTraits::TraitType traitType) const - { return _receivedSimpleTraitVersions[traitType]; } + + AvatarTraits::TraitVersions& getLastReceivedTraitVersions() { return _lastReceivedTraitVersions; } + const AvatarTraits::TraitVersions& getLastReceivedTraitVersions() const { return _lastReceivedTraitVersions; } TraitsCheckTimestamp getLastOtherAvatarTraitsSendPoint(Node::LocalID otherAvatar) const; void setLastOtherAvatarTraitsSendPoint(Node::LocalID otherAvatar, TraitsCheckTimestamp sendPoint) { _lastSentTraitsTimestamps[otherAvatar] = sendPoint; } - AvatarTraits::TraitVersion getLastSentSimpleTraitVersion(Node::LocalID otherAvatar, AvatarTraits::TraitType traitType) const; - void setLastSentSimpleTraitVersion(Node::LocalID otherAvatar, AvatarTraits::TraitType traitType, - AvatarTraits::TraitVersion traitVersion); + AvatarTraits::TraitVersions& getLastSentTraitVersions(Node::LocalID otherAvatar) { return _sentTraitVersions[otherAvatar]; } private: struct PacketQueue : public std::queue> { @@ -176,11 +175,11 @@ private: QString _baseDisplayName{}; // The santized key used in determinging unique sessionDisplayName, so that we can remove from dictionary. bool _requestsDomainListData { false }; - AvatarTraits::SimpleTraitVersions _receivedSimpleTraitVersions; + AvatarTraits::TraitVersions _lastReceivedTraitVersions; TraitsCheckTimestamp _lastReceivedTraitsChange; std::unordered_map _lastSentTraitsTimestamps; - std::unordered_map _sentSimpleTraitVersions; + std::unordered_map _sentTraitVersions; }; #endif // hifi_AvatarMixerClientData_h diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 88e394bc95..ebbaeb7a35 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -99,20 +99,76 @@ void AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* liste auto sendingAvatar = sendingNodeData->getAvatarSharedPointer(); // compare trait versions so we can see what exactly needs to go out - for (int i = 0; i < AvatarTraits::TotalTraitTypes; ++i) { - AvatarTraits::TraitType traitType = static_cast(i); + auto& lastSentVersions = listeningNodeData->getLastSentTraitVersions(otherNodeLocalID); + const auto& lastReceivedVersions = sendingNodeData->getLastReceivedTraitVersions(); - auto lastSentVersion = listeningNodeData->getLastSentSimpleTraitVersion(otherNodeLocalID, traitType); - auto lastReceivedVersion = sendingNodeData->getLastReceivedSimpleTraitVersion(traitType); + auto simpleReceivedIt = lastReceivedVersions.simpleCBegin(); + while (simpleReceivedIt != lastReceivedVersions.simpleCEnd()) { + auto traitType = static_cast(std::distance(lastReceivedVersions.simpleCBegin(), + simpleReceivedIt)); - if (lastReceivedVersion > lastSentVersion) { - // there is an update to this trait, add it to the traits packet + // we need to double check that this is actually a simple trait type, since the instanced + // trait types are in the simple vector for access efficiency + if (AvatarTraits::isSimpleTrait(traitType)) { + auto lastReceivedVersion = *simpleReceivedIt; + auto& lastSentVersionRef = lastSentVersions[traitType]; - // update the last sent version - listeningNodeData->setLastSentSimpleTraitVersion(otherNodeLocalID, traitType, lastReceivedVersion); + if (lastReceivedVersions[traitType] > lastSentVersionRef) { + // there is an update to this trait, add it to the traits packet + sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion); - sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion); + // update the last sent version + lastSentVersionRef = lastReceivedVersion; + } } + + ++simpleReceivedIt; + } + + // enumerate the received instanced trait versions + auto instancedReceivedIt = lastReceivedVersions.instancedCBegin(); + while (instancedReceivedIt != lastReceivedVersions.instancedCEnd()) { + auto traitType = instancedReceivedIt->traitType; + + // get or create the sent trait versions for this trait type + auto& sentIDValuePairs = lastSentVersions.getInstanceIDValuePairs(traitType); + + // enumerate each received instance + for (auto& receivedInstance : instancedReceivedIt->instances) { + auto instanceID = receivedInstance.id; + const auto receivedVersion = receivedInstance.value; + + // to track deletes and maintain version information for traits + // the mixer stores the negative value of the received version when a trait instance is deleted + bool isDeleted = receivedVersion < 0; + const auto absoluteReceivedVersion = std::abs(receivedVersion); + + // look for existing sent version for this instance + auto sentInstanceIt = std::find_if(sentIDValuePairs.begin(), sentIDValuePairs.end(), + [instanceID](auto& sentInstance) + { + return sentInstance.id == instanceID; + }); + + if (!isDeleted && (sentInstanceIt == sentIDValuePairs.end() || receivedVersion > sentInstanceIt->value)) { + // this instance version exists and has never been sent or is newer so we need to send it + sendingAvatar->packTraitInstance(traitType, instanceID, traitsPacketList, receivedVersion); + + if (sentInstanceIt != sentIDValuePairs.end()) { + sentInstanceIt->value = receivedVersion; + } else { + sentIDValuePairs.emplace_back(instanceID, receivedVersion); + } + } else if (isDeleted && sentInstanceIt != sentIDValuePairs.end() && absoluteReceivedVersion > sentInstanceIt->value) { + // this instance version was deleted and we haven't sent the delete to this client yet + AvatarTraits::packInstancedTraitDelete(traitType, instanceID, traitsPacketList, absoluteReceivedVersion); + + // update the last sent version for this trait instance to the absolute value of the deleted version + sentInstanceIt->value = absoluteReceivedVersion; + } + } + + ++instancedReceivedIt; } // write a null trait type to mark the end of trait data for this avatar diff --git a/libraries/avatars/src/AssociatedTraitValues.h b/libraries/avatars/src/AssociatedTraitValues.h new file mode 100644 index 0000000000..b2c0197e5c --- /dev/null +++ b/libraries/avatars/src/AssociatedTraitValues.h @@ -0,0 +1,158 @@ +// +// AssociatedTraitValues.h +// libraries/avatars/src +// +// Created by Stephen Birarda on 8/8/18. +// Copyright 2018 High Fidelity, Inc. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html +// + +#ifndef hifi_AssociatedTraitValues_h +#define hifi_AssociatedTraitValues_h + +#include "AvatarTraits.h" + +namespace AvatarTraits { + template + class AssociatedTraitValues { + public: + AssociatedTraitValues() : _simpleTypes(TotalTraitTypes, defaultValue) {} + + void insert(TraitType type, T value) { _simpleTypes[type] = value; } + void erase(TraitType type) { _simpleTypes[type] = defaultValue; } + + T& getInstanceValueRef(TraitType traitType, TraitInstanceID instanceID); + void instanceInsert(TraitType traitType, TraitInstanceID instanceID, T value); + + struct InstanceIDValuePair { + TraitInstanceID id; + T value; + + InstanceIDValuePair(TraitInstanceID id, T value) : id(id), value(value) {}; + }; + + using InstanceIDValuePairs = std::vector; + + InstanceIDValuePairs& getInstanceIDValuePairs(TraitType traitType); + + void instanceErase(TraitType traitType, TraitInstanceID instanceID); + void eraseAllInstances(TraitType traitType); + + // will return defaultValue for instanced traits + T operator[](TraitType traitType) const { return _simpleTypes[traitType]; } + T& operator[](TraitType traitType) { return _simpleTypes[traitType]; } + + void reset() { + std::fill(_simpleTypes.begin(), _simpleTypes.end(), defaultValue); + _instancedTypes.clear(); + } + + typename std::vector::const_iterator simpleCBegin() const { return _simpleTypes.cbegin(); } + typename std::vector::const_iterator simpleCEnd() const { return _simpleTypes.cend(); } + + typename std::vector::iterator simpleBegin() { return _simpleTypes.begin(); } + typename std::vector::iterator simpleEnd() { return _simpleTypes.end(); } + + struct TraitWithInstances { + TraitType traitType; + InstanceIDValuePairs instances; + + TraitWithInstances(TraitType traitType) : traitType(traitType) {}; + TraitWithInstances(TraitType traitType, TraitInstanceID instanceID, T value) : + traitType(traitType), instances({{ instanceID, value }}) {}; + }; + + typename std::vector::const_iterator instancedCBegin() const { return _instancedTypes.cbegin(); } + typename std::vector::const_iterator instancedCEnd() const { return _instancedTypes.cend(); } + + typename std::vector::iterator instancedBegin() { return _instancedTypes.begin(); } + typename std::vector::iterator instancedEnd() { return _instancedTypes.end(); } + + private: + std::vector _simpleTypes; + + typename std::vector::iterator instancesForTrait(TraitType traitType) { + return std::find_if(_instancedTypes.begin(), _instancedTypes.end(), + [traitType](TraitWithInstances& traitWithInstances){ + return traitWithInstances.traitType == traitType; + }); + } + + std::vector _instancedTypes; + }; + + template + inline typename AssociatedTraitValues::InstanceIDValuePairs& + AssociatedTraitValues::getInstanceIDValuePairs(TraitType traitType) { + auto it = instancesForTrait(traitType); + + if (it != _instancedTypes.end()) { + return it->instances; + } else { + _instancedTypes.emplace_back(traitType); + return _instancedTypes.back().instances; + } + } + + template + inline T& AssociatedTraitValues::getInstanceValueRef(TraitType traitType, TraitInstanceID instanceID) { + auto it = instancesForTrait(traitType); + + if (it != _instancedTypes.end()) { + auto& instancesVector = it->instances; + auto instanceIt = std::find_if(instancesVector.begin(), instancesVector.end(), + [instanceID](InstanceIDValuePair& idValuePair){ + return idValuePair.id == instanceID; + }); + if (instanceIt != instancesVector.end()) { + return instanceIt->value; + } else { + instancesVector.emplace_back(instanceID, defaultValue); + return instancesVector.back().value; + } + } else { + _instancedTypes.emplace_back(traitType, instanceID, defaultValue); + return _instancedTypes.back().instances.back().value; + } + } + + template + inline void AssociatedTraitValues::instanceInsert(TraitType traitType, TraitInstanceID instanceID, T value) { + auto it = instancesForTrait(traitType); + + if (it != _instancedTypes.end()) { + auto instancesVector = it->instances; + auto instanceIt = std::find_if(instancesVector.begin(), instancesVector.end(), + [instanceID](InstanceIDValuePair& idValuePair){ + return idValuePair.id == instanceID; + }); + if (instanceIt != instancesVector.end()) { + instanceIt->value = value; + } else { + instancesVector.emplace_back(instanceID, value); + } + } else { + _instancedTypes.emplace_back(traitType, instanceID, value); + } + } + + template + inline void AssociatedTraitValues::instanceErase(TraitType traitType, TraitInstanceID instanceID) { + auto it = instancesForTrait(traitType); + + if (it != _instancedTypes.end()) { + auto instancesVector = it->instances; + instancesVector.erase(std::remove_if(instancesVector.begin(), + instancesVector.end(), + [&instanceID](InstanceIDValuePair& idValuePair){ + return idValuePair.id == instanceID; + })); + } + } + + using TraitVersions = AssociatedTraitValues; +}; + +#endif // hifi_AssociatedTraitValues_h diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index f32da39bba..36dbe00937 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1778,7 +1778,6 @@ void AvatarData::processAvatarIdentity(const QByteArray& identityData, bool& ide >> identity.displayName >> identity.sessionDisplayName >> identity.isReplicated - >> identity.avatarEntityData >> identity.lookAtSnappingEnabled ; @@ -1802,16 +1801,6 @@ void AvatarData::processAvatarIdentity(const QByteArray& identityData, bool& ide identityChanged = true; } - bool avatarEntityDataChanged = false; - _avatarEntitiesLock.withReadLock([&] { - avatarEntityDataChanged = (identity.avatarEntityData != _avatarEntityData); - }); - - if (avatarEntityDataChanged) { - setAvatarEntityData(identity.avatarEntityData); - identityChanged = true; - } - if (identity.lookAtSnappingEnabled != _lookAtSnappingEnabled) { setProperty("lookAtSnappingEnabled", identity.lookAtSnappingEnabled); identityChanged = true; @@ -1831,19 +1820,25 @@ void AvatarData::processAvatarIdentity(const QByteArray& identityData, bool& ide } } -void AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, int64_t traitVersion) { +QUrl AvatarData::getWireSafeSkeletonModelURL() const { + if (_skeletonModelURL.scheme() != "file" && _skeletonModelURL.scheme() != "qrc") { + return _skeletonModelURL; + } else { + return QUrl(); + } +} + +void AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, + AvatarTraits::TraitVersion traitVersion) { destination.writePrimitive(traitType); - if (traitVersion > 0) { + if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) { AvatarTraits::TraitVersion typedVersion = traitVersion; destination.writePrimitive(typedVersion); } if (traitType == AvatarTraits::SkeletonModelURL) { - QByteArray encodedSkeletonURL; - if (_skeletonModelURL.scheme() != "file" && _skeletonModelURL.scheme() != "qrc") { - encodedSkeletonURL = _skeletonModelURL.toEncoded(); - } + QByteArray encodedSkeletonURL = getWireSafeSkeletonModelURL().toEncoded(); AvatarTraits::TraitWireSize encodedURLSize = encodedSkeletonURL.size(); destination.writePrimitive(encodedURLSize); @@ -1852,15 +1847,61 @@ void AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& } } +void AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID traitInstanceID, + ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion) { + destination.writePrimitive(traitType); + + if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) { + AvatarTraits::TraitVersion typedVersion = traitVersion; + destination.writePrimitive(typedVersion); + } + + destination.write(traitInstanceID.toRfc4122()); + + if (traitType == AvatarTraits::AvatarEntity) { + // grab a read lock on the avatar entities and check for entity data for the given ID + QByteArray entityBinaryData; + + _avatarEntitiesLock.withReadLock([this, &entityBinaryData, &traitInstanceID] { + if (_avatarEntityData.contains(traitInstanceID)) { + entityBinaryData = _avatarEntityData[traitInstanceID]; + } + }); + + if (!entityBinaryData.isNull()) { + AvatarTraits::TraitWireSize entityBinarySize = entityBinaryData.size(); + + qDebug() << QJsonDocument::fromBinaryData(entityBinaryData).toJson(); + + destination.writePrimitive(entityBinarySize); + destination.write(entityBinaryData); + } else { + destination.writePrimitive(AvatarTraits::DELETED_TRAIT_SIZE); + } + } +} + void AvatarData::processTrait(AvatarTraits::TraitType traitType, QByteArray traitBinaryData) { if (traitType == AvatarTraits::SkeletonModelURL) { // get the URL from the binary data auto skeletonModelURL = QUrl::fromEncoded(traitBinaryData); - qDebug() << "Setting skeleton model URL from trait packet to" << skeletonModelURL; setSkeletonModelURL(skeletonModelURL); } } +void AvatarData::processTraitInstance(AvatarTraits::TraitType traitType, + AvatarTraits::TraitInstanceID instanceID, QByteArray traitBinaryData) { + if (traitType == AvatarTraits::AvatarEntity) { + updateAvatarEntity(instanceID, traitBinaryData); + } +} + +void AvatarData::processDeletedTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID) { + if (traitType == AvatarTraits::AvatarEntity) { + removeAvatarEntityAndDetach(instanceID); + } +} + QByteArray AvatarData::identityByteArray(bool setIsReplicated) const { QByteArray identityData; QDataStream identityStream(&identityData, QIODevice::Append); @@ -1868,17 +1909,13 @@ QByteArray AvatarData::identityByteArray(bool setIsReplicated) const { // when mixers send identity packets to agents, they simply forward along the last incoming sequence number they received // whereas agents send a fresh outgoing sequence number when identity data has changed - _avatarEntitiesLock.withReadLock([&] { - identityStream << getSessionUUID() - << (udt::SequenceNumber::Type) _identitySequenceNumber - << _attachmentData - << _displayName - << getSessionDisplayNameForTransport() // depends on _sessionDisplayName - << (_isReplicated || setIsReplicated) - << _avatarEntityData - << _lookAtSnappingEnabled - ; - }); + identityStream << getSessionUUID() + << (udt::SequenceNumber::Type) _identitySequenceNumber + << _attachmentData + << _displayName + << getSessionDisplayNameForTransport() // depends on _sessionDisplayName + << (_isReplicated || setIsReplicated) + << _lookAtSnappingEnabled; return identityData; } @@ -1899,7 +1936,7 @@ void AvatarData::setSkeletonModelURL(const QUrl& skeletonModelURL) { updateJointMappings(); if (_clientTraitsHandler) { - _clientTraitsHandler->markTraitChanged(AvatarTraits::SkeletonModelURL); + _clientTraitsHandler->markTraitUpdated(AvatarTraits::SkeletonModelURL); } emit skeletonModelURLChanged(); @@ -2095,7 +2132,6 @@ void AvatarData::sendIdentityPacket() { nodeList->sendPacketList(std::move(packetList), *node); }); - _avatarEntityDataLocallyEdited = false; _identityDataChanged = false; } @@ -2650,23 +2686,37 @@ void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& ent if (itr == _avatarEntityData.end()) { if (_avatarEntityData.size() < MAX_NUM_AVATAR_ENTITIES) { _avatarEntityData.insert(entityID, entityData); - _avatarEntityDataLocallyEdited = true; - markIdentityDataChanged(); } } else { itr.value() = entityData; - _avatarEntityDataLocallyEdited = true; - markIdentityDataChanged(); } }); + + if (_clientTraitsHandler) { + // we have a client traits handler, so we need to mark this instanced trait as changed + // so that changes will be sent next frame + _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID); + } } void AvatarData::clearAvatarEntity(const QUuid& entityID) { _avatarEntitiesLock.withWriteLock([&] { _avatarEntityData.remove(entityID); - _avatarEntityDataLocallyEdited = true; - markIdentityDataChanged(); }); + + if (_clientTraitsHandler) { + // we have a client traits handler, so we need to mark this removed instance trait as changed + // so that changes are sent next frame + _clientTraitsHandler->markInstancedTraitDeleted(AvatarTraits::AvatarEntity, entityID); + } +} + +void AvatarData::removeAvatarEntityAndDetach(const QUuid &entityID) { + _avatarEntitiesLock.withWriteLock([this, &entityID]{ + _avatarEntityData.remove(entityID); + }); + + insertDetachedEntityID(entityID); } AvatarEntityMap AvatarData::getAvatarEntityData() const { diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index a5d2d0749b..97ae90f694 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -947,12 +947,10 @@ public: const HeadData* getHeadData() const { return _headData; } struct Identity { - QUrl skeletonModelURL; QVector attachmentData; QString displayName; QString sessionDisplayName; bool isReplicated; - AvatarEntityMap avatarEntityData; bool lookAtSnappingEnabled; }; @@ -960,12 +958,21 @@ public: // identityChanged returns true if identity has changed, false otherwise. Similarly for displayNameChanged and skeletonModelUrlChange. void processAvatarIdentity(const QByteArray& identityData, bool& identityChanged, bool& displayNameChanged); - void packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, int64_t traitVersion = -1); + void packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, + AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION); + void packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID, + ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION); + void processTrait(AvatarTraits::TraitType traitType, QByteArray traitBinaryData); + void processTraitInstance(AvatarTraits::TraitType traitType, + AvatarTraits::TraitInstanceID instanceID, QByteArray traitBinaryData); + void processDeletedTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID); QByteArray identityByteArray(bool setIsReplicated = false) const; + QUrl getWireSafeSkeletonModelURL() const; const QUrl& getSkeletonModelURL() const { return _skeletonModelURL; } + const QString& getDisplayName() const { return _displayName; } const QString& getSessionDisplayName() const { return _sessionDisplayName; } bool getLookAtSnappingEnabled() const { return _lookAtSnappingEnabled; } @@ -1311,6 +1318,8 @@ protected: virtual const QString& getSessionDisplayNameForTransport() const { return _sessionDisplayName; } virtual void maybeUpdateSessionDisplayNameFromTransport(const QString& sessionDisplayName) { } // No-op in AvatarMixer + void removeAvatarEntityAndDetach(const QUuid& entityID); + // Body scale float _targetScale; float _domainMinimumHeight { MIN_AVATAR_HEIGHT }; @@ -1415,7 +1424,6 @@ protected: mutable ReadWriteLockable _avatarEntitiesLock; AvatarEntityIDs _avatarEntityDetached; // recently detached from this avatar AvatarEntityMap _avatarEntityData; - bool _avatarEntityDataLocallyEdited { false }; bool _avatarEntityDataChanged { false }; // used to transform any sensor into world space, including the _hmdSensorMat, or hand controllers. diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 407e88e27c..529614b20d 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -194,26 +194,6 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer } } -bool AvatarHashMap::checkLastProcessedTraitVersion(QUuid avatarID, - AvatarTraits::TraitType traitType, AvatarTraits::TraitVersion newVersion) { - auto it = _processedSimpleTraitVersions.find(avatarID); - if (it == _processedSimpleTraitVersions.end()) { - auto pair = _processedSimpleTraitVersions.insert({ - avatarID, - { AvatarTraits::TotalTraitTypes, AvatarTraits::DEFAULT_TRAIT_VERSION } - }); - - it = pair.first; - }; - - if (it->second[traitType] < newVersion) { - it->second[traitType] = newVersion; - return true; - } else { - return false; - } -} - void AvatarHashMap::processBulkAvatarTraits(QSharedPointer message, SharedNodePointer sendingNode) { while (message->getBytesLeftToRead()) { // read the avatar ID to figure out which avatar this is for @@ -233,24 +213,55 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer mess AvatarTraits::TraitType traitType; message->readPrimitive(&traitType); + // grab the last trait versions for this avatar + auto& lastProcessedVersions = _processedTraitVersions[avatarID]; + while (traitType != AvatarTraits::NullTrait) { - AvatarTraits::TraitVersion traitVersion; - message->readPrimitive(&traitVersion); + AvatarTraits::TraitVersion packetTraitVersion; + message->readPrimitive(&packetTraitVersion); AvatarTraits::TraitWireSize traitBinarySize; - message->readPrimitive(&traitBinarySize); + bool skipBinaryTrait = false; - if (avatar) { - // check if this trait version is newer than what we already have for this avatar - bool traitIsNewer = checkLastProcessedTraitVersion(avatarID, traitType, traitVersion); - if (traitIsNewer) { - avatar->processTrait(traitType, message->readWithoutCopy(traitBinarySize)); - } else { - message->seek(message->getPosition() + traitBinarySize); + if (!avatar) { + skipBinaryTrait = true; + } + + if (AvatarTraits::isSimpleTrait(traitType)) { + message->readPrimitive(&traitBinarySize); + + if (avatar) { + // check if this trait version is newer than what we already have for this avatar + if (packetTraitVersion > lastProcessedVersions[traitType]) { + avatar->processTrait(traitType, message->read(traitBinarySize)); + lastProcessedVersions[traitType] = packetTraitVersion; + } else { + skipBinaryTrait = true; + } } } else { - // though we have no avatar pointer, we still hop through the packet in case there are - // traits for avatars we do have later in the packet + AvatarTraits::TraitInstanceID traitInstanceID = + QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); + + message->readPrimitive(&traitBinarySize); + + if (avatar) { + auto& processedInstanceVersion = lastProcessedVersions.getInstanceValueRef(traitType, traitInstanceID); + if (packetTraitVersion > processedInstanceVersion) { + if (traitBinarySize == AvatarTraits::DELETED_TRAIT_SIZE) { + avatar->processDeletedTraitInstance(traitType, traitInstanceID); + } else { + avatar->processTraitInstance(traitType, traitInstanceID, message->read(traitBinarySize)); + } + processedInstanceVersion = packetTraitVersion; + } else { + skipBinaryTrait = true; + } + } + } + + if (skipBinaryTrait) { + // we didn't read this trait because it was older or because we didn't have an avatar to process it for message->seek(message->getPosition() + traitBinarySize); } diff --git a/libraries/avatars/src/AvatarHashMap.h b/libraries/avatars/src/AvatarHashMap.h index ed8440eb89..ba16fa9568 100644 --- a/libraries/avatars/src/AvatarHashMap.h +++ b/libraries/avatars/src/AvatarHashMap.h @@ -30,7 +30,7 @@ #include "ScriptAvatarData.h" #include "AvatarData.h" -#include "AvatarTraits.h" +#include "AssociatedTraitValues.h" /**jsdoc * Note: An AvatarList API is also provided for Interface and client entity scripts: it is a @@ -155,10 +155,7 @@ protected: virtual void removeAvatar(const QUuid& sessionUUID, KillAvatarReason removalReason = KillAvatarReason::NoReason); virtual void handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason = KillAvatarReason::NoReason); - - bool checkLastProcessedTraitVersion(QUuid avatarID, - AvatarTraits::TraitType traitType, AvatarTraits::TraitVersion newVersion); - + AvatarHash _avatarHash; struct PendingAvatar { std::chrono::steady_clock::time_point creationTime; @@ -169,7 +166,7 @@ protected: AvatarPendingHash _pendingAvatars; mutable QReadWriteLock _hashLock; - std::unordered_map _processedSimpleTraitVersions; + std::unordered_map _processedTraitVersions; private: QUuid _lastOwnerSessionUUID; }; diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h index d30da0e1af..acac215799 100644 --- a/libraries/avatars/src/AvatarTraits.h +++ b/libraries/avatars/src/AvatarTraits.h @@ -16,46 +16,43 @@ #include #include +#include + namespace AvatarTraits { enum TraitType : int8_t { NullTrait = -1, SkeletonModelURL, + AvatarEntity, TotalTraitTypes }; - class TraitTypeSet { - public: - TraitTypeSet() {}; - - TraitTypeSet(std::initializer_list types) { - for (auto type : types) { - _types[type] = true; - } - }; + using TraitInstanceID = QUuid; - bool contains(TraitType type) const { return _types[type]; } + inline bool isSimpleTrait(TraitType traitType) { + return traitType == SkeletonModelURL; + } - bool hasAny() const { return std::find(_types.begin(), _types.end(), true) != _types.end(); } - int size() const { return std::count(_types.begin(), _types.end(), true); } - - void insert(TraitType type) { _types[type] = true; } - void erase(TraitType type) { _types[type] = false; } - void clear() { std::fill(_types.begin(), _types.end(), false); } - private: - std::vector _types = { AvatarTraits::TotalTraitTypes, false }; - }; - - const TraitTypeSet SimpleTraitTypes = { SkeletonModelURL }; - - using TraitVersion = uint32_t; + using TraitVersion = int32_t; const TraitVersion DEFAULT_TRAIT_VERSION = 0; + const TraitVersion NULL_TRAIT_VERSION = -1; - using NullableTraitVersion = int64_t; - const NullableTraitVersion NULL_TRAIT_VERSION = -1; + using TraitWireSize = int16_t; + const TraitWireSize DELETED_TRAIT_SIZE = -1; - using TraitWireSize = uint16_t; + inline void packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination, + TraitVersion traitVersion = NULL_TRAIT_VERSION) { + destination.writePrimitive(traitType); - using SimpleTraitVersions = std::vector; + if (traitVersion > DEFAULT_TRAIT_VERSION) { + AvatarTraits::TraitVersion typedVersion = traitVersion; + destination.writePrimitive(typedVersion); + } + + destination.write(instanceID.toRfc4122()); + + destination.writePrimitive(DELETED_TRAIT_SIZE); + + } }; #endif // hifi_AvatarTraits_h diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp index 8b3ded1e1c..cf67304937 100644 --- a/libraries/avatars/src/ClientTraitsHandler.cpp +++ b/libraries/avatars/src/ClientTraitsHandler.cpp @@ -36,11 +36,11 @@ void ClientTraitsHandler::resetForNewMixer() { _currentTraitVersion = AvatarTraits::DEFAULT_TRAIT_VERSION; // mark that all traits should be sent next time - _performInitialSend = true; + _shouldPerformInitialSend = true; } void ClientTraitsHandler::sendChangedTraitsToMixer() { - if (hasChangedTraits() || _performInitialSend) { + if (hasChangedTraits() || _shouldPerformInitialSend) { // we have at least one changed trait to send auto nodeList = DependencyManager::get(); @@ -51,31 +51,57 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() { } // we have a mixer to send to, setup our set traits packet + auto traitsPacketList = NLPacketList::create(PacketType::SetAvatarTraits, QByteArray(), true, true); // bump and write the current trait version to an extended header // the trait version is the same for all traits in this packet list - ++_currentTraitVersion; - QByteArray extendedHeader(reinterpret_cast(&_currentTraitVersion), sizeof(_currentTraitVersion)); - - auto traitsPacketList = NLPacketList::create(PacketType::SetAvatarTraits, extendedHeader, true); + traitsPacketList->writePrimitive(++_currentTraitVersion); // take a copy of the set of changed traits and clear the stored set - auto changedTraitsCopy { _changedTraits }; - _changedTraits.clear(); + auto traitStatusesCopy { _traitStatuses }; + _traitStatuses.reset(); + _hasChangedTraits = false; - if (_performInitialSend || changedTraitsCopy.contains(AvatarTraits::SkeletonModelURL)) { - traitsPacketList->startSegment(); - _owningAvatar->packTrait(AvatarTraits::SkeletonModelURL, *traitsPacketList); - traitsPacketList->endSegment(); + auto simpleIt = traitStatusesCopy.simpleCBegin(); + while (simpleIt != traitStatusesCopy.simpleCEnd()) { + // because the vector contains all trait types (for access using trait type as index) + // we double check that it is a simple iterator here + auto traitType = static_cast(std::distance(traitStatusesCopy.simpleCBegin(), simpleIt)); - // keep track of our skeleton version in case we get an override back - _currentSkeletonVersion = _currentTraitVersion; + if (AvatarTraits::isSimpleTrait(traitType)) { + if (_shouldPerformInitialSend || *simpleIt == Updated) { + if (traitType == AvatarTraits::SkeletonModelURL) { + _owningAvatar->packTrait(traitType, *traitsPacketList); + + // keep track of our skeleton version in case we get an override back + _currentSkeletonVersion = _currentTraitVersion; + } + } + } + + ++simpleIt; + } + + auto instancedIt = traitStatusesCopy.instancedCBegin(); + while (instancedIt != traitStatusesCopy.instancedCEnd()) { + for (auto& instanceIDValuePair : instancedIt->instances) { + if (_shouldPerformInitialSend || instanceIDValuePair.value == Updated) { + // this is a changed trait we need to send, ask the owning avatar to pack it + _owningAvatar->packTraitInstance(instancedIt->traitType, instanceIDValuePair.id, *traitsPacketList); + } else if (instanceIDValuePair.value == Deleted) { + // pack delete for this trait instance + AvatarTraits::packInstancedTraitDelete(instancedIt->traitType, instanceIDValuePair.id, + *traitsPacketList); + } + } + + ++instancedIt; } nodeList->sendPacketList(std::move(traitsPacketList), *avatarMixer); // if this was an initial send of all traits, consider it completed - _performInitialSend = false; + _shouldPerformInitialSend = false; } } @@ -95,13 +121,13 @@ void ClientTraitsHandler::processTraitOverride(QSharedPointer m // and the version matches what we last sent for skeleton if (traitType == AvatarTraits::SkeletonModelURL && traitVersion == _currentSkeletonVersion - && !hasTraitChanged(AvatarTraits::SkeletonModelURL)) { + && _traitStatuses[AvatarTraits::SkeletonModelURL] != Updated) { // override the skeleton URL but do not mark the trait as having changed // so that we don't unecessarily sent a new trait packet to the mixer with the overriden URL auto encodedSkeletonURL = QUrl::fromEncoded(message->readWithoutCopy(traitBinarySize)); _owningAvatar->setSkeletonModelURL(encodedSkeletonURL); - _changedTraits.erase(AvatarTraits::SkeletonModelURL); + _traitStatuses.erase(AvatarTraits::SkeletonModelURL); } else { message->seek(message->getPosition() + traitBinarySize); } diff --git a/libraries/avatars/src/ClientTraitsHandler.h b/libraries/avatars/src/ClientTraitsHandler.h index 1d4c67d0c4..27ba58d46b 100644 --- a/libraries/avatars/src/ClientTraitsHandler.h +++ b/libraries/avatars/src/ClientTraitsHandler.h @@ -14,7 +14,7 @@ #include -#include "AvatarTraits.h" +#include "AssociatedTraitValues.h" #include "Node.h" class AvatarData; @@ -26,10 +26,14 @@ public: void sendChangedTraitsToMixer(); - bool hasChangedTraits() { return _changedTraits.hasAny(); } - void markTraitChanged(AvatarTraits::TraitType changedTrait) { _changedTraits.insert(changedTrait); } + bool hasChangedTraits() { return _hasChangedTraits; } - bool hasTraitChanged(AvatarTraits::TraitType checkTrait) { return _changedTraits.contains(checkTrait) > 0; } + void markTraitUpdated(AvatarTraits::TraitType updatedTrait) + { _traitStatuses[updatedTrait] = Updated; _hasChangedTraits = true; } + void markInstancedTraitUpdated(AvatarTraits::TraitType traitType, QUuid updatedInstanceID) + { _traitStatuses.instanceInsert(traitType, updatedInstanceID, Updated); _hasChangedTraits = true; } + void markInstancedTraitDeleted(AvatarTraits::TraitType traitType, QUuid deleteInstanceID) + { _traitStatuses.instanceInsert(traitType, deleteInstanceID, Deleted); _hasChangedTraits = true; } void resetForNewMixer(); @@ -37,14 +41,21 @@ public slots: void processTraitOverride(QSharedPointer message, SharedNodePointer sendingNode); private: + enum ClientTraitStatus { + Unchanged, + Updated, + Deleted + }; + AvatarData* _owningAvatar; - AvatarTraits::TraitTypeSet _changedTraits; + AvatarTraits::AssociatedTraitValues _traitStatuses; AvatarTraits::TraitVersion _currentTraitVersion { AvatarTraits::DEFAULT_TRAIT_VERSION }; - AvatarTraits::NullableTraitVersion _currentSkeletonVersion { AvatarTraits::NULL_TRAIT_VERSION }; + AvatarTraits::TraitVersion _currentSkeletonVersion { AvatarTraits::NULL_TRAIT_VERSION }; - bool _performInitialSend { false }; + bool _shouldPerformInitialSend { false }; + bool _hasChangedTraits { false }; }; #endif // hifi_ClientTraitsHandler_h diff --git a/libraries/networking/src/udt/PacketHeaders.cpp b/libraries/networking/src/udt/PacketHeaders.cpp index 94137786cd..9ed9a4f385 100644 --- a/libraries/networking/src/udt/PacketHeaders.cpp +++ b/libraries/networking/src/udt/PacketHeaders.cpp @@ -40,7 +40,7 @@ PacketVersion versionForPacketType(PacketType packetType) { case PacketType::AvatarData: case PacketType::BulkAvatarData: case PacketType::KillAvatar: - return static_cast(AvatarMixerPacketVersion::MigrateSkeletonURLToTraits); + return static_cast(AvatarMixerPacketVersion::MigrateAvatarEntitiesToTraits); case PacketType::MessagesData: return static_cast(MessageDataVersion::TextOrBinaryData); // ICE packets diff --git a/libraries/networking/src/udt/PacketHeaders.h b/libraries/networking/src/udt/PacketHeaders.h index 31724ab5dc..8a2add3bb3 100644 --- a/libraries/networking/src/udt/PacketHeaders.h +++ b/libraries/networking/src/udt/PacketHeaders.h @@ -290,8 +290,9 @@ enum class AvatarMixerPacketVersion : PacketVersion { FBXReaderNodeReparenting, FixMannequinDefaultAvatarFeet, ProceduralFaceMovementFlagsAndBlendshapes, - FarGrabJoints - MigrateSkeletonURLToTraits + FarGrabJoints, + MigrateSkeletonURLToTraits, + MigrateAvatarEntitiesToTraits }; enum class DomainConnectRequestVersion : PacketVersion { From a56e9b08603e5cbacf1119a28f759d91e15ac1c2 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 9 Aug 2018 19:01:52 -0700 Subject: [PATCH 10/31] allow agent to create and get avatar entities from script --- assignment-client/src/Agent.cpp | 7 +++++++ libraries/avatars/src/ScriptAvatarData.cpp | 20 ++++++++++++++++++++ libraries/avatars/src/ScriptAvatarData.h | 4 ++++ 3 files changed, 31 insertions(+) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index 049e7d0ede..a8466fa368 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -365,6 +365,8 @@ void Agent::executeScript() { // setup an Avatar for the script to use auto scriptedAvatar = DependencyManager::get(); + scriptedAvatar->setID(getSessionUUID()); + connect(_scriptEngine.data(), SIGNAL(update(float)), scriptedAvatar.data(), SLOT(update(float)), Qt::ConnectionType::QueuedConnection); scriptedAvatar->setForceFaceTrackerConnected(true); @@ -606,6 +608,11 @@ void Agent::setIsAvatar(bool isAvatar) { } QMetaObject::invokeMethod(&_avatarAudioTimer, "stop"); + + _entityEditSender.setMyAvatar(nullptr); + } else { + auto scriptableAvatar = DependencyManager::get(); + _entityEditSender.setMyAvatar(scriptableAvatar.data()); } } diff --git a/libraries/avatars/src/ScriptAvatarData.cpp b/libraries/avatars/src/ScriptAvatarData.cpp index 8491e5368b..a716a40ad8 100644 --- a/libraries/avatars/src/ScriptAvatarData.cpp +++ b/libraries/avatars/src/ScriptAvatarData.cpp @@ -269,6 +269,26 @@ QVector ScriptAvatarData::getAttachmentData() const { // END // +#if PR_BUILD || DEV_BUILD +// +// ENTITY PROPERTIES +// START +// +AvatarEntityMap ScriptAvatarData::getAvatarEntities() const { + AvatarEntityMap scriptEntityData; + + if (AvatarSharedPointer sharedAvatarData = _avatarData.lock()) { + return sharedAvatarData->getAvatarEntityData(); + } + + return scriptEntityData; +} +// +// ENTITY PROPERTIES +// END +// +#endif + // // AUDIO PROPERTIES diff --git a/libraries/avatars/src/ScriptAvatarData.h b/libraries/avatars/src/ScriptAvatarData.h index 13713ff15f..91bac61728 100644 --- a/libraries/avatars/src/ScriptAvatarData.h +++ b/libraries/avatars/src/ScriptAvatarData.h @@ -116,6 +116,10 @@ public: Q_INVOKABLE QStringList getJointNames() const; Q_INVOKABLE QVector getAttachmentData() const; +#if DEV_BUILD || PR_BUILD + Q_INVOKABLE AvatarEntityMap getAvatarEntities() const; +#endif + // // AUDIO PROPERTIES // From f9230eca7f4f4a44eee606f6a8b927e24c4ec8c3 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 9 Aug 2018 22:05:15 -0700 Subject: [PATCH 11/31] don't send override avatar URL if override matches --- .../src/avatars/AvatarMixerClientData.cpp | 23 +++++++++++-------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 34b7ec97ff..274a76d0fa 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -178,19 +178,22 @@ void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(SlaveSharedData *sl } if (!inWhitelist) { - // we need to change this avatar's skeleton URL, and send them a traits packet informing them of the change - _avatar->setSkeletonModelURL(slaveSharedData->skeletonReplacementURL); + // make sure we're not unecessarily overriding the default avatar with the default avatar + if (_avatar->getWireSafeSkeletonModelURL() != slaveSharedData->skeletonReplacementURL) { + // we need to change this avatar's skeleton URL, and send them a traits packet informing them of the change + qDebug() << "Overwriting avatar URL" << _avatar->getWireSafeSkeletonModelURL() + << "to replacement" << slaveSharedData->skeletonReplacementURL << "for" << sendingNode.getUUID(); + _avatar->setSkeletonModelURL(slaveSharedData->skeletonReplacementURL); - qDebug() << "Sending overwritten" << _avatar->getSkeletonModelURL() << "back to sending avatar"; + auto packet = NLPacket::create(PacketType::SetAvatarTraits, -1, true); - auto packet = NLPacket::create(PacketType::SetAvatarTraits, -1, true); - - // the returned set traits packet uses the trait version from the incoming packet - // so the client knows they should not overwrite if they have since changed the trait - _avatar->packTrait(AvatarTraits::SkeletonModelURL, *packet, traitVersion); + // the returned set traits packet uses the trait version from the incoming packet + // so the client knows they should not overwrite if they have since changed the trait + _avatar->packTrait(AvatarTraits::SkeletonModelURL, *packet, traitVersion); - auto nodeList = DependencyManager::get(); - nodeList->sendPacket(std::move(packet), sendingNode); + auto nodeList = DependencyManager::get(); + nodeList->sendPacket(std::move(packet), sendingNode); + } } } } From de6fe43dda15804eb7e9ea8991a3850c6d589370 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 9 Aug 2018 22:05:53 -0700 Subject: [PATCH 12/31] ensure joint mapping is processed for current FST url --- libraries/avatars/src/AvatarData.cpp | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 36dbe00937..4e396e2a9e 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2034,6 +2034,13 @@ void AvatarData::setJointMappingsFromNetworkReply() { QNetworkReply* networkReply = static_cast(sender()); + // before we process this update, make sure that the skeleton model URL hasn't changed + // since we made the FST request + if (networkReply->url() != _skeletonModelURL) { + qCDebug(avatars) << "Refusing to set joint mappings for FST URL that does not match the current URL"; + return; + } + { QWriteLocker writeLock(&_jointDataLock); QByteArray line; From 1158ec8d50b290ab36afcc6be9faf6d87899e898 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Thu, 9 Aug 2018 22:06:28 -0700 Subject: [PATCH 13/31] improve change avoidance after avatar url override --- libraries/avatars/src/ClientTraitsHandler.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp index cf67304937..a31808a916 100644 --- a/libraries/avatars/src/ClientTraitsHandler.cpp +++ b/libraries/avatars/src/ClientTraitsHandler.cpp @@ -122,12 +122,19 @@ void ClientTraitsHandler::processTraitOverride(QSharedPointer m if (traitType == AvatarTraits::SkeletonModelURL && traitVersion == _currentSkeletonVersion && _traitStatuses[AvatarTraits::SkeletonModelURL] != Updated) { + // override the skeleton URL but do not mark the trait as having changed - // so that we don't unecessarily sent a new trait packet to the mixer with the overriden URL + // so that we don't unecessarily send a new trait packet to the mixer with the overriden URL auto encodedSkeletonURL = QUrl::fromEncoded(message->readWithoutCopy(traitBinarySize)); + + auto hasChangesBefore = _hasChangedTraits; + _owningAvatar->setSkeletonModelURL(encodedSkeletonURL); + // setSkeletonModelURL will flag us for changes to the SkeletonModelURL so we reset some state here to + // avoid unnecessarily sending the overriden skeleton model URL back to the mixer _traitStatuses.erase(AvatarTraits::SkeletonModelURL); + _hasChangedTraits = hasChangesBefore; } else { message->seek(message->getPosition() + traitBinarySize); } From fc5b72e9b952c8bcd82692c1dd8d21ee6afe5c60 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 10 Aug 2018 15:09:35 -0700 Subject: [PATCH 14/31] cleanup sent trait versions for removed avatars on mixer --- assignment-client/src/avatars/AvatarMixer.cpp | 3 ++- assignment-client/src/avatars/AvatarMixerClientData.cpp | 7 +++++++ assignment-client/src/avatars/AvatarMixerClientData.h | 5 +---- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 228102ee53..167c1cd29c 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/assignment-client/src/avatars/AvatarMixer.cpp @@ -470,7 +470,8 @@ void AvatarMixer::handleAvatarKilled(SharedNodePointer avatarNode) { QMetaObject::invokeMethod(node->getLinkedData(), "cleanupKilledNode", Qt::AutoConnection, - Q_ARG(const QUuid&, QUuid(avatarNode->getUUID()))); + Q_ARG(const QUuid&, QUuid(avatarNode->getUUID())), + Q_ARG(Node::LocalID, avatarNode->getLocalID())); } ); } diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 274a76d0fa..cc4356fb1a 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -285,3 +285,10 @@ AvatarMixerClientData::TraitsCheckTimestamp AvatarMixerClientData::getLastOtherA return TraitsCheckTimestamp(); } } + +void AvatarMixerClientData::cleanupKilledNode(const QUuid& nodeUUID, Node::LocalID nodeLocalID) { + removeLastBroadcastSequenceNumber(nodeUUID); + removeLastBroadcastTime(nodeUUID); + _lastSentTraitsTimestamps.erase(nodeLocalID); + _sentTraitVersions.erase(nodeLocalID); +} diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index dcbf8a6dba..02b37e08f8 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -57,10 +57,7 @@ public: void setLastBroadcastTime(const QUuid& nodeUUID, uint64_t broadcastTime) { _lastBroadcastTimes[nodeUUID] = broadcastTime; } Q_INVOKABLE void removeLastBroadcastTime(const QUuid& nodeUUID) { _lastBroadcastTimes.erase(nodeUUID); } - Q_INVOKABLE void cleanupKilledNode(const QUuid& nodeUUID) { - removeLastBroadcastSequenceNumber(nodeUUID); - removeLastBroadcastTime(nodeUUID); - } + Q_INVOKABLE void cleanupKilledNode(const QUuid& nodeUUID, Node::LocalID nodeLocalID); uint16_t getLastReceivedSequenceNumber() const { return _lastReceivedSequenceNumber; } From 0f03764c97693e026e0eed7be8ee5cdd541a8d35 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 10 Aug 2018 15:10:50 -0700 Subject: [PATCH 15/31] create missing avatar when processing traits --- libraries/avatars/src/AvatarHashMap.cpp | 45 +++++++++---------------- 1 file changed, 16 insertions(+), 29 deletions(-) diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 529614b20d..0ab602e233 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -200,14 +200,8 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer mess auto avatarID = QUuid::fromRfc4122(message->readWithoutCopy(NUM_BYTES_RFC4122_UUID)); // grab the avatar so we can ask it to process trait data - AvatarSharedPointer avatar; - - QReadLocker locker(&_hashLock); - auto it = _avatarHash.find(avatarID); - if (it != _avatarHash.end()) { - avatar = *it; - } - locker.unlock(); + bool isNewAvatar; + auto avatar = newOrExistingAvatar(avatarID, sendingNode, isNewAvatar); // read the first trait type for this avatar AvatarTraits::TraitType traitType; @@ -223,21 +217,16 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer mess AvatarTraits::TraitWireSize traitBinarySize; bool skipBinaryTrait = false; - if (!avatar) { - skipBinaryTrait = true; - } if (AvatarTraits::isSimpleTrait(traitType)) { message->readPrimitive(&traitBinarySize); - if (avatar) { - // check if this trait version is newer than what we already have for this avatar - if (packetTraitVersion > lastProcessedVersions[traitType]) { - avatar->processTrait(traitType, message->read(traitBinarySize)); - lastProcessedVersions[traitType] = packetTraitVersion; - } else { - skipBinaryTrait = true; - } + // check if this trait version is newer than what we already have for this avatar + if (packetTraitVersion > lastProcessedVersions[traitType]) { + avatar->processTrait(traitType, message->read(traitBinarySize)); + lastProcessedVersions[traitType] = packetTraitVersion; + } else { + skipBinaryTrait = true; } } else { AvatarTraits::TraitInstanceID traitInstanceID = @@ -245,18 +234,16 @@ void AvatarHashMap::processBulkAvatarTraits(QSharedPointer mess message->readPrimitive(&traitBinarySize); - if (avatar) { - auto& processedInstanceVersion = lastProcessedVersions.getInstanceValueRef(traitType, traitInstanceID); - if (packetTraitVersion > processedInstanceVersion) { - if (traitBinarySize == AvatarTraits::DELETED_TRAIT_SIZE) { - avatar->processDeletedTraitInstance(traitType, traitInstanceID); - } else { - avatar->processTraitInstance(traitType, traitInstanceID, message->read(traitBinarySize)); - } - processedInstanceVersion = packetTraitVersion; + auto& processedInstanceVersion = lastProcessedVersions.getInstanceValueRef(traitType, traitInstanceID); + if (packetTraitVersion > processedInstanceVersion) { + if (traitBinarySize == AvatarTraits::DELETED_TRAIT_SIZE) { + avatar->processDeletedTraitInstance(traitType, traitInstanceID); } else { - skipBinaryTrait = true; + avatar->processTraitInstance(traitType, traitInstanceID, message->read(traitBinarySize)); } + processedInstanceVersion = packetTraitVersion; + } else { + skipBinaryTrait = true; } } From e33f349d5390ec7f4535d1fca9b050a6fd99bdfa Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 10 Aug 2018 16:06:09 -0700 Subject: [PATCH 16/31] fix flagging for avatar entity update/delete --- interface/src/Application.cpp | 18 +++----- interface/src/avatar/MyAvatar.cpp | 1 - .../src/avatars-renderer/Avatar.cpp | 20 ++++++--- libraries/avatars/src/AvatarData.cpp | 41 +++++++++++-------- libraries/avatars/src/AvatarData.h | 6 +-- .../entities/src/EntityEditPacketSender.cpp | 7 +--- .../entities/src/EntityEditPacketSender.h | 1 - .../entities/src/EntityScriptingInterface.cpp | 13 ++++-- 8 files changed, 56 insertions(+), 51 deletions(-) diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index e556fd734f..60af79bfda 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -1013,7 +1013,7 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo // This is done so as not break previous command line scripts if (testScriptPath.left(URL_SCHEME_HTTP.length()) == URL_SCHEME_HTTP || testScriptPath.left(URL_SCHEME_FTP.length()) == URL_SCHEME_FTP) { - + setProperty(hifi::properties::TEST, QUrl::fromUserInput(testScriptPath)); } else if (QFileInfo(testScriptPath).exists()) { setProperty(hifi::properties::TEST, QUrl::fromLocalFile(testScriptPath)); @@ -1830,14 +1830,6 @@ Application::Application(int& argc, char** argv, QElapsedTimer& startupTimer, bo } }); - connect(getEntities()->getTree().get(), &EntityTree::deletingEntity, [](const EntityItemID& entityItemID) { - auto avatarManager = DependencyManager::get(); - auto myAvatar = avatarManager ? avatarManager->getMyAvatar() : nullptr; - if (myAvatar) { - myAvatar->clearAvatarEntity(entityItemID); - } - }); - EntityTree::setAddMaterialToEntityOperator([this](const QUuid& entityID, graphics::MaterialLayer material, const std::string& parentMaterialName) { // try to find the renderable auto renderable = getEntities()->renderableForEntityId(entityID); @@ -2617,7 +2609,7 @@ Application::~Application() { // Can't log to file passed this point, FileLogger about to be deleted qInstallMessageHandler(LogHandler::verboseMessageHandler); - + _renderEventHandler->deleteLater(); } @@ -5498,8 +5490,8 @@ void Application::update(float deltaTime) { quint64 now = usecTimestampNow(); // Check for flagged EntityData having arrived. auto entityTreeRenderer = getEntities(); - if (isServerlessMode() || - (_octreeProcessor.isLoadSequenceComplete() )) { + if (isServerlessMode() || + (entityTreeRenderer && _octreeProcessor.octreeSequenceIsComplete(entityTreeRenderer->getLastOctreeMessageSequence()) )) { // we've received a new full-scene octree stats packet, or it's been long enough to try again anyway _lastPhysicsCheckTime = now; _fullSceneCounterAtLastPhysicsCheck = _fullSceneReceivedCounter; @@ -6393,7 +6385,7 @@ void Application::nodeActivated(SharedNodePointer node) { if (_avatarOverrideUrl.isValid()) { getMyAvatar()->useFullAvatarURL(_avatarOverrideUrl); } - + if (getMyAvatar()->getFullAvatarURLFromPreferences() != getMyAvatar()->getSkeletonModelURL()) { getMyAvatar()->resetFullAvatarURL(); } diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 2c88f917a1..deef69d980 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1292,7 +1292,6 @@ void MyAvatar::loadData() { // HACK: manually remove empty 'avatarEntityData' else legacy data may persist in settings file settings.remove("avatarEntityData"); } - setAvatarEntityDataChanged(true); // Flying preferences must be loaded before calling setFlyingEnabled() Setting::Handle firstRunVal { Settings::firstRun, true }; diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 0b43fd5433..150ecb6d44 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -224,14 +224,24 @@ void Avatar::setAvatarEntityDataChanged(bool value) { void Avatar::updateAvatarEntities() { PerformanceTimer perfTimer("attachments"); + + // AVATAR ENTITY UPDATE FLOW // - if queueEditEntityMessage sees clientOnly flag it does _myAvatar->updateAvatarEntity() - // - updateAvatarEntity saves the bytes and sets _avatarEntityDataLocallyEdited - // - MyAvatar::update notices _avatarEntityDataLocallyEdited and calls sendIdentityPacket - // - sendIdentityPacket sends the entity bytes to the server which relays them to other interfaces - // - AvatarHashMap::processAvatarIdentityPacket on other interfaces call avatar->setAvatarEntityData() - // - setAvatarEntityData saves the bytes and sets _avatarEntityDataChanged = true + // - updateAvatarEntity saves the bytes and flags the trait instance for the entity as updated + // - ClientTraitsHandler::sendChangedTraitsToMixer sends the entity bytes to the mixer which relays them to other interfaces + // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processTraitInstace + // - AvatarData::processTraitInstance calls updateAvatarEntity, which sets _avatarEntityDataChanged = true // - (My)Avatar::simulate notices _avatarEntityDataChanged and here we are... + // AVATAR ENTITY DELETE FLOW + // - EntityScriptingInterface::deleteEntity calls _myAvatar->clearAvatarEntity() for deleted avatar entities + // - clearAvatarEntity removes the avatar entity and flags the trait instance for the entity as deleted + // - ClientTraitsHandler::sendChangedTraitsToMixer sends a deletion to the mixer which relays to other interfaces + // - AvatarHashMap::processBulkAvatarTraits on other interfaces calls avatar->processDeletedTraitInstace + // - AvatarData::processDeletedTraitInstance calls clearAvatarEntity + // - AvatarData::clearAvatarEntity sets _avatarEntityDataChanged = true and adds the ID to the detached list + // - Avatar::simulate notices _avatarEntityDataChanged and here we are... + if (!_avatarEntityDataChanged) { return; } diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 4e396e2a9e..f1b9986186 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1898,7 +1898,7 @@ void AvatarData::processTraitInstance(AvatarTraits::TraitType traitType, void AvatarData::processDeletedTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID) { if (traitType == AvatarTraits::AvatarEntity) { - removeAvatarEntityAndDetach(instanceID); + clearAvatarEntity(instanceID); } } @@ -2687,7 +2687,7 @@ void AvatarData::setAttachmentsVariant(const QVariantList& variant) { const int MAX_NUM_AVATAR_ENTITIES = 42; -void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) { +void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData, bool requiresTreeUpdate) { _avatarEntitiesLock.withWriteLock([&] { AvatarEntityMap::iterator itr = _avatarEntityData.find(entityID); if (itr == _avatarEntityData.end()) { @@ -2699,6 +2699,10 @@ void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& ent } }); + if (requiresTreeUpdate) { + _avatarEntityDataChanged = true; + } + if (_clientTraitsHandler) { // we have a client traits handler, so we need to mark this instanced trait as changed // so that changes will be sent next frame @@ -2706,26 +2710,27 @@ void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& ent } } -void AvatarData::clearAvatarEntity(const QUuid& entityID) { - _avatarEntitiesLock.withWriteLock([&] { - _avatarEntityData.remove(entityID); +void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) { + + bool removedEntity = false; + + _avatarEntitiesLock.withWriteLock([this, &removedEntity, &entityID] { + removedEntity = _avatarEntityData.remove(entityID); }); - if (_clientTraitsHandler) { - // we have a client traits handler, so we need to mark this removed instance trait as changed - // so that changes are sent next frame - _clientTraitsHandler->markInstancedTraitDeleted(AvatarTraits::AvatarEntity, entityID); + if (removedEntity) { + if (requiresRemovalFromTree) { + insertDetachedEntityID(entityID); + } + + if (_clientTraitsHandler) { + // we have a client traits handler, so we need to mark this removed instance trait as changed + // so that changes are sent next frame + _clientTraitsHandler->markInstancedTraitDeleted(AvatarTraits::AvatarEntity, entityID); + } } } -void AvatarData::removeAvatarEntityAndDetach(const QUuid &entityID) { - _avatarEntitiesLock.withWriteLock([this, &entityID]{ - _avatarEntityData.remove(entityID); - }); - - insertDetachedEntityID(entityID); -} - AvatarEntityMap AvatarData::getAvatarEntityData() const { AvatarEntityMap result; _avatarEntitiesLock.withReadLock([&] { @@ -2738,6 +2743,8 @@ void AvatarData::insertDetachedEntityID(const QUuid entityID) { _avatarEntitiesLock.withWriteLock([&] { _avatarEntityDetached.insert(entityID); }); + + _avatarEntityDataChanged = true; } void AvatarData::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 97ae90f694..6f1871974d 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -925,13 +925,13 @@ public: * @param {Uuid} entityID * @param {string} entityData */ - Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData); + Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData, bool requiresTreeUpdate = true); /**jsdoc * @function MyAvatar.clearAvatarEntity * @param {Uuid} entityID */ - Q_INVOKABLE void clearAvatarEntity(const QUuid& entityID); + Q_INVOKABLE void clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree = true); /**jsdoc @@ -1318,8 +1318,6 @@ protected: virtual const QString& getSessionDisplayNameForTransport() const { return _sessionDisplayName; } virtual void maybeUpdateSessionDisplayNameFromTransport(const QString& sessionDisplayName) { } // No-op in AvatarMixer - void removeAvatarEntityAndDetach(const QUuid& entityID); - // Body scale float _targetScale; float _domainMinimumHeight { MIN_AVATAR_HEIGHT }; diff --git a/libraries/entities/src/EntityEditPacketSender.cpp b/libraries/entities/src/EntityEditPacketSender.cpp index 0982775b09..d288126348 100644 --- a/libraries/entities/src/EntityEditPacketSender.cpp +++ b/libraries/entities/src/EntityEditPacketSender.cpp @@ -74,7 +74,7 @@ void EntityEditPacketSender::queueEditAvatarEntityMessage(PacketType type, jsonProperties = QJsonDocument(jsonObject); QByteArray binaryProperties = jsonProperties.toBinaryData(); - _myAvatar->updateAvatarEntity(entityItemID, binaryProperties); + _myAvatar->updateAvatarEntity(entityItemID, binaryProperties, false); entity->setLastBroadcast(usecTimestampNow()); } @@ -149,11 +149,6 @@ void EntityEditPacketSender::queueEditEntityMessage(PacketType type, void EntityEditPacketSender::queueEraseEntityMessage(const EntityItemID& entityItemID) { - // in case this was a clientOnly entity: - if(_myAvatar) { - _myAvatar->clearAvatarEntity(entityItemID); - } - QByteArray bufferOut(NLPacket::maxPayloadSize(PacketType::EntityErase), 0); if (EntityItemProperties::encodeEraseEntityMessage(entityItemID, bufferOut)) { diff --git a/libraries/entities/src/EntityEditPacketSender.h b/libraries/entities/src/EntityEditPacketSender.h index 31f91707b8..9bf9095f7f 100644 --- a/libraries/entities/src/EntityEditPacketSender.h +++ b/libraries/entities/src/EntityEditPacketSender.h @@ -27,7 +27,6 @@ public: void setMyAvatar(AvatarData* myAvatar) { _myAvatar = myAvatar; } AvatarData* getMyAvatar() { return _myAvatar; } - void clearAvatarEntity(QUuid entityID) { assert(_myAvatar); _myAvatar->clearAvatarEntity(entityID); } /// Queues an array of several voxel edit messages. Will potentially send a pending multi-command packet. Determines /// which voxel-server node or nodes the packet should be sent to. Can be called even before voxel servers are known, in diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp index c080fbbb88..d9924cb9fd 100644 --- a/libraries/entities/src/EntityScriptingInterface.cpp +++ b/libraries/entities/src/EntityScriptingInterface.cpp @@ -574,7 +574,7 @@ void EntityScriptingInterface::deleteEntity(QUuid id) { _activityTracking.deletedEntityCount++; EntityItemID entityID(id); - bool shouldDelete = true; + bool shouldSendDeleteToServer = true; // If we have a local entity tree set, then also update it. if (_entityTree) { @@ -591,16 +591,21 @@ void EntityScriptingInterface::deleteEntity(QUuid id) { auto avatarHashMap = DependencyManager::get(); AvatarSharedPointer myAvatar = avatarHashMap->getAvatarBySessionID(myNodeID); myAvatar->insertDetachedEntityID(id); - shouldDelete = false; + shouldSendDeleteToServer = false; return; } if (entity->getLocked()) { - shouldDelete = false; + shouldSendDeleteToServer = false; } else { // only delete local entities, server entities will round trip through the server filters if (entity->getClientOnly() || _entityTree->isServerlessMode()) { + shouldSendDeleteToServer = false; _entityTree->deleteEntity(entityID); + + if (entity->getClientOnly() && getEntityPacketSender()->getMyAvatar()) { + getEntityPacketSender()->getMyAvatar()->clearAvatarEntity(entityID, false); + } } } } @@ -608,7 +613,7 @@ void EntityScriptingInterface::deleteEntity(QUuid id) { } // if at this point, we know the id, and we should still delete the entity, send the update to the entity server - if (shouldDelete) { + if (shouldSendDeleteToServer) { getEntityPacketSender()->queueEraseEntityMessage(entityID); } } From 6c204b682d5862d8c1bfa007e69089768e64a97c Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 10 Aug 2018 16:47:28 -0700 Subject: [PATCH 17/31] include trait bytes written in over budget calculation --- .../src/avatars/AvatarMixerSlave.cpp | 25 +++++++++------ .../src/avatars/AvatarMixerSlave.h | 6 ++-- libraries/avatars/src/AvatarData.cpp | 31 ++++++++++++------- libraries/avatars/src/AvatarData.h | 8 ++--- libraries/avatars/src/AvatarTraits.h | 13 +++++--- 5 files changed, 50 insertions(+), 33 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index ebbaeb7a35..14818870d1 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -79,9 +79,9 @@ int AvatarMixerSlave::sendIdentityPacket(const AvatarMixerClientData* nodeData, } } -void AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* listeningNodeData, - const AvatarMixerClientData* sendingNodeData, - NLPacketList& traitsPacketList) { +qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* listeningNodeData, + const AvatarMixerClientData* sendingNodeData, + NLPacketList& traitsPacketList) { auto otherNodeLocalID = sendingNodeData->getNodeLocalID(); @@ -90,11 +90,13 @@ void AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* liste auto timeOfLastTraitsSent = listeningNodeData->getLastOtherAvatarTraitsSendPoint(otherNodeLocalID); auto timeOfLastTraitsChange = sendingNodeData->getLastReceivedTraitsChange(); + qint64 bytesWritten = 0; + if (timeOfLastTraitsChange > timeOfLastTraitsSent) { // there is definitely new traits data to send // add the avatar ID to mark the beginning of traits for this avatar - traitsPacketList.write(sendingNodeData->getNodeID().toRfc4122()); + bytesWritten += traitsPacketList.write(sendingNodeData->getNodeID().toRfc4122()); auto sendingAvatar = sendingNodeData->getAvatarSharedPointer(); @@ -115,7 +117,7 @@ void AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* liste if (lastReceivedVersions[traitType] > lastSentVersionRef) { // there is an update to this trait, add it to the traits packet - sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion); + bytesWritten += sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion); // update the last sent version lastSentVersionRef = lastReceivedVersion; @@ -152,7 +154,7 @@ void AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* liste if (!isDeleted && (sentInstanceIt == sentIDValuePairs.end() || receivedVersion > sentInstanceIt->value)) { // this instance version exists and has never been sent or is newer so we need to send it - sendingAvatar->packTraitInstance(traitType, instanceID, traitsPacketList, receivedVersion); + bytesWritten += sendingAvatar->packTraitInstance(traitType, instanceID, traitsPacketList, receivedVersion); if (sentInstanceIt != sentIDValuePairs.end()) { sentInstanceIt->value = receivedVersion; @@ -161,7 +163,7 @@ void AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* liste } } else if (isDeleted && sentInstanceIt != sentIDValuePairs.end() && absoluteReceivedVersion > sentInstanceIt->value) { // this instance version was deleted and we haven't sent the delete to this client yet - AvatarTraits::packInstancedTraitDelete(traitType, instanceID, traitsPacketList, absoluteReceivedVersion); + bytesWritten += AvatarTraits::packInstancedTraitDelete(traitType, instanceID, traitsPacketList, absoluteReceivedVersion); // update the last sent version for this trait instance to the absolute value of the deleted version sentInstanceIt->value = absoluteReceivedVersion; @@ -172,12 +174,14 @@ void AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* liste } // write a null trait type to mark the end of trait data for this avatar - traitsPacketList.writePrimitive(AvatarTraits::NullTrait); + bytesWritten += traitsPacketList.writePrimitive(AvatarTraits::NullTrait); // since we send all traits for this other avatar, update the time of last traits sent // to match the time of last traits change listeningNodeData->setLastOtherAvatarTraitsSendPoint(otherNodeLocalID, timeOfLastTraitsChange); } + + return bytesWritten; } int AvatarMixerSlave::sendReplicatedIdentityPacket(const Node& agentNode, const AvatarMixerClientData* nodeData, const Node& destinationNode) { @@ -239,6 +243,7 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) // keep track of outbound data rate specifically for avatar data int numAvatarDataBytes = 0; int identityBytesSent = 0; + int traitBytesSent = 0; // max number of avatarBytes per frame auto maxAvatarBytesPerFrame = (_maxKbpsPerNode * BYTES_PER_KILOBIT) / AVATAR_MIXER_BROADCAST_FRAMES_PER_SECOND; @@ -550,7 +555,9 @@ void AvatarMixerSlave::broadcastAvatarDataToAgent(const SharedNodePointer& node) _stats.avatarDataPackingElapsedTime += (endAvatarDataPacking - startAvatarDataPacking); // use helper to add any changed traits to our packet list - addChangedTraitsToBulkPacket(nodeData, otherNodeData, *traitsPacketList); + traitBytesSent += addChangedTraitsToBulkPacket(nodeData, otherNodeData, *traitsPacketList); + + traitsPacketList->getDataSize(); } quint64 startPacketSending = usecTimestampNow(); diff --git a/assignment-client/src/avatars/AvatarMixerSlave.h b/assignment-client/src/avatars/AvatarMixerSlave.h index 5112faae29..bcb70f8743 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.h +++ b/assignment-client/src/avatars/AvatarMixerSlave.h @@ -104,9 +104,9 @@ private: int sendIdentityPacket(const AvatarMixerClientData* nodeData, const SharedNodePointer& destinationNode); int sendReplicatedIdentityPacket(const Node& agentNode, const AvatarMixerClientData* nodeData, const Node& destinationNode); - void addChangedTraitsToBulkPacket(AvatarMixerClientData* listeningNodeData, - const AvatarMixerClientData* sendingNodeData, - NLPacketList& traitsPacketList); + qint64 addChangedTraitsToBulkPacket(AvatarMixerClientData* listeningNodeData, + const AvatarMixerClientData* sendingNodeData, + NLPacketList& traitsPacketList); void broadcastAvatarDataToAgent(const SharedNodePointer& node); void broadcastAvatarDataToDownstreamMixer(const SharedNodePointer& node); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index f1b9986186..d63fde4cbe 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1828,35 +1828,40 @@ QUrl AvatarData::getWireSafeSkeletonModelURL() const { } } -void AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, +qint64 AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion) { - destination.writePrimitive(traitType); + qint64 bytesWritten = 0; + bytesWritten += destination.writePrimitive(traitType); if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) { AvatarTraits::TraitVersion typedVersion = traitVersion; - destination.writePrimitive(typedVersion); + bytesWritten += destination.writePrimitive(typedVersion); } if (traitType == AvatarTraits::SkeletonModelURL) { QByteArray encodedSkeletonURL = getWireSafeSkeletonModelURL().toEncoded(); AvatarTraits::TraitWireSize encodedURLSize = encodedSkeletonURL.size(); - destination.writePrimitive(encodedURLSize); + bytesWritten += destination.writePrimitive(encodedURLSize); - destination.write(encodedSkeletonURL); + bytesWritten += destination.write(encodedSkeletonURL); } + + return bytesWritten; } -void AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID traitInstanceID, +qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID traitInstanceID, ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion) { - destination.writePrimitive(traitType); + qint64 bytesWritten = 0; + + bytesWritten += destination.writePrimitive(traitType); if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) { AvatarTraits::TraitVersion typedVersion = traitVersion; - destination.writePrimitive(typedVersion); + bytesWritten += destination.writePrimitive(typedVersion); } - destination.write(traitInstanceID.toRfc4122()); + bytesWritten += destination.write(traitInstanceID.toRfc4122()); if (traitType == AvatarTraits::AvatarEntity) { // grab a read lock on the avatar entities and check for entity data for the given ID @@ -1873,12 +1878,14 @@ void AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTrai qDebug() << QJsonDocument::fromBinaryData(entityBinaryData).toJson(); - destination.writePrimitive(entityBinarySize); - destination.write(entityBinaryData); + bytesWritten += destination.writePrimitive(entityBinarySize); + bytesWritten += destination.write(entityBinaryData); } else { - destination.writePrimitive(AvatarTraits::DELETED_TRAIT_SIZE); + bytesWritten += destination.writePrimitive(AvatarTraits::DELETED_TRAIT_SIZE); } } + + return bytesWritten; } void AvatarData::processTrait(AvatarTraits::TraitType traitType, QByteArray traitBinaryData) { diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 6f1871974d..0f5e1d9ee4 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -958,10 +958,10 @@ public: // identityChanged returns true if identity has changed, false otherwise. Similarly for displayNameChanged and skeletonModelUrlChange. void processAvatarIdentity(const QByteArray& identityData, bool& identityChanged, bool& displayNameChanged); - void packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, - AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION); - void packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID, - ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION); + qint64 packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination, + AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION); + qint64 packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID, + ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION); void processTrait(AvatarTraits::TraitType traitType, QByteArray traitBinaryData); void processTraitInstance(AvatarTraits::TraitType traitType, diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h index acac215799..16e897319b 100644 --- a/libraries/avatars/src/AvatarTraits.h +++ b/libraries/avatars/src/AvatarTraits.h @@ -39,19 +39,22 @@ namespace AvatarTraits { using TraitWireSize = int16_t; const TraitWireSize DELETED_TRAIT_SIZE = -1; - inline void packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination, + inline qint64 packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination, TraitVersion traitVersion = NULL_TRAIT_VERSION) { - destination.writePrimitive(traitType); + qint64 bytesWritten = 0; + + bytesWritten += destination.writePrimitive(traitType); if (traitVersion > DEFAULT_TRAIT_VERSION) { AvatarTraits::TraitVersion typedVersion = traitVersion; - destination.writePrimitive(typedVersion); + bytesWritten += destination.writePrimitive(typedVersion); } - destination.write(instanceID.toRfc4122()); + bytesWritten += destination.write(instanceID.toRfc4122()); - destination.writePrimitive(DELETED_TRAIT_SIZE); + bytesWritten += destination.writePrimitive(DELETED_TRAIT_SIZE); + return bytesWritten; } }; From f0ba61ff0522b8aca88b3a9fc5014e4a219dbdea Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 20 Aug 2018 12:29:28 -0700 Subject: [PATCH 18/31] add missing local ID, reset client processed trait versions --- assignment-client/src/avatars/AvatarMixerClientData.cpp | 2 +- interface/src/avatar/AvatarManager.cpp | 1 - libraries/avatars/src/AvatarHashMap.cpp | 3 +++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index cc4356fb1a..5c84e3f755 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -19,7 +19,7 @@ #include "AvatarMixerSlave.h" AvatarMixerClientData::AvatarMixerClientData(const QUuid& nodeID, Node::LocalID nodeLocalID) : - NodeData(nodeID) + NodeData(nodeID, nodeLocalID) { // in case somebody calls getSessionUUID on the AvatarData instance, make sure it has the right ID _avatar->setID(nodeID); diff --git a/interface/src/avatar/AvatarManager.cpp b/interface/src/avatar/AvatarManager.cpp index 8569aaf05a..0d180bc40d 100644 --- a/interface/src/avatar/AvatarManager.cpp +++ b/interface/src/avatar/AvatarManager.cpp @@ -14,7 +14,6 @@ #include #include -#include #include "AvatarLogging.h" diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 0ab602e233..64b26131be 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -279,6 +279,9 @@ void AvatarHashMap::removeAvatar(const QUuid& sessionUUID, KillAvatarReason remo } void AvatarHashMap::handleRemovedAvatar(const AvatarSharedPointer& removedAvatar, KillAvatarReason removalReason) { + // remove any information about processed traits for this avatar + _processedTraitVersions.erase(removedAvatar->getID()); + qCDebug(avatars) << "Removed avatar with UUID" << uuidStringWithoutCurlyBraces(removedAvatar->getSessionUUID()) << "from AvatarHashMap" << removalReason; emit avatarRemovedEvent(removedAvatar->getSessionUUID()); From cd05d9335a8ea0d433e9ef0fdc3c365295fb25fc Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 20 Aug 2018 13:29:06 -0700 Subject: [PATCH 19/31] remove debug of entity json document --- libraries/avatars/src/AvatarData.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index d63fde4cbe..8677d31cb5 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1876,8 +1876,6 @@ qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTr if (!entityBinaryData.isNull()) { AvatarTraits::TraitWireSize entityBinarySize = entityBinaryData.size(); - qDebug() << QJsonDocument::fromBinaryData(entityBinaryData).toJson(); - bytesWritten += destination.writePrimitive(entityBinarySize); bytesWritten += destination.write(entityBinaryData); } else { From 7e127749f789ce882bc76455f38688ef85071a70 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 20 Aug 2018 14:27:11 -0700 Subject: [PATCH 20/31] move AssignmentDynamicFactory to entity-server only --- assignment-client/src/Agent.cpp | 4 ---- assignment-client/src/entities/EntityServer.cpp | 6 ++++++ 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/assignment-client/src/Agent.cpp b/assignment-client/src/Agent.cpp index a8466fa368..06a14927d3 100644 --- a/assignment-client/src/Agent.cpp +++ b/assignment-client/src/Agent.cpp @@ -52,7 +52,6 @@ #include #include // TODO: consider moving to scriptengine.h -#include "AssignmentDynamicFactory.h" #include "entities/AssignmentParentFinder.h" #include "RecordingScriptingInterface.h" #include "AbstractAudioInterface.h" @@ -72,9 +71,6 @@ Agent::Agent(ReceivedMessage& message) : DependencyManager::set(); DependencyManager::set(false); - DependencyManager::registerInheritance(); - DependencyManager::set(); - DependencyManager::set(); DependencyManager::set(); diff --git a/assignment-client/src/entities/EntityServer.cpp b/assignment-client/src/entities/EntityServer.cpp index 868e570e0c..089fb3e52f 100644 --- a/assignment-client/src/entities/EntityServer.cpp +++ b/assignment-client/src/entities/EntityServer.cpp @@ -24,6 +24,7 @@ #include #include +#include "../AssignmentDynamicFactory.h" #include "AssignmentParentFinder.h" #include "EntityNodeData.h" #include "EntityServerConsts.h" @@ -42,6 +43,9 @@ EntityServer::EntityServer(ReceivedMessage& message) : DependencyManager::set(); DependencyManager::set(); + DependencyManager::registerInheritance(); + DependencyManager::set(); + auto& packetReceiver = DependencyManager::get()->getPacketReceiver(); packetReceiver.registerListenerForTypes({ PacketType::EntityAdd, PacketType::EntityClone, @@ -71,6 +75,8 @@ EntityServer::~EntityServer() { void EntityServer::aboutToFinish() { DependencyManager::get()->cleanup(); + DependencyManager::destroy(); + OctreeServer::aboutToFinish(); } From eda84d281600a856f5b0ce5f75d3aea4253512a1 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 20 Aug 2018 14:29:42 -0700 Subject: [PATCH 21/31] flag deleted avatar entities when replacing full map --- libraries/avatars/src/AvatarData.cpp | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 8677d31cb5..5094327c9a 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2729,7 +2729,7 @@ void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFr } if (_clientTraitsHandler) { - // we have a client traits handler, so we need to mark this removed instance trait as changed + // we have a client traits handler, so we need to mark this removed instance trait as deleted // so that changes are sent next frame _clientTraitsHandler->markInstancedTraitDeleted(AvatarTraits::AvatarEntity, entityID); } @@ -2769,6 +2769,12 @@ void AvatarData::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { foreach (auto entityID, previousAvatarEntityIDs) { if (!_avatarEntityData.contains(entityID)) { _avatarEntityDetached.insert(entityID); + + if (_clientTraitsHandler) { + // we have a client traits handler, so we flag this removed entity as deleted + // so that changes are sent next frame + _clientTraitsHandler->markInstancedTraitDeleted(AvatarTraits::AvatarEntity, entityID); + } } } } From 25876bca6397ed1b9036ef5a444beee08e78c02f Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 20 Aug 2018 14:55:07 -0700 Subject: [PATCH 22/31] send attachment clears through setAvatarEntityData --- interface/src/AvatarBookmarks.cpp | 15 ++------------- interface/src/avatar/MyAvatar.cpp | 19 +++---------------- interface/src/avatar/MyAvatar.h | 2 +- 3 files changed, 6 insertions(+), 30 deletions(-) diff --git a/interface/src/AvatarBookmarks.cpp b/interface/src/AvatarBookmarks.cpp index 6afab71c90..c7c87aee3e 100644 --- a/interface/src/AvatarBookmarks.cpp +++ b/interface/src/AvatarBookmarks.cpp @@ -151,13 +151,7 @@ bool isWearableEntity(const EntityItemPointer& entity) { void AvatarBookmarks::updateAvatarEntities(const QVariantList &avatarEntities) { auto myAvatar = DependencyManager::get()->getMyAvatar(); - auto treeRenderer = DependencyManager::get(); - EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - myAvatar->removeAvatarEntities([&](const QUuid& entityID) { - auto entity = entityTree->findEntityByID(entityID); - return entity && isWearableEntity(entity); - }); - + myAvatar->clearAvatarEntities(); addAvatarEntities(avatarEntities); } @@ -180,12 +174,7 @@ void AvatarBookmarks::loadBookmark(const QString& bookmarkName) { if (!bookmark.empty()) { auto myAvatar = DependencyManager::get()->getMyAvatar(); - auto treeRenderer = DependencyManager::get(); - EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - myAvatar->removeAvatarEntities([&](const QUuid& entityID) { - auto entity = entityTree->findEntityByID(entityID); - return entity && isWearableEntity(entity); - }); + myAvatar->clearAvatarEntities(); const QString& avatarUrl = bookmark.value(ENTRY_AVATAR_URL, "").toString(); myAvatar->useFullAvatarURL(avatarUrl); qCDebug(interfaceapp) << "Avatar On " << avatarUrl; diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index deef69d980..709ddfd17a 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1703,21 +1703,6 @@ void MyAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { emit skeletonChanged(); } -void MyAvatar::removeAvatarEntities(const std::function& condition) { - auto treeRenderer = DependencyManager::get(); - EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; - if (entityTree) { - entityTree->withWriteLock([&] { - AvatarEntityMap avatarEntities = getAvatarEntityData(); - for (auto entityID : avatarEntities.keys()) { - if (!condition || condition(entityID)) { - entityTree->deleteEntity(entityID, true, true); - } - } - }); - } -} - QVariantList MyAvatar::getAvatarEntitiesVariant() { QVariantList avatarEntitiesData; QScriptEngine scriptEngine; @@ -2114,7 +2099,9 @@ void MyAvatar::setAttachmentData(const QVector& attachmentData) attachmentDataToEntityProperties(data, properties); newEntitiesProperties.push_back(properties); } - removeAvatarEntities(); + + clearAvatarEntities(); + for (auto& properties : newEntitiesProperties) { DependencyManager::get()->addEntity(properties, true); } diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index ba6348cc22..247a0c79c8 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -931,7 +931,7 @@ public: * @returns {object[]} */ Q_INVOKABLE QVariantList getAvatarEntitiesVariant(); - void removeAvatarEntities(const std::function& condition = {}); + void clearAvatarEntities() { setAvatarEntityData(AvatarEntityMap()); } /**jsdoc * @function MyAvatar.isFlying From 99f532a20e0d7119dfa2a0a68b103cf73eb088a1 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 20 Aug 2018 16:40:20 -0700 Subject: [PATCH 23/31] always check added avatar entities once in updateAvatarEntities --- libraries/avatars/src/AvatarData.cpp | 6 ++---- libraries/avatars/src/AvatarData.h | 2 +- libraries/entities/src/EntityEditPacketSender.cpp | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index 5094327c9a..c7cc32ee3e 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2692,7 +2692,7 @@ void AvatarData::setAttachmentsVariant(const QVariantList& variant) { const int MAX_NUM_AVATAR_ENTITIES = 42; -void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData, bool requiresTreeUpdate) { +void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) { _avatarEntitiesLock.withWriteLock([&] { AvatarEntityMap::iterator itr = _avatarEntityData.find(entityID); if (itr == _avatarEntityData.end()) { @@ -2704,9 +2704,7 @@ void AvatarData::updateAvatarEntity(const QUuid& entityID, const QByteArray& ent } }); - if (requiresTreeUpdate) { - _avatarEntityDataChanged = true; - } + _avatarEntityDataChanged = true; if (_clientTraitsHandler) { // we have a client traits handler, so we need to mark this instanced trait as changed diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h index 0f5e1d9ee4..53a2a69119 100644 --- a/libraries/avatars/src/AvatarData.h +++ b/libraries/avatars/src/AvatarData.h @@ -925,7 +925,7 @@ public: * @param {Uuid} entityID * @param {string} entityData */ - Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData, bool requiresTreeUpdate = true); + Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData); /**jsdoc * @function MyAvatar.clearAvatarEntity diff --git a/libraries/entities/src/EntityEditPacketSender.cpp b/libraries/entities/src/EntityEditPacketSender.cpp index d288126348..4aa66db227 100644 --- a/libraries/entities/src/EntityEditPacketSender.cpp +++ b/libraries/entities/src/EntityEditPacketSender.cpp @@ -74,7 +74,7 @@ void EntityEditPacketSender::queueEditAvatarEntityMessage(PacketType type, jsonProperties = QJsonDocument(jsonObject); QByteArray binaryProperties = jsonProperties.toBinaryData(); - _myAvatar->updateAvatarEntity(entityItemID, binaryProperties, false); + _myAvatar->updateAvatarEntity(entityItemID, binaryProperties); entity->setLastBroadcast(usecTimestampNow()); } From 4875738a05242ef57f9b3743b1d06d6ef6e09a49 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 20 Aug 2018 17:06:13 -0700 Subject: [PATCH 24/31] always add to detached, flag update on full replace --- .../src/avatars-renderer/Avatar.cpp | 8 +++---- libraries/avatars/src/AvatarData.cpp | 22 +++++++++++-------- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp index 150ecb6d44..1ec5ff595a 100644 --- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp +++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp @@ -355,21 +355,21 @@ void Avatar::updateAvatarEntities() { stateItr.value().success = success; } - AvatarEntityIDs recentlyDettachedAvatarEntities = getAndClearRecentlyDetachedIDs(); - if (!recentlyDettachedAvatarEntities.empty()) { + AvatarEntityIDs recentlyDetachedAvatarEntities = getAndClearRecentlyDetachedIDs(); + if (!recentlyDetachedAvatarEntities.empty()) { // only lock this thread when absolutely necessary AvatarEntityMap avatarEntityData; _avatarEntitiesLock.withReadLock([&] { avatarEntityData = _avatarEntityData; }); - foreach (auto entityID, recentlyDettachedAvatarEntities) { + foreach (auto entityID, recentlyDetachedAvatarEntities) { if (!avatarEntityData.contains(entityID)) { entityTree->deleteEntity(entityID, true, true); } } // remove stale data hashes - foreach (auto entityID, recentlyDettachedAvatarEntities) { + foreach (auto entityID, recentlyDetachedAvatarEntities) { MapOfAvatarEntityDataHashes::iterator stateItr = _avatarEntityDataHashes.find(entityID); if (stateItr != _avatarEntityDataHashes.end()) { _avatarEntityDataHashes.erase(stateItr); diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index c7cc32ee3e..f32f883a7e 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -2721,16 +2721,12 @@ void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFr removedEntity = _avatarEntityData.remove(entityID); }); - if (removedEntity) { - if (requiresRemovalFromTree) { - insertDetachedEntityID(entityID); - } + insertDetachedEntityID(entityID); - if (_clientTraitsHandler) { - // we have a client traits handler, so we need to mark this removed instance trait as deleted - // so that changes are sent next frame - _clientTraitsHandler->markInstancedTraitDeleted(AvatarTraits::AvatarEntity, entityID); - } + if (removedEntity && _clientTraitsHandler) { + // we have a client traits handler, so we need to mark this removed instance trait as deleted + // so that changes are sent next frame + _clientTraitsHandler->markInstancedTraitDeleted(AvatarTraits::AvatarEntity, entityID); } } @@ -2775,6 +2771,14 @@ void AvatarData::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) { } } } + + if (_clientTraitsHandler) { + // if we have a client traits handler, flag any updated or created entities + // so that we send changes for them next frame + foreach (auto entityID, _avatarEntityData) { + _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID); + } + } } }); } From 3e2d4dc69618468395a4a80241353956813aafa5 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Mon, 20 Aug 2018 17:47:32 -0700 Subject: [PATCH 25/31] fixes and improvments addressing CR comments --- .../src/avatars/AvatarMixerClientData.cpp | 15 +++++++------- .../src/avatars/AvatarMixerClientData.h | 6 +++--- .../src/avatars/AvatarMixerSlave.cpp | 20 ++++++++----------- interface/src/Application.cpp | 5 +---- libraries/avatars/src/AssociatedTraitValues.h | 2 +- libraries/avatars/src/AvatarData.cpp | 6 ++---- libraries/avatars/src/AvatarTraits.h | 8 ++++---- libraries/avatars/src/ClientTraitsHandler.cpp | 12 +++++------ libraries/networking/src/NodeData.cpp | 2 -- libraries/networking/src/NodeData.h | 2 +- libraries/networking/src/udt/BasePacket.cpp | 6 ------ libraries/networking/src/udt/BasePacket.h | 2 +- 12 files changed, 34 insertions(+), 52 deletions(-) diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 5c84e3f755..b2490fc7b4 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -49,7 +49,7 @@ void AvatarMixerClientData::queuePacket(QSharedPointer message, _packetQueue.push(message); } -int AvatarMixerClientData::processPackets(SlaveSharedData* slaveSharedData) { +int AvatarMixerClientData::processPackets(const SlaveSharedData& slaveSharedData) { int packetsProcessed = 0; SharedNodePointer node = _packetQueue.node; assert(_packetQueue.empty() || node); @@ -93,7 +93,8 @@ int AvatarMixerClientData::parseData(ReceivedMessage& message) { return _avatar->parseDataFromBuffer(message.readWithoutCopy(message.getBytesLeftToRead())); } -void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, SlaveSharedData* slaveSharedData, Node& sendingNode) { +void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, + const SlaveSharedData& slaveSharedData, Node& sendingNode) { // pull the trait version from the message AvatarTraits::TraitVersion packetTraitVersion; message.readPrimitive(&packetTraitVersion); @@ -155,9 +156,9 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, Sl } } -void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(SlaveSharedData *slaveSharedData, Node& sendingNode, +void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(const SlaveSharedData &slaveSharedData, Node& sendingNode, AvatarTraits::TraitVersion traitVersion) { - const auto& whitelist = slaveSharedData->skeletonURLWhitelist; + const auto& whitelist = slaveSharedData.skeletonURLWhitelist; if (!whitelist.isEmpty()) { bool inWhitelist = false; @@ -179,11 +180,11 @@ void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(SlaveSharedData *sl if (!inWhitelist) { // make sure we're not unecessarily overriding the default avatar with the default avatar - if (_avatar->getWireSafeSkeletonModelURL() != slaveSharedData->skeletonReplacementURL) { + if (_avatar->getWireSafeSkeletonModelURL() != slaveSharedData.skeletonReplacementURL) { // we need to change this avatar's skeleton URL, and send them a traits packet informing them of the change qDebug() << "Overwriting avatar URL" << _avatar->getWireSafeSkeletonModelURL() - << "to replacement" << slaveSharedData->skeletonReplacementURL << "for" << sendingNode.getUUID(); - _avatar->setSkeletonModelURL(slaveSharedData->skeletonReplacementURL); + << "to replacement" << slaveSharedData.skeletonReplacementURL << "for" << sendingNode.getUUID(); + _avatar->setSkeletonModelURL(slaveSharedData.skeletonReplacementURL); auto packet = NLPacket::create(PacketType::SetAvatarTraits, -1, true); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.h b/assignment-client/src/avatars/AvatarMixerClientData.h index 02b37e08f8..1c2694af48 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.h +++ b/assignment-client/src/avatars/AvatarMixerClientData.h @@ -116,10 +116,10 @@ public: QVector& getLastOtherAvatarSentJoints(QUuid otherAvatar) { return _lastOtherAvatarSentJoints[otherAvatar]; } void queuePacket(QSharedPointer message, SharedNodePointer node); - int processPackets(SlaveSharedData* slaveSharedData); // returns number of packets processed + int processPackets(const SlaveSharedData& slaveSharedData); // returns number of packets processed - void processSetTraitsMessage(ReceivedMessage& message, SlaveSharedData* slaveSharedData, Node& sendingNode); - void checkSkeletonURLAgainstWhitelist(SlaveSharedData* slaveSharedData, Node& sendingNode, + void processSetTraitsMessage(ReceivedMessage& message, const SlaveSharedData& slaveSharedData, Node& sendingNode); + void checkSkeletonURLAgainstWhitelist(const SlaveSharedData& slaveSharedData, Node& sendingNode, AvatarTraits::TraitVersion traitVersion); using TraitsCheckTimestamp = std::chrono::steady_clock::time_point; diff --git a/assignment-client/src/avatars/AvatarMixerSlave.cpp b/assignment-client/src/avatars/AvatarMixerSlave.cpp index 14818870d1..f347ff1f10 100644 --- a/assignment-client/src/avatars/AvatarMixerSlave.cpp +++ b/assignment-client/src/avatars/AvatarMixerSlave.cpp @@ -59,7 +59,7 @@ void AvatarMixerSlave::processIncomingPackets(const SharedNodePointer& node) { auto nodeData = dynamic_cast(node->getLinkedData()); if (nodeData) { _stats.nodesProcessed++; - _stats.packetsProcessed += nodeData->processPackets(_sharedData); + _stats.packetsProcessed += nodeData->processPackets(*_sharedData); } auto end = usecTimestampNow(); _stats.processIncomingPacketsElapsedTime += (end - start); @@ -109,19 +109,15 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis auto traitType = static_cast(std::distance(lastReceivedVersions.simpleCBegin(), simpleReceivedIt)); - // we need to double check that this is actually a simple trait type, since the instanced - // trait types are in the simple vector for access efficiency - if (AvatarTraits::isSimpleTrait(traitType)) { - auto lastReceivedVersion = *simpleReceivedIt; - auto& lastSentVersionRef = lastSentVersions[traitType]; + auto lastReceivedVersion = *simpleReceivedIt; + auto& lastSentVersionRef = lastSentVersions[traitType]; - if (lastReceivedVersions[traitType] > lastSentVersionRef) { - // there is an update to this trait, add it to the traits packet - bytesWritten += sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion); + if (lastReceivedVersions[traitType] > lastSentVersionRef) { + // there is an update to this trait, add it to the traits packet + bytesWritten += sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion); - // update the last sent version - lastSentVersionRef = lastReceivedVersion; - } + // update the last sent version + lastSentVersionRef = lastReceivedVersion; } ++simpleReceivedIt; diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp index 60af79bfda..2d76791a5e 100644 --- a/interface/src/Application.cpp +++ b/interface/src/Application.cpp @@ -5488,10 +5488,7 @@ void Application::update(float deltaTime) { // we haven't yet enabled physics. we wait until we think we have all the collision information // for nearby entities before starting bullet up. quint64 now = usecTimestampNow(); - // Check for flagged EntityData having arrived. - auto entityTreeRenderer = getEntities(); - if (isServerlessMode() || - (entityTreeRenderer && _octreeProcessor.octreeSequenceIsComplete(entityTreeRenderer->getLastOctreeMessageSequence()) )) { + if (isServerlessMode() || _octreeProcessor.isLoadSequenceComplete()) { // we've received a new full-scene octree stats packet, or it's been long enough to try again anyway _lastPhysicsCheckTime = now; _fullSceneCounterAtLastPhysicsCheck = _fullSceneReceivedCounter; diff --git a/libraries/avatars/src/AssociatedTraitValues.h b/libraries/avatars/src/AssociatedTraitValues.h index b2c0197e5c..e8bbce891d 100644 --- a/libraries/avatars/src/AssociatedTraitValues.h +++ b/libraries/avatars/src/AssociatedTraitValues.h @@ -18,7 +18,7 @@ namespace AvatarTraits { template class AssociatedTraitValues { public: - AssociatedTraitValues() : _simpleTypes(TotalTraitTypes, defaultValue) {} + AssociatedTraitValues() : _simpleTypes(FirstInstancedTrait, defaultValue) {} void insert(TraitType type, T value) { _simpleTypes[type] = value; } void erase(TraitType type) { _simpleTypes[type] = defaultValue; } diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp index f32f883a7e..c1ec19b307 100644 --- a/libraries/avatars/src/AvatarData.cpp +++ b/libraries/avatars/src/AvatarData.cpp @@ -1834,8 +1834,7 @@ qint64 AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice bytesWritten += destination.writePrimitive(traitType); if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) { - AvatarTraits::TraitVersion typedVersion = traitVersion; - bytesWritten += destination.writePrimitive(typedVersion); + bytesWritten += destination.writePrimitive(traitVersion); } if (traitType == AvatarTraits::SkeletonModelURL) { @@ -1857,8 +1856,7 @@ qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTr bytesWritten += destination.writePrimitive(traitType); if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) { - AvatarTraits::TraitVersion typedVersion = traitVersion; - bytesWritten += destination.writePrimitive(typedVersion); + bytesWritten += destination.writePrimitive(traitVersion); } bytesWritten += destination.write(traitInstanceID.toRfc4122()); diff --git a/libraries/avatars/src/AvatarTraits.h b/libraries/avatars/src/AvatarTraits.h index 16e897319b..f0c807a432 100644 --- a/libraries/avatars/src/AvatarTraits.h +++ b/libraries/avatars/src/AvatarTraits.h @@ -22,14 +22,15 @@ namespace AvatarTraits { enum TraitType : int8_t { NullTrait = -1, SkeletonModelURL, - AvatarEntity, + FirstInstancedTrait, + AvatarEntity = FirstInstancedTrait, TotalTraitTypes }; using TraitInstanceID = QUuid; inline bool isSimpleTrait(TraitType traitType) { - return traitType == SkeletonModelURL; + return traitType > NullTrait && traitType < FirstInstancedTrait; } using TraitVersion = int32_t; @@ -46,8 +47,7 @@ namespace AvatarTraits { bytesWritten += destination.writePrimitive(traitType); if (traitVersion > DEFAULT_TRAIT_VERSION) { - AvatarTraits::TraitVersion typedVersion = traitVersion; - bytesWritten += destination.writePrimitive(typedVersion); + bytesWritten += destination.writePrimitive(traitVersion); } bytesWritten += destination.write(instanceID.toRfc4122()); diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp index a31808a916..151b2a3809 100644 --- a/libraries/avatars/src/ClientTraitsHandler.cpp +++ b/libraries/avatars/src/ClientTraitsHandler.cpp @@ -68,14 +68,12 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() { // we double check that it is a simple iterator here auto traitType = static_cast(std::distance(traitStatusesCopy.simpleCBegin(), simpleIt)); - if (AvatarTraits::isSimpleTrait(traitType)) { - if (_shouldPerformInitialSend || *simpleIt == Updated) { - if (traitType == AvatarTraits::SkeletonModelURL) { - _owningAvatar->packTrait(traitType, *traitsPacketList); + if (_shouldPerformInitialSend || *simpleIt == Updated) { + if (traitType == AvatarTraits::SkeletonModelURL) { + _owningAvatar->packTrait(traitType, *traitsPacketList); - // keep track of our skeleton version in case we get an override back - _currentSkeletonVersion = _currentTraitVersion; - } + // keep track of our skeleton version in case we get an override back + _currentSkeletonVersion = _currentTraitVersion; } } diff --git a/libraries/networking/src/NodeData.cpp b/libraries/networking/src/NodeData.cpp index dd926852cd..d22b970154 100644 --- a/libraries/networking/src/NodeData.cpp +++ b/libraries/networking/src/NodeData.cpp @@ -18,5 +18,3 @@ NodeData::NodeData(const QUuid& nodeID, NetworkPeer::LocalID nodeLocalID) : { } - -NodeData::~NodeData() {} diff --git a/libraries/networking/src/NodeData.h b/libraries/networking/src/NodeData.h index ffbc0c2376..b4cb87d0c2 100644 --- a/libraries/networking/src/NodeData.h +++ b/libraries/networking/src/NodeData.h @@ -26,7 +26,7 @@ class NodeData : public QObject { Q_OBJECT public: NodeData(const QUuid& nodeID = QUuid(), NetworkPeer::LocalID localID = NetworkPeer::NULL_LOCAL_ID); - virtual ~NodeData() = 0; + virtual ~NodeData() = default; virtual int parseData(ReceivedMessage& message) { return 0; } const QUuid& getNodeID() const { return _nodeID; } diff --git a/libraries/networking/src/udt/BasePacket.cpp b/libraries/networking/src/udt/BasePacket.cpp index 92ccdd6117..12a174b7d3 100644 --- a/libraries/networking/src/udt/BasePacket.cpp +++ b/libraries/networking/src/udt/BasePacket.cpp @@ -77,12 +77,6 @@ BasePacket::BasePacket(std::unique_ptr data, qint64 size, const HifiSock } -BasePacket::BasePacket(const BasePacket& other) : - ExtendedIODevice() -{ - *this = other; -} - BasePacket& BasePacket::operator=(const BasePacket& other) { _packetSize = other._packetSize; _packet = std::unique_ptr(new char[_packetSize]); diff --git a/libraries/networking/src/udt/BasePacket.h b/libraries/networking/src/udt/BasePacket.h index 9c3244e08b..9054131337 100644 --- a/libraries/networking/src/udt/BasePacket.h +++ b/libraries/networking/src/udt/BasePacket.h @@ -88,7 +88,7 @@ public: protected: BasePacket(qint64 size); BasePacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr); - BasePacket(const BasePacket& other); + BasePacket(const BasePacket& other) { *this = other; } BasePacket& operator=(const BasePacket& other); BasePacket(BasePacket&& other); BasePacket& operator=(BasePacket&& other); From 25ed166f07631f2b6afdce3b2ed625b0cdb2f771 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Aug 2018 11:49:48 -0700 Subject: [PATCH 26/31] fix logic for initial send of deleted trait instance --- libraries/avatars/src/ClientTraitsHandler.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/libraries/avatars/src/ClientTraitsHandler.cpp b/libraries/avatars/src/ClientTraitsHandler.cpp index 151b2a3809..c4073cb86a 100644 --- a/libraries/avatars/src/ClientTraitsHandler.cpp +++ b/libraries/avatars/src/ClientTraitsHandler.cpp @@ -83,10 +83,12 @@ void ClientTraitsHandler::sendChangedTraitsToMixer() { auto instancedIt = traitStatusesCopy.instancedCBegin(); while (instancedIt != traitStatusesCopy.instancedCEnd()) { for (auto& instanceIDValuePair : instancedIt->instances) { - if (_shouldPerformInitialSend || instanceIDValuePair.value == Updated) { - // this is a changed trait we need to send, ask the owning avatar to pack it + if ((_shouldPerformInitialSend && instanceIDValuePair.value != Deleted) + || instanceIDValuePair.value == Updated) { + // this is a changed trait we need to send or we haven't send out trait information yet + // ask the owning avatar to pack it _owningAvatar->packTraitInstance(instancedIt->traitType, instanceIDValuePair.id, *traitsPacketList); - } else if (instanceIDValuePair.value == Deleted) { + } else if (!_shouldPerformInitialSend && instanceIDValuePair.value == Deleted) { // pack delete for this trait instance AvatarTraits::packInstancedTraitDelete(instancedIt->traitType, instanceIDValuePair.id, *traitsPacketList); From 8c58ae4e60b3628077cd72e70568254611e95eb7 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Aug 2018 11:56:28 -0700 Subject: [PATCH 27/31] fix insert of second trait instance --- libraries/avatars/src/AssociatedTraitValues.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/avatars/src/AssociatedTraitValues.h b/libraries/avatars/src/AssociatedTraitValues.h index e8bbce891d..fb780e252e 100644 --- a/libraries/avatars/src/AssociatedTraitValues.h +++ b/libraries/avatars/src/AssociatedTraitValues.h @@ -123,7 +123,7 @@ namespace AvatarTraits { auto it = instancesForTrait(traitType); if (it != _instancedTypes.end()) { - auto instancesVector = it->instances; + auto& instancesVector = it->instances; auto instanceIt = std::find_if(instancesVector.begin(), instancesVector.end(), [instanceID](InstanceIDValuePair& idValuePair){ return idValuePair.id == instanceID; From 8226ffb916a111bbc255bfdc6547502616a2a397 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Aug 2018 11:57:21 -0700 Subject: [PATCH 28/31] fix ref to instances vector for instance erase --- libraries/avatars/src/AssociatedTraitValues.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/avatars/src/AssociatedTraitValues.h b/libraries/avatars/src/AssociatedTraitValues.h index fb780e252e..cf1edef7f7 100644 --- a/libraries/avatars/src/AssociatedTraitValues.h +++ b/libraries/avatars/src/AssociatedTraitValues.h @@ -143,7 +143,7 @@ namespace AvatarTraits { auto it = instancesForTrait(traitType); if (it != _instancedTypes.end()) { - auto instancesVector = it->instances; + auto& instancesVector = it->instances; instancesVector.erase(std::remove_if(instancesVector.begin(), instancesVector.end(), [&instanceID](InstanceIDValuePair& idValuePair){ From a177e49877cb97bea92106841b67d90ea26be484 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Aug 2018 12:18:11 -0700 Subject: [PATCH 29/31] Revert "send attachment clears through setAvatarEntityData" This reverts commit 25876bca6397ed1b9036ef5a444beee08e78c02f. --- interface/src/AvatarBookmarks.cpp | 15 +++++++++++++-- interface/src/avatar/MyAvatar.cpp | 19 ++++++++++++++++--- interface/src/avatar/MyAvatar.h | 2 +- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/interface/src/AvatarBookmarks.cpp b/interface/src/AvatarBookmarks.cpp index c7c87aee3e..6afab71c90 100644 --- a/interface/src/AvatarBookmarks.cpp +++ b/interface/src/AvatarBookmarks.cpp @@ -151,7 +151,13 @@ bool isWearableEntity(const EntityItemPointer& entity) { void AvatarBookmarks::updateAvatarEntities(const QVariantList &avatarEntities) { auto myAvatar = DependencyManager::get()->getMyAvatar(); - myAvatar->clearAvatarEntities(); + auto treeRenderer = DependencyManager::get(); + EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; + myAvatar->removeAvatarEntities([&](const QUuid& entityID) { + auto entity = entityTree->findEntityByID(entityID); + return entity && isWearableEntity(entity); + }); + addAvatarEntities(avatarEntities); } @@ -174,7 +180,12 @@ void AvatarBookmarks::loadBookmark(const QString& bookmarkName) { if (!bookmark.empty()) { auto myAvatar = DependencyManager::get()->getMyAvatar(); - myAvatar->clearAvatarEntities(); + auto treeRenderer = DependencyManager::get(); + EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; + myAvatar->removeAvatarEntities([&](const QUuid& entityID) { + auto entity = entityTree->findEntityByID(entityID); + return entity && isWearableEntity(entity); + }); const QString& avatarUrl = bookmark.value(ENTRY_AVATAR_URL, "").toString(); myAvatar->useFullAvatarURL(avatarUrl); qCDebug(interfaceapp) << "Avatar On " << avatarUrl; diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index 709ddfd17a..deef69d980 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -1703,6 +1703,21 @@ void MyAvatar::setSkeletonModelURL(const QUrl& skeletonModelURL) { emit skeletonChanged(); } +void MyAvatar::removeAvatarEntities(const std::function& condition) { + auto treeRenderer = DependencyManager::get(); + EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr; + if (entityTree) { + entityTree->withWriteLock([&] { + AvatarEntityMap avatarEntities = getAvatarEntityData(); + for (auto entityID : avatarEntities.keys()) { + if (!condition || condition(entityID)) { + entityTree->deleteEntity(entityID, true, true); + } + } + }); + } +} + QVariantList MyAvatar::getAvatarEntitiesVariant() { QVariantList avatarEntitiesData; QScriptEngine scriptEngine; @@ -2099,9 +2114,7 @@ void MyAvatar::setAttachmentData(const QVector& attachmentData) attachmentDataToEntityProperties(data, properties); newEntitiesProperties.push_back(properties); } - - clearAvatarEntities(); - + removeAvatarEntities(); for (auto& properties : newEntitiesProperties) { DependencyManager::get()->addEntity(properties, true); } diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h index 247a0c79c8..ba6348cc22 100644 --- a/interface/src/avatar/MyAvatar.h +++ b/interface/src/avatar/MyAvatar.h @@ -931,7 +931,7 @@ public: * @returns {object[]} */ Q_INVOKABLE QVariantList getAvatarEntitiesVariant(); - void clearAvatarEntities() { setAvatarEntityData(AvatarEntityMap()); } + void removeAvatarEntities(const std::function& condition = {}); /**jsdoc * @function MyAvatar.isFlying From 1723f3d3d88b94e4de62d62e960437220cf670b9 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Aug 2018 12:21:29 -0700 Subject: [PATCH 30/31] don't use entity tree to clear all avatar entities --- interface/src/avatar/MyAvatar.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp index deef69d980..b30f05225e 100755 --- a/interface/src/avatar/MyAvatar.cpp +++ b/interface/src/avatar/MyAvatar.cpp @@ -2114,7 +2114,10 @@ void MyAvatar::setAttachmentData(const QVector& attachmentData) attachmentDataToEntityProperties(data, properties); newEntitiesProperties.push_back(properties); } - removeAvatarEntities(); + + // clear any existing avatar entities + setAvatarEntityData(AvatarEntityMap()); + for (auto& properties : newEntitiesProperties) { DependencyManager::get()->addEntity(properties, true); } From 7d8db482a3b551aef6b9dbcea9b8de90c41774ab Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Tue, 21 Aug 2018 13:21:21 -0700 Subject: [PATCH 31/31] put back ExtendedIODevice ctor for ubuntu warning --- libraries/networking/src/udt/BasePacket.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libraries/networking/src/udt/BasePacket.h b/libraries/networking/src/udt/BasePacket.h index 9054131337..4981cb4720 100644 --- a/libraries/networking/src/udt/BasePacket.h +++ b/libraries/networking/src/udt/BasePacket.h @@ -88,7 +88,7 @@ public: protected: BasePacket(qint64 size); BasePacket(std::unique_ptr data, qint64 size, const HifiSockAddr& senderSockAddr); - BasePacket(const BasePacket& other) { *this = other; } + BasePacket(const BasePacket& other) : ExtendedIODevice() { *this = other; } BasePacket& operator=(const BasePacket& other); BasePacket(BasePacket&& other); BasePacket& operator=(BasePacket&& other);