refactor(Core/ChatHandler): Cleanup retriving session and checking fo… (#19585)

* refactor(Core/ChatHandler): Cleanup retriving session and checking for valid session

* redundant

* make it clear what a "valid" session is

* HasValidSession -> HasSession

* Differentiate from "valid" which in this case would mean the player is in world which isnt checked here

src/server/game/Chat/Chat.cpp

@@ -101,12 +101,6 @@ bool ChatHandler::HasLowerSecurityAccount(WorldSession* target, uint32 target_ac
 
 void ChatHandler::SendNotification(std::string_view str)
 {
-    if (!m_session)
-    {
-        LOG_ERROR("chat.chat", "ChatHandler::SendNotification sent without a session. Skipped.");
-        return;
-    }
-
     std::vector<std::string_view> lines = Acore::Tokenize(str, '\n', true);
     for (std::string_view line : lines)
     {
@@ -123,16 +117,11 @@ void ChatHandler::SendGMText(std::string_view str)
     if (AccountMgr::IsPlayerAccount(m_session->GetSecurity()))
         return;
 
-    // Player should be in world
-    Player* player = m_session->GetPlayer();
-    if (!player || !player->IsInWorld())
-        return;
-
     for (std::string_view line : lines)
     {
         WorldPacket data;
         ChatHandler::BuildChatPacket(data, CHAT_MSG_SYSTEM, LANG_UNIVERSAL, nullptr, nullptr, line);
-        player->SendDirectMessage(&data);
+        m_session->SendPacket(&data);
     }
 }
 
@@ -140,15 +129,11 @@ void ChatHandler::SendWorldText(std::string_view str)
 {
     std::vector<std::string_view> lines = Acore::Tokenize(str, '\n', true);
 
-    Player* player = m_session->GetPlayer();
-    if (!player || !player->IsInWorld())
-        return;
-
     for (std::string_view line : lines)
     {
         WorldPacket data;
         ChatHandler::BuildChatPacket(data, CHAT_MSG_SYSTEM, LANG_UNIVERSAL, nullptr, nullptr, line);
-        player->SendDirectMessage(&data);
+        m_session->SendPacket(&data);
     }
 }
 
@@ -157,9 +142,6 @@ void ChatHandler::SendWorldTextOptional(std::string_view str, uint32 flag)
     std::vector<std::string_view> lines = Acore::Tokenize(str, '\n', true);
 
     Player* player = m_session->GetPlayer();
-    if (!player || !player->IsInWorld())
-        return;
-
     if (sWorld->getBoolConfig(CONFIG_PLAYER_SETTINGS_ENABLED))
         if (player->GetPlayerSetting(AzerothcorePSSource, SETTING_ANNOUNCER_FLAGS).HasFlag(flag))
             return;
@@ -174,12 +156,6 @@ void ChatHandler::SendWorldTextOptional(std::string_view str, uint32 flag)
 
 void ChatHandler::SendSysMessage(std::string_view str, bool escapeCharacters)
 {
-    if (!m_session)
-    {
-        LOG_ERROR("chat.chat", "ChatHandler::SendSysMessage sent without a session. Skipped.");
-        return;
-    }
-
     std::string msg{ str };
 
     // Replace every "|" with "||" in msg
@@ -454,6 +430,23 @@ Player* ChatHandler::getSelectedPlayerOrSelf() const
     return targetPlayer;
 }
 
+bool ChatHandler::HasSession()
+{
+    if (!m_session)
+        return false;
+
+    return true;
+}
+
+void ChatHandler::DoForAllValidSessions(std::function<void(Player*)> exec)
+{
+    SessionMap::const_iterator itr;
+    for (itr = sWorld->GetAllSessions().begin(); itr != sWorld->GetAllSessions().end(); ++itr)
+        if (Player* player = itr->second->GetPlayer())
+            if (player->IsInWorld())
+                exec(player);
+}
+
 char* ChatHandler::extractKeyFromLink(char* text, char const* linkType, char** something1)
 {
     // skip empty

src/server/game/Chat/Chat.h

@@ -55,12 +55,14 @@ public:
     template<typename... Args>
     void SendNotification(uint32 strId, Args&&... args)
     {
-        SendNotification(Acore::StringFormatFmt(GetAcoreString(strId), std::forward<Args>(args)...));
+        if (HasSession())
+            SendNotification(Acore::StringFormatFmt(GetAcoreString(strId), std::forward<Args>(args)...));
     }
     template<typename... Args>
     void SendNotification(char const* fmt, Args&&... args)
     {
-        SendNotification(Acore::StringFormatFmt(fmt, std::forward<Args>(args)...));
+        if (HasSession())
+            SendNotification(Acore::StringFormatFmt(fmt, std::forward<Args>(args)...));
     }
 
     void SendGMText(std::string_view str);
@@ -68,21 +70,21 @@ public:
     void SendGMText(uint32 strId, Args&&... args)
     {
         // GMText should be sent to all sessions
-        SessionMap::const_iterator itr;
+        DoForAllValidSessions([&](Player* player)
-        for (itr = sWorld->GetAllSessions().begin(); itr != sWorld->GetAllSessions().end(); ++itr)
-        {
-            Player* player = itr->second->GetPlayer();
-            if (player && player->IsInWorld())
             {
                 m_session = player->GetSession();
                 SendGMText(Acore::StringFormatFmt(GetAcoreString(strId), std::forward<Args>(args)...));
-            }
+            });
-        }
     }
     template<typename... Args>
     void SendGMText(char const* fmt, Args&&... args)
     {
-        SendGMText(Acore::StringFormatFmt(fmt, std::forward<Args>(args)...));
+        // GMText should be sent to all sessions
+        DoForAllValidSessions([&](Player* player)
+            {
+                m_session = player->GetSession();
+                SendGMText(Acore::StringFormatFmt(fmt, std::forward<Args>(args)...));
+            });
     }
 
     void SendWorldText(std::string_view str);
@@ -90,31 +92,21 @@ public:
     void SendWorldText(uint32 strId, Args&&... args)
     {
         // WorldText should be sent to all sessions
-        SessionMap::const_iterator itr;
+        DoForAllValidSessions([&](Player* player)
-        for (itr = sWorld->GetAllSessions().begin(); itr != sWorld->GetAllSessions().end(); ++itr)
-        {
-            Player* player = itr->second->GetPlayer();
-            if (player && player->IsInWorld())
             {
                 m_session = player->GetSession();
                 SendWorldText(Acore::StringFormatFmt(GetAcoreString(strId), std::forward<Args>(args)...));
-            }
+            });
-        }
     }
     template<typename... Args>
     void SendWorldText(char const* fmt, Args&&... args)
     {
-        // WorldTextOptional should be sent to all sessions
+        // WorldText should be sent to all sessions
-        SessionMap::const_iterator itr;
+        DoForAllValidSessions([&](Player* player)
-        for (itr = sWorld->GetAllSessions().begin(); itr != sWorld->GetAllSessions().end(); ++itr)
-        {
-            Player* player = itr->second->GetPlayer();
-            if (player && player->IsInWorld())
             {
                 m_session = player->GetSession();
                 SendWorldText(Acore::StringFormatFmt(fmt, std::forward<Args>(args)...));
-            }
+            });
-        }
     }
 
     void SendWorldTextOptional(std::string_view str, uint32 flag);
@@ -122,31 +114,21 @@ public:
     void SendWorldTextOptional(uint32 strId, uint32 flag, Args&&... args)
     {
         // WorldTextOptional should be sent to all sessions
-        SessionMap::const_iterator itr;
+        DoForAllValidSessions([&](Player* player)
-        for (itr = sWorld->GetAllSessions().begin(); itr != sWorld->GetAllSessions().end(); ++itr)
-        {
-            Player* player = itr->second->GetPlayer();
-            if (player && player->IsInWorld())
             {
                 m_session = player->GetSession();
                 SendWorldTextOptional(Acore::StringFormatFmt(GetAcoreString(strId), std::forward<Args>(args)...), flag);
-            }
+            });
-        }
     }
     template<typename... Args>
     void SendWorldTextOptional(char const* fmt, uint32 flag, Args&&... args)
     {
         // WorldTextOptional should be sent to all sessions
-        SessionMap::const_iterator itr;
+        DoForAllValidSessions([&](Player* player)
-        for (itr = sWorld->GetAllSessions().begin(); itr != sWorld->GetAllSessions().end(); ++itr)
-        {
-            Player* player = itr->second->GetPlayer();
-            if (player && player->IsInWorld())
             {
                 m_session = player->GetSession();
                 SendWorldTextOptional(Acore::StringFormatFmt(fmt, std::forward<Args>(args)...), flag);
-            }
+            });
-        }
     }
 
     // function with different implementation for chat/console
