Improve API, readability, and remove unused or duplicate functions by Metalit · Pull Request #57 · QuestPackageManager/beatsaber-hook

Metalit · 2026-03-22T04:44:37Z

Changes:

Removes folder structure since almost everything was in utils anyway
Renames main namespace from il2cpp_utils to i2c for faster typing
Made variables, structs, and functions consistently snake_case, except for those that represent C# objects such as ArrayW
Moved code around to (in my opinion) more sensibly grouped files
Updated and ran auto formatting
Streamlined hooking API:
- Only two macros exist - MAKE_HOOK and MAKE_HOOK_MATCH
- MAKE_HOOK_MATCH is unchanged, except it now resolves overloaded functions automatically
- MAKE_HOOK uses a parenthesized list in its second parameter to allow the functionality of all previous macros, except those with compile time type checking (just use MAKE_HOOK_MATCH)
- Added ways to access more of flamingo's api, such as uninstalling
Massively reduced methods (and fields/properties) API
- Removed multiplicative function overloads by using find_x_info structs with overloaded constructors
- Used those constructors to allow the combination of find and get/set/run calls in the same expression
- Removed type checking parameter, but still allowed it to be disabled by splitting the find and run calls
- Added explicit template arguments used for type checking and automatically creating generic instantiations
- Runs methods through methodPointer invokes, instead of runtime_invoke
- Replaced throwing, optional, and custom result type overloads with only throwing (but may reintroduce the result option if desired, by detecting it as the return type instead of doubling the number of methods)
Improved method cache lookup performance
Removed null checks in every il2cpp API call
Replaced SafePtrUnity with a switch in safe_ptr (automatically set when using cordl)
Fixed ListW for use
Consolidated and slightly improved ArrayW and StringW
Removed unused or outdated APIs, most notably:
- Catch wrapper
- Configuration struct (config file path is still available)
- Delegates
- Support for lacking exceptions or concepts
- arg_class and arg_type
- Result type (see note at end of methods API changes)

Still TODO:

Get reviewed by others, obviously
Actually run the new tests, most importantly if methods that throw in C# can be handled well
Look at the older PR for flamingo support in hooks and merge what I can into this

Sorry about the un-reviewability btw, but between the formatting changes, code moving, file renaming, snake casing, and simply the only way I could efficiently manage to actually get this code written, it kinda had to be this way. Hopefully the list above helps.

Fernthedev · 2026-03-22T17:00:57Z

Replaced throwing, optional, and custom result type overloads with only throwing (but may reintroduce the result option if desired, by detecting it as the return type instead of doubling the number of methods)

I'm not sure about the methods being throw by default. If you really reduced the find/run calls, then having RunMethodRethrow, RunMethodOpt and RunMethod functions doesn't seem excessive to me. Especially in the case where you're not interested in bubbling up exceptions, which you can use noexcept for to improve codegen too.

Removed Delegates

Delegates being removed too is a bit risky as custom-types isn't supported by default and, as far as I know, does not require xrefs in bs-hooks to support unlike custom-types.

Removed result type

The result type was confusing to use in its original design but if we moved to something more like std::expected or a better Result type I'd like to still offer it in bs-hooks to avoid try/catch ergonomics even if bs-hooks itself doesn't use it.

Support for disabling exceptions or concepts

I would also like to add noexcept or @throws where possible to make error handling a little easier in the long run, which goes alongside maybe using the Result type too.

Streamlined hooking API:

You would likely need to rebase this PR onto my draft flamingo work since the hooking API is going to be different moving forward, especially given we will be allowing uninstalls too. The HOOK_AFTER and HOOK_BEFORE macros would not fit with the PR's design as they work with static initialization instead of the INSTALL_HOOK call, at least that's what I gathered.

Other things we could explore: (if not already explored/discussed)

Making GC xrefs an optional feature
Using C++ 20 concepts, span and string_view where ergonomic
Documenting migration patterns e.g for RunMethod or Hook transformations. We could leverage LLM-style prompts to allow developers to migrate codebases without too much friction. (yes yes I know, AI)

