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

Critical cull chance uses hit rate #5378

Merged

Conversation

andrewbelu
Copy link
Contributor

@andrewbelu andrewbelu commented Dec 16, 2022

Description of the problem being solved:

Critical cull chance was using just critical strike chance before. This should be based on hit rate because we are measuring DPS rather than a single hit.

Steps taken to verify a working solution:

Ensure that culling critical strikes take hit rate into account

  • Ensured DPS increases on Assassin's Ambush and Assassinate and Marylene's Fallacy
  • Ensured max cull is used appropriately with critical culling strike (with Culling Strike gem and Slayer's Headsman)

Link to a build that showcases this PR:

https://pastebin.com/C97LKLP7

Before screenshot:

image

After screenshot:

image

@andrewbelu andrewbelu changed the title critical cull chance uses hit rate Critical cull chance uses hit rate Dec 16, 2022
@QuickStick123 QuickStick123 added the enhancement New feature, calculation, or mod label Dec 17, 2022
@LocalIdentity LocalIdentity merged commit 25f4270 into PathOfBuildingCommunity:dev Dec 19, 2022
Dullson pushed a commit to Dullson/PathOfBuilding that referenced this pull request Dec 26, 2022
* critical cull chance uses hit rate

* Move lines

Co-authored-by: LocalIdentity <localidentity2@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature, calculation, or mod
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants