Some cleanup of ConfigurationManager

include/openspace/engine/configurationmanager.h

@@ -25,11 +25,10 @@
 #ifndef __OPENSPACE_CORE___CONFIGURATIONMANAGER___H__
 #define __OPENSPACE_CORE___CONFIGURATIONMANAGER___H__
 
-#include <openspace/documentation/documentation.h>
-
 #include <ghoul/misc/dictionary.h>
 
 namespace openspace {
+namespace documentation {  struct Documentation; }
 
 /**
  * The ConfigurationManager takes care of loading the major configuration file
@@ -142,27 +141,17 @@ public:
      * that are specified in the configuration file will automatically be registered in
      * the ghoul::filesystem::FileSystem.
      * \param filename The filename to be loaded
-     * \throw ghoul::FileNotFoundException If the \p filename did not exist
-     * \throw ghoul::RuntimeError If the configuration file was not complete (i.e., did
+     * \throw SpecificationError If the configuration file was not complete (i.e., did
      * not specify the necessary keys KeyPaths, KeyPaths.KeyCache, KeyFonts, and
      * KeyConfigSgct)
      * \throw ghoul::lua::LuaRuntimeException If there was Lua-based error loading the
      * configuration file
      * \pre \p filename must not be empty
+     * \pre \p filename must exist
      */
     void loadFromFile(const std::string& filename);
 
-    static openspace::Documentation Documentation();
-
-private:
-    /**
-     * Checks whether the loaded configuration file is complete, that is specifying the
-     * necessary keys KeyPaths, KeyPaths.KeyCache, KeyFonts, and KeyConfigSgct. The method
-     * will log fatal errors if a key is missing.
-     * \return <code>true</code> if the configuration file was complete;
-     * <code>false</code> otherwise
-     */
-    bool checkCompleteness() const;
+    static documentation::Documentation Documentation();
 };
 
 } // namespace openspace

src/engine/configurationmanager.cpp

@@ -24,22 +24,22 @@
 
 #include <openspace/engine/configurationmanager.h>
 
+#include <openspace/documentation/documentation.h>
+
+#include <ghoul/filesystem/filesystem.h>
+#include <ghoul/logging/logmanager.h>
 #include <ghoul/lua/lua_helper.h>
 #include <ghoul/lua/luastate.h>
 #include <ghoul/misc/exception.h>
