Improved Talents?

[The0dark0one]

I'm not sure if this is the right forum for this but I've been working on my own custom server and have made some very interesting changes. Most spells and talents have been rearranged onto different classes to create a fresh experience.

Here's the problem I have ran into. When I moved talents like Improved Frostbolt onto a different character, it ceased to function.

This is easy to reproduce:

1. Make a non-mage character.
   
2. .learn 10181
   
3. .learn 16766
   
4. Check the cast time of frostbolt in the tooltip.
   

I've verified that the Improved Frostbolt aura is on my characters but it is not actually reducing the cast time as intended. I've exhaustively researched this issue in the source code for about 5 days now and I'm not sure I'm closer to figuring this out.

I know how the HandleAddModifier function works.

void Aura::HandleAddModifier(bool apply, bool Real)
{
   if (GetTarget()->GetTypeId() != TYPEID_PLAYER || !Real)
       { return; }

   if (m_modifier.m_miscvalue >= MAX_SPELLMOD)
       { return; }

   if (apply)
   {
       SpellEntry const* spellProto = GetSpellProto();

       // Add custom charges for some mod aura
       switch (spellProto->Id)
       {
           case 17941:                                     // Shadow Trance
           case 22008:                                     // Netherwind Focus
               GetHolder()->SetAuraCharges(1);
               break;
       }

       m_spellmod = new SpellModifier(
           SpellModOp(m_modifier.m_miscvalue),
           SpellModType(m_modifier.m_auraname),            // SpellModType value == spell aura types
           m_modifier.m_amount,
           this,
           // prevent expire spell mods with (charges > 0 && m_stackAmount > 1)
           // all this spell expected expire not at use but at spell proc event check
           spellProto->StackAmount > 1 ? 0 : GetHolder()->GetAuraCharges());
   }

   ((Player*)GetTarget())->AddSpellMod(m_spellmod, apply);

   ReapplyAffectedPassiveAuras();
}

It creates the Spell Modifier using info from the DBC. AuraName is 107 which indicates that it is adding a flat spell modifier. Misc value is 10 which indicates that it is a spellmod_casting_time modifier. The amount comes from a SimpleCalculation of the base dice 1 and the base amount -501 adding together to make -500 which is 500 milliseconds less than the original cast time.

My theory is that the problem for me is somewhere in this Template <class T> Player::ApplySpellMod function.

template <class T> T Player::ApplySpellMod(uint32 spellId, SpellModOp op, T& basevalue, Spell const* spell)
{
   SpellEntry const* spellInfo = sSpellStore.LookupEntry(spellId);
   if (!spellInfo) { return 0; }
   int32 totalpct = 0;
   int32 totalflat = 0;
   for (SpellModList::iterator itr = m_spellMods[op].begin(); itr != m_spellMods[op].end(); ++itr)
   {
       SpellModifier* mod = *itr;

       if (!IsAffectedBySpellmod(spellInfo, mod, spell))
           { continue; }
       if (mod->type == SPELLMOD_FLAT)
           { totalflat += mod->value; }
       else if (mod->type == SPELLMOD_PCT)
       {
           // skip percent mods for null basevalue (most important for spell mods with charges )
           if (basevalue == T(0))
               { continue; }

           // special case (skip >10sec spell casts for instant cast setting)
           if (mod->op == SPELLMOD_CASTING_TIME  && basevalue >= T(10 * IN_MILLISECONDS) && mod->value <= -100)
               { continue; }

           totalpct += mod->value;
       }

       if (mod->charges > 0)
       {
           if (!spell)
               { spell = FindCurrentSpellBySpellId(spellId); }

           // avoid double use spellmod charge by same spell
           if (!mod->lastAffected || mod->lastAffected != spell)
           {
               --mod->charges;

               if (mod->charges == 0)
               {
                   mod->charges = -1;
                   ++m_SpellModRemoveCount;
               }

               mod->lastAffected = spell;
           }
       }
   }

   float diff = (float)basevalue * (float)totalpct / 100.0f + (float)totalflat;
   basevalue = T((float)basevalue + diff);
   return T(diff);
}

