From 257c5a1a54396a3610f63bf469d6cf50ec76aef5 Mon Sep 17 00:00:00 2001 From: Mattes D Date: Sun, 12 Jun 2016 18:11:40 +0200 Subject: [PATCH] cPluginManager: Use a callback for command handler registration. --- src/Bindings/LuaState.cpp | 4 ++ src/Bindings/ManualBindings.cpp | 94 ++++++++++++++++++--------- src/Bindings/Plugin.h | 15 ----- src/Bindings/PluginLua.cpp | 109 +------------------------------- src/Bindings/PluginLua.h | 23 ------- src/Bindings/PluginManager.cpp | 47 ++++++++------ src/Bindings/PluginManager.h | 49 ++++++++++++-- src/Server.cpp | 35 +++++++--- 8 files changed, 167 insertions(+), 209 deletions(-) diff --git a/src/Bindings/LuaState.cpp b/src/Bindings/LuaState.cpp index 85c6c2e12..0d958cc20 100644 --- a/src/Bindings/LuaState.cpp +++ b/src/Bindings/LuaState.cpp @@ -1021,6 +1021,10 @@ bool cLuaState::GetStackValue(int a_StackPos, cCallback & a_Callback) bool cLuaState::GetStackValue(int a_StackPos, cCallbackPtr & a_Callback) { + if (a_Callback == nullptr) + { + a_Callback = std::make_shared(); + } return a_Callback->RefStack(*this, a_StackPos); } diff --git a/src/Bindings/ManualBindings.cpp b/src/Bindings/ManualBindings.cpp index 28120fdc5..12cdd3d71 100644 --- a/src/Bindings/ManualBindings.cpp +++ b/src/Bindings/ManualBindings.cpp @@ -43,6 +43,50 @@ +//////////////////////////////////////////////////////////////////////////////// +// LuaCommandHandler: + +/** Defines a bridge between cPluginManager::cCommandHandler and cLuaState::cCallback. */ +class LuaCommandHandler: + public cPluginManager::cCommandHandler +{ +public: + LuaCommandHandler(cLuaState::cCallbackPtr a_Callback): + m_Callback(a_Callback) + { + } + + virtual bool ExecuteCommand( + const AStringVector & a_Split, + cPlayer * a_Player, + const AString & a_Command, + cCommandOutputCallback * a_Output + ) override + { + bool res = false; + AString s; + if (!m_Callback->Call(a_Split, a_Player, a_Command, cLuaState::Return, res, s)) + { + return false; + } + if (res && (a_Output != nullptr) && !s.empty()) + { + a_Output->Out(s); + } + return res; + } + +protected: + cLuaState::cCallbackPtr m_Callback; +}; + + + + + +//////////////////////////////////////////////////////////////////////////////// +// cManualBindings: + // Better error reporting for Lua int cManualBindings::tolua_do_error(lua_State * L, const char * a_pMsg, tolua_Error * a_pToLuaError) { @@ -1268,12 +1312,13 @@ static int tolua_cPluginManager_ForEachConsoleCommand(lua_State * tolua_S) -static int tolua_cPluginManager_BindCommand(lua_State * L) +static int tolua_cPluginManager_BindCommand(lua_State * a_LuaState) { /* Function signatures: cPluginManager:BindCommand(Command, Permission, Function, HelpString) cPluginManager.BindCommand(Command, Permission, Function, HelpString) -- without the "self" param */ + cLuaState L(a_LuaState); cPluginLua * Plugin = cManualBindings::GetLuaPlugin(L); if (Plugin == nullptr) { @@ -1306,29 +1351,24 @@ static int tolua_cPluginManager_BindCommand(lua_State * L) return 0; } cPluginManager * self = cPluginManager::Get(); - AString Command (tolua_tocppstring(L, idx, "")); - AString Permission(tolua_tocppstring(L, idx + 1, "")); - AString HelpString(tolua_tocppstring(L, idx + 3, "")); - - // Store the function reference: - lua_pop(L, 1); // Pop the help string off the stack - int FnRef = luaL_ref(L, LUA_REGISTRYINDEX); // Store function reference - if (FnRef == LUA_REFNIL) + AString Command, Permission, HelpString; + cLuaState::cCallbackPtr Handler; + L.GetStackValues(idx, Command, Permission, Handler, HelpString); + if (!Handler->IsValid()) { LOGERROR("\"BindCommand\": Cannot create a function reference. Command \"%s\" not bound.", Command.c_str()); return 0; } - if (!self->BindCommand(Command, Plugin, Permission, HelpString)) + auto CommandHandler = std::make_shared(Handler); + if (!self->BindCommand(Command, Plugin, CommandHandler, Permission, HelpString)) { // Refused. Possibly already bound. Error message has been given, display the callstack: - cLuaState LS(L); - LS.LogStackTrace(); + L.LogStackTrace(); return 0; } - Plugin->BindCommand(Command, FnRef); - lua_pushboolean(L, true); + L.Push(true); return 1; } @@ -1336,7 +1376,7 @@ static int tolua_cPluginManager_BindCommand(lua_State * L) -static int tolua_cPluginManager_BindConsoleCommand(lua_State * L) +static int tolua_cPluginManager_BindConsoleCommand(lua_State * a_LuaState) { /* Function signatures: cPluginManager:BindConsoleCommand(Command, Function, HelpString) @@ -1344,6 +1384,7 @@ static int tolua_cPluginManager_BindConsoleCommand(lua_State * L) */ // Get the plugin identification out of LuaState: + cLuaState L(a_LuaState); cPluginLua * Plugin = cManualBindings::GetLuaPlugin(L); if (Plugin == nullptr) { @@ -1375,28 +1416,23 @@ static int tolua_cPluginManager_BindConsoleCommand(lua_State * L) return 0; } cPluginManager * self = cPluginManager::Get(); - AString Command (tolua_tocppstring(L, idx, "")); - AString HelpString(tolua_tocppstring(L, idx + 2, "")); - - // Store the function reference: - lua_pop(L, 1); // Pop the help string off the stack - int FnRef = luaL_ref(L, LUA_REGISTRYINDEX); // Store function reference - if (FnRef == LUA_REFNIL) + AString Command, HelpString; + cLuaState::cCallbackPtr Handler; + L.GetStackValues(idx, Command, Handler, HelpString); + if (!Handler->IsValid()) { - LOGERROR("\"BindConsoleCommand\": Cannot create a function reference. Console Command \"%s\" not bound.", Command.c_str()); + LOGERROR("\"BindConsoleCommand\": Cannot create a function reference. Console command \"%s\" not bound.", Command.c_str()); return 0; } - if (!self->BindConsoleCommand(Command, Plugin, HelpString)) + auto CommandHandler = std::make_shared(Handler); + if (!self->BindConsoleCommand(Command, Plugin, CommandHandler, HelpString)) { // Refused. Possibly already bound. Error message has been given, display the callstack: - cLuaState LS(L); - LS.LogStackTrace(); + L.LogStackTrace(); return 0; } - - Plugin->BindConsoleCommand(Command, FnRef); - lua_pushboolean(L, true); + L.Push(true); return 1; } diff --git a/src/Bindings/Plugin.h b/src/Bindings/Plugin.h index 3ee417361..3276fde67 100644 --- a/src/Bindings/Plugin.h +++ b/src/Bindings/Plugin.h @@ -110,21 +110,6 @@ public: virtual bool OnWorldStarted (cWorld & a_World) = 0; virtual bool OnWorldTick (cWorld & a_World, std::chrono::milliseconds a_Dt, std::chrono::milliseconds a_LastTickDurationMSec) = 0; - /** Handles the command split into a_Split, issued by player a_Player. - Command permissions have already been checked. - Returns true if command handled successfully. */ - virtual bool HandleCommand(const AStringVector & a_Split, cPlayer & a_Player, const AString & a_FullCommand) = 0; - - /** Handles the console command split into a_Split. - Returns true if command handled successfully. Output is to be sent to the a_Output callback. */ - virtual bool HandleConsoleCommand(const AStringVector & a_Split, cCommandOutputCallback & a_Output, const AString & a_FullCommand) = 0; - - /** All bound commands are to be removed, do any language-dependent cleanup here */ - virtual void ClearCommands(void) {} - - /** All bound console commands are to be removed, do any language-dependent cleanup here */ - virtual void ClearConsoleCommands(void) {} - // tolua_begin const AString & GetName(void) const { return m_Name; } void SetName(const AString & a_Name) { m_Name = a_Name; } diff --git a/src/Bindings/PluginLua.cpp b/src/Bindings/PluginLua.cpp index 04ea76f0a..ee0d289a1 100644 --- a/src/Bindings/PluginLua.cpp +++ b/src/Bindings/PluginLua.cpp @@ -62,9 +62,7 @@ void cPluginLua::Close(void) return; } - // Remove the command bindings and web tabs: - ClearCommands(); - ClearConsoleCommands(); + // Remove the web tabs: ClearWebTabs(); // Release all the references in the hook map: @@ -994,91 +992,6 @@ bool cPluginLua::OnWorldTick(cWorld & a_World, std::chrono::milliseconds a_Dt, s -bool cPluginLua::HandleCommand(const AStringVector & a_Split, cPlayer & a_Player, const AString & a_FullCommand) -{ - ASSERT(!a_Split.empty()); - cOperation op(*this); - CommandMap::iterator cmd = m_Commands.find(a_Split[0]); - if (cmd == m_Commands.end()) - { - LOGWARNING("Command handler is registered in cPluginManager but not in cPlugin, wtf? Command \"%s\".", a_Split[0].c_str()); - return false; - } - - bool res = false; - op().Call(cmd->second, a_Split, &a_Player, a_FullCommand, cLuaState::Return, res); - return res; -} - - - - - -bool cPluginLua::HandleConsoleCommand(const AStringVector & a_Split, cCommandOutputCallback & a_Output, const AString & a_FullCommand) -{ - ASSERT(!a_Split.empty()); - cOperation op(*this); - CommandMap::iterator cmd = m_ConsoleCommands.find(a_Split[0]); - if (cmd == m_ConsoleCommands.end()) - { - LOGWARNING("Console command handler is registered in cPluginManager but not in cPlugin, wtf? Console command \"%s\", plugin \"%s\".", - a_Split[0].c_str(), GetName().c_str() - ); - return false; - } - - bool res = false; - AString str; - op().Call(cmd->second, a_Split, a_FullCommand, cLuaState::Return, res, str); - if (res && !str.empty()) - { - a_Output.Out(str); - } - return res; -} - - - - - -void cPluginLua::ClearCommands(void) -{ - cOperation op(*this); - - // Unreference the bound functions so that Lua can GC them - if (m_LuaState != nullptr) - { - for (CommandMap::iterator itr = m_Commands.begin(), end = m_Commands.end(); itr != end; ++itr) - { - luaL_unref(m_LuaState, LUA_REGISTRYINDEX, itr->second); - } - } - m_Commands.clear(); -} - - - - - -void cPluginLua::ClearConsoleCommands(void) -{ - cOperation op(*this); - - // Unreference the bound functions so that Lua can GC them - if (m_LuaState != nullptr) - { - for (CommandMap::iterator itr = m_ConsoleCommands.begin(), end = m_ConsoleCommands.end(); itr != end; ++itr) - { - luaL_unref(m_LuaState, LUA_REGISTRYINDEX, itr->second); - } - } - m_ConsoleCommands.clear(); -} - - - - - bool cPluginLua::CanAddOldStyleHook(int a_HookType) { const char * FnName = GetHookFnName(a_HookType); @@ -1227,26 +1140,6 @@ int cPluginLua::CallFunctionFromForeignState( -void cPluginLua::BindCommand(const AString & a_Command, int a_FnRef) -{ - ASSERT(m_Commands.find(a_Command) == m_Commands.end()); - m_Commands[a_Command] = a_FnRef; -} - - - - - -void cPluginLua::BindConsoleCommand(const AString & a_Command, int a_FnRef) -{ - ASSERT(m_ConsoleCommands.find(a_Command) == m_ConsoleCommands.end()); - m_ConsoleCommands[a_Command] = a_FnRef; -} - - - - - bool cPluginLua::CallbackWindowClosing(int a_FnRef, cWindow & a_Window, cPlayer & a_Player, bool a_CanRefuse) { ASSERT(a_FnRef != LUA_REFNIL); diff --git a/src/Bindings/PluginLua.h b/src/Bindings/PluginLua.h index 8e40ff584..9e6b53e75 100644 --- a/src/Bindings/PluginLua.h +++ b/src/Bindings/PluginLua.h @@ -138,23 +138,9 @@ public: virtual bool OnWorldStarted (cWorld & a_World) override; virtual bool OnWorldTick (cWorld & a_World, std::chrono::milliseconds a_Dt, std::chrono::milliseconds a_LastTickDurationMSec) override; - virtual bool HandleCommand(const AStringVector & a_Split, cPlayer & a_Player, const AString & a_FullCommand) override; - - virtual bool HandleConsoleCommand(const AStringVector & a_Split, cCommandOutputCallback & a_Output, const AString & a_FullCommand) override; - - virtual void ClearCommands(void) override; - - virtual void ClearConsoleCommands(void) override; - /** Returns true if the plugin contains the function for the specified hook type, using the old-style registration (#121) */ bool CanAddOldStyleHook(int a_HookType); - /** Binds the command to call the function specified by a Lua function reference. Simply adds to CommandMap. */ - void BindCommand(const AString & a_Command, int a_FnRef); - - /** Binds the console command to call the function specified by a Lua function reference. Simply adds to CommandMap. */ - void BindConsoleCommand(const AString & a_Command, int a_FnRef); - /** Calls the plugin-specified "cLuaWindow closing" callback. Returns true only if the callback returned true */ bool CallbackWindowClosing(int a_FnRef, cWindow & a_Window, cPlayer & a_Player, bool a_CanRefuse); @@ -186,9 +172,6 @@ public: } protected: - /** Maps command name into Lua function reference */ - typedef std::map CommandMap; - /** Provides an array of Lua function references */ typedef std::vector cLuaCallbacks; @@ -199,12 +182,6 @@ protected: /** The plugin's Lua state. */ cLuaState m_LuaState; - /** In-game commands that the plugin has registered. */ - CommandMap m_Commands; - - /** Console commands that the plugin has registered. */ - CommandMap m_ConsoleCommands; - /** Hooks that the plugin has registered. */ cHookMap m_HookMap; diff --git a/src/Bindings/PluginManager.cpp b/src/Bindings/PluginManager.cpp index 203450505..19d2e8b4d 100644 --- a/src/Bindings/PluginManager.cpp +++ b/src/Bindings/PluginManager.cpp @@ -1569,9 +1569,9 @@ cPluginManager::CommandResult cPluginManager::HandleCommand(cPlayer & a_Player, return crNoPermission; } - ASSERT(cmd->second.m_Plugin != nullptr); + ASSERT(cmd->second.m_Handler != nullptr); - if (!cmd->second.m_Plugin->HandleCommand(Split, a_Player, a_Command)) + if (!cmd->second.m_Handler->ExecuteCommand(Split, &a_Player, a_Command, nullptr)) { return crError; } @@ -1654,11 +1654,6 @@ void cPluginManager::RemoveHooks(cPlugin * a_Plugin) void cPluginManager::RemovePluginCommands(cPlugin * a_Plugin) { - if (a_Plugin != nullptr) - { - a_Plugin->ClearCommands(); - } - for (CommandMap::iterator itr = m_Commands.begin(); itr != m_Commands.end();) { if (itr->second.m_Plugin == a_Plugin) @@ -1694,7 +1689,13 @@ bool cPluginManager::IsPluginLoaded(const AString & a_PluginName) -bool cPluginManager::BindCommand(const AString & a_Command, cPlugin * a_Plugin, const AString & a_Permission, const AString & a_HelpString) +bool cPluginManager::BindCommand( + const AString & a_Command, + cPlugin * a_Plugin, + cCommandHandlerPtr a_Handler, + const AString & a_Permission, + const AString & a_HelpString +) { CommandMap::iterator cmd = m_Commands.find(a_Command); if (cmd != m_Commands.end()) @@ -1703,9 +1704,11 @@ bool cPluginManager::BindCommand(const AString & a_Command, cPlugin * a_Plugin, return false; } - m_Commands[a_Command].m_Plugin = a_Plugin; - m_Commands[a_Command].m_Permission = a_Permission; - m_Commands[a_Command].m_HelpString = a_HelpString; + auto & reg = m_Commands[a_Command]; + reg.m_Plugin = a_Plugin; + reg.m_Handler = a_Handler; + reg.m_Permission = a_Permission; + reg.m_HelpString = a_HelpString; return true; } @@ -1768,11 +1771,6 @@ cPluginManager::CommandResult cPluginManager::ForceExecuteCommand(cPlayer & a_Pl void cPluginManager::RemovePluginConsoleCommands(cPlugin * a_Plugin) { - if (a_Plugin != nullptr) - { - a_Plugin->ClearConsoleCommands(); - } - for (CommandMap::iterator itr = m_ConsoleCommands.begin(); itr != m_ConsoleCommands.end();) { if (itr->second.m_Plugin == a_Plugin) @@ -1792,7 +1790,12 @@ void cPluginManager::RemovePluginConsoleCommands(cPlugin * a_Plugin) -bool cPluginManager::BindConsoleCommand(const AString & a_Command, cPlugin * a_Plugin, const AString & a_HelpString) +bool cPluginManager::BindConsoleCommand( + const AString & a_Command, + cPlugin * a_Plugin, + cCommandHandlerPtr a_Handler, + const AString & a_HelpString +) { CommandMap::iterator cmd = m_ConsoleCommands.find(a_Command); if (cmd != m_ConsoleCommands.end()) @@ -1808,9 +1811,11 @@ bool cPluginManager::BindConsoleCommand(const AString & a_Command, cPlugin * a_P return false; } - m_ConsoleCommands[a_Command].m_Plugin = a_Plugin; - m_ConsoleCommands[a_Command].m_Permission = ""; - m_ConsoleCommands[a_Command].m_HelpString = a_HelpString; + auto & reg = m_ConsoleCommands[a_Command]; + reg.m_Plugin = a_Plugin; + reg.m_Handler = a_Handler; + reg.m_Permission = ""; + reg.m_HelpString = a_HelpString; return true; } @@ -1873,7 +1878,7 @@ bool cPluginManager::ExecuteConsoleCommand(const AStringVector & a_Split, cComma return (res == crExecuted); } - return cmd->second.m_Plugin->HandleConsoleCommand(a_Split, a_Output, a_Command); + return cmd->second.m_Handler->ExecuteCommand(a_Split, nullptr, a_Command, &a_Output); } diff --git a/src/Bindings/PluginManager.h b/src/Bindings/PluginManager.h index a97582fbe..0423d6af1 100644 --- a/src/Bindings/PluginManager.h +++ b/src/Bindings/PluginManager.h @@ -152,6 +152,7 @@ public: HOOK_MAX = HOOK_NUM_HOOKS - 1, } ; // tolua_export + /** Used as a callback for enumerating bound commands */ class cCommandEnumCallback { @@ -164,6 +165,30 @@ public: virtual bool Command(const AString & a_Command, const cPlugin * a_Plugin, const AString & a_Permission, const AString & a_HelpString) = 0; } ; + + /** Interface that must be provided by any class that implements a command handler, either in-game or console command. */ + class cCommandHandler + { + public: + // Force a virtual destructor in descendants + virtual ~cCommandHandler() {} + + /** Executes the specified in-game command. + a_Split is the command string, split at the spaces. + a_Player is the player executing the command, nullptr in case of the console. + a_Command is the entire command string. + a_Output is the sink into which the additional text returned by the command handler should be sent; only used for console commands. */ + virtual bool ExecuteCommand( + const AStringVector & a_Split, + cPlayer * a_Player, + const AString & a_Command, + cCommandOutputCallback * a_Output = nullptr + ) = 0; + }; + + typedef SharedPtr cCommandHandlerPtr; + + /** The interface used for enumerating and extern-calling plugins */ typedef cItemCallback cPluginCallback; @@ -281,8 +306,16 @@ public: /** Returns true if the specified plugin is loaded. */ bool IsPluginLoaded(const AString & a_PluginName); // tolua_export - /** Binds a command to the specified plugin. Returns true if successful, false if command already bound. */ - bool BindCommand(const AString & a_Command, cPlugin * a_Plugin, const AString & a_Permission, const AString & a_HelpString); // Exported in ManualBindings.cpp, without the a_Plugin param + /** Binds a command to the specified handler. + Returns true if successful, false if command already bound. + Exported in ManualBindings.cpp. */ + bool BindCommand( + const AString & a_Command, + cPlugin * a_Plugin, + cCommandHandlerPtr a_Handler, + const AString & a_Permission, + const AString & a_HelpString + ); /** Calls a_Callback for each bound command, returns true if all commands were enumerated */ bool ForEachCommand(cCommandEnumCallback & a_Callback); // Exported in ManualBindings.cpp @@ -302,8 +335,15 @@ public: /** Removes all console command bindings that the specified plugin has made */ void RemovePluginConsoleCommands(cPlugin * a_Plugin); - /** Binds a console command to the specified plugin. Returns true if successful, false if command already bound. */ - bool BindConsoleCommand(const AString & a_Command, cPlugin * a_Plugin, const AString & a_HelpString); // Exported in ManualBindings.cpp, without the a_Plugin param + /** Binds a console command to the specified handler. + Returns true if successful, false if command already bound. + Exported in ManualBindings.cpp. */ + bool BindConsoleCommand( + const AString & a_Command, + cPlugin * a_Plugin, + cCommandHandlerPtr a_Handler, + const AString & a_HelpString + ); /** Calls a_Callback for each bound console command, returns true if all commands were enumerated */ bool ForEachConsoleCommand(cCommandEnumCallback & a_Callback); // Exported in ManualBindings.cpp @@ -348,6 +388,7 @@ private: cPlugin * m_Plugin; AString m_Permission; // Not used for console commands AString m_HelpString; + cCommandHandlerPtr m_Handler; } ; typedef std::map HookMap; diff --git a/src/Server.cpp b/src/Server.cpp index 5548e77d1..8405109de 100644 --- a/src/Server.cpp +++ b/src/Server.cpp @@ -607,18 +607,35 @@ void cServer::PrintHelp(const AStringVector & a_Split, cCommandOutputCallback & void cServer::BindBuiltInConsoleCommands(void) { + // Create an empty handler - the actual handling for the commands is performed before they are handed off to cPluginManager + class cEmptyHandler: + public cPluginManager::cCommandHandler + { + virtual bool ExecuteCommand( + const AStringVector & a_Split, + cPlayer * a_Player, + const AString & a_Command, + cCommandOutputCallback * a_Output = nullptr + ) override + { + return false; + } + }; + auto handler = std::make_shared(); + + // Register internal commands: cPluginManager * PlgMgr = cPluginManager::Get(); - PlgMgr->BindConsoleCommand("help", nullptr, "Shows the available commands"); - PlgMgr->BindConsoleCommand("reload", nullptr, "Reloads all plugins"); - PlgMgr->BindConsoleCommand("restart", nullptr, "Restarts the server cleanly"); - PlgMgr->BindConsoleCommand("stop", nullptr, "Stops the server cleanly"); - PlgMgr->BindConsoleCommand("chunkstats", nullptr, "Displays detailed chunk memory statistics"); - PlgMgr->BindConsoleCommand("load ", nullptr, "Adds and enables the specified plugin"); - PlgMgr->BindConsoleCommand("unload ", nullptr, "Disables the specified plugin"); - PlgMgr->BindConsoleCommand("destroyentities", nullptr, "Destroys all entities in all worlds"); + PlgMgr->BindConsoleCommand("help", nullptr, handler, "Shows the available commands"); + PlgMgr->BindConsoleCommand("reload", nullptr, handler, "Reloads all plugins"); + PlgMgr->BindConsoleCommand("restart", nullptr, handler, "Restarts the server cleanly"); + PlgMgr->BindConsoleCommand("stop", nullptr, handler, "Stops the server cleanly"); + PlgMgr->BindConsoleCommand("chunkstats", nullptr, handler, "Displays detailed chunk memory statistics"); + PlgMgr->BindConsoleCommand("load", nullptr, handler, "Adds and enables the specified plugin"); + PlgMgr->BindConsoleCommand("unload", nullptr, handler, "Disables the specified plugin"); + PlgMgr->BindConsoleCommand("destroyentities", nullptr, handler, "Destroys all entities in all worlds"); #if defined(_MSC_VER) && defined(_DEBUG) && defined(ENABLE_LEAK_FINDER) - PlgMgr->BindConsoleCommand("dumpmem", nullptr, " - Dumps all used memory blocks together with their callstacks into memdump.xml"); + PlgMgr->BindConsoleCommand("dumpmem", nullptr, handler, " - Dumps all used memory blocks together with their callstacks into memdump.xml"); #endif }