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: Fixed spell Health Funnel and Improved Health Funnel #20559

Closed
wants to merge 1 commit into from

Conversation

Keader
Copy link
Member

@Keader Keader commented Oct 5, 2017

Changes proposed:

  • Fixed spell Health Funnel consuming 3 times more health that should.
  • Fixed reduce of health using Health Funnel when warlock has Improved Health Funnel talent.

Issue:

Current script remove warlock health in 2 points.
Here: https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/game/Spells/Auras/SpellAuraEffects.cpp#L5941
And Here: https://github.com/TrinityCore/TrinityCore/blob/3.3.5/src/server/game/Spells/Auras/SpellAuras.cpp#L706
The second point, is called 2 times: First call is caused by Warlock Aura. The second is caused by Pet Aura.
And current script, dont check any SPELLMOD_COST, so reduce cost part of Improved Health Funnel not work.

Target branch(es): 3.3.5

Issues addressed: has no open issues.

Tests performed: Builded and tested in game

@Shauren
Copy link
Member

Shauren commented Oct 7, 2017

What do you think about removing the health drop from PERIODIC_HEAL handler and only keeping the one in Aura::Update? The way I see it is SPELL_ATTR2_HEALTH_FUNNEL only changes the way health is removed - from raw ModifyHealth to DealDamage

@Keader
Copy link
Member Author

Keader commented Oct 7, 2017

You say something like: https://gist.github.com/Keader/ba483cbfae82c2a30393f78cba470c7c ?
Ps: m_owner == caster because AuraUpdate is called for both auras, Pet and Warlock

@Shauren
Copy link
Member

Shauren commented Oct 7, 2017

Hmm, I did not expect this - caster != owner check breaks all auras with health cost that target anything other than caster but at the same time they are already broken in the same way as health funnel when targeting multiple units
Probably each spell cast should only be creating a single aura object for channeled spells with multiple applications (could also fix mind control DR)

@Keader
Copy link
Member Author

Keader commented Oct 12, 2017

@Shauren you will do it? because idk how do it

@Keader
Copy link
Member Author

Keader commented Dec 16, 2017

Maybe @ariel- can do it?

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

3 participants