I believe this is where spell cast times are being calculated for the different classes I believe but I've no idea why it only works for the right classes. Frankly I'm not even sure what a template function is. Can anyone offer any help or suggestions?

[The0dark0one]

So now I need to figure out what causes tooltips to update. Here is the situation:

I've modified the Priest talent Shadow Reach so that it has the same SpellFamilyName as Warlock abilities and placed it in the warlock talent trees.

When a player selects this talent, the benefit is not reflected in their spell tooltips.

If a player then selects the Warlock talent Grim Reach, it causes their affliction spells to update their range.

Then it works perfectly and they get the combined effect of both talents (42 yard range corruption ftw).

So the issue is that only their own talents cause tooltips to update but talents coming from other classes do not force the tooltips to update.

My questions are thus:

1. What handles the display of tooltips? An interface LUA file perhaps?
   
2. Why do class fitted spells cause the tooltips to update but non class fitted spells do not?
   

[Olion]

Why do class fitted spells cause the tooltips to update but non class fitted spells do not?

It is exactly the matter I had begin with: the client limitations. I can neither help with the client modifications (I suspect LUA will be of no help to the problem, which is due to some checks with the code; if you're lucky, a proper modification of client Spell.dbc will be sufficient), nor I'm interested in that topic. Sorry.

[Olion]

90% of antiblizzlike modifications, irrespectively of its meaningfulness and beauty, are stopped by the limited client functionality. It is because the client was created not as a multi-purpose kit, but as implementation of particular game with fixed algorithms. I believe it is the case.

C++ templates are used for shortening the code. It is just a reduced form of the "full" code (generated by the compiler) which you can get substituting actual types for T symbol.

Tutorials - Templated Functions - Cprogramming.com

https://msdn.microsoft.com/en-us/library/y2se4kz7.aspx

The example above defines a method called in the core with either uint32 of float for "T" type. Thus, you can get the full code writing that method twice, substituting uint32 and float for "T". So, it contains nothing related to the functionality in question.

Also, the SpellFamilyName field of spells from Spell.dbc may (and should) be checked at spell interactions (in particular, proc'ing). The spells remain fitted, but the value does not correspond to the character class. It may be the reason for client to prevent the spell interaction.

[The0dark0one]

Also, the SpellFamilyName field of spells from Spell.dbc may (and should) be checked at spell interactions (in particular, proc'ing). The spells remain fitted, but the value does not correspond to the character class. It may be the reason for client to prevent the spell interaction.

You're certainly right about that. I tried compiling the server without the SpellFamilyName checks and then it seemed like multiple spells were getting effects from multiple talents. Rogues ended up getting crits of 7000 because their crit damage bonuses were stacking with other crit damage bonuses. It was interesting for about an hour.

I've been steadily attempting to trace this cast time process. And this is where I'm at now:

uint32 GetSpellCastTime(SpellEntry const* spellInfo, Spell const* spell)
{
   if (spell)
   {
       // some triggered spells have data only usable for client
       if (spell->IsTriggeredSpellWithRedundentCastTime())
           { return 0; }

       // spell targeted to non-trading trade slot item instant at trade success apply
       if (spell->GetCaster()->GetTypeId() == TYPEID_PLAYER)
           if (TradeData* my_trade = ((Player*)(spell->GetCaster()))->GetTradeData())
               if (Item* nonTrade = my_trade->GetTraderData()->GetItem(TRADE_SLOT_NONTRADED))
                   if (nonTrade == spell->m_targets.getItemTarget())
                       { return 0; }
   }

   SpellCastTimesEntry const* spellCastTimeEntry = sSpellCastTimesStore.LookupEntry(spellInfo->CastingTimeIndex);

   // not all spells have cast time index and this is all is pasiive abilities
   if (!spellCastTimeEntry)
       { return 0; }

   int32 castTime = spellCastTimeEntry->CastTime;

   if (spell)
   {
       if (Player* modOwner = spell->GetCaster()->GetSpellModOwner())
           { modOwner->ApplySpellMod(spellInfo->Id, SPELLMOD_CASTING_TIME, castTime, spell); }

       if (!spellInfo->HasAttribute(SPELL_ATTR_UNK4) && !spellInfo->HasAttribute(SPELL_ATTR_TRADESPELL))
           { castTime = int32(castTime * spell->GetCaster()->GetFloatValue(UNIT_MOD_CAST_SPEED)); }
       else
       {
           if (spell->IsRangedSpell() && !spell->IsAutoRepeat())
               { castTime = int32(castTime * spell->GetCaster()->m_modAttackSpeedPct[RANGED_ATTACK]); }
       }
   }

   if (spellInfo->HasAttribute(SPELL_ATTR_RANGED) && (!spell || !spell->IsAutoRepeat()))
       { castTime += 500; }

   // [workaround] holy light (spell 19968) has a 2.5 sec cast time in DBC but it should be an instant cast.
   // TODO: Once DBC's are moved to the database, a hotfix can be applied directly to the db and this code removed
   if (spellInfo->Id == 19968)
   {
       castTime = 0;
   }
   return (castTime > 0) ? uint32(castTime) : 0;
}

So I believe I'm certainly close to the solution. I assume that the following condition is what is failing somehow:

        if (Player* modOwner = spell->GetCaster()->GetSpellModOwner())
           { modOwner->ApplySpellMod(spellInfo->Id, SPELLMOD_CASTING_TIME, castTime, spell); }

Which if it is true, then it applys the spellmod. The spellmod only takes as arguments variables about the spell and the mod and doesn't seem to consider the player but the condition does reference the player.

PS: quick question about spell.dbc; columns 161+162 are listed as:

ClassFamilyMask SpellFamilyFlags;                   // 161+162

Does this mean I need to add the 2 columns together to get the full uint64 ClassFamilyMask? I can't figure out if these are supposed to be separate entries with separate purposes or one single entry?

[Olion]

PS: quick question about spell.dbc; columns 161+162 are listed as:

ClassFamilyMask SpellFamilyFlags;                   // 161+162

Does this mean I need to add the 2 columns together to get the full uint64 ClassFamilyMask? I can't figure out if these are supposed to be separate entries with separate purposes or one single entry?

It means that 64-bit integer is represented as two consequent 32-bit ones. Plus means rather integrity of the data, index in these comments enumerates 32-bit words.

After a quick check, I do not see why the core shouldn't send the spellmod arising from Aura::HandleAddModifier to the client. So I would begin with checking the world-packets.log for the packet SMSG_SET_FLAT_SPELL_MODIFIER, in both cases: for legitimate and "non-legitimate" mage. If the packet is present in both cases, then you barely can help the problem serverside.

[The0dark0one]

It means that 64-bit integer is represented as two consequent 32-bit ones. Plus means rather integrity of the data, index in these comments enumerates 32-bit words.

After a quick check, I do not see why the core shouldn't send the spellmod arising from Aura::HandleAddModifier to the client. So I would begin with checking the world-packets.log for the packet SMSG_SET_FLAT_SPELL_MODIFIER, in both cases: for legitimate and "non-legitimate" mage. If the packet is present in both cases, then you barely can help the problem serverside.

Well I really feel like an ass right now. I have spent way too much time and effort only to learn that these spell mods have been working all along. I did as you suggested and packets were being sent in both cases. I hand measured the cast times and found that they were indeed receiving the modified cast times. It was only the cast time in the tooltip not updating. I suppose now it's just a matter of modifying some client XML files or some tooltip code somewhere and I will be on my way to glory.