Refactor the script a bit

src/anki/scene/components/ScriptComponent.cpp

@@ -30,10 +30,10 @@ Error ScriptComponent::load(CString fname)
 	ANKI_CHECK(m_node->getSceneGraph().getResourceManager().loadResource(fname, m_script));
 
 	// Create the env
-	ANKI_CHECK(m_node->getSceneGraph().getScriptManager().newScriptEnvironment(m_env));
+	ANKI_CHECK(m_env.init(&m_node->getSceneGraph().getScriptManager()));
 
 	// Exec the script
-	ANKI_CHECK(m_env->evalString(m_script->getSource()));
+	ANKI_CHECK(m_env.evalString(m_script->getSource()));
 
 	return Error::NONE;
 }
@@ -42,7 +42,7 @@ Error ScriptComponent::update(SceneNode& node, Second prevTime, Second crntTime,
 {
 	ANKI_ASSERT(&node == m_node);
 	updated = false;
-	lua_State* lua = &m_env->getLuaState();
+	lua_State* lua = &m_env.getLuaState();
 
 	// Push function name
 	lua_getglobal(lua, "update");

src/anki/scene/components/ScriptComponent.h

@@ -7,7 +7,7 @@
 
 #include <anki/scene/components/SceneComponent.h>
 #include <anki/resource/Forward.h>
-#include <anki/script/Forward.h>
+#include <anki/script/ScriptEnvironment.h>
 
 namespace anki
 {
@@ -32,7 +32,7 @@ public:
 private:
 	SceneNode* m_node;
 	ScriptResourcePtr m_script;
-	ScriptEnvironmentPtr m_env;
+	ScriptEnvironment m_env;
 };
 /// @}
 

src/anki/scene/events/ScriptEvent.cpp

@@ -29,7 +29,7 @@ Error ScriptEvent::init(Second startTime, Second duration, CString script)
 	Event::init(startTime, duration);
 
 	// Create the env
-	ANKI_CHECK(getSceneGraph().getScriptManager().newScriptEnvironment(m_env));
+	ANKI_CHECK(m_env.init(&getSceneGraph().getScriptManager()));
 
 	// Do the rest
 	StringAuto extension(getAllocator());
@@ -41,7 +41,7 @@ Error ScriptEvent::init(Second startTime, Second duration, CString script)
 		ANKI_CHECK(getSceneGraph().getResourceManager().loadResource(script, m_scriptRsrc));
 
 		// Exec the script
-		ANKI_CHECK(m_env->evalString(m_scriptRsrc->getSource()));
+		ANKI_CHECK(m_env.evalString(m_scriptRsrc->getSource()));
 	}
 	else
 	{
@@ -49,7 +49,7 @@ Error ScriptEvent::init(Second startTime, Second duration, CString script)
 		m_script.create(getAllocator(), script);
 
 		// Exec the script
-		ANKI_CHECK(m_env->evalString(m_script.toCString()));
+		ANKI_CHECK(m_env.evalString(m_script.toCString()));
 	}
 
 	return Error::NONE;
@@ -57,7 +57,7 @@ Error ScriptEvent::init(Second startTime, Second duration, CString script)
 
 Error ScriptEvent::update(Second prevUpdateTime, Second crntTime)
 {
-	lua_State* lua = &m_env->getLuaState();
+	lua_State* lua = &m_env.getLuaState();
 
 	// Push function name
 	lua_getglobal(lua, "update");
@@ -96,7 +96,7 @@ Error ScriptEvent::update(Second prevUpdateTime, Second crntTime)
 
 Error ScriptEvent::onKilled(Second prevUpdateTime, Second crntTime)
 {
-	lua_State* lua = &m_env->getLuaState();
+	lua_State* lua = &m_env.getLuaState();
 
 	// Push function name
 	lua_getglobal(lua, "onKilled");

src/anki/scene/events/ScriptEvent.h

@@ -7,7 +7,7 @@
 
 #include <anki/scene/events/Event.h>
 #include <anki/resource/Forward.h>
-#include <anki/script/Forward.h>
+#include <anki/script/ScriptEnvironment.h>
 
 namespace anki
 {
@@ -44,7 +44,7 @@ public:
 private:
 	ScriptResourcePtr m_scriptRsrc;
 	String m_script;
-	ScriptEnvironmentPtr m_env;
+	ScriptEnvironment m_env;
 };
 /// @}
 

src/anki/script/Common.h

@@ -14,8 +14,6 @@ namespace anki
 // Forward
 class LuaBinder;
 class ScriptManager;
-
-class ScriptObject;
 class ScriptEnvironment;
 
 #define ANKI_SCRIPT_LOGI(...) ANKI_LOG("SCRI", NORMAL, __VA_ARGS__)
@@ -25,6 +23,4 @@ class ScriptEnvironment;
 
 using ScriptAllocator = HeapAllocator<U8>;
 
-using ScriptEnvironmentPtr = IntrusivePtr<ScriptEnvironment>;
-
 } // end namespace anki

src/anki/script/LuaBinder.cpp

@@ -40,15 +40,17 @@ LuaBinder::LuaBinder()
 
 LuaBinder::~LuaBinder()
 {
-	lua_close(m_l);
+	if(m_l)
+	{
+		lua_close(m_l);
+	}
 	m_userDataSigToDataInfo.destroy(m_alloc);
-
-	ANKI_ASSERT(m_alloc.getMemoryPool().getAllocationsCount() == 0 && "Leaking memory");
 }
 
-Error LuaBinder::create(ScriptAllocator alloc, void* parent)
+Error LuaBinder::init(ScriptAllocator alloc, LuaBinderOtherSystems* otherSystems)
 {
-	m_parent = parent;
+	ANKI_ASSERT(otherSystems);
+	m_otherSystems = otherSystems;
 	m_alloc = alloc;
 
 	m_l = lua_newstate(luaAllocCallback, this);
@@ -272,33 +274,6 @@ void LuaBinder::luaFree(lua_State* l, void* ptr)
 	binder->m_alloc.getMemoryPool().free(ptr);
 }
 
-LuaThread LuaBinder::newLuaThread()
-{
-	LuaThread out;
-
-	void* ud;
-	auto allocCallback = lua_getallocf(m_l, &ud);
-	ANKI_ASSERT(ud && allocCallback);
-
-	out.m_luaState = lua_newstate(allocCallback, ud);
-	luaL_openlibs(out.m_luaState);
-	lua_atpanic(out.m_luaState, &luaPanic);
-
-	wrapModules(out.m_luaState);
-
-	return out;
-}
-
-void LuaBinder::destroyLuaThread(LuaThread& luaThread)
-{
-	if(luaThread.m_luaState)
-	{
-		lua_close(luaThread.m_luaState);
-	}
-
-	luaThread.m_luaState = nullptr;
-}
-
 void LuaBinder::serializeGlobals(lua_State* l, LuaBinderSerializeGlobalsCallback& callback)
 {
 	ANKI_ASSERT(l);

src/anki/script/LuaBinder.h

@@ -22,6 +22,8 @@ namespace anki
 
 // Forward
 class LuaUserData;
+class SceneGraph;
+class MainRenderer;
 
 /// @addtogroup script
 /// @{
@@ -129,40 +131,20 @@ private:
 	const LuaUserDataTypeInfo* m_info = nullptr;
 };
 
-/// An instance of the original lua state with its own state.
-class LuaThread : public NonCopyable
+/// @memberof LuaBinder
+class LuaBinderSerializeGlobalsCallback
 {
-	friend class LuaBinder;
-
 public:
-	lua_State* m_luaState = nullptr;
-
-	LuaThread() = default;
-
-	LuaThread(LuaThread&& b)
-	{
-		*this = std::move(b);
-	}
-
-	~LuaThread()
-	{
-		ANKI_ASSERT(m_luaState == nullptr && "Forgot to deleteLuaThread");
-	}
-
-	LuaThread& operator=(LuaThread&& b)
-	{
-		ANKI_ASSERT(m_luaState == nullptr);
-		m_luaState = b.m_luaState;
-		b.m_luaState = nullptr;
-		return *this;
-	}
+	virtual void write(const void* data, PtrSize dataSize) = 0;
 };
 
+/// A list of systems that the LuaBinder should be aware of.
 /// @memberof LuaBinder
-class LuaBinderSerializeGlobalsCallback
+class LuaBinderOtherSystems
 {
 public:
-	virtual void write(const void* data, PtrSize dataSize) = 0;
+	SceneGraph* m_sceneGraph;
+	MainRenderer* m_renderer;
 };
 
 /// Lua binder class. A wrapper on top of LUA
@@ -172,10 +154,11 @@ public:
 	LuaBinder();
 	~LuaBinder();
 
-	ANKI_USE_RESULT Error create(ScriptAllocator alloc, void* parent);
+	ANKI_USE_RESULT Error init(ScriptAllocator alloc, LuaBinderOtherSystems* otherSystems);
 
 	lua_State* getLuaState()
 	{
+		ANKI_ASSERT(m_l);
 		return m_l;
 	}
 
@@ -184,9 +167,10 @@ public:
 		return m_alloc;
 	}
 
-	void* getParent() const
+	LuaBinderOtherSystems& getOtherSystems()
 	{
-		return m_parent;
+		ANKI_ASSERT(m_otherSystems);
+		return *m_otherSystems;
 	}
 
 	/// Expose a variable to the lua state
@@ -217,12 +201,6 @@ public:
 		lua_gc(state, LUA_GCCOLLECT, 0);
 	}
 
-	/// New LuaThread.
-	LuaThread newLuaThread();
-
-	/// Destroy a LuaThread.
-	void destroyLuaThread(LuaThread& luaThread);
-
 	/// For debugging purposes
 	static void stackDump(lua_State* l);
 
@@ -276,9 +254,9 @@ public:
 	static void luaFree(lua_State* l, void* ptr);
 
 private:
+	LuaBinderOtherSystems* m_otherSystems;
 	ScriptAllocator m_alloc;
 	lua_State* m_l = nullptr;
-	void* m_parent = nullptr; ///< Point to the ScriptManager
 	HashMap<I64, const LuaUserDataTypeInfo*> m_userDataSigToDataInfo;
 
 	static void* luaAllocCallback(void* userData, void* ptr, PtrSize osize, PtrSize nsize);

src/anki/script/Renderer.cpp

@@ -17,9 +17,9 @@ static MainRenderer* getMainRenderer(lua_State* l)
 	LuaBinder* binder = nullptr;
 	lua_getallocf(l, reinterpret_cast<void**>(&binder));
 
-	ScriptManager* scriptManager = reinterpret_cast<ScriptManager*>(binder->getParent());
-
-	return &scriptManager->getMainRenderer();
+	MainRenderer* r = binder->getOtherSystems().m_renderer;
+	ANKI_ASSERT(r);
+	return r;
 }
 
 LuaUserDataTypeInfo luaUserDataTypeInfoDbg = {

src/anki/script/Renderer.xml

@@ -17,10 +17,9 @@ static MainRenderer* getMainRenderer(lua_State* l)
 	LuaBinder* binder = nullptr;
 	lua_getallocf(l, reinterpret_cast<void**>(&binder));
 
-	ScriptManager* scriptManager =
-		reinterpret_cast<ScriptManager*>(binder->getParent());
-
-	return &scriptManager->getMainRenderer();
+	MainRenderer* r = binder->getOtherSystems().m_renderer;
+	ANKI_ASSERT(r);
+	return r;
 }	
 ]]></head>
 

src/anki/script/Scene.cpp

@@ -49,9 +49,9 @@ static SceneGraph* getSceneGraph(lua_State* l)
 	LuaBinder* binder = nullptr;
 	lua_getallocf(l, reinterpret_cast<void**>(&binder));
 
-	ScriptManager* scriptManager = reinterpret_cast<ScriptManager*>(binder->getParent());
-
-	return &scriptManager->getSceneGraph();
+	SceneGraph* scene = binder->getOtherSystems().m_sceneGraph;
+	ANKI_ASSERT(scene);
+	return scene;
 }
 
 static EventManager* getEventManager(lua_State* l)

src/anki/script/Scene.xml

@@ -49,10 +49,9 @@ static SceneGraph* getSceneGraph(lua_State* l)
 	LuaBinder* binder = nullptr;
 	lua_getallocf(l, reinterpret_cast<void**>(&binder));
 
-	ScriptManager* scriptManager =
-		reinterpret_cast<ScriptManager*>(binder->getParent());
-
-	return &scriptManager->getSceneGraph();
+	SceneGraph* scene = binder->getOtherSystems().m_sceneGraph;
+	ANKI_ASSERT(scene);
+	return scene;
 }
 
 static EventManager* getEventManager(lua_State* l)

src/anki/script/ScriptEnvironment.cpp

@@ -9,15 +9,12 @@
 namespace anki
 {
 
-ScriptEnvironment::~ScriptEnvironment()
+Error ScriptEnvironment::init(ScriptManager* manager)
 {
-	m_manager->destroyLuaThread(m_thread);
-}
-
-Error ScriptEnvironment::init()
-{
-	m_thread = m_manager->newLuaThread();
-	return Error::NONE;
+	ANKI_ASSERT(!isCreated());
+	ANKI_ASSERT(manager);
+	m_manager = manager;
+	return m_thread.init(m_manager->getAllocator(), &m_manager->getOtherSystems());
 }
 
 } // end namespace anki

src/anki/script/ScriptEnvironment.h

@@ -5,7 +5,6 @@
 
 #pragma once
 
-#include <anki/script/ScriptObject.h>
 #include <anki/script/LuaBinder.h>
 
 namespace anki
@@ -15,49 +14,60 @@ namespace anki
 /// @{
 
 /// A sandboxed LUA environment.
-class ScriptEnvironment : public ScriptObject
+class ScriptEnvironment
 {
 public:
-	ScriptEnvironment(ScriptManager* manager)
-		: ScriptObject(manager)
+	ScriptEnvironment()
 	{
 	}
 
-	~ScriptEnvironment();
+	~ScriptEnvironment()
+	{
+	}
 
-	Error init();
+	Error init(ScriptManager* manager);
 
 	/// Expose a variable to the scripting engine.
 	template<typename T>
 	void exposeVariable(const char* name, T* y)
 	{
-		LuaBinder::exposeVariable<T>(m_thread.m_luaState, name, y);
+		ANKI_ASSERT(isCreated());
+		LuaBinder::exposeVariable<T>(m_thread.getLuaState(), name, y);
 	}
 
 	/// Evaluate a string
 	ANKI_USE_RESULT Error evalString(const CString& str)
 	{
-		return LuaBinder::evalString(m_thread.m_luaState, str);
+		ANKI_ASSERT(isCreated());
+		return LuaBinder::evalString(m_thread.getLuaState(), str);
 	}
 
 	void serializeGlobals(LuaBinderSerializeGlobalsCallback& callback)
 	{
-		LuaBinder::serializeGlobals(m_thread.m_luaState, callback);
+		ANKI_ASSERT(isCreated());
+		LuaBinder::serializeGlobals(m_thread.getLuaState(), callback);
 	}
 
 	void deserializeGlobals(const void* data, PtrSize dataSize)
 	{
-		LuaBinder::deserializeGlobals(m_thread.m_luaState, data, dataSize);
+		ANKI_ASSERT(isCreated());
+		LuaBinder::deserializeGlobals(m_thread.getLuaState(), data, dataSize);
 	}
 
 	lua_State& getLuaState()
 	{
-		ANKI_ASSERT(m_thread.m_luaState);
-		return *m_thread.m_luaState;
+		ANKI_ASSERT(isCreated());
+		return *m_thread.getLuaState();
 	}
 
 private:
-	LuaThread m_thread;
+	ScriptManager* m_manager = nullptr;
+	LuaBinder m_thread;
+
+	Bool isCreated() const
+	{
+		return m_manager != nullptr;
+	}
 };
 /// @}
 

src/anki/script/ScriptManager.cpp

@@ -25,15 +25,9 @@ Error ScriptManager::init(AllocAlignedCallback allocCb, void* allocCbData)
 
 	m_alloc = ScriptAllocator(allocCb, allocCbData);
 
-	ANKI_CHECK(m_lua.create(m_alloc, this));
+	ANKI_CHECK(m_lua.init(m_alloc, &m_otherSystems));
 
 	return Error::NONE;
 }
 
-Error ScriptManager::newScriptEnvironment(ScriptEnvironmentPtr& out)
-{
-	out.reset(m_alloc.newInstance<ScriptEnvironment>(this));
-	return out->init();
-}
-
 } // end namespace anki

src/anki/script/ScriptManager.h

@@ -29,12 +29,17 @@ public:
 
 	void setRenderer(MainRenderer* renderer)
 	{
-		m_r = renderer;
+		m_otherSystems.m_renderer = renderer;
 	}
 
 	void setSceneGraph(SceneGraph* scene)
 	{
-		m_scene = scene;
+		m_otherSystems.m_sceneGraph = scene;
+	}
+
+	LuaBinderOtherSystems& getOtherSystems()
+	{
+		return m_otherSystems;
 	}
 
 	/// Expose a variable to the scripting engine.
@@ -52,21 +57,7 @@ public:
 		return LuaBinder::evalString(m_lua.getLuaState(), str);
 	}
 
-	ANKI_USE_RESULT Error newScriptEnvironment(ScriptEnvironmentPtr& out);
-
 anki_internal:
-	SceneGraph& getSceneGraph()
-	{
-		ANKI_ASSERT(m_scene);
-		return *m_scene;
-	}
-
-	MainRenderer& getMainRenderer()
-	{
-		ANKI_ASSERT(m_r);
-		return *m_r;
-	}
-
 	LuaBinder& getLuaBinder()
 	{
 		return m_lua;
@@ -77,21 +68,8 @@ anki_internal:
 		return m_alloc;
 	}
 
-	LuaThread newLuaThread()
-	{
-		LockGuard<Mutex> lock(n_luaMtx);
-		return m_lua.newLuaThread();
-	}
-
-	void destroyLuaThread(LuaThread& thread)
-	{
-		LockGuard<Mutex> lock(n_luaMtx);
-		m_lua.destroyLuaThread(thread);
-	}
-
 private:
-	SceneGraph* m_scene = nullptr;
-	MainRenderer* m_r = nullptr;
+	LuaBinderOtherSystems m_otherSystems;
 	ScriptAllocator m_alloc;
 	LuaBinder m_lua;
 	Mutex n_luaMtx;

src/anki/script/ScriptObject.cpp

@@ -1,17 +0,0 @@
-// Copyright (C) 2009-2018, Panagiotis Christopoulos Charitos and contributors.
-// All rights reserved.
-// Code licensed under the BSD License.
-// http://www.anki3d.org/LICENSE
-
-#include <anki/script/ScriptObject.h>
-#include <anki/script/ScriptManager.h>
-
-namespace anki
-{
-
-ScriptAllocator ScriptObject::getAllocator() const
-{
-	return m_manager->getAllocator();
-}
-
-} // end namespace anki

src/anki/script/ScriptObject.h

