Skip to content

fix(blizz): stop comparing one-handed weapon upgrades against shields without dual wield - #2013

Merged
EllesmereGaming merged 1 commit into
EllesmereGaming:mainfrom
dfrisone:fix/better-items-weapon-slot
Sep 8, 2026
Merged

fix(blizz): stop comparing one-handed weapon upgrades against shields without dual wield#2013
EllesmereGaming merged 1 commit into
EllesmereGaming:mainfrom
dfrisone:fix/better-items-weapon-slot

Conversation

@dfrisone

@dfrisone dfrisone commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes a one-handed weapon being listed as an upgrade over a shield for a character that cannot dual wield. Reported by Zuo on 9.1.6: a Holy paladin with a 295 main-hand mace and 272 shield was told a 279 bag mace was better.

INVTYPE_WEAPON compares against both weapon slots. The scanner now checks CanDualWield() once per scan and excludes slot 17 for one-handed/off-hand weapons when dual wield is unavailable. A real main-hand upgrade still qualifies; shields and held off-hands retain their normal comparisons.

This covers a different case from #736, whose protection for an empty off-hand behind a two-handed weapon remains intact. Existing equipment/spec-change and sheet-open cache invalidation is unchanged.

How was it tested?

  • Don confirmed the fix looks good in game after installing the patch. Exact client build and broader combat/taint testing were not recorded for this fix.
  • The actual scanner passed 14 Lua 5.1 mocked regression cases: the reported 295/272/279 case, genuine main-hand upgrades, shield/held-item upgrades, off-hand-only weapons, dual wield, two-handed/ranged empty-off-hand protection, occupied off-hand behind a two-hander, and trinkets. The original code reproduces the reported false positive.
  • Full-file Lua 5.1 syntax, changed-line EUI style gate, and whitespace checks passed. Locale extraction produced no content changes. The installed scanner matches the reviewed scanner.
  • Code review found no new frame writes, event registrations, timers or allocations. Cost is one capability query per scan, plus a local slot guard. Dual-wield alternatives were mock-tested, not separately confirmed in game.

Screenshots

The before screenshot was supplied with the report but is not attached here. An after screenshot is not available; in-game confirmation is recorded above.

Checklist

  • New settings default OFF (no behavior change without opt-in) -- N/A: fixes existing upgrade detection; no new settings.
  • Zero cost while disabled: no events registered, no polling, no hooks doing work, no frames built -- this diff adds none and preserves the existing scan lifecycle.
  • Cheap while enabled: event-driven (no polling, no timer-based logic, no per-frame allocations) -- one capability query per scan; no added scheduling or allocations.
  • No writes onto Blizzard-owned frames (weak-table pattern used); HookScript/hooksecurefunc only, never SetScript on Blizzard frames -- no frame writes or hooks in the diff.
  • Tested in-game on live; no version gates or pre-Midnight APIs added -- Don confirmed the installed fix in game; alternate cases and limits noted above.

Cute cat

@EllesmereGaming
EllesmereGaming merged commit b77829a into EllesmereGaming:main Sep 8, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants