Reconcile /startattack against real action-bar state, not cached flag

The autoAttack flag could drift stale-true (optimistic set after
AttackTarget, or a target dying without PLAYER_LEAVE_COMBAT), making
/startattack skip the attack until the target was dropped and reselected.
The old fallback called the overridden IsCurrentAction, which just echoes
the cached flag for the attack slot, so it never detected drift. Use the
original Hooks.IsCurrentAction as ground truth and sync the flag to it.

Core.lua

@@ -3100,15 +3100,17 @@ end
 -- PERFORMANCE: Module-level action to avoid closure allocation per call
 local function _startAttackAction()
     if not UnitExists("target") or CleveRoids.IsUnitDead("target") then TargetNearestEnemy() end
-    -- Check both event-based flag AND action bar state for reliable detection
+    -- Ground truth is the real action-bar state, NOT the cached autoAttack flag.
+    -- The flag can drift stale-true (e.g. set optimistically after AttackTarget below,
+    -- or a target dying without PLAYER_LEAVE_COMBAT firing), which would make us wrongly
+    -- believe we're already swinging and skip the attack. Use the ORIGINAL API here:
+    -- the overridden global IsCurrentAction just echoes the cached flag for the attack
+    -- slot, so it can't detect drift. Fall back to the flag only if the slot is unknown.
     local isAttacking = CleveRoids.CurrentSpell.autoAttack
-    if not isAttacking then
-        -- Fallback: check action bar state via IsCurrentAction
-        local slot = CleveRoids.GetProxyActionSlot(CleveRoids.Localized.Attack)
-        if slot and IsCurrentAction(slot) then
-            CleveRoids.CurrentSpell.autoAttack = true
-            isAttacking = true
-        end
+    local slot = CleveRoids.GetProxyActionSlot(CleveRoids.Localized.Attack)
+    if slot then
+        isAttacking = CleveRoids.Hooks.IsCurrentAction(slot) and true or false
+        CleveRoids.CurrentSpell.autoAttack = isAttacking
     end
     if not isAttacking and not CleveRoids.CurrentSpell.autoAttackLock and UnitExists("target") and UnitCanAttack("player", "target") then
         CleveRoids.CurrentSpell.autoAttackLock = true
@@ -4425,9 +4427,11 @@ function GameTooltip.SetAction(self, slot)
 
         local current_spell_data = CleveRoids.GetSpell(action_name)
         if current_spell_data and current_spell_data.id then
-            -- ClassicAPI: render by spellID (rank included) instead of routing
-            -- through the spellbook slot via SetSpell(spellSlot, bookType).
-            GameTooltip:SetSpellByID(current_spell_data.id)
+            if current_spell_data.spellSlot and current_spell_data.bookType then
+                GameTooltip:SetSpell(current_spell_data.spellSlot, current_spell_data.bookType)
+            else
+                GameTooltip:SetSpellByID(current_spell_data.id)
+            end
             GameTooltip:Show()
             return
         end
@@ -4453,7 +4457,11 @@ function GameTooltip.SetAction(self, slot)
 
             current_spell_data = CleveRoids.GetSpell(nested_action_name)
             if current_spell_data and current_spell_data.id then
-                GameTooltip:SetSpellByID(current_spell_data.id)
+                if current_spell_data.spellSlot and current_spell_data.bookType then
+                    GameTooltip:SetSpell(current_spell_data.spellSlot, current_spell_data.bookType)
+                else
+                    GameTooltip:SetSpellByID(current_spell_data.id)
+                end
                 GameTooltip:Show()
                 return
             end
@@ -4650,6 +4658,10 @@ function IsCurrentAction(slot)
     else
         local name
         if actionToCheck.spell then
+            if CleveRoids.IsAutoAttackSpell(actionToCheck.spell) then
+                return CleveRoids.CurrentSpell.autoAttack and 1 or nil
+            end
+
             local rank = actionToCheck.spell.rank or actionToCheck.spell.highest.rank
             name = actionToCheck.spell.name..(rank and ("("..rank..")"))
 
@@ -4692,9 +4704,6 @@ function IsCurrentAction(slot)
     end
 end
 
--- Macro icon for an action slot, via ClassicAPI's GetActionInfo (macro slot
--- directly, no GetActionText -> GetMacroIndexByName name round-trip).
--- Returns the macro texture, or nil if the slot isn't a macro / has no icon.
 local function GetSlotMacroTexture(slot)
     local kind, macroId = CleveRoids.ClassicAPI.GetActionInfo(slot)
     if kind == "macro" and macroId then
@@ -4704,6 +4713,18 @@ local function GetSlotMacroTexture(slot)
     return nil
 end
 
+local function IsAutoAttackSpell(spell)
+    if not spell then return false end
+    if C_Spell and C_Spell.IsAutoAttackSpell and spell.id then
+        return C_Spell.IsAutoAttackSpell(spell.id)
+    end
+    if C_SpellBook and C_SpellBook.IsAutoAttackSpellBookItem and spell.spellSlot then
+        return C_SpellBook.IsAutoAttackSpellBookItem(spell.spellSlot, spell.bookType)
+    end
+    return false
+end
+CleveRoids.IsAutoAttackSpell = IsAutoAttackSpell
+
 CleveRoids.Hooks.GetActionTexture = GetActionTexture
 function GetActionTexture(slot)
     if not slot then return nil end
@@ -4792,6 +4813,13 @@ function GetActionTexture(slot)
             end
         end
 
+        if a and a.spell and CleveRoids.IsAutoAttackSpell(a.spell) then
+            local mainHandTexture = GetInventoryItemTexture("player", 16)
+            if mainHandTexture then
+                texture = mainHandTexture
+            end
+        end
+
         if texture then
             return texture
         end
@@ -5088,6 +5116,7 @@ CleveRoids.Frame:RegisterEvent("PLAYER_REGEN_DISABLED") -- Entered actual combat
 CleveRoids.Frame:RegisterEvent("PLAYER_REGEN_ENABLED")  -- Left actual combat (no threat)
 CleveRoids.Frame:RegisterEvent("UPDATE_SHAPESHIFT_FORM")
 CleveRoids.Frame:RegisterEvent("SPELL_UPDATE_COOLDOWN")
+CleveRoids.Frame:RegisterEvent("PLAYER_STARTED_MOVING")
 -- Use GUID events when available (v2.39+), fall back to standard per-token events
 if CleveRoids.NampowerAPI.features.hasUnitGuidEvents then
     CleveRoids.Frame:RegisterEvent("UNIT_AURA_GUID")
@@ -5804,6 +5833,48 @@ function CleveRoids.Frame:PLAYER_REGEN_ENABLED()
     end
 end
 
+-- Movement -> [moving]/[nomoving] icon refresh.
+-- PLAYER_STARTED_MOVING is reliable; PLAYER_STOPPED_MOVING is not (key-release
+-- based, misses geometry/click-to-move/root/knockback stops). So STARTED flips
+-- the icon to "moving" and starts a bounded poll; the poll detects the real
+-- stop from IsPlayerMoving() (the signal [moving] uses), refreshes once, and
+-- shuts itself off. Only runs while actually moving, so no idle cost.
+local MOVE_POLL_INTERVAL = 0.1
+local moveTicker = nil
+
+local function StopMovePoll()
+    if moveTicker then
+        moveTicker:Cancel()
+        moveTicker = nil
+    end
+end
+
+local function StartMovePoll()
+    if moveTicker then return end  -- already polling this movement
+    moveTicker = C_Timer.NewTicker(MOVE_POLL_INTERVAL, function()
+        if CleveRoids.isShuttingDown then
+            StopMovePoll()
+            return
+        end
+        if not CleveRoids.IsPlayerMoving() then
+            -- Movement actually ended -- repaint [moving]/[nomoving] icons, stop polling.
+            StopMovePoll()
+            if CleveRoidMacros.realtime == 0 then
+                CleveRoids.QueueActionUpdate()
+            end
+        end
+    end)
+end
+
+function CleveRoids.Frame:PLAYER_STARTED_MOVING()
+    -- Started moving: flip to the [moving] icon now...
+    if CleveRoidMacros.realtime == 0 then
+        CleveRoids.QueueActionUpdate()
+    end
+    -- ...and watch for the (unreliable-event) stop ourselves.
+    StartMovePoll()
+end
+
 function CleveRoids.Frame:PLAYER_TARGET_CHANGED()
     CleveRoids.CurrentSpell.autoAttack = false
     CleveRoids.CurrentSpell.autoAttackLock = false