nos4r2zod

That's exactly what mangos does currently.

What bug does the patch fix? What features does the patch add? This patch fixes a potential exploit with multicast bars (currently only totem bars use this). Spell:EffectSummonAllTotems checks through the player's action buttons in order to find which totems to cast. This makes it a rare case where a mangos spell implementation is dependent on information on the client's UI. This can currently be exploited by third party programs which can place spells on the totem bar that are not normally allowed by client. Doing so allows those spells (even passive abilities/talents) to be cast on self with Call of the Elements/Ancestors/Spirits instantly. The patch prevents passive spells from being put on action bars in general, and checks that only totem spells are placed the totem bar using AttributesEx7 data. For which repository revision was the patch created? 10755 Is there a thread in the bug report section or at lighthouse? If yes, please add a link to the thread. Nope. Who has been writing this patch? Please include either forum user names or email addresses. Myself Patch: http://paste2.org/p/1099765 Edit: Typo in defines fixed Edit2: Update after 10693

GCD must be limited to between 1 second and 1.5 seconds, no matter what haste ratings/aura effects are applied. This can be tested from client-side only, because client calculated GCD independently from server based only on haste rating and auras. Based on my understanding, GCD should be limited to between 1000ms and 1500ms: With >>100% haste rating, I saw GCD only decrease to 1.0s, so for this part, patch as given is incorrect. Cap at bottom of 1.0s does not happen just for haste rating either: see comments at http://www.wowhead.com/spell=18288. Haste debuffs (like http://www.wowhead.com/spell=72297) increase GCD, but not beyond 1.5s. When you receive this buff with 0 other haste mods, GCD remains 1.5s. When you decrease GCD to 1.0s via only haste rating at haste rating cap, then apply this debuff, GCD does increase again to 1.5s.

The problem exists not in the database table, but rather in how the way Mangos currently handles calculations involving HP/power, uint32 overflow is not protected against. Here is the code that is causing Srage's bug: case EVENT_T_HP: { if (!m_creature->isInCombat() || !m_creature->GetMaxHealth()) return false; uint32 perc = (m_creature->GetHealth()*100) / m_creature->GetMaxHealth(); if (perc > event.percent_range.percentMax || perc < event.percent_range.percentMin) return false; .... For 25-man normal Halion's HP (43.5 million), GetHealth()*100 returns a value greater than 2^32, causing perc to be calculated as something much lower than expected. (Another problem exists in that perc truncates this calculation, so that the range check passes when the percent is 75.99, rather than at 75.00 and below. For this reason I feel a ceil() is necessary here.) From what I see, preferred style in code is to do Get[Health|Power]() * int / GetMax[Health|Power]() to avoid floating point arithmetic / casts from int to float and back, but I think this may be bad to do now, since correctness trumps efficiency. For instance in health recalculation in Aura::HandleModTotalPercentStat, after receiving stamina % buff //recalculate current HP/MP after applying aura modifications (only for spells with 0x10 flag) if ((m_modifier.m_miscvalue == STAT_STAMINA) && (maxHPValue > 0) && (GetSpellProto()->Attributes & 0x10)) if ((m_modifier.m_miscvalue == STAT_STAMINA) && (GetSpellProto()->Attributes & 0x10)) { // newHP = (curHP / maxHP) * newMaxHP = (newMaxHP * curHP) / maxHP -> which is better because no int -> double -> int conversion is needed uint32 newHPValue = (target->GetMaxHealth() * curHPValue) / maxHPValue; ... Comment may be true, but when target player has current hp >= 2^16 (65536 hp, which is reasonably possible), GetMaxHealth() * curHPValue returns ~0! Other problem exists in lack of standard way for checking HP/power percent in spell code: // Prey on the Weak if (spell->SpellIconID == 2983) { Unit *victim = target->getVictim(); if (victim && (target->GetHealth() * 100 / target->GetMaxHealth() > victim->GetHealth() * 100 / victim->GetMaxHealth())) .... // Decimation else if (auraSpellInfo->Id == 63156 || auraSpellInfo->Id == 63158) { // Looking for dummy effect Aura *aur = GetAura(auraSpellInfo->Id, EFFECT_INDEX_1); if (!aur) return SPELL_AURA_PROC_FAILED; // If target's health is not below equal certain value (35%) not proc if (int32(pVictim->GetHealth() * 100 / pVictim->GetMaxHealth()) > aur->GetModifier()->m_amount) return SPELL_AURA_PROC_FAILED; Use of implicit uint32(GetHealth() * 100) allows rogues with Prey on the Weak to always gain 20% crit versus big raid bosses at high health or warlocks with Decimation to always gain that proc. This is my attempted fix (rev. 10448). (Personally, I think all instances of these calculations should be the same, despite the increased use of floating point and the cost of efficiency. But it is reasonable to say, for example, that only AI checks at %HP/power, health recalculation on gaining %stamina, and "pseudo" health aurastate checks for spells that could be used on bosses like Prey on the Weak or Decimation strictly need this change, since player vitals would never get close to the values that would cause overflow.) Edit: Fixed typo as pointed out by Patman128 in post below, thanks!

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.

