[3.3.5] Hand of Freedom Bug (40$)

[immvp]

Description

Hand of Freedom doesn't remove Slows, but instead makes the Slow effect not work. Additionally, if an effect has a Slow effect AND an additional effect, then the ability bugs out, resulting in you being slowed with Hand of Freedom up. (Example is Frostbolt with Permafrost Talent)

Expected behaviour

Hand of Freedom should remove Roots and Slows off the target. If a spell has a Slow effect AND an additional effect (Like Frostbolt with Permafrost then Hand of Freedom should keep the Frostbolt aura active, while removing the Slow effect from it.

Steps to reproduce the problem

1. Log on a Mage and spec into Permafrost
2. Cast an abiltiy with a Chill effect like Frostbolt or Cone of Cold on a Paladin
3. Cast Hand of Freedom on the Paladin
4. Try to walk around - You'll notice that you're still slowed.

Branch

3.3.5

TC rev. hash/commit

TrinityCore rev. 5e56b14d08d1 2022-07-06 17:15:57 +0200 (main branch) (Unix, Release, Static)

Operating system

ubuntu 20.04

Custom changes

Transmog

[adeutscher]

I'm having some difficulty reproducing this as described. I'd like to add on the following test case:

1. Apply a longer slow effect like Hamstring (.aura 1715)
2. Cast Hand of Freedom on the target.

In Hamstring and Frostolt tests, I'm seeing that:

• When Hand of Freedom is applied, the target's run speed returns to normal.
• When Hand of Freedom fades from the Hamstring victim, the target's speed does not go back down to hamstring levels. This is harder to see with Frostbolt because the debuff time is so close to the Hand of Freedom time.
• If a new slow effect is applied (like if the victim is getting spammed with Frostbolt), then Frostbolts that hit after Hand of Freedom is applied to the target will apply a slow that is effective.

[Nyr97]

This seems more like a bug regarding speed calculation after the spell effect is gone while the slow is still up instead of being the spell itself.

[immvp]

Hand of Freedom should remove Slow effects, while making you immune. Currently, if you Hand of Freedom while having Hamstring active on you, then you'll run around as normal but Hamstring will still be active. (This is important to fix since it makes another bugfix impossible that I'll post after this one)

[immvp]

However if a Spell has multiple effects outside of a Slow, then it SHOULD apply the aura to the target THROUGH Freedom. However it should not slow, which it does currently.

1. Cast Hand of Freedom
2. Spam cast Frostbolt on the target (With Permafrost Talented)
3. You'll see the target gets slowed through Hand of Freedom

[immvp]

@adeutscher if you could add my Discord, I'd love to work with you on getting as many TrinityCore fixes done together. I'd pay you for your time. Mvq#0670

[adeutscher]

Discussed with @immvp in Discord and hammered out the below phrasing so that I could my head around how this is supposed to act:

1. Pre-existing snare effects are removed (secondary effect or no)
2. Snare-applying spells without a secondary effect cast during the Hand are ignored (immune)
3. Snare-applying spells with a secondary effect cast during the Hand are applied, but only the secondary non-slow effects actually have an effect. After Freedom fades, the snare part will have an effect

I'm still really conflicted on the first bullet, because I later found out that the existing code is clear about only zero-ing snare effects. SpellInfo::ApplyAllSpellImmunitiesTo has a carve-out that calls out Hand of Freedom by name:

if (apply && HasAttribute(SPELL_ATTR1_DISPEL_AURAS_ON_IMMUNITY))
{
    // exception for purely snare mechanic (eg. hands of freedom)!
    if (mechanicImmunity == (1 << MECHANIC_SNARE))
        target->RemoveMovementImpairingAuras(false);
    else
        target->RemoveAurasWithMechanic(mechanicImmunity, AURA_REMOVE_BY_DEFAULT, Id);
}

• https://www.youtube.com/watch?v=u-78kE_oNK0 : In favour of axing the aura. Freedom wipes Frost Armor at 1:58, wipes Frost Armor and Frostbolt at 3:15. This is my best lead because it was posted in August 2010.
• https://www.youtube.com/watch?v=zmB2YjUH5jE : In favour of axing the aura. Freedom appears to wipe frost armour at 1:00. Video is from 2011, so can't guarantee that it was recorded on WotLK retail.
• https://wowwiki-archive.fandom.com/wiki/Hand_of_Freedom: In favour of zeroing the slow effect. The patch notes for 3.2.2 reports "This ability now correctly removes the snaring component of Infected Wounds and Frostfire Bolt.", implying that it doesn't remove the auras outright in the WotLK-era.

