Add sorting for modifier dropdowns in crafting UI

spec/System/TestItemMods_spec.lua

@@ -803,6 +803,80 @@ describe("TetsItemMods", function()
 		end
 		assert.are.equals(2, count)
 	end)
+
+	it("does not sort cluster jewel modifiers when the sorting control is hidden", function()
+		local item = new("Item", [[
+			Rarity: RARE
+			New Item
+			Large Cluster Jewel
+			Crafted: true
+			Prefix: {range:0.5}AfflictionNotableWickedPall_
+			Prefix: {range:0.5}AfflictionNotableMiseryEverlasting
+			Suffix: {range:0.5}AfflictionNotableUnholyGrace_
+			Suffix: None
+			Cluster Jewel Skill: affliction_chaos_damage
+			Cluster Jewel Node Count: 8
+			Quality: 0
+			LevelReq: 40
+			Implicits: 3
+			{crafted}Adds 8 Passive Skills
+			{crafted}2 Added Passive Skills are Jewel Sockets
+			{crafted}Added Small Passive Skills grant: 12% increased Chaos Damage
+			1 Added Passive Skill is Misery Everlasting
+			1 Added Passive Skill is Unholy Grace
+			1 Added Passive Skill is Wicked Pall
+		]])
+		local calcCount = 0
+		build.itemsTab.displayItem = item
+		build.itemsTab.controls.craftingSorting:SetSel(2, true)
+		build.calcsTab.GetMiscCalculator = function()
+			return function()
+				calcCount = calcCount + 1
+				return { }
+			end
+		end
+
+		assert.is_false(build.itemsTab.controls.craftingSortingLabel.shown())
+		build.itemsTab:UpdateAffixControls()
+		assert.are.equals(0, calcCount)
+	end)
+
+	it("sorts crafted modifier replacements without retaining the selected modifier", function()
+		local item = new("Item", [[
+			Rarity: RARE
+			New Item
+			Cobalt Jewel
+			Crafted: true
+			Prefix: {range:1}PercentIncreasedLifeJewel
+			Prefix: None
+			Suffix: None
+			Suffix: None
+			Quality: 0
+			LevelReq: 0
+			Implicits: 0
+			7% increased maximum Life
+		]])
+		local calcCount = 0
+		local retainedCount = 0
+		build.itemsTab.displayItem = item
+		build.itemsTab.controls.craftingSorting:SetSel(2, true)
+		build.calcsTab.GetMiscCalculator = function()
+			return function(args)
+				calcCount = calcCount + 1
+				for _, modLine in ipairs(args.repItem.explicitModLines) do
+					if modLine.line == "7% increased maximum Life" then
+						retainedCount = retainedCount + 1
+						break
+					end
+				end
+				return { }
+			end
+		end
+
+		build.itemsTab:UpdateAffixControl(build.itemsTab.controls.displayItemAffix1, item, "Prefix", "prefixes", 1, { })
+		assert.is_true(calcCount > 1)
+		assert.are.equals(0, retainedCount)
+	end)
 
 	it("shows a fallback tooltip when an item's base is no longer supported", function()
 		local item = new("Item", [[

src/Classes/ItemsTab.lua

@@ -626,7 +626,7 @@ holding Shift will put it in the second.]])
 	self.controls.displayItemSectionQuality = new("Control", {"TOPLEFT",self.controls.displayItemSectionInfluence,"BOTTOMLEFT"}, {0, 0, 0, function()
 		return (self.controls.displayItemQuality:IsShown() and self.controls.displayItemQualityEdit:IsShown()) and 28 or 0
 	end})
-	self.controls.displayItemQuality = new("LabelControl", {"TOPLEFT",self.controls.displayItemSectionQuality,"TOPRIGHT"}, {-4, 0, 0, 16}, "^7Quality:")
+	self.controls.displayItemQuality = new("LabelControl", { "TOPLEFT", self.controls.displayItemSectionQuality, "TOPRIGHT" }, { 0, 0, 0, 16 }, "^7Quality:")
 	self.controls.displayItemQuality.shown = function()
 		return self.displayItem and self.displayItem.quality and (self.displayItem.base.type ~= "Amulet" or self.displayItem.base.type ~= "Belt" or self.displayItem.base.type ~= "Jewel" or self.displayItem.base.type ~= "Quiver" or self.displayItem.base.type ~= "Ring" or self.displayItem.type ~= "Graft")
 	end
@@ -640,6 +640,14 @@ holding Shift will put it in the second.]])
 		return self.displayItem and self.displayItem.quality and (self.displayItem.base.type ~= "Amulet" or self.displayItem.base.type ~= "Belt" or self.displayItem.base.type ~= "Jewel" or self.displayItem.base.type ~= "Quiver" or self.displayItem.base.type ~= "Ring" or self.displayItem.type ~= "Graft")
 	end
 
