From 0b34524ba0cef85fb73b4a9cbb83632ae329f627 Mon Sep 17 00:00:00 2001
From: Andrew Meadows <andrew@highfidelity.io>
Date: Wed, 26 Sep 2018 16:09:44 -0700
Subject: [PATCH] minimimze time holding locks on tree in editEntity()

---
 .../entities/src/EntityScriptingInterface.cpp | 20 +++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/libraries/entities/src/EntityScriptingInterface.cpp b/libraries/entities/src/EntityScriptingInterface.cpp
index a668c806cb..199e9eaaf8 100644
--- a/libraries/entities/src/EntityScriptingInterface.cpp
+++ b/libraries/entities/src/EntityScriptingInterface.cpp
@@ -541,8 +541,7 @@ QUuid EntityScriptingInterface::editEntity(QUuid id, const EntityItemProperties&
     }
     // If we have a local entity tree set, then also update it.
 
-    bool updatedEntity = false;
-    _entityTree->withWriteLock([&] {
+    _entityTree->withReadLock([&] {
         EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID);
         if (!entity) {
             return;
@@ -575,7 +574,14 @@ QUuid EntityScriptingInterface::editEntity(QUuid id, const EntityItemProperties&
         }
         properties.setClientOnly(entity->getClientOnly());
         properties.setOwningAvatarID(entity->getOwningAvatarID());
-        properties = convertPropertiesFromScriptSemantics(properties, properties.getScalesWithParent());
+    });
+    properties = convertPropertiesFromScriptSemantics(properties, properties.getScalesWithParent());
+
+    // TODO: avoid setting properties not allowed to be changed according to simulation ownership rules
+
+    // done reading, start write
+    bool updatedEntity = false;
+    _entityTree->withWriteLock([&] {
         updatedEntity = _entityTree->updateEntity(entityID, properties);
     });
 
@@ -588,6 +594,7 @@ QUuid EntityScriptingInterface::editEntity(QUuid id, const EntityItemProperties&
     //     return QUuid();
     // }
 
+    // done writing, send update
     bool entityFound { false };
     _entityTree->withReadLock([&] {
         EntityItemPointer entity = _entityTree->findEntityByEntityItemID(entityID);
@@ -641,7 +648,10 @@ QUuid EntityScriptingInterface::editEntity(QUuid id, const EntityItemProperties&
                     }
                 }
             });
-        } else {
+        }
+    });
+    if (!entityFound) {
+        {
             // Sometimes ESS don't have the entity they are trying to edit in their local tree.  In this case,
             // convertPropertiesFromScriptSemantics doesn't get called and local* edits will get dropped.
             // This is because, on the script side, "position" is in world frame, but in the network
@@ -663,8 +673,6 @@ QUuid EntityScriptingInterface::editEntity(QUuid id, const EntityItemProperties&
                 properties.setDimensions(properties.getLocalDimensions());
             }
         }
-    });
-    if (!entityFound) {
         // we've made an edit to an entity we don't know about, or to a non-entity.  If it's a known non-entity,
         // print a warning and don't send an edit packet to the entity-server.
         QSharedPointer<SpatialParentFinder> parentFinder = DependencyManager::get<SpatialParentFinder>();