Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core/Spells: Fix dispelling movement impairing auras by PvP-trinket and similar spells. #29966

Merged
merged 5 commits into from May 26, 2024

Conversation

r4d1sh
Copy link
Contributor

@r4d1sh r4d1sh commented May 8, 2024

Changes proposed:

After 3306a4d spells that grant immunity to mechanics do not completely remove the auras that these mechanics have on only one of their effects. Instead, these effects are only temporarily disabled.

However, there seem to be exceptions to this. For example, PvP-trinkets, or Human racial "Every Man for Himself".
Such spells are distinguished by the duration of their aura - only 100 ms. Of course, they should completely remove the auras they grant immunity to, and not just disable some effects for 100 milliseconds.

How to reproduce:

  1. Target yourself and use .aura 31589
  2. Use PvP-trinket or "Every Man for Himself"
  3. See that nothing worked as expected

This PR fixed the issue.

Tests performed:

Builded and tested in-game.

@r4d1sh
Copy link
Contributor Author

r4d1sh commented May 10, 2024

Spells with Duration ID 36 (1 second) have also been added to the exception list. Primarily, these are Blink and its various variations.

How to reproduce:

  1. Target yourself and use .aura 29300
  2. Use Blink
  3. See that nothing worked as expected

Fixed.

@Shauren
Copy link
Member

Shauren commented May 11, 2024

You are wrong about 29300 - I tested this on retail (both that spell and blink are unchanged) and it behaves like on TC, grants immunity only for blink mini duration and does not remove the aura (because mechanic is on one effect instead of entire spell)

@r4d1sh
Copy link
Contributor Author

r4d1sh commented May 11, 2024

You are wrong about 29300 - I tested this on retail (both that spell and blink are unchanged) and it behaves like on TC, grants immunity only for blink mini duration and does not remove the aura (because mechanic is on one effect instead of entire spell)

Wow, that's an amazing fact. Well, then you can merge only the first commit.

@Shauren
Copy link
Member

Shauren commented May 12, 2024

Did some more testing with 29300
Using pvp trinket on it removes the stun effect only, not whole aura (threat modifier remains)

@r4d1sh
Copy link
Contributor Author

r4d1sh commented May 14, 2024

Did some more testing with 29300
Using pvp trinket on it removes the stun effect only, not whole aura (threat modifier remains)

Stun effect removed completely or only for 100 ms?

And do you mean wotlk classic?

@Shauren
Copy link
Member

Shauren commented May 14, 2024

Completely removed

@r4d1sh
Copy link
Contributor Author

r4d1sh commented May 15, 2024

Tested on WotLK Classic (Cataclysm pre-patch): Human racial "Every Man for Himself" completely removes 29300, not only stun effect.

@Shauren
Copy link
Member

Shauren commented May 15, 2024

Alright then, just move setting the variable to spell id cases for pvp trinket and human racial that are already there instead of doing it by duration

@r4d1sh
Copy link
Contributor Author

r4d1sh commented May 15, 2024

What about spells like:

ID - 53490 Bullheaded (duration 407, 100 ms) - pretty sure that it should completely remove the auras it grant immunity to
ID - 48020 Demonic Circle: Teleport (duration 36, 1 sec) - pretty sure too, need to check on cata classic with 29300
Blink (duration 36, 1 sec) - pretty sure too, need to check on cata classic with 29300

Do you think it's better to hardcode specific spells?

@Shauren
Copy link
Member

Shauren commented May 15, 2024

Demonic circle only removes slows, not stuns or roots, you wont test it with 29300

@r4d1sh
Copy link
Contributor Author

r4d1sh commented May 15, 2024

I'll try to find an opportunity to test how Blink works with 29300 on wotlk/cata classic.

I'm 95% sure that both Blink and Demonic Circle should completely remove auras they grant immunity to, even if the mechanic is only stated on one of the effects.

I also suspect that Hand of Freedom is the only spell that worked this way in WotLK. But not sure...

@Shauren
Copy link
Member

Shauren commented May 15, 2024

Try your luck with feral druid talent Infected Wounds (it's the reason why current code behaves the way it does)

@r4d1sh
Copy link
Contributor Author

r4d1sh commented May 15, 2024

Try your luck with feral druid talent Infected Wounds (it's the reason why current code behaves the way it does)

"Every Man for Himself" completely dispelled "Infected Wounds" on 4.4.0.54670

@Shauren
Copy link
Member

Shauren commented May 15, 2024

Try pvp trinket and hand of freedom

@Shauren Shauren merged commit 45c579e into TrinityCore:3.3.5 May 26, 2024
1 of 5 checks passed
@r4d1sh r4d1sh deleted the pvp_trinket_fix branch May 26, 2024 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants