Fixed ConVar/ServerCommand registration conflicts.

Fixed Client/Say/Server commands decorators from raising on unload when no callback was registered. Remove the public/private hack. Fixed server commands from potentially unregistering overrides rather than themselves. Added the ability to override convars as server commands and prevent their assignment from the console. Fixed memory leak for new convars allocated by the interpreter. Fixed execute_server_command, queue_server_command and insert_server_command being executed with an extra empty space.
Source-Python-Dev-Team · Oct 7, 2021 · 02e3490 · 02e3490
1 parent ff7819a
commit 02e3490
Show file tree

Hide file tree

Showing 16 changed files with 209 additions and 93 deletions.
diff --git a/addons/source-python/packages/source-python/commands/command.py b/addons/source-python/packages/source-python/commands/command.py
@@ -53,4 +53,12 @@ def __call__(self, callback):
 
     def _unload_instance(self):
         """Unregister the commands."""
-        self._manager_class.unregister_commands(self.names, self.callback)
+        # Get the registered callback
+        callback = self.callback
+
+        # Was no callback registered?
+        if callback is None:
+            return
+
+        # Unregister the commands
+        self._manager_class.unregister_commands(self.names, callback)
diff --git a/src/core/modules/commands/commands.h b/src/core/modules/commands/commands.h
@@ -30,7 +30,7 @@
 // Includes.
 //-----------------------------------------------------------------------------
 #include "utilities/wrap_macros.h"
-#include "convar.h"
+#include "utilities/convar.h"
 #include "utilities/ipythongenerator.h"
 #include "boost/typeof/typeof.hpp" 
 
@@ -99,34 +99,6 @@ class CCommandExt
 };
 
 
-//-----------------------------------------------------------------------------
-// ConCommandBase extension class.
-//-----------------------------------------------------------------------------
-class ConCommandBaseExt
-{
-public:
-	static int GetFlags(ConCommandBase* command)
-	{
-		return command->m_nFlags;
-	}
-
-	static void SetFlags(ConCommandBase* command, int flags)
-	{
-		command->m_nFlags = flags;
-	}
-
-	static void AddFlags(ConCommandBase* command, int flags)
-	{
-		command->m_nFlags |= flags;
-	}
-
-	static void RemoveFlags(ConCommandBase* command, int flags)
-	{
-		command->m_nFlags &= ~flags;
-	}
-};
-
-
 //-----------------------------------------------------------------------------
 // ConCommand extension class.
 //-----------------------------------------------------------------------------

diff --git a/src/core/modules/commands/commands_client.cpp b/src/core/modules/commands/commands_client.cpp
@@ -27,14 +27,11 @@
 //-----------------------------------------------------------------------------
 // Includes
 //-----------------------------------------------------------------------------
-// This is required for accessing m_nFlags without patching convar.h
-#define private public
-
 #include "boost/unordered_map.hpp"
 #include "commands_client.h"
 #include "commands.h"
 #include "edict.h"
-#include "convar.h"
+#include "utilities/convar.h"
 #include "engine/iserverplugin.h"
 #include "utilities/call_python.h"
 #include "boost/python/call.hpp"

diff --git a/src/core/modules/commands/commands_client.h b/src/core/modules/commands/commands_client.h
@@ -56,7 +56,7 @@ class CClientCommandManager
 
 	const char* GetName();
 
-private:
+public:
 	CListenerManager m_vecCallables;
 	const char* m_Name;
 };

diff --git a/src/core/modules/commands/commands_client_wrap.cpp b/src/core/modules/commands/commands_client_wrap.cpp
@@ -27,9 +27,6 @@
 //-----------------------------------------------------------------------------
 // Includes.
 //-----------------------------------------------------------------------------
-// This is required for accessing m_nFlags without patching convar.h
-#define private public
-
 #include "export_main.h"
 #include "utilities/wrap_macros.h"
 #include "modules/memory/memory_tools.h"

diff --git a/src/core/modules/commands/commands_say.cpp b/src/core/modules/commands/commands_say.cpp
@@ -27,9 +27,6 @@
 //-----------------------------------------------------------------------------
 // Includes.
 //-----------------------------------------------------------------------------
