[Feature Update] Lua's RegisterHook Function

[UE4SS]

While updating the docs for RegisterHook, I noticed how badly designed RegisterHook is. Check the changes introduced by #372 to learn how carefully one must tread in order to use RegisterHook properly.

I think that the hidden type-dependent behavior for the callback params is awful and something should probably be done about this.

If the solution ends up being a breaking change, we could always leave RegisterHook alone but deprecated and introduce one or more new functions.

I have a few of ideas for solutions:

1. This is a non-breaking change: Introduce two new functions: RegisterNativeHook and RegisterKismetHook, and splitting the behavior of RegisterHook. The use of RegisterHook would become deprecated, and guides would need to be updated, and eventually the function would be removed.
2. This is a breaking change: Redesign RegisterHook so that the first callback is always the pre-hook, which means it must be just an empty function or nil if the UFunction is non-native, and the second callback would alway the post-hook.
3. This is a breaking change: Investigate why we don't support pre-hooks for non-native functions, and possibly combine option 2 with supporting pre-hooks for non-native functions.

Options 2 and 3 are breaking changes because currently the first param is always used with the assumption that it's a post-hook and if we apply option 2 or 3 it will now be a pre-hook. While it won't generate a syntax error (unfortunately), it can lead to code that's dependent on the first callback being a post-hook being silently broken.

[Yangff]

I'm not quite sure how difficult it would be to force all hooks to change flags to native and then use the same hooks for normal script functions, and I'm guessing that maybe there would be a lot of hassle in restoring frames and such.

But I do have a proposal that for functions that are not actually implemented (such as events in the K2 series), neither of the current hooks will work. Is it possible to provide an additional variant of the ReigsterHook to handle them. An example would be /Script/Engine.Character:K2_OnMovementModeChanged, with a corresponding signature of void K2_OnMovementModeChanged( TEnumAsByte<EMovementMode> PrevMovementMode, TEnumAsByte<EMovementMode> NewMovementMode, uint8 PrevCustomMode, uint8 NewCustomMode ); Or /Script/Engine.GameModeBase:K2_OnLogout with K2_OnLogout(class AController* ExitingController).

Since firstly it's not a native function, since there is no corresponding exec implementation, and secondly it doesn't have any script implementation, the processevent won't call the corresponding script execution. I add an addreplacementcallback function to UFunction, set its FuncPtr to a hook function and set flags to FUNC_Native. then LuaMod sets the following substitution function for it (only does one callback to pre)

    static auto lua_unreal_script_function_replaced(Unreal::UnrealScriptFunctionCallableContext context, void* custom_data) -> void
    {
        lua_unreal_script_function_hook_pre(context, custom_data);
        lua_unreal_script_function_hook_post(context, custom_data);
    }

Of course the post hook is always null, because there's no point. It actually works, at least in the case where I only trigger one callback.

But anyway, I think in addition to adding the warning on the scripts/native diff, the docs should also mention that for such functions/events, the current registerhook can't be hooked successfully even if they return succ, and not seeing the callback does not mean they're not triggered.

[Yangff]

I come up with the following patch purely for PoC, by replacing the original logic of script hook. And you can test it with the following mod. Without the patch, the event will never trigger the callback.

(With that said, the patch itself is not meture and I'm already seeing some crash about remalloc that I've no clue.. maybe becasue it accidently hooked some real scripts function and not setting the return correctly etc.. but I'm just wondering about this conept and if anyone tried this before.. what possible problem there might be etc..)

BTW I think this code might still have some problem like it's not clean up the fframe ..

RegisterHook("/Script/Engine.Character:K2_OnMovementModeChanged", function (self)
    print("Movement mode changed")
end)

diff --git a/UE4SS/src/Mod/LuaMod.cpp b/UE4SS/src/Mod/LuaMod.cpp
index 6c6483d..aac0d71 100644
--- a/UE4SS/src/Mod/LuaMod.cpp
+++ b/UE4SS/src/Mod/LuaMod.cpp
@@ -368,6 +368,12 @@ namespace RC
         set_is_in_game_thread(lua_data.lua, false);
     }

+    static auto lua_unreal_script_function_replaced(Unreal::UnrealScriptFunctionCallableContext context, void* custom_data) -> void
+    {
+        lua_unreal_script_function_hook_pre(context, custom_data);
+        lua_unreal_script_function_hook_post(context, custom_data);
+    }
+
     static auto register_input_globals(const LuaMadeSimple::Lua& lua) -> void
     {
         LuaMadeSimple::Lua::Table key_table = lua.prepare_new_table();
@@ -3025,6 +3031,7 @@ Overloads:
             else if (func_ptr && func_ptr == Unreal::UObject::ProcessInternalInternal.get_function_address() &&
                      !unreal_function->HasAnyFunctionFlags(Unreal::EFunctionFlags::FUNC_Native))
             {
+                /*
                 ++m_last_generic_hook_id;
                 auto [callback_data, _] = LuaMod::m_script_hook_callbacks.emplace(unreal_function->GetFullName(), LuaCallbackData{*hook_lua, nullptr, {}});
                 callback_data->second.registry_indexes.emplace_back(hook_lua,
@@ -3035,6 +3042,17 @@ Overloads:
                                                 generic_pre_id,
                                                 generic_post_id,
                                                 unreal_function->GetFullName());
+                */
+                auto& custom_data = g_hooked_script_function_data.emplace_back(std::make_unique<LuaUnrealScriptFunctionData>(
+                        LuaUnrealScriptFunctionData{0, 0, unreal_function, mod, *hook_lua, lua_callback_registry_index, -1, lua_thread_registry_index}));
+                auto rep_id = unreal_function->RegisterReplacement(&lua_unreal_script_function_replaced, custom_data.get());
+                custom_data->pre_callback_id = rep_id;
+                m_generic_hook_id_to_native_hook_id.emplace(++m_last_generic_hook_id, rep_id);
+                generic_pre_id = m_last_generic_hook_id;
+                generic_post_id = m_last_generic_hook_id;
+                Output::send<LogLevel::Verbose>(STR("[RegisterHook] Registered replacement hook ({}) for {}\n"),
+                                                generic_pre_id,
+                                                unreal_function->GetFullName());
             }
             else
             {
Submodule deps/first/Unreal contains modified content
diff --git a/deps/first/Unreal/include/Unreal/UFunction.hpp b/deps/first/Unreal/include/Unreal/UFunction.hpp
index 8c36de6..a97c637 100644
--- a/deps/first/Unreal/include/Unreal/UFunction.hpp
+++ b/deps/first/Unreal/include/Unreal/UFunction.hpp
@@ -73,9 +73,11 @@ namespace RC::Unreal
          * The provided function will be executed when this UFunction object is called
          */
         auto SetFuncPtr(UnrealScriptFunction NewFuncPtr) -> void;
+        auto ForceNative() -> void;

         auto RegisterPreHook(const UnrealScriptFunctionCallable& PreCallback, void* CustomData = nullptr) -> CallbackId;
         auto RegisterPostHook(const UnrealScriptFunctionCallable& PostCallback, void* CustomData = nullptr) -> CallbackId;
+        auto RegisterReplacement(const UnrealScriptFunctionCallable& PostCallback, void* CustomData = nullptr) -> CallbackId;

         // Same as 'RegisterPre/PostHook' except it only fires if the instance pointer matches the context pointer.
         // Only safe if you can guarantee the validity of the instance passed to these functions.
diff --git a/deps/first/Unreal/include/Unreal/UFunctionStructs.hpp b/deps/first/Unreal/include/Unreal/UFunctionStructs.hpp
index 0ebfd56..3a6ea37 100644
--- a/deps/first/Unreal/include/Unreal/UFunctionStructs.hpp
+++ b/deps/first/Unreal/include/Unreal/UFunctionStructs.hpp
@@ -54,6 +54,7 @@ namespace RC::Unreal
         CallbackId HookIndexCounter;
         std::map<int32_t, UnrealScriptCallbackData> PreCallbacks;
         std::map<int32_t, UnrealScriptCallbackData> PostCallbacks;
+        std::map<int32_t, UnrealScriptCallbackData> ReplacementCallbacks;

     public:
         std::atomic<bool> bIsMidExecution{};
@@ -64,11 +65,13 @@ namespace RC::Unreal

         CallbackId AddPreCallback(const UnrealScriptFunctionCallable& Callable, void* CustomData = nullptr, UObject* FireOnInstance = nullptr);
         CallbackId AddPostCallback(const UnrealScriptFunctionCallable& Callable, void* CustomData = nullptr, UObject* FireOnInstance = nullptr);
+        CallbackId AddReplacementCallback(const UnrealScriptFunctionCallable& Callable, void* CustomData = nullptr, UObject* FireOnInstance = nullptr);
         bool RemoveCallback(CallbackId CallbackId);

         void RemoveAllCallbacks();
-
+        
         void FirePreCallbacks(UnrealScriptFunctionCallableContext& Context);
+        bool FireReplacementCallbacks(UnrealScriptFunctionCallableContext& Context);
         void FirePostCallbacks(UnrealScriptFunctionCallableContext& Context);

         UnrealScriptCallbackData* GetCallbackData(CallbackId);
diff --git a/deps/first/Unreal/src/UFunction.cpp b/deps/first/Unreal/src/UFunction.cpp
index a849313..dc0228d 100644
--- a/deps/first/Unreal/src/UFunction.cpp
+++ b/deps/first/Unreal/src/UFunction.cpp
@@ -23,6 +23,11 @@ namespace RC::Unreal
         GetFunc() = std::bit_cast<std::remove_reference_t<decltype(std::declval<UFunction>().GetFunc())>>(NewFuncPtr);
     }

+    auto UFunction::ForceNative() -> void
+   {
+       GetFunctionFlags() |= EFunctionFlags::FUNC_Native;
+   }
+
     //auto UFunction::GetNumParms() -> uint8_t
     //{
     //    uint8_t NumParameters = 0;
@@ -76,6 +81,13 @@ namespace RC::Unreal
         return Iterator->second;
     }

+    auto UFunction::RegisterReplacement(const UnrealScriptFunctionCallable& PreCallback, void* CustomData) -> CallbackId
+   {
+       UnrealScriptFunctionData& FunctionData = GetFunctionHookData();
+        ForceNative();
+       return FunctionData.AddReplacementCallback(PreCallback, CustomData);
+   }
+
     auto UFunction::RegisterPreHook(const UnrealScriptFunctionCallable& PreCallback, void* CustomData) -> CallbackId
     {
         UnrealScriptFunctionData& FunctionData = GetFunctionHookData();
diff --git a/deps/first/Unreal/src/UFunctionStructs.cpp b/deps/first/Unreal/src/UFunctionStructs.cpp
index 19eccd3..d15f8c4 100644
--- a/deps/first/Unreal/src/UFunctionStructs.cpp
+++ b/deps/first/Unreal/src/UFunctionStructs.cpp
@@ -17,7 +17,7 @@ namespace RC::Unreal
         this->PreCallbacks.emplace(std::piecewise_construct, std::forward_as_tuple(NewCallbackId), std::forward_as_tuple(Callable, CustomData, FireOnInstance));
         return NewCallbackId;
     }
-
+    
     CallbackId UnrealScriptFunctionData::AddPostCallback(const UnrealScriptFunctionCallable& Callable, void* CustomData, UObject* FireOnInstance)
     {
         CallbackId NewCallbackId = this->HookIndexCounter++;
@@ -25,6 +25,13 @@ namespace RC::Unreal
         return NewCallbackId;
     }

+    CallbackId UnrealScriptFunctionData::AddReplacementCallback(const UnrealScriptFunctionCallable& Callable, void* CustomData, UObject* FireOnInstance)
+    {
+        CallbackId NewCallbackId = this->HookIndexCounter++;
+        this->ReplacementCallbacks.emplace(std::piecewise_construct, std::forward_as_tuple(NewCallbackId), std::forward_as_tuple(Callable, CustomData, FireOnInstance));
+        return NewCallbackId;
+    }
+
     bool UnrealScriptFunctionData::RemoveCallback(CallbackId CallbackId)
     {
         auto CallbackData = GetCallbackData(CallbackId);
@@ -38,7 +45,7 @@ namespace RC::Unreal
             }
             else
             {
-                return PreCallbacks.erase(CallbackId) || PostCallbacks.erase(CallbackId);
+                return PreCallbacks.erase(CallbackId) || PostCallbacks.erase(CallbackId) || ReplacementCallbacks.erase(CallbackId);
             }
         }
         else
@@ -75,6 +82,17 @@ namespace RC::Unreal
             }
         }
     }
+    
+    bool UnrealScriptFunctionData::FireReplacementCallbacks(UnrealScriptFunctionCallableContext& Context)
+    {
+        for (auto ReplacementCallbackIterator = ReplacementCallbacks.begin(); ReplacementCallbackIterator != ReplacementCallbacks.end();)
+        {
+            if (ReplacementCallbackIterator->second.FireOnInstance && ReplacementCallbackIterator->second.FireOnInstance != Context.Context) { continue; }
+            ReplacementCallbackIterator->second.Callable(Context, ReplacementCallbackIterator->second.CustomData);
+            return true;
+        }
+        return false;
+    }

     void UnrealScriptFunctionData::FirePreCallbacks(UnrealScriptFunctionCallableContext& Context)
     {
@@ -169,27 +187,39 @@ namespace RC::Unreal
                 UnrealScriptFunctionCallableContext FuncContext(Context, TheStack, RESULT_DECL);

                 Iterator->second.bIsMidExecution = true;
-
-                try
-                {
-                    Iterator->second.FirePreCallbacks(FuncContext);
-                }
-                catch (std::exception& e)
-                {
-                    Output::send(STR("Error executing hook pre-callback {}: {}\n"), TheStack.CurrentNativeFunction()->GetPathName(), to_wstring(e.what()));
-                }
-
-                Iterator->second.GetOriginalFuncPtr()(Context, TheStack, RESULT_DECL);
-
-                try
-                {
-                    Iterator->second.FirePostCallbacks(FuncContext);
-                }
-                catch (std::exception& e)
-                {
-                    Output::send(STR("Error executing hook post-callback {}: {}\n"), TheStack.CurrentNativeFunction()->GetPathName(), to_wstring(e.what()));
-                }
-
+                do {
+                    try
+                    {
+                        if (Iterator->second.FireReplacementCallbacks(FuncContext)) {
+                            break;
+                        }
+                    }
+
+                    catch (std::exception& e)
+                   {
+                       Output::send(STR("Error executing hook replacement-callback {}: {}\n"), TheStack.CurrentNativeFunction()->GetPathName(), to_wstring(e.what()));
+                   }
+
+                    try
+                    {
+                        Iterator->second.FirePreCallbacks(FuncContext);
+                    }
+                    catch (std::exception& e)
+                    {
+                        Output::send(STR("Error executing hook pre-callback {}: {}\n"), TheStack.CurrentNativeFunction()->GetPathName(), to_wstring(e.what()));
+                    }
+
+                    Iterator->second.GetOriginalFuncPtr()(Context, TheStack, RESULT_DECL);
+
+                    try
+                    {
+                        Iterator->second.FirePostCallbacks(FuncContext);
+                    }
+                    catch (std::exception& e)
+                    {
+                        Output::send(STR("Error executing hook post-callback {}: {}\n"), TheStack.CurrentNativeFunction()->GetPathName(), to_wstring(e.what()));
+                    }
+                } while (false);
                 Iterator->second.bIsMidExecution = false;
             }
         }

[KO-9]

Not sure why this doesn't have more eyes on it. PreHook support is incredibly important and without it some (quote a lot) things are impossible.

I hope you can get this working without issue. I and many others would appreciate such support.

[igromanru]

Please put a PreHook support to the top on the ToDo list. Being able to modify parameters before they are getting passed to the original function is very powerful and would allow us to make mods much easier in a lot of cases.

[UE4SS]

Not sure why this doesn't have more eyes on it. PreHook support is incredibly important and without it some (quote a lot) things are impossible.

I hope you can get this working without issue. I and many others would appreciate such support.

Please put a PreHook support to the top on the ToDo list. Being able to modify parameters before they are getting passed to the original function is very powerful and would allow us to make mods much easier in a lot of cases.

Pre-hooks are currently a thing if the function is native, if it's a BP function, it's not a thing atm, as the Lua docs say. Just wanted to clarify this in case you missed this information above.

[igromanru]

Pre-hooks are currently a thing if the function is native, if it's a BP function, it's not a thing atm, as the Lua docs say. Just wanted to clarify this in case you missed this information above.

Thanks for clarification. I've seen the documentation, but I didn't realize that “/Script/” functions are all native. Sadly I'm working on a game that was made mainly in BPs. And lack of PreHook and being able to pass complex UScriptStruct as parameters makes it hard to find a way to achieve certain things.