From faa887c05eb7ee32c0bab96ca4a61f0144f5611a Mon Sep 17 00:00:00 2001 From: Lasse Nielsen Date: Sun, 1 Mar 2026 20:37:28 +0100 Subject: [PATCH] fix: name-based talent matching for TBC Anniversary index reordering (#12) The TBC Anniversary client orders talents by internal talentID rather than row/column grid position. This caused 15 of 18 Warlock TalentMap keys to point to wrong talents, producing incorrect rank readings (e.g. "Improved Life Tap 5/2") and wildly inflated calculations. - Rewrite StateCollector.CollectTalents to match talents by name instead of positional index, making it immune to reordering - Add defensive maxRank clamping in StateCollector - Fix all Warlock TalentMap indices to match in-game verified data - Add grid layout documentation to TalentMap_Warlock.lua - Add 8 unit tests for name-based matching logic - Update existing Warlock test files with corrected keys Fixes #12 --- AGENTS.md | 16 ++ Core/StateCollector.lua | 27 ++- Data/TalentMap_Warlock.lua | 220 ++++++++++-------- tests/test_new_talents.lua | 62 ++--- tests/test_statecollector_talents.lua | 314 ++++++++++++++++++++++++++ tests/test_warlock_completion.lua | 10 +- 6 files changed, 520 insertions(+), 129 deletions(-) create mode 100644 tests/test_statecollector_talents.lua diff --git a/AGENTS.md b/AGENTS.md index b1d043e..960d73d 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -198,3 +198,19 @@ PhDamage uses the **DragonAddons** org-level GitHub project board (#2) for issue 3. **Start** - Move to *In progress*, create a feature branch, add a comment. 4. **Review** - Open PR, move to *In review*, link the issue. 5. **Ship** - Squash-merge, auto-move to *Done* on close. + +--- + +## Communication Style + +When responding to or commenting on issues, always write in **first-person singular** ("I") +as the repo owner -- never use "we" or "our team". Speak as if you are the developer personally. + +**Writing style:** +- Direct, structured, solution-driven. Get to the point fast. Text is a tool, not decoration. +- Think in systems. Break things into flows, roles, rules, and frameworks. +- Bias toward precision. Concrete output, copy-paste-ready solutions, clear constraints. Low + tolerance for fluff. +- Tone is calm and rational with small flashes of humor and self-awareness. +- When confident in a topic, become more informal and creative. +- When something matters, become sharp and focused. diff --git a/Core/StateCollector.lua b/Core/StateCollector.lua index ff0ac96..7ad97c5 100644 --- a/Core/StateCollector.lua +++ b/Core/StateCollector.lua @@ -344,14 +344,37 @@ function StateCollector.CollectPlayerState() end function StateCollector.CollectTalents(state) + -- Build reverse lookup from talent name -> TalentMap key for this class. + -- This makes talent collection immune to index reordering across client + -- versions (e.g. TBC Anniversary orders by internal talentID, not grid position). + local nameToKey = {} + local classPrefix = (state.class or "UNKNOWN") .. ":" + for mapKey, entry in pairs(ns.TalentMap) do + if mapKey:sub(1, #classPrefix) == classPrefix then + local talentKey = mapKey:sub(#classPrefix + 1) + nameToKey[entry.name] = talentKey + end + end + local numTabs = GetNumTalentTabs and GetNumTalentTabs() or 0 for tab = 1, numTabs do local numTalents = GetNumTalents and GetNumTalents(tab) or 0 for index = 1, numTalents do local ok, name, _, _, _, rank = pcall(GetTalentInfo, tab, index) if ok and name and rank and rank > 0 then - local key = tab .. ":" .. index - state.talents[key] = rank + local mappedKey = nameToKey[name] + if mappedKey then + -- Clamp rank to maxRank as defense-in-depth + local entry = ns.TalentMap[classPrefix .. mappedKey] + if entry and entry.maxRank and rank > entry.maxRank then + rank = entry.maxRank + end + state.talents[mappedKey] = rank + else + -- Untracked talent: store with raw key for diagnostics visibility + local rawKey = tab .. ":" .. index + state.talents[rawKey] = rank + end end end end diff --git a/Data/TalentMap_Warlock.lua b/Data/TalentMap_Warlock.lua index f674a53..841f9fa 100644 --- a/Data/TalentMap_Warlock.lua +++ b/Data/TalentMap_Warlock.lua @@ -1,7 +1,7 @@ ------------------------------------------------------------------------------- -- TalentMap_Warlock.lua -- Warlock talent effects mapped to modifier descriptors for TBC Anniversary --- Talent positions (tab:index) sourced from Wowhead TBC Classic talent calculator +-- Talent positions (tab:index) verified in-game on TBC Anniversary (ordered by internal talentID) -- -- Supported versions: TBC Anniversary ------------------------------------------------------------------------------- @@ -16,28 +16,23 @@ local TalentMap = {} ------------------------------------------------------------------------------- -- Tab 1: Affliction +-- +-- In-game ordering (by internal talentID): +-- 1:1 = Fel Concentration (5) 1:2 = Nightfall (2) +-- 1:3 = Improved Corruption (5) 1:4 = Soul Siphon (2) +-- 1:5 = Suppression (5) 1:6 = Improved Curse of Weakness (2) +-- 1:7 = Improved Life Tap (2) 1:8 = Grim Reach (2) +-- 1:9 = Dark Pact (1) 1:10 = Siphon Life (1) +-- 1:11 = Shadow Mastery (5) 1:12 = Amplify Curse (1) +-- 1:13 = Curse of Exhaustion (1) 1:14 = Improved Drain Soul (2) +-- 1:15 = Improved Curse of Agony (2) 1:16 = Malediction (3) +-- 1:17 = Improved Howl of Terror (2) 1:18 = Contagion (5) +-- 1:19 = Unstable Affliction (1) 1:20 = Shadow Embrace (5) +-- 1:21 = Empowered Corruption (3) ------------------------------------------------------------------------------- --- Suppression: +1% spell hit to Affliction (Shadow) spells per rank -TalentMap["1:1"] = { - name = "Suppression", - maxRank = 5, - effects = { - { type = MOD.SPELL_HIT_BONUS, value = 0.01, perRank = true, - filter = { spellNames = { - "Corruption", "Curse of Agony", "Curse of Doom", - "Unstable Affliction", "Siphon Life", "Drain Life", - "Drain Soul", "Death Coil", "Seed of Corruption", - "Fear", "Howl of Terror", "Curse of Tongues", - "Curse of Weakness", "Curse of Recklessness", - "Curse of the Elements", "Curse of Shadow", - "Drain Mana" - } } }, - }, -} - -- Improved Corruption: -0.4s cast time on Corruption per rank (5 ranks = instant) -TalentMap["1:2"] = { +TalentMap["1:3"] = { name = "Improved Corruption", maxRank = 5, effects = { @@ -46,18 +41,8 @@ TalentMap["1:2"] = { }, } --- Improved Life Tap: +10/20% mana from Life Tap (Affliction 1:3, 2 ranks) -TalentMap["1:3"] = { - name = "Improved Life Tap", - maxRank = 2, - effects = { - { type = MOD.DAMAGE_MULTIPLIER, value = 0.10, perRank = true, stacking = "additive", - filter = { spellNames = {"Life Tap"} } }, - }, -} - -- Soul Siphon: +2/4% damage per Affliction effect on target for Drain Life/Soul (capped) -TalentMap["1:5"] = { +TalentMap["1:4"] = { name = "Soul Siphon", maxRank = 2, effects = { @@ -71,30 +56,38 @@ TalentMap["1:5"] = { }, } --- Improved Curse of Agony: +5% Curse of Agony damage per rank -TalentMap["1:6"] = { - name = "Improved Curse of Agony", - maxRank = 2, +-- Suppression: +1% spell hit to Affliction (Shadow) spells per rank +TalentMap["1:5"] = { + name = "Suppression", + maxRank = 5, effects = { - { type = MOD.DAMAGE_MULTIPLIER, value = 0.05, perRank = true, stacking = "additive", - filter = { spellNames = {"Curse of Agony"} } }, + { type = MOD.SPELL_HIT_BONUS, value = 0.01, perRank = true, + filter = { spellNames = { + "Corruption", "Curse of Agony", "Curse of Doom", + "Unstable Affliction", "Siphon Life", "Drain Life", + "Drain Soul", "Death Coil", "Seed of Corruption", + "Fear", "Howl of Terror", "Curse of Tongues", + "Curse of Weakness", "Curse of Recklessness", + "Curse of the Elements", "Curse of Shadow", + "Drain Mana" + } } }, }, } --- Empowered Corruption: +12% of SP added to Corruption coefficient per rank -TalentMap["1:11"] = { - name = "Empowered Corruption", - maxRank = 3, +-- Improved Life Tap: +10/20% mana from Life Tap (Affliction 1:7, 2 ranks) +TalentMap["1:7"] = { + name = "Improved Life Tap", + maxRank = 2, effects = { - { type = MOD.COEFFICIENT_BONUS, value = 0.12, perRank = true, - filter = { spellNames = {"Corruption"} } }, + { type = MOD.DAMAGE_MULTIPLIER, value = 0.10, perRank = true, stacking = "additive", + filter = { spellNames = {"Life Tap"} } }, }, } --- Siphon Life (1:13): 1 rank, enables the spell. No damage modifier. +-- Siphon Life (1:10): 1 rank, enables the spell. No damage modifier. -- Shadow Mastery: +2% Shadow damage per rank -TalentMap["1:15"] = { +TalentMap["1:11"] = { name = "Shadow Mastery", maxRank = 5, effects = { @@ -103,8 +96,27 @@ TalentMap["1:15"] = { }, } --- Contagion: +1% damage to Corruption, Seed of Corruption, and Curse of Agony per rank +-- Improved Curse of Agony: +5% Curse of Agony damage per rank +TalentMap["1:15"] = { + name = "Improved Curse of Agony", + maxRank = 2, + effects = { + { type = MOD.DAMAGE_MULTIPLIER, value = 0.05, perRank = true, stacking = "additive", + filter = { spellNames = {"Curse of Agony"} } }, + }, +} + +-- Malediction: +1/2/3% to Curse of the Elements and Curse of Shadow damage amplification +-- The actual amplification is handled via talentAmplify in AuraMap entries for those curses; +-- this entry provides the talent definition for rank lookup. TalentMap["1:16"] = { + name = "Malediction", + maxRank = 3, + effects = {}, +} + +-- Contagion: +1% damage to Corruption, Seed of Corruption, and Curse of Agony per rank +TalentMap["1:18"] = { name = "Contagion", maxRank = 5, effects = { @@ -113,19 +125,33 @@ TalentMap["1:16"] = { }, } --- Unstable Affliction (1:21): 1 rank, enables the spell. No damage modifier. +-- Unstable Affliction (1:19): 1 rank, enables the spell. No damage modifier. --- Malediction: +1/2/3% to Curse of the Elements and Curse of Shadow damage amplification --- The actual amplification is handled via talentAmplify in AuraMap entries for those curses; --- this entry provides the talent definition for rank lookup. -TalentMap["1:19"] = { - name = "Malediction", +-- Empowered Corruption: +12% of SP added to Corruption coefficient per rank +TalentMap["1:21"] = { + name = "Empowered Corruption", maxRank = 3, - effects = {}, + effects = { + { type = MOD.COEFFICIENT_BONUS, value = 0.12, perRank = true, + filter = { spellNames = {"Corruption"} } }, + }, } ------------------------------------------------------------------------------- -- Tab 2: Demonology +-- +-- In-game ordering (by internal talentID): +-- 2:1 = Improved Healthstone (2) 2:2 = Improved Imp (3) +-- 2:3 = Demonic Embrace (5) 2:4 = Improved Health Funnel (2) +-- 2:5 = Improved Voidwalker (3) 2:6 = Fel Domination (1) +-- 2:7 = Master Summoner (2) 2:8 = Fel Stamina (3) +-- 2:9 = Fel Intellect (3) 2:10 = Improved Sayaad (3) +-- 2:11 = Master Demonologist (5) 2:12 = Master Conjuror (2) +-- 2:13 = Unholy Power (5) 2:14 = Demonic Knowledge (3) +-- 2:15 = Demonic Sacrifice (1) 2:16 = Soul Link (1) +-- 2:17 = Improved Subjugate Demon (2) 2:18 = Demonic Aegis (3) +-- 2:19 = Summon Felguard (1) 2:20 = Demonic Tactics (5) +-- 2:21 = Demonic Resilience (3) 2:22 = Mana Feed (3) ------------------------------------------------------------------------------- -- Improved Health Funnel: +10/20% healing (Demonology 2:4, 2 ranks) @@ -140,25 +166,24 @@ TalentMap["2:4"] = { -- Master Demonologist: pet-dependent damage buff. Actual values come from AuraMap entries -- (spellIDs 23761/35702) which use talentAmplify to scale by this talent's rank. -TalentMap["2:16"] = { +TalentMap["2:11"] = { name = "Master Demonologist", maxRank = 5, effects = {}, } --- Demonic Sacrifice (2:18): 1 rank — sacrifice pet for a damage buff that depends on pet +-- Demonic Sacrifice (2:15): 1 rank - sacrifice pet for a damage buff that depends on pet -- type (Imp = +15% Fire, Succubus = +15% Shadow, etc.). Skipped for Phase 1 because it -- requires tracking which pet was sacrificed. TODO: implement pet-sacrifice state tracking. --- Demonic Knowledge (2:18): 3 ranks — +4% of pet (Stamina + Intellect) as spell damage per +-- Demonic Knowledge (2:14): 3 ranks - +4% of pet (Stamina + Intellect) as spell damage per -- rank. Skipped for Phase 1 because it requires reading live pet stats. -- TODO: integrate with pet stat snapshot from StateCollector. ------------------------------------------------------------------------------- --- Demonic Tactics (Demonology, Tier 9) — +1/2/3/4/5% crit to all spells --- Talent index 2:19 is a placeholder — needs in-game verification +-- Demonic Tactics (Demonology, Tier 9) - +1/2/3/4/5% crit to all spells ------------------------------------------------------------------------------- -TalentMap["2:19"] = { +TalentMap["2:20"] = { name = "Demonic Tactics", maxRank = 5, effects = { @@ -168,14 +193,23 @@ TalentMap["2:19"] = { ------------------------------------------------------------------------------- -- Tab 3: Destruction +-- +-- In-game ordering (by internal talentID): +-- 3:1 = Cataclysm (5) 3:2 = Bane (5) +-- 3:3 = Improved Shadow Bolt (5) 3:4 = Improved Immolate (5) +-- 3:5 = Shadowburn (1) 3:6 = Destructive Reach (2) +-- 3:7 = Improved Searing Pain (3) 3:8 = Emberstorm (5) +-- 3:9 = Ruin (1) 3:10 = Conflagrate (1) +-- 3:11 = Devastation (5) 3:12 = Aftermath (5) +-- 3:13 = Improved Firebolt (2) 3:14 = Improved Lash of Pain (2) +-- 3:15 = Intensity (2) 3:16 = Pyroclasm (2) +-- 3:17 = Shadowfury (1) 3:18 = Shadow and Flame (5) +-- 3:19 = Soul Leech (3) 3:20 = Nether Protection (3) +-- 3:21 = Backlash (3) ------------------------------------------------------------------------------- --- Improved Shadow Bolt (3:1): 5 ranks — Shadow Bolt crits apply a debuff increasing Shadow --- damage taken by 4% per stack (up to 5 stacks). The debuff itself is modeled in AuraMap; --- this talent merely enables it. No modifier entry needed here. - -- Bane: -0.1s cast time to Shadow Bolt and Immolate per rank, -0.4s to Soul Fire per rank -TalentMap["3:3"] = { +TalentMap["3:2"] = { name = "Bane", maxRank = 5, effects = { @@ -186,6 +220,10 @@ TalentMap["3:3"] = { }, } +-- Improved Shadow Bolt (3:3): 5 ranks - Shadow Bolt crits apply a debuff increasing Shadow +-- damage taken by 4% per stack (up to 5 stacks). The debuff itself is modeled in AuraMap; +-- this talent merely enables it. No modifier entry needed here. + -- Improved Immolate: +5% Immolate direct damage per rank TalentMap["3:4"] = { name = "Improved Immolate", @@ -199,19 +237,8 @@ TalentMap["3:4"] = { }, } --- Devastation: +1% crit chance to Destruction spells per rank -TalentMap["3:7"] = { - name = "Devastation", - maxRank = 5, - effects = { - { type = MOD.CRIT_BONUS, value = 0.01, perRank = true, - filter = { spellNames = {"Shadow Bolt", "Shadowburn", "Searing Pain", "Soul Fire", - "Incinerate", "Conflagrate", "Shadowfury", "Immolate"} } }, - }, -} - -- Improved Searing Pain: +4/7/10% crit to Searing Pain per rank -TalentMap["3:11"] = { +TalentMap["3:7"] = { name = "Improved Searing Pain", maxRank = 3, effects = { @@ -220,8 +247,18 @@ TalentMap["3:11"] = { }, } +-- Emberstorm: +2% Fire damage per rank +TalentMap["3:8"] = { + name = "Emberstorm", + maxRank = 5, + effects = { + { type = MOD.DAMAGE_MULTIPLIER, value = 0.02, perRank = true, stacking = "additive", + filter = { school = SCHOOL_FIRE } }, + }, +} + -- Ruin: Destruction spell crits deal 200% damage instead of 150% (+0.5 crit multiplier) -TalentMap["3:13"] = { +TalentMap["3:9"] = { name = "Ruin", maxRank = 1, effects = { @@ -231,22 +268,14 @@ TalentMap["3:13"] = { }, } --- Emberstorm: +2% Fire damage per rank -TalentMap["3:14"] = { - name = "Emberstorm", +-- Devastation: +1% crit chance to Destruction spells per rank +TalentMap["3:11"] = { + name = "Devastation", maxRank = 5, effects = { - { type = MOD.DAMAGE_MULTIPLIER, value = 0.02, perRank = true, stacking = "additive", - filter = { school = SCHOOL_FIRE } }, - }, -} - --- Backlash: +1% crit to all spells per rank (no school filter) -TalentMap["3:15"] = { - name = "Backlash", - maxRank = 3, - effects = { - { type = MOD.CRIT_BONUS, value = 0.01, perRank = true }, + { type = MOD.CRIT_BONUS, value = 0.01, perRank = true, + filter = { spellNames = {"Shadow Bolt", "Shadowburn", "Searing Pain", "Soul Fire", + "Incinerate", "Conflagrate", "Shadowfury", "Immolate"} } }, }, } @@ -260,6 +289,15 @@ TalentMap["3:18"] = { }, } +-- Backlash: +1% crit to all spells per rank (no school filter) +TalentMap["3:21"] = { + name = "Backlash", + maxRank = 3, + effects = { + { type = MOD.CRIT_BONUS, value = 0.01, perRank = true }, + }, +} + for key, data in pairs(TalentMap) do ns.TalentMap["WARLOCK:" .. key] = data end diff --git a/tests/test_new_talents.lua b/tests/test_new_talents.lua index c9b3a13..f5c2719 100644 --- a/tests/test_new_talents.lua +++ b/tests/test_new_talents.lua @@ -113,7 +113,7 @@ describe("Master Demonologist", function() describe("Succubus (23761)", function() it("aura + talent 5/5 should give +10% damageMultiplier", function() - playerState.talents["2:16"] = 5 + playerState.talents["2:11"] = 5 playerState.auras.player[23761] = true local spellData = ns.SpellData[686] local rankData = spellData.ranks[11] @@ -127,7 +127,7 @@ describe("Master Demonologist", function() end) it("aura + talent 3/5 should give +6% damageMultiplier", function() - playerState.talents["2:16"] = 3 + playerState.talents["2:11"] = 3 playerState.auras.player[23761] = true local spellData = ns.SpellData[686] local rankData = spellData.ranks[11] @@ -154,7 +154,7 @@ describe("Master Demonologist", function() end) it("talent set but no aura should give no bonus", function() - playerState.talents["2:16"] = 5 + playerState.talents["2:11"] = 5 -- No aura active local spellData = ns.SpellData[686] local rankData = spellData.ranks[11] @@ -167,7 +167,7 @@ describe("Master Demonologist", function() end) it("should apply to all schools (Fire spell too)", function() - playerState.talents["2:16"] = 5 + playerState.talents["2:11"] = 5 playerState.auras.player[23761] = true local spellData = ns.SpellData[5676] -- Searing Pain (Fire) local rankData = spellData.ranks[8] @@ -182,7 +182,7 @@ describe("Master Demonologist", function() describe("Felguard (35702)", function() it("aura + talent 5/5 should give +5% damageMultiplier", function() - playerState.talents["2:16"] = 5 + playerState.talents["2:11"] = 5 playerState.auras.player[35702] = true local spellData = ns.SpellData[686] local rankData = spellData.ranks[11] @@ -196,7 +196,7 @@ describe("Master Demonologist", function() end) it("should differ from Succubus value (0.01 vs 0.02 per rank)", function() - playerState.talents["2:16"] = 5 + playerState.talents["2:11"] = 5 -- Calculate with Felguard aura playerState.auras.player[35702] = true local spellData = ns.SpellData[686] @@ -208,7 +208,7 @@ describe("Master Demonologist", function() -- Calculate with Succubus aura (separate state) local ps2 = makePlayerState() - ps2.talents["2:16"] = 5 + ps2.talents["2:11"] = 5 ps2.auras.player[23761] = true local baseResult2 = SpellCalc.ComputeBase(spellData, rankData, ps2) local _, modsSu = ModifierCalc.ApplyModifiers( @@ -226,7 +226,7 @@ describe("Master Demonologist", function() it("Succubus 5/5 should boost Shadow Bolt total damage by 10%", function() local r0 = Pipeline.Calculate(686, playerState) - playerState.talents["2:16"] = 5 + playerState.talents["2:11"] = 5 playerState.auras.player[23761] = true local r5 = Pipeline.Calculate(686, playerState) @@ -236,9 +236,9 @@ describe("Master Demonologist", function() end) ------------------------------------------------------------------------------- --- Soul Siphon (talent 1:5) — count-based multiplier for Drain Life/Drain Soul +-- Soul Siphon (talent 1:4) — count-based multiplier for Drain Life/Drain Soul ------------------------------------------------------------------------------- -describe("Soul Siphon (1:5)", function() +describe("Soul Siphon (1:4)", function() local playerState local SpellCalc = ns.Engine.SpellCalc local ModifierCalc = ns.Engine.ModifierCalc @@ -250,7 +250,7 @@ describe("Soul Siphon (1:5)", function() describe("modifier accumulator (Drain Life)", function() local function getMods(talentRank, afflictionCount) - playerState.talents["1:5"] = talentRank + playerState.talents["1:4"] = talentRank playerState.afflictionCountOnTarget = afflictionCount local spellData = ns.SpellData[689] -- Drain Life local rankData = spellData.ranks[8] @@ -312,7 +312,7 @@ describe("Soul Siphon (1:5)", function() describe("spell filtering", function() it("should NOT affect Shadow Bolt (filter is Drain Life/Drain Soul only)", function() - playerState.talents["1:5"] = 2 + playerState.talents["1:4"] = 2 playerState.afflictionCountOnTarget = 5 local spellData = ns.SpellData[686] -- Shadow Bolt local rankData = spellData.ranks[11] @@ -325,7 +325,7 @@ describe("Soul Siphon (1:5)", function() end) it("should affect Drain Soul", function() - playerState.talents["1:5"] = 2 + playerState.talents["1:4"] = 2 playerState.afflictionCountOnTarget = 3 local spellData = ns.SpellData[1120] -- Drain Soul local rankData = spellData.ranks[5] @@ -339,7 +339,7 @@ describe("Soul Siphon (1:5)", function() end) it("should NOT affect Corruption", function() - playerState.talents["1:5"] = 2 + playerState.talents["1:4"] = 2 playerState.afflictionCountOnTarget = 5 local spellData = ns.SpellData[172] -- Corruption local rankData = spellData.ranks[8] @@ -356,7 +356,7 @@ describe("Soul Siphon (1:5)", function() it("rank 2 + 3 afflictions should increase Drain Life damage by 12%", function() local r0 = Pipeline.Calculate(689, playerState) - playerState.talents["1:5"] = 2 + playerState.talents["1:4"] = 2 playerState.afflictionCountOnTarget = 3 local r1 = Pipeline.Calculate(689, playerState) @@ -366,7 +366,7 @@ describe("Soul Siphon (1:5)", function() it("rank 2 + 15 afflictions should cap Drain Life at +60%", function() local r0 = Pipeline.Calculate(689, playerState) - playerState.talents["1:5"] = 2 + playerState.talents["1:4"] = 2 playerState.afflictionCountOnTarget = 15 local r1 = Pipeline.Calculate(689, playerState) @@ -376,7 +376,7 @@ describe("Soul Siphon (1:5)", function() it("rank 1 + 12 afflictions should cap Drain Life at +24%", function() local r0 = Pipeline.Calculate(689, playerState) - playerState.talents["1:5"] = 1 + playerState.talents["1:4"] = 1 playerState.afflictionCountOnTarget = 12 local r1 = Pipeline.Calculate(689, playerState) @@ -387,7 +387,7 @@ describe("Soul Siphon (1:5)", function() it("Drain Soul pipeline with rank 2 + 3 afflictions", function() local r0 = Pipeline.Calculate(1120, playerState) - playerState.talents["1:5"] = 2 + playerState.talents["1:4"] = 2 playerState.afflictionCountOnTarget = 3 local r1 = Pipeline.Calculate(1120, playerState) @@ -395,7 +395,7 @@ describe("Soul Siphon (1:5)", function() end) it("Shadow Bolt pipeline should be unaffected by Soul Siphon", function() - playerState.talents["1:5"] = 2 + playerState.talents["1:4"] = 2 playerState.afflictionCountOnTarget = 10 local r0 = Pipeline.Calculate(686, makePlayerState()) local r1 = Pipeline.Calculate(686, playerState) @@ -406,7 +406,7 @@ describe("Soul Siphon (1:5)", function() end) ------------------------------------------------------------------------------- --- Demonic Tactics (talent 2:19) — +1% crit per rank, 5 ranks max +-- Demonic Tactics (talent 2:20) — +1% crit per rank, 5 ranks max ------------------------------------------------------------------------------- describe("Demonic Tactics", function() local playerState @@ -420,7 +420,7 @@ describe("Demonic Tactics", function() describe("modifier accumulation", function() it("should add 1% crit at rank 1", function() - playerState.talents["2:19"] = 1 + playerState.talents["2:20"] = 1 -- Test with Shadow Bolt (686) local spellData = ns.SpellData[686] local rankData = spellData.ranks[1] @@ -432,7 +432,7 @@ describe("Demonic Tactics", function() end) it("should add 3% crit at rank 3", function() - playerState.talents["2:19"] = 3 + playerState.talents["2:20"] = 3 local spellData = ns.SpellData[686] local rankData = spellData.ranks[1] local base = SpellCalc.ComputeBase(spellData, rankData, playerState) @@ -443,7 +443,7 @@ describe("Demonic Tactics", function() end) it("should add 5% crit at rank 5", function() - playerState.talents["2:19"] = 5 + playerState.talents["2:20"] = 5 local spellData = ns.SpellData[686] local rankData = spellData.ranks[1] local base = SpellCalc.ComputeBase(spellData, rankData, playerState) @@ -454,7 +454,7 @@ describe("Demonic Tactics", function() end) it("should apply to fire spells too (no filter)", function() - playerState.talents["2:19"] = 5 + playerState.talents["2:20"] = 5 -- Test with Searing Pain (5676) - fire spell local spellData = ns.SpellData[5676] local rankData = spellData.ranks[1] @@ -471,38 +471,38 @@ describe("Demonic Tactics", function() -- Without talent local resultBase = Pipeline.Calculate(686, playerState) -- With talent rank 5 - playerState.talents["2:19"] = 5 + playerState.talents["2:20"] = 5 local resultTalent = Pipeline.Calculate(686, playerState) assert.is_true(resultTalent.expectedDamage > resultBase.expectedDamage) assert.is_near(resultBase.critChance + 0.05, resultTalent.critChance, 0.001) end) it("should stack with Devastation crit bonus", function() - -- Devastation (3:7) gives +5% crit to shadow/fire - playerState.talents["3:7"] = 5 + -- Devastation (3:11) gives +5% crit to shadow/fire + playerState.talents["3:11"] = 5 local resultDev = Pipeline.Calculate(686, playerState) -- Add Demonic Tactics rank 5 - playerState.talents["2:19"] = 5 + playerState.talents["2:20"] = 5 local resultBoth = Pipeline.Calculate(686, playerState) -- Should have +10% total crit bonus assert.is_near(resultDev.critChance + 0.05, resultBoth.critChance, 0.001) end) it("should affect Shadow Bolt expected damage correctly", function() - playerState.talents["2:19"] = 3 -- +3% crit + playerState.talents["2:20"] = 3 -- +3% crit local result = Pipeline.Calculate(686, playerState) -- Base crit is 0.10 (from playerState), +0.03 from talent = 0.13 assert.is_near(0.13, result.critChance, 0.001) end) it("should affect Immolate (hybrid) crit", function() - playerState.talents["2:19"] = 2 -- +2% crit + playerState.talents["2:20"] = 2 -- +2% crit local result = Pipeline.Calculate(348, playerState) assert.is_near(0.12, result.critChance, 0.001) end) it("should not affect canCrit=false spells (Corruption)", function() - playerState.talents["2:19"] = 5 + playerState.talents["2:20"] = 5 local result = Pipeline.Calculate(172, playerState) -- Corruption has canCrit=false, so critChance should be 0 assert.are.equal(0, result.critChance) diff --git a/tests/test_statecollector_talents.lua b/tests/test_statecollector_talents.lua new file mode 100644 index 0000000..6fbd217 --- /dev/null +++ b/tests/test_statecollector_talents.lua @@ -0,0 +1,314 @@ +------------------------------------------------------------------------------- +-- test_statecollector_talents.lua +-- Unit tests for StateCollector.CollectTalents name-based matching logic +------------------------------------------------------------------------------- + +------------------------------------------------------------------------------- +-- WoW API global stubs +-- StateCollector.lua caches these at module load time (lines 12-36), +-- so they must exist BEFORE we loadfile the module. +------------------------------------------------------------------------------- + +-- Shared mock state table. Closures reference this single table so that +-- clearing its fields in resetMocks() is visible to the cached locals +-- inside StateCollector (which capture the function references at load time). +local mockState = { + talentData = {}, -- [tab][index] = { name, iconTexture, tier, column, rank, maxRank, ... } + numTabs = 0, + numTalentsPerTab = {}, +} + +_G.GetTalentInfo = function(tab, index) + local t = mockState.talentData[tab] + if not t or not t[index] then return nil end + local d = t[index] + return d.name, d.iconTexture, d.tier, d.column, d.rank, d.maxRank, d.isExceptional, d.available +end + +_G.GetNumTalentTabs = function() + return mockState.numTabs +end + +_G.GetNumTalents = function(tab) + return mockState.numTalentsPerTab[tab] or 0 +end + +-- Other globals cached by StateCollector at load time +_G.UnitLevel = function() return 70 end +_G.UnitClass = function() return "Warlock", "WARLOCK" end +_G.GetSpellBonusDamage = function() return 0 end +_G.GetSpellBonusHealing = function() return 0 end +_G.GetSpellCritChance = function() return 0 end +_G.GetCombatRatingBonus = function() return 0 end +_G.GetManaRegen = function() return 0, 0 end +_G.UnitRangedAttackPower = function() return 0, 0, 0 end +_G.GetRangedCritChance = function() return 0 end +_G.UnitCreatureType = function() return "Humanoid" end +_G.UnitExists = function() return false end +_G.UnitHealth = function() return 100 end +_G.UnitHealthMax = function() return 100 end +_G.UnitCanAttack = function() return false end +_G.UnitRangedDamage = function() return 0, 0, 0 end +_G.UnitAttackPower = function() return 0, 0, 0 end +_G.GetCritChance = function() return 0 end +_G.GetExpertise = function() return 0 end +_G.UnitDamage = function() return 0, 0, 0, 0, 0, 0, 0 end +_G.UnitAttackSpeed = function() return 2.0, 2.0 end +_G.GetInventoryItemLink = function() return nil end +_G.GetItemInfo = function() return nil end +_G.UnitStat = function() return 0, 0 end +_G.Enum = Enum or {} +_G.C_UnitAuras = nil -- CollectAuras is not under test; nil avoids scanning + +------------------------------------------------------------------------------- +-- Namespace setup - minimal ns with only what StateCollector needs +------------------------------------------------------------------------------- +local ns = {} + +-- Load Constants.lua to populate ns.SCHOOL_*, ns.MOD, ns.CR_*, ns.SCALING_TYPE +local fn, err = loadfile("Core/Constants.lua") +if not fn then error("Failed to load Core/Constants.lua: " .. tostring(err)) end +fn("PhDamage", ns) + +ns.SpellData = {} +ns.AuraMap = {} +ns.TalentMap = {} +ns.Engine = {} + +------------------------------------------------------------------------------- +-- Load StateCollector.lua into ns +------------------------------------------------------------------------------- +local scFn, scErr = loadfile("Core/StateCollector.lua") +if not scFn then error("Failed to load Core/StateCollector.lua: " .. tostring(scErr)) end +scFn("PhDamage", ns) + +local StateCollector = ns.StateCollector + +------------------------------------------------------------------------------- +-- Helper: reset mock state between tests +------------------------------------------------------------------------------- +local function resetMocks() + mockState.talentData = {} + mockState.numTabs = 0 + mockState.numTalentsPerTab = {} + ns.TalentMap = {} +end + +------------------------------------------------------------------------------- +-- Helper: create a talent entry in the mock API +------------------------------------------------------------------------------- +local function setTalent(tab, index, name, rank, maxRank) + if not mockState.talentData[tab] then mockState.talentData[tab] = {} end + mockState.talentData[tab][index] = { + name = name, + iconTexture = "Interface\\Icons\\Spell_Shadow_ShadowBolt", + tier = 1, + column = 1, + rank = rank, + maxRank = maxRank or 5, + isExceptional = false, + available = true, + } + -- Update tab count and max talent index per tab + -- CollectTalents iterates `for index = 1, GetNumTalents(tab)` so we need + -- the max index, not the entry count. + if tab > mockState.numTabs then mockState.numTabs = tab end + local maxIndex = mockState.numTalentsPerTab[tab] or 0 + if index > maxIndex then mockState.numTalentsPerTab[tab] = index end +end + +------------------------------------------------------------------------------- +-- Tests +------------------------------------------------------------------------------- +describe("StateCollector.CollectTalents", function() + before_each(function() + resetMocks() + end) + + ----------------------------------------------------------------------- + -- Test 1: Name-based matching maps talents to TalentMap keys + ----------------------------------------------------------------------- + it("maps talents by name rather than API index", function() + -- TalentMap says "Improved Life Tap" is at key WARLOCK:1:7 + ns.TalentMap["WARLOCK:1:7"] = { + name = "Improved Life Tap", + maxRank = 2, + effects = {}, + } + + -- But the WoW API returns "Improved Life Tap" at tab=1, index=3 + -- (simulating a client reorder) + setTalent(1, 3, "Improved Life Tap", 2, 2) + + local state = { class = "WARLOCK", talents = {} } + StateCollector.CollectTalents(state) + + -- Should use the TalentMap key (1:7), not the API position (1:3) + assert.are.equal(2, state.talents["1:7"]) + assert.is_nil(state.talents["1:3"]) + end) + + ----------------------------------------------------------------------- + -- Test 2: maxRank clamping + ----------------------------------------------------------------------- + it("clamps rank to maxRank from TalentMap entry", function() + ns.TalentMap["WARLOCK:2:5"] = { + name = "Emberstorm", + maxRank = 2, + effects = {}, + } + + -- API returns rank 5, but maxRank is 2 + setTalent(2, 1, "Emberstorm", 5, 5) + + local state = { class = "WARLOCK", talents = {} } + StateCollector.CollectTalents(state) + + assert.are.equal(2, state.talents["2:5"]) + end) + + ----------------------------------------------------------------------- + -- Test 3: Untracked talents use raw tab:index key + ----------------------------------------------------------------------- + it("stores untracked talents under raw tab:index key", function() + -- TalentMap has nothing for this talent + setTalent(3, 2, "Some Untracked Talent", 3, 5) + + local state = { class = "WARLOCK", talents = {} } + StateCollector.CollectTalents(state) + + assert.are.equal(3, state.talents["3:2"]) + end) + + ----------------------------------------------------------------------- + -- Test 4: Zero-rank talents are skipped + ----------------------------------------------------------------------- + it("does not store talents with rank 0", function() + ns.TalentMap["WARLOCK:1:1"] = { + name = "Suppression", + maxRank = 5, + effects = {}, + } + + setTalent(1, 1, "Suppression", 0, 5) + + local state = { class = "WARLOCK", talents = {} } + StateCollector.CollectTalents(state) + + assert.is_nil(state.talents["1:1"]) + -- Also verify no raw key + assert.is_nil(state.talents["1:1"]) + end) + + ----------------------------------------------------------------------- + -- Test 5: Multiple talents with mixed matching + ----------------------------------------------------------------------- + it("handles a mix of tracked, untracked, and zero-rank talents", function() + -- Tracked: TalentMap key 1:7, API returns at tab=1, index=3 + ns.TalentMap["WARLOCK:1:7"] = { + name = "Improved Corruption", + maxRank = 5, + effects = {}, + } + + -- Tracked: TalentMap key 2:5, API returns at tab=2, index=4 + ns.TalentMap["WARLOCK:2:5"] = { + name = "Improved Shadow Bolt", + maxRank = 5, + effects = {}, + } + + -- Tab 1: tracked, untracked, and zero-rank talents + setTalent(1, 1, "Something Else", 2, 5) -- untracked, rank > 0 + setTalent(1, 2, "Another Talent", 0, 3) -- rank 0, should skip + setTalent(1, 3, "Improved Corruption", 3, 5) -- tracked -> key 1:7 + setTalent(1, 4, "Filler Talent", 0, 3) -- rank 0, should skip + + -- Tab 2: tracked, untracked, and zero-rank talents + setTalent(2, 1, "Bane", 0, 5) -- rank 0, skip + setTalent(2, 2, "Aftermath", 2, 5) -- untracked, rank > 0 + setTalent(2, 3, "Cataclysm", 1, 3) -- untracked, rank > 0 + setTalent(2, 4, "Improved Shadow Bolt", 5, 5) -- tracked -> key 2:5 + + local state = { class = "WARLOCK", talents = {} } + StateCollector.CollectTalents(state) + + -- Tracked talents use TalentMap keys (not API indices) + assert.are.equal(3, state.talents["1:7"]) -- Improved Corruption + assert.are.equal(5, state.talents["2:5"]) -- Improved Shadow Bolt + + -- Untracked talents use raw tab:index keys + assert.are.equal(2, state.talents["1:1"]) -- Something Else + assert.are.equal(2, state.talents["2:2"]) -- Aftermath + assert.are.equal(1, state.talents["2:3"]) -- Cataclysm + + -- Zero-rank talents should not appear at all + assert.is_nil(state.talents["1:2"]) -- Another Talent (rank 0) + assert.is_nil(state.talents["1:4"]) -- Filler Talent (rank 0) + assert.is_nil(state.talents["2:1"]) -- Bane (rank 0) + end) + + ----------------------------------------------------------------------- + -- Test 6: Only the matching class prefix is scanned + ----------------------------------------------------------------------- + it("ignores TalentMap entries for other classes", function() + -- MAGE talent in the map + ns.TalentMap["MAGE:1:2"] = { + name = "Arcane Focus", + maxRank = 5, + effects = {}, + } + + -- WARLOCK talent in the map + ns.TalentMap["WARLOCK:1:1"] = { + name = "Suppression", + maxRank = 5, + effects = {}, + } + + -- API returns both talent names but state.class is WARLOCK + setTalent(1, 1, "Suppression", 3, 5) + setTalent(1, 2, "Arcane Focus", 2, 5) -- same name but wrong class + + local state = { class = "WARLOCK", talents = {} } + StateCollector.CollectTalents(state) + + -- Suppression is tracked (WARLOCK:1:1 -> key "1:1") + assert.are.equal(3, state.talents["1:1"]) + -- "Arcane Focus" has no WARLOCK TalentMap entry, so it uses raw key + assert.are.equal(2, state.talents["1:2"]) + end) + + ----------------------------------------------------------------------- + -- Test 7: Talent with no maxRank in TalentMap entry is not clamped + ----------------------------------------------------------------------- + it("does not clamp when TalentMap entry has no maxRank", function() + ns.TalentMap["WARLOCK:3:1"] = { + name = "Demonic Embrace", + -- no maxRank field + effects = {}, + } + + setTalent(3, 2, "Demonic Embrace", 5, 5) + + local state = { class = "WARLOCK", talents = {} } + StateCollector.CollectTalents(state) + + -- Should use mapped key "3:1" with unclamped rank + assert.are.equal(5, state.talents["3:1"]) + end) + + ----------------------------------------------------------------------- + -- Test 8: Empty talent tree produces empty talents table + ----------------------------------------------------------------------- + it("produces empty talents when there are no talent tabs", function() + mockState.numTabs = 0 + + local state = { class = "WARLOCK", talents = {} } + StateCollector.CollectTalents(state) + + local count = 0 + for _ in pairs(state.talents) do count = count + 1 end + assert.are.equal(0, count) + end) +end) diff --git a/tests/test_warlock_completion.lua b/tests/test_warlock_completion.lua index 5727c5d..33f4bed 100644 --- a/tests/test_warlock_completion.lua +++ b/tests/test_warlock_completion.lua @@ -11,13 +11,13 @@ local Pipeline = ns.Engine.Pipeline describe("Warlock Completion", function() - describe("Improved Life Tap (1:3)", function() + describe("Improved Life Tap (1:7)", function() it("should increase Life Tap mana by 10% at rank 1", function() local state = makePlayerState() local base = Pipeline.Calculate(1454, state) - state.talents["1:3"] = 1 + state.talents["1:7"] = 1 local buffed = Pipeline.Calculate(1454, state) assert.is_near(base.manaGain * 1.10, buffed.manaGain, 0.01) @@ -27,7 +27,7 @@ describe("Warlock Completion", function() local state = makePlayerState() local base = Pipeline.Calculate(1454, state) - state.talents["1:3"] = 2 + state.talents["1:7"] = 2 local buffed = Pipeline.Calculate(1454, state) assert.is_near(base.manaGain * 1.20, buffed.manaGain, 0.01) @@ -37,7 +37,7 @@ describe("Warlock Completion", function() local state = makePlayerState() local base = Pipeline.Calculate(686, state) - state.talents["1:3"] = 2 + state.talents["1:7"] = 2 local buffed = Pipeline.Calculate(686, state) assert.is_near(base.expectedDamageWithMiss, buffed.expectedDamageWithMiss, 0.01) @@ -85,7 +85,7 @@ describe("Warlock Completion", function() local base = Pipeline.Calculate(686, state) -- Add Shadow Mastery + Soul Link state.auras.player[25228] = true - state.talents["1:15"] = 5 -- Shadow Mastery +10% + state.talents["1:11"] = 5 -- Shadow Mastery +10% local result = Pipeline.Calculate(686, state) local expected = base.expectedDamageWithMiss * 1.10 * 1.05 assert.is_near(expected, result.expectedDamageWithMiss, 0.5)