Script_EquipmentSetContainsLockedItems: Missing items are not locked

The two functions have different signatures in modern (Classic Era
1.15.x) WoW, verified against BlizzardInterfaceCode's generated
ItemDocumentation.lua:

  GetItemUniqueness(itemLocation) → (limitCategory, limitMax)
  GetItemUniquenessByID(itemID)   →
      (isUnique, limitCategoryName, limitCategoryCount, limitCategoryID)

We were returning a 3-tuple in the wrong order for both. For a
"Unique (10)" item the byID variant now correctly returns
(true, nil, 10, nil) instead of (10, nil, 0).

docs/API.md

@@ -4302,31 +4302,42 @@ Reads `ITEM_FIELD_STACK_COUNT` directly off the item's
 
 ### `C_Item.GetItemUniqueness(itemLocation)` / `C_Item.GetItemUniquenessByID(item)`
 
-Returns `(maxAllowed, category, equippedCount)` — the item's
-uniqueness cap, category name, and current equipped-of-category
-count.
+The two functions have **different signatures** in modern (Classic
+Era 1.15.x) WoW — we mirror them exactly:
+
+`C_Item.GetItemUniqueness(itemLocation)` returns
+`(limitCategory, limitMax)`:
 
 | Field | Vanilla source |
 |-------|----------------|
-| `maxAllowed` | `ItemStats_C.m_maxCount` — `0` = unlimited, `1` = unique ("Unique" tag in tooltip), higher = inventory cap |
-| `category` | Always `nil` — vanilla's client has no unique-equipped category system (added in TBC). |
-| `equippedCount` | Always `0` — without categories there's nothing meaningful to count against. |
+| `limitCategory` | Always `0` — vanilla has no `LimitCategory.dbc` (TBC addition). |
+| `limitMax` | `ItemStats_C.m_maxCount` — `0` = unlimited, `1` = "Unique", higher = inventory cap. |
 
-```lua
-local max = C_Item.GetItemUniqueness({equipmentSlotIndex = 13})
--- max = 1 for typical trinkets (Unique)
-local max = C_Item.GetItemUniquenessByID(2943)
--- max = 1 (Eye of Paleth — "Unique")
-```
+`C_Item.GetItemUniquenessByID(itemID)` returns
+`(isUnique, limitCategoryName, limitCategoryCount, limitCategoryID)`:
 
