[Cleanup][8191] Remove long-time unused spell_affect table

[XTZGZoReX]

After talking it over with vladimir on IRC, we agreed that we no longer have need for this table. The original purpose of it was to override DBC values if needed, but so far (for many months), that has not been needed, and the table is pretty much obsolete now.

Update SQL: DROP TABLE IF EXISTS spell_affect;

Patch:

diff --git a/sql/mangos.sql b/sql/mangos.sql
index c63f3b6..6d93bee 100644
--- a/sql/mangos.sql
+++ b/sql/mangos.sql
@@ -13457,29 +13457,6 @@ LOCK TABLES `skinning_loot_template` WRITE;
UNLOCK TABLES;

--
--- Table structure for table `spell_affect`
---
-
-DROP TABLE IF EXISTS `spell_affect`;
-CREATE TABLE `spell_affect` (
-  `entry` smallint(5) unsigned NOT NULL default '0',
-  `effectId` tinyint(3) unsigned NOT NULL default '0',
-  `SpellClassMask0` int(5) unsigned NOT NULL default '0',
-  `SpellClassMask1` int(5) unsigned NOT NULL default '0',
-  `SpellClassMask2` int(5) unsigned NOT NULL default '0',
-  PRIMARY KEY  (`entry`,`effectId`)
-) ENGINE=MyISAM DEFAULT CHARSET=utf8;
-
---
--- Dumping data for table `spell_affect`
---
-
-LOCK TABLES `spell_affect` WRITE;
-/*!40000 ALTER TABLE `spell_affect` DISABLE KEYS */;
-/*!40000 ALTER TABLE `spell_affect` ENABLE KEYS */;
-UNLOCK TABLES;
-
---
-- Table structure for table `spell_area`
--

diff --git a/src/game/Chat.cpp b/src/game/Chat.cpp
index af7c914..05ae1ef 100644
--- a/src/game/Chat.cpp
+++ b/src/game/Chat.cpp
@@ -442,7 +442,6 @@ ChatCommand * ChatHandler::getCommandTable()
        { "skill_extra_item_template",   SEC_ADMINISTRATOR, true,  &ChatHandler::HandleReloadSkillExtraItemTemplateCommand,  "", NULL },
        { "skill_fishing_base_level",    SEC_ADMINISTRATOR, true,  &ChatHandler::HandleReloadSkillFishingBaseLevelCommand,   "", NULL },
        { "skinning_loot_template",      SEC_ADMINISTRATOR, true,  &ChatHandler::HandleReloadLootTemplatesSkinningCommand,   "", NULL },
-        { "spell_affect",                SEC_ADMINISTRATOR, true,  &ChatHandler::HandleReloadSpellAffectCommand,             "", NULL },
        { "spell_area",                  SEC_ADMINISTRATOR, true,  &ChatHandler::HandleReloadSpellAreaCommand,               "", NULL },
        { "spell_bonus_data",            SEC_ADMINISTRATOR, true,  &ChatHandler::HandleReloadSpellBonusesCommand,            "", NULL },
        { "spell_chain",                 SEC_ADMINISTRATOR, true,  &ChatHandler::HandleReloadSpellChainCommand,              "", NULL },
diff --git a/src/game/Chat.h b/src/game/Chat.h
index cd235d4..84b3cb4 100644
--- a/src/game/Chat.h
+++ b/src/game/Chat.h
@@ -369,7 +369,6 @@ class ChatHandler
        bool HandleReloadSkillDiscoveryTemplateCommand(const char* args);
        bool HandleReloadSkillExtraItemTemplateCommand(const char* args);
        bool HandleReloadSkillFishingBaseLevelCommand(const char* args);
-        bool HandleReloadSpellAffectCommand(const char* args);
        bool HandleReloadSpellAreaCommand(const char* args);
        bool HandleReloadSpellChainCommand(const char* args);
        bool HandleReloadSpellElixirCommand(const char* args);
diff --git a/src/game/Level3.cpp b/src/game/Level3.cpp
index 65fb77a..1b5ba65 100644
--- a/src/game/Level3.cpp
+++ b/src/game/Level3.cpp
@@ -151,7 +151,6 @@ bool ChatHandler::HandleReloadAllSpellCommand(const char*)
{
    HandleReloadSkillDiscoveryTemplateCommand("a");
    HandleReloadSkillExtraItemTemplateCommand("a");
-    HandleReloadSpellAffectCommand("a");
    HandleReloadSpellAreaCommand("a");
    HandleReloadSpellChainCommand("a");
    HandleReloadSpellElixirCommand("a");
@@ -481,14 +480,6 @@ bool ChatHandler::HandleReloadSkillFishingBaseLevelCommand(const char* /*args*/)
    return true;
}

-bool ChatHandler::HandleReloadSpellAffectCommand(const char*)
-{
-    sLog.outString( "Re-Loading SpellAffect definitions..." );
-    spellmgr.LoadSpellAffects();
-    SendGlobalSysMessage("DB table `spell_affect` (spell mods apply requirements) reloaded.");
-    return true;
-}
-
bool ChatHandler::HandleReloadSpellAreaCommand(const char*)
{
    sLog.outString( "Re-Loading SpellArea Data..." );
diff --git a/src/game/Spell.cpp b/src/game/Spell.cpp
diff --git a/src/game/SpellAuras.cpp b/src/game/SpellAuras.cpp
index df1cd54..2246b78 100644
--- a/src/game/SpellAuras.cpp
+++ b/src/game/SpellAuras.cpp
@@ -1283,19 +1283,13 @@ void Aura::HandleAddModifier(bool apply, bool Real)
        mod->spellId = GetId();

        uint32 const *ptr;
-        SpellAffectEntry const *spellAffect = spellmgr.GetSpellAffect(GetId(), m_effIndex);
-        if (spellAffect)
-            ptr = &spellAffect->SpellClassMask[0];
-        else
+        switch (m_effIndex)
        {
-            switch (m_effIndex)
-            {
-                case 0: ptr = &m_spellProto->EffectSpellClassMaskA[0]; break;
-                case 1: ptr = &m_spellProto->EffectSpellClassMaskB[0]; break;
-                case 2: ptr = &m_spellProto->EffectSpellClassMaskC[0]; break;
-                default:
-                    return;
-            }
+            case 0: ptr = &m_spellProto->EffectSpellClassMaskA[0]; break;
+            case 1: ptr = &m_spellProto->EffectSpellClassMaskB[0]; break;
+            case 2: ptr = &m_spellProto->EffectSpellClassMaskC[0]; break;
+            default:
+                return;
        }

        mod->mask = (uint64)ptr[0] | (uint64)ptr[1]<<32;
@@ -1329,19 +1323,13 @@ void Aura::HandleAddTargetTrigger(bool apply, bool /*Real*/)
        mod->spellId = GetId();

        uint32 const *ptr;
-        SpellAffectEntry const *spellAffect = spellmgr.GetSpellAffect(GetId(), m_effIndex);
-        if (spellAffect)
-            ptr = &spellAffect->SpellClassMask[0];
-        else
+        switch (m_effIndex)
        {
-            switch (m_effIndex)
-            {
-                case 0: ptr = &m_spellProto->EffectSpellClassMaskA[0]; break;
-                case 1: ptr = &m_spellProto->EffectSpellClassMaskB[0]; break;
-                case 2: ptr = &m_spellProto->EffectSpellClassMaskC[0]; break;
-                default:
-                    return;
-            }
+            case 0: ptr = &m_spellProto->EffectSpellClassMaskA[0]; break;
+            case 1: ptr = &m_spellProto->EffectSpellClassMaskB[0]; break;
+            case 2: ptr = &m_spellProto->EffectSpellClassMaskC[0]; break;
+            default:
+                return;
        }

        mod->mask = (uint64)ptr[0] | (uint64)ptr[1]<<32;
diff --git a/src/game/SpellMgr.cpp b/src/game/SpellMgr.cpp
index 1dd4544..eb3bc84 100644
--- a/src/game/SpellMgr.cpp
+++ b/src/game/SpellMgr.cpp
@@ -692,126 +692,6 @@ void SpellMgr::LoadSpellTargetPositions()
    sLog.outString( ">> Loaded %u spell teleport coordinates", count );
}

