[Patch] Big Cleanup in SpellMgr::IsNoStackSpellDueToSpell

[Auntie Mangos]

What bug does the patch fix? What features does the patch add?

This patch rewrites parts of SpellMgr::IsNoStackSpellDueToSpell so that much fewer exceptions are needed.

For which repository revision was the patch created?

8379

Is there a thread in the bug report section or at lighthouse? If yes, please add a link to the thread.

http://getmangos.eu/community/viewtopic.php?id=2305

Who has been writing this patch? Please include either forum user names or email addresses.

Myself

The real changes in this patch are the things changed under the comment "// more generic checks", at the bottom of the patch.

• * Checks for SPELLFAMILY_GENERIC in the switch make no sense, these lines should take care of these already, but I believe they were misplaced or superceded by the isModifier check at some point:
  

  if (spellInfo_1->SpellFamilyName == 0 || spellInfo_2->SpellFamilyName == 0)
        return false;
  

  
  (I changed this line to use SPELLFAMILY_GENERIC instead of 0; if this part of the code were more readable in the first place, this mess wouldn't have happened ) Now, the only exceptions to be added that I can think of are scrolls (these exceptions are needed on master branch too, so it's unrelated to this patch).
  * IsRankSpellDueToSpell is already checked numerous times before this method is called, so I removed this check.
  * At the very end of these checks, return false if the SpellIcons are different. This allows many of the "exception" checks in the code above to be removed. (Note: the problem with Greater Blessings stacking occurs elsewhere; they are handled differently since they have SpellSpecifics)
  

This is what can be removed or changed based on these changes:

• * GENERIC
  * Simply removed all of these
  * MAGE
  * Detect Invisibility and Mana Shield - Would return false due to having different SpellFamilies
  * Combustion and Fire Resistance Aura - Would return false due to having different SpellFamilies
  * Arcane Intellect and Insight - Insight uses SPELLFAMILY_GENERIC
  * WARLOCK
  * Siphon Life and Drain Life - different SpellIcons
  * Corruption and Unstable Affliction - different SpellIcons
  * (Corruption or Unstable Affliction) and (Curse of Agony or Curse of Doom) - different SpellIcons
  * Detect Invisibility and Mana Shield - Would return false due to having different SpellFamilies
  * WARRIOR
  * Hamstring and Improved Hamstring - the triggered debuff from Improved Hamstring is SPELLFAMILY_GENERIC
  * Rend and Deep Wound - different SpellIcons
  * Battle Shout and Rampage - not sure why this was in here in the first place
  * Defensive Stance and Scroll of Protection - scrolls are SPELLFAMILY_GENERIC
  * Bloodlust and Bloodthirst - different SpellFamilies
  * PRIEST
  * Devouring Plague and Shadow Vulnerability - different SpellIcons, so the result is false. (There is no spell called "Shadow Vulnerability," but the family flag points to Shadow Weaving)
  * Starshards - spell removed in 3.0.2
  * DRUID
  * Omen of Clarity and Blood Frenzy - different SpellIcons
  * Wrath of Elune and Nature's Grace - different SpellIcons
  * Omen of Clarity and T4 Buffs - different SpellIcons
  * Leader of the Pack and Scroll of Stamina - Scrolls are SPELLFAMILY_GENERIC
  * Dragonmaw Illusion - one of the spells (40216) is SPELLFAMILY_GENERIC
  * ROGUE
  * Overkill - this exception deals with the two Overkill auras that both have SPELLFAMILY_ROGUE, so it can be moved up into the 'if' block. (not removed)
  * Garrote and Garrote-Silence - Garrote-Silence is SPELLFAMILY_GENERIC
  * HUNTER
  * Rapid Fire & Quick Shots - different SpellIcons
  * Serpent Sting & (Immolation/Explosive Trap Effect) - different SpellIcons
  * Wing Clip and Improved Wing Clip - Improved Wing Clip is SPELLFAMILY_GENERIC
  * Imp. Concussive Shot - spell 19410 no longer exists
  * PALADIN
  * Paladin Seals - handled by SpellSpecific checks; don't even think this method is called
  * Combustion and Fire Resistance Aura - Would return false due to having different SpellFamilies
  * Sanctity Aura and Unstable Currents - Unstable Currents is SPELLFAMILY_GENERIC
  * Band of Eternal Champion - uses SPELLFAMILY_GENERIC
  * SHAMAN
  * Shaman Shields - just want to note that the SPELL_ELEMENTAL_SHIELD SpellSpecific is never used (not removed)
  * Windfury Weapon and ??? - Not sure, but this looks like it used to handle the old Windfury totem and the passive for Windfury Weapon. So it's no longer needed. (The two spells affected by this check are 32911 and 33757)
  * Bloodlust and Bloodthirst - different SpellFamilies
  

PATCH HERE

It goes without saying, but please reply if you have any comments or if I managed to break something that was working before

