Core/Spells: rework part 2: Split up target selection logic

Port From (https://github.com/TrinityCore/TrinityCore/commit/52ae3c89d1d4dbcff309d940fd41c7212edc77b7)

Source/Framework/Constants/CreatureConst.cs

@@ -161,6 +161,7 @@ namespace Framework.Constants
         ContributionCollector = 0x400,
     }
 
+    [Flags]
     public enum CreatureTypeFlags : uint
     {
         TameablePet = 0x01, // Makes the mob tameable (must also be a beast and have family set)

Source/Game/Entities/AreaTrigger/AreaTrigger.cs

@@ -596,9 +596,9 @@ namespace Game.Entities
             switch (action.TargetType)
             {
                 case AreaTriggerActionUserTypes.Friend:
-                        return caster._IsValidAssistTarget(unit, Global.SpellMgr.GetSpellInfo(action.Param, caster.GetMap().GetDifficultyID()));
+                        return caster.IsValidAssistTarget(unit, Global.SpellMgr.GetSpellInfo(action.Param, caster.GetMap().GetDifficultyID()));
                 case AreaTriggerActionUserTypes.Enemy:
-                        return caster._IsValidAttackTarget(unit, Global.SpellMgr.GetSpellInfo(action.Param, caster.GetMap().GetDifficultyID()));
+                        return caster.IsValidAttackTarget(unit, Global.SpellMgr.GetSpellInfo(action.Param, caster.GetMap().GetDifficultyID()));
                 case AreaTriggerActionUserTypes.Raid:
                         return caster.IsInRaidWith(unit);
                 case AreaTriggerActionUserTypes.Party:

Source/Game/Entities/Unit/Unit.Combat.cs

@@ -643,7 +643,7 @@ namespace Game.Entities
                 Unit magnet = eff.GetBase().GetCaster();
                 if (magnet != null)
                 {
-                    if (spellInfo.CheckExplicitTarget(this, magnet) == SpellCastResult.SpellCastOk && _IsValidAttackTarget(magnet, spellInfo))
+                    if (spellInfo.CheckExplicitTarget(this, magnet) == SpellCastResult.SpellCastOk && IsValidAttackTarget(magnet, spellInfo))
                     {
                         // @todo handle this charge drop by proc in cast phase on explicit target
                         if (spellInfo.HasHitDelay())
@@ -675,7 +675,7 @@ namespace Game.Entities
             {
                 Unit magnet = i.GetCaster();
                 if (magnet != null)
-                    if (_IsValidAttackTarget(magnet, spellInfo) && magnet.IsWithinLOSInMap(this)
+                    if (IsValidAttackTarget(magnet, spellInfo) && magnet.IsWithinLOSInMap(this)
                        && (spellInfo == null || (spellInfo.CheckExplicitTarget(this, magnet) == SpellCastResult.SpellCastOk
                        && spellInfo.CheckTarget(this, magnet, false) == SpellCastResult.SpellCastOk)))
                     {
@@ -686,11 +686,6 @@ namespace Game.Entities
             return victim;
         }
 
-        public bool IsValidAttackTarget(Unit target)
-        {
-            return _IsValidAttackTarget(target, null);
-        }
-
         public void SendAttackStateUpdate(HitInfo HitInfo, Unit target, SpellSchoolMask damageSchoolMask, uint Damage, uint AbsorbDamage, uint Resist, VictimState TargetState, uint BlockedAmount)
         {
             CalcDamageInfo dmgInfo = new();
@@ -1567,8 +1562,8 @@ namespace Game.Entities
             }
         }
 
-        // function based on function Unit.CanAttack from 13850 client
+        // function based on function Unit::CanAttack from 13850 client
-        public bool _IsValidAttackTarget(Unit target, SpellInfo bySpell, WorldObject obj = null)
+        public bool IsValidAttackTarget(WorldObject target, SpellInfo bySpell = null, bool spellCheck = true)
         {
             Cypher.Assert(target != null);
 
@@ -1576,88 +1571,39 @@ namespace Game.Entities
             if (this == target)
                 return false;
 
-            // can't attack unattackable units or GMs
+            // can't attack GMs
-            if (target.HasUnitState(UnitState.Unattackable)
+            if (target.IsPlayer() && target.ToPlayer().IsGameMaster())
-                || (target.IsTypeId(TypeId.Player) && target.ToPlayer().IsGameMaster()))
                 return false;
 
-            // visibility checks
+            Unit unit = ToUnit();
-            // skip visibility check for GO casts, needs removal when go cast is implemented. Also ignore for gameobject and dynauras
+            Unit targetUnit = target.ToUnit();
-            if (GetEntry() != SharedConst.WorldTrigger && (!obj || !obj.IsTypeMask(TypeMask.GameObject | TypeMask.DynamicObject)))
-            {
-                // can't attack invisible
-                if (bySpell == null || !bySpell.HasAttribute(SpellAttr6.CanTargetInvisible))
-                {
-                    if (obj && !obj.CanSeeOrDetect(target, bySpell != null && bySpell.IsAffectingArea()))
-                        return false;
-                    else if (!obj)
-                    {
-                        // ignore stealth for aoe spells. Ignore stealth if target is player and unit in combat with same player
-                        bool ignoreStealthCheck = (bySpell != null && bySpell.IsAffectingArea()) ||
-                            (target.GetTypeId() == TypeId.Player && target.HasStealthAura() && IsInCombatWith(target));
-
-                        if (!CanSeeOrDetect(target, ignoreStealthCheck))
-                            return false;
-                    }
-                }
-            }
-
-            // can't attack dead
-            if ((bySpell == null || !bySpell.IsAllowingDeadTarget()) && !target.IsAlive())
-                return false;
-
-            // can't attack untargetable
-            if ((bySpell == null || !bySpell.HasAttribute(SpellAttr6.CanTargetUntargetable))
-                && target.HasUnitFlag(UnitFlags.NotSelectable))
-                return false;
-
-            Player playerAttacker = ToPlayer();
-            if (playerAttacker != null)
-            {
-                if (playerAttacker.HasPlayerFlag(PlayerFlags.Commentator2))
-                    return false;
-            }
-
-            // check flags
-            if (target.HasUnitFlag(UnitFlags.NonAttackable | UnitFlags.TaxiFlight | UnitFlags.NotAttackable1 | UnitFlags.Unk16)
-                || (!HasUnitFlag(UnitFlags.PvpAttackable) && target.IsImmuneToNPC())
-                || (!target.HasUnitFlag(UnitFlags.PvpAttackable) && IsImmuneToNPC()))
-                return false;
-
-            if ((bySpell == null || !bySpell.HasAttribute(SpellAttr8.AttackIgnoreImmuneToPCFlag))
-                && (HasUnitFlag(UnitFlags.PvpAttackable) && target.IsImmuneToPC())
-                // check if this is a world trigger cast - GOs are using world triggers to cast their spells, so we need to ignore their immunity flag here, this is a temp workaround, needs removal when go cast is implemented properly
-                && GetEntry() != SharedConst.WorldTrigger)
-                return false;
 
             // CvC case - can attack each other only when one of them is hostile
-            if (!HasUnitFlag(UnitFlags.PvpAttackable) && !target.HasUnitFlag(UnitFlags.PvpAttackable))
+            if (unit && !unit.HasUnitFlag(UnitFlags.PvpAttackable) && targetUnit && !targetUnit.HasUnitFlag(UnitFlags.PvpAttackable))
-                return GetReactionTo(target) <= ReputationRank.Hostile || target.GetReactionTo(this) <= ReputationRank.Hostile;
+                return IsHostileTo(target) || target.IsHostileTo(this);
 
             // PvP, PvC, CvP case
             // can't attack friendly targets
-            if (GetReactionTo(target) > ReputationRank.Neutral || target.GetReactionTo(this) > ReputationRank.Neutral)
+            if (IsFriendlyTo(target) || target.IsFriendlyTo(this))
                 return false;
 
-            Player playerAffectingAttacker = HasUnitFlag(UnitFlags.PvpAttackable) ? GetAffectingPlayer() : null;
+            Player playerAffectingAttacker = unit && unit.HasUnitFlag(UnitFlags.PvpAttackable) ? GetAffectingPlayer() : null;
-            Player playerAffectingTarget = target.HasUnitFlag(UnitFlags.PvpAttackable) ? target.GetAffectingPlayer() : null;
+            Player playerAffectingTarget = targetUnit && targetUnit.HasUnitFlag(UnitFlags.PvpAttackable) ? target.GetAffectingPlayer() : null;
 
             // Not all neutral creatures can be attacked (even some unfriendly faction does not react aggresive to you, like Sporaggar)
             if ((playerAffectingAttacker && !playerAffectingTarget) || (!playerAffectingAttacker && playerAffectingTarget))
             {
-                if (!(target.IsTypeId(TypeId.Player) && IsTypeId(TypeId.Player)) &&
+                Player player = playerAffectingAttacker ? playerAffectingAttacker : playerAffectingTarget;
-                    !(target.IsTypeId(TypeId.Unit) && IsTypeId(TypeId.Unit)))
+                Unit creature = playerAffectingAttacker ? targetUnit : unit;
+                if (creature != null)
                 {
-                    Player player = playerAffectingAttacker ? playerAffectingAttacker : playerAffectingTarget;
-                    Unit creature = playerAffectingAttacker ? target : this;
-
                     if (creature.IsContestedGuard() && player.HasPlayerFlag(PlayerFlags.ContestedPVP))
                         return true;
 
                     var factionTemplate = creature.GetFactionTemplateEntry();
                     if (factionTemplate != null)
                     {
-                        if (player.GetReputationMgr().GetForcedRankIfAny(factionTemplate) == ReputationRank.None)
+                        if (player.GetReputationMgr().GetForcedRankIfAny(factionTemplate) == 0)
                         {
                             var factionEntry = CliDB.FactionStorage.LookupByKey(factionTemplate.Faction);
                             if (factionEntry != null)
@@ -1674,40 +1620,116 @@ namespace Game.Entities
             }
 
             Creature creatureAttacker = ToCreature();
-            if (creatureAttacker != null && Convert.ToBoolean(creatureAttacker.GetCreatureTemplate().TypeFlags & CreatureTypeFlags.TreatAsRaidUnit))
+            if (creatureAttacker && creatureAttacker.GetCreatureTemplate().TypeFlags.HasFlag(CreatureTypeFlags.TreatAsRaidUnit))
                 return false;
 
+            if (bySpell == null)
+                spellCheck = false;
+
+            if (spellCheck && !IsValidSpellAttackTarget(target, bySpell))
+                return false;
+
+            return true;
+        }
+
+        public bool IsValidSpellAttackTarget(WorldObject target, SpellInfo bySpell)
+        {
+            Cypher.Assert(target != null);
+            Cypher.Assert(bySpell != null);
+
+            // can't attack unattackable units
+            Unit unitTarget = target.ToUnit();
+            if (unitTarget && unitTarget.HasUnitState(UnitState.Unattackable))
+                return false;
+
+            Unit unit = ToUnit();
+            // visibility checks (only units)
+            if (unit)
+            {
+                // can't attack invisible
+                if (!bySpell.HasAttribute(SpellAttr6.CanTargetInvisible))
+                {
+                    if (!unit.CanSeeOrDetect(target, bySpell.IsAffectingArea()))
+                        return false;
+
+                    /*
+                    else if (!obj)
+                    {
+                        // ignore stealth for aoe spells. Ignore stealth if target is player and unit in combat with same player
+                        bool const ignoreStealthCheck = (bySpell && bySpell.IsAffectingArea()) ||
+                            (target.GetTypeId() == TYPEID_PLAYER && target.HasStealthAura() && IsInCombatWith(target));
+
+                        if (!CanSeeOrDetect(target, ignoreStealthCheck))
+                            return false;
+                    }
+                    */
+                }
+            }
+
+            // can't attack dead
+            if (!bySpell.IsAllowingDeadTarget() && unitTarget && !unitTarget.IsAlive())
+                return false;
+
+            // can't attack untargetable
+            if (!bySpell.HasAttribute(SpellAttr6.CanTargetUntargetable) && unitTarget && unitTarget.HasUnitFlag(UnitFlags.NotSelectable))
+                return false;
+
+            Player playerAttacker = ToPlayer();
+            if (playerAttacker != null)
+            {
+                if (playerAttacker.HasPlayerFlag(PlayerFlags.Uber))
+                    return false;
+            }
+
+            // check flags
+            if (unitTarget != null && unitTarget.HasUnitFlag(UnitFlags.NonAttackable | UnitFlags.TaxiFlight | UnitFlags.NotAttackable1 | UnitFlags.Unk16))
+                return false;
+
+            if (unit != null && !unit.HasUnitFlag(UnitFlags.PvpAttackable) && unitTarget && unitTarget.IsImmuneToNPC())
+                return false;
+
+            if (unitTarget != null && !unitTarget.HasUnitFlag(UnitFlags.PvpAttackable) && unit && unit.IsImmuneToNPC())
+                return false;
+
+            if (!bySpell.HasAttribute(SpellAttr8.AttackIgnoreImmuneToPCFlag))
+            {
+                if (unit && unit.HasUnitFlag(UnitFlags.PvpAttackable) && unitTarget && unitTarget.IsImmuneToPC())
+                    return false;
+
+                if (unitTarget && unitTarget.HasUnitFlag(UnitFlags.PvpAttackable) && unit && unit.IsImmuneToPC())
+                    return false;
+            }
+
             // check duel - before sanctuary checks
-            if (playerAffectingAttacker != null && playerAffectingTarget != null)
+            Player playerAffectingAttacker = unit && unit.HasUnitFlag(UnitFlags.PvpAttackable) ? GetAffectingPlayer() : null;
+            Player playerAffectingTarget = unitTarget && unitTarget.HasUnitFlag(UnitFlags.PvpAttackable) ? target.GetAffectingPlayer() : null;
+            if (playerAffectingAttacker && playerAffectingTarget)
                 if (playerAffectingAttacker.duel != null && playerAffectingAttacker.duel.opponent == playerAffectingTarget && playerAffectingAttacker.duel.startTime != 0)
                     return true;
 
             // PvP case - can't attack when attacker or target are in sanctuary
             // however, 13850 client doesn't allow to attack when one of the unit's has sanctuary flag and is pvp
-            if (target.HasUnitFlag(UnitFlags.PvpAttackable) && HasUnitFlag(UnitFlags.PvpAttackable) && (target.IsInSanctuary() || IsInSanctuary()))
+            if (unitTarget && unitTarget.HasUnitFlag(UnitFlags.PvpAttackable) && unit && unit.HasUnitFlag(UnitFlags.PvpAttackable) && (unitTarget.IsInSanctuary() || unit.IsInSanctuary()))
                 return false;
 
             // additional checks - only PvP case
-            if (playerAffectingAttacker != null && playerAffectingTarget != null)
+            if (playerAffectingAttacker && playerAffectingTarget)
             {
-                if (target.IsPvP())
+                if (unitTarget.IsPvP())
                     return true;
 
-                if (IsFFAPvP() && target.IsFFAPvP())
+                if (unit.IsFFAPvP() && unitTarget.IsFFAPvP())
                     return true;
 
-                return HasPvpFlag(UnitPVPStateFlags.Unk1) || target.HasPvpFlag(UnitPVPStateFlags.Unk1);
+                return unit.HasPvpFlag(UnitPVPStateFlags.Unk1) ||
+                    unitTarget.HasPvpFlag(UnitPVPStateFlags.Unk1);
             }
+
             return true;
         }
 
-        public bool IsValidAssistTarget(Unit target)
+        // function based on function Unit::CanAssist from 13850 client
-        {
+        public bool IsValidAssistTarget(WorldObject target, SpellInfo bySpell = null, bool spellCheck = true)
-            return _IsValidAssistTarget(target, null);
-        }
-
-        // function based on function Unit.CanAssist from 13850 client
-        public bool _IsValidAssistTarget(Unit target, SpellInfo bySpell)
         {
             Cypher.Assert(target != null);
 
@@ -1715,85 +1737,108 @@ namespace Game.Entities
             if (this == target)
                 return true;
 
-            // can't assist unattackable units or GMs
+            // can't assist GMs
-            if (target.HasUnitState(UnitState.Unattackable)
+            if (target.IsPlayer() && target.ToPlayer().IsGameMaster())
-                || (target.IsTypeId(TypeId.Player) && target.ToPlayer().IsGameMaster()))
+                return false;
+
+            // can't assist non-friendly targets
+            if (GetReactionTo(target) < ReputationRank.Neutral && target.GetReactionTo(this) < ReputationRank.Neutral && (!ToCreature() || !ToCreature().GetCreatureTemplate().TypeFlags.HasFlag(CreatureTypeFlags.TreatAsRaidUnit)))
+                return false;
+
+            if (bySpell == null)
+                spellCheck = false;
+
+            if (spellCheck && !IsValidSpellAssistTarget(target, bySpell))
+                return false;
+
+            return true;
+        }
+
+        public bool IsValidSpellAssistTarget(WorldObject target, SpellInfo bySpell)
+        {
+            Cypher.Assert(target != null);
+            Cypher.Assert(bySpell != null);
+
+            // can't assist unattackable units
+            Unit unitTarget = target.ToUnit();
+            if (unitTarget && unitTarget.HasUnitState(UnitState.Unattackable))
                 return false;
 
             // can't assist own vehicle or passenger
-            if (m_vehicle != null)
+            Unit unit = ToUnit();
-                if (IsOnVehicle(target) || m_vehicle.GetBase().IsOnVehicle(target))
+            if (unit && unitTarget && unit.GetVehicle())
+            {
+                if (unit.IsOnVehicle(unitTarget))
                     return false;
 
+                if (unit.GetVehicleBase().IsOnVehicle(unitTarget))
+                    return false;
+            }
+
             // can't assist invisible
-            if ((bySpell == null || !bySpell.HasAttribute(SpellAttr6.CanTargetInvisible)) && !CanSeeOrDetect(target, bySpell != null && bySpell.IsAffectingArea()))
+            if (!bySpell.HasAttribute(SpellAttr6.CanTargetInvisible) && !CanSeeOrDetect(target, bySpell.IsAffectingArea()))
                 return false;
 
             // can't assist dead
-            if ((bySpell == null || !bySpell.IsAllowingDeadTarget()) && !target.IsAlive())
+            if (!bySpell.IsAllowingDeadTarget() && unitTarget && !unitTarget.IsAlive())
                 return false;
 
             // can't assist untargetable
-            if ((bySpell == null || !bySpell.HasAttribute(SpellAttr6.CanTargetUntargetable))
+            if (!bySpell.HasAttribute(SpellAttr6.CanTargetUntargetable) && unitTarget && unitTarget.HasUnitFlag(UnitFlags.NotSelectable))
-                && target.HasUnitFlag(UnitFlags.NotSelectable))
                 return false;
 
-            if (bySpell == null || !bySpell.HasAttribute(SpellAttr6.AssistIgnoreImmuneFlag))
+            if (!bySpell.HasAttribute(SpellAttr6.AssistIgnoreImmuneFlag))
             {
-                if (HasUnitFlag(UnitFlags.PvpAttackable))
+                if (unit && unit.HasUnitFlag(UnitFlags.PvpAttackable))
                 {
-                    if (target.IsImmuneToPC())
+                    if (unitTarget && unitTarget.IsImmuneToPC())
                         return false;
                 }
                 else
                 {
-                    if (target.IsImmuneToNPC())
+                    if (unitTarget && unitTarget.IsImmuneToNPC())
                         return false;
                 }
             }
 
-            // can't assist non-friendly targets
-            if (GetReactionTo(target) <= ReputationRank.Neutral
-                && target.GetReactionTo(this) <= ReputationRank.Neutral
-                && (!IsTypeId(TypeId.Unit) || !Convert.ToBoolean(ToCreature().GetCreatureTemplate().TypeFlags & CreatureTypeFlags.TreatAsRaidUnit)))
-                return false;
-
-            // Controlled player case, we can assist creatures (reaction already checked above, our faction == charmer faction)
-            if (IsTypeId(TypeId.Player) && IsCharmed() && GetCharmerGUID().IsCreature())
-                return true;
-
             // PvP case
-            else if (target.HasUnitFlag(UnitFlags.PvpAttackable))
+            if (unitTarget && unitTarget.HasUnitFlag(UnitFlags.PvpAttackable))
             {
                 Player targetPlayerOwner = target.GetAffectingPlayer();
-                if (HasUnitFlag(UnitFlags.PvpAttackable))
+                if (unit && unit.HasUnitFlag(UnitFlags.PvpAttackable))
                 {
                     Player selfPlayerOwner = GetAffectingPlayer();
-                    if (selfPlayerOwner != null && targetPlayerOwner != null)
+                    if (selfPlayerOwner && targetPlayerOwner)
                     {
                         // can't assist player which is dueling someone
                         if (selfPlayerOwner != targetPlayerOwner && targetPlayerOwner.duel != null)
                             return false;
                     }
                     // can't assist player in ffa_pvp zone from outside
-                    if (target.HasPvpFlag(UnitPVPStateFlags.FFAPvp) && !HasPvpFlag(UnitPVPStateFlags.FFAPvp))
+                    if (unitTarget.IsFFAPvP() && unit && !unit.IsFFAPvP())
                         return false;
+
                     // can't assist player out of sanctuary from sanctuary if has pvp enabled
-                    if (target.HasPvpFlag(UnitPVPStateFlags.PvP))
+                    if (unitTarget.IsPvP())
-                        if (IsInSanctuary() && !target.IsInSanctuary())
+                        if (unit && unit.IsInSanctuary() && !unitTarget.IsInSanctuary())
                             return false;
                 }
             }
             // PvC case - player can assist creature only if has specific type flags
-            else if (HasUnitFlag(UnitFlags.PvpAttackable)
+            // !target.HasFlag(UNIT_FIELD_FLAGS, UnitFlags.PvpAttackable) &&
-                && (bySpell == null || !bySpell.HasAttribute(SpellAttr6.AssistIgnoreImmuneFlag))
+            else if (unit && unit.HasUnitFlag(UnitFlags.PvpAttackable))
-                && !target.HasPvpFlag(UnitPVPStateFlags.PvP))
             {
-                Creature creatureTarget = target.ToCreature();
+                if (!bySpell.HasAttribute(SpellAttr6.AssistIgnoreImmuneFlag))
-                if (creatureTarget != null)
+                {
-                    return Convert.ToBoolean(creatureTarget.GetCreatureTemplate().TypeFlags & CreatureTypeFlags.TreatAsRaidUnit)
+                    if (unitTarget && !unitTarget.IsPvP())
-                        || Convert.ToBoolean(creatureTarget.GetCreatureTemplate().TypeFlags & CreatureTypeFlags.CanAssist);
+                    {
+                        Creature creatureTarget = target.ToCreature();
+                        if (creatureTarget != null)
+                            return creatureTarget.GetCreatureTemplate().TypeFlags.HasFlag(CreatureTypeFlags.TreatAsRaidUnit) || creatureTarget.GetCreatureTemplate().TypeFlags.HasFlag(CreatureTypeFlags.CanAssist);
+                    }
+                }
             }
+
             return true;
         }
 

Source/Game/Maps/GridNotifiers.cs

@@ -1916,7 +1916,7 @@ namespace Game.Maps
             if (!u.IsTargetableForAttack(false))
                 return false;
 
-            if (!i_obj.IsWithinDistInMap(u, i_range) || i_funit._IsValidAttackTarget(u, null, i_obj))
+            if (!i_obj.IsWithinDistInMap(u, i_range) || i_funit.IsValidAttackTarget(u))
                 return false;
 
             i_range = i_obj.GetDistance(u);
@@ -2088,7 +2088,7 @@ namespace Game.Maps
             if (_spellInfo != null && _spellInfo.HasAttribute(SpellAttr3.OnlyTargetPlayers) && !u.IsPlayer())
                 return false;
 
-            if (!i_funit._IsValidAttackTarget(u, _spellInfo, i_obj.GetTypeId() == TypeId.DynamicObject ? i_obj : null))
+            if (!i_funit.IsValidAttackTarget(u, _spellInfo))
                 return false;
 
             float searchRadius = i_range;

Source/Game/Spells/Spell.cs

@@ -2201,15 +2201,12 @@ namespace Game.Spells
                 if (m_spellInfo.HasHitDelay() && unit.HasUnitFlag(UnitFlags.NonAttackable) && unit.GetCharmerOrOwnerGUID() != m_caster.GetGUID())
                     return SpellMissInfo.Evade;
 
-                if (m_caster._IsValidAttackTarget(unit, m_spellInfo))
+                if (m_caster.IsValidAttackTarget(unit, m_spellInfo))
                     unit.RemoveAurasWithInterruptFlags(SpellAuraInterruptFlags.HostileActionReceived);
                 else if (m_caster.IsFriendlyTo(unit))
                 {
                     // for delayed spells ignore negative spells (after duel end) for friendly targets
-                    // @todo this cause soul transfer bugged
+                    if (m_spellInfo.HasHitDelay() && unit.IsPlayer() && !IsPositive() && !m_caster.IsValidSpellAttackTarget(unit, m_spellInfo))
-                    // 63881 - Malady of the Mind jump spell (Yogg-Saron)
-                    // 45034 - Curse of Boundless Agony jump spell (Kalecgos)
-                    if (m_spellInfo.HasHitDelay() && unit.IsPlayer() && !IsPositive() && m_spellInfo.Id != 63881 && m_spellInfo.Id != 45034)
                         return SpellMissInfo.Evade;
 
                     // assisting case, healing and resurrection
@@ -7936,7 +7933,7 @@ namespace Game.Spells
             Corpse corpseTarget = obj.ToCorpse();
             if (corpseTarget != null)
             {
-                // use ofter for party/assistance checks
+                // use owner for party/assistance checks
                 Player owner = Global.ObjAccessor.FindPlayer(corpseTarget.GetOwnerGUID());
                 if (owner != null)
                     unitTarget = owner;
@@ -7946,24 +7943,25 @@ namespace Game.Spells
 
             if (unitTarget != null)
             {
+                // do only faction checks here
                 switch (_targetSelectionType)
                 {
                     case SpellTargetCheckTypes.Enemy:
                         if (unitTarget.IsTotem())
                             return false;
-                        if (!_caster._IsValidAttackTarget(unitTarget, _spellInfo))
+                        if (!_caster.IsValidAttackTarget(unitTarget, _spellInfo, null, false))
                             return false;
                         break;
                     case SpellTargetCheckTypes.Ally:
                         if (unitTarget.IsTotem())
                             return false;
-                        if (!_caster._IsValidAssistTarget(unitTarget, _spellInfo))
+                        if (!_caster.IsValidAssistTarget(unitTarget, _spellInfo))
                             return false;
                         break;
                     case SpellTargetCheckTypes.Party:
                         if (unitTarget.IsTotem())
                             return false;
-                        if (!_caster._IsValidAssistTarget(unitTarget, _spellInfo))
+                        if (!_caster.IsValidAssistTarget(unitTarget, _spellInfo))
                             return false;
                         if (!_referer.IsInPartyWith(unitTarget))
                             return false;
@@ -7974,7 +7972,7 @@ namespace Game.Spells
                             return false;
                         if (unitTarget.IsTotem())
                             return false;
-                        if (!_caster._IsValidAssistTarget(unitTarget, _spellInfo))
+                        if (!_caster.IsValidAssistTarget(unitTarget, _spellInfo))
                             return false;
                         if (!_referer.IsInRaidWith(unitTarget))
                             return false;
@@ -8010,6 +8008,19 @@ namespace Game.Spells
                     default:
                         break;
                 }
+
+                // then check actual spell positivity to determine if the target is valid
+                // (negative spells may be targeted on allies)
+                if (_spellInfo.IsPositive())
+                {
+                    if (!_caster.IsValidSpellAssistTarget(unitTarget, _spellInfo))
+                        return false;
+                }
+                else
+                {
+                    if (!_caster.IsValidSpellAttackTarget(unitTarget, _spellInfo))
+                        return false;
+                }
             }
             if (_condSrcInfo == null)
                 return true;

Source/Game/Spells/SpellInfo.cs

@@ -1186,12 +1186,12 @@ namespace Game.Spells
                 if (Convert.ToBoolean(neededTargets & (SpellCastTargetFlags.UnitEnemy | SpellCastTargetFlags.UnitAlly | SpellCastTargetFlags.UnitRaid | SpellCastTargetFlags.UnitParty | SpellCastTargetFlags.UnitMinipet | SpellCastTargetFlags.UnitPassenger)))
                 {
                     if (Convert.ToBoolean(neededTargets & SpellCastTargetFlags.UnitEnemy))
-                        if (caster._IsValidAttackTarget(unitTarget, this))
+                        if (caster.IsValidAttackTarget(unitTarget, this))
                             return SpellCastResult.SpellCastOk;
                     if (neededTargets.HasAnyFlag(SpellCastTargetFlags.UnitAlly)
                         || (neededTargets.HasAnyFlag(SpellCastTargetFlags.UnitParty) && caster.IsInPartyWith(unitTarget))
                         || (neededTargets.HasAnyFlag(SpellCastTargetFlags.UnitRaid) && caster.IsInRaidWith(unitTarget)))
-                        if (caster._IsValidAssistTarget(unitTarget, this))
+                        if (caster.IsValidAssistTarget(unitTarget, this))
                             return SpellCastResult.SpellCastOk;
                     if (Convert.ToBoolean(neededTargets & SpellCastTargetFlags.UnitMinipet))
                         if (unitTarget.GetGUID() == caster.GetCritterGUID())