General code cleanup

include/openspace/scenegraph/scenegraph.h

@@ -84,11 +84,6 @@ public:
      */
     void render(const RenderData& data);
 
-    /*
-     * Prints the SceneGraph tree. For debugging purposes
-     */
-    void printChildren() const;
-
     /*
      * Returns the root SceneGraphNode
      */

include/openspace/scenegraph/scenegraphnode.h

@@ -78,8 +78,6 @@ public:
 
     SceneGraphNode* childNode(const std::string& name);
 
-    void print() const;
-
 	const PerformanceRecord& performanceRecord() const { return _performanceRecord; }
 
     void setRenderable(Renderable* renderable);

src/abuffer/abuffer.cpp

@@ -299,11 +299,9 @@ void ABuffer::openspaceSamplerCalls() {
 }
 
 void ABuffer::openspaceSamplers() {
-
 	std::ofstream f(absPath(generatedSamplersPath));
-	for (auto sampler : _samplers) {
+	for (const std::string& sampler : _samplers)
 		f << sampler << std::endl;
-	}
 	f.close();
 }
 

src/engine/configurationmanager.cpp

@@ -100,7 +100,7 @@ bool ConfigurationManager::loadFromFile(const std::string& filename) {
 }
 
 bool ConfigurationManager::checkCompleteness() const {
-	std::list<std::string> requiredTokens = {
+	std::vector<std::string> requiredTokens = {
 		constants::configurationmanager::keyPaths,
 		constants::configurationmanager::keyCachePath,
 		constants::configurationmanager::keyFonts,
@@ -108,7 +108,7 @@ bool ConfigurationManager::checkCompleteness() const {
 	};
 
 	bool totalSuccess = true;
-	for (auto token : requiredTokens) {
+	for (const std::string& token : requiredTokens) {
 		bool success = hasKey(token);
 
 		if (!success)

src/engine/gui.cpp

@@ -107,10 +107,8 @@ static void ImImpl_RenderDrawLists(ImDrawList** const commandLists, int nCommand
     unsigned char* buffer_data = (unsigned char*)glMapBuffer(GL_ARRAY_BUFFER, GL_WRITE_ONLY);
     if (!buffer_data)
         return;
-	int totalSize = 0;
     for (int n = 0; n < nCommandLists; ++n) {
         const ImDrawList* cmd_list = commandLists[n];
-		totalSize += cmd_list->vtx_buffer.size() * sizeof(ImDrawVert);
         memcpy(buffer_data, &cmd_list->vtx_buffer[0], cmd_list->vtx_buffer.size() * sizeof(ImDrawVert));
         buffer_data += cmd_list->vtx_buffer.size() * sizeof(ImDrawVert);
     }
@@ -154,7 +152,7 @@ void renderOptionProperty(Property* prop, const std::string& ownerName) {
 
 	int value = *p;
 	std::vector<OptionProperty::Option> options = p->options();
-	for (auto o : options) {
+	for (const OptionProperty::Option& o : options) {
 		ImGui::RadioButton((ownerName + "." + name).c_str(), &value, o.value);
 		ImGui::SameLine();
 		ImGui::Text(o.description.c_str());
@@ -438,9 +436,9 @@ void GUI::renderPropertyWindow() {
 	//ImGui::ShowUserGuide();
 	ImGui::Spacing();
 
-	for (auto p : _propertiesByOwner) {
+	for (const auto& p : _propertiesByOwner) {
 		if (ImGui::CollapsingHeader(p.first.c_str())) {
-			for (auto prop : p.second) {
+			for (properties::Property* prop : p.second) {
 				if (_boolProperties.find(prop) != _boolProperties.end()) {
 					renderBoolProperty(prop, p.first);
 					continue;

src/engine/openspaceengine.cpp

@@ -161,7 +161,7 @@ bool OpenSpaceEngine::create(int argc, char** argv,
 
 	// Create directories that doesn't exist
 	auto tokens = FileSys.tokens();
-	for (auto token : tokens) {
+	for (const std::string& token : tokens) {
 		if (!FileSys.directoryExists(token)) {
 			std::string p = absPath(token);
 			LDEBUG("Directory '" << p << "' does not exist, creating.");
@@ -398,7 +398,7 @@ void OpenSpaceEngine::loadFonts() {
 	ghoul::Dictionary fonts;
 	configurationManager().getValue(constants::configurationmanager::keyFonts, fonts);
 
-	for (auto key : fonts.keys()) {
+	for (const std::string& key : fonts.keys()) {
 		std::string font;
 		fonts.getValue(key, font);
 		font = absPath(font);

src/interaction/luaconsole.cpp

@@ -206,7 +206,7 @@ LuaConsole::~LuaConsole() {
 	if (file.is_open()) {
 		size_t n = _commandsHistory.size();
 		file.write(reinterpret_cast<const char*>(&n), sizeof(size_t));
-		for (auto s : _commandsHistory) {
+		for (const std::string& s : _commandsHistory) {
 			size_t length = s.length();
 			file.write(reinterpret_cast<const char*>(&length), sizeof(size_t));
 			file.write(s.c_str(), sizeof(char)*length);

src/properties/optionproperty.cpp

@@ -44,7 +44,7 @@ const std::vector<OptionProperty::Option>& OptionProperty::options() const {
 }
 
 void OptionProperty::addOption(Option option) {
-	for (auto o : _options) {
+	for (const Option& o : _options) {
 		if (o.value == option.value) {
 			LWARNING("The value of option {" << o.value << " -> " << o.description <<
 				"} was already registered when trying to add option {" << option.value <<
@@ -57,7 +57,7 @@ void OptionProperty::addOption(Option option) {
 
 void OptionProperty::setValue(int value) {
 	// Check if the passed value belongs to any option
-	for (auto o : _options) {
+	for (const Option& o : _options) {
 		if (o.value == value) {
 			// If it does, set it by calling the superclasses setValue method
 			NumericalProperty::setValue(value);

src/properties/propertyowner.cpp

@@ -65,7 +65,7 @@ std::vector<Property*> PropertyOwner::propertiesRecursive() const
 {
 	std::vector<Property*> props = properties();
 
-	for (auto owner : _subOwners) {
+	for (const PropertyOwner* owner : _subOwners) {
 		std::vector<Property*> p = owner->propertiesRecursive();
 		props.insert(props.end(), p.begin(), p.end());
 	}

src/properties/selectionproperty.cpp

@@ -37,7 +37,7 @@ SelectionProperty::SelectionProperty(std::string identifier, std::string guiName
 
 void SelectionProperty::addOption(Option option) {
 	// @COPY-N-PASTE from optionproperty.cpp, possible refactoring? ---abock
-	for (auto o : _options) {
+	for (const Option& o : _options) {
 		if (o.value == option.value) {
 			LWARNING("The value of option {" << o.value << " -> " << o.description <<
 				"} was already registered when trying to add option {" << option.value <<

src/rendering/renderablefieldlines.cpp

@@ -214,7 +214,7 @@ std::vector<std::vector<LinePoint> > RenderableFieldlines::getFieldlinesData(std
 		_seedPoints.clear();
 		if (hintsDictionary.hasKey("Seedpoints") && hintsDictionary.getValue("Seedpoints", seedpointsDictionary)) {
 			glm::vec3 seedPos;
-			for (auto index : seedpointsDictionary.keys()) {
+			for (const std::string& index : seedpointsDictionary.keys()) {
 				hintsDictionary.getValue("Seedpoints."+index, seedPos);
 				_seedPoints.push_back(seedPos);
 			}

src/rendering/renderengine.cpp

@@ -422,7 +422,7 @@ void RenderEngine::render()
 			const glm::vec4 blue(0, 0, 1, 1);
 
 			size_t nr = 1;
-			for (auto it = entries.first; it != entries.second; ++it) {
+			for (auto& it = entries.first; it != entries.second; ++it) {
 				const ScreenLog::LogEntry* e = &(*it);
 
 				const double t = sgct::Engine::instance()->getTime();

src/rendering/stars/renderableconstellationbounds.cpp

@@ -155,7 +155,7 @@ void RenderableConstellationBounds::render(const RenderData& data) {
 	_program->setUniform("ModelTransform", glm::mat4(glm::dmat4(_stateMatrix)));
 
 	glBindVertexArray(_vao);
-	for (auto bound : _constellationBounds)
+	for (const ConstellationBound& bound : _constellationBounds) {
 		if (bound.isEnabled) {
 			glDrawArrays(
 				//GL_LINE_STRIP,
@@ -164,6 +164,7 @@ void RenderableConstellationBounds::render(const RenderData& data) {
 				static_cast<GLsizei>(bound.nVertices)
 			);
 		}
+	}
 	glBindVertexArray(0);
 	_program->deactivate();
 }

src/scenegraph/scenegraph.cpp

@@ -233,9 +233,8 @@ bool SceneGraph::deinitialize()
 
 	// clean up all programs
 	_programsToUpdate.clear();
-	for (auto program : _programs) {
+	for (ghoul::opengl::ProgramObject* program : _programs)
 		delete program;
-	}
 	_programs.clear();
     return true;
 }
@@ -252,7 +251,7 @@ void SceneGraph::update(const UpdateData& data)
 		OsEng.renderEngine().abuffer()->invalidateABuffer();
 #endif
 	}
-    for (auto node : _nodes)
+    for (SceneGraphNode* node : _nodes)
         node->update(data);
 }
 
@@ -267,7 +266,7 @@ void SceneGraph::render(const RenderData& data)
 	bool emptyProgramsToUpdate = _programsToUpdate.empty();
 		
 	_programUpdateLock.lock();
-	for (auto program : _programsToUpdate) {
+	for (ghoul::opengl::ProgramObject* program : _programsToUpdate) {
 		LDEBUG("Attempting to recompile " << program->name());
 		program->rebuildFromFile();
 	}
@@ -277,9 +276,8 @@ void SceneGraph::render(const RenderData& data)
 	if (!emptyProgramsToUpdate) {
 		LDEBUG("Setting uniforms");
 		// Ignore attribute locations
-		for (auto program : _programs) {
+		for (ghoul::opengl::ProgramObject* program : _programs)
 			program->setIgnoreSubroutineUniformLocationError(true);
-		}
 	}
 
 	if (_root)
@@ -291,7 +289,7 @@ void SceneGraph::scheduleLoadSceneFile(const std::string& sceneDescriptionFilePa
 }
 
 void SceneGraph::clearSceneGraph() {
-	for (auto node : _nodes)
+	for (SceneGraphNode* node : _nodes)
 		node->deinitialize();
 
 	// deallocate the scene graph. Recursive deallocation will occur
@@ -389,7 +387,7 @@ bool SceneGraph::loadSceneInternal(const std::string& sceneDescriptionFilePath)
     }
 
     // Initialize all nodes
-    for (auto node : _nodes) {
+    for (SceneGraphNode* node : _nodes) {
 		bool success = node->initialize();
         if (success)
             LDEBUG(node->name() << " initialized successfully!");
@@ -399,7 +397,7 @@ bool SceneGraph::loadSceneInternal(const std::string& sceneDescriptionFilePath)
 
     // update the position of all nodes
 	// TODO need to check this; unnecessary? (ab)
-	for (auto node : _nodes)
+	for (SceneGraphNode* node : _nodes)
 		node->update({ Time::ref().currentTime() });
 
     // Calculate the bounding sphere for the scenegraph
@@ -459,9 +457,9 @@ bool SceneGraph::loadSceneInternal(const std::string& sceneDescriptionFilePath)
 	c->setRotation(la);
 
 
-	for (auto node : _nodes) {
-		std::vector<properties::Property*>&& properties = node->propertiesRecursive();
-		for (auto p : properties) {
+	for (SceneGraphNode* node : _nodes) {
+		std::vector<properties::Property*> properties = node->propertiesRecursive();
+		for (properties::Property* p : properties) {
 			OsEng.gui().registerProperty(p);
 		}
 	}
@@ -506,11 +504,6 @@ void SceneGraph::loadModule(const std::string& modulePath)
     //printTree(_root);
 }
 
-void SceneGraph::printChildren() const
-{
-    _root->print();
-}
-
 SceneGraphNode* SceneGraph::root() const
 {
     return _root;

src/scenegraph/scenegraphnode.cpp

@@ -240,9 +240,8 @@ void SceneGraphNode::evaluate(const Camera* camera, const psc& parentPosition) {
     }
 
     // evaluate all the children, tail-recursive function(?)
-    for (auto& child : _children) {
+    for (SceneGraphNode* child : _children)
         child->evaluate(camera, psc());
-    }
 }
 
 void SceneGraphNode::render(const RenderData& data) {
@@ -268,9 +267,8 @@ void SceneGraphNode::render(const RenderData& data) {
 
     // evaluate all the children, tail-recursive function(?)
 
-    for (auto& child : _children) {
+    for (SceneGraphNode* child : _children)
         child->render(newData);
-    }
 }
 
 // set & get
@@ -398,21 +396,12 @@ SceneGraphNode* SceneGraphNode::childNode(const std::string& name)
     if (this->name() == name)
         return this;
     else
-        for (auto it : _children) {
+        for (SceneGraphNode* it : _children) {
             SceneGraphNode* tmp = it->childNode(name);
-            if (tmp != nullptr) {
+            if (tmp != nullptr)
                 return tmp;
-            }
         }
     return nullptr;
 }
 
-void SceneGraphNode::print() const
-{
-    std::cout << name() << std::endl;
-    for (auto it : _children) {
-        it->print();
-    }
-}
-
-}  // namespace openspace
+}  // namespace openspace

src/scripting/scriptengine.cpp

@@ -228,9 +228,11 @@ void ScriptEngine::addLibrary(LuaLibrary library) {
 		// otherwise, we merge the libraries
 
 		LuaLibrary merged = *it;
-		for (auto fun : library.functions) {
+		for (const LuaLibrary::Function& fun : library.functions) {
 			auto it = std::find_if(merged.functions.begin(), merged.functions.end(),
-				[&fun](const LuaLibrary::Function& function) { return fun.name == function.name; });
+				[&fun](const LuaLibrary::Function& function) {
+					return fun.name == function.name;
+			});
 			if (it != merged.functions.end()) {
 				// the function with the desired name is already present, but we don't
 				// want to overwrite it
@@ -295,10 +297,20 @@ bool ScriptEngine::runScriptFile(const std::string& filename) {
 
 bool ScriptEngine::hasLibrary(const std::string& name)
 {
-	for (auto it = _registeredLibraries.begin(); it != _registeredLibraries.end(); ++it)
-		if (it->name == name)
-			return true;
-	return false;
+	auto it = std::find_if(
+		_registeredLibraries.begin(),
+		_registeredLibraries.end(),
+		[name](const LuaLibrary& it) { return it.name == name; }
+	);
+
+	return (it != _registeredLibraries.end());
+
+
+	//for (const LuaLibrary& it : _registeredLibraries) {
+	//	if (it.name == name)
+	//		return true;
+	//}
+	//return false;
 }
 
 bool ScriptEngine::isLibraryNameAllowed(const std::string& name)
@@ -451,7 +463,7 @@ void ScriptEngine::initializeLuaState(lua_State* state) {
     lua_setglobal(_state, _openspaceLibraryName.c_str());
 
 	LDEBUG("Add OpenSpace modules");
-	for (auto lib : _registeredLibraries)
+	for (const LuaLibrary& lib : _registeredLibraries)
 		registerLuaLibrary(state, lib);
 }
 
@@ -521,8 +533,8 @@ bool ScriptEngine::writeDocumentation(const std::string& filename, const std::st
 
 		file << "Available commands:\n";
 		// Now write out the functions
-		for (auto library : _registeredLibraries) {
-			for (auto function : library.functions) {
+		for (const LuaLibrary& library : _registeredLibraries) {
+			for (const LuaLibrary::Function& function : library.functions) {
 
 				std::string functionName = concatenate(library.name, function.name);
 				file << padding << functionName;
@@ -534,14 +546,15 @@ bool ScriptEngine::writeDocumentation(const std::string& filename, const std::st
 		file << std::endl;
 
 		// Now write out the functions definitions
-		for (auto library : _registeredLibraries) {
-			for (auto function : library.functions) {
+		for (const LuaLibrary& library : _registeredLibraries) {
+			for (const LuaLibrary::Function& function : library.functions) {
 
 				std::string functionName = concatenate(library.name, function.name);
 				file << functionName << "(" << function.argumentText << "):" << std::endl;
 
 				std::string remainingHelptext = function.helpText;
 
+				// @CLEANUP This needs to become a bit prettier ---abock
 				while (!remainingHelptext.empty()) {
 					const auto length = remainingHelptext.length();
 					const auto paddingLength = padding.length();