Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Negative Impale DPS on Dual Strike of Ambidexterity #8472

Open
3 tasks done
GoodOldMalk opened this issue Feb 13, 2025 · 1 comment · May be fixed by #8554
Open
3 tasks done

Negative Impale DPS on Dual Strike of Ambidexterity #8472

GoodOldMalk opened this issue Feb 13, 2025 · 1 comment · May be fixed by #8554

Comments

@GoodOldMalk
Copy link

Check version

  • I'm running the latest version of Path of Building and I've verified this by checking the changelog

Check for duplicates

  • I've checked for duplicate open and closed issues by using the search function of the issue tracker

Check for support

  • I've checked that the calculation is supposed to be supported. If it isn't please open a feature request instead (Red text is a feature request).

What platform are you running Path of Building on?

Windows

What is the value from the calculation in-game?

PoE does not report impale DPS.

What is the value from the calculation in Path of Building?

Currently when using Dual Strike of Ambidexterity with a non-zero chance to impale PoB reports a negative impale DPS number if certain conditions are met, usually when less than 100% chance to impale is present.

The problem seems to be around this line in CalcOffence.lua:

if skillFlags.attack and skillData.doubleHitsWhenDualWielding and skillFlags.bothWeaponAttack then
-- due to how its being combined
output.ImpaleModifier = output.ImpaleModifier / 2
end
output.ImpaleDPS = output.impaleStoredHitAvg * ((output.ImpaleModifier or 1) - 1) * output.HitChance / 100 * skillData.dpsMultiplier

In line 5545, if ImpaleModifier is smaller than 1 the DPS becomes negative.

I believe the problem results from the following. In line 5036:

output.ImpaleModifier = 1 + impaleDMGModifier

Adding 1 to impaleDMGModifier seems incorrect. The modifier can take values between 0 and 1, specially if the impale chance is lower than 100% and/or if not enough impale effect is stacked.

Similarly:

output.ImpaleDPS = output.impaleStoredHitAvg * ((output.ImpaleModifier or 1) - 1) * output.HitChance / 100 * skillData.dpsMultiplier

If adding 1 to impaleDMGModifier is incorrect, then substracting 1 from ImpaleModifier should also be incorrect.

Normally the above operations would not result in a noticeable effect, since adding 1 and substracting 1 cancel each other, but because Dual Strike of Ambidexterity has the flags doubleHitsWhenDualWielding and bothWeaponAttack, the modifier gets halved to account for the dual strike effect:

output.ImpaleModifier = output.ImpaleModifier / 2

This causes an imbalance in the equations and the negative DPS scenario.

I ignore what the correct calculation would look like, but if I had to guess it's probably better to calculate the output.ImpaleDPS for the main-hand and the off-hand separately, and if not possible, use the average between the main-hand and the off-hand.

How to reproduce the issue

No response

PoB for PoE1 build code

https://pobb.in/Rz44K7FGpysr

Screenshots

No response

@andyli00
Copy link
Contributor

andyli00 commented Mar 4, 2025

#8253 #8155 #7784 #5228

Found some potentially related issues regarding simultaneous attacks, will look into it further

As for this specific issue, it seems to be caused here:

if skillData.doubleHitsWhenDualWielding then
mainChance = mainChance / 10000
offChance = offChance / 10000
output[stat] = output.MainHand[stat] * mainChance + output.OffHand[stat] * offChance

When stat = ImpaleModifier, low enough impale chance (mainChance and offChance) can cause output[ImpaleModifier] to be lower than 1, which would be a negative multiplier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants