Cleanup some expedition logging and formatting

Remove logging unsanitized input

Make unsigned comparison not compare < 0

Cleanup some FormatName and string usage. Some of these strings could
probably be moved instead

Remove unnecessary expedition lookup in a world message handler
This commit is contained in:
hg 2020-07-07 18:15:04 -04:00
parent d61879fd3c
commit cb4a117503
6 changed files with 32 additions and 47 deletions

View File

@ -95,7 +95,7 @@ void ExpeditionCache::LoadActiveExpeditions()
m_expeditions = ExpeditionDatabase::LoadExpeditions(); m_expeditions = ExpeditionDatabase::LoadExpeditions();
auto elapsed = benchmark.elapsed(); auto elapsed = benchmark.elapsed();
LogExpeditions("World caching [{}] expeditions took {}s", m_expeditions.size(), elapsed); LogExpeditions("World caching [{}] expeditions took [{}s]", m_expeditions.size(), elapsed);
} }
void ExpeditionCache::AddExpedition(uint32_t expedition_id) void ExpeditionCache::AddExpedition(uint32_t expedition_id)

View File

@ -9593,7 +9593,7 @@ Expedition* Client::CreateExpedition(DynamicZone& dz_instance, ExpeditionRequest
} }
Expedition* Client::CreateExpedition( Expedition* Client::CreateExpedition(
std::string zone_name, uint32 version, uint32 duration, std::string expedition_name, const std::string& zone_name, uint32 version, uint32 duration, const std::string& expedition_name,
uint32 min_players, uint32 max_players, bool disable_messages) uint32 min_players, uint32 max_players, bool disable_messages)
{ {
DynamicZone dz_instance{ zone_name, version, duration, DynamicZoneType::Expedition }; DynamicZone dz_instance{ zone_name, version, duration, DynamicZoneType::Expedition };
@ -9747,8 +9747,8 @@ void Client::SendExpeditionLockoutTimers()
// erases expired lockouts while building lockout timer list // erases expired lockouts while building lockout timer list
for (auto it = m_expedition_lockouts.begin(); it != m_expedition_lockouts.end();) for (auto it = m_expedition_lockouts.begin(); it != m_expedition_lockouts.end();)
{ {
auto seconds_remaining = it->GetSecondsRemaining(); uint32_t seconds_remaining = it->GetSecondsRemaining();
if (seconds_remaining <= 0) if (seconds_remaining == 0)
{ {
it = m_expedition_lockouts.erase(it); it = m_expedition_lockouts.erase(it);
} }
@ -9933,11 +9933,7 @@ void Client::MovePCDynamicZone(uint32 zone_id)
} }
else if (client_dzs.size() == 1) else if (client_dzs.size() == 1)
{ {
if (single_dz.GetInstanceID() == 0) if (single_dz.IsValid())
{
LogDynamicZones("Character [{}] has dz for zone [{}] with no instance id", CharacterID(), zone_id);
}
else
{ {
DynamicZoneLocation zonein = single_dz.GetZoneInLocation(); DynamicZoneLocation zonein = single_dz.GetZoneInLocation();
ZoneMode zone_mode = ZoneMode::ZoneToSafeCoords; ZoneMode zone_mode = ZoneMode::ZoneToSafeCoords;

View File

@ -1122,7 +1122,7 @@ public:
const std::string& expedition_name, const std::string& event_name, uint32_t duration, std::string uuid = {}); const std::string& expedition_name, const std::string& event_name, uint32_t duration, std::string uuid = {});
Expedition* CreateExpedition(DynamicZone& dz_instance, ExpeditionRequest& request); Expedition* CreateExpedition(DynamicZone& dz_instance, ExpeditionRequest& request);
Expedition* CreateExpedition( Expedition* CreateExpedition(
std::string zone_name, uint32 version, uint32 duration, std::string expedition_name, const std::string& zone_name, uint32 version, uint32 duration, const std::string& expedition_name,
uint32 min_players, uint32 max_players, bool disable_messages = false); uint32 min_players, uint32 max_players, bool disable_messages = false);
Expedition* GetExpedition() const; Expedition* GetExpedition() const;
uint32 GetExpeditionID() const { return m_expedition_id; } uint32 GetExpeditionID() const { return m_expedition_id; }

View File

@ -45,7 +45,7 @@ const int32_t Expedition::REPLAY_TIMER_ID = -1;
const int32_t Expedition::EVENT_TIMER_ID = 1; const int32_t Expedition::EVENT_TIMER_ID = 1;
Expedition::Expedition( Expedition::Expedition(
uint32_t id, const std::string& uuid, const DynamicZone& dynamic_zone, std::string expedition_name, uint32_t id, const std::string& uuid, const DynamicZone& dynamic_zone, const std::string& expedition_name,
const ExpeditionMember& leader, uint32_t min_players, uint32_t max_players const ExpeditionMember& leader, uint32_t min_players, uint32_t max_players
) : ) :
m_id(id), m_id(id),
@ -103,20 +103,18 @@ Expedition* Expedition::TryCreate(
{ {
dynamiczone.SaveToDatabase(); dynamiczone.SaveToDatabase();
ExpeditionMember leader{request.GetLeaderID(), request.GetLeaderName()};
auto expedition = std::unique_ptr<Expedition>(new Expedition( auto expedition = std::unique_ptr<Expedition>(new Expedition(
expedition_id, expedition_id,
expedition_uuid, expedition_uuid,
dynamiczone, dynamiczone,
request.GetExpeditionName(), request.GetExpeditionName(),
leader, ExpeditionMember{ request.GetLeaderID(), request.GetLeaderName() },
request.GetMinPlayers(), request.GetMinPlayers(),
request.GetMaxPlayers() request.GetMaxPlayers()
)); ));
LogExpeditions( LogExpeditions(
"Created [{}] ({}) instance id: [{}] leader: [{}] minplayers: [{}] maxplayers: [{}]", "Created [{}] [{}] instance id: [{}] leader: [{}] minplayers: [{}] maxplayers: [{}]",
expedition->GetID(), expedition->GetID(),
expedition->GetName(), expedition->GetName(),
expedition->GetInstanceID(), expedition->GetInstanceID(),
@ -133,10 +131,8 @@ Expedition* Expedition::TryCreate(
inserted.first->second->SendUpdatesToZoneMembers(); inserted.first->second->SendUpdatesToZoneMembers();
inserted.first->second->SendWorldExpeditionUpdate(ServerOP_ExpeditionCreate); // cache in other zones inserted.first->second->SendWorldExpeditionUpdate(ServerOP_ExpeditionCreate); // cache in other zones
Client* leader_client = request.GetLeaderClient(); inserted.first->second->SendLeaderMessage(
request.GetLeaderClient(), Chat::Yellow, EXPEDITION_AVAILABLE, { request.GetExpeditionName() }
Client::SendCrossZoneMessageString(
leader_client, leader.name, Chat::Yellow, EXPEDITION_AVAILABLE, { request.GetExpeditionName() }
); );
return inserted.first->second.get(); return inserted.first->second.get();
@ -264,7 +260,7 @@ void Expedition::CacheFromDatabase(uint32_t expedition_id)
CacheExpeditions(results); CacheExpeditions(results);
auto elapsed = benchmark.elapsed(); auto elapsed = benchmark.elapsed();
LogExpeditions("Caching new expedition [{}] took {}s", expedition_id, elapsed); LogExpeditions("Caching new expedition [{}] took [{}s]", expedition_id, elapsed);
} }
} }
@ -290,7 +286,7 @@ bool Expedition::CacheAllFromDatabase()
CacheExpeditions(results); CacheExpeditions(results);
auto elapsed = benchmark.elapsed(); auto elapsed = benchmark.elapsed();
LogExpeditions("Caching [{}] expedition(s) took {}s", zone->expedition_cache.size(), elapsed); LogExpeditions("Caching [{}] expedition(s) took [{}s]", zone->expedition_cache.size(), elapsed);
return true; return true;
} }
@ -923,7 +919,7 @@ void Expedition::TryAddClient(
} }
void Expedition::DzAddPlayer( void Expedition::DzAddPlayer(
Client* requester, std::string add_char_name, std::string swap_remove_name) Client* requester, const std::string& add_char_name, const std::string& swap_remove_name)
{ {
if (!requester || !ConfirmLeaderCommand(requester)) if (!requester || !ConfirmLeaderCommand(requester))
{ {
@ -1015,7 +1011,7 @@ void Expedition::DzMakeLeader(Client* requester, std::string new_leader_name)
else else
{ {
// new leader not in this zone, let world verify and pass to new leader's zone // new leader not in this zone, let world verify and pass to new leader's zone
SendWorldMakeLeaderRequest(requester->GetName(), FormatName(new_leader_name)); SendWorldMakeLeaderRequest(requester->GetName(), new_leader_name);
} }
} }
@ -1026,22 +1022,15 @@ void Expedition::DzRemovePlayer(Client* requester, std::string char_name)
return; return;
} }
LogExpeditionsModerate(
"Remove player request for expedition [{}] by [{}] leader [{}] remove name [{}]",
m_id, requester->GetName(), m_leader.name, char_name
);
char_name = FormatName(char_name);
// live only seems to enforce min_players for requesting expeditions, no need to check here // live only seems to enforce min_players for requesting expeditions, no need to check here
bool removed = RemoveMember(char_name); bool removed = RemoveMember(char_name);
if (!removed) if (!removed)
{ {
requester->MessageString(Chat::Red, EXPEDITION_NOT_MEMBER, char_name.c_str()); requester->MessageString(Chat::Red, EXPEDITION_NOT_MEMBER, FormatName(char_name).c_str());
} }
else else
{ {
requester->MessageString(Chat::Yellow, EXPEDITION_REMOVED, char_name.c_str(), m_expedition_name.c_str()); requester->MessageString(Chat::Yellow, EXPEDITION_REMOVED, FormatName(char_name).c_str(), m_expedition_name.c_str());
} }
} }
@ -1165,7 +1154,7 @@ void Expedition::ProcessMakeLeader(
} }
} }
void Expedition::ProcessMemberAdded(std::string char_name, uint32_t added_char_id) void Expedition::ProcessMemberAdded(const std::string& char_name, uint32_t added_char_id)
{ {
// adds the member to this expedition and notifies both leader and new member // adds the member to this expedition and notifies both leader and new member
Client* leader_client = entity_list.GetClientByCharID(m_leader.char_id); Client* leader_client = entity_list.GetClientByCharID(m_leader.char_id);
@ -1188,7 +1177,7 @@ void Expedition::ProcessMemberAdded(std::string char_name, uint32_t added_char_i
SendUpdatesToZoneMembers(); // live sends full update when member added SendUpdatesToZoneMembers(); // live sends full update when member added
} }
void Expedition::ProcessMemberRemoved(std::string removed_char_name, uint32_t removed_char_id) void Expedition::ProcessMemberRemoved(const std::string& removed_char_name, uint32_t removed_char_id)
{ {
if (m_members.empty()) if (m_members.empty())
{ {
@ -1731,11 +1720,10 @@ void Expedition::HandleWorldMessage(ServerPacket* pack)
case ServerOP_ExpeditionMemberChange: case ServerOP_ExpeditionMemberChange:
{ {
auto buf = reinterpret_cast<ServerExpeditionMemberChange_Struct*>(pack->pBuffer); auto buf = reinterpret_cast<ServerExpeditionMemberChange_Struct*>(pack->pBuffer);
if (zone && !zone->IsZone(buf->sender_zone_id, buf->sender_instance_id))
auto expedition = Expedition::FindCachedExpeditionByID(buf->expedition_id);
if (expedition && zone)
{ {
if (!zone->IsZone(buf->sender_zone_id, buf->sender_instance_id)) auto expedition = Expedition::FindCachedExpeditionByID(buf->expedition_id);
if (expedition)
{ {
if (buf->removed) if (buf->removed)
{ {
@ -1834,8 +1822,9 @@ void Expedition::HandleWorldMessage(ServerPacket* pack)
Client* leader = entity_list.GetClientByName(buf->requester_name); Client* leader = entity_list.GetClientByName(buf->requester_name);
if (leader) if (leader)
{ {
leader->MessageString(Chat::Red, DZADD_NOT_ONLINE, FormatName(buf->target_name).c_str()); std::string target_name = FormatName(buf->target_name);
leader->MessageString(Chat::Red, DZADD_INVITE_FAIL, FormatName(buf->target_name).c_str()); leader->MessageString(Chat::Red, DZADD_NOT_ONLINE, target_name.c_str());
leader->MessageString(Chat::Red, DZADD_INVITE_FAIL, target_name.c_str());
} }
} }
break; break;

View File

@ -66,7 +66,7 @@ class Expedition
public: public:
Expedition() = delete; Expedition() = delete;
Expedition( Expedition(
uint32_t id, const std::string& uuid, const DynamicZone& dz, std::string expedition_name, uint32_t id, const std::string& uuid, const DynamicZone& dz, const std::string& expedition_name,
const ExpeditionMember& leader, uint32_t min_players, uint32_t max_players); const ExpeditionMember& leader, uint32_t min_players, uint32_t max_players);
static Expedition* TryCreate(Client* requester, DynamicZone& dynamiczone, ExpeditionRequest& request); static Expedition* TryCreate(Client* requester, DynamicZone& dynamiczone, ExpeditionRequest& request);
@ -128,7 +128,7 @@ public:
void SendClientExpeditionInfo(Client* client); void SendClientExpeditionInfo(Client* client);
void SendWorldPendingInvite(const ExpeditionInvite& invite, const std::string& add_name); void SendWorldPendingInvite(const ExpeditionInvite& invite, const std::string& add_name);
void DzAddPlayer(Client* requester, std::string add_char_name, std::string swap_remove_name = {}); void DzAddPlayer(Client* requester, const std::string& add_char_name, const std::string& swap_remove_name = {});
void DzAddPlayerContinue(std::string leader_name, std::string add_char_name, std::string swap_remove_name = {}); void DzAddPlayerContinue(std::string leader_name, std::string add_char_name, std::string swap_remove_name = {});
void DzInviteResponse(Client* add_client, bool accepted, const std::string& swap_remove_name); void DzInviteResponse(Client* add_client, bool accepted, const std::string& swap_remove_name);
void DzMakeLeader(Client* requester, std::string new_leader_name); void DzMakeLeader(Client* requester, std::string new_leader_name);
@ -161,8 +161,8 @@ private:
void ProcessLeaderChanged(uint32_t new_leader_id, const std::string& new_leader_name); void ProcessLeaderChanged(uint32_t new_leader_id, const std::string& new_leader_name);
void ProcessLockoutUpdate(const ExpeditionLockoutTimer& lockout, bool remove, bool members_only = false); void ProcessLockoutUpdate(const ExpeditionLockoutTimer& lockout, bool remove, bool members_only = false);
void ProcessMakeLeader(Client* old_leader, Client* new_leader, const std::string& new_leader_name, bool is_online); void ProcessMakeLeader(Client* old_leader, Client* new_leader, const std::string& new_leader_name, bool is_online);
void ProcessMemberAdded(std::string added_char_name, uint32_t added_char_id); void ProcessMemberAdded(const std::string& added_char_name, uint32_t added_char_id);
void ProcessMemberRemoved(std::string removed_char_name, uint32_t removed_char_id); void ProcessMemberRemoved(const std::string& removed_char_name, uint32_t removed_char_id);
void SaveLockouts(ExpeditionRequest& request); void SaveLockouts(ExpeditionRequest& request);
void SaveMembers(ExpeditionRequest& request); void SaveMembers(ExpeditionRequest& request);
void SendClientExpeditionInvite( void SendClientExpeditionInvite(

View File

@ -79,7 +79,7 @@ bool ExpeditionRequest::Validate(Client* requester)
} }
auto elapsed = benchmark.elapsed(); auto elapsed = benchmark.elapsed();
LogExpeditions("Create validation for [{}] members took {}s", m_members.size(), elapsed); LogExpeditions("Create validation for [{}] members took [{}s]", m_members.size(), elapsed);
return requirements_met; return requirements_met;
} }