Rewriting Unit.cpp

Hi, I’m in the process of rewriting unit.cpp to make it faster (possibly) by using switches instead of ifs.

I’ll submit the patch here, because I can’t make a pull request (I can’t, I don’t know why), but in order to create a working unit.cpp I need to know if I should put before the dummySpell->Id or dummySpell->SpellIconId.

Basically: What’s dummySpell->Id for? And what’s dummySpell->SpellIconId for?

Which one is more specific and unambiguous (therefore should go first)

What makes you think that switches are any faster than ifs?

EDIT:

besides I think this is in the wrong section, as Unit.cpp is part of TrinityCore and it’s not a “script”, it more belongs to the “core” development section. /emoticons/default_wink.png

What

The

Fuck.

  1. if the section is not right someone please change it

  2. switches are cleaner to read

  3. to give homogeneity to the code

  4. probably in unit.cpp it doesn’t make much of a difference, because all ifs are break-ed or returne-ed, but a switch = a series of “if - else if…”, while in unit.cpp there’s only “if - if - if …”.

As I said it shouldn’t make any difference because they are all breaking / returning, but it’s definitely faster to read.

[SPOILER]

[CODE]

        // Frozen Power

        if (dummySpell->SpellIconID == 3780)

        {

            if (GetDistance(target) < 15.0f)

                return false;

            float chance = (float)triggerAmount;

            if (!roll_chance_f(chance))

                return false;


            triggered_spell_id = 63685;

            break;

        }

        // Storm, Earth and Fire

        if (dummySpell->SpellIconID == 3063)

        {

            // Earthbind Totem summon only

            if (procSpell->Id != 2484)

                return false;


            float chance = (float)triggerAmount;

            if (!roll_chance_f(chance))

                return false;


            triggered_spell_id = 64695;

            break;

        }

        // Ancestral Awakening

        if (dummySpell->SpellIconID == 3065)

        {

            triggered_spell_id = 52759;

            basepoints0 = CalculatePctN(int32(damage), triggerAmount);

            target = this;

            break;

        }[/CODE][/SPOILER]

This is becoming:

[CODE]

        switch (dummySpell->SpellIconID)

        {

            // Frozen Power

            case 3780:

            {

                if (GetDistance(target) < 15.0f)

                    return false;

                float chance = (float)triggerAmount;

                if (!roll_chance_f(chance))

                    return false;


                triggered_spell_id = 63685;

                break;

            }

            // Storm, Earth and Fire

            case 3063:

            {

                // Earthbind Totem summon only

                if (procSpell->Id != 2484)

                    return false;


                float chance = (float)triggerAmount;

                if (!roll_chance_f(chance))

                    return false;


                triggered_spell_id = 64695;

                break;

            }

            // Ancestral Awakening

            case 3065:

            {

                triggered_spell_id = 52759;

                basepoints0 = CalculatePctN(int32(damage), triggerAmount);

                target = this;

                break;

            }

        }[/CODE]

It’s nothing big, but I would like to give my contribute, so I don’t get Kaelima’s message «What The Fuck.»

I’m not sure, but I think the compiler optimizes the if/else usage and makes almost the same performance than switch. You should try to rewrite some parts and check the performance to you don’t waste your time rewriting the whole unit.cpp and the performance still the same. And god bless the compilers optimizations magic /emoticons/default_tongue.png