add instanced traits and migrate avatar entities

assignment-client/src/avatars/AvatarMixerClientData.cpp

@@ -19,8 +19,7 @@
 #include "AvatarMixerSlave.h"
 
 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);
@@ -107,21 +106,47 @@ void AvatarMixerClientData::processSetTraitsMessage(ReceivedMessage& message, Sl
         AvatarTraits::TraitType traitType;
         message.readPrimitive(&traitType);
 
-        AvatarTraits::TraitWireSize traitSize;
+        if (AvatarTraits::isSimpleTrait(traitType)) {
-        message.readPrimitive(&traitSize);
+            AvatarTraits::TraitWireSize traitSize;
+            message.readPrimitive(&traitSize);
 
-        if (packetTraitVersion > _receivedSimpleTraitVersions[traitType]) {
+            if (packetTraitVersion > _lastReceivedTraitVersions[traitType]) {
-            _avatar->processTrait(traitType, message.readWithoutCopy(traitSize));
+                _avatar->processTrait(traitType, message.read(traitSize));
-            _receivedSimpleTraitVersions[traitType] = packetTraitVersion;
+                _lastReceivedTraitVersions[traitType] = packetTraitVersion;
 
-            if (traitType == AvatarTraits::SkeletonModelURL) {
+                if (traitType == AvatarTraits::SkeletonModelURL) {
-                // special handling for skeleton model URL, since we need to make sure it is in the whitelist
+                    // special handling for skeleton model URL, since we need to make sure it is in the whitelist
-                checkSkeletonURLAgainstWhitelist(slaveSharedData, sendingNode, packetTraitVersion);
+                    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;
-}

assignment-client/src/avatars/AvatarMixerClientData.h

@@ -22,7 +22,7 @@
 #include <QtCore/QUrl>
 
 #include <AvatarData.h>
-#include <AvatarTraits.h>
+#include <AssociatedTraitValues.h>
 #include <NodeData.h>
 #include <NumericalConstants.h>
 #include <udt/PacketHeaders.h>
@@ -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;
+    AvatarTraits::TraitVersions& getLastSentTraitVersions(Node::LocalID otherAvatar) { return _sentTraitVersions[otherAvatar]; }
-    void setLastSentSimpleTraitVersion(Node::LocalID otherAvatar, AvatarTraits::TraitType traitType,
-                                       AvatarTraits::TraitVersion traitVersion);
 
 private:
     struct PacketQueue : public std::queue<QSharedPointer<ReceivedMessage>> {
@@ -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<Node::LocalID, TraitsCheckTimestamp> _lastSentTraitsTimestamps;
-    std::unordered_map<Node::LocalID, AvatarTraits::SimpleTraitVersions> _sentSimpleTraitVersions;
+    std::unordered_map<Node::LocalID, AvatarTraits::TraitVersions> _sentTraitVersions;
 };
 
 #endif // hifi_AvatarMixerClientData_h

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) {
+        auto& lastSentVersions = listeningNodeData->getLastSentTraitVersions(otherNodeLocalID);
-            AvatarTraits::TraitType traitType = static_cast<AvatarTraits::TraitType>(i);
+        const auto& lastReceivedVersions = sendingNodeData->getLastReceivedTraitVersions();
 
-            auto lastSentVersion = listeningNodeData->getLastSentSimpleTraitVersion(otherNodeLocalID, traitType);
+        auto simpleReceivedIt = lastReceivedVersions.simpleCBegin();
-            auto lastReceivedVersion = sendingNodeData->getLastReceivedSimpleTraitVersion(traitType);
+        while (simpleReceivedIt != lastReceivedVersions.simpleCEnd()) {
+            auto traitType = static_cast<AvatarTraits::TraitType>(std::distance(lastReceivedVersions.simpleCBegin(),
+                                                                                simpleReceivedIt));
 
-            if (lastReceivedVersion > lastSentVersion) {
+            // we need to double check that this is actually a simple trait type, since the instanced
-                // there is an update to this trait, add it to the traits packet
+            // 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
+                if (lastReceivedVersions[traitType] > lastSentVersionRef) {
-                listeningNodeData->setLastSentSimpleTraitVersion(otherNodeLocalID, traitType, lastReceivedVersion);
+                    // 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

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<typename T, T defaultValue>
+    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<InstanceIDValuePair>;
+
+        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<T>::const_iterator simpleCBegin() const { return _simpleTypes.cbegin(); }
+        typename std::vector<T>::const_iterator simpleCEnd() const { return _simpleTypes.cend(); }
+
+        typename std::vector<T>::iterator simpleBegin() { return _simpleTypes.begin(); }
+        typename std::vector<T>::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<TraitWithInstances>::const_iterator instancedCBegin() const { return _instancedTypes.cbegin(); }
+        typename std::vector<TraitWithInstances>::const_iterator instancedCEnd() const { return _instancedTypes.cend(); }
+
+        typename std::vector<TraitWithInstances>::iterator instancedBegin() { return _instancedTypes.begin(); }
+        typename std::vector<TraitWithInstances>::iterator instancedEnd() { return _instancedTypes.end(); }
+
+    private:
+        std::vector<T> _simpleTypes;
+
+        typename std::vector<TraitWithInstances>::iterator instancesForTrait(TraitType traitType) {
+            return std::find_if(_instancedTypes.begin(), _instancedTypes.end(),
+                                [traitType](TraitWithInstances& traitWithInstances){
+                                    return traitWithInstances.traitType == traitType;
+                                });
+        }
+
+        std::vector<TraitWithInstances> _instancedTypes;
+    };
+
+    template <typename T, T defaultValue>
+    inline typename AssociatedTraitValues<T, defaultValue>::InstanceIDValuePairs&
+    AssociatedTraitValues<T, defaultValue>::getInstanceIDValuePairs(TraitType traitType) {
+        auto it = instancesForTrait(traitType);
+
+        if (it != _instancedTypes.end()) {
+            return it->instances;
+        } else {
+            _instancedTypes.emplace_back(traitType);
+            return _instancedTypes.back().instances;
+        }
+    }
+
+    template <typename T, T defaultValue>
+    inline T& AssociatedTraitValues<T, defaultValue>::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 <typename T, T defaultValue>
+    inline void AssociatedTraitValues<T, defaultValue>::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 <typename T, T defaultValue>
+    inline void AssociatedTraitValues<T, defaultValue>::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<TraitVersion, DEFAULT_TRAIT_VERSION>;
+};
+
+#endif // hifi_AssociatedTraitValues_h

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;
+        QByteArray encodedSkeletonURL = getWireSafeSkeletonModelURL().toEncoded();
-        if (_skeletonModelURL.scheme() != "file" && _skeletonModelURL.scheme() != "qrc") {
-            encodedSkeletonURL = _skeletonModelURL.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()
-        identityStream << getSessionUUID()
+        << (udt::SequenceNumber::Type) _identitySequenceNumber
-            << (udt::SequenceNumber::Type) _identitySequenceNumber
+        << _attachmentData
-            << _attachmentData
+        << _displayName
-            << _displayName
+        << getSessionDisplayNameForTransport() // depends on _sessionDisplayName
-            << getSessionDisplayNameForTransport() // depends on _sessionDisplayName
+        << (_isReplicated || setIsReplicated)
-            << (_isReplicated || setIsReplicated)
+        << _lookAtSnappingEnabled;
-            << _avatarEntityData
-            << _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 {

libraries/avatars/src/AvatarData.h

@@ -947,12 +947,10 @@ public:
     const HeadData* getHeadData() const { return _headData; }
 
     struct Identity {
-        QUrl skeletonModelURL;
         QVector<AttachmentData> 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.

libraries/avatars/src/AvatarHashMap.cpp

@@ -194,26 +194,6 @@ void AvatarHashMap::processAvatarIdentityPacket(QSharedPointer<ReceivedMessage>
     }
 }
 
-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<ReceivedMessage> 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<ReceivedMessage> 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;
+            AvatarTraits::TraitVersion packetTraitVersion;
-            message->readPrimitive(&traitVersion);
+            message->readPrimitive(&packetTraitVersion);
 
             AvatarTraits::TraitWireSize traitBinarySize;
-            message->readPrimitive(&traitBinarySize);
+            bool skipBinaryTrait = false;
 
-            if (avatar) {
+            if (!avatar) {
-                // check if this trait version is newer than what we already have for this avatar
+                skipBinaryTrait = true;
-                bool traitIsNewer = checkLastProcessedTraitVersion(avatarID, traitType, traitVersion);
+            }
-                if (traitIsNewer) {
+
-                    avatar->processTrait(traitType, message->readWithoutCopy(traitBinarySize));
+            if (AvatarTraits::isSimpleTrait(traitType)) {
-                } else {
+                message->readPrimitive(&traitBinarySize);
-                    message->seek(message->getPosition() + 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
+                AvatarTraits::TraitInstanceID traitInstanceID =
-                // traits for avatars we do have later in the packet
+                    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);
             }
 

libraries/avatars/src/AvatarHashMap.h

@@ -30,7 +30,7 @@
 #include "ScriptAvatarData.h"
 
 #include "AvatarData.h"
-#include "AvatarTraits.h"
+#include "AssociatedTraitValues.h"
 
 /**jsdoc
  * <strong>Note:</strong> An <code>AvatarList</code> 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<QUuid, AvatarTraits::SimpleTraitVersions> _processedSimpleTraitVersions;
+    std::unordered_map<QUuid, AvatarTraits::TraitVersions> _processedTraitVersions;
 private:
     QUuid _lastOwnerSessionUUID;
 };

libraries/avatars/src/AvatarTraits.h

@@ -16,46 +16,43 @@
 #include <cstdint>
 #include <vector>
 
+#include <QtCore/QUuid>
+
 namespace AvatarTraits {
     enum TraitType : int8_t {
         NullTrait = -1,
         SkeletonModelURL,
+        AvatarEntity,
         TotalTraitTypes
     };
 
-    class TraitTypeSet {
+    using TraitInstanceID = QUuid;
-    public:
-        TraitTypeSet() {};
-        
-        TraitTypeSet(std::initializer_list<TraitType> types) {
-            for (auto type : types) {
-                _types[type] = true;
-            }
-        };
 
-        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(); }
+    using TraitVersion = int32_t;
-        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<bool> _types = { AvatarTraits::TotalTraitTypes, false };
-    };
-
-    const TraitTypeSet SimpleTraitTypes = { SkeletonModelURL };
-
-    using TraitVersion = uint32_t;
     const TraitVersion DEFAULT_TRAIT_VERSION = 0;
+    const TraitVersion NULL_TRAIT_VERSION = -1;
 
-    using NullableTraitVersion = int64_t;
+    using TraitWireSize = int16_t;
-    const NullableTraitVersion NULL_TRAIT_VERSION = -1;
+    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<TraitVersion>;
+        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

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<NodeList>();
@@ -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;
+        traitsPacketList->writePrimitive(++_currentTraitVersion);
-        QByteArray extendedHeader(reinterpret_cast<char*>(&_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 };
+        auto traitStatusesCopy { _traitStatuses };
-        _changedTraits.clear();
+        _traitStatuses.reset();
+        _hasChangedTraits = false;
 
-        if (_performInitialSend || changedTraitsCopy.contains(AvatarTraits::SkeletonModelURL)) {
+        auto simpleIt = traitStatusesCopy.simpleCBegin();
-            traitsPacketList->startSegment();
+        while (simpleIt != traitStatusesCopy.simpleCEnd()) {
-            _owningAvatar->packTrait(AvatarTraits::SkeletonModelURL, *traitsPacketList);
+            // because the vector contains all trait types (for access using trait type as index)
-            traitsPacketList->endSegment();
+            // we double check that it is a simple iterator here
+            auto traitType = static_cast<AvatarTraits::TraitType>(std::distance(traitStatusesCopy.simpleCBegin(), simpleIt));
 
-            // keep track of our skeleton version in case we get an override back
+            if (AvatarTraits::isSimpleTrait(traitType)) {
-            _currentSkeletonVersion = _currentTraitVersion;
+                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<ReceivedMessage> 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);
             }

libraries/avatars/src/ClientTraitsHandler.h

@@ -14,7 +14,7 @@
 
 #include <ReceivedMessage.h>
 
-#include "AvatarTraits.h"
+#include "AssociatedTraitValues.h"
 #include "Node.h"
 
 class AvatarData;
@@ -26,10 +26,14 @@ public:
 
     void sendChangedTraitsToMixer();
 
-    bool hasChangedTraits() { return _changedTraits.hasAny(); }
+    bool hasChangedTraits() { return _hasChangedTraits; }
-    void markTraitChanged(AvatarTraits::TraitType changedTrait) { _changedTraits.insert(changedTrait); }
 
-    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<ReceivedMessage> message, SharedNodePointer sendingNode);
 
 private:
+    enum ClientTraitStatus {
+        Unchanged,
+        Updated,
+        Deleted
+    };
+
     AvatarData* _owningAvatar;
 
-    AvatarTraits::TraitTypeSet _changedTraits;
+    AvatarTraits::AssociatedTraitValues<ClientTraitStatus, Unchanged> _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

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<PacketVersion>(AvatarMixerPacketVersion::MigrateSkeletonURLToTraits);
+            return static_cast<PacketVersion>(AvatarMixerPacketVersion::MigrateAvatarEntitiesToTraits);
         case PacketType::MessagesData:
             return static_cast<PacketVersion>(MessageDataVersion::TextOrBinaryData);
         // ICE packets

libraries/networking/src/udt/PacketHeaders.h

@@ -290,8 +290,9 @@ enum class AvatarMixerPacketVersion : PacketVersion {
     FBXReaderNodeReparenting,
     FixMannequinDefaultAvatarFeet,
     ProceduralFaceMovementFlagsAndBlendshapes,
-    FarGrabJoints
+    FarGrabJoints,
-    MigrateSkeletonURLToTraits
+    MigrateSkeletonURLToTraits,
+    MigrateAvatarEntitiesToTraits
 };
 
 enum class DomainConnectRequestVersion : PacketVersion {