Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow SDKHooks_TakeDamage to bypass OnTakeDamageAlive hooks #2217

Open
Bradasparky opened this issue Nov 3, 2024 · 1 comment
Open

Allow SDKHooks_TakeDamage to bypass OnTakeDamageAlive hooks #2217

Bradasparky opened this issue Nov 3, 2024 · 1 comment

Comments

@Bradasparky
Copy link

Description

As of version 1.13.0.7168, the bypassHooks parameter of the function SDKHooks_TakeDamage only bypasses the OnTakeDamage and OnTakeDamagePost hooks. I request for a way to also bypass OnTakeDamageAlive and OnTakeDamageAlivePost by either making bypassHooks also apply to alive hooks or by adding another parameter to optionally block the alive hooks.

According to psychonic, who looked into the reason that the alive hooks aren't blocked:

  • "it's not checked in the hook, but rather just changes how otd is called"
  • "since otda isn't called by sdkhooks, but rather by the game itself, the same thing doesn't apply"

Test Case

public void OnPluginStart() 
{
	RegConsoleCmd("hitme", HitMe);

	for (int i = MaxClients; i; i--) 
	{
		if (!IsClientInGame(i))
			continue;
		
		SDKHook(i, SDKHook_OnTakeDamage, OnTakeDamage);
		SDKHook(i, SDKHook_OnTakeDamagePost, OnTakeDamagePost);
		SDKHook(i, SDKHook_OnTakeDamageAlive, OnTakeDamageAlive);
		SDKHook(i, SDKHook_OnTakeDamageAlivePost, OnTakeDamageAlivePost);
	}
}

Action HitMe(int client, int args)
{
	PrintToServer("Hitting %N", client);
	SDKHooks_TakeDamage(client, client, client, 1.0, .bypassHooks = true);
	return Plugin_Handled;
}

Action OnTakeDamage(int victim, int& attacker, int& inflictor, float& damage, int& damagetype, int& weapon, float damageForce[3], float damagePosition[3], int damagecustom) 
{
	PrintToServer("OnTakeDamage");
	return Plugin_Continue;
}

void OnTakeDamagePost(int victim, int attacker, int inflictor, float damage, int damagetype, int weapon, float damageForce[3], float damagePosition[3], int damagecustom) 
{
	PrintToServer("OnTakeDamagePost");
}

Action OnTakeDamageAlive(int victim, int& attacker, int& inflictor, float& damage, int& damagetype, int& weapon, float damageForce[3], float damagePosition[3], int damagecustom) 
{
	PrintToServer("OnTakeDamageAlive");
	return Plugin_Continue;
}

void OnTakeDamageAlivePost(int victim, int attacker, int inflictor, float damage, int damagetype, int weapon, const float damageForce[3], const float damagePosition[3], int damagecustom) 
{
	PrintToServer("OnTakeDamageAlivePost");
}

Log

Upon using the hitme command:

Hitting Bradasparky
OnTakeDamageAlive
OnTakeDamageAlivePost
@Kenzzer
Copy link
Member

Kenzzer commented Nov 4, 2024

Mostly against this (at least as an extension of bypasshook).

SDKHooks TakeDamage native has always been problematic in my eyes as it calls CBaseEntity::OnTakeDamage rather than CBaseEntity::TakeDamage. Two very distinct functions, as the former is where health calculation takes place while the latter is where damage filtering happens. Now the reason why that matters to the issue at hand, if the native had been properly named SDKHook_OnTakeDamage rather than SDKHook_TakeDamage the ambiguity surrounding bypasshook parameter wouldn't have emerged. I suppose it's a consequence that there's an enum entry with the same name making that function nane unavailable.

Still if we are to suddenly also have OnTakeDamageAlive hooks bypassed then so should OnTakeDamageDead as well. It's not exposed and perhaps never will be. But I believe making the distinction now, will save troubles for later. A bypassalivehook would make for the most sense.

Finally as personal opinion, I believe nothing should be changed. Bypass parameters are a lot of troubles when it comes down to building your plugin logic. In the present day bypass is often used because the plugin author does not want the damage blocked, usually in instant death scenario. However there are other ways to go about instant death than relying on hook bypass. On the flip side, a plugin might have a very good reason to block or modify damage, let's say a ghost or friendly plugin. Bypass parameter wrecks havoc on all of that. The responsibility of when to intercept damage should fall on the plugin, rather than relying on bypass. That's what natives and forwards are for.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants