From ac858dd1e2382d16027835a7c4e0c29541a43cad Mon Sep 17 00:00:00 2001 From: Aaron Kimbrell Date: Sat, 26 Sep 2026 10:28:08 -0500 Subject: [PATCH] fix(wire): UnSmash only writes duration when it is not the default THIS CHANGES THE BYTES SENT TO THE CLIENT (intentionally). UnSmash::Serialize wrote the "duration != 3.0f" flag but guarded the value with "builderID != 3.0f" (a typo), so the 32-bit duration was always written, even after a 0 flag. The LEGO Universe 1.10.64 client (GameMessage::UnSmash::Serialize/Deserialize @ 00dc0e80/00dc0f50, default 3.0f @ 017e66b4) writes it only when duration != 3.0f. The client read the flag and ignored the trailing 32 bits, so this removes 4 junk bytes from default UnSmash messages; messages with a non-default duration are unchanged. Uses WriteOptional/ReadOptional and adds Deserialize. Verified with hand computed golden bits for every flag combination and round trips. Co-Authored-By: Claude Opus 5.5 --- dGame/dGameMessages/GameMessages.cpp | 12 +++-- dGame/dGameMessages/GameMessages.h | 1 + .../dGameMessagesTests/CMakeLists.txt | 1 + .../dGameMessagesTests/UnSmashTests.cpp | 54 +++++++++++++++++++ 4 files changed, 64 insertions(+), 4 deletions(-) create mode 100644 tests/dGameTests/dGameMessagesTests/UnSmashTests.cpp diff --git a/dGame/dGameMessages/GameMessages.cpp b/dGame/dGameMessages/GameMessages.cpp index ef1c939ef..c1eb1cd7b 100644 --- a/dGame/dGameMessages/GameMessages.cpp +++ b/dGame/dGameMessages/GameMessages.cpp @@ -6450,10 +6450,14 @@ namespace GameMessages { } void UnSmash::Serialize(RakNet::BitStream& stream) const { - stream.Write(builderID != LWOOBJID_EMPTY); - if (builderID != LWOOBJID_EMPTY) stream.Write(builderID); - stream.Write(duration != 3.0f); - if (builderID != 3.0f) stream.Write(duration); + // Both fields are optional with a default, like the client's GameMessage::UnSmash::Serialize. + BitStreamUtils::WriteOptional(stream, builderID, LWOOBJID_EMPTY); + BitStreamUtils::WriteOptional(stream, duration, 3.0f); + } + bool UnSmash::Deserialize(RakNet::BitStream& stream) { + VALIDATE_READ(BitStreamUtils::ReadOptional(stream, builderID, LWOOBJID_EMPTY)); + VALIDATE_READ(BitStreamUtils::ReadOptional(stream, duration, 3.0f)); + return true; } void PlayBehaviorSound::Serialize(RakNet::BitStream& stream) const { diff --git a/dGame/dGameMessages/GameMessages.h b/dGame/dGameMessages/GameMessages.h index 2f33c9ff5..54ad3dbc8 100644 --- a/dGame/dGameMessages/GameMessages.h +++ b/dGame/dGameMessages/GameMessages.h @@ -890,6 +890,7 @@ namespace GameMessages { UnSmash() : NetGameMsg(MessageType::Game::UN_SMASH) {} void Serialize(RakNet::BitStream& stream) const override; + bool Deserialize(RakNet::BitStream& stream) override; LWOOBJID builderID{ LWOOBJID_EMPTY }; float duration{ 3.0f }; diff --git a/tests/dGameTests/dGameMessagesTests/CMakeLists.txt b/tests/dGameTests/dGameMessagesTests/CMakeLists.txt index 253539ab9..113f190b2 100644 --- a/tests/dGameTests/dGameMessagesTests/CMakeLists.txt +++ b/tests/dGameTests/dGameMessagesTests/CMakeLists.txt @@ -1,6 +1,7 @@ SET(DGAMEMESSAGES_TESTS "GameMessageTests.cpp" "GameMsgSplitTests.cpp" + "UnSmashTests.cpp" "LegacyGameMessageTests.cpp") # Get the folder name and prepend it to the files above diff --git a/tests/dGameTests/dGameMessagesTests/UnSmashTests.cpp b/tests/dGameTests/dGameMessagesTests/UnSmashTests.cpp new file mode 100644 index 000000000..435b2ebd4 --- /dev/null +++ b/tests/dGameTests/dGameMessagesTests/UnSmashTests.cpp @@ -0,0 +1,54 @@ +#include "GameMessages.h" +#include "GameDependencies.h" +#include "PacketTestUtils.h" + +#include + +using namespace PacketTestUtils; + +// UnSmash layout from the 1.10.64 client (GameMessage::UnSmash::Serialize @ 00dc0e80): +// bit + i64 builderID, the i64 only if builderID != LWOOBJID_EMPTY +// bit + f32 duration, the f32 only if duration != 3.0f +// Expected bits below are hand computed (RakNet writes MSB first). +namespace { + PacketBytes Payload(const GameMessages::UnSmash& msg) { + RakNet::BitStream bitStream; + msg.Serialize(bitStream); + return FromBitStream(bitStream); + } + + GameMessages::UnSmash Make(LWOOBJID builderID, float duration) { + GameMessages::UnSmash msg; + msg.builderID = builderID; + msg.duration = duration; + return msg; + } +} + +TEST(UnSmashTests, DefaultsWriteOnlyTwoFlagBits) { + // Before the fix, duration was written whenever builderID != 3.0f, i.e. always: 34 bits here. + EXPECT_PACKET_EQ(FromHex("00", 2), Payload(Make(LWOOBJID_EMPTY, 3.0f))); +} + +TEST(UnSmashTests, GoldenBytes) { + EXPECT_PACKET_EQ(FromHex("84 03 83 02 82 01 81 00 80", 66), Payload(Make(0x0102030405060708LL, 3.0f))); + EXPECT_PACKET_EQ(FromHex("40 00 30 0f c0", 34), Payload(Make(LWOOBJID_EMPTY, 1.5f))); + EXPECT_PACKET_EQ(FromHex("84 03 83 02 82 01 81 00 c0 00 30 0f c0", 98), Payload(Make(0x0102030405060708LL, 1.5f))); +} + +TEST(UnSmashTests, RoundTrip) { + for (const LWOOBJID builderID : { LWOOBJID_EMPTY, LWOOBJID{ 0x0102030405060708LL } }) { + for (const float duration : { 3.0f, 0.0f, 1.5f }) { + const auto msg = Make(builderID, duration); + RakNet::BitStream bitStream; + msg.Serialize(bitStream); + GameMessages::UnSmash read; + read.builderID = 42; + read.duration = 42.0f; + ASSERT_TRUE(read.Deserialize(bitStream)); + EXPECT_EQ(read.builderID, builderID); + EXPECT_EQ(read.duration, duration); + EXPECT_EQ(bitStream.GetNumberOfUnreadBits(), 0); + } + } +}