ScriptableAvatar::setAvatarEntityData() works

assignment-client/src/avatars/ScriptableAvatar.cpp

@@ -252,26 +252,79 @@ void ScriptableAvatar::setHasAudioEnabledFaceMovement(bool hasAudioEnabledFaceMo
     _headData->setHasAudioEnabledFaceMovement(hasAudioEnabledFaceMovement);
 }
 
-void ScriptableAvatar::updateAvatarEntity(const QUuid& id, const QByteArray& data) {
+AvatarEntityMap ScriptableAvatar::getAvatarEntityData() const {
-    /* TODO: fix this
+    // DANGER: Now that we store the AvatarEntityData in packed format this call is potentially Very Expensive!
-    if (data.isNull()) {
+    // Avoid calling this method if possible.
-        // interpret this as a DELETE
+    AvatarEntityMap data;
-        std::map<QUuid, EntityItemPointer>::iterator itr = _entities.find(id);
+    QUuid sessionID = getID();
-        if (itr != _entities.end()) {
+    _avatarEntitiesLock.withReadLock([&] {
-            _entities.erase(itr);
+        for (const auto& itr : _entities) {
-            clearAvatarEntity(id);
+            QUuid id = itr.first;
+            EntityItemPointer entity = itr.second;
+            EntityItemProperties properties = entity->getProperties();
+            QByteArray blob;
+            EntityItemProperties::propertiesToBlob(_scriptEngine, sessionID, properties, blob);
+            data[id] = blob;
         }
-    } else {
+    });
-        EntityItemPointer entity;
+    return data;
-        EntityItemProperties properties;
+}
-        bool honorReadOnly = true;
-        properties.copyFromScriptValue(data, honorReadOnly);
 
-        std::map<QUuid, EntityItemPointer>::iterator itr = _entities.find(id);
+void ScriptableAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) {
-        if (itr == _entities.end()) {
+    // Note: this is an invokable Script call
-            // this is an ADD
+    // avatarEntityData is expected to be a map of QByteArrays that represent EntityItemProperties objects from JavaScript
-            entity = EntityTypes::constructEntityItem(id, properties);
+    //
+    if (avatarEntityData.size() > MAX_NUM_AVATAR_ENTITIES) {
+        // the data is suspect
+        qCDebug(avatars) << "discard suspect avatarEntityData with size =" << avatarEntityData.size();
+        return;
+    }
+
+    // convert binary data to EntityItemProperties
+    // NOTE: this operation is NOT efficient
+    std::map<QUuid, EntityItemProperties> newProperties;
+    AvatarEntityMap::const_iterator dataItr = avatarEntityData.begin();
+    while (dataItr != avatarEntityData.end()) {
+        EntityItemProperties properties;
+        const QByteArray& blob = dataItr.value();
+        if (!blob.isNull() && EntityItemProperties::blobToProperties(_scriptEngine, blob, properties)) {
+            newProperties[dataItr.key()] = properties;
+        }
+        ++dataItr;
+    }
+
+    // delete existing entities not found in avatarEntityData
+    std::vector<QUuid> idsToClear;
+    _avatarEntitiesLock.withWriteLock([&] {
+        std::map<QUuid, EntityItemPointer>::iterator entityItr = _entities.begin();
+        while (entityItr != _entities.end()) {
+            QUuid id = entityItr->first;
+            std::map<QUuid, EntityItemProperties>::const_iterator propertiesItr = newProperties.find(id);
+            if (propertiesItr == newProperties.end()) {
+                idsToClear.push_back(id);
+                entityItr = _entities.erase(entityItr);
+            } else {
+                ++entityItr;
+            }
+        }
+    });
+
+    // add or update entities
+    _avatarEntitiesLock.withWriteLock([&] {
+        std::map<QUuid, EntityItemProperties>::const_iterator propertiesItr = newProperties.begin();
+        while (propertiesItr != newProperties.end()) {
+            QUuid id = propertiesItr->first;
+            const EntityItemProperties& properties = propertiesItr->second;
+            std::map<QUuid, EntityItemPointer>::iterator entityItr = _entities.find(id);
+            EntityItemPointer entity;
+            if (entityItr != _entities.end()) {
+                entity = entityItr->second;
+                entity->setProperties(properties);
+            } else {
+                entity = EntityTypes::constructEntityItem(id, properties);
+            }
             if (entity) {
+                // build outgoing payload
                 OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE);
                 EncodeBitstreamParams params;
                 EntityTreeElementExtraEncodeDataPointer extra { nullptr };
@@ -281,24 +334,74 @@ void ScriptableAvatar::updateAvatarEntity(const QUuid& id, const QByteArray& dat
                     _entities[id] = entity;
                     QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize());
                     storeAvatarEntityDataPayload(id, tempArray);
+                } else {
+                    // payload doesn't fit
+                    entityItr = _entities.find(id);
+                    if (entityItr != _entities.end()) {
+                        _entities.erase(entityItr);
+                        idsToClear.push_back(id);
+                    }
+
                 }
             }
-        } else {
+            ++propertiesItr;
-            // this is an UPDATE
+        }
-            entity = itr->second;
+    });
-            bool somethingChanged = entity->setProperties(properties);
-            if (somethingChanged) {
-                OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE);
-                EncodeBitstreamParams params;
-                EntityTreeElementExtraEncodeDataPointer extra { nullptr };
-                OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra);
 
-                if (appendState == OctreeElement::COMPLETED) {
+    // clear deleted traits
-                    QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize());
+    for (const auto& id : idsToClear) {
-                    storeAvatarEntityDataPayload(id, tempArray);
+        clearAvatarEntity(id);
-                }
+    }
+}
+
+void ScriptableAvatar::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) {
+    if (entityData.isNull()) {
+        // interpret this as a DELETE
+        std::map<QUuid, EntityItemPointer>::iterator itr = _entities.find(entityID);
+        if (itr != _entities.end()) {
+            _entities.erase(itr);
+            clearAvatarEntity(entityID);
+        }
+        return;
+    }
+
+    EntityItemPointer entity;
+    EntityItemProperties properties;
+    if (!EntityItemProperties::blobToProperties(_scriptEngine, entityData, properties)) {
+        // entityData is corrupt
+        return;
+    }
+
+    std::map<QUuid, EntityItemPointer>::iterator itr = _entities.find(entityID);
+    if (itr == _entities.end()) {
+        // this is an ADD
+        entity = EntityTypes::constructEntityItem(entityID, properties);
+        if (entity) {
+            OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE);
+            EncodeBitstreamParams params;
+            EntityTreeElementExtraEncodeDataPointer extra { nullptr };
+            OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra);
+
+            if (appendState == OctreeElement::COMPLETED) {
+                _entities[entityID] = entity;
+                QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize());
+                storeAvatarEntityDataPayload(entityID, tempArray);
+            }
+        }
+    } else {
+        // this is an UPDATE
+        entity = itr->second;
+        bool somethingChanged = entity->setProperties(properties);
+        if (somethingChanged) {
+            OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE);
+            EncodeBitstreamParams params;
+            EntityTreeElementExtraEncodeDataPointer extra { nullptr };
+            OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra);
+
+            if (appendState == OctreeElement::COMPLETED) {
+                QByteArray tempArray((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize());
+                storeAvatarEntityDataPayload(entityID, tempArray);
             }
         }
     }
-    */
 }

assignment-client/src/avatars/ScriptableAvatar.h

@@ -185,7 +185,26 @@ public:
     bool getHasProceduralEyeFaceMovement() const override { return _headData->getHasProceduralEyeFaceMovement(); }
     void setHasAudioEnabledFaceMovement(bool hasAudioEnabledFaceMovement);
     bool getHasAudioEnabledFaceMovement() const override { return _headData->getHasAudioEnabledFaceMovement(); }
-    void updateAvatarEntity(const QUuid& id, const QByteArray& data) override;
+
+     /**jsdoc
+     * Potentially Very Expensive.  Do not use.
+     * @function Avatar.getAvatarEntityData
+     * @returns {object}
+     */
+    Q_INVOKABLE AvatarEntityMap getAvatarEntityData() const override;
+
+    /**jsdoc
+    * @function MyAvatar.setAvatarEntityData
+    * @param {object} avatarEntityData
+    */
+    Q_INVOKABLE void setAvatarEntityData(const AvatarEntityMap& avatarEntityData) override;
+
+    /**jsdoc
+     * @function MyAvatar.updateAvatarEntity
+     * @param {Uuid} entityID
+     * @param {string} entityData
+     */
+    Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) override;
 
 public slots:
     void update(float deltatime);