+	local sortingOptions = {
+		{ stat = nil, label = "Default" }
+	}
+	for _, option in ipairs(data.powerStatList) do
+		if not option.ignoreForItems and option.label ~= "Name" then
+			table.insert(sortingOptions, option)
+		end
+	end
 	-- Section: Catalysts
 	self.controls.displayItemSectionCatalyst = new("Control", {"TOPLEFT",self.controls.displayItemSectionQuality,"BOTTOMLEFT"}, {0, 0, 0, function()
 		return (self.controls.displayItemCatalyst:IsShown() or self.controls.displayItemCatalystQualityEdit:IsShown()) and 28 or 0
@@ -703,9 +711,25 @@ holding Shift will put it in the second.]])
 		self:CraftClusterJewel()
 	end)
 
+	self.controls.craftingSortingLabel = new("LabelControl", { "TOPLEFT", self.controls.displayItemSectionClusterJewel, "BOTTOMLEFT" }, { 0, 0, 0, 16 }, "^7Modifier sorting:")
+	self.controls.craftingSortingLabel.shown = function()
+		return self.displayItem and self.displayItem.crafted and
+			-- cluster jewels don't have good comparison support and sorting would be misleading
+			not (self.displayItem.base.type == "Jewel" and self.displayItem.base.subType == "Cluster")
+	end
+	self.controls.craftingSorting = new("DropDownControl", { "LEFT", self.controls.craftingSortingLabel, "RIGHT" }, { 4, 0, 200, 20 }, sortingOptions, function()
+		self:UpdateAffixControls()
+	end)
+
 	-- Section: Affix Selection
 	local maxModCount = 9
-	self.controls.displayItemSectionAffix = new("Control", {"TOPLEFT",self.controls.displayItemSectionClusterJewel,"BOTTOMLEFT"}, {0, 0, 0, function()
+	self.controls.displayItemSectionAffix = new("Control", { "TOPLEFT", self.controls.craftingSortingLabel, "BOTTOMLEFT" }, { 0, function()
+		if self.controls.craftingSortingLabel.shown() then
+			return 8
+		else
+			return -16
+		end
+	end, 0, function()
 		if not self.displayItem or not self.displayItem.crafted then
 			return 0
 		end
@@ -720,6 +744,7 @@ holding Shift will put it in the second.]])
 		end
 		return h
 	end})
+
 	for i = 1, maxModCount do
 		local prev = self.controls["displayItemAffix"..(i-1)] or self.controls.displayItemSectionAffix
 		local drop, slider
@@ -887,7 +912,7 @@ holding Shift will put it in the second.]])
 					if value.modId or #modList == 1 then
 						mod = self.displayItem.affixes[value.modId or modList[1]]
 					else
-						mod = self.displayItem.affixes[modList[1 + round((#modList - 1) * main.defaultItemAffixQuality)]]
+						mod = self.displayItem.affixes[modList[1 + round((#modList - 1) * (main.defaultItemAffixQuality or 0.5))]]
 					end
 
 					-- Adding Mod
@@ -2079,23 +2104,24 @@ function ItemsTabClass:UpdateAffixControls()
 	local item = self.displayItem
 	local prefixLimit = item.prefixes.limit or (item.affixLimit / 2)
 	local ignoreModType = item.rareLikeUnique and item.rareLikeUnique.ignoreModType
+	local powerCache = {}
 	for i = 1, item.affixLimit do
 		if i <= prefixLimit then
 			local modType = "Prefix"
 			if ignoreModType then
 				modType = nil
 			end
-			self:UpdateAffixControl(self.controls["displayItemAffix" .. i], item, modType, "prefixes", i)
+			self:UpdateAffixControl(self.controls["displayItemAffix" .. i], item, modType, "prefixes", i, powerCache)
 		else
-			self:UpdateAffixControl(self.controls["displayItemAffix"..i], item, "Suffix", "suffixes", i - prefixLimit)
+			self:UpdateAffixControl(self.controls["displayItemAffix" .. i], item, "Suffix", "suffixes", i - prefixLimit, powerCache)
 		end
 	end	
 	-- The custom affixes may have had their indexes changed, so the custom control UI is also rebuilt so that it will
 	-- reference the correct affix index.
 	self:UpdateCustomControls()
 end
 
-function ItemsTabClass:UpdateAffixControl(control, item, affixType, outputTable, outputIndex)
+function ItemsTabClass:UpdateAffixControl(control, item, affixType, outputTable, outputIndex, powerCache)
 	local extraTags = { }
 	local excludeGroups = { }
 	local allowDuplicateGroups = item.rareLikeUnique and item.rareLikeUnique.allowDuplicateGroups
@@ -2159,50 +2185,128 @@ function ItemsTabClass:UpdateAffixControl(control, item, affixType, outputTable,
 	control.slider.shown = false
 	control.slider.val = main.defaultItemAffixQuality or 0.5
 	local selAffix = item[outputTable][outputIndex].modId
-	if (item.type == "Jewel" and item.base.subType ~= "Abyss") then
-		for i, modId in pairs(affixList) do
-			local mod = item.affixes[modId]
-			if selAffix == modId then
-				control.selIndex = i + 1
-			end
+	local lastSeries
+	-- combine runs of modifiers to one group, which will only take up one row
+	-- in the list
+	for _, modId in ipairs(affixList) do
+		local mod = item.affixes[modId]
+		if not lastSeries or not tableDeepEquals(lastSeries.statOrder, mod.statOrder) then
 			local modString = table.concat(mod, "/")
-			local label = modString
-			if retainedAffixes[modId] then
-				label = "^8[Retained] " .. modString
-			elseif item.type == "Flask" then
-				label = mod.affix .. "   ^8[" .. modString .. "]"
-			end
-			control.list[i + 1] = {
-				label = label,
-				modList = { modId },
-				modId = modId,
+			lastSeries = {
+				label = modString,
+				modList = {},
 				haveRange = modString:match("%(%-?[%d%.]+%-%-?[%d%.]+%)"),
+				statOrder = mod.statOrder,
 			}
+			t_insert(control.list, lastSeries)
 		end
-	else
-		local lastSeries
-		for _, modId in ipairs(affixList) do
-			local mod = item.affixes[modId]
-			if not lastSeries or not tableDeepEquals(lastSeries.statOrder, mod.statOrder) then
-				local modString = table.concat(mod, "/")
-				lastSeries = {
-					label = modString,
-					modList = { },
-					haveRange = modString:match("%(%-?[%d%.]+%-%-?[%d%.]+%)"),
-					statOrder = mod.statOrder,
-				}
-				t_insert(control.list, lastSeries)
+		-- cluster jewel mods retained after changing the cluster type
+		if retainedAffixes[modId] then
+			lastSeries.label = "^8[Retained] " .. lastSeries.label
+		end
+		t_insert(lastSeries.modList, modId)
+		if #lastSeries.modList == 2 then
+			lastSeries.label = lastSeries.label:gsub("%(%-?[%d%.]+%-%-?[%d%.]+%)", "#"):gsub("%-?%d+%.?%d*", "#")
+			lastSeries.haveRange = true
+		end
+	end
+
+	local sortOption = self.controls.craftingSorting:GetSelValue()
+	-- sort modifier groups by power
+	if sortOption.stat and self.controls.craftingSortingLabel.shown() then
+		local calcFunc = self.build.calcsTab:GetMiscCalculator()
+		local slotName = self.displayItem:GetPrimarySlot()
+		local testSubject = new("Item", self.displayItem:BuildRaw())
+		local controlPowerCache = powerCache
+		if selAffix and selAffix ~= "None" then
+			testSubject[outputTable][outputIndex] = { modId = "None" }
+			testSubject:Craft()
+			controlPowerCache = { }
+		end
+		local function pickModifierFromList(modList)
+			-- pick mid tier modifier from a group
+			if #modList == 1 then
+				return modList[1]
+			else
+				return modList[1 + round((#modList - 1) * main.defaultItemAffixQuality)]
+			end
+		end
+		local function getPower(modId)
+			if controlPowerCache[modId] then
+				return controlPowerCache[modId]
+			end
+			local mod = testSubject.affixes[modId]
+
+			local modCount = #mod
+			-- magnitude scaling happens during item parsing, which means we
+			-- can't use the faster path where items don't need to be
+			-- re-parsed. note that this wouldn't be correct if we were
+			-- adding a mod magnitude mod here, but currently all mod
+			-- magnitude mods are custom modifiers and so this works
+			local power
+			if (#testSubject.modMagnitudeMods > 0) or (testSubject.catalyst and testSubject.catalyst > 0) then
+				local originalItem = testSubject:BuildRaw()
+				for _, subMod in ipairs(mod) do
+					local modLine = { line = subMod, modTags = mod.modTags, [mod.type] = true }
+					t_insert(testSubject.explicitModLines, modLine)
+				end
+				testSubject:BuildAndParseRaw()
+				power = data.powerStatList.GetFromOutput(
+					calcFunc({ repSlotName = slotName, repItem = testSubject }),
+					sortOption
+				)
+				testSubject = new("Item", originalItem)
+			else
+				for _, line in ipairs(mod) do
+					local rangedLine = itemLib.applyRange(line, main.defaultItemAffixQuality or 0.5, 1, 1)
+					local modList, extra = modLib.parseMod(rangedLine)
+					local modLine = { line = line, modList = modList, extra = extra, modTags = mod.modTags, [mod.type] = true }
+					t_insert(testSubject.explicitModLines, modLine)
+				end
+
+				testSubject:BuildModList()
+				power = data.powerStatList.GetFromOutput(
+					calcFunc({ repSlotName = slotName, repItem = testSubject }),
+					sortOption
+				)
+				for _ = 1, modCount do
+					t_remove(testSubject.explicitModLines, #testSubject.explicitModLines)
+				end
 			end
-			if selAffix == modId then
-				control.selIndex = #control.list
+			controlPowerCache[modId] = power
+			return power
+		end
+		table.sort(control.list, function(a, b)
+			-- keep "None" as the first option
+			if not a.modList then
+				return true
+			elseif not b.modList then
+				return false
 			end
-			t_insert(lastSeries.modList, modId)
-			if #lastSeries.modList == 2 then
-				lastSeries.label = lastSeries.label:gsub("%(%-?[%d%.]+%-%-?[%d%.]+%)","#"):gsub("%-?%d+%.?%d*","#")
-				lastSeries.haveRange = true
+
+			local modIdA = pickModifierFromList(a.modList)
+			local modIdB = pickModifierFromList(b.modList)
+
+			return getPower(modIdA) > getPower(modIdB)
+		end)
+	end
+	local function findSelectedIdx()
+		for i, entry in ipairs(control.list) do
+			if entry.modList then
+				for _, modId in ipairs(entry.modList) do
+					if selAffix == modId then
+						return i
+					end
+				end
+			else
+				if selAffix == entry then
+					return i
+				end
 			end
 		end
+		return 1
 	end
+	control.selIndex = findSelectedIdx()
 	if control.list[control.selIndex].haveRange then
 		control.slider.divCount = #control.list[control.selIndex].modList
 		local index = isValueInArray(control.list[control.selIndex].modList, selAffix)