[bug] Bonus T9 4P DK & t9 2P Paladin Critical damage.

[moony]

Critical damage blood plaque and rigteous vengeance from bonus item set cause triple damage, instead double damage.

[kbinside]

you forget to mention hunter t9 2P too.

these bonuses make those skill crit for about 300%-500% instead of 100%.

[tobmaps]

think i know why DK T9 2P is bugged, it's because

ID - 55078 Blood Plague

has

DamageClass = 2 (SPELL_DAMAGE_CLASS_MELEE)

and in the code

// Calculate critical bonus
int32 crit_bonus;
switch(spellProto->DmgClass)
{
    case SPELL_DAMAGE_CLASS_MELEE:                      // for melee based spells is 100%
    case SPELL_DAMAGE_CLASS_RANGED:
        // TODO: write here full calculation for melee/ranged spells
        crit_bonus = damage;
        break;
    default:
        crit_bonus = damage / 2;                        // for spells is 50%
        break;
}

ofc because our spell are MELEE then it means if your spell hit for 500 with non-crit then it will hit 1000 (500+500), let's think that this is ok, just see next strings of code...

// adds additional damage to crit_bonus (from talents)
if (Player* modOwner = GetSpellModOwner())
    modOwner->ApplySpellMod(spellProto->Id, SPELLMOD_CRIT_DAMAGE_BONUS, crit_bonus);

HINT: not only talents...

then we looking to

ID - 67118 Item - Death Knight T9 Melee 4P Bonus

and what we see?

Aura Id 108 (SPELL_AURA_ADD_PCT_MODIFIER), value = 100, misc = 15 (SPELLMOD_CRIT_DAMAGE_BONUS), miscB = 0, periodic = 0
SpellClassMask = 00000000 02000000 00000000
    - 55078 - Blood Plague
    + 59879 - Blood Plague (Passive)

it not only enables a crit, it also increases a damage for this crit %) BUT THE DAMAGE IS ALREADY INCREASED BEFORE THIS SPELLMOD (!)
so, 500+500*100% will cause 2000 damage in result :) instead of 1000 that should be

possible (just because it can be done with a more wide checks for more spells) solution:
make

crit_bonus = damage / 2;

for all these spells in this

switch(spellProto->DmgClass)

I will think about more wide solution but later... now I'm busy

[tobmaps]

try this https://gist.github.com/1011199 as temp solution (also included Hunter T9 bonus case)
I just can't find any generic rule for this. Blizz vs me 1:0 :(

[kbinside]

and this to commit please, and huge thanks.

[moony]

ty

[tobmaps]

do not close not-fixed Issues just because patch posted here. I just don't like to add hacks if there may exists any other generic solution...

[moony]

wrong button, i don't mean to.

[EnteringCombat]

Confirmed running 04862de

[Lat89]

Can anyone check this out? Not my stuff i suck at c++ and i've got no idea if its a hacky fix or not. Thanks.

diff --git a/src/server/game/Spells/SpellMgr.cpp b/src/server/game/Spells/SpellMgr.cpp
index d40c08f..c22228a 100755
--- a/src/server/game/Spells/SpellMgr.cpp
+++ b/src/server/game/Spells/SpellMgr.cpp
@@ -3202,6 +3202,11 @@ void SpellMgr::LoadDbcDataCorrections()
             case 63675: // Improved Devouring Plague
                 spellInfo->AttributesEx3 |= SPELL_ATTR3_NO_DONE_BONUS;
                 break;
+            case 67188: // Item - Paladin T9 Retribution 2P Bonus (Righteous Vengeance)
+            case 67118: // Item - Death Knight T9 Melee 4P Bonus
+            case 67150: // Item - Hunter T9 2P Bonus
+                spellInfo->Effect[1] = 0;
+                break;
             case 8145: // Tremor Totem (instant pulse)
             case 6474: // Earthbind Totem (instant pulse)
                 spellInfo->AttributesEx5 |= SPELL_ATTR5_START_PERIODIC_AT_APPLY;

[Aokromes]

Try after 9f09713