[Expeditions] Refactor expedition requests (#1301)

Move ExpeditionLockoutTimer to common

This simplifies expedition request conflict checks and uses repository
for the queries instead of processing the query result directly.

common/CMakeLists.txt

@@ -31,6 +31,7 @@ SET(common_sources
 	eq_stream_proxy.cpp
 	eqtime.cpp
 	event_sub.cpp
+	expedition_lockout_timer.cpp
 	extprofile.cpp
 	faction.cpp
 	file_util.cpp
@@ -488,6 +489,7 @@ SET(common_headers
 	eqtime.h
 	errmsg.h
 	event_sub.h
+	expedition_lockout_timer.h
 	extprofile.h
 	faction.h
 	file_util.h

common/expedition_lockout_timer.cpp

@@ -27,16 +27,16 @@
 const char* const DZ_REPLAY_TIMER_NAME = "Replay Timer"; // see December 14, 2016 patch notes
 
 ExpeditionLockoutTimer::ExpeditionLockoutTimer(
-	const std::string& expedition_uuid, const std::string& expedition_name,
+	std::string expedition_uuid, std::string expedition_name,
-	const std::string& event_name, uint64_t expire_time, uint32_t duration
+	std::string event_name, uint64_t expire_time, uint32_t duration
 ) :
-	m_expedition_uuid(expedition_uuid),
+	m_expedition_uuid{std::move(expedition_uuid)},
-	m_expedition_name(expedition_name),
+	m_expedition_name{std::move(expedition_name)},
-	m_event_name(event_name),
+	m_event_name{std::move(event_name)},
 	m_expire_time(std::chrono::system_clock::from_time_t(expire_time)),
 	m_duration(duration)
 {
-	if (event_name == DZ_REPLAY_TIMER_NAME)
+	if (m_event_name == DZ_REPLAY_TIMER_NAME)
 	{
 		m_is_replay_timer = true;
 	}

common/expedition_lockout_timer.h

@@ -31,8 +31,8 @@ class ExpeditionLockoutTimer
 public:
 	ExpeditionLockoutTimer() = default;
 	ExpeditionLockoutTimer(
-		const std::string& expedition_uuid, const std::string& expedition_name,
+		std::string expedition_uuid, std::string expedition_name,
-		const std::string& event_name, uint64_t expire_time, uint32_t duration);
+		std::string event_name, uint64_t expire_time, uint32_t duration);
 
 	static ExpeditionLockoutTimer CreateLockout(
 		const std::string& expedition_name, const std::string& event_name,

common/repositories/base/base_character_expedition_lockouts_repository.h

@@ -81,7 +81,7 @@ public:
 		entry.character_id         = 0;
 		entry.expedition_name      = "";
 		entry.event_name           = "";
-		entry.expire_time          = current_timestamp();
+		entry.expire_time          = "";
 		entry.duration             = 0;
 		entry.from_expedition_uuid = "";
 

common/repositories/character_expedition_lockouts_repository.h

@@ -22,8 +22,10 @@
 #define EQEMU_CHARACTER_EXPEDITION_LOCKOUTS_REPOSITORY_H
 
 #include "../database.h"
+#include "../expedition_lockout_timer.h"
 #include "../string_util.h"
 #include "base/base_character_expedition_lockouts_repository.h"
+#include <unordered_map>
 
 class CharacterExpeditionLockoutsRepository: public BaseCharacterExpeditionLockoutsRepository {
 public:
@@ -65,6 +67,78 @@ public:
 
 	// Custom extended repository methods here
 
+	struct CharacterExpeditionLockoutsTimeStamp {
+		int         id;
+		int         character_id;
+		std::string expedition_name;
+		std::string event_name;
+		time_t      expire_time;
+		int         duration;
+		std::string from_expedition_uuid;
+	};
+
+	static ExpeditionLockoutTimer GetExpeditionLockoutTimerFromEntry(
+		CharacterExpeditionLockoutsTimeStamp&& entry)
+	{
+		ExpeditionLockoutTimer lockout_timer{
+			std::move(entry.from_expedition_uuid),
+			std::move(entry.expedition_name),
+			std::move(entry.event_name),
+			static_cast<uint64_t>(entry.expire_time),
+			static_cast<uint32_t>(entry.duration)
+		};
+
+		return lockout_timer;
+	}
+
+	static std::unordered_map<uint32_t, std::vector<ExpeditionLockoutTimer>> GetManyCharacterLockoutTimers(
+		Database& db, const std::vector<uint32_t>& character_ids,
+		const std::string& expedition_name, const std::string& ordered_event_name)
+	{
+		auto joined_character_ids = fmt::join(character_ids, ",");
+
+		auto results = db.QueryDatabase(fmt::format(SQL(
+			SELECT
+				character_id,
+				UNIX_TIMESTAMP(expire_time),
+				duration,
+				event_name,
+				from_expedition_uuid
+			FROM character_expedition_lockouts
+			WHERE
+				character_id IN ({})
+				AND expire_time > NOW()
+				AND expedition_name = '{}'
+			ORDER BY
+				FIELD(character_id, {}),
+				FIELD(event_name, '{}') DESC
+		),
+			joined_character_ids,
+			EscapeString(expedition_name),
+			joined_character_ids,
+			EscapeString(ordered_event_name)
+		));
+
+		std::unordered_map<uint32_t, std::vector<ExpeditionLockoutTimer>> lockouts;
+
+		for (auto row = results.begin(); row != results.end(); ++row)
+		{
+			CharacterExpeditionLockoutsTimeStamp entry{};
+
+			int col = 0;
+			entry.character_id         = std::strtoul(row[col++], nullptr, 10);
+			entry.expire_time          = std::strtoull(row[col++], nullptr, 10);
+			entry.duration             = std::strtoul(row[col++], nullptr, 10);
+			entry.event_name           = row[col++];
+			entry.expedition_name      = expedition_name;
+			entry.from_expedition_uuid = row[col++];
+
+			auto lockout = GetExpeditionLockoutTimerFromEntry(std::move(entry));
+			lockouts[entry.character_id].emplace_back(std::move(lockout));
+		}
+
+		return lockouts;
+	}
 };
 
 #endif //EQEMU_CHARACTER_EXPEDITION_LOCKOUTS_REPOSITORY_H

common/repositories/expeditions_repository.h

@@ -65,6 +65,61 @@ public:
 
 	// Custom extended repository methods here
 
+	struct CharacterExpedition
+	{
+		uint32_t    id;
+		std::string name;
+		uint32_t    expedition_id;
+	};
+
+	static std::vector<CharacterExpedition> GetCharactersWithExpedition(
+		Database& db, const std::vector<std::string>& character_names)
+	{
+		if (character_names.empty())
+		{
+			return {};
+		}
+
+		std::vector<CharacterExpedition> entries;
+
+		auto joined_character_names = fmt::format("'{}'", fmt::join(character_names, "','"));
+
+		auto results = db.QueryDatabase(fmt::format(SQL(
+			SELECT
+				character_data.id,
+				character_data.name,
+				MAX(expeditions.id)
+			FROM character_data
+				LEFT JOIN expedition_members
+					ON character_data.id = expedition_members.character_id
+					AND expedition_members.is_current_member = TRUE
+				LEFT JOIN expeditions
+					ON expedition_members.expedition_id = expeditions.id
+			WHERE character_data.name IN ({})
+			GROUP BY character_data.id
+			ORDER BY FIELD(character_data.name, {})
+		),
+			joined_character_names,
+			joined_character_names
+		));
+
+		if (results.Success())
+		{
+			entries.reserve(results.RowCount());
+
+			for (auto row = results.begin(); row != results.end(); ++row)
+			{
+				CharacterExpedition entry{};
+				entry.id            = std::strtoul(row[0], nullptr, 10);
+				entry.name          = row[1];
+				entry.expedition_id = row[2] ? std::strtoul(row[2], nullptr, 10) : 0;
+
+				entries.emplace_back(std::move(entry));
+			}
+		}
+
+		return entries;
+	}
 };
 
 #endif //EQEMU_EXPEDITIONS_REPOSITORY_H

zone/CMakeLists.txt

@@ -33,7 +33,6 @@ SET(zone_sources
 	exp.cpp
 	expedition.cpp
 	expedition_database.cpp
-	expedition_lockout_timer.cpp
 	expedition_request.cpp
 	fastmath.cpp
 	fearpath.cpp
@@ -186,7 +185,6 @@ SET(zone_headers
 	event_codes.h
 	expedition.h
 	expedition_database.h
-	expedition_lockout_timer.h
 	expedition_request.h
 	fastmath.h
 	forage.h

zone/client.cpp

@@ -42,7 +42,6 @@ extern volatile bool RunLoops;
 #include "data_bucket.h"
 #include "expedition.h"
 #include "expedition_database.h"
-#include "expedition_lockout_timer.h"
 #include "expedition_request.h"
 #include "position.h"
 #include "worldserver.h"
@@ -61,6 +60,7 @@ extern volatile bool RunLoops;
 #include "queryserv.h"
 #include "mob_movement_manager.h"
 #include "../common/content/world_content_service.h"
+#include "../common/expedition_lockout_timer.h"
 
 extern QueryServ* QServ;
 extern EntityList entity_list;

zone/expedition.cpp

@@ -20,13 +20,13 @@
 
 #include "expedition.h"
 #include "expedition_database.h"
-#include "expedition_lockout_timer.h"
 #include "expedition_request.h"
 #include "client.h"
 #include "string_ids.h"
 #include "worldserver.h"
 #include "zonedb.h"
 #include "../common/eqemu_logsys.h"
+#include "../common/expedition_lockout_timer.h"
 #include "../common/util/uuid.h"
 
 extern WorldServer worldserver;

zone/expedition.h

@@ -22,8 +22,8 @@
 #define EXPEDITION_H
 
 #include "dynamic_zone.h"
-#include "expedition_lockout_timer.h"
 #include "../common/eq_constants.h"
+#include "../common/expedition_lockout_timer.h"
 #include <cstdint>
 #include <memory>
 #include <string>

zone/expedition_database.cpp

@@ -20,9 +20,9 @@
 
 #include "expedition_database.h"
 #include "expedition.h"
-#include "expedition_lockout_timer.h"
 #include "zonedb.h"
 #include "../common/database.h"
+#include "../common/expedition_lockout_timer.h"
 #include "../common/string_util.h"
 #include <fmt/core.h>
 
@@ -218,53 +218,6 @@ ExpeditionDatabase::LoadMultipleExpeditionLockouts(
 	return lockouts;
 }
 
-MySQLRequestResult ExpeditionDatabase::LoadMembersForCreateRequest(
-	const std::vector<std::string>& character_names, const std::string& expedition_name)
-{
-	LogExpeditionsDetail(
-		"Loading data of [{}] characters for [{}] request", character_names.size(), expedition_name
-	);
-
-	std::string in_character_names_query;
-	for (const auto& character_name : character_names)
-	{
-		fmt::format_to(std::back_inserter(in_character_names_query), "'{}',", character_name);
-	}
-
-	MySQLRequestResult results;
-
-	if (!in_character_names_query.empty())
-	{
-		in_character_names_query.pop_back(); // trailing comma
-
-		// for create validation, loads each character's lockouts and possible current expedition
-		auto query = fmt::format(SQL(
-			SELECT
-				character_data.id,
-				character_data.name,
-				member.expedition_id,
-				lockout.from_expedition_uuid,
-				UNIX_TIMESTAMP(lockout.expire_time),
-				lockout.duration,
-				lockout.event_name
-			FROM character_data
-				LEFT JOIN character_expedition_lockouts lockout
-					ON character_data.id = lockout.character_id
-					AND lockout.expire_time > NOW()
-					AND lockout.expedition_name = '{}'
-				LEFT JOIN expedition_members member
-					ON character_data.id = member.character_id
-					AND member.is_current_member = TRUE
-			WHERE character_data.name IN ({})
-			ORDER BY FIELD(character_data.name, {})
-		), EscapeString(expedition_name), in_character_names_query, in_character_names_query);
-
-		results = database.QueryDatabase(query);
-	}
-
-	return results;
-}
-
 void ExpeditionDatabase::DeleteAllCharacterLockouts(uint32_t character_id)
 {
 	LogExpeditionsDetail("Deleting all character [{}] lockouts", character_id);

zone/expedition_database.h

@@ -41,8 +41,6 @@ namespace ExpeditionDatabase
 	std::string LoadExpeditionsSelectQuery();
 	MySQLRequestResult LoadExpedition(uint32_t expedition_id);
 	MySQLRequestResult LoadAllExpeditions();
-	MySQLRequestResult LoadMembersForCreateRequest(
-		const std::vector<std::string>& character_names, const std::string& expedition_name);
 	std::vector<ExpeditionLockoutTimer> LoadCharacterLockouts(uint32_t character_id);
 	std::vector<ExpeditionLockoutTimer> LoadCharacterLockouts(uint32_t character_id,
 		const std::string& expedition_name);
@@ -94,18 +92,4 @@ namespace LoadExpeditionColumns
 	};
 };
 
-namespace LoadMembersForCreateRequestColumns
-{
-	enum eLoadMembersForCreateRequestColumns
-	{
-		character_id = 0,
-		character_name,
-		character_expedition_id,
-		lockout_uuid,
-		lockout_expire_time,
-		lockout_duration,
-		lockout_event_name
-	};
-};
-
 #endif

zone/expedition_request.cpp

@@ -21,27 +21,19 @@
 #include "expedition_request.h"
 #include "client.h"
 #include "expedition.h"
-#include "expedition_database.h"
-#include "expedition_lockout_timer.h"
 #include "groups.h"
 #include "raids.h"
 #include "string_ids.h"
-#include "worldserver.h"
+#include "../common/expedition_lockout_timer.h"
-
+#include "../common/repositories/character_expedition_lockouts_repository.h"
-extern WorldServer worldserver;
+#include "../common/repositories/expeditions_repository.h"
 
 constexpr char SystemName[] = "expedition";
 
-struct ExpeditionRequestConflict
+ExpeditionRequest::ExpeditionRequest(std::string expedition_name,
-{
+	uint32_t min_players, uint32_t max_players, bool disable_messages
-	std::string character_name;
-	ExpeditionLockoutTimer lockout;
-};
-
-ExpeditionRequest::ExpeditionRequest(
-	std::string expedition_name, uint32_t min_players, uint32_t max_players, bool disable_messages
 ) :
-	m_expedition_name(expedition_name),
+	m_expedition_name(std::move(expedition_name)),
 	m_min_players(min_players),
 	m_max_players(max_players),
 	m_disable_messages(disable_messages)
@@ -186,117 +178,97 @@ bool ExpeditionRequest::CanMembersJoin(const std::vector<std::string>& member_na
 	return requirements_met;
 }
 
-bool ExpeditionRequest::LoadLeaderLockouts()
+bool ExpeditionRequest::SaveLeaderLockouts(const std::vector<ExpeditionLockoutTimer>& lockouts)
 {
-	// leader's lockouts are used to check member conflicts and later stored in expedition
+	bool has_replay_lockout = false;
-	auto lockouts = ExpeditionDatabase::LoadCharacterLockouts(m_leader_id, m_expedition_name);
 
-	for (auto& lockout : lockouts)
+	for (const auto& lockout : lockouts)
 	{
 		if (!lockout.IsExpired())
 		{
-			m_lockouts.emplace(lockout.GetEventName(), lockout);
+			m_lockouts[lockout.GetEventName()] = lockout;
 
-			// on live if leader has a replay lockout it never bothers checking for event conflicts
+			if (lockout.IsReplayTimer())
-			if (m_check_event_lockouts && lockout.IsReplayTimer())
 			{
-				m_check_event_lockouts = false;
+				has_replay_lockout = true;
 			}
 		}
 	}
 
-	return true;
+	return has_replay_lockout;
 }
 
 bool ExpeditionRequest::CheckMembersForConflicts(const std::vector<std::string>& member_names)
 {
-	// load data for each member and compare with leader lockouts
+	// order of member_names is preserved by queries for use with max member truncation
-	auto results = ExpeditionDatabase::LoadMembersForCreateRequest(member_names, m_expedition_name);
+	auto entries = ExpeditionsRepository::GetCharactersWithExpedition(database, member_names);
-	if (!results.Success() || !LoadLeaderLockouts())
+	if (entries.empty())
 	{
-		LogExpeditions("Failed to load data to verify members for expedition request");
+		LogExpeditions("Failed to load members for expedition request");
 		return true;
 	}
 
 	bool is_solo = (member_names.size() == 1);
 	bool has_conflicts = false;
 
-	using col = LoadMembersForCreateRequestColumns::eLoadMembersForCreateRequestColumns;
+	std::vector<uint32_t> character_ids;
-
+	for (const auto& character : entries)
-	std::vector<ExpeditionRequestConflict> member_lockout_conflicts;
-
-	uint32_t last_character_id = 0;
-	for (auto row = results.begin(); row != results.end(); ++row)
 	{
-		uint32_t character_id      = std::strtoul(row[col::character_id], nullptr, 10);
+		if (is_solo && character.expedition_id != 0)
-		std::string character_name = row[col::character_name];
-		bool has_expedition        = (row[col::character_expedition_id] != nullptr);
-
-		if (character_id != last_character_id)
 		{
-			// defaults to online status, if offline group members implemented this needs to change
+			// live doesn't bother checking replay lockout here
-			m_members.emplace_back(character_id, character_name);
+			SendLeaderMemberInExpedition(character.name, is_solo);
-
+			return true;
-			// process event lockout conflict messages from the previous character
-			for (const auto& member_lockout : member_lockout_conflicts)
-			{
-				SendLeaderMemberEventLockout(member_lockout.character_name, member_lockout.lockout);
-			}
-			member_lockout_conflicts.clear();
-
-			if (has_expedition)
-			{
-				has_conflicts = true;
-				SendLeaderMemberInExpedition(character_name, is_solo);
-
-				// solo requests break out early if requester in an expedition
-				if (is_solo)
-				{
-					return has_conflicts;
-				}
-			}
 		}
 
-		last_character_id = character_id;
+		m_members.emplace_back(character.id, character.name, ExpeditionMemberStatus::Online);
+		character_ids.emplace_back(character.id);
+	}
 
-		// compare member lockouts with leader lockouts
+	auto member_lockouts = CharacterExpeditionLockoutsRepository::GetManyCharacterLockoutTimers(
-		if (row[col::lockout_uuid]) // lockout results may be null
+		database, character_ids, m_expedition_name, DZ_REPLAY_TIMER_NAME);
+
+	// on live if leader has a replay lockout it never checks for event conflicts
+	bool leader_has_replay_lockout = false;
+	auto lockout_iter = member_lockouts.find(m_leader_id);
+	if (lockout_iter != member_lockouts.end())
+	{
+		leader_has_replay_lockout = SaveLeaderLockouts(lockout_iter->second);
+	}
+
+	for (const auto& character : entries)
+	{
+		if (character.expedition_id != 0)
 		{
-			auto expire_time = strtoull(row[col::lockout_expire_time], nullptr, 10);
+			has_conflicts = true;
-			uint32_t duration = strtoul(row[col::lockout_duration], nullptr, 10);
+			SendLeaderMemberInExpedition(character.name, is_solo);
+		}
 
-			ExpeditionLockoutTimer lockout{
+		auto lockout_iter = member_lockouts.find(character.id);
-				row[col::lockout_uuid], m_expedition_name, row[col::lockout_event_name], expire_time, duration
+		if (lockout_iter != member_lockouts.end())
-			};
+		{
-
+			for (const auto& lockout : lockout_iter->second)
-			if (!lockout.IsExpired())
 			{
-				if (lockout.IsReplayTimer())
+				if (!lockout.IsExpired())
 				{
-					// replay timer conflict messages always show up before event conflicts
+					// replay timers were sorted by query so they show up before event conflicts
-					has_conflicts = true;
+					if (lockout.IsReplayTimer())
-					SendLeaderMemberReplayLockout(character_name, lockout, is_solo);
-				}
-				else if (m_check_event_lockouts && character_id != m_leader_id)
-				{
-					if (m_lockouts.find(lockout.GetEventName()) == m_lockouts.end())
 					{
-						// leader doesn't have this lockout. queue instead of messaging
-						// now so message comes after any replay lockout messages
 						has_conflicts = true;
-						member_lockout_conflicts.push_back({character_name, lockout});
+						SendLeaderMemberReplayLockout(character.name, lockout, is_solo);
+					}
+					else if (!leader_has_replay_lockout && character.id != m_leader_id &&
+					         m_lockouts.find(lockout.GetEventName()) == m_lockouts.end())
+					{
+						// leader doesn't have this lockout
+						has_conflicts = true;
+						SendLeaderMemberEventLockout(character.name, lockout);
 					}
 				}
 			}
 		}
 	}
 
-	// event lockout messages for last processed character
-	for (const auto& member_lockout : member_lockout_conflicts)
-	{
-		SendLeaderMemberEventLockout(member_lockout.character_name, member_lockout.lockout);
-	}
-
 	return has_conflicts;
 }
 

zone/expedition_request.h

@@ -22,7 +22,7 @@
 #define EXPEDITION_REQUEST_H
 
 #include "expedition.h"
-#include "expedition_lockout_timer.h"
+#include "../common/expedition_lockout_timer.h"
 #include <cstdint>
 #include <string>
 #include <vector>
@@ -30,16 +30,13 @@
 
 class Client;
 class Group;
-class MySQLRequestResult;
 class Raid;
-class ServerPacket;
 
 class ExpeditionRequest
 {
 public:
-	ExpeditionRequest(
+	ExpeditionRequest(std::string expedition_name, uint32_t min_players,
-		std::string expedition_name, uint32_t min_players, uint32_t max_players,
+		uint32_t max_players, bool disable_messages = false);
-		bool disable_messages = false);
 
 	bool Validate(Client* requester);
 
@@ -60,7 +57,7 @@ private:
 	bool CheckMembersForConflicts(const std::vector<std::string>& member_names);
 	std::string GetGroupLeaderName(uint32_t group_id);
 	bool IsPlayerCountValidated();
-	bool LoadLeaderLockouts();
+	bool SaveLeaderLockouts(const std::vector<ExpeditionLockoutTimer>& leader_lockouts);
 	void SendLeaderMemberInExpedition(const std::string& member_name, bool is_solo);
 	void SendLeaderMemberReplayLockout(const std::string& member_name, const ExpeditionLockoutTimer& lockout, bool is_solo);
 	void SendLeaderMemberEventLockout(const std::string& member_name, const ExpeditionLockoutTimer& lockout);
@@ -71,7 +68,6 @@ private:
 	uint32_t m_leader_id            = 0;
 	uint32_t m_min_players          = 0;
 	uint32_t m_max_players          = 0;
-	bool     m_check_event_lockouts = true;
 	bool     m_disable_messages     = false;
 	std::string m_expedition_name;
 	std::string m_leader_name;

zone/lua_client.cpp

@@ -5,7 +5,6 @@
 
 #include "client.h"
 #include "dynamic_zone.h"
-#include "expedition_lockout_timer.h"
 #include "expedition_request.h"
 #include "lua_client.h"
 #include "lua_expedition.h"
@@ -16,6 +15,7 @@
 #include "lua_group.h"
 #include "lua_raid.h"
 #include "lua_packet.h"
+#include "../common/expedition_lockout_timer.h"
 
 struct InventoryWhere { };