[UI] Results table refactor

[1337LutZ]

• Tweaked the Results UI to give a clearer overview and hide data in tooltips
• Added SpellSchool for spell coloring (and multi spell)
  • I don't have the knowledge to refactor Backend to use the shared Proto and migrate the current Encounter specific SpellSchool proto. (Imho can be done in a later PR)
• Add CritDamage, CritHealing, Ticks, CritTicks, TickDamage, CritTickDamage, GlanceDamage, BlockDamage, CritBlockDamage
• Add missing pet names for ActionId matching
• Moved all Threat columns under their specific "regular" metrics into a tooltip
• Removed obsolete DPASP metric
• Fixed CastTime metric for Channeled spells

Todo

• Create a "compound pet metric" to allow Pet (summon) casts to be separated from the actions to accurately show Avg Hit/Casts and DPET for 1 pet summon.

Screenshots

Tank

DPS

[1337LutZ]

@rosenrusinov In the code I saw that Bloodworms and Rune of the Fallen Crusader can't crit their heals, however they can;

• https://classic.warcraftlogs.com/reports/wAtVXFd6YbhmLWya#boss=-2&difficulty=0&wipes=2&type=healing&source=37
• https://classic.warcraftlogs.com/reports/wAtVXFd6YbhmLWya#boss=-2&difficulty=0&wipes=2&type=healing&source=16

Can I just add the default crit modifier and OutcomeHealingCrit or is this more involved?

[rosenrusinov]

@rosenrusinov In the code I saw that Bloodworms and Rune of the Fallen Crusader can't crit their heals, however they can;

• https://classic.warcraftlogs.com/reports/wAtVXFd6YbhmLWya#boss=-2&difficulty=0&wipes=2&type=healing&source=37
• https://classic.warcraftlogs.com/reports/wAtVXFd6YbhmLWya#boss=-2&difficulty=0&wipes=2&type=healing&source=16

Can I just add the default crit modifier and OutcomeHealingCrit or is this more involved?

If they work with the standard spell crit when they roll for their critical sure. If not they will require their own implementations of the outcome to calculate the crit. I havent done any testing but from taking a look at the 2 links you shared it doesnt seem to be using the DKs crit chance for them.

[rosenrusinov]

Refactor/Remove topline bar as it shows data that is shown in the left sidebar

This is not true for the Raid Sim, where you open individual sims without a sidebar and that information is visible only there

[rosenrusinov]

I like how this looks, only thing I don't like is the DPS column. I kinda like the full number being shown there instead of it being abbreviated with a K when above a thousand. It has the same length and just makes the information have 2 visual states instead of just 1 (abbreviated and non abbreviated). I think we should revert to just showing the full dps number.

[1337LutZ]

@rosenrusinov In the code I saw that Bloodworms and Rune of the Fallen Crusader can't crit their heals, however they can;

• https://classic.warcraftlogs.com/reports/wAtVXFd6YbhmLWya#boss=-2&difficulty=0&wipes=2&type=healing&source=37
• https://classic.warcraftlogs.com/reports/wAtVXFd6YbhmLWya#boss=-2&difficulty=0&wipes=2&type=healing&source=16

Can I just add the default crit modifier and OutcomeHealingCrit or is this more involved?

If they work with the standard spell crit when they roll for their critical sure. If not they will require their own implementations of the outcome to calculate the crit. I havent done any testing but from taking a look at the 2 links you shared it doesnt seem to be using the DKs crit chance for them.

Yea, it seemed that the crit didn't match the default crit / reflected the crit of the ability itself.

[1337LutZ]

I like how this looks, only thing I don't like is the DPS column. I kinda like the full number being shown there instead of it being abbreviated with a K when above a thousand. It has the same length and just makes the information have 2 visual states instead of just 1 (abbreviated and non abbreviated). I think we should revert to just showing the full dps number.

I've un-compacted the DPS/HPS/TDPS values 👍

[1337LutZ]

@rosenrusinov In the code I saw that Bloodworms and Rune of the Fallen Crusader can't crit their heals, however they can;

• https://classic.warcraftlogs.com/reports/wAtVXFd6YbhmLWya#boss=-2&difficulty=0&wipes=2&type=healing&source=37
• https://classic.warcraftlogs.com/reports/wAtVXFd6YbhmLWya#boss=-2&difficulty=0&wipes=2&type=healing&source=16

Can I just add the default crit modifier and OutcomeHealingCrit or is this more involved?

If they work with the standard spell crit when they roll for their critical sure. If not they will require their own implementations of the outcome to calculate the crit. I havent done any testing but from taking a look at the 2 links you shared it doesnt seem to be using the DKs crit chance for them.

Unholy Strength seems to match the player crit in logs and I added it like that for now.

Blood worms range between 2.5-3.5% crit chance, so I think we can either create a variance for this to end up around 3% if needed (this is how it looks in logs, there's not a real correlation between stats or other pets).

[1337LutZ]

Added

• Glancing blow damage
• Block damage
• Critical Block damage / hits
• Normalized Casts tooltip (Hits + Misses)
• Normalized Missed tooltip

[InDebt]

So let me preface this with: I love this! 😍

Soo I checked this out and I am kind of lost with MF. It might very well be that there is some stuff that needs to be adjusted core side but right now this looks like this.

I have three different crit chances shown to me and somehow Average Tick damage is higher than Average Crit damage 🤔 Soo is it calculating as AverageDamage * %? At least right now it does not seem intuitive to me.

[1337LutZ]

@InDebt I believe I found the issue for the crit %. Mind flay has a CalcOutcome OutcomeMagicHitAndCrit which I missed which adds Hits++, this should be OutcomeMagicHitAndCritNoHitCounter to prevent it act like a instant + Dot spell. Thanks for finding this.

[1337LutZ]

@InDebt You were also right about the Crit averages, I was taking the Avg based off the avgDamage, I fixed it for all the types of hits in the Damage done breakdown.

[InDebt]

Soo the things you fixed work very nice now! There is something else though I noticed.

Not sure why it seems to only effect the Darkmoon Card: Volcano but the damage in the timeline / log is wrong. I guess only there as you say there are not actual test differences. It seems the 'resist' is calculated wrong here. 0% resist is treated as 0% damage done and 10% resist as 10% damage done.

I do not see this behaviour in the current live sim, so I guess this is part of theses changes?

[InDebt]

Issue has been resolved. Was a migration issue of having the stat refactor branch running before I checked this PR.