Ravenholdt-TC / Rogue

Anything you want to add into the dashboard should be done here.
4 stars 0 forks source link

Redundant Vanish Pool Action #76

Closed EvanMichaels closed 6 years ago

EvanMichaels commented 6 years ago

I noticed something while debugging some things tonight.

You can effectively remove the pool_resource above the Vanish line in CDs for Sub, as it doesn't actually do anything. Since the Vanish line itself checks the same Energy threshold, pool_resource internally in SimC only functions when the for_next action is "ready" and since it that will always be false it will never actually pool.

Since Vanish costs 0 Energy, the Energy check here is functionally the same, so it will never be more or less than the conditional. (But, more importantly, it will never actually pool.)

actions.cds+=/pool_resource,for_next=1,extra_amount=55-talent.shadow_focus.enabled10 actions.cds+=/vanish,if=energy>=55-talent.shadow_focus.enabled10&variable.dsh_dfa&(!equipped.mantle_of_the_master_assassin|buff.symbols_of_death.up)&cooldown.shadow_dance.charges_fractional<=variable.shd_fractional&!buff.shadow_dance.up&!buff.stealth.up&mantle_duration=0&(dot.nightblade.remains>=cooldown.death_from_above.remains+6&!(buff.the_first_of_the_dead.remains>1&combo_points>=5)|target.time_to_die-dot.nightblade.remains<=6)&cooldown.death_from_above.remains<=1|target.time_to_die<=7

Removing the pool line was within the margin of error for me on 100k iterations. I tried making it function "correctly" by removing the energy conditional, but that seemed ever so slightly worse, but still very close.

Mystler commented 6 years ago

As we discussed on Discord, the energy check condition is technically not redundant and, due to the way pool_resource works at the moment, necessary for it to work properly. Pooling with for_next checks energy requirements from the following condition as well and pools as specified if that condition is not satisfied. This may be a little unintuitive.

Aside from the technical aspect, it seems true that for current T21 sims this does not play a role and pooling occurs only very rarely without a big impact. I noticed a very slight loss with T19, so let's just keep it for now, if it is otherwise neutral.