fixes from 78

interface/resources/qml/hifi/avatarapp/AdjustWearables.qml

@@ -177,8 +177,9 @@ Rectangle {
         repeat: true
         onTriggered: {
             var currentWearable = getCurrentWearable();
-            var soft = currentWearable ? currentWearable.relayParentJoints : false;
+            var hasSoft = currentWearable && currentWearable.relayParentJoints !== undefined;
-            var softEnabled = currentWearable ? entityHasAvatarJoints(currentWearable.id) : false;
+            var soft = hasSoft ? currentWearable.relayParentJoints : false;
+            var softEnabled = hasSoft ? entityHasAvatarJoints(currentWearable.id) : false;
             isSoft.set(soft);
             isSoft.enabled = softEnabled;
         }
@@ -511,7 +512,7 @@ Rectangle {
 
                 function set(value) {
                     notify = false;
-                    checked = value
+                    checked = value;
                     notify = true;
                 }
 

interface/src/AvatarBookmarks.cpp

@@ -55,7 +55,7 @@ void addAvatarEntities(const QVariantList& avatarEntities) {
         QVariantMap asMap = variantProperties.toMap();
         QScriptValue scriptProperties = variantMapToScriptValue(asMap, scriptEngine);
         EntityItemProperties entityProperties;
-        EntityItemPropertiesFromScriptValueHonorReadOnly(scriptProperties, entityProperties);
+        EntityItemPropertiesFromScriptValueIgnoreReadOnly(scriptProperties, entityProperties);
 
         entityProperties.setParentID(myNodeID);
         entityProperties.setEntityHostType(entity::HostType::AVATAR);
@@ -151,8 +151,29 @@ void AvatarBookmarks::removeBookmark(const QString& bookmarkName) {
 
 void AvatarBookmarks::updateAvatarEntities(const QVariantList &avatarEntities) {
     auto myAvatar = DependencyManager::get<AvatarManager>()->getMyAvatar();
-    myAvatar->removeWearableAvatarEntities();
+    auto currentAvatarEntities = myAvatar->getAvatarEntityData();
-    addAvatarEntities(avatarEntities);
+    std::set<QUuid> newAvatarEntities;
+
+    // Update or add all the new avatar entities
+    for (auto& avatarEntityVariant : avatarEntities) {
+        auto avatarEntityVariantMap = avatarEntityVariant.toMap();
+        auto idItr = avatarEntityVariantMap.find("id");
+        if (idItr != avatarEntityVariantMap.end()) {
+            auto propertiesItr = avatarEntityVariantMap.find("properties");
+            if (propertiesItr != avatarEntityVariantMap.end()) {
+                EntityItemID id = idItr.value().toUuid();
+                newAvatarEntities.insert(id);
+                myAvatar->updateAvatarEntity(id, QJsonDocument::fromVariant(propertiesItr.value()).toBinaryData());
+            }
+        }
+    }
+
+    // Remove any old entities not in the new list
+    for (auto& avatarEntityID : currentAvatarEntities.keys()) {
+        if (newAvatarEntities.find(avatarEntityID) == newAvatarEntities.end()) {
+            myAvatar->removeAvatarEntity(avatarEntityID);
+        }
+    }
 }
 
 void AvatarBookmarks::loadBookmark(const QString& bookmarkName) {
@@ -176,7 +197,7 @@ void AvatarBookmarks::loadBookmark(const QString& bookmarkName) {
             auto myAvatar = DependencyManager::get<AvatarManager>()->getMyAvatar();
             auto treeRenderer = DependencyManager::get<EntityTreeRenderer>();
             EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr;
-            myAvatar->removeWearableAvatarEntities();
+            myAvatar->clearAvatarEntities();
             const QString& avatarUrl = bookmark.value(ENTRY_AVATAR_URL, "").toString();
             myAvatar->useFullAvatarURL(avatarUrl);
             qCDebug(interfaceapp) << "Avatar On";

interface/src/avatar/MyAvatar.cpp

@@ -1610,10 +1610,12 @@ void MyAvatar::handleChangedAvatarEntityData() {
                 skip = true;
             }
         });
-        sanitizeAvatarEntityProperties(properties);
+        if (!skip) {
-        entityTree->withWriteLock([&] {
+            sanitizeAvatarEntityProperties(properties);
-            entityTree->updateEntity(id, properties);
+            entityTree->withWriteLock([&] {
-        });
+                entityTree->updateEntity(id, properties);
+            });
+        }
     }
 
     // DELETE cached blobs
@@ -1810,19 +1812,27 @@ void MyAvatar::setAvatarEntityData(const AvatarEntityMap& avatarEntityData) {
 
 void MyAvatar::updateAvatarEntity(const QUuid& entityID, const QByteArray& entityData) {
     // NOTE: this is an invokable Script call
-    // TODO: we should handle the case where entityData is corrupt or invalid
+    bool changed = false;
-    // BEFORE we store into _cachedAvatarEntityBlobs
-    _needToSaveAvatarEntitySettings = true;
     _avatarEntitiesLock.withWriteLock([&] {
-        AvatarEntityMap::iterator itr = _cachedAvatarEntityBlobs.find(entityID);
+        auto data = QJsonDocument::fromBinaryData(entityData);
-        if (itr != _cachedAvatarEntityBlobs.end()) {
+        if (data.isEmpty() || data.isNull() || !data.isObject() || data.object().isEmpty()) {
-            _entitiesToUpdate.push_back(entityID);
+            qDebug() << "ERROR!  Trying to update with invalid avatar entity data.  Skipping." << data;
-            itr.value() = entityData;
         } else {
-            _entitiesToAdd.push_back(entityID);
+            auto itr = _cachedAvatarEntityBlobs.find(entityID);
-            _cachedAvatarEntityBlobs.insert(entityID, entityData);
+            if (itr == _cachedAvatarEntityBlobs.end()) {
+                _entitiesToAdd.push_back(entityID);
+                _cachedAvatarEntityBlobs.insert(entityID, entityData);
+                changed = true;
+            } else {
+                _entitiesToUpdate.push_back(entityID);
+                itr.value() = entityData;
+                changed = true;
+            }
         }
     });
+    if (changed) {
+        _needToSaveAvatarEntitySettings = true;
+    }
 }
 
 void MyAvatar::avatarEntityDataToJson(QJsonObject& root) const {
@@ -2362,50 +2372,36 @@ bool isWearableEntity(const EntityItemPointer& entity) {
             || entity->getParentID() == AVATAR_SELF_ID);
 }
 
-void MyAvatar::clearAvatarEntities() {
+void MyAvatar::removeAvatarEntity(const EntityItemID& entityID) {
     auto treeRenderer = DependencyManager::get<EntityTreeRenderer>();
     EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr;
 
-    QList<QUuid> avatarEntityIDs;
-    _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);
-        });
-
-        // remove the avatar entity from our internal list
-        // (but indicate it doesn't need to be pulled from the tree)
-        clearAvatarEntity(entityID, false);
-    }
-}
-
-void MyAvatar::removeWearableAvatarEntities() {
-    auto treeRenderer = DependencyManager::get<EntityTreeRenderer>();
-    EntityTreePointer entityTree = treeRenderer ? treeRenderer->getTree() : nullptr;
     if (entityTree) {
-        QList<QUuid> avatarEntityIDs;
+        auto entity = entityTree->findEntityByID(entityID);
-        _avatarEntitiesLock.withReadLock([&] {
+        if (entity && isWearableEntity(entity)) {
-                avatarEntityIDs = _packedAvatarEntityData.keys();
+            entityTree->withWriteLock([&entityID, &entityTree] {
-        });
+                // remove this entity first from the entity tree
-        for (const auto& entityID : avatarEntityIDs) {
+                entityTree->deleteEntity(entityID, true, true);
-            auto entity = entityTree->findEntityByID(entityID);
+            });
-            if (entity && isWearableEntity(entity)) {
-                entityTree->withWriteLock([&entityID, &entityTree] {
-                    // remove this entity first from the entity tree
-                    entityTree->deleteEntity(entityID, true, true);
-                });
 
-                // remove the avatar entity from our internal list
+            // remove the avatar entity from our internal list
-                // (but indicate it doesn't need to be pulled from the tree)
+            // (but indicate it doesn't need to be pulled from the tree)
-                clearAvatarEntity(entityID, false);
+            clearAvatarEntity(entityID, false);
-            }
         }
     }
 }
 
+void MyAvatar::clearAvatarEntities() {
+    QList<QUuid> avatarEntityIDs;
+    _avatarEntitiesLock.withReadLock([&] {
+        avatarEntityIDs = _packedAvatarEntityData.keys();
+    });
+    for (auto entityID : avatarEntityIDs) {
+        removeAvatarEntity(entityID);
+    }
+}
+
+
 QVariantList MyAvatar::getAvatarEntitiesVariant() {
     // NOTE: this method is NOT efficient
     QVariantList avatarEntitiesData;
@@ -2428,6 +2424,7 @@ QVariantList MyAvatar::getAvatarEntitiesVariant() {
             desiredProperties += PROP_LOCAL_ROTATION;
             EntityItemProperties entityProperties = entity->getProperties(desiredProperties);
             QScriptValue scriptProperties = EntityItemPropertiesToScriptValue(_myScriptEngine, entityProperties);
+            avatarEntityData["id"] = entityID;
             avatarEntityData["properties"] = scriptProperties.toVariant();
             avatarEntitiesData.append(QVariant(avatarEntityData));
         }

interface/src/avatar/MyAvatar.h

@@ -968,8 +968,8 @@ public:
     * @returns {object[]}
     */
     Q_INVOKABLE QVariantList getAvatarEntitiesVariant();
+    void removeAvatarEntity(const EntityItemID& entityID);
     void clearAvatarEntities();
-    void removeWearableAvatarEntities();
 
     /**jsdoc
      * Check whether your avatar is flying or not.

libraries/avatars/src/AvatarData.cpp

@@ -2744,23 +2744,28 @@ void AvatarData::setAttachmentsVariant(const QVariantList& variant) {
 }
 
 void AvatarData::storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& data) {
+    bool changed = false;
     _avatarEntitiesLock.withWriteLock([&] {
-        PackedAvatarEntityMap::iterator itr = _packedAvatarEntityData.find(entityID);
+        auto itr = _packedAvatarEntityData.find(entityID);
         if (itr == _packedAvatarEntityData.end()) {
             if (_packedAvatarEntityData.size() < MAX_NUM_AVATAR_ENTITIES) {
                 _packedAvatarEntityData.insert(entityID, data);
+                changed = true;
             }
         } else {
             itr.value() = data;
+            changed = true;
         }
     });
 
-    _avatarEntityDataChanged = true;
+    if (changed) {
+        _avatarEntityDataChanged = true;
 
-    if (_clientTraitsHandler) {
+        if (_clientTraitsHandler) {
-        // we have a client traits handler, so we need to mark this instanced trait as changed
+            // we have a client traits handler, so we need to mark this instanced trait as changed
-        // so that changes will be sent next frame
+            // so that changes will be sent next frame
-        _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID);
+            _clientTraitsHandler->markInstancedTraitUpdated(AvatarTraits::AvatarEntity, entityID);
+        }
     }
 }
 

libraries/entities/src/EntityItemProperties.cpp

@@ -4616,14 +4616,14 @@ bool EntityItemProperties::blobToProperties(QScriptEngine& scriptEngine, const Q
     // DANGER: this method is NOT efficient.
     // begin recipe for converting unfortunately-formatted-binary-blob to EntityItemProperties
     QJsonDocument jsonProperties = QJsonDocument::fromBinaryData(blob);
-    if (!jsonProperties.isObject()) {
+    if (jsonProperties.isEmpty() || jsonProperties.isNull() || !jsonProperties.isObject() || jsonProperties.object().isEmpty()) {
         qCDebug(entities) << "bad avatarEntityData json" << QString(blob.toHex());
         return false;
     }
     QVariant variant = jsonProperties.toVariant();
     QVariantMap variantMap = variant.toMap();
     QScriptValue scriptValue = variantMapToScriptValue(variantMap, scriptEngine);
-    EntityItemPropertiesFromScriptValueHonorReadOnly(scriptValue, properties);
+    EntityItemPropertiesFromScriptValueIgnoreReadOnly(scriptValue, properties);
     // end recipe
     return true;
 }

scripts/system/avatarapp.js

@@ -70,9 +70,9 @@ function getMyAvatarSettings() {
     }
 }
 
-function updateAvatarWearables(avatar, callback) {
+function updateAvatarWearables(avatar, callback, wearablesOverride) {
     executeLater(function() {
-        var wearables = getMyAvatarWearables();
+        var wearables = wearablesOverride ? wearablesOverride : getMyAvatarWearables();
         avatar[ENTRY_AVATAR_ENTITIES] = wearables;
 
         sendToQml({'method' : 'wearablesUpdated', 'wearables' : wearables})
@@ -210,7 +210,7 @@ function fromQml(message) { // messages are {method, params}, like json-rpc. See
         break;
     case 'adjustWearable':
         if(message.properties.localRotationAngles) {
-            message.properties.localRotation = Quat.fromVec3Degrees(message.properties.localRotationAngles)
+            message.properties.localRotation = Quat.fromVec3Degrees(message.properties.localRotationAngles);
         }
 
         Entities.editEntity(message.entityID, message.properties);
@@ -235,7 +235,7 @@ function fromQml(message) { // messages are {method, params}, like json-rpc. See
             // revert changes using snapshot of wearables
             if(currentAvatarWearablesBackup !== null) {
                 AvatarBookmarks.updateAvatarEntities(currentAvatarWearablesBackup);
-                updateAvatarWearables(currentAvatar);
+                updateAvatarWearables(currentAvatar, null, currentAvatarWearablesBackup);
             }
         } else {
             sendToQml({'method' : 'updateAvatarInBookmarks'});