[7901] Refactoting pet action bar related code. Fixed some related bugs. Author: VladimirMangos

* Correctly update action bar at loading and other cases when listed unlearned/not existed spells
    * Avoid send data by PetSpellInitialize() many times while pet loading

--HG--
branch : trunk

src/game/Pet.cpp

@@ -272,26 +272,12 @@ bool Pet::LoadPetFromDB( Player* owner, uint32 petentry, uint32 petnumber, bool
         // Load action bar data
         if (!is_temporary_summoned)
         {
-            // permanent controlled pets store state in DB
+            if(!m_charmInfo->LoadActionBar(fields[14].GetCppString()))
-            Tokens tokens = StrSplit(fields[14].GetString(), " ");
-
-            if (tokens.size() != 20)
             {
                 delete result;
                 return false;
             }
 
-            int index;
-            Tokens::iterator iter;
-            for(iter = tokens.begin(), index = 0; index < 10; ++iter, ++index )
-            {
-                m_charmInfo->GetActionBarEntry(index)->Type = atol((*iter).c_str());
-                ++iter;
-                m_charmInfo->GetActionBarEntry(index)->SpellOrAction = atol((*iter).c_str());
-                if(m_charmInfo->GetActionBarEntry(index)->Type == ACT_ENABLED && !IsAutocastableSpell(m_charmInfo->GetActionBarEntry(index)->SpellOrAction))
-                    m_charmInfo->GetActionBarEntry(index)->Type = ACT_PASSIVE;
-            }
-
             _LoadSpells();
             _LoadSpellCooldowns();
             LearnPetPassives();
@@ -300,6 +286,8 @@ bool Pet::LoadPetFromDB( Player* owner, uint32 petentry, uint32 petnumber, bool
         }
     }
 
+    CleanupActionBar();                                     // remove unknown spells from action bar after load
+
     delete result;
     sLog.outDebug("New Pet has guid %u", GetGUIDLow());
 
@@ -326,8 +314,6 @@ bool Pet::LoadPetFromDB( Player* owner, uint32 petentry, uint32 petnumber, bool
         }
     }
 
-    //InitLevelupSpellsForLevel();
-
     m_loading = false;
 
     SynchronizeLevelWithOwner();
@@ -414,8 +400,12 @@ void Pet::SavePetToDB(PetSaveMode mode)
             << curmana << ", "
             << GetPower(POWER_HAPPINESS) << ", '";
 
-        for(uint32 i = 0; i < 10; ++i)
+        for(uint32 i = 0; i < MAX_UNIT_ACTION_BAR_INDEX; ++i)
-            ss << uint32(m_charmInfo->GetActionBarEntry(i)->Type) << " " << uint32(m_charmInfo->GetActionBarEntry(i)->SpellOrAction) << " ";
+        {
+            ss << uint32(m_charmInfo->GetActionBarEntry(i)->Type) << " "
+               << uint32(m_charmInfo->GetActionBarEntry(i)->SpellOrAction) << " ";
+        };
+
         ss  << "', "
             << time(NULL) << ", "
             << uint32(m_resetTalentsCost) << ", "
@@ -1267,7 +1257,7 @@ bool Pet::addSpell(uint32 spell_id,ActiveStates active /*= ACT_DECIDE*/, PetSpel
                 // skip unknown ranks
                 if(!HasSpell(rankSpellId))
                     continue;
-                removeSpell(rankSpellId,false);
+                removeSpell(rankSpellId,false,false);
             }
         }
     }
@@ -1288,7 +1278,7 @@ bool Pet::addSpell(uint32 spell_id,ActiveStates active /*= ACT_DECIDE*/, PetSpel
                         ToggleAutocast(itr2->first, false);
 
                     oldspell_id = itr2->first;
-                    unlearnSpell(itr2->first,false);
+                    unlearnSpell(itr2->first,false,false);
                     break;
                 }
                 // ignore new lesser rank
@@ -1303,7 +1293,7 @@ bool Pet::addSpell(uint32 spell_id,ActiveStates active /*= ACT_DECIDE*/, PetSpel
     if (IsPassiveSpell(spell_id))
         CastSpell(this, spell_id, true);
     else
-        m_charmInfo->AddSpellToAB(oldspell_id, spell_id);
+        m_charmInfo->AddSpellToActionBar(spell_id);
 
     if(newspell.active == ACT_ENABLED)
         ToggleAutocast(spell_id, true);
@@ -1367,7 +1357,7 @@ void Pet::InitLevelupSpellsForLevel()
 
             // will called first if level down
             if(spellEntry->spellLevel > level)
-                unlearnSpell(spellEntry->Id,false);
+                unlearnSpell(spellEntry->Id,true);
             // will called if level up
             else
                 learnSpell(spellEntry->Id);
@@ -1375,9 +1365,9 @@ void Pet::InitLevelupSpellsForLevel()
     }
 }
 
-bool Pet::unlearnSpell(uint32 spell_id, bool learn_prev)
+bool Pet::unlearnSpell(uint32 spell_id, bool learn_prev, bool clear_ab)
 {
-    if(removeSpell(spell_id,learn_prev))
+    if(removeSpell(spell_id,learn_prev,clear_ab))
     {
         if(!m_loading)
         {
@@ -1390,7 +1380,7 @@ bool Pet::unlearnSpell(uint32 spell_id, bool learn_prev)
     return false;
 }
 
-bool Pet::removeSpell(uint32 spell_id, bool learn_prev)
+bool Pet::removeSpell(uint32 spell_id, bool learn_prev, bool clear_ab)
 {
     PetSpellMap::iterator itr = m_spells.find(spell_id);
     if (itr == m_spells.end())
@@ -1421,29 +1411,35 @@ bool Pet::removeSpell(uint32 spell_id, bool learn_prev)
     if (learn_prev)
     {
         if (uint32 prev_id = spellmgr.GetPrevSpellInChain (spell_id))
-        {
-            // replace to next spell
-            if(!talentCost && !IsPassiveSpell(prev_id))
-                m_charmInfo->AddSpellToAB(spell_id, prev_id);
-
             learnSpell(prev_id);
-        }
         else
             learn_prev = false;
     }
 
     // if remove last rank or non-ranked then update action bar at server and client if need
-    if(!learn_prev && m_charmInfo->AddSpellToAB(spell_id, 0))
+    if (clear_ab && !learn_prev && m_charmInfo->RemoveSpellFromActionBar(spell_id))
     {
-        // need update action bar for last removed rank
+        if(!m_loading)
-        if (Unit* owner = GetOwner())
+        {
-            if (owner->GetTypeId() == TYPEID_PLAYER)
+            // need update action bar for last removed rank
-                ((Player*)owner)->PetSpellInitialize();
+            if (Unit* owner = GetOwner())
+                if (owner->GetTypeId() == TYPEID_PLAYER)
+                    ((Player*)owner)->PetSpellInitialize();
+        }
     }
 
     return true;
 }
 
+
+void Pet::CleanupActionBar()
+{
+    for(int i = 0; i < MAX_UNIT_ACTION_BAR_INDEX; ++i)
+        if(UnitActionBarEntry const* ab = m_charmInfo->GetActionBarEntry(i))
+            if(ab->SpellOrAction && ab->IsActionBarForSpell() && !HasSpell(ab->SpellOrAction))
+                m_charmInfo->SetActionBar(i,0,ACT_DISABLED);
+}
+
 void Pet::InitPetCreateSpells()
 {
     m_charmInfo->InitPetActionBar();

src/game/Pet.h

@@ -199,8 +199,9 @@ class Pet : public Guardian
         bool learnSpell(uint32 spell_id);
         void learnSpellHighRank(uint32 spellid);
         void InitLevelupSpellsForLevel();
-        bool unlearnSpell(uint32 spell_id, bool learn_prev);
+        bool unlearnSpell(uint32 spell_id, bool learn_prev, bool clear_ab = true);
-        bool removeSpell(uint32 spell_id, bool learn_prev);
+        bool removeSpell(uint32 spell_id, bool learn_prev, bool clear_ab = true);
+        void CleanupActionBar();
 
         PetSpellMap     m_spells;
         AutoSpellList   m_autospells;
@@ -249,4 +250,4 @@ class Pet : public Guardian
             assert(false);
         }
 };
-#endif
+#endif

src/game/PetHandler.cpp

@@ -393,7 +393,11 @@ void WorldSession::HandlePetSetAction( WorldPacket & recv_data )
 
         sLog.outDetail( "Player %s has changed pet spell action. Position: %u, Spell: %u, State: 0x%X", _player->GetName(), position, spell_id, act_state);
 
-                                                            //if it's act for spell (en/disable/cast) and there is a spell given (0 = remove spell) which pet doesn't know, don't add
+        //ignore invalid position
+        if(position >= MAX_UNIT_ACTION_BAR_INDEX)
+            return;
+
+        //if it's act for spell (en/disable/cast) and there is a spell given (0 = remove spell) which pet doesn't know, don't add
         if(!((act_state == ACT_ENABLED || act_state == ACT_DISABLED || act_state == ACT_PASSIVE) && spell_id && !pet->HasSpell(spell_id)))
         {
             //sign for autocast
@@ -414,8 +418,7 @@ void WorldSession::HandlePetSetAction( WorldPacket & recv_data )
 
             }
 
-            charmInfo->GetActionBarEntry(position)->Type = act_state;
+            charmInfo->SetActionBar(position,spell_id,ActiveStates(act_state));
-            charmInfo->GetActionBarEntry(position)->SpellOrAction = spell_id;
         }
     }
 }