@@ -159,13 +141,15 @@ public:
     template<typename... Args>
     void PSendSysMessage(char const* fmt, Args&&... args)
     {
-        SendSysMessage(Acore::StringFormatFmt(fmt, std::forward<Args>(args)...));
+        if (HasSession())
+            SendSysMessage(Acore::StringFormatFmt(fmt, std::forward<Args>(args)...));
     }
 
     template<typename... Args>
     void PSendSysMessage(uint32 entry, Args&&... args)
     {
-        SendSysMessage(PGetParseString(entry, std::forward<Args>(args)...));
+        if (HasSession())
+            SendSysMessage(PGetParseString(entry, std::forward<Args>(args)...));
     }
 
     template<typename... Args>
@@ -214,6 +198,11 @@ public:
     // Returns either the selected player or self if there is no selected player
     Player* getSelectedPlayerOrSelf() const;
 
+    bool HasSession();
+    // Do whatever you want to all the players with a valid session [including GameMasters], i.e.: param exec = [&](Player* p) { p->Whatever(); }
+    // A "valid" session requires player->IsInWorld() to be true
+    void DoForAllValidSessions(std::function<void(Player*)> exec);
+
     char* extractKeyFromLink(char* text, char const* linkType, char** something1 = nullptr);
     char* extractKeyFromLink(char* text, char const* const* linkTypes, int* found_idx, char** something1 = nullptr);
     char* extractQuotedArg(char* args);