Implosion spam at 3 imps seems inefficient for Implosive Potential

25haberianh commented 2 years ago

Before You Begin

[X] I confirm that I have downloaded the latest version of the addon.
[X] I am not playing on a private server.
[X] I checked for an existing, open ticket for this issue and was not able to find one.
[X] 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

I have Visions of Implosive Potential as my crafted legendary for my Demonology Warlock. The addon keeps recommending that I cast Implosion at 3 imps. I feel like the "Potential" aspect of "Implosive Potential" is being wasted.

How to Reproduce

I believe a good solution for this problem is adding a slider similar to that of the one for Summon Demonic Tyrant, or just re-adjust the priority. I'm really missing my 30% Haste bonus.

Thanks for the great addon, though!

Snapshot (Link)

pastebin.com/iLiwYsXu

Raidbots Sim Report (Link)

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

Additional Information

I've adjusted my priority to only recommend Implosion on 3+ targets, but I don't think that's why. Gotta get that +5 haste per imp!

Contact Information

No response

Hekili commented 2 years ago

Hi, could you please put your snapshot on Pastebin and provide the link; otherwise it's basically unreadable. Thanks!

It would also be helpful if you were to sim your character in circumstances that match what you're concerned about (i.e., imploding in single target = Patchwerk with 1 boss).

25haberianh commented 2 years ago

Sorry about the monster of a recommendation, lol.

I've added both your suggestions - I set the Raidbots sim for 3 bosses, and light movement. Thanks for your help!

Hekili commented 2 years ago

Looking at your snapshot, your priority does not match my default priority. Did you edit it intentionally?

The first ability in your default action list is Implosion with absolutely no conditions associated with it. That is inconsistent with the default.

The default priority would use Implosion in the following cases:

actions+=/implosion,if=boss&fight_remains<2*gcd actions+=/implosion,if=active_enemies>1+(1*talent.sacrificed_souls.enabled)&buff.wild_imps.stack>=6&buff.tyrant.down&variable.next_tyrant_cd>5 actions+=/implosion,if=active_enemies>2&buff.wild_imps.stack>=6&buff.tyrant.down&variable.next_tyrant_cd>5&!runeforge.implosive_potential&(!talent.from_the_shadows.enabled|debuff.from_the_shadows.up) actions+=/implosion,if=active_enemies>2&buff.wild_imps.stack>=6&buff.implosive_potential.remains<2&runeforge.implosive_potential actions+=/implosion,if=buff.wild_imps.stack>=12&talent.soul_conduit.enabled&talent.from_the_shadows.enabled&runeforge.implosive_potential&buff.tyrant.down&variable.next_tyrant_cd>5

You can reset the priority by clicking the circle-arrow on /hekili > Priorities > Demonology, first tab.

25haberianh commented 2 years ago

I think I was looking for an option for minimum Imps. I've reset my priority. Want me to re-send it?

Hekili commented 2 years ago

If you're using the default priority and you believe you have an opportunity for improvement, sure. Provide a snapshot when it's recommending Implosion and you think it shouldn't. Make sure your sim and snapshot are the same scenario -- if your snapshot is against 1 target but your sim is 3 targets, they're not comparable. Thanks.

dubudevs commented 2 years ago

The demo APL is highly unoptimised for implosion cycles, especially with SC. With SC you can "reliably" get 9, and without SC you alternate between 6 and 9. SimC does not have the functionality built in to handle implosion cycles intelligently so it is unlikely to be updated any time soon, but imploding with 3 imps should never be recommended unless the target is about to die.

For IP there is also the consideration of whether the current implosion would overwrite a higher haste buff, and whether or not that is worth it or not in the context. "Thanks" to the nerf that makes 15 the cap for IP it is rarely worth thinking about this.

Unless there is the chance that the addon will be updated to provide functionality that is no present in SimC (namely tracking oldest imps and calculating whether we have time to build more imps before our old ones die) then there's not a whole lot to be done.

There is some logic in the current APL which is broken:

actions+=/implosion,if=active_enemies>2&buff.wild_imps.stack>=6&buff.implosive_potential.remains<2&runeforge.implosive_potential

actions+=/implosion,if=buff.wild_imps.stack>=12&talent.soul_conduit.enabled&talent.from_the_shadows.enabled&runeforge.implosive_potential&buff.tyrant.down&variable.next_tyrant_cd>5

I will have a look at fixing this.

dubudevs commented 2 years ago

Changing these two lines into four lines with a SC check on each one is a DPS gain, and reflects "correct" gameplay more accurately

actions+=/implosion,if=!talent.soul_conduit.enabled&active_enemies>1+(1*talent.sacrificed_souls.enabled)&buff.wild_imps.stack>=6&buff.tyrant.down&variable.next_tyrant_cd>5
actions+=/implosion,if=talent.soul_conduit.enabled&active_enemies>1+(1*talent.sacrificed_souls.enabled)&buff.wild_imps.stack>=9&buff.tyrant.down&variable.next_tyrant_cd>5

actions+=/implosion,if=!talent.soul_conduit.enabled&active_enemies>2&buff.wild_imps.stack>=6&buff.tyrant.down&variable.next_tyrant_cd>5&!runeforge.implosive_potential&(!talent.from_the_shadows.enabled|debuff.from_the_shadows.up)
actions+=/implosion,if=talent.soul_conduit.enabled&active_enemies>2&buff.wild_imps.stack>=9&buff.tyrant.down&variable.next_tyrant_cd>5&!runeforge.implosive_potential&(!talent.from_the_shadows.enabled|debuff.from_the_shadows.up)

Hekili commented 2 years ago

Unless there is the chance that the addon will be updated to provide functionality that is no present in SimC (namely tracking oldest imps and calculating whether we have time to build more imps before our old ones die) then there's not a whole lot to be done.

I occasionally add things I need or would prefer, like creating generic expressions for when the next 'major demon' would expire. Catching how many imps will expire in the next X seconds and comparing it to how long it would take to generate more (time to 3 shards + hand of guldan cast time) is not that complicated.

25haberianh commented 2 years ago

@dubudevs What you're saying is, it isn't recommending me to generate 6 imps before imploding, because that would make the Bloodlust bonus useless? You would think the two stack...

Either way, I think adding a slider for "Minimum Implosion Imps" is a smart idea, because it would give players flexibility based on their build. Lets say I set the slider to 9 imps. I could do multiple large Implosions and gain a massive haste buff. (I know, it might just override the Bloodlust, but I'm speculating here.) Another player might decide, 'You know what, I'm just going to spam Implosion, cause why not?' Allowing them to have this option would be convenient. I do understand that it may be a hassle to add all that, but stupid people like me would really appreciate it.

dubudevs commented 2 years ago

If this can be implemented it would make it significantly more accurate in aoe than the current simc implementation, this is one of the biggest differences between real gameplay and sims right now. The logic is relatively simple for implosion cycles, the only "real" logic would come with optimisations that aren't implemented elsewhere in the APL yet, so it does basically boil down to just replacing those lines with "summon as many imps as you can and implode when you don't have time to summon more", or in other words "implode just before your oldest imps will die".

Also, the "logic" which is separate in those lines doesn't really need to be separate. Whether you are playing IP or not, you still just want to "implode just before your oldest imps will die". The only exception is if you have very few imps (not sure exactly where the breakpoint is) such that the global spent imploding isn't an overall gain. I expect this is 3 or fewer imps without IP or 2 targets, and less than 2 with IP in 3+ targets.

My only concern is the performance hit, generally speaking imp tracking/imp related WAs/addons are quite performance intensive, so hopefully there's a way to efficiently manage it.

dubudevs commented 2 years ago

25haberianh It should always be recommending 6 imps, unless the target is about to die (in which case it will recommend to implode as the last global, roughly). But, as Hekili mentioned it looks like there is an issue with your APL.

I'll un-hijack this thread now lol.

25haberianh commented 2 years ago

@dubudevs No problem, man! I haven't had a ton of time to test my newly-reset APL, so I'll send an update on that later. Thanks so much!

dubudevs commented 2 years ago

@25haberianh FWIW, as I eluded to earlier, if you are playing IP you probably want to play SC too, in which case you can get 45% haste reliably from 9 imps.

25haberianh commented 2 years ago

@dubudevs I thought about that, I think I'll start running it now. The only reason I've been running Grimoire: Felguard is because IcyVeins says that it "deals a lot of burst damage, and is a particularly powerful cooldown for cleave." Then I read the next sentence, which says that this only happens with Demonic Consumption (which I am running) and Summon Vilefiend (which I'm not) Thanks so much for your help, both of you!

25haberianh commented 2 years ago

I ran a M+ yesterday, it seems to be fixed. And, I'm doing significantly more damage now that I've switched to Soul Conduit.

Thanks for everything!

Hekili / hekili