-void SpellMgr::LoadSpellAffects()
-{
-    mSpellAffectMap.clear();                                // need for reload case
-
-    uint32 count = 0;
-
-    //                                                0      1         2                3                4
-    QueryResult *result = WorldDatabase.Query("SELECT entry, effectId, SpellClassMask0, SpellClassMask1, SpellClassMask2 FROM spell_affect");
-    if( !result )
-    {
-
-        barGoLink bar( 1 );
-
-        bar.step();
-
-        sLog.outString();
-        sLog.outString( ">> Loaded %u spell affect definitions", count );
-        return;
-    }
-
-    barGoLink bar( result->GetRowCount() );
-
-    do
-    {
-        Field *fields = result->Fetch();
-
-        bar.step();
-
-        uint32 entry = fields[0].GetUInt32();
-        uint8 effectId = fields[1].GetUInt8();
-
-        SpellEntry const* spellInfo = sSpellStore.LookupEntry(entry);
-
-        if (!spellInfo)
-        {
-            sLog.outErrorDb("Spell %u listed in `spell_affect` does not exist", entry);
-            continue;
-        }
-
-        if (effectId >= 3)
-        {
-            sLog.outErrorDb("Spell %u listed in `spell_affect` have invalid effect index (%u)", entry,effectId);
-            continue;
-        }
-
-        if( spellInfo->Effect[effectId] != SPELL_EFFECT_APPLY_AURA ||
-            spellInfo->EffectApplyAuraName[effectId] != SPELL_AURA_ADD_FLAT_MODIFIER &&
-            spellInfo->EffectApplyAuraName[effectId] != SPELL_AURA_ADD_PCT_MODIFIER  &&
-            spellInfo->EffectApplyAuraName[effectId] != SPELL_AURA_ADD_TARGET_TRIGGER )
-        {
-            sLog.outErrorDb("Spell %u listed in `spell_affect` have not SPELL_AURA_ADD_FLAT_MODIFIER (%u) or SPELL_AURA_ADD_PCT_MODIFIER (%u) or SPELL_AURA_ADD_TARGET_TRIGGER (%u) for effect index (%u)", entry,SPELL_AURA_ADD_FLAT_MODIFIER,SPELL_AURA_ADD_PCT_MODIFIER,SPELL_AURA_ADD_TARGET_TRIGGER,effectId);
-            continue;
-        }
-
-        SpellAffectEntry affect;
-        affect.SpellClassMask[0] = fields[2].GetUInt32();
-        affect.SpellClassMask[1] = fields[3].GetUInt32();
-        affect.SpellClassMask[2] = fields[4].GetUInt32();
-
-        // Spell.dbc have own data
-        uint32 const *ptr = 0;
-        switch (effectId)
-        {
-            case 0: ptr = &spellInfo->EffectSpellClassMaskA[0]; break;
-            case 1: ptr = &spellInfo->EffectSpellClassMaskB[0]; break;
-            case 2: ptr = &spellInfo->EffectSpellClassMaskC[0]; break;
-            default:
-                continue;
-        }
-        if(ptr[0] == affect.SpellClassMask[0] || ptr[1] == affect.SpellClassMask[1] || ptr[2] == affect.SpellClassMask[2])
-        {
-            char text[]="ABC";
-            sLog.outErrorDb("Spell %u listed in `spell_affect` have redundant (same with EffectSpellClassMask%c) data for effect index (%u) and not needed, skipped.", entry, text[effectId], effectId);
-            continue;
-        }
-
-        mSpellAffectMap[(entry<<8) + effectId] = affect;
-
-        ++count;
-    } while( result->NextRow() );
-
-    delete result;
-
-    sLog.outString();
-    sLog.outString( ">> Loaded %u custom spell affect definitions", count );
-
-    for (uint32 id = 0; id < sSpellStore.GetNumRows(); ++id)
-    {
-        SpellEntry const* spellInfo = sSpellStore.LookupEntry(id);
-        if (!spellInfo)
-            continue;
-
-        for (int effectId = 0; effectId < 3; ++effectId)
-        {
-            if( spellInfo->Effect[effectId] != SPELL_EFFECT_APPLY_AURA ||
-                (spellInfo->EffectApplyAuraName[effectId] != SPELL_AURA_ADD_FLAT_MODIFIER &&
-                spellInfo->EffectApplyAuraName[effectId] != SPELL_AURA_ADD_PCT_MODIFIER  &&
-                spellInfo->EffectApplyAuraName[effectId] != SPELL_AURA_ADD_TARGET_TRIGGER) )
-                continue;
-
-            uint32 const *ptr = 0;
-            switch (effectId)
-            {
-                case 0: ptr = &spellInfo->EffectSpellClassMaskA[0]; break;
-                case 1: ptr = &spellInfo->EffectSpellClassMaskB[0]; break;
-                case 2: ptr = &spellInfo->EffectSpellClassMaskC[0]; break;
-                default:
-                    continue;
-            }
-            if(ptr[0] || ptr[1] || ptr[2])
-                continue;
-
-            if(mSpellAffectMap.find((id<<8) + effectId) !=  mSpellAffectMap.end())
-                continue;
-
-            sLog.outErrorDb("Spell %u (%s) misses spell_affect for effect %u",id,spellInfo->SpellName[sWorld.GetDefaultDbcLocale()], effectId);
-        }
-    }
-}
-
bool SpellMgr::IsAffectedByMod(SpellEntry const *spellInfo, SpellModifier *mod) const
{
    // false for spellInfo == NULL
diff --git a/src/game/SpellMgr.h b/src/game/SpellMgr.h
index f6b71ae..cb4e975 100644
--- a/src/game/SpellMgr.h
+++ b/src/game/SpellMgr.h
@@ -335,13 +335,6 @@ bool IsDiminishingReturnsGroupDurationLimited(DiminishingGroup group);
DiminishingReturnsType GetDiminishingReturnsGroupType(DiminishingGroup group);
int32 GetDiminishingReturnsLimitDuration(DiminishingGroup group, SpellEntry const* spellproto);

-// Spell affects related declarations (accessed using SpellMgr functions)
-struct SpellAffectEntry
-{
-    uint32 SpellClassMask[3];
-};
-typedef UNORDERED_MAP<uint32, SpellAffectEntry> SpellAffectMap;
-
// Spell proc event related declarations (accessed using SpellMgr functions)
enum ProcFlags
{
@@ -632,14 +625,6 @@ class SpellMgr

    // Accessors (const or static functions)
    public:
-        // Spell affects
-        SpellAffectEntry const*GetSpellAffect(uint32 spellId, uint8 effectId) const
-        {
-            SpellAffectMap::const_iterator itr = mSpellAffectMap.find((spellId<<8) + effectId);
-            if( itr != mSpellAffectMap.end( ) )
-                return &itr->second;
-            return 0;
-        }

        bool IsAffectedByMod(SpellEntry const *spellInfo, SpellModifier *mod) const;

@@ -899,7 +884,6 @@ class SpellMgr
        void LoadSpellLearnSkills();
        void LoadSpellLearnSpells();
        void LoadSpellScriptTarget();
-        void LoadSpellAffects();
        void LoadSpellElixirs();
        void LoadSpellProcEvents();
        void LoadSpellBonusess();
@@ -918,7 +902,6 @@ class SpellMgr
        SpellLearnSkillMap mSpellLearnSkills;
        SpellLearnSpellMap mSpellLearnSpells;
        SpellTargetPositionMap mSpellTargetPositions;
-        SpellAffectMap     mSpellAffectMap;
        SpellElixirMap     mSpellElixirs;
        SpellProcEventMap  mSpellProcEventMap;
        SpellBonusMap      mSpellBonusMap;
diff --git a/src/game/World.cpp b/src/game/World.cpp
index bf20ccf..b7f3b4a 100644
--- a/src/game/World.cpp
+++ b/src/game/World.cpp
@@ -1253,9 +1253,6 @@ void World::SetInitialWorldSettings()
    sLog.outString( "Loading Spell target coordinates..." );
    spellmgr.LoadSpellTargetPositions();

-    sLog.outString( "Loading SpellAffect definitions..." );
-    spellmgr.LoadSpellAffects();
-
    sLog.outString( "Loading spell pet auras..." );
    spellmgr.LoadSpellPetAuras();

[divisorxx]

It's only a personal opinion but why delete it? I mean if it's working why i has to be deleted even if any use it, nobody knows if in future are going to use it and I think that iis not causing any damage have it. Like you said I never use it but, it's still working like it sopouse I don't see a reason to deleted. It's only an opinion

[Wast3d]

This is done to keep sources clean... Why should it be overloaded by unused spells? this may can bring up problems later on

[XTZGZoReX]

My opinion: If you need to override something with spell_affect... You're doing it wrong . It's a hack in the first place.

[hunuza]

My opinion: If you need to override something with spell_affect... You're doing it wrong . It's a hack in the first place.

It wasn't before 3.x. I introduced it about 3 years ago, because since 1.12.x the EffItemType fields in the dbc were no longer correctly filled. I don't know whether that data is now needed by the client or why they decided to now put the correct data there once again.

If it is still not needed, they could at any point decide to not put the data there anymore, just like they did with 1.12.x. Then we would need the table again.

[XTZGZoReX]

If we really need to override Spell.dbc data, I think it'd be better to just have a table with the exact same structure as the DBC file, where you could override any values.

[hunuza]

It wasn't meant to overwrite data in the Spell.dbc, but to add data that just wasn't there.

EffItemType were just 32 bits, where the SpellFamilyFlags were 64 bits.

Now however, they both are 96 bits.

[XTZGZoReX]

Ah, I understand.

I doubt they're going to remove it, though. That's just more work on their part, isn't it - much more work, actually?

[hunuza]

I doubt they're going to remove it, though. That's just more work on their part, isn't it - much more work, actually?

Well, I don't whether it is more work on their part. They once already did so, that's why I don't see any reason why they should not do it again.

[VladimirMangos]

But it long period time not used... Is we need keep some code just beacuse with some low chance it will need later...

Anyway we easy can revert this patch if it will be added later if we will need this functuonality.

[XTZGZoReX]

But it long period time not used... Is we need keep some code just beacuse with some low chance it will need later...

Anyway we easy can revert this patch if it will be added later if we will need this functuonality.

... the wonders of git .

[VladimirMangos]

In [4191]. Thank you

[Atari007]

In [4191]. Thank you

Probably 8191?

[pelle]

In [4191]. Thank you

lolwut?

[XTZGZoReX]

Haha, wrong rev number

[VladimirMangos]

Fixed in thread title..

[Sephiroth1983]

Hi Vlad, a friend of mine finds the DBC values wrong, sometimes

for example for Twisted Faith - it affects only Mind Blast with DBC data and not Mind Flay

he fixed it by overwriting (forcing) table values in spell_affect.

This is the data:

insert into `spell_affect` (`entry`, `effectId`, `SpellClassMask0`, `SpellClassMask1`, `SpellClassMask2`) values('51167','1','8396800','0','0');
insert into `spell_affect` (`entry`, `effectId`, `SpellClassMask0`, `SpellClassMask1`, `SpellClassMask2`) values('51166','1','8396800','0','0');
insert into `spell_affect` (`entry`, `effectId`, `SpellClassMask0`, `SpellClassMask1`, `SpellClassMask2`) values('47578','1','8396800','0','0');
insert into `spell_affect` (`entry`, `effectId`, `SpellClassMask0`, `SpellClassMask1`, `SpellClassMask2`) values('47573','1','8396800','0','0');
insert into `spell_affect` (`entry`, `effectId`, `SpellClassMask0`, `SpellClassMask1`, `SpellClassMask2`) values('47577','1','8396800','0','0');

What do you think, it is correct?

[VladimirMangos]

Hmm, in 3.0.3 effect 1 have effectClassMask = 0x802000 that match to both, but later have only 0x002000

BUT! How this related in any form. SPELL_AURA_OVERRIDE_CLASS_SCRIPTS not use spell_affect data.

It, or

1) triggred as spell proc event and then affected by spell_proc_event table and need case implementation in to HandleOverrideClassScriptAuraProc.

But htis is _not_ our case.

2) explictly check in related damage code. "damage done by ". So int SpellDamageBonus code