replace usages of ForEachClientWeak with ForEachClient

src/TConsole.cpp

@@ -298,39 +298,37 @@ void TConsole::Command_Debug(const std::string&, const std::vector<std::string>&
               connection and do not necessarily reflect the *current* data rate
               of that client.
 )"));
-    mLuaEngine->Server().ForEachClientWeak([&](std::weak_ptr<TClient> Client) -> bool {
+    mLuaEngine->Server().ForEachClient([&](const auto& Client) {
-        if (!Client.expired()) {
+        std::string State = "";
-            auto Locked = Client.lock();
+        if (Client->IsSyncing()) {
-            std::string State = "";
+            State += "Syncing";
-            if (Locked->IsSyncing()) {
+        }
-                State += "Syncing";
+        if (Client->IsSynced()) {
+            if (!State.empty()) {
+                State += " & ";
             }
-            if (Locked->IsSynced()) {
+            State += "Synced";
-                if (!State.empty()) {
+        }
-                    State += " & ";
+        if (Client->IsConnected()) {
-                }
+            if (!State.empty()) {
-                State += "Synced";
+                State += " & ";
             }
-            if (Locked->IsConnected()) {
+            State += "Connected";
-                if (!State.empty()) {
+        }
-                    State += " & ";
+        if (Client->IsDisconnected()) {
-                }
+            if (!State.empty()) {
-                State += "Connected";
+                State += " & ";
             }
-            if (Locked->IsDisconnected()) {
+            State += "Disconnected";
-                if (!State.empty()) {
+        }
-                    State += " & ";
+        auto Now = TimeType::now();
-                }
+        auto Seconds = std::chrono::duration_cast<std::chrono::seconds>(Now - Client->ConnectionTime);
-                State += "Disconnected";
+        std::string ConnectedSince = fmt::format("{:%Y/%m/%d %H:%M:%S}, {:%H:%M:%S} ago ({} seconds)",
-            }
+            fmt::localtime(TimeType::to_time_t(Client->ConnectionTime)),
-            auto Now = TimeType::now();
+            Seconds,
-            auto Seconds = std::chrono::duration_cast<std::chrono::seconds>(Now - Locked->ConnectionTime);
+            Seconds.count());
-            std::string ConnectedSince = fmt::format("{:%Y/%m/%d %H:%M:%S}, {:%H:%M:%S} ago ({} seconds)",
+        Application::Console().WriteRaw(fmt::format(
-                fmt::localtime(TimeType::to_time_t(Locked->ConnectionTime)),
+            R"(        {} ('{}'):
-                Seconds,
-                Seconds.count());
-            Application::Console().WriteRaw(fmt::format(
-                R"(        {} ('{}'):
             Roles:              {}
             Cars:               {}
             Is guest:           {}
@@ -347,27 +345,23 @@ void TConsole::Command_Debug(const std::string&, const std::vector<std::string>&
             Connected since:    {}
             Average send:       {}/s
             Average receive:    {}/s)",
-                Locked->GetID(), Locked->GetName(),
+            Client->GetID(), Client->GetName(),
-                Locked->GetRoles(),
+            Client->GetRoles(),
-                Locked->GetCarCount(),
+            Client->GetCarCount(),
-                Locked->IsGuest() ? "yes" : "no",
+            Client->IsGuest() ? "yes" : "no",
-                Locked->GetUnicycleID() == -1 ? "no" : "yes",
+            Client->GetUnicycleID() == -1 ? "no" : "yes",
-                Locked->GetTCPSock().remote_endpoint().address() == ip::address::from_string("0.0.0.0") ? "not connected" : "connected", Locked->GetTCPSock().remote_endpoint().port(),
+            Client->GetTCPSock().remote_endpoint().address() == ip::address::from_string("0.0.0.0") ? "not connected" : "connected", Client->GetTCPSock().remote_endpoint().port(),
-                Locked->GetUDPAddr().address() == ip::address::from_string("0.0.0.0") ? "NOT connected" : "connected", Locked->GetUDPAddr().port(),
+            Client->GetUDPAddr().address() == ip::address::from_string("0.0.0.0") ? "NOT connected" : "connected", Client->GetUDPAddr().port(),
-                ToHumanReadableSize(Locked->TcpSent),
+            ToHumanReadableSize(Client->TcpSent),
-                ToHumanReadableSize(Locked->TcpReceived),
+            ToHumanReadableSize(Client->TcpReceived),
-                ToHumanReadableSize(Locked->UdpSent), Locked->UdpPacketsSent,
+            ToHumanReadableSize(Client->UdpSent), Client->UdpPacketsSent,
-                ToHumanReadableSize(Locked->UdpReceived), Locked->UdpPacketsReceived,
+            ToHumanReadableSize(Client->UdpReceived), Client->UdpPacketsReceived,
-                State.empty() ? "None (likely pre-sync)" : State,
+            State.empty() ? "None (likely pre-sync)" : State,
-                Locked->MissedPacketQueueSize(),
+            Client->MissedPacketQueueSize(),
-                Locked->SecondsSinceLastPing(),
+            Client->SecondsSinceLastPing(),
-                ConnectedSince,
+            ConnectedSince,
-                ToHumanReadableSize((Locked->TcpSent + Locked->UdpSent) / Seconds.count()),
+            ToHumanReadableSize((Client->TcpSent + Client->UdpSent) / Seconds.count()),
-                ToHumanReadableSize((Locked->TcpReceived + Locked->UdpReceived) / Seconds.count())));
+            ToHumanReadableSize((Client->TcpReceived + Client->UdpReceived) / Seconds.count())));
-        } else {
-            Application::Console().WriteRaw(fmt::format(R"(        <expired client>)"));
-        }
-        return true;
     });
 }
 
@@ -390,16 +384,13 @@ void TConsole::Command_Kick(const std::string&, const std::vector<std::string>&
     beammp_trace("attempt to kick '" + Name + "' for '" + Reason + "'");
     bool Kicked = false;
 
-    mLuaEngine->Server().ForEachClientWeak([&](std::weak_ptr<TClient> Client) -> bool {
+    mLuaEngine->Server().ForEachClient([&](const auto& Client) -> IterationDecision {
-        auto Locked = Client.lock();
+        if (StringStartsWithLower(Client->GetName(), Name)) {
-        if (Locked) {
+            mLuaEngine->Network().ClientKick(*Client, Reason);
-            if (StringStartsWithLower(Locked->GetName(), Name)) {
+            Kicked = true;
-                mLuaEngine->Network().ClientKick(*Locked, Reason);
+            return Break;
-                Kicked = true;
-                return false;
-            }
         }
-        return true;
+        return Continue;
     });
     if (!Kicked) {
         Application::Console().WriteRaw("Error: No player with name matching '" + Name + "' was found.");
@@ -552,14 +543,10 @@ void TConsole::Command_List(const std::string&, const std::vector<std::string>&
     } else {
         std::stringstream ss;
         ss << std::left << std::setw(25) << "Name" << std::setw(6) << "ID" << std::setw(6) << "Cars" << std::endl;
-        mLuaEngine->Server().ForEachClientWeak([&](std::weak_ptr<TClient> Client) -> bool {
+        mLuaEngine->Server().ForEachClient([&](const auto& Client) {
-            if (!Client.expired()) {
+            ss << std::left << std::setw(25) << Client->GetName()
-                auto locked = Client.lock();
+               << std::setw(6) << Client->GetID()
-                ss << std::left << std::setw(25) << locked->GetName()
+               << std::setw(6) << Client->GetCarCount() << "\n";
-                   << std::setw(6) << locked->GetID()
-                   << std::setw(6) << locked->GetCarCount() << "\n";
-            }
-            return true;
         });
         auto Str = ss.str();
         Application::Console().WriteRaw(Str.substr(0, Str.size() - 1));
@@ -579,20 +566,16 @@ void TConsole::Command_Status(const std::string&, const std::vector<std::string>
     size_t SyncingCount = 0;
     size_t MissedPacketQueueSum = 0;
     int LargestSecondsSinceLastPing = 0;
-    mLuaEngine->Server().ForEachClientWeak([&](std::weak_ptr<TClient> Client) -> bool {
+    mLuaEngine->Server().ForEachClient([&](const auto& Client) {
-        if (!Client.expired()) {
+        CarCount += Client->GetCarCount();
-            auto Locked = Client.lock();
+        ConnectedCount += Client->IsConnected() ? 1 : 0;
-            CarCount += Locked->GetCarCount();
+        GuestCount += Client->IsGuest() ? 1 : 0;
-            ConnectedCount += Locked->IsConnected() ? 1 : 0;
+        SyncedCount += Client->IsSynced() ? 1 : 0;
-            GuestCount += Locked->IsGuest() ? 1 : 0;
+        SyncingCount += Client->IsSyncing() ? 1 : 0;
-            SyncedCount += Locked->IsSynced() ? 1 : 0;
+        MissedPacketQueueSum += Client->MissedPacketQueueSize();
-            SyncingCount += Locked->IsSyncing() ? 1 : 0;
+        if (Client->SecondsSinceLastPing() < LargestSecondsSinceLastPing) {
-            MissedPacketQueueSum += Locked->MissedPacketQueueSize();
+            LargestSecondsSinceLastPing = Client->SecondsSinceLastPing();
-            if (Locked->SecondsSinceLastPing() < LargestSecondsSinceLastPing) {
-                LargestSecondsSinceLastPing = Locked->SecondsSinceLastPing();
-            }
         }
-        return true;
     });
 
     size_t SystemsStarting = 0;
@@ -680,14 +663,10 @@ void TConsole::Autocomplete_Lua(const std::string& stub, std::vector<std::string
 void TConsole::Autocomplete_Kick(const std::string& stub, std::vector<std::string>& suggestions) {
     std::string stub_lower = boost::algorithm::to_lower_copy(stub);
 
-    mLuaEngine->Server().ForEachClientWeak([&](std::weak_ptr<TClient> Client) -> bool {
+    mLuaEngine->Server().ForEachClient([&](const auto& Client) {
-        auto Locked = Client.lock();
+        if (StringStartsWithLower(Client->GetName(), stub_lower)) {
-        if (Locked) {
+            suggestions.push_back("kick " + Client->GetName());
-            if (StringStartsWithLower(Locked->GetName(), stub_lower)) {
-                suggestions.push_back("kick " + Locked->GetName());
-            }
         }
-        return true;
     });
 }
 

src/THeartbeatThread.cpp

@@ -167,12 +167,8 @@ THeartbeatThread::THeartbeatThread(TResourceManager& ResourceManager, TServer& S
 }
 std::string THeartbeatThread::GetPlayers() {
     std::string Return;
-    mServer.ForEachClientWeak([&](const std::weak_ptr<TClient>& ClientPtr) -> bool {
+    mServer.ForEachClient([&](const auto& Client) {
-        ReadLock Lock(mServer.GetClientMutex());
+        Return += Client->GetName() + ";";
-        if (!ClientPtr.expired()) {
-            Return += ClientPtr.lock()->GetName() + ";";
-        }
-        return true;
     });
     return Return;
 }

src/TNetwork.cpp

@@ -1,6 +1,7 @@
 #include "TNetwork.h"
 #include "Client.h"
 #include "Common.h"
+#include "IterationDecision.h"
 #include "LuaAPI.h"
 #include "TLuaEngine.h"
 #include "nlohmann/json.hpp"
@@ -52,11 +53,8 @@ TNetwork::TNetwork(TServer& Server, TPPSMonitor& PPSMonitor, TResourceManager& R
         auto Futures = LuaAPI::MP::Engine->TriggerEvent("onShutdown", "");
         TLuaEngine::WaitForAll(Futures, std::chrono::seconds(60));
         beammp_debug("Kicking all players due to shutdown");
-        Server.ForEachClientWeak([&](std::weak_ptr<TClient> client) -> bool {
+        Server.ForEachClient([&](const auto& Client) {
-            if (!client.expired()) {
+            ClientKick(*Client, "Server shutdown");
-                ClientKick(*client.lock(), "Server shutdown");
-            }
-            return true;
         });
     });
 }
@@ -212,15 +210,12 @@ void TNetwork::HandleDownload(TConnection&& Conn) {
         return;
     }
     auto ID = uint8_t(D);
-    mServer.ForEachClientWeak([&](const std::weak_ptr<TClient>& ClientPtr) -> bool {
+    mServer.ForEachClient([&](const auto& Client) -> IterationDecision {
-        ReadLock Lock(mServer.GetClientMutex());
+        if (Client->GetID() == ID) {
-        if (!ClientPtr.expired()) {
+            Client->SetDownSock(std::move(Conn.Socket));
-            auto c = ClientPtr.lock();
+            return Break;
-            if (c->GetID() == ID) {
-                c->SetDownSock(std::move(Conn.Socket));
-            }
         }
-        return true;
+        return Continue;
     });
 }
 
@@ -297,21 +292,13 @@ std::shared_ptr<TClient> TNetwork::Authentication(TConnection&& RawConnection) {
     }
 
     beammp_debug("Name -> " + Client->GetName() + ", Guest -> " + std::to_string(Client->IsGuest()) + ", Roles -> " + Client->GetRoles());
-    mServer.ForEachClientWeak([&](const std::weak_ptr<TClient>& ClientPtr) -> bool {
+    mServer.ForEachClient([&](const auto& ThisClient) -> IterationDecision {
-        std::shared_ptr<TClient> Cl;
+        // FIXME: This doesn't respect forum ID, and it should :^)
-        {
+        if (ThisClient->GetName() == Client->GetName() && ThisClient->IsGuest() == Client->IsGuest()) {
-            ReadLock Lock(mServer.GetClientMutex());
+            ThisClient->Disconnect("Stale Client (replaced by new client)");
-            if (!ClientPtr.expired()) {
+            return Break;
-                Cl = ClientPtr.lock();
-            } else
-                return true;
         }
-        if (Cl->GetName() == Client->GetName() && Cl->IsGuest() == Client->IsGuest()) {
+        return Continue;
-            Cl->Disconnect("Stale Client (not a real player)");
-            return false;
-        }
-
-        return true;
     });
 
     auto Futures = LuaAPI::MP::Engine->TriggerEvent("onPlayerAuth", "", Client->GetName(), Client->GetRoles(), Client->IsGuest(), Client->GetIdentifiers());
@@ -533,13 +520,8 @@ void TNetwork::TCPClient(const std::weak_ptr<TClient>& c) {
 
 void TNetwork::UpdatePlayer(TClient& Client) {
     std::string Packet = ("Ss") + std::to_string(mServer.ClientCount()) + "/" + std::to_string(Application::GetSettingInt(StrMaxPlayers)) + ":";
-    mServer.ForEachClientWeak([&](const std::weak_ptr<TClient>& ClientPtr) -> bool {
+    mServer.ForEachClient([&](const auto& ThisClient) {
-        ReadLock Lock(mServer.GetClientMutex());
+        Packet += ThisClient->GetName() + ",";
-        if (!ClientPtr.expired()) {
-            auto c = ClientPtr.lock();
-            Packet += c->GetName() + ",";
-        }
-        return true;
     });
     Packet = Packet.substr(0, Packet.length() - 1);
     Client.EnqueuePacket(StringToVector(Packet));
@@ -585,16 +567,11 @@ int TNetwork::OpenID() {
     bool found;
     do {
         found = true;
-        mServer.ForEachClientWeak([&](const std::weak_ptr<TClient>& ClientPtr) -> bool {
+        mServer.ForEachClient([&](const auto& Client) {
-            ReadLock Lock(mServer.GetClientMutex());
+            if (Client->GetID() == ID) {
-            if (!ClientPtr.expired()) {
+                found = false;
-                auto c = ClientPtr.lock();
+                ID++;
-                if (c->GetID() == ID) {
-                    found = false;
-                    ID++;
-                }
             }
-            return true;
         });
     } while (!found);
     return ID;
@@ -855,32 +832,23 @@ bool TNetwork::SyncClient(const std::weak_ptr<TClient>& c) {
     LockedClient->SetIsSyncing(true);
     bool Return = false;
     bool res = true;
-    mServer.ForEachClientWeak([&](const std::weak_ptr<TClient>& ClientPtr) -> bool {
+    mServer.ForEachClient([&](const auto& Client) -> IterationDecision {
-        std::shared_ptr<TClient> client;
-        {
-            ReadLock Lock(mServer.GetClientMutex());
-            if (!ClientPtr.expired()) {
-                client = ClientPtr.lock();
-            } else
-                return true;
-        }
         TClient::TSetOfVehicleData VehicleData;
         { // Vehicle Data Lock Scope
-            auto LockedData = client->GetAllCars();
+            auto LockedData = Client->GetAllCars();
             VehicleData = *LockedData.VehicleData;
         } // End Vehicle Data Lock Scope
-        if (client != LockedClient) {
+        if (Client != LockedClient) {
             for (auto& v : VehicleData) {
                 if (LockedClient->IsDisconnected()) {
                     Return = true;
                     res = false;
-                    return false;
+                    return Break;
                 }
                 res = Respond(*LockedClient, StringToVector(v.Data()), true, true);
             }
         }
-
+        return Continue;
-        return true;
     });
     LockedClient->SetIsSyncing(false);
     if (Return) {
@@ -896,16 +864,7 @@ void TNetwork::SendToAll(TClient* c, const std::vector<uint8_t>& Data, bool Self
         beammp_assert(c);
     char C = Data.at(0);
     bool ret = true;
-    mServer.ForEachClientWeak([&](std::weak_ptr<TClient> ClientPtr) -> bool {
+    mServer.ForEachClient([&](const auto& Client) {
-        std::shared_ptr<TClient> Client;
-        try {
-            ReadLock Lock(mServer.GetClientMutex());
-            Client = ClientPtr.lock();
-        } catch (const std::exception&) {
-            // continue
-            beammp_warn("Client expired, shouldn't happen - if a client disconnected recently, you can ignore this");
-            return true;
-        }
         if (Self || Client.get() != c) {
             if (Client->IsSynced() || Client->IsSyncing()) {
                 if (Rel || C == 'W' || C == 'Y' || C == 'V' || C == 'E') {
@@ -927,7 +886,6 @@ void TNetwork::SendToAll(TClient* c, const std::vector<uint8_t>& Data, bool Self
                 }
             }
         }
-        return true;
     });
     if (!ret) {
         // TODO: handle

src/TPPSMonitor.cpp

@@ -33,26 +33,16 @@ void TPPSMonitor::operator()() {
             Application::SetPPS("-");
             continue;
         }
-        mServer.ForEachClientWeak([&](const std::weak_ptr<TClient>& ClientPtr) -> bool {
+        mServer.ForEachClient([&](const auto& Client) {
-            std::shared_ptr<TClient> c;
+            if (Client->GetCarCount() > 0) {
-            {
-                ReadLock Lock(mServer.GetClientMutex());
-                if (!ClientPtr.expired()) {
-                    c = ClientPtr.lock();
-                } else
-                    return true;
-            }
-            if (c->GetCarCount() > 0) {
                 C++;
-                V += c->GetCarCount();
+                V += Client->GetCarCount();
             }
             // kick on "no ping"
-            if (c->SecondsSinceLastPing() > (20 * 60)) {
+            if (Client->SecondsSinceLastPing() > (20 * 60)) {
-                beammp_debug("client " + std::string("(") + std::to_string(c->GetID()) + ")" + c->GetName() + " timing out: " + std::to_string(c->SecondsSinceLastPing()) + ", pps: " + Application::PPS());
+                beammp_debugf("Client {} ({}) timing out: {}s since last contact", Client->GetName(), Client->GetID(), Client->SecondsSinceLastPing());
-                TimedOutClients.push_back(c);
+                TimedOutClients.push_back(Client);
             }
-
-            return true;
         });
         for (auto& ClientToKick : TimedOutClients) {
             Network().ClientKick(*ClientToKick, "Timeout (no ping for way too long)");