Skip to content

[Tracker]: Engine accuracy improvements (audit warnings W1-W9) #48

Description

@Xerrion

Summary

Tracker for nine WARNING-severity findings from the engine accuracy audit. Each is independently fixable; this issue serves as the umbrella for sub-PRs. Some are quick wins (W1, W2, W7), others require encoding audits (W3, W4) or design decisions (W5, W6, W9). W8 is deferred per #44.

Findings

W1 - Hunter ranged base crit multiplier 1.5x instead of 2.0x

Location: Engine/CritCalc.lua:102-108
Issue: BASE_CRIT_MULTIPLIER (1.5x spell crit) is applied for scalingType="ranged" spells. Per Wowwiki Critical_strike, ranged crits use the 2.0x melee multiplier in TBC.
Impact: ~11% understatement at 25% crit chance for Hunter spells (Arcane Shot, Multi-Shot, Aimed Shot, Steady Shot, Volley, Serpent Sting tick crits).
Fix: Branch on isMelee or isRanged to use 2.0x base, or add BASE_RANGED_CRIT_MULTIPLIER = 2.0 constant.

W2 - Rogue/Cat-form GCD floor 1.5s instead of 1.0s

Location: Engine/CritCalc.lua:42-56, Core/Constants.lua:85
Issue: GLOBAL_COOLDOWN = 1.5 applied uniformly. Energy-based abilities (Rogue, Cat-form Druid) have a 1.0s GCD in TBC.
Impact: Up to 33% DPS understatement on Sinister Strike, Backstab, Eviscerate, Shred, Mangle (Cat), Claw, Rake.
Fix: Per-spell gcd field in SpellData, or derive from playerState.class + spell category.

W3 - CRIT_MULT_BONUS data convention unverified

Location: Engine/CritCalc.lua:108, all Data/TalentMap_*.lua
Issue: critMultBonus is added to baseCritMult. Talents like Ruin (Warlock, "+100% crit damage" → 2.0x crits) require encoding the delta (value = 0.5), not the full multiplier.
Affected talents: Ruin, Ice Shards 5/5, Spell Power 5/5 (0.1), Vengeance 5/5 (0.5), Shadow Power 5/5 (0.5), Mortal Shots 5/5 (0.3), Lethality 5/5 (0.3).
Impact: If TalentMap encoded the full multiplier, math is off by 50-100%.
Fix: Spot-check audit of each TalentMap entry; document convention in code comments.

W4 - Heal-flagged spells use spell damage instead of healing power

Location: Engine/SpellCalc.lua:70,244,276,331,363, StateCollector.lua:196-199
Issue: state.stats.healingPower = GetSpellBonusHealing() is collected but unused. All Compute* paths read playerState.stats.spellPower[school] regardless of isHeal flag.
Impact: Currently affects only Paladin Holy Shock heal variant (synthetic ID 200473, isHeal=true). Latent for future heal additions.
Fix: Branch on spellData.isHeal in each Compute* and in ModifierCalc.BuildModifiedResult:347-354 to read healingPower instead.

W5 - Curse of Agony ramping flagged but not modeled

Location: Data/SpellData_Warlock.lua:211-233 isRamping=true
Issue: Flag exists with no consumer. CoA delivers 50%/100%/150% per third of duration. Total damage is correct (sum stored). Per-tick UI display would be uniform (wrong if any consumer reads tickDamage per-tier).
Impact: Averaged-EV math correct; fidelity gap only.
Fix: Either remove flag with explanatory comment, or compute per-third tick distributions in BuildPeriodicResult.

W6 - Hybrid coefficientBonus only applies to direct portion

Location: Engine/ModifierCalc.lua:455-462
Issue: Empowered Fireball (direct-only DoT scaling) is correctly handled. Future hybrid talents whose DoT also scales would silently break.
Fix: Add MOD.DOT_COEFFICIENT_BONUS modifier type for symmetry, or appliesToDot filter on hybrid coefficient bonuses.

W7 - expertiseReduction magic number 0.0025 instead of EXPERTISE_PER_PERCENT=4.0

Location: Engine/CritCalc.lua:122, Constants.lua:72
Issue: Hardcoded 0.0025 instead of using existing constant.
Fix: expertiseReduction = expertise / (EXPERTISE_PER_PERCENT * 100). Pure refactor.

W8 - Partial resist (~6% systematic loss) unmodeled - DEFERRED

Location: Engine/CritCalc.lua:189,241,313
Status: Out of scope per #44 deferral. Logged for catalog completeness.
Future fix: Multiply expectedWithMiss * (1 - 0.06) for boss-level same-school targets when target-level signal is available.

W9 - damageBeforeMods field semantics inconsistent

Location: Engine/ModifierCalc.lua:476
Issue: Direct/DoT/channel paths set damageBeforeMods pre-multiplier; hybrid path sets it to baseResult.totalDamage (post-SP).
Impact: Diagnostics consumers expecting consistent semantics may misrender hybrid spells.
Fix: Standardize across all three paths to mean "post-SP, post-flat, pre-multiplier".

Sequencing recommendation

  1. Quick wins first (W1, W2, W7) - small, isolated, high signal.
  2. Encoding audit (W3) - audit + fix TalentMap entries; may reveal cascading corrections.
  3. Healing path (W4) - enables future Paladin/Druid heal work.
  4. Polish (W5, W6, W9) - fidelity and consistency.
  5. W8 stays deferred until [Bug]: Expected damage overestimates actual; add diagnostic for missing-overlay spells #44 resolves.

Dependencies

  • W1, W2, W3 are touched by the BLOCKER fixes for downranking penalty (separate issue) only insofar as both modify CritCalc.lua and SpellData_*.lua. Sequence: BLOCKERs first, then warnings.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions