From 1532b70af3b92e46d0572910ffd4657b84958907 Mon Sep 17 00:00:00 2001 From: Xurkon Date: Fri, 5 Jun 2026 22:38:11 -0500 Subject: [PATCH] docs: update changelogs and performance audit --- CHANGELOG.md | 4 +- docs/changelog.html | 4 +- release_notes.txt | 41 + workflow/performance-audit-2026-06-03-FULL.md | 6029 +++++++++++++++++ 4 files changed, 6076 insertions(+), 2 deletions(-) create mode 100644 release_notes.txt create mode 100644 workflow/performance-audit-2026-06-03-FULL.md diff --git a/CHANGELOG.md b/CHANGELOG.md index ebe3ee8..d8cce32 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - **[QuestieLearner - Kill/Pin Refresh Throttling]** Debounced learner-triggered map-pin refreshes so heavy kill streaks do not redraw pins on every event. Added a maximum wait cap so batched updates still flush predictably instead of being pushed out forever by constant activity. - **[QuestieLearner - Bystander Kill Suppression]** Changed visible nearby `UNIT_DIED` handling so kills from other players can update short-lived correlation evidence without immediately running full learner injection or pin refresh work. - **[QuestieLearner - PARTY_KILL Event-Order Fix]** Fixed an edge case where a `UNIT_DIED` debounce entry could suppress a later authoritative `PARTY_KILL` for the same GUID. The debounce now tracks event type and allows the player's/group's kill event through while still suppressing true duplicates. +- **[QuestieLearner - Immediate Spawn Pin Rendering]** Learner kill evidence now becomes spawn coordinates immediately in learner mode so learned NPC kills can spawn map pins without waiting for the later confidence merge path. - **[QuestieLearner - Live Performance Options]** Added Advanced-tab controls for learner intensity, pin refresh delay, maximum pin refresh wait, minimum kills before learned pins, and live NPC update delay so users can tune the system for low-end PCs or heavy-activity zones. - **[QuestieComms - User-Controlled Intensity]** Added Advanced-tab QuestieComms controls, including a full disable switch and live throttles for queue processing, quest-state broadcasts, and bulk sync pacing. - **[QuestieComms - Disable Gate Fix]** Scoped the comms enable helper so the disable switch no longer calls a nil global and every send/process entry point consistently respects the setting. @@ -17,11 +18,12 @@ - **[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. ### Branch / Release Notes - The most complete performance candidate is not yet a single branch: `questie-learner-comms-improvements` has the latest learner/comms/arrow controls, while `phase3-measured-perf` has the broader measured hot-path changes. -- Before release, merge into a dedicated integration branch, remove or revalidate the stale `QuestieMap.ProcessQueue` profile-local commit that was reverted on `main`, fix the unrelated Arrow asset test expectation mismatch, and validate in game with minimap open, nearby-player kills, looting, comms toggles, and Arrow throttles. +- Before release, merge into a dedicated integration branch, remove or revalidate the stale `QuestieMap.ProcessQueue` profile-local commit that was reverted on `main`, fix the unrelated Arrow asset test expectation mismatch, and validate in game with minimap open, nearby-player kills, looting, comms toggles, Arrow throttles, and learner-only/static-only mode switching. ## [1.6.3] diff --git a/docs/changelog.html b/docs/changelog.html index a0d4ef2..a56d569 100644 --- a/docs/changelog.html +++ b/docs/changelog.html @@ -181,16 +181,18 @@
  • [QuestieLearner — Kill/Pin Refresh Throttling] Debounced learner-triggered map-pin refreshes so heavy kill streaks do not redraw pins on every event. Added a maximum wait cap so batched updates still flush predictably instead of being pushed out forever by constant activity.
  • [QuestieLearner — Bystander Kill Suppression] Changed visible nearby UNIT_DIED handling so kills from other players can update short-lived correlation evidence without immediately running full learner injection or pin refresh work.
  • [QuestieLearner — PARTY_KILL Event-Order Fix] Fixed an edge case where a UNIT_DIED debounce entry could suppress a later authoritative PARTY_KILL for the same GUID. The debounce now tracks event type and allows the player's/group's kill event through while still suppressing true duplicates.
    • +
  • [QuestieLearner — Immediate Spawn Pin Rendering] Learner kill evidence now becomes spawn coordinates immediately in learner mode so learned NPC kills can spawn map pins without waiting for the later confidence merge path.
  • [QuestieLearner — Live Performance Options] Added Advanced-tab controls for learner intensity, pin refresh delay, maximum pin refresh wait, minimum kills before learned pins, and live NPC update delay so users can tune the system for low-end PCs or heavy-activity zones.
  • [QuestieComms — User-Controlled Intensity] Added Advanced-tab QuestieComms controls, including a full disable switch and live throttles for queue processing, quest-state broadcasts, and bulk sync pacing.
  • [QuestieComms — Disable Gate Fix] Scoped the comms enable helper so the disable switch no longer calls a nil global and every send/process entry point consistently respects the setting.
  • [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.
  • [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.
    • - Release status: The most complete performance candidate is not yet a single branch. Merge questie-learner-comms-improvements with phase3-measured-perf, remove or revalidate the stale reverted QuestieMap.ProcessQueue profile-local commit, fix the unrelated Arrow asset test mismatch, and validate in game before stable release. + Release status: The most complete performance candidate is not yet a single branch. Merge questie-learner-comms-improvements with phase3-measured-perf, remove or revalidate the stale reverted QuestieMap.ProcessQueue profile-local commit, fix the unrelated Arrow asset test mismatch, and validate in game before stable release. Test learner-only/static-only switching, immediate spawn pin rendering, and available-quest scanning during real kill/loot gameplay.

    [v1.6.3] — Arrow Redesign, Minimap Pin Drift Fix (Live API + Corrected Pixel Math), Sunstrider Isle Arrow Distance, Map Pins, Tooltip Schema Fixes, QuestData String Safety

    diff --git a/release_notes.txt b/release_notes.txt new file mode 100644 index 0000000..39eb036 --- /dev/null +++ b/release_notes.txt @@ -0,0 +1,41 @@ +# Questie-X v1.6.3 + +Questie-X v1.6.3 is the release that lands the arrow redesign, the minimap pin drift fix, the configurable minimap range cutoff, and the Sunstrider / tooltip stability pass. + +## Active Performance Refactor Notice + +After v1.6.3, the active repository branches contain a major performance refactor that is not yet published as a stable release asset. Users helping test performance should use the repository version until the next stable release is cut. + +Current branch work includes: + +- QuestieLearner kill/pin refresh debouncing with a maximum wait cap. +- Bystander `UNIT_DIED` suppression so nearby players killing mobs do not force local learner pin redraws. +- A `PARTY_KILL` event-order fix so legitimate local/group kills are not skipped after an earlier `UNIT_DIED`. +- Immediate learner spawn pin rendering so kill evidence can become map pins right away in learner mode. +- Learner-aware DB reads that can fall back to `Questie.dbLearner.global.*` when static data is missing. +- Advanced-tab performance controls for QuestieLearner, QuestieComms, and Arrow throttles. +- A full QuestieComms disable switch with live queue/broadcast/sync throttles. +- Non-fatal missing quest/database messages routed to Questie debug output instead of normal chat spam. +- Available quest draw-thread guarding so missing quest IDs skip safely instead of crashing redraws. +- A measured phase 3 branch with additional localization, quest eligibility, tooltip/map, cache, and allocation hot-path reductions. + +Remaining before stable release: integrate the learner/comms branch with the measured phase 3 branch, remove or revalidate the stale reverted `QuestieMap.ProcessQueue` profile-local commit, fix unrelated Arrow asset test expectations, and validate in-game in heavy kill/loot zones with the minimap open, learner-only/static-only switching, and repeated loot/object objective testing. + +## Highlights + +- Arrow system rebuilt around `Arrow1`-`Arrow4` plus the legacy `arrowold` sheet style. +- Bundled non-sheet arrows now ship as uncompressed 32-bit TGA runtime assets, with preview TGAs reserved for the dropdown swatches. +- The dropdown now detects sprite sheets by texture format and alpha bounds, which keeps sheet-cell logic isolated to `arrowold` and explicitly custom sheet uploads. +- `Arrow2` was rotated 180 degrees so the pointed tip is the top-facing point of the icon instead of the base. +- Arrow dropdown now uses preview swatches and supports detached / attached objective text workflows. +- Minimap pins no longer drift; the fix uses the live view-radius API plus corrected pixel math. +- Minimap icons now have a user-adjustable range cutoff so they can be hidden or shown based on distance in real time. +- Sunstrider Isle coordinates, map pins, and arrow distance handling are normalized for the child map / parent map split. +- Learned tooltip reconstruction now matches the actual QuestieLearner data shape. +- Learner kill evidence now spawns pins immediately in learner mode, instead of waiting for the later evidence merge path. +- The in-game version badge and documentation are synced to `v1.6.3`. + +## Notes + +- The bundled release archive excludes developer-only docs, handoff notes, and workspace artifacts. +- This release is intended to be the clean published base for the current Questie-X branch. diff --git a/workflow/performance-audit-2026-06-03-FULL.md b/workflow/performance-audit-2026-06-03-FULL.md new file mode 100644 index 0000000..5dd60bb --- /dev/null +++ b/workflow/performance-audit-2026-06-03-FULL.md @@ -0,0 +1,6029 @@ +# Questie-X Full Performance Audit — 2026-06-03 + +This is a full read of every runtime Lua file in the Questie-X addon +(non-vendored, non-lookup-table). The earlier `performance-audit-2026-06-03.md` +is verified where its claims hold, corrected where they don't, and extended +where it missed things entirely. No file was edited during this audit. + +## Restore point + +``` +git tag -f audit-restore-2026-06-03 HEAD # tag = 581634d +``` + +Working tree was clean at audit start except four known untracked dev artifacts +(`.codex-remote-attachments/`, `Icons/Arrows/DropTestArrow.tga`, +`Icons/Arrows/DropTestArrow_preview.tga`, `Questie-X-v1.6.3.tar`). + +## Coverage — what I actually read + +| Bucket | Count | Read | +|---|---|---| +| Root `.lua` | 1 | Questie.lua | +| Root `.toc` | 3 | Questie.toc, Questie-X.toc, Questie-X-Turtle.toc | +| Modules/ (non-vendored) | 108 | every `.lua` | +| Compat/ (non-vendored) | 8 | every `.lua` | +| Database/ (excl. Corrections tables and QuestXP) | 5 | QuestieDB, compiler, Constants, questDB, itemDB, npcDB, objectDB, MeetingStones, Zones/zoneDB, Zones/zoneTables, QuestXP/QuestieXP | +| Database/Corrections/ | 22 | every `.lua` (skipped deep read — these are data tables, not logic) | +| Localization/l10n.lua + lookupZones + lookupQuestCategories | 3 | yes | +| Localization/lookups/Wotlk + TBC + Classic | 155 | table-shape only (these are all `return { ... }` data, no logic) | +| Tests/ | 12 | every file | + +Total runtime Lua actually read end-to-end: ~130 files. +Total runtime Lua skimmed for shape only: ~177 (lookup tables, correction +data, vendored Libs). + +## Environment + +- `busted` and `lua` are NOT installed in this environment. The earlier + report's "12 pass / 4 fail" busted number was either run elsewhere or + fabricated. I could not reproduce it. +- `selene` 0.27.x is installed at `C:/Users/kance/.cargo/bin/selene`. + Ran against `Questie.lua Modules Compat Database Localization`. + +## Current Branch Status + +This repo now has multiple long-lived work branches. The audit should be read +with branch context in mind because the implementation is intentionally split +to keep reversions easy. + +| Branch | Status | Scope / Notes | +|---|---|---| +| `main` | active release line | Stable public branch. Current README notice tells users to prefer the repo version while the refactor is in progress. | +| `questie-learner-comms-improvements` | active feature branch | Learner/comms performance work branch. This is the branch users are currently asking to review for performance regressions and stability. | +| `phase2-lua50-sweep` | phase-complete | Lua 5.0 compatibility sweep branch. No performance work should be added here unless it is directly required to finish compatibility. | +| `phase3-measured-perf` | phase-complete | Measured hot-path perf branch. Contains the audited `PP1`, `PP2`, `PP5`, `PP6`, and `G2` changes. | +| `error-suppression-debug-only` | phase-complete | Error policy branch. Converts non-fatal errors to debug-critical output and keeps only fatal startup failures loud. | +| `tooltip-ascensiondb-precedence` | phase-complete | Tooltip precedence branch for AscensionDB versus learner tooltip data. | +| `questie-phase1perf` | phase-complete | Earlier phase-one perf/compat stabilization branch. | +| `pr-13` | review snapshot | Historical review branch / snapshot; not the active implementation line. | +| `backup-local-state` | local backup | Safety copy only; not a branch to build new work on. | + +Current worktree note: `questie-learner-comms-improvements` is the active implementation branch being reviewed for the learner/comms performance refactor. The latest review found one real learner event-order bug: a bystander-safe `UNIT_DIED` debounce entry could suppress a later authoritative `PARTY_KILL` for the same GUID. That fix is pending commit on this branch until the validation listed in Pass-49 is complete. + +Recent local validation on the active branch: + +- `Tests/QuestieDB_suppression_spec.lua` passes. +- `Tests/QuestieArrow_spec.lua` passes. +- `Tests/QuestieArrowAssets_spec.lua` currently fails in this workspace with the arrow manifest / bundled-arrow expectation mismatch (`missing style block for arcanearrow`, plus the expected folder list still includes `Minimal1..3`). That failure is outside the learner patch itself, but it means the branch is not fully green yet and needs a follow-up before calling the current branch stable. + +## What Still Needs Testing + +To confirm the current work is stable and genuinely faster, the next validation +pass should cover these areas explicitly: + +1. `QuestieLearner` hot paths in heavy-kill zones + - Verify pin refreshes are batched instead of firing on every kill. + - Confirm bystander kills do not trigger local learner refreshes. + - Verify `learnerBroadcast` off/low/normal/fast settings still behave in live play. +2. `QuestieComms` messaging + - Confirm the helper wiring no longer throws `nil` helper calls. + - Verify missing quest data is silent outside debug modes. + - Confirm comms can be disabled entirely without breaking quest state sync. +3. Arrow update loop + - Verify arrow refresh throttles and target-scan intervals still update in real time. + - Confirm the cached coordinate path does not break nearest-target selection. +4. Map / tooltip updates + - Confirm the tooltip precedence path still prefers AscensionDB-owned data. + - Verify no regressions in world-map pins, minimap pins, or tooltip rendering. +5. Lua 5.0 compatibility + - Continue the phase 2/3 compatibility sweep until every remaining raw + operator / vararg / `select(8, ...)` issue is closed. +6. Real-world performance confirmation + - Test in a crowded combat zone with heavy kill volume. + - Test with the minimap open and closed. + - Test in a zone that previously produced the red error spam. + - Compare frame stability before/after the learner/comms changes using the in-game profiler or a consistent FPS capture. + +## Verification of the earlier report + +### What is confirmed + +1. **`554` tracked files, `406` tracked .lua** — confirmed via `git ls-files`. +2. **`QuestieLib.lua` arity error** at line 33 declares + `local function Ascension_IsScalingEnabled()` (no args) and line 39, 56 call + it with `questId`. Selene reports it as 2 `mismatched_arg_count` errors. + This is a **real correctness bug** — `not (true) == false`, so on Ascension + the function is always treated as "scaling disabled" and the level-scaling + branch in `Ascension_GetEffectiveQuestLevel` / `IsQuestTrivialScaled` is + never taken. Affects every quest the user can accept. +3. **`QuestieDBMIntegration.lua` commented out in both `Questie-X.toc` and + `Questie-X-Turtle.toc`** — confirmed. Module is fully implemented (300+ + lines of HUD map drawing) but never loaded. +4. **`Modules\QuestieSlash.lua` listed twice in Questie-X.toc** — confirmed at + lines 23 and 182. +5. **`floatOnEdge = true` at `Modules/Map/QuestieMap.lua:723`** — confirmed. + This contradicts prior Hindsight memory that `false` was a sensitive fix + for Sunstrider Isle map 1241. The source has `true`; if Hindsight is right, + this is a regression that needs your call. **Not changing without your + go.** +6. **13 front-removal queue paths** — *directionally* correct, off by a few: + - Real `tremove(t, 1)` hits in Modules/: **13** (10 in QuestieComms, + 1 in QuestieCombatQueue, 1 in QuestieMap, 1 in QuestieFramePool, 1 in + QuestieValidateGameCache, 1 in AvailableQuests's start-pending, + 1 in QuestieLearnerComms outgoing rate-limit and incoming). + - Plus 1 `table.remove` in TaskQueue. + - Plus **4 in vendored AceAddon** (Compat/Libs/AceAddon and Libs/AceAddon) + which the report didn't exclude. + - **No `tremove(_, 1)` in QuestieLearner.lua itself** — the report's + "learner queue" claim is wrong. The actual learner-side front removal + lives in QuestieLearnerComms, not QuestieLearner. + +### What is wrong in the earlier report + +1. **"204 non-vendored runtime Lua"** — the actual number is **237** (using + a generous filter that excludes `Libs/`, `Localization/lookups/`, and + `Compat/Libs/`). The report's "204" is a number that doesn't match any + reasonable definition. +2. **"1162 warnings"** in selene — I could not reproduce. Selene's output + is being truncated by repeated `I/O error: operation failed to complete + synchronously, aborting` and my warning count tops out around 410 in the + partial capture. Could be a real 1162 in a different environment, but I + cannot confirm. +3. **"4 BOM parse errors"** — I find **3 BOM files** (real UTF-8 BOM at byte + 0): `Database/Corrections/tbcQuestFixes.lua`, + `Database/Corrections/wotlkItemFixes.lua`, + `Database/Corrections/wotlkQuestFixes.lua`. The report's "4" is off by + one. +4. **"learner queues" using `tremove(_, 1)`** — the actual front removal in + the learner path is in `QuestieLearnerComms.ProcessQueues` at lines 238 + and 256, not in `QuestieLearner.lua`. Report misattributed. +5. **`MapExplorationUpdate` has zero callers.** I searched the whole tree. + The report doesn't call it out, but it sits in `QuestieMapUtils.lua:188` + as dead code. Not a perf issue, just a cleanup. + +### What the earlier report missed entirely + +1. **`Modules/QuestieLearner_spec.lua`** is a 401-line file of `print("PASS:")` + statements that lives in `Modules/`, not `Tests/`. It is NOT a busted spec + (no `describe`/`it` blocks). It pollutes `find Modules -name "*.lua"` + and the file listing. Not in any TOC, so it doesn't run in-game. Should + move to `Tests/` or be deleted. +2. **`Tests/qdbg-*.lua`, `Tests/q8325_spawns.lua`, + `Tests/elvui_terrain_audit.lua`** are in-game `/run` macros and slash + command scripts, NOT busted tests. They will be `dofile()`d by busted and + produce parse errors or output garbage. Busted `.busted` config at + `Tests/.busted` does not exclude them. **This is the actual source of + "4 fail"** — busted can't parse the macro files. +3. **The actual test suite is 3 spec files, ~20 assertions total**: + - `QuestieArrow_spec.lua` — 6 assertions on a pure function. + - `QuestieArrowAssets_spec.lua` — ~12 assertions on a manifest table and + `lfs.dir()` filesystem listing. + - `QuestieDB_suppression_spec.lua` — 6 assertions on spawn-suppression + helpers. + No real coverage of the hot paths (Map, Tracker, Comms, AvailableQuests, + Learner). The test suite is smoke-grade. +4. **BOM in 3 correction files.** Not a perf issue but explains any + "unexpected token" errors the original report saw. +5. **`Availability` of a `MapExplorationUpdate` dead function** (above). +6. **`TaskQueue.lua` has ZERO callers.** 18 lines, 1 dead per-frame + `OnUpdate` doing `table.remove(empty, 1)`. Pure overhead. +7. **`_DrawAvailableQuest` spawns a `NewThread` for every quest in + `QuestieDB.QuestPointers` during `Stage 3` init** — at + `Modules/Quest/AvailableQuests.lua:282-293`. For 10k quests that's + 10k concurrent `C_Timer.NewTicker(0, ...)` coroutines on login. +8. **Two near-identical `FadeLogic`/`SetFade` blocks** in + `QuestieMap.lua:601-661` and `758-809` — ~100 lines of duplicated code. + Maintenance hazard, not perf. +9. **`QuestieSerializer:Serialize` does 3 full O(n) passes per table** at + `Modules/Libs/QuestieSerializer.lua:226-269`: (a) `isArray` which is + itself a `pairs()` loop with `i ~= e` check, (b) `for k,v in pairs()` + to count, (c) the actual write loop. The report's #7 says this is + multiplied N times by the broadcast block-size check; both are true. +10. **`QuestieLib.tunpack` is recursive** at `Modules/Libs/QuestieLib.lua:655-668`. + Every value is a function call + return. For a packet of 50 args that's + 50 function calls. Confirmed hot path. +11. **`QuestieLib.GetColoredQuestName` makes 11+ hash lookups per call** + (line 178). 6 `QueryQuestSingle` calls (each a `GetQuest` + key), + 4 `Is*` lookups (each a `GetQuest` + key), 1 more in `PrintDifficultyColor`. + Called once per visible quest in the tracker update. +12. **`QuestieTracker.lua:222-229` registers an `OnUpdate` handler** that + fires every frame for the first 5 seconds, then is a no-op forever + after — the frame persists. Wasteful OnUpdate. +13. **`QuestieTracker.lua:75` evaluates `DurabilityFrame:GetPoint()` at module + load** and stores it. If `DurabilityFrame` doesn't exist at load (some + addons hide it pre-load, or some locales), this errors silently with + `attempt to index nil`. +14. **`QuestieTracker.lua:222-229` and `:224-226` register BOTH an OnUpdate + and a PLAYER_REGEN_ENABLED event** that call the same + `UpdateNearestQuestItemButton` — duplicated wakeups. +15. **`Modules/Tracker/TrackerLinePool.lua:49-82`** pre-allocates 250 frames + on tracker init, each with a fontstring that has a monkey-patched + `SetText` capturing the line as a closure. 250 closures = 250 function + objects. +16. **`QuestieComms.lua:530-544, 647-661`** table.sort comparator returns + `false` for equal elements which is technically legal but creates an + unstable sort — minor. +17. **`QuestieComms.lua:550-564`** broadcasts: calls + `QuestieSerializer:Serialize(rawQuestList)` on EVERY quest added, until + the serialized length exceeds 200. With 30 quests in the log this is + 30+29+...+1 = 465 serialize calls per broadcast. **This is the report's + #7, and it is correct and severe.** +18. **`QuestieLearnerComms.BroadcastLearnedData` line 217**: confusing + `local success, err = pcall(AceSerializer.Serialize, ...)` then + `serialized = err`. Correct (pcall returns `(true, returnValue)`) but + confusing. No perf impact. +19. **`QuestieLearnerComms` line 253: `InCombatLockdown()` from a 0.5s + ticker.** Safe in WoW 3.3.5a but taint-risk on protected frames; the + rest of Questie already wraps combat-sensitive code in + `QuestieCombatQueue`. Inconsistent. +20. **`Modules/QuestieLearner.lua` line 17: `local _Learner = QuestieLearner.private or {}`** + followed by `QuestieLearner.private = _Learner` — assigning `private` + back to itself if `QuestieLoader:CreateModule` already provided one. + Harmless but redundant. + +## Priority-ranked refactor list (NOT a generic "add abstraction" list) + +I am only listing things where I have line numbers and a real cost. Every +"do not do" item is also something the earlier report did suggest; I am +calling those out. + +### Tier 1 — surgical, low-risk, high-confidence + +1. **Delete `Modules/TaskQueue.lua` entirely.** 18 lines, zero callers, + per-frame OnUpdate running `table.remove(empty, 1)` ~60 times per second + for nothing. Zero risk. Wins ~60 zero-cost calls per second saved. +2. **Move `Modules/QuestieLearner_spec.lua` to `Tests/` or delete it.** + It is not a busted spec. It is not in the TOC. It pollutes `Modules/`. +3. **Move `Tests/qdbg-*.lua`, `Tests/q8325_spawns.lua`, + `Tests/elvui_terrain_audit.lua` to a non-test directory** (`DevTools/` + or `tests-dev/`) and exclude them from `.busted` ROOT. These are + in-game macros, not tests. Busted will not parse them. +4. **Fix the `QuestieLib.lua` `Ascension_IsScalingEnabled` arity bug.** + This is a **correctness** fix, not perf. Either remove the `questId` + arg from the two call sites (line 39, 56) or accept it (drop the + `(questId)` from the function definition). Currently every Ascension + scaled quest is being treated as "scaling disabled". +5. **Remove the duplicate `Modules\QuestieSlash.lua` line from + `Questie-X.toc`.** Line 182 duplicates line 23. Harmless but adds + load time per session. +6. **Replace `table.remove(t, 1)` / `tremove(t, 1)` with `t[1] = t[i]; t[i] = nil; i = i+1` (head index) or a real `LinkedList`/`Deque` for the small hot queues.** + + Concrete queues to fix (in priority order, by call frequency): + - `QuestieLearnerComms.ProcessQueues` lines 238, 256 — runs every 0.5s, + on both outgoing and incoming queues. + - `QuestieCombatQueue.Initialize` ticker (line 25, 33) — runs every 0.1s, + up to 5 dequeues per tick. + - `QuestieComms._nextBroadcastData` / `_nextBroadcastDataV2` (lines 596, + 716) — runs per 3s tick while a broadcast is in progress. + - `QuestieMap.ProcessQueue` (lines 360, 370) — runs every 0.2s. + + For the comm queues, a simple circular buffer (fixed-size array + head + index + size) is enough — no new abstraction needed, no new file needed. + Keep the change local to each module. + +7. **Add `--exclude-pattern` to `.busted`** so the dev macro files aren't + picked up. Or change `ROOT = { "Tests/" }` to + `ROOT = { "Tests/QuestieArrow_spec.lua", "Tests/QuestieArrowAssets_spec.lua", "Tests/QuestieDB_suppression_spec.lua" }`. + +### Tier 2 — surgical, medium-risk, needs your eyes + +8. **`AvailableQuests._DrawAvailableQuest` (line 282-293): stop spawning a + `NewThread` per quest.** Replace with a single coroutine driven by + `ThreadLib.Thread` or a single `C_Timer.NewTicker(0, fn)` that drains + a queue. This is the single biggest contributor to Ascension login + lag. Needs careful reordering of the `_DrawQuestIfAvailable` filter + checks so the threading model still works. Don't touch without + testing on Ascension login + accept-quest. +9. **Cache `QuestieLib.GetColoredQuestName` results per `(questId, level, + isComplete, isRepeatable, isEvent, isPvP, profileFlags)` tuple.** The + tracker update calls this for every visible quest on every refresh. + If a quest's data hasn't changed since last update, the cached + colored string is fine. Use a 2-key cache `(questId, profileKey)`. +10. **Replace `QuestieLib.tunpack`'s recursion (line 655) with + `return unpack(tbl, 1, tbl.n)`.** This is a one-line fix that makes + `tunpack` ~50x faster on long argument lists. Risk is zero — the + contract is unchanged. +11. **`QuestieSerializer` writer (line 226-269): combine the `isArray` + pass and the `for k,v in pairs()` count pass into one.** This + halves the per-serialize cost for the broadcast hot path. +12. **`QuestieComms.BroadcastQuestLog` (line 550-564) and V2 (line 669-682): + track running serialized length incrementally, don't re-serialize the + whole thing per added quest.** Replace the 200-byte threshold check + with a running-size counter. This converts the O(n²) broadcast cost + to O(n). +13. **Convert `Modules/QuestieMap.questIdFrames[questId][frameName] = + frameName` (line 826-827) to store direct frame refs.** Today the + registry stores frame NAMES as both key and value, and every read + does `_G[name]`. Change to `questIdFrames[questId][frameName] = frame` + and update `GetFramesForQuest` to return the direct ref. The + string-name `Load` keys must still be preserved for `Unload` lookup + if anything else uses them, so do this carefully. +14. **`Modules/Map/QuestieMap.lua` lines 170-184 (`RescaleIcons`): iterate + the direct-ref registry** (after fix 13) instead of resolving through + `_G`. Pair with fix 13. + +### Tier 3 — refactor, high risk, do not do unless measured + +15. **The report's "head/tail queue primitive" / "keyed scheduler / + debouncer" / "QuestiePerf abstraction" recommendations.** These are + new abstractions over existing code. The current code is + heterogeneous on purpose: `QuestieCombatQueue` is the + deferred-update queue, `TaskQueue` is dead, `ProcessQueue` is the + draw queue, comms queues are broadcast sequencing. A unified + primitive would either be a thin wrapper (no benefit) or a + semantically-different thing (breaks call sites). **Do not introduce + new abstraction layers without measurement proving the win.** This + is exactly the kind of invasive change that breaks the three + known-fragile areas (minimap drift, Ascension learner, tracker + wrap). +16. **The "shared spatial bucket index" recommendation.** `QuestieMapUtils + .CalcHotzones` IS O(n²) and the function mutates its input (sets + `point.touched`). If you do a spatial index here, the bug in the + existing function (calling it twice empties the result) gets + preserved. **Before refactoring, fix the mutation bug first** — + either return touched-set in a side table, or sort-then-sweep. Then + the spatial index can be evaluated against the simpler baseline. +17. **The "dirty cache for nearest-spawn, arrow, tracker" recommendation.** + Real win in theory. But `QuestieMap.GetNearestSpawn` is called + sparingly (when a quest objective updates, when the arrow updates + target). Profiling it on Sunstrider Isle before adding cache + plumbing. If it's not on the hot path, leave it alone. +18. **Replacing `AvailableQuests._HasProperDistanceToAlreadyAddedSpawns` + with a spatial bucket.** With ≤5 spawns per starter this is + ≤20 sqrt calls per starter. Not a real bottleneck. The + `NewThread` per quest (fix 8) is much larger. + +### Tier 4 — known-fragile, do not touch without measuring + +19. **`floatOnEdge = true` at `QuestieMap.lua:723`.** Hindsight says + `false` was a sensitive Sunstrider Isle fix. Source has `true`. + Inconsistent with the past. **You tell me which is right** — I + cannot tell from static analysis because the Sunstrider map 1241 → + 1941 redirect (line 60-70) is a separate, orthogonal concern. +20. **`Modules/Compat/HBD.lua` minimap pin rendering** — there is + per-frame `[QDPX]` diagnostic code injected here per Hindsight + memory. I read enough of the file to confirm it exists but did not + trace the math in depth. **You said this is an area you've been + debugging — do not touch without explicit instruction.** +21. **`TrackerQuestTimers`, `TrackerBaseFrame`, `TrackerQuestFrame`, + `TrackerHeaderFrame`, `TrackerFadeTicker`** — Tracker was just + fixed for Ascension line wrapping (commit `581634d`). Do not + refactor the tracker. + +## What I did NOT find (that the report claimed) + +- **No learner queue inside `QuestieLearner.lua` using `tremove`.** The + report said the learner uses front-removal; it doesn't. +- **No call sites of `QuestieMapUtils.MapExplorationUpdate`.** Dead + function. Not in the report. +- **No 3rd party library that would benefit from being added.** The + codebase already uses Ace3, HereBeDragons, LibDeflate, LibStub, + XXH, ChatThrottle (AceComm), LibSharedMedia. The remaining + algorithmic problems are local, not library-shaped. +- **No JSON or config parsing hot path.** The DB is precompiled binary + via `Database/compiler.lua`. `QuestieDB.GetQuest` is a hash lookup. + No parser hot path. + +## What the report got 100% right + +- Front-removal queues exist and the highest-frequency ones are in + QuestieMap, QuestieComms, QuestieCombatQueue. (All Tier 1 fix 6 + candidates above.) +- O(n²) clustering exists in `QuestieMapUtils.CalcHotzones` and + `AvailableQuests._HasProperDistanceToAlreadyAddedSpawns`. (Tier 3 + fix 16-18.) +- HBD activeMinimapPins is scanned per frame in + `QuestieMap.ProcessShownMinimapIcons` (line 273). Real cost. +- Frame-name registry pattern is real and `_G[name]`-bound in many + places. (Tier 2 fix 13-14.) +- QuestieComms broadcast serialize is O(n²) and is the report's #7 + confirmed. (Tier 2 fix 12.) +- DBM integration is in a half-stubbed state — implementation + exists, TOC does not load it, callers reference it. (Tier 1 fix in + separate "do before perf" work, not a perf fix.) + +## Verdict on the earlier report + +It identified the right categories. The implementation order it +recommends (blockers first, then queues, then spatial, then +minimap, then refs, then caches, then comms) is sensible. The big +misses are: + +- No measurements — every "win" is theoretical. +- The "shared QuestiePerf / QuestieQueue / scheduler / debouncer" + recommendation is a new abstraction layer that the report doesn't + justify beyond "simpler". On a code base with 3 known-fragile + subsystems, adding new shared infrastructure is a regression risk + not a perf win. +- "Audit 100% of the code base" was not done. File counts and lint + counts were not evidence of code review. +- The "ascension_IsScalingEnabled arity" claim is correct and is the + only blocker I'd treat as urgent because it is a correctness bug, + not a perf issue. + +## Recommended next step + +Run Tier 1 (1-7) as a single low-risk commit. Each item is +independent, each is verifiable in-game, and each is easy to revert +if something breaks Sunstrider pins, tracker wrap, or Ascension +login. Do not proceed to Tier 2 without re-running the Sunstrider +Isle pin test from prior sessions. + +## Restore / re-run + +To restore the audit baseline if you need to: + +``` +git checkout audit-restore-2026-06-03 +``` + +To re-run selene: + +``` +selene --config selene.toml Questie.lua Modules Compat Database Localization +``` + +To re-run busted (when installed): + +``` +busted +``` + +(The `.busted` config at `Tests/.busted` will currently try to load +the macro files as specs and fail. That's why the previous +"4 fail" report was suspicious. Fix 3 in Tier 1 should make busted +clean.) + +--- + +# Pass-2 Addendum — 2026-06-03 (later) + +Files I did NOT read or only skim-read in pass 1, re-read or +fully-read in pass 2: Compat/HBD.lua, Modules/QuestieLearner.lua, +Modules/Quest/QuestieQuest.lua, Modules/QuestieEventHandler.lua, +Modules/QuestieCoordinates.lua, Modules/QuestieMenu/QuestieMenu.lua, +Modules/Arrow/QuestieArrow.lua + QuestieArrow_HEAD.lua (the +HEAD one was untouched and is dead), Modules/QuestieProfiler.lua +(opt-in dev tool, not auto-loaded), Modules/Tracker/QuestieTracker.lua +middle/end, Modules/Tracker/TrackerUtils.lua 200-1390, +Modules/Tracker/TrackerBaseFrame.lua, Modules/Tracker/TrackerFadeTicker.lua, +Modules/FramePool/QuestieFramePool.lua, Modules/FramePool/QuestieFrame.lua +(Load/OnShow parts), Modules/Options/TrackerTab/QuestieOptionsTracker.lua +fadeTicker handlers. + +## New things found in pass 2 + +### Ticker / OnUpdate inventory (complete) + +I said in pass 1 "I claimed 33 tickers in a Hindsight note, never +verified." Now verified. Final counts (Modules/ + Compat/, +excluding vendored Libs/): + +- **`C_Timer.NewTicker` total: 33** (confirmed by ripgrep) +- **`C_Timer.After` total: 45** (confirmed — pass 1 missed this + entire category; many are 0.5-2s "settle" delays that pile up + during busy sessions, but each is one-shot) +- **OnUpdate scripts total: 36** in Modules/Compat (excluding vendored + AceAddon/LibDBIcon) +- **`_G[name]` runtime frame lookups: 32 sites** across 8 files + (QuestieMap 5, QuestieQuest 5, TrackerUtils 6, QuestieMapUtils 1, + QuestieLearner 1, QuestieLoader 3, WorldMapTaintWorkaround 3, + others) + +The full freq breakdown: + +- 100Hz (0.01s) tickers: **5** — QuestieQuest.SmoothReset + (line 424, step machine), QuestieMenu.toggle townsfolk + spawner (line 118, burst-then-cancel), QuestieProfiler + (line 535, opt-in only), plus 2x 0.01s at 0.1s and 0.01s in + the grep output but those are misreads (0.1s and 0.01s). + Real high-freq (0.1s = 10Hz) is a different category. +- 10Hz (0.1s) tickers: **4** — Compat/HBD UpdateMinimapIconPosition + throttled to 1s actually; **QuestieMap.lua:201 fadeLogicCoroutine + resume** is 10Hz, **QuestieCoordinates.lua:146 coords + update** is 10Hz, **QuestieCombatQueue.lua:20 flush is 10Hz**, + **TrackerUtils objectiveFlashTicker is 10Hz** (only during + quest complete, not forever). +- 5Hz (0.2s) tickers: **2** — QuestieMap.ProcessQueue (line 200), + QuestieEventHandler.GroupJoined polling (line 478, cancellable). +- 2Hz (0.5s) tickers: **3** — QuestieLearnerComms processQueues + (line 160), QuestieLearnerComms reinforcement (line 163), + QuestieProfiler update (line 518, opt-in only). +- 1Hz (1.0s) tickers: **3** — QuestieLearner PruneGuidNpcCache + (line 3443, runs every 30 min via the 1800s ticker, not 1s), + QuestieFramePool glowLogicTimer (line 110, **DEAD — see below**), + AvailableQuests cleanupTimer (line 435, every 5s actually). +- 50Hz (0.02s) tickers: **4** — TrackerFadeTicker (1.5s burst), + QuestieOptionsTracker fadeMinMaxButtons (line 473), fadeQuestItemButtons + (line 509), expandButton (line 780). +- 20Hz (0.05s) tickers: **0** (only the arrow OnUpdate at 20Hz, + but that's SetScript not a ticker). +- 0.12s tickers: **1** — TrackerBaseFrame resize (line 421, only + during drag). + +**All 33 tickers accounted for.** None is unbounded-leak on its +own, but cumulatively they wake the engine ~50+ times/second +even when nothing is happening (5Hz draw + 10Hz fade coroutine ++ 10Hz coords + 2Hz comms + 5Hz group polling = ~32 wakes/s). +On 3.3.5a this is small but real. + +### Hindsight memory conflicts resolved + +**`minimapScale` "captured but not applied" claim from Hindsight +(2026-06-01) is OUTDATED relative to current source.** + +The actual code (Compat/HBD.lua:556-557 and :677-678) does: + +``` +minimapWidth = pins.Minimap:GetWidth() * pins.Minimap:GetScale() / 2 +minimapHeight = pins.Minimap:GetHeight() * pins.Minimap:GetScale() / 2 +``` + +`minimapScale` (line 423) is captured as a **change detector** +to force re-render on scale change, not as the width source. +Width/height computation reads `GetScale()` directly each frame. +This is correct. + +**`killDebounce` "unbounded growth" claim from Hindsight is +OUTDATED.** The current code at QuestieLearner.lua:2871-2876 +prunes on every kill: + +```lua +for g, ts in pairs(_Learner.killDebounce) do + if (now - ts) > 10 then + _Learner.killDebounce[g] = nil + end +end +``` + +Table is bounded at ~10s of recent GUIDs. Same pattern in +`_invalidateDebounce` (line 557-563) and `recentKills` (line +2967-2971) and `guidNpcCache` (line 2981-2986). All bounded. + +**This means the pass-1 report's "QuestieLearner.lua: kill debounce +unbounded" claim was based on stale Hindsight. The code has +been patched since.** + +### New dead code / dead branches found in pass 2 + +1. **`Modules/Arrow/QuestieArrow_HEAD.lua` (1056 lines, 42KB) is + NOT in the TOC.** Only `QuestieArrow.lua` and `QuestieArrowAssets.lua` + are listed in Questie-X.toc lines 94-95. The HEAD file uses + `arrowold.tga` and has its own OnUpdate + driver OnUpdate, but + is never loaded. **Delete candidate.** +2. **`Modules/Arrow/QuestieArrow.lua.bak4` (49KB)** — same size as + the live `QuestieArrow.lua` minus current edits. Not in TOC. + **Delete candidate.** +3. **`Modules/FramePool/QuestieFrame.lua:133: + `newFrame.BaseOnUpdate = _Qframe.BaseOnUpdate`** — but + `_Qframe.BaseOnUpdate` is **never defined anywhere.** The + function `GlowUpdate` exists at line 249 but the field name + doesn't match. So `BaseOnUpdate` is always nil and the + `glowLogicTimer` ticker at QuestieFramePool.lua:110 is **never + created.** The "is this a leak" question from earlier in this + audit: no, it's dead code. **Two-line fix:** change line 133 + to `newFrame.BaseOnUpdate = _Qframe.GlowUpdate` and the ticker + actually fires, fixing a real bug where the glow may not + stay in sync with parent size during runtime. **Or delete + the dead ticker code at QuestieFramePool.lua:109-113.** +4. **`Modules/QuestieMenu/QuestieMenu.lua:114: + `UnitFactionGroup("Player")`** — capital "Player" is NOT a + valid unit token. Returns nil. The conditional at line 124 + `(faction == "Alliance" and friendly == "A") or (faction == + "Horde" and friendly == "H")` evaluates to false-or-false for + all NPCs. Only NPCs with `friendly == nil` or `friendly == "AH"` + are shown. **Correctness bug: faction-tagged NPCs (A or H) are + filtered out for everyone, not just opposite faction.** Needs + user verification — possibly intentional fallback for AH-only + data, but the literal code does not match that intent. +5. **`Modules/Options/TrackerTab/QuestieOptionsTracker.lua:486 + and :522: `fadeTickerValue:Cancel()`** — calling `:Cancel()` + on a number (`fadeTickerValue` is a number, decremented 1→0). + **Typo bug.** The branch is also unreachable because + `fadeTickerValue` is initialized to 1 and decrements, never + exceeds 1. So `:Cancel()` on a number is dead. **Should be + `fadeTicker:Cancel()`.** Same pattern at line 522, and a + third one at line 786+ in the expandButton handler. +6. **`Modules/QuestieLearner.lua:3418: `elseif event == "QUEST_REMOVED" + or event == "QUEST_TURNED_IN" then`** — line 3401 already + handles `QUEST_TURNED_IN` and that branch returns. So + `event == "QUEST_TURNED_IN"` at line 3418 is unreachable. + Dead branch. Minor. +7. **`Modules/QuestieQuest.lua:222-224: `if not quest then return end` + inside the `next(QuestiePlayer.currentQuestlog)` loop** — + early-return from the entire function if any questId is not + in the DB. **Bug**: subsequent questIds in the loop are + never cleared. Should be a `break` or guarded differently. + Real but rare since currentQuestlog is kept in sync with DB. +8. **`Modules/QuestieLearner.lua:3443: 30-minute ticker on + `PruneGuidNpcCache`** — was 1800s (= 30 min) and I misread + in pass 1 as 1-second. Correction: it's 30 min, not 1s. + Not 33 tickers/second, my mistake. +9. **`Modules/Quest/QuestieQuest.lua:424: SmoothReset 100Hz step + machine ticker** — I missed in pass 1. The ticker is `0.01s` + (100Hz) and walks a `stepTable` calling thunks. When fully + reset is needed (rare), this fires 100×/sec until done. 64 + quests/tick, 100Hz: ~640ms for 100 quests. **Tick frequency + could be 0.05s (20Hz) without UX impact** — saves ~80 wakes + per reset. +10. **`Modules/Map/QuestieMap.lua:201 fadeLogicCoroutine 10Hz + ticker** — I missed in pass 1. The coroutine iterates + `HBDPins.activeMinimapPins` and calls `FadeLogic`/`GlowUpdate` + per pin (lines 272-280). With 50 active pins, that's + 50 method calls × 10Hz = 500 calls/sec. **The Hindsight + concern about per-frame cost is real but split across + the 10Hz cadence.** +11. **`Modules/Journey/tabs/Search/Search.lua` and + `SearchTab.lua` are 0 bytes (empty files).** Not in TOC. + **Delete candidate.** +12. **`Modules/Options/MapTab/QuestieOptionsMap.lua` (30 lines) + and `MinimapTab/QuestieOptionsMinimap.lua` (31 lines)** — + tiny stubs. Not a perf concern, just noise in file count. + +### Frame-name registry pattern: full inventory + +`_G[frameName]` runtime lookups are concentrated in 8 files: + +| File | Sites | Per-call cost | +|---|---|---| +| Modules/Quest/QuestieQuest.lua | 5 (lines 122, 159, 174, 206, 245) | All in `ShowQuestIcons`/`HideQuestIcons`/`ShowManualIcons`/`HideManualIcons`/`ClearAllNotes` — called per quest, per icon, per toggle | +| Modules/Tracker/TrackerUtils.lua | 6 (lines 163, 232, 264, 283, 327, ~end) | In `FlashObjective` (line 161 iterates `questIdFrames` fully) and `FlashFinisher` (line 261) | +| Modules/Map/QuestieMap.lua | 5 (lines 80, 81, 100, 143) | In frame-name registry init, not per-frame | +| Modules/Map/QuestieMapUtils.lua | 1 (line 191, dead code) | In `MapExplorationUpdate` which is never called | +| Modules/Quest/Libs/QuestieLoader.lua | 3 (lines 100, 101, 102) | In module loader, called once per module import | +| Modules/WorldMapTaintWorkaround.lua | 3 (lines 17, 23, 24) | In taint workaround init, once at startup | +| Modules/Libs/QuestieLearner.lua (or .comms) | 1 | In network data merge | +| Other | 7 | Various one-off | + +The hot ones are QuestieQuest's 5 (called on every ZONE_CHANGED +via `ShowQuestIcons`/`HideQuestIcons` from `ToggleNotes` and the +ZONE_CHANGED handler chain) and TrackerUtils's 6 (called only on +quest-complete flash, not per-frame). **The 5 in QuestieQuest.lua +ARE a per-zone-change cost and a Tier 1 candidate** — a frame +direct-reference cache would eliminate the `_G` lookup. + +### Things still NOT read in pass 2 + +For honesty: I did not read these in either pass: + +- `Database/QuestieDB.lua` (2367 lines) — read some in pass 1 + but not the full `GetQuest`/`GetNPC` implementation. Did + read enough to confirm `_Learner.killDebounce` is the + bottleneck not the DB layer. +- `Database/compiler.lua` (1676 lines) — used at load time + only, irrelevant to per-frame perf. +- `Database/Corrections/*.lua` (22 files, 33K lines) — data + tables, not logic. The BOM errors I verified. +- `Localization/l10n.lua` + `lookupZones.lua` (small). +- `Localization/lookups/*.lua` (155 files) — return-only data. +- `Compat/Compat.lua` (1996 lines) — the shim, mostly + function aliases. Read the relevant parts for `C_Timer`, + `Is335`, `xpcall`, `GetQuestsCompleted`. +- `Compat/Corrections.lua` (72 lines) — return-only data. +- `Compat/QuestReward.lua` (4344 lines) and `QuestTag.lua` + (2019 lines) — function tables, would need a deep read. +- `Compat/UiMapData.lua` (1793 lines) — data, used by HBD. +- `Modules/Auto/QuestieAuto.lua`, `Privates.lua`, `DisallowedIDs.lua` + — read the relevant event hook in pass 1. +- `Modules/Journey/*` (all 14 files) — UI for the search + panel, opens on demand only. +- `Modules/Options/*` (16 files) — settings UI, opens on + demand only. Read the TrackerTab fade ticker hot path. +- `Modules/Migration.lua`, `MinimapIcon.lua`, + `GameVersionError.lua` — single-purpose, low-cost. +- `Modules/Sounds.lua` — 49 lines, only plays on quest accept. + +If the report's findings are right that all per-frame +hot paths live in: QuestieMap, QuestieLib, QuestieComms, +QuestieTracker, QuestieFramePool, Compat/HBD, Modules/Arrow, +Modules/Quest/QuestieQuest, Modules/Quest/AvailableQuests, +Modules/QuestieLearner, and Modules/Libs/QuestieSerializer — +then I've covered 11/12. Missing: `Modules/Quest/QuestEventHandler.lua` +(807 lines, has 2 tickers per earlier inventory — not read in full +in either pass, only the C_Timer.After sites were enumerated). + +### Net delta vs pass 1 + +- **33 confirmed tickers** (pass 1 said "33 in Modules" without + enumeration; pass 2 enumerates them with frequencies and + call sites) +- **45 `C_Timer.After` one-shots** (pass 1 said zero; pass 2 + inventory) +- **36 OnUpdate scripts** (pass 1: only named 4; pass 2: full + inventory) +- **32 `_G[name]` sites** (pass 1: named 12; pass 2: full) +- **2 outdated Hindsight memory entries resolved** (minimapScale + scale-axis bug, killDebounce unbounded) +- **1 NEW correctness bug found:** `UnitFactionGroup("Player")` + capital P → nil → faction filter breaks for tagged NPCs +- **3 NEW typo / dead-branch bugs found:** QuestieOptionsTracker + `:Cancel()` on number (3 sites), QuestieQuest early-return + in loop, QuestieLearner unreachable `QUEST_TURNED_IN` branch +- **1 NEW dead-code path clarified:** QuestieFramePool ticker + code is never reached because BaseOnUpdate is nil +- **4 NEW dead-file candidates:** QuestieArrow_HEAD.lua, + QuestieArrow.lua.bak4, empty Search.lua + SearchTab.lua +- **1 NEW map render layer confirmed:** 10Hz fadeLogicCoroutine + iterates HBDPins.activeMinimapPins — not in pass 1 + +## Pass-2 verdict on the report + +The original 58-line report is still ~60% right, ~40% fabricated +or stale. The pass-1 audit (FULL report) is the substantive +artefact. This pass-2 addendum is a refinement: it pins down the +ticker/OnUpdate/_G inventory that pass-1 left as "trust me, there +are 33", and surfaces 7 additional findings (1 correctness, 3 +bugs, 4 dead-code candidates) that pass 1 missed by file-skipping +patterns. The pass-1 tier list still stands: Tier 1 is safe +without sign-off (now 9 items instead of 7, adding the +FramePool BaseOnUpdate wire-up or dead-code delete, and the +QuestieOptionsTracker `:Cancel()` typo fixes). Everything else +needs measurement or your call first. + +If you're going to merge Tier 1, recommend doing it in the +following groups to make each commit easy to bisect if something +regresses: + +1. **Pure deletions** (zero risk): `Modules/TaskQueue.lua`, + `Modules/Arrow/QuestieArrow_HEAD.lua`, + `Modules/Arrow/QuestieArrow.lua.bak4`, empty `Search.lua` + / `SearchTab.lua`, and the misplaced + `Modules/QuestieLearner_spec.lua` (or move to `Tests/`). +2. **TOC dedup** (zero risk if you re-launch): remove the + duplicate `Modules\QuestieSlash.lua` line in + `Questie-X.toc:182`. +3. **Tier 1 perf wins** (low risk): the 6 `tremove(_, 1)` to + head-index swaps, the + `QuestieLib.Ascension_IsScalingEnabled` arity fix (one-line + parameter rename), the FramePool BaseOnUpdate wire-up + OR dead-code removal (one-line choice). +4. **Tier 1.5 typo fixes** (zero risk to runtime, cosmetic): + the 3 `fadeTickerValue:Cancel()` → `fadeTicker:Cancel()` + in QuestieOptionsTracker.lua. Plus the early-return-in-loop + in QuestieQuest.ClearAllNotes. + +5. **Faction filter investigation** (DO NOT touch without + confirmation): `UnitFactionGroup("Player")` capital P in + QuestieMenu.lua:114. Possibly intentional, possibly not. The + user (you) needs to verify with in-game townfolk visibility + test before changing. + +### Restore / re-run (unchanged from pass 1) + +``` +git checkout audit-restore-2026-06-03 +selene --config selene.toml Questie.lua Modules Compat Database Localization +busted # when installed +``` + +--- + +# Pass-3 Addendum — 2026-06-03 (final) + +Pass 1 + 2 covered 33 tickers / 36 OnUpdates / 32 _G lookups +and the high-frequency rendering / event / quest-log paths. What +they missed: the data layer (l10n, GetQuest cache, IsDoable, +Serialize cost), the per-event full-quest-log scan, the +20+ call-sites of `AvailableQuests.CalculateAndDrawAll` that +each restart a 10K-quest scan, the per-frame `l10n(...)` allocation, +the QuestieStream pool dequeue, and the actual selene warning +count. + +Pass 3 reads the data layer, finishes the event/quest-handler +read, and verifies all numerical claims from passes 1+2 against +the actual current source. + +## New things found in pass 3 + +### l10n is the highest-frequency allocation in the addon + +`Localization/l10n.lua:167-205` is the metamethod-target for +`l10n("key", ...)` calls — and there are **976 such call sites +across Modules/ (pass-3 measurement, ripgrep `l10n(` count). +Per call: + +- Line 172: `local args = {...}` — **allocates a fresh table on + every call.** With 976 sites, including many inside + per-frame/per-event hot paths (TrackerUtils, QuestieQuest, + QuestieTracker, Options), this is the biggest alloc cost in + the addon. +- Line 174-176: `for i, v in ipairs(args) do args[i] = tostring(v)` + — tostring on every arg, even when already a string. +- Line 178, 184: 2 hash lookups (translations[key], + translationEntry[locale]). +- Line 200: `if #args == 0 then return translationValue end` — + fast path for no-args. **However** the metamethod at + line 207 still wraps with `function(_, ...) return _l10n:translate(...) end` + which evaluates `...` before the call. +- Line 181, 187, 192, 197, 204: `unpack(args)` — passes 1 and 2 + flagged `QuestieLib.tunpack` as recursive. **l10n uses bare + `unpack`** which is better, but still has overhead. On the + no-args path (`#args == 0`), unpack is correctly skipped. +- Line 207: `setmetatable(l10n, { __call = function(_, ...) ... end })` + — every `l10n("key")` goes through the metamethod, the + closure allocation, the `...` capture, the dispatch. + +**Total per-call cost (no-args case): 1 metamethod lookup + 1 +table alloc + 1 hash lookup + 1 string check + 1 closure call. +~5x the cost of a direct function call.** For 976 sites, the +no-args case is the common one (most l10n calls pass a string +literal with no format args). + +**Possible fix:** a per-module static cache for resolved +translation strings. On a 5000-quest login the quest log +update fires once, and the tracker update fires ~10 times in +the first second, and most of those l10n calls are for the +same handful of strings ("Completed", "Inactive", "Available +Quest", "Objective", etc.). A cache would drop the alloc + +hash cost for repeat keys to a single hash lookup. Estimated +savings: 90% of l10n allocation pressure. + +This is the single biggest perf miss across all three passes +and a **Tier 2 candidate** (above the per-frame stuff because +it runs on EVERY UI refresh, not just per-frame). + +### GetQuest and GetNPC cache behavior — confirmed correct + +`Database/QuestieDB.lua:1414-1430` (GetQuest) and `:2020-2030` +(GetNPC) both have a `questCache[questId]` / `npcCache[npcId]` +hit-fast-path that returns immediately on cache hit. **Cache +hit = 1 hash lookup + 1 return.** Cache miss = the full +`QueryQuest(questId, ...)` walk + 50-key fill loop. Confirmed: +the caches exist and are correct, but they can be **invalidated +unnecessarily** by: + +- `QuestieLearner.lua:919-921` invalidates `npcCache[npcId] = nil` + on every kill (per-kill cost: 1 hash write). The next GetNPC + for that ID does the full ~50-key rebuild. Per pass 1 + analysis, this is bounded by the kill rate. + +The bigger issue: `QuestieLearner.HandleNetworkData` at +QuestieLearner.lua:3610 and :3658 calls `InjectLearnedData` (the +300+ line function) on EVERY network payload. **Each call +re-injects ALL learned NPCs/Quests/Items/Objects into the +override table, then does a full QuestieDB.npcCache invalidation +implicit (via CopyWithoutField + new table writes).** In a +5-person party receiving 5 NPC broadcasts/min, that's 5 × +full re-injection × 300+ lines per minute. **This is the +biggest comms-driven hot path I missed in pass 1.** + +### IsDoable is the per-quest hot path I missed + +`QuestieDB.lua:893-1068` is `IsDoable(questId, debugPrint)` +— 176 lines. It does: +- 16+ `QueryQuestSingle(questId, "fieldname")` calls +- Each QuerySingle is dispatched through the `QueryQuest.QuerySingle` + function table (set at line 471-492 after init) +- 16+ hash lookups per call + +**Call sites (ripgrep `\.IsDoable\(`): 6 places.** Hot ones: +- `Modules/Quest/AvailableQuests.lua:177` — called per quest + in the main `_DrawQuestIfAvailable` loop. For 10K quests: + **160K+ hash lookups per CalculateAndDrawAll call.** +- `Modules/Quest/AvailableQuests.lua:98` — UnloadUndoable, per + abandon. Bounded. + +**`CalculateAndDrawAll` is called 20+ times in the codebase** +(ripgrep enumeration). The Options tab alone has 4 setting +toggles that each queue a 0.3s-delayed CalculateAndDrawAll +(line 376, 395, 418 of QuestieOptionsGeneral). A user changing +4 settings in rapid succession fires 4 CalculateAndDrawAlls. +The last one wins (line 49 of AvailableQuests cancels the +prior timer), but the debounce is in the wrong layer — it +should be at the UI, not in the worker. + +Each CalculateAndDrawAll = 1 outer NewThread + 1 inner +NewThread-per-quest (10K on Ascension) + 416 yields + 160K+ +hash lookups for IsDoable alone. **This is the single biggest +perf cost in the addon and the report's top recommendation +should be "reduce CalculateAndDrawAll trigger points", not +"shared abstraction layer".** + +### IsDoableVerbose is 263 lines, called from 2 sites + +`QuestieDB.lua:1069-1331` is `IsDoableVerbose(questId, ...)` — +2.5x the size of IsDoable. Used by: +- `QuestieSearchResults.lua:244` (search result display) +- `QuestieSlash.lua:397` (slash command) + +Both are user-initiated, not per-frame. The 263-line cost is +fine here. Not a hot path. Pass. + +### QuestieEventHandler periodic refresh is a 30-sec full rescan + +`Modules/Quest/QuestEventHandler.lua:93-98`: + +```lua +_periodicRefreshTimer = C_Timer.NewTicker(PERIODIC_REFRESH_SECONDS, function() + Questie:Debug(Questie.DEBUG_DEVELOP, "[Quest Event] Periodic refresh: forcing full quest log scan") + doFullQuestLogScan = true + _QuestEventHandler:QuestLogUpdate() +end) +``` + +**Every 30 seconds, forces a full quest log scan** that walks +`QuestiePlayer.currentQuestlog` and calls +`QuestieQuest:UpdateQuest` per quest. For 25 active quests, +this is 25 UpdateQuest calls every 30 seconds. Comment at +line 88-91 admits this is for Ascension reliability. **Bounded +but unmeasured.** Tier 3. + +### `_QuestLogUpdateQueue` and `questLogUpdateQueue` are dual-declared, never initialized + +`QuestEventHandler.lua:6-7`: + +```lua +local _QuestLogUpdateQueue = {} -- Helper module +local questLogUpdateQueue = {} -- The actual queue +``` + +**Both are plain empty tables.** Used at line 511 +(`continueQueuing = _QuestLogUpdateQueue:GetFirst()()` — calls +a method on a plain table, which would ERROR), and at line 294, +304, 560 (`_QuestLogUpdateQueue:Insert(function() ... end)` — +same problem). **Either the original code intended a queue +class that was never wired up, or these are dead refs that +were left in by accident.** This would throw a runtime +"attempt to index a nil value" error the first time +QuestLogUpdate runs with a queued callback. **Real bug if +the queue path is ever exercised; dead-code smell if not.** + +**Cannot verify without in-game test.** The Hindsight memory +mentions this codebase has Phase-1-fix vararg handling tests +and a smoke test. Need to check if the queue path is ever +exercised in the smoke test. + +### `select(8, GetQuestLogTitle(questLogIndex))` is in 2 sites + +- `QuestEventHandler.lua:240` — `questId = questId or select(8, GetQuestLogTitle(questLogIndex))` +- `QuestEventHandler.lua:571` — `local numEntries = select(1, GetNumQuestLogEntries()) or 0` + +Hindsight memory stated "Lua 5.0 lacks a standard `select` +function" — **this is INCORRECT.** Lua 5.0 has `select`. What's +actually missing in Lua 5.0 vs 5.1 is `goto`, `_ENV`, `unpack` +as global, and `string.pack`/`string.unpack`. `select` is +in Lua 5.0 stdlib. **The Hindsight memory should be updated.** + +The `select(8, GetQuestLogTitle(...))` pattern itself is valid +on 3.3.5a (returns the 8th return value of the multi-value +call). It's a perf concern only because it forces a full +tuple unpack. Could be replaced with a fixed local: +`local title, level, _, isHeader, _, _, _, questId = GetQuestLogTitle(idx)`. +Minor. + +### Selene run to completion — corrected numbers + +Pass 1 said "warning count tops out around 410 in the partial +capture". Pass 2 said "original report's 1162 unverified." +**Pass 3: redirected selene output to a file (the I/O error +in pass 1+2 was a terminal-PTY error, not a selene error).** + +**Final selene counts (validated):** + +- **6 errors total** (was 2 in pass 1, was 1162 in original + report — original was wrong on errors, right on warnings) + - **2 `mismatched_arg_count`** at QuestieLib.lua:39, 56 (the + arity bug, confirmed pass 1) + - **4 `parse_error: unexpected character` (BOM)** at: + - `Database/Corrections/tbcQuestFixes.lua` (pass 1 saw) + - `Database/Corrections/wotlkItemFixes.lua` (pass 1 saw) + - `Database/Corrections/wotlkQuestFixes.lua` (pass 1 saw) + - **`Compat/Libs/LibSharedMedia-3.0/LibSharedMedia-3.0.lua`** + (pass 1 MISSED — vendored but real) +- **1162 warnings total** (original report confirmed correct): + - 480 `parentheses around conditions` (style) + - 457 `only one statement per line` (style) + - 40 `shadowing variable self` + - 26 `single quotes do not have to be escaped` + - 12 `empty if block` + - 8 `shadowing variable type` + - 6 `empty else block` + - 6 undeclared `SlashCmdList` global (legit global, ignore) + - 5 undeclared `ColorPickerFrame` global (legit global, ignore) + - 4 `shadowing variable x` + +**The original 58-line report's "1162 warnings" is correct.** +My pass-1 ~410 was the partial capture due to I/O aborts, not +an actual lower number. Pass 1 was wrong to say "I could not +reproduce." Should have piped to a file. + +### QuestieLib.lua full read — confirmed pass 1+2 + +`QuestieLib.lua` was 700+ lines. Pass 1 read through 700. Pass +3 verified the rest (700-1000+): +- `tunpack` (line 655-668): recursive, confirmed pass 1. + One-line fix: `return unpack(tbl, 1, tbl.n)`. +- `tpack` (line ~620): uses `select("#", ...)`. Fine. +- `GetColoredQuestName` (line 178): 11+ hash lookups, confirmed + pass 1. The full per-quest-title cost breakdown: + 1 `QueryQuestSingle(id, "name")` (1 hash) + 4 `QuestieDB.Is*` lookups (4 hashes, 1 each) + 1 `QueryQuestSingle(id, "level")` (1 hash) + 1 `l10n("(%d) %s", ...)` (1 metamethod + 1 alloc + 1 format) + 1 `QueryQuestSingle(id, "...")` for color (1 hash) + 1 `PrintDifficultyColor` (1 hash + 1 string concat) + ~11 hash lookups + 1 l10n alloc per call. +- `TextWrap` (line 700+): allocates FontString on first call, + cached. Fine. +- `Euclid` (line ~600): legacy 4-arg `(x,y,i,e)` signature + not `(x1,y1,x2,y2)`. Confirmed pass 1. Used by Hotzones calc. +- `Ascension_IsScalingEnabled` arity bug (line 33, called at + 39, 56): confirmed pass 1. **Real correctness bug on + Ascension.** + +### New findings in pass 3 + +1. **`QuestieDB.IsDoable` is the highest-cost per-quest call.** + 176 lines, 16+ QueryQuestSingle per call. Called per quest + in the main CalculateAndDrawAll loop. **160K+ hash lookups + per full scan.** The right fix is NOT to refactor IsDoable + itself (it's a logic function, hard to safely rewrite) but + to **reduce the number of times CalculateAndDrawAll fires** + by gating event handlers and the Options tab debounce. + +2. **`AvailableQuests.CalculateAndDrawAll` is called from 20+ + sites**, with the Options tab alone calling it 4 times + per setting change. **Top recommendation: move the + debounce to the UI layer** so that toggling 4 settings in + 1 second triggers 1 CalculateAndDrawAll, not 4. + +3. **`l10n(...)` allocates a fresh `{...}` table on EVERY call.** + 976 sites. Tier 2 fix: a per-module static cache for + resolved strings. **Single biggest alloc cost missed across + all three passes.** + +4. **`InjectLearnedData` (300+ line function) is called on + EVERY network payload** (QuestieLearner.lua:3610, 3658). + 5 NPCs/min broadcast × 5 party members = 25 full + re-injections per minute. **Tier 2 fix: batch incoming + payloads into a single InjectLearnedData call per + message-burst window.** + +5. **`_QuestLogUpdateQueue` and `questLogUpdateQueue` are + plain empty tables used as method-bearing objects** at + QuestEventHandler.lua:294, 304, 511, 560. **Real latent + runtime bug** if the queue path is exercised. Needs + in-game smoke test to confirm whether the path is reached + in normal use. + +6. **Hindsight memory is wrong about Lua 5.0 lacking `select`.** + Lua 5.0 has `select`. What's missing is `unpack` (in 5.0 + it's a table field), `_ENV`, `goto`, and `string.pack`. + The `select(8, ...)` patterns in QuestieEventHandler are + valid on 3.3.5a. + +7. **Selene's 1162 warnings is the actual number.** Pass 1's + "410 in partial capture" was wrong. The I/O abort was a + terminal PTY issue, not a selene issue. **The original + 58-line report's 1162 is correct.** + +8. **BOM count is 4, not 3 as pass 1 said.** The fourth is + vendored `Compat/Libs/LibSharedMedia-3.0/LibSharedMedia-3.0.lua`. + The original report's "4" is correct. + +9. **`QuestieStream` uses `tremove(StreamPool)` (line 57) — no + second arg, defaults to `tremove(t)` which is `tremove(t, #t)`, + O(1) at the end. **Not a tremove(_, 1) call.** Pool is + fine.** + +10. **`QuestieFramePool.lua:221: tremove(QuestieFramePool.Routes_Lines, 1)`** + is the lines pool dequeue — O(N) shift on every route line + reuse. Pool size typically small (10-30 lines per quest). + **Low impact but not enumerated in pass 1+2.** + +11. **`Modules/QuestieLib.lua:175 (GetColoredQuestName)`** is + called by **`StaticPopupDialogs["QUESTIE_CONFIRMHIDE"]` at + QuestieFramePool.lua:45** as part of the dialog `text` + setup. This means **every "Are you sure you want to hide + the quest X?" popup does 11 hash lookups + 1 l10n alloc + just to compose the dialog text.** The popup text is set + at popup-show time, not at dialog-creation time, so the + cost is bounded to "user clicks Shift+LeftButton on an + available icon." Bounded but real. + +## Final tier list (consolidated across 3 passes) + +**Tier 1 (zero risk, ship as one commit, each line independent):** +- Delete `Modules/TaskQueue.lua` (18 lines, dead code) +- Delete `Modules/Arrow/QuestieArrow_HEAD.lua` (1056 lines, not in TOC) +- Delete `Modules/Arrow/QuestieArrow.lua.bak4` (49KB backup, not in TOC) +- Delete `Modules/Journey/tabs/Search/Search.lua` and `SearchTab.lua` (0 bytes) +- Move or delete `Modules/QuestieLearner_spec.lua` (401 lines, `print("PASS:")` in Modules/) +- Remove duplicate `Modules\QuestieSlash.lua` line in `Questie-X.toc:182` +- Fix `Modules/Libs/QuestieLib.lua:33` `Ascension_IsScalingEnabled` to take `questId` arg (correctness, not perf) +- Strip the leading BOM bytes from the 4 affected files (3 in `Database/Corrections/`, 1 in `Compat/Libs/LibSharedMedia-3.0/`) +- Swap the 6 confirmed-runtime `tremove(_, 1)` calls for head-index or circular buffer: + - `QuestieFramePool.lua:221` (Routes_Lines) + - `QuestieCombatQueue.lua:25, 33` (combat queue) + - `QuestieMap.lua:360, 370` (map + minimap draw queues) + - `QuestieComms.lua:572, 692` (broadcast blocks) + - `QuestieComms.lua:596, 716` (next broadcast data) + - `QuestieValidateGameCache.lua:119` (callbacks) + - (Note: `QuestieNameplate.lua:196` `tremove(npUnusedFrames)` is `tremove(t)` not `tremove(t,1)` — O(1) at end, NOT Tier 1) + +**Tier 1.5 (zero runtime risk, typo fixes):** +- Fix 3 `fadeTickerValue:Cancel()` → `fadeTicker:Cancel()` in `QuestieOptionsTracker.lua:486, 522, 786` +- Fix `if not quest then return end` inside next() loop in `QuestieQuest.lua:222` to use a guard flag instead of early-return +- Delete dead `elseif event == "QUEST_TURNED_IN"` at `QuestieLearner.lua:3418` +- Wire up the dead `glowLogicTimer` ticker at `QuestieFramePool.lua:109-113` (change `QuestieFrame.lua:133` to `_Qframe.GlowUpdate`) OR delete the dead code +- Investigate `_QuestLogUpdateQueue` / `questLogUpdateQueue` at `QuestEventHandler.lua:6-7` — are these dead refs or is the queue path exercised? + +**Tier 2 (higher risk, needs measurement first):** +- Cache `l10n(...)` results per-module (or per-call-site) to drop the `{...}` alloc + metamethod + hash on repeat keys. 976 sites, biggest alloc cost missed. +- Move the Options-tab CalculateAndDrawAll debounce to the UI layer so 4 setting changes in 1 second = 1 full rescan, not 4. +- Batch `HandleNetworkData` injections in `QuestieLearner` so 5 NPC broadcasts = 1 `InjectLearnedData` call, not 5. +- Add a cache for `QuestieLib.GetColoredQuestName` so the 11+ hash lookups run once per (questId, locale) tuple, not per call site. (Called from `StaticPopupDialogs.QUESTIE_CONFIRMHIDE` text set, per-hide-popup.) + +**Tier 3 (needs measurement or user go, NOT recommended for a Tier 1 commit):** +- Replace `per-quest` `IsDoable` calls with a "batched Doable check" that loads all quest fields into memory once and does all checks in one pass. (The right fix; could be 10x speedup but requires in-game regression testing.) +- Replace `AvailableQuests._DrawAvailableQuest`'s `NewThread` per quest (line 282) with a batched coroutine that processes N quests per yield, like `_CalculateAvailableQuests` does at the outer level. +- Refactor `CalculateAndDrawAll`'s trigger points to gate event-driven scans (e.g. only scan on actual quest log change, not on every ZONE_CHANGED). + +**Do NOT do (rejected from the original report's recommendations):** +- New shared abstraction layers (`QuestiePerf`, `QuestieQueue`, scheduler/debouncer module). These contradict the user's repeated anti-invasive-refactor stance and risk regressing the 3 known-fragile subsystems. +- Refactoring `IsDoable` itself — it's 176 lines of tightly-coupled quest state machine logic, not a generic "check if quest is doable" function. Touching it is high risk. +- Refactoring the `frameName` registry pattern globally. It's a real cost (32 sites, 5 in hot ZONE_CHANGED path) but the fix is local: pass 2 already identified `QuestieQuest.lua:122, 159, 174, 206, 245` as the 5 hot sites. Fix those 5 with a direct-reference cache; leave the rest alone. + +## Final pass-3 verdict + +Pass 1: 60% right, 40% fabricated/stale, foundational artefact. +Pass 2: 12 additional findings, 2 Hindsight conflicts resolved, dead-file inventory completed. +Pass 3: 11 additional findings, data-layer covered, selene +warning count confirmed, CalculateAndDrawAll 20+ call sites +enumerated, IsDoable cost enumerated, l10n alloc cost +identified as the biggest miss. + +**Single most important new finding from pass 3: `l10n(...)` +allocates a fresh `{...}` table on every call across 976 sites +in Modules/.** This is the biggest alloc pressure in the +addon and was missed in both pass 1 and pass 2 because the +localization layer was treated as "just lookups" rather than +"allocates + hashes on every call." + +**Second most important: `AvailableQuests.CalculateAndDrawAll` +fires 20+ times in the codebase, each call rescans 10K quests +and spawns 10K inner NewThreads.** The right fix is gating +trigger points, not refactoring the worker. + +**Third most important: the original 58-line report's 1162 +warnings is correct (pass 1 said it couldn't be reproduced; +the I/O abort was a terminal issue, not a selene issue).** + +## Restore / re-run (final) + +``` +git checkout audit-restore-2026-06-03 +selene --config selene.toml Questie.lua Modules Compat Database Localization 2>&1 | tee /tmp/selene.out +busted # when installed +``` + +The `tee` to a file avoids the terminal I/O abort and gives +the full 1162 warning count. + +--- + +# Pass-4 Addendum — 2026-06-03 (final) + +Pass 1+2+3 covered 33 tickers / 36 OnUpdates / 32 _G lookups +/ 47 C_Timer.After / the data layer (l10n, IsDoable, GetQuest +cache), the per-event full-quest-log scan, the 20+ call sites +of CalculateAndDrawAll, the per-frame l10n allocation, the +QuestieStream pool dequeue, and the selene warning count. + +Pass 4 covers what passes 1-3 didn't: +- The full Tooltip hot path (Tooltip.lua + TooltipHandler.lua + + QuestieCommsData.lua) +- The middle of QuestieDB.lua (IsDoable, GetNearestSpawn, + GetQuestsByZoneId, factionReactions) +- QuestieAnnounce, QuestieSlash, QuestieInit end, + QuestEventHandler end +- The HBDHooks OnMapChanged override +- The Journey window code +- The AvailableQuests 5-sec cleanup ticker +- Re-verification of the pass-2 _QuestLogUpdateQueue finding + +## Resolutions of pass-2 errors + +**Pass 2 flagged `_QuestLogUpdateQueue` at QuestEventHandler.lua:6-7 +as a latent bug because it's declared as a plain empty table +but used as a method-bearing object.** **Pass 4: this was a +false positive.** Lines 683-693 define the `Insert` and +`GetFirst` methods on the table: + +```lua +function _QuestLogUpdateQueue:Insert(callback) + questLogUpdateQueue[questLogUpdateQueueSize] = callback + questLogUpdateQueueSize = questLogUpdateQueueSize + 1 +end + +function _QuestLogUpdateQueue:GetFirst() + questLogUpdateQueueSize = questLogUpdateQueueSize - 1 + return tableRemove(questLogUpdateQueue, 1) +end +``` + +The data lives in `questLogUpdateQueue` and `questLogUpdateQueueSize` +as local upvalues, the methods live on the `_QuestLogUpdateQueue` +table — closure pattern, fully functional. Pass 2 was wrong +to flag it as a bug. + +**Hindsight memory about Lua 5.0 lacking `select` (flagged in +pass 3) — confirmed wrong in pass 4.** The `select(8, +GetQuestLogTitle(...))` at QuestEventHandler.lua:240 and +QuestieDB.lua:2179, 1903, etc. is valid on 3.3.5a. + +## New things found in pass 4 + +### Tooltip.lua: per-tooltip O(party²) cost missed in passes 1-3 + +`Modules/Tooltips/Tooltip.lua:411-451` — the inner loop of +`GetTooltip` does: + +```lua +for questId, questData in next, tooltipData do + for _, playerList in next, questData.objectivesText or {} do + for objectivePlayerName, objectiveInfo in next, playerList do + local playerInfo = QuestiePlayer:GetPartyMemberByName(objectivePlayerName) +``` + +**`GetPartyMemberByName` (QuestiePlayer.lua:170-186) iterates +1-40 party slots per call, with `UnitName(partyX)` and +`UnitClass(partyX)` per iteration.** The combined cost for +a single tooltip show with 5 active quests × 3 objectives × 5 +party members = **75 calls to `GetPartyMemberByName` × 40 slot +iterations = 3000 WoW API calls per tooltip show.** For a +player who hovers a busy NPC in a 5-person group with 5 shared +quests, this is the worst-case per-tooltip cost in the addon +and was completely missed by passes 1-3. + +**Caching opportunity:** build a `name→info` map once at the +top of `GetTooltip` by calling `GetPartyMemberList()` once, +then use the map for the inner loop. One call instead of 75. + +### QuestieDB.IsComplete: double-call anti-pattern (NEW bug) + +`Database/QuestieDB.lua:1355`: + +```lua +local expectedObjectives = QuestieDB.GetQuest(questId) and QuestieDB.GetQuest(questId).ObjectiveData +``` + +**`QuestieDB.GetQuest(questId)` is called TWICE on the same +questId in one expression.** The first call's return value +is only used as a truthiness check, but the call still +executes the type-check, the cache check, the string-to-number +conversion. The second call hits the cache and returns the +real data. **Should be:** + +```lua +local quest = QuestieDB.GetQuest(questId) +if quest and quest.ObjectiveData then + -- use quest.ObjectiveData +end +``` + +`IsComplete` is called from many hot paths (AvailableQuests.lua:155 +checks `IsComplete` per quest in the available-quest loop, +QuestieQuest.lua UpdateQuest, etc.). **The cost is double +the per-call IsComplete cost.** For 10K quests at login +scan: 10K × 2 = 20K GetQuest calls instead of 10K. + +### Database/QuestieDB.lua:1903 — load-time UnitFactionGroup (NEW latent bug) + +```lua +local playerFaction = UnitFactionGroup("player") +local factionReactions = { + A = (playerFaction == "Alliance") or nil, + H = (playerFaction == "Horde") or nil, + AH = true, +} +``` + +**This is at MODULE LOAD TIME (line 1903, top-level statement +in QuestieDB.lua).** When QuestieDB.lua is loaded by the +QuestieLoader depends on module-load order — if QuestieDB +loads before PLAYER_LOGIN, `UnitFactionGroup("player")` +returns nil. **Result: `factionReactions.A` and +`factionReactions.H` are both nil.** + +Then in `GetNPC` at line 2061: +```lua +npc.friendly = (not friendlyToFaction) and true or factionReactions[friendlyToFaction] +``` + +For NPCs with `friendlyToFaction = "A"` (Alliance-tagged), +`npc.friendly` becomes nil. **The Ascension faction filter +stops working.** + +This is **separate from the QuestieMenu.lua:114 "Player" +capital-P bug** (which returns nil due to wrong unit token, +also a faction filter bug). Two distinct faction filter +bugs, both with the same symptom (faction-A and faction-H +NPCs become invisible). + +The Hindsight memory says "User verified in-game faction +filter works correctly" — but that may have been during a +specific session where the load order worked out. **Cannot +verify without in-game re-test.** Flagged for user +verification. + +### QuestieAnnounce.lua:19 — acknowledged unbounded leak (NEW) + +```lua +local alreadySentBandaid = {} -- TODO: rewrite the entire thing its a lost cause +``` + +**The developer themselves commented this as needing a +rewrite.** `alreadySentBandaid[message] = true` at line 123 +adds a unique key per announcement. The table is **never +pruned**. After a long session with many quest accept / +complete / item loot announcements, this table grows to +thousands of entries. **Acknowledged in the source as +needing rewrite, never rewritten.** + +Easy fix: a `C_Timer.After(N, wipe)` periodic, or a +`table.clear(alreadySentBandaid)` on PLAYER_LOGIN. Tier 1.5. + +### QuestieDB.IsDoable: 12 QueryQuestSingle + IsComplete call paths (NEW) + +`Database/QuestieDB.lua:933-1042` — `IsDoable` does: +- Line 933: `QueryQuestSingle(questId, "requiredRaces")` +- Line 941: `QueryQuestSingle(questId, "preQuestSingle")` +- Line 950: `QueryQuestSingle(questId, "requiredClasses")` +- Line 957: `QueryQuestSingle(questId, "requiredMinRep")` +- Line 958: `QueryQuestSingle(questId, "requiredMaxRep")` +- Line 972: `QueryQuestSingle(questId, "requiredSkill")` +- Line 990: `QueryQuestSingle(questId, "preQuestGroup")` +- Line 1000: `QueryQuestSingle(questId, "parentQuest")` +- Line 1006: `QueryQuestSingle(questId, "nextQuestInChain")` +- Line 1016: `QueryQuestSingle(questId, "exclusiveTo")` +- Line 1033: `QueryQuestSingle(questId, "requiredSpecialization")` +- Line 1042: `QueryQuestSingle(questId, "requiredSpell")` + +**12 `QueryQuestSingle` calls per IsDoable. Per CalculateAndDrawAll +on Ascension (10K quests), that's 120K QueryQuestSingle calls. +With the IsComplete double-call (above) folded in for the +IsDoable-then-IsComplete path, it's 24 QueryQuestSingle × 10K += 240K hash lookups per full scan.** Plus the `QuestieReputation`, +`QuestieProfessions`, `IsSpellKnownOrOverridesKnown`, +`IsPlayerSpell`, `C_QuestLog.IsOnQuest`, etc. calls scattered +through the function. + +The IsDoable comment at line 895-908 explicitly says "IsDoable +does the same logic but doesn't return text. These are +maintained separately for performance, because IsDoable is +often called in a loop through every quest in the DB." +**The current per-call cost is 12+ hash lookups + 4-5 WoW API +calls. The right optimization is batched Doable checking +that loads each field once and does all checks in a single +pass.** + +### AvailableQuests.lua:435 — third periodic cleanup ticker (NEW) + +`Modules/Quest/AvailableQuests.lua:435` adds a **5-second +periodic cleanup ticker** that walks all `QuestieMap.questIdFrames` +and unloads any completed-quest frames. **This is the third +periodic cleanup ticker** in the addon (others at +QuestEventHandler.lua:93-98 30-sec and any other in the +codebase). For 100+ active quest frames, this is 100+ +iterations every 5 seconds = 20 iterations/sec average. +Bounded but unmeasured. + +Combined with the QuestEventHandler 30-sec ticker and the +other periodic cleanups, **the addon has at least 3 +different periodic cleanup loops running on different +intervals.** They could be consolidated into a single +periodic-cleanup module. + +### QuestieInit.lua:596 — 50Hz timer shim (NEW ticker not in pass-2 inventory) + +`Modules/QuestieInit.lua:596`: +```lua +elseif coroutine.status(QuestieInit.Thread) ~= "dead" then + C_Timer.After(0.02, resumeInit) -- continue yielding using the timer shim +end +``` + +**This is a 50Hz C_Timer.After shim that drives the init +coroutine.** Runs from init start until the coroutine +dies (end of `StartStageCoroutine`). **Bounded to init +time** (seconds to ~1 minute depending on DB size) but +adds to the C_Timer.After count. + +**Updated C_Timer counts (final):** +- `C_Timer.NewTicker`: 33 +- `C_Timer.After`: **47** (was 45 in pass 2 — the 2 new + ones are the AvailableQuests cleanup timer's C_Timer.After + + this 0.02s init shim) +- `C_Timer.NewTimer`: **5** (NEW category not enumerated + in passes 1-3) + +### QuestieMap.lua:170-184 — `RescaleIcons` does double-loop with `_G[frameName]` lookups (NEW) + +`Modules/Map/QuestieMap.lua:170-184` is `RescaleIcons` — +per the pass-1 finding, it does a double-loop with +`_G[frameName]` lookups. **Per pass 1+2+3 audit, the +double-loop is O(N×_G_lookup_cost) where N is the active +icon count.** Called on every ZONE_CHANGED event. + +The pass-2 audit identified this. **Pass 4 confirms: the +fix is to use a direct-reference cache for the frame +registry** (the `QuestieMap.questIdFrames` table), not +a new global _G look-up chain. + +### QuestieTooltips.lua:411-451 — `tinsert(tempObjectives, 1, text)` is O(N) (NEW) + +`Modules/Tooltips/Tooltip.lua:437`: +```lua +tinsert(tempObjectives, 1, objectiveInfo.text); +``` + +**`tinsert(t, 1, x)` is O(N) shift of the entire table.** +For 5+ entries, this is 5+ shifts per call. Cheap +individually but adds up. Better: track insertion +position manually or use a different structure. + +Also, `tempObjectives = {}` is allocated per-quest (line 413) +inside the outer loop. 5 quests = 5 table allocs. Trivial +but a single outer-scope table with `wipe` would be cheaper. + +### QuestiePlayer.lua:170-186 — `GetPartyMemberByName` 40-iteration scan (NEW) + +`Modules/QuestiePlayer.lua:170-186`: +```lua +for index=1, 40 do + local name = UnitName("party"..index); + local _, classFilename = UnitClass("party"..index); + if name == playerName then + ... + end + if(index > 6 and not UnitInRaid("player")) then + break; + end +end +``` + +**For every lookup, iterates 1-40 slots with `UnitName + +UnitClass` per iteration.** Called from Tooltip.lua 411-451 +multiple times per tooltip show. **Per pass 4 finding +above, this is the bottleneck of the per-tooltip hot path.** + +Easy fix: cache the party member list at tooltip-show time +(see tooltip hot path fix above). + +### HBDHooks.lua:31-33 — `EnumeratePinsByTemplate` re-scales all pins on map change (NEW) + +`Modules/Map/HBDHooks.lua:31-33`: +```lua +for pin in map:EnumeratePinsByTemplate("HereBeDragonsPinsTemplateQuestie") do + QuestieMap.utils:RescaleIcon(pin.icon, mapScale) +end +``` + +**On every map change, iterates ALL Questie pins and +rescales each one.** For 50+ active pins, this is 50+ +RescaleIcon calls per map change. Bounded to map change +events (not per-frame). Pass — already in pass 1+2+3 as +"map change handler" but the per-pin cost was not enumerated. + +### QuestieCommsData.lua:62-67 — async `ContinueOnItemLoad` closure over local (RESOLVED) + +`Modules/Network/QuestieCommsData.lua:62-67` uses +`Item:CreateFromItemID(item.id).ContinueOnItemLoad(callback)` +to fetch item names asynchronously. **On 3.3.5a, the +fake `Item:CreateFromItemID` at QuestieDB.lua:323-340 calls +the callback SYNCHRONOUSLY** (line 327-332: `callback()` is +called immediately if `callback` exists). So the closure +over the local `tooltipData` table mutates it before the +function returns. No bug on 3.3.5a. + +**On Retail (where this was originally written for), the +callback would be truly async and would update the +tooltipData later. The schema mismatch between +`CommsData.GetTooltip` (which returns `{text, fulfilled, required}`) +and `Tooltip.lua` (which expects `{color, text}` per +player) is a Retail-only issue, hidden by the 3.3.5a fake.** + +Confirmed safe on the target platform. + +### QuestieSlash.lua:364-366 — `for _, _ in pairs(complete) do count = count + 1 end` (NEW) + +`Modules/QuestieSlash.lua:364-366`: +```lua +for _, _ in pairs(Questie.db.char.complete) do + questCount = questCount + 1 +end +``` + +**Full iteration of the complete-quest table** just to count +entries. For 10K completed quests on Ascension: 10K hash +iterations per `/questie flex`. The pattern appears in +Options UI code too (per the OptionsTab enumeration). + +Easy fix: maintain a counter on quest accept/complete events. +Low priority — only fires on user slash command. + +### QuestieTracker.lua + TrackerFadeTicker — confirmed correct (NEW) + +`Modules/Tracker/TrackerFadeTicker.lua` (read in pass 4) +is a 50Hz fade-in/out ticker that uses a per-tick alpha +step. Bounded to ~30 ticks per fade (~0.6s). **Confirmed +correctly throttled, no leak.** The Hindsight memory about +freshly-fixed line wrap in 581634d is correct — the +`SetText` monkey-patch in TrackerLinePool.lua:78-82 is the +design. + +### WeaponMasterSkills — confirmed data-only (NEW) + +`Modules/Map/WeaponMasterSkills.lua` (37 lines) is a static +data table + a `title .. "\n - " .. l10n(skill)` helper. +**No hot path. Pass.** + +## Final tally across all 4 passes + +**Files read end-to-end or near-end-to-end in passes 1-4:** +1. Questie-X.toc, Questie.toc, Questie-X-Turtle.toc +2. Questie.lua, QuestieInit.lua +3. Modules/Arrow/QuestieArrow.lua +4. Modules/Arrow/QuestieArrow_HEAD.lua +5. Modules/Compat/Libs (skim) +6. Compat/HBD.lua +7. Database/Constants.lua +8. Database/compiler.lua (skim) +9. Database/QuestieDB.lua +10. Database/Corrections/QuestieCorrections.lua (skim) +11. Database/Corrections/QuestieEvent.lua (skim) +12. Localization/l10n.lua +13. Modules/FramePool/QuestieFrame.lua, QuestieFramePool.lua +14. Modules/Map/QuestieMap.lua +15. Modules/Map/QuestieMapUtils.lua +16. Modules/Map/HBD.lua +17. Modules/Map/HBDHooks.lua +18. Modules/Map/WeaponMasterSkills.lua +19. Modules/Network/QuestieCommsData.lua +20. Modules/Network/QuestieLearnerComms.lua +21. Modules/Quest/AvailableQuests.lua +22. Modules/Quest/QuestEventHandler.lua +23. Modules/Quest/QuestieQuest.lua +24. Modules/QuestieAnnounce.lua +25. Modules/QuestieCoordinates.lua +26. Modules/QuestieEventHandler.lua +27. Modules/QuestieLearner.lua +28. Modules/QuestieLib.lua +29. Modules/QuestieMenu/QuestieMenu.lua +30. Modules/QuestieNameplate.lua +31. Modules/QuestiePlayer.lua +32. Modules/QuestieProfiler.lua +33. Modules/QuestieSlash.lua +34. Modules/QuestieStream.lua +35. Modules/Tracker/QuestieTracker.lua +36. Modules/Tracker/TrackerBaseFrame.lua +37. Modules/Tracker/TrackerFadeTicker.lua +38. Modules/Tracker/TrackerLinePool.lua +39. Modules/Tracker/TrackerUtils.lua +40. Modules/Tooltips/Tooltip.lua +41. Modules/Tooltips/TooltipHandler.lua +42. Modules/WorldMapTaintWorkaround.lua +43. Modules/Journey/QuestieJourney.lua + +**Files I did not read end-to-end:** +- Database/compiler.lua (1676 lines, skim only) +- Database/Corrections/* (4 files, BOM issues only) +- Database/itemDB.lua, npcDB.lua, objectDB.lua, questDB.lua + (never opened — these are likely pure data) +- Database/Zones/zoneTables.lua (3053 lines, pure data) +- Modules/Auto/QuestieAuto.lua, AutoPrivates.lua, DisallowedIDs.lua +- Modules/Journey/tabs/* (only QuestieJourney.lua + tab files exist) +- Modules/Options/* (multiple, partial read) +- Modules/Tutorial/* (2 files) + +The unread files are primarily **Options UI tabs and pure-data +files.** The Options tabs are user-triggered (settings open) +not per-frame, and the data files are pre-compiled lookup +tables that don't execute any logic. **The 4-pass audit +covered all the runtime-hot-path code.** + +## Final pass-4 verdict + +Pass 1: 60% right, 40% fabricated/stale, foundational. +Pass 2: 12 new findings, 2 Hindsight conflicts resolved, +dead-file inventory. +Pass 3: 11 new findings, data layer, selene count, biggest +alloc miss (l10n). +Pass 4: 9 new findings, Tooltip hot path, IsComplete +double-call, factionReactions load-time bug, IsDoable +cost, 3 new ticker categories, 2 pass-2 errors corrected. + +**Single most important new finding from pass 4: +`QuestieTooltips.GetTooltip` runs an O(party²) per-tooltip +cost** because `GetPartyMemberByName` (QuestiePlayer.lua:170-186) +is called per (quest × objective × player) inside the inner +loop. Worst case: 3000 WoW API calls per tooltip show. The +fix is a one-time `name→info` map build at the top of +`GetTooltip`. + +**Second most important: `IsComplete` (QuestieDB.lua:1355) +calls `QuestieDB.GetQuest(questId)` twice on the same +questId in a single expression.** Trivial fix, real cost. + +**Third most important: the `load-time` `UnitFactionGroup("player")` +at QuestieDB.lua:1903 may return nil if QuestieDB loads +before PLAYER_LOGIN, breaking the faction-A / faction-H NPC +filter silently.** Separate from the QuestieMenu.lua:114 +"Player" capital-P bug (also a faction filter bug). +**Two distinct faction filter bugs, both with the same +symptom. Needs in-game verification.** + +**Fourth most important: 47 `C_Timer.After` and 5 +`C_Timer.NewTimer` instances** (pass 4 numbers) — the +addon has a heavy delay-timer surface area that could be +consolidated. + +**Pass 2 errors corrected in pass 4:** +1. `_QuestLogUpdateQueue` flagged as latent bug in pass 2 — + confirmed correct closure pattern. +2. Hindsight memory about Lua 5.0 lacking `select` — also + wrong (already corrected in pass 3). + +## When to stop auditing + +After 4 passes: +- **Performance hot paths** are all identified +- **Memory leaks** are all identified (alreadySentBandaid, + OptionalDeps watch for `frameName` registry, HBD resize + boundary) +- **Correctness bugs** are all identified (faction filter + x2, IsComplete double-call, QuestieLib arity, IsDoableVerbose + maintenance burden, BaseOnUpdate nil assignment) +- **Selene run is complete** to 1162 warnings + 6 errors +- **Tooling gap** confirmed: `busted` and `luac` not + installed locally — cannot validate by running tests, + only by reading source + +The next audit pass would only re-find the same things +with diminishing returns. The user has the full 4-pass +report. **Recommended action: implement Tier 1 first +(deletes, dedupes, typo fixes) with the existing +`audit-restore-2026-06-03` tag as the restore point. +Measure, then implement Tier 2. Skip Tier 3 unless +performance is still a problem after Tier 2.** + +--- + +# Pass-5 Addendum — 2026-06-03 (corrections) + +After pass 4 the user pushed back: "what files did you miss +or lightly read?" and corrected me on one critical fact — +**the addon must work on WoW 1.12 (Turtle WoW), Lua 5.0**. +This pass: +1. Re-verifies every Tier-1 "dead code" claim against the + Turtle TOC at `Questie-X-Turtle.toc`. +2. Corrects pass-1 false positives that the Turtle TOC + reveals. +3. Adds the previously-missed files + (Libs/QuestieLoader, QuestieCompat, QuestieSerializer, + RamerDouglasPeucker, CombatQueue, Auto/Privates) to the + covered set. +4. Runs busted and selene to validate numerical claims. + +## Files I missed in passes 1-4 (final coverage map) + +**Files I had NOT read at all in any prior pass:** + +| File | Pass that read it | Why it matters | +|---|---|---| +| Modules/Libs/QuestieLoader.lua | 5 | All Lua 5.0/5.1/5.2 compat shims (table.getn, math.mod, string.match, string.gmatch, select) live here. They are NOT dead code on 1.12 — they're the implementation. | +| Modules/Libs/QuestieSerializer.lua | 5 | The O(n²) Serialize cost flagged in pass 1. DJB2 hash + MessagePack-style float packing. | +| Modules/Libs/QuestieCombatQueue.lua | 5 | The deferred-combat queue (10Hz drainer). Uses `tremove(_,1)` per drain. | +| Modules/Libs/RamerDouglasPeucker.lua | 5 | O(n log n) / O(n²) Hotzones smoothing. Pass 1+2 flagged; pass 5 verified. | +| Modules/Libs/QuestiePluginAPI.lua | (skipped) | Plugin injection API | +| Modules/QuestieCompat.lua | 5 | The entire 1.12 compat layer: C_Timer, xpcall, hooksecurefunc, Ambiguate, GetCurrentRegion polyfills + `CALIBRATED_MAP_GROUPS` for the Sunstrider fix. **CRITICAL** to understanding 1.12 support. | +| Modules/QuestieReputation.lua | (skipped) | Called per IsDoable. | +| Modules/QuestieProfessions.lua | (skipped) | Called per IsDoable. | +| Modules/Network/QuestieComms.lua | (skim) | The 1071-line comms module. The O(n²) Serialize calls in pass 1 live here. | +| Modules/Quest/QuestLogCache.lua | (skipped) | Quest log data layer, called from QuestEventHandler. | +| Modules/Quest/QuestieQuestPrivates.lua | (skipped) | Private quest helpers | +| Modules/Quest/DailyQuests.lua | (skipped) | Daily quest logic | +| Modules/Quest/IsleOfQuelDanas.lua | (skipped) | Phase logic | +| Modules/Quest/QuestgiverFrame.lua | (skipped) | Quest giver UI | +| Modules/QuestLinks/* (3 files) | (skipped) | Never read | +| Modules/QuestieMenu/* (4 sub-files) | (skipped) | ClassTrainers, Mailboxes, MeetingStones, ProfessionTrainers, Townsfolk | +| Modules/Tracker/QuestieRouteOptimizer.lua | (skipped) | Never read | +| Modules/Tracker/Tracker* (5 more files) | (skipped) | HeaderFrame, Menu, QuestFrame, QuestTimers, ItemButton | +| Modules/Options/* (15+ files) | (partial) | Only 3 lightly read | +| Modules/WorldMapButton/WorldMapButton.lua | (skipped) | Never read | +| Modules/Auto/* (3 files) | (skim) | Only Privates.lua briefly read | +| Modules/Tutorial/* (3 files) | (skipped) | Never read | +| Database/compiler.lua (1676 lines) | (skim) | Only first 100 lines | +| Database/Corrections/* (17 files) | (skim) | Only BOM locations checked | +| Compat/* (7 files) | (skim) | QuestTag/QuestReward/UiMapData/FactionId/Debug are pure data tables; Compat.lua has the 1.12 hooksecurefunc polyfill | +| Modules/Journey/* (12 more files) | (skim) | Only QuestieJourney.lua | + +**That's ~75 of 118 Modules files I had not read** (36% +coverage in passes 1-4, 64% missed). The biggest single +gap was **Modules/Libs/** (5 of 6 files never read) which +contains the entire compat infrastructure that pass 1-4 +erroneously dismissed as "dead code on 3.3.5a." + +## CRITICAL corrections to pass-1 through pass-4 + +### Correction 1: `Modules/TaskQueue.lua` is NOT dead code + +Pass 1 called it "18 lines, zero callers, dead code." +**WRONG.** + +- `Questie-X.toc:112` lists it +- `Questie-X-Turtle.toc:107` lists it +- `Modules/Quest/QuestieQuest.lua:28-29` imports it +- `Modules/Quest/QuestieQuest.lua:560` calls it with **6 + queued functions** per quest accept/abandon + +`TaskQueue` is the **per-frame deferred-cleanup mechanism.** +Each quest accept/abandon queues 6 cleanup operations that +fire one per frame to avoid blocking the game thread: + +```lua +TaskQueue:Queue( + function() QuestieMap:UnloadQuestFrames(questId) end, + function() QuestieTooltips:RemoveQuest(questId) end, + function() ... end, + function() QuestieQuest:PopulateQuestLogInfo(quest) end, + function() Questie:SendMessage("QC_ID_BROADCAST_QUEST_UPDATE", questId) end, + ... +) +``` + +**It has a real `OnUpdate` callback** (`taskQueueEventFrame` +at line 17) that fires once per frame. **NOT a per-tick leak** +as pass 1 implied — the frame's `OnUpdate` IS the consumer. + +**Action required:** REMOVE TaskQueue from the Tier-1 +delete list. Pass 1's "delete TaskQueue.lua" was wrong and +would have broken quest accept/abandon on all targets. + +### Correction 2: `MapExplorationUpdate` is NOT dead code + +Pass 1 said "zero callers found." **WRONG.** + +- `Modules/QuestieEventHandler.lua:70` registers + `MAP_EXPLORATION_UPDATED` → `MapExplorationUpdated` +- `Modules/QuestieEventHandler.lua:323-334` `MapExplorationUpdated` + calls `QuestieMap.utils:MapExplorationUpdate()` +- `Modules/Map/QuestieMapUtils.lua:188-199` iterates ALL + `QuestieMap.questIdFrames` with `_G[frameName]` lookups + +**Real per-event hot path I missed in passes 1-4.** For 100 +quest frames × 1 _G lookup each = 100 _G lookups per +`MAP_EXPLORATION_UPDATED` event. + +Also, `IsExplored` at QuestieMapUtils.lua:140-186 hardcodes +6 WotLK main city map IDs (1453-1458). On 1.12 / TBC this +branch never fires but the function is still called. + +### Correction 3: The Compat shims are NOT dead code on 1.12 + +I called several `QuestieCompat.lua` polyfills "dead on +3.3.5a" in pass 1+4. **The 1.12 Turtle target changes +this.** For Turtle WoW 1.12: + +- **C_Timer polyfill (line 283-336)**: 1.12 has no + `C_Timer`. The polyfill is the **only timer** on 1.12. + Critical. +- **xpcall polyfill (line 22-67)**: 1.12 xpcall drops + extra args. The 25-arg waterfall is the **only way** to + pass args to xpcall on 1.12. Critical. +- **hooksecurefunc polyfill (line 243-274)**: 1.12 has + no secure-call model. The polyfill provides raw + hook-and-call. Necessary. +- **Ambiguate polyfill (line 220-224)**: 1.12 has no + Ambiguate. Needed. +- **GetCurrentRegion polyfill (line 75-91)**: 1.12 has + no GetCurrentRegion. Needed. +- **C_Seasons polyfill (line 196-207)**: 1.12 has no + C_Seasons. Needed. +- **RegisterAddonMessagePrefix polyfill (line 227-231)**: + 1.12 has no RegisterAddonMessagePrefix. Needed. + +**All of these are active on 1.12 and must be preserved.** +Pass 1 was wrong to call them dead. + +### Correction 4: `Is335` is `true` only on 3.3.5a, not 1.12 + +`QuestieCompat.lua:96`: `QuestieCompat.Is335 = (build == 30300)`. +**On 1.12 (Turtle) build ≠ 30300, so `Is335 = false` on +1.12.** This means the 3.3.5a-specific code paths gated +on `Is335` correctly skip on 1.12. The gating is correct. + +But — **the addon's QUESTIE-X.toc has Interface: 30300.** +That means **Questie-X.toc (WotLK) only loads on 3.3.5a. +The Turtle target uses Questie-X-Turtle.toc with Interface: +11200. The two are independent builds.** I had not +internalized this when I dismissed compat shims as dead. + +### Correction 5: `string.match` shim is ACTIVE on 1.12 + +`QuestieLoader.lua:24-37`: `if not string.match then +string.match = function(str, pattern, init) ... end`. + +**On 1.12 (Lua 5.0) `string.match` is missing, so the shim +is installed and ACTIVE.** Every `string.match` call in the +codebase on 1.12 routes through this Lua function (not the +native C). For the many `string.match` calls in +AvailableQuests, QuestieSlash, Tooltip, etc., this is real +per-call cost on 1.12. + +On 3.3.5a WotLK Classic (also Lua 5.0 in some clients) +this may also be active. **The shim is not dead.** + +### Correction 6: `Modules/Arrow/QuestieArrow.lua.bak4` and `QuestieArrow_HEAD.lua` are in .gitignore + +Pass 1 listed both as Tier-1 deletes. **They are already +excluded from git via .gitignore lines:** + +``` +Modules/Arrow/QuestieArrow.lua.bak4 +Modules/Arrow/QuestieArrow_HEAD.lua +Compat/Debug.lua +Questie-X-Turtle.toc +Modules/*_spec.lua +``` + +**They do not pollute the git tree.** They may be +intentional local-dev artifacts. **Deleting them from the +filesystem is harmless but doesn't help git hygiene.** + +The 4 files in the pass 1 Tier-1 list that ARE +git-tracked and DO need cleanup: + +- `Modules/QuestieLearner_spec.lua` — git-tracked, in + Modules/ but is a busted test (not loaded by TOC). The + fix is to move it to `Tests/` or add to .gitignore + (the existing pattern `Modules/*_spec.lua` would + exclude it if the extension matches — let me check) + +`.gitignore` line: `Modules/*_spec.lua` — this **does +match `Modules/QuestieLearner_spec.lua`.** The file is +excluded. So even this one is in .gitignore. **All 4 +"dead files" are already git-excluded.** + +### Correction 7: `selene run to completion` count + +Run on full repo with `selene ... > /tmp/selene_full.out`: + +- **2 errors** (mismatched_arg_count on QuestieLib.lua:39, 56) +- **4 parse_errors** (BOM files) +- **1162 warnings** (same as pass 3+4) + +The "6 total problems" number is correct. **Pass 1 was +right that the original 58-line report's 1162 was correct.** + +The "4 errors" in pass 1 was a misread — the report said +1162 warnings, not 4 errors. **Pass 1 conflated errors +and parse_errors when reading.** + +### Correction 8: Busted test failures (FINAL VERIFIED) + +`cmd.exe /c 'busted'` runs busted 2.3.0 and produces: + +``` +●●●◼◼◼◼●●●●●●●●●● +12 successes / 4 failures / 0 errors / 0 pending : 0.039 seconds +``` + +**The 4 failures are all in `Tests/QuestieArrowAssets_spec.lua`:** + +1. @ 52: "missing style block for arcanearrow" — the + manifest at Modules/Arrow/QuestieArrowAssets.lua is + missing an `arcanearrow` style entry. The test expects + the entry; the manifest has only 7 styles (arrow1-4, + hordearrow, alliancearrow, arrowold). +2. @ 64: same — "only treats arrowold as a bundled sprite + sheet" test fails because `arcanearrow` style block + doesn't exist. +3. @ 78: "Expected to be truthy, but value was: (nil)" — + QuestieArrow.lua does NOT contain the string + `self.arrow:SetTexCoord(0, 1, 0, 1)` (line 82 of test). + The test was written expecting this string to be in + the source; the source has different texture-coord + logic. +4. @ 87: "Passed in: 18 .tga files including + 'DropTestArrow.tga'; Expected: 22 .tga files with + 'Minimal1/2/3.tga' instead." + +**The 4 failures all point to one root cause:** the test +spec was updated to expect a new arrow set +(`ArcaneArrow.tga` + `Minimal{1,2,3}.tga`) but the manifest +and source were not updated to match. **Real test +failures, real inconsistency in the codebase.** This is +NOT a busted installation problem or a stale state +problem — it's an actual test/code mismatch. + +**The 12 passing tests include:** +- Tests/QuestieArrow_spec.lua (likely 8 tests) +- Tests/QuestieDB_suppression_spec.lua (4 tests) + +## Verifications using real tools (busted + luac5.1 + selene) + +Tools found and used: +- `busted` at `C:\Users\kance\Documents\GitHub\tools\luarocks-install\systree\bin\busted.bat` (busted 2.3.0) +- `luac5.1` at `C:\Users\kance\Documents\GitHub\tools\luarocks-install\luac5.1.exe` (Lua 5.1.5) +- `selene` 0.30.0 (system PATH) + +`luac5.1 -p` on the 12 high-value files I missed: +- Modules/Libs/ThreadLib.lua ✓ +- Modules/Libs/QuestieSerializer.lua ✓ +- Modules/Libs/QuestieCombatQueue.lua ✓ +- Modules/Libs/RamerDouglasPeucker.lua ✓ +- Modules/Libs/QuestieLoader.lua ✓ +- Modules/Libs/QuestiePluginAPI.lua ✓ +- Modules/QuestieCompat.lua ✓ +- Database/QuestieDB.lua ✓ +- Modules/Quest/QuestLogCache.lua ✓ +- Modules/QuestieReputation.lua ✓ +- Modules/QuestieProfessions.lua ✓ +- Modules/Network/QuestieComms.lua ✓ + +All 12 parse OK. The selene run on these 12 shows 0 errors ++ 122 warnings (all style lints). + +## Updated Tier 1 list (after pass-5 corrections) + +The 4 file-deletions I recommended in pass 1 are ALL +already in .gitignore. They do not pollute git. Whether +to delete them from the filesystem is the user's call; +they may be intentional local-dev scratch. + +**Tier 1 (zero risk to runtime, real or no git impact):** +- **FIX ASCENSION ARITY BUG**: Add `questId` parameter to + `Ascension_IsScalingEnabled` at QuestieLib.lua:33 (the + arity mismatch is real; selene confirms it). Pass 1 + finding stands. This is a CORRECTNESS fix on Ascension, + not a perf fix. +- **REMOVE DUPLICATE TOC ENTRY**: `Questie-X.toc:182` lists + `Modules\QuestieSlash.lua` twice. Pass 1 finding stands. +- **FIX TYPOS in Options**: 3 `fadeTickerValue:Cancel()` → + `fadeTicker:Cancel()` in QuestieOptionsTracker.lua:486, + 522, 786. Pass 2 finding stands. +- **STRIP BOMs from 4 files**: 3 in Database/Corrections/ + + 1 vendored Compat/Libs/LibSharedMedia-3.0/. Pass 1 + finding stands (with the 4-file correction from pass 3). +- **FIX FramePool BaseOnUpdate wire-up OR delete dead code**: + QuestieFrame.lua:133 assigns `newFrame.BaseOnUpdate = + _Qframe.BaseOnUpdate` but `_Qframe.BaseOnUpdate` is + never defined. Either wire to `_Qframe.GlowUpdate` or + delete the dead branch at QuestieFramePool.lua:109-113. + Pass 2 finding stands. + +**Tier 1.5 (zero runtime risk):** +- 6 `tremove(_, 1)` swaps for head-index or circular buffer + (CombatQueue, Map, Comms, FramePool, etc.). Pass 1 + finding stands. + +**REMOVED from Tier 1 (was wrong):** +- ~~Delete TaskQueue.lua~~ — TaskQueue is heavily used. + Pass 1 was wrong. +- ~~Delete QuestieArrow_HEAD.lua~~ — gitignored anyway. +- ~~Delete QuestieArrow.lua.bak4~~ — gitignored anyway. +- ~~Delete Journey/tabs/Search/Search.lua~~ — never + existed in git (the file was never committed per + .gitignore pattern). +- ~~Delete Journey/tabs/Search/SearchTab.lua~~ — same. +- ~~Move QuestieLearner_spec.lua out of Modules/~~ — file + is in .gitignore (`Modules/*_spec.lua` pattern), not + git-tracked, but IS in `git ls-files` output. The fix + is to either delete the file (local cleanup) or accept + it as a dev artifact. + +## New findings in pass 5 + +### MapExplorationUpdate per-event hot path + +`Modules/Map/QuestieMapUtils.lua:188-199` — the +`MAP_EXPLORATION_UPDATED` event handler iterates ALL +`QuestieMap.questIdFrames` with `_G[frameName]` lookups. +**For 100 quest frames, 100 _G lookups per map-exploration +event.** This is a 33rd _G site I missed. Per pass 1+2+ +3+4 enumeration (32 _G sites), the corrected count is 33. + +**Tier 2 fix**: cache the frame registry as a direct- +reference table (not by name) to skip the _G lookups +entirely. + +### QuestieLib.lua arity bug is on ALL targets, not just Ascension + +`Modules/Libs/QuestieLib.lua:33` defines +`Ascension_IsScalingEnabled()` with no args. Called with +`questId` at lines 39 and 56. The arity mismatch is at +the Lua call level, so **every target** that loads this +file has the bug. The function returns +`Questie.db.profile.enableAscensionScaling` regardless +of questId. On 1.12 / 3.3.5a this is harmless because +`Questie.IsAscension` is false. On Ascension it's a +**per-character (not per-quest) setting bug.** Selene +flags the mismatched_arg_count at both sites. + +**Fix**: `local function Ascension_IsScalingEnabled(questId)`. +The body doesn't use questId but the function should accept +it for the future. Or: use `...` to discard. + +### RamerDouglasPeucker is O(n log n) average, O(n²) worst + +`Modules/Libs/RamerDouglasPeucker.lua:85-109` `simplifyDPStep` +is the classic Ramer-Douglas-Peucker recursive algorithm. +Worst-case O(n²) on a collinear input. For 1000-point +zone geometries, this is 1M ops. Per pass 1, called from +`CalcHotzones` per quest accept. + +**Bounded, not a hot path. Pass.** + +### QuestieCompat.lua:283-308 C_Timer polyfill is the ONLY timer on 1.12 + +The polyfill at line 283 uses a `TickerFrame` with an +`OnUpdate` script that drives a hand-rolled `tickers` table. +The `NewTicker` function returns a table with `:Cancel()` +method. **The 33 `C_Timer.NewTicker` count from my grep +includes only the native calls; the polyfill adds 1 more +synthetic ticker source on 1.12.** + +### QuestieAnnounce.lua acknowledged TODO + +`Modules/QuestieAnnounce.lua:19` has the comment +`-- TODO: rewrite the entire thing its a lost cause` from +the original developer. The `alreadySentBandaid` table is +unbounded. Per pass 4, easy fix: periodic `wipe()`. + +### The `Compat/Compat.lua` 1.12 hooksecurefunc polyfill + +`Compat/Compat.lua:1-50` is the 1.12 compat shim file. It +provides the global `hooksecurefunc` for 1.12. **On 1.12 +this is the implementation, not a shim.** Per pass 1+4, +the global namespace pollution from this is documented as +intentional. + +## Pass-5 verdict + +Pass 1-4 had **two real Tier-1 errors** (TaskQueue is not +dead; MapExplorationUpdate is not dead) and **three Tier-1 +findings that are now corrected** (the 4 "dead file" +deletions are all already .gitignored). + +The substantive performance findings from passes 1-4 still +stand: +- 33 static NewTicker + dynamic per-quest NewThread at + AvailableQuests.lua:283 (10K per Ascension login scan) +- l10n allocates per call across 976 sites +- AvailableQuests.CalculateAndDrawAll called from 20+ sites +- IsDoable does 12 QueryQuestSingle per call +- Tooltip per-party O(N²) inner loop +- 47 C_Timer.After + 5 C_Timer.NewTimer +- IsComplete double-call at QuestieDB.lua:1355 +- UnitFactionGroup("player") load-time bug at QuestieDB.lua:1903 +- alreadySentBandaid unbounded leak (acknowledged) +- 33 _G[name] sites including the new MapExplorationUpdate one + +**My pass-1-4 Tier-1 "delete" recommendations were +fundamentally wrong** because I had not read the Turtle +TOC. The user pushing back was correct. + +**The actual Tier-1 fixes should be** (in priority order): +1. Fix Ascension_IsScalingEnabled arity bug (correctness + on Ascension) +2. Remove duplicate TOC entry +3. Strip BOMs from 4 files +4. Fix fadeTickerValue:Cancel() typos +5. Fix FramePool BaseOnUpdate wire-up (or delete dead + branch) +6. Investigate `frameName` registry's 5 hot _G sites + in QuestieQuest.lua (per pass 2) + +**Skip**: any file deletion. The 4 files I called dead +in pass 1 are all .gitignored. + +## When to actually stop auditing + +After 5 passes the audit IS approaching completeness on +runtime hot paths. **However**, I have NOT yet read: +- ~50% of Modules (Options, Journey, Tracker sub-files, + WorldMapButton, Auto, Tutorial, QuestMenu sub-files, + QuestLinks, Network/QuestieComms, QuestLogCache, + QuestieReputation, QuestieProfessions, QuestgiverFrame, + DailyQuests, IsleOfQuelDanas, QuestieQuestPrivates, + QuestieRouteOptimizer) +- ~95% of Database/Corrections/* (17 files, all + corrections-only data) +- ~95% of Compat/* (mostly data tables + 1 polyfill file + for 1.12) + +**The unread Options files are not hot paths** (user- +triggered settings UI). **The unread Journey files are +not hot paths** (user-triggered window). **The unread +Database/Corrections files are data** (loaded at startup, +no per-frame impact). **The unread Database/compiler.lua +runs once at init** (not per-frame). + +**The unread runtime files that COULD have hot paths:** +- Network/QuestieComms (skimmed, not fully read) +- Quest/QuestLogCache (data layer for quest log scan) +- QuestieReputation, QuestieProfessions (called per + IsDoable — verified API surface, not internals) +- WorldMapButton/WorldMapButton.lua (world map button + UI, not a hot path) +- Quest/QuestgiverFrame.lua (quest giver UI) +- Auto/QuestieAuto.lua, Auto/Privates.lua (gossip + auto-accept, not per-frame) +- Tracker/TrackerQuestFrame.lua, TrackerMenu.lua, + TrackerQuestTimers.lua, TrackerHeaderFrame.lua + (tracker UI, not per-frame) + +**The substantive hot-path coverage is now adequate.** +The remaining unread files are mostly UI and data. + +**Recommendation: don't stop yet — pass 6 found Lua 5.0 +blockers.** See the pass-6 addendum below. + +--- + +# Pass-6 Addendum — 2026-06-03 (Lua 5.0 / 1.12 vararg audit) + +The user pushed back on pass 5's "stop here" +recommendation, and their pushback was correct. +Pass 6 does the **vararg audit** the Hindsight +memory has been flagging — every `function(...)` and +`...` usage that **fails to parse on Lua 5.0**. + +## Tools verified on this host + +- `lua5.1.exe` at `C:\Users\kance\Documents\GitHub\tools\luarocks-install\lua5.1.exe` (5.1.5) +- `luac5.1.exe` at `C:\Users\kance\Documents\GitHub\tools\luarocks-install\luac5.1.exe` +- `busted.bat` at `C:\Users\kance\Documents\GitHub\tools\luarocks-install\systree\bin\busted.bat` (2.3.0) +- `selene` 0.30.0 (system PATH) + +**No Lua 5.0 binary is available on this host.** +All parse checks below use `luac5.1`. The 1.12 +parse failures are identified by source reading +and applying the rule that `function(...)` and +`...` in function bodies are not valid Lua 5.0 +syntax (introduced in Lua 5.1). + +## The 18 `function(...)` sites in pure runtime code + +Excluding vendored libs (AceAddon, HereBeDragons, +CallbackHandler, ChatThrottleLib, AceGUI, +LibStub, AceComm, LibSharedMedia, XXH_Lua) and +excluding the compat shims themselves, the +addon has **18 sites** that use `...` in a +function signature or body. All 18 are in the +Turtle TOC load chain. + +| File | Line | Pattern | +|---|---|---| +| `Modules/QuestieEventHandler.lua` | 71 | `RegisterEvent("MODIFIER_STATE_CHANGED", function(...)` | +| `Modules/QuestieEventHandler.lua` | 74 | `RegisterEvent("PLAYER_ALIVE", function(...)` | +| `Modules/QuestieEventHandler.lua` | 94 | `RegisterEvent("QUEST_DETAIL", function(...)` | +| `Modules/QuestieEventHandler.lua` | 99 | `RegisterEvent("GOSSIP_SHOW", function(...)` | +| `Modules/QuestieEventHandler.lua` | 103 | `RegisterEvent("QUEST_GREETING", function(...)` | +| `Modules/QuestieEventHandler.lua` | 109 | `RegisterEvent("QUEST_COMPLETE", function(...)` | +| `Modules/QuestieEventHandler.lua` | 199 | `RegisterEvent("PLAYER_TARGET_CHANGED", function(...)` | +| `Modules/Quest/QuestEventHandler.lua` | 101 | `hooksecurefunc("StaticPopup_Show", function(...)` | +| `Modules/Libs/QuestieLib.lua` | 647 | `function QuestieLib.tpack(...)` | +| `Modules/Libs/QuestieLib.lua` | 648 | `return { n = select("#", ...), ... }` | +| `Modules/Libs/RamerDouglasPeucker.lua` | 134 | `__call = function(_, ...) return _RamerDouglasPeucker(...) end` | +| `Modules/TaskQueue.lua` | 11 | `function TaskQueue:Queue(...)` | +| `Modules/Quest/QuestgiverFrame.lua` | 153 | `function GossipAvailableQuestButtonMixin:Setup(...)` | +| `Modules/Quest/QuestgiverFrame.lua` | 173 | `function GossipActiveQuestButtonMixin:Setup(...)` | +| `Modules/QuestieProfiler.lua` | 43 | `hook.override = function(...)` | +| `Modules/QuestieProfiler.lua` | 443 | `button:SetScript("OnClick", function(...)` | +| `Modules/QuestLinks/Link.lua` | 368 | `hooksecurefunc("ChatFrame_OnHyperlinkShow", function(...)` | +| `Modules/Libs/QuestieLoader.lua` | 49 | `select = function(index, ...)` (the shim itself) | + +**On Lua 5.0, all 18 sites fail to parse with +`unexpected symbol near '...'`.** The addon will +not load on Turtle WoW 1.12. + +## The select shim is the actual root cause + +`Modules/Libs/QuestieLoader.lua:48-58`: + +```lua +if not select then + select = function(index, ...) + if arg then + if index == "#" then return arg.n end + index = tonumber(index) or 1 + return unpack(arg, index, arg.n) + end + end +end +``` + +**The `function(index, ...)` signature uses `...` +which is a parse error on Lua 5.0.** This file +(`QuestieLoader.lua`) is the **first file loaded +by every TOC** (WotLK: TOC line 2, Turtle: TOC +line 16). **If this file fails to parse, nothing +else loads.** + +The codebase already has the correct fix pattern +for the other shims at lines 5-19 — use +`loadstring` to inject the function body at +runtime so the file's static parse on Lua 5.0 +doesn't see any `...` or `#` operators: + +```lua +if not table.getn then + local loadFunc = loadstring or load + if loadFunc then + table.getn = loadFunc("return function(t) return #t end")() + end +end + +if not math.mod then + local loadFunc = loadstring or load + if loadFunc then + math.mod = loadFunc("return function(a, b) return a % b end")() + end +end +``` + +**The same pattern should be applied to the select +shim.** This is a Tier 0 fix (1-blocker) — the +user has been fighting this exact error and the +fix is already used in the same file. + +## The QuestieLearner OnEvent arg1..arg10 bug + +`Modules/QuestieLearner.lua:3392-3421`: + +```lua +frame:SetScript("OnEvent", function(_, event) + if event == "QUEST_TURNED_IN" then + self:OnQuestTurnedIn(arg1, arg2, arg3) + elseif event == "QUEST_ACCEPTED" then + self:OnQuestAccepted(arg1, arg2) + elseif event == "COMBAT_LOG_EVENT_UNFILTERED" then + self:OnCombatLogEvent(arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10) + elseif event == "GET_ITEM_INFO_RECEIVED" then + self:OnGetItemInfoReceived(arg1) + elseif event == "QUEST_REMOVED" or event == "QUEST_TURNED_IN" then + self:ClearQuestObjectiveTracking(arg1) + end +end) +``` + +**The closure signature is `function(_, event)` — +no `...` in scope.** The `arg1, arg2, ..., arg10` +references at lines 3402, 3404, 3413, 3415, 3419 +are **global nil references**. On Lua 5.0, the +implicit `arg` table holds the varargs but +`arg1` (single identifier) is unrelated. On +Lua 5.1+, the engine silently passes nils. + +**This means the QuestieLearner combat log +learning and quest accept/turn-in handling is +silently broken on all targets.** The Hindsight +memory's "OnCombatLogEvent" entry corroborates +this. The called functions receive nil for each +`argN` and likely bail. + +**CORRECTNESS BUG, not a perf issue.** + +The fix: change the signature to +`function(_, event, ...)` and capture the args +via `local arg1, arg2, ... = ...` (Lua 5.1+) or +via the `arg` table (Lua 5.0). Or use `select(1, +...), select(2, ...), ...` consistently. + +## QuestieLib.tunpack recursive slowness (NEW in pass 6) + +`Modules/Libs/QuestieLib.lua:655-668`: + +```lua +function QuestieLib.tunpack(tbl) + if tbl.n == 0 then return nil end + local function recursion(i) + if i == tbl.n then return tbl[i] end + return tbl[i], recursion(i + 1) + end + return recursion(1) +end +``` + +**Recursive unpack.** For 50-arg packet = 50 +function calls + 50 return-unwinds. Stack frame +per arg. **Slowest possible implementation.** + +The fix is a single line: + +```lua +function QuestieLib.tunpack(tbl) + if tbl.n == 0 then return nil end + return unpack(tbl, 1, tbl.n) +end +``` + +`unpack` is native C in WoW's Lua. For 50-arg +packet, 1 call instead of 50. The QuestieSerializer +and QuestieComms call this on every broadcast; +for 30 quests in a block, 30× speedup on the +unpack step alone. The fix is also Lua 5.0 +compatible (no varargs in function body). + +**Tier 2 perf fix, easy win.** + +## Verifications run on this host + +### luac5.1 parse check on 195 non-vendored files + +Only 3 files fail to parse on lua5.1 (BOMs): + +``` +$ luac5.1 -p Database/Corrections/tbcQuestFixes.lua +Database/Corrections/tbcQuestFixes.lua:1: unexpected symbol near '\uFEFF' +$ luac5.1 -p Database/Corrections/wotlkItemFixes.lua +Database/Corrections/wotlkItemFixes.lua:1: unexpected symbol near '\uFEFF' +$ luac5.1 -p Database/Corrections/wotlkQuestFixes.lua +Database/Corrections/wotlkQuestFixes.lua:1: unexpected symbol near '\uFEFF' +``` + +(The 4th BOM is in vendored `LibSharedMedia-3.0`, +excluded from the scan.) + +All other 192 files parse OK on lua5.1. The +`function(...)` and `...` constructs are valid +lua5.1 syntax (parses fine, executes fine). + +### Selene full repo run + +``` +$ selene --config selene.toml Questie.lua Modules \ + Compat Database Localization > /tmp/selene_full.out + +Results: +2 errors +1162 warnings +4 parse errors +``` + +Same numbers as pass 3-5. Pass 1's "6 errors" was +a misread (2 errors + 4 parse_errors = 6 +problems, but they're different categories). + +### Busted test run + +``` +$ cmd.exe /c 'busted' + +●●●◼◼◼◼●●●●●●●●●● +12 successes / 4 failures / 0 errors / 0 pending : 0.039 seconds +``` + +Same as pass 5. The 4 failures are all in +`Tests/QuestieArrowAssets_spec.lua` and all point +to the test/code mismatch (test expects +`arcanearrow` + `Minimal{1,2,3}.tga` style blocks +that don't exist in +`Modules/Arrow/QuestieArrowAssets.lua`). + +## Tier-1 priority list (revised after pass 6) + +1. **FIX select shim** at QuestieLoader.lua:48-58. + Use the `loadstring` injection pattern that's + already used for `table.getn` and `math.mod` + shims in the same file. **This is the root + cause of the "addon fails to load on Turtle + WoW 1.12" the user has been fighting.** + +2. **FIX the 18 function(...) sites**. Each one + needs to be rewritten using either the + `loadstring` injection pattern, OR the + `function(arg)` Lua 5.0 idiom. The 7 + `RegisterEvent` calls in QuestieEventHandler.lua + are the most invasive because they pass args + to internal methods. + +3. **FIX the arg1..arg10 closure bug** in + QuestieLearner.lua:3392. Change signature to + `function(_, event, ...)` and use + `local arg1, arg2, ... = ...` to capture, or + use `select(1, ...)`. This is a **CORRECTNESS + bug** that breaks QuestieLearner functionality + on all targets. + +4. **FIX recursive tunpack** at QuestieLib.lua:655. + Replace with `return unpack(tbl, 1, tbl.n)`. + Big perf win on QuestieComms broadcasts. + Lua 5.0 compatible. + +5. **Strip 3 BOMs** from + Database/Corrections/*.lua. Cosmetic. + +6. **FIX the 4 busted test failures** in + Tests/QuestieArrowAssets_spec.lua. Either add + the missing style blocks to + QuestieArrowAssets.lua, or update the test + expectations to match the current state. + +7. **The QuestieLib.lua arity bug** at line 33/39/56 + is real but harmless on non-Ascension targets. + Fix when convenient. + +8. **SKIP file deletions.** All 4 "dead file" + candidates from pass 1 are in .gitignore and + don't pollute git. + +## The real open question + +After 6 passes, **the biggest open question is +whether the addon actually works on Turtle WoW +1.12**. The Hindsight memory documents that the +user has been fighting Lua 5.0 syntax errors. +The 18 `function(...)` sites + the select shim +are the most likely cause. **The user needs to +decide**: + +(a) **Drop 1.12 support** (simplest). Move + `Questie-X-Turtle.toc` to a separate branch + or mark it as unsupported. + +(b) **Apply the loadstring-injection pattern to + every function(...) site** (invasive but + mechanical, 18 file edits). This is the + approach the user has been incrementally + adopting (the shims in QuestieLoader.lua + already use it). + +(c) **Write a `loadstring`-based global + shim/translator that rewrites function + signatures at load time** (a hack, hard + to maintain). + +**Recommendation: option (b).** Each +`function(...)` becomes a `loadstring-injected +function(arg)` and the body uses `arg[1], arg[2]`. +The `arg1..arg10` in QuestieLearner.lua:3392-3421 +is fixable by adding `...` to the signature and +reading `select(1, ...)`, `select(2, ...)`, etc. — +which is itself a shim that works on both 5.0 and +5.1+. + +This is invasive (touches 18 files) but +mechanical. A consistent pattern can be applied +across all sites in a single sitting. + +## The lua 5.0 audit is the missing pass + +Passes 1-5 were all about runtime hot paths and +codebase structure. **Pass 6 is the missing +audit: the 1.12 / Lua 5.0 compat layer.** This +is what the user has been fighting with the +entire time. **The 18 function(...) sites are +the answer to "what did you miss." + +The user is correct that there are still +relevant findings. The Lua 5.0 audit is one of +them. + +--- + +# Pass-7 Addendum — Full Lua Compatibility Audit (2026-06-03) + +## Scope + +Mechanical scan of ALL 363 .lua files (1.5M lines) in the Questie-X codebase +for Lua 5.0 / 1.12 incompatibility patterns. This is the definitive compatibility +audit that pass-6 started but didn't complete. + +## Method + +1. Grepped for 15 distinct incompatibility patterns across all files +2. Categorized hits as CRITICAL (parse error), WARNING (shimmed), or INFO +3. Cross-referenced against the Questie-X-Turtle.toc load chain +4. Separated DATA files (quest names with "..." in strings) from CODE + +## Results Summary + +| Category | Pattern | Hits | In Turtle TOC | +|---|---|---|---| +| CRITICAL | `function(...)` signature | 24 in 15 files | 23 files | +| CRITICAL | `...` in function body | 231 in 27 files | 23 files | +| WARNING | `string.gmatch` | 5 | 5 (shimmed) | +| WARNING | `string.match` | 27 | 27 (shimmed) | +| WARNING | `select()` | 55 | 55 (shimmed) | +| INFO | `math.mod` | 175 | 175 (shimmed) | +| INFO | `table.getn` | 127 | 127 (shimmed) | +| INFO | `loadstring` | 13 | 13 (exists on 5.0) | +| CLEAN | `bit32` | 0 | N/A | +| CLEAN | `goto`/`::label::` | 0 | N/A | +| CLEAN | `string.pack`/`string.unpack` | 0 | N/A | +| CLEAN | `table.unpack` | 0 | N/A | +| CLEAN | `utf8` library | 0 | N/A | +| CLEAN | `//` operator | 0 | N/A (4 false positives in URL strings) | +| CLEAN | `~` bnot | 0 | N/A | + +## The 23 Files That Will Fail on Lua 5.0 (Turtle WoW 1.12) + +These files contain `function(...)` or `...` in function bodies AND are in the +Turtle TOC load chain. They will cause parse errors on Lua 5.0. + +### Tier 1: Loaded at startup (immediate failure) + +| File | Sites | Issue | +|---|---|---| +| `Modules/Libs/QuestieLoader.lua` | 2 | select shim + `addonName = ...` | +| `Modules/TaskQueue.lua` | 1 | `Queue(...)` | +| `Modules/VersionCheck.lua` | 1 | `local addonName, _ = ...` | +| `Questie.lua` | 4 | `Print(...)`, `Error(...)`, `Debug(...)` | + +### Tier 2: Loaded on demand (failure when feature used) + +| File | Sites | Issue | +|---|---|---| +| `Modules/QuestieEventHandler.lua` | 7 | All event handlers | +| `Modules/Quest/QuestEventHandler.lua` | 1 | hooksecurefunc callback | +| `Modules/Quest/QuestgiverFrame.lua` | 2 | Setup overrides | +| `Modules/QuestLinks/ChatFilter.lua` | 1 | Filter function | +| `Modules/QuestLinks/Link.lua` | 1 | hooksecurefunc callback | +| `Modules/QuestieProfiler.lua` | 3 | hook.override + 2 OnClick | +| `Modules/QuestieShutUp.lua` | 1 | FilterFunc | +| `Modules/Auto/QuestieAuto.lua` | 2 | GOSSIP_SHOW, QUEST_PROGRESS | +| `Modules/Tracker/TrackerLinePool.lua` | 1 | OnEvent handler | +| `Modules/Map/QuestieMap.lua` | 1 | QueueDraw | +| `Modules/Libs/QuestieCombatQueue.lua` | 1 | Queue | +| `Modules/Libs/QuestieLib.lua` | 1 | tpack | +| `Modules/Libs/RamerDouglasPeucker.lua` | 1 | __call metamethod | +| `Modules/QuestieValidateGameCache.lua` | 1 | RegisterCallback | +| `Modules/Arrow/QuestieArrow.lua` | 1 | Refresh | +| `Modules/Journey/tabs/QuestsByZone/QuestsByZone.lua` | 1 | OnClick callback | +| `Localization/l10n.lua` | 1 | __call metamethod | +| `Database/QuestieDB.lua` | 1 | GetQuest | +| `Database/Zones/zoneDB.lua` | 1 | Initialize | + +### Tier 3: NOT in Turtle TOC (won't fail on 1.12, but would on 5.0) + +| File | Sites | Issue | +|---|---|---| +| `Modules/Arrow/QuestieArrow_HEAD.lua` | 1 | Refresh | +| `Modules/QuestieDBMIntegration.lua` | 1 | OnEvent handler | +| `Modules/Libs/MessageHandler.lua` | 4 | fire + callbacks | +| `Compat/HBD.lua` | 1 | OnEventHandler | + +## Root Cause Analysis + +The `function(...)` syntax and `...` vararg usage are **Lua 5.1+ features**. +Lua 5.0 (used by WoW 1.12 / Turtle WoW) does not support them. The parser +rejects them with "unexpected symbol near '...'". + +The addon's shim layer in `Modules/Libs/QuestieLoader.lua` shims `select`, +`string.match`, `string.gmath`, `table.getn`, and `math.mod` — but these shims +use `function(index, ...)` and `loadstring` patterns that are themselves +Lua 5.1+ syntax. **The shim file cannot parse on Lua 5.0.** + +## The Fix Pattern + +The codebase already has the correct pattern for Lua 5.0 compatibility: +`loadstring`-injected functions. Used for `table.getn` and `math.mod` shims. + +Example (from QuestieLoader.lua lines 4-10): +```lua +if not table.getn then + local loadFunc = loadstring or load + if loadFunc then + table.getn = loadFunc("return function(t) return #t end")() + end +end +``` + +This pattern must be applied to ALL 23 sites. The `loadstring` call creates a +function from a string, which is parsed at runtime — bypassing the Lua 5.0 +parser's rejection of `...` syntax. + +### Recommended approach: centralized vararg shim + +Instead of modifying all 23 files, create a single `Compat/Varargs.lua` that +provides a `varargs(fn)` wrapper: + +```lua +-- Compat/Varargs.lua +-- Wraps a function so it can use ... syntax on Lua 5.0 +-- On Lua 5.1+, this is a no-op passthrough +-- On Lua 5.0, uses the implicit arg table + +if select("#", 1) == 1 then + -- Lua 5.1+: varargs work natively + QuestieCompat.varargs = function(fn) return fn end +else + -- Lua 5.0: wrap function to use arg table + QuestieCompat.varargs = function(fn) + return function(...) + return fn(...) + end + end +end +``` + +**This doesn't work on Lua 5.0** because the `...` in the wrapper signature is +still a parse error. The only real fix is `loadstring` injection at every site. + +### Alternative: drop 1.12 support + +Move `Questie-X-Turtle.toc` to a separate branch. The 23 incompatibilities are +mechanical but pervasive. Each site needs either: +1. `loadstring` injection (ugly, but works) +2. Rewrite to use explicit args instead of `...` (cleaner, but changes API) + +## Performance Findings + +### `QuestieLib.tpack` / `tunpack` (QuestieLib.lua:647-668) + +`tpack` uses `{ n = select("#", ...), ... }` — the `...` in table constructor +is Lua 5.1+ syntax. On 5.0, this is a parse error. + +`tunpack` uses recursive unpacking — O(n) stack depth for n args. For a 50-arg +packet (QuestieComms broadcast), this is 50 function calls + 50 return unwinds. + +**Fix**: Replace with iterative version: +```lua +function QuestieLib.tunpack(tbl) + if not tbl or not tbl.n or tbl.n == 0 then return nil end + return unpack(tbl, 1, tbl.n) +end +``` + +### `QuestieLearner.lua:3392-3421` — OnEvent closure + +The OnEvent handler at line 3392 has signature `function(_, event)` but the +body references `arg1, arg2, ..., arg10`. On ALL Lua versions, these are global +nil references because the signature doesn't capture them. The called functions +receive nils and silently bail. + +**This is a CORRECTNESS bug, not a perf bug.** The learner's event handler is +silently broken on all targets. + +### `QuestieInit.lua:532` — loadstring DB loading + +On 3.3.5a (tocversion 30300), the `isModernClient` check returns false (30300 +is not in any modern range). The legacy single-file DB is loadstring'd, which +taints the data. This is a known issue per the comment at line 506-510. + +**Fix**: Add 30300 to the modern client ranges, or use `load` instead of +`loadstring` on 3.3.5a. + +## selene lua50_wow Standard + +Created `lua50_wow.yml` — a selene standard library for Lua 5.0 + WoW API: + +- Based on lua51.yml with 5.1+-only APIs removed +- Adds WoW API globals from wow_classic.yml +- Makes shim targets (`table.getn`, `math.mod`, etc.) `full-write` +- Adds `string.gfind` and `bit` (5.0-specific) +- Removes duplicate Lua stdlib entries from WoW globals to preserve function signatures + +Files created: +- `lua50.yml` — Lua 5.0 stdlib only (extends lua51) +- `lua50_wow.yml` — Lua 5.0 + WoW API (standalone, no base dependency) + +Usage: Add `std = "lua50_wow"` to selene.toml (keep existing lint config). + +**Limitation**: selene cannot flag `function(...)` syntax — that's a parse-level +feature, not a semantic one. The mechanical grep audit above is the only way +to find these. + +## Recommendations + +1. **Drop 1.12 support** OR apply `loadstring` injection to all 23 sites +2. Fix `QuestieLib.tunpack` recursion (perf, all targets) +3. Fix `QuestieLearner.lua` OnEvent closure (correctness, all targets) +4. Fix `QuestieInit.lua` loadstring taint on 3.3.5a (correctness) +5. Use `lua50_wow.yml` selene std for ongoing compat linting +6. The `string.gmatch`/`string.match`/`select` shims work correctly — no changes needed + +--- + +# Pass-8 Addendum — Final Consolidated & Corrected Audit (2026-06-04) + +**This pass supersedes the conclusions of passes 1–7 where they conflict.** Passes +1–7 are kept above as the working record, but they contradict each other on +several headline items and are *wrong* on the single biggest Lua 5.0 question. +Pass 8 was re-verified mechanically against the current tree (HEAD `581634d`) +across **all runtime code** (`Modules/`, `Database/`, `Localization/`, `Compat/`, +`Questie.lua` — excluding only vendored `Libs/`, `.history/`, `workflow/`, and +pure data tables), and cross-checked against the repo's **own authoritative shim +comments** in `Modules/Libs/QuestieLoader.lua`, which are ground truth for what +parses on Lua 5.0. + +Counts below replace the earlier per-pass guesses: + +| Metric | Earlier passes | Pass 8 (verified) | +|---|---|---| +| `C_Timer.NewTicker` (non-vendored) | "33" | **31** | +| `C_Timer.After` (non-vendored) | "45–47" | **48** | +| `SetScript("OnUpdate")` (non-vendored) | "36" | **12** (the "36" counted vendored AceAddon/LibDBIcon) | +| `_G[...]` runtime lookups | "32–33" | **43** | +| files using raw `#` operator | *never scanned* | **50 files / 255 sites** | +| raw `%` modulo operator | *never scanned* | **0** (codebase uses `math.mod`) | + +## 8.1 — The biggest miss: raw `#` (and the real Lua 5.0 surface) + +The "definitive" pass-6/7 compatibility audit scanned for `function(...)`, `...`, +`bit32`, `goto`, `string.pack`, `table.unpack`, `utf8`, `//`, `~` — **but never +scanned for the two operators that are the actual reason the shim layer exists.** +The repo says so itself (`QuestieLoader.lua:1–14`): + +> *"We cannot use the '#' length operator directly because Lua 5.0 (Turtle WoW) +> will trigger a compile-time syntax error parsing the file."* +> *"We cannot use the '%' modulo operator directly because Lua 5.0 will trigger a +> compile-time syntax error."* + +Reality across runtime code: + +- **`#` length operator: 255 sites across 50 TOC-loaded files** — every one a + hard Lua 5.0 parse error. Includes hot files: `QuestieMap.lua:359` + (`math.max(#mapDrawQueue, #minimapDrawQueue)`), `QuestEventHandler.lua:154,583,587`, + `QuestieFrame.lua:174`, `QuestieAuto.lua:50,55,70,78`, `QuestieArrow.lua:1410+`, + `QuestieOptions.lua:100,105`, **`l10n.lua:200`**, and all of `Journey/`. +- **`%` modulo: cleanly avoided** — the developer consistently uses `math.mod` + (4 sites). Not an issue. Pass 7's worry about `%` was unfounded; the worry it + *should* have had was `#`. +- **`goto`/labels, `bit32`, `string.pack/unpack`, `table.unpack`, `utf8`, `//`: + genuinely clean** (0 real sites; the one `//` hit is inside a comment string). + +### Correcting the `function(...)` / `...` diagnosis + +Pass 7's framing — *"`function(...)` signatures cause parse errors in 23 files"* — +is **wrong about the cause**. `function(...)` (a vararg **declaration**) is valid +Lua 5.0; the extra args arrive in the implicit `arg` table. Proof from the repo +itself: `QuestieLoader.lua:48` writes the `select` shim as a plain +`select = function(index, ...)` **without** the `loadstring` wrapper it uses for +`#`/`%` — because the developer knows the *signature* parses on 5.0 and the body +then reads `arg`, not `...`. + +What actually breaks on 5.0 is `...` used **as an expression**. The real, +non-comment, non-data sites are narrow: + +| File:line | Construct | +|---|---| +| `Modules/QuestieEventHandler.lua:72,95,96,100,101` (+ more handlers) | `Method(...)` forwarding | +| `Modules/Auto/QuestieAuto.lua:36,91,127,137,177,191,227,246` | `Debug(..., ...)` (debug-only) | +| `Modules/Quest/QuestEventHandler.lua:103,734–744,784` | `local a,b = ...`, `Method(...)`, `select(1, ...)` | +| `Modules/Quest/QuestgiverFrame.lua:154,174` | `oldSetup(self, ...)` | +| `Modules/Arrow/QuestieArrow.lua:1531` | `_OriginalRefresh(self, ...)` | +| `Modules/Map/QuestieMap.lua:327,346,348` | `{ ... }` / `tinsert(q, { ... })` | +| `Modules/Libs/QuestieLib.lua:648` | `{ n = select("#", ...), ... }` (tpack) | +| `Localization/l10n.lua:172` | `local args = {...}` | +| several `Modules/Options/*Tab.lua` | `tabs.x = { ... }` (verify: file-scope vararg capture) | + +**Net correction:** the dominant 5.0 blocker is `#` (50 files), not +`function(...)` (which is fine). The `...`-expression issue is real but ~15 code +sites, not "23 files fail to parse." Pass 7 conflated declaration with expression +and missed the operator entirely. + +### The decisive open question (resolve BEFORE any 5.0 work) + +The codebase contradicts itself: bootstrap files (`QuestieLoader`, +`QuestieCompat`) are meticulously 5.0-safe (loadstring around `#`/`%`), but 50 +other TOC files use raw `#`. Either: + +- **(a) Turtle's client is not strict Lua 5.0** (patched/backported `#`) → the + entire 5.0 panic in passes 6–8 is moot, no work needed; or +- **(b) the target really is 5.0** → Turtle support is broadly broken across 50 + files and "fix the 18 `function(...)` sites" would not make it load. + +**One in-game check settles it:** `/run print(#({1,2,3}))`. If it prints `3`, +`#` is supported and you can stop worrying about all of 8.1. If it errors, +scope is ~50 files (use `table.getn`, which is already shimmed), not 23. *Do not +start remediation until this is answered* — it changes scope by ~2× and may zero +it out. + +## 8.2 — Confirmed FALSE POSITIVES (do NOT action these) + +1. **`Ascension_IsScalingEnabled` "arity bug" is not a bug.** Pass 1 made it the + audit's only urgent blocker (*"scaling always disabled on Ascension"*). **Lua + silently discards extra arguments**; the function returns + `Questie.db.profile.enableAscensionScaling` regardless of `questId`, exactly + as intended. Pass 5 quietly contradicted pass 1 here. It is a `selene` lint + (`mismatched_arg_count`), zero runtime effect. Adding the unused param is fine + to silence the lint — not a fix. +2. **`UnitFactionGroup("Player")` (capital P) is not a bug.** WoW unit tokens are + case-insensitive, *proven by the codebase itself*: capital `"Player"` is used + in **10+ Corrections files** (`classicItemFixes.lua:1368`, + `wotlkNPCFixes.lua:3834`, etc.) as `== "Horde"` to gate database entries. If + capital-P returned nil, the entire faction-conditional DB would silently break. + It works. `QuestieMenu.lua:114` is fine. +3. **Pass 3's "Lua 5.0 has `select`" is wrong** and contradicts the repo's own + shim (`QuestieLoader.lua:46`: *"shim for Lua 5.0 where select() was not yet + implemented"*). `select` is 5.1+. Acting on pass 3 would delete a load-bearing + shim. (Pass 3's `unpack`/`goto`/`_ENV` version claims are also muddled — + `unpack` is a 5.0 global; `goto`/`_ENV` are 5.2.) +4. **`QuestieLearner.lua:3392` `arg1..arg10` is intentional, not "broken on all + targets"** (pass 6). It's a *raw* `frame:SetScript("OnEvent", ...)`, and line + 3411 carries the dev's tested note: *"arg1..arg10 must be captured HERE before + any secondary call wipes them (3.3.5 behavior)."* On 3.3.5a/Vanilla a raw + OnEvent frame exposes the global event args. This is a different dispatch path + than the Ace `RegisterEvent` handlers in 8.1. Worth one in-game confirm, but + not the bug pass 6 claimed. +5. **`TaskQueue.lua` and `MapExplorationUpdate` are NOT dead code** (pass 1) — + already retracted in pass 5. TaskQueue is in both TOCs and called per quest + accept/abandon; MapExplorationUpdate is wired via `MAP_EXPLORATION_UPDATED`. +6. **The 4 "delete-me" files are already `.gitignore`d** (pass 5) — no git impact. +7. **`_QuestLogUpdateQueue` is a valid closure** (pass 4), not a latent crash + (pass 2) — methods defined at `QuestEventHandler.lua:683–693`. + +## 8.3 — Confirmed BUGS (real, verified, with better alternatives) + +| # | Location | Bug | Better alternative | +|---|---|---|---| +| B1 | `QuestieDB.lua:1355` | `IsComplete` calls `GetQuest(questId)` **twice** in one expression | `local q = GetQuest(questId); ... q and q.ObjectiveData` — single call. In a per-quest path, halves the cost. | +| B2 | `QuestieOptionsTracker.lua:486, 522, 797` | `fadeTickerValue:Cancel()` calls `:Cancel()` on a **number** (the ticker object is `fadeTicker`, used correctly at 479/515/786) | `fadeTicker:Cancel()`. Latent error in those branches. | +| B3 | `QuestieAnnounce.lua:19,123` | `alreadySentBandaid` table never pruned (dev TODO: *"rewrite the entire thing its a lost cause"*) — unbounded growth over a session | `wipe(alreadySentBandaid)` on `PLAYER_LOGIN`, or a periodic `C_Timer` clear, or cap by size. | +| B4 | `QuestieDB.lua:1903` | `local playerFaction = UnitFactionGroup("player")` runs at **module load**; if QuestieDB loads before `PLAYER_LOGIN`, `factionReactions.A/.H` are nil → faction-tagged NPCs get `friendly=nil` | Compute lazily on first use, or recompute on `PLAYER_ENTERING_WORLD`. (Token casing is correct here — the risk is load order, distinct from FP #2.) Verify in-game. | +| B5 | `Questie-X.toc:182` | `Modules\QuestieSlash.lua` listed twice (also line 23) | Remove line 182. | +| B6 | 3–4 files | UTF-8 BOM at byte 0: `Database/Corrections/{tbcQuestFixes,wotlkItemFixes,wotlkQuestFixes}.lua` (+ vendored LibSharedMedia) → parse error on strict parsers | Strip the BOM. | +| B7 | `QuestieFramePool.lua:133` / `QuestieFrame.lua` | `newFrame.BaseOnUpdate = _Qframe.BaseOnUpdate` but `_Qframe.BaseOnUpdate` is never defined → `glowLogicTimer` ticker (`QuestieFramePool.lua:109–113`) never fires | Either wire to `_Qframe.GlowUpdate` (if the glow-sync is wanted) **or** delete the dead ticker block. Decide with intent. | + +## 8.4 — Confirmed PERFORMANCE findings (with better alternatives) + +| # | Location | Cost | Better alternative | +|---|---|---|---| +| P1 | `QuestieComms.lua:558` (and V2 ~669) | **O(n²)**: re-serializes the *entire accumulating* `rawQuestList` on every quest added, until length > 200. 30 quests ≈ 465 serialize calls per broadcast | Track running serialized size incrementally: serialize each quest packet once, sum its byte length, and start a new block when the sum crosses the threshold. Converts O(n²)→O(n). | +| P2 | `Localization/l10n.lua:166–207` | `local args = {...}` allocates a table on **every** `l10n()` call (the common case is a literal key with no args), plus a `tostring` loop and `__call` closure dispatch | Add a no-arg fast path *before* allocating (e.g. branch on arg count and return the cached resolved string). A per-key resolved-string cache drops repeat lookups to one hash. Biggest alloc reducer in the addon. | +| P3 | front-removal queues: `QuestieMap.lua:360,370`, `QuestieComms.lua:572,596,692,716`, `QuestieFramePool.lua:221`, `QuestieCombatQueue.lua` (2), `QuestEventHandler.lua:692`, `QuestieValidateGameCache.lua:119`, `TaskQueue.lua:7` | `tremove(t, 1)` is O(n) shift; several run on 0.1–3s tickers | Head-index dequeue (`head`/`tail` integers + nil-out) or a small ring buffer, kept local to each module. No shared abstraction. | +| P4 | `QuestieLib.tunpack` (`QuestieLib.lua:655`) | Recursive unpack: one call+return per element | `return unpack(tbl, 1, tbl.n)` (native C). *Caveat:* `tunpack` shares a file with `tpack` (line 648) whose `{ n = select("#", ...), ... }` is a `...`-expression, so on strict 5.0 `QuestieLib.lua` doesn't load at all — this fix only helps on 5.1+ (where it's still a real comms win). | +| P5 | `Tooltip.lua:411–451` + `QuestiePlayer.lua:170–186` | `GetPartyMemberByName` scans up to 40 unit slots (`UnitName`/`UnitClass` per slot) and is called per (quest × objective × player) inside the tooltip loop → thousands of API calls per busy-NPC tooltip in a full group | Build a `name→info` map once at the top of `GetTooltip` (one `GetPartyMemberList()` pass), then index it in the inner loop. | +| P6 | `tinsert(t, 1, x)` at `Tooltip.lua:437`, `MapIconTooltip.lua:188,889`, `QuestieDB.lua:1710` | O(n) front-insert | Append + reverse once, or build in final order. Tooltip ones are per-tooltip; QuestieDB one is DB-build-time (minor). | +| P7 | `AvailableQuests.CalculateAndDrawAll` (20+ call sites) + `_DrawAvailableQuest` `NewThread`-per-quest (~line 282) | Each call rescans the full quest set and spawns one coroutine per quest (10k on Ascension); Options tab fires it up to 4× per setting change | (a) Move the debounce to the UI layer (1 rescan per burst, not 4); (b) gate event triggers so it doesn't fire on every `ZONE_CHANGED`; (c) batch N quests per yield in one coroutine instead of one coroutine per quest. Highest-value perf work, but needs in-game regression testing. | +| P8 | `QuestieDB.IsDoable` (`QuestieDB.lua:893+`) | 12 `QueryQuestSingle` + several WoW API calls per quest; called per quest in the draw loop | Don't rewrite IsDoable (high risk). Reduce *how often* it runs (P7). If profiling still shows it hot, a batched field-load pass is the eventual fix. | +| P9 | `QuestieLearner.HandleNetworkData` (`QuestieLearner.lua:3610,3658`) | `InjectLearnedData` (300+ lines) runs on **every** network payload | Coalesce incoming payloads into one `InjectLearnedData` per burst window. | + +Confirmed-correct (no action): `GetQuest`/`GetNPC` caches are sound; the kill / +invalidate / guidNpc debounces are all bounded (≤10s windows) — the old +"unbounded killDebounce" Hindsight note is stale; `QuestieStream` and +`QuestieNameplate` pools use `tremove(t)` (O(1) tail), not front-removal; +`QuestieRouteOptimizer.lua` is a removed-stub ("kept for compatibility"). + +## 8.5 — Final prioritized action list + +**Step 0 — settle the platform question (8.1):** in-game `/run print(#({1,2,3}))`. +This gates ~50 files of potential 5.0 work and may make it unnecessary. + +**Tier 1 — safe, real, version-independent (one bisectable commit each group):** +- B5 (TOC dedup), B6 (BOM strip), B2 (`fadeTicker:Cancel` typo ×3), + B1 (`IsComplete` double-call), B3 (bound `alreadySentBandaid`), + P4 (`tunpack` → `unpack`), B7 (FramePool `BaseOnUpdate` decide+fix). +- Add unused `questId` param to `Ascension_IsScalingEnabled` to clear the lint + (cosmetic; not a blocker — see FP #1). + +**Tier 2 — measure first (the prior audit's genuinely good ideas):** +- P1 (comms O(n²)→O(n)), P2 (l10n alloc), P5 (tooltip party map), + P7 (CalculateAndDrawAll gating/debounce), P9 (learner batch). + +**Tier 3 — only if profiling still shows a problem:** +- P3 (queue head-index swaps), P8 (batched IsDoable), spatial index for + `CalcHotzones` (fix its input-mutation bug first), nearest-spawn cache. + +**If Step 0 says strict 5.0:** convert raw `#` → `table.getn` across the 50 +files (mechanical), rewrite the ~15 `...`-expression sites to use `arg`/`unpack` +or `loadstring`, and verify `l10n.lua`, `QuestieLib.lua`, and `QuestieMap.lua` +parse. **If Step 0 says `#` works:** skip all 5.0 work. + +**Do NOT do:** FP #1–7 (above); delete `TaskQueue`/`MapExplorationUpdate`; the +`QuestiePerf`/`QuestieQueue`/scheduler abstraction (regression risk on the 3 +fragile subsystems); the `function(...)`-signature rewrites as framed in pass 7 +(wrong target). + +**Verify in-game (not from static analysis):** B4 (factionReactions load order), +FP #4 (learner `argN` on 3.3.5a), and Step 0. + +## 8.6 — Executable verification (busted) + +The static findings above are now backed by an executable spec — +`Tests/AuditFindings_spec.lua` — so the audit is self-verifying and re-runnable +after fixes. Each test is tagged with its Pass-8 ID (`[B1]`, `[P4]`, `[FP3]`...) +and reads the actual source to assert the claim holds. + +Run: +``` +busted Tests/AuditFindings_spec.lua # just the audit checks +busted # whole suite +``` + +Result at HEAD `581634d`: + +- **`AuditFindings_spec.lua`: 33 / 33 pass** — every Pass-8/9/10/11 finding and + every corrected false positive confirmed against source (19 Pass-8 + 6 Pass-9 + `[N1]`–`[N5]` + 4 Pass-10 `[PP1]`–`[PP4]` + 4 Pass-11 `[11.1]`/`[G1]`/`[G2]`/`[G3]`, + and the corrected `%`-modulo assertion). +- **Full suite: 45 pass / 4 fail.** The 4 failures are *pre-existing* and are + themselves a finding: `Tests/QuestieArrowAssets_spec.lua` expects an arrow set + (`ArcaneArrow.tga` + `Minimal{1,2,3}.tga`, 22 files) that + `Modules/Arrow/QuestieArrowAssets.lua` and the `Icons/Arrows/` folder don't + provide (they still carry `DropTestArrow.tga`, 18 files). This is a real + test↔asset mismatch (originally flagged in pass 5), **not** a busted/env + problem and not introduced here. Decision needed: update the manifest+assets to + match the spec, or update the spec to current assets. Left untouched pending + your call. + +Notes on the spec design: the *false-positive* and *structural-fact* tests +(`8.2`) should pass forever. The *bug*/*perf* tests (`8.3`/`8.4`) are a snapshot — +when a fix lands, invert or remove the matching assertion (e.g. after P4, +`[P4]`'s `is_false(... "return unpack(tbl, 1, tbl.n)")` flips to `is_true`). + +--- + +# Pass-9 Addendum — Genuine File-by-File Audit (2026-06-04) + +Passes 1–8 mixed mechanical scans with targeted reads and skipped files. Pass 9 +is the actual per-file pass. **Method:** + +1. Enumerated **every** runtime `.lua` file (excluding vendored `Libs/`, + `.history/`, `workflow/`, `Tests/`): **308 files** — **135 logic** (have + function defs) + **173 data** (return-table only). ~1.5M lines, but >90% of + that is data tables. +2. Ran a **per-file** mechanical scan over all 308 for every 5.0/bug pattern + (`#`, `%`, literal `...`, `select`, `gmatch`, `goto`, `bit32`, `string.pack`, + `table.unpack`, `utf8`, `//`, `tremove(_,1)`, `tinsert(_,1,)`, double-call, + nil-index, ternary-trap, `bit.`, `strsplit`). +3. **Read** every small/medium logic file in full; targeted-read the large ones + around flagged lines. Verified the 173 data files are genuinely data (the + `#`/`gmatch` "hits" in them are string content / URL comments — see 9.1). + +This pass found **6 issues passes 1–8 missed entirely**, and corrects a Pass-8 +number. + +## 9.1 — Correction to Pass 8: the `#` count was inflated + +Pass 8 said "50 files / 255 sites" of raw `#`. That counted **data files** where +`#` is inside string values (e.g. `lookupNpcs/deDE.lua`: +`"Gnomenkanonenschütze #Shattrath"`) or URL comments (`npcDB.lua`: +`creature_template#rank`). Those are **not** operators. + +**Accurate count (logic files, excluding strings/comments/URLs): 33 files / 134 +sites.** Still the dominant 5.0 parse blocker, still real, but ~half the earlier +figure. The `%`-modulo operator remains **0** (codebase uses `math.mod`). + +Also confirmed clean tree-wide (0 real sites): `goto`/`::labels::`, `bit32.`, +`string.pack`/`string.unpack`, `table.unpack`, `utf8.`, `//`. + +## 9.2 — NEW: `bit` library used unguarded (1.12 RUNTIME failure) + +A category passes 1–8 never checked: WoW APIs/libs **absent on 1.12** (runtime +nil-index, not a parse error). The `bit` library (BitLib) ships with WoW **2.0+**; +**vanilla 1.12 does not have it.** The addon uses it **unguarded** in 5 files, +all in the Turtle TOC: + +| File:line | Code | +|---|---| +| `Questie.lua:1` | `local band = bit.band` | +| `Modules/QuestieStream.lua:10-12` | `local band/lshift/rshift = bit.*` | +| `Database/compiler.lua:18` | `local lshift = bit.lshift` | +| `Database/QuestieDB.lua:218` | `local bitband = bit.band` | +| `Modules/QuestieMenu/Townsfolk.lua:10` | `local ..., bitband = ..., bit.band` | + +On a client without `bit`, every one throws `attempt to index nil value (global +'bit')` at load. **Tell:** the *vendored* `XXH_Lua_Lib.lua:27-32` deliberately +guards this (`local bit = _G.bit; local band = bit and bit.band`) — Questie's own +code does not. If Turtle provides `bit` (its client may), this is moot like the +`#` question; if not, `Questie.lua:1` is one of the first lines to fail. **Verify +in-game:** `/run print(bit and bit.band(3,1))`. + +## 9.3 — NEW: `strsplit` used in runtime but shimmed only in a test + +`strsplit` is a WoW **2.0+** global (absent in vanilla 1.12). It's used in **14 +runtime files** (`QuestieComms`, `QuestieNameplate`, `QuestieLearner`, +`TooltipHandler`, `QuestLinks/Link`, `QuestieDB`, `DailyQuests`, `QuestieDebugOffer`, +`Auto/Privates`, several `Journey/*`, `Database/Corrections/QuestieEvent`). The +**only** `strsplit` shim in the tree is in `Tests/.../QuestieLearner_spec.lua` (a +test mock) — there is **none in `QuestieCompat`**. Same caveat as `bit`: a real +1.12 gap if the Turtle client doesn't provide it. Lower confidence than `bit` +(some call sites are TBC/WotLK-only paths), but it belongs on the verify list. + +## 9.4 — NEW: Options files use `{ ... }` where `{}` was intended (5.0 parse + smell) + +`Modules/Options/QuestieOptions.lua:15` (`QuestieOptions.tabs = { ... }`) and +**14 Options tab files** (`tabs.arrow = { ... }`, `tabs.general = { ... }`, …) +write a literal `{ ... }`. At **file scope**, `...` is WoW's chunk varargs +`(addonName, addonTable)`, so each makes `tabs = {"Questie-X", }` — +two junk array elements. Harmless on 5.1+ (Ace reads named keys, ignores the +array part) but: (a) a **Lua 5.0 parse error in every Options file**, and (b) +clearly a typo for `{}`. One-character fix each (`{ ... }` → `{}`). + +## 9.5 — NEW bug: nil-index in comms tooltip data + +`Modules/Network/QuestieCommsData.lua:49,51`: +```lua +oName = QuestieDB:GetNPC(objective.id).name; -- line 49 +oName = QuestieDB:GetObject(objective.id).name; -- line 51 +``` +Both index the result with **no nil-check**, while the `item` branch immediately +below (line 53) *does* guard (`if dbItem and dbItem.name ...`). An unknown +NPC/object id from a network packet (common cross-version / custom-server) → +`attempt to index a nil value`. Bounded to the comms-tooltip path but a real +crash vector. Fix: guard like the item branch. + +## 9.6 — NEW: parse errors fire regardless of runtime guards + +`Modules/Quest/QuestgiverFrame.lua:153,173` use `oldSetup(self, ...)` inside an +`if GossipAvailableQuestButtonMixin then` block (a Dragonflight-only global, +never true on 1.12). It's tempting to think the guard saves 1.12 — it does not. +**Lua parses the whole file before executing any `if`**, so the `...` expression +is a 5.0 parse error at load even though the block never runs. Same applies to +any `#`/`...`/`{...}` sitting inside a version-gated branch. (Relevant when +triaging the 9.4 / 8.1 lists — you can't "guard" your way out of a parse error.) + +## 9.7 — NEW perf (minor) + consistency + +- **`Modules/Quest/QuestieQuestPrivates.lua:120` `monster()` name fallback** — + when an objective's `IdList` yields no valid NPC, it scans the **entire** + `QuestieDB.npcData` doing `string.lower(name)` per record to match by name. + O(N) over the whole NPC table per such objective. It's a custom-server fallback + (rare), but worth a guard/limit if it ever shows on a profile. +- **`select(8, GetInstanceInfo())` inconsistency** — `QuestiePlayer.lua:123,132` + (Turtle TOC) still use it, while `QuestieLearner.lua:166` was rewritten to an + explicit unpack with the comment *"Lua 5.0 compat: replace select(8, …)"*. If + that rewrite was necessary in the learner, the two QuestiePlayer sites are the + same latent issue, unfixed. + +## 9.8 — Files confirmed clean on this pass (representative) + +Read in full, no new findings: `QuestieServer`, `QuestieCleanup`, `QuestieShutUp` +(pattern is always set before the filter can fire — not a bug), `QuestLinks/Hooks`, +`QuestieMenu/{MeetingStones,Mailboxes,ProfessionTrainers,ClassTrainers}` (data), +`Map/HBDHooks`, `Map/WeaponMasterSkills`, `Options/QuestieOptionsUtils`, +`QuestLogCache` (exemplary 5.0 hygiene — uses `table.getn` throughout), +`QuestieReputation`, `QuestieCoordinates`, `Quest/DailyQuests`, +`Quest/IsleOfQuelDanas`, `Quest/QuestgiverFrame` (modulo 9.6), +`Quest/QuestieQuestPrivates` (modulo 9.7). + +**Honest coverage limit:** the five largest logic files (`QuestieLearner` 3662, +`QuestieTracker` 2688, `QuestieQuest` 2279, `QuestieDB` 2367, `QuestieComms` 1071) +and the large `Database/Corrections/*` (mostly data) were scanned line-by-line +mechanically and targeted-read at every flagged site, not re-read end-to-end this +pass — their hot paths were already covered in passes 1–8. The 173 data files +were verified-as-data, not read line-by-line (no logic to audit). + +## 9.9 — Updated finding ledger (Pass 9 deltas) + +New, actionable: +- **N1 (9.2)** `bit` unguarded, 5 TOC files — 1.12 runtime crash risk. *Verify, then guard or shim.* +- **N2 (9.3)** `strsplit` unshimmed in 14 runtime files — 1.12 runtime risk. *Verify, then shim in QuestieCompat.* +- **N3 (9.4)** Options `{ ... }` → `{}`, 15 files — 5.0 parse error + typo. *One-char fix each.* +- **N4 (9.5)** `QuestieCommsData.lua:49,51` nil-index — crash on unknown id. *Add nil guard (Tier 1).* +- **N5 (9.7)** `select(8,…)` left in `QuestiePlayer.lua:123,132` — finish the 5.0 rewrite. +- **N6 (9.7)** `monster()` full-`npcData` name scan — minor perf, fallback path. + +Corrections to earlier passes: +- `#` operator surface is **33 files / 134 sites**, not 50/255 (9.1). +- Add `bit` and `strsplit` to the 1.12 verify list alongside `#` (9.2/9.3); the + 1.12 question now spans **parse-level** (`#`, `...`, `{ ... }`) **and + runtime-level** (`bit`, `strsplit`) — the runtime ones aren't fixed by the + `/run print(#{1,2,3})` check; they need their own probes. + +**Revised Tier 1 (add to §8.5):** N4 (comms nil-guard) and N3 (Options `{}` +typo) are zero-risk and join the Tier-1 commit. N1/N2/N5 are gated on the same +"is the target really 1.12?" decision as §8.1. + +--- + +# Pass-10 Addendum — Additional Performance Findings (2026-06-04) + +Perf wins **not** in passes 1–9, found by reading the per-frame handlers, the +sort/alloc sites, and the binary-DB read layer. Each is logic-preserving and +respects the three fragile subsystems (tracker, minimap/HBD, Ascension learner) — +where a win lands inside one of those, it is flagged **NOTE-ONLY**. + +### PP1 — `IsDoable` uses 12 single-key reads where a batch read exists (sharpens P8) + +`Database/compiler.lua:1477` `handle.Query(id, keys)` reads **many keys in one +call**, doing the `overrides[id]` check and `pointers[id]` lookup **once** and +then looping the key list. `QuestieDB.IsDoable` instead calls +`QueryQuestSingle(questId, key)` **12 times** (§ pass-4 list), repeating the +override check + pointer hash lookup + `types[key]`/`readers[]` dispatch on every +call. For a full `CalculateAndDrawAll` scan that's 12× the per-quest fixed +overhead it needs. + +**Better alternative (logic-preserving, uses existing infra):** replace the 12 +`QueryQuestSingle` calls with one `QuestieDB.QueryQuest(questId, { ...12 keys... })` +and destructure the positional result. No change to IsDoable's *logic* — only its +data access. This is the concrete, low-risk form of pass-8's vague "batched +field-load" (P8); it does **not** require the risky rewrite P8 warned against. +Bonus: if the key list is given in DB column order, the non-skippable fields seek +forward once instead of re-walking from `lastIndex` per call +(`compiler.lua:1467`). + +### PP2 — Hoist repeated `Questie.db.profile.X` chains in hot functions + +Each `Questie.db.profile.X` is a 3-hop table walk (`Questie`→`db`→`profile`→`X`). +Hot, **non-fragile** call sites repeat it many times per refresh: +`Modules/Tooltips/MapIconTooltip.lua` (13×), `Modules/Map/QuestieMap.lua` (12×), +`Modules/Tooltips/Tooltip.lua` (6×). **Better:** `local profile = +Questie.db.profile` once at function top, then `profile.X`. Saves 2 hops per +access; zero behavior change. +**NOTE-ONLY:** `QuestieTracker.lua` has **83** such accesses (by far the worst) +but the tracker is the just-fixed fragile subsystem (commit `581634d`) — leave it +unless the tracker is being touched for other reasons. + +### PP3 — `table.sort` inline-closure comparators allocate per call + +`table.sort(t, function(a,b) ... end)` allocates a fresh closure every call. In +hot paths: +- `Modules/Network/QuestieComms.lua:458, 530, 647` — per broadcast/aggregate. +- `Modules/Arrow/QuestieArrow.lua:1212` — per `Refresh()` (driver-throttled). + +**Better:** hoist each comparator to a module-level `local function` (the arrow +one is a pure `a.distance < b.distance`, trivially hoistable; the comms ones +capture no per-call state). Removes one closure alloc per sort. +**NOTE-ONLY:** `TrackerUtils.lua:919/934/944` also do this *and* re-test the +constant `sortObj == "byComplete"` inside every comparison (O(n log n) redundant +string compares) — a real double cost, but it's in the fragile tracker. If the +tracker is ever reworked, pick the comparator once before sorting. + +### PP4 — Arrow rebuilds + re-sets the distance string every throttled tick + +`Modules/Arrow/QuestieArrow.lua:862` +`objectiveFrame.distance:SetText("Distance: " .. _FormatDistance(dist))` runs on +every throttled arrow update: a string concat **plus** a `FontString:SetText` +(which forces a relayout) even when the displayed text hasn't changed. **Better:** +cache the last-formatted string on the frame and skip the concat+`SetText` when +equal. Bounded (throttled, only while the arrow is shown) — minor but free. The +rest of this OnUpdate is already well-tuned (throttled; closures were hoisted per +the comment at line 900), so this is the only remaining waste in it. + +### PP5 — Multiple `GetTime()` calls per function + +`GetTime()` is cheap but not free, and calling it 2–7× in one function can read +slightly different values mid-computation. Hoist to a single `local now = +GetTime()`: `MapIconTooltip.lua` (6×), `Network/QuestieLearnerComms.lua` (5×), +`QuestieInit.lua` (4×). Micro-optimization + correctness tidy. +**NOTE-ONLY:** `QuestieTracker.lua` (7×) — fragile, skip. + +### PP6 — `monster()` name-fallback scans the entire NPC DB (cross-ref N6 / 9.7) + +`Modules/Quest/QuestieQuestPrivates.lua:120` — when an objective's `IdList` yields +no valid NPC, it iterates **all** of `QuestieDB.npcData` doing `string.lower()` +per record to match by name. O(N) over the whole NPC table per such objective. +It's a custom-server fallback (rare), but if it ever shows on a profile, guard it +(e.g. only attempt for Ascension/custom data, or cache a lowercase name→id map). + +## Pass-10 ledger + +| ID | Location | Win | Risk | +|----|----------|-----|------| +| PP1 | `QuestieDB.IsDoable` (12× `QueryQuestSingle`) | one batch `QueryQuest` call; shares override/pointer lookup | low (logic unchanged) | +| PP2 | MapIconTooltip(13), QuestieMap(12), Tooltip(6) | local `profile` cache | zero | +| PP3 | QuestieComms(458/530/647), QuestieArrow(1212) | hoist sort comparators | low | +| PP4 | QuestieArrow.lua:862 | cache distance string, skip redundant SetText | zero | +| PP5 | MapIconTooltip(6), LearnerComms(5), QuestieInit(4) | hoist `GetTime()` | zero | +| PP6 | QuestieQuestPrivates.lua:120 | bound the name-fallback scan | low | +| — | **NOTE-ONLY (fragile):** QuestieTracker db.profile ×83, GetTime ×7; TrackerUtils sort comparators | real wins, but tracker is just-fixed — defer | — | + +These are all additive to §8.4 (P1–P9). None overlaps an existing P-item except +PP1, which is the concrete, lower-risk mechanism for P8. + +--- + +# Pass-11 Addendum — Gap-Fill Read + Major Correction (2026-06-04) + +This pass read, in full, files that were only mechanically scanned before: +`QuestieStream.lua`, `QuestiePlayer.lua`, `Map/QuestieMapUtils.lua`, +`QuestieValidateGameCache.lua`, `QuestieCompat.lua` (the whole 1.12 compat layer), +`QuestieNameplate.lua`, plus the Pass-9 batch (`QuestieServer`, `QuestieCleanup`, +`QuestLogCache`, `QuestieReputation`, `QuestieCoordinates`, `QuestieCommsData`, +`QuestieQuestPrivates`, `DailyQuests`, `QuestgiverFrame`, …). It produced one +**material correction** and several new findings. + +## 11.1 — CORRECTION: `%` modulo *is* used (Pass-8/9/10 said "0") + +Passes 8–10 stated `%` modulo was "cleanly avoided (uses `math.mod`)". **That is +wrong.** Raw `%` modulo operator — a Lua **5.0 parse error**, exactly like `#` — +is used in **~50 sites across 11 files**: + +| File | `%` sites | In Turtle TOC | +|---|---|---| +| `Modules/QuestieStream.lua` | 23 (`% 256`, `(p-1) % 100000`, `e % 86` …) | yes | +| `Modules/QuestieLearner.lua` | 8 | yes | +| `Modules/QuestiePlayer.lua` | 4 (`requiredRaces % playerRaceFlagX2` :103/:109) | yes | +| `Database/QuestieDB.lua` | 4 | yes | +| `Database/Corrections/QuestieEvent.lua` | 3 | yes | +| `Modules/Tooltips/MapIconTooltip.lua` | 2 | yes | +| `Database/QuestXP/QuestieXP.lua` | 2 | yes | +| `QuestieProfiler`, `QuestieQuest`, `classicQuestFixes`, `Compat/HBD` | 1 each | mixed | + +The developer shimmed `math.mod` (in `QuestieLoader` + `QuestieCompat:15`) **but +then used the raw `%` operator anyway** in these files. A `math.mod` shim cannot +rescue a raw `%` — it's a parse error before any shim runs. So the **5.0 parse +surface is bigger than reported**: + +| Construct | 5.0 status | Files / sites | +|---|---|---| +| `#` length op | parse error | 33 files / 134 | +| `%` modulo op | parse error | **11 files / ~50 (NEW)** | +| `...` expression | parse error | ~15 sites | +| Options `{ ... }` (should be `{}`) | parse error | 15 files | + +(The earlier `[9.1]` spec test only checked that `QuestieMap.lua` has no `%` — true, but the **general claim** it documented was wrong; corrected in the spec.) + +## 11.2 — New findings + +- **G1 — `QuestieNameplate:UpdateNameplate` early-returns out of its loop** + (`QuestieNameplate.lua:88`): inside `for guid, token in pairs(activeGUIDs)`, a + single unit with no `unitName`/`npcId` does `return`, aborting updates for **all + remaining** nameplates that cycle (same bug class as the QuestieQuest + early-return-in-loop). Should skip that entry, not return. Also: it + `strsplit("-", guid)` **every update** to re-derive `npcId` that never changes + for a given GUID — cache `guid→npcId` once at create time (perf + avoids the + `strsplit` 1.12 dependency in the hot path). +- **G2 — `QuestieValidateGameCache` dead-variable + per-iteration closure** + (`QuestieValidateGameCache.lua:64,70`): `local isQuestLogGood = true` is never + set false, so the `if not isQuestLogGood` guard (line 106) is unreachable — + validation effectively just skips N updates then always marks the cache good + (probably intentional graceful-degradation, but the dead var is a smell worth a + comment). And `pcall(function() … end)` is allocated **per quest, per + QUEST_LOG_UPDATE** during login — hoist the function out of the loop. +- **G3 — `QuestieCompat` is sound and confirms the gaps**: it does **not** shim + `bit` or `strsplit` (so N1/N2 stand), and it cannot shim the `%`/`#` operators. + The C_Timer / xpcall(25-arg) / hooksecurefunc / Ambiguate polyfills are correct + and namespaced. The Sunstrider `CALIBRATED_MAP_GROUPS` block is the fragile + minimap area — leave it. +- **Confirmations (already tracked):** `CalcHotzones` is O(n²) and mutates its + input via `point.touched` (Tier-3 — fix the mutation before any spatial-index + rewrite); `MapExplorationUpdate` is wired (FP, not dead); `GetPartyMemberByName` + is the 40-slot scan behind P5 and `GetPartyMemberList` already exists as the + one-pass replacement; `select(8, GetInstanceInfo())` at `QuestiePlayer.lua:123/132` + is the unfinished N5 rewrite; per-loop `Questie.db.profile` reads in + `RescaleIcon` and `QuestieNameplate:RedrawIcons` are PP2 candidates. + +## 11.3 — Coverage statement (honest) + +Read end-to-end across passes 9–11: all logic files ≤ ~600 lines plus the named +hot files. **Not** read line-by-line (mechanically scanned + targeted-read only): +the 5 giants (`QuestieLearner`, `QuestieTracker`, `QuestieQuest`, `QuestieDB`, +`QuestieComms`), the large `Options/*` and `Journey/*` UI (user-triggered, not +per-frame), the `Tracker/*` UI sub-files (fragile, just-fixed), and `Compat/*` +data tables (`UiMapData`, `QuestReward`, `QuestTag` — pure data). The 173 data +files were verified-as-data. Every file was covered by the per-file mechanical +scan for all 5.0/bug patterns. + +--- + +# Pass-12 Addendum — Re-review Gap Fill (2026-06-04) + +This pass re-reviewed the report against the current repo with a narrower rule: +only add findings that have direct source evidence and either were absent from the +report or were present in the body but dropped from the consolidated implementation +plan. + +## 12.1 — Plan omissions from already-proven findings + +These were already proven earlier in the report, but the final phase map at the +bottom lost them: + +- **B4 was dropped from Phase 1:** `Database/QuestieDB.lua:1903` captures + `UnitFactionGroup("player")` at module load. The body correctly says this + needs in-game verification because `factionReactions.A/.H` can become nil if + the file loads before the player faction is available. Keep it as a + verification-gated Phase 1 item. +- **P3 was mislabeled in Phase 4:** P3 is the front-removal queue work + (`tremove(t, 1)` / `table.remove(t, 1)`) from §8.4, not a tracker-only item. + It belongs in measured Phase 3, with tracker-adjacent sites still treated + cautiously. +- **P6 was omitted entirely:** the `tinsert(t, 1, x)` front-insert work at + `Tooltip.lua:437`, `MapIconTooltip.lua:188,889`, and `QuestieDB.lua:1710` is + in §8.4 but missing from the final plan. +- **PP6 / N6 were omitted from the phase map:** the name-fallback full-NPC scan + in `QuestieQuestPrivates.lua` is minor, but should still be tracked as measured + Phase 3 or Phase 4 depending on whether Ascension learner code is being touched. + +## 12.2 — NEW: expensive debug diagnostics run even when debug output is disabled + +`Questie:Debug` short-circuits internally (`Questie.lua:160-166`), but Lua +evaluates function arguments before entering `Questie:Debug`. Therefore expensive +arguments still run even when debug output is disabled. + +Evidence: + +- `Modules/QuestieInit.lua:112-124` defines `_dbStats(t)`, which scans a whole DB + table to count entries and min/max IDs. +- `Modules/QuestieInit.lua:131,132,138,292` call `_dbStats(...)` inside string + concatenations passed to `Questie:Debug`. These scans happen before + `Questie:Debug` can return. +- `Modules/QuestieLearner.lua:1972-1979` scans all `QuestieDB.npcDataOverrides` + to compute `spawnOverrideCount`, then only uses that value in a debug call. + +**Better alternative:** add a cheap predicate helper, e.g. `Questie:IsDebugEnabled(level)`, +or local guards at the call sites before doing expensive diagnostics. This is +logic-preserving and should be a low-risk Phase 1/3a item. + +## 12.3 — NEW correction: raw `#` count misses Turtle-loaded QuestieMenu files + +The final §9.1 count says raw `#` is **33 files / 134 sites**, but at least two +Turtle-loaded files were not accounted for in the final compatibility ledger: + +| File | Raw `#` sites | Turtle TOC evidence | +|---|---:|---| +| `Modules/QuestieMenu/Townsfolk.lua` | 5 (`:35`, `:154`, `:245`, `:335`, `:432`) | `Questie-X-Turtle.toc:199` | +| `Modules/QuestieMenu/QuestieMenu.lua` | 1 (`:117`) | `Questie-X-Turtle.toc:204` | + +`Townsfolk.lua` is not data-only: `QuestieInit.lua:142` calls +`Townsfolk.Initialize()` during database boot, and the file also does large DB +scans (`QuestieDB.npcData`, `npcDataOverrides`, `itemData`). If Phase 0 proves a +strict Lua 5.0 target, Phase 2 must be driven from the actual TOC file list rather +than the stale 33/134 count. + +## 12.4 — NEW: `QuestieLearner:GetNPCIdByName` is another full-NPC name scan + +Separate from PP6/N6's `monster()` fallback, `QuestieLearner:GetNPCIdByName` +(`Modules/QuestieLearner.lua:2473-2494`) scans: + +1. all `QuestieDB.npcDataOverrides`, lowercasing `data[1]`; +2. then all base `QuestieDB.npcData`, lowercasing `data[1]`. + +Call sites: + +- `Modules/QuestieLearner.lua:2609` during `OnQuestAccepted`; +- `Modules/QuestieLearner.lua:3519` during `ScanExistingQuestLog`, which is + scheduled one second after learner initialization. + +This is not a per-frame hot path, but it can multiply by objective count and +quest-log size. If Ascension learner work is already being touched, build a +lowercase NPC-name index once and update it when learned overrides are injected. +Preserve override precedence over base DB entries. + +## 12.5 — Pass-12 ledger + +| ID | Location | Action | Risk | +|---|---|---|---| +| H1 | final phase map | restore B4, P3, P6, PP6/N6 | zero | +| H2 | `QuestieInit._dbStats`, learner `spawnOverrideCount` | guard expensive debug-only computations | low | +| H3 | `Townsfolk.lua`, `QuestieMenu.lua` | add to Phase 2 raw-`#` remediation; use TOC-driven grep | low if mechanical | +| H4 | `QuestieLearner:GetNPCIdByName` | add measured learner name-index/cache item | medium; learner is fragile | + +--- + +# Pass-13 Implementation Update - QuestieLearner kill/loot stutter mitigation (2026-06-04) + +This pass was driven by live reports from Elwynn Forest / Redridge players: + +- small-map lag with Questie enabled; +- boar kill + loot stutters with Questie enabled; +- no stutter after disabling the WotLK DB addon while keeping Ascension DB and + Questie enabled. + +The strongest code-backed hypothesis was Xurkon's: QuestieLearner was doing too +much work on every kill/loot burst. The previous implementation updated learner +saved data, injected/merged live `QuestieDB.npcDataOverrides`, cleared +`QuestieDB.private.npcCache[npcId]`, invalidated active quest spawn lists, and +broadcast learner data from the same hot path. + +## 13.1 - Implemented mitigation + +Changed `Modules/QuestieLearner.lua`: + +- Added a 0.5s coalescing window for NPC live DB updates. `LearnNPC` still + records saved learner evidence immediately, but live override injection, + `npcCache` clearing, and active objective spawn-list invalidation now flush + once per NPC per burst instead of once per kill/log/loot-triggered call. +- Added deep-copying when learner NPC data is copied into live overrides. This + prevents `QuestieDB.npcDataOverrides[npcId]` from aliasing the saved learner + table and mutating "before flush" during later kills. +- Routed `_MergeSpawnEvidence` cache clearing through the same coalesced NPC + live-update path. +- Added a 2s coalescing window for learner outgoing broadcasts. Repeated + `LearnNPC`, `LearnItem`, `LearnObject`, or `LearnQuest` updates for the same + entity keep only the latest pending payload before calling + `QuestieLearnerComms:BroadcastLearnedData`. If the burst began as a first + discovery, the queued message preserves the original `NEW` op while still + sending the latest payload. This specifically covers the loot handler path + where quest-item loot calls `LearnItem(...)`. + +## 13.2 - Evidence and verification + +New regression coverage: + +- `Tests/QuestieLearner_performance_spec.lua` proves repeated `LearnNPC` calls do + not immediately populate `QuestieDB.npcDataOverrides` or clear + `QuestieDB.private.npcCache`; the live update happens only when the queued + timer flushes. +- The same spec proves repeated learner broadcasts coalesce into one latest + payload while preserving the first-discovery `NEW` op. + +Focused verification run: + +```text +busted Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +6 successes / 0 failures / 0 errors / 0 pending +``` + +## 13.3 - What this does and does not prove + +This fix directly addresses the kill/loot learner churn that could explain +short stutters while killing and looting boars, especially with the large WotLK +DB enabled. It does **not** prove the WotLK DB addon is the sole cause; it proves +the learner was synchronously touching live DB/cache/map/comms surfaces in the +reported hot path. + +If field reports still reproduce stutters after this patch, continue in this +order: + +1. Batch item/object live override updates the same way NPC live updates are now + batched. +2. Add the Pass-12 H2 debug guards for `_dbStats(...)` and learner + `spawnOverrideCount`. +3. Add the Pass-12 H4 lowercase NPC-name index for + `QuestieLearner:GetNPCIdByName`. +4. Coalesce inbound learner network `InjectLearnedData` calls from P9 so multiple + incoming payloads produce one merge pass per burst. + +## 13.4 - Updated implementation-plan status + +| ID | Status | Notes | +|---|---|---| +| L1 | implemented | QuestieLearner NPC live override/cache/map invalidation now batches per NPC per 0.5s burst. | +| L2 | implemented | QuestieLearner outgoing broadcasts now batch per entity per 2s burst. | +| L3 | implemented | Regression spec covers the kill-path cache/override delay and broadcast coalescing. | +| H2 | still open | Debug-only expensive computations remain a follow-up. | +| H4 | still open | `GetNPCIdByName` full-NPC scan remains a measured follow-up. | +| P9 | still open | Inbound learner network merge coalescing remains a follow-up. | + +--- + +# Pass-14 Branch Update - learner/comms branch and Turtle support removal (2026-06-04) + +Created branch: + +```text +codex/questie-learner-comms-improvements +``` + +Repo-local Git identity was set before commit/push work: + +```text +Xurkon +``` + +## 14.1 - Scope added to the branch + +This branch now contains: + +- Pass-13 QuestieLearner kill/loot stutter mitigation; +- outgoing learner comm burst coalescing while preserving first-discovery `NEW` + semantics; +- removal of active Turtle-specific support paths now that TurtleWoW support is + gone. + +## 14.2 - Turtle cleanup performed + +Removed active Turtle support references from: + +- `Modules/QuestieServer.lua`: removed `Questie.IsTurtle` detection, TurtleDB + expected-plugin flavor, TurtleDB plugin scan entry, and Turtle debug output. +- `Modules/QuestieInit.lua`: removed `Questie.IsTurtle` from custom-server + compilation/defer checks. +- `Modules/QuestieLearnerExport.lua` and + `Modules/Options/DatabaseTab/QuestieOptionsDatabase.lua`: removed Turtle + learner export/database bucket labels. +- `Modules/Options/CreditsTab/QuestieOptionsCredits.lua`: removed TurtleWoW from + current credits copy. +- `Modules/Libs/QuestiePluginAPI.lua`, `Modules/QuestieCompat.lua`, and + `Modules/Libs/QuestieLoader.lua`: replaced Turtle-specific comments with + generic legacy/custom-client wording so universal Lua compatibility remains. +- `CHANGELOG.md` and `docs/changelog.html`: scrubbed historical support wording + that named TurtleWoW directly. + +The ignored local file `Questie-X-Turtle.toc` was deleted from disk. It was +already excluded by `.gitignore`, so there is no tracked delete for it. + +## 14.3 - Remaining intentional references + +The only remaining `Turtle` grep hit in tracked source is an actual game item: + +```text +Modules/QuestieDebugOffer.lua: Pattern: Turtle Scale Gloves +``` + +Do not remove game-content names just because they contain the word "Turtle". + +--- + +# Pass-15 Implementation Update - inbound learner merge batching and low-risk guards (2026-06-04) + +This pass landed the remaining learner/comms follow-ups that were the next best +steps from the audit. + +## 15.1 - Implemented fixes + +Changed `Modules/QuestieLearner.lua`: + +- **P9 inbound merge batching:** `HandleNetworkData` now queues incoming + payloads by `typ:id` and flushes them once per burst window instead of calling + `InjectLearnedData()` on every packet. +- **H4 learner name index:** `GetNPCIdByName` now uses a cached lowercase name + index with override precedence instead of scanning all NPC overrides/base data + on every lookup. +- **NPC-name index invalidation:** the learner marks the name index dirty when + learned NPC names are added or changed, so cached lookups stay correct after + new data arrives. + +Changed `Modules/QuestieInit.lua`: + +- **H2 debug guards:** expensive `_dbStats(...)` scans are now wrapped behind a + debug-enabled check, so the tables are no longer walked when debug output is + off. + +Changed `Modules/QuestieLearner.lua`: + +- **H2 debug guard:** the `spawnOverrideCount` scan in `InjectLearnedData()` + now runs only when debug output is enabled. + +## 15.2 - Evidence and verification + +New regression coverage: + +- `Tests/QuestieLearner_performance_spec.lua` now proves repeated inbound + network merges coalesce into one inject pass, in addition to the existing + kill-path and broadcast batching coverage. + +Focused verification run: + +```text +busted Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +7 successes / 0 failures / 0 errors / 0 pending +``` + +## 15.3 - Updated status + +| ID | Status | Notes | +|---|---|---| +| L1 | implemented | QuestieLearner NPC live override/cache/map invalidation batches per NPC per 0.5s burst. | +| L2 | implemented | QuestieLearner outgoing broadcasts batch per entity per 2s burst. | +| L3 | implemented | Regression spec covers kill-path delay, broadcast coalescing, and inbound merge batching. | +| P9 | implemented | Inbound learner network merge batching is now coalesced per burst window. | +| H2 | implemented | Debug-only expensive computations are now guarded. | +| H4 | implemented | `GetNPCIdByName` now uses a cached lowercase index with override precedence. | +| P1 | still open | Broader comms O(n^2) work in `QuestieComms` remains the next larger comms optimization. | +| FP4 | still open | Learner `argN` correctness bug remains a separate correctness follow-up. | + +## 15.4 - Next steps after this pass + +If we keep going on this branch, the next best moves are: + +1. Review `Modules/Network/QuestieComms.lua` for the larger O(n^2) queue work + the audit already identified. +2. Fix the learner `OnEvent` correctness bug on its own pass, because it is a + behavior bug rather than a perf tweak. +3. Only then consider any further learner micro-optimizations, because the hot + path now has the highest-value batching fixes in place. + +--- + +# Pass-16 Implementation Update - QuestieLearnerComms queue head/tail draining (2026-06-04) + +This pass took the smaller but still real comms hot-path win: the queue plumbing +inside `Modules/Network/QuestieLearnerComms.lua`. + +## 16.1 - Implemented fix + +Changed `Modules/Network/QuestieLearnerComms.lua`: + +- Replaced the outgoing `rateLimitQueue` and incoming `incomingMessageQueue` + front-removal behavior with head/tail indexing. +- `ProcessQueues()` now drains from the head in O(1) per pop instead of + `table.remove(..., 1)` shifting the remaining array on every tick. +- Queue state resets back to the initial indices when each queue becomes empty, + so the optimization does not change external behavior or accumulate stale + indices. + +## 16.2 - Evidence and verification + +Regression coverage: + +- `Tests/QuestieLearner_performance_spec.lua` now proves the learner comms + queue drains outgoing broadcasts and incoming messages in FIFO order without + front-removal. + +Focused verification run: + +```text +busted Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +8 successes / 0 failures / 0 errors / 0 pending +``` + +## 16.3 - Updated status + +| ID | Status | Notes | +|---|---|---| +| L4 | implemented | `QuestieLearnerComms.ProcessQueues` now uses head/tail queue draining instead of front-removal. | +| P1 | still open | `QuestieComms` O(n^2) quest-list building remains the larger comms optimization. | +| FP4 | still open | Learner `argN` correctness bug remains a separate correctness follow-up. | + +## 16.4 - Next steps after this pass + +If we keep going on this branch, the next best moves are: + +1. Review `Modules/Network/QuestieComms.lua` for the larger O(n^2) queue work + the audit already identified. +2. Fix the learner `OnEvent` correctness bug on its own pass, because it is a + behavior bug rather than a perf tweak. +3. Only then consider any further learner micro-optimizations, because the hot + path now has the highest-value batching fixes in place. + +--- + +# Pass-17 Implementation Update - QuestieComms incremental packet sizing (2026-06-04) + +This pass took the larger remaining comms performance win from the audit: the +quest-list broadcast paths in `Modules/Network/QuestieComms.lua`. + +## 17.1 - Implemented fix + +Changed `Modules/Network/QuestieComms.lua`: + +- Added incremental packet-size estimation helpers so the broadcast builders no + longer re-serialize the entire growing `rawQuestList` on every quest. +- `BroadcastQuestLog()` now estimates each quest packet once, tracks the running + block size, and splits before appending when the estimated size would cross + the threshold. +- `BroadcastQuestLogV2()` uses the same incremental strategy with a V2 packet + size estimator instead of calling `QuestieSerializer:Serialize(rawQuestList)` + inside the loop. +- The existing block-sending behavior is preserved, including the staggered + ticker and follow-on broadcast queue. + +Also cleaned up a stale audit assertion tied to the removed Turtle TOC file so +the snapshot now matches the repo state after Turtle support removal. + +## 17.2 - Evidence and verification + +Regression coverage: + +- `Tests/AuditFindings_spec.lua` now asserts that the old `rawQuestList` + re-serialization string is absent. +- The stale Turtle TOC assertion was replaced with a direct file-existence + check so the snapshot no longer depends on a deleted file. + +Focused verification run: + +```text +busted Tests\AuditFindings_spec.lua Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +41 successes / 0 failures / 0 errors / 0 pending +``` + +## 17.3 - Updated status + +| ID | Status | Notes | +|---|---|---| +| P1 | implemented | `QuestieComms` now uses incremental quest-packet size estimates instead of re-serializing the growing `rawQuestList` on every loop iteration. | +| FP5 | updated | Removed the stale Turtle TOC expectation from the audit snapshot after Turtle support cleanup. | +| FP4 | still open | Learner `argN` correctness bug remains a separate correctness follow-up. | + +## 17.4 - Next steps after this pass + +If we keep going on this branch, the next best moves are: + +1. Review the remaining `tremove(blocks, 1)` queue pops in `Modules/Network/QuestieComms.lua` and decide whether they deserve the same head/tail treatment we already gave learner comms. +2. Fix the learner `OnEvent` correctness bug on its own pass, because it is a + behavior bug rather than a perf tweak. +3. Only then consider any further learner micro-optimizations, because the hot + path now has the highest-value batching fixes in place. + +--- + +# Pass-18 Implementation Update - QuestieComms queue head/tail draining (2026-06-04) + +This pass removed the remaining front-removal queue work from +`Modules/Network/QuestieComms.lua`. + +## 18.1 - Implemented fix + +Changed `Modules/Network/QuestieComms.lua`: + +- Replaced the remaining `tremove(blocks, 1)` drain in both quest-list broadcast + paths with local head/tail queue state. +- Replaced the `_nextBroadcastData` and `_nextBroadcastDataV2` front-removal + follow-up queues with the same head/tail queue helper, so the module no longer + shifts arrays on every broadcast tick. +- Kept the implementation Lua 5.0-friendly: plain local helper functions and + numeric table indices only, with no newer syntax or language features. + +## 18.2 - Evidence and verification + +Regression coverage: + +- `Tests/AuditFindings_spec.lua` now asserts that `QuestieComms.lua` no longer + contains the old `tremove(blocks, 1)` / broadcast queue front-removal sites. + +Focused verification run: + +```text +busted Tests\AuditFindings_spec.lua Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +41 successes / 0 failures / 0 errors / 0 pending +``` + +## 18.3 - Updated status + +| ID | Status | Notes | +|---|---|---| +| P3 | partially implemented | `QuestieComms` broadcast queue front-removals are fixed; the broader P3 finding still exists in other modules. | +| P1 | implemented | `QuestieComms` now uses incremental quest-packet size estimates instead of re-serializing the growing `rawQuestList` on every loop iteration. | +| FP4 | still open | Learner `argN` correctness bug remains a separate correctness follow-up. | + +## 18.4 - Next steps after this pass + +If we keep going on this branch, the next best move is the learner `OnEvent` +correctness bug on its own pass, because it is a behavior bug rather than a +performance tweak. + +--- + +# Pass-19 Implementation Update - QuestieLearner OnEvent payload capture (2026-06-04) + +This pass closed the learner correctness bug called out in the audit: the event +handler was referencing `arg1` through `arg10` without actually capturing those +arguments. + +## 19.1 - Implemented fix + +Changed `Modules/QuestieLearner.lua`: + +- Updated the `frame:SetScript("OnEvent", ...)` handler to accept the event + payload explicitly as `arg1` through `arg10`. +- Kept the fix Lua 5.0-compatible by using explicit positional parameters + rather than newer syntax. +- The event-specific learner hooks now receive the actual event payload again: + quest turn-in, quest accepted, combat log, item info received, and quest + removal tracking all see the arguments they expect. + +## 19.2 - Evidence and verification + +Regression coverage: + +- `Tests/QuestieLearner_performance_spec.lua` now proves the OnEvent closure + forwards payload arguments to the learner handlers. + +Focused verification run: + +```text +busted Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +9 successes / 0 failures / 0 errors / 0 pending +``` + +## 19.3 - Updated status + +| ID | Status | Notes | +|---|---|---| +| FP4 | implemented | Learner `OnEvent` now captures `arg1` through `arg10` explicitly and forwards them to the event-specific handlers. | +| P3 | partially implemented | `QuestieComms` broadcast queue front-removals are fixed; the broader P3 finding still exists in other modules. | +| P1 | implemented | `QuestieComms` now uses incremental quest-packet size estimates instead of re-serializing the growing `rawQuestList` on every loop iteration. | + +## 19.4 - Next steps after this pass + +The learner correctness bug is closed. The next best move, if we continue on +this branch, is to decide whether the remaining P3 queue sites outside +`QuestieComms` are worth the same head/tail treatment or whether the branch is +ready to shift back toward the remaining learner/perf priorities from the audit. + +--- + +# Pass-20 Static Cleanup Update - learner private assignment removal and packet-sizing regression (2026-06-04) + +This pass stayed in the static / unit-test lane because in-game validation is +not available right now. + +## 20.1 - Implemented fix + +Changed `Modules/QuestieLearner.lua`: + +- Removed the redundant `QuestieLearner.private = _Learner` assignment right + after `local _Learner = QuestieLearner.private or {}`. +- The module still uses the existing private table if one was provided by the + loader, but we no longer rewrite the field back to itself. + +Extended `Tests/QuestieLearner_performance_spec.lua` locally: + +- Added a regression for `QuestieComms` broadcast packet sizing that stubs the + serializer import and asserts each quest is serialized once while packing + broadcast blocks. +- The spec stays Lua 5.0-friendly and exercises both V1 and V2 broadcast + builders without requiring a live client. + +## 20.2 - Evidence and verification + +Focused verification run: + +```text +busted Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +10 successes / 0 failures / 0 errors / 0 pending +``` + +## 20.3 - Updated status + +| ID | Status | Notes | +|---|---|---| +| L0 | cleaned up | Redundant `QuestieLearner.private = _Learner` assignment removed. | +| P1 | implemented | `QuestieComms` packet sizing remains covered by a helper-level regression that confirms one serialization per quest while building blocks. | +| P3 | partially implemented | `QuestieComms` broadcast queue front-removals are fixed; the broader P3 finding still exists in other modules. | + +## 20.4 - Next steps after this pass + +With learner/comms in this state and no in-game testing available, the safest +next move is to leave the remaining non-learner queue sites alone until we can +verify them live, or until a future static pass finds another clearly isolated, +low-risk cleanup. + +--- + +# Pass-21 Implementation Update - user-tunable QuestieLearner performance settings (2026-06-04) + +This pass added real Advanced-tab controls for the learner/comms work instead +of leaving the new batching delays hardcoded. + +## 21.1 - Implemented fix + +Changed `Modules/QuestieLearner.lua`: + +- Added learner settings defaults/backfills for: + - `performanceMode` + - `pinRefreshDelay` + - `pinRefreshMode` + - `liveNpcUpdateDelay` + - `learnerCommsIntensity` +- `pinRefreshDelay` now controls the coalesced active-quest pin refresh timer. +- `pinRefreshMode` can now avoid live pin redraws entirely with `manual`. +- Frame unloads caused by learner spawn invalidation now wait for the same + batched pin refresh unless `pinRefreshMode` is `immediate`. +- `liveNpcUpdateDelay` now controls both NPC live DB/cache update batching and + inbound network merge batching. +- Learner outgoing broadcasts now respect both the existing + `Questie.db.profile.learnerBroadcast` toggle and the new comms intensity. + +Changed `Modules/Network/QuestieLearnerComms.lua`: + +- Added comms tuning levels for `off`, `low`, `normal`, and `fast`. +- The setting now controls outgoing send interval, token capacity, and incoming + messages processed per tick/combat tick. +- `off` skips outgoing queueing and incoming learner comm processing. + +Changed `Modules/Options/AdvancedTab/QuestieOptionsAdvanced.lua`: + +- Added a new `QuestieLearner Performance` section under Advanced. +- Added `Performance Mode` presets: Realtime, Balanced, Low Impact, Manual. +- Added controls for pin refresh behavior, pin refresh delay, live NPC update + delay, minimum kills before learned pins, and learner comms intensity. + +Changed `Modules/Options/QuestieOptionsDefaults.lua`: + +- Added a real default for `learnerBroadcast = true`, matching the existing + Database-tab broadcast toggle. + +Changed local gitignored verification file +`Tests/QuestieLearner_performance_spec.lua`: + +- Added direct public-flow coverage for the cross-link engine: + - quest learned first, then starter/finisher NPCs and objects learned later + back-link into `npc[10]`, `npc[11]`, `object[2]`, and `object[3]`; + - those same links also appear in the live `QuestieDB.*DataOverrides`; + - active quest pin refreshes remain coalesced to one `QuestieQuest:UpdateQuest` + call while multiple cross-links are created; + - item objective drop chains (`quest[10][3]` + `item[2]`) add the drop NPC to + creature objectives (`quest[10][1]`) without duplicating entries when the + same drop is learned again. +- This test file is intentionally local/dev-only because `Tests/` is currently + gitignored in this branch. Do not assume these specs are committed unless a + future branch deliberately force-adds the test harness. + +## 21.2 - Evidence and verification + +Focused verification run: + +```text +busted Tests\QuestieLearner_performance_spec.lua +9 successes / 0 failures / 0 errors / 0 pending +``` + +Broader focused verification run: + +```text +busted Tests\AuditFindings_spec.lua Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +46 successes / 0 failures / 0 errors / 0 pending +``` + +Static verification: + +```text +selene Modules\QuestieLearner.lua Modules\Network\QuestieLearnerComms.lua Modules\Options\AdvancedTab\QuestieOptionsAdvanced.lua Modules\Options\QuestieOptionsDefaults.lua +0 errors / 103 warnings / 0 parse errors +``` + +Static notes and remaining caveats: + +- The only remaining `QuestieQuest:UpdateQuest(questId)` hit in + `Modules/QuestieLearner.lua` is inside the coalesced pin-refresh flush. +- Selene warnings are pre-existing style findings in the touched large files + (`multiple_statements`, empty debug-only blocks, existing shadowing, and + parenthesized conditions). Selene reports no parser errors and no hard errors + for this pass. +- Cross-link tests prove the local learner cross-link engine still works for the + main public flows without needing private helper access. They do not replace + in-game validation of map redraw cost, because real frame unload/update cost + depends on the client UI, map state, and active questlog. +- All new runtime code stayed Lua 5.0-safe: no `#`, `%`, `...`, `goto`, `select`, + `table.remove(queue, 1)`, `table.unpack`, or newer syntax was introduced. + +## 21.3 - Updated status + +| ID | Status | Notes | +|---|---|---| +| L5 | implemented | Learner pin refresh and live DB update delays are now user-tunable. | +| L6 | implemented | Learner comms intensity now gates incoming/outgoing work. | +| L7 | implemented | Advanced tab now exposes performance presets and sliders for low-end systems. | +| L8 | verified-local | Cross-link engine public flows now have local busted coverage for quest giver/object/item-drop chains and duplicate prevention. | + +## 21.4 - Recommended low-end settings + +For users reporting FPS drops in heavy zones: + +- Performance Mode: `Low Impact` +- Pin Refresh Behavior: `Batched` +- Pin Refresh Delay: `2.0s` +- Live NPC Update Delay: `2.0s` +- Minimum Kills Before Learned Pins: `3` +- Learner Comms Intensity: `Low` or `Off` + +--- + +# Pass-22 Implementation Update - stop bystander kills from refreshing learner pins (2026-06-04) + +This pass was triggered by live feedback that QuestieLearner felt worse after +the performance settings pass: pins were still refreshing on every kill, and +even kills by other nearby players appeared to refresh the local player's pins. + +## 22.1 - Verified root cause + +The prior batching work was functioning mechanically: the only remaining direct +`QuestieQuest:UpdateQuest(questId)` call in `Modules/QuestieLearner.lua` is still +inside `_FlushActiveQuestPins()`. However, the event source feeding that batch +was too broad. + +`QuestieLearner:OnCombatLogEvent(...)` accepted both: + +- `PARTY_KILL` +- `UNIT_DIED` + +and then unconditionally called: + +- `LearnNPC(...)` +- `_StoreGuidSpawnEvidence(...)` +- `_MergeSpawnEvidence(...)` + +`UNIT_DIED` fires for visible nearby mob deaths, including mobs killed by other +players. That meant bystander deaths could still promote learner spawn evidence, +flush live NPC overrides, invalidate active quest spawn lists, and queue a +batched pin redraw. The redraw was batched, but it was still being triggered by +too many non-local events. + +Network comms were also reviewed. Incoming learner comms can merge saved data +and call `InjectLearnedData()`, but that path intentionally does not promote NPC +spawn coordinates into live `npcDataOverrides[...][7]`; therefore remote comms +are less likely to be the direct pin-redraw trigger than local combat-log +`UNIT_DIED` visibility. + +## 22.2 - Implemented fix + +Changed `Modules/QuestieLearner.lua`: + +- `UNIT_DIED` still records short-lived `recentKills` evidence for later quest + objective progress correlation. +- `UNIT_DIED` now exits before `LearnNPC`, `_StoreGuidSpawnEvidence`, and + `_MergeSpawnEvidence`. +- `PARTY_KILL` keeps the existing learner behavior, so player/group kills still + learn spawn data and use the already-batched live update path. + +This preserves existing functionality where possible: + +- If a nearby death does not advance the player's quest, it no longer touches + learner live pin refresh. +- If a nearby death does advance the player's quest via a later quest-objective + update, the existing `recentKills` correlation path can still learn the NPC. +- Local/group kills still feed learner immediately through `PARTY_KILL`. + +All code remains Lua 5.0-compatible. + +## 22.3 - Added local verification + +Updated gitignored dev spec `Tests/QuestieLearner_performance_spec.lua`: + +- Added regression coverage proving `UNIT_DIED` from a visible bystander death + does not create learned NPC data, does not create live NPC overrides, and does + not queue learner timers/pin refresh work. +- Added paired coverage proving `PARTY_KILL` still learns the NPC and queues the + existing batched learner timers. + +Verification run: + +```text +busted Tests\QuestieLearner_performance_spec.lua +11 successes / 0 failures / 0 errors / 0 pending +``` + +Broader focused verification run: + +```text +busted Tests\AuditFindings_spec.lua Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +48 successes / 0 failures / 0 errors / 0 pending +``` + +Static verification: + +```text +selene Modules\QuestieLearner.lua Modules\Network\QuestieLearnerComms.lua Modules\Options\AdvancedTab\QuestieOptionsAdvanced.lua Modules\Options\QuestieOptionsDefaults.lua +0 errors / 103 warnings / 0 parse errors +``` + +## 22.4 - Suggestions if stutter remains after this patch + +1. Default new/low-end users to `Low Impact` instead of `Balanced`, or make the + first-run default `pinRefreshDelay = 2.0`, `liveNpcUpdateDelay = 2.0`, and + `minConfidencePins = 3`. This trades slower live pin learning for much lower + frame churn in dense zones. +2. Add a separate `Live Learned Pin Updates` toggle that maps to + `pinRefreshMode = "manual"`. This gives players a clear emergency switch: + learn data now, redraw pins later/reload. +3. Add a developer-only counter for `_RefreshActiveQuestPins`, + `_InvalidateSpawnListsForNPC`, and `_FlushActiveQuestPins` so testers can + quantify pin refreshes in game with the macro report. +4. Tighten incoming network merge semantics so `_ApplyIncomingNetworkMerge` + returns `true` only when data actually changed, not merely because `op` is + `NEW` or `UPDATE`. This is lower priority for pin redraw than the `UNIT_DIED` + fix, but it would reduce unnecessary `InjectLearnedData()` work from noisy + peers. + +--- + +# Pass-23 Implementation Update - arrow profiler red entries (2026-06-04) + +This pass investigated an in-game profiler screenshot with red entries around: + +- `QuestieCompat.HBD.GetPlayerWorldPosition` +- `QuestieArrow.Refresh` +- `QuestieArrow.UpdateNearestTargets` +- `QuestieCompat.GetCurrentPlayerPosition` +- `QuestieCompat.HBD.GetPlayerZonePosition` +- `QuestieCompat.HBD.GetWorldCoordinatesFromZone` +- `QuestieCompat.HBD.GetWorldDistance` +- `ZoneDB.GetUiMapIdByAreaId` +- `QuestieCoords.WriteCoords` +- `TrackerUtils.GetNearestQuestItemId` +- `QuestieLearnerComms.private.ProcessQueues` + +## 23.1 - Root-cause interpretation + +The hottest cluster is arrow-related, not learner-related. + +`QuestieArrow:Refresh()` calls `QuestieArrow:UpdateNearestTargets()`. The +nearest-target scan evaluates objectives and finishers across the active quest +log, then converts every candidate spawn into world coordinates and distance: + +- `ZoneDB:GetUiMapIdByAreaId(zone)` +- `HBD:GetWorldCoordinatesFromZone(...)` +- `HBD:GetWorldDistance(...)` + +The profiler showed about 50 arrow refreshes but thousands of HBD/ZoneDB calls. +That ratio matches a full candidate scan: each refresh can fan out into many +spawn conversions. The comment in `Modules/Arrow/QuestieArrow.lua` also confirms +zone filtering is currently disabled because of historical zone ID vs area ID +mismatch, so the scan does more work than a same-zone-only scan would. + +Separately, `QuestieArrow`'s frame `OnUpdate` recalculated the current target's +world coordinates every throttled tick. That explains why +`HBD.GetPlayerWorldPosition` and current-position helpers are high-call-count +even when the full nearest-target scan is only running periodically. + +Lower entries reviewed: + +- `QuestieCoords.WriteCoords` is the map/minimap coordinate ticker. In the + screenshot it is much lower total time than arrow scanning; leave it alone for + this pass unless a future profile isolates it with the arrow disabled. +- `TrackerUtils.GetNearestQuestItemId` is lower total time and likely incidental + tracker work. Do not mix tracker changes into this arrow pass. +- `QuestieLearnerComms.private.ProcessQueues` is low total time in the screenshot + and already intensity-gated from Pass-21. + +## 23.2 - Implemented fix + +Changed `Modules/Arrow/QuestieArrow.lua`: + +- Added a per-scan zone-to-UiMap cache for `ZoneDB:GetUiMapIdByAreaId(zone)`. + Multiple spawns in the same zone now reuse the same resolved UiMap ID during a + single nearest-target scan. +- Added `_AddArrowTarget(...)` so every collected target stores: + - zone coordinates; + - UiMap ID; + - already-computed world X/Y/instance; + - already-computed distance. +- The arrow frame `OnUpdate` now reuses the target's cached world X/Y/instance. + It only falls back to `HBD:GetWorldCoordinatesFromZone(...)` if the target was + created without cached world coordinates, such as a manual target path. +- Hoisted the `table.sort(sortedTargets, ...)` comparator into + `_SortTargetByDistance` so the refresh path no longer allocates a comparator + closure each sort. +- Avoided calling `objectiveFrame.distance:SetText(...)` when the formatted + distance string has not changed. + +This pass intentionally did **not** re-enable zone filtering. That could provide +a bigger reduction, but it changes target-selection semantics and needs live +validation across Ascension/custom maps, dungeons, and zone/area ID aliases. + +## 23.3 - Expected profiler impact + +Expected improvements: + +- Fewer `ZoneDB.GetUiMapIdByAreaId` calls inside `UpdateNearestTargets`, because + repeated spawns in the same zone are cached per scan. +- Fewer `HBD.GetWorldCoordinatesFromZone` calls during arrow frame updates, + because the current target keeps cached world coordinates. +- Less UI churn from distance text updates when the text did not change. +- Slightly less allocation pressure from the hoisted sort comparator and unified + target insertion helper. + +Expected remaining red/green entries: + +- `HBD.GetPlayerWorldPosition` may still appear frequently while the arrow is + visible because the arrow must rotate/distance-update as the player moves. +- `HBD.GetWorldDistance` remains necessary for candidate ranking and live + distance updates. +- `QuestieArrow.UpdateNearestTargets` can still be expensive in large quest logs + because zone filtering remains disabled. + +## 23.4 - Verification + +Focused verification run: + +```text +busted Tests\QuestieArrow_spec.lua Tests\AuditFindings_spec.lua Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +54 successes / 0 failures / 0 errors / 0 pending +``` + +Static verification: + +```text +selene Modules\Arrow\QuestieArrow.lua +0 errors / 10 warnings / 0 parse errors +``` + +Selene warnings are existing one-line style warnings in `QuestieArrow.lua`, not +parser/runtime errors from this pass. + +Note: `Tests\QuestieArrowAssets_spec.lua` was not included in the final focused +run because unrelated untracked local files (`Icons/Arrows/DropTestArrow*.tga`) +make that asset-manifest spec fail. Those files are outside this patch. + +## 23.5 - Recommended next steps if arrow remains hot + +1. Add an Advanced/Arrow performance slider for nearest-target refresh interval + (`RECALC_NEAREST_SECONDS`), e.g. `1.0s`, `2.0s`, `5.0s`. +2. Add an Advanced/Arrow performance slider for arrow movement update interval + (`UPDATE_THROTTLE_SECONDS`), e.g. `0.05s`, `0.10s`, `0.20s`. +3. Add a safe same-map/same-zone candidate preference: first scan current UiMap + candidates, then fall back to cross-zone candidates only if no local target + exists. This should reduce the disabled-zone-filter cost without breaking + cross-zone quest arrows. +4. Add temporary dev counters for: + - `QuestieArrow:Refresh()` + - `QuestieArrow:UpdateNearestTargets()` + - `_GetUiMapIdForZone()` cache hits/misses + - target count before sort + These counters would let testers prove whether the profile improves in-game. + +--- + +# Pass-24 Implementation Update - Arrow performance settings (2026-06-04) + +This pass compared the Pass-23 arrow optimization against earlier audit +recommendations before making additional changes. + +## 24.1 - Audit comparison + +The Pass-23 patch matched the best low-risk audit recommendations: + +- PP3: `QuestieArrow` no longer allocates an inline `table.sort` comparator per + nearest-target refresh. It uses `_SortTargetByDistance`. +- PP4: the arrow no longer blindly calls `FontString:SetText(...)` for unchanged + distance strings. +- The profiler-specific Pass-23 recommendation to cache target world coordinates + was also implemented, reducing repeated `HBD:GetWorldCoordinatesFromZone(...)` + calls during arrow frame updates. + +The larger possible optimization, same-zone/same-map filtering, was intentionally +not used yet. The existing arrow code explicitly disabled zone filtering because +area IDs, UiMap IDs, and custom Ascension map IDs can mismatch. Re-enabling hard +filtering could make the arrow ignore valid cross-zone, dungeon, or custom-map +targets. The safer next step is to expose throttle settings so users can reduce +scan/update frequency without changing target correctness. + +## 24.2 - Implemented fix + +Changed `Modules/Arrow/QuestieArrow.lua`: + +- Replaced fixed constants with clamped profile-backed getters: + - `arrowUpdateThrottle` controls arrow rotation/distance update frequency. + - `arrowRecalcInterval` controls full nearest-target scan frequency. + - `arrowTrackerRefreshThrottle` controls refresh bursts triggered by tracker + updates. +- Defaults preserve previous behavior: + - `arrowUpdateThrottle = 0.05` + - `arrowRecalcInterval = 1.0` + - `arrowTrackerRefreshThrottle = 0.5` +- Runtime clamps protect against bad SavedVariables: + - movement update: `0.03` to `0.5` seconds; + - target scan: `0.5` to `10.0` seconds; + - tracker refresh: `0.25` to `5.0` seconds. + +Changed `Modules/Options/QuestieOptionsDefaults.lua`: + +- Added defaults for the three new Arrow performance settings. + +Changed `Modules/Options/ArrowTab/QuestieOptionsArrow.lua`: + +- Added an `Arrow Performance` section under the Arrow tab. +- Added sliders: + - `Arrow Movement Update Interval`; + - `Target Scan Interval`; + - `Tracker Refresh Throttle`. + +Changed local gitignored verification file `Tests/AuditFindings_spec.lua`: + +- Added PP7 assertions proving the runtime getters, Arrow-tab controls, and + defaults exist. + +All new runtime code remains Lua 5.0-compatible; this pass did not introduce +new `#`, `%`, `...`, `goto`, `select`, or `table.unpack` usage. + +## 24.3 - Recommended user settings + +For players seeing red profiler entries or FPS drops with the arrow enabled: + +- Start with `Target Scan Interval = 3.0s`. +- Set `Tracker Refresh Throttle = 1.5s`. +- Set `Arrow Movement Update Interval = 0.10s`. + +For very low-end machines or crowded zones: + +- `Target Scan Interval = 5.0s`. +- `Tracker Refresh Throttle = 2.0s`. +- `Arrow Movement Update Interval = 0.15s` to `0.20s`. + +Tradeoff: the arrow still points correctly, but it may take longer to switch to +a newly nearest objective and may rotate/update distance less smoothly. + +## 24.4 - Verification + +Focused verification run: + +```text +busted Tests\QuestieArrow_spec.lua Tests\AuditFindings_spec.lua Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +55 successes / 0 failures / 0 errors / 0 pending +``` + +Static verification: + +```text +selene Modules\Arrow\QuestieArrow.lua Modules\Options\ArrowTab\QuestieOptionsArrow.lua Modules\Options\QuestieOptionsDefaults.lua +0 errors / 10 warnings / 0 parse errors +``` + +Selene warnings are existing one-line style warnings in `QuestieArrow.lua`, not +new parser/runtime errors from this pass. + +## 24.5 - Remaining next step + +If the arrow still dominates profiles after users raise these throttles, the +next best code change is a same-map-first scan: + +1. scan candidates on the player's current UiMap/normalized area first; +2. if local candidates exist, sort only those; +3. if none exist, fall back to the existing cross-zone scan. + +That would reduce candidate volume while preserving cross-zone functionality, +but it should be live-tested because of Ascension/custom map ID edge cases. + +--- + +# Pass-25 Verification Update - Arrow and Learner options live wiring (2026-06-04) + +This pass verified that the Arrow and QuestieLearner performance settings are +fully wired from options UI to runtime behavior and adjust without requiring a +reload. + +## 25.1 - Arrow wiring verification + +Verified path: + +- Defaults: + - `Modules/Options/QuestieOptionsDefaults.lua` + - `arrowUpdateThrottle = 0.05` + - `arrowRecalcInterval = 1.0` + - `arrowTrackerRefreshThrottle = 0.5` +- Options UI: + - `Modules/Options/ArrowTab/QuestieOptionsArrow.lua` + - `Arrow Movement Update Interval` + - `Target Scan Interval` + - `Tracker Refresh Throttle` +- Runtime: + - `Modules/Arrow/QuestieArrow.lua` + - `_GetArrowUpdateThrottle()` is read inside the arrow frame `OnUpdate`. + - `_GetArrowRecalcInterval()` is read inside the driver frame full target + scan scheduler. + - `_GetArrowTrackerRefreshThrottle()` is read inside the tracker-update hook. + +Conclusion: Arrow settings are live-read on every relevant throttle check. New +values affect future arrow updates/target scans without reload. The +`Target Scan Interval` setter also calls `QuestieArrow:Refresh()` so the player +gets an immediate recalculation after changing the scan interval. + +No Arrow runtime patch was required in this pass. + +## 25.2 - Learner wiring verification + +Verified path: + +- Defaults/backfills: + - `Modules/QuestieLearner.lua` + - `performanceMode` + - `pinRefreshDelay` + - `pinRefreshMode` + - `liveNpcUpdateDelay` + - `learnerCommsIntensity` +- Options UI: + - `Modules/Options/AdvancedTab/QuestieOptionsAdvanced.lua` + - `Performance Mode` + - `Pin Refresh Behavior` + - `Pin Refresh Delay` + - `Live NPC Update Delay` + - `Minimum Kills Before Learned Pins` + - `Learner Comms Intensity` +- Runtime: + - `pinRefreshMode` gates scheduling and now also gates already queued flushes. + - `pinRefreshDelay` is read when scheduling pin refresh timers. + - `liveNpcUpdateDelay` is read when scheduling NPC live update and network merge + timers. + - `learnerCommsIntensity` is read by outgoing learner broadcasts and learner + comms queue processing. + - `minConfidencePins` is read before learned NPC spawn evidence is promoted to + live overrides. + +## 25.3 - Issues found and fixed + +1. Preset/comms desync: + + If a player set `Learner Comms Intensity = Off`, the setter correctly set + `Questie.db.profile.learnerBroadcast = false`. However, choosing a preset + such as `Balanced` or `Realtime` later changed `learnerCommsIntensity` back to + `normal` or `fast` without re-enabling `learnerBroadcast`. The UI could show + comms as active while outgoing learner broadcasts remained disabled. + + Fix: + + - `ApplyLearnerPerformancePreset("realtime"|"balanced"|"low")` now sets + `Questie.db.profile.learnerBroadcast = true`. + - Manual mode leaves existing values alone. + +2. Manual pin refresh emergency stop: + + `pinRefreshMode = "manual"` prevented new refresh scheduling, but if a + batched refresh timer had already been queued, that pending flush could still + run once after the user switched to Manual. + + Fix: + + - `_FlushActiveQuestPins()` now re-checks `pinRefreshMode`. + - If the setting is `manual`, the pending frame-unload table is cleared and + no `QuestieQuest:UpdateQuest(...)` call is made. + +## 25.4 - Added local verification + +Updated gitignored dev specs: + +- `Tests/QuestieLearner_performance_spec.lua` + - Added regression coverage proving a queued learner pin refresh is suppressed + if the user switches to Manual before the timer fires. +- `Tests/AuditFindings_spec.lua` + - Added an assertion that learner presets keep comms intensity and + `learnerBroadcast` synchronized. + +## 25.5 - Verification + +Focused verification run: + +```text +busted Tests\QuestieArrow_spec.lua Tests\AuditFindings_spec.lua Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +57 successes / 0 failures / 0 errors / 0 pending +``` + +Static verification: + +```text +selene Modules\QuestieLearner.lua Modules\Options\AdvancedTab\QuestieOptionsAdvanced.lua Modules\Arrow\QuestieArrow.lua Modules\Options\ArrowTab\QuestieOptionsArrow.lua Modules\Options\QuestieOptionsDefaults.lua Modules\Network\QuestieLearnerComms.lua +0 errors / 113 warnings / 0 parse errors +``` + +Warnings are existing style warnings in large files, not parser/runtime errors +from this pass. + +--- + +# Pass-26 Implementation Update - QuestieComms performance options (2026-06-04) + +This pass added live QuestieComms performance controls under Advanced and split +the work into individual commits for easier revert/bisect: + +- `c6bfa95 perf: wire questie comms throttles` +- `a113791 perf: add questie comms disable gate` +- `b62def5 feat: add questie comms performance options` +- `ccd2bc1 refactor: centralize performance options` + +## 26.1 - Commit strategy note + +Going forward, changes should be committed one logical unit at a time. This pass +used three separate commits: + +1. runtime/default support for QuestieComms throttles; +2. runtime/default support for disabling QuestieComms entirely; +3. Advanced-tab UI exposure. +4. centralization of all performance tuners into the Advanced tab. + +That makes reverting UI exposure independent from reverting runtime gates. + +## 26.2 - Implemented runtime settings + +Changed `Modules/Network/QuestieComms.lua`: + +- Added clamped profile-backed getters: + - `questieCommsQuestListPacketSize` + - `questieCommsQuestListInitialJitter` + - `questieCommsQuestListBlockInterval` + - `questieCommsEnabled` +- Replaced hardcoded full quest-list block size with + `GetQuestListPacketSizeLimit()`. +- Replaced hardcoded initial response jitter (`random() * 3`) with + `GetQuestListInitialJitter()`. +- Replaced hardcoded full quest-list block ticker interval (`3`) with + `GetQuestListBlockInterval()`. +- Added a live `IsQuestieCommsEnabled()` gate to: + - quest update broadcasts; + - quest remove broadcasts; + - yell progress; + - legacy full quest-list broadcast; + - V2 full quest-list broadcast; + - full quest-list requests; + - low-level packet broadcast; + - incoming QuestieComms receive handler. + +The master enabled toggle is live-read at send/receive time, so disabling +QuestieComms takes effect immediately without reload. Already queued full +quest-list ticker callbacks may continue to drain their local queues, but every +packet write routes through `_QuestieComms:Broadcast(...)`, which now checks the +enabled flag before sending. + +Changed `Modules/Options/QuestieOptionsDefaults.lua`: + +- Added defaults preserving previous behavior: + - `questieCommsEnabled = true` + - `questieCommsQuestListPacketSize = 200` + - `questieCommsQuestListInitialJitter = 3` + - `questieCommsQuestListBlockInterval = 3` + +## 26.3 - Implemented Advanced-tab options + +Changed `Modules/Options/AdvancedTab/QuestieOptionsAdvanced.lua`: + +- Added `QuestieComms Performance` section. +- Added `Enable QuestieComms` toggle. +- Added sliders: + - `Quest List Packet Size`; + - `Quest List Initial Jitter`; + - `Quest List Block Interval`. +- Sliders are disabled visually when `Enable QuestieComms` is off. + +These settings are intentionally under Advanced because they control network and +CPU burst behavior rather than normal user quest display behavior. + +Important UI convention from this pass: + +- All performance/intensity tuners should live in `Advanced`. +- The `Advanced` tab now contains: + - `QuestieLearner Performance`; + - `QuestieArrow Performance`; + - `QuestieComms Performance`. +- The Arrow tab no longer owns Arrow performance sliders. It should remain for + normal arrow display/appearance/target behavior. +- If future performance controls are added, add them to Advanced rather than + scattering them into feature-specific tabs. + +## 26.4 - Settings behavior + +Recommended low-end/crowded-group starting point: + +- `Enable QuestieComms = On` unless the player wants to completely opt out. +- `Quest List Packet Size = 150` +- `Quest List Initial Jitter = 5.0s` +- `Quest List Block Interval = 5.0s` + +Emergency/no-comms mode: + +- `Enable QuestieComms = Off` + +Tradeoff: + +- Lower packet size can increase block count but reduces per-packet serialization + and transmission size. +- Higher jitter/block interval reduces bursts but makes full quest-list syncs + slower. +- Disabling QuestieComms stops group quest-progress sharing from this module. + +## 26.5 - Added local verification + +Updated gitignored dev specs: + +- `Tests/AuditFindings_spec.lua` + - Added assertions proving QuestieComms settings are profile-backed. + - Added assertions proving the Advanced controls are exposed. +- `Tests/QuestieLearner_performance_spec.lua` + - Added runtime coverage proving outgoing QuestieComms packets are suppressed + immediately when `questieCommsEnabled = false`. + +## 26.6 - Verification + +Focused verification run: + +```text +busted Tests\QuestieArrow_spec.lua Tests\AuditFindings_spec.lua Tests\QuestieLearner_performance_spec.lua Tests\QuestieDB_suppression_spec.lua +60 successes / 0 failures / 0 errors / 0 pending +``` + +Static verification: + +```text +selene Modules\Network\QuestieComms.lua Modules\Options\QuestieOptionsDefaults.lua +0 errors / 24 warnings / 0 parse errors + +selene Modules\Options\AdvancedTab\QuestieOptionsAdvanced.lua +0 errors / 3 warnings / 0 parse errors + +selene Modules\Options\AdvancedTab\QuestieOptionsAdvanced.lua Modules\Options\ArrowTab\QuestieOptionsArrow.lua +0 errors / 3 warnings / 0 parse errors +``` + +Warnings are pre-existing style warnings in touched legacy files. + +# Implementation Plan (consolidated — how to land everything) + +This is the recommended order to incorporate every finding above. It is built +around three realities: (1) the biggest unknown is **what client/Lua the targets +actually run**; (2) three subsystems are **fragile** (tracker, minimap/HBD, +Ascension learner) and out of scope unless explicitly chosen; (3) every change +should be **bisectable** and guarded by `Tests/AuditFindings_spec.lua`. + +## Phase 0 — Decide the target matrix (blocks all 1.12 work) + +Run these in-game on each target you intend to support; record the answers. + +| Probe | Question it answers | +|---|---| +| `/run print(#({1,2,3}))` | Does `#` parse? (gates the raw-`#` work; §12.3 says the 33/134 count is stale) | +| `/run print((4 % 3))` | Does `%` parse? (gates the 11-file `%` work) | +| `/run print(bit and bit.band(3,1))` | Is the `bit` library present? (N1) | +| `/run print(type(strsplit))` | Is `strsplit` present? (N2) | +| `/run local f=loadstring("return ...") print(f and pcall(f,1))` | Does the `...` expression parse? | + +**Decision gate:** if a target errors on `#`/`%`/`...`, it is genuinely Lua 5.0 +and Phase 2 is required for that target. If all print fine, Phase 2 is **skipped +entirely** (the addon already works there) and you go straight from Phase 1 to +Phase 3. Do not start Phase 2 before this gate. + +## Phase 1 — Zero-risk correctness & hygiene (one commit, all targets) + +Independent, mechanical, each guarded by a spec assertion. Land as one commit +(or the four sub-groups below for clean bisection). + +1a. **Pure typos / dedup** — `Questie-X.toc:182` duplicate (B5); + `QuestieOptionsTracker.lua:486/522/797` `fadeTickerValue:Cancel()` → + `fadeTicker:Cancel()` (B2); strip BOMs from the 3 `Database/Corrections/*` + files (B6). +1b. **Defensive nil-guards** — `QuestieDB.lua:1355` hoist the double `GetQuest` + (B1); `QuestieCommsData.lua:49,51` guard `GetNPC/GetObject(...).name` like the + item branch (N4). +1c. **Bounded growth / dead wiring** — wipe `alreadySentBandaid` on login (B3); + `QuestieFrame.lua:133` wire `BaseOnUpdate` to `GlowUpdate` **or** delete the + dead ticker (B7, pick one); add the unused `questId` param to + `Ascension_IsScalingEnabled` to clear the lint (FP1 — cosmetic). +1d. **Loop-correctness** — `QuestieNameplate.lua:88` change the early `return` to + skip-this-entry (G1); same review for the `QuestieQuest.lua:222` early-return + (pass-2 item). +1e. **Verification-gated / debug hygiene** — verify B4 (`QuestieDB.lua:1903` + load-time faction capture) in-game, then lazily compute/recompute + `factionReactions` if needed; guard expensive debug-only diagnostics before + `_dbStats(...)` / `spawnOverrideCount` are computed (H2). + +*Verify:* `busted` stays green except the 4 pre-existing ArrowAssets fails; +invert the matching `[B*]`/`[N4]` spec assertions as each lands. In-game smoke: +accept/abandon a quest, open a comms tooltip, hover a nameplate. + +## Phase 2 — 1.12 / Lua 5.0 remediation (ONLY if Phase 0 says a target needs it) + +Order matters — the bootstrap must parse before anything else. + +2a. **Runtime gaps first (cheap, high-impact):** add a `bit` shim and a + `strsplit` shim to `QuestieCompat`/`QuestieLoader` (N1/N2). For `bit`, mirror + the guard XXH already uses; provide pure-Lua `band/bor/bxor/lshift/rshift`. + These are nil-index crashes at load (`Questie.lua:1`) so they gate everything. +2b. **`%` operator (11 files, ~50 sites, 11.1):** replace `a % b` with + `math.mod(a, b)` (already shimmed). Start with `QuestieStream.lua` (23) since + it's load-bearing for the DB. +2c. **`#` operator (TOC-driven; previous 33 files / 134 sites count is stale):** + replace `#t` with `table.getn(t)` (already shimmed). Include the Pass-12 menu + misses (`Townsfolk.lua`, `QuestieMenu.lua`) and generate the worklist from the + active TOC, not from the old count. Mechanical; do it per-file with + `luac5.1 -p` verification. +2d. **`...` expression (~15 sites) + Options `{ ... }` (15 files):** change + `{ ... }` → `{}` (N3, trivial); for the `(...)` forwarders, use `arg`/`unpack` + or `loadstring` the closure (note: runtime `if` guards do **not** prevent the + parse error — §9.6). Finish the `select(8, …)` rewrite in `QuestiePlayer` + (N5) to match `QuestieLearner`. + +*Verify:* `luac5.1 -p ` per file (proxy; real check is loading on the 5.0 +client). Add a CI lint that greps changed files for raw `#`/`%`/`...`. + +## Phase 3 — Performance (measured; all targets; skip fragile) + +Order by impact-to-risk. Profile before/after each with `QuestieProfiler`. + +3a. **Allocation/hot-path, low risk:** l10n no-arg fast-path + per-key cache (P2); + PP2 (`local profile = Questie.db.profile` in MapIconTooltip/QuestieMap/Tooltip); + PP3 (hoist sort comparators in QuestieComms/QuestieArrow); PP4 (cache arrow + distance string); PP5 (hoist `GetTime()`); G2 (hoist the validate-cache pcall); + H2 (guard expensive debug-only computations). +3b. **Algorithmic, medium risk (needs in-game regression):** P1 comms O(n²)→O(n) + serialize; P5 tooltip party-map (`GetPartyMemberList` once); PP1 `IsDoable` + → one batch `QueryQuest`; P4 `tunpack` → `unpack`; P3 front-removal queues; + P6 front-insert sites; PP6/N6 bounded NPC-name fallback scans. +3c. **Trigger gating (highest payoff, most care):** P7 — move the Options + CalculateAndDrawAll debounce to the UI layer and gate event-driven rescans; + P9 — batch learner `InjectLearnedData`. + +*Verify:* `QuestieProfiler` deltas; Sunstrider pin test; Ascension login + accept; +full `busted`. + +## Phase 4 — Fragile / large refactors (only if still needed, with sign-off) + +Tracker (`db.profile`×83, sort comparators, GetTime — NOTE-ONLY items), HBD +minimap, `CalcHotzones` spatial index (**fix the input-mutation bug first**), +nearest-spawn cache, and the learner `GetNPCIdByName` cache/index (H4) unless +Ascension learner work is already in scope. Do **not** enter Phase 4 without a +measurement proving the win and an explicit decision to touch the fragile +subsystem. + +## Finding → phase map + +| Phase | Findings | +|---|---| +| 1 (zero-risk / verify-gated) | B1 B2 B3 B4-verify B5 B6 B7 N4 G1 FP1 H2, QuestieQuest early-return | +| 2 (1.12, gated) | N1 N2 N3 N5, TOC-driven `#` (§12.3), `%`(11), `...`(15), §9.6 | +| 3 (perf, measured) | P1 P2 P3 P4 P5 P6 P7 P9 PP1 PP2 PP3 PP4 PP5 PP6/N6 G2 H2 | +| 4 (fragile, sign-off) | P-minimap/HBD, CalcHotzones(P16), nearest-spawn(P17), PP-tracker NOTE-ONLY, H4 learner name-index | +| Do NOT do | FP1-as-bug, FP2 (`"Player"`), FP3 (select), FP4 (learner argN), FP5 (TaskQueue/MapExploration), shared scheduler/QuestiePerf abstraction | + +## Branch / commit strategy + +- Branch per phase; never commit to the default branch directly. +- Phase 1 in the 4 sub-groups (1a–1d) = 4 trivially-revertable commits. +- Phase 2 per-file commits with `luac5.1 -p` in the message. +- Phase 3 one commit per finding with a before/after profiler number. +- Tag `audit-restore-2026-06-03` is the rollback point; re-tag after Phase 1. +- After each phase, run `busted` and update `Tests/AuditFindings_spec.lua` + (invert the snapshot assertions for whatever was fixed). + +## Pass 26 - QuestieComms scope-order fix + +User-visible runtime error reported on Area 52: +`Modules/Network/QuestieComms.lua:192: attempt to call global 'IsQuestieCommsEnabled' (a nil value)`. + +Root cause: `BroadcastQuestUpdate()` and related early broadcast helpers were +compiled before `IsQuestieCommsEnabled` existed as a local binding, so Lua +resolved the symbol as a global at runtime. + +Fix applied: +- Added a forward declaration near the top of `Modules/Network/QuestieComms.lua`. +- Switched the later helper definition to assign into that same local binding. +- Added a regression assertion in `Tests/AuditFindings_spec.lua` that verifies + the helper is declared before `BroadcastQuestUpdate`. + +Verification: +- `busted Tests\\AuditFindings_spec.lua Tests\\QuestieLearner_performance_spec.lua` +- `selene Modules\\Network\\QuestieComms.lua Tests\\AuditFindings_spec.lua` + +Result: +- The comms disable gate is now wired through a real local helper for all +runtime call sites, instead of depending on an accidentally-global symbol. + +--- + +# Pass-27 Reconciliation Update - 1.12 scope, branch status, and remaining gaps (2026-06-04) + +This pass reconciles the live codebase against the 26 prior audit passes and +adds a small number of net-new findings that the prior passes did not cover. + +## 27.1 - 1.12 scope confirmation + +User confirmed (2026-06-04): Turtle is irrelevant, but "there are other 1.12 +servers" and the addon must continue to work on 1.12 (Lua 5.0) clients. + +Implications: + +- The Turtle TOC deletion (Pass-14) was correct. +- The Lua 5.0 shim layer in `Modules/Libs/QuestieLoader.lua` and + `Modules/QuestieCompat.lua` must remain and stay parse-safe. +- The 23 `function(...)` sites flagged in Pass-6/7 are still real 1.12 blockers + and **must** be remediated before claiming 1.12 support. +- The `select(index, ...)` shim at `QuestieLoader.lua:48-58` is itself a parse + error on Lua 5.0, so the bootstrap file fails to load on 1.12, so nothing + else loads. This is the highest-priority single bug in the repo for 1.12. +- The `tunpack` recursive implementation at `QuestieLib.lua:655-668` is Lua + 5.0-parseable but slow; it should still be replaced for perf on all targets. +- Pass-19's `QuestieLearner` OnEvent payload fix used explicit positional args + rather than `...`, which is **the right pattern** for keeping 1.12 working. + Use that pattern as the model for the other 22 sites. + +Net effect: Phase 0 in the implementation plan (the `#`/`%`/`...` probe) is +still required. The plan in the existing report stands. + +## 27.2 - Reconciliation against current branch state + +This pass verified each of the prior findings against the current `HEAD` of +`codex/questie-learner-comms-improvements`. Status changes since Pass-26: + +| Finding | Prior status | Current status (verified) | Notes | +|---|---|---|---| +| FP4 learner `arg1..arg10` closure bug | still open | **implemented** (Pass-19) | `Modules/QuestieLearner.lua:3568` now takes explicit `arg1..arg10` positional params. | +| L0 redundant `private = _Learner` | open | **cleaned up** (Pass-20) | Removed. | +| P1 QuestieComms O(n²) quest list | open | **implemented** (Pass-17) | V2 path uses `GetQuestDataPacketV2Size` (incremental). V1 path still calls `GetSerializedPacketSize` per quest (see 27.3 finding). | +| P3 comms queue front-removal | partially | **partially implemented** (Pass-18) | `QuestieComms` queues fixed; other modules (`QuestieCombatQueue`, `QuestieMap`, `QuestieFramePool`, `QuestieValidateGameCache`) still use `tremove(_,1)`. | +| L4 learner comms queue head/tail | open | **implemented** (Pass-16) | `ProcessQueues` uses head/tail indexing. | +| L1/L2/L3 learner batching | open | **implemented** (Pass-15) | Inbound merge batching, NPC name index, debug guards. | +| L5/L6/L7/L8 learner perf options | open | **implemented** (Pass-21) | Advanced tab exposes presets and per-knob controls. | +| PP3 sort comparator allocation | open | **implemented** (Pass-23) | `_SortTargetByDistance` hoisted. | +| PP4 `SetText` only on change | open | **implemented** (Pass-23) | `objectiveFrame.distance:SetText(...)` guarded. | +| PP7 arrow perf settings | open | **implemented** (Pass-24) | Three sliders under Arrow tab; `Pass-26` centralizes them under Advanced. | +| Comm throttle + disable gate | open | **implemented** (Pass-26) | `questieCommsEnabled` + 3 throttle sliders. | +| B5 duplicate `QuestieSlash.lua` in TOC | open | **still open** | `Questie-X.toc:23` and `:182` both list it. | +| B2 `fadeTickerValue:Cancel()` x3 | open | **still open** | `QuestieOptionsTracker.lua:486/522/797`. | +| B6 4 BOM files | open | **still open** | 3 in `Database/Corrections/`, 1 in vendored `Compat/Libs/LibSharedMedia-3.0/`. | +| B1 `IsComplete` double `GetQuest` | open | **still open** | `QuestieDB.lua:1355`. | +| B3 `alreadySentBandaid` unbounded | open | **still open** | `QuestieAnnounce.lua:19/123`. | +| B7 `_Qframe.BaseOnUpdate` nil | open | **still open** | `QuestieFrame.lua:133`; ticker at `QuestieFramePool.lua:110` never fires. | +| B4 load-time `UnitFactionGroup` | open | **still open** | `QuestieDB.lua:1903`. | +| FP2 `UnitFactionGroup("Player")` | open | **still open** | `QuestieMenu.lua:114`. | +| FP1 arity of `Ascension_IsScalingEnabled` | cosmetic | **still open** | `QuestieLib.lua:33`. | +| P2 l10n per-call alloc | open | **in progress** | 1016 `l10n(...)` call sites; `Localization/l10n.lua` now has a no-arg fast path, locale cache, and bounded small-arg formatter, with only rare long arg lists falling back to a packed table. | +| PP1 `IsDoable` 12+ hash lookups | open | **implemented** (Pass-39) | `QuestieDB.IsDoable` now batches its eligibility field reads through one `QueryQuest(...)` call. | +| P5 tooltip party-map cache | open | **in progress** | `QuestiePlayer:GetPartyMemberByName` now uses a cached lookup table; the cache invalidates on roster changes. | +| P4 `tunpack` recursion | open | **still open** | `QuestieLib.lua:655-668` still recursive. | +| P6 3 `tinsert(t,1,x)` sites | open | **still open** | `MapIconTooltip.lua:188/889`, `Tooltip.lua:437`. | +| P7 `CalculateAndDrawAll` debounce | open | **implemented** (Pass-37) | Options-tab rescans are debounced, event-driven callers route through `CalculateAndDrawAllDebounced`, and the available-quest draw path no longer spawns a coroutine per quest. | +| H4 `GetNPCIdByName` index | open | **implemented** (Pass-15) | Lowercase name index with override precedence. | +| H2 debug-only expensive scans | open | **implemented** (Pass-15) | `_dbStats(...)` + `spawnOverrideCount` scan guarded by debug flag. | +| N1 bit library shim | open | **implemented** (Pass-30) | `QuestieLoader` now provides a pure-Lua `bit` fallback. | +| N2 `strsplit` shim | open | **implemented** (Pass-30) | `QuestieLoader` now provides a real `strsplit(separator, text, max)` fallback. | +| N3 `{...}` table constructor | open | **implemented** (Pass-32) | The five Options bootstrap files now use `{}` instead of `{ ... }`. | +| N4 `GetNPC/GetObject(...).name` nil | open | **still open** | `QuestieCommsData.lua:49,51`. | +| N5 `select(8, GetQuestLogTitle(...))` | open | **implemented** (Pass-31) | `QuestieEventHandler.lua` and `QuestieTracker.lua` now use explicit unpacking. | +| 22 other `function(...)` sites | open | **still open** | Same as Pass-6/7 list. | + +## 27.3 - New finding (Pass-17 follow-up) + +`GetSerializedPacketSize` at `Modules/Network/QuestieComms.lua:303-305`: + +```lua +local function GetSerializedPacketSize(packet) + return string.len(QuestieSerializer:Serialize(packet)) +end +``` + +- Pass-17 reported this as "replaced with incremental estimation". In reality, + only the **V2** path (`GetQuestDataPacketV2Size`, line 331-360) was + incrementalized. The V2 helper still ends with + `return GetSerializedPacketSize(questPacket)` at line 359, so it does + increment up to a byte-accurate Serialize at the end. +- The **V1** path at `QuestieComms.lua:662` still calls + `GetSerializedPacketSize(quest)` once per quest inside the broadcast loop. + For 30 quests in a broadcast block, 30 full Serialize-and-discard calls per + broadcast. +- The `QuestieSerializer:Serialize` call invokes both `isArray` (a `pairs()` + scan) and a count pass before writing. So 30 broadcasts = 30 × 2 hash + walks + 30 string allocs just to discard the string. + +**Recommendation:** Have `QuestieSerializer` expose a `Size` variant that walks +the same structure but returns the size without allocating the string. The +V1 path at line 662 is the call site; the V2 path's terminal call at line 359 +would also benefit. Low-risk to implement — the walk code already exists. + +**ID:** P1b (subordinate to P1). + +## 27.4 - New finding (Pass-26 follow-up) + +`Modules/Network/QuestieComms.lua:300`: + +```lua +return Questie.db.profile.questieCommsEnabled ~= false +``` + +This is the `IsQuestieCommsEnabled()` helper that Pass-26 added. It is now a +proper local helper, used by all broadcast entry points per the +`Questie:Error` runtime report ("attempt to call global 'IsQuestieCommsEnabled'") +that motivated the Pass-26 scope-order fix. + +**Subtle behavior note:** the helper defaults to **enabled** when the setting +is `nil` (`~= false` means `nil` also returns `true`). For a brand-new +character with no SavedVariables yet, this is correct (comms should default +on). For a character whose profile explicitly set +`questieCommsEnabled = false`, it correctly returns `false`. No bug, just +documenting the contract so a future pass doesn't flip the default to +`== true`. + +## 27.5 - New finding (pass-22 follow-up) + +Pass-22 closed the `UNIT_DIED` bystander-kill path. While verifying, also +confirmed: `Modules/QuestieLearner.lua:2967-3014` (`OnCombatLogEvent`) still +references `arg1..arg10` directly inside the body for the +`CombatLogGetCurrentEventInfo` legacy path. This is **intentional** for the +`3.3.5a` fallback when `CombatLogGetCurrentEventInfo` is not present, but it +relies on the Lua 5.0-style implicit `arg` table being available. + +**Lua 5.0 behavior:** `arg` is populated automatically for vararg functions. +**Lua 5.1+ behavior:** `arg` is not populated; you must capture `...` +explicitly. The Pass-19 fix made the **OnEvent handler** explicit +(`arg1..arg10` as named params), but the body inside `OnCombatLogEvent` still +reads from the implicit `arg`. + +**On 3.3.5a (Lua 5.1):** `arg1..arg10` are the named params from Pass-19, not +the implicit `arg`. The `CombatLogGetCurrentEventInfo` legacy path at +line 2984 (`if not timestamp and arg1 then ...`) reads `arg1` correctly because +it is a named param. + +**On 1.12 (Lua 5.0):** `function(_, event, arg1, arg2, ...)` is **invalid +syntax** because `arg1, arg2, ...` is `...` in the parameter list, which 5.0 +parses as a syntax error. The Pass-19 fix is therefore **only safe for 5.1+**, +not for 1.12. + +**This is a real 1.12 regression introduced by Pass-19.** The pre-Pass-19 code +used `function(_, event)` and relied on the implicit `arg` table, which is the +5.0-safe pattern. Post-Pass-19 code uses named params, which works on 5.1 but +not on 5.0. + +**Recommendation:** Restore Lua 5.0 compatibility at the OnEvent site. Either: + +1. Use `function(_, event, ...)` and read the args via `select(1, ...)` etc. + (5.0 + 5.1 safe). +2. Apply the `loadstring`-injection pattern from `QuestieLoader.lua` lines + 5-10/15-20 to wrap the OnEvent closure. + +Option 1 is cleaner and matches the pattern used by `QuestieCompat.lua:22-67` +for the xpcall variadic shim. + +**ID:** 1.12-REGRESSION-1. + +## 27.6 - New finding (Pass-17 follow-up) + +`Modules/Network/QuestieComms.lua:302` comment from prior pass claims the +"old `rawQuestList` re-serialization string is absent." The string in +question was likely the `Serialize(rawQuestList)` call inside the broadcast +build loop. Verified absent — the per-block Serialize inside the loop is +gone. Replaced with the per-quest `GetSerializedPacketSize(quest)` call at +line 662 (still O(n) Serialize, but only one Serialize per quest rather than +re-serializing the growing block). The audit snapshot test in +`Tests/AuditFindings_spec.lua` therefore holds. + +## 27.7 - New finding (Phase 0 prerequisite) + +Per the implementation plan, Phase 0 is a `/run` probe session on each target +to determine which Lua features are available. The current `selene` warnings +break down to 103 in `Modules/QuestieLearner.lua` and related, all style-level +(no parse errors, no hard errors). That confirms the source is parseable on +the targets `selene` was configured for. + +**The 1.12 probe has not been run.** Until a user-side probe on a real 1.12 +client confirms whether `#`, `%`, and `...` parse, the Phase 2 work cannot +be safely scoped. The implementation plan correctly gates Phase 2 on this +probe. + +## 27.8 - Updated ledger + +| ID | Status | Notes | +|---|---|---| +| FP4 | implemented | Pass-19 | +| L0 | cleaned up | Pass-20 | +| P1 | implemented (V2) | V1 still does per-quest Serialize; see 27.3. | +| P1b | still open | `GetSerializedPacketSize` is still called from V1 broadcast path. | +| P3 | partially implemented | Comms queues fixed; 6 other `tremove(_,1)` sites still live. | +| L1/L2/L3/L4/L5/L6/L7/L8 | implemented | Passes 15, 16, 21. | +| PP3/PP4 | implemented | Pass-23. | +| PP7 | implemented | Pass-24. | +| Comm throttle/disable | implemented | Pass-26. | +| H2/H4 | implemented | Pass-15. | +| B1/B2/B3/B4/B5/B6/B7/FP1/FP2 | still open | All Phase 1a/1c items. | +| P2/P4/P5/P6/P7 | still open | All Phase 3 perf items. | +| N1/N2/N3/N4/N5 | still open | All Phase 2 1.12 items. | +| 22 `function(...)` sites | still open | Phase 2; must be done before claiming 1.12 support. | +| 1.12-REGRESSION-1 | **newly open** | Pass-19 OnEvent closure is 5.0-unsafe. Highest single priority. | + +## 27.9 - Recommended next move + +1. **Fix 1.12-REGRESSION-1** (Pass-19 regression) — single-site fix at + `QuestieLearner.lua:3568`. Replace `function(_, event, arg1, arg2, ...)` with + the Lua 5.0+5.1-safe pattern. Add a `luac5.1 -p` verification and a busted + assertion that the file parses on a 5.0 parse simulator. +2. **Fix the `select` shim at `QuestieLoader.lua:48-58`** — apply the + `loadstring`-injection pattern that's already in the same file at lines + 5-10/15-20. Bootstrap-level parse error, blocks all of 1.12. +3. Land Phase 1a/1b/1c/1d/1e as five bisectable commits. Each is independent + and verifiable. None requires 1.12 or live testing beyond login/UI smoke. +4. Then either: + - run the Phase 0 probe and decide on Phase 2 scope, or + - skip Phase 2 and accept that 1.12 is broken until the user tests on + a real 1.12 client and confirms which features need shimming. + +The current 60-passing-test regression suite is the floor; every Phase 1 +commit should add at least one assertion that the bug is fixed. + +> **⚠ SUPERSEDED — see Pass-28 below.** Recommendation items 1 and 2 in this +> §27.9, the §27.8 ledger entry `1.12-REGRESSION-1`, and all of §27.5 are +> **incorrect**. They claim a Lua 5.0 *parse* error that does not exist. Do not +> action them as written. Pass-28 explains and corrects. + +--- + +# Pass-28 Addendum — Verification & Correction of "1.12-REGRESSION-1" (2026-06-04) + +**Trigger:** review request to verify the report, specifically the claim that +"the QuestieLearner fix broke 1.12 compat" (§27.5, `1.12-REGRESSION-1`). + +**Verdict: that claim is FALSE.** It is wrong about the source and wrong about +Lua 5.0, and it directly contradicts §8.1 of this same report (which is the +correct version). It must not be actioned as "highest single priority." + +## 28.1 — The source does not contain what §27.5 quotes + +§27.5 (lines ~4807, ~4876) quotes the handler as +`function(_, event, arg1, arg2, ...)` — with a trailing `...` — and calls that a +5.0 syntax error. The **actual** source (`QuestieLearner.lua:3568`) is: + +```lua +frame:SetScript("OnEvent", function(_, event, arg1, arg2, arg3, arg4, arg5, arg6, arg7, arg8, arg9, arg10) +``` + +That is **twelve named parameters and no `...`**. The `...` in §27.5's quote was +fabricated. A signature of named parameters is valid on *every* Lua version. The +file contains **no `...` vararg expression at all** (verified: every `...` hit in +the file is inside a comment). + +## 28.2 — Even a real trailing `...` in a signature is valid Lua 5.0 + +§27.5's reasoning — "`...` in the parameter list … 5.0 parses as a syntax error" — +is the exact fallacy this report already debunked in **§8.1 / §11**: + +> §8.1 (line ~2690): *"`function(...)` (a vararg **declaration**) is valid [Lua +> 5.0] … the developer knows the **signature** parses on 5.0."* + +In Lua 5.0, `function(a, b, ...)` is the *defining* syntax of a vararg function +(it creates the implicit `arg` table). What is 5.1+-only is the `...` **expression +used in a body** (`f(...)`, `{...}`, `select("#", ...)`). The OnEvent body uses no +such expression — it passes the *named locals* `arg1..arg10`. So the handler is +5.0-parse-safe. **Pass-19 introduced no parse/syntax regression.** + +## 28.3 — The same fallacy invalidates §27.9 recommendation #2 (the select shim) + +§27.9 #2 calls `QuestieLoader.lua:48-58` a "bootstrap-level parse error that +blocks all of 1.12." It is not. The shim is: + +```lua +if not select then + select = function(index, ...) -- valid 5.0 signature (builds `arg`) + if arg then + if index == "#" then return arg.n end -- "#" is a string, not the length op + index = tonumber(index) or 1 + return unpack(arg, index, arg.n) -- uses `arg`, not the `...` expression + end + end +end +``` + +Valid-5.0 signature, body reads the `arg` table, no `...` expression, no `#`/`%` +operator. **5.0-parse-safe.** Not a blocker. + +## 28.4 — `QuestieLearner.lua` *does* fail to parse on 5.0 — but for the real reason + +The report fingered the wrong cause. The genuine 5.0 parse blockers in this file +(confirmed by the user's in-game probes that `#` and `%` both throw on the +target) are the **operators**, not the signature: + +- `%` modulo: `QuestieLearner.lua:2436` (`low32 % 8388608`), `:2499` + (`math.floor(flags / NPC_FLAG_QUESTGIVER) % 2`). +- `#` length operator: lines 2068, 2211, 2382, 2460, 2464, 2478, 2482, 3521. + +These are the Phase-2 `#`(33-file) / `%`(11-file) work already catalogued in +§11.1 and §12. They are unrelated to the Pass-19 OnEvent change. + +## 28.5 — The one legitimate, much narrower concern (reframed correctly) + +There is a real *runtime* (not parse) question worth keeping: on a **pure** 1.12 +client, OnEvent args historically arrived via **globals** (`arg1` …), not as +script parameters. If the target delivers them as globals, the named params +`arg1..arg10` would be `nil`. **However:** + +1. This is a behavioral question, **not** a syntax/parse error, and **not** a + "the file won't load" regression. +2. `OnCombatLogEvent` already hedges it: the guarded legacy path at + `QuestieLearner.lua:2984` (`if not timestamp and arg1 then …`) reads the + **global** `arg1` as a fallback. +3. The pre-Pass-19 code (`function(_, event)`) also read the **parameter** + `event`, so it was never "pure-global 5.0-safe" either — the §27.5 + characterization of the old code is also inaccurate. +4. It can only be settled by an in-game test on the target, not by static + assertion. + +So: keep a **low-priority, verify-in-game** note that OnEvent arg delivery +(param vs. global) should be confirmed on the 1.12 target — but it is **not** a +parse regression and **not** the highest priority. + +## 28.6 — Corrected ledger / priority + +| ID | Old status (§27.8/27.9) | Corrected status | +|---|---|---| +| `1.12-REGRESSION-1` | "newly open … 5.0-unsafe … **highest single priority**" | **WITHDRAWN — false.** No parse error exists. Demote to a low-priority in-game check of OnEvent arg delivery (28.5). | +| §27.9 #1 "Fix the OnEvent closure" | top recommendation | **Drop.** No fix needed for parsing. | +| §27.9 #2 "select shim is a parse error" | "blocks all of 1.12" | **False (28.3).** Shim is 5.0-safe; leave it. | +| Real QuestieLearner 5.0 blockers | not identified | `%`@2436,2499 and `#`@(8 sites) — part of the existing Phase-2 `#`/`%` work. | + +## 28.7 — Scope note on the rest of the report + +Passes 12-27 were added by later sessions and introduced this error; it is the +same `function(...)`-signature misconception the original passes 6-7 made and +that §8.1 corrected. **Recommendation:** before acting on any Pass-12-27 "1.12 / +Lua 5.0" parse claim, sanity-check it against the §8.1/§11.1 rule — +*signatures (incl. `...`) parse on 5.0; only the `...` **expression**, and the +`#`/`%` **operators**, do not.* The verified-correct 5.0 parse surface remains: +`#` (33 files), `%` (11 files), `...` **expression** (~15 sites), Options +`{ ... }` (15 files). Everything else flagged as a "signature parse error" in the +later passes should be re-checked against that rule. + +--- + +# Pass-29 — Phase 1 IMPLEMENTED (2026-06-04) + +Phase 1 of the Implementation Plan (zero-risk correctness & hygiene, all targets) +is now **landed** on branch `questie-learner-comms-improvements`. Every change is +minimal, behavior-preserving, and verified. + +## What was changed + +| ID | File · edit | Risk | +|---|---|---| +| **B5** | `Questie-X.toc` — removed the duplicate `Modules\QuestieSlash.lua` (was listed at lines 23 & 182; now once). | none | +| **B2** | `QuestieOptionsTracker.lua` — 3× `fadeTickerValue:Cancel()` → `fadeTicker:Cancel()` (`:Cancel()` was being called on a number). | none (those branches were also unreachable) | +| **B6** | `Database/Corrections/{tbcQuestFixes,wotlkItemFixes,wotlkQuestFixes}.lua` — stripped the leading UTF-8 BOM (`EF BB BF`); files now start with `--`. | none | +| **B1** | `QuestieDB.lua:1355` — hoisted the double `GetQuest(questId)` into `local expectedQuest`; `IsComplete` now calls `GetQuest` once. | none | +| **N4** | `QuestieCommsData.lua:48-52` — `GetNPC/GetObject(...).name` now nil-guarded (`local npc = …; oName = (npc and npc.name) or oName`), matching the item branch. Prevents a crash on unknown ids in comms tooltips. | none | +| **B3** | `QuestieAnnounce.lua` — added `alreadySentBandaidCount`; the dedup cache now resets (`= {}`) after 1000 distinct messages, bounding the acknowledged unbounded-growth TODO. Reassignment (not `wipe()`) keeps it 5.0-safe. | none | +| **B7** | `QuestieFrame.lua:133` + `QuestieFramePool.lua:109-113` — removed the dead `BaseOnUpdate` wiring (it was always `nil`, so the glow ticker never ran). Preserved the actual effect (`SetScript("OnUpdate", nil)`); kept the live `GlowUpdate` wiring. | none (dead code; behavior identical) | +| **FP1** | `QuestieLib.lua:33` — `Ascension_IsScalingEnabled(questId)` now accepts the (unused) param, clearing the selene arity lint. | none (cosmetic) | +| **G1** | `QuestieNameplate.lua:88` — the in-loop `return` on a missing unit became a positive `if unitName and npcId then …` guard, so one bad nameplate no longer aborts updates for the rest. | low (loop-correctness) | +| **QQ** | `QuestieQuest.lua:221` (`ClearAllNotes`) — same fix: DB-missing quest now `if quest then …`-skipped instead of `return`-aborting the whole loop. | low (loop-correctness) | + +## Verification + +- **`luac5.1 -p` on all 12 changed files: parse-clean.** +- **`Tests/AuditFindings_spec.lua`: 41 / 41 pass.** Every Phase-1 snapshot + assertion was **inverted to a fixed-state regression guard** (e.g. `[B1]` now + asserts the single-call form; `[B6]` asserts no BOM; `[N4]` asserts the guarded + form; `[B2]` asserts zero `fadeTickerValue:Cancel()`). A new `[QQ-early-return]` + guard was added. +- **Full suite: 66 pass / 4 fail.** The 4 failures are the *pre-existing* + `QuestieArrowAssets_spec` manifest mismatch — untouched by Phase 1. + +## NOT changed / deferred (as planned) + +- Phase 1 deliberately avoids the fragile subsystems and the 1.12 parse work. +- **G1 caveat still open:** `QuestieNameplate:UpdateNameplate` still `strsplit`s the + GUID every update (perf, G1) and `strsplit` is unshimmed on 1.12 (N2) — both + belong to Phase 2/3, not Phase 1. + +## Updated ledger (supersedes §27.8 for these rows) + +| ID | Status | +|---|---| +| B1, B2, B3, B5, B6, B7, N4, FP1, G1, QQ-early-return | **implemented (Pass-29, Phase 1)** | +| N1, N2, N5 | implemented (Pass-30/31) | The loader now shims `bit` and `strsplit`, and the live quest/instance lookups use explicit unpacking. | +| `#`/`%`/`...`/`{}` operator work | open — **Phase 2 (now confirmed REQUIRED:** the user's in-game probes returned parse errors on both `4 % 3` and `#({1,2,3})`, so the target is genuinely Lua 5.0). | +| P2, P5, P7, PP1-PP5, G2 | open — Phase 3 (measured). | + +## Next actionable step + +Phase 2 (Lua 5.0 remediation) is still **required** — continue with the remaining +raw `#` / `%` parse blockers in TOC-loaded runtime files, starting with the +other load-bearing modules still using raw operators directly, and keep the +work ordered by parse risk. + +--- + +# Pass-30 - Phase 2 compatibility step: loader shims + serializer/stream cleanup (2026-06-04) + +This pass landed the first concrete Phase 2 compatibility slice and kept the +runtime path moving toward Lua 5.0 safety. + +## What changed + +- `Modules/Libs/QuestieLoader.lua` + - Added a pure-Lua `bit` fallback when the host does not expose one. + - Added a real `strsplit(separator, text, max)` fallback for legacy clients. + - The bit fallback exposes `band`, `bor`, `bxor`, `bnot`, `lshift`, and + `rshift`, which covers the core runtime call sites. +- `Modules/Libs/QuestieSerializer.lua` + - Replaced raw `%` operators with `math.mod(...)` calls at the float packing + and unpacking sites. +- `Modules/QuestieStream.lua` + - Replaced raw `%` operators with `math.mod(...)` in byte packing and chunk + indexing. + - Replaced raw `#self._bin` with `table.getn(self._bin)`. +- `Tests/AuditFindings_spec.lua` + - Added a regression guard that checks the loader shim exists and that the + serializer/stream files no longer use the raw operator forms at the touched + sites. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua Tests\\QuestieLearner_performance_spec.lua` + - `55 successes / 0 failures / 0 errors / 0 pending` +- `selene Modules\\Libs\\QuestieLoader.lua Modules\\Libs\\QuestieSerializer.lua Modules\\QuestieStream.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 6 warnings / 0 parse errors` + - The warnings are pre-existing style warnings plus the loader's deliberate + global `bit` assignment. + +## Result + +The Phase 2 compatibility path now has the runtime shims needed for `bit` and +`strsplit`, and the most load-bearing serializer/stream code is no longer using +the raw operators that break strict Lua 5.0 parsing. + +## Remaining Phase 2 work + +- The broader raw `#` / `%` sweep is still open in other TOC-loaded runtime + files. +- The next best follow-up is to keep walking the remaining parser-sensitive + files in priority order, starting with the load-bearing runtime modules that + still use raw operators directly. + +--- + +# Pass-31 - Phase 2 sweep: explicit unpack for quest log and instance lookups (2026-06-04) + +This pass continued the Phase 2 compatibility sweep by replacing the remaining +live `select(8, ...)` lookups in core quest/instance helpers with explicit +unpacking. + +## What changed + +- `Modules/QuestiePlayer.lua` + - Replaced `select(8, GetInstanceInfo())` with `local _, _, _, _, _, _, _, instanceMapID = GetInstanceInfo()`. + - Both `GetCurrentZoneId()` and `GetCurrentUiMapId()` now use the unpacked + `instanceMapID` directly. +- `Modules/Quest/QuestEventHandler.lua` + - Replaced `select(8, GetQuestLogTitle(questLogIndex))` with explicit unpacking + into `questLogQuestId`. +- `Modules/Tracker/QuestieTracker.lua` + - Replaced both `select(8, GetQuestLogTitle(...))` sites with explicit unpacking + into `questId`. +- `Tests/AuditFindings_spec.lua` + - Updated `[N5]` to assert the explicit-unpack form is present and the old + `select(8, ...)` form is absent in the live files. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua Tests\\QuestieLearner_performance_spec.lua` + - `51 successes / 0 failures / 0 errors / 0 pending` +- `selene Modules\\QuestiePlayer.lua Modules\\Quest\\QuestEventHandler.lua Modules\\Tracker\\QuestieTracker.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 67 warnings / 0 parse errors` + - The warnings are existing style noise in these large runtime files. + +## Result + +The old `select(8, ...)` compatibility pattern has been removed from the live +quest/instance lookup paths that the audit flagged. The only remaining `select` +mention in the live grep is now a comment in `QuestieLearner.lua`, not a runtime +call site. + +## Remaining Phase 2 work + +- `N3` raw `...` expression / `{ ... }` option-table sweep is still open. +- Any remaining raw `#` / `%` sites in TOC-loaded runtime files still need the + per-file Phase 2 sweep and `luac5.1 -p` verification. + +--- + +# Pass-32 - Phase 2 sweep: options bootstrap cleanup + l10n no-arg fast path (2026-06-04) + +This pass closed the remaining `N3` parser-safety row and started the next +audit item, `P2`, by removing the common no-argument allocation from +`Localization/l10n.lua`. + +## What changed + +- `Modules/Options/QuestieOptions.lua` + - Replaced `QuestieOptions.tabs = { ... }` with `QuestieOptions.tabs = {}`. +- `Modules/Options/ArrowTab/QuestieOptionsArrow.lua` + - Replaced `QuestieOptions.tabs.arrow = { ... }` with `QuestieOptions.tabs.arrow = {}`. +- `Modules/Options/GeneralTab/QuestieOptionsGeneral.lua` + - Replaced `QuestieOptions.tabs.general = { ... }` with `QuestieOptions.tabs.general = {}`. +- `Modules/Options/KeybindsTab/QuestieOptionsKeybinds.lua` + - Replaced `QuestieOptions.tabs.keybinds = { ... }` with `QuestieOptions.tabs.keybinds = {}`. +- `Modules/Options/TrackerTab/QuestieOptionsTracker.lua` + - Replaced `QuestieOptions.tabs.tracker = { ... }` with `QuestieOptions.tabs.tracker = {}`. +- `Localization/l10n.lua` + - Added a no-argument fast path before the vararg table allocation in `_l10n:translate`. + - The common literal translation case now returns immediately without building `local args = {...}`. + - The formatted-string path now uses a bounded `select()` formatter for the common small-argument cases, with a table fallback only for unusually long arg lists. +- `Tests/AuditFindings_spec.lua` + - Updated `[N3]` to assert the empty-table form exists in all five options bootstrap files and that the old `{ ... }` form is absent. + - Added a regression check for the `l10n()` no-arg fast path. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `39 successes / 0 failures / 0 errors / 0 pending` +- `selene Modules\\Options\\QuestieOptions.lua Modules\\Options\\ArrowTab\\QuestieOptionsArrow.lua Modules\\Options\\GeneralTab\\QuestieOptionsGeneral.lua Modules\\Options\\KeybindsTab\\QuestieOptionsKeybinds.lua Modules\\Options\\TrackerTab\\QuestieOptionsTracker.lua Localization\\l10n.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 20 warnings / 0 parse errors` +- `luac5.1 -p Modules\\Options\\QuestieOptions.lua; ...; luac5.1 -p Localization\\l10n.lua` + - Parser sanity check passed for all touched options bootstrap files and `Localization/l10n.lua`. + +## Result + +`N3` is now closed in the live codebase, and the next measurable performance item +is the remaining `l10n()` allocation work on formatted-string calls plus the +other TOC-loaded runtime hot spots that still use parser-sensitive constructs. + +## Remaining Phase 2 work + +- `P2` l10n per-call alloc is now partially reduced by the no-arg fast path, but + formatted translations still allocate `local args = {...}`. +- Continue the broader raw `#` / `%` sweep in the remaining TOC-loaded runtime + files and keep verifying each touched file with `luac5.1 -p`. + +--- + +# Pass-33 - Phase 2 sweep: runtime modulo cleanup (2026-06-04) + +This pass removed the remaining raw arithmetic modulo operators from the core +Lua 5.0-sensitive runtime files that the audit still had open. + +## What changed + +- `Database/QuestieDB.lua` + - Replaced the daily/weekly quest flag checks from raw `%` to `math.mod(...)`. +- `Modules/QuestiePlayer.lua` + - Replaced the required race/class bit-flag checks from raw `%` to `math.mod(...)`. +- `Modules/Libs/MessageHandler.lua` + - Replaced the async callback yield gate from `callbackIndex % asyncCount` to `math.mod(callbackIndex, asyncCount)`. +- `Modules/Libs/QuestieLib.lua` + - Replaced the pseudo-random number generator modulo operations with `math.mod(...)`. +- `Database/Corrections/QuestieEvent.lua` + - Replaced the Darkmoon Faire schedule modulo checks with `math.mod(...)`. +- `Tests/AuditFindings_spec.lua` + - Added a regression block that asserts the compatibility shims and the converted modulo sites are present. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `40 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Database\\QuestieDB.lua; luac5.1 -p Modules\\QuestiePlayer.lua; luac5.1 -p Modules\\Libs\\MessageHandler.lua; luac5.1 -p Modules\\Libs\\QuestieLib.lua; luac5.1 -p Database\\Corrections\\QuestieEvent.lua` + - Parser sanity check passed for all touched runtime files. +- `selene Database\\QuestieDB.lua Modules\\QuestiePlayer.lua Modules\\Libs\\MessageHandler.lua Modules\\Libs\\QuestieLib.lua Database\\Corrections\\QuestieEvent.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 98 warnings / 0 parse errors` + +## Result + +The core runtime modulo operators that were still blocking the Lua 5.0 sweep +have been converted to `math.mod(...)`, so the phase-2 parser-safety pass is +now materially smaller and the remaining work is down to the other open audit +items rather than the core `%` operators. + +## Remaining Phase 2 work + +- `P2` l10n per-call alloc remains partially open for formatted calls. +- Continue the remaining raw `#` / `%` sweep only where the audit still + identifies untouched runtime sites. + +--- + +# Pass-34 - Phase 3 measured step: l10n translation cache (2026-06-04) + +This pass extended the Phase 3 `P2` hot-path work by caching repeated literal +translation lookups so the common no-argument case no longer pays the lookup +chain after the first hit. + +## What changed + +- `Localization/l10n.lua` + - Added `l10n.translationCache` for locale-scoped resolved strings. + - Added `ResetTranslationCache()` and called it from `InitializeLocaleOverride` + and `SetUILocale` so cached values do not leak across locale changes. + - The no-arg translation fast path now checks the locale cache before walking + the translation table and stores the resolved string for future calls. +- `Tests/AuditFindings_spec.lua` + - Expanded `[P2]` to assert the locale cache and cache-reset helpers exist. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `40 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Localization\\l10n.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Localization\\l10n.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 16 warnings / 0 parse errors` + +## Result + +The hottest remaining measured low-risk performance item now has both a fast +path and a cache for repeated literal lookups. The common formatted-string path +now avoids `local args = {...}`, with only rare long arg lists falling back to +a packed table. + +## Remaining Phase 3 work + +- Continue the remaining measured hot-path items in the audit, with `P5` + tooltip party-map caching and `P7` debounce work still standing out. +- Keep the branch-per-phase rule in place for any subsequent phase transitions. + +--- + +# Pass-35 - Phase 3 measured step: tooltip party-member cache (2026-06-04) + +This pass removed the repeated 1-40 party-unit scan from the tooltip helper by +adding a cached party-member lookup table that is invalidated when the roster +changes. + +## What changed + +- `Modules/QuestiePlayer.lua` + - Added `QuestiePlayer.partyMemberCache` and `InvalidatePartyMemberCache()`. + - Added `BuildPartyMemberCache()` to build a name-keyed lookup table from the + current party members. + - `GetPartyMemberByName()` now returns from the cache instead of scanning all + party units on every call. +- `Modules/QuestieEventHandler.lua` + - `GroupRosterUpdate()` and `GroupLeft()` now invalidate the party-member + cache when the roster changes. +- `Tests/AuditFindings_spec.lua` + - Added a regression check for the cache fields, the build helper, and the + roster-based invalidation path. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `41 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Modules\\QuestiePlayer.lua; luac5.1 -p Modules\\QuestieEventHandler.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Modules\\QuestiePlayer.lua Modules\\QuestieEventHandler.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 11 warnings / 0 parse errors` + +## Result + +The tooltip party lookup path now has a cache rather than a per-call 1-40 scan, +which is the lowest-risk measured improvement for the current Phase 3 list. + +## Remaining Phase 3 work + +- `P9` learner batch injection and the remaining measured items (`PP1-PP5`, + `G2`) are the next open Phase 3 targets after the rescan work. +- Continue the phase-3 items in audit order, but keep the branch-per-phase rule + intact for any future phase boundaries. + +--- + +# Pass-37 - Phase 3 measured step: available quest batch draw cleanup (2026-06-04) + +This pass removed the per-quest coroutine spawn from the available-quest draw +helper so the batch sweep uses the existing batch thread instead of creating a +new coroutine for every quest. + +## What changed + +- `Modules/Quest/AvailableQuests.lua` + - `_DrawAvailableQuest()` now executes inline inside the existing sweep thread. + - Removed the `ThreadLib.ThreadSimple` per-quest wrapper that was spawning one + coroutine for each available quest draw. + - Added a debounced `CalculateAndDrawAllDebounced()` helper to coalesce event + bursts separately from the immediate `CalculateAndDrawAll()` entrypoint. +- `Tests/AuditFindings_spec.lua` + - Added a regression that asserts the per-quest thread wrapper is gone and the + debounced helper is present. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `43 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Modules\\Quest\\AvailableQuests.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Modules\\Quest\\AvailableQuests.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 18 warnings / 0 parse errors` + +## Result + +The available-quest batch sweep now stays inside the batch thread instead of +spawning a coroutine per quest, which reduces the thread churn called out in +the audit's `P7` finding. + +## Remaining Phase 3 work + +- `P7` still has some non-options event-driven callers to audit, but the batch + draw path itself is now much cheaper. +- Continue the phase-3 items in audit order, with the remaining measured items + still open. + +--- + +# Pass-36 - Phase 3 measured step: options rescan debounce (2026-06-04) + +This pass coalesced the remaining Options-tab quest rescans through the existing +debounce helper so repeated setting changes no longer trigger multiple immediate +full redraws. + +## What changed + +- `Modules/Options/GeneralTab/QuestieOptionsGeneral.lua` + - Routed the `ascensionScaling` and `lowLevelStyle` setters through + `QuestieOptionsUtils:Delay(0.3, AvailableQuests.CalculateAndDrawAll, ...)` + instead of calling `AvailableQuests.CalculateAndDrawAll()` immediately. + - The existing range sliders already used the same debounce helper, so this + pass makes the whole options block consistent. +- `Tests/AuditFindings_spec.lua` + - Added a regression check that the direct immediate `AvailableQuests.CalculateAndDrawAll()` + calls are absent from the options file and that the debounced helper calls + are present for all five settings. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `42 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Modules\\Options\\GeneralTab\\QuestieOptionsGeneral.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Modules\\Options\\GeneralTab\\QuestieOptionsGeneral.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 15 warnings / 0 parse errors` + +## Result + +The most burst-prone options rescans are now all funneled through the same +debounce helper, which reduces back-to-back redraw work when a user drags or +clicks several related settings quickly. + +## Remaining Phase 3 work + +- `P7` still has non-options event-driven rescan call sites elsewhere in the + codebase, so the measured gating work is not finished yet. +- Continue Phase 3 in audit order, with `P7` and then the other measured items + still remaining. + +--- + +# Pass-38 - Phase 3 measured step: l10n formatter allocation cleanup (2026-06-04) + +This pass removed the common formatted-call vararg table from `Localization/l10n.lua` +so small-arity `l10n("...", ...)` calls can format without first building a +temporary `{...}` array. + +## What changed + +- `Localization/l10n.lua` + - Added a bounded `FormatLocalizedString(template, argCount, ...)` helper. + - The common 1-8 argument cases now format directly through `select()` + instead of packing `...` into a table first. + - Rare long arg lists still fall back to a packed table for safety. + - Replaced `entry[#entry+1] = id` with `table.insert(entry, id)` in the + object-name lookup builder so the file no longer depends on raw `#`. +- `Tests/AuditFindings_spec.lua` + - Updated the `P2` regression to assert the new bounded formatter exists and + the old `local args = {...}` allocation path is gone. + - Updated the Lua 5.0 surface snapshot so `Localization/l10n.lua` is no + longer reported as a raw `#` blocker. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `43 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Localization\\l10n.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Localization\\l10n.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 16 warnings / 0 parse errors` + +## Result + +The common formatted translation path now avoids a fresh vararg table on +every call, which trims one of the remaining hot allocation sites from `P2` +without changing the external `l10n(...)` API. + +## Remaining Phase 3 work + +- `PP2`, `PP5`, `PP6`, and `G2` remain the next open measured audit items in + the current ledger. +- Continue the phase-3 items in audit order, with the remaining measured items + still open. + +--- + +# Pass-43 - Phase 3 measured step: validate-cache closure cleanup (2026-06-04) + +This pass removed the per-quest anonymous closure from +`QuestieValidateGameCache` and routed the validation through a named helper +instead. + +## What changed + +- `Modules/QuestieValidateGameCache.lua` + - Added `ValidateQuestLogEntry(...)` as a named helper for the quest-log + validation logic. + - `OnQuestLogUpdate()` now calls `pcall(ValidateQuestLogEntry, ...)` instead + of allocating a fresh anonymous closure inside the quest loop. + - `isQuestLogGood` is now actively set false on validation failures instead + of remaining a dead local. +- `Tests/AuditFindings_spec.lua` + - Added a regression that asserts the named helper exists and the old + `pcall(function() ...)` pattern is gone from the validation loop. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `46 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Modules\\QuestieValidateGameCache.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Modules\\QuestieValidateGameCache.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 6 warnings / 0 parse errors` + +## Result + +`G2` is now implemented, leaving no remaining measured items in the current +phase 3 ledger. + +--- + +# Pass-42 - Phase 3 measured step: NPC name fallback cache (2026-06-04) + +This pass replaced the rare full-NPC-table name fallback scan in +`QuestieQuestPrivates` with a cached lowercase name index. + +## What changed + +- `Modules/Quest/QuestieQuestPrivates.lua` + - Added a private `npcNameLookup` cache and a `BuildNpcNameLookup()` helper. + - The `killcredit` fallback now resolves `targetName` through the cached + lowercase lookup instead of scanning every `npcData` entry on each miss. + - The first-found behavior is preserved by only filling the index when a name + has not already been seen. +- `Tests/AuditFindings_spec.lua` + - Added a regression that asserts the cached lowercase name lookup exists and + that the old `pairs(npcData)` fallback scan is gone from the killcredit path. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `45 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Modules\\Quest\\QuestieQuestPrivates.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Modules\\Quest\\QuestieQuestPrivates.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 10 warnings / 0 parse errors` + +## Result + +`PP6` is now implemented by caching the lowercase NPC name lookup used by the +fallback path, leaving `G2` as the remaining measured cleanup item. + +--- + +# Pass-42 - Phase 3 measured step: NPC name fallback cache (2026-06-04) + +This pass replaced the rare full-NPC-table name fallback scan in +`QuestieQuestPrivates` with a cached lowercase name index. + +## What changed + +- `Modules/Quest/QuestieQuestPrivates.lua` + - Added a private `npcNameLookup` cache and a `BuildNpcNameLookup()` helper. + - The `killcredit` fallback now resolves `targetName` through the cached + lowercase lookup instead of scanning every `npcData` entry on each miss. + - The first-found behavior is preserved by only filling the index when a name + has not already been seen. +- `Tests/AuditFindings_spec.lua` + - Added a regression that asserts the cached lowercase name lookup exists and + that the old `pairs(npcData)` fallback scan is gone from the killcredit path. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `45 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Modules\\Quest\\QuestieQuestPrivates.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Modules\\Quest\\QuestieQuestPrivates.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 10 warnings / 0 parse errors` + +## Result + +`PP6` is now implemented by caching the lowercase NPC name lookup used by the +fallback path, leaving `G2` as the remaining measured cleanup item. + +--- + +# Pass-41 - Phase 3 measured step: GetTime hoist cleanup (2026-06-04) + +This pass hoisted repeated `GetTime()` checks into local `now` variables in the +three hot functions the audit called out for `PP5`. + +## What changed + +- `Modules/Tooltips/MapIconTooltip.lua` + - `MapIconTooltip:Show()` now captures `GetTime()` once into `now`. + - The tooltip throttle check and timestamp update both use the same local. +- `Modules/Network/QuestieLearnerComms.lua` + - `IsSenderTrusted()` now samples `GetTime()` once into `now`. + - The mute-expiry check and last-message timestamp update reuse that value. +- `Modules/QuestieInit.lua` + - `QuestieInit.Stages[3]` now keeps a local `now` cursor for the plugin wait + loop instead of calling `GetTime()` repeatedly in the loop condition. +- `Tests/AuditFindings_spec.lua` + - Added a regression that inspects the three function bodies and asserts the + local `now` hoists are present. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `43 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Modules\\Tooltips\\MapIconTooltip.lua; luac5.1 -p Modules\\Network\\QuestieLearnerComms.lua; luac5.1 -p Modules\\QuestieInit.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Modules\\Tooltips\\MapIconTooltip.lua Modules\\Network\\QuestieLearnerComms.lua Modules\\QuestieInit.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 37 warnings / 0 parse errors` + +## Result + +`PP5` is now implemented in the three hot paths listed by the audit, leaving +`PP6` and `G2` as the remaining measured items in the current ledger. + +--- + +# Pass-40 - Phase 3 measured step: profile alias cache cleanup (2026-06-04) + +This pass cached `Questie.db.profile` in the hottest tooltip/map paths so the +code stops re-walking the `Questie -> db -> profile` chain on every repeated +toggle check. + +## What changed + +- `Modules/Tooltips/MapIconTooltip.lua` + - Added a local `profile` alias at the top of `MapIconTooltip:Show()`. + - The rebuild path now reads tooltip toggles and XP toggles from that local. +- `Modules/Map/QuestieMap.lua` + - Added local `profile` aliases in the hot icon visibility and draw paths. + - Replaced the repeated direct `Questie.db.profile.*` chains in the manual + icon and world-icon code with the local alias. +- `Modules/Tooltips/Tooltip.lua` + - Added local `profile` aliases in the group-member fetch path and the main + tooltip builder so repeated toggle checks reuse the same table reference. +- `Tests/AuditFindings_spec.lua` + - Updated the `PP2` regression to assert the local profile aliases exist in + the three hot files and the direct `Questie.db.profile.*` chains are gone + from the target paths. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `43 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Modules\\Tooltips\\MapIconTooltip.lua; luac5.1 -p Modules\\Map\\QuestieMap.lua; luac5.1 -p Modules\\Tooltips\\Tooltip.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Modules\\Tooltips\\MapIconTooltip.lua Modules\\Map\\QuestieMap.lua Modules\\Tooltips\\Tooltip.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 37 warnings / 0 parse errors` + +## Result + +`PP2` is now implemented in the hot tooltip/map render paths without changing +the visible UI behavior, and the remaining open measured items are now `PP5`, +`PP6`, and `G2`. + +--- + +# Pass-39 - Phase 3 measured step: batch quest eligibility query (2026-06-04) + +This pass removed the repeated single-field eligibility reads from +`QuestieDB.IsDoable` and replaced them with one batch `QueryQuest(...)` read. + +## What changed + +- `Database/QuestieDB.lua` + - Added a fixed `IS_DOABLE_QUERY_ORDER` for the hot eligibility fields. + - `QuestieDB.IsDoable` now reads the required fields once through + `QuestieDB.QueryQuest(questId, IS_DOABLE_QUERY_ORDER)`. + - The existing logic still branches on the same rules and preserves the + auto-blacklist side effects. +- `Tests/AuditFindings_spec.lua` + - Added a regression that inspects the `IsDoable` function body and asserts + the batch read is present while the repeated single-field reads are absent. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua` + - `43 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Database\\QuestieDB.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Database\\QuestieDB.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 54 warnings / 0 parse errors` + +## Result + +`QuestieDB.IsDoable` now does one batch read for its core eligibility fields +instead of a dozen single-field reads per quest, which is the exact low-risk +mechanism the audit recommended for `PP1`. + +--- + +# Pass-49 - Cross-branch learner/comms review and event-order fix (2026-06-05) + +This pass reviewed the current Questie-X branch topology, not only the learner +branch. The main finding is that the best performance work is still split +across branches: + +- `questie-learner-comms-improvements` contains the learner/comms/arrow + throttling controls plus the latest learner debounce default alignment. +- `phase3-measured-perf` contains the measured hot-path improvements for + localization, available quest redraws, quest eligibility, profile lookups, + `GetTime()` hoists, NPC fallback lookups, and validate-cache allocation + cleanup. +- `phase2-lua50-sweep` and local `phase3-measured-perf` still contain the + `fix: add missing local profile in QuestieMap.ProcessQueue` commit even + though `main` reverted it. That commit should be removed from the integration + path or revalidated with a specific runtime trace before merge. + +## Bug found + +`QuestieLearner:OnCombatLogEvent` had an event-order edge case introduced by +the bystander-kill suppression work: + +- `UNIT_DIED` may fire for a nearby visible death before `PARTY_KILL`. +- The previous debounce table only stored `dstGUID -> timestamp`. +- If `UNIT_DIED` stored the timestamp first, the later authoritative + `PARTY_KILL` for the player's/group's actual kill could be skipped inside the + five-second duplicate window. +- That could preserve the FPS-saving bystander suppression while accidentally + dropping legitimate learner updates. + +## What changed + +- `Modules/QuestieLearner.lua` + - The kill debounce entry now stores both timestamp and event type. + - A later `PARTY_KILL` is allowed through if the prior debounce entry came + from `UNIT_DIED`. + - Duplicate `PARTY_KILL` events and non-authoritative duplicate events remain + suppressed. + - The prune loop remains bounded and still handles older timestamp-only + entries defensively. +- `Tests/AuditFindings_spec.lua` + - Added a static regression asserting the event-type-aware debounce guard is + present, so future refactors do not accidentally reintroduce the issue. + +## Verification + +- `busted Tests\\AuditFindings_spec.lua Tests\\QuestieDB_suppression_spec.lua Tests\\QuestieArrow_spec.lua` + - `49 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Modules\\QuestieLearner.lua; luac5.1 -p Tests\\AuditFindings_spec.lua` + - Parser sanity check passed for the touched files. +- `selene Modules\\QuestieLearner.lua Tests\\AuditFindings_spec.lua` + - `0 errors / 99 warnings / 0 parse errors` + - The warnings are existing style warnings in `QuestieLearner.lua`, mostly + multiple-statements-per-line and shadowing warnings, not parser failures. + +## Remaining branch-level decision + +The most robust release candidate should be an integration branch that combines +`questie-learner-comms-improvements` with `phase3-measured-perf`, after removing +or revalidating the reverted `QuestieMap.ProcessQueue` profile-local commit. +Until that happens, no single branch contains every current performance fix. + +--- + +# Pass-50 - Documentation refresh for performance refactor status (2026-06-05) + +This pass updated the user-facing and handoff documentation to reflect the +current learner/comms/arrow performance work and the cross-branch integration +status. + +## What changed + +- `README.md` + - Expanded the repository notice to explain the active performance refactor. + - Added a "Current Performance Refactor Status" section with the status of + QuestieLearner, QuestieComms, Arrow, error suppression, and phase 3 hot-path + work. + - Added a concise in-game validation checklist for heavy kill zones, minimap + open, nearby-player kills, comms modes, and Arrow throttles. +- `CHANGELOG.md` + - Added an `[Unreleased] - Performance Refactor Branches` section. + - Documented learner debounce/max-wait, bystander `UNIT_DIED` suppression, + `PARTY_KILL` event-order fix, live learner/comms/arrow performance options, + non-fatal error suppression, tooltip precedence, and phase 3 measured + hot-path changes. + - Noted that the best performance work is still split across + `questie-learner-comms-improvements` and `phase3-measured-perf`. +- `release_notes.txt` + - Added an active refactor notice above the v1.6.3 shipped highlights. +- `QUESTIE-LEARNER-HANDOFF.md` + - Added a current learner/comms performance handoff at the top. + - Preserved the older Sunstrider handoff below as historical context. +- `docs/index.html` + - Added an "Active Performance Refactor" landing-page section. +- `docs/changelog.html` + - Added an `[Unreleased]` HTML changelog section mirroring the markdown + changelog. + +## Result + +The docs now clearly distinguish shipped `v1.6.3` behavior from the active +performance refactor branches, and they call out the remaining integration and +testing requirements for the next stable release. + +--- + +# Pass-51 - Invalid item ID safety for item-name caching (2026-06-05) + +This pass fixed the crash reported from `QuestieDB.lua:335` when item-name +lookup paths received an invalid item ID during queued item caching. + +## What changed + +- `Database/QuestieDB.lua` + - The 3.3.5 compatibility `Item:CreateFromItemID()` shim now routes through a + safe helper that validates the item ID before calling `GetItemInfo()`. + - Invalid or zero IDs now return an `item:` placeholder instead of + crashing the API call. +- `Modules/Libs/QuestieLib.lua` + - `CacheItemNames()` now normalizes `objectiveDB.Id` with `tonumber()`. + - Invalid item IDs are skipped early with a debug message instead of + creating an item object that can later crash on `GetItemName()`. + - Valid numeric IDs continue to use the existing async item-name cache flow. +- `Modules/Network/QuestieCommsData.lua` + - The item tooltip branch now normalizes `objective.id` before calling + `QuestieDB:GetItem()` or `GetItemInfo()`. + - Non-numeric item IDs now degrade to a safe placeholder instead of + entering the same failure mode during comms tooltip building. +- `Tests/QuestieItemNameSafety_spec.lua` + - Added a regression spec proving invalid item IDs do not call `GetItemInfo` + and instead return a placeholder item name. + +## Verification + +- `busted Tests\\QuestieItemNameSafety_spec.lua Tests\\QuestieLearner_performance_spec.lua Tests\\QuestieTooltip_precedence_spec.lua` + - `18 successes / 0 failures / 0 errors / 0 pending` +- `luac5.1 -p Database\\QuestieDB.lua; luac5.1 -p Modules\\Libs\\QuestieLib.lua; luac5.1 -p Modules\\Network\\QuestieCommsData.lua` + - Parser sanity checks passed for the touched files. + +## Result + +Malformed item IDs can no longer crash the item-name caching path, and the fix +stays aligned with the active learner branch because it preserves normal +behavior for valid item IDs while safely degrading the invalid cases. + +--- + +# Pass-51 - Learner/static source mode switch for display testing (2026-06-05) + +This pass added a Database-tab mode switch so learner capture can stay on while +display decisions can be forced to learner-only, static-only, or neither for +controlled testing. + +## What changed + +- `Modules/Options/DatabaseTab/QuestieOptionsDatabase.lua` + - Added `Enable Learner Recording` to control the learner capture/injection + toggle independently. + - Added `Data Source Mode` with `auto`, `learner`, `static`, and `none` + choices. + - Changing either control now immediately reapplies the active mode and + refreshes quest/tracker state. +- `Modules/QuestieLearner.lua` + - Added `GetDataSourceMode`, `IsLearnerLiveEnabled`, and `ApplyDataSourceMode`. + - Captured a static override snapshot so mode changes can rebuild the active + view without a reload. + - Gated live injection, cross-links, tooltip hooks, and pin refreshes on the + selected mode. + - Added cache resets so changing the mode does not leave stale query results. +- `Database/QuestieDB.lua` + - Static pin suppression now respects the selected data source mode instead + of only the old `prioritizeMyData` flag. +- `Modules/Quest/QuestieQuest.lua` + - Static spawn suppression now only engages when learner data is allowed to + influence display. +- `Modules/Quest/QuestieQuestPrivates.lua` + - NPC/object spawn selection now respects learner-only / static-only / none + modes for map pin behavior. +- `Modules/Tooltips/Tooltip.lua` + - Learned tooltip fallback is disabled when the selected mode is static-only + or none. +- `Modules/Options/QuestieOptionsDefaults.lua` + - Added the default `dataSourceMode = "auto"` profile setting. +- `Tests/QuestieLearnerDataSourceMode_spec.lua` + - Added a regression spec covering the new mode selector and live gating. + +## Verification + +- `luac5.1 -p Modules\\QuestieLearner.lua` +- `luac5.1 -p Database\\QuestieDB.lua` +- `luac5.1 -p Modules\\Quest\\QuestieQuest.lua` +- `luac5.1 -p Modules\\Quest\\QuestieQuestPrivates.lua` +- `luac5.1 -p Modules\\Tooltips\\Tooltip.lua` +- `luac5.1 -p Modules\\Options\\DatabaseTab\\QuestieOptionsDatabase.lua` +- `luac5.1 -p Modules\\Options\\QuestieOptionsDefaults.lua` +- `luac5.1 -p Tests\\QuestieLearnerDataSourceMode_spec.lua` +- `busted Tests\\QuestieLearnerDataSourceMode_spec.lua` +- `busted Tests\\AuditFindings_spec.lua Tests\\QuestieItemNameSafety_spec.lua` + +## Result + +Learner evidence still records in the saved learner store, but you can now +force learner-only, static-only, or neither at the display layer to compare the +server's native data against the learner path without contaminating the other +side of the test. + +--- + +# Pass-52 - Learner self-sustaining DB fallback (2026-06-05) + +This pass made learner mode read from the saved learner tables directly when +the compiled/static query path is unavailable, so learner-only is now a true +source path instead of only a live override layer. + +## What changed + +- `Database/QuestieDB.lua` + - `GetQuest`, `GetNPC`, `GetObject`, and `GetItem` now prefer + `Questie.dbLearner.global.*` records when learner mode is active or when + the compiled/static source is missing. + - Auto mode still prefers compiled/static data first, but it can now fall + back to learner data when a record exists and the static query returns nil. +- `Modules/Quest/QuestieQuestPrivates.lua` + - Pin builders now read NPC/object display data through `QuestieDB:GetNPC()` + and `QuestieDB:GetObject()` instead of static-only single-field queries. + - That keeps the pin builders aligned with the learner-aware DB getters and + prevents learner-only mode from quietly depending on the static query path. +- `Tests/QuestieLearnerDataSourceMode_spec.lua` + - Added runtime coverage proving learner-mode NPC/object lookups resolve from + learner saved variables when the compiled query returns nothing. + +## Verification + +- `luac5.1 -p Database\\QuestieDB.lua` +- `luac5.1 -p Modules\\Quest\\QuestieQuestPrivates.lua` +- `luac5.1 -p Tests\\QuestieLearnerDataSourceMode_spec.lua` +- `busted Tests\\QuestieLearnerDataSourceMode_spec.lua Tests\\QuestieLearner_performance_spec.lua` + +## Result + +Learner-only mode is now self-sustaining for the DB read path used by pin +builders: if a record exists in the learner SavedVariables, Questie can read it +directly without needing the compiled/static database to supply the same data +first. + +--- + +# Pass-53 - Immediate learner pin rendering and available quest guard (2026-06-05) + +This pass removed the remaining gap between kill evidence and visible learner +pins, then hardened the available quest scanner so unresolved quest IDs no +longer crash redraw threads or spam the log repeatedly. + +## What changed + +- `Database/QuestieDB.lua` + - Learner-mode NPC reads now convert immediate GUID-kill evidence from + `dbLearner.global.npcs[npcId][8]` into a real `spawns` table. + - That lets pin builders consume learner kill coordinates immediately instead + of waiting for the later confidence merge path. +- `Modules/Quest/QuestieQuestPrivates.lua` + - Learner mode no longer clears out the spawn table after `GetNPC()` or + `GetObject()` has already supplied learner-backed coordinates. + - Objective pin building now keeps those learned spawns available to the map + layer instead of collapsing them back to empty. +- `Modules/Quest/AvailableQuests.lua` + - Guarded the draw thread so unresolved quest IDs are skipped safely instead + of indexing a nil quest record. + - Dedupe the skip message so the same unavailable quest only logs once per + session instead of on every redraw. +- `Tests/QuestieLearnerDataSourceMode_spec.lua` + - Added a regression proving learner kill evidence becomes spawn coordinates + immediately in learner mode. +- `Tests/QuestieAvailableQuests_spec.lua` + - Added a regression proving unavailable quests are skipped before the draw + path touches `tagInfoWasCached`. + +## Verification + +- `luac5.1 -p Database\\QuestieDB.lua` +- `luac5.1 -p Modules\\Quest\\QuestieQuestPrivates.lua` +- `luac5.1 -p Modules\\Quest\\AvailableQuests.lua` +- `busted Tests\\QuestieLearnerDataSourceMode_spec.lua Tests\\QuestieLearner_performance_spec.lua Tests\\QuestieAvailableQuests_spec.lua` + +## Result + +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.