From 0c4806efb814af46ff386072213eecab823b6206 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Thu, 3 Sep 2026 23:00:10 +0200 Subject: [PATCH 1/2] refactor(item): typed refusals across the tier contract The Module refused an item Durable Request with a free-form `Result<(), String>` and the Gateway called any error without "reducer transport disconnected" a gameplay refusal. A 10 second reducer timeout therefore reached the client as `SMSG_INVENTORY_CHANGE_FAILURE` while the Module could still commit the move. One shared `ItemRefusal` enum now defines the contract. Every equip, unequip, move, split and use core returns it; the reducer edge sends only the tag and keeps the human detail in the Module log. The Gateway parses a tag only from a reducer the Module rejected, so a timeout or transport failure stays an error with an unknown outcome and ends the session. Each Refusal reaches the client as its own vanilla inventory result instead of one generic popup. Co-Authored-By: Claude Fable 5.1 --- crates/lyracore-shared/src/item.rs | 83 ++++++++ gateway/src/codec/item.rs | 73 ++++++- gateway/src/stdb/reducers.rs | 131 ++++++++++--- gateway/src/world/handlers/item.rs | 293 ++++++++++++++++++++++------- gateway/src/world/handlers/mod.rs | 4 +- gateway/src/world/mod.rs | 3 +- gateway/src/world/tests.rs | 52 +++-- module/src/debug/mod.rs | 8 +- module/src/gw.rs | 7 +- module/src/items/inventory.rs | 170 ++++++++++++----- module/src/items/mod.rs | 14 ++ module/src/items/ops.rs | 50 +++-- 12 files changed, 701 insertions(+), 187 deletions(-) diff --git a/crates/lyracore-shared/src/item.rs b/crates/lyracore-shared/src/item.rs index b380e7d39..8e45701e6 100644 --- a/crates/lyracore-shared/src/item.rs +++ b/crates/lyracore-shared/src/item.rs @@ -1,5 +1,76 @@ //! Item rules shared by the module and gateway. +/// Why the Module refused an item Durable Request. The tag is the whole reducer error text, so +/// neither tier matches on human prose; the detail stays in the Module log. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ItemRefusal { + /// The named slot holds no item, or the item has no template. + ItemNotFound, + /// No free backpack, bag, or bag-equip slot is left to receive the item. + InventoryFull, + /// The destination slot is out of range, is not an equipment slot, or is already taken. + WrongSlot, + /// The item cannot go on the body: the wrong slot family for its type, or a required level + /// the Character has not reached. + CannotEquip, + /// The Character's class, race, or armor and weapon proficiency does not cover the item. + NoProficiency, + /// The item needs a trained skill rank the Character does not hold. + RequiredSkill, + /// The item needs a reputation standing the Character has not earned. + RequiredReputation, + /// A dead Character cannot act on its inventory. + PlayerDead, + /// The action reaches bank space with no banker in range. + BankUnavailable, + /// The item carries no on-use spell, sits in the bank, or restores a power the class lacks. + ItemNotUsable, + /// The action is blocked for now: a live bandage cooldown, or a cast Gate. + NotRightNow, + /// The Module could not act and has no gameplay reason to name. + Internal, +} + +impl ItemRefusal { + pub const ALL: [Self; 12] = [ + Self::ItemNotFound, + Self::InventoryFull, + Self::WrongSlot, + Self::CannotEquip, + Self::NoProficiency, + Self::RequiredSkill, + Self::RequiredReputation, + Self::PlayerDead, + Self::BankUnavailable, + Self::ItemNotUsable, + Self::NotRightNow, + Self::Internal, + ]; + + pub fn as_tag(self) -> &'static str { + match self { + Self::ItemNotFound => "item:item_not_found", + Self::InventoryFull => "item:inventory_full", + Self::WrongSlot => "item:wrong_slot", + Self::CannotEquip => "item:cannot_equip", + Self::NoProficiency => "item:no_proficiency", + Self::RequiredSkill => "item:required_skill", + Self::RequiredReputation => "item:required_reputation", + Self::PlayerDead => "item:player_dead", + Self::BankUnavailable => "item:bank_unavailable", + Self::ItemNotUsable => "item:item_not_usable", + Self::NotRightNow => "item:not_right_now", + Self::Internal => "item:internal", + } + } + + pub fn parse_tag(tag: &str) -> Option { + Self::ALL + .into_iter() + .find(|refusal| refusal.as_tag() == tag) + } +} + pub mod armor_subclass { pub const MISC: u8 = 0; pub const CLOTH: u8 = 1; @@ -172,6 +243,18 @@ mod tests { /// The nine playable classes (6 and 10 are unused in 1.12). const PLAYABLE: [u8; 9] = [1, 2, 3, 4, 5, 7, 8, 9, 11]; + #[test] + fn every_refusal_tag_round_trips() { + for refusal in ItemRefusal::ALL { + assert_eq!(ItemRefusal::parse_tag(refusal.as_tag()), Some(refusal)); + } + assert_eq!(ItemRefusal::parse_tag("item:"), None); + assert_eq!( + ItemRefusal::parse_tag("gw_move_item reducer timed out after 10s"), + None + ); + } + /// What a class can equip with both level-40 upgrades trained — the auction "usable" filter's /// question, and the shape the pre-training class table was written in. fn ceiling_can_equip(player_class: u8, item_class: u8, item_subclass: u8) -> bool { diff --git a/gateway/src/codec/item.rs b/gateway/src/codec/item.rs index c59aa139a..485d3f6fd 100644 --- a/gateway/src/codec/item.rs +++ b/gateway/src/codec/item.rs @@ -3,7 +3,7 @@ use super::*; -use lyracore_shared::item::{item_class, weapon_subclass, Proficiency}; +use lyracore_shared::item::{item_class, weapon_subclass, ItemRefusal, Proficiency}; /// An item-template row as the gateway reads it from `game_item_template`, flattened for the /// `CMSG_ITEM_QUERY_SINGLE` reply + the item CREATE (items slice-1). Decoupled from the SDK row @@ -260,6 +260,77 @@ pub fn build_inventory_change_failure() -> SMSG_INVENTORY_CHANGE_FAILURE { } } +/// Build `SMSG_INVENTORY_CHANGE_FAILURE` for one typed item Refusal — the vanilla result code the +/// 1.12 client turns into its own error line. The item guids stay zero: the module names the reason, +/// not the rows. `Internal` has no gameplay meaning, so it reads as the client's internal bag error. +/// A required level has no payload-free code (vanilla's level result carries the level itself), so it +/// falls back to the generic equip and use failures. +pub fn build_inventory_refusal(refusal: ItemRefusal) -> SMSG_INVENTORY_CHANGE_FAILURE { + let (item1, item2, bag_type_subclass) = (Guid::new(0), Guid::new(0), 0); + match refusal { + ItemRefusal::ItemNotFound => SMSG_INVENTORY_CHANGE_FAILURE::ItemNotFound { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::InventoryFull => SMSG_INVENTORY_CHANGE_FAILURE::InventoryFull { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::WrongSlot => SMSG_INVENTORY_CHANGE_FAILURE::ItemDoesntGoToSlot { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::CannotEquip => SMSG_INVENTORY_CHANGE_FAILURE::ItemCantBeEquipped { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::NoProficiency => SMSG_INVENTORY_CHANGE_FAILURE::NoRequiredProficiency { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::RequiredSkill => SMSG_INVENTORY_CHANGE_FAILURE::CantEquipSkill { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::RequiredReputation => SMSG_INVENTORY_CHANGE_FAILURE::CantEquipReputation { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::PlayerDead => SMSG_INVENTORY_CHANGE_FAILURE::YouAreDead { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::BankUnavailable => SMSG_INVENTORY_CHANGE_FAILURE::TooFarAwayFromBank { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::ItemNotUsable => SMSG_INVENTORY_CHANGE_FAILURE::YouCanNeverUseThatItem { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::NotRightNow => SMSG_INVENTORY_CHANGE_FAILURE::CantDoRightNow { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::Internal => SMSG_INVENTORY_CHANGE_FAILURE::IntBagError { + item1, + item2, + bag_type_subclass, + }, + } +} + /// Build `SMSG_BUY_FAILED` — the red on-screen error for a rejected vendor purchase. /// Maps the module's Err string to the closest `BuyResult` code (displayed as a toast by the /// 1.12 client). `vendor_guid` is the NPC, `item_entry` is the `c.item` from the client packet. diff --git a/gateway/src/stdb/reducers.rs b/gateway/src/stdb/reducers.rs index 6e112b2a6..645cce5eb 100644 --- a/gateway/src/stdb/reducers.rs +++ b/gateway/src/stdb/reducers.rs @@ -11,7 +11,9 @@ use std::time::{Duration, SystemTime, UNIX_EPOCH}; use super::bindings::*; use super::connection::{call_reducer, recv_reducer_on, reducer_refusal_reason, Coordinator}; use super::views::entity_view; +use crate::world::ItemActionResult; use lyracore_shared::auction::AuctionRefusal; +use lyracore_shared::item::ItemRefusal; use lyracore_shared::trainer::TrainerRefusal; static NEXT_TAXI_REQUEST_ID: OnceLock = OnceLock::new(); @@ -2183,45 +2185,60 @@ impl Coordinator { /// Equip the item in main-inventory `from_slot` (`CMSG_AUTOEQUIP_ITEM`) over the coordinator /// connection. The module resolves the matching equipment slot and gates the required level. /// Rides the coordinator connection as `gw_equip_item`. - pub fn equip_item(&self, _account_id: u64, actor_guid: u64, from_slot: u8) -> Result<()> { - if actor_guid == 0 { - return Err(anyhow!("equip_item: actor_guid unresolved")); - } + pub fn equip_item( + &self, + _account_id: u64, + actor_guid: u64, + from_slot: u8, + ) -> Result { + let Some(actor_guid) = resolved_item_actor("equip_item", actor_guid) else { + return Ok(ItemRefusal::Internal.into()); + }; let coord = self.0.call_pipe(); - call_reducer!( + item_action(call_reducer!( coord.conn.reducers, "gw_equip_item", gw_equip_item_then(actor_guid, from_slot) - ) + )) } /// Unequip the item in equipment `from_slot` to a free backpack slot (`CMSG_AUTOSTORE_BAG_ITEM`) /// over the coordinator connection. The module gates "is equipped" + "backpack has room". - pub fn unequip_item(&self, _account_id: u64, actor_guid: u64, from_slot: u8) -> Result<()> { - if actor_guid == 0 { - return Err(anyhow!("unequip_item: actor_guid unresolved")); - } + pub fn unequip_item( + &self, + _account_id: u64, + actor_guid: u64, + from_slot: u8, + ) -> Result { + let Some(actor_guid) = resolved_item_actor("unequip_item", actor_guid) else { + return Ok(ItemRefusal::Internal.into()); + }; let coord = self.0.call_pipe(); - call_reducer!( + item_action(call_reducer!( coord.conn.reducers, "gw_unequip_item", gw_unequip_item_then(actor_guid, from_slot) - ) + )) } /// Use the consumable in main-inventory `slot` (`CMSG_USE_ITEM`) over the coordinator connection — /// eat/drink/potion/bandage. The module applies the on-use effect (flat heal for slice food) and /// decrements the stack; a gameplay `Err` (no item / not usable) is per-action. - pub fn use_item(&self, _account_id: u64, actor_guid: u64, slot: u8) -> Result<()> { - if actor_guid == 0 { - return Err(anyhow!("use_item: actor_guid unresolved")); - } + pub fn use_item( + &self, + _account_id: u64, + actor_guid: u64, + slot: u8, + ) -> Result { + let Some(actor_guid) = resolved_item_actor("use_item", actor_guid) else { + return Ok(ItemRefusal::Internal.into()); + }; let coord = self.0.call_pipe(); - call_reducer!( + item_action(call_reducer!( coord.conn.reducers, "gw_use_item", gw_use_item_then(actor_guid, slot) - ) + )) } /// Bind the caller's hearthstone home to their current position (`CMSG_GOSSIP_SELECT_OPTION` on an @@ -2247,16 +2264,16 @@ impl Coordinator { actor_guid: u64, from_slot: u8, to_slot: u8, - ) -> Result<()> { - if actor_guid == 0 { - return Err(anyhow!("move_item: actor_guid unresolved")); - } + ) -> Result { + let Some(actor_guid) = resolved_item_actor("move_item", actor_guid) else { + return Ok(ItemRefusal::Internal.into()); + }; let coord = self.0.call_pipe(); - call_reducer!( + item_action(call_reducer!( coord.conn.reducers, "gw_move_item", gw_move_item_then(actor_guid, from_slot, to_slot) - ) + )) } /// Auto-bank/auto-store-bank the item in `slot` (`CMSG_AUTOBANK_ITEM`/`CMSG_AUTOSTORE_BANK_ITEM`) @@ -3111,6 +3128,28 @@ fn trainer_refusal(error: &anyhow::Error) -> Option { reducer_refusal_reason(error).and_then(TrainerRefusal::parse_tag) } +/// The Module's typed item Refusal, on the same rule as the auction family: only a reducer the +/// Module rejected carries a tag, so a timeout or transport failure keeps its unknown outcome. +fn item_action(result: Result<()>) -> Result { + match result { + Ok(()) => Ok(ItemActionResult::Done), + Err(error) => match reducer_refusal_reason(&error).and_then(ItemRefusal::parse_tag) { + Some(refusal) => Ok(refusal.into()), + None => Err(error), + }, + } +} + +/// An item action needs the caller's own entity. Without one there is nothing to request, so the +/// client gets a Refusal rather than a dead session. +fn resolved_item_actor(operation: &str, actor_guid: u64) -> Option { + if actor_guid == 0 { + log::warn!("stdb: {operation} has no resolved actor"); + return None; + } + Some(actor_guid) +} + fn bid_payload_matches(hold: &AuctionBidHold, decision: &AuctionBidDecision) -> bool { hold.operation_id == decision.operation_id && hold.bidder_guid == decision.bidder_guid @@ -3161,6 +3200,50 @@ fn bid_outcome(hold: &AuctionBidHold) -> Result { }) } +#[cfg(test)] +mod item_reducer_tests { + use super::*; + use crate::stdb::connection::ReducerCallError; + + #[test] + fn only_a_rejected_reducer_carries_a_typed_refusal() { + for refusal in ItemRefusal::ALL { + let rejected = Err(anyhow::Error::from(ReducerCallError::Rejected { + operation: "gw_move_item".to_string(), + reason: refusal.as_tag().to_string(), + }) + .context("move phase")); + assert_eq!( + item_action(rejected).unwrap(), + ItemActionResult::Refused(refusal) + ); + } + + assert_eq!(item_action(Ok(())).unwrap(), ItemActionResult::Done); + + let not_refusals = [ + anyhow::Error::from(ReducerCallError::fatal( + "gw_move_item reducer timed out after 10s".to_string(), + )), + anyhow::Error::from(ReducerCallError::fatal( + "gw_use_item reducer failed: transport disconnected".to_string(), + )), + anyhow::Error::from(ReducerCallError::Rejected { + operation: "gw_equip_item".to_string(), + reason: "operator only".to_string(), + }), + anyhow!( + "wrapped text that mentions {}", + ItemRefusal::Internal.as_tag() + ), + ]; + for error in not_refusals { + let text = format!("{error:#}"); + assert!(item_action(Err(error)).is_err(), "{text}"); + } + } +} + #[cfg(test)] mod auction_reducer_tests { use super::*; diff --git a/gateway/src/world/handlers/item.rs b/gateway/src/world/handlers/item.rs index de8e63fd8..5254c737d 100644 --- a/gateway/src/world/handlers/item.rs +++ b/gateway/src/world/handlers/item.rs @@ -2,24 +2,64 @@ use super::super::*; use super::quest::{item_started_quest, QuestActionPlayer, QuestActionStore}; +use lyracore_shared::item::ItemRefusal; const MAIN_BAG: u8 = 255; // INVENTORY_SLOT_BAG_0 — backpack + equipped slots share this pseudo-bag const EQUIP_SLOT_END: u8 = 18; // EQUIPMENT_SLOT_END — last equipment slot (main-hand=15, off=16…) +/// How the Module answered one item Durable Request. A Refusal is an outcome; a timeout, transport, +/// or SDK failure stays an `Err` with an unknown durable result and ends the session. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum ItemActionResult { + Done, + Refused(ItemRefusal), +} + +impl From for ItemActionResult { + fn from(refusal: ItemRefusal) -> Self { + Self::Refused(refusal) + } +} + pub(crate) trait ItemActionStore: Send + Sync { - fn equip_item(&self, account_id: u64, actor_guid: u64, from_slot: u8) -> Result<()>; - fn unequip_item(&self, account_id: u64, actor_guid: u64, from_slot: u8) -> Result<()>; - fn move_item(&self, account_id: u64, actor_guid: u64, from_slot: u8, to_slot: u8) - -> Result<()>; - fn use_item(&self, account_id: u64, actor_guid: u64, slot: u8) -> Result<()>; + fn equip_item( + &self, + account_id: u64, + actor_guid: u64, + from_slot: u8, + ) -> Result; + fn unequip_item( + &self, + account_id: u64, + actor_guid: u64, + from_slot: u8, + ) -> Result; + fn move_item( + &self, + account_id: u64, + actor_guid: u64, + from_slot: u8, + to_slot: u8, + ) -> Result; + fn use_item(&self, account_id: u64, actor_guid: u64, slot: u8) -> Result; } impl ItemActionStore for crate::stdb::Coordinator { - fn equip_item(&self, account_id: u64, actor_guid: u64, from_slot: u8) -> Result<()> { + fn equip_item( + &self, + account_id: u64, + actor_guid: u64, + from_slot: u8, + ) -> Result { crate::stdb::Coordinator::equip_item(self, account_id, actor_guid, from_slot) } - fn unequip_item(&self, account_id: u64, actor_guid: u64, from_slot: u8) -> Result<()> { + fn unequip_item( + &self, + account_id: u64, + actor_guid: u64, + from_slot: u8, + ) -> Result { crate::stdb::Coordinator::unequip_item(self, account_id, actor_guid, from_slot) } @@ -29,11 +69,11 @@ impl ItemActionStore for crate::stdb::Coordinator { actor_guid: u64, from_slot: u8, to_slot: u8, - ) -> Result<()> { + ) -> Result { crate::stdb::Coordinator::move_item(self, account_id, actor_guid, from_slot, to_slot) } - fn use_item(&self, account_id: u64, actor_guid: u64, slot: u8) -> Result<()> { + fn use_item(&self, account_id: u64, actor_guid: u64, slot: u8) -> Result { crate::stdb::Coordinator::use_item(self, account_id, actor_guid, slot) } } @@ -49,39 +89,25 @@ pub(crate) enum ItemActionOutcome { PassThrough(ClientOpcodeMessage), } -#[derive(Clone, Copy, Debug, PartialEq, Eq)] -enum ItemActionErrorClass { - GameplayRefusal, - Fatal, -} - -fn classify_item_action_error(error: &anyhow::Error) -> ItemActionErrorClass { - if error - .chain() - .any(|cause| cause.to_string().contains("reducer transport disconnected")) - { - ItemActionErrorClass::Fatal - } else { - ItemActionErrorClass::GameplayRefusal - } -} - +/// A Refusal answers the client with its own result code; a completed action answers nothing. fn inventory_action_outbound( account_id: u64, operation: &str, - result: Result<()>, -) -> Result> { + result: ItemActionResult, +) -> Vec { match result { - Ok(()) => Ok(Vec::new()), - Err(e) if classify_item_action_error(&e) == ItemActionErrorClass::GameplayRefusal => { - log::debug!("world: {operation} rejected (account {account_id}): {e}"); - Ok(vec![Outbound::One( + ItemActionResult::Done => Vec::new(), + ItemActionResult::Refused(refusal) => { + log::debug!( + "world: {operation} refused (account {account_id}): {}", + refusal.as_tag() + ); + vec![Outbound::One( ServerOpcodeMessage::SMSG_INVENTORY_CHANGE_FAILURE(Box::new( - codec::build_inventory_change_failure(), + codec::build_inventory_refusal(refusal), )), - )]) + )] } - Err(e) => Err(e), } } @@ -101,8 +127,8 @@ pub(crate) fn dispatch_item_action { @@ -127,8 +153,8 @@ pub(crate) fn dispatch_item_action { @@ -151,8 +177,8 @@ pub(crate) fn dispatch_item_action { @@ -191,8 +217,8 @@ pub(crate) fn dispatch_item_action inventory_action_outbound( player.account_id, "use_item", - store.use_item(player.account_id, player.self_guid.unwrap_or(0), c.bag_slot), - )?, + store.use_item(player.account_id, player.self_guid.unwrap_or(0), c.bag_slot)?, + ), }; Ok(ItemActionOutcome::Handled { outbound }) } @@ -225,33 +251,49 @@ mod tests { unequip_requests: Mutex>, move_requests: Mutex>, use_requests: Mutex>, - equip_error: Option, - unequip_error: Option, - move_error: Option, - use_error: Option, + equip_result: Option>, + unequip_result: Option>, + move_result: Option>, + use_result: Option>, start_quest: Option<(u64, u32)>, quest_detail: Option, } + /// The Coordinator answers either a typed Refusal or a failure with an unknown durable outcome, + /// so the Fake answers in exactly those two shapes. + fn answer(canned: &Option>) -> Result { + match canned { + None => Ok(ItemActionResult::Done), + Some(Ok(result)) => Ok(*result), + Some(Err(failure)) => Err(anyhow::anyhow!("{failure}")), + } + } + impl ItemActionStore for InMemoryItemActions { - fn equip_item(&self, account_id: u64, actor_guid: u64, from_slot: u8) -> Result<()> { + fn equip_item( + &self, + account_id: u64, + actor_guid: u64, + from_slot: u8, + ) -> Result { self.equip_requests .lock() .unwrap() .push((account_id, actor_guid, from_slot)); - self.equip_error - .as_ref() - .map_or_else(|| Ok(()), |error| Err(anyhow::anyhow!("{error}"))) + answer(&self.equip_result) } - fn unequip_item(&self, account_id: u64, actor_guid: u64, from_slot: u8) -> Result<()> { + fn unequip_item( + &self, + account_id: u64, + actor_guid: u64, + from_slot: u8, + ) -> Result { self.unequip_requests .lock() .unwrap() .push((account_id, actor_guid, from_slot)); - self.unequip_error - .as_ref() - .map_or_else(|| Ok(()), |error| Err(anyhow::anyhow!("{error}"))) + answer(&self.unequip_result) } fn move_item( @@ -260,24 +302,20 @@ mod tests { actor_guid: u64, from_slot: u8, to_slot: u8, - ) -> Result<()> { + ) -> Result { self.move_requests .lock() .unwrap() .push((account_id, actor_guid, from_slot, to_slot)); - self.move_error - .as_ref() - .map_or_else(|| Ok(()), |error| Err(anyhow::anyhow!("{error}"))) + answer(&self.move_result) } - fn use_item(&self, account_id: u64, actor_guid: u64, slot: u8) -> Result<()> { + fn use_item(&self, account_id: u64, actor_guid: u64, slot: u8) -> Result { self.use_requests .lock() .unwrap() .push((account_id, actor_guid, slot)); - self.use_error - .as_ref() - .map_or_else(|| Ok(()), |error| Err(anyhow::anyhow!("{error}"))) + answer(&self.use_result) } } @@ -421,6 +459,13 @@ mod tests { )); } + fn handled_outbound(outcome: ItemActionOutcome) -> Vec { + match outcome { + ItemActionOutcome::Handled { outbound } => outbound, + ItemActionOutcome::PassThrough(_) => panic!("an item action must be handled here"), + } + } + fn assert_no_durable_requests(actions: &InMemoryItemActions) { assert!(actions.equip_requests.lock().unwrap().is_empty()); assert!(actions.unequip_requests.lock().unwrap().is_empty()); @@ -446,7 +491,7 @@ mod tests { #[test] fn equip_refusal_returns_inventory_failure_without_ending_the_session() { let actions = InMemoryItemActions { - equip_error: Some("required level not met".into()), + equip_result: Some(Ok(ItemRefusal::CannotEquip.into())), ..Default::default() }; @@ -480,7 +525,7 @@ mod tests { #[test] fn unequip_refusal_returns_inventory_failure_without_ending_the_session() { let actions = InMemoryItemActions { - unequip_error: Some("backpack full".into()), + unequip_result: Some(Ok(ItemRefusal::InventoryFull.into())), ..Default::default() }; @@ -560,7 +605,7 @@ mod tests { #[test] fn move_refusal_returns_inventory_failure_without_ending_the_session() { let actions = InMemoryItemActions { - move_error: Some("cannot equip that there".into()), + move_result: Some(Ok(ItemRefusal::WrongSlot.into())), ..Default::default() }; @@ -595,7 +640,7 @@ mod tests { #[test] fn swap_refusal_returns_inventory_failure_without_ending_the_session() { let actions = InMemoryItemActions { - move_error: Some("cannot equip that there".into()), + move_result: Some(Ok(ItemRefusal::WrongSlot.into())), ..Default::default() }; @@ -629,7 +674,7 @@ mod tests { #[test] fn use_refusal_returns_inventory_failure_without_ending_the_session() { let actions = InMemoryItemActions { - use_error: Some("item is not usable".into()), + use_result: Some(Ok(ItemRefusal::ItemNotUsable.into())), ..Default::default() }; @@ -750,10 +795,116 @@ mod tests { ); } + #[test] + fn every_module_refusal_has_one_client_result_code() { + use wow_world_messages::vanilla::SMSG_INVENTORY_CHANGE_FAILURE as Failure; + use wow_world_messages::Guid; + let (item1, item2, bag_type_subclass) = (Guid::new(0), Guid::new(0), 0u8); + // The vanilla 1.12 result each Refusal must reach the client as. + let expected = |refusal| match refusal { + ItemRefusal::ItemNotFound => Failure::ItemNotFound { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::InventoryFull => Failure::InventoryFull { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::WrongSlot => Failure::ItemDoesntGoToSlot { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::CannotEquip => Failure::ItemCantBeEquipped { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::NoProficiency => Failure::NoRequiredProficiency { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::RequiredSkill => Failure::CantEquipSkill { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::RequiredReputation => Failure::CantEquipReputation { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::PlayerDead => Failure::YouAreDead { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::BankUnavailable => Failure::TooFarAwayFromBank { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::ItemNotUsable => Failure::YouCanNeverUseThatItem { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::NotRightNow => Failure::CantDoRightNow { + item1, + item2, + bag_type_subclass, + }, + ItemRefusal::Internal => Failure::IntBagError { + item1, + item2, + bag_type_subclass, + }, + }; + for refusal in ItemRefusal::ALL { + let actions = InMemoryItemActions { + equip_result: Some(Ok(refusal.into())), + ..Default::default() + }; + + let outbound = handled_outbound( + dispatch_item_action(&actions, player(), equip(MAIN_BAG)).unwrap(), + ); + + assert!( + matches!( + outbound.as_slice(), + [Outbound::One(ServerOpcodeMessage::SMSG_INVENTORY_CHANGE_FAILURE(failure))] + if **failure == expected(refusal) + ), + "{refusal:?}" + ); + } + } + + #[test] + fn a_reducer_timeout_is_not_answered_as_a_refusal() { + let actions = InMemoryItemActions { + move_result: Some(Err("gw_move_item reducer timed out after 10s".into())), + ..Default::default() + }; + + let error = match dispatch_item_action(&actions, player(), swap(MAIN_BAG)) { + Err(error) => error, + Ok(_) => panic!("an unknown move outcome must end the session"), + }; + + assert!(error.to_string().contains("timed out")); + } + #[test] fn reducer_transport_failure_is_session_fatal() { let actions = InMemoryItemActions { - equip_error: Some("equip_item reducer transport disconnected: channel closed".into()), + equip_result: Some(Err( + "equip_item reducer transport disconnected: channel closed".into(), + )), ..Default::default() }; diff --git a/gateway/src/world/handlers/mod.rs b/gateway/src/world/handlers/mod.rs index cac925923..1a8820869 100644 --- a/gateway/src/world/handlers/mod.rs +++ b/gateway/src/world/handlers/mod.rs @@ -42,7 +42,9 @@ pub(crate) use cast::{dispatch_cast, CastOutcome, CastPlayer, CastStore, CastTra pub(crate) use char::handle_char; pub(crate) use combat::handle_combat; pub(crate) use duel::{dispatch_duel_action, DuelActionOutcome, DuelActionPlayer, DuelActionStore}; -pub(crate) use item::{dispatch_item_action, ItemActionOutcome, ItemActionPlayer, ItemActionStore}; +pub(crate) use item::{ + dispatch_item_action, ItemActionOutcome, ItemActionPlayer, ItemActionResult, ItemActionStore, +}; #[cfg(test)] pub(crate) use loot::LootWindowRequestStatus; pub(crate) use loot::{ diff --git a/gateway/src/world/mod.rs b/gateway/src/world/mod.rs index 5b2db3b60..e990826c5 100644 --- a/gateway/src/world/mod.rs +++ b/gateway/src/world/mod.rs @@ -62,7 +62,8 @@ use handlers::{ }; pub(crate) use handlers::{ zone_weather_message, AuctionBrowseRequest, AuctionPage, AuctionQuery, CreateAuctionOutcome, - CreateAuctionRequest, PlaceBidOutcome, PlaceBidRequest, TrainerBuyOutcome, WeatherStore, + CreateAuctionRequest, ItemActionResult, PlaceBidOutcome, PlaceBidRequest, TrainerBuyOutcome, + WeatherStore, }; use login_queue::{Admission, LoginQueue}; use social::handle_social; diff --git a/gateway/src/world/tests.rs b/gateway/src/world/tests.rs index 389e4801d..8f87e2a9e 100644 --- a/gateway/src/world/tests.rs +++ b/gateway/src/world/tests.rs @@ -5,6 +5,7 @@ use super::handlers::{ }; use super::*; use crate::read_deadline::{DeadlineClock, PreAuthDeadline}; +use lyracore_shared::item::ItemRefusal; use std::cell::Cell; use std::io::Cursor; use std::os::unix::net::UnixStream; @@ -3101,19 +3102,36 @@ impl VendorActionStore for InMemoryStore { } } -impl ItemActionStore for InMemoryStore { - fn equip_item(&self, _account_id: u64, _self_guid: u64, _from_slot: u8) -> Result<()> { +impl InMemoryStore { + /// The Coordinator answers a Refusal tag as an outcome and anything else as a failure with an + /// unknown durable result, so `trade_error` reaches the item family the same way. + fn canned_item_action(&self) -> Result { match &self.trade_error { - Some(e) => Err(anyhow!("{e}")), - None => Ok(()), + None => Ok(ItemActionResult::Done), + Some(e) => ItemRefusal::parse_tag(e) + .map(ItemActionResult::from) + .ok_or_else(|| anyhow!("{e}")), } } +} - fn unequip_item(&self, _account_id: u64, _self_guid: u64, _from_slot: u8) -> Result<()> { - match &self.trade_error { - Some(e) => Err(anyhow!("{e}")), - None => Ok(()), - } +impl ItemActionStore for InMemoryStore { + fn equip_item( + &self, + _account_id: u64, + _self_guid: u64, + _from_slot: u8, + ) -> Result { + self.canned_item_action() + } + + fn unequip_item( + &self, + _account_id: u64, + _self_guid: u64, + _from_slot: u8, + ) -> Result { + self.canned_item_action() } fn move_item( @@ -3122,19 +3140,13 @@ impl ItemActionStore for InMemoryStore { _self_guid: u64, _from_slot: u8, _to_slot: u8, - ) -> Result<()> { - match &self.trade_error { - Some(e) => Err(anyhow!("{e}")), - None => Ok(()), - } + ) -> Result { + self.canned_item_action() } - fn use_item(&self, _account_id: u64, _self_guid: u64, slot: u8) -> Result<()> { + fn use_item(&self, _account_id: u64, _self_guid: u64, slot: u8) -> Result { self.used_items.lock().unwrap().push(slot); - match &self.trade_error { - Some(e) => Err(anyhow!("{e}")), - None => Ok(()), - } + self.canned_item_action() } } @@ -6159,7 +6171,7 @@ fn equip_item_err_sends_smsg_inventory_change_failure() { // SMSG_INVENTORY_CHANGE_FAILURE frame and the session keeps serving the next action. let store = std::sync::Arc::new(InMemoryStore { login_entity: Some(warrior_entity()), - trade_error: Some("required level not met".into()), + trade_error: Some(ItemRefusal::CannotEquip.as_tag().into()), ..tester_store(7) }); let (mut client, mut c_enc, mut c_dec, server) = enter_world(store, 1); diff --git a/module/src/debug/mod.rs b/module/src/debug/mod.rs index 475da4b90..5f0df16ae 100644 --- a/module/src/debug/mod.rs +++ b/module/src/debug/mod.rs @@ -550,7 +550,7 @@ pub fn debug_spawn_ground_area( /// without the mouse-only inventory UI. Delegates to the SAME `apply_item_use` the player reducer uses. #[reducer] pub fn debug_use_item(ctx: &ReducerContext, character_guid: u64, slot: u8) -> Result<(), String> { - crate::actor::use_item(ctx, character_guid, slot) + crate::actor::use_item(ctx, character_guid, slot).map_err(crate::items::refused) } /// Grant `count`x item `entry` into `character_guid`'s bags — the test lever for item-dependent flows @@ -592,6 +592,7 @@ pub fn debug_move_item( to_slot: u8, ) -> Result<(), String> { crate::items::apply_item_move(ctx, character_guid, from_slot, to_slot) + .map_err(crate::items::refused) } /// Equip `character_guid`'s item in inventory `from_slot` into its matching `EQUIPMENT_SLOT_*` (auto- @@ -604,7 +605,7 @@ pub fn debug_equip_item( character_guid: u64, from_slot: u8, ) -> Result<(), String> { - crate::actor::equip_item(ctx, character_guid, from_slot) + crate::actor::equip_item(ctx, character_guid, from_slot).map_err(crate::items::refused) } /// Unequip `character_guid`'s item in equipment `from_slot` (0..=18) into the first free backpack slot — @@ -615,7 +616,7 @@ pub fn debug_unequip_item( character_guid: u64, from_slot: u8, ) -> Result<(), String> { - crate::items::apply_unequip_item(ctx, character_guid, from_slot) + crate::items::apply_unequip_item(ctx, character_guid, from_slot).map_err(crate::items::refused) } /// Auto-bank/auto-store-bank `character_guid`'s item in `slot` — drives the right-click-to-bank and @@ -982,6 +983,7 @@ pub fn debug_split_item( to_slot: u8, ) -> Result<(), String> { crate::items::apply_item_split(ctx, character_guid, slot, count, to_slot) + .map_err(crate::items::refused) } /// Arm a melee engagement `attacker_guid` → `target_guid` directly (insert/retarget a `game_melee_attack` diff --git a/module/src/gw.rs b/module/src/gw.rs index 2182f9f0c..2e6cec9c8 100644 --- a/module/src/gw.rs +++ b/module/src/gw.rs @@ -397,7 +397,7 @@ pub fn gw_turn_in_quest( pub fn gw_use_item(ctx: &ReducerContext, actor_guid: u64, slot: u8) -> Result<(), String> { require_operator(ctx)?; actor(ctx, actor_guid)?; - crate::actor::use_item(ctx, actor_guid, slot) + crate::actor::use_item(ctx, actor_guid, slot).map_err(crate::items::refused) } /// Resolve a spell whose explicit target is an owned inventory item. The effect kind selects the @@ -454,7 +454,7 @@ pub fn gw_sell_item( pub fn gw_equip_item(ctx: &ReducerContext, actor_guid: u64, from_slot: u8) -> Result<(), String> { require_operator(ctx)?; actor(ctx, actor_guid)?; - crate::actor::equip_item(ctx, actor_guid, from_slot) + crate::actor::equip_item(ctx, actor_guid, from_slot).map_err(crate::items::refused) } /// [`crate::actor::use_gameobject`] behind the gateway gate. @@ -953,6 +953,7 @@ pub fn gw_move_item( require_operator(ctx)?; actor(ctx, actor_guid)?; crate::items::apply_item_move(ctx, actor_guid, from_slot, to_slot) + .map_err(crate::items::refused) } /// [`crate::items::apply_unequip_item`] with the owner named by guid. @@ -960,7 +961,7 @@ pub fn gw_move_item( pub fn gw_unequip_item(ctx: &ReducerContext, actor_guid: u64, from_slot: u8) -> Result<(), String> { require_operator(ctx)?; actor(ctx, actor_guid)?; - crate::items::apply_unequip_item(ctx, actor_guid, from_slot) + crate::items::apply_unequip_item(ctx, actor_guid, from_slot).map_err(crate::items::refused) } /// [`crate::items::apply_auto_bank_item`] with the owner named by guid — right-click to bank / right- diff --git a/module/src/items/inventory.rs b/module/src/items/inventory.rs index d7e8d2065..c0d426e76 100644 --- a/module/src/items/inventory.rs +++ b/module/src/items/inventory.rs @@ -6,6 +6,9 @@ use spacetimedb::{ReducerContext, Table}; use lyracore_shared::constants::starter_item; +use lyracore_shared::item::ItemRefusal; + +use super::{refuse, refused}; use super::rules::{ binds_on_equip, can_equip_into, eligibility_mask_allows, equip_slot, invtype, @@ -37,18 +40,18 @@ pub(crate) fn apply_item_split( slot: u8, count: u32, to_slot: u8, -) -> Result<(), String> { +) -> Result<(), ItemRefusal> { // The bank is a place, not a portable bag: either endpoint in bank space needs an open bank. if is_bank_slot(slot) || is_bank_slot(to_slot) { - super::economy::bank_access_gate(ctx, player_guid)?; + bank_access(ctx, player_guid)?; } let instances = ctx.db.game_item_instance(); - let mut inst = - item_in_slot(ctx, player_guid, slot).ok_or_else(|| format!("no item in slot {slot}"))?; + let mut inst = item_in_slot(ctx, player_guid, slot) + .ok_or_else(|| refuse(ItemRefusal::ItemNotFound, format!("no item in slot {slot}")))?; // A split must leave at least one unit in BOTH slots — splitting off none or the whole stack isn't // a split (the latter is a move). if !valid_split_count(count, inst.stack_count) { - return Err("invalid split count".to_string()); + return Err(refuse(ItemRefusal::WrongSlot, "invalid split count")); } // Reject an out-of-range destination (anti-overflow; same phantom-slot dupe vector as apply_item_move) // AND reject an equipment-region destination (0..=18): a split can never legitimately target the @@ -56,14 +59,17 @@ pub(crate) fn apply_item_split( // admitting 0..=18 here let a modified client CMSG_SPLIT_ITEM a stack straight into an empty head or // off-hand slot, bypassing every gate the move path enforces for that region. if !valid_split_dest_slot(to_slot) { - return Err(format!("invalid destination slot {to_slot}")); + return Err(refuse( + ItemRefusal::WrongSlot, + format!("invalid destination slot {to_slot}"), + )); } // Bag-content destination: validate that the corresponding bag is equipped and the slot is // within its capacity — same phantom-slot dupe vector as apply_item_move. validate_bag_dest_slot(ctx, player_guid, to_slot)?; // The destination slot must be free; we never merge/swap on a split. if slot_occupied(ctx, player_guid, to_slot) { - return Err("destination slot occupied".to_string()); + return Err(refuse(ItemRefusal::WrongSlot, "destination slot occupied")); } inst.stack_count -= count; let entry = inst.entry; @@ -102,23 +108,30 @@ pub(crate) fn apply_item_move( player_guid: u64, from_slot: u8, to_slot: u8, -) -> Result<(), String> { +) -> Result<(), ItemRefusal> { if from_slot == to_slot { return Ok(()); } // The bank is a place, not a portable bag: either endpoint in bank space needs an open bank. if is_bank_slot(from_slot) || is_bank_slot(to_slot) { - super::economy::bank_access_gate(ctx, player_guid)?; + bank_access(ctx, player_guid)?; } let instances = ctx.db.game_item_instance(); - let mut src = item_in_slot(ctx, player_guid, from_slot) - .ok_or_else(|| format!("no item in slot {from_slot}"))?; + let mut src = item_in_slot(ctx, player_guid, from_slot).ok_or_else(|| { + refuse( + ItemRefusal::ItemNotFound, + format!("no item in slot {from_slot}"), + ) + })?; // Reject an out-of-range destination. Valid slots: equipment 0..=18, bag-equip 19..=22, // backpack 23..=38, or a bag-content slot (120..=191) for items landing inside an equipped bag. // Anything outside these ranges (e.g. 39..=119 = bank/keyring we don't model, or 192..255) is // an inventory-overflow dupe vector from a modified client and is rejected. if !valid_dest_slot(to_slot) { - return Err(format!("invalid destination slot {to_slot}")); + return Err(refuse( + ItemRefusal::WrongSlot, + format!("invalid destination slot {to_slot}"), + )); } // If the destination is in the bag-content region, validate that the corresponding bag is // equipped and the slot is within its capacity (prevents stashing items in unequipped bags). @@ -142,29 +155,41 @@ pub(crate) fn apply_item_move( { tmpl } - _ => return Err(format!("cannot equip that item in slot {to_slot}")), + _ => { + return Err(refuse( + ItemRefusal::CannotEquip, + format!("cannot equip that item in slot {to_slot}"), + )) + } }; // Required-level gate: you can carry a too-high item in the bag, but can't EQUIP it. Read the // player entity just for its level here (the move path doesn't otherwise need it); a missing // entity fails closed. Seeded items are required_level 1, so this never trips for the loadout. let player = crate::helpers::live_entity(ctx, player_guid) - .map_err(|_| "user not in world".to_string())?; + .map_err(|_| refuse(ItemRefusal::Internal, "user not in world"))?; + // Vanilla's level result carries the required level in the packet, and a Refusal tag has no + // payload, so a level Gate reads as the generic equip failure. if !meets_required_level(player.level, tmpl.required_level) { - return Err(format!("requires level {}", tmpl.required_level)); + return Err(refuse( + ItemRefusal::CannotEquip, + format!("requires level {}", tmpl.required_level), + )); } // Proficiency gate: enforce class armor/weapon restrictions (e.g. a Mage can't equip // plate). The class is byte 1 of unit_bytes_0 (race | class<<8 | gender<<16 | power<<24). // Creatures (class 0) never call equip_item; fail closed for unknown classes. let player_class = player.class(); if !eligibility_mask_allows(tmpl.allowed_class, player_class) { - return Err(format!( - "class {player_class} is not allowed to equip this item" + return Err(refuse( + ItemRefusal::NoProficiency, + format!("class {player_class} is not allowed to equip this item"), )); } let player_race = player.race(); if !eligibility_mask_allows(tmpl.allowed_race, player_race) { - return Err(format!( - "race {player_race} is not allowed to equip this item" + return Err(refuse( + ItemRefusal::NoProficiency, + format!("race {player_race} is not allowed to equip this item"), )); } let current_skill = ctx @@ -175,9 +200,12 @@ pub(crate) fn apply_item_move( .find(|skill| skill.skill_line == tmpl.required_skill) .map(|skill| skill.current); if !meets_required_skill(tmpl.required_skill, tmpl.required_skill_rank, current_skill) { - return Err(format!( - "requires skill {} at rank {}", - tmpl.required_skill, tmpl.required_skill_rank + return Err(refuse( + ItemRefusal::RequiredSkill, + format!( + "requires skill {} at rank {}", + tmpl.required_skill, tmpl.required_skill_rank + ), )); } let reputation_standing = ctx @@ -192,9 +220,12 @@ pub(crate) fn apply_item_move( tmpl.required_reputation_rank, reputation_standing, ) { - return Err(format!( - "requires reputation faction {} at rank {}", - tmpl.required_reputation_faction, tmpl.required_reputation_rank + return Err(refuse( + ItemRefusal::RequiredReputation, + format!( + "requires reputation faction {} at rank {}", + tmpl.required_reputation_faction, tmpl.required_reputation_rank + ), )); } // Armor proficiency is class base set PLUS the two upgrades a class trainer teaches at 40: @@ -205,9 +236,12 @@ pub(crate) fn apply_item_move( crate::spell::knows_spell(ctx, player_guid, spell_id) }); if !proficiency.can_equip(tmpl.class, tmpl.subclass) { - return Err(format!( - "class {} lacks proficiency for item class {}/subclass {}", - player_class, tmpl.class, tmpl.subclass + return Err(refuse( + ItemRefusal::NoProficiency, + format!( + "class {} lacks proficiency for item class {}/subclass {}", + player_class, tmpl.class, tmpl.subclass + ), )); } // BoE: a Bind-on-Equip item binds the FIRST time it lands on the body — not on pickup. Every @@ -275,21 +309,30 @@ pub(crate) fn apply_equip_item( ctx: &ReducerContext, player_guid: u64, from_slot: u8, -) -> Result<(), String> { - let src = item_in_slot(ctx, player_guid, from_slot) - .ok_or_else(|| format!("no item in slot {from_slot}"))?; +) -> Result<(), ItemRefusal> { + let src = item_in_slot(ctx, player_guid, from_slot).ok_or_else(|| { + refuse( + ItemRefusal::ItemNotFound, + format!("no item in slot {from_slot}"), + ) + })?; let tmpl = ctx .db .game_item_template() .entry() .find(src.entry) - .ok_or_else(|| format!("no template for item entry {}", src.entry))?; + .ok_or_else(|| { + refuse( + ItemRefusal::ItemNotFound, + format!("no template for item entry {}", src.entry), + ) + })?; // Bags (INVTYPE_BAG) equip into bag-equip slots 19..=22, not the equipment region 0..=18. // Route them separately: find the first free bag-equip slot and move the bag there. A bag // already in the bag slots (dragged manually) won't come through here, but autoequip does. if tmpl.inventory_type == invtype::BAG { let to_slot = first_free_bag_equip_slot(ctx, player_guid) - .ok_or_else(|| "all four bag slots are full".to_string())?; + .ok_or_else(|| refuse(ItemRefusal::InventoryFull, "all four bag slots are full"))?; return apply_item_move(ctx, player_guid, from_slot, to_slot); } // Dual Wield: redirect a second one-hander to OFFHAND (instead of swapping MAINHAND) when the @@ -303,7 +346,12 @@ pub(crate) fn apply_equip_item( let to_slot = resolve_equip_slot(tmpl.inventory_type, can_dual_wield, |s| { slot_occupied(ctx, player_guid, s) }) - .ok_or_else(|| format!("item {} is not equippable", src.entry))?; + .ok_or_else(|| { + refuse( + ItemRefusal::CannotEquip, + format!("item {} is not equippable", src.entry), + ) + })?; // Equip == a validated move into the resolved equip slot (reuses the equip-validation + swap there). apply_item_move(ctx, player_guid, from_slot, to_slot) } @@ -316,16 +364,22 @@ pub(crate) fn apply_unequip_item( ctx: &ReducerContext, player_guid: u64, from_slot: u8, -) -> Result<(), String> { +) -> Result<(), ItemRefusal> { if from_slot > equip_slot::END { - return Err(format!("slot {from_slot} is not an equipment slot")); + return Err(refuse( + ItemRefusal::WrongSlot, + format!("slot {from_slot} is not an equipment slot"), + )); } // Must actually hold an equipped item to unequip. if !slot_occupied(ctx, player_guid, from_slot) { - return Err(format!("no item equipped in slot {from_slot}")); + return Err(refuse( + ItemRefusal::ItemNotFound, + format!("no item equipped in slot {from_slot}"), + )); } - let free = - first_free_backpack_slot(ctx, player_guid).ok_or_else(|| "inventory full".to_string())?; + let free = first_free_backpack_slot(ctx, player_guid) + .ok_or_else(|| refuse(ItemRefusal::InventoryFull, "inventory full"))?; apply_item_move(ctx, player_guid, from_slot, free) } @@ -419,29 +473,42 @@ pub(crate) fn validate_bag_dest_slot( ctx: &ReducerContext, player_guid: u64, to_slot: u8, -) -> Result<(), String> { +) -> Result<(), ItemRefusal> { if to_slot < BAG_CONTENT_OFFSET { return Ok(()); } let (bag_idx, slot_in_bag) = bag_content_decompose(to_slot); let bag_equip_slot = BAG_SLOT_START + bag_idx; - let bag_inst = item_in_slot(ctx, player_guid, bag_equip_slot) - .ok_or_else(|| format!("no bag equipped in slot {bag_equip_slot}"))?; + let bag_inst = item_in_slot(ctx, player_guid, bag_equip_slot).ok_or_else(|| { + refuse( + ItemRefusal::WrongSlot, + format!("no bag equipped in slot {bag_equip_slot}"), + ) + })?; let bag_tmpl = ctx .db .game_item_template() .entry() .find(bag_inst.entry) - .ok_or_else(|| "equipped bag has no template".to_string())?; + .ok_or_else(|| refuse(ItemRefusal::WrongSlot, "equipped bag has no template"))?; if slot_in_bag >= bag_tmpl.container_slots.min(MAX_BAG_SIZE) { - return Err(format!( - "slot {} out of range for bag with {} slots", - slot_in_bag, bag_tmpl.container_slots + return Err(refuse( + ItemRefusal::WrongSlot, + format!( + "slot {} out of range for bag with {} slots", + slot_in_bag, bag_tmpl.container_slots + ), )); } Ok(()) } +/// The banker-proximity Gate as an item Refusal: its own prose stays in the Module log. +fn bank_access(ctx: &ReducerContext, player_guid: u64) -> Result<(), ItemRefusal> { + super::economy::bank_access_gate(ctx, player_guid) + .map_err(|detail| refuse(ItemRefusal::BankUnavailable, detail)) +} + // Live only under `debug_reducers`: `apply_item_split` is its sole caller. #[cfg_attr(not(feature = "debug_reducers"), allow(dead_code))] /// A split must leave at least one unit in BOTH the source and the new stack — splitting off none @@ -575,11 +642,12 @@ pub(crate) fn apply_auto_bank_item( let to_slot = if is_bank_slot(slot) { first_free_backpack_slot(ctx, player_guid) .or_else(|| first_free_bag_slot(ctx, player_guid)) - .ok_or_else(|| "inventory full".to_string())? + .ok_or_else(|| refused(refuse(ItemRefusal::InventoryFull, "inventory full")))? } else { - first_free_bank_slot(ctx, player_guid).ok_or_else(|| "bank full".to_string())? + first_free_bank_slot(ctx, player_guid) + .ok_or_else(|| refused(refuse(ItemRefusal::InventoryFull, "bank full")))? }; - apply_item_move(ctx, player_guid, slot, to_slot) + apply_item_move(ctx, player_guid, slot, to_slot).map_err(refused) } #[cfg(test)] @@ -602,7 +670,7 @@ mod tests { } } - /// BANK ACCESS GATE WIRING: both mutation paths must consult `bank_access_gate` when either + /// BANK ACCESS GATE WIRING: both mutation paths must consult `bank_access` when either /// endpoint is a bank slot, or the bank becomes a portable 24-slot bag. There is no /// `ReducerContext` harness in this crate, so the call's PRESENCE is pinned by a source scan; the /// gate's own decision is asserted directly on `economy::banker_in_reach`. @@ -615,7 +683,7 @@ mod tests { ] { let body = code_of(src, signature); assert!( - body.contains("bank_access_gate(ctx, player_guid)?"), + body.contains("bank_access(ctx, player_guid)?"), "`{signature}` must refuse a bank endpoint without an open bank" ); assert!( diff --git a/module/src/items/mod.rs b/module/src/items/mod.rs index 5af54c19a..8a5c800f4 100644 --- a/module/src/items/mod.rs +++ b/module/src/items/mod.rs @@ -25,6 +25,20 @@ mod ops; mod rules; mod tables; +use lyracore_shared::item::ItemRefusal; + +/// Refuse an item action. The human detail stays in the Module log and only the typed Refusal +/// travels, so neither tier reads prose. +pub(crate) fn refuse(refusal: ItemRefusal, detail: impl std::fmt::Display) -> ItemRefusal { + spacetimedb::log::info!("item refused {}: {detail}", refusal.as_tag()); + refusal +} + +/// Reducer edge: a reducer answers with a bare tag, which is the whole error text the Gateway reads. +pub(crate) fn refused(refusal: ItemRefusal) -> String { + refusal.as_tag().to_string() +} + pub(crate) use economy::*; // the apply_* cores are pub(crate) (no pub items to re-export) pub(crate) use inventory::*; // ditto pub(crate) use ops::*; // ditto diff --git a/module/src/items/ops.rs b/module/src/items/ops.rs index 1bb39ee06..107529002 100644 --- a/module/src/items/ops.rs +++ b/module/src/items/ops.rs @@ -7,7 +7,9 @@ use spacetimedb::{Identity, ReducerContext, Table}; use lyracore_shared::constants::starter_item; +use lyracore_shared::item::ItemRefusal; +use super::refuse; use crate::game_character; // the durable char holds `class` (the live WorldEntity does not) use crate::game_corpse_loot; // the loot.rs accessor trait — re-exported at crate root (`pub use loot::*`) use crate::game_gameobject; @@ -454,36 +456,56 @@ pub(crate) fn apply_item_use( ctx: &ReducerContext, player_guid: u64, slot: u8, -) -> Result<(), String> { +) -> Result<(), ItemRefusal> { let entities = ctx.db.game_world_entity(); let player = entities .guid() .find(player_guid) - .ok_or_else(|| "user not in world".to_string())?; + .ok_or_else(|| refuse(ItemRefusal::Internal, "user not in world"))?; if player.dead { - return Err("dead players cannot use items".to_string()); + return Err(refuse( + ItemRefusal::PlayerDead, + "dead players cannot use items", + )); } let instances = ctx.db.game_item_instance(); - let mut inst = - item_in_slot(ctx, player_guid, slot).ok_or_else(|| format!("no item in slot {slot}"))?; + let mut inst = item_in_slot(ctx, player_guid, slot) + .ok_or_else(|| refuse(ItemRefusal::ItemNotFound, format!("no item in slot {slot}")))?; // A banked item must be taken out before it can be used — the bank is not a second action bar. if !is_carried_slot(inst.slot) { - return Err("cannot use a banked item".to_string()); + return Err(refuse( + ItemRefusal::ItemNotUsable, + "cannot use a banked item", + )); } let tmpl = ctx .db .game_item_template() .entry() .find(inst.entry) - .ok_or_else(|| format!("no template for item entry {}", inst.entry))?; + .ok_or_else(|| { + refuse( + ItemRefusal::ItemNotFound, + format!("no template for item entry {}", inst.entry), + ) + })?; // spellid_1 (trigger 0) is the item's on-use spell — the single authority now. No mapped // spell means nothing to "use" (a plain reagent, a piece of gear, an un-migrated item). - let spell_id = - use_spell_for(&tmpl).ok_or_else(|| format!("item {} is not consumable", inst.entry))?; + let spell_id = use_spell_for(&tmpl).ok_or_else(|| { + refuse( + ItemRefusal::ItemNotUsable, + format!("item {} is not consumable", inst.entry), + ) + })?; // Required-level gate: a too-high item can sit in the bag, but can't be USED. Seeded items are // required_level 1, so this never trips for the loadout. + // Vanilla's level result carries the required level in the packet, and a Refusal tag has no + // payload, so a level Gate reads as the generic "can never use that item". if !meets_required_level(player.level, tmpl.required_level) { - return Err(format!("requires level {}", tmpl.required_level)); + return Err(refuse( + ItemRefusal::ItemNotUsable, + format!("requires level {}", tmpl.required_level), + )); } // RE-BANDAGE GATE (vanilla "Recently Bandaged"): refuse the BANDAGE while its cooldown debuff is // live — BEFORE consuming the item or casting, so a blocked re-bandage costs nothing. `has_aura` @@ -493,7 +515,7 @@ pub(crate) fn apply_item_use( inst.entry, crate::spell::has_aura(ctx, player_guid, RECENTLY_BANDAGED_SPELL), ) { - return Err("Recently Bandaged".to_string()); + return Err(refuse(ItemRefusal::NotRightNow, "Recently Bandaged")); } // GATE A (the one surviving mana-class gate, retired its legacy-branch duplicate): a mana potion // / drink restores POWER through E_ENERGIZE / A_PERIODIC_ENERGIZE — for a non-mana class that is @@ -502,7 +524,10 @@ pub(crate) fn apply_item_use( // class. Only energize spells are gated (`spell_restores_power`); a heal/HoT/food/buff/recall is // class-agnostic (Warriors can quaff a healing potion / eat / hearth). if spell_restores_power(ctx, spell_id) && !is_mana_class(ctx, player_guid) { - return Err("only mana users can use that".to_string()); + return Err(refuse( + ItemRefusal::ItemNotUsable, + "only mana users can use that", + )); } // Consume one unit FIRST — UNLESS the on-use spell is one of the permanent kinds (a recall trinket, // a mount): `spell_keeps_item` is the data-driven exception to "using an item consumes it", keyed on @@ -530,6 +555,7 @@ pub(crate) fn apply_item_use( false, None, ) + .map_err(|detail| refuse(ItemRefusal::NotRightNow, detail)) } /// The total `which` bonus from every piece of gear `owner_guid` has EQUIPPED — the sum of From 6d6f9c45cfe77a91ebe772f206ff80ed9063b64e Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Fri, 4 Sep 2026 23:05:40 +0200 Subject: [PATCH 2/2] fix(item): classify taxi flight as a refusal --- module/src/gw.rs | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/module/src/gw.rs b/module/src/gw.rs index 2e6cec9c8..11a8fc923 100644 --- a/module/src/gw.rs +++ b/module/src/gw.rs @@ -392,11 +392,25 @@ pub fn gw_turn_in_quest( crate::actor::turn_in_quest(ctx, actor_guid, giver_guid, quest_entry, reward_index) } +/// Taxi flight is an expected item Refusal. Missing actors still carry the generic actor error, +/// which the Gateway treats as a failure with an unknown result. +fn item_actor(ctx: &ReducerContext, actor_guid: u64) -> Result { + let actor = acting_entity_by_guid(ctx, actor_guid) + .ok_or_else(|| "mover not in world".to_string())?; + if crate::taxi::is_in_flight(ctx, actor_guid) { + return Err(crate::items::refused(crate::items::refuse( + lyracore_shared::item::ItemRefusal::NotRightNow, + "Character is in taxi flight", + ))); + } + Ok(actor) +} + /// [`crate::actor::use_item`] behind the gateway gate. #[reducer] pub fn gw_use_item(ctx: &ReducerContext, actor_guid: u64, slot: u8) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + item_actor(ctx, actor_guid)?; crate::actor::use_item(ctx, actor_guid, slot).map_err(crate::items::refused) } @@ -453,7 +467,7 @@ pub fn gw_sell_item( #[reducer] pub fn gw_equip_item(ctx: &ReducerContext, actor_guid: u64, from_slot: u8) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + item_actor(ctx, actor_guid)?; crate::actor::equip_item(ctx, actor_guid, from_slot).map_err(crate::items::refused) } @@ -951,7 +965,7 @@ pub fn gw_move_item( to_slot: u8, ) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + item_actor(ctx, actor_guid)?; crate::items::apply_item_move(ctx, actor_guid, from_slot, to_slot) .map_err(crate::items::refused) } @@ -960,7 +974,7 @@ pub fn gw_move_item( #[reducer] pub fn gw_unequip_item(ctx: &ReducerContext, actor_guid: u64, from_slot: u8) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + item_actor(ctx, actor_guid)?; crate::items::apply_unequip_item(ctx, actor_guid, from_slot).map_err(crate::items::refused) }