Fix items from builds saved before 2.67 loading with wrong values

spec/System/TestItemParse_spec.lua

@@ -54,6 +54,50 @@ describe("TestItemParse", function()
 		data.itemBases[baseName] = nil
 	end)
 
+	it("keeps saved rolls on their lines for a talisman saved before its base enchant", function()
+		local baseName = "Test Talisman"
+		data.itemBases[baseName] = {
+			type = "Amulet",
+			subType = "Talisman",
+			tags = { amulet = true, talisman = true },
+			req = { },
+			enchant = "+10 to Strength",
+		}
+		loadBuildFromXML([[
+<PathOfBuilding>
+	<Build level="1" targetVersion="3_0" className="Scion" ascendClassName="None" mainSocketGroup="1"/>
+	<Items activeItemSet="1">
+		<Item id="1">
+Rarity: RARE
+Old Save
+Test Talisman
+Implicits: 1
++(10-20) to Dexterity
+			<ModRange id="1" range="1"/>
+		</Item>
+		<ItemSet id="1"/>
+	</Items>
+</PathOfBuilding>]])
+		data.itemBases[baseName] = nil
+
+		assert.are.equals(1, build.itemsTab.items[1].implicitModLines[1].range)
+	end)
+
+	it("does not add a talisman base enchant already saved as an implicit", function()
+		local baseName = "Test Talisman"
+		data.itemBases[baseName] = {
+			type = "Amulet",
+			subType = "Talisman",
+			tags = { amulet = true, talisman = true },
+			req = { },
+			enchant = "+10 to Strength",
+		}
+		local item = new("Item"):Item("Rarity: Normal\n" .. baseName .. "\nImplicits: 1\n+10 to Strength")
+		data.itemBases[baseName] = nil
+
+		assert.are.equals(0, #item.enchantModLines)
+	end)
+
 	it("Two-Toned Boots", function()
 		local item = new("Item"):Item(raw("", "Two-Toned Boots"))
 		assert.are.equals("Two-Toned Boots (Armour/Energy Shield)", item.baseName)
@@ -886,6 +930,26 @@ describe("TestAdvancedItemParse #item", function()
 		assert.are.equals("Commissioned 150720 coins to commemorate Chitus", seedLine)
 	end)
 
+	it("keeps the roll written in a saved value(min-max) line over its ModRange", function()
+		loadBuildFromXML([[
+<PathOfBuilding>
+	<Build level="1" targetVersion="3_0" className="Scion" ascendClassName="None" mainSocketGroup="1"/>
+	<Items activeItemSet="1">
+		<Item id="1">
+Rarity: UNIQUE
+Crown of the Inward Eye
+Prophet Crown
+Implicits: 0
+{range:0.5}14(9-21)% increased maximum Life, Mana and Global Energy Shield
+			<ModRange id="1" range="0.5"/>
+		</Item>
+		<ItemSet id="1"/>
+	</Items>
+</PathOfBuilding>]])
+
+		assert.are.equals(0.416667, build.itemsTab.items[1].explicitModLines[1].range)
+	end)
+
 	it("preserves independently rolled values on the same modifier line", function()
 		local item = new("Item"):Item([[
 			Rarity: Unique
@@ -989,6 +1053,28 @@ describe("TestAdvancedItemParse #item", function()
 		assert.are.equals(1, #item.explicitModLines)
 	end)
 
+	it("keeps saved rolls when a property was saved among the modifier lines", function()
+		loadBuildFromXML([[
+<PathOfBuilding>
+	<Build level="1" targetVersion="3_0" className="Scion" ascendClassName="None" mainSocketGroup="1"/>
+	<Items activeItemSet="1">
+		<Item id="1">
+Rarity: RARE
+Pain Shell
+Vaal Regalia
+Implicits: 0
+Intangibility: 7%
+{range:0}+(10-13) to all Attributes
+			<ModRange id="1" range="0.5"/>
+			<ModRange id="2" range="0"/>
+		</Item>
+		<ItemSet id="1"/>
+	</Items>
+</PathOfBuilding>]])
+
+		assert.are.equals(0, build.itemsTab.items[1].explicitModLines[1].range)
+	end)
+
 	it("preserves cluster jewel enchants from advanced copy", function()
 		newBuild()
 		runCallback("onFrame")

src/Classes/Item.lua

@@ -1181,6 +1181,7 @@ function ItemClass:ParseRaw(raw, rarity, highQuality)
 						line = advancedCopyLine:gsub("(%-?%d+%.?%d*)%(%-?%d+%.?%d*%-%-?%d+%.?%d*%)", "%1")
 					elseif bestPrecisionRange <= 1 and bestPrecisionRange >= 0 then
 						modLine.range = bestPrecisionRange
+						modLine.rangeFromValue = true
 					end
 				end
 				local rangedLine = itemLib.applyRange(line, 1, catalystScalar, modLine.corruptedRange)
@@ -1618,18 +1619,28 @@ function ItemClass:ParseRaw(raw, rarity, highQuality)
 		end
 	end
 	if self.base and self.base.enchant and #self.enchantModLines == 0 then
+		-- Items saved before their base gained this enchant carry it as an implicit (Black Maw Talisman)
+		local activeImplicitLines = { }
+		for _, modLine in ipairs(self.implicitModLines) do
+			if self:CheckModLineVariant(modLine) then
+				activeImplicitLines[modLine.line] = true
+			end
+		end
 		local enchantIndex = 1
 		for line in self.base.enchant:gmatch("[^\n]+") do
-			local modList, extra = modLib.parseMod(line)
-			t_insert(self.enchantModLines, {
-				line = line,
-				crafted = true,
-				implicit = true,
-				enchant = true,
-				extra = extra,
-				modList = modList or { },
-				modTags = self.base.enchantModTypes and self.base.enchantModTypes[enchantIndex] or { },
-			})
+			if not activeImplicitLines[line] then
+				local modList, extra = modLib.parseMod(line)
+				t_insert(self.enchantModLines, {
+					line = line,
+					crafted = true,
+					implicit = true,
+					enchant = true,
+					addedFromBase = true,
+					extra = extra,
+					modList = modList or { },
+					modTags = self.base.enchantModTypes and self.base.enchantModTypes[enchantIndex] or { },
+				})
+			end
 			enchantIndex = enchantIndex + 1
 		end
 	end

src/Classes/ItemsTab.lua

@@ -1218,6 +1218,11 @@ function ItemsTabClass:Load(xml, dbFileName)
 	self.itemSets = { }
 	self.itemSetOrderList = { }
 	self.tradeQuery.statSortSelectionList = { }
+	-- Enchants from the base data are only added to an item text without enchants and were not in the saved text,
+	-- so saved ModRange ids skip the whole list
+	local function countSavedModLines(modLines)
+		return modLines[1] and modLines[1].addedFromBase and 0 or #modLines
+	end
 	for _, node in ipairs(xml) do
 		if node.elem == "Item" then
 			local item = new("Item"):Item("")
@@ -1243,22 +1248,37 @@ function ItemsTabClass:Load(xml, dbFileName)
 				item.hasAltVariant5 = true
 				item.variantAlt5 = tonumber(node.attrib.variantAlt5)
 			end
+			local modRangeCount = 0
+			for _, child in ipairs(node) do
+				if type(child) == "table" and child.elem == "ModRange" then
+					modRangeCount = modRangeCount + 1
+				end
+			end
+			local ignoreModRanges
 			for _, child in ipairs(node) do
 				if type(child) == "string" then
 					item:ParseRaw(child)
-				elseif child.elem == "ModRange" then
+					-- Save writes at most one ModRange per mod line. More entries than lines means some saved lines are
+					-- no longer mod lines (e.g. Intangibility before 2.67), so the ids no longer match their lines and the
+					-- {range:} tags of the item text are kept instead
+					ignoreModRanges = modRangeCount > countSavedModLines(item.enchantModLines) + #item.scourgeModLines + #item.implicitModLines + #item.explicitModLines + #item.crucibleModLines
+				elseif child.elem == "ModRange" and not ignoreModRanges then
 					local id = tonumber(child.attrib.id) or 0
 					local range = tonumber(child.attrib.range) or 1
 					-- This is garbage, but needed due to change to separate mod line lists
 					-- 'ModRange' elements are legacy though, so is this actually needed? :<
 					-- Maybe it is? Maybe it isn't? Maybe up is down? Maybe good is bad? AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
 					-- Sorry, cluster jewels are making me crazy(-ier)
 					for _, list in ipairs{item.buffModLines, item.enchantModLines, item.scourgeModLines, item.implicitModLines, item.explicitModLines, item.crucibleModLines} do
-						if id <= #list then
-							list[id].range = range
+						local savedLineCount = countSavedModLines(list)
+						if id <= savedLineCount then
+							-- Versions that could not parse "value(min-max)" lines saved them as is with a default ModRange
+							if not list[id].rangeFromValue then
+								list[id].range = range
+							end
 							break
 						end
-						id = id - #list
+						id = id - savedLineCount
 					end
 				end
 			end