[FIX][8260]Shaman Talent - Improved Stromstike

[HiddenDweller]

What does this patch do:

The patch fixes an error when improved stromstrike (rank 1 , rank 2)gives 20 mana instead of 20% of the caster's mana. The patch checks for improved stromstrike when Energize effect is activated and replaces 20 with 20%.

For which repository revision was the patch created?

tested on 8252.

Is there a thread in the bug report section or at lighthouse? If yes, please add a link to the thread.

no.

Who has been writing this patch? Please include either forum user names or email addresses.

only me - HiddenDweller

diff --git a/src/game/SpellEffects.cpp b/src/game/SpellEffects.cpp
index 7a6ed0c..ab2d1c8 100644
--- a/src/game/SpellEffects.cpp
+++ b/src/game/SpellEffects.cpp
@@ -2888,6 +2888,12 @@ void Spell::EffectEnergize(uint32 i)
    if(unitTarget->GetMaxPower(power) == 0)
        return;

+	//Improved Stormstrike
+	if (m_spellInfo->Id == 63375)
+	{
+		damage = unitTarget->GetMaxPower(power)*damage/100;
+	}
+
    m_caster->EnergizeBySpell(unitTarget, m_spellInfo->Id, damage, power);

    // Mad Alchemist's Potion
-- 

[nos4r2zod]

Improved Stormstrike returns 20% of base mana, not total mana. So GetCreateMana() or GetCreatePowers(power) should be used, not GetMaxPower.

Out of personal preference, I would also place the custom basepoint calculation in the the proc aura, since other procs are handled that way:

diff --git a/src/game/Unit.cpp b/src/game/Unit.cpp
index 9f37a13..448753e 100644
--- a/src/game/Unit.cpp
+++ b/src/game/Unit.cpp
@@ -6879,6 +6879,12 @@ bool Unit::HandleProcTriggerSpell(Unit *pVictim, uint32 damage, Aura* triggeredB
                return false;
            break;
        }
+        // Improved Stormstrike
+        case 63375:
+        {
+            basepoints0 = int32(GetCreateMana() * 20 / 100);
+            break;
+        }
    }

    if( cooldown && GetTypeId()==TYPEID_PLAYER && ((Player*)this)->HasSpellCooldown(trigger_spell_id))

[HiddenDweller]

You're right about GetCreateMana. The place is also better. Though, 20 should still be damage, to prevent changes in talent from ruining the fix.

Somewhy I am getting strange random amounts around 40% when using damage instead of 20..

[nos4r2zod]

Though, 20 should still be damage, to prevent changes in talent from ruining the fix. Also, shouldn't it be target->getCreateMana() and not only getCreateMana()?

The unit that calls Unit::HandleProcTriggerSpell is the unit with the aura, or the shaman. So target-> is not necessary.

The flip side of putting it there though is that damage in that context is the amount of damage done by the attack; the Spell object for the triggered spell hasn't be created yet, and Spell::CalculateDamage hasn't been called yet (which is where you get damage = 20 in your case). That's why I point out that that's just my personal preference... as I'm not completely sure what MaNGOS would prefer.

[HiddenDweller]

Oh, that exaplins it. Well, then I think we should leave it in SpellEffects.cpp to not hardcode it being 20%, as there is no way to do it in Unit.cpp.

that leaves us with:

diff --git a/src/game/SpellEffects.cpp b/src/game/SpellEffects.cpp
index 7a6ed0c..ab2d1c8 100644
--- a/src/game/SpellEffects.cpp
+++ b/src/game/SpellEffects.cpp
@@ -2888,6 +2888,12 @@ void Spell::EffectEnergize(uint32 i)
    if(unitTarget->GetMaxPower(power) == 0)
        return;

+	//Improved Stormstrike
+	if (m_spellInfo->Id == 63375)
+	{
+		damage = unitTarget->GetCreateMana()*damage/100;
+	}
+
    m_caster->EnergizeBySpell(unitTarget, m_spellInfo->Id, damage, power);

    // Mad Alchemist's Potion
-- 

On a side note: If there is any way to make that spell (63375) to be handled by Spell::EffectEnergisePct rather then by Spell::EffectEnergise, it would be a more correct solution, I just can't seem to find a way to do that.

[thenecromancer]

No, it should be in trigger handler, just use triggerammount or whatever is ammount of triggering aura called ( it will be get from m_ammount of triggeredbyaura )

[ApoC]

Applied in [8260]

Thank You

[nos4r2zod]

Just use triggerammount or whatever is ammount of triggering aura called ( it will be get from m_ammount of triggeredbyaura )

I looked in the dbc and found no basepoints value for the auras that proc Improved Stormstrike. I compiled it this way before 8260, and I compiled it afterwards just to make sure; the triggeredAmount is zero, and this patch will not work like this.

Edit: Here is the general rule for whether triggerAmount can be used:

1) The proc aura will have basepoints. If this is the case, then when you look on wowhead, you will clearly see that it has data for value: Blessed Recovery, Nature's Guardian, Bloodthirst

2) The proc aura itself has no basepoints, but it is modified by another spell that increases the value: Leader of the Pack's health restore trigger (Effect 2), Improved Leader of the Pack

For reference, here is Improved Stormstrike

[HiddenDweller]

Thanks for applying, though you applied it with

basepoints0 = int32(GetCreateMana() * triggerAmount / 100);

which obliously does not work.

So, this commit has to be reverted and either mine second fix or nos4r2zod's fix applied

[VladimirMangos]

Fixed in [8266]