From 32c538de1571ffd6eaf33d72e09dc2d910795083 Mon Sep 17 00:00:00 2001 From: Xurkon Date: Thu, 11 Jun 2026 15:49:29 -0500 Subject: [PATCH] fix(learner): stop InjectLearnedData from corrupting saved spawns Two bugs made saved learner spawns collapse to one pin on every login: 1. The spawn zone-key migration was inverted: it ran GetAreaIdByUiMapId(1241) -> Eversong parent areaId 3430 and moved Sunstrider coords there (wrong map / NE corner), deduping distinct coords via InsertIfNewBucket along the way. Now it converts only legacy areaId keys FORWARD to the canonical uiMapId and leaves uiMapId keys untouched. Same for the [9]/[5] home-zone fields. 2. Sanitize de-duplicated coords with the flat COORD_GRID (2.0) instead of the per-zone grid, collapsing Sunstrider's tightly-packed spawns (grid 0.5). Now uses GetCoordGridForZone(zoneId). Both rewrote learned.npcs in place, persisting the damage to SavedVariables. Regression test in QuestieLearnerMultiSpawn_spec.lua. --- CHANGELOG.md | 1 + Modules/QuestieLearner.lua | 96 +++++++++++++++++++------------------- 2 files changed, 49 insertions(+), 48 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fdabe6..9cd612b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ ### Bug Fixes +- **[Learner - InjectLearnedData Was Corrupting Saved Spawns]** The real cause of learner pins collapsing to one (and the data appearing to "not persist"): `InjectLearnedData` ran on every login/redraw and **rewrote the saved learner spawns**. Its zone-key migration was inverted — it called `GetAreaIdByUiMapId(1241)`, which returns the Eversong **parent areaId 3430**, and *moved* Sunstrider's coordinates there (rendering them on the wrong map) while `InsertIfNewBucket` silently deduped distinct coords away. On top of that, `Sanitize` re-deduplicated coordinates with the flat `COORD_GRID` (2.0) instead of the per-zone grid, collapsing Sunstrider's tightly-packed spawns (its grid is 0.5). Together these turned seven saved Arcane Wraith coordinates into one pin and persisted the damage back to SavedVariables. Fixed: the migration now converts only legacy areaId keys **forward** to the canonical uiMapId (never the reverse) and leaves uiMapId keys untouched; the `[9]`/`[5]` home-zone fields are normalized the same direction; and `Sanitize` now de-dups with the correct per-zone grid. A regression test (`Tests/QuestieLearnerMultiSpawn_spec.lua`) drives `InjectLearnedData` + `GetNPC` and asserts all distinct learner spawns survive. - **[Learner - Sunstrider Shows Every Learned Pin Again]** Learner-recorded spawns on Sunstrider Isle were being collapsed to a single pin: a prior change kept clustering enabled there in learner mode, so several distinct learned coordinates (e.g. seven Arcane Wraith spots) consolidated into one icon — breaking the DB-building workflow where every learned spawn must be visible. Sunstrider (uiMapID 1241) now shows every distinct pin by default in **all** data-source modes; consolidation only happens if the player explicitly raises the Dense Pin Clustering Aggressiveness knob. (The data was correct in SavedVariables the whole time — this was purely a display/clustering regression.) - **[Tooltip - Collapse Stacked Ascension Quest-Progress Lines]** (#9) Tooltips could pile up a stack of progress lines for the same objective — `0/8`, `1/8`, `2/8`, `3/8` — because the Ascension server appends a new progress line on every objective update instead of replacing the old one. A new always-on pass (`DedupeAscensionProgressLines`) collapses each stack to its single most-progressed line. It only ever hides a line when the same tooltip holds another progress line with the same objective text and denominator, so it never touches other addons' tooltip lines or Questie's own single-line objectives (unit-tested). This is separate from the opt-in "Hide Ascension quest progress lines" option (#16), which removes such lines entirely. - **[Map - Completed Quest Pins Stuck On The World Map Until Reload]** (#9) A completed quest/objective's pin could stay on the world map (while the minimap correctly cleared it) until a `/reload`. Cause: a draw-queue race. The world-map and minimap draw queues are processed independently in `ProcessQueue`, so when a frame was unloaded while still queued (e.g. an objective that completes the same tick its pins were queued), the frame's minimap-queue entry could trigger the deferred `Unload` first — removing both icons and clearing the unload flag — and then a later map-queue entry would re-add the world-map icon. Because `Unload` had already nil'd the `questIdFrames`/`_G` reference, that re-added pin was orphaned and survived until `/reload`. `Unload` now drops the frame's pending draw-queue entries (new `QuestieMap:DequeueFrameDrawCalls`), so an unloaded frame can never be re-added to a map — keeping both maps in sync. diff --git a/Modules/QuestieLearner.lua b/Modules/QuestieLearner.lua index 2dca8bb..0b68dc1 100644 --- a/Modules/QuestieLearner.lua +++ b/Modules/QuestieLearner.lua @@ -2571,7 +2571,11 @@ function QuestieLearner:Sanitize(data) if data[coordKey] and type(data[coordKey]) == "table" then for zoneId, coords in pairs(data[coordKey]) do local unique = {} - local grid = COORD_GRID -- use standard for static sanitization + -- Use the SAME per-zone grid the coords were stored with. The flat + -- COORD_GRID (2.0) is too coarse for tightly-packed zones like Sunstrider + -- Isle, whose grid is 0.5 — sanitizing at 2.0 collapsed distinct learned + -- spawns into fewer pins on every InjectLearnedData. + local grid = GetCoordGridForZone(zoneId) for _, c in ipairs(coords) do local bx, by = floor(c[1] / grid) * grid, floor(c[2] / grid) * grid local key = bx .. "," .. by @@ -2713,32 +2717,30 @@ function QuestieLearner:InjectLearnedData() end end - -- Migration: fix spawn zone keys that were stored as uiMapId instead of areaId. - -- Before the GetZoneId() fix, kills on maps like Sunstrider Isle (uiMapId 1241) - -- were stored under key 1241 instead of the correct areaId 3431. - -- Convert any uiMapId keys to areaId using ZoneDB. + -- Migration: normalize legacy areaId spawn keys to the canonical uiMapId render key. + -- The pin pipeline (DrawWorldIcon / HBD) keys spawns by uiMapId (Sunstrider 1241, + -- Eversong 1941). Some old learner rows stored coords under the AREA id (3431/3430) + -- instead, so convert those forward to the uiMapId via NormalizeSpawnZoneKey. + -- + -- CRITICAL: keys that are ALREADY uiMapIds (1241, 1941) must be left untouched. The + -- previous version of this code did the reverse — it ran GetAreaIdByUiMapId(1241), + -- which returns the Eversong PARENT areaId 3430, and moved Sunstrider coords there + -- (rendering them on the wrong map / NE corner) while InsertIfNewBucket silently + -- deduped distinct coords down. That corrupted the saved learner data on every + -- InjectLearnedData. Never convert a uiMapId to an areaId here. local zonesFixed = 0 for npcId, data in pairs(learned.npcs) do if data[7] then local zonesToMigrate = {} for zoneKey, coords in pairs(data[7]) do - local originalZoneKey = zoneKey - zoneKey = NormalizeSpawnZoneKey(zoneKey) - -- Older Ascension learner data stored native Sunstrider coords - -- under parent areaId 3430 while [9] still identified the row as - -- Sunstrider. Move those coords to areaId 3431 so they render on - -- uiMapId 1241 instead of Eversong. - if originalZoneKey == 3430 and IsSunstriderNativeZone(data[9]) then - zonesToMigrate[originalZoneKey] = 3431 - end - -- If zoneKey looks like a uiMapId (a map ID rather than an areaId), - -- ZoneDB:GetAreaIdByUiMapId will return the corresponding areaId. - -- If it returns nil, zoneKey is already an areaId — no migration needed. - -- Skip very common areaIds that happen to look like small numbers. - if not zonesToMigrate[originalZoneKey] and ZoneDB and ZoneDB.GetAreaIdByUiMapId then - local maybeAreaId = ZoneDB:GetAreaIdByUiMapId(zoneKey) - if maybeAreaId and maybeAreaId ~= zoneKey then - zonesToMigrate[originalZoneKey] = maybeAreaId + -- Legacy Sunstrider coords mis-stored under the Eversong parent areaId 3430 + -- belong on Sunstrider's uiMapId 1241, not Eversong's 1941. + if zoneKey == 3430 and IsSunstriderNativeZone(data[9]) then + zonesToMigrate[zoneKey] = 1241 + else + local normalized = NormalizeSpawnZoneKey(zoneKey) + if normalized and normalized ~= zoneKey then + zonesToMigrate[zoneKey] = normalized end end end @@ -2755,20 +2757,17 @@ function QuestieLearner:InjectLearnedData() end end end - -- Same migration for object spawn data (field 4) + -- Same migration for object spawn data (field 4) — areaId -> uiMapId, never the reverse. for objId, data in pairs(learned.objects) do if data[4] then local zonesToMigrate = {} for zoneKey, coords in pairs(data[4]) do - local originalZoneKey = zoneKey - zoneKey = NormalizeSpawnZoneKey(zoneKey) - if originalZoneKey == 3430 and IsSunstriderNativeZone(data[5]) then - zonesToMigrate[originalZoneKey] = 3431 - end - if not zonesToMigrate[originalZoneKey] and ZoneDB and ZoneDB.GetAreaIdByUiMapId then - local maybeAreaId = ZoneDB:GetAreaIdByUiMapId(zoneKey) - if maybeAreaId and maybeAreaId ~= zoneKey then - zonesToMigrate[originalZoneKey] = maybeAreaId + if zoneKey == 3430 and IsSunstriderNativeZone(data[5]) then + zonesToMigrate[zoneKey] = 1241 + else + local normalized = NormalizeSpawnZoneKey(zoneKey) + if normalized and normalized ~= zoneKey then + zonesToMigrate[zoneKey] = normalized end end end @@ -2786,36 +2785,37 @@ function QuestieLearner:InjectLearnedData() end end if zonesFixed > 0 then - Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] Migrated", zonesFixed, "spawn zone keys from uiMapId to areaId") + Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] Migrated", zonesFixed, "legacy areaId spawn zone keys to uiMapId") end - -- Also fix [9] zone field for NPCs and [5] zone field for Objects - -- that were stored as uiMapId instead of areaId (e.g. 1241 → 3431). + -- Normalize the [9] (NPC) / [5] (Object) home-zone field to the canonical uiMapId, + -- matching the spawn keys and how LearnNPC/LearnObject now store it. Legacy areaIds + -- (e.g. 3431) become uiMapIds (1241); values that are already uiMapIds are left alone. + -- (The previous version forced these to the Eversong parent areaId 3430, which broke + -- IsSunstriderNativeZone detection for Sunstrider rows.) local fieldsFixed = 0 for npcId, data in pairs(learned.npcs) do - if type(data[9]) == "number" and ZoneDB and ZoneDB.GetAreaIdByUiMapId then - local normalizedZone = NormalizeSpawnZoneKey(data[9]) - local maybeAreaId = ZoneDB:GetAreaIdByUiMapId(normalizedZone) - if maybeAreaId and maybeAreaId ~= data[9] then - Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] NPC", npcId, "zone field [9]", data[9], "->", maybeAreaId) - data[9] = maybeAreaId + if type(data[9]) == "number" then + local normalized = NormalizeSpawnZoneKey(data[9]) + if normalized and normalized ~= data[9] then + Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] NPC", npcId, "zone field [9]", data[9], "->", normalized) + data[9] = normalized fieldsFixed = fieldsFixed + 1 end end end for objId, data in pairs(learned.objects) do - if type(data[5]) == "number" and ZoneDB and ZoneDB.GetAreaIdByUiMapId then - local normalizedZone = NormalizeSpawnZoneKey(data[5]) - local maybeAreaId = ZoneDB:GetAreaIdByUiMapId(normalizedZone) - if maybeAreaId and maybeAreaId ~= data[5] then - Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] Object", objId, "zone field [5]", data[5], "->", maybeAreaId) - data[5] = maybeAreaId + if type(data[5]) == "number" then + local normalized = NormalizeSpawnZoneKey(data[5]) + if normalized and normalized ~= data[5] then + Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] Object", objId, "zone field [5]", data[5], "->", normalized) + data[5] = normalized fieldsFixed = fieldsFixed + 1 end end end if fieldsFixed > 0 then - Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] Fixed", fieldsFixed, "zone fields from uiMapId to areaId") + Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] Normalized", fieldsFixed, "home-zone fields to uiMapId") end -- Purge player-spawned totems from learned NPCs (they are not real world spawns)