From 507dfa5ec3e1069ff1b7dda4fe1d7a269da971d0 Mon Sep 17 00:00:00 2001
From: Seth Alves <seth.alves@gmail.com>
Date: Fri, 2 Mar 2018 11:35:34 -0800
Subject: [PATCH] more code review response

---
 interface/src/Application.cpp                              | 7 +++----
 interface/src/Application.h                                | 4 +---
 libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp | 6 ++++--
 libraries/networking/src/AddressManager.cpp                | 6 ++++--
 4 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/interface/src/Application.cpp b/interface/src/Application.cpp
index 3ad0fa9392..927372ca39 100644
--- a/interface/src/Application.cpp
+++ b/interface/src/Application.cpp
@@ -3109,10 +3109,7 @@ bool Application::importFromZIP(const QString& filePath) {
 }
 
 void Application::setServerlessDomain(bool serverlessDomain) {
-    if (_serverlessDomain != serverlessDomain) {
-        _serverlessDomain = serverlessDomain;
-        getEntities()->getTree()->setIsServerlessMode(_serverlessDomain);
-    }
+    getEntities()->getTree()->setIsServerlessMode(serverlessDomain);
 }
 
 void Application::loadServerlessDomain(QUrl domainURL) {
@@ -3136,6 +3133,8 @@ void Application::loadServerlessDomain(QUrl domainURL) {
     permissions.setAll(true);
     DependencyManager::get<NodeList>()->setPermissions(permissions);
 
+    // we can't import directly into the main tree because we would need to lock it, and
+    // Octree::readFromURL calls loop.exec which can run code which will also attempt to lock the tree.
     EntityTreePointer tmpTree(new EntityTree());
     tmpTree->setIsServerlessMode(true);
     tmpTree->createRootElement();
diff --git a/interface/src/Application.h b/interface/src/Application.h
index 948916ba7c..ec385503ee 100644
--- a/interface/src/Application.h
+++ b/interface/src/Application.h
@@ -286,7 +286,7 @@ public:
     bool getSaveAvatarOverrideUrl() { return _saveAvatarOverrideUrl; }
     void saveNextPhysicsStats(QString filename);
 
-    bool isServerlessMode() const { return _serverlessDomain; }
+    bool isServerlessMode() const { return getEntities()->getTree()->isServerlessMode(); }
 
     void replaceDomainContent(const QString& url);
 
@@ -720,7 +720,5 @@ private:
 
     std::atomic<bool> _pendingIdleEvent { true };
     std::atomic<bool> _pendingRenderEvent { true };
-
-    bool _serverlessDomain { true };
 };
 #endif // hifi_Application_h
diff --git a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp
index 8092f3dee1..9cb8ed7376 100644
--- a/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp
+++ b/libraries/avatars-renderer/src/avatars-renderer/Avatar.cpp
@@ -221,10 +221,12 @@ void Avatar::updateAvatarEntities() {
         return;
     }
 
-    if (getID() == QUuid() ||
+    if (getID().isNull() ||
         getID() == AVATAR_SELF_ID ||
         DependencyManager::get<NodeList>()->getSessionUUID() == QUuid()) {
-        return; // wait until MyAvatar and this Node gets an ID before doing this.
+        // wait until MyAvatar and this Node gets an ID before doing this.  Otherwise, various things go wrong --
+        // things get their parent fixed up from AVATAR_SELF_ID to a null uuid which means "no parent".
+        return;
     }
 
     auto treeRenderer = DependencyManager::get<EntityTreeRenderer>();
diff --git a/libraries/networking/src/AddressManager.cpp b/libraries/networking/src/AddressManager.cpp
index b736ca6c48..3a666c4740 100644
--- a/libraries/networking/src/AddressManager.cpp
+++ b/libraries/networking/src/AddressManager.cpp
@@ -303,8 +303,10 @@ bool AddressManager::handleUrl(const QUrl& lookupUrl, LookupTrigger trigger) {
 
 bool isPossiblePlaceName(QString possiblePlaceName) {
     bool result { false };
-    int len = possiblePlaceName.length();
-    if (possiblePlaceName != "localhost" && len >= 4 && len <= 64) {
+    int length = possiblePlaceName.length();
+    static const int MINIMUM_PLACENAME_LENGTH = 4;
+    static const int MAXIMUM_PLACENAME_LENGTH = 64;
+    if (possiblePlaceName != "localhost" && length >= MINIMUM_PLACENAME_LENGTH && length <= MAXIMUM_PLACENAME_LENGTH) {
         const QRegExp PLACE_NAME_REGEX = QRegExp("^[0-9A-Za-z](([0-9A-Za-z]|-(?!-))*[^\\W_]$|$)");
         result = PLACE_NAME_REGEX.indexIn(possiblePlaceName) == 0;
     }