@@ -594,11 +597,7 @@ void WorldSession::HandlePetSpellAutocastOpcode( WorldPacket& recvPacket )
     else
         pet->GetCharmInfo()->ToggleCreatureAutocast(spellid, state);
 
-    for(uint8 i = 0; i < 10; ++i)
+    charmInfo->SetSpellAutocast(spellid,state);
-    {
-        if((charmInfo->GetActionBarEntry(i)->Type == ACT_ENABLED || charmInfo->GetActionBarEntry(i)->Type == ACT_DISABLED) && spellid == charmInfo->GetActionBarEntry(i)->SpellOrAction)
-            charmInfo->GetActionBarEntry(i)->Type = state ? ACT_ENABLED : ACT_DISABLED;
-    }
 }
 
 void WorldSession::HandlePetCastSpellOpcode( WorldPacket& recvPacket )

src/game/Player.cpp

@@ -16798,10 +16798,7 @@ void Player::PetSpellInitialize()
     data << uint8(pet->GetReactState()) << uint8(charmInfo->GetCommandState()) << uint16(0);
 
     // action bar loop
-    for(uint32 i = 0; i < MAX_SPELL_CONTROL_BAR; ++i)
+    charmInfo->BuildActionBar(&data);
-    {
-        data << uint32(charmInfo->GetActionBarEntry(i)->Raw);
-    }
 
     size_t spellsCountPos = data.wpos();
 
@@ -16878,8 +16875,7 @@ void Player::PossessSpellInitialize()
     data << uint32(0);
 
     //action bar 40
-    for(uint32 i = 0; i < 10; i++)
+    charmInfo->BuildActionBar(&data);                       //40
-        data << uint16(charmInfo->GetActionBarEntry(i)->SpellOrAction) << uint16(charmInfo->GetActionBarEntry(i)->Type);
 
     //addlist 1
     data << uint8(0);
@@ -16969,10 +16965,7 @@ void Player::CharmSpellInitialize()
     data << uint16(0);
 
     //action bar 40
