Hekili Priority Helper

Hekili Priority Helper

44M Downloads

[Feral] pool_resource issues

ShmooDude opened this issue ยท 4 comments

commented

Describe the Bug/Issue
pool_resource,for_next=1 isn't functioning correctly. Abilities are not stopping the action list when the only thing they are waiting on is resources.

To Reproduce
This is most noticeable during Feral's Berserk because the action list should be using Shred at <4 CPs but is instead dropping down to the bloodtalons action list. Usually trying to cast either Rake or Brutal Slash even when at 2 CPs. You can see this in the snapshot below, where Shred is ready at +0.44 but it ultimately recommends Brutal Slash which is ready at +0.00

Expected behavior
pool_resource,for_next=1 should stop the action list at the current recommendation if the only thing we are waiting on is energy. Abilities without this should continue down the action list to see if anything else is ready earlier.

PUT ISSUE REPORT LINK HERE: https://pastebin.com/YMkyTr6m
 
PUT SNAPSHOT LINK HERE: https://pastebin.com/sjC5iLx2

Note: I noticed that pool_resource is completely disabled in the code. I tried re-enabling it but only ended up with Attempted to Pool Resources for invalid next entry in the APL. Skipping. instead.

commented

I'll revisit it, but have you noticed that all the pool_resource logic rarely produces a DPS benefit in sims?

https://www.raidbots.com/simbot/report/26QofNw3tRZcZJJqrwBhyP

image

commented

Because most of the time, you're right, the pool_resource logic doesn't actually increase dps (for feral, I can't say for other classes). It doesn't really matter if you use Brutal Slash before a Rake refresh or after. It does often confer a computational speed increase (in simcraft) by stopping the action list at that point and not having to run the calculations on any subsequent actions. Whether that would be the case in Hekili after you account for whatever overhead is required to support pool_resource or not I don't know.

In the case of the Berserk case I pointed out, it's dropping down into the bloodtalons action improperly because I'm relying on that specific pool_resource for an important behavior. It's really the only one that matters (dps wise). as I experimented a bit with your nopool profile and adding back this single pool_resource restores all lost damage. Note that your nopool profile its actually a larger loss (~20 dps instead of ~10 for my gear) for MoC because its not gaining the benefit of additional bloodtalons procs.

That said, its the feel that makes the biggest difference for me. Since I know that during Berserk, all finishers will be followed by Shred (and the occasional Rake refresh), I can tell when Berserk is going to be over by when its first suggestion after a finisher is Brutal Slash. Not having the pool_resource breaks this.


I re-created the behavior of this particular line in my custom priority using run_action_list:

actions.stealth+=run_action_list,name=stealth_shred,if=combo_points<4&spell_targets.thrash_cat<5

actions.stealth_shred=shred

Although that wouldn't work if shred was ever put on cooldown or otherwise unusable but I don't believe there are any such mechanics this expansion so it works for now.

So if you don't think its worthwhile to add back pool_resource functionality, you should probably change the official feral profile with the changes above.

commented

Because most of the time, you're right, the pool_resource logic doesn't actually increase dps (for feral, I can't say for other classes).

It tends to be true for Rogues as well, but I haven't retested it throughout. I wasn't really pointing that as resistance to reimplementing pool_resource, just noting its lack of value. That higher top-end without pooling is pretty attractive here for Feral.

It does often confer a computational speed increase (in simcraft) by stopping the action list at that point and not having to run the calculations on any subsequent actions.

Likely a wash, in the addon. The effect in the addon is more like having an extra, invisible queued action. The addon would decide on pool_resource, handle it by advancing and generating resources, then I'd have to reinvestigate whether I can (A) continue down the action list from where I am or (B) restart from the top again.

That said, its the feel that makes the biggest difference for me. Since I know that during Berserk, all finishers will be followed by Shred (and the occasional Rake refresh), I can tell when Berserk is going to be over by when its first suggestion after a finisher is Brutal Slash. Not having the pool_resource breaks this.

I'm not opposed to reimplementing a more honest pool_resource (instead of ignoring it), but I just wanted to be transparent about my rationale. Outcomes are nearly identical, but you've pointed out where there's a visible difference and I'll tinker with it.

Your trick here to make the stealth action list terminate with by using run_action_list is really clever. I could internally reimplement pool_resource in a similar way. Maybe. I need to look back at SimC to determine if the behavior is going back to the top of this list or the top of the whole APL before sorting that out. Will look into it this weekend, though.

commented

pool_resource should be functional now (when used with for_next).