Getting rid of g_pevLastInflictor global var as a pseudo param for CBasePlayer::Killed

[dystopm]

This was an old programming way in Half-Life to handle parameters, without being parameters itself passed on functions. According to HLSDK, g_pevLastInflictor declaration says: "Set in combat.cpp. Used to pass the damage inflictor for death messages." while this can (stupidly) be a parameter in Killed() vfunc, and that we also cannot add as a parameter in Killed() since many hooks from different plugins would end broken.

If nowadays I want to rewrite TakeDamage function (totally valid I guess coming from a modding community) I wouldn't be able to because g_pevLastInflictor var cannot be changed unless I extend the API or trick a bit with memhacks.

In any case, an (almost) forgotten entvar (pev->dmg_inflictor) that is only used in players is being used correctly now, just like this global variable. Code comments tells out certain details anyway.

[StevenKal]

Good to update the EntVar "dmg_inflictor", but I might also recommend to keep the variable "g_pevLastInflictor" and its values set/reset for backward compatibility with some plugins that could have used it to get the inflictor from "CBasePlayer::Killed" (as information, I have some plugins using such global variable).
And yes, would have been good to have function "CBasePlayer::Killed" handling it as additional parameter, but for some reasons they did not think about it on the past, or, did not want to!

[s1lentq]

@dystopm
I would prefer to remove g_pevLastInflictor from the project
The code behavior remains unchanged (except case with memhacks) and keeping this global variable in the project only leads to unnecessary semantic complexity in the code

[dystopm]

@dystopm I would prefer to remove g_pevLastInflictor from the project The code behavior remains unchanged (except case with memhacks) and keeping this global variable in the project only leads to unnecessary semantic complexity in the code

@s1lentq

I have done a commit deleting g_pevLastInflictor global variable, and also added a commit to correctly clean dmg_inflictor entvar after Damage message is sent, but then Test demos failed. I reverted the last commit mentioned, and still failed, at which state everything was like before (in exception of commiting upstream/master into this current branch), so I guess Test demos failed because dmg_inflictor is set to null on CBaseMonster::TakeDamage when no inflictor passes damages (falldamage maybe?) and that provoked a different message encoding on Damage message internal values; which is also "strange" because the initial commit had that and Test demos passed, so I don't know actually. What's done it's a fix to an incorrect behaviour which is dmg_inflictor was not set to null when invalid inflictor is passed on TakeDamage; instead, the last inflictor keeps on memory and that's the position read on Damage message execution inside CBasePlayer::UpdateClientData. You could also receive dmg from a player, and then take fall damage and you'll have last player coordinates being shown on Damage message, which is also a fix here.

So a possible fix to the fix, would be to use another member/entvar to hold the original-behaved dmg_inflictor entvar value (maybe m_pChaseTarget from CBasePlayer which is unused, maybe a member from CCSPlayer or CCSEntity) and rework the original intended fix.

Remember the intention of this commit was to get rid of a global variable that worked as pseudoparameter and use an entity variable to hold that value so modders were able to alter it, without memhacks.

What do you think?

[s1lentq]

@dystopm
I think it's better to use members from CSSEntity to cover all entities,
but at now we can't add new members to CSSEntity base class without losing compatibility,
so we need to expand the reserve space and bump up the version of CSSEntity CSENTITY_API_INTERFACE_VERSION interface
I will do this, and after that you can use this the reserve space for new members.

[dystopm]

Closed in favour of #896