Skip to content

Commit

Permalink
[12835] Fix channeling spells for unit ( will not cast channeling spe…
Browse files Browse the repository at this point in the history 
…ll when walking, attacking and etc )
  • Loading branch information
@kvipka
kvipka authored and Cyberium committed Jan 28, 2015
1 parent 8bd1648 commit 5330f91
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 7 deletions.
12 changes: 6 additions & 6 deletions src/game/Spell.cpp
View file
Expand Up @@ -3558,10 +3558,10 @@ void Spell::update(uint32 difftime)
return;
}

// check if the player caster has moved before the spell finished
if ((m_caster->GetTypeId() == TYPEID_PLAYER && m_timer != 0) &&
(m_castPositionX != m_caster->GetPositionX() || m_castPositionY != m_caster->GetPositionY() || m_castPositionZ != m_caster->GetPositionZ()) &&
(m_spellInfo->Effect[EFFECT_INDEX_0] != SPELL_EFFECT_STUCK || !((Player*)m_caster)->m_movementInfo.HasMovementFlag(MOVEFLAG_FALLINGFAR)))
// check if the player or unit caster has moved before the spell finished (exclude casting on vehicles)
if (((m_caster->GetTypeId() == TYPEID_PLAYER || m_caster->GetTypeId() == TYPEID_UNIT) && m_timer != 0) &&
(m_castPositionX != m_caster->GetPositionX() || m_castPositionY != m_caster->GetPositionY() || m_castPositionZ != m_caster->GetPositionZ()) &&
(m_spellInfo->Effect[EFFECT_INDEX_0] != SPELL_EFFECT_STUCK || !m_caster->m_movementInfo.HasMovementFlag(MOVEFLAG_FALLINGFAR)))
{
// always cancel for channeled spells
if (m_spellState == SPELL_STATE_CASTING)
Expand Down Expand Up @@ -3590,10 +3590,10 @@ void Spell::update(uint32 difftime)
{
if (m_timer > 0)
{
if (m_caster->GetTypeId() == TYPEID_PLAYER)
if (m_caster->GetTypeId() == TYPEID_PLAYER || m_caster->GetTypeId() == TYPEID_UNIT)
{
// check if player has jumped before the channeling finished
if (((Player*)m_caster)->m_movementInfo.HasMovementFlag(MOVEFLAG_FALLING))
if (m_caster->m_movementInfo.HasMovementFlag(MOVEFLAG_FALLING))

This comment has been minimized.

Sign in to view

Copy link
@Ve1vet

Ve1vet Mar 13, 2015

if( m_caster->m_movementInfo.HasMovementFlag(MOVEFLAG_FALLING) && m_spellInfo->HasAttribute(SPELL_ATTR_EX3_UNK28 ) )

Will fix curator & all spells with this attribute(in this case). In magos this attribute still unk, but it should be "always cast ok". Those spells should be casted in any cases even if caster under crowd control/moving etc. except maybe while not dead :)

This comment has been minimized.

Sign in to view

Copy link
@xfurry

xfurry May 3, 2015

Member

I'm not very sure about attr_ex_unk28. Too many spells have that.
Besides, the Curator doesn't have any problem with the falling flag. It'a actually related to the line just below, regarding incapacitating states, because the Evocation spell, also applies a stun effect for the duration of the cast.

This patch should fix the curator (and probably others as well)

diff --git a/src/game/Spell.cpp b/src/game/Spell.cpp
index f8762ea..a68e0fa 100644
--- a/src/game/Spell.cpp
+++ b/src/game/Spell.cpp
@@ -3597,7 +3597,7 @@ void Spell::update(uint32 difftime)
                         cancel();

                     // check for incapacitating player states
-                    if (m_caster->hasUnitState(UNIT_STAT_CAN_NOT_REACT))
+                    if (m_caster->hasUnitState(UNIT_STAT_CAN_NOT_REACT) && !m_spellInfo->HasAttribute(SPELL_ATTR_UNAFFECTED_BY_INVULNERABILITY))
                         cancel();

                     // check if player has turned if flag is set

But there are other spells, such as 36142, which doesn't have any attribute that can avoid interruption.
cancel();

// check for incapacitating player states
Expand Down
2 changes: 1 addition & 1 deletion src/shared/revision_nr.h
View file
@@ -1,4 +1,4 @@
#ifndef __REVISION_NR_H__
#define __REVISION_NR_H__
#define REVISION_NR "12834"
#define REVISION_NR "12835"
#endif // __REVISION_NR_H__

21 comments on commit 5330f91

@Grz3s
Copy link
Member

@Grz3s Grz3s commented on 5330f91 Jan 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome !!

@kvipka
Copy link
Contributor Author

@kvipka kvipka commented on 5330f91 Jan 28, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grz3s it working? When i have tested, on my repo this way haven't worked.
( m_caster->m_movementInfo )
And on this reason i did 2 ways, special for players and special for units.

@cyberium
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kvipka

((Player*)m_caster)->m_movementInfo.HasMovementFlag(MOVEFLAG_FALLING)
and
m_caster->m_movementInfo.HasMovementFlag(MOVEFLAG_FALLING)

Is same in that case, no need for typecast.

@overy
Copy link

@overy overy commented on 5330f91 Feb 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are problem with boss Curator after this commit, he spam spell SPELL_EVOCATION because he break this channeling by move.

@Grz3s
Copy link
Member

@Grz3s Grz3s commented on 5330f91 Feb 14, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this patch only breaks channeling - when channel is forced to do. (should he be forced to cast this spell?)
Also ... he shouldnt move at all while channeling this spell

Effect 1: Id 6 (SPELL_EFFECT_APPLY_AURA)
BasePoints = 1
Targets (1, 0) (TARGET_SELF, NO_TARGET)
Aura Id 12 (SPELL_AURA_MOD_STUN), value = 1, misc = 0 (0), miscB = 0, periodic = 0

pls report this on sd2.

@Schmoozerd
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks a bit like some other issue (like bad moveflag or such).

if (DoCastSpellIfCan(m_creature, SPELL_EVOCATION, CAST_INTERRUPT_PREVIOUS) == CAST_OK)

normal cast spell, should not result in any issue at all.

@xfurry
Copy link
Member

@xfurry xfurry commented on 5330f91 Mar 10, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Schmoozerd I tried to remove the CAST_INTERRUPT_PREVIOUS flag, but there is still a problem.
It seems that the boss will still interrupt the channeling despite the fact that it should have AURA_MOD_STUN.

Also, the patch proposed by @kvipka won't apply to this spell (but might be good for others)

diff --git a/src/game/SharedDefines.h b/src/game/SharedDefines.h
index 26dd5f4..e7f4f51 100644
--- a/src/game/SharedDefines.h
+++ b/src/game/SharedDefines.h
@@ -429,7 +429,7 @@ enum SpellAttributesEx4

 enum SpellAttributesEx5
 {
-    SPELL_ATTR_EX5_UNK0                        = 0x00000001,// 0
+    SPELL_ATTR_EX5_CAN_CHANNEL_WHEN_MOVING     = 0x00000001,// 0 don't interrupt channeling spells when moving
     SPELL_ATTR_EX5_NO_REAGENT_WHILE_PREP       = 0x00000002,// 1 not need reagents if UNIT_FLAG_PREPARATION
     SPELL_ATTR_EX5_UNK2                        = 0x00000004,// 2 removed at enter arena (e.g. 31850 since 3.3.3)
     SPELL_ATTR_EX5_USABLE_WHILE_STUNNED        = 0x00000008,// 3 usable while stunned
diff --git a/src/game/Spell.cpp b/src/game/Spell.cpp
index 18d6ec4..1bb6720 100644
--- a/src/game/Spell.cpp
+++ b/src/game/Spell.cpp
@@ -3564,7 +3564,7 @@ void Spell::update(uint32 difftime)
         (m_spellInfo->Effect[EFFECT_INDEX_0] != SPELL_EFFECT_STUCK || !m_caster->m_movementInfo.HasMovementFlag(MOVEFLAG_FALLINGFAR)))
     {
         // always cancel for channeled spells
-        if (m_spellState == SPELL_STATE_CASTING)
+        if (m_spellState == SPELL_STATE_CASTING && !m_spellInfo->HasAttribute(SPELL_ATTR_EX5_CAN_CHANNEL_WHEN_MOVING))
             cancel();
         // don't cancel for melee, autorepeat, triggered and instant spells
         else if (!IsNextMeleeSwingSpell() && !IsAutoRepeat() && !m_IsTriggeredSpell && (m_spellInfo->InterruptFlags & SPELL_INTERRUPT_FLAG_MOVEMENT))

@Ve1vet
Copy link

@Ve1vet Ve1vet commented on 5330f91 Mar 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xfurry this spell has AttributesEx3: 0x10000000 (SPELL_ATTR_EX3_ALWAYS_CAST_OK). So this attribute can be used in this fix, because this spells should always be casted in any case(moving, under crowd control etc).

So this should be implement in core. This is not SD2 bug.

@kvipka
Copy link
Contributor Author

@kvipka kvipka commented on 5330f91 Mar 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

vovk , what spell?

@xfurry
Copy link
Member

@xfurry xfurry commented on 5330f91 Mar 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vovk I think we are talking about the Curator spell problem
the spell is casted, but because of this commit it will interrupt on movement.

The issue is that the Evocation spell that the npc casts should also apply AURA_MOD_STUN
However for some reason it doesn't happen. Maybe it's because these auras are restricted to be used on self?

@VladimirMangos
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe some negative/positive target check....

@Ve1vet
Copy link

@Ve1vet Ve1vet commented on 5330f91 Mar 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VladimirMangos, @xfurry, no. This is bacause of immune stun flag in DB :) Curator immune to stuns, so stun aura won't apply.

Maybe we can add exception for all CC auras if player/unit cast spell on itself, so immune flags for that cases will not work. This should fix curator stun.

P.S. That flag should be in this place. There are many spells with this flag, that have casting time > 0. They all will not be cast while mob move.

@xfurry
Copy link
Member

@xfurry xfurry commented on 5330f91 Mar 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that @Schmoozerd implemented some fix for this kind of self cast auras that apply root / stun / freeze effects.
But I'm not sure if this covered everything or was incomplete.

@Ve1vet
Copy link

@Ve1vet Ve1vet commented on 5330f91 Mar 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bool Unit::IsImmuneToSpell(SpellEntry const* spellInfo, bool castOnSelf)
bool Unit::IsImmuneToSpellEffect(SpellEntry const* spellInfo, SpellEffectIndex index, bool castOnSelf)
Both has castOnSelf, but not implemented in those functions at all :) Someone start do this thing, but stop on halfway...

@VladimirMangos
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is virtual function and args used in Creature::IsImmuneToSpell override for creatures and totems

@Ve1vet
Copy link

@Ve1vet Ve1vet commented on 5330f91 Mar 13, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VladimirMangos in Player also. But thats not a point. In Unit.cpp we can add check if cast on self, return false. Something like that.
By the way, it also will fix manaspring/manatide totems etc. My old fix mangosArchives/Mangos-One-server-old@bd31a09
This is the same issue.

Upd. After some thoughts... This check should be in Player/Creature/Totem file at the start of functions, because if it will be in Unit.cpp - already to late.

@VladimirMangos
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exist proper self stun casts as i remeber. But meybe i am wrong in this. At least this need re-check in spell data before add.

@xfurry
Copy link
Member

@xfurry xfurry commented on 5330f91 Apr 19, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One more problem / question regarding this commit.

Are we 100% sure that this applies to both SPELL_ATTR_EX_CHANNELED_1 and SPELL_ATTR_EX_CHANNELED_2 ?

@Ve1vet
Copy link

@Ve1vet Ve1vet commented on 5330f91 Apr 19, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xfurry this commit should be, but there is one thing that should be done before this commit... prevent mobs move while casting, for now all of them get movement generator for move to target & they run while casting, so we have bug that brokes many spells like, Electrical Storm Akil'Zon, Sukkubus Seduction & many other spells...

@xfurry
Copy link
Member

@xfurry xfurry commented on 5330f91 Apr 19, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, there are mobs that should move while casting.
For example Vazruden in Hellfire Ramparts. This mob should be able to cast the fireball while moving.

@Ve1vet
Copy link

@Ve1vet Ve1vet commented on 5330f91 Apr 20, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@xfurry you have two spells on Vazruden in HR 36920 & 34653 with casting time 0(instant), but for some reason they casts only while landed. This spell could be casted while moving easy. In TC for example only two fireballs 36920(heroic) & 34653(normal), so they perfect mach for this case.

Please sign in to comment.