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] [Spell][Hunter] Freezing Arrow(60192) is not working #15794

Closed
shenhuyong opened this Issue Nov 1, 2015 · 47 comments

Comments

Projects
None yet
@shenhuyong

shenhuyong commented Nov 1, 2015

The category of spell Freezing Arrow(60192) is 411, and the category of trigger spell 60202 is 411 too. So after 60192 casted, the trigger spell 60202 is in a cooldown.
After ver. 7dcddd9

@shenhuyong shenhuyong changed the title from [3.3.5] Frozen arrow(60192) is not working to [3.3.5] Spell Frozen arrow(60192) is not working Nov 1, 2015

@shenhuyong shenhuyong changed the title from [3.3.5] Spell Frozen arrow(60192) is not working to [3.3.5] [Spell][Hunter] Frozen arrow(60192) is not working Nov 1, 2015

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 1, 2015

Isn't it correct for spells to share cooldown when the spells are tied together in the same action?
If the cooldown is identical for both, they will both be available again at the same time, right?

ghost commented Nov 1, 2015

Isn't it correct for spells to share cooldown when the spells are tied together in the same action?
If the cooldown is identical for both, they will both be available again at the same time, right?

@shenhuyong

This comment has been minimized.

Show comment
Hide comment
@shenhuyong

shenhuyong Nov 1, 2015

Yes, after ver.7dcddd9 , it Improved spell category cooldown handling. All of spells in same category will start cooldowns and save to database after a spell one of them be casted. So spell 60192 can't triggering spell 60202, because the trigger spell 60202 also has a cooldown.

shenhuyong commented Nov 1, 2015

Yes, after ver.7dcddd9 , it Improved spell category cooldown handling. All of spells in same category will start cooldowns and save to database after a spell one of them be casted. So spell 60192 can't triggering spell 60202, because the trigger spell 60202 also has a cooldown.

@Aokromes Aokromes added the Comp-Core label Nov 1, 2015

@ariel-

This comment has been minimized.

Show comment
Hide comment
@ariel-

ariel- Nov 1, 2015

Member

Shouldn't triggered spells (specially with triggered flag TRIGGERED_IGNORE_SPELL_AND_CATEGORY_CD) do just that and go ahead with the casting anyways?

Member

ariel- commented Nov 1, 2015

Shouldn't triggered spells (specially with triggered flag TRIGGERED_IGNORE_SPELL_AND_CATEGORY_CD) do just that and go ahead with the casting anyways?

@shenhuyong

This comment has been minimized.

Show comment
Hide comment
@shenhuyong

shenhuyong Nov 1, 2015

After I used “.cheat cooldown” command, the spell 60192 works fine, it can triggered spell 60202.

shenhuyong commented Nov 1, 2015

After I used “.cheat cooldown” command, the spell 60192 works fine, it can triggered spell 60202.

@sirikfoll

This comment has been minimized.

Show comment
Hide comment
@sirikfoll

sirikfoll Nov 3, 2015

Contributor

Maybe this?

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 76eefa0..c8d0b84 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -4668,7 +4668,7 @@ SpellCastResult Spell::CheckCast(bool strict)
                 return SPELL_FAILED_NOT_READY;
         }

-        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
+        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && !IsIgnoringCooldowns())
         {
             if (m_triggeredByAuraSpell)
                 return SPELL_FAILED_DONT_REPORT;
Contributor

sirikfoll commented Nov 3, 2015

Maybe this?

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 76eefa0..c8d0b84 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -4668,7 +4668,7 @@ SpellCastResult Spell::CheckCast(bool strict)
                 return SPELL_FAILED_NOT_READY;
         }

-        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
+        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && !IsIgnoringCooldowns())
         {
             if (m_triggeredByAuraSpell)
                 return SPELL_FAILED_DONT_REPORT;
@superllout

This comment has been minimized.

Show comment
Hide comment
@superllout

superllout Nov 9, 2015

if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && !IsIgnoringCooldowns())

This will broke Deathbringer's Will. All auras from "Deathbringer's Will" will apply on player together.
Tested.

superllout commented Nov 9, 2015

if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && !IsIgnoringCooldowns())

This will broke Deathbringer's Will. All auras from "Deathbringer's Will" will apply on player together.
Tested.

@sirikfoll

This comment has been minimized.

Show comment
Hide comment
@sirikfoll

sirikfoll Nov 10, 2015

Contributor

Did anybody notest any other broken spells, besides Frozen arrow?

Contributor

sirikfoll commented Nov 10, 2015

Did anybody notest any other broken spells, besides Frozen arrow?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 12, 2015

That's what I noticed a problem with the Frozen arrow .

ghost commented Nov 12, 2015

That's what I noticed a problem with the Frozen arrow .

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Nov 12, 2015

I would like to point out that Frozen Arrow (http://www.wowhead.com/spell=126321/frozen-arrow) is an NPC ability used by Huo-Shuang (http://www.wowhead.com/npc=63691/huo-shuang).

Freezing Arrow (http://wow.gamepedia.com/Freezing_Arrow / http://wotlk.openwow.com/spell=60192),
on the other hand, is the level 80 hunter trap spell in 3.3.5 (removed in patch 4.0.1).


Additional Freezing Arrow information details (Freezing Arrow vs Freezing Trap) :
http://wotlk.openwow.com/spell=60192/freezing-arrow 3.3.5 version, actual spell used by player
http://wotlk.openwow.com/spell=60202/freezing-arrow spell 60202 trigger for 60192
http://www.wowhead.com/spell=60192/freezing-trap reused as ordinary trap spell after patch 4.0.1
http://www.wowhead.com/spell=60202/freezing-trap reused as trap spell trigger after patch 4.0.1
http://wow.gamepedia.com/Freezing_Arrow Level 80 survival hunter ability

ghost commented Nov 12, 2015

I would like to point out that Frozen Arrow (http://www.wowhead.com/spell=126321/frozen-arrow) is an NPC ability used by Huo-Shuang (http://www.wowhead.com/npc=63691/huo-shuang).

Freezing Arrow (http://wow.gamepedia.com/Freezing_Arrow / http://wotlk.openwow.com/spell=60192),
on the other hand, is the level 80 hunter trap spell in 3.3.5 (removed in patch 4.0.1).


Additional Freezing Arrow information details (Freezing Arrow vs Freezing Trap) :
http://wotlk.openwow.com/spell=60192/freezing-arrow 3.3.5 version, actual spell used by player
http://wotlk.openwow.com/spell=60202/freezing-arrow spell 60202 trigger for 60192
http://www.wowhead.com/spell=60192/freezing-trap reused as ordinary trap spell after patch 4.0.1
http://www.wowhead.com/spell=60202/freezing-trap reused as trap spell trigger after patch 4.0.1
http://wow.gamepedia.com/Freezing_Arrow Level 80 survival hunter ability

@ccrs

This comment has been minimized.

Show comment
Hide comment
@ccrs

ccrs Nov 13, 2015

Contributor

http://wotlk.openwow.com/spell=56453 Lock and Load doesnt seem to proc too.

Contributor

ccrs commented Nov 13, 2015

http://wotlk.openwow.com/spell=56453 Lock and Load doesnt seem to proc too.

@ccrs

This comment has been minimized.

Show comment
Hide comment
@ccrs

ccrs Nov 13, 2015

Contributor

http://wotlk.openwow.com/spell=69487 overheat doesnt proc anymore

Contributor

ccrs commented Nov 13, 2015

http://wotlk.openwow.com/spell=69487 overheat doesnt proc anymore

@shenhuyong shenhuyong changed the title from [3.3.5] [Spell][Hunter] Frozen arrow(60192) is not working to [3.3.5] [Spell][Hunter] Freezing Arrow(60192) is not working Nov 15, 2015

@ccrs

This comment has been minimized.

Show comment
Hide comment
@ccrs

ccrs Nov 21, 2015

Contributor

it seems like several mechanics are broken right now, id say this is an important issue xd

Contributor

ccrs commented Nov 21, 2015

it seems like several mechanics are broken right now, id say this is an important issue xd

@Demonid

This comment has been minimized.

Show comment
Hide comment
@Demonid

Demonid Nov 22, 2015

@ccrs Yep, lets hope @Shauren gets some free time and checks this 👍

Demonid commented Nov 22, 2015

@ccrs Yep, lets hope @Shauren gets some free time and checks this 👍

@ShinDarth

This comment has been minimized.

Show comment
Hide comment
@ShinDarth

ShinDarth Nov 23, 2015

Member

Confirmed on 302623a

Member

ShinDarth commented Nov 23, 2015

Confirmed on 302623a

@Re3os

This comment has been minimized.

Show comment
Hide comment
@Re3os

Re3os Nov 24, 2015

@sirikfoll I'm tested you fix,this aura stacked ID 71560,71484,71559,71491.

Re3os commented Nov 24, 2015

@sirikfoll I'm tested you fix,this aura stacked ID 71560,71484,71559,71491.

@Re3os

This comment has been minimized.

Show comment
Hide comment
@Re3os

Re3os commented Nov 24, 2015

@tkrokli sori,mobile.
All auras from
http://www.wowhead.com/item=50362
http://www.wowhead.com/item=50363
stack.

@ariel-

This comment has been minimized.

Show comment
Hide comment
@ariel-

ariel- Nov 24, 2015

Member

Interesting you mention DBW procs, since they're handled differently, in HandleDummyAuraProc.

Also, IsReady doesn't check only for cooldowns, it also checks school locks, that way, if a spell is school locked, and cast using TRIGGERED_IGNORE_COOLDOWNS will be casted anyways.

Member

ariel- commented Nov 24, 2015

Interesting you mention DBW procs, since they're handled differently, in HandleDummyAuraProc.

Also, IsReady doesn't check only for cooldowns, it also checks school locks, that way, if a spell is school locked, and cast using TRIGGERED_IGNORE_COOLDOWNS will be casted anyways.

@Re3os

This comment has been minimized.

Show comment
Hide comment
@Re3os

Re3os Nov 24, 2015

@ariel- Maybe you give us hack?

DELETE FROM spell_group WHERE `spell_id` IN (71559, 71491, 71484, 71492, 71561, 71560);
INSERT INTO spell_group (id, spell_id) VALUES
('1222','71559'),
('1222','71491'),
('1222','71484'),
('1222','71492'),
('1222','71561'),
('1222','71560');
DELETE FROM spell_group_stack_rules WHERE `group_id` = 1222;
INSERT INTO spell_group_stack_rules (group_id,stack_rule) VALUES
('1222' ,'1');

Re3os commented Nov 24, 2015

@ariel- Maybe you give us hack?

DELETE FROM spell_group WHERE `spell_id` IN (71559, 71491, 71484, 71492, 71561, 71560);
INSERT INTO spell_group (id, spell_id) VALUES
('1222','71559'),
('1222','71491'),
('1222','71484'),
('1222','71492'),
('1222','71561'),
('1222','71560');
DELETE FROM spell_group_stack_rules WHERE `group_id` = 1222;
INSERT INTO spell_group_stack_rules (group_id,stack_rule) VALUES
('1222' ,'1');
@Laintime

This comment has been minimized.

Show comment
Hide comment
@Laintime

Laintime Nov 25, 2015

Contributor

@Re3os thx.
confirm 5df0bcd

Contributor

Laintime commented Nov 25, 2015

@Re3os thx.
confirm 5df0bcd

@Re3os

This comment has been minimized.

Show comment
Hide comment
@Re3os

Re3os Nov 28, 2015

@ccrs Not fiks this commit,confirm e266278

Re3os commented Nov 28, 2015

@ccrs Not fiks this commit,confirm e266278

@ShinDarth

This comment has been minimized.

Show comment
Hide comment
@ShinDarth

ShinDarth Nov 28, 2015

Member

you can easily hackfix it just by replacing:

if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))

with:

if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && m_spellInfo->Id != 60202)

but it would be nice to have a proper solution for this issue.

Member

ShinDarth commented Nov 28, 2015

you can easily hackfix it just by replacing:

if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))

with:

if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && m_spellInfo->Id != 60202)

but it would be nice to have a proper solution for this issue.

@Re3os

This comment has been minimized.

Show comment
Hide comment
@Re3os

Re3os Nov 28, 2015

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 99e2d3a..0966900 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -4856,7 +4856,7 @@ SpellCastResult Spell::CheckCast(bool strict)
                 return SPELL_FAILED_NOT_READY;
         }

-        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
+        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && m_spellInfo->Id != 60202)
         {
             if (m_triggeredByAuraSpell)
                 return SPELL_FAILED_DONT_REPORT;

Re3os commented Nov 28, 2015

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 99e2d3a..0966900 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -4856,7 +4856,7 @@ SpellCastResult Spell::CheckCast(bool strict)
                 return SPELL_FAILED_NOT_READY;
         }

-        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
+        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && m_spellInfo->Id != 60202)
         {
             if (m_triggeredByAuraSpell)
                 return SPELL_FAILED_DONT_REPORT;
@ShinDarth

This comment has been minimized.

Show comment
Hide comment
@ShinDarth

ShinDarth Nov 28, 2015

Member

yes @Re3os that is it, but as I said we need a proper solution

Member

ShinDarth commented Nov 28, 2015

yes @Re3os that is it, but as I said we need a proper solution

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Dec 19, 2015

any news?

ghost commented Dec 19, 2015

any news?

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 18, 2016

@shenhuyong , @Re3os , @Demonid : check if the suggested fix above solves the issue for you.

ghost commented Jan 18, 2016

@shenhuyong , @Re3os , @Demonid : check if the suggested fix above solves the issue for you.

@Eliminationzx

This comment has been minimized.

Show comment
Hide comment
@Eliminationzx

Eliminationzx Jan 18, 2016

Contributor

Try it

 src/server/game/Spells/Spell.cpp | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 84bff4a..ca9f642 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -4701,13 +4701,13 @@ SpellCastResult Spell::CheckCast(bool strict)
                 return SPELL_FAILED_NOT_READY;
         }

-        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
-        {
-            if (m_triggeredByAuraSpell)
-                return SPELL_FAILED_DONT_REPORT;
-            else
-                return SPELL_FAILED_NOT_READY;
-        }
+       if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && !(_triggeredCastFlags & TRIGGERED_IGNORE_SPELL_AND_CATEGORY_CD))
+       {
+           if (m_triggeredByAuraSpell)
+               return SPELL_FAILED_DONT_REPORT;
+           else
+               return SPELL_FAILED_NOT_READY;
+       }
Contributor

Eliminationzx commented Jan 18, 2016

Try it

 src/server/game/Spells/Spell.cpp | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 84bff4a..ca9f642 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -4701,13 +4701,13 @@ SpellCastResult Spell::CheckCast(bool strict)
                 return SPELL_FAILED_NOT_READY;
         }

-        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
-        {
-            if (m_triggeredByAuraSpell)
-                return SPELL_FAILED_DONT_REPORT;
-            else
-                return SPELL_FAILED_NOT_READY;
-        }
+       if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && !(_triggeredCastFlags & TRIGGERED_IGNORE_SPELL_AND_CATEGORY_CD))
+       {
+           if (m_triggeredByAuraSpell)
+               return SPELL_FAILED_DONT_REPORT;
+           else
+               return SPELL_FAILED_NOT_READY;
+       }
@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Jan 18, 2016

That's a strange (and incorrect?) diff, should not need to replace more than the first line (the if test).
I think it happened because of some unneeded change in indentation for the subsequent lines.
I believe it should look like this:

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 84bff4a..ca9f642 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -4701,7 +4701,7 @@ SpellCastResult Spell::CheckCast(bool strict)
                 return SPELL_FAILED_NOT_READY;
         }

-        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
+        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && !(_triggeredCastFlags & TRIGGERED_IGNORE_SPELL_AND_CATEGORY_CD))
         {
             if (m_triggeredByAuraSpell)
                 return SPELL_FAILED_DONT_REPORT;

ghost commented Jan 18, 2016

That's a strange (and incorrect?) diff, should not need to replace more than the first line (the if test).
I think it happened because of some unneeded change in indentation for the subsequent lines.
I believe it should look like this:

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 84bff4a..ca9f642 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -4701,7 +4701,7 @@ SpellCastResult Spell::CheckCast(bool strict)
                 return SPELL_FAILED_NOT_READY;
         }

-        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
+        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && !(_triggeredCastFlags & TRIGGERED_IGNORE_SPELL_AND_CATEGORY_CD))
         {
             if (m_triggeredByAuraSpell)
                 return SPELL_FAILED_DONT_REPORT;
@Eliminationzx

This comment has been minimized.

Show comment
Hide comment
@Eliminationzx

Eliminationzx Jan 18, 2016

Contributor

Thank you for your corrections @tkrokli

Contributor

Eliminationzx commented Jan 18, 2016

Thank you for your corrections @tkrokli

@Re3os

This comment has been minimized.

Show comment
Hide comment
@Re3os

Re3os Jan 18, 2016

@tkrokli work fine fo me.

a89a3cf6225d1381142d4679ed7ba3ad3301d1f4
 src/server/game/Spells/Spell.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 99e2d3a..0966900 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -4856,7 +4856,7 @@ SpellCastResult Spell::CheckCast(bool strict)
                 return SPELL_FAILED_NOT_READY;
         }

-        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
+        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && m_spellInfo->Id != 60202)
         {
             if (m_triggeredByAuraSpell)
                 return SPELL_FAILED_DONT_REPORT;

Re3os commented Jan 18, 2016

@tkrokli work fine fo me.

a89a3cf6225d1381142d4679ed7ba3ad3301d1f4
 src/server/game/Spells/Spell.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/server/game/Spells/Spell.cpp b/src/server/game/Spells/Spell.cpp
index 99e2d3a..0966900 100644
--- a/src/server/game/Spells/Spell.cpp
+++ b/src/server/game/Spells/Spell.cpp
@@ -4856,7 +4856,7 @@ SpellCastResult Spell::CheckCast(bool strict)
                 return SPELL_FAILED_NOT_READY;
         }

-        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
+        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && m_spellInfo->Id != 60202)
         {
             if (m_triggeredByAuraSpell)
                 return SPELL_FAILED_DONT_REPORT;
@ccrs

This comment has been minimized.

Show comment
Hide comment
@ccrs

ccrs Jan 18, 2016

Contributor

that's a hack, already told u before #15794 (comment)

Contributor

ccrs commented Jan 18, 2016

that's a hack, already told u before #15794 (comment)

@Demonid

This comment has been minimized.

Show comment
Hide comment
@Demonid

Demonid Jan 18, 2016

@tkrokli Which one ?

This one? #15794 (comment)

Demonid commented Jan 18, 2016

@tkrokli Which one ?

This one? #15794 (comment)

@Eliminationzx

This comment has been minimized.

Show comment
Hide comment
@Eliminationzx

Eliminationzx Jan 18, 2016

Contributor

@tkrokli I delete that fix because it was wrong and I just didn't see when Aokromes labeled it.
Sorry for that.

Contributor

Eliminationzx commented Jan 18, 2016

@tkrokli I delete that fix because it was wrong and I just didn't see when Aokromes labeled it.
Sorry for that.

@Shauren

This comment has been minimized.

Show comment
Hide comment
@Shauren

Shauren Jan 18, 2016

Member

LOL.
Checking the cooldown only if the spell can be silenced? hahahhaha good one

Member

Shauren commented Jan 18, 2016

LOL.
Checking the cooldown only if the spell can be silenced? hahahhaha good one

@Eliminationzx

This comment has been minimized.

Show comment
Hide comment
@Eliminationzx

Eliminationzx Jan 18, 2016

Contributor

@tkrokli, it's couldn't be valid I guess, because we should use this check "HasCooldown" for all prevention types not only for silence.

Contributor

Eliminationzx commented Jan 18, 2016

@tkrokli, it's couldn't be valid I guess, because we should use this check "HasCooldown" for all prevention types not only for silence.

@Demonid

This comment has been minimized.

Show comment
Hide comment
@Demonid

Demonid Jan 18, 2016

Freezing Trap and Overheat seems to work well using #15794 (comment)

Demonid commented Jan 18, 2016

Freezing Trap and Overheat seems to work well using #15794 (comment)

@ccrs

This comment has been minimized.

Show comment
Hide comment
@ccrs

ccrs Jan 18, 2016

Contributor

#15794 (comment) is the same as #15794 (comment) and there are some related issues as deathbringer's will triggering auras simultaneously.

Contributor

ccrs commented Jan 18, 2016

#15794 (comment) is the same as #15794 (comment) and there are some related issues as deathbringer's will triggering auras simultaneously.

@Demonid

This comment has been minimized.

Show comment
Hide comment
@Demonid

Demonid Jan 18, 2016

@ccrs Yep, you are right, too bad...

Demonid commented Jan 18, 2016

@ccrs Yep, you are right, too bad...

@Shauren

This comment has been minimized.

Show comment
Hide comment
@Shauren

Shauren Jan 18, 2016

Member

@Demonid @ccrs these patches break cooldowns on all procs
Probably we should skip checking the cooldown only if it was added by the spell that triggered it

Spell X triggers Y -> X has category cd 10s, when checking cooldown on Y skip only the one added by X but not other cds (if applicable)

Member

Shauren commented Jan 18, 2016

@Demonid @ccrs these patches break cooldowns on all procs
Probably we should skip checking the cooldown only if it was added by the spell that triggered it

Spell X triggers Y -> X has category cd 10s, when checking cooldown on Y skip only the one added by X but not other cds (if applicable)

@ccrs

This comment has been minimized.

Show comment
Hide comment
@ccrs

ccrs Jan 18, 2016

Contributor

(dead link)

doesn't work for gunship overheat

Category Name SpellId trigger
1152 Cannon Blast 70172 70173
0 Cannon Blast 70173 nothing
1152 Cannon Blast 69399 69400
0 Cannon Blast 69400 nothing
1152 Overheat 69487 69488
0 Overheat 69488 nothing

69487 is casted inside 70172/69399 spellscript, so there is no way to search for that in the spellInfo

customSpell value to avoid categoryCooldown?

Contributor

ccrs commented Jan 18, 2016

(dead link)

doesn't work for gunship overheat

Category Name SpellId trigger
1152 Cannon Blast 70172 70173
0 Cannon Blast 70173 nothing
1152 Cannon Blast 69399 69400
0 Cannon Blast 69400 nothing
1152 Overheat 69487 69488
0 Overheat 69488 nothing

69487 is casted inside 70172/69399 spellscript, so there is no way to search for that in the spellInfo

customSpell value to avoid categoryCooldown?

@Shauren

This comment has been minimized.

Show comment
Hide comment
@Shauren

Shauren Jan 18, 2016

Member

I meant something more like the triggeredByAura arg to CastSpell, what you made won't even work with SpellDifficulty

Member

Shauren commented Jan 18, 2016

I meant something more like the triggeredByAura arg to CastSpell, what you made won't even work with SpellDifficulty

@ccrs

This comment has been minimized.

Show comment
Hide comment
@ccrs

ccrs Jan 18, 2016

Contributor
        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
        {
            if (m_triggeredByAuraSpell)
            {
                if (m_triggeredByAuraSpell->GetCategory() != m_spellInfo->GetCategory())
                    return SPELL_FAILED_DONT_REPORT;
            }
            else
                return SPELL_FAILED_NOT_READY;
        }
Contributor

ccrs commented Jan 18, 2016

        if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))
        {
            if (m_triggeredByAuraSpell)
            {
                if (m_triggeredByAuraSpell->GetCategory() != m_spellInfo->GetCategory())
                    return SPELL_FAILED_DONT_REPORT;
            }
            else
                return SPELL_FAILED_NOT_READY;
        }
@ariel-

This comment has been minimized.

Show comment
Hide comment
@ariel-

ariel- Jan 19, 2016

Member

Doesn't mean the patch is bad either, maybe its the proc system that needs a repair after this.
(well maybe look at the patch a little, actually if the spell is triggered without cooldown checking it'll also bypass school locking as it is)
This seems correct as Spell::IsIgnoringCooldowns is checked on cooldown add, but not on cooldown check.

If you put the cooldown in the passive (triggering aura) and check that for avoiding repeated procs?

Member

ariel- commented Jan 19, 2016

Doesn't mean the patch is bad either, maybe its the proc system that needs a repair after this.
(well maybe look at the patch a little, actually if the spell is triggered without cooldown checking it'll also bypass school locking as it is)
This seems correct as Spell::IsIgnoringCooldowns is checked on cooldown add, but not on cooldown check.

If you put the cooldown in the passive (triggering aura) and check that for avoiding repeated procs?

@ccrs

This comment has been minimized.

Show comment
Hide comment
@ccrs

ccrs Jan 19, 2016

Contributor

#15794 (comment) that comment was referring to a previous workaround :)

Contributor

ccrs commented Jan 19, 2016

#15794 (comment) that comment was referring to a previous workaround :)

@Shauren Shauren closed this in 044edce Jan 22, 2016

Shauren added a commit that referenced this issue Jan 22, 2016

Core/Spells: Ignore category cooldowns for triggered spells.
* This fixes weird issues where triggered spell fails because it has the same category as the spell triggering it

Closes #15794
Closes #15048

(cherry picked from commit 044edce)
@mayaren888

This comment has been minimized.

Show comment
Hide comment
@mayaren888

mayaren888 Mar 15, 2016

@tkrokli
`- if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))

  •    if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && !(_triggeredCastFlags & TRIGGERED_IGNORE_SPELL_AND_CATEGORY_CD))`
    

This repair method will cause the skill (spell:71519) trigger is not correct

mayaren888 commented Mar 15, 2016

@tkrokli
`- if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry))

  •    if (!m_caster->GetSpellHistory()->IsReady(m_spellInfo, m_castItemEntry) && !(_triggeredCastFlags & TRIGGERED_IGNORE_SPELL_AND_CATEGORY_CD))`
    

This repair method will cause the skill (spell:71519) trigger is not correct

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Mar 15, 2016

I should probably delete my comments with code. None of it works. I just tried to show some editing.

ghost commented Mar 15, 2016

I should probably delete my comments with code. None of it works. I just tried to show some editing.

@mayaren888

This comment has been minimized.

Show comment
Hide comment
@mayaren888

mayaren888 Mar 16, 2016

@tkrokli
I'm really sorry, didn't mean that your code is given a lot of people inspired.
You are a good contributor.

mayaren888 commented Mar 16, 2016

@tkrokli
I'm really sorry, didn't mean that your code is given a lot of people inspired.
You are a good contributor.

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