Breaking push credit for other addons(?)

[rrredface]

So I dev for a friend's server that has an achievements addon. Recently added were achievements for getting kills with specific weapons. The hook that handles this is below, the important parts are the prints at the very top and the part that handles DMG_FALL and DMG_GENERIC

hook.Add("DoPlayerDeath", "ACVWeapons", function (victim, killer, dmginfo)
    print("killer: " .. tostring(killer))
    print("inflictor: " .. dmginfo:GetInflictor():GetClass())
    print("dmg type: " .. tostring(dmginfo:GetDamageType()))
    print("=========")
    if victim != killer and killer:IsPlayer() then
        if dmginfo:IsDamageType(DMG_BULLET + DMG_CLUB + DMG_SLASH) then
            local wepstring = dmginfo:GetAttacker():GetActiveWeapon():GetClass()
            local func = inc_tbl[wepstring]
            if (func) then
                func(killer)
            end
        elseif dmginfo:IsDamageType(DMG_CRUSH) then
            local inflictor = dmginfo:GetInflictor()
            if inflictor:IsPlayer() then
                inc_goomba(killer)
            else
                local wepstring = dmginfo:GetAttacker():GetActiveWeapon():GetClass()
                local func = inc_tbl[wepstring]
                if (func) then
                    func(killer)
                end
            end
        -- DMG_GENERIC for when players die to a kill trigger instead of fall damage
        elseif dmginfo:IsDamageType(DMG_FALL) or (dmginfo:GetDamageType() == 0) then
            local was_pushed = victim.was_pushed
            if not was_pushed then return nil end
            print("was pushed by " .. was_pushed.att:GetName() .. " with " .. was_pushed.wep)
            -- 5 second window after being pushed for the pusher to get
            -- credit for the kill
            if was_pushed and (was_pushed.t >= CurTime() - 5) then
                if IsValid(was_pushed.att) and IsPlayer(was_pushed.att) then
                    local wepstring = was_pushed.wep
                    local func = inc_tbl[wepstring]
                    if (func) then
                        func(was_pushed.att)
                    end
                end
            end
        -- DMG_SONIC is telefrags
        elseif dmginfo:IsDamageType(DMG_BLAST + DMG_SONIC) then
            local ent = dmginfo:GetInflictor()
            if not IsValid(ent) then return nil end
            local entstring = ent:GetClass()
            if entstring == "env_explosion" then
                local infl = ent.m_hInflictor
                if (infl) and infl:GetClass() == "weapon_ttt_rigger" then
                    inc_rigger(killer)
                end
            else
                local func = inc_tbl[entstring]
                if (func) then
                    func(killer)
                end
            end
        elseif dmginfo:IsDamageType(DMG_BURN) then
            local entstring = dmginfo:GetInflictor():GetClass()
            -- i don't know why the flare gun works this way
            if entstring == "player" then
                inc_flaregun(killer)
            else
                local func = inc_tbl[entstring]
                if (func) then
                    func(killer)
                end
            end
        end

    end
end)

When I test this without damagelogs installed, i get the expected output, things like this:

killer: Player [1][alchemist]
inflictor: worldspawn
dmg type: 32
=========
was pushed by alchemist with weapon_zm_improvised

killer: Player [1][alchemist]
inflictor: trigger_hurt
dmg type: 0
=========
was pushed by alchemist with weapon_ttt_push

Trying the same thing with damagelogs installed, I stop getting credit:

killer: Entity [0][worldspawn]
inflictor: worldspawn
dmg type: 32
=========

The kill is credited to worldspawn, so none of the damage type handling in that hook gets run. Credit still works fine, as far as I have seen, in every other case. But push credit breaks. I tested this pushing from as close to the same spot as possible each time as well, so it shouldn't be because the victim is falling for too long. Also curiously, when I check the damage logs, it does give me credit.

I don't know if I'm missing something, I've skimmed through the source of this addon for anything that might cause this to happen but I'm not sure what it could be.

[BadgerCode]

Heyo!

Here is the code, used by the damage logs to determine if someone was pushed to death https://github.com/BadgerCode/tttdamagelogs/blob/master/lua/damagelogs/shared/events/kills.lua#L12

function Player:GetPlayerThatRecentlyPushedMe()
    local pushInfo = self.was_pushed
    if(pushInfo == nil) then return nil end

    -- player.was_pushed is never reset. We must always check the time on the push event.
    -- Copied from TTT: Only consider pushes in the last 4 seconds
    local pushTime = math.max(pushInfo.t or 0, pushInfo.hurt or 0)
    if(pushTime < CurTime() - 4) then return nil end

    return pushInfo.att
end

This is copied from the TTT gamemode files, which performs the same check https://github.com/Facepunch/garrysmod/blob/9bbd7c8af0dda5bed88e3f09fbdf5d4be7e012f2/garrysmod/gamemodes/terrortown/gamemode/player.lua#L825

