Code review improvements.

libraries/entities/src/EntityItemProperties.cpp

@@ -208,20 +208,18 @@ void EntityItemProperties::setBackgroundModeFromString(const QString& background
     }
 }
 
-using ComponentPair = std::pair<const ComponentMode, const QString>;
+QString EntityItemProperties::getComponentModeAsString(uint32_t mode) {
-const std::array<ComponentPair, COMPONENT_MODE_ITEM_COUNT> COMPONENT_MODES = { {
+    // return "inherit" if mode is not valid
-        ComponentPair{ COMPONENT_MODE_INHERIT,{ "inherit" } },
+    if (mode < COMPONENT_MODE_ITEM_COUNT) {
-        ComponentPair{ COMPONENT_MODE_DISABLED,{ "disabled" } },
+        return COMPONENT_MODES[mode].second;
-        ComponentPair{ COMPONENT_MODE_ENABLED,{ "enabled" } }
-} };
-
-QString EntityItemProperties::getHazeModeAsString() const {
-    // return "inherit" if _hazeMode is not valid
-    if (_hazeMode < COMPONENT_MODE_ITEM_COUNT) {
-        return COMPONENT_MODES[_hazeMode].second;
     } else {
         return COMPONENT_MODES[COMPONENT_MODE_INHERIT].second;
     }
+
+}
+
+QString EntityItemProperties::getHazeModeAsString() const {
+    return getComponentModeAsString(_hazeMode);
 }
 
 QString EntityItemProperties::getComponentModeString(uint32_t mode) {
@@ -233,10 +231,14 @@ QString EntityItemProperties::getComponentModeString(uint32_t mode) {
     }
 }
 
-void EntityItemProperties::setHazeModeFromString(const QString& hazeMode) {
+const auto EntityItemProperties::findComponent(const QString& mode) {
-    auto result = std::find_if(COMPONENT_MODES.begin(), COMPONENT_MODES.end(), [&](const ComponentPair& pair) {
+    return std::find_if(COMPONENT_MODES.begin(), COMPONENT_MODES.end(), [&](const ComponentPair& pair) { 
-        return (pair.second == hazeMode);
+        return (pair.second == mode);
     });
+}
+
+void EntityItemProperties::setHazeModeFromString(const QString& hazeMode) {
+    auto result = findComponent(hazeMode);
 
     if (result != COMPONENT_MODES.end()) {
         _hazeMode = result->first;
@@ -245,18 +247,11 @@ void EntityItemProperties::setHazeModeFromString(const QString& hazeMode) {
 }
 
 QString EntityItemProperties::getKeyLightModeAsString() const {
-    // return "enabled" if _keyLightMode is not valid
+    return getComponentModeAsString(_keyLightMode);
-    if (_keyLightMode < COMPONENT_MODE_ITEM_COUNT) {
-        return COMPONENT_MODES[_keyLightMode].second;
-    } else {
-        return COMPONENT_MODES[COMPONENT_MODE_ENABLED].second;
-    }
 }
 
 void EntityItemProperties::setKeyLightModeFromString(const QString& keyLightMode) {
-    auto result = std::find_if(COMPONENT_MODES.begin(), COMPONENT_MODES.end(), [&](const ComponentPair& pair) {
+    auto result = findComponent(keyLightMode);
-        return (pair.second == keyLightMode);
-    });
 
     if (result != COMPONENT_MODES.end()) {
         _keyLightMode = result->first;
@@ -265,18 +260,11 @@ void EntityItemProperties::setKeyLightModeFromString(const QString& keyLightMode
 }
 
 QString EntityItemProperties::getAmbientLightModeAsString() const {
-    // return "enabled" if _ambientLightMode is not valid
+    return getComponentModeAsString(_ambientLightMode);
-    if (_ambientLightMode < COMPONENT_MODE_ITEM_COUNT) {
-        return COMPONENT_MODES[_ambientLightMode].second;
-    } else {
-        return COMPONENT_MODES[COMPONENT_MODE_ENABLED].second;
-    }
 }
 
 void EntityItemProperties::setAmbientLightModeFromString(const QString& ambientLightMode) {
-    auto result = std::find_if(COMPONENT_MODES.begin(), COMPONENT_MODES.end(), [&](const ComponentPair& pair) {
+    auto result = findComponent(ambientLightMode);
-        return (pair.second == ambientLightMode);
-    });
 
     if (result != COMPONENT_MODES.end()) {
         _ambientLightMode = result->first;
@@ -285,18 +273,11 @@ void EntityItemProperties::setAmbientLightModeFromString(const QString& ambientL
 }
 
 QString EntityItemProperties::getSkyboxModeAsString() const {
-    // return "enabled" if _skyboxMode is not valid
+    return getComponentModeAsString(_skyboxMode);
-    if (_skyboxMode < COMPONENT_MODE_ITEM_COUNT) {
-        return COMPONENT_MODES[_skyboxMode].second;
-    } else {
-        return COMPONENT_MODES[COMPONENT_MODE_ENABLED].second;
-    }
 }
 
 void EntityItemProperties::setSkyboxModeFromString(const QString& skyboxMode) {
-    auto result = std::find_if(COMPONENT_MODES.begin(), COMPONENT_MODES.end(), [&](const ComponentPair& pair) {
+    auto result = findComponent(skyboxMode);
-        return (pair.second == skyboxMode);
-    });
 
     if (result != COMPONENT_MODES.end()) {
         _skyboxMode = result->first;

libraries/entities/src/EntityItemProperties.h

@@ -47,6 +47,13 @@
 
 const quint64 UNKNOWN_CREATED_TIME = 0;
 
+using ComponentPair = std::pair<const ComponentMode, const QString>;
+const std::array<ComponentPair, COMPONENT_MODE_ITEM_COUNT> COMPONENT_MODES = { {
+    ComponentPair { COMPONENT_MODE_INHERIT, { "inherit" } },
+    ComponentPair { COMPONENT_MODE_DISABLED, { "disabled" } },
+    ComponentPair { COMPONENT_MODE_ENABLED, { "enabled" } }
+} };
+
 /// A collection of properties of an entity item used in the scripting API. Translates between the actual properties of an
 /// entity and a JavaScript style hash/QScriptValue storing a set of properties. Used in scripting to set/get the complete
 /// set of entity item properties via JavaScript hashes/QScriptValues
@@ -255,7 +262,11 @@ public:
     DEFINE_PROPERTY_REF(PROP_SERVER_SCRIPTS, ServerScripts, serverScripts, QString, ENTITY_ITEM_DEFAULT_SERVER_SCRIPTS);
 
     static QString getBackgroundModeString(BackgroundMode mode);
+
     static QString getComponentModeString(uint32_t mode);
+    static QString getComponentModeAsString(uint32_t mode);
+
+    const auto findComponent(const QString& mode);
 
 public:
     float getMaxDimension() const { return glm::compMax(_dimensions); }

libraries/entities/src/EntityTree.cpp

@@ -2281,22 +2281,24 @@ bool EntityTree::readFromMap(QVariantMap& map) {
             properties.setOwningAvatarID(myNodeID);
         }
 
-        // TEMPORARY fix for older content not containing these fields
+        // TEMPORARY fix for older content not containing these fields in the zones
-        if (!entityMap.contains("keyLightMode")) {
+        if (properties.getType() == EntityTypes::EntityType::Zone) {
-            properties.setKeyLightMode(COMPONENT_MODE_ENABLED);
+            if (!entityMap.contains("keyLightMode")) {
-        }
+                properties.setKeyLightMode(COMPONENT_MODE_ENABLED);
-
-        if (!entityMap.contains("skyboxMode")) {
-            if (entityMap.contains("backgroundMode") && properties.getBackgroundModeAsString() == "inherit") {
-                // The content creator has set the combo to NOTHING - this is actually inherit
-                properties.setSkyboxMode(COMPONENT_MODE_INHERIT);
-            } else {
-                properties.setSkyboxMode(COMPONENT_MODE_ENABLED);
             }
-        }
 
-        if (!entityMap.contains("ambientLightMode")) {
+            if (!entityMap.contains("skyboxMode")) {
-            properties.setAmbientLightMode(COMPONENT_MODE_ENABLED);
+                if (entityMap.contains("backgroundMode") && properties.getBackgroundModeAsString() == "inherit") {
+                    // The content creator has set the combo to NOTHING - this is actually inherit
+                    properties.setSkyboxMode(COMPONENT_MODE_INHERIT);
+                } else {
+                    properties.setSkyboxMode(COMPONENT_MODE_ENABLED);
+                }
+            }
+
+            if (!entityMap.contains("ambientLightMode")) {
+                properties.setAmbientLightMode(COMPONENT_MODE_ENABLED);
+            }
         }
 
         EntityItemPointer entity = addEntity(entityItemID, properties);