The code was last edited in 2021 with c6e2b6e8. That commit is a revert of #20621. It looks like it was reverted after a user had the same read as me of the Infected Wounds/Frostfire Bolt patch, plus a bunch of WotLK-era video clips to back up their claim. I think the code revert puts a pretty big dent in my understanding of the first bullet point at the top of this comment.

All that being said, this logic is also making Living Action Potions not behave. Unlike Hand of Freedom, Living Action's tooltip is very clear about completely removing existing Stun/Root/Snare debuffs yet doesn't remove a Hamstring. This issue has plenty going on elsewhere with handling new applications and restoring the effect amount when Freedom goes away, so I'm going to file a separate issue for Living Action.

For the moment, I'm going to put aura application to the side for other people to hash out. Whichever way the above discussion pans out, Hand of Freedom can still have some flavour of aura active during it with a neutered snare effect. On that, we have the following ToDos:

• If single-effect snares auras are removed, need to confirm application behavior for single-effect snares vs. multi-effect snares. Lowest priority compared with the other bullets. If single-effect snares are not removed, then no change needed?
  • I'm kind of wondering if this has wider implications. For example, I don't remember being able to affect bosses (tested on Doom Lord Kazzak) a non-Permafrost Frostbolt aura.
• When Hand of Freedom fades, any snares on the target need to slow the target again. I've got a fix working for for this where movement impairing auras are re-calculated as @Nyr97 suggested. In 3.3.5, the handler does nothing to remedy squashed amounts when the immunity effect is de-applied. This fix is working. My test for this was cast Hamstring->Cast Freedom->Cancel Freedom, measuring movement in between each.
• Snares effects applied during Hand of Freedom need to have their amount reduced to 0 when applied to snare-immune targets as pre-existing effects.

After I've got new snares not reducing the speed of of an already Freedom'd target, I'm going to make a PR for those last two bullets.

[adeutscher]

Preview: My fix to re-calculate movement reduction when immunity fades is 93f317e0 .

[adeutscher]

After more discord discussion, revised the ToDo list:

1. Restore 0'd snares when immunity fades (done, 93f317e)
2. Instead of 0'ing single-effect snares (e.g. Hamstring), remove them completely (have a proof-of-concept in notepad, will apply to branch).
3. Newly applied auras (e.g. Permafrost Frostbolt) should 0 out snare effects if the target is mechanic-immune to snares (TODO)
4. If a newly-applied aura has only snare effects (e.g. Non-Permafrost Frostbolt), then don't apply the aura at all. Basically point 2, but for new auras (TODO)
5. (Edit): Make mechanic removal respect auras that ignore immunity (TODO)

[adeutscher]

All of these ToDos could address what I saw on Living Action. I'll test again after doing things for this before I make the separate issue.

[adeutscher]

Tests to run and include in the PR's Tests-Run section

1. Test restoration of snare effects when immunity is removed
   1. Select a spell with a multi-effect aura. Suggested: Frostbolt with Permafrost talent
   2. Cast the selected spell on target
   3. Observe speed, should be slower
   4. Cast Hand of Freedom on target.
   5. Observe debuffs, Multi-effect aura aura should still be on the target
   6. Observe speed, should be normal
   7. Before the snare aura ends, cancel Hand of Freedom
   8. Observe speed, should be slower again
2. Test removal of single-effect auras
   1. Select a spell that applies an aura whose only effect is a snare. Suggested: Blast Wave, hamstring
   2. Cast the selected spell on the target
   3. Cast Hand of Freedom on the target
   4. Observe debuffs, the snare aura should be removed.
3. Test ignoring of single-effect auras with a snare effect when Hand of Freedom is already active
   1. Select a spell with a non-aura effect and a snare effect. Suggested: Blast Wave
   2. Cast Hand of Freedom on the target.
   3. While Hand of Freedom is active, cast the selected snare spell on the target.
   4. Expected behavior: Target takes damage from Frostbolt, Frostbolt aura is not applied.
