Fix wand hit calculation to use the Wands skill.#3439
Merged
ratkosrb merged 1 commit intoMay 29, 2026
Conversation
Stoabrogga
pushed a commit
to Stoabrogga/vmangos
that referenced
this pull request
May 30, 2026
(cherry picked from commit 5d67ee5)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🍰 Pullrequest
Wand
Shoot(spell5019) deals spell damage but is, mechanically, a ranged weapon attack.SpellCaster::SpellHitResult()dispatches the hit roll purely onDmgClass, and because the wand'sDmgClassisSPELL_DAMAGE_CLASS_MAGICit gets resolved bySpellCaster::MagicSpellHitChance(). That formula is keyed on the attacker/target level difference and never looks at the Wands weapon skill (228).As a result, wand hit chance is completely independent of the Wands skill: a level 60 character with
1/300skill has the exact same chance to hit as with300/300. As the video evidence in #2361 shows, this is incorrect, at least for Classic.Interestingly, VMaNGOS already treats wands as ranged for the crit roll via the
SPELL_ATTR_EX3_NORMAL_RANGED_ATTACKattribute inUnit::IsSpellCrit()here:core/src/game/Objects/Unit.cpp
Lines 5478 to 5482 in d41b70f
In CMaNGOS Classic, wand hit rate is resolved through the ranged weapon skill hit table (
CalculateEffectiveMissChance()keyed onGetWeaponSkillValue(RANGED_ATTACK)for spells flaggedSPELL_ATTR_EX3_NORMAL_RANGED_ATTACK).This PR makes wands use ranged hit calculation, which aligns with CMaNGOS's approach:
In
SpellCaster::SpellHitResult(), aSPELL_DAMAGE_CLASS_MAGICspell flaggedSPELL_ATTR_EX3_NORMAL_RANGED_ATTACKnow goes toSpellCaster::MeleeSpellHitResult()here:core/src/game/Objects/SpellCaster.cpp
Lines 219 to 227 in bfc51c2
It is then given the ranged attack type here:
core/src/game/Objects/SpellCaster.cpp
Lines 389 to 391 in bfc51c2
That path already derives the weapon skill from
GetWeaponSkillValue()(which returns the Wands skill228for an equipped wand), so the hit roll now scales accordingly with the skill. Because wands use the ranged attack type, they still cannot be dodged, parried or blocked (the ranged path inMeleeSpellHitResult()returns before those rolls).One side effect worth noting: a wand that fails its hit roll on the skill-based miss now reports
SPELL_MISS_MISS(from the ranged hit table) instead ofSPELL_MISS_RESIST, which also matches CMaNGOS. School-based resists still apply separately during damage calculation.Disclaimer: I wasn't able to find any Vanilla sources, but the Classic video evidence (I think it's fair to assume Classic matches Vanilla here) and the fact that CMaNGOS already does this both point to it being more correct than the current behavior.
Also noteworthy,
SPELL_ATTR_EX3_NORMAL_RANGED_ATTACKis only set onShootfrom1.9.4.5086onward, so this change only applies to those builds; this is the same as the existing wand crit handling, which is gated on the same flag. I wasn't able to find patch note evidence that this was changed, so I'm assuming this was an undocumented change back then. Maybe someone can confirm if this is true.Proof
Issues
How2Test
.levelup 59(use level 60 if you also want to observe the base miss chance at the300skill cap).additem 5240 1and equip the wand (Torchlight Wand).go creature 40917(teleports you to an isolated level 59-60 Frostsaber Stalker in Winterspring)300and confirm the base miss chance is correct. If the mob dies while attacking it, attack another one.Todo / Checklist