Zidras / DBM-Warmane

DBM for Warmane (Icecrown, Lordaeron, Frostmourne, Onyxia)
https://discord.gg/CyVWDWS
146 stars 61 forks source link

Core: Fix filtering UNIT_* events by unitId #200

Closed oomshroom closed 2 weeks ago

oomshroom commented 4 months ago

Fix filtering by unit in UNIT_* events. Fixes event filters such as "UNIT_HEALTH boss1" not working at all.

Zidras commented 4 months ago

@oomshroom, can you briefly explain why this is needed? I'd have to check on some recent logs but I have no ticket open for unit health not working

oomshroom commented 4 months ago

@oomshroom, can you briefly explain why this is needed? I'd have to check on some recent logs but I have no ticket open for unit health not working

Currently all timers/warnings which use "UNIT_HEALTH boss1" are broken. These are mostly "Next Phase Soon"-style warnings, such as Deathbringer's Frenzy and Sindragosa's P2. In practice UNIT_HEALTH is the only event affected, but it is a commonly used event.

In DBM:ADDON_LOADED(), DBM registers "UNIT_HEALTH mouseover target focus player" on the main frame. In the existing code, handleEvent(self, "UNIT_HEALTH", "boss1") checks the first entry in registeredEvents, which for UNIT_HEALTH is the DBM main frame, and immediately returns because boss1 is not wanted there. Other entries in registeredEvents, such as those from a boss mod, are never checked!

Zidras commented 4 months ago

@oomshroom, it is a valid issue, thanks for bringing this up and PR'ing a fix. I will try to look into it during next week

Zidras commented 2 weeks ago

Merged at last! Thank you @oomshroom for your contribution