[fix] isPositiveEffect, periodic triggering auras

[Auntie Mangos]

IsPositiveEffect() currently does not return the correct value for periodic triggering auras in every cases. The logic used is based on that fact, that normally the dummy aura, that is used in all class spells - using any periodic triggering aura - is the first spell effect (maybe see more here to this issue: http://getmangos.eu/community/viewtopic.php?id=8653). So while iterating over all effects this dummy aura (with a !IsPositiveTarget) is found first and the whole spell is considered as a negative.

The problem is that the ordering of the different spelleffects is not always like that (dummy aura at first spell effect). Especially "Mind Flay" (http://www.wowhead.com/?spell=15407).

So in the end this causes a bug, that you can not cast a high rank of mind flay on a low level mob because of such an iteration in checkcast().

Here is my suggestion to make it work correct:

--old patch removed--

One additional note:

IsPositiveTarget() always returns false for TARGET_SINGLE_ENEMY (that is used by those periodic triggered spells), which is not always correct (e.g. "Penance"), but as far as I can evaluate that, it won't cause problems for now at no place in the code (IsPositiveEffect, canReflect).

-------------------------------------------------

Edit 30.07.09:

As I discovered with the help of nos4r2zod, I understood IsPositiveEffect() the wrong way. Periodic triggering auras are positive, if they are at the caster and cast their spells on the (harmful) target. So the logic used is in fact right besides a typo :-/.

The real problem is, that in CheckCast() and SelectAuraRankForPlayerLevel() the client provided unit target is used, to check, if the spells level fits to the targets level. The problem is, that this unit target is not the correct target in every case. For example with those periodic trigggering auras, the triggering aura has TARGET_SELF (EffectImplicitTarget), so it does not have to do anything with the unit target. So we get this "SPELL_FAILED_LOWLEVEL" here if "Mindflay" is casted on a low level target.

If there wasn't this above mentioned typo, "Mindflay" would just be downranked in SelectAuraRankForPlayerLevel() bout would work at least^^. Other similar spells like "Arcane Missiles" won't suffer of this problem because they have -whyever- SPELL_ATTR_EX_NEGATIVE, so they are always considere as negative. Maybe it has to do with ordering of the spelleffects, that blizz does not need this flag in it's own code, ... I don't know^^

Anyway, here is a piece of code in witch I tried to seperate witch implicit target types are using the client provided unit target an which don't (and fix the typo, of course). In fact I don't like it to add one more of this helper function to the spellmanager, but this is the most generic way I found for now :-/

From ad16688f1066e96525c97dd8d7fa2eaff86bff04 Mon Sep 17 00:00:00 2001
From: pasdVn <[email protected]>
Date: Sun, 26 Jul 2009 12:12:31 +0200
Subject: [PATCH] fix in checkcast

* prevent using the client sended unit target for spelleffects using an independent implicit target
---
src/game/Spell.cpp    |    4 +++-
src/game/SpellMgr.cpp |   29 +++++++++++++++++++++++++++--
src/game/SpellMgr.h   |    2 ++
3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/src/game/Spell.cpp b/src/game/Spell.cpp
index cdd2247..9e98f7c 100644
--- a/src/game/Spell.cpp
+++ b/src/game/Spell.cpp
@@ -3754,7 +3754,9 @@ SpellCastResult Spell::CheckCast(bool strict)
            // this case can be triggered if rank not found (too low-level target for first rank)
            if(m_caster->GetTypeId() == TYPEID_PLAYER && !IsPassiveSpell(m_spellInfo->Id) && !m_CastItem)
                for(int i=0;i<3;++i)
-                    if(IsPositiveEffect(m_spellInfo->Id, i) && m_spellInfo->Effect[i] == SPELL_EFFECT_APPLY_AURA)
+                    if( IsPositiveEffect(m_spellInfo->Id, i) &&
+                        IsUnitTargetUsingTarget(m_spellInfo->EffectImplicitTargetA[i], m_spellInfo->EffectImplicitTargetB[i]) &&
+                        m_spellInfo->Effect[i] == SPELL_EFFECT_APPLY_AURA )
                        if(target->getLevel() + 10 < m_spellInfo->spellLevel)
                            return SPELL_FAILED_LOWLEVEL;
        }
diff --git a/src/game/SpellMgr.cpp b/src/game/SpellMgr.cpp
index 86816e5..b0d3080 100644
--- a/src/game/SpellMgr.cpp
+++ b/src/game/SpellMgr.cpp
@@ -292,6 +292,29 @@ bool IsPositiveTarget(uint32 targetA, uint32 targetB)
    return true;
}

