rework GetClient to use new ForEachClient, return shared_ptr

this simplifies a lot of functions, and removes a lot of potential
errors when using the function. You now only check whether the return
value is null, and if it's not, you have a valid, reference-counted,
client pointer.
This commit is contained in:
Lion Kortlepel
2022-11-13 13:36:04 +01:00
parent 844b64f5d9
commit 2f85c708c5
5 changed files with 51 additions and 63 deletions

View File

@@ -129,4 +129,5 @@ private:
std::chrono::time_point<TimeType> mLastPingTime;
};
std::optional<std::weak_ptr<TClient>> GetClient(class TServer& Server, int ID);
// Returns a valid client, or nullptr if no such client exists
std::shared_ptr<TClient> GetClient(class TServer& Server, int ID);

View File

@@ -136,20 +136,14 @@ int TClient::SecondsSinceLastPing() {
return int(seconds);
}
std::optional<std::weak_ptr<TClient>> GetClient(TServer& Server, int ID) {
std::optional<std::weak_ptr<TClient>> MaybeClient { std::nullopt };
Server.ForEachClientWeak([&](std::weak_ptr<TClient> CPtr) -> bool {
ReadLock Lock(Server.GetClientMutex());
try {
auto C = CPtr.lock();
if (C->GetID() == ID) {
MaybeClient = CPtr;
return false;
}
} catch (const std::exception&) {
// ignore
std::shared_ptr<TClient> GetClient(TServer& Server, int ID) {
std::shared_ptr<TClient> Result {};
Server.ForEachClient([&](const auto& Client) {
if (Client->GetID() == ID) {
Result = Client;
return Break;
}
return true;
return Continue;
});
return MaybeClient;
return Result;
}

View File

@@ -119,15 +119,14 @@ static inline std::pair<bool, std::string> InternalTriggerClientEvent(int Player
LuaAPI::MP::Engine->Network().SendToAll(nullptr, StringToVector(Packet), true, true);
return { true, "" };
} else {
auto MaybeClient = GetClient(LuaAPI::MP::Engine->Server(), PlayerID);
if (!MaybeClient || MaybeClient.value().expired()) {
auto Client = GetClient(LuaAPI::MP::Engine->Server(), PlayerID);
if (!Client) {
beammp_lua_errorf("TriggerClientEvent invalid Player ID '{}'", PlayerID);
return { false, "Invalid Player ID" };
}
auto c = MaybeClient.value().lock();
if (!LuaAPI::MP::Engine->Network().Respond(*c, StringToVector(Packet), true)) {
if (!LuaAPI::MP::Engine->Network().Respond(*Client, StringToVector(Packet), true)) {
beammp_lua_errorf("Respond failed, dropping client {}", PlayerID);
LuaAPI::MP::Engine->Network().ClientKick(*c, "Disconnected after failing to receive packets");
LuaAPI::MP::Engine->Network().ClientKick(*Client, "Disconnected after failing to receive packets");
return { false, "Respond failed, dropping client" };
}
return { true, "" };
@@ -140,13 +139,12 @@ std::pair<bool, std::string> LuaAPI::MP::TriggerClientEvent(int PlayerID, const
}
std::pair<bool, std::string> LuaAPI::MP::DropPlayer(int ID, std::optional<std::string> MaybeReason) {
auto MaybeClient = GetClient(Engine->Server(), ID);
if (!MaybeClient || MaybeClient.value().expired()) {
auto Client = GetClient(Engine->Server(), ID);
if (!Client) {
beammp_lua_errorf("Tried to drop client with id {}, who doesn't exist", ID);
return { false, "Player does not exist" };
}
auto c = MaybeClient.value().lock();
LuaAPI::MP::Engine->Network().ClientKick(*c, MaybeReason.value_or("No reason"));
LuaAPI::MP::Engine->Network().ClientKick(*Client, MaybeReason.value_or("No reason"));
return { true, "" };
}
@@ -158,16 +156,15 @@ std::pair<bool, std::string> LuaAPI::MP::SendChatMessage(int ID, const std::stri
Engine->Network().SendToAll(nullptr, StringToVector(Packet), true, true);
Result.first = true;
} else {
auto MaybeClient = GetClient(Engine->Server(), ID);
if (MaybeClient && !MaybeClient.value().expired()) {
auto c = MaybeClient.value().lock();
if (!c->IsSynced()) {
auto Client = GetClient(Engine->Server(), ID);
if (Client) {
if (!Client->IsSynced()) {
Result.first = false;
Result.second = "Player still syncing data";
return Result;
}
LogChatMessage("<Server> (to \"" + c->GetName() + "\")", -1, Message);
if (!Engine->Network().Respond(*c, StringToVector(Packet), true)) {
LogChatMessage("<Server> (to \"" + Client->GetName() + "\")", -1, Message);
if (!Engine->Network().Respond(*Client, StringToVector(Packet), true)) {
beammp_errorf("Failed to send chat message back to sender (id {}) - did the sender disconnect?", ID);
// TODO: should we return an error here?
}
@@ -184,18 +181,17 @@ std::pair<bool, std::string> LuaAPI::MP::SendChatMessage(int ID, const std::stri
std::pair<bool, std::string> LuaAPI::MP::RemoveVehicle(int PID, int VID) {
std::pair<bool, std::string> Result;
auto MaybeClient = GetClient(Engine->Server(), PID);
if (!MaybeClient || MaybeClient.value().expired()) {
auto Client = GetClient(Engine->Server(), PID);
if (!Client) {
beammp_lua_error("RemoveVehicle invalid Player ID");
Result.first = false;
Result.second = "Invalid Player ID";
return Result;
}
auto c = MaybeClient.value().lock();
if (!c->GetCarData(VID).empty()) {
if (!Client->GetCarData(VID).empty()) {
std::string Destroy = "Od:" + std::to_string(PID) + "-" + std::to_string(VID);
Engine->Network().SendToAll(nullptr, StringToVector(Destroy), true, true);
c->DeleteCar(VID);
Client->DeleteCar(VID);
Result.first = true;
} else {
Result.first = false;
@@ -273,18 +269,18 @@ void LuaAPI::MP::Sleep(size_t Ms) {
}
bool LuaAPI::MP::IsPlayerConnected(int ID) {
auto MaybeClient = GetClient(Engine->Server(), ID);
if (MaybeClient && !MaybeClient.value().expired()) {
return MaybeClient.value().lock()->IsConnected();
auto Client = GetClient(Engine->Server(), ID);
if (Client) {
return Client->IsConnected();
} else {
return false;
}
}
bool LuaAPI::MP::IsPlayerGuest(int ID) {
auto MaybeClient = GetClient(Engine->Server(), ID);
if (MaybeClient && !MaybeClient.value().expired()) {
return MaybeClient.value().lock()->IsGuest();
auto Client = GetClient(Engine->Server(), ID);
if (Client) {
return Client->IsGuest();
} else {
return false;
}

View File

@@ -528,9 +528,9 @@ sol::table TLuaEngine::StateThreadData::Lua_TriggerLocalEvent(const std::string&
}
sol::table TLuaEngine::StateThreadData::Lua_GetPlayerIdentifiers(int ID) {
auto MaybeClient = GetClient(mEngine->Server(), ID);
if (MaybeClient && !MaybeClient.value().expired()) {
auto IDs = MaybeClient.value().lock()->GetIdentifiers();
auto Client = GetClient(mEngine->Server(), ID);
if (Client) {
auto IDs = Client->GetIdentifiers();
if (IDs.empty()) {
return sol::lua_nil;
}
@@ -591,18 +591,17 @@ sol::table TLuaEngine::StateThreadData::Lua_FS_ListDirectories(const std::string
}
std::string TLuaEngine::StateThreadData::Lua_GetPlayerName(int ID) {
auto MaybeClient = GetClient(mEngine->Server(), ID);
if (MaybeClient && !MaybeClient.value().expired()) {
return MaybeClient.value().lock()->GetName();
auto Client = GetClient(mEngine->Server(), ID);
if (Client) {
return Client->GetName();
} else {
return "";
}
}
sol::table TLuaEngine::StateThreadData::Lua_GetPlayerVehicles(int ID) {
auto MaybeClient = GetClient(mEngine->Server(), ID);
if (MaybeClient && !MaybeClient.value().expired()) {
auto Client = MaybeClient.value().lock();
auto Client = GetClient(mEngine->Server(), ID);
if (Client) {
TClient::TSetOfVehicleData VehicleData;
{ // Vehicle Data Lock Scope
auto LockedData = Client->GetAllCars();
@@ -623,9 +622,8 @@ sol::table TLuaEngine::StateThreadData::Lua_GetPlayerVehicles(int ID) {
std::pair<sol::table, std::string> TLuaEngine::StateThreadData::Lua_GetPositionRaw(int PID, int VID) {
std::pair<sol::table, std::string> Result;
auto MaybeClient = GetClient(mEngine->Server(), PID);
if (MaybeClient && !MaybeClient.value().expired()) {
auto Client = MaybeClient.value().lock();
auto Client = GetClient(mEngine->Server(), PID);
if (Client) {
std::string VehiclePos = Client->GetCarPositionRaw(VID);
if (VehiclePos.empty()) {

View File

@@ -97,22 +97,21 @@ void TNetwork::UDPServerMain() {
// to represent packets. This would mean that player 0 would
// cause empty packets.
uint8_t ID = uint8_t(Data.at(0)) - 1;
auto MaybeClient = GetClient(mServer, ID);
if (MaybeClient) {
auto Client = GetClient(mServer, ID);
if (Client) {
try {
auto Locked = MaybeClient.value().lock();
if (Locked->IsConnected() && Locked->GetUDPAddr() != client) {
beammp_debugf("Client at {}:{} tried to send UDP for client {}", client.address().to_string(), client.port(), Locked->GetID());
if (Client->IsConnected() && Client->GetUDPAddr() != client) {
beammp_debugf("Client at {}:{} tried to send UDP for client {}", client.address().to_string(), client.port(), Client->GetID());
++Application::InvalidUdpPackets;
continue;
} else {
Locked->SetUDPAddr(client);
Locked->SetIsConnected(true);
Client->SetUDPAddr(client);
Client->SetIsConnected(true);
}
Locked->UdpReceived += Data.size();
++Locked->UdpPacketsReceived;
Client->UdpReceived += Data.size();
++Client->UdpPacketsReceived;
Data.erase(Data.begin() + 0, Data.begin() + 2);
TServer::GlobalParser(Locked, std::move(Data), *this);
TServer::GlobalParser(Client, std::move(Data), *this);
} catch (const std::exception&) {
++Application::InvalidUdpPackets;
}