Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing shadow trap announcements #56

Open
Serkora opened this issue Dec 10, 2023 · 6 comments
Open

Missing shadow trap announcements #56

Serkora opened this issue Dec 10, 2023 · 6 comments
Labels
question Further information is requested

Comments

@Serkora
Copy link

Serkora commented Dec 10, 2023

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
Copy link
Member

MysticalOS commented Dec 10, 2023

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
Copy link
Member

MysticalOS commented Dec 10, 2023

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
Copy link
Author

Serkora commented Dec 10, 2023

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 — DeadlyBossMods/DBM-Unified@07988cb

@MysticalOS
Copy link
Member

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.
DeadlyBossMods/DBM-Unified@5895b54

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
Copy link
Author

Serkora commented Dec 10, 2023

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

@MysticalOS
Copy link
Member

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)

@MysticalOS MysticalOS added the question Further information is requested label Jan 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants