Rewritten Lua ChunkStay API into a single function, cWorld:ChunkStay().

This fixes problems with indeterminate class object lifespan (Lua-GC) and forgetting to disable it or keep it until ready.

src/Bindings/AllToLua.pkg

@@ -24,7 +24,6 @@ $cfile "Plugin.h"
 $cfile "PluginLua.h"
 $cfile "WebPlugin.h"
 $cfile "LuaWindow.h"
-$cfile "LuaChunkStay.h"
 
 $cfile "../BlockID.h"
 $cfile "../StringUtils.h"
@@ -71,7 +70,6 @@ $cfile "../Generating/ChunkDesc.h"
 $cfile "../CraftingRecipes.h"
 $cfile "../UI/Window.h"
 $cfile "../Mobs/Monster.h"
-$cfile "../ChunkStay.h"
 
 
 

src/Bindings/LuaChunkStay.cpp

@@ -1,18 +1,20 @@
 
 // LuaChunkStay.cpp
 
-// Implements the cLuaChunkStay class representing a cChunkStay binding for plugins
+// Implements the cLuaChunkStay class representing a cChunkStay binding for plugins, used by cWorld:ChunkStay() Lua API
 
 #include "Globals.h"
 #include "LuaChunkStay.h"
+#include "PluginLua.h"
 #include "../World.h"
 
 
 
 
 
-cLuaChunkStay::cLuaChunkStay(void) :
-	m_LuaState((lua_State *)NULL)  // Want a detached Lua state by default
+cLuaChunkStay::cLuaChunkStay(cPluginLua & a_Plugin) :
+	m_Plugin(a_Plugin),
+	m_LuaState(NULL)
 {
 }
 
@@ -20,39 +22,107 @@ cLuaChunkStay::cLuaChunkStay(void) :
 
 
 
-void cLuaChunkStay::Enable(
-	cWorld & a_World, lua_State * a_LuaState,
-	int a_OnChunkAvailableStackPos, int a_OnAllChunksAvailableStackPos
-)
+bool cLuaChunkStay::AddChunks(int a_ChunkCoordTableStackPos)
 {
-	if (m_LuaState.IsValid())
+	// This function is expected to be called just once, with all the coords in a table
+	ASSERT(m_Chunks.empty());
+	
+	cPluginLua::cOperation Op(m_Plugin);
+	cLuaState & L = Op();
+	
+	// Check that we got a table:
+	if (!lua_istable(L, a_ChunkCoordTableStackPos))
 	{
-		LOGWARNING("LuaChunkStay: Already enabled. Ignoring this call.");
-		cLuaState::LogStackTrace(a_LuaState);
-		return;
+		LOGWARNING("%s: The parameter is not a table of coords (got %s). Ignoring the call.",
+			__FUNCTION__, lua_typename(L, lua_type(L, a_ChunkCoordTableStackPos))
+		);
+		L.LogStackTrace();
+		return false;
 	}
-	m_LuaState.Attach(a_LuaState);
-	m_OnChunkAvailable.RefStack(m_LuaState, a_OnChunkAvailableStackPos);
-	m_OnAllChunksAvailable.RefStack(m_LuaState, a_OnAllChunksAvailableStackPos);
-	super::Enable(*a_World.GetChunkMap());
+	
+	// Add each set of coords:
+	int NumChunks = luaL_getn(L, a_ChunkCoordTableStackPos);
+	m_Chunks.reserve(NumChunks);
+	for (int idx = 1; idx <= NumChunks; idx++)
+	{
+		// Push the idx-th element of the array onto stack top, check that it's a table:
+		lua_rawgeti(L, a_ChunkCoordTableStackPos, idx);
+		if (!lua_istable(L, -1))
+		{
+			LOGWARNING("%s: Element #%d is not a table (got %s). Ignoring the element.",
+				__FUNCTION__, idx, lua_typename(L, -1)
+			);
+			L.LogStackTrace();
+			lua_pop(L, 1);
+			continue;
+		}
+		AddChunkCoord(L, idx);
+		lua_pop(L, 1);
+	}
+	
+	// If there are no chunks, log a warning and return failure:
+	if (m_Chunks.empty())
+	{
+		LOGWARNING("%s: Zero chunks to stay.", __FUNCTION__);
+		L.LogStackTrace();
+		return false;
+	}
+	
+	// All ok
+	return true;
 }
 
 
 
 
 
-void cLuaChunkStay::Disable(void)
+void cLuaChunkStay::AddChunkCoord(cLuaState & L, int a_Index)		
 {
-	super::Disable();
-
-	// If the state was valid (bound to Lua functions), unbind those functions:
-	if (!m_LuaState.IsValid())
+	// Check that the element has 2 coords:
+	int NumCoords = luaL_getn(L, -1);
+	if (NumCoords != 2)
 	{
+		LOGWARNING("%s: Element #%d doesn't contain 2 coords (got %d). Ignoring the element.",
+			__FUNCTION__, a_Index, NumCoords
+		);
 		return;
 	}
-	m_OnAllChunksAvailable.UnRef();
-	m_OnChunkAvailable.UnRef();
-	m_LuaState.Detach();
+	
+	// Read the two coords from the element:
+	lua_rawgeti(L, -1, 1);
+	lua_rawgeti(L, -2, 2);
+	int ChunkX = luaL_checkint(L, -2);
+	int ChunkZ = luaL_checkint(L, -1);
+	lua_pop(L, 2);
+	
+	// Check that a coord is not yet present:
+	for (cChunkCoordsVector::iterator itr = m_Chunks.begin(), end = m_Chunks.end(); itr != end; ++itr)
+	{
+		if ((itr->m_ChunkX == ChunkX) && (itr->m_ChunkZ == ChunkZ))
+		{
+			LOGWARNING("%s: Element #%d is a duplicate, ignoring it.",
+				__FUNCTION__, a_Index
+			);
+			return;
+		}
+	}  // for itr - m_Chunks[]
+	
+	m_Chunks.push_back(cChunkCoords(ChunkX, ZERO_CHUNK_Y, ChunkZ));
+}
+
+
+
+
+
+void cLuaChunkStay::Enable(cChunkMap & a_ChunkMap, int a_OnChunkAvailableStackPos, int a_OnAllChunksAvailableStackPos)
+{
+	// Get the references to the callback functions:
+	m_LuaState = &m_Plugin.GetLuaState();
+	m_OnChunkAvailable.RefStack(*m_LuaState, a_OnChunkAvailableStackPos);
+	m_OnAllChunksAvailable.RefStack(*m_LuaState, a_OnAllChunksAvailableStackPos);
+	
+	// Enable the ChunkStay:
+	super::Enable(a_ChunkMap);
 }
 
 
@@ -61,19 +131,43 @@ void cLuaChunkStay::Disable(void)
 
 void cLuaChunkStay::OnChunkAvailable(int a_ChunkX, int a_ChunkZ)
 {
-	m_LuaState.Call(m_OnChunkAvailable, a_ChunkX, a_ChunkZ);
+	// DEBUG:
+	LOGD("LuaChunkStay: Chunk [%d, %d] is now available, calling the callback...", a_ChunkX, a_ChunkZ);
+	
+	cPluginLua::cOperation Op(m_Plugin);
+	Op().Call((int)m_OnChunkAvailable, a_ChunkX, a_ChunkZ);
 }
 
 
 
 
 
-void cLuaChunkStay::OnAllChunksAvailable(void)
+bool cLuaChunkStay::OnAllChunksAvailable(void)
 {
-	m_LuaState.Call(m_OnAllChunksAvailable);
+	{
+		// Call the callback:
+		cPluginLua::cOperation Op(m_Plugin);
+		Op().Call((int)m_OnAllChunksAvailable);
+		
+		// Remove the callback references - they won't be needed anymore
+		m_OnChunkAvailable.UnRef();
+		m_OnAllChunksAvailable.UnRef();
+	}
+	
+	// Disable the ChunkStay by returning true
+	return true;
 }
 
 
 
 
 
+void cLuaChunkStay::OnDisabled(void)
+{
+	// This object is no longer needed, delete it
+	delete this;
+}
+
+
+
+

src/Bindings/LuaChunkStay.h

@@ -1,7 +1,7 @@
 
 // LuaChunkStay.h
 
-// Declares the cLuaChunkStay class representing a cChunkStay binding for plugins
+// Declares the cLuaChunkStay class representing a cChunkStay binding for plugins, used by cWorld:ChunkStay() Lua API
 
 
 
@@ -16,36 +16,36 @@
 
 
 
-// tolua_begin
+// fwd:
+class cPluginLua;
+
+
+
+
+
 class cLuaChunkStay
 	: public cChunkStay
 {
 	typedef cChunkStay super;
 	
 public:
-	// Allow Lua to construct objects of this class:
-	cLuaChunkStay(void);
+	cLuaChunkStay(cPluginLua & a_Plugin);
 	
-	// Allow Lua to garbage-collect objects of this class:
 	~cLuaChunkStay() { }
 
-	// tolua_end	
+	/** Adds chunks in the specified on-stack Lua table.
+	Returns true if any chunk added, false (plus log warning) if none. */
+	bool AddChunks(int a_ChunkCoordTableStackPos);
 	
-	/** Enables the ChunkStay for the specified world, with the specified Lua callbacks.
-	Exported in ManualBindings. */
-	void Enable(
-		cWorld & a_World, lua_State * a_LuaState,
-		int a_OnChunkAvailableStackPos, int a_OnAllChunksAvailableStackPos
-	);
-	
-	// tolua_begin
-	
-	/** Disables the ChunkStay. Cleans up the bound Lua state and callbacks */
-	virtual void Disable(void);
+	/** Enables the ChunkStay for the specified chunkmap, with the specified Lua callbacks. */
+	void Enable(cChunkMap & a_ChunkMap, int a_OnChunkAvailableStackPos, int a_OnAllChunksAvailableStackPos);
 	
 protected:
+	/** The plugin which has created the ChunkStay, via cWorld:ChunkStay() binding method.  */
+	cPluginLua & m_Plugin;
+	
 	/** The Lua state associated with the callbacks. Only valid when enabled. */
-	cLuaState m_LuaState;
+	cLuaState * m_LuaState;
 	
 	/** The Lua function to call in OnChunkAvailable. Only valid when enabled. */
 	cLuaState::cRef m_OnChunkAvailable;
@@ -53,12 +53,20 @@ protected:
 	/** The Lua function to call in OnAllChunksAvailable. Only valid when enabled. */
 	cLuaState::cRef m_OnAllChunksAvailable;
 	
+	
 	// cChunkStay overrides:
 	virtual void OnChunkAvailable(int a_ChunkX, int a_ChunkZ) override;
-	virtual void OnAllChunksAvailable(void) override;
+	virtual bool OnAllChunksAvailable(void) override;
+	virtual void OnDisabled(void) override;
+	
+	/** Adds a single chunk coord from the table at the top of the Lua stack.
+	Expects the top element to be a table, checks that it contains two numbers.
+	Uses those two numbers as chunk coords appended to m_Chunks.
+	If the coords are already present, gives a warning and ignores the pair.
+	The a_Index parameter is only for the error messages. */
+	void AddChunkCoord(cLuaState & a_LuaState, int a_Index);
 } ;
 
-// tolua_end
 
 
 

src/Bindings/ManualBindings.cpp

@@ -1639,35 +1639,46 @@ static int tolua_cPluginManager_CallPlugin(lua_State * tolua_S)
 
 
 
-static int tolua_cLuaChunkStay_Enable(lua_State * tolua_S)
+static int tolua_cWorld_ChunkStay(lua_State * tolua_S)
 {
+	/* Function signature:
+	World:ChunkStay(ChunkCoordTable, OnChunkAvailable, OnAllChunksAvailable)
+	ChunkCoordTable == { {Chunk1x, Chunk1z}, {Chunk2x, Chunk2z}, ... }
+	*/
+	
 	cLuaState L(tolua_S);
 	if (
-		!L.CheckParamUserType(1, "cLuaChunkStay") ||
-		!L.CheckParamUserType(2, "cWorld") ||
+		!L.CheckParamUserType(1, "cWorld") ||
+		!L.CheckParamTable   (2) ||
 		!L.CheckParamFunction(3, 4)
 	)
 	{
 		return 0;
 	}
 	
-	// Read the params:
-	cLuaChunkStay * ChunkStay = (cLuaChunkStay *)tolua_tousertype(tolua_S, 1, NULL);
-	if (ChunkStay == NULL)
+	cPluginLua * Plugin = GetLuaPlugin(tolua_S);
+	if (Plugin == NULL)
 	{
-		LOGWARNING("cLuaChunkStay:Enable(): invalid self");
-		L.LogStackTrace();
 		return 0;
 	}
-	cWorld * World = (cWorld *)tolua_tousertype(tolua_S, 2, NULL);
+	cLuaChunkStay * ChunkStay = new cLuaChunkStay(*Plugin);
+	
+	// Read the params:
+	cWorld * World = (cWorld *)tolua_tousertype(tolua_S, 1, NULL);
 	if (World == NULL)
 	{
-		LOGWARNING("cLuaChunkStay:Enable(): invalid world parameter");
+		LOGWARNING("World:ChunkStay(): invalid world parameter");
 		L.LogStackTrace();
 		return 0;
 	}
-	
-	ChunkStay->Enable(*World, tolua_S, 3, 4);
+	L.LogStack("Before AddChunks()");
+	if (!ChunkStay->AddChunks(2))
+	{
+		return 0;
+	}
+	L.LogStack("After params read");
+
+	ChunkStay->Enable(*World->GetChunkMap(), 3, 4);
 	return 0;
 }
 
