From 7f67b5b510975efb531d726484b3aeb169a4e88f Mon Sep 17 00:00:00 2001 From: sruon Date: Sun, 3 May 2026 20:11:01 -0600 Subject: [PATCH] Route item usage through transactions --- scripts/enum/item_state.lua | 14 +++ scripts/specs/core/CItem.lua | 5 + scripts/tests/systems/items/item_use.lua | 65 +++++++++++++ src/map/ai/states/item_state.cpp | 31 ++++++- src/map/ai/states/item_state.h | 23 +++-- src/map/entities/charentity.cpp | 10 +- src/map/items/CMakeLists.txt | 4 + src/map/items/item_access.h | 34 +++++++ src/map/items/transaction.cpp | 113 +++++++++++++++++++++++ src/map/items/transaction.h | 69 ++++++++++++++ src/map/items/transactions/item_use.cpp | 100 ++++++++++++++++++++ src/map/items/transactions/item_use.h | 65 +++++++++++++ src/map/lua/lua_item.cpp | 7 ++ src/map/lua/lua_item.h | 2 + 14 files changed, 527 insertions(+), 15 deletions(-) create mode 100644 scripts/enum/item_state.lua create mode 100644 scripts/tests/systems/items/item_use.lua create mode 100644 src/map/items/transaction.cpp create mode 100644 src/map/items/transaction.h create mode 100644 src/map/items/transactions/item_use.cpp create mode 100644 src/map/items/transactions/item_use.h diff --git a/scripts/enum/item_state.lua b/scripts/enum/item_state.lua new file mode 100644 index 00000000000..ac0ed073f93 --- /dev/null +++ b/scripts/enum/item_state.lua @@ -0,0 +1,14 @@ +----------------------------------- +-- Item State +----------------------------------- +xi = xi or {} + +---@enum xi.itemState +xi.itemState = +{ + FREE = 0, + EQUIPPED = 1, + BAZAAR = 2, + PLACED_FURNITURE = 3, + IN_TRANSACTION = 4, +} diff --git a/scripts/specs/core/CItem.lua b/scripts/specs/core/CItem.lua index e6b98ade204..d6eea50ac00 100644 --- a/scripts/specs/core/CItem.lua +++ b/scripts/specs/core/CItem.lua @@ -71,6 +71,11 @@ end function CItem:isSubType(subtype) end +---@nodiscard +---@return xi.itemState +function CItem:state() +end + ---@param reserved integer ---@return nil function CItem:setReservedValue(reserved) diff --git a/scripts/tests/systems/items/item_use.lua b/scripts/tests/systems/items/item_use.lua new file mode 100644 index 00000000000..b0744c171c9 --- /dev/null +++ b/scripts/tests/systems/items/item_use.lua @@ -0,0 +1,65 @@ +describe('Item use', function() + ---@type CClientEntityPair + local player + + before_each(function() + player = xi.test.world:spawnPlayer( + { + job = xi.job.WAR, + level = 75, + zone = xi.zone.SOUTHERN_SAN_DORIA, + }) + end) + + it('idle item is in Free state', function() + player:addItem(xi.item.POTION) + local potion = player:findItem(xi.item.POTION) + assert(potion) + assert(potion:state() == xi.itemState.FREE) + end) + + it('equipped item is in Equipped state', function() + player:addItem(xi.item.BRONZE_SWORD) + player:equipItem(xi.item.BRONZE_SWORD, nil, xi.slot.MAIN) + + local sword = player:findItem(xi.item.BRONZE_SWORD) + assert(sword) + assert(sword:state() == xi.itemState.EQUIPPED) + end) + + it('food is InTransaction during cast then consumed', function() + player:addItem(xi.item.MEAT_MITHKABOB) + local kabob = player:findItem(xi.item.MEAT_MITHKABOB) + assert(kabob) + assert(kabob:state() == xi.itemState.FREE) + + player.actions:useItem(player, kabob:getSlotID(), xi.inventoryLocation.INVENTORY) + + local midUse = player:findItem(xi.item.MEAT_MITHKABOB) + assert(midUse, 'item should still be in inventory mid-cast') + assert(midUse:state() == xi.itemState.IN_TRANSACTION, + 'expected InTransaction during cast, got ' .. tostring(midUse:state())) + + xi.test.world:skipTime(5) + + player.assert + :hasEffect(xi.effect.FOOD) + .no:hasItem(xi.item.MEAT_MITHKABOB) + end) + + it('using charged equipment keeps it in Equipped state', function() + local ok = player:addUsedItem(xi.item.WARP_RING) + assert(ok) + local ring = player:findItem(xi.item.WARP_RING) + assert(ring) + + player:equipItem(xi.item.WARP_RING, nil, xi.slot.RING1) + assert(player:getEquippedItem(xi.slot.RING1)) + + player.actions:useItem(player, ring:getSlotID(), xi.inventoryLocation.INVENTORY) + + local midUse = player:findItem(xi.item.WARP_RING) + assert(midUse) + assert(midUse:state() == xi.itemState.EQUIPPED) + end) +end) diff --git a/src/map/ai/states/item_state.cpp b/src/map/ai/states/item_state.cpp index e90f8e6df2a..07d9154e9ee 100644 --- a/src/map/ai/states/item_state.cpp +++ b/src/map/ai/states/item_state.cpp @@ -29,6 +29,7 @@ #include "action/interrupts.h" #include "enums/item_lockflg.h" #include "item_container.h" +#include "items/transactions/item_use.h" #include "status_effect_container.h" #include "universal_container.h" @@ -118,6 +119,7 @@ CItemState::CItemState(CCharEntity* PEntity, const uint16 targid, const uint8 lo m_PEntity->UContainer->SetType(UCONTAINER_USEITEM); m_PEntity->UContainer->SetItem(0, m_PItem); + tx_ = ItemUseTransaction::start(m_PEntity, m_PItem); m_startPos = m_PEntity->loc.p; m_castTime = m_PItem->getActivationTime(); m_animationTime = m_PItem->getAnimationTime(); @@ -148,6 +150,8 @@ CItemState::CItemState(CCharEntity* PEntity, const uint16 targid, const uint8 lo m_PEntity->pushPacket(m_PEntity); } +CItemState::~CItemState() = default; + void CItemState::UpdateTarget(CBaseEntity* target) { if (target != nullptr) @@ -236,6 +240,15 @@ void CItemState::Cleanup(timer::time_point tick) { m_PEntity->UContainer->Clean(); + // Clear InTransaction before the ITEM_LIST packet below. + if (m_interrupted || !IsCompleted()) + { + if (tx_) + { + tx_->rollback(); + } + } + if (m_PItem && (m_interrupted || !IsCompleted()) && !m_PItem->isType(ITEM_EQUIPMENT)) { m_PItem->setSubType(ITEM_UNLOCKED); @@ -330,7 +343,23 @@ void CItemState::InterruptItem(action_t& action) auto CItemState::FinishItem(action_t& action) -> bool { - return m_PEntity->OnItemFinish(*this, action); + // OnItemFinish returns true to signal the tx should commit (consumable), + // false for equipment / rejected use. + const bool shouldCommit = m_PEntity->OnItemFinish(*this, action); + if (!shouldCommit || !tx_) + { + return false; + } + + const bool willBeDestroyed = m_PItem != nullptr && m_PItem->getQuantity() == 1; + if (!tx_->commit()) + { + ShowWarningFmt("CItemState: ItemUseTransaction commit failed for item {}", + m_PItem ? m_PItem->getID() : 0); + return false; + } + + return willBeDestroyed; } auto CItemState::HasMoved() const -> bool diff --git a/src/map/ai/states/item_state.h b/src/map/ai/states/item_state.h index 130b231ba8a..fdc7c687249 100644 --- a/src/map/ai/states/item_state.h +++ b/src/map/ai/states/item_state.h @@ -23,9 +23,12 @@ #include "state.h" +#include + class CBattleEntity; class CCharEntity; class CItemUsable; +class ItemUseTransaction; struct action_t; @@ -33,6 +36,7 @@ class CItemState : public CState { public: CItemState(CCharEntity* PEntity, uint16 targid, uint8 loc, uint8 slotid); + ~CItemState() override; void UpdateTarget(CBaseEntity* target) override; void UpdateTarget(uint16 targid) override; auto Update(timer::time_point tick) -> bool override; @@ -58,13 +62,14 @@ class CItemState : public CState protected: bool HasMoved() const; - CCharEntity* m_PEntity; - CItemUsable* m_PItem; - uint8 m_location; - uint8 m_slot; - timer::duration m_castTime{}; - timer::duration m_animationTime{}; - position_t m_startPos; - bool m_interrupted{ false }; - bool m_interruptable{ true }; + CCharEntity* m_PEntity; + CItemUsable* m_PItem; + uint8 m_location; + uint8 m_slot; + timer::duration m_castTime{}; + timer::duration m_animationTime{}; + position_t m_startPos; + bool m_interrupted{ false }; + bool m_interruptable{ true }; + std::unique_ptr tx_; }; diff --git a/src/map/entities/charentity.cpp b/src/map/entities/charentity.cpp index e877e539b01..ed9cd89000e 100644 --- a/src/map/entities/charentity.cpp +++ b/src/map/entities/charentity.cpp @@ -2593,6 +2593,8 @@ auto CCharEntity::OnItemFinish(CItemState& state, action_t& action) -> bool actionResult.resolution = ActionResolution::Hit; actionResult.animation = PItem->getAnimationID(); + // TODO: guard charutils::UpdateItem against InTransaction items so a + // Lua delItem inside OnItemUse can't decrement out-of-tx. int32 value = luautils::OnItemUse(this, PTargetFound, PItem, action); actionResult.param = value; @@ -2646,11 +2648,9 @@ auto CCharEntity::OnItemFinish(CItemState& state, action_t& action) -> bool return false; } - // Consumable items - PItem->setSubType(ITEM_UNLOCKED); - const bool willBeDestroyed = PItem->getQuantity() == 1; - charutils::UpdateItem(this, PItem->getLocationID(), PItem->getSlotID(), -1, true); - return willBeDestroyed; + // Consumable items: signal CItemState::FinishItem to commit the ItemUseTransaction + // TODO: Some non-equipment items should not be consumed on use + return true; } CBattleEntity* CCharEntity::IsValidTarget(uint16 targid, uint16 validTargetFlags, std::unique_ptr& errMsg) diff --git a/src/map/items/CMakeLists.txt b/src/map/items/CMakeLists.txt index 7fe9961e661..63807f1a284 100644 --- a/src/map/items/CMakeLists.txt +++ b/src/map/items/CMakeLists.txt @@ -89,5 +89,9 @@ set(ITEM_SOURCES ${CMAKE_CURRENT_SOURCE_DIR}/item.cpp ${CMAKE_CURRENT_SOURCE_DIR}/item.h ${CMAKE_CURRENT_SOURCE_DIR}/item_access.h + ${CMAKE_CURRENT_SOURCE_DIR}/transaction.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/transaction.h + ${CMAKE_CURRENT_SOURCE_DIR}/transactions/item_use.cpp + ${CMAKE_CURRENT_SOURCE_DIR}/transactions/item_use.h PARENT_SCOPE ) diff --git a/src/map/items/item_access.h b/src/map/items/item_access.h index d935d9d3f73..f6e30f90484 100644 --- a/src/map/items/item_access.h +++ b/src/map/items/item_access.h @@ -29,10 +29,44 @@ #include +class Transaction; + namespace xi::items::detail { struct ItemAccess { + // Privileged InTransaction transitions, only callable from Transaction subclasses. + [[nodiscard]] static auto enterTransaction(CItem* item, xi::Badge) -> bool + { + if (item == nullptr) + { + ShowErrorFmt("ItemAccess::enterTransaction: null item"); + return false; + } + + if (item->state() != ItemState::Free) + { + ShowErrorFmt("ItemAccess::enterTransaction: item {} not Free (current={})", + item->getID(), + magic_enum::enum_name(item->state())); + return false; + } + + item->setState(ItemState::InTransaction, xi::Badge{}); + return true; + } + + static void exitTransaction(CItem* item, xi::Badge) + { + if (item == nullptr) + { + ShowErrorFmt("ItemAccess::exitTransaction: null item"); + return; + } + + item->setState(ItemState::Free, xi::Badge{}); + } + [[nodiscard]] static auto mark(CItem* item, const ItemState target) -> bool { if (item == nullptr) diff --git a/src/map/items/transaction.cpp b/src/map/items/transaction.cpp new file mode 100644 index 00000000000..3c8e182df96 --- /dev/null +++ b/src/map/items/transaction.cpp @@ -0,0 +1,113 @@ +/* +=========================================================================== + + Copyright (c) 2026 LandSandBoat Dev Teams + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see http://www.gnu.org/licenses/ + +=========================================================================== +*/ + +#include "transaction.h" + +#include "common/cbasetypes.h" +#include "common/logging.h" +#include "common/types/badge.h" +#include "items/item_access.h" + +#include + +#include +#include + +namespace +{ +auto allocTxId() -> uint64 +{ + static std::atomic counter{ 1 }; + return counter.fetch_add(1, std::memory_order_relaxed); +} +} // namespace + +Transaction::Transaction() +: id_(allocTxId()) +{ +} + +Transaction::~Transaction() +{ + if (this->state_ == TransactionState::Open) + { + ShowErrorFmt("Transaction::~Transaction: tx {} still Open — subclass dtor must call rollbackIfOpen()", this->id_); + std::abort(); + } +} + +auto Transaction::id() const -> uint64 +{ + return this->id_; +} + +auto Transaction::isOpen() const -> bool +{ + return this->state_ == TransactionState::Open; +} + +auto Transaction::commit() -> bool +{ + if (this->state_ != TransactionState::Open) + { + ShowWarningFmt("Transaction::commit: tx {} not Open (state={})", this->id_, magic_enum::enum_name(this->state_)); + return false; + } + + if (!this->doCommit()) + { + ShowWarningFmt("Transaction::commit: doCommit rejected for tx {}", this->id_); + return false; + } + + this->state_ = TransactionState::Committed; + return true; +} + +void Transaction::rollback() +{ + if (this->state_ != TransactionState::Open) + { + ShowWarningFmt("Transaction::rollback: tx {} not Open (state={})", this->id_, magic_enum::enum_name(this->state_)); + return; + } + + this->doRollback(); + this->state_ = TransactionState::RolledBack; +} + +void Transaction::rollbackIfOpen() +{ + if (this->state_ == TransactionState::Open) + { + this->rollback(); + } +} + +auto Transaction::enterTx(CItem* item) -> bool +{ + return xi::items::detail::ItemAccess::enterTransaction(item, xi::Badge{}); +} + +void Transaction::exitTx(CItem* item) +{ + xi::items::detail::ItemAccess::exitTransaction(item, xi::Badge{}); +} diff --git a/src/map/items/transaction.h b/src/map/items/transaction.h new file mode 100644 index 00000000000..0652d433eac --- /dev/null +++ b/src/map/items/transaction.h @@ -0,0 +1,69 @@ +/* +=========================================================================== + + Copyright (c) 2026 LandSandBoat Dev Teams + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see http://www.gnu.org/licenses/ + +=========================================================================== +*/ + +#pragma once + +#include "common/cbasetypes.h" +#include "common/macros.h" + +class CItem; + +enum class TransactionState : uint8 +{ + Open, + Committed, + RolledBack, +}; + +// Open/commit/rollback wrapper for multistep item interactions (item use, NPC trade, synth, dbox). +// Subclasses override doCommit/doRollback/holds. +// Derived must call rollbackIfOpen() in destructor else base aborts. + +class Transaction +{ +public: + Transaction(); + virtual ~Transaction(); + + DISALLOW_COPY_AND_MOVE(Transaction); + + auto id() const -> uint64; + auto isOpen() const -> bool; + + virtual auto holds(const CItem* item) const -> bool = 0; + + [[nodiscard]] auto commit() -> bool; + void rollback(); + +protected: + virtual auto doCommit() -> bool = 0; + virtual void doRollback() = 0; + + void rollbackIfOpen(); + + // Subclass entry points to the badge-gated InTransaction transitions. + [[nodiscard]] static auto enterTx(CItem* item) -> bool; + static void exitTx(CItem* item); + +private: + uint64 id_; + TransactionState state_{ TransactionState::Open }; +}; diff --git a/src/map/items/transactions/item_use.cpp b/src/map/items/transactions/item_use.cpp new file mode 100644 index 00000000000..9d63f2ca7fb --- /dev/null +++ b/src/map/items/transactions/item_use.cpp @@ -0,0 +1,100 @@ +/* +=========================================================================== + + Copyright (c) 2026 LandSandBoat Dev Teams + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see http://www.gnu.org/licenses/ + +=========================================================================== +*/ + +#include "item_use.h" + +#include "entities/charentity.h" +#include "items/item.h" +#include "items/item_usable.h" +#include "utils/charutils.h" + +ItemUseTransaction::ItemUseTransaction(xi::Badge, CCharEntity* player, CItemUsable* item, TakesCustody takesCustody) +: player_(player) +, item_(item) +, takesCustody_(takesCustody) +{ +} + +auto ItemUseTransaction::start(CCharEntity* player, CItemUsable* item) -> std::unique_ptr +{ + if (player == nullptr || item == nullptr) + { + return nullptr; + } + + // Equipment with charges stays in its Equipped role during use; the + // tx just carries the state machine and commit/rollback are no-ops. + const auto takesCustody = item->isType(ITEM_EQUIPMENT) ? TakesCustody::No : TakesCustody::Yes; + + auto tx = std::unique_ptr(new ItemUseTransaction(xi::Badge{}, player, item, takesCustody)); + if (takesCustody && !enterTx(item)) + { + // Item wasn't Free; refuse to start the tx. + return nullptr; + } + + return tx; +} + +auto ItemUseTransaction::holds(const CItem* item) const -> bool +{ + return this->isOpen() && this->takesCustody_ && item != nullptr && this->item_ == item; +} + +void ItemUseTransaction::doRollback() +{ + if (this->takesCustody_ && this->item_ != nullptr) + { + exitTx(this->item_); + // Clear ITEM_LOCKED here so dtor-only paths (disconnect that skips + // CItemState::Cleanup) don't leave the item wedged. + this->item_->setSubType(ITEM_UNLOCKED); + this->item_ = nullptr; + } +} + +auto ItemUseTransaction::doCommit() -> bool +{ + if (!this->takesCustody_) + { + // Charged equipment: nothing to do. + // OnItemFinish handles the charge decrement through the existing path. + return true; + } + + if (this->item_ == nullptr) + { + return false; + } + + // exitTx before UpdateItem: UpdateItem may free the CItem. + const uint8 location = this->item_->getLocationID(); + const uint8 slot = this->item_->getSlotID(); + + exitTx(this->item_); + + // Unlock so a partial-consume remainder is reusable. + this->item_->setSubType(ITEM_UNLOCKED); + + charutils::UpdateItem(this->player_, location, slot, -1, true); + this->item_ = nullptr; + return true; +} diff --git a/src/map/items/transactions/item_use.h b/src/map/items/transactions/item_use.h new file mode 100644 index 00000000000..49febb01003 --- /dev/null +++ b/src/map/items/transactions/item_use.h @@ -0,0 +1,65 @@ +/* +=========================================================================== + + Copyright (c) 2026 LandSandBoat Dev Teams + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + This program is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with this program. If not, see http://www.gnu.org/licenses/ + +=========================================================================== +*/ + +#pragma once + +#include "common/types/badge.h" +#include "common/types/flag.h" +#include "items/transaction.h" + +#include + +class CCharEntity; +class CItem; +class CItemUsable; + +using TakesCustody = xi::Flag; + +// Tx for one item-use (potion, scroll, charged equipment). +// +// Consumables: stamped InTransaction; doCommit decrements the stack, doRollback releases the stamp. +// Charged equipment: no stamp; commit/rollback are no-ops (the charge decrement runs in OnItemFinish). + +class ItemUseTransaction : public Transaction +{ +public: + static auto start(CCharEntity* player, CItemUsable* item) -> std::unique_ptr; + + ItemUseTransaction(xi::Badge, CCharEntity* player, CItemUsable* item, TakesCustody takesCustody); + + ~ItemUseTransaction() override + { + this->rollbackIfOpen(); + } + + DISALLOW_COPY_AND_MOVE(ItemUseTransaction); + + auto holds(const CItem* item) const -> bool override; + +protected: + auto doCommit() -> bool override; + void doRollback() override; + +private: + CCharEntity* player_{}; + CItemUsable* item_{}; + TakesCustody takesCustody_{ TakesCustody::No }; +}; diff --git a/src/map/lua/lua_item.cpp b/src/map/lua/lua_item.cpp index 809eb39d01f..1cf63a7682e 100644 --- a/src/map/lua/lua_item.cpp +++ b/src/map/lua/lua_item.cpp @@ -32,6 +32,7 @@ #include "items/item_linkshell.h" #include "items/item_usable.h" #include "items/item_weapon.h" +#include "map/enums/item_state.h" #include "utils/itemutils.h" CLuaItem::CLuaItem(CItem* PItem) @@ -124,6 +125,11 @@ bool CLuaItem::isSubType(uint8 subtype) return m_readItem->isSubType(static_cast(subtype)); } +auto CLuaItem::state() const -> ItemState +{ + return m_readItem->state(); +} + void CLuaItem::setReservedValue(uint8 reserved) { if (!m_writeItem) @@ -493,6 +499,7 @@ void CLuaItem::Register() SOL_REGISTER("isType", CLuaItem::isType); SOL_REGISTER("setSubType", CLuaItem::setSubType); SOL_REGISTER("isSubType", CLuaItem::isSubType); + SOL_REGISTER("state", CLuaItem::state); SOL_REGISTER("setReservedValue", CLuaItem::setReservedValue); SOL_REGISTER("getReservedValue", CLuaItem::getReservedValue); SOL_REGISTER("getName", CLuaItem::getName); diff --git a/src/map/lua/lua_item.h b/src/map/lua/lua_item.h index 9347eeccee8..3800c734007 100644 --- a/src/map/lua/lua_item.h +++ b/src/map/lua/lua_item.h @@ -25,6 +25,7 @@ #include "common/cbasetypes.h" #include "luautils.h" +enum class ItemState : uint8; class CItem; class CLuaItem { @@ -60,6 +61,7 @@ class CLuaItem void setSubType(uint8 subtype); // set the item's sub type bool isSubType(uint8 subtype); // check the item's sub type + auto state() const -> ItemState; // current ItemState (xi.itemState.*) void setReservedValue(uint8 reserved); // set the item's reserved value uint8 getReservedValue(); // get the item's reserved value