Sort hooks and callbacks by mod order, with return hooks in reverse order (#115)

Author: Mr-Wiseguy

librecomp/include/librecomp/mods.hpp

@@ -345,6 +345,7 @@ namespace recomp {
             std::string get_mod_id_from_filename(const std::filesystem::path& mod_filename) const;
             std::filesystem::path get_mod_filename(const std::string& mod_id) const;
             size_t get_mod_order_index(const std::string& mod_id) const;
+            size_t get_mod_order_index(size_t mod_index) const;
             std::optional<ModDetails> get_details_for_mod(const std::string& mod_id) const;
             std::vector<ModDetails> get_all_mod_details(const std::string& mod_game_id);
             recomp::Version get_mod_version(size_t mod_index);
@@ -378,6 +379,7 @@ namespace recomp {
                 const std::unordered_map<recomp_func_t*, overlays::BasePatchedFunction>& base_patched_funcs,
                 std::span<const uint8_t> decompressed_rom);
             void dirty_mod_configuration_thread_process();
+            void rebuild_mod_order_lookup();
 
             static void on_code_mod_enabled(ModContext& context, const ModHandle& mod);
 
@@ -389,7 +391,8 @@ namespace recomp {
             std::vector<ModHandle> opened_mods;
             std::unordered_map<std::string, size_t> opened_mods_by_id;
             std::unordered_map<std::filesystem::path::string_type, size_t> opened_mods_by_filename;
-            std::vector<size_t> opened_mods_order;
+            std::vector<size_t> opened_mods_order; // order index -> mod index
+            std::vector<size_t> mod_order_lookup; // mod index -> order index
             std::mutex opened_mods_mutex;
             std::unordered_set<std::string> mod_ids;
             std::unordered_set<std::string> enabled_mods;
@@ -579,11 +582,14 @@ namespace recomp {
         };
 
         void setup_events(size_t num_events);
-        void register_event_callback(size_t event_index, GenericFunction callback);
+        void register_event_callback(size_t event_index, size_t mod_index, GenericFunction callback);
         void reset_events();
 
         void setup_hooks(size_t num_hook_slots);
-        void register_hook(size_t hook_slot_index, GenericFunction callback);
+        void set_hook_type(size_t hook_slot_index, bool is_return_hook);
+        void register_hook(size_t hook_slot_index, size_t mod_index, GenericFunction callback);
+        void finish_event_setup(const ModContext& context);
+        void finish_hook_setup(const ModContext& context);
         void reset_hooks();
         void run_hook(uint8_t* rdram, recomp_context* ctx, size_t hook_slot_index);
 
@@ -611,6 +617,7 @@ namespace recomp {
         std::string get_mod_id_from_filename(const std::filesystem::path& mod_filename);
         std::filesystem::path get_mod_filename(const std::string& mod_id);
         size_t get_mod_order_index(const std::string& mod_id);
+        size_t get_mod_order_index(size_t mod_index);
         ModContentTypeId register_mod_content_type(const ModContentType& type);
         bool register_mod_container_type(const std::string& extension, const std::vector<ModContentTypeId>& content_types, bool requires_manifest);
 

librecomp/src/mod_events.cpp

@@ -8,8 +8,13 @@ struct overloaded : Ts... { using Ts::operator()...; };
 template<class... Ts>
 overloaded(Ts...) -> overloaded<Ts...>;
 
+struct EventCallback {
+    size_t mod_index;
+    recomp::mods::GenericFunction func;
+};
+
 // Vector of callbacks for each registered event.
-std::vector<std::vector<recomp::mods::GenericFunction>> event_callbacks{};
+std::vector<std::vector<EventCallback>> event_callbacks{};
 
 extern "C" {
     // This can stay at 0 since the base events are always first in the list.
@@ -29,14 +34,14 @@ extern "C" void recomp_trigger_event(uint8_t* rdram, recomp_context* ctx, uint32
     recomp_context initial_context = *ctx;
 
     // Call every callback attached to the event.
-    const std::vector<recomp::mods::GenericFunction>& callbacks = event_callbacks[event_index];
-    for (recomp::mods::GenericFunction func : callbacks) {
+    const std::vector<EventCallback>& callbacks = event_callbacks[event_index];
+    for (const EventCallback& callback : callbacks) {
         // Run the callback.
         std::visit(overloaded {
             [rdram, ctx](recomp_func_t* native_func) {
                 native_func(rdram, ctx);
             },
-        }, func);
+        }, callback.func);
 
         // Restore the original context.
         *ctx = initial_context;
@@ -47,8 +52,19 @@ void recomp::mods::setup_events(size_t num_events) {
     event_callbacks.resize(num_events);
 }
 
-void recomp::mods::register_event_callback(size_t event_index, GenericFunction callback) {
-    event_callbacks[event_index].emplace_back(callback);
+void recomp::mods::register_event_callback(size_t event_index, size_t mod_index, GenericFunction callback) {
+    event_callbacks[event_index].emplace_back(EventCallback{ mod_index, callback });
+}
+
+void recomp::mods::finish_event_setup(const ModContext& context) {
+    // Sort callbacks by mod order.
+    for (std::vector<EventCallback>& cur_entry : event_callbacks) {
+        std::sort(cur_entry.begin(), cur_entry.end(), 
+            [&context](const EventCallback& lhs, const EventCallback& rhs) {
+                return context.get_mod_order_index(lhs.mod_index) < context.get_mod_order_index(rhs.mod_index);
+            }
+        );
+    }
 }
 
 void recomp::mods::reset_events() {

librecomp/src/mod_hooks.cpp

@@ -8,8 +8,18 @@ struct overloaded : Ts... { using Ts::operator()...; };
 template<class... Ts>
 overloaded(Ts...) -> overloaded<Ts...>;
 
+struct HookEntry {
+    size_t mod_index;
+    recomp::mods::GenericFunction func;
+};
+
+struct HookTableEntry {
+    std::vector<HookEntry> hooks;
+    bool is_return_hook;
+};
+
 // Vector of individual hooks for each hook slot.
-std::vector<std::vector<recomp::mods::GenericFunction>> hook_table{};
+std::vector<HookTableEntry> hook_table{};
 
 void recomp::mods::run_hook(uint8_t* rdram, recomp_context* ctx, size_t hook_slot_index) {
     // Sanity check the hook slot index.
@@ -24,14 +34,14 @@ void recomp::mods::run_hook(uint8_t* rdram, recomp_context* ctx, size_t hook_slo
     recomp_context initial_context = *ctx;
 
     // Call every hook attached to the hook slot.
-    const std::vector<recomp::mods::GenericFunction>& hooks = hook_table[hook_slot_index];
-    for (recomp::mods::GenericFunction func : hooks) {
+    const std::vector<HookEntry>& hooks = hook_table[hook_slot_index].hooks;
+    for (HookEntry hook : hooks) {
         // Run the hook.
         std::visit(overloaded {
             [rdram, ctx](recomp_func_t* native_func) {
                 native_func(rdram, ctx);
             },
-        }, func);
+        }, hook.func);
 
         // Restore the original context.
         *ctx = initial_context;
@@ -42,8 +52,34 @@ void recomp::mods::setup_hooks(size_t num_hook_slots) {
     hook_table.resize(num_hook_slots);
 }
 
-void recomp::mods::register_hook(size_t hook_slot_index, GenericFunction callback) {
-    hook_table[hook_slot_index].emplace_back(callback);
+void recomp::mods::set_hook_type(size_t hook_slot_index, bool is_return) {
+    hook_table[hook_slot_index].is_return_hook = is_return;
+}
+
+void recomp::mods::register_hook(size_t hook_slot_index, size_t mod_index, GenericFunction callback) {
+    hook_table[hook_slot_index].hooks.emplace_back(HookEntry{ mod_index, callback });
+}
+
+void recomp::mods::finish_hook_setup(const ModContext& context) {
+    // Sort hooks by mod order (and return hooks in reverse order).
+    for (HookTableEntry& cur_entry : hook_table) {
+        // Reverse sort if this slot is a return hook.
+        if (cur_entry.is_return_hook) {
+            std::sort(cur_entry.hooks.begin(), cur_entry.hooks.end(), 
+                [&context](const HookEntry& lhs, const HookEntry& rhs) {
+                    return context.get_mod_order_index(lhs.mod_index) > context.get_mod_order_index(rhs.mod_index);
+                }
+            );
+        }
+        // Otherwise sort normally.
+        else {
+            std::sort(cur_entry.hooks.begin(), cur_entry.hooks.end(), 
+                [&context](const HookEntry& lhs, const HookEntry& rhs) {
+                    return context.get_mod_order_index(lhs.mod_index) < context.get_mod_order_index(rhs.mod_index);
+                }
+            );
+        }
+    }
 }
 
 void recomp::mods::reset_hooks() {

librecomp/src/mods.cpp

@@ -131,7 +131,7 @@ class recomp::mods::DynamicLibrary {
 
     template <typename T>
     bool get_dll_symbol(T& out, const char* name) const {
-        out = (T)GetProcAddress(native_handle, name);
+        out = (T)(void*)GetProcAddress(native_handle, name);
         if (out == nullptr) {
             return false;
         }
@@ -638,6 +638,7 @@ void recomp::mods::ModContext::close_mods() {
     opened_mods_by_filename.clear();
     opened_mods.clear();
     opened_mods_order.clear();
+    mod_order_lookup.clear();
     mod_ids.clear();
     enabled_mods.clear();
     auto_enabled_mods.clear();
@@ -877,6 +878,8 @@ void recomp::mods::ModContext::load_mods_config() {
         return sort_order[i] < sort_order[j];
     });
 
+    rebuild_mod_order_lookup();
+
     // Enable mods that are specified in the configuration or mods that are considered new.
     for (size_t i = 0; i < opened_mods.size(); i++) {
         const ModHandle& mod = opened_mods[i];
@@ -889,6 +892,18 @@ void recomp::mods::ModContext::load_mods_config() {
     }
 }
 
+void recomp::mods::ModContext::rebuild_mod_order_lookup() {
+    // Initialize the mod order lookup to all -1 so that mods that aren't enabled have an order index of -1.
+    mod_order_lookup.resize(opened_mods.size());
+    std::fill(mod_order_lookup.begin(), mod_order_lookup.end(), static_cast<size_t>(-1));
+
+    // Build the lookup of mod index to mod order by inverting the opened mods order list.
+    for (size_t mod_order_index = 0; mod_order_index < opened_mods_order.size(); mod_order_index++) {
+        size_t mod_index = opened_mods_order[mod_order_index];
+        mod_order_lookup[mod_index] = mod_order_index;
+    }
+}
+
 recomp::mods::ModContext::ModContext() {
     // Register the code content type.
     ModContentType code_content_type {
@@ -1128,14 +1143,18 @@ size_t recomp::mods::ModContext::get_mod_order_index(const std::string& mod_id)
         return static_cast<size_t>(-1);
     }
 
-    // TODO keep a mapping of mod index to mod order index to prevent needing a lookup here.
-    auto find_order_it = std::find(opened_mods_order.begin(), opened_mods_order.end(), find_it->second);
-    if (find_order_it == opened_mods_order.end()) {
+    return get_mod_order_index(find_it->second);
+}
+
+size_t recomp::mods::ModContext::get_mod_order_index(size_t mod_index) const {
+    size_t order_index = mod_order_lookup[mod_index];
+    // Check if the mod has a proper order index and assert if it doesn't, as that means the mod isn't actually loaded.
+    if (order_index == static_cast<size_t>(-1)) {
         assert(false);
         return static_cast<size_t>(-1);
     }
 
-    return find_order_it - opened_mods_order.begin();
+    return order_index;
 }
 
 std::optional<recomp::mods::ModDetails> recomp::mods::ModContext::get_details_for_mod(const std::string& mod_id) const {
@@ -1347,6 +1366,8 @@ void recomp::mods::ModContext::set_mod_index(const std::string &mod_game_id, con
         opened_mods_order.push_back(mod_index);
     }
 
+    rebuild_mod_order_lookup();
+
     for (ModContentTypeId type_id : opened_mods[mod_index].content_types) {
         content_reordered_callback* callback = content_types[type_id.value].on_reordered;
         if (callback) {
@@ -1655,12 +1676,13 @@ std::vector<recomp::mods::ModLoadErrorDetails> recomp::mods::ModContext::load_mo
 
     // Regenerate any remaining hook slots that weren't handled during mod recompilation.
 
-    // List of unprocessed hooks and their hook index.
+    // List of unprocessed hooks and their hook index. Also set up which hooks are return hooks.
     std::vector<std::pair<recomp::mods::HookDefinition, size_t>> unprocessed_hooks;
     for (const auto& [def, index] : hook_slots) {
         if (!processed_hook_slots[index]) {
             unprocessed_hooks.emplace_back(std::make_pair(def, index));
         }
+        recomp::mods::set_hook_type(index, def.at_return);
     }
 
     if (!unprocessed_hooks.empty()) {
@@ -1685,6 +1707,9 @@ std::vector<recomp::mods::ModLoadErrorDetails> recomp::mods::ModContext::load_mo
         }
     }
 
+    finish_event_setup(*this);
+    finish_hook_setup(*this);
+
     active_game = mod_game_index;
     return ret;
 }
@@ -2389,7 +2414,7 @@ recomp::mods::CodeModLoadError recomp::mods::ModContext::resolve_code_dependenci
             return CodeModLoadError::InvalidCallbackEvent;
         }
 
-        recomp::mods::register_event_callback(event_index, func);
+        recomp::mods::register_event_callback(event_index, mod_index, func);
     }
 
     // Register hooks.
@@ -2411,7 +2436,7 @@ recomp::mods::CodeModLoadError recomp::mods::ModContext::resolve_code_dependenci
 
         // Register the function handle for this hook slot.
         GenericFunction func = mod.code_handle->get_function_handle(cur_hook.func_index);
-        recomp::mods::register_hook(find_it->second, func);
+        recomp::mods::register_hook(find_it->second, mod_index, func);
     }
 
     // Populate the relocated section addresses for the mod.

librecomp/src/recomp.cpp

@@ -585,6 +585,11 @@ size_t recomp::mods::get_mod_order_index(const std::string& mod_id) {
     return mod_context->get_mod_order_index(mod_id);
 }
 
+size_t recomp::mods::get_mod_order_index(size_t mod_index) {
+    std::lock_guard lock { mod_context_mutex };
+    return mod_context->get_mod_order_index(mod_index);
+}
+
 std::optional<recomp::mods::ModDetails> recomp::mods::get_details_for_mod(const std::string& mod_id) {
     std::lock_guard lock { mod_context_mutex };
     return mod_context->get_details_for_mod(mod_id);