Verify new expedition leader in world

Cleanup makeleader methods

World now checks if character is expedition member instead of zone

common/servertalk.h

@@ -2108,6 +2108,14 @@ struct ServerDzCommand_Struct {
 	char   remove_name[64];    // used for swap command
 };
 
+struct ServerDzCommandMakeLeader_Struct {
+	uint32 expedition_id;
+	uint8  is_online;  // set by world, 0: new leader name offline, 1: online
+	uint8  is_success; // set by world, 0: makeleader failed, 1: success (is online member)
+	char   requester_name[64];
+	char   new_leader_name[64];
+};
+
 struct ServerDzLocation_Struct {
 	uint32 owner_id;           // system associated with the dz (expedition, shared task, etc)
 	uint16 dz_zone_id;

world/expedition.cpp

@@ -55,6 +55,12 @@ void Expedition::AddMember(uint32_t character_id)
 	}
 }
 
+bool Expedition::HasMember(uint32_t character_id)
+{
+	return std::any_of(m_member_ids.begin(), m_member_ids.end(),
+		[&](uint32_t member_id) { return member_id == character_id; });
+}
+
 void Expedition::RemoveMember(uint32_t character_id)
 {
 	m_member_ids.erase(std::remove_if(m_member_ids.begin(), m_member_ids.end(),
@@ -89,12 +95,18 @@ void Expedition::ChooseNewLeader()
 	}
 }
 
-void Expedition::SetNewLeader(uint32_t character_id)
+bool Expedition::SetNewLeader(uint32_t character_id)
 {
+	if (!HasMember(character_id))
+	{
+		return false;
+	}
+
 	LogExpeditionsModerate("Replacing [{}] leader [{}] with [{}]", m_expedition_id, m_leader_id, character_id);
 	ExpeditionDatabase::UpdateLeaderID(m_expedition_id, character_id);
 	m_leader_id = character_id;
 	SendZonesLeaderChanged();
+	return true;
 }
 
 void Expedition::SendZonesExpeditionDeleted()

world/expedition.h

@@ -41,6 +41,7 @@ public:
 	uint32_t GetID() const { return m_expedition_id; }
 	uint16_t GetInstanceID() const { return static_cast<uint16_t>(m_dz_instance_id); }
 	uint16_t GetZoneID() const { return static_cast<uint16_t>(m_dz_zone_id); }
+	bool HasMember(uint32_t character_id);
 	bool IsEmpty() const { return m_member_ids.empty(); }
 	bool IsExpired() const { return m_expire_time < std::chrono::system_clock::now(); }
 	bool IsPendingDelete() const { return m_pending_delete; }
@@ -48,7 +49,7 @@ public:
 	void SendZonesDurationUpdate();
 	void SendZonesExpeditionDeleted();
 	void SendZonesExpireWarning(uint32_t minutes_remaining);
-	void SetNewLeader(uint32_t new_leader_id);
+	bool SetNewLeader(uint32_t new_leader_id);
 	void SetPendingDelete(bool pending) { m_pending_delete = pending; }
 	void UpdateDzSecondsRemaining(uint32_t seconds_remaining);
 	std::chrono::system_clock::duration GetRemainingDuration() const;

world/expedition_message.cpp

@@ -137,22 +137,22 @@ void ExpeditionMessage::AddPlayer(ServerPacket* pack)
 
 void ExpeditionMessage::MakeLeader(ServerPacket* pack)
 {
-	auto buf = reinterpret_cast<ServerDzCommand_Struct*>(pack->pBuffer);
+	auto buf = reinterpret_cast<ServerDzCommandMakeLeader_Struct*>(pack->pBuffer);
 
 	// notify requester (old leader) and new leader of the result
 	ZoneServer* new_leader_zs = nullptr;
-	ClientListEntry* new_leader_cle = client_list.FindCharacter(buf->target_name);
+	ClientListEntry* new_leader_cle = client_list.FindCharacter(buf->new_leader_name);
 	if (new_leader_cle && new_leader_cle->Server())
 	{
-		buf->is_char_online = true;
-		new_leader_zs = new_leader_cle->Server();
-		new_leader_zs->SendPacket(pack);
-
 		auto expedition = expedition_state.GetExpedition(buf->expedition_id);
 		if (expedition)
 		{
-			expedition->SetNewLeader(new_leader_cle->CharID());
+			buf->is_success = expedition->SetNewLeader(new_leader_cle->CharID());
 		}
+
+		buf->is_online = true;
+		new_leader_zs = new_leader_cle->Server();
+		new_leader_zs->SendPacket(pack);
 	}
 
 	// if old and new leader are in the same zone only send one message

zone/command.cpp

@@ -6985,16 +6985,9 @@ void command_dz(Client* c, const Seperator* sep)
 		if (expedition)
 		{
 			auto char_name = FormatName(sep->arg[3]);
-			if (!expedition->HasMember(char_name))
-			{
-				c->Message(Chat::Red, fmt::format("Character [{}] is not in that expedition", char_name).c_str());
-			}
-			else
-			{
 			c->Message(Chat::White, fmt::format("Setting expedition [{}] leader to [{}]", expedition_id, char_name).c_str());
 			expedition->SendWorldMakeLeaderRequest(c->GetName(), char_name);
 		}
-		}
 		else
 		{
 			c->Message(Chat::Red, fmt::format("Failed to find expedition [{}]", expedition_id).c_str());

zone/expedition.cpp

@@ -1010,24 +1010,14 @@ void Expedition::DzMakeLeader(Client* requester, std::string new_leader_name)
 		return;
 	}
 
-	// live uses sanitized input name for all /dzmakeleader messages
-	new_leader_name = FormatName(new_leader_name);
-
 	if (new_leader_name.empty())
 	{
 		requester->MessageString(Chat::Red, DZMAKELEADER_NOT_ONLINE, new_leader_name.c_str());
 		return;
 	}
 
-	auto new_leader_data = GetMemberData(new_leader_name);
-	if (!new_leader_data.IsValid())
-	{
-		requester->MessageString(Chat::Red, EXPEDITION_NOT_MEMBER, new_leader_name.c_str());
-		return;
-	}
-
 	// leader can only be changed by world
-	SendWorldMakeLeaderRequest(requester->GetName(), new_leader_name);
+	SendWorldMakeLeaderRequest(requester->GetName(), FormatName(new_leader_name));
 }
 
 void Expedition::DzRemovePlayer(Client* requester, std::string char_name)
@@ -1160,34 +1150,30 @@ void Expedition::ProcessLeaderChanged(uint32_t new_leader_id)
 	}
 }
 