sc2ad

Haven't made it to the meat yet, I'll leave these comments for now as I go slep

sc2ad · 2026-04-02T06:25:07Z

+    /// This is VERY DANGEROUS (and NOT THREAD SAFE!) and may cause all sorts of race conditions. Use at your own risk.
+    /// @param size The size to allocate the unsafe object with.
+    /// @return The returned GC-allocated instance.
+    [[deprecated("DO NOT USE")]] void* __allocate_unsafe(std::size_t size);


I think this was only done for delegates, so maybe can be killed? It might be used in some form for pinned allocations tho

sc2ad · 2026-04-02T06:32:16Z

+    struct func_wrapper<R (*)(TArgs...)> : abstract_func<R(void*, TArgs...)> {
+        func_wrapper(auto&& f) : held(f) {}
+
+        R operator()(TArgs... args) const noexcept override {


Only no except if held is noexcept

sc2ad · 2026-04-02T06:33:04Z

+                void* data;
+            };
+            dat d{.wrapper = held};
+            return d.data;


nit: can use bitcast here instead of union trick

sc2ad · 2026-04-02T06:34:40Z

+
+    template <typename R, typename T, typename... TArgs>
+    bool operator<(abstract_func<R(T*, TArgs...)> const& a, abstract_func<R(T*, TArgs...)> const& b) {
+        return a.ptr() < b.ptr();


Is this for ordered sets? This operation is inherently wack without conversion to an int anyways (since tag byte of pointer could ruin the pointer comparison)

sc2ad · 2026-04-02T06:41:08Z

+#define CONFIG_PATH_FORMAT "/sdcard/ModData/{}/Configs/"
+#endif
+
+#define BS_HOOK_HIDDEN __attribute__((visibility("hidden")))


I don't really know why these need to be bs-hook scoped (could be an old scad mistake) unless they are also conditioned on ifndef

sc2ad · 2026-04-02T06:42:02Z

@@ -0,0 +1,37 @@
+#pragma once


This file feels more like "type debug" rather than strictly "debug"

Fernthedev · 2026-04-05T04:43:58Z

        struct BS_HOOK_HIDDEN no_arg_type {
-            static inline Il2CppType const* get() { return &no_arg_class<T>::get()->byval_arg; }
+            static inline Il2CppType const* get() {
+                auto klass = no_arg_class<T>::get();


is there any reason we don't store this in static storage? It would increase binary size but it'd be a useful optimization

It's because the return value of no_arg_class::get() should always already be in static storage, and I don't think caching the pointer access would be a useful optimization but correct me if I'm wrong

Fernthedev · 2026-04-05T04:46:20Z

+#include "types.hpp"
+
+template <size_t S, i2c::str_lit Namespace, i2c::str_lit Name>
+struct ValueW {


it'd be nice to add docs explaining what this type is or is used for

ditto, is this meant to wrap an arbitrary object? If so, using an array of bytes is fundamentally flawed. Is it meant to wrap an arbitrary value type? In which case an array of bytes of proper storage size CAN be legal, but is still strange. If it's meant to "look" like the provided object, like for my hacky manual override, it's a scary concept but could work (though the size needs to be very carefully selected)

Added some docs, but basically the second/third option (not sure what the distinction is). Still open for discussion, but the main case I imagine for this is for a value type as a parameter to a hook or function where the data is entirely or almost entirely unused.

Fernthedev · 2026-04-05T04:48:26Z

+// Creates all directories for a provided file path
+// Ex: /sdcard/Android/data/something/files/libs/
+int mkpath(std::string_view file_path);
+// Reads all of the text of a file at the given filename. If the file does not exist, returns an empty string.


do we need to provide these filesystem utilities? Unless the std library does not offer a cross-platform solution I'm not sure this is necesary

I just kept them around because they can be more convenient than std in some cases, but they could definitely be removed

sc2ad

Made it to the tests, but I'm going to bed as well. Rest will be reviewed in due time (I hope)

sc2ad · 2026-04-05T07:16:22Z

+        };
+
+        // template <typename T>
+        // struct BS_HOOK_HIDDEN arg_type<T&> {


w/o this, what is the intended way of resolving an object passed as a parameter to its actual type?

Ex, w/o cordl, if I use Il2CppObject* to hold some object B which inherits A, I want to ensure that I overload resolve using B instead of A or object.

This case is handled in extract_type (now in the same file) and find_class_info and is explicitly only done for Il2CppObject*, with the reasoning that if a more specific type is used, it should be accurate.
It can lead to a slightly strange case though where you cast from A to Il2CppObject* in order to get B because you don't know exactly what B is - not sure how to improve this.

sc2ad · 2026-04-05T07:23:37Z

+#include "types.hpp"
+
+template <size_t S, i2c::str_lit Namespace, i2c::str_lit Name>
+struct ValueW {


ditto, is this meant to wrap an arbitrary object? If so, using an array of bytes is fundamentally flawed. Is it meant to wrap an arbitrary value type? In which case an array of bytes of proper storage size CAN be legal, but is still strange. If it's meant to "look" like the provided object, like for my hacky manual override, it's a scary concept but could work (though the size needs to be very carefully selected)

Metalit added 5 commits March 20, 2026 19:10

Rewrite yay

e983774

Fix tests (some ai generation) and compilation

d33338a

Add hook test

9c38b9a

Fix some other errors

332e31d

Fix r29 and span annoyances

df0a312

Metalit added 2 commits March 31, 2026 17:20

Sprinkle in noexcept

8bd7be8

Revert debug change

baa9b5e

sc2ad reviewed Apr 2, 2026

View reviewed changes

Metalit added 21 commits April 2, 2026 21:33

Keep r28 compat

621bb06

Fix type marking bugs

0a9d4cc

Add helper for generic methods

efef301

Fix method running bugs

05da159

Fix concerning typo

687df5f

Fix tests

ab580e2

Fix valuew

5107c17

Restore stack trace exception

7b71066

Remove source location actually

4b58150

I don't wanna have a reverse type checking flag

2f2dba0

Fix generic classes

42b1c17

Fix method finds

61505a3

Update README

ae4968f

Small safety

68787e0

Fix test

ea95c02

Return potentially useful log

85be1f2

Fix uninitialized field

d5b9eb3

Add generics to log method

95bd94f

Fix size type

4dfa16c

Fix accidental copy

d79d2f0

Document other defines

d6ba29f

Fix typo

c4d612a

Metalit marked this pull request as ready for review April 4, 2026 06:33

Metalit added 2 commits April 4, 2026 15:27

Fix conversion check order

9fe5f91

Fix void* test

88d7d9d

Fernthedev reviewed Apr 5, 2026

View reviewed changes

Comment thread src/tests/types.cpp

Fernthedev reviewed Apr 5, 2026

View reviewed changes

Fix cast functions

739c9a3

sc2ad reviewed Apr 5, 2026

View reviewed changes

Metalit added 18 commits April 5, 2026 14:02

Remove unnecessary decay_t

4ae9d5a

Lazier instance conversion

64be0c6

Change concept to bool

56f5964

Document ValueW and disincentivize inheritance

3d9168a

Move ctors for clarity

e8bebe7

Better IWYU

ab0c2f6

Move functions to more sensible file

65d7ff9

Remove commented code

2ec60ee

Reorganize members with detail namespace

8d6d3f3

Change pair to more readable enum

a990fc6

Clarify type

b7f51e0

Allow method info in hooks

82e818b

Fixes for cordl

63253ce

Allow result type

0815930

Format api

692ac5f

Idk why I did this

3b9f140

Update cast and noexcept with result

a9e21eb

Fix build

00e8c65

Conversation

Metalit commented Mar 22, 2026

Uh oh!

Fernthedev commented Mar 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sc2ad left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sc2ad left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fernthedev commented Mar 22, 2026 •

edited

Loading