changed pointer into unique pointer and addressed review concerns

common/patches/IMessage.h

@@ -37,12 +37,14 @@ public:
 	virtual ~IMessage() = default;
 
 	// these two are the basic string message packets
-	[[nodiscard]] virtual EQApplicationPacket* Simple(uint32_t color, uint32_t id) const = 0;
+	virtual std::unique_ptr<EQApplicationPacket> Simple(uint32_t color, uint32_t id) const = 0;
-	[[nodiscard]] virtual EQApplicationPacket* Formatted(uint32_t color, uint32_t id, const std::array<const char*, 9>& args) const = 0;
+	virtual std::unique_ptr<EQApplicationPacket> Formatted(uint32_t color, uint32_t id,
+		const std::array<const char*, 9>& args) const = 0;
 
 	// These aren't technically messages, but they use the same format and are similar enough to include here
-	virtual EQApplicationPacket* InterruptSpell(uint32_t message, uint32_t spawn_id, const char* spell_link) const = 0;
+	virtual std::unique_ptr<EQApplicationPacket> InterruptSpell(uint32_t message, uint32_t spawn_id,
-	virtual EQApplicationPacket* InterruptSpellOther(Mob* sender, uint32_t message, uint32_t spawn_id,
+		const char* spell_link) const = 0;
+	virtual std::unique_ptr<EQApplicationPacket> InterruptSpellOther(Mob* sender, uint32_t message, uint32_t spawn_id,
 		const char* name, const char* spell_link) const = 0;
 };
 

common/patches/client_version.cpp

@@ -34,31 +34,31 @@ struct ClientComponents
 	{
 		switch (version) {
 		case Version::TOB:
-			messageComponent = std::make_shared<Message::TOB>();
+			messageComponent = std::make_unique<Message::TOB>();
 			break;
 		case Version::RoF2:
-			messageComponent = std::make_shared<Message::RoF2>();
+			messageComponent = std::make_unique<Message::RoF2>();
 			break;
 		case Version::RoF:
-			messageComponent = std::make_shared<Message::RoF>();
+			messageComponent = std::make_unique<Message::RoF>();
 			break;
 		case Version::UF:
-			messageComponent = std::make_shared<Message::UF>();
+			messageComponent = std::make_unique<Message::UF>();
 			break;
 		case Version::SoD:
-			messageComponent = std::make_shared<Message::SoD>();
+			messageComponent = std::make_unique<Message::SoD>();
 			break;
 		case Version::SoF:
-			messageComponent = std::make_shared<Message::SoF>();
+			messageComponent = std::make_unique<Message::SoF>();
 			break;
 		default:
-			messageComponent = std::make_shared<Message::Titanium>();
+			messageComponent = std::make_unique<Message::Titanium>();
 			break;
 		}
 	}
 
 	const Version version;
-	std::shared_ptr<Message::IMessage> messageComponent;
+	std::unique_ptr<Message::IMessage> messageComponent;
 };
 
 static const ClientComponents& GetComponents(Version version)
@@ -78,7 +78,7 @@ static const ClientComponents& GetComponents(Version version)
 	return patches.at(version);
 }
 
-const std::shared_ptr<Message::IMessage>& GetMessageComponent(Version version)
+const std::unique_ptr<Message::IMessage>& GetMessageComponent(Version version)
 {
 	return GetComponents(version).messageComponent;
 }

common/patches/client_version.h

@@ -10,4 +10,4 @@
 namespace Message { class IMessage; }
 
 // store all static functions for the different patches here
-const std::shared_ptr<Message::IMessage>& GetMessageComponent(EQ::versions::ClientVersion version);
+const std::unique_ptr<Message::IMessage>& GetMessageComponent(EQ::versions::ClientVersion version);

common/patches/titanium.cpp

@@ -3922,12 +3922,11 @@ namespace Titanium
 } /*Titanium*/
 
 namespace Message {
-
+std::unique_ptr<EQApplicationPacket> Titanium::Simple(uint32_t color, uint32_t id) const
-EQApplicationPacket* Titanium::Simple(uint32_t color, uint32_t id) const
 {
 	uint32_t string_id = ResolveID(id);
 	if (string_id > 0) {
-		auto outapp = new EQApplicationPacket(OP_SimpleMessage, sizeof(SimpleMessage_Struct));
+		auto outapp = std::make_unique<EQApplicationPacket>(OP_SimpleMessage, sizeof(SimpleMessage_Struct));
 		auto* sms = reinterpret_cast<SimpleMessage_Struct*>(outapp->pBuffer);
 		sms->string_id = string_id;
 		sms->color = color;
@@ -3939,7 +3938,7 @@ EQApplicationPacket* Titanium::Simple(uint32_t color, uint32_t id) const
 	return nullptr;
 }
 
-EQApplicationPacket* Titanium::Formatted(
+std::unique_ptr<EQApplicationPacket> Titanium::Formatted(
 	uint32_t color, uint32_t id, const std::array<const char*, 9>& args) const
 {
 	uint32_t string_id = ResolveID(id);
@@ -3961,15 +3960,16 @@ EQApplicationPacket* Titanium::Formatted(
 
 		buf.WriteUInt8(0);
 
-		return new EQApplicationPacket(OP_FormattedMessage, std::move(buf));
+		return std::make_unique<EQApplicationPacket>(OP_FormattedMessage, std::move(buf));
 	}
 
 	return nullptr;
 }
 
-EQApplicationPacket* Titanium::InterruptSpell(uint32_t message, uint32_t spawn_id, const char* spell_link) const
+std::unique_ptr<EQApplicationPacket> Titanium::InterruptSpell(uint32_t message, uint32_t spawn_id,
+	const char* spell_link) const
 {
-	auto outapp = new EQApplicationPacket(OP_InterruptCast, sizeof(InterruptCast_Struct));
+	auto outapp = std::make_unique<EQApplicationPacket>(OP_InterruptCast, sizeof(InterruptCast_Struct));
 	auto ic = reinterpret_cast<InterruptCast_Struct*>(outapp->pBuffer);
 	ic->messageid = ResolveID(message);
 	ic->spawnid = spawn_id;
@@ -3978,10 +3978,11 @@ EQApplicationPacket* Titanium::InterruptSpell(uint32_t message, uint32_t spawn_i
 	return outapp;
 }
 
-EQApplicationPacket* Titanium::InterruptSpellOther(Mob* sender, uint32_t message, uint32_t spawn_id, const char* name,
+std::unique_ptr<EQApplicationPacket> Titanium::InterruptSpellOther(Mob* sender, uint32_t message, uint32_t spawn_id,
+	const char* name,
 	const char* spell_link) const
 {
-	auto outapp = new EQApplicationPacket(OP_InterruptCast, sizeof(InterruptCast_Struct) + strlen(name) + 1);
+	auto outapp = std::make_unique<EQApplicationPacket>(OP_InterruptCast, sizeof(InterruptCast_Struct) + strlen(name) + 1);
 	auto ic = reinterpret_cast<InterruptCast_Struct*>(outapp->pBuffer);
 	ic->messageid = ResolveID(message);
 	ic->spawnid = spawn_id;

common/patches/titanium.h

@@ -59,11 +59,14 @@ public:
 	Titanium() = default;
 	~Titanium() override = default;
 
-	[[nodiscard]] EQApplicationPacket* Simple(uint32_t color, uint32_t id) const override;
+	std::unique_ptr<EQApplicationPacket> Simple(uint32_t color, uint32_t id) const override;
-	[[nodiscard]] EQApplicationPacket* Formatted(uint32_t color, uint32_t id, const std::array<const char*, 9>& args) const override;
+	std::unique_ptr<EQApplicationPacket> Formatted(uint32_t color, uint32_t id,
+		const std::array<const char*, 9>& args) const override;
 
-	EQApplicationPacket* InterruptSpell(uint32_t message, uint32_t spawn_id, const char* spell_link) const override;
+	std::unique_ptr<EQApplicationPacket> InterruptSpell(uint32_t message, uint32_t spawn_id,
-	EQApplicationPacket* InterruptSpellOther(Mob* sender, uint32_t message, uint32_t spawn_id, const char* name,
+		const char* spell_link) const override;
+	std::unique_ptr<EQApplicationPacket> InterruptSpellOther(Mob* sender, uint32_t message, uint32_t spawn_id,
+		const char* name,
 		const char* spell_link) const override;
 
 protected:

common/patches/tob.cpp

@@ -5634,7 +5634,8 @@ void TOB::ResolveArguments(uint32_t id, std::array<const char*, 9>& args) const
 	}
 }
 
-EQApplicationPacket* TOB::Formatted(uint32_t color, uint32_t id, const std::array<const char*, 9>& args) const
+std::unique_ptr<EQApplicationPacket> TOB::Formatted(uint32_t color, uint32_t id,
+	const std::array<const char*, 9>& args) const
 {
 	uint32_t string_id = ResolveID(id);
 	if (string_id > 0) {
@@ -5660,15 +5661,16 @@ EQApplicationPacket* TOB::Formatted(uint32_t color, uint32_t id, const std::arra
 				buffer.WriteUInt32(0);
 		}
 
-		return new EQApplicationPacket(OP_FormattedMessage, std::move(buffer));
+		return std::make_unique<EQApplicationPacket>(OP_FormattedMessage, std::move(buffer));
 	}
 
 	return nullptr;
 }
 
-EQApplicationPacket* TOB::InterruptSpell(uint32_t message, uint32_t spawn_id, const char* spell_link) const
+std::unique_ptr<EQApplicationPacket> TOB::InterruptSpell(uint32_t message, uint32_t spawn_id,
+	const char* spell_link) const
 {
-	auto outapp = new EQApplicationPacket(OP_InterruptCast, sizeof(InterruptCast_Struct) + strlen(spell_link) + 1);
+	auto outapp = std::make_unique<EQApplicationPacket>(OP_InterruptCast, sizeof(InterruptCast_Struct) + strlen(spell_link) + 1);
 	auto ic = reinterpret_cast<InterruptCast_Struct*>(outapp->pBuffer);
 	ic->messageid = ResolveID(message);
 	ic->spawnid = spawn_id;
@@ -5678,10 +5680,11 @@ EQApplicationPacket* TOB::InterruptSpell(uint32_t message, uint32_t spawn_id, co
 	return outapp;
 }
 
-EQApplicationPacket* TOB::InterruptSpellOther(Mob* sender, uint32_t message, uint32_t spawn_id, const char* name,
+std::unique_ptr<EQApplicationPacket> TOB::InterruptSpellOther(Mob* sender, uint32_t message, uint32_t spawn_id,
+	const char* name,
 	const char* spell_link) const
 {
-	auto outapp = new EQApplicationPacket(OP_InterruptCast,
+	auto outapp = std::make_unique<EQApplicationPacket>(OP_InterruptCast,
 		sizeof(InterruptCast_Struct) + strlen(name) + strlen(spell_link) + 2);
 	auto ic = reinterpret_cast<InterruptCast_Struct*>(outapp->pBuffer);
 	ic->messageid = ResolveID(message);

common/patches/tob.h

@@ -42,10 +42,13 @@ public:
 	TOB() {}
 	~TOB() override {}
 
-	[[nodiscard]] EQApplicationPacket* Formatted(uint32_t color, uint32_t id, const std::array<const char*, 9>& args) const override;
+	std::unique_ptr<EQApplicationPacket> Formatted(uint32_t color, uint32_t id,
+		const std::array<const char*, 9>& args) const override;
 
-	EQApplicationPacket* InterruptSpell(uint32_t message, uint32_t spawn_id, const char* spell_link) const override;
+	std::unique_ptr<EQApplicationPacket> InterruptSpell(uint32_t message, uint32_t spawn_id,
-	EQApplicationPacket* InterruptSpellOther(Mob* sender, uint32_t message, uint32_t spawn_id, const char* name, const char* spell_link) const override;
+		const char* spell_link) const override;
+	std::unique_ptr<EQApplicationPacket> InterruptSpellOther(Mob* sender, uint32_t message, uint32_t spawn_id,
+		const char* name, const char* spell_link) const override;
 
 protected:
 	[[nodiscard]] uint32_t ResolveID(uint32_t id) const override;

zone/client_version.h

@@ -21,11 +21,9 @@ template <typename Fun, typename Obj, typename... Args>
 static void QueuePacket(Client* c, Fun fun, Obj* obj, Args&&... args)
 {
 	static_assert(std::is_member_function_pointer_v<Fun>);
-	EQApplicationPacket* app = std::invoke(fun, obj, std::forward<Args>(args)...);
+	std::unique_ptr<EQApplicationPacket> app = std::invoke(fun, obj, std::forward<Args>(args)...);
-	if (app != nullptr) {
+	if (app)
-		c->QueuePacket(app);
+		c->QueuePacket(app.get());
-		delete app;
-	}
 }
 
 // packet generator queue functions
@@ -35,23 +33,24 @@ static auto QueueClients(Mob* sender, bool ignore_sender = false, bool ackreq =
 		std::function<Obj*(const Client*)> component_getter, Args&&... args) {
 		static_assert(std::is_member_function_pointer_v<Fun> && "Function is required to be a member function");
 
-		std::unordered_map<EQ::versions::ClientVersion, EQApplicationPacket*> build_packets;
+		std::vector<std::pair<EQ::versions::ClientVersion, std::unique_ptr<EQApplicationPacket>>> build_packets;
 		std::unordered_map<uint16, Client*> client_list = entity_list.GetClientList();
 
 		for (auto [_, ent] : client_list) {
 			if (!ignore_sender || ent != sender) {
-				auto [packet, _] = build_packets.try_emplace(
+				auto packet_it = std::find_if(build_packets.begin(), build_packets.end(),
-					ent->ClientVersion(),
+					[version = ent->GetClientVersion()](const auto& build_packet) {
-					std::invoke(fun, component_getter(ent), std::forward<Args>(args)...));
+						return build_packet.first == version;
+					});
 
-				if (packet->second != nullptr)
+				if (packet_it == build_packets.end())
-					ent->QueuePacket(packet->second, ackreq, Client::CLIENT_CONNECTED);
+					packet_it = build_packets.emplace(build_packets.end(), ent->ClientVersion(),
+						std::invoke(fun, component_getter(ent), std::forward<Args>(args)...));
+
+				if (packet_it->second != nullptr)
+					ent->QueuePacket(packet_it->second.get(), ackreq, Client::CLIENT_CONNECTED);
 			}
 		}
-
-		for (auto [_, packet] : build_packets)
-			if (packet != nullptr)
-				delete packet;
 	};
 }
 
@@ -70,7 +69,7 @@ static auto QueueCloseClients(
 			QueueClients(sender, ignore_sender, is_ack_required)(fun, component_getter, std::forward<Args>(args)...);
 		} else {
 			float distance_squared = distance * distance;
-			std::unordered_map<EQ::versions::ClientVersion, EQApplicationPacket*> build_packets;
+			std::vector<std::pair<EQ::versions::ClientVersion, std::unique_ptr<EQApplicationPacket>>> build_packets;
 
 			for (auto& [_, mob] : sender->GetCloseMobList(distance)) {
 				if (mob && mob->IsClient()) {
@@ -79,20 +78,22 @@ static auto QueueCloseClients(
 						&& client != skipped_mob
 						&& DistanceSquared(client->GetPosition(), sender->GetPosition()) < distance_squared
 						&& client->Connected()
-						&& client->ShouldGetPacket(sender, filter)) {
+						&& client->ShouldGetPacket(sender, filter))
-						auto [packet, _] = build_packets.try_emplace(
+					{
-							client->ClientVersion(),
+						auto packet_it = std::find_if(build_packets.begin(), build_packets.end(),
-							std::invoke(fun, component_getter(client), std::forward<Args>(args)...));
+							[version = client->GetClientVersion()](const auto& build_packet) {
+								return build_packet.first == version;
+							});
 
-						if (packet->second != nullptr)
+						if (packet_it == build_packets.end())
-							client->QueuePacket(packet->second, is_ack_required, Client::CLIENT_CONNECTED);
+							packet_it = build_packets.emplace(build_packets.end(), client->ClientVersion(),
+								std::invoke(fun, component_getter(client), std::forward<Args>(args)...));
+
+						if (packet_it->second != nullptr)
+							client->QueuePacket(packet_it->second.get(), is_ack_required, Client::CLIENT_CONNECTED);
 					}
 				}
 			}
-
-			for (auto [_, packet] : build_packets)
-				if (packet != nullptr)
-					delete packet;;
 		}
 	};
 }