+bool IsUnitTargetUsingTarget(uint32 targetA, uint32 targetB)
+{
+    switch(targetA)
+    {
+        case TARGET_CHAIN_DAMAGE:
+        case TARGET_CHAIN_HEAL:
+        case TARGET_SINGLE_FRIEND:
+        case TARGET_SINGLE_FRIEND_2:
+        case TARGET_SINGLE_PARTY:
+        case TARGET_NONCOMBAT_PET:
+        case TARGET_DUELVSPLAYER:
+        case TARGET_AREAEFFECT_PARTY_AND_CLASS:
+        case TARGET_CURRENT_ENEMY_COORDINATES:
+        case TARGET_BEHIND_VICTIM:
+            return true;
+        default:
+            break;
+    }
+    if (targetB)
+            return IsUnitTargetUsingTarget(targetB,0);
+        return false;
+}
+
bool IsPositiveEffect(uint32 spellId, uint32 effIndex)
{
    SpellEntry const *spellproto = sSpellStore.LookupEntry(spellId);
@@ -358,6 +381,7 @@ bool IsPositiveEffect(uint32 spellId, uint32 effIndex)
                case SPELL_AURA_ADD_TARGET_TRIGGER:
                    return true;
                case SPELL_AURA_PERIODIC_TRIGGER_SPELL:
+                case SPELL_AURA_PERIODIC_TRIGGER_SPELL_WITH_VALUE:
                    if(spellId != spellproto->EffectTriggerSpell[effIndex])
                    {
                        uint32 spellTriggeredId = spellproto->EffectTriggerSpell[effIndex];
@@ -370,7 +394,7 @@ bool IsPositiveEffect(uint32 spellId, uint32 effIndex)
                            {
                                // if non-positive trigger cast targeted to positive target this main cast is non-positive
                                // this will place this spell auras as debuffs
-                                if(IsPositiveTarget(spellTriggeredProto->EffectImplicitTargetA[effIndex],spellTriggeredProto->EffectImplicitTargetB[effIndex]) && !IsPositiveEffect(spellTriggeredId,i))
+                                if(IsPositiveTarget(spellTriggeredProto->EffectImplicitTargetA[i],spellTriggeredProto->EffectImplicitTargetB[i]) && !IsPositiveEffect(spellTriggeredId,i))
                                    return false;
                            }
                        }
@@ -1577,7 +1601,8 @@ SpellEntry const* SpellMgr::SelectAuraRankForPlayerLevel(SpellEntry const* spell
    bool needRankSelection = false;
    for(int i=0;i<3;++i)
    {
-        if( IsPositiveEffect(spellInfo->Id, i) && (
+        if( IsPositiveEffect(spellInfo->Id, i) &&
+            IsUnitTargetUsingTarget(spellInfo->EffectImplicitTargetA[i], spellInfo->EffectImplicitTargetB[i]) && (
            spellInfo->Effect[i] == SPELL_EFFECT_APPLY_AURA ||
            spellInfo->Effect[i] == SPELL_EFFECT_APPLY_AREA_AURA_PARTY ||
            spellInfo->Effect[i] == SPELL_EFFECT_APPLY_AREA_AURA_RAID
diff --git a/src/game/SpellMgr.h b/src/game/SpellMgr.h
index a97515f..3d27b4c 100644
--- a/src/game/SpellMgr.h
+++ b/src/game/SpellMgr.h
@@ -191,6 +191,8 @@ bool IsPositiveSpell(uint32 spellId);
bool IsPositiveEffect(uint32 spellId, uint32 effIndex);
bool IsPositiveTarget(uint32 targetA, uint32 targetB);

+bool IsUnitTargetUsingTarget(uint32 targetA, uint32 targetB);
+
bool IsSingleTargetSpell(SpellEntry const *spellInfo);
bool IsSingleTargetSpells(SpellEntry const *spellInfo1, SpellEntry const *spellInfo2);

-- 
1.6.0.2.1172.ga5ed0

Github: http://github.com/pasdVn/mangos/commit/c98e43e12207fe697e2812d211c13aa638535f93.patch

Sorry for writing that much to such a small problem. Just tried to make my thougts a bit reproducable

[nos4r2zod]

I like the idea, but this is making Druid's Starfall appear as a debuff. So you still might need to include part of the targeting check.

[pasdVn]

Ahh, thank you very much!

I did not understood the whole system as I see. So patch is incorrect in the suggested way

, but I'm going to post a working one soon, I hope .

[pasdVn]

Updatet. Explanations see first post.

[KAPATEJIb]

Can you upload patch to filebeam or paste2.org? Because highlighting is bugged... replaced to

[i]

[pasdVn]

Added github link

[pasdVn]

Please move to reverted. Nearly same sollution was implemented in [8362] (http://github.com/mangos/mangos/commit/99e58a0d0f444c579e2be7d8b316198a1aa0d48a).

(exept the typo fix and the addition and the change in SelectAuraRankForPlayerLevel, as I just noticed. Maybe I will just provide an update...)