From 22f0b2c3ccdbb322f1e3fd41784f6fbe3b77965b Mon Sep 17 00:00:00 2001 From: samisalreadytaken <46823719+samisalreadytaken@users.noreply.github.com> Date: Mon, 18 Jul 2022 22:37:05 +0300 Subject: [PATCH] Refactor script hook system --- sp/src/game/client/c_baseentity.cpp | 2 +- sp/src/game/server/baseentity.cpp | 2 +- sp/src/game/server/filters.cpp | 10 +- sp/src/game/shared/baseentity_shared.cpp | 2 +- .../shared/mapbase/weapon_custom_scripted.cpp | 9 +- sp/src/public/vscript/ivscript.h | 89 ++++----- sp/src/vscript/vscript_squirrel.cpp | 92 ++++------ sp/src/vscript/vscript_squirrel.nut | 170 ++++++++++-------- 8 files changed, 201 insertions(+), 175 deletions(-) diff --git a/sp/src/game/client/c_baseentity.cpp b/sp/src/game/client/c_baseentity.cpp index b7ded40f..43471e9d 100644 --- a/sp/src/game/client/c_baseentity.cpp +++ b/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 ); } diff --git a/sp/src/game/server/baseentity.cpp b/sp/src/game/server/baseentity.cpp index 147dfc39..67c3b17e 100644 --- a/sp/src/game/server/baseentity.cpp +++ b/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 ); } diff --git a/sp/src/game/server/filters.cpp b/sp/src/game/server/filters.cpp index 84021a95..da5dd61f 100644 --- a/sp/src/game/server/filters.cpp +++ b/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(&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(&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(&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 ); diff --git a/sp/src/game/shared/baseentity_shared.cpp b/sp/src/game/shared/baseentity_shared.cpp index af49c966..3393afa8 100644 --- a/sp/src/game/shared/baseentity_shared.cpp +++ b/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(&info) ); diff --git a/sp/src/game/shared/mapbase/weapon_custom_scripted.cpp b/sp/src/game/shared/mapbase/weapon_custom_scripted.cpp index d9155f32..4d5c744d 100644 --- a/sp/src/game/shared/mapbase/weapon_custom_scripted.cpp +++ b/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)) - cached = hook.CanRunInScope(m_ScriptScope); + if ( !cached ) + { + if ( hook.CanRunInScope( m_ScriptScope ) ) + { + cached = hook.m_hFunc; + } + } if (cached) { diff --git a/sp/src/public/vscript/ivscript.h b/sp/src/public/vscript/ivscript.h index 001e1c24..fc16a143 100644 --- a/sp/src/public/vscript/ivscript.h +++ b/sp/src/public/vscript/ivscript.h @@ -885,9 +885,8 @@ public: //-------------------------------------------------------- // Hooks //-------------------------------------------------------- - virtual bool ScopeIsHooked( HSCRIPT hScope, const char *pszEventName ) = 0; - virtual HSCRIPT LookupHookFunction( const char *pszEventName, HSCRIPT hScope, bool &bLegacy ) = 0; - virtual ScriptStatus_t ExecuteHookFunction( const char *pszEventName, HSCRIPT hFunction, ScriptVariant_t *pArgs, int nArgs, ScriptVariant_t *pReturn, HSCRIPT hScope, bool bWait ) = 0; + virtual HSCRIPT LookupHookFunction( const char *pszEventName, HSCRIPT hScope ) = 0; + virtual ScriptStatus_t ExecuteHookFunction( HSCRIPT hFunction, const char *pszEventName, ScriptVariant_t *pArgs, int nArgs, ScriptVariant_t *pReturn, HSCRIPT hScope, bool bWait ) = 0; #endif //-------------------------------------------------------- @@ -1593,17 +1592,6 @@ typedef CScriptScopeT<> CScriptScope; // // 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 +1608,72 @@ struct ScriptHook_t // ----------------------------------------------------------------- - // Cached for when CanRunInScope() is called before Call() + // Only valid between CanRunInScope() and Call() HSCRIPT m_hFunc; bool m_bLegacy; - // Checks if there's a function of this name which would run in this scope - HSCRIPT CanRunInScope( HSCRIPT hScope ) + ScriptHook_t() : + m_bLegacy(false), + m_hFunc(NULL) { - extern IScriptVM *g_pScriptVM; - m_hFunc = g_pScriptVM->LookupHookFunction( m_desc.m_pszScriptName, hScope, m_bLegacy ); - return m_hFunc; } - // Checks if an existing func can be used - bool CheckFuncValid( HSCRIPT hFunc ) +#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 ) { - // TODO: Better crtieria for this? - if (hFunc) + 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 + bool CanRunInScope( HSCRIPT hScope ) + { + extern IScriptVM *g_pScriptVM; + + // Check the hook system first to make sure an unintended function in the script scope does not cancel out all script hooks. + m_hFunc = g_pScriptVM->LookupHookFunction( m_desc.m_pszScriptName, hScope ); + if ( !m_hFunc ) { - m_hFunc = hFunc; - return true; + // Legacy support if the new system is not being used + m_hFunc = g_pScriptVM->LookupFunction( m_desc.m_pszScriptName, hScope ); + m_bLegacy = true; } - return false; + else + { + m_bLegacy = false; + } + + 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 - if (!m_hFunc && !CanRunInScope(hScope)) - return false; + // Call() should not be called without CanRunInScope() check first, it caches m_hFunc for legacy support + Assert( CanRunInScope( hScope ) ); + // Legacy - else if (m_bLegacy) + if ( m_bLegacy ) { + Assert( 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) + if ( bRelease ) g_pScriptVM->ReleaseFunction( m_hFunc ); - m_hFunc = NULL; for (int i = 0; i < m_desc.m_Parameters.Count(); i++) @@ -1672,19 +1681,19 @@ 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 ); - if (bRelease) + ScriptStatus_t status = g_pScriptVM->ExecuteHookFunction( m_hFunc, m_desc.m_pszScriptName, pArgs, m_desc.m_Parameters.Count(), pReturn, hScope, true ); + + if ( bRelease ) g_pScriptVM->ReleaseFunction( m_hFunc ); m_hFunc = NULL; - return true; - } - return false; + return status == SCRIPT_DONE; + } } }; #endif diff --git a/sp/src/vscript/vscript_squirrel.cpp b/sp/src/vscript/vscript_squirrel.cpp index 15d8c1ff..932ab26f 100644 --- a/sp/src/vscript/vscript_squirrel.cpp +++ b/sp/src/vscript/vscript_squirrel.cpp @@ -188,9 +188,8 @@ public: //-------------------------------------------------------- // Hooks //-------------------------------------------------------- - virtual bool ScopeIsHooked( HSCRIPT hScope, const char *pszEventName ) override; - virtual HSCRIPT LookupHookFunction( const char *pszEventName, HSCRIPT hScope, bool &bLegacy ) override; - virtual ScriptStatus_t ExecuteHookFunction( const char *pszEventName, HSCRIPT hFunction, ScriptVariant_t *pArgs, int nArgs, ScriptVariant_t *pReturn, HSCRIPT hScope, bool bWait ) override; + virtual HSCRIPT LookupHookFunction( const char *pszEventName, HSCRIPT hScope ) override; + virtual ScriptStatus_t ExecuteHookFunction( HSCRIPT hFunction, const char *pszEventName, ScriptVariant_t *pArgs, int nArgs, ScriptVariant_t *pReturn, HSCRIPT hScope, bool bWait ) override; //-------------------------------------------------------- // External functions @@ -2348,55 +2347,38 @@ ScriptStatus_t SquirrelVM::ExecuteFunction(HSCRIPT hFunction, ScriptVariant_t* p return SCRIPT_DONE; } -bool SquirrelVM::ScopeIsHooked( HSCRIPT hScope, const char *pszEventName ) +HSCRIPT SquirrelVM::LookupHookFunction(const char *pszEventName, HSCRIPT hScope) { + SquirrelSafeCheck safeCheck(vm_); + // For now, assume null scope (which is used for global hooks) is always hooked - if (!hScope) - return true; - - SquirrelSafeCheck safeCheck(vm_); - - Assert(hScope != INVALID_HSCRIPT); - - 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))) + if ( hScope ) { - sq_pop(vm_, 3); - return false; + Assert( hScope != INVALID_HSCRIPT ); + + sq_pushroottable(vm_); + sq_pushstring(vm_, "Hooks", -1); + sq_get(vm_, -2); + sq_pushstring(vm_, "IsEventHookedInScope", -1); + sq_get(vm_, -2); + sq_push(vm_, -2); + sq_pushstring(vm_, pszEventName, -1); + sq_pushobject(vm_, *((HSQOBJECT*)hScope)); + sq_call(vm_, 3, SQTrue, SQTrue); + + SQBool val; + if (SQ_FAILED(sq_getbool(vm_, -1, &val))) + { + sq_pop(vm_, 3); + return nullptr; + } + + sq_pop(vm_, 4); + + if (!val) + return nullptr; } - 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); @@ -2411,31 +2393,31 @@ HSCRIPT SquirrelVM::LookupHookFunction(const char *pszEventName, HSCRIPT hScope, 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) +ScriptStatus_t SquirrelVM::ExecuteHookFunction(HSCRIPT hFunction, const char *pszEventName, ScriptVariant_t* pArgs, int nArgs, ScriptVariant_t* pReturn, HSCRIPT hScope, bool bWait) { - SquirrelSafeCheck safeCheck(vm_); - if (!hFunction) + if ( !hFunction ) return SCRIPT_ERROR; - if (hFunction == INVALID_HSCRIPT) - return SCRIPT_ERROR; + SquirrelSafeCheck safeCheck(vm_); 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]); diff --git a/sp/src/vscript/vscript_squirrel.nut b/sp/src/vscript/vscript_squirrel.nut index bafd55b7..1c1bd4e7 100644 --- a/sp/src/vscript/vscript_squirrel.nut +++ b/sp/src/vscript/vscript_squirrel.nut @@ -152,114 +152,144 @@ Hooks <- // table, string, closure, string function Add( scope, event, callback, context ) { + switch ( typeof scope ) + { + case "table": + case "instance": + case "class": + break; + default: + throw "invalid scope param"; + } + + if ( typeof event != "string" ) + throw "invalid event param"; + if ( typeof callback != "function" ) - throw "invalid callback param" + throw "invalid callback param"; - if ( !(scope in s_List) ) - s_List[scope] <- {} + if ( typeof context != "string" ) + throw "invalid context param"; - local t = s_List[scope] + if ( !(event in s_List) ) + s_List[event] <- {}; - if ( !(event in t) ) - t[event] <- {} + local t = s_List[event]; - t[event][context] <- callback + if ( !(scope in t) ) + t[scope] <- {}; + + t[scope][context] <- callback; } - function Remove( context, event = null ) + 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 t ) + if ( context in ctx ) { - delete t[context] + delete ctx[context]; } - // cleanup? - if ( !t.len() ) - delete scope[event] + if ( !ctx.len() ) + { + 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 ( !ev.len() ) - delete scope[kk] - } - - // cleanup? - if ( !scope.len() ) - delete s_List[k] - } - } - } - - function Call( scope, event, ... ) - { - local firstReturn - - // global hook; call all scopes - if ( !scope ) - { - vargv.insert( 0, null ) - foreach( sc,t in s_List ) - { - if ( event in t ) - { - vargv[0] = sc - foreach( context, callback in t[event] ) + if ( !ctx.len() ) { - //printf( "(%.4f) Calling hook '%s' of context '%s' in static iteration\n", Time(), event, context ) - - local curReturn = callback.acall(vargv) - if (firstReturn == null) - firstReturn = curReturn + if ( !rem ) + rem = []; + rem.append( ev ); + rem.append( scope ); } } } } - else if ( scope in s_List ) - { - local t = s_List[scope] - if ( event in t ) - { - vargv.insert( 0, scope ) - foreach( context, callback in t[event] ) - { - //printf( "(%.4f) Calling hook '%s' of context '%s'\n", Time(), event, context ) - local curReturn = callback.acall(vargv) - if (firstReturn == null) - firstReturn = curReturn + if ( rem ) + { + local c = rem.len() - 1; + for ( local i = 0; i < c; i += 2 ) + { + local ev = rem[i]; + local scope = rem[i+1]; + + if ( !s_List[ev][scope].len() ) + delete s_List[ev][scope]; + + if ( !s_List[ev].len() ) + delete s_List[ev]; + } + } + } + + 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 = r; + } + } + else if ( !scope ) // global hook + { + foreach ( sc, ctx in t ) + { + vargv[0] = sc; + + 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 = r; + } } } } - return firstReturn + return firstReturn; } - function ScopeHookedToEvent( scope, event ) + function IsEventHookedInScope( event, scope ) { - return ( scope in s_List ) && ( event in s_List[scope] ) + return ( event in s_List ) && ( scope in s_List[event] ) } }