From 6597d20b7dad0336fa500f2d28a9af81bd7b03f2 Mon Sep 17 00:00:00 2001 From: Xurkon Date: Sat, 6 Jun 2026 00:04:50 -0500 Subject: [PATCH] feat: refine learner data source and pin rendering --- CHANGELOG.md | 4 + Database/QuestieDB.lua | 88 ++++++++-- Modules/Map/QuestieMapUtils.lua | 11 +- .../AdvancedTab/QuestieOptionsAdvanced.lua | 41 +++++ .../DatabaseTab/QuestieOptionsDatabase.lua | 20 ++- Modules/Options/QuestieOptionsDefaults.lua | 5 + Modules/Quest/QuestieQuest.lua | 34 ++-- Modules/QuestieLearner.lua | 99 ++++++++--- Tests/QuestieLearnerDataSourceMode_spec.lua | 166 ++++++++++++++++++ Tests/QuestieLearner_performance_spec.lua | 36 ++++ docs/changelog.html | 4 + workflow/performance-audit-2026-06-03-FULL.md | 126 +++++++++++++ 12 files changed, 576 insertions(+), 58 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d8cce32..98a6616 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ - **[Error Suppression - Debug Modes]** Moved missing quest and other non-fatal database/error spam out of normal chat output and into Questie debug-critical/developer output. Fatal startup failures remain loud. - **[Tooltip Data Precedence]** Updated tooltip handling so QuestieLearner defers to AscensionDB-owned tooltip/objective data instead of hiding or replacing server-plugin data for active quests. - **[QuestieQuest - Unavailable Quest Guard]** Guarded the available-quest draw thread so unresolved quest IDs are skipped safely instead of crashing the thread, and deduped the skip log so the same missing quest does not spam every redraw. +- **[QuestieLearner - Data Source Mode Cohesion]** Reworked the Auto / Learner / Static / Neither data-source modes so switching between them applies live and consistently. A single missing static sub-table (npc/object/quest/item) no longer locks the whole addon into learner mode — only the genuinely-missing store falls back. Static and Neither no longer silently fall back to learner records, switching modes now clears the per-zone quest cache, and the mode switch drives a full pin/tracker redraw through `QuestieQuest:SmoothReset()` (the previous redraw call imported a mis-named module and silently did nothing). +- **[QuestieLearner - Pin Refresh Latency]** Collapsed a redundant second debounce stage on the live-learn pin-refresh path. Newly learned spawns now redraw within a single debounce window instead of waiting out both the NPC live-update delay and a separate pin-refresh delay, roughly halving perceived pin-update latency in the Balanced and Low presets. Also fixed a latent infinite timer re-arm that could occur once the pin flush was triggered directly. +- **[Map - Dense Pin Clustering Aggressiveness Knob]** Re-implemented density-adaptive clustering for crowded kill objectives, now controlled by a new "Dense pin clustering aggressiveness" slider on the Advanced tab (0 = show every pin, higher = tighter consolidation where many pins share a zone). Coincident pins are always deduplicated regardless of the clustering settings, and the intentional per-zone (Sunstrider Isle) and object-icon range overrides are preserved. +- **[QuestieLearner - One Pin Per Spawn, Not Per Kill]** Fixed learner kill evidence rendering a separate pin for every kill. Because kill coordinates are the player's position at kill time (and respawns carry fresh GUIDs), repeated kills at the same spot drifted just enough to dodge the exact-match dedup. The immediate learner-mode spawn builder now merges evidence within a small radius into one pin per physical spawn, and the weighted spawn-evidence merge groups kills by coordinate bucket so a single spot can actually accumulate enough evidence to clear the confidence threshold. The merge distance is exposed as a new "Spawn Pin Dedup Radius" slider on the Advanced tab (0 = show every distinct position, higher = fewer/tighter pins per spawn) which redraws live. ### Branch / Release Notes diff --git a/Database/QuestieDB.lua b/Database/QuestieDB.lua index 3e44faa..cec7cb0 100644 --- a/Database/QuestieDB.lua +++ b/Database/QuestieDB.lua @@ -210,8 +210,45 @@ QuestieDB.ItemPointers = _dummyHandle.pointers QuestieDB.baseDatabaseMissing = false QuestieDB.baseDatabaseMissingKeys = {} +-- The four core static stores. The base DB is only considered fully "missing" +-- (which forces learner mode) when EVERY one of these failed to load. +QuestieDB._baseDatabaseStores = { "npcData", "objectData", "questData", "itemData" } + +-- True only when a specific static store failed to load. Reads use this so a +-- single missing/mismatched store falls back to learner data for that store +-- alone, instead of locking the whole addon into learner mode. +function QuestieDB:IsStoreMissing(storeKey) + return QuestieDB.baseDatabaseMissingKeys and QuestieDB.baseDatabaseMissingKeys[storeKey] == true +end + +-- Clears every per-entity cache. Used when the data source mode changes so that +-- quest/npc/item/object AND per-zone results are rebuilt against the new mode. +function QuestieDB:ClearModeCaches() + _QuestieDB.questCache = {} + _QuestieDB.itemCache = {} + _QuestieDB.npcCache = {} + _QuestieDB.objectCache = {} + _QuestieDB.zoneCache = {} +end + function QuestieDB:IsBaseDatabaseMissing() - return QuestieDB.baseDatabaseMissing == true + if QuestieDB.baseDatabaseMissing ~= true then + return false + end + -- Only report the base DB as missing when EVERY core store failed. A partial + -- failure (e.g. itemData missing but npcData present) must not override the + -- user's Data Source Mode selection — per-read fallback handles the gaps. + local keys = QuestieDB.baseDatabaseMissingKeys + if not keys then + return false + end + local stores = QuestieDB._baseDatabaseStores + for i = 1, 4 do + if not keys[stores[i]] then + return false + end + end + return true end local function _GetLearnerSettings() @@ -233,11 +270,32 @@ local function _GetLearnerRecord(storeName, id) return store[id] or store[tostring(id)] end +-- Map-percent radius used to collapse per-GUID kill evidence into one pin per +-- physical spawn. Kill coordinates are the player's position at kill time, so +-- repeated kills (and respawns, which carry fresh GUIDs) land on slightly +-- different coords. Without this merge, every kill would render its own pin. +-- A distance test (rather than a grid bucket) avoids the boundary artifact where +-- two near-identical coords straddle a cell edge and split into separate pins. +-- The radius is user-tunable via the learner "Spawn Pin Dedup Radius" knob; 0 +-- disables merging (every distinct coord shown). +local GUID_SPAWN_DEDUP_RADIUS = 4.0 + +local function _GetSpawnDedupRadius() + local settings = _GetLearnerSettings() + local r = settings and tonumber(settings.spawnDedupRadius) + if r and r >= 0 then + return r + end + return GUID_SPAWN_DEDUP_RADIUS +end + local function _BuildSpawnTableFromGuidEvidence(evidence) if type(evidence) ~= "table" then return nil end + local radius = _GetSpawnDedupRadius() + local radiusSq = radius * radius local spawns = {} local hasEntries = false for _, entry in pairs(evidence) do @@ -250,7 +308,9 @@ local function _BuildSpawnTableFromGuidEvidence(evidence) local zoneSpawns = spawns[zoneId] local exists = false for _, coord in ipairs(zoneSpawns) do - if coord[1] == x and coord[2] == y then + local dx = coord[1] - x + local dy = coord[2] - y + if (dx * dx + dy * dy) <= radiusSq then exists = true break end @@ -604,12 +664,14 @@ function QuestieDB:GetObject(objectId) local rawdata local override - if mode == "learner" or QuestieDB:IsBaseDatabaseMissing() then + if mode == "learner" or QuestieDB:IsStoreMissing("objectData") then rawdata = learnerRecord override = nil else rawdata = QuestieDB.QueryObject(objectId, QuestieDB._objectAdapterQueryOrder) - if not rawdata and learnerRecord then + if not rawdata and learnerRecord and mode == "auto" then + -- Only "auto" overlays learner data on top of the static DB. "static" + -- and "none" must never silently fall back to learner records. rawdata = learnerRecord end override = QuestieDB.objectDataOverrides and (QuestieDB.objectDataOverrides[objectId] or QuestieDB.objectDataOverrides[tostring(objectId)]) @@ -660,12 +722,14 @@ function QuestieDB:GetItem(itemId) local learnerRecord = _GetLearnerRecord("items", itemId) local rawdata local override - if mode == "learner" or QuestieDB:IsBaseDatabaseMissing() then + if mode == "learner" or QuestieDB:IsStoreMissing("itemData") then rawdata = learnerRecord override = nil else rawdata = QuestieDB.QueryItem(itemId, QuestieDB._itemAdapterQueryOrder) - if not rawdata and learnerRecord then + if not rawdata and learnerRecord and mode == "auto" then + -- Only "auto" overlays learner data on top of the static DB. "static" + -- and "none" must never silently fall back to learner records. rawdata = learnerRecord end override = QuestieDB.itemDataOverrides and (QuestieDB.itemDataOverrides[itemId] or QuestieDB.itemDataOverrides[tostring(itemId)]) @@ -1542,12 +1606,14 @@ function QuestieDB.GetQuest(questId, ...) -- /dump QuestieDB.GetQuest(867) local learnerRecord = _GetLearnerRecord("quests", questId) local rawdata local overrideData - if mode == "learner" or QuestieDB:IsBaseDatabaseMissing() then + if mode == "learner" or QuestieDB:IsStoreMissing("questData") then rawdata = learnerRecord overrideData = nil else rawdata = QuestieDB.QueryQuest(questId, QuestieDB._questAdapterQueryOrder) - if not rawdata and learnerRecord then + if not rawdata and learnerRecord and mode == "auto" then + -- Only "auto" overlays learner data on top of the static DB. "static" + -- and "none" must never silently fall back to learner records. rawdata = learnerRecord end overrideData = QuestieDB.questDataOverrides and (QuestieDB.questDataOverrides[questId] or QuestieDB.questDataOverrides[tostring(questId)]) @@ -2155,12 +2221,14 @@ function QuestieDB:GetNPC(npcId) local learnerRecord = _GetLearnerRecord("npcs", npcId) local rawdata local override - if mode == "learner" or QuestieDB:IsBaseDatabaseMissing() then + if mode == "learner" or QuestieDB:IsStoreMissing("npcData") then rawdata = learnerRecord override = nil else rawdata = QuestieDB.QueryNPC(npcId, QuestieDB._npcAdapterQueryOrder) - if not rawdata and learnerRecord then + if not rawdata and learnerRecord and mode == "auto" then + -- Only "auto" overlays learner data on top of the static DB. "static" + -- and "none" must never silently fall back to learner records. rawdata = learnerRecord end override = QuestieDB.npcDataOverrides and (QuestieDB.npcDataOverrides[npcId] or QuestieDB.npcDataOverrides[tostring(npcId)]) diff --git a/Modules/Map/QuestieMapUtils.lua b/Modules/Map/QuestieMapUtils.lua index a92a440..a16cb1b 100644 --- a/Modules/Map/QuestieMapUtils.lua +++ b/Modules/Map/QuestieMapUtils.lua @@ -10,6 +10,11 @@ local HBD = QuestieCompat.HBD or LibStub("HereBeDragonsQuestie-2.0") local ZOOM_MODIFIER = 1; +-- Pins whose world-coordinate distance is within this epsilon are treated as the +-- same physical spot and always merged, independent of the clustering range. This +-- is the baseline pin deduplication that runs even when clustering is turned off. +QuestieMap.utils.COINCIDENT_EPSILON = 0.2; + -- All the speed we can get is worth it. local tinsert = table.insert local next = next @@ -146,7 +151,11 @@ function QuestieMap.utils:CalcHotzones(points, rangeR, count) -- Do not cluster icons if they have no coordinates and aX ~= 0 and aY ~= 0 and point2.worldX ~= 0 and point2.worldY ~= 0 then local distance = QuestieLib:Euclid(aX, aY, point2.worldX, point2.worldY) - if (distance < movingRange) then + -- Always deduplicate pins that sit on (essentially) the same spot, + -- even when clustering is disabled (movingRange == 0). This keeps two + -- icons from stacking on an identical coordinate regardless of the + -- clusterDensityAggressiveness / clusterLevelHotzone settings. + if (distance < movingRange) or (distance <= QuestieMap.utils.COINCIDENT_EPSILON) then point2.touched = true tinsert(notes, point2) end diff --git a/Modules/Options/AdvancedTab/QuestieOptionsAdvanced.lua b/Modules/Options/AdvancedTab/QuestieOptionsAdvanced.lua index c370513..6bb01ef 100644 --- a/Modules/Options/AdvancedTab/QuestieOptionsAdvanced.lua +++ b/Modules/Options/AdvancedTab/QuestieOptionsAdvanced.lua @@ -19,6 +19,8 @@ local IsleOfQuelDanas = QuestieLoader:ImportModule("IsleOfQuelDanas"); local l10n = QuestieLoader:ImportModule("l10n") ---@type QuestieCompat local QuestieCompat = QuestieLoader:ImportModule("QuestieCompat") +---@type QuestieDB +local QuestieDB = QuestieLoader:ImportModule("QuestieDB") QuestieOptions.tabs.advanced = {} local optionsDefaults = QuestieOptionsDefaults:Load() @@ -50,6 +52,9 @@ local function GetLearnerSettings() if settings.minConfidencePins == nil then settings.minConfidencePins = 1 end + if settings.spawnDedupRadius == nil then + settings.spawnDedupRadius = 4.0 + end return settings end @@ -168,6 +173,22 @@ function QuestieOptions.tabs.advanced:Initialize() QuestieOptionsUtils.DetermineTheme() end, }, + clusterDensityAggressiveness = { + type = "range", + order = 1.41, + name = function() return l10n('Dense pin clustering aggressiveness'); end, + desc = function() return l10n('How aggressively crowded kill objectives are consolidated into fewer pins. 0 shows every pin; higher values tighten clustering where many pins share a zone. Coincident pins are always deduplicated.'); end, + width = 1.5, + disabled = function() return (not Questie.db.profile.enabled); end, + min = 0, + max = 100, + step = 5, + get = function(info) return QuestieOptions:GetProfileValue(info); end, + set = function(info, value) + QuestieOptions:SetProfileValue(info, value) + QuestieOptionsUtils:Delay(0.5, QuestieOptions.ClusterRedraw, l10n('Setting dense pin clustering aggressiveness to %s : Redrawing!', value)) + end, + }, quelDanasSpacer1 = QuestieOptionsUtils:Spacer(1.45, (not Questie.IsTBC)), npcrules_group = { type = "group", @@ -323,6 +344,26 @@ function QuestieOptions.tabs.advanced:Initialize() settings.performanceMode = "manual" end, }, + learnerSpawnDedupRadius = { + type = "range", + order = 2.55, + name = function() return l10n('Spawn Pin Dedup Radius'); end, + desc = function() return l10n('How close two learned kill positions must be (in map %) to merge into a single pin. Higher values show fewer, tighter pins per spawn; 0 shows every distinct position.'); end, + min = 0, + max = 15, + step = 0.5, + width = 1.5, + get = function() return GetLearnerSettings().spawnDedupRadius or 4.0 end, + set = function(_, value) + GetLearnerSettings().spawnDedupRadius = value + -- Spawn tables are cached per NPC; clear so the new radius is + -- applied on the redraw instead of serving stale merged coords. + if QuestieDB and QuestieDB.ClearModeCaches then + QuestieDB:ClearModeCaches() + end + QuestieOptionsUtils:Delay(0.5, QuestieQuest.SmoothReset, l10n('Setting spawn pin dedup radius to %s : Redrawing!', value)) + end, + }, learnerCommsIntensity = { type = "select", order = 2.6, diff --git a/Modules/Options/DatabaseTab/QuestieOptionsDatabase.lua b/Modules/Options/DatabaseTab/QuestieOptionsDatabase.lua index a4499e5..03c7fdc 100644 --- a/Modules/Options/DatabaseTab/QuestieOptionsDatabase.lua +++ b/Modules/Options/DatabaseTab/QuestieOptionsDatabase.lua @@ -52,14 +52,18 @@ local function ApplyLearnerMode() QuestieLearner:ApplyDataSourceMode() end - local QuestieEventHandler = QuestieLoader:ImportModule("QuestieEventHandler") - if QuestieEventHandler and QuestieEventHandler.UpdateAllQuests then - QuestieEventHandler:UpdateAllQuests() - end - - local QuestieTracker = QuestieLoader:ImportModule("QuestieTracker") - if QuestieTracker and QuestieTracker.Update then - QuestieTracker:Update() + -- SmoothReset is the canonical full refresh: it clears all map/minimap notes + -- and tooltips, recalculates and redraws available quests, re-updates every + -- active quest, and refreshes the tracker. This makes a data-source-mode + -- switch (auto/learner/static/none) take effect everywhere in real time. + local QuestieQuest = QuestieLoader:ImportModule("QuestieQuest") + if QuestieQuest and QuestieQuest.SmoothReset then + QuestieQuest:SmoothReset() + else + local QuestieTracker = QuestieLoader:ImportModule("QuestieTracker") + if QuestieTracker and QuestieTracker.Update then + QuestieTracker:Update() + end end end diff --git a/Modules/Options/QuestieOptionsDefaults.lua b/Modules/Options/QuestieOptionsDefaults.lua index 618aa6a..8aa9dd1 100644 --- a/Modules/Options/QuestieOptionsDefaults.lua +++ b/Modules/Options/QuestieOptionsDefaults.lua @@ -9,6 +9,11 @@ function QuestieOptionsDefaults:Load() ascensionScalingAsked = false, --Ascension clusterLevelHotzone = 50, + -- How aggressively dense objectives (many pins in one zone) get + -- consolidated. 0 = off (every pin shown); higher tightens the + -- clustering range for crowded kill objectives. Coincident pins are + -- always deduplicated regardless of this value. See QuestieQuest:_DrawObjectiveIcons. + clusterDensityAggressiveness = 35, enableIconLimit = false, iconLimit = 200, availableScale = 1.2, diff --git a/Modules/Quest/QuestieQuest.lua b/Modules/Quest/QuestieQuest.lua index 9030ea6..b971871 100644 --- a/Modules/Quest/QuestieQuest.lua +++ b/Modules/Quest/QuestieQuest.lua @@ -1847,18 +1847,30 @@ _DrawObjectiveIcons = function(questId, iconsToDraw, objective, maxPerType) local iconCount, orderedList = _GetIconsSortedByDistance(iconsToDraw) - -- Dense kill objectives (like Sunstrider Isle mana wyrms) previously used - -- a lower clustering hotzone here. Leave the old behavior commented so we - -- can restore it quickly if we need to revisit consolidation again. - --[[ - if iconCount >= 20 then - range = math.max(6, math.floor(range * 0.25)) - elseif iconCount >= 12 then - range = math.max(10, math.floor(range * 0.4)) - elseif iconCount >= 6 then - range = math.max(16, math.floor(range * 0.65)) + -- Dense kill objectives (like Sunstrider Isle mana wyrms) consolidate more + -- aggressively the more pins share a zone. This is now user-tunable via the + -- clusterDensityAggressiveness knob (0 = off / show every pin, 100 = the + -- original aggressive consolidation). The per-zone and object overrides below + -- still take precedence, and coincident pins are always deduped in CalcHotzones. + local densityAggression = Questie.db.profile.clusterDensityAggressiveness or 0 + if densityAggression > 0 then + local factor = densityAggression / 100 + if factor > 1 then factor = 1 end + local baseMult + if iconCount >= 20 then + baseMult = 0.25 + elseif iconCount >= 12 then + baseMult = 0.4 + elseif iconCount >= 6 then + baseMult = 0.65 + end + if baseMult then + -- Interpolate between no reduction (factor 0) and the full base + -- multiplier (factor 1) so the knob scales smoothly. + local mult = 1 - (1 - baseMult) * factor + range = math.max(1, math.floor(range * mult)) + end end - --]] if orderedList[1] and orderedList[1].Icon == Questie.ICON_TYPE_OBJECT then -- new clustering / limit code should prevent problems, always show all object notes range = range * 0.2; -- Only use 20% of the default range. diff --git a/Modules/QuestieLearner.lua b/Modules/QuestieLearner.lua index db146d1..2362a1a 100644 --- a/Modules/QuestieLearner.lua +++ b/Modules/QuestieLearner.lua @@ -409,6 +409,7 @@ local function EnsureLearnedData() learnItems = true, learnObjects = true, minConfidencePins = 1, + spawnDedupRadius = 4.0, prioritizeMyData = true, dataSourceMode = "auto", staleThreshold = 90, -- days @@ -434,6 +435,7 @@ local function EnsureLearnedData() if s.learnItems == nil then s.learnItems = true end if s.learnObjects == nil then s.learnObjects = true end if s.minConfidencePins == nil then s.minConfidencePins = 1 end + if s.spawnDedupRadius == nil then s.spawnDedupRadius = 4.0 end if s.prioritizeMyData == nil then s.prioritizeMyData = true end if s.dataSourceMode == nil then if s.prioritizeMyData == false then @@ -517,11 +519,16 @@ function QuestieLearner:ApplyDataSourceMode() CaptureStaticOverrideSnapshot() RestoreStaticOverridesForMode() self:InjectLearnedData() - if QuestieDB and QuestieDB.private then + -- Rebuild every per-entity cache (including the per-zone quest cache) so the + -- mode switch takes effect immediately for reads, pins, and zone lookups. + if QuestieDB and QuestieDB.ClearModeCaches then + QuestieDB:ClearModeCaches() + elseif QuestieDB and QuestieDB.private then QuestieDB.private.questCache = {} QuestieDB.private.itemCache = {} QuestieDB.private.npcCache = {} QuestieDB.private.objectCache = {} + QuestieDB.private.zoneCache = {} end QuestieLearner.data = Questie.dbLearner.global end @@ -618,27 +625,11 @@ local _pendingQuestFrameUnloads = {} -- caps the worst-case latency: once that many seconds have elapsed since the first -- pending change, the flush fires even if kills are still coming (0 = pure debounce, -- never force). Only "batched" mode debounces; "immediate" flushes on first fire. -local function _FlushActiveQuestPins() - if GetTime and GetLearnerSetting("pinRefreshMode", "batched") == "batched" then - local timer = (C_Timer) or (QuestieCompat and QuestieCompat.C_Timer) - local now = GetTime() - local delay = GetLearnerSetting("pinRefreshDelay", 0.75) - local maxWait = GetLearnerSetting("pinRefreshMaxWait", 5.0) - local quiet = now - (_pendingQuestPinLastActivity or now) - local waited = now - (_pendingQuestPinFirstDirty or now) - if timer and timer.After and quiet < delay and (maxWait <= 0 or waited < maxWait) then - local remaining = delay - quiet - if maxWait > 0 then - local capRemaining = maxWait - waited - if capRemaining < remaining then remaining = capRemaining end - end - if remaining < 0 then remaining = 0 end - -- _pendingQuestPinRefreshTimer stays true so concurrent queues don't double-arm. - timer.After(remaining, _FlushActiveQuestPins) - return - end - end - +-- Performs the actual pin rebuild for every pending quest. No debounce gate — the +-- caller is responsible for deciding when to fire (either the trailing-debounce +-- wrapper below, or an immediate flush from the already-debounced NPC live-update +-- flush, which makes the redundant second debounce stage unnecessary). +local function _DoFlushActiveQuestPins() local questIdSet = _pendingQuestPinRefreshes _pendingQuestPinRefreshes = {} _pendingQuestPinRefreshTimer = nil @@ -664,6 +655,42 @@ local function _FlushActiveQuestPins() _pendingQuestFrameUnloads = {} end +local function _FlushActiveQuestPins() + -- Only defer while there is genuine pending activity. If the timestamps were + -- already cleared (e.g. the NPC live-update flush force-flushed the pins via + -- _DoFlushActiveQuestPins), a leftover timer must NOT treat the nil timestamp + -- as "now" and re-arm forever — it should fall through and flush (a no-op when + -- the pending set is empty). + if GetTime and GetLearnerSetting("pinRefreshMode", "batched") == "batched" + and _pendingQuestPinLastActivity then + local timer = (C_Timer) or (QuestieCompat and QuestieCompat.C_Timer) + local now = GetTime() + local delay = GetLearnerSetting("pinRefreshDelay", 0.75) + local maxWait = GetLearnerSetting("pinRefreshMaxWait", 5.0) + local quiet = now - _pendingQuestPinLastActivity + local waited = now - (_pendingQuestPinFirstDirty or _pendingQuestPinLastActivity) + if timer and timer.After and quiet < delay and (maxWait <= 0 or waited < maxWait) then + local remaining = delay - quiet + if maxWait > 0 then + local capRemaining = maxWait - waited + if capRemaining < remaining then remaining = capRemaining end + end + if remaining < 0 then remaining = 0 end + -- _pendingQuestPinRefreshTimer stays true so concurrent queues don't double-arm. + timer.After(remaining, _FlushActiveQuestPins) + return + end + end + + _DoFlushActiveQuestPins() +end + +-- When true, _RefreshActiveQuestPins only accumulates pending quests and does NOT +-- arm its own trailing-debounce timer. Set by _FlushNpcLiveUpdates, which already +-- debounced via liveNpcUpdateDelay and force-flushes the pins itself afterwards — +-- so the second debounce stage would only add redundant latency. +local _deferPinRefreshScheduling = false + local function _RefreshActiveQuestPins(questIdSet) -- Skip scheduling when the set is empty (avoids no-op timer callbacks) if not next(questIdSet) then return end @@ -684,6 +711,11 @@ local function _RefreshActiveQuestPins(questIdSet) _pendingQuestPinFirstDirty = now end + -- Caller (NPC live-update flush) will force-flush; don't arm a redundant timer. + if _deferPinRefreshScheduling then + return + end + if _pendingQuestPinRefreshTimer then return end @@ -916,11 +948,17 @@ local function _FlushNpcLiveUpdates() _Learner.pendingNpcLiveUpdateFirstDirty = nil _Learner.pendingNpcLiveUpdateLastActivity = nil + -- This flush already coalesced kills over liveNpcUpdateDelay. Suppress the + -- per-NPC pin-refresh debounce while invalidating, then flush all affected + -- quests once, immediately — instead of waiting out a second pinRefreshDelay. + _deferPinRefreshScheduling = true for npcId in pairs(pending) do if _ApplyNpcLiveUpdate(npcId) then _InvalidateSpawnListsForNPC(npcId) end end + _deferPinRefreshScheduling = false + _DoFlushActiveQuestPins() end local function _QueueNpcLiveUpdate(npcId) @@ -1496,12 +1534,17 @@ local function _MergeSpawnEvidence(npcId) entry.y = evidenceY local rx, ry = NormalizeCoordPair(evidenceX, evidenceY) - local key = entry.zoneId .. "|" .. tostring(rx) .. "|" .. tostring(ry) - -- DEBUG: log each entry being grouped - Questie:Debug(Questie.DEBUG_LEARNER, - "[QuestieLearner] _MergeSpawnEvidence GROUPING: spawnUID=", spawnUID, - "entry.x=", entry.x, "entry.y=", entry.y, - "rx=", rx, "ry=", ry, "key=", key) + -- Group kills by a coordinate bucket, not by exact coords. Kill + -- evidence is the player's position at kill time, which drifts a + -- little every kill, so exact keys would treat each kill as its own + -- "location" — producing one pin per kill and never letting any + -- single spot accumulate enough evidence to clear the confidence + -- threshold. Bucketing collapses repeated kills at the same spawn + -- into one location (matching the [7] InsertIfNewBucket behavior). + local grid = GetCoordGridForZone(entry.zoneId) + local bx = floor(rx / grid) + local by = floor(ry / grid) + local key = entry.zoneId .. "|" .. bx .. "|" .. by if not evidence[key] then evidence[key] = { zoneId = entry.zoneId, x = rx, y = ry, count = 0 } end diff --git a/Tests/QuestieLearnerDataSourceMode_spec.lua b/Tests/QuestieLearnerDataSourceMode_spec.lua index ab9ef79..813f777 100644 --- a/Tests/QuestieLearnerDataSourceMode_spec.lua +++ b/Tests/QuestieLearnerDataSourceMode_spec.lua @@ -188,6 +188,78 @@ describe("QuestieDB learner source fallback", function() assert.equals(27.25, npc.spawns[44][1][2]) end) + it("collapses many nearby kills (distinct GUIDs) into one spawn pin", function() + -- Five kills of respawns at the same spot: distinct GUID keys, slightly + -- drifting player coords. This must render as ONE pin, not five. + Questie.dbLearner.global.npcs[9005] = { + [1] = "Respawning Boar", + [8] = { + [201] = { zoneId = 44, x = 50.0, y = 50.0 }, + [202] = { zoneId = 44, x = 50.4, y = 50.3 }, + [203] = { zoneId = 44, x = 49.7, y = 50.6 }, + [204] = { zoneId = 44, x = 50.9, y = 49.8 }, + [205] = { zoneId = 44, x = 50.2, y = 50.1 }, + }, + } + + local npc = QuestieDB:GetNPC(9005) + assert.is_table(npc) + assert.is_table(npc.spawns[44]) + assert.equals(1, #npc.spawns[44]) + end) + + it("keeps genuinely separate spawn locations as distinct pins", function() + Questie.dbLearner.global.npcs[9006] = { + [1] = "Field Boars", + [8] = { + [301] = { zoneId = 44, x = 20.0, y = 20.0 }, + [302] = { zoneId = 44, x = 20.3, y = 20.2 }, -- same spot as 301 + [303] = { zoneId = 44, x = 70.0, y = 65.0 }, -- far corner + }, + } + + local npc = QuestieDB:GetNPC(9006) + assert.is_table(npc) + assert.is_table(npc.spawns[44]) + assert.equals(2, #npc.spawns[44]) + end) + + it("honors the spawn dedup radius knob (0 disables proximity merge)", function() + Questie.dbLearner.global.settings.spawnDedupRadius = 0 + Questie.dbLearner.global.npcs[9007] = { + [1] = "Drifting Kills", + [8] = { + [401] = { zoneId = 44, x = 50.0, y = 50.0 }, + [402] = { zoneId = 44, x = 50.4, y = 50.3 }, + [403] = { zoneId = 44, x = 49.7, y = 50.6 }, + [404] = { zoneId = 44, x = 50.9, y = 49.8 }, + [405] = { zoneId = 44, x = 50.2, y = 50.1 }, + }, + } + + local npc = QuestieDB:GetNPC(9007) + assert.is_table(npc) + assert.is_table(npc.spawns[44]) + -- With merging off, each distinct kill coordinate stays its own pin. + assert.equals(5, #npc.spawns[44]) + end) + + it("widens merging when the dedup radius is increased", function() + Questie.dbLearner.global.settings.spawnDedupRadius = 12 + Questie.dbLearner.global.npcs[9008] = { + [1] = "Loose Cluster", + [8] = { + [501] = { zoneId = 44, x = 40.0, y = 40.0 }, + [502] = { zoneId = 44, x = 48.0, y = 46.0 }, -- ~10 away: merges at radius 12 + }, + } + + local npc = QuestieDB:GetNPC(9008) + assert.is_table(npc) + assert.is_table(npc.spawns[44]) + assert.equals(1, #npc.spawns[44]) + end) + it("returns learner object data when static queries are unavailable", function() local obj = QuestieDB:GetObject(9002) assert.is_table(obj) @@ -196,3 +268,97 @@ describe("QuestieDB learner source fallback", function() assert.equals(44, obj.zoneID) end) end) + +describe("QuestieDB partial base DB missing", function() + before_each(function() + dofile("Tests/wow_api_mock.lua") + dofile("Database/QuestieDB.lua") + end) + + it("does not report the base DB missing when only one store failed", function() + QuestieDB.baseDatabaseMissing = true + QuestieDB.baseDatabaseMissingKeys = { itemData = true } + assert.is_false(QuestieDB:IsBaseDatabaseMissing()) + assert.is_true(QuestieDB:IsStoreMissing("itemData")) + assert.is_false(QuestieDB:IsStoreMissing("npcData")) + end) + + it("reports the base DB missing only when every core store failed", function() + QuestieDB.baseDatabaseMissing = true + QuestieDB.baseDatabaseMissingKeys = { + npcData = true, objectData = true, questData = true, itemData = true, + } + assert.is_true(QuestieDB:IsBaseDatabaseMissing()) + end) + + it("honors the static selection for a present store even when another is missing", function() + QuestieDB.baseDatabaseMissing = true + QuestieDB.baseDatabaseMissingKeys = { itemData = true } + Questie.dbLearner.global.settings.dataSourceMode = "static" + Questie.dbLearner.global.npcs = { + [9100] = { [1] = "Should Not Win", [7] = { [44] = { { 1, 2 } } }, [9] = 44 }, + } + QuestieDB.private.npcCache = {} + local queried = false + QuestieDB.QueryNPC = function() queried = true; return nil end + + pcall(function() QuestieDB:GetNPC(9100) end) + assert.is_true(queried) + end) +end) + +describe("QuestieDB mode cohesion", function() + before_each(function() + dofile("Tests/wow_api_mock.lua") + dofile("Database/QuestieDB.lua") + dofile("Database/npcDB.lua") + Questie.dbLearner.global.settings.enabled = true + Questie.dbLearner.global.npcs = { + [9200] = { [1] = "Learner Only NPC", [7] = { [44] = { { 5, 6 } } }, [9] = 44 }, + } + QuestieDB.QueryNPC = function() return nil end + QuestieDB.baseDatabaseMissing = false + QuestieDB.baseDatabaseMissingKeys = {} + QuestieDB.private.npcCache = {} + end) + + it("does NOT leak learner data into static mode when the store is present", function() + Questie.dbLearner.global.settings.dataSourceMode = "static" + QuestieDB.private.npcCache = {} + assert.is_nil(QuestieDB:GetNPC(9200)) + end) + + it("does NOT leak learner data into none mode when the store is present", function() + Questie.dbLearner.global.settings.dataSourceMode = "none" + QuestieDB.private.npcCache = {} + assert.is_nil(QuestieDB:GetNPC(9200)) + end) + + it("DOES overlay learner data in auto mode when the static DB lacks it", function() + Questie.dbLearner.global.settings.dataSourceMode = "auto" + QuestieDB.private.npcCache = {} + local npc = QuestieDB:GetNPC(9200) + assert.is_table(npc) + assert.equals("Learner Only NPC", npc.name) + end) + + it("clears every cache including the zone cache on mode switch", function() + QuestieDB.private.questCache[1] = {} + QuestieDB.private.zoneCache[1] = {} + QuestieDB:ClearModeCaches() + assert.is_nil(QuestieDB.private.questCache[1]) + assert.is_nil(QuestieDB.private.zoneCache[1]) + end) +end) + +describe("QuestieLearner mode switch redraw wiring", function() + it("drives a full real-time refresh via SmoothReset, not the mis-named event handler", function() + local function read(path) + local f = assert(io.open(path, "r")); local c = f:read("*a"); f:close(); return c + end + local dbOptions = read("Modules/Options/DatabaseTab/QuestieOptionsDatabase.lua") + assert.is_true(string.find(dbOptions, "QuestieQuest:SmoothReset()", 1, true) ~= nil) + -- The old import name resolved to nil and silently skipped the redraw. + assert.is_nil(string.find(dbOptions, "ImportModule(\"QuestieEventHandler\")", 1, true)) + end) +end) diff --git a/Tests/QuestieLearner_performance_spec.lua b/Tests/QuestieLearner_performance_spec.lua index e94866c..f68a25d 100644 --- a/Tests/QuestieLearner_performance_spec.lua +++ b/Tests/QuestieLearner_performance_spec.lua @@ -105,6 +105,42 @@ describe("QuestieLearner kill-path batching", function() assert.is_true(table.getn(queuedTimers) >= 2) end) + it("force-flushes active quest pins within the NPC live-update flush (no second debounce)", function() + local updateCount = 0 + local originalUpdateQuest = QuestieQuest.UpdateQuest + QuestieQuest.UpdateQuest = function(_, questId) + updateCount = updateCount + 1 + end + + QuestiePlayer.currentQuestlog = { [5500] = true } + local originalGetQuest = QuestieDB.GetQuest + QuestieDB.GetQuest = function(questId) + if questId == 5500 then + return { Objectives = { [1] = { Id = 7500, spawnList = { [7500] = {} } } } } + end + return nil + end + + QuestieLearner:LearnNPC(7500, "Quest Boar", nil, nil, nil, nil, 41.0, 52.0, 44) + + -- Drain exactly ONE timer round (the NPC live-update flush). The pin refresh + -- must happen inside that same flush, not in a later pinRefreshDelay cycle. + local firstRound = queuedTimers + queuedTimers = {} + for i = 1, table.getn(firstRound) do + simulatedTime = simulatedTime + 1 + firstRound[i]() + end + + assert.is_true(updateCount >= 1) + + -- And draining the rest must terminate (no infinite self-re-arming timer). + drainQueuedTimers() + + QuestieDB.GetQuest = originalGetQuest + QuestieQuest.UpdateQuest = originalUpdateQuest + end) + it("coalesces repeated quest-pin refreshes into one flush", function() local updateCount = 0 local originalUpdateQuest = QuestieQuest.UpdateQuest diff --git a/docs/changelog.html b/docs/changelog.html index a56d569..7557139 100644 --- a/docs/changelog.html +++ b/docs/changelog.html @@ -188,6 +188,10 @@
  • [Arrow — Low-End Performance Controls] Added live Arrow update throttles to reduce repeated nearest-target and coordinate work while preserving existing arrow behavior.
  • [Measured Hot Paths — Phase 3] Landed measured optimizations on the phase 3 branch for literal localization caching, available quest redraw batching, QuestieDB.IsDoable batch reads, hot profile aliases, GetTime() hoists, NPC fallback lookup caching, and validate-cache allocation cleanup.
  • [QuestieQuest — Unavailable Quest Guard] Guarded the available-quest draw thread so unresolved quest IDs are skipped safely instead of crashing the thread, and deduped the skip log so the same missing quest does not spam every redraw.
    • +
  • [QuestieLearner — Data Source Mode Cohesion] Reworked the Auto / Learner / Static / Neither data-source modes so switching between them applies live and consistently. A single missing static sub-table (npc/object/quest/item) no longer locks the whole addon into learner mode — only the genuinely-missing store falls back. Static and Neither no longer silently fall back to learner records, switching modes now clears the per-zone quest cache, and the mode switch drives a full pin/tracker redraw through QuestieQuest:SmoothReset() (the previous redraw call imported a mis-named module and silently did nothing).
    • +
  • [QuestieLearner — Pin Refresh Latency] Collapsed a redundant second debounce stage on the live-learn pin-refresh path. Newly learned spawns now redraw within a single debounce window instead of waiting out both the NPC live-update delay and a separate pin-refresh delay, roughly halving perceived pin-update latency in the Balanced and Low presets. Also fixed a latent infinite timer re-arm that could occur once the pin flush was triggered directly.
    • +
  • [Map — Dense Pin Clustering Aggressiveness Knob] Re-implemented density-adaptive clustering for crowded kill objectives, now controlled by a new Dense pin clustering aggressiveness slider on the Advanced tab (0 = show every pin, higher = tighter consolidation where many pins share a zone). Coincident pins are always deduplicated regardless of the clustering settings, and the intentional per-zone (Sunstrider Isle) and object-icon range overrides are preserved.
    • +
  • [QuestieLearner — One Pin Per Spawn, Not Per Kill] Fixed learner kill evidence rendering a separate pin for every kill. Because kill coordinates are the player's position at kill time (and respawns carry fresh GUIDs), repeated kills at the same spot drifted just enough to dodge the exact-match dedup. The immediate learner-mode spawn builder now merges evidence within a small radius into one pin per physical spawn, and the weighted spawn-evidence merge groups kills by coordinate bucket so a single spot can actually accumulate enough evidence to clear the confidence threshold. The merge distance is exposed as a new Spawn Pin Dedup Radius slider on the Advanced tab (0 = show every distinct position, higher = fewer/tighter pins per spawn) which redraws live.
  • [Error Suppression — Debug Modes] Moved missing quest and other non-fatal database/error spam out of normal chat output and into Questie debug-critical/developer output. Fatal startup failures remain loud.
  • [Tooltip Data Precedence] Updated tooltip handling so QuestieLearner defers to AscensionDB-owned tooltip/objective data instead of hiding or replacing server-plugin data for active quests.
    • diff --git a/workflow/performance-audit-2026-06-03-FULL.md b/workflow/performance-audit-2026-06-03-FULL.md index 5dd60bb..56bb49d 100644 --- a/workflow/performance-audit-2026-06-03-FULL.md +++ b/workflow/performance-audit-2026-06-03-FULL.md @@ -6027,3 +6027,129 @@ Learner kill evidence now turns into visible spawn coordinates quickly enough to spawn pins in learner mode, and the available quest scanner now fails closed when a quest record is missing instead of flooding chat or crashing the draw thread. + +--- + +# 2026-06-05 — Data Source Mode Cohesion, Pin Refresh Latency, and Pin Clustering Knob + +## Summary + +Three related areas were addressed after the immediate-spawn work above: + +1. Switching the Data Source Mode (Auto / Learner / Static / Neither) did not + reliably take effect — including a case where it could never switch back to + the static database even across `/reload`. +2. Newly learned spawns were slow to redraw pins because two trailing-debounce + stages were stacked on the live-learn path. +3. The density-adaptive pin clustering that was previously commented out needed + re-implementing as a user-tunable knob, plus a guaranteed coincident-pin + deduplication baseline. + +## Root causes + +- **Mode lock-in.** `QuestieDB.baseDatabaseMissing` was a single global flag set + to `true` when ANY one of the four core stores (`npcData`, `objectData`, + `questData`, `itemData`) failed to load. Both the per-read DB functions and + `GetDataSourceMode()` force learner mode whenever `IsBaseDatabaseMissing()` is + true, so a single missing/format-mismatched sub-table pinned the entire addon + to learner data and survived `/reload` (the flag is recomputed identically at + load). +- **Learner leak into Static/Neither.** The per-read fallback + (`if not rawdata and learnerRecord then rawdata = learnerRecord`) ran in the + `else` branch for every non-learner mode, so Static and Neither silently used + learner records when the static DB lacked an entry. +- **Dead redraw on mode switch.** `ApplyLearnerMode` imported + `"QuestieEventHandler"` (the module is registered as `"QuestEventHandler"`) and + called `UpdateAllQuests`, which lives on the private table — so the import + returned nil and available-quest pins were never redrawn on a mode switch. +- **Stale zone cache.** `ApplyDataSourceMode` cleared the quest/npc/item/object + caches but not `zoneCache`, so per-zone quest results lingered after a switch. +- **Stacked pin-refresh debounce.** A learned kill flowed through + `liveNpcUpdateDelay` (NPC live-update flush) and THEN a separate + `pinRefreshDelay` gate before `UpdateQuest`. Because the only caller of the + pin-invalidation path is the already-debounced NPC flush, the second stage was + pure added latency (~1.5s Balanced, ~4s Low), with two independent + `pinRefreshMaxWait` caps under sustained kills. + +## What changed + +- `Database/QuestieDB.lua` + - `IsBaseDatabaseMissing()` now reports missing only when ALL four core stores + failed; added `IsStoreMissing(storeKey)` for per-store checks. + - Each read (`GetNPC` / `GetQuest` / `GetItem` / `GetObject`) gates force-learner + on its own store and only overlays a learner-record fallback in Auto mode. + - Added `ClearModeCaches()` which clears quest/item/npc/object AND zone caches. +- `Modules/QuestieLearner.lua` + - `ApplyDataSourceMode` now calls `ClearModeCaches()`. + - Split the active-quest pin flush into a debounce-gate wrapper plus a + `_DoFlushActiveQuestPins` body. The NPC live-update flush now suppresses the + redundant second debounce and force-flushes pins once, immediately. + - Hardened the flush gate so it only defers while there is genuine pending + activity (fixes an infinite timer re-arm exposed by the direct flush). +- `Modules/Options/DatabaseTab/QuestieOptionsDatabase.lua` + - Mode switches now refresh through `QuestieQuest:SmoothReset()` (clears notes + and tooltips, recalculates and redraws available quests, re-updates active + quests, refreshes the tracker). +- `Modules/Quest/QuestieQuest.lua` + - Re-implemented density-adaptive clustering, scaled by a new + `clusterDensityAggressiveness` profile knob. The intentional per-zone + (Sunstrider Isle, uiMapID 1241, range 0) and object-icon (`range * 0.2`) + overrides are preserved and still take precedence. +- `Modules/Map/QuestieMapUtils.lua` + - `CalcHotzones` now always merges coincident pins + (within `COINCIDENT_EPSILON`), independent of the clustering range, so two + icons never stack on the same spot even when clustering is disabled. +- `Modules/Options/QuestieOptionsDefaults.lua`, + `Modules/Options/AdvancedTab/QuestieOptionsAdvanced.lua` + - Added the `clusterDensityAggressiveness` default (35) and a real-time + Advanced-tab slider that redraws via `QuestieOptions:ClusterRedraw`. +- Tests + - `Tests/QuestieLearnerDataSourceMode_spec.lua`: partial-missing store handling, + Static/Neither no-leak, Auto overlay, full cache clear, SmoothReset wiring. + - `Tests/QuestieLearner_performance_spec.lua`: pins force-flush within a single + NPC live-update round and draining terminates (infinite-loop guard). + - `Tests/QuestiePinClustering_spec.lua` (new): coincident dedup, distinct-pin + preservation at range 0, range-based clustering, cross-map isolation, and + knob wiring. + +## Verification + +- `luac5.1 -p` on every changed Lua file. +- `busted` full suite: 100 successes / 4 failures (the 4 failures are the + pre-existing, unrelated `QuestieArrowAssets_spec` asset checks). + +## Result + +All four data-source modes now switch live and cohesively (and partial static +DB failures no longer trap the addon in learner mode). Newly learned spawns +redraw within a single debounce window. Dense kill objectives can be +consolidated to taste via the new aggressiveness knob, while coincident pins are +always deduplicated and the curated per-zone/object overrides remain intact. + +### Follow-up — learner pin-per-kill fix + +Learner kill evidence was rendering one pin per kill. Kill coordinates come from +the player's position at kill time and respawns carry fresh GUIDs, so repeated +kills at the same spawn drifted just enough to defeat the exact-match dedup in +`_BuildSpawnTableFromGuidEvidence` (`Database/QuestieDB.lua`) and the +full-precision grouping key in `_MergeSpawnEvidence` (`Modules/QuestieLearner.lua`). + +- `_BuildSpawnTableFromGuidEvidence` now merges evidence coords within a small + radius (distance test, not a grid bucket — avoids the cell-boundary split where + two near-identical coords land in different buckets) into one pin per spawn. +- `_MergeSpawnEvidence` now groups kills by a per-zone coordinate bucket + (`GetCoordGridForZone`) instead of exact coords, so one location accumulates + enough evidence to clear the >60% confidence threshold instead of every kill + registering as its own single-count "location". +- The `[7]` spawn path already deduped via `InsertIfNewBucket`; only the GUID + evidence (`[8]`) consumers needed the fix. +- The merge radius is user-tunable via the new `spawnDedupRadius` learner setting + (default 4.0, map-percent) and a "Spawn Pin Dedup Radius" slider on the + Advanced tab. `_BuildSpawnTableFromGuidEvidence` reads it through + `_GetSpawnDedupRadius` (0 = exact-match only / every distinct position shown); + changing it clears the NPC spawn cache and triggers a live redraw. +- Regressions added in `Tests/QuestieLearnerDataSourceMode_spec.lua`: many nearby + kills collapse to one pin, genuinely separate spawns stay distinct, radius 0 + disables proximity merge, and a larger radius widens merging. + `Tests/QuestiePinClustering_spec.lua` asserts the knob wiring (default + UI + + cache-clear redraw).