[fix][8901] Sudden Death - Rage management

[Auntie Mangos]

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

Sudden death rage management.

For which repository revision was the patch created?

8896

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

Well, there are many threads about sudden death, but I did not find any about this problem

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

Me. Part of my Aura 262 patch.

diff --git a/src/game/SpellEffects.cpp b/src/game/SpellEffects.cpp
index 614175b..957adc3 100644
--- a/src/game/SpellEffects.cpp
+++ b/src/game/SpellEffects.cpp
@@ -1435,6 +1435,7 @@ void Spell::EffectDummy(uint32 i)
                    return;

                uint32 rage = m_caster->GetPower(POWER_RAGE);
+                uint32 lastrage=0;

                // up to max 30 rage cost
                if(rage > 30)
@@ -1450,7 +1451,19 @@ void Spell::EffectDummy(uint32 i)
                                                 m_caster->GetTotalAttackPowerValue(BASE_ATTACK)*0.2f);

                m_caster->CastCustomSpell(unitTarget, 20647, &basePoints0, NULL, NULL, true, 0);
-                m_caster->SetPower(POWER_RAGE, m_caster->GetPower(POWER_RAGE)-rage);
+
+                //Sudden Death
+                if(m_caster->HasAura(52437))
+                {
+                    if(m_caster->HasAura(29723)) lastrage=3;
+                    else if (m_caster->HasAura(29725)) lastrage=7;
+                    else if (m_caster->HasAura(29724)) lastrage=10;
+
+                    if(lastrage < rage)
+                        rage -= lastrage;
+                }
+
+                m_caster->SetPower(POWER_RAGE,m_caster->GetPower(POWER_RAGE)-rage);
                return;
            }
            // Slam

[darkstalker]

the remaining rage value is stored in EffectBasePoints[1]

SELECT Id,SpellName,Rank,EffectBasePoints1+1 FROM `spell` WHERE Id IN (29723,29725,29724);

29723      Sudden Death      Rank 1      3
29725     Sudden Death     Rank 2     7
29724     Sudden Death     Rank 3     10

...

always when i see a bunch of HasAura()'s i wonder why not implement something like GetTalentRank(), talents dont change continuoulsy so can be a constant time lookup

[Lynx3d]

Hm i had the impression 1 rage gets stored as 10 "energy units", for whatever reason (guess only people dealing with heavy snowfall know...).

But that would mean the current limitation for 30 rage on execute is implemented wrong...maybe will try later if execute (without talents/glyphs) currently even works as intended.

[Wojta]

to lynx3d: no, execute works fine, i tested this.

to darkstalker: so you suggest to implent this with getAurasByType()? Maybe i am not enought experienced, but i dont know why its better. please, explain.

[darkstalker]

something like this

Unit::AuraList const& auras = m_caster->GetAurasByType(SPELL_AURA_PROC_TRIGGER_SPELL);
for (Unit::AuraList::const_iterator it = auras.begin(); it != auras.end(); it++)
   if ((*it)->GetId() == 29723 || (*it)->GetId() == 29724 || (*it)->GetId() == 29725)
   {
       lastrage = (*it)->GetSpellProto()->EffectBasePoints[1] + 1;
       break;
   }

[Lynx3d]

Hm seriously i went through all code again, and tested ingame, and really think internally 1 rage is 10 power units.

My executes never actually take 30 rage, only up to 19. Bonus damage also seems close to non-existent.

First hint: "manaCost" of Execute is 150, not 15.

Second hint: bonus damage is calculated as:

rage_modified * m_spellInfo->DmgMultiplier

Execute Rank9 reads: "converting each extra point of rage into 38 additional damage (up to a maximum cost of 30 rage)"

But m_spellInfo->DmgMultiplier for spell 47471 is 3.8, not 38.

So currently you effectively get at most 3 rage converted to bonus damage. But there's more wrong:

It says "extra point of rage", that is, max. 30 rage total minus the cost of Execute itself (15 non-talented) so that is 15 extra points of rage that are multiplied by 38.

Furthermore, if i'm not totally wrong, the rage cost of Execute has already been taken when calling Spell::EffectDummy.

In other words, in my opinion current code in SpellEffects.cpp, line 1439 onwards should read:

// up to max 30 rage cost
if(rage > 300 - m_powerCost)
rage = 300 - m_powerCost;

That's what i already had proposed ages ago in my first Sudden Death patch attempt, and i still think it's valid, only that the 30 rage limit now always applies, SD proc or not.

[VladimirMangos]

lastrage = (*it)->GetSpellProto()->EffectBasePoints[1] + 1;

Always better use GetSpellProto()->CalculateSimpleValue(1) instead explicit access to EffectBasePoints

+1 is not something _always_case, this another field value that sometime 0 for example...

[VladimirMangos]

to lynx3d: no, execute works fine, i tested this.

to darkstalker: so you suggest to implent this with getAurasByType()? Maybe i am not enought experienced, but i dont know why its better. please, explain.

Because each HasAura is access 1-3 times access to std::map with all auras (and you have 3 like cases = 3-9 lookups

Loop by auratypelist is 1 loop by aura list with possible 1-2.. elements

[VladimirMangos]

With suggested and additional changes in [8901]. Thank you

[DaemonCantor]

If this makes my Warrior Nerf kill correctly I'll be a verry happy Propellerhead!

[VladimirMangos]

Note: as written in first post, this fix only rage return, and not fix remove req. for health level for target

[thenecromancer]

By the way, was not always rage 10x higher than displayed value? (eg. 10 rage returns as 100 in GetPower(POWER_RAGE)...)

[Lynx3d]

I was trying to point that out in post #6...

GetMaxPower(POWER_RAGE) definitely returns 1000, not 100. I just tried again just to be really sure now i'm not telling crap...

I'll also repeat that i believe the rage cost for Execute is already taken when calling Spell::EffectDummy.

And on a second look, i even think the refund logic here is flawed...

"if(lastrage < rage)" does not give any hint about the value of "m_caster->GetPower(POWER_RAGE)-rage" which must not be smaller than "lastrage" due to the talent.

[VladimirMangos]

By the way, was not always rage 10x higher than displayed value? (eg. 10 rage returns as 100 in GetPower(POWER_RAGE)...)

You are right, fixed in [8915]