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.
This commit is contained in:
@@ -41,6 +41,7 @@
|
|||||||
|
|
||||||
### Bug Fixes
|
### 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.)
|
- **[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.
|
- **[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.
|
- **[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.
|
||||||
|
|||||||
+48
-48
@@ -2571,7 +2571,11 @@ function QuestieLearner:Sanitize(data)
|
|||||||
if data[coordKey] and type(data[coordKey]) == "table" then
|
if data[coordKey] and type(data[coordKey]) == "table" then
|
||||||
for zoneId, coords in pairs(data[coordKey]) do
|
for zoneId, coords in pairs(data[coordKey]) do
|
||||||
local unique = {}
|
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
|
for _, c in ipairs(coords) do
|
||||||
local bx, by = floor(c[1] / grid) * grid, floor(c[2] / grid) * grid
|
local bx, by = floor(c[1] / grid) * grid, floor(c[2] / grid) * grid
|
||||||
local key = bx .. "," .. by
|
local key = bx .. "," .. by
|
||||||
@@ -2713,32 +2717,30 @@ function QuestieLearner:InjectLearnedData()
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
-- Migration: fix spawn zone keys that were stored as uiMapId instead of areaId.
|
-- Migration: normalize legacy areaId spawn keys to the canonical uiMapId render key.
|
||||||
-- Before the GetZoneId() fix, kills on maps like Sunstrider Isle (uiMapId 1241)
|
-- The pin pipeline (DrawWorldIcon / HBD) keys spawns by uiMapId (Sunstrider 1241,
|
||||||
-- were stored under key 1241 instead of the correct areaId 3431.
|
-- Eversong 1941). Some old learner rows stored coords under the AREA id (3431/3430)
|
||||||
-- Convert any uiMapId keys to areaId using ZoneDB.
|
-- 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
|
local zonesFixed = 0
|
||||||
for npcId, data in pairs(learned.npcs) do
|
for npcId, data in pairs(learned.npcs) do
|
||||||
if data[7] then
|
if data[7] then
|
||||||
local zonesToMigrate = {}
|
local zonesToMigrate = {}
|
||||||
for zoneKey, coords in pairs(data[7]) do
|
for zoneKey, coords in pairs(data[7]) do
|
||||||
local originalZoneKey = zoneKey
|
-- Legacy Sunstrider coords mis-stored under the Eversong parent areaId 3430
|
||||||
zoneKey = NormalizeSpawnZoneKey(zoneKey)
|
-- belong on Sunstrider's uiMapId 1241, not Eversong's 1941.
|
||||||
-- Older Ascension learner data stored native Sunstrider coords
|
if zoneKey == 3430 and IsSunstriderNativeZone(data[9]) then
|
||||||
-- under parent areaId 3430 while [9] still identified the row as
|
zonesToMigrate[zoneKey] = 1241
|
||||||
-- Sunstrider. Move those coords to areaId 3431 so they render on
|
else
|
||||||
-- uiMapId 1241 instead of Eversong.
|
local normalized = NormalizeSpawnZoneKey(zoneKey)
|
||||||
if originalZoneKey == 3430 and IsSunstriderNativeZone(data[9]) then
|
if normalized and normalized ~= zoneKey then
|
||||||
zonesToMigrate[originalZoneKey] = 3431
|
zonesToMigrate[zoneKey] = normalized
|
||||||
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
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
@@ -2755,20 +2757,17 @@ function QuestieLearner:InjectLearnedData()
|
|||||||
end
|
end
|
||||||
end
|
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
|
for objId, data in pairs(learned.objects) do
|
||||||
if data[4] then
|
if data[4] then
|
||||||
local zonesToMigrate = {}
|
local zonesToMigrate = {}
|
||||||
for zoneKey, coords in pairs(data[4]) do
|
for zoneKey, coords in pairs(data[4]) do
|
||||||
local originalZoneKey = zoneKey
|
if zoneKey == 3430 and IsSunstriderNativeZone(data[5]) then
|
||||||
zoneKey = NormalizeSpawnZoneKey(zoneKey)
|
zonesToMigrate[zoneKey] = 1241
|
||||||
if originalZoneKey == 3430 and IsSunstriderNativeZone(data[5]) then
|
else
|
||||||
zonesToMigrate[originalZoneKey] = 3431
|
local normalized = NormalizeSpawnZoneKey(zoneKey)
|
||||||
end
|
if normalized and normalized ~= zoneKey then
|
||||||
if not zonesToMigrate[originalZoneKey] and ZoneDB and ZoneDB.GetAreaIdByUiMapId then
|
zonesToMigrate[zoneKey] = normalized
|
||||||
local maybeAreaId = ZoneDB:GetAreaIdByUiMapId(zoneKey)
|
|
||||||
if maybeAreaId and maybeAreaId ~= zoneKey then
|
|
||||||
zonesToMigrate[originalZoneKey] = maybeAreaId
|
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
@@ -2786,36 +2785,37 @@ function QuestieLearner:InjectLearnedData()
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
if zonesFixed > 0 then
|
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
|
end
|
||||||
|
|
||||||
-- Also fix [9] zone field for NPCs and [5] zone field for Objects
|
-- Normalize the [9] (NPC) / [5] (Object) home-zone field to the canonical uiMapId,
|
||||||
-- that were stored as uiMapId instead of areaId (e.g. 1241 → 3431).
|
-- 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
|
local fieldsFixed = 0
|
||||||
for npcId, data in pairs(learned.npcs) do
|
for npcId, data in pairs(learned.npcs) do
|
||||||
if type(data[9]) == "number" and ZoneDB and ZoneDB.GetAreaIdByUiMapId then
|
if type(data[9]) == "number" then
|
||||||
local normalizedZone = NormalizeSpawnZoneKey(data[9])
|
local normalized = NormalizeSpawnZoneKey(data[9])
|
||||||
local maybeAreaId = ZoneDB:GetAreaIdByUiMapId(normalizedZone)
|
if normalized and normalized ~= data[9] then
|
||||||
if maybeAreaId and maybeAreaId ~= data[9] then
|
Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] NPC", npcId, "zone field [9]", data[9], "->", normalized)
|
||||||
Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] NPC", npcId, "zone field [9]", data[9], "->", maybeAreaId)
|
data[9] = normalized
|
||||||
data[9] = maybeAreaId
|
|
||||||
fieldsFixed = fieldsFixed + 1
|
fieldsFixed = fieldsFixed + 1
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
for objId, data in pairs(learned.objects) do
|
for objId, data in pairs(learned.objects) do
|
||||||
if type(data[5]) == "number" and ZoneDB and ZoneDB.GetAreaIdByUiMapId then
|
if type(data[5]) == "number" then
|
||||||
local normalizedZone = NormalizeSpawnZoneKey(data[5])
|
local normalized = NormalizeSpawnZoneKey(data[5])
|
||||||
local maybeAreaId = ZoneDB:GetAreaIdByUiMapId(normalizedZone)
|
if normalized and normalized ~= data[5] then
|
||||||
if maybeAreaId and maybeAreaId ~= data[5] then
|
Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] Object", objId, "zone field [5]", data[5], "->", normalized)
|
||||||
Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] Object", objId, "zone field [5]", data[5], "->", maybeAreaId)
|
data[5] = normalized
|
||||||
data[5] = maybeAreaId
|
|
||||||
fieldsFixed = fieldsFixed + 1
|
fieldsFixed = fieldsFixed + 1
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
if fieldsFixed > 0 then
|
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
|
end
|
||||||
|
|
||||||
-- Purge player-spawned totems from learned NPCs (they are not real world spawns)
|
-- Purge player-spawned totems from learned NPCs (they are not real world spawns)
|
||||||
|
|||||||
Reference in New Issue
Block a user