Make some adjustments to .summon and .group summon behavior to make them more permissive:

- Now only requires the either target's group leader or target itself to be on your map
- Now summons all applicable group members even if one member fails checks
- No longer has some truly weird edge case instance unbind code that could cause exploit behavior (Really, I have no idea why this existed, because it certainly didn't do what it might've been meant to do.)

3 files changed, 50 insertions, 41 deletions

diff --git a/src/server/game/Miscellaneous/Language.h b/src/server/game/Miscellaneous/Language.h
index a36acf88614..ea86658e771 100644
--- a/src/server/game/Miscellaneous/Language.h
+++ b/src/server/game/Miscellaneous/Language.h
@@ -215,7 +215,8 @@ enum TrinityStrings
     LANG_GRID_POSITION                  = 178,
     // 179-185 used in 6.x branch
     LANG_TRANSPORT_POSITION             = 186,
-    // Room for more level 1              187-199 not used
+    LANG_PARTIAL_GROUP_SUMMON           = 187,
+    // Room for more level 1              188-199 not used
 
     // level 2 chat
     LANG_NO_SELECTION                   = 200,
diff --git a/src/server/scripts/Commands/cs_group.cpp b/src/server/scripts/Commands/cs_group.cpp
index 60d6f3116b5..75733e0170c 100644
--- a/src/server/scripts/Commands/cs_group.cpp
+++ b/src/server/scripts/Commands/cs_group.cpp
@@ -66,7 +66,6 @@ public:
         if (!group)
         {
             handler->PSendSysMessage(LANG_NOT_IN_GROUP, nameLink.c_str());
-            handler->SetSentErrorMessage(true);
             return false;
         }
 
@@ -74,19 +73,22 @@ public:
         Group* gmGroup = gmPlayer->GetGroup();
         Map* gmMap = gmPlayer->GetMap();
         bool toInstance = gmMap->Instanceable();
+        bool onlyLocalSummon = false;
 
-        // we are in instance, and can summon only player in our group with us as lead
-        if (toInstance && (
-            !gmGroup || group->GetLeaderGUID() != gmPlayer->GetGUID() ||
-            gmGroup->GetLeaderGUID() != gmPlayer->GetGUID()))
-            // the last check is a bit excessive, but let it be, just in case
+        // make sure people end up on our instance of the map, disallow far summon if intended destination is different from actual destination
+        // note: we could probably relax this further by checking permanent saves and the like, but eh
+        // :close enough:
+        if (toInstance)
         {
-            handler->SendSysMessage(LANG_CANNOT_SUMMON_TO_INST);
-            handler->SetSentErrorMessage(true);
-            return false;
+            Player* groupLeader = ObjectAccessor::GetPlayer(gmMap, group->GetLeaderGUID());
+            if (!groupLeader || (groupLeader->GetMapId() != gmMap->GetId()) || (groupLeader->GetInstanceId() != gmMap->GetInstanceId()))
+            {
+                handler->SendSysMessage(LANG_PARTIAL_GROUP_SUMMON);
+                onlyLocalSummon = true;
+            }
         }
 
-        for (GroupReference* itr = group->GetFirstMember(); itr != NULL; itr = itr->next())
+        for (GroupReference* itr = group->GetFirstMember(); itr != nullptr; itr = itr->next())
         {
             Player* player = itr->GetSource();
 
@@ -95,27 +97,28 @@ public:
 
             // check online security
             if (handler->HasLowerSecurity(player, ObjectGuid::Empty))
-                return false;
+                continue;
 
             std::string plNameLink = handler->GetNameLink(player);
 
             if (player->IsBeingTeleported())
             {
                 handler->PSendSysMessage(LANG_IS_TELEPORTED, plNameLink.c_str());
-                handler->SetSentErrorMessage(true);
-                return false;
+                continue;
             }
 
             if (toInstance)
             {
                 Map* playerMap = player->GetMap();
 
-                if (playerMap->Instanceable() && playerMap->GetInstanceId() != gmMap->GetInstanceId())
+                if (
+                    (onlyLocalSummon || (playerMap->Instanceable() && playerMap->GetId() == gmMap->GetId())) && // either no far summon allowed or we're in the same map as player (no map switch)
+                    ((playerMap->GetId() != gmMap->GetId()) || (playerMap->GetInstanceId() != gmMap->GetInstanceId())) // so we need to be in the same map and instance of the map, otherwise skip
+                    )
                 {
                     // cannot summon from instance to instance
-                    handler->PSendSysMessage(LANG_CANNOT_SUMMON_TO_INST, plNameLink.c_str());
-                    handler->SetSentErrorMessage(true);
-                    return false;
+                    handler->PSendSysMessage(LANG_CANNOT_SUMMON_INST_INST, plNameLink.c_str());
+                    continue;
                 }
             }
 
diff --git a/src/server/scripts/Commands/cs_misc.cpp b/src/server/scripts/Commands/cs_misc.cpp
index decb37757f8..5eadcf0e07d 100644
--- a/src/server/scripts/Commands/cs_misc.cpp
+++ b/src/server/scripts/Commands/cs_misc.cpp
@@ -504,7 +504,7 @@ public:
                 return false;
             }
 
-            Map* map = handler->GetSession()->GetPlayer()->GetMap();
+            Map* map = _player->GetMap();
 
             if (map->IsBattlegroundOrArena())
             {
@@ -516,30 +516,35 @@ public:
                     return false;
                 }
                 // if both players are in different bgs
-                else if (target->GetBattlegroundId() && handler->GetSession()->GetPlayer()->GetBattlegroundId() != target->GetBattlegroundId())
+                else if (target->GetBattlegroundId() && _player->GetBattlegroundId() != target->GetBattlegroundId())
                     target->LeaveBattleground(false); // Note: should be changed so target gets no Deserter debuff
 
                 // all's well, set bg id
                 // when porting out from the bg, it will be reset to 0
-                target->SetBattlegroundId(handler->GetSession()->GetPlayer()->GetBattlegroundId(), handler->GetSession()->GetPlayer()->GetBattlegroundTypeId());
+                target->SetBattlegroundId(_player->GetBattlegroundId(), _player->GetBattlegroundTypeId());
                 // remember current position as entry point for return at bg end teleportation
                 if (!target->GetMap()->IsBattlegroundOrArena())
                     target->SetBattlegroundEntryPoint();
             }
-            else if (map->IsDungeon())
+            else if (map->Instanceable())
             {
-                Map* destMap = target->GetMap();
-
-                if (destMap->Instanceable() && destMap->GetInstanceId() != map->GetInstanceId())
-                    target->UnbindInstance(map->GetInstanceId(), target->GetDungeonDifficulty(), true);
+                Group* targetGroup = target->GetGroup();
+                Map* targetMap = target->GetMap();
+                Player* targetGroupLeader = ObjectAccessor::GetPlayer(map, targetGroup->GetLeaderGUID());
+                
+                // check if far teleport is allowed
+                if (!targetGroupLeader || (targetGroupLeader->GetMapId() != map->GetId()) || (targetGroupLeader->GetInstanceId() != map->GetInstanceId()))
+                    if ((targetMap->GetId() != map->GetId()) || (targetMap->GetInstanceId() != map->GetInstanceId()))
+                    {
+                        handler->PSendSysMessage(LANG_CANNOT_SUMMON_TO_INST);
+                        handler->SetSentErrorMessage(true);
+                        return false;
+                    }
 
-                // we are in an instance, and can only summon players in our group with us as leader
-                if (!handler->GetSession()->GetPlayer()->GetGroup() || !target->GetGroup() ||
-                    (target->GetGroup()->GetLeaderGUID() != handler->GetSession()->GetPlayer()->GetGUID()) ||
-                    (handler->GetSession()->GetPlayer()->GetGroup()->GetLeaderGUID() != handler->GetSession()->GetPlayer()->GetGUID()))
-                    // the last check is a bit excessive, but let it be, just in case
+                // check if we're already in a different instance of the same map
+                if ((targetMap->GetId() == map->GetId()) && (targetMap->GetInstanceId() != map->GetInstanceId()))
                 {
-                    handler->PSendSysMessage(LANG_CANNOT_SUMMON_TO_INST, nameLink.c_str());
+                    handler->PSendSysMessage(LANG_CANNOT_SUMMON_INST_INST, nameLink.c_str());
                     handler->SetSentErrorMessage(true);
                     return false;
                 }
@@ -561,9 +566,9 @@ public:
 
             // before GM
             float x, y, z;
-            handler->GetSession()->GetPlayer()->GetClosePoint(x, y, z, target->GetCombatReach());
-            target->TeleportTo(handler->GetSession()->GetPlayer()->GetMapId(), x, y, z, target->GetOrientation());
-            target->SetPhaseMask(handler->GetSession()->GetPlayer()->GetPhaseMask(), true);
+            _player->GetClosePoint(x, y, z, target->GetCombatReach());
+            target->TeleportTo(_player->GetMapId(), x, y, z, target->GetOrientation());
+            target->SetPhaseMask(_player->GetPhaseMask(), true);
         }
         else
         {
@@ -577,12 +582,12 @@ public:
 
             // in point where GM stay
             SQLTransaction dummy;
-            Player::SavePositionInDB(WorldLocation(handler->GetSession()->GetPlayer()->GetMapId(),
-                handler->GetSession()->GetPlayer()->GetPositionX(),
-                handler->GetSession()->GetPlayer()->GetPositionY(),
-                handler->GetSession()->GetPlayer()->GetPositionZ(),
-                handler->GetSession()->GetPlayer()->GetOrientation()),
-                handler->GetSession()->GetPlayer()->GetZoneId(),
+            Player::SavePositionInDB(WorldLocation(_player->GetMapId(),
+                _player->GetPositionX(),
+                _player->GetPositionY(),
+                _player->GetPositionZ(),
+                _player->GetOrientation()),
+                _player->GetZoneId(),
                 targetGuid, dummy);
         }