From b1abc54bfcc9f634f5a4915a20d5a0e54c8dd1d4 Mon Sep 17 00:00:00 2001 From: Lasse Nielsen Date: Thu, 3 Sep 2026 15:08:12 +0200 Subject: [PATCH 1/2] feat(rollframe): confirm greed and pass selections --- DragonLoot/Core/Config.lua | 9 +- DragonLoot/Display/RollFrame.lua | 10 +- DragonLoot/Display/RollManager.lua | 112 ++++++++++++ DragonLoot/Locales/enUS.lua | 5 + DragonLoot_Options/Tabs/LootRollTab.lua | 15 ++ spec/Config_spec.lua | 34 +++- spec/RollSelectionConfirmation_spec.lua | 217 ++++++++++++++++++++++++ spec/wow_mock.lua | 32 ++++ 8 files changed, 417 insertions(+), 17 deletions(-) create mode 100644 spec/RollSelectionConfirmation_spec.lua diff --git a/DragonLoot/Core/Config.lua b/DragonLoot/Core/Config.lua index a97a465..e2c19c4 100644 --- a/DragonLoot/Core/Config.lua +++ b/DragonLoot/Core/Config.lua @@ -36,6 +36,7 @@ local defaults = { scale = 1.0, lock = false, autoConfirmRolls = false, + confirmGreedAndPass = false, keepOpenAfterVote = false, resultLingerDuration = 3, showRollTally = false, @@ -144,7 +145,7 @@ local defaults = { -- Profile Migration ------------------------------------------------------------------------------- -local CURRENT_SCHEMA = 4 +local CURRENT_SCHEMA = 5 local function DeepCopyValue(value) if type(value) ~= "table" then @@ -245,10 +246,8 @@ local function MigrateProfile(db) end end - -- v3 -> v4: introduce db.char.history.entries for persistent loot history (issue #104). - -- No profile data needs transformation - the new char scope is added by AceDB's defaults - -- handling when InitializeDB passes the updated defaults table to AceDB:New. The schema - -- bump is recorded by the unconditional assignment to profile.schemaVersion below. + -- v3 -> v4 introduced db.char.history.entries. AceDB applies that char-scope + -- default when InitializeDB creates the database, so no profile transform is needed. profile.schemaVersion = CURRENT_SCHEMA end diff --git a/DragonLoot/Display/RollFrame.lua b/DragonLoot/Display/RollFrame.lua index 4f1ef8a..babb543 100755 --- a/DragonLoot/Display/RollFrame.lua +++ b/DragonLoot/Display/RollFrame.lua @@ -22,7 +22,6 @@ local ShoppingTooltip2 = ShoppingTooltip2 local UIParent = UIParent local GetLootRollItemInfo = GetLootRollItemInfo local GetLootRollItemLink = GetLootRollItemLink -local RollOnLoot = RollOnLoot local HandleModifiedItemClick = HandleModifiedItemClick local C_Texture = C_Texture local C_Item = C_Item @@ -494,14 +493,7 @@ local function OnRollButtonClick(self) return end if frame.rollID then - -- Mark pending hide BEFORE RollOnLoot; synchronous CONFIRM_LOOT_ROLL - -- will clear the flag if a confirmation popup is needed. - ns.RollManager.MarkPendingHide(frame.rollID) - - RollOnLoot(frame.rollID, self.rollType) - - -- Hide now unless CONFIRM_LOOT_ROLL intercepted (flag cleared) - ns.RollManager.TryHideAfterVote(frame.rollID, self.rollType) + ns.RollManager.RequestRollSelection(frame.rollID, self.rollType) end end diff --git a/DragonLoot/Display/RollManager.lua b/DragonLoot/Display/RollManager.lua index 1ff2a36..6767e13 100755 --- a/DragonLoot/Display/RollManager.lua +++ b/DragonLoot/Display/RollManager.lua @@ -14,6 +14,8 @@ local _, ns = ... local GetTime = GetTime local GetLootRollItemInfo = GetLootRollItemInfo local GetLootRollItemLink = GetLootRollItemLink +local RollOnLoot = RollOnLoot +local StaticPopup_Show = StaticPopup_Show local UnitName = UnitName local UnitClass = UnitClass local max = math.max @@ -109,6 +111,53 @@ local function GetResultLingerDuration() return (settings and settings.resultLingerDuration) or DEFAULT_RESULT_LINGER_SECONDS end +local function ShouldConfirmSelection(rollType) + local settings = GetRollFrameSettings() + if not settings or not settings.confirmGreedAndPass then + return false + end + return rollType == ROLL_TYPE_GREED or rollType == ROLL_TYPE_PASS +end + +local function ClearSelectionConfirmation(roll, dialog) + if not roll or roll.selectionConfirmationDialog ~= dialog then + return false + end + roll.pendingSelectionRollType = nil + roll.selectionConfirmationDialog = nil + return true +end + +local function DismissSelectionConfirmation(roll) + local dialog = roll and roll.selectionConfirmationDialog + if not dialog then + return + end + dialog.data = nil + roll.pendingSelectionRollType = nil + roll.selectionConfirmationDialog = nil + dialog:Hide() +end + +local function DismissOtherSelectionConfirmations(rollID) + for activeRollID, activeRoll in pairs(activeRolls) do + if activeRollID ~= rollID then + DismissSelectionConfirmation(activeRoll) + end + end +end + +local function SubmitRollSelection(rollID, rollType) + local roll = activeRolls[rollID] + if not roll or not roll.frameIndex or roll.heldAfterVote then + return + end + + ns.RollManager.MarkPendingHide(rollID) + RollOnLoot(rollID, rollType) + ns.RollManager.TryHideAfterVote(rollID, rollType) +end + ------------------------------------------------------------------------------- -- StaticPopup for roll confirmations (shared by Retail and Classic listeners) ------------------------------------------------------------------------------- @@ -129,6 +178,35 @@ StaticPopupDialogs["DRAGONLOOT_CONFIRM_LOOT_ROLL"] = { hideOnEscape = 1, } +StaticPopupDialogs["DRAGONLOOT_CONFIRM_ROLL_SELECTION"] = { + text = L["Are you sure you want to choose %s for %s?"], + button1 = YES, + button2 = NO, + OnAccept = function(self) + local data = self.data + self.data = nil + if not data then + return + end + local roll = activeRolls[data.rollID] + if not ClearSelectionConfirmation(roll, self) then + return + end + SubmitRollSelection(data.rollID, data.rollType) + end, + OnCancel = function(self) + local data = self.data + self.data = nil + if not data then + return + end + ClearSelectionConfirmation(activeRolls[data.rollID], self) + end, + timeout = 0, + whileDead = 1, + hideOnEscape = 1, +} + ------------------------------------------------------------------------------- -- Frame index management ------------------------------------------------------------------------------- @@ -632,6 +710,8 @@ function ns.RollManager.CancelRoll(rollID) local lifecycleToken = LifecycleUtil.CaptureToken(lifecycleState) local frameIndex = roll.frameIndex + DismissSelectionConfirmation(roll) + activeRolls[rollID] = nil notifiedRolls[rollID] = nil @@ -667,6 +747,7 @@ end function ns.RollManager.CancelAllRolls() for rollID, roll in pairs(activeRolls) do + DismissSelectionConfirmation(roll) activeRolls[rollID] = nil if roll.frameIndex then ReleaseFrameIndex(roll.frameIndex) @@ -736,6 +817,37 @@ function ns.RollManager.IsNotified(rollID) return notifiedRolls[rollID] or false end +function ns.RollManager.RequestRollSelection(rollID, rollType) + local roll = activeRolls[rollID] + if not roll or not roll.frameIndex or roll.heldAfterVote then + return + end + + if roll.selectionConfirmationDialog then + if roll.pendingSelectionRollType == rollType then + return + end + DismissSelectionConfirmation(roll) + end + + if not ShouldConfirmSelection(rollType) then + SubmitRollSelection(rollID, rollType) + return + end + + local rollTypeName = ns.RollTypeNames[rollType] or L["Unknown"] + local itemName = roll.itemName or L["Unknown"] + DismissOtherSelectionConfirmations(rollID) + local dialog = StaticPopup_Show("DRAGONLOOT_CONFIRM_ROLL_SELECTION", rollTypeName, itemName) + if not dialog then + return + end + + roll.pendingSelectionRollType = rollType + roll.selectionConfirmationDialog = dialog + dialog.data = { rollID = rollID, rollType = rollType } +end + function ns.RollManager.MarkPendingHide(rollID) local roll = activeRolls[rollID] if not roll then diff --git a/DragonLoot/Locales/enUS.lua b/DragonLoot/Locales/enUS.lua index 1a8aad6..5302e95 100755 --- a/DragonLoot/Locales/enUS.lua +++ b/DragonLoot/Locales/enUS.lua @@ -239,6 +239,11 @@ L["Show roll notifications while in raids"] = true L["Show roll notifications while in the open world"] = true L["Size of Need/Greed/Pass buttons"] = true L["Skip Roll Confirmations"] = true +L["Confirm Greed and Pass"] = true +-- stylua: ignore +L["Ask before submitting Greed or Pass from DragonLoot's roll frame." + .. " This does not change confirmations required by Blizzard."] = true +L["Are you sure you want to choose %s for %s?"] = true -- stylua: ignore L["Skip bind-on-pickup and disenchant roll confirmations. The item binds to you" .. " without asking, and disenchant rolls convert the item to materials."] = true diff --git a/DragonLoot_Options/Tabs/LootRollTab.lua b/DragonLoot_Options/Tabs/LootRollTab.lua index daff2d5..b1b3945 100755 --- a/DragonLoot_Options/Tabs/LootRollTab.lua +++ b/DragonLoot_Options/Tabs/LootRollTab.lua @@ -155,6 +155,21 @@ local function CreateRollFrameSection(parent, db, yOffset, layoutWidgets, reappl layoutWidgets[#layoutWidgets + 1] = autoConfirmToggle innerY = LC.AnchorWidget(autoConfirmToggle, content, innerY) - LC.SPACING_BETWEEN_WIDGETS + local confirmGreedAndPassToggle = W.CreateToggle(content, { + label = L["Confirm Greed and Pass"], + -- stylua: ignore + tooltip = L["Ask before submitting Greed or Pass from DragonLoot's roll frame." + .. " This does not change confirmations required by Blizzard."], + get = function() + return db.profile.rollFrame.confirmGreedAndPass + end, + set = function(value) + db.profile.rollFrame.confirmGreedAndPass = value + end, + }) + layoutWidgets[#layoutWidgets + 1] = confirmGreedAndPassToggle + innerY = LC.AnchorWidget(confirmGreedAndPassToggle, content, innerY) - LC.SPACING_BETWEEN_WIDGETS + local lingerSlider -- forward declared; only usable while keepOpenAfterVote is on local keepOpenToggle = W.CreateToggle(content, { diff --git a/spec/Config_spec.lua b/spec/Config_spec.lua index e3d5967..7eaba1a 100644 --- a/spec/Config_spec.lua +++ b/spec/Config_spec.lua @@ -41,7 +41,7 @@ describe("Config", function() local db = initWithSeed(ns, nil) -- Keep in sync with CURRENT_SCHEMA in DragonLoot/Core/Config.lua - assert.are.equal(4, db.profile.schemaVersion) + assert.are.equal(5, db.profile.schemaVersion) end) it("has lootIconSize in a fresh profile", function() @@ -57,6 +57,12 @@ describe("Config", function() assert.is_false(db.profile.rollFrame.autoConfirmRolls) end) + it("defaults rollFrame.confirmGreedAndPass to false (opt-in)", function() + local db = initWithSeed(ns, nil) + + assert.is_false(db.profile.rollFrame.confirmGreedAndPass) + end) + it("defaults rollFrame.keepOpenAfterVote to false (opt-in)", function() local db = initWithSeed(ns, nil) @@ -133,6 +139,28 @@ describe("Config", function() assert.is_true(db.profile.rollFrame.autoConfirmRolls) end) + it("back-fills missing rollFrame.confirmGreedAndPass with false", function() + local db = initWithSeed(ns, { + schemaVersion = 4, + rollFrame = { + enabled = true, + }, + }) + + assert.is_false(db.profile.rollFrame.confirmGreedAndPass) + end) + + it("preserves an opted-in rollFrame.confirmGreedAndPass", function() + local db = initWithSeed(ns, { + schemaVersion = 4, + rollFrame = { + confirmGreedAndPass = true, + }, + }) + + assert.is_true(db.profile.rollFrame.confirmGreedAndPass) + end) + it("back-fills missing rollFrame.keepOpenAfterVote and resultLingerDuration", function() local db = initWithSeed(ns, { rollFrame = { @@ -248,7 +276,7 @@ describe("Config", function() -- Seed at current schema so FillMissingDefaults is skipped and the -- (unconditional) iconSize-split migration can propagate iconSize=48. -- Keep in sync with CURRENT_SCHEMA in DragonLoot/Core/Config.lua. - schemaVersion = 4, + schemaVersion = 5, appearance = { iconSize = 48, -- lootIconSize intentionally absent to test migration propagation @@ -272,7 +300,7 @@ describe("Config", function() }) -- Keep in sync with CURRENT_SCHEMA in DragonLoot/Core/Config.lua - assert.are.equal(4, db.profile.schemaVersion) + assert.are.equal(5, db.profile.schemaVersion) end) end) end) diff --git a/spec/RollSelectionConfirmation_spec.lua b/spec/RollSelectionConfirmation_spec.lua new file mode 100644 index 0000000..2033dee --- /dev/null +++ b/spec/RollSelectionConfirmation_spec.lua @@ -0,0 +1,217 @@ +------------------------------------------------------------------------------- +-- RollSelectionConfirmation_spec.lua +-- Tests for optional confirmation before DragonLoot submits Greed or Pass. +------------------------------------------------------------------------------- + +local mock = require("spec.wow_mock") + +local ROLL_PASS = 0 +local ROLL_NEED = 1 +local ROLL_GREED = 2 +local ROLL_DISENCHANT = 3 + +local function NewRollNamespace(recorder, settings) + local ns = mock.CreateNamespace() + ns.L = setmetatable({}, { + __index = function(_, key) + return key + end, + }) + ns.RollFrame = { + Initialize = function() end, + Shutdown = function() end, + ApplySettings = function() end, + HideAllRolls = function() end, + ShowRoll = function() end, + HideRoll = function(frameIndex, onComplete) + recorder.hidden[#recorder.hidden + 1] = frameIndex + if onComplete then + onComplete() + end + end, + MarkVoted = function(frameIndex, rollType) + recorder.voted[#recorder.voted + 1] = { frameIndex = frameIndex, rollType = rollType } + end, + } + ns.RollListener = { + Initialize = function() end, + Shutdown = function() end, + ResolveWinner = function() end, + } + + local rollFrameSettings = { + enabled = true, + confirmGreedAndPass = false, + keepOpenAfterVote = false, + resultLingerDuration = 3, + } + for key, value in pairs(settings or {}) do + rollFrameSettings[key] = value + end + + local addon = { + db = { + profile = { + rollFrame = rollFrameSettings, + rollNotifications = {}, + }, + }, + ScheduleRepeatingTimer = function() + return {} + end, + CancelTimer = function() end, + SendMessage = function() end, + } + ns.Addon = addon + + mock.LoadLifecycle(ns) + mock.LoadFile(ns, "DragonLoot/Display/RollManager.lua") + ns.RollManager.Initialize(addon) + ns.RollManager.StartRoll(7, 30) + return ns +end + +local function AcceptLatest() + local dialog = mock._popupDialogs[#mock._popupDialogs] + mock.AcceptPopup(dialog) + return dialog +end + +local function CancelLatest() + local dialog = mock._popupDialogs[#mock._popupDialogs] + mock.CancelPopup(dialog) + return dialog +end + +describe("Greed and Pass selection confirmation", function() + local recorder + + before_each(function() + mock.Reset() + recorder = { hidden = {}, voted = {} } + end) + + it("submits Greed immediately when disabled", function() + local ns = NewRollNamespace(recorder) + + ns.RollManager.RequestRollSelection(7, ROLL_GREED) + + assert.are.same({ { rollID = 7, rollType = ROLL_GREED } }, mock._rollSubmissions) + assert.are.equal(0, #mock._popupDialogs) + assert.are.equal(1, #recorder.hidden) + end) + + for _, choice in ipairs({ + { name = "Greed", rollType = ROLL_GREED }, + { name = "Pass", rollType = ROLL_PASS }, + }) do + it("asks before submitting " .. choice.name, function() + local ns = NewRollNamespace(recorder, { confirmGreedAndPass = true }) + + ns.RollManager.RequestRollSelection(7, choice.rollType) + + assert.are.equal(0, #mock._rollSubmissions) + assert.are.equal(1, #mock._popupDialogs) + assert.are.equal(choice.name, mock._popupDialogs[1].textArg1) + assert.are.equal("Test Item", mock._popupDialogs[1].textArg2) + + AcceptLatest() + + assert.are.same({ { rollID = 7, rollType = choice.rollType } }, mock._rollSubmissions) + end) + end + + it("submits no roll when cancelled and allows another choice", function() + local ns = NewRollNamespace(recorder, { confirmGreedAndPass = true }) + ns.RollManager.RequestRollSelection(7, ROLL_PASS) + + CancelLatest() + + assert.are.equal(0, #mock._rollSubmissions) + assert.is_not_nil(ns.RollManager.GetActiveRolls()[7].frameIndex) + + ns.RollManager.RequestRollSelection(7, ROLL_NEED) + assert.are.same({ { rollID = 7, rollType = ROLL_NEED } }, mock._rollSubmissions) + end) + + it("keeps custom confirmation separate from auto-confirming Blizzard prompts", function() + local ns = NewRollNamespace(recorder, { + autoConfirmRolls = true, + confirmGreedAndPass = true, + }) + + ns.RollManager.RequestRollSelection(7, ROLL_GREED) + + assert.are.equal(1, #mock._popupDialogs) + assert.are.equal(0, #mock._rollSubmissions) + end) + + it("replaces a pending prompt when another roll asks for confirmation", function() + local ns = NewRollNamespace(recorder, { confirmGreedAndPass = true }) + ns.RollManager.StartRoll(8, 30) + ns.RollManager.RequestRollSelection(7, ROLL_PASS) + local firstDialog = mock._popupDialogs[1] + + ns.RollManager.RequestRollSelection(8, ROLL_GREED) + + assert.is_true(firstDialog.hidden) + assert.are.equal(2, #mock._popupDialogs) + mock.AcceptPopup(firstDialog) + assert.are.equal(0, #mock._rollSubmissions) + + AcceptLatest() + assert.are.same({ { rollID = 8, rollType = ROLL_GREED } }, mock._rollSubmissions) + end) + + it("does not submit twice when an accepted callback repeats", function() + local ns = NewRollNamespace(recorder, { confirmGreedAndPass = true }) + ns.RollManager.RequestRollSelection(7, ROLL_GREED) + local dialog = AcceptLatest() + + mock.AcceptPopup(dialog) + + assert.are.equal(1, #mock._rollSubmissions) + end) + + it("does not change Need or Disenchant selections", function() + local ns = NewRollNamespace(recorder, { confirmGreedAndPass = true }) + + ns.RollManager.RequestRollSelection(7, ROLL_NEED) + ns.RollManager.StartRoll(8, 30) + ns.RollManager.RequestRollSelection(8, ROLL_DISENCHANT) + + assert.are.same({ + { rollID = 7, rollType = ROLL_NEED }, + { rollID = 8, rollType = ROLL_DISENCHANT }, + }, mock._rollSubmissions) + assert.are.equal(0, #mock._popupDialogs) + end) + + it("dismisses an unanswered prompt when the roll ends", function() + local ns = NewRollNamespace(recorder, { confirmGreedAndPass = true }) + ns.RollManager.RequestRollSelection(7, ROLL_PASS) + local dialog = mock._popupDialogs[1] + + ns.RollManager.CancelRoll(7) + mock.AcceptPopup(dialog) + + assert.is_true(dialog.hidden) + assert.are.equal(0, #mock._rollSubmissions) + end) + + it("holds the frame open only after the confirmed vote", function() + local ns = NewRollNamespace(recorder, { + confirmGreedAndPass = true, + keepOpenAfterVote = true, + }) + ns.RollManager.RequestRollSelection(7, ROLL_GREED) + + assert.are.equal(0, #recorder.voted) + + AcceptLatest() + + assert.are.equal(1, #recorder.voted) + assert.are.equal(ROLL_GREED, recorder.voted[1].rollType) + assert.is_true(ns.RollManager.GetActiveRolls()[7].heldAfterVote) + end) +end) diff --git a/spec/wow_mock.lua b/spec/wow_mock.lua index 6928068..c17d2ed 100644 --- a/spec/wow_mock.lua +++ b/spec/wow_mock.lua @@ -109,6 +109,35 @@ StaticPopupDialogs = {} function ConfirmLootRoll() end +M._rollSubmissions = {} + +function RollOnLoot(rollID, rollType) + M._rollSubmissions[#M._rollSubmissions + 1] = { rollID = rollID, rollType = rollType } +end + +M._popupDialogs = {} + +function StaticPopup_Show(which, textArg1, textArg2) + local dialog = { + which = which, + textArg1 = textArg1, + textArg2 = textArg2, + Hide = function(self) + self.hidden = true + end, + } + M._popupDialogs[#M._popupDialogs + 1] = dialog + return dialog +end + +function M.AcceptPopup(dialog) + _G.StaticPopupDialogs[dialog.which].OnAccept(dialog) +end + +function M.CancelPopup(dialog) + _G.StaticPopupDialogs[dialog.which].OnCancel(dialog) +end + function GetLootRollItemInfo() return 12345, "Test Item", 1, 4 end @@ -526,6 +555,9 @@ function M.Reset() M._lootHistory.items = {} M._lootHistory.players = {} + M._rollSubmissions = {} + M._popupDialogs = {} + M._group.numRaid = 0 M._group.numParty = 0 M._group.isMasterLooter = true From a7710c9d34413da2ac58feea4cd84616eae04164 Mon Sep 17 00:00:00 2001 From: Lasse Nielsen Date: Fri, 4 Sep 2026 13:44:29 +0200 Subject: [PATCH 2/2] docs(agents): document greed and pass confirmation --- AGENTS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 7c477eb..035ae57 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -66,6 +66,7 @@ The repository is structured as a multi-addon project separating core logic, con | scale | number | 1.0 | | lock | boolean | false | | autoConfirmRolls | boolean | false | +| confirmGreedAndPass | boolean | false | | keepOpenAfterVote | boolean | false | | resultLingerDuration | number | 3 | | showRollTally | boolean | false | @@ -94,7 +95,7 @@ The repository is structured as a multi-addon project separating core logic, con | timerBarStyle | string | "normal" | | timerBarMinimalHeight | number | 3 | -`resultLingerDuration` applies only when `keepOpenAfterVote` is enabled. `showRollTally` is available only on Classic (TBC/MoP); Retail removed `C_LootHistory.GetItem` and `GetPlayerInfo` in patch 10.1.0. +`confirmGreedAndPass` asks for confirmation before Greed or Pass is submitted. `resultLingerDuration` applies only when `keepOpenAfterVote` is enabled. `showRollTally` is available only on Classic (TBC/MoP); Retail removed `C_LootHistory.GetItem` and `GetPlayerInfo` in patch 10.1.0. ### History (`db.profile.history`)