From b8a7ac897d790b11d143d905af7c4315cd0270aa Mon Sep 17 00:00:00 2001 From: Aaron Kimbrell Date: Tue, 29 Sep 2026 12:54:34 -0500 Subject: [PATCH] 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 --- dGame/dComponents/PetComponent.cpp | 8 ++- dGame/dComponents/QuickBuildComponent.cpp | 55 +++++++++++++++-- dGame/dComponents/QuickBuildComponent.h | 27 +++++++++ dGame/dUtilities/Preconditions.cpp | 59 ++++++++++++------- dGame/dUtilities/Preconditions.h | 28 +++++++-- dWorldServer/WorldServer.cpp | 4 ++ tests/dGameTests/CMakeLists.txt | 1 + .../dGameTests/PreconditionItemCostTests.cpp | 32 ++++++++++ 8 files changed, 180 insertions(+), 34 deletions(-) create mode 100644 tests/dGameTests/PreconditionItemCostTests.cpp diff --git a/dGame/dComponents/PetComponent.cpp b/dGame/dComponents/PetComponent.cpp index 0a8671c96..9e3296b6b 100644 --- a/dGame/dComponents/PetComponent.cpp +++ b/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(); diff --git a/dGame/dComponents/QuickBuildComponent.cpp b/dGame/dComponents/QuickBuildComponent.cpp index ab5813c5c..f607cb924 100644 --- a/dGame/dComponents/QuickBuildComponent.cpp +++ b/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(); 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(); + 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(); + 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() : 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& callback) { m_QuickBuildCompleteCallbacks.push_back(callback); } diff --git a/dGame/dComponents/QuickBuildComponent.h b/dGame/dComponents/QuickBuildComponent.h index 4a3b9ef87..3e0e311b1 100644 --- a/dGame/dComponents/QuickBuildComponent.h +++ b/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 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 diff --git a/dGame/dUtilities/Preconditions.cpp b/dGame/dUtilities/Preconditions.cpp index d050f8aac..e9aaa0f98 100644 --- a/dGame/dUtilities/Preconditions.cpp +++ b/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(); @@ -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 Precondition::PickItemCost(const std::vector& lots, const uint32_t count, const std::function& lotCount) { + for (const auto lot : lots) { + if (lotCount(static_cast(lot)) >= count) return ItemCost{ static_cast(lot), count }; + } + return std::nullopt; +} + +std::optional Precondition::GetItemCost(Entity* player) const { + if (type != PreconditionType::HasItem || count == 0) return std::nullopt; + auto* const inventoryComponent = player->GetComponent(); + if (!inventoryComponent) return std::nullopt; + return PickItemCost(values, count, [inventoryComponent](const LOT lot) { return inventoryComponent->GetLotCount(lot); }); +} + +std::vector PreconditionExpression::GetItemCosts(Entity* player) const { + std::vector 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); } diff --git a/dGame/dUtilities/Preconditions.h b/dGame/dUtilities/Preconditions.h index 0a1ac70b7..2da9e82cb 100644 --- a/dGame/dUtilities/Preconditions.h +++ b/dGame/dUtilities/Preconditions.h @@ -1,4 +1,6 @@ #pragma once +#include +#include #include #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 GetItemCost(Entity* player) const; + + // The pick GetItemCost makes, given how many of a LOT the player has + static std::optional PickItemCost(const std::vector& lots, uint32_t count, const std::function& 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 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); diff --git a/dWorldServer/WorldServer.cpp b/dWorldServer/WorldServer.cpp index 25b8a8275..31cae21a6 100644 --- a/dWorldServer/WorldServer.cpp +++ b/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()); diff --git a/tests/dGameTests/CMakeLists.txt b/tests/dGameTests/CMakeLists.txt index 2c8ac6fbe..17c6614af 100644 --- a/tests/dGameTests/CMakeLists.txt +++ b/tests/dGameTests/CMakeLists.txt @@ -23,6 +23,7 @@ set(DGAMETEST_SOURCES "SetCurrencySourceTests.cpp" "LootDropPositionTests.cpp" "LootActivityCoinsTests.cpp" + "PreconditionItemCostTests.cpp" "SpiderQueenTests.cpp" "TacArcTests.cpp" ) diff --git a/tests/dGameTests/PreconditionItemCostTests.cpp b/tests/dGameTests/PreconditionItemCostTests.cpp new file mode 100644 index 000000000..92cb9809f --- /dev/null +++ b/tests/dGameTests/PreconditionItemCostTests.cpp @@ -0,0 +1,32 @@ +#include "Preconditions.h" + +#include + +#include + +// 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 Holding(const std::map& 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 })); +}