-    for(uint32 i = 0; i < MAX_SPELL_CONTROL_BAR; ++i)                          //40
+    charmInfo->BuildActionBar(&data);                       //40
-    {
-        data << uint16(charmInfo->GetActionBarEntry(i)->SpellOrAction) << uint16(charmInfo->GetActionBarEntry(i)->Type);
-    }
 
     //add list
     data << uint8(addlist);                                 //1

src/game/Unit.cpp

@@ -11712,42 +11712,23 @@ CharmInfo::~CharmInfo()
 
 void CharmInfo::InitPetActionBar()
 {
-    if (m_barInit)
-        return;
-
     // the first 3 SpellOrActions are attack, follow and stay
     for(uint32 i = 0; i < 3; i++)
     {
-        PetActionBar[i].Type = ACT_COMMAND;
+        SetActionBar(i,COMMAND_ATTACK - i,ACT_COMMAND);
-        PetActionBar[i].SpellOrAction = COMMAND_ATTACK - i;
+        SetActionBar(i + 7,COMMAND_ATTACK - i,ACT_REACTION);
-
-        PetActionBar[i + 7].Type = ACT_REACTION;
-        PetActionBar[i + 7].SpellOrAction = COMMAND_ATTACK - i;
     }
-    for(uint32 i=0; i < 4; i++)
+    for(uint32 i = 0; i < 4; ++i)
-    {
+        SetActionBar(i,0,ACT_DISABLED);
-        PetActionBar[i + 3].Type = ACT_DISABLED;
-        PetActionBar[i + 3].SpellOrAction = 0;
-    }
-    m_barInit = true;
 }
 
 void CharmInfo::InitEmptyActionBar(bool withAttack)
 {
-    if (m_barInit)
+    if(withAttack)
-        return;
+        SetActionBar(0,COMMAND_ATTACK,ACT_COMMAND);
 
-    for(uint32 x = 0; x < 10; ++x)
+    for(uint32 x = withAttack ? 1 : 0; x < MAX_UNIT_ACTION_BAR_INDEX; ++x)
-    {
+        SetActionBar(x,0,ACT_PASSIVE);
-        PetActionBar[x].Type = ACT_PASSIVE;
-        PetActionBar[x].SpellOrAction = 0;
-    }
-    if (withAttack)
-    {
-        PetActionBar[0].Type = ACT_COMMAND;
-        PetActionBar[0].SpellOrAction = COMMAND_ATTACK;
-    }
-    m_barInit = true;
 }
 
 void CharmInfo::InitPossessCreateSpells()
@@ -11764,7 +11745,7 @@ void CharmInfo::InitPossessCreateSpells()
             if(IsPassiveSpell(spellId))
                 m_unit->CastSpell(m_unit, spellId, true);
             else
-                AddSpellToAB(0, spellId, ACT_DISABLED);
+                AddSpellToActionBar(((Creature*)m_unit)->m_spells[i], ACT_PASSIVE);
         }
     }
 }
@@ -11819,41 +11800,57 @@ void CharmInfo::InitCharmCreateSpells()
                         newstate = ACT_DISABLED;
                 }
             }
-            AddSpellToAB(0, spellId, newstate);
+
+            AddSpellToActionBar(spellId, newstate);
         }
     }
 }
 
