diff --git a/CHANGELOG.md b/CHANGELOG.md index e8225852..af7e8743 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,7 @@ DandersFrames has been rebuilt for WoW 12.1 (Midnight), which fundamentally chan * (Profiles) Custom filters and your category tweaks travel with profile exports — including filters linked in the Aura Designer. * (Profiles) Custom filters picked inside a raid auto layout's settings are handled everywhere the rest of the profile is: deleting a filter now removes it from every layout (including the active one), exports carry filters only a layout references, and imports re-link them instead of pointing at the wrong filter on the receiving account. * (Aura Filters) **New: share a single filter.** Filters can now be exported and imported one at a time, so you can send someone one filter instead of your whole profile. "+ Import Filter" sits under "+ New Buff Filter" in the list, and Export is in the button strip below it. Built-in presets can be shared too — they arrive as a custom filter holding whatever spells you had enabled. If you import a filter you already have, DandersFrames says so and lets you pick between keeping yours or adding a copy. +* (Aura Filters) Importing a filter whose name you already use now asks you to name it, pre-filled with something that doesn't clash — the way Duplicate always has. It previously took the name straight from the string without showing it to you, so you could end up with two rows reading exactly the same thing and no way to tell from the Buffs page which one you'd ticked. Worst through "Import as Copy", which is meant to give you a copy you can tell apart. A clean import, where nothing clashes, is still a single paste with no extra step. *(Reported by Rytiou.)* * (Frames) The Out of Range "Text Alpha" slider now also governs pet frame text and the test-mode preview — those still followed hidden per-element values no control could change. * (Interface) In Aura Filters, a spell added by ID now shows its real name and icon (resolved from the game) instead of a bare "#id" and a question mark. It's tagged "not in database" so you can spot spells that aren't part of a built-in preset yet; only a genuinely invalid ID still reads "unknown ID". * (Interface) Fixed the Aura Filters page's Add, Add from Database, Rename and Delete buttons filling with a solid colour on hover after switching between a built-in preset and a custom filter, and gave their disabled-state tooltips a consistent two-line layout. @@ -104,7 +105,11 @@ DandersFrames has been rebuilt for WoW 12.1 (Midnight), which fundamentally chan * (Class Power) Removed the Class Power Pips display and its settings page, along with its Test Mode toggle. * (Performance) Disabling the Blizzard party/raid frames now fully shuts them down instead of just hiding them, stopping their background work. Toggling this (or Show Side Menu) now prompts for a UI reload. * (Dispel) **The Dispel Overlay is now one unified system driven by the game's aura engine — and it covers boss debuffs (private auras) natively.** The old source selector is a single Enable toggle (existing settings migrate), colours come from the game's dispel palette, and everything else — border, glow, gradient, blend, darken, icons, pulse — stays yours to style, live and in combat. Also fixed a Lua error when opening Edit Mode with the overlay active. -* (Auras) Opening WoW's Edit Mode no longer floods the aura rows with random sample icons — rows hide during Edit Mode and restore when it closes. +* (Auras) **Your aura rows now keep showing your real auras while WoW's Edit Mode is open.** Edit Mode replaces the game's aura data with random sample icons, which used to flood the rows — so they were hidden for as long as Edit Mode stayed open. They now stay up and stay correct, with no flicker going in or out. One side effect worth knowing: Blizzard's own aura displays (the buff frame, the cooldown manager) show your real auras instead of sample ones while you position them, so they look empty if you have no buffs at the time. +* (Test Mode) **Fixed test mode getting stuck on — and, in other orders, switching itself off.** Test frames are wanted by two different things: you, and unlocking (which needs frames on screen to drag). Those were tracked as one setting, so whichever acted last won: turn test mode off during an unlock and the preview could survive every attempt to clear it short of a reload, or turn it *on* during an unlock and locking would take it away again. They are now tracked separately, so locking only ends the preview *unlocking* asked for and leaves yours alone. The "Cannot disable test mode while frames are unlocked" refusal is gone with it — turning test mode off while unlocked now simply leaves the frames unlock still needs, and says so. +* (Test Mode) The toolbar's Test button and the panel's Enable/Disable button are now one control: either turns the preview off and closes the panel, and the button reflects whether *you* have test mode on rather than whether the panel happens to be open. +* (Test Mode) Raid mode now confirms "Test mode enabled/disabled" in chat, which only party mode did. +* (Interface) **Opening WoW's Edit Mode now puts DandersFrames away properly.** The settings window and test panel already closed, but the position panel, the alignment grid, the frame movers and any test preview stayed up — so two grids overlapped and fake frames sat on top of Blizzard's editor. DandersFrames now locks its frames and clears the preview when Edit Mode opens. It does not re-open them when you leave, on purpose. * (Profiles) Refreshed the default profile's look — new installs and profile resets only. Tighter frames, buff borders on, Missing Buffs on by default, a cleaner pet style, and the Big/External Defensive filters no longer default on. * (Profiles) Fixed several border settings (Hide Intro Flash, border Colour Source) silently reverting on profile export/import and party↔raid copies. * (Test Mode) **Rebuilt the Quick Presets** (Static / Combat / Healer / Full). Each preset now sets every toggle, so they're reproducible — "Full" is genuinely everything — and applying one repaints the preview immediately. @@ -179,7 +184,7 @@ DandersFrames has been rebuilt for WoW 12.1 (Midnight), which fundamentally chan * (Frames) **Fixed dead and offline units not updating their status text or their missing-buff badge.** A unit that died kept whatever its frame was showing a moment earlier — no "Dead" or "Offline" status, and a missing-buff badge still asking you to buff someone who could no longer receive it. Both corrected only when something else forced a full refresh, often when you died as well, which is what made it look like your own state mattered. Neither check was wrong; neither was being re-run. Most visible in follower dungeons, where companions go down often. * (Profiles) Fixed Copy, Sync and Reset Page quietly leaving some settings behind on seven pages, with nothing to say they'd been skipped. Fading was the reported case. * (Internal) Code cleanup: removed unreferenced functions, orphaned locale strings and permanently-disabled blocks. No behaviour change. -* (Auras) **New: Important Debuffs.** Boss, role and priority debuffs can now stand out in the normal debuff bar without moving them somewhere else — make them larger and mark them with a warning badge, so the debuff that matters reads at a glance among the rest. The new **Important Debuffs** section on the Debuffs page has the size step, badge size, corner and offset, and colours for both the badge and its mark. Off by default. +* (Auras) **New: Important Debuffs.** Boss, role and priority debuffs can now stand out in the normal debuff bar without moving them somewhere else — make them larger and mark them with a warning badge, so the debuff that matters reads at a glance among the rest. The new **Important Debuffs** section on the Debuffs page has the size step, badge size, corner and offset, and colours for both the badge and its mark. On by default — untick "Important Debuffs" in that section to turn it off. * (Auras) **Targeted List no longer loses the opening cast of a pull.** "Hide Out of Combat" was checking whether the caster was in combat before checking who the cast was aimed at — and a mob whose opening move is a cast isn't flagged in combat yet, so the cast you most want to see was dropped. The setting defaults on, so this happened out of the box (measured at 10 lost casts in a single 19-minute session). It still hides idle NPC chatter, but no longer costs you openers aimed at your group. * (Auras) **Targeted List now picks up casts that were already in progress.** Walk into range, turn the camera, or have a mob join a pull mid-cast and that cast produced no bar at all — it only appeared on the mob's *next* one. Personal Targeted Spells always handled this; the list now does too. * (Auras) **New: "Show Offscreen Nameplates" on the Targeted List and Personal Targeted Spells pages.** Both features can only see a cast if the caster has a nameplate, so with WoW's offscreen-nameplate setting off, an enemy behind you produces no cast bar — confirmed in game with a targeted, visibly-casting mob that wasn't picked up until you turned to face it. The checkbox changes that Blizzard setting directly, so it affects all nameplates and not just cast detection; the tooltip says so plainly. @@ -191,6 +196,17 @@ DandersFrames has been rebuilt for WoW 12.1 (Midnight), which fundamentally chan * (Interface) **The Debug Console is now one system.** Every trace in the addon routes into a single console with named categories, so capturing a problem means turning on the relevant category and reproducing it — rather than hunting for whichever per-feature flag happened to cover that code. Logs persist across `/reload` and can be exported as text, which is the thing to attach to a bug report. Noisy categories stay off unless you ask for them, and every category is listed before it has logged anything, so you can pre-disable the chatty ones and stop the trace you actually want being pushed out by the line cap. The old per-feature debug flags are gone — six of them had already stopped doing anything. * (Interface) **One debug command form: `/df debug `.** The debug commands are down from 36 to 25, with three overlapping pairs merged into single commands (auras, dispel, headers) and developer-only commands no longer listed on normal builds. Note: the standalone one-word `/dfsomething` commands are no longer registered. They put around 25 DandersFrames commands into the global slash namespace, where they collide with other addons and clutter every slash autocomplete, to document a spelling nobody needed twice. A diagnostic is now typed in full — `/dfpixelcheck` becomes `/df debug pixelcheck`, and likewise for the rest — so any macro using a one-word form or the short `/df pixelcheck` spelling needs updating. Everyday commands (`/df test`, `/df lock` and so on) are unchanged. `/rl` is kept, since it has no `/df` form to fall back on. * (Performance) **DandersFrames now uses about 25% less memory.** The settings window, both designers and the debug tools now load only when you actually open them, instead of at every login. You'll see a new **DandersFrames Options** entry in your AddOns list — leave it enabled, it's part of the addon and loads on demand. +* (Performance) **A profiling pass over the whole addon.** Measured before and after against the same scenarios, the addon now allocates around a third less while you're playing, and the one-off spikes are much smaller. The changes are all the same shape — reuse work instead of redoing it: aura rows, filters and sort orders now tune in place where they used to tear down and rebuild; borders, fonts and the Text Designer cache the strings and tables they were rebuilding every frame; and the highlight pass skips entirely when nothing that affects it has changed. +* (Test Mode) **Opening and closing test mode no longer freezes the game for a second or two.** Each preview slot was being given a whole aura group, and every group eagerly builds a batch of icons before its own cap is applied — so a preview that shows five icons was paying for far more. Rows that can only ever show one icon now use a single slot instead of a group, and leaving test mode no longer rebuilds the live frames along with the preview. The toggle is roughly four times cheaper. +* (Auras) **The dispel overlay stopped building itself onto empty raid slots.** Raid frames keep their full set of slots alive and hand units out as the group fills, so in a smaller group most slots hold nobody — and each of those was still getting a complete dispel overlay built onto it, hidden, on a slot that could never show a dispellable aura. Logging in built 85 aura containers where 45 were needed. +* (Auras) **Fixed the Important Debuffs highlight doing nothing for about half the debuffs it covers**, whenever the Blizzard category filters were in use rather than "Show All Debuffs". Boss, role and priority debuffs were being outranked by the Crowd Control, Raid and Dispellable categories, so any important debuff that also carried one of those tags — which most of them do in group content — landed in an unstyled row instead. The important categories now take precedence, and genuinely lead the row. +* (Auras) Fixed a "forbidden object" error storm from the dispel overlay after a dungeon fight, which also left the overlay dead on the affected frames until reload. The overlay's art is built onto the game's protected aura slots, and the game can reclaim a slot underneath us; it now notices and rebuilds instead of erroring on every refresh. +* (Frames) **Fixed the role icon and the role-coloured resource bar showing nothing when your group hasn't assigned roles.** Both read the group's assigned role only, so in a party where nobody has set one — most open-world and follower-dungeon groups — a tank got no tank icon. They now fall back to the role your current spec implies. +* (Interface) **Aggro, hover and selection highlights now draw above the frame's content** instead of being buried under the health bar's overlays, absorb art and text. +* (Interface) **Fixed dragging and slider hit-testing being wrong when the settings window isn't at 100% scale.** Grabbing a slider handle picked the neighbouring one and clicking a track jumped to the wrong value, because the cursor position was being converted using the screen's scale rather than the window's own. +* (Interface) **The settings window's size, position and scale are now remembered account-wide** instead of being stored inside each profile. Switching or creating a profile could previously move, resize or rescale the window out from under you — a new profile is born from the defaults, so its scale was 100% while the open window was still at yours. +* (Profiles) Fixed "Sync with Raid" overwriting the raid frames' own growth direction with the party's. +* (Interface) `/df debug` diagnostics that only concern the main addon no longer pull in the settings addon just to answer. ### Bug Fixes @@ -211,8 +227,7 @@ DandersFrames has been rebuilt for WoW 12.1 (Midnight), which fundamentally chan ### Known Issues (12.1 alpha) * The 12.1 aura displays are rebuilt on Blizzard's new container system and are under active testing — please report any case where buff, debuff, defensive or missing-buff displays stop updating, **especially in combat**. -* A debuff that counts as both a Priority debuff and a Boss/Role debuff can show one icon per matching filter when both are enabled — the only duplicate case the new filter system can't remove. "Show All Debuffs" avoids it. -* Dispel Overlay: a unit with dispellable debuffs of two different types can show both type icons overlapped (rare). +* Dispel Overlay: the dispel-type symbol the overlay draws on the frame can render two symbols stacked on the same spot, when a unit has dispellable debuffs of two different types (rare). The coloured overlay itself is unaffected — it always shows a single type. * Aura Designer text colouring is drawn as a cover over the text: it ignores the out-of-range text fade, and group parts with their own inline colours keep them. * Dragging certain aura sliders can briefly stutter. * Settings-window borders re-derive their thickness from DandersFrames' own **UI Scale** slider (top of the settings window). Changing WoW's global UI Scale instead won't re-derive them for pages already on screen — reopen the window, or nudge the addon's own slider, if a border looks off after doing that. diff --git a/DandersFrames/AuraDesigner/Factory.lua b/DandersFrames/AuraDesigner/Factory.lua index 8b2174c3..8cbb20b5 100644 --- a/DandersFrames/AuraDesigner/Factory.lua +++ b/DandersFrames/AuraDesigner/Factory.lua @@ -423,7 +423,12 @@ end local function colSig(c) if type(c) ~= "table" then return "" end local r, g, b, a = readADColor(c) - return tconcat({ tostring(r), tostring(g), tostring(b), tostring(a) }, ",") + -- Direct concat, not tconcat over a throwaway array: the four-element table was + -- pure garbage, and a single chained concat compiles to ONE concat over a + -- register range, so this allocates the result string and nothing else. colSig + -- runs several times per indicator per UNIT_AURA (once per border colour key), + -- which put it at 3.5% of trash allocation on its own. + return tostring(r) .. "," .. tostring(g) .. "," .. tostring(b) .. "," .. tostring(a) end -- Health-bar overlay alpha per mode — the exact semantics of Indicators:ApplyHealthBar @@ -578,12 +583,22 @@ end -- painted with (field-caught: "I can see the gradient but not the colours I set" -- the -- direction dropdown worked, because direction is a scalar). spec.shadow.color had the -- same latent hole. Depth-capped purely as a cycle guard; real specs are 2-3 deep. +-- ☠ PER-DEPTH SCRATCH, NOT ONE SHARED PAIR — subSig RECURSES. +-- The two throwaway tables per call (keys + parts) ran per indicator per +-- UNIT_AURA inside syncPlacedPool, a walk whose whole point is to be +-- allocation-free; subSig alone was 2.1% of trash allocation and it recurses up +-- to four levels, so a single shared scratch would have a nested call wipe its +-- caller's half-built list. Depth is hard-bounded at 4 by the guard below, so one +-- pair per level is both sufficient and safe. +local subSigKeys, subSigParts = {}, {} local function subSig(t, depth) depth = depth or 1 - local keys = {} + local keys = subSigKeys[depth] + if not keys then keys = {}; subSigKeys[depth] = keys else wipe(keys) end for kk in pairs(t) do keys[#keys + 1] = kk end tsort(keys) - local parts = {} + local parts = subSigParts[depth] + if not parts then parts = {}; subSigParts[depth] = parts else wipe(parts) end for _, kk in ipairs(keys) do local v = t[kk] local tv = type(v) @@ -806,9 +821,15 @@ local PLACED_BORDER_COLOR_KEYS = { "BorderColor", "BorderGradientStartColor", "BorderGradientEndColor", "BorderShadowColor", "BorderAnimationColor", } +-- Shared scratch: unlike subSig this CANNOT recurse (it only reaches colSig, which +-- allocates nothing and calls nothing), so one table is safe. It was 4.8% of trash +-- allocation on its own -- a fresh array per indicator per UNIT_AURA. +local placedBorderSigParts = {} local function placedBorderRawSig(indicator, borderOn) if not borderOn then return "" end - local parts = { ppSigToken() } + local parts = placedBorderSigParts + wipe(parts) + parts[1] = ppSigToken() for _, kk in ipairs(PLACED_BORDER_KEYS) do parts[#parts + 1] = tostring(indicator[kk]) end @@ -1205,6 +1226,12 @@ local function buildPlacedConfig(frame, unit, map, indicator, isSquare, borderSp unit = unit, mode = "row", max = 1, + -- ONE icon, so declare an AuraSlot rather than a one-icon AuraGroup: + -- AddAuraGroup eagerly creates a whole FrameCreationBatchSize batch BEFORE + -- maxFrameCount is applied; AddAuraSlot creates exactly one frame. Same + -- selection (the slot carries the sort comparator), same size + -- (styleButton_regions sizes both paths identically). + singleSlot = true, filter = poolFilter(indicator, mine), -- "HELPFUL|PLAYER" on My Buffs; othersOnly rides the other pool (structural) candidateFilters = { includeSpellIDs = map }, testEntries = testEntryForMap(map), @@ -1231,9 +1258,15 @@ end -- toggling a region OFF must Rebuild the container to drop it; a plain ApplyStyle would leave -- the old region visible. A change here forces a whole-container Rebuild (slots can't be -- patched). Cosmetic styling of a live region is coSig. -local function placedStructSig(map, isSquare, hideIcon, showStacks, showDuration, borderOn, indicator, defs, mine) - return includeSig(map) - .. "|" .. (isSquare and "sq" or "ic") +-- STRUCTURAL signature: CREATE-ONLY properties only. A change here costs a full +-- teardown+recreate -- and teardown can only Hide(), because WoW never destroys frames, +-- so every rebuild permanently strands the container plus a 10-frame batch per group +-- (AddAuraGroup always creates FrameCreationBatchSize frames up front). Anything the +-- native API can mutate live therefore MUST stay out of this sig or it leaks on every +-- edit. The tracked spell-ID map used to live here; it is live-tunable via +-- candidateFilters and now rides placedTuningSig. +local function placedStructSig(isSquare, hideIcon, showStacks, showDuration, borderOn, indicator, defs, mine) + return (isSquare and "sq" or "ic") .. "|" .. (hideIcon and "hi" or "") .. "|" .. (showStacks and "st" or "") .. "|" .. (showDuration and "du" or "") @@ -1257,6 +1290,16 @@ local function placedStructSig(map, isSquare, hideIcon, showStacks, showDuration or "") end +-- TUNING signature: the live-mutable half of what placedStructSig used to carry. The +-- tracked spell-ID map becomes config.candidateFilters ({ includeSpellIDs = map }), and +-- the native SetAuraGroupCandidateFilters mutates that in place — so a selection edit is +-- an ApplyTuning, never a Rebuild. A placed indicator pins max = 1 (buildPlacedConfig) +-- and has no per-indicator sort, so the map IS the whole tuning sig. Mirrors the +-- filter-group path's tuningSig, which has worked this way since Wave 1. +local function placedTuningSig(map) + return includeSig(map) +end + -- COSMETIC signature: size/anchor/offset/scale/alpha, swipe, duration/stack styling, square -- colour, and the RAW-config border sig (no BuildSpec alloc — FIX C). A change here -- hot-applies via ApplyStyle(style, layout); the actual border spec is built only then. @@ -1460,6 +1503,12 @@ local function buildBarConfig(frame, unit, map, indicator, borderSpec, defs, min unit = unit, mode = "row", max = 1, + -- ONE icon, so declare an AuraSlot rather than a one-icon AuraGroup: + -- AddAuraGroup eagerly creates a whole FrameCreationBatchSize batch BEFORE + -- maxFrameCount is applied; AddAuraSlot creates exactly one frame. Same + -- selection (the slot carries the sort comparator), same size + -- (styleButton_regions sizes both paths identically). + singleSlot = true, filter = poolFilter(indicator, mine), -- "HELPFUL|PLAYER" on My Buffs; othersOnly rides the other pool (structural) candidateFilters = { includeSpellIDs = map }, testEntries = testEntryForMap(map), @@ -1547,6 +1596,12 @@ local function buildAlertCompanionConfig(unit, map, indicator, layout, mine, geo unit = unit, mode = "row", max = 1, + -- ONE icon, so declare an AuraSlot rather than a one-icon AuraGroup: + -- AddAuraGroup eagerly creates a whole FrameCreationBatchSize batch BEFORE + -- maxFrameCount is applied; AddAuraSlot creates exactly one frame. Same + -- selection (the slot carries the sort comparator), same size + -- (styleButton_regions sizes both paths identically). + singleSlot = true, filter = poolFilter(indicator, mine), -- mirror the indicator: My Buffs alerts only on YOUR cast candidateFilters = { includeSpellIDs = map }, testEntries = testEntryForMap(map), @@ -1581,6 +1636,12 @@ function Factory:BuildAlertPreviewConfig(indicator, geom, layout, entries) return { mode = "row", max = 1, + -- ONE icon, so declare an AuraSlot rather than a one-icon AuraGroup: + -- AddAuraGroup eagerly creates a whole FrameCreationBatchSize batch BEFORE + -- maxFrameCount is applied; AddAuraSlot creates exactly one frame. Same + -- selection (the slot carries the sort comparator), same size + -- (styleButton_regions sizes both paths identically). + singleSlot = true, filter = "HELPFUL", -- canvas sample: the pool never gates a preview slot testEntries = entries, tooltips = false, @@ -1589,15 +1650,16 @@ function Factory:BuildAlertPreviewConfig(indicator, geom, layout, entries) } end --- Companion sigs. STRUCTURAL: identity map + filter (bind at build), EVERY --- alert key (alertElemStructKey — formatter and placement are creation-frozen --- -> Rebuild), frame level. COSMETIC (ApplyStyle): the mirrored indicator --- geometry — dragging / resizing the indicator hot-moves its companion — plus --- font and alpha. Raw-config, alloc-light, computed per pass like the other --- placed sigs (FIX C discipline). -local function alertCompanionStructSig(map, indicator, mine, geom, defs) - return includeSig(map) - .. "|xalert" +-- Companion sigs. STRUCTURAL: the filter string (binds at build), EVERY alert key +-- (alertElemStructKey — formatter and placement are creation-frozen -> Rebuild), +-- frame level. TUNING: the identity map, via the shared placedTuningSig — it is +-- config.candidateFilters and mutates live, so it must NOT sit here (a Rebuild +-- strands frames permanently; see placedStructSig). COSMETIC (ApplyStyle): the +-- mirrored indicator geometry — dragging / resizing the indicator hot-moves its +-- companion — plus font and alpha. Raw-config, alloc-light, computed per pass like +-- the other placed sigs (FIX C discipline). +local function alertCompanionStructSig(indicator, mine, geom, defs) + return "xalert" .. "|xa=" .. alertElemStructKey(indicator, geom) .. "|fl=" .. tostring(resolveLevel(indicator, defs.level)) .. "|fs=" .. tostring(resolveStrata(indicator, defs.strata) or "") @@ -1634,10 +1696,12 @@ local function syncAlertCompanion(frame, placed, live, key, map, indicator, isBa if not alertElemMode(indicator) then return end local akey = key .. ":alert" local geom = alertGeometry(frame, indicator, isBar) -- square (icon) or rect (bar) - local structSig = alertCompanionStructSig(map, indicator, mine, geom, defs) + local structSig = alertCompanionStructSig(indicator, mine, geom, defs) + local tuningSig = placedTuningSig(map) local coSig = alertCompanionCoSig(frame, indicator, isBar, alpha) local entry = placed[akey] - if entry and entry.structSig == structSig and entry.coSig == coSig then + if entry and entry.structSig == structSig and entry.tuningSig == tuningSig + and entry.coSig == coSig then live[akey] = true -- steady state: no config build, no touch return end @@ -1648,18 +1712,28 @@ local function syncAlertCompanion(frame, placed, live, key, map, indicator, isBa local handle = DF.AuraContainer:Create(frame, cfg) if handle then applyPlacedAlpha(handle, alpha) - placed[akey] = { handle = handle, structSig = structSig, coSig = coSig } + placed[akey] = { handle = handle, structSig = structSig, + tuningSig = tuningSig, coSig = coSig } live[akey] = true end elseif entry.structSig ~= structSig then - entry.structSig, entry.coSig = structSig, coSig + entry.structSig, entry.tuningSig, entry.coSig = structSig, tuningSig, coSig entry.handle:Rebuild(cfg) applyPlacedAlpha(entry.handle, alpha) live[akey] = true else - entry.coSig = coSig - entry.handle:ApplyStyle(cfg.style, cfg.layout) - applyPlacedAlpha(entry.handle, alpha) + -- Selection edit with the struct sig stable: swap the include map on the live + -- container (row mode, so applyGroupTuning runs) instead of recreating it. + if entry.tuningSig ~= tuningSig then + entry.tuningSig = tuningSig + entry.handle.config.testEntries = cfg.testEntries + entry.handle:ApplyTuning(cfg) + end + if entry.coSig ~= coSig then + entry.coSig = coSig + entry.handle:ApplyStyle(cfg.style, cfg.layout) + applyPlacedAlpha(entry.handle, alpha) + end live[akey] = true end end @@ -1698,7 +1772,7 @@ function Factory:BuildPreviewConfig(frame, indicator, typeKey, spellID, defs) local layout = buildBarLayout(frame, indicator) local geom = alertGeometry(frame, indicator, true) local cfg = { - mode = "row", max = 1, filter = "HELPFUL", + mode = "row", max = 1, singleSlot = true, filter = "HELPFUL", adBorderAnim = true, layout = layout, style = buildBarStyle(indicator, borderSpec, defs), @@ -1721,7 +1795,7 @@ function Factory:BuildPreviewConfig(frame, indicator, typeKey, spellID, defs) and buildPlacedBorderSpec(frame, indicator, hideIcon) or nil local layout = buildPlacedLayout(indicator) local cfg = { - mode = "row", max = 1, filter = "HELPFUL", + mode = "row", max = 1, singleSlot = true, filter = "HELPFUL", adBorderAnim = true, layout = layout, style = buildPlacedStyle(indicator, isSquare, borderSpec, defs), @@ -1743,9 +1817,10 @@ end -- STRUCTURAL signature: identity, duration-text on/off + format key (SetDurationText / SetDuration -- Bar bind ONCE), border on/off, frame level. Cosmetic bar styling is barCoSig. -local function barStructSig(map, indicator, borderOn, defs, mine) - return includeSig(map) - .. "|bar" +-- Create-only properties ONLY; the identity map is live-tunable and rides +-- placedTuningSig (see placedStructSig for why a needless Rebuild is a leak). +local function barStructSig(indicator, borderOn, defs, mine) + return "bar" .. "|df=" .. durationFmtKey(indicator, false, defs.cbt) -- (No alert keys: the expiry alert lives on the COMPANION slot, whose own -- structSig carries alertElemStructKey — an alert edit rebuilds only it.) @@ -1931,7 +2006,7 @@ function Factory:BuildGroupPreviewConfig(frame, group) -- filter is inert here (also for debuff groups): the editor always -- supplies its own testEntries, so _paintTestSlot's category-pool -- fallback — the only preview reader of this string — never runs. - mode = "row", max = 1, filter = "HELPFUL", + mode = "row", max = 1, singleSlot = true, filter = "HELPFUL", adBorderAnim = borderSpec and true or nil, layout = { size = math.max(8, tonumber(group.iconSize) or 24) }, style = buildFilterGroupStyle(group, borderSpec), @@ -2731,7 +2806,8 @@ local function syncPlacedPool(frame, placed, live, hasMG, auras, keyPrefix, idSp local eff = memberEffective(hasMG, key, indicator) local borderOn = placedBorderOn(indicator, false) local alpha = tonumber(indicator.alpha) or 1 - local structSig = barStructSig(map, indicator, borderOn, defs, mine) + local structSig = barStructSig(indicator, borderOn, defs, mine) + local tuningSig = placedTuningSig(map) local coSig = barCoSig(frame, eff, borderOn, alpha) local entry = placed[key] @@ -2741,20 +2817,32 @@ local function syncPlacedPool(frame, placed, live, hasMG, auras, keyPrefix, idSp buildBarConfig(frame, frame.unit, map, eff, borderSpec, defs, mine)) if handle then applyPlacedAlpha(handle, alpha) - placed[key] = { handle = handle, structSig = structSig, coSig = coSig } + placed[key] = { handle = handle, structSig = structSig, + tuningSig = tuningSig, coSig = coSig } end elseif entry.structSig ~= structSig then local borderSpec = borderOn and buildBarBorderSpec(frame, indicator) or nil - entry.structSig, entry.coSig = structSig, coSig + entry.structSig, entry.tuningSig, entry.coSig = structSig, tuningSig, coSig entry.handle:Rebuild(buildBarConfig(frame, frame.unit, map, eff, borderSpec, defs, mine)) applyPlacedAlpha(entry.handle, alpha) - elseif entry.coSig ~= coSig then - local borderSpec = borderOn and buildBarBorderSpec(frame, indicator) or nil - entry.coSig = coSig - entry.handle:ApplyStyle( - buildBarStyle(indicator, borderSpec, defs), - buildBarLayout(frame, eff)) - applyPlacedAlpha(entry.handle, alpha) + else + if entry.tuningSig ~= tuningSig then + -- Selection edit, struct sig stable: swap the include map on + -- the live container (row mode) rather than recreating it. + -- borderSpec nil on purpose — ApplyTuning reads only the trio. + entry.tuningSig = tuningSig + local cfg = buildBarConfig(frame, frame.unit, map, eff, nil, defs, mine) + entry.handle.config.testEntries = cfg.testEntries + entry.handle:ApplyTuning(cfg) + end + if entry.coSig ~= coSig then + local borderSpec = borderOn and buildBarBorderSpec(frame, indicator) or nil + entry.coSig = coSig + entry.handle:ApplyStyle( + buildBarStyle(indicator, borderSpec, defs), + buildBarLayout(frame, eff)) + applyPlacedAlpha(entry.handle, alpha) + end end -- Expiry-alert companion slot (own container, own sigs — @@ -2860,8 +2948,9 @@ local function syncPlacedPool(frame, placed, live, hasMG, auras, keyPrefix, idSp -- Sigs are computed from RAW config every tick (no BuildSpec -- alloc — FIX C); the actual border spec is built ONLY inside a -- create/rebuild/restyle branch below, never per pass. - local structSig = placedStructSig(map, isSquare, hideIcon, showStacks, + local structSig = placedStructSig(isSquare, hideIcon, showStacks, showDuration, borderOn, indicator, defs, mine) + local tuningSig = placedTuningSig(map) local coSig = placedCoSig(eff, isSquare, borderOn, alpha) local entry = placed[key] @@ -2871,20 +2960,41 @@ local function syncPlacedPool(frame, placed, live, hasMG, auras, keyPrefix, idSp buildPlacedConfig(frame, frame.unit, map, eff, isSquare, borderSpec, defs, mine)) if handle then applyPlacedAlpha(handle, alpha) - placed[key] = { handle = handle, structSig = structSig, coSig = coSig } + placed[key] = { handle = handle, structSig = structSig, + tuningSig = tuningSig, coSig = coSig } end elseif entry.structSig ~= structSig then local borderSpec = borderOn and buildPlacedBorderSpec(frame, indicator, hideIcon) or nil - entry.structSig, entry.coSig = structSig, coSig + entry.structSig, entry.tuningSig, entry.coSig = structSig, tuningSig, coSig entry.handle:Rebuild(buildPlacedConfig(frame, frame.unit, map, eff, isSquare, borderSpec, defs, mine)) applyPlacedAlpha(entry.handle, alpha) - elseif entry.coSig ~= coSig then - local borderSpec = borderOn and buildPlacedBorderSpec(frame, indicator, hideIcon) or nil - entry.coSig = coSig - entry.handle:ApplyStyle( - buildPlacedStyle(indicator, isSquare, borderSpec, defs), - buildPlacedLayout(eff)) - applyPlacedAlpha(entry.handle, alpha) + else + if entry.tuningSig ~= tuningSig then + -- Selection edit with the struct sig stable: swap the include + -- map on the LIVE container instead of recreating it. + -- ApplyTuning replaces the trio wholesale (max/sort/ + -- candidateFilters) off the fresh config and self-defers in + -- combat. testEntries rides along so a test-mode rebuild + -- previews the NEW selection, not a stale one — same pairing + -- as the filter-group path. borderSpec is nil here on purpose: + -- ApplyTuning reads only the trio, and the cosmetic branch + -- below owns the style (building a spec here would be thrown + -- away). + entry.tuningSig = tuningSig + local cfg = buildPlacedConfig(frame, frame.unit, map, eff, isSquare, nil, defs, mine) + entry.handle.config.testEntries = cfg.testEntries + entry.handle:ApplyTuning(cfg) + end + if entry.coSig ~= coSig then + local borderSpec = borderOn and buildPlacedBorderSpec(frame, indicator, hideIcon) or nil + entry.coSig = coSig + entry.handle:ApplyStyle( + buildPlacedStyle(indicator, isSquare, borderSpec, defs), + buildPlacedLayout(eff)) + -- alpha is part of coSig, so it re-applies here and NOT on the + -- steady-state path — this block runs per indicator per tick. + applyPlacedAlpha(entry.handle, alpha) + end end -- Expiry-alert companion slot (own container, own sigs — @@ -3117,7 +3227,12 @@ function Factory:SyncFrame(frame) -- replace mode always uses the fill-matched mirror. local wholeBar = (mode == "tint") and (bestCfg.tintWholeBar and true or false) or false - local structSig = includeSig(bestMap) .. "|" .. (wholeBar and "flat" or "mirror") .. "|" .. filt + -- The tracked map is live-tunable (overlay slots take + -- SetAuraSlotCandidateFilters), so it rides its own sig rather than forcing + -- a teardown+recreate. wholeBar STAYS structural: it picks a different + -- config builder entirely. + local structSig = (wholeBar and "flat" or "mirror") .. "|" .. filt + local tuningSig = placedTuningSig(bestMap) local entry = hb[bestName] if wholeBar then @@ -3128,15 +3243,22 @@ function Factory:SyncFrame(frame) frame.dfADHealthMirror = nil local handle = DF.AuraContainer:Create(healthBar, buildOverlayTintConfig(frame.unit, bestMap, r, g, b, blend, 1, filt)) if handle then - hb[bestName] = { handle = handle, structSig = structSig, coSig = coSig } + hb[bestName] = { handle = handle, structSig = structSig, + tuningSig = tuningSig, coSig = coSig } end elseif entry.structSig ~= structSig then frame.dfADHealthMirror = nil - entry.structSig, entry.coSig = structSig, coSig + entry.structSig, entry.tuningSig, entry.coSig = structSig, tuningSig, coSig entry.handle:Rebuild(buildOverlayTintConfig(frame.unit, bestMap, r, g, b, blend, 1, filt)) - elseif entry.coSig ~= coSig then - entry.coSig = coSig - entry.handle:ApplyStyle({ overlay = { tintColor = { r, g, b, blend } } }) + else + if entry.tuningSig ~= tuningSig then + entry.tuningSig = tuningSig + entry.handle:ApplyTuning(buildOverlayTintConfig(frame.unit, bestMap, r, g, b, blend, 1, filt)) + end + if entry.coSig ~= coSig then + entry.coSig = coSig + entry.handle:ApplyStyle({ overlay = { tintColor = { r, g, b, blend } } }) + end end else -- FILLED MIRROR PATH — duplicate StatusBar fed the secret health percent. @@ -3149,15 +3271,26 @@ function Factory:SyncFrame(frame) frame.dfADHealthMirror = nil -- onBar re-stashes when the slot builds local handle = DF.AuraContainer:Create(healthBar, buildHealthMirrorConfig(frame.unit, bestMap, r, g, b, alpha, tex, onBar, filt)) if handle then - hb[bestName] = { handle = handle, structSig = structSig, coSig = coSig } + hb[bestName] = { handle = handle, structSig = structSig, + tuningSig = tuningSig, coSig = coSig } end elseif entry.structSig ~= structSig then frame.dfADHealthMirror = nil -- old slot torn down; onBar re-stashes - entry.structSig, entry.coSig = structSig, coSig + entry.structSig, entry.tuningSig, entry.coSig = structSig, tuningSig, coSig entry.handle:Rebuild(buildHealthMirrorConfig(frame.unit, bestMap, r, g, b, alpha, tex, onBar, filt)) - elseif entry.coSig ~= coSig then - entry.coSig = coSig - entry.handle:ApplyStyle({ overlay = { healthMirror = { texture = tex, color = { r, g, b }, alpha = alpha, onBar = onBar } } }) + else + if entry.tuningSig ~= tuningSig then + -- ☠ Do NOT clear frame.dfADHealthMirror here. The two branches + -- above clear it because the slot is torn down and onBar re-stashes + -- the new StatusBar; a tuning pass keeps the SAME slot and the same + -- bar, so clearing the ref would strand it (nothing re-stashes). + entry.tuningSig = tuningSig + entry.handle:ApplyTuning(buildHealthMirrorConfig(frame.unit, bestMap, r, g, b, alpha, tex, onBar, filt)) + end + if entry.coSig ~= coSig then + entry.coSig = coSig + entry.handle:ApplyStyle({ overlay = { healthMirror = { texture = tex, color = { r, g, b }, alpha = alpha, onBar = onBar } } }) + end end end end @@ -3219,21 +3352,29 @@ function Factory:SyncFrame(frame) local mode = slower(bestCfg.mode or "tint") -- background defaults to tint local blend = healthbarBlend(mode, bestCfg.blend, a) - local structSig = includeSig(bestMap) .. "|" .. filt + local structSig = filt + local tuningSig = placedTuningSig(bestMap) local coSig = tconcat({ tostring(r), tostring(g), tostring(b), tostring(blend) }, "|") local entry = bg[bestName] if not entry then local handle = DF.AuraContainer:Create(bgAnchor, buildOverlayTintConfig(frame.unit, bestMap, r, g, b, blend, 0, filt)) if handle then - bg[bestName] = { handle = handle, structSig = structSig, coSig = coSig } + bg[bestName] = { handle = handle, structSig = structSig, + tuningSig = tuningSig, coSig = coSig } end elseif entry.structSig ~= structSig then - entry.structSig, entry.coSig = structSig, coSig + entry.structSig, entry.tuningSig, entry.coSig = structSig, tuningSig, coSig entry.handle:Rebuild(buildOverlayTintConfig(frame.unit, bestMap, r, g, b, blend, 0, filt)) - elseif entry.coSig ~= coSig then - entry.coSig = coSig - entry.handle:ApplyStyle({ overlay = { tintColor = { r, g, b, blend } } }) + else + if entry.tuningSig ~= tuningSig then + entry.tuningSig = tuningSig + entry.handle:ApplyTuning(buildOverlayTintConfig(frame.unit, bestMap, r, g, b, blend, 0, filt)) + end + if entry.coSig ~= coSig then + entry.coSig = coSig + entry.handle:ApplyStyle({ overlay = { tintColor = { r, g, b, blend } } }) + end end end end @@ -3290,21 +3431,29 @@ function Factory:SyncFrame(frame) -- drawAboveFrameBorder rides the STRUCT sig: it resolves to frameLevelOffset in -- buildBorderConfig, which only a Rebuild re-reads (ApplyStyle carries the spec only). local drawAbove = bestCfg.drawAboveFrameBorder ~= false - local structSig = includeSig(bestMap) .. "|" .. filt .. "|da=" .. tostring(drawAbove) + local structSig = filt .. "|da=" .. tostring(drawAbove) + local tuningSig = placedTuningSig(bestMap) local coSig = borderSpecSig(bestSpec) local entry = bd[bestName] if not entry then local handle = DF.AuraContainer:Create(frame, buildBorderConfig(frame.unit, bestMap, bestSpec, filt, drawAbove)) if handle then - bd[bestName] = { handle = handle, structSig = structSig, coSig = coSig } + bd[bestName] = { handle = handle, structSig = structSig, + tuningSig = tuningSig, coSig = coSig } end elseif entry.structSig ~= structSig then - entry.structSig, entry.coSig = structSig, coSig + entry.structSig, entry.tuningSig, entry.coSig = structSig, tuningSig, coSig entry.handle:Rebuild(buildBorderConfig(frame.unit, bestMap, bestSpec, filt, drawAbove)) - elseif entry.coSig ~= coSig then - entry.coSig = coSig - entry.handle:ApplyStyle({ border = { spec = bestSpec } }) + else + if entry.tuningSig ~= tuningSig then + entry.tuningSig = tuningSig + entry.handle:ApplyTuning(buildBorderConfig(frame.unit, bestMap, bestSpec, filt, drawAbove)) + end + if entry.coSig ~= coSig then + entry.coSig = coSig + entry.handle:ApplyStyle({ border = { spec = bestSpec } }) + end end end end @@ -3332,7 +3481,8 @@ function Factory:SyncFrame(frame) local filt = poolFilter(bestCfg, bestPool == 1) local r, g, b, a = readADColor(bestCfg.color) local color = { r = r, g = g, b = b, a = a } - local structSig = includeSig(bestMap) .. "|" .. filt + local structSig = filt + local tuningSig = placedTuningSig(bestMap) local coSig = colSig(bestCfg.color) -- onHost fires on every style pass (create/ApplyStyle/Blizzard re-init): -- stash the host for the TD-teardown recovery below and (re)register the @@ -3349,11 +3499,22 @@ function Factory:SyncFrame(frame) buildMirrorHostConfig(frame.unit, bestMap, onHost, filt)) if handle then st[bestName] = { handle = handle, structSig = structSig, - coSig = coSig, host = st._lastHost } + tuningSig = tuningSig, coSig = coSig, + host = st._lastHost } end elseif entry.structSig ~= structSig then - entry.structSig, entry.coSig = structSig, coSig + entry.structSig, entry.tuningSig, entry.coSig = structSig, tuningSig, coSig entry.handle:Rebuild(buildMirrorHostConfig(frame.unit, bestMap, onHost, filt)) + elseif entry.tuningSig ~= tuningSig then + -- Selection edit only: swap the include map on the live slot. Kept as a + -- branch of this elseif chain (rather than folded into the else) so the + -- one-action-per-pass shape the coSig and host-recovery branches below + -- already rely on is preserved — the sync runs every tick, so a second + -- pending change lands on the next one. + -- entry.host is deliberately untouched: the slot survives a tuning pass, + -- so onHost does not re-fire and the stashed host stays valid. + entry.tuningSig = tuningSig + entry.handle:ApplyTuning(buildMirrorHostConfig(frame.unit, bestMap, onHost, filt)) elseif entry.coSig ~= coSig then entry.coSig = coSig entry.handle:ApplyStyle({ overlay = { mirrorHost = { onHost = onHost } } }) diff --git a/DandersFrames/Core.lua b/DandersFrames/Core.lua index f486bec2..87e3ca74 100644 --- a/DandersFrames/Core.lua +++ b/DandersFrames/Core.lua @@ -227,6 +227,26 @@ DF.DebugSubCommands = {} -- This lookup is what the dispatcher gate consults so the two halves now match. DF.DEBUG_SUB_DEV = {} +-- ☠ EVERY sub() NAME, dev or not. This is what stops "/df debug " loading +-- the settings addon for a command that lives entirely in this one. +-- +-- The dispatcher's unknown-word fallback exists because a handful of debug tools +-- (icons, colorhook, atlas, auraexp, memtest) register their slashes only when +-- the companion loads, so an unrecognised word has to load it and retry. But +-- "recognised" was read from DebugSlashBySub, which ONLY RegisterDebugSlash +-- fills -- so all ~38 commands registered through sub() looked unknown and pulled +-- in ~3 MB of settings UI before running a branch that never needed it. That is +-- the whole saving of splitting the addon in two, spent on one diagnostic. +-- +-- Worse for the probes that exist to MEASURE memory: loading the companion +-- perturbs exactly what they report. +-- +-- Safe against shadowing: no sub() name collides with a companion-registered +-- slash, so this set can never swallow a word that genuinely needs the load. The +-- sub() branches that DO need the companion (profiler, ...) call +-- EnsureOptionsLoaded themselves, at the point of need. +DF.DEBUG_SUB_KNOWN = {} + -- ============================================================ -- CHAT OUTPUT HOUSE STYLE (DF:Out) -- ============================================================ @@ -526,6 +546,9 @@ function DF:RegisterDebugSub(cmd, desc, devOnly, args, hidden) -- command cannot be marked dev in the listing while staying runnable on -- release — the two can no longer drift apart. if devOnly then DF.DEBUG_SUB_DEV[cmd] = true end + -- Registering IS the gate here too: a branch added later is covered on the + -- day it is written, with no second list to keep in step. + DF.DEBUG_SUB_KNOWN[cmd] = true end -- ============================================================ @@ -1219,8 +1242,13 @@ function DF:LightweightUpdateHighlight(highlightType) end if highlight and highlight:IsShown() then + -- ☠ This function writes the four line textures DIRECTLY, bypassing + -- ApplyHighlightStyle, so its cached style is stale the moment we touch + -- them. Drop it or the next full update sees a matching signature and + -- skips, leaving these drag-time values in place permanently. + if DF.InvalidateHighlightStyle then DF:InvalidateHighlightStyle(highlight) end highlight:SetAlpha(alpha) - + -- Update border textures - check both naming conventions local top = highlight.top or highlight.topLine local bottom = highlight.bottom or highlight.bottomLine @@ -1647,6 +1675,36 @@ function DF:GetClassColor(class) return RAID_CLASS_COLORS[class] or DEFAULT_CLASS_COLOR end +-- ============================================================ +-- UNIT ROLE RESOLUTION +-- ============================================================ +-- ☠ NEVER CALL UnitGroupRolesAssigned DIRECTLY FOR A GATE. Use this. +-- +-- UnitGroupRolesAssigned answers "what role did the GROUP assign", not "what +-- does this player do". It returns "NONE" solo, in the open world, in open-world +-- groups, and in delves until something forces an assignment -- so a Holy +-- Paladin standing in Silvermoon reads as NONE, and any caller that maps NONE +-- onto DAMAGER decides a healer is a DPS. That is what made the resource bar's +-- Healers toggle inert while solo (the bar answered to the DPS toggle instead), +-- and why a delve only resolved the role after a spec change. +-- +-- The player is the one unit we can do better for: GetSpecializationRole is +-- authoritative and always available. Other units expose no public spec API, so +-- they stay NONE and the caller keeps whatever fallback it had. +-- +-- Returns nil for a unit that does not exist, otherwise a role token that may +-- still be "NONE" -- resolution only, no policy. Callers decide what NONE means. +function DF:GetUnitRole(unit) + if not unit or not UnitExists(unit) then return nil end + local role = UnitGroupRolesAssigned and UnitGroupRolesAssigned(unit) + if (not role or role == "NONE") and UnitIsUnit and UnitIsUnit(unit, "player") + and GetSpecialization and GetSpecializationRole then + local spec = GetSpecialization() + if spec then role = GetSpecializationRole(spec) or role end + end + return role +end + -- Resolve the frame border colour: the static borderColor by default, or -- (Stage 2.1+) the unit's class / role colour with its own alpha slider when -- the canonical frameBorderColorSource picks one. Non-player / unknown-class @@ -1701,17 +1759,10 @@ function DF:GetFrameBorderColor(frame, db) if frame.dfIsTestFrame then local testData = DF.GetTestUnitData and DF:GetTestUnitData(frame.index, frame.isRaidFrame) role = testData and testData.role - elseif frame.unit and UnitExists(frame.unit) and UnitGroupRolesAssigned then - role = UnitGroupRolesAssigned(frame.unit) - -- UnitGroupRolesAssigned returns "NONE" outside instances where - -- roles aren't assigned (solo, world content). For the player, - -- fall back to spec role so role colour stays meaningful. Other - -- units expose no public spec API; they stay on picker fallback. - if (not role or role == "NONE") and UnitIsUnit and UnitIsUnit(frame.unit, "player") - and GetSpecialization and GetSpecializationRole then - local spec = GetSpecialization() - if spec then role = GetSpecializationRole(spec) end - end + else + -- Player falls back to the spec role when the group assigned none; + -- other units stay NONE and drop to the picker fallback below. + role = DF:GetUnitRole(frame.unit) end local c = rc and role and role ~= "NONE" and (rc[role] or rc[string.lower(role)]) if c then @@ -2535,6 +2586,34 @@ function DF:DebugAuraFilters(unit) o:Field("AuraUtil.ShouldDisplayBuff", hasSDB and "present" or "absent", hasSDB and "good" or "warn") o:Field("AuraUtil.ForEachAura", hasFEA and "present" or "absent", hasFEA and "good" or "warn") + -- Which Edit Mode strategy is actually live. Deafening the container to + -- AURA_DATA_PROVIDER_SWITCH can only be PROVEN at runtime (it is a base widget + -- method on a forbidden-table object), so this reports the client's answer + -- rather than an assumption — see EDIT-MODE DEAFENING in Frames/AuraContainer.lua. + o:Section("Edit Mode isolation") + local ac = DF.AuraContainer + local deafOK = ac and ac._providerDeafOK + if not ac or deafOK == nil then + o:Field("strategy", "not probed yet (no container built)", "neutral") + elseif deafOK then + o:Field("strategy", "deafened container — rows keep live auras", "good") + else + o:Field("strategy", "rebirth fallback — rebuild on switch (OOC), hide in combat", "warn") + end + if ac and ac._providerDeafWhy then + o:Field("unregister probe", tostring(ac._providerDeafWhy), deafOK and "good" or "neutral") + end + -- Populated the first time Edit Mode is opened. QUEUED means the client deferred + -- our re-entrant switch to after the in-flight dispatch (safe, and the inline reset + -- gains nothing); NESTED means it dispatched re-entrantly (zero blip, but containers + -- the outer dispatch had not reached can strand on the sample source). + if ac and ac._inlineDispatch then + o:Field("inline reset dispatch", ac._inlineDispatch, + ac._inlineDispatch == "QUEUED" and "good" or "warn") + elseif ac then + o:Field("inline reset dispatch", "not observed yet (open Edit Mode once)", "neutral") + end + o:Section("ShouldDisplayBuff per aura") if AuraUtil and AuraUtil.ForEachAura then local rows = {} @@ -3610,6 +3689,37 @@ DF._MainEventDispatcher = function(self, event, arg1) DandersFramesCharDB.currentProfile = currentProfile DandersFramesDB_v2.currentProfile = currentProfile + -- Settings-window geometry moves from db.party to account-wide + -- windowState (see DF:GetWindowState for why). Seed once from whichever + -- profile is active at this login -- that is the window the user last + -- sized and scaled, so it is the only correct source. + if not DandersFramesDB_v2.windowState then + local ws = {} + local src = DandersFramesDB_v2.profiles[currentProfile] + src = src and src.party + if type(src) == "table" then + ws.scale, ws.width, ws.height = src.guiScale, src.guiWidth, src.guiHeight + ws.point, ws.relPoint, ws.x, ws.y = src.guiPoint, src.guiRelPoint, src.guiX, src.guiY + end + DandersFramesDB_v2.windowState = ws + end + -- Clean up the legacy per-profile keys (no longer read anywhere). Same + -- shape as the languageOverride strip above: unconditional, so profiles + -- that were not the seed source are cleared too. Both modes -- only + -- db.party was ever read, but RaidDefaults is copied from PartyDefaults + -- so every profile carries a dead db.raid set as well. + for _, profile in pairs(DandersFramesDB_v2.profiles) do + if type(profile) == "table" then + for _, modeKey in ipairs({ "party", "raid" }) do + local m = profile[modeKey] + if type(m) == "table" then + m.guiScale, m.guiWidth, m.guiHeight = nil, nil, nil + m.guiPoint, m.guiRelPoint, m.guiX, m.guiY = nil, nil, nil, nil + end + end + end + end + DF.db = DandersFramesDB_v2.profiles[currentProfile] -- Ensure both modes exist in current profile @@ -4941,15 +5051,23 @@ DF._MainEventDispatcher = function(self, event, arg1) local dbgWord, dbgRest = rawMsg:match("^%s*[Dd][Ee][Bb][Uu][Gg]%s+(%S+)%s*(.-)%s*$") -- "on"/"off" are the logging toggle, not commands named on/off. if dbgWord and dbgWord:lower() ~= "on" and dbgWord:lower() ~= "off" then - local dbgKey = DF.DebugSlashBySub[dbgWord:lower()] + local dbgLower = dbgWord:lower() + local dbgKey = DF.DebugSlashBySub[dbgLower] -- Several debug tools live in the companion and register their -- slashes only when it loads. If the word is unknown and the -- companion is not in yet, load it and retry once -- otherwise -- the first use of /df debug memtest fell through to the final -- else and opened the settings window instead of the tool. - if not dbgKey and not DF._optionsAddonLoaded + -- + -- ☠ DEBUG_SUB_KNOWN FIRST. A sub()-registered command is a branch + -- in THIS addon; it is recognised, it just is not in the slash + -- registry. Without this test every one of them read as unknown + -- and loaded the companion for nothing -- see the note at the + -- DEBUG_SUB_KNOWN declaration. + if not dbgKey and not DF.DEBUG_SUB_KNOWN[dbgLower] + and not DF._optionsAddonLoaded and DF.EnsureOptionsLoaded and DF:EnsureOptionsLoaded() then - dbgKey = DF.DebugSlashBySub[dbgWord:lower()] + dbgKey = DF.DebugSlashBySub[dbgLower] end if dbgKey and SlashCmdList[dbgKey] then SlashCmdList[dbgKey](dbgRest or "") @@ -5144,15 +5262,14 @@ DF._MainEventDispatcher = function(self, event, arg1) DF:ResetFullProfile() elseif msg == "resetgui" then -- Reset GUI scale, size, and position to defaults - if DF.db and DF.db.party then - DF.db.party.guiScale = 1.0 - DF.db.party.guiWidth = 760 - DF.db.party.guiHeight = 520 - DF.db.party.guiPoint = nil - DF.db.party.guiRelPoint = nil - DF.db.party.guiX = nil - DF.db.party.guiY = nil - end + local ws = DF:GetWindowState() + ws.scale = 1.0 + ws.width = 760 + ws.height = 520 + ws.point = nil + ws.relPoint = nil + ws.x = nil + ws.y = nil if DF.GUIFrame then DF.GUIFrame:ClearAllPoints() DF.GUIFrame:SetPoint("CENTER", UIParent, "CENTER", 0, 0) diff --git a/DandersFrames/Core/Config.lua b/DandersFrames/Core/Config.lua index b4d0c407..402473cc 100644 --- a/DandersFrames/Core/Config.lua +++ b/DandersFrames/Core/Config.lua @@ -219,6 +219,11 @@ end -- Cache for created font families local fontFamilies = {} +-- Raw outline string -> { shadow, flags, slug }. See the ☠ note in DF:SafeSetFont: +-- the parse allocates a capture string per call and that call is per FontString per +-- frame. Pure function of the key, so entries never go stale. +local outlineParseMemo = {} + -- Clear font cache (kept for compatibility) function DF:ClearFontCache() -- Clear font families when new fonts are registered @@ -302,13 +307,23 @@ local fontValidationFrame = CreateFrame("Frame") fontValidationFrame:Hide() local fontValidationString = fontValidationFrame:CreateFontString(nil, "OVERLAY") --- Preload/validate a font to ensure WoW has it loaded +-- Preload/validate a font to ensure WoW has it loaded. +-- +-- Memoised per path. This runs on EVERY DF:SafeSetFont call, and SafeSetFont is one of +-- the hottest functions in the addon (5.7% of a boss trace's allocation just here), but +-- a path only ever needs loading once -- the file does not unload and the mapping from +-- path to file never changes. Keyed on ATTEMPTED rather than succeeded, which matches +-- the old behaviour exactly: the pcall result was discarded, so a failed load was never +-- retried anyway. +-- +-- pcall(fn, args...) not pcall(function() ... end): the closure form allocated one +-- closure per call on that same hot path. +local preloadedFonts = {} local function PreloadFont(fontPath) - if not fontPath then return end + if not fontPath or preloadedFonts[fontPath] then return end + preloadedFonts[fontPath] = true -- Attempt to set the font - this forces WoW to load the font file - pcall(function() - fontValidationString:SetFont(fontPath, 12, "") - end) + pcall(fontValidationString.SetFont, fontValidationString, fontPath, 12, "") end -- Build font family members for CreateFontFamily @@ -375,8 +390,30 @@ end -- SafeSetFont's broken-family eviction must reproduce it exactly (a mismatched evict -- key would strand the broken entry). Shadow stays LAST so RefreshFontFamilyShadows' -- "|shadow" suffix check still identifies shadowed families. +-- ☠ The key STRING is memoised, not just the family it looks up. +-- GetOrCreateFontFamily calls this on every SafeSetFont to probe its cache, so +-- the four concats ran on every cache HIT too -- once per FontString per frame. +-- The input set is tiny (a handful of fonts x a few outlines x quantized sizes), +-- so a nested table costs three small tables per NEW combination and nothing +-- ever again, while the returned key stays a plain string: fontFamilies is still +-- string-keyed and the eviction in SafeSetFont still finds its entry. +local fontKeyMemo = {} local function FontFamilyKey(fontPath, outline, useShadow, quantizedSize) - return (fontPath or "default"):lower() .. "|" .. (outline or "") .. "|" .. tostring(quantizedSize) .. "|" .. (useShadow and "shadow" or "noshadow") + local p = fontPath or "default" + local byOutline = fontKeyMemo[p] + if not byOutline then byOutline = {}; fontKeyMemo[p] = byOutline end + local o = outline or "" + local bySize = byOutline[o] + if not bySize then bySize = {}; byOutline[o] = bySize end + local byShadow = bySize[quantizedSize] + if not byShadow then byShadow = {}; bySize[quantizedSize] = byShadow end + local sh = useShadow and true or false + local key = byShadow[sh] + if not key then + key = p:lower() .. "|" .. o .. "|" .. tostring(quantizedSize) .. "|" .. (sh and "shadow" or "noshadow") + byShadow[sh] = key + end + return key end local fontFamilyCounter = 0 local function GetOrCreateFontFamily(fontPath, outline, useShadow, size) @@ -679,6 +716,14 @@ function DF:ComposeOutline(flag, shadow) return flag end +-- Hoisted out of DF:SafeSetFont so the pcall below takes the allocation-free +-- pcall(fn, args...) form. It is two statements (the GameFontNormal set first is +-- deliberate — see the call site), so it cannot be inlined as a bare method reference. +local function setFontObjectPair(fontString, familyObject) + fontString:SetFontObject(GameFontNormal) + fontString:SetFontObject(familyObject) +end + function DF:SafeSetFont(fontString, fontNameOrPath, fontSize, outline) if not fontString then return false end @@ -696,18 +741,35 @@ function DF:SafeSetFont(fontString, fontNameOrPath, fontSize, outline) -- shadow combined with any flag, e.g. "SHADOW;MONOCHROME, OUTLINE"). The legacy -- value "SHADOW" on its own means shadow with no outline. Shadow is rendered via -- SetShadow* below, never as a font flag, so strip it out of the flag string. - local useShadow = false - local rest = outline:match("^SHADOW;(.*)$") - if rest then - useShadow = true - outline = rest - elseif outline == "SHADOW" then - useShadow = true - outline = "" + -- + -- ☠ MEMOISED because string.match WITH A CAPTURE ALLOCATES the captured + -- substring on every call -- and this runs once per FontString per frame, twice + -- over for any element carrying an AD mirror. It was 40.9% of all boss-fight + -- allocation. The parse is a pure function of the raw outline string and the + -- stored values come from a handful of dropdown options, so the memo fills once + -- and never needs invalidating: a given string always parses the same way. + local parsed = outlineParseMemo[outline] + if not parsed then + local shadow, flags = false, outline + local rest = outline:match("^SHADOW;(.*)$") + if rest then + shadow, flags = true, rest + elseif outline == "SHADOW" then + shadow, flags = true, "" + end + -- Normalize "NONE" to empty string (NONE is not a valid WoW font flag) + if flags == "NONE" then flags = "" end + -- The SLUG variant is derived from `flags` alone, so it caches with it -- + -- the concat below used to run per call for every slug-eligible element. + parsed = { + shadow = shadow, + flags = flags, + slug = (flags == "") and "SLUG" or (flags .. ", SLUG"), + } + outlineParseMemo[outline] = parsed end - - -- Normalize "NONE" to empty string (NONE is not a valid WoW font flag) - if outline == "NONE" then outline = "" end + local useShadow = parsed.shadow + outline = parsed.flags local actualOutline = outline @@ -720,7 +782,9 @@ function DF:SafeSetFont(fontString, fontNameOrPath, fontSize, outline) and not useShadow and (actualOutline == "" or actualOutline == "OUTLINE") if useSlug then - actualOutline = (actualOutline == "") and "SLUG" or (actualOutline .. ", SLUG") + -- Cached alongside the parse above; fontSlug only selects between the two + -- precomputed variants, so toggling it needs no invalidation. + actualOutline = parsed.slug end if fontString.SetScaleAnimationMode and FontStringScaleAnimationMode then fontString:SetScaleAnimationMode(useSlug and FontStringScaleAnimationMode.Vertex or FontStringScaleAnimationMode.FontSize) @@ -754,10 +818,7 @@ function DF:SafeSetFont(fontString, fontNameOrPath, fontSize, outline) -- data may not be initialized yet, causing an ACCESS_VIOLATION crash when -- SetFontObject tries to read it. Wrap in pcall to fall through to the -- direct SetFont() path if the object is broken. - local ok = pcall(function() - fontString:SetFontObject(GameFontNormal) - fontString:SetFontObject(_G[fontFamilyName]) - end) + local ok = pcall(setFontObjectPair, fontString, _G[fontFamilyName]) if not ok then -- Evict the broken cache entry so it gets recreated later. Shared key -- builder + same size quantization as GetOrCreateFontFamily — a @@ -1150,8 +1211,10 @@ DF.PartyDefaults = { -- groups are declared first, so they already lead the row. These keys style them. -- Membership of the group IS the "is this important" test — nothing reads aura data, -- which is what makes this expressible at all under the 12.1 secret rules. - -- OFF by default: it changes the look of a row every user already has. - debuffImportantHighlight = false, -- master toggle for the treatment below + -- ON by default from v5: the aura row is rebuilt on this lane anyway, so there is no + -- established look to preserve, and a boss/priority debuff standing out is the + -- behaviour most users would pick. Off is one tick away on the Debuffs page. + debuffImportantHighlight = true, -- master toggle for the treatment below debuffImportantScale = 1.25, -- icon size step for important debuffs (1 = same as the rest) debuffImportantBadge = true, -- corner "!" badge debuffImportantBadgeSize = 10, -- badge diameter in px (centred inside the corner, inset by size/4) @@ -1445,10 +1508,11 @@ DF.PartyDefaults = { groupLabelOutline = "SHADOW", groupLabelPosition = "START", - -- GUI State - guiHeight = 693.33349609375, - guiScale = 1, - guiWidth = 816.6666259765625, + -- (Removed) GUI State: guiHeight / guiScale / guiWidth. Settings-window + -- geometry is account-wide machine state now -- DandersFramesDB_v2.windowState, + -- reached via DF:GetWindowState. As per-profile DEFAULTS they actively caused + -- a bug: a new profile is born from this table, so creating one silently + -- reset the window's scale and size while the open window kept the old ones. -- Heal Absorb Bar healAbsorbBarAnchor = "BOTTOM", diff --git a/DandersFrames/Core/Profile.lua b/DandersFrames/Core/Profile.lua index 4f94210b..0601e487 100644 --- a/DandersFrames/Core/Profile.lua +++ b/DandersFrames/Core/Profile.lua @@ -21,6 +21,32 @@ function DF:DeepCopy(src) return dest end +-- ============================================================ +-- SETTINGS-WINDOW STATE (ACCOUNT-WIDE, NOT PROFILE CONTENT) +-- ============================================================ +-- Scale, size and position of the settings window are machine state, not +-- settings -- ExportCategories has declared them local-only for as long as +-- exports have existed. They used to be STORED in db.party anyway, which meant +-- a profile carried them: a freshly created profile is born from PartyDefaults, +-- so its guiScale was 1 while the already-open window kept the old scale. The +-- Test Mode and Unlock windows re-read the db on every OnShow, so they snapped +-- to 100% and only the UI Scale slider (the single writer) could put them back +-- in step. Holding the state account-wide is what actually enforces the +-- declared intent -- switching or creating a profile can no longer move, +-- resize or rescale the window out from under the user. +-- +-- Fields: scale, width, height, point, relPoint, x, y. All optional; every +-- reader supplies its own fallback. +function DF:GetWindowState() + if not DandersFramesDB_v2 then DandersFramesDB_v2 = {} end + local ws = DandersFramesDB_v2.windowState + if not ws then + ws = {} + DandersFramesDB_v2.windowState = ws + end + return ws +end + -- ============================================================ -- PROFILE MANAGEMENT -- ============================================================ diff --git a/DandersFrames/Features/Auras.lua b/DandersFrames/Features/Auras.lua index 81fc15d3..f537be43 100644 --- a/DandersFrames/Features/Auras.lua +++ b/DandersFrames/Features/Auras.lua @@ -254,14 +254,18 @@ local function BuildDirectDebuffFilters(db, claimed) -- Negation suffix for a group, given which higher-priority token filters -- apply to it. ALL-mode dispel dedups via excludeDispelTypes (see cfFor). - local function neg(excludeDispel, excludeCC, excludeRaid) + -- Token precedence among the NON-important records is dispel > CC > raid, so only + -- those two exclusions are ever needed. There is deliberately no raid exclusion: + -- raid is the last token record, and the important records above no longer negate + -- anything (they subtract via candidateFilters instead — see IMPORTANT-FIRST + -- PRECEDENCE below), so nothing is left that would need to exclude it. + local function neg(excludeDispel, excludeCC) local s = "" if excludeDispel then if playerMode then s = s .. "|!" .. dispelToken elseif anyToken then s = s .. "|!" .. anyToken end end if excludeCC and ccToken then s = s .. "|!" .. ccToken end - if excludeRaid and raidOn then s = s .. "|!RAID" end return s end -- candidateFilters for one record. Hands each record its OWN table (extra @@ -284,34 +288,63 @@ local function BuildDirectDebuffFilters(db, claimed) -- machinery above deliberately keeps reading the RAW enabled flags). local effBoss = boss and not (claimed and claimed.boss) local effRole = role and not (claimed and claimed.role) + -- IMPORTANT-FIRST PRECEDENCE. These records used to carry neg(true, true, true), + -- i.e. boss/role and priority EXCLUDED anything dispellable, CC or raid-flagged. + -- That made the important categories the LOWEST precedence: a priority debuff that + -- also carried the RAID token was pushed out of this styled record and into the + -- unstyled "raid" one below, so the Important Debuffs highlight silently did nothing + -- for it. Most boss/priority debuffs in group content DO carry RAID, so with the + -- Blizzard category filters enabled the highlight looked broken for about half the + -- auras it should have covered (field-reported; Show All mode was unaffected). + -- + -- Exclusivity now runs the same direction Show All mode has always used (see the + -- ALL-mode block near the top of this file): the important records claim their auras + -- FIRST with no negation, and the token records below subtract them via + -- candidateFilter flags. Same no-double-render guarantee, correct precedence, and + -- important debuffs now genuinely lead the row rather than only sometimes. + local importantFlag -- boss/role flag actually declared; nil if there is no such record + local priorityDeclared = false if effBoss or effRole then - local flag = (effBoss and effRole) and "isBossOrRoleAura" or (effBoss and "isBossAura" or "isRoleAura") - filters[#filters + 1] = { filter = "HARMFUL" .. neg(true, true, true), key = "bossrole", - candidateFilters = cfFor(true, { [flag] = true }), + importantFlag = (effBoss and effRole) and "isBossOrRoleAura" or (effBoss and "isBossAura" or "isRoleAura") + filters[#filters + 1] = { filter = "HARMFUL", key = "bossrole", + candidateFilters = cfFor(true, { [importantFlag] = true }), style = importantStyle } end if db.debuffFilterPriority and not (claimed and claimed.priority) then - filters[#filters + 1] = { filter = "HARMFUL" .. neg(true, true, true), key = "priority", - candidateFilters = cfFor(true, { isPriorityAura = true }), + priorityDeclared = true + local extra = { isPriorityAura = true } + -- Only subtract boss/role when that record actually exists, and subtract the + -- SAME flag it was declared with (isBossOrRoleAura / isBossAura / isRoleAura). + if importantFlag then extra[importantFlag] = false end + filters[#filters + 1] = { filter = "HARMFUL", key = "priority", + candidateFilters = cfFor(true, extra), style = importantStyle } end + -- Subtract whichever important records were declared. Returns a FRESH table each + -- call because cfFor mutates and returns the table it is handed. + local function notImportant(extra) + extra = extra or {} + if importantFlag then extra[importantFlag] = false end + if priorityDeclared then extra.isPriorityAura = false end + return extra + end if ccToken and not (claimed and claimed.crowdControl) then - filters[#filters + 1] = { filter = "HARMFUL|" .. ccToken .. neg(true, false, false), - key = "cc", candidateFilters = cfFor(false) } + filters[#filters + 1] = { filter = "HARMFUL|" .. ccToken .. neg(true, false), + key = "cc", candidateFilters = cfFor(false, notImportant()) } end if raidOn and not (claimed and claimed.raid) then - filters[#filters + 1] = { filter = "HARMFUL|RAID" .. neg(true, true, false), - key = "raid", candidateFilters = cfFor(false) } + filters[#filters + 1] = { filter = "HARMFUL|RAID" .. neg(true, true), + key = "raid", candidateFilters = cfFor(false, notImportant()) } end if dispelOn and not (claimed and claimed.dispellable) then if playerMode then filters[#filters + 1] = { filter = "HARMFUL|" .. dispelToken, - key = "dispel", candidateFilters = cfFor(false) } + key = "dispel", candidateFilters = cfFor(false, notImportant()) } elseif anyToken then filters[#filters + 1] = { filter = "HARMFUL|" .. anyToken, - key = "dispel", candidateFilters = cfFor(false) } + key = "dispel", candidateFilters = cfFor(false, notImportant()) } else - local cf = { includeDispelTypes = DISPEL_TYPES } + local cf = notImportant({ includeDispelTypes = DISPEL_TYPES }) if maxDur then cf.maxDuration = maxDur end filters[#filters + 1] = { filter = "HARMFUL", key = "dispel", candidateFilters = cf } end @@ -592,8 +625,30 @@ end -- PER-INDICATOR setting — DF.Expiration:Unit(cfg) — so one global could not express it: -- a glyph revealing at 5 seconds and a border revealing at 30% are both legitimate at the -- same time. Use DF:GetDurationRampKey(DF.Expiration:Unit(cfg)) to reach its ramp.) +-- ☠ MEMOISED PER SCALE, AND THE RETURNED LIST IS SHARED — do not mutate it. +-- Every call rebuilt the whole ladder: one table for the list, one table per +-- breakpoint, and a colorToHex string per breakpoint — from account-wide config +-- that only changes on a Colours-page edit. It runs per aura per tick and was +-- 6.6% of all trash-fight allocation. +-- +-- Verified before sharing: all five call sites only READ (ipairs, indexed reads, +-- breakpointsSig) — none writes into the list or into an entry. A future caller +-- that needs to mutate must copy first. +-- +-- Keyed on the resolved DEF TABLE rather than the scale string, so an unrecognised +-- scale (which falls back to TEXT_SECONDS) shares that entry instead of growing a +-- duplicate under its own name. +-- +-- Invalidation is the one that already exists: DF:InvalidateDurationFormatters +-- wipes this next to the formatter and curve caches, and its own comment already +-- makes it the required call for anything mutating these stops. Nothing new to +-- remember, and the PERCENT scale now gets cached too — GetDurationBreakpointsSig +-- only ever memoised TEXT_SECONDS. +local durationBreakpointsCache = {} local function GetDurationColorBreakpoints(scale) local def = COLOR_SCALES[scale] or COLOR_SCALES.TEXT_SECONDS + local cached = durationBreakpointsCache[def] + if cached then return cached end local g = DF.GetGlobalDB and DF:GetGlobalDB() local raw = g and g[def.key] local out = {} @@ -605,9 +660,13 @@ local function GetDurationColorBreakpoints(scale) end end end - if #out == 0 then return def.fallback end + if #out == 0 then + durationBreakpointsCache[def] = def.fallback + return def.fallback + end table.sort(out, function(a, b) return a.threshold > b.threshold end) -- descending if out[#out].threshold ~= 0 then out[#out + 1] = { threshold = 0, hex = out[#out].hex, color = out[#out].color } end + durationBreakpointsCache[def] = out return out end @@ -806,6 +865,10 @@ local durationBreakpointsSigCache -- memoized DF:GetDurationBreakpointsSig() s function DF:InvalidateDurationFormatters() wipe(durationFormatterCache) durationBreakpointsSigCache = nil + -- The resolved ladders themselves are cached now (see GetDurationColorBreakpoints); + -- without this the memoized signature would rebuild from stale stops and every + -- consumer would keep painting the old ramp. + wipe(durationBreakpointsCache) -- Colour curves are built from the same stops (DF:GetDurationColorSpec) — a stop edit -- must drop them too or the cached curve keeps painting the old ramp. if DF._wipeDurationCurves then DF:_wipeDurationCurves() end diff --git a/DandersFrames/Features/Dispel.lua b/DandersFrames/Features/Dispel.lua index d211ffd8..92d0c742 100644 --- a/DandersFrames/Features/Dispel.lua +++ b/DandersFrames/Features/Dispel.lua @@ -1631,6 +1631,42 @@ local function StyleGameEdgeSlot(btn, frame, db, edge) ApplySlotPulse(btn.dfDispelEdgeHolder and btn.dfDispelEdgeHolder[edge], db.dispelAnimate) end +-- STALE SLOT ART (bug #1004's shape, second consumer). Every widget hung off a slot +-- button -- the overlay frame, its border StatusBars, the bound carriers -- is a +-- descendant of the SECRET aura button, and the client can turn that subtree forbidden +-- underneath us when it reclaims or re-initialises the slot. Nothing ever cleared +-- btn.dfDispelWidget, so the stash outlived the art: ApplyOverlayLayout's first touch +-- (borderLeft:ClearAllPoints) then threw on every state-changing refresh, which aborted +-- the whole style pass for that button and left the overlay dead. Reported 32x after a +-- dungeon and 98x on one Show Overlay For dropdown -- the same frames, once per refresh, +-- so it is persistent rather than transient. Frames\Core.lua's health mirror carries the +-- identical guard for the identical reason; this is that guard for the dispel art. +-- +-- Read-only, and it touches the object that actually throws (borderLeft) rather than +-- IsForbidden() -- inside the secret container IsForbidden can hand back a SECRET on a +-- perfectly healthy widget, and the codebase's guard idiom reads a secret as "skip", +-- which would silently kill the overlay for everyone. Declared once, not inlined as a +-- closure, so the pcall below stays allocation-free on a per-slot-per-pass path. +local function ProbeSlotArt(w) + return w:GetFrameLevel() and w.borderLeft:GetFrameLevel() +end + +-- Drop EVERY stash hanging off the button. The tainted painters (icon, and the widget +-- itself via EnsureSlotWidget) rebuild lazily from nil; the bound carriers do not, which +-- is why the caller re-runs DispelSlotSecureInit behind this. +local function ResetSlotArt(btn) + btn.dfDispelWidget = nil + btn._dfDispelCarriers = nil + btn._dfDispelCurveGen = nil + btn._dfDispelGradientCarrier = nil + btn.dfDispelEdgeTex = nil + btn.dfDispelEdgeHolder = nil + btn.dfDispelRing = nil + btn.dfDispelRingHolder = nil + btn.dfDispelIconTex = nil + btn.dfDispelIconHolder = nil +end + -- Style every live slot button per the plan. Returns false while no buttons exist -- (combat-deferred build / fake backend) so the caller doesn't latch the version. local function StyleDispelSlots(frame, db, h, slots) @@ -1640,7 +1676,31 @@ local function StyleDispelSlots(frame, db, h, slots) for i = 1, #slots do local info = slots[i] local btn = buttons[info.key] - if btn then + -- Recover a button whose art went forbidden before anything tries to paint it. + -- ONE retry per button: DispelSlotSecureInit rebuilds and re-binds from here + -- (tainted create+bind is legal on 68914 -- see the re-bind note below), but if + -- the fresh art comes back untouchable too, retrying every pass would trade an + -- error spam for a rebuild spam. The latch clears the moment a probe passes, so + -- a later reclaim is still recoverable. + local artOK = true + if btn and btn.dfDispelWidget and not pcall(ProbeSlotArt, btn.dfDispelWidget) then + if btn._dfDispelArtStale then + artOK = false + if not frame.dfDispelArtForbiddenLogged then + frame.dfDispelArtForbiddenLogged = true + DF:DebugWarn("AURACONTAINER", + "StyleDispelSlots: slot art still forbidden after a rebuild, " + .. "skipping the dispel overlay on %s", tostring(frame.unit)) + end + else + btn._dfDispelArtStale = true + ResetSlotArt(btn) + if info.roles then DispelSlotSecureInit(btn, info, db, frame) end + end + elseif btn then + btn._dfDispelArtStale = nil + end + if btn and artOK then styled = true -- IN-PLACE PALETTE RE-BIND (68914): Blizzard securecopy's the colour map at -- bind time, so a Colours-page edit needs the carrier RE-BOUND. That used to @@ -1718,6 +1778,20 @@ function DF:DriveDispelOverlayFactory(frame, db) return end + -- EMPTY HEADER SLOT. A raid header keeps its whole child set alive and hands + -- units out as the roster fills, so outside a full group most children carry + -- no unit at all. They are hidden, they can never match a dispellable aura, + -- and AuraContainer:Create defaults a nil unit to "player" -- so building + -- here stood up a six-slot container per EMPTY slot and pointed each one at + -- the wrong unit. UpdateAllDispelOverlays walks IterateAllFrames unfiltered + -- (the aura rows come through FullFrameRefresh, which already returns on a + -- unitless frame -- that is why they never paid this). + -- Measured, login while solo: 41 containers built, 40 of them empty raid + -- slots -- 10.8 MB of the 14.1 MB the addon spent standing containers up. + -- Existing handles are deliberately LEFT ALONE: the roster churns, and + -- SetUnit re-targets a live one for free when a unit lands in this slot. + if not frame.unit then return end + -- Fast path (this runs per UNIT_AURA in combat): already built AND styled at -- the current layout version + build generation -> only unit upkeep remains. -- Skips the per-call plan/signature allocation entirely; any settings change diff --git a/DandersFrames/Features/Highlights.lua b/DandersFrames/Features/Highlights.lua index a6925a35..ccbe323f 100644 --- a/DandersFrames/Features/Highlights.lua +++ b/DandersFrames/Features/Highlights.lua @@ -216,27 +216,57 @@ end -- HIGHLIGHT FRAME CREATION -- ============================================================ +-- ☠ THESE ARE ABSOLUTE-ERA OFFSETS. They were +9 / +10 / +11, written when a unit +-- frame's children topped out around +4 and anything above the health bar was +-- "on top". Since the frame-level rework every DF element carries an ABSOLUTE +-- offset from the unit frame, and the content stack now runs: resource bar 20, +-- contentOverlay 25, status icons 30, missing buff 35, buff/debuff rows 40, +-- defensive 65 -- whose border art reaches 77, because a row is 13 levels thick. +-- At +10 the hover highlight sat under ALL of that. The reported "resource bar +-- draws over the hover highlight" is just the shallowest instance of it. +-- +-- Safe to raise: every mode draws at the frame PERIMETER only (SOLID, CORNERS and +-- DASHED are edge lines, GLOW is an edge glow), so nothing here can obscure the +-- name or health text. It only stops frame content covering the frame's own +-- selection affordance. +-- +-- Relative order among the three is preserved: aggro < hover < selection. +-- +-- The numbers: 65 is the highest default in Config (defensiveIconFrameLevel), and +-- a row's art sits above its baseline -- by +12 or +14 depending on which element +-- is tallest, so call the real ceiling 79. These sit clear of it rather than on +-- the boundary, deliberately: land on 78 and a single off-by-one in that estimate +-- puts the highlight back under the art with no visible reason why. +-- ⚠ Clears the DEFAULT stack only. The per-element sliders run 0-100, so pushing +-- an element above these covers the highlight again -- that is the slider doing +-- what it says, not a regression. +local HIGHLIGHT_LEVEL = { Aggro = 82, Hover = 83, Selection = 84 } + +-- Applied on REUSE as well as creation: the level is absolute, derived from the +-- owner's level at the time it is set, so a highlight created before the owner's +-- level changed would otherwise keep a stale one forever. +local function ApplyHighlightZOrder(ch, frame, highlightType) + ch:SetFrameStrata(frame:GetFrameStrata()) + ch:SetFrameLevel(frame:GetFrameLevel() + (HIGHLIGHT_LEVEL[highlightType] or HIGHLIGHT_LEVEL.Aggro)) +end + local function GetOrCreateHighlight(frame, highlightType) local key = "df" .. highlightType .. "Highlight" - if frame[key] then + if frame[key] then -- Update points on existing frame to ensure proper positioning local ch = frame[key] ch:ClearAllPoints() ch:SetPoint("TOPLEFT", frame, "TOPLEFT", 0, 0) ch:SetPoint("BOTTOMRIGHT", frame, "BOTTOMRIGHT", 0, 0) + ApplyHighlightZOrder(ch, frame, highlightType) return ch end - + -- Parent to UIParent to avoid any clipping from ancestors local ch = CreateFrame("Frame", nil, UIParent) ch:SetPoint("TOPLEFT", frame, "TOPLEFT", 0, 0) ch:SetPoint("BOTTOMRIGHT", frame, "BOTTOMRIGHT", 0, 0) - ch:SetFrameStrata(frame:GetFrameStrata()) - -- Frame levels: Aggro = +9, Hover = +10, Selection = +11 - local levelOffset = 9 - if highlightType == "Hover" then levelOffset = 10 - elseif highlightType == "Selection" then levelOffset = 11 end - ch:SetFrameLevel(frame:GetFrameLevel() + levelOffset) + ApplyHighlightZOrder(ch, frame, highlightType) ch:Hide() -- Track the owner frame so we can hide when owner hides @@ -283,20 +313,72 @@ end -- APPLY HIGHLIGHT STYLE -- ============================================================ +-- Forget a highlight's cached style so the next ApplyHighlightStyle re-runs in full. +-- ☠ EVERY path that changes those textures behind this function's back MUST call +-- this. There are two classes and both are live: +-- 1. DF:LightweightUpdateHighlight (Core.lua) writes size/points/colour straight +-- onto the four line textures during a slider drag, bypassing this function. +-- 2. The three "not wanted" branches below hide the highlight AND call +-- SelectionAnimator_Remove. Without invalidation, turning a highlight off and +-- back on with identical settings would early-out and never re-add it to the +-- animator -- an ANIMATED highlight that silently stops animating. +function DF:InvalidateHighlightStyle(ch) + if ch then ch._hlSig = nil end +end + +-- ☠ THE SIGNATURE MUST INCLUDE THE THREE IMPLICIT INPUTS, not just the arguments. +-- This function is 22.4% of trash-fight allocation and target-switch driven, so it +-- is worth skipping -- but "hide everything then re-apply" means a missed input +-- leaves stale art with no visible cause. Beyond the eight parameters: +-- * ch:GetEffectiveScale() -- thickness and inset are pixel-snapped against it, +-- so a UI-scale change must re-run. +-- * ch:GetWidth()/GetHeight() -- CORNERS mode derives cornerLen from them, so a +-- frame resize must re-run. +-- * db.pixelPerfect -- the only field read off db. local function ApplyHighlightStyle(ch, mode, thickness, inset, r, g, b, alpha, db) if not ch then return end - + + local scale = ch:GetEffectiveScale() + local w, h = ch:GetWidth(), ch:GetHeight() + local pp = (db and db.pixelPerfect) and 1 or 0 + -- ☠ SHOW FIRST, AHEAD OF THE EARLY-OUT. Showing the highlight is an OUTPUT of + -- this function (it used to be the last statement), not part of the styling the + -- signature guards -- and several paths hide `ch` WITHOUT touching the signature: + -- the owner frame's own OnHide hook (installed in GetOrCreateHighlight), the + -- party-frames-while-in-raid branch, and the not-visible branch. With the Show + -- below the early-out, a frame that hid and re-showed while every style input + -- stayed identical never got shown again -- target a party member, open Test + -- Mode, close it, and the selection border was gone until you changed target. + -- Hoisting it fixes every such path at once, including any added later, which + -- invalidating at the three hide sites would not. + -- No flash risk: the styling below runs synchronously in this same call, so + -- nothing is drawn between the Show and the restyle. + ch:Show() + -- Numeric fields compared individually: building a string key here would + -- allocate exactly what the early-out exists to avoid. + if ch._hlSig + and ch._hlMode == mode and ch._hlThick == thickness and ch._hlInset == inset + and ch._hlR == r and ch._hlG == g and ch._hlB == b and ch._hlA == alpha + and ch._hlPP == pp and ch._hlScale == scale and ch._hlW == w and ch._hlH == h then + return + end + ch._hlSig = true + ch._hlMode, ch._hlThick, ch._hlInset = mode, thickness, inset + ch._hlR, ch._hlG, ch._hlB, ch._hlA = r, g, b, alpha + ch._hlPP, ch._hlScale, ch._hlW, ch._hlH = pp, scale, w, h + local top, bottom, left, right = ch.topLine, ch.bottomLine, ch.leftLine, ch.rightLine - + -- Hide all styles first top:Hide() bottom:Hide() left:Hide() right:Hide() HideAnimatedBorder(ch) HideCornerTextures(ch) HideGlowLayers(ch) SelectionAnimator_Remove(ch) - - -- Snap thickness to whole screen pixels so every +1 step is visible - local scale = ch:GetEffectiveScale() + + -- Snap thickness to whole screen pixels so every +1 step is visible. + -- `scale` is the one read above for the signature -- deliberately the same + -- value, so what gets cached and what gets applied can never disagree. local px = thickness * scale -- desired thickness in pixels px = math.max(1, math.ceil(px - 0.01)) -- round up (with tiny epsilon for exact integers) thickness = px / scale @@ -447,8 +529,7 @@ local function ApplyHighlightStyle(ch, mode, thickness, inset, r, g, b, alpha, d top:Show() left:Show() end - - ch:Show() + -- (ch:Show() moved ABOVE the early-out -- see the note there.) end -- Expose for reuse by the Aura Designer border indicator @@ -739,6 +820,10 @@ function DF:UpdateHighlights(frame, forceSelection, forceAggro) HideGlowLayers(selectionHighlight) selectionHighlight:Hide() SelectionAnimator_Remove(selectionHighlight) + -- Removing it from the animator undoes what ApplyHighlightStyle set up, so + -- the cached style no longer describes reality: without this, re-showing + -- with identical settings would early-out and never re-add it. + DF:InvalidateHighlightStyle(selectionHighlight) end -- Hover Highlight @@ -772,6 +857,7 @@ function DF:UpdateHighlights(frame, forceSelection, forceAggro) HideGlowLayers(hoverHighlight) hoverHighlight:Hide() SelectionAnimator_Remove(hoverHighlight) + DF:InvalidateHighlightStyle(hoverHighlight) end -- Aggro Highlight @@ -859,6 +945,7 @@ function DF:UpdateHighlights(frame, forceSelection, forceAggro) HideGlowLayers(aggroHighlight) aggroHighlight:Hide() SelectionAnimator_Remove(aggroHighlight) + DF:InvalidateHighlightStyle(aggroHighlight) end end diff --git a/DandersFrames/Features/TargetedSpells.lua b/DandersFrames/Features/TargetedSpells.lua index 607a5874..ac406ad0 100644 --- a/DandersFrames/Features/TargetedSpells.lua +++ b/DandersFrames/Features/TargetedSpells.lua @@ -3423,8 +3423,26 @@ end -- Called per bar during render (both real and test paths), and again -- from UpdateTargetedListLayout when settings change. The function -- runs at drag-tick rate during slider interaction so keep it cheap. +-- Everything this function writes derives from `db` alone -- never from the spell, the +-- bar's state or the clock -- so it is pure config and does not belong on the render +-- path. TargetedList_LayoutBars called it for every active bar on EVERY render, which +-- made it 86% of all Border:Apply allocation in a combat trace (BuildSpec + Apply + +-- three or four SafeSetFont calls, per bar, per tick). +-- +-- Gated on targetedListLayoutVersion, which already existed at the top of this file and +-- was incremented by DF:UpdateTargetedListLayout but never read by anything. That hook +-- is the single funnel for every settings change: the Options pages call it, preset +-- application calls it, and a profile switch reaches it via Position.lua. So a bumped +-- version is exactly "the config changed", which is exactly when this work is needed. +-- +-- ☠ TargetedList_ResetBar clears the stamp on release, next to the _lastTexturePath it +-- already cleared for the same reason. Without that, a recycled bar would skip this +-- function entirely and lose the SetStatusBarTexture that clearing _lastTexturePath +-- exists to force. local function TargetedList_ApplyBarAppearance(bar, db) if not bar or not db then return end + if bar._appearanceVersion == targetedListLayoutVersion then return end + bar._appearanceVersion = targetedListLayoutVersion local barH = db.targetedListHeight or 22 local showIcon = db.targetedListShowIcon ~= false local iconPos = db.targetedListIconPosition or "LEFT" @@ -3538,6 +3556,10 @@ local function TargetedList_ResetBar(pool, bar) bar._testDuration = nil bar.icon:SetTexture(nil) bar._lastTexturePath = nil + -- Same reason as _lastTexturePath above: drop the cached appearance stamp so a + -- recycled bar re-runs TargetedList_ApplyBarAppearance on acquire. Without this the + -- version gate would skip it and the bar would never get its status-bar texture back. + bar._appearanceVersion = nil if bar.highlightFrame then bar.highlightFrame:Hide() end diff --git a/DandersFrames/FilterRegistry/Registry.lua b/DandersFrames/FilterRegistry/Registry.lua index e7433c94..68d3a9df 100644 --- a/DandersFrames/FilterRegistry/Registry.lua +++ b/DandersFrames/FilterRegistry/Registry.lua @@ -373,6 +373,41 @@ function R:DecodeFilterString(str) return { name = name, spells = spells, rawIDs = rawIDs } end +-- ------------------------------------------------------------ +-- NAME COLLISIONS +-- ------------------------------------------------------------ +-- Filter names are NOT unique, and deliberately stay that way: New, Rename and +-- Duplicate all let you pick whatever you like, and people have deliberately +-- similar names saved. What matters is that you SEE the name before it is +-- committed. Duplicate does — it pre-fills " copy" and prompts. Import did +-- not, so a shared string could silently add a second row indistinguishable from +-- one you already had; field-reported, and worst through "Import as Copy", the +-- option whose whole promise is a copy you can tell apart. +-- +-- These exist so the import path can ask ONLY when it needs to, leaving a clean +-- import (no clash) with no extra step. +function R:IsCustomFilterNameTaken(name, exceptID) + if not name or name == "" then return false end + for id, f in pairs(self:GetStore().customFilters or {}) do + if id ~= exceptID and f.name == name then return true end + end + return false +end + +-- "X" -> "X copy" -> "X copy 2" ... reusing Duplicate's suffix so both paths +-- produce names in the same shape. Bounded rather than while-true: a pathological +-- store should degrade to a duplicate name, not hang the client. +function R:SuggestUniqueFilterName(name) + if not self:IsCustomFilterNameTaken(name) then return name end + local base = name .. " copy" + if not self:IsCustomFilterNameTaken(base) then return base end + for n = 2, 99 do + local candidate = base .. " " .. n + if not self:IsCustomFilterNameTaken(candidate) then return candidate end + end + return base +end + -- ALWAYS creates. Collision handling is the caller's call — a deliberate share -- should surface "you already have this" and let the user choose, where profile -- import silently reuses (ImportCustomFilters). diff --git a/DandersFrames/Frames/AuraContainer.lua b/DandersFrames/Frames/AuraContainer.lua index 9f523df8..850abec4 100644 --- a/DandersFrames/Frames/AuraContainer.lua +++ b/DandersFrames/Frames/AuraContainer.lua @@ -185,6 +185,103 @@ do end) end +-- ============================================================ +-- EDIT-MODE DEAFENING — the per-container opt-out +-- ============================================================ +-- 12.1 build 68569 folded CustomAuraContainer onto the (new in that build) +-- ManagedAuraContainer, and with it our containers inherited Edit Mode's sample +-- data. On 68412 CustomAuraContainerPrivateMixin:ParseAllAuras called +-- C_UnitAuras.GetUnitAuras HARD-CODED, so custom containers were immune by +-- construction; now parsing runs through GetAuraSources(), which returns +-- AuraContainerAuraSourceLists.EditMode whenever the container's own flag is set. +-- Exactly ONE thing sets that flag — the container's own handler: +-- +-- ManagedAuraContainerPrivateMixin:OnAuraDataProviderSwitch(useRealDataProvider) +-- self:SetUseEditModeSource(not useRealDataProvider) +-- +-- The normal source reads C_UnitAuras DIRECTLY — it does NOT go through the +-- swapped AuraUtil provider — so a container that never HEARS the switch keeps +-- rendering live auras with Edit Mode open. Probe 33 already proved this from the +-- other side ("born-deaf containers": one built after a switch stays on the +-- previous source); the old "the swap is below the container layer" reading was +-- wrong for this build. +-- +-- ☠ WHY IT GOES IN THE BUILD PATH: AURA_DATA_PROVIDER_SWITCH is a STATIC event — +-- registered once in AuraContainerPrivateMixin:OnLoad_Intrinsic, and +-- UpdateEventRegistrations only ever churns the DYNAMIC lists — so an unregister +-- sticks for the life of that frame. But only THAT frame: every structural +-- rebuild creates a brand new container, which registers it afresh. A one-shot +-- deafening expires silently, which is the likeliest reason probe 34 read as a +-- false negative. So it is re-applied on every build, at the creation site. +-- +-- DF's own test mode RIDES this event (the provider bounce is how the curated +-- preview gets its data), so this is a TOGGLE, not a permanent unregister: +-- containers are born deaf, and are born hearing while _testMode is on. +local PROVIDER_EVENT = "AURA_DATA_PROVIDER_SWITCH" + +-- AuraContainer._providerDeafOK: nil = not yet probed, true/false = the live +-- client's answer. The container is a forbidden-table object; base widget methods +-- ARE reachable (we already call SetScale / SetPoint / SetMouseClickEnabled on +-- it), but the event methods have to be proven at runtime rather than assumed. +-- The dangerous case is a SILENT refusal — the unregister does not error but the +-- event stays registered — which would leave every row showing sample icons, so +-- it is tested for explicitly. Anything short of a proven success keeps the old +-- hide-the-rows guard (see EDIT-MODE GUARD below) as the fallback. +local function setContainerProviderDeaf(c, deaf) + if not c then return end + + -- ☠ ANSWERED IN GAME (68914): this is refused, and it is refused BY DESIGN. + -- Frame:UnregisterEvent(): Function call not permitted on forbidden aspect + -- 'EventRegistrations' (execution tainted by 'DandersFrames') + -- Enum.ForbiddenAspect.EventRegistrations is documented as "Restricts querying + -- or modifying registered script events for this object", every Register* method + -- ADDS it, and UnregisterEvent / UnregisterAllEvents / IsEventRegistered CHECK + -- it. Blizzard's own OnLoad_Intrinsic registers AURA_DATA_PROVIDER_SWITCH from + -- secure code, so the container carries the aspect from birth — there is no + -- window before it, and no ordering trick. The REBIRTH FALLBACK below is the + -- real mechanism; keep this only as a one-shot capability probe so a future + -- build that relaxes the rule is noticed rather than assumed away. + if AuraContainer._providerDeafOK ~= nil then return end -- answered once, never retried + + if not deaf then + -- Test mode wants the container to hear the bounce. Nothing to do: a fresh + -- container is already registered by OnLoad, and re-registering would hit + -- the same forbidden aspect anyway. + return + end + + -- ⚠ Written as an explicit branch, not `deaf and c.UnregisterEvent or + -- c.RegisterEvent`: that idiom falls through to RegisterEvent when + -- UnregisterEvent is nil, i.e. it would REGISTER the event while reporting + -- that it had deafened the container. Exactly backwards. + local why + if not c.UnregisterEvent then + AuraContainer._providerDeafOK, AuraContainer._providerDeafWhy = + false, "UnregisterEvent missing on the container" + return + end + local ok, err = pcall(c.UnregisterEvent, c, PROVIDER_EVENT) + if not ok then + AuraContainer._providerDeafOK = false + AuraContainer._providerDeafWhy = "UnregisterEvent errored: " .. tostring(err) + DF:DebugWarn(DBG, "provider deafening refused: %s", tostring(err)) + return + end + + -- "It did not error" is NOT proof it took — a silent refusal is the case that + -- error" is NOT proof it took — a silent refusal is the case that matters. + local okQ, stillRegistered = pcall(c.IsEventRegistered, c, PROVIDER_EVENT) + if not okQ then + AuraContainer._providerDeafOK, why = true, "accepted; IsEventRegistered unavailable (UNVERIFIED)" + elseif stillRegistered then + AuraContainer._providerDeafOK, why = false, "silently refused — still registered after UnregisterEvent" + else + AuraContainer._providerDeafOK, why = true, "confirmed deaf" + end + AuraContainer._providerDeafWhy = why + DF:Debug(DBG, "provider deafening: %s", why) +end + -- TEST MODE (P5 hybrid, probe 33 live-proven). A real CustomAuraContainer reads real -- unit auras — nothing renders on a fabricated test unit. Instead of faking the -- CONTAINER we fake the DATA: the game's own sample provider @@ -225,9 +322,16 @@ function AuraContainer._queueTestBounce() -- never fill, so every badge sits parked in its window — the "missing" -- preview. Enabling would fill the groups with sample HELPFUL auras -- (spell-ID filters are stripped in test) and push every badge out. + -- TEST HANDLES ONLY, matching rebuildAll. "Built DISABLED" above is the + -- other half of this pair, and only a container built while _testMode was + -- on is disabled (build()'s SetEnabled folds in `not testMode`). Once + -- rebuildAll stopped rebuilding live handles they were no longer disabled + -- either, so an unscoped loop issued a pointless setEnabled+refresh on + -- every live container at test entry — refresh() is a Hide/Show bounce + -- that re-arms a full aura parse, so it was not free. for h in pairs(AuraContainer._handles or {}) do - if not h._destroyed and h.backend and h.config and h.config.enabled ~= false - and h.config.mode ~= "missing" then + if not h._destroyed and h._testFrame and h.backend and h.config + and h.config.enabled ~= false and h.config.mode ~= "missing" then if h.backend.setEnabled then pcall(function() h.backend:setEnabled(true) end) end if h.backend.refresh then pcall(function() h.backend:refresh() end) end end @@ -241,10 +345,56 @@ function AuraContainer.SetTestMode(on) AuraContainer._testMode = on DF:Debug(DBG, "SetTestMode -> %s", tostring(on)) + -- TEST HANDLES ONLY. A container has to be REBUILT across a test transition + -- because the two shapes declare different groups and groups can never be + -- removed: test mode declares dfTestStyled/dfTestPlain and skips the real + -- filter loop entirely (build(), `filters = {}`), so neither shape can be + -- reached from the other in place. + -- + -- But that only ever applied to the handles that RENDER the preview. Live + -- frames are hidden for the whole session by SetTestModeStateDrivers, so + -- rebuilding them into test shape and back rendered nothing either way. + -- + -- ☠ DO NOT re-justify this with "live containers are built deaf". They are + -- NOT. setContainerProviderDeaf (top of this file) is a one-shot CAPABILITY + -- PROBE that no-ops after its first call -- UnregisterEvent on the container + -- is refused by design (ForbiddenAspect.EventRegistrations, answered in game + -- on 68914). Every container hears AURA_DATA_PROVIDER_SWITCH, so a live + -- handle left standing does follow the bounce onto the sample provider. + -- It is HIDDEN, so that is invisible -- with one pre-existing exception: + -- the [combat] state driver reveals live frames the instant combat starts + -- (see TestMode.lua's note), and those rows then show sample auras. That + -- edge was already wrong before this change (it showed curated TEST paint on + -- live frames instead), so this neither introduces nor fixes it. Unverified + -- in game; if it matters, the fix is to re-point live handles on reveal, not + -- to rebuild every one of them on both transitions. + -- + -- ⚠ It is NOT true that the entry pass can be dropped altogether -- that was + -- the first read of the trace and it is wrong. The test frame pool is created + -- once (testFramePoolInitialized) and its frames are only HIDDEN on exit, so + -- from the SECOND entry onward the test handles already exist here and do + -- need the rebuild. On the very first entry this loop legitimately does + -- nothing: the pool is built after SetTestMode returns and those handles are + -- born in test shape (build() reads _testMode itself). + -- ☠ LOAD-BEARING INVARIANT, and it is not local to this function: build() picks + -- its SHAPE from the GLOBAL AuraContainer._testMode (the `local testMode` read, + -- and SetEnabled's `not testMode`), while the rebuild below is keyed on the + -- PER-HANDLE flag. They must agree. A handle built while _testMode is on but + -- whose _testFrame is false would be born test-shaped and never rebuilt out of + -- it — groups can never be removed, so that container is stuck showing curated + -- paint on a live frame forever. What guarantees they agree is that NO live + -- handle is ever built during test mode, enforced by the UseFactoryFor* gates + -- in Features/Auras.lua, Frames/Icons.lua and AuraDesigner (each excludes + -- DF.testMode/raidTestMode). Before this became conditional the unconditional + -- rebuild made the invariant unnecessary. The asymmetry is what makes it worth + -- naming: a false NEGATIVE silently corrupts what is on screen, a false + -- POSITIVE only costs one extra rebuild. local function rebuildAll() if AuraContainer._handles then for h in pairs(AuraContainer._handles) do - if not h._destroyed then pcall(function() h:OnTestModeChanged() end) end + if not h._destroyed and h._testFrame then + pcall(function() h:OnTestModeChanged() end) + end end end end @@ -997,9 +1147,28 @@ local function bindNative(slot, config) -- text beat vertex colour, so a spec never carries both (the row builders send -- a curve OR a coloured formatter, never each). if durSpec.colorCurve and durSpec.colorProperty ~= nil then - opts.textColor = { curve = durSpec.colorCurve, property = durSpec.colorProperty } + -- Cached on the config by spec identity, exactly like _dfDurBind above. + -- This runs once per BUTTON, so a fresh table here cost one allocation + -- per slot per rebuild across every row on every frame. + -- ☠ The key is sufficient because a duration spec is never mutated in + -- place -- every writer of colorCurve/colorProperty fills a table that + -- is still a local (Auras.lua's TextStyle:BuildSpec result, Factory's + -- constructor literal), and a curve rebuild copies BY VALUE into a + -- brand-new dur table. Do NOT restate this as "a structural Rebuild + -- hands over a fresh style table": SetFilter, the test-mode ApplyTuning + -- and the deferred regen rebuild all call _rebuild() on the SAME config + -- and style, so the cache demonstrably survives a rebuild. It is the + -- no-in-place-mutation property that makes it safe, nothing else. + if config._dfDurColorSpec ~= durSpec then + config._dfDurColorSpec = durSpec + config._dfDurColor = { curve = durSpec.colorCurve, property = durSpec.colorProperty } + end + opts.textColor = config._dfDurColor end - local ok, err = pcall(function() slot:SetDurationText(slot.dfDur, opts) end) + -- pcall(fn, self, args...) rather than pcall(function() ... end): no wrapper + -- closure per button (same reason as the AddAuraGroup call in build()). + -- Protection is unchanged. + local ok, err = pcall(slot.SetDurationText, slot, slot.dfDur, opts) if ok then slot._boundDur = true elseif not warnedCurve then @@ -1414,6 +1583,19 @@ local function applyContainerLayout(c, handle) -- and protect each call SEPARATELY: pre-68914 this rode the pin pcall above, -- so the rename made the first layout call throw and silently dropped -- growth/wrap/padding for the whole row. + -- Stashed for the SINGLE-SLOT row path: the flow does not lay slots out, so + -- build() pins its one button at the corner the flow would have placed + -- element 1 at. Kept here so there is ONE derivation of the corner. + handle._flowAnchor = G.flowAnchor + -- ...and the STRIP RESERVATION with it. The reservation reaches a group button + -- as flow-layout PADDING (setFlowPadding below), which the flow applies to + -- element 1 — a hand-pinned slot button never sees it, so it has to be folded + -- into the pin offset instead. Only one of the two can be non-zero (the branch + -- above is exclusive), and the anchor corner always matches the growth + -- direction, so the difference carries the right sign in WoW's y-up space: + -- top strip on downward growth pushes the icon DOWN (-padTop), bottom strip on + -- upward growth pushes it UP (+padBottom). + handle._flowPadY = padBottom - padTop local setFlowAnchor = c.SetFlowLayoutAnchorPoint or c.SetAuraLayoutAnchorPoint local setFlowGrowth = c.SetFlowLayoutGrowthDirection or c.SetAuraLayoutGrowthDirection local setFlowMaxLine = c.SetFlowLayoutMaximumLineSize or c.SetAuraLayoutRowWidth @@ -1641,8 +1823,28 @@ function NativeBackend:build() end self.container = c AuraContainer.stats.builds = AuraContainer.stats.builds + 1 + -- Deafen to Blizzard's Edit Mode provider switch FIRST, before anything else can + -- fire (see EDIT-MODE DEAFENING). Every build gets its own fresh container, so + -- this has to be re-applied here rather than once. Inverted while OUR test mode + -- is on: that preview needs to hear the bounce. + setContainerProviderDeaf(c, not AuraContainer._testMode) local isOverlay = config.mode == "overlay" local isMissing = config.mode == "missing" + -- SINGLE-SLOT ROW. A row that can only ever show ONE icon declares an AuraSlot + -- instead of an AuraGroup: AddAuraGroup hardcodes batchSize = + -- CustomAuraContainerConstants.FrameCreationBatchSize and calls CreateFrameBatch() + -- BEFORE maxFrameCount is applied, so a max=1 group created a full batch of + -- buttons to display one icon. AddAuraSlot calls CreateAuraSlotFrame once. + -- Selection is NOT degraded: RegisterAuraSlot builds an auraComparator from + -- sortMethod/sortDirection, so the slot shows the same best-ranked match. + -- + -- ☠ OPT-IN ONLY, never inferred from max == 1. AD filter groups derive their max + -- from group.maxIcons, which is USER-TUNABLE in place via ApplyTuning — a group + -- that happened to sit at 1 would become slot-backed and a later 1 -> 4 edit + -- would silently fail, because a slot cannot grow and the tuning path has no + -- group to re-max. Keying off an explicit flag keeps container TOPOLOGY + -- independent of any tuning value. + local isSingleSlot = config.singleSlot and not isOverlay and not isMissing if isOverlay then c:SetAllPoints(handle.frame) -- overlay covers the host region elseif isMissing then @@ -1695,6 +1897,17 @@ function NativeBackend:build() -- keys are remembered so ApplyStyle can hot-apply per-group layout and ApplyTuning -- can hot-apply max/sort/candidateFilters (all live mutators). local filters = normalizeFilters(config.filter) + -- ☠ SINGLE-SLOT means exactly ONE slot. The declaration loop below runs per + -- filter record, and every slot it declares pins to the same corner -- so a + -- multi-record config would stack its buttons on top of each other where the + -- group path would have flowed them side by side. No current consumer can hit + -- this (poolFilter returns one string, which normalizeFilters turns into one + -- record), but the flag's name promises something the loop does not enforce. + -- Fall back to groups rather than render wrong: correct output, no saving. + if isSingleSlot and #filters ~= 1 then + DF:DebugWarn(DBG, "singleSlot config has %d filter records; using groups", #filters) + isSingleSlot = false + end local maxCount = handle:_slotCount() local groupLayout if isMissing then @@ -1729,7 +1942,9 @@ function NativeBackend:build() -- layouts on every restyle and would otherwise reset a scaled group's cell back to -- the shared size, so it needs to know which groups are scaled. self.groupStyles = {} - self.slotButtons = isOverlay and {} or nil -- overlay: key -> native slot button (consumer styling) + -- key -> native slot button (consumer styling). Overlay AND single-slot rows: + -- both declare AuraSlots, and both need the button reachable by key afterwards. + self.slotButtons = (isOverlay or isSingleSlot) and {} or nil handle._idGateVulnerable = nil -- re-derived from this build's records (see the record loop) handle._idGateSourceRelative = nil -- PLAYER-token / isFromPlayerOrPlayerPet pools (visibility gate) if testMode and not isOverlay and not isMissing then @@ -1763,16 +1978,36 @@ function NativeBackend:build() local testStyleSlots = testStyle and math.min(1, maxCount) or 0 local testStyleLayout = scaleGroupLayout(groupLayout, testStyle) filters = {} -- the normal declaration loop below is skipped - for k = 1, maxCount do - local key = "dfTest" .. k - local styled = (k <= testStyleSlots) or nil - local okGroup, err = pcall(function() - c:AddAuraGroup(key, category, { - maxFrameCount = 1, - initializeFrame = handle:_makeInitializeFrame(handle._gen, k, nil, styled and testStyle or nil), - layout = styled and testStyleLayout or groupLayout, -- groupSpacing = 0 (buildGroupLayout) = uniform spacing - }) - end) + -- TWO groups, not one per preview slot. Every AddAuraGroup eagerly creates + -- FrameCreationBatchSize (10) button frames — before maxFrameCount is even + -- applied — so one group per slot cost maxCount × 10 frames per container per + -- unit frame. Measured at 40 test frames that was the single largest allocation + -- anywhere in the addon: ~530 MB across four toggles, 65% of the test-mode + -- trace, and a visible ~1 s freeze on every toggle. + -- + -- The styled slot keeps its OWN group so declaration order still pins it to + -- position 1 — that was the point of the original split, one styled icon leading + -- a row of plain ones. Every remaining slot now shares a single group and + -- numbers itself via seqStart, so each still paints a distinct curated entry. + -- + -- ⚠ TRADE-OFF, and it is a real one: inside the shared group the flow assigns + -- auras to buttons in the CONTAINER's order, which is not creation order, so the + -- plain entries may appear in a different order than the curated pool lists + -- them. It is deterministic (test mode declares no sort, so the same samples + -- land the same way on every build) and every entry is still shown. The original + -- "Lightning Shield mid-row" failure this shape was built to prevent was about + -- the STYLED entry drifting, which declaration order still guarantees, and the + -- mismatched-tooltip half is moot now that tooltips are forced off in test mode. + local function addTestGroup(key, count, styled, seqStart) + if count <= 0 then return end + -- pcall(fn, args...) rather than pcall(function() ... end): no wrapper + -- closure. Protection is unchanged — AddAuraGroup asserts. + local okGroup, err = pcall(c.AddAuraGroup, c, key, category, { + maxFrameCount = count, + initializeFrame = handle:_makeInitializeFrame(handle._gen, + styled and 1 or nil, nil, styled and testStyle or nil, seqStart), + layout = styled and testStyleLayout or groupLayout, -- groupSpacing = 0 (buildGroupLayout) = uniform spacing + }) if okGroup then self.groupKeys[#self.groupKeys + 1] = key self.groupStyles[key] = styled and testStyle or nil @@ -1780,6 +2015,31 @@ function NativeBackend:build() DF:DebugWarn(DBG, "test group failed: %s", tostring(err)) end end + if isSingleSlot then + -- SINGLE-SLOT ROW preview. The two-group split above exists to pin a + -- styled icon AHEAD of plain ones in a multi-icon row; this row has + -- exactly one icon, so it declares the same single AuraSlot the live + -- path does and paints curated entry 1 into it. fixedIndex = 1 (not + -- the handle-wide counter) because there is only ever one button and + -- its paint must be deterministic across rebuilds. + local okSlot, btn = pcall(c.AddAuraSlot, c, "dfTestSlot", category, + { initializeFrame = handle:_makeInitializeFrame(handle._gen, 1, nil, testStyle) }) + if okSlot and btn then + -- Same strip-reservation fold as the live pin below: the preview + -- must sit where the live icon sits, and _positionTestTip already + -- applies this inset to the hover zone — without it the zone and + -- the icon would disagree by the reservation in test mode. + local fa = handle._flowAnchor or "TOPLEFT" + pcall(btn.ClearAllPoints, btn) + pcall(btn.SetPoint, btn, fa, c, fa, 0, handle._flowPadY or 0) + self.slotButtons["dfTestSlot"] = btn + elseif not okSlot then + DF:DebugWarn(DBG, "test slot failed: %s", tostring(btn)) + end + else + addTestGroup("dfTestStyled", testStyleSlots, true, nil) + addTestGroup("dfTestPlain", maxCount - testStyleSlots, false, testStyleSlots + 1) + end end for i, rec in ipairs(filters) do local f = rec.f @@ -1805,14 +2065,35 @@ function NativeBackend:build() -- dispel carriers) so it can create+bind SetAuraBorder in secure -- context; else the shared initFn. local slotInit = rec.onInit and handle:_makeInitializeFrame(handle._gen, nil, rec.onInit) or initFn - local okSlot, btn = pcall(function() - return c:AddAuraSlot(key, f, { initializeFrame = slotInit, candidateFilters = cf, - sortMethod = sortMethod, sortDirection = sortDirection }) - end) + local okSlot, btn = pcall(c.AddAuraSlot, c, key, f, + { initializeFrame = slotInit, candidateFilters = cf, + sortMethod = sortMethod, sortDirection = sortDirection }) if okSlot and btn then - pcall(function() btn:SetAllPoints(handle.frame) end) + pcall(btn.SetAllPoints, btn, handle.frame) self.slotButtons[key] = btn elseif not okSlot then DF:DebugWarn(DBG, "AddAuraSlot failed: %s", tostring(btn)) end + elseif isSingleSlot then + -- Same declaration as the overlay branch, different GEOMETRY: an + -- overlay button covers the whole host, a row button is icon-sized + -- and sits where the flow would have put element 1. + local slotInit = (rec.style or rec.onInit) + and handle:_makeInitializeFrame(handle._gen, nil, rec.onInit, rec.style) + or initFn + local okSlot, btn = pcall(c.AddAuraSlot, c, key, f, + { initializeFrame = slotInit, candidateFilters = cf, + sortMethod = sortMethod, sortDirection = sortDirection }) + if okSlot and btn then + -- Size is NOT set here: styleButton_regions sizes every button + -- from the shared config, group-backed or not, so the icon comes + -- out identical. Only the position the flow would have supplied + -- has to be replaced. + local fa = handle._flowAnchor or "TOPLEFT" + pcall(btn.ClearAllPoints, btn) + pcall(btn.SetPoint, btn, fa, c, fa, 0, handle._flowPadY or 0) + self.slotButtons[key] = btn + elseif not okSlot then + DF:DebugWarn(DBG, "AddAuraSlot (single-slot row) failed: %s", tostring(btn)) + end else -- A record carrying its own style (or an onInit) needs its OWN init -- closure — initFn is shared across every group and would apply the @@ -1823,11 +2104,10 @@ function NativeBackend:build() groupInit = handle:_makeInitializeFrame(handle._gen, nil, rec.onInit, rec.style) recLayout = scaleGroupLayout(groupLayout, rec.style) end - local okGroup, err = pcall(function() - c:AddAuraGroup(key, f, { maxFrameCount = maxCount, initializeFrame = groupInit, - layout = recLayout, candidateFilters = cf, - sortMethod = sortMethod, sortDirection = sortDirection }) - end) + local okGroup, err = pcall(c.AddAuraGroup, c, key, f, + { maxFrameCount = maxCount, initializeFrame = groupInit, + layout = recLayout, candidateFilters = cf, + sortMethod = sortMethod, sortDirection = sortDirection }) if okGroup then self.groupKeys[#self.groupKeys + 1] = key self.groupStyles[key] = rec.style @@ -1923,6 +2203,19 @@ function NativeBackend:applyLayout() -- hot re-apply here would overwrite them with row semantics. if not c or self.handle.config.mode == "overlay" or self.handle.config.mode == "missing" then return end applyContainerLayout(c, self.handle) + -- SINGLE-SLOT ROW: the flow does not move slot buttons, so the pin build() + -- made has to be re-made here or a growth/anchor change would leave the icon + -- at the old corner while the container moved under it. applyContainerLayout + -- has just refreshed handle._flowAnchor, so this reads the new corner. Size is + -- still styleButton_regions' job (ApplyStyle re-runs it right after this). + if self.slotButtons and self.handle.config.singleSlot then + local fa = self.handle._flowAnchor or "TOPLEFT" + local py = self.handle._flowPadY or 0 + for _, btn in pairs(self.slotButtons) do + pcall(btn.ClearAllPoints, btn) + pcall(btn.SetPoint, btn, fa, c, fa, 0, py) + end + end if self.groupKeys and c.SetAuraGroupLayout then local groupLayout = buildGroupLayout(self.handle.config) for _, key in ipairs(self.groupKeys) do @@ -1933,7 +2226,7 @@ function NativeBackend:applyLayout() -- because AddAuraGroup got the scaled layout there. Button size and reserved -- cell are separate things and both have to be re-pushed. local gl = scaleGroupLayout(groupLayout, self.groupStyles and self.groupStyles[key]) - pcall(function() c:SetAuraGroupLayout(key, gl) end) + pcall(c.SetAuraGroupLayout, c, key, gl) end end -- ★ PARTITION KICK (live-confirmed 2026-07-09): inbound mutators set the dirty @@ -1959,12 +2252,35 @@ end -- in combat. function NativeBackend:applyGroupTuning() local c = self.container - if not c or self.handle.config.mode == "overlay" or self.handle.config.mode == "missing" then return end - -- Test mode declares per-slot PIN groups (maxFrameCount = 1, curated paint) — - -- tuning them would break the slot pinning. Handle:ApplyTuning rebuilds the - -- preview instead, so this path is never reached in test mode; guard anyway. + if not c then return end + local mode = self.handle.config.mode + -- ☠ MISSING mode stays on the Rebuild path. Its layout-push inversion is + -- load-bearing and hard-won (the badge only clears the clip window because ONE + -- blank button's layout CELL pushes it out), and a live candidateFilters swap + -- changes exactly which buttons exist. Nothing has exercised that combination, so + -- it is left alone rather than enabled blind. + if mode == "missing" then return end + -- Test mode declares its own groups with curated paint stamped per button at create + -- (a styled group pinned first, then one shared plain group) — tuning them in place + -- would not re-stamp that paint. Handle:ApplyTuning rebuilds the preview instead, so + -- this path is never reached in test mode; guard anyway. if AuraContainer._testMode then return end - if not (self.groupKeys and c.SetAuraGroupMaxFrameCount) then return end + -- OVERLAY declares AuraSLOTs, not groups, so groupKeys is empty and the group + -- setters have nothing to act on — it needs the slot-side setters instead. The + -- cfByKey derivation below is shared: build() keys slots and groups identically + -- (rec.key or positional "df"), so the same map serves both. + -- SINGLE-SLOT ROWS declare AuraSlots too, so they take the slot-side setters for + -- exactly the same reason: groupKeys is empty and the group setters have nothing + -- to act on. ☠ Without this a spell-map edit on a placed indicator would silently + -- no-op — include/excludeSpellIDs ride the TUNING signature, not the struct one, + -- so they never re-enter build(), and the indicator would keep the old spell list + -- until some unrelated change happened to rebuild it. + local usesSlots = mode == "overlay" or self.handle.config.singleSlot + if usesSlots then + if not (self.slotButtons and c.SetAuraSlotCandidateFilters) then return end + elseif not (self.groupKeys and c.SetAuraGroupMaxFrameCount) then + return + end local config = self.handle.config local maxCount = self.handle:_slotCount() -- SetAuraGroupSortMethod validates BOTH args as enum members (nil asserts), so an @@ -2005,13 +2321,35 @@ function NativeBackend:applyGroupTuning() self.handle._idGateSourceRelative = true end end - for _, key in ipairs(self.groupKeys) do - pcall(function() c:SetAuraGroupMaxFrameCount(key, maxCount) end) - -- nil CLEARS: the inbound copy runs over an EMPTY defaults table, so a - -- toggled-off filter set doesn't survive (the old Rebuild-merge lesson). - pcall(function() c:SetAuraGroupCandidateFilters(key, cfByKey[key]) end) - if sortMethod ~= nil and sortDirection ~= nil then - pcall(function() c:SetAuraGroupSortMethod(key, sortMethod, sortDirection) end) + -- pcall(fn, args...) not pcall(function() ... end): the closure form allocated THREE + -- closures per group key per tuning pass. Protection is unchanged. + -- + -- Ordering note (Blizzard source): SetAuraGroupMaxFrameCount and + -- SetAuraGroupSortMethod only MarkDirty, and dirty flags coalesce into one + -- ProcessDirtyFlags on the next OnUpdate — so those are near-free however many + -- groups there are. SetAuraGroupCandidateFilters runs an immediate UpdateAllAuras + -- per call and has no equality guard of its own, so a container with N groups pays + -- N full updates here. There is no batch setter; the only real lever is declaring + -- fewer groups. + if usesSlots then + -- A slot is a single button: no maxFrameCount and no layout to push, so only + -- the candidate filters and the sort (which decides WHICH aura wins the one + -- slot) are tunable. Same nil-CLEARS semantics as the group setter. + for key in pairs(self.slotButtons) do + pcall(c.SetAuraSlotCandidateFilters, c, key, cfByKey[key]) + if sortMethod ~= nil and sortDirection ~= nil and c.SetAuraSlotSortMethod then + pcall(c.SetAuraSlotSortMethod, c, key, sortMethod, sortDirection) + end + end + else + for _, key in ipairs(self.groupKeys) do + pcall(c.SetAuraGroupMaxFrameCount, c, key, maxCount) + -- nil CLEARS: the inbound copy runs over an EMPTY defaults table, so a + -- toggled-off filter set doesn't survive (the old Rebuild-merge lesson). + pcall(c.SetAuraGroupCandidateFilters, c, key, cfByKey[key]) + if sortMethod ~= nil and sortDirection ~= nil then + pcall(c.SetAuraGroupSortMethod, c, key, sortMethod, sortDirection) + end end end -- ★ PARTITION KICK (same mechanism as applyLayout): the inbound mutators mark @@ -2721,8 +3059,16 @@ function applyRecordStyle(button, handle, recStyle) end end -function Handle:_makeInitializeFrame(gen, fixedIndex, onInit, recStyle) +-- seqStart: when set, this group numbers its OWN buttons sequentially from that base +-- rather than taking a fixed index or the handle-wide creation counter. That is what +-- lets a SINGLE test group paint a distinct curated entry per button. The handle-wide +-- counter cannot do it: Blizzard eagerly creates FrameCreationBatchSize frames per +-- group, so the shared counter runs far past the preview's slot range. Overshoot is +-- harmless either way — _paintTestSlot wraps the index modulo the pool size, and any +-- button past maxFrameCount is never displayed. +function Handle:_makeInitializeFrame(gen, fixedIndex, onInit, recStyle, seqStart) local handle = self + local seq = seqStart return function(button) local ok, err = pcall(function() if handle._destroyed or handle._gen ~= gen or not button then return end @@ -2759,8 +3105,15 @@ function Handle:_makeInitializeFrame(gen, fixedIndex, onInit, recStyle) -- fixedIndex = the per-slot test group's position (creation order -- is NOT layout order; the group key is) — stamped on the button -- so ApplyStyle repaints the same entry. - button._dfTestIndex = fixedIndex or i - handle:_paintTestSlot(button, button._dfTestIndex) + local testIndex + if seq then + testIndex = seq + seq = seq + 1 + else + testIndex = fixedIndex or i + end + button._dfTestIndex = testIndex + handle:_paintTestSlot(button, testIndex) else handle:_bindNativeSlot(button) -- native inbound setters -- Consumer secure init (overlay dispel carriers): runs in THIS @@ -3086,8 +3439,10 @@ function Handle:ApplyStyle(style, layout) -- Blizzard instantly overwrote the curated icons with the -- sample auras' random art and zero durations (static swipes) -- — until the next rebuild repainted them (Krathe 2026-07-10). - -- _dfTestIndex = the button's per-slot group position (creation - -- order ≠ layout order). + -- _dfTestIndex = the curated entry this button was stamped with at + -- create (fixed for the styled group, sequential within the shared + -- plain group). Repaint the SAME entry — creation order is not + -- layout order, so recomputing it here would reshuffle the preview. self:_paintTestSlot(b, b._dfTestIndex or i) else bindNative(b, self.config) @@ -3441,49 +3796,263 @@ end -- ============================================================ -- EDIT-MODE GUARD (shared) --- Blizzard Edit Mode swaps the ENTIRE aura data layer: AURA_DATA_PROVIDER_SWITCH --- installs the edit-mode sample provider at the AuraUtil level (AuraUtil.lua:19, --- AuraUtilDataProvider replaces C_UnitAuras wholesale), so every container in the --- game re-parses to random-icon fake auras. Per-container opt-out is IMPOSSIBLE — --- deafening a container to the event was live-disproved (DF_AuraLab probe 34's --- deaf twin still flipped: the swap is below the container layer). So while a --- FOREIGN switch is active we HIDE the factory rows (plain anchor frames — the --- drives' shown-caches keep them from re-showing), and restore + refresh when the --- real provider returns. DF's own test mode (P5) sets _ownsProviderSwitch around --- its switches so its curated preview is exempt from the guard. +-- ============================================================ +-- Blizzard Edit Mode flips every aura container onto its sample-data source. We +-- cannot stop OUR containers hearing that switch -- the container carries the +-- EventRegistrations forbidden aspect from birth, so UnregisterEvent is refused +-- (see EDIT-MODE DEAFENING near the top of this file). But we do not need to. +-- +-- ★ PRIMARY: hand the real provider straight back. C_UnitAuras.ResetAuraDataProvider +-- is public and unrestricted (no HasRestrictions in the generated docs; test mode +-- already drives it), and it re-fires the switch with useRealDataProvider = true, so +-- every container -- ours and Blizzard's -- returns to the real source. No rebuild, +-- no teardown, no button pools recreated. The restore is just the real-switch branch. +-- +-- THE TRADE: this is GLOBAL. Blizzard's own Edit Mode preview loses its sample +-- auras, so their buff frame and cooldown manager show your REAL auras while you +-- position them. That is the same class of trade DF test mode already makes in the +-- other direction, and the alternative is our own rows flashing. +-- +-- ORDER, and why each step is where it is: +-- 1. PARK synchronously, inside this dispatch. AURA_DATA_PROVIDER_SWITCH is a +-- SYNCHRONOUS event, so nothing is drawn between the containers' handler and +-- ours whichever order they run in -- parking here means the sample icons are +-- never painted at all. +-- 2. RESET one frame later, never inline. An inline reset would nest a dispatch +-- inside this one, and any container the OUTER fake dispatch had not yet +-- visited would receive fake AFTER our reset and be stranded on the sample +-- source. A frame later the outer dispatch has finished and they all flip +-- together. +-- 3. Only if the reset is unavailable or fails, fall back to REBIRTH: a container +-- built after a switch never receives it and useEditModeSource initialises +-- false, so a fresh container is on the real source by construction (probe 33's +-- born-deaf finding, which test mode's ordering already depends on). +-- +-- DF's own test mode (P5) sets _ownsProviderSwitch around its switches so its +-- curated preview is exempt from all of this. local function ensureProviderWatch() if AuraContainer._providerWatch then return end local f = CreateFrame("Frame") AuraContainer._providerWatch = f f._hidden = setmetatable({}, { __mode = "k" }) f._fakeActive = false - f:RegisterEvent("AURA_DATA_PROVIDER_SWITCH") + + -- Hide every currently-shown row and remember which ones we hid. + local function parkAll(watch) + watch._fakeActive = true + for h in pairs(AuraContainer._handles or {}) do + if not h._destroyed and h:GetFrame():IsShown() then + watch._hidden[h] = true + safeHideWindow(h:GetFrame(), function() return watch._hidden[h] end) + end + end + end + + -- Clear the park BEFORE any restore: _applyVisibility forces a handle hidden + -- while _fakeActive and _hidden[h] both hold, so restoring first strands them. + local function unpark(watch) + watch._fakeActive = false + for h in pairs(watch._hidden) do watch._hidden[h] = nil end + end + + -- ★ STRANDING SWEEP — the belt to the inline reset's braces. + -- + -- The inline reset leaves one hole, and it is the only way Edit Mode's sample icons + -- can still reach a LIVE frame: if the outer fake dispatch had not yet visited some + -- container when our reset ran, that container takes fake AFTER the reset and sits + -- on the sample source until Edit Mode closes. Measured as not happening on 68914, + -- but that rests on an undocumented dispatch order, and this ships to configurations + -- we will never test. + -- + -- The sweep removes the need to reason about order at all: bounce the provider + -- fake→real BACK TO BACK in one frame. Every container hears both, in that order, + -- whatever order it is visited in, and ends on the real source. Nothing paints + -- between two consecutive Lua statements. + -- + -- It is near-free: UpdateAllAuras is MarkDirty(FullAuraRebuild), a bit-set, and + -- ProcessDirtyFlags runs once on the next OnUpdate — so two marks in one frame + -- produce ONE reparse, the same one the reset already scheduled. + -- + -- ☠ THE DANGEROUS HALF: if Switch succeeds and Reset does not, the whole GAME is + -- left on the fake provider — every aura display, ours and Blizzard's, showing + -- sample icons. So Reset is retried hard, and as a last resort re-armed on a timer. + -- Reset is known to work by the time we get here (the inline reset used it moments + -- ago), which is why the bounce is safe to attempt at all. + local function sweepStranded() + -- Only NESTED dispatch can strand: QUEUED means our reset landed after the + -- outer dispatch finished, so every container had already taken fake first. + if AuraContainer._inlineDispatch ~= "NESTED" then return end + if AuraContainer._ownsProviderSwitch then return end + local switch = C_UnitAuras and C_UnitAuras.SwitchAuraDataProvider + local reset = C_UnitAuras and C_UnitAuras.ResetAuraDataProvider + if not (switch and reset) then return end + + -- Our own handler must ignore both halves; reuse the flag it already honours. + AuraContainer._ownsProviderSwitch = true + local okS = pcall(switch) + local okR = pcall(reset) + if okS and not okR then + for _ = 1, 3 do + if pcall(reset) then okR = true; break end + end + end + AuraContainer._ownsProviderSwitch = false + + if okS and not okR then + -- Worst case: the world is on fake data and we could not undo it. Keep + -- trying rather than leaving it; a stuck retry is recoverable, a stuck + -- fake provider is not. + DF:DebugWarn(DBG, "stranding sweep left the fake provider installed — retrying") + local function retry(n) + if pcall(reset) or n <= 0 then return end + C_Timer.After(0.25, function() retry(n - 1) end) + end + retry(20) + elseif not okS then + -- Switch refused, so nothing was changed and nothing needs undoing. + DF:DebugWarn(DBG, "stranding sweep skipped: SwitchAuraDataProvider refused") + end + end + + -- LAST RESORT (see step 3 above). Two passes: a raid's worth of teardown + + -- CreateFrame + AddAuraGroup in one frame is itself a visible stall, so rebuild + -- what is ON SCREEN now and drain the rest 25 per tick. Offscreen handles stay + -- flipped for those few frames, which costs nothing because nothing draws them. + local function rebirthAll(watch) + unpark(watch) + local function rebirth(h) + if h._destroyed then return end + pcall(function() h:_rebuild() end) -- reborn on the real source + pcall(function() h:_applyVisibility() end) -- and back on screen + end + local later, n = {}, 0 + for h in pairs(AuraContainer._handles or {}) do + if not h._destroyed then + if h:GetFrame():IsShown() then + rebirth(h) + else + n = n + 1 + later[n] = h + end + end + end + if n == 0 then return end + local i = 1 + local function drain() + if AuraContainer._ownsProviderSwitch then return end + local stop = math.min(i + 24, n) + while i <= stop do rebirth(later[i]); i = i + 1 end + if i <= n then C_Timer.After(0, drain) end + end + C_Timer.After(0, drain) + end + + f:RegisterEvent(PROVIDER_EVENT) f:SetScript("OnEvent", function(self, _, useRealDataProvider) if AuraContainer._ownsProviderSwitch then self._fakeActive = false -- P5's own preview manages its rows itself return end + if useRealDataProvider then self._fakeActive = false for h in pairs(self._hidden) do self._hidden[h] = nil if not h._destroyed then - -- Restore through the visibility channel: hover-safe, and - -- composes intent + identity gate (a bare Show() re-opened - -- gate-hidden windows on Edit Mode exit). + -- Restore through the visibility channel: hover-safe, and composes + -- intent + identity gate (a bare Show() re-opened gate-hidden + -- windows on Edit Mode exit). + -- + -- ⚠ NO Refresh() here. It used to bounce every container + -- (NativeBackend:refresh = Hide+Show), and that bounce was most of + -- the remaining visible delay: N containers each re-registering and + -- re-laying-out. It is redundant twice over — + -- * the real switch already ran SetUseEditModeSource(false) on + -- every container, and that calls UpdateAllAuras itself; and + -- * showing the parent fires the container's OnShow_Intrinsic, + -- which does UpdateEventRegistrations + UpdateAllAuras anyway. + -- The second also closes the only gap worth worrying about: while + -- parked the frame is hidden, so the container drops its UNIT_AURA + -- registration (ShouldRegisterForDynamicEvents = IsVisible and + -- IsEnabled) and would otherwise miss changes made during the park. h:_applyVisibility() - h:Refresh() -- re-parse real data (Edit Mode exit is OOC) - end - end - else - self._fakeActive = true - for h in pairs(AuraContainer._handles or {}) do - if not h._destroyed and h:GetFrame():IsShown() then - self._hidden[h] = true - safeHideWindow(h:GetFrame(), function() return self._hidden[h] end) end end + return + end + + if AuraContainer._providerDeafOK then + -- A future build that permits the unregister: our containers never heard + -- this switch and are still on the real source. Nothing to do. + self._fakeActive = false + return + end + + parkAll(self) + + -- ⚗ EXPERIMENT (INLINE_PROVIDER_RESET): reset from INSIDE this dispatch rather + -- than a frame later. Parking and restoring then both happen before anything is + -- drawn, so the remaining sub-50ms blip disappears entirely. + -- + -- ✅ VERIFIED IN GAME on 68914: NESTED, and NOTHING stranded. Confirmed from the + -- saved debug log, which also shows neither fallback firing — no reset failure, + -- no rebirth. One dispatch, no visible artifact. + -- + -- ⚠ But note WHY that is luck rather than design. The stranding hazard depends on + -- where our watch frame sits in the dispatch order, and the obvious prediction was + -- wrong: the watch registers before any container exists (ensureProviderWatch runs + -- ahead of the first _build), so registration order would have put it FIRST and + -- stranded everything. It stranded nothing, so dispatch order is NOT registration + -- order — and the real rule is undocumented. Treat this as empirical, not sound. + -- If it ever regresses the symptom is rows of random spellbook icons during Edit + -- Mode, cleared on exit (Edit Mode's own reset flips every container back), so it + -- is self-limiting: no error, no data loss, one toggle to clear. + -- + -- Whether that is safe depends on undocumented dispatch semantics, and the + -- outcome is self-reporting — the debug line below says which the client does: + -- * QUEUED — pcall returns with _fakeActive still set, the real switch lands + -- later. Identical to the deferred path; safe, no gain. + -- * NESTED — the real switch is dispatched re-entrantly, our restore has + -- already run, _fakeActive is clear. Zero blip — BUT any container + -- the OUTER fake dispatch had not yet visited receives fake AFTER + -- our reset and is stranded on the sample source until Edit Mode + -- is toggled again. Visible as rows of random spellbook icons. + -- If you see stranded rows, flip this to false; the deferred path below is + -- unchanged and known good. + local INLINE_PROVIDER_RESET = true + local resetNow = C_UnitAuras and C_UnitAuras.ResetAuraDataProvider + if INLINE_PROVIDER_RESET and resetNow and pcall(resetNow) then + -- Recorded, not just logged: which of the two it is decides whether the + -- inline reset is worth keeping, and it is not inferable from "it looked + -- fine" — one imperceptible frame and zero frames look identical. + AuraContainer._inlineDispatch = self._fakeActive and "QUEUED" or "NESTED" + DF:Debug(DBG, "inline provider reset: dispatch was %s", + self._fakeActive and "QUEUED (no gain, safe)" or "NESTED (zero blip; watch for stranded rows)") + -- Deliberately NOT returning here. If the dispatch was NESTED the restore has + -- already run and the backstop below sees _fakeActive clear and no-ops. If it + -- was QUEUED the real switch is still in flight — and if it somehow never + -- arrives, the backstop is the only thing standing between us and rows parked + -- forever. It costs one no-op timer. end + + C_Timer.After(0, function() + if AuraContainer._ownsProviderSwitch then return end + if not self._fakeActive then + -- Resolved inline. Sweep anyway: that path is the one that can leave a + -- container stranded on the sample source, and this is where we stop + -- depending on dispatch order to have been kind to us. + sweepStranded() + return + end + local reset = C_UnitAuras and C_UnitAuras.ResetAuraDataProvider + if reset and pcall(reset) then return end -- the real-switch branch restores + DF:DebugWarn(DBG, "ResetAuraDataProvider unavailable/failed; rebuilding instead") + -- Containers cannot be stood up in lockdown; stay parked until the real + -- switch arrives (Edit Mode cannot be opened in combat anyway -- this only + -- covers another addon switching the provider mid-fight). + if InCombatLockdown() then return end + rebirthAll(self) + end) end) end @@ -3550,6 +4119,20 @@ function AuraContainer:Create(parent, config) -- h.frame is the plain anchor frame DF positions; the backend parents its OWN -- CustomAuraContainer to it (one container per consumer). h.frame = CreateFrame("Frame", nil, parent) + -- TEST-FRAME PROVENANCE, resolved ONCE here rather than per toggle. Only a + -- handle that will actually render the preview needs SetTestMode's rebuild; + -- see rebuildAll for why the live ones never do. Resolving at Create is safe + -- because every test frame carries dfIsTestFrame from its OWN creation -- + -- TestFramePool.lua stamps it on the pool frames and CreatePlayerTestFrame on + -- the pinned ones, both long before any consumer drives a row onto them. + -- Bounded walk: the anchor's parent is usually the unit frame, but AD hangs + -- containers off the healthBar / background anchor / aura-bar strip instead. + local ancestor = parent + for _ = 1, 6 do + if not ancestor then break end + if ancestor.dfIsTestFrame then h._testFrame = true break end + ancestor = ancestor.GetParent and ancestor:GetParent() or nil + end if cfg.mode == "missing" then -- MISSING mode (probe 32, live-confirmed 2026-07-10): h.frame is a CLIP WINDOW -- exactly the badge's size — the caller positions it. The backend pins its @@ -3615,8 +4198,9 @@ function AuraContainer:Create(parent, config) else h:_build() end - -- Born during a foreign fake-data period (e.g. roster change while the user - -- sits in Edit Mode): start hidden like the rest, restored on the real switch. + -- FALLBACK PATH ONLY (_fakeActive never latches once deafening is confirmed): + -- born during a foreign fake-data period (e.g. roster change while the user + -- sits in Edit Mode), start hidden like the rest, restored on the real switch. local watch = AuraContainer._providerWatch if watch and watch._fakeActive then watch._hidden[h] = true diff --git a/DandersFrames/Frames/Bars.lua b/DandersFrames/Frames/Bars.lua index 01e22243..f2615792 100644 --- a/DandersFrames/Frames/Bars.lua +++ b/DandersFrames/Frames/Bars.lua @@ -33,7 +33,15 @@ function DF:ShouldShowResourceBar(unit, db) local hasAnyRoleFilter = db.resourceBarShowHealer or db.resourceBarShowTank or db.resourceBarShowDPS if hasAnyRoleFilter then - local role = UnitGroupRolesAssigned(unit) + -- ☠ DF:GetUnitRole, not UnitGroupRolesAssigned. The raw call returns + -- "NONE" for the player whenever the group has not assigned a role -- + -- solo, open world, and inside a delve until a spec change forces one -- + -- and the NONE arm below reads as DAMAGER. So a solo Holy Paladin was + -- gated by the DPS toggle: the Healers checkbox did nothing, and ticking + -- DPS showed the bar for every spec. GetUnitRole falls the PLAYER back to + -- the spec role; other units have no public spec API and still arrive + -- NONE, which is why the arm stays. + local role = DF:GetUnitRole(unit) local inSoloMode = not IsInGroup() and not IsInRaid() if inSoloMode and db.resourceBarShowInSoloMode then @@ -45,7 +53,7 @@ function DF:ShouldShowResourceBar(unit, db) elseif role == "DAMAGER" then roleAllowed = db.resourceBarShowDPS == true elseif not role or role == "NONE" then - -- Unassigned role (e.g. delves) — treat as DPS + -- Still unresolved: another unit with no assigned role. Treat as DPS. roleAllowed = db.resourceBarShowDPS == true end else @@ -2249,6 +2257,31 @@ function DF:UpdateName(frame) -- consistency with the other live text hooks. end +-- The role the role ICON should display for `unit` -- may be nil or "NONE", +-- both meaning "show nothing". +-- +-- ☠ THE GROUP TEST IS THE POINT, not a cheap guard. In a group the icon answers +-- "what is this unit here to do", so the player's spec role is the right answer +-- when the group has assigned none -- the case throughout a delve, where Brann +-- makes IsInGroup() true but no role is ever assigned, so the icon stayed hidden +-- until a spec change forced one. Solo, that question is not being asked at all: +-- resolving there would put a permanent role icon on your own frame in the open +-- world, where there has never been one. Hence group-only, deliberately NOT the +-- same policy as the resource bar, which wants the spec role everywhere. +-- +-- ☠ ProcessRoleUpdate's dirty check MUST call this, not UnitGroupRolesAssigned. +-- That cache decides whether UpdateRoleIcon runs at all, so keying it on the raw +-- value while the icon displays a resolved one makes the resolution unreachable: +-- across a whole delve the raw value never leaves "NONE", the cache sees no +-- change, and the icon is never asked to redraw. +function DF:GetRoleIconRole(unit) + if not unit then return nil end + if IsInGroup() or IsInRaid() then + return DF:GetUnitRole(unit) + end + return UnitGroupRolesAssigned(unit) +end + function DF:UpdateRoleIcon(frame, source) if DF.RosterDebugCount then DF:RosterDebugCount("UpdateRoleIcon") @@ -2268,12 +2301,12 @@ function DF:UpdateRoleIcon(frame, source) -- Use raid DB for raid frames, party DB for party frames local db = DF:GetFrameDB(frame) - local role = UnitGroupRolesAssigned(frame.unit) - + local role = DF:GetRoleIconRole(frame.unit) + -- Use our tracked combat state (set by PLAYER_REGEN events) local inCombat = DF.playerInCombat or false - - if role == "NONE" then + + if not role or role == "NONE" then frame.roleIcon:Hide() return end diff --git a/DandersFrames/Frames/Border.lua b/DandersFrames/Frames/Border.lua index 185aba5d..516a03a4 100644 --- a/DandersFrames/Frames/Border.lua +++ b/DandersFrames/Frames/Border.lua @@ -135,6 +135,15 @@ function Border:New(parent, opts) return border end +-- Shared read-only fallbacks for `readColor(anim.color or )`. Seven sites +-- built one of these two tables inline, so an animation whose colour is unset +-- allocated a throwaway table on every driver tick. readColor only ever reads its +-- argument, and nothing downstream retains it, so one instance each is safe. +-- (ANIM_GOLD is the shared default for the border effects; ANIM_WHITE keeps the +-- proc atlas's own art untinted -- see the desaturate note in setupProcGlow.) +local ANIM_GOLD = { r = 0.95, g = 0.95, b = 0.32, a = 1 } +local ANIM_WHITE = { r = 1, g = 1, b = 1, a = 1 } + -- Resolve a colour from either an array {r,g,b,a} or a keyed {r=,g=,b=,a=} -- table, so consumers can pass whichever they already store. local function readColor(color) @@ -161,9 +170,37 @@ end -- Resolvers silently fall through when their required ctx is missing, so a -- consumer that only knows the unit can still flip on classColor without -- worrying about time/type ctx. +-- Memoised db-key strings, per prefix. BuildSpec calls k() 37 times and each call +-- used to concatenate a fresh string -- 37 allocations per BuildSpec, on a function +-- that shows up in both the steady-state traces (2.7% of a boss run) and the rebuild +-- traces (2.6%). The prefix set is tiny and fixed, and prefix .. suffix is +-- deterministic, so the built keys are cached and reused forever. +local borderKeyMemo = {} + +-- Key builder for BuildSpec, hoisted OUT of it so there is no closure per call. +-- 395af09f killed the 37 string concats but left one allocation behind: `k` +-- captured memo and prefix, so a fresh closure was built on every call -- and +-- BuildSpec runs per bordered element per tick (4.6% of trash allocation, 4.3% +-- of boss). +-- +-- ☠ THE UPVALUES ARE SHARED, which is safe ONLY because BuildSpec cannot +-- re-enter: its body reaches DF:GetClassColor, DF:GetTestUnitData, DF:GetUnitRole +-- and the Border:Resolve* helpers, and none of those calls BuildSpec. Verified +-- against every caller, not assumed. If a resolver ever needs to build a spec it +-- must not do it through here, or this silently starts reading another prefix's +-- keys -- a wrong-border bug with nothing at the call site to explain it. +local bsMemo, bsPrefix +local function k(suffix) + local key = bsMemo[suffix] + if not key then key = bsPrefix .. suffix; bsMemo[suffix] = key end + return key +end + function Border:BuildSpec(dbTable, prefix, ctx) if not dbTable or not prefix then return {} end - local function k(suffix) return prefix .. suffix end + local memo = borderKeyMemo[prefix] + if not memo then memo = {}; borderKeyMemo[prefix] = memo end + bsMemo, bsPrefix = memo, prefix -- Style is the top-level choice: SOLID | GRADIENT | TEXTURE. -- GRADIENT owns its own colours (start/end pickers) so the colour-source @@ -362,18 +399,10 @@ function Border:ResolveRoleColor(unit, fallback, roleColors, frame) if frame and frame.dfIsTestFrame then local testData = DF.GetTestUnitData and DF:GetTestUnitData(frame.index, frame.isRaidFrame) role = testData and testData.role - elseif unit and UnitExists and UnitExists(unit) and UnitGroupRolesAssigned then - role = UnitGroupRolesAssigned(unit) - -- UnitGroupRolesAssigned returns "NONE" outside instances where roles - -- aren't assigned (solo, world content, open-world groups). For the - -- player, fall back to the spec role so role colour is meaningful - -- regardless of group context. Other units expose no public spec API, - -- so they stay on the picker fallback when role is NONE. - if (not role or role == "NONE") and UnitIsUnit and UnitIsUnit(unit, "player") - and GetSpecialization and GetSpecializationRole then - local spec = GetSpecialization() - if spec then role = GetSpecializationRole(spec) end - end + else + -- Player falls back to the spec role when the group assigned none; + -- other units stay NONE and drop to the picker fallback below. + role = DF:GetUnitRole(unit) end local c = role and role ~= "NONE" and (roleColors[role] or roleColors[string.lower(role)]) @@ -825,7 +854,7 @@ local function setupAnimOverlay(border, anim) o.right:SetPoint("BOTTOMLEFT", rect, "BOTTOMRIGHT", 0, -th) o.right:SetWidth(th) - local r, g, b, a = readColor(anim.color or { r = 0.95, g = 0.95, b = 0.32, a = 1 }) + local r, g, b, a = readColor(anim.color or ANIM_GOLD) for _, e in ipairs({ o.top, o.bottom, o.left, o.right }) do e:SetColorTexture(r, g, b, a) e:SetAlpha(0) -- tick functions raise alpha as the effect plays @@ -923,7 +952,7 @@ local function setupOrbitParticles(border, anim) if N > 16 then N = 16 end local total = N * 4 local scale = anim.scale or 1 - local r, g, b, a = readColor(anim.color or { r = 0.95, g = 0.95, b = 0.32, a = 1 }) + local r, g, b, a = readColor(anim.color or ANIM_GOLD) border.orbitTex = border.orbitTex or {} local tex = border.orbitTex for i = 1, total do @@ -1134,7 +1163,7 @@ local function setupPixelParticles(border, anim) if N > 16 then N = 16 end local th = anim.thickness or 2; if th < 1 then th = 1 end local len = anim.length or 6; if len < 1 then len = 1 end - local r, g, b, a = readColor(anim.color or { r = 0.95, g = 0.95, b = 0.32, a = 1 }) + local r, g, b, a = readColor(anim.color or ANIM_GOLD) border.pixelTex = border.pixelTex or {} local tex = border.pixelTex for i = 1, N do @@ -1243,16 +1272,45 @@ local function hideProcGlow(border) if border.procStartTex then border.procStartTex:Hide() end end +-- ☠ C_Texture.GetAtlasInfo RETURNS A FRESH TABLE ON EVERY CALL, and setupProcGlow +-- asked for two of them per call — on a path that runs per proc-glowing element +-- (3.0% of trash allocation, 5.6% of boss). Both atlases are compile-time +-- constants, so their info is fetched once and shared. +-- +-- Safe to share: every consumer only READS (.file, and stepProcFlipbook's frame +-- maths) — swept _procAtlas / _procStartAtlas for writes and there are none +-- beyond these two assignments. +-- +-- Resolved lazily rather than at file scope: C_Texture may not be ready when this +-- file loads, and the old code re-checked it on every call. +local procAtlasInfo, procStartAtlasInfo, procAtlasResolved + local function setupProcGlow(border, anim) local host = ensureAnimRect(border, anim.inset, anim.offsetX, anim.offsetY) border._procHost = host - local getInfo = C_Texture and C_Texture.GetAtlasInfo - border._procAtlas = getInfo and getInfo(PROC_ATLAS) - border._procStartAtlas = getInfo and getInfo(PROC_START_ATLAS) + if not procAtlasResolved then + local getInfo = C_Texture and C_Texture.GetAtlasInfo + if getInfo then + procAtlasInfo = getInfo(PROC_ATLAS) + procStartAtlasInfo = getInfo(PROC_START_ATLAS) + -- Only latch once the API actually answered; a nil result before the + -- texture system is up would otherwise cache "no atlas" permanently + -- and the proc glow would never draw for the rest of the session. + -- ☠ BOTH atlases, not just the loop one. Latching on procAtlasInfo + -- alone let a single nil START result freeze in permanently: the loop + -- glow drew normally while the intro burst was silently dead on every + -- border for the rest of the session, which reads as a design change + -- rather than a fault. The pre-memo code re-fetched both on every + -- setup, so it always recovered on the next one. + procAtlasResolved = (procAtlasInfo ~= nil) and (procStartAtlasInfo ~= nil) + end + end + border._procAtlas = procAtlasInfo + border._procStartAtlas = procStartAtlasInfo -- Colour handling: white keeps the atlas's native golden gradient; any other -- colour DESATURATES the art first so the tint reads clean (multiplying a -- strong colour over gold goes muddy). - local r, g, b, a = readColor(anim.color or { r = 1, g = 1, b = 1, a = 1 }) + local r, g, b, a = readColor(anim.color or ANIM_WHITE) local desat = not (r > 0.985 and g > 0.985 and b > 0.985) -- LCG-style hand-off: the LOOP fills the icon, while the intro BURST is a -- larger, CENTERED texture whose flipbook art contracts down onto the loop by @@ -1383,7 +1441,7 @@ local function setupFlashGlow(border, anim) border._flashHost = host -- Colour handling mirrors DF Proc: white keeps the native golden art; any -- other colour desaturates first so the tint reads clean. - local r, g, b, a = readColor(anim.color or { r = 1, g = 1, b = 1, a = 1 }) + local r, g, b, a = readColor(anim.color or ANIM_WHITE) local desat = not (r > 0.985 and g > 0.985 and b > 0.985) local spark = flashSheetTexture(border, "flashSpark", "BACKGROUND", 0, FLASH_UV_SPARK) local inner = flashSheetTexture(border, "flashInner", "ARTWORK", 0, FLASH_UV_GLOW) @@ -1559,7 +1617,7 @@ local function applyCornersOnly(border, anim) if not length or length <= 0 then length = 8 end local rect = ensureAnimRect(border, anim.inset, anim.offsetX, anim.offsetY) - local r, g, b, a = readColor(anim.color or { r = 0.95, g = 0.95, b = 0.32, a = 1 }) + local r, g, b, a = readColor(anim.color or ANIM_GOLD) local function paint(e) e:SetColorTexture(r, g, b, a) e:SetAlpha(1) @@ -1773,7 +1831,7 @@ function Border:StartAnimation(border, spec) -- (0 = static "dashed"). Fixed dash length/gap; thickness / inset / colour -- from the spec. Dashes clip per edge so the pattern flows around the corners. if anim.type == "DF_DASH" then - local r, g, b, a = readColor(anim.color or { r = 0.95, g = 0.95, b = 0.32, a = 1 }) + local r, g, b, a = readColor(anim.color or ANIM_GOLD) border._dfDashTh = math.max(1, anim.thickness or 2) border._dfDashInset = anim.inset or 0 border._dfDashR, border._dfDashG, border._dfDashB, border._dfDashA = r, g, b, a diff --git a/DandersFrames/Frames/Headers.lua b/DandersFrames/Frames/Headers.lua index 1916d75c..42629930 100755 --- a/DandersFrames/Frames/Headers.lua +++ b/DandersFrames/Frames/Headers.lua @@ -7596,7 +7596,11 @@ function DF:ProcessRoleUpdate() end local unit = frame.unit - local newRole = UnitGroupRolesAssigned(unit) + -- Must match what UpdateRoleIcon will DISPLAY, not the raw assignment -- + -- see the note on DF:GetRoleIconRole. Keyed on the raw value, this cache + -- never fires inside a delve (the raw role stays "NONE" start to finish) + -- and the icon never gets a chance to draw the spec-derived role. + local newRole = DF:GetRoleIconRole(unit) local oldRole = unitRoleCache[unit] -- Only update if role actually changed (dirty-check pattern) diff --git a/DandersFrames/Frames/Init.lua b/DandersFrames/Frames/Init.lua index 85ae4352..b8494ed6 100644 --- a/DandersFrames/Frames/Init.lua +++ b/DandersFrames/Frames/Init.lua @@ -997,12 +997,12 @@ function DF:UnlockRaidFrames() end end - -- Remember whether raid test mode was already active before this unlock cycle. - -- LockRaidFrames reads this to decide whether to keep or hide test frames. - DF.raidTestModeBeforeUnlock = DF.raidTestMode - - -- Enable raid test mode using the proper function - DF:ShowRaidTestFrames() + -- Unlock claims test frames for as long as it stays unlocked: you need frames + -- on screen to have something to drag. It deliberately does NOT snapshot what + -- the user had -- that is the user's own claim, tracked separately. The old + -- snapshot went stale whenever test mode changed mid-session, in both + -- directions (see TestMode/Shim.lua). + DF:SetTestModeOwner("raid", "unlock", true, true) -- silent: unlock announces itself -- Pinned frames ride the global lock: show their drag chrome too. No mode -- gate — SetMoversShown shows the right handles (live movers for the current @@ -1073,12 +1073,9 @@ function DF:LockRaidFrames() DF.PinnedFrames:SetMoversShown(false) end - -- Only disable raid test mode if it was not already active before the last unlock. - -- Preserves the user's test mode state across the lock/unlock cycle. - if not DF.raidTestModeBeforeUnlock then - DF:HideRaidTestFrames() - end - DF.raidTestModeBeforeUnlock = nil + -- Release unlock's claim. If the user still wants a preview it stays up; if + -- nobody does, this is what hides it. Nothing to go stale either way. + DF:SetTestModeOwner("raid", "unlock", false, true) -- silent: lock announces itself -- Hide container if not in raid if not IsInRaid() then diff --git a/DandersFrames/Frames/Position.lua b/DandersFrames/Frames/Position.lua index c30165ed..247b140d 100644 --- a/DandersFrames/Frames/Position.lua +++ b/DandersFrames/Frames/Position.lua @@ -514,8 +514,7 @@ function DF:CreatePermanentMoverPopup() -- Apply GUI scale popup:SetScript("OnShow", function(self) - local guiScale = DF.db and DF.db.party and DF.db.party.guiScale or 1.0 - self:SetScale(guiScale) + self:SetScale(DF:GetWindowState().scale or 1.0) self.closer:Show() end) @@ -1569,8 +1568,7 @@ function DF:CreatePositionPanel() -- Apply scale from settings when shown panel:SetScript("OnShow", function(self) - local guiScale = DF.db and DF.db.party and DF.db.party.guiScale or 1.0 - self:SetScale(guiScale) + self:SetScale(DF:GetWindowState().scale or 1.0) end) -- Store for theme updates @@ -2486,12 +2484,12 @@ function DF:UnlockFrames() DF.displayLockButton.Text:SetText(L["Lock Frames"]) end - -- Remember whether test mode was already active before this unlock cycle. - -- LockFrames reads this to decide whether to keep or hide test frames. - DF.partyTestModeBeforeUnlock = DF.testMode - - -- Enable test mode so user can position with full group visible - DF:ShowTestFrames(true) + -- Unlock claims test frames for as long as it stays unlocked, so there is a + -- full group to position against. It deliberately does NOT snapshot what the + -- user had -- that is the user's own claim, tracked separately. The old + -- snapshot went stale whenever test mode changed mid-session, in both + -- directions (see TestMode/Shim.lua). + DF:SetTestModeOwner("party", "unlock", true, true) -- silent: unlock announces itself -- Sync GUI toolbar buttons if DF.GUI then @@ -2561,13 +2559,59 @@ function DF:LockFrames() if DF.GUI.UpdateTestButtonState then DF.GUI.UpdateTestButtonState() end end - -- Only disable test mode if it was not already active before the last unlock. - -- Preserves the user's test mode state across the lock/unlock cycle. - if not DF.partyTestModeBeforeUnlock then - DF:HideTestFrames(true) - end - DF.partyTestModeBeforeUnlock = nil + -- Release unlock's claim. If the user still wants a preview it stays up; if + -- nobody does, this is what hides it. Nothing to go stale either way. + DF:SetTestModeOwner("party", "unlock", false, true) -- silent: lock announces itself DF:Say(L["Frames locked."]) end +-- ============================================================ +-- BLIZZARD EDIT MODE — stand down while it is open +-- ============================================================ +-- Entering /edit left DF half-dressed. The game closes DandersFramesGUI and +-- DandersFramesTestPanel for us -- both are in UISpecialFrames -- but the position +-- panel is NOT, so it stayed up, and behind it the grid, the movers and the test +-- preview all kept running: two grids overlapping and fake party frames sitting on +-- top of Blizzard's own editor. Field-reported. +-- +-- The actual gap is that DF had no Edit Mode integration at all; the two windows +-- that did close were closing by accident of UISpecialFrames membership. +-- +-- Locking is the right response and needs nothing new: LockFrames/LockRaidFrames +-- already hide the position panel, the grid, the movers and the pinned drag chrome, +-- and release unlock's test claim. +-- +-- ⚠ NOT restored on exit, deliberately. Blizzard's Edit Mode does not put anyone +-- else's windows back either, and silently re-unlocking frames under someone who +-- has just finished rearranging their UI is worse than making them click Unlock. +local function standDownForEditMode() + if InCombatLockdown() then return end -- Edit Mode is unreachable in combat; belt anyway + + local partyDb = DF.GetDB and DF:GetDB() + if partyDb and not partyDb.locked then + partyDb.locked = true + if DF.LockFrames then DF:LockFrames() end + end + + local raidDb = DF.GetRaidDB and DF:GetRaidDB() + if raidDb and not raidDb.raidLocked then + raidDb.raidLocked = true + if DF.LockRaidFrames then DF:LockRaidFrames() end + end + + -- Drop the user's claim too, so no preview survives into Edit Mode. This also + -- settles the aura containers: turning test mode off hands the real data + -- provider back, which is what keeps Edit Mode's sample auras out of our rows + -- (see Frames/AuraContainer.lua -- test mode owns the provider switch while it + -- is running, so the usual guard stands aside for it). + if DF.SetTestModeOwner then + DF:SetTestModeOwner("party", "user", false, true) + DF:SetTestModeOwner("raid", "user", false, true) + end +end + +if EventRegistry and EventRegistry.RegisterCallback then + EventRegistry:RegisterCallback("EditMode.Enter", standDownForEditMode, DF) +end + diff --git a/DandersFrames/GUI/GUI.lua b/DandersFrames/GUI/GUI.lua index 96ca8d4b..948ef486 100644 --- a/DandersFrames/GUI/GUI.lua +++ b/DandersFrames/GUI/GUI.lua @@ -331,8 +331,33 @@ local function GetThemeColor() end GUI.GetThemeColor = GetThemeColor +-- Cursor position in FRAME's own coordinate space. +-- +-- ☠ NEVER DIVIDE THE CURSOR BY UIParent:GetEffectiveScale() FOR A FRAME INSIDE +-- THE SETTINGS WINDOW. GetCursorPosition returns a value you divide by a frame's +-- EFFECTIVE scale to get that frame's coordinates. The settings window carries +-- the user's UI Scale on top of UIParent's, so for anything inside it the two +-- differ by exactly that factor -- while every frame:GetTop()/GetLeft() you +-- compare against is already in the frame's own space. Divide by the wrong one +-- and the comparison is nonsense: at 140% the cursor reads 40% further from the +-- screen edge than it is, which is why drag-to-reorder only ever dropped in the +-- right place at 100%. +-- +-- The exception, and why this takes a frame rather than assuming: a frame +-- PARENTED AND ANCHORED to UIParent (the Aura Designer's drag ghost) really does +-- have UIParent's effective scale, so there UIParent is the correct divisor. +-- Pass the frame whose coordinates you are comparing against and it is right +-- either way. +function GUI:CursorPos(frame) + local scale = frame and frame.GetEffectiveScale and frame:GetEffectiveScale() + if not scale or scale == 0 then scale = UIParent:GetEffectiveScale() end + if not scale or scale == 0 then scale = 1 end + local x, y = GetCursorPosition() + return x / scale, y / scale +end + -- Physical pixels per UI unit, measured from the frame's OWN effective scale. --- The GUI window carries a user scale (guiScale) on top of UIParent's and is +-- The GUI window carries a user scale (windowState.scale) on top of UIParent's and is -- freely resizable, so this is almost never 1 and cannot be read from the -- addon-wide DF:GetPixelScale (which is relative to UIParent). local function PixelsPerUnit(frame) diff --git a/DandersFrames/Locales/enUS.lua b/DandersFrames/Locales/enUS.lua index 125c7e7a..10211c95 100644 --- a/DandersFrames/Locales/enUS.lua +++ b/DandersFrames/Locales/enUS.lua @@ -550,6 +550,9 @@ L["Built-in presets can't be renamed or deleted."] = true L["By Power Type"] = true L["Cancel Fade on Dispellable Debuff"] = true L["Cannot delete Default profile."] = true +-- Orphaned: test mode is no longer REFUSED while unlocked, it just leaves up the +-- frames unlock still needs. Superseded by "Test frames stay visible while frames +-- are unlocked...". Kept until the locale audit sweeps orphans across all locales. L["Cannot disable test mode while frames are unlocked. Lock frames first."] = true L["Cannot Edit"] = true L["Cannot enter test mode during combat."] = true @@ -1512,6 +1515,7 @@ L["Targeted List is a Party-only feature. Switch to Party mode to configure."] = L["Targeted Spells"] = true L["Test"] = true L["Test Count"] = true +L["Test frames stay visible while frames are unlocked - they will hide when you lock."] = true L["Test mode disabled."] = true L["Test mode enabled."] = true L["Test mode ended — entering combat."] = true @@ -1643,6 +1647,7 @@ L["X Mark"] = true L["X Size"] = true L["Yellow=high, Orange=highest, Red=tanking."] = true L["Yes"] = true +L["You already have a filter with that name. Name the imported filter:"] = true L["You already have a filter with these spells: \"%s\". Import a separate copy anyway?"] = true L["You can enable or disable the spells shown, but not add new ones. Create a custom filter to add your own."] = true L["Your UI Scale is already pixel-perfect for this resolution."] = true diff --git a/DandersFrames/TestMode/Shim.lua b/DandersFrames/TestMode/Shim.lua index d908a9fb..d0d9328a 100644 --- a/DandersFrames/TestMode/Shim.lua +++ b/DandersFrames/TestMode/Shim.lua @@ -50,3 +50,102 @@ if not DF.UpdateRaidTestFrames then DF.UpdateRaidTestFrames = noop end if not DF.HideRaidTestFrames then DF.HideRaidTestFrames = noop end if not DF.StopTestAnimation then DF.StopTestAnimation = noop end if not DF.TeardownTestModeEngines then DF.TeardownTestModeEngines = noop end + +-- ============================================================ +-- TEST MODE OWNERSHIP — who is asking for the preview +-- ============================================================ +-- TWO independent things want test frames on screen: the USER (the test panel, +-- /df test, the toolbar) and UNLOCK (which needs frames to have something to +-- drag). A single boolean cannot tell them apart, and that was the whole bug: +-- +-- * Unlock SNAPSHOTTED DF.testMode and lock trusted the snapshot +-- (partyTestModeBeforeUnlock / raidTestModeBeforeUnlock). Anything that +-- changed test mode DURING the unlock session left it stale, and it failed +-- in both directions -- field-reported, both orders: +-- stale TRUE -> lock keeps a preview the user had turned off (stuck on) +-- stale FALSE -> lock kills a preview the user had turned on +-- * The "Cannot disable test mode while frames are unlocked" error existed +-- only to stop the snapshot going stale. It papered over the model, and it +-- did not even cover the toolbar button, which closes the PANEL without +-- going through ToggleTestMode at all -- that was the reported repro. +-- +-- Ownership says it directly: frames show while ANY owner wants them. Lock +-- releases the unlock claim; if the user still wants a preview, it stays. +-- +-- ☠ Show*/Hide*TestFrames stay the ONLY mechanism that moves frames -- these +-- helpers just decide WHEN to call them. Everything downstream that follows +-- test mode (PinnedFrames' testModeActive and its mover chrome, the aura +-- containers, the animation driver) hangs off those two, so it keeps working +-- without knowing ownership exists. +DF._testOwners = DF._testOwners or { party = {}, raid = {} } + +local function testScope(scope) + return (scope == "raid") and "raid" or "party" +end + +-- Is a preview actually on screen right now? +function DF:IsTestModeActive(scope) + if testScope(scope) == "raid" then return DF.raidTestMode and true or false end + return DF.testMode and true or false +end + +-- Does anyone still want one? +function DF:IsTestModeWanted(scope) + for _, wanted in pairs(DF._testOwners[testScope(scope)]) do + if wanted then return true end + end + return false +end + +-- Does one specific owner hold a claim? The test panel's toggle asks this about +-- "user": it is the USER's switch, so it must show the user's own claim rather +-- than whether a preview happens to be on screen. Those differ while unlocked -- +-- unlock holds its own claim -- and showing the preview state there made clicking +-- the toggle look like it did nothing. +function DF:IsTestModeOwnedBy(scope, owner) + return DF._testOwners[testScope(scope)][owner] and true or false +end + +-- Claim or release `owner` ("user" | "unlock"), then settle. +-- `silent` suppresses the "Test mode enabled/disabled" chat line: pass it when the +-- change is a SIDE EFFECT of something the user already sees (unlock/lock announce +-- themselves), and omit it when the user asked for test mode directly. +function DF:SetTestModeOwner(scope, owner, wanted, silent) + scope = testScope(scope) + local had = DF._testOwners[scope][owner] + DF._testOwners[scope][owner] = wanted and true or nil + DF:ReconcileTestMode(scope, silent) + + -- ☠ Explain the one outcome that otherwise reads as a broken click: the user + -- turned the preview off and the frames stayed, because unlock still needs + -- them. It lives HERE rather than in ToggleTestMode so every route says it -- + -- the panel's toggle, the toolbar button (which releases via the panel's + -- OnHide and never touches ToggleTestMode), /df test and the mover action. It + -- was in ToggleTestMode first and the toolbar route silently skipped it. + if owner == "user" and had and not wanted and not silent + and DF:IsTestModeActive(scope) and DF.Say and DF.L then + DF:Say(DF.L["Test frames stay visible while frames are unlocked - they will hide when you lock."]) + end +end + +-- Bring the screen in line with what the owners want. Idempotent, so it is safe +-- to call from anywhere; a no-op when they already agree. +function DF:ReconcileTestMode(scope, silent) + scope = testScope(scope) + local wanted, active = DF:IsTestModeWanted(scope), DF:IsTestModeActive(scope) + if wanted == active then return end + if scope == "raid" then + if wanted then DF:ShowRaidTestFrames(silent) else DF:HideRaidTestFrames(silent) end + else + if wanted then DF:ShowTestFrames(silent) else DF:HideTestFrames(silent) end + end + -- Show* refuses in combat, so `active` may still disagree afterwards; the + -- next reconcile picks that up rather than us pretending it succeeded. + if DF.GUI and DF.GUI.UpdateTestButtonState then DF.GUI.UpdateTestButtonState() end +end + +-- For paths that hide test frames DIRECTLY (mode switches, the Click Casting +-- tab): drop every claim, so no owner can outlive the frames it asked for. +function DF:ClearTestModeOwners(scope) + wipe(DF._testOwners[testScope(scope)]) +end diff --git a/DandersFrames/TextDesigner/DataSource.lua b/DandersFrames/TextDesigner/DataSource.lua index 7f4e2881..95cf3bf7 100644 --- a/DandersFrames/TextDesigner/DataSource.lua +++ b/DandersFrames/TextDesigner/DataSource.lua @@ -329,8 +329,22 @@ function LiveSource:IsInRange() return getMS().SafeBoolean(UnitInRange(self.unit), true) end +-- Cached per frame. This wrapper carries nothing but the frame and its unit, and every +-- LiveSource method is a pure read -- none of them write to self -- so a fresh table +-- plus setmetatable on each call was pure churn. It ran once per frame per Text +-- Designer update and accounted for 100% of DataSource.Live's allocation (520 KB in a +-- boss trace). The instance never outlives the update either: Render:UpdateFrame only +-- calls methods on it and never stores it. +-- +-- Both fields are re-stamped every call, so a retargeted or recycled frame still gets +-- the correct unit rather than a stale one. function DataSource.Live(frame) - local instance = setmetatable({}, LiveSource) + local instance = frame and frame._tdLiveSource + if not instance then + instance = setmetatable({}, LiveSource) + -- No frame means nothing to hang the cache off; fall back to a one-off. + if frame then frame._tdLiveSource = instance end + end instance.frame = frame instance.unit = frame and frame.unit or nil return instance diff --git a/DandersFrames/TextDesigner/Render.lua b/DandersFrames/TextDesigner/Render.lua index 73adf4c6..f29f325b 100644 --- a/DandersFrames/TextDesigner/Render.lua +++ b/DandersFrames/TextDesigner/Render.lua @@ -19,6 +19,11 @@ local wipe = wipe -- Enabled-element id set, rebuilt by UpdateFrame. Module-local and reused -- (wipe, not {}) because UpdateFrame runs in the unit-event hot path. local enabledScratch = {} +-- Second scratch for Render:UpdateFrame's delete-sweep. Same reason and same lifetime +-- as enabledScratch above -- it is filled and consumed inside one call and never +-- escapes -- but it holds ALL element ids where enabledScratch holds only the enabled +-- ones, so the two cannot share a table. +local liveIdScratch = {} -- ============================================================ -- HINT CATEGORIES — which content types refresh on which hints @@ -63,9 +68,40 @@ local CONTENT_HINTS = { -- FONT/COLOR RESOLUTION (overrides + globalDefaults) -- ============================================================ +-- Shared read-only stand-ins for the absent-table cases. Both are ONLY ever indexed +-- (never written) in the body below, so one shared instance is safe and saves two +-- throwaway tables per call on elements that carry no overrides -- which is most of +-- them. Same for the white fallback, which callers only read fields from. +local EMPTY_APPEARANCE = {} +local DEFAULT_TEXT_COLOR = { r = 1, g = 1, b = 1, a = 1 } + +-- ☠ SHARED SCRATCH — the returned table is the SAME table on every call. +-- resolveAppearance is #2 in every combat trace and ran once per element per +-- frame per tick, so this one table was most of what remained after ac55fb85 +-- shared the three fallbacks. +-- +-- SAFE ONLY WHILE AT MOST ONE RESOLVED APPEARANCE IS LIVE AT A TIME. That holds +-- by construction today, and all four parts are load-bearing: +-- 1. applyAppearance is resolveAppearance's only caller, and updateOne is +-- applyAppearance's only caller (both verified, not assumed). +-- 2. updateOne holds the result across applyPosition -> Resolve -> +-- mirrorElement. None of those re-enters: resolveAppearance is a FILE-LOCAL, +-- so nothing outside Render.lua can reach it at all. +-- 3. Nothing RETAINS it. mirrorElement reads app.font/fontSize/outline at the +-- point of use and stores nothing; the result is dead when updateOne returns. +-- 4. Every field is reassigned on every call, so no stale value can carry over. +-- +-- ☠ WHAT WOULD BREAK IT: a second resolve while a first result is still in scope. +-- That exact shape existed until ac55fb85 -- the class-colour branch re-resolved +-- the same element just to read one alpha, while the outer result was still bound +-- for mirrorElement -- and a shared table then would have aliased the two +-- silently, which is why that commit declined to share this one. If you add a +-- caller, give it its own table or pass it the fields it needs. +local appearanceScratch = {} + local function resolveAppearance(elem, globalDefaults) - globalDefaults = globalDefaults or {} - local overrides = elem.overrides or {} + globalDefaults = globalDefaults or EMPTY_APPEARANCE + local overrides = elem.overrides or EMPTY_APPEARANCE -- useClassColor is the one boolean field, so the `(override and value) or -- global` pattern the others use would swallow an override of FALSE (it -- falls through to the global default). Branch on the override flag instead. @@ -75,13 +111,13 @@ local function resolveAppearance(elem, globalDefaults) else useClassColor = globalDefaults.useClassColor or false end - return { - font = (overrides.font and elem.font) or globalDefaults.font or "DF Roboto SemiBold", - fontSize = (overrides.fontSize and elem.fontSize) or globalDefaults.fontSize or 10, - color = (overrides.color and elem.color) or globalDefaults.color or {r=1, g=1, b=1, a=1}, - outline = (overrides.outline and elem.outline) or globalDefaults.outline or "SHADOW;NONE", - useClassColor = useClassColor, - } + local app = appearanceScratch + app.font = (overrides.font and elem.font) or globalDefaults.font or "DF Roboto SemiBold" + app.fontSize = (overrides.fontSize and elem.fontSize) or globalDefaults.fontSize or 10 + app.color = (overrides.color and elem.color) or globalDefaults.color or DEFAULT_TEXT_COLOR + app.outline = (overrides.outline and elem.outline) or globalDefaults.outline or "SHADOW;NONE" + app.useClassColor = useClassColor + return app end -- ============================================================ @@ -297,7 +333,11 @@ local function updateOne(frame, elem, source, globalDefaults, enabledById) local token = source:GetClassToken() local color = token and RAID_CLASS_COLORS and RAID_CLASS_COLORS[token] if color then - local app = resolveAppearance(elem, globalDefaults) + -- Reuse the appearance applyAppearance already resolved above: same elem, + -- same globalDefaults, same deterministic function. This used to re-resolve + -- the whole thing just to read one alpha, which is a wasted table (up to + -- four) per class-coloured element per tick -- resolveAppearance is #2 in + -- every combat trace. fs:SetTextColor(color.r, color.g, color.b, (app.color and app.color.a) or 1) end end @@ -382,12 +422,12 @@ function Render:UpdateFrame(frame, tdDB, source, hint, isPreview) -- are intentionally left in place so a subsequent add reusing the id -- recovers the same FontString instead of leaking another one. if frame._tdFontStrings then - local liveIds = {} + wipe(liveIdScratch) for _, elem in ipairs(tdDB.elements or {}) do - liveIds[elem.id] = true + liveIdScratch[elem.id] = true end for id, fs in pairs(frame._tdFontStrings) do - if not liveIds[id] then + if not liveIdScratch[id] then fs:Hide() end end diff --git a/DandersFrames/TextDesigner/Resolver.lua b/DandersFrames/TextDesigner/Resolver.lua index 7b68cbc2..3c54c05f 100644 --- a/DandersFrames/TextDesigner/Resolver.lua +++ b/DandersFrames/TextDesigner/Resolver.lua @@ -326,19 +326,43 @@ end -- groups as just their separator-joined item resolutions. (The real -- implementation will deduplicate against per-item rendering once -- live rendering ships; preview just shows the static concatenation.) +-- Scratch tables for RESOLVERS.group. This resolver is the single biggest allocator in +-- the whole addon -- 38% of a dungeon trace, 44% of a boss trace -- purely because it +-- rebuilt `parts` plus a 14-field element table PER ITEM on every tick, from config +-- that only changes when the user edits it. +-- +-- Reuse is safe because groups CANNOT NEST: the Text Designer's Add Item picker excludes +-- the "group" type (DandersFrames_Options/TextDesigner/UI/Options.lua passes +-- excludeKey = "group"), so RESOLVERS[typeKey] below can never route back into this +-- function and it is never re-entered. No resolver retains its elem argument either -- +-- they read fields, and applyNameTrunc only reads nameLength/truncateMode. This path is +-- single-threaded with no yields, so one call always completes before the next starts. +-- +-- ☠ Every field of groupItemElem MUST be assigned on EVERY iteration, nil included, or a +-- value silently leaks from the previous item into the next. +local groupParts = {} +local groupItemElem = {} + RESOLVERS.group = function(elem, source) -- groupItems is an array whose entries are either a typeKey string (live -- data) or a table { type = "custom_static", text = "..." } for custom text. -- Each item resolves as if it were a standalone element (settings cascade -- from the group). The group's separator joins the non-empty results. if not elem.groupItems or #elem.groupItems == 0 then - DF:Debug("TD", "group resolver: elem id=%s has no groupItems", tostring(elem.id)) + if DF:DebugActive("TD") then + DF:Debug("TD", "group resolver: elem id=%s has no groupItems", tostring(elem.id)) + end return "" end - DF:Debug("TD", "group resolver: elem id=%s items=%d separator=%q", - tostring(elem.id), #elem.groupItems, tostring(elem.groupSeparator or " / ")) + -- Guarded: the tostring() args are evaluated by the CALLER, so an unguarded + -- DF:Debug allocated two strings per call even with the trace off. + if DF:DebugActive("TD") then + DF:Debug("TD", "group resolver: elem id=%s items=%d separator=%q", + tostring(elem.id), #elem.groupItems, tostring(elem.groupSeparator or " / ")) + end local MS = getMS() - local parts = {} + local parts = groupParts + for pi = #parts, 1, -1 do parts[pi] = nil end for i, rawItem in ipairs(elem.groupItems) do -- Each item carries its OWN formatting (per-item, not cascaded from the -- group). Missing flags fall back to the standard per-type defaults. @@ -346,26 +370,27 @@ RESOLVERS.group = function(elem, source) local typeKey = item.contentType local itemResolver = RESOLVERS[typeKey] if not itemResolver then - DF:Debug("TD", " [%d] %s: NO RESOLVER", i, tostring(typeKey)) + if DF:DebugActive("TD") then + DF:Debug("TD", " [%d] %s: NO RESOLVER", i, tostring(typeKey)) + end else local ab = item.abbreviate; if ab == nil then ab = true end local hz = item.hideWhenZero; if hz == nil then hz = true end - local itemElem = { - contentType = typeKey, - abbreviate = ab, - hideWhenZero = hz, - hidePercent = item.hidePercent, - decimals = item.decimals or 0, - staticText = item.staticText, - aggroText1 = item.aggroText1, - aggroText2 = item.aggroText2, - aggroText3 = item.aggroText3, - rangeInText = item.rangeInText, - rangeOutText = item.rangeOutText, - nameLength = item.nameLength, - truncateMode = item.truncateMode, - groupFormat = item.groupFormat, - } + local itemElem = groupItemElem + itemElem.contentType = typeKey + itemElem.abbreviate = ab + itemElem.hideWhenZero = hz + itemElem.hidePercent = item.hidePercent + itemElem.decimals = item.decimals or 0 + itemElem.staticText = item.staticText + itemElem.aggroText1 = item.aggroText1 + itemElem.aggroText2 = item.aggroText2 + itemElem.aggroText3 = item.aggroText3 + itemElem.rangeInText = item.rangeInText + itemElem.rangeOutText = item.rangeOutText + itemElem.nameLength = item.nameLength + itemElem.truncateMode = item.truncateMode + itemElem.groupFormat = item.groupFormat local v = itemResolver(itemElem, source) local isSec = MS.IsSecret(v) if v then @@ -388,7 +413,9 @@ RESOLVERS.group = function(elem, source) end end end - DF:Debug("TD", "group resolver: parts collected=%d", #parts) + if DF:DebugActive("TD") then + DF:Debug("TD", "group resolver: parts collected=%d", #parts) + end -- IMPORTANT: cannot use table.concat() here — it throws on secret-tainted -- entries ("invalid value (secret) at index N in table for 'concat'"). -- Manual `..` concat IS safe with secret strings as long as the final diff --git a/DandersFrames_Options/Core/ExportCategories.lua b/DandersFrames_Options/Core/ExportCategories.lua index 0e266cb6..80c9a153 100644 --- a/DandersFrames_Options/Core/ExportCategories.lua +++ b/DandersFrames_Options/Core/ExportCategories.lua @@ -1337,7 +1337,6 @@ end -- internal escape hatches, not profile content). Underscore-prefixed -- migration flags are excluded by rule and don't need listing. DF.ExportLocalOnly = { - guiScale = true, guiWidth = true, guiHeight = true, -- settings-window geometry minimapIcon = true, -- minimap button state useSecureHeaders = true, -- internal escape hatch (no GUI) diff --git a/DandersFrames_Options/FilterRegistry/UI/Options.lua b/DandersFrames_Options/FilterRegistry/UI/Options.lua index eda3e4f7..0ea47e75 100644 --- a/DandersFrames_Options/FilterRegistry/UI/Options.lua +++ b/DandersFrames_Options/FilterRegistry/UI/Options.lua @@ -110,6 +110,38 @@ local function ChainPopup(fn) C_Timer.After(0, fn) end +-- Import a decoded payload, asking for a name ONLY if that name is already in the +-- list. A clean import (nothing clashes) stays a single paste with no extra step; +-- a clashing one gets Duplicate's treatment -- you see the name, pre-filled with a +-- suggestion that does not collide, and can change it before anything is saved. +-- +-- Without this, import was the one path that committed a name you never saw: two +-- rows reading the same thing, and no way to tell from the Buffs page which one you +-- had ticked. Worst through "Import as Copy", which promises a distinguishable copy +-- and produced a row identical in both name AND spells. +-- +-- ⚠ The prompt is CHAINED. Popups are a singleton here, so opening one from inside +-- another's handler needs the next frame; the "Import as Copy" route reaches this +-- from a popup button. +local function ImportNamed(def) + if not R:IsCustomFilterNameTaken(def.name) then + SelectFilter("custom", R:ImportFilterPayload(def)) + return + end + ChainPopup(function() + PromptFilterName( + L["You already have a filter with that name. Name the imported filter:"], + R:SuggestUniqueFilterName(def.name), + L["Import"], + function(text) + text = Trim(text or "") + if text == "" then return end + def.name = text -- decoded payload is ours; safe to retitle + SelectFilter("custom", R:ImportFilterPayload(def)) + end) + end) +end + local function ShowFilterStringError(title, errKey) local message = FilterStringError(errKey) ChainPopup(function() @@ -1325,7 +1357,10 @@ function DF.BuildFilterDesignerPage(guiRef, pageRef, dbRef) -- matching the New and Duplicate paths. local match = R:FindContentMatch(def) if not match then - SelectFilter("custom", R:ImportFilterPayload(def)) + -- No content match, but the NAME can still collide -- that is the + -- case nothing checked: two rows reading the same thing with + -- different spells behind them. ImportNamed asks only if it does. + ImportNamed(def) return end -- Content-equal filter already present. Profile import silently @@ -1341,7 +1376,7 @@ function DF.BuildFilterDesignerPage(guiRef, pageRef, dbRef) message = message, buttons = { { label = L["Import as Copy"], onClick = function() - SelectFilter("custom", R:ImportFilterPayload(def)) + ImportNamed(def) end }, { label = L["Use Existing"], onClick = function() SelectFilter("custom", match) diff --git a/DandersFrames_Options/GUI/Controls.lua b/DandersFrames_Options/GUI/Controls.lua index 194dc7e4..fe7f9f0a 100644 --- a/DandersFrames_Options/GUI/Controls.lua +++ b/DandersFrames_Options/GUI/Controls.lua @@ -1705,7 +1705,7 @@ function GUI:CreateRoleOrderList(parent, dbTable, dbKey, callback, separateMelee item:SetScript("OnMouseDown", function(self, button) if button == "LeftButton" then draggingItem = self - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local itemTop = self:GetTop() dragOffsetY = itemTop - cursorY @@ -1719,7 +1719,7 @@ function GUI:CreateRoleOrderList(parent, dbTable, dbKey, callback, separateMelee item:SetScript("OnMouseUp", function(self, button) if button == "LeftButton" and draggingItem == self then - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local dropIndex = GetIndexFromY(cursorY) local order = GetCurrentOrder() @@ -1745,7 +1745,7 @@ function GUI:CreateRoleOrderList(parent, dbTable, dbKey, callback, separateMelee item:SetScript("OnUpdate", function(self) if draggingItem ~= self then return end - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local containerTop = container:GetTop() local containerBottom = container:GetBottom() @@ -2028,7 +2028,7 @@ function GUI:CreateClassOrderList(parent, dbTable, dbKey, callback) item:SetScript("OnMouseDown", function(self, button) if button == "LeftButton" then draggingItem = self - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local itemTop = self:GetTop() dragOffsetY = itemTop - cursorY @@ -2042,7 +2042,7 @@ function GUI:CreateClassOrderList(parent, dbTable, dbKey, callback) item:SetScript("OnMouseUp", function(self, button) if button == "LeftButton" and draggingItem == self then - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local dropIndex = GetIndexFromY(cursorY) local order = GetCurrentOrder() @@ -2068,7 +2068,7 @@ function GUI:CreateClassOrderList(parent, dbTable, dbKey, callback) item:SetScript("OnUpdate", function(self) if draggingItem ~= self then return end - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local containerTop = container:GetTop() local containerBottom = container:GetBottom() @@ -2332,7 +2332,7 @@ function GUI:CreateGroupOrderList(parent, dbTable, dbKey, callback, playerGroupF item:SetScript("OnMouseDown", function(self, button) if button == "LeftButton" then draggingItem = self - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local itemTop = self:GetTop() dragOffsetY = itemTop - cursorY @@ -2346,7 +2346,7 @@ function GUI:CreateGroupOrderList(parent, dbTable, dbKey, callback, playerGroupF item:SetScript("OnMouseUp", function(self, button) if button == "LeftButton" and draggingItem == self then - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local newIndex = GetIndexFromY(cursorY) -- Reorder @@ -2372,7 +2372,7 @@ function GUI:CreateGroupOrderList(parent, dbTable, dbKey, callback, playerGroupF item:SetScript("OnUpdate", function(self) if draggingItem ~= self then return end - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local containerTop = container:GetTop() local containerBottom = container:GetBottom() @@ -2896,7 +2896,7 @@ function GUI:CreateHighlightRosterWidget(parent, getPlayersFunc, setPlayersFunc, item:SetScript("OnMouseDown", function(self, button) if button == "LeftButton" then draggingItem = self - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local itemTop = self:GetTop() dragOffsetY = itemTop - cursorY @@ -2909,7 +2909,7 @@ function GUI:CreateHighlightRosterWidget(parent, getPlayersFunc, setPlayersFunc, item:SetScript("OnMouseUp", function(self, button) if button == "LeftButton" and draggingItem == self then - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local contentTop = rightContent:GetTop() if contentTop then local relativeY = contentTop - cursorY @@ -2935,7 +2935,7 @@ function GUI:CreateHighlightRosterWidget(parent, getPlayersFunc, setPlayersFunc, item:SetScript("OnUpdate", function(self) if draggingItem ~= self then return end - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local contentTop = rightContent:GetTop() local contentBottom = rightContent:GetBottom() diff --git a/DandersFrames_Options/GUI/Pages/NicknamesPage.lua b/DandersFrames_Options/GUI/Pages/NicknamesPage.lua index e1f7e927..2f560ae6 100644 --- a/DandersFrames_Options/GUI/Pages/NicknamesPage.lua +++ b/DandersFrames_Options/GUI/Pages/NicknamesPage.lua @@ -397,7 +397,7 @@ function DF.BuildNicknamesPage(guiRef, pageRef, dbRef, Add, AddSpace) -- reflow the other rows to leave a gap at the prospective drop slot. local function dragFollow(self) if draggingRow ~= self then return end - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(scrollContent)) local top = scrollContent:GetTop() if not top then return end local d = NK:GetDB() @@ -540,7 +540,7 @@ function DF.BuildNicknamesPage(guiRef, pageRef, dbRef, Add, AddSpace) row:SetScript("OnMouseDown", function(self, button) if button ~= "LeftButton" or not self.entryIndex then return end draggingRow = self - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) dragOffsetY = (self:GetTop() or 0) - cursorY self:SetFrameLevel(scrollContent:GetFrameLevel() + 10) self.dragHL:Show() @@ -550,7 +550,7 @@ function DF.BuildNicknamesPage(guiRef, pageRef, dbRef, Add, AddSpace) row:SetScript("OnMouseUp", function(self, button) if button ~= "LeftButton" or draggingRow ~= self then return end self:SetScript("OnUpdate", nil) - local cursorY = select(2, GetCursorPosition()) / UIParent:GetEffectiveScale() + local cursorY = select(2, GUI:CursorPos(self)) local drop = indexFromY(cursorY) local from = self.entryIndex draggingRow = nil diff --git a/DandersFrames_Options/GUI/Pages/Options.lua b/DandersFrames_Options/GUI/Pages/Options.lua index 05f60b73..e864cd4e 100644 --- a/DandersFrames_Options/GUI/Pages/Options.lua +++ b/DandersFrames_Options/GUI/Pages/Options.lua @@ -1561,10 +1561,10 @@ function DF:SetupGUIPages(GUI, CreateCategory, CreateSubTab, BuildPage) -- "background"/"missingHealth" belong to bars_health (which hosts all -- controls for those keys) — not registered here so its Copy/Sync/Reset -- solely owns them. - -- "permanentMover" (16 keys) and the two growth keys were reached by nothing: - -- the whole permanent mover was skipped by Copy, Sync and Reset. ("border" and - -- "anchor" match nothing either — the real keys are frameBorder* / frameAnchor*, - -- already covered by "frame"; left in place as harmless intent.) + -- "permanentMover" (16 keys) was reached by nothing: the whole permanent + -- mover was skipped by Copy, Sync and Reset. ("border" and "anchor" match + -- nothing either — the real keys are frameBorder* / frameAnchor*, already + -- covered by "frame"; left in place as harmless intent.) -- -- ⚠ DELIBERATELY NOT LISTED: the 14 raid* layout keys (raidUseGroups, -- raidPlayersPerRow, raidGroup*, raidFlat*, raidRowColSpacing...). They are @@ -1575,7 +1575,27 @@ function DF:SetupGUIPages(GUI, CreateCategory, CreateSubTab, BuildPage) -- cost of leaving them out is that Reset Page does not clear raid layout; -- that is the lesser of the two, and fixing it properly needs per-direction -- ownership, which SectionOwnsKey does not currently express. - Add(CreateCopyButton(self.child, {"frame", "permanentMover", "growDirection", "growthAnchor", "border", "anchor"}, L["Frame"], "general_frame"), 25, 2) + -- + -- ☠ growDirection / growthAnchor BELONG TO THAT SAME EXCLUSION and were + -- wrongly added to it (3b912fb0, alongside permanentMover). They are exactly + -- the case the paragraph above describes -- per-mode, edited from BOTH the + -- party and raid pages -- and they escaped it only because they are not + -- spelled "raid...". Owning them meant Sync with Raid overwrote the raid + -- growth direction from party's every refresh, so raid could never keep a + -- different one: field-reported as an imported profile arriving with raid + -- laid out in columns instead of rows, reproducible on every import. The + -- import was never at fault; the post-import refresh re-synced it. + -- + -- ☠ THE SYMPTOM IS ALMOST INVISIBLE. The sync copies every key under these + -- prefixes, but party and raid agree on nearly all of them, so the only + -- evidence is the one key where a user's two modes genuinely differ. + -- growDirection is that key for most people; do not read "only one setting + -- moved" as "small blast radius". + -- + -- The raid page's own dropdown inverts the labels (HORIZONTAL reads as + -- "Columns" there and "Rows" on party/flat), which is the clearest signal + -- these were never meant to be one shared setting. + Add(CreateCopyButton(self.child, {"frame", "permanentMover", "border", "anchor"}, L["Frame"], "general_frame"), 25, 2) -- Migration: Ensure new flat raid settings have defaults if db.raidFlatGrowthAnchor == nil then db.raidFlatGrowthAnchor = "START" end diff --git a/DandersFrames_Options/GUI/Panel.lua b/DandersFrames_Options/GUI/Panel.lua index 8d2c137f..d1dac784 100644 --- a/DandersFrames_Options/GUI/Panel.lua +++ b/DandersFrames_Options/GUI/Panel.lua @@ -29,18 +29,19 @@ function DF:CreateGUI() local minWidth, minHeight = 520, 400 local maxWidth, maxHeight = 1200, 900 - -- Load saved position and size (stored in party db since it's always available) - local guiDb = DF.db and DF.db.party or {} - local savedScale = guiDb.guiScale or 1.0 - local savedWidth = guiDb.guiWidth or defaultWidth - local savedHeight = guiDb.guiHeight or defaultHeight + -- Load saved position and size. Account-wide, NOT per-profile: see + -- DF:GetWindowState in Core/Profile.lua for why it must not follow profiles. + local guiDb = DF:GetWindowState() + local savedScale = guiDb.scale or 1.0 + local savedWidth = guiDb.width or defaultWidth + local savedHeight = guiDb.height or defaultHeight -- Main frame (matching old addon approach - no BackdropTemplate in CreateFrame) local frame = CreateFrame("Frame", "DandersFramesGUI", UIParent) frame:SetSize(savedWidth, savedHeight) -- Restore saved position, or default to center - if guiDb.guiPoint and guiDb.guiX then - frame:SetPoint(guiDb.guiPoint, UIParent, guiDb.guiRelPoint or "CENTER", guiDb.guiX, guiDb.guiY) + if guiDb.point and guiDb.x then + frame:SetPoint(guiDb.point, UIParent, guiDb.relPoint or "CENTER", guiDb.x, guiDb.y) else frame:SetPoint("CENTER") end @@ -81,12 +82,8 @@ function DF:CreateGUI() frame:StopMovingOrSizing() -- Save position so it persists across sessions local point, _, relPoint, x, y = frame:GetPoint() - if DF.db and DF.db.party then - DF.db.party.guiPoint = point - DF.db.party.guiRelPoint = relPoint - DF.db.party.guiX = x - DF.db.party.guiY = y - end + local ws = DF:GetWindowState() + ws.point, ws.relPoint, ws.x, ws.y = point, relPoint, x, y end) titleBar:SetFrameStrata("FULLSCREEN_DIALOG") titleBar:SetFrameLevel(200) @@ -229,8 +226,8 @@ function DF:CreateGUI() resizeHandle:SetScript("OnMouseUp", function(self, button) frame:StopMovingOrSizing() -- Save new size - DF.db.party.guiWidth = frame:GetWidth() - DF.db.party.guiHeight = frame:GetHeight() + local ws = DF:GetWindowState() + ws.width, ws.height = frame:GetWidth(), frame:GetHeight() -- Update content layout if GUI.SelectedMode == "clicks" then -- Refresh click casting UI on resize (skip scroll reset) @@ -495,9 +492,7 @@ function DF:CreateGUI() scaleSlider:SetScript("OnMouseUp", function(self) local value = math.floor(self:GetValue() * 20 + 0.5) / 20 frame:SetScale(value) - if DF.db and DF.db.party then - DF.db.party.guiScale = value - end + DF:GetWindowState().scale = value -- Also update popup panels if DF.positionPanel then DF.positionPanel:SetScale(value) @@ -508,7 +503,7 @@ function DF:CreateGUI() -- A new scale changes how many device pixels a UI unit covers, so -- every border on screen has to be re-derived at the new thickness. -- This is the ONLY action that does: nothing else in the GUI writes - -- guiScale, and moving or resizing the window leaves it alone. + -- windowState.scale, and moving or resizing the window leaves it alone. GUI:RefreshPixelBorders() end) @@ -589,13 +584,19 @@ function DF:CreateGUI() -- Function to update test button state (called externally) UpdateTestButtonState = function() - -- Active toggle look based on whether the test panel is visible. - local testActive = DF.TestPanel and DF.TestPanel:IsShown() - btnTest:SetActive(testActive) - -- Swap the framed-eye glyph: open (preview) when test mode is showing the - -- preview frames, slashed (preview_off) when it's off. + -- Both cues read the USER's claim -- the same thing the panel's toggle shows + -- -- so the two controls the user thinks of as one can never disagree. + -- + -- ⚠ Deliberately NOT "is a preview on screen". Unlocking puts frames up under + -- its OWN claim, and driving the glyph off that made a plain unlock look like + -- the user had switched test mode on. Frames being visible during an unlock is + -- unlock's business; the chat line explains it. + local panelOpen = DF.TestPanel and DF.TestPanel:IsShown() + local scope = (GUI.SelectedMode == "raid") and "raid" or "party" + local userWants = DF.IsTestModeOwnedBy and DF:IsTestModeOwnedBy(scope, "user") + btnTest:SetActive(panelOpen) btnTest.Icon:SetTexture("Interface\\AddOns\\DandersFrames\\Media\\Icons\\" - .. (testActive and "preview" or "preview_off")) + .. (userWants and "preview" or "preview_off")) -- White text/icon in both states (state shown by the toggle border/fill). btnTest.Text:SetTextColor(C_TEXT.r, C_TEXT.g, C_TEXT.b) btnTest.Icon:SetVertexColor(C_TEXT.r, C_TEXT.g, C_TEXT.b) @@ -645,6 +646,9 @@ function DF:CreateGUI() -- and rebuilding it a moment later. Cleared below. DF._testModeHandover = true DF:HideRaidTestFrames(true) -- silent + -- Hides directly rather than through the owners, so drop raid's + -- claims to match. carryTest re-claims in the scope we land in. + if DF.ClearTestModeOwners then DF:ClearTestModeOwners("raid") end end end @@ -658,9 +662,11 @@ function DF:CreateGUI() GUI:UpdateTabAvailability() GUI:RefreshCurrentPage() - -- Keep test mode active when switching modes (just switch which mode it runs in) - if carryTest and DF.ShowTestFrames then - DF:ShowTestFrames(true) -- silent + -- Keep test mode active when switching modes (just switch which mode it runs in). + -- Carried as the USER's claim: they had a preview up and are still asking for + -- one, just in the other scope. + if carryTest and DF.SetTestModeOwner then + DF:SetTestModeOwner("party", "user", true, true) -- silent: the mode swap is the visible event -- ShowTestFrames (unlike ShowRaidTestFrames) doesn't refresh the GUI, -- so the test panel's toggle label would stay on "Enable Test Mode". -- Refresh it now that party test mode is active. @@ -700,6 +706,9 @@ function DF:CreateGUI() -- Hand-over: see the raid->party handler above. DF._testModeHandover = true DF:HideTestFrames(true) -- silent + -- Hides directly rather than through the owners, so drop party's + -- claims to match. carryTest re-claims in the scope we land in. + if DF.ClearTestModeOwners then DF:ClearTestModeOwners("party") end end end @@ -713,8 +722,12 @@ function DF:CreateGUI() GUI:UpdateTabAvailability() GUI:RefreshCurrentPage() - -- Keep test mode active when switching modes (just switch which mode it runs in) - if carryTest and DF.ShowRaidTestFrames then + -- Keep test mode active when switching modes (just switch which mode it runs in). + -- Carried as the USER's claim: they had a preview up and are still asking for + -- one, just in the other scope. + if carryTest and DF.SetTestModeOwner then + DF:SetTestModeOwner("raid", "user", true, true) -- silent: the mode swap is the visible event + elseif carryTest and DF.ShowRaidTestFrames then DF:ShowRaidTestFrames() end if carryTest then @@ -734,6 +747,9 @@ function DF:CreateGUI() if DF.LockFrames then DF:LockFrames() end end if DF.testMode then DF:HideTestFrames(true) end + -- Leaving for a tab with no frames: nobody is asking for a preview + -- any more, so drop the claims rather than let them outlive it. + if DF.ClearTestModeOwners then DF:ClearTestModeOwners("party") end elseif GUI.SelectedMode == "raid" then local raidDb = DF:GetRaidDB() if raidDb and not raidDb.raidLocked then @@ -741,8 +757,10 @@ function DF:CreateGUI() if DF.LockRaidFrames then DF:LockRaidFrames() end end if DF.raidTestMode then DF:HideRaidTestFrames(true) end + -- As above: no preview is wanted on a tab that has no frames. + if DF.ClearTestModeOwners then DF:ClearTestModeOwners("raid") end end - + GUI.SelectedMode = "clicks" if DF.Search then DF.Search:HideResults() @@ -908,12 +926,8 @@ function DF:CreateGUI() bottomBar:SetScript("OnDragStop", function() frame:StopMovingOrSizing() local point, _, relPoint, x, y = frame:GetPoint() - if DF.db and DF.db.party then - DF.db.party.guiPoint = point - DF.db.party.guiRelPoint = relPoint - DF.db.party.guiX = x - DF.db.party.guiY = y - end + local ws = DF:GetWindowState() + ws.point, ws.relPoint, ws.x, ws.y = point, relPoint, x, y end) local footer = CreateFrame("Frame", nil, bottomBar) diff --git a/DandersFrames_Options/GUI/SettingsWidgets.lua b/DandersFrames_Options/GUI/SettingsWidgets.lua index c02e044a..93a2a990 100644 --- a/DandersFrames_Options/GUI/SettingsWidgets.lua +++ b/DandersFrames_Options/GUI/SettingsWidgets.lua @@ -1627,7 +1627,7 @@ function GUI:CreateRangeSlider(parent, opts) local dragging = nil local function ApplyCursor() - local x = select(1, GetCursorPosition()) / UIParent:GetEffectiveScale() + local x = select(1, GUI:CursorPos(track)) local trackLeft = track:GetLeft() if not trackLeft then return end local pos = math.max(2, math.min(x - trackLeft, width - 2)) @@ -1652,7 +1652,7 @@ function GUI:CreateRangeSlider(parent, opts) track:EnableMouse(true) track:SetScript("OnMouseDown", function(_, b) if b ~= "LeftButton" then return end - local x = select(1, GetCursorPosition()) / UIParent:GetEffectiveScale() + local x = select(1, GUI:CursorPos(track)) local trackLeft = track:GetLeft() if not trackLeft then return end local value = PosToValue(x - trackLeft) diff --git a/DandersFrames_Options/TestMode/TestMode.lua b/DandersFrames_Options/TestMode/TestMode.lua index 3c2b8f98..5bb7ef48 100644 --- a/DandersFrames_Options/TestMode/TestMode.lua +++ b/DandersFrames_Options/TestMode/TestMode.lua @@ -2303,41 +2303,25 @@ function DF:ToggleTestMode() return end - local isRaidMode = DF.GUI and DF.GUI.SelectedMode == "raid" - - if isRaidMode then - local db = DF:GetRaidDB() - -- Don't allow toggling test mode off while frames are unlocked - if not db.raidLocked and DF.raidTestMode then - DF:Say(L["Cannot disable test mode while frames are unlocked. Lock frames first."]) - return - end - - -- Toggle raid test mode - if DF.raidTestMode then - DF:HideRaidTestFrames() - else - DF:ShowRaidTestFrames() - end - else - local db = DF:GetDB() - -- Don't allow toggling test mode off while frames are unlocked - if not db.locked and DF.testMode then - DF:Say(L["Cannot disable test mode while frames are unlocked. Lock frames first."]) - return - end - - -- Toggle party test mode - if DF.testMode then - DF:HideTestFrames() - else - DF:ShowTestFrames() - end - end + -- Flip the USER's claim. Unlock holds its own, so this no longer needs to + -- refuse while frames are unlocked: turning the preview off mid-unlock drops + -- your claim, unlock keeps the frames it still needs to have something to + -- drag, and locking then hides them because nobody is left asking. + -- + -- The old refusal ("Cannot disable test mode while frames are unlocked") + -- existed only to stop the snapshot going stale, and it never covered the + -- toolbar button — which closes the PANEL without coming through here at + -- all. That was the reported repro. See TestMode/Shim.lua. + local scope = (DF.GUI and DF.GUI.SelectedMode == "raid") and "raid" or "party" + -- The "frames stay visible while unlocked" line is emitted by SetTestModeOwner, + -- not here: the toolbar button releases the claim through the panel's OnHide and + -- never comes through this function, so a message here reached only one of the + -- two buttons the user thinks of as the same control. + DF:SetTestModeOwner(scope, "user", not DF:IsTestModeOwnedBy(scope, "user")) end -- Show raid test frames -function DF:ShowRaidTestFrames() +function DF:ShowRaidTestFrames(silent) if InCombatLockdown() then DF:Say(L["Cannot enter test mode during combat."]) return @@ -2438,10 +2422,17 @@ function DF:ShowRaidTestFrames() if DF.PinnedFrames and DF.PinnedFrames.EnterTestMode then DF.PinnedFrames:EnterTestMode() end + + -- Same confirmation party mode gives. Raid never announced itself at all, and + -- did not even take `silent` -- Panel.lua has been passing one to the Hide half + -- for a while, which quietly did nothing. + if not silent then + DF:Say(L["Test mode enabled."]) + end end -- Hide raid test frames -function DF:HideRaidTestFrames() +function DF:HideRaidTestFrames(silent) DF.raidTestMode = false -- Restore the real aura provider only when NEITHER test mode remains active -- (party + raid share the global data-provider switch). @@ -2540,6 +2531,11 @@ function DF:HideRaidTestFrames() and not DF.testMode then DF.PinnedFrames:ExitTestMode() end + + -- Same confirmation party mode gives; see ShowRaidTestFrames. + if not silent then + DF:Say(L["Test mode disabled."]) + end end -- Update raid test frames with test data @@ -3550,19 +3546,18 @@ function DF:CreateTestPanel() panel:Hide() local function ApplyScale(self) - local guiScale = DF.db and DF.db.party and DF.db.party.guiScale or 1.0 - self:SetScale(guiScale) + self:SetScale(DF:GetWindowState().scale or 1.0) end panel:SetScript("OnHide", function() - if DF.testMode then - local db = DF:GetDB() - if db.locked then DF:HideTestFrames() end - end - if DF.raidTestMode then - local db = DF:GetRaidDB() - if db.raidLocked then DF:HideRaidTestFrames() end - end + -- Closing the panel drops the USER's claim on both scopes -- and only + -- that. It used to hide the frames itself, but ONLY when locked, which is + -- exactly how the preview got stranded: close the panel while unlocked and + -- the frames stayed up with nothing tracking that you had dismissed them. + -- Now unlock's own claim decides whether they stay, and the later lock + -- takes them down. + DF:SetTestModeOwner("party", "user", false) + DF:SetTestModeOwner("raid", "user", false) if DF.GUI and DF.GUI.UpdateTestButtonState then DF.GUI.UpdateTestButtonState() end @@ -3620,7 +3615,20 @@ function DF:CreateTestPanel() }) toggleBtn:SetScript("OnClick", function() DF:ToggleTestMode() - panel:UpdateState() + -- This button and the toolbar's test button are the SAME action: turn the + -- preview off and close the panel. Leaving the panel open with test mode off + -- was the odd state -- one control closed everything, the other printed a + -- line and sat there. So an open panel now always means "the user is asking + -- for a preview", which is also what makes the toggle's label unambiguous. + -- + -- Hide() runs OnHide, which releases the user's claim on both scopes; that is + -- idempotent with the release ToggleTestMode just did. + local scope = (DF.GUI and DF.GUI.SelectedMode == "raid") and "raid" or "party" + if not DF:IsTestModeOwnedBy(scope, "user") then + panel:Hide() + else + panel:UpdateState() + end end) panel.toggleBtn = toggleBtn @@ -4452,7 +4460,13 @@ function DF:CreateTestPanel() local isRaidMode = DF.GUI and DF.GUI.SelectedMode == "raid" local db = isRaidMode and DF:GetRaidDB() or DF:GetDB() local themeColor = GetThemeColor() - local testActive = IsTestActive() + -- The toggle is the USER's switch, so it reflects the USER's claim, not + -- whether a preview is on screen. Those differ while unlocked (unlock holds + -- its own claim), and using the preview state there left the button stuck on + -- "Disable Test Mode" after a click that had genuinely worked. + local scope = isRaidMode and "raid" or "party" + local testActive = DF.IsTestModeOwnedBy and DF:IsTestModeOwnedBy(scope, "user") + or (not DF.IsTestModeOwnedBy and IsTestActive()) -- Title self.title:SetText(L["Test Mode"]) @@ -4589,19 +4603,12 @@ function DF:ToggleTestPanel() else panel:UpdateState() panel:Show() - - -- Auto-enable test mode when panel opens - local isRaidMode = DF.GUI and DF.GUI.SelectedMode == "raid" - if isRaidMode then - if not DF.raidTestMode then - DF:ShowRaidTestFrames() - panel:UpdateState() - end - else - if not DF.testMode then - DF:ShowTestFrames() - panel:UpdateState() - end - end + + -- Opening the panel claims the preview for the user (it exists to show + -- one). Claiming rather than calling Show* directly means the matching + -- release on close is symmetric, and unlock's claim is untouched either way. + local scope = (DF.GUI and DF.GUI.SelectedMode == "raid") and "raid" or "party" + DF:SetTestModeOwner(scope, "user", true) + panel:UpdateState() end end