simulationcraft / simc

Simulationcraft engine/GUI
GNU General Public License v3.0
1.4k stars 694 forks source link

[Warrior] [Arms] - Dauntless/Deadly Calm and Execute interaction bugs #2790

Closed amulder87 closed 8 years ago

amulder87 commented 8 years ago

Dauntless is reducing the damage of execute and its chance to proc Tactician whenever execute is used below 40 rage.

If I sim a profile that just auto attacks and executes only with 40 or more rage and compare with/without dauntless taken, the damage per execute is identical with both, giving execute 25% more damage per rage and proccing tactician 25% more often when dauntless is taken, which is what we'd expect.

Toggle Dauntless on/off with this APL for this comparison:

warrior="Test"
level=100
race=orc
role=attack
position=back
talents=0000000
spec=arms

actions+=/auto_attack
actions+=/execute,if=rage>=40

main_hand=large_club,id=2480

If you change the conditions on execute to only execute below 32 rage and compare with/without dauntless again, Execute has an identical damage per rage with/without Dauntless when it should still be 25% higher with Dauntless, and Tactician procs no more often when it should still be 25% higher too.

Toggle Dauntless on/off with this APL for comparison. Tactician procs are slightly higher since spending rage on WW is granting a higher chance to proc technician than execute:

warrior="Test"
level=100
race=orc
role=attack
position=back
talents=0000000
spec=arms

actions+=/auto_attack
actions+=/execute,if=rage<=32
actions+=/whirlwind,if=target.health.pct<20&rage>32

main_hand=large_club,id=2480

With Deadly Calm taken, the current rage of the player is being used to calculate both the damage of execute and the amount of rage that counts towards tactician procs(counts as spending 10 minimum though I think?), instead of using 40 for both. Regardless of the player's current rage, an execute during Deadly Calm should count as a max rage execute for damage calculations(4x dmg) and tactician proc chances. Rage is also being consumed when execute is used during Deadly Calm when it should be entirely free.

Example APL that gives 0 damage from execute and reduced tactician procs:

warrior="Test"
level=100
race=orc
role=attack
position=back
talents=1000311
spec=arms

actions+=/battle_cry,if=target.health.pct<20
actions+=/execute,if=buff.battle_cry.up

main_hand=large_club,id=2480

Just adding auto attack to that APL will let execute deal damage and quadruple tactician procs, when it should make no difference for this scenario.

Collisionc commented 8 years ago

Hah, you keep reporting bugs right when I'm in the middle of fixing them. Thanks for the report, it should be fixed here: https://github.com/simulationcraft/simc/commit/f99b61ec7ba469800827298b2a26fd8aa7520cd2

Collisionc commented 8 years ago

Ok uploaded a fixed version after testing it out a little.

amulder87 commented 8 years ago

I think there may be an issue with MS/Dauntless/Tactician now. I can't really figure out what's going on here. Making an APL that just auto attacks and mortal strikes shows significantly fewer tactician procs with dauntless than without it, even though the number of MS uses is only like 2 apart since it's CD constrained outside of tactician procs anyways. If I set debug=1 active, MS isn't showing a log entry for tactician rage use calculations too. I'd have expected the same number of procs with/without dauntless when casting just MS.

Without Dauntless, the profile cast MS 67.9 times for 14.1 procs(matches expectations, 67.920.0065*1.6 = 14.1). With Dauntless, the profile cast MS 65.5 times for 10.9 procs. I expected this to match the previous scenario in both MS casts and procs, but 65.5 MS casts should still be 13.6 procs, which is 25% higher than what was seen.

amulder87 commented 8 years ago

I hate to pile on.. but Execute + Precise Strikes looks like it needs the same treatment you just gave to Dauntless too. Executing above 40 rage with precise strikes makes everything look normal, in terms of damage dealt and tactician proc chances. But at lower rage levels, precise strikes executes are showing a reduced damage level and tactician proc chance, even when they should still be max rage executes.

A max rage execute with dauntless and 3 ranks of precise strikes should only need 17.6 rage, but executing between 20 and 32 rage deals less damage and procs tactician fewer times than just executing above 32 rage and is reducing the rage the execute actually uses too. E.g. a dauntless/precise strikes execute cast at 25 rage only consumed 14 rage instead of a full 17.6, while an execute above 32 rage is taking the full 17.6.

Collisionc commented 8 years ago

Execute + (whatever) will be the death of me. :P

It's ok, there are so many various mechanics with execute that I'll get them all one day.

Collisionc commented 8 years ago

Also, if you use log=1 it will show the tactician cost

amulder87 commented 8 years ago

Yeah, execute interacts with a silly amount of things now. It's touched by, what, 4 different abilities that modify its cost, if you include ring? Lots of combinations for things to go awry, haha.

Also thanks for letting me know about log=1, I didn't know that one existed. I was using the debug=1 mode, which is awfully verbose, to look for tactician logs. I did see them showing up for every other ability I was looking at(even auto attacks), but just MS was not reporting tactician rage usage for some reason.

Collisionc commented 8 years ago

https://github.com/simulationcraft/simc/commit/1dfbf6326ebc9e165109c44a2b6607a334dd2043

I'm just gonna leave this issue open for a few days, as I'm sure there's some other case that'll come up. :)

amulder87 commented 8 years ago

Alrighty, I grabbed latest and re-tested it a bit. The MS tactician proc rate is all cleared up and looks identical with or w/o dauntless now. Execute damage with precise strikes is also looking completely accurate now, showing max damage for any execute at over 17.6 rage with dauntless + precise strikes.

But, I'm still seeing reduced tactician procs and rage consumption from precise strikes executes that should act like max rage executes.

The testing profile I'm using to isolate it is:

warrior="Warrior_Arms_T19P"
level=110
race=blood_elf
role=attack
position=back
talents=1332311
artifact=36:137469:137363:137377:0:1136:1:1137:1:1139:1:1142:1:1145:3:1147:3:1148:3:1149:3:1150:6:1356:1
spec=arms
log=1

actions.precombat+=/snapshot_stats

actions+=/auto_attack
actions+=/execute,if=rage>=17.6&rage<=32&buff.precise_strikes.up
actions+=/whirlwind,if=target.health.pct<20&rage>37.6
actions+=/hamstring,if=target.health.pct<20&rage>25.6
actions+=/colossus_smash,if=buff.precise_strikes.down&target.health.pct<20
actions+=/warbreaker,if=buff.precise_strikes.down&target.health.pct<20

main_hand=large_club,id=2480

Since 17.6 should be the most rage an execute can cost during precise strikes with dauntless, every execute the profile casts ought to deal full damage, consume 17.6 rage, and count as 40 rage for a tactician proc. It's dealing the damage I'd expect, but the log entries for tactician show that it's still treating the cost and chance to proc tactician as acting as if the max rage is 32 still, e.g.:

at 20.8 rage:

Rage used to calculate tactician chance from ability execute: 26.0000, actual rage used: 11.4400

at 23.76 rage:

Rage used to calculate tactician chance from ability execute: 29.7000, actual rage used: 13.0680

The tactician proc rage is being modified by current rage, up to 32, and then factoring in dauntless, e.g. 20.8/0.8 = 26, and the actual rage consumed is the current rage, up to 32, modified down by precise strikes, e.g. 20.8*.55= 11.44. I think all those calculations need to work like the damage one does now, and factor in the max cap after being modded by precise strikes.

Collisionc commented 8 years ago

346.242 Warrior_Arms_T19P execute hits Fluffy_Pillow for 79674 physical damage (hit) 346.242 Strength: 10229.0000, AP: 10229.0000, Crit: 6.0000%, Crit Dmg Mult: 1.0000, Mastery: 16.0000%, Haste: 0.0000%, Versatility: 0.0000%, Bonus Armor: 0.0000, Tick Multiplier: 1.0000, Direct Multiplier: 7.4932, Action Multiplier: 5.2000 346.242 Rage used to calculate tactician chance from ability execute: 40.0000, actual rage used: 17.6000 346.242 Warrior_Arms_T19P consumes 17.6 rage for execute (1) 346.242 Warrior_Arms_T19P loses shattered_defenses 346.242 Warrior_Arms_T19P loses precise_strikes

plz work tactician

https://github.com/simulationcraft/simc/commit/77ce449312a888e07b2db85d95349470637559df

amulder87 commented 8 years ago

Aw yeah, all of that now looks like it's working like a charm!

One last quibble though :)

All the upper limits of executes look perfect now with precise strikes/dauntless, but the lower end doesn't look like it takes precise strikes into account yet. You should only need at least 4.4 rage with dauntless/precise strikes for it to be usable, but it still takes at least 8 even with precise strikes up. Test APL I was using ends up with 0 executes cast even though it lands between 4.4 and 8 rage fairly frequently while precise strikes is up:

actions+=/auto_attack
actions+=/execute,if=rage<8
actions+=/whirlwind,if=rage>24.4
actions+=/hamstring,if=rage>12.4
actions+=/colossus_smash,if=buff.precise_strikes.down&target.health.pct<20
actions+=/warbreaker,if=buff.precise_strikes.down&target.health.pct<20
Collisionc commented 8 years ago

I would love to see how blizzard has this coded on their end, because it's a mess here.

https://github.com/simulationcraft/simc/commit/8d14f9d09550eb4dc133fd223c5690b50e7ff280

amulder87 commented 8 years ago

In b4 they decide tactician doesn't fit the arms theme anymore, and it all goes away :)