Core/Spells: Add a generic way of selecting spell explicit targets. Make explicit targets independant of calling convention in core - this makes targets sent to client to be much more blizzlike than before (for example CastSpell(Unit*) won't set TARGET_FLAG_UNIT for spells which are not using explicit unit target) for SMSG_SPELL_START packets. Also remove hacks, which became obsolete with this commit.

src/server/game/Spells/Spell.cpp

@@ -82,42 +82,11 @@ SpellCastTargets::~SpellCastTargets()
 {
 }
 
-SpellCastTargets& SpellCastTargets::operator=(const SpellCastTargets &target)
-{
-    m_unitTarget = target.m_unitTarget;
-    m_itemTarget = target.m_itemTarget;
-    m_GOTarget   = target.m_GOTarget;
-
-    m_unitTargetGUID   = target.m_unitTargetGUID;
-    m_GOTargetGUID     = target.m_GOTargetGUID;
-    m_CorpseTargetGUID = target.m_CorpseTargetGUID;
-    m_itemTargetGUID   = target.m_itemTargetGUID;
-
-    m_itemTargetEntry  = target.m_itemTargetEntry;
-
-    m_srcTransGUID = target.m_srcTransGUID;
-    m_srcTransOffset = target.m_srcTransOffset;
-    m_srcPos = target.m_srcPos;
-
-    m_dstTransGUID = target.m_dstTransGUID;
-    m_dstTransOffset = target.m_dstTransOffset;
-    m_dstPos = target.m_dstPos;
-
-    m_elevation = target.m_elevation;
-    m_speed = target.m_speed;
-
-    m_strTarget = target.m_strTarget;
-
-    m_targetMask = target.m_targetMask;
-
-    return *this;
-}
-
 void SpellCastTargets::Read(ByteBuffer& data, Unit* caster)
 {
     data >> m_targetMask;
 
-    if (m_targetMask == TARGET_FLAG_SELF)
+    if (m_targetMask == TARGET_FLAG_NONE)
         return;
 
     if (m_targetMask & (TARGET_FLAG_UNIT | TARGET_FLAG_UNIT_MINIPET))
@@ -238,6 +207,65 @@ void SpellCastTargets::SetUnitTarget(Unit* target)
     m_targetMask |= TARGET_FLAG_UNIT;
 }
 
+void SpellCastTargets::RemoveUnitTarget()
+{
+    m_unitTarget = NULL;
+    m_unitTargetGUID = 0LL;
+    m_targetMask &= ~(TARGET_FLAG_UNIT | TARGET_FLAG_UNIT_MINIPET);
+}
+
+void SpellCastTargets::SetGOTarget(GameObject* target)
+{
+    if (!target)
+        return;
+
+    m_GOTarget = target;
+    m_GOTargetGUID = target->GetGUID();
+    m_targetMask |= TARGET_FLAG_GAMEOBJECT;
+}
+
+void SpellCastTargets::RemoveGOTarget()
+{
+    m_GOTarget = NULL;
+    m_GOTargetGUID = 0LL;
+    m_targetMask &= ~(TARGET_FLAG_GAMEOBJECT);
+}
+
+void SpellCastTargets::RemoveCorpseTarget()
+{
+    m_CorpseTargetGUID = 0;
+    m_targetMask &= ~(TARGET_FLAG_CORPSE_ENEMY | TARGET_FLAG_CORPSE_ALLY);
+}
+
+void SpellCastTargets::SetItemTarget(Item* item)
+{
+    if (!item)
+        return;
+
+    m_itemTarget = item;
+    m_itemTargetGUID = item->GetGUID();
+    m_itemTargetEntry = item->GetEntry();
+    m_targetMask |= TARGET_FLAG_ITEM;
+}
+
+void SpellCastTargets::SetTradeItemTarget(Player* caster)
+{
+    m_itemTargetGUID = uint64(TRADE_SLOT_NONTRADED);
+    m_itemTargetEntry = 0;
+    m_targetMask |= TARGET_FLAG_TRADE_ITEM;
+
+    Update(caster);
+}
+
+void SpellCastTargets::UpdateTradeSlotItem()
+{
+    if (m_itemTarget && (m_targetMask & TARGET_FLAG_TRADE_ITEM))
+    {
+        m_itemTargetGUID = m_itemTarget->GetGUID();
+        m_itemTargetEntry = m_itemTarget->GetEntry();
+    }
+}
+
 Position const* SpellCastTargets::GetSrc() const
 {
     return &m_srcPos;
@@ -279,6 +307,11 @@ void SpellCastTargets::ModSrc(Position const& pos)
     m_srcPos.Relocate(pos);
 }
 
+void SpellCastTargets::RemoveSrc()
+{
+    m_targetMask &= ~(TARGET_FLAG_SOURCE_LOCATION);
+}
+
 WorldLocation const* SpellCastTargets::GetDst() const
 {
     return &m_dstPos;
@@ -330,45 +363,9 @@ void SpellCastTargets::ModDst(Position const& pos)
     m_dstPos.Relocate(pos);
 }
 
-void SpellCastTargets::SetGOTarget(GameObject* target)
+void SpellCastTargets::RemoveDst()
 {
-    m_GOTarget = target;
+    m_targetMask &= ~(TARGET_FLAG_DEST_LOCATION);
-    m_GOTargetGUID = target->GetGUID();
-    m_targetMask |= TARGET_FLAG_GAMEOBJECT;
-}
-
-void SpellCastTargets::SetItemTarget(Item* item)
-{
-    if (!item)
-        return;
-
-    m_itemTarget = item;
-    m_itemTargetGUID = item->GetGUID();
-    m_itemTargetEntry = item->GetEntry();
-    m_targetMask |= TARGET_FLAG_ITEM;
-}
-
-void SpellCastTargets::SetTradeItemTarget(Player* caster)
-{
-    m_itemTargetGUID = uint64(TRADE_SLOT_NONTRADED);
-    m_itemTargetEntry = 0;
-    m_targetMask |= TARGET_FLAG_TRADE_ITEM;
-
-    Update(caster);
-}
-
-void SpellCastTargets::UpdateTradeSlotItem()
-{
-    if (m_itemTarget && (m_targetMask & TARGET_FLAG_TRADE_ITEM))
-    {
-        m_itemTargetGUID = m_itemTarget->GetGUID();
-        m_itemTargetEntry = m_itemTarget->GetEntry();
-    }
-}
-
-void SpellCastTargets::SetCorpseTarget(Corpse* corpse)
-{
-    m_CorpseTargetGUID = corpse->GetGUID();
 }
 
 void SpellCastTargets::Update(Unit* caster)
@@ -414,7 +411,7 @@ void SpellCastTargets::Update(Unit* caster)
 void SpellCastTargets::OutDebug() const
 {
     if (!m_targetMask)
-        sLog->outString("TARGET_FLAG_SELF");
+        sLog->outString("TARGET_FLAG_NONE");
 
     if (m_targetMask & TARGET_FLAG_UNIT)
         sLog->outString("TARGET_FLAG_UNIT: " UI64FMTD, m_unitTargetGUID);
@@ -603,6 +600,73 @@ WorldObject* Spell::FindCorpseUsing()
     return result;
 }
 
+void Spell::InitExplicitTargets(SpellCastTargets const& targets)
+{
+    m_targets = targets;
+    // this function tries to correct spell explicit targets for spell
+    // client doesn't send explicit targets correctly sometimes - we need to fix such spells serverside
+    // this also makes sure that we correctly send explicit targets to client (removes redundant data)
+    uint32 neededTargets = m_spellInfo->GetExplicitTargetMask();
+
+    // check if spell needs unit target
+    if (neededTargets & TARGET_FLAG_UNIT_MASK)
+    {
+        Unit* target = targets.GetUnitTarget();
+
+        if (!target)
+        {
+            // try to use player selection as a target
+            if (Player* playerCaster = m_caster->ToPlayer())
+            {
+                // selection has to be found and to be valid target for the spell
+                if (Unit* selectedUnit = ObjectAccessor::GetUnit(*m_caster, playerCaster->GetSelection()))
+                    if (IsValidSingleTargetSpell(selectedUnit))
+                        target = selectedUnit;
+            }
+            // try to use attacked unit as a target
+            else if ((m_caster->GetTypeId() == TYPEID_UNIT) && neededTargets & (TARGET_FLAG_UNIT_ENEMY | TARGET_FLAG_UNIT))
+                target = m_caster->getVictim();
+            // didn't find anything - let's use self as target
+            if (!target && neededTargets & (TARGET_FLAG_UNIT_RAID | TARGET_FLAG_UNIT_PARTY | TARGET_FLAG_UNIT_ALLY | TARGET_FLAG_UNIT))
+                target = m_caster;
+        }
+        m_targets.SetUnitTarget(target);
+    }
+    else
+        m_targets.RemoveUnitTarget();
+
+    if (!(neededTargets & TARGET_FLAG_GAMEOBJECT_MASK))
+        m_targets.RemoveGOTarget();
+
+    if (!(neededTargets & TARGET_FLAG_CORPSE_MASK))
+        m_targets.RemoveCorpseTarget();
+
+    // check if spell needs dst target
+    if (neededTargets & TARGET_FLAG_DEST_LOCATION)
+    {
+        // and target isn't set
+        if (!targets.GetDst())
+        {
+            // try to use unit target if provided
+            if (Unit* target = m_targets.GetUnitTarget())
+                m_targets.SetDst(*target);
+            // or use self if not available
+            else
+                m_targets.SetDst(*m_caster);
+        }
+    }
+    else
+        m_targets.RemoveDst();
+
+    if (neededTargets & TARGET_FLAG_SOURCE_LOCATION)
+    {
+        if (!targets.GetSrc())
+            m_targets.SetSrc(*m_caster);
+    }
+    else
+        m_targets.RemoveSrc();
+}
+
 void Spell::SelectSpellTargets()
 {
     for (uint32 i = 0; i < MAX_SPELL_EFFECTS; ++i)
@@ -2776,25 +2840,8 @@ void Spell::prepare(SpellCastTargets const* targets, AuraEffect const* triggered
     else
         m_castItemGUID = 0;
 
-    m_targets = *targets;
+    InitExplicitTargets(*targets);
 
-    if (!m_targets.GetUnitTargetGUID() && m_spellInfo->Targets & TARGET_FLAG_UNIT)
-    {
-        Unit* target = NULL;
-        if (m_caster->GetTypeId() == TYPEID_UNIT)
-            target = m_caster->getVictim();
-        else
-            target = ObjectAccessor::GetUnit(*m_caster, m_caster->ToPlayer()->GetSelection());
-
-        if (target && IsValidSingleTargetSpell(target))
-            m_targets.SetUnitTarget(target);
-        else
-        {
-            SendCastResult(SPELL_FAILED_BAD_TARGETS);
-            finish(false);
-            return;
-        }
-    }
     if (Player* plrCaster = m_caster->GetCharmerOrOwnerPlayerOrPlayerItself())
     {
         //check for special spell conditions
@@ -2808,29 +2855,6 @@ void Spell::prepare(SpellCastTargets const* targets, AuraEffect const* triggered
                 return;
             }
     }
-    if (!m_targets.HasSrc() && m_spellInfo->Targets & TARGET_FLAG_SOURCE_LOCATION)
-        m_targets.SetSrc(*m_caster);
-
-    if (!m_targets.HasDst() && m_spellInfo->Targets & TARGET_FLAG_DEST_LOCATION)
-    {
-        Unit* target = m_targets.GetUnitTarget();
-        if (!target)
-        {
-            if (m_caster->GetTypeId() == TYPEID_UNIT)
-                target = m_caster->getVictim();
-            else
-                target = ObjectAccessor::GetUnit(*m_caster, m_caster->ToPlayer()->GetSelection());
-        }
-
-        if (target)
-            m_targets.SetDst(*target);
-        else
-        {
-            SendCastResult(SPELL_FAILED_BAD_TARGETS);
-            finish(false);
-            return;
-        }
-    }
 
     // Fill aura scaling information
     if (m_caster->IsControlledByPlayer() && !m_spellInfo->IsPassive() && m_spellInfo->SpellLevel && !m_spellInfo->IsChanneled() && !(_triggeredCastFlags & TRIGGERED_IGNORE_AURA_SCALING))
@@ -3834,15 +3858,6 @@ void Spell::SendSpellGo()
     data << uint32(castFlags);                              // cast flags
     data << uint32(getMSTime());                            // timestamp
 
-    /*
-    // statement below seems to be wrong - i've seen spells with both unit and dest target
-    // Can't have TARGET_FLAG_UNIT when *_LOCATION is present - it breaks missile visuals
-    if (m_targets.GetTargetMask() & (TARGET_FLAG_SOURCE_LOCATION | TARGET_FLAG_DEST_LOCATION))
-        m_targets.setTargetMask(m_targets.GetTargetMask() & ~TARGET_FLAG_UNIT);
-    else if (m_targets.getIntTargetFlags() & FLAG_INT_UNIT)
-        m_targets.setTargetMask(m_targets.GetTargetMask() | TARGET_FLAG_UNIT);
-    */
-
     WriteSpellGoTargets(&data);
 
     m_targets.Write(data);
@@ -4693,19 +4708,11 @@ SpellCastResult Spell::CheckCast(bool strict)
             return SPELL_FAILED_DONT_REPORT;
     }
 
-    Unit* target = m_targets.GetUnitTarget();
+    if (Unit* target = m_targets.GetUnitTarget())
-    // In pure self-cast spells, the client won't send any unit target
-    if (!target && (m_targets.GetTargetMask() == TARGET_FLAG_SELF || m_targets.GetTargetMask() & TARGET_FLAG_UNIT_ALLY)) // TARGET_FLAG_SELF == 0, remember!
-        target = m_caster;
-
-    if (target)
-    {
-        if (target != m_caster && m_spellInfo->IsRequiringSelectedTarget())
     {
         SpellCastResult castResult = m_spellInfo->CheckTarget(m_caster, target, false);
         if (castResult != SPELL_CAST_OK)
             return castResult;
-        }
 
         if (target != m_caster)
         {
@@ -4724,18 +4731,6 @@ SpellCastResult Spell::CheckCast(bool strict)
         {
             if (m_caster->GetTypeId() == TYPEID_PLAYER) // Target - is player caster
             {
-                // Additional check for some spells
-                // If 0 spell effect empty - client not send target data (need use selection)
-                // TODO: check it on next client version
-                if (m_targets.GetTargetMask() == TARGET_FLAG_SELF &&
-                    m_spellInfo->Effects[1].TargetA.GetTarget() == TARGET_UNIT_TARGET_ENEMY)
-                {
-                    target = m_caster->GetUnit(*m_caster, m_caster->ToPlayer()->GetSelection());
-                    if (target)
-                        m_targets.SetUnitTarget(target);
-                    else
-                        return SPELL_FAILED_BAD_TARGETS;
-                }
                 // Lay on Hands - cannot be self-cast on paladin with Forbearance or after using Avenging Wrath
                 if (m_spellInfo->SpellFamilyName == SPELLFAMILY_PALADIN && m_spellInfo->SpellFamilyFlags[0] & 0x0008000)
                     if (target->HasAura(61988)) // Immunity shield marker

src/server/game/Spells/Spell.h

@@ -35,33 +35,6 @@ class ByteBuffer;
 
 #define SPELL_CHANNEL_UPDATE_INTERVAL (1 * IN_MILLISECONDS)
 
-enum SpellCastTargetFlags
-{
-    TARGET_FLAG_SELF            = 0x00000000,
-    TARGET_FLAG_UNUSED_1        = 0x00000001,               // not used
-    TARGET_FLAG_UNIT            = 0x00000002,               // pguid
-    TARGET_FLAG_UNIT_RAID       = 0x00000004,               // not sent, used to validate target (if raid member)
-    TARGET_FLAG_UNIT_PARTY      = 0x00000008,               // not sent, used to validate target (if party member)
-    TARGET_FLAG_ITEM            = 0x00000010,               // pguid
-    TARGET_FLAG_SOURCE_LOCATION = 0x00000020,               // pguid, 3 float
-    TARGET_FLAG_DEST_LOCATION   = 0x00000040,               // pguid, 3 float
-    TARGET_FLAG_UNIT_ENEMY      = 0x00000080,               // not sent, used to validate target (if enemy)
-    TARGET_FLAG_UNIT_ALLY       = 0x00000100,               // not sent, used to validate target (if ally)
-    TARGET_FLAG_CORPSE_ENEMY    = 0x00000200,               // pguid
-    TARGET_FLAG_UNIT_DEAD       = 0x00000400,               // not sent, used to validate target (if dead creature)
-    TARGET_FLAG_GAMEOBJECT      = 0x00000800,               // pguid, used with TARGET_GAMEOBJECT_TARGET
-    TARGET_FLAG_TRADE_ITEM      = 0x00001000,               // pguid
-    TARGET_FLAG_STRING          = 0x00002000,               // string
-    TARGET_FLAG_GAMEOBJECT_ITEM = 0x00004000,               // not sent, used with TARGET_GAMEOBJECT_ITEM_TARGET
-    TARGET_FLAG_CORPSE_ALLY     = 0x00008000,               // pguid
-    TARGET_FLAG_UNIT_MINIPET    = 0x00010000,               // pguid, used to validate target (if non combat pet)
-    TARGET_FLAG_GLYPH_SLOT      = 0x00020000,               // used in glyph spells
-    TARGET_FLAG_UNK19           = 0x00040000,               // sometimes appears with DEST_TARGET spells (may appear or not for a given spell)
-    TARGET_FLAG_UNUSED20        = 0x00080000,               // uint32 counter, loop { vec3 - screen position (?), guid }, not used so far
-    TARGET_FLAG_UNIT_PASSENGER  = 0x00100000,               // guessed, used to validate target (if vehicle passenger)
-};
-#define MAX_TARGET_FLAGS 21
-
 enum SpellCastFlags
 {
     CAST_FLAG_NONE               = 0x00000000,
@@ -125,8 +98,6 @@ class SpellCastTargets
         SpellCastTargets();
         ~SpellCastTargets();
 
-        SpellCastTargets& operator=(const SpellCastTargets &target);
-
         void Read(ByteBuffer& data, Unit* caster);
         void Write(ByteBuffer& data);
 
@@ -136,26 +107,15 @@ class SpellCastTargets
         uint64 GetUnitTargetGUID() const { return m_unitTargetGUID; }
         Unit* GetUnitTarget() const { return m_unitTarget; }
         void SetUnitTarget(Unit* target);
-
+        void RemoveUnitTarget();
-        Position const* GetSrc() const;
-        void SetSrc(float x, float y, float z);
-        void SetSrc(Position const& pos);
-        void SetSrc(WorldObject const& wObj);
-        void ModSrc(Position const& pos);
-
-        WorldLocation const* GetDst() const;
-        void SetDst(float x, float y, float z, float orientation, uint32 mapId = MAPID_INVALID);
-        void SetDst(Position const& pos);
-        void SetDst(WorldObject const& wObj);
-        void SetDst(SpellCastTargets const& spellTargets);
-        void ModDst(Position const& pos);
 
         uint64 GetGOTargetGUID() const { return m_GOTargetGUID; }
         GameObject* GetGOTarget() const { return m_GOTarget; }
         void SetGOTarget(GameObject* target);
+        void RemoveGOTarget();
 
         uint64 GetCorpseTargetGUID() const { return m_CorpseTargetGUID; }
-        void SetCorpseTarget(Corpse* corpse);
+        void RemoveCorpseTarget();
 
         uint64 GetItemTargetGUID() const { return m_itemTargetGUID; }
         Item* GetItemTarget() const { return m_itemTarget; }
@@ -164,6 +124,21 @@ class SpellCastTargets
         void SetTradeItemTarget(Player* caster);
         void UpdateTradeSlotItem();
 
+        Position const* GetSrc() const;
+        void SetSrc(float x, float y, float z);
+        void SetSrc(Position const& pos);
+        void SetSrc(WorldObject const& wObj);
+        void ModSrc(Position const& pos);
+        void RemoveSrc();
+
+        WorldLocation const* GetDst() const;
+        void SetDst(float x, float y, float z, float orientation, uint32 mapId = MAPID_INVALID);
+        void SetDst(Position const& pos);
+        void SetDst(WorldObject const& wObj);
+        void SetDst(SpellCastTargets const& spellTargets);
+        void ModDst(Position const& pos);
+        void RemoveDst();
+
         bool IsEmpty() const { return m_GOTargetGUID == 0 && m_unitTargetGUID == 0 && m_itemTarget == 0 && m_CorpseTargetGUID == 0; }
         bool HasSrc() const { return GetTargetMask() & TARGET_FLAG_SOURCE_LOCATION; }
         bool HasDst() const { return GetTargetMask() & TARGET_FLAG_DEST_LOCATION; }
@@ -423,6 +398,7 @@ class Spell
         void WriteSpellGoTargets(WorldPacket * data);
         void WriteAmmoToPacket(WorldPacket * data);
 
+        void InitExplicitTargets(SpellCastTargets const& targets);
         void SelectSpellTargets();
         void SelectEffectTargets(uint32 i, SpellImplicitTargetInfo const& cur);
         void SelectTrajTargets();

src/server/game/Spells/SpellEffects.cpp

@@ -1600,14 +1600,6 @@ void Spell::EffectForceCast(SpellEffIndex effIndex)
                 break;
         }
     }
-
-    // temphack
-    if (m_spellInfo->Id == 51888)
-    {
-        unitTarget->CastSpell(unitTarget, spellInfo->Id, true, NULL, NULL, m_originalCasterGUID);
-        return;
-    }
-
     unitTarget->CastSpell(m_caster, spellInfo, true);
 }
 

src/server/game/Spells/SpellInfo.cpp

@@ -21,6 +21,30 @@
 #include "Spell.h"
 #include "DBCStores.h"
 
+uint32 GetTargetFlagMask(SpellTargetObjectTypes objType)
+{
+    switch (objType)
+    {
+        case TARGET_OBJECT_TYPE_DEST:
+            return TARGET_FLAG_DEST_LOCATION;
+        case TARGET_OBJECT_TYPE_UNIT_AND_DEST:
+            return TARGET_FLAG_DEST_LOCATION | TARGET_FLAG_UNIT;
+        case TARGET_OBJECT_TYPE_CORPSE:
+        case TARGET_OBJECT_TYPE_UNIT:
+            return TARGET_FLAG_UNIT;
+        case TARGET_OBJECT_TYPE_GOBJ:
+            return TARGET_FLAG_GAMEOBJECT;
+        case TARGET_OBJECT_TYPE_GOBJ_ITEM:
+            return TARGET_FLAG_GAMEOBJECT_ITEM;
+        case TARGET_OBJECT_TYPE_ITEM:
+            return TARGET_FLAG_ITEM;
+        case TARGET_OBJECT_TYPE_SRC:
+            return TARGET_FLAG_SOURCE_LOCATION;
+        default:
+            return TARGET_FLAG_NONE;
+    }
+}
+
 SpellImplicitTargetInfo::SpellImplicitTargetInfo(uint32 target)
 {
     _target = Targets(target);
@@ -93,6 +117,66 @@ Targets SpellImplicitTargetInfo::GetTarget() const
     return _target;
 }
 
+uint32 SpellImplicitTargetInfo::GetExplicitTargetMask(bool& srcSet, bool& dstSet) const
+{
+    switch (GetObjectType())
+    {
+        case TARGET_OBJECT_TYPE_SRC:
+            srcSet = true;
+            break;
+        case TARGET_OBJECT_TYPE_DEST:
+        case TARGET_OBJECT_TYPE_UNIT_AND_DEST:
+            dstSet = true;
+            break;
+        default:
+            break;
+    }
+
+    switch (GetReferenceType())
+    {
+        case TARGET_REFERENCE_TYPE_SRC:
+            if (srcSet)
+                break;
+            return TARGET_FLAG_SOURCE_LOCATION;
+        case TARGET_REFERENCE_TYPE_DEST:
+            if (dstSet)
+                break;
+            return TARGET_FLAG_DEST_LOCATION;
+        case TARGET_REFERENCE_TYPE_TARGET:
+            switch (GetObjectType())
+            {
+                case TARGET_OBJECT_TYPE_GOBJ:
+                    return TARGET_FLAG_GAMEOBJECT;
+                case TARGET_OBJECT_TYPE_GOBJ_ITEM:
+                    return TARGET_FLAG_GAMEOBJECT_ITEM;
+                case TARGET_OBJECT_TYPE_UNIT_AND_DEST:
+                case TARGET_OBJECT_TYPE_UNIT:
+                    switch (GetSelectionCheckType())
+                    {
+                        case TARGET_SELECT_CHECK_ENEMY:
+                            return TARGET_FLAG_UNIT_ENEMY;
+                        case TARGET_SELECT_CHECK_ALLY:
+                            return TARGET_FLAG_UNIT_ALLY;
+                        case TARGET_SELECT_CHECK_PARTY:
+                            return TARGET_FLAG_UNIT_PARTY;
+                        case TARGET_SELECT_CHECK_RAID:
+                            return TARGET_FLAG_UNIT_RAID;
+                        case TARGET_SELECT_CHECK_PASSENGER:
+                            return TARGET_FLAG_UNIT_PASSENGER;
+                        default:
+                            return TARGET_FLAG_UNIT;
+                    }
+                    break;
+                default:
+                    break;
+            }
+            break;
+        default:
+            break;
+    }
+    return TARGET_FLAG_NONE;
+}
+
 bool SpellImplicitTargetInfo::IsPosition(uint32 targetType)
 {
     switch (SpellImplicitTargetInfo::Type[targetType])
@@ -885,6 +969,7 @@ SpellInfo::SpellInfo(SpellEntry const* spellEntry)
     SchoolMask = spellEntry->SchoolMask;
     for (uint8 i = 0; i < MAX_SPELL_EFFECTS; ++i)
         Effects[i] = SpellEffectInfo(spellEntry, this, i);
+    ExplicitTargetMask = _GetExplicitTargetMask();
     ChainEntry = NULL;
 }
 
@@ -1021,17 +1106,7 @@ bool SpellInfo::IsAOE() const
 
 bool SpellInfo::IsRequiringSelectedTarget() const
 {
-    for (uint8 i = 0 ; i < MAX_SPELL_EFFECTS; ++i)
+    return (GetExplicitTargetMask() & TARGET_FLAG_UNIT_MASK) != 0;
-    {
-        if (Effects[i].TargetA.GetType() == TARGET_TYPE_UNIT_TARGET
-            || Effects[i].TargetB.GetType() == TARGET_TYPE_UNIT_TARGET
-            || Effects[i].TargetA.GetType() == TARGET_TYPE_CHANNEL
-            || Effects[i].TargetB.GetType() == TARGET_TYPE_CHANNEL
-            || Effects[i].TargetA.GetType() == TARGET_TYPE_DEST_TARGET
-            || Effects[i].TargetB.GetType() == TARGET_TYPE_DEST_TARGET)
-            return true;
-    }
-    return false;
 }
 
 bool SpellInfo::IsPassive() const
@@ -1672,6 +1747,11 @@ uint32 SpellInfo::GetDispelMask(DispelType type)
         return uint32(1 << type);
 }
 
+uint32 SpellInfo::GetExplicitTargetMask() const
+{
+    return ExplicitTargetMask;
+}
+
 AuraStateType SpellInfo::GetAuraState() const
 {
     // Seals
@@ -2119,6 +2199,81 @@ bool SpellInfo::IsHighRankOf(SpellInfo const* spellInfo) const
     return false;
 }
 
+uint32 SpellInfo::_GetExplicitTargetMask() const
+{
+    bool srcSet = false;
+    bool dstSet = false;
+    uint32 targetMask = Targets;
+    // prepare target mask using effect target entries
+    for (uint8 i = 0; i < MAX_SPELL_EFFECTS; ++i)
+    {
+        if (!Effects[i].IsEffect())
+            continue;
+        targetMask |= Effects[i].TargetA.GetExplicitTargetMask(srcSet, dstSet);
+        targetMask |= Effects[i].TargetB.GetExplicitTargetMask(srcSet, dstSet);
+    }
+    // spells with range may need explicit targets, even if target entries not set
+    // for example many SPELL_EFFECT_LEARN_SPELL spells need to have unit target
+    if (GetMaxRange(true) > 0.0f || GetMaxRange(false) > 0.0f)
+    {
+        for (uint8 i = 0; i < MAX_SPELL_EFFECTS; ++i)
+        {
+            if (!Effects[i].IsEffect())
+                continue;
+
+            uint32 effImplicitTargetMask = GetTargetFlagMask(Effects[i].GetImplicitTargetObjectType());
+            uint32 providedTargetMask = GetTargetFlagMask(Effects[i].TargetA.GetObjectType()) | GetTargetFlagMask(Effects[i].TargetB.GetObjectType()) | targetMask;
+
+            // check if valid targets already present, prevent adding redundant flags
+            switch (Effects[i].GetImplicitTargetObjectType())
+            {
+                case TARGET_OBJECT_TYPE_UNIT_AND_DEST:
+                    if (providedTargetMask & TARGET_FLAG_UNIT_MASK)
+                        effImplicitTargetMask &= ~(TARGET_FLAG_UNIT_MASK);
+                    if (dstSet || providedTargetMask & TARGET_FLAG_DEST_LOCATION)
+                        effImplicitTargetMask &= ~(TARGET_FLAG_DEST_LOCATION);
+                    if (!effImplicitTargetMask)
+                        continue;
+                    break;
+                case TARGET_OBJECT_TYPE_SRC:
+                    if (srcSet || providedTargetMask & TARGET_FLAG_SOURCE_LOCATION)
+                        continue;
+                    break;
+                case TARGET_OBJECT_TYPE_DEST:
+                    if (dstSet || providedTargetMask & TARGET_FLAG_DEST_LOCATION)
+                        continue;
+                    break;
+                case TARGET_OBJECT_TYPE_UNIT:
+                    if (providedTargetMask & TARGET_FLAG_UNIT_MASK)
+                        continue;
+                    break;
+                case TARGET_OBJECT_TYPE_CORPSE:
+                    if (providedTargetMask & (TARGET_FLAG_CORPSE_MASK | TARGET_FLAG_UNIT_MASK))
+                        continue;
+                    break;
+                case TARGET_OBJECT_TYPE_ITEM:
+                    if (providedTargetMask & (TARGET_FLAG_GAMEOBJECT_ITEM | TARGET_FLAG_ITEM))
+                        continue;
+                    break;
+                case TARGET_OBJECT_TYPE_GOBJ:
+                    if (providedTargetMask & TARGET_FLAG_GAMEOBJECT_MASK)
+                        continue;
+                    break;
+                case TARGET_OBJECT_TYPE_GOBJ_ITEM:
+                    if (providedTargetMask & (TARGET_FLAG_GAMEOBJECT_ITEM | TARGET_FLAG_GAMEOBJECT | TARGET_FLAG_ITEM))
+                        continue;
+                    break;
+                default:
+                    continue;
+            }
+
+            // extend explicit target mask only if valid targets for effect could not be provided by target types
+            targetMask |= effImplicitTargetMask &~(providedTargetMask);
+        }
+    }
+    return targetMask;
+}
+
 bool SpellInfo::_IsPositiveEffect(uint8 effIndex, bool deep) const
 {
     // not found a single positive spell with this attribute

src/server/game/Spells/SpellInfo.h

@@ -35,6 +35,37 @@ struct SpellRadiusEntry;
 struct SpellEntry;
 struct SpellCastTimesEntry;
 
+enum SpellCastTargetFlags
+{
+    TARGET_FLAG_NONE            = 0x00000000,
+    TARGET_FLAG_UNUSED_1        = 0x00000001,               // not used
+    TARGET_FLAG_UNIT            = 0x00000002,               // pguid
+    TARGET_FLAG_UNIT_RAID       = 0x00000004,               // not sent, used to validate target (if raid member)
+    TARGET_FLAG_UNIT_PARTY      = 0x00000008,               // not sent, used to validate target (if party member)
+    TARGET_FLAG_ITEM            = 0x00000010,               // pguid
+    TARGET_FLAG_SOURCE_LOCATION = 0x00000020,               // pguid, 3 float
+    TARGET_FLAG_DEST_LOCATION   = 0x00000040,               // pguid, 3 float
+    TARGET_FLAG_UNIT_ENEMY      = 0x00000080,               // not sent, used to validate target (if enemy)
+    TARGET_FLAG_UNIT_ALLY       = 0x00000100,               // not sent, used to validate target (if ally)
+    TARGET_FLAG_CORPSE_ENEMY    = 0x00000200,               // pguid
+    TARGET_FLAG_UNIT_DEAD       = 0x00000400,               // not sent, used to validate target (if dead creature)
+    TARGET_FLAG_GAMEOBJECT      = 0x00000800,               // pguid, used with TARGET_GAMEOBJECT_TARGET
+    TARGET_FLAG_TRADE_ITEM      = 0x00001000,               // pguid
+    TARGET_FLAG_STRING          = 0x00002000,               // string
+    TARGET_FLAG_GAMEOBJECT_ITEM = 0x00004000,               // not sent, used with TARGET_GAMEOBJECT_ITEM_TARGET
+    TARGET_FLAG_CORPSE_ALLY     = 0x00008000,               // pguid
+    TARGET_FLAG_UNIT_MINIPET    = 0x00010000,               // pguid, used to validate target (if non combat pet)
+    TARGET_FLAG_GLYPH_SLOT      = 0x00020000,               // used in glyph spells
+    TARGET_FLAG_DEST_TARGET     = 0x00040000,               // sometimes appears with DEST_TARGET spells (may appear or not for a given spell)
+    TARGET_FLAG_UNUSED20        = 0x00080000,               // uint32 counter, loop { vec3 - screen position (?), guid }, not used so far
+    TARGET_FLAG_UNIT_PASSENGER  = 0x00100000,               // guessed, used to validate target (if vehicle passenger)
+
+    TARGET_FLAG_UNIT_MASK = TARGET_FLAG_UNIT | TARGET_FLAG_UNIT_RAID | TARGET_FLAG_UNIT_PARTY 
+        | TARGET_FLAG_UNIT_ENEMY | TARGET_FLAG_UNIT_ALLY | TARGET_FLAG_UNIT_DEAD | TARGET_FLAG_UNIT_MINIPET | TARGET_FLAG_UNIT_PASSENGER,
+    TARGET_FLAG_GAMEOBJECT_MASK = TARGET_FLAG_GAMEOBJECT | TARGET_FLAG_GAMEOBJECT_ITEM,
+    TARGET_FLAG_CORPSE_MASK = TARGET_FLAG_CORPSE_ALLY | TARGET_FLAG_CORPSE_ENEMY,
+};
+
 enum SpellEffectTargetTypes
 {
     SPELL_REQUIRE_NONE,
@@ -171,6 +202,8 @@ enum SpellCustomAttributes
     SPELL_ATTR0_CU_NEGATIVE                      = SPELL_ATTR0_CU_NEGATIVE_EFF0 | SPELL_ATTR0_CU_NEGATIVE_EFF1 | SPELL_ATTR0_CU_NEGATIVE_EFF2,
 };
 
+uint32 GetTargetFlagMask(SpellTargetObjectTypes objType);
+
 class SpellImplicitTargetInfo
 {
 private:
@@ -189,6 +222,7 @@ public:
     float CalcDirectionAngle() const;
 
     Targets GetTarget() const;
+    uint32 GetExplicitTargetMask(bool& srcSet, bool& dstSet) const;
 
     // temporarily avalible to public
     static bool IsPosition(uint32 targetType);
@@ -354,6 +388,7 @@ public:
     int32  AreaGroupId;
     uint32 SchoolMask;
     SpellEffectInfo Effects[MAX_SPELL_EFFECTS];
+    uint32 ExplicitTargetMask;
     SpellChainNode const* ChainEntry;
 
     SpellInfo(SpellEntry const* spellEntry);
@@ -414,6 +449,7 @@ public:
     Mechanics GetEffectMechanic(uint8 effIndex) const;
     uint32 GetDispelMask() const;
     static uint32 GetDispelMask(DispelType type);
+    uint32 GetExplicitTargetMask() const;
 
     AuraStateType GetAuraState() const;
     SpellSpecificType GetSpellSpecific() const;
@@ -441,6 +477,7 @@ public:
     bool IsHighRankOf(SpellInfo const* spellInfo) const;
 
     // loading helpers
+    uint32 _GetExplicitTargetMask() const;
     bool _IsPositiveEffect(uint8 effIndex, bool deep) const;
     bool _IsPositiveSpell() const;
     static bool _IsPositiveTarget(uint32 targetA, uint32 targetB);

src/server/game/Spells/SpellMgr.cpp

@@ -2876,16 +2876,6 @@ void SpellMgr::LoadDbcDataCorrections()
                         spellInfo->Effect[j] = SPELL_EFFECT_TRIGGER_MISSILE;
                     break;
             }
-
-            switch (SpellImplicitTargetInfo::Type[spellInfo->EffectImplicitTargetA[j]])
-            {
-                case TARGET_TYPE_UNIT_TARGET:
-                case TARGET_TYPE_DEST_TARGET:
-                    spellInfo->Targets |= TARGET_FLAG_UNIT;
-                    break;
-                default:
-                    break;
-            }
         }
 
         if (spellInfo->activeIconID == 2158)  // flight