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

Fix new trigger calculation #6581

Conversation

ProphetLamb
Copy link
Contributor

Fixes #6492.
@Paliak Please verify my Lua <3
@KonstantinosKoubetsos I will create another branch with the simulation method we discussed and link the draft later.

Description of the problem being solved:

The previous heuristic used for computing trigger rates was wrong. This replicates the same behaviour as the old by tick simulation.

Steps taken to verify a working solution:

  • Loaded up a CoC build
  • Verified the Triggerrates

Link to a build that showcases this PR:

Before screenshot:

After screenshot:

  • No visual changes

@ProphetLamb ProphetLamb marked this pull request as draft September 2, 2023 16:44
@Paliak
Copy link
Contributor

Paliak commented Sep 3, 2023

Quite busy with irl responsibilities, sorry.

Playing with the numbers a bit they seem odd. Though the issue i mentioned in the thread seems to be fixed. This may be solved by the unaligned version.

New sim:
obraz

Old sim:
obraz

This assumes the old sim is correct. The code itself looks fine. I played around with the python sim and the issue seems related to the rather sizable flat steps on the plot. Increasing the count doesn't seem to change those much.

@Paliak
Copy link
Contributor

Paliak commented Jul 21, 2024

#7244 is what i ended up going with. I really appreciate the help. Doubt i'd be able to reason about this problem without your help.

@Paliak Paliak closed this Jul 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug: calculation Numerical differences enhancement New feature, calculation, or mod
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cast on Critical Strike Trigger Rate being calculated incorrectly
2 participants