-#include <ghoul/filesystem/filesystem.h>
-#include <ghoul/logging/logmanager.h>
-
-#include <list>
 
 using std::string;
 
 #include "configurationmanager_doc.inl"
 
 namespace {
-    const string _configurationFile = "openspace.cfg";
-    const string _keyBasePath = "BASE_PATH";
-    const string _initialConfigHelper = "${BASE_PATH}/scripts/configuration_helper.lua";
+    const char* _configurationFile = "openspace.cfg";
+    const char* _keyBasePath = "BASE_PATH";
+    const char* _initialConfigHelper = "${BASE_PATH}/scripts/configuration_helper.lua";
 }
 
 namespace openspace {
@@ -89,15 +89,20 @@ string ConfigurationManager::findConfiguration(const string& filename) {
     using ghoul::filesystem::Directory;
     
     Directory directory = FileSys.currentDirectory();
-    std::string configurationName = _configurationFile;
     
     while (true) {
-        std::string&& fullPath = FileSys.pathByAppendingComponent(directory,
-                                                                  configurationName);
-        bool exists = FileSys.fileExists(fullPath);
-        if (exists)
+        std::string fullPath = FileSys.pathByAppendingComponent(
+            directory,
+            _configurationFile
+        );
+
+        if (FileSys.fileExists(fullPath)) {
+            // We have found the configuration file and can bail out
             return fullPath;
+        }
         
+        // Otherwise, we traverse the directory tree up
+    
         Directory nextDirectory = directory.parentDirectory(
             ghoul::filesystem::Directory::AbsolutePath::Yes
         );
@@ -116,10 +121,9 @@ string ConfigurationManager::findConfiguration(const string& filename) {
 void ConfigurationManager::loadFromFile(const string& filename) {
     using ghoul::filesystem::FileSystem;
     
-    if (!FileSys.fileExists(filename)) {
-        throw ghoul::FileNotFoundError(filename, "ConfigurationManager");
-    }
-
+    ghoul_assert(!filename.empty(), "Filename must not be empty");
+    ghoul_assert(FileSys.fileExists(filename), "File must exist");
+    
     // ${BASE_PATH}
     string basePathToken = FileSystem::TokenOpeningBraces + _keyBasePath
         + FileSystem::TokenClosingBraces;
@@ -148,32 +152,22 @@ void ConfigurationManager::loadFromFile(const string& filename) {
     // Register all the paths
     ghoul::Dictionary dictionary = value<ghoul::Dictionary>(KeyPaths);
 
-    std::vector<std::string> pathKeys = dictionary.keys();
-    for (std::string key : pathKeys) {
-        std::string p;
-        if (dictionary.getValue(key, p)) {
-            std::string fullKey =
-                FileSystem::TokenOpeningBraces + key + FileSystem::TokenClosingBraces;
-            LDEBUGC("ConfigurationManager", "Registering path " << fullKey << ": " << p);
-            
-            bool override = (basePathToken == fullKey);
-            if (override)
-                LINFOC("ConfigurationManager", "Overriding base path with '" << p << "'");
-
-            using Override = ghoul::filesystem::FileSystem::Override;
-            FileSys.registerPathToken(
-                std::move(fullKey),
-                std::move(p),
-                override ? Override::Yes : Override::No
-            );
+    for (std::string key : dictionary.keys()) {
+        std::string p = dictionary.value<std::string>(key);
+        std::string fullKey =
+            FileSystem::TokenOpeningBraces + key + FileSystem::TokenClosingBraces;
+        LDEBUGC("ConfigurationManager", "Registering path " << fullKey << ": " << p);
+        
+        bool override = (basePathToken == fullKey);
+        if (override) {
+            LINFOC("ConfigurationManager", "Overriding base path with '" << p << "'");
         }
-    }
 
-    bool complete = checkCompleteness();
-    if (!complete) {
-        throw ghoul::RuntimeError(
-            "Configuration file '" + filename + "' was not complete",
-            "ConfigurationManager"
+        using Override = ghoul::filesystem::FileSystem::Override;
+        FileSys.registerPathToken(
+            std::move(fullKey),
+            std::move(p),
+            override ? Override::Yes : Override::No
         );
     }
 
@@ -182,29 +176,4 @@ void ConfigurationManager::loadFromFile(const string& filename) {
     removeKey(KeyPaths);
 }
 
-bool ConfigurationManager::checkCompleteness() const {
-    std::vector<std::string> requiredTokens = {
-        KeyPaths,
-        KeyPaths + "." + KeyCache,
-        KeyFonts,
-        KeyConfigSgct
-    };
-
-    bool totalSuccess = true;
-    for (const std::string& token : requiredTokens) {
-        bool success = hasKey(token);
-
-        if (!success) {
-            LFATALC(
-                "ConfigurationManager",
-                "Configuration file did not contain required key '" << token << "'"
-            );
-        }
-
-        totalSuccess &= success;
-    }
-
-    return totalSuccess;
-}
-
 }  // namespace openspace

src/engine/configurationmanager_doc.inl

@@ -37,7 +37,8 @@ Documentation ConfigurationManager::Documentation() {
             ConfigurationManager::KeyConfigSgct,
             new StringAnnotationVerifier("A valid SGCT configuration file"),
             "The SGCT configuration file that determines the window and view frustum "
-            "settings that are being used when OpenSpace is started."
+            "settings that are being used when OpenSpace is started.",
+            Optional::No
         },
         {
             ConfigurationManager::KeyConfigScene,
@@ -47,7 +48,8 @@ Documentation ConfigurationManager::Documentation() {
             "The scene description that is used to populate the application after "
             "startup. The scene determines which objects are loaded, the startup "
             "time and other scene-specific settings. More information is provided in "
-            "the Scene documentation."
+            "the Scene documentation.",
+            Optional::No
         },
         {
             ConfigurationManager::KeyPaths,
@@ -55,7 +57,14 @@ Documentation ConfigurationManager::Documentation() {
             "A list of paths that are automatically registered with the file system. "
             "If a key X is used in the table, it is then useable by referencing ${X} "
             "in all other configuration files or scripts.",
-            Optional::Yes
+            Optional::No
+        },
+        {
+            ConfigurationManager::KeyPaths + '.' + ConfigurationManager::KeyCache,
+            new StringVerifier,
+            "The path to be used as a cache folder. If per scene caching is enabled, the "
+            "name of the scene will be appended to this folder",
+            Optional::No
         },
         {
             ConfigurationManager::KeyFonts,

src/engine/openspaceengine.cpp

@@ -259,6 +259,12 @@ void OpenSpaceEngine::create(int argc, char** argv,
             ConfigurationManager::findConfiguration(configurationFilePath);
     }
     configurationFilePath = absPath(configurationFilePath);
+    
+    if (!FileSys.fileExists(configurationFilePath)) {
+        throw ghoul::FileNotFoundError(
+            "Configuration file '" + configurationFilePath + "' not found"
+        );
+    }
     LINFO("Configuration Path: '" << configurationFilePath << "'");
 
     // Loading configuration from disk