fix call_hook crash with FSL and script function hook improvements (#223)

docs/lua/classes/scr_value_wrapper.md

@@ -2,7 +2,21 @@
 
 Class for wrapping arguments and return values of GTA script functions, used by add_script_function_hook.
 
-## Functions (6)
+## Functions (10)
+
+### `get_count()`
+
+Get the total number of elements in the wrapper.
+
+- **Returns:**
+  - `int`: The number of elements in the wrapper.
+
+**Example Usage:**
+```lua
+count = scr_value_wrapper:get_count()
+```
+
+---
 
 ### `get_int()`
 
@@ -36,6 +50,38 @@ scr_value_wrapper:set_int(0, 42)
 
 ---
 
+### `get_uns()`
+
+Get the unsigned int value currently contained by the wrapper.
+
+- **Parameters:**
+  - `index` (int): The index to access.
+
+- **Returns:**
+  - `int`: The current unsigned value.
+
+**Example Usage:**
+```lua
+value = scr_value_wrapper:get_uns(0)
+```
+
+---
+
+### `set_uns()`
+
+Set the unsigned int value contained by the wrapper.
+
+- **Parameters:**
+  - `index` (int): The index to access.
+  - `value` (int): The new unsigned value.
+
+**Example Usage:**
+```lua
+scr_value_wrapper:set_uns(0, 42)
+```
+
+---
+
 ### `get_float()`
 
 Get the float value currently contained by the wrapper.
@@ -97,3 +143,20 @@ Set the string value contained by the wrapper.
 ```lua
 scr_value_wrapper:set_string(0, "hello")
 ```
+
+---
+
+### `get_reference()`
+
+Get the reference value currently contained by the wrapper.
+
+- **Parameters:**
+  - `index` (int): The index to access.
+
+- **Returns:**
+  - `class`: The current value.
+
+**Example Usage:**
+```lua
+value = scr_value_wrapper:get_reference(0)
+```

docs/lua/tables/scr_function.md

@@ -2,7 +2,7 @@
 
 Table for calling and hooking GTA script functions. Calls must be made in the fiber pool.
 
-## Functions (4)
+## Functions (5)
 
 ### `call_script_function(script_name, function_name, pattern, return_type_string, args_)`
 
@@ -57,7 +57,7 @@ Hooks a script function. If the callback returns `false`, the original function
   - `script_name` (string): Name of the script.
   - `hook_name` (string): Name of the hook. This parameter needs to be unique.
   - `pattern` (string): Pattern to scan for within the script.
-  - `hook_func` (string): The callback function. It receives args and rets, which can be read or set via `get/set_int/float/string` methods. Return value determines whether to skip or execute the original function.
+  - `hook_func` (string): The callback function. It receives args and rets, which can be read or set via `get/set_int/uns/float/string/reference` methods. Return value determines whether to skip or execute the original function.
 
 **Example Usage:**
 ```lua
@@ -71,6 +71,28 @@ scr_function.add_script_function_hook("some_script", "my_hook", "2D 00 ? ? 00 61
 end)
 ```
 
+### `add_script_function_hook(script_name, hook_name, instruction_pointer, hook_func)`
+
+Hooks a script function directly using the function position. If the callback returns `false`, the original function is skipped, and values in `rets` are pushed to the stack. If `true`, the original function executes normally.
+
+- **Parameters:**
+  - `script_name` (string): Name of the script.
+  - `hook_name` (string): Name of the hook. This parameter needs to be unique.
+  - `instruction_pointer` (int): Position of the function within the script.
+  - `hook_func` (string): The callback function. It receives args and rets, which can be read or set via `get/set_int/uns/float/string/reference` methods. Return value determines whether to skip or execute the original function.
+
+**Example Usage:**
+```lua
+scr_function.add_script_function_hook("some_script", "my_hook", 0x10BE, function(args, rets)
+  local val = args:get_int(0)
+  if val == 1 then
+    rets:set_int(0, 100)
+    return false
+  end
+  return true
+end)
+```
+
 ### `remove_script_function_hook(script_name, hook_name)`
 
 Removes an existing script function hook.

src/hooking/call_hook.cpp

@@ -11,7 +11,7 @@ namespace big
 {
 	call_hook_memory::call_hook_memory()
 	{ 
-		m_memory = VirtualAlloc((void*)((uintptr_t)GetModuleHandle(0) + 0x20000000), 1024, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE);
+		m_memory = VirtualAlloc((void*)((uintptr_t)GetModuleHandle(0) + 0x40000000), 1024, MEM_COMMIT | MEM_RESERVE, PAGE_EXECUTE_READWRITE);
 		m_offset = 0;
 	}
 

src/lua/bindings/scr_function.cpp

@@ -17,7 +17,7 @@ namespace lua::scr_function
 
 	// Lua API: Table
 	// Name: scr_function
-	// Table for calling GTA script functions. Needs to be called in the fiber pool. Only call the function when necessary.
+	// Table for calling and hooking GTA script functions. Calls must be made in the fiber pool.
 
 	// Lua API: function
 	// Table: scr_function
@@ -358,22 +358,34 @@ namespace lua::scr_function
 	class scr_value_wrapper_t
 	{
 		rage::scrValue* m_data;
+		std::uint32_t m_count;
 
 	public:
-		scr_value_wrapper_t(rage::scrValue* data) :
-		    m_data(data)
+		scr_value_wrapper_t(rage::scrValue* data, std::uint32_t count) :
+		    m_data(data),
+		    m_count(count)
 		{
 		}
 
+		// Lua API: Function
+		// Class: scr_value_wrapper
+		// Name: get_count
+		// Returns: int: The number of elements in the wrapper.
+		// Get the total number of elements in the wrapper.
+		std::uint32_t get_count()
+		{
+			return m_count;
+		}
+
 		// Lua API: Function
 		// Class: scr_value_wrapper
 		// Name: get_int
 		// Param: index: int: The index to access.
 		// Returns: int: The current value.
 		// Get the int value currently contained by the wrapper.
-		int get_int(int index)
+		std::int32_t get_int(std::uint32_t index)
 		{
-			return m_data[index].Int;
+			return index < m_count ? m_data[index].Int : 0;
 		}
 
 		// Lua API: Function
@@ -382,20 +394,44 @@ namespace lua::scr_function
 		// Param: index: int: The index to access.
 		// Param: value: int: The new value.
 		// Set the int value contained by the wrapper.
-		void set_int(int index, int value)
+		void set_int(std::uint32_t index, std::int32_t value)
 		{
+			if (index < m_count)
 				m_data[index].Int = value;
 		}
 
+		// Lua API: Function
+		// Class: scr_value_wrapper
+		// Name: get_uns
+		// Param: index: int: The index to access.
+		// Returns: int: The current unsigned value.
+		// Get the unsigned int value currently contained by the wrapper.
+		std::uint32_t get_uns(std::uint32_t index)
+		{
+			return index < m_count ? m_data[index].Uns : 0;
+		}
+
+		// Lua API: Function
+		// Class: scr_value_wrapper
+		// Name: set_uns
+		// Param: index: int: The index to access.
+		// Param: value: int: The new unsigned value.
+		// Set the unsigned int value contained by the wrapper.
+		void set_uns(std::uint32_t index, std::uint32_t value)
+		{
+			if (index < m_count)
+				m_data[index].Uns = value;
+		}
+
 		// Lua API: Function
 		// Class: scr_value_wrapper
 		// Name: get_float
 		// Param: index: int: The index to access.
 		// Returns: float: The current value.
 		// Get the float value currently contained by the wrapper.
-		float get_float(int index)
+		float get_float(std::uint32_t index)
 		{
-			return m_data[index].Float;
+			return index < m_count ? m_data[index].Float : 0.0f;
 		}
 
 		// Lua API: Function
@@ -404,8 +440,9 @@ namespace lua::scr_function
 		// Param: index: int: The index to access.
 		// Param: value: float: The new value.
 		// Set the float value contained by the wrapper.
-		void set_float(int index, float value)
+		void set_float(std::uint32_t index, float value)
 		{
+			if (index < m_count)
 				m_data[index].Float = value;
 		}
 
@@ -415,9 +452,9 @@ namespace lua::scr_function
 		// Param: index: int: The index to access.
 		// Returns: string: The current value.
 		// Get the string value currently contained by the wrapper.
-		const char* get_string(int index)
+		const char* get_string(std::uint32_t index)
 		{
-			return m_data[index].String;
+			return index < m_count ? m_data[index].String : nullptr;
 		}
 
 		// Lua API: Function
@@ -426,10 +463,22 @@ namespace lua::scr_function
 		// Param: index: int: The index to access.
 		// Param: value: string: The new value.
 		// Set the string value contained by the wrapper.
-		void set_string(int index, const char* value)
+		void set_string(std::uint32_t index, const char* value)
 		{
+			if (index < m_count)
 				m_data[index].String = value;
 		}
+
+		// Lua API: Function
+		// Class: scr_value_wrapper
+		// Name: get_reference
+		// Param: index: int: The index to access.
+		// Returns: class: The current value.
+		// Get the reference value currently contained by the wrapper.
+		scr_value_wrapper_t get_reference(std::uint32_t index)
+		{
+			return index < m_count ? scr_value_wrapper_t(m_data[index].Reference, m_count) : scr_value_wrapper_t(nullptr, 0);
+		}
 	};
 
 	// Lua API: function
@@ -451,11 +500,44 @@ namespace lua::scr_function
 	//   return true
 	// end)
 	// ```
-	static void add_script_function_hook(const std::string& script_name, const std::string& hook_name, const std::string& pattern, sol::protected_function hook_func)
+	static void add_script_function_hook_by_signature(const std::string& script_name, const std::string& hook_name, const std::string& pattern, sol::protected_function hook_func)
 	{
-		big::g_script_function_hook_service->add_hook(rage::joaat(script_name), hook_name, pattern, [hook_func](rage::scrValue* args, rage::scrValue* rets) -> bool {
-			scr_value_wrapper_t lua_args(args);
-			scr_value_wrapper_t lua_rets(rets);
+		big::g_script_function_hook_service->add_hook(rage::joaat(script_name), hook_name, pattern, [hook_func](rage::scrValue* args, const std::uint32_t argCount, rage::scrValue* rets, const std::uint32_t retCount) -> bool {
+			scr_value_wrapper_t lua_args(args, argCount);
+			scr_value_wrapper_t lua_rets(rets, retCount);
+
+			auto result = hook_func(lua_args, lua_rets);
+			if (!result.valid())
+				return true; // don't skip the original if lua fails
+
+			return result.get<bool>();
+		});
+	}
+
+	// Lua API: function
+	// Table: scr_function
+	// Name: add_script_function_hook
+	// Param: script_name: string: Name of the script.
+	// Param: hook_name: string: Name of the hook. This parameter needs to be unique.
+	// Param: instruction_pointer: integer: Position of the function within the script.
+	// Param: hook_func: function: The callback function. It receives args and rets, which can be read or set via `get/set_int/float/string` methods. Return value determines whether to skip or execute the original function.
+	// Hooks a script function directly using the function position. If the callback returns `false`, the original function is skipped, and values in `rets` are pushed to the stack. If `true`, the original function executes normally.
+	// **Example Usage:**
+	// ```lua
+	// scr_function.add_script_function_hook("some_script", "my_hook", 0x10BE, function(args, rets)
+	//   local val = args:get_int(0)
+	//   if val == 1 then
+	//     rets:set_int(0, 100)
+	//     return false
+	//   end
+	//   return true
+	// end)
+	// ```
+	static void add_script_function_hook_by_instruction_pointer(const std::string& script_name, const std::string& hook_name, const std::uint32_t instruction_pointer, sol::protected_function hook_func)
+	{
+		big::g_script_function_hook_service->add_hook(rage::joaat(script_name), hook_name, instruction_pointer, [hook_func](rage::scrValue* args, const std::uint32_t argCount, rage::scrValue* rets, const std::uint32_t retCount) -> bool {
+			scr_value_wrapper_t lua_args(args, argCount);
+			scr_value_wrapper_t lua_rets(rets, retCount);
 
 			auto result = hook_func(lua_args, lua_rets);
 			if (!result.valid())
@@ -484,17 +566,21 @@ namespace lua::scr_function
 	{
 		auto ut = state.new_usertype<scr_value_wrapper_t>("scr_value_wrapper");
 
+        ut["get_count"]     = &scr_value_wrapper_t::get_count;
 		ut["get_int"]       = &scr_value_wrapper_t::get_int;
 		ut["set_int"]       = &scr_value_wrapper_t::set_int;
+		ut["get_uns"]       = &scr_value_wrapper_t::get_uns;
+		ut["set_uns"]       = &scr_value_wrapper_t::set_uns;
 		ut["get_float"]     = &scr_value_wrapper_t::get_float;
 		ut["set_float"]     = &scr_value_wrapper_t::set_float;
 		ut["get_string"]    = &scr_value_wrapper_t::get_string;
 		ut["set_string"]    = &scr_value_wrapper_t::set_string;
+		ut["get_reference"] = &scr_value_wrapper_t::get_reference;
 
 		auto ns = state["scr_function"].get_or_create<sol::table>();
 
 		ns["call_script_function"]        = sol::overload(call_script_function_by_signature, call_script_function_by_instruction_pointer);
-        ns["add_script_function_hook"]    = add_script_function_hook;
+		ns["add_script_function_hook"]    = sol::overload(add_script_function_hook_by_signature, add_script_function_hook_by_instruction_pointer);
 		ns["remove_script_function_hook"] = remove_script_function_hook;
 	}
 }

src/services/script_function_hook/script_function_hook.asm

@@ -28,7 +28,6 @@ script_vm_on_enter_end_handler PROC
     mov [rsp + 20h], r15 ; ctx
     mov [rsp + 28h], r13 ; code
     call process_script_function_hooks
-
     add rsp, 30h
 
     ; restore stuff

src/services/script_function_hook/script_function_hook_service.cpp

@@ -38,10 +38,13 @@ namespace big
 		g_script_function_hook_service = nullptr;
 	}
 
-	void script_function_hook_service::process_hook(const function_hook& hook, std::uint8_t*& ip, std::uint8_t*& base, rage::scrValue*& sp, rage::scrValue* fp, std::uint8_t** code)
+	void script_function_hook_service::process_hook(function_hook& hook, std::uint8_t*& ip, std::uint8_t*& base, rage::scrValue*& sp, rage::scrValue* fp, std::uint8_t** code)
 	{
+		// prevent reentrancy in case the hook calls the original function via the script_function class
+		hook.m_active = true;
+
 		rage::scrValue ret_val[256]{};
-		if (!hook.m_hook_func(fp, ret_val))
+		if (!hook.m_hook_func(fp, hook.m_arg_count, ret_val, hook.m_ret_count))
 		{
 			// push return value onto stack
 			for (std::uint32_t i = 0; i < hook.m_ret_count; i++)
@@ -52,9 +55,14 @@ namespace big
 			ip = &code[offset >> 14][offset & 0x3FFF] - 1;
 			base = &ip[-offset];
 		}
+
+		hook.m_active = false;
 	}
 
 	void script_function_hook_service::resolve_hook(function_hook& hook, rage::scrProgram* program)
+	{
+		std::uint32_t entry_ip = hook.m_start_ip;
+		if (entry_ip == 0)
 		{
 			auto location = scripts::get_code_location_by_pattern(program, memory::pattern(hook.m_pattern));
 			if (!location)
@@ -64,8 +72,10 @@ namespace big
 			}
 
 			LOG(VERBOSE) << "Found pattern " << hook.m_name << " at " << HEX_TO_UPPER(*location) << " in script " << program->m_name;
+			entry_ip = *location;
+		}
 
-        std::uint32_t post_enter = *location + scripts::get_insn_size(program, *location);
+        std::uint32_t post_enter = entry_ip + scripts::get_insn_size(program, entry_ip);
 		std::uint32_t pos = post_enter;
 
 		while (pos < program->m_code_size)
@@ -83,6 +93,7 @@ namespace big
 				{
 					hook.m_start_ip = post_enter;
 					hook.m_end_ip = pos;
+					hook.m_arg_count = op[1];
 					hook.m_ret_count = op[2];
 					hook.m_resolved = true;
 					break;
@@ -95,9 +106,9 @@ namespace big
 
 	void script_function_hook_service::process_hooks(std::uint8_t*& ip, std::uint8_t*& base, rage::scrValue*& sp, rage::scrValue* fp, rage::scrThreadContext* ctx, std::uint8_t** code)
 	{
-		for (const auto& hook : m_hooks)
+		for (auto& hook : m_hooks)
 		{
-			if (!hook.m_resolved || hook.m_script != ctx->m_script_hash || hook.m_start_ip != static_cast<std::uint32_t>(ip - base))
+			if (!hook.m_resolved || hook.m_active || hook.m_script != ctx->m_script_hash || hook.m_start_ip != static_cast<std::uint32_t>(ip - base))
 				continue;
 
 			process_hook(hook, ip, base, sp, fp, code);
@@ -137,6 +148,28 @@ namespace big
 		m_hooks.push_back(hook);
 	}
 
+	void script_function_hook_service::add_hook(rage::joaat_t script, const std::string& name, std::uint32_t ip, const hook_func& hook_func)
+	{
+		// prevent duplicates
+		for (const auto& hook : m_hooks)
+		{
+			if (hook.m_script == script && hook.m_name == name)
+				return;
+		}
+
+		function_hook hook{};
+		hook.m_script = script;
+		hook.m_name = name;
+		hook.m_start_ip = ip; // init with entry IP now, we will update it to point to post-enter in resolve_hook
+		hook.m_hook_func = hook_func;
+
+		// if the program is already available, set the data now, otherwise we will try in init_native_tables
+		if (auto program = gta_util::find_script_program(script))
+			resolve_hook(hook, program);
+
+		m_hooks.push_back(hook);
+	}
+
     void script_function_hook_service::remove_hook(rage::joaat_t script, const std::string& name)
 	{
 		m_hooks.erase(std::remove_if(m_hooks.begin(), m_hooks.end(), [&](const function_hook& hook) {

src/services/script_function_hook/script_function_hook_service.hpp

@@ -12,7 +12,7 @@ namespace big
 	class script_function_hook_service
 	{
 	public:
-		using hook_func = std::function<bool(rage::scrValue*, rage::scrValue*)>;
+		using hook_func = std::function<bool(rage::scrValue* args, const std::uint32_t argCount, rage::scrValue* rets, const std::uint32_t retCount)>;
 
 		script_function_hook_service();
 		~script_function_hook_service();
@@ -20,6 +20,7 @@ namespace big
 		void process_hooks(std::uint8_t*& ip, std::uint8_t*& base, rage::scrValue*& sp, rage::scrValue* fp, rage::scrThreadContext* ctx, std::uint8_t** code);
 		void resolve_hooks(rage::scrProgram* program);
 		void add_hook(rage::joaat_t script, const std::string& name, const std::string& pattern, const hook_func& hook_func);
+		void add_hook(rage::joaat_t script, const std::string& name, std::uint32_t ip, const hook_func& hook_func);
 		void remove_hook(rage::joaat_t script, const std::string& name);
 
 	private:
@@ -30,12 +31,14 @@ namespace big
 			std::string m_pattern;
 			std::uint32_t m_start_ip;
 			std::uint32_t m_end_ip;
+			std::uint32_t m_arg_count;
 			std::uint32_t m_ret_count;
 			hook_func m_hook_func;
 			bool m_resolved;
+			bool m_active;
 		};
 
-		static void process_hook(const function_hook& hook, std::uint8_t*& ip, std::uint8_t*& base, rage::scrValue*& sp, rage::scrValue* fp, std::uint8_t** code);
+		static void process_hook(function_hook& hook, std::uint8_t*& ip, std::uint8_t*& base, rage::scrValue*& sp, rage::scrValue* fp, std::uint8_t** code);
 		static void resolve_hook(function_hook& hook, rage::scrProgram* program);
 
 		std::vector<function_hook> m_hooks;