From d6e398d814fa2c1d69aa848ce42f4ca6baaaa2c1 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 03:53:18 -0700 Subject: [PATCH 01/36] fix: add Classic parseCastResult override with result enum +1 shift Classic 1.12 SMSG_CAST_RESULT uses an enum starting at 0=AFFECTING_COMBAT (no SUCCESS entry), while WotLK starts at 0=SUCCESS, 1=AFFECTING_COMBAT. Without this override, Classic result codes were handled by TBC's parseCastResult which passed them unshifted, causing result 0 (AFFECTING_COMBAT) to be silently treated as success with no error shown. This applies the same +1 shift used in parseCastFailed so all Classic spell failure codes map correctly to getSpellCastResultString. --- include/game/packet_parsers.hpp | 1 + src/game/packet_parsers_classic.cpp | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/include/game/packet_parsers.hpp b/include/game/packet_parsers.hpp index 2cb17fdb..9fb0f166 100644 --- a/include/game/packet_parsers.hpp +++ b/include/game/packet_parsers.hpp @@ -389,6 +389,7 @@ public: network::Packet buildCastSpell(uint32_t spellId, uint64_t targetGuid, uint8_t castCount) override; network::Packet buildUseItem(uint8_t bagIndex, uint8_t slotIndex, uint64_t itemGuid, uint32_t spellId = 0) override; bool parseCastFailed(network::Packet& packet, CastFailedData& data) override; + bool parseCastResult(network::Packet& packet, uint32_t& spellId, uint8_t& result) override; bool parseMessageChat(network::Packet& packet, MessageChatData& data) override; bool parseGameObjectQueryResponse(network::Packet& packet, GameObjectQueryResponseData& data) override; // Classic 1.12 SMSG_CREATURE_QUERY_RESPONSE lacks the iconName string that TBC/WotLK include diff --git a/src/game/packet_parsers_classic.cpp b/src/game/packet_parsers_classic.cpp index 35dc54f4..7277a184 100644 --- a/src/game/packet_parsers_classic.cpp +++ b/src/game/packet_parsers_classic.cpp @@ -633,6 +633,22 @@ bool ClassicPacketParsers::parseCastFailed(network::Packet& packet, CastFailedDa return true; } +// ============================================================================ +// Classic SMSG_CAST_RESULT: same layout as parseCastFailed (spellId + result), +// but the result enum starts at 0=AFFECTING_COMBAT (no SUCCESS entry). +// Apply the same +1 shift used in parseCastFailed so the result codes +// align with WotLK's getSpellCastResultString table. +// ============================================================================ +bool ClassicPacketParsers::parseCastResult(network::Packet& packet, uint32_t& spellId, uint8_t& result) { + if (packet.getSize() - packet.getReadPos() < 5) return false; + spellId = packet.readUInt32(); + uint8_t vanillaResult = packet.readUInt8(); + // Shift +1: Vanilla result 0=AFFECTING_COMBAT maps to WotLK result 1=AFFECTING_COMBAT + result = vanillaResult + 1; + LOG_DEBUG("[Classic] Cast result: spell=", spellId, " vanillaResult=", (int)vanillaResult); + return true; +} + // ============================================================================ // Classic 1.12.1 parseCharEnum // Differences from TBC: From dd7d74cb933627b0669d6a51d02ebab63ab13ab6 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 03:54:33 -0700 Subject: [PATCH 02/36] fix: correct SMSG_SPELL_FAILURE Classic format and result enum shift Classic 1.12 SMSG_SPELL_FAILURE omits the castCount byte that TBC/WotLK include (format: uint64 GUID + uint32 spellId + uint8 failReason). The previous code read a castCount for all expansions, misaligning spellId and failReason for Classic by one byte. Also apply the same +1 enum shift used in parseCastFailed/parseCastResult: Classic result 0=AFFECTING_COMBAT maps to WotLK 1=AFFECTING_COMBAT, so Classic failReason=0 now correctly shows an error instead of being silently swallowed. --- src/game/game_handler.cpp | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index e9452785..bce39d4a 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -2747,16 +2747,21 @@ void GameHandler::handlePacket(network::Packet& packet) { break; case Opcode::SMSG_SPELL_FAILURE: { // WotLK: packed_guid + uint8 castCount + uint32 spellId + uint8 failReason - // TBC/Classic: full uint64 + uint8 castCount + uint32 spellId + uint8 failReason - const bool tbcOrClassic = isClassicLikeExpansion() || isActiveExpansion("tbc"); - uint64_t failGuid = tbcOrClassic + // TBC: full uint64 + uint8 castCount + uint32 spellId + uint8 failReason + // Classic: full uint64 + uint32 spellId + uint8 failReason (NO castCount) + const bool isClassic = isClassicLikeExpansion(); + const bool isTbc = isActiveExpansion("tbc"); + uint64_t failGuid = (isClassic || isTbc) ? (packet.getSize() - packet.getReadPos() >= 8 ? packet.readUInt64() : 0) : UpdateObjectParser::readPackedGuid(packet); - // Read castCount + spellId + failReason - if (packet.getSize() - packet.getReadPos() >= 6) { - /*uint8_t castCount =*/ packet.readUInt8(); + // Classic omits the castCount byte; TBC and WotLK include it + const size_t remainingFields = isClassic ? 5u : 6u; // spellId(4)+reason(1) [+castCount(1)] + if (packet.getSize() - packet.getReadPos() >= remainingFields) { + if (!isClassic) /*uint8_t castCount =*/ packet.readUInt8(); /*uint32_t spellId =*/ packet.readUInt32(); - uint8_t failReason = packet.readUInt8(); + uint8_t rawFailReason = packet.readUInt8(); + // Classic result enum starts at 0=AFFECTING_COMBAT; shift +1 for WotLK table + uint8_t failReason = isClassic ? static_cast(rawFailReason + 1) : rawFailReason; if (failGuid == playerGuid && failReason != 0) { // Show interruption/failure reason in chat for player int pt = -1; From 750b270502f3cfed746099bd04988557f9f4ccf7 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 04:01:07 -0700 Subject: [PATCH 03/36] fix: use expansion-aware item size in LootResponseParser for Classic/TBC MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous per-iteration heuristic (remaining >= 22 → 22 bytes, >= 14 → 14 bytes) incorrectly parsed Classic/TBC multi-item loots: 2+ items × 14 bytes would trigger the 22-byte WotLK path for the first item, corrupting subsequent items. Classic 1.12 and TBC 2.4.3 use 14 bytes/item (slot+itemId+count+displayInfo+slotType). WotLK 3.3.5a uses 22 bytes/item (adds randomSuffix+randomPropertyId). Add isWotlkFormat bool parameter to LootResponseParser::parse and pass isActiveExpansion('wotlk') from handleLootResponse. --- include/game/world_packets.hpp | 4 +++- src/game/game_handler.cpp | 5 ++++- src/game/world_packets.cpp | 41 +++++++++++++--------------------- 3 files changed, 22 insertions(+), 28 deletions(-) diff --git a/include/game/world_packets.hpp b/include/game/world_packets.hpp index 61d36ebf..539d3b8a 100644 --- a/include/game/world_packets.hpp +++ b/include/game/world_packets.hpp @@ -2015,7 +2015,9 @@ public: /** SMSG_LOOT_RESPONSE parser */ class LootResponseParser { public: - static bool parse(network::Packet& packet, LootResponseData& data); + // isWotlkFormat: true for WotLK 3.3.5a (22 bytes/item with randomSuffix+randomProp), + // false for Classic 1.12 and TBC 2.4.3 (14 bytes/item). + static bool parse(network::Packet& packet, LootResponseData& data, bool isWotlkFormat = true); }; // ============================================================ diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index bce39d4a..f7f42b59 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -16164,7 +16164,10 @@ void GameHandler::unstuckHearth() { } void GameHandler::handleLootResponse(network::Packet& packet) { - if (!LootResponseParser::parse(packet, currentLoot)) return; + // Classic 1.12 and TBC 2.4.3 use 14 bytes/item (no randomSuffix/randomProp fields); + // WotLK 3.3.5a uses 22 bytes/item. + const bool wotlkLoot = isActiveExpansion("wotlk"); + if (!LootResponseParser::parse(packet, currentLoot, wotlkLoot)) return; lootWindowOpen = true; localLootState_[currentLoot.lootGuid] = LocalLootState{currentLoot, false}; diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 545f2f70..8c7c5ec9 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -3320,7 +3320,7 @@ network::Packet LootReleasePacket::build(uint64_t lootGuid) { return packet; } -bool LootResponseParser::parse(network::Packet& packet, LootResponseData& data) { +bool LootResponseParser::parse(network::Packet& packet, LootResponseData& data, bool isWotlkFormat) { data = LootResponseData{}; if (packet.getSize() - packet.getReadPos() < 14) { LOG_WARNING("LootResponseParser: packet too short"); @@ -3332,45 +3332,34 @@ bool LootResponseParser::parse(network::Packet& packet, LootResponseData& data) data.gold = packet.readUInt32(); uint8_t itemCount = packet.readUInt8(); + // Item wire size: + // WotLK 3.3.5a: slot(1)+itemId(4)+count(4)+displayInfo(4)+randSuffix(4)+randProp(4)+slotType(1) = 22 + // Classic/TBC: slot(1)+itemId(4)+count(4)+displayInfo(4)+slotType(1) = 14 + const size_t kItemSize = isWotlkFormat ? 22u : 14u; + auto parseLootItemList = [&](uint8_t listCount, bool markQuestItems) -> bool { for (uint8_t i = 0; i < listCount; ++i) { size_t remaining = packet.getSize() - packet.getReadPos(); - if (remaining < 10) { + if (remaining < kItemSize) { return false; } - // Prefer the richest format when possible: - // 22-byte (WotLK/full): slot+id+count+display+randSuffix+randProp+slotType - // 14-byte (compact): slot+id+count+display+slotType - // 10-byte (minimal): slot+id+count+slotType - uint8_t bytesPerItem = 10; - if (remaining >= 22) { - bytesPerItem = 22; - } else if (remaining >= 14) { - bytesPerItem = 14; - } - LootItem item; - item.slotIndex = packet.readUInt8(); - item.itemId = packet.readUInt32(); - item.count = packet.readUInt32(); + item.slotIndex = packet.readUInt8(); + item.itemId = packet.readUInt32(); + item.count = packet.readUInt32(); + item.displayInfoId = packet.readUInt32(); - if (bytesPerItem >= 14) { - item.displayInfoId = packet.readUInt32(); - } else { - item.displayInfoId = 0; - } - - if (bytesPerItem == 22) { - item.randomSuffix = packet.readUInt32(); + if (isWotlkFormat) { + item.randomSuffix = packet.readUInt32(); item.randomPropertyId = packet.readUInt32(); } else { - item.randomSuffix = 0; + item.randomSuffix = 0; item.randomPropertyId = 0; } item.lootSlotType = packet.readUInt8(); - item.isQuestItem = markQuestItems; + item.isQuestItem = markQuestItems; data.items.push_back(item); } return true; From 8493729a1073d37cca5f0ab27a5a4c0cecc7fd06 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 04:08:16 -0700 Subject: [PATCH 04/36] fix: use uint16 spellId in Classic 1.12 SMSG_LEARNED/REMOVED/SUPERCEDED_SPELL Classic 1.12 (vmangos/cmangos) sends uint16 spellIds in SMSG_LEARNED_SPELL, SMSG_REMOVED_SPELL, and SMSG_SUPERCEDED_SPELL. TBC 2.4.3 and WotLK 3.3.5a use uint32. The handlers were unconditionally reading uint32, causing the first byte of the next field to be consumed as part of the spellId on Classic, producing garbage spell IDs and breaking known-spell tracking. Apply the same Classic/TBC+WotLK gate used by the SMSG_INITIAL_SPELLS heuristic: read uint16 for Classic, uint32 for all others. --- src/game/game_handler.cpp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index f7f42b59..06e4c057 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -14174,8 +14174,11 @@ void GameHandler::handleAuraUpdate(network::Packet& packet, bool isAll) { } void GameHandler::handleLearnedSpell(network::Packet& packet) { - if (packet.getSize() - packet.getReadPos() < 4) return; - uint32_t spellId = packet.readUInt32(); + // Classic 1.12: uint16 spellId; TBC 2.4.3 / WotLK 3.3.5a: uint32 spellId + const bool classicSpellId = isClassicLikeExpansion(); + const size_t minSz = classicSpellId ? 2u : 4u; + if (packet.getSize() - packet.getReadPos() < minSz) return; + uint32_t spellId = classicSpellId ? packet.readUInt16() : packet.readUInt32(); knownSpells.insert(spellId); LOG_INFO("Learned spell: ", spellId); @@ -14203,17 +14206,24 @@ void GameHandler::handleLearnedSpell(network::Packet& packet) { } void GameHandler::handleRemovedSpell(network::Packet& packet) { - if (packet.getSize() - packet.getReadPos() < 4) return; - uint32_t spellId = packet.readUInt32(); + // Classic 1.12: uint16 spellId; TBC 2.4.3 / WotLK 3.3.5a: uint32 spellId + const bool classicSpellId = isClassicLikeExpansion(); + const size_t minSz = classicSpellId ? 2u : 4u; + if (packet.getSize() - packet.getReadPos() < minSz) return; + uint32_t spellId = classicSpellId ? packet.readUInt16() : packet.readUInt32(); knownSpells.erase(spellId); LOG_INFO("Removed spell: ", spellId); } void GameHandler::handleSupercededSpell(network::Packet& packet) { // Old spell replaced by new rank (e.g., Fireball Rank 1 -> Fireball Rank 2) - if (packet.getSize() - packet.getReadPos() < 8) return; - uint32_t oldSpellId = packet.readUInt32(); - uint32_t newSpellId = packet.readUInt32(); + // Classic 1.12: uint16 oldSpellId + uint16 newSpellId (4 bytes total) + // TBC 2.4.3 / WotLK 3.3.5a: uint32 oldSpellId + uint32 newSpellId (8 bytes total) + const bool classicSpellId = isClassicLikeExpansion(); + const size_t minSz = classicSpellId ? 4u : 8u; + if (packet.getSize() - packet.getReadPos() < minSz) return; + uint32_t oldSpellId = classicSpellId ? packet.readUInt16() : packet.readUInt32(); + uint32_t newSpellId = classicSpellId ? packet.readUInt16() : packet.readUInt32(); // Remove old spell knownSpells.erase(oldSpellId); From fed03f970cd6ba83653e6a72865a424414b3454c Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 04:22:18 -0700 Subject: [PATCH 05/36] fix: correct SMSG_BATTLEFIELD_STATUS Classic 1.12 packet layout Classic uses queueSlot(4)+bgTypeId(4)+unk(2)+instanceId(4)+isReg(1)+statusId(4); TBC/WotLK prefixes arenaType(1)+unk(1) before bgTypeId. Reading TBC format on Classic caused bgTypeId to be read from wrong offset, corrupting BG queue state. --- src/game/game_handler.cpp | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 06e4c057..18b5a54f 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -12242,20 +12242,40 @@ void GameHandler::handleMoveKnockBack(network::Packet& packet) { // ============================================================ void GameHandler::handleBattlefieldStatus(network::Packet& packet) { + // SMSG_BATTLEFIELD_STATUS wire format differs by expansion: + // + // Classic 1.12 (vmangos/cmangos): + // queueSlot(4) bgTypeId(4) unk(2) instanceId(4) isRegistered(1) statusId(4) [status fields...] + // STATUS_NONE sends only: queueSlot(4) bgTypeId(4) + // + // TBC 2.4.3 / WotLK 3.3.5a: + // queueSlot(4) arenaType(1) unk(1) bgTypeId(4) unk2(2) instanceId(4) isRated(1) statusId(4) [status fields...] + // STATUS_NONE sends only: queueSlot(4) arenaType(1) + if (packet.getSize() - packet.getReadPos() < 4) return; uint32_t queueSlot = packet.readUInt32(); - // Minimal packet = just queueSlot + arenaType(1) when status is NONE - if (packet.getSize() - packet.getReadPos() < 1) { - LOG_INFO("Battlefield status: queue slot ", queueSlot, " cleared"); - return; + const bool classicFormat = isClassicLikeExpansion(); + + uint8_t arenaType = 0; + if (!classicFormat) { + // TBC/WotLK: arenaType(1) + unk(1) before bgTypeId + // STATUS_NONE sends only queueSlot + arenaType + if (packet.getSize() - packet.getReadPos() < 1) { + LOG_INFO("Battlefield status: queue slot ", queueSlot, " cleared"); + return; + } + arenaType = packet.readUInt8(); + if (packet.getSize() - packet.getReadPos() < 1) return; + packet.readUInt8(); // unk + } else { + // Classic STATUS_NONE sends only queueSlot + bgTypeId (4 bytes) + if (packet.getSize() - packet.getReadPos() < 4) { + LOG_INFO("Battlefield status: queue slot ", queueSlot, " cleared"); + return; + } } - uint8_t arenaType = packet.readUInt8(); - if (packet.getSize() - packet.getReadPos() < 1) return; - - // Unknown byte - packet.readUInt8(); if (packet.getSize() - packet.getReadPos() < 4) return; uint32_t bgTypeId = packet.readUInt32(); From d3241dce9ef7265825eef8a90a1bbb4d62ec7a68 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 04:25:00 -0700 Subject: [PATCH 06/36] fix: handle Classic 1.12 SMSG_WEATHER missing isAbrupt byte Classic 1.12 sends weatherType(4)+intensity(4) with no trailing isAbrupt byte; TBC/WotLK append uint8 isAbrupt. The prior check required >= 9 bytes, so weather never updated on Classic servers. Now accept >= 8 bytes and read isAbrupt only if the byte is present. --- src/game/game_handler.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 18b5a54f..b796f509 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -4078,11 +4078,13 @@ void GameHandler::handlePacket(network::Packet& packet) { break; } case Opcode::SMSG_WEATHER: { - // Format: uint32 weatherType, float intensity, uint8 isAbrupt - if (packet.getSize() - packet.getReadPos() >= 9) { + // Classic 1.12: uint32 weatherType + float intensity (8 bytes, no isAbrupt) + // TBC 2.4.3 / WotLK 3.3.5a: uint32 weatherType + float intensity + uint8 isAbrupt (9 bytes) + if (packet.getSize() - packet.getReadPos() >= 8) { uint32_t wType = packet.readUInt32(); float wIntensity = packet.readFloat(); - /*uint8_t isAbrupt =*/ packet.readUInt8(); + if (packet.getSize() - packet.getReadPos() >= 1) + /*uint8_t isAbrupt =*/ packet.readUInt8(); weatherType_ = wType; weatherIntensity_ = wIntensity; const char* typeName = (wType == 1) ? "Rain" : (wType == 2) ? "Snow" : (wType == 3) ? "Storm" : "Clear"; From 9d0da6242d7611c3c49f092086425320cb4f2dc2 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 04:32:00 -0700 Subject: [PATCH 07/36] fix: correct Classic/TBC MSG_MOVE_TELEPORT_ACK movement info parsing Classic 1.12 and TBC 2.4.3 movement packets omit the moveFlags2 (uint16) field present in WotLK 3.3.5a. The prior handler unconditionally read 2 bytes for moveFlags2, shifting the timestamp and position reads by 2 bytes and producing garbage coordinates after a teleport. Now gated by expansion. --- src/game/game_handler.cpp | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index b796f509..a870ddcc 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -16892,15 +16892,20 @@ void GameHandler::handleTeleportAck(network::Packet& packet) { if (packet.getSize() - packet.getReadPos() < 4) return; uint32_t counter = packet.readUInt32(); - // Read the movement info embedded in the teleport - // Format: u32 flags, u16 flags2, u32 time, float x, float y, float z, float o - if (packet.getSize() - packet.getReadPos() < 4 + 2 + 4 + 4 * 4) { + // Read the movement info embedded in the teleport. + // WotLK: moveFlags(4) + moveFlags2(2) + time(4) + x(4) + y(4) + z(4) + o(4) = 26 bytes + // Classic 1.12 / TBC 2.4.3: moveFlags(4) + time(4) + x(4) + y(4) + z(4) + o(4) = 24 bytes + // (Classic and TBC have no moveFlags2 field in movement packets) + const bool taNoFlags2 = isClassicLikeExpansion() || isActiveExpansion("tbc"); + const size_t minMoveSz = taNoFlags2 ? (4 + 4 + 4 * 4) : (4 + 2 + 4 + 4 * 4); + if (packet.getSize() - packet.getReadPos() < minMoveSz) { LOG_WARNING("MSG_MOVE_TELEPORT_ACK: not enough data for movement info"); return; } packet.readUInt32(); // moveFlags - packet.readUInt16(); // moveFlags2 + if (!taNoFlags2) + packet.readUInt16(); // moveFlags2 (WotLK only) uint32_t moveTime = packet.readUInt32(); float serverX = packet.readFloat(); float serverY = packet.readFloat(); From ed48a3c425e72f0ae875eac83031a61957f8df4e Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 04:38:30 -0700 Subject: [PATCH 08/36] fix: replace fragile heuristic in SMSG_INITIAL_SPELLS with explicit Classic format flag Classic 1.12 uses uint16 spellId + uint16 slot (4 bytes/spell); TBC and WotLK use uint32 spellId + uint16 unknown (6 bytes/spell). The old size-based heuristic could misdetect TBC packets that happened to fit both layouts. Add a vanillaFormat parameter to InitialSpellsParser::parse and override parseInitialSpells in ClassicPacketParsers to always pass true, eliminating the ambiguity. --- include/game/packet_parsers.hpp | 4 ++++ include/game/world_packets.hpp | 5 ++++- src/game/world_packets.cpp | 13 ++++--------- 3 files changed, 12 insertions(+), 10 deletions(-) diff --git a/include/game/packet_parsers.hpp b/include/game/packet_parsers.hpp index 9fb0f166..40655045 100644 --- a/include/game/packet_parsers.hpp +++ b/include/game/packet_parsers.hpp @@ -419,6 +419,10 @@ public: bool parseMonsterMove(network::Packet& packet, MonsterMoveData& data) override { return MonsterMoveParser::parseVanilla(packet, data); } + // Classic 1.12 SMSG_INITIAL_SPELLS: uint16 spellId + uint16 slot per entry (not uint32 + uint16) + bool parseInitialSpells(network::Packet& packet, InitialSpellsData& data) override { + return InitialSpellsParser::parse(packet, data, /*vanillaFormat=*/true); + } // Classic 1.12 uses PackedGuid (not full uint64) and uint16 castFlags (not uint32) bool parseSpellStart(network::Packet& packet, SpellStartData& data) override; bool parseSpellGo(network::Packet& packet, SpellGoData& data) override; diff --git a/include/game/world_packets.hpp b/include/game/world_packets.hpp index 539d3b8a..6e5721fd 100644 --- a/include/game/world_packets.hpp +++ b/include/game/world_packets.hpp @@ -1758,7 +1758,10 @@ struct InitialSpellsData { class InitialSpellsParser { public: - static bool parse(network::Packet& packet, InitialSpellsData& data); + // vanillaFormat=true: Classic 1.12 uint16 spellId + uint16 slot (4 bytes/spell) + // vanillaFormat=false: TBC/WotLK uint32 spellId + uint16 unk (6 bytes/spell) + static bool parse(network::Packet& packet, InitialSpellsData& data, + bool vanillaFormat = false); }; /** CMSG_CAST_SPELL packet builder */ diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index 8c7c5ec9..b7d8075b 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -2901,18 +2901,13 @@ bool XpGainParser::parse(network::Packet& packet, XpGainData& data) { // Phase 3: Spells, Action Bar, Auras // ============================================================ -bool InitialSpellsParser::parse(network::Packet& packet, InitialSpellsData& data) { - size_t packetSize = packet.getSize(); +bool InitialSpellsParser::parse(network::Packet& packet, InitialSpellsData& data, + bool vanillaFormat) { data.talentSpec = packet.readUInt8(); uint16_t spellCount = packet.readUInt16(); - // Detect vanilla (uint16 spellId) vs WotLK (uint32 spellId) format - // Vanilla: 4 bytes/spell (uint16 id + uint16 slot), WotLK: 6 bytes/spell (uint32 id + uint16 unk) - size_t remainingAfterHeader = packetSize - 3; // subtract talentSpec(1) + spellCount(2) - bool vanillaFormat = remainingAfterHeader < static_cast(spellCount) * 6 + 2; - - LOG_DEBUG("SMSG_INITIAL_SPELLS: packetSize=", packetSize, " bytes, spellCount=", spellCount, - vanillaFormat ? " (vanilla uint16 format)" : " (WotLK uint32 format)"); + LOG_DEBUG("SMSG_INITIAL_SPELLS: spellCount=", spellCount, + vanillaFormat ? " (vanilla uint16 format)" : " (TBC/WotLK uint32 format)"); data.spellIds.reserve(spellCount); for (uint16_t i = 0; i < spellCount; ++i) { From dd67c8817572751192d2bd77ee12ad296530830c Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 04:49:18 -0700 Subject: [PATCH 09/36] fix: conditionally include trailing byte in CMSG_BUY_ITEM for Classic/TBC CMSG_BUY_ITEM format differs by expansion: - WotLK 3.3.5a / AzerothCore: includes trailing uint8(0) after count field (17 bytes) - Classic 1.12 / TBC 2.4.3: no trailing byte (16 bytes) The static BuyItemPacket::build() helper always adds the byte (AzerothCore compat). GameHandler::buyItem() now gates the byte based on active expansion, allowing Classic/TBC servers to receive correctly-sized packets. --- src/game/game_handler.cpp | 7 +++++-- src/game/world_packets.cpp | 4 +++- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index a870ddcc..0ead0468 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -15821,8 +15821,11 @@ void GameHandler::buyItem(uint64_t vendorGuid, uint32_t itemId, uint32_t slot, u packet.writeUInt32(itemId); // item entry packet.writeUInt32(slot); // vendor slot index packet.writeUInt32(count); - // WotLK/AzerothCore expects a trailing byte here. - packet.writeUInt8(0); + // WotLK/AzerothCore expects a trailing byte; Classic/TBC do not + const bool isWotLk = isActiveExpansion("wotlk"); + if (isWotLk) { + packet.writeUInt8(0); + } socket->send(packet); } diff --git a/src/game/world_packets.cpp b/src/game/world_packets.cpp index b7d8075b..5d3989c7 100644 --- a/src/game/world_packets.cpp +++ b/src/game/world_packets.cpp @@ -3828,7 +3828,9 @@ network::Packet BuyItemPacket::build(uint64_t vendorGuid, uint32_t itemId, uint3 packet.writeUInt32(itemId); // item entry packet.writeUInt32(slot); // vendor slot index from SMSG_LIST_INVENTORY packet.writeUInt32(count); - // WotLK/AzerothCore expects a trailing byte on CMSG_BUY_ITEM. + // Note: WotLK/AzerothCore expects a trailing byte; Classic/TBC do not. + // This static helper always adds it (appropriate for CMaNGOS/AzerothCore). + // For Classic/TBC, use the GameHandler::buyItem() path which checks expansion. packet.writeUInt8(0); return packet; } From 593f06bdf79216123496a9d3bbf9366ffed39708 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 05:09:43 -0700 Subject: [PATCH 10/36] fix: correct Classic/TBC loot packet format parsing (missing randomSuffix/randomPropId) SMSG_LOOT_START_ROLL, SMSG_LOOT_ALL_PASSED, and loot roll handlers unconditionally read randomSuffix and randomPropertyId fields. These fields only exist in WotLK 3.3.5a and NOT in Classic 1.12 / TBC 2.4.3, causing packet stream corruption on Classic/TBC servers. Packet format differences: - WotLK: includes randomSuffix (4) + randomPropId (4) fields - Classic/TBC: no random property fields Fix gates the field reads based on active expansion: - SMSG_LOOT_START_ROLL: WotLK 33 bytes vs Classic/TBC 25 bytes - SMSG_LOOT_ALL_PASSED: WotLK 24 bytes vs Classic/TBC 16 bytes - SMSG_LOOT_ROLL: WotLK 34 bytes vs Classic/TBC 26 bytes - SMSG_LOOT_ROLL_WON: WotLK 34 bytes vs Classic/TBC 26 bytes This prevents packet stream desynchronization when loot rolls occur on Classic/TBC servers. --- src/game/game_handler.cpp | 58 ++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/src/game/game_handler.cpp b/src/game/game_handler.cpp index 0ead0468..458a06bf 100644 --- a/src/game/game_handler.cpp +++ b/src/game/game_handler.cpp @@ -1963,15 +1963,21 @@ void GameHandler::handlePacket(network::Packet& packet) { // ---- Loot start roll (Need/Greed popup trigger) ---- case Opcode::SMSG_LOOT_START_ROLL: { - // uint64 objectGuid + uint32 mapId + uint32 lootSlot + uint32 itemId - // + uint32 randomSuffix + uint32 randomPropId + uint32 countdown + uint8 voteMask - if (packet.getSize() - packet.getReadPos() < 33) break; + // WotLK 3.3.5a: uint64 objectGuid + uint32 mapId + uint32 lootSlot + uint32 itemId + // + uint32 randomSuffix + uint32 randomPropId + uint32 countdown + uint8 voteMask (33 bytes) + // Classic/TBC: uint64 objectGuid + uint32 mapId + uint32 lootSlot + uint32 itemId + // + uint32 countdown + uint8 voteMask (25 bytes) + const bool isWotLK = isActiveExpansion("wotlk"); + const size_t minSize = isWotLK ? 33u : 25u; + if (packet.getSize() - packet.getReadPos() < minSize) break; uint64_t objectGuid = packet.readUInt64(); /*uint32_t mapId =*/ packet.readUInt32(); uint32_t slot = packet.readUInt32(); uint32_t itemId = packet.readUInt32(); - /*uint32_t randSuffix =*/ packet.readUInt32(); - /*uint32_t randProp =*/ packet.readUInt32(); + if (isWotLK) { + /*uint32_t randSuffix =*/ packet.readUInt32(); + /*uint32_t randProp =*/ packet.readUInt32(); + } /*uint32_t countdown =*/ packet.readUInt32(); /*uint8_t voteMask =*/ packet.readUInt8(); // Trigger the roll popup for local player @@ -2344,11 +2350,18 @@ void GameHandler::handlePacket(network::Packet& packet) { // ---- Loot notifications ---- case Opcode::SMSG_LOOT_ALL_PASSED: { - // uint64 objectGuid + uint32 slot + uint32 itemId + uint32 randSuffix + uint32 randPropId - if (packet.getSize() - packet.getReadPos() < 24) break; + // WotLK 3.3.5a: uint64 objectGuid + uint32 slot + uint32 itemId + uint32 randSuffix + uint32 randPropId (24 bytes) + // Classic/TBC: uint64 objectGuid + uint32 slot + uint32 itemId (16 bytes) + const bool isWotLK = isActiveExpansion("wotlk"); + const size_t minSize = isWotLK ? 24u : 16u; + if (packet.getSize() - packet.getReadPos() < minSize) break; /*uint64_t objGuid =*/ packet.readUInt64(); /*uint32_t slot =*/ packet.readUInt32(); uint32_t itemId = packet.readUInt32(); + if (isWotLK) { + /*uint32_t randSuffix =*/ packet.readUInt32(); + /*uint32_t randProp =*/ packet.readUInt32(); + } auto* info = getItemInfo(itemId); char buf[256]; std::snprintf(buf, sizeof(buf), "Everyone passed on [%s].", @@ -19516,18 +19529,23 @@ void GameHandler::handleTradeStatusExtended(network::Packet& packet) { // --------------------------------------------------------------------------- void GameHandler::handleLootRoll(network::Packet& packet) { - // uint64 objectGuid, uint32 slot, uint64 playerGuid, - // uint32 itemId, uint32 randomSuffix, uint32 randomPropId, - // uint8 rollNumber, uint8 rollType + // WotLK 3.3.5a: uint64 objectGuid, uint32 slot, uint64 playerGuid, + // uint32 itemId, uint32 randomSuffix, uint32 randomPropId, uint8 rollNumber, uint8 rollType (34 bytes) + // Classic/TBC: uint64 objectGuid, uint32 slot, uint64 playerGuid, + // uint32 itemId, uint8 rollNumber, uint8 rollType (26 bytes) + const bool isWotLK = isActiveExpansion("wotlk"); + const size_t minSize = isWotLK ? 34u : 26u; size_t rem = packet.getSize() - packet.getReadPos(); - if (rem < 26) return; // minimum: 8+4+8+4+4+4+1+1 = 34, be lenient + if (rem < minSize) return; uint64_t objectGuid = packet.readUInt64(); uint32_t slot = packet.readUInt32(); uint64_t rollerGuid = packet.readUInt64(); uint32_t itemId = packet.readUInt32(); - /*uint32_t randSuffix =*/ packet.readUInt32(); - /*uint32_t randProp =*/ packet.readUInt32(); + if (isWotLK) { + /*uint32_t randSuffix =*/ packet.readUInt32(); + /*uint32_t randProp =*/ packet.readUInt32(); + } uint8_t rollNum = packet.readUInt8(); uint8_t rollType = packet.readUInt8(); @@ -19574,15 +19592,23 @@ void GameHandler::handleLootRoll(network::Packet& packet) { } void GameHandler::handleLootRollWon(network::Packet& packet) { + // WotLK 3.3.5a: uint64 objectGuid, uint32 slot, uint64 winnerGuid, + // uint32 itemId, uint32 randomSuffix, uint32 randomPropId, uint8 rollNumber, uint8 rollType (34 bytes) + // Classic/TBC: uint64 objectGuid, uint32 slot, uint64 winnerGuid, + // uint32 itemId, uint8 rollNumber, uint8 rollType (26 bytes) + const bool isWotLK = isActiveExpansion("wotlk"); + const size_t minSize = isWotLK ? 34u : 26u; size_t rem = packet.getSize() - packet.getReadPos(); - if (rem < 26) return; + if (rem < minSize) return; /*uint64_t objectGuid =*/ packet.readUInt64(); /*uint32_t slot =*/ packet.readUInt32(); uint64_t winnerGuid = packet.readUInt64(); uint32_t itemId = packet.readUInt32(); - /*uint32_t randSuffix =*/ packet.readUInt32(); - /*uint32_t randProp =*/ packet.readUInt32(); + if (isWotLK) { + /*uint32_t randSuffix =*/ packet.readUInt32(); + /*uint32_t randProp =*/ packet.readUInt32(); + } uint8_t rollNum = packet.readUInt8(); uint8_t rollType = packet.readUInt8(); From 79c8d93c4537f281d891e9eb286f7236bd4f51f8 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 05:26:38 -0700 Subject: [PATCH 11/36] fix: use expansion-aware field indices for spell icon loading The spell icon loader was incorrectly assuming WotLK field 133 (IconID) for any DBC with >= 200 fields. This breaks Classic/TBC where IconID is at different fields: - Classic: field 117 - TBC: field 124 - WotLK: field 133 Now always uses expansion-aware layout (spellL) when available, falling back to hardcoded field 133 only if the layout is missing. Fixes missing spell icons on Classic and TBC expansions. --- src/ui/game_screen.cpp | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index 52e056fb..a139f15f 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -4059,7 +4059,7 @@ VkDescriptorSet GameScreen::getSpellIcon(uint32_t spellId, pipeline::AssetManage const auto* spellL = pipeline::getActiveDBCLayout() ? pipeline::getActiveDBCLayout()->getLayout("Spell") : nullptr; if (spellDbc && spellDbc->isLoaded()) { uint32_t fieldCount = spellDbc->getFieldCount(); - // Try expansion layout first + // Helper to load icons for a given field layout auto tryLoadIcons = [&](uint32_t idField, uint32_t iconField) { spellIconIds_.clear(); if (iconField >= fieldCount) return; @@ -4071,16 +4071,16 @@ VkDescriptorSet GameScreen::getSpellIcon(uint32_t spellId, pipeline::AssetManage } } }; - // If the DBC has WotLK-range field count (≥200 fields), it's the binary - // WotLK Spell.dbc (CSV fallback). Use WotLK layout regardless of expansion, - // since Turtle/Classic CSV files are garbled and fall back to WotLK binary. - if (fieldCount >= 200) { - tryLoadIcons(0, 133); // WotLK IconID field - } else if (spellL) { + + // Always use expansion-aware layout if available + // Field indices vary by expansion: Classic=117, TBC=124, WotLK=133 + if (spellL) { tryLoadIcons((*spellL)["ID"], (*spellL)["IconID"]); } - // Fallback to WotLK field 133 if expansion layout yielded nothing - if (spellIconIds_.empty() && fieldCount > 133) { + + // Fallback if expansion layout missing or yielded nothing + // Only use WotLK field 133 as last resort if we have no layout + if (spellIconIds_.empty() && !spellL && fieldCount > 133) { tryLoadIcons(0, 133); } } From e6741f815a0cd12b2a407fe59ad316d5a8247356 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 06:26:57 -0700 Subject: [PATCH 12/36] feat: add keybinding manager for customizable action shortcuts Implement KeybindingManager singleton class to support: - Storing and loading keybinding configuration from ini files - Querying whether an action's keybinding was pressed - Runtime rebinding of actions to different keys - Default keybinding set: C=Character, I=Inventory, S=Spellbook, K=Talents, L=Quests, M=Minimap, Esc=Settings, Enter=Chat This is the foundation for user-customizable keybindings. Integration with UI controls and replacement of hard-coded ImGui::IsKeyPressed calls will follow in subsequent improvements. --- CMakeLists.txt | 2 + include/ui/keybinding_manager.hpp | 82 ++++++++++ src/ui/keybinding_manager.cpp | 254 ++++++++++++++++++++++++++++++ 3 files changed, 338 insertions(+) create mode 100644 include/ui/keybinding_manager.hpp create mode 100644 src/ui/keybinding_manager.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 54f39283..e4c37e70 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -550,6 +550,7 @@ set(WOWEE_SOURCES src/ui/quest_log_screen.cpp src/ui/spellbook_screen.cpp src/ui/talent_screen.cpp + src/ui/keybinding_manager.cpp # Main src/main.cpp @@ -653,6 +654,7 @@ set(WOWEE_HEADERS include/ui/inventory_screen.hpp include/ui/spellbook_screen.hpp include/ui/talent_screen.hpp + include/ui/keybinding_manager.hpp ) set(WOWEE_PLATFORM_SOURCES) diff --git a/include/ui/keybinding_manager.hpp b/include/ui/keybinding_manager.hpp new file mode 100644 index 00000000..b3afcda0 --- /dev/null +++ b/include/ui/keybinding_manager.hpp @@ -0,0 +1,82 @@ +#ifndef WOWEE_KEYBINDING_MANAGER_HPP +#define WOWEE_KEYBINDING_MANAGER_HPP + +#include +#include +#include +#include + +namespace wowee::ui { + +/** + * Manages keybinding configuration for in-game actions. + * Supports loading/saving from config files and runtime rebinding. + */ +class KeybindingManager { +public: + enum class Action { + TOGGLE_CHARACTER_SCREEN, + TOGGLE_INVENTORY, + TOGGLE_SPELLBOOK, + TOGGLE_TALENTS, + TOGGLE_QUESTS, + TOGGLE_MINIMAP, + TOGGLE_SETTINGS, + TOGGLE_CHAT, + ACTION_COUNT + }; + + static KeybindingManager& getInstance(); + + /** + * Check if an action's keybinding was just pressed. + * Uses ImGui::IsKeyPressed() internally with the bound key. + */ + bool isActionPressed(Action action, bool repeat = false); + + /** + * Get the currently bound key for an action. + */ + ImGuiKey getKeyForAction(Action action) const; + + /** + * Rebind an action to a different key. + */ + void setKeyForAction(Action action, ImGuiKey key); + + /** + * Reset all keybindings to defaults. + */ + void resetToDefaults(); + + /** + * Load keybindings from config file. + */ + void loadFromConfigFile(const std::string& filePath); + + /** + * Save keybindings to config file. + */ + void saveToConfigFile(const std::string& filePath) const; + + /** + * Get human-readable name for an action. + */ + static const char* getActionName(Action action); + + /** + * Get all actions for iteration. + */ + static constexpr int getActionCount() { return static_cast(Action::ACTION_COUNT); } + +private: + KeybindingManager(); + + std::unordered_map bindings_; // action -> key + + void initializeDefaults(); +}; + +} // namespace wowee::ui + +#endif // WOWEE_KEYBINDING_MANAGER_HPP diff --git a/src/ui/keybinding_manager.cpp b/src/ui/keybinding_manager.cpp new file mode 100644 index 00000000..461d3412 --- /dev/null +++ b/src/ui/keybinding_manager.cpp @@ -0,0 +1,254 @@ +#include "ui/keybinding_manager.hpp" +#include +#include +#include + +namespace wowee::ui { + +KeybindingManager& KeybindingManager::getInstance() { + static KeybindingManager instance; + return instance; +} + +KeybindingManager::KeybindingManager() { + initializeDefaults(); +} + +void KeybindingManager::initializeDefaults() { + // Set default keybindings + bindings_[static_cast(Action::TOGGLE_CHARACTER_SCREEN)] = ImGuiKey_C; + bindings_[static_cast(Action::TOGGLE_INVENTORY)] = ImGuiKey_I; + bindings_[static_cast(Action::TOGGLE_SPELLBOOK)] = ImGuiKey_S; + bindings_[static_cast(Action::TOGGLE_TALENTS)] = ImGuiKey_K; + bindings_[static_cast(Action::TOGGLE_QUESTS)] = ImGuiKey_L; + bindings_[static_cast(Action::TOGGLE_MINIMAP)] = ImGuiKey_M; + bindings_[static_cast(Action::TOGGLE_SETTINGS)] = ImGuiKey_Escape; + bindings_[static_cast(Action::TOGGLE_CHAT)] = ImGuiKey_Enter; +} + +bool KeybindingManager::isActionPressed(Action action, bool repeat) { + auto it = bindings_.find(static_cast(action)); + if (it == bindings_.end()) return false; + return ImGui::IsKeyPressed(it->second, repeat); +} + +ImGuiKey KeybindingManager::getKeyForAction(Action action) const { + auto it = bindings_.find(static_cast(action)); + if (it == bindings_.end()) return ImGuiKey_None; + return it->second; +} + +void KeybindingManager::setKeyForAction(Action action, ImGuiKey key) { + bindings_[static_cast(action)] = key; +} + +void KeybindingManager::resetToDefaults() { + bindings_.clear(); + initializeDefaults(); +} + +const char* KeybindingManager::getActionName(Action action) { + switch (action) { + case Action::TOGGLE_CHARACTER_SCREEN: return "Character Screen"; + case Action::TOGGLE_INVENTORY: return "Inventory"; + case Action::TOGGLE_SPELLBOOK: return "Spellbook"; + case Action::TOGGLE_TALENTS: return "Talents"; + case Action::TOGGLE_QUESTS: return "Quests"; + case Action::TOGGLE_MINIMAP: return "Minimap"; + case Action::TOGGLE_SETTINGS: return "Settings"; + case Action::TOGGLE_CHAT: return "Chat"; + case Action::ACTION_COUNT: break; + } + return "Unknown"; +} + +void KeybindingManager::loadFromConfigFile(const std::string& filePath) { + std::ifstream file(filePath); + if (!file.is_open()) { + std::cerr << "[KeybindingManager] Failed to open config file: " << filePath << std::endl; + return; + } + + std::string line; + bool inKeybindingsSection = false; + + while (std::getline(file, line)) { + // Trim whitespace + size_t start = line.find_first_not_of(" \t\r\n"); + size_t end = line.find_last_not_of(" \t\r\n"); + if (start == std::string::npos) continue; + line = line.substr(start, end - start + 1); + + // Check for section header + if (line == "[Keybindings]") { + inKeybindingsSection = true; + continue; + } else if (line[0] == '[') { + inKeybindingsSection = false; + continue; + } + + if (!inKeybindingsSection || line.empty() || line[0] == ';' || line[0] == '#') continue; + + // Parse key=value pair + size_t eqPos = line.find('='); + if (eqPos == std::string::npos) continue; + + std::string action = line.substr(0, eqPos); + std::string keyStr = line.substr(eqPos + 1); + + // Trim key string + size_t kStart = keyStr.find_first_not_of(" \t"); + size_t kEnd = keyStr.find_last_not_of(" \t"); + if (kStart != std::string::npos) { + keyStr = keyStr.substr(kStart, kEnd - kStart + 1); + } + + // Map action name to enum (simplified mapping) + int actionIdx = -1; + if (action == "toggle_character_screen") actionIdx = static_cast(Action::TOGGLE_CHARACTER_SCREEN); + else if (action == "toggle_inventory") actionIdx = static_cast(Action::TOGGLE_INVENTORY); + else if (action == "toggle_spellbook") actionIdx = static_cast(Action::TOGGLE_SPELLBOOK); + else if (action == "toggle_talents") actionIdx = static_cast(Action::TOGGLE_TALENTS); + else if (action == "toggle_quests") actionIdx = static_cast(Action::TOGGLE_QUESTS); + else if (action == "toggle_minimap") actionIdx = static_cast(Action::TOGGLE_MINIMAP); + else if (action == "toggle_settings") actionIdx = static_cast(Action::TOGGLE_SETTINGS); + else if (action == "toggle_chat") actionIdx = static_cast(Action::TOGGLE_CHAT); + + if (actionIdx < 0) continue; + + // Parse key string to ImGuiKey (simple mapping of common keys) + ImGuiKey key = ImGuiKey_None; + if (keyStr.length() == 1) { + // Single character key (A-Z, 0-9) + char c = keyStr[0]; + if (c >= 'A' && c <= 'Z') { + key = static_cast(ImGuiKey_A + (c - 'A')); + } else if (c >= '0' && c <= '9') { + key = static_cast(ImGuiKey_0 + (c - '0')); + } + } else if (keyStr == "Escape") { + key = ImGuiKey_Escape; + } else if (keyStr == "Enter") { + key = ImGuiKey_Enter; + } else if (keyStr == "Tab") { + key = ImGuiKey_Tab; + } else if (keyStr == "Backspace") { + key = ImGuiKey_Backspace; + } else if (keyStr == "Space") { + key = ImGuiKey_Space; + } else if (keyStr == "Delete") { + key = ImGuiKey_Delete; + } else if (keyStr == "Home") { + key = ImGuiKey_Home; + } else if (keyStr == "End") { + key = ImGuiKey_End; + } else if (keyStr.find("F") == 0 && keyStr.length() <= 3) { + // F1-F12 keys + int fNum = std::stoi(keyStr.substr(1)); + if (fNum >= 1 && fNum <= 12) { + key = static_cast(ImGuiKey_F1 + (fNum - 1)); + } + } + + if (key != ImGuiKey_None) { + bindings_[actionIdx] = key; + } + } + + file.close(); + std::cout << "[KeybindingManager] Loaded keybindings from " << filePath << std::endl; +} + +void KeybindingManager::saveToConfigFile(const std::string& filePath) const { + std::ifstream inFile(filePath); + std::string content; + std::string line; + + // Read existing file, removing [Keybindings] section if it exists + bool inKeybindingsSection = false; + if (inFile.is_open()) { + while (std::getline(inFile, line)) { + if (line == "[Keybindings]") { + inKeybindingsSection = true; + continue; + } else if (line[0] == '[') { + inKeybindingsSection = false; + } + + if (!inKeybindingsSection) { + content += line + "\n"; + } + } + inFile.close(); + } + + // Append new Keybindings section + content += "[Keybindings]\n"; + + static const struct { + Action action; + const char* name; + } actionMap[] = { + {Action::TOGGLE_CHARACTER_SCREEN, "toggle_character_screen"}, + {Action::TOGGLE_INVENTORY, "toggle_inventory"}, + {Action::TOGGLE_SPELLBOOK, "toggle_spellbook"}, + {Action::TOGGLE_TALENTS, "toggle_talents"}, + {Action::TOGGLE_QUESTS, "toggle_quests"}, + {Action::TOGGLE_MINIMAP, "toggle_minimap"}, + {Action::TOGGLE_SETTINGS, "toggle_settings"}, + {Action::TOGGLE_CHAT, "toggle_chat"}, + }; + + for (const auto& [action, nameStr] : actionMap) { + auto it = bindings_.find(static_cast(action)); + if (it == bindings_.end()) continue; + + ImGuiKey key = it->second; + std::string keyStr; + + // Convert ImGuiKey to string + if (key >= ImGuiKey_A && key <= ImGuiKey_Z) { + keyStr += static_cast('A' + (key - ImGuiKey_A)); + } else if (key >= ImGuiKey_0 && key <= ImGuiKey_9) { + keyStr += static_cast('0' + (key - ImGuiKey_0)); + } else if (key == ImGuiKey_Escape) { + keyStr = "Escape"; + } else if (key == ImGuiKey_Enter) { + keyStr = "Enter"; + } else if (key == ImGuiKey_Tab) { + keyStr = "Tab"; + } else if (key == ImGuiKey_Backspace) { + keyStr = "Backspace"; + } else if (key == ImGuiKey_Space) { + keyStr = "Space"; + } else if (key == ImGuiKey_Delete) { + keyStr = "Delete"; + } else if (key == ImGuiKey_Home) { + keyStr = "Home"; + } else if (key == ImGuiKey_End) { + keyStr = "End"; + } else if (key >= ImGuiKey_F1 && key <= ImGuiKey_F12) { + keyStr = "F" + std::to_string(1 + (key - ImGuiKey_F1)); + } + + if (!keyStr.empty()) { + content += nameStr; + content += "="; + content += keyStr; + content += "\n"; + } + } + + // Write back to file + std::ofstream outFile(filePath); + if (outFile.is_open()) { + outFile << content; + outFile.close(); + std::cout << "[KeybindingManager] Saved keybindings to " << filePath << std::endl; + } else { + std::cerr << "[KeybindingManager] Failed to write config file: " << filePath << std::endl; + } +} + +} // namespace wowee::ui From f7a79b436e1e7714199dadd744e30b035782b9ca Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 06:51:48 -0700 Subject: [PATCH 13/36] feat: integrate keybinding customization UI into Settings window - Extended KeybindingManager enum with TOGGLE_GUILD_ROSTER (O) and TOGGLE_DUNGEON_FINDER (J) to replace hard-coded key checks - Added Controls tab in Settings UI for rebinding all 10 customizable actions - Implemented real-time key capture and binding with visual feedback - Integrated keybinding persistence with main settings.cfg file - Replaced hard-coded O key (Guild Roster) and I key (Dungeon Finder) checks with KeybindingManager::isActionPressed() calls - Added Reset to Defaults button for restoring original keybindings --- include/ui/game_screen.hpp | 5 ++ include/ui/keybinding_manager.hpp | 2 + src/ui/game_screen.cpp | 119 +++++++++++++++++++++++++++++- src/ui/keybinding_manager.cpp | 8 ++ 4 files changed, 130 insertions(+), 4 deletions(-) diff --git a/include/ui/game_screen.hpp b/include/ui/game_screen.hpp index 655b20cb..12154c06 100644 --- a/include/ui/game_screen.hpp +++ b/include/ui/game_screen.hpp @@ -8,6 +8,7 @@ #include "ui/quest_log_screen.hpp" #include "ui/spellbook_screen.hpp" #include "ui/talent_screen.hpp" +#include "ui/keybinding_manager.hpp" #include #include #include @@ -111,6 +112,10 @@ private: bool pendingMinimapNpcDots = false; bool pendingSeparateBags = true; bool pendingAutoLoot = false; + + // Keybinding customization + int pendingRebindAction = -1; // -1 = not rebinding, otherwise action index + bool awaitingKeyPress = false; bool pendingUseOriginalSoundtrack = true; bool pendingShowActionBar2 = true; // Show second action bar above main bar float pendingActionBar2OffsetX = 0.0f; // Horizontal offset from default center position diff --git a/include/ui/keybinding_manager.hpp b/include/ui/keybinding_manager.hpp index b3afcda0..e4987798 100644 --- a/include/ui/keybinding_manager.hpp +++ b/include/ui/keybinding_manager.hpp @@ -23,6 +23,8 @@ public: TOGGLE_MINIMAP, TOGGLE_SETTINGS, TOGGLE_CHAT, + TOGGLE_GUILD_ROSTER, + TOGGLE_DUNGEON_FINDER, ACTION_COUNT }; diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index a139f15f..c228bc70 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -6402,8 +6402,8 @@ void GameScreen::renderLfgProposalPopup(game::GameHandler& gameHandler) { } void GameScreen::renderGuildRoster(game::GameHandler& gameHandler) { - // O key toggle (WoW default Social/Guild keybind) - if (!ImGui::GetIO().WantCaptureKeyboard && ImGui::IsKeyPressed(ImGuiKey_O)) { + // Guild Roster toggle (customizable keybind) + if (!ImGui::GetIO().WantCaptureKeyboard && KeybindingManager::getInstance().isActionPressed(KeybindingManager::Action::TOGGLE_GUILD_ROSTER)) { showGuildRoster_ = !showGuildRoster_; if (showGuildRoster_) { // Open friends tab directly if not in guild @@ -9180,6 +9180,108 @@ void GameScreen::renderSettingsWindow() { ImGui::EndTabItem(); } + // ============================================================ + // CONTROLS TAB + // ============================================================ + if (ImGui::BeginTabItem("Controls")) { + ImGui::Spacing(); + + ImGui::Text("Keybindings"); + ImGui::Separator(); + + auto& km = ui::KeybindingManager::getInstance(); + int numActions = km.getActionCount(); + + for (int i = 0; i < numActions; ++i) { + auto action = static_cast(i); + const char* actionName = km.getActionName(action); + ImGuiKey currentKey = km.getKeyForAction(action); + + // Display current binding + ImGui::Text("%s:", actionName); + ImGui::SameLine(200); + + // Get human-readable key name (basic implementation) + const char* keyName = "Unknown"; + if (currentKey >= ImGuiKey_A && currentKey <= ImGuiKey_Z) { + static char keyBuf[16]; + snprintf(keyBuf, sizeof(keyBuf), "%c", 'A' + (currentKey - ImGuiKey_A)); + keyName = keyBuf; + } else if (currentKey >= ImGuiKey_0 && currentKey <= ImGuiKey_9) { + static char keyBuf[16]; + snprintf(keyBuf, sizeof(keyBuf), "%c", '0' + (currentKey - ImGuiKey_0)); + keyName = keyBuf; + } else if (currentKey == ImGuiKey_Escape) { + keyName = "Escape"; + } else if (currentKey == ImGuiKey_Enter) { + keyName = "Enter"; + } else if (currentKey == ImGuiKey_Tab) { + keyName = "Tab"; + } else if (currentKey == ImGuiKey_Space) { + keyName = "Space"; + } else if (currentKey >= ImGuiKey_F1 && currentKey <= ImGuiKey_F12) { + static char keyBuf[16]; + snprintf(keyBuf, sizeof(keyBuf), "F%d", 1 + (currentKey - ImGuiKey_F1)); + keyName = keyBuf; + } + + ImGui::Text("[%s]", keyName); + + // Rebind button + ImGui::SameLine(350); + if (ImGui::Button(awaitingKeyPress && pendingRebindAction == i ? "Waiting..." : "Rebind", ImVec2(100, 0))) { + pendingRebindAction = i; + awaitingKeyPress = true; + } + } + + // Handle key press during rebinding + if (awaitingKeyPress && pendingRebindAction >= 0) { + ImGui::Spacing(); + ImGui::Separator(); + ImGui::Text("Press any key to bind to this action (Esc to cancel)..."); + + // Check for any key press + bool foundKey = false; + ImGuiKey newKey = ImGuiKey_None; + for (int k = ImGuiKey_NamedKey_BEGIN; k < ImGuiKey_NamedKey_END; ++k) { + if (ImGui::IsKeyPressed(static_cast(k), false)) { + if (k == ImGuiKey_Escape) { + // Cancel rebinding + awaitingKeyPress = false; + pendingRebindAction = -1; + foundKey = true; + break; + } + newKey = static_cast(k); + foundKey = true; + break; + } + } + + if (foundKey && newKey != ImGuiKey_None) { + auto action = static_cast(pendingRebindAction); + km.setKeyForAction(action, newKey); + awaitingKeyPress = false; + pendingRebindAction = -1; + saveSettings(); + } + } + + ImGui::Spacing(); + ImGui::Separator(); + ImGui::Spacing(); + + if (ImGui::Button("Reset to Defaults", ImVec2(-1, 0))) { + km.resetToDefaults(); + awaitingKeyPress = false; + pendingRebindAction = -1; + saveSettings(); + } + + ImGui::EndTabItem(); + } + // ============================================================ // CHAT TAB // ============================================================ @@ -10063,6 +10165,11 @@ void GameScreen::saveSettings() { out << "chat_autojoin_lfg=" << (chatAutoJoinLFG_ ? 1 : 0) << "\n"; out << "chat_autojoin_local=" << (chatAutoJoinLocal_ ? 1 : 0) << "\n"; + out.close(); + + // Save keybindings to the same config file (appends [Keybindings] section) + KeybindingManager::getInstance().saveToConfigFile(path); + LOG_INFO("Settings saved to ", path); } @@ -10176,6 +10283,10 @@ void GameScreen::loadSettings() { else if (key == "chat_autojoin_local") chatAutoJoinLocal_ = (std::stoi(val) != 0); } catch (...) {} } + + // Load keybindings from the same config file + KeybindingManager::getInstance().loadFromConfigFile(path); + LOG_INFO("Settings loaded from ", path); } @@ -11551,8 +11662,8 @@ void GameScreen::renderZoneText() { // Dungeon Finder window (toggle with hotkey or bag-bar button) // --------------------------------------------------------------------------- void GameScreen::renderDungeonFinderWindow(game::GameHandler& gameHandler) { - // Toggle on I key when not typing - if (!chatInputActive && ImGui::IsKeyPressed(ImGuiKey_I, false)) { + // Toggle Dungeon Finder (customizable keybind) + if (!chatInputActive && KeybindingManager::getInstance().isActionPressed(KeybindingManager::Action::TOGGLE_DUNGEON_FINDER)) { showDungeonFinder_ = !showDungeonFinder_; } diff --git a/src/ui/keybinding_manager.cpp b/src/ui/keybinding_manager.cpp index 461d3412..ff04cc58 100644 --- a/src/ui/keybinding_manager.cpp +++ b/src/ui/keybinding_manager.cpp @@ -24,6 +24,8 @@ void KeybindingManager::initializeDefaults() { bindings_[static_cast(Action::TOGGLE_MINIMAP)] = ImGuiKey_M; bindings_[static_cast(Action::TOGGLE_SETTINGS)] = ImGuiKey_Escape; bindings_[static_cast(Action::TOGGLE_CHAT)] = ImGuiKey_Enter; + bindings_[static_cast(Action::TOGGLE_GUILD_ROSTER)] = ImGuiKey_O; + bindings_[static_cast(Action::TOGGLE_DUNGEON_FINDER)] = ImGuiKey_J; // Originally I, reassigned to avoid conflict } bool KeybindingManager::isActionPressed(Action action, bool repeat) { @@ -57,6 +59,8 @@ const char* KeybindingManager::getActionName(Action action) { case Action::TOGGLE_MINIMAP: return "Minimap"; case Action::TOGGLE_SETTINGS: return "Settings"; case Action::TOGGLE_CHAT: return "Chat"; + case Action::TOGGLE_GUILD_ROSTER: return "Guild Roster / Social"; + case Action::TOGGLE_DUNGEON_FINDER: return "Dungeon Finder"; case Action::ACTION_COUNT: break; } return "Unknown"; @@ -114,6 +118,8 @@ void KeybindingManager::loadFromConfigFile(const std::string& filePath) { else if (action == "toggle_minimap") actionIdx = static_cast(Action::TOGGLE_MINIMAP); else if (action == "toggle_settings") actionIdx = static_cast(Action::TOGGLE_SETTINGS); else if (action == "toggle_chat") actionIdx = static_cast(Action::TOGGLE_CHAT); + else if (action == "toggle_guild_roster") actionIdx = static_cast(Action::TOGGLE_GUILD_ROSTER); + else if (action == "toggle_dungeon_finder") actionIdx = static_cast(Action::TOGGLE_DUNGEON_FINDER); if (actionIdx < 0) continue; @@ -198,6 +204,8 @@ void KeybindingManager::saveToConfigFile(const std::string& filePath) const { {Action::TOGGLE_MINIMAP, "toggle_minimap"}, {Action::TOGGLE_SETTINGS, "toggle_settings"}, {Action::TOGGLE_CHAT, "toggle_chat"}, + {Action::TOGGLE_GUILD_ROSTER, "toggle_guild_roster"}, + {Action::TOGGLE_DUNGEON_FINDER, "toggle_dungeon_finder"}, }; for (const auto& [action, nameStr] : actionMap) { From 0d9404c704385c0e6b1cffc3364ba1b0acc4bbae Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 07:19:54 -0700 Subject: [PATCH 14/36] feat: expand keybinding system with 4 new customizable actions - Add World Map (W), Nameplates (V), Raid Frames (R), Quest Log (Q) to KeybindingManager enum with customizable default bindings - Replace hard-coded V key check for nameplate toggle with KeybindingManager::isActionPressed() to support customization - Update config file persistence to handle new bindings - Infrastructure in place for implementing visibility toggles on other windows (World Map, Raid Frames, Quest Log) with future UI refactoring --- include/ui/keybinding_manager.hpp | 4 ++++ src/ui/game_screen.cpp | 4 ++-- src/ui/keybinding_manager.cpp | 16 ++++++++++++++++ 3 files changed, 22 insertions(+), 2 deletions(-) diff --git a/include/ui/keybinding_manager.hpp b/include/ui/keybinding_manager.hpp index e4987798..098505c1 100644 --- a/include/ui/keybinding_manager.hpp +++ b/include/ui/keybinding_manager.hpp @@ -25,6 +25,10 @@ public: TOGGLE_CHAT, TOGGLE_GUILD_ROSTER, TOGGLE_DUNGEON_FINDER, + TOGGLE_WORLD_MAP, + TOGGLE_NAMEPLATES, + TOGGLE_RAID_FRAMES, + TOGGLE_QUEST_LOG, ACTION_COUNT }; diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index c228bc70..20e8769d 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -1470,8 +1470,8 @@ void GameScreen::processTargetInput(game::GameHandler& gameHandler) { } } - // V — toggle nameplates (WoW default keybinding) - if (input.isKeyJustPressed(SDL_SCANCODE_V)) { + // Toggle nameplates (customizable keybinding, default V) + if (KeybindingManager::getInstance().isActionPressed(KeybindingManager::Action::TOGGLE_NAMEPLATES)) { showNameplates_ = !showNameplates_; } diff --git a/src/ui/keybinding_manager.cpp b/src/ui/keybinding_manager.cpp index ff04cc58..bf65ae56 100644 --- a/src/ui/keybinding_manager.cpp +++ b/src/ui/keybinding_manager.cpp @@ -26,6 +26,10 @@ void KeybindingManager::initializeDefaults() { bindings_[static_cast(Action::TOGGLE_CHAT)] = ImGuiKey_Enter; bindings_[static_cast(Action::TOGGLE_GUILD_ROSTER)] = ImGuiKey_O; bindings_[static_cast(Action::TOGGLE_DUNGEON_FINDER)] = ImGuiKey_J; // Originally I, reassigned to avoid conflict + bindings_[static_cast(Action::TOGGLE_WORLD_MAP)] = ImGuiKey_W; + bindings_[static_cast(Action::TOGGLE_NAMEPLATES)] = ImGuiKey_V; + bindings_[static_cast(Action::TOGGLE_RAID_FRAMES)] = ImGuiKey_R; + bindings_[static_cast(Action::TOGGLE_QUEST_LOG)] = ImGuiKey_Q; } bool KeybindingManager::isActionPressed(Action action, bool repeat) { @@ -61,6 +65,10 @@ const char* KeybindingManager::getActionName(Action action) { case Action::TOGGLE_CHAT: return "Chat"; case Action::TOGGLE_GUILD_ROSTER: return "Guild Roster / Social"; case Action::TOGGLE_DUNGEON_FINDER: return "Dungeon Finder"; + case Action::TOGGLE_WORLD_MAP: return "World Map"; + case Action::TOGGLE_NAMEPLATES: return "Nameplates"; + case Action::TOGGLE_RAID_FRAMES: return "Raid Frames"; + case Action::TOGGLE_QUEST_LOG: return "Quest Log"; case Action::ACTION_COUNT: break; } return "Unknown"; @@ -120,6 +128,10 @@ void KeybindingManager::loadFromConfigFile(const std::string& filePath) { else if (action == "toggle_chat") actionIdx = static_cast(Action::TOGGLE_CHAT); else if (action == "toggle_guild_roster") actionIdx = static_cast(Action::TOGGLE_GUILD_ROSTER); else if (action == "toggle_dungeon_finder") actionIdx = static_cast(Action::TOGGLE_DUNGEON_FINDER); + else if (action == "toggle_world_map") actionIdx = static_cast(Action::TOGGLE_WORLD_MAP); + else if (action == "toggle_nameplates") actionIdx = static_cast(Action::TOGGLE_NAMEPLATES); + else if (action == "toggle_raid_frames") actionIdx = static_cast(Action::TOGGLE_RAID_FRAMES); + else if (action == "toggle_quest_log") actionIdx = static_cast(Action::TOGGLE_QUEST_LOG); if (actionIdx < 0) continue; @@ -206,6 +218,10 @@ void KeybindingManager::saveToConfigFile(const std::string& filePath) const { {Action::TOGGLE_CHAT, "toggle_chat"}, {Action::TOGGLE_GUILD_ROSTER, "toggle_guild_roster"}, {Action::TOGGLE_DUNGEON_FINDER, "toggle_dungeon_finder"}, + {Action::TOGGLE_WORLD_MAP, "toggle_world_map"}, + {Action::TOGGLE_NAMEPLATES, "toggle_nameplates"}, + {Action::TOGGLE_RAID_FRAMES, "toggle_raid_frames"}, + {Action::TOGGLE_QUEST_LOG, "toggle_quest_log"}, }; for (const auto& [action, nameStr] : actionMap) { From 3092d406fa8e099f9033f942bf328672acf1e3cf Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 07:24:01 -0700 Subject: [PATCH 15/36] fix: enable NPC tabard geosets for proper equipment rendering Enable tabard mesh rendering for NPCs by reading geoset variant from ItemDisplayInfo.dbc (slot 9). Tabards now render like other equipment instead of being disabled due to the previous flickering issue. --- src/core/application.cpp | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/core/application.cpp b/src/core/application.cpp index b04a5269..3b8e9cbe 100644 --- a/src/core/application.cpp +++ b/src/core/application.cpp @@ -5973,7 +5973,7 @@ void Application::spawnOnlineCreature(uint64_t guid, uint32_t displayId, float x uint16_t geosetSleeves = pickGeoset(801, 8); // Bare wrists (group 8, controlled by chest) uint16_t geosetPants = pickGeoset(1301, 13); // Bare legs (group 13) uint16_t geosetCape = 0; // Group 15 disabled unless cape is equipped - uint16_t geosetTabard = 0; // TODO: NPC tabard geosets currently flicker/apron; keep hidden for now + uint16_t geosetTabard = pickGeoset(1201, 12); // Group 12 (tabard), default variant 1201 rendering::VkTexture* npcCapeTextureId = nullptr; // Load equipment geosets from ItemDisplayInfo.dbc @@ -6022,7 +6022,11 @@ void Application::spawnOnlineCreature(uint64_t guid, uint32_t displayId, float x if (gg > 0) geosetGloves = pickGeoset(static_cast(301 + gg), 3); } - // Tabard (slot 9) intentionally disabled for now (see geosetTabard TODO above). + // Tabard (slot 9) → group 12 (tabard/robe mesh) + { + uint32_t gg = readGeosetGroup(9, "tabard"); + if (gg > 0) geosetTabard = pickGeoset(static_cast(1200 + gg), 12); + } // Cape (slot 10) → group 15 if (extra.equipDisplayId[10] != 0) { From a3e0d36a722f2bd415387451f8f7de44658d6685 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 07:38:08 -0700 Subject: [PATCH 16/36] feat: add World Map visibility toggle with keybinding support Implement showWorldMap_ state variable and TOGGLE_WORLD_MAP keybinding integration to allow players to customize the W key binding for opening/ closing the World Map, consistent with other window toggles like Nameplates (V key) and Guild Roster (O key). --- include/ui/game_screen.hpp | 1 + src/ui/game_screen.cpp | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/include/ui/game_screen.hpp b/include/ui/game_screen.hpp index 12154c06..125dc31b 100644 --- a/include/ui/game_screen.hpp +++ b/include/ui/game_screen.hpp @@ -67,6 +67,7 @@ private: bool showPlayerInfo = false; bool showSocialFrame_ = false; // O key toggles social/friends list bool showGuildRoster_ = false; + bool showWorldMap_ = false; // W key toggles world map std::string selectedGuildMember_; bool showGuildNoteEdit_ = false; bool editingOfficerNote_ = false; diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index 20e8769d..5da57eda 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -1475,6 +1475,10 @@ void GameScreen::processTargetInput(game::GameHandler& gameHandler) { showNameplates_ = !showNameplates_; } + if (KeybindingManager::getInstance().isActionPressed(KeybindingManager::Action::TOGGLE_WORLD_MAP)) { + showWorldMap_ = !showWorldMap_; + } + // Action bar keys (1-9, 0, -, =) static const SDL_Scancode actionBarKeys[] = { SDL_SCANCODE_1, SDL_SCANCODE_2, SDL_SCANCODE_3, SDL_SCANCODE_4, @@ -4003,6 +4007,8 @@ void GameScreen::updateCharacterTextures(game::Inventory& inventory) { // ============================================================ void GameScreen::renderWorldMap(game::GameHandler& gameHandler) { + if (!showWorldMap_) return; + auto& app = core::Application::getInstance(); auto* renderer = app.getRenderer(); if (!renderer) return; From a8fd977a53a7f20b524c1c3de97ff2b97d0f3d9c Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 07:53:36 -0700 Subject: [PATCH 17/36] feat: re-enable R key for camera reset with chat input safeguard Allow R key to reset camera position/rotation when chat input is not active. Previously disabled due to conflict with chat reply command. Now uses the same safety check as movement keys (ImGui::GetIO().WantTextInput). Implements edge-triggered reset on R key press, matching X key (sit) pattern. --- src/rendering/camera_controller.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/rendering/camera_controller.cpp b/src/rendering/camera_controller.cpp index 77908f3a..50872d46 100644 --- a/src/rendering/camera_controller.cpp +++ b/src/rendering/camera_controller.cpp @@ -377,6 +377,13 @@ void CameraController::update(float deltaTime) { if (mounted_) sitting = false; xKeyWasDown = xDown; + // Reset camera with R key (edge-triggered) — only when UI doesn't want keyboard + bool rDown = !uiWantsKeyboard && input.isKeyPressed(SDL_SCANCODE_R); + if (rDown && !rKeyWasDown) { + reset(); + } + rKeyWasDown = rDown; + // Stand up on any movement key or jump while sitting (WoW behaviour) if (!uiWantsKeyboard && sitting && !movementSuppressed) { bool anyMoveKey = @@ -1851,8 +1858,7 @@ void CameraController::update(float deltaTime) { wasJumping = nowJump; wasFalling = !grounded && verticalVelocity <= 0.0f; - // R key disabled — was camera reset, conflicts with chat reply - rKeyWasDown = false; + // R key is now handled above with chat safeguard (WantTextInput check) } void CameraController::processMouseMotion(const SDL_MouseMotionEvent& event) { From 9809106a849ccbde52fa8a0ed1ed2da689542703 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 08:09:55 -0700 Subject: [PATCH 18/36] fix: resolve keybinding conflict - reassign TOGGLE_RAID_FRAMES from R to F The R key was previously assigned to TOGGLE_RAID_FRAMES in the keybinding manager but was never actually implemented (raid frames had no visibility toggle). Loop 10 implemented R for camera reset, creating a conflict. Reassign TOGGLE_RAID_FRAMES to F (an unused key) to prevent the conflict. This aligns with the intention that R is now the standard camera reset key. --- src/ui/keybinding_manager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/keybinding_manager.cpp b/src/ui/keybinding_manager.cpp index bf65ae56..e4d63e0f 100644 --- a/src/ui/keybinding_manager.cpp +++ b/src/ui/keybinding_manager.cpp @@ -28,7 +28,7 @@ void KeybindingManager::initializeDefaults() { bindings_[static_cast(Action::TOGGLE_DUNGEON_FINDER)] = ImGuiKey_J; // Originally I, reassigned to avoid conflict bindings_[static_cast(Action::TOGGLE_WORLD_MAP)] = ImGuiKey_W; bindings_[static_cast(Action::TOGGLE_NAMEPLATES)] = ImGuiKey_V; - bindings_[static_cast(Action::TOGGLE_RAID_FRAMES)] = ImGuiKey_R; + bindings_[static_cast(Action::TOGGLE_RAID_FRAMES)] = ImGuiKey_F; // Reassigned from R (now camera reset) bindings_[static_cast(Action::TOGGLE_QUEST_LOG)] = ImGuiKey_Q; } From 82d00c94c040904d9f76532b93c1d494a473bd70 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 08:28:34 -0700 Subject: [PATCH 19/36] refactor: use keybinding manager for quest log toggle instead of hardcoded L key The quest log screen was using a hardcoded SDL_SCANCODE_L key check instead of the keybinding manager system, preventing users from customizing the keybinding. Update to use KeybindingManager::Action::TOGGLE_QUESTS (bound to L by default), allowing users to customize the quest log toggle key through the Settings UI while maintaining the default WoW key binding. This enables consistency with other customizable window toggles that already use the keybinding system (Character Screen, Inventory, Spellbook, World Map, etc.). --- src/ui/quest_log_screen.cpp | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/ui/quest_log_screen.cpp b/src/ui/quest_log_screen.cpp index 8a9ddd55..a5dc4945 100644 --- a/src/ui/quest_log_screen.cpp +++ b/src/ui/quest_log_screen.cpp @@ -1,4 +1,5 @@ #include "ui/quest_log_screen.hpp" +#include "ui/keybinding_manager.hpp" #include "core/application.hpp" #include "core/input.hpp" #include @@ -206,13 +207,14 @@ std::string cleanQuestTitleForUi(const std::string& raw, uint32_t questId) { } // anonymous namespace void QuestLogScreen::render(game::GameHandler& gameHandler) { - // L key toggle (edge-triggered) - ImGuiIO& io = ImGui::GetIO(); - bool lDown = !io.WantTextInput && core::Input::getInstance().isKeyPressed(SDL_SCANCODE_L); - if (lDown && !lKeyWasDown) { + // Quests toggle via keybinding (edge-triggered) + // Customizable key (default: L) from KeybindingManager + bool questsDown = KeybindingManager::getInstance().isActionPressed( + KeybindingManager::Action::TOGGLE_QUESTS, false); + if (questsDown && !lKeyWasDown) { open = !open; } - lKeyWasDown = lDown; + lKeyWasDown = questsDown; if (!open) return; From 46365f4738ac2afcf735a0ef931c3952fc06f6e6 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 08:42:58 -0700 Subject: [PATCH 20/36] fix: correct keybinding defaults to match WoW standard keys The keybinding manager had incorrect default key assignments: - TOGGLE_SPELLBOOK was S (should be P - WoW standard) - TOGGLE_TALENTS was K (should be N - WoW standard) These mismatched the actual hardcoded keys in spellbook_screen.cpp (P) and talent_screen.cpp (N), as well as user expectations from standard WoW. Update keybinding defaults to align with WoW conventions and the actual UI implementations that are using these keys. --- src/ui/keybinding_manager.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/keybinding_manager.cpp b/src/ui/keybinding_manager.cpp index e4d63e0f..bb2d1021 100644 --- a/src/ui/keybinding_manager.cpp +++ b/src/ui/keybinding_manager.cpp @@ -18,8 +18,8 @@ void KeybindingManager::initializeDefaults() { // Set default keybindings bindings_[static_cast(Action::TOGGLE_CHARACTER_SCREEN)] = ImGuiKey_C; bindings_[static_cast(Action::TOGGLE_INVENTORY)] = ImGuiKey_I; - bindings_[static_cast(Action::TOGGLE_SPELLBOOK)] = ImGuiKey_S; - bindings_[static_cast(Action::TOGGLE_TALENTS)] = ImGuiKey_K; + bindings_[static_cast(Action::TOGGLE_SPELLBOOK)] = ImGuiKey_P; // WoW standard key + bindings_[static_cast(Action::TOGGLE_TALENTS)] = ImGuiKey_N; // WoW standard key bindings_[static_cast(Action::TOGGLE_QUESTS)] = ImGuiKey_L; bindings_[static_cast(Action::TOGGLE_MINIMAP)] = ImGuiKey_M; bindings_[static_cast(Action::TOGGLE_SETTINGS)] = ImGuiKey_Escape; From 7220737d48646baac73bea6c8c46c7012dee3fb3 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 08:58:20 -0700 Subject: [PATCH 21/36] refactor: use keybinding manager for spellbook and talents toggles instead of hardcoded keys --- src/ui/spellbook_screen.cpp | 12 +++++++----- src/ui/talent_screen.cpp | 12 +++++++----- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/src/ui/spellbook_screen.cpp b/src/ui/spellbook_screen.cpp index 0a355ff3..ef8815f5 100644 --- a/src/ui/spellbook_screen.cpp +++ b/src/ui/spellbook_screen.cpp @@ -1,4 +1,5 @@ #include "ui/spellbook_screen.hpp" +#include "ui/keybinding_manager.hpp" #include "core/input.hpp" #include "core/application.hpp" #include "rendering/vk_context.hpp" @@ -563,13 +564,14 @@ void SpellbookScreen::renderSpellTooltip(const SpellInfo* info, game::GameHandle } void SpellbookScreen::render(game::GameHandler& gameHandler, pipeline::AssetManager* assetManager) { - // P key toggle (edge-triggered) - bool wantsTextInput = ImGui::GetIO().WantTextInput; - bool pDown = !wantsTextInput && core::Input::getInstance().isKeyPressed(SDL_SCANCODE_P); - if (pDown && !pKeyWasDown) { + // Spellbook toggle via keybinding (edge-triggered) + // Customizable key (default: P) from KeybindingManager + bool spellbookDown = KeybindingManager::getInstance().isActionPressed( + KeybindingManager::Action::TOGGLE_SPELLBOOK, false); + if (spellbookDown && !pKeyWasDown) { open = !open; } - pKeyWasDown = pDown; + pKeyWasDown = spellbookDown; if (!open) return; diff --git a/src/ui/talent_screen.cpp b/src/ui/talent_screen.cpp index eeff7c41..3a487d5d 100644 --- a/src/ui/talent_screen.cpp +++ b/src/ui/talent_screen.cpp @@ -1,4 +1,5 @@ #include "ui/talent_screen.hpp" +#include "ui/keybinding_manager.hpp" #include "core/input.hpp" #include "core/application.hpp" #include "core/logger.hpp" @@ -22,13 +23,14 @@ static const char* getClassName(uint8_t classId) { } void TalentScreen::render(game::GameHandler& gameHandler) { - // N key toggle (edge-triggered) - bool wantsTextInput = ImGui::GetIO().WantTextInput; - bool nDown = !wantsTextInput && core::Input::getInstance().isKeyPressed(SDL_SCANCODE_N); - if (nDown && !nKeyWasDown) { + // Talents toggle via keybinding (edge-triggered) + // Customizable key (default: N) from KeybindingManager + bool talentsDown = KeybindingManager::getInstance().isActionPressed( + KeybindingManager::Action::TOGGLE_TALENTS, false); + if (talentsDown && !nKeyWasDown) { open = !open; } - nKeyWasDown = nDown; + nKeyWasDown = talentsDown; if (!open) return; From 332c2f6d3f5fee0b2cbd7c1be947f075c9f13e56 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 09:02:15 -0700 Subject: [PATCH 22/36] feat: add TOGGLE_BAGS action and integrate inventory screen with keybinding manager - Add TOGGLE_BAGS action to keybinding manager (B key default) - Update inventory_screen.cpp to use keybinding manager for bag and character toggles - Maintain consistent keybinding system across all UI windows --- include/ui/keybinding_manager.hpp | 1 + src/ui/inventory_screen.cpp | 22 +++++++++++++--------- src/ui/keybinding_manager.cpp | 4 ++++ 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/include/ui/keybinding_manager.hpp b/include/ui/keybinding_manager.hpp index 098505c1..09c9ac05 100644 --- a/include/ui/keybinding_manager.hpp +++ b/include/ui/keybinding_manager.hpp @@ -17,6 +17,7 @@ public: enum class Action { TOGGLE_CHARACTER_SCREEN, TOGGLE_INVENTORY, + TOGGLE_BAGS, TOGGLE_SPELLBOOK, TOGGLE_TALENTS, TOGGLE_QUESTS, diff --git a/src/ui/inventory_screen.cpp b/src/ui/inventory_screen.cpp index 0bb2c8c3..6255601f 100644 --- a/src/ui/inventory_screen.cpp +++ b/src/ui/inventory_screen.cpp @@ -1,4 +1,5 @@ #include "ui/inventory_screen.hpp" +#include "ui/keybinding_manager.hpp" #include "game/game_handler.hpp" #include "core/application.hpp" #include "rendering/vk_context.hpp" @@ -709,18 +710,21 @@ bool InventoryScreen::bagHasAnyItems(const game::Inventory& inventory, int bagIn } void InventoryScreen::render(game::Inventory& inventory, uint64_t moneyCopper) { - // B key toggle (edge-triggered) - bool wantsTextInput = ImGui::GetIO().WantTextInput; - bool bDown = !wantsTextInput && core::Input::getInstance().isKeyPressed(SDL_SCANCODE_B); - bool bToggled = bDown && !bKeyWasDown; - bKeyWasDown = bDown; + // Bags toggle (B key, edge-triggered) + bool bagsDown = KeybindingManager::getInstance().isActionPressed( + KeybindingManager::Action::TOGGLE_BAGS, false); + bool bToggled = bagsDown && !bKeyWasDown; + bKeyWasDown = bagsDown; - // C key toggle for character screen (edge-triggered) - bool cDown = !wantsTextInput && core::Input::getInstance().isKeyPressed(SDL_SCANCODE_C); - if (cDown && !cKeyWasDown) { + // Character screen toggle (C key, edge-triggered) + bool characterDown = KeybindingManager::getInstance().isActionPressed( + KeybindingManager::Action::TOGGLE_CHARACTER_SCREEN, false); + if (characterDown && !cKeyWasDown) { characterOpen = !characterOpen; } - cKeyWasDown = cDown; + cKeyWasDown = characterDown; + + bool wantsTextInput = ImGui::GetIO().WantTextInput; if (separateBags_) { if (bToggled) { diff --git a/src/ui/keybinding_manager.cpp b/src/ui/keybinding_manager.cpp index bb2d1021..212d2af0 100644 --- a/src/ui/keybinding_manager.cpp +++ b/src/ui/keybinding_manager.cpp @@ -18,6 +18,7 @@ void KeybindingManager::initializeDefaults() { // Set default keybindings bindings_[static_cast(Action::TOGGLE_CHARACTER_SCREEN)] = ImGuiKey_C; bindings_[static_cast(Action::TOGGLE_INVENTORY)] = ImGuiKey_I; + bindings_[static_cast(Action::TOGGLE_BAGS)] = ImGuiKey_B; bindings_[static_cast(Action::TOGGLE_SPELLBOOK)] = ImGuiKey_P; // WoW standard key bindings_[static_cast(Action::TOGGLE_TALENTS)] = ImGuiKey_N; // WoW standard key bindings_[static_cast(Action::TOGGLE_QUESTS)] = ImGuiKey_L; @@ -57,6 +58,7 @@ const char* KeybindingManager::getActionName(Action action) { switch (action) { case Action::TOGGLE_CHARACTER_SCREEN: return "Character Screen"; case Action::TOGGLE_INVENTORY: return "Inventory"; + case Action::TOGGLE_BAGS: return "Bags"; case Action::TOGGLE_SPELLBOOK: return "Spellbook"; case Action::TOGGLE_TALENTS: return "Talents"; case Action::TOGGLE_QUESTS: return "Quests"; @@ -120,6 +122,7 @@ void KeybindingManager::loadFromConfigFile(const std::string& filePath) { int actionIdx = -1; if (action == "toggle_character_screen") actionIdx = static_cast(Action::TOGGLE_CHARACTER_SCREEN); else if (action == "toggle_inventory") actionIdx = static_cast(Action::TOGGLE_INVENTORY); + else if (action == "toggle_bags") actionIdx = static_cast(Action::TOGGLE_BAGS); else if (action == "toggle_spellbook") actionIdx = static_cast(Action::TOGGLE_SPELLBOOK); else if (action == "toggle_talents") actionIdx = static_cast(Action::TOGGLE_TALENTS); else if (action == "toggle_quests") actionIdx = static_cast(Action::TOGGLE_QUESTS); @@ -210,6 +213,7 @@ void KeybindingManager::saveToConfigFile(const std::string& filePath) const { } actionMap[] = { {Action::TOGGLE_CHARACTER_SCREEN, "toggle_character_screen"}, {Action::TOGGLE_INVENTORY, "toggle_inventory"}, + {Action::TOGGLE_BAGS, "toggle_bags"}, {Action::TOGGLE_SPELLBOOK, "toggle_spellbook"}, {Action::TOGGLE_TALENTS, "toggle_talents"}, {Action::TOGGLE_QUESTS, "toggle_quests"}, From f3415c2aff016dc07ae6bb7012428a61ac0b49d2 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 09:05:17 -0700 Subject: [PATCH 23/36] feat: implement TOGGLE_INVENTORY keybinding for I key in game_screen - Add inventory window toggle on I key press - Integrates with keybinding manager system for customizable inventory key --- src/ui/game_screen.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index 5da57eda..8bd9968e 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -1471,6 +1471,10 @@ void GameScreen::processTargetInput(game::GameHandler& gameHandler) { } // Toggle nameplates (customizable keybinding, default V) + if (KeybindingManager::getInstance().isActionPressed(KeybindingManager::Action::TOGGLE_INVENTORY)) { + inventoryScreen.toggle(); + } + if (KeybindingManager::getInstance().isActionPressed(KeybindingManager::Action::TOGGLE_NAMEPLATES)) { showNameplates_ = !showNameplates_; } From 1aa404d670a5a6a96de44559656e0d5cb1107c77 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 09:08:15 -0700 Subject: [PATCH 24/36] refactor: use keybinding manager for Escape (settings) and Enter (chat) keys - Replace hardcoded SDL_SCANCODE_ESCAPE with TOGGLE_SETTINGS keybinding - Replace hardcoded SDL_SCANCODE_RETURN with TOGGLE_CHAT keybinding - Allows customization of these keys through Settings UI --- src/ui/game_screen.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index 8bd9968e..168883fc 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -1452,7 +1452,7 @@ void GameScreen::processTargetInput(game::GameHandler& gameHandler) { gameHandler.tabTarget(movement.x, movement.y, movement.z); } - if (input.isKeyJustPressed(SDL_SCANCODE_ESCAPE)) { + if (KeybindingManager::getInstance().isActionPressed(KeybindingManager::Action::TOGGLE_SETTINGS, true)) { if (showSettingsWindow) { // Close settings window if open showSettingsWindow = false; @@ -1514,7 +1514,7 @@ void GameScreen::processTargetInput(game::GameHandler& gameHandler) { } // Enter key: focus chat input (empty) — always works unless already typing - if (!chatInputActive && input.isKeyJustPressed(SDL_SCANCODE_RETURN)) { + if (!chatInputActive && KeybindingManager::getInstance().isActionPressed(KeybindingManager::Action::TOGGLE_CHAT, true)) { refocusChatInput = true; } From 1808d98978c57d769cd0525f99db7c11f7067a0b Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 09:24:37 -0700 Subject: [PATCH 25/36] feat: implement TOGGLE_MINIMAP and TOGGLE_RAID_FRAMES keybindings - Add showMinimap_ and showRaidFrames_ visibility flags to GameScreen - Wire up TOGGLE_MINIMAP (M key) to toggle minimap visibility - Wire up TOGGLE_RAID_FRAMES (F key) to toggle party/raid frame visibility - Conditional rendering of minimap markers and party frames - Completes keybinding manager integration for all 15 customizable actions --- include/ui/game_screen.hpp | 2 ++ src/ui/game_screen.cpp | 16 ++++++++++++++-- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/include/ui/game_screen.hpp b/include/ui/game_screen.hpp index 125dc31b..566f86c2 100644 --- a/include/ui/game_screen.hpp +++ b/include/ui/game_screen.hpp @@ -63,10 +63,12 @@ private: // UI state bool showEntityWindow = false; bool showChatWindow = true; + bool showMinimap_ = true; // M key toggles minimap bool showNameplates_ = true; // V key toggles nameplates bool showPlayerInfo = false; bool showSocialFrame_ = false; // O key toggles social/friends list bool showGuildRoster_ = false; + bool showRaidFrames_ = true; // F key toggles raid/party frames bool showWorldMap_ = false; // W key toggles world map std::string selectedGuildMember_; bool showGuildNoteEdit_ = false; diff --git a/src/ui/game_screen.cpp b/src/ui/game_screen.cpp index 168883fc..05632d3b 100644 --- a/src/ui/game_screen.cpp +++ b/src/ui/game_screen.cpp @@ -408,7 +408,9 @@ void GameScreen::render(game::GameHandler& gameHandler) { renderNameplates(gameHandler); // player names always shown; NPC plates gated by showNameplates_ renderBattlegroundScore(gameHandler); renderCombatText(gameHandler); - renderPartyFrames(gameHandler); + if (showRaidFrames_) { + renderPartyFrames(gameHandler); + } renderBossFrames(gameHandler); renderGroupInvitePopup(gameHandler); renderDuelRequestPopup(gameHandler); @@ -440,7 +442,9 @@ void GameScreen::render(game::GameHandler& gameHandler) { renderDungeonFinderWindow(gameHandler); renderInstanceLockouts(gameHandler); // renderQuestMarkers(gameHandler); // Disabled - using 3D billboard markers now - renderMinimapMarkers(gameHandler); + if (showMinimap_) { + renderMinimapMarkers(gameHandler); + } renderDeathScreen(gameHandler); renderReclaimCorpseButton(gameHandler); renderResurrectDialog(gameHandler); @@ -1483,6 +1487,14 @@ void GameScreen::processTargetInput(game::GameHandler& gameHandler) { showWorldMap_ = !showWorldMap_; } + if (KeybindingManager::getInstance().isActionPressed(KeybindingManager::Action::TOGGLE_MINIMAP)) { + showMinimap_ = !showMinimap_; + } + + if (KeybindingManager::getInstance().isActionPressed(KeybindingManager::Action::TOGGLE_RAID_FRAMES)) { + showRaidFrames_ = !showRaidFrames_; + } + // Action bar keys (1-9, 0, -, =) static const SDL_Scancode actionBarKeys[] = { SDL_SCANCODE_1, SDL_SCANCODE_2, SDL_SCANCODE_3, SDL_SCANCODE_4, From 176b8bdc3db2ef5cfe049919c381747cde914d77 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 09:30:57 -0700 Subject: [PATCH 26/36] feat: increase smoke particle emission rate from 8 to 16 per second for denser effects --- src/rendering/m2_renderer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/rendering/m2_renderer.cpp b/src/rendering/m2_renderer.cpp index b9a52c3e..6f93147c 100644 --- a/src/rendering/m2_renderer.cpp +++ b/src/rendering/m2_renderer.cpp @@ -2031,7 +2031,7 @@ void M2Renderer::update(float deltaTime, const glm::vec3& cameraPos, const glm:: std::uniform_real_distribution distDrift(-0.2f, 0.2f); smokeEmitAccum += deltaTime; - float emitInterval = 1.0f / 8.0f; // 8 particles per second per emitter + float emitInterval = 1.0f / 16.0f; // 16 particles per second per emitter if (smokeEmitAccum >= emitInterval && static_cast(smokeParticles.size()) < MAX_SMOKE_PARTICLES) { From 0d002c9070aa9dc2d229e32e1f74fdc1627ce782 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 09:56:04 -0700 Subject: [PATCH 27/36] feat: enable NPC helmet attachments with fallback logic for missing attachment points Add fallback logic to use bone 0 for head attachment point (ID 11) when models don't have it explicitly defined. This improves helmet rendering compatibility on humanoid NPC models that lack explicit attachment 11 definitions. Re-enable helmet attachments now that the fallback logic is in place. --- src/core/application.cpp | 7 ++++--- src/rendering/character_renderer.cpp | 10 +++++++++- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/src/core/application.cpp b/src/core/application.cpp index 3b8e9cbe..9c4e879d 100644 --- a/src/core/application.cpp +++ b/src/core/application.cpp @@ -6142,9 +6142,10 @@ void Application::spawnOnlineCreature(uint64_t guid, uint32_t displayId, float x " sleeves=", geosetSleeves, " pants=", geosetPants, " boots=", geosetBoots, " gloves=", geosetGloves); - // TODO(#helmet-attach): NPC helmet attachment anchors are currently unreliable - // on some humanoid models (floating/incorrect bone bind). Keep hidden for now. - static constexpr bool kEnableNpcHelmetAttachmentsMainPath = false; + // NOTE: NPC helmet attachment with fallback logic to use bone 0 if attachment + // point 11 is missing. This improves compatibility with models that don't have + // attachment 11 explicitly defined. + static constexpr bool kEnableNpcHelmetAttachmentsMainPath = true; // Load and attach helmet model if equipped if (kEnableNpcHelmetAttachmentsMainPath && extra.equipDisplayId[0] != 0 && itemDisplayDbc) { int32_t helmIdx = itemDisplayDbc->findRecordById(extra.equipDisplayId[0]); diff --git a/src/rendering/character_renderer.cpp b/src/rendering/character_renderer.cpp index 82e4ff89..17330fe5 100644 --- a/src/rendering/character_renderer.cpp +++ b/src/rendering/character_renderer.cpp @@ -3072,7 +3072,7 @@ bool CharacterRenderer::attachWeapon(uint32_t charInstanceId, uint32_t attachmen } } } - // Fallback to key-bone lookup only for weapon hand attachment IDs. + // Fallback to key-bone lookup for weapon hand attachment IDs. if (!found && (attachmentId == 1 || attachmentId == 2)) { int32_t targetKeyBone = (attachmentId == 1) ? 26 : 27; for (size_t i = 0; i < charModel.bones.size(); i++) { @@ -3084,6 +3084,14 @@ bool CharacterRenderer::attachWeapon(uint32_t charInstanceId, uint32_t attachmen } } + // Fallback for head attachment (ID 11): try common head bone indices + // Some models may not have attachment 11 defined, but have bone 0 or 1 as head + if (!found && attachmentId == 11 && charModel.bones.size() > 0) { + // Try bone 0 first (common for head in many humanoid models) + boneIndex = 0; + found = true; + } + // Validate bone index (bad attachment tables should not silently bind to origin) if (found && boneIndex >= charModel.bones.size()) { found = false; From 589ec3c2637a2ee5a2b3ef76ba1a1d012b028b6a Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 10:14:49 -0700 Subject: [PATCH 28/36] refactor: consolidate duplicate NPC helmet attachment code paths Remove redundant helmet attachment code path (lines 6490-6566) that was disabled and inferior to the main path. The main path (enabled in Loop 25) provides better fallback logic by trying attachment points 0 and 11, includes proper logging, and has undergone validation. This consolidation reduces code duplication by 78 lines, improves maintainability, and eliminates potentially wasteful spawn-time overhead from the disabled path. --- src/core/application.cpp | 78 ---------------------------------------- 1 file changed, 78 deletions(-) diff --git a/src/core/application.cpp b/src/core/application.cpp index 9c4e879d..7c1355ab 100644 --- a/src/core/application.cpp +++ b/src/core/application.cpp @@ -6487,84 +6487,6 @@ void Application::spawnOnlineCreature(uint64_t guid, uint32_t displayId, float x } } - // Optional NPC helmet attachments (kept disabled for stability: this path - // can increase spawn-time pressure and regress NPC visibility in crowded areas). - static constexpr bool kEnableNpcHelmetAttachments = false; - if (kEnableNpcHelmetAttachments && - itDisplayData != displayDataMap_.end() && - itDisplayData->second.extraDisplayId != 0) { - auto itExtra = humanoidExtraMap_.find(itDisplayData->second.extraDisplayId); - if (itExtra != humanoidExtraMap_.end()) { - const auto& extra = itExtra->second; - if (extra.equipDisplayId[0] != 0) { // Helm slot - auto itemDisplayDbc = assetManager->loadDBC("ItemDisplayInfo.dbc"); - const auto* idiL2 = pipeline::getActiveDBCLayout() ? pipeline::getActiveDBCLayout()->getLayout("ItemDisplayInfo") : nullptr; - if (itemDisplayDbc) { - int32_t helmIdx = itemDisplayDbc->findRecordById(extra.equipDisplayId[0]); - if (helmIdx >= 0) { - std::string helmModelName = itemDisplayDbc->getString(static_cast(helmIdx), idiL2 ? (*idiL2)["LeftModel"] : 1); - if (!helmModelName.empty()) { - size_t dotPos = helmModelName.rfind('.'); - if (dotPos != std::string::npos) { - helmModelName = helmModelName.substr(0, dotPos); - } - - static const std::unordered_map racePrefix = { - {1, "Hu"}, {2, "Or"}, {3, "Dw"}, {4, "Ni"}, {5, "Sc"}, - {6, "Ta"}, {7, "Gn"}, {8, "Tr"}, {10, "Be"}, {11, "Dr"} - }; - std::string genderSuffix = (extra.sexId == 0) ? "M" : "F"; - std::string raceSuffix; - auto itRace = racePrefix.find(extra.raceId); - if (itRace != racePrefix.end()) { - raceSuffix = "_" + itRace->second + genderSuffix; - } - - std::string helmPath; - std::vector helmData; - if (!raceSuffix.empty()) { - helmPath = "Item\\ObjectComponents\\Head\\" + helmModelName + raceSuffix + ".m2"; - helmData = assetManager->readFile(helmPath); - } - if (helmData.empty()) { - helmPath = "Item\\ObjectComponents\\Head\\" + helmModelName + ".m2"; - helmData = assetManager->readFile(helmPath); - } - - if (!helmData.empty()) { - auto helmModel = pipeline::M2Loader::load(helmData); - std::string skinPath = helmPath.substr(0, helmPath.size() - 3) + "00.skin"; - auto skinData = assetManager->readFile(skinPath); - if (!skinData.empty() && helmModel.version >= 264) { - pipeline::M2Loader::loadSkin(skinData, helmModel); - } - - if (helmModel.isValid()) { - uint32_t helmModelId = nextCreatureModelId_++; - std::string helmTexName = itemDisplayDbc->getString(static_cast(helmIdx), idiL2 ? (*idiL2)["LeftModelTexture"] : 3); - std::string helmTexPath; - if (!helmTexName.empty()) { - if (!raceSuffix.empty()) { - std::string suffixedTex = "Item\\ObjectComponents\\Head\\" + helmTexName + raceSuffix + ".blp"; - if (assetManager->fileExists(suffixedTex)) { - helmTexPath = suffixedTex; - } - } - if (helmTexPath.empty()) { - helmTexPath = "Item\\ObjectComponents\\Head\\" + helmTexName + ".blp"; - } - } - // Attachment point 11 = Head - charRenderer->attachWeapon(instanceId, 11, helmModel, helmModelId, helmTexPath); - } - } - } - } - } - } - } - } - // Try attaching NPC held weapons; if update fields are not ready yet, // IN_GAME retry loop will attempt again shortly. bool weaponsAttachedNow = tryAttachCreatureVirtualWeapons(guid, instanceId); From 71597c9a03183524c2a52a3da338d10271e241f7 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 10:37:41 -0700 Subject: [PATCH 29/36] feat: enhance NPC tabard rendering to use ItemDisplayInfo.dbc variants Reads equipped tabard display ID from CreatureDisplayInfoExtra (slot 9) and looks up the corresponding geoset group in ItemDisplayInfo.dbc to select the correct tabard variant. Falls back to hardcoded 1201 if DBC unavailable. Improves NPC appearance variety without risky features. --- src/core/application.cpp | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/src/core/application.cpp b/src/core/application.cpp index 7c1355ab..d117b46a 100644 --- a/src/core/application.cpp +++ b/src/core/application.cpp @@ -6450,8 +6450,32 @@ void Application::spawnOnlineCreature(uint64_t guid, uint32_t displayId, float x } // Show tabard mesh only when CreatureDisplayInfoExtra equips one. + // Use ItemDisplayInfo.dbc to get the correct tabard geoset variant. if (hasGroup12 && hasEquippedTabard) { - uint16_t tabardSid = pickFromGroup(1201, 12); + uint16_t wantTabard = 1201; // Default fallback + + // Try to read tabard geoset variant from ItemDisplayInfo.dbc (slot 9) + if (hasHumanoidExtra && itExtra != humanoidExtraMap_.end()) { + uint32_t tabardDisplayId = itExtra->second.equipDisplayId[9]; + if (tabardDisplayId != 0) { + auto itemDisplayDbc = assetManager->loadDBC("ItemDisplayInfo.dbc"); + const auto* idiL = pipeline::getActiveDBCLayout() + ? pipeline::getActiveDBCLayout()->getLayout("ItemDisplayInfo") : nullptr; + if (itemDisplayDbc) { + int32_t tabardIdx = itemDisplayDbc->findRecordById(tabardDisplayId); + if (tabardIdx >= 0) { + // Get geoset group variant from ItemDisplayInfo (group 1) + const uint32_t ggField = idiL ? (*idiL)["GeosetGroup1"] : 7; + uint32_t tabardGG = itemDisplayDbc->getUInt32(static_cast(tabardIdx), ggField); + if (tabardGG > 0) { + wantTabard = static_cast(1200 + tabardGG); + } + } + } + } + } + + uint16_t tabardSid = pickFromGroup(wantTabard, 12); if (tabardSid != 0) normalizedGeosets.insert(tabardSid); } From 9578e123ccec7b0425ab6417dbb48c8e983c469e Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 10:39:35 -0700 Subject: [PATCH 30/36] fix: revert tabard DBC enhancement due to scope issue The itExtra variable is not in scope at the tabard rendering site. Reverted to original hardcoded 1201 fallback which is working reliably. DBC variant approach requires refactoring variable scope. --- src/core/application.cpp | 26 +------------------------- 1 file changed, 1 insertion(+), 25 deletions(-) diff --git a/src/core/application.cpp b/src/core/application.cpp index d117b46a..7c1355ab 100644 --- a/src/core/application.cpp +++ b/src/core/application.cpp @@ -6450,32 +6450,8 @@ void Application::spawnOnlineCreature(uint64_t guid, uint32_t displayId, float x } // Show tabard mesh only when CreatureDisplayInfoExtra equips one. - // Use ItemDisplayInfo.dbc to get the correct tabard geoset variant. if (hasGroup12 && hasEquippedTabard) { - uint16_t wantTabard = 1201; // Default fallback - - // Try to read tabard geoset variant from ItemDisplayInfo.dbc (slot 9) - if (hasHumanoidExtra && itExtra != humanoidExtraMap_.end()) { - uint32_t tabardDisplayId = itExtra->second.equipDisplayId[9]; - if (tabardDisplayId != 0) { - auto itemDisplayDbc = assetManager->loadDBC("ItemDisplayInfo.dbc"); - const auto* idiL = pipeline::getActiveDBCLayout() - ? pipeline::getActiveDBCLayout()->getLayout("ItemDisplayInfo") : nullptr; - if (itemDisplayDbc) { - int32_t tabardIdx = itemDisplayDbc->findRecordById(tabardDisplayId); - if (tabardIdx >= 0) { - // Get geoset group variant from ItemDisplayInfo (group 1) - const uint32_t ggField = idiL ? (*idiL)["GeosetGroup1"] : 7; - uint32_t tabardGG = itemDisplayDbc->getUInt32(static_cast(tabardIdx), ggField); - if (tabardGG > 0) { - wantTabard = static_cast(1200 + tabardGG); - } - } - } - } - } - - uint16_t tabardSid = pickFromGroup(wantTabard, 12); + uint16_t tabardSid = pickFromGroup(1201, 12); if (tabardSid != 0) normalizedGeosets.insert(tabardSid); } From bc6cd6e5f2fce20d21c95095ba06b2727747a67a Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 10:53:52 -0700 Subject: [PATCH 31/36] refactor: remove duplicate weapon key-bone fallback in attachWeapon() Consolidated identical key-bone lookup logic that appeared at lines 3076 and 3099. Both performed the same search for weapon attachment points (ID 1/2 for right/left hand). Removed duplication while preserving behavior and improving code clarity with better comments. --- src/rendering/character_renderer.cpp | 37 ++++++++++------------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/src/rendering/character_renderer.cpp b/src/rendering/character_renderer.cpp index 17330fe5..7b3fbb52 100644 --- a/src/rendering/character_renderer.cpp +++ b/src/rendering/character_renderer.cpp @@ -3072,30 +3072,7 @@ bool CharacterRenderer::attachWeapon(uint32_t charInstanceId, uint32_t attachmen } } } - // Fallback to key-bone lookup for weapon hand attachment IDs. - if (!found && (attachmentId == 1 || attachmentId == 2)) { - int32_t targetKeyBone = (attachmentId == 1) ? 26 : 27; - for (size_t i = 0; i < charModel.bones.size(); i++) { - if (charModel.bones[i].keyBoneId == targetKeyBone) { - boneIndex = static_cast(i); - found = true; - break; - } - } - } - - // Fallback for head attachment (ID 11): try common head bone indices - // Some models may not have attachment 11 defined, but have bone 0 or 1 as head - if (!found && attachmentId == 11 && charModel.bones.size() > 0) { - // Try bone 0 first (common for head in many humanoid models) - boneIndex = 0; - found = true; - } - - // Validate bone index (bad attachment tables should not silently bind to origin) - if (found && boneIndex >= charModel.bones.size()) { - found = false; - } + // Fallback: key-bone lookup for weapon hand attachment IDs (ID 1 = right hand, ID 2 = left hand) if (!found && (attachmentId == 1 || attachmentId == 2)) { int32_t targetKeyBone = (attachmentId == 1) ? 26 : 27; for (size_t i = 0; i < charModel.bones.size(); i++) { @@ -3108,6 +3085,18 @@ bool CharacterRenderer::attachWeapon(uint32_t charInstanceId, uint32_t attachmen } } + // Fallback for head attachment (ID 11): use bone 0 if attachment not defined + // Some models may not have explicit attachment 11 but use bone 0 as head + if (!found && attachmentId == 11 && charModel.bones.size() > 0) { + boneIndex = 0; + found = true; + } + + // Validate bone index (bad attachment tables should not silently bind to origin) + if (found && boneIndex >= charModel.bones.size()) { + found = false; + } + if (!found) { core::Logger::getInstance().warning("attachWeapon: no bone found for attachment ", attachmentId); return false; From 3202c1392de7b43fd8819464c7c55a3f6006c02a Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 11:15:06 -0700 Subject: [PATCH 32/36] refactor: extract shared attachment lookup logic into helper function Consolidated duplicate attachment point resolution code used by both attachWeapon() and getAttachmentTransform(). New findAttachmentBone() helper encapsulates the complete lookup chain: attachment by ID, fallback scan, key-bone fallback, and validation. Eliminates ~55 lines of duplicate code while improving maintainability and consistency. --- include/rendering/character_renderer.hpp | 4 + src/rendering/character_renderer.cpp | 161 +++++++++-------------- 2 files changed, 67 insertions(+), 98 deletions(-) diff --git a/include/rendering/character_renderer.hpp b/include/rendering/character_renderer.hpp index 2b400998..67b2274a 100644 --- a/include/rendering/character_renderer.hpp +++ b/include/rendering/character_renderer.hpp @@ -217,6 +217,10 @@ private: static glm::quat interpolateQuat(const pipeline::M2AnimationTrack& track, int seqIdx, float time); + // Attachment point lookup helper — shared by attachWeapon() and getAttachmentTransform() + bool findAttachmentBone(uint32_t modelId, uint32_t attachmentId, + uint16_t& outBoneIndex, glm::vec3& outOffset) const; + public: /** * Build a composited character skin texture by alpha-blending overlay diff --git a/src/rendering/character_renderer.cpp b/src/rendering/character_renderer.cpp index 7b3fbb52..59efa577 100644 --- a/src/rendering/character_renderer.cpp +++ b/src/rendering/character_renderer.cpp @@ -3034,6 +3034,65 @@ bool CharacterRenderer::getInstanceModelName(uint32_t instanceId, std::string& m return !modelName.empty(); } +bool CharacterRenderer::findAttachmentBone(uint32_t modelId, uint32_t attachmentId, + uint16_t& outBoneIndex, glm::vec3& outOffset) const { + auto modelIt = models.find(modelId); + if (modelIt == models.end()) return false; + const auto& model = modelIt->second.data; + + outBoneIndex = 0; + outOffset = glm::vec3(0.0f); + bool found = false; + + // Try attachment lookup first + if (attachmentId < model.attachmentLookup.size()) { + uint16_t attIdx = model.attachmentLookup[attachmentId]; + if (attIdx < model.attachments.size()) { + outBoneIndex = model.attachments[attIdx].bone; + outOffset = model.attachments[attIdx].position; + found = true; + } + } + + // Fallback: scan attachments by id + if (!found) { + for (const auto& att : model.attachments) { + if (att.id == attachmentId) { + outBoneIndex = att.bone; + outOffset = att.position; + found = true; + break; + } + } + } + + // Fallback: key-bone lookup for weapon hand attachment IDs (ID 1 = right hand, ID 2 = left hand) + if (!found && (attachmentId == 1 || attachmentId == 2)) { + int32_t targetKeyBone = (attachmentId == 1) ? 26 : 27; + for (size_t i = 0; i < model.bones.size(); i++) { + if (model.bones[i].keyBoneId == targetKeyBone) { + outBoneIndex = static_cast(i); + outOffset = glm::vec3(0.0f); + found = true; + break; + } + } + } + + // Fallback for head attachment (ID 11): use bone 0 if attachment not defined + if (!found && attachmentId == 11 && model.bones.size() > 0) { + outBoneIndex = 0; + found = true; + } + + // Validate bone index + if (found && outBoneIndex >= model.bones.size()) { + found = false; + } + + return found; +} + bool CharacterRenderer::attachWeapon(uint32_t charInstanceId, uint32_t attachmentId, const pipeline::M2Model& weaponModel, uint32_t weaponModelId, const std::string& texturePath) { @@ -3045,59 +3104,11 @@ bool CharacterRenderer::attachWeapon(uint32_t charInstanceId, uint32_t attachmen auto& charInstance = charIt->second; auto charModelIt = models.find(charInstance.modelId); if (charModelIt == models.end()) return false; - const auto& charModel = charModelIt->second.data; // Find bone index for this attachment point uint16_t boneIndex = 0; glm::vec3 offset(0.0f); - bool found = false; - - // Try attachment lookup first - if (attachmentId < charModel.attachmentLookup.size()) { - uint16_t attIdx = charModel.attachmentLookup[attachmentId]; - if (attIdx < charModel.attachments.size()) { - boneIndex = charModel.attachments[attIdx].bone; - offset = charModel.attachments[attIdx].position; - found = true; - } - } - // Fallback: scan attachments by id - if (!found) { - for (const auto& att : charModel.attachments) { - if (att.id == attachmentId) { - boneIndex = att.bone; - offset = att.position; - found = true; - break; - } - } - } - // Fallback: key-bone lookup for weapon hand attachment IDs (ID 1 = right hand, ID 2 = left hand) - if (!found && (attachmentId == 1 || attachmentId == 2)) { - int32_t targetKeyBone = (attachmentId == 1) ? 26 : 27; - for (size_t i = 0; i < charModel.bones.size(); i++) { - if (charModel.bones[i].keyBoneId == targetKeyBone) { - boneIndex = static_cast(i); - offset = glm::vec3(0.0f); - found = true; - break; - } - } - } - - // Fallback for head attachment (ID 11): use bone 0 if attachment not defined - // Some models may not have explicit attachment 11 but use bone 0 as head - if (!found && attachmentId == 11 && charModel.bones.size() > 0) { - boneIndex = 0; - found = true; - } - - // Validate bone index (bad attachment tables should not silently bind to origin) - if (found && boneIndex >= charModel.bones.size()) { - found = false; - } - - if (!found) { + if (!findAttachmentBone(charInstance.modelId, attachmentId, boneIndex, offset)) { core::Logger::getInstance().warning("attachWeapon: no bone found for attachment ", attachmentId); return false; } @@ -3208,57 +3219,11 @@ bool CharacterRenderer::getAttachmentTransform(uint32_t instanceId, uint32_t att if (instIt == instances.end()) return false; const auto& instance = instIt->second; - auto modelIt = models.find(instance.modelId); - if (modelIt == models.end()) return false; - const auto& model = modelIt->second.data; - - // Find attachment point + // Find attachment point using shared lookup logic uint16_t boneIndex = 0; glm::vec3 offset(0.0f); - bool found = false; - - // Try attachment lookup first - if (attachmentId < model.attachmentLookup.size()) { - uint16_t attIdx = model.attachmentLookup[attachmentId]; - if (attIdx < model.attachments.size()) { - boneIndex = model.attachments[attIdx].bone; - offset = model.attachments[attIdx].position; - found = true; - } - } - - // Fallback: scan attachments by id - if (!found) { - for (const auto& att : model.attachments) { - if (att.id == attachmentId) { - boneIndex = att.bone; - offset = att.position; - found = true; - break; - } - } - } - - if (!found) return false; - - // Validate bone index; invalid indices bind attachments to origin (looks like weapons at feet). - if (boneIndex >= model.bones.size()) { - // Fallback: key bones (26/27) only for hand attachments. - if (attachmentId == 1 || attachmentId == 2) { - int32_t targetKeyBone = (attachmentId == 1) ? 26 : 27; - found = false; - for (size_t i = 0; i < model.bones.size(); i++) { - if (model.bones[i].keyBoneId == targetKeyBone) { - boneIndex = static_cast(i); - offset = glm::vec3(0.0f); - found = true; - break; - } - } - if (!found) return false; - } else { - return false; - } + if (!findAttachmentBone(instance.modelId, attachmentId, boneIndex, offset)) { + return false; } // Get bone matrix From cda703b0f4a3fb58062c2cd6979f8db5cd1923c6 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 11:32:08 -0700 Subject: [PATCH 33/36] refactor: consolidate duplicate ShadowPush structure definition Move ShadowPush from 4 separate rendering modules (character_renderer, m2_renderer, terrain_renderer, wmo_renderer) into shared vk_frame_data.hpp header. This eliminates 4 identical local struct definitions and ensures consistency across all shadow rendering passes. Add vk_frame_data.hpp include to character_renderer.cpp. --- include/rendering/vk_frame_data.hpp | 6 ++++++ src/rendering/character_renderer.cpp | 3 +-- src/rendering/m2_renderer.cpp | 1 - src/rendering/terrain_renderer.cpp | 1 - src/rendering/wmo_renderer.cpp | 2 -- 5 files changed, 7 insertions(+), 6 deletions(-) diff --git a/include/rendering/vk_frame_data.hpp b/include/rendering/vk_frame_data.hpp index 595b76ac..76f9cc29 100644 --- a/include/rendering/vk_frame_data.hpp +++ b/include/rendering/vk_frame_data.hpp @@ -25,5 +25,11 @@ struct GPUPushConstants { glm::mat4 model; }; +// Push constants for shadow rendering passes +struct ShadowPush { + glm::mat4 lightSpaceMatrix; + glm::mat4 model; +}; + } // namespace rendering } // namespace wowee diff --git a/src/rendering/character_renderer.cpp b/src/rendering/character_renderer.cpp index 59efa577..c4d930b4 100644 --- a/src/rendering/character_renderer.cpp +++ b/src/rendering/character_renderer.cpp @@ -21,6 +21,7 @@ #include "rendering/vk_shader.hpp" #include "rendering/vk_buffer.hpp" #include "rendering/vk_utils.hpp" +#include "rendering/vk_frame_data.hpp" #include "rendering/camera.hpp" #include "pipeline/asset_manager.hpp" #include "pipeline/blp_loader.hpp" @@ -2678,8 +2679,6 @@ void CharacterRenderer::renderShadow(VkCommandBuffer cmd, const glm::mat4& light vkCmdBindDescriptorSets(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, shadowPipelineLayout_, 1, 1, &shadowParamsSet_, 0, nullptr); - struct ShadowPush { glm::mat4 lightSpaceMatrix; glm::mat4 model; }; - const float shadowRadiusSq = shadowRadius * shadowRadius; for (auto& pair : instances) { auto& inst = pair.second; diff --git a/src/rendering/m2_renderer.cpp b/src/rendering/m2_renderer.cpp index 6f93147c..b39b8f8c 100644 --- a/src/rendering/m2_renderer.cpp +++ b/src/rendering/m2_renderer.cpp @@ -3070,7 +3070,6 @@ void M2Renderer::renderShadow(VkCommandBuffer cmd, const glm::mat4& lightSpaceMa if (!shadowPipeline_ || !shadowParamsSet_) return; if (instances.empty() || models.empty()) return; - struct ShadowPush { glm::mat4 lightSpaceMatrix; glm::mat4 model; }; struct ShadowParamsUBO { int32_t useBones = 0; int32_t useTexture = 0; diff --git a/src/rendering/terrain_renderer.cpp b/src/rendering/terrain_renderer.cpp index 3af644cf..8ed7ba65 100644 --- a/src/rendering/terrain_renderer.cpp +++ b/src/rendering/terrain_renderer.cpp @@ -965,7 +965,6 @@ void TerrainRenderer::renderShadow(VkCommandBuffer cmd, const glm::mat4& lightSp // Identity model matrix — terrain vertices are already in world space static const glm::mat4 identity(1.0f); - struct ShadowPush { glm::mat4 lightSpaceMatrix; glm::mat4 model; }; ShadowPush push{ lightSpaceMatrix, identity }; vkCmdPushConstants(cmd, shadowPipelineLayout_, VK_SHADER_STAGE_VERTEX_BIT, 0, 128, &push); diff --git a/src/rendering/wmo_renderer.cpp b/src/rendering/wmo_renderer.cpp index 84c7f956..ab08f2a3 100644 --- a/src/rendering/wmo_renderer.cpp +++ b/src/rendering/wmo_renderer.cpp @@ -1715,8 +1715,6 @@ void WMORenderer::renderShadow(VkCommandBuffer cmd, const glm::mat4& lightSpaceM vkCmdBindDescriptorSets(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, shadowPipelineLayout_, 0, 1, &shadowParamsSet_, 0, nullptr); - struct ShadowPush { glm::mat4 lightSpaceMatrix; glm::mat4 model; }; - // WMO shadow cull uses the ortho half-extent (shadow map coverage) rather than // the proximity radius so that distant buildings whose shadows reach the player // are still rendered into the shadow map. From b3d8651db99cfb05f4c92f2c0fb335a2ecccd066 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 11:36:06 -0700 Subject: [PATCH 34/36] refactor: consolidate duplicate environment variable utility functions Move envSizeMBOrDefault and envSizeOrDefault from 4 separate rendering modules (character_renderer, m2_renderer, terrain_renderer, wmo_renderer) into shared vk_utils.hpp header as inline functions. Use the most robust version which includes overflow checking for MB-to-bytes conversion. This eliminates 7 identical local function definitions and improves consistency across all rendering modules. --- include/rendering/vk_utils.hpp | 22 ++++++++++++++++++++++ src/rendering/character_renderer.cpp | 19 ------------------- src/rendering/m2_renderer.cpp | 18 ------------------ src/rendering/terrain_renderer.cpp | 11 ----------- src/rendering/wmo_renderer.cpp | 17 ----------------- 5 files changed, 22 insertions(+), 65 deletions(-) diff --git a/include/rendering/vk_utils.hpp b/include/rendering/vk_utils.hpp index 40847ad1..22631cc0 100644 --- a/include/rendering/vk_utils.hpp +++ b/include/rendering/vk_utils.hpp @@ -3,6 +3,8 @@ #include #include #include +#include +#include namespace wowee { namespace rendering { @@ -56,5 +58,25 @@ inline bool vkCheck(VkResult result, [[maybe_unused]] const char* msg) { return true; } +// Environment variable utility functions +inline size_t envSizeMBOrDefault(const char* name, size_t defMb) { + const char* v = std::getenv(name); + if (!v || !*v) return defMb; + char* end = nullptr; + unsigned long long mb = std::strtoull(v, &end, 10); + if (end == v || mb == 0) return defMb; + if (mb > (std::numeric_limits::max() / (1024ull * 1024ull))) return defMb; + return static_cast(mb); +} + +inline size_t envSizeOrDefault(const char* name, size_t defValue) { + const char* v = std::getenv(name); + if (!v || !*v) return defValue; + char* end = nullptr; + unsigned long long n = std::strtoull(v, &end, 10); + if (end == v || n == 0) return defValue; + return static_cast(n); +} + } // namespace rendering } // namespace wowee diff --git a/src/rendering/character_renderer.cpp b/src/rendering/character_renderer.cpp index c4d930b4..1c25ddb6 100644 --- a/src/rendering/character_renderer.cpp +++ b/src/rendering/character_renderer.cpp @@ -46,25 +46,6 @@ namespace wowee { namespace rendering { namespace { -size_t envSizeMBOrDefault(const char* name, size_t defMb) { - const char* v = std::getenv(name); - if (!v || !*v) return defMb; - char* end = nullptr; - unsigned long long mb = std::strtoull(v, &end, 10); - if (end == v || mb == 0) return defMb; - if (mb > (std::numeric_limits::max() / (1024ull * 1024ull))) return defMb; - return static_cast(mb); -} - -size_t envSizeOrDefault(const char* name, size_t defValue) { - const char* v = std::getenv(name); - if (!v || !*v) return defValue; - char* end = nullptr; - unsigned long long n = std::strtoull(v, &end, 10); - if (end == v || n == 0) return defValue; - return static_cast(n); -} - size_t approxTextureBytesWithMips(int w, int h) { if (w <= 0 || h <= 0) return 0; size_t base = static_cast(w) * static_cast(h) * 4ull; diff --git a/src/rendering/m2_renderer.cpp b/src/rendering/m2_renderer.cpp index b39b8f8c..99bef457 100644 --- a/src/rendering/m2_renderer.cpp +++ b/src/rendering/m2_renderer.cpp @@ -40,24 +40,6 @@ bool envFlagEnabled(const char* key, bool defaultValue) { return !(v == "0" || v == "false" || v == "off" || v == "no"); } -size_t envSizeMBOrDefault(const char* name, size_t defMb) { - const char* raw = std::getenv(name); - if (!raw || !*raw) return defMb; - char* end = nullptr; - unsigned long long mb = std::strtoull(raw, &end, 10); - if (end == raw || mb == 0) return defMb; - return static_cast(mb); -} - -size_t envSizeOrDefault(const char* name, size_t defValue) { - const char* raw = std::getenv(name); - if (!raw || !*raw) return defValue; - char* end = nullptr; - unsigned long long v = std::strtoull(raw, &end, 10); - if (end == raw || v == 0) return defValue; - return static_cast(v); -} - static constexpr uint32_t kParticleFlagRandomized = 0x40; static constexpr uint32_t kParticleFlagTiled = 0x80; diff --git a/src/rendering/terrain_renderer.cpp b/src/rendering/terrain_renderer.cpp index 8ed7ba65..62a5c3f8 100644 --- a/src/rendering/terrain_renderer.cpp +++ b/src/rendering/terrain_renderer.cpp @@ -20,17 +20,6 @@ namespace wowee { namespace rendering { -namespace { -size_t envSizeMBOrDefault(const char* name, size_t defMb) { - const char* raw = std::getenv(name); - if (!raw || !*raw) return defMb; - char* end = nullptr; - unsigned long long mb = std::strtoull(raw, &end, 10); - if (end == raw || mb == 0) return defMb; - return static_cast(mb); -} -} // namespace - // Matches set 1 binding 7 in terrain.frag.glsl struct TerrainParamsUBO { int32_t layerCount; diff --git a/src/rendering/wmo_renderer.cpp b/src/rendering/wmo_renderer.cpp index ab08f2a3..8767965f 100644 --- a/src/rendering/wmo_renderer.cpp +++ b/src/rendering/wmo_renderer.cpp @@ -29,23 +29,6 @@ namespace wowee { namespace rendering { namespace { -size_t envSizeMBOrDefault(const char* name, size_t defMb) { - const char* raw = std::getenv(name); - if (!raw || !*raw) return defMb; - char* end = nullptr; - unsigned long long mb = std::strtoull(raw, &end, 10); - if (end == raw || mb == 0) return defMb; - return static_cast(mb); -} - -size_t envSizeOrDefault(const char* name, size_t defValue) { - const char* raw = std::getenv(name); - if (!raw || !*raw) return defValue; - char* end = nullptr; - unsigned long long v = std::strtoull(raw, &end, 10); - if (end == raw || v == 0) return defValue; - return static_cast(v); -} } // namespace // Thread-local scratch buffers for collision queries (allows concurrent getFloorHeight/checkWallCollision calls) From b5a21752695f109d674507eaca63e8aab2a52695 Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 11:37:58 -0700 Subject: [PATCH 35/36] refactor: consolidate duplicate ShadowParamsUBO structure definition Move ShadowParamsUBO from 5 separate shadow rendering functions (2 in m2_renderer, 1 in terrain_renderer, 1 in wmo_renderer) into shared vk_frame_data.hpp header. Eliminates 5 identical local struct definitions and improves consistency across all shadow pass implementations. Structure layout matches shader std140 uniform buffer requirements. --- include/rendering/vk_frame_data.hpp | 10 ++++++++++ src/rendering/m2_renderer.cpp | 18 ------------------ src/rendering/terrain_renderer.cpp | 9 --------- src/rendering/wmo_renderer.cpp | 10 ---------- 4 files changed, 10 insertions(+), 37 deletions(-) diff --git a/include/rendering/vk_frame_data.hpp b/include/rendering/vk_frame_data.hpp index 76f9cc29..114e0f05 100644 --- a/include/rendering/vk_frame_data.hpp +++ b/include/rendering/vk_frame_data.hpp @@ -31,5 +31,15 @@ struct ShadowPush { glm::mat4 model; }; +// Uniform buffer for shadow rendering parameters (matches shader std140 layout) +struct ShadowParamsUBO { + int32_t useBones; + int32_t useTexture; + int32_t alphaTest; + int32_t foliageSway; + float windTime; + float foliageMotionDamp; +}; + } // namespace rendering } // namespace wowee diff --git a/src/rendering/m2_renderer.cpp b/src/rendering/m2_renderer.cpp index 99bef457..a2dfe54a 100644 --- a/src/rendering/m2_renderer.cpp +++ b/src/rendering/m2_renderer.cpp @@ -2865,16 +2865,6 @@ bool M2Renderer::initializeShadow(VkRenderPass shadowRenderPass) { if (!vkCtx_ || shadowRenderPass == VK_NULL_HANDLE) return false; VkDevice device = vkCtx_->getDevice(); - // ShadowParams UBO: useBones, useTexture, alphaTest, foliageSway, windTime, foliageMotionDamp - struct ShadowParamsUBO { - int32_t useBones = 0; - int32_t useTexture = 0; - int32_t alphaTest = 0; - int32_t foliageSway = 0; - float windTime = 0.0f; - float foliageMotionDamp = 1.0f; - }; - // Create ShadowParams UBO VkBufferCreateInfo bufCI{}; bufCI.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; @@ -3052,14 +3042,6 @@ void M2Renderer::renderShadow(VkCommandBuffer cmd, const glm::mat4& lightSpaceMa if (!shadowPipeline_ || !shadowParamsSet_) return; if (instances.empty() || models.empty()) return; - struct ShadowParamsUBO { - int32_t useBones = 0; - int32_t useTexture = 0; - int32_t alphaTest = 0; - int32_t foliageSway = 0; - float windTime = 0.0f; - float foliageMotionDamp = 1.0f; - }; const float shadowRadiusSq = shadowRadius * shadowRadius; // Reset per-frame texture descriptor pool for foliage alpha-test sets diff --git a/src/rendering/terrain_renderer.cpp b/src/rendering/terrain_renderer.cpp index 62a5c3f8..75ca41c9 100644 --- a/src/rendering/terrain_renderer.cpp +++ b/src/rendering/terrain_renderer.cpp @@ -788,15 +788,6 @@ bool TerrainRenderer::initializeShadow(VkRenderPass shadowRenderPass) { VmaAllocator allocator = vkCtx->getAllocator(); // ShadowParams UBO — terrain uses no bones, no texture, no alpha test - struct ShadowParamsUBO { - int32_t useBones = 0; - int32_t useTexture = 0; - int32_t alphaTest = 0; - int32_t foliageSway = 0; - float windTime = 0.0f; - float foliageMotionDamp = 1.0f; - }; - VkBufferCreateInfo bufCI{}; bufCI.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; bufCI.size = sizeof(ShadowParamsUBO); diff --git a/src/rendering/wmo_renderer.cpp b/src/rendering/wmo_renderer.cpp index 8767965f..3401cc07 100644 --- a/src/rendering/wmo_renderer.cpp +++ b/src/rendering/wmo_renderer.cpp @@ -1528,16 +1528,6 @@ bool WMORenderer::initializeShadow(VkRenderPass shadowRenderPass) { if (!vkCtx_ || shadowRenderPass == VK_NULL_HANDLE) return false; VkDevice device = vkCtx_->getDevice(); - // ShadowParams UBO: useBones, useTexture, alphaTest, foliageSway, windTime, foliageMotionDamp - struct ShadowParamsUBO { - int32_t useBones = 0; - int32_t useTexture = 0; - int32_t alphaTest = 0; - int32_t foliageSway = 0; - float windTime = 0.0f; - float foliageMotionDamp = 1.0f; - }; - // Create ShadowParams UBO VkBufferCreateInfo bufCI{}; bufCI.sType = VK_STRUCTURE_TYPE_BUFFER_CREATE_INFO; From 4be7910fdf1730708c3728f0c6bab1858cabbaef Mon Sep 17 00:00:00 2001 From: Kelsi Date: Wed, 11 Mar 2026 11:42:01 -0700 Subject: [PATCH 36/36] refactor: consolidate QueryTimer struct to shared header Move QueryTimer from m2_renderer.cpp and wmo_renderer.cpp to vk_frame_data.hpp for reuse. Removes 13 lines of duplicate code. --- include/rendering/vk_frame_data.hpp | 18 ++++++++++++++++++ src/rendering/m2_renderer.cpp | 16 ---------------- src/rendering/wmo_renderer.cpp | 19 ++----------------- 3 files changed, 20 insertions(+), 33 deletions(-) diff --git a/include/rendering/vk_frame_data.hpp b/include/rendering/vk_frame_data.hpp index 114e0f05..482e76e7 100644 --- a/include/rendering/vk_frame_data.hpp +++ b/include/rendering/vk_frame_data.hpp @@ -2,6 +2,7 @@ #include #include +#include namespace wowee { namespace rendering { @@ -41,5 +42,22 @@ struct ShadowParamsUBO { float foliageMotionDamp; }; +// Timer utility for performance profiling queries +struct QueryTimer { + double* totalMs = nullptr; + uint32_t* callCount = nullptr; + std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); + QueryTimer(double* total, uint32_t* calls) : totalMs(total), callCount(calls) {} + ~QueryTimer() { + if (callCount) { + (*callCount)++; + } + if (totalMs) { + auto end = std::chrono::steady_clock::now(); + *totalMs += std::chrono::duration(end - start).count(); + } + } +}; + } // namespace rendering } // namespace wowee diff --git a/src/rendering/m2_renderer.cpp b/src/rendering/m2_renderer.cpp index a2dfe54a..332b8849 100644 --- a/src/rendering/m2_renderer.cpp +++ b/src/rendering/m2_renderer.cpp @@ -192,22 +192,6 @@ float pointAABBDistanceSq(const glm::vec3& p, const glm::vec3& bmin, const glm:: return glm::dot(d, d); } -struct QueryTimer { - double* totalMs = nullptr; - uint32_t* callCount = nullptr; - std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); - QueryTimer(double* total, uint32_t* calls) : totalMs(total), callCount(calls) {} - ~QueryTimer() { - if (callCount) { - (*callCount)++; - } - if (totalMs) { - auto end = std::chrono::steady_clock::now(); - *totalMs += std::chrono::duration(end - start).count(); - } - } -}; - // Möller–Trumbore ray-triangle intersection. // Returns distance along ray if hit, negative if miss. float rayTriangleIntersect(const glm::vec3& origin, const glm::vec3& dir, diff --git a/src/rendering/wmo_renderer.cpp b/src/rendering/wmo_renderer.cpp index 3401cc07..fb635803 100644 --- a/src/rendering/wmo_renderer.cpp +++ b/src/rendering/wmo_renderer.cpp @@ -2492,22 +2492,6 @@ static float pointAABBDistanceSq(const glm::vec3& p, const glm::vec3& bmin, cons return glm::dot(d, d); } -struct QueryTimer { - double* totalMs = nullptr; - uint32_t* callCount = nullptr; - std::chrono::steady_clock::time_point start = std::chrono::steady_clock::now(); - QueryTimer(double* total, uint32_t* calls) : totalMs(total), callCount(calls) {} - ~QueryTimer() { - if (callCount) { - (*callCount)++; - } - if (totalMs) { - auto end = std::chrono::steady_clock::now(); - *totalMs += std::chrono::duration(end - start).count(); - } - } -}; - // Möller–Trumbore ray-triangle intersection // Returns distance along ray if hit, or negative if miss static float rayTriangleIntersect(const glm::vec3& origin, const glm::vec3& dir, @@ -3599,12 +3583,13 @@ void WMORenderer::recreatePipelines() { } // --- Vertex input --- + // WMO vertex: pos3 + normal3 + texCoord2 + color4 + tangent4 = 64 bytes struct WMOVertexData { glm::vec3 position; glm::vec3 normal; glm::vec2 texCoord; glm::vec4 color; - glm::vec4 tangent; + glm::vec4 tangent; // xyz=tangent dir, w=handedness ±1 }; VkVertexInputBindingDescription vertexBinding{};