Actually, it was added in 2.2: But it just didn't do nearly as much damage; only around single digit damage. You can see that 3.2 only bumped up the damage done by the triggered effect and nothing else.

Updated past 8428, which implemented the attack power bonus of Savage Defense; parts of this patch are still usable.

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

This is the way it worked prior to patch to 3.0.8. And in 3.0.8 patch notes, no mention that the weapon had to be specifically an off-hand weapon only item.

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);

Actually, I meant to say that it's not up to date and I haven't tested it since 8193, but I think the bugs covered still persist. Edit: Updated for 8379. Maybe there's a better way to do + // Procs can miss, weapon enchants can miss, triggered spells and effects cannot miss (miss already calculated in triggering spell) + bool canMiss = (m_triggeredByAuraSpell || !m_IsTriggeredSpell); though, it's too strict in that it requires scripted spells triggered from auras to NOT have a m_triggeredByAuraSpell for this spell to work (like Judgements as triggered from Seals, or Chimera Shot as triggered from stings). Also, removed the cooldown hack in this patch, wasn't really related, and perhaps a better way to have done would have been in the AddSpellAndCategoryCooldowns, like this.

I guess I forgot to write a comment for Improved Concentration Aura in IsNoStackDueToSpell when I added it, so I also forgot the fact that it also needed an exception in INSDTS when the patch was changed to use SpellIcons. As I said in the head of this topic, Aspect of the Dragonhawk won't work correctly without there being a way to not have a cooldown on the triggered spell. Relogging will give you the dodge bonus because when the triggered spell is cast on login, the player won't have a cooldown on the Aspects spell category... The best way I could think of is to not add a cooldown on category spells that the player doesn't really have (except when the cast is based on an item) (This also works with freezing arrow) http://paste2.org/p/375371 EDIT: Though the addition in 8358 prevents Concentration Aura from being deleted when the triggered spell is cast now, I think it's probably better to make a INSDTS exception consistent with the other auras than to use SetInUse, as long as Improved Concentration Aura was the only thing that was breaking.

It's because totem buffs use SPELL_ATTR_PASSIVE, so they don't call IsNoStackSpellDueToSpell at all. My apologies for not checking this more carefully. I would add a more generic check in IsRankSpellDueToSpell in that patch, but that'd require a bit more restructuring, because IsHighRankOfSpell assumes that both spells have spell_chain data after IsRankSpellDueToSpell returns true. So the only alternative for now is just to add the spell_chain data, I think. I'm not certain about this. Most debuffs are checked in the ImmuneMechanicMask, but I see CoT doesn't have a mechanic. On a whim, I think it might have something to do with SPELL_ATTR_EX4_UNK29. It looks like that is shared with many debuffs, with or without mechanics, that should be "partly" effective against bosses. In addition to CoT, Frostbrand Attack has it (slow not always effective, but Aura applied anyway for Frozen Power talent) and Disarm has it (disarm not always effective, but damage increase (for when the warrior has Improved Disarm Talent) should be applied). Edit: nvm, don't think it'll work; rank 9 Frostbrand Attack doesn't have it (unless it's related to the SP bug that Frostbrand had in 3.1.x?) If you want to know what creatures are considered raid bosses, it's in the Rank column of creature_template (I think it's set to 4).

A coefficient of zero is the correct way to show no bonus from coefficients, at least for now. Having a negative coefficient will result in the spell's damage decreasing as a result of spell power, or having the spell treated as a miss, or in some cases, having the spell's damage wrap around and causing damage orders of magnitude too high.

It doesn't increase spell power, it just increases the damage you do. You won't see it on your character tab. http://www.wowhead.com/?spell=48412#comments:id=662334

OT: It won't work with judgements, no aura involved; but I supposed you could change the script effect to treat the seal as the proccing aura. I agree.

That would work, but it would be hacky and might be unnecessary when you consider that Earth Shield, Prayer of Mending, Paladin Judgements, etc. would also need the same fix. Edit: I mean this patch is fine as is once you add the coefficient, and the downranking problem isn't an issue completely related to this patch. But back when the procs actually did the damage, there wasn't this problem with spellLevels/downranking (currently, they seem to just supply client display data with their unused dummy effect). Edit2: No, I meant I personally liked it as is, without the hardcoded coefficient

Mangos coding style uses 4 spaces instead of tabs, because tabs vary in length across Git/IDEs/paste2/etc. Your patch works alright, but SQL changes are necessary as well: DELETE FROM `spell_bonus_data` WHERE `entry` IN (8026, 10444); INSERT INTO `spell_bonus_data` VALUES (10444, 0.1, 0, 0, "Shaman - Flametongue Weapon Proc"); Now this brings to light some more generic fixes that need to be made for Flametongue Weapon before it deals the correct amount of damage. The spell that is procced has a spellLevel of 1, so the coefficient is subject to downranking, and for a level 80, it'd only get ~0.007 * SP instead of 0.1 * SP.

So that false is returned when a presence is compared with anything at all. It's not explicitly comparing a presence with it's triggered spell, it's comparing a presence with any spell. Edit: I think I misunderstood, but last I checked, this function was being called and the spells not stacking without this check. Edit2: Ok sorry, you are right, this part is not needed Also, in _RemoveAura(), I think the if(IsSpellLastAuraEffect(GetSpellProto(),GetEffIndex())) is not necessary, because it is within the "if (lastaura)" code block. Edit: Also, I notice that in some cases, auras are not being removed properly. Edit2: Specifically, Frost Presence's 10% hp bonus isn't being removed; not sure if there are others.

Only Deep Freeze had this. Shatter and Ice Lance had no aurastate, so I'm not quite sure I follow what you're saying.

It was just a hack fix to make a point... Well as Lynx3d pointed out, first we have to add the server-side check to prevent the casting of non-combat spells in combat I am also not sure if fingers of frost should have an aurastate. That seems to be the case.

That's what Lightguard did. The skills are grayed out and disabled on the client side. You can't even begin to cast them unless the client knows that Juggernaut/Warbringer's auras are in effect. Edit: See this hack-fix to see what I mean ->Hack<- (not thoroughly tested). No, you don't really need to assign Juggernaut and Warbringer different slots since you can't learn them both at the same time, but I did so for testing purposes. Also, _RemoveAura needs to be edited so that auras above MAX_AURAS can be removed. This is only assuming that Warbringer/Juggernaut have special aura slots. If not, ignore this.

Actually, it's not a sniff, it was trial + error, so it probably will not be 100% correct. I'm only working with the client-side problem of implementing this aura in Juggernaut and Warbringer. With Juggernaut and Warbringer, the spells affected remain greyed out in combat / with the wrong stances. The other spells work fine on the client side since they are visible auras (Taste for Blood and Sudden Death). Unfortunately, I think the way to implement Fingers of Frost will still be through a hack in Shatter/Ice Lance's custom script code. Edit: @Pelle: patience, please... Sudden Death works with Lightguard's patch, I already stated Juggernaut is the same issue as Warbringer, Shadow Dance doesn't even use 262.

I have wracked my head with this one for a very long time and I finally realize why Warbringer doesn't allow the client to use Charge in combat. It's because, as a passive, invisible aura, it is never sent to the client. So the client never does anything to enable spells affected by Warbringer, Juggernaut, etc. outside their bypassed restraints. I think one logical way to solve this problem would be to, in the handler of 262, custom send this packet to the client. You can confirm this works by .debug send opcode with the following parameters in opcode.txt: 1174 uint 16 xxxxx uint8 255 uint32 57499 uint8 19 uint8 80 uint8 1 uint8 0 SMSG_AURA_UPDATE whatever GetPackGUID() is max allowed auraslot Warbringer ID see AFLAG enum AuraLevel Charges What this opcode does is send the client the invisible Warbringer aura (you can also try this with Juggernaut), and allows the client to use charge in combat / use intercept and intervene in combat. Edit: Also, in the patch you wrote, you could have just used (*i)->IsAffectedOn(m_spellInfo)

Wow, good observation! You can look in the \\contrib\\dbcEditer\\bin\\ folder for something to view Spell.dbc with. Just look for the DmgClass column.