AntiLag rework

[L3D451R7]

This PR entirely reworks existing AntiLag, adding optimized mode and debug logs.

AntiLag was rebuilt, it was debugged and some of the issues were fixed with optimized mode.
I also added back IW3 behavior of AntiLag when its disabled adding optimization too.

As bug example - the throwables were calling AntiLag's rewind even after they stuck in/hit the ground, I decided to keep that optimization with g_antilag 1 since it's obviously a bug.

With optimized mode (g_antilag 2) I also disable AntiLag's rewind on teammates. (I did not find scr_FriendlyFire there so its just calling ingame function)

Also I added attacker in arguments of G_AntiLagRewindClientPos which required rebuilding Weapon_Melee, Bullet_Fire and G_RunMissile.

Throwables issue visualization:

2024-11-12.23-57-01.mp4

After fix:

2024-11-13.00-15-45.mp4

[diamante0018]

Here are a few things that can be improved in the code

remove __cdecl from function definitions

remove return statements on void functions

See other comments

[mxve]

Played a few rounds, 2 players on a dedicated server with g_antilag set to 2. Doesn't seem to break anything, at least at low ping.

[Rackover]

This is just a preliminary review, I need to check every hook address and we'll probably have a bit of back and forth for the stubs to be rewritten (as diamante explained), but I like where this is going 😃

[Rackover]

I'm gonna side with Diamante on this, it's a bit nasty to have a C funcution with uintptr_t stack and return address and have it perform operations like that instead of a ASM stub - as in, it's highly unusual here.
I'd rather not have the C code perform low level stuff like this and that this is broken down into an ASM stub + a C/C++ function.
I'll look further into this later so I can give you more precise suggestions if you need it

[L3D451R7]

alright i went other route, had some problems with making __declspec(naked) functions so i went over inline asm in the Bullet_Fire func.

[Rackover]

In addition to the comments, while testing I got a hang while rotating to devmap mp_rust after playing on afghan with bots. I could not reproduce it, although it's a bit concerning it might be my set up. I'll keep an eye open in case it happens again (i'll attach a debugger next time)

[Rackover]

Since these addresses are never going to change, i think I'd rather have this Dvar (and the missile debug one too) specified as private members of the class, probably static, and grabbed in the constructor or in the declaration or something like that - rather than in the function's body whenever used

[Rackover]

Really not a fan of ASM blocks standing in the middle of an otherwise C++ function body!
Same goes for the other one - please make these either separate functions or, if that's possible, express that code in C/C++ form instead.
Reading this it is not immediatly obvious to me what it does - assembly is for the processor, not for humans, so let's keep that codebase as human as we can

[Rackover]

If we don't need it, we can remove it

[Rackover]

So the jump hook was replaced with, hooking every call of that function, and (if I'm correct) reimplementing it pretty much completely - I'm not 100% convinced by this approach for a few reasons:

• Reimplementing a lot of code from the game means adding a lot of unnecessary code in IW4x (the game already does it)
• The original "Bullet_Fire" now becomes a trap function that should never be called, and this one should be called instead, which also means that if somebody naively hooks into any sub-part of Bullet_Fire from the engine their code won't be called as expected, since the entire Game function has become unused
• In general, while re-implementing something the game already does it's easy to introduce a bug accidentally (or a side effect) that the game didn't have, especially when touching something as core as "Bullet_Fire". Right now I don't see any, but it's easy enough to introduce at a later point if this is refactored one day and treated as "code from IW4x" and not, code from IW4 that was rewritten completely.

For all these reasons I'd really be in favour of using the existing Bullet_Fire function and plugging the antilag in it whenever you need it, writing a few stubs if necessary, rather than replacing the whole "chunk" of it with our version.

Others can weight in on this of course, maybe there's something I'm not considering properly here 🙂

[Laupetin]

Without having a lot of context on whether rewriting a function here would make sense or not, there are a couple of advantages of rewriting a function instead of hooking it in the middle of its execution that might be worth considering:

• Hooking in the middle of a function is a lot more complex and increases the risk of making a mistake. I can remember fixing a bunch of these "random crashes" issues a few years back for example (patches from 4d1) that did exactly that, have been in some clients for ages and just had a minor mistake in the asm that most of the time did not lead to a crash but just then sometimes did on a map rotate for example. With things like jumps and tests you might destroy assertions that the compiler did about the state of flags, even when you perfectly recreate the registers state. Compared to that, the ABI between function calls is often a lot more lenient and easier to understand.

• Recreating a function gives you the context in which your modification is called. Reviewing code that is called within a function requires you to more closely and more often lookup the iw4 binary and is therefore harder and takes more time. This leads to sometimes patches not being examined anymore in the future (do we still need this? does this still do, what it is supposed to?) and avoided to be refactored.

• More complicated modifications (like moderate modifications to the execution flow) or multiple patches for multiple usecases often require multiple hooks inside the same function, increasing the effort of making a modification significantly compared to modifying a reimplementation.

My personal (current) way to go is to prefer reimplementations and only hooking at function call level unless it is very unfeasible because a function is too long, too complex or the modification is very minor so a simple patch is sufficient.

But that doesn't mean it has to be the approach for this project. I just wanted to give a few reasons to consider because i feel like generalizing a rule like

whenever modifying the game, the minimal amount of code should be rewritten and only when it is absolutely necessary

might not be consider this. In the end it is a trade-off of course though because your point are valid as well (especially the one with accidentally disabling other patches).

[L3D451R7]

I went with rebuilding the thing entirely so I can debug it properly (seeing all the input and output for example), and I was taught to avoid Assembly entirely in code - it's not a C++ practice (I dont mean that everyone should follow the things i was taught - just explaining why I went with solution you can see).

Since the original code in PR was using return address and stack hacks - I added assembler code to avoid those hacky things.

I will look into rumble issue soon, 4000 km away from home right now :)

[L3D451R7]

Also the call conversions are nasty in some functions, rebuilding was simpler than adding dirty inline assembly.

[Rackover]

Yea okay so this is guilty of the same thing - since it's rewriting a whole part of the engine, not only is it throwing off other parts of the client who expect to be able to hook into "the game" (and do not expect a whole chunk of the game to be sealed off and replaced with client code) but it also creates bug that are extremely hard to discover if you don't know exactly what you should be looking for.

As an example: this reimplementation breaks rumble.
Not all rumble, but the melee weapon rumble specifically - of course, because the melee rumble plugs somewhere in FireWeapon_Melee or in Melee_Trace and expects the game function to be called. Rewriting it completely breaks this behaviour. If I had not written the rumble, I would not have spotted this, and even with that it took me about ten minutes to realize this bad interaction could exist!

As a general safety rule, whenever modifying the game, the minimal amount of code should be rewritten and only when it is absolutely necessary - the game engine should be running the game, always, until we want to replace a specific part of it and then only that specific part should be replaced

[Rackover]

(Additionally - i suppose g_antilag should be an enum rather than an int?)

[wroyca]

Didn't mean to close this, oops.