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

[3.3.5] Core/Spells: Vampiric Touch and Unstable Affliction (dispel effect) shouldn't ignore resilience #22270

Open
Jildor opened this issue Aug 15, 2018 · 12 comments

Comments

@Jildor
Copy link
Contributor

Jildor commented Aug 15, 2018

Description:

ID - 64085 Vampiric Touch (dispel effect)
ID - 31117 Unstable Affliction (dispel effect)
Those spells shouldn't ignore resilience

Seems this @ariel- commit doesn't work: 3753ec5

Steps to reproduce the problem:

  1. Cast Vampiric Touch or Unstable Affliction on player
  2. Clean debuff, by example paladin Cleanse spell
  3. See damage
  4. Get resilience equipping by example pvp trinket
  5. Repeat and see damage

Branch(es):

3.3.5

TC rev. hash/commit:

TrinityCore rev. 9f3e6bfe9b76 2018-08-14 01:58:24 +0200 (3.3.5 branch) (Unix, Release, Static)
Using SSL version: OpenSSL 1.0.2l  25 May 2017 (library: OpenSSL 1.0.2l  25 May 2017)
Using Boost version: 1.62.0
Using MySQL version: 10.1.26-MariaDB
Using CMake version: 3.7.2
Compiled on: Linux 4.9.0-4-amd64
Automatic database updates are enabled for the following databases: Auth, Characters, World
Worldserver listening connections on port 8085
Realmlist (Realm Id: 1) configured in port 8085
VMAPs status: Enabled. LineOfSight: 1, getHeight: 1, indoorCheck: 1
MMAPs status: Enabled
maps directory located in /home/trinity/data/335/data/maps. Total size: 252191207 bytes
vmaps directory located in /home/trinity/data/335/data/vmaps. Total size: 588247501 bytes
mmaps directory located in /home/trinity/data/335/data/mmaps. Total size: 2152621156 bytes
Using esES DBC Locale as default. All available DBC locales: esES
Using World DB: TDB 335.64
@Keader
Copy link
Member

Keader commented Aug 15, 2018

#21684

#21556

#21393

@Jildor
Copy link
Contributor Author

Jildor commented Aug 15, 2018

Yep, I know those issues @Keader, the issue is real, what is the problem? 😄
(maybe you only are linking old issues, but I don't understand you very well 😄 )

@Exenu
Copy link

Exenu commented Aug 15, 2018

@Jildor He's referencing the issues so they all come under one title.

@Langerz82
Copy link
Contributor

Can you apply patch below and re-test?
#22051 (comment)

@Strazhnik
Copy link

Strazhnik commented Mar 17, 2019

Confirmed on c76931c
In 1000 res, damage =15000 with 3.5k spd

@Strazhnik
Copy link

Strazhnik commented Apr 17, 2019

@Langerz82 , fix don't work and have crash

@Strazhnik
Copy link

@xvwyh, can you comment this spell?

@xvwyh
Copy link
Contributor

xvwyh commented Apr 26, 2019

The reason they ignore resilience is obvious:
Unit::CalculateSpellDamageTaken

    // Spells with SPELL_ATTR4_FIXED_DAMAGE ignore resilience because their damage is based off another spell's damage.
    if (!spellInfo->HasAttribute(SPELL_ATTR4_FIXED_DAMAGE))
    {

Resilience is ignored if spell has SPELL_ATTR4_FIXED_DAMAGE attribute. Unstable Affliction has it. Vampiric Touch doesn't, but it's getting added in a hack in SpellMgr.cpp:

    // Vampiric Touch (dispel effect)
    ApplySpellFix({ 64085 }, [](SpellInfo* spellInfo)
    {
        // copy from similar effect of Unstable Affliction (31117)
        spellInfo->AttributesEx4 |= SPELL_ATTR4_FIXED_DAMAGE;
        spellInfo->AttributesEx6 |= SPELL_ATTR6_LIMIT_PCT_DAMAGE_MODS;
    });

If those spells (Unstable Affliction primarily, as the code in SpellMgr.cpp is a hack and shouldn't be considered blizzlike behavior) should be affected by resilience, then you need to reevaluate whether the condition in Unit::CalculateSpellDamageTaken is correct. If there are enough cases with SPELL_ATTR4_FIXED_DAMAGE that should be affected by resilience - you need to change the condition, maybe resort to flagging the spells that DON'T need to affected by resilience manually with some sort of a custom attribute. If these are the only outliers and you don't want to make such a big change - add a script and manually apply resilience reduction when calculating the damage, I dunno.

@Strazhnik
Copy link

@jackpoz , after the great fix gargoyles, you can check vampiric touch?)

@jackpoz
Copy link
Member

jackpoz commented May 4, 2019

You can start by doing the research explained above

@Warpten
Copy link
Member

Warpten commented Nov 6, 2019

The obvious issue here is that it using fixed damage needs to only happen on the dispel effect - otherwise the damage applied from dispelling double dips on resilience, making this dot a non-threat while it should be.

If you want the expected damage on a target with no magic resistance, just take the tooltip value and apply resilience to it.

@V3nomAnc3r
Copy link

Ops xD 17k Dmg when i Cleanse (vampiric touch) resil 1500

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants