*Add error checks at spell_required table loading

*Allow spell_required table to store more than 1 spell learn requirement for a spell
*spell_required table data is removed from world.sql and since this commit the data for it should be maintained by db project you're using.

--HG--
branch : trunk

6 files changed, 96 insertions, 42 deletions

diff --git a/sql/updates/7261_world_spell_required.sql b/sql/updates/7261_world_spell_required.sql
new file mode 100644
index 00000000000..820a95adca5
--- /dev/null
+++ b/sql/updates/7261_world_spell_required.sql
@@ -0,0 +1,3 @@
+ALTER TABLE spell_required
+DROP PRIMARY KEY,
+ADD PRIMARY KEY (`spell_id`, `req_spell`);
diff --git a/sql/world.sql b/sql/world.sql
index 2d78c8e0a48..bb6b794af29 100644
--- a/sql/world.sql
+++ b/sql/world.sql
@@ -6668,7 +6668,7 @@ DROP TABLE IF EXISTS `spell_required`;
 CREATE TABLE `spell_required` (
   `spell_id` mediumint(9) NOT NULL DEFAULT '0',
   `req_spell` mediumint(9) NOT NULL DEFAULT '0',
-  PRIMARY KEY (`spell_id`)
+  PRIMARY KEY (`spell_id`, `req_spell`)
 ) ENGINE=MyISAM DEFAULT CHARSET=utf8 ROW_FORMAT=FIXED COMMENT='Spell Additinal Data';
 /*!40101 SET character_set_client = @saved_cs_client */;
 
@@ -6678,7 +6678,6 @@ CREATE TABLE `spell_required` (
 
 LOCK TABLES `spell_required` WRITE;
 /*!40000 ALTER TABLE `spell_required` DISABLE KEYS */;
-INSERT INTO `spell_required` VALUES (9788,9785),(10656,10662),(10658,10662),(10660,10662),(16689,339),(16810,1062),(16811,5195),(16812,5196),(16813,9852),(17039,9787),(17040,9787),(17041,9787),(17329,9853),(20219,12656),(20222,12656),(25782,19838),(25894,19854),(25899,20911),(25916,25291),(25918,25290),(26797,26790),(26798,26790),(26801,26790),(27009,26989),(27141,27140),(27143,27142),(27681,14752),(28672,28596),(28675,28596),(28677,28596),(48933,48931),(48934,48932),(48937,48935),(48938,48936),(53312,53308),(34767,34769),(23214,13819),(23161,5784),(40120,33943);
 /*!40000 ALTER TABLE `spell_required` ENABLE KEYS */;
 UNLOCK TABLES;
 
diff --git a/src/game/NPCHandler.cpp b/src/game/NPCHandler.cpp
index 9e674e74706..cba689d0a76 100644
--- a/src/game/NPCHandler.cpp
+++ b/src/game/NPCHandler.cpp
@@ -164,7 +164,6 @@ void WorldSession::SendTrainerList( uint64 guid, const std::string& strTitle )
 
         bool primary_prof_first_rank = spellmgr.IsPrimaryProfessionFirstRankSpell(tSpell->learnedSpell);
         SpellChainNode const* chain_node = spellmgr.GetSpellChainNode(tSpell->learnedSpell);
-        uint32 req_spell = spellmgr.GetSpellRequired(tSpell->spell);
         TrainerSpellState state = _player->GetTrainerSpellState(tSpell);
 
         data << uint32(tSpell->spell);                      // learned spell (or cast-spell in profession case)
@@ -178,9 +177,23 @@ void WorldSession::SendTrainerList( uint64 guid, const std::string& strTitle )
         data << uint32(tSpell->reqSkill);
         data << uint32(tSpell->reqSkillValue);
         //prev + req or req + 0
-        data << uint32(!tSpell->IsCastable() && chain_node && chain_node->prev ? chain_node->prev : req_spell);
-        data << uint32(!tSpell->IsCastable() && chain_node && chain_node->prev ? req_spell : 0);
-        data << uint32(0);
+        uint8 maxReq = 0;
+        if (!tSpell->IsCastable() && chain_node && chain_node->prev)
+        {
+            data << uint32(chain_node->prev);
+            ++maxReq;
+        }
+        SpellsRequiringSpellMapBounds spellsRequired = spellmgr.GetSpellsRequiredForSpellBounds(tSpell->spell);
+        for (SpellsRequiringSpellMap::const_iterator itr2 = spellsRequired.first; itr2 != spellsRequired.second && maxReq < 3; ++itr2)
+        {
+            data << uint32(itr2->second);
+            ++maxReq;
+        }
+        while (maxReq < 3)
+        {
+            data << uint32(0);
+            ++maxReq;
+        }
 
         ++count;
     }
diff --git a/src/game/Player.cpp b/src/game/Player.cpp
index 474c62d0198..e5aff30d2a1 100644
--- a/src/game/Player.cpp
+++ b/src/game/Player.cpp
@@ -3441,10 +3441,9 @@ void Player::removeSpell(uint32 spell_id, bool disabled, bool learn_low_rank)
         removeSpell(node->next,disabled, false);
     }
     //unlearn spells dependent from recently removed spells
-    SpellsRequiringSpellMap const &reqMap = spellmgr.GetSpellsRequiringSpell();
-    SpellsRequiringSpellMap::const_iterator itr2 = reqMap.find(spell_id);
-    for (uint32 i = reqMap.count(spell_id); i > 0; --i, ++itr2)
-        removeSpell(itr2->second,disabled,false);
+    SpellsRequiringSpellMapBounds spellsRequiringSpell = spellmgr.GetSpellsRequiringSpellBounds(spell_id);
+    for (SpellsRequiringSpellMap::const_iterator itr2 = spellsRequiringSpell.first; itr2 != spellsRequiringSpell.second; ++itr2)
+        removeSpell(itr2->second,disabled);
 
     // re-search, it can be corrupted in prev loop
     itr = m_spells.find(spell_id);
@@ -4116,10 +4115,11 @@ TrainerSpellState Player::GetTrainerSpellState(TrainerSpell const* trainer_spell
             return TRAINER_SPELL_RED;
     }
 
-    if(uint32 spell_req = spellmgr.GetSpellRequired(trainer_spell->spell))
+    SpellsRequiringSpellMapBounds spellsRequired = spellmgr.GetSpellsRequiredForSpellBounds(trainer_spell->spell);
+    for (SpellsRequiringSpellMap::const_iterator itr = spellsRequired.first; itr != spellsRequired.second; ++itr)
     {
         // check additional spell requirement
-        if(!HasSpell(spell_req))
+        if(!HasSpell(itr->second))
             return TRAINER_SPELL_RED;
     }
 
@@ -20404,28 +20404,32 @@ void Player::learnQuestRewardedSpells(Quest const* quest)
         if(!learnedInfo)
             return;
 
-        uint32 profSpell = spellmgr.GetSpellRequired(learned_0);
-
-        // specialization
-        if(learnedInfo->Effect[0]==SPELL_EFFECT_TRADE_SKILL && learnedInfo->Effect[1]==0 && profSpell)
+        SpellsRequiringSpellMapBounds spellsRequired = spellmgr.GetSpellsRequiredForSpellBounds(learned_0);
+        for (SpellsRequiringSpellMap::const_iterator itr2 = spellsRequired.first; itr2 != spellsRequired.second; ++itr2)
         {
-            // search other specialization for same prof
-            for (PlayerSpellMap::const_iterator itr = m_spells.begin(); itr != m_spells.end(); ++itr)
+            uint32 profSpell = itr2->second;
+
+            // specialization
+            if(learnedInfo->Effect[0]==SPELL_EFFECT_TRADE_SKILL && learnedInfo->Effect[1]==0 && profSpell)
             {
-                if(itr->second->state == PLAYERSPELL_REMOVED || itr->first==learned_0)
-                    continue;
+                // search other specialization for same prof
+                for (PlayerSpellMap::const_iterator itr = m_spells.begin(); itr != m_spells.end(); ++itr)
+                {
+                    if(itr->second->state == PLAYERSPELL_REMOVED || itr->first==learned_0)
+                        continue;
 
-                SpellEntry const *itrInfo = sSpellStore.LookupEntry(itr->first);
-                if(!itrInfo)
-                    return;
+                    SpellEntry const *itrInfo = sSpellStore.LookupEntry(itr->first);
+                    if(!itrInfo)
+                        return;
 
-                // compare only specializations
-                if(itrInfo->Effect[0]!=SPELL_EFFECT_TRADE_SKILL || itrInfo->Effect[1]!=0)
-                    continue;
+                    // compare only specializations
+                    if(itrInfo->Effect[0]!=SPELL_EFFECT_TRADE_SKILL || itrInfo->Effect[1]!=0)
+                        continue;
 
-                // compare same chain spells
-                if (spellmgr.GetSpellRequired(itr->first) == profSpell)
-                    return;
+                    // compare same chain spells
+                    if (spellmgr.IsSpellRequiringSpell(itr->first, profSpell))
+                        return;
+                }
             }
         }
     }
