Skip to content

[Bug]: Expected damage overestimates actual; add diagnostic for missing-overlay spells #44

@Xerrion

Description

@Xerrion

Problem

Two related accuracy / coverage issues observed in real combat on TBC Anniversary as a Warlock:

  1. Expected damage is consistently 5-10% above actual sustained damage. Magnitude points at modeling bugs rather than a missing major mechanic.
  2. Hellfire (spell ID 1949) does not show an action-bar overlay even though all four ranks are present in Data/SpellData_Warlock.lua and the engine has no skip path for channels, AoE, or self-damage spells. Spell is placed on the bar via direct spellbook drag (no macro).

Root cause analysis

A static audit identified two engine-level bugs that together explain the 5-10% overestimate:

Bug A: spell crit chance double-counted

StateCollector reads GetSpellCritChance(school) (StateCollector.lua:202-207). The WoW API value already includes class talent crit bonuses such as Devastation, Demonic Tactics, etc.

Several Data/TalentMap_*.lua entries declare effect = MOD.CRIT_BONUS for the same talents (e.g. TalentMap_Warlock["3:11"] Devastation, line 271). ModifierCalc then sums those into mods.critBonus, which CritCalc adds on top of the already-talent-adjusted base crit. The bonus is counted twice.

Effect: with 5/5 Devastation and Ruin (crit multiplier 2.0x), spells crit ~5% more often in the model than in reality, inflating expected damage by ~5%.

Bug B: stacking debuffs assumed full-stack

Stacking debuffs declared in AuraMap_*.lua (Shadow Vulnerability / Improved Shadow Bolt 17800/17803, Shadow Weaving, etc.) apply their full multiplier the moment a single stack is detected. The current code path does not read auraData.applications. The Sunder Armor handler in StateCollector.lua:87-92 is the only place stack count is honored today.

Effect: a fresh Shadow Bolt landing into a 1-stack ISB shows *1.20 in the overlay when actual damage is only *1.04. Adds another ~3-15% overestimate during ramp-up.

Hellfire missing overlay

Static analysis cannot identify the runtime cause - data is present, no skip filter exists, BuildSpellIDMap correctly maps all four ranks back to base 1949. Speculative fixes risk shipping a non-fix. A diagnostic command is needed to capture GetActionInfo, ResolveSpellID, the data lookup, and Pipeline.Calculate output for a user-specified action slot.

Proposed fix

Single PR addressing all three:

  1. Crit double-count audit and fix. Walk every Data/TalentMap_*.lua and remove CRIT_BONUS effect descriptors that target talents already reflected in GetSpellCritChance(school). Keep entries that grant spell-specific crit bonuses (e.g. Improved Searing Pain on a single spell). Add busted test that asserts a Destro warlock with 0% base API crit and a separate API-already-included +5% crit produces the same expected damage as the version where talent map adds the +5% (regression guard).

  2. Stack-aware aura modifiers. When applying an aura entry whose flagged effect scales linearly with stacks, multiply by (auraData.applications or 1) / aura.maxStacks of the declared bonus. Update Shadow Vulnerability (assumed-5-stack hard-coded behavior) and Shadow Weaving as the seed cases. Add busted tests at 1, 3, 5 stacks.

  3. /phd debug diagnostic command. New slash subcommand that takes an optional action slot (default: currently hovered or action slot 1) and prints:

    • GetActionInfo(slot) raw return values
    • ResolveSpellID outcome
    • Spell data table lookup
    • Pipeline.Calculate result or skip reason
      Output is plain print() to chat - no UI. Used to debug Hellfire and any future missing-overlay reports.

Out of scope (deliberately deferred)

  • Average partial-resist modeling (would risk swinging into underestimate once Bugs A+B are fixed)
  • Channel coefficient sourcing for Hellfire / Rain of Fire (open question whether 0.4286 is correct)
  • Sustained-DPS execution-efficiency multiplier (movement, clipping, latency)

These will be filed as follow-up issues after the diagnostic command surfaces real data.

Verification

  • luacheck . passes 0/0
  • busted --verbose all tests green including new regression tests
  • In-game smoke test on Warlock: /phd debug works on a Hellfire button; expected damage on Shadow Bolt with full Devastation/Ruin no longer inflates by ~5% relative to actual; Shadow Bolt expected scales correctly with ISB stack count

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions