Merge pull request #15235 from Atlante45/fix/traits

Case 21108: Make traits easier to extend

assignment-client/src/avatars/AvatarMixerClientData.cpp

@@ -341,7 +341,7 @@ void AvatarMixerClientData::checkSkeletonURLAgainstWhitelist(const SlaveSharedDa
 
                 // 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);
+                AvatarTraits::packVersionedTrait(AvatarTraits::SkeletonModelURL, *packet, traitVersion, *_avatar);
 
                 auto nodeList = DependencyManager::get<NodeList>();
                 nodeList->sendPacket(std::move(packet), sendingNode);

assignment-client/src/avatars/AvatarMixerSlave.cpp

@@ -139,7 +139,8 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis
                 if (lastReceivedVersion > lastSentVersionRef) {
                     bytesWritten += addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten);
                     // there is an update to this trait, add it to the traits packet
-                    bytesWritten += sendingAvatar->packTrait(traitType, traitsPacketList, lastReceivedVersion);
+                    bytesWritten += AvatarTraits::packVersionedTrait(traitType, traitsPacketList,
+                                                                     lastReceivedVersion, *sendingAvatar);
                     // update the last sent version
                     lastSentVersionRef = lastReceivedVersion;
                     // Remember which versions we sent in this particular packet
@@ -194,7 +195,8 @@ qint64 AvatarMixerSlave::addChangedTraitsToBulkPacket(AvatarMixerClientData* lis
                     bytesWritten += addTraitsNodeHeader(listeningNodeData, sendingNodeData, traitsPacketList, bytesWritten);
 
                     // this instance version exists and has never been sent or is newer so we need to send it
-                    bytesWritten += sendingAvatar->packTraitInstance(traitType, instanceID, traitsPacketList, receivedVersion);
+                    bytesWritten += AvatarTraits::packVersionedTraitInstance(traitType, instanceID, traitsPacketList,
+                                                                             receivedVersion, *sendingAvatar);
 
                     if (sentInstanceIt != sentIDValuePairs.end()) {
                         sentInstanceIt->value = receivedVersion;

libraries/avatars/src/AssociatedTraitValues.h

@@ -28,9 +28,10 @@
 namespace AvatarTraits {
     template<typename T, T defaultValue>
     class AssociatedTraitValues {
+        using SimpleTypesArray = std::array<T, NUM_SIMPLE_TRAITS>;
     public:
         // constructor that pre-fills _simpleTypes with the default value specified by the template
-        AssociatedTraitValues() : _simpleTypes(FirstInstancedTrait, defaultValue) {}
+        AssociatedTraitValues() { std::fill(_simpleTypes.begin(), _simpleTypes.end(), defaultValue); }
 
         /// inserts the given value for the given simple trait type
         void insert(TraitType type, T value) { _simpleTypes[type] = value; }
@@ -71,12 +72,12 @@ namespace AvatarTraits {
         }
 
         /// const iterators for the vector of simple type values
-        typename std::vector<T>::const_iterator simpleCBegin() const { return _simpleTypes.cbegin(); }
+        typename SimpleTypesArray::const_iterator simpleCBegin() const { return _simpleTypes.cbegin(); }
-        typename std::vector<T>::const_iterator simpleCEnd() const { return _simpleTypes.cend(); }
+        typename SimpleTypesArray::const_iterator simpleCEnd() const { return _simpleTypes.cend(); }
 
         /// non-const iterators for the vector of simple type values
-        typename std::vector<T>::iterator simpleBegin() { return _simpleTypes.begin(); }
+        typename SimpleTypesArray::iterator simpleBegin() { return _simpleTypes.begin(); }
-        typename std::vector<T>::iterator simpleEnd() { return _simpleTypes.end(); }
+        typename SimpleTypesArray::iterator simpleEnd() { return _simpleTypes.end(); }
 
         struct TraitWithInstances {
             TraitType traitType;
@@ -96,7 +97,7 @@ namespace AvatarTraits {
         typename std::vector<TraitWithInstances>::iterator instancedEnd() { return _instancedTypes.end(); }
 
     private:
-        std::vector<T> _simpleTypes;
+        SimpleTypesArray _simpleTypes;
 
         /// return the iterator to the matching TraitWithInstances object for a given instanced trait type
         typename std::vector<TraitWithInstances>::iterator instancesForTrait(TraitType traitType) {

libraries/avatars/src/AvatarData.cpp

@@ -1990,42 +1990,16 @@ QUrl AvatarData::getWireSafeSkeletonModelURL() const {
     }
 }
 
-qint64 AvatarData::packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination,
+QByteArray AvatarData::packSkeletonModelURL() const {
-                           AvatarTraits::TraitVersion traitVersion) {
+    return getWireSafeSkeletonModelURL().toEncoded();
-
-    qint64 bytesWritten = 0;
-
-    if (traitType == AvatarTraits::SkeletonModelURL) {
-
-        QByteArray encodedSkeletonURL = getWireSafeSkeletonModelURL().toEncoded();
-
-        if (encodedSkeletonURL.size() > AvatarTraits::MAXIMUM_TRAIT_SIZE) {
-            qWarning() << "Refusing to pack simple trait" << traitType << "of size" << encodedSkeletonURL.size()
-                << "bytes since it exceeds the maximum size" << AvatarTraits::MAXIMUM_TRAIT_SIZE << "bytes";
-            return 0;
-        }
-
-        bytesWritten += destination.writePrimitive(traitType);
-
-        if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) {
-            bytesWritten += destination.writePrimitive(traitVersion);
-        }
-        
-        AvatarTraits::TraitWireSize encodedURLSize = encodedSkeletonURL.size();
-        bytesWritten += destination.writePrimitive(encodedURLSize);
-
-        bytesWritten += destination.write(encodedSkeletonURL);
-    }
-
-    return bytesWritten;
 }
 
+void AvatarData::unpackSkeletonModelURL(const QByteArray& data) {
+    auto skeletonModelURL = QUrl::fromEncoded(data);
+    setSkeletonModelURL(skeletonModelURL);
+}
 
-qint64 AvatarData::packAvatarEntityTraitInstance(AvatarTraits::TraitType traitType,
+QByteArray AvatarData::packAvatarEntityTraitInstance(AvatarTraits::TraitInstanceID traitInstanceID) {
-                                                 AvatarTraits::TraitInstanceID traitInstanceID,
-                                                 ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion) {
-    qint64 bytesWritten = 0;
-
     // grab a read lock on the avatar entities and check for entity data for the given ID
     QByteArray entityBinaryData;
     _avatarEntitiesLock.withReadLock([this, &entityBinaryData, &traitInstanceID] {
@@ -2034,104 +2008,48 @@ qint64 AvatarData::packAvatarEntityTraitInstance(AvatarTraits::TraitType traitTy
         }
     });
 
-    if (entityBinaryData.size() > AvatarTraits::MAXIMUM_TRAIT_SIZE) {
+    return entityBinaryData;
-        qWarning() << "Refusing to pack instanced trait" << traitType << "of size" << entityBinaryData.size()
-                   << "bytes since it exceeds the maximum size " << AvatarTraits::MAXIMUM_TRAIT_SIZE << "bytes";
-        return 0;
-    }
-
-    bytesWritten += destination.writePrimitive(traitType);
-
-    if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) {
-        bytesWritten += destination.writePrimitive(traitVersion);
-    }
-
-    bytesWritten += destination.write(traitInstanceID.toRfc4122());
-
-    if (!entityBinaryData.isNull()) {
-        AvatarTraits::TraitWireSize entityBinarySize = entityBinaryData.size();
-
-        bytesWritten += destination.writePrimitive(entityBinarySize);
-        bytesWritten += destination.write(entityBinaryData);
-    } else {
-        bytesWritten += destination.writePrimitive(AvatarTraits::DELETED_TRAIT_SIZE);
-    }
-
-    return bytesWritten;
 }
 
-
+QByteArray AvatarData::packGrabTraitInstance(AvatarTraits::TraitInstanceID traitInstanceID) {
-qint64 AvatarData::packGrabTraitInstance(AvatarTraits::TraitType traitType,
-                                         AvatarTraits::TraitInstanceID traitInstanceID,
-                                         ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion) {
-    qint64 bytesWritten = 0;
-
     // grab a read lock on the avatar grabs and check for grab data for the given ID
     QByteArray grabBinaryData;
-
     _avatarGrabsLock.withReadLock([this, &grabBinaryData, &traitInstanceID] {
         if (_avatarGrabData.contains(traitInstanceID)) {
             grabBinaryData = _avatarGrabData[traitInstanceID];
         }
     });
 
-    if (grabBinaryData.size() > AvatarTraits::MAXIMUM_TRAIT_SIZE) {
+    return grabBinaryData;
-        qWarning() << "Refusing to pack instanced trait" << traitType << "of size" << grabBinaryData.size()
-                   << "bytes since it exceeds the maximum size " << AvatarTraits::MAXIMUM_TRAIT_SIZE << "bytes";
-        return 0;
-    }
-
-    bytesWritten += destination.writePrimitive(traitType);
-
-    if (traitVersion > AvatarTraits::DEFAULT_TRAIT_VERSION) {
-        bytesWritten += destination.writePrimitive(traitVersion);
-    }
-
-    bytesWritten += destination.write(traitInstanceID.toRfc4122());
-
-    if (!grabBinaryData.isNull()) {
-        AvatarTraits::TraitWireSize grabBinarySize = grabBinaryData.size();
-
-        bytesWritten += destination.writePrimitive(grabBinarySize);
-        bytesWritten += destination.write(grabBinaryData);
-    } else {
-        bytesWritten += destination.writePrimitive(AvatarTraits::DELETED_TRAIT_SIZE);
-    }
-
-    return bytesWritten;
 }
 
-qint64 AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID traitInstanceID,
+QByteArray AvatarData::packTrait(AvatarTraits::TraitType traitType) const {
-                                   ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion) {
+    QByteArray traitBinaryData;
-    qint64 bytesWritten = 0;
 
+    // Call packer function
+    if (traitType == AvatarTraits::SkeletonModelURL) {
+        traitBinaryData = packSkeletonModelURL();
+    }
+
+    return traitBinaryData;
+}
+
+QByteArray AvatarData::packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID traitInstanceID) {
+    QByteArray traitBinaryData;
+
+    // Call packer function
     if (traitType == AvatarTraits::AvatarEntity) {
-        bytesWritten += packAvatarEntityTraitInstance(traitType, traitInstanceID, destination, traitVersion);
+        traitBinaryData = packAvatarEntityTraitInstance(traitInstanceID);
     } else if (traitType == AvatarTraits::Grab) {
-        bytesWritten += packGrabTraitInstance(traitType, traitInstanceID, destination, traitVersion);
+        traitBinaryData = packGrabTraitInstance(traitInstanceID);
     }
 
-    return bytesWritten;
+    return traitBinaryData;
-}
-
-void AvatarData::prepareResetTraitInstances() {
-    if (_clientTraitsHandler) {
-        _avatarEntitiesLock.withReadLock([this]{
-            foreach (auto entityID, _packedAvatarEntityData.keys()) {
-                _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID);
-            }
-            foreach (auto grabID, _avatarGrabData.keys()) {
-                _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::Grab, grabID);
-            }
-        });
-    }
 }
 
 void AvatarData::processTrait(AvatarTraits::TraitType traitType, QByteArray traitBinaryData) {
     if (traitType == AvatarTraits::SkeletonModelURL) {
-        // get the URL from the binary data
+        unpackSkeletonModelURL(traitBinaryData);
-        auto skeletonModelURL = QUrl::fromEncoded(traitBinaryData);
-        setSkeletonModelURL(skeletonModelURL);
     }
 }
 
@@ -2152,6 +2070,19 @@ void AvatarData::processDeletedTraitInstance(AvatarTraits::TraitType traitType,
     }
 }
 
+void AvatarData::prepareResetTraitInstances() {
+    if (_clientTraitsHandler) {
+        _avatarEntitiesLock.withReadLock([this]{
+            foreach (auto entityID, _packedAvatarEntityData.keys()) {
+                _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID);
+            }
+            foreach (auto grabID, _avatarGrabData.keys()) {
+                _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::Grab, grabID);
+            }
+        });
+    }
+}
+
 QByteArray AvatarData::identityByteArray(bool setIsReplicated) const {
     QByteArray identityData;
     QDataStream identityStream(&identityData, QIODevice::Append);

libraries/avatars/src/AvatarData.h

@@ -1134,18 +1134,16 @@ public:
     // identityChanged returns true if identity has changed, false otherwise. Similarly for displayNameChanged and skeletonModelUrlChange.
     void processAvatarIdentity(QDataStream& packetStream, bool& identityChanged, bool& displayNameChanged);
 
-    qint64 packTrait(AvatarTraits::TraitType traitType, ExtendedIODevice& destination,
+    QByteArray packTrait(AvatarTraits::TraitType traitType) const;
-                     AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION);
+    QByteArray packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID);
-    qint64 packTraitInstance(AvatarTraits::TraitType traitType, AvatarTraits::TraitInstanceID instanceID,
-                             ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion = AvatarTraits::NULL_TRAIT_VERSION);
-
-    void prepareResetTraitInstances();
 
     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);
 
