[patch] spell threat multiplicator

[Auntie Mangos]

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

Implements database stored threat multipliers for spells

e.g.

Death and Decay should generate 1,9*damage as threat. So adding the spell entry 49938 and multiplier 1,9 to column `Threat_multiplier` in table `spell_threat` will provide the wanted effect.

http://www.wowwiki.com/Threat

With the release of v3.0, WoW now provides the players with threat values on your target. These values are normalized differently: they are 100x damage done instead of 1-to-1. This is most likely done to optimize for integer calculations, rather than slower floating-point math.

I also added this to threat calculation in ThreatManager::addThreat(...)

void ThreatManager::addThreat(Unit* pVictim, float pThreat, bool crit, SpellSchoolMask schoolMask, SpellEntry const *pThreatSpell)
{
[...]
   MANGOS_ASSERT(getOwner()->GetTypeId()== TYPEID_UNIT);

   float threat = ThreatCalcHelper::calcThreat(pVictim, iOwner, pThreat, crit, schoolMask, pThreatSpell);

   threat *= 100.0f; // change is here

   if (threat > 0.0f)
   {
[..]

Of course that doesn't move the whole threat calculation from float to integer, but it's a quick way to provide more "accurate" values in player's point of view. (Damage-Threat is 1-to-1, without modifiers. Just have a look at the Omen Threat Meter addon.)

But this is not part of this patch.

* For which repository revision was the patch created?

10944

* 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=15271

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

Me. Thanks to anybody who contributed to the bug discussion, linked above

Link to the patch file: http://paste2.org/p/1172466

Link to table structure changes: http://paste2.org/p/1172464

UPDATED: 1st Jan. 2011

[Auntie Mangos]

And i think we need a third column for attack power bonus, because non-damaging spells like sunder armor scale with AP.

How can be handled spell like 45477 Icy Touch (Rank 1) "Very high threat when in Frost Presence.", x7 damage according wowwiki?

I moved threat multiplication to ThreatCalcHelper::calcThreat(), seemed more logical to me unless we really need different cases for damage and healing.

I'm not sure but with you patch healing and damage do the same threat while, for what I know, healing usually should generate half amount

Since you are using both a mutiplier and flat values http://getmangos.eu/community/post/124975/#p124975 must be solved to find if 1 damage = 1 or 100 threat otherwise it's impossibile to fill the table properly

In part realated to this, time ago I was working on threat from float to uint32

it's based on an old revision and I had no time to test it but maybe it can be usefull, the main idea was to move threat math to uint since that's how it must be send to client and because, IMHO, a unit's threat can be negative that's why now some spell or scripts bork the threat meter.

[Auntie Mangos]

In any case this not related in any form to discussed table

You are right, Vladimir. Sorry for this, but it's still very intresting, since I'm trying to figure out how spell system and threat works. I will take a closer look on that later.

Well then. I will try to improve the patch the next days, since this could be still useful for us.

We could move discussion on topic back to http://getmangos.eu/community/topic/15271/bug-threat-calculation/page/2/ I think.

[darkstalker]

why not just add an extra column to spell_threat?

[sysexpection]

Thanks for your comment. Of course I could, but in my opinion it's the best way, to seperate these two functionalities.

By the way... I think it would be better to multiplicate threat values with 100 in ThreatManager::calcThreat()

[Ambal]

It is really better to add another column to 'spell_threat' table since it is not 'separate functionality'

[sysexpection]

Well, first of all, happy new year

So here we go again. I've updated the patch and made a diff for rev. 10944. You might wanna use it know.

darkstalker and Ambal thanks for you comments again. I've added an extra column to spell_threat, but the maps, load functions, reload function, etc. are still seperated. If you think it's better to combine, I can extend the existing functions for additional spell threat. In my opinion the code is a litte bit easier to read this way and the performance loss is very small in this case.

regards

[Schmoozerd]

hmm, why not load (and reload by same command) with the rest of spell_threat table?

should make some parts easier of the code, you simyple push to the different map the values for the entries;

Speeding up the whole problem should be possible by setting an invalid (perhaps -1.0f - might also be 0 possible) as default for the table, and only push spells with not -1 values to the map.

(Just an idea)

Also I think you can remove the wrapper in unit-class, such that your new damage calculation will be:

damage * sSpellMgr.GetSpellThreatMultiplier(spellProto->Id);

[sysexpection]

Well, you're right. Guess I'm just lazy, hah

My bad. Work in progress

[sysexpection]

Updated again. I've extended the existing functions for additional threat in order to load multipliers.

Speeding up the whole problem should be possible by setting an invalid (perhaps -1.0f - might also be 0 possible) as default for the table, and only push spells with not -1 values to the map.

Well, very good idea. The new column `Threat_multiplier` has default value -1 and the older `Threat` column has now default value '0'.

So if `Threat` == 0 we won't push to SpellThreatMap and if `Threat_multiplier` == -1 we won't push to SpellThreatMultiplierMap (cause there might be some cases we could use the 0 multiplier to remove threat generation).

Kind regards

[Lynx3d]

I've been working on this a bit lately, but i did a few things differently.

- I'm not adding another map, but follow the rest of the code, that is, one table => one map => one (re)load function.

- I added support for auto-filling higher spell ranks as in spell_proc_event, i'm just not very happy with the amount of duplicated code yet. Suggestions welcome.

- I moved threat multiplication to ThreatCalcHelper::calcThreat(), seemed more logical to me unless we really need different cases for damage and healing.

And i think we need a third column for attack power bonus, because non-damaging spells like sunder armor scale with AP.

https://gist.github.com/762020

[VladimirMangos]

Only duplication avoid, maybe adding some abstract template helper for hide duplicate code, but i not see how this do nice way.

[sysexpection]

- I moved threat multiplication to ThreatCalcHelper::calcThreat(), seemed more logical to me unless we really need different cases for damage and healing.

There is nothing to discuss. You're right of course. Sometimes I'm just too blind or too lazy, whatever.

And i think we need a third column for attack power bonus, because non-damaging spells like sunder armor scale with AP.

https://gist.github.com/762020

I've totally forgotten about cases like that. Well thanks for gist and your ideas. Now I have something to workout. That may take some time, since I'm not the best C++ programmer here. Mostly I work on process XML data, or general data to import it into databases and then push it into the ERP system. But it's time to grow up

[sysexpection]

I have implemented an AP based multiplier for now:

https://gist.github.com/763765

This is based on Lynx3d's patch (thanks a lot for the well commented sourcecode, helped me a lot)

I added an extra column, named `AP_Muliplier`. Now: If AP_Multiplier != 0.0 it will call Unit::calculateDamage(..) (base attack, normalised = false) and multiply the return value with the given AP multiplier from the database.

Here an example. Values based on this: http://www.tankspot.com/showthread.php?39775-wow-3-0-threat-values

Let's take Sunder Armor:

mysql> select * from spell_threat where entry = 7386;
------- -------- ------------ --------------- 
| entry | Threat | Multiplier | AP_Multiplier |
------- -------- ------------ --------------- 
|  7386 |    345 |          1 |          0,05 |
------- -------- ------------ --------------- 
1 row in set

My Testwarrior has 708 to 972 AP. I'm in Defense Stance.

Let's start with the basic threat of 375.

CalculateDamage returns 954 * AP_Multiplier 0.05 = 47.7

Threat is now 392.7.

Multiplied with our databased threat multiplier (default value 1.0) it's still 392.7.

After applying total threat modifier (1.45, defense stance) we have 569.415.

Sounds much more better than the 100.0 * 1.45 from actual YTDB.

Well, I'm not sure if ThreatCalcHelper::calcThreat is the best place to put this code. Also I'm not sure, if there is any spell, generating AP based threat, that can crit, or if stance has to be applied in every case.

Well thanks for any comments, ideas for improvement, etc.

And thanks to Lynx3d again

Updated gist: Changed something and added the queries (sorry)

[sysexpection]

And i think we need a third column for attack power bonus, because non-damaging spells like sunder armor scale with AP.

How can be handled spell like 45477 Icy Touch (Rank 1) "Very high threat when in Frost Presence.", x7 damage according wowwiki?

Well, thats a very good question. Maybe a switch/case would help. Are there many spells handled like this?

Easiest way would be a dirty "hack" fix like this:

float ThreatCalcHelper::calcThreat(Unit* pHatedUnit, Unit* /*pHatingUnit*/, float pThreat, bool crit, SpellSchoolMask schoolMask, SpellEntry const *pThreatSpell)
{
   // all flat mods applied early
   if(!pThreat)
       return 0.0f;

   if (pThreatSpell)
   {
       SpellThreatEntry const* threatEntry = sSpellMgr.GetSpellThreatEntry(pThreatSpell->Id);

       if (Player* modOwner = pHatedUnit->GetSpellModOwner())
       {
           switch (pThreatSpell->Id)
           {
               case 45477:
                   if(modOwner->HasAura(48266))
                       pThreat *= 7.0f;
               break;
           }

           modOwner->ApplySpellMod(pThreatSpell->Id, SPELLMOD_THREAT, pThreat);

           if(threatEntry && threatEntry->ap_multiplier)
               pThreat += modOwner->CalculateDamage(BASE_ATTACK, false) * threatEntry->ap_multiplier;
       }

       if (crit)
           pThreat *= pHatedUnit->GetTotalAuraMultiplierByMiscMask(SPELL_AURA_MOD_CRITICAL_THREAT,schoolMask);

       if (threatEntry)
           pThreat *= threatEntry->multiplier;
   }

   float threat = pHatedUnit->ApplyTotalThreatModifier(pThreat, schoolMask);
   return threat;
}

Maybe we could think about a conditional multiplier (condition may be also stored in database...)?

if(player->HasAura(....)) { pThreat *= x.xf); }

I moved threat multiplication to ThreatCalcHelper::calcThreat(), seemed more logical to me unless we really need different cases for damage and healing.

I'm not sure but with you patch healing and damage do the same threat while, for what I know, healing usually should generate half amount

Well, healing threat should be multiplied with 0.5. In fact every groupmember sould gain (threat*0.5)/amount_of_groupmembers threat. This is allready implement for most (maybe not all) cases.

As you can see here in the callstack, healing spells are processed by ThreatCalcHelper::calcThreat(), too.

In part realated to this, time ago I was working on threat from float to uint32

it's based on an old revision and I had no time to test it but maybe it can be usefull, the main idea was to move threat math to uint since that's how it must be send to client and because, IMHO, a unit's threat can be negative that's why now some spell or scripts bork the threat meter.

Yeah, I know, in the bug report thread. Well would like to hear some more voices on that topic. Another point is that integer calculations are faster then floating point math. It's worth to take a look on that.

[Lynx3d]

How can be handled spell like 45477 Icy Touch (Rank 1) "Very high threat when in Frost Presence.", x7 damage according wowwiki?

This in fact already works, it's handled by spell aura 61261.

I already tested it after Vladimir pointed me to that...

[sysexpection]

How can be handled spell like 45477 Icy Touch (Rank 1) "Very high threat when in Frost Presence.", x7 damage according wowwiki?

This in fact already works, it's handled by spell aura 61261.

I already tested it after Vladimir pointed me to that...

I have tested this too, and you're right. Unfortunetly, this applies for Unholy and Blood Presence too, but I couldn't figure out why...

As you can see, the iterator returns the SpellModifier for ID 61261 (Frost Presence) but I've just switched to Bloodpresence a few seconds ago. Also I cannot see the SpellModifier, which the iterator returns, in the player object? Any ideas or explainations? Maybe I just don't get it...

[Lynx3d]

Hm i figured it was because my test char had Improved Frost Presence skilled, but there might be a bug.

The aura is applied in SpellAuraHolder::HandleSpellSpecificBoosts()

[VladimirMangos]

as i already notes to Lynx3d auras applied as custom cast and expected already for some from effects custom base points set

at normal triggering from own presense and triggering from other presences with improved talent.

Possible in last case for related effect basepoints must be explicitly set to 0. In any case this not related in any form to discussed table

[Ambal]

Any progress on this patch?

[Lynx3d]

Sorry for taking so long, but i had to investigate why the existing bonus threat from spells like warrior shouts didn't work at all, and while writing a fix i noticed there's even more wrong in threatAssist, basicllay due to a false assumption, self-healing/-buffing didn't actually have any effect because it tried to make the player hate the target instead of vice versa

Anyway, after all this was sorted, i could finally decide where to best put things, and basically went back to the original suggestion instead of ThreatCalcHelper::calcThreat(), because there's at least one ability (Lacerate) that has a flat threat bonus and a custom multiplier.

With some tweaks here and there, accepted in [11023]

[sysexpection]

Any progress on this patch?

I'm sorry that I had not reported something, but my old computer is broken.

Well I am back and thanks