-void Expedition::ProcessMakeLeader(
+void Expedition::ProcessMakeLeader(Client* old_leader_client, Client* new_leader_client,
-	Client* old_leader_client, Client* new_leader_client, const std::string& new_leader_name, bool is_online)
+	const std::string& new_leader_name, bool is_success, bool is_online)
 {
 	if (old_leader_client)
 	{
-		// online flag is set by world to verify new leader is online or not
+		// success flag is set by world to indicate new leader set to an online member
-		if (is_online)
+		if (is_success)
 		{
 			old_leader_client->MessageString(Chat::Yellow, DZMAKELEADER_NAME, new_leader_name.c_str());
 		}
-		else
+		else if (!is_online)
 		{
 			old_leader_client->MessageString(Chat::Red, DZMAKELEADER_NOT_ONLINE, new_leader_name.c_str());
 		}
+		else
+		{
+			old_leader_client->MessageString(Chat::Red, EXPEDITION_NOT_MEMBER, new_leader_name.c_str());
+		}
 	}
 
-	if (!new_leader_client)
+	if (is_success && new_leader_client && !RuleB(Expedition, AlwaysNotifyNewLeaderOnChange))
-	{
-		new_leader_client = entity_list.GetClientByName(new_leader_name.c_str());
-	}
-
-	if (new_leader_client)
-	{
-		if (!RuleB(Expedition, AlwaysNotifyNewLeaderOnChange))
 	{
 		new_leader_client->MessageString(Chat::Yellow, DZMAKELEADER_YOU);
 	}
-	}
 }
 
 void Expedition::ProcessMemberAdded(const std::string& char_name, uint32_t added_char_id)
@@ -1596,13 +1582,12 @@ void Expedition::SendWorldLockoutUpdate(
 void Expedition::SendWorldMakeLeaderRequest(
 	const std::string& requester_name, const std::string& new_leader_name)
 {
-	uint32_t pack_size = sizeof(ServerDzCommand_Struct);
+	uint32_t pack_size = sizeof(ServerDzCommandMakeLeader_Struct);
 	auto pack = std::unique_ptr<ServerPacket>(new ServerPacket(ServerOP_ExpeditionDzMakeLeader, pack_size));
-	auto buf = reinterpret_cast<ServerDzCommand_Struct*>(pack->pBuffer);
+	auto buf = reinterpret_cast<ServerDzCommandMakeLeader_Struct*>(pack->pBuffer);
 	buf->expedition_id = GetID();
-	buf->is_char_online = false;
 	strn0cpy(buf->requester_name, requester_name.c_str(), sizeof(buf->requester_name));
-	strn0cpy(buf->target_name, new_leader_name.c_str(), sizeof(buf->target_name));
+	strn0cpy(buf->new_leader_name, new_leader_name.c_str(), sizeof(buf->new_leader_name));
 	worldserver.SendPacket(pack.get());
 }
 
@@ -1999,13 +1984,14 @@ void Expedition::HandleWorldMessage(ServerPacket* pack)
 	}
 	case ServerOP_ExpeditionDzMakeLeader:
 	{
-		auto buf = reinterpret_cast<ServerDzCommand_Struct*>(pack->pBuffer);
+		auto buf = reinterpret_cast<ServerDzCommandMakeLeader_Struct*>(pack->pBuffer);
 		auto expedition = Expedition::FindCachedExpeditionByID(buf->expedition_id);
 		if (expedition)
 		{
 			auto old_leader_client = entity_list.GetClientByName(buf->requester_name);
-			auto new_leader_client = entity_list.GetClientByName(buf->target_name);
+			auto new_leader_client = entity_list.GetClientByName(buf->new_leader_name);
-			expedition->ProcessMakeLeader(old_leader_client, new_leader_client, buf->target_name, buf->is_char_online);
+			expedition->ProcessMakeLeader(old_leader_client, new_leader_client,
+				buf->new_leader_name, buf->is_success, buf->is_online);
 		}
 		break;
 	}

zone/expedition.h

@@ -181,7 +181,8 @@ private:
 	void ProcessLeaderChanged(uint32_t new_leader_id);
 	void ProcessLockoutDuration(const ExpeditionLockoutTimer& lockout, int seconds, 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_success, bool is_online);
 	void ProcessMemberAdded(const std::string& added_char_name, uint32_t added_char_id);
 	void ProcessMemberRemoved(const std::string& removed_char_name, uint32_t removed_char_id);
 	void SaveLockouts(ExpeditionRequest& request);