chore: foundations for struct based packets and game messages

First step of moving hand written bitstream code to struct based messages (see docs/PacketArchitecture.md). No wire changes. - GameMsg is now only a server-internal event (delivered to handlers registered with RegisterMsg); NetGameMsg is a wire message with Send(sysAddr) (UNASSIGNED broadcasts), SendToClient(sysAddr) (one client, never broadcasts), WritePacket, Serialize, Deserialize and Handle. Mixing them up is now a compile error. NetGameMsgEvent<T> / DeliverLocally carry a wire message to local handlers; loot drops, pickup, the object debugger, GM invisibility and model RequestUse use them. The GM invisibility message keeps its target, so its bytes are unchanged. - Behaviour change: GameMessageHandler logs and drops messages whose Deserialize fails instead of handling them with default fields (the 4 messages already registered: RequestUse, RequestServerObjectInfo, ShootingGalleryFire, PickupItem). - LUBitStream (kept as on main) gets a virtual destructor (Mail deleted derived packets through the base) and WritePacket, also used by ChatPackets::SendRoutedMsg with identical bytes. - BitStreamUtils::WriteOptional/ReadOptional for default-flag fields and WriteLengthPrefixed/ReadLengthPrefixed for length prefixed strings. - Every message ID enumerator (MessageType::*, ServiceType, Mail's wire enums) is pinned with static_asserts, so renumbering or removing one fails to compile. - Tests: dServerMock copies each sent packet (it kept a pointer to the caller's destroyed BitStream); PacketTestUtils.h compares packets bit for bit; helper tests use hand computed golden bytes and equality with the hand written patterns they replace; compile-time checks keep the wire/internal split in place. - docs/PacketArchitecture.md: survey, target architecture, conventions, verification method and migration plan. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-21 20:22:48 +00:00 · 2026-09-26 10:25:27 -05:00
parent 6f3a8d502f
commit 5f60900934
27 changed files with 2796 additions and 81 deletions
--- a/tests/dCommonTests/CMakeLists.txt
+++ b/tests/dCommonTests/CMakeLists.txt
@@ -9,6 +9,8 @@ set(DCOMMONTEST_SOURCES
 	"TestEncoding.cpp"
 	"TestLUString.cpp"
 	"TestLUWString.cpp"
+	"TestBitStreamUtils.cpp"
+	"MessageIdPinTests.cpp"
 	"dCommonDependencies.cpp"
 	"LxfmlTests.cpp"
 )
--- a/tests/dCommonTests/MessageIdPinTests.cpp
+++ b/tests/dCommonTests/MessageIdPinTests.cpp
--- a/tests/dCommonTests/TestBitStreamUtils.cpp
+++ b/tests/dCommonTests/TestBitStreamUtils.cpp
@@ -0,0 +1,114 @@
+#include <gtest/gtest.h>
+
+#include "BitStreamUtils.h"
+
+#include <vector>
+
+namespace {
+	std::vector<uint8_t> Bytes(RakNet::BitStream& bitStream) {
+		return { bitStream.GetData(), bitStream.GetData() + bitStream.GetNumberOfBytesUsed() };
+	}
+}
+
+// The helper must produce exactly what the hand written "size then one char at a time" loops produce.
+TEST(BitStreamUtilsTests, WriteLengthPrefixedU16MatchesPerCharacterLoop) {
+	const std::u16string value = u"sfx/countdown";
+
+	RakNet::BitStream expected;
+	expected.Write(true); // misalign the stream by one bit, like a leading bool field would
+	expected.Write<uint32_t>(value.size());
+	for (const auto character : value) expected.Write(character);
+
+	RakNet::BitStream actual;
+	actual.Write(true);
+	BitStreamUtils::WriteLengthPrefixed(actual, value);
+
+	ASSERT_EQ(actual.GetNumberOfBitsUsed(), expected.GetNumberOfBitsUsed());
+	ASSERT_EQ(Bytes(actual), Bytes(expected));
+}
+
+TEST(BitStreamUtilsTests, WriteLengthPrefixedStringGolden) {
+	RakNet::BitStream bitStream;
+	BitStreamUtils::WriteLengthPrefixed<uint16_t>(bitStream, std::string("ab"));
+	const std::vector<uint8_t> golden = { 0x02, 0x00, 'a', 'b' };
+	ASSERT_EQ(Bytes(bitStream), golden);
+}
+
+TEST(BitStreamUtilsTests, LengthPrefixedRoundTrip) {
+	const std::u16string value = u"Hello ü";
+	RakNet::BitStream bitStream;
+	bitStream.Write(false);
+	BitStreamUtils::WriteLengthPrefixed(bitStream, value);
+	BitStreamUtils::WriteLengthPrefixed(bitStream, std::u16string());
+
+	bool leading = true;
+	std::u16string read = u"garbage";
+	std::u16string empty = u"garbage";
+	ASSERT_TRUE(bitStream.Read(leading));
+	ASSERT_TRUE(BitStreamUtils::ReadLengthPrefixed(bitStream, read));
+	ASSERT_TRUE(BitStreamUtils::ReadLengthPrefixed(bitStream, empty));
+	EXPECT_FALSE(leading);
+	EXPECT_EQ(read, value);
+	EXPECT_TRUE(empty.empty());
+	EXPECT_EQ(bitStream.GetNumberOfUnreadBits(), 0);
+}
+
+TEST(BitStreamUtilsTests, ReadLengthPrefixedRejectsBadLengths) {
+	{
+		RakNet::BitStream bitStream;
+		bitStream.Write<int32_t>(-1);
+		std::u16string out;
+		EXPECT_FALSE(BitStreamUtils::ReadLengthPrefixed<int32_t>(bitStream, out));
+	}
+	{
+		RakNet::BitStream bitStream;
+		bitStream.Write<uint32_t>(11);
+		std::string out;
+		EXPECT_FALSE(BitStreamUtils::ReadLengthPrefixed(bitStream, out, 10));
+	}
+	{
+		// Claims 4 characters but only has 1.
+		RakNet::BitStream bitStream;
+		bitStream.Write<uint32_t>(4);
+		bitStream.Write<char16_t>(u'a');
+		std::u16string out;
+		EXPECT_FALSE(BitStreamUtils::ReadLengthPrefixed(bitStream, out));
+	}
+}
+
+// WriteOptional must match the hand written "flag, then value if not default" pattern.
+TEST(BitStreamUtilsTests, WriteOptionalMatchesHandWrittenPattern) {
+	for (const int32_t value : { 0, -1, 7 }) {
+		RakNet::BitStream expected;
+		expected.Write(value != -1);
+		if (value != -1) expected.Write(value);
+
+		RakNet::BitStream actual;
+		BitStreamUtils::WriteOptional<int32_t>(actual, value, -1);
+		ASSERT_EQ(actual.GetNumberOfBitsUsed(), expected.GetNumberOfBitsUsed());
+		ASSERT_EQ(Bytes(actual), Bytes(expected));
+
+		int32_t read = 12345;
+		ASSERT_TRUE(BitStreamUtils::ReadOptional<int32_t>(actual, read, -1));
+		EXPECT_EQ(read, value);
+		EXPECT_EQ(actual.GetNumberOfUnreadBits(), 0);
+	}
+}
+
+TEST(BitStreamUtilsTests, WriteOptionalGolden) {
+	RakNet::BitStream bitStream;
+	BitStreamUtils::WriteOptional(bitStream, 3.0f, 3.0f); // default: a single 0 bit
+	BitStreamUtils::WriteOptional(bitStream, 1.0f, 3.0f); // 1 bit, then 00 00 80 3f
+	// 0 1 00000000 00000000 10000000 00111111 (MSB first) padded
+	const std::vector<uint8_t> golden = { 0x40, 0x00, 0x20, 0x0f, 0xc0 };
+	ASSERT_EQ(bitStream.GetNumberOfBitsUsed(), 34);
+	ASSERT_EQ(Bytes(bitStream), golden);
+}
+
+TEST(BitStreamUtilsTests, ReadOptionalFailsOnTruncatedValue) {
+	RakNet::BitStream bitStream;
+	bitStream.Write(true);
+	bitStream.Write<uint8_t>(1);
+	uint32_t out{};
+	EXPECT_FALSE(BitStreamUtils::ReadOptional<uint32_t>(bitStream, out, 0));
+}
--- a/tests/dGameTests/CMakeLists.txt
+++ b/tests/dGameTests/CMakeLists.txt
@@ -1,5 +1,7 @@
 set(DGAMETEST_SOURCES
 	"GameDependencies.cpp"
+	"LUBitStreamTests.cpp"
+	"MailIdPinTests.cpp"
 )

 add_subdirectory(dComponentsTests)
--- a/tests/dGameTests/GameDependencies.h
+++ b/tests/dGameTests/GameDependencies.h
@@ -12,17 +12,42 @@
 #include "GameDatabase/TestSQL/TestSQLDatabase.h"
 #include "Database.h"
 #include <gtest/gtest.h>
+#include <vector>

 class dZoneManager;
 class AssetManager;

+// A packet captured by dServerMock::Send. The bytes are copied, so the capture stays valid
+// after the sender's local BitStream goes out of scope.
+struct CapturedPacket {
+	std::vector<uint8_t> bytes;
+	uint32_t bits = 0;
+	SystemAddress sysAddr = UNASSIGNED_SYSTEM_ADDRESS;
+	bool broadcast = false;
+};
+
 class dServerMock : public dServer {
-	RakNet::BitStream* sentBitStream = nullptr;
+	std::vector<CapturedPacket> sentPackets;
+	RakNet::BitStream mostRecent;
 public:
 	dServerMock() {};
 	~dServerMock() {};
-	RakNet::BitStream* GetMostRecentBitStream() { return sentBitStream; };
-	void Send(RakNet::BitStream& bitStream, const SystemAddress& sysAddr, bool broadcast) override { sentBitStream = &bitStream; };
+
+	// Returns a copy of the most recently sent packet, with its read pointer at the start, or nullptr if nothing was sent.
+	RakNet::BitStream* GetMostRecentBitStream() { return sentPackets.empty() ? nullptr : &mostRecent; };
+	const std::vector<CapturedPacket>& GetSentPackets() const { return sentPackets; }
+	void ClearSentPackets() { sentPackets.clear(); mostRecent.Reset(); }
+
+	void Send(RakNet::BitStream& bitStream, const SystemAddress& sysAddr, bool broadcast) override {
+		CapturedPacket& packet = sentPackets.emplace_back();
+		packet.bits = bitStream.GetNumberOfBitsUsed();
+		packet.bytes.assign(bitStream.GetData(), bitStream.GetData() + bitStream.GetNumberOfBytesUsed());
+		packet.sysAddr = sysAddr;
+		packet.broadcast = broadcast;
+
+		mostRecent.Reset();
+		mostRecent.WriteBits(packet.bytes.data(), packet.bits, false);
+	};
 	void SetZoneId(unsigned int zoneId) { mZoneID = zoneId; }
 };

--- a/tests/dGameTests/LUBitStreamTests.cpp
+++ b/tests/dGameTests/LUBitStreamTests.cpp
@@ -0,0 +1,56 @@
+#include <gtest/gtest.h>
+
+#include "BitStreamUtils.h"
+#include "GameDependencies.h"
+#include "PacketTestUtils.h"
+
+#include <vector>
+
+namespace {
+	std::vector<uint8_t> Bytes(RakNet::BitStream& bitStream) {
+		return { bitStream.GetData(), bitStream.GetData() + bitStream.GetNumberOfBytesUsed() };
+	}
+}
+
+namespace {
+	struct TestPacket : public LUBitStream {
+		uint32_t value = 0;
+		TestPacket() : LUBitStream(ServiceType::CHAT, 0x1234u) {}
+		void Serialize(RakNet::BitStream& bitStream) const override { bitStream.Write(value); }
+		bool Deserialize(RakNet::BitStream& bitStream) override { return bitStream.Read(value); }
+	};
+}
+
+TEST(LUBitStreamStructTests, WritePacketGoldenAndRoundTrip) {
+	TestPacket packet;
+	packet.value = 0xAABBCCDD;
+	RakNet::BitStream bitStream;
+	packet.WritePacket(bitStream);
+	// 0x53 | service u16 | packet id u32 | pad u8 | payload
+	const std::vector<uint8_t> golden = { 0x53, 0x02, 0x00, 0x34, 0x12, 0x00, 0x00, 0x00, 0xDD, 0xCC, 0xBB, 0xAA };
+	ASSERT_EQ(Bytes(bitStream), golden);
+
+	TestPacket read;
+	ASSERT_TRUE(read.ReadHeader(bitStream));
+	EXPECT_EQ(read.connectionType, ServiceType::CHAT);
+	EXPECT_EQ(read.internalPacketID, 0x1234u);
+	ASSERT_TRUE(read.Deserialize(bitStream));
+	EXPECT_EQ(read.value, 0xAABBCCDD);
+}
+
+class LUBitStreamTests : public GameDependenciesTest {
+protected:
+	void SetUp() override { SetUpDependencies(); }
+	void TearDown() override { TearDownDependencies(); }
+};
+
+TEST_F(LUBitStreamTests, SendWritesWhatWritePacketWrites) {
+	TestPacket packet;
+	packet.value = 7;
+	RakNet::BitStream expected;
+	packet.WritePacket(expected);
+	const auto sent = PacketTestUtils::Capture([&] { packet.Send(UNASSIGNED_SYSTEM_ADDRESS); });
+	ASSERT_EQ(sent.size(), 1);
+	EXPECT_TRUE(sent[0].broadcast);
+	EXPECT_PACKET_EQ(PacketTestUtils::FromBitStream(expected), PacketTestUtils::FromCapture(sent[0]));
+}
--- a/tests/dGameTests/MailIdPinTests.cpp
+++ b/tests/dGameTests/MailIdPinTests.cpp
@@ -0,0 +1,94 @@
+// GENERATED by a one-off script from the enum definitions at the time this file was added.
+// Message IDs are sent on the wire and are append-only: never renumber, reuse or remove one.
+// If one of these fails to compile, the change to the enum is wrong, not this file.
+// New enumerators may be added freely; add a pin for them here too.
+
+#include "Mail.h"
+#include <cstdint>
+#include <gtest/gtest.h>
+
+// Mail::eMessageID: 21 enumerators
+static_assert(static_cast<int64_t>(Mail::eMessageID::SendRequest) == 0);
+static_assert(static_cast<int64_t>(Mail::eMessageID::SendResponse) == 1);
+static_assert(static_cast<int64_t>(Mail::eMessageID::NotificationResponse) == 2);
+static_assert(static_cast<int64_t>(Mail::eMessageID::DataRequest) == 3);
+static_assert(static_cast<int64_t>(Mail::eMessageID::DataResponse) == 4);
+static_assert(static_cast<int64_t>(Mail::eMessageID::AttachmentCollectRequest) == 5);
+static_assert(static_cast<int64_t>(Mail::eMessageID::AttachmentCollectResponse) == 6);
+static_assert(static_cast<int64_t>(Mail::eMessageID::DeleteRequest) == 7);
+static_assert(static_cast<int64_t>(Mail::eMessageID::DeleteResponse) == 8);
+static_assert(static_cast<int64_t>(Mail::eMessageID::ReadRequest) == 9);
+static_assert(static_cast<int64_t>(Mail::eMessageID::ReadResponse) == 10);
+static_assert(static_cast<int64_t>(Mail::eMessageID::NotificationRequest) == 11);
+static_assert(static_cast<int64_t>(Mail::eMessageID::AuctionCreate) == 12);
+static_assert(static_cast<int64_t>(Mail::eMessageID::AuctionCreationResponse) == 13);
+static_assert(static_cast<int64_t>(Mail::eMessageID::AuctionCancel) == 14);
+static_assert(static_cast<int64_t>(Mail::eMessageID::AuctionCancelResponse) == 15);
+static_assert(static_cast<int64_t>(Mail::eMessageID::AuctionList) == 16);
+static_assert(static_cast<int64_t>(Mail::eMessageID::AuctionListResponse) == 17);
+static_assert(static_cast<int64_t>(Mail::eMessageID::AuctionBid) == 18);
+static_assert(static_cast<int64_t>(Mail::eMessageID::AuctionBidResponse) == 19);
+static_assert(static_cast<int64_t>(Mail::eMessageID::UnknownError) == 20);
+
+// Mail::eSendResponse: 15 enumerators
+static_assert(static_cast<int64_t>(Mail::eSendResponse::Success) == 0);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::NotEnoughCoins) == 1);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::AttachmentNotFound) == 2);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::ItemCannotBeMailed) == 3);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::CannotMailSelf) == 4);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::RecipientNotFound) == 5);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::RecipientDifferentFaction) == 6);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::UnHandled7) == 7);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::ModerationFailure) == 8);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::SenderAccountIsMuted) == 9);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::UnHandled10) == 10);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::RecipientIsIgnored) == 11);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::UnHandled12) == 12);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::RecipientIsFTP) == 13);
+static_assert(static_cast<int64_t>(Mail::eSendResponse::UnknownError) == 14);
+
+// Mail::eDeleteResponse: 5 enumerators
+static_assert(static_cast<int64_t>(Mail::eDeleteResponse::Success) == 0);
+static_assert(static_cast<int64_t>(Mail::eDeleteResponse::HasAttachments) == 1);
+static_assert(static_cast<int64_t>(Mail::eDeleteResponse::NotFound) == 2);
+static_assert(static_cast<int64_t>(Mail::eDeleteResponse::Throttled) == 3);
+static_assert(static_cast<int64_t>(Mail::eDeleteResponse::UnknownError) == 4);
+
+// Mail::eAttachmentCollectResponse: 6 enumerators
+static_assert(static_cast<int64_t>(Mail::eAttachmentCollectResponse::Success) == 0);
+static_assert(static_cast<int64_t>(Mail::eAttachmentCollectResponse::AttachmentNotFound) == 1);
+static_assert(static_cast<int64_t>(Mail::eAttachmentCollectResponse::NoSpaceInInventory) == 2);
+static_assert(static_cast<int64_t>(Mail::eAttachmentCollectResponse::MailNotFound) == 3);
+static_assert(static_cast<int64_t>(Mail::eAttachmentCollectResponse::Throttled) == 4);
+static_assert(static_cast<int64_t>(Mail::eAttachmentCollectResponse::UnknownError) == 5);
+
+// Mail::eNotificationResponse: 9 enumerators
+static_assert(static_cast<int64_t>(Mail::eNotificationResponse::NewMail) == 0);
+static_assert(static_cast<int64_t>(Mail::eNotificationResponse::UnHandled) == 1);
+static_assert(static_cast<int64_t>(Mail::eNotificationResponse::AuctionWon) == 2);
+static_assert(static_cast<int64_t>(Mail::eNotificationResponse::AuctionSold) == 3);
+static_assert(static_cast<int64_t>(Mail::eNotificationResponse::AuctionOutbided) == 4);
+static_assert(static_cast<int64_t>(Mail::eNotificationResponse::AuctionExpired) == 5);
+static_assert(static_cast<int64_t>(Mail::eNotificationResponse::AuctionCancelled) == 6);
+static_assert(static_cast<int64_t>(Mail::eNotificationResponse::AuctionUpdated) == 7);
+static_assert(static_cast<int64_t>(Mail::eNotificationResponse::UnknownError) == 8);
+
+// Mail::eReadResponse: 2 enumerators
+static_assert(static_cast<int64_t>(Mail::eReadResponse::Success) == 0);
+static_assert(static_cast<int64_t>(Mail::eReadResponse::UnknownError) == 1);
+
+// Mail::eAuctionCreateResponse: 5 enumerators
+static_assert(static_cast<int64_t>(Mail::eAuctionCreateResponse::Success) == 0);
+static_assert(static_cast<int64_t>(Mail::eAuctionCreateResponse::NotEnoughMoney) == 1);
+static_assert(static_cast<int64_t>(Mail::eAuctionCreateResponse::ItemNotFound) == 2);
+static_assert(static_cast<int64_t>(Mail::eAuctionCreateResponse::ItemNotSellable) == 3);
+static_assert(static_cast<int64_t>(Mail::eAuctionCreateResponse::UnknownError) == 4);
+
+// Mail::eAuctionCancelResponse: 5 enumerators
+static_assert(static_cast<int64_t>(Mail::eAuctionCancelResponse::NotFound) == 0);
+static_assert(static_cast<int64_t>(Mail::eAuctionCancelResponse::NotYours) == 1);
+static_assert(static_cast<int64_t>(Mail::eAuctionCancelResponse::HasBid) == 2);
+static_assert(static_cast<int64_t>(Mail::eAuctionCancelResponse::NoLongerExists) == 3);
+static_assert(static_cast<int64_t>(Mail::eAuctionCancelResponse::UnknownError) == 4);
+
+TEST(MailIdPinTests, MailEnumValuesArePinned) { SUCCEED(); }
--- a/tests/dGameTests/PacketTestUtils.h
+++ b/tests/dGameTests/PacketTestUtils.h
@@ -0,0 +1,76 @@
+#ifndef PACKETTESTUTILS_H
+#define PACKETTESTUTILS_H
+
+// Helpers for byte-for-byte packet comparisons used while migrating packets / game messages
+// to the struct based architecture (see docs/PacketArchitecture.md).
+
+#include "BitStream.h"
+#include "GameDependencies.h"
+
+#include <cstdio>
+#include <functional>
+#include <string>
+#include <vector>
+
+#include <gtest/gtest.h>
+
+namespace PacketTestUtils {
+	struct PacketBytes {
+		std::vector<uint8_t> bytes;
+		uint32_t bits = 0;
+	};
+
+	inline PacketBytes FromBitStream(const RakNet::BitStream& bitStream) {
+		auto& nonConst = const_cast<RakNet::BitStream&>(bitStream); // GetData is not const in RakNet
+		return { { nonConst.GetData(), nonConst.GetData() + nonConst.GetNumberOfBytesUsed() }, static_cast<uint32_t>(nonConst.GetNumberOfBitsUsed()) };
+	}
+
+	inline PacketBytes FromCapture(const CapturedPacket& packet) {
+		return { packet.bytes, packet.bits };
+	}
+
+	inline std::string ToHex(const PacketBytes& packet) {
+		std::string out;
+		char buf[4];
+		for (size_t i = 0; i < packet.bytes.size(); i++) {
+			std::snprintf(buf, sizeof(buf), "%02x", packet.bytes[i]);
+			if (i != 0) out += ' ';
+			out += buf;
+		}
+		return out + " (" + std::to_string(packet.bits) + " bits)";
+	}
+
+	inline PacketBytes FromHex(const std::string& hex, uint32_t bits = 0) {
+		PacketBytes packet;
+		for (size_t i = 0; i + 1 < hex.size();) {
+			if (hex[i] == ' ') { i++; continue; }
+			packet.bytes.push_back(static_cast<uint8_t>(std::stoul(hex.substr(i, 2), nullptr, 16)));
+			i += 2;
+		}
+		packet.bits = bits == 0 ? static_cast<uint32_t>(packet.bytes.size() * 8) : bits;
+		return packet;
+	}
+
+	// Compares the exact number of bits and every byte, printing both packets as hex on failure.
+	inline ::testing::AssertionResult PacketsEqual(const PacketBytes& expected, const PacketBytes& actual) {
+		if (expected.bits == actual.bits && expected.bytes == actual.bytes) return ::testing::AssertionSuccess();
+		return ::testing::AssertionFailure()
+			<< "\n  expected: " << ToHex(expected)
+			<< "\n  actual:   " << ToHex(actual);
+	}
+
+	// Runs sendFunction against the mock server and returns every packet it sent.
+	inline std::vector<CapturedPacket> Capture(const std::function<void()>& sendFunction) {
+		auto* server = static_cast<dServerMock*>(Game::server);
+		server->ClearSentPackets();
+		sendFunction();
+		auto packets = server->GetSentPackets();
+		server->ClearSentPackets();
+		return packets;
+	}
+}
+
+#define EXPECT_PACKET_EQ(expected, actual) EXPECT_TRUE(PacketTestUtils::PacketsEqual(expected, actual))
+#define ASSERT_PACKET_EQ(expected, actual) ASSERT_TRUE(PacketTestUtils::PacketsEqual(expected, actual))
+
+#endif // PACKETTESTUTILS_H
--- a/tests/dGameTests/dGameMessagesTests/CMakeLists.txt
+++ b/tests/dGameTests/dGameMessagesTests/CMakeLists.txt
@@ -1,5 +1,6 @@
 SET(DGAMEMESSAGES_TESTS
 	"GameMessageTests.cpp"
+	"GameMsgSplitTests.cpp"
 	"LegacyGameMessageTests.cpp")

 # Get the folder name and prepend it to the files above
