[7464] Move check of spell using for lock open to single function. Remove more hacks.

src/game/Spell.cpp

@@ -4161,114 +4161,31 @@ uint8 Spell::CanCast(bool strict)
                     return SPELL_FAILED_TRY_AGAIN;
 
                 // get the lock entry
-                LockEntry const *lockInfo = NULL;
+                uint32 lockId = 0;
                 if (GameObject* go=m_targets.getGOTarget())
-                    lockInfo = sLockStore.LookupEntry(go->GetLockId());
+                    lockId = go->GetLockId();
                 else if(Item* itm=m_targets.getItemTarget())
-                    lockInfo = sLockStore.LookupEntry(itm->GetProto()->LockID);
+                    lockId = itm->GetProto()->LockID;
+
+                SkillType skillId =SKILL_NONE;
+                int32 reqSkillValue = 0;
+                int32 skillValue = 0;
 
                 // check lock compatibility
-                if (lockInfo)
+                SpellCastResult res = CanOpenLock(i,lockId,skillId,reqSkillValue,skillValue);
-                {
+                if(res != SPELL_CAST_OK)
-                    // check for lock - key pair (checked by client also, just prevent cheating
+                    return res;
-                    bool ok_key = false;
-                    bool req_key = false;
-                    for(int it = 0; it < 8; ++it)
-                    {
-                        switch(lockInfo->Type[it])
-                        {
-                            case LOCK_KEY_NONE:
-                                break;
-                            case LOCK_KEY_ITEM:
-                            {
-                                req_key = true;
-                                if(lockInfo->Index[it])
-                                {
-                                    if(m_CastItem && m_CastItem->GetEntry()==lockInfo->Index[it])
-                                        ok_key =true;
-                                    break;
-                                }
-                            }
-                            case LOCK_KEY_SKILL:
-                            {
-                                req_key = true;
-                                if(uint32(m_spellInfo->EffectMiscValue[i])!=lockInfo->Index[it])
-                                    break;
-
-                                SkillType skill = SkillByLockType(LockType(lockInfo->Index[it]));
-                                if(skill==SKILL_NONE)
-                                    ok_key =true;
-                                else if(((Player*)m_caster)->HasSkill(skill))
-                                    ok_key =true;
-                            }
-                        }
-                        if(ok_key)
-                            break;
-                    }
-
-                    if(!ok_key && req_key)
-                        return SPELL_FAILED_BAD_TARGETS;
-                }
 
                 // chance for fail at orange mining/herb/LockPicking gathering attempt
-                if (!m_selfContainer || ((*m_selfContainer) != this))
+                // second check prevent fail at rechecks
-                    break;
+                if(skillId != SKILL_NONE && (!m_selfContainer || ((*m_selfContainer) != this)))
-
-                // get the skill value of the player
-                int32 SkillValue = 0;
-                bool canFailAtMax = true;
-                if (m_spellInfo->EffectMiscValue[i] == LOCKTYPE_HERBALISM)
                 {
-                    SkillValue = ((Player*)m_caster)->GetSkillValue(SKILL_HERBALISM);
+                    bool canFailAtMax = skillId != SKILL_HERBALISM && skillId != SKILL_MINING;
-                    canFailAtMax = false;
-                }
-                else if (m_spellInfo->EffectMiscValue[i] == LOCKTYPE_MINING)
-                {
-                    SkillValue = ((Player*)m_caster)->GetSkillValue(SKILL_MINING);
-                    canFailAtMax = false;
-                }
-                else if (m_spellInfo->EffectMiscValue[i] == LOCKTYPE_PICKLOCK)
-                    SkillValue = ((Player*)m_caster)->GetSkillValue(SKILL_LOCKPICKING);
-
-                // castitem check: rogue using skeleton keys. the skill values should not be added in this case.
-                if(m_CastItem)
-                    SkillValue = 0;
-
-                // add the damage modifier from the spell casted (cheat lock / skeleton key etc.) (use m_currentBasePoints, CalculateDamage returns wrong value)
-                SkillValue += m_currentBasePoints[i]+1;
-
-                // get the required lock value
-                int32 ReqValue=0;
-                if (lockInfo)
-                {
-                    // check for lock - key pair
-                    bool ok = false;
-                    for(int it = 0; it < 8; ++it)
-                    {
-                        if(lockInfo->Type[it]==LOCK_KEY_ITEM && lockInfo->Index[it] && m_CastItem && m_CastItem->GetEntry()==lockInfo->Index[it])
-                        {
-                            // if so, we're good to go
-                            ok = true;
-                            break;
-                        }
-                    }
-                    if(ok)
-                        break;
-
-                    if (m_spellInfo->EffectMiscValue[i] == LOCKTYPE_PICKLOCK)
-                        ReqValue = lockInfo->Skill[1];
-                    else
-                        ReqValue = lockInfo->Skill[0];
-                }
-
-                // skill doesn't meet the required value
-                if (ReqValue > SkillValue)
-                    return SPELL_FAILED_LOW_CASTLEVEL;
 
                     // chance for failure in orange gather / lockpick (gathering skill can't fail at maxskill)