@@ -2397,6 +2408,7 @@ void ManualBindings::Bind(lua_State * tolua_S)
 		tolua_endmodule(tolua_S);
 		
 		tolua_beginmodule(tolua_S, "cWorld");
+			tolua_function(tolua_S, "ChunkStay",                 tolua_cWorld_ChunkStay);
 			tolua_function(tolua_S, "DoWithBlockEntityAt",       tolua_DoWithXYZ<cWorld, cBlockEntity,        &cWorld::DoWithBlockEntityAt>);
 			tolua_function(tolua_S, "DoWithChestAt",             tolua_DoWithXYZ<cWorld, cChestEntity,        &cWorld::DoWithChestAt>);
 			tolua_function(tolua_S, "DoWithDispenserAt",         tolua_DoWithXYZ<cWorld, cDispenserEntity,    &cWorld::DoWithDispenserAt>);
@@ -2440,10 +2452,6 @@ void ManualBindings::Bind(lua_State * tolua_S)
 			tolua_function(tolua_S, "LogStackTrace",         tolua_cPluginManager_LogStackTrace);
 		tolua_endmodule(tolua_S);
 		
-		tolua_beginmodule(tolua_S, "cLuaChunkStay");
-			tolua_function(tolua_S, "Enable", tolua_cLuaChunkStay_Enable);
-		tolua_endmodule(tolua_S);
-		
 		tolua_beginmodule(tolua_S, "cPlayer");
 			tolua_function(tolua_S, "GetGroups",              tolua_cPlayer_GetGroups);
 			tolua_function(tolua_S, "GetResolvedPermissions", tolua_cPlayer_GetResolvedPermissions);

src/ChunkMap.cpp

@@ -912,9 +912,18 @@ void cChunkMap::SetChunkData(
 		}
 		
 		// Notify relevant ChunkStays:
-		for (cChunkStays::iterator itr = m_ChunkStays.begin(), end = m_ChunkStays.end(); itr != end; ++itr)
+		for (cChunkStays::iterator itr = m_ChunkStays.begin(); itr != m_ChunkStays.end(); )
 		{
-			(*itr)->ChunkAvailable(a_ChunkX, a_ChunkZ);
+			if ((*itr)->ChunkAvailable(a_ChunkX, a_ChunkZ))
+			{
+				cChunkStays::iterator cur = itr;
+				++itr;
+				m_ChunkStays.erase(cur);
+			}
+			else
+			{
+				++itr;
+			}
 		}  // for itr - m_ChunkStays[]
 	}
 

src/ChunkStay.cpp

@@ -105,7 +105,7 @@ void cChunkStay::Disable(void)
 
 
 
-void cChunkStay::ChunkAvailable(int a_ChunkX, int a_ChunkZ)
+bool cChunkStay::ChunkAvailable(int a_ChunkX, int a_ChunkZ)
 {
 	// Check if this is a chunk that we want:
 	bool IsMine = false;
@@ -120,15 +120,16 @@ void cChunkStay::ChunkAvailable(int a_ChunkX, int a_ChunkZ)
 	}  // for itr - m_OutstandingChunks[]
 	if (!IsMine)
 	{
-		return;
+		return false;
 	}
 	
 	// Call the appropriate callbacks:
 	OnChunkAvailable(a_ChunkX, a_ChunkZ);
 	if (m_OutstandingChunks.empty())
 	{
-		OnAllChunksAvailable();
+		return OnAllChunksAvailable();
 	}
+	return false;
 }
 
 

src/ChunkStay.h

@@ -32,48 +32,42 @@ This class is abstract, the descendants are expected to provide the OnChunkAvail
 the OnAllChunksAvailable() callback implementations. Note that those are called from the contexts of
 different threads' - the caller, the Loader or the Generator thread.
 */
-// tolua_begin
 class cChunkStay
 {
 public:
-	// tolua_end
 	cChunkStay(void);
 	~cChunkStay();
 	
-	// tolua_begin
-	
 	void Clear(void);
 	
 	/** Adds a chunk to be locked from unloading.
 	To be used only while the ChunkStay object is not enabled. */
-	void Add   (int a_ChunkX, int a_ChunkZ);
+	void Add(int a_ChunkX, int a_ChunkZ);
 	
 	/** Releases the chunk so that it's no longer locked from unloading.
 	To be used only while the ChunkStay object is not enabled. */
 	void Remove(int a_ChunkX, int a_ChunkZ);
 	
-	// tolua_end
-	
 	/** Enables the ChunkStay on the specified chunkmap, causing it to load and generate chunks.
 	All the contained chunks are queued for loading / generating. */
 	void Enable (cChunkMap & a_ChunkMap);
 	
-	// tolua_begin
-	
 	/** Disables the ChunkStay, the chunks are released and the ChunkStay
 	object can be edited with Add() and Remove() again*/
 	virtual void Disable(void);
 	
-	// tolua_end
-	
 	/** Returns all the chunks that should be kept */
 	const cChunkCoordsVector & GetChunks(void) const { return m_Chunks; }
 	
 	/** Called when a specific chunk become available. */
 	virtual void OnChunkAvailable(int a_ChunkX, int a_ChunkZ) = 0;
 	
-	/** Caled once all of the contained chunks are available. */
-	virtual void OnAllChunksAvailable(void) = 0;
+	/** Caled once all of the contained chunks are available.
+	If returns true, the ChunkStay is automatically disabled by the ChunkMap; if it returns false, the ChunkStay is kept. */
+	virtual bool OnAllChunksAvailable(void) = 0;
+	
+	/** Called by the ChunkMap when the ChunkStay is disabled. The object may choose to delete itself. */
+	virtual void OnDisabled(void) = 0;
 	
 protected:
 
@@ -92,9 +86,10 @@ protected:
 	
 	
 	/** Called by cChunkMap when a chunk is available, checks m_NumLoaded and triggers the appropriate callbacks.
-	May be called for chunks outside this ChunkStay. */
-	void ChunkAvailable(int a_ChunkX, int a_ChunkZ);
-} ;  // tolua_export
+	May be called for chunks outside this ChunkStay.
+	Returns true if the ChunkStay is to be automatically disabled by the ChunkMap; returns false to keep the ChunkStay. */
+	bool ChunkAvailable(int a_ChunkX, int a_ChunkZ);
+} ;
 
 
 

src/LightingThread.cpp

@@ -548,9 +548,21 @@ cLightingThread::cLightingChunkStay::cLightingChunkStay(cLightingThread & a_Ligh
 
 
 
-void cLightingThread::cLightingChunkStay::OnAllChunksAvailable(void)
+bool cLightingThread::cLightingChunkStay::OnAllChunksAvailable(void)
 {
 	m_LightingThread.QueueChunkStay(*this);
+	
+	// Keep the ChunkStay alive:
+	return false;
+}
+
+
+
+
+
+void cLightingThread::cLightingChunkStay::OnDisabled(void)
+{
+	// Nothing needed in this callback
 }
 
 

src/LightingThread.h

@@ -83,7 +83,8 @@ protected:
 		
 	protected:
 		virtual void OnChunkAvailable(int a_ChunkX, int a_ChunkZ) override {}
-		virtual void OnAllChunksAvailable(void) override;
+		virtual bool OnAllChunksAvailable(void) override;
+		virtual void OnDisabled(void) override;
 	} ;
 	
 	typedef std::list<cChunkStay *> cChunkStays;