From 16a14bcf98ba66cc984af78ed80de5681008991e Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Thu, 3 Sep 2026 23:03:05 +0200 Subject: [PATCH 1/4] refactor(loot): typed refusals across the tier contract The Module refused a loot Durable Request with prose, and the Gateway searched the rendered error text for a `loot_tag_ineligible:` prefix. One shared `LootRefusal` enum now carries the contract as a stable tag; the reducer edge sends only the tag and keeps the detail in the Module log. The Gateway parses the tag from a Rejected reducer and maps it to one `SMSG_LOOT_RESPONSE` error code. Co-Authored-By: Claude Fable 5.1 --- crates/lyracore-shared/src/lib.rs | 1 + crates/lyracore-shared/src/loot.rs | 72 ++++++ gateway/src/codec/loot.rs | 8 +- gateway/src/stdb/mod.rs | 1 - gateway/src/stdb/reducers.rs | 125 ++++++++-- gateway/src/world/handlers/loot.rs | 385 ++++++++++++++--------------- gateway/src/world/handlers/mod.rs | 6 +- gateway/src/world/mod.rs | 4 +- gateway/src/world/tests.rs | 10 +- module/src/loot/mod.rs | 57 +++-- module/src/loot/rolls.rs | 23 +- module/src/loot/tag.rs | 48 ++-- 12 files changed, 456 insertions(+), 284 deletions(-) create mode 100644 crates/lyracore-shared/src/loot.rs diff --git a/crates/lyracore-shared/src/lib.rs b/crates/lyracore-shared/src/lib.rs index 4b99a2c43..d4f1d2da7 100644 --- a/crates/lyracore-shared/src/lib.rs +++ b/crates/lyracore-shared/src/lib.rs @@ -17,6 +17,7 @@ pub mod faction; pub mod group; pub mod instance; pub mod item; +pub mod loot; pub mod loot_roll; pub mod mail; pub mod nav; diff --git a/crates/lyracore-shared/src/loot.rs b/crates/lyracore-shared/src/loot.rs new file mode 100644 index 000000000..85b5eceb8 --- /dev/null +++ b/crates/lyracore-shared/src/loot.rs @@ -0,0 +1,72 @@ +//! The loot tier contract shared by the Module and the Gateway. + +/// Why the Module refused a loot Durable Request. The tag is the whole reducer error text, so +/// neither tier matches on human prose. The Module keeps the detail in its own log. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum LootRefusal { + /// The Actor is not in the Loot Source's Loot Tag eligibility set. + LootTagIneligible, + /// The Loot Source is on another map, in another instance, or beyond loot range. + OutOfRange, + /// The named Loot Source does not exist, or is not a lootable creature corpse. + NoLootSource, + /// The looter is not in the world, or is dead. + LooterUnavailable, + /// The Loot Source has nothing left in the requested slot or purse. + NothingToLoot, + /// No open Loot Roll answers this vote, or the voter already voted. + RollUnavailable, + /// The Actor does not hold the master-looter right on that row. + NotMasterLooter, +} + +impl LootRefusal { + pub const ALL: [Self; 7] = [ + Self::LootTagIneligible, + Self::OutOfRange, + Self::NoLootSource, + Self::LooterUnavailable, + Self::NothingToLoot, + Self::RollUnavailable, + Self::NotMasterLooter, + ]; + + pub fn as_tag(self) -> &'static str { + match self { + Self::LootTagIneligible => "loot:loot_tag_ineligible", + Self::OutOfRange => "loot:out_of_range", + Self::NoLootSource => "loot:no_loot_source", + Self::LooterUnavailable => "loot:looter_unavailable", + Self::NothingToLoot => "loot:nothing_to_loot", + Self::RollUnavailable => "loot:roll_unavailable", + Self::NotMasterLooter => "loot:not_master_looter", + } + } + + pub fn parse_tag(tag: &str) -> Option { + Self::ALL + .into_iter() + .find(|refusal| refusal.as_tag() == tag) + } +} + +#[cfg(test)] +mod tests { + use super::LootRefusal; + + #[test] + fn every_refusal_tag_round_trips() { + for refusal in LootRefusal::ALL { + assert_eq!(LootRefusal::parse_tag(refusal.as_tag()), Some(refusal)); + } + assert_eq!(LootRefusal::parse_tag("loot:"), None); + assert_eq!( + LootRefusal::parse_tag("loot_tag_ineligible: actor_guid=7 corpse_guid=11"), + None + ); + assert_eq!( + LootRefusal::parse_tag("gw_take_loot reducer timed out after 10s"), + None + ); + } +} diff --git a/gateway/src/codec/loot.rs b/gateway/src/codec/loot.rs index 997db8fbe..8d000d748 100644 --- a/gateway/src/codec/loot.rs +++ b/gateway/src/codec/loot.rs @@ -44,13 +44,11 @@ pub fn build_loot_response_raw(guid: u64, money: u32, items: &[LootItemView]) -> (SMSG_LOOT_RESPONSE_OPCODE, body) } -/// Build the vanilla response for a character who cannot loot this creature corpse. -pub fn build_loot_didnt_kill_response(guid: u64) -> SMSG_LOOT_RESPONSE { +/// Build the vanilla response that answers a loot Refusal on the named Loot Source. +pub fn build_loot_error_response(guid: u64, loot_error: LootMethodError) -> SMSG_LOOT_RESPONSE { SMSG_LOOT_RESPONSE { guid: Guid::new(guid), - loot_method: SMSG_LOOT_RESPONSE_LootMethod::ErrorX { - loot_error: LootMethodError::DidntKill, - }, + loot_method: SMSG_LOOT_RESPONSE_LootMethod::ErrorX { loot_error }, gold: Gold::new(0), items: Vec::new(), } diff --git a/gateway/src/stdb/mod.rs b/gateway/src/stdb/mod.rs index d2de1ced4..35ebee065 100644 --- a/gateway/src/stdb/mod.rs +++ b/gateway/src/stdb/mod.rs @@ -29,7 +29,6 @@ pub(crate) mod world_index; pub(crate) mod world_view; // shared per-shard spatial, broadcast, private, and owner dispatch pub use connection::Coordinator; -pub(crate) use connection::{is_reducer_refusal, reducer_refusal_reason}; pub use subscriptions::PlayerSubscriptions; // Re-exported so `crate::stdb::{RealmRow, AccountRow}` resolves (they are the return types of // `Coordinator::realm` / `account_by_username`). `allow(unused_imports)` because in this *binary* diff --git a/gateway/src/stdb/reducers.rs b/gateway/src/stdb/reducers.rs index 645cce5eb..4f57660c6 100644 --- a/gateway/src/stdb/reducers.rs +++ b/gateway/src/stdb/reducers.rs @@ -11,9 +11,10 @@ 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 crate::world::{ItemActionResult, LootWindowRefusal, LootWindowRequestStatus}; use lyracore_shared::auction::AuctionRefusal; use lyracore_shared::item::ItemRefusal; +use lyracore_shared::loot::LootRefusal; use lyracore_shared::trainer::TrainerRefusal; static NEXT_TAXI_REQUEST_ID: OnceLock = OnceLock::new(); @@ -1047,16 +1048,21 @@ impl Coordinator { /// Use a gameobject (`CMSG_GAMEOBJ_USE`) — a chest rolls its loot into the corpse-loot table keyed /// on the GO guid, a quest-use object grants quest credit. The module gates range + type. /// Rides the coordinator connection as `gw_use_gameobject`. - pub fn use_gameobject(&self, _account_id: u64, actor_guid: u64, go_guid: u64) -> Result<()> { + pub fn use_gameobject( + &self, + _account_id: u64, + actor_guid: u64, + go_guid: u64, + ) -> Result { if actor_guid == 0 { return Err(anyhow!("use_gameobject: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + loot_request_status(call_reducer!( coord.conn.reducers, "gw_use_gameobject", gw_use_gameobject_then(actor_guid, go_guid) - ) + )) } /// Enter an area trigger (`CMSG_AREATRIGGER`) — credit any active explore quest tied to `trigger_id`. @@ -1820,16 +1826,21 @@ impl Coordinator { /// Take the money from a corpse (`CMSG_LOOT_MONEY`, slice 3) over the coordinator connection so /// the module attributes the loot to the caller (as `gw_loot_money`). - pub fn loot_money(&self, _account_id: u64, actor_guid: u64, target_guid: u64) -> Result<()> { + pub fn loot_money( + &self, + _account_id: u64, + actor_guid: u64, + target_guid: u64, + ) -> Result { if actor_guid == 0 { return Err(anyhow!("loot_money: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + loot_request_status(call_reducer!( coord.conn.reducers, "gw_loot_money", gw_loot_money_then(actor_guid, target_guid) - ) + )) } /// Authorize opening a creature corpse before the Gateway reads its loot rows. @@ -1838,16 +1849,16 @@ impl Coordinator { _account_id: u64, actor_guid: u64, corpse_guid: u64, - ) -> Result<()> { + ) -> Result { if actor_guid == 0 { return Err(anyhow!("open_creature_loot: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + loot_request_status(call_reducer!( coord.conn.reducers, "gw_open_creature_loot", gw_open_creature_loot_then(actor_guid, corpse_guid) - ) + )) } /// Take one item from the open corpse into the backpack (`CMSG_AUTOSTORE_LOOT_ITEM`, slice 4) over @@ -1860,28 +1871,33 @@ impl Coordinator { actor_guid: u64, corpse_guid: u64, loot_slot: u8, - ) -> Result<()> { + ) -> Result { if actor_guid == 0 { return Err(anyhow!("take_loot: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + loot_request_status(call_reducer!( coord.conn.reducers, "gw_take_loot", gw_take_loot_then(actor_guid, corpse_guid, loot_slot) - ) + )) } - pub fn skin_corpse(&self, _account_id: u64, actor_guid: u64, corpse_guid: u64) -> Result<()> { + pub fn skin_corpse( + &self, + _account_id: u64, + actor_guid: u64, + corpse_guid: u64, + ) -> Result { if actor_guid == 0 { return Err(anyhow!("skin_corpse: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + loot_request_status(call_reducer!( coord.conn.reducers, "gw_skin", gw_skin_then(actor_guid, corpse_guid) - ) + )) } /// `CMSG_LOOT_ROLL` — record the caller's need/greed/pass vote on a @@ -3116,6 +3132,25 @@ fn next_auction_operation_id() -> Result { } } +/// A loot Durable Request the Module rejected is a Refusal outcome; a timeout, transport, or SDK +/// failure stays an error with an unknown durable result and ends the session. Module cores outside +/// the loot family still refuse with untagged prose, so an untagged Rejection is unanswered. +fn loot_request_status(result: Result<()>) -> Result { + match result { + Ok(()) => Ok(LootWindowRequestStatus::Applied), + Err(error) => match reducer_refusal_reason(&error) { + Some(reason) => { + log::debug!("stdb: loot Durable Request refused: {error:#}"); + Ok(LootWindowRequestStatus::Refused( + LootRefusal::parse_tag(reason) + .map_or(LootWindowRefusal::Unanswered, Into::into), + )) + } + None => Err(error), + }, + } +} + /// The Module's typed auction Refusal. Only a reducer the Module rejected carries a tag; a timeout, /// transport, or SDK failure stays an error with an unknown outcome. fn auction_refusal(error: &anyhow::Error) -> Option { @@ -3244,6 +3279,64 @@ mod item_reducer_tests { } } +#[cfg(test)] +mod loot_reducer_tests { + use super::*; + use crate::stdb::connection::ReducerCallError; + + fn refusal_of(result: Result<()>) -> LootWindowRefusal { + match loot_request_status(result) { + Ok(LootWindowRequestStatus::Refused(refusal)) => refusal, + Ok(LootWindowRequestStatus::Applied) => panic!("a Refusal was applied"), + Err(error) => panic!("a Refusal ended the session: {error:#}"), + } + } + + fn rejected(reason: &str) -> Result<()> { + Err(anyhow::Error::from(ReducerCallError::Rejected { + operation: "gw_take_loot".to_string(), + reason: reason.to_string(), + }) + .context("loot window")) + } + + #[test] + fn every_module_refusal_tag_becomes_one_client_answer() { + for refusal in LootRefusal::ALL { + assert_eq!( + refusal_of(rejected(refusal.as_tag())), + LootWindowRefusal::from(refusal), + "{refusal:?}" + ); + } + // A Module core outside the loot family still refuses with prose. + assert_eq!( + refusal_of(rejected("inventory is full")), + LootWindowRefusal::Unanswered + ); + } + + #[test] + fn a_timeout_is_not_answered_as_a_refusal() { + let not_refusals = [ + anyhow::Error::from(ReducerCallError::fatal( + "gw_take_loot reducer timed out after 10s".to_string(), + )), + anyhow::Error::from(ReducerCallError::fatal( + "gw_loot_money reducer failed: transport disconnected".to_string(), + )), + anyhow!( + "wrapped text that mentions {}", + LootRefusal::LootTagIneligible.as_tag() + ), + ]; + for error in not_refusals { + let text = format!("{error:#}"); + assert!(loot_request_status(Err(error)).is_err(), "{text}"); + } + } +} + #[cfg(test)] mod auction_reducer_tests { use super::*; diff --git a/gateway/src/world/handlers/loot.rs b/gateway/src/world/handlers/loot.rs index fcbd2f7ca..185dcb082 100644 --- a/gateway/src/world/handlers/loot.rs +++ b/gateway/src/world/handlers/loot.rs @@ -1,6 +1,8 @@ //! Loot-window action dispatch plus the remaining corpse, GameObject, and group-loot handler. use super::super::*; +use lyracore_shared::loot::LootRefusal; +use wow_world_messages::vanilla::LootMethodError; /// Durable reads and requests needed by the loot-window lifecycle. pub(crate) trait LootWindowStore: Send + Sync { @@ -43,30 +45,48 @@ pub(crate) trait LootWindowStore: Send + Sync { ) -> Result; } -/// The durable adapter distinguishes an expected gameplay refusal from a completed request. +/// How the Module answered a loot Durable Request. A Refusal is an outcome; a timeout or transport +/// failure stays an error and ends the session. pub(crate) enum LootWindowRequestStatus { Applied, + Refused(LootWindowRefusal), +} + +/// A loot Refusal as the vanilla client can hear it. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum LootWindowRefusal { + /// The Actor is not in the Loot Source's Loot Tag eligibility set. LootTagIneligible, - Refused(anyhow::Error), + /// The Loot Source is on another map, in another instance, or beyond loot range. + OutOfRange, + /// A Refusal vanilla has no loot-window code for. It is logged at the Store seam and dropped. + Unanswered, } -fn coordinator_request_status(result: Result<()>) -> Result { - match result { - Ok(()) => Ok(LootWindowRequestStatus::Applied), - Err(error) if is_loot_tag_refusal(&error) => Ok(LootWindowRequestStatus::LootTagIneligible), - Err(error) if crate::stdb::is_reducer_refusal(&error) => { - Ok(LootWindowRequestStatus::Refused(error)) +// Module cores outside the loot family still refuse with untagged prose, so an untyped Rejection +// lands on `Unanswered` rather than ending the session. +impl From for LootWindowRefusal { + fn from(refusal: LootRefusal) -> Self { + match refusal { + LootRefusal::LootTagIneligible => Self::LootTagIneligible, + LootRefusal::OutOfRange => Self::OutOfRange, + LootRefusal::NoLootSource + | LootRefusal::LooterUnavailable + | LootRefusal::NothingToLoot + | LootRefusal::RollUnavailable + | LootRefusal::NotMasterLooter => Self::Unanswered, } - Err(error) => Err(error), } } -fn is_loot_tag_refusal(error: &anyhow::Error) -> bool { - crate::stdb::reducer_refusal_reason(error).is_some_and(is_loot_tag_refusal_reason) -} - -fn is_loot_tag_refusal_reason(reason: &str) -> bool { - reason.starts_with("loot_tag_ineligible:") +impl LootWindowRefusal { + fn loot_error(self) -> Option { + match self { + Self::LootTagIneligible => Some(LootMethodError::DidntKill), + Self::OutOfRange => Some(LootMethodError::TooFar), + Self::Unanswered => None, + } + } } impl LootWindowStore for crate::stdb::Coordinator { @@ -88,12 +108,7 @@ impl LootWindowStore for crate::stdb::Coordinator { actor_guid: u64, target_guid: u64, ) -> Result { - coordinator_request_status(crate::stdb::Coordinator::use_gameobject( - self, - account_id, - actor_guid, - target_guid, - )) + crate::stdb::Coordinator::use_gameobject(self, account_id, actor_guid, target_guid) } fn open_creature_loot( @@ -102,12 +117,7 @@ impl LootWindowStore for crate::stdb::Coordinator { actor_guid: u64, corpse_guid: u64, ) -> Result { - coordinator_request_status(crate::stdb::Coordinator::open_creature_loot( - self, - account_id, - actor_guid, - corpse_guid, - )) + crate::stdb::Coordinator::open_creature_loot(self, account_id, actor_guid, corpse_guid) } fn skin_corpse( @@ -116,12 +126,7 @@ impl LootWindowStore for crate::stdb::Coordinator { actor_guid: u64, target_guid: u64, ) -> Result { - coordinator_request_status(crate::stdb::Coordinator::skin_corpse( - self, - account_id, - actor_guid, - target_guid, - )) + crate::stdb::Coordinator::skin_corpse(self, account_id, actor_guid, target_guid) } fn loot_money( @@ -130,12 +135,7 @@ impl LootWindowStore for crate::stdb::Coordinator { actor_guid: u64, target_guid: u64, ) -> Result { - coordinator_request_status(crate::stdb::Coordinator::loot_money( - self, - account_id, - actor_guid, - target_guid, - )) + crate::stdb::Coordinator::loot_money(self, account_id, actor_guid, target_guid) } fn take_loot( @@ -145,13 +145,7 @@ impl LootWindowStore for crate::stdb::Coordinator { target_guid: u64, loot_slot: u8, ) -> Result { - coordinator_request_status(crate::stdb::Coordinator::take_loot( - self, - account_id, - actor_guid, - target_guid, - loot_slot, - )) + crate::stdb::Coordinator::take_loot(self, account_id, actor_guid, target_guid, loot_slot) } } @@ -185,10 +179,30 @@ pub(crate) enum LootWindowDurableRequest { TakeItem { target_guid: u64, loot_slot: u8 }, } -fn loot_tag_ineligible_outbound(corpse_guid: u64) -> Vec { - vec![Outbound::One(ServerOpcodeMessage::SMSG_LOOT_RESPONSE( - Box::new(codec::build_loot_didnt_kill_response(corpse_guid)), - ))] +/// The client traffic a loot Refusal earns. An unanswered Refusal stays silent. +fn refusal_outbound(refusal: LootWindowRefusal, target_guid: u64) -> Vec { + refusal + .loot_error() + .map(|loot_error| { + Outbound::One(ServerOpcodeMessage::SMSG_LOOT_RESPONSE(Box::new( + codec::build_loot_error_response(target_guid, loot_error), + ))) + }) + .into_iter() + .collect() +} + +/// An answered Refusal invalidates the open Loot Window; an unanswered one leaves it alone. +fn refusal_transition( + refusal: LootWindowRefusal, + current_state: OpenLootState, + target_guid: u64, +) -> (OpenLootState, Vec) { + let next_state = match refusal.loot_error() { + Some(_) => OpenLootState::default(), + None => current_state, + }; + (next_state, refusal_outbound(refusal, target_guid)) } /// A handled loot request returns all session state and client traffic to apply in order. @@ -219,30 +233,14 @@ pub(crate) fn dispatch_loot_window( }; let target_guid = request.guid.guid(); let durable_request = Some(LootWindowDurableRequest::UseGameObject { target_guid }); - match store.use_gameobject(player.account_id, actor_guid, target_guid)? { - LootWindowRequestStatus::Applied => {} - LootWindowRequestStatus::LootTagIneligible => { - log::debug!( - "world: use_gameobject returned an inapplicable loot tag refusal (account {})", - player.account_id - ); - return Ok(LootWindowOutcome::Handled { - next_state: current_state, - durable_request, - outbound: Vec::new(), - }); - } - LootWindowRequestStatus::Refused(error) => { - log::debug!( - "world: use_gameobject rejected (account {}): {error}", - player.account_id - ); - return Ok(LootWindowOutcome::Handled { - next_state: current_state, - durable_request, - outbound: Vec::new(), - }); - } + if let LootWindowRequestStatus::Refused(refusal) = + store.use_gameobject(player.account_id, actor_guid, target_guid)? + { + return Ok(LootWindowOutcome::Handled { + next_state: current_state, + durable_request, + outbound: refusal_outbound(refusal, target_guid), + }); } let items = store.loot_target_items(target_guid, actor_guid)?; if items.is_empty() { @@ -271,40 +269,22 @@ pub(crate) fn dispatch_loot_window( }; let target_guid = request.guid.guid(); let open_request = LootWindowDurableRequest::OpenCreature { target_guid }; - match store.open_creature_loot(player.account_id, viewer_guid, target_guid)? { - LootWindowRequestStatus::Applied => {} - LootWindowRequestStatus::LootTagIneligible => { - return Ok(LootWindowOutcome::Handled { - next_state: OpenLootState::default(), - durable_request: Some(open_request), - outbound: loot_tag_ineligible_outbound(target_guid), - }); - } - LootWindowRequestStatus::Refused(error) => { - log::debug!( - "world: open_creature_loot Refusal (account {}): {error}", - player.account_id - ); - return Ok(LootWindowOutcome::Handled { - next_state: current_state, - durable_request: Some(open_request), - outbound: Vec::new(), - }); - } + if let LootWindowRequestStatus::Refused(refusal) = + store.open_creature_loot(player.account_id, viewer_guid, target_guid)? + { + let (next_state, outbound) = + refusal_transition(refusal, current_state, target_guid); + return Ok(LootWindowOutcome::Handled { + next_state, + durable_request: Some(open_request), + outbound, + }); } let money = store.loot_target_money(target_guid)?; let items = store.loot_target_items(target_guid, viewer_guid)?; let durable_request = if items.is_empty() && money == 0 { - match store.skin_corpse(player.account_id, viewer_guid, target_guid)? { - LootWindowRequestStatus::Applied => {} - LootWindowRequestStatus::LootTagIneligible => {} - LootWindowRequestStatus::Refused(error) => { - log::debug!( - "world: skin_corpse noop (account {}): {error}", - player.account_id - ); - } - } + // Skinning an empty corpse is opportunistic: a Refusal still shows the empty window. + store.skin_corpse(player.account_id, viewer_guid, target_guid)?; Some(LootWindowDurableRequest::SkinCreature { target_guid }) } else { Some(open_request) @@ -335,16 +315,8 @@ pub(crate) fn dispatch_loot_window( current_state, vec![Outbound::One(ServerOpcodeMessage::SMSG_LOOT_CLEAR_MONEY)], ), - LootWindowRequestStatus::LootTagIneligible => ( - OpenLootState::default(), - loot_tag_ineligible_outbound(target_guid), - ), - LootWindowRequestStatus::Refused(error) => { - log::debug!( - "world: loot_money Refusal (account {}): {error}", - player.account_id - ); - (current_state, Vec::new()) + LootWindowRequestStatus::Refused(refusal) => { + refusal_transition(refusal, current_state, target_guid) } }; Ok(LootWindowOutcome::Handled { @@ -379,16 +351,8 @@ pub(crate) fn dispatch_loot_window( codec::build_loot_removed(request.item_slot), ))], ), - LootWindowRequestStatus::LootTagIneligible => ( - OpenLootState::default(), - loot_tag_ineligible_outbound(target_guid), - ), - LootWindowRequestStatus::Refused(error) => { - log::debug!( - "world: take_loot Refusal (account {}): {error}", - player.account_id - ); - (current_state, Vec::new()) + LootWindowRequestStatus::Refused(refusal) => { + refusal_transition(refusal, current_state, target_guid) } }; Ok(LootWindowOutcome::Handled { @@ -482,17 +446,10 @@ pub(crate) fn handle_loot( send(tx, Outbound::Raw { opcode, body })?; } } - LootWindowRequestStatus::LootTagIneligible => { - log::debug!( - "world: use_gameobject returned an inapplicable loot tag refusal (account {})", - conn.account_id - ); - } - LootWindowRequestStatus::Refused(error) => { - log::debug!( - "world: use_gameobject ignored (account {}): {error}", - conn.account_id - ); + LootWindowRequestStatus::Refused(refusal) => { + for outbound in refusal_outbound(refusal, target_guid) { + send(tx, outbound)?; + } } } } @@ -638,36 +595,26 @@ mod tests { skin_requests: Mutex>, money_take_requests: Mutex>, item_take_requests: Mutex>, - skin_refusal: Option, + skin_refusal: Option, skin_fatal_error: Option, - use_refusal: Option, + use_refusal: Option, use_fatal_error: Option, - open_refusal: Option, + open_refusal: Option, open_fatal_error: Option, - open_loot_tag_ineligible: bool, - money_take_refusal: Option, + money_take_refusal: Option, money_take_fatal_error: Option, - money_take_loot_tag_ineligible: bool, - item_take_refusal: Option, + item_take_refusal: Option, item_take_fatal_error: Option, - item_take_loot_tag_ineligible: bool, } fn request_status( - refusal: &Option, + refusal: Option, fatal_error: &Option, - loot_tag_ineligible: bool, ) -> Result { - if let Some(error) = fatal_error { - Err(anyhow::anyhow!(error.clone())) - } else if loot_tag_ineligible { - Ok(LootWindowRequestStatus::LootTagIneligible) - } else if let Some(error) = refusal { - Ok(LootWindowRequestStatus::Refused(anyhow::anyhow!( - error.clone() - ))) - } else { - Ok(LootWindowRequestStatus::Applied) + match (fatal_error, refusal) { + (Some(error), _) => Err(anyhow::anyhow!(error.clone())), + (None, Some(refusal)) => Ok(LootWindowRequestStatus::Refused(refusal)), + (None, None) => Ok(LootWindowRequestStatus::Applied), } } @@ -705,7 +652,7 @@ mod tests { .lock() .unwrap() .push((account_id, actor_guid, target_guid)); - request_status(&self.use_refusal, &self.use_fatal_error, false) + request_status(self.use_refusal, &self.use_fatal_error) } fn open_creature_loot( @@ -719,11 +666,7 @@ mod tests { .lock() .unwrap() .push((account_id, actor_guid, corpse_guid)); - request_status( - &self.open_refusal, - &self.open_fatal_error, - self.open_loot_tag_ineligible, - ) + request_status(self.open_refusal, &self.open_fatal_error) } fn skin_corpse( @@ -736,7 +679,7 @@ mod tests { .lock() .unwrap() .push((account_id, actor_guid, target_guid)); - request_status(&self.skin_refusal, &self.skin_fatal_error, false) + request_status(self.skin_refusal, &self.skin_fatal_error) } fn loot_money( @@ -749,11 +692,7 @@ mod tests { .lock() .unwrap() .push((account_id, actor_guid, target_guid)); - request_status( - &self.money_take_refusal, - &self.money_take_fatal_error, - self.money_take_loot_tag_ineligible, - ) + request_status(self.money_take_refusal, &self.money_take_fatal_error) } fn take_loot( @@ -769,11 +708,7 @@ mod tests { target_guid, loot_slot, )); - request_status( - &self.item_take_refusal, - &self.item_take_fatal_error, - self.item_take_loot_tag_ineligible, - ) + request_status(self.item_take_refusal, &self.item_take_fatal_error) } } @@ -796,30 +731,23 @@ mod tests { }) } - fn assert_didnt_kill(outbound: &[Outbound], corpse_guid: u64) { + fn assert_loot_error(outbound: &[Outbound], corpse_guid: u64, expected: LootMethodError) { let [Outbound::One(ServerOpcodeMessage::SMSG_LOOT_RESPONSE(response))] = outbound else { - panic!("expected one DIDNT_KILL loot response") + panic!("expected one loot error response") }; assert_eq!(response.guid.guid(), corpse_guid); - assert!(matches!( - &response.loot_method, + assert_eq!( + response.loot_method, SMSG_LOOT_RESPONSE_LootMethod::ErrorX { - loot_error: LootMethodError::DidntKill, + loot_error: expected, } - )); + ); assert_eq!(response.gold.as_int(), 0); assert!(response.items.is_empty()); } - #[test] - fn loot_tag_refusal_class_requires_the_module_reason_prefix() { - assert!(is_loot_tag_refusal_reason( - "loot_tag_ineligible: actor_guid=7 corpse_guid=11" - )); - assert!(!is_loot_tag_refusal_reason( - "other refusal mentions loot_tag_ineligible: later" - )); - assert!(!is_loot_tag_refusal_reason("loot_tag_ineligible")); + fn assert_didnt_kill(outbound: &[Outbound], corpse_guid: u64) { + assert_loot_error(outbound, corpse_guid, LootMethodError::DidntKill); } #[test] @@ -960,7 +888,7 @@ mod tests { #[test] fn refused_chest_use_does_not_read_loot_or_change_window_state() { let store = InMemoryLootWindow { - use_refusal: Some("out of range".into()), + use_refusal: Some(LootWindowRefusal::Unanswered), ..Default::default() }; let current_state = OpenLootState { @@ -1012,7 +940,7 @@ mod tests { #[test] fn creature_open_ownership_refusal_closes_the_window_without_loot_reads() { let store = InMemoryLootWindow { - open_loot_tag_ineligible: true, + open_refusal: Some(LootWindowRefusal::LootTagIneligible), ..Default::default() }; @@ -1073,7 +1001,7 @@ mod tests { #[test] fn unrelated_creature_open_refusal_keeps_the_window_without_loot_reads() { let store = InMemoryLootWindow { - open_refusal: Some("out of range".into()), + open_refusal: Some(LootWindowRefusal::Unanswered), ..Default::default() }; let current_state = OpenLootState { @@ -1160,7 +1088,7 @@ mod tests { #[test] fn money_take_refusal_keeps_the_window_without_a_false_clear() { let store = InMemoryLootWindow { - money_take_refusal: Some("corpse has no money".into()), + money_take_refusal: Some(LootWindowRefusal::Unanswered), ..Default::default() }; let current_state = OpenLootState { @@ -1191,10 +1119,77 @@ mod tests { ); } + #[test] + fn every_module_refusal_has_one_client_result_code() { + let expected = |refusal| match refusal { + LootRefusal::LootTagIneligible => Some(LootMethodError::DidntKill), + LootRefusal::OutOfRange => Some(LootMethodError::TooFar), + LootRefusal::NoLootSource + | LootRefusal::LooterUnavailable + | LootRefusal::NothingToLoot + | LootRefusal::RollUnavailable + | LootRefusal::NotMasterLooter => None, + }; + let open_window = OpenLootState { + target_guid: Some(60), + }; + + for refusal in LootRefusal::ALL { + let loot_error = expected(refusal); + + for (message, store) in [ + ( + open_creature(60), + InMemoryLootWindow { + open_refusal: Some(refusal.into()), + ..Default::default() + }, + ), + ( + ClientOpcodeMessage::CMSG_LOOT_MONEY, + InMemoryLootWindow { + money_take_refusal: Some(refusal.into()), + ..Default::default() + }, + ), + ( + ClientOpcodeMessage::CMSG_AUTOSTORE_LOOT_ITEM(CMSG_AUTOSTORE_LOOT_ITEM { + item_slot: 3, + }), + InMemoryLootWindow { + item_take_refusal: Some(refusal.into()), + ..Default::default() + }, + ), + ] { + let outcome = dispatch_loot_window(&store, player(), open_window, message).unwrap(); + let LootWindowOutcome::Handled { + next_state, + outbound, + .. + } = outcome + else { + panic!("{refusal:?} passed through") + }; + match loot_error { + // An answered Refusal invalidates the Loot Window the client is showing. + Some(loot_error) => { + assert_loot_error(&outbound, 60, loot_error); + assert_eq!(next_state, OpenLootState::default(), "{refusal:?}"); + } + None => { + assert!(outbound.is_empty(), "{refusal:?}"); + assert_eq!(next_state, open_window, "{refusal:?}"); + } + } + } + } + } + #[test] fn money_take_ownership_refusal_closes_the_window_and_returns_didnt_kill() { let store = InMemoryLootWindow { - money_take_loot_tag_ineligible: true, + money_take_refusal: Some(LootWindowRefusal::LootTagIneligible), ..Default::default() }; @@ -1287,7 +1282,7 @@ mod tests { #[test] fn item_take_refusal_keeps_the_window_without_a_false_slot_removal() { let store = InMemoryLootWindow { - item_take_refusal: Some("inventory full".into()), + item_take_refusal: Some(LootWindowRefusal::Unanswered), ..Default::default() }; let current_state = OpenLootState { @@ -1323,7 +1318,7 @@ mod tests { #[test] fn item_take_ownership_refusal_closes_the_window_and_returns_didnt_kill() { let store = InMemoryLootWindow { - item_take_loot_tag_ineligible: true, + item_take_refusal: Some(LootWindowRefusal::LootTagIneligible), ..Default::default() }; @@ -1629,7 +1624,7 @@ mod tests { #[test] fn skinning_refusal_is_a_handled_empty_window() { let store = InMemoryLootWindow { - skin_refusal: Some("target is not skinnable".into()), + skin_refusal: Some(LootWindowRefusal::Unanswered), ..Default::default() }; diff --git a/gateway/src/world/handlers/mod.rs b/gateway/src/world/handlers/mod.rs index 1a8820869..4d10141c6 100644 --- a/gateway/src/world/handlers/mod.rs +++ b/gateway/src/world/handlers/mod.rs @@ -45,11 +45,9 @@ pub(crate) use duel::{dispatch_duel_action, DuelActionOutcome, DuelActionPlayer, pub(crate) use item::{ dispatch_item_action, ItemActionOutcome, ItemActionPlayer, ItemActionResult, ItemActionStore, }; -#[cfg(test)] -pub(crate) use loot::LootWindowRequestStatus; pub(crate) use loot::{ - dispatch_loot_window, handle_loot, LootWindowOutcome, LootWindowPlayer, LootWindowStore, - OpenLootState, + dispatch_loot_window, handle_loot, LootWindowOutcome, LootWindowPlayer, LootWindowRefusal, + LootWindowRequestStatus, LootWindowStore, OpenLootState, }; pub(crate) use mail::handle_mail; pub(crate) use melee::{ diff --git a/gateway/src/world/mod.rs b/gateway/src/world/mod.rs index e990826c5..1a8c546c3 100644 --- a/gateway/src/world/mod.rs +++ b/gateway/src/world/mod.rs @@ -62,8 +62,8 @@ use handlers::{ }; pub(crate) use handlers::{ zone_weather_message, AuctionBrowseRequest, AuctionPage, AuctionQuery, CreateAuctionOutcome, - CreateAuctionRequest, ItemActionResult, PlaceBidOutcome, PlaceBidRequest, TrainerBuyOutcome, - WeatherStore, + CreateAuctionRequest, ItemActionResult, LootWindowRefusal, LootWindowRequestStatus, + 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 8f87e2a9e..019bd9354 100644 --- a/gateway/src/world/tests.rs +++ b/gateway/src/world/tests.rs @@ -1,7 +1,7 @@ use super::handlers::{ AuctionActionStore, AuctionEntity, AuctionInteraction, CastStore, DuelActionStore, - ItemActionStore, LootWindowRequestStatus, LootWindowStore, MeleeActionStore, QuestActionStore, - TaxiActionStore, VendorActionStore, WeatherStore, + ItemActionStore, LootWindowRefusal, LootWindowRequestStatus, LootWindowStore, MeleeActionStore, + QuestActionStore, TaxiActionStore, VendorActionStore, WeatherStore, }; use super::*; use crate::read_deadline::{DeadlineClock, PreAuthDeadline}; @@ -3209,8 +3209,10 @@ impl LootWindowStore for InMemoryStore { _actor_guid: u64, target_guid: u64, ) -> Result { - if let Some(error) = &self.trade_error { - return Ok(LootWindowRequestStatus::Refused(anyhow!(error.clone()))); + if self.trade_error.is_some() { + return Ok(LootWindowRequestStatus::Refused( + LootWindowRefusal::Unanswered, + )); } self.skinned.lock().unwrap().push(target_guid); Ok(LootWindowRequestStatus::Applied) diff --git a/module/src/loot/mod.rs b/module/src/loot/mod.rs index 94e735b10..c25457cf9 100644 --- a/module/src/loot/mod.rs +++ b/module/src/loot/mod.rs @@ -50,6 +50,7 @@ use crate::game_group_member; // clone_quest_loot_for_group's GameObject roster use crate::game_world_entity; use crate::quest::objective_kind; use crate::{game_character_quest, game_quest_objective}; // killer_needs_item (fishing's zone resolve now lives in terrain::zone_id_at) +use lyracore_shared::loot::LootRefusal; use lyracore_shared::loot_roll::event_kind as roll_event_kind; // apply_loot_money's MONEY_SHARE push mod rolls; @@ -756,6 +757,14 @@ pub(crate) fn corpse_is_looted(ctx: &ReducerContext, corpse_guid: u64, money: u3 .is_none() } +/// The reducer edge for every loot Refusal. Only the tag crosses the tier boundary, so the Gateway +/// never reads Module prose; the operator detail stays in the Module log. +pub(crate) fn refused(refusal: LootRefusal, detail: &str) -> String { + let tag = refusal.as_tag(); + spacetimedb::log::info!("loot refused {tag}: {detail}"); + tag.to_string() +} + /// Open a creature corpse for the read that follows. This reducer core only authorizes the read; /// it does not create durable loot-window state. pub(crate) fn open_creature_corpse( @@ -768,25 +777,34 @@ pub(crate) fn open_creature_corpse( .game_world_entity() .guid() .find(actor_guid) - .ok_or_else(|| "looter not in world".to_string())?; + .ok_or_else(|| refused(LootRefusal::LooterUnavailable, "looter not in world"))?; if actor.dead { - return Err("dead Characters cannot loot".to_string()); + return Err(refused( + LootRefusal::LooterUnavailable, + "dead Characters cannot loot", + )); } let corpse = ctx .db .game_world_entity() .guid() .find(corpse_guid) - .ok_or_else(|| "no such corpse".to_string())?; + .ok_or_else(|| refused(LootRefusal::NoLootSource, "no such corpse"))?; if corpse.is_player() || !corpse.dead { - return Err("target is not a creature corpse".to_string()); + return Err(refused( + LootRefusal::NoLootSource, + "Loot Source is not a creature corpse", + )); } if corpse.map_id != actor.map_id || corpse.instance_id != actor.instance_id { - return Err("corpse is out of reach".to_string()); + return Err(refused( + LootRefusal::OutOfRange, + "Loot Source is on another map or instance", + )); } let (dx, dy, dz) = (corpse.x - actor.x, corpse.y - actor.y, corpse.z - actor.z); if dx * dx + dy * dy + dz * dz > LOOT_RANGE_SQ { - return Err("corpse is out of reach".to_string()); + return Err(refused(LootRefusal::OutOfRange, "corpse is out of reach")); } corpse_access_gate(ctx, actor_guid, corpse_guid) } @@ -802,33 +820,42 @@ pub(crate) fn apply_loot_money( let mut looter = entities .guid() .find(looter_guid) - .ok_or_else(|| "looter not in world".to_string())?; + .ok_or_else(|| refused(LootRefusal::LooterUnavailable, "looter not in world"))?; // A dead looter (killed with a loot window still open) can't loot — gate on the same `dead` flag // every other action checks, so death stays server-authoritative. if looter.dead { - return Err("dead players cannot loot".to_string()); + return Err(refused( + LootRefusal::LooterUnavailable, + "dead players cannot loot", + )); } let mut corpse = entities .guid() .find(target_guid) - .ok_or_else(|| "no such corpse".to_string())?; + .ok_or_else(|| refused(LootRefusal::NoLootSource, "no such corpse"))?; // Only CREATURE corpses are lootable. A dead/ghost PLAYER is also `dead` with a non-zero purse // (loaded from character.money), so without this guard a second player in range could drain the // victim's coinage (PvP purse theft). Money loot is creature-only until real player-loot rules. if corpse.is_player() { - return Err("cannot loot a player".to_string()); + return Err(refused(LootRefusal::NoLootSource, "cannot loot a player")); } if !corpse.dead { - return Err("target is not a corpse".to_string()); + return Err(refused( + LootRefusal::NoLootSource, + "Loot Source is not a corpse", + )); } // Map + instance gated (190 slice 2): a creature corpse is a `game_world_entity` row, so its // `instance_id` came free with slice 1 — a looter can never reach across an instance wall. if corpse.map_id != looter.map_id { - return Err("corpse on another map".to_string()); + return Err(refused(LootRefusal::OutOfRange, "corpse on another map")); } if corpse.instance_id != looter.instance_id { - return Err("corpse in another instance".to_string()); + return Err(refused( + LootRefusal::OutOfRange, + "corpse in another instance", + )); } let (dx, dy, dz) = ( corpse.x - looter.x, @@ -836,12 +863,12 @@ pub(crate) fn apply_loot_money( corpse.z - looter.z, ); if dx * dx + dy * dy + dz * dz > LOOT_RANGE_SQ { - return Err("corpse out of range".to_string()); + return Err(refused(LootRefusal::OutOfRange, "corpse out of range")); } corpse_access_gate(ctx, looter_guid, target_guid)?; if corpse.money == 0 { - return Err("nothing to loot".to_string()); + return Err(refused(LootRefusal::NothingToLoot, "corpse purse is empty")); } let amount = corpse.money; diff --git a/module/src/loot/rolls.rs b/module/src/loot/rolls.rs index 041605ebf..e24aa139b 100644 --- a/module/src/loot/rolls.rs +++ b/module/src/loot/rolls.rs @@ -59,9 +59,10 @@ use crate::game_corpse_loot; use crate::game_group; // Group row: loot_method/loot_threshold/rr_cursor/master_looter_guid use crate::game_item_template; use crate::game_world_entity; +use lyracore_shared::loot::LootRefusal; use lyracore_shared::loot_roll::{event_kind as roll_event_kind, vote_kind}; -use super::CorpseLoot; +use super::{refused, CorpseLoot}; /// A member of a kill's loot-eligible set, resolved from the Loot Tag at death. One row per /// `(corpse_guid, recipient)`. Private, no @@ -412,7 +413,7 @@ pub(crate) fn cast_vote_on( vote: u8, ) -> Result<(), String> { if !matches!(vote, vote_kind::PASS | vote_kind::NEED | vote_kind::GREED) { - return Err("invalid vote".to_string()); + return Err(refused(LootRefusal::RollUnavailable, "invalid vote")); } let roll = ctx .db @@ -420,15 +421,15 @@ pub(crate) fn cast_vote_on( .by_corpse() .filter(&corpse_guid) .find(|r| r.slot == slot && !r.resolved) - .ok_or_else(|| "no roll open on that item".to_string())?; + .ok_or_else(|| refused(LootRefusal::RollUnavailable, "no roll open on that item"))?; let votes = ctx.db.game_loot_roll_vote(); let mut my_vote = votes .by_roll() .filter(&roll.id) .find(|v| v.voter_guid == voter_guid) - .ok_or_else(|| "not eligible for this roll".to_string())?; + .ok_or_else(|| refused(LootRefusal::RollUnavailable, "voter is not on this roll"))?; if my_vote.voted { - return Err("already voted".to_string()); + return Err(refused(LootRefusal::RollUnavailable, "already voted")); } my_vote.voted = true; my_vote.vote = vote; @@ -807,9 +808,12 @@ pub(crate) fn apply_master_give( .by_corpse() .filter(&corpse_guid) .find(|l| l.slot == loot_slot) - .ok_or_else(|| "no loot in that slot".to_string())?; + .ok_or_else(|| refused(LootRefusal::NothingToLoot, "no loot in that slot"))?; if !row.master_only || row.designated_looter_guid != master_guid { - return Err("you are not the master looter for that item".to_string()); + return Err(refused( + LootRefusal::NotMasterLooter, + "Actor does not hold the master-looter right on that row", + )); } let is_eligible = ctx .db @@ -818,7 +822,10 @@ pub(crate) fn apply_master_give( .filter(&corpse_guid) .any(|e| e.eligible_guid == target_guid); if !is_eligible { - return Err("that player is not eligible for this loot".to_string()); + return Err(refused( + LootRefusal::LootTagIneligible, + &format!("master-give target {target_guid} is not eligible"), + )); } crate::items::grant_item(ctx, target_guid, row.item_entry, row.count.max(1))?; loot.id().delete(row.id); diff --git a/module/src/loot/tag.rs b/module/src/loot/tag.rs index 18a6f353e..d5090cd97 100644 --- a/module/src/loot/tag.rs +++ b/module/src/loot/tag.rs @@ -9,6 +9,8 @@ use std::collections::BTreeSet; use spacetimedb::{table, ReducerContext, Table}; +use lyracore_shared::loot::LootRefusal; + use crate::game_world_entity; use super::game_corpse_loot_eligible; @@ -264,14 +266,6 @@ pub(crate) fn corpse_eligible_recipients(ctx: &ReducerContext, corpse_guid: u64) .collect() } -/// Stable classifier for a Loot Tag Gate Refusal. The detail keeps both durable identities for -/// logs and the Gateway's protocol mapping. -pub(crate) const LOOT_TAG_REFUSAL_CLASS: &str = "loot_tag_ineligible"; - -pub(crate) fn loot_tag_refusal(actor_guid: u64, corpse_guid: u64) -> String { - format!("{LOOT_TAG_REFUSAL_CLASS}: actor_guid={actor_guid} corpse_guid={corpse_guid}") -} - pub(crate) fn corpse_eligible_for_access(recipients: &[u64], actor_guid: u64) -> bool { recipients.binary_search(&actor_guid).is_ok() } @@ -288,9 +282,10 @@ pub(crate) fn corpse_access_gate( if corpse_eligible_for_access(&eligible, actor_guid) { return Ok(()); } - - spacetimedb::log::info!("loot tag refusal: actor_guid={actor_guid} corpse_guid={corpse_guid}"); - Err(loot_tag_refusal(actor_guid, corpse_guid)) + Err(super::refused( + LootRefusal::LootTagIneligible, + &format!("actor_guid={actor_guid} corpse_guid={corpse_guid}"), + )) } /// Clear a creature's live Loot Tag at combat end or despawn. Corpse and other dynamic flags stay @@ -598,14 +593,12 @@ fn verify_corpse_loot_gates( let empty = fixture_creature_guid(11); insert_fixture_corpse(ctx, origin, empty, x, 0); - expect_loot_tag_refusal( - corpse_access_gate(ctx, LOOT_TAG_FIXTURE_CHARACTER_A, empty), - empty, - )?; - expect_loot_tag_refusal( - crate::loot::apply_loot_money(ctx, LOOT_TAG_FIXTURE_CHARACTER_A, empty), + expect_loot_tag_refusal(corpse_access_gate(ctx, LOOT_TAG_FIXTURE_CHARACTER_A, empty))?; + expect_loot_tag_refusal(crate::loot::apply_loot_money( + ctx, + LOOT_TAG_FIXTURE_CHARACTER_A, empty, - )?; + ))?; let party_corpse = fixture_creature_guid(12); insert_fixture_corpse(ctx, origin, party_corpse, x, 9); @@ -634,7 +627,7 @@ fn verify_corpse_loot_gates( crate::loot::apply_loot_money(ctx, LOOT_TAG_FIXTURE_CHARACTER_E, party_corpse), crate::professions::skin_corpse(ctx, LOOT_TAG_FIXTURE_CHARACTER_E, party_corpse), ] { - expect_loot_tag_refusal(refusal, party_corpse)?; + expect_loot_tag_refusal(refusal)?; } if ctx .db @@ -714,14 +707,9 @@ fn insert_fixture_skinning(ctx: &ReducerContext, character_guid: u64) { } #[cfg(feature = "debug_reducers")] -fn expect_loot_tag_refusal(result: Result<(), String>, corpse_guid: u64) -> Result<(), String> { +fn expect_loot_tag_refusal(result: Result<(), String>) -> Result<(), String> { match result { - Err(reason) - if reason.starts_with(LOOT_TAG_REFUSAL_CLASS) - && reason.contains(&corpse_guid.to_string()) => - { - Ok(()) - } + Err(reason) if reason == LootRefusal::LootTagIneligible.as_tag() => Ok(()), Ok(()) => Err("foreign Actor passed the Loot Tag Gate".to_string()), Err(reason) => Err(format!("unexpected Loot Tag Refusal: {reason}")), } @@ -855,12 +843,4 @@ mod tests { assert!(!corpse_eligible_for_access(&[7], 8)); assert!(corpse_eligible_for_access(&[2, 7, 11], 7)); } - - #[test] - fn loot_tag_refusal_has_one_stable_class_and_both_guids() { - assert_eq!( - loot_tag_refusal(41, 99), - "loot_tag_ineligible: actor_guid=41 corpse_guid=99" - ); - } } From 814362721293d1971999f84ed4a98f6ae075eda6 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Fri, 4 Sep 2026 23:16:51 +0200 Subject: [PATCH 2/4] fix(loot): preserve unknown reducer outcomes --- gateway/src/stdb/reducers.rs | 77 ++++++++++++++++-- gateway/src/stdb/world_store.rs | 14 +++- gateway/src/world/handlers/loot.rs | 49 ++++++++---- gateway/src/world/handlers/mod.rs | 4 +- gateway/src/world/loot.rs | 15 +--- gateway/src/world/mod.rs | 3 +- gateway/src/world/store.rs | 26 ++++++- gateway/src/world/tests.rs | 121 ++++++++++++++++++++++++++--- module/src/gw.rs | 28 +++++-- 9 files changed, 279 insertions(+), 58 deletions(-) diff --git a/gateway/src/stdb/reducers.rs b/gateway/src/stdb/reducers.rs index 4f57660c6..72855a168 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, LootWindowRefusal, LootWindowRequestStatus}; +use crate::world::{ + ItemActionResult, LootActionStatus, LootWindowRefusal, LootWindowRequestStatus, +}; use lyracore_shared::auction::AuctionRefusal; use lyracore_shared::item::ItemRefusal; use lyracore_shared::loot::LootRefusal; @@ -1910,16 +1912,16 @@ impl Coordinator { corpse_guid: u64, loot_slot: u32, vote: u8, - ) -> Result<()> { + ) -> Result { if actor_guid == 0 { return Err(anyhow!("loot_roll: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + loot_action_status(call_reducer!( coord.conn.reducers, "gw_loot_roll", gw_loot_roll_then(actor_guid, corpse_guid, loot_slot, vote) - ) + )) } /// `CMSG_LOOT_MASTER_GIVE` — the master looter assigns an above- @@ -1931,16 +1933,16 @@ impl Coordinator { corpse_guid: u64, loot_slot: u8, target_guid: u64, - ) -> Result<()> { + ) -> Result { if actor_guid == 0 { return Err(anyhow!("loot_master_give: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + loot_action_status(call_reducer!( coord.conn.reducers, "gw_loot_master_give", gw_loot_master_give_then(actor_guid, corpse_guid, loot_slot, target_guid) - ) + )) } pub fn disenchant_item(&self, _account_id: u64, actor_guid: u64, slot: u8) -> Result<()> { @@ -2913,6 +2915,30 @@ impl Coordinator { ) } + /// Cast a realm-core Loot Roll vote and classify only a known typed Refusal as an answer. + pub fn realm_loot_vote( + &self, + corpse_guid: u64, + slot: u8, + actor_guid: u64, + vote: u8, + ) -> Result { + loot_action_status(call_reducer!( + self.0.call_pipe().conn.reducers, + "realm_loot_op", + realm_loot_op_then( + lyracore_shared::loot_roll::loot_op::VOTE, + corpse_guid, + slot, + 0, + actor_guid, + vote, + 0, + Vec::new() + ) + )) + } + /// `settle_loot_roll` — grant a resolved roll's item on THIS world shard, if it holds the /// matching corpse row. Operator-gated, coordinator connection; the loot-roll relay calls /// it on every connected world shard after observing realm-core's `ROLL_WON` event — the @@ -3151,6 +3177,21 @@ fn loot_request_status(result: Result<()>) -> Result { } } +/// The typed answer for Loot Roll and master-loot requests. An unknown rejection or transport +/// failure stays an error with an unknown durable result. +fn loot_action_status(result: Result<()>) -> Result { + match result { + Ok(()) => Ok(LootActionStatus::Applied), + Err(error) => match reducer_refusal_reason(&error).and_then(LootRefusal::parse_tag) { + Some(refusal) => { + log::debug!("stdb: loot action refused: {error:#}"); + Ok(LootActionStatus::Refused(refusal)) + } + None => Err(error), + }, + } +} + /// The Module's typed auction Refusal. Only a reducer the Module rejected carries a tag; a timeout, /// transport, or SDK failure stays an error with an unknown outcome. fn auction_refusal(error: &anyhow::Error) -> Option { @@ -3335,6 +3376,28 @@ mod loot_reducer_tests { assert!(loot_request_status(Err(error)).is_err(), "{text}"); } } + + #[test] + fn loot_action_status_keeps_unknown_results_as_errors() { + for refusal in [LootRefusal::RollUnavailable, LootRefusal::NotMasterLooter] { + assert_eq!( + loot_action_status(rejected(refusal.as_tag())).unwrap(), + LootActionStatus::Refused(refusal) + ); + } + + for error in [ + anyhow::Error::from(ReducerCallError::Rejected { + operation: "gw_loot_roll".to_string(), + reason: "loot:newer_module_refusal".to_string(), + }), + anyhow::Error::from(ReducerCallError::fatal( + "gw_loot_roll reducer timed out after 10s".to_string(), + )), + ] { + assert!(loot_action_status(Err(error)).is_err()); + } + } } #[cfg(test)] diff --git a/gateway/src/stdb/world_store.rs b/gateway/src/stdb/world_store.rs index 86af51a9c..d37feb25e 100644 --- a/gateway/src/stdb/world_store.rs +++ b/gateway/src/stdb/world_store.rs @@ -1029,7 +1029,7 @@ impl WorldStore for Coordinator { corpse_guid: u64, loot_slot: u32, vote: u8, - ) -> Result<()> { + ) -> Result { self.loot_roll(account_id, self_guid, corpse_guid, loot_slot, vote) } @@ -1058,6 +1058,16 @@ impl WorldStore for Coordinator { ) } + fn realm_loot_vote( + &self, + corpse_guid: u64, + slot: u8, + actor_guid: u64, + vote: u8, + ) -> Result { + self.realm_loot_vote(corpse_guid, slot, actor_guid, vote) + } + fn pending_local_rolls(&self) -> Result> { self.pending_local_rolls() } @@ -1080,7 +1090,7 @@ impl WorldStore for Coordinator { corpse_guid: u64, loot_slot: u8, target_guid: u64, - ) -> Result<()> { + ) -> Result { self.loot_master_give(account_id, self_guid, corpse_guid, loot_slot, target_guid) } fn gossip_select( diff --git a/gateway/src/world/handlers/loot.rs b/gateway/src/world/handlers/loot.rs index 185dcb082..cae9db46f 100644 --- a/gateway/src/world/handlers/loot.rs +++ b/gateway/src/world/handlers/loot.rs @@ -52,6 +52,13 @@ pub(crate) enum LootWindowRequestStatus { Refused(LootWindowRefusal), } +/// How the Module answered a Loot Roll or master-loot Durable Request. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum LootActionStatus { + Applied, + Refused(LootRefusal), +} + /// A loot Refusal as the vanilla client can hear it. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub(crate) enum LootWindowRefusal { @@ -205,6 +212,12 @@ fn refusal_transition( (next_state, refusal_outbound(refusal, target_guid)) } +fn finish_loot_action(status: LootActionStatus) { + if let LootActionStatus::Refused(refusal) = status { + log::debug!("world: loot action refused: {}", refusal.as_tag()); + } +} + /// A handled loot request returns all session state and client traffic to apply in order. pub(crate) enum LootWindowOutcome { Handled { @@ -236,10 +249,12 @@ pub(crate) fn dispatch_loot_window( if let LootWindowRequestStatus::Refused(refusal) = store.use_gameobject(player.account_id, actor_guid, target_guid)? { + let (next_state, outbound) = + refusal_transition(refusal, current_state, target_guid); return Ok(LootWindowOutcome::Handled { - next_state: current_state, + next_state, durable_request, - outbound: refusal_outbound(refusal, target_guid), + outbound, }); } let items = store.loot_target_items(target_guid, actor_guid)?; @@ -398,35 +413,25 @@ pub(crate) fn handle_loot( WorldState::InWorld(iw) => iw.self_guid, WorldState::CharSelect => 0, }; - if let Err(e) = loot::run_vote( + finish_loot_action(loot::run_vote( store, conn.account_id, self_guid, corpse_guid, c.item_slot, vote, - ) { - log::debug!( - "world: loot_roll ignored (account {}): {e}", - conn.account_id - ); - } + )?); } ClientOpcodeMessage::CMSG_LOOT_MASTER_GIVE(c) => { let corpse_guid = c.loot.guid(); let target_guid = c.player.guid(); - if let Err(e) = store.loot_master_give( + finish_loot_action(store.loot_master_give( conn.account_id, social::self_guid(conn).unwrap_or(0), corpse_guid, c.slot_id, target_guid, - ) { - log::debug!( - "world: loot_master_give ignored (account {}): {e}", - conn.account_id - ); - } + )?); } ClientOpcodeMessage::CMSG_GAMEOBJ_USE(request) => { let Some(actor_guid) = social::self_guid(conn).filter(|guid| *guid != 0) else { @@ -447,6 +452,11 @@ pub(crate) fn handle_loot( } } LootWindowRequestStatus::Refused(refusal) => { + if refusal.loot_error().is_some() { + if let WorldState::InWorld(iw) = &mut conn.state { + iw.open_loot = OpenLootState::default(); + } + } for outbound in refusal_outbound(refusal, target_guid) { send(tx, outbound)?; } @@ -1145,6 +1155,13 @@ mod tests { ..Default::default() }, ), + ( + open_chest(60), + InMemoryLootWindow { + use_refusal: Some(refusal.into()), + ..Default::default() + }, + ), ( ClientOpcodeMessage::CMSG_LOOT_MONEY, InMemoryLootWindow { diff --git a/gateway/src/world/handlers/mod.rs b/gateway/src/world/handlers/mod.rs index 4d10141c6..23090a013 100644 --- a/gateway/src/world/handlers/mod.rs +++ b/gateway/src/world/handlers/mod.rs @@ -46,8 +46,8 @@ pub(crate) use item::{ dispatch_item_action, ItemActionOutcome, ItemActionPlayer, ItemActionResult, ItemActionStore, }; pub(crate) use loot::{ - dispatch_loot_window, handle_loot, LootWindowOutcome, LootWindowPlayer, LootWindowRefusal, - LootWindowRequestStatus, LootWindowStore, OpenLootState, + dispatch_loot_window, handle_loot, LootActionStatus, LootWindowOutcome, LootWindowPlayer, + LootWindowRefusal, LootWindowRequestStatus, LootWindowStore, OpenLootState, }; pub(crate) use mail::handle_mail; pub(crate) use melee::{ diff --git a/gateway/src/world/loot.rs b/gateway/src/world/loot.rs index 3caef4139..5b9d7e21a 100644 --- a/gateway/src/world/loot.rs +++ b/gateway/src/world/loot.rs @@ -89,7 +89,7 @@ use anyhow::Result; -use super::WorldStore; +use super::{LootActionStatus, WorldStore}; use lyracore_shared::loot_roll::loot_op; /// One unresolved loot roll a world shard has created but not yet had promoted onto realm-core — @@ -120,20 +120,11 @@ pub(crate) fn run_vote( corpse_guid: u64, slot: u32, vote: u8, -) -> Result<()> { +) -> Result { let Some(realm) = store.realm_store() else { return store.loot_roll(account_id, self_guid, corpse_guid, slot, vote); }; - realm.realm_loot_op( - loot_op::VOTE, - corpse_guid, - slot as u8, - 0, - self_guid, - vote, - 0, - Vec::new(), - ) + realm.realm_loot_vote(corpse_guid, slot as u8, self_guid, vote) } /// Promote ONE world shard's staging roll onto realm-core, then clear the staging copy — the shared diff --git a/gateway/src/world/mod.rs b/gateway/src/world/mod.rs index 1a8c546c3..f158de852 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, ItemActionResult, LootWindowRefusal, LootWindowRequestStatus, + CreateAuctionRequest, ItemActionResult, LootActionStatus, LootWindowRefusal, + LootWindowRequestStatus, PlaceBidOutcome, PlaceBidRequest, TrainerBuyOutcome, WeatherStore, }; use login_queue::{Admission, LoginQueue}; diff --git a/gateway/src/world/store.rs b/gateway/src/world/store.rs index 2ec689e6d..aa63698ad 100644 --- a/gateway/src/world/store.rs +++ b/gateway/src/world/store.rs @@ -264,6 +264,28 @@ pub trait WorldStore: Err(anyhow!("this store does not host realm-wide loot rolls")) } + /// Cast one player vote on realm-core. The Store returns a typed gameplay answer while keeping + /// failures with an unknown durable result as `Err`. + fn realm_loot_vote( + &self, + corpse_guid: u64, + slot: u8, + actor_guid: u64, + vote: u8, + ) -> Result { + self.realm_loot_op( + lyracore_shared::loot_roll::loot_op::VOTE, + corpse_guid, + slot, + 0, + actor_guid, + vote, + 0, + Vec::new(), + )?; + Ok(LootActionStatus::Applied) + } + /// Every UNRESOLVED loot roll this WORLD SHARD has created but not yet had promoted onto /// realm-core — the relay's promotion queue. Empty by default, which is what makes the /// relay a no-op on an unsharded store and on realm-core's own handle (nothing is ever created @@ -988,7 +1010,7 @@ pub trait WorldStore: corpse_guid: u64, loot_slot: u32, vote: u8, - ) -> Result<()>; + ) -> Result; /// `CMSG_LOOT_MASTER_GIVE` — the master looter assigns an above- /// threshold row to `target_guid`. fn loot_master_give( @@ -998,7 +1020,7 @@ pub trait WorldStore: corpse_guid: u64, loot_slot: u8, target_guid: u64, - ) -> Result<()>; + ) -> Result; /// NOTIFY-ONLY module chokepoint for a gossip-option click — fired best-effort /// before the gateway's own gossip handling; failure never blocks the gossip reply. fn gossip_select( diff --git a/gateway/src/world/tests.rs b/gateway/src/world/tests.rs index 019bd9354..d498ab646 100644 --- a/gateway/src/world/tests.rs +++ b/gateway/src/world/tests.rs @@ -6,6 +6,7 @@ use super::handlers::{ use super::*; use crate::read_deadline::{DeadlineClock, PreAuthDeadline}; use lyracore_shared::item::ItemRefusal; +use lyracore_shared::loot::LootRefusal; use std::cell::Cell; use std::io::Cursor; use std::os::unix::net::UnixStream; @@ -464,6 +465,10 @@ struct InMemoryStore { loot_rolls: std::sync::Mutex>, /// Recorded `loot_master_give` calls: (corpse_guid, loot_slot, target_guid). loot_master_gives: std::sync::Mutex>, + /// Typed gameplay Refusal returned by loot-roll and master-loot action tests. + loot_action_refusal: Option, + /// Infrastructure failure returned by loot-roll and master-loot action tests. + loot_action_failure: Option, /// Recorded `use_item` slots. used_items: std::sync::Mutex>, /// Recorded `player_login` call count — the WORLDPORT_ACK test distinguishes the @@ -2518,15 +2523,18 @@ impl WorldStore for InMemoryStore { corpse_guid: u64, loot_slot: u32, vote: u8, - ) -> Result<()> { - if let Some(e) = &self.trade_error { - return Err(anyhow!("{e}")); + ) -> Result { + if let Some(failure) = &self.loot_action_failure { + return Err(anyhow!(failure.clone())); + } + if let Some(refusal) = self.loot_action_refusal { + return Ok(LootActionStatus::Refused(refusal)); } self.loot_rolls .lock() .unwrap() .push((corpse_guid, loot_slot, vote)); - Ok(()) + Ok(LootActionStatus::Applied) } fn loot_master_give( &self, @@ -2535,15 +2543,18 @@ impl WorldStore for InMemoryStore { corpse_guid: u64, loot_slot: u8, target_guid: u64, - ) -> Result<()> { - if let Some(e) = &self.trade_error { - return Err(anyhow!("{e}")); + ) -> Result { + if let Some(failure) = &self.loot_action_failure { + return Err(anyhow!(failure.clone())); + } + if let Some(refusal) = self.loot_action_refusal { + return Ok(LootActionStatus::Refused(refusal)); } self.loot_master_gives .lock() .unwrap() .push((corpse_guid, loot_slot, target_guid)); - Ok(()) + Ok(LootActionStatus::Applied) } // --- Realm-wide loot rolls --- @@ -2577,6 +2588,31 @@ impl WorldStore for InMemoryStore { Ok(()) } + fn realm_loot_vote( + &self, + corpse_guid: u64, + slot: u8, + actor_guid: u64, + vote: u8, + ) -> Result { + self.realm_loot_op( + lyracore_shared::loot_roll::loot_op::VOTE, + corpse_guid, + slot, + 0, + actor_guid, + vote, + 0, + Vec::new(), + )?; + if let Some(failure) = &self.loot_action_failure { + return Err(anyhow!(failure.clone())); + } + Ok(self + .loot_action_refusal + .map_or(LootActionStatus::Applied, LootActionStatus::Refused)) + } + fn pending_local_rolls(&self) -> Result> { Ok(self.pending_rolls.lock().unwrap().clone()) } @@ -7178,7 +7214,7 @@ fn loot_roll_rejection_is_logged_and_ignored_not_session_fatal() { // A rejection (no roll open / already voted / not eligible) must not tear the connection down — // the SAME session keeps working afterward (mirrors take_loot's per-action ignore discipline). let mut s = quest_store(); - s.trade_error = Some("no roll open on that item".to_string()); + s.loot_action_refusal = Some(LootRefusal::RollUnavailable); let store = std::sync::Arc::new(s); let (mut client, mut c_enc, mut c_dec, server) = enter_world(store.clone(), 1); CMSG_LOOT_ROLL { @@ -7203,6 +7239,73 @@ fn loot_roll_rejection_is_logged_and_ignored_not_session_fatal() { server.join().unwrap(); } +#[test] +fn loot_master_give_refusal_keeps_the_world_session_alive() { + let mut s = quest_store(); + s.loot_action_refusal = Some(LootRefusal::NotMasterLooter); + let store = std::sync::Arc::new(s); + let (mut client, mut c_enc, mut c_dec, server) = enter_world(store, 1); + CMSG_LOOT_MASTER_GIVE { + loot: Guid::new(60), + slot_id: 3, + player: Guid::new(9), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + CMSG_LOOT { + guid: Guid::new(61), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + + let (op, _) = read_raw_frame(&mut client, &mut c_dec); + assert_eq!(op, OP_LOOT_RESPONSE); + drop(client); + server.join().unwrap(); +} + +#[test] +fn loot_roll_timeout_ends_the_world_session() { + let mut s = quest_store(); + s.loot_action_failure = Some("gw_loot_roll reducer timed out after 10s".to_string()); + let store = std::sync::Arc::new(s); + let (mut client, mut c_enc, _c_dec, server) = enter_world(store, 1); + CMSG_LOOT_ROLL { + item: Guid::new(60), + item_slot: 2, + vote: RollVote::Greed, + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + drop(client); + + assert!( + server.join().is_err(), + "an unknown vote result must end the World Session" + ); +} + +#[test] +fn loot_master_give_transport_failure_ends_the_world_session() { + let mut s = quest_store(); + s.loot_action_failure = Some("gw_loot_master_give reducer transport disconnected".to_string()); + let store = std::sync::Arc::new(s); + let (mut client, mut c_enc, _c_dec, server) = enter_world(store, 1); + CMSG_LOOT_MASTER_GIVE { + loot: Guid::new(60), + slot_id: 3, + player: Guid::new(9), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + drop(client); + + assert!( + server.join().is_err(), + "an unknown master-loot result must end the World Session" + ); +} + fn loot_item_bytes(body: &[u8], index: usize) -> (u8, u32, u32, u32) { // Item N starts at byte 14 (8 guid + 1 method + 4 money + 1 count), 22 bytes each. let base = 14 + index * 22; diff --git a/module/src/gw.rs b/module/src/gw.rs index 11a8fc923..60e7cee0a 100644 --- a/module/src/gw.rs +++ b/module/src/gw.rs @@ -309,6 +309,20 @@ pub fn gw_cast_spell_at( crate::spell::apply_cast_spell_at(ctx, &caster, spell_id, target_guid, x, y, z) } +/// Taxi flight is an expected loot Refusal. Missing actors still carry the generic actor error, +/// which the Gateway treats as a failure with an unknown result. +fn loot_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::loot::refused( + lyracore_shared::loot::LootRefusal::LooterUnavailable, + "looter is in taxi flight", + )); + } + Ok(actor) +} + /// [`crate::actor::take_loot`] behind the gateway gate — one loot slot off a corpse/GO. #[reducer] pub fn gw_take_loot( @@ -318,7 +332,7 @@ pub fn gw_take_loot( loot_slot: u8, ) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + loot_actor(ctx, actor_guid)?; crate::actor::take_loot(ctx, actor_guid, corpse_guid, loot_slot) } @@ -331,7 +345,7 @@ pub fn gw_open_creature_loot( corpse_guid: u64, ) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + loot_actor(ctx, actor_guid)?; crate::actor::open_creature_loot(ctx, actor_guid, corpse_guid) } @@ -432,7 +446,7 @@ pub fn gw_cast_item_target( #[reducer] pub fn gw_loot_money(ctx: &ReducerContext, actor_guid: u64, target_guid: u64) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + loot_actor(ctx, actor_guid)?; crate::actor::loot_money(ctx, actor_guid, target_guid) } @@ -479,7 +493,7 @@ pub fn gw_use_gameobject( go_guid: u64, ) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + loot_actor(ctx, actor_guid)?; crate::actor::use_gameobject(ctx, actor_guid, go_guid) } @@ -1079,7 +1093,7 @@ pub fn gw_bind_home(ctx: &ReducerContext, actor_guid: u64) -> Result<(), String> #[reducer] pub fn gw_skin(ctx: &ReducerContext, actor_guid: u64, corpse_guid: u64) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + loot_actor(ctx, actor_guid)?; crate::professions::skin_corpse(ctx, actor_guid, corpse_guid) } @@ -1093,7 +1107,7 @@ pub fn gw_loot_roll( vote: u8, ) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + loot_actor(ctx, actor_guid)?; crate::loot::cast_vote_on(ctx, corpse_guid, loot_slot as u8, actor_guid, vote) } @@ -1107,7 +1121,7 @@ pub fn gw_loot_master_give( target_guid: u64, ) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + loot_actor(ctx, actor_guid)?; crate::loot::apply_master_give(ctx, actor_guid, corpse_guid, loot_slot, target_guid) } From 61c306cfbb34dbc7569b276532bc050ca547b239 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Fri, 4 Sep 2026 23:58:30 +0200 Subject: [PATCH 3/4] fix(loot): reject unknown reducer outcomes --- crates/lyracore-shared/src/loot.rs | 39 +++++++++- gateway/src/stdb/reducers.rs | 120 ++++++++++++++++++++++------- gateway/src/world/handlers/loot.rs | 27 +++++++ module/src/gw.rs | 37 ++++++--- 4 files changed, 184 insertions(+), 39 deletions(-) diff --git a/crates/lyracore-shared/src/loot.rs b/crates/lyracore-shared/src/loot.rs index 85b5eceb8..e1439e9ea 100644 --- a/crates/lyracore-shared/src/loot.rs +++ b/crates/lyracore-shared/src/loot.rs @@ -50,9 +50,35 @@ impl LootRefusal { } } +/// A failure in the trusted Gateway entry before a loot core can answer the request. These tags +/// cross the reducer boundary so legacy untagged gameplay compatibility cannot hide a broken +/// Operator or Actor invariant. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum LootBoundaryFailure { + OperatorRejected, + MissingActor, +} + +impl LootBoundaryFailure { + pub const ALL: [Self; 2] = [Self::OperatorRejected, Self::MissingActor]; + + pub fn as_tag(self) -> &'static str { + match self { + Self::OperatorRejected => "loot:boundary_operator_rejected", + Self::MissingActor => "loot:boundary_missing_actor", + } + } + + pub fn parse_tag(tag: &str) -> Option { + Self::ALL + .into_iter() + .find(|failure| failure.as_tag() == tag) + } +} + #[cfg(test)] mod tests { - use super::LootRefusal; + use super::{LootBoundaryFailure, LootRefusal}; #[test] fn every_refusal_tag_round_trips() { @@ -69,4 +95,15 @@ mod tests { None ); } + + #[test] + fn every_boundary_failure_tag_round_trips() { + for failure in LootBoundaryFailure::ALL { + assert_eq!( + LootBoundaryFailure::parse_tag(failure.as_tag()), + Some(failure) + ); + assert_eq!(LootRefusal::parse_tag(failure.as_tag()), None); + } + } } diff --git a/gateway/src/stdb/reducers.rs b/gateway/src/stdb/reducers.rs index 72855a168..34b51aa7a 100644 --- a/gateway/src/stdb/reducers.rs +++ b/gateway/src/stdb/reducers.rs @@ -16,7 +16,7 @@ use crate::world::{ }; use lyracore_shared::auction::AuctionRefusal; use lyracore_shared::item::ItemRefusal; -use lyracore_shared::loot::LootRefusal; +use lyracore_shared::loot::{LootBoundaryFailure, LootRefusal}; use lyracore_shared::trainer::TrainerRefusal; static NEXT_TAXI_REQUEST_ID: OnceLock = OnceLock::new(); @@ -1060,7 +1060,7 @@ impl Coordinator { return Err(anyhow!("use_gameobject: actor_guid unresolved")); } let coord = self.0.call_pipe(); - loot_request_status(call_reducer!( + legacy_loot_request_status(call_reducer!( coord.conn.reducers, "gw_use_gameobject", gw_use_gameobject_then(actor_guid, go_guid) @@ -1838,7 +1838,7 @@ impl Coordinator { return Err(anyhow!("loot_money: actor_guid unresolved")); } let coord = self.0.call_pipe(); - loot_request_status(call_reducer!( + strict_loot_request_status(call_reducer!( coord.conn.reducers, "gw_loot_money", gw_loot_money_then(actor_guid, target_guid) @@ -1856,7 +1856,7 @@ impl Coordinator { return Err(anyhow!("open_creature_loot: actor_guid unresolved")); } let coord = self.0.call_pipe(); - loot_request_status(call_reducer!( + strict_loot_request_status(call_reducer!( coord.conn.reducers, "gw_open_creature_loot", gw_open_creature_loot_then(actor_guid, corpse_guid) @@ -1878,7 +1878,7 @@ impl Coordinator { return Err(anyhow!("take_loot: actor_guid unresolved")); } let coord = self.0.call_pipe(); - loot_request_status(call_reducer!( + strict_loot_request_status(call_reducer!( coord.conn.reducers, "gw_take_loot", gw_take_loot_then(actor_guid, corpse_guid, loot_slot) @@ -1895,7 +1895,7 @@ impl Coordinator { return Err(anyhow!("skin_corpse: actor_guid unresolved")); } let coord = self.0.call_pipe(); - loot_request_status(call_reducer!( + legacy_loot_request_status(call_reducer!( coord.conn.reducers, "gw_skin", gw_skin_then(actor_guid, corpse_guid) @@ -3158,19 +3158,50 @@ fn next_auction_operation_id() -> Result { } } -/// A loot Durable Request the Module rejected is a Refusal outcome; a timeout, transport, or SDK -/// failure stays an error with an unknown durable result and ends the session. Module cores outside -/// the loot family still refuse with untagged prose, so an untagged Rejection is unanswered. -fn loot_request_status(result: Result<()>) -> Result { +#[derive(Clone, Copy)] +enum UntaggedLootRejection { + Fatal, + LegacyUnanswered, +} + +/// Classify a Durable Request from a core whose gameplay refusals all have loot tags. +fn strict_loot_request_status(result: Result<()>) -> Result { + loot_request_status(result, UntaggedLootRejection::Fatal) +} + +/// Preserve silent gameplay refusals from the legacy GameObject and skinning cores. Boundary +/// failures and every tagged result remain explicit, so this compatibility cannot hide them. +fn legacy_loot_request_status(result: Result<()>) -> Result { + loot_request_status(result, UntaggedLootRejection::LegacyUnanswered) +} + +fn loot_request_status( + result: Result<()>, + untagged: UntaggedLootRejection, +) -> Result { match result { Ok(()) => Ok(LootWindowRequestStatus::Applied), Err(error) => match reducer_refusal_reason(&error) { Some(reason) => { - log::debug!("stdb: loot Durable Request refused: {error:#}"); - Ok(LootWindowRequestStatus::Refused( - LootRefusal::parse_tag(reason) - .map_or(LootWindowRefusal::Unanswered, Into::into), - )) + if LootBoundaryFailure::parse_tag(reason).is_some() { + return Err(error); + } + if let Some(refusal) = LootRefusal::parse_tag(reason) { + log::debug!("stdb: loot Durable Request refused: {error:#}"); + return Ok(LootWindowRequestStatus::Refused(refusal.into())); + } + if reason.starts_with("loot:") { + return Err(error); + } + match untagged { + UntaggedLootRejection::LegacyUnanswered => { + log::debug!("stdb: legacy loot Durable Request refused: {error:#}"); + Ok(LootWindowRequestStatus::Refused( + LootWindowRefusal::Unanswered, + )) + } + UntaggedLootRejection::Fatal => Err(error), + } } None => Err(error), }, @@ -3325,8 +3356,11 @@ mod loot_reducer_tests { use super::*; use crate::stdb::connection::ReducerCallError; - fn refusal_of(result: Result<()>) -> LootWindowRefusal { - match loot_request_status(result) { + fn refusal_of( + result: Result<()>, + classify: fn(Result<()>) -> Result, + ) -> LootWindowRefusal { + match classify(result) { Ok(LootWindowRequestStatus::Refused(refusal)) => refusal, Ok(LootWindowRequestStatus::Applied) => panic!("a Refusal was applied"), Err(error) => panic!("a Refusal ended the session: {error:#}"), @@ -3344,17 +3378,51 @@ mod loot_reducer_tests { #[test] fn every_module_refusal_tag_becomes_one_client_answer() { for refusal in LootRefusal::ALL { + for classify in [ + strict_loot_request_status as fn(Result<()>) -> Result, + legacy_loot_request_status, + ] { + assert_eq!( + refusal_of(rejected(refusal.as_tag()), classify), + LootWindowRefusal::from(refusal), + "{refusal:?}" + ); + } + } + } + + #[test] + fn only_legacy_cores_keep_untagged_gameplay_refusals_unanswered() { + for reason in ["it is locked", "not a beast", "inventory is full"] { assert_eq!( - refusal_of(rejected(refusal.as_tag())), - LootWindowRefusal::from(refusal), - "{refusal:?}" + refusal_of(rejected(reason), legacy_loot_request_status), + LootWindowRefusal::Unanswered, + "{reason}" + ); + assert!( + strict_loot_request_status(rejected(reason)).is_err(), + "{reason}" + ); + } + } + + #[test] + fn boundary_failures_and_unknown_loot_tags_are_fatal() { + let reasons = LootBoundaryFailure::ALL + .into_iter() + .map(LootBoundaryFailure::as_tag) + .chain(["loot:newer_module_refusal"]); + + for reason in reasons { + assert!( + strict_loot_request_status(rejected(reason)).is_err(), + "{reason}" + ); + assert!( + legacy_loot_request_status(rejected(reason)).is_err(), + "{reason}" ); } - // A Module core outside the loot family still refuses with prose. - assert_eq!( - refusal_of(rejected("inventory is full")), - LootWindowRefusal::Unanswered - ); } #[test] @@ -3373,7 +3441,7 @@ mod loot_reducer_tests { ]; for error in not_refusals { let text = format!("{error:#}"); - assert!(loot_request_status(Err(error)).is_err(), "{text}"); + assert!(strict_loot_request_status(Err(error)).is_err(), "{text}"); } } diff --git a/gateway/src/world/handlers/loot.rs b/gateway/src/world/handlers/loot.rs index cae9db46f..1cf9bbe6e 100644 --- a/gateway/src/world/handlers/loot.rs +++ b/gateway/src/world/handlers/loot.rs @@ -947,6 +947,33 @@ mod tests { assert!(store.item_reads.lock().unwrap().is_empty()); } + #[test] + fn missing_actor_chest_failure_propagates_without_reading_loot() { + let failure = lyracore_shared::loot::LootBoundaryFailure::MissingActor.as_tag(); + let store = InMemoryLootWindow { + use_fatal_error: Some(failure.into()), + ..Default::default() + }; + + let error = dispatch_loot_window( + &store, + player(), + OpenLootState { + target_guid: Some(11), + }, + open_chest(90), + ) + .err() + .expect("missing Actor failure was handled"); + + assert_eq!(error.to_string(), failure); + assert_eq!( + store.operations.lock().unwrap().as_slice(), + &["use gameobject"] + ); + assert!(store.item_reads.lock().unwrap().is_empty()); + } + #[test] fn creature_open_ownership_refusal_closes_the_window_without_loot_reads() { let store = InMemoryLootWindow { diff --git a/module/src/gw.rs b/module/src/gw.rs index 60e7cee0a..74f77a9bd 100644 --- a/module/src/gw.rs +++ b/module/src/gw.rs @@ -21,6 +21,7 @@ //! are deleted, and every player action reaches the module through a `gw_*` verb on the //! gateway's privileged connection. +use lyracore_shared::loot::LootBoundaryFailure; use spacetimedb::{reducer, table, Identity, ReducerContext, ScheduleAt, Table, Timestamp}; use crate::game_account; @@ -309,11 +310,12 @@ pub fn gw_cast_spell_at( crate::spell::apply_cast_spell_at(ctx, &caster, spell_id, target_guid, x, y, z) } -/// Taxi flight is an expected loot Refusal. Missing actors still carry the generic actor error, -/// which the Gateway treats as a failure with an unknown result. +/// Taxi flight is an expected loot Refusal. A missing Actor is a tagged boundary failure so legacy +/// untagged core compatibility cannot turn it into a gameplay outcome. fn loot_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())?; + let actor = acting_entity_by_guid(ctx, actor_guid).ok_or_else(|| { + loot_boundary_failure(LootBoundaryFailure::MissingActor, "mover not in world") + })?; if crate::taxi::is_in_flight(ctx, actor_guid) { return Err(crate::loot::refused( lyracore_shared::loot::LootRefusal::LooterUnavailable, @@ -323,6 +325,16 @@ fn loot_actor(ctx: &ReducerContext, actor_guid: u64) -> Result String { + let tag = failure.as_tag(); + spacetimedb::log::error!("loot boundary failure {tag}: {detail}"); + tag.to_string() +} + +fn loot_operator_error(detail: String) -> String { + loot_boundary_failure(LootBoundaryFailure::OperatorRejected, &detail) +} + /// [`crate::actor::take_loot`] behind the gateway gate — one loot slot off a corpse/GO. #[reducer] pub fn gw_take_loot( @@ -331,7 +343,7 @@ pub fn gw_take_loot( corpse_guid: u64, loot_slot: u8, ) -> Result<(), String> { - require_operator(ctx)?; + require_operator(ctx).map_err(loot_operator_error)?; loot_actor(ctx, actor_guid)?; crate::actor::take_loot(ctx, actor_guid, corpse_guid, loot_slot) } @@ -344,7 +356,7 @@ pub fn gw_open_creature_loot( actor_guid: u64, corpse_guid: u64, ) -> Result<(), String> { - require_operator(ctx)?; + require_operator(ctx).map_err(loot_operator_error)?; loot_actor(ctx, actor_guid)?; crate::actor::open_creature_loot(ctx, actor_guid, corpse_guid) } @@ -445,7 +457,7 @@ pub fn gw_cast_item_target( /// [`crate::actor::loot_money`] behind the gateway gate. #[reducer] pub fn gw_loot_money(ctx: &ReducerContext, actor_guid: u64, target_guid: u64) -> Result<(), String> { - require_operator(ctx)?; + require_operator(ctx).map_err(loot_operator_error)?; loot_actor(ctx, actor_guid)?; crate::actor::loot_money(ctx, actor_guid, target_guid) } @@ -492,7 +504,7 @@ pub fn gw_use_gameobject( actor_guid: u64, go_guid: u64, ) -> Result<(), String> { - require_operator(ctx)?; + require_operator(ctx).map_err(loot_operator_error)?; loot_actor(ctx, actor_guid)?; crate::actor::use_gameobject(ctx, actor_guid, go_guid) } @@ -1092,7 +1104,7 @@ pub fn gw_bind_home(ctx: &ReducerContext, actor_guid: u64) -> Result<(), String> /// [`crate::professions::skin_corpse`] with the skinner named by guid. #[reducer] pub fn gw_skin(ctx: &ReducerContext, actor_guid: u64, corpse_guid: u64) -> Result<(), String> { - require_operator(ctx)?; + require_operator(ctx).map_err(loot_operator_error)?; loot_actor(ctx, actor_guid)?; crate::professions::skin_corpse(ctx, actor_guid, corpse_guid) } @@ -1106,7 +1118,7 @@ pub fn gw_loot_roll( loot_slot: u32, vote: u8, ) -> Result<(), String> { - require_operator(ctx)?; + require_operator(ctx).map_err(loot_operator_error)?; loot_actor(ctx, actor_guid)?; crate::loot::cast_vote_on(ctx, corpse_guid, loot_slot as u8, actor_guid, vote) } @@ -1120,7 +1132,7 @@ pub fn gw_loot_master_give( loot_slot: u8, target_guid: u64, ) -> Result<(), String> { - require_operator(ctx)?; + require_operator(ctx).map_err(loot_operator_error)?; loot_actor(ctx, actor_guid)?; crate::loot::apply_master_give(ctx, actor_guid, corpse_guid, loot_slot, target_guid) } @@ -1521,8 +1533,9 @@ mod tests { .trim_start(); assert!( body.starts_with("require_operator(ctx)?;") + || body.starts_with("require_operator(ctx).map_err(loot_operator_error)?;") || body.starts_with("if ctx.sender() != ctx.database_identity()"), - "a reducer here must open with require_operator(ctx)?; (gateway verb) or the \ + "a reducer here must open with the direct Operator gate (gateway verb) or the \ scheduler-only sender fence (scheduled reducer) — got:\n{}", &body[..body.len().min(120)] ); From 89d8b5c41060e817b519740f64eebec86ba9d1c6 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Sat, 5 Sep 2026 00:21:42 +0200 Subject: [PATCH 4/4] fix(loot): classify master delivery refusals --- crates/lyracore-shared/src/loot.rs | 10 ++- gateway/src/stdb/reducers.rs | 7 +- gateway/src/world/handlers/loot.rs | 12 ++-- gateway/src/world/tests.rs | 106 ++++++++++++++++++++++------- module/src/loot/rolls.rs | 30 +++++++- 5 files changed, 133 insertions(+), 32 deletions(-) diff --git a/crates/lyracore-shared/src/loot.rs b/crates/lyracore-shared/src/loot.rs index e1439e9ea..38eeabd96 100644 --- a/crates/lyracore-shared/src/loot.rs +++ b/crates/lyracore-shared/src/loot.rs @@ -18,10 +18,14 @@ pub enum LootRefusal { RollUnavailable, /// The Actor does not hold the master-looter right on that row. NotMasterLooter, + /// The master-loot recipient is no longer in the world. + RecipientUnavailable, + /// The master-loot recipient has no room for the item. + RecipientInventoryFull, } impl LootRefusal { - pub const ALL: [Self; 7] = [ + pub const ALL: [Self; 9] = [ Self::LootTagIneligible, Self::OutOfRange, Self::NoLootSource, @@ -29,6 +33,8 @@ impl LootRefusal { Self::NothingToLoot, Self::RollUnavailable, Self::NotMasterLooter, + Self::RecipientUnavailable, + Self::RecipientInventoryFull, ]; pub fn as_tag(self) -> &'static str { @@ -40,6 +46,8 @@ impl LootRefusal { Self::NothingToLoot => "loot:nothing_to_loot", Self::RollUnavailable => "loot:roll_unavailable", Self::NotMasterLooter => "loot:not_master_looter", + Self::RecipientUnavailable => "loot:recipient_unavailable", + Self::RecipientInventoryFull => "loot:recipient_inventory_full", } } diff --git a/gateway/src/stdb/reducers.rs b/gateway/src/stdb/reducers.rs index 34b51aa7a..66bc5cb80 100644 --- a/gateway/src/stdb/reducers.rs +++ b/gateway/src/stdb/reducers.rs @@ -3447,7 +3447,12 @@ mod loot_reducer_tests { #[test] fn loot_action_status_keeps_unknown_results_as_errors() { - for refusal in [LootRefusal::RollUnavailable, LootRefusal::NotMasterLooter] { + for refusal in [ + LootRefusal::RollUnavailable, + LootRefusal::NotMasterLooter, + LootRefusal::RecipientUnavailable, + LootRefusal::RecipientInventoryFull, + ] { assert_eq!( loot_action_status(rejected(refusal.as_tag())).unwrap(), LootActionStatus::Refused(refusal) diff --git a/gateway/src/world/handlers/loot.rs b/gateway/src/world/handlers/loot.rs index 1cf9bbe6e..f05bba70d 100644 --- a/gateway/src/world/handlers/loot.rs +++ b/gateway/src/world/handlers/loot.rs @@ -70,8 +70,8 @@ pub(crate) enum LootWindowRefusal { Unanswered, } -// Module cores outside the loot family still refuse with untagged prose, so an untyped Rejection -// lands on `Unanswered` rather than ending the session. +// Only recognized Module tags reach this conversion. The Coordinator maps untagged legacy +// GameObject and skinning refusals to `Unanswered` and propagates strict-core failures. impl From for LootWindowRefusal { fn from(refusal: LootRefusal) -> Self { match refusal { @@ -81,7 +81,9 @@ impl From for LootWindowRefusal { | LootRefusal::LooterUnavailable | LootRefusal::NothingToLoot | LootRefusal::RollUnavailable - | LootRefusal::NotMasterLooter => Self::Unanswered, + | LootRefusal::NotMasterLooter + | LootRefusal::RecipientUnavailable + | LootRefusal::RecipientInventoryFull => Self::Unanswered, } } } @@ -1165,7 +1167,9 @@ mod tests { | LootRefusal::LooterUnavailable | LootRefusal::NothingToLoot | LootRefusal::RollUnavailable - | LootRefusal::NotMasterLooter => None, + | LootRefusal::NotMasterLooter + | LootRefusal::RecipientUnavailable + | LootRefusal::RecipientInventoryFull => None, }; let open_window = OpenLootState { target_guid: Some(60), diff --git a/gateway/src/world/tests.rs b/gateway/src/world/tests.rs index d498ab646..d6e0b43c6 100644 --- a/gateway/src/world/tests.rs +++ b/gateway/src/world/tests.rs @@ -397,6 +397,10 @@ struct InMemoryStore { items_looted: std::sync::Mutex>, /// Recorded `skin_corpse` targets (the empty-loot-window skinning fallback). skinned: std::sync::Mutex>, + /// Typed legacy skinning refusal returned by the empty-loot fallback. + skinning_refusal: Option, + /// Infrastructure failure returned by the empty-loot skinning fallback. + skinning_failure: Option, /// Recorded `vendor_buyback` calls: (vendor_guid, slot) — pins the 69→0 slot mapping. bought_back: std::sync::Mutex>, /// What `talent_grant_spell` returns (0 = passive talent → no SMSG_LEARNED_SPELL push). @@ -3245,10 +3249,11 @@ impl LootWindowStore for InMemoryStore { _actor_guid: u64, target_guid: u64, ) -> Result { - if self.trade_error.is_some() { - return Ok(LootWindowRequestStatus::Refused( - LootWindowRefusal::Unanswered, - )); + if let Some(error) = &self.skinning_failure { + return Err(anyhow!(error.clone())); + } + if let Some(refusal) = self.skinning_refusal { + return Ok(LootWindowRequestStatus::Refused(refusal)); } self.skinned.lock().unwrap().push(target_guid); Ok(LootWindowRequestStatus::Applied) @@ -7018,6 +7023,51 @@ fn loot_with_a_zero_player_guid_is_handled_without_panicking() { assert!(store.skinned.lock().unwrap().is_empty()); } +#[test] +fn skinning_refusal_keeps_the_world_session_alive() { + let store = std::sync::Arc::new(InMemoryStore { + skinning_refusal: Some(LootWindowRefusal::Unanswered), + ..quest_store() + }); + let (mut client, mut c_enc, mut c_dec, server) = enter_world(store.clone(), 1); + + for target_guid in [60, 61] { + CMSG_LOOT { + guid: Guid::new(target_guid), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + let (opcode, body) = read_raw_frame(&mut client, &mut c_dec); + assert_eq!(opcode, OP_LOOT_RESPONSE); + assert_eq!(&body[0..8], &target_guid.to_le_bytes()); + } + + drop(client); + server.join().unwrap(); + assert!(store.skinned.lock().unwrap().is_empty()); +} + +#[test] +fn skinning_infrastructure_failure_ends_the_world_session() { + let store = std::sync::Arc::new(InMemoryStore { + skinning_failure: Some("gw_skin reducer timed out after 10s".to_string()), + ..quest_store() + }); + let (mut client, mut c_enc, _c_dec, server) = enter_world(store, 1); + + CMSG_LOOT { + guid: Guid::new(60), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + drop(client); + + assert!( + server.join().is_err(), + "a skinning timeout must end the World Session" + ); +} + #[test] fn loot_opens_the_window_and_loot_money_drives_the_tracked_guid() { // CMSG_LOOT arms the open-loot state and replies the RAW loot window (guid + money in the body); @@ -7240,28 +7290,34 @@ fn loot_roll_rejection_is_logged_and_ignored_not_session_fatal() { } #[test] -fn loot_master_give_refusal_keeps_the_world_session_alive() { - let mut s = quest_store(); - s.loot_action_refusal = Some(LootRefusal::NotMasterLooter); - let store = std::sync::Arc::new(s); - let (mut client, mut c_enc, mut c_dec, server) = enter_world(store, 1); - CMSG_LOOT_MASTER_GIVE { - loot: Guid::new(60), - slot_id: 3, - player: Guid::new(9), - } - .write_encrypted_client(&mut client, &mut c_enc) - .unwrap(); - CMSG_LOOT { - guid: Guid::new(61), - } - .write_encrypted_client(&mut client, &mut c_enc) - .unwrap(); +fn loot_master_give_refusals_keep_the_world_session_alive() { + for refusal in [ + LootRefusal::NotMasterLooter, + LootRefusal::RecipientUnavailable, + LootRefusal::RecipientInventoryFull, + ] { + let mut s = quest_store(); + s.loot_action_refusal = Some(refusal); + let store = std::sync::Arc::new(s); + let (mut client, mut c_enc, mut c_dec, server) = enter_world(store, 1); + CMSG_LOOT_MASTER_GIVE { + loot: Guid::new(60), + slot_id: 3, + player: Guid::new(9), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + CMSG_LOOT { + guid: Guid::new(61), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); - let (op, _) = read_raw_frame(&mut client, &mut c_dec); - assert_eq!(op, OP_LOOT_RESPONSE); - drop(client); - server.join().unwrap(); + let (op, _) = read_raw_frame(&mut client, &mut c_dec); + assert_eq!(op, OP_LOOT_RESPONSE, "{refusal:?}"); + drop(client); + server.join().unwrap(); + } } #[test] diff --git a/module/src/loot/rolls.rs b/module/src/loot/rolls.rs index e24aa139b..b96217684 100644 --- a/module/src/loot/rolls.rs +++ b/module/src/loot/rolls.rs @@ -827,16 +827,44 @@ pub(crate) fn apply_master_give( &format!("master-give target {target_guid} is not eligible"), )); } - crate::items::grant_item(ctx, target_guid, row.item_entry, row.count.max(1))?; + crate::helpers::live_entity(ctx, target_guid).map_err(|_| { + refused( + LootRefusal::RecipientUnavailable, + &format!("master-loot recipient {target_guid} is not in the world"), + ) + })?; + crate::items::grant_item(ctx, target_guid, row.item_entry, row.count.max(1)) + .map_err(master_delivery_error)?; loot.id().delete(row.id); super::refresh_lootable(ctx, corpse_guid); Ok(()) } +/// Convert the stable capacity refusal. Missing templates and other grant failures remain errors. +fn master_delivery_error(error: String) -> String { + if error == lyracore_shared::mail::INVENTORY_FULL { + refused(LootRefusal::RecipientInventoryFull, &error) + } else { + error + } +} + #[cfg(test)] mod tests { use super::*; + #[test] + fn master_delivery_classifies_only_the_stable_capacity_refusal() { + assert_eq!( + master_delivery_error(lyracore_shared::mail::INVENTORY_FULL.to_string()), + LootRefusal::RecipientInventoryFull.as_tag() + ); + assert_eq!( + master_delivery_error("no such item 123".to_string()), + "no such item 123" + ); + } + /// `group_loot_decision_for_row` — the threshold-split matrix: FFA never restricts; ROUND_ROBIN /// always designates regardless of quality; GROUP splits on the threshold (below → designate, /// at/above → roll); NEED_BEFORE_GREED always rolls regardless of quality; MASTER splits on the