Ralgathor / LibClassicSwingTimerAPI

GNU General Public License v3.0
4 stars 5 forks source link

Added check to not prevent attack speed update on form change when th… #38

Closed hypernormalisation closed 1 year ago

hypernormalisation commented 1 year ago

…e druid's bar is full

Logic exists in the library at present to snapshot druid's attack speed when a form change occurs, correctly stopping any attack speed update when the druid is mid-swing.

This also locks out attack speed changes for form changes when the druid's swing timer is full, and therefore any attack speed reported by the library until the druid begins attacking with their new form will be wrong/the old form's speed (see attached, where the bar reports the current attack speed on the left of the bar).

Adding a simple check to only trigger the snapshotting when the current time is less than the mainhand expiration time results in form changes while the timer is full correctly triggering attack speed updates in the library.

image

hypernormalisation commented 1 year ago

There's an additional issue where if the druid changes form mid-swing and this is their final attack, we get a similar problem with the correct speed just never being picked up or broadcast by the library until the druid makes another attack, or changes form again.

This could be fixable by changing the SwingEnd function to something like the following, where if any skipNextAttackSpeedUpdate exists and the player is a druid, we trigger the UNIT_ATTACK_SPEED function - but I'm unsure if this will screw up any behaviour elsewhere in the library's logic.

function lib:SwingEnd(hand)
    if hand == "mainhand" then
        self.mainTimer:Cancel()
        if self.class == "DRUID" and self.skipNextAttackSpeedUpdate then
            self:UNIT_ATTACK_SPEED()
        end
    elseif hand == "offhand" then
        self.offTimer:Cancel()
    elseif hand == "ranged" then
        self.rangedTimer:Cancel()
    end
    self:Fire("SWING_TIMER_STOP", hand)
    if (self.casting or self.channeling) and self.isAttacking and hand ~= "ranged" then
        local now = GetTime()
        if isRetail and hand == "mainhand" then     
            self:SwingStart(hand, now, true)
            self:Fire("SWING_TIMER_CLIPPED", hand)
        elseif isClassicOrBCCOrWrath then
            self:SwingStart(hand, now, true)
            self:Fire("SWING_TIMER_CLIPPED", hand)
        end
    end
end
Ralgathor commented 1 year ago

There's an additional issue where if the druid changes form mid-swing and this is their final attack, we get a similar problem with the correct speed just never being picked up or broadcast by the library until the druid makes another attack, or changes form again.

This could be fixable by changing the SwingEnd function to something like the following, where if any skipNextAttackSpeedUpdate exists and the player is a druid, we trigger the UNIT_ATTACK_SPEED function - but I'm unsure if this will screw up any behaviour elsewhere in the library's logic.

function lib:SwingEnd(hand)
  if hand == "mainhand" then
      self.mainTimer:Cancel()
      if self.class == "DRUID" and self.skipNextAttackSpeedUpdate then
          self:UNIT_ATTACK_SPEED()
      end
  elseif hand == "offhand" then
      self.offTimer:Cancel()
  elseif hand == "ranged" then
      self.rangedTimer:Cancel()
  end
  self:Fire("SWING_TIMER_STOP", hand)
  if (self.casting or self.channeling) and self.isAttacking and hand ~= "ranged" then
      local now = GetTime()
      if isRetail and hand == "mainhand" then     
          self:SwingStart(hand, now, true)
          self:Fire("SWING_TIMER_CLIPPED", hand)
      elseif isClassicOrBCCOrWrath then
          self:SwingStart(hand, now, true)
          self:Fire("SWING_TIMER_CLIPPED", hand)
      end
  end
end

If I correctly understand the issue is if the attack never land the state of the swing timer is still at the previous form attack speed. But the correct attack speed is display when the swing start.

For me this change seem logical as the attack speed should be updated when the swing end as the snapshot only leave for the duration of the swing. I don't see any backfire to other lib logic.

Maybe just need to set the self.skipNextAttackSpeedUpdate to nil to prevent unecessary trigger of this condition block on every swingEnd as the skipNextAttackSpeedUpdate are set to a GetTime() value and nerver nillify after that in the rest of the code.

 function lib:SwingEnd(hand)
    if hand == "mainhand" then
        self.mainTimer:Cancel()
        if self.class == "DRUID" and self.skipNextAttackSpeedUpdate then
                        self.skipNextAttackSpeedUpdate = nil
            self:UNIT_ATTACK_SPEED()
        end
    elseif hand == "offhand" then
        self.offTimer:Cancel()
    elseif hand == "ranged" then
        self.rangedTimer:Cancel()
    end
    self:Fire("SWING_TIMER_STOP", hand)
    if (self.casting or self.channeling) and self.isAttacking and hand ~= "ranged" then
        local now = GetTime()
        if isRetail and hand == "mainhand" then     
            self:SwingStart(hand, now, true)
            self:Fire("SWING_TIMER_CLIPPED", hand)
        elseif isClassicOrBCCOrWrath then
            self:SwingStart(hand, now, true)
            self:Fire("SWING_TIMER_CLIPPED", hand)
        end
    end
 end
hypernormalisation commented 1 year ago

I'll do some testing on this tonight and if it works I'll add a commit to this pull request 👍

hypernormalisation commented 1 year ago

@Ralgathor, I've worked with the fix for a couple days and everything looks good, I've merged the changelogs from main and this branch, should be ready to merge if you want to give it a look over and confirm it.

Ralgathor commented 1 year ago

@hypernormalisation All good for me I merge it !