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

Implement CBreakable:: hooks #988

Closed
wants to merge 2 commits into from
Closed

Conversation

lozatto
Copy link

@lozatto lozatto commented Aug 9, 2024

including Hooks:
Spawn
Restart
TraceAttack
TakeDamage
Use
Die
BreakTouch

@Vaqtincha
Copy link
Contributor

Apart from Die, nothing is needed here. You can use hamsandwich.

@lozatto
Copy link
Author

lozatto commented Aug 9, 2024

problem that having to use ham and reapi ends up forcing us to use several different functions since we cannot use a brakeble with player calling the same final function and this ends up increasing our source code, I see that the best thing is to unify everything in the way of reapi

@lozatto
Copy link
Author

lozatto commented Aug 9, 2024

@Vaqtincha
very basic example I have the player function but I don't have it for CBreakable

RegisterHam(Ham_TraceAttack,		"func_breakable",	"HamHook_Entity_TraceAttack", 	false);
RegisterHam(Ham_TraceAttack,		"info_target", 		"HamHook_Entity_TraceAttack", 	false);
RegisterHam(Ham_TraceAttack,		"player", 		"HamHook_Entity_TraceAttack", 	false);

What do we do to create a function for the player and one for the others?

it is not possible to use reapi and hamsandwich, together in several cases if we follow the logic of we can use hamsandwich

@dystopm
Copy link
Contributor

dystopm commented Aug 9, 2024

@Vaqtincha very basic example I have the player function but I don't have it for CBreakable

RegisterHam(Ham_TraceAttack,		"func_breakable",	"HamHook_Entity_TraceAttack", 	false);
RegisterHam(Ham_TraceAttack,		"info_target", 		"HamHook_Entity_TraceAttack", 	false);
RegisterHam(Ham_TraceAttack,		"player", 		"HamHook_Entity_TraceAttack", 	false);

What do we do to create a function for the player and one for the others?

it is not possible to use reapi and hamsandwich, together in several cases if we follow the logic of we can use hamsandwich

I am not seeing the problem, can you elaborate?

@lozatto
Copy link
Author

lozatto commented Aug 9, 2024

@Vaqtincha very basic example I have the player function but I don't have it for CBreakable

RegisterHam(Ham_TraceAttack,		"func_breakable",	"HamHook_Entity_TraceAttack", 	false);
RegisterHam(Ham_TraceAttack,		"info_target", 		"HamHook_Entity_TraceAttack", 	false);
RegisterHam(Ham_TraceAttack,		"player", 		"HamHook_Entity_TraceAttack", 	false);

What do we do to create a function for the player and one for the others?
it is not possible to use reapi and hamsandwich, together in several cases if we follow the logic of we can use hamsandwich

I am not seeing the problem, can you elaborate?

all three are calling the same function, if I use reapi I am forced to use these two

RegisterHam(Ham_TraceAttack,		"func_breakable",	"HamHook_Entity_TraceAttack", 	false);
RegisterHam(Ham_TraceAttack,		"info_target", 		"HamHook_Entity_TraceAttack", 	false);

calling a different function than I will call the reapi one

RegisterHookChain(RG_CBasePlayer_TraceAttack, "@CBasePlayer_TraceAttack_Pre", false);

Then we get into the following situation, you call a function with reapi with any of the following

HAM_IGNORED,HAM_HANDLED,HAM_OVERRIDE,HAM_SUPERCEDE

or call a ham with the HC_CONTINUE,HC_SUPERCEDE,HC_BREAK,HC_BYPASS

If it doesn't work correctly, since reapi and ham don't communicate, we get into the following situation, we will be forced to create two functions with different names and with exactly the same code inside just for Reapi and Ham to work, and that doesn't make a minimum of meaning in terms of performance in programming

@dystopm
Copy link
Contributor

dystopm commented Aug 9, 2024

and that doesn't make a minimum of meaning in terms of performance in programming

Not really, this is some kind of exaggeration. You just don't want to split the handling of those callbacks. Having more than 1 Ham_* hook is not close to be a problem, you will have N different calls, and called in different times, because player targets CBasePlayer, func_breakable targets CBreakable, and going. You only need to group your code in a function and handle things a bit different. Die is the only really needed function here to hook, all others are hookable virtual functions.

It is like the classic arguing of "fakemeta > engine" and related. Use both, there is no warm, adapt your code ane be aware of the fact that you can have your own project forks.

I'm not saying that you are wrong, but this is a bit useless tbh, but can be merged anyway - I assume you also can export it to reapi, including class members, to fully support entity

@lozatto
Copy link
Author

lozatto commented Aug 9, 2024

and that doesn't make a minimum of meaning in terms of performance in programming

Not really, this is some kind of exaggeration. You just don't want to split the handling of those callbacks. Having more than 1 Ham_* hook is not close to be a problem, you will have N different calls, and called in different times, because player targets CBasePlayer, func_breakable targets CBreakable, and going. You only need to group your code in a function and handle things a bit different. Die is the only really needed function here to hook, all others are hookable virtual functions.

It is like the classic arguing of "fakemeta > engine" and related. Use both, there is no warm, adapt your code ane be aware of the fact that you can have your own project forks.

I'm not saying that you are wrong, but this is a bit useless tbh, but can be merged anyway - I assume you also can export it to reapi, including class members, to fully support entity

If we want to use ReAPI and Ham Sandwich in the same plugin and both have the same code, we would have to do something like this:

RegisterHookChain(RG_CBasePlayer_TakeDamage, "@CBasePlayer_TakeDamage_Pre", false);
RegisterHam(Ham_TakeDamage, "func_breakable", "@CBaseEntity_TakeDamage_Pre", false);

@CBasePlayer_TakeDamage_Pre(victim, inflictor, attacker, Float:damage, bitsDamageType)
{
	if (!is_user_alive(attacker))
		return;

	new activeItem = get_member(attacker, m_pActiveItem);
		
	if (is_nullent(activeItem))
		return;

	if (get_member(activeItem, m_iId) != WEAPON_KNIFE)
		return;

	SetHookChainArg(4, ATYPE_FLOAT, damage * 2.0);
}

@CBaseEntity_TakeDamage_Pre(victim, inflictor, attacker, Float:damage, bitsDamageType)
{
	if (!is_user_alive(attacker))
		return HAM_IGNORED;

	new activeItem = get_member(attacker, m_pActiveItem);
		
	if (is_nullent(activeItem))
		return HAM_IGNORED;

	if (get_member(activeItem, m_iId) != WEAPON_KNIFE)
		return HAM_IGNORED;

	SetHamParamFloat(4, damage * 2.0);
	return HAM_HANDLED;
}

So we could do everything using:

RegisterHookChain(RG_CBasePlayer_TakeDamage, "@CBaseEntity_TakeDamage_Pre", false);
RegisterHookChain(RG_CBreakable_TakeDamage, "@CBaseEntity_TakeDamage_Pre", false);

@CBaseEntity_TakeDamage_Pre(victim, inflictor, attacker, Float:damage, bitsDamageType)
{
	if (!is_user_alive(attacker))
		return;

	new activeItem = get_member(attacker, m_pActiveItem);
		
	if (is_nullent(activeItem))
		return;

	if (get_member(activeItem, m_iId) != WEAPON_KNIFE)
		return;

	SetHookChainArg(4, ATYPE_FLOAT, damage * 2.0);
}

In addition to helping expand projects such as ReZombie which are made using ReGameDLL

@Vaqtincha @dystopm
here is the comment about Reapi and HAM so I'm not talking exaggeratedly
https://forums.alliedmods.net/showpost.php?p=2461259&postcount=4

@dystopm
Copy link
Contributor

dystopm commented Aug 10, 2024

If we want to use ReAPI and Ham Sandwich in the same plugin and both have the same code, we would have to do something like this:

Not really, you can group your code.

RegisterHookChain(RG_CBasePlayer_TakeDamage, "@CBasePlayer_TakeDamage_Pre", false);
RegisterHam(Ham_TakeDamage, "func_breakable", "@CBaseEntity_TakeDamage_Pre", false);

@CBasePlayer_TakeDamage_Pre(victim, inflictor, attacker, Float:damage, bitsDamageType)
{
	if(Handle_TakeDamage(victim, inflictor, attacker, damage, bitsDamageType))
		SetHookChainArg(4, ATYPE_FLOAT, damage);
}

@CBaseEntity_TakeDamage_Pre(victim, inflictor, attacker, Float:damage, bitsDamageType)
{
	if(Handle_TakeDamage(victim, inflictor, attacker, damage, bitsDamageType))
		SetHamParamFloat(4, damage); // no need to ensure HAM_HANDLED as return value
}

bool:Handle_TakeDamage(victim, inflictor, attacker, &Float:damage, bitsDamageType)
{
	if (!is_user_alive(attacker))
		return false;

	new activeItem = get_member(attacker, m_pActiveItem);
		
	if (is_nullent(activeItem))
		return false;

	if (get_member(activeItem, m_iId) != WEAPON_KNIFE)
		return false;

	damage *= 2.0;
	return true;
}

In addition to helping expand projects such as ReZombie which are made using ReGameDLL
It is reasonable

@Vaqtincha @dystopm here is the comment about Reapi and HAM so I'm not talking exaggeratedly https://forums.alliedmods.net/showpost.php?p=2461259&postcount=4

It is an 8 year old post, I see the point, there's no profiling and versions were developed since that. You got there an answer also:

Yes, but it's not full replacement of all other modules. The main purpose of Reapi is to replace modules that using hacks to access certain features of engine and gamedll

and Hamsandwich has no documentation of an slow behaviour in some manner, it's just a comparison to motivate developers migration and I see it reasonable - but if this was certainly an explicit and veridic assumption, the entire Hamsandwich module would have been rewritten in reapi, which is not the case.

I'm just politely giving my thoughts on something that has been used for years. There are plenty of things that we can focus in develop better approaches IMO, and I may receive different opinions - but I'm not against this PR, I'm just averse to rumors (just seen somebody told get_entvar is 50% faster than fakemeta, what?)

@wopox1337
Copy link
Collaborator

Just use Hamsandwich, there will be no difference in performance. your requests for code convenience are strange, all your problems are completely solvable without reapi and code complications.

@wopox1337 wopox1337 closed this Aug 29, 2024
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

Successfully merging this pull request may close these issues.

4 participants