Merge pull request #1760 from OpenSpace/issue/1757

Fixes Problem with Profile Properties Not Handling Table Entries (Issue 1757)

apps/OpenSpace/ext/launcher/src/profile/timedialog.cpp

@@ -50,7 +50,7 @@ TimeDialog::TimeDialog(QWidget* parent, std::optional<openspace::Profile::Time>*
         _timeData = **_time;
         if (_timeData.type == Profile::Time::Type::Relative) {
             if (_timeData.value == "") {
-                _timeData.value = "now";
+                _timeData.value = "0d";
             }
             _relativeEdit->setSelection(0, _relativeEdit->text().length());
         }
@@ -60,7 +60,7 @@ TimeDialog::TimeDialog(QWidget* parent, std::optional<openspace::Profile::Time>*
     }
     else {
         _timeData.type = Profile::Time::Type::Relative;
-        _timeData.value = "now";
+        _timeData.value = "0d";
     }
     _initializedAsAbsolute = (_timeData.type == Profile::Time::Type::Absolute);
     enableAccordingToType(static_cast<int>(_timeData.type));
@@ -114,7 +114,7 @@ void TimeDialog::enableAccordingToType(int idx) {
     if (comboIdx == Profile::Time::Type::Relative) {
         _relativeEdit->setText("<font color='black'>Relative Time:</font>");
         if (_initializedAsAbsolute) {
-            _relativeEdit->setText("now");
+            _relativeEdit->setText("0d");
         }
         else {
             _relativeEdit->setText(QString::fromStdString(_timeData.value));

include/openspace/scene/scene.h

@@ -33,6 +33,7 @@
 #include <ghoul/misc/easing.h>
 #include <ghoul/misc/exception.h>
 #include <ghoul/misc/memorypool.h>
+#include <functional>
 #include <mutex>
 #include <set>
 #include <unordered_map>
@@ -46,6 +47,15 @@ namespace openspace {
 namespace documentation { struct Documentation; }
 namespace scripting { struct LuaLibrary; }
 
+enum class PropertyValueType {
+    Boolean = 0,
+    Float,
+    String,
+    Table,
+    Nil
+};
+using ProfilePropertyLua = std::variant<bool, float, std::string, ghoul::lua::nil_t>;
+
 class SceneInitializer;
 
 // Notifications:
@@ -246,6 +256,59 @@ public:
     void setPropertiesFromProfile(const Profile& p);
 
 private:
+    /**
+     * Accepts string version of a property value from a profile, converts it to the
+     * appropriate type, and then pushes the value onto the lua state.
+     *
+     * \param L the lua state to push value to
+     * \param value string representation of the value with which to set property
+     */
+    void propertyPushProfileValueToLua(ghoul::lua::LuaState& L, const std::string& value);
+
+    /**
+     * Accepts string version of a property value from a profile, and processes it
+     * according to the data type of the value
+     *
+     * \param L the lua state to (eventually) push to
+     * \param value string representation of the value with which to set property
+     * \param didPushToLua Bool reference that represents the lua push state at the end
+     *        of this function call. This will be set to true if the value (e.g. table)
+     *        has already been pushed to the lua stack
+     * \return The ProfilePropertyLua variant type translated from string representation
+     */
+    ProfilePropertyLua propertyProcessValue(ghoul::lua::LuaState& L,
+        const std::string& value);
+
+    /**
+     * Accepts string version of a property value from a profile, and returns the
+     * supported data types that can be pushed to a lua state. Currently, the full
+     * range of possible lua values is not supported.
+     *
+     * \param value string representation of the value with which to set property
+     */
+    PropertyValueType propertyValueType(const std::string& value);
+
+    /**
+     * Accepts string version of a property value from a profile, and adds it to a vector
+     * which will later be used to push as a lua table containing values of type T
+     *
+     * \param L the lua state to (eventually) push to
+     * \param value string representation of the value with which to set property
+     * \param table the std::vector container which has elements of type T for a lua table
+     */
+    template <typename T>
+    void processPropertyValueTableEntries(ghoul::lua::LuaState& L,
+        const std::string& value, std::vector<T>& table);
+
+    /**
+     * Handles a lua table entry, creating a vector of the correct variable type based
+     * on the profile string, and pushes this vector to the lua stack.
+     *
+     * \param L the lua state to (eventually) push to
+     * \param value string representation of the value with which to set property
+     */
+    void handlePropertyLuaTableEntry(ghoul::lua::LuaState& L, const std::string& value);
+
     /**
      * Update dependencies.
      */
@@ -260,8 +323,9 @@ private:
     bool _dirtyNodeRegistry = false;
     SceneGraphNode _rootDummy;
     std::unique_ptr<SceneInitializer> _initializer;
-
+    std::string _profilePropertyName;
     std::vector<InterestingTime> _interestingTimes;
+    bool _valueIsTable = false;
 
     std::mutex _programUpdateLock;
     std::set<ghoul::opengl::ProgramObject*> _programsToUpdate;
@@ -279,16 +343,6 @@ private:
     ghoul::MemoryPool<4096> _memoryPool;
 };
 
-/**
- * Accepts string version of a property value from a profile, converts it to the
- * appropriate type, and then pushes the value onto the lua state.
- *
- * \param L the lua state to push value to
- * \param value string representation of the value with which to set property
- */
-void propertyPushValueFromProfileToLuaState(ghoul::lua::LuaState& L,
-    const std::string& value);
-
 } // namespace openspace
 
 #endif // __OPENSPACE_CORE___SCENE___H__

src/engine/openspaceengine.cpp

@@ -1553,6 +1553,9 @@ void OpenSpaceEngine::toggleShutdownMode() {
 }
 
 void setCameraFromProfile(const Profile& p) {
+    if (!p.camera.has_value()) {
+        throw ghoul::RuntimeError("No 'camera' entry exists in the startup profile");
+    }
     std::visit(
         overloaded{
             [](const Profile::CameraNavState& navStateProfile) {

src/scene/scene.cpp

@@ -39,6 +39,7 @@
 #include <openspace/util/updatestructures.h>
 #include <ghoul/opengl/programobject.h>
 #include <ghoul/logging/logmanager.h>
+#include <ghoul/misc/misc.h>
 #include <ghoul/misc/profiling.h>
 #include <string>
 #include <stack>
@@ -73,6 +74,9 @@ namespace {
         }
     }
 #endif // TRACY_ENABLE
+
+    template <class... Ts> struct overloaded : Ts... { using Ts::operator()...; };
+    template <class... Ts> overloaded(Ts...)->overloaded<Ts...>;
 } // namespace
 
 namespace openspace {
@@ -613,10 +617,14 @@ void Scene::setPropertiesFromProfile(const Profile& p) {
             // Remove group name from start of regex and replace with '*'
             uriOrRegex = removeGroupNameFromUri(uriOrRegex);
         }
+        _profilePropertyName = uriOrRegex;
         ghoul::lua::push(L, uriOrRegex);
         ghoul::lua::push(L, 0.0);
+
+        std::string workingValue = prop.value;
+        ghoul::trimSurroundingCharacters(workingValue, ' ');
         // Later functions expect the value to be at the last position on the stack
-        propertyPushValueFromProfileToLuaState(L, prop.value);
+        propertyPushProfileValueToLua(L, workingValue);
 
         applyRegularExpression(
             L,
@@ -631,21 +639,149 @@ void Scene::setPropertiesFromProfile(const Profile& p) {
     }
 }
 
-void propertyPushValueFromProfileToLuaState(ghoul::lua::LuaState& L,
-                                            const std::string& value)
+void Scene::propertyPushProfileValueToLua(ghoul::lua::LuaState& L,
+                                                                 const std::string& value)
 {
-    if (luascriptfunctions::isBoolValue(value)) {
-        ghoul::lua::push(L, (value == "true") ? true : false);
+    _valueIsTable = false;
+    ProfilePropertyLua elem = propertyProcessValue(L, value);
+    if (!_valueIsTable) {
+        std::visit(overloaded{
+            [&L](const bool value) {
+                ghoul::lua::push(L, value);
+            },
+            [&L](const float value) {
+                ghoul::lua::push(L, value);
+            },
+            [&L](const std::string value) {
+                ghoul::lua::push(L, value);
+            },
+            [&L](const ghoul::lua::nil_t nilValue) {
+                ghoul::lua::push(L, nilValue);
+            }
+        }, elem);
     }
-    else if (luascriptfunctions::isFloatValue(value)) {
-        ghoul::lua::push(L, std::stof(value));
+}
+
+ProfilePropertyLua Scene::propertyProcessValue(ghoul::lua::LuaState& L,
+                                                                 const std::string& value)
+{
+    ProfilePropertyLua result;
+    PropertyValueType pType = propertyValueType(value);
+
+    switch (pType) {
+        case PropertyValueType::Boolean:
+            result = (value == "true") ? true : false;
+            break;
+        case PropertyValueType::Float:
+            result = std::stof(value);
+            break;
+        case PropertyValueType::Nil:
+            result = ghoul::lua::nil_t();
+            break;
+        case PropertyValueType::Table:
+            ghoul::trimSurroundingCharacters(const_cast<std::string&>(value), '{');
+            ghoul::trimSurroundingCharacters(const_cast<std::string&>(value), '}');
+            handlePropertyLuaTableEntry(L, value);
+            _valueIsTable = true;
+            break;
+        case PropertyValueType::String:
+        default:
+            ghoul::trimSurroundingCharacters(const_cast<std::string&>(value), '\"');
+            ghoul::trimSurroundingCharacters(const_cast<std::string&>(value), '[');
+            ghoul::trimSurroundingCharacters(const_cast<std::string&>(value), ']');
+            result = value;
+            break;
+    }
+    return result;
+}
+
+void Scene::handlePropertyLuaTableEntry(ghoul::lua::LuaState& L, const std::string& value)
+{
+    PropertyValueType enclosedType;
+    size_t commaPos = value.find(',', 0);
+    if (commaPos != std::string::npos) {
+        enclosedType = propertyValueType(value.substr(0, commaPos));
     }
     else {
-        std::string stringRepresentation = value;
-        if (value.compare("nil") != 0) {
-            stringRepresentation = "[[" + stringRepresentation + "]]";
+        enclosedType = propertyValueType(value);
+    }
+
+    switch (enclosedType) {
+        case PropertyValueType::Boolean:
+            LERROR(fmt::format(
+                "A lua table of bool values is not supported. (processing property {})",
+                _profilePropertyName)
+            );
+            break;
+        case PropertyValueType::Float:
+            {
+                std::vector<float> valsF;
+                processPropertyValueTableEntries(L, value, valsF);
+                ghoul::lua::push(L, valsF);
+            }
+            break;
+        case PropertyValueType::String:
+            {
+                std::vector<std::string> valsS;
+                processPropertyValueTableEntries(L, value, valsS);
+                ghoul::lua::push(L, valsS);
+            }
+            break;
+        case PropertyValueType::Table:
+        default:
+            LERROR(fmt::format(
+                "Table-within-a-table values are not supported for profile a "
+                "property (processing property {})", _profilePropertyName
+            ));
+            break;
+    }
+}
+
+template <typename T>
+void Scene::processPropertyValueTableEntries(ghoul::lua::LuaState& L,
+    const std::string& value, std::vector<T>& table)
+{
+    size_t commaPos = 0;
+    size_t prevPos = 0;
+    std::string nextValue;
+    while (commaPos != std::string::npos) {
+        commaPos = value.find(',', prevPos);
+        if (commaPos != std::string::npos) {
+            nextValue = value.substr(prevPos, commaPos - prevPos);
+            prevPos = commaPos + 1;
         }
-        ghoul::lua::push(L, stringRepresentation);
+        else {
+            nextValue = value.substr(prevPos);
+        }
+        ghoul::trimSurroundingCharacters(nextValue, ' ');
+        ProfilePropertyLua tableElement = propertyProcessValue(L, nextValue);
+        try {
+            table.push_back(std::get<T>(tableElement));
+        }
+        catch (std::bad_variant_access& e) {
+            LERROR(fmt::format(
+                "Error attempting to parse profile property setting for "
+                "{} using value = {}", _profilePropertyName, value
+            ));
+        }
+    }
+}
+
+PropertyValueType Scene::propertyValueType(const std::string& value) {
+    if (luascriptfunctions::isBoolValue(value)) {
+        return PropertyValueType::Boolean;
+    }
+    else if (luascriptfunctions::isFloatValue(value)) {
+        return PropertyValueType::Float;
+    }
+    else if (luascriptfunctions::isNilValue(value)) {
+        return PropertyValueType::Nil;
+    }
+    else if (luascriptfunctions::isTableValue(value)) {
+        return PropertyValueType::Table;
+    }
+    else {
+        return PropertyValueType::String;
     }
 }
 

src/scene/scene_lua.inl

@@ -904,7 +904,7 @@ int worldRotation(lua_State* L) {
  * isBoolValue(const std::string& s):
  * Used to check if a string is a lua bool type. Returns false if not a valid bool string.
  */
-bool isBoolValue(const std::string& s) {
+bool isBoolValue(std::string_view s) {
     return (s == "true" || s == "false");
 }
 
@@ -915,12 +915,32 @@ bool isBoolValue(const std::string& s) {
  */
 bool isFloatValue(const std::string& s) {
     try {
-        float converted = std::stof(s);
-        return true;
+        float converted = std::numeric_limits<float>::min();
+        converted = std::stof(s);
+        return (converted != std::numeric_limits<float>::min());
     }
     catch (...) {
         return false;
     }
 }
 
+/**
+ * \ingroup LuaScripts
+ * isNilValue(const std::string& s):
+ * Used to check if a string is a lua 'nil' value. Returns false if not.
+ */
+bool isNilValue(std::string_view s) {
+    return (s == "nil");
+}
+
+/**
+ * \ingroup LuaScripts
+ * isTableValue(const std::string& s):
+ * Used to check if a string contains a lua table rather than an individual value.
+ * Returns false if not.
+ */
+bool isTableValue(std::string_view s) {
+    return ((s.front() == '{') && (s.back() == '}'));
+}
+
 }  // namespace openspace::luascriptfunctions

src/util/time_lua.inl

@@ -447,24 +447,29 @@ int time_advancedTime(lua_State* L) {
             }
         );
 
-        double value = std::stod(std::string(modifier.begin(), it));
+        try {
+            double value = std::stod(std::string(modifier.begin(), it));
+            std::string unitName = std::string(it, modifier.end());
 
-        std::string unitName = std::string(it, modifier.end());
+            TimeUnit unit = TimeUnit::Second;
+            if (unitName == "s") { unit = TimeUnit::Second; }
+            else if (unitName == "m") { unit = TimeUnit::Minute; }
+            else if (unitName == "h") { unit = TimeUnit::Hour; }
+            else if (unitName == "d") { unit = TimeUnit::Day; }
+            else if (unitName == "M") { unit = TimeUnit::Month; }
+            else if (unitName == "y") { unit = TimeUnit::Year; }
+            else {
+                return ghoul::lua::luaError(L, fmt::format("Unknown unit '{}'", unitName));
+            }
 
-        TimeUnit unit = TimeUnit::Second;
-        if (unitName == "s")      { unit = TimeUnit::Second; }
-        else if (unitName == "m") { unit = TimeUnit::Minute; }
-        else if (unitName == "h") { unit = TimeUnit::Hour; }
-        else if (unitName == "d") { unit = TimeUnit::Day; }
-        else if (unitName == "M") { unit = TimeUnit::Month; }
-        else if (unitName == "y") { unit = TimeUnit::Year; }
-        else {
-            return ghoul::lua::luaError(L, fmt::format("Unknown unit '{}'", unitName));
+            dt = convertTime(value, unit, TimeUnit::Second);
+            if (isNegative) {
+                dt *= -1.0;
+            }
         }
-
-        dt = convertTime(value, unit, TimeUnit::Second);
-        if (isNegative) {
-            dt *= -1.0;
+        catch (...) {
+            return ghoul::lua::luaError(L, fmt::format("Error parsing relative time "
+                "offset '{}'", modifier));
         }
     }
 

src/util/timemanager.cpp

@@ -871,17 +871,22 @@ double TimeManager::previousApplicationTimeForInterpolation() const {
 void TimeManager::setTimeFromProfile(const Profile& p) {
     Time t;
 
-    switch (p.time.value().type) {
-    case Profile::Time::Type::Relative:
-        t.setTimeRelativeFromProfile(p.time.value().value);
-        break;
+    if (p.time.has_value()) {
+        switch (p.time.value().type) {
+        case Profile::Time::Type::Relative:
+            t.setTimeRelativeFromProfile(p.time.value().value);
+            break;
 
-    case Profile::Time::Type::Absolute:
-        t.setTimeAbsoluteFromProfile(p.time.value().value);
-        break;
+        case Profile::Time::Type::Absolute:
+            t.setTimeAbsoluteFromProfile(p.time.value().value);
+            break;
 
-    default:
-        throw ghoul::MissingCaseException();
+        default:
+            throw ghoul::MissingCaseException();
+        }
+    }
+    else {
+        throw ghoul::RuntimeError("No 'time' entry exists in the startup profile");
     }
 }