perf: avoid repeated charge tooltip scans

.github/workflows/optimize-charge-cache.yml

@@ -1,85 +0,0 @@
-name: Optimize charge cache
-
-on:
-  push:
-    branches:
-      - refactor/consolidate-patch-layers
-
-permissions:
-  contents: write
-
-jobs:
-  optimize:
-    if: github.actor != 'github-actions[bot]'
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v4
-        with:
-          ref: refactor/consolidate-patch-layers
-          fetch-depth: 0
-
-      - name: Optimize explicit charge detection cache
-        shell: python
-        run: |
-          from pathlib import Path
-          import re
-
-          path = Path('Core/ItemDetectionClassicAPI.lua')
-          text = path.read_text(encoding='utf-8')
-
-          old = '''local detectionCache = {}\nlocal cacheHits = 0\nlocal cacheMisses = 0\n'''
-          new = '''local detectionCache = {}\nlocal cacheHits = 0\nlocal cacheMisses = 0\n\n-- Explicit charge overlays are cached per live slot. Negative results are kept\n-- across ordinary BAG_UPDATE events so normal stacks do not trigger a tooltip\n-- scan every time their count changes.\nlocal chargesCache = {}\nlocal chargeCapableLinks = {}\n'''
-          if old not in text:
-              raise SystemExit('Top cache declaration block not found')
-          text = text.replace(old, new, 1)
-
-          old = '''    local numLines = tooltip:NumLines() or 0\n    local complete = numLines >= 2\n\n    for i = 1, numLines do\n'''
-          new = '''    local numLines = tooltip:NumLines() or 0\n    local complete = numLines >= 2\n    local explicitCharges = nil\n\n    for i = 1, numLines do\n'''
-          if old not in text:
-              raise SystemExit('ScanProperties numLines block not found')
-          text = text.replace(old, new, 1)
-
-          old = '''        local leftLower = leftText ~= "" and string.lower(leftText) or ""\n\n        local lr, lg, lb = 1, 1, 1\n'''
-          new = '''        local leftLower = leftText ~= "" and string.lower(leftText) or ""\n\n        -- Reuse this same tooltip pass for charge detection. This avoids a\n        -- second synchronous tooltip scan later when ItemButton asks for xN.\n        local _, _, chargeCount = string.find(leftLower, "^(%d+) charges?$")\n        if chargeCount then\n            explicitCharges = tonumber(chargeCount)\n        end\n\n        local lr, lg, lb = 1, 1, 1\n'''
-          if old not in text:
-              raise SystemExit('ScanProperties line text block not found')
-          text = text.replace(old, new, 1)
-
-          old = '''    if result.isPermanentEnchant then\n        result.isQuestItem = false\n        result.isQuestStarter = false\n        result.isQuestUsable = false\n    end\n\n    return result, complete\nend\n'''
-          new = '''    if result.isPermanentEnchant then\n        result.isQuestItem = false\n        result.isQuestStarter = false\n        result.isQuestUsable = false\n    end\n\n    -- Seed the per-slot charge cache from the property scan we already paid\n    -- for. Store the item link with the result so slot swaps cannot reuse stale\n    -- charge data.\n    if bagID and slotID and complete then\n        local slotKey = bagID .. ":" .. slotID\n        chargesCache[slotKey] = {\n            link = itemData.link,\n            charges = explicitCharges or false,\n        }\n        if explicitCharges and itemData.link then\n            chargeCapableLinks[itemData.link] = true\n        end\n    end\n\n    return result, complete\nend\n'''
-          if old not in text:
-              raise SystemExit('ScanProperties return block not found')
-          text = text.replace(old, new, 1)
-
-          # Remove the obsolete early ClassicAPI charge override; the explicit
-          # tooltip-compatible implementation below is authoritative and avoids
-          # the historical normal-stack x20 regression.
-          pattern = re.compile(
-              r'\n-- ClassicAPI can read charges directly from the live item descriptor, making\n'
-              r'.*?'
-              r'\naddon:Debug\("ClassicAPI single-pass ItemDetection enabled"\)\n',
-              re.S,
-          )
-          replacement = '\naddon:Debug("ClassicAPI single-pass ItemDetection enabled")\n'
-          text, count = pattern.subn(replacement, text, count=1)
-          if count != 1:
-              raise SystemExit(f'Expected one obsolete ClassicAPI charge block, found {count}')
-
-          start = text.find('--=====================================================\n-- Consolidated from Core/ItemChargesClassicAPI.lua')
-          end = text.find('--=====================================================\n-- Consolidated from Core/ItemDetectionCacheSafety.lua')
-          if start < 0 or end < 0 or end <= start:
-              raise SystemExit('Consolidated charge block markers not found')
-
-          charge_block = '''--=====================================================\n-- Explicit charge overlay cache\n--=====================================================\n-- ClassicAPI charge values can mirror ordinary stack counts on this client,\n-- so a yellow xN overlay is still shown only after the tooltip explicitly\n-- confirms a Charges line. The cache below keeps negative results across bag\n-- updates and invalidates real charge items conservatively.\n\nlocal function ChargeSafeSetHyperlink(tooltip, link)\n    if not link then return false end\n    local _, _, bare = string.find(link, "|H(item:[^|]+)|h")\n    if not bare and string.find(link, "^item:") then bare = link end\n    if not bare then return false end\n    return pcall(tooltip.SetHyperlink, tooltip, bare)\nend\n\nlocal function GetExplicitTooltipCharges(itemData, bagID, slotID)\n    local tooltip, tooltipName = addon.Modules.Utils:GetScanTooltip()\n    tooltip:SetOwner(WorldFrame, "ANCHOR_NONE")\n    tooltip:ClearLines()\n\n    local ok = false\n    if bagID and slotID and bagID ~= -1 then\n        ok = pcall(tooltip.SetBagItem, tooltip, bagID, slotID)\n    end\n    if not ok then\n        ok = ChargeSafeSetHyperlink(tooltip, itemData and itemData.link)\n    end\n    if not ok then return nil, false end\n\n    local numLines = tooltip:NumLines() or 0\n    for i = 1, numLines do\n        local line = getglobal(tooltipName .. "TextLeft" .. i)\n        local text = line and line:GetText()\n        if text then\n            local _, _, num = string.find(string.lower(text), "^(%d+) charges?$")\n            if num then return tonumber(num), numLines >= 2 end\n        end\n    end\n    return nil, numLines >= 2\nend\n\nfunction ItemDetection:GetCharges(itemData, bagID, slotID)\n    if not bagID or not slotID then return nil end\n\n    local slotKey = bagID .. ":" .. slotID\n    local itemLink = itemData and itemData.link or nil\n    local cached = chargesCache[slotKey]\n\n    -- A slot cache is valid only for the exact item link currently occupying\n    -- it. Normal stacks usually hit the cached `false` path here with no\n    -- tooltip work at all.\n    if cached and cached.link == itemLink then\n        if cached.charges == false then return nil end\n        return cached.charges\n    end\n\n    local charges, complete = GetExplicitTooltipCharges(itemData, bagID, slotID)\n    if complete then\n        chargesCache[slotKey] = {\n            link = itemLink,\n            charges = charges or false,\n        }\n        if charges and itemLink then\n            chargeCapableLinks[itemLink] = true\n        end\n    end\n    return charges\nend\n\nfunction ItemDetection:InvalidateCharges(bagID)\n    if not bagID then\n        chargesCache = {}\n        chargeCapableLinks = {}\n        return\n    end\n\n    local prefix = bagID .. ":"\n    for key, cached in pairs(chargesCache) do\n        if string.find(key, "^" .. prefix) then\n            -- Keep proven negative results for ordinary items. A changed slot\n            -- is still safe because GetCharges validates cached.link against\n            -- the current item link. Real/known charge items are discarded so\n            -- their remaining charge count is refreshed exactly from tooltip.\n            if not cached or cached.charges ~= false\n               or (cached.link and chargeCapableLinks[cached.link]) then\n                chargesCache[key] = nil\n            end\n        end\n    end\nend\n\naddon:Debug("Explicit charge cache optimization enabled")\n\n'''
-          text = text[:start] + charge_block + text[end:]
-
-          path.write_text(text, encoding='utf-8')
-          Path('.github/workflows/optimize-charge-cache.yml').unlink()
-
-      - name: Commit optimized charge cache
-        run: |
-          git config user.name "github-actions[bot]"
-          git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
-          git add -A
-          git commit -m "perf: avoid repeated charge tooltip scans"
-          git push origin HEAD:refactor/consolidate-patch-layers

