Fixed broken missile visuals in certain DST and SRC type spells (Void Reaver's Arcane Orb and many others). Achieved by fixing wildly incorrect SMSG_SPELL_GO target mask data handling.

--HG--
branch : trunk

src/game/Spell.cpp

@@ -127,8 +127,10 @@ SpellCastTargets::SpellCastTargets() : m_elevation(0), m_speed(0)
     m_itemTargetEntry  = 0;
 
     m_srcPos.Relocate(0,0,0,0);
+    m_dstPos.Relocate(0,0,0,0);
     m_strTarget = "";
     m_targetMask = 0;
+    m_intTargetFlags = 0;
 }
 
 SpellCastTargets::~SpellCastTargets()
@@ -142,13 +144,13 @@ void SpellCastTargets::setUnitTarget(Unit *target)
 
     m_unitTarget = target;
     m_unitTargetGUID = target->GetGUID();
-    m_targetMask |= TARGET_FLAG_UNIT;
+    m_intTargetFlags |= FLAG_INT_UNIT;
 }
 
 void SpellCastTargets::setSrc(float x, float y, float z)
 {
     m_srcPos.Relocate(x, y, z);
-    m_targetMask |= TARGET_FLAG_SOURCE_LOCATION;
+    m_intTargetFlags |= FLAG_INT_SRC_LOC;
 }
 
 void SpellCastTargets::setSrc(Position *pos)
@@ -156,14 +158,14 @@ void SpellCastTargets::setSrc(Position *pos)
     if(pos)
     {
         m_srcPos.Relocate(pos);
-        m_targetMask |= TARGET_FLAG_SOURCE_LOCATION;
+        m_intTargetFlags |= FLAG_INT_SRC_LOC;
     }
 }
 
 void SpellCastTargets::setDst(float x, float y, float z, float orientation, uint32 mapId)
 {
     m_dstPos.Relocate(x, y, z, orientation);
-    m_targetMask |= TARGET_FLAG_DEST_LOCATION;
+    m_intTargetFlags |= FLAG_INT_DST_LOC;
     if(mapId != MAPID_INVALID)
         m_dstPos.m_mapId = mapId;
 }
@@ -173,7 +175,7 @@ void SpellCastTargets::setDst(Position *pos)
     if(pos)
     {
         m_dstPos.Relocate(pos);
-        m_targetMask |= TARGET_FLAG_DEST_LOCATION;
+        m_intTargetFlags |= FLAG_INT_DST_LOC;
     }
 }
 
@@ -181,7 +183,7 @@ void SpellCastTargets::setGOTarget(GameObject *target)
 {
     m_GOTarget = target;
     m_GOTargetGUID = target->GetGUID();
-    //    m_targetMask |= TARGET_FLAG_OBJECT;
+    m_intTargetFlags |= FLAG_INT_OBJECT;
 }
 
 void SpellCastTargets::setItemTarget(Item* item)
@@ -237,12 +239,19 @@ bool SpellCastTargets::read ( WorldPacket * data, Unit *caster )
 
     // TARGET_FLAG_UNK2 is used for non-combat pets, maybe other?
     if( m_targetMask & ( TARGET_FLAG_UNIT | TARGET_FLAG_UNK2 ))
+    {
         if(!data->readPackGUID(m_unitTargetGUID))
             return false;
+        if (m_targetMask & TARGET_FLAG_UNIT)
+            m_intTargetFlags |= FLAG_INT_UNIT;
+    }
 
     if( m_targetMask & ( TARGET_FLAG_OBJECT ))
+    {
         if(!data->readPackGUID(m_GOTargetGUID))
             return false;
+        m_intTargetFlags |= FLAG_INT_OBJECT;
+    }
 
     if(( m_targetMask & ( TARGET_FLAG_ITEM | TARGET_FLAG_TRADE_ITEM )) && caster->GetTypeId() == TYPEID_PLAYER)
         if(!data->readPackGUID(m_itemTargetGUID))
@@ -263,6 +272,7 @@ bool SpellCastTargets::read ( WorldPacket * data, Unit *caster )
         *data >> m_srcPos.m_positionX >> m_srcPos.m_positionY >> m_srcPos.m_positionZ;
         if(!m_srcPos.IsPositionValid())
             return false;
+         m_intTargetFlags |= FLAG_INT_SRC_LOC;
     }
     else
         m_srcPos.Relocate(caster);
@@ -279,6 +289,8 @@ bool SpellCastTargets::read ( WorldPacket * data, Unit *caster )
         if(!m_dstPos.IsPositionValid())
             return false;
 
+        m_intTargetFlags |= FLAG_INT_DST_LOC;
+
         if( m_targetMask & TARGET_FLAG_SOURCE_LOCATION )
         {
             if(data->rpos() + 4 + 4 <= data->size())
@@ -344,21 +356,13 @@ void SpellCastTargets::write ( WorldPacket * data )
 
     if( m_targetMask & TARGET_FLAG_SOURCE_LOCATION )
     {
-        if(m_unitTarget)
+        *data << uint8(0); // It seems the client doesn't like unit target GUID being sent here, we must send 0
-            data->append(m_unitTarget->GetPackGUID());
-        else
-            *data << uint8(0);
-
         *data << m_srcPos.m_positionX << m_srcPos.m_positionY << m_srcPos.m_positionZ;
     }
 
     if( m_targetMask & TARGET_FLAG_DEST_LOCATION )
     {
-        if(m_unitTarget)
+        *data << uint8(0); // It seems the client doesn't like unit target GUID being sent here, we must send 0
-            data->append(m_unitTarget->GetPackGUID());
-        else
-            *data << uint8(0);
-
         *data << m_dstPos.m_positionX << m_dstPos.m_positionY << m_dstPos.m_positionZ;
     }
 
@@ -2290,7 +2294,6 @@ void Spell::SelectEffectTargets(uint32 i, uint32 cur)
                 {
                     case 46584: // Raise Dead
                     {
-                        m_targets.m_targetMask &= ~TARGET_FLAG_DEST_LOCATION;
                         if (WorldObject* result = FindCorpseUsing<Trinity::RaiseDeadObjectCheck> ())
                         {
                             switch(result->GetTypeId())
@@ -2597,6 +2600,30 @@ void Spell::prepare(SpellCastTargets const* targets, AuraEffect const * triggere
         }
     }
 
+    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() && !IsPassiveSpell(m_spellInfo->Id) && m_spellInfo->spellLevel && !IsChanneledSpell(m_spellInfo) && !m_IsTriggeredSpell)
     {
@@ -2697,7 +2724,7 @@ void Spell::prepare(SpellCastTargets const* targets, AuraEffect const * triggere
     // set timer base at cast time
     ReSetTimer();
 
-    sLog.outDebug("Spell::prepare: spell id %u source %u caster %d triggered %u mask %u", m_spellInfo->Id, m_caster->GetEntry(), m_originalCaster ? m_originalCaster->GetEntry() : -1, m_IsTriggeredSpell ? 1 : 0, m_targets.m_targetMask);
+    sLog.outDebug("Spell::prepare: spell id %u source %u caster %d triggered %u mask %u", m_spellInfo->Id, m_caster->GetEntry(), m_originalCaster ? m_originalCaster->GetEntry() : -1, m_IsTriggeredSpell ? 1 : 0, m_targets.getTargetMask());
     //if(m_targets.getUnitTarget())
     //    sLog.outError("Spell::prepare: unit target %u", m_targets.getUnitTarget()->GetEntry());
     //if(m_targets.HasDst())
@@ -3045,7 +3072,7 @@ uint64 Spell::handle_delayed(uint64 t_offset)
         m_immediateHandled = true;
     }
 
-    bool single_missile = (m_targets.m_targetMask & TARGET_FLAG_DEST_LOCATION);
+    bool single_missile = (m_targets.getIntTargetFlags() & FLAG_INT_DST_LOC);
 
     // now recheck units targeting correctness (need before any effects apply to prevent adding immunity at first effect not allow apply second spell effect and similar cases)
     for (std::list<TargetInfo>::iterator ihit= m_UniqueTargetInfo.begin(); ihit != m_UniqueTargetInfo.end(); ++ihit)
@@ -3530,6 +3557,19 @@ void Spell::SendSpellStart()
     data << uint32(castFlags);                              // cast flags
     data << uint32(m_timer);                                // delay?
 
+    // Preliminary setting of the target mask for the SMSG_SPELL_START packet. This will be
+    // adjusted again later in SendSpellGo() based on the real targets obtained in SelectSpellTargets()
+    if (m_spellInfo->Targets & TARGET_FLAG_SOURCE_LOCATION)
+        m_targets.setTargetMask(m_targets.getTargetMask() | TARGET_FLAG_SOURCE_LOCATION);
+
+    if (m_spellInfo->Targets & TARGET_FLAG_DEST_LOCATION)
+        m_targets.setTargetMask(m_targets.getTargetMask() | TARGET_FLAG_DEST_LOCATION);
+
+    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);
+
     m_targets.write(&data);
 
     if(castFlags & CAST_FLAG_POWER_LEFT_SELF)
@@ -3597,6 +3637,22 @@ void Spell::SendSpellGo()
     data << uint32(castFlags);                              // cast flags
     data << uint32(getMSTime());                            // timestamp
 
+    if ((m_spellInfo->Targets & TARGET_FLAG_SOURCE_LOCATION) && m_targets.HasSrc())
+        m_targets.setTargetMask(m_targets.getTargetMask() | TARGET_FLAG_SOURCE_LOCATION);
+    else 
+        m_targets.setTargetMask(m_targets.getTargetMask() & ~TARGET_FLAG_SOURCE_LOCATION);
+
+    if ((m_spellInfo->Targets & TARGET_FLAG_DEST_LOCATION) && m_targets.HasDst())
+        m_targets.setTargetMask(m_targets.getTargetMask() | TARGET_FLAG_DEST_LOCATION);
+    else
+        m_targets.setTargetMask(m_targets.getTargetMask() & ~TARGET_FLAG_DEST_LOCATION);
+
+    // 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);
@@ -3634,7 +3690,7 @@ void Spell::SendSpellGo()
         data << uint32(0);
     }
 
-    if ( m_targets.m_targetMask & TARGET_FLAG_DEST_LOCATION )
+    if ( m_targets.getTargetMask() & TARGET_FLAG_DEST_LOCATION )
     {
         data << uint8(0);
     }
@@ -4514,7 +4570,7 @@ SpellCastResult Spell::CheckCast(bool strict)
                 // 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.m_targetMask == TARGET_FLAG_SELF &&
+                if (m_targets.getTargetMask() == TARGET_FLAG_SELF &&
                     m_spellInfo->EffectImplicitTargetA[1] == TARGET_UNIT_TARGET_ENEMY)
                 {
                     if (target = m_caster->GetUnit(*m_caster, m_caster->ToPlayer()->GetSelection()))

src/game/Spell.h

@@ -30,6 +30,16 @@ class GameObject;
 class DynamicObject;
 class Aura;
 
+// These flags represent the inner states of the targeting system
+enum SpellInternalTargetFlags
+{
+    FLAG_INT_UNIT               = 0x00000002,
+    FLAG_INT_SRC_LOC            = 0x00000020,
+    FLAG_INT_DST_LOC            = 0x00000040,
+    FLAG_INT_OBJECT             = 0x00000800
+};
+
+// These flags are used in client - server communication only
 enum SpellCastTargetFlags
 {
     TARGET_FLAG_SELF            = 0x00000000,
@@ -143,10 +153,15 @@ class SpellCastTargets
             m_strTarget = target.m_strTarget;
 
             m_targetMask = target.m_targetMask;
+            m_intTargetFlags = target.getIntTargetFlags();
 
             return *this;
         }
 
+        uint32 getTargetMask() const { return m_targetMask; }
+        void setTargetMask(uint32 newMask) { m_targetMask = newMask; }
+        uint32 getIntTargetFlags() const { return m_intTargetFlags; }
+        
         uint64 getUnitTargetGUID() const { return m_unitTargetGUID; }
         Unit *getUnitTarget() const { return m_unitTarget; }
         void setUnitTarget(Unit *target);
@@ -175,8 +190,8 @@ class SpellCastTargets
         }
 
         bool IsEmpty() const { return m_GOTargetGUID==0 && m_unitTargetGUID==0 && m_itemTarget==0 && m_CorpseTargetGUID==0; }
-        bool HasSrc() const { return m_targetMask & TARGET_FLAG_SOURCE_LOCATION; }
+        bool HasSrc() const { return m_intTargetFlags & FLAG_INT_SRC_LOC; }
-        bool HasDst() const { return m_targetMask & TARGET_FLAG_DEST_LOCATION; }
+        bool HasDst() const { return m_intTargetFlags & FLAG_INT_DST_LOC; }
         bool HasTraj() const { return m_speed != 0; }
 
         float GetDist2d() const { return m_srcPos.GetExactDist2d(&m_dstPos); }
@@ -190,8 +205,9 @@ class SpellCastTargets
         float m_elevation, m_speed;
         std::string m_strTarget;
 
-        uint32 m_targetMask;
     private:
+        uint32 m_targetMask;
+        uint32 m_intTargetFlags;
         // objects (can be used at spell creating and after Update at casting
         Unit *m_unitTarget;
         GameObject *m_GOTarget;

src/game/SpellEffects.cpp

@@ -4811,7 +4811,7 @@ void Spell::EffectSummonObjectWild(uint32 i)
         target = m_caster;
 
     float x, y, z;
-    if(m_targets.m_targetMask & TARGET_FLAG_DEST_LOCATION)
+    if(m_targets.HasDst())
         m_targets.m_dstPos.GetPosition(x, y, z);
     else
         m_caster->GetClosePoint(x, y, z, DEFAULT_WORLD_OBJECT_SIZE);
@@ -7492,7 +7492,7 @@ void Spell::SummonGuardian(uint32 i, uint32 entry, SummonPropertiesEntry const *
             ((Guardian*)summon)->InitStatsForLevel(level);
 
         summon->SetUInt32Value(UNIT_CREATED_BY_SPELL, m_spellInfo->Id);
-        if (summon->HasUnitTypeMask(UNIT_MASK_MINION) && m_targets.m_targetMask & TARGET_FLAG_DEST_LOCATION)
+        if (summon->HasUnitTypeMask(UNIT_MASK_MINION) && m_targets.HasDst())
             ((Minion*)summon)->SetFollowAngle(m_caster->GetAngle(summon));
 
         if (summon->GetEntry() == 27893)
@@ -7512,7 +7512,7 @@ void Spell::GetSummonPosition(uint32 i, Position &pos, float radius, uint32 coun
 {
     pos.SetOrientation(m_caster->GetOrientation());
 
-    if (m_targets.m_targetMask & TARGET_FLAG_DEST_LOCATION)
+    if (m_targets.HasDst())
     {
         // Summon 1 unit in dest location
         if (count == 0)

src/game/SpellMgr.cpp

@@ -3506,10 +3506,6 @@ void SpellMgr::LoadSpellCustomAttr()
                     spellInfo->Targets |= TARGET_FLAG_UNIT;
                     count++;
                     break;
-                //case TARGET_TYPE_AREA_DST:
-                //case TARGET_TYPE_DEST_DEST:
-                //    spellInfo->Targets |= TARGET_FLAG_DEST_LOCATION;
-                //    break;
             }
         }
 

src/game/Unit.cpp

@@ -884,39 +884,8 @@ void Unit::CastSpell(Unit* Victim,SpellEntry const *spellInfo, bool triggered, I
             originalCaster=owner->GetGUID();
 
     SpellCastTargets targets;
-    uint32 targetMask = spellInfo->Targets;
-    //if (targetMask & (TARGET_FLAG_UNIT|TARGET_FLAG_UNK2))
-    for (uint8 i = 0; i < MAX_SPELL_EFFECTS; ++i)
-    {
-        if (SpellTargetType[spellInfo->EffectImplicitTargetA[i]] == TARGET_TYPE_UNIT_TARGET)
-        {
-            /*SpellRangeEntry const* srange = sSpellRangeStore.LookupEntry(spellInfo->rangeIndex);
-            if (srange && GetSpellMaxRange(srange) == 0.0f)
-            {
-                Victim = this;
-                break;
-            }
-            else */if (!Victim)
-            {
-                sLog.outError("CastSpell: spell id %i by caster: %s %u) does not have unit target", spellInfo->Id,(GetTypeId() == TYPEID_PLAYER ? "player (GUID:" : "creature (Entry:"),(GetTypeId() == TYPEID_PLAYER ? GetGUIDLow() : GetEntry()));
-                return;
-            }
-            else
-                break;
-        }
-    }
     targets.setUnitTarget(Victim);
 
-    if (targetMask & (TARGET_FLAG_SOURCE_LOCATION|TARGET_FLAG_DEST_LOCATION))
-    {
-        if (!Victim)
-        {
-            sLog.outError("CastSpell: spell id %i by caster: %s %u) does not have destination", spellInfo->Id,(GetTypeId() == TYPEID_PLAYER ? "player (GUID:" : "creature (Entry:"),(GetTypeId() == TYPEID_PLAYER ? GetGUIDLow() : GetEntry()));
-            return;
-        }
-        targets.setDst(Victim);
-    }
-
     if (castItem)
         DEBUG_LOG("WORLD: cast Item spellId - %i", spellInfo->Id);
 
@@ -958,35 +927,8 @@ void Unit::CastCustomSpell(uint32 spellId, CustomSpellValues const &value, Unit*
     }
 
     SpellCastTargets targets;
-    uint32 targetMask = spellInfo->Targets;
-
-    //check unit target
-    for (uint8 i = 0; i < MAX_SPELL_EFFECTS; ++i)
-    {
-        if (SpellTargetType[spellInfo->EffectImplicitTargetA[i]] == TARGET_TYPE_UNIT_TARGET)
-        {
-            if (!Victim)
-            {
-                sLog.outError("CastSpell: spell id %i by caster: %s %u) does not have unit target", spellInfo->Id,(GetTypeId() == TYPEID_PLAYER ? "player (GUID:" : "creature (Entry:"),(GetTypeId() == TYPEID_PLAYER ? GetGUIDLow() : GetEntry()));
-                return;
-            }
-            else
-                break;
-        }
-    }
     targets.setUnitTarget(Victim);
 
-    //check destination
-    if (targetMask & (TARGET_FLAG_SOURCE_LOCATION|TARGET_FLAG_DEST_LOCATION))
-    {
-        if (!Victim)
-        {
-            sLog.outError("CastSpell: spell id %i by caster: %s %u) does not have destination", spellInfo->Id,(GetTypeId() == TYPEID_PLAYER ? "player (GUID:" : "creature (Entry:"),(GetTypeId() == TYPEID_PLAYER ? GetGUIDLow() : GetEntry()));
-            return;
-        }
-        targets.setDst(Victim);
-    }
-
     if (!originalCaster && triggeredByAura)
         originalCaster = triggeredByAura->GetCasterGUID();