Hekili Priority Helper

Hekili Priority Helper

44M Downloads

[Retail,Rec,OutlawRogue] Pistol shot CP generation problem

IIeTpoc opened this issue ยท 5 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).

WoW Version

Retail (Dragonflight)

Describe the Issue

It feels like precalculation of ComboPoints gain from Pistol Shot are either slow or not correct.

Rec 1 is pistol shot with buff opportunity up which at the moment on retail with ONLY talent.fan_the_hammer without any further buffs for some reason gives 6 combo points (i do not know if that is actually a bug or it is intended, but i tested that sever times in game at it is so.
I assume the logic is the following: Pistol Shot grants 1 CP, QuickDraw talent with Opportunity buff grants +1, FanTheHammer talent grants 3 stacks of opportunity, thus after pressing PistolShot it fires 3 pistol shot at once and each of them grants 2 CP and 6 CP in total).

Rec 2 and 3 are both sinister strikes but as soon as i press Pistol Shot as #1 recomendation for 0.5 sec appears sinister strike, then addon calculation CP gain from Pistol Shot and changes Recommendation #1 on a finishing move.
It might be the case as upon using buffed by opportunity and talent.fan_the_hammer Pistol Shot does not grant CP instantly, but rather needs a moment to gradually increase CP.

How to Reproduce

  1. Go to orgri and DPS target dummy as outlaw rogue

Talent Loadout

BQQAOB/7AZ//CwQOf4fPMswHVDAAABJBkkkQSLRgEIRiEAJCAAAAAAQSQaSJJQSSKBAAAA

Snapshot

https://pastebin.com/Cay39ZBJ

Raidbots Sim Report (Link)

No response

Additional Information

No response

Contact Information

IIeTpoc#3818 discord ID

commented

Also yes, I understand I am supposed to use default priority but in this case i impemented PullRequest #1919 to represent finish condition typo fix and lines with pistol shot were not changed at all.
I was testing without Seal Of Fate talent, thus critical hits did not grant me an extra CP.

Another test I made is change line 1556 in Outlaw.lua in cp_gain function where i just hardcoded it to gain 6 CP upon use with Opportunity buff (talent.quick_draw.enabled and buff.opportunity.up and 5 or 0), and did a test right after:

  1. Before Pistol Shot is cast with Opportunity 3 stacks Rec 2 is a finishing move (Snapshot - https://pastebin.com/7NsLpPLm)
  2. Bur DURING Pistol Shot is being cast (the entire GCD duration after Pistol Shot is being pressed) every single recommendation (1,2,3) are changed to Sinister Strike, after GCD becomes available, it recalculates and recommends a finishing move (Snapshot - https://pastebin.com/Cay39ZBJ)
commented

There are a lot of different things I'm trying to track in here all at once.

represent finish condition typo fix and lines with pistol shot were not changed at all.

For:
combo_points>=cp_max_spend-buff.broadside.up-(buff.opportunity.up*(talent.quick_draw|talent.fan_the_hammer)|buff.concealed_blunderbuss.up)|effective_combo_points>=cp_max_spend

What made you think the * is a typo? What is not calculating correctly?

  1. Before Pistol Shot is cast with Opportunity 3 stacks Rec 2 is a finishing move (Snapshot - https://pastebin.com/7NsLpPLm)
  2. Bur DURING Pistol Shot is being cast (the entire GCD duration after Pistol Shot is being pressed) every single recommendation (1,2,3) are changed to Sinister Strike, after GCD becomes available, it recalculates and recommends a finishing move (Snapshot - https://pastebin.com/Cay39ZBJ)

Yeah, there's a lot of fundamentals here that I don't think you're following. For forecasting (looking ahead), I can tell the addon to do all sorts of things. That's what the abilities and cp_gain and stuff do -- it tells the addon how to model a pretend cast of the ability for looking forward.

When generating recommendations, the addon starts from the actual game state. So in the actual game, you just hit Pistol Shot and time passes before the Fan the Hammer shots also occur. I can model those, and you can ask me to model those, but these fixes you've provided are not the right way to do it.

Being candid, it's more helpful to just clearly explain the issue you're seeing per the original ticket instructions before proposing fixes that don't fit right and I probably won't use.

image

You can see here that I used Pistol Shot and a second cast came later (15:10:10 vs. 15:10:11). I can model that and implement it accurately.

commented

Thank you for the thorough reply! I appreciate it and I definetely did not take into account the interaction between forecasting and processing recommendations during Pistol Shot with Fan the Hammer talent.
By any means i did not want to offend you and provide my "fix", it was purely just for test purposes and more snapshots which i considered to be useful when i was creating this ticket. I will try to be more precise with my future tickets if i will create any.

Of course it would be nice if you could model that Fan The Hammer talent behaviour and implement it into the addon.

As for finish condition:

For: combo_points>=cp_max_spend-buff.broadside.up-(buff.opportunity.up*(talent.quick_draw|talent.fan_the_hammer)|buff.concealed_blunderbuss.up)|effective_combo_points>=cp_max_spend

What made you think the * is a typo? What is not calculating correctly?

I thought it was a Lua syntaxis obligation to use "&" instead of "*". The same fix was made by you and was present since June 13 2022. 05ce0ef

commented

I did "fix" it there once for clarity purposes, but the interpreter handles it. If something is a boolean true/false value and a math operator is being used, it's converted to 1/0 internally. I'll double-check that it is working correctly, though.

commented

Ok, I double-checked and there was still an issue here, so I've made more edits to the script translator that should resolve that.