diff --git a/spec/System/TestCustomModControl_spec.lua b/spec/System/TestCustomModControl_spec.lua index f76eceeff42..b292551e3a6 100644 --- a/spec/System/TestCustomModControl_spec.lua +++ b/spec/System/TestCustomModControl_spec.lua @@ -54,10 +54,40 @@ describe("Custom modifier controls", function() end end - assert.are.same({ "Minions deal 12% increased Damage" }, minionDamageEntries) + assert.are.same({ "Minions deal 10% increased Damage" }, minionDamageEntries) popup.controls.listControl.selIndex = minionDamageIndex popup.controls.save.onClick() - assert.are.equal("Minions deal 12% increased Damage", blockData.text) + assert.are.equal("Minions deal 10% increased Damage", blockData.text) + end) + + it("lists the same modifiers regardless of table iteration order", function() + local function listModsWithOrderedPairs(reverse) + local iteratedTables = { } + -- busted runs specs in their own environment; the browser module reads the real global + local originalPairs = _G.pairs + _G.pairs = function(tbl) + iteratedTables[tbl] = true + local keys = { } + for key in next, tbl do + table.insert(keys, key) + end + table.sort(keys, function(a, b) return type(a) .. tostring(a) < type(b) .. tostring(b) end) + local index = reverse and #keys + 1 or 0 + return function() + index = index + (reverse and -1 or 1) + return keys[index], tbl[keys[index]] + end + end + local ok, popup = pcall(openModBrowser) + _G.pairs = originalPairs + assert(ok, popup) + main:ClosePopup() + local tree = build.treeTab.specList[build.treeTab.activeSpec].tree + assert.is_true(iteratedTables[tree.nodes] and iteratedTables[build.data.itemMods], "the browser did not use the ordered pairs") + return popup.controls.listControl.list + end + + assert.are.same(listModsWithOrderedPairs(false), listModsWithOrderedPairs(true)) end) it("orders modifiers alphabetically while ignoring numeric values", function() diff --git a/src/Modules/ConfigModBrowser.lua b/src/Modules/ConfigModBrowser.lua index 2408963bb6f..2143336c6f8 100644 --- a/src/Modules/ConfigModBrowser.lua +++ b/src/Modules/ConfigModBrowser.lua @@ -52,6 +52,7 @@ end ---@param source string ---@param supported boolean? -- either adds a new entry or adds the source to an existing entry +-- the entry keeps a supported line over an unsupported one, then the line that sorts first local function addModLine(set, line, source, supported) local template = getModTemplate(line) local modEntry = set[template] @@ -60,6 +61,10 @@ local function addModLine(set, line, source, supported) set[template] = modEntry else modEntry.sources[source] = true + -- the text order makes the result independent of table iteration order + if line < modEntry.text and (supported or not modEntry.supported) then + modEntry.text = line + end end if supported ~= nil then if supported and not modEntry.supported then @@ -68,34 +73,40 @@ local function addModLine(set, line, source, supported) modEntry.supported = modEntry.supported or supported end end ----@param seenGroups table +-- raw item mod line -> { rangedLine, supported }, kept for the session +local rangedItemLineCache = {} +---@param set table +---@param line string +---@param source string +local function addItemModLine(set, line, source) + local rangedEntry = rangedItemLineCache[line] + if not rangedEntry then + -- note that exactly 0.5 range is necessary as that is what the + -- mod cache uses + local rangedLine = itemLib.applyRange(line, 0.5) + local modList, extra = modLib.parseMod(rangedLine) + rangedEntry = { rangedLine = rangedLine, supported = (not not modList) and not extra } + rangedItemLineCache[line] = rangedEntry + end + addModLine(set, rangedEntry.rangedLine, source, rangedEntry.supported) +end ---@param set table ---@param mod table ---@param source string -local function addItemMod(seenGroups, set, mod, source) - if not seenGroups[mod.group] then - seenGroups[mod.group] = true - -- the actual trade hashes aren't relevant, but this - -- field is separated by stat description, which - -- means that getting rid of split lines won't - -- result in accidentally combining two separate - -- stats - if mod.tradeHashes then - for _, statDescription in pairs(mod.tradeHashes) do - local line = table.concat(statDescription, " ") - -- note that exactly 0.5 range is necessary as that is what the - -- mod cache uses - local rangedLine = itemLib.applyRange(line, 0.5) - local modList, extra = modLib.parseMod(rangedLine) - addModLine(set, rangedLine, source, (not not modList) and not extra) - end - else - -- crafted mods have a different format - for _, line in ipairs(mod) do - local rangedLine = itemLib.applyRange(line, 0.5) - local modList, extra = modLib.parseMod(rangedLine) - addModLine(set, rangedLine, source, (not not modList) and not extra) - end +local function addItemMod(set, mod, source) + -- the actual trade hashes aren't relevant, but this + -- field is separated by stat description, which + -- means that getting rid of split lines won't + -- result in accidentally combining two separate + -- stats + if mod.tradeHashes then + for _, statDescription in pairs(mod.tradeHashes) do + addItemModLine(set, table.concat(statDescription, " "), source) + end + else + -- crafted mods have a different format + for _, line in ipairs(mod) do + addItemModLine(set, line, source) end end end @@ -184,29 +195,28 @@ function M.OpenAddModPopup(configTab, blockData) end end end - local recordedGroups = {} if bData.masterMods then for _, mod in ipairs(bData.masterMods) do - addItemMod(recordedGroups, modSet, mod, "Crafted mod") + addItemMod(modSet, mod, "Crafted mod") end end if bData.itemMods then for catName, catMods in pairs(bData.itemMods) do if (catName ~= "Item" and catName ~= "JewelCluster") and type(catMods) == "table" then for _, mod in pairs(catMods) do - addItemMod(recordedGroups, modSet, mod, "Item mod") + addItemMod(modSet, mod, "Item mod") end end end end if bData.veiledMods then for _, mod in pairs(bData.veiledMods) do - addItemMod(recordedGroups, modSet, mod, "Veiled mod") + addItemMod(modSet, mod, "Veiled mod") end end if bData.beastCraft then for _, mod in pairs(bData.beastCraft) do - addItemMod(recordedGroups, modSet, mod, "Item mod") + addItemMod(modSet, mod, "Item mod") end end