From 6c81e8845bc800f80fdff4d583f356490319cfdd Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Fri, 7 Dec 2018 14:43:44 -0800
Subject: [PATCH] cleanup

---
 interface/src/avatar/MyAvatar.cpp             | 59 ++++++++++++-------
 interface/src/avatar/MyAvatar.h               |  3 +-
 libraries/avatars/src/AvatarData.cpp          |  2 +-
 libraries/avatars/src/AvatarData.h            |  4 +-
 .../entities/src/EntityEditPacketSender.cpp   |  4 +-
 libraries/octree/src/OctreePacketData.cpp     |  9 ---
 libraries/octree/src/OctreePacketData.h       |  2 -
 7 files changed, 44 insertions(+), 39 deletions(-)

diff --git a/interface/src/avatar/MyAvatar.cpp b/interface/src/avatar/MyAvatar.cpp
index 7d6318ddbb..04f1cb599e 100755
--- a/interface/src/avatar/MyAvatar.cpp
+++ b/interface/src/avatar/MyAvatar.cpp
@@ -1303,6 +1303,10 @@ void MyAvatar::saveData() {
 }
 
 void MyAvatar::saveAvatarEntityDataToSettings() {
+    if (!_entitiesToSaveToSettings.empty()) {
+        // TODO: save these to settings.
+        _entitiesToSaveToSettings.clear();
+    }
     // save new settings
     uint32_t numEntities = _avatarEntityStrings.size();
     _avatarEntityCountSetting.set(numEntities);
@@ -1429,17 +1433,28 @@ void MyAvatar::setEnableInverseKinematics(bool isEnabled) {
     _skeletonModel->getRig().setEnableInverseKinematics(isEnabled);
 }
 
-void MyAvatar::updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) {
-    /* TODO: implement this so JS can add/update (and delete?) AvatarEntitieskj:w
-    // convert string to properties
-    // NOTE: this path from EntityItemProperties JSON string to EntityItemProperties is NOT efficient
-    EntityItemProperties properties;
-    properties.copyFromJSONString(scriptEngine, entityPropertiesString);
+void MyAvatar::storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload) {
+    AvatarData::storeAvatarEntityDataPayload(entityID, payload);
+    _entitiesToSaveToSettings.insert(entityID);
+}
 
+/* TODO: verify we don't need this
+void MyAvatar::updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) {
     auto entityTreeRenderer = qApp->getEntities();
     EntityTreePointer entityTree = entityTreeRenderer ? entityTreeRenderer->getTree() : nullptr;
     EntityItemPointer entity;
     if (entityTree) {
+        return;
+    }
+    // convert string to properties
+    EntityItemProperties properties;
+    {
+        // NOTE: this path from EntityItemProperties JSON string to EntityItemProperties is NOT efficient
+        QScriptEngine scriptEngine;
+        properties.copyFromJSONString(scriptEngine, entityPropertiesString);
+    }
+
+    entityTree->withWriteLock([&] {
         entity = entityTree->findEntityByID(entityID);
         if (!entity) {
             entity = entityTree->addEntity(entityID, properties);
@@ -1450,25 +1465,25 @@ void MyAvatar::updateAvatarEntity(const QUuid& entityID, const QString& entityPr
             }
             // TODO: remember this entity and its properties, so we can save to settings
         } else {
-            // TODO: propagate changes to entity
-            // TODO: and remember these changes so we can save to settings
+            entityTree->updateEntity(entityID, properties);
         }
-    }
+        if (entity) {
+            // build update packet for later
+            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 = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize());
+                storeAvatarEntityDataPayload(entity->getID(), tempArray);
+                properties = entity->getProperties();
+                _entitiesToSaveToSettings.insert(entityID);
+            }
 
-    OctreePacketData packetData(false, AvatarTraits::MAXIMUM_TRAIT_SIZE);
-    EncodeBitstreamParams params;
-    EntityTreeElementExtraEncodeDataPointer extra { nullptr };
-    OctreeElement::AppendState appendState = entity->appendEntityData(&packetData, params, extra);
-
-    if (appendState != OctreeElement::COMPLETED) {
-        // this entity's data is too big
-        return;
-    }
-
-    QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize());
-    storeAvatarEntityDataPayload(entity->getID(), tempArray);
-    */
+        }
+    });
 }
+*/
 
 void MyAvatar::updateAvatarEntities() {
     // TODO: modify this info for MyAvatar
diff --git a/interface/src/avatar/MyAvatar.h b/interface/src/avatar/MyAvatar.h
index b05d9ed875..b100eebb73 100644
--- a/interface/src/avatar/MyAvatar.h
+++ b/interface/src/avatar/MyAvatar.h
@@ -1407,7 +1407,7 @@ public slots:
      */
     bool getEnableMeshVisible() const override;
 
-    void updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) override;
+    void storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload) override;
 
     /**jsdoc
      * Set whether or not your avatar mesh is visible.
@@ -1948,6 +1948,7 @@ private:
     std::vector<Setting::Handle<QString>> _avatarEntityDataSettings;
     std::map<QUuid, QString> _avatarEntityStrings;
     std::map<QUuid, QString> _avatarEntityProperties;
+    std::set<QUuid> _entitiesToSaveToSettings;
 };
 
 QScriptValue audioListenModeToScriptValue(QScriptEngine* engine, const AudioListenerMode& audioListenerMode);
diff --git a/libraries/avatars/src/AvatarData.cpp b/libraries/avatars/src/AvatarData.cpp
index e015410582..9380699504 100644
--- a/libraries/avatars/src/AvatarData.cpp
+++ b/libraries/avatars/src/AvatarData.cpp
@@ -2777,7 +2777,7 @@ void AvatarData::storeAvatarEntityDataPayload(const QUuid& entityID, const QByte
 }
 
 void AvatarData::updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString) {
-    // TODO: implement this as API exposed to JS
+    // TODO: implement this to expose AvatarEntity to JS
 }
 
 void AvatarData::clearAvatarEntity(const QUuid& entityID, bool requiresRemovalFromTree) {
diff --git a/libraries/avatars/src/AvatarData.h b/libraries/avatars/src/AvatarData.h
index 1137cff869..f2f856678b 100644
--- a/libraries/avatars/src/AvatarData.h
+++ b/libraries/avatars/src/AvatarData.h
@@ -952,14 +952,14 @@ public:
     // FIXME: Can this name be improved? Can it be deprecated?
     Q_INVOKABLE virtual void setAttachmentsVariant(const QVariantList& variant);
 
-    void storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload);
+    virtual void storeAvatarEntityDataPayload(const QUuid& entityID, const QByteArray& payload);
 
     /**jsdoc
      * @function MyAvatar.updateAvatarEntity
      * @param {Uuid} entityID
      * @param {string} entityData
      */
-    Q_INVOKABLE void updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString);
+    Q_INVOKABLE virtual void updateAvatarEntity(const QUuid& entityID, const QString& entityPropertiesString);
 
     /**jsdoc
      * @function MyAvatar.clearAvatarEntity
diff --git a/libraries/entities/src/EntityEditPacketSender.cpp b/libraries/entities/src/EntityEditPacketSender.cpp
index dfc2d45d36..40dd26a467 100644
--- a/libraries/entities/src/EntityEditPacketSender.cpp
+++ b/libraries/entities/src/EntityEditPacketSender.cpp
@@ -69,8 +69,8 @@ void EntityEditPacketSender::queueEditAvatarEntityMessage(EntityTreePointer enti
         return;
     }
 
-    packetData.shrinkByteArrays();
-    _myAvatar->storeAvatarEntityDataPayload(entity->getID(), packetData.getUncompressedByteArray());
+    QByteArray tempArray = QByteArray::fromRawData((const char*)packetData.getUncompressedData(), packetData.getUncompressedSize());
+    _myAvatar->storeAvatarEntityDataPayload(entityItemID, tempArray);
 }
 
 void EntityEditPacketSender::queueEditEntityMessage(PacketType type,
diff --git a/libraries/octree/src/OctreePacketData.cpp b/libraries/octree/src/OctreePacketData.cpp
index 60b0e6fbad..fd57f2fa3a 100644
--- a/libraries/octree/src/OctreePacketData.cpp
+++ b/libraries/octree/src/OctreePacketData.cpp
@@ -654,15 +654,6 @@ void OctreePacketData::loadFinalizedContent(const unsigned char* data, int lengt
     }
 }
 
-void OctreePacketData::shrinkByteArrays() {
-    _uncompressedByteArray.resize(_bytesInUse);
-    _compressedByteArray.resize(_compressedBytes);
-    // if you call this method then you are expected to be done packing to raw pointers
-    // and you just want the ByteArrays
-    // therefore we reset
-    reset();
-}
-
 void OctreePacketData::debugContent() {
     qCDebug(octree, "OctreePacketData::debugContent()... COMPRESSED DATA.... size=%d",_compressedBytes);
     int perline=0;
diff --git a/libraries/octree/src/OctreePacketData.h b/libraries/octree/src/OctreePacketData.h
index cca78e19b3..01ed4977b0 100644
--- a/libraries/octree/src/OctreePacketData.h
+++ b/libraries/octree/src/OctreePacketData.h
@@ -245,8 +245,6 @@ public:
 
     int getBytesAvailable() { return _bytesAvailable; }
 
-    void shrinkByteArrays();
-
     /// displays contents for debugging
     void debugContent();
     void debugBytes();