YaLTeR / BunnymodXT

Speedrun and TAS tool for Half-Life & friends.
Other
200 stars 37 forks source link

Sort of refactoring #437

Closed SmileyAG closed 8 months ago

SmileyAG commented 1 year ago

I am writing this for the future so as not to forget, well, or if anyone has more motivation to do it faster than me That list is starting from important changes to less significant ones Some of the goals may have already been done, but I specifically leave them on the list as an option if someone does not follow to it

Replacing patterns with hooking from structs

I will give you all the functions and variables from the current BXT code that can be easily replaced with calls from a struct instead of using patterns:

Got wroted a simple macro to hook functions through structs:

        #define GET_FUTURE_FROM_STRUCT(future_name, engtype, engfunc) \
            if (engtype) { \
                ORIG_##future_name = engtype->engfunc; \
            } if (ORIG_##future_name) { \
                EngineDevMsg("[hw dll] Found " #future_name " at %p (using the engine struct).\n", ORIG_##future_name); \
            } else { \
                EngineDevWarning("[hw dll] Could not find " #future_name ".\n"); \
            }

Also I heard from someone that functions which tied to commands (such as notarget, noclip, stop, changelevel2, record, reload and etc) possible to hook through some linked list instead of using patterns too, but I didn't figured for now how it exactly to do for BXT code.

Defining all functions with macros

Macros are actually used for defining functions in BXT, but not as much as I would like

We could easily reduce ~300-500 lines at least if we used macros for defining functions everywhere, It would also reduce the cases of incorrect from manually defining (example: https://github.com/YaLTeR/BunnymodXT/pull/427/files#diff-b58f1c7e435bd309aec7753e367c8b66be053568295603136aa146254bd04f46L1230-R1229) Also, don't forget to keep the same defining order as it was before, because it can cause issues with the code if order is swapped (e.g. look to is_czeror variable in ServerDLL.cpp)

Renaming both pEngfuncs to engClient and engServer

Firstly, it would look more aesthetically pleasing, and secondly, less than a year ago I made a request where we started to looking for access to cl_enginefunc_t and enginefuncs_t in the engine (which allowed to use the functions from these structs in all GoldSrc games and mods!), and so it would be more correct to move these variables to HwDLL too Also, don't forget to remove from ClientDLL.cpp and ServerDLL.cpp those codes

https://github.com/YaLTeR/BunnymodXT/blob/13f984c63ce5c0c45664aa1687efe1675ddf338b/BunnymodXT/modules/ClientDLL.cpp#L543-L547 https://github.com/YaLTeR/BunnymodXT/blob/13f984c63ce5c0c45664aa1687efe1675ddf338b/BunnymodXT/modules/ServerDLL.cpp#L1554-L1558

And replace them with that code in HwDLL.cpp file to make it hooking on Linux:

        engClient = reinterpret_cast<cl_enginefunc_t*>(MemUtils::GetSymbolAddress(m_Handle, "cl_enginefuncs"));
        if (engClient)
            EngineDevMsg("[hw dll] Found cl_enginefuncs [Linux] at %p.\n", engClient);
        else
            EngineDevWarning("[hw dll] Could not find cl_enginefuncs [Linux].\n");

        engServer = reinterpret_cast<enginefuncs_t*>(MemUtils::GetSymbolAddress(m_Handle, "g_engfuncsExportedToDlls"));
        if (engServer)
            EngineDevMsg("[hw dll] Found g_engfuncsExportedToDlls [Linux] at %p.\n", engServer);
        else
            EngineDevWarning("[hw dll] Could not find g_engfuncsExportedToDlls [Linux].\n");

Optimize everywhere method of finding functions through the others one

Example of how it should look right:

https://github.com/YaLTeR/BunnymodXT/blob/13f984c63ce5c0c45664aa1687efe1675ddf338b/BunnymodXT/modules/ClientDLL.cpp#L459-L496 https://github.com/YaLTeR/BunnymodXT/blob/13f984c63ce5c0c45664aa1687efe1675ddf338b/BunnymodXT/modules/ClientDLL.cpp#L697-L701

Reduce the number of hardcoded offsets in favor of automatic finding

An example of a pull request in which this was done: https://github.com/YaLTeR/BunnymodXT/pull/432

Use functions/variables from the engine instead of the client/server if possible

This allows you to support things for all GoldSrc mods, since the number of engines is limited due to its closed code from Valve, at the same time there are thousands of different client/server dlls

Good examples of this: https://github.com/YaLTeR/BunnymodXT/pull/286 and https://github.com/YaLTeR/BunnymodXT/pull/321/commits/a9bd88847ee12bf08459a019b10b4c57c92adf78

YaLTeR commented 1 year ago

So, two reasons why patterns might be preferable:

Also I heard from someone that functions which tied to commands (such as notarget, noclip, stop, changelevel2, record, reload and etc) possible to hook through some linked list instead of using patterns too, but I didn't figured for now how it exactly to do for BXT code.

Yeah, in theory you can find the console command linked list and find the pointers there, but the same caveats apply, plus you need to find them at some point after the respective commands have been registered with the engine.

SmileyAG commented 1 year ago

Those structs are incomplete in some older engine versions and might be different in some mod engines (CoF?).

All examples of hooks through structs that I gave above is exist in engine builds starting at least from 1712, so this is not a issue

Also I have looked to engine structs and their sizes in Cry of Fear and Sven Co-op engines and can say they are mostly compatible with GoldSrc interfaces (this also applies to cl_enginefunc_t/enginefuncs_t)

If in Steam version of Cry of Fear can be noted entvars_t as a significant change, then in Sven Co-op most of the structs is same as in GoldSrc (already supported most features of BXT to Sven Co-op in my local BXT build a months ago for experiment, probably I'll push it in some day to here too)

The only versions of GoldSrc engine that differs in all structures from GoldSrc and that definitely cannot be supported to BXT repo:

UPD (29.03.2023): Alright, seems that in engine versions that uses HLSDK 1.0 the structs a bit differs in comparing to HLSDK 2.0+ and newer, so the hooking functions by structs is not good idea to be honest, so we will have to keep the hooking by patterns for some engine functions even if someone not like it

SmileyAG commented 8 months ago

Closing that issue in favor to split it for multiple issues for easier sort and discuss per issue.