refactor: replace magic slot offset 23 with NUM_EQUIP_SLOTS, simplify channel search

- Replace all 11 occurrences of magic number 23 in backpack slot
  calculations with Inventory::NUM_EQUIP_SLOTS across inventory_handler,
  spell_handler, and inventory.cpp
- Add why-comment to NUM_EQUIP_SLOTS explaining WoW slot layout
  (equipment 0-22, backpack starts at 23 in bag 0xFF)
- Add why-comment on 0x80000000 bit mask in item query response
  (high bit flags negative/missing entry response)
- Replace manual channel membership loops with std::find in
  chat_handler.cpp (YOU_JOINED and PLAYER_ALREADY_MEMBER cases)
- Add why-comment on PLAYER_ALREADY_MEMBER reconnect edge case
This commit is contained in:
Kelsi 2026-03-30 14:01:34 -07:00
parent a9ce22f315
commit f313eec24e
5 changed files with 21 additions and 23 deletions

View file

@ -70,6 +70,8 @@ class Inventory {
public: public:
static constexpr int BACKPACK_SLOTS = 16; static constexpr int BACKPACK_SLOTS = 16;
static constexpr int KEYRING_SLOTS = 32; static constexpr int KEYRING_SLOTS = 32;
// WoW slot layout: 0-22 are equipment (head, neck, ... tabard, mainhand, offhand, ranged, ammo).
// Backpack inventory starts at slot 23 in bag 0xFF, so packet slot = NUM_EQUIP_SLOTS + backpackIndex.
static constexpr int NUM_EQUIP_SLOTS = 23; static constexpr int NUM_EQUIP_SLOTS = 23;
static constexpr int NUM_BAG_SLOTS = 4; static constexpr int NUM_BAG_SLOTS = 4;
static constexpr int MAX_BAG_SIZE = 36; static constexpr int MAX_BAG_SIZE = 36;

View file

@ -435,11 +435,7 @@ void ChatHandler::handleChannelNotify(network::Packet& packet) {
switch (data.notifyType) { switch (data.notifyType) {
case ChannelNotifyType::YOU_JOINED: { case ChannelNotifyType::YOU_JOINED: {
bool found = false; if (std::find(joinedChannels_.begin(), joinedChannels_.end(), data.channelName) == joinedChannels_.end()) {
for (const auto& ch : joinedChannels_) {
if (ch == data.channelName) { found = true; break; }
}
if (!found) {
joinedChannels_.push_back(data.channelName); joinedChannels_.push_back(data.channelName);
} }
MessageChatData msg; MessageChatData msg;
@ -461,11 +457,9 @@ void ChatHandler::handleChannelNotify(network::Packet& packet) {
break; break;
} }
case ChannelNotifyType::PLAYER_ALREADY_MEMBER: { case ChannelNotifyType::PLAYER_ALREADY_MEMBER: {
bool found = false; // Server confirms we're in this channel but our local list doesn't have it yet —
for (const auto& ch : joinedChannels_) { // can happen after reconnect or if the join notification was missed.
if (ch == data.channelName) { found = true; break; } if (std::find(joinedChannels_.begin(), joinedChannels_.end(), data.channelName) == joinedChannels_.end()) {
}
if (!found) {
joinedChannels_.push_back(data.channelName); joinedChannels_.push_back(data.channelName);
LOG_INFO("Already in channel: ", data.channelName); LOG_INFO("Already in channel: ", data.channelName);
} }

View file

@ -239,7 +239,7 @@ std::vector<Inventory::SwapOp> Inventory::computeSortSwaps() const {
entries.reserve(BACKPACK_SLOTS + NUM_BAG_SLOTS * MAX_BAG_SIZE); entries.reserve(BACKPACK_SLOTS + NUM_BAG_SLOTS * MAX_BAG_SIZE);
for (int i = 0; i < BACKPACK_SLOTS; ++i) { for (int i = 0; i < BACKPACK_SLOTS; ++i) {
entries.push_back({0xFF, static_cast<uint8_t>(23 + i), entries.push_back({0xFF, static_cast<uint8_t>(NUM_EQUIP_SLOTS + i),
backpack[i].item.itemId, backpack[i].item.quality, backpack[i].item.itemId, backpack[i].item.quality,
backpack[i].item.stackCount}); backpack[i].item.stackCount});
} }

View file

@ -1047,7 +1047,7 @@ void InventoryHandler::autoEquipItemBySlot(int backpackIndex) {
if (slot.empty()) return; if (slot.empty()) return;
if (owner_.state == WorldState::IN_WORLD && owner_.socket) { if (owner_.state == WorldState::IN_WORLD && owner_.socket) {
auto packet = AutoEquipItemPacket::build(0xFF, static_cast<uint8_t>(23 + backpackIndex)); auto packet = AutoEquipItemPacket::build(0xFF, static_cast<uint8_t>(Inventory::NUM_EQUIP_SLOTS + backpackIndex));
owner_.socket->send(packet); owner_.socket->send(packet);
} }
} }
@ -1087,8 +1087,8 @@ void InventoryHandler::useItemBySlot(int backpackIndex) {
" spellId=", useSpellId, " spellCount=", info->spells.size()); " spellId=", useSpellId, " spellCount=", info->spells.size());
} }
auto packet = owner_.packetParsers_ auto packet = owner_.packetParsers_
? owner_.packetParsers_->buildUseItem(0xFF, static_cast<uint8_t>(23 + backpackIndex), itemGuid, useSpellId) ? owner_.packetParsers_->buildUseItem(0xFF, static_cast<uint8_t>(Inventory::NUM_EQUIP_SLOTS + backpackIndex), itemGuid, useSpellId)
: UseItemPacket::build(0xFF, static_cast<uint8_t>(23 + backpackIndex), itemGuid, useSpellId); : UseItemPacket::build(0xFF, static_cast<uint8_t>(Inventory::NUM_EQUIP_SLOTS + backpackIndex), itemGuid, useSpellId);
owner_.socket->send(packet); owner_.socket->send(packet);
} else if (itemGuid == 0) { } else if (itemGuid == 0) {
LOG_WARNING("useItemBySlot: itemGuid=0 for item='", slot.item.name, LOG_WARNING("useItemBySlot: itemGuid=0 for item='", slot.item.name,
@ -1145,8 +1145,8 @@ void InventoryHandler::openItemBySlot(int backpackIndex) {
if (backpackIndex < 0 || backpackIndex >= owner_.inventory.getBackpackSize()) return; if (backpackIndex < 0 || backpackIndex >= owner_.inventory.getBackpackSize()) return;
if (owner_.inventory.getBackpackSlot(backpackIndex).empty()) return; if (owner_.inventory.getBackpackSlot(backpackIndex).empty()) return;
if (owner_.state != WorldState::IN_WORLD || !owner_.socket) return; if (owner_.state != WorldState::IN_WORLD || !owner_.socket) return;
auto packet = OpenItemPacket::build(0xFF, static_cast<uint8_t>(23 + backpackIndex)); auto packet = OpenItemPacket::build(0xFF, static_cast<uint8_t>(Inventory::NUM_EQUIP_SLOTS + backpackIndex));
LOG_INFO("openItemBySlot: CMSG_OPEN_ITEM bag=0xFF slot=", (23 + backpackIndex)); LOG_INFO("openItemBySlot: CMSG_OPEN_ITEM bag=0xFF slot=", (Inventory::NUM_EQUIP_SLOTS + backpackIndex));
owner_.socket->send(packet); owner_.socket->send(packet);
} }
@ -1181,7 +1181,7 @@ void InventoryHandler::splitItem(uint8_t srcBag, uint8_t srcSlot, uint8_t count)
int freeBp = owner_.inventory.findFreeBackpackSlot(); int freeBp = owner_.inventory.findFreeBackpackSlot();
if (freeBp >= 0) { if (freeBp >= 0) {
uint8_t dstBag = 0xFF; uint8_t dstBag = 0xFF;
uint8_t dstSlot = static_cast<uint8_t>(23 + freeBp); uint8_t dstSlot = static_cast<uint8_t>(Inventory::NUM_EQUIP_SLOTS + freeBp);
LOG_INFO("splitItem: src(bag=", (int)srcBag, " slot=", (int)srcSlot, LOG_INFO("splitItem: src(bag=", (int)srcBag, " slot=", (int)srcSlot,
") count=", (int)count, " -> dst(bag=0xFF slot=", (int)dstSlot, ")"); ") count=", (int)count, " -> dst(bag=0xFF slot=", (int)dstSlot, ")");
auto packet = SplitItemPacket::build(srcBag, srcSlot, dstBag, dstSlot, count); auto packet = SplitItemPacket::build(srcBag, srcSlot, dstBag, dstSlot, count);
@ -1248,7 +1248,7 @@ void InventoryHandler::unequipToBackpack(EquipSlot equipSlot) {
uint8_t srcBag = 0xFF; uint8_t srcBag = 0xFF;
uint8_t srcSlot = static_cast<uint8_t>(equipSlot); uint8_t srcSlot = static_cast<uint8_t>(equipSlot);
uint8_t dstBag = 0xFF; uint8_t dstBag = 0xFF;
uint8_t dstSlot = static_cast<uint8_t>(23 + freeSlot); uint8_t dstSlot = static_cast<uint8_t>(Inventory::NUM_EQUIP_SLOTS + freeSlot);
LOG_INFO("UnequipToBackpack: equipSlot=", (int)srcSlot, LOG_INFO("UnequipToBackpack: equipSlot=", (int)srcSlot,
" -> backpackIndex=", freeSlot, " (dstSlot=", (int)dstSlot, ")"); " -> backpackIndex=", freeSlot, " (dstSlot=", (int)dstSlot, ")");
@ -1538,7 +1538,7 @@ bool InventoryHandler::attachItemFromBackpack(int backpackIndex) {
mailAttachments_[i].itemGuid = itemGuid; mailAttachments_[i].itemGuid = itemGuid;
mailAttachments_[i].item = slot.item; mailAttachments_[i].item = slot.item;
mailAttachments_[i].srcBag = 0xFF; mailAttachments_[i].srcBag = 0xFF;
mailAttachments_[i].srcSlot = static_cast<uint8_t>(23 + backpackIndex); mailAttachments_[i].srcSlot = static_cast<uint8_t>(Inventory::NUM_EQUIP_SLOTS + backpackIndex);
return true; return true;
} }
} }
@ -1730,7 +1730,7 @@ void InventoryHandler::withdrawItem(uint8_t srcBag, uint8_t srcSlot) {
owner_.addSystemChatMessage("Inventory is full."); owner_.addSystemChatMessage("Inventory is full.");
return; return;
} }
uint8_t dstSlot = static_cast<uint8_t>(23 + freeSlot); uint8_t dstSlot = static_cast<uint8_t>(Inventory::NUM_EQUIP_SLOTS + freeSlot);
auto packet = SwapItemPacket::build(0xFF, dstSlot, srcBag, srcSlot); auto packet = SwapItemPacket::build(0xFF, dstSlot, srcBag, srcSlot);
owner_.socket->send(packet); owner_.socket->send(packet);
} }
@ -2225,7 +2225,7 @@ void InventoryHandler::useEquipmentSet(uint32_t setId) {
for (int bp = 0; bp < 16 && !found; ++bp) { for (int bp = 0; bp < 16 && !found; ++bp) {
if (owner_.getBackpackItemGuid(bp) == itemGuid) { if (owner_.getBackpackItemGuid(bp) == itemGuid) {
srcBag = 0xFF; srcBag = 0xFF;
srcSlot = static_cast<uint8_t>(23 + bp); srcSlot = static_cast<uint8_t>(Inventory::NUM_EQUIP_SLOTS + bp);
found = true; found = true;
} }
} }
@ -2355,6 +2355,8 @@ void InventoryHandler::handleItemQueryResponse(network::Packet& packet) {
// Without this, the entry stays in pendingItemQueries_ forever, blocking retries. // Without this, the entry stays in pendingItemQueries_ forever, blocking retries.
if (packet.getSize() >= 4) { if (packet.getSize() >= 4) {
packet.setReadPos(0); packet.setReadPos(0);
// High bit indicates a negative (invalid/missing) item entry response;
// mask it off so we can still clear the pending query by entry ID.
uint32_t rawEntry = packet.readUInt32() & ~0x80000000u; uint32_t rawEntry = packet.readUInt32() & ~0x80000000u;
owner_.pendingItemQueries_.erase(rawEntry); owner_.pendingItemQueries_.erase(rawEntry);
} }

View file

@ -471,8 +471,8 @@ void SpellHandler::useItemBySlot(int backpackIndex) {
if (itemGuid != 0 && owner_.state == WorldState::IN_WORLD && owner_.socket) { if (itemGuid != 0 && owner_.state == WorldState::IN_WORLD && owner_.socket) {
uint32_t useSpellId = findOnUseSpellId(slot.item.itemId); uint32_t useSpellId = findOnUseSpellId(slot.item.itemId);
auto packet = owner_.packetParsers_ auto packet = owner_.packetParsers_
? owner_.packetParsers_->buildUseItem(0xFF, static_cast<uint8_t>(23 + backpackIndex), itemGuid, useSpellId) ? owner_.packetParsers_->buildUseItem(0xFF, static_cast<uint8_t>(Inventory::NUM_EQUIP_SLOTS + backpackIndex), itemGuid, useSpellId)
: UseItemPacket::build(0xFF, static_cast<uint8_t>(23 + backpackIndex), itemGuid, useSpellId); : UseItemPacket::build(0xFF, static_cast<uint8_t>(Inventory::NUM_EQUIP_SLOTS + backpackIndex), itemGuid, useSpellId);
owner_.socket->send(packet); owner_.socket->send(packet);
} else if (itemGuid == 0) { } else if (itemGuid == 0) {
owner_.addSystemChatMessage("Cannot use that item right now."); owner_.addSystemChatMessage("Cannot use that item right now.");