fix(quickbuild): take precondition items when the build starts, give them back on cancel

A quickbuild's HasItem preconditions took their items only when the build
completed and never gave them back. Live took them when the build started
and gave them back when it was cancelled: the FV Stone Warrior pedestal
(LOT 8551, precondition 99: 5 of LOT 6194) took the items at Building 5
times and added them back with the loot source Quickbuild on each of the 3
cancels in the live captures.

Preconditions now report their item costs instead of removing items while
checking. A quickbuild takes them at the start, gives them back on cancel
or a reset during the build, keeps them on completion, and gives them back
when the builder leaves the world mid-build.

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

dGame/dComponents/PetComponent.cpp

@@ -140,8 +140,12 @@ void PetComponent::OnUse(Entity* originator) {
 		return;
 	}
 
-	if (m_Preconditions.has_value() && !m_Preconditions->Check(originator, true)) {
-		return;
+	if (m_Preconditions.has_value()) {
+		if (!m_Preconditions->Check(originator)) return;
+		// Taming uses up the items its preconditions ask for
+		for (const auto& cost : m_Preconditions->GetItemCosts(originator)) {
+			inventoryComponent->RemoveItem(cost.lot, cost.count, eInventoryType::ALL);
+		}
 	}
 
 	auto* const movementAIComponent = m_Parent->GetComponent<MovementAIComponent>();

dGame/dComponents/QuickBuildComponent.cpp

@@ -11,6 +11,7 @@
 #include "Logger.h"
 #include "CharacterComponent.h"
 #include "MissionComponent.h"
+#include "InventoryComponent.h"
 #include "eMissionTaskType.h"
 #include "eTriggerEventType.h"
 #include "eQuickBuildFailReason.h"
@@ -427,6 +428,8 @@ void QuickBuildComponent::StartQuickBuild(Entity* const user) {
 		SetState(eQuickBuildState::BUILDING);
 		Game::entityManager->SerializeEntity(m_Parent);
 
+		TakeItemCosts(*user);
+
 		auto* movingPlatform = m_Parent->GetComponent<MovingPlatformComponent>();
 		if (movingPlatform != nullptr) {
 			movingPlatform->OnQuickBuildInitilized();
@@ -487,12 +490,9 @@ void QuickBuildComponent::CompleteQuickBuild(Entity* const user) {
 
 	Game::entityManager->SerializeEntity(m_Parent);
 
-	// Removes extra item requirements, isn't live accurate.
-	// In live, all items were removed at the start of the quickbuild, then returned if it was cancelled.
-	// TODO: fix?
-	if (m_Precondition != nullptr) {
-		m_Precondition->Check(user, true);
-	}
+	// The items taken when the build started are used up
+	m_TakenItems.clear();
+	m_TakenItemsFrom = LWOOBJID_EMPTY;
 
 	DespawnActivator();
 
@@ -575,6 +575,8 @@ void QuickBuildComponent::ResetQuickBuild(const bool failed) {
 	notifyState.player = LWOOBJID_EMPTY;
 	notifyState.Send(UNASSIGNED_SYSTEM_ADDRESS);
 
+	RefundItemCosts();
+
 	SetState(eQuickBuildState::RESETTING);
 	SetTimer(0.0f);
 	SetIncompleteTimer(0.0f);
@@ -626,6 +628,8 @@ void QuickBuildComponent::CancelQuickBuild(Entity* const entity, const eQuickBui
 		// Now update the component itself
 		SetState(eQuickBuildState::INCOMPLETE);
 
+		RefundItemCosts();
+
 		// Notify scripts and possible subscribers
 		m_Parent->GetScript()->OnQuickBuildNotifyState(m_Parent, m_State);
 		for (const auto& cb : m_QuickBuildStateCallbacks)
@@ -643,6 +647,45 @@ void QuickBuildComponent::CancelQuickBuild(Entity* const entity, const eQuickBui
 	}
 }
 
+void QuickBuildComponent::CancelBuildsBy(Entity& player) {
+	for (auto* const entity : Game::entityManager->GetEntitiesByComponent(eReplicaComponentType::QUICK_BUILD)) {
+		auto* const quickBuild = entity->GetComponent<QuickBuildComponent>();
+		if (quickBuild && quickBuild->m_State == eQuickBuildState::BUILDING && quickBuild->m_Builder == player.GetObjectID()) {
+			quickBuild->CancelQuickBuild(&player, eQuickBuildFailReason::BUILD_ENDED, true);
+		}
+	}
+}
+
+void QuickBuildComponent::TakeItemCosts(Entity& user) {
+	RefundItemCosts();
+	if (!m_Precondition) return;
+
+	auto* const inventoryComponent = user.GetComponent<InventoryComponent>();
+	if (!inventoryComponent) return;
+
+	// Live took the precondition items (for example the FV Stone Warrior pedestal's 5 Maelstrom Infected Bricks)
+	// when the build started and gave them back when it was cancelled
+	for (const auto& cost : m_Precondition->GetItemCosts(&user)) {
+		if (inventoryComponent->RemoveItem(cost.lot, cost.count, eInventoryType::ALL)) m_TakenItems.push_back(cost);
+	}
+	if (!m_TakenItems.empty()) m_TakenItemsFrom = user.GetObjectID();
+}
+
+void QuickBuildComponent::RefundItemCosts() {
+	if (m_TakenItems.empty()) return;
+
+	auto* const taker = Game::entityManager->GetEntity(m_TakenItemsFrom);
+	auto* const inventoryComponent = taker ? taker->GetComponent<InventoryComponent>() : nullptr;
+	if (inventoryComponent) {
+		for (const auto& cost : m_TakenItems) inventoryComponent->AddItem(cost.lot, cost.count, eLootSourceType::QUICKBUILD);
+	} else {
+		LOG("Quickbuild %llu could not give back the items it took from %llu", m_Parent->GetObjectID(), m_TakenItemsFrom);
+	}
+
+	m_TakenItems.clear();
+	m_TakenItemsFrom = LWOOBJID_EMPTY;
+}
+
 void QuickBuildComponent::AddQuickBuildCompleteCallback(const std::function<void(Entity* user)>& callback) {
 	m_QuickBuildCompleteCallbacks.push_back(callback);
 }

dGame/dComponents/QuickBuildComponent.h

@@ -219,6 +219,12 @@ public:
 	 */
 	void CancelQuickBuild(Entity* const builder, const eQuickBuildFailReason failReason, const bool skipChecks = false);
 
+	/**
+	 * Cancels the quickbuilds the player is building, giving back the items they took. For a player leaving the world.
+	 * @param player the player
+	 */
+	static void CancelBuildsBy(Entity& player);
+
 	void SetState(const eQuickBuildState state) {
 		if (m_State == state) return;
 		m_State = state;
@@ -393,6 +399,27 @@ private:
 	 */
 	PreconditionExpression* m_Precondition = nullptr;
 
+	/**
+	 * The items the preconditions took from the builder when the build started, given back if it is cancelled
+	 */
+	std::vector<ItemCost> m_TakenItems;
+
+	/**
+	 * The player the items in m_TakenItems were taken from
+	 */
+	LWOOBJID m_TakenItemsFrom = LWOOBJID_EMPTY;
+
+	/**
+	 * Takes the item costs of the preconditions from the builder, as live did when a build started
+	 * @param user the builder
+	 */
+	void TakeItemCosts(Entity& user);
+
+	/**
+	 * Gives back the items TakeItemCosts took, as live did when a build was cancelled
+	 */
+	void RefundItemCosts();
+
 	/**
 	 * Starts the quickbuild for a certain entity
 	 * @param user the entity to start the quickbuild

dGame/dUtilities/Preconditions.cpp

@@ -54,7 +54,7 @@ Precondition::Precondition(const uint32_t condition) {
 }
 
 
-bool Precondition::Check(Entity* player, bool evaluateCosts) const {
+bool Precondition::Check(Entity* player) const {
 	if (values.empty()) {
 		return true; // There are very few of these
 	}
@@ -99,7 +99,7 @@ bool Precondition::Check(Entity* player, bool evaluateCosts) const {
 	auto passedAny = false;
 
 	for (const auto value : values) {
-		const auto passed = CheckValue(player, value, evaluateCosts);
+		const auto passed = CheckValue(player, value);
 
 		if (passed && any) {
 			return true;
@@ -118,7 +118,7 @@ bool Precondition::Check(Entity* player, bool evaluateCosts) const {
 }
 
 
-bool Precondition::CheckValue(Entity* player, const uint32_t value, bool evaluateCosts) const {
+bool Precondition::CheckValue(Entity* player, const uint32_t value) const {
 	auto* character = player->GetCharacter();
 	auto [missionComponent, inventoryComponent, destroyableComponent, levelComponent] =
 		player->GetComponentsMut<const MissionComponent, /* not const */ InventoryComponent, const DestroyableComponent, const LevelProgressionComponent>();
@@ -135,11 +135,6 @@ bool Precondition::CheckValue(Entity* player, const uint32_t value, bool evaluat
 	case PreconditionType::ItemNotEquipped:
 		return !inventoryComponent->IsEquipped(value);
 	case PreconditionType::HasItem:
-		if (evaluateCosts) // As far as I know this is only used for quickbuilds, and removal shouldn't actually be handled here.
-		{
-			return inventoryComponent->RemoveItem(value, count, eInventoryType::ALL);
-		}
-
 		return inventoryComponent->GetLotCount(value) >= count;
 	case PreconditionType::DoesNotHaveItem:
 		return inventoryComponent->IsEquipped(value) && count > 0;
@@ -285,12 +280,35 @@ PreconditionExpression::PreconditionExpression(const std::string& conditions) {
 }
 
 
-bool PreconditionExpression::Check(Entity* player, bool evaluateCosts) const {
+std::optional<ItemCost> Precondition::PickItemCost(const std::vector<uint32_t>& lots, const uint32_t count, const std::function<uint32_t(LOT)>& lotCount) {
+	for (const auto lot : lots) {
+		if (lotCount(static_cast<LOT>(lot)) >= count) return ItemCost{ static_cast<LOT>(lot), count };
+	}
+	return std::nullopt;
+}
+
+std::optional<ItemCost> Precondition::GetItemCost(Entity* player) const {
+	if (type != PreconditionType::HasItem || count == 0) return std::nullopt;
+	auto* const inventoryComponent = player->GetComponent<InventoryComponent>();
+	if (!inventoryComponent) return std::nullopt;
+	return PickItemCost(values, count, [inventoryComponent](const LOT lot) { return inventoryComponent->GetLotCount(lot); });
+}
+
+std::vector<ItemCost> PreconditionExpression::GetItemCosts(Entity* player) const {
+	std::vector<ItemCost> costs;
+	for (const auto* expression = this; expression && !expression->empty; expression = expression->next) {
+		const auto cost = Preconditions::Get(expression->condition).GetItemCost(player);
+		if (cost) costs.push_back(*cost);
+	}
+	return costs;
+}
+
+bool PreconditionExpression::Check(Entity* player) const {
 	if (empty) {
 		return true;
 	}
 
-	const auto a = Preconditions::Check(player, condition, evaluateCosts);
+	const auto a = Preconditions::Check(player, condition);
 
 	if (!a) {
 		GameMessages::NotifyClientFailedPrecondition failedPrecondition;
@@ -300,7 +318,7 @@ bool PreconditionExpression::Check(Entity* player, bool evaluateCosts) const {
 		failedPrecondition.Send(player->GetSystemAddress());
 	}
 
-	const auto b = next == nullptr ? true : next->Check(player, evaluateCosts);
+	const auto b = next == nullptr ? true : next->Check(player);
 
 	return m_or ? a || b : a && b;
 }
@@ -310,20 +328,17 @@ PreconditionExpression::~PreconditionExpression() {
 }
 
 
-bool Preconditions::Check(Entity* player, const uint32_t condition, bool evaluateCosts) {
-	Precondition* precondition;
-
+const Precondition& Preconditions::Get(const uint32_t condition) {
 	const auto& index = cache.find(condition);
+	if (index != cache.end()) return *index->second;
 
-	if (index != cache.end()) {
-		precondition = index->second;
-	} else {
-		precondition = new Precondition(condition);
+	auto* const precondition = new Precondition(condition);
+	cache.insert_or_assign(condition, precondition);
+	return *precondition;
+}
 
-		cache.insert_or_assign(condition, precondition);
-	}
-
-	return precondition->Check(player, evaluateCosts);
+bool Preconditions::Check(Entity* player, const uint32_t condition) {
+	return Get(condition).Check(player);
 }
 
 

dGame/dUtilities/Preconditions.h

@@ -1,4 +1,6 @@
 #pragma once
+#include <functional>
+#include <optional>
 #include <vector>
 
 #include "Entity.h"
@@ -33,15 +35,28 @@ enum class PreconditionType
 };
 
 
+struct ItemCost {
+	LOT lot{ LOT_NULL };
+	uint32_t count{ 0 };
+	bool operator==(const ItemCost&) const = default;
+};
+
 class Precondition final
 {
 public:
 	explicit Precondition(uint32_t condition);
 
-	bool Check(Entity* player, bool evaluateCosts = false) const;
+	bool Check(Entity* player) const;
+
+	// The items a HasItem precondition takes as a cost (a quickbuild's): the first of its LOTs the player has enough
+	// of, and how many. Nothing for other types or when the player has none of them.
+	std::optional<ItemCost> GetItemCost(Entity* player) const;
+
+	// The pick GetItemCost makes, given how many of a LOT the player has
+	static std::optional<ItemCost> PickItemCost(const std::vector<uint32_t>& lots, uint32_t count, const std::function<uint32_t(LOT)>& lotCount);
 
 private:
-	bool CheckValue(Entity* player, uint32_t value, bool evaluateCosts = false) const;
+	bool CheckValue(Entity* player, uint32_t value) const;
 
 	PreconditionType type;
 
@@ -56,7 +71,10 @@ class PreconditionExpression final
 public:
 	explicit PreconditionExpression(const std::string& conditions);
 
-	bool Check(Entity* player, bool evaluateCosts = false) const;
+	bool Check(Entity* player) const;
+
+	// The item costs of the HasItem preconditions in this expression that the player meets
+	std::vector<ItemCost> GetItemCosts(Entity* player) const;
 
 	~PreconditionExpression();
 
@@ -73,7 +91,9 @@ private:
 class Preconditions final
 {
 public:
-	static bool Check(Entity* player, uint32_t condition, bool evaluateCosts = false);
+	static bool Check(Entity* player, uint32_t condition);
+
+	static const Precondition& Get(uint32_t condition);
 
 	static PreconditionExpression CreateExpression(const std::string& conditions);
 

dWorldServer/WorldServer.cpp

@@ -72,6 +72,7 @@
 #include "Mail.h"
 #include "TeamManager.h"
 #include "SkillComponent.h"
+#include "QuickBuildComponent.h"
 #include "DestroyableComponent.h"
 #include "Game.h"
 #include "PropertyManagementComponent.h"
@@ -1201,6 +1202,9 @@ void CleanupDisconnectedUser(const SystemAddress& sysAddr) {
 			skillComponent->Reset();
 		}
 
+		// Give back the items a quickbuild took when the build started, before they are saved without them
+		QuickBuildComponent::CancelBuildsBy(*entity);
+
 		if (!savedByMigration) entity->GetCharacter()->SaveXMLToDatabase();
 
 		LOG("Deleting player %llu", entity->GetObjectID());

tests/dGameTests/CMakeLists.txt

@@ -23,6 +23,7 @@ set(DGAMETEST_SOURCES
 	"SetCurrencySourceTests.cpp"
 	"LootDropPositionTests.cpp"
 	"LootActivityCoinsTests.cpp"
+	"PreconditionItemCostTests.cpp"
 	"SpiderQueenTests.cpp"
 	"TacArcTests.cpp"
 )

tests/dGameTests/PreconditionItemCostTests.cpp

@@ -0,0 +1,32 @@
+#include "Preconditions.h"
+
+#include <map>
+
+#include <gtest/gtest.h>
+
+// The items a quickbuild's HasItem precondition takes when the build starts
+// (the FV Stone Warrior pedestal: precondition 99, 5 of LOT 6194).
+
+namespace {
+	std::function<uint32_t(LOT)> Holding(const std::map<LOT, uint32_t>& inventory) {
+		return [inventory](const LOT lot) {
+			const auto it = inventory.find(lot);
+			return it == inventory.end() ? 0u : it->second;
+		};
+	}
+}
+
+TEST(PreconditionItemCostTests, TakesTheCountOfTheHeldLot) {
+	EXPECT_EQ(Precondition::PickItemCost({ 6194 }, 5, Holding({ { 6194, 7 } })), (ItemCost{ 6194, 5 }));
+	EXPECT_EQ(Precondition::PickItemCost({ 6194 }, 5, Holding({ { 6194, 5 } })), (ItemCost{ 6194, 5 }));
+}
+
+TEST(PreconditionItemCostTests, NotEnoughTakesNothing) {
+	EXPECT_FALSE(Precondition::PickItemCost({ 6194 }, 5, Holding({ { 6194, 4 } })));
+	EXPECT_FALSE(Precondition::PickItemCost({ 6194 }, 5, Holding({})));
+}
+
+// A precondition that lists several LOTs is met by any of them: the first one held is taken
+TEST(PreconditionItemCostTests, TakesTheFirstHeldOfSeveralLots) {
+	EXPECT_EQ(Precondition::PickItemCost({ 100, 200, 300 }, 2, Holding({ { 100, 1 }, { 200, 2 }, { 300, 9 } })), (ItemCost{ 200, 2 }));
+}