-// This is required for accessing m_nFlags without patching convar.h
-#define private public
-
 #include <iostream>
 #include <string>
 #include "utilities/call_python.h"
@@ -38,7 +35,7 @@
 #include "boost/unordered_map.hpp"
 #include "sp_main.h"
 #include "modules/listeners/listeners_manager.h"
-#include "convar.h"
+#include "utilities/convar.h"
 
 #include "commands_say.h"
 #include "commands.h"
@@ -158,7 +155,7 @@ SayConCommand* SayConCommand::CreateCommand(const char* szName, const char* szHe
 	{
 		// Store the current command's help text and flags
 		szHelpTextCopy = strdup(pConCommand->GetHelpText());
-		iFlags = pConCommand->m_nFlags;
+		iFlags = GetConCommandFlags(pConCommand);
 
 		// Unregister the old command
 		g_pCVar->UnregisterConCommand(pConCommand);
@@ -192,8 +189,8 @@ SayConCommand::~SayConCommand()
 	// Get the ConCommand instance
 	ConCommand* pConCommand = g_pCVar->FindCommand(m_Name);
 
-	// Was the command overwritten as a ConVar or by another DLL?
-	if (pConCommand && pConCommand->GetDLLIdentifier() == CVarDLLIdentifier())
+	// Make sure we only unregister ourselves
+	if (pConCommand == this)
 	{
 		// Unregister the ConCommand
 		g_pCVar->UnregisterConCommand(pConCommand);

diff --git a/src/core/modules/commands/commands_say.h b/src/core/modules/commands/commands_say.h
@@ -50,7 +50,7 @@ class SayConCommand: public ConCommand
 protected:
 	void Dispatch( const CCommand& command );
 
-private:
+public:
 	SayConCommand(ConCommand* pConCommand, const char* szName, const char* szHelpText, int iFlags);
 	const char* m_Name;
 	ConCommand* m_pOldCommand;
@@ -86,7 +86,7 @@ class CSayCommandManager
 
 	const char* GetName();
 
-private:
+public:
 	const char* m_Name;
 	CListenerManager m_vecCallables;
 };

diff --git a/src/core/modules/commands/commands_say_wrap.cpp b/src/core/modules/commands/commands_say_wrap.cpp
@@ -27,9 +27,6 @@
 //-----------------------------------------------------------------------------
 // Includes.
 //-----------------------------------------------------------------------------
-// This is required for accessing m_nFlags without patching convar.h
-#define private public
-
 #include "export_main.h"
 #include "utilities/wrap_macros.h"
 #include "modules/memory/memory_tools.h"

diff --git a/src/core/modules/commands/commands_server.cpp b/src/core/modules/commands/commands_server.cpp
@@ -27,9 +27,6 @@
 //-----------------------------------------------------------------------------
 // Includes
 //-----------------------------------------------------------------------------
-// This is required for accessing m_nFlags without patching convar.h
-#define private public
-
 #include "utilities/call_python.h"
 
 #include "boost/unordered_map.hpp"
@@ -56,8 +53,12 @@ class CPluginConVarAccessor : public IConCommandBaseAccessor
 public:
 	virtual bool RegisterConCommandBase(ConCommandBase* pCommand)
 	{
-		g_pCVar->RegisterConCommand(pCommand);
-		return true;
+		if (!g_pCVar->FindCommandBase(pCommand->GetName())) {
+			g_pCVar->RegisterConCommand(pCommand);
+			return true;
+		}
+
+		return false;
 	}
 };
 
@@ -121,15 +122,17 @@ CServerCommandManager* CServerCommandManager::CreateCommand(const char* szName,
 	char* szHelpTextCopy = NULL;
 
 	// FInd if the command already exists
-	ConCommand* pConCommand = g_pCVar->FindCommand(szName);
+	ConCommandBase* pConCommand = g_pCVar->FindCommandBase(szName);
 	if( pConCommand )
 	{
 		// Store the current command's help text and flags
 		szHelpTextCopy = strdup(pConCommand->GetHelpText());
-		iFlags = pConCommand->m_nFlags;
+		iFlags = GetConCommandFlags(pConCommand);
 
 		// Unregister the old command
-		g_pCVar->UnregisterConCommand(pConCommand);
+		if (pConCommand->IsRegistered()) {
+			g_pCVar->UnregisterConCommand(pConCommand);
+		}
 	}
 	else if( szHelpText != NULL )
 	{
@@ -144,7 +147,7 @@ CServerCommandManager* CServerCommandManager::CreateCommand(const char* szName,
 //-----------------------------------------------------------------------------
 // CServerCommandManager constructor.
 //-----------------------------------------------------------------------------
-CServerCommandManager::CServerCommandManager(ConCommand* pConCommand,
+CServerCommandManager::CServerCommandManager(ConCommandBase* pConCommand,
 		const char* szName, const char* szHelpText, int iFlags):
 	ConCommand(szName, (FnCommandCallback_t)NULL, szHelpText, iFlags),
 	m_pOldCommand(pConCommand)
@@ -163,8 +166,8 @@ CServerCommandManager::~CServerCommandManager()
 	// Get the ConCommand instance
 	ConCommand* pConCommand = g_pCVar->FindCommand(m_Name);
 
-	// Was the command overwritten as a ConVar or by another DLL?
-	if (pConCommand && pConCommand->GetDLLIdentifier() == CVarDLLIdentifier())
+	// Make sure we only unregister ourselves
+	if (pConCommand == this)
 	{
 		// Unregister the ConCommand
 		g_pCVar->UnregisterConCommand(pConCommand);
@@ -236,7 +239,12 @@ void CServerCommandManager::Dispatch( const CCommand& command )
 	// Was the command previously registered?
 	if(m_pOldCommand)
 	{
-		m_pOldCommand->Dispatch(command);
+		if (m_pOldCommand->IsCommand()) {
+			static_cast<ConCommand *>(m_pOldCommand)->Dispatch(command);
+		}
+		else {
+			static_cast<ConVar *>(m_pOldCommand)->SetValue(command.ArgS());
+		}
 	}
 
 	// Post hook callbacks

diff --git a/src/core/modules/commands/commands_server.h b/src/core/modules/commands/commands_server.h
@@ -50,11 +50,11 @@ class CServerCommandManager : public ConCommand
 protected:
 	void Dispatch( const CCommand& command);
 
-private:
-	CServerCommandManager(ConCommand* pConCommand, const char* szName, const char* szHelpString = 0, int iFlags = 0);
+public:
+	CServerCommandManager(ConCommandBase* pConCommand, const char* szName, const char* szHelpString = 0, int iFlags = 0);
 	std::map< HookType_t, CListenerManager* > m_vecCallables;
 	const char* m_Name;
-	ConCommand* m_pOldCommand;
+	ConCommandBase* m_pOldCommand;
 };
 
 #endif // _COMMANDS_SERVER_H
diff --git a/src/core/modules/commands/commands_server_wrap.cpp b/src/core/modules/commands/commands_server_wrap.cpp
@@ -27,9 +27,6 @@
 //-----------------------------------------------------------------------------
 // Includes.
 //-----------------------------------------------------------------------------
-// This is required for accessing m_nFlags without patching convar.h
-#define private public
-
 #include "boost/unordered_map.hpp"
 #include "utilities/wrap_macros.h"
 #include "commands_server.h"

diff --git a/src/core/modules/commands/commands_wrap.cpp b/src/core/modules/commands/commands_wrap.cpp
@@ -27,9 +27,6 @@
 //-----------------------------------------------------------------------------
 // Includes.
 //-----------------------------------------------------------------------------
-// This is required for accessing m_nFlags without patching convar.h
-#define private public
-
 #include "utilities/wrap_macros.h"
 #include "export_main.h"
 #include "modules/memory/memory_tools.h"
@@ -170,20 +167,20 @@ void export_concommandbase(scope _commands)
 		)
 
 		.def("add_flags",
-			&ConCommandBaseExt::AddFlags,
+			&AddConCommandFlags,
 			"Adds the given flags to the ConVar.",
 			args("flag")
 		)
 
 		.def("remove_flags",
-			&ConCommandBaseExt::RemoveFlags,
+			&RemoveConCommandFlags,
 			"Removes the given flags from the ConVar.",
 			args("flag")
 		)
 
 		.add_property("flags",
-			&ConCommandBaseExt::GetFlags,
-			&ConCommandBaseExt::SetFlags,
+			&GetConCommandFlags,
+			&SetConCommandFlags,
 			"Returns its flags."
 		)
 

diff --git a/src/core/modules/cvars/cvars.h b/src/core/modules/cvars/cvars.h
@@ -30,18 +30,37 @@
 //-----------------------------------------------------------------------------
 // Includes.
 //-----------------------------------------------------------------------------
-#include "convar.h"
+#include "icvar.h"
+#include "utilities/convar.h"
 #include "utilities/sp_util.h"
+#include "modules/commands/commands_server.h"
 
 
 //-----------------------------------------------------------------------------
-// Returns Source.Python's DLL identifier.
+// ICvar shared extension class.
 //-----------------------------------------------------------------------------
-inline CVarDLLIdentifier_t CVarDLLIdentifier()
+class ICVarSharedExt
 {
-	static CVarDLLIdentifier_t s_nDLLIdentifier = ConCommandBase().GetDLLIdentifier();
-	return s_nDLLIdentifier;
-}
+public:
+	static ConVar *FindVar(ICvar *pCVar, const char *szName)
+	{
+		ConCommandBase *pBase = pCVar->FindCommandBase(szName);
+		if (pBase)
+		{
+			if (pBase->IsCommand()) {
+				CServerCommandManager *pManager = dynamic_cast<CServerCommandManager *>(pBase);
+				if (pManager && pManager->m_pOldCommand && !pManager->m_pOldCommand->IsCommand()) {
+					return static_cast<ConVar *>(pManager->m_pOldCommand);
+				}
+			}
+			else {
+				return static_cast<ConVar *>(pBase);
+			}
+		}
+
+		return NULL;
+	};
+};
 
 
 //-----------------------------------------------------------------------------
@@ -73,18 +92,34 @@ class ConVarExt
 			PyErr_Clear();
 		}
 
-		ConVar *pConVar = g_pCVar->FindVar(name);
+		ConVar *pConVar = ICVarSharedExt::FindVar(g_pCVar, name);
 		if (!pConVar)
 		{
 			ConVar* pConVar = new ConVar(strdup(name), strdup(value), flags,
 				strdup(description), !min_value.is_none(), fMin, !max_value.is_none(), fMax);
 
-			return boost::shared_ptr<ConVar>(pConVar, &NeverDeleteDeleter<ConVar *>);
+			return boost::shared_ptr<ConVar>(pConVar, &Deleter);
 		}
 
 		return boost::shared_ptr<ConVar>(pConVar, &NeverDeleteDeleter<ConVar *>);
 	}
 
+	static void Deleter(ConVar *pConVar)
+	{
+		ConCommandBase *pBase = g_pCVar->FindCommandBase(pConVar->GetName());
+		if (pBase) {
+			CServerCommandManager *pManager = dynamic_cast<CServerCommandManager *>(pBase);
+			if (pManager && pManager->m_pOldCommand == pConVar) {
+				pManager->m_pOldCommand = NULL;
+			}
+			else if (pBase == pConVar) {
+				g_pCVar->UnregisterConCommand(pConVar);
+			}
+		}
+
+		delete pConVar;
+	}
+
 	static bool HasMin(ConVar* pConVar)
 	{
 		float fMin;
@@ -118,13 +153,13 @@ class ConVarExt
 
 	static void MakePublic(ConVar* pConVar)
 	{
-		pConVar->m_nFlags |= FCVAR_NOTIFY;
+		AddConCommandFlags(pConVar, FCVAR_NOTIFY);
 		g_pCVar->CallGlobalChangeCallbacks(pConVar, pConVar->GetString(), pConVar->GetFloat());
 	}
 
 	static void RemovePublic(ConVar* pConVar)
 	{
-		pConVar->m_nFlags &= ~FCVAR_NOTIFY;
+		RemoveConCommandFlags(pConVar, FCVAR_NOTIFY);
 		g_pCVar->CallGlobalChangeCallbacks(pConVar, pConVar->GetString(), pConVar->GetFloat());
 	}
 };