Some cleanup of ScriptEngine

include/openspace/scripting/scriptengine.h

@@ -27,48 +27,69 @@
 
 #include <ghoul/lua/ghoul_lua.h>
 
-#include <set>
 #include <map>
-
-/**
- * \defgroup LuaScripts Lua Scripts
- */
+#include <memory>
+#include <set>
 
 namespace openspace {
-	class SyncBuffer;
+
+class SyncBuffer;
 
 namespace scripting {
 
+/**
+ * The ScriptEngine is responsible for handling the execution of custom Lua functions and
+ * executing scripts (#runScript and #runScriptFile). Before usage, it has to be
+ * #initialize%d and #deinitialize%d. New ScriptEngine::Library%s consisting of
+ * ScriptEngine::Library::Function%s have to be added which can then be called using the
+ * <code>openspace</code> namespac prefix in Lua. The same functions can be exposed to
+ * other Lua states by passing them to the #initializeLuaState method.
+ */
 class ScriptEngine {
 public:
+    /**
+     * This structure represents a Lua library, itself consisting of a unique \m name and
+     * an arbitrary number of \m functions
+     */
     struct LuaLibrary {
+        /**
+         * This structure represents a Lua function with its \m name, \m function pointer
+         * \m argumentText describing the arguments this function takes, the \m helpText
+         * descripbing the function, and whether it should be shared in a parallel
+         * connection (\m parallelShared)
+         */
 		struct Function {
+            /// The name of the function
 			std::string name;
+            /// The function pointer that is executed if the function is called
 			lua_CFunction function;
+            /// A text describing the arugments to this function
 			std::string argumentText;
+            /// A help text describing what the function does/
 			std::string helpText;
+            /// If <code>true</code>, this function will be shared with other parallel
+            /// connections
             bool parallelShared;
-            
-            Function(std::string n, lua_CFunction f, std::string a, std::string h , bool ps = false):
-            name(n),
-            function(f),
-            argumentText(a),
-            helpText(h),
-            parallelShared(ps)
-            {
-            
-            }
 		};
+        /// The name of the library
         std::string name;
+        /// The list of all functions for this library
 		std::vector<Function> functions;
 
+        /// Comparison function that compares two LuaLibrary%s name
 		bool operator<(const LuaLibrary& rhs) const;
     };
 
-    ScriptEngine();
-    ~ScriptEngine();
-
-    bool initialize();
+    /**
+     * Initializes the internal Lua state and registers a common set of library functions
+     * \throw LuaRuntimeException If the creation of the new Lua state fails
+     */
+    void initialize();
+    
+    /**
+     * Cleans the internal Lua state and leaves the ScriptEngine in a state to be newly
+     * initialize%d.
+     */
     void deinitialize();
     
 	void initializeLuaState(lua_State* state);
@@ -109,7 +130,7 @@ private:
     void addBaseLibrary();
     void remapPrintFunction();
     
-    lua_State* _state;
+    lua_State* _state = nullptr;
     std::set<LuaLibrary> _registeredLibraries;
     
 	//sync variables
@@ -121,7 +142,6 @@ private:
     //parallel variables
     std::map<std::string, std::map<std::string, std::string>> _cachedScripts;
     std::mutex _cachedScriptsMutex;
-    
 };
 
 } // namespace scripting

src/scripting/scriptengine.cpp

@@ -55,35 +55,15 @@ bool ScriptEngine::LuaLibrary::operator<(const LuaLibrary& rhs) const {
 	return name < rhs.name;
 }
 
-ScriptEngine::ScriptEngine()
-    : _state(nullptr)
-{
-}
-
-ScriptEngine::~ScriptEngine() {
-	//deinitialize();
-}
-
-bool ScriptEngine::initialize() {
-    LDEBUG("Adding base functions");
+void ScriptEngine::initialize() {
+    LDEBUG("Adding base library");
     addBaseLibrary();
-
-    _state = luaL_newstate();
-    LDEBUG("Creating Lua state");
-    if (_state == nullptr) {
-        LFATAL("Error creating new Lua state: Memory allocation error");
-        return false;
-    }
-
-    LDEBUG("Open Lua libraries");
-    luaL_openlibs(_state);
-
+    LDEBUG("Creating new Lua state");
+    _state = ghoul::lua::createNewLuaState();
+    LDEBUG("Initializing Lua state");
 	initializeLuaState(_state);
-    
-    LDEBUG("Remap print function");
+    LDEBUG("Remapping Print functions");
     remapPrintFunction();
-    
-    return true;
 }
 
 void ScriptEngine::deinitialize() {
@@ -93,6 +73,16 @@ void ScriptEngine::deinitialize() {
     }
 }
 
+void ScriptEngine::initializeLuaState(lua_State* state) {
+    LDEBUG("Create openspace base library");
+    lua_newtable(state);
+    lua_setglobal(state, _openspaceLibraryName.c_str());
+    
+    LDEBUG("Add OpenSpace modules");
+    for (const LuaLibrary& lib : _registeredLibraries)
+        registerLuaLibrary(state, lib);
+}
+
 void ScriptEngine::addLibrary(LuaLibrary library) {
 	auto sortFunc = [](const LuaLibrary::Function& lhs, const LuaLibrary::Function& rhs)
 	{
@@ -136,6 +126,15 @@ void ScriptEngine::addLibrary(LuaLibrary library) {
 	}
 }
 
+bool ScriptEngine::hasLibrary(const std::string& name) {
+    auto it = std::find_if(
+        _registeredLibraries.begin(),
+        _registeredLibraries.end(),
+        [name](const LuaLibrary& it) { return it.name == name; }
+    );
+    return (it != _registeredLibraries.end());
+}
+
 bool ScriptEngine::runScript(const std::string& script) {
 	if (script.empty()){
 		LWARNING("Script was empty");
@@ -168,7 +167,6 @@ bool ScriptEngine::runScript(const std::string& script) {
     return true;
 }
     
-    
 bool ScriptEngine::runScriptFile(const std::string& filename) {
     if (filename.empty()) {
         LWARNING("Filename was empty");
@@ -194,8 +192,7 @@ bool ScriptEngine::runScriptFile(const std::string& filename) {
     return true;
 }
 
-bool ScriptEngine::shouldScriptBeSent(const std::string &library, const std::string &function){
-
+bool ScriptEngine::shouldScriptBeSent(const std::string& library, const std::string& function) {
     std::set<LuaLibrary>::const_iterator libit;
     for (libit = _registeredLibraries.cbegin();
          libit != _registeredLibraries.cend();
@@ -291,24 +288,6 @@ bool ScriptEngine::parseLibraryAndFunctionNames(std::string &library, std::strin
 	return !function.empty();
 }
 
-bool ScriptEngine::hasLibrary(const std::string& name)
-{
-	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(lua_State* state, const std::string& name) {
     bool result = false;
     lua_getglobal(state, _openspaceLibraryName.c_str());
@@ -452,16 +431,6 @@ void ScriptEngine::remapPrintFunction() {
 	//ghoul::lua::logStack(_state);
 }
 
-void ScriptEngine::initializeLuaState(lua_State* state) {
-    LDEBUG("Create openspace base library");
-    lua_newtable(state);
-    lua_setglobal(state, _openspaceLibraryName.c_str());
-
-	LDEBUG("Add OpenSpace modules");
-	for (const LuaLibrary& lib : _registeredLibraries)
-		registerLuaLibrary(state, lib);
-}
-
 bool ScriptEngine::registerLuaLibrary(lua_State* state, const LuaLibrary& library) {
 	assert(state);
     if (library.functions.empty()) {