diff --git a/src/game/SpellMgr.cpp b/src/game/SpellMgr.cpp
index e1792787b02..716a7c3a886 100644
--- a/src/game/SpellMgr.cpp
+++ b/src/game/SpellMgr.cpp
@@ -3221,9 +3221,32 @@ void SpellMgr::LoadSpellRequired()
 
         uint32 spell_id =  fields[0].GetUInt32();
         uint32 spell_req = fields[1].GetUInt32();
+        // check if chain is made with valid first spell
+        SpellEntry const * spell = sSpellStore.LookupEntry(spell_id);
+        if (!spell)
+        {
+            sLog.outErrorDb("spell_id %u in `spell_required` table is not found in dbcs, skipped", spell_id);
+            continue;
+        }
+        SpellEntry const * req_spell = sSpellStore.LookupEntry(spell_req);
+        if (!req_spell)
+        {
+            sLog.outErrorDb("req_spell %u in `spell_required` table is not found in dbcs, skipped", spell_req);
+            continue;
+        }
+        if (GetFirstSpellInChain(spell_id) == GetFirstSpellInChain(spell_req))
+        {
+            sLog.outErrorDb("req_spell %u and spell_id %u in `spell_required` table are ranks of the same spell, entry not needed, skipped", spell_req, spell_id);
+            continue;
+        }
+        if (IsSpellRequiringSpell(spell_id, spell_req))
+        {
+            sLog.outErrorDb("duplicated entry of req_spell %u and spell_id %u in `spell_required`, skipped", spell_req, spell_id);
+            continue;
+        }
 
+        mSpellReq.insert (std::pair<uint32, uint32>(spell_id, spell_req));
         mSpellsReqSpell.insert (std::pair<uint32, uint32>(spell_req, spell_id));
-        mSpellReq[spell_id] = spell_req;
         ++rows;
     } while (result->NextRow());
 
diff --git a/src/game/SpellMgr.h b/src/game/SpellMgr.h
index 550f9d7bf37..6034a9d10ce 100644
--- a/src/game/SpellMgr.h
+++ b/src/game/SpellMgr.h
@@ -762,10 +762,13 @@ struct SpellChainNode
 
 typedef UNORDERED_MAP<uint32, SpellChainNode> SpellChainMap;
 
-//                 spell_id  req_spell
-typedef UNORDERED_MAP<uint32, uint32> SpellRequiredMap;
+//                   spell_id  req_spell
+typedef std::multimap<uint32, uint32> SpellRequiredMap;
+typedef std::pair<SpellRequiredMap::const_iterator,SpellRequiredMap::const_iterator> SpellRequiredMapBounds;
 
+//                   req_spell spell_id
 typedef std::multimap<uint32, uint32> SpellsRequiringSpellMap;
+typedef std::pair<SpellsRequiringSpellMap::const_iterator,SpellsRequiringSpellMap::const_iterator> SpellsRequiringSpellMapBounds;
 
 // Spell learning properties (accessed using SpellMgr functions)
 struct SpellLearnSkillNode
@@ -966,15 +969,6 @@ class SpellMgr
             return &itr->second;
         }
 
-        uint32 GetSpellRequired(uint32 spell_id) const
-        {
-            SpellRequiredMap::const_iterator itr = mSpellReq.find(spell_id);
-            if(itr == mSpellReq.end())
-                return NULL;
-
-            return itr->second;
-        }
-
         uint32 GetFirstSpellInChain(uint32 spell_id) const
         {
             if(SpellChainNode const* node = GetSpellChainNode(spell_id))
@@ -1009,7 +1003,25 @@ class SpellMgr
             return 0;
         }
 
-        SpellsRequiringSpellMap const& GetSpellsRequiringSpell() const { return mSpellsReqSpell; }
+        SpellRequiredMapBounds GetSpellsRequiredForSpellBounds(uint32 spell_id) const 
+        { 
+            return SpellRequiredMapBounds(mSpellReq.lower_bound(spell_id),mSpellReq.upper_bound(spell_id));
+        }
+
+        SpellsRequiringSpellMapBounds GetSpellsRequiringSpellBounds(uint32 spell_id) const 
+        { 
+            return SpellsRequiringSpellMapBounds(mSpellsReqSpell.lower_bound(spell_id),mSpellsReqSpell.upper_bound(spell_id));
+        }
+        bool IsSpellRequiringSpell(uint32 spellid, uint32 req_spellid) const 
+        {
+            SpellsRequiringSpellMapBounds spellsRequiringSpell = GetSpellsRequiringSpellBounds(req_spellid);
+            for (SpellsRequiringSpellMap::const_iterator itr = spellsRequiringSpell.first; itr != spellsRequiringSpell.second; ++itr)
+            {
+                if (itr->second == spellid)
+                    return true;
+            }
+            return false;
+        }
 
         uint8 GetSpellRank(uint32 spell_id) const
         {