virtual functions and skip_hooks

[Predz]

Hey all,

Trying to get my head around a problem I have just solved...

What is skip_hooks actually doing? My understanding is that it ignores all pre/post hooks and calls the function like normal... however running some code I am still ending up in a server crash when using skip_hooks. Rather than this, I have replaced it to directly use call_trampoline and the problem is resolved.

I know this code is not ideal to debug as it is part of a plugin (but as the problem is solved) I wanted others' opinions on how this even resolves the crashes

For sanity's sake, any of the functions wrapper in

Code: Select all

@events

or

Code: Select all

@clientcommands

are directly called from the event or from a clientcommandfilter respectively. Any event with "pre" in the title is called from the respective prehook of that function. So "player_pre_victim" is just the victim's perspective of on_take_damage.

The crash occurs any time 2 people are attacking each other and they both have these callbacks registered.

TLDR: Why is call_trampoline working in the SpikedCarapace skill, and the commented skip_hooks line causes a server crash?

Syntax: Select all

## cryptlord declaration

class CryptLord(Race):

    @classproperty
    def description(cls):
        return 'Recoded Crypt Lord. (Kryptonite)'

    @classproperty
    def max_level(cls):
        return 99

    @classproperty
    def requirement_sort_key(cls):
        return 8


@CryptLord.add_skill
class Impale(Skill):

    @classproperty
    def description(cls):
        return 'Upon attacking, knock your enemy up and shake. 7-15% chance.'

    @classproperty
    def max_level(cls):
        return 8

    _msg_a = '{{PALE_GREEN}}You {{DULL_RED}}impaled {{RED}}{name}{{GREEN}}!'

    @events('player_pre_attack')
    def _on_player_pre_attack_impale(self, attacker, victim, **kwargs):
        if randint(1, 100) <= 7 + self.level and not victim.dead:
            victim.push(1, 200, True)
            Shake(100, 1.5).send(victim.index)
            send_wcs_saytext_by_index(self._msg_a.format(name=victim.name), attacker.index)


@CryptLord.add_skill
class SpikedCarapace(Skill):

    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)

        self.total_damage_prevented = 0

    @classproperty
    def description(cls):
        return 'Reduces incoming damage by 5-20% and deal 3-7 reflected damage when being attacked.'

    @classproperty
    def max_level(cls):
        return 8

    @property
    def reduction(self):
        return min(5 + (2 * self.level), 20)

    @property
    def reflect_damage(self):
        return 3 + (self.level / 2)

    _msg_a = '{{GREEN}}Spiked Carapace {{PALE_GREEN}}prevented {{DULL_RED}}{damage} {{PALE_GREEN}}last round.'

    @events('player_spawn')
    def _on_player_spawn(self, player, **kwargs):
        send_wcs_saytext_by_index(self._msg_a.format(damage=int(self.total_damage_prevented)), player.index)
        self.total_damage_prevented = 0

    @events('player_pre_victim')
    def _on_player_pre_victim(self, attacker, victim, info, **kwargs):
        multiplier = 1 - (self.reduction / 100)
        old_damage = info.damage
        info.damage *= multiplier
        reduced_damage = old_damage - info.damage
        self.total_damage_prevented += reduced_damage

    @events('player_victim')
    def _on_player_victim(self, attacker, victim, **kwargs):
        if attacker.dead:
            return

        ##attacker.take_damage(self.reflect_damage, attacker_index=victim.index, skip_hooks=True)

        info = TakeDamageInfo()
        info.inflictor = victim.index
        info.damage = self.reflect_damage
        attacker.on_take_damage.call_trampoline(info)


@CryptLord.add_skill
class ShadowStrike(Skill):

    @classproperty
    def description(cls):
        return 'Grants you 7-15% chance to deal an additional 9-17 damage when attacking.'

    @classproperty
    def max_level(cls):
        return 8

    @property
    def extra_damage(self):
        return 9 + self.level
    
    @property
    def chance(self):
        return 7 + self.level

    _msg_a = '{{DARK_BLUE}}Shadow Strike {{PALE_GREEN}}dealt {{DULL_RED}}{damage} {{PALE_GREEN}}extra to {{RED}}{name}{{PALE_GREEN}}.'

    @events('player_pre_attack')
    def _on_player_pre_attack(self, attacker, victim, info, **kwargs):
        if randint(0, 101) <= self.chance and not victim.dead:
            info.damage += self.extra_damage
            send_wcs_saytext_by_index(self._msg_a.format(damage=self.extra_damage, name=victim.name), attacker.index)

[Predz]

Additional code for the calling of the decorated functions.

Syntax: Select all

@Event('player_hurt')
def _on_hurt_call_events(event_data):
    if event_data['userid'] == event_data['attacker'] or event_data['attacker'] == 0:
        return

    kwargs = event_data.variables.as_dict()
    attacker = player_dict.from_userid(event_data['attacker'])
    victim = player_dict.from_userid(event_data['userid'])
    del kwargs['attacker'] ## remove duplicate keyword argument

    if victim.team == attacker.team:
        attacker.call_events('player_teammate_attack', player=attacker,
            victim=victim, attacker=attacker, **kwargs)
        victim.call_events('player_teammate_victim', player=victim,
            attacker=attacker, victim=victim, **kwargs)
        return

    attacker.call_events('player_attack', player=attacker, victim=victim,
        attacker=attacker, **kwargs)
    victim.call_events('player_victim', player=victim, attacker=attacker,
        victim=victim, **kwargs)

