Jump to content

nos4r2zod

Members
  • Posts

    84
  • Joined

  • Last visited

    Never
  • Donations

    0.00 GBP 

nos4r2zod's Achievements

Advanced Member

Advanced Member (3/3)

0

Reputation

  1. 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
  2. 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.
  3. 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!
  4. 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.
  5. 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.
  6. Updated past 8428, which implemented the attack power bonus of Savage Defense; parts of this patch are still usable.
  7. This is what remains of this patch after commits 8399-8401: PATCH
  8. 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.
  9. 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);
  10. 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.
  11. 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.
  12. 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).
  13. 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.
  14. 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
×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue. Privacy Policy Terms of Use