Skip to content

Handle missing cooldown data for aura countdown text - #2

Merged
DanderBot merged 2 commits into
mainfrom
codex/add-countdown-formatting-options-for-buffs-7lgski
Dec 1, 2025
Merged

Handle missing cooldown data for aura countdown text#2
DanderBot merged 2 commits into
mainfrom
codex/add-countdown-formatting-options-for-buffs-7lgski

Conversation

@DanderBot

Copy link
Copy Markdown
Owner

Summary

  • capture cooldown start/duration from existing cooldown state when SetCooldown wasn't hooked
  • ensure countdown text shows by recovering timers before formatting remaining time

Testing

  • not run (not requested)

Codex Task

@DanderBot
DanderBot merged commit 8d7ed70 into main Dec 1, 2025
@DanderBot
DanderBot deleted the codex/add-countdown-formatting-options-for-buffs-7lgski branch February 14, 2026 13:29
DanderBot added a commit that referenced this pull request Apr 9, 2026
UpdateAbsorb was the #2 CPU consumer after the Dispel refactor landed:
16.1% of DF CPU, ~111 us/call, peak 96 calls per game frame in raid.
The function was doing a full layout rebuild on every invocation —
~20 frame API calls for anchors/strata/level/orientation, texture +
color + blend mode re-apply, plus up to ~12 more API calls for the
overshield glow repositioning. Almost all of it was layout work that
only needs to run when settings actually change.

Same pattern as the Dispel overlay refactor (cd6746e): compare the
current layout settings against a cache stored on the frame, and
take a minimal fast path when nothing has changed.

Three changes:

1. Delete dead Blizzard-visuals hiding code

   The old code had:

     local glow = frame.overAbsorbGlow
     local absorbFrame = frame.totalAbsorb
     local overlay = frame.totalAbsorbOverlay
     if absorbFrame then absorbFrame:Hide() end
     if overlay then overlay:Hide() end
     if glow then glow:Hide() end

   Those fields (totalAbsorb / overAbsorbGlow / totalAbsorbOverlay)
   are Blizzard CompactUnitFrame elements. DF frames are built from
   SecureUnitButtonTemplate and DO NOT inherit those fields. A grep
   for those field names across the entire codebase confirmed they
   are never created, set, or read on DF frames anywhere. The Hide()
   guards were nil-checks that always failed — the calls literally
   never executed. Dead defensive code, removed entirely.

2. Add AbsorbLayoutStateChanged(frame, db) + CacheAbsorbLayoutState

   Compares ~25 layout settings + parent healthBar dimensions against
   values cached on frame.dfAbsorbState:

     * mode, strata, texture, color (rgba), blendMode, pixelPerfect
     * oorEnabled + oorAbsorbBarAlpha
     * Frame-border settings (affect inset calculations)
     * Floating-mode: orientation, width, height, anchor, x, y,
       reverse, frameLevel, background color
     * Attached/overlay: healthOrientation, overlayReverse, clampMode
     * Parent healthBar width/height (detects frame resize)
     * Overshield: enabled, style, color, alpha, reverse

   Returns true if anything changed (-> full rebuild needed).
   Returns false when everything matches (-> fast path safe).

   The cache is stored directly on the frame so there's no global
   state to worry about. DF:InvalidateAbsorbLayout(frame) is exposed
   for any future code that needs to force a rebuild.

3. Mode-aware early-exit fast path

   When AbsorbLayoutStateChanged returns false AND the bar is shown,
   take one of two fast paths:

   OVERLAY / FLOATING modes:
     customBar:SetMinMaxValues(0, maxHealth)
     DF.SetBarValue(customBar, absorbs, frame)
     -- OOR alpha refresh
     return

   ATTACHED / ATTACHED_OVERFLOW modes:
     -- isClamped is a secret bool that changes per-event, so we
     -- still need to query the calculator API every call. The
     -- calculator object itself is cached on the frame.
     UnitGetDetailedHealPrediction(unit, nil, frame.absorbCalculator)
     local attachedAbsorbs, isClamped = calc:GetDamageAbsorbs()

     customBar:SetMinMaxValues(0, maxHealth)
     DF.SetBarValue(customBar, attachedAbsorbs, frame)

     -- ATTACHED_OVERFLOW: secret-safe visibility toggle between
     -- the attached bar and the overflow bar via visibility helpers
     -- + SetAlphaFromBoolean(isClamped, ...).
     -- ATTACHED: update overshield glow alpha via SetAlphaFromBoolean.
     return

   The full rebuild path (everything past the fast-path block) runs
   unchanged when any layout setting differs. CacheAbsorbLayoutState
   is called at the very end of the rebuild so subsequent calls can
   short-circuit.

   The defensive early-returns inside the rebuild path (no
   healthFillTexture) intentionally skip CacheAbsorbLayoutState —
   if the bar couldn't be fully set up, the cache stays empty/stale
   and the next call will re-attempt the full rebuild.

