Fix dual wield skills with "both weapons hit..." combining into a single hit#2006
Open
majochem wants to merge 4 commits into
Open
Fix dual wield skills with "both weapons hit..." combining into a single hit#2006majochem wants to merge 4 commits into
majochem wants to merge 4 commits into
Conversation
In PoE2 `doubleHitsWhenDualWielding` does not actually cause a combined hit from both weapons, but instead causes two separate near simultaneous hits. In order to still be able to reuse the old logic, I changed all checks for `doublenHitsWhenDualWielding` to `combinesHitsWhenDualWielding` instead. Support for the old flag will be in separate commit.
Now correctly reflects behavior of the three possible dual wield hitRate scenarios: 1. One combined hit from both weapons (Harmonic Mean) 2. Two separate simultaneous hits from each weapon (2x Harmonic Mean) 3. Alternating between Mainhand and Offhand (Harmonic Mean)
This is still not ideal imo, but it's mostly due to the fact that we don't have a good breakdown for "DPS" mods
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Description of the problem being solved:
PoB2 until now assumed that the
doubleHitsWhenDualWieldingbehaved similar to PoE1, where both weapons do a single combined hit. However, as confirmed by Viperesque from GGG and in-game testing, the skills actually perform two separate near simultaneous hits. As a result, Average Damage, Attack Rate and the Effective Crit Chance are all currently wrong (combined hit makes the hit count as critical, as long as one of the weapons crits)This PR:
doubleHitsWhenDualWieldingto instead perform two hitscombinesHitsWhenDualWielding. *(Note: There's currently no skill in the game that has such behavior, but it is likely to be included with future weapon types, so we will still need the logic)Steps taken to verify a working solution:
Link to a build that showcases this PR:
Somewhat random test build
Limitations:
"DPS"i.e. hit rate multiplier mods. The current one shows that it's multiplying the "Average Damage", but that value is also affected by "Eff. DPS Mod" like Armour, which is why the values don't add up properly. That should be its own separate PR though because it's not limited to dual wielding.Before screenshot:
Average hit wrongly combines damage
No DPS Multiplier
After screenshot:
Average hit corrected
New DPS Multiplier
Note: Hit DPS is lower because the individual hits are smaller and therefore more strongly affected by enemy Armour. Poison DPS is higher because hitting more often increases the "Stack Potential"