Seperate skill cooldown from trigger cooldown and fix multiple related issues.

[Paliak]

Fixes #2444 , #4107 , #4210 , #3558, #4959, #4679

Description of the problems being solved:

Separation of Skill cooldown and trigger cooldown in CalcPerform. Correct calculation of triggers for Kitava's Thirst and Craft trigger with improved source selection for the latter. Change Kitava's Thirst to use the in game trigger mod and add parsing for it. Improved handling of hard skill cooldown overrides. Improved handling of a gem being supported by more than one trigger. Adds isTriger property to skills that can trigger other skills. Adds triggeredBy property to skills triggered. Handling of helmet focus trigger (focus trigger now assumes the player recasts focus when the cooldown ends). Correctly account for the fact that some skills have no cooldown in calcMultiSpellRotationImpact. Attack rate calculation for counter attack skills. Fix handling of hard skill cooldown overrides (Discharge). Add support for more unique triggers. Refactoring of trigger code. Improved breakdowns for skills where average trigger rate is affected by accuracy and or crit chance.

The changes are extensive and there may be more edge cases that i was not able to find.

Rundown:

There are flags and properties scattered all over, but the main code starts in CalcActiveSkill.lua

PathOfBuilding/src/Modules/CalcActiveSkill.lua

Lines 451 to 460 in e7fccb1

	-- Handle multiple triggers situation and if triggered by a trigger skill save a reference to the trigger.
	local match = skillEffect.grantedEffect.addSkillTypes and activeSkill.skillTypes[SkillType.Triggerable] and (not skillFlags.disable)
	if match and skillEffect.grantedEffect.isTrigger then
	if activeSkill.triggeredBy then
	skillFlags.disable = true
	activeSkill.disableReason = "This skill is supported by more than one trigger"
	else
	activeSkill.triggeredBy = skillEffect
	end
	end

Currently, pob inherits the cooldown from supports when a skill has no cooldown it self. For example, if you link arc to Cast on Crit arc will inherit the cooldown of cast on crit. Later in the code this looks exactly like any other cooldown; Therefore, making it impossible to tell where that cooldown came from (skill itself ? support? cooldown override (discharge)?) This caused issues with cooldown manipulation such as in the case of discharge where the cooldown of the skill itself needs to be overwritten but because both the trigger cooldown and cooldown of the skill get effectively max() 'ed there's just no way to do it. This also cases issues when trying to simulate the impact of skill rotation. If multiple skills are linked to a trigger they will be fired off sequentially, but if a skill happens to be on cooldown when the trigger tries to fire it off nothing will happen. @Nostrademous created the calcMultiSpellRotationImpact to simulate this so more accurate dps numbers can be provided. The function itself works great, but it was being fed the wrong numbers. You can see the difference that makes in the comment @QuickStick123 has made. It was also missing support for newer trigger mechanics such as lightning conduit adding cast time to cooldown if triggered, and it didn't take cooldown overrides into account. The heart of the code starts where it used to. I didn't touch the mirage archer handling code much, if at all. Simply adapted it to work with the changes I made. Lower starts special handling for the focus trigger. Because it uses the focus mechanic and can trigger all linked skills at the same time getting it to work with the rest of the trigger handling code was difficult, so I decided to make a special if case for it. Going lower, there's the special handling for cast while channeling. Cast while channeling differs from other triggers as it casts the linked skills on a monotonic interval. This means that normal trigger rules don't apply, and thus it has its own section. Just below that is the main trigger handling code. This handles the majority of "normal" triggers. Currently, in pob each trigger has its own section, but because I wanted to add support for more triggers adding a section for every single one quickly turned into copy pasting dozens of lines of code and every small change i made had to be applied and adjusted to every single code block. I decided to create a single block but split it into segments, flexible enough to handle the majority of cases. Stating off, I define some local variables that allow handling specific cases and abstract away some parts to make handling cases such as minion triggers (holy relic) easier.

PathOfBuilding/src/Modules/CalcPerform.lua

Lines 2848 to 2862 in e7fccb1

	local uniqueTriggerName = getUniqueItemTriggerName(env.player.mainSkill)
	local triggerName = uniqueTriggerName
	local skip = false
	local spellCount = {}
	local trigRate = 0
	local source = nil
	local triggerChance = env.player.mainSkill.activeEffect and env.player.mainSkill.activeEffect.srcInstance and env.player.mainSkill.activeEffect.srcInstance.triggerChance
	local globalTrigger = false
	local actor = env.player
	local output = output
	local breakdown = breakdown
	local minion = false
	local triggerSkillCond = nil
	local triggeredSkillCond = nil
	local assumingEveryHitKills = nil

each skill has different requirements for a triggering skill and can trigger different skills, so it sets different triggerSkillCond and triggerSkillCond functions that are later used to find and select valid skills. Some triggers also need special handling, such as arcanist brand or global triggers, such as cast on stun or cwdt. If the current skill is not a trigger skill the skip flag is set then CalcPerform goes on to do other processing but if a trigger skill is found then first the affected skills need to be found.

PathOfBuilding/src/Modules/CalcPerform.lua

Lines 3079 to 3097 in e7fccb1

	--find trigger skill and triggered skills
	if triggeredSkillCond or triggerSkillCond then
	for _, skill in ipairs(env.player.activeSkillList) do
	local triggered = skill.skillData.triggeredByUnique or skill.skillData.triggered or skill.skillTypes[SkillType.InbuiltTrigger] or skill.skillTypes[SkillType.Triggered]
	if triggerSkillCond and triggerSkillCond(env, skill) and not triggered then
	source, trigRate = findTriggerSkill(env, skill, source, trigRate)
	end
	if triggeredSkillCond and triggeredSkillCond(env,skill) and spellCount ~= nil then
	local addsCastTime = nil
	if skill.skillModList:Flag(skill.skillCfg, "SpellCastTimeAddedToCooldownIfTriggered") then
	baseCastTime = skill.skillData.castTimeOverride or skill.activeEffect.grantedEffect.castTime or 1
	local inc = skill.skillModList:Sum("INC", skill.skillCfg, "Speed")
	local more = skill.skillModList:More(skill.skillCfg, "Speed")
	addsCastTime = baseCastTime / round((1 + inc/100) * more, 2)
	end
	t_insert(spellCount, { uuid = cacheSkillUUID(skill), cd = skill.skillData.cooldown, cdOverride = skill.skillModList:Override(skill.skillCfg, "CooldownRecovery"), addsCastTime = addsCastTime})
	end
	end
	end

This loop uses the functions defined by each trigger to find an appropriate triggering source and to find all other skills that are triggered, so they can be fed into the skill rotation simulation function. Going lower, some validation takes place and the main trigger processing code starts.

PathOfBuilding/src/Modules/CalcPerform.lua

Lines 3110 to 3119 in e7fccb1

	if source and (source.skillTypes[SkillType.Melee] or source.skillTypes[SkillType.Damage] or source.skillTypes[SkillType.Attack]) then
	sourceAPS, dualWield = calcDualWieldImpact(env, trigRate, source.skillData.doubleHitsWhenDualWielding)
	end
	--Account for source unleash
	if GlobalCache.cachedData["CACHE"][uuid] and source.skillModList:Flag(nil, "HasSeals") and source.skillTypes[SkillType.CanRapidFire] then
	sourceAPS = sourceAPS * (GlobalCache.cachedData["CACHE"][uuid].ActiveSkill.skillData.dpsMultiplier or 1)
	env.player.mainSkill.skillFlags.HasSeals = true
	source.skillData.hasUnleash = GlobalCache.cachedData["CACHE"][uuid].ActiveSkill.skillData.dpsMultiplier
	end

First source attack/cast speed modifiers are applied and later the main function that calculates trigger speed is called calcActualTriggerRate. This function, along with getTriggerRateCap and calcMultiSpellRotationImpact calculates the raw trigger rate. It takes into account cooldown recovery and other factors that influence trigger speed. A good chunk of those functions is dedicated to generating a decent looking breakdown for the user as there are so many edge cases.

TODO:

• Implement proper handling for cwdt loops.

Steps taken to verify a working solution:

• Check builds from poe.ninja
• Test build: https://pobb.in/GM-W5q3HkJq7
• Test build2 https://pobb.in/9ADoZbUn-lQK

Live vs pr side by side pictures:

[Paliak]

Found issue. When there's no triggering skill the triggered skill is assumed to be cast by the trigger not self cast. Fixing.

Fixed by: #5b6ebd9e143a8b7540dd2fb95aa454ca24d81ba7

[Paliak]

Cast on melee kill and cast on stun not working correctly. Should behave closer to CWDT. Cast on Damage Taken currently doesn't take skill rotation into account. Fixing.

Should be Fixed.

Basic support for global triggers added.

[QuickStick123]

This skill isn't really implemented.


This skill is supposed to trigger Tawhoa's Chosen which summon's a Mirage Chieftain that attacks once with the given slam that triggered it with no echo/repeat.

#4732 Will fix this crash by dropping support.

[Paliak]

I've moved trigger related code to a new file CalcTriggers.lua.

Some things are likely still broken but i've tested everything that makes sense to implement from https://www.poewiki.net/wiki/Trigger and everything should behave correctly.