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.

Sign up for GitHub

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

Bossmod Timer fix #4584

Merged
merged 3 commits into from
Sep 3, 2023
Merged

Bossmod Timer fix #4584

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f406714
Bossmod Timer triggers: improve remainingTime precision and fix timer…
mrbuds Aug 26, 2023
6563685
FixFixFix
InfusOnWoW Sep 3, 2023
124b61d
More fixfixfix
InfusOnWoW Sep 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 47 additions & 26 deletions WeakAuras/BossMods.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,9 @@ Private.ExecEnv.BossMods.DBM = {
for timerId, bar in pairs(self.bars) do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. For the nextExpire we should also take scheduleScanExpire into account, that is I think we should add:
if self.nextExpire == nil then
     self.nextExpire = bar.scheduledScanExpireAt
elseif bar.scheduledScanExpireAt and bar.scheduledScanExpireAt > GetTime() and bar.scheduledScanExpireAt< self.nextExpire
    self.nextExpire= self.scheduledScanExpireAt
end

So that, at scheduledScanExpireAt the bar is actually removed.

  1. For extended timers, we end up sending a TimerStop event everytime RecheckTimers is called. That doesn't sound too bad, though it's bit unfortunate.

  2. Another point to consider is that this changes GetTimer/GetAllTimers beahviour in that they can and will now return expired bars (if they are kept alive via an extension). Looking at the code I think some callers of that need adjustment to not consider expired timers. In general GetTimer/GetAllTimers returning expired bars is correct, in so far as we explictly want to keep them around. But we need to add checks for auras that aren't responsible for the extension.

That is consider two auras:
Aura a: Shows 'foo', extends timers by 5s
Aura b: Shows all bars, no extension.

Then if a bar 'foo' expires (for which aura a matches), the bar is kept alive via the scheduledScanExpireAt code.

If in those 5s a DBM_TimerUpdate is fired for an unrelated timer, e.g. bar 'baz'
Then for Aura b, that will result in WeakAuras.ScanEvents('BossMod_TimerUpdate'), then the code (in useClone) will use:

  • TimerMatchesGeneric, which doesn't care that the bar is expired
  • copyAndSchedule, which again doesn't check that remainingTime is negative, to
  • CopyBarToState, which doesn't check either.

So Aura b shows a dead timer.

if not bar.paused then
if bar.expirationTime < now then
self.bars[timerId] = nil
if bar.scheduledScanExpireAt == nil or bar.scheduledScanExpireAt <= GetTime() then
self.bars[timerId] = nil
end
WeakAuras.ScanEvents("DBM_TimerStop", timerId)
if self.isGeneric then
WeakAuras.ScanEvents("BossMod_TimerStop", timerId)
Expand Down Expand Up @@ -212,10 +214,15 @@ Private.ExecEnv.BossMods.DBM = {
end
elseif event == "DBM_TimerStop" then
local timerId = ...
self.bars[timerId] = nil
WeakAuras.ScanEvents("DBM_TimerStop", timerId)
if self.isGeneric then
WeakAuras.ScanEvents("BossMod_TimerStop", timerId)
local bar = self.bars[timerId]
if bar then
if bar.scheduledScanExpireAt == nil or bar.scheduledScanExpireAt <= GetTime() then
self.bars[timerId] = nil
end
WeakAuras.ScanEvents("DBM_TimerStop", timerId)
if self.isGeneric then
WeakAuras.ScanEvents("BossMod_TimerStop", timerId)
end
end
elseif event == "DBM_TimerPause" then
local timerId = ...
Expand Down Expand Up @@ -321,7 +328,7 @@ Private.ExecEnv.BossMods.DBM = {

ScheduleCheck = function(self, fireTime)
if not self.scheduled_scans[fireTime] then
self.scheduled_scans[fireTime] = timer:ScheduleTimerFixed(self.DoScan, fireTime - GetTime() + 0.1, self, fireTime)
self.scheduled_scans[fireTime] = timer:ScheduleTimerFixed(self.DoScan, fireTime - GetTime(), self, fireTime)
end
end
}
Expand Down Expand Up @@ -471,6 +478,9 @@ Private.event_prototypes["DBM Timer"] = {
end
end
if remainingTime >= triggerRemaining and not bar.paused then
if extendTimer >= triggerRemaining then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because RecheckTimers can run at any time (due to it being triggered also by boss mod events), and RecheckTimers needs to know whether to keep a bar alive or not, I think we need to write scheduledScanExpiresAt regardless of any triggerRemainingCheck, and this check should be extendTimer > 0.

So this code should probably use extendTimer > 0, and we also need to move it out of the remainingTime check and it needs to be duplicated in the else branch ~7 lines below.

bar.scheduledScanExpireAt = math.max(bar.scheduledScanExpireAt or 0, bar.expirationTime + extendTimer)
end
Private.ExecEnv.BossMods.DBM:ScheduleCheck(bar.expirationTime - triggerRemaining + extendTimer)
end
else
Expand All @@ -491,8 +501,8 @@ Private.event_prototypes["DBM Timer"] = {
end
end
elseif event == "DBM_TimerStop" and state then
local bar_remainingTime = GetTime() - state.expirationTime + (state.extend or 0)
if state.extend == 0 or bar_remainingTime > 0.2 then
local bar_remainingTime = state.expirationTime - GetTime() + (state.extend or 0)
if state.extend == 0 or bar_remainingTime <= 0 then
state.show = false
state.changed = true
return true
Expand All @@ -506,8 +516,8 @@ Private.event_prototypes["DBM Timer"] = {
else
local state = states[timerId]
if state then
local bar_remainingTime = GetTime() - state.expirationTime + (state.extend or 0)
if state.extend == 0 or bar_remainingTime > 0.2 then
local bar_remainingTime = state.expirationTime - GetTime() + (state.extend or 0)
if state.extend == 0 or bar_remainingTime <= 0 then
state.show = false
state.changed = true
changed = true
Expand Down Expand Up @@ -551,8 +561,8 @@ Private.event_prototypes["DBM Timer"] = {
end
else
if state and state.show then
local bar_remainingTime = GetTime() - state.expirationTime + (state.extend or 0)
if state.extend == 0 or bar_remainingTime > 0.2 then
local bar_remainingTime = state.expirationTime - GetTime() + (state.extend or 0)
if state.extend == 0 or bar_remainingTime <= 0 then
state.show = false
state.changed = true
return true
Expand Down Expand Up @@ -751,7 +761,9 @@ Private.ExecEnv.BossMods.BigWigs = {
for timerId, bar in pairs(self.bars) do
if not bar.paused then
if bar.expirationTime < now then
self.bars[timerId] = nil
if bar.scheduledScanExpireAt == nil or bar.scheduledScanExpireAt <= GetTime() then
self.bars[timerId] = nil
end
WeakAuras.ScanEvents("BigWigs_StopBar", timerId)
if self.isGeneric then
WeakAuras.ScanEvents("BossMod_TimerStop", timerId)
Expand Down Expand Up @@ -812,8 +824,11 @@ Private.ExecEnv.BossMods.BigWigs = {
end
elseif event == "BigWigs_StopBar" then
local addon, text = ...
if self.bars[text] then
self.bars[text] = nil
local bar = self.bars[text]
if bar then
if bar.scheduledScanExpireAt == nil or bar.scheduledScanExpireAt <= GetTime() then
self.bars[text] = nil
end
WeakAuras.ScanEvents("BigWigs_StopBar", text)
if self.isGeneric then
WeakAuras.ScanEvents("BossMod_TimerStop", text)
Expand Down Expand Up @@ -936,7 +951,7 @@ Private.ExecEnv.BossMods.BigWigs = {

ScheduleCheck = function(self, fireTime)
if not self.scheduled_scans[fireTime] then
self.scheduled_scans[fireTime] = timer:ScheduleTimerFixed(self.DoScan, fireTime - GetTime() + 0.1, self, fireTime)
self.scheduled_scans[fireTime] = timer:ScheduleTimerFixed(self.DoScan, fireTime - GetTime(), self, fireTime)
end
end
}
Expand Down Expand Up @@ -1084,6 +1099,9 @@ Private.event_prototypes["BigWigs Timer"] = {
end
end
if remainingTime >= triggerRemaining and not bar.paused then
if extendTimer >= triggerRemaining then
bar.scheduledScanExpireAt = math.max(bar.scheduledScanExpireAt or 0, bar.expirationTime + extendTimer)
end
Private.ExecEnv.BossMods.BigWigs:ScheduleCheck(bar.expirationTime - triggerRemaining + extendTimer)
end
else
Expand All @@ -1104,8 +1122,8 @@ Private.event_prototypes["BigWigs Timer"] = {
end
end
elseif event == "BigWigs_StopBar" and state then
local bar_remainingTime = GetTime() - state.expirationTime + (state.extend or 0)
if state.extend == 0 or bar_remainingTime > 0.2 then
local bar_remainingTime = state.expirationTime - GetTime() + (state.extend or 0)
if state.extend == 0 or bar_remainingTime <= 0 then
state.show = false
state.changed = true
return true
Expand Down Expand Up @@ -1154,8 +1172,8 @@ Private.event_prototypes["BigWigs Timer"] = {
end
else
if state and state.show then
local bar_remainingTime = GetTime() - state.expirationTime + (state.extend or 0)
if state.extend == 0 or bar_remainingTime > 0.2 then
local bar_remainingTime = state.expirationTime - GetTime() + (state.extend or 0)
if state.extend == 0 or bar_remainingTime <= 0 then
state.show = false
state.changed = true
return true
Expand Down Expand Up @@ -1451,6 +1469,9 @@ Private.event_prototypes["Boss Mod Timer"] = {
end
end
if remainingTime >= triggerRemaining and not bar.paused then
if extendTimer >= triggerRemaining then
bar.scheduledScanExpireAt = math.max(bar.scheduledScanExpireAt or 0, bar.expirationTime + extendTimer)
end
Private.ExecEnv.BossMods.Generic:ScheduleCheck(bar.expirationTime - triggerRemaining + extendTimer)
end
else
Expand All @@ -1471,8 +1492,8 @@ Private.event_prototypes["Boss Mod Timer"] = {
end
end
elseif event == "BossMod_TimerStop" and state then
local bar_remainingTime = GetTime() - state.expirationTime + (state.extend or 0)
if state.extend == 0 or bar_remainingTime > 0.2 then
local bar_remainingTime = state.expirationTime - GetTime() + (state.extend or 0)
if state.extend == 0 or bar_remainingTime <= 0 then
state.show = false
state.changed = true
return true
Expand All @@ -1486,8 +1507,8 @@ Private.event_prototypes["Boss Mod Timer"] = {
else
local state = states[timerId]
if state then
local bar_remainingTime = GetTime() - state.expirationTime + (state.extend or 0)
if state.extend == 0 or bar_remainingTime > 0.2 then
local bar_remainingTime = state.expirationTime - GetTime() + (state.extend or 0)
if state.extend == 0 or bar_remainingTime <= 0 then
state.show = false
state.changed = true
changed = true
Expand Down Expand Up @@ -1531,8 +1552,8 @@ Private.event_prototypes["Boss Mod Timer"] = {
end
else
if state and state.show then
local bar_remainingTime = GetTime() - state.expirationTime + (state.extend or 0)
if state.extend == 0 or bar_remainingTime > 0.2 then
local bar_remainingTime = state.expirationTime - GetTime() + (state.extend or 0)
if state.extend == 0 or bar_remainingTime <= 0 then
state.show = false
state.changed = true
return true
Expand Down