Jump to content

value overflow in acid


Guest Srage87

Recommended Posts

Hi!

I encountered a problem in Acid, such things like event_t_mana or even_t_hp has a limitation, 32million as far as i could see, so if i want a creature with more hp than 32mill, for example 67mill and set an even at 90% of its hp, it wont be processed well, because it's out of the value type what the system is using.

for example i made a good script for halion in ruby sanctum with acid, but in modes where he has more hp than 32mill phases are messed up, so for example in heroic 25 he skips first 2 phases and starts in the 3rd phase(light & dark together).

if i modify his hp down to 32mill, it works perfect.

in case im posting this in wrong section sorry, i hope this report can help.

Link to comment
Share on other sites

well, im using this script at halion:

-- Halion
DELETE FROM creature_ai_scripts_custom WHERE creature_id IN(39863);
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 11, 0, 100, 30, 0, 0, 0, 0, 35, 1, 0, 0, 34, 1, 0, 0, 22, 0, 0, 0, 'Halion - On Spawn');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 4, 0, 100, 30, 0, 0, 0, 0, 38, 0, 0, 0, 1, -131186, 0, 0, 34, 1, 1, 0, 'Halion - On Aggro');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 4, 0, 100, 30, 0, 0, 0, 0, 22, 0, 0, 0, 11, 78243, 0, 23, 39, 200, 0, 0, 'Halion - On Aggro');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 6, 0, 100, 30, 0, 0, 0, 0, 34, 1, 3, 0, 1, -131187, 0, 0, 0, 0, 0, 0, 'Halion - On Death');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 5, 0, 100, 31, 0, 0, 0, 0, 1, -131188, -131189, 0, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - On Kill');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 10, 0, 100, 31, 0, 100, 60000, 90000, 1, -131191, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - On LOS');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 7, 0, 100, 30, 0, 0, 0, 0, 34, 1, 2, 0, 0, 0, 0, 0, 22, 0, 0, 0, 'Halion - On Evade');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 0, 100, 31, 480000, 480000, 480000, 480000, 11, 47008, 0, 23, 1, -131190, 0, 0, 0, 0, 0, 0, 'Halion - Enrage 8 Minutes');

-- Cleave & Tail Lash
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 0, 100, 31, 22000, 36000, 26000, 32000, 11, 74524, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Cleave');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 0, 100, 31, 9000, 16000, 9000, 16000, 11, 74531, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Tail Lash');

-- Phase 1
-- Flame Breath
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 3, 9000, 16000, 10000, 11000, 11, 74525, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Flame Breath N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 9, 9000, 16000, 10000, 11000, 11, 74525, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Flame Breath H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 5, 9000, 16000, 10000, 11000, 11, 74527, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Flame Breath N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 17, 9000, 16000, 10000, 11000, 11, 74527, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Flame Breath H25');
-- Meteor Strike
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 3, 9000, 16000, 16000, 22000, 12, 239863, 5, 120000, 1, -131192, 0, 0, 0, 0, 0, 0, 'Halion - Meteor Strike N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 9, 9000, 16000, 12000, 18000, 12, 239863, 5, 120000, 1, -131192, 0, 0, 0, 0, 0, 0, 'Halion - Meteor Strike H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 5, 9000, 16000, 16000, 22000, 12, 239863, 5, 120000, 1, -131192, 0, 0, 0, 0, 0, 0, 'Halion - Meteor Strike N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 17, 9000, 16000, 12000, 18000, 12, 239863, 5, 120000, 1, -131192, 0, 0, 0, 0, 0, 0, 'Halion - Meteor Strike H25');
-- Fiery Combustion
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 3, 9000, 16000, 6000, 9000, 11, 74562, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Fiery Combustion N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 9, 9000, 16000, 5000, 6000, 11, 74562, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Fiery Combustion H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 5, 9000, 16000, 6000, 9000, 11, 74562, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Fiery Combustion N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 62, 100, 17, 9000, 16000, 5000, 6000, 11, 74562, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Fiery Combustion H25');
-- Switch to phase 2 at 75% hp
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 2, 62, 100, 31, 75, 1, 1000, 1000, 22, 1, 0, 0, 1, -131194, 0, 0, 0, 0, 0, 0, 'Halion - Switch to phase2 at 75p');

-- Phase 2
-- Dark Breath
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 3, 9000, 16000, 10000, 11000, 11, 74806, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Dark Breath N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 9, 9000, 16000, 10000, 11000, 11, 74806, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Dark Breath H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 5, 9000, 16000, 10000, 11000, 11, 75954, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Dark Breath N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 17, 9000, 16000, 10000, 11000, 11, 75954, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Dark Breath H25');
-- Dusk Shroud
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 3, 9000, 16000, 16000, 22000, 11, 75484, 1, 4, 1, -131193, 0, 0, 0, 0, 0, 0, 'Halion - Dusk Shroud N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 9, 9000, 16000, 12000, 18000, 11, 75484, 1, 4, 1, -131193, 0, 0, 0, 0, 0, 0, 'Halion - Dusk Shroud H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 5, 9000, 16000, 16000, 22000, 11, 75485, 1, 4, 1, -131193, 0, 0, 0, 0, 0, 0, 'Halion - Dusk Shroud N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 17, 9000, 16000, 12000, 18000, 11, 75485, 1, 4, 1, -131193, 0, 0, 0, 0, 0, 0, 'Halion - Dusk Shroud H25');
--  Soul Consumption
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 3, 9000, 16000, 6000, 9000, 11, 74792, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Soul Consumption N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 9, 9000, 16000, 5000, 6000, 11, 74792, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Soul Consumption H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 5, 9000, 16000, 6000, 9000, 11, 74792, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Soul Consumption N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 61, 100, 17, 9000, 16000, 5000, 6000, 11, 74792, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Soul Consumption H25');
-- Switch to phase 3 at 50% hp
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 2, 61, 100, 31, 50, 1, 1000, 1000, 22, 2, 0, 0, 1, -131195, 0, 0, 0, 0, 0, 0, 'Halion - Switch to phase3 at 50p');

-- Phase 3
-- Flame Breath
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 3, 9000, 21000, 20000, 21000, 11, 74525, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Flame Breath N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 9, 9000, 21000, 20000, 21000, 11, 74525, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Flame Breath H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 5, 9000, 21000, 20000, 21000, 11, 74527, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Flame Breath N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 17, 9000, 21000, 20000, 21000, 11, 74527, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Flame Breath H25');
-- Meteor Strike
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 3, 9000, 16000, 18000, 28000, 12, 239863, 5, 120000, 1, -131192, 0, 0, 0, 0, 0, 0, 'Halion - Meteor Strike N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 9, 9000, 16000, 16000, 22000, 12, 239863, 5, 120000, 1, -131192, 0, 0, 0, 0, 0, 0, 'Halion - Meteor Strike H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 5, 9000, 16000, 18000, 28000, 12, 239863, 5, 120000, 1, -131192, 0, 0, 0, 0, 0, 0, 'Halion - Meteor Strike N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 17, 9000, 16000, 16000, 22000, 12, 239863, 5, 120000, 1, -131192, 0, 0, 0, 0, 0, 0, 'Halion - Meteor Strike H25');
-- Fiery Combustion
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 3, 9000, 21000, 9000, 12000, 11, 74562, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Fiery Combustion N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 9, 9000, 21000, 6000, 9000, 11, 74562, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Fiery Combustion H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 5, 9000, 21000, 9000, 12000, 11, 74562, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Fiery Combustion N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 17, 9000, 21000, 6000, 9000, 11, 74562, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Fiery Combustion H25');
-- Dark Breath
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 3, 9000, 21000, 20000, 21000, 11, 74806, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Dark Breath N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 9, 9000, 21000, 20000, 21000, 11, 74806, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Dark Breath H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 5, 9000, 21000, 20000, 21000, 11, 75954, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Dark Breath N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 17, 9000, 21000, 20000, 21000, 11, 75954, 1, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Dark Breath H25');
-- Dusk Shroud
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 3, 9000, 21000, 22000, 26000, 11, 75484, 1, 4, 1, -131193, 0, 0, 0, 0, 0, 0, 'Halion - Dusk Shroud N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 9, 9000, 21000, 18000, 12000, 11, 75484, 1, 4, 1, -131193, 0, 0, 0, 0, 0, 0, 'Halion - Dusk Shroud H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 5, 9000, 21000, 22000, 26000, 11, 75485, 1, 4, 1, -131193, 0, 0, 0, 0, 0, 0, 'Halion - Dusk Shroud N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 17, 9000, 21000, 18000, 12000, 11, 75485, 1, 4, 1, -131193, 0, 0, 0, 0, 0, 0, 'Halion - Dusk Shroud H25');
-- Soul Consumption
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 3, 9000, 21000, 9000, 12000, 11, 74792, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Soul Consumption N10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 9, 9000, 21000, 6000, 9000, 11, 74792, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Soul Consumption H10');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 5, 9000, 21000, 9000, 12000, 11, 74792, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Soul Consumption N25');
REPLACE INTO creature_ai_scripts_custom VALUES
(NULL, 39863, 0, 59, 100, 17, 9000, 21000, 6000, 9000, 11, 74792, 4, 4, 0, 0, 0, 0, 0, 0, 0, 0, 'Halion - Soul Consumption H2');

if halion has less than 32mill hp, it works perfectly, if he has more, he is skipping those phases until the x% percentage value gets under 32mill...

so if i set his hp to 64 mill, then any event_t_hp will immediatly passed which points some hp value over 32mill... so a 40% would work, but 60% would be immediatly passed...

Link to comment
Share on other sites

`event_param1` int(11) signed NOT NULL default '0',

so normal int ~ +-2000million

But even_t_hp store percents, not raw hp values

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!

Link to comment
Share on other sites

Forgive me if I'm wrong, but shouldn't

uint32 GetPercentOfMaxPower(Powers power, float pct) const { return uint32(GetUInt32Value(UNIT_FIELD_MAXPOWER1+power) * pct); }

be

uint32 GetPercentOfMaxPower(Powers power, float pct) const { return uint32(GetUInt32Value(UNIT_FIELD_MAXPOWER1+power) * (pct / 100)); }

if pct is a number from 0.0 to 100.0?

Link to comment
Share on other sites

×
×
  • 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