Merge pull request #53 from apple1417/master

console command fixes

Author: apple1417

changelog.md

@@ -16,6 +16,29 @@
 
   [427c8734](https://github.com/bl-sdk/unrealsdk/commit/427c8734)
 
+- Fixed that `unrealsdk::commands::has_command` and `unrealsdk::commands::remove_command` were case
+  sensitive, while `unrealsdk::commands::add_command` and the callbacks were not. Commands should be
+  now be case insensitive everywhere.
+
+  [b641706d](https://github.com/bl-sdk/unrealsdk/commit/b641706d)
+
+- Fixed that the executed command message of custom sdk commands would not appear in console if you
+  increased the minimum log level, and that they may have appeared out of order with respects to
+  native engine messages.
+
+  [b652da13](https://github.com/bl-sdk/unrealsdk/commit/b652da13)
+
+- Added an additional console command hook in BL2, to cover commands not run directly via console.
+
+  [1200fca4](https://github.com/bl-sdk/unrealsdk/commit/1200fca4)
+
+- Renamed the `unrealsdk.locking_process_event` (previously `UNREALSDK_LOCKING_PROCESS_EVENT`)
+  setting to `unrealsdk.locking_function_calls`, and expanded it's scope to cover all function
+  calls. This fixes a few more possibilities for lockups.
+
+  [bebaeab4](https://github.com/bl-sdk/unrealsdk/commit/bebaeab4)
+
+
 ## v1.4.0
 - Fixed that UE3 `WeakPointer`s would always return null, due to an incorrect offset in the
   `UObject` header layout.

src/unrealsdk/commands.cpp

@@ -29,7 +29,7 @@ UNREALSDK_CAPI(bool, add_command, const wchar_t* cmd, size_t size, DLLSafeCallba
         return false;
     }
 
-    commands.emplace(lower_cmd, callback);
+    commands.emplace(std::move(lower_cmd), callback);
     return true;
 }
 #endif
@@ -44,8 +44,9 @@ UNREALSDK_CAPI(bool, has_command, const wchar_t* cmd, size_t size);
 #endif
 #ifndef UNREALSDK_IMPORTING
 UNREALSDK_CAPI(bool, has_command, const wchar_t* cmd, size_t size) {
-    const std::wstring_view view{cmd, size};
-    return commands.contains(view);
+    std::wstring lower_cmd(size, '\0');
+    std::transform(cmd, cmd + size, lower_cmd.begin(), &std::towlower);
+    return commands.contains(lower_cmd);
 }
 #endif
 
@@ -58,8 +59,9 @@ UNREALSDK_CAPI(bool, remove_command, const wchar_t* cmd, size_t size);
 #endif
 #ifndef UNREALSDK_IMPORTING
 UNREALSDK_CAPI(bool, remove_command, const wchar_t* cmd, size_t size) {
-    const std::wstring_view view{cmd, size};
-    auto iter = commands.find(view);
+    std::wstring lower_cmd(size, '\0');
+    std::transform(cmd, cmd + size, lower_cmd.begin(), &std::towlower);
+    auto iter = commands.find(lower_cmd);
 
     if (iter == commands.end()) {
         return false;

src/unrealsdk/game/bl2/console.cpp

@@ -45,13 +45,22 @@ const std::wstring SAY_CRASH_FIX_ID = L"unrealsdk_bl2_say_crash_fix";
 // We could combine this with the say bypass, but by keeping them separate it'll let users disable
 // one if they really want to
 const std::wstring CONSOLE_COMMAND_FUNC = L"Engine.Console:ConsoleCommand";
+// This is the actual end point of all console commands, the above function normally calls through
+// into this one - but we needed to hook it to be able to manage the console history. If something
+/// directly calls `PC.ConsoleCommand("my_cmd")`, we need this hook to be able to catch it.
+const std::wstring PC_CONSOLE_COMMAND_FUNC = L"Engine.PlayerController:ConsoleCommand";
+
 const constexpr auto CONSOLE_COMMAND_TYPE = hook_manager::Type::PRE;
 const std::wstring CONSOLE_COMMAND_ID = L"unrealsdk_bl2_console_command";
 
 const std::wstring INJECT_CONSOLE_FUNC = L"WillowGame.WillowGameViewportClient:PostRender";
 const constexpr auto INJECT_CONSOLE_TYPE = hook_manager::Type::PRE;
 const std::wstring INJECT_CONSOLE_ID = L"unrealsdk_bl2_inject_console";
 
+// Would prefer to call a native function where possible, however best I can tell, OutputText is
+// actually implemented directly in unrealscript (along most of the console mechanics).
+BoundFunction console_output_text{};
+
 bool say_bypass_hook(hook_manager::Details& hook) {
     /*
     This is a native function so we don't have exact source, but we expect it's essentially:
@@ -69,6 +78,7 @@ bool say_bypass_hook(hook_manager::Details& hook) {
     static const auto command_property =
         hook.args->type->find_prop_and_validate<UStrProperty>(L"Command"_fn);
 
+    // Since these are different functions, we can't just forward the args struct, have to copy it
     hook.obj->get<UFunction, BoundFunction>(console_command_func)
         .call<void, UStrProperty>(hook.args->get<UStrProperty>(command_property));
     return true;
@@ -201,7 +211,25 @@ bool console_command_hook(hook_manager::Details& hook) {
         hook.obj->get<UFunction, BoundFunction>(save_config_func).call<void>();
     }
 
-    LOG(INFO, L">>> {} <<<", line);
+    /*
+    This is a little awkward.
+    Since we can't let execution though to the unreal function, we're responsible for printing the
+    executed command line.
+
+    We do this via output text directly, rather than the LOG macro, so that it's not affected by the
+    console log level, and so that it happens immediately (the LOG macro is queued, and can get out
+    of order with respect to native engine messages).
+
+    However, for custom console commands it's also nice to see what the command was in the log file,
+    since you'll see all their output too.
+
+    We don't really expose a "write to log file only", since it's not usually something useful, so
+    as a compromise just use the LOG macro on the lowest possible log level, and assume the lowest
+    people practically set their console log level to is dev warning.
+    */
+    auto msg = unrealsdk::fmt::format(L">>> {} <<<", line);
+    console_output_text.call<void, UStrProperty>(msg);
+    LOG(MIN, L"{}", msg);
 
     try {
         callback->operator()(line.c_str(), line.size(), cmd_len);
@@ -212,9 +240,26 @@ bool console_command_hook(hook_manager::Details& hook) {
     return true;
 }
 
-// Would prefer to call a native function where possible, however best I can tell, OutputText is
-// actually implemented directly in unrealscript (along most of the console mechanics).
-BoundFunction console_output_text{};
+bool pc_console_command_hook(hook_manager::Details& hook) {
+    static const auto command_property =
+        hook.args->type->find_prop_and_validate<UStrProperty>(L"Command"_fn);
+
+    auto line = hook.args->get<UStrProperty>(command_property);
+
+    auto [callback, cmd_len] = commands::impl::find_matching_command(line);
+    if (callback == nullptr) {
+        return false;
+    }
+
+    // This hook does not go to console, so there's no extra processing to be done, we can just run
+    // the callback immediately
+    try {
+        callback->operator()(line.c_str(), line.size(), cmd_len);
+    } catch (const std::exception& ex) {
+        LOG(ERROR, "An exception occurred while running a console command: {}", ex.what());
+    }
+    return true;
+}
 
 bool inject_console_hook(hook_manager::Details& hook) {
     hook_manager::remove_hook(INJECT_CONSOLE_FUNC, INJECT_CONSOLE_TYPE, INJECT_CONSOLE_ID);
@@ -247,6 +292,9 @@ void BL2Hook::inject_console(void) {
 
     hook_manager::add_hook(CONSOLE_COMMAND_FUNC, CONSOLE_COMMAND_TYPE, CONSOLE_COMMAND_ID,
                            &console_command_hook);
+    hook_manager::add_hook(PC_CONSOLE_COMMAND_FUNC, CONSOLE_COMMAND_TYPE, CONSOLE_COMMAND_ID,
+                           &pc_console_command_hook);
+
     hook_manager::add_hook(INJECT_CONSOLE_FUNC, INJECT_CONSOLE_TYPE, INJECT_CONSOLE_ID,
                            &inject_console_hook);
 }

src/unrealsdk/game/bl2/hooks.cpp

@@ -3,6 +3,7 @@
 #include "unrealsdk/config.h"
 #include "unrealsdk/game/bl2/bl2.h"
 #include "unrealsdk/hook_manager.h"
+#include "unrealsdk/locks.h"
 #include "unrealsdk/memory.h"
 #include "unrealsdk/unreal/classes/ufunction.h"
 #include "unrealsdk/unreal/classes/uobject.h"
@@ -112,48 +113,33 @@ void __fastcall process_event_hook(UObject* obj,
     process_event_ptr(obj, edx, func, params, null);
 }
 
-std::recursive_mutex process_event_mutex{};
-
 void __fastcall locking_process_event_hook(UObject* obj,
                                            void* edx,
                                            UFunction* func,
                                            void* params,
                                            void* null) {
-    const std::lock_guard<std::recursive_mutex> lock{process_event_mutex};
+    const locks::FunctionCall lock{};
     process_event_hook(obj, edx, func, params, null);
 }
 
 static_assert(std::is_same_v<decltype(&process_event_hook), process_event_func>,
               "process_event signature is incorrect");
 static_assert(std::is_same_v<decltype(&process_event_hook), decltype(&locking_process_event_hook)>,
               "process_event signature is incorrect");
-
-/**
- * @brief Checks if we should use a locking process event implementation.
- *
- * @return True if we should use locks.
- */
-bool locking(void) {
-    // Basically just a function so we can be sure this static is initialized late - LTO hopefully
-    // takes care of it
-    static auto locking = config::get_bool("unrealsdk.locking_process_event").value_or(false);
-    return locking;
-}
-
 }  // namespace
 
 void BL2Hook::hook_process_event(void) {
-    detour(PROCESS_EVENT_SIG, locking() ? locking_process_event_hook : process_event_hook,
+    detour(PROCESS_EVENT_SIG,
+           // If we don't need locks, it's slightly more efficient to detour directly to the
+           // non-locking version
+           locks::FunctionCall::enabled() ? locking_process_event_hook : process_event_hook,
            &process_event_ptr, "ProcessEvent");
 }
 
 void BL2Hook::process_event(UObject* object, UFunction* func, void* params) const {
-    if (locking()) {
-        const std::lock_guard<std::recursive_mutex> lock{process_event_mutex};
-        process_event_hook(object, nullptr, func, params, nullptr);
-    } else {
-        process_event_hook(object, nullptr, func, params, nullptr);
-    }
+    // When we call it manually, always call the locking version, it will pass right through if
+    // locks are disabled
+    locking_process_event_hook(object, nullptr, func, params, nullptr);
 }
 
 namespace {
@@ -244,13 +230,27 @@ void __fastcall call_function_hook(UObject* obj,
 
     call_function_ptr(obj, edx, stack, result, func);
 }
+
+void __fastcall locking_call_function_hook(UObject* obj,
+                                           void* edx,
+                                           FFrame* stack,
+                                           void* result,
+                                           UFunction* func) {
+    const locks::FunctionCall lock{};
+    call_function_hook(obj, edx, stack, result, func);
+}
+
 static_assert(std::is_same_v<decltype(&call_function_hook), call_function_func>,
               "call_function signature is incorrect");
+static_assert(std::is_same_v<decltype(&locking_call_function_hook), call_function_func>,
+              "call_function signature is incorrect");
 
 }  // namespace
 
 void BL2Hook::hook_call_function(void) {
-    detour(CALL_FUNCTION_SIG, call_function_hook, &call_function_ptr, "CallFunction");
+    detour(CALL_FUNCTION_SIG,
+           locks::FunctionCall::enabled() ? locking_call_function_hook : call_function_hook,
+           &call_function_ptr, "CallFunction");
 }
 
 }  // namespace unrealsdk::game

src/unrealsdk/game/bl3/console.cpp

@@ -38,6 +38,22 @@ const constexpr auto MAX_HISTORY_ENTRIES = 50;
 
 UObject* console = nullptr;
 
+void static_uconsole_output_text(const std::wstring& str) {
+    static auto idx = config::get_int("unrealsdk.uconsole_output_text_vf_index")
+                          .value_or(81);  // NOLINT(readability-magic-numbers)
+
+    if (console == nullptr) {
+        return;
+    }
+
+    // UConsole::OutputText does not print anything on a completely empty line - we would prefer to
+    // call UConsole::OutputTextLine, but that got completely inlined
+    // If we have an empty string, replace with with a single newline.
+    static const std::wstring newline = L"\n";
+    TemporaryFString fstr{str.empty() ? newline : str};
+    console->call_virtual_function<void, TemporaryFString*>(idx, &fstr);
+}
+
 using console_command_func = void(UObject* console_obj, UnmanagedFString* raw_line);
 console_command_func* console_command_ptr;
 
@@ -105,7 +121,25 @@ void console_command_hook(UObject* console_obj, UnmanagedFString* raw_line) {
                 // just this, and we'll see if people actually complain
             }
 
-            LOG(INFO, L">>> {} <<<", line);
+            /*
+            This is a little awkward.
+            Since we can't let execution though to the unreal function, we're responsible for
+            printing the executed command line.
+
+            We do this via output text directly, rather than the LOG macro, so that it's not
+            affected by the console log level, and so that it happens immediately (the LOG macro is
+            queued, and can get out of order with respect to native engine messages).
+
+            However, for custom console commands it's also nice to see what the command was in the
+            log file, since you'll see all their output too.
+
+            We don't really expose a "write to log file only", since it's not usually something
+            useful, so as a compromise just use the LOG macro on the lowest possible log level, and
+            assume the lowest people practically set their console log level to is dev warning.
+            */
+            auto msg = unrealsdk::fmt::format(L">>> {} <<<", line);
+            static_uconsole_output_text(msg);
+            LOG(MIN, L"{}", msg);
 
             try {
                 callback->operator()(line.c_str(), line.size(), cmd_len);
@@ -193,19 +227,7 @@ void BL3Hook::inject_console(void) {
 }
 
 void BL3Hook::uconsole_output_text(const std::wstring& str) const {
-    static auto idx = config::get_int("unrealsdk.uconsole_output_text_vf_index")
-                          .value_or(81);  // NOLINT(readability-magic-numbers)
-
-    if (console == nullptr) {
-        return;
-    }
-
-    // UConsole::OutputText does not print anything on a completely empty line - we would prefer to
-    // call UConsole::OutputTextLine, but that got completely inlined
-    // If we have an empty string, replace with with a single newline.
-    static const std::wstring newline = L"\n";
-    TemporaryFString fstr{str.empty() ? newline : str};
-    console->call_virtual_function<void, TemporaryFString*>(idx, &fstr);
+    static_uconsole_output_text(str);
 }
 
 bool BL3Hook::is_console_ready(void) const {