Skip to content

Commit

Permalink
Core/Spells: attempt to correct issue with triggered spells happening…
Browse files Browse the repository at this point in the history
… before the spell actually hits the target. reply with any bugs that this causes please!
  • Loading branch information
Star-lion committed May 24, 2012
1 parent 51388c4 commit d98f361
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions src/server/game/Spells/SpellEffects.cpp
Expand Up @@ -849,15 +849,15 @@ void Spell::EffectDummy(SpellEffIndex effIndex)

void Spell::EffectTriggerSpell(SpellEffIndex effIndex)
{
if (effectHandleMode != SPELL_EFFECT_HANDLE_LAUNCH_TARGET
&& effectHandleMode != SPELL_EFFECT_HANDLE_LAUNCH)
if (effectHandleMode != SPELL_EFFECT_HANDLE_HIT_TARGET
&& effectHandleMode != SPELL_EFFECT_HANDLE_HIT)
return;

uint32 triggered_spell_id = m_spellInfo->Effects[effIndex].TriggerSpell;

// todo: move those to spell scripts
if (m_spellInfo->Effects[effIndex].Effect == SPELL_EFFECT_TRIGGER_SPELL
&& effectHandleMode == SPELL_EFFECT_HANDLE_LAUNCH_TARGET)
&& effectHandleMode == SPELL_EFFECT_HANDLE_HIT_TARGET)
{
// special cases
switch (triggered_spell_id)
Expand Down Expand Up @@ -970,13 +970,13 @@ void Spell::EffectTriggerSpell(SpellEffIndex effIndex)
}

SpellCastTargets targets;
if (effectHandleMode == SPELL_EFFECT_HANDLE_LAUNCH_TARGET)
if (effectHandleMode == SPELL_EFFECT_HANDLE_HIT_TARGET)
{
if (!spellInfo->NeedsToBeTriggeredByCaster())
return;
targets.SetUnitTarget(unitTarget);
}
else //if (effectHandleMode == SPELL_EFFECT_HANDLE_LAUNCH)
else //if (effectHandleMode == SPELL_EFFECT_HANDLE_HIT)
{
if (spellInfo->NeedsToBeTriggeredByCaster() && (m_spellInfo->Effects[effIndex].GetProvidedTargetMask() & TARGET_FLAG_UNIT_MASK))
return;
Expand Down

11 comments on commit d98f361

@Judiny
Copy link

@Judiny Judiny commented on d98f361 May 25, 2012

Choose a reason for hiding this comment

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

Spell lock (warlock's pet silence) fix?

@Star-lion
Copy link
Contributor Author

Choose a reason for hiding this comment

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

hopefully

@Phaseowner
Copy link

Choose a reason for hiding this comment

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

yup spell lock work fine
thank you

@Chrisnetika
Copy link

Choose a reason for hiding this comment

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

Thanks Kandera.. your awesome.. thank you for your hard work here :D

@PuniCZ
Copy link

@PuniCZ PuniCZ commented on d98f361 May 25, 2012

Choose a reason for hiding this comment

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

We have it fixed like that for few months, works fine.

@QAston
Copy link
Contributor

@QAston QAston commented on d98f361 May 25, 2012

Choose a reason for hiding this comment

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

EffectTriggerSpell is all about triggering before spell hits target. See warrior's charge as an example. Charge stun should be applied before player reaches target.

@Warpten
Copy link
Member

Choose a reason for hiding this comment

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

Agreed Q.

@QAston
Copy link
Contributor

@QAston QAston commented on d98f361 May 25, 2012

Choose a reason for hiding this comment

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

I've checked the reason of spell lock problem. The spell is not working correctly because we currently handle haunch and hit phases separately in a case where spell is not a delayed spell. This makes all effects which work on LAUNCH phase to be executed before effects on HIT phase for instant hit spells, so the order of effect execution is disturbed and therfore SPELL_EFFECT_INTERRUPT_CAST is executed after SPELL_EFFECT_TRIGGER_SPELL, and that causes spelllock to fail. In conclusion: the problem is caused by how we handle hit on targets currently, SPELL_EFFECT_TRIGGER_SPELL should not be handled on hit (because delayed spells require this). This commit needs to be reverted and replaced with sollution that doesn't break delayed spells with SPELL_EFFECT_TRIGGER_SPELL. Fixing hit phase handling would be perfect, but it's rarther a long term goal.

@Star-lion
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i was waiting for someone to come to me with this. ill revert it :D

@Toshik
Copy link

@Toshik Toshik commented on d98f361 May 25, 2012

Choose a reason for hiding this comment

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

issue #6042 related?

@Star-lion
Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

Please sign in to comment.