local att = ply

-- if the faller was pushed, that person should get attrib
local push = ply.was_pushed
if push then
   -- TODO: move push time checking stuff into fn?
   if math.max(push.t or 0, push.hurt or 0) > CurTime() - 4 then
      att = push.att
   end
end

This means that when you fall to your death, if someone pushed you in the last 4 seconds, they will get credit for the kill.

I can see in your code, it checks if the player was pushed in the last 5 seconds. if was_pushed and (was_pushed.t >= CurTime() - 5) then This might be why?

[rrredface]

I can see in your code, it checks if the player was pushed in the last 5 seconds. if was_pushed and (was_pushed.t >= CurTime() - 5) then This might be why?

I didn't realize that TTT had its own 4 second limit, however that doesn't seem to be the problem. My code seems to behave differently with and without damagelogs installed, in an otherwise more or less identical situation. Here's a demonstration.

To be clear, your addon in and of itself is working correctly. But having it installed seems to change the way that mine works.

[BadgerCode]

Ah yeah that is strange

The hook itself doesn't have a return value, so it shouldn't matter what the damagelog code or your code does with the DoPlayerDeath event.

The damagelogs just records information for this hook, so that also shouldn't affect your code.

[rrredface]

A return value in your hook would stop my hook from executing at all so that's definitely not it.

Making this even more confusing, it even seems like my hook runs before yours. I added identical prints to my hook and the DoPlayerDeath hook in both events/kills.lua and events/suicide.lua (all the way at the top, before any checks) and got this output:

from achievements DoPlayerDeath hook:
player: Bot05
attacker: worldspawn
dmg type: 32
========
from tttdamagelogs DoPlayerDeath hook (kills.lua):
player: Bot05
attacker: worldspawn
dmg type: 32
========
from tttdamagelogs DoPlayerDeath hook (suicide.lua):
player: Bot05
attacker: worldspawn
dmg type: 32
========

And then again, without damage logs:

from achievements DoPlayerDeath hook:
player: Bot07
attacker: player
dmg type: 32
========
was pushed by alchemist with weapon_zm_improvised

I'm not even positive this is even a problem with your addon and not some bizarre issue with gmod anymore.

[rrredface]

Alright, I've got the issue narrowed down pretty far. I removed files from your addon, mine worked normally, and I added them back until mine broke again. The culprit is events/damages.lua. Without that file, pushing a player gives me the following output:

from achievements DoPlayerDeath hook:
player: Bot05
attacker: player
dmg type: 32
========
was pushed by alchemist with weapon_zm_improvised
from tttdamagelogs DoPlayerDeath hook (kills.lua):
player: Bot05
attacker: player
dmg type: 32
========
from tttdamagelogs DoPlayerDeath hook (suicide.lua):
player: Bot05
attacker: player
dmg type: 32
========

With that file, push kills always get blamed on worldspawn. The issue also doesn't seem to be the PlayerTakeRealDamage hook. I did lua_run hook.Remove("PlayerTakeRealDamage", "Damagelog_events_PlayerTakeRealDamage"). Printed all hooks and got this:

...
["PlayerSpawnedVehicle"]:
        ["ULXLogVehicleSpawn"]  =   function: 0x88df586a
["PlayerTakeRealDamage"]:
["PlayerTick"]:
        ["TickWidgets"] =   function: 0xa488e8b2
...

Tried pushing a bot again with the hook removed, and it still credits worldspawn. All of that said, I believe this function in particular is causing the problem:

function event:Initialize()
    local old_func = GAMEMODE.PlayerTakeDamage

    function GAMEMODE:PlayerTakeDamage(ent, infl, att, amount, dmginfo)
        local original_dmg = dmginfo:GetDamage()

        if IsValid(att) then
            old_func(self, ent, infl, att, amount, dmginfo)
        end

        hook.Call("PlayerTakeRealDamage", GAMEMODE, ent, dmginfo, original_dmg)
    end
end

Why this would cause problems with my addon, I don't know. Obviously without that file nothing besides deaths get logged. I'm not familiar enough with the workings of your addon to know what to even change that might fix it, but maybe this will give you an idea.

[rrredface]

This seems to unbreak my addon and from what I can tell doesn't break logging.

function event:Initialize()
    local old_func = GAMEMODE.PlayerTakeDamage

    function GAMEMODE:PlayerTakeDamage(ent, infl, att, amount, dmginfo)
        local original_dmg = dmginfo:GetDamage()

-       if IsValid(att) then
+   if att or infl then
            old_func(self, ent, infl, att, amount, dmginfo)
        end

        hook.Call("PlayerTakeRealDamage", GAMEMODE, ent, dmginfo, original_dmg)
    end
end

[BadgerCode]

Ah nice! An easy fix is always ideal 😛

Am I able to close this issue? Or is there more work needed?