baaron4 / GW2-Elite-Insights-Parser

Binary parser for the .evtc files that arcdps generates after a boss encounter. This will generate a .html file where the results can be easily reviewed.
MIT License
133 stars 47 forks source link

Add secondary targets #925

Closed Zerthox closed 4 weeks ago

Zerthox commented 1 month ago

This adds some "progress-blocking" split phase DPS targets as secondary targets to the main phase (akin to Kaineng Overlook).

Fractals:

Raids:

Zerthox commented 1 month ago

One open question would be whether we should be merging "duplicate" targets, for example the 2 instances of Red/Green/Blue Guardian at Vale Guardian or Guldhem at Samarog.

And Hands at Adina would technically be progress-blocking DPS targets as well, but can spawn as non-blocking targets during the final phase. Not sure how we should handle that.

Linkaaaaa commented 1 month ago

Number them like we do for Tormented Deads, Anomalies and others

https://github.com/baaron4/GW2-Elite-Insights-Parser/blob/master/GW2EIEvtcParser/EncounterLogic/Raids/W5/SoullessHorror.cs#L127-L132

Maybe could make this slice of code an API since we do this for multiple encounters, don't know where to locate it though.

EliphasNUIT commented 1 month ago

From the code the only agents that would need renaming would be the Gorseval ones, others are already renamed with numbers or cardinal positions.

EliphasNUIT commented 1 month ago

For Adina, you could do a phases[0].AddSecondaryTargets(splitPhase.Targets) in GetPhases

Zerthox commented 1 month ago

That would mean they only get added to the main phase when phases are being computed, right? Is it fine to have different behavior for the main phase targets depending on whether other phases are computed?

EliphasNUIT commented 1 month ago

You are right, in that case something like var firstBelow25 = log.CombatData.GetHealthUpdateEvents(adina).FirstOrDefault(x => x.HPPercent <= 25) var mandatoryHandMaxTime = firstBelow25 ? firstBelow25.Time : log.FightData.FightEnd phases[0].addSecondaryTargets(Targets.Where(x => isHand && x.FirstAware <= mandatoryHandMaxTime));

Zerthox commented 1 month ago

Health check is a bit problematic since Adina can easily go below 25% before the 3rd split starts. In the log I am testing with she gains Determined nearly 1s after a < 25% HP update. And Bladesworn Slashes can hit for like 250k, good timing on one could push her below 24% going into the split.

Technically we could check < 23% as the Phase 4 Hands are not supposed to spawn before 20%. Alternatively I could try to do it based on Determined applies instead. They are needed for phase computation below anyway, only need to move it up.

Zerthox commented 1 month ago

Should I add the "optional" hands as secondary targets to Phase 4?

Edit: added it for now, can be removed if it's not wanted.

EliphasNUIT commented 1 month ago

Determined check could fail on late start logs though and add actual P4 hands as secondary targets to the main phase. If last Determined event is an apply, use log.FightData.FightEnd, if last determined event is a remove, us event.Time. That should be more robust and logStart agnostic. Edit: also use log.FightData.FightEnd if no determined event at all.

Zerthox commented 1 month ago

We can go back to checking <23% health if thats more robust?

EliphasNUIT commented 1 month ago

No, a buff based system will always be more robust, we only care for hands during determined periods, with how the fight works, we only want hands from before last determined remove or everything (if no remove or last determined event is an apply, aka log ended during split phase)

EliphasNUIT commented 4 weeks ago

Thanks!