EDIT: Removed SpellIcon check completely, very experimental

EDIT2: Put back SpellIcon checks for SpellFamily Generic spells until I figure out how to handle Food/Drinks and how to use SPELL_ATTR_EX2_UNK31 .

EDIT3: Food/Drinks/Well Fed buffs no longer stack with each other, updated to 8379

[VladimirMangos]

I think need some more wide checking before apply...

[nos4r2zod]

Updated to 8379, and added checks to prevent Food/Drinks/Well Fed buffs from stacking with each other.

Some explanation of these checks:

• * The Replenishment effects of food and drinks are handled in IsNoStackSpellDueToSpell. They are selected based on the AuraInterruptFlag AURA_INTERRUPT_FLAG_NOT_SEATED, which only food and drinks use. The spells are then compared to each other using IsSpellHaveAura, since there does not seem to be a better way to separate foods from drinks. I only found one food spell (48720) without this interrupt flag, but that can only be used underwater, where other foods shouldn't be usable anyway.
  * The Well Fed buffs of foods are handled using a spellspecific, since they were not all SPELLFAMILY_GENERIC. Some were also SPELLFAMILY_PRIEST (18125, 18141, 23697), and some were SPELLFAMILY_POTION (18191, 18192, 18193, 18194, 18222, 22730, 25661).
  * Of the SPELLFAMILY_GENERIC spells, only 46687 was not covered by SPELL_ATTR_EX2_FOOD_BUFF, so I added a SpellIcon check for that.
  

EDIT:

forget the spell_elixir flag for the Well Fed buffs that are SPELLFAMILY_POTION:

-- Well Fed (SPELLFAMILY_POTION)
DELETE FROM spell_elixir WHERE entry IN (18191, 18192, 18193, 18194, 18222, 22730, 25661);
INSERT INTO spell_elixir (entry, mask) VALUES
(18191, 16),
(18192, 16),
(18193, 16),
(18194, 16),
(18222, 16),
(22730, 16),
(25661, 16);

[JordanMckeag]

We need more patches like this anyone tested it

[bulek]

veryyyyy nice patch, working perfect:cool:

thank you

[JordanMckeag]

Tested

50+ online sorts it out can anyone else test it aswell

[VladimirMangos]

Just for note: this patch is 100% stable so main test point not in stability but in strange stacking problems not existed before this patch. Base at patch changes if problems exist it will be in unexpected stacking for spells that before propertly rejected.

[JordanMckeag]

I ain't found anything

uptime with patch 3 days 40+

Nothing so far will keep updateing

[VladimirMangos]

In last version for well fed priest case must be:

if ((spellInfo->Attributes & SPELL_ATTR_CASTABLE_WHILE_SITTING)==0 &&

Typo as i think.

[added]Just for note: original check has been right and this my missunderstanding and misscheck.

[VladimirMangos]

Well fed checking part added in [8399]

[nos4r2zod]

This is what remains of this patch after commits 8399-8401:

PATCH

[JordanMckeag]

As i can see the patch is ok its just mainly a clean up am not to sure about mangos tho on its structure but as i can see this works great for me

Realm ************** Online for 18 hours, 19 minutes, 53 seconds

[VladimirMangos]

Just for note: base at nos4r2zod and my tests patch have real problem with some auras stacking. So generic rules must be rewrited.

[Tassader2]

Just for note: base at nos4r2zod and my tests patch have real problem with some auras stacking. So generic rules must be rewrited.

Which auras?

[nos4r2zod]

I haven't had much time to work on my patches/keep them up-to-date, but iirc, I encountered some problems mainly with pet spells; they often share the same spell family mask, so using those as the main generic checks here wouldn't be ideal. (Check the ones with SpellFamilyFlags 0x10000000, SpellFamilyName = 9 for example)

I was thinking that SpellVisuals might be a better fall back method than SpellIcons, but I never finished checking. At the least, they might work for these pet spells, and they are the same for spells like Gift of the Wild and Mark of the Wild, Arcane Brilliance and Arcane Intellect, etc. where SpellIcons vary.

Anyway, I haven't looked in the latest client files, so there might be a better way to do these checks now.

[Wojta]

bump bump

[bulek]

kung fu bump this patch is correct

[Tassader2]

bump ;-)

what about keeping the icon check as last resort check if removing it causes problems?

[bulek]

bump :-) still not review it

[Abim]

The patch creater himself said, he encountered some problems... what do you expect ?

Research the topics zor mentioned, if you want to help get the patch into magnos.

[Tassader2]

It's a pitty that previous versions with not removed icon check are not aviable

[darkstalker]

It's a pitty that previous versions with not removed icon check are not aviable

the iconid check sucks, causes lots of problems

[The_Game_Master]

For further development of this patch, does anybody got a complete list of what pet spell should not stack? Or PM me how to get one.

[Hundekuchen]

I am also interested in a complete list of what pet spell should not stack

bump