-
Notifications
You must be signed in to change notification settings - Fork 194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added 'RegisterHookFromBP' and 'UnregisterHookFromBP' for BP Mods #421
base: main
Are you sure you want to change the base?
Conversation
I wonder if this can be made more ergonomic and less error prone, e.g. instead of passing the function name to call you pass an event |
I agree that this would be better, and it should be possible at least for the |
Yeah, I personally think the HookName is fine, but I honestly don't like having 'FunctionToCall' as it is either and being able to pass a function as the param didn't cross my mind, so if it's possible I'd prefer it over passing an FString. |
There is no type-checking to make sure the param types of |
We should probably implement an overload to EDIT: // UE4SS/src/LuaType/LuaXProperty.cpp -> XProperty::setup_member_functions
if (lua.is_table()) // PropertyTypes table
{
// ...
}
else if (lua.is_userdata()) // FFieldClass, returned from Property:GetClass()
{
auto ffield_class = lua.get_userdata<LuaType::XFieldClass>();
lua.set_bool(lua_object.get_remote_cpp_object()->IsA(ffield_class.get_local_cpp_object()));
return 1;
} I don't have time to make any kind of PR for this right now but this would be the ideal way to compare the types. |
@Okaetsu Maybe we can put the new |
Do you mind elaborating on this part further? I assume you want me to type check the properties of both UFunctions which I'm not sure how to do exactly. From a quick look it doesn't seem to be possible to iterate the properties for UObjects/UFunctions with the functions from those Docs unless I missed something. Also yeah I can include IsA in this PR. |
UFunction inherits from UStruct so you should be able to use |
I assume the above should be correct? It results in the following:
I checked and HookFunctionObj is both valid and a UFunction so I'm not sure if I'm calling |
Your code is correct. class UFunction : public UObjectBase<Unreal::UFunction, UFunctionName>
{
+ public:
+ using Super = UStruct;
+
private:
// This is the 'this' pointer for this UFunction
Unreal::UObject* m_base{}; And this to template <LuaMadeSimple::Type::IsFinal is_final>
auto UFunction::setup_member_functions(const LuaMadeSimple::Lua::Table& table) -> void
{
+ Super::setup_member_functions<LuaMadeSimple::Type::IsFinal::No>(table);
+
table.add_pair("GetFunctionFlags", [](const LuaMadeSimple::Lua& lua) -> int { |
If the -- 'property1' being the property from the original function
-- 'property2' being the property from the user provided callback
if (property2:IsA(property1:GetClass())) then
-- Success
else
-- Type didn't match
end This is one way to implement the type-checking: local original_types = {}
local callback_types = {}
-- Pretend there's code here iterating types for both functions and
-- adding each property to the corresponding table above.
if #original_types != #callback_types then
-- Failure, number of params (including return value) doesn't match.
return
end
for i = 1, #original_types, 1 do
if (callback_types[i]:IsA(original_types[i]:GetClass())) then
-- Success
else
-- Failure
end
end |
I've run into a small problem which I'm not sure how to solve myself: for i = 1, #OriginalTypes, 1 do
if (CallbackTypes[i]:IsA(OriginalTypes[i]:GetClass())) then
else
error("Param #" .. i .. " did not match the expected type '" .. OriginalTypes[i]:GetClass():GetFName():ToString() ..
"' got '" .. CallbackTypes[i]:GetClass():GetFName():ToString() .. "'")
end
end
To my knowledge it's not possible to have normal floats anymore atleast through blueprints in UE5 which means the params in my callback function are always going to be DoubleProperty while the original function has FloatProperty params. |
This is interesting. |
According to this, it looks to me like a BP float can be either double or single precision automatically depending on context. |
This means that we will either need a string comparison against Float/DoubleProperty for every param type, or we need to implement NumericProperty and its member function IsFloatingPoint and do: if ((CallbackTypes[i]:IsA(OriginalTypes[i]:GetClass())) or (CallbackTypes[i]:IsFloatingPoint() and OriginalTypes[i]:IsFloatingPoint())) then
-- Success
end NumericProperty and IsFloatingPoint are already implemented in C++, we'd just need to implement the Lua part. |
I added NumericProperty & IsFloatingPoint to Lua. -- Excuse the extremely long line but I believe Lua has short circuiting so
-- this should be more performant than separating this into two variables.
if ((CallbackTypes[i]:IsA(OriginalTypes[i]:GetClass())) or (type(OriginalTypes[i].IsFloatingPoint) == "function" and CallbackTypes[i]:IsFloatingPoint() and OriginalTypes[i]:IsFloatingPoint())) then
-- Success
end |
What's the status of this PR? Ready for review? |
Changelog & docs need updating to reflect the changes in this PR. |
Just to be clear, my previous comment wasn't a response to the question about the status of this PR. |
I think we can improve performance by removing all the string comparisons to "UFunction"`: -- At file/global-scope:
local FunctionClass = StaticFindObject("/Script/CoreUObject.Function")
if not FunctionClass:IsValid() then
-- No need to try again because the function class is guaranteed to exist when Lua mods are initialized.
-- If it doesn't exist, then something's gone horribly wrong or a massive engine change has occurred that removed/renamed this object.
error("/Script/CoreUObject.Function not found!")
end
-- Anywhere there's a comparison with "UFunction":
if not OriginalFunction:IsA(FunctionClass) then
error("Not a function!")
end This does dive back into C++ land to execute |
Sorry for the wait, I've been busy with some stuff. It's ready for review now. I had to add EPropertyFlag because it was also including ReferenceParams for the function params and I'd want this part double checked in case it could cause issues: // LuaMadeSimple.hpp
else if constexpr (std::is_same_v<ValueType, unsigned long long>)
{
lua_pushinteger(get_lua_instance().get_lua_state(), static_cast<long long>(value));
} I had to include uint64_t aswell since that's what EPropertyFlags is using and I wasn't sure exactly how lua wants it passed down, but it did seem to work from my testing. |
This is a problem because Lua doesn't support unsigned 64-bit integers. I think we either need to implement EPropertyFlags as a non-flag type in Lua and convert to the real enum-flag when it's passed to C++, or do some fancy thing where the type is actually two 32-bit integers under the hood and we convert that to an unsigned 64-bit integer when we actually use it in C++. |
Regarding the property flags problem, I wonder if it's possible to make an opaque Lua type that's just a uint64 on the C++ side. struct UInt64Name
{
constexpr static const char* ToString()
{
return "UInt64";
}
};
class UInt64 : LocalObjectBase<uint64, UInt64Name>
{
private:
explicit UInt64(uint64 number) : LocalObjectBase<uint64, UInt64Name>(number)
{
}
public:
UInt64() = delete;
auto static construct(const LuaMadeSimple::Lua& lua, uint64 number)
{
UInt64 lua_object{number};
lua.transfer_stack_object(std::move(lua_object));
}
}; Seems there are plenty of way to get around not having proper uint64 support. |
Changed HasAll/AnyPropertyFlags to use this new type. Also changed the EPropertyFlags table to use this type. This required adding __bor and __band metamethods.
I've gone ahead and added and switched to |
A changelog entry is missing for this feature, and the docs haven't been updated either. |
I haven't done docs yet since I wasn't sure if we're going with the original way of registering a callback function with |
I completely forgot about that. |
Something I forgot is that this implementation defaults to using pre-hooks and there's no way to hook post-hooks at the moment. I'm wondering if I should add a third optional param (can be left empty) for specifying a post-hook callback function. EDIT: I was also thinking of renaming the params since the implementation isn't final yet. Something like: Any suggestions for other names are welcome. |
I'm not sure. I guess adding an optional extra param to the BP function makes sense but it must be documented identically to the Lua function, and in fact, it must be documented identically even if an additional param isn't added because the defualt "pre-hook" still has to play by the same rules as the "pre-hook" of the Lua function. |
This was making LuaType::ObjectProperty use the wrong metatable, and preventing a call to 'GetPropertyClass' from working.
Also added an extra check to make sure it's actually valid. Note that there were some formatting errors in this file and this commit also fixes those.
@Okaetsu I don't know if you've tested this after you added type-checking but for me it was broken due to a pre-existing bug in UE4SS. I've now tested this PR as of my last commit and it seems to be working. I'm not sure that I'm happy with requiring the precise type for the context param in the BP callback. I've already made the code change so if you agree with this, I'll push the change. |
I didn't notice anything from my personal testing, but good to know it's fixed now.
I guess we can do that, but I'll just have to document it so people are aware in case any crashing happens. Feel free to push the change. Also regarding the docs do you have any ideas for some generic Unreal functions that'd get called for every game? I tried using ClientRestart as an example, but depending on the game it'll get called before the ModActor is spawned. Another one I tried using was a KismetMathLibrary function and calling it manually, but you can't hook those at the moment and I'll make a separate issue for this since it's off-topic. |
For what purpose ? |
Yeah, I figured since there's a lot going on with the feature plus it's a blueprint so it might be better to have a full example that can showcase it actually working. |
I don't know of any functions of the top of my head but here's some C++ code that you can use to save all functions that go through ProcessEvent and ProcessInternal/ProcessLocalScriptFunction. Here's some "working" code, it'll work as long as you put the code in the correct place. class MyAwesomeMod : public RC::CppUserModBase
{
private:
Output::Targets<Output::NewFileDevice> NativeFunctionsExecuted{};
Output::Targets<Output::NewFileDevice> ScriptFunctionsExecuted{};
public:
// Pretending the constructor exists for brevity.
auto on_unreal_init() -> void override
{
auto& NativeFunctionsExecutedDevice = NativeFunctionsExecuted.get_device<Output::NewFileDevice>();
NativeFunctionsExecutedDevice.set_file_name_and_path(StringType{UE4SSProgram::get_program().get_working_directory()} + STR("\\FUNCTIONS_EXECUTED_NATIVE.txt"));
NativeFunctionsExecutedDevice.set_formatter([](File::StringViewType string) {
return File::StringType{string};
});
Hook::RegisterProcessEventPreCallback([&](UObject* Context, UFunction* Function, ...) {
if (!Function) { return; }
NativeFunctionsExecuted.send(STR("{}.{}\n"), Context->GetName(), Function->GetName());
});
auto& ScriptFunctionsExecutedDevice = ScriptFunctionsExecuted.get_device<Output::NewFileDevice>();
ScriptFunctionsExecutedDevice.set_file_name_and_path(StringType{UE4SSProgram::get_program().get_working_directory()} + STR("\\FUNCTIONS_EXECUTED_SCRIPT.txt"));
ScriptFunctionsExecutedDevice.set_formatter([](File::StringViewType string) {
return File::StringType{string};
});
auto script_callback = [&](UObject* Context, FFrame& Stack, ...) {
ScriptFunctionsExecuted.send(STR("{}.{}\n"), Context->GetName(), Stack.Node()->GetName());
};
if (UObject::ProcessLocalScriptFunctionInternal.is_ready() && Version::IsAtLeast(4, 22))
{
Hook::RegisterProcessLocalScriptFunctionPreCallback(script_callback);
}
else if (UObject::ProcessInternalInternal.is_ready() && Version::IsBelow(4, 22))
{
Hook::RegisterProcessInternalPreCallback(script_callback);
}
}
}; |
Thanks! I'll do some testing. |
Looking at this again, I realized there's another limitation to this implementation which is not being able to modify any of the params or return values compared to Lua, but I think just being able to hook functions is already very helpful. I'll note it in the docs. |
What's the status here? |
Description
I added ways to register and unregister hooks from within Blueprints since there's a lot of mods combining both Lua and Blueprints to achieve this at the moment. I've also put checks in place to prevent the same Actor from registering multiple hooks per function and type checking. Context from the hook is also passed to blueprints.
Param count must match the hooked function and this includes Context.
I've done some testing and it seems to be working as intended, but there might be improvements that could be made to the code. The following pictures are all that is needed for registering functions from within blueprints.
You will need the custom events
RegisterHookFromBP
,UnregisterHookFromBP
and a custom function of your choice that will be called by the hook. Param types will be automatically resolved by the lua script handling the hooks so all you have to do is ensure that the Params for your custom function match that hook.Contents of
MyFunction
that gets called by the hook.Results in the following output when the hook gets called:
Palworld was used for the example.
Type of change
How Has This Been Tested?
I've done some very basic testing in both UE 4.27 and UE 5.1 games by trying to hook to different functions with different types. Lua limitations will still apply to some types as of UE4SS 3.0.1.
Checklist