Merge pull request #192 from samisalreadytaken/scripthooks

Script hooks fixes and script hook manager

sp/src/game/client/c_baseentity.cpp

@@ -1353,7 +1353,7 @@ void C_BaseEntity::Term()
 	if ( m_hScriptInstance )
 	{
 #ifdef MAPBASE_VSCRIPT
-		if ( m_ScriptScope.IsInitialized() )
+		if ( m_ScriptScope.IsInitialized() && g_Hook_UpdateOnRemove.CanRunInScope( m_ScriptScope ) )
 		{
 			g_Hook_UpdateOnRemove.Call( m_ScriptScope, NULL, NULL );
 		}

sp/src/game/client/vscript_client.cpp

@@ -114,7 +114,7 @@ public:
 
 	void OnEntityCreated( CBaseEntity *pEntity )
 	{
-		if ( g_pScriptVM )
+		if ( g_pScriptVM && GetScriptHookManager().IsEventHooked( "OnEntityCreated" ) )
 		{
 			// entity
 			ScriptVariant_t args[] = { ScriptVariant_t( pEntity->GetScriptInstance() ) };
@@ -124,7 +124,7 @@ public:
 
 	void OnEntityDeleted( CBaseEntity *pEntity )
 	{
-		if ( g_pScriptVM )
+		if ( g_pScriptVM && GetScriptHookManager().IsEventHooked( "OnEntityDeleted" ) )
 		{
 			// entity
 			ScriptVariant_t args[] = { ScriptVariant_t( pEntity->GetScriptInstance() ) };
@@ -645,6 +645,11 @@ bool VScriptClientInit()
 #else
 				Log( "VSCRIPT: Started VScript virtual machine using script language '%s'\n", g_pScriptVM->GetLanguageName() );
 #endif
+
+#ifdef MAPBASE_VSCRIPT
+				GetScriptHookManager().OnInit();
+#endif
+
 				ScriptRegisterFunction( g_pScriptVM, GetMapName, "Get the name of the map.");
 				ScriptRegisterFunction( g_pScriptVM, Time, "Get the current server time" );
 				ScriptRegisterFunction( g_pScriptVM, DoUniqueString, SCRIPT_ALIAS( "UniqueString", "Generate a string guaranteed to be unique across the life of the script VM, with an optional root string." ) );
@@ -770,6 +775,8 @@ public:
 	{
 #ifdef MAPBASE_VSCRIPT
 		g_ScriptNetMsg->LevelShutdownPreVM();
+
+		GetScriptHookManager().OnShutdown();
 #endif
 		VScriptClientTerm();
 	}

sp/src/game/server/baseentity.cpp

@@ -2646,7 +2646,7 @@ void CBaseEntity::UpdateOnRemove( void )
 	if ( m_hScriptInstance )
 	{
 #ifdef MAPBASE_VSCRIPT
-		if (m_ScriptScope.IsInitialized())
+		if ( m_ScriptScope.IsInitialized() && g_Hook_UpdateOnRemove.CanRunInScope( m_ScriptScope ) )
 		{
 			g_Hook_UpdateOnRemove.Call( m_ScriptScope, NULL, NULL );
 		}

sp/src/game/server/filters.cpp

@@ -2157,7 +2157,7 @@ class CFilterScript : public CBaseFilter
 public:
 	bool PassesFilterImpl( CBaseEntity *pCaller, CBaseEntity *pEntity )
 	{
-		if (m_ScriptScope.IsInitialized())
+		if ( m_ScriptScope.IsInitialized() && g_Hook_PassesFilter.CanRunInScope( m_ScriptScope ) )
 		{
 			// caller, activator
 			ScriptVariant_t functionReturn;
@@ -2176,7 +2176,7 @@ public:
 
 	bool PassesDamageFilterImpl( CBaseEntity *pCaller, const CTakeDamageInfo &info )
 	{
-		if (m_ScriptScope.IsInitialized())
+		if ( m_ScriptScope.IsInitialized() && g_Hook_PassesDamageFilter.CanRunInScope( m_ScriptScope ) )
 		{
 			HSCRIPT pInfo = g_pScriptVM->RegisterInstance( const_cast<CTakeDamageInfo*>(&info) );
 
@@ -2201,7 +2201,7 @@ public:
 
 	bool PassesFinalDamageFilter( CBaseEntity *pCaller, const CTakeDamageInfo &info )
 	{
-		if (m_ScriptScope.IsInitialized())
+		if ( m_ScriptScope.IsInitialized() && g_Hook_PassesFinalDamageFilter.CanRunInScope( m_ScriptScope ) )
 		{
 			HSCRIPT pInfo = g_pScriptVM->RegisterInstance( const_cast<CTakeDamageInfo*>(&info) );
 
@@ -2225,7 +2225,7 @@ public:
 
 	bool BloodAllowed( CBaseEntity *pCaller, const CTakeDamageInfo &info )
 	{
-		if (m_ScriptScope.IsInitialized())
+		if ( m_ScriptScope.IsInitialized() && g_Hook_BloodAllowed.CanRunInScope( m_ScriptScope ) )
 		{
 			HSCRIPT pInfo = g_pScriptVM->RegisterInstance( const_cast<CTakeDamageInfo*>(&info) );
 
@@ -2249,7 +2249,7 @@ public:
 
 	bool DamageMod( CBaseEntity *pCaller, CTakeDamageInfo &info )
 	{
-		if (m_ScriptScope.IsInitialized())
+		if ( m_ScriptScope.IsInitialized() && g_Hook_DamageMod.CanRunInScope( m_ScriptScope ) )
 		{
 			HSCRIPT pInfo = g_pScriptVM->RegisterInstance( &info );
 

sp/src/game/server/vscript_server.cpp

@@ -154,7 +154,7 @@ public:
 
 	void OnEntityCreated( CBaseEntity *pEntity )
 	{
-		if ( g_pScriptVM )
+		if ( g_pScriptVM && GetScriptHookManager().IsEventHooked( "OnEntityCreated" ) )
 		{
 			// entity
 			ScriptVariant_t args[] = { ScriptVariant_t( pEntity->GetScriptInstance() ) };
@@ -164,7 +164,7 @@ public:
 
 	void OnEntitySpawned( CBaseEntity *pEntity )
 	{
-		if ( g_pScriptVM )
+		if ( g_pScriptVM && GetScriptHookManager().IsEventHooked( "OnEntitySpawned" ) )
 		{
 			// entity
 			ScriptVariant_t args[] = { ScriptVariant_t( pEntity->GetScriptInstance() ) };
@@ -174,7 +174,7 @@ public:
 
 	void OnEntityDeleted( CBaseEntity *pEntity )
 	{
-		if ( g_pScriptVM )
+		if ( g_pScriptVM && GetScriptHookManager().IsEventHooked( "OnEntityDeleted" ) )
 		{
 			// entity
 			ScriptVariant_t args[] = { ScriptVariant_t( pEntity->GetScriptInstance() ) };
@@ -600,6 +600,10 @@ bool VScriptServerInit()
 				Log( "VSCRIPT: Started VScript virtual machine using script language '%s'\n", g_pScriptVM->GetLanguageName() );
 #endif
 
+#ifdef MAPBASE_VSCRIPT
+				GetScriptHookManager().OnInit();
+#endif
+
 #ifdef MAPBASE_VSCRIPT
 				// MULTIPLAYER
 				// ScriptRegisterFunctionNamed( g_pScriptVM, UTIL_PlayerByIndex, "GetPlayerByIndex", "PlayerInstanceFromIndex" );
@@ -836,6 +840,8 @@ public:
 	{
 #ifdef MAPBASE_VSCRIPT
 		g_ScriptNetMsg->LevelShutdownPreVM();
+
+		GetScriptHookManager().OnShutdown();
 #endif
 		VScriptServerTerm();
 	}

sp/src/game/shared/baseentity_shared.cpp

@@ -1613,7 +1613,7 @@ typedef CTraceFilterSimpleList CBulletsTraceFilter;
 void CBaseEntity::FireBullets( const FireBulletsInfo_t &info )
 {
 #if defined(MAPBASE_VSCRIPT) && defined(GAME_DLL)
-	if (m_ScriptScope.IsInitialized())
+	if ( m_ScriptScope.IsInitialized() && g_Hook_FireBullets.CanRunInScope( m_ScriptScope ) )
 	{
 		HSCRIPT hInfo = g_pScriptVM->RegisterInstance( const_cast<FireBulletsInfo_t*>(&info) );
 

sp/src/game/shared/mapbase/vscript_singletons.cpp

@@ -872,6 +872,7 @@ public: // IGameSystem
 	{
 		if ( g_pScriptVM )
 		{
+			if ( GetScriptHookManager().IsEventHooked( "OnSave" ) )
 				g_Hook_OnSave.Call( NULL, NULL, NULL );
 
 			// Legacy hook
@@ -893,6 +894,7 @@ public: // IGameSystem
 	{
 		if ( g_pScriptVM )
 		{
+			if ( GetScriptHookManager().IsEventHooked( "OnRestore" ) )
 				g_Hook_OnRestore.Call( NULL, NULL, NULL );
 
 			// Legacy hook

sp/src/game/shared/mapbase/weapon_custom_scripted.cpp

@@ -177,8 +177,13 @@ CWeaponCustomScripted::CWeaponCustomScripted()
 
 bool CWeaponCustomScripted::RunWeaponHook( ScriptHook_t &hook, HSCRIPT &cached, ScriptVariant_t *retVal, ScriptVariant_t *pArgs )
 {
-	if (!hook.CheckFuncValid(cached))
+	if ( !cached )
-		cached = hook.CanRunInScope(m_ScriptScope);
+	{
+		if ( hook.CanRunInScope( m_ScriptScope ) )
+		{
+			cached = hook.m_hFunc;
+		}
+	}
 
 	if (cached)
 	{

sp/src/game/shared/vscript_shared.cpp

@@ -451,6 +451,10 @@ public:
 		}
 		m_InstanceMap.Purge();
 
+#ifdef MAPBASE_VSCRIPT
+		GetScriptHookManager().OnRestore();
+#endif
+
 #if defined(MAPBASE_VSCRIPT) && defined(CLIENT_DLL)
 		VScriptSaveRestoreUtil_OnVMRestore();
 #endif

sp/src/public/vscript/ivscript.h

@@ -98,6 +98,9 @@
 #include <type_traits>
 #include <utility>
 
+#include "utlmap.h"
+#include "utlvector.h"
+
 #include "platform.h"
 #include "datamap.h"
 #include "appframework/IAppSystem.h"
@@ -137,6 +140,8 @@ class KeyValues;
 // This has been moved up a bit for IScriptManager
 DECLARE_POINTER_HANDLE( HSCRIPT );
 #define INVALID_HSCRIPT ((HSCRIPT)-1)
+
+typedef unsigned int HScriptRaw;
 #endif
 
 enum ScriptLanguage_t
@@ -885,9 +890,9 @@ public:
 	//--------------------------------------------------------
 	// Hooks
 	//--------------------------------------------------------
-	virtual bool ScopeIsHooked( HSCRIPT hScope, const char *pszEventName ) = 0;
+	// Persistent unique identifier for an HSCRIPT variable
-	virtual HSCRIPT LookupHookFunction( const char *pszEventName, HSCRIPT hScope, bool &bLegacy ) = 0;
+	virtual HScriptRaw HScriptToRaw( HSCRIPT val ) = 0;
-	virtual ScriptStatus_t ExecuteHookFunction( const char *pszEventName, HSCRIPT hFunction, ScriptVariant_t *pArgs, int nArgs, ScriptVariant_t *pReturn, HSCRIPT hScope, bool bWait ) = 0;
+	virtual ScriptStatus_t ExecuteHookFunction( const char *pszEventName, ScriptVariant_t *pArgs, int nArgs, ScriptVariant_t *pReturn, HSCRIPT hScope, bool bWait ) = 0;
 #endif
 
 	//--------------------------------------------------------
@@ -1587,23 +1592,297 @@ typedef CScriptScopeT<> CScriptScope;
 #define VScriptAddEnumToRoot( enumVal )					g_pScriptVM->SetValue( #enumVal, (int)enumVal )
 
 #ifdef MAPBASE_VSCRIPT
+
+//
+// Map pointer iteration
+//
+#define FOR_EACH_MAP_PTR( mapName, iteratorName ) \
+	for ( int iteratorName = (mapName)->FirstInorder(); (mapName)->IsUtlMap && iteratorName != (mapName)->InvalidIndex(); iteratorName = (mapName)->NextInorder( iteratorName ) )
+
+#define FOR_EACH_MAP_PTR_FAST( mapName, iteratorName ) \
+	for ( int iteratorName = 0; (mapName)->IsUtlMap && iteratorName < (mapName)->MaxElement(); ++iteratorName ) if ( !(mapName)->IsValidIndex( iteratorName ) ) continue; else
+
+#define FOR_EACH_VEC_PTR( vecName, iteratorName ) \
+	for ( int iteratorName = 0; iteratorName < (vecName)->Count(); iteratorName++ )
+
+//-----------------------------------------------------------------------------
+//
+// Keeps track of which events and scopes are hooked without polling this from the script VM on each request.
+// Local cache is updated each time there is a change to script hooks: on Add, on Remove, on game restore
+//
+//-----------------------------------------------------------------------------
+class CScriptHookManager
+{
+private:
+	typedef CUtlVector< char* > contextmap_t;
+	typedef CUtlMap< HScriptRaw, contextmap_t* > scopemap_t;
+	typedef CUtlMap< char*, scopemap_t* > hookmap_t;
+
+	HSCRIPT m_hfnHookFunc;
+
+	// { [string event], { [HSCRIPT scope], { [string context], [HSCRIPT callback] } } }
+	hookmap_t m_HookList;
+
+public:
+
+	CScriptHookManager() : m_HookList( DefLessFunc(char*) ), m_hfnHookFunc(NULL)
+	{
+	}
+
+	HSCRIPT GetHookFunction()
+	{
+		return m_hfnHookFunc;
+	}
+
+	// For global hooks
+	bool IsEventHooked( const char *szEvent )
+	{
+		return m_HookList.Find( const_cast< char* >( szEvent ) ) != m_HookList.InvalidIndex();
+	}
+
+	bool IsEventHookedInScope( const char *szEvent, HSCRIPT hScope )
+	{
+		extern IScriptVM *g_pScriptVM;
+
+		Assert( hScope );
+
+		int eventIdx = m_HookList.Find( const_cast< char* >( szEvent ) );
+		if ( eventIdx == m_HookList.InvalidIndex() )
+			return false;
+
+		scopemap_t *scopeMap = m_HookList.Element( eventIdx );
+		return scopeMap->Find( g_pScriptVM->HScriptToRaw( hScope ) ) != scopeMap->InvalidIndex();
+	}
+
+	static void __UpdateScriptHooks( HSCRIPT hooksList )
+	{
+		extern CScriptHookManager &GetScriptHookManager();
+		GetScriptHookManager().Update( hooksList );
+	}
+
+	//
+	// On VM init, registers script func and caches the hook func.
+	//
+	void OnInit()
+	{
+		extern IScriptVM *g_pScriptVM;
+
+		ScriptRegisterFunctionNamed( g_pScriptVM, __UpdateScriptHooks, "__UpdateScriptHooks", SCRIPT_HIDE );
+
+		ScriptVariant_t hHooks;
+		g_pScriptVM->GetValue( "Hooks", &hHooks );
+
+		Assert( hHooks.m_type == FIELD_HSCRIPT );
+
+		if ( hHooks.m_type == FIELD_HSCRIPT )
+		{
+			m_hfnHookFunc = g_pScriptVM->LookupFunction( "Call", hHooks );
+		}
+
+		Clear();
+	}
+
+	//
+	// On VM shutdown, clear the cache.
+	// Not exactly necessary, as the cache will be cleared on VM init next time.
+	//
+	void OnShutdown()
+	{
+		extern IScriptVM *g_pScriptVM;
+
+		if ( m_hfnHookFunc )
+			g_pScriptVM->ReleaseFunction( m_hfnHookFunc );
+
+		m_hfnHookFunc = NULL;
+
+		Clear();
+	}
+
+	//
+	// On VM restore, update local cache.
+	//
+	void OnRestore()
+	{
+		extern IScriptVM *g_pScriptVM;
+
+		ScriptVariant_t hHooks;
+		g_pScriptVM->GetValue( "Hooks", &hHooks );
+
+		if ( hHooks.m_type == FIELD_HSCRIPT )
+		{
+			// Existing m_hfnHookFunc is invalid
+			m_hfnHookFunc = g_pScriptVM->LookupFunction( "Call", hHooks );
+
+			HSCRIPT func = g_pScriptVM->LookupFunction( "__UpdateHooks", hHooks );
+			g_pScriptVM->Call( func );
+			g_pScriptVM->ReleaseFunction( func );
+			g_pScriptVM->ReleaseValue( hHooks );
+		}
+	}
+
+	//
+	// Clear local cache.
+	//
+	void Clear()
+	{
+		if ( m_HookList.Count() )
+		{
+			FOR_EACH_MAP_FAST( m_HookList, i )
+			{
+				scopemap_t *scopeMap = m_HookList.Element(i);
+
+				FOR_EACH_MAP_PTR_FAST( scopeMap, j )
+				{
+					contextmap_t *contextMap = scopeMap->Element(j);
+					contextMap->PurgeAndDeleteElements();
+				}
+
+				char *szEvent = m_HookList.Key(i);
+				free( szEvent );
+
+				scopeMap->PurgeAndDeleteElements();
+			}
+
+			m_HookList.PurgeAndDeleteElements();
+		}
+	}
+
+	//
+	// Called from script, update local cache.
+	//
+	void Update( HSCRIPT hooksList )
+	{
+		extern IScriptVM *g_pScriptVM;
+
+		// Rebuild from scratch
+		Clear();
+		{
+			ScriptVariant_t varEvent, varScopeMap;
+			int it = -1;
+			while ( ( it = g_pScriptVM->GetKeyValue( hooksList, it, &varEvent, &varScopeMap ) ) != -1 )
+			{
+				// types are checked in script
+				Assert( varEvent.m_type == FIELD_CSTRING );
+				Assert( varScopeMap.m_type == FIELD_HSCRIPT );
+
+				scopemap_t *scopeMap;
+
+				int eventIdx = m_HookList.Find( const_cast< char* >( varEvent.m_pszString ) );
+				if ( eventIdx != m_HookList.InvalidIndex() )
+				{
+					scopeMap = m_HookList.Element( eventIdx );
+				}
+				else
+				{
+					scopeMap = new scopemap_t( DefLessFunc(HScriptRaw) );
+					m_HookList.Insert( strdup( varEvent.m_pszString ), scopeMap );
+				}
+
+				ScriptVariant_t varScope, varContextMap;
+				int it2 = -1;
+				while ( ( it2 = g_pScriptVM->GetKeyValue( varScopeMap, it2, &varScope, &varContextMap ) ) != -1 )
+				{
+					Assert( varScope.m_type == FIELD_HSCRIPT );
+					Assert( varContextMap.m_type == FIELD_HSCRIPT);
+
+					contextmap_t *contextMap;
+
+					int scopeIdx = scopeMap->Find( g_pScriptVM->HScriptToRaw( varScope.m_hScript ) );
+					if ( scopeIdx != scopeMap->InvalidIndex() )
+					{
+						contextMap = scopeMap->Element( scopeIdx );
+					}
+					else
+					{
+						contextMap = new contextmap_t();
+						scopeMap->Insert( g_pScriptVM->HScriptToRaw( varScope.m_hScript ), contextMap );
+					}
+
+					ScriptVariant_t varContext, varCallback;
+					int it3 = -1;
+					while ( ( it3 = g_pScriptVM->GetKeyValue( varContextMap, it3, &varContext, &varCallback ) ) != -1 )
+					{
+						Assert( varContext.m_type == FIELD_CSTRING );
+						Assert( varCallback.m_type == FIELD_HSCRIPT );
+
+						bool skip = false;
+
+						FOR_EACH_VEC_PTR( contextMap, k )
+						{
+							char *szContext = contextMap->Element(k);
+							if ( V_strcmp( szContext, varContext.m_pszString ) == 0 )
+							{
+								skip = true;
+								break;
+							}
+						}
+
+						if ( !skip )
+							contextMap->AddToTail( strdup( varContext.m_pszString ) );
+
+						g_pScriptVM->ReleaseValue( varContext );
+						g_pScriptVM->ReleaseValue( varCallback );
+					}
+
+					g_pScriptVM->ReleaseValue( varScope );
+					g_pScriptVM->ReleaseValue( varContextMap );
+				}
+
+				g_pScriptVM->ReleaseValue( varEvent );
+				g_pScriptVM->ReleaseValue( varScopeMap );
+			}
+		}
+	}
+#ifdef _DEBUG
+	void Dump()
+	{
+		extern IScriptVM *g_pScriptVM;
+
+		FOR_EACH_MAP( m_HookList, i )
+		{
+			scopemap_t *scopeMap = m_HookList.Element(i);
+			char *szEvent = m_HookList.Key(i);
+
+			Msg( "%s [%x]\n", szEvent, (void*)scopeMap );
+			Msg( "{\n" );
+
+			FOR_EACH_MAP_PTR( scopeMap, j )
+			{
+				HScriptRaw hScope = scopeMap->Key(j);
+				contextmap_t *contextMap = scopeMap->Element(j);
+
+				Msg( "\t(0x%X) [%x]\n", hScope, (void*)contextMap );
+				Msg( "\t{\n" );
+
+				FOR_EACH_VEC_PTR( contextMap, k )
+				{
+					char *szContext = contextMap->Element(k);
+
+					Msg( "\t\t%-.50s\n", szContext );
+				}
+
+				Msg( "\t}\n" );
+			}
+
+			Msg( "}\n" );
+		}
+	}
+#endif
+};
+
+inline CScriptHookManager &GetScriptHookManager()
+{
+	static CScriptHookManager g_ScriptHookManager;
+	return g_ScriptHookManager;
+}
+
+
 //-----------------------------------------------------------------------------
 // Function bindings allow script functions to run C++ functions.
 // Hooks allow C++ functions to run script functions.
 // 
 // This was previously done with raw function lookups, but Mapbase adds more and
 // it's hard to keep track of them without proper standards or documentation.
-// 
-// At the moment, this simply plugs hook documentation into VScript and maintains
-// the same function lookup method on the inside, but it's intended to be open for
-// more complex hook mechanisms with proper parameters in the future.
-// 
-// For example:
-// 
-//	if (m_hFunc)
-//	{
-//		g_pScriptVM->ExecuteFunction( m_Func, pArgs, m_desc.m_Parameters.Count(), pReturn, m_ScriptScope, true );
-//	}
 //-----------------------------------------------------------------------------
 struct ScriptHook_t
 {
@@ -1620,51 +1899,65 @@ struct ScriptHook_t
 
 	// -----------------------------------------------------------------
 
-	// Cached for when CanRunInScope() is called before Call()
+	// Only valid between CanRunInScope() and Call()
 	HSCRIPT m_hFunc;
-	bool m_bLegacy;
+
+	ScriptHook_t() :
+		m_hFunc(NULL)
+	{
+	}
+
+#ifdef _DEBUG
+	//
+	// An uninitialised script scope will pass as null scope which is considered a valid hook scope (global hook)
+	// This should catch CanRunInScope() calls without CScriptScope::IsInitalised() checks first.
+	//
+	bool CanRunInScope( CScriptScope &hScope )
+	{
+		Assert( hScope.IsInitialized() );
+		return hScope.IsInitialized() && CanRunInScope( (HSCRIPT)hScope );
+	}
+#endif
 
 	// Checks if there's a function of this name which would run in this scope
-	HSCRIPT CanRunInScope( HSCRIPT hScope )
+	bool CanRunInScope( HSCRIPT hScope )
 	{
-		extern IScriptVM *g_pScriptVM;
+		// For now, assume null scope (which is used for global hooks) is always hooked
-		m_hFunc = g_pScriptVM->LookupHookFunction( m_desc.m_pszScriptName, hScope, m_bLegacy );
+		if ( !hScope || GetScriptHookManager().IsEventHookedInScope( m_desc.m_pszScriptName, hScope ) )
-		return m_hFunc;
-	}
-
-	// Checks if an existing func can be used
-	bool CheckFuncValid( HSCRIPT hFunc )
 		{
-		// TODO: Better crtieria for this?
+			m_hFunc = NULL;
-		if (hFunc)
-		{
-			m_hFunc = hFunc;
 			return true;
 		}
-		return false;
+
+		extern IScriptVM *g_pScriptVM;
+
+		// Legacy support if the new system is not being used
+		m_hFunc = g_pScriptVM->LookupFunction( m_desc.m_pszScriptName, hScope );
+
+		return !!m_hFunc;
 	}
 
 	// Call the function
+	// NOTE: `bRelease` only exists for weapon_custom_scripted legacy script func caching
 	bool Call( HSCRIPT hScope, ScriptVariant_t *pReturn, ScriptVariant_t *pArgs, bool bRelease = true )
 	{
 		extern IScriptVM *g_pScriptVM;
 
-		// Make sure we have a function in this scope
+		// Call() should not be called without CanRunInScope() check first, it caches m_hFunc for legacy support
-		if (!m_hFunc && !CanRunInScope(hScope))
+		Assert( CanRunInScope( hScope ) );
-			return false;
+
 		// Legacy
-		else if (m_bLegacy)
+		if ( m_hFunc )
 		{
 			for (int i = 0; i < m_desc.m_Parameters.Count(); i++)
 			{
 				g_pScriptVM->SetValue( m_pszParameterNames[i], pArgs[i] );
 			}
 
-			g_pScriptVM->ExecuteFunction( m_hFunc, NULL, 0, pReturn, hScope, true );
+			ScriptStatus_t status = g_pScriptVM->ExecuteFunction( m_hFunc, NULL, 0, pReturn, hScope, true );
 
 			if ( bRelease )
 				g_pScriptVM->ReleaseFunction( m_hFunc );
-
 			m_hFunc = NULL;
 
 			for (int i = 0; i < m_desc.m_Parameters.Count(); i++)
@@ -1672,19 +1965,14 @@ struct ScriptHook_t
 				g_pScriptVM->ClearValue( m_pszParameterNames[i] );
 			}
 
-			return true;
+			return status == SCRIPT_DONE;
 		}
 		// New Hook System
 		else
 		{
-			g_pScriptVM->ExecuteHookFunction( m_desc.m_pszScriptName, m_hFunc, pArgs, m_desc.m_Parameters.Count(), pReturn, hScope, true );
+			ScriptStatus_t status = g_pScriptVM->ExecuteHookFunction( m_desc.m_pszScriptName, pArgs, m_desc.m_Parameters.Count(), pReturn, hScope, true );
-			if (bRelease)
+			return status == SCRIPT_DONE;
-				g_pScriptVM->ReleaseFunction( m_hFunc );
-			m_hFunc = NULL;
-			return true;
 		}
-
-		return false;
 	}
 };
 #endif

sp/src/vscript/vscript_squirrel.cpp

@@ -188,9 +188,8 @@ public:
 	//--------------------------------------------------------
 	// Hooks
 	//--------------------------------------------------------
-	virtual bool ScopeIsHooked( HSCRIPT hScope, const char *pszEventName ) override;
+	virtual HScriptRaw HScriptToRaw( HSCRIPT val ) override;
-	virtual HSCRIPT LookupHookFunction( const char *pszEventName, HSCRIPT hScope, bool &bLegacy ) override;
+	virtual ScriptStatus_t ExecuteHookFunction( const char *pszEventName, ScriptVariant_t *pArgs, int nArgs, ScriptVariant_t *pReturn, HSCRIPT hScope, bool bWait ) override;
-	virtual ScriptStatus_t ExecuteHookFunction( const char *pszEventName, HSCRIPT hFunction, ScriptVariant_t *pArgs, int nArgs, ScriptVariant_t *pReturn, HSCRIPT hScope, bool bWait ) override;
 
 	//--------------------------------------------------------
 	// External functions
@@ -2348,94 +2347,37 @@ ScriptStatus_t SquirrelVM::ExecuteFunction(HSCRIPT hFunction, ScriptVariant_t* p
 	return SCRIPT_DONE;
 }
 
-bool SquirrelVM::ScopeIsHooked( HSCRIPT hScope, const char *pszEventName )
+HScriptRaw SquirrelVM::HScriptToRaw( HSCRIPT val )
 {
-	// For now, assume null scope (which is used for global hooks) is always hooked
+	Assert( val );
-	if (!hScope)
+	Assert( val != INVALID_HSCRIPT );
-		return true;
 
+	HSQOBJECT *obj = (HSQOBJECT*)val;
+#if 0
+	if ( sq_isweakref(*obj) )
+		return obj->_unVal.pWeakRef->_obj._unVal.raw;
+#endif
+	return obj->_unVal.raw;
+}
+
+ScriptStatus_t SquirrelVM::ExecuteHookFunction(const char *pszEventName, ScriptVariant_t* pArgs, int nArgs, ScriptVariant_t* pReturn, HSCRIPT hScope, bool bWait)
+{
 	SquirrelSafeCheck safeCheck(vm_);
 
-	Assert(hScope != INVALID_HSCRIPT);
+	HSQOBJECT* pFunc = (HSQOBJECT*)GetScriptHookManager().GetHookFunction();
-
-	sq_pushroottable(vm_);
-	sq_pushstring(vm_, "Hooks", -1);
-	sq_get(vm_, -2);
-	sq_pushstring(vm_, "ScopeHookedToEvent", -1);
-	sq_get(vm_, -2);
-	sq_push(vm_, -2);
-	sq_pushobject(vm_, *((HSQOBJECT*)hScope));
-	sq_pushstring(vm_, pszEventName, -1);
-	sq_call(vm_, 3, SQTrue, SQTrue);
-
-	SQBool val;
-	if (SQ_FAILED(sq_getbool(vm_, -1, &val)))
-	{
-		sq_pop(vm_, 3);
-		return false;
-	}
-
-	sq_pop(vm_, 4);
-	return val ? true : false;
-}
-
-HSCRIPT SquirrelVM::LookupHookFunction(const char *pszEventName, HSCRIPT hScope, bool &bLegacy)
-{
-	HSCRIPT hFunc = hScope ? LookupFunction( pszEventName, hScope ) : nullptr;
-	if (hFunc)
-	{
-		bLegacy = true;
-		return hFunc;
-	}
-	else
-	{
-		bLegacy = false;
-	}
-
-	if (!ScopeIsHooked(hScope, pszEventName))
-		return nullptr;
-
-	SquirrelSafeCheck safeCheck(vm_);
-
-	sq_pushroottable(vm_);
-	sq_pushstring(vm_, "Hooks", -1);
-	sq_get(vm_, -2);
-	sq_pushstring(vm_, "Call", -1);
-	sq_get(vm_, -2);
-
-	HSQOBJECT obj;
-	sq_resetobject(&obj);
-	sq_getstackobj(vm_, -1, &obj);
-	sq_addref(vm_, &obj);
-	sq_pop(vm_, 3);
-
-	HSQOBJECT* pObj = new HSQOBJECT;
-	*pObj = obj;
-	return (HSCRIPT)pObj;
-}
-
-ScriptStatus_t SquirrelVM::ExecuteHookFunction(const char *pszEventName, HSCRIPT hFunction, ScriptVariant_t* pArgs, int nArgs, ScriptVariant_t* pReturn, HSCRIPT hScope, bool bWait)
-{
-	SquirrelSafeCheck safeCheck(vm_);
-	if (!hFunction)
-		return SCRIPT_ERROR;
-
-	if (hFunction == INVALID_HSCRIPT)
-		return SCRIPT_ERROR;
-
-	HSQOBJECT* pFunc = (HSQOBJECT*)hFunction;
 	sq_pushobject(vm_, *pFunc);
 
-	// TODO: Run in hook scope
+	// The call environment of the Hooks::Call function does not matter
+	// as the function does not access any member variables.
 	sq_pushroottable(vm_);
 
+	sq_pushstring(vm_, pszEventName, -1);
+
 	if (hScope)
 		sq_pushobject(vm_, *((HSQOBJECT*)hScope));
 	else
 		sq_pushnull(vm_); // global hook
 
-	sq_pushstring(vm_, pszEventName, -1);
-
 	for (int i = 0; i < nArgs; ++i)
 	{
 		PushVariant(vm_, pArgs[i]);

sp/src/vscript/vscript_squirrel.nut

@@ -152,114 +152,148 @@ Hooks <-
 	// table, string, closure, string
 	function Add( scope, event, callback, context )
 	{
-		if ( typeof callback != "function" )
+		switch ( typeof scope )
-			throw "invalid callback param"
+		{
-
+			case "table":
-		if ( !(scope in s_List) )
+			case "instance":
-			s_List[scope] <- {}
+			case "class":
-
+				break;
-		local t = s_List[scope]
+			default:
-
+				throw "invalid scope param";
-		if ( !(event in t) )
-			t[event] <- {}
-
-		t[event][context] <- callback
 		}
 
-	function Remove( context, event = null )
+		if ( typeof event != "string" )
+			throw "invalid event param";
+
+		if ( typeof callback != "function" )
+			throw "invalid callback param";
+
+		if ( typeof context != "string" )
+			throw "invalid context param";
+
+		if ( !(event in s_List) )
+			s_List[event] <- {};
+
+		local t = s_List[event];
+
+		if ( !(scope in t) )
+			t[scope] <- {};
+
+		t[scope][context] <- callback;
+
+		return __UpdateHooks();
+	}
+
+	function Remove( event, context )
 	{
+		local rem;
+
 		if ( event )
 		{
-			foreach( k,scope in s_List )
+			if ( event in s_List )
 			{
-				if ( event in scope )
+				foreach ( scope, ctx in s_List[event] )
 				{
-					local t = scope[event]
+					if ( context in ctx )
-					if ( context in t )
 					{
-						delete t[context]
+						delete ctx[context];
 					}
 
-					// cleanup?
+					if ( !ctx.len() )
-					if ( !t.len() )
+					{
-						delete scope[event]
+						if ( !rem )
+							rem = [];
+						rem.append( event );
+						rem.append( scope );
+					}
 				}
-
-				// cleanup?
-				if ( !scope.len() )
-					delete s_List[k]
 			}
 		}
 		else
 		{
-			foreach( k,scope in s_List )
+			foreach ( ev, t in s_List )
 			{
-				foreach( kk,ev in scope )
+				foreach ( scope, ctx in t )
 				{
-					if ( context in ev )
+					if ( context in ctx )
 					{
-						delete ev[context]
+						delete ctx[context];
 					}
 
-					// cleanup?
+					if ( !ctx.len() )
-					if ( !ev.len() )
+					{
-						delete scope[kk]
+						if ( !rem )
+							rem = [];
+						rem.append( ev );
+						rem.append( scope );
 					}
-
-				// cleanup?
-				if ( !scope.len() )
-					delete s_List[k]
 				}
 			}
 		}
 
-	function Call( scope, event, ... )
+		if ( rem )
 		{
-		local firstReturn
+			local c = rem.len() - 1;
+			for ( local i = 0; i < c; i += 2 )
+			{
+				local ev = rem[i];
+				local scope = rem[i+1];
 
-		// global hook; call all scopes
+				if ( !s_List[ev][scope].len() )
-		if ( !scope )
+					delete s_List[ev][scope];
-		{
-			vargv.insert( 0, null )
-			foreach( sc,t in s_List )
-			{
-				if ( event in t )
-				{
-					vargv[0] = sc
-					foreach( context, callback in t[event] )
-					{
-						//printf( "(%.4f) Calling hook '%s' of context '%s' in static iteration\n", Time(), event, context )
 
-						local curReturn = callback.acall(vargv)
+				if ( !s_List[ev].len() )
+					delete s_List[ev];
+			}
+		}
+
+		return __UpdateHooks();
+	}
+
+	function Call( event, scope, ... )
+	{
+		local firstReturn;
+
+		if ( event in s_List )
+		{
+			vargv.insert( 0, scope );
+
+			local t = s_List[event];
+			if ( scope in t )
+			{
+				foreach ( fn in t[scope] )
+				{
+					//printf( "(%.4f) Calling hook %s:%s\n", Time(), event, context );
+
+					local r = fn.acall( vargv );
 					if ( firstReturn == null )
-							firstReturn = curReturn
+						firstReturn = r;
 				}
 			}
-			}
+			else if ( !scope ) // global hook
-		}
-		else if ( scope in s_List )
 			{
-			local t = s_List[scope]
+				foreach ( sc, ctx in t )
-			if ( event in t )
 				{
-				vargv.insert( 0, scope )
+					vargv[0] = sc;
-				foreach( context, callback in t[event] )
-				{
-					//printf( "(%.4f) Calling hook '%s' of context '%s'\n", Time(), event, context )
 
-					local curReturn = callback.acall(vargv)
+					foreach ( context, fn in ctx )
+					{
+						//printf( "(%.4f) Calling hook (g) %s:%s\n", Time(), event, context );
+
+						local r = fn.acall( vargv );
 						if ( firstReturn == null )
-						firstReturn = curReturn
+							firstReturn = r;
+					}
 				}
 			}
 		}
 
-		return firstReturn
+		return firstReturn;
 	}
 
-	function ScopeHookedToEvent( scope, event )
+	function __UpdateHooks()
 	{
-		return ( scope in s_List ) && ( event in s_List[scope] )
+		return __UpdateScriptHooks( s_List );
 	}
 }