fix(learner): defer to AscensionDB curated spawns for owned NPCs

Mana Wyrm 15274 (quest 8325) pins rendered in the NE corner because the
learner's Sunstrider spawns leaked into the curated AscensionDB coords.
Root causes:
- In learner mode GetNPC uses rawdata = learner record, and _MergeOverride
  deep-merges spawn tables, so the learner's bad zone (1445 @ 54,12)
  survived since AscensionDB had no entry to displace it.
- The injection guards used IsAscensionProtected(), which returns false in
  learner mode, so the learner overrode AscensionDB-owned spawns. Commit
  0f20ea8 had also added a (not learnerLiveMode) bypass to the
  _MergeSpawnEvidence Sunstrider guard, re-enabling the e80a008 regression.

Fixes:
- QuestieDB.GetNPC: for AscensionDB-owned NPC spawns (direct
  ascensionOverrideKeys check), use ONLY the curated override spawns and
  discard the merged learner zones.
- New mode-independent AscensionOwnsNpcSpawns() helper; used by the
  InjectLearnedData spawn restore and the _MergeSpawnEvidence guard
  (removed the (not learnerLiveMode) bypass).

Regression test asserts GetNPC returns only the curated 1241 coords and
drops the learner 1445 zone.
This commit is contained in:
Xurkon
2026-06-08 17:53:16 -05:00
parent a525813f12
commit 011cdb58a9
3 changed files with 43 additions and 5 deletions
+1
View File
@@ -32,6 +32,7 @@
- **[QuestieDB - Auto Mode Learner Enrichment]** In `auto` data-source mode, `QuestieDB.GetQuest` now overlays the SavedVariables learner payload on top of the static DB record, filling only fields the static record leaves `nil` and deep-merging `objectives`/`objIndex`. This fixes quests like 8325 where the shipped DB carries only a stub record but QuestieLearner holds the real objective mapping — previously the stub blocked the learned objectives from ever surfacing. Learner data never clobbers a present static value, so it strictly enriches and cannot regress shipped data.
- **[QuestieLearner - Accept-Time Objective Pin Seeding]** `OnQuestAccepted` now seeds objective pins directly from the SavedVariables learner payload (`objIndex`) the moment a quest is accepted in learner mode, instead of waiting for quest-log text sync. This makes quests like 8325 spawn pins immediately on accept when the objective mapping already exists. The accept-time ID lookup was also widened from `killcredit` to also cover `monster` objectives, and now falls back to `objData.Id` when no `IdList` is present, catching cases where the quest-log text does not normalize cleanly to the NPC name.
- **[QuestieLearner - Persisted Spawn Restore on Load]** `InjectLearnedData` stripped each learned NPC's spawn field (`[7]`) when injecting into `QuestieDB.npcDataOverrides`, deferring to `_MergeSpawnEvidence`. But that promoter only runs on **live kill evidence**, so spawns learned in a prior session never returned to the queryable DB on `/reload` — pins for a freshly accepted quest (e.g. 8325 → Mana Wyrm 15274 on Sunstrider) stayed missing until the player re-killed the mob. The saved spawns are now restored into the override at injection time under their **native uiMapId keys**, captured from a snapshot taken *before* the internal uiMapId→areaId migration runs. This matches exactly what the live kill path stores (e.g. Sunstrider `1241`), so pins render on the correct map and HBD `isSameZoneSpace` shows them on the Eversong map too. The restore deliberately does **not** convert keys through areaId — uiMapId `1241` resolves to the Eversong parent areaId `3430` → Eversong map `1941`, which had been placing Sunstrider pins in the wrong (NE) corner. Gated by `IsAscensionProtected` so curated AscensionDB coords are never overwritten (always restored in learner mode; in auto mode only non-curated NPCs). Regression introduced by commit `7ce0cdc`.
- **[QuestieLearner - AscensionDB Spawn Deference (Sunstrider pin placement)]** For NPCs whose spawn data AscensionDB hand-curates (e.g. Sunstrider's Mana Wyrm 15274 on quest 8325), the learner now **defers entirely** to the curated coords instead of mixing in its own — even in learner mode. The learner's Sunstrider coordinates are frequently stored under the wrong map/zone (a kill mis-recorded under zone `1445` rendered as a pin in the NE corner of the map), and `_MergeOverride` deep-merges spawn tables, so in learner mode (where `rawdata` is the learner record) those bad zones survived because AscensionDB had no matching entry to displace them. Fixes: (1) `QuestieDB.GetNPC` now uses the curated override spawns **only** for AscensionDB-owned NPCs, discarding merged learner zones; (2) `InjectLearnedData`'s spawn restore and (3) `_MergeSpawnEvidence`'s Sunstrider guard both use a new mode-independent `AscensionOwnsNpcSpawns()` check instead of `IsAscensionProtected()` (which returns false in learner mode). This restores the protection commit `e80a008` documented and commit `0f20ea8` had silently bypassed with a `(not learnerLiveMode)` condition.
- **[Options - Tab Table Vararg Typo]** Five Options files initialized their tab tables with `{ ... }` instead of `{}` (`QuestieOptions.tabs` plus the Arrow, General, Keybinds, and Tracker tabs). At chunk scope WoW passes `...` = `(addonName, addonTable)`, so each table was seeded with two stray junk entries (`[1]="Questie"`, `[2]=<addonTable>`) instead of being empty, and the `{ ... }` expression is also a hard parse error under strict Lua 5.0. Changed all five to `{}`.
### Tooltip
+19
View File
@@ -2314,6 +2314,25 @@ function QuestieDB:GetNPC(npcId)
end
end
-- AscensionDB hand-curates spawn coordinates for Ascension-specific zones
-- (e.g. Sunstrider's Mana Wyrm 15274). When AscensionDB owns this NPC's spawns,
-- use ONLY those curated coords and discard the learner-record zones that
-- _MergeOverride deep-merged in: the learner's Sunstrider coords are frequently
-- stored under the wrong map/zone (Sunstrider 1241 vs Eversong space), which
-- otherwise leaks in as pins in the wrong corner of the map. In learner mode
-- rawdata IS the learner record, so the bad zone survives the merge because
-- AscensionDB has no entry to displace it — this check removes it. Direct
-- ascensionOverrideKeys lookup (NOT IsAscensionProtected, which returns false
-- in learner mode).
if override and QuestieDB.ascensionOverrideKeys and QuestieDB.ascensionOverrideKeys["NPC"]
and QuestieDB.ascensionOverrideKeys["NPC"][npcId]
and QuestieDB.ascensionOverrideKeys["NPC"][npcId][7] then
local curated = override[7] or override.spawns
if curated and next(curated) then
npc.spawns = CopySpawnTable(curated)
end
end
local friendlyToFaction = npc.friendlyToFaction
npc.friendly = (not friendlyToFaction) and true or factionReactions[friendlyToFaction]
+23 -5
View File
@@ -83,6 +83,20 @@ local function IsAscensionProtected(dbType, id, key)
return protected and protected[key] == true
end
-- Direct AscensionDB-ownership check that is NOT bypassed in learner/none mode.
-- IsAscensionProtected() returns false in learner mode so the learner can override
-- most fields — but spawn data ([7]) for AscensionDB-curated NPCs (e.g. Sunstrider's
-- Mana Wyrm) must NEVER be overridden by learner coords, which are often in the wrong
-- map/zone space and produce pins in the wrong corner of the map. Use this for the
-- spawn-injection guards so AscensionDB always wins for owned NPCs regardless of mode.
local function AscensionOwnsNpcSpawns(npcId)
return QuestieDB
and QuestieDB.ascensionOverrideKeys
and QuestieDB.ascensionOverrideKeys["NPC"]
and QuestieDB.ascensionOverrideKeys["NPC"][npcId]
and QuestieDB.ascensionOverrideKeys["NPC"][npcId][7] == true
end
local function HasAscensionQuestObjectiveData(questId)
return IsAscensionProtected("QUEST", questId, 10)
end
@@ -1690,9 +1704,6 @@ local function _MergeSpawnEvidence(npcId)
-- Confidence threshold is bypassed for Sunstrider because spawn points are
-- distributed across 5+ locations — no single point ever reaches 60% of kills.
local isSunstrider = IsSunstriderNativeZone(topEvidence.zoneId)
local learnerLiveMode = QuestieLearner
and QuestieLearner.IsLearnerLiveEnabled
and QuestieLearner:IsLearnerLiveEnabled()
local confidenceThreshold = isSunstrider and 0 or 60
-- Only override if > confidence threshold AND spawn differs from static DB
@@ -1714,7 +1725,11 @@ local function _MergeSpawnEvidence(npcId)
-- AscensionDB coords with in-game kill evidence, causing wrong pin counts.
-- REGRESSION NOTE: If AscensionDB protection check is removed or disabled,
-- learner pins will reappear at wrong locations. Do not remove this guard.
if (not learnerLiveMode) and IsAscensionProtected("NPC", npcId, 7) then
-- This MUST be unconditional — commit 0f20ea8 added a `(not learnerLiveMode)`
-- bypass that re-enabled the bug in learner mode (Sunstrider Mana Wyrm kills
-- overwrote AscensionDB's curated coords). Use the mode-independent ownership
-- check, not IsAscensionProtected (which returns false in learner mode).
if AscensionOwnsNpcSpawns(npcId) then
Questie:Debug(Questie.DEBUG_INFO,
"[QuestieLearner] _MergeSpawnEvidence: npcId", npcId,
"Sunstrider zone but AscensionDB owns spawns — skipping learner injection")
@@ -2824,7 +2839,10 @@ function QuestieLearner:InjectLearnedData()
-- auto mode only non-curated NPCs are restored. Deep-merged via InsertIfNewBucket.
local realNpcId = nid or npcId
local nativeSpawns = nativeNpcSpawns[realNpcId]
if nativeSpawns and not IsAscensionProtected("NPC", realNpcId, 7) then
-- Use the mode-independent ownership check: AscensionDB-curated NPC spawns
-- (e.g. Sunstrider Mana Wyrm) must never be overlaid with learner coords, even
-- in learner mode where IsAscensionProtected would return false.
if nativeSpawns and not AscensionOwnsNpcSpawns(realNpcId) then
local ovr = QuestieDB.npcDataOverrides[realNpcId]
ovr[7] = ovr[7] or {}
for zoneId, coords in pairs(nativeSpawns) do