From 9cebc9157cf43ba639227b9d79b980b3613dda1e Mon Sep 17 00:00:00 2001 From: madmaxoft Date: Mon, 10 Feb 2014 22:47:10 +0100 Subject: 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 ++++++----- 4 files changed, 172 insertions(+), 64 deletions(-) (limited to 'src/Bindings') 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; + } + + // 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); } - m_LuaState.Attach(a_LuaState); - m_OnChunkAvailable.RefStack(m_LuaState, a_OnChunkAvailableStackPos); - m_OnAllChunksAvailable.RefStack(m_LuaState, a_OnAllChunksAvailableStackPos); - super::Enable(*a_World.GetChunkMap()); + + // 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); -- cgit v1.2.3