Fix Bleed/Ignite Stack potential issues #6386
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The config was not affecting the number of bleeds/ignites affecting the enemy and was instead only affecting the average roll % Ignite and Bleed roll % was not calculated to be at least 50% roll as a minimum Ignite was also not taking into account Hit Chance when calculating the average roll Breakdowns were improved to show the potential when max stacks is more than 1
Aryk-st
reviewed
Aug 17, 2023
| @@ -4125,20 +4136,26 @@ function calcs.offence(env, actor, activeSkill) | |||
| local igniteStacks = 1 | |||
| if not skillData.triggeredOnDeath then | |||
| if output.Cooldown then | |||
| igniteStacks = (globalOutput.IgniteDuration / m_max(output.Cooldown, (output.HitTime or output.Time)) * skillData.dpsMultiplier) / maxStacks | |||
| igniteStacks = ((output.HitChance / 100) * globalOutput.IgniteDuration / m_max(output.Cooldown, (output.HitTime or output.Time)) * skillData.dpsMultiplier) / maxStacks | |||
There was a problem hiding this comment.
I'm pretty sure dividing by maxStacks here and then calculating
local igniteRollAverage = igniteStacks / (igniteStacks + 1) * 100
is not correct.
If we apply 10 stacks per duration and have maximum stacks of 2, current calculations give
stack potential = 10 / 2 == 5
igniteRollAverage = 5/6 * 100 == 83.3%
while the correct value is the average of 10/11 and 9/10 == 90.45% -- two most powerful ignites currently.
EDIT: I see you clamped value of stacks to be 1 minimum which solves the following.
It also undervalues ignites with low uptime. If your ignite duration is 0.1s and you can apply 1 stack per second igniteStacks will be 0.1 resulting in average roll of 9% which is not true, average roll will be 50%
There was a problem hiding this comment.
Also was looking at #6272 and realized that igniteStacks here should also address ignite chance. Otherwise reducing ignite chance increases damage as potential stays unchanged and calcs think that more of ignites are from crits now
There was a problem hiding this comment.
I agree with your comments and will fix them for a future PR but I need to merge this now for the next update
LocalIdentity
added a commit
that referenced
this pull request
Aug 24, 2023
…ork code. (#6447) * Fix version number mishap * move timeless jewel stuff (#5747) * move timeless jewel stuff * add back all the local versions of functions for future use * Help section improvements (#6156) * increase size of box and text * implement hotkey to directly open help at a given section * add ability to "fast scroll" to next section * add section heights to changelog * update txt * fix merge issue * fix line overlap and add note about scrollbars * Add lot of stuff for ancestor league (#6288) * FEAT: impl explode and extra support mods * FEAT: impl new aura recovery mod * FEAT: impl between you and linked target mods * FEAT: impl Sentinel of Radiance mod Currently implemented as less damage taken as that's what the spreadsheet said. I think this may be possible to implement like frost shield or other "taken before you" mods given more info on stats of the sentinel. * FEAT: impl reserved mana as armor mod * FIX: use arbitrary taken before you instead of generic less dmg taken * FIX: cache result of dmg type max hit str concat * FEAT: code for generally supporting have no mods * FEAT: impl resistance conversion mods * FEAT: impl extra jewel func code for new chief mod * FEAT: impl mod on all other slots condition * FEAT: add new uniques * Wording fixes, new unique * Remove debugging lines * Adding fire mastery explosion mod * FIX: use ailment immunity flags from #6297 * Late breaking Chieftain change * Update mod ranges * Fix Amulet and belt --------- Co-authored-by: Wires77 <Wires77@users.noreply.github.com> Co-authored-by: LocalIdentity <31035929+LocalIdentity@users.noreply.github.com> * FEAT: add immunity flags to breakdown (#6389) * FIX: fix totem duration mods not applying (#6388) * Fix DPS on Vaal Flicker when using 2x 1h weapons (#6380) The block was inside the 2 pass loop so was running twice Also fixes the issue when you used a custom `more DPS` mod Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Apply RepeatCount according to their SkillTypes (#6376) * Apply RepeatCount according to their SkillTypes * Fix Vaal Flicker Strike --------- Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Fix Immunities breakdown * Fix Maata'a Teaching typo * Fix allocation issue on convert while maintaining node id/name filtering (#6364) * Fix lower channel time stat using red text (#6381) * Fix lower channel time stat using red text * Fix Channel time showing on calcs page for more skills than necessary --------- Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Fix "Enemies maimed by you take inc damage over time` not in breakdown (#6400) The mod was not displaying in the breakdown as it was using the dot flag instead of `DamageTakenOverTime` Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Fix Pierce and Chain count config not appearing sometimes (#6401) * Fix Pierced count config not appearing sometimes * Fix Chain config not appearing --------- Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Regen ModCache * Fix Snipe damage going negative (#6399) * FIX: snipe damage going negative * Set SnipeStacks min value earlier --------- Co-authored-by: LocalIdentity <31035929+LocalIdentity@users.noreply.github.com> * Null-check node.finalModList when hovering trees (#6408) Looks like node.finalModList is not initalized when hovering trees, this simply null checks it. Closes #6405 Signed-off-by: Tomas Slusny <slusnucky@gmail.com> * Update 3.22 tree (#6411) * update tree * Fix rewording on Chieftain Node --------- Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Fix Snipe showing DPS values when triggering support skills (#6415) Added damage was still contributing to the damage of the skill so it was showing DPS numbers even if the skill was actually dealing no damage Also removes the debug tooltip I forgot to remove from testing Co-authored-by: LocalIdentity <localidentity2@gmail.com> * 3.22 Additions (#6418) * FEAT: impl more sources of self hit dmg This code has been moved from #4599 after abandoning cwdt loop support. * FIX: spelling * FIX: remove file implemented in #4599 * Add support for non-Boneshatter Trauma * Add new gems from 3.22 and other stuff * Fix Crucible Export * Fix Ruthless support to work with all ailments * Update for actual support gem * Support inc stun duration per trauma * add devour, volatility and locus mine support (#6412) * add devour, volatility and locus mine support * Add full support for gems Add mine tag to Locus Mine Add support for Anomalous Devour Fix Mine PvP damage multiplier Fix flags for damage against full/low life enemies --------- Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Spellblade support (#6343) * Initial spellblade support * Update label for spellblade damage breakdown * Update with actual stats and qualities * Update .txt too * Floor all final flat values --------- Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Fix export * Add spacing and fix typos Alter the breakdowns to no longer show the damage taken multi/taken % line if the value has not changed Added spacing to the lines so you can more clearly read the breakdown when you have multiple taken as elements Fix some spacing and spelling mistakes --------- Co-authored-by: Paliak <91493239+Paliak@users.noreply.github.com> Co-authored-by: Lilylicious <lily.hammerskog@gmail.com> Co-authored-by: LocalIdentity <localidentity2@gmail.com> Co-authored-by: Regisle <49933620+Regisle@users.noreply.github.com> Co-authored-by: Lily <Lilylicious@users.noreply.github.com> * Regen ModCache * FIX: allow setting inspiration charges to 0 (#6421) * Add support for Guardian's Blessing no reservation for auras (#6425) Signed-off-by: Tomas Slusny <slusnucky@gmail.com> * FEAT: impl sadism less ailment duration mod (#6431) * FIX: incorrect increased usage mod range (#6434) * Implement Corrupting Cry support (#6436) * FEAT: impl Corrupting Cry support * FIX: crash caused by incorrect capitalization * FIX: add statMap and baseFlags to txt template * FIX: spelling * Revert "FEAT: impl Corrupting Cry support" * FIX: reimplement using generic physDot * FIX: copy paste issue * FIX: cap corrupting blood staged. Note that this config option also is affected by the one off bug in config tab code. * FIX: off by one in the config * FIX: don't override skillData.durationSecondary Overriding skillData.durationSecondary of exterted attacks may mess with secondary effect of skills such as Dominating Blow * FIX: CorruptingCry cfg stages not affecting exerts * FIX: crash when hovering over more mult * change dot type to corrupting blood to prevent dot from stacking * Remove Attack dot addition + move config * Fix removal of line * Empty --------- Co-authored-by: Paliak <91493239+Paliak@users.noreply.github.com> Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Add support for new Ancestor uniques (#6426) * Add mod text only Signed-off-by: Wires77 <Wires77@users.noreply.github.com> * Fix Level requirements and add Replica Dragonfang's Flight * Support Ahuana's Bite * Support Bound Fate * Support Kahuturoa's Certainty * Support Rakiata's Dance * Fix global gem level/quality name change --------- Signed-off-by: Wires77 <Wires77@users.noreply.github.com> Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Removing allocated mastery from hover list (#6374) * Removing allocated mastery from hover list * Updating wording on mastery --------- Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Add Ruthless tree (#6367) * Add support for Ruthless trees * Fix export URL for Ruthless trees * Removed data.json * Fix PR comments * Fix edge cases around converting and importing * Updating support for 3rd party ruthless tree links * Update tree and add Tattoos * Add support for new Ruthless tree mods * Fix comment --------- Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Support ruthless for poeskilltree * Optimize itemLib.applyRange, improve startup time (#6407) * Add applyRange tests * Optimize itemLib.applyRange, improve startup time * Fix tree import crash * Tattoo implementation (#6396) * Initial tattoo implementation * Tattoo spec update * Tattoo export * Update spec * Fix spec * New dat files * More fixes * Add artwork, restrict Makanga tattoos Signed-off-by: Wires77 <Wires77@users.noreply.github.com> * Fix bug with saving/loading, cleanup code * Fixed import bug and matched format for save files * Fixing logic issues preventing proper tattoos from appearing * Fixed bug with linked node count and supported new tattoo jewel * Temp fix for effectSprites * Can't tattoo changed nodes * Fix tattoo interaction with Timeless jewels * Update tree, fix more bugs * Add dropdown search * Add help text to tattoo dropdown * Fix stat description error * Dynamically set popup width * Made popup width fully dynamic * Truncate mod lines in tattoo popup * text wrap in popup * Dynamic word wrapping, ignoring GGG wrapping * Revert tree merge --------- Signed-off-by: Wires77 <Wires77@users.noreply.github.com> Co-authored-by: LocalIdentity <localidentity2@gmail.com> Co-authored-by: Peechey <92683202+Peechey@users.noreply.github.com> * Fix secondary effect skills being added as supports (#6393) * FIX:secondary effect skills being added as support * FIX: cleanup duplicate code * Fix Bleed/Ignite Stack potential issues (#6386) The config was not affecting the number of bleeds/ignites affecting the enemy and was instead only affecting the average roll % Ignite and Bleed roll % was not calculated to be at least 50% roll as a minimum Ignite was also not taking into account Hit Chance when calculating the average roll Breakdowns were improved to show the potential when max stacks is more than 1 Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Regen ModCache * Version dropdown QoL (#6365) * Version dropdown QoL * ruthless update * Add support for Fresh Meat Support (#6443) * Add support for Fresh Meat Support * Adding general support for increased minion critical strike chance * Move Config --------- Co-authored-by: LocalIdentity <31035929+LocalIdentity@users.noreply.github.com> * Implement Sacrifice, Frigid Bond, Guardian's Blessing, Controlled Blaze * Add initial support for Guardian Minions (#6445) Adds support for the new Guardian minions to show in the sidebar. The level is manually set to 85 right now as we don't support the `minion_actor_level_is_user_level_up_to_maximum` stat yet Co-authored-by: LocalIdentity <localidentity2@gmail.com> * Release 2.32.0 (#6440) * Prepare release 2.32.0 * Fix changelogs * Spelling --------- Co-authored-by: LocalIdentity <LocalIdentity@users.noreply.github.com> Co-authored-by: LocalIdentity <localidentity2@gmail.com> --------- Signed-off-by: Tomas Slusny <slusnucky@gmail.com> Signed-off-by: Wires77 <Wires77@users.noreply.github.com> Co-authored-by: Wires77 <Wires77@users.noreply.github.com> Co-authored-by: Regisle <49933620+Regisle@users.noreply.github.com> Co-authored-by: LocalIdentity <31035929+LocalIdentity@users.noreply.github.com> Co-authored-by: LocalIdentity <localidentity2@gmail.com> Co-authored-by: Peechey <92683202+Peechey@users.noreply.github.com> Co-authored-by: Tomas Slusny <slusnucky@gmail.com> Co-authored-by: Lilylicious <lily.hammerskog@gmail.com> Co-authored-by: Lily <Lilylicious@users.noreply.github.com> Co-authored-by: Lancej <1243476+Lancej@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: LocalIdentity <LocalIdentity@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The config was not affecting the number of bleeds/ignites affecting the enemy and was instead only affecting the average roll %
Ignite and Bleed roll % was not calculated to be at least 50% roll as a minimum
Ignite was also not taking into account Hit Chance when calculating the average roll
Breakdowns were improved to show the potential when max stacks is more than 1
Fixes #6366