@EntityPreHook(EntityCondition.is_player, 'on_take_damage')
def _pre_damage_call_events(stack_data):
    take_damage_info = make_object(TakeDamageInfo, stack_data[1])
    if take_damage_info.attacker:
        entity = Entity(take_damage_info.attacker)
        attacker = player_dict[entity.index] if entity.is_player() else None
        victim = player_dict[index_from_pointer(stack_data[0])]

        event_args = {
            'attacker': attacker,
            'victim': victim,
            'info': take_damage_info,
        }

        if attacker:
            if victim.team == attacker.team:
                attacker.call_events('player_pre_teammate_attack', player=attacker,
                    **event_args)
                victim.call_events('player_pre_teammate_victim', player=victim, **event_args)
            else:
                attacker.call_events('player_pre_attack', player=attacker, **event_args)
                victim.call_events('player_pre_victim', player=victim, **event_args)

                if victim.health <= take_damage_info.damage:
                    victim.call_events('player_pre_death', player=victim, **event_args)

[L'In20Cible]

I don't have the time to go into the technicalities as to how and why, etc. but what you are looking for is the hooks_disabled context to ensure your hook do not get recursively called through a polymorphic call.

[L'In20Cible]

Alright, I will try to add some details.

TLDR: Why is call_trampoline working in the SpikedCarapace skill, and the commented skip_hooks line causes a server crash?

Both methods will forward the call to the trampoline when a hook is registered. The only difference, is that call_trampoline will raise a ValueError if the function isn't hooked while skip_hooks will call the function directly in such case. Basically, one explicitly call the trampoline or raise, while the other skip the hooks if any. Here's the source:

Syntax: Select all

object CFunction::CallTrampoline(tuple args, dict kw)
{
	CHook* pHook = GetHookManager()->FindHook((void *) m_ulAddr);
	if (!pHook)
		BOOST_RAISE_EXCEPTION(PyExc_ValueError, "Function was not hooked.")

	return CFunction((unsigned long) pHook->m_pTrampoline, m_eCallingConvention,
		m_iCallingConvention, m_tArgs, m_eReturnType, m_oConverter).Call(args, kw);
}

object CFunction::SkipHooks(tuple args, dict kw)
{
	CHook* pHook = GetHookManager()->FindHook((void *) m_ulAddr);
	if (pHook)
		return CFunction((unsigned long) pHook->m_pTrampoline, m_eCallingConvention,
			m_iCallingConvention, m_tArgs, m_eReturnType, m_oConverter).Call(args, kw);

	return Call(args, kw);
}

So, having said that, the fact the following:

Syntax: Select all

attacker.take_damage(self.reflect_damage, attacker_index=victim.index, skip_hooks=True)

Is crashing, while the following:

Syntax: Select all

info = TakeDamageInfo()
info.inflictor = victim.index
info.damage = self.reflect_damage
attacker.on_take_damage.call_trampoline(info)

Isn't, is not directly because you use call_trampoline instead of skip_hooks. My guess here, is that you have a callback that is dealing extra damage based on the weapon. Remember that, take_damage will resolve and set the weapon that is causing the damage, while you don't set that information.

What is skip_hooks actually doing? My understanding is that it ignores all pre/post hooks and calls the function like normal...

That is correct. However, it will only skip the hooks of the original call. For example, suppose you have the following:

Syntax: Select all

class Player:
    def take_damage(self):
        ...

class Bot(Player):
    def take_damage(self):
        super().take_damage()

Bot().take_damage.skip_hooks()

The hooks on Bot.take_damage will not be called, however, the ones registered on Player.take_damage will. So, if you have the same callback registered on bot and do not filter bot/human calls, this will result into an infinite recursion if you don't have the necessary checks (for instance, if you had an isdead check, the recursion would happens but would stop when the player is actually dead). This problem was referred into #236.

In conclusion, if you are using the hooks_disabled context, no hooks will be called during that scope. This can cause some issue though. For example, if you are waiting to know about the damage through a player_hurt/player_death event, you won't be noticed because the hook on fire_game_event will not be called. If that is an issue for you, simply make your own context and exit your callback based on your own guard variable.

[Predz]

Thanks for the clarification! Yeh I had understood this was most probably me causing an infinite loop during the attack/victim stuff

It seems that the reason for the crash is due to setting the `attacker` attribute of TakeDamageInfo; as adding it, instantly causes the server crash.

Removing it also causes the damage to not count as the user actually causing the damage. So I can presume that the `inflictor` attribute is not actually doing anything without the `attacker` attribute set.

I tried running the TakeDamageInfo with the `attacker` attribute set within the hooks_disabled contextmanager, however still ends in the crash. Not really too sure where to go with that

[L'In20Cible]

It seems that the reason for the crash is due to setting the `attacker` attribute of TakeDamageInfo; as adding it, instantly causes the server crash.

Make sense, because the following condition is met:

Syntax: Select all

@Event('player_hurt')
def _on_hurt_call_events(event_data):
    if event_data['userid'] == event_data['attacker'] or event_data['attacker'] == 0:
        return

Therefore, the recursion do not happens because you exit the call when there is no attacker. If you add a dead check in there, chances are the recursion will happens until the player dies from all the damage.

Removing it also causes the damage to not count as the user actually causing the damage. So I can presume that the `inflictor` attribute is not actually doing anything without the `attacker` attribute set.

Have a look at Entity.take_damage that illustrate the differences between the attacker and the inflictor. In short, the attacker is the player and the inflictor is the projectile. The engine handle them separately, because it has to know where the actual damage is coming from as well by who it was caused.

I tried running the TakeDamageInfo with the `attacker` attribute set within the hooks_disabled contextmanager, however still ends in the crash. Not really too sure where to go with that

Try to use that context while using Entity.take_damage directly. No reason to rewrite that function yourself. The only thing I can think would still cause a recursion, is if you are delaying the extra damage. When the delay will be processed, the context will be done and hooks will be enabled again.