Narcasung/Questie-X
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

feat(learner): exclude critters; fix Sunstrider object mis-keyed to areaId 3430

Browse Source 
- Critters are never quest-relevant. Added a static CRITTER_NPC_SET (common
  classic critters) plus runtime detection via UnitCreatureType on
  mouseover/target (covers Ascension custom critters). LearnNPC and the kill
  handler skip critters; InjectLearnedData purges already-recorded ones; and
  critters are purged on sight when a unit token is available.
- Object/NPC spawns mis-stored under Eversong parent areaId 3430 (e.g. object
  180516 'Shrine of Dath'Remar' on Sunstrider) now migrate to uiMapId 1241,
  along with the [9]/[5] home-zone field. Legit Eversong data is keyed 1941,
  so any 3430 key is mis-stored Sunstrider data.

Verified in Tests/QuestieLearnerCritterPurge_spec.lua.
This commit is contained in:
Xurkon
2026-06-11 16:29:41 -05:00
parent 32c538de15
commit 7bd14b80fc
2 changed files with 85 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files
CHANGELOG.md
+1
View File
@@ -41,6 +41,7 @@
### Bug Fixes
- **[Learner - Exclude Critters & Fix Sunstrider Object Mis-Key]** Critters (Rabbit, Deer, Sheep, Cow, Cat, …) are never quest-relevant but were being recorded by the learner when killed, polluting the DB. The learner now refuses to record critters (a static list of common classic critters plus runtime detection via `UnitCreatureType` for Ascension's custom critters, flagged the moment a unit token is available on mouseover/target), purges any already-recorded critters on `InjectLearnedData` and on sight, and skips them in every learn path. Also fixed object spawns mis-stored under the Eversong parent areaID `3430` (e.g. object 180516 "Shrine of Dath'Remar", which is on Sunstrider): the migration now moves any `3430`-keyed spawn — and the matching home-zone field — to Sunstrider's uiMapID `1241`, since legitimate Eversong data is always keyed by `1941`.
- **[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.
Modules/QuestieLearner.lua
+84 -6
View File
@@ -189,6 +189,7 @@ local UnitLevel = UnitLevel
local UnitFactionGroup = UnitFactionGroup
local UnitReaction = UnitReaction
local UnitCreatureFamily = UnitCreatureFamily
local UnitCreatureType = UnitCreatureType
local GetRealZoneText = GetRealZoneText
local GetTitleText = GetTitleText
local GetObjectiveText = GetObjectiveText
@@ -1435,12 +1436,63 @@ local PLAYER_SPAWNED_NPC_SET = {
[1107398] = true, -- Stoneclaw Totem V
}
-- Critters are never quest-relevant, so they should never enter the learner DB.
-- This static set covers the common classic critters; Ascension's custom critters are
-- caught at runtime via UnitCreatureType (see _NoteUnitCreatureType / _Learner.critterIds).
local CRITTER_NPC_SET = {
[721] = true, -- Rabbit
[883] = true, -- Deer
[1933] = true, -- Sheep
[2442] = true, -- Cow
[6368] = true, -- Cat
[2620] = true, -- Prairie Dog
[4953] = true, -- Cat (Wisp)
[9700] = true, -- Squirrel
[5113] = true, -- Mouse
[5114] = true, -- Rat
[5115] = true, -- Snake
[5116] = true, -- Toad
[2914] = true, -- Frog
[385] = true, -- Small Frog
[890] = true, -- Crab
[299] = true, -- Chicken
[620] = true, -- Chicken
[2719] = true, -- Battered Rabbit
}
-- Runtime-discovered critters (npcId -> true), populated by _NoteUnitCreatureType when a
-- unit token is available, so Ascension's custom critters are excluded even if not in the
-- static set above.
_Learner.critterIds = _Learner.critterIds or {}
local function IsCritterNpc(npcId)
return npcId and (CRITTER_NPC_SET[npcId] or _Learner.critterIds[npcId]) or false
end
-- Records the creature type for a unit we currently have a token for (mouseover/target).
-- When the unit is a Critter, remember its npcId and purge any learner data for it, since
-- critters are never quest-relevant and only pollute the learner DB.
local function _NoteUnitCreatureType(unit, npcId)
if not unit or not npcId or npcId <= 0 then return end
if not (UnitCreatureType and UnitExists and UnitExists(unit)) then return end
if UnitCreatureType(unit) == "Critter" then
_Learner.critterIds[npcId] = true
local ld = Questie.dbLearner and Questie.dbLearner.global
if ld and ld.npcs and ld.npcs[npcId] then
ld.npcs[npcId] = nil
Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] Removed critter from learner data:", npcId)
end
end
end
function QuestieLearner:LearnNPC(npcId, name, level, subName, npcFlags, factionString, spawnX, spawnY, spawnZoneId)
if not self:IsEnabled() then return end
if not Questie.dbLearner.global.settings.learnNpcs then return end
if not npcId or npcId <= 0 then return end
-- Never learn player-spawned totems
if PLAYER_SPAWNED_NPC_SET[npcId] then return end
-- Never learn critters (not quest-relevant)
if IsCritterNpc(npcId) then return end
-- Use provided spawn coords (e.g. from kill event) or fall back to current player position.
-- Normalize area IDs → map IDs immediately so all storage uses the same key space
@@ -2733,9 +2785,11 @@ function QuestieLearner:InjectLearnedData()
if data[7] then
local zonesToMigrate = {}
for zoneKey, coords in pairs(data[7]) do
-- 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
-- Any spawn under the Eversong PARENT areaId 3430 is mis-stored Sunstrider
-- data: legitimate Eversong coords are keyed by uiMapId 1941, never 3430 (3430
-- only appears via the old uiMapId->areaId bug, which mangled Sunstrider's
-- 1241). Move it to Sunstrider's uiMapId 1241.
if zoneKey == 3430 then
zonesToMigrate[zoneKey] = 1241
else
local normalized = NormalizeSpawnZoneKey(zoneKey)
@@ -2762,7 +2816,9 @@ function QuestieLearner:InjectLearnedData()
if data[4] then
local zonesToMigrate = {}
for zoneKey, coords in pairs(data[4]) do
if zoneKey == 3430 and IsSunstriderNativeZone(data[5]) then
-- Same rule as NPCs: spawns under Eversong parent areaId 3430 are mis-stored
-- Sunstrider data (e.g. object 180516 "Shrine of Dath'Remar") -> uiMapId 1241.
if zoneKey == 3430 then
zonesToMigrate[zoneKey] = 1241
else
local normalized = NormalizeSpawnZoneKey(zoneKey)
@@ -2796,7 +2852,7 @@ function QuestieLearner:InjectLearnedData()
local fieldsFixed = 0
for npcId, data in pairs(learned.npcs) do
if type(data[9]) == "number" then
local normalized = NormalizeSpawnZoneKey(data[9])
local normalized = (data[9] == 3430) and 1241 or 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
@@ -2806,7 +2862,7 @@ function QuestieLearner:InjectLearnedData()
end
for objId, data in pairs(learned.objects) do
if type(data[5]) == "number" then
local normalized = NormalizeSpawnZoneKey(data[5])
local normalized = (data[5] == 3430) and 1241 or 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
@@ -2833,6 +2889,12 @@ function QuestieLearner:InjectLearnedData()
learned.npcs[npcId] = nil
purgedNpcs = purgedNpcs + 1
Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] Purged player-spawned NPC", npcId, data[1] or "?")
elseif IsCritterNpc(npcId) then
-- Critters are never quest-relevant; drop any that were recorded before
-- critter filtering existed.
learned.npcs[npcId] = nil
purgedNpcs = purgedNpcs + 1
Questie:Debug(Questie.DEBUG_INFO, "[QuestieLearner] Purged critter NPC", npcId, data[1] or "?")
elseif data[7] then
-- Check for empty spawn table (no coords at all = stale learner artifact).
-- Only purge if the NPC has no other useful state — keep entries that
@@ -3413,6 +3475,11 @@ function QuestieLearner:OnMouseoverUnit()
end
if unitType ~= "Creature" and unitType ~= "Vehicle" then return end
-- Flag (and purge) critters the moment we have a unit token for them, so Ascension's
-- custom critters are excluded going forward and any already-recorded critter is removed.
_NoteUnitCreatureType("mouseover", entityId)
if IsCritterNpc(entityId) then return end
-- Only learn this NPC if it carries the questgiver flag OR if it is already
-- known in the database as a starter/finisher (so we can update its coords).
local npcFlags = UnitNPCFlags and UnitNPCFlags("mouseover") or 0
@@ -3480,6 +3547,10 @@ function QuestieLearner:OnTargetChanged()
end
if unitType ~= "Creature" and unitType ~= "Vehicle" then return end
-- Flag (and purge) critters as soon as we target one.
_NoteUnitCreatureType("target", entityId)
if IsCritterNpc(entityId) then return end
local level = UnitLevel("target")
_Learner.guidNpcCache = _Learner.guidNpcCache or {}
@@ -4252,6 +4323,13 @@ function QuestieLearner:OnCombatLogEvent(timestamp, eventType, srcGUID, srcName,
if not npcId or npcId <= 0 then return end
-- If we have the dead unit as our current target, note its creature type so critters
-- get flagged/purged; then skip recording any critter kill entirely.
if UnitGUID and UnitGUID("target") == dstGUID then
_NoteUnitCreatureType("target", npcId)
end
if IsCritterNpc(npcId) then return end
-- Determine whether this kill is "credited" to us (our own/party kill we engaged)
-- BEFORE using it to decide whether to record our position. This MUST come first:
-- previously `credited` was read while still nil (defined further below), so the