Add test + fix interaction with bifurcated crits

Author: LocalIdentity

spec/System/TestAttacks_spec.lua

@@ -114,7 +114,7 @@ describe("TestAttacks", function()
 		runCallback("OnFrame")
 		build.calcsTab:BuildOutput()
 		runCallback("OnFrame")
-		
+
 		-- 1: Get base damage with no crits
 		local critChance = 0
 		local critMult = 2
@@ -180,7 +180,7 @@ describe("TestAttacks", function()
 		runCallback("OnFrame")
 		build.calcsTab:BuildOutput()
 		runCallback("OnFrame")
-		
+
 		-- 1: Get base damage with no crits
 		local critChance = 0.0
 		local critMult = 2
@@ -202,10 +202,10 @@ describe("TestAttacks", function()
 		critChance = 0.1
 		local nonCritChance = 1 - critChance
 
-		local critBonusMultiplier = 
-			1 * critChance 
-			+ .7 * nonCritChance * critChance 
-			+ .4 * nonCritChance * nonCritChance * critChance 
+		local critBonusMultiplier =
+			1 * critChance
+			+ .7 * nonCritChance * critChance
+			+ .4 * nonCritChance * nonCritChance * critChance
 			+ .1 * nonCritChance * nonCritChance * nonCritChance * critChance
 
 		-- When adding them as MORE mods, they get auto rounded after *100, so we need to do the same
@@ -218,4 +218,68 @@ describe("TestAttacks", function()
 		local forcedExpectedAvgHit = averageHit * critMult
 		assert.are.equals(forcedExpectedAvgHit, build.calcsTab.mainOutput.MainHand.AverageHit)
 	end)
-end)
+
+	it("does not force critical hits when critical hit chance is zero", function()
+		build.itemsTab:CreateDisplayItemFromRaw([[
+			New Item
+			Heavy Bow
+			Quality: 0
+			-100% increased critical hit chance
+			-100% increased physical damage
+			adds 1 to 1 physical damage to attacks
+			nearby enemies have 100% less armour
+			nearby enemies have 100% less evasion
+		]])
+		build.itemsTab:AddDisplayItem()
+		runCallback("OnFrame")
+
+		build.configTab.input.customMods = "inevitable critical hits"
+		build.configTab:BuildModList()
+		runCallback("OnFrame")
+		build.calcsTab:BuildOutput()
+		runCallback("OnFrame")
+
+		assert.are.equals(0, build.calcsTab.mainOutput.MainHand.CritChance)
+		assert.are.equals(1, build.calcsTab.mainOutput.MainHand.AverageHit)
+	end)
+
+	it("correctly calculates forced outcome with bifurcated critical hits", function()
+		build.itemsTab:CreateDisplayItemFromRaw([[
+			New Item
+			Heavy Bow
+			Quality: 0
+			-100% increased critical hit chance
+			-100% increased physical damage
+			adds 1 to 1 physical damage to attacks
+			nearby enemies have 100% less armour
+			nearby enemies have 100% less evasion
+		]])
+		build.itemsTab:AddDisplayItem()
+		runCallback("OnFrame")
+
+		build.configTab.input.customMods = [[
+			+10% to critical hit chance
+			inevitable critical hits
+			bifurcates critical hits
+		]]
+		build.configTab:BuildModList()
+		runCallback("OnFrame")
+		build.calcsTab:BuildOutput()
+		runCallback("OnFrame")
+
+		local critChance = 0.1
+		local failedStageChance = (1 - critChance) ^ 2
+		local critBonusMultiplier = 2 * critChance * (
+			1
+			+ .7 * failedStageChance
+			+ .4 * failedStageChance ^ 2
+			+ .1 * failedStageChance ^ 3
+		)
+		critBonusMultiplier = math.floor(critBonusMultiplier * 100 + 0.5) / 100
+
+		assert.are.equals(100, build.calcsTab.mainOutput.MainHand.CritChance)
+		local bifurcateChance = (critChance * 100) ^ 2 / ((1 - failedStageChance) * 100)
+		assert.is_true(math.abs(bifurcateChance - build.calcsTab.mainOutput.MainHand.CritBifurcates) < 0.000001)
+		assert.are.equals(1 + critBonusMultiplier, build.calcsTab.mainOutput.MainHand.AverageHit)
+	end)
+end)

src/Modules/CalcOffence.lua

@@ -3340,6 +3340,7 @@ function calcs.offence(env, actor, activeSkill)
 		end
 
 		-- Calculate crit chance, crit multiplier, and their combined effect
+		local forcedOutcomeApplies = false
 		if skillModList:Flag(cfg, "NeverCrit") then
 			output.PreEffectiveCritChance = 0
 			output.CritChance = 0
@@ -3429,10 +3430,11 @@ function calcs.offence(env, actor, activeSkill)
 				if env.mode_effective and skillModList:Flag(cfg, "BifurcateCrit") then
 					output.CritChance = (1 - (1 - output.CritChance / 100) ^ 2) * 100
 				end
-				if env.mode_effective and skillModList:Flag(cfg, "ForcedOutcome") then
-					-- Lucky crits: not considered a 'reroll' for the sake of forced outcome penalties, just a higher crit chance
-					-- Bifurcated crits: only considered a 'reroll' if both of them fail to crit
-					-- So just use the crit chance as calculated, and it already works with both of them
+				local preForcedOutcomeCritChance = output.CritChance
+				if env.mode_effective and skillModList:Flag(cfg, "ForcedOutcome") and output.CritChance > 0 then
+					forcedOutcomeApplies = true
+					-- Lucky crits use their effective crit chance without an extra roll-down penalty.
+					-- Bifurcated crits roll twice per roll-down; only both rolls failing advances the penalty.
 					local critChance = output.CritChance / 100
 					local nonCritChance = 1 - critChance
 
@@ -3441,6 +3443,12 @@ function calcs.offence(env, actor, activeSkill)
 						0.7 * nonCritChance * critChance +				  -- 70% if we roll non-crit then a crit
 						0.4 * m_pow(nonCritChance, 2) * critChance + -- 40% if we roll two non-crit then a crit
 						0.1 * m_pow(nonCritChance, 3) * critChance   -- 10% if we roll three non-crits then a crit
+					if skillModList:Flag(cfg, "BifurcateCrit") then
+						-- The terminating stage can crit twice, so scale its one-or-more-crit bonus
+						-- by the expected number of crits on that stage.
+						critBonusMultiplier = critBonusMultiplier * 2 * output.PreBifurcateCritChance / output.CritChance
+						output.CritBifurcates = output.PreBifurcateCritChance ^ 2 / output.CritChance
+					end
 
 					-- This gets rounded when used in damage logic, so round it ahead of time to make the breakdown accurate (and less ugly)
 					local lessCritBonus = round((1 - critBonusMultiplier) * -100.0, 0)
@@ -3484,9 +3492,9 @@ function calcs.offence(env, actor, activeSkill)
 					if env.mode_effective and skillModList:Flag(cfg, "BifurcateCrit") then
 						t_insert(breakdown.CritChance, "Critical Strike Bifurcates:")
 						t_insert(breakdown.CritChance, s_format("1 - (1 - %.4f) x (1 - %.4f)", preBifurcateCritChance / 100, preBifurcateCritChance / 100))
-						t_insert(breakdown.CritChance, s_format("= %.2f%%", output.CritChance))
+						t_insert(breakdown.CritChance, s_format("= %.2f%%", preForcedOutcomeCritChance))
 					end
-					if env.mode_effective and skillModList:Flag(cfg, "ForcedOutcome") then
+					if forcedOutcomeApplies then
 						t_insert(breakdown.CritChance, "Inevitable Critical Hits (Forced Outcome):")
 						t_insert(breakdown.CritChance, "= 100% ^8(override)")
 					end
@@ -3508,7 +3516,7 @@ function calcs.offence(env, actor, activeSkill)
 
 				output.PreEffectiveCritMultiplier = 1 + extraDamage
 				-- if crit bifurcates are enabled, roll for crit twice and add multiplier for each
-				if env.mode_effective and skillModList:Flag(cfg, "BifurcateCrit") then
+				if env.mode_effective and skillModList:Flag(cfg, "BifurcateCrit") and not forcedOutcomeApplies then
 					-- get crit chance and calculate odds of critting twice
 					local critChancePercentage = output.PreBifurcateCritChance
 					local bifurcateMultiChance = (critChancePercentage ^ 2) / 100