rehlds / ReGameDLL_CS

Enhanced server-side GameDLL for Counter-Strike 1.6 (and CS: Condition Zero), offering new features and fixes.
https://rehlds.dev/docs/regamedll-cs
GNU General Public License v3.0
590 stars 203 forks source link

Implement CBreakable:: hooks #988

Closed lozatto closed 2 months ago

lozatto commented 3 months ago

including Hooks: Spawn Restart TraceAttack TakeDamage Use Die BreakTouch

Vaqtincha commented 3 months ago

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

lozatto commented 3 months ago

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 commented 3 months ago

@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 commented 3 months ago

@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 commented 3 months ago

@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 commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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?)

SergeyShorokhov commented 2 months ago

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.