Core/ItemDetectionClassicAPI.lua

@@ -12,6 +12,12 @@ local detectionCache = {}
 local cacheHits = 0
 local cacheMisses = 0
 
+-- Explicit charge overlays are cached per live slot. Negative results are kept
+-- across ordinary BAG_UPDATE events so normal stacks do not trigger a tooltip
+-- scan every time their count changes.
+local chargesCache = {}
+local chargeCapableLinks = {}
+
 local EMPTY_PROPERTIES = {
     isQuestItem = false,
     isQuestStarter = false,
@@ -199,6 +205,7 @@ local function ScanProperties(itemData, bagID, slotID)
 
     local numLines = tooltip:NumLines() or 0
     local complete = numLines >= 2
+    local explicitCharges = nil
 
     for i = 1, numLines do
         local leftLine = getglobal(tooltipName .. "TextLeft" .. i)
@@ -207,6 +214,13 @@ local function ScanProperties(itemData, bagID, slotID)
         local rightText = rightLine and rightLine:GetText() or ""
         local leftLower = leftText ~= "" and string.lower(leftText) or ""
 
+        -- Reuse this same tooltip pass for charge detection. This avoids a
+        -- second synchronous tooltip scan later when ItemButton asks for xN.
+        local _, _, chargeCount = string.find(leftLower, "^(%d+) charges?$")
+        if chargeCount then
+            explicitCharges = tonumber(chargeCount)
+        end
+
         local lr, lg, lb = 1, 1, 1
         if leftLine and leftLine.GetTextColor then
             lr, lg, lb = leftLine:GetTextColor()
@@ -271,6 +285,20 @@ local function ScanProperties(itemData, bagID, slotID)
         result.isQuestUsable = false
     end
 
+    -- Seed the per-slot charge cache from the property scan we already paid
+    -- for. Store the item link with the result so slot swaps cannot reuse stale
+    -- charge data.
+    if bagID and slotID and complete then
+        local slotKey = bagID .. ":" .. slotID
+        chargesCache[slotKey] = {
+            link = itemData.link,
+            charges = explicitCharges or false,
+        }
+        if explicitCharges and itemData.link then
+            chargeCapableLinks[itemData.link] = true
+        end
+    end
+
     return result, complete
 end
 
@@ -334,53 +362,17 @@ function ItemDetection:IsUnusableCached(itemData)
     return props.isUnusable
 end
 
--- ClassicAPI can read charges directly from the live item descriptor, making
--- tooltip text parsing unnecessary for the common path.
-function ItemDetection:GetCharges(itemData, bagID, slotID)
-    if not bagID or not slotID then return nil end
-
-    local api = addon.Modules.ClassicAPI
-    if api and api.HasContainerItemCharges and api:HasContainerItemCharges() then
-        return api:GetContainerItemCharges(bagID, slotID)
-    end
-
-    if originalGetCharges then
-        return originalGetCharges(self, itemData, bagID, slotID)
-    end
-    return nil
-end
-
--- No per-slot tooltip charge cache is needed when ClassicAPI provides the
--- descriptor value. Keep this method for callers and for the Vanilla fallback.
-local originalInvalidateCharges = ItemDetection.InvalidateCharges
-function ItemDetection:InvalidateCharges(bagID)
-    local api = addon.Modules.ClassicAPI
-    if api and api.HasContainerItemCharges and api:HasContainerItemCharges() then
-        return
-    end
-    if originalInvalidateCharges then
-        return originalInvalidateCharges(self, bagID)
-    end
-end
-
 addon:Debug("ClassicAPI single-pass ItemDetection enabled")
 
 --=====================================================
--- Consolidated from Core/ItemChargesClassicAPI.lua
+-- Explicit charge overlay cache
 --=====================================================
--- Preserve Guda's original charge overlay behavior.
+-- ClassicAPI charge values can mirror ordinary stack counts on this client,
--- ClassicAPI's container charge value is not reliable for distinguishing
+-- so a yellow xN overlay is still shown only after the tooltip explicitly
--- ordinary stack counts from explicit item charges on the target client, so
+-- confirms a Charges line. The cache below keeps negative results across bag
--- only show the yellow "xN" overlay when the item tooltip actually contains a
+-- updates and invalidates real charge items conservatively.
--- Charges line (matching upstream Guda behavior).
 
-local addon = Guda
+local function ChargeSafeSetHyperlink(tooltip, link)
-local ItemDetection = addon.Modules.ItemDetection
-if not ItemDetection then return end
-
-local chargesCache = {}
-
-local function SafeSetHyperlink(tooltip, link)
     if not link then return false end
     local _, _, bare = string.find(link, "|H(item:[^|]+)|h")
     if not bare and string.find(link, "^item:") then bare = link end
@@ -398,7 +390,7 @@ local function GetExplicitTooltipCharges(itemData, bagID, slotID)
         ok = pcall(tooltip.SetBagItem, tooltip, bagID, slotID)
     end
     if not ok then
-        ok = SafeSetHyperlink(tooltip, itemData and itemData.link)
+        ok = ChargeSafeSetHyperlink(tooltip, itemData and itemData.link)
     end
     if not ok then return nil, false end
 
@@ -418,33 +410,53 @@ function ItemDetection:GetCharges(itemData, bagID, slotID)
     if not bagID or not slotID then return nil end
 
     local slotKey = bagID .. ":" .. slotID
+    local itemLink = itemData and itemData.link or nil
     local cached = chargesCache[slotKey]
-    if cached ~= nil then
+
-        if cached == false then return nil end
+    -- A slot cache is valid only for the exact item link currently occupying
-        return cached
+    -- it. Normal stacks usually hit the cached `false` path here with no
+    -- tooltip work at all.
+    if cached and cached.link == itemLink then
+        if cached.charges == false then return nil end
+        return cached.charges
     end
 
     local charges, complete = GetExplicitTooltipCharges(itemData, bagID, slotID)
     if complete then
-        chargesCache[slotKey] = charges or false
+        chargesCache[slotKey] = {
+            link = itemLink,
+            charges = charges or false,
+        }
+        if charges and itemLink then
+            chargeCapableLinks[itemLink] = true
+        end
     end
     return charges
 end
 
 function ItemDetection:InvalidateCharges(bagID)
-    if bagID then
+    if not bagID then
-        local prefix = bagID .. ":"
+        chargesCache = {}
-        for key in pairs(chargesCache) do
+        chargeCapableLinks = {}
-            if string.find(key, "^" .. prefix) then
+        return
+    end
+
+    local prefix = bagID .. ":"
+    for key, cached in pairs(chargesCache) do
+        if string.find(key, "^" .. prefix) then
+            -- Keep proven negative results for ordinary items. A changed slot
+            -- is still safe because GetCharges validates cached.link against
+            -- the current item link. Real/known charge items are discarded so
+            -- their remaining charge count is refreshed exactly from tooltip.
+            if not cached or cached.charges ~= false
+               or (cached.link and chargeCapableLinks[cached.link]) then
                 chargesCache[key] = nil
             end
         end
-    else
-        chargesCache = {}
     end
 end
 
-addon:Debug("Upstream-compatible charge display enabled")
+addon:Debug("Explicit charge cache optimization enabled")
 
 --=====================================================
 -- Consolidated from Core/ItemDetectionCacheSafety.lua