[patch] Level Penalty for not-ranked spells

[Auntie Mangos]

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

- It should fix spell like Unstable affliction dispel trigger, which suffer from the level penalty and doesn't gain proper amount of spell power bonus.

For which repository revision was the patch created?

Rev. 8578

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

It was mentioned in : http://getmangos.eu/community/viewtopic.php?id=5937

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

Me, reeshack

Paste2.org :

V1 : http://paste2.org/p/477024

V2 : http://paste2.org/p/476974

Quotation from wowwiki :" Downranking is the process of using a lower than maximum spell rank in order to gain higher efficiency per mana. "

That means if there isn't specified rank of the spell (or there is only one), it should not get any level penalty. Currently the calculation of level penalty is based only on spell level, which some spells, like f.e. Unstable Affliction dispel trigger, has 1 and therefore they are treated in a wrong way and gain very low spell power bonus.

[reeshack]

First post updated due to some new information - I realized that some spells that has only one rank were still wrongly a subject to level penalty (like Judgement of Righteousness).

So after a little research I've decided to use SPELL_ATTR_EX2_CANT_REFLECTED which is currently used to determine some spell from being reflected - in my opinion wrongly because some of them (like Holy Vengeance, some Judgements and probably a lot of other) should be able to be reflected.

Unfortunately I don't know how to select all spells specified by this bitmask so I couldn't find out if they are all not-or-only-one-ranked (but till now I haven't found a spell with this flag which should be a subject of downranking and also all of the spells I know that are wrongly treated has only this attribut in common).

I'm also thinking about removing the level check because it seems to me unneeded now - but as I've said I don't have complete information .

Please if somebody gave me an advice in sorting and verifing the data I would be very grateful

[pasdVn]

Hmm, would be really a nice catch if this bit is correct.

I usually do such researches quite unprofessional: Simply make a loop over all spells in the code anywhere you like (e.g. a spelleffect) and print out all spellids that have this flag.

[reeshack]

Thank you a lot pasdVn

OK and now some results:

- I separated all the spells using this bitmask - here is the list: http://paste2.org/p/479570. As you can see it is about 6,5k spells.

- Then I used the rank check from V1 of the patch and I got 63 spells which are ranked. Those are mostly triggered spells like Arcane Missiles tick damage etc. At that time I realized that if a spell has only one rank, the GetSpellRank() function returns it as Rank 0 (probably because it is not a part of any SpellChain). The list: http://paste2.org/p/479574

In conlusion I'm quite sure that this spell mask is not used to prevent some spells from being reflected, but I can't say certainly that it is used for ignoring level penalty because of those 63 spells which have rank and also correct level. Also, as I said , I've realized that the V1 also covers spells with only one rank (because of which I was doing V2 ).

[DasMy]

Hello out there ,

I tried to apply your patch, but had to find out, that SPELL_ATTR_EX2_CANT_REFLECTED is now being used in several places in code. Hence removing it is not that simple.

I would propose a solution like http://paste2.org/p/520649.

Using mSpellChainMapNext, that is already being initialized on spell-chain load (but afaik used nowhere until now in mangos-0.12), it is checked, whether a higher-rank spell exists. If this is not the case, downranking is prevented. -- Hopefully, I understood things correctly with mSpellChainMapNext :lol:

I am looking forward to your comments.

DasMy

Here again for a quick look:

fadc0020a4696443d1e995303e1b6d201f18a4fb
src/game/SpellMgr.h |   10 ++++++++++
src/game/Unit.cpp   |    3 +++
2 files changed, 13 insertions(+), 0 deletions(-)

diff --git a/src/game/SpellMgr.h b/src/game/SpellMgr.h
index fd5bca1..c5b1d24 100644
--- a/src/game/SpellMgr.h
+++ b/src/game/SpellMgr.h
@@ -740,6 +740,16 @@ class SpellMgr
            return false;
        }

+        uint8 IsHighestRankOfSpell(uint32 spell) const
+        {
+            SpellChainMapNext::const_iterator itr = mSpellChainsNext.find(spell);
+
+            if (!(itr == mSpellChainsNext.end()) && (itr->second)) // the spell is in the chain list and a higher-rank spell is available
+                return false;
+            else
+                return true;
+        }
+
        bool IsRankSpellDueToSpell(SpellEntry const *spellInfo_1,uint32 spellId_2) const;
        static bool canStackSpellRanks(SpellEntry const *spellInfo);
        bool IsNoStackSpellDueToSpell(uint32 spellId_1, uint32 spellId_2) const;
diff --git a/src/game/Unit.cpp b/src/game/Unit.cpp
index 017a9be..de7eef4 100644
--- a/src/game/Unit.cpp
+++ b/src/game/Unit.cpp
@@ -2594,6 +2594,9 @@ float Unit::CalculateLevelPenalty(SpellEntry const* spellProto) const
    if(spellProto->spellLevel <= 0)
        return 1.0f;

+    if (sSpellMgr.IsHighestRankOfSpell(spellProto->Id))
+        return 1.0f;
+
    float LvlPenalty = 0.0f;

    if(spellProto->spellLevel < 20)

[reeshack]

Firstly, thank you for your interest

Well "SPELL_ATTR_EX2_CANT_REFLECTED" has been used in a commit, which has been done after the patch and I haven't updated the V2 yet - and I probably won't do because the matter with the attribut is really very unsure and strange, as the the test results say. Just the fact that it has been used again in the master branch tells me that I'm most likely wrong

To your patch - I haven't tested it yet, but it seems to me as a good alternative to my patch

[DasMy]

Updated my Post: renamed spellmgr to sSpellMgr.

Regards, DasMY

[Tassader2]

Note that, at some point beween 2.2 and wotlk, the downranking formula has been changed. AFAIK Mangos still uses old formula.

Ghostcrawler made the following statement about the downranking formula:

If caster level is less than max caster level, then the penalty = 1.0.

If caster level is at or greater than max caster level, then the penalty = (22 + max level - caster level) / 20.

The penalty is capped at 0. Max caster level per spell is also pretty inconsistent, but it's around 4 levels higher than the level at which you get the spell.

Basically, once the caster is 3 levels higher than the max cast level of the spell, the standard coefficient of the spell is multiplied by that penalty (losing 5% of the coefficient per level), with the coefficient reaching zero when the player is 22 levels higher than the max cast level.

[DasMy]

Well, we could include the new formula, but in my eyes it would be better, to do it in another patch, since

1) its some different issue

2) its a version issue: someon, who is running an older server may want to stick to the old formula but wants do get rid of the problem that is adressed by the actual patch.

[bulek]

well done guys

[Tassader2]

Well, we could include the new formula, but in my eyes it would be better, to do it in another patch, since

1) its some different issue

2) its a version issue: someon, who is running an older server may want to stick to the old formula but wants do get rid of the problem that is adressed by the actual patch.

Well, yes, I just thought that there is (although a very little) chance that this bug gets magically fixed by switching to new formula (and using max level instead of spell level)

[reeshack]

This is a good point, f.e. Unstable Affliction trigger has maxlevel = 0, which would exclude it from being "penalted". I will try to implement the new formula, it seems that it really is related to this problem

[reeshack]

So, here it is .

Hopefully it is right in this way, I did it according to Ghostcrawler's statement, I also tested it a bit and it seems to work allright.

diff --git a/src/game/Unit.cpp b/src/game/Unit.cpp
index c45f8cb..31f2825 100644
--- a/src/game/Unit.cpp
+++ b/src/game/Unit.cpp
@@ -2367,18 +2353,23 @@ uint32 Unit::CalculateDamage (WeaponAttackType attType, bool normalized)

float Unit::CalculateLevelPenalty(SpellEntry const* spellProto) const
{
-    if(spellProto->spellLevel <= 0)
+    if(spellProto->maxLevel <= 0)
+        return 1.0f;
+    //if caster level is lower that max caster level
+    if(getLevel() < spellProto->maxLevel)
        return 1.0f;

    float LvlPenalty = 0.0f;

-    if(spellProto->spellLevel < 20)
-        LvlPenalty = 20.0f - spellProto->spellLevel * 3.75f;
-    float LvlFactor = (float(spellProto->spellLevel) + 6.0f) / float(getLevel());
-    if(LvlFactor > 1.0f)
-        LvlFactor = 1.0f;
+    LvlPenalty = (22.0f + float (spellProto->maxLevel) - float (getLevel())) / 20.0f;
+    //to prevent positive effect
+    if(LvlPenalty > 1.0f)
+        return 1.0f;
+    //level penalty is capped at 0
+    if(LvlPenalty < 0.0f)
+        return 0.0f;

-    return (100.0f - LvlPenalty) * LvlFactor / 100.0f;
+    return LvlPenalty;
}

void Unit::SendMeleeAttackStart(Unit* pVictim)

Paste2.org: http://paste2.org/p/549177

[dogeht]

anyone tested it?

[Wojta]

Yeah, works perfect

[lu1gui]

Working as intended