-The by-ID form skips item-location resolution and reads `ItemStats_C`
-directly. Fires a background cache fill on miss and returns nil;
+| Field | Vanilla source |
+|-------|----------------|
+| `isUnique` | `m_maxCount > 0` — true for any unique-tagged item. |
+| `limitCategoryName` | Always `nil` — no categories in vanilla. |
+| `limitCategoryCount` | `m_maxCount` when `isUnique`, else `nil`. |
+| `limitCategoryID` | Always `nil`. |
+
+Both functions return nothing (zero Lua return values, matching
+modern's `MayReturnNothing` annotation) if the item record isn't
+cached. The by-ID variant fires a background cache fill on miss;
 re-call after `GET_ITEM_INFO_RECEIVED`.
 
-Modern WoW returns the same 3-tuple but with the category half
-populated for unique-equipped categories (e.g. "Brewfest Mug",
-"Heart of Azeroth"). On this backport, only `maxAllowed` carries
-information.
+```lua
+local _, max = C_Item.GetItemUniqueness({equipmentSlotIndex = 13})
+-- max = 1 for typical trinkets (Unique)
+local isUnique, _, count = C_Item.GetItemUniquenessByID(81013)
+-- isUnique = true, count = 10  (Southern Sand Crawler Leg, "Unique (10)")
+```
+
+Modern WoW populates the category half for items like Brewfest Mug
+or Heart of Azeroth ("Unique-Equipped: Eye of Azshara"). Vanilla
+has no such items, so the category fields stay nil/0.
 
 ### `C_Item.GetItemLink(itemLocation)`
 

src/Offsets.h

@@ -1488,6 +1488,35 @@ enum Offsets {
     VAR_SKILL_LINE_COUNT = 0x00C0D930,
     OFF_SKILL_LINE_NAME = 0x0C,
 
+    // Player's known-skills table — separate from the DBC. Populated
+    // by the engine on login from server data; entries are pointers
+    // to per-skill records. Each record's `+0x00` is the
+    // `SkillLine.dbc` row (or `0` for header / category rows), and
+    // `+0x10` is the current skill rank (other rank/modifier fields
+    // follow). Source: `Script_GetSkillLineInfo` at `0x004D3610`,
+    // which walks this array indexed by the Lua arg.
+    VAR_PLAYER_SKILL_LIST = 0x00B7318C,
+    VAR_PLAYER_SKILL_COUNT = 0x00B731B8,
+    OFF_PLAYER_SKILL_LINE_ID = 0x00,
+    OFF_PLAYER_SKILL_RANK = 0x10,
+
+    // Per-itemClass proficiency bitmap maintained by the engine.
+    // 17 u32 entries (indexed by Item.dbc `m_class`, 0..16). Each
+    // entry is a bitmask where bit N is set if the player has
+    // proficiency in subclass N of that item class — i.e.
+    // `table[4] & (1 << 3)` = "can wear Mail" (armor=class 4,
+    // mail=subclass 3).
+    //
+    // Populated by SMSG_SET_PROFICIENCY's handler at
+    // `FUN_005E7B70`, which writes `table[itemClass] = mask`
+    // verbatim from the packet. Same source the engine itself uses
+    // for equip eligibility (the tooltip "Mail" red-text logic
+    // and the cursor-drop-on-paperdoll check both read this). Note:
+    // covers **weapons too** (class 2), making it strictly more
+    // general than walking the per-skill-line table for individual
+    // armor proficiencies.
+    VAR_PROFICIENCY_TABLE = 0x00C4D4A0,
+
     // Spell.dbc `BaseLevel` — the level a spell becomes available
     // (trainer offers it, quest reward grants it, etc.). Distinct
     // from `SpellLevel` at +0x74 which is the effective level used

src/addons/Info.cpp

@@ -115,13 +115,24 @@ int PushFieldOrNil(void *L, FieldByName_t accessor) {
 // `C_AddOns.GetAddOnName(indexOrName)` — the addon's directory name.
 // For numeric input, returns the engine's canonical casing. For
 // string input, returns the input verbatim once existence is
-// confirmed.
+// confirmed; nil for an unregistered name.
+//
+// String-input existence isn't free: `ResolveAddOnName` echoes the
+// string for the field accessors below (`GetAddOnTitle`, etc.) which
+// short-circuit cleanly on unknown names via the engine's own NULL-
+// return paths. `GetAddOnName` doesn't call any engine accessor, so
+// it'd otherwise echo any string. Probe the registry explicitly.
 int __fastcall Script_GetAddOnName(void *L) {
     const uint8_t *entry = ResolveAddOnName(L);
     if (entry == nullptr) {
         Game::Lua::PushNil(L);
         return 1;
     }
+    if (Game::Lua::Type(L, 1) == Game::Lua::TYPE_STRING &&
+        !AddOnExistsByName(reinterpret_cast<const char *>(entry))) {
+        Game::Lua::PushNil(L);
+        return 1;
+    }
     Game::Lua::PushString(L, reinterpret_cast<const char *>(entry));
     return 1;
 }

src/equipmentset/Api.cpp

@@ -331,6 +331,19 @@ int __fastcall Script_ClearIgnoredSlotsForSave(void *L) {
 // would race with). Reads the per-CGItem instance flag at
 // `OFF_ITEM_CLIENT_LOCK` bit 0 — same bit `C_Item.IsLocked` checks
 // and the engine's own pickup/unlock paths manipulate.
+//
+// Gates on `Locations::FindGUID` BEFORE consulting the lock flag:
+// for items the player has deleted/mailed/traded away, the engine's
+// CGItem cache often retains a stale entry that `ResolveByGUID`
+// still hands back, but the inventory walk no longer finds the GUID
+// anywhere. Reading the lock flag off the stale CGItem reflects
+// whatever bits were last set on it (frequently the "locked"
+// in-transaction state from when it was being deleted/swapped),
+// which would falsely report the set as "contains locked items"
+// and prevent `UseEquipmentSet` from running through addon
+// wrappers that gate on this check. The inventory presence check
+// matches modern semantics: a missing item is *missing*, not
+// *locked*.
 int __fastcall Script_EquipmentSetContainsLockedItems(void *L) {
     const uint32_t setID = ArgSetID(L, 1);
     const Set *s = Data::FindByID(setID);
@@ -341,6 +354,8 @@ int __fastcall Script_EquipmentSetContainsLockedItems(void *L) {
         const uint64_t g = s->items[i];
         if (g == GUID_EMPTY || g == GUID_IGNORED)
             continue;
+        if (Locations::FindGUID(g) == 0)
+            continue; // not in inventory — treat as missing, not locked
         const uint8_t *item = Locations::ResolveItemByGUID(g);
         if (item == nullptr)
             continue;

src/item/InventoryItemsForSlot.cpp

@@ -105,6 +105,47 @@ bool FitsSlot(int slot1Based, uint32_t invType) {
     return (kSlotMaskByInvType[invType] & (1u << (slot1Based - 1))) != 0;
 }
 
+// ── Proficiency gate ──────────────────────────────────────────────
+// `FitsSlot` only checks invType→slot shape compatibility; without
+// also gating on the player's proficiencies this would return (e.g.)
+// mail helms for a warlock since both are `invType=HEAD`. Modern
+// WoW's equipment-set picker filters by what the player can actually
+// equip; we mirror that.
+//
+// Source of truth: the engine's per-itemClass proficiency bitmap at
+// `[VAR_PROFICIENCY_TABLE]` (17 u32 entries indexed by `m_class`).
+// SMSG_SET_PROFICIENCY's handler at `0x005E7B70` writes the bitmask
+// for each class verbatim from the wire. Bit N of `table[class]` =
+// "player can equip subclass N of this class". Same data the engine
+// itself reads for the tooltip "Mail/Leather/Plate" red-text and the
+// cursor-drop-on-paperdoll-slot check — covers both armor and weapon
+// proficiencies in one lookup.
+//
+// 4.3.4's `Script_GetInventoryItemsForSlot` does the equivalent check
+// via `FUN_00553AD0(itemClass) & (1 << subclass)`, reading from
+// `DAT_00DD4D38` (its own copy of the same packet-fed table).
+
+bool PlayerHasProficiency(uint32_t itemClass, uint32_t subclass) {
+    if (itemClass > 16)
+        return true; // table only covers 0..16; be permissive past that
+    const uint32_t mask = *reinterpret_cast<const uint32_t *>(
+        static_cast<uintptr_t>(Offsets::VAR_PROFICIENCY_TABLE) + itemClass * 4);
+    if (mask == 0)
+        return true; // table not populated yet (pre-SMSG_SET_PROFICIENCY)
+                     // — fail open so empty-mask doesn't blank the list
+    return (mask & (1u << subclass)) != 0;
+}
+
+bool PlayerCanEquip(const uint8_t *record) {
+    if (record == nullptr)
+        return true; // missing record — be permissive, don't hide the item
+    const uint32_t classCode = *reinterpret_cast<const uint32_t *>(
+        record + Offsets::OFF_ITEMSTATS_CLASS);
+    const uint32_t subclass = *reinterpret_cast<const uint32_t *>(
+        record + Offsets::OFF_ITEMSTATS_SUBCLASS);
+    return PlayerHasProficiency(classCode, subclass);
+}
+
 const uint8_t *PeekItemRecord(uint32_t itemID) {
     auto fn = reinterpret_cast<GetItemRecord_t>(Offsets::FUN_DBCACHE_ITEMSTATS_GET_RECORD);
     auto *cache = reinterpret_cast<void *>(Offsets::VAR_ITEMDB_CACHE);
@@ -112,11 +153,14 @@ const uint8_t *PeekItemRecord(uint32_t itemID) {
     return fn(cache, itemID, &zeroGuid, nullptr, nullptr, 0);
 }
 
-uint32_t ReadInventoryType(const uint8_t *cgItem) {
+const uint8_t *ItemRecordForCGItem(const uint8_t *cgItem) {
     const int itemID = Item::ID::FromCGItem(cgItem);
     if (itemID <= 0)
-        return 0;
-    auto *record = PeekItemRecord(static_cast<uint32_t>(itemID));
+        return nullptr;
+    return PeekItemRecord(static_cast<uint32_t>(itemID));
+}
+
+uint32_t ReadInventoryType(const uint8_t *record) {
     if (record == nullptr)
         return 0;
     return *reinterpret_cast<const uint32_t *>(
@@ -132,7 +176,10 @@ void CollectIfFits(int slot, const uint8_t *cgItem, int location,
                    std::vector<Hit> &out) {
     if (cgItem == nullptr)
         return;
-    if (!FitsSlot(slot, ReadInventoryType(cgItem)))
+    const uint8_t *record = ItemRecordForCGItem(cgItem);
+    if (!FitsSlot(slot, ReadInventoryType(record)))
+        return;
+    if (!PlayerCanEquip(record))
         return;
     out.push_back({location, cgItem});
 }

src/item/Uniqueness.cpp

@@ -11,18 +11,31 @@
 // You should have received a copy of the GNU Lesser General Public License along with
 // ClassicAPI. If not, see <https://www.gnu.org/licenses/>.
 
-// `C_Item.GetItemUniqueness(itemLocation)` — uniqueness cap. Reads
-// `ItemStats_C.m_maxCount` from the item's cached record; that's the
-// only uniqueness signal vanilla's client carries. `1` = unique
-// ("Held In Off-hand" trinkets, special quest items, etc.); `0` =
-// unlimited; higher values cap the inventory count.
+// `C_Item.GetItemUniqueness(itemLocation)` and the `*ByID` variant
+// — uniqueness cap. Both read `ItemStats_C.m_maxCount` from the
+// item's cached record; that's the only uniqueness signal vanilla
+// carries. `1` = unique-equipped (one at a time); higher values
+// cap the inventory count; `0` = unlimited.
 //
-// Modern WoW returns a 3-tuple `(maxEquipped, category, equippedCount)`
-// keyed off the post-TBC category system (e.g. "Unique-Equipped:
-// Brewfest Mug"). Vanilla has no category system, so we surface
-// `m_maxCount` as the first return and leave the category nil and
-// equipped count `0` — matching modern's "field present but inapplicable"
-// convention.
+// Modern WoW (Classic Era 1.15.x) gives the two functions **different
+// signatures**, verified against `_classic_` BlizzardInterfaceCode's
+// `ItemDocumentation.lua`:
+//
+//   GetItemUniqueness(itemLocation) →
+//       limitCategory (number, non-nilable),
+//       limitMax      (number, non-nilable)
+//
+//   GetItemUniquenessByID(itemInfo) →
+//       isUnique           (bool, non-nilable),
+//       limitCategoryName  (string, nilable),
+//       limitCategoryCount (number, nilable),
+//       limitCategoryID    (number, nilable)
+//
+// Vanilla has no `LimitCategory.dbc` system (TBC+ addition for
+// "Unique-Equipped: Brewfest Mug"-style cross-item category caps),
+// so the category fields are always nil/0. `isUnique` collapses to
+// `m_maxCount > 0`; `limitMax` and `limitCategoryCount` both surface
+// the raw `m_maxCount`.
 
 #include "Game.h"
 #include "Offsets.h"
@@ -50,54 +63,58 @@ const uint8_t *PeekItemRecord(uint32_t itemID) {
     return fn(cache, itemID, &zeroGuid, nullptr, nullptr, 0);
 }
 
-} // namespace
-
-// Pushes the 3-tuple `(maxAllowed, nil, 0)` derived from the
-// ItemStats_C record, or nil + early-return on cache miss.
-int PushFromItemID(void *L, uint32_t itemID) {
+// Returns the item's `m_maxCount` or -1 if the record isn't cached.
+// `-1` is a "warm-and-bail" sentinel; caller pushes nothing on -1
+// so the Lua side sees the modern "MayReturnNothing" behaviour.
+int ReadMaxCount(uint32_t itemID) {
     const uint8_t *record = PeekItemRecord(itemID);
     if (record == nullptr) {
         Item::Data::WarmCache(itemID);
-        Game::Lua::PushNil(L);
-        return 1;
+        return -1;
     }
-    const int maxCount = *reinterpret_cast<const int *>(
+    return *reinterpret_cast<const int *>(
         record + Offsets::OFF_ITEMSTATS_MAX_COUNT);
-    Game::Lua::PushNumber(L, static_cast<double>(maxCount));
-    Game::Lua::PushNil(L);              // category (vanilla has none)
-    Game::Lua::PushNumber(L, 0.0);      // equippedCount (not tracked)
-    return 3;
 }
 
+} // namespace
+
+// `GetItemUniqueness(itemLocation)` shape: `(limitCategory, limitMax)`.
 static int __fastcall Script_GetItemUniqueness(void *L) {
-    if (!Item::Location::IsLocationArg(L, 1)) {
-        Game::Lua::PushNil(L);
-        return 1;
-    }
+    if (!Item::Location::IsLocationArg(L, 1))
+        return 0;
     auto *item = Item::Location::Resolve(L, 1);
-    if (item == nullptr) {
-        Game::Lua::PushNil(L);
-        return 1;
-    }
+    if (item == nullptr)
+        return 0;
     const int itemID = Item::ID::FromCGItem(item);
-    if (itemID <= 0) {
-        Game::Lua::PushNil(L);
-        return 1;
-    }
-    return PushFromItemID(L, static_cast<uint32_t>(itemID));
+    if (itemID <= 0)
+        return 0;
+    const int maxCount = ReadMaxCount(static_cast<uint32_t>(itemID));
+    if (maxCount < 0)
+        return 0; // cache miss — modern API documents "MayReturnNothing"
+    Game::Lua::PushNumber(L, 0.0); // limitCategory (vanilla has no LimitCategory.dbc)
+    Game::Lua::PushNumber(L, static_cast<double>(maxCount));
+    return 2;
 }
 
+// `GetItemUniquenessByID(itemID)` shape:
+// `(isUnique, limitCategoryName, limitCategoryCount, limitCategoryID)`.
 static int __fastcall Script_GetItemUniquenessByID(void *L) {
-    if (!Game::Lua::IsNumber(L, 1)) {
-        Game::Lua::PushNil(L);
-        return 1;
-    }
+    if (!Game::Lua::IsNumber(L, 1))
+        return 0;
     const int itemID = static_cast<int>(Game::Lua::ToNumber(L, 1));
-    if (itemID <= 0) {
-        Game::Lua::PushNil(L);
-        return 1;
-    }
-    return PushFromItemID(L, static_cast<uint32_t>(itemID));
+    if (itemID <= 0)
+        return 0;
+    const int maxCount = ReadMaxCount(static_cast<uint32_t>(itemID));
+    if (maxCount < 0)
+        return 0;
+    Game::Lua::PushBool(L, maxCount > 0);
+    Game::Lua::PushNil(L);          // limitCategoryName (no categories in vanilla)
+    if (maxCount > 0)
+        Game::Lua::PushNumber(L, static_cast<double>(maxCount));
+    else
+        Game::Lua::PushNil(L);      // limitCategoryCount — nil when not limited
+    Game::Lua::PushNil(L);          // limitCategoryID
+    return 4;
 }
 
 static void RegisterLuaFunctions() {