Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[3.3.5] DoT damage bonus not working #14817

Closed
jackpoz opened this issue May 31, 2015 · 10 comments
Closed

[3.3.5] DoT damage bonus not working #14817

jackpoz opened this issue May 31, 2015 · 10 comments

Comments

@jackpoz
Copy link
Member

jackpoz commented May 31, 2015

Casting Corruption http://wotlk.openwow.com/spell=47813 with Improved Corruption http://wotlk.openwow.com/spell=17814 doesn't add any bonus damage. I used a fresh new lvl80 character with just Corruption rank 10 learnt and Improved Corruption rank 5 on the test dummies at Stormwing (same result with all dummies), doing always 180 dmg per tick

03a5d6d#commitcomment-11443669

Definitely related to #14812

@Keader
Copy link
Member

Keader commented May 31, 2015

@jackpoz
Copy link
Member Author

jackpoz commented May 31, 2015

#14812 (comment) confirmed this bug. It seems also that equipping some raid gear there is a low bonus dmg of 1.6%

@robinsch
Copy link
Contributor

robinsch commented Jun 1, 2015

EDITED!

From 5012ff51214e8f12242d8c7ad23280b778a17b56 Mon Sep 17 00:00:00 2001
Date: Mon, 1 Jun 2015 14:39:16 +0200
Subject: [PATCH] #14817

---
 src/server/game/Entities/Unit/Unit.cpp            | 22 ++++++++++++++--------
 src/server/game/Spells/Auras/SpellAuraEffects.cpp |  6 ++++++
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/server/game/Entities/Unit/Unit.cpp b/src/server/game/Entities/Unit/Unit.cpp
index 6be3d65..d290b0a 100644
--- a/src/server/game/Entities/Unit/Unit.cpp
+++ b/src/server/game/Entities/Unit/Unit.cpp
@@ -9949,11 +9949,14 @@ uint32 Unit::SpellDamageBonusDone(Unit* victim, SpellInfo const* spellProto, uin
         DoneTotal += int32(DoneAdvertisedBenefit * coeff * factorMod);
     }

-    // Done Percentage for DOT is already calculated, no need to do it again. The percentage mod is applied in Aura::HandleAuraSpecificMods.
     float tmpDamage = (int32(pdamage) + DoneTotal) * (damagetype == DOT ? 1.0f : SpellDamagePctDone(victim, spellProto, damagetype));
-    // apply spellmod to Done damage (flat and pct)
-    if (Player* modOwner = GetSpellModOwner())
-        modOwner->ApplySpellMod(spellProto->Id, damagetype == DOT ? SPELLMOD_DOT : SPELLMOD_DAMAGE, tmpDamage);
+    // SPELLMOD_DOT will be applied in AuraEffect::HandlePeriodicDamageAurasTick.
+    if (damagetype == SPELL_DIRECT_DAMAGE)
+    {
+        // apply spellmod to Done damage (flat and pct)
+        if (Player* modOwner = GetSpellModOwner())
+            modOwner->ApplySpellMod(spellProto->Id, SPELLMOD_DAMAGE, tmpDamage);
+    }

     return uint32(std::max(tmpDamage, 0.0f));
 }
@@ -10793,11 +10796,14 @@ uint32 Unit::SpellHealingBonusDone(Unit* victim, SpellInfo const* spellProto, ui
             DoneTotal = 0;
     }

-    // Done Percentage for DOT is already calculated, no need to do it again. The percentage mod is applied in Aura::HandleAuraSpecificMods.
     float heal = float(int32(healamount) + DoneTotal) * (damagetype == DOT ? 1.0f : SpellHealingPctDone(victim, spellProto));
-    // apply spellmod to Done amount
-    if (Player* modOwner = GetSpellModOwner())
-        modOwner->ApplySpellMod(spellProto->Id, damagetype == DOT ? SPELLMOD_DOT : SPELLMOD_DAMAGE, heal);
+    // SPELLMOD_DOT will be applied in AuraEffect::HandlePeriodicHealAurasTick.
+    if (damagetype == SPELL_DIRECT_DAMAGE)
+    {
+        // apply spellmod to Done amount
+        if (Player* modOwner = GetSpellModOwner())
+            modOwner->ApplySpellMod(spellProto->Id, SPELLMOD_DAMAGE, heal);
+    }

     return uint32(std::max(heal, 0.0f));
 }
diff --git a/src/server/game/Spells/Auras/SpellAuraEffects.cpp b/src/server/game/Spells/Auras/SpellAuraEffects.cpp
index 34024d7..d2df5df 100644
--- a/src/server/game/Spells/Auras/SpellAuraEffects.cpp
+++ b/src/server/game/Spells/Auras/SpellAuraEffects.cpp
@@ -5845,6 +5845,9 @@ void AuraEffect::HandlePeriodicDamageAurasTick(Unit* target, Unit* caster) const
         else
             damage = std::max(int32(damage * GetDonePct()), 0);

+        if (Player* modOwner = caster->GetSpellModOwner())
+            modOwner->ApplySpellMod(GetSpellInfo()->Id, SPELLMOD_DOT, damage);
+
         damage = target->SpellDamageBonusTaken(caster, GetSpellInfo(), damage, DOT, GetBase()->GetStackAmount());

         // Calculate armor mitigation
@@ -6148,6 +6151,9 @@ void AuraEffect::HandlePeriodicHealAurasTick(Unit* target, Unit* caster) const
         else
             damage = std::max(int32(damage * GetDonePct()), 0);

+        if (Player* modOwner = caster->GetSpellModOwner())
+            modOwner->ApplySpellMod(GetSpellInfo()->Id, SPELLMOD_DOT, damage);
+
         damage = target->SpellHealingBonusTaken(caster, GetSpellInfo(), damage, DOT, GetBase()->GetStackAmount());
     }

-- 
1.9.5.msysgit.1

@Shauren
Copy link
Member

Shauren commented Jun 1, 2015

@robinsch This is partially what I had in mind with fixing it but your patch is not complete. ApplySpellMod is still called for !isAreaAura much earlier in the process but using base value 0 (and 0 * anything is still 0). This is a major problem if the spellmod has limited charges, you would consume them twice.
My proposal is to remove applying SPELLMOD_DOT entirely from SpellDamageBonusDone/SpellHealingBonusDone and just apply it unconditionally in AuraEffect::HandlePeriodic__Tick, just before the call to target->Spell__BonusTaken

@robinsch
Copy link
Contributor

robinsch commented Jun 1, 2015

EDITED:
You are right.

From 5012ff51214e8f12242d8c7ad23280b778a17b56 Mon Sep 17 00:00:00 2001
Date: Mon, 1 Jun 2015 14:39:16 +0200
Subject: [PATCH] #14817

---
 src/server/game/Entities/Unit/Unit.cpp            | 22 ++++++++++++++--------
 src/server/game/Spells/Auras/SpellAuraEffects.cpp |  6 ++++++
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/server/game/Entities/Unit/Unit.cpp b/src/server/game/Entities/Unit/Unit.cpp
index 6be3d65..d290b0a 100644
--- a/src/server/game/Entities/Unit/Unit.cpp
+++ b/src/server/game/Entities/Unit/Unit.cpp
@@ -9949,11 +9949,14 @@ uint32 Unit::SpellDamageBonusDone(Unit* victim, SpellInfo const* spellProto, uin
         DoneTotal += int32(DoneAdvertisedBenefit * coeff * factorMod);
     }

-    // Done Percentage for DOT is already calculated, no need to do it again. The percentage mod is applied in Aura::HandleAuraSpecificMods.
     float tmpDamage = (int32(pdamage) + DoneTotal) * (damagetype == DOT ? 1.0f : SpellDamagePctDone(victim, spellProto, damagetype));
-    // apply spellmod to Done damage (flat and pct)
-    if (Player* modOwner = GetSpellModOwner())
-        modOwner->ApplySpellMod(spellProto->Id, damagetype == DOT ? SPELLMOD_DOT : SPELLMOD_DAMAGE, tmpDamage);
+    // SPELLMOD_DOT will be applied in AuraEffect::HandlePeriodicDamageAurasTick.
+    if (damagetype == SPELL_DIRECT_DAMAGE)
+    {
+        // apply spellmod to Done damage (flat and pct)
+        if (Player* modOwner = GetSpellModOwner())
+            modOwner->ApplySpellMod(spellProto->Id, SPELLMOD_DAMAGE, tmpDamage);
+    }

     return uint32(std::max(tmpDamage, 0.0f));
 }
@@ -10793,11 +10796,14 @@ uint32 Unit::SpellHealingBonusDone(Unit* victim, SpellInfo const* spellProto, ui
             DoneTotal = 0;
     }

-    // Done Percentage for DOT is already calculated, no need to do it again. The percentage mod is applied in Aura::HandleAuraSpecificMods.
     float heal = float(int32(healamount) + DoneTotal) * (damagetype == DOT ? 1.0f : SpellHealingPctDone(victim, spellProto));
-    // apply spellmod to Done amount
-    if (Player* modOwner = GetSpellModOwner())
-        modOwner->ApplySpellMod(spellProto->Id, damagetype == DOT ? SPELLMOD_DOT : SPELLMOD_DAMAGE, heal);
+    // SPELLMOD_DOT will be applied in AuraEffect::HandlePeriodicHealAurasTick.
+    if (damagetype == SPELL_DIRECT_DAMAGE)
+    {
+        // apply spellmod to Done amount
+        if (Player* modOwner = GetSpellModOwner())
+            modOwner->ApplySpellMod(spellProto->Id, SPELLMOD_DAMAGE, heal);
+    }

     return uint32(std::max(heal, 0.0f));
 }
diff --git a/src/server/game/Spells/Auras/SpellAuraEffects.cpp b/src/server/game/Spells/Auras/SpellAuraEffects.cpp
index 34024d7..d2df5df 100644
--- a/src/server/game/Spells/Auras/SpellAuraEffects.cpp
+++ b/src/server/game/Spells/Auras/SpellAuraEffects.cpp
@@ -5845,6 +5845,9 @@ void AuraEffect::HandlePeriodicDamageAurasTick(Unit* target, Unit* caster) const
         else
             damage = std::max(int32(damage * GetDonePct()), 0);

+        if (Player* modOwner = caster->GetSpellModOwner())
+            modOwner->ApplySpellMod(GetSpellInfo()->Id, SPELLMOD_DOT, damage);
+
         damage = target->SpellDamageBonusTaken(caster, GetSpellInfo(), damage, DOT, GetBase()->GetStackAmount());

         // Calculate armor mitigation
@@ -6148,6 +6151,9 @@ void AuraEffect::HandlePeriodicHealAurasTick(Unit* target, Unit* caster) const
         else
             damage = std::max(int32(damage * GetDonePct()), 0);

+        if (Player* modOwner = caster->GetSpellModOwner())
+            modOwner->ApplySpellMod(GetSpellInfo()->Id, SPELLMOD_DOT, damage);
+
         damage = target->SpellHealingBonusTaken(caster, GetSpellInfo(), damage, DOT, GetBase()->GetStackAmount());
     }

-- 
1.9.5.msysgit.1

@Shauren
Copy link
Member

Shauren commented Jun 1, 2015

Aaaaaand that is bad too :P you removed too much, float(int32(healamount) + DoneTotal) should be always there (notice its multiplied by 1.0 for DOT, not 0)

@robinsch
Copy link
Contributor

robinsch commented Jun 1, 2015

SpellDamage(Healing)BonusDone is always called with 0. Let me quote you, "base value 0 (and 0 * anything is still 0)." :P

@Shauren
Copy link
Member

Shauren commented Jun 1, 2015

Yes, for !isAreaAura case :P, otherwise its proper damage.
Also i mentioned calling ApplySpellMod just before target->Spell__BonusTaken (after if (isAreaAura) damage = caster->....) block

@robinsch
Copy link
Contributor

robinsch commented Jun 1, 2015

From 5012ff51214e8f12242d8c7ad23280b778a17b56 Mon Sep 17 00:00:00 2001
From: robinsch <robin.schriever.hude@web.de>
Date: Mon, 1 Jun 2015 14:39:16 +0200
Subject: [PATCH] #14817

---
 src/server/game/Entities/Unit/Unit.cpp            | 22 ++++++++++++++--------
 src/server/game/Spells/Auras/SpellAuraEffects.cpp |  6 ++++++
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/server/game/Entities/Unit/Unit.cpp b/src/server/game/Entities/Unit/Unit.cpp
index 6be3d65..d290b0a 100644
--- a/src/server/game/Entities/Unit/Unit.cpp
+++ b/src/server/game/Entities/Unit/Unit.cpp
@@ -9949,11 +9949,14 @@ uint32 Unit::SpellDamageBonusDone(Unit* victim, SpellInfo const* spellProto, uin
         DoneTotal += int32(DoneAdvertisedBenefit * coeff * factorMod);
     }

-    // Done Percentage for DOT is already calculated, no need to do it again. The percentage mod is applied in Aura::HandleAuraSpecificMods.
     float tmpDamage = (int32(pdamage) + DoneTotal) * (damagetype == DOT ? 1.0f : SpellDamagePctDone(victim, spellProto, damagetype));
-    // apply spellmod to Done damage (flat and pct)
-    if (Player* modOwner = GetSpellModOwner())
-        modOwner->ApplySpellMod(spellProto->Id, damagetype == DOT ? SPELLMOD_DOT : SPELLMOD_DAMAGE, tmpDamage);
+    // SPELLMOD_DOT will be applied in AuraEffect::HandlePeriodicDamageAurasTick.
+    if (damagetype == SPELL_DIRECT_DAMAGE)
+    {
+        // apply spellmod to Done damage (flat and pct)
+        if (Player* modOwner = GetSpellModOwner())
+            modOwner->ApplySpellMod(spellProto->Id, SPELLMOD_DAMAGE, tmpDamage);
+    }

     return uint32(std::max(tmpDamage, 0.0f));
 }
@@ -10793,11 +10796,14 @@ uint32 Unit::SpellHealingBonusDone(Unit* victim, SpellInfo const* spellProto, ui
             DoneTotal = 0;
     }

-    // Done Percentage for DOT is already calculated, no need to do it again. The percentage mod is applied in Aura::HandleAuraSpecificMods.
     float heal = float(int32(healamount) + DoneTotal) * (damagetype == DOT ? 1.0f : SpellHealingPctDone(victim, spellProto));
-    // apply spellmod to Done amount
-    if (Player* modOwner = GetSpellModOwner())
-        modOwner->ApplySpellMod(spellProto->Id, damagetype == DOT ? SPELLMOD_DOT : SPELLMOD_DAMAGE, heal);
+    // SPELLMOD_DOT will be applied in AuraEffect::HandlePeriodicHealAurasTick.
+    if (damagetype == SPELL_DIRECT_DAMAGE)
+    {
+        // apply spellmod to Done amount
+        if (Player* modOwner = GetSpellModOwner())
+            modOwner->ApplySpellMod(spellProto->Id, SPELLMOD_DAMAGE, heal);
+    }

     return uint32(std::max(heal, 0.0f));
 }
diff --git a/src/server/game/Spells/Auras/SpellAuraEffects.cpp b/src/server/game/Spells/Auras/SpellAuraEffects.cpp
index 34024d7..d2df5df 100644
--- a/src/server/game/Spells/Auras/SpellAuraEffects.cpp
+++ b/src/server/game/Spells/Auras/SpellAuraEffects.cpp
@@ -5845,6 +5845,9 @@ void AuraEffect::HandlePeriodicDamageAurasTick(Unit* target, Unit* caster) const
         else
             damage = std::max(int32(damage * GetDonePct()), 0);

+        if (Player* modOwner = caster->GetSpellModOwner())
+            modOwner->ApplySpellMod(GetSpellInfo()->Id, SPELLMOD_DOT, damage);
+
         damage = target->SpellDamageBonusTaken(caster, GetSpellInfo(), damage, DOT, GetBase()->GetStackAmount());

         // Calculate armor mitigation
@@ -6148,6 +6151,9 @@ void AuraEffect::HandlePeriodicHealAurasTick(Unit* target, Unit* caster) const
         else
             damage = std::max(int32(damage * GetDonePct()), 0);

+        if (Player* modOwner = caster->GetSpellModOwner())
+            modOwner->ApplySpellMod(GetSpellInfo()->Id, SPELLMOD_DOT, damage);
+
         damage = target->SpellHealingBonusTaken(caster, GetSpellInfo(), damage, DOT, GetBase()->GetStackAmount());
     }

-- 
1.9.5.msysgit.1

@ghost
Copy link

ghost commented Jun 1, 2015

@robinsch : do you want your #14817 (comment) made into a PR now from the patch diff above?

Shauren pushed a commit that referenced this issue Jun 1, 2015
Closes #14817

Signed-off-by: Shauren <shauren.trinity@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants