Fixing crits for spells

TrinityCore Development TrinityCore Code Discussion
1

Would this be a proper fix for spells that can not crit? (i got this idea from this commit : https://github.com/TrinityCore/TrinityCore/commit/e1c7d95699c823ca380a04965010809ac0a75fd3)


float crit_chance = 0.0f;


     switch(spellProto->DmgClass)

     {

 case SPELL_DAMAGE_CLASS_NONE:

            switch (spellProto->Id)

         {

                case 379:   // Earth Shield

                case 33778: // Lifebloom Final Bloom

                case 64844: // Divine Hymn

                case 8349: // Fire Nova

                case 8190: // Magma Totem

                case 3599: // Searing Totem

                    break;

                default:

                    return false;

            }
2

Are you sure that all those totems spells don’t have a damage class?

3

Totems should be able to crit, as well as pets.

4

It’s quite a bad idea, we should always find a more general and wiser way to do stuff (like the comment that is in that commit); hardcoding spell ids is bad

5

So this is a bad way of making these totems and fire nova able to crit? Anyone have any other ways of doing this?

And Nay, you mean we should use the database to enable certain spells to crit? I do think storing it in the database would be a easier and faster way of doing this

6

Those spells, except Searing totem have SPELL_DAMAGE_CLASS_MAGIC, so it wont work.

In Mangos fire totems can crit, maybe you can look their code and get some idea for a fix

7

It’s very likely because the code stoping mobs to crit.

8

In Unit::isSpellCrit(…) :

There is if(IS_CREATURE_GUID(…)) or something like that, you have to add the check if the NPC is NOT a minion, or a pet, etc.

9

No that would be bad as well; I’m talking about finding one attribute or those spell classes (etc) that match all the spells that are able to crit. Or do what Cron said

10

Fixed in https://github.com/TrinityCore/TrinityCore/commit/ff3c988ee7d57eeceebb4bc8665654efa957110f