@@ -1,45 +0,0 @@
-// Copyright (C) 2009-2018, Panagiotis Christopoulos Charitos and contributors.
-// All rights reserved.
-// Code licensed under the BSD License.
-// http://www.anki3d.org/LICENSE
-
-#pragma once
-
-#include <anki/script/Common.h>
-
-namespace anki
-{
-
-/// @addtogroup script
-/// @{
-
-/// The base of all script objects.
-class ScriptObject
-{
-public:
-	ScriptObject(ScriptManager* manager)
-		: m_manager(manager)
-	{
-		ANKI_ASSERT(manager);
-	}
-
-	virtual ~ScriptObject()
-	{
-	}
-
-	ScriptAllocator getAllocator() const;
-
-	Atomic<I32>& getRefcount()
-	{
-		return m_refcount;
-	}
-
-protected:
-	ScriptManager* m_manager;
-
-private:
-	Atomic<I32> m_refcount = {0};
-};
-/// @}
-
-} // end namespace anki

tests/script/LuaBinder.cpp

@@ -44,8 +44,8 @@ ANKI_TEST(Script, LuaBinderThreads)
 	ScriptManager sm;
 	ANKI_TEST_EXPECT_NO_ERR(sm.init(allocAligned, nullptr));
 
-	ScriptEnvironmentPtr env;
-	ANKI_TEST_EXPECT_NO_ERR(sm.newScriptEnvironment(env));
+	ScriptEnvironment env;
+	ANKI_TEST_EXPECT_NO_ERR(env.init(&sm));
 
 	static const char* script = R"(
 vec = Vec4.new(0, 0, 0, 0)
@@ -56,14 +56,14 @@ function myFunc()
 end
 )";
 
-	ANKI_TEST_EXPECT_NO_ERR(env->evalString(script));
+	ANKI_TEST_EXPECT_NO_ERR(env.evalString(script));
 
 	static const char* script1 = R"(
 myFunc()
 )";
 
-	ANKI_TEST_EXPECT_NO_ERR(env->evalString(script1));
-	ANKI_TEST_EXPECT_NO_ERR(env->evalString(script1));
+	ANKI_TEST_EXPECT_NO_ERR(env.evalString(script1));
+	ANKI_TEST_EXPECT_NO_ERR(env.evalString(script1));
 }
 
 ANKI_TEST(Script, LuaBinderSerialize)
@@ -71,8 +71,8 @@ ANKI_TEST(Script, LuaBinderSerialize)
 	ScriptManager sm;
 	ANKI_TEST_EXPECT_NO_ERR(sm.init(allocAligned, nullptr));
 
-	ScriptEnvironmentPtr env;
-	ANKI_TEST_EXPECT_NO_ERR(sm.newScriptEnvironment(env));
+	ScriptEnvironment env;
+	ANKI_TEST_EXPECT_NO_ERR(env.init(&sm));
 
 	static const char* script = R"(
 num = 123.4
@@ -80,7 +80,7 @@ str = "lala"
 vec = Vec3.new(1, 2, 3)
 )";
 
-	ANKI_TEST_EXPECT_NO_ERR(env->evalString(script));
+	ANKI_TEST_EXPECT_NO_ERR(env.evalString(script));
 
 	class Callback : public LuaBinderSerializeGlobalsCallback
 	{
@@ -95,11 +95,12 @@ vec = Vec3.new(1, 2, 3)
 		}
 	} callback;
 
-	env->serializeGlobals(callback);
-	env.reset(nullptr);
-	ANKI_TEST_EXPECT_NO_ERR(sm.newScriptEnvironment(env));
+	env.serializeGlobals(callback);
 
-	env->deserializeGlobals(&callback.m_buff[0], callback.m_offset);
+	ScriptEnvironment env2;
+	ANKI_TEST_EXPECT_NO_ERR(env2.init(&sm));
+
+	env2.deserializeGlobals(&callback.m_buff[0], callback.m_offset);
 
 	static const char* script2 = R"(
 print(num)
@@ -107,5 +108,5 @@ print(str)
 print(vec:getX(), vec:getY(), vec:getZ())
 )";
 
-	ANKI_TEST_EXPECT_NO_ERR(env->evalString(script2));
+	ANKI_TEST_EXPECT_NO_ERR(env2.evalString(script2));
 }