-bool CharmInfo::AddSpellToAB(uint32 oldid, uint32 newid, ActiveStates newstate)
+bool CharmInfo::AddSpellToActionBar(uint32 spell_id, ActiveStates newstate)
 {
-    // new spell already listed for example in case prepered switch to lesser rank in Pet::removeSpell
+    uint32 first_id = spellmgr.GetFirstSpellInChain(spell_id);
-    for(uint8 i = 0; i < 10; ++i)
-        if (PetActionBar[i].Type == ACT_DISABLED || PetActionBar[i].Type == ACT_ENABLED || PetActionBar[i].Type == ACT_PASSIVE)
-            if (newid && PetActionBar[i].SpellOrAction == newid)
-                return true;
 
-    // old spell can be leasted for example in case learn high rank
+    // new spell rank can be already listed
-    for(uint8 i = 0; i < 10; ++i)
+    for(uint8 i = 0; i < MAX_UNIT_ACTION_BAR_INDEX; ++i)
     {
-        if (PetActionBar[i].Type == ACT_DISABLED || PetActionBar[i].Type == ACT_ENABLED || PetActionBar[i].Type == ACT_PASSIVE)
+        if (PetActionBar[i].SpellOrAction && PetActionBar[i].IsActionBarForSpell())
         {
-            if (PetActionBar[i].SpellOrAction == oldid)
+            if (spellmgr.GetFirstSpellInChain(PetActionBar[i].SpellOrAction) == first_id)
             {
-                PetActionBar[i].SpellOrAction = newid;
+                PetActionBar[i].SpellOrAction = spell_id;
-                if (!oldid)
-                {
-                    if(!IsAutocastableSpell(newid))
-                        PetActionBar[i].Type = ACT_PASSIVE;
-                    else if (newstate == ACT_DECIDE)
-                        PetActionBar[i].Type = ACT_DISABLED;
-                    else
-                        PetActionBar[i].Type = newstate;
-                }
-
                 return true;
             }
         }
     }
+
+    // or use empty slot in other case 
+    for(uint8 i = 0; i < MAX_UNIT_ACTION_BAR_INDEX; ++i)
+    {
+        if (!PetActionBar[i].SpellOrAction && PetActionBar[i].IsActionBarForSpell())
+        {
+            SetActionBar(i,spell_id,newstate == ACT_DECIDE ? IsAutocastableSpell(spell_id) ? ACT_DISABLED : ACT_PASSIVE : newstate);
+            return true;
+        }
+    }
+    return false;
+}
+
+bool CharmInfo::RemoveSpellFromActionBar(uint32 spell_id)
+{
+    uint32 first_id = spellmgr.GetFirstSpellInChain(spell_id);
+
+    for(uint8 i = 0; i < MAX_UNIT_ACTION_BAR_INDEX; ++i)
+    {
+        if (PetActionBar[i].SpellOrAction && PetActionBar[i].IsActionBarForSpell())
+        {
+            if (spellmgr.GetFirstSpellInChain(PetActionBar[i].SpellOrAction) == first_id)
+            {
+                SetActionBar(i,0,ACT_DISABLED);
+                return true;
+            }
+        }
+    }
+
     return false;
 }
 
@@ -11880,6 +11877,50 @@ void CharmInfo::SetPetNumber(uint32 petnumber, bool statwindow)
         m_unit->SetUInt32Value(UNIT_FIELD_PETNUMBER, 0);
 }
 
