#5452: More shifting around, less static members and cross-references

plugins/script/PythonModule.cpp

@@ -16,16 +16,18 @@ namespace
     constexpr const char* ModuleName = "darkradiant";
 }
 
-PythonModule::PythonModule(const NamedInterfaces& interfaceList) :
-    _namedInterfaces(interfaceList),
+PythonModule::PythonModule() :
     _outputWriter(false, _outputBuffer),
-    _errorWriter(true, _errorBuffer)
+    _errorWriter(true, _errorBuffer),
+    _interpreterInitialised(false)
 {
     registerModule();
 }
 
 PythonModule::~PythonModule()
 {
+    _namedInterfaces.clear();
+
     // Release the references to trigger the internal cleanup before Py_Finalize
     _module.dec_ref();
     _module.release();
@@ -136,33 +138,31 @@ py::dict& PythonModule::getGlobals()
 #if PY_MAJOR_VERSION >= 3
 PyObject* PythonModule::InitModule()
 {
-	return InitModuleImpl();
+    if (!_instance) throw new std::runtime_error("_instance reference not set");
+	return _instance->initialiseModule();
 }
 #else
 void PythonModule::InitModule()
 {
-	InitModuleImpl();
+    if (!_instance) throw new std::runtime_error("_instance reference not set");
+    _instance->initialiseModule();
 }
 #endif
 
-PyObject* PythonModule::InitModuleImpl()
+PyObject* PythonModule::initialiseModule()
 {
 	try
 	{
-		if (!_instance)
-		{
-            throw new std::runtime_error("_instance reference not set");
-		}
-
-        _instance->_module = py::module::create_extension_module(ModuleName, "DarkRadiant Main Module", &_moduleDef);
+        static py::module::module_def _moduleDef;
+        _module = py::module::create_extension_module(ModuleName, "DarkRadiant Main Module", &_moduleDef);
 
         // Add the registered interfaces
-        for (const auto& i : _instance->_namedInterfaces)
+        for (const auto& i : _namedInterfaces)
         {
             // Handle each interface in its own try/catch block
             try
             {
-                i.second->registerInterface(_instance->getModule(), _instance->getGlobals());
+                i.second->registerInterface(getModule(), getGlobals());
             }
             catch (const py::error_already_set& ex)
             {
@@ -176,10 +176,12 @@ PyObject* PythonModule::InitModuleImpl()
 
 		for (auto i = globals.begin(); i != globals.end(); ++i)
 		{
-            _instance->getGlobals()[(*i).first] = (*i).second;
+            getGlobals()[(*i).first] = (*i).second;
 		}
 
-		return _instance->getModule().ptr();
+        _interpreterInitialised = true;
+
+		return getModule().ptr();
 	}
 	catch (py::error_already_set& e)
 	{
@@ -194,7 +196,39 @@ PyObject* PythonModule::InitModuleImpl()
 	}
 }
 
+void PythonModule::addInterface(const NamedInterface& iface)
+{
+    // Check if exists
+    if (interfaceExists(iface.first))
+    {
+        rError() << "A script interface with the name " << iface.first << " is already registered." << std::endl;
+        return;
+    }
+
+    // Try to insert
+    _namedInterfaces.emplace_back(iface);
+
+    if (_interpreterInitialised)
+    {
+        // Add the interface at once, all the others are already added
+        iface.second->registerInterface(getModule(), getGlobals());
+    }
+}
+
+bool PythonModule::interfaceExists(const std::string& name)
+{
+    // Traverse the interface list
+    for (const auto& namedInterface : _namedInterfaces)
+    {
+        if (namedInterface.first == name)
+        {
+            return true;
+        }
+    }
+
+    return false;
+}
+
 PythonModule* PythonModule::_instance = nullptr;
-py::module::module_def PythonModule::_moduleDef;
 
 }

plugins/script/PythonModule.h

@@ -16,13 +16,12 @@ namespace script
 class PythonModule final
 {
 private:
-	// Python objects and initialisation stuff
+	// Python module and global dictionary
 	py::module _module;
 	py::dict _globals;
-    static py::module::module_def _moduleDef;
 
-    // Reference to the list owned by the ScriptingSystem
-	const NamedInterfaces& _namedInterfaces;
+    // List of registered interfaces
+	NamedInterfaces _namedInterfaces;
 
     PythonModule(const PythonModule& other) = delete;
     PythonModule& operator=(const PythonModule& other) = delete;
@@ -38,8 +37,10 @@ class PythonModule final
     PythonConsoleWriter _outputWriter;
     PythonConsoleWriter _errorWriter;
 
+    bool _interpreterInitialised;
+
 public:
-    PythonModule(const NamedInterfaces& interfaceList);
+    PythonModule();
     ~PythonModule();
 
     // Starts up the interpreter, imports the darkradiant module
@@ -53,18 +54,22 @@ class PythonModule final
 	// Get the globals
 	py::dict& getGlobals();
 
+    void addInterface(const NamedInterface& iface);
+
 private:
     // Register the darkradiant module with the inittab pointing to InitModule
     void registerModule();
 
+    bool interfaceExists(const std::string& name);
+
 	// Endpoint called by the Python interface to acquire the module
 #if PY_MAJOR_VERSION >= 3
 	static PyObject* InitModule();
 #else
 	static void InitModule();
 #endif
 
-	static PyObject* InitModuleImpl();
+	PyObject* initialiseModule();
 };
 
 }

plugins/script/ScriptingSystem.cpp

@@ -54,36 +54,9 @@ ScriptingSystem::ScriptingSystem() :
 	_initialised(false)
 {}
 
-void ScriptingSystem::addInterface(const std::string& name, const IScriptInterfacePtr& iface) {
-	// Check if exists
-	if (interfaceExists(name)) {
-		rError() << "Cannot add script interface " << name
-			<< ", this interface is already registered." << std::endl;
-		return;
-	}
-
-	// Try to insert
-	_interfaces.push_back(NamedInterface(name, iface));
-
-	if (_initialised) 
-	{
-		// Add the interface at once, all the others are already added
-		iface->registerInterface(_pythonModule->getModule(), _pythonModule->getGlobals());
-	}
-}
-
-bool ScriptingSystem::interfaceExists(const std::string& name)
+void ScriptingSystem::addInterface(const std::string& name, const IScriptInterfacePtr& iface)
 {
-	// Traverse the interface list
-	for (const NamedInterface& i : _interfaces)
-	{
-		if (i.first == name)
-		{
-			return true;
-		}
-	}
-
-	return false;
+    _pythonModule->addInterface(NamedInterface(name, iface));
 }
 
 void ScriptingSystem::executeScriptFile(const std::string& filename)
@@ -335,7 +308,7 @@ void ScriptingSystem::initialiseModule(const IApplicationContext& ctx)
 #endif
 
 	// Set up the python interpreter
-    _pythonModule.reset(new PythonModule(_interfaces));
+    _pythonModule.reset(new PythonModule);
 
 	// Add the built-in interfaces (the order is important, as we don't have dependency-resolution yet)
 	addInterface("Math", std::make_shared<MathInterface>());
@@ -397,9 +370,6 @@ void ScriptingSystem::shutdownModule()
 
 	_scriptPath.clear();
 
-	// Free all interfaces
-	_interfaces.clear();
-
     _pythonModule.reset();
 }
 

plugins/script/ScriptingSystem.h

@@ -23,7 +23,6 @@ class ScriptingSystem :
 {
 private:
 	bool _initialised;
-	NamedInterfaces _interfaces;
     std::unique_ptr<PythonModule> _pythonModule;
 
 	// The path where the script files are hosted
@@ -80,8 +79,6 @@ class ScriptingSystem :
 private:
 	void executeScriptFile(const std::string& filename, bool setExecuteCommandAttr);
 
-	bool interfaceExists(const std::string& name);
-
 	void reloadScripts();
 
 	void loadCommandScript(const std::string& scriptFilename);