@@ -204,6 +223,7 @@ private:
     QHash<QString, int> _fstJointIndices; ///< 1-based, since zero is returned for missing keys
     QStringList _fstJointNames; ///< in order of depth-first traversal
     QUrl _skeletonFBXURL;
+    mutable QScriptEngine _scriptEngine;
     std::map<QUuid, EntityItemPointer> _entities;
 
     /// Loads the joint indices, names from the FST file (if any)

interface/src/avatar/MyAvatar.cpp

@@ -2353,8 +2353,11 @@ void MyAvatar::clearAvatarEntities() {
     auto treeRenderer = DependencyManager::get<EntityTreeRenderer>();
     EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr;
 
-    AvatarEntityMap avatarEntities = getAvatarEntityData();
+    QList<QUuid> avatarEntityIDs;
-    for (auto entityID : avatarEntities.keys()) {
+    _avatarEntitiesLock.withReadLock([&] {
+            avatarEntityIDs = _packedAvatarEntityData.keys();
+    });
+    for (const auto& entityID : avatarEntityIDs) {
         entityTree->withWriteLock([&entityID, &entityTree] {
             // remove this entity first from the entity tree
             entityTree->deleteEntity(entityID, true, true);
@@ -2369,10 +2372,12 @@ void MyAvatar::clearAvatarEntities() {
 void MyAvatar::removeWearableAvatarEntities() {
     auto treeRenderer = DependencyManager::get<EntityTreeRenderer>();
     EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr;
-    
     if (entityTree) {
-        AvatarEntityMap avatarEntities = getAvatarEntityData();
+        QList<QUuid> avatarEntityIDs;
-        for (auto entityID : avatarEntities.keys()) {
+        _avatarEntitiesLock.withReadLock([&] {
+                avatarEntityIDs = _packedAvatarEntityData.keys();
+        });
+        for (const auto& entityID : avatarEntityIDs) {
             auto entity = entityTree->findEntityByID(entityID);
             if (entity && isWearableEntity(entity)) {
                 entityTree->withWriteLock([&entityID, &entityTree] {
@@ -2394,8 +2399,11 @@ QVariantList MyAvatar::getAvatarEntitiesVariant() {
     auto treeRenderer = DependencyManager::get<EntityTreeRenderer>();
     EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr;
     if (entityTree) {
-        AvatarEntityMap avatarEntities = getAvatarEntityData();
+        QList<QUuid> avatarEntityIDs;
-        for (auto entityID : avatarEntities.keys()) {
+        _avatarEntitiesLock.withReadLock([&] {
+                avatarEntityIDs = _packedAvatarEntityData.keys();
+        });
+        for (const auto& entityID : avatarEntityIDs) {
             auto entity = entityTree->findEntityByID(entityID);
             if (!entity) {
                 continue;
@@ -2795,17 +2803,17 @@ void MyAvatar::setAttachmentData(const QVector<AttachmentData>& attachmentData)
 }
 
 QVector<AttachmentData> MyAvatar::getAttachmentData() const {    
-    QVector<AttachmentData> avatarData;
+    QVector<AttachmentData> attachmentData;
-    auto avatarEntities = getAvatarEntityData();
+    QList<QUuid> avatarEntityIDs;
-    AvatarEntityMap::const_iterator dataItr = avatarEntities.begin();
+    _avatarEntitiesLock.withReadLock([&] {
-    while (dataItr != avatarEntities.end()) {
+        avatarEntityIDs = _packedAvatarEntityData.keys();
-        QUuid entityID = dataItr.key();
+    });
+    for (const auto& entityID : avatarEntityIDs) {
         auto properties = DependencyManager::get<EntityScriptingInterface>()->getEntityProperties(entityID);
         AttachmentData data = entityPropertiesToAttachmentData(properties);
-        avatarData.append(data);
+        attachmentData.append(data);
-        dataItr++;
     }
-    return avatarData;
+    return attachmentData;
 }
 
 QVariantList MyAvatar::getAttachmentsVariant() const {
@@ -2834,16 +2842,16 @@ void MyAvatar::setAttachmentsVariant(const QVariantList& variant) {
 }
 
 bool MyAvatar::findAvatarEntity(const QString& modelURL, const QString& jointName, QUuid& entityID) {
-    auto avatarEntities = getAvatarEntityData();
+    QList<QUuid> avatarEntityIDs;
-    AvatarEntityMap::const_iterator dataItr = avatarEntities.begin();
+    _avatarEntitiesLock.withReadLock([&] {
-    while (dataItr != avatarEntities.end()) {
+            avatarEntityIDs = _packedAvatarEntityData.keys();
-        entityID = dataItr.key();
+    });
+    for (const auto& entityID : avatarEntityIDs) {
         auto props = DependencyManager::get<EntityScriptingInterface>()->getEntityProperties(entityID);
         if (props.getModelURL() == modelURL &&
             (jointName.isEmpty() || props.getParentJointIndex() == getJointIndex(jointName))) {
             return true;
         }
-        dataItr++;
     }
     return false;
 }

libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp

@@ -340,10 +340,13 @@ void Avatar::updateAvatarEntities() {
         return;
     }
 
+    PackedAvatarEntityMap packedAvatarEntityData;
+    _avatarEntitiesLock.withReadLock([&] {
+        packedAvatarEntityData = _packedAvatarEntityData;
+    });
     entityTree->withWriteLock([&] {
-        AvatarEntityMap avatarEntities = getAvatarEntityData();
+        AvatarEntityMap::const_iterator dataItr = packedAvatarEntityData.begin();
-        AvatarEntityMap::const_iterator dataItr = avatarEntities.begin();
+        while (dataItr != packedAvatarEntityData.end()) {
-        while (dataItr != avatarEntities.end()) {
             // compute hash of data.  TODO? cache this?
             QByteArray data = dataItr.value();
             uint32_t newHash = qHash(data);
@@ -381,10 +384,11 @@ void Avatar::updateAvatarEntities() {
             ++dataItr;
 
             EntityItemProperties properties;
-            {
+            int32_t bytesLeftToRead = data.size();
-                int32_t bytesLeftToRead = data.size();
+            unsigned char* dataAt = (unsigned char*)(data.data());
-                unsigned char* dataAt = (unsigned char*)(data.data());
+            if (!properties.constructFromBuffer(dataAt, bytesLeftToRead)) {
-                properties.constructFromBuffer(dataAt, bytesLeftToRead);
+                // properties are corrupt
+                continue;
             }
 
             properties.setEntityHostType(entity::HostType::AVATAR);
@@ -454,7 +458,7 @@ void Avatar::updateAvatarEntities() {
                 }
             }
         }
-        if (avatarEntities.size() != _avatarEntityForRecording.size()) {
+        if (packedAvatarEntityData.size() != _avatarEntityForRecording.size()) {
             createRecordingIDs();
         }
     });
@@ -466,9 +470,12 @@ void Avatar::removeAvatarEntitiesFromTree() {
     auto treeRenderer = DependencyManager::get<EntityTreeRenderer>();
     EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr;
     if (entityTree) {
+        QList<QUuid> avatarEntityIDs;
+        _avatarEntitiesLock.withReadLock([&] {
+            avatarEntityIDs = _packedAvatarEntityData.keys();
+        });
         entityTree->withWriteLock([&] {
-            AvatarEntityMap avatarEntities = getAvatarEntityData();
+            for (const auto& entityID : avatarEntityIDs) {
-            for (auto entityID : avatarEntities.keys()) {
                 entityTree->deleteEntity(entityID, true, true);
             }
         });

libraries/entities/src/EntityItemProperties.cpp

@@ -4613,6 +4613,7 @@ void EntityItemProperties::convertToCloneProperties(const EntityItemID& entityID
 }
 
 bool EntityItemProperties::blobToProperties(QScriptEngine& scriptEngine, const QByteArray& blob, EntityItemProperties& properties) {
+    // DANGER: this method is NOT efficient.
     // begin recipe for converting unfortunately-formatted-binary-blob to EntityItemProperties
     QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(blob);
     if (!jsonProperties.isObject()) {
@@ -4628,6 +4629,7 @@ bool EntityItemProperties::blobToProperties(QScriptEngine& scriptEngine, const Q
 }
 
 void EntityItemProperties::propertiesToBlob(QScriptEngine& scriptEngine, const QUuid& myAvatarID, const EntityItemProperties& properties, QByteArray& blob) {
+    // DANGER: this method is NOT efficient.
     // begin recipe for extracting unfortunately-formatted-binary-blob from EntityItem
     QScriptValue scriptValue = EntityItemNonDefaultPropertiesToScriptValue(&scriptEngine, properties);
     QVariant variantProperties = scriptValue.toVariant();