ClassicWoWCommunity / cata-classic-bugs

This is a place for Cataclysm & Wrath of the Lich King Classic bug reports and research
66 stars 6 forks source link

[BUG] Rend snapshotting after mortal strike/thunder clap extra tick change #1532

Open zoristaken opened 1 month ago

zoristaken commented 1 month ago

1. Current Behavior

1.1. Description

Mortal Strike and Thunder clap were doing an extra rend tick previously to the recent patch. This was changed and introduced rend snapshotting.

1.2. How to Reproduce

https://classic.warcraftlogs.com/reports/7GhKxnLt6R3gXNkD#fight=6&type=damage-done&source=1&pins=2%24Off%24%23244F4B%24expression%24ability.name%20in%20(%22Mortal%20Strike%22,%20%22Thunder%20Clap%22,%20%22Rend%22)&view=events

  1. normal rend without procs (2340)
  2. popped angerforge + potion, kept refreshing with mortal strike, still same damage
  3. rend recast at 51 seconds, new baseline damage (3535)
  4. kept refreshing with mortal strike until the end, damage didn't change

thunder clap: https://classic.warcraftlogs.com/reports/J4kaGQFYBy28rPK3#fight=7&view=events&type=damage-done&source=1&pins=2%24Off%24%23244F4B%24expression%24ability.name%20in%20(%22Thunder%20Clap%22,%20%22Rend%22)

  1. normal rend on target 1 with snapshot (3557)
  2. thunder clap refreshes on target 1, but doesn't update value, just like mortal strike refresh bug fix
  3. thunder clap applies rend to target 2, with new value according to current stats
  4. they keep ticking for different values until the end of the log

    1.3 Source Material

    https://classic.warcraftlogs.com/reports/J4kaGQFYBy28rPK3 there's more examples in that log

    2. Expected Behavior

    2.1. Description

    Rend should dynamically update with every refresh

    2.2 Source Material

    https://www.youtube.com/watch?v=2yYrZqDSLig rend is dynamically updating, can see that at 44 seconds rend ticks for 5229 with potion, enrage and landslide? up, and then at 56 seconds it does 4844 damage with only enrage and landslide up. Mortal strike being the only ability refreshing rend in this scenario

zoristaken commented 1 month ago

Copy from my reply to the other ticket in case it doesn't get seen

I have reason to assume that the extra tick from mortal strike that was removed in the latest patch was correct. 4.3 simulationCraft has mortal strike causing an extra tick. Using the original video posted here (https://youtu.be/OYM_rhC-ldA?t=424), from the timestamp 7:04 until 7:13 (excluding the 3 rend ticks shown at the 7:04 time frame that come from previous events) there were a total of 5 rend ticks and 2 mortal strikes cast. If there were no extra ticks from mortal strike there would be 3 total rend ticks at 7:07, 7:10 and 7:13. As that is not the case, adding the 2 extra ticks from the mortal strikes match the total ticks value. https://www.youtube.com/watch?v=2yYrZqDSLig&t=54s the timestamp shows another example that further solidifies this scenario as there are 2 non-aggregated rend ticks in sub 1 second, exactly after a mortal strike was cast. I also tried to count the first minute of the fight (excluding the mortal strike before the initial rend application and the rend tick on cast - from 0:28 to 1:28 - and got 31 rend ticks and 12 mortal strikes, which shouldn't be possible to get more than 20 unless mortal strike refresh would cause a tick. Using an addon that can be configured to aggregate dot ticks with different options shouldn't be used to confirm any of this anyway and some sort of confirmation on the intended behavior from your side would be nice. Cheers

zoristaken commented 1 month ago

Copy from my reply to the other ticket in case it doesn't get seen

I have reason to assume that the extra tick from mortal strike that was removed in the latest patch was correct. 4.3 simulationCraft has mortal strike causing an extra tick. Using the original video posted here (https://youtu.be/OYM_rhC-ldA?t=424), from the timestamp 7:04 until 7:13 (excluding the 3 rend ticks shown at the 7:04 time frame that come from previous events) there were a total of 5 rend ticks and 2 mortal strikes cast. If there were no extra ticks from mortal strike there would be 3 total rend ticks at 7:07, 7:10 and 7:13. As that is not the case, adding the 2 extra ticks from the mortal strikes match the total ticks value. https://www.youtube.com/watch?v=2yYrZqDSLig&t=54s the timestamp shows another example that further solidifies this scenario as there are 2 non-aggregated rend ticks in sub 1 second, exactly after a mortal strike was cast. I also tried to count the first minute of the fight (excluding the mortal strike before the initial rend application and the rend tick on cast - from 0:28 to 1:28 - and got 31 rend ticks and 12 mortal strikes, which shouldn't be possible to get more than 20 unless mortal strike refresh would cause a tick. Using an addon that can be configured to aggregate dot ticks with different options shouldn't be used to confirm any of this anyway and some sort of confirmation on the intended behavior from your side would be nice. Cheers

https://web.archive.org/web/20120508014554/http://elitistjerks.com/f81/t106913-arms_dps_4_0_cataclysm/p16/#post1933673 found a post talking about this situation that was recently changed. Could not find any patch notes or comments in that thread contradicting this statement. image

https://web.archive.org/web/20120209232143/http://elitistjerks.com/f81/t106913-arms_dps_4_0_cataclysm/p19/#post2005839 different person confirming the behavior described by the previous user image

Another post dated after patch 4.3.2, which was the last patch with class changes I could find https://web.archive.org/web/20120207224856/http://elitistjerks.com/f81/t106913-arms_dps_4_0_cataclysm/p24/#post2091835 image

Kethryllia commented 1 month ago

I've bugged htis. We want to keep the Rend refresh, but some way of preventing snapshotting would be nice too.

zoristaken commented 1 month ago

I've bugged htis. We want to keep the Rend refresh, but some way of preventing snapshotting would be nice too.

If the info I gathered about mortal strike/thunderclap extra rend ticks is correct, what was the intent of this change to begin with? I don't seem to understand what would be the benefit. The spec will play the same, but we are getting hit by a somewhat considerable nerf on single target and aoe. #somechanges done wrong?

Kethryllia commented 1 month ago

The change was due to feedback from Github: https://github.com/ClassicWoWCommunity/cata-classic-bugs/issues/985

TLDR, it was causing Rend to fall out of sync with Taste for Blood's internal cooldown, causing players to lose Overpower procs.

zoristaken commented 1 month ago

The change was due to feedback from Github: #985

TLDR, it was causing Rend to fall out of sync with Taste for Blood's internal cooldown, causing players to lose Overpower procs.

I think when the original ticket was created, rend refreshes were making the original tick frequency change. This was not the situation already, but I might be missing something here.

Rend was already ticking every 3 seconds no matter when you got the refresh e.g: rend would tick at 0,3,6, even if you mortal strike at 4.5, this is easily verified on logs prior to the patch.

If you played normally, and opened on an enemy by doing rend > mortal strike, or mortal strike > rend into a normal rotation, e.g: Mortal strike > gcd > gcd > Mortal Strike, ... you will always have a rend tick match with a mortal strike every 9 seconds, but no mortal strike refreshing Taste for Blood between the 5-6 second period. Am I missing how we are losing overpower casts?

This also doesn't fix any issue that you described as you can apply rend manually to 2 different targets and have them trigger Taste for Blood ICD before the next 6 second rend tick.

edit: reworded some sentences and added examples

Kethryllia commented 1 month ago

I've passed your feedback along to Design. Thanks!

zoristaken commented 1 month ago

I've passed your feedback along to Design. Thanks!

The change was due to feedback from Github: #985 TLDR, it was causing Rend to fall out of sync with Taste for Blood's internal cooldown, causing players to lose Overpower procs.

I think when the original ticket was created, rend refreshes were making the original tick frequency change. This was not the situation already, but I might be missing something here.

Rend was already ticking every 3 seconds no matter when you got the refresh e.g: rend would tick at 0,3,6, even if you mortal strike at 4.5, this is easily verified on logs prior to the patch.

If you played normally, and opened on an enemy by doing rend > mortal strike, or mortal strike > rend into a normal rotation, e.g: Mortal strike > gcd > gcd > Mortal Strike, ... you will always have a rend tick match with a mortal strike every 9 seconds, but no mortal strike refreshing Taste for Blood between the 5-6 second period. Am I missing how we are losing overpower casts?

This also doesn't fix any issue that you described as you can apply rend manually to 2 different targets and have them trigger Taste for Blood ICD before the next 6 second rend tick. Why was Taste for Blood ICD being changed to 6 sec not considered instead if there was reason to believe inconsistencies were going on?

edit: reworded some sentences and added examples

Logs with Taste for Blood consistency https://classic.warcraftlogs.com/reports/6GQzcRFWfCJqgkbp#fight=44&type=auras&source=29&translate=true&ability=60503&view=events&pins=2%24Off%24%23244F4B%24auras-gained%24-1%240.0.0.Any%240.0.0.Any%24true%240.0.0.Any%24true%240%2433

image

https://classic.warcraftlogs.com/reports/dFLkjaMC4y18fwhm#fight=7&type=auras&source=23&view=events&pins=2%24Separate%24%23244F4B%24auras-gained%24-1%240.0.0.Any%240.0.0.Any%24true%240.0.0.Any%24true%240%2433&ability=60503

image

you can note that the last Taste for blood in this image has a bigger interval, that is because there was no overpower usage in the last seconds of the fight

Kethryllia commented 1 month ago

After further investigation with Design, the current implementation is consistent how Mortal Strike and Rend interact in our reference client. While it does lower Arms damage somewhat, this was still a bug that had to be resolved especially due to issues that were reported with Overpower. We're definitely going to try and fix the snapshotting though.

I definitely appreciate the feedback though!

bobsaft commented 1 month ago

Not sure if this is the place to ask. I have seen statements, i believe they are from Blizzard, that bug fixes which cause dps loss would be compensated with other buffs. Is that the case here? Asking because this change will make many warriors, myself included, consider rerolling the class to something that is more competetitive

We are facing a 1500ish dps loss in T3 gear, and worse in T4, equivalent to getting a bis weapon. And likely shoving the class to the bottom of the meters

If this is change is intended and will stand as is, it would be good to have that information now, since we are switching phases and it is the best time to reroll for many people

bobsaft commented 1 month ago

As an aside, there seems to be a consensus on warrior discord, and among warriors ive talked to, that this OP loss is phantom. Ive yet to encounter a warrior that agrees they were losing OP procs

chandam64 commented 1 month ago

I am confused how zoristaken can post evidence that Taste for Blood was not broken and there were no issues with Overpower usage, but is met with such a dismissive statement. Appreciate the attention on this topic but there is evidence that a fix was unnecessary and should be reverted.

JamminL commented 1 month ago

This is not the right forum for discussions guys - I understand where ya'll are coming from but the point of this github is strictly for bug reporting and #somechanges suggestions. I think both have been made, further discussion is best left in other forums otherwise this gets a bit hectic.

Kethryllia commented 4 weeks ago

I am going to make a mea culpa. I used some flawed methodology yesterday and missed that our reference client does in fact have an instant tick after refreshing it with Mortal Strike or Thunder Clap with Lambs to the Slaughter and Blood and Thunder. I want to apologize for overlooking this.

I sent my findings to Design, and there is a hotfix in flight to revert the change.