From 28e5cd9281f9d4989a6ca504ad86268a78ce157f Mon Sep 17 00:00:00 2001 From: Kelsi Date: Mon, 30 Mar 2026 14:20:39 -0700 Subject: [PATCH] refactor: replace magic bag slot offset 19 with FIRST_BAG_EQUIP_SLOT - Add Inventory::FIRST_BAG_EQUIP_SLOT = 19 constant with why-comment explaining WoW equip slot layout (bags occupy slots 19-22) - Replace all 19 occurrences of magic number 19 in bag slot calculations across inventory_handler, spell_handler, inventory, and game_handler - Add UNIT_FIELD_FLAGS / UNIT_FLAG_PVP comment in combat_handler - Add why-comment on network packet budget constants (prevent server data bursts from starving the render loop) --- include/game/inventory.hpp | 3 +++ src/game/combat_handler.cpp | 1 + src/game/game_handler.cpp | 2 +- src/game/inventory.cpp | 2 +- src/game/inventory_handler.cpp | 28 ++++++++++++++-------------- src/game/spell_handler.cpp | 4 ++-- src/network/world_socket.cpp | 3 +++ 7 files changed, 25 insertions(+), 18 deletions(-) diff --git a/include/game/inventory.hpp b/include/game/inventory.hpp index f510d6ab..fd64aa24 100644 --- a/include/game/inventory.hpp +++ b/include/game/inventory.hpp @@ -73,6 +73,9 @@ public: // 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; + // Bag containers occupy equipment slots 19-22 (bag1, bag2, bag3, bag4). + // Packet bag byte = FIRST_BAG_EQUIP_SLOT + bagIndex. + static constexpr int FIRST_BAG_EQUIP_SLOT = 19; static constexpr int NUM_BAG_SLOTS = 4; static constexpr int MAX_BAG_SIZE = 36; static constexpr int BANK_SLOTS = 28; diff --git a/src/game/combat_handler.cpp b/src/game/combat_handler.cpp index 274cfb04..32f120b7 100644 --- a/src/game/combat_handler.cpp +++ b/src/game/combat_handler.cpp @@ -1362,6 +1362,7 @@ void CombatHandler::togglePvp() { auto entity = owner_.getEntityManager().getEntity(owner_.playerGuid); bool currentlyPvp = false; if (entity) { + // UNIT_FIELD_FLAGS (index 59), bit 0x1000 = UNIT_FLAG_PVP currentlyPvp = (entity->getField(59) & 0x00001000) != 0; } if (currentlyPvp) { diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 8cbe34be..6a8f37a4 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -6371,7 +6371,7 @@ void GameHandler::offerQuestFromItem(uint64_t itemGuid, uint32_t questId) { uint64_t GameHandler::getBagItemGuid(int bagIndex, int slotIndex) const { if (bagIndex < 0 || bagIndex >= inventory.NUM_BAG_SLOTS) return 0; if (slotIndex < 0) return 0; - uint64_t bagGuid = equipSlotGuids_[19 + bagIndex]; + uint64_t bagGuid = equipSlotGuids_[Inventory::FIRST_BAG_EQUIP_SLOT + bagIndex]; if (bagGuid == 0) return 0; auto it = containerContents_.find(bagGuid); if (it == containerContents_.end()) return 0; diff --git a/src/game/inventory.cpp b/src/game/inventory.cpp index d2f0f488..b0fe6c48 100644 --- a/src/game/inventory.cpp +++ b/src/game/inventory.cpp @@ -245,7 +245,7 @@ std::vector Inventory::computeSortSwaps() const { } for (int b = 0; b < NUM_BAG_SLOTS; ++b) { for (int s = 0; s < bags[b].size; ++s) { - entries.push_back({static_cast(19 + b), static_cast(s), + entries.push_back({static_cast(FIRST_BAG_EQUIP_SLOT + b), static_cast(s), bags[b].slots[s].item.itemId, bags[b].slots[s].item.quality, bags[b].slots[s].item.stackCount}); } diff --git a/src/game/inventory_handler.cpp b/src/game/inventory_handler.cpp index 5e05167b..385a5850 100644 --- a/src/game/inventory_handler.cpp +++ b/src/game/inventory_handler.cpp @@ -981,7 +981,7 @@ void InventoryHandler::sellItemInBag(int bagIndex, int slotIndex) { } uint64_t itemGuid = 0; - uint64_t bagGuid = owner_.equipSlotGuids_[19 + bagIndex]; + uint64_t bagGuid = owner_.equipSlotGuids_[Inventory::FIRST_BAG_EQUIP_SLOT + bagIndex]; if (bagGuid != 0) { auto it = owner_.containerContents_.find(bagGuid); if (it != owner_.containerContents_.end() && slotIndex < static_cast(it->second.numSlots)) { @@ -1058,7 +1058,7 @@ void InventoryHandler::autoEquipItemInBag(int bagIndex, int slotIndex) { if (owner_.state == WorldState::IN_WORLD && owner_.socket) { auto packet = AutoEquipItemPacket::build( - static_cast(19 + bagIndex), static_cast(slotIndex)); + static_cast(Inventory::FIRST_BAG_EQUIP_SLOT + bagIndex), static_cast(slotIndex)); owner_.socket->send(packet); } } @@ -1104,7 +1104,7 @@ void InventoryHandler::useItemInBag(int bagIndex, int slotIndex) { if (slot.empty()) return; uint64_t itemGuid = 0; - uint64_t bagGuid = owner_.equipSlotGuids_[19 + bagIndex]; + uint64_t bagGuid = owner_.equipSlotGuids_[Inventory::FIRST_BAG_EQUIP_SLOT + bagIndex]; if (bagGuid != 0) { auto it = owner_.containerContents_.find(bagGuid); if (it != owner_.containerContents_.end() && slotIndex < static_cast(it->second.numSlots)) { @@ -1128,7 +1128,7 @@ void InventoryHandler::useItemInBag(int bagIndex, int slotIndex) { } } } - uint8_t wowBag = static_cast(19 + bagIndex); + uint8_t wowBag = static_cast(Inventory::FIRST_BAG_EQUIP_SLOT + bagIndex); auto packet = owner_.packetParsers_ ? owner_.packetParsers_->buildUseItem(wowBag, static_cast(slotIndex), itemGuid, useSpellId) : UseItemPacket::build(wowBag, static_cast(slotIndex), itemGuid, useSpellId); @@ -1155,7 +1155,7 @@ void InventoryHandler::openItemInBag(int bagIndex, int slotIndex) { if (slotIndex < 0 || slotIndex >= owner_.inventory.getBagSize(bagIndex)) return; if (owner_.inventory.getBagSlot(bagIndex, slotIndex).empty()) return; if (owner_.state != WorldState::IN_WORLD || !owner_.socket) return; - uint8_t wowBag = static_cast(19 + bagIndex); + uint8_t wowBag = static_cast(Inventory::FIRST_BAG_EQUIP_SLOT + bagIndex); auto packet = OpenItemPacket::build(wowBag, static_cast(slotIndex)); LOG_INFO("openItemInBag: CMSG_OPEN_ITEM bag=", (int)wowBag, " slot=", slotIndex); owner_.socket->send(packet); @@ -1192,7 +1192,7 @@ void InventoryHandler::splitItem(uint8_t srcBag, uint8_t srcSlot, uint8_t count) int bagSize = owner_.inventory.getBagSize(b); for (int s = 0; s < bagSize; s++) { if (owner_.inventory.getBagSlot(b, s).empty()) { - uint8_t dstBag = static_cast(19 + b); + uint8_t dstBag = static_cast(Inventory::FIRST_BAG_EQUIP_SLOT + b); uint8_t dstSlot = static_cast(s); LOG_INFO("splitItem: src(bag=", (int)srcBag, " slot=", (int)srcSlot, ") count=", (int)count, " -> dst(bag=", (int)dstBag, @@ -1227,8 +1227,8 @@ void InventoryHandler::swapBagSlots(int srcBagIndex, int dstBagIndex) { owner_.inventory.swapBagContents(srcBagIndex, dstBagIndex); if (owner_.socket && owner_.socket->isConnected()) { - uint8_t srcSlot = static_cast(19 + srcBagIndex); - uint8_t dstSlot = static_cast(19 + dstBagIndex); + uint8_t srcSlot = static_cast(Inventory::FIRST_BAG_EQUIP_SLOT + srcBagIndex); + uint8_t dstSlot = static_cast(Inventory::FIRST_BAG_EQUIP_SLOT + dstBagIndex); LOG_INFO("swapBagSlots: bag ", srcBagIndex, " (slot ", (int)srcSlot, ") <-> bag ", dstBagIndex, " (slot ", (int)dstSlot, ")"); auto packet = SwapItemPacket::build(255, dstSlot, 255, srcSlot); @@ -1550,7 +1550,7 @@ bool InventoryHandler::attachItemFromBag(int bagIndex, int slotIndex) { if (slotIndex < 0 || slotIndex >= owner_.inventory.getBagSize(bagIndex)) return false; const auto& slot = owner_.inventory.getBagSlot(bagIndex, slotIndex); if (slot.empty()) return false; - uint64_t bagGuid = owner_.equipSlotGuids_[19 + bagIndex]; + uint64_t bagGuid = owner_.equipSlotGuids_[Inventory::FIRST_BAG_EQUIP_SLOT + bagIndex]; if (bagGuid == 0) return false; auto it = owner_.containerContents_.find(bagGuid); if (it == owner_.containerContents_.end()) return false; @@ -1561,7 +1561,7 @@ bool InventoryHandler::attachItemFromBag(int bagIndex, int slotIndex) { if (!mailAttachments_[i].occupied()) { mailAttachments_[i].itemGuid = itemGuid; mailAttachments_[i].item = slot.item; - mailAttachments_[i].srcBag = static_cast(19 + bagIndex); + mailAttachments_[i].srcBag = static_cast(Inventory::FIRST_BAG_EQUIP_SLOT + bagIndex); mailAttachments_[i].srcSlot = static_cast(slotIndex); return true; } @@ -2233,7 +2233,7 @@ void InventoryHandler::useEquipmentSet(uint32_t setId) { int bagSize = owner_.inventory.getBagSize(bag); for (int s = 0; s < bagSize && !found; ++s) { if (owner_.getBagItemGuid(bag, s) == itemGuid) { - srcBag = static_cast(19 + bag); + srcBag = static_cast(Inventory::FIRST_BAG_EQUIP_SLOT + bag); srcSlot = static_cast(s); found = true; } @@ -2712,7 +2712,7 @@ void InventoryHandler::rebuildOnlineInventory() { // Bag contents (BAG1-BAG4 are equip slots 19-22) for (int bagIdx = 0; bagIdx < 4; bagIdx++) { - uint64_t bagGuid = owner_.equipSlotGuids_[19 + bagIdx]; + uint64_t bagGuid = owner_.equipSlotGuids_[Inventory::FIRST_BAG_EQUIP_SLOT + bagIdx]; if (bagGuid == 0) continue; // Determine bag size from container fields or item template @@ -2736,11 +2736,11 @@ void InventoryHandler::rebuildOnlineInventory() { owner_.inventory.setBagSize(bagIdx, numSlots); // Also set bagSlots on the equipped bag item (for UI display) - auto& bagEquipSlot = owner_.inventory.getEquipSlot(static_cast(19 + bagIdx)); + auto& bagEquipSlot = owner_.inventory.getEquipSlot(static_cast(Inventory::FIRST_BAG_EQUIP_SLOT + bagIdx)); if (!bagEquipSlot.empty()) { ItemDef bagDef = bagEquipSlot.item; bagDef.bagSlots = numSlots; - owner_.inventory.setEquipSlot(static_cast(19 + bagIdx), bagDef); + owner_.inventory.setEquipSlot(static_cast(Inventory::FIRST_BAG_EQUIP_SLOT + bagIdx), bagDef); } // Populate bag slot items diff --git a/src/game/spell_handler.cpp b/src/game/spell_handler.cpp index a265e33a..3fc626de 100644 --- a/src/game/spell_handler.cpp +++ b/src/game/spell_handler.cpp @@ -486,7 +486,7 @@ void SpellHandler::useItemInBag(int bagIndex, int slotIndex) { if (slot.empty()) return; uint64_t itemGuid = 0; - uint64_t bagGuid = owner_.equipSlotGuids_[19 + bagIndex]; + uint64_t bagGuid = owner_.equipSlotGuids_[Inventory::FIRST_BAG_EQUIP_SLOT + bagIndex]; if (bagGuid != 0) { auto it = owner_.containerContents_.find(bagGuid); if (it != owner_.containerContents_.end() && slotIndex < static_cast(it->second.numSlots)) { @@ -502,7 +502,7 @@ void SpellHandler::useItemInBag(int bagIndex, int slotIndex) { if (itemGuid != 0 && owner_.state == WorldState::IN_WORLD && owner_.socket) { uint32_t useSpellId = findOnUseSpellId(slot.item.itemId); - uint8_t wowBag = static_cast(19 + bagIndex); + uint8_t wowBag = static_cast(Inventory::FIRST_BAG_EQUIP_SLOT + bagIndex); auto packet = owner_.packetParsers_ ? owner_.packetParsers_->buildUseItem(wowBag, static_cast(slotIndex), itemGuid, useSpellId) : UseItemPacket::build(wowBag, static_cast(slotIndex), itemGuid, useSpellId); diff --git a/src/network/world_socket.cpp b/src/network/world_socket.cpp index ba15003d..ca34cf11 100644 --- a/src/network/world_socket.cpp +++ b/src/network/world_socket.cpp @@ -15,6 +15,9 @@ namespace { constexpr size_t kMaxReceiveBufferBytes = 8 * 1024 * 1024; +// Per-frame packet budgets prevent a burst of server data from starving the +// render loop. Tunable via env vars for debugging heavy-traffic scenarios +// (e.g. SMSG_UPDATE_OBJECT floods on login to crowded zones). constexpr int kDefaultMaxParsedPacketsPerUpdate = 64; constexpr int kAbsoluteMaxParsedPacketsPerUpdate = 220; constexpr int kMinParsedPacketsPerUpdate = 8;