fix: property model and brick by brick fixes (#2041)

* test: the server mock keeps a copy of the last bitstream sent

The bitstreams sent are usually on the sender's stack, so the pointer it kept was dangling once Send returned.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: PlaceModelResponse writes the model's rotation

The client's PlaceModelResponse::Deserialize (0x00dc0170) reads the rotation as an optional w, x, y, z quaternion, but the server wrote the 4-byte response after the rotation flag, so with any rotation other than identity the client ran out of data.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: naming a model no longer renames the property

The model info window sends UpdatePropertyOrModelForFilterCheck with isProperty false (client 0x00cc42e0); only the property editor sends it true (0x00c2e710). A model's name and description aren't stored yet.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: keep a model item's blueprint across saves

A brick built model item's config has blueprintid (lowercase), which the saved item XML didn't keep (only blueprintID), so a picked up model lost its blueprint on the next load and could not be placed again. It is now saved as bp.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: write model behaviors only for property and inventory models

The client makes the mutable model component, which reads the behaviors and editing info, only when the object's config has propertyObjectID or inInventory (ObjectLoader2::LoadModelBehaviorsComponent); every other model gets the plain one, which reads only the model block. Writing the behaviors for every model made the client read them as the next component's data.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

* fix: the model vault uses the models' inventory groups, as the client

LWOInventoryComponent_Common::msgUpdateInventoryGroup (0x00c955b0) and msgUpdateInventoryGroupContents (0x00ce1f50) look the inventory type up in m_VaultToPlayerInventory first, which maps VAULT_MODELS to MODELS. The server refused group updates for the model vault.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

dGame/dComponents/InventoryComponent.cpp

@@ -1688,12 +1688,14 @@ bool InventoryComponent::SetSkill(BehaviorSlot slot, uint32_t skillId) {
 void InventoryComponent::UpdateGroup(const GroupUpdate& groupUpdate) {
 	if (groupUpdate.groupId.empty()) return;
 
-	if (groupUpdate.inventory != eInventoryType::BRICKS && groupUpdate.inventory != eInventoryType::MODELS) {
+	// The model vault shares the models' groups (the client's m_VaultToPlayerInventory: VAULT_MODELS -> MODELS)
+	const auto inventory = groupUpdate.inventory == eInventoryType::VAULT_MODELS ? eInventoryType::MODELS : groupUpdate.inventory;
+	if (inventory != eInventoryType::BRICKS && inventory != eInventoryType::MODELS) {
 		LOG("Invalid inventory type for grouping %s", StringifiedEnum::ToString(groupUpdate.inventory).data());
 		return;
 	}
 
-	auto& groups = m_Groups[groupUpdate.inventory];
+	auto& groups = m_Groups[inventory];
 	auto groupItr = std::ranges::find_if(groups, [&groupUpdate](const Group& group) {
 		return group.groupId == groupUpdate.groupId;
 		});

dGame/dComponents/InventoryComponent.h

@@ -409,6 +409,8 @@ public:
 
 	void UpdateGroup(const GroupUpdate& groupUpdate);
 
+	const std::map<eInventoryType, std::vector<Group>>& GetGroups() const { return m_Groups; }
+
 	std::unordered_map<LWOOBJID, DatabasePet>& GetPetsMut() { return m_Pets; };
 
 	void FixInvisibleItems();

dGame/dComponents/ModelComponent.cpp

@@ -156,6 +156,11 @@ void ModelComponent::Serialize(RakNet::BitStream& outBitStream, bool bIsInitialU
 	outBitStream.Write(m_OriginalPosition); // Original position
 	outBitStream.Write(m_OriginalRotation); // Original rotation
 
+	// The client makes the mutable model component, which reads the behaviors and editing info, only for objects whose
+	// config (the settings sent with a model) has propertyObjectID or inInventory; the plain one it makes for every
+	// other model reads only the block above, as live wrote it
+	if (!m_Parent->HasVar(u"propertyObjectID") && !m_Parent->GetVar<bool>(u"inInventory")) return;
+
 	outBitStream.Write1(); // We are writing behavior info
 	outBitStream.Write<uint32_t>(m_Behaviors.size()); // Number of behaviors
 	outBitStream.Write(m_IsPaused); // Is this model paused

dGame/dGameMessages/GameMessages.cpp

@@ -2033,9 +2033,13 @@ void GameMessages::SendPlaceModelResponse(LWOOBJID objectId, const SystemAddress
 		bitStream.Write(response);
 	}
 
+	// The client's PlaceModelResponse::Deserialize (0x00dc0170) reads the rotation as a w, x, y, z quaternion
 	bitStream.Write(rotation != QuatUtils::IDENTITY);
 	if (rotation != QuatUtils::IDENTITY) {
-		bitStream.Write(response);
+		bitStream.Write(rotation.w);
+		bitStream.Write(rotation.x);
+		bitStream.Write(rotation.y);
+		bitStream.Write(rotation.z);
 	}
 
 	if (sysAddr == UNASSIGNED_SYSTEM_ADDRESS) SEND_PACKET_BROADCAST;
@@ -2151,6 +2155,10 @@ void GameMessages::HandleUpdatePropertyOrModelForFilterCheck(RakNet::BitStream&
 		name.push_back(character);
 	}
 
+	// A model's name and description (the property editor's model info window,
+	// LWOPropertyEditorComponent::OnUpdateModelDescription 0x00cc42e0) aren't stored yet; they must not rename the property
+	if (!isProperty) return;
+
 	PropertyManagementComponent::Instance()->UpdatePropertyDetails(GeneralUtils::UTF16ToWTF8(name), GeneralUtils::UTF16ToWTF8(description));
 }
 

dGame/dInventory/Item.cpp

@@ -34,6 +34,8 @@ namespace {
 	const std::map<std::string, std::string> ExtraSettingAbbreviations = {
 		{ "assemblyPartLOTs", "ma" },
 		{ "blueprintID", "b" },
+		// A brick built model item's blueprint, kept so the model can be placed again after a reload
+		{ "blueprintid", "bp" },
 		{ "userModelID", "ui" },
 		{ "userModelName", "un" },
 		{ "userModelDesc", "ud" },

tests/dGameTests/GameDependencies.h

@@ -17,12 +17,16 @@ class dZoneManager;
 class AssetManager;
 
 class dServerMock : public dServer {
-	RakNet::BitStream* sentBitStream = nullptr;
+	// A copy: the bitstreams sent are usually on the sender's stack
+	std::unique_ptr<RakNet::BitStream> sentBitStream;
 public:
 	dServerMock() {};
 	~dServerMock() {};
-	RakNet::BitStream* GetMostRecentBitStream() { return sentBitStream; };
-	void Send(RakNet::BitStream& bitStream, const SystemAddress& sysAddr, bool broadcast) override { sentBitStream = &bitStream; };
+	RakNet::BitStream* GetMostRecentBitStream() { return sentBitStream.get(); };
+	void Send(RakNet::BitStream& bitStream, const SystemAddress& sysAddr, bool broadcast) override {
+		sentBitStream = std::make_unique<RakNet::BitStream>(bitStream.GetData(), bitStream.GetNumberOfBytesUsed(), true);
+		sentBitStream->SetWriteOffset(bitStream.GetNumberOfBitsUsed());
+	};
 	void SetZoneId(unsigned int zoneId) { mZoneID = zoneId; }
 };
 

tests/dGameTests/dComponentsTests/CMakeLists.txt

@@ -1,5 +1,7 @@
 set(DCOMPONENTS_TESTS
 	"DestroyableComponentTests.cpp"
+	"InventoryGroupTests.cpp"
+	"ModelComponentTests.cpp"
 	"PetComponentTests.cpp"
 	"SimplePhysicsComponentTests.cpp"
 	"SavingTests.cpp"

tests/dGameTests/dComponentsTests/InventoryGroupTests.cpp

@@ -0,0 +1,56 @@
+#include "GameDependencies.h"
+
+#include "CDClientManager.h"
+#include "CDComponentsRegistryTable.h"
+#include "Entity.h"
+#include "InventoryComponent.h"
+
+#include <gtest/gtest.h>
+
+class InventoryGroupTests : public GameDependenciesTest {
+protected:
+	std::unique_ptr<Entity> entity;
+	InventoryComponent* inventory{};
+
+	void SetUp() override {
+		SetUpDependencies();
+		CDClientManager::GetEntriesMutable<CDComponentsRegistryTable>().insert_or_assign(static_cast<uint64_t>(info.lot), 0);
+		entity = std::make_unique<Entity>(1, info);
+		inventory = entity->AddComponent<InventoryComponent>(-1);
+	}
+
+	void TearDown() override {
+		entity.reset();
+		TearDownDependencies();
+	}
+
+	void Update(const InventoryComponent::GroupUpdateCommand command, const std::string& id, const eInventoryType type, const LOT lot = LOT_NULL) {
+		InventoryComponent::GroupUpdate update{};
+		update.groupId = id;
+		update.groupName = "name";
+		update.lot = lot;
+		update.inventory = type;
+		update.command = command;
+		inventory->UpdateGroup(update);
+	}
+};
+
+// The client's LWOInventoryComponent_Common::msgUpdateInventoryGroup (0x00c955b0) and msgUpdateInventoryGroupContents
+// (0x00ce1f50) map the model vault to the models inventory (m_VaultToPlayerInventory)
+TEST_F(InventoryGroupTests, TheModelVaultUsesTheModelsGroups) {
+	Update(InventoryComponent::GroupUpdateCommand::ADD, "user_group1", eInventoryType::VAULT_MODELS);
+	Update(InventoryComponent::GroupUpdateCommand::ADD_LOT, "user_group1", eInventoryType::VAULT_MODELS, 3001);
+
+	const auto& groups = inventory->GetGroups();
+	ASSERT_EQ(groups.at(eInventoryType::MODELS).size(), 1u);
+	EXPECT_EQ(groups.at(eInventoryType::MODELS)[0].lots.size(), 1u);
+	EXPECT_FALSE(groups.contains(eInventoryType::VAULT_MODELS));
+}
+
+TEST_F(InventoryGroupTests, OnlyBricksAndModelsHaveGroups) {
+	Update(InventoryComponent::GroupUpdateCommand::ADD, "user_group1", eInventoryType::ITEMS);
+	EXPECT_FALSE(inventory->GetGroups().contains(eInventoryType::ITEMS));
+
+	Update(InventoryComponent::GroupUpdateCommand::ADD, "user_group1", eInventoryType::BRICKS);
+	EXPECT_EQ(inventory->GetGroups().at(eInventoryType::BRICKS).size(), 1u);
+}

tests/dGameTests/dComponentsTests/ModelComponentTests.cpp

@@ -0,0 +1,55 @@
+#include "GameDependencies.h"
+
+#include "BitStream.h"
+#include "CDClientManager.h"
+#include "CDComponentsRegistryTable.h"
+#include "Entity.h"
+#include "GameMessages.h"
+#include "ModelComponent.h"
+
+#include <gtest/gtest.h>
+
+class ModelComponentTests : public GameDependenciesTest {
+protected:
+	void SetUp() override {
+		SetUpDependencies();
+		CDClientManager::GetEntriesMutable<CDComponentsRegistryTable>().insert_or_assign(static_cast<uint64_t>(info.lot), 0);
+	}
+
+	void TearDown() override {
+		TearDownDependencies();
+	}
+
+	// The bits a model's serialization writes
+	static uint32_t SerializedBits(Entity& entity) {
+		auto* model = entity.AddComponent<ModelComponent>(-1);
+		RakNet::BitStream bitStream;
+		model->Serialize(bitStream, true);
+		return bitStream.GetNumberOfBitsUsed();
+	}
+};
+
+// Item block (1 + 64 + 32 + 1), model block (1 + 1 + 32 + 96 + 128)
+constexpr uint32_t ModelBlockBits = 98 + 258;
+// Behavior info: flag, count, paused, and on the initial update the editing info flag
+constexpr uint32_t BehaviorBits = 1 + 32 + 1 + 1;
+
+// The client reads the behaviors only for property and inventory models (ObjectLoader2::LoadModelBehaviorsComponent)
+TEST_F(ModelComponentTests, OtherModelsHaveNoBehaviorInfo) {
+	Entity entity(1, info);
+	EXPECT_EQ(SerializedBits(entity), ModelBlockBits);
+}
+
+TEST_F(ModelComponentTests, PropertyModelsHaveBehaviorInfo) {
+	EntityInfo propertyInfo = info;
+	propertyInfo.settings.Insert<bool>(u"propertyObjectID", true);
+	Entity entity(1, propertyInfo);
+	EXPECT_EQ(SerializedBits(entity), ModelBlockBits + BehaviorBits);
+}
+
+TEST_F(ModelComponentTests, InventoryModelsHaveBehaviorInfo) {
+	EntityInfo inventoryInfo = info;
+	inventoryInfo.settings.Insert<bool>(u"inInventory", true);
+	Entity entity(1, inventoryInfo);
+	EXPECT_EQ(SerializedBits(entity), ModelBlockBits + BehaviorBits);
+}

tests/dGameTests/dGameMessagesTests/GameMessageTests.cpp

@@ -85,6 +85,51 @@ TEST_F(GameMessageTests, SendBlueprintLoadItemResponse) {
 	ASSERT_EQ(bitStream->GetNumberOfUnreadBits(), 0);
 }
 
+/**
+ * @brief PlaceModelResponse is laid out as the client's PlaceModelResponse::Deserialize (0x00dc0170) reads it: every
+ * field optional, the rotation a w, x, y, z quaternion
+ */
+TEST_F(GameMessageTests, SendPlaceModelResponse) {
+	const NiQuaternion rotation = NiQuaternion(0.5f, 0.1f, 0.2f, 0.3f); // w, x, y, z
+	GameMessages::SendPlaceModelResponse(77, UNASSIGNED_SYSTEM_ADDRESS, NiPoint3(1.0f, 2.0f, 3.0f), LWOOBJID_EMPTY, 14, rotation);
+	auto* bitStream = static_cast<dServerMock*>(Game::server)->GetMostRecentBitStream();
+	ASSERT_NE(bitStream, nullptr);
+	bitStream->IgnoreBits(64); // packet header
+
+	LWOOBJID objectId{};
+	MessageType::Game messageId{};
+	bitStream->Read(objectId);
+	bitStream->Read(messageId);
+	EXPECT_EQ(objectId, 77);
+	EXPECT_EQ(messageId, MessageType::Game::PLACE_MODEL_RESPONSE);
+
+	bool hasPosition{}, hasPlaque{}, hasResponse{}, hasRotation{};
+	NiPoint3 position{};
+	int32_t response{};
+	float w{}, x{}, y{}, z{};
+	bitStream->Read(hasPosition);
+	bitStream->Read(position);
+	bitStream->Read(hasPlaque);
+	bitStream->Read(hasResponse);
+	bitStream->Read(response);
+	bitStream->Read(hasRotation);
+	bitStream->Read(w);
+	bitStream->Read(x);
+	bitStream->Read(y);
+	bitStream->Read(z);
+	EXPECT_TRUE(hasPosition);
+	EXPECT_EQ(position, NiPoint3(1.0f, 2.0f, 3.0f));
+	EXPECT_FALSE(hasPlaque);
+	EXPECT_TRUE(hasResponse);
+	EXPECT_EQ(response, 14);
+	EXPECT_TRUE(hasRotation);
+	EXPECT_FLOAT_EQ(w, rotation.w);
+	EXPECT_FLOAT_EQ(x, rotation.x);
+	EXPECT_FLOAT_EQ(y, rotation.y);
+	EXPECT_FLOAT_EQ(z, rotation.z);
+	EXPECT_EQ(bitStream->GetNumberOfUnreadBits(), 0);
+}
+
 TEST_F(GameMessageTests, ControlBehaviorAddStrip) {
 	auto data = ReadFromFile("addStrip");
 	RakNet::BitStream inStream(reinterpret_cast<unsigned char*>(&data[0]), data.length(), true);