--- a/tests/dGameTests/dGameMessagesTests/GameMsgSplitTests.cpp
+++ b/tests/dGameTests/dGameMessagesTests/GameMsgSplitTests.cpp
@@ -0,0 +1,91 @@
+#include "GameMessages.h"
+#include "GameDependencies.h"
+#include "PacketTestUtils.h"
+#include "Entity.h"
+#include "EntityManager.h"
+
+#include <type_traits>
+
+#include <gtest/gtest.h>
+
+// Wire messages (NetGameMsg) and server-internal events (GameMsg) are separate types, so a message can only be
+// sent where it belongs. These checks fail to compile if the split is undone.
+namespace {
+	template<typename T>
+	concept CanSendLocally = requires(T msg) { msg.Send(); msg.Send(LWOOBJID{}); };
+
+	template<typename T>
+	concept CanSendToClient = requires(const T msg) { msg.Send(UNASSIGNED_SYSTEM_ADDRESS); };
+
+	template<typename T>
+	concept CanHandleLocally = requires(Entity & entity, T msg) { entity.HandleMsg(msg); };
+
+	static_assert(!std::is_base_of_v<GameMessages::GameMsg, GameMessages::NetGameMsg>);
+	static_assert(!std::is_base_of_v<GameMessages::NetGameMsg, GameMessages::GameMsg>);
+
+	// A wire message can go to a client, never to local handlers.
+	static_assert(CanSendToClient<GameMessages::DropClientLoot>);
+	static_assert(!CanSendLocally<GameMessages::DropClientLoot>);
+	static_assert(!CanHandleLocally<GameMessages::DropClientLoot>);
+
+	// A local event can go to local handlers, never to a client.
+	static_assert(CanSendLocally<GameMessages::GetPosition>);
+	static_assert(!CanSendToClient<GameMessages::GetPosition>);
+	static_assert(CanHandleLocally<GameMessages::GetPosition>);
+
+	// A wire message wrapped for local delivery is a local event.
+	static_assert(CanSendLocally<GameMessages::DropClientLootEvent>);
+	static_assert(!CanSendToClient<GameMessages::DropClientLootEvent>);
+}
+
+class GameMsgSplitTests : public GameDependenciesTest {
+protected:
+	void SetUp() override { SetUpDependencies(); }
+	void TearDown() override { TearDownDependencies(); }
+};
+
+TEST_F(GameMsgSplitTests, EventCarriesACopyWithTheTarget) {
+	auto entity = std::make_unique<Entity>(15, info);
+
+	LWOOBJID seenTarget = LWOOBJID_EMPTY;
+	LOT seenItem = LOT_NULL;
+	entity->RegisterMsg(MessageType::Game::DROP_CLIENT_LOOT, [&](GameMessages::GameMsg& msg) {
+		auto& event = static_cast<GameMessages::DropClientLootEvent&>(msg);
+		seenTarget = event.target;
+		seenItem = event.msg.item;
+		event.msg.item = 1; // handlers only modify the copy
+		return true;
+		});
+
+	GameMessages::DropClientLoot loot;
+	loot.target = 15;
+	loot.item = 1234;
+
+	GameMessages::DropClientLootEvent event(loot);
+	EXPECT_EQ(event.msgId, MessageType::Game::DROP_CLIENT_LOOT);
+	const auto sent = PacketTestUtils::Capture([&] { EXPECT_TRUE(entity->HandleMsg(event)); });
+	EXPECT_TRUE(sent.empty()); // nothing went on the wire
+	EXPECT_EQ(seenTarget, 15);
+	EXPECT_EQ(seenItem, 1234);
+	EXPECT_EQ(loot.item, 1234);
+}
+
+TEST_F(GameMsgSplitTests, DeliverLocallyNeverSendsToAClient) {
+	GameMessages::DropClientLoot loot;
+	loot.target = 0x7777; // no such entity
+	const auto sent = PacketTestUtils::Capture([&] { EXPECT_FALSE(GameMessages::DeliverLocally(loot)); });
+	EXPECT_TRUE(sent.empty());
+}
+
+TEST_F(GameMsgSplitTests, SendToClientNeverBroadcasts) {
+	GameMessages::DropClientLoot loot;
+	for (const auto& address : { SystemAddress(), UNASSIGNED_SYSTEM_ADDRESS }) {
+		const auto sent = PacketTestUtils::Capture([&] { loot.SendToClient(address); });
+		ASSERT_EQ(sent.size(), 1);
+		EXPECT_FALSE(sent[0].broadcast);
+		EXPECT_EQ(sent[0].sysAddr, address);
+	}
+	const auto broadcast = PacketTestUtils::Capture([&] { loot.Send(UNASSIGNED_SYSTEM_ADDRESS); });
+	ASSERT_EQ(broadcast.size(), 1);
+	EXPECT_TRUE(broadcast[0].broadcast);
+}