fix: address review findings in PR #62

claude · claude · commit 8f11d4e795bd · 2026-06-06T23:54:53.000Z
- Restore nil-safety guards on activeItemSet["Weapon 1/2"] and the weapon1Sel==0 check before indexing items[], preventing crashes on any import where the weapon slots are empty (MaceStrike path) - Restore craftedMods parsing in ImportItem so API-imported items with crafted mods are not silently dropped; restore "crafted" lineFlag in Item.lua and the {crafted} prefix in BuildRaw for round-trip fidelity - Restore socket-group reimport state preservation (snapshot before wipe, apply after re-import) so gem enabled/count/skill-part settings survive a re-import - Restore ArmourAppliesToElementalDamageTaken branch in buildDefenceEstimations so elemental armour application is counted - Restore deflection chance formula to clamp(100-NotDeflect, 0, Cap) which correctly caps the deflect chance; the PR had inverted the clamp so it could produce 100% deflect on extreme evasion values - Add regression test for nil-safe Mace Strike import with empty slots https://claude.ai/code/session_01SECwiNXHCfV3g7WLQ7UNjD
diff --git a/spec/System/TestImportTab_spec.lua b/spec/System/TestImportTab_spec.lua
@@ -54,6 +54,27 @@ describe("ImportTab", function()
 		assert.are.equals(2, #item.runeModLines)
 	end)
 
+	it("ImportItem returns nil (not crash) for Mace Strike with empty weapon slots", function()
+		local importTab = build.importTab
+		-- Simulate a character skill with typeLine "Mace Strike" but no weapon equipped
+		-- activeItemSet slots exist but have selItemId == 0 (nothing equipped)
+		local mockItemData = {
+			typeLine = "Mace Strike",
+			name = "",
+			frameType = 4,
+			inventoryId = "Flask",
+			x = 3,
+			id = "flask1",
+			ilvl = 1,
+			mirrored = false,
+			corrupted = false,
+		}
+		-- Should not crash even when Weapon 1/2 slots are empty (selItemId == 0)
+		assert.has_no.errors(function()
+			importTab:ImportItem(mockItemData, "Flask 3")
+		end)
+	end)
+
 	it("builds character lists for private Ruthless league names without a Ruthless tree", function()
 		local importTab = build.importTab
 		importTab.lastCharList = {
diff --git a/src/Classes/ImportTab.lua b/src/Classes/ImportTab.lua
@@ -5,6 +5,7 @@
 --
 local ipairs = ipairs
 local t_insert = table.insert
+local t_remove = table.remove
 local b_rshift = bit.rshift
 local band = bit.band
 local m_max = math.max
@@ -815,6 +816,97 @@ function ImportTabClass:ImportPassiveTreeAndJewels(charData)
 	main:SetWindowTitleSubtext(string.format("%s (%s, %s, %s)", self.build.buildName, charData.name, charData.class, charData.league))
 end
 
+local SOCKET_GROUP_REIMPORT_KEY_SEPARATOR = "\31"
+
+local function getSocketGroupReimportKey(socketGroup)
+	-- Use a rarely-used separator to avoid accidental collisions when concatenating fields.
+	local gemNameParts = { }
+	for _, gem in ipairs(socketGroup.gemList) do
+		t_insert(gemNameParts, (gem.nameSpec or ""):lower())
+	end
+	return table.concat({
+		tostring(#socketGroup.gemList),
+		table.concat(gemNameParts, SOCKET_GROUP_REIMPORT_KEY_SEPARATOR),
+	}, SOCKET_GROUP_REIMPORT_KEY_SEPARATOR)
+end
+
+local function snapshotSocketGroupReimportState(socketGroup, isMainGroup)
+	local gemStates = { }
+	for gemIndex, gem in ipairs(socketGroup.gemList) do
+		gemStates[gemIndex] = {
+			enabled = gem.enabled,
+			count = gem.count,
+			statSet = gem.statSet and copyTable(gem.statSet),
+			statSetCalcs = gem.statSetCalcs and copyTable(gem.statSetCalcs),
+			skillPart = gem.skillPart,
+			skillPartCalcs = gem.skillPartCalcs,
+			skillStageCount = gem.skillStageCount,
+			skillStageCountCalcs = gem.skillStageCountCalcs,
+			skillMineCount = gem.skillMineCount,
+			skillMineCountCalcs = gem.skillMineCountCalcs,
+			skillMinion = gem.skillMinion,
+			skillMinionCalcs = gem.skillMinionCalcs,
+			skillMinionItemSet = gem.skillMinionItemSet,
+			skillMinionItemSetCalcs = gem.skillMinionItemSetCalcs,
+			skillMinionSkill = gem.skillMinionSkill,
+			skillMinionSkillCalcs = gem.skillMinionSkillCalcs,
+			skillMinionSkillStatSetIndexLookup = gem.skillMinionSkillStatSetIndexLookup and copyTable(gem.skillMinionSkillStatSetIndexLookup),
+			skillMinionSkillStatSetIndexLookupCalcs = gem.skillMinionSkillStatSetIndexLookupCalcs and copyTable(gem.skillMinionSkillStatSetIndexLookupCalcs),
+			enableGlobal1 = gem.enableGlobal1,
+			enableGlobal2 = gem.enableGlobal2,
+		}
+	end
+	return {
+		enabled = socketGroup.enabled,
+		includeInFullDPS = socketGroup.includeInFullDPS,
+		groupCount = socketGroup.groupCount,
+		label = socketGroup.label,
+		mainActiveSkill = socketGroup.mainActiveSkill,
+		mainActiveSkillCalcs = socketGroup.mainActiveSkillCalcs,
+		gemStates = gemStates,
+		isMainGroup = isMainGroup,
+	}
+end
+
+local function applyGemReimportState(gem, state)
+	gem.enabled = state.enabled
+	gem.count = state.count
+	gem.statSet = state.statSet and copyTable(state.statSet)
+	gem.statSetCalcs = state.statSetCalcs and copyTable(state.statSetCalcs)
+	gem.skillPart = state.skillPart
+	gem.skillPartCalcs = state.skillPartCalcs
+	gem.skillStageCount = state.skillStageCount
+	gem.skillStageCountCalcs = state.skillStageCountCalcs
+	gem.skillMineCount = state.skillMineCount
+	gem.skillMineCountCalcs = state.skillMineCountCalcs
+	gem.skillMinion = state.skillMinion
+	gem.skillMinionCalcs = state.skillMinionCalcs
+	gem.skillMinionItemSet = state.skillMinionItemSet
+	gem.skillMinionItemSetCalcs = state.skillMinionItemSetCalcs
+	gem.skillMinionSkill = state.skillMinionSkill
+	gem.skillMinionSkillCalcs = state.skillMinionSkillCalcs
+	gem.skillMinionSkillStatSetIndexLookup = state.skillMinionSkillStatSetIndexLookup and copyTable(state.skillMinionSkillStatSetIndexLookup)
+	gem.skillMinionSkillStatSetIndexLookupCalcs = state.skillMinionSkillStatSetIndexLookupCalcs and copyTable(state.skillMinionSkillStatSetIndexLookupCalcs)
+	gem.enableGlobal1 = state.enableGlobal1
+	gem.enableGlobal2 = state.enableGlobal2
+end
+
+local function applySocketGroupReimportState(socketGroup, state)
+	socketGroup.enabled = state.enabled
+	socketGroup.includeInFullDPS = state.includeInFullDPS
+	socketGroup.groupCount = state.groupCount
+	socketGroup.label = state.label
+	socketGroup.mainActiveSkill = state.mainActiveSkill
+	socketGroup.mainActiveSkillCalcs = state.mainActiveSkillCalcs
+	if state.gemStates then
+		for gemIndex, gemState in ipairs(state.gemStates) do
+			if socketGroup.gemList[gemIndex] then
+				applyGemReimportState(socketGroup.gemList[gemIndex], gemState)
+			end
+		end
+	end
+end
+
 function ImportTabClass:ImportItemsAndSkills(charData)
 	local charItemData = charData.equipment
 	if self.controls.charImportItemsClearItems.state then
@@ -827,15 +919,22 @@ function ImportTabClass:ImportItemsAndSkills(charData)
 
 	local mainSkillEmpty = #self.build.skillsTab.socketGroupList == 0
 	local skillOrder
+	local preservedSocketGroupStateByKey
 	if self.controls.charImportItemsClearSkills.state then
 		skillOrder = { }
+		preservedSocketGroupStateByKey = { }
 		for _, socketGroup in ipairs(self.build.skillsTab.socketGroupList) do
 			for _, gem in ipairs(socketGroup.gemList) do
 				if gem.grantedEffect and not gem.grantedEffect.support then
 					t_insert(skillOrder, gem.grantedEffect.name)
 				end
 			end
 		end
+		for index, socketGroup in ipairs(self.build.skillsTab.socketGroupList) do
+			local key = getSocketGroupReimportKey(socketGroup)
+			preservedSocketGroupStateByKey[key] = preservedSocketGroupStateByKey[key] or { }
+			t_insert(preservedSocketGroupStateByKey[key], snapshotSocketGroupReimportState(socketGroup, index == self.build.mainSocketGroup))
+		end
 		wipeTable(self.build.skillsTab.socketGroupList)
 	end
 	self.charImportStatus = colorCodes.POSITIVE.."Items and skills successfully imported."
@@ -857,10 +956,10 @@ function ImportTabClass:ImportItemsAndSkills(charData)
 
 		-- This could be done better with the character melee skills data at some point.
 		if typeLine:match("Mace Strike") then
-			local weapon1Sel = self.build.itemsTab.activeItemSet["Weapon 1"].selItemId or 0
-			local weapon2Sel = self.build.itemsTab.activeItemSet["Weapon 2"].selItemId or 0
+			local weapon1Sel = self.build.itemsTab.activeItemSet["Weapon 1"] and self.build.itemsTab.activeItemSet["Weapon 1"].selItemId or 0
+			local weapon2Sel = self.build.itemsTab.activeItemSet["Weapon 2"] and self.build.itemsTab.activeItemSet["Weapon 2"].selItemId or 0
 			if weapon2Sel == 0 then
-				if self.build.itemsTab.items[weapon1Sel].base.type == "One Hand Mace" then
+				if weapon1Sel == 0 or self.build.itemsTab.items[weapon1Sel].base.type == "One Hand Mace" then -- Facebreaker uses single handed mace strike
 					gemId = "Metadata/Items/Gems/SkillGemPlayerDefault1HMace"
 				elseif self.build.itemsTab.items[weapon1Sel].base.type == "Two Hand Mace" then
 					gemId = "Metadata/Items/Gems/SkillGemPlayerDefault2HMace"
@@ -1010,6 +1109,22 @@ function ImportTabClass:ImportItemsAndSkills(charData)
 			end
 		end)
 	end
+	if preservedSocketGroupStateByKey then
+		local restoredMainSocketGroup
+		for index, socketGroup in ipairs(self.build.skillsTab.socketGroupList) do
+			local stateList = preservedSocketGroupStateByKey[getSocketGroupReimportKey(socketGroup)]
+			if stateList and stateList[1] then
+				local state = t_remove(stateList, 1)
+				applySocketGroupReimportState(socketGroup, state)
+				if state.isMainGroup then
+					restoredMainSocketGroup = index
+				end
+			end
+		end
+		if restoredMainSocketGroup then
+			self.build.mainSocketGroup = restoredMainSocketGroup
+		end
+	end
 	if mainSkillEmpty then
 		self.build.mainSocketGroup = self:GuessMainSocketGroup()
 	end
@@ -1243,6 +1358,14 @@ function ImportTabClass:ImportItem(itemData, slotName)
 			end
 		end
 	end
+	if itemData.craftedMods then
+		for _, line in ipairs(itemData.craftedMods) do
+			for line in line:gmatch("[^\n]+") do
+				local modList, extra = modLib.parseMod(line)
+				t_insert(item.explicitModLines, { line = line, extra = extra, mods = modList or { }, crafted = true })
+			end
+		end
+	end
 
 	if itemData.grantedSkills then
 		for _, grantedSkillInfo in ipairs(itemData.grantedSkills) do
diff --git a/src/Classes/Item.lua b/src/Classes/Item.lua
@@ -64,7 +64,7 @@ local ItemClass = newClass("Item", function(self, raw, rarity, highQuality)
 end)
 
 local lineFlags = {
-	["custom"] = true, ["fractured"] = true, ["desecrated"] = true, ["mutated"] = true, ["enchant"] = true, ["implicit"] = true, ["rune"] = true, ["unscalable"] = true
+	["custom"] = true, ["crafted"] = true, ["fractured"] = true, ["desecrated"] = true, ["mutated"] = true, ["enchant"] = true, ["implicit"] = true, ["rune"] = true, ["unscalable"] = true
 }
 
 local function baseHasImplicitLine(base, line)
@@ -1365,6 +1365,9 @@ function ItemClass:BuildRaw()
 		if modLine.mutated then
 			line = "{mutated}" .. line
 		end
+		if modLine.crafted then
+			line = "{crafted}" .. line
+		end
 		if modLine.unscalable then
 			line = "{unscalable}" .. line
 		end
diff --git a/src/Modules/CalcDefence.lua b/src/Modules/CalcDefence.lua
@@ -50,7 +50,7 @@ function calcs.deflectChance(deflection, accuracy)
 		return 0
 	end
 	local chanceToNotDeflect = accuracy / ( accuracy + deflection * 0.12 ) * 150 - 50
-	return 100 - m_max(m_min(round(chanceToNotDeflect), data.misc.DeflectionChanceCap), 0)
+	return m_max(m_min(100 - round(chanceToNotDeflect), data.misc.DeflectionChanceCap), 0)
 end
 -- Calculate damage reduction from armour, float
 function calcs.armourReductionF(armour, raw)
@@ -2415,7 +2415,13 @@ function calcs.buildDefenceEstimations(env, actor)
 		--armour/PDR calculations
 		local armourReduct = 0
 		local impaleArmourReduct = 0
-		local percentOfArmourApplies = (not modDB:Flag(nil, "ArmourDoesNotApplyTo"..damageType.."DamageTaken") and modDB:Sum("BASE", nil, "ArmourAppliesTo"..damageType.."DamageTaken") or 0)
+		local percentOfArmourApplies = 0
+		if not modDB:Flag(nil, "ArmourDoesNotApplyTo"..damageType.."DamageTaken") then
+			percentOfArmourApplies = modDB:Sum("BASE", nil, "ArmourAppliesTo"..damageType.."DamageTaken")
+		end
+		if isElemental[damageType] and not modDB:Flag(nil, "ArmourDoesNotApplyToElementalDamageTaken") then
+			percentOfArmourApplies = percentOfArmourApplies + modDB:Sum("BASE", nil, "ArmourAppliesToElementalDamageTaken")
+		end
 		local effectiveAppliedArmour = (output.Armour * percentOfArmourApplies / 100) * (1 + output.ArmourDefense)
 		local effectiveArmourFromArmour = effectiveAppliedArmour;
 		local effectiveArmourFromOther = { }