4. Test ignoring of snare effects on multi-effect auras when Hand of Freedom is already active
   1. Select a spell with a multi-effect aura. Suggested: Frostbolt with Permafrost talent
   2. Cast Hand of Freedom on the target.
   3. While Hand of Freedom is active, cast the selected snare spell on the target.
   4. Expected behavior: Target takes damage from Frostbolt, Frostbolt aura is applied
   5. Observe speed, should not be reduced
   6. Before Frostbolt ends, cancel Hand of Freedom
   7. Observe speed, should be slower
5. Test immunity respect
   1. Find a spell that snares AND ignores immunity. If you cannot find such a spell, then throw this entire test case out the window and exclude it from the PR description
   2. Apply spell (probably with .aura)
   3. Cast Hand of Freedom
   4. Observe speed, should still be snared

[adeutscher]

All this time I've been using Frostbolt - Permafrost as an example of a straightforward snare+effect. However, I didn't know until now that Frostbolt (and Cone of Cold) always have a -Healing effect. It's just that the -Healing % is 0 unless you pick up Permafrost. This means that I'll have to find a different test spell for test 3. Blast Wave is looking promising.

[immvp]

That's weird. Guess you'll have to check if Secondary Value is 0 rather than just NULL @adeutscher

[Keader]

Current behavior is correct. (not remove the whole aura)

http://www.warcraftmovies.com/movieview.php?id=164185 - (12:55) http://www.warcraftmovies.com/movieview.php?id=163271 - (06:37) http://www.warcraftmovies.com/movieview.php?id=162601 - (06:30) http://www.warcraftmovies.com/movieview.php?id=162601 - (07:04)

if i remember right, it should drop slow effect amount to 0 while it's active

[Shauren]

All of the slow auras used on those videos also have a second non-slow effect that prevents full removal

[Jildor]

Current behavior is correct. (not remove the whole aura)

http://www.warcraftmovies.com/movieview.php?id=164185 - (12:55) http://www.warcraftmovies.com/movieview.php?id=163271 - (06:37) http://www.warcraftmovies.com/movieview.php?id=162601 - (06:30) http://www.warcraftmovies.com/movieview.php?id=162601 - (07:04)

if i remember right, it should drop slow effect amount to 0 while it's active

the problem was introduced here: https://github.com/TrinityCore/TrinityCore/commit/93746e8c4a79c8256cd4896533315683f143508c removed here: https://github.com/TrinityCore/TrinityCore/pull/20621 and added again here: https://github.com/TrinityCore/TrinityCore/commit/c6e2b6e88c43b3ef79e4eab25b99b1799af161c3

the behavior is correct if you don't refresh aura problem is that ChangeAmount(0) in RemoveMovementImpairingAuras when you refresh spell with snare effect and you have immunity like hand of freedom, the amount set by default again instead of 0, ancient bug.

[immvp]

Shauren is correct, the auras in the videos have a SECOND NON SLOW EFFECT, so they should not removed completely.

This behavior can literally be tested on TBC Classic, and WOTLK Classic beta. If an ability has NO SECOND NON SLOW EFFECT then it should be removed completely and you should be immune from them for the remaining duration of Hand of Freedom.

Proof: https://youtu.be/-EqsWWzGRb8?t=420 (07:00) - Earthbind is down, but Paladin doesn't get the effect re-applied. Additionally, the effect gets completely removed. https://youtu.be/-EqsWWzGRb8?t=592 (09:52) - Rogue is spam re-applying Crippling Poison, yet the Paladin doesn't get affected.

[immvp]

In the videos you can even see that the NON SECOND SLOW EFFECT spells (Earthbind and Crippling Poison) get completely removed when Hand of Freedom is cast.

[adeutscher]

My PR (#28106) proposed a solution to this, but it was pointed out by @Shauren that the fix doubled down on a special exemption for snare immunity that probably shouldn't exist at all. I agree with this. If there's a more consistent way to attack this, then we should travel that route.

Due to outside factors, I don't think I'll be able to fully close the loop on this one. To anyone looking to pick up this bug: make sure that you read the comments in #28106, especially @Shauren 's review/suggestions near the end.

[github-actions[bot]]

3306a4d06cc557967c23adcc60c2e3257811b324

[shenhuyong]

After 3306a4d06cc557967c23adcc60c2e3257811b324 , Cyclone(33786) and Banish(18647) are bugged, their auras always disappears and appears every second until the duration ends.

[Faq]

@shenhuyong please make new issue.