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

Cancel/Restart timer methods not unscheduling multi arg timers #153

Open
Zidras opened this issue Sep 4, 2022 · 0 comments
Open

Cancel/Restart timer methods not unscheduling multi arg timers #153

Zidras opened this issue Sep 4, 2022 · 0 comments
Labels
🐛 Bug Something isn't working Low Priority This issue will not be worked on unless there are no high or medium tasks left

Comments

@Zidras
Copy link
Contributor

Zidras commented Sep 4, 2022

Describe the bug

Using :Cancel(arg1, ...) or :Restart(arg1, arg2, ...) will fail to unschedule timers, specifically when dealing with multi arg timers (example, NewCDCountTimer with a count).

Do you have an error log of what happened?
NA

To Reproduce
TLDR:

local timerCount = mod:NewCDCountTimer(5, 2061)

timerCount:Start(nil, 4)
timerCount:Schedule(5, nil, 4)

timerCount:Cancel(4) -- bar timer stopped but fail to unschedule
timerCount:Cancel(nil, 4) -- bar timer failed to stop but succeed in the unschedule above

Now for the troubleshooting steps:
Let’s consider a dummy example (used priest flash heal for easy repro with cast start for testing):

local timerCount = mod:NewCDCountTimer(5, 2061)

If I start and cancel, with a dummy count value = 4, no problem:

timerCount:Start(5, 4)
--
timerCount:Cancel(4)

If I restart, or add nil timer to use default is also no issue:

timerCount:Restart(nil, 4)

Increasing arg complexity with for example a GUID (using target guid just for illustration) is also no problem for now. Adapted timer with allowdouble flag accordingly:

local timerCountGUID = mod:NewCDCountTimer("d5", 2061)

timerCount:Start(nil, 4, UnitGUID("target"))
timerCount:Cancel(4, UnitGUID("target"))

Problem begins if we also add Scheduling into the mix. Due to how the methods are designed, the order of args between the Cancel/Unschedule and the Start is different and therefore will fail to match the timer to unschedule:

timerCount:Restart(nil, 4, UnitGUID("target")) -- will fail to unschedule if called again
timerCount:Schedule(5, nil, 4, UnitGUID("target"))
--
timerCount:Cancel(4, UnitGUID("target")) --will hide started timer, but fail to unschedule

I was only able to avoid this trap with arg mismatch by being verbose and not rely on restart/cancel to unschedule (notice the nil arg difference, if you add prints in each method vararg inside Core it will be more apparent):

timerCount:Start(nil, 4, UnitGUID("target"))
timerCount:Schedule(5, nil, 4, UnitGUID("target"))
--
timerCount:Unschedule(nil, 4, UnitGUID("target"))
timerCount:Stop(4, UnitGUID("target"))

Screenshots
prints are coming from Core, on timer Start, Cancel, Unschedule, with their respective method name.
image
Exception is the "scheduled timer in 5s", which is fired in the mod event.
In these demos, flash heal is used to start + schedule timer, hearthstone to cancel.

  1. with timerCount:Cancel(4), started bar is stopped, but not unscheduled:
Wow_JIFW9vm7XU.mp4
  1. with timerCount:Cancel(nil, 4), started bar is not stopped, only unscheduled:
Wow_Su0BTPqG1Y.mp4

A :Restart would be the same behaviour, since it's a copy of Cancel.

Did you try having DeadlyBossMods as the only enabled addon and everything else (especially something like ElvUI) disabled?
Yes

Which version of DeadlyBossMods are you using?
DBM 9.2.33 alpha (2022/09/04 16:11:37)

Was it working in a previous version? If yes, which was the last good one?
Unknown.

Additional context
AFAICT, there isn't a mod that have these conditions in order to be a live problem, nervertheless this is a bug in core methods and thus I feel like it should be addressed.
I have created a mod just for testing this matter in the dummy, if you wish to have it for your testing I will zip it and attach here.

@MysticalOS MysticalOS added Low Priority This issue will not be worked on unless there are no high or medium tasks left 🐛 Bug Something isn't working labels Sep 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Something isn't working Low Priority This issue will not be worked on unless there are no high or medium tasks left
Projects
None yet
Development

No branches or pull requests

2 participants