From 9cebc9157cf43ba639227b9d79b980b3613dda1e Mon Sep 17 00:00:00 2001 From: madmaxoft Date: Mon, 10 Feb 2014 22:47:10 +0100 Subject: [PATCH] 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 | 2 - src/Bindings/LuaChunkStay.cpp | 146 ++++++++++++++++++++++++++------ src/Bindings/LuaChunkStay.h | 48 ++++++----- src/Bindings/ManualBindings.cpp | 40 +++++---- src/ChunkMap.cpp | 13 ++- src/ChunkStay.cpp | 7 +- src/ChunkStay.h | 27 +++--- src/LightingThread.cpp | 14 ++- src/LightingThread.h | 3 +- 9 files changed, 213 insertions(+), 87 deletions(-) diff --git a/src/Bindings/AllToLua.pkg b/src/Bindings/AllToLua.pkg index a78ee7d56..f65aed9bb 100644 --- a/src/Bindings/AllToLua.pkg +++ b/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" diff --git a/src/Bindings/LuaChunkStay.cpp b/src/Bindings/LuaChunkStay.cpp index f84964f56..0e982637f 100644 --- a/src/Bindings/LuaChunkStay.cpp +++ b/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; +} + + + + diff --git a/src/Bindings/LuaChunkStay.h b/src/Bindings/LuaChunkStay.h index 0ab73f669..49ab9a0ad 100644 --- a/src/Bindings/LuaChunkStay.h +++ b/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 diff --git a/src/Bindings/ManualBindings.cpp b/src/Bindings/ManualBindings.cpp index 3571fd7ea..f0bad92e8 100644 --- a/src/Bindings/ManualBindings.cpp +++ b/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); tolua_function(tolua_S, "DoWithChestAt", tolua_DoWithXYZ); tolua_function(tolua_S, "DoWithDispenserAt", tolua_DoWithXYZ); @@ -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); diff --git a/src/ChunkMap.cpp b/src/ChunkMap.cpp index 7726a0b7e..0c5a8d9b9 100644 --- a/src/ChunkMap.cpp +++ b/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[] } diff --git a/src/ChunkStay.cpp b/src/ChunkStay.cpp index cce8d5f4d..6b1d5ee34 100644 --- a/src/ChunkStay.cpp +++ b/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; } diff --git a/src/ChunkStay.h b/src/ChunkStay.h index 1e9cd69e8..80b084aee 100644 --- a/src/ChunkStay.h +++ b/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); +} ; diff --git a/src/LightingThread.cpp b/src/LightingThread.cpp index 688f119ff..9c81d004d 100644 --- a/src/LightingThread.cpp +++ b/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 } diff --git a/src/LightingThread.h b/src/LightingThread.h index 0e17c485f..81dd9d61f 100644 --- a/src/LightingThread.h +++ b/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 cChunkStays;