Make use of new Dictionary::getValueSafe method

Clean up code in various places

src/engine/openspaceengine.cpp

@@ -160,7 +160,6 @@ bool OpenSpaceEngine::registerBasePathFromConfigurationFile(const std::string& f
 
 bool OpenSpaceEngine::findConfiguration(std::string& filename)
 {
-    //return FileSys.fileExists(filename);
     std::string currentDirectory = FileSys.absolutePath(FileSys.currentDirectory());
     size_t occurrences = std::count(currentDirectory.begin(), currentDirectory.end(),
                                     ghoul::filesystem::FileSystem::PathSeparator);
@@ -249,13 +248,10 @@ bool OpenSpaceEngine::create(int argc, char** argv,
     LDEBUG("Loading configuration from disk");
     ghoul::Dictionary& configuration = _engine->configurationManager();
     ghoul::lua::loadDictionaryFromFile(configurationFilePath, configuration);
-    const bool hasKey = configuration.hasKey(constants::openspaceengine::keyPaths);
-    const bool hasValue = configuration.hasValue<ghoul::Dictionary>(constants::openspaceengine::keyPaths);
-    if (hasKey && hasValue) {
-        ghoul::Dictionary pathsDictionary;
-        configuration.getValue(constants::openspaceengine::keyPaths, pathsDictionary);
+    ghoul::Dictionary pathsDictionary;
+	const bool success = configuration.getValueSafe(constants::openspaceengine::keyPaths, pathsDictionary);
+	if (success)
         OpenSpaceEngine::registerPathsFromDictionary(pathsDictionary);
-    }
     else {
         LFATAL("Configuration file does not contain paths token '" << constants::openspaceengine::keyPaths << "'");
         return false;
@@ -264,10 +260,9 @@ bool OpenSpaceEngine::create(int argc, char** argv,
     
     // Determining SGCT configuration file
     LDEBUG("Determining SGCT configuration file");
-    std::string sgctConfigurationPath = _sgctDefaultConfigFile;
-    if (configuration.hasKey(constants::openspaceengine::keyConfigSgct))
-        configuration.getValue(constants::openspaceengine::keyConfigSgct, sgctConfigurationPath);
-
+	std::string sgctConfigurationPath = _sgctDefaultConfigFile;
+	configuration.getValueSafe(constants::openspaceengine::keyConfigSgct,
+								sgctConfigurationPath);
     
     // Prepend the outgoing sgctArguments with the program name
     // as well as the configuration file that sgct is supposed to use
@@ -296,8 +291,8 @@ bool OpenSpaceEngine::initialize()
     glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
     GLFWwindow* win = sgct::Engine::instance()->getActiveWindowPtr()->getWindowHandle();
     glfwSwapBuffers(win);
-    int samples = sqrt(sgct::Engine::instance()->getActiveWindowPtr()->getNumberOfAASamples());
-    LDEBUG("samples: " << samples);
+    //int samples = sqrt(sgct::Engine::instance()->getActiveWindowPtr()->getNumberOfAASamples());
+    //LDEBUG("samples: " << samples);
 
     int x1, xSize, y1, ySize;
     sgct::Engine::instance()->getActiveWindowPtr()->getCurrentViewportPixelCoords(x1, y1, xSize, ySize);
@@ -323,11 +318,8 @@ bool OpenSpaceEngine::initialize()
     SysCap.detectCapabilities();
     SysCap.logCapabilities();
 
-	std::string timeKernel = "";
-	using constants::openspaceengine::keyConfigTimekernel;
-	if (OsEng.configurationManager().hasKeyAndValue<std::string>(keyConfigTimekernel)) {
-		OsEng.configurationManager().getValue(keyConfigTimekernel, timeKernel);
-	}
+	std::string timeKernel;
+	OsEng.configurationManager().getValueSafe(constants::openspaceengine::keyConfigTimekernel, timeKernel);
 
     // initialize OpenSpace helpers
 	SpiceManager::initialize();
@@ -347,25 +339,27 @@ bool OpenSpaceEngine::initialize()
     // Load scenegraph
     SceneGraph* sceneGraph = new SceneGraph;
     _renderEngine->setSceneGraph(sceneGraph);
-    if (!OsEng.configurationManager().hasValue<std::string>(
-              constants::openspaceengine::keyConfigScene)) {
-        LFATAL("Configuration needs to point to the scene file");
+	
+    std::string sceneDescriptionPath;
+	bool success = OsEng.configurationManager().getValueSafe(
+		constants::openspaceengine::keyConfigScene, sceneDescriptionPath);
+	if (!success) {
+        LFATAL("The configuration does not contain a scene file under key '" <<
+				constants::openspaceengine::keyConfigScene << "'");
         return false;
     }
 
-    std::string sceneDescriptionPath;
-    bool success = _configurationManager->getValue(
-          constants::openspaceengine::keyConfigScene, sceneDescriptionPath);
-
-    if (!FileSys.fileExists(sceneDescriptionPath)) {
-        LFATAL("Could not find '" << sceneDescriptionPath << "'");
+	if (!FileSys.fileExists(sceneDescriptionPath)) {
+        LFATAL("Could not find scene description '" << sceneDescriptionPath << "'");
         return false;
     }
 
     std::string scenePath;
-    success = _configurationManager->getValue(constants::openspaceengine::keyPathScene, scenePath);
+    success = _configurationManager->getValueSafe(
+		constants::openspaceengine::keyPathScene, scenePath);
     if (!success) {
-        LFATAL("Could not find SCENEPATH key in configuration file");
+        LFATAL("Could not find key '" << constants::openspaceengine::keyPathScene <<
+			"' in configuration file '" << sceneDescriptionPath << "'");
         return false;
     }
 
@@ -385,13 +379,12 @@ bool OpenSpaceEngine::initialize()
     _engine->_interactionHandler->connectDevices();
 
     // Run start up scripts
-    using ghoul::Dictionary;
-    using constants::openspaceengine::keyStartupScript;
-    const bool hasScript = _engine->configurationManager().hasKeyAndValue<Dictionary>(keyStartupScript);
-    if (hasScript) {
-        Dictionary scripts;
-        _engine->configurationManager().getValue(keyStartupScript, scripts);
-        
+    //using ghoul::Dictionary;
+    //using constants::openspaceengine::keyStartupScript;
+    ghoul::Dictionary scripts;
+	success = _engine->configurationManager().getValueSafe(
+		constants::openspaceengine::keyStartupScript, scripts);
+    if (success) {
         for (size_t i = 0; i < scripts.size(); ++i) {
             std::stringstream stream;
             // Dictionary-size is 0-based; script numbers are 1-based
@@ -405,7 +398,7 @@ bool OpenSpaceEngine::initialize()
             
             std::string scriptPath;
             scripts.getValue(key, scriptPath);
-            const std::string absoluteScriptPath = absPath(scriptPath);
+            std::string&& absoluteScriptPath = absPath(scriptPath);
             _engine->scriptEngine().runScriptFile(absoluteScriptPath);
         }
     }

src/properties/property.cpp

@@ -30,6 +30,7 @@ namespace openspace {
 namespace properties {
 
 namespace {
+	const std::string _loggerCat = "Property";
 	const std::string _metaDataKeyGuiName = "guiName";
     const std::string _metaDataKeyGroup = "group";
     const std::string _metaDataKeyVisible = "isVisible";
@@ -47,6 +48,11 @@ const std::string Property::ViewOptions::PowerScaledScalar = "powerScaledScalar"
 Property::Property(std::string identifier, std::string guiName)
     : _identifier(std::move(identifier))
 {
+	if (_identifier.empty())
+		LWARNING("Property identifier is empty");
+	if (guiName.empty())
+		LWARNING("Property GUI name is empty");
+
     setVisible(true);
 	_metaData.setValue(_metaDataKeyGuiName, std::move(guiName));
 }
@@ -80,8 +86,8 @@ int Property::typeLua() const {
 }
 
 const std::string& Property::guiName() const {
-	std::string result = "";
-	_metaData.getValue(_metaDataKeyGuiName, result);
+	std::string result;
+	_metaData.getValueSafe(_metaDataKeyGuiName, result);
     return std::move(result);
 }
 
@@ -90,8 +96,8 @@ void Property::setGroupIdentifier(std::string groupId) {
 }
 
 std::string Property::groupIdentifier() const {
-    std::string result = "";
-    _metaData.getValue(_metaDataKeyGroup, result);
+	std::string result;
+	_metaData.getValueSafe(_metaDataKeyGroup, result);
     return std::move(result);
 }
 

src/rendering/planets/planetgeometry.cpp

@@ -36,17 +36,14 @@ namespace planetgeometry {
 
 PlanetGeometry* PlanetGeometry::createFromDictionary(const ghoul::Dictionary& dictionary)
 {
-    std::string name;
-    dictionary.getValue(constants::scenegraphnode::keyName, name);
-
-    if (!dictionary.hasValue<std::string>(constants::planetgeometry::keyType)) {
-        LERROR("PlanetGeometry for '" << name << "' did not contain a '"
-                                      << constants::planetgeometry::keyType << "' key");
+	std::string geometryType;
+	const bool success = dictionary.getValueSafe(
+		constants::planetgeometry::keyType, geometryType);
+	if (!success) {
+        LERROR("PlanetGeometry did not contain a correct value of the key '"
+			<< constants::planetgeometry::keyType << "'");
         return nullptr;
-    }
-    std::string geometryType;
-    dictionary.getValue(constants::planetgeometry::keyType, geometryType);
-
+	}
     ghoul::TemplateFactory<PlanetGeometry>* factory
           = FactoryManager::ref().factory<PlanetGeometry>();
 

src/rendering/planets/renderableplanet.cpp

@@ -47,18 +47,22 @@ RenderablePlanet::RenderablePlanet(const ghoul::Dictionary& dictionary)
     , _texture(nullptr)
     , _geometry(nullptr)
 {
+	std::string name;
+	bool success = dictionary.getValue(constants::scenegraphnode::keyName, name);
+	assert(success);
+
     std::string path;
     dictionary.getValue(constants::scenegraph::keyPathModule, path);
 
-    if (dictionary.hasKey(constants::renderableplanet::keyGeometry)) {
-        ghoul::Dictionary geometryDictionary;
-        dictionary.getValue(constants::renderableplanet::keyGeometry, geometryDictionary);
+    ghoul::Dictionary geometryDictionary;
+    success = dictionary.getValueSafe(
+		constants::renderableplanet::keyGeometry, geometryDictionary);
+	if (success) {
+		geometryDictionary.setValue(constants::scenegraphnode::keyName, name);
         geometryDictionary.setValue(constants::scenegraph::keyPathModule, path);
-        geometryDictionary.setValue(constants::scenegraphnode::keyName, name());
-
         _geometry
               = planetgeometry::PlanetGeometry::createFromDictionary(geometryDictionary);
-    }
+	}
 
     // TODO: textures need to be replaced by a good system similar to the geometry as soon
     // as the requirements are fixed (ab)

src/rendering/planets/simplespheregeometry.cpp

@@ -47,38 +47,37 @@ SimpleSphereGeometry::SimpleSphereGeometry(const ghoul::Dictionary& dictionary)
     , _segments("segments", "Segments", 20, 1, 1000)
     , _planet(nullptr)
 {
-    if (!dictionary.hasValue<glm::vec2>(constants::simplespheregeometry::keyRadius)) {
-        std::string name;
-        dictionary.getValue(constants::scenegraphnode::keyName, name);
-        LERROR("SimpleSphereGeometry of '" << name << "' did not provide a key '"
-                                           << constants::simplespheregeometry::keyRadius
-                                           << "'");
-    }
-    else {
-        glm::vec2 radius;
-        dictionary.getValue(constants::simplespheregeometry::keyRadius, radius);
-        _radius = radius;
-    }
+	using constants::scenegraphnode::keyName;
+	using constants::simplespheregeometry::keyRadius;
+	using constants::simplespheregeometry::keySegments;
 
-    if (!dictionary.hasValue<float>(constants::simplespheregeometry::keySegments)) {
-        std::string name;
-        dictionary.getValue(constants::scenegraphnode::keyName, name);
+	// The name is passed down from the SceneGraphNode
+    std::string name;
+    bool success = dictionary.getValue(keyName, name);
+	assert(success);
+
+    glm::vec2 radius;
+    success = dictionary.getValueSafe(keyRadius, radius);
+	if (!success) {
         LERROR("SimpleSphereGeometry of '" << name << "' did not provide a key '"
-                                           << constants::simplespheregeometry::keySegments
-                                           << "'");
-    }
-    else {
-        float segments;
-        dictionary.getValue(constants::simplespheregeometry::keySegments, segments);
-        _segments = static_cast<int>(segments);
-    }
+                                           << keyRadius << "'");
+	}
+	else
+		_radius = radius;
+
+    int segments;
+    success = dictionary.getValueSafe(keySegments, segments);
+	if (!success) {
+        LERROR("SimpleSphereGeometry of '" << name << "' did not provide a key '"
+                                           << keySegments << "'");
+	}
+	else
+		_segments = segments;
 
     addProperty(_radius);
     _radius.onChange(std::bind(&SimpleSphereGeometry::createSphere, this));
     addProperty(_segments);
     _segments.onChange(std::bind(&SimpleSphereGeometry::createSphere, this));
-
-    //createSphere();
 }
 
 SimpleSphereGeometry::~SimpleSphereGeometry()

src/rendering/renderable.cpp

@@ -37,16 +37,19 @@ namespace openspace {
 
 Renderable* Renderable::createFromDictionary(const ghoul::Dictionary& dictionary)
 {
+	// The name is passed down from the SceneGraphNode
     std::string name;
-    dictionary.getValue(constants::scenegraphnode::keyName, name);
+    bool success = dictionary.getValue(constants::scenegraphnode::keyName, name);
+	assert(success);
 
-    if (!dictionary.hasValue<std::string>(constants::renderable::keyType)) {
+    std::string renderableType;
+    success = dictionary.getValueSafe(constants::renderable::keyType, renderableType);
+	if (!success) {
         LERROR("Renderable '" << name << "' did not have key '"
                               << constants::renderable::keyType << "'");
         return nullptr;
-    }
-    std::string renderableType;
-    dictionary.getValue(constants::renderable::keyType, renderableType);
+	}
+
     ghoul::TemplateFactory<Renderable>* factory
           = FactoryManager::ref().factory<Renderable>();
     Renderable* result = factory->create(renderableType, dictionary);
@@ -64,11 +67,9 @@ Renderable::Renderable(const ghoul::Dictionary& dictionary)
     setName("renderable");
 
     // get path if available
-    _relativePath = "";
-    if(dictionary.hasKey(constants::scenegraph::keyPathModule)) {
-    	dictionary.getValue(constants::scenegraph::keyPathModule, _relativePath);
-    	_relativePath += "/";
-    }
+	const bool success = dictionary.getValueSafe(constants::scenegraph::keyPathModule, _relativePath);
+	if (success)
+		_relativePath += ghoul::filesystem::FileSystem::PathSeparator;
 
 	addProperty(_enabled);
 }

src/rendering/renderablefieldlines.cpp

@@ -27,70 +27,97 @@
 #include <openspace/util/powerscaledcoordinate.h>
 #include <openspace/util/kameleonwrapper.h>
 
+#include <openspace/util/constants.h>
+
 namespace {
-	std::string _loggerCat = "RenderableFieldlines";
+	const std::string _loggerCat = "RenderableFieldlines";
+
+	const std::string keyFieldlines = "Fieldlines";
+	const std::string keyFilename = "File";
+	const std::string keyHints = "Hints";
+	const std::string keyShaders = "Shaders";
+	const std::string keyVertexShader = "VertexShader";
+	const std::string keyFragmentShader = "FragmentShader";
 }
 
 namespace openspace {
 
-RenderableFieldlines::RenderableFieldlines(const ghoul::Dictionary& dictionary) :
-			Renderable(dictionary), _VAO(0), _programUpdateOnSave(false),_update(false) {
+RenderableFieldlines::RenderableFieldlines(const ghoul::Dictionary& dictionary) 
+	: Renderable(dictionary)
+	, _VAO(0)
+	, _programUpdateOnSave(false)
+	, _update(false)
+{
+	std::string name;
+	bool success = dictionary.getValue(constants::scenegraphnode::keyName, name);
+	assert(success);
 
-	if(dictionary.hasKey("Fieldlines")) {
-		ghoul::Dictionary fieldlines;
-		if(dictionary.getValue("Fieldlines", fieldlines)) {
-			for(auto key: fieldlines.keys()) {
-				ghoul::Dictionary fieldline;
-				if(fieldlines.getValue(key, fieldline)) {
-					if (fieldline.hasKey("File")) {
-						std::string file = "";
-						if (fieldline.getValue("File", file)) {
-							file = findPath(file);
-							if (file != "") {
+	ghoul::Dictionary fieldlines;
+	success = dictionary.getValueSafe(keyFieldlines, fieldlines);
+	if (!success) {
+		LERROR("RenderableFieldlines '" << name << "' did not contain a '" <<
+			keyFieldlines << "' key");
+		return;
+	}
+	for (const std::string& key : fieldlines.keys()) {
+		ghoul::Dictionary fieldline;
+		success = fieldlines.getValueSafe(key, fieldline);
 
-								// read hints into dictionary
-								ghoul::Dictionary hintsDictionary;
-								if(fieldline.hasKey("Hints"))
-									fieldline.getValue("Hints", hintsDictionary);
+		if (!success) {
+			LERROR("Key '" << key << "' in '" << keyFieldlines <<
+				"' of the RenderableFieldlines '" << name <<
+				"' does not have a table as value");
+			continue;
+		}
 
-								_filenames.push_back(file);
-								_hintsDictionaries.push_back(hintsDictionary);
-							} else
-								LERROR("File not found!");
-						}
-					}
-				}
-			}
+		std::string fileName;
+		fieldline.getValueSafe(keyFilename, fileName);
+		fileName = findPath(fileName);
+		if (fileName.empty())
+			LERROR("File not found!");
+		else {
+			ghoul::Dictionary hintsDictionary;
+			fieldline.getValueSafe(keyHints, hintsDictionary);
+
+			_filenames.push_back(fileName);
+			_hintsDictionaries.push_back(hintsDictionary);
 		}
 	}
 
-	std::string vshaderpath = "";
-	std::string fshaderpath = "";
-
-	if (dictionary.hasKey("Shaders")) {
-		ghoul::Dictionary shaderDictionary;
-		if(dictionary.getValue("Shaders", shaderDictionary)) {
-			if (shaderDictionary.hasKey("VertexShader"))
-				shaderDictionary.getValue("VertexShader", vshaderpath);
-
-			if (shaderDictionary.hasKey("FragmentShader"))
-				shaderDictionary.getValue("FragmentShader", fshaderpath);
-
-			vshaderpath = findPath(vshaderpath);
-			fshaderpath = findPath(fshaderpath);
-
-			_vertexSourceFile = new ghoul::filesystem::File(vshaderpath, false);
-			_fragmentSourceFile = new ghoul::filesystem::File(fshaderpath, false);
-
-
-    		ShaderCreator sc = OsEng.shaderBuilder();
-    		_fieldlinesProgram = sc.buildShader("FieldlinesProgram", vshaderpath, fshaderpath);
-		}
+	ghoul::Dictionary shaderDictionary;
+	success = dictionary.getValueSafe(keyShaders, shaderDictionary);
+	if (!success) {
+		LERROR("RenderableFieldlines '" << name << "' does not contain a '" <<
+			keyShaders << "' table");
+		return;
 	}
 
-	if(dictionary.hasKey("UpdateOnSave")) {
-		dictionary.getValue("UpdateOnSave", _programUpdateOnSave);
+	std::string vshaderpath;
+	success = shaderDictionary.getValueSafe(keyVertexShader, vshaderpath);
+	if (!success) {
+		LERROR("RenderableFieldlines '" << name << "' does not have a '" <<
+			keyVertexShader << "'");
+		return;
 	}
+	vshaderpath = findPath(vshaderpath);
+
+	std::string fshaderpath;
+	success = shaderDictionary.getValueSafe(keyFragmentShader, fshaderpath);
+	if (!success) {
+		LERROR("RenderableFieldlines '" << name << "' does not have a '" <<
+			keyFragmentShader << "'");
+		return;
+	}
+	fshaderpath = findPath(fshaderpath);
+
+	_vertexSourceFile = new ghoul::filesystem::File(vshaderpath, false);
+	_fragmentSourceFile = new ghoul::filesystem::File(fshaderpath, false);
+
+
+    ShaderCreator sc = OsEng.shaderBuilder();
+    _fieldlinesProgram = sc.buildShader("FieldlinesProgram", vshaderpath, fshaderpath);
+
+	dictionary.getValueSafe("UpdateOnSave", _programUpdateOnSave);
 
 	setBoundingSphere(PowerScaledScalar::CreatePSS(5)); // FIXME a non-magic number perhaps
 }

src/scenegraph/scenegraphnode.cpp

@@ -74,6 +74,8 @@ SceneGraphNode* SceneGraphNode::createFromDictionary(const ghoul::Dictionary& di
     if (dictionary.hasValue<ghoul::Dictionary>(keyRenderable)) {
         ghoul::Dictionary renderableDictionary;
         dictionary.getValue(keyRenderable, renderableDictionary);
+
+		renderableDictionary.setValue(keyName, name);
         renderableDictionary.setValue(keyPathModule, path);
 
         result->_renderable = Renderable::createFromDictionary(renderableDictionary);