nwnxee / unified

Binaries available under the Releases tab on Github
https://nwnxee.github.io/unified
GNU General Public License v3.0
129 stars 92 forks source link

DAMAGE_TYPE_BASE_WEAPON doesn't propogate if added in ResolveDeathAttackHook #1726

Open pF-arQon opened 8 months ago

pF-arQon commented 8 months ago

this: static void s_ResolveDeathAttackHook( CNWSCreature *pThis, CNWSCreature *pTarget) { ... CNWSCombatAttackData *pAttackData = pThis->m_pcCombatRound->GetAttack(pThis->m_pcCombatRound->m_nCurrentAttack); pAttackData->m_nDamage[4] += 100; } works fine, and adds 100 acid to each attack, as you'd expect, and likewise for every other index except [12], i.e. DAMAGE_TYPE_BASE_WEAPON. I'd expect / hope for that to be converted to the appropriate physical fields at some point in the chain, but it doesn't seem to be.

(No idea if the same applies to the SA hook - haven't had time to test that one).

Daztek commented 8 months ago

It's not really a bug, BaseDamage gets overridden in CNWSCreature::ResolveDamage which runs some time after ResolveDeathAttack(), other damage types work because it adds to them instead.

pF-arQon commented 8 months ago

Thanks. It's that inconsistency that bothers me, but if it's impractical to fix it then so be it. I do get that this is a lot earlier in the chain than people generally mess around with the damage part of an attack, but since DA is already "special" you have to catch it here if you want to change its behavior.

That being the case, it would have been nice to be able to modify the damage here so I can keep it all nicely contained in the DA hook rather than having to also hook Damage and carry the data over in locals. I may try just doing the BASE_WEAPON conversion here myself instead, but either way I think we're done with the behavior of this part unless someone wants to try fixing it, and given the potential dominoes from doing so that seems very unlikely. :)