-                if((canFailAtMax || SkillValue < sWorld.GetConfigMaxSkillValue()) && ReqValue > irand(SkillValue-25, SkillValue+37))
+                    if((canFailAtMax || skillValue < sWorld.GetConfigMaxSkillValue()) && reqSkillValue > irand(skillValue-25, skillValue+37))
                         return SPELL_FAILED_TRY_AGAIN;
-
+                }
                 break;
             }
             case SPELL_EFFECT_SUMMON_DEAD_PET:
@@ -5631,3 +5548,65 @@ bool SpellEvent::IsDeletable() const
 {
     return m_Spell->IsDeletable();
 }
+
+SpellCastResult Spell::CanOpenLock(uint32 effIndex, uint32 lockId, SkillType& skillId, int32& reqSkillValue, int32& skillValue)
+{
+    if(!lockId)                                             // possible case for GO and maybe for items.
+        return SPELL_CAST_OK;
+
+    // Get LockInfo
+    LockEntry const *lockInfo = sLockStore.LookupEntry(lockId);
+
+    if (!lockInfo)
+        return SPELL_FAILED_BAD_TARGETS;
+
+    bool reqKey = false;                                    // some locks not have reqs
+
+    for(int j = 0; j < 8; ++j)
+    {
+        switch(lockInfo->Type[j])
+        {
+            // check key item (many fit cases can be)
+            case LOCK_KEY_ITEM:
+                if(lockInfo->Index[j] && m_CastItem && m_CastItem->GetEntry()==lockInfo->Index[j])
+                    return SPELL_CAST_OK;
+                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;
+
+                skillId = SkillByLockType(LockType(lockInfo->Index[j]));
+
+                if ( skillId != SKILL_NONE )
+                {
+                    // skill bonus provided by casting spell (mostly item spells)
+                    // add the damage modifier from the spell casted (cheat lock / skeleton key etc.) (use m_currentBasePoints, CalculateDamage returns wrong value)
+                    uint32 spellSkillBonus = uint32(m_currentBasePoints[effIndex]+1);
+                    reqSkillValue = lockInfo->Skill[j];
+
+                    // castitem check: rogue using skeleton keys. the skill values should not be added in this case.
+                    skillValue = m_CastItem || m_caster->GetTypeId()!= TYPEID_PLAYER ?
+                        0 : ((Player*)m_caster)->GetSkillValue(skillId);
+
+                    skillValue += spellSkillBonus;
+
+                    if (skillValue < reqSkillValue)
+                        return SPELL_FAILED_LOW_CASTLEVEL;
+                }
+
+                return SPELL_CAST_OK;
+            }
+        }
+    }
+
+    if(reqKey)
+        return SPELL_FAILED_BAD_TARGETS;
+
+    return SPELL_CAST_OK;
+}

src/game/Spell.h

@@ -548,6 +548,7 @@ class Spell
         void DoAllEffectOnTarget(GOTargetInfo *target);
         void DoAllEffectOnTarget(ItemTargetInfo *target);
         bool IsAliveUnitPresentInTargetList();
+        SpellCastResult CanOpenLock(uint32 effIndex, uint32 lockid, SkillType& skillid, int32& reqSkillValue, int32& skillValue);
         // -------------------------------------------
 
         //List For Triggered Spells

src/game/SpellEffects.cpp

@@ -2999,92 +2999,39 @@ void Spell::EffectOpenLock(uint32 effIndex)
         return;
     }
 
-    if(!lockId)                                             // possible case for GO and maybe for items.
+    SkillType skillId = SKILL_NONE;
+    int32 reqSkillValue = 0;
+    int32 skillValue;
+
+    SpellCastResult res = CanOpenLock(effIndex,lockId,skillId,reqSkillValue,skillValue);
+    if(res != SPELL_CAST_OK)
     {
-        SendLoot(guid, LOOT_CORPSE);
+        SendCastResult(res);
         return;
     }
 
-    // Get LockInfo
+    SendLoot(guid, LOOT_SKINNING);
-    LockEntry const *lockInfo = sLockStore.LookupEntry(lockId);
 
-    if (!lockInfo)
+    // not allow use skill grou at item base open
+    if(!m_CastItem && skillId != SKILL_NONE)
     {
-        sLog.outError( "Spell::EffectOpenLock: %s [guid = %u] has an unknown lockId: %u!",
-            (gameObjTarget ? "gameobject" : "item"), GUID_LOPART(guid), lockId);
-        SendCastResult(SPELL_FAILED_BAD_TARGETS);
-        return;
-    }
-
-    bool reqKey = false;                                    // some locks not have reqs
-
-    for(int j = 0; j < 8; ++j)
-    {
-        switch(lockInfo->Type[j])
-        {
-            // check key item (many fit cases can be)
-            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(uint32 pureSkillValue = player->GetPureSkillValue(skillId))
         {
             if(gameObjTarget)
             {
                 // Allow one skill-up until respawned
                 if ( !gameObjTarget->IsInSkillupList( player->GetGUIDLow() ) &&
-                                player->UpdateGatherSkill(skillId, SkillValue, reqSkillValue) )
+                    player->UpdateGatherSkill(skillId, pureSkillValue, reqSkillValue) )
                     gameObjTarget->AddToSkillupList( player->GetGUIDLow() );
             }
             else if(itemTarget)
             {
                 // Do one skill-up
-                            player->UpdateGatherSkill(skillId, SkillValue, reqSkillValue);
+                player->UpdateGatherSkill(skillId, pureSkillValue, reqSkillValue);
             }
         }
     }
-
-                SendLoot(guid, LOOT_SKINNING);
-                return;
-            }
-        }
-    }
-
-    if(reqKey)
-    {
-        SendCastResult(SPELL_FAILED_BAD_TARGETS);
-        return;
-    }
-
-    SendLoot(guid, LOOT_SKINNING);
 }
 
 void Spell::EffectSummonChangeItem(uint32 i)

src/shared/Database/DBCStructure.h

@@ -974,13 +974,15 @@ struct ItemSetEntry
     uint32    required_skill_value;                         // 52       m_requiredSkillRank
 };
 
+#define MAX_LOCK_CASE 8
+
 struct LockEntry
 {
     uint32      ID;                                         // 0        m_ID
-    uint32      Type[8];                                    // 1-8      m_Type
+    uint32      Type[MAX_LOCK_CASE];                        // 1-8      m_Type
-    uint32      Index[8];                                   // 9-16     m_Index
+    uint32      Index[MAX_LOCK_CASE];                       // 9-16     m_Index
-    uint32      Skill[8];                                   // 17-24    m_Skill
+    uint32      Skill[MAX_LOCK_CASE];                       // 17-24    m_Skill
-    //uint32      Action[8];                                // 25-32    m_Action
+    //uint32      Action[MAX_LOCK_CASE];                    // 25-32    m_Action
 };
 
 struct MailTemplateEntry

src/shared/revision_nr.h

@@ -1,4 +1,4 @@
 #ifndef __REVISION_NR_H__
 #define __REVISION_NR_H__
- #define REVISION_NR "7463"
+ #define REVISION_NR "7464"
 #endif // __REVISION_NR_H__