Replace hack code in Spell::EffectOpenLock by generic way for check lock key items/skills

src/game/SharedDefines.h

@@ -1696,6 +1696,8 @@ inline uint8 ClassByQuestSort(int32 QuestSort)
 
 enum SkillType
 {
+    SKILL_NONE                     = 0,
+
     SKILL_FROST                    = 6,
     SKILL_FIRE                     = 8,
     SKILL_ARMS                     = 26,
@@ -1850,6 +1852,20 @@ enum SkillType
 
 #define MAX_SKILL_TYPE               789
 
+inline SkillType SkillByLockType(LockType locktype)
+{
+    switch(locktype)
+    {
+        case LOCKTYPE_PICKLOCK:    return SKILL_LOCKPICKING;
+        case LOCKTYPE_HERBALISM:   return SKILL_HERBALISM;
+        case LOCKTYPE_MINING:      return SKILL_MINING;
+        case LOCKTYPE_FISHING:     return SKILL_FISHING;
+        case LOCKTYPE_INSCRIPTION: return SKILL_INSCRIPTION;
+        default: break;
+    }
+    return SKILL_NONE;
+}
+
 inline uint32 SkillByQuestSort(int32 QuestSort)
 {
     switch(QuestSort)

src/game/Spell.cpp

@@ -4172,6 +4172,7 @@ uint8 Spell::CanCast(bool strict)
                 {
                     // check for lock - key pair (checked by client also, just prevent cheating
                     bool ok_key = false;
+                    bool req_key = false;
                     for(int it = 0; it < 8; ++it)
                     {
                         switch(lockInfo->Type[it])
@@ -4180,6 +4181,7 @@ uint8 Spell::CanCast(bool strict)
                                 break;
                             case LOCK_KEY_ITEM:
                             {
+                                req_key = true;
                                 if(lockInfo->Index[it])
                                 {
                                     if(m_CastItem && m_CastItem->GetEntry()==lockInfo->Index[it])
@@ -4189,30 +4191,22 @@ uint8 Spell::CanCast(bool strict)
                             }
                             case LOCK_KEY_SKILL:
                             {
+                                req_key = true;
                                 if(uint32(m_spellInfo->EffectMiscValue[i])!=lockInfo->Index[it])
                                     break;
 
-                                switch(lockInfo->Index[it])
+                                SkillType skill = SkillByLockType(LockType(lockInfo->Index[it]));
-                                {
+                                if(skill==SKILL_NONE)
-                                    case LOCKTYPE_HERBALISM:
+                                    ok_key =true;
-                                        if(((Player*)m_caster)->HasSkill(SKILL_HERBALISM))
+                                else if(((Player*)m_caster)->HasSkill(skill))
-                                            ok_key =true;
+                                    ok_key =true;
-                                        break;
-                                    case LOCKTYPE_MINING:
-                                        if(((Player*)m_caster)->HasSkill(SKILL_MINING))
-                                            ok_key =true;
-                                        break;
-                                    default:
-                                        ok_key =true;
-                                        break;
-                                }
                             }
                         }
                         if(ok_key)
                             break;
                     }
 
-                    if(!ok_key)
+                    if(!ok_key && req_key)
                         return SPELL_FAILED_BAD_TARGETS;
                 }
 

src/game/SpellEffects.cpp

@@ -2943,7 +2943,7 @@ void Spell::SendLoot(uint64 guid, LootType loottype)
     player->SendLoot(guid, loottype);
 }
 
-void Spell::EffectOpenLock(uint32 /*i*/)
+void Spell::EffectOpenLock(uint32 effIndex)
 {
     if(!m_caster || m_caster->GetTypeId() != TYPEID_PLAYER)
     {
@@ -2953,7 +2953,6 @@ void Spell::EffectOpenLock(uint32 /*i*/)
 
     Player* player = (Player*)m_caster;
 
-    LootType loottype = LOOT_CORPSE;
     uint32 lockId = 0;
     uint64 guid = 0;
 
@@ -3002,7 +3001,7 @@ void Spell::EffectOpenLock(uint32 /*i*/)
 
     if(!lockId)                                             // possible case for GO and maybe for items.
     {
-        SendLoot(guid, loottype);
+        SendLoot(guid, LOOT_CORPSE);
         return;
     }
 
@@ -3017,74 +3016,75 @@ void Spell::EffectOpenLock(uint32 /*i*/)
         return;
     }
 
-    // check key
+    bool reqKey = false;                                    // some locks not have reqs
-    for(int i = 0; i < 8; ++i)
+
+    for(int j = 0; j < 8; ++j)
     {
-        // Type==1 This means lockInfo->Index[i] is an item
+        switch(lockInfo->Type[j])
-        if(lockInfo->Type[i]==LOCK_KEY_ITEM && lockInfo->Index[i] && m_CastItem && m_CastItem->GetEntry()==lockInfo->Index[i])
         {
-            SendLoot(guid, loottype);
+            // check key item (many fit cases can be)
-            return;
+            case LOCK_KEY_ITEM:
+                if(lockInfo->Index[j] && m_CastItem && m_CastItem->GetEntry()==lockInfo->Index[j])
+                {
+                    SendLoot(guid, LOOT_CORPSE);
+                    return;
+                }
+                reqKey = true;
+                break;
+            // check key skill (only single first fit case can be)
+            case LOCK_KEY_SKILL:
+            {
+                reqKey = true;
+
+                // wrong locktype, skip
+                if(uint32(m_spellInfo->EffectMiscValue[effIndex]) != lockInfo->Index[j])
+                    continue;
+
+                SkillType skillId = SkillByLockType(LockType(lockInfo->Index[j]));
+
+                if ( skillId != SKILL_NONE )
+                {
+                    // skill bonus provided by casting spell (mostly item spells)
+                    uint32 spellSkillBonus = uint32(m_currentBasePoints[0]+1);
+                    uint32 reqSkillValue = lockInfo->Skill[j];
+
+                    if ( player->GetSkillValue(skillId) + spellSkillBonus < reqSkillValue )
+                    {
+                        SendCastResult(SPELL_FAILED_LOW_CASTLEVEL);
+                        return;
+                    }
+
+                    // update skill if really known
+                    if(uint32 SkillValue = player->GetPureSkillValue(skillId))
+                    {
+                        if(gameObjTarget)
+                        {
+                            // Allow one skill-up until respawned
+                            if ( !gameObjTarget->IsInSkillupList( player->GetGUIDLow() ) &&
+                                player->UpdateGatherSkill(skillId, SkillValue, reqSkillValue) )
+                                gameObjTarget->AddToSkillupList( player->GetGUIDLow() );
+                        }
+                        else if(itemTarget)
+                        {
+                            // Do one skill-up
+                            player->UpdateGatherSkill(skillId, SkillValue, reqSkillValue);
+                        }
+                    }
+                }
+
+                SendLoot(guid, LOOT_SKINNING);
+                return;
+            }
         }
     }
 
-    uint32 SkillId = 0;
+    if(reqKey)
-    // Check and skill-up skill
-    if( m_spellInfo->Effect[1] == SPELL_EFFECT_SKILL )
-        SkillId = m_spellInfo->EffectMiscValue[1];
-                                                            // pickpocketing spells
-    else if( m_spellInfo->EffectMiscValue[0] == LOCKTYPE_PICKLOCK )
-        SkillId = SKILL_LOCKPICKING;
-
-    // skill bonus provided by casting spell (mostly item spells)
-    uint32 spellSkillBonus = uint32(m_currentBasePoints[0]+1);
-
-    uint32 reqSkillValue = lockInfo->Skill[0];
-
-    if(lockInfo->Skill[1])                                  // required pick lock skill applying
-    {
-        if(SkillId != SKILL_LOCKPICKING)                    // wrong skill (cheating?)
-        {
-            SendCastResult(SPELL_FAILED_FIZZLE);
-            return;
-        }
-
-        reqSkillValue = lockInfo->Skill[1];
-    }
-    else if(SkillId == SKILL_LOCKPICKING)                   // apply picklock skill to wrong target
     {
         SendCastResult(SPELL_FAILED_BAD_TARGETS);
         return;
     }
 
-    if ( SkillId )
+    SendLoot(guid, LOOT_SKINNING);
-    {
-        loottype = LOOT_SKINNING;
-        if ( player->GetSkillValue(SkillId) + spellSkillBonus < reqSkillValue )
-        {
-            SendCastResult(SPELL_FAILED_LOW_CASTLEVEL);
-            return;
-        }
-
-        // update skill if really known
-        if(uint32 SkillValue = player->GetPureSkillValue(SkillId))
-        {
-            if(gameObjTarget)
-            {
-                // Allow one skill-up until respawned
-                if ( !gameObjTarget->IsInSkillupList( player->GetGUIDLow() ) &&
-                    player->UpdateGatherSkill(SkillId, SkillValue, reqSkillValue) )
-                    gameObjTarget->AddToSkillupList( player->GetGUIDLow() );
-            }
-            else if(itemTarget)
-            {
-                // Do one skill-up
-                player->UpdateGatherSkill(SkillId, SkillValue, reqSkillValue);
-            }
-        }
-    }
-
-    SendLoot(guid, loottype);
 }
 
 void Spell::EffectSummonChangeItem(uint32 i)