Expected profile impact (extrapolating from the raid baseline):

  OVERLAY / FLOATING modes (the typical fast path):
    us/call:  ~100-110 us  ->  ~15-25 us  (-80%)

  ATTACHED / ATTACHED_OVERFLOW modes (fast path with calculator):
    us/call:  ~130-150 us  ->  ~40-60 us  (-65%)
    The calculator API call is unavoidable per-event because
    isClamped is a secret bool that changes independently of the
    absorb value itself.

  CPU share overall:  16.1%  ->  ~3-5%
  Expected CPU freed in raid:  ~8-10 ms/sec

Verification checklist:

  [x] Fast path runs when layout is stable (common case in combat)
  [x] Full rebuild runs when mode changes
  [x] Full rebuild runs when user changes absorb settings in options
  [x] Full rebuild runs when frame is resized (parent dimensions check)
  [x] Overshield glow visibility still toggles with isClamped in ATTACHED
  [x] Overflow bar visibility still toggles with isClamped in ATTACHED_OVERFLOW
  [x] OOR alpha still applies on fast path (range state can change
      independently of layout settings)
  [x] Test mode still works (frame.dfAbsorbState is cleared on mode
      entry/exit paths via the full rebuild)
  [x] PerfTest disable path clears dfAbsorbState so re-enabling
      does a full rebuild

Rollback: git reset --hard HEAD^ if anything breaks. The previous
state is also captured by the dispel-fix-complete tag (cd6746e)
for deeper rollback if multiple recent commits need to be reverted.
DanderBot added a commit that referenced this pull request Apr 10, 2026
Replaces the scaffolded stubs with the full cast-lifecycle
implementation. Debug output via DF:Debug("TARGETEDLIST", ...) is
used as a stand-in for visual bars until commit #5 lands the render
pipeline — this lets in-game verification happen against the debug
console without requiring the full rendering work first.

All 13 correctness gotchas from the TS3 cross-reference in
_Reference/targeted-spells-findings.md are handled and tagged inline:

  #1  0.2s delay before reading cast data (start events fire before
      UnitSpellTargetName / duration are populated)
  #2  Cast-ID matching at both pickup (via UnitCastingInfo) and stop
      (via event castGuid) — prevents rapid-restart flicker
  #3  UNIT_SPELLCAST_SUCCEEDED during an active channel is treated
      as a no-op (channel tick, not a stop)
  #4  INTERRUPTED with nil interrupter or a dead caster is treated
      as a normal stop (no interrupter flash)
  #6  Empower spellId/castGuid offset handled (same shape as regular
      start on current retail — documented inline)
  #7  Interrupter lookup via UnitNameFromGUID / UnitClassFromGUID
  #8  Uninterruptible flag from UnitCastingInfo/UnitChannelInfo is
      treated as secret-tainted and only ever fed to
      SetVertexColorFromBoolean (applied in commit #5)
  #9  UNIT_SPELLCAST_INTERRUPTIBLE / NOT_INTERRUPTIBLE overwrite the
      stored uninterruptible field with a clean boolean
  #11 LOADING_SCREEN_DISABLED triggers a full release sweep

Also adds the cast-targeting filter pipeline:

  * TargetedList_IsRelevantCaster filters out non-nameplate units,
    friendly units, and party-member nameplates
  * TargetedList_ReadCastData unifies UnitCastingInfo / UnitChannelInfo
    read paths, handling the positional offset difference for
    castID vs notInterruptible between the two APIs
  * Important-spells filter via C_Spell.IsSpellImportant
  * Hide-own-casts filter
  * Roster name cache used for O(1) target lookup

Event dispatcher extended to route NAME_PLATE_UNIT_ADDED and
UNIT_TARGET through the start handler so new nameplates and mid-cast
target swaps are picked up. UNIT_TARGET also fires an immediate stop
to drop the old tracking — the 0.2s delayed re-pickup recreates it
if the new target is still a party member.

Content-type filter (gotcha #13) is intentionally deferred to
commit #5 since the existing TargetedSpells content-type detection
can be reused rather than duplicated.
DanderBot added a commit that referenced this pull request Apr 29, 2026
Blizzard's API now validates the isContainer field explicitly. The per-
slot reanchor call was the only AddPrivateAuraAnchor site that wasn't
passing it, throwing 'bad argument #2 [isContainer]' on every roster
shift. The pcall removal surfaced this — it was silently failing before
and getting recovered from on the next full Setup.
Krathe82 pushed a commit to Krathe82/DandersFrames that referenced this pull request Jun 6, 2026
UpdateAbsorb was the DanderBot#2 CPU consumer after the Dispel refactor landed:
16.1% of DF CPU, ~111 us/call, peak 96 calls per game frame in raid.
The function was doing a full layout rebuild on every invocation —
~20 frame API calls for anchors/strata/level/orientation, texture +
color + blend mode re-apply, plus up to ~12 more API calls for the
overshield glow repositioning. Almost all of it was layout work that
only needs to run when settings actually change.

Same pattern as the Dispel overlay refactor (cd6746e): compare the
current layout settings against a cache stored on the frame, and
take a minimal fast path when nothing has changed.

Three changes:

1. Delete dead Blizzard-visuals hiding code

   The old code had:

     local glow = frame.overAbsorbGlow
     local absorbFrame = frame.totalAbsorb
     local overlay = frame.totalAbsorbOverlay
     if absorbFrame then absorbFrame:Hide() end
     if overlay then overlay:Hide() end
     if glow then glow:Hide() end

   Those fields (totalAbsorb / overAbsorbGlow / totalAbsorbOverlay)
   are Blizzard CompactUnitFrame elements. DF frames are built from
   SecureUnitButtonTemplate and DO NOT inherit those fields. A grep
   for those field names across the entire codebase confirmed they
   are never created, set, or read on DF frames anywhere. The Hide()
   guards were nil-checks that always failed — the calls literally
   never executed. Dead defensive code, removed entirely.

2. Add AbsorbLayoutStateChanged(frame, db) + CacheAbsorbLayoutState

   Compares ~25 layout settings + parent healthBar dimensions against
   values cached on frame.dfAbsorbState:

     * mode, strata, texture, color (rgba), blendMode, pixelPerfect
     * oorEnabled + oorAbsorbBarAlpha
     * Frame-border settings (affect inset calculations)
     * Floating-mode: orientation, width, height, anchor, x, y,
       reverse, frameLevel, background color
     * Attached/overlay: healthOrientation, overlayReverse, clampMode
     * Parent healthBar width/height (detects frame resize)
     * Overshield: enabled, style, color, alpha, reverse

   Returns true if anything changed (-> full rebuild needed).
   Returns false when everything matches (-> fast path safe).

   The cache is stored directly on the frame so there's no global
   state to worry about. DF:InvalidateAbsorbLayout(frame) is exposed
   for any future code that needs to force a rebuild.

3. Mode-aware early-exit fast path

   When AbsorbLayoutStateChanged returns false AND the bar is shown,
   take one of two fast paths:

   OVERLAY / FLOATING modes:
     customBar:SetMinMaxValues(0, maxHealth)
     DF.SetBarValue(customBar, absorbs, frame)
     -- OOR alpha refresh
     return

   ATTACHED / ATTACHED_OVERFLOW modes:
     -- isClamped is a secret bool that changes per-event, so we
     -- still need to query the calculator API every call. The
     -- calculator object itself is cached on the frame.
     UnitGetDetailedHealPrediction(unit, nil, frame.absorbCalculator)
     local attachedAbsorbs, isClamped = calc:GetDamageAbsorbs()

     customBar:SetMinMaxValues(0, maxHealth)
     DF.SetBarValue(customBar, attachedAbsorbs, frame)

     -- ATTACHED_OVERFLOW: secret-safe visibility toggle between
     -- the attached bar and the overflow bar via visibility helpers
     -- + SetAlphaFromBoolean(isClamped, ...).
     -- ATTACHED: update overshield glow alpha via SetAlphaFromBoolean.
     return

   The full rebuild path (everything past the fast-path block) runs
   unchanged when any layout setting differs. CacheAbsorbLayoutState
   is called at the very end of the rebuild so subsequent calls can
   short-circuit.

   The defensive early-returns inside the rebuild path (no
   healthFillTexture) intentionally skip CacheAbsorbLayoutState —
   if the bar couldn't be fully set up, the cache stays empty/stale
   and the next call will re-attempt the full rebuild.

Expected profile impact (extrapolating from the raid baseline):

  OVERLAY / FLOATING modes (the typical fast path):
    us/call:  ~100-110 us  ->  ~15-25 us  (-80%)

  ATTACHED / ATTACHED_OVERFLOW modes (fast path with calculator):
    us/call:  ~130-150 us  ->  ~40-60 us  (-65%)
    The calculator API call is unavoidable per-event because
    isClamped is a secret bool that changes independently of the
    absorb value itself.

  CPU share overall:  16.1%  ->  ~3-5%
  Expected CPU freed in raid:  ~8-10 ms/sec

Verification checklist:

  [x] Fast path runs when layout is stable (common case in combat)
  [x] Full rebuild runs when mode changes
  [x] Full rebuild runs when user changes absorb settings in options
  [x] Full rebuild runs when frame is resized (parent dimensions check)
  [x] Overshield glow visibility still toggles with isClamped in ATTACHED
  [x] Overflow bar visibility still toggles with isClamped in ATTACHED_OVERFLOW
  [x] OOR alpha still applies on fast path (range state can change
      independently of layout settings)
  [x] Test mode still works (frame.dfAbsorbState is cleared on mode
      entry/exit paths via the full rebuild)
  [x] PerfTest disable path clears dfAbsorbState so re-enabling
      does a full rebuild

Rollback: git reset --hard HEAD^ if anything breaks. The previous
state is also captured by the dispel-fix-complete tag (cd6746e)
for deeper rollback if multiple recent commits need to be reverted.
Krathe82 pushed a commit to Krathe82/DandersFrames that referenced this pull request Jun 6, 2026
Replaces the scaffolded stubs with the full cast-lifecycle
implementation. Debug output via DF:Debug("TARGETEDLIST", ...) is
used as a stand-in for visual bars until commit DanderBot#5 lands the render
pipeline — this lets in-game verification happen against the debug
console without requiring the full rendering work first.

All 13 correctness gotchas from the TS3 cross-reference in
_Reference/targeted-spells-findings.md are handled and tagged inline:

  #1  0.2s delay before reading cast data (start events fire before
      UnitSpellTargetName / duration are populated)
  DanderBot#2  Cast-ID matching at both pickup (via UnitCastingInfo) and stop
      (via event castGuid) — prevents rapid-restart flicker
  DanderBot#3  UNIT_SPELLCAST_SUCCEEDED during an active channel is treated
      as a no-op (channel tick, not a stop)
  DanderBot#4  INTERRUPTED with nil interrupter or a dead caster is treated
      as a normal stop (no interrupter flash)
  DanderBot#6  Empower spellId/castGuid offset handled (same shape as regular
      start on current retail — documented inline)
  DanderBot#7  Interrupter lookup via UnitNameFromGUID / UnitClassFromGUID
  DanderBot#8  Uninterruptible flag from UnitCastingInfo/UnitChannelInfo is
      treated as secret-tainted and only ever fed to
      SetVertexColorFromBoolean (applied in commit DanderBot#5)
  DanderBot#9  UNIT_SPELLCAST_INTERRUPTIBLE / NOT_INTERRUPTIBLE overwrite the
      stored uninterruptible field with a clean boolean
  DanderBot#11 LOADING_SCREEN_DISABLED triggers a full release sweep

Also adds the cast-targeting filter pipeline:

  * TargetedList_IsRelevantCaster filters out non-nameplate units,
    friendly units, and party-member nameplates
  * TargetedList_ReadCastData unifies UnitCastingInfo / UnitChannelInfo
    read paths, handling the positional offset difference for
    castID vs notInterruptible between the two APIs
  * Important-spells filter via C_Spell.IsSpellImportant
  * Hide-own-casts filter
  * Roster name cache used for O(1) target lookup

Event dispatcher extended to route NAME_PLATE_UNIT_ADDED and
UNIT_TARGET through the start handler so new nameplates and mid-cast
target swaps are picked up. UNIT_TARGET also fires an immediate stop
to drop the old tracking — the 0.2s delayed re-pickup recreates it
if the new target is still a party member.

Content-type filter (gotcha DanderBot#13) is intentionally deferred to
commit DanderBot#5 since the existing TargetedSpells content-type detection
can be reused rather than duplicated.
Krathe82 pushed a commit to Krathe82/DandersFrames that referenced this pull request Jun 6, 2026
Blizzard's API now validates the isContainer field explicitly. The per-
slot reanchor call was the only AddPrivateAuraAnchor site that wasn't
passing it, throwing 'bad argument DanderBot#2 [isContainer]' on every roster
shift. The pcall removal surfaced this — it was silently failing before
and getting recovered from on the next full Setup.
DanderBot pushed a commit that referenced this pull request Jun 18, 2026
…mbat

An Aura Designer icon/square/bar border with a Gradient base style and Expiring
Colour Override flattens to SOLID below threshold. In combat the expiring colour
is resolved through the aura's secret Duration object, so its channels are
secret — and the solid branch's gradient-clear did CreateColor(secret) ->
SetGradient, which throws 'bad argument #2' (spammed ~58x). The following
SetColorTexture is secret-safe, so only SetGradient choked.

Both gradient-clear sites (Apply's solid branch and border:SetColor) now use the
real colour only when non-secret (unchanged behaviour, keeps the edge-case
pipeline correct) and fall back to a constant white when any channel is secret.
SetColorTexture paints the real, secret-safe colour either way.
DanderBot pushed a commit that referenced this pull request Aug 3, 2026
…acks

resolveAppearance is #2 in every combat trace (13.9% dungeon, 16.1% boss) and
allocates up to four tables per call: two `or {}` fallbacks, the white colour
fallback, and the returned table.

The class-colour branch in UpdateOne called it a SECOND time for the same elem
and globalDefaults -- a deterministic function with identical inputs -- purely to
read one alpha. applyAppearance had already resolved exactly that and its result
is still live in scope (it is passed to mirrorElement further down). Reuse it and
the whole second resolve disappears for every class-coloured element, every tick.

The two `or {}` fallbacks and the white colour now point at module-level shared
tables. Verified safe: every use in TextDesigner reads these fields and nothing
writes them, and mirrorElement reads app.font/fontSize/outline immediately rather
than retaining the table.

☠ Deliberately NOT sharing the returned appearance table. Before the change above
two results were live simultaneously -- the outer one bound for mirrorElement and
the inner class-colour one -- so a single scratch table would have aliased and
corrupted. That trap is gone now that the inner call is, but one aliasing hazard
in a function is enough reason not to introduce a second for one table per call.
DanderBot pushed a commit that referenced this pull request Aug 3, 2026
resolveAppearance is #2 in every combat trace and ran once per element per
frame per tick. ac55fb8 shared its three fallback tables, leaving exactly one
allocation per call -- and that one was 55.7% of all remaining steady-state
allocation, because of how often it runs.

ac55fb8 declined to share the return table, correctly at the time: the
class-colour branch re-resolved the same element just to read one alpha while
the outer result was still bound for mirrorElement, so two results were live at
once and a shared table would have aliased them silently. That commit removed
the second call. The hazard went with it.

Verified before sharing rather than trusting that: applyAppearance is the only
caller of resolveAppearance and updateOne the only caller of applyAppearance;
resolveAppearance is a file-local so nothing outside Render.lua can reach it;
mirrorElement reads app.font/fontSize/outline at the point of use and retains
nothing; and all five fields are reassigned every call so nothing stale carries
over. The invariant and what would break it are written at the declaration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant