[fix] Spellmod Spell Bonus Damage

[Auntie Mangos]

* What bug does the patch fix? What features does the patch add?

Fixes the spell mod SPELLMOD_SPELL_BONUS_DAMAGE (aura 107), currently incorrectly applied as a multiplicative value of the total damage instead of modifying the spell coefficient value as it should.

* For which repository revision was the patch created?

8719

* Is there a thread in the bug report section or at lighthouse?

http://getmangos.eu/community/viewtopic.php?id=9595

* Who has been writing this patch?

darkstalker

diff --git a/src/game/Unit.cpp b/src/game/Unit.cpp
index de56693..5b2bea3 100644
--- a/src/game/Unit.cpp
+++ b/src/game/Unit.cpp
@@ -8457,11 +8457,8 @@ uint32 Unit::SpellDamageBonus(Unit *pVictim, SpellEntry const *spellProto, uint3
        DoneAdvertisedBenefit += ((Pet*)this)->GetBonusDamage();

    float LvlPenalty = CalculateLevelPenalty(spellProto);
-    // Spellmod SpellDamage
-    float SpellModSpellDamage = 100.0f;
-    if(Player* modOwner = GetSpellModOwner())
-        modOwner->ApplySpellMod(spellProto->Id,SPELLMOD_SPELL_BONUS_DAMAGE,SpellModSpellDamage);
-    SpellModSpellDamage /= 100.0f;
+
+    Player* modOwner = GetSpellModOwner();

    // Check for table values
    if (SpellBonusEntry const* bonus = spellmgr.GetSpellBonusData(spellProto->Id))
@@ -8475,7 +8472,15 @@ uint32 Unit::SpellDamageBonus(Unit *pVictim, SpellEntry const *spellProto, uint3
        if (bonus->ap_bonus)
            DoneTotal += int32(bonus->ap_bonus * GetTotalAttackPowerValue(BASE_ATTACK) * stack);

-        DoneTotal  += int32(DoneAdvertisedBenefit * coeff * SpellModSpellDamage);
+        // Spellmod SpellBonusDamage
+        if (modOwner)
+        {
+            coeff *= 100.0f;
+            modOwner->ApplySpellMod(spellProto->Id,SPELLMOD_SPELL_BONUS_DAMAGE,coeff);
+            coeff /= 100.0f;
+        }
+
+        DoneTotal  += int32(DoneAdvertisedBenefit * coeff);
        TakenTotal += int32(TakenAdvertisedBenefit * coeff);
    }
    // Default calculation
@@ -8508,8 +8513,19 @@ uint32 Unit::SpellDamageBonus(Unit *pVictim, SpellEntry const *spellProto, uint3
                break;
            }
        }
-        DoneTotal += int32(DoneAdvertisedBenefit * (CastingTime / 3500.0f) * DotFactor * LvlPenalty * SpellModSpellDamage);
-        TakenTotal+= int32(TakenAdvertisedBenefit * (CastingTime / 3500.0f) * DotFactor * LvlPenalty);
+
+        float coeff = (CastingTime / 3500.0f) * DotFactor;
+
+        // Spellmod SpellBonusDamage
+        if (modOwner)
+        {
+            coeff *= 100.0f;
+            modOwner->ApplySpellMod(spellProto->Id,SPELLMOD_SPELL_BONUS_DAMAGE,coeff);
+            coeff /= 100.0f;
+        }
+
+        DoneTotal += int32(DoneAdvertisedBenefit * coeff * LvlPenalty);
+        TakenTotal+= int32(TakenAdvertisedBenefit * coeff * LvlPenalty);
    }

    float tmpDamage = (pdamage + DoneTotal) * DoneTotalMod;
@@ -8906,11 +8922,8 @@ uint32 Unit::SpellHealingBonus(Unit *pVictim, SpellEntry const *spellProto, uint
    int32 TakenAdvertisedBenefit = SpellBaseHealingBonusForVictim(GetSpellSchoolMask(spellProto), pVictim);

    float LvlPenalty = CalculateLevelPenalty(spellProto);
-    // Spellmod SpellDamage
-    float SpellModSpellDamage = 100.0f;
-    if(Player* modOwner = GetSpellModOwner())
-        modOwner->ApplySpellMod(spellProto->Id, SPELLMOD_SPELL_BONUS_DAMAGE, SpellModSpellDamage);
-    SpellModSpellDamage /= 100.0f;
+
+    Player* modOwner = GetSpellModOwner();

    // Check for table values
    SpellBonusEntry const* bonus = spellmgr.GetSpellBonusData(spellProto->Id);
@@ -8925,7 +8938,15 @@ uint32 Unit::SpellHealingBonus(Unit *pVictim, SpellEntry const *spellProto, uint
        if (bonus->ap_bonus)
            DoneTotal += int32(bonus->ap_bonus * GetTotalAttackPowerValue(BASE_ATTACK) * stack);

-        DoneTotal  += int32(DoneAdvertisedBenefit * coeff * SpellModSpellDamage);
+        // Spellmod SpellBonusDamage
+        if (modOwner)
+        {
+            coeff *= 100.0f;
+            modOwner->ApplySpellMod(spellProto->Id,SPELLMOD_SPELL_BONUS_DAMAGE,coeff);
+            coeff /= 100.0f;
+        }
+
+        DoneTotal  += int32(DoneAdvertisedBenefit * coeff);
        TakenTotal += int32(TakenAdvertisedBenefit * coeff);
    }
    // Default calculation
@@ -8957,8 +8978,19 @@ uint32 Unit::SpellHealingBonus(Unit *pVictim, SpellEntry const *spellProto, uint
                break;
            }
        }
-        DoneTotal  += int32(DoneAdvertisedBenefit * (CastingTime / 3500.0f) * DotFactor * LvlPenalty * SpellModSpellDamage * 1.88f);
-        TakenTotal += int32(TakenAdvertisedBenefit * (CastingTime / 3500.0f) * DotFactor * LvlPenalty * 1.88f);
+
+        float coeff = (CastingTime / 3500.0f) * DotFactor * 1.88f;
+
+        // Spellmod SpellBonusDamage
+        if (modOwner)
+        {
+            coeff *= 100.0f;
+            modOwner->ApplySpellMod(spellProto->Id,SPELLMOD_SPELL_BONUS_DAMAGE,coeff);
+            coeff /= 100.0f;
+        }
+
+        DoneTotal  += int32(DoneAdvertisedBenefit * coeff * LvlPenalty);
+        TakenTotal += int32(TakenAdvertisedBenefit * coeff * LvlPenalty);
    }

    // use float as more appropriate for negative values and percent applying

[DasBlub]

+            coeff *= 100.0f;
+            modOwner->ApplySpellMod(spellProto->Id,SPELLMOD_SPELL_BONUS_DAMAGE,coeff);
+            coeff /= 100.0f;

i suggest to do simply this, then you don't need to modify the coeff variable:

modOwner->ApplySpellMod(spellProto->Id,SPELLMOD_SPELL_BONUS_DAMAGE,coeff*100);

[ApoC]

But coeff is passed by reference into method and modified. With your change it is not.

[DasBlub]

But coeff is passed by reference into method and modified. With your change it is not.

oh, ok, sorry =/

i haven't known that about the function, i just saw the diff and wondered why it is done that way.

in that case: ignore my prev. post

[LickedLurk]

Is it working? or are there some bugs or assues ?

I cant test it for myself yet.. but i will

[darkstalker]

yes it works, i tested this a lot before posting it here

maybe the patch could be outdated, but i don't think this part of the code changed

[Ranger]

bump

works perfect

[darkstalker]

been using this for months and its perfect.

[Forusim]

devs please review

[laise]

strange: when using spell bonus data LvlPenalty is part of coeff but in default calculation its not..

[darkstalker]

its copied from existing code, all i did just was correctly implement the spellmod

[laise]

its copied from existing code, all i did just was correctly implement the spellmod

yea but in default calculation (not spell bonus data related part), u haven't added LvlPenalty to coeff unlike it is where coeff taken from db.. so I wanted to know if LvlPenalty should be part of coeff or not - cause it shouldn't be different for db spell bonus and default cases

[Wowka321]

Yes, with patch now working fine ))

[darkstalker]

yea lvlpenalty should be there in both cases, didn't notice that before.

And maybe we should take coeff from DBC for default case instead of calculating it, but thats really another subject.

[laise]

in [9900]. Thanks =)