Fix custom mod browser list changing between launches

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()

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