Hekili Priority Helper

Hekili Priority Helper

44M Downloads

[REC] Discipline Priest - Penance and Dark Reprimand don't count as the same spell for events like duration_expected

taherbert opened this issue ยท 7 comments

commented

Before You Begin

  • I confirm that I have downloaded the latest version of the addon.
  • I am not playing on a private server.
  • I checked for an existing, open ticket for this issue and was not able to find one.
  • I edited the title of this issue (above) so that it describes the issue I am reporting.
  • I am reporting an issue with the default priority included with the specialization (imported or edited priorities are not supported).

Describe the Issue

When an APL evaluates penance for duration_expected or remains_expected, the values generated are incorrect. After some investigation, I believe this has to do with penance and dark reprimand being considered as separate cooldowns, rather than the same cooldown.

When casting strictly penance and never dark reprimand (triggered by shadow covenant), duration_expected and remains_expected work appropriately. However, as soon as dark reprimand is cast, the _expected values evaluate into very high numbers -- into the hundreds, or even thousands.

image

How to Reproduce

  1. Enter game as a Discipline priest
  2. Take the shadow covenant talent
  3. Add a duration_expected or remains_expected evaluation to the APL for penance
  4. Alternate casting penance and dark reprimand (shadow covenant -> penance), and observe the result

Snapshot (Link)

https://pastebin.com/mABCVi5K

Raidbots Sim Report (Link)

https://www.raidbots.com/simbot/report/hkoG2mwZL2YswdkCPHZvRD

Additional Information

The APL in the pastebin above is the draft APL for discipline being reviewed in simc: simulationcraft/simc#7491

Discipline and Holy priest DPS has recently been overhauled in simc, and are now fully up to date with the exception of the draft APLs.

I started a draft update for a future PR revising PriestDiscipline.lua to reflect the simc changes: https://pastebin.com/jMTCk0dy

Contact Information

jom#4860

commented

More clearly: What are your expected values here?

commented

What is it that you need duration_expected for? It really won't accomplish anything for a short duration spell.

commented

Alright, I think I've addressed this without additional information. The problem was that for remains_guess, it's easy enough to compare how much time has passed since the ability went on CD vs. how much time remains. For duration_guess, the addon will return the base cooldown when the ability isn't on CD. If the ability is on CD, it will then compare time passed vs. time remaining to forecast any additional CDR.

commented

Thanks for the effort! To answer the question about expected values, I would look for duration_expected to be an approximation of the ability's cooldown, adjusted for cooldown affecting mechanics. Specifically, from the simc wiki: "The expected duration is evaluated from the difference between the elapsed time since the cooldown started and how much the cooldown duration has actually gone down."

Was there actually no issue with penance vs. dark reprimand? When casting only penance I didn't observe and issues, I only noticed inaccuracies once casting dark reprimand subsequently after a penance.

commented

I meant more like, what value were you expecting to see prior to casting Penance and so forth. What purpose would the value serve in making any kind of determination from a priority standpoint?

I would look for duration_expected to be an approximation of the ability's cooldown, adjusted for cooldown affecting mechanics.

It won't really accomplish that in SimC. Actually, I'm wrong here. I don't record times spells come off CD, and I'm not sure if I want to do so. SimC does collect and use that to average out going forward.

For my current implementation, Penance's cooldown is too short to use a ratio of recovery time / recovered time and determine how much overall time the cooldown's duration will be. And their implementation should also not have any CDR values factored in when the ability isn't already on cooldown.

Was there actually no issue with penance vs. dark reprimand?

Not significantly, no. It just turns out that calculating time from last cast when the ability isn't on cooldown means you can be talking about a cast that was a minute ago. That ratio of recovery time / recovered time gets really silly when the equation suggests that you took 80 seconds to recover a 9 second cooldown. It should be fine now.

I also made it so the spell IDs and textures will swap when Shadow Covenant is up.

commented

The timing estimations are primarily for checking if we can cast a spell while shadow covenant is on cooldown and have it back up in time for the next shadow covenant window. In the case of penance, because it's such a large portion of the overall DPS it's a decent DPS boost to account for cooldown reductions in that evaluation (even if its only ~1.5-2secs off its 9sec cooldown -- could be the difference between an extra cast or not). As a workaround, I can just hardcode an average with something like this, but feels a bit brittle:

actions.variables+=/variable,name=expected_penance_reduction,op=setif,condition=talent.train_of_thought,value=1.5,value_else=0

I think there is still a bit of funkiness around penance and dark reprimand, too. Experimenting with some APL enhancements in simc and noticed, for example:

prev_gcd.1.penance will not evaluate to true in Hekili if the previous spell was dark reprimand (but does in simc). prev_gcd.1.dark_reprimand doesn't seem to work either. Is it possible to essentially consider both of those the "same" spell for purposes of the APL? This is simc's current behavior.

Thanks again for the help! Let me know if I can assist.

commented

I'll look into tracking average CD recovery time.

The prev_gcd piece might be resolved by removing the function for Penance's id and just putting the Penance ID back in there. That should make the (actual) CD tracking map to Penance. Probably. If not, I'll investigate a bit.