+bool CharmInfo::LoadActionBar( std::string data )
+{
+    Tokens tokens = StrSplit(data, " ");
+
+    if (tokens.size() != MAX_UNIT_ACTION_BAR_INDEX*2)
+        return false;
+
+    int index;
+    Tokens::iterator iter;
+    for(iter = tokens.begin(), index = 0; index < MAX_UNIT_ACTION_BAR_INDEX; ++iter, ++index )
+    {
+        // use unsigned cast to avoid sign negative format use at long-> ActiveStates (int) conversion
+        PetActionBar[index].Type  = atol((*iter).c_str());
+        ++iter;
+        PetActionBar[index].SpellOrAction = atol((*iter).c_str());
+
+        // check correctness
+        if(PetActionBar[index].IsActionBarForSpell() && !sSpellStore.LookupEntry(PetActionBar[index].SpellOrAction))
+            SetActionBar(index,0,ACT_DISABLED);
+    }
+    return true;
+}
+
+void CharmInfo::BuildActionBar( WorldPacket* data )
+{
+    for(uint32 i = 0; i < MAX_UNIT_ACTION_BAR_INDEX; ++i)
+    {
+        *data << uint16(PetActionBar[i].SpellOrAction);
+        *data << uint16(PetActionBar[i].Type);
+    }
+}
+
+void CharmInfo::SetSpellAutocast( uint32 spell_id, bool state )
+{
+    for(int i = 0; i < MAX_UNIT_ACTION_BAR_INDEX; ++i)
+    {
+        if(spell_id == PetActionBar[i].SpellOrAction && PetActionBar[i].IsActionBarForSpell())
+        {
+            PetActionBar[i].Type = state ? ACT_ENABLED : ACT_DISABLED;
+            break;
+        }
+    }
+}
+
 bool Unit::isFrozen() const
 {
     return HasAuraState(AURA_STATE_FROZEN);

src/game/Unit.h

@@ -770,24 +770,6 @@ struct SpellNonMeleeDamage{
 
 uint32 createProcExtendMask(SpellNonMeleeDamage *damageInfo, SpellMissInfo missCondition);
 
-struct UnitActionBarEntry
-{
-    UnitActionBarEntry() : Raw(0) {}
-
-    union
-    {
-        struct
-        {
-            uint16 SpellOrAction;
-            uint16 Type;
-        };
-        struct
-        {
-            uint32 Raw;
-        };
-    };
-};
-
 #define MAX_DECLINED_NAME_CASES 5
 
 struct DeclinedName
@@ -830,6 +812,20 @@ enum CommandStates
     COMMAND_ABANDON = 3
 };
 
+struct UnitActionBarEntry
+{
+    UnitActionBarEntry() : SpellOrAction(0), Type(ACT_DISABLED) {}
+
+    uint16 SpellOrAction;
+    uint16 Type;
+
+    // helper
+    bool IsActionBarForSpell() const
+    {
+        return Type == ACT_DISABLED || Type == ACT_ENABLED || Type == ACT_PASSIVE;
+    }
+};
+
 struct CharmSpellEntry
 {
     uint16 spellId;
@@ -838,7 +834,9 @@ struct CharmSpellEntry
 
 typedef std::list<Player*> SharedVisionList;
 
-struct TRINITY_DLL_SPEC CharmInfo
+#define MAX_UNIT_ACTION_BAR_INDEX 10
+
+struct CharmInfo
 {
     public:
         explicit CharmInfo(Unit* unit);
@@ -857,15 +855,27 @@ struct TRINITY_DLL_SPEC CharmInfo
         void InitCharmCreateSpells();
         void InitPetActionBar();
         void InitEmptyActionBar(bool withAttack = true);
+
                                                             //return true if successful
-        bool AddSpellToAB(uint32 oldid, uint32 newid, ActiveStates newstate = ACT_DECIDE);
+        bool AddSpellToActionBar(uint32 spellid, ActiveStates newstate = ACT_DECIDE);
+        bool RemoveSpellFromActionBar(uint32 spell_id);
+        bool LoadActionBar(std::string data);
+        void BuildActionBar(WorldPacket* data);
+        void SetSpellAutocast(uint32 spell_id, bool state);
+        void SetActionBar(uint8 index, uint32 spellOrAction,ActiveStates type)
+        {
+            PetActionBar[index].Type = type;
+            PetActionBar[index].SpellOrAction = spellOrAction;
+        }
+        UnitActionBarEntry const* GetActionBarEntry(uint8 index) const { return &(PetActionBar[index]); }
+
         void ToggleCreatureAutocast(uint32 spellid, bool apply);
 
-        UnitActionBarEntry* GetActionBarEntry(uint8 index) { return &(PetActionBar[index]); }
         CharmSpellEntry* GetCharmSpell(uint8 index) { return &(m_charmspells[index]); }
     private:
+
         Unit* m_unit;
-        UnitActionBarEntry PetActionBar[10];
+        UnitActionBarEntry PetActionBar[MAX_UNIT_ACTION_BAR_INDEX];
         CharmSpellEntry m_charmspells[4];
         CommandStates   m_CommandState;
         //ReactStates     m_reactState;