🐛 [Bug] *All* humans and undead are pickpocketable, even if they shouldn't be

[Simp-N]

tl;dr All humanoids and undead are pickpocketable even if they shouldn't be. I believe this is due to a simple coding error.

The Issue
Currently on Vmangos there are a lot of creatures that are pickpocketable, that shouldn't be. Virtually all humanoids and undead are pickpocketable, even yetis and owlbears. When I was 'virtually' I mean it might as well be all, but I havn't actually gone and tried to pickpocket every humanoid or undead in the game, but I've tried it against a lot.

How it should work, simplified, is you can assume a creature should be pickpocketable if its model appears to have pockets. In vmangos, creatures that are supposed to be pickpocketable have entries in the database table pickpocketing_loot_template which details the drops from pickpocket. But with this bug, even creatures with no entry in this table are pickpocketable. The bugged mobs only give silver when pickpocketed.

Before going to some examples, I'd like to point out that this bug report only addresses the fact that all humanoids and undead are pickpocketable unintendedly. It does not address which mobs should be pickpocketable or not to be vanilla-like. It is possible that fixing this issue might make creature X no longer pickpocketable, while perhaps that creature X was pickpocketable back in actual vanilla - but that issue is that it does not have an entry in the database.

Examples
To test pick pocket you can log in on GM account and do .learn 921 and .learn 1787 and then do .go cr "creature name" to teleport to the creature you want to check.

Here are a few examples of creatures that have no pickpocket-entry but are still pickpocketable currently: Ice Thistle Yeti, Ragged Owlbeast, Hungering Wraith, Wailing Death. The SQL query SELECT * FROM pickpocketing_loot_template WHERE entry IN (7458, 7450, 1802, 1804) will give no results, meaning they shouldn't be pickpocketable, but if you try it ingame they actually are.

If you want more examples of humanoids and undead with no pickpocket-entry you could try this SQL query: SELECT entry, name FROM creature_template WHERE type IN (6,7) AND NOT EXISTS (SELECT * FROM pickpocketing_loot_template AS pplt WHERE pplt.entry = creature_template.entry)

Fix
In Spell.cpp there's this code:

            case SPELL_EFFECT_PICKPOCKET:
            {
                Creature* target = ToCreature(m_targets.getUnitTarget());
                if (!target || target->GetOwnerGuid().IsPlayer())
                    return SPELL_FAILED_BAD_TARGETS;

                if (!target->GetCreatureInfo()->pickpocket_loot_id &&
                    !(target->GetCreatureTypeMask() & CREATURE_TYPEMASK_HUMANOID_OR_UNDEAD))
                    return SPELL_FAILED_TARGET_NO_POCKETS;

                break;
            }

I believe the second if statement is the culprit in this bug. What it does is check if the target lacks a pickpocket entry and if its not a humanoid or undead. If both it will give the "no pockets to pick" error message. But what that means is if only one of the conditions fail, such as the target having no pickpocket-entry but is a humanoid, the spell will be successful. Initially I thought it may have been a simple mistake and that changing the && to || would've solved it, but that will cause demons and oozes with pickpocket-entries to no longer be pickpocketable.

Ultimately I don't know what the creature type check is supposed to accomplish. I think simply checking for a pickpocket-entry will be sufficient, so I propose changing the if-statement to only if (!target->GetCreatureInfo()->pickpocket_loot_id)

Having changed this line the problem appears to be solved. Of the examples and a few of those of the SQL query mentioned above that I tested they no longer are pickpocketable. I also tried on some creatures with no pickpocket-entry of types other than humanoid and undead to make sure the change didn't make them pickpocketable unintentionally. Examples: Searing Infernal, Silithid Searcher, Mottled Boar, Black Dragon Whelp, Dust Devil, Deepstrider Giant, Sheep and XT:4. They were not pickpocketable which is correct. Also, to make sure the fix doesn't break pickpocketing on some creatures that should be, I tried it on a few such examples too: Winterfall Pathfinder, Skeletal Flayer, Felguard and Black Ooze and they were all pickpocketable.

Thanks for reading and have a nice day.