+    void prepareResetTraitInstances();
+
     QByteArray identityByteArray(bool setIsReplicated = false) const;
 
     QUrl getWireSafeSkeletonModelURL() const;
@@ -1596,13 +1594,13 @@ protected:
     bool hasParent() const { return !getParentID().isNull(); }
     bool hasFaceTracker() const { return _headData ? _headData->_isFaceTrackerConnected : false; }
 
-    qint64 packAvatarEntityTraitInstance(AvatarTraits::TraitType traitType,
+    QByteArray packSkeletonModelURL() const;
-                                         AvatarTraits::TraitInstanceID traitInstanceID,
+    QByteArray packAvatarEntityTraitInstance(AvatarTraits::TraitInstanceID traitInstanceID);
-                                         ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion);
+    QByteArray packGrabTraitInstance(AvatarTraits::TraitInstanceID traitInstanceID);
-    qint64 packGrabTraitInstance(AvatarTraits::TraitType traitType,
-                                 AvatarTraits::TraitInstanceID traitInstanceID,
-                                 ExtendedIODevice& destination, AvatarTraits::TraitVersion traitVersion);
 
+    void unpackSkeletonModelURL(const QByteArray& data);
+
+    
     // isReplicated will be true on downstream Avatar Mixers and their clients, but false on the upstream "master"
     // Audio Mixer that the replicated avatar is connected to.
     bool _isReplicated{ false };

libraries/avatars/src/AvatarTraits.cpp

@@ -0,0 +1,135 @@
+//
+//  AvatarTraits.cpp
+//  libraries/avatars/src
+//
+//  Created by Clement Brisset on 3/19/19.
+//  Copyright 2019 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 "AvatarTraits.h"
+
+#include <ExtendedIODevice.h>
+
+#include "AvatarData.h"
+
+namespace AvatarTraits {
+
+    qint64 packTrait(TraitType traitType, ExtendedIODevice& destination, const AvatarData& avatar) {
+        // Call packer function
+        auto traitBinaryData = avatar.packTrait(traitType);
+        auto traitBinaryDataSize = traitBinaryData.size();
+
+        // Verify packed data
+        if (traitBinaryDataSize > MAXIMUM_TRAIT_SIZE) {
+            qWarning() << "Refusing to pack simple trait" << traitType << "of size" << traitBinaryDataSize
+                        << "bytes since it exceeds the maximum size" << MAXIMUM_TRAIT_SIZE << "bytes";
+            return 0;
+        }
+
+        // Write packed data to stream
+        qint64 bytesWritten = 0;
+        bytesWritten += destination.writePrimitive((TraitType)traitType);
+        bytesWritten += destination.writePrimitive((TraitWireSize)traitBinaryDataSize);
+        bytesWritten += destination.write(traitBinaryData);
+        return bytesWritten;
+    }
+
+    qint64 packVersionedTrait(TraitType traitType, ExtendedIODevice& destination,
+                              TraitVersion traitVersion, const AvatarData& avatar) {
+        // Call packer function
+        auto traitBinaryData = avatar.packTrait(traitType);
+        auto traitBinaryDataSize = traitBinaryData.size();
+
+        // Verify packed data
+        if (traitBinaryDataSize > MAXIMUM_TRAIT_SIZE) {
+            qWarning() << "Refusing to pack simple trait" << traitType << "of size" << traitBinaryDataSize
+                        << "bytes since it exceeds the maximum size" << MAXIMUM_TRAIT_SIZE << "bytes";
+            return 0;
+        }
+
+        // Write packed data to stream
+        qint64 bytesWritten = 0;
+        bytesWritten += destination.writePrimitive((TraitType)traitType);
+        bytesWritten += destination.writePrimitive((TraitVersion)traitVersion);
+        bytesWritten += destination.writePrimitive((TraitWireSize)traitBinaryDataSize);
+        bytesWritten += destination.write(traitBinaryData);
+        return bytesWritten;
+    }
+
+
+    qint64 packTraitInstance(TraitType traitType, TraitInstanceID traitInstanceID,
+                             ExtendedIODevice& destination, AvatarData& avatar) {
+        // Call packer function
+        auto traitBinaryData = avatar.packTraitInstance(traitType, traitInstanceID);
+        auto traitBinaryDataSize = traitBinaryData.size();
+
+
+        // Verify packed data
+        if (traitBinaryDataSize > AvatarTraits::MAXIMUM_TRAIT_SIZE) {
+            qWarning() << "Refusing to pack instanced trait" << traitType << "of size" << traitBinaryDataSize
+                        << "bytes since it exceeds the maximum size " << AvatarTraits::MAXIMUM_TRAIT_SIZE << "bytes";
+            return 0;
+        }
+
+        // Write packed data to stream
+        qint64 bytesWritten = 0;
+        bytesWritten += destination.writePrimitive((TraitType)traitType);
+        bytesWritten += destination.write(traitInstanceID.toRfc4122());
+
+        if (!traitBinaryData.isNull()) {
+            bytesWritten += destination.writePrimitive((TraitWireSize)traitBinaryDataSize);
+            bytesWritten += destination.write(traitBinaryData);
+        } else {
+            bytesWritten += destination.writePrimitive(AvatarTraits::DELETED_TRAIT_SIZE);
+        }
+
+        return bytesWritten;
+    }
+
+    qint64 packVersionedTraitInstance(TraitType traitType, TraitInstanceID traitInstanceID,
+                                      ExtendedIODevice& destination, TraitVersion traitVersion,
+                                      AvatarData& avatar) {
+        // Call packer function
+        auto traitBinaryData = avatar.packTraitInstance(traitType, traitInstanceID);
+        auto traitBinaryDataSize = traitBinaryData.size();
+
+
+        // Verify packed data
+        if (traitBinaryDataSize > AvatarTraits::MAXIMUM_TRAIT_SIZE) {
+            qWarning() << "Refusing to pack instanced trait" << traitType << "of size" << traitBinaryDataSize
+                        << "bytes since it exceeds the maximum size " << AvatarTraits::MAXIMUM_TRAIT_SIZE << "bytes";
+            return 0;
+        }
+
+        // Write packed data to stream
+        qint64 bytesWritten = 0;
+        bytesWritten += destination.writePrimitive((TraitType)traitType);
+        bytesWritten += destination.writePrimitive((TraitVersion)traitVersion);
+        bytesWritten += destination.write(traitInstanceID.toRfc4122());
+
+        if (!traitBinaryData.isNull()) {
+            bytesWritten += destination.writePrimitive((TraitWireSize)traitBinaryDataSize);
+            bytesWritten += destination.write(traitBinaryData);
+        } else {
+            bytesWritten += destination.writePrimitive(AvatarTraits::DELETED_TRAIT_SIZE);
+        }
+
+        return bytesWritten;
+    }
+
+
+    qint64 packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination,
+                                         TraitVersion traitVersion) {
+        qint64 bytesWritten = 0;
+        bytesWritten += destination.writePrimitive(traitType);
+        if (traitVersion > DEFAULT_TRAIT_VERSION) {
+            bytesWritten += destination.writePrimitive(traitVersion);
+        }
+        bytesWritten += destination.write(instanceID.toRfc4122());
+        bytesWritten += destination.writePrimitive(DELETED_TRAIT_SIZE);
+        return bytesWritten;
+    }
+};

libraries/avatars/src/AvatarTraits.h

@@ -14,20 +14,35 @@
 
 #include <algorithm>
 #include <cstdint>
+#include <array>
 #include <vector>
 
 #include <QtCore/QUuid>
 
+class ExtendedIODevice;
+class AvatarData;
+
 namespace AvatarTraits {
     enum TraitType : int8_t {
+        // Null trait
         NullTrait = -1,
-        SkeletonModelURL,
+
+        // Simple traits
+        SkeletonModelURL = 0,
+
+        // Instanced traits
         FirstInstancedTrait,
         AvatarEntity = FirstInstancedTrait,
         Grab,
+
+        // Traits count
         TotalTraitTypes
     };
 
+    const int NUM_SIMPLE_TRAITS = (int)FirstInstancedTrait;
+    const int NUM_INSTANCED_TRAITS = (int)TotalTraitTypes - (int)FirstInstancedTrait;
+    const int NUM_TRAITS = (int)TotalTraitTypes;
+
     using TraitInstanceID = QUuid;
 
     inline bool isSimpleTrait(TraitType traitType) {
@@ -46,22 +61,19 @@ namespace AvatarTraits {
     const TraitMessageSequence FIRST_TRAIT_SEQUENCE = 0;
     const TraitMessageSequence MAX_TRAIT_SEQUENCE = INT64_MAX;
 
-    inline qint64 packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination,
+    qint64 packTrait(TraitType traitType, ExtendedIODevice& destination, const AvatarData& avatar);
-                                         TraitVersion traitVersion = NULL_TRAIT_VERSION) {
+    qint64 packVersionedTrait(TraitType traitType, ExtendedIODevice& destination,
-        qint64 bytesWritten = 0;
+                              TraitVersion traitVersion, const AvatarData& avatar);
 
-        bytesWritten += destination.writePrimitive(traitType);
+    qint64 packTraitInstance(TraitType traitType, TraitInstanceID traitInstanceID,
+                             ExtendedIODevice& destination, AvatarData& avatar);
+    qint64 packVersionedTraitInstance(TraitType traitType, TraitInstanceID traitInstanceID,
+                                      ExtendedIODevice& destination, TraitVersion traitVersion,
+                                      AvatarData& avatar);
 
-        if (traitVersion > DEFAULT_TRAIT_VERSION) {
+    qint64 packInstancedTraitDelete(TraitType traitType, TraitInstanceID instanceID, ExtendedIODevice& destination,
-            bytesWritten += destination.writePrimitive(traitVersion);
+                                           TraitVersion traitVersion = NULL_TRAIT_VERSION);
-        }
 
-        bytesWritten += destination.write(instanceID.toRfc4122());
-
-        bytesWritten += destination.writePrimitive(DELETED_TRAIT_SIZE);
-
-        return bytesWritten;
-    }
 };
 
 #endif // hifi_AvatarTraits_h

libraries/avatars/src/ClientTraitsHandler.cpp

@@ -106,9 +106,10 @@ int ClientTraitsHandler::sendChangedTraitsToMixer() {
             auto traitType = static_cast<AvatarTraits::TraitType>(std::distance(traitStatusesCopy.simpleCBegin(), simpleIt));
 
             if (initialSend || *simpleIt == Updated) {
-                if (traitType == AvatarTraits::SkeletonModelURL) {
+                bytesWritten += AvatarTraits::packTrait(traitType, *traitsPacketList, *_owningAvatar);
-                    bytesWritten += _owningAvatar->packTrait(traitType, *traitsPacketList);
 
+
+                if (traitType == AvatarTraits::SkeletonModelURL) {
                     // keep track of our skeleton version in case we get an override back
                     _currentSkeletonVersion = _currentTraitVersion;
                 }
@@ -124,7 +125,9 @@ int ClientTraitsHandler::sendChangedTraitsToMixer() {
                     || 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
-                    bytesWritten += _owningAvatar->packTraitInstance(instancedIt->traitType, instanceIDValuePair.id, *traitsPacketList);
+                    bytesWritten += AvatarTraits::packTraitInstance(instancedIt->traitType, instanceIDValuePair.id,
+                                                                    *traitsPacketList, *_owningAvatar);
+
                 } else if (!initialSend && instanceIDValuePair.value == Deleted) {
                     // pack delete for this trait instance
                     bytesWritten += AvatarTraits::packInstancedTraitDelete(instancedIt->traitType, instanceIDValuePair.id,
@@ -162,11 +165,11 @@ void ClientTraitsHandler::processTraitOverride(QSharedPointer<ReceivedMessage> m
 
                 // override the skeleton URL but do not mark the trait as having changed
                 // 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);
+                auto traitBinaryData = message->readWithoutCopy(traitBinarySize);
+                _owningAvatar->processTrait(traitType, traitBinaryData);
 
                 // 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