Core/Groups: Group system cleaning sweep.

src/server/game/Battlegrounds/Battleground.cpp

@@ -1203,7 +1203,7 @@ void Battleground::AddOrSetPlayerToCorrectBgGroup(Player *player, uint32 team)
     {
         group = new Group;
         SetBgRaid(team, group);
-        group->Create(playerGuid, player->GetName());
+        group->Create(player);
     }
     else                                            // raid already exist
     {
@@ -1214,10 +1214,13 @@ void Battleground::AddOrSetPlayerToCorrectBgGroup(Player *player, uint32 team)
         }
         else
         {
-            group->AddMember(playerGuid, player->GetName());
+            group->AddMember(player);
             if (Group* originalGroup = player->GetOriginalGroup())
                 if (originalGroup->IsLeader(playerGuid))
+                {
                     group->ChangeLeader(playerGuid);
+                    group->SendUpdate();
+                }
         }
     }
 }

src/server/game/Chat/Commands/Level3.cpp

@@ -4634,7 +4634,10 @@ bool ChatHandler::HandleGroupLeaderCommand(const char *args)
 
     if (GetPlayerGroupAndGUIDByName(cname, plr, group, guid))
         if (group && group->GetLeaderGUID() != guid)
+        {
             group->ChangeLeader(guid);
+            group->SendUpdate();
+        }
 
     return true;
 }

src/server/game/DungeonFinding/LFGMgr.cpp

@@ -1345,14 +1345,14 @@ void LFGMgr::UpdateProposal(uint32 proposalId, const uint64& guid, bool accept)
             if (!grp)
             {
                 grp = new Group();
-                grp->Create(pguid, plr->GetName());
+                grp->Create(plr);
                 grp->ConvertToLFG();
                 uint64 gguid = grp->GetGUID();
                 SetState(gguid, LFG_STATE_PROPOSAL);
                 sObjectMgr->AddGroup(grp);
             }
             else if (group != grp)
-                grp->AddMember(pguid, plr->GetName());
+                grp->AddMember(plr);
 
             // Update timers
             uint8 role = GetRoles(pguid);

src/server/game/Entities/Player/Player.cpp

@@ -7214,7 +7214,7 @@ void Player::UpdateZone(uint32 newZone, uint32 newArea)
 
     // group update
     if (GetGroup())
-        SetGroupUpdateFlag(GROUP_UPDATE_FLAG_ZONE);
+        SetGroupUpdateFlag(GROUP_UPDATE_FULL);
 
     UpdateZoneDependentAuras(newZone);
 }
@@ -17785,50 +17785,26 @@ void Player::SendSavedInstances()
 }
 
 /// convert the player's binds to the group
-void Player::ConvertInstancesToGroup(Player *player, Group *group, uint64 player_guid)
+void Player::ConvertInstancesToGroup(Player *player, Group *group, bool switchLeader)
 {
-    bool has_binds = false;
-    bool has_solo = false;
-
-    if (player)
-    {
-        player_guid = player->GetGUID();
-        if (!group)
-            group = player->GetGroup();
-    }
-    ASSERT(player_guid);
-
     // copy all binds to the group, when changing leader it's assumed the character
     // will not have any solo binds
 
-    if (player)
+    for (uint8 i = 0; i < MAX_DIFFICULTY; ++i)
     {
-        for (uint8 i = 0; i < MAX_DIFFICULTY; ++i)
+        for (BoundInstancesMap::iterator itr = player->m_boundInstances[i].begin(); itr != player->m_boundInstances[i].end();)
         {
-            for (BoundInstancesMap::iterator itr = player->m_boundInstances[i].begin(); itr != player->m_boundInstances[i].end();)
+            group->BindToInstance(itr->second.save, itr->second.perm, false);
+            // permanent binds are not removed
+            if (switchLeader && !itr->second.perm)
             {
-                has_binds = true;
+                // increments itr in call
-                if (group)
+                player->UnbindInstance(itr, Difficulty(i), false);
-                    group->BindToInstance(itr->second.save, itr->second.perm, true);
-                // permanent binds are not removed
-                if (!itr->second.perm)
-                {
-                    // increments itr in call
-                    player->UnbindInstance(itr, Difficulty(i), true);
-                    has_solo = true;
-                }
-                else
-                    ++itr;
             }
+            else
+                ++itr;
         }
     }
-
-    // if the player's not online we don't know what binds it has
-    if (!player || !group || has_binds)
-        CharacterDatabase.PExecute("INSERT INTO group_instance SELECT guid, instance, permanent FROM character_instance WHERE guid = '%u'", GUID_LOPART(player_guid));
-    // the following should not get executed when changing leaders
-    if (!player || has_solo)
-        CharacterDatabase.PExecute("DELETE FROM character_instance WHERE guid = '%d' AND permanent = 0", GUID_LOPART(player_guid));
 }
 
 bool Player::Satisfy(AccessRequirement const* ar, uint32 target_map, bool report)

src/server/game/Entities/Player/Player.h

@@ -2305,7 +2305,7 @@ class Player : public Unit, public GridObject<Player>
         bool HasPendingBind() const { return _pendingBind != NULL; }
         void SendRaidInfo();
         void SendSavedInstances();
-        static void ConvertInstancesToGroup(Player *player, Group *group = NULL, uint64 player_guid = 0);
+        static void ConvertInstancesToGroup(Player *player, Group *group, bool switchLeader);
         bool Satisfy(AccessRequirement const* ar, uint32 target_map, bool report = false);
         bool CheckInstanceLoginValid();
         bool CheckInstanceCount(uint32 instanceId) const

src/server/game/Groups/Group.cpp

@@ -92,12 +92,13 @@ Group::~Group()
     delete[] m_subGroupsCounts;
 }
 
-bool Group::Create(const uint64 &guid, const char * name)
+bool Group::Create(Player *leader)
 {
+    uint64 leaderGuid = leader->GetGUID();
     uint32 lowguid = sObjectMgr->GenerateLowGuid(HIGHGUID_GROUP);
     m_guid = MAKE_NEW_GUID(lowguid, 0, HIGHGUID_GROUP);
-    m_leaderGuid = guid;
+    m_leaderGuid = leaderGuid;
-    m_leaderName = name;
+    m_leaderName = leader->GetName();
 
     m_groupType  = isBGGroup() ? GROUPTYPE_BGRAID : GROUPTYPE_NORMAL;
 
@@ -106,37 +107,29 @@ bool Group::Create(const uint64 &guid, const char * name)
 
     m_lootMethod = GROUP_LOOT;
     m_lootThreshold = ITEM_QUALITY_UNCOMMON;
-    m_looterGuid = guid;
+    m_looterGuid = leaderGuid;
 
     m_dungeonDifficulty = DUNGEON_DIFFICULTY_NORMAL;
     m_raidDifficulty = RAID_DIFFICULTY_10MAN_NORMAL;
+
     if (!isBGGroup())
     {
-        Player *leader = sObjectMgr->GetPlayer(guid);
+        m_dungeonDifficulty = leader->GetDungeonDifficulty();
-        if (leader)
+        m_raidDifficulty = leader->GetRaidDifficulty();
-        {
-            m_dungeonDifficulty = leader->GetDungeonDifficulty();
-            m_raidDifficulty = leader->GetRaidDifficulty();
-        }
-
-        Player::ConvertInstancesToGroup(leader, this, guid);
-
-        if (!AddMember(guid, name))
-            return false;
 
         // store group in database
-        SQLTransaction trans = CharacterDatabase.BeginTransaction();
+        CharacterDatabase.PExecute("INSERT INTO groups (guid,leaderGuid,lootMethod,looterGuid,lootThreshold,icon1,icon2,icon3,icon4,icon5,icon6,icon7,icon8,groupType,difficulty,raiddifficulty) "
-        trans->PAppend("DELETE FROM groups WHERE guid ='%u'", lowguid);
-        trans->PAppend("DELETE FROM group_member WHERE guid = %u OR memberGuid = %u", lowguid, GUID_LOPART(guid));
-        trans->PAppend("INSERT INTO group_member (guid, memberGuid, subgroup) VALUES (%u, %u, 0)", lowguid, GUID_LOPART(guid));
-        trans->PAppend("INSERT INTO groups (guid,leaderGuid,lootMethod,looterGuid,lootThreshold,icon1,icon2,icon3,icon4,icon5,icon6,icon7,icon8,groupType,difficulty,raiddifficulty) "
             "VALUES ('%u','%u','%u','%u','%u','" UI64FMTD "','" UI64FMTD "','" UI64FMTD "','" UI64FMTD "','" UI64FMTD "','" UI64FMTD "','" UI64FMTD "','" UI64FMTD "','%u','%u','%u')",
             lowguid, GUID_LOPART(m_leaderGuid), uint32(m_lootMethod),
-            GUID_LOPART(m_looterGuid), uint32(m_lootThreshold), m_targetIcons[0], m_targetIcons[1], m_targetIcons[2], m_targetIcons[3], m_targetIcons[4], m_targetIcons[5], m_targetIcons[6], m_targetIcons[7], uint8(m_groupType), uint32(m_dungeonDifficulty), m_raidDifficulty);
+            GUID_LOPART(m_looterGuid), uint32(m_lootThreshold), m_targetIcons[0], m_targetIcons[1], m_targetIcons[2], m_targetIcons[3], m_targetIcons[4], m_targetIcons[5], m_targetIcons[6],
+            m_targetIcons[7], uint8(m_groupType), uint32(m_dungeonDifficulty), m_raidDifficulty);
+
+        ASSERT(AddMember(leader)); // If the leader can't be added to a new group because it appears full, something is clearly wrong.
+
+        Player::ConvertInstancesToGroup(leader, this, false);
 
-        CharacterDatabase.CommitTransaction(trans);
     }
-    else if (!AddMember(guid, name))
+    else if (!AddMember(leader))
         return false;
 
     return true;
@@ -315,15 +308,66 @@ Player* Group::GetInvited(const std::string& name) const
     return NULL;
 }
 
-bool Group::AddMember(const uint64 &guid, const char* name)
+bool Group::AddMember(Player *player)
 {
-    if (!_addMember(guid, name))
+    // Get first not-full group
-        return false;
+    uint8 subGroup = 0;
+    if (m_subGroupsCounts)
+    {
+        bool groupFound = false;
+        for (; subGroup < MAX_RAID_SUBGROUPS; ++subGroup)
+        {
+            if (m_subGroupsCounts[subGroup] < MAXGROUPSIZE)
+            {
+                groupFound = true;
+                break;
+            }
+        }
+        // We are raid group and no one slot is free
+        if (!groupFound)
+            return false;
+    }
+
+    MemberSlot member;
+    member.guid      = player->GetGUID();
+    member.name      = player->GetName();
+    member.group     = subGroup;
+    member.flags     = 0;
+    member.roles     = 0;
+    m_memberSlots.push_back(member);
+
+    SubGroupCounterIncrease(subGroup);
+
+    if (player)
+    {
+        player->SetGroupInvite(NULL);
+        if (player->GetGroup() && isBGGroup()) //if player is in group and he is being added to BG raid group, then call SetBattlegroundRaid()
+            player->SetBattlegroundRaid(this, subGroup);
+        else if (player->GetGroup()) //if player is in bg raid and we are adding him to normal group, then call SetOriginalGroup()
+            player->SetOriginalGroup(this, subGroup);
+        else //if player is not in group, then call set group
+            player->SetGroup(this, subGroup);
+
+        // if the same group invites the player back, cancel the homebind timer
+        InstanceGroupBind *bind = GetBoundInstance(player);
+        if (bind && bind->save->GetInstanceId() == player->GetInstanceId())
+            player->m_InstanceValid = true;
+    }
+
+    if (!isRaidGroup())                                      // reset targetIcons for non-raid-groups
+    {
+        for (uint8 i = 0; i < TARGETICONCOUNT; ++i)
+            m_targetIcons[i] = 0;
+    }
+
+    // insert into the table if we're not a battleground group
+    if (!isBGGroup())
+        CharacterDatabase.PExecute("INSERT INTO group_member (guid, memberGuid, memberFlags, subgroup, roles) VALUES(%u, %u, %u, %u, %u)",
+                                    GUID_LOPART(m_guid), GUID_LOPART(member.guid), member.flags, member.group, member.roles);
 
     SendUpdate();
-    sScriptMgr->OnGroupAddMember(this, guid);
+    sScriptMgr->OnGroupAddMember(this, player->GetGUID());
 
-    Player *player = sObjectMgr->GetPlayer(guid);
     if (player)
     {
         if (!IsLeader(player->GetGUID()) && !isBGGroup())
@@ -367,20 +411,30 @@ uint32 Group::RemoveMember(const uint64 &guid, const RemoveMethod &method /* = G
 
     sScriptMgr->OnGroupRemoveMember(this, guid, method, kicker, reason);
 
-    // Lfg group vote kick handled in scripts
+    // LFG group vote kick handled in scripts
     if (isLFGGroup() && method == GROUP_REMOVEMETHOD_KICK)
         return m_memberSlots.size();
 
     // remove member and change leader (if need) only if strong more 2 members _before_ member remove (BG allow 1 member group)
     if (GetMembersCount() > (isBGGroup() ? 1u : 2u))
     {
-        bool leaderChanged = _removeMember(guid);
+        Player *player = sObjectMgr->GetPlayer(guid);
-
+        if (player)
-        if (Player *player = sObjectMgr->GetPlayer(guid))
         {
-            // quest related GO state dependent from raid membership
+            // Battleground group handling
-            if (isRaidGroup())
+            if (isBGGroup())
+                player->RemoveFromBattlegroundRaid();
+            else
+            // Regular group
+            {
+                if (player->GetOriginalGroup() == this)
+                    player->SetOriginalGroup(NULL);
+                else
+                    player->SetGroup(NULL);
+
+                // quest related GO state dependent from raid membership
                 player->UpdateForQuestWorldObjects();
+            }
 
             WorldPacket data;
 
@@ -390,31 +444,69 @@ uint32 Group::RemoveMember(const uint64 &guid, const RemoveMethod &method /* = G
                 player->GetSession()->SendPacket(&data);
             }
 
-            //we already removed player from group and in player->GetGroup() is his original group!
+            // Do we really need to send this opcode?
-            if (Group* group = player->GetGroup())
+            data.Initialize(SMSG_GROUP_LIST, 1+1+1+1+8+4+4+8);
-                group->SendUpdate();
+            data << uint8(0x10) << uint8(0) << uint8(0) << uint8(0);
-            else
+            data << uint64(m_guid) << uint32(m_counter) << uint32(0) << uint64(0);
-            {
+            player->GetSession()->SendPacket(&data);
-                data.Initialize(SMSG_GROUP_LIST, 1+1+1+1+8+4+4+8);
-                data << uint8(0x10) << uint8(0) << uint8(0) << uint8(0);
-                data << uint64(m_guid) << uint32(m_counter) << uint32(0) << uint64(0);
-                player->GetSession()->SendPacket(&data);
-            }
 
             _homebindIfInstance(player);
         }
 
-        if (leaderChanged)
+        // Remove player from group in DB
+        CharacterDatabase.PExecute("DELETE FROM group_member WHERE memberGuid=%u", GUID_LOPART(guid));
+
+        // Reevaluate group enchanter if the leaving player had enchanting skill or the player is offline
+        if (player && player->GetSkillValue(SKILL_ENCHANTING) || !player)
+            ResetMaxEnchantingLevel();
+
+        // Remove player from loot rolls
+        for (Rolls::iterator it = RollId.begin(); it != RollId.end(); ++it)
         {
-            WorldPacket data(SMSG_GROUP_SET_LEADER, (m_memberSlots.front().name.size()+1));
+            Roll* roll = *it;
-            data << m_memberSlots.front().name;
+            Roll::PlayerVote::iterator itr2 = roll->playerVote.find(guid);
-            BroadcastPacket(&data, true);
+            if (itr2 == roll->playerVote.end())
+                continue;
+
+            if (itr2->second == GREED || itr2->second == DISENCHANT)
+                --roll->totalGreed;
+            else if (itr2->second == NEED)
+                --roll->totalNeed;
+            else if (itr2->second == PASS)
+                --roll->totalPass;
+
+            if (itr2->second != NOT_VALID)
+                --roll->totalPlayersRolling;
+
+            roll->playerVote.erase(itr2);
+
+            CountRollVote(guid, roll->itemGUID, GetMembersCount()-1, MAX_ROLL_TYPE);
+        }
+
+        // Update subgroups
+        member_witerator slot = _getMemberWSlot(guid);
+        if (slot != m_memberSlots.end())
+        {
+            SubGroupCounterDecrease(slot->group);
+            m_memberSlots.erase(slot);
+        }
+
+        // Pick new leader if necessary
+        if (m_leaderGuid == guid)
+        {
+            for (member_witerator itr = m_memberSlots.begin(); itr != m_memberSlots.end(); ++itr)
+            {
+                if (sObjectMgr->GetPlayer(itr->guid))
+                {
+                    ChangeLeader(itr->guid);
+                    break;
+                }
+            }
         }
 
         SendUpdate();
-        ResetMaxEnchantingLevel();
     }
-    // if group before remove <= 2 disband it
+    // If group size before player removal <= 2 then disband it
     else
         Disband();
 
@@ -423,17 +515,54 @@ uint32 Group::RemoveMember(const uint64 &guid, const RemoveMethod &method /* = G
 
 void Group::ChangeLeader(const uint64 &guid)
 {
-    member_citerator slot = _getMemberCSlot(guid);
+    member_witerator slot = _getMemberWSlot(guid);
 
     if (slot == m_memberSlots.end())
         return;
 
-    _setLeader(guid);
+    Player *player = sObjectMgr->GetPlayer(slot->guid);
 
-    WorldPacket data(SMSG_GROUP_SET_LEADER, slot->name.size()+1);
+    // Don't allow switching leader to offline players
+    if (!player)
+        return;
+
+    sScriptMgr->OnGroupChangeLeader(this, m_leaderGuid, guid);
+
+    if (!isBGGroup())
+    {
+        // Remove the groups permanent instance bindings
+        for (uint8 i = 0; i < MAX_DIFFICULTY; ++i)
+        {
+            for (BoundInstancesMap::iterator itr = m_boundInstances[i].begin(); itr != m_boundInstances[i].end();)
+            {
+                if (itr->second.perm)
+                {
+                    itr->second.save->RemoveGroup(this);
+                    m_boundInstances[i].erase(itr++);
+                }
+                else
+                    ++itr;
+            }
+        }
+
+        // Same in the database
+        CharacterDatabase.PExecute("DELETE FROM group_instance WHERE guid=%u AND (permanent = 1 OR instance IN (SELECT instance FROM character_instance WHERE guid = '%u'))",
+            GUID_LOPART(m_guid), player->GetGUIDLow());
+
+        // Copy the permanent binds from the new leader to the group
+        Player::ConvertInstancesToGroup(player, this, true);
+
+        // update the group leader
+        CharacterDatabase.PExecute("UPDATE groups SET leaderGuid='%u' WHERE guid='%u'", player->GetGUIDLow(), GUID_LOPART(m_guid));
+    }
+
+    m_leaderGuid = player->GetGUID();
+    m_leaderName = player->GetName();
+    ToggleGroupMemberFlag(slot, MEMBER_FLAG_ASSISTANT, false);
+
+    WorldPacket data(SMSG_GROUP_SET_LEADER, m_leaderName.size()+1);
     data << slot->name;
     BroadcastPacket(&data, true);
-    SendUpdate();
 }
 
 void Group::Disband(bool hideDestroy /* = false */)
@@ -1203,199 +1332,6 @@ void Group::OfflineReadyCheck()
     }
 }
 
-bool Group::_addMember(const uint64 &guid, const char* name)
-{
-    // get first not-full group
-    uint8 groupid = 0;
-    if (m_subGroupsCounts)
-    {
-        bool groupFound = false;
-        for (; groupid < MAX_RAID_SUBGROUPS; ++groupid)
-        {
-            if (m_subGroupsCounts[groupid] < MAXGROUPSIZE)
-            {
-                groupFound = true;
-                break;
-            }
-        }
-        // We are raid group and no one slot is free
-        if (!groupFound)
-            return false;
-    }
-
-    return _addMember(guid, name, groupid);
-}
-
-bool Group::_addMember(const uint64 &guid, const char* name, uint8 group)
-{
-    if (IsFull())
-        return false;
-
-    if (!guid)
-        return false;
-
-    Player *player = sObjectMgr->GetPlayer(guid);
-
-    MemberSlot member;
-    member.guid      = guid;
-    member.name      = name;
-    member.group     = group;
-    member.flags     = 0;
-    member.roles     = 0;
-    m_memberSlots.push_back(member);
-
-    SubGroupCounterIncrease(group);
-
-    if (player)
-    {
-        player->SetGroupInvite(NULL);
-        if (player->GetGroup() && isBGGroup()) //if player is in group and he is being added to BG raid group, then call SetBattlegroundRaid()
-            player->SetBattlegroundRaid(this, group);
-        else if (player->GetGroup()) //if player is in bg raid and we are adding him to normal group, then call SetOriginalGroup()
-            player->SetOriginalGroup(this, group);
-        else //if player is not in group, then call set group
-            player->SetGroup(this, group);
-
-        // if the same group invites the player back, cancel the homebind timer
-        InstanceGroupBind *bind = GetBoundInstance(player);
-        if (bind && bind->save->GetInstanceId() == player->GetInstanceId())
-            player->m_InstanceValid = true;
-    }
-
-    if (!isRaidGroup())                                      // reset targetIcons for non-raid-groups
-    {
-        for (uint8 i = 0; i < TARGETICONCOUNT; ++i)
-            m_targetIcons[i] = 0;
-    }
-
-    // insert into the table if we're not a battleground group
-    if (!isBGGroup())
-        CharacterDatabase.PExecute("INSERT INTO group_member (guid, memberGuid, memberFlags, subgroup, roles) VALUES(%u, %u, %u, %u, %u)", GUID_LOPART(m_guid), GUID_LOPART(member.guid), member.flags, member.group, member.roles);
-
-    return true;
-}
-
-bool Group::_removeMember(const uint64 &guid)
-{
-    Player *player = sObjectMgr->GetPlayer(guid);
-    if (player)
-    {
-        //if we are removing player from battleground raid
-        if (isBGGroup())
-            player->RemoveFromBattlegroundRaid();
-        else
-        {
-            //we can remove player who is in battleground from his original group
-            if (player->GetOriginalGroup() == this)
-                player->SetOriginalGroup(NULL);
-            else
-                player->SetGroup(NULL);
-        }
-    }
-
-    _removeRolls(guid);
-
-    member_witerator slot = _getMemberWSlot(guid);
-    if (slot != m_memberSlots.end())
-    {
-        SubGroupCounterDecrease(slot->group);
-        m_memberSlots.erase(slot);
-    }
-
-    if (!isBGGroup())
-        CharacterDatabase.PExecute("DELETE FROM group_member WHERE memberGuid=%u", GUID_LOPART(guid));
-
-    if (m_leaderGuid == guid)                                // leader was removed
-    {
-        if (GetMembersCount() > 0)
-            _setLeader(m_memberSlots.front().guid);
-        return true;
-    }
-
-    return false;
-}
-
-void Group::_setLeader(const uint64 &guid)
-{
-    member_witerator slot = _getMemberWSlot(guid);
-    if (slot == m_memberSlots.end())
-        return;
-
-    sScriptMgr->OnGroupChangeLeader(this, m_leaderGuid, guid);
-
-    if (!isBGGroup())
-    {
-        // TODO: set a time limit to have this function run rarely cause it can be slow
-        SQLTransaction trans = CharacterDatabase.BeginTransaction();
-
-        // update the group's bound instances when changing leaders
-        // remove all permanent binds from the group
-        // in the DB also remove solo binds that will be replaced with permbinds
-        // from the new leader
-        trans->PAppend(
-            "DELETE FROM group_instance WHERE guid=%u AND (permanent = 1 OR "
-            "instance IN (SELECT instance FROM character_instance WHERE guid = '%u')"
-            ")", GUID_LOPART(m_guid), GUID_LOPART(slot->guid)
-        );
-
-        Player *player = sObjectMgr->GetPlayer(slot->guid);
-        if (player)
-        {
-            for (uint8 i = 0; i < MAX_DIFFICULTY; ++i)
-            {
-                for (BoundInstancesMap::iterator itr = m_boundInstances[i].begin(); itr != m_boundInstances[i].end();)
-                {
-                    if (itr->second.perm)
-                    {
-                        itr->second.save->RemoveGroup(this);
-                        m_boundInstances[i].erase(itr++);
-                    }
-                    else
-                        ++itr;
-                }
-            }
-        }
-
-        // copy the permanent binds from the new leader to the group
-        // overwriting the solo binds with permanent ones if necessary
-        // in the DB those have been deleted already
-        Player::ConvertInstancesToGroup(player, this, slot->guid);
-
-        // update the group leader
-        trans->PAppend("UPDATE groups SET leaderGuid='%u' WHERE guid='%u'", GUID_LOPART(slot->guid), GUID_LOPART(m_guid));
-        CharacterDatabase.CommitTransaction(trans);
-    }
-
-    m_leaderGuid = slot->guid;
-    m_leaderName = slot->name;
-    ToggleGroupMemberFlag(slot, MEMBER_FLAG_ASSISTANT, false);
-}
-
-void Group::_removeRolls(const uint64 &guid)
-{
-    for (Rolls::iterator it = RollId.begin(); it != RollId.end(); ++it)
-    {
-        Roll* roll = *it;
-        Roll::PlayerVote::iterator itr2 = roll->playerVote.find(guid);
-        if (itr2 == roll->playerVote.end())
-            continue;
-
-        if (itr2->second == GREED || itr2->second == DISENCHANT)
-            --roll->totalGreed;
-        else if (itr2->second == NEED)
-            --roll->totalNeed;
-        else if (itr2->second == PASS)
-            --roll->totalPass;
-
-        if (itr2->second != NOT_VALID)
-            --roll->totalPlayersRolling;
-
-        roll->playerVote.erase(itr2);
-
-        CountRollVote(guid, roll->itemGUID, GetMembersCount()-1, MAX_ROLL_TYPE);
-    }
-}
-
 bool Group::_setMembersGroup(const uint64 &guid, const uint8 &group)
 {
     member_witerator slot = _getMemberWSlot(guid);
@@ -1412,50 +1348,6 @@ bool Group::_setMembersGroup(const uint64 &guid, const uint8 &group)
     return true;
 }
 
-bool Group::_setAssistantFlag(const uint64 &guid, const bool &apply)
-{
-    member_witerator slot = _getMemberWSlot(guid);
-    if (slot == m_memberSlots.end())
-        return false;
-
-    ToggleGroupMemberFlag(slot, MEMBER_FLAG_ASSISTANT, apply);
-
-    if (!isBGGroup())
-        CharacterDatabase.PExecute("UPDATE group_member SET memberFlags='%u' WHERE memberGuid='%u'", slot->flags, GUID_LOPART(guid));
-
-    return true;
-}
-
-bool Group::_setMainTank(const uint64 &guid, const bool &apply)
-{
-    member_witerator slot = _getMemberWSlot(guid);  // First check member slots to see if the target exists
-    if (slot == m_memberSlots.end())
-        return false;
-
-    RemoveUniqueGroupMemberFlag(MEMBER_FLAG_MAINTANK);          // Remove main tank flag from current if any.
-    ToggleGroupMemberFlag(slot, MEMBER_FLAG_MAINTANK, apply);   // And apply main tank flag on new main tank.
-
-    if (!isBGGroup())
-        CharacterDatabase.PExecute("UPDATE group_member SET memberFlags='%u' WHERE memberGuid='%u'", slot->flags, GUID_LOPART(guid));
-
-    return true;
-}
-
-bool Group::_setMainAssistant(const uint64 &guid, const bool &apply)
-{
-    member_witerator slot = _getMemberWSlot(guid);
-    if (slot == m_memberSlots.end())
-        return false;
-
-    RemoveUniqueGroupMemberFlag(MEMBER_FLAG_MAINASSIST);         // Remove main assist flag from current if any.
-    ToggleGroupMemberFlag(slot, MEMBER_FLAG_MAINASSIST, apply);  // Apply main assist flag on new main assist.
-
-    if (!isBGGroup())
-        CharacterDatabase.PExecute("UPDATE group_member SET memberFlags='%u' WHERE memberGuid='%u'", slot->flags, GUID_LOPART(guid));
-
-    return true;
-}
-
 bool Group::SameSubGroup(Player const* member1, Player const* member2) const
 {
     if (!member1 || !member2)
@@ -1466,55 +1358,53 @@ bool Group::SameSubGroup(Player const* member1, Player const* member2) const
         return member1->GetSubGroup() == member2->GetSubGroup();
 }
 
-// allows setting subgroup for offline members
+// Allows setting sub groups both for online or offline members
 void Group::ChangeMembersGroup(const uint64 &guid, const uint8 &group)
 {
+    // Only raid groups have sub groups
     if (!isRaidGroup())
         return;
 
-    Player *player = sObjectMgr->GetPlayer(guid);
+    // Check if player is really in the raid
-
+    member_witerator slot = _getMemberWSlot(guid);
-    if (!player)
+    if (slot == m_memberSlots.end())
-    {
-        uint8 prevSubGroup = GetMemberGroup(guid);
-        if (prevSubGroup == group)
-            return;
-
-        if (_setMembersGroup(guid, group))
-        {
-            SubGroupCounterDecrease(prevSubGroup);
-            SendUpdate();
-        }
-    }
-    else
-        // This methods handles itself groupcounter decrease
-        ChangeMembersGroup(player, group);
-}
-
-// only for online members
-void Group::ChangeMembersGroup(Player *player, const uint8 &group)
-{
-    if (!player || !isRaidGroup())
         return;
 
-    uint8 prevSubGroup = player->GetSubGroup();
+    // Abort if the player is already in the target sub group
+    uint8 prevSubGroup = GetMemberGroup(guid);
     if (prevSubGroup == group)
         return;
 
-    if (_setMembersGroup(player->GetGUID(), group))
+    // Update the player slot with the new sub group setting
+    slot->group = group;
+
+    // Increase the counter of the new sub group..
+    SubGroupCounterIncrease(group);
+
+    // ..and decrease the counter of the previous one
+    SubGroupCounterDecrease(prevSubGroup);
+
+    // Preserve new sub group in database for non-raid groups
+    if (!isBGGroup())
+        CharacterDatabase.PExecute("UPDATE group_member SET subgroup='%u' WHERE memberGuid='%u'", group, GUID_LOPART(guid));
+
+    Player *player = sObjectMgr->GetPlayer(guid);
+
+    // In case the moved player is online, update the player object with the new sub group references
+    if (player)
     {
         if (player->GetGroup() == this)
             player->GetGroupRef().setSubGroup(group);
         else
         {
-            //if player is in BG raid, it is possible that he is also in normal raid - and that normal raid is stored in m_originalGroup reference
+            // If player is in BG raid, it is possible that he is also in normal raid - and that normal raid is stored in m_originalGroup reference
             prevSubGroup = player->GetOriginalSubGroup();
             player->GetOriginalGroupRef().setSubGroup(group);
         }
-
-        SubGroupCounterDecrease(prevSubGroup);
-        SendUpdate();
     }
+
+    // Broadcast the changes to the group
+    SendUpdate();
 }
 
 // Retrieve the next Round-Roubin player for the group
@@ -2068,31 +1958,40 @@ void Group::SetBattlegroundGroup(Battleground *bg)
     m_bgGroup = bg;
 }
 
-void Group::SetAssistant(uint64 guid, const bool &apply)
+
+void Group::SetGroupMemberFlag(uint64 guid, const bool &apply, GroupMemberFlags flag)
 {
+    // Assistants, main assistants and main tanks are only available in raid groups
     if (!isRaidGroup())
        return;
 
-    if (_setAssistantFlag(guid, apply))
+    // Check if player is really in the raid
-        SendUpdate();
+    member_witerator slot = _getMemberWSlot(guid);
-}
+    if (slot == m_memberSlots.end())
-
-void Group::SetMainTank(uint64 guid, const bool &apply)
-{
-    if (!isRaidGroup())
         return;
 
-    if (_setMainTank(guid, apply))
+    // Do flag specific actions, e.g ensure uniqueness
-        SendUpdate();
+    switch (flag) {
-}
+        case MEMBER_FLAG_MAINASSIST:
+            RemoveUniqueGroupMemberFlag(MEMBER_FLAG_MAINASSIST);         // Remove main assist flag from current if any.
+            break;
+        case MEMBER_FLAG_MAINTANK:
+            RemoveUniqueGroupMemberFlag(MEMBER_FLAG_MAINTANK);           // Remove main tank flag from current if any.
+            break;
+        case MEMBER_FLAG_ASSISTANT:
+            break;
+        default:
+            return;                                                      // This should never happen
+    }
 
-void Group::SetMainAssistant(uint64 guid, const bool &apply)
+    // Switch the actual flag
-{
+    ToggleGroupMemberFlag(slot, flag, apply);
-    if (!isRaidGroup())
-        return;
 
-    if (_setMainAssistant(guid, apply))
+    // Preserve the new setting in the db
-        SendUpdate();
+    CharacterDatabase.PExecute("UPDATE group_member SET memberFlags='%u' WHERE memberGuid='%u'", slot->flags, GUID_LOPART(guid));
+
+    // Broadcast the changes to the group
+    SendUpdate();
 }
 
 Difficulty Group::GetDifficulty(bool isRaid) const

src/server/game/Groups/Group.h

@@ -73,6 +73,12 @@ enum GroupMemberFlags
     MEMBER_FLAG_MAINASSIST  = 0x04,
 };
 
+enum GroupMemberAssignment
+{
+    GROUP_ASSIGN_MAINTANK   = 0,
+    GROUP_ASSIGN_MAINASSIST = 1,
+};
+
 enum GroupType
 {
     GROUPTYPE_NORMAL = 0x00,
@@ -176,14 +182,14 @@ class Group
         ~Group();
 
         // group manipulation methods
-        bool   Create(const uint64 &guid, const char * name);
+        bool   Create(Player *leader);
         bool   LoadGroupFromDB(const uint32 &guid, QueryResult result, bool loadMembers = true);
         bool   LoadMemberFromDB(uint32 guidLow, uint8 memberFlags, uint8 subgroup, uint8 roles);
         bool   AddInvite(Player *player);
         void   RemoveInvite(Player *player);
         void   RemoveAllInvites();
         bool   AddLeaderInvite(Player *player);
-        bool   AddMember(const uint64 &guid, const char* name);
+        bool   AddMember(Player *player);
         uint32 RemoveMember(const uint64 &guid, const RemoveMethod &method = GROUP_REMOVEMETHOD_DEFAULT, uint64 kicker = 0, const char* reason = NULL);
         void   ChangeLeader(const uint64 &guid);
         void   SetLootMethod(LootMethod method);
@@ -235,10 +241,9 @@ class Group
 
         void ChangeMembersGroup(const uint64 &guid, const uint8 &group);
         void ChangeMembersGroup(Player *player, const uint8 &group);
-        void SetAssistant(uint64 guid, const bool &apply);
-        void SetMainTank(uint64 guid, const bool &apply);
-        void SetMainAssistant(uint64 guid, const bool &apply);
         void SetTargetIcon(uint8 id, uint64 whoGuid, uint64 targetGuid);
+        void SetGroupMemberFlag(uint64 guid, const bool &apply, GroupMemberFlags flag);
+        void RemoveUniqueGroupMemberFlag(GroupMemberFlags flag);
 
         Difficulty GetDifficulty(bool isRaid) const;
         Difficulty GetDungeonDifficulty() const;
@@ -294,18 +299,7 @@ class Group
         void BroadcastGroupUpdate(void);
 
     protected:
-        bool _addMember(const uint64 &guid, const char* name);
-        bool _addMember(const uint64 &guid, const char* name, uint8 group);
-        bool _removeMember(const uint64 &guid);             // returns true if leader has changed
-        void _setLeader(const uint64 &guid);
-
-        void _removeRolls(const uint64 &guid);
-
         bool _setMembersGroup(const uint64 &guid, const uint8 &group);
-        bool _setAssistantFlag(const uint64 &guid, const bool &apply);
-        bool _setMainTank(const uint64 &guid, const bool &apply);
-        bool _setMainAssistant(const uint64 &guid, const bool &apply);
-
         void _homebindIfInstance(Player *player);
 
         void _initRaidSubGroupsCounter();
@@ -313,7 +307,6 @@ class Group
         member_witerator _getMemberWSlot(uint64 Guid);
         void SubGroupCounterIncrease(uint8 subgroup);
         void SubGroupCounterDecrease(uint8 subgroup);
-        void RemoveUniqueGroupMemberFlag(GroupMemberFlags flag);
         void ToggleGroupMemberFlag(member_witerator slot, uint8 flag, bool apply);
 
         MemberSlotList      m_memberSlots;

src/server/game/Server/Protocol/Handlers/GroupHandler.cpp

@@ -57,6 +57,8 @@ void WorldSession::SendPartyResult(PartyOperation operation, const std::string&
 
 void WorldSession::HandleGroupInviteOpcode(WorldPacket & recv_data)
 {
+    sLog->outDebug("WORLD: Received CMSG_GROUP_INVITE");
+
     std::string membername;
     recv_data >> membername;
     recv_data.read_skip<uint32>();
@@ -191,9 +193,16 @@ void WorldSession::HandleGroupInviteOpcode(WorldPacket & recv_data)
 
 void WorldSession::HandleGroupAcceptOpcode(WorldPacket& recv_data)
 {
+    sLog->outDebug("WORLD: Received CMSG_GROUP_ACCEPT");
+
     recv_data.read_skip<uint32>();
     Group *group = GetPlayer()->GetGroupInvite();
-    if (!group) return;
+
+    if (!group)
+        return;
+
+    // Remove player from invitees in any case
+    group->RemoveInvite(GetPlayer());
 
     if (group->GetLeaderGUID() == GetPlayer()->GetGUID())
     {
@@ -201,13 +210,7 @@ void WorldSession::HandleGroupAcceptOpcode(WorldPacket& recv_data)
         return;
     }
 
-    // remove in from ivites in any case
+    // Group is full
-    group->RemoveInvite(GetPlayer());
-
-    /** error handling **/
-    /********************/
-
-    // not have place
     if (group->IsFull())
     {
         SendPartyResult(PARTY_OP_INVITE, "", ERR_GROUP_FULL);
@@ -216,17 +219,25 @@ void WorldSession::HandleGroupAcceptOpcode(WorldPacket& recv_data)
 
     Player* leader = sObjectMgr->GetPlayer(group->GetLeaderGUID());
 
-    // forming a new group, create it
+    // Forming a new group, create it
     if (!group->IsCreated())
     {
-        if (leader)
+        // This can happen if the leader is zoning. To be removed once delayed actions for zoning are implemented
-            group->RemoveInvite(leader);
+        if (!leader)
-        group->Create(group->GetLeaderGUID(), group->GetLeaderName());
+        {
+            group->RemoveAllInvites();
+            return;
+        }
+
+        // If we're about to create a group there really should be a leader present
+        ASSERT(leader);
+        group->RemoveInvite(leader);
+        group->Create(leader);
         sObjectMgr->AddGroup(group);
     }
 
-    // everything's fine, do it, PLAYER'S GROUP IS SET IN ADDMEMBER!!!
+    // Everything is fine, do it, PLAYER'S GROUP IS SET IN ADDMEMBER!!!
-    if (!group->AddMember(GetPlayer()->GetGUID(), GetPlayer()->GetName()))
+    if (!group->AddMember(GetPlayer()))
         return;
 
     group->BroadcastGroupUpdate();
@@ -234,6 +245,8 @@ void WorldSession::HandleGroupAcceptOpcode(WorldPacket& recv_data)
 
 void WorldSession::HandleGroupDeclineOpcode(WorldPacket & /*recv_data*/)
 {
+    sLog->outDebug("WORLD: Received CMSG_GROUP_DECLINE");
+
     Group  *group  = GetPlayer()->GetGroupInvite();
     if (!group) return;
 
@@ -255,6 +268,8 @@ void WorldSession::HandleGroupDeclineOpcode(WorldPacket & /*recv_data*/)
 
 void WorldSession::HandleGroupUninviteGuidOpcode(WorldPacket & recv_data)
 {
+    sLog->outDebug("WORLD: Received CMSG_GROUP_UNINVITE_GUID");
+
     uint64 guid;
     std::string reason;
     recv_data >> guid;
@@ -301,6 +316,8 @@ void WorldSession::HandleGroupUninviteGuidOpcode(WorldPacket & recv_data)
 
 void WorldSession::HandleGroupUninviteOpcode(WorldPacket & recv_data)
 {
+    sLog->outDebug("WORLD: Received CMSG_GROUP_UNINVITE");
+
     std::string membername;
     recv_data >> membername;
 
@@ -343,6 +360,8 @@ void WorldSession::HandleGroupUninviteOpcode(WorldPacket & recv_data)
 
 void WorldSession::HandleGroupSetLeaderOpcode(WorldPacket & recv_data)
 {
+    sLog->outDebug("WORLD: Received CMSG_GROUP_SET_LEADER");
+
     Group *group = GetPlayer()->GetGroup();
     if (!group)
         return;
@@ -357,12 +376,16 @@ void WorldSession::HandleGroupSetLeaderOpcode(WorldPacket & recv_data)
         return;
     /********************/
 
-    // everything's fine, do it
+    // Everything's fine, do it
     group->ChangeLeader(guid);
+
+    group->SendUpdate();
 }
 
 void WorldSession::HandleGroupDisbandOpcode(WorldPacket & /*recv_data*/)
 {
+    sLog->outDebug("WORLD: Received CMSG_GROUP_DISBAND");
+
     Group *grp = GetPlayer()->GetGroup();
     if (!grp)
         return;
@@ -384,6 +407,8 @@ void WorldSession::HandleGroupDisbandOpcode(WorldPacket & /*recv_data*/)
 
 void WorldSession::HandleLootMethodOpcode(WorldPacket & recv_data)
 {
+    sLog->outDebug("WORLD: Received CMSG_LOOT_METHOD");
+
     Group *group = GetPlayer()->GetGroup();
     if (!group)
         return;
@@ -439,6 +464,8 @@ void WorldSession::HandleLootRoll(WorldPacket &recv_data)
 
 void WorldSession::HandleMinimapPingOpcode(WorldPacket& recv_data)
 {
+    sLog->outDebug("WORLD: Received MSG_MINIMAP_PING");
+
     if (!GetPlayer()->GetGroup())
         return;
 
@@ -461,6 +488,8 @@ void WorldSession::HandleMinimapPingOpcode(WorldPacket& recv_data)
 
 void WorldSession::HandleRandomRollOpcode(WorldPacket& recv_data)
 {
+    sLog->outDebug("WORLD: Received MSG_RANDOM_ROLL");
+
     uint32 minimum, maximum, roll;
     recv_data >> minimum;
     recv_data >> maximum;
@@ -488,6 +517,8 @@ void WorldSession::HandleRandomRollOpcode(WorldPacket& recv_data)
 
 void WorldSession::HandleRaidTargetUpdateOpcode(WorldPacket & recv_data)
 {
+    sLog->outDebug("WORLD: Received MSG_RAID_TARGET_UPDATE");
+
     Group *group = GetPlayer()->GetGroup();
     if (!group)
         return;
@@ -516,6 +547,8 @@ void WorldSession::HandleRaidTargetUpdateOpcode(WorldPacket & recv_data)
 
 void WorldSession::HandleGroupRaidConvertOpcode(WorldPacket & /*recv_data*/)
 {
+    sLog->outDebug("WORLD: Received CMSG_GROUP_RAID_CONVERT");
+
     Group *group = GetPlayer()->GetGroup();
     if (!group)
         return;
@@ -535,6 +568,8 @@ void WorldSession::HandleGroupRaidConvertOpcode(WorldPacket & /*recv_data*/)
 
 void WorldSession::HandleGroupChangeSubGroupOpcode(WorldPacket & recv_data)
 {
+    sLog->outDebug("WORLD: Received CMSG_GROUP_CHANGE_SUB_GROUP");
+
     // we will get correct pointer for group here, so we don't have to check if group is BG raid
     Group *group = GetPlayer()->GetGroup();
     if (!group)
@@ -548,78 +583,87 @@ void WorldSession::HandleGroupChangeSubGroupOpcode(WorldPacket & recv_data)
     if (groupNr >= MAX_RAID_SUBGROUPS)
         return;
 
-    /** error handling **/
     uint64 senderGuid = GetPlayer()->GetGUID();
     if (!group->IsLeader(senderGuid) && !group->IsAssistant(senderGuid))
         return;
 
     if (!group->HasFreeSlotSubGroup(groupNr))
         return;
-    /********************/
 
     Player *movedPlayer = sObjectMgr->GetPlayer(name.c_str());
+    uint64 guid;
     if (movedPlayer)
     {
-        //Do not allow leader to change group of player in combat
+        guid = movedPlayer->GetGUID();
-        if (movedPlayer->isInCombat())
-            return;
-
-        // everything's fine, do it
-        group->ChangeMembersGroup(movedPlayer, groupNr);
     }
     else
-        group->ChangeMembersGroup(sObjectMgr->GetPlayerGUIDByName(name.c_str()), groupNr);
+    {
+        CharacterDatabase.escape_string(name);
+        guid = sObjectMgr->GetPlayerGUIDByName(name.c_str());
+    }
+
+    group->ChangeMembersGroup(guid, groupNr);
 }
 
 void WorldSession::HandleGroupAssistantLeaderOpcode(WorldPacket & recv_data)
 {
+    sLog->outDebug("WORLD: Received CMSG_GROUP_ASSISTANT_LEADER");
+
     Group *group = GetPlayer()->GetGroup();
     if (!group)
         return;
 
-    uint64 guid;
-    uint8 flag;
-    recv_data >> guid;
-    recv_data >> flag;
-
-    /** error handling **/
     if (!group->IsLeader(GetPlayer()->GetGUID()))
         return;
-    /********************/
 
-    // everything's fine, do it
+    uint64 guid;
-    group->SetAssistant(guid, (flag != 0));
+    bool apply;
+    recv_data >> guid;
+    recv_data >> apply;
+
+    group->SetGroupMemberFlag(guid, apply, MEMBER_FLAG_ASSISTANT);
+
+    group->SendUpdate();
 }
 
 void WorldSession::HandlePartyAssignmentOpcode(WorldPacket & recv_data)
 {
-    sLog->outDebug("MSG_PARTY_ASSIGNMENT");
+    sLog->outDebug("WORLD: Received MSG_PARTY_ASSIGNMENT");
 
     Group *group = GetPlayer()->GetGroup();
     if (!group)
         return;
 
-    uint8 flag, apply;
-    uint64 guid;
-    recv_data >> flag >> apply;
-    recv_data >> guid;
-
-    /** error handling **/
     uint64 senderGuid = GetPlayer()->GetGUID();
     if (!group->IsLeader(senderGuid) && !group->IsAssistant(senderGuid))
         return;
-    /********************/
 
-    // everything's fine, do it
+    uint8 assignment;
-    if (flag == 0)
+    bool apply;
-        group->SetMainTank(guid, apply);
+    uint64 guid;
+    recv_data >> assignment >> apply;
+    recv_data >> guid;
 
-    else if (flag == 1)
+    switch (assignment)
-        group->SetMainAssistant(guid, apply);
+    {
+        case GROUP_ASSIGN_MAINASSIST:
+            group->RemoveUniqueGroupMemberFlag(MEMBER_FLAG_MAINASSIST);
+            group->SetGroupMemberFlag(guid, apply, MEMBER_FLAG_MAINASSIST);
+            break;
+        case GROUP_ASSIGN_MAINTANK:
+            group->RemoveUniqueGroupMemberFlag(MEMBER_FLAG_MAINTANK);           // Remove main assist flag from current if any.
+            group->SetGroupMemberFlag(guid, apply, MEMBER_FLAG_MAINTANK);
+        default:
+            break;
+    }
+
+    group->SendUpdate();
 }
 
 void WorldSession::HandleRaidReadyCheckOpcode(WorldPacket & recv_data)
 {
+    sLog->outDebug("WORLD: Received MSG_RAID_READY_CHECK");
+
     Group *group = GetPlayer()->GetGroup();
     if (!group)
         return;