Jump to content

[11623][dev] SpellFamilyFlags wrapper


darkstalker

Recommended Posts

This is an in-progress implementation of a bitset-like class that wraps the 96 bit SpellFamilyFlags + SpellFamilyFlags2 into a single object that can be accessed by individual bit position. Its implemented via template metaprogramming in order to achieve compile-time bit mask calculation in a transparent way with no overhead. This gives a more readable and verbose version of stuff like:

           case SPELLFAMILY_WARLOCK:
               // For Hellfire Effect / Rain of Fire / Seed of Corruption triggers need do it
               if (m_spellInfo->SpellFamilyFlags & UI64LIT(0x0000800000000060))
                   m_canTrigger = true;
               break;
           case SPELLFAMILY_PRIEST:
               // For Penance,Mind Sear,Mind Flay heal/damage triggers need do it
               if (m_spellInfo->SpellFamilyFlags & UI64LIT(0x0001800000800000) || (m_spellInfo->SpellFamilyFlags2 & 0x00000040))
                   m_canTrigger = true;
               break;

that becomes:

           case SPELLFAMILY_WARLOCK:
               // For Hellfire Effect / Rain of Fire / Seed of Corruption triggers need do it
               if (m_spellInfo->SpellClassMask.test<CF_WARLOCK_RAIN_OF_FIRE, CF_WARLOCK_HELLFIRE, CF_WARLOCK_SEED_OF_CORRUPTION2>())
                   m_canTrigger = true;
               break;
           case SPELLFAMILY_PRIEST:
               // For Penance,Mind Sear,Mind Flay heal/damage triggers need do it
               if (m_spellInfo->SpellClassMask.test<CF_PRIEST_MIND_FLAY1, CF_PRIEST_PENANCE_DMG, CF_PRIEST_PENANCE_HEAL, CF_PRIEST_MIND_FLAY2>())
                   m_canTrigger = true;
               break;

Where CF_* is an enum that associates the bit position with a name like:

enum CLASS_FLAG
{
   CF_MAGE_FIREBALL                        = 0,
   CF_MAGE_FIRE_BLAST                      = 1,
   CF_MAGE_FLAMESTRIKE                     = 2,
   CF_MAGE_FIRE_WARD                       = 3,
   CF_MAGE_SCORCH                          = 4,
   CF_MAGE_FROSTBOLT                       = 5,
   // ...

Source code with the class here: http://paste2.org/p/1094906

sample implementation here

new version for 11623+

Link to comment
Share on other sites

Somme points at suggested patch:

1) I agree that have some wrapper for int64+int32 bit field (or at least for field access) can be useful and link really connected field in single object.

2) BUT for me totally look wrong use bit pos: (a) this just create additional problems for devs because most often steps in work with masks/bitpos is recheck it in spell.dbc and most dbc viewers let easy visually compare hex/bit view. In fact same spell can have many bits set for different purposes, and mask store not spell info but for selection some spells list (often not ranks of same spell)

3) use enums for mask can be useful or maybe not. Currently we use comments for mask using line for referecne what spell we check in line. In comment mostly referecne _exactly_ spell name that can be easy copied to clipboard for later search by web or spell.dbc or code.. enum will not let do it easy, one more problem for devs.

Link to comment
Share on other sites

  • 6 months later...
  • 7 months later...

A little bump here.

I am told - and can agree - that the current uint64 magic numbers are rather hard to read.

But I also agree with vladimir that using "custom" enums to hide the actual masks make things very complicated, and also the template interface is not really easy to understand when trying to research.

actually I think the version that would be best would look something like

HasMask<WARLOCK>(CF_WARLOCK_INFERNO | CF_WARLOCK_FIRERING);
..
enum WarlockClassMasks uint64
{
 CF_WARLOCK_INFERNO  = 0x0000010000000000,
 CF_WARLOCK_FIRERING = 0x0010000000000000,
}

However as far as I know such constructs are not possible with compiler < C++11

But it might be possible to use (or something very similar)

static const uint64 CF_WARLOCK_FIRERING = 0x0010000000000000;
...

Any input about this?

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