DeadlyBossMods / DBM-WotLK

Deadly Boss Mods (DBM) - WotLK mods
https://www.curseforge.com/wow/addons/deadly-boss-mods-wotlk
Other
8 stars 10 forks source link

Missing shadow trap announcements #56

Open Serkora opened 9 months ago

Serkora commented 9 months ago

Describe the bug Shadow Trap target is not announced if the target is the same as previous plague, regardless of the time difference between the two.

This code in DBM-Raids-WoTLK/Icecrown/TheFrozenThrone/LichKing.lua:242

elseif args.spellId == 73539 then -- Shadow Trap (Heroic)
    self:ScheduleMethod(0.01, "BossTargetScanner", args.sourceGUID, "TrapTarget", 0.02, 15, nil, nil, nil, self.vb.lastPlague, nil, nil, true)
    timerTrapCD:Start()

Sets the last plague target to be filtered out and ignored on trap cast.

To Reproduce Get shadow trap cast on a person that had the last plague.

Additional context I understand it's to protect from possibly misannouncing the trap target if LK doesn't switch the target quick enough, but I think the current implementation is doing more harm than good.

I personally deleted the filter completely, but if this protection is still desired, self.vb.lastPlague should at least be cleared on a timer that ensures the current target cannot be the plague target anymore (like 0.5-1s after plague cast or smth, should be good enough and will get rid of at least the vast majority of missed announcements).

MysticalOS commented 9 months ago

the true at end tells it to announce trap target after timer expires if none other was found in that period of time though. is it not doing that?

the filter absolutely had to be there cause it was a frequent problem, like very frequently at least once per pull mis announcing trap on wrong target.and i had users tell me opposite. that it did more harm to announce wrong target than right one just a little slower.

MysticalOS commented 9 months ago

I just carefully looked at code. it sends filterFallback true which causes target scanner to run this code block

--Cache the filtered target if using a filter target fallback
--so when scan ends we can return that instead of tank when scan ends
--(because boss might have already swapped back to aggro target by then)
if targetname and targetname ~= CL.UNKNOWN and filterFallback and targetFilter and targetFilter == targetname then
    filteredTargetCache[cidOrGuid] = {}
    filteredTargetCache[cidOrGuid].target = targetname
    filteredTargetCache[cidOrGuid].targetuid = targetuid
end

on final scan. and returning that as a valid target and announcing that target.

Serkora commented 9 months ago

That might be what it is supposed to do, but it doesn't work. Everyone in our raid that uses DBM (instead of some WA pack like Fojji) has this issue. And the message in the latest commit likely explains why it doesn't do what you describe — https://github.com/DeadlyBossMods/DBM-Unified/commit/07988cbc00b268dd67aaa6b1959dcc654913a85d

MysticalOS commented 9 months ago

I'm gonna push another attempt to fix issue, the only remaining failure condition is if the lich king is looking at an invalid target at 0.3 seconds (the failure timer) and doesn't schedule a final scan, so it never runs condition for fallback target. https://github.com/DeadlyBossMods/DBM-Unified/commit/5895b5413889d3f9a6ec5e2e866e3985409500e9

but the way it should work, trap scan runs every 0.02 seconds for 0.3 seconds. upon which it should return plague target. it's only supposed to filter the target until scan finish, then fallback to it. hopefully above commit addresses one more potential for failure. beyond that I'm out of ideas cause the code is very explicit in saying "only ignore this target until final scan"

Serkora commented 9 months ago

Ok, thanks. If it's still there afterwards, I'll try to add some debug things and investigate.

MysticalOS commented 9 months ago

I appreciate that, but yeah you see the code intent now, so that helps debug it. it probably wasn't clear before the filter was just a short term one (literally only .3 sec)