Script hooks fixes and script hook manager #192
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The first commit simplifies the purpose of
IScriptVM::LookupHookFunction()
by merging it withIScriptVM::ScopeIsHooked()
and making it only return the hook function instead of also callingLookupFunction()
, fixesScriptHook_t::Call()
always returning true even when callbacks fail, gives the new hook system priority over legacy support by checking the new system before function name which reduces the amount of unnecessary script lookup on each hook call.When I wrote the
Hooks
functions I did not expect it to be added as is - it was more of an example; I've re-structured the storage to store event names first, which reduces the amount of tables created, and also swapped the parameters ofHooks.Remove()
, aseventname, context
is more natural and in-line with theAdd()
function.The second commit implements
CScriptHookManager
which caches script hook events and the hook function in C++ to reduce 3 consecutive script function lookup on eachScriptHook_t::CanRunInScope()
check - which is run for each hook, often every frame on every entity - to 2 RB tree lookups on C++ and a legacy script function lookup fallback. Though I'm not entirely sure if CUtlMap is the right container to use for this purpose.I highly suggest adding a flag on
ScriptHook_t
(or creating a legacy hook class) for all hooks implemented before mapbase 7.0 that enables the legacy fallback. Otherwise all future unused hooks will keep polling the script VM for a hook function.PR Checklist
develop
branch OR targets another branch with a specific goal in mind