From 530e5c5343d079471e7ae11179d60ae3ab0d9cda Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Thu, 3 Sep 2026 23:13:16 +0200 Subject: [PATCH 1/8] refactor(social): typed refusals across the tier contract The friends, ignore and party paths matched the Module's error prose to pick a client result code, so a reducer timeout reached the client as a gameplay answer. Two shared enums now define the contract: `GroupRefusal` in `lyracore_shared::group` (replacing the `err` string constants) and `ContactRefusal` in a new `lyracore_shared::social`. Module cores return the enum, the reducer edge sends only the tag, and the Gateway parses a tag only from a reducer the Module rejected. Co-Authored-By: Claude Fable 5.1 --- crates/lyracore-shared/src/group.rs | 102 +++++++- crates/lyracore-shared/src/lib.rs | 1 + crates/lyracore-shared/src/social.rs | 61 +++++ gateway/src/stdb/reducers.rs | 140 +++++++--- gateway/src/stdb/subscriptions.rs | 15 +- gateway/src/stdb/world_store.rs | 71 ++++- gateway/src/world/handlers/query.rs | 30 ++- gateway/src/world/mod.rs | 1 + gateway/src/world/party.rs | 109 +++++--- gateway/src/world/party_tests.rs | 35 +-- gateway/src/world/social.rs | 277 ++++++++++++-------- gateway/src/world/store.rs | 73 ++++-- gateway/src/world/tests.rs | 377 +++++++++++++++++++++------ module/src/chat.rs | 67 +++-- module/src/group.rs | 169 +++++++----- 15 files changed, 1106 insertions(+), 422 deletions(-) create mode 100644 crates/lyracore-shared/src/social.rs diff --git a/crates/lyracore-shared/src/group.rs b/crates/lyracore-shared/src/group.rs index 07a8f8488..4ed50286b 100644 --- a/crates/lyracore-shared/src/group.rs +++ b/crates/lyracore-shared/src/group.rs @@ -1,6 +1,6 @@ //! The module↔gateway GROUP wire contract — event-kind codes, the roster-payload grammar, and the -//! reducer error strings the gateway classifies into `PARTY_COMMAND_RESULT`. Cross-boundary constants -//! live HERE, both crates import: a module-side renumber, delimiter change, or reworded `Err` becomes +//! typed [`GroupRefusal`] the gateway turns into `PARTY_COMMAND_RESULT`. Cross-boundary constants +//! live HERE, both crates import: a module-side renumber, delimiter change, or new refusal becomes //! a compile error on the gateway side instead of a runtime mismatch. Same precedent as //! `type_mask`/`npc_flags`. @@ -105,15 +105,83 @@ pub mod bot_op { pub const LEAVE: u8 = 1; } -/// The group reducers' stable `Err` strings. Most map to `PartyResult` variants. The intent claim -/// result instead tells a losing Gateway callback to stop. Exact matches keep both crates in sync. -pub mod err { - pub const ALREADY_IN_GROUP: &str = "already in a group"; - pub const BOT_INVITE_INTENT_ALREADY_CLAIMED: &str = "bot invite intent already claimed"; - pub const GROUP_FULL: &str = "group is full"; - pub const NOT_LEADER: &str = "not the leader"; - pub const NOT_IN_GROUP: &str = "not in a group"; - pub const TARGET_NOT_IN_GROUP: &str = "not in your group"; +/// Why the Module refused a party Durable Request. The tag is the whole reducer error text, so +/// neither tier matches on human prose. Most variants become a `PartyResult` the client renders; +/// [`GroupRefusal::IntentAlreadyClaimed`] instead tells a losing Gateway callback to stop. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum GroupRefusal { + /// A Character may not invite itself. + InviteSelf, + /// No Character row holds a guid the op named. + NoSuchPlayer, + /// The invited Character has no live entity. + TargetOffline, + /// The invited Character is already in a party. + AlreadyInGroup, + /// The party is at its member cap. + GroupFull, + /// The actor is in a party but does not lead it. + NotLeader, + /// The actor is in no party. + NotInGroup, + /// The named target is in no party, or in a different one. + TargetNotInGroup, + /// Accept or decline ran with no invite standing. + NoPendingInvite, + /// The inviter is gone, or no longer leads the party the invite named. + InviterUnavailable, + /// A leader tried to kick itself; leaving is the op for that. + KickSelf, + /// The loot method, threshold, or master looter is not a legal setting. + InvalidLootRules, + /// Another Gateway already claimed this bot invite intent. + IntentAlreadyClaimed, + /// A durable row the party rules require is missing. + Database, +} + +impl GroupRefusal { + pub const ALL: [Self; 14] = [ + Self::InviteSelf, + Self::NoSuchPlayer, + Self::TargetOffline, + Self::AlreadyInGroup, + Self::GroupFull, + Self::NotLeader, + Self::NotInGroup, + Self::TargetNotInGroup, + Self::NoPendingInvite, + Self::InviterUnavailable, + Self::KickSelf, + Self::InvalidLootRules, + Self::IntentAlreadyClaimed, + Self::Database, + ]; + + pub fn as_tag(self) -> &'static str { + match self { + Self::InviteSelf => "group:invite_self", + Self::NoSuchPlayer => "group:no_such_player", + Self::TargetOffline => "group:target_offline", + Self::AlreadyInGroup => "group:already_in_group", + Self::GroupFull => "group:group_full", + Self::NotLeader => "group:not_leader", + Self::NotInGroup => "group:not_in_group", + Self::TargetNotInGroup => "group:target_not_in_group", + Self::NoPendingInvite => "group:no_pending_invite", + Self::InviterUnavailable => "group:inviter_unavailable", + Self::KickSelf => "group:kick_self", + Self::InvalidLootRules => "group:invalid_loot_rules", + Self::IntentAlreadyClaimed => "group:intent_already_claimed", + Self::Database => "group:database", + } + } + + pub fn parse_tag(tag: &str) -> Option { + Self::ALL + .into_iter() + .find(|refusal| refusal.as_tag() == tag) + } } /// Encode the LIST payload: `leader,loot_method,loot_threshold,master_looter_guid|guid,name,online;...`. @@ -176,6 +244,18 @@ pub fn decode_roster(payload: &str) -> Option<(u64, u8, u8, u64, Vec<(u64, Strin mod tests { use super::*; + #[test] + fn every_group_refusal_tag_round_trips() { + for refusal in GroupRefusal::ALL { + assert_eq!(GroupRefusal::parse_tag(refusal.as_tag()), Some(refusal)); + } + assert_eq!(GroupRefusal::parse_tag("group:"), None); + assert_eq!( + GroupRefusal::parse_tag("gw_group_invite reducer timed out after 10s"), + None + ); + } + #[test] fn roster_round_trips_including_the_delimiter_defense() { let members = vec![ diff --git a/crates/lyracore-shared/src/lib.rs b/crates/lyracore-shared/src/lib.rs index d4f1d2da7..bd52fb87d 100644 --- a/crates/lyracore-shared/src/lib.rs +++ b/crates/lyracore-shared/src/lib.rs @@ -25,6 +25,7 @@ pub mod opcodes; pub mod packing; pub mod quest; pub mod region; +pub mod social; pub mod spatial; pub mod terrain; pub mod trade; diff --git a/crates/lyracore-shared/src/social.rs b/crates/lyracore-shared/src/social.rs new file mode 100644 index 000000000..90163806c --- /dev/null +++ b/crates/lyracore-shared/src/social.rs @@ -0,0 +1,61 @@ +//! The Module↔Gateway CONTACT wire contract: why the Module refused a friends- or ignore-list +//! change. The party half of the social pane is [`crate::group`]. + +/// Why the Module refused a contact-list Durable Request. The tag is the whole reducer error text, +/// so neither tier matches on human prose. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ContactRefusal { + /// A Character may not befriend or ignore itself. + AddSelf, + /// No Character row holds the guid the Gateway resolved. + NoSuchPlayer, + /// The target is already on that same list. + AlreadyOnList, + /// That list is at its cap. + ListFull, + /// The remove named a target that is not on that list. + NotOnList, +} + +impl ContactRefusal { + pub const ALL: [Self; 5] = [ + Self::AddSelf, + Self::NoSuchPlayer, + Self::AlreadyOnList, + Self::ListFull, + Self::NotOnList, + ]; + + pub fn as_tag(self) -> &'static str { + match self { + Self::AddSelf => "social:add_self", + Self::NoSuchPlayer => "social:no_such_player", + Self::AlreadyOnList => "social:already_on_list", + Self::ListFull => "social:list_full", + Self::NotOnList => "social:not_on_list", + } + } + + pub fn parse_tag(tag: &str) -> Option { + Self::ALL + .into_iter() + .find(|refusal| refusal.as_tag() == tag) + } +} + +#[cfg(test)] +mod tests { + use super::ContactRefusal; + + #[test] + fn every_contact_refusal_tag_round_trips() { + for refusal in ContactRefusal::ALL { + assert_eq!(ContactRefusal::parse_tag(refusal.as_tag()), Some(refusal)); + } + assert_eq!(ContactRefusal::parse_tag("social:"), None); + assert_eq!( + ContactRefusal::parse_tag("gw_add_friend reducer timed out after 10s"), + None + ); + } +} diff --git a/gateway/src/stdb/reducers.rs b/gateway/src/stdb/reducers.rs index 66bc5cb80..ba0f0bd3c 100644 --- a/gateway/src/stdb/reducers.rs +++ b/gateway/src/stdb/reducers.rs @@ -11,12 +11,15 @@ 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::party::PartyOutcome; use crate::world::{ - ItemActionResult, LootActionStatus, LootWindowRefusal, LootWindowRequestStatus, + ContactOutcome, ItemActionResult, LootActionStatus, LootWindowRefusal, LootWindowRequestStatus, }; use lyracore_shared::auction::AuctionRefusal; +use lyracore_shared::group::GroupRefusal; use lyracore_shared::item::ItemRefusal; use lyracore_shared::loot::{LootBoundaryFailure, LootRefusal}; +use lyracore_shared::social::ContactRefusal; use lyracore_shared::trainer::TrainerRefusal; static NEXT_TAXI_REQUEST_ID: OnceLock = OnceLock::new(); @@ -62,10 +65,7 @@ impl Coordinator { claim_bot_invite_intent_then(intent_id) ) { Ok(()) => Ok(true), - Err(error) - if reducer_refusal_reason(&error) - == Some(lyracore_shared::group::err::BOT_INVITE_INTENT_ALREADY_CLAIMED) => - { + Err(error) if group_refusal(&error) == Some(GroupRefusal::IntentAlreadyClaimed) => { Ok(false) } Err(error) => Err(error), @@ -1399,16 +1399,21 @@ impl Coordinator { /// `CMSG_MESSAGECHAT` Party (`/p`) — over the coordinator connection so the module /// attributes the line (and its group-membership check) to the caller. - pub fn party_chat(&self, _account_id: u64, actor_guid: u64, message: String) -> Result<()> { + pub fn party_chat( + &self, + _account_id: u64, + actor_guid: u64, + message: String, + ) -> Result { if actor_guid == 0 { return Err(anyhow!("party_chat: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + party_outcome(call_reducer!( coord.conn.reducers, "gw_party_chat", gw_party_chat_then(actor_guid, message) - ) + )) } /// GM playtest dot-command: resolve current Account authority on Realm-core, then convey it to @@ -1490,16 +1495,21 @@ impl Coordinator { } /// `CMSG_GROUP_INVITE` — `target_guid` is already resolved by the gateway. - pub fn group_invite(&self, _account_id: u64, actor_guid: u64, target_guid: u64) -> Result<()> { + pub fn group_invite( + &self, + _account_id: u64, + actor_guid: u64, + target_guid: u64, + ) -> Result { if actor_guid == 0 { return Err(anyhow!("group_invite: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + party_outcome(call_reducer!( coord.conn.reducers, "gw_group_invite", gw_group_invite_then(actor_guid, target_guid) - ) + )) } /// `CMSG_INITIATE_TRADE` — `target_guid` is the client's targeted player (#120). @@ -1674,42 +1684,42 @@ impl Coordinator { /// `CMSG_GROUP_ACCEPT`. Rides the coordinator connection as /// `gw_accept_group_invite`. - pub fn group_accept(&self, _account_id: u64, actor_guid: u64) -> Result<()> { + pub fn group_accept(&self, _account_id: u64, actor_guid: u64) -> Result { if actor_guid == 0 { return Err(anyhow!("group_accept: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + party_outcome(call_reducer!( coord.conn.reducers, "gw_accept_group_invite", gw_accept_group_invite_then(actor_guid) - ) + )) } /// `CMSG_GROUP_DECLINE`. - pub fn group_decline(&self, _account_id: u64, actor_guid: u64) -> Result<()> { + pub fn group_decline(&self, _account_id: u64, actor_guid: u64) -> Result { if actor_guid == 0 { return Err(anyhow!("group_decline: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + party_outcome(call_reducer!( coord.conn.reducers, "gw_group_decline", gw_group_decline_then(actor_guid) - ) + )) } /// `CMSG_GROUP_DISBAND` — leave the caller's group. - pub fn group_leave(&self, _account_id: u64, actor_guid: u64) -> Result<()> { + pub fn group_leave(&self, _account_id: u64, actor_guid: u64) -> Result { if actor_guid == 0 { return Err(anyhow!("group_leave: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + party_outcome(call_reducer!( coord.conn.reducers, "gw_group_leave", gw_group_leave_then(actor_guid) - ) + )) } /// `CMSG_GROUP_UNINVITE` — the leader kicks `target_guid`. @@ -1718,16 +1728,16 @@ impl Coordinator { _account_id: u64, actor_guid: u64, target_guid: u64, - ) -> Result<()> { + ) -> Result { if actor_guid == 0 { return Err(anyhow!("group_uninvite: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + party_outcome(call_reducer!( coord.conn.reducers, "gw_group_uninvite", gw_group_uninvite_then(actor_guid, target_guid) - ) + )) } /// `CMSG_LOOT_METHOD` — the leader sets the party's loot method/ @@ -1741,16 +1751,16 @@ impl Coordinator { loot_setting: u8, master_guid: u64, loot_threshold: u8, - ) -> Result<()> { + ) -> Result { if actor_guid == 0 { return Err(anyhow!("group_loot_method: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + party_outcome(call_reducer!( coord.conn.reducers, "gw_group_loot_method", gw_group_loot_method_then(actor_guid, loot_setting, master_guid, loot_threshold) - ) + )) } /// `CMSG_GOSSIP_SELECT_OPTION` — the NOTIFY-ONLY module chokepoint. Fired @@ -1775,55 +1785,75 @@ impl Coordinator { } /// `CMSG_ADD_FRIEND` — `target_guid` is already resolved by the gateway. - pub fn add_friend(&self, _account_id: u64, actor_guid: u64, target_guid: u64) -> Result<()> { + pub fn add_friend( + &self, + _account_id: u64, + actor_guid: u64, + target_guid: u64, + ) -> Result { if actor_guid == 0 { return Err(anyhow!("add_friend: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + contact_outcome(call_reducer!( coord.conn.reducers, "gw_add_friend", gw_add_friend_then(actor_guid, target_guid) - ) + )) } /// `CMSG_DEL_FRIEND`. - pub fn del_friend(&self, _account_id: u64, actor_guid: u64, target_guid: u64) -> Result<()> { + pub fn del_friend( + &self, + _account_id: u64, + actor_guid: u64, + target_guid: u64, + ) -> Result { if actor_guid == 0 { return Err(anyhow!("del_friend: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + contact_outcome(call_reducer!( coord.conn.reducers, "gw_del_friend", gw_del_friend_then(actor_guid, target_guid) - ) + )) } /// `CMSG_ADD_IGNORE` — `target_guid` is already resolved by the gateway. - pub fn add_ignore(&self, _account_id: u64, actor_guid: u64, target_guid: u64) -> Result<()> { + pub fn add_ignore( + &self, + _account_id: u64, + actor_guid: u64, + target_guid: u64, + ) -> Result { if actor_guid == 0 { return Err(anyhow!("add_ignore: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + contact_outcome(call_reducer!( coord.conn.reducers, "gw_add_ignore", gw_add_ignore_then(actor_guid, target_guid) - ) + )) } /// `CMSG_DEL_IGNORE`. - pub fn del_ignore(&self, _account_id: u64, actor_guid: u64, target_guid: u64) -> Result<()> { + pub fn del_ignore( + &self, + _account_id: u64, + actor_guid: u64, + target_guid: u64, + ) -> Result { if actor_guid == 0 { return Err(anyhow!("del_ignore: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + contact_outcome(call_reducer!( coord.conn.reducers, "gw_del_ignore", gw_del_ignore_then(actor_guid, target_guid) - ) + )) } /// Take the money from a corpse (`CMSG_LOOT_MONEY`, slice 3) over the coordinator connection so @@ -2571,12 +2601,12 @@ impl Coordinator { target_guid: u64, arg_a: u8, arg_b: u8, - ) -> Result<()> { - call_reducer!( + ) -> Result { + party_outcome(call_reducer!( self.0.call_pipe().conn.reducers, "realm_group_op", realm_group_op_then(op, actor_guid, target_guid, arg_a, arg_b) - ) + )) } /// `realm_whisper` — deliver one whisper against the database THIS handle points at. The @@ -3257,6 +3287,34 @@ fn resolved_item_actor(operation: &str, actor_guid: u64) -> Option { Some(actor_guid) } +/// The Module's typed party Refusal. Only a reducer the Module rejected carries a tag; a timeout, +/// transport, or SDK failure stays an error with an unknown outcome. +fn group_refusal(error: &anyhow::Error) -> Option { + reducer_refusal_reason(error).and_then(GroupRefusal::parse_tag) +} + +/// A refused party reducer is an outcome the client renders; anything else ends the session. +fn party_outcome(result: Result<()>) -> Result { + match result { + Ok(()) => Ok(PartyOutcome::Ran), + Err(error) => match group_refusal(&error) { + Some(refusal) => Ok(refusal.into()), + None => Err(error), + }, + } +} + +/// [`party_outcome`] for the friends and ignore lists. +fn contact_outcome(result: Result<()>) -> Result { + match result { + Ok(()) => Ok(ContactOutcome::Done), + Err(error) => match reducer_refusal_reason(&error).and_then(ContactRefusal::parse_tag) { + Some(refusal) => Ok(refusal.into()), + None => Err(error), + }, + } +} + fn bid_payload_matches(hold: &AuctionBidHold, decision: &AuctionBidDecision) -> bool { hold.operation_id == decision.operation_id && hold.bidder_guid == decision.bidder_guid diff --git a/gateway/src/stdb/subscriptions.rs b/gateway/src/stdb/subscriptions.rs index fff5bef51..60fe0e50b 100644 --- a/gateway/src/stdb/subscriptions.rs +++ b/gateway/src/stdb/subscriptions.rs @@ -3457,17 +3457,18 @@ impl Coordinator { let spawned = std::thread::Builder::new() .name("bot-invite-intent".into()) .spawn(move || { - if let Err(e) = crate::world::party::run_bot_invite_intent( + match crate::world::party::run_bot_invite_intent( &store, intent_id, op, inviter_guid, target_guid, ) { - log::debug!( + Ok(crate::world::party::PartyOutcome::Ran) => {} + outcome => log::debug!( "playerbots: group intent {intent_id} op {op} ({inviter_guid} -> \ - {target_guid}) did not execute: {e:#}" - ); + {target_guid}) did not execute: {outcome:?}" + ), } }); if let Err(error) = spawned { @@ -3551,13 +3552,13 @@ impl Coordinator { 0, 0, ) { - Ok(()) => log::info!( + Ok(crate::world::party::PartyOutcome::Ran) => log::info!( "party: character {} exists on no Shard and left its realm-core party", row.guid ), - Err(e) => log::debug!( + outcome => log::debug!( "party: character {} exists on no Shard; realm-core had no party to leave \ - ({e:#})", + ({outcome:?})", row.guid ), } diff --git a/gateway/src/stdb/world_store.rs b/gateway/src/stdb/world_store.rs index d37feb25e..b7ae0ec54 100644 --- a/gateway/src/stdb/world_store.rs +++ b/gateway/src/stdb/world_store.rs @@ -816,7 +816,12 @@ impl WorldStore for Coordinator { self.send_whisper(account_id, self_guid, target_player, message) } - fn party_chat(&self, account_id: u64, self_guid: u64, message: String) -> Result<()> { + fn party_chat( + &self, + account_id: u64, + self_guid: u64, + message: String, + ) -> Result { self.party_chat(account_id, self_guid, message) } @@ -876,7 +881,12 @@ impl WorldStore for Coordinator { // `self_guid` is unused here on purpose — the module resolves the actor from `ctx.sender()`'s // live entity, which is the whole reason this arm needs no operator gate. `world::party` is what // chooses between this and the realm-core arm below. - fn group_invite(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()> { + fn group_invite( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result { self.group_invite(account_id, self_guid, target_guid) } fn initiate_trade(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()> { @@ -915,16 +925,33 @@ impl WorldStore for Coordinator { fn ignore_trade(&self, account_id: u64, self_guid: u64) -> Result<()> { self.ignore_trade(account_id, self_guid) } - fn group_accept(&self, account_id: u64, self_guid: u64) -> Result<()> { + fn group_accept( + &self, + account_id: u64, + self_guid: u64, + ) -> Result { self.group_accept(account_id, self_guid) } - fn group_decline(&self, account_id: u64, self_guid: u64) -> Result<()> { + fn group_decline( + &self, + account_id: u64, + self_guid: u64, + ) -> Result { self.group_decline(account_id, self_guid) } - fn group_leave(&self, account_id: u64, self_guid: u64) -> Result<()> { + fn group_leave( + &self, + account_id: u64, + self_guid: u64, + ) -> Result { self.group_leave(account_id, self_guid) } - fn group_uninvite(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()> { + fn group_uninvite( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result { self.group_uninvite(account_id, self_guid, target_guid) } fn group_loot_method( @@ -934,7 +961,7 @@ impl WorldStore for Coordinator { loot_setting: u8, master_guid: u64, loot_threshold: u8, - ) -> Result<()> { + ) -> Result { self.group_loot_method( account_id, self_guid, @@ -991,7 +1018,7 @@ impl WorldStore for Coordinator { target_guid: u64, arg_a: u8, arg_b: u8, - ) -> Result<()> { + ) -> Result { self.realm_group_op(op, actor_guid, target_guid, arg_a, arg_b) } @@ -1103,19 +1130,39 @@ impl WorldStore for Coordinator { ) -> Result<()> { self.gossip_select(account_id, self_guid, npc_guid, option_id, option_row_id) } - fn add_friend(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()> { + fn add_friend( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result { self.add_friend(account_id, self_guid, target_guid) } - fn del_friend(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()> { + fn del_friend( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result { self.del_friend(account_id, self_guid, target_guid) } - fn add_ignore(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()> { + fn add_ignore( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result { self.add_ignore(account_id, self_guid, target_guid) } - fn del_ignore(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()> { + fn del_ignore( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result { self.del_ignore(account_id, self_guid, target_guid) } } diff --git a/gateway/src/world/handlers/query.rs b/gateway/src/world/handlers/query.rs index 9b4a7c46e..6717774d6 100644 --- a/gateway/src/world/handlers/query.rs +++ b/gateway/src/world/handlers/query.rs @@ -370,21 +370,23 @@ pub(crate) fn handle_query( // needle list). Any OTHER rejection (not in world / empty message) is silently // dropped, matching say/yell — the client never sends an empty line anyway. CMSG_MESSAGECHAT_ChatType::Party => { - if let Err(e) = store.party_chat(conn.account_id, self_guid, message) { - if e.to_string() - .contains(lyracore_shared::group::err::NOT_IN_GROUP) - { - send( - tx, - Outbound::One(ServerOpcodeMessage::SMSG_PARTY_COMMAND_RESULT( - Box::new(codec::build_party_command_result( - PartyOperation::Leave, - String::new(), - PartyResult::NotInGroup, - )), + // Speaking from no party is the one refusal vanilla answers. Every other + // refusal, and a failed call, drop like a rejected say or yell line. + match store.party_chat(conn.account_id, self_guid, message) { + Ok(crate::world::party::PartyOutcome::Refused( + lyracore_shared::group::GroupRefusal::NotInGroup, + )) => send( + tx, + Outbound::One(ServerOpcodeMessage::SMSG_PARTY_COMMAND_RESULT( + Box::new(codec::build_party_command_result( + PartyOperation::Leave, + String::new(), + PartyResult::NotInGroup, )), - )?; - } + )), + )?, + Ok(_) => {} + Err(e) => log::debug!("world: party chat dropped: {e:#}"), } } _ => {} // guild/channel/etc. need systems that don't exist yet diff --git a/gateway/src/world/mod.rs b/gateway/src/world/mod.rs index f158de852..e98975e06 100644 --- a/gateway/src/world/mod.rs +++ b/gateway/src/world/mod.rs @@ -67,6 +67,7 @@ pub(crate) use handlers::{ PlaceBidOutcome, PlaceBidRequest, TrainerBuyOutcome, WeatherStore, }; use login_queue::{Admission, LoginQueue}; +pub(crate) use social::ContactOutcome; use social::handle_social; pub use store::WorldStore; use transfer::{EscrowedTransfer, TransferPlan}; diff --git a/gateway/src/world/party.rs b/gateway/src/world/party.rs index 3880f6973..d8e30dd31 100644 --- a/gateway/src/world/party.rs +++ b/gateway/src/world/party.rs @@ -42,7 +42,7 @@ use anyhow::Result; use super::{send, Outbound, SessionTx, WorldStore}; use crate::codec; -use lyracore_shared::group::{bot_op, realm_op}; +use lyracore_shared::group::{bot_op, realm_op, GroupRefusal}; use wow_world_messages::vanilla::opcodes::ServerOpcodeMessage; /// One party, as the database that holds it sees it. Read from realm-core it is the authority; read @@ -95,6 +95,21 @@ pub enum Op { }, } +/// What one party op answered. A [`GroupRefusal`] is a gameplay answer the client renders, so it +/// arrives as `Ok`; a timeout, transport failure, or untagged reducer error stays `Err` and ends the +/// session, because the durable outcome is then unknown. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum PartyOutcome { + Ran, + Refused(GroupRefusal), +} + +impl From for PartyOutcome { + fn from(refusal: GroupRefusal) -> Self { + Self::Refused(refusal) + } +} + /// Resolve a typed player name to a guid ACROSS every connected shard. /// /// **This is the read that made a cross-shard invite impossible.** `/invite Bob` resolves the typed @@ -279,8 +294,9 @@ pub(crate) fn session_less_in_world(store: &St, guid: u /// The bot acts as ITSELF: `realm_group_op`'s `actor` slot carries the bot's own guid for both the /// accept and the decline, never the inviter's — the impersonation hazard this slice already hit once. /// -/// A refusal DECLINES rather than walking away. Every accept gate in the module (`already in a group`, -/// `group full`, `inviter no longer leads`) returns `Err`, which rolls its transaction back and leaves +/// A refusal DECLINES rather than walking away. Every accept gate in the module +/// ([`GroupRefusal::AlreadyInGroup`], [`GroupRefusal::GroupFull`], +/// [`GroupRefusal::InviterUnavailable`]) rolls its transaction back and leaves /// the invite row standing — so ignoring the refusal would leave the dialog hanging, which is /// indistinguishable from the bug this fixes. The decline consumes the invite and pushes /// `SMSG_GROUP_DECLINE` at the inviter. @@ -295,17 +311,21 @@ fn answer_for_session_less(store: &St, realm: &dyn Worl if !session_less_in_world(store, guid) { return; // a real player's own client answers its own dialog } - match realm.realm_group_op(realm_op::ACCEPT, guid, 0, 0, 0) { - Ok(()) => log::info!("party: session-less {guid} accepted its group invite"), - Err(e) => { - log::info!("party: session-less {guid} cannot join ({e:#}) — declining explicitly"); - if let Err(e) = realm.realm_group_op(realm_op::DECLINE, guid, 0, 0, 0) { - log::warn!( - "party: session-less {guid} could neither join nor decline ({e:#}) — the \ - inviter's dialog stands until realm-core's invite GC reaps it" - ); - } + let joined = match realm.realm_group_op(realm_op::ACCEPT, guid, 0, 0, 0) { + Ok(PartyOutcome::Ran) => { + log::info!("party: session-less {guid} accepted its group invite"); + return; } + Ok(PartyOutcome::Refused(refusal)) => format!("{refusal:?}"), + Err(e) => format!("{e:#}"), + }; + log::info!("party: session-less {guid} cannot join ({joined}) — declining explicitly"); + match realm.realm_group_op(realm_op::DECLINE, guid, 0, 0, 0) { + Ok(PartyOutcome::Ran) => {} + outcome => log::warn!( + "party: session-less {guid} could neither join nor decline ({outcome:?}) — the \ + inviter's dialog stands until realm-core's invite GC reaps it" + ), } } @@ -321,7 +341,7 @@ pub(crate) fn run( account_id: u64, self_guid: u64, op: Op, -) -> Result<()> { +) -> Result { let Some(realm) = store.realm_store() else { return match op { Op::Invite(target) => store.group_invite(account_id, self_guid, target), @@ -339,18 +359,14 @@ pub(crate) fn run( // The two gates realm-core cannot run for itself, because the directory database holds neither // characters nor live entities: does the target EXIST, and is it ONLINE. The gateway is the only // party that can answer them across a boundary — which is precisely the bug being fixed — and it - // answers with the module's own error strings so the gateway's `PartyResult` classifier maps - // them exactly as it maps the shard plane's. + // answers with the module's own Refusals, so the client-facing code is the same on both planes. // // Each gate is the module's own read, unioned across the shards — EXISTS is a `game_character` // row ([`presence`]), ONLINE is a `game_world_entity` row ([`live_anywhere`]). Reading the // session flag for the second would silently refuse every playerbot; see [`live_anywhere`]. if let Op::Invite(target) = op { - if presence(store, target)?.is_none() { - anyhow::bail!("no such player"); - } - if !live_anywhere(store, target) { - anyhow::bail!("player not online"); + if let Some(refusal) = invite_gate(store, target)? { + return Ok(refusal.into()); } } // The group this character was in BEFORE the op — the only way to reach the party they may have @@ -381,7 +397,11 @@ pub(crate) fn run( threshold, } => (realm_op::LOOT_METHOD, master, setting, threshold), }; - realm.realm_group_op(code, self_guid, target, arg_a, arg_b)?; + if let PartyOutcome::Refused(refusal) = + realm.realm_group_op(code, self_guid, target, arg_a, arg_b)? + { + return Ok(PartyOutcome::Refused(refusal)); + } // Nobody is at the keyboard of a playerbot, so nobody answers its dialog. Done // BEFORE the mirror push, so the ONE push that follows already carries the bot as a member — // which is what the shard's own party reads (kill-XP split, `/p`, follow-the-leader) need. @@ -389,7 +409,19 @@ pub(crate) fn run( answer_for_session_less(store, realm.as_ref(), target); } sync_mirrors(store, realm.as_ref(), self_guid, before); - Ok(()) + Ok(PartyOutcome::Ran) +} + +/// The invite gates realm-core cannot run for itself: does the target exist anywhere, and is it in +/// the world anywhere. `None` means the invite may proceed. +fn invite_gate(store: &St, target: u64) -> Result> { + if presence(store, target)?.is_none() { + return Ok(Some(GroupRefusal::NoSuchPlayer)); + } + if !live_anywhere(store, target) { + return Ok(Some(GroupRefusal::TargetOffline)); + } + Ok(None) } /// Claim one subscribed intent on its World Shard, then execute it only for the winning Gateway. @@ -403,9 +435,9 @@ pub(crate) fn run_bot_invite_intent( op: u8, inviter_guid: u64, target_guid: u64, -) -> Result<()> { +) -> Result { if !store.claim_bot_invite_intent(intent_id)? { - return Ok(()); + return Ok(PartyOutcome::Ran); } match op { bot_op::INVITE => run_bot_invite(store, inviter_guid, target_guid), @@ -435,7 +467,7 @@ pub(crate) fn run_bot_invite( store: &St, inviter_guid: u64, target_guid: u64, -) -> Result<()> { +) -> Result { let owned_realm; let realm: &dyn WorldStore = match store.realm_store() { Some(r) => { @@ -444,17 +476,18 @@ pub(crate) fn run_bot_invite( } None => store, }; - if presence(store, target_guid)?.is_none() { - anyhow::bail!("no such player"); - } - if !live_anywhere(store, target_guid) { - anyhow::bail!("player not online"); + if let Some(refusal) = invite_gate(store, target_guid)? { + return Ok(refusal.into()); } let before = realm.group_roster(inviter_guid)?.map(|r| r.group_id); - realm.realm_group_op(realm_op::INVITE, inviter_guid, target_guid, 0, 0)?; + if let PartyOutcome::Refused(refusal) = + realm.realm_group_op(realm_op::INVITE, inviter_guid, target_guid, 0, 0)? + { + return Ok(PartyOutcome::Refused(refusal)); + } answer_for_session_less(store, realm, target_guid); sync_mirrors(store, realm, inviter_guid, before); - Ok(()) + Ok(PartyOutcome::Ran) } /// Run a SERVER-DRIVEN leave with no client behind it — a bot leader whose party has run out of @@ -471,7 +504,7 @@ pub(crate) fn run_bot_invite( /// The pending-roll flush is [`run`]'s, verbatim, and belongs here for the reason it gives: a LEAVE /// can shrink a group below two members and reach realm-core's disband branch, which force-resolves /// live loot rolls that the periodic relay may not have promoted yet. -pub(crate) fn run_bot_leave(store: &St, leaver_guid: u64) -> Result<()> { +pub(crate) fn run_bot_leave(store: &St, leaver_guid: u64) -> Result { let owned_realm; let realm: &dyn WorldStore = match store.realm_store() { Some(r) => { @@ -482,9 +515,13 @@ pub(crate) fn run_bot_leave(store: &St, leaver_guid: u64) -> Res }; let before = realm.group_roster(leaver_guid)?.map(|r| r.group_id); crate::world::loot::flush_pending_promotions(store, realm); - realm.realm_group_op(realm_op::LEAVE, leaver_guid, 0, 0, 0)?; + if let PartyOutcome::Refused(refusal) = + realm.realm_group_op(realm_op::LEAVE, leaver_guid, 0, 0, 0)? + { + return Ok(PartyOutcome::Refused(refusal)); + } sync_mirrors(store, realm, leaver_guid, before); - Ok(()) + Ok(PartyOutcome::Ran) } /// Push realm-core's roster for every party this op could have touched onto every connected world diff --git a/gateway/src/world/party_tests.rs b/gateway/src/world/party_tests.rs index 4ba4e95b6..163890333 100644 --- a/gateway/src/world/party_tests.rs +++ b/gateway/src/world/party_tests.rs @@ -9,6 +9,7 @@ //! A child module of `world::tests` so it can reach `InMemoryStore` without widening anything. use super::*; +use lyracore_shared::group::GroupRefusal; pub(super) const GINGER: u64 = 1; // in the open world, on `world` pub(super) const VIM: u64 = 2; // inside the dungeon, on `instances` @@ -471,19 +472,20 @@ fn world_entry_clears_a_mirror_that_still_lists_a_character_the_authority_droppe } /// The two gates realm-core cannot run for itself. Both are refused BEFORE the authority is touched, -/// and both carry the module's own error strings so `social::party_result_for` classifies them +/// and both answer the module's own Refusals so `social::party_result_for` classifies them /// identically on either plane. #[test] fn an_invite_to_a_missing_or_offline_target_never_reaches_realm_core() { let (realm, world, _instances, _calls) = party_topology(); - let err = party::run(world.as_ref(), 7, GINGER, party::Op::Invite(DORMANT)) - .expect_err("an offline target is refused"); - assert!(err.to_string().contains("player not online"), "got {err}"); - - let err = party::run(world.as_ref(), 7, GINGER, party::Op::Invite(999)) - .expect_err("an unknown target is refused"); - assert!(err.to_string().contains("no such player"), "got {err}"); + assert_eq!( + party::run(world.as_ref(), 7, GINGER, party::Op::Invite(DORMANT)).unwrap(), + PartyOutcome::Refused(GroupRefusal::TargetOffline) + ); + assert_eq!( + party::run(world.as_ref(), 7, GINGER, party::Op::Invite(999)).unwrap(), + PartyOutcome::Refused(GroupRefusal::NoSuchPlayer) + ); assert!( realm.party.lock().unwrap().ops.is_empty(), @@ -1092,7 +1094,8 @@ fn a_bot_invite_forms_a_party_on_realm_core_across_a_shard_boundary() { "the op must reach realm-core; calls were {log:?}" ); assert!( - !log.iter().any(|(_, call)| call == "group_invite" || call == "group_accept"), + !log.iter() + .any(|(_, call)| call == "group_invite" || call == "group_accept"), "a bot invite must not write either shard's own party tables directly — that is the \ serendipity-invite shard-local-write bug. \ Calls were {log:?}" @@ -1321,12 +1324,14 @@ fn a_shard_local_only_group_realm_core_never_heard_of_is_wiped_by_the_next_push( fn a_bot_invite_to_a_missing_or_offline_target_never_reaches_realm_core() { let (realm, world, _instances, _calls) = party_topology(); - let err = - party::run_bot_invite(world.as_ref(), BOT, DORMANT).expect_err("offline target refused"); - assert!(err.to_string().contains("player not online"), "got {err}"); - - let err = party::run_bot_invite(world.as_ref(), BOT, 999).expect_err("unknown target refused"); - assert!(err.to_string().contains("no such player"), "got {err}"); + assert_eq!( + party::run_bot_invite(world.as_ref(), BOT, DORMANT).unwrap(), + PartyOutcome::Refused(GroupRefusal::TargetOffline) + ); + assert_eq!( + party::run_bot_invite(world.as_ref(), BOT, 999).unwrap(), + PartyOutcome::Refused(GroupRefusal::NoSuchPlayer) + ); assert!( realm.party.lock().unwrap().ops.is_empty(), diff --git a/gateway/src/world/social.rs b/gateway/src/world/social.rs index 1c305f763..be5ecc8c3 100644 --- a/gateway/src/world/social.rs +++ b/gateway/src/world/social.rs @@ -2,13 +2,31 @@ //! carved out of `handle_query` in `world/mod.rs` — pure code-motion, same per-family `handle_*` //! shape as the rest of the dispatch chain. +use super::party::PartyOutcome; use super::{party, send, Outbound, SessionTx, WorldConn, WorldState, WorldStore}; use crate::codec; use anyhow::Result; +use lyracore_shared::group::GroupRefusal; +use lyracore_shared::social::ContactRefusal; use wow_world_base::shared::friend_result_vanilla_tbc::FriendResult; use wow_world_messages::vanilla::opcodes::{ClientOpcodeMessage, ServerOpcodeMessage}; use wow_world_messages::vanilla::{PartyOperation, PartyResult}; +/// What one contact-list op answered. A [`ContactRefusal`] is a gameplay answer `SMSG_FRIEND_STATUS` +/// renders; a timeout, transport failure, or untagged reducer error stays `Err` and ends the +/// session, because the durable outcome is then unknown. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum ContactOutcome { + Done, + Refused(ContactRefusal), +} + +impl From for ContactOutcome { + fn from(refusal: ContactRefusal) -> Self { + Self::Refused(refusal) + } +} + /// Social family: /who, the friends/ignore lists, and party/group management — the social-pane /// opcodes. Each arm consumes its opcode (`Ok(None)`) or passes the message on (`Ok(Some(msg))`), /// like the other per-family handlers. @@ -55,7 +73,13 @@ pub(super) fn handle_social( // insensitive), then the module re-validates self/duplicate/cap server-side. Either way the // client gets an SMSG_FRIEND_STATUS its system message reads the result code off. ClientOpcodeMessage::CMSG_ADD_FRIEND(c) => { - let (result, guid) = resolve_add_contact(store, conn.account_id, self_guid(conn).unwrap_or(0), &c.name, false)?; + let (result, guid) = resolve_add_contact( + store, + conn.account_id, + self_guid(conn).unwrap_or(0), + &c.name, + false, + )?; send( tx, Outbound::One(ServerOpcodeMessage::SMSG_FRIEND_STATUS(Box::new( @@ -64,7 +88,13 @@ pub(super) fn handle_social( )?; } ClientOpcodeMessage::CMSG_ADD_IGNORE(c) => { - let (result, guid) = resolve_add_contact(store, conn.account_id, self_guid(conn).unwrap_or(0), &c.name, true)?; + let (result, guid) = resolve_add_contact( + store, + conn.account_id, + self_guid(conn).unwrap_or(0), + &c.name, + true, + )?; send( tx, Outbound::One(ServerOpcodeMessage::SMSG_FRIEND_STATUS(Box::new( @@ -74,7 +104,13 @@ pub(super) fn handle_social( } // Remove a friend/ignore by guid (the client already has it from the list row). ClientOpcodeMessage::CMSG_DEL_FRIEND(c) => { - let (result, guid) = resolve_del_contact(store, conn.account_id, self_guid(conn).unwrap_or(0), c.guid.guid(), false)?; + let (result, guid) = resolve_del_contact( + store, + conn.account_id, + self_guid(conn).unwrap_or(0), + c.guid.guid(), + false, + )?; send( tx, Outbound::One(ServerOpcodeMessage::SMSG_FRIEND_STATUS(Box::new( @@ -83,7 +119,13 @@ pub(super) fn handle_social( )?; } ClientOpcodeMessage::CMSG_DEL_IGNORE(c) => { - let (result, guid) = resolve_del_contact(store, conn.account_id, self_guid(conn).unwrap_or(0), c.guid.guid(), true)?; + let (result, guid) = resolve_del_contact( + store, + conn.account_id, + self_guid(conn).unwrap_or(0), + c.guid.guid(), + true, + )?; send( tx, Outbound::One(ServerOpcodeMessage::SMSG_FRIEND_STATUS(Box::new( @@ -102,12 +144,12 @@ pub(super) fn handle_social( // act as, and none of these opcodes is reachable at character select. ClientOpcodeMessage::CMSG_GROUP_INVITE(c) => { let result = match (self_guid(conn), party::resolve_by_name(store, &c.name)?) { - (Some(me), Some(guid)) => { - match party::run(store, conn.account_id, me, party::Op::Invite(guid)) { - Ok(()) => PartyResult::Success, - Err(e) => party_result_for(&e.to_string()), - } - } + (Some(me), Some(guid)) => party_result(party::run( + store, + conn.account_id, + me, + party::Op::Invite(guid), + )?), _ => PartyResult::BadPlayerName, }; send( @@ -119,9 +161,11 @@ pub(super) fn handle_social( } ClientOpcodeMessage::CMSG_GROUP_ACCEPT => { if let Some(me) = self_guid(conn) { - if let Err(e) = party::run(store, conn.account_id, me, party::Op::Accept) { + if let PartyOutcome::Refused(refusal) = + party::run(store, conn.account_id, me, party::Op::Accept)? + { log::debug!( - "world: group_accept ignored (account {}): {e}", + "world: group_accept refused (account {}): {refusal:?}", conn.account_id ); } @@ -129,9 +173,11 @@ pub(super) fn handle_social( } ClientOpcodeMessage::CMSG_GROUP_DECLINE => { if let Some(me) = self_guid(conn) { - if let Err(e) = party::run(store, conn.account_id, me, party::Op::Decline) { + if let PartyOutcome::Refused(refusal) = + party::run(store, conn.account_id, me, party::Op::Decline)? + { log::debug!( - "world: group_decline ignored (account {}): {e}", + "world: group_decline refused (account {}): {refusal:?}", conn.account_id ); } @@ -139,14 +185,16 @@ pub(super) fn handle_social( } ClientOpcodeMessage::CMSG_GROUP_DISBAND => { if let Some(me) = self_guid(conn) { - if let Err(e) = party::run(store, conn.account_id, me, party::Op::Leave) { + if let PartyOutcome::Refused(refusal) = + party::run(store, conn.account_id, me, party::Op::Leave)? + { send( tx, Outbound::One(ServerOpcodeMessage::SMSG_PARTY_COMMAND_RESULT(Box::new( codec::build_party_command_result( PartyOperation::Leave, String::new(), - party_result_for(&e.to_string()), + party_result_for(refusal), ), ))), )?; @@ -155,12 +203,12 @@ pub(super) fn handle_social( } ClientOpcodeMessage::CMSG_GROUP_UNINVITE(c) => { let result = match (self_guid(conn), party::resolve_by_name(store, &c.name)?) { - (Some(me), Some(guid)) => { - match party::run(store, conn.account_id, me, party::Op::Uninvite(guid)) { - Ok(()) => PartyResult::Success, - Err(e) => party_result_for(&e.to_string()), - } - } + (Some(me), Some(guid)) => party_result(party::run( + store, + conn.account_id, + me, + party::Op::Uninvite(guid), + )?), _ => PartyResult::BadPlayerName, }; if result != PartyResult::Success { @@ -187,9 +235,10 @@ pub(super) fn handle_social( master, threshold, }; - if let Err(e) = party::run(store, conn.account_id, me, op) { + if let PartyOutcome::Refused(refusal) = party::run(store, conn.account_id, me, op)? + { log::debug!( - "world: group_loot_method ignored (account {}): {e}", + "world: group_loot_method refused (account {}): {refusal:?}", conn.account_id ); } @@ -217,33 +266,60 @@ pub(super) fn self_guid(conn: &WorldConn) -> Option { } } -/// Map the group reducers' Err strings onto the vanilla `PartyResult` codes the client renders -/// ("X is already in a group" etc.). Unknown reducer errors degrade to BadPlayerName — a visible, -/// non-crashing line — rather than being swallowed. -fn party_result_for(e: &str) -> PartyResult { - // `contains`, not `==`: the reducer's Err string arrives wrapped in the SDK's error context. - // The needles themselves are the lyracore-shared group contract — the module returns these exact - // strings, so a reword is a one-place edit both sides see at compile time. - use lyracore_shared::group::err as group_err; - if e.contains(group_err::ALREADY_IN_GROUP) { - PartyResult::AlreadyInGroup - } else if e.contains(group_err::GROUP_FULL) { - PartyResult::GroupFull - } else if e.contains(group_err::NOT_LEADER) { - PartyResult::NotLeader - } else if e.contains(group_err::NOT_IN_GROUP) { - PartyResult::NotInGroup - } else if e.contains(group_err::TARGET_NOT_IN_GROUP) { - PartyResult::TargetNotInGroup - } else { - PartyResult::BadPlayerName +/// The `PartyResult` code one party outcome renders as. +fn party_result(outcome: PartyOutcome) -> PartyResult { + match outcome { + PartyOutcome::Ran => PartyResult::Success, + PartyOutcome::Refused(refusal) => party_result_for(refusal), + } +} + +/// Map each [`GroupRefusal`] onto the vanilla `PartyResult` the client renders ("X is already in a +/// group" etc.). Vanilla has no code for an offline or self-named target, a stale invite, or a +/// broken durable row, so those read as BadPlayerName — a visible, non-crashing line. The intent +/// claim never reaches a client; it is listed so a new Refusal cannot be forgotten here. +fn party_result_for(refusal: GroupRefusal) -> PartyResult { + match refusal { + GroupRefusal::AlreadyInGroup => PartyResult::AlreadyInGroup, + GroupRefusal::GroupFull => PartyResult::GroupFull, + GroupRefusal::NotLeader => PartyResult::NotLeader, + GroupRefusal::NotInGroup => PartyResult::NotInGroup, + GroupRefusal::TargetNotInGroup => PartyResult::TargetNotInGroup, + GroupRefusal::InviteSelf + | GroupRefusal::NoSuchPlayer + | GroupRefusal::TargetOffline + | GroupRefusal::NoPendingInvite + | GroupRefusal::InviterUnavailable + | GroupRefusal::KickSelf + | GroupRefusal::InvalidLootRules + | GroupRefusal::IntentAlreadyClaimed + | GroupRefusal::Database => PartyResult::BadPlayerName, + } +} + +/// The `FriendResult` code one contact Refusal renders as. The friends and ignore lists have +/// separate code families for the same three conditions, so the list decides the answer. +fn friend_result_for(refusal: ContactRefusal, is_ignore: bool) -> FriendResult { + match (refusal, is_ignore) { + (ContactRefusal::AddSelf, false) => FriendResult::SelfX, + (ContactRefusal::AddSelf, true) => FriendResult::IgnoreSelf, + (ContactRefusal::AlreadyOnList, false) => FriendResult::Already, + (ContactRefusal::AlreadyOnList, true) => FriendResult::IgnoreAlready, + (ContactRefusal::ListFull, false) => FriendResult::ListFull, + (ContactRefusal::ListFull, true) => FriendResult::IgnoreFull, + (ContactRefusal::NotOnList, true) => FriendResult::IgnoreNotFound, + // A guid the gateway resolved that the module cannot see, and the friend-list remove of a + // row that is not there, both read as the one "no such entry" line vanilla has. + (ContactRefusal::NotOnList, false) | (ContactRefusal::NoSuchPlayer, _) => { + FriendResult::NotFound + } } } /// Resolve a typed contact name, call the module's add reducer (`add_friend`/`add_ignore`), and /// translate the outcome into the `(FriendResult, guid)` pair `SMSG_FRIEND_STATUS` needs. An unknown -/// name never reaches the module (guid 0, `NotFound`) — everything else (self/duplicate/cap) is the -/// module's own rejection, string-matched here the same way `handle_trainer` parses buy failures. +/// name never reaches the module (guid 0, `NotFound`); everything else (self/duplicate/cap) is the +/// module's own typed Refusal. fn resolve_add_contact( store: &St, account_id: u64, @@ -255,13 +331,13 @@ fn resolve_add_contact( return Ok((FriendResult::NotFound, 0)); }; let outcome = if is_ignore { - store.add_ignore(account_id, actor_guid, target_guid) + store.add_ignore(account_id, actor_guid, target_guid)? } else { - store.add_friend(account_id, actor_guid, target_guid) + store.add_friend(account_id, actor_guid, target_guid)? }; let result = match outcome { - Ok(()) if is_ignore => FriendResult::IgnoreAdded, - Ok(()) => { + ContactOutcome::Done if is_ignore => FriendResult::IgnoreAdded, + ContactOutcome::Done => { let online = store .character_presence(target_guid)? .map(|(online, ..)| online) @@ -272,26 +348,7 @@ fn resolve_add_contact( FriendResult::AddedOffline } } - Err(e) => { - let es = e.to_string(); - if es.contains("cannot add yourself") { - FriendResult::SelfX - } else if es.contains("already added") { - if is_ignore { - FriendResult::IgnoreAlready - } else { - FriendResult::Already - } - } else if es.contains("list full") { - if is_ignore { - FriendResult::IgnoreFull - } else { - FriendResult::ListFull - } - } else { - FriendResult::NotFound - } - } + ContactOutcome::Refused(refusal) => friend_result_for(refusal, is_ignore), }; Ok((result, target_guid)) } @@ -306,15 +363,14 @@ fn resolve_del_contact( is_ignore: bool, ) -> Result<(FriendResult, u64)> { let outcome = if is_ignore { - store.del_ignore(account_id, actor_guid, target_guid) + store.del_ignore(account_id, actor_guid, target_guid)? } else { - store.del_friend(account_id, actor_guid, target_guid) + store.del_friend(account_id, actor_guid, target_guid)? }; let result = match outcome { - Ok(()) if is_ignore => FriendResult::IgnoreRemoved, - Ok(()) => FriendResult::Removed, - Err(_) if is_ignore => FriendResult::IgnoreNotFound, - Err(_) => FriendResult::NotFound, + ContactOutcome::Done if is_ignore => FriendResult::IgnoreRemoved, + ContactOutcome::Done => FriendResult::Removed, + ContactOutcome::Refused(refusal) => friend_result_for(refusal, is_ignore), }; Ok((result, target_guid)) } @@ -323,41 +379,40 @@ fn resolve_del_contact( mod tests { use super::*; + /// The five conditions vanilla has a code for keep their own code; the rest share the one + /// visible fallback line rather than being swallowed. + #[test] + fn each_group_refusal_renders_its_own_party_result() { + for refusal in GroupRefusal::ALL { + let expected = match refusal { + GroupRefusal::AlreadyInGroup => PartyResult::AlreadyInGroup, + GroupRefusal::GroupFull => PartyResult::GroupFull, + GroupRefusal::NotLeader => PartyResult::NotLeader, + GroupRefusal::NotInGroup => PartyResult::NotInGroup, + GroupRefusal::TargetNotInGroup => PartyResult::TargetNotInGroup, + _ => PartyResult::BadPlayerName, + }; + assert_eq!(party_result_for(refusal), expected, "{refusal:?}"); + } + assert_eq!(party_result(PartyOutcome::Ran), PartyResult::Success); + } + + /// The friends and ignore lists carry separate code families for the same conditions, so the + /// same Refusal must not answer both lists with one code. #[test] - fn party_result_for_maps_each_group_error_needle_and_falls_back() { - // The needles are the lyracore-shared group contract — the module's Err strings - // match these exact substrings, one PartyResult per reducer rejection reason. - use lyracore_shared::group::err as group_err; - assert_eq!( - party_result_for(group_err::ALREADY_IN_GROUP), - PartyResult::AlreadyInGroup - ); - assert_eq!( - party_result_for(group_err::GROUP_FULL), - PartyResult::GroupFull - ); - assert_eq!( - party_result_for(group_err::NOT_LEADER), - PartyResult::NotLeader - ); - assert_eq!( - party_result_for(group_err::NOT_IN_GROUP), - PartyResult::NotInGroup - ); - assert_eq!( - party_result_for(group_err::TARGET_NOT_IN_GROUP), - PartyResult::TargetNotInGroup - ); - // `contains`, not `==`: the SDK wraps the reducer's raw Err string in error context, so the - // needle must still hit through a wrapping prefix. - assert_eq!( - party_result_for(&format!("reducer call failed: {}", group_err::GROUP_FULL)), - PartyResult::GroupFull - ); - // An unrecognized error degrades to BadPlayerName (a visible line) rather than being swallowed. - assert_eq!( - party_result_for("some other error"), - PartyResult::BadPlayerName - ); + fn each_contact_refusal_renders_the_code_of_the_list_it_names() { + for refusal in ContactRefusal::ALL { + let (friend, ignore) = match refusal { + ContactRefusal::AddSelf => (FriendResult::SelfX, FriendResult::IgnoreSelf), + ContactRefusal::AlreadyOnList => { + (FriendResult::Already, FriendResult::IgnoreAlready) + } + ContactRefusal::ListFull => (FriendResult::ListFull, FriendResult::IgnoreFull), + ContactRefusal::NotOnList => (FriendResult::NotFound, FriendResult::IgnoreNotFound), + ContactRefusal::NoSuchPlayer => (FriendResult::NotFound, FriendResult::NotFound), + }; + assert_eq!(friend_result_for(refusal, false), friend, "{refusal:?}"); + assert_eq!(friend_result_for(refusal, true), ignore, "{refusal:?}"); + } } } diff --git a/gateway/src/world/store.rs b/gateway/src/world/store.rs index aa63698ad..d844af395 100644 --- a/gateway/src/world/store.rs +++ b/gateway/src/world/store.rs @@ -201,7 +201,7 @@ pub trait WorldStore: _target_guid: u64, _arg_a: u8, _arg_b: u8, - ) -> Result<()> { + ) -> Result { Err(anyhow!("this store does not host realm-wide party state")) } @@ -662,12 +662,16 @@ pub trait WorldStore: /// Party chat (`CMSG_MESSAGECHAT` Party, `/p`): deliver `message` to every OTHER /// current group member plus an echo to the caller, over the `game_group_event` relay (no - /// gateway-subscribed table — see `module/src/chat.rs::party_chat`'s doc). `Err` when the caller - /// isn't in a group ([`lyracore_shared::group::err::NOT_IN_GROUP`] — the gateway maps it to - /// `SMSG_PARTY_COMMAND_RESULT(NotInGroup)`, "You aren't in a party") or on the other - /// `send_chat`-style rejections (not in world / empty message), which are silently dropped like - /// say/yell. - fn party_chat(&self, account_id: u64, self_guid: u64, message: String) -> Result<()>; + /// gateway-subscribed table — see `module/src/chat.rs::party_chat`'s doc). Speaking from no + /// party answers [`lyracore_shared::group::GroupRefusal::NotInGroup`], which the gateway renders + /// as `SMSG_PARTY_COMMAND_RESULT(NotInGroup)`, "You aren't in a party". Every other refusal and + /// every failure is dropped like a rejected say or yell line. + fn party_chat( + &self, + account_id: u64, + self_guid: u64, + message: String, + ) -> Result; /// GM playtest dot-command for the proof-validated, realm-wide `account_name`: `text` is the /// raw Say line, STILL carrying its @@ -965,14 +969,37 @@ pub trait WorldStore: /// Added-Online-vs-Offline split. `None` if the guid doesn't resolve to any character. fn character_presence(&self, guid: u64) -> Result>; + // Contact ops answer a [`social::ContactOutcome`]: a typed Refusal is a gameplay answer the + // client renders, and only a failure with an unknown durable result stays `Err`. + /// `CMSG_ADD_FRIEND` (the name is already resolved to `target_guid` by the gateway). - fn add_friend(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()>; + fn add_friend( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result; /// `CMSG_DEL_FRIEND`. - fn del_friend(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()>; + fn del_friend( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result; /// `CMSG_ADD_IGNORE` (the name is already resolved to `target_guid` by the gateway). - fn add_ignore(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()>; + fn add_ignore( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result; /// `CMSG_DEL_IGNORE`. - fn del_ignore(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()>; + fn del_ignore( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result; // The SINGLE-DATABASE party path (`world::party::run`'s `None` arm). Each takes the caller's // `self_guid` as well as its account: the account is what identifies the player CONNECTION these @@ -980,16 +1007,26 @@ pub trait WorldStore: // Both are threaded through one call site (`world::social`), so the two planes take the same // arguments and a mock sees which character the op was for either way. - /// `CMSG_GROUP_INVITE` (name gateway-resolved). Module Err strings map to PartyResult codes. - fn group_invite(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()>; + /// `CMSG_GROUP_INVITE` (name gateway-resolved). A `GroupRefusal` arrives as an outcome. + fn group_invite( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result; /// `CMSG_GROUP_ACCEPT`. - fn group_accept(&self, account_id: u64, self_guid: u64) -> Result<()>; + fn group_accept(&self, account_id: u64, self_guid: u64) -> Result; /// `CMSG_GROUP_DECLINE`. - fn group_decline(&self, account_id: u64, self_guid: u64) -> Result<()>; + fn group_decline(&self, account_id: u64, self_guid: u64) -> Result; /// `CMSG_GROUP_DISBAND` (the client's "Leave Party"). - fn group_leave(&self, account_id: u64, self_guid: u64) -> Result<()>; + fn group_leave(&self, account_id: u64, self_guid: u64) -> Result; /// `CMSG_GROUP_UNINVITE` (name gateway-resolved) — the leader kicks a member. - fn group_uninvite(&self, account_id: u64, self_guid: u64, target_guid: u64) -> Result<()>; + fn group_uninvite( + &self, + account_id: u64, + self_guid: u64, + target_guid: u64, + ) -> Result; /// `CMSG_LOOT_METHOD` — the leader sets the party's loot method/ /// threshold/master. `loot_setting`/`loot_threshold` are the gateway-decoded `GroupLootSetting`/ /// `ItemQuality` wire bytes, passed straight through (the module adopted the wire ordering @@ -1001,7 +1038,7 @@ pub trait WorldStore: loot_setting: u8, master_guid: u64, loot_threshold: u8, - ) -> Result<()>; + ) -> Result; /// `CMSG_LOOT_ROLL` — record the caller's need/greed/pass vote. fn loot_roll( &self, diff --git a/gateway/src/world/tests.rs b/gateway/src/world/tests.rs index d6e0b43c6..98849fe4c 100644 --- a/gateway/src/world/tests.rs +++ b/gateway/src/world/tests.rs @@ -3,10 +3,13 @@ use super::handlers::{ ItemActionStore, LootWindowRefusal, LootWindowRequestStatus, LootWindowStore, MeleeActionStore, QuestActionStore, TaxiActionStore, VendorActionStore, WeatherStore, }; +use super::party::PartyOutcome; use super::*; use crate::read_deadline::{DeadlineClock, PreAuthDeadline}; +use lyracore_shared::group::GroupRefusal; use lyracore_shared::item::ItemRefusal; use lyracore_shared::loot::LootRefusal; +use lyracore_shared::social::ContactRefusal; use std::cell::Cell; use std::io::Cursor; use std::os::unix::net::UnixStream; @@ -351,8 +354,9 @@ struct InMemoryStore { /// NAME as the pre-realm-core path passes it (the module resolves it). The single-database plane's /// byte-identity is asserted against this. whispers: std::sync::Mutex>, - /// When set, `party_chat` returns this error — e.g. `group_err::NOT_IN_GROUP` - /// to drive the "not in a group" → `SMSG_PARTY_COMMAND_RESULT(NotInGroup)` mapping. + /// When set, `party_chat` answers this reducer error — a `GroupRefusal` tag becomes a Refusal, + /// anything else stays a failure. `NotInGroup`'s tag drives the + /// `SMSG_PARTY_COMMAND_RESULT(NotInGroup)` mapping. party_chat_error: Option, /// Recorded `party_chat` messages — the dispatch test asserts the RIGHT text /// reached the reducer call. @@ -738,7 +742,36 @@ struct InMemoryStore { ignore_trades: std::sync::Mutex>, } +/// The Fake's reducer edge for a party op: the Module answers a Refusal as the bare tag, and +/// anything else — a timeout, a dead transport — is a failure with an unknown durable outcome. +fn faked_party(error: &str) -> Result { + match lyracore_shared::group::GroupRefusal::parse_tag(error) { + Some(refusal) => Ok(refusal.into()), + None => Err(anyhow!("{error}")), + } +} + +/// [`faked_party`] for the friends and ignore lists. +fn faked_contact(error: &str) -> Result { + match lyracore_shared::social::ContactRefusal::parse_tag(error) { + Some(refusal) => Ok(refusal.into()), + None => Err(anyhow!("{error}")), + } +} + impl InMemoryStore { + /// Drop one contact row, or refuse when the owner does not hold it. + fn remove_contact(&self, target_guid: u64, is_ignore: bool) -> Result { + let owner = self.login_entity.as_ref().map(|e| e.guid).unwrap_or(0); + let mut contacts = self.contacts.lock().unwrap(); + let before = contacts.len(); + contacts.retain(|&(o, t, ig)| !(o == owner && t == target_guid && ig == is_ignore)); + if contacts.len() == before { + return Ok(lyracore_shared::social::ContactRefusal::NotOnList.into()); + } + Ok(ContactOutcome::Done) + } + /// Record one player-scoped call against THIS handle's shard. fn rec(&self, what: &str) { self.calls @@ -2078,12 +2111,17 @@ impl WorldStore for InMemoryStore { None => Ok(()), } } - fn party_chat(&self, _account_id: u64, _self_guid: u64, message: String) -> Result<()> { + fn party_chat( + &self, + _account_id: u64, + _self_guid: u64, + message: String, + ) -> Result { match &self.party_chat_error { - Some(e) => Err(anyhow!("{e}")), + Some(e) => faked_party(e), None => { self.party_chats.lock().unwrap().push(message); - Ok(()) + Ok(PartyOutcome::Ran) } } } @@ -2226,17 +2264,22 @@ impl WorldStore for InMemoryStore { // Each of these records the SHARD it ran on (`rec`), so a test can tell the // single-database path (the op lands on the player's own shard, here) apart from the realm-core // one (it lands in `FakeParty::ops` and never reaches these at all). - fn group_invite(&self, _account_id: u64, _self_guid: u64, target_guid: u64) -> Result<()> { + fn group_invite( + &self, + _account_id: u64, + _self_guid: u64, + target_guid: u64, + ) -> Result { self.rec("group_invite"); if let Some(e) = &self.trade_error { - return Err(anyhow!("{e}")); + return faked_party(e); } self.group_invites.lock().unwrap().push(target_guid); - Ok(()) + Ok(PartyOutcome::Ran) } - fn group_accept(&self, _account_id: u64, _self_guid: u64) -> Result<()> { + fn group_accept(&self, _account_id: u64, _self_guid: u64) -> Result { self.rec("group_accept"); - Ok(()) + Ok(PartyOutcome::Ran) } // Trade (#120): pure recorders — the module owns every gate, so the fake just proves which // verb the dispatch chose and which args survived the wire. @@ -2308,13 +2351,13 @@ impl WorldStore for InMemoryStore { self.ignore_trades.lock().unwrap().push(self_guid); Ok(()) } - fn group_decline(&self, _account_id: u64, _self_guid: u64) -> Result<()> { + fn group_decline(&self, _account_id: u64, _self_guid: u64) -> Result { self.rec("group_decline"); - Ok(()) + Ok(PartyOutcome::Ran) } - fn group_leave(&self, _account_id: u64, _self_guid: u64) -> Result<()> { + fn group_leave(&self, _account_id: u64, _self_guid: u64) -> Result { self.rec("group_leave"); - Ok(()) + Ok(PartyOutcome::Ran) } fn group_loot_method( &self, @@ -2323,16 +2366,16 @@ impl WorldStore for InMemoryStore { loot_setting: u8, master_guid: u64, loot_threshold: u8, - ) -> Result<()> { + ) -> Result { self.rec("group_loot_method"); if let Some(e) = &self.trade_error { - return Err(anyhow!("{e}")); + return faked_party(e); } self.group_loot_methods .lock() .unwrap() .push((loot_setting, master_guid, loot_threshold)); - Ok(()) + Ok(PartyOutcome::Ran) } // --- The realm-core plane (party/group routing) --- @@ -2370,15 +2413,15 @@ impl WorldStore for InMemoryStore { target_guid: u64, arg_a: u8, arg_b: u8, - ) -> Result<()> { - use lyracore_shared::group::{err as group_err, event_kind as kind, realm_op}; + ) -> Result { + use lyracore_shared::group::{event_kind as kind, realm_op, GroupRefusal}; self.rec("realm_group_op"); let mut p = self.party.lock().unwrap(); p.ops.push((op, actor_guid, target_guid, arg_a, arg_b)); match op { realm_op::INVITE => { if p.group_of(target_guid).is_some() { - return Err(anyhow!("{}", group_err::ALREADY_IN_GROUP)); + return Ok(GroupRefusal::AlreadyInGroup.into()); } p.invites.retain(|(t, _)| *t != target_guid); p.invites.push((target_guid, actor_guid)); @@ -2386,14 +2429,16 @@ impl WorldStore for InMemoryStore { } realm_op::ACCEPT => { if let Some(e) = &self.party_accept_error { - return Err(anyhow!("{e}")); + return faked_party(e); } - let inviter = p + let Some(inviter) = p .invites .iter() .find(|(t, _)| *t == actor_guid) .map(|(_, i)| *i) - .ok_or_else(|| anyhow!("no pending invite"))?; + else { + return Ok(GroupRefusal::NoPendingInvite.into()); + }; p.invites.retain(|(t, _)| *t != actor_guid); let group_id = match p.group_of(inviter) { Some(g) => g, @@ -2410,40 +2455,42 @@ impl WorldStore for InMemoryStore { p.push_list(group_id); } realm_op::DECLINE => { - let inviter = p + let Some(inviter) = p .invites .iter() .find(|(t, _)| *t == actor_guid) .map(|(_, i)| *i) - .ok_or_else(|| anyhow!("no pending invite"))?; + else { + return Ok(GroupRefusal::NoPendingInvite.into()); + }; p.invites.retain(|(t, _)| *t != actor_guid); p.events.push((inviter, kind::DECLINE)); } realm_op::LEAVE => { if p.group_of(actor_guid).is_none() { - return Err(anyhow!("{}", group_err::NOT_IN_GROUP)); + return Ok(GroupRefusal::NotInGroup.into()); } p.remove_member(actor_guid); } realm_op::UNINVITE => { - let group_id = p - .group_of(actor_guid) - .ok_or_else(|| anyhow!("{}", group_err::NOT_IN_GROUP))?; + let Some(group_id) = p.group_of(actor_guid) else { + return Ok(GroupRefusal::NotInGroup.into()); + }; if p.groups.iter().find(|(g, ..)| *g == group_id).map(|e| e.1) != Some(actor_guid) { - return Err(anyhow!("{}", group_err::NOT_LEADER)); + return Ok(GroupRefusal::NotLeader.into()); } if p.group_of(target_guid) != Some(group_id) { - return Err(anyhow!("{}", group_err::TARGET_NOT_IN_GROUP)); + return Ok(GroupRefusal::TargetNotInGroup.into()); } p.remove_member(target_guid); } realm_op::LOOT_METHOD => { - let group_id = p - .group_of(actor_guid) - .ok_or_else(|| anyhow!("{}", group_err::NOT_IN_GROUP))?; + let Some(group_id) = p.group_of(actor_guid) else { + return Ok(GroupRefusal::NotInGroup.into()); + }; if let Some(entry) = p.groups.iter_mut().find(|(g, ..)| *g == group_id) { if entry.1 != actor_guid { - return Err(anyhow!("{}", group_err::NOT_LEADER)); + return Ok(GroupRefusal::NotLeader.into()); } entry.2 = arg_a; entry.3 = arg_b; @@ -2453,7 +2500,7 @@ impl WorldStore for InMemoryStore { } other => return Err(anyhow!("unknown realm group op {other}")), } - Ok(()) + Ok(PartyOutcome::Ran) } fn group_roster(&self, character_guid: u64) -> Result> { @@ -2651,9 +2698,14 @@ impl WorldStore for InMemoryStore { Ok((watermark, wins)) } - fn group_uninvite(&self, _account_id: u64, _self_guid: u64, _target_guid: u64) -> Result<()> { + fn group_uninvite( + &self, + _account_id: u64, + _self_guid: u64, + _target_guid: u64, + ) -> Result { self.rec("group_uninvite"); - Ok(()) + Ok(PartyOutcome::Ran) } fn gossip_select( &self, @@ -2669,47 +2721,53 @@ impl WorldStore for InMemoryStore { .push((option_id, option_row_id)); Ok(()) } - fn add_friend(&self, _account_id: u64, _self_guid: u64, target_guid: u64) -> Result<()> { + fn add_friend( + &self, + _account_id: u64, + _self_guid: u64, + target_guid: u64, + ) -> Result { if let Some(e) = &self.trade_error { - return Err(anyhow!("{e}")); + return faked_contact(e); } let owner = self.login_entity.as_ref().map(|e| e.guid).unwrap_or(0); self.contacts .lock() .unwrap() .push((owner, target_guid, false)); - Ok(()) + Ok(ContactOutcome::Done) } - fn del_friend(&self, _account_id: u64, _self_guid: u64, target_guid: u64) -> Result<()> { - let owner = self.login_entity.as_ref().map(|e| e.guid).unwrap_or(0); - let mut contacts = self.contacts.lock().unwrap(); - let before = contacts.len(); - contacts.retain(|&(o, t, ig)| !(o == owner && t == target_guid && !ig)); - if contacts.len() == before { - return Err(anyhow!("not on that list")); - } - Ok(()) + fn del_friend( + &self, + _account_id: u64, + _self_guid: u64, + target_guid: u64, + ) -> Result { + self.remove_contact(target_guid, false) } - fn add_ignore(&self, _account_id: u64, _self_guid: u64, target_guid: u64) -> Result<()> { + fn add_ignore( + &self, + _account_id: u64, + _self_guid: u64, + target_guid: u64, + ) -> Result { if let Some(e) = &self.trade_error { - return Err(anyhow!("{e}")); + return faked_contact(e); } let owner = self.login_entity.as_ref().map(|e| e.guid).unwrap_or(0); self.contacts .lock() .unwrap() .push((owner, target_guid, true)); - Ok(()) + Ok(ContactOutcome::Done) } - fn del_ignore(&self, _account_id: u64, _self_guid: u64, target_guid: u64) -> Result<()> { - let owner = self.login_entity.as_ref().map(|e| e.guid).unwrap_or(0); - let mut contacts = self.contacts.lock().unwrap(); - let before = contacts.len(); - contacts.retain(|&(o, t, ig)| !(o == owner && t == target_guid && ig)); - if contacts.len() == before { - return Err(anyhow!("not on that list")); - } - Ok(()) + fn del_ignore( + &self, + _account_id: u64, + _self_guid: u64, + target_guid: u64, + ) -> Result { + self.remove_contact(target_guid, true) } } @@ -5517,6 +5575,85 @@ fn group_invite_by_name_replies_party_command_result_success() { let _ = server.join(); } +/// Every party Refusal the Module can send reaches the client as exactly one `PartyResult`, through +/// the real store seam rather than the mapping function alone. +#[test] +fn every_group_refusal_reaches_the_client_as_one_party_result() { + use wow_world_messages::vanilla::PartyResult; + for refusal in GroupRefusal::ALL { + let want = match refusal { + GroupRefusal::AlreadyInGroup => PartyResult::AlreadyInGroup, + GroupRefusal::GroupFull => PartyResult::GroupFull, + GroupRefusal::NotLeader => PartyResult::NotLeader, + GroupRefusal::NotInGroup => PartyResult::NotInGroup, + GroupRefusal::TargetNotInGroup => PartyResult::TargetNotInGroup, + _ => PartyResult::BadPlayerName, + }; + let mut s = quest_store(); + s.characters = vec![codec::CharacterView { + guid: 2, + name: "Buddy".into(), + ..Default::default() + }]; + s.trade_error = Some(refusal.as_tag().to_string()); + let store = std::sync::Arc::new(s); + let (mut client, mut c_enc, mut c_dec, server) = enter_world(store, 1); + wow_world_messages::vanilla::CMSG_GROUP_INVITE { + name: "Buddy".into(), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + match ServerOpcodeMessage::read_encrypted(&mut client, &mut c_dec).unwrap() { + ServerOpcodeMessage::SMSG_PARTY_COMMAND_RESULT(r) => { + assert_eq!(r.result, want, "{refusal:?} must map to {want:?}") + } + other => panic!("expected SMSG_PARTY_COMMAND_RESULT, got {other}"), + } + drop(client); + let _ = server.join(); + } +} + +/// A reducer that timed out left the party in an unknown state, so it must end the session rather +/// than pose as a gameplay answer the client renders. +#[test] +fn a_group_invite_timeout_is_not_answered_as_a_refusal() { + let mut s = quest_store(); + s.characters = vec![codec::CharacterView { + guid: 2, + name: "Buddy".into(), + ..Default::default() + }]; + s.login_entity = Some(warrior_entity()); + s.trade_error = Some("gw_group_invite reducer timed out after 10s".into()); + let store = std::sync::Arc::new(s); + let (mut client, server_end) = world_session_socket_pair(); + let server_store = store.clone(); + let (result_tx, result_rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + result_tx + .send(run_world_session(server_end, server_store.as_ref())) + .unwrap(); + }); + let (mut c_enc, mut c_dec) = client_handshake(&mut client, "TESTER", K); + CMSG_PLAYER_LOGIN { guid: Guid::new(1) } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + for _ in 0..WORLD_ENTRY_PACKETS { + ServerOpcodeMessage::read_encrypted(&mut client, &mut c_dec).unwrap(); + } + wow_world_messages::vanilla::CMSG_GROUP_INVITE { + name: "Buddy".into(), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + let error = result_rx + .recv_timeout(std::time::Duration::from_secs(1)) + .expect("an unknown party outcome must end the session promptly") + .expect_err("a timed-out party reducer must be session-fatal"); + assert!(format!("{error:#}").contains("timed out")); +} + #[test] fn group_invite_unknown_name_replies_bad_player_name() { // An unresolvable name never reaches the store — the reply is BadPlayerName ("player not found"). @@ -5650,10 +5787,10 @@ fn add_ignore_by_name_replies_ignore_added() { #[test] fn add_friend_maps_self_already_and_full_errors() { - for (err, want) in [ - ("cannot add yourself", FriendResult::SelfX), - ("already added", FriendResult::Already), - ("list full", FriendResult::ListFull), + for (refusal, want) in [ + (ContactRefusal::AddSelf, FriendResult::SelfX), + (ContactRefusal::AlreadyOnList, FriendResult::Already), + (ContactRefusal::ListFull, FriendResult::ListFull), ] { let mut s = quest_store(); s.characters = vec![codec::CharacterView { @@ -5661,7 +5798,7 @@ fn add_friend_maps_self_already_and_full_errors() { name: "Buddy".into(), ..Default::default() }]; - s.trade_error = Some(err.into()); + s.trade_error = Some(refusal.as_tag().to_string()); let store = std::sync::Arc::new(s); let (mut client, mut c_enc, mut c_dec, server) = enter_world(store, 1); CMSG_ADD_FRIEND { @@ -5671,7 +5808,7 @@ fn add_friend_maps_self_already_and_full_errors() { .unwrap(); match ServerOpcodeMessage::read_encrypted(&mut client, &mut c_dec).unwrap() { ServerOpcodeMessage::SMSG_FRIEND_STATUS(s) => { - assert_eq!(s.result, want, "store error {err:?} must map to {want:?}") + assert_eq!(s.result, want, "{refusal:?} must map to {want:?}") } other => panic!("expected SMSG_FRIEND_STATUS, got {other}"), } @@ -5682,9 +5819,10 @@ fn add_friend_maps_self_already_and_full_errors() { #[test] fn add_ignore_maps_already_and_full_errors_to_the_ignore_variants() { - for (err, want) in [ - ("already added", FriendResult::IgnoreAlready), - ("list full", FriendResult::IgnoreFull), + for (refusal, want) in [ + (ContactRefusal::AddSelf, FriendResult::IgnoreSelf), + (ContactRefusal::AlreadyOnList, FriendResult::IgnoreAlready), + (ContactRefusal::ListFull, FriendResult::IgnoreFull), ] { let mut s = quest_store(); s.characters = vec![codec::CharacterView { @@ -5692,7 +5830,7 @@ fn add_ignore_maps_already_and_full_errors_to_the_ignore_variants() { name: "Pest".into(), ..Default::default() }]; - s.trade_error = Some(err.into()); + s.trade_error = Some(refusal.as_tag().to_string()); let store = std::sync::Arc::new(s); let (mut client, mut c_enc, mut c_dec, server) = enter_world(store, 1); CMSG_ADD_IGNORE { @@ -5702,7 +5840,7 @@ fn add_ignore_maps_already_and_full_errors_to_the_ignore_variants() { .unwrap(); match ServerOpcodeMessage::read_encrypted(&mut client, &mut c_dec).unwrap() { ServerOpcodeMessage::SMSG_FRIEND_STATUS(s) => { - assert_eq!(s.result, want, "store error {err:?} must map to {want:?}") + assert_eq!(s.result, want, "{refusal:?} must map to {want:?}") } other => panic!("expected SMSG_FRIEND_STATUS, got {other}"), } @@ -5711,6 +5849,92 @@ fn add_ignore_maps_already_and_full_errors_to_the_ignore_variants() { } } +/// Every contact Refusal the Module can send reaches the client as exactly one `FriendResult`, and +/// the friends and ignore lists get their own code family for the same condition. +#[test] +fn every_contact_refusal_reaches_the_client_as_one_friend_result() { + for refusal in ContactRefusal::ALL { + let (friend, ignore) = match refusal { + ContactRefusal::AddSelf => (FriendResult::SelfX, FriendResult::IgnoreSelf), + ContactRefusal::AlreadyOnList => (FriendResult::Already, FriendResult::IgnoreAlready), + ContactRefusal::ListFull => (FriendResult::ListFull, FriendResult::IgnoreFull), + ContactRefusal::NotOnList => (FriendResult::NotFound, FriendResult::IgnoreNotFound), + ContactRefusal::NoSuchPlayer => (FriendResult::NotFound, FriendResult::NotFound), + }; + for (is_ignore, want) in [(false, friend), (true, ignore)] { + let mut s = quest_store(); + s.characters = vec![codec::CharacterView { + guid: 2, + name: "Buddy".into(), + ..Default::default() + }]; + s.trade_error = Some(refusal.as_tag().to_string()); + let store = std::sync::Arc::new(s); + let (mut client, mut c_enc, mut c_dec, server) = enter_world(store, 1); + if is_ignore { + CMSG_ADD_IGNORE { + name: "Buddy".into(), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + } else { + CMSG_ADD_FRIEND { + name: "Buddy".into(), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + } + match ServerOpcodeMessage::read_encrypted(&mut client, &mut c_dec).unwrap() { + ServerOpcodeMessage::SMSG_FRIEND_STATUS(s) => { + assert_eq!(s.result, want, "{refusal:?} on ignore={is_ignore}") + } + other => panic!("expected SMSG_FRIEND_STATUS, got {other}"), + } + drop(client); + server.join().unwrap(); + } + } +} + +/// The contact half of the same rule: a timed-out add left the list in an unknown state. +#[test] +fn an_add_friend_timeout_is_not_answered_as_a_refusal() { + let mut s = quest_store(); + s.characters = vec![codec::CharacterView { + guid: 2, + name: "Buddy".into(), + ..Default::default() + }]; + s.login_entity = Some(warrior_entity()); + s.trade_error = Some("gw_add_friend reducer timed out after 10s".into()); + let store = std::sync::Arc::new(s); + let (mut client, server_end) = world_session_socket_pair(); + let server_store = store.clone(); + let (result_tx, result_rx) = std::sync::mpsc::channel(); + std::thread::spawn(move || { + result_tx + .send(run_world_session(server_end, server_store.as_ref())) + .unwrap(); + }); + let (mut c_enc, mut c_dec) = client_handshake(&mut client, "TESTER", K); + CMSG_PLAYER_LOGIN { guid: Guid::new(1) } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + for _ in 0..WORLD_ENTRY_PACKETS { + ServerOpcodeMessage::read_encrypted(&mut client, &mut c_dec).unwrap(); + } + CMSG_ADD_FRIEND { + name: "Buddy".into(), + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + let error = result_rx + .recv_timeout(std::time::Duration::from_secs(1)) + .expect("an unknown contact outcome must end the session promptly") + .expect_err("a timed-out contact reducer must be session-fatal"); + assert!(format!("{error:#}").contains("timed out")); +} + #[test] fn del_friend_unknown_contact_replies_not_found() { let store = std::sync::Arc::new(quest_store()); @@ -9055,11 +9279,10 @@ fn messagechat_party_from_a_grouped_caller_routes_to_party_chat() { #[test] fn messagechat_party_from_an_ungrouped_caller_replies_not_in_group() { - // The module's "not in a group" rejection maps to the SAME - // SMSG_PARTY_COMMAND_RESULT(NotInGroup) line `group_leave`/`group_uninvite` already use for - // this exact reducer error (the shared `lyracore_shared::group::err::NOT_IN_GROUP` contract). + // The module's NotInGroup Refusal maps to the SAME SMSG_PARTY_COMMAND_RESULT(NotInGroup) line + // `group_leave`/`group_uninvite` already use for this exact condition. let mut s = quest_store(); - s.party_chat_error = Some(lyracore_shared::group::err::NOT_IN_GROUP.to_string()); + s.party_chat_error = Some(GroupRefusal::NotInGroup.as_tag().to_string()); let store = std::sync::Arc::new(s); let (mut client, mut c_enc, mut c_dec, server) = enter_world(store.clone(), 1); CMSG_MESSAGECHAT { diff --git a/module/src/chat.rs b/module/src/chat.rs index 5ba3c8a1a..d2d346a96 100644 --- a/module/src/chat.rs +++ b/module/src/chat.rs @@ -10,7 +10,8 @@ use spacetimedb::{reducer, table, Identity, ReducerContext, Table, Timestamp}; use crate::game_character; -use lyracore_shared::group::{err as group_err, event_kind as group_event_kind}; +use lyracore_shared::group::{event_kind as group_event_kind, GroupRefusal}; +use lyracore_shared::social::ContactRefusal; // `game_character_contact` is defined further down in THIS module — its `#[table]` accessor // (`game_character_contact`) is generated there, so it's in scope for `add_friend`/etc. without a @@ -570,9 +571,9 @@ fn push_whisper( /// already carries (INVITE/LIST/DECLINE/DESTROYED, the work-item 187 roll/master-loot/money-share /// kinds). Bounds-checked identically to `send_chat` (trim + [`MAX_CHAT_LEN`]-cap, empty rejected). /// The caller must currently be in a group — `send_whisper`'s "no such/offline target" analog here -/// is simply "not in a group" ([`lyracore_shared::group::err::NOT_IN_GROUP`], the SAME shared-contract -/// string `group_leave`/`group_uninvite` already return for this exact condition), which the gateway -/// maps to `SMSG_PARTY_COMMAND_RESULT(NotInGroup)` — the standard "You aren't in a party" line. +/// is [`GroupRefusal::NotInGroup`], the SAME shared-contract Refusal `group_leave`/`group_uninvite` +/// already return for this exact condition, which the gateway maps to +/// `SMSG_PARTY_COMMAND_RESULT(NotInGroup)` — the standard "You aren't in a party" line. /// Unlike `send_chat`, this core takes no `language` argument: like whispers, party lines aren't /// language-filtered (always Universal on the wire), so there is nothing to thread through. /// @@ -583,8 +584,12 @@ pub(crate) fn apply_party_chat( text: String, ) -> Result<(), String> { let message = normalized_message(&text).ok_or_else(|| "empty message".to_string())?; - let membership = crate::group::group_of(ctx, sender.guid) - .ok_or_else(|| group_err::NOT_IN_GROUP.to_string())?; + let membership = crate::group::group_of(ctx, sender.guid).ok_or_else(|| { + crate::group::refused( + GroupRefusal::NotInGroup, + &format!("{} spoke on /p from no party", sender.guid), + ) + })?; let payload = lyracore_shared::group::encode_party_chat(&message); let member_guids: Vec = crate::group::members_of(ctx, membership.group_id) .into_iter() @@ -682,20 +687,44 @@ crate::character_owned!(restamp, fn sweep_restamp_game_character_contact(ctx, ch } }); -/// Shared add path for `add_friend`/`add_ignore`: `target_guid` is resolved by the GATEWAY (name → -/// guid, same lookup `/who` uses) before the reducer is called, so this only re-validates -/// server-side (never trusts the caller) — reject self, an unknown guid, a duplicate, or a full list. +/// Reducer edge for the contact list: only the tag crosses to the gateway, the detail stays here. +fn refused_contact(refusal: ContactRefusal, detail: &str) -> String { + let tag = refusal.as_tag(); + spacetimedb::log::info!("contact refused {tag}: {detail}"); + tag.to_string() +} + +/// Shared add path for `add_friend`/`add_ignore`, at the reducer edge: the core's typed Refusal +/// becomes its stable tag. pub(crate) fn add_contact( ctx: &ReducerContext, sender: crate::WorldEntity, target_guid: u64, is_ignore: bool, ) -> Result<(), String> { + let owner_guid = sender.guid; + add_contact_core(ctx, sender, target_guid, is_ignore).map_err(|refusal| { + refused_contact( + refusal, + &format!("{owner_guid} could not add {target_guid}"), + ) + }) +} + +/// `target_guid` is resolved by the GATEWAY (name → guid, same lookup `/who` uses) before the +/// reducer is called, so this only re-validates server-side (never trusts the caller) — reject +/// self, an unknown guid, a duplicate, or a full list. +fn add_contact_core( + ctx: &ReducerContext, + sender: crate::WorldEntity, + target_guid: u64, + is_ignore: bool, +) -> Result<(), ContactRefusal> { if target_guid == sender.guid { - return Err("cannot add yourself".to_string()); + return Err(ContactRefusal::AddSelf); } if ctx.db.game_character().guid().find(target_guid).is_none() { - return Err("no such player".to_string()); + return Err(ContactRefusal::NoSuchPlayer); } let contacts = ctx.db.game_character_contact(); let existing: Vec<_> = contacts.by_owner().filter(&sender.guid).collect(); @@ -703,11 +732,11 @@ pub(crate) fn add_contact( .iter() .any(|c| c.target_guid == target_guid && c.is_ignore == is_ignore) { - return Err("already added".to_string()); + return Err(ContactRefusal::AlreadyOnList); } let cap = if is_ignore { MAX_IGNORED } else { MAX_FRIENDS }; if existing.iter().filter(|c| c.is_ignore == is_ignore).count() >= cap { - return Err("list full".to_string()); + return Err(ContactRefusal::ListFull); } contacts.insert(ContactEntry { id: 0, @@ -722,7 +751,8 @@ pub(crate) fn add_contact( } /// Shared remove path for `del_friend`/`del_ignore`: deletes the caller's own row for `target_guid` -/// in the given list, or a clean `Err` if it isn't there (idempotent double-remove from the client). +/// in the given list, or [`ContactRefusal::NotOnList`] if it isn't there (idempotent double-remove +/// from the client). pub(crate) fn remove_contact( ctx: &ReducerContext, sender: crate::WorldEntity, @@ -730,11 +760,16 @@ pub(crate) fn remove_contact( is_ignore: bool, ) -> Result<(), String> { let contacts = ctx.db.game_character_contact(); - let row = contacts + let Some(row) = contacts .by_owner() .filter(&sender.guid) .find(|c| c.target_guid == target_guid && c.is_ignore == is_ignore) - .ok_or_else(|| "not on that list".to_string())?; + else { + return Err(refused_contact( + ContactRefusal::NotOnList, + &format!("{} does not hold {target_guid}", sender.guid), + )); + }; contacts.id().delete(row.id); Ok(()) } diff --git a/module/src/group.rs b/module/src/group.rs index d22633012..9c43d1e00 100644 --- a/module/src/group.rs +++ b/module/src/group.rs @@ -32,9 +32,7 @@ //! follow-up, not this slice. Kill quest-credit goes to every in-range member (vanilla). Group //! LOOT methods, round-robin, and raid groups are out of scope. -use spacetimedb::{ - reducer, table, Identity, ReducerContext, Table, Timestamp, -}; +use spacetimedb::{reducer, table, Identity, ReducerContext, Table, Timestamp}; use crate::{game_character, game_world_entity}; @@ -295,15 +293,25 @@ fn emit_bot_group_intent(ctx: &ReducerContext, op: u8, actor_guid: u64, target_g pub fn claim_bot_invite_intent(ctx: &ReducerContext, intent_id: u64) -> Result<(), String> { crate::helpers::require_operator(ctx)?; if !ctx.db.game_bot_invite_intent().id().delete(intent_id) { - return Err(lyracore_shared::group::err::BOT_INVITE_INTENT_ALREADY_CLAIMED.to_string()); + return Err(refused( + GroupRefusal::IntentAlreadyClaimed, + &format!("bot invite intent {intent_id} is gone"), + )); } Ok(()) } +/// Reducer edge: only the tag crosses to the gateway; the detail stays in module logs. +pub(crate) fn refused(refusal: GroupRefusal, detail: &str) -> String { + let tag = refusal.as_tag(); + spacetimedb::log::info!("group refused {tag}: {detail}"); + tag.to_string() +} + // Event kinds, roster grammar, and classified error strings are the SHARED wire contract: // lyracore_shared::group is the one definition both crates import — a renumber, // reword, or delimiter change is a cross-crate compile-visible edit, never a runtime drift. -use lyracore_shared::group::{bot_op, err as group_err, event_kind as group_event_kind}; +use lyracore_shared::group::{bot_op, event_kind as group_event_kind, GroupRefusal}; /// A per-recipient group notification (the `game_whisper_event` pattern): public + RLS-scoped so /// only the recipient's connection sees it; reaped by the shared event GC. `other_name` is @@ -428,27 +436,25 @@ pub(crate) fn group_of(ctx: &ReducerContext, character_guid: u64) -> Option Result<(GroupMember, Group), String> { - let m = group_of(ctx, guid).ok_or_else(|| group_err::NOT_IN_GROUP.to_string())?; +) -> Result<(GroupMember, Group), GroupRefusal> { + let m = group_of(ctx, guid).ok_or(GroupRefusal::NotInGroup)?; let group = ctx .db .game_group() .group_id() .find(m.group_id) - .ok_or("group row missing")?; + .ok_or(GroupRefusal::Database)?; if group.leader_guid != guid { - return Err(group_err::NOT_LEADER.to_string()); + return Err(GroupRefusal::NotLeader); } Ok((m, group)) } @@ -500,6 +506,7 @@ pub(crate) fn invite_core( target_guid: u64, ) -> Result<(), String> { invite_core_on(ctx, Plane::Shard, inviter_guid, target_guid) + .map_err(|r| refused(r, &format!("{inviter_guid} could not invite {target_guid}"))) } fn invite_core_on( @@ -507,20 +514,20 @@ fn invite_core_on( plane: Plane, inviter_guid: u64, target_guid: u64, -) -> Result<(), String> { +) -> Result<(), GroupRefusal> { if target_guid == inviter_guid { - return Err("cannot invite yourself".to_string()); + return Err(GroupRefusal::InviteSelf); } // EXISTENCE + PRESENCE are the two gates that need a database holding characters and live // entities, so they are the two the directory plane cannot run: realm-core has // neither table populated, and a shard's copy only knows about its own players — which is the // whole bug this slice fixes (a target inside Deadmines "does not exist" to the open world). // On REALM-CORE the gateway has already resolved both ACROSS every connected shard before - // calling (`world::party::resolve_target`), and it maps the same two refusals onto the same two - // `PartyResult`s the strings below classify to, so the player-visible answer is unchanged. + // calling (`world::party::resolve_target`), and it answers with the same two Refusals, so the + // player-visible answer is unchanged. if plane == Plane::Shard { if ctx.db.game_character().guid().find(target_guid).is_none() { - return Err("no such player".to_string()); + return Err(GroupRefusal::NoSuchPlayer); } // Vanilla requires the target online; a session-less playerbot's live entity counts (its // auto-accept rides the hook below, not a client). @@ -531,23 +538,23 @@ fn invite_core_on( .find(target_guid) .is_none() { - return Err("player not online".to_string()); + return Err(GroupRefusal::TargetOffline); } } if group_of(ctx, target_guid).is_some() { - return Err(group_err::ALREADY_IN_GROUP.to_string()); + return Err(GroupRefusal::AlreadyInGroup); } // The inviter having NO group yet is fine (they'll lead a brand-new one) — only propagate - // NOT_LEADER (inviter is in a group but isn't its leader); a led group additionally enforces the + // NotLeader (inviter is in a group but isn't its leader); a led group additionally enforces the // member cap. match led_group_of(ctx, inviter_guid) { Ok((m, _group)) => { if members_of(ctx, m.group_id).len() >= GROUP_MAX_MEMBERS { - return Err(group_err::GROUP_FULL.to_string()); + return Err(GroupRefusal::GroupFull); } } - Err(e) if e == group_err::NOT_IN_GROUP => {} - Err(e) => return Err(e), + Err(GroupRefusal::NotInGroup) => {} + Err(refusal) => return Err(refusal), } let invites = ctx.db.game_group_invite(); for stale in invites.by_target().filter(&target_guid).collect::>() { @@ -594,18 +601,23 @@ fn invite_core_on( /// acceptor (a playerbot's auto-accept hook calls this with the bot's guid). pub(crate) fn accept_invite_for(ctx: &ReducerContext, acceptor_guid: u64) -> Result<(), String> { accept_invite_on(ctx, Plane::Shard, acceptor_guid) + .map_err(|r| refused(r, &format!("{acceptor_guid} could not accept its invite"))) } -fn accept_invite_on(ctx: &ReducerContext, plane: Plane, acceptor_guid: u64) -> Result<(), String> { +fn accept_invite_on( + ctx: &ReducerContext, + plane: Plane, + acceptor_guid: u64, +) -> Result<(), GroupRefusal> { let invites = ctx.db.game_group_invite(); let invite = invites .by_target() .filter(&acceptor_guid) .next() - .ok_or_else(|| "no pending invite".to_string())?; + .ok_or(GroupRefusal::NoPendingInvite)?; invites.id().delete(invite.id); if group_of(ctx, acceptor_guid).is_some() { - return Err(group_err::ALREADY_IN_GROUP.to_string()); + return Err(GroupRefusal::AlreadyInGroup); } let inviter_guid = invite.inviter_guid; // The member row's `owner_identity` is the SHARD's binding for that character. On realm-core @@ -620,7 +632,7 @@ fn accept_invite_on(ctx: &ReducerContext, plane: Plane, acceptor_guid: u64) -> R .game_character() .guid() .find(inviter_guid) - .ok_or_else(|| "inviter no longer exists".to_string())? + .ok_or(GroupRefusal::InviterUnavailable)? .owner_identity, ), Plane::RealmCore => None, @@ -637,12 +649,12 @@ fn accept_invite_on(ctx: &ReducerContext, plane: Plane, acceptor_guid: u64) -> R .game_group() .group_id() .find(m.group_id) - .ok_or("group row missing")?; + .ok_or(GroupRefusal::Database)?; if group.leader_guid != inviter_guid { - return Err("inviter no longer leads a group".to_string()); + return Err(GroupRefusal::InviterUnavailable); } if members_of(ctx, m.group_id).len() >= GROUP_MAX_MEMBERS { - return Err(group_err::GROUP_FULL.to_string()); + return Err(GroupRefusal::GroupFull); } m.group_id } @@ -674,7 +686,7 @@ fn accept_invite_on(ctx: &ReducerContext, plane: Plane, acceptor_guid: u64) -> R .guid() .find(acceptor_guid) .map(|c| c.owner_identity) - .ok_or_else(|| "acceptor no longer exists".to_string())?, + .ok_or(GroupRefusal::NoSuchPlayer)?, Plane::RealmCore => Identity::ZERO, }; members.insert(GroupMember { @@ -690,12 +702,17 @@ fn accept_invite_on(ctx: &ReducerContext, plane: Plane, acceptor_guid: u64) -> R /// The identity-free decline core: the body `group_decline` used to inline, so the realm-core /// plane runs the SAME code rather than a second implementation of it. pub(crate) fn decline_invite_for(ctx: &ReducerContext, decliner_guid: u64) -> Result<(), String> { + decline_invite_on(ctx, decliner_guid) + .map_err(|r| refused(r, &format!("{decliner_guid} could not decline an invite"))) +} + +fn decline_invite_on(ctx: &ReducerContext, decliner_guid: u64) -> Result<(), GroupRefusal> { let invites = ctx.db.game_group_invite(); let invite = invites .by_target() .filter(&decliner_guid) .next() - .ok_or_else(|| "no pending invite".to_string())?; + .ok_or(GroupRefusal::NoPendingInvite)?; invites.id().delete(invite.id); push_event( ctx, @@ -709,8 +726,13 @@ pub(crate) fn decline_invite_for(ctx: &ReducerContext, decliner_guid: u64) -> Re /// The identity-free leave core — the body `group_leave` used to inline. pub(crate) fn leave_group_for(ctx: &ReducerContext, leaver_guid: u64) -> Result<(), String> { + leave_group_on(ctx, leaver_guid) + .map_err(|r| refused(r, &format!("{leaver_guid} could not leave its party"))) +} + +fn leave_group_on(ctx: &ReducerContext, leaver_guid: u64) -> Result<(), GroupRefusal> { if group_of(ctx, leaver_guid).is_none() { - return Err(group_err::NOT_IN_GROUP.to_string()); + return Err(GroupRefusal::NotInGroup); } remove_member(ctx, leaver_guid); Ok(()) @@ -722,14 +744,22 @@ pub(crate) fn uninvite_from_group( leader_guid: u64, target_guid: u64, ) -> Result<(), String> { + uninvite_on(ctx, leader_guid, target_guid) + .map_err(|r| refused(r, &format!("{leader_guid} could not kick {target_guid}"))) +} + +fn uninvite_on( + ctx: &ReducerContext, + leader_guid: u64, + target_guid: u64, +) -> Result<(), GroupRefusal> { let (m, _group) = led_group_of(ctx, leader_guid)?; - let target = - group_of(ctx, target_guid).ok_or_else(|| group_err::TARGET_NOT_IN_GROUP.to_string())?; + let target = group_of(ctx, target_guid).ok_or(GroupRefusal::TargetNotInGroup)?; if target.group_id != m.group_id { - return Err(group_err::TARGET_NOT_IN_GROUP.to_string()); + return Err(GroupRefusal::TargetNotInGroup); } if target_guid == leader_guid { - return Err("use leave, not kick".to_string()); + return Err(GroupRefusal::KickSelf); } remove_member(ctx, target_guid); Ok(()) @@ -743,18 +773,29 @@ pub(crate) fn set_loot_method_for( master_guid: u64, loot_threshold: u8, ) -> Result<(), String> { + set_loot_method_on(ctx, leader_guid, loot_setting, master_guid, loot_threshold).map_err(|r| { + refused( + r, + &format!("{leader_guid} could not set the party loot rules"), + ) + }) +} + +fn set_loot_method_on( + ctx: &ReducerContext, + leader_guid: u64, + loot_setting: u8, + master_guid: u64, + loot_threshold: u8, +) -> Result<(), GroupRefusal> { let (m, mut group) = led_group_of(ctx, leader_guid)?; - if !valid_loot_method(loot_setting) { - return Err("invalid loot method".to_string()); - } - if !valid_loot_threshold(loot_threshold) { - return Err("invalid loot threshold".to_string()); + if !valid_loot_method(loot_setting) || !valid_loot_threshold(loot_threshold) { + return Err(GroupRefusal::InvalidLootRules); } let resolved_master = if loot_setting == loot_method::MASTER { - let target = - group_of(ctx, master_guid).ok_or_else(|| group_err::TARGET_NOT_IN_GROUP.to_string())?; + let target = group_of(ctx, master_guid).ok_or(GroupRefusal::TargetNotInGroup)?; if target.group_id != m.group_id { - return Err(group_err::TARGET_NOT_IN_GROUP.to_string()); + return Err(GroupRefusal::TargetNotInGroup); } master_guid } else { @@ -923,16 +964,19 @@ pub fn realm_group_op( ) -> Result<(), String> { crate::helpers::require_operator(ctx)?; use lyracore_shared::group::realm_op; - match op { + // An op byte this module does not know is a gateway newer than the module — a deployment fault, + // not a party outcome, so it stays an untagged error the gateway treats as a failure. + let ran = match op { realm_op::INVITE => invite_core_on(ctx, Plane::RealmCore, actor_guid, target_guid), realm_op::ACCEPT => accept_invite_on(ctx, Plane::RealmCore, actor_guid), - realm_op::DECLINE => decline_invite_for(ctx, actor_guid), - realm_op::LEAVE => leave_group_for(ctx, actor_guid), - realm_op::UNINVITE => uninvite_from_group(ctx, actor_guid, target_guid), + realm_op::DECLINE => decline_invite_on(ctx, actor_guid), + realm_op::LEAVE => leave_group_on(ctx, actor_guid), + realm_op::UNINVITE => uninvite_on(ctx, actor_guid, target_guid), // `CMSG_LOOT_METHOD`'s own field order: setting, master, threshold. - realm_op::LOOT_METHOD => set_loot_method_for(ctx, actor_guid, arg_a, target_guid, arg_b), - other => Err(format!("unknown realm group op {other}")), - } + realm_op::LOOT_METHOD => set_loot_method_on(ctx, actor_guid, arg_a, target_guid, arg_b), + other => return Err(format!("unknown realm group op {other}")), + }; + ran.map_err(|r| refused(r, &format!("realm group op {op} for {actor_guid}"))) } /// Replace this database's MIRROR of one party with realm-core's authoritative roster. @@ -1288,18 +1332,15 @@ mod tests { "realm_op::ACCEPT =>", "accept_invite_on(ctx, Plane::RealmCore, actor_guid)", ), - ( - "realm_op::DECLINE =>", - "decline_invite_for(ctx, actor_guid)", - ), - ("realm_op::LEAVE =>", "leave_group_for(ctx, actor_guid)"), + ("realm_op::DECLINE =>", "decline_invite_on(ctx, actor_guid)"), + ("realm_op::LEAVE =>", "leave_group_on(ctx, actor_guid)"), ( "realm_op::UNINVITE =>", - "uninvite_from_group(ctx, actor_guid, target_guid)", + "uninvite_on(ctx, actor_guid, target_guid)", ), ( "realm_op::LOOT_METHOD =>", - "set_loot_method_for(ctx, actor_guid, arg_a, target_guid, arg_b)", + "set_loot_method_on(ctx, actor_guid, arg_a, target_guid, arg_b)", ), ] { let arm = body.split(op).nth(1).unwrap_or_else(|| { From de9877d5d56f4c1a58c318f65bb5e83915097773 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Fri, 4 Sep 2026 23:08:11 +0200 Subject: [PATCH 2/8] fix(social): separate invariant failures from refusals --- crates/lyracore-shared/src/group.rs | 8 +- crates/lyracore-shared/src/social.rs | 6 +- gateway/src/world/social.rs | 18 +-- gateway/src/world/tests.rs | 4 +- module/src/chat.rs | 2 +- module/src/group.rs | 160 ++++++++++++++++++++------- module/src/gw.rs | 50 +++++++-- 7 files changed, 182 insertions(+), 66 deletions(-) diff --git a/crates/lyracore-shared/src/group.rs b/crates/lyracore-shared/src/group.rs index 4ed50286b..4c3ed0371 100644 --- a/crates/lyracore-shared/src/group.rs +++ b/crates/lyracore-shared/src/group.rs @@ -110,6 +110,8 @@ pub mod bot_op { /// [`GroupRefusal::IntentAlreadyClaimed`] instead tells a losing Gateway callback to stop. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum GroupRefusal { + /// The acting Character is temporarily unable to perform a party action. + ActorUnavailable, /// A Character may not invite itself. InviteSelf, /// No Character row holds a guid the op named. @@ -136,12 +138,11 @@ pub enum GroupRefusal { InvalidLootRules, /// Another Gateway already claimed this bot invite intent. IntentAlreadyClaimed, - /// A durable row the party rules require is missing. - Database, } impl GroupRefusal { pub const ALL: [Self; 14] = [ + Self::ActorUnavailable, Self::InviteSelf, Self::NoSuchPlayer, Self::TargetOffline, @@ -155,11 +156,11 @@ impl GroupRefusal { Self::KickSelf, Self::InvalidLootRules, Self::IntentAlreadyClaimed, - Self::Database, ]; pub fn as_tag(self) -> &'static str { match self { + Self::ActorUnavailable => "group:actor_unavailable", Self::InviteSelf => "group:invite_self", Self::NoSuchPlayer => "group:no_such_player", Self::TargetOffline => "group:target_offline", @@ -173,7 +174,6 @@ impl GroupRefusal { Self::KickSelf => "group:kick_self", Self::InvalidLootRules => "group:invalid_loot_rules", Self::IntentAlreadyClaimed => "group:intent_already_claimed", - Self::Database => "group:database", } } diff --git a/crates/lyracore-shared/src/social.rs b/crates/lyracore-shared/src/social.rs index 90163806c..1de47d7f3 100644 --- a/crates/lyracore-shared/src/social.rs +++ b/crates/lyracore-shared/src/social.rs @@ -5,6 +5,8 @@ /// so neither tier matches on human prose. #[derive(Clone, Copy, Debug, PartialEq, Eq)] pub enum ContactRefusal { + /// The acting Character is temporarily unable to change its contact lists. + ActorUnavailable, /// A Character may not befriend or ignore itself. AddSelf, /// No Character row holds the guid the Gateway resolved. @@ -18,7 +20,8 @@ pub enum ContactRefusal { } impl ContactRefusal { - pub const ALL: [Self; 5] = [ + pub const ALL: [Self; 6] = [ + Self::ActorUnavailable, Self::AddSelf, Self::NoSuchPlayer, Self::AlreadyOnList, @@ -28,6 +31,7 @@ impl ContactRefusal { pub fn as_tag(self) -> &'static str { match self { + Self::ActorUnavailable => "social:actor_unavailable", Self::AddSelf => "social:add_self", Self::NoSuchPlayer => "social:no_such_player", Self::AlreadyOnList => "social:already_on_list", diff --git a/gateway/src/world/social.rs b/gateway/src/world/social.rs index be5ecc8c3..05f6bafcc 100644 --- a/gateway/src/world/social.rs +++ b/gateway/src/world/social.rs @@ -276,8 +276,8 @@ fn party_result(outcome: PartyOutcome) -> PartyResult { /// Map each [`GroupRefusal`] onto the vanilla `PartyResult` the client renders ("X is already in a /// group" etc.). Vanilla has no code for an offline or self-named target, a stale invite, or a -/// broken durable row, so those read as BadPlayerName — a visible, non-crashing line. The intent -/// claim never reaches a client; it is listed so a new Refusal cannot be forgotten here. +/// temporarily unavailable actor, so those read as BadPlayerName — a visible, non-crashing line. +/// The intent claim never reaches a client; it is listed so a new Refusal cannot be forgotten here. fn party_result_for(refusal: GroupRefusal) -> PartyResult { match refusal { GroupRefusal::AlreadyInGroup => PartyResult::AlreadyInGroup, @@ -285,15 +285,15 @@ fn party_result_for(refusal: GroupRefusal) -> PartyResult { GroupRefusal::NotLeader => PartyResult::NotLeader, GroupRefusal::NotInGroup => PartyResult::NotInGroup, GroupRefusal::TargetNotInGroup => PartyResult::TargetNotInGroup, - GroupRefusal::InviteSelf + GroupRefusal::ActorUnavailable + | GroupRefusal::InviteSelf | GroupRefusal::NoSuchPlayer | GroupRefusal::TargetOffline | GroupRefusal::NoPendingInvite | GroupRefusal::InviterUnavailable | GroupRefusal::KickSelf | GroupRefusal::InvalidLootRules - | GroupRefusal::IntentAlreadyClaimed - | GroupRefusal::Database => PartyResult::BadPlayerName, + | GroupRefusal::IntentAlreadyClaimed => PartyResult::BadPlayerName, } } @@ -310,7 +310,9 @@ fn friend_result_for(refusal: ContactRefusal, is_ignore: bool) -> FriendResult { (ContactRefusal::NotOnList, true) => FriendResult::IgnoreNotFound, // A guid the gateway resolved that the module cannot see, and the friend-list remove of a // row that is not there, both read as the one "no such entry" line vanilla has. - (ContactRefusal::NotOnList, false) | (ContactRefusal::NoSuchPlayer, _) => { + (ContactRefusal::NotOnList, false) + | (ContactRefusal::NoSuchPlayer, _) + | (ContactRefusal::ActorUnavailable, _) => { FriendResult::NotFound } } @@ -409,7 +411,9 @@ mod tests { } ContactRefusal::ListFull => (FriendResult::ListFull, FriendResult::IgnoreFull), ContactRefusal::NotOnList => (FriendResult::NotFound, FriendResult::IgnoreNotFound), - ContactRefusal::NoSuchPlayer => (FriendResult::NotFound, FriendResult::NotFound), + ContactRefusal::NoSuchPlayer | ContactRefusal::ActorUnavailable => { + (FriendResult::NotFound, FriendResult::NotFound) + } }; assert_eq!(friend_result_for(refusal, false), friend, "{refusal:?}"); assert_eq!(friend_result_for(refusal, true), ignore, "{refusal:?}"); diff --git a/gateway/src/world/tests.rs b/gateway/src/world/tests.rs index 98849fe4c..5863f685d 100644 --- a/gateway/src/world/tests.rs +++ b/gateway/src/world/tests.rs @@ -5859,7 +5859,9 @@ fn every_contact_refusal_reaches_the_client_as_one_friend_result() { ContactRefusal::AlreadyOnList => (FriendResult::Already, FriendResult::IgnoreAlready), ContactRefusal::ListFull => (FriendResult::ListFull, FriendResult::IgnoreFull), ContactRefusal::NotOnList => (FriendResult::NotFound, FriendResult::IgnoreNotFound), - ContactRefusal::NoSuchPlayer => (FriendResult::NotFound, FriendResult::NotFound), + ContactRefusal::NoSuchPlayer | ContactRefusal::ActorUnavailable => { + (FriendResult::NotFound, FriendResult::NotFound) + } }; for (is_ignore, want) in [(false, friend), (true, ignore)] { let mut s = quest_store(); diff --git a/module/src/chat.rs b/module/src/chat.rs index d2d346a96..4a9434ed7 100644 --- a/module/src/chat.rs +++ b/module/src/chat.rs @@ -688,7 +688,7 @@ crate::character_owned!(restamp, fn sweep_restamp_game_character_contact(ctx, ch }); /// Reducer edge for the contact list: only the tag crosses to the gateway, the detail stays here. -fn refused_contact(refusal: ContactRefusal, detail: &str) -> String { +pub(crate) fn refused_contact(refusal: ContactRefusal, detail: &str) -> String { let tag = refusal.as_tag(); spacetimedb::log::info!("contact refused {tag}: {detail}"); tag.to_string() diff --git a/module/src/group.rs b/module/src/group.rs index 9c43d1e00..f719ab59f 100644 --- a/module/src/group.rs +++ b/module/src/group.rs @@ -308,6 +308,30 @@ pub(crate) fn refused(refusal: GroupRefusal, detail: &str) -> String { tag.to_string() } +#[derive(Debug, PartialEq, Eq)] +enum GroupOpError { + Refused(GroupRefusal), + Invariant(String), +} + +impl From for GroupOpError { + fn from(refusal: GroupRefusal) -> Self { + Self::Refused(refusal) + } +} + +/// Serialize a gameplay Refusal as its stable tag, while leaving broken durable relationships +/// untagged so the Gateway treats them as session failures. +fn group_op_error(error: GroupOpError, detail: &str) -> String { + match error { + GroupOpError::Refused(refusal) => refused(refusal, detail), + GroupOpError::Invariant(reason) => { + spacetimedb::log::error!("group invariant failed: {detail}: {reason}"); + format!("group invariant failed: {reason}") + } + } +} + // Event kinds, roster grammar, and classified error strings are the SHARED wire contract: // lyracore_shared::group is the one definition both crates import — a renumber, // reword, or delimiter change is a cross-crate compile-visible edit, never a runtime drift. @@ -442,19 +466,24 @@ pub(crate) fn group_of(ctx: &ReducerContext, character_guid: u64) -> Option Result<(GroupMember, Group), GroupRefusal> { +) -> Result<(GroupMember, Group), GroupOpError> { let m = group_of(ctx, guid).ok_or(GroupRefusal::NotInGroup)?; let group = ctx .db .game_group() .group_id() .find(m.group_id) - .ok_or(GroupRefusal::Database)?; + .ok_or_else(|| { + GroupOpError::Invariant(format!( + "member {guid} points to missing group {}", + m.group_id + )) + })?; if group.leader_guid != guid { - return Err(GroupRefusal::NotLeader); + return Err(GroupRefusal::NotLeader.into()); } Ok((m, group)) } @@ -506,7 +535,12 @@ pub(crate) fn invite_core( target_guid: u64, ) -> Result<(), String> { invite_core_on(ctx, Plane::Shard, inviter_guid, target_guid) - .map_err(|r| refused(r, &format!("{inviter_guid} could not invite {target_guid}"))) + .map_err(|error| { + group_op_error( + error, + &format!("{inviter_guid} could not invite {target_guid}"), + ) + }) } fn invite_core_on( @@ -514,9 +548,9 @@ fn invite_core_on( plane: Plane, inviter_guid: u64, target_guid: u64, -) -> Result<(), GroupRefusal> { +) -> Result<(), GroupOpError> { if target_guid == inviter_guid { - return Err(GroupRefusal::InviteSelf); + return Err(GroupRefusal::InviteSelf.into()); } // EXISTENCE + PRESENCE are the two gates that need a database holding characters and live // entities, so they are the two the directory plane cannot run: realm-core has @@ -527,7 +561,7 @@ fn invite_core_on( // player-visible answer is unchanged. if plane == Plane::Shard { if ctx.db.game_character().guid().find(target_guid).is_none() { - return Err(GroupRefusal::NoSuchPlayer); + return Err(GroupRefusal::NoSuchPlayer.into()); } // Vanilla requires the target online; a session-less playerbot's live entity counts (its // auto-accept rides the hook below, not a client). @@ -538,11 +572,11 @@ fn invite_core_on( .find(target_guid) .is_none() { - return Err(GroupRefusal::TargetOffline); + return Err(GroupRefusal::TargetOffline.into()); } } if group_of(ctx, target_guid).is_some() { - return Err(GroupRefusal::AlreadyInGroup); + return Err(GroupRefusal::AlreadyInGroup.into()); } // The inviter having NO group yet is fine (they'll lead a brand-new one) — only propagate // NotLeader (inviter is in a group but isn't its leader); a led group additionally enforces the @@ -550,11 +584,11 @@ fn invite_core_on( match led_group_of(ctx, inviter_guid) { Ok((m, _group)) => { if members_of(ctx, m.group_id).len() >= GROUP_MAX_MEMBERS { - return Err(GroupRefusal::GroupFull); + return Err(GroupRefusal::GroupFull.into()); } } - Err(GroupRefusal::NotInGroup) => {} - Err(refusal) => return Err(refusal), + Err(GroupOpError::Refused(GroupRefusal::NotInGroup)) => {} + Err(error) => return Err(error), } let invites = ctx.db.game_group_invite(); for stale in invites.by_target().filter(&target_guid).collect::>() { @@ -601,14 +635,19 @@ fn invite_core_on( /// acceptor (a playerbot's auto-accept hook calls this with the bot's guid). pub(crate) fn accept_invite_for(ctx: &ReducerContext, acceptor_guid: u64) -> Result<(), String> { accept_invite_on(ctx, Plane::Shard, acceptor_guid) - .map_err(|r| refused(r, &format!("{acceptor_guid} could not accept its invite"))) + .map_err(|error| { + group_op_error( + error, + &format!("{acceptor_guid} could not accept its invite"), + ) + }) } fn accept_invite_on( ctx: &ReducerContext, plane: Plane, acceptor_guid: u64, -) -> Result<(), GroupRefusal> { +) -> Result<(), GroupOpError> { let invites = ctx.db.game_group_invite(); let invite = invites .by_target() @@ -617,7 +656,7 @@ fn accept_invite_on( .ok_or(GroupRefusal::NoPendingInvite)?; invites.id().delete(invite.id); if group_of(ctx, acceptor_guid).is_some() { - return Err(GroupRefusal::AlreadyInGroup); + return Err(GroupRefusal::AlreadyInGroup.into()); } let inviter_guid = invite.inviter_guid; // The member row's `owner_identity` is the SHARD's binding for that character. On realm-core @@ -649,12 +688,17 @@ fn accept_invite_on( .game_group() .group_id() .find(m.group_id) - .ok_or(GroupRefusal::Database)?; + .ok_or_else(|| { + GroupOpError::Invariant(format!( + "member {inviter_guid} points to missing group {}", + m.group_id + )) + })?; if group.leader_guid != inviter_guid { - return Err(GroupRefusal::InviterUnavailable); + return Err(GroupRefusal::InviterUnavailable.into()); } if members_of(ctx, m.group_id).len() >= GROUP_MAX_MEMBERS { - return Err(GroupRefusal::GroupFull); + return Err(GroupRefusal::GroupFull.into()); } m.group_id } @@ -702,11 +746,15 @@ fn accept_invite_on( /// The identity-free decline core: the body `group_decline` used to inline, so the realm-core /// plane runs the SAME code rather than a second implementation of it. pub(crate) fn decline_invite_for(ctx: &ReducerContext, decliner_guid: u64) -> Result<(), String> { - decline_invite_on(ctx, decliner_guid) - .map_err(|r| refused(r, &format!("{decliner_guid} could not decline an invite"))) + decline_invite_on(ctx, decliner_guid).map_err(|error| { + group_op_error( + error, + &format!("{decliner_guid} could not decline an invite"), + ) + }) } -fn decline_invite_on(ctx: &ReducerContext, decliner_guid: u64) -> Result<(), GroupRefusal> { +fn decline_invite_on(ctx: &ReducerContext, decliner_guid: u64) -> Result<(), GroupOpError> { let invites = ctx.db.game_group_invite(); let invite = invites .by_target() @@ -726,13 +774,17 @@ fn decline_invite_on(ctx: &ReducerContext, decliner_guid: u64) -> Result<(), Gro /// The identity-free leave core — the body `group_leave` used to inline. pub(crate) fn leave_group_for(ctx: &ReducerContext, leaver_guid: u64) -> Result<(), String> { - leave_group_on(ctx, leaver_guid) - .map_err(|r| refused(r, &format!("{leaver_guid} could not leave its party"))) + leave_group_on(ctx, leaver_guid).map_err(|error| { + group_op_error( + error, + &format!("{leaver_guid} could not leave its party"), + ) + }) } -fn leave_group_on(ctx: &ReducerContext, leaver_guid: u64) -> Result<(), GroupRefusal> { +fn leave_group_on(ctx: &ReducerContext, leaver_guid: u64) -> Result<(), GroupOpError> { if group_of(ctx, leaver_guid).is_none() { - return Err(GroupRefusal::NotInGroup); + return Err(GroupRefusal::NotInGroup.into()); } remove_member(ctx, leaver_guid); Ok(()) @@ -744,22 +796,26 @@ pub(crate) fn uninvite_from_group( leader_guid: u64, target_guid: u64, ) -> Result<(), String> { - uninvite_on(ctx, leader_guid, target_guid) - .map_err(|r| refused(r, &format!("{leader_guid} could not kick {target_guid}"))) + uninvite_on(ctx, leader_guid, target_guid).map_err(|error| { + group_op_error( + error, + &format!("{leader_guid} could not kick {target_guid}"), + ) + }) } fn uninvite_on( ctx: &ReducerContext, leader_guid: u64, target_guid: u64, -) -> Result<(), GroupRefusal> { +) -> Result<(), GroupOpError> { let (m, _group) = led_group_of(ctx, leader_guid)?; let target = group_of(ctx, target_guid).ok_or(GroupRefusal::TargetNotInGroup)?; if target.group_id != m.group_id { - return Err(GroupRefusal::TargetNotInGroup); + return Err(GroupRefusal::TargetNotInGroup.into()); } if target_guid == leader_guid { - return Err(GroupRefusal::KickSelf); + return Err(GroupRefusal::KickSelf.into()); } remove_member(ctx, target_guid); Ok(()) @@ -773,12 +829,14 @@ pub(crate) fn set_loot_method_for( master_guid: u64, loot_threshold: u8, ) -> Result<(), String> { - set_loot_method_on(ctx, leader_guid, loot_setting, master_guid, loot_threshold).map_err(|r| { - refused( - r, - &format!("{leader_guid} could not set the party loot rules"), - ) - }) + set_loot_method_on(ctx, leader_guid, loot_setting, master_guid, loot_threshold).map_err( + |error| { + group_op_error( + error, + &format!("{leader_guid} could not set the party loot rules"), + ) + }, + ) } fn set_loot_method_on( @@ -787,15 +845,15 @@ fn set_loot_method_on( loot_setting: u8, master_guid: u64, loot_threshold: u8, -) -> Result<(), GroupRefusal> { +) -> Result<(), GroupOpError> { let (m, mut group) = led_group_of(ctx, leader_guid)?; if !valid_loot_method(loot_setting) || !valid_loot_threshold(loot_threshold) { - return Err(GroupRefusal::InvalidLootRules); + return Err(GroupRefusal::InvalidLootRules.into()); } let resolved_master = if loot_setting == loot_method::MASTER { let target = group_of(ctx, master_guid).ok_or(GroupRefusal::TargetNotInGroup)?; if target.group_id != m.group_id { - return Err(GroupRefusal::TargetNotInGroup); + return Err(GroupRefusal::TargetNotInGroup.into()); } master_guid } else { @@ -976,7 +1034,9 @@ pub fn realm_group_op( realm_op::LOOT_METHOD => set_loot_method_on(ctx, actor_guid, arg_a, target_guid, arg_b), other => return Err(format!("unknown realm group op {other}")), }; - ran.map_err(|r| refused(r, &format!("realm group op {op} for {actor_guid}"))) + ran.map_err(|error| { + group_op_error(error, &format!("realm group op {op} for {actor_guid}")) + }) } /// Replace this database's MIRROR of one party with realm-core's authoritative roster. @@ -1157,6 +1217,24 @@ pub(crate) fn eligible_for_kill_reward( mod tests { use super::*; + #[test] + fn broken_group_relationship_is_not_a_gameplay_refusal() { + let failure = group_op_error( + GroupOpError::Invariant("member 7 points to missing group 9".to_string()), + "party operation", + ); + + assert!(failure.contains("group invariant failed")); + assert_eq!(GroupRefusal::parse_tag(&failure), None); + assert_eq!( + group_op_error( + GroupOpError::Refused(GroupRefusal::NotLeader), + "party operation" + ), + GroupRefusal::NotLeader.as_tag() + ); + } + /// The share filter's range gate is the vanilla 74yd group-XP distance, squared for the /// comparison `kill_reward_recipients` runs — pin it so a tuning edit is a conscious act. #[test] diff --git a/module/src/gw.rs b/module/src/gw.rs index 74f77a9bd..7af1414de 100644 --- a/module/src/gw.rs +++ b/module/src/gw.rs @@ -568,7 +568,7 @@ pub fn gw_spirit_res(ctx: &ReducerContext, actor_guid: u64) -> Result<(), String #[reducer] pub fn gw_accept_group_invite(ctx: &ReducerContext, actor_guid: u64) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + group_actor(ctx, actor_guid)?; crate::actor::accept_group_invite(ctx, actor_guid) } @@ -714,7 +714,7 @@ pub fn gw_send_whisper( #[reducer] pub fn gw_party_chat(ctx: &ReducerContext, actor_guid: u64, text: String) -> Result<(), String> { require_operator(ctx)?; - let sender = actor(ctx, actor_guid)?; + let sender = group_actor(ctx, actor_guid)?; crate::chat::apply_party_chat(ctx, sender, text) } @@ -755,11 +755,25 @@ pub fn gw_send_channel_message( crate::chat::apply_send_channel_message(ctx, sender, channel, message) } +/// Taxi flight is an expected contact Refusal. Missing actors still carry the generic actor error, +/// which the Gateway treats as a failure with an unknown result. +fn contact_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::chat::refused_contact( + lyracore_shared::social::ContactRefusal::ActorUnavailable, + &format!("actor {actor_guid} is in taxi flight"), + )); + } + Ok(actor) +} + /// [`crate::chat::add_contact`] (friend arm) with the owner named by guid. #[reducer] pub fn gw_add_friend(ctx: &ReducerContext, actor_guid: u64, target_guid: u64) -> Result<(), String> { require_operator(ctx)?; - let sender = actor(ctx, actor_guid)?; + let sender = contact_actor(ctx, actor_guid)?; crate::chat::add_contact(ctx, sender, target_guid, false) } @@ -767,7 +781,7 @@ pub fn gw_add_friend(ctx: &ReducerContext, actor_guid: u64, target_guid: u64) -> #[reducer] pub fn gw_del_friend(ctx: &ReducerContext, actor_guid: u64, target_guid: u64) -> Result<(), String> { require_operator(ctx)?; - let sender = actor(ctx, actor_guid)?; + let sender = contact_actor(ctx, actor_guid)?; crate::chat::remove_contact(ctx, sender, target_guid, false) } @@ -775,7 +789,7 @@ pub fn gw_del_friend(ctx: &ReducerContext, actor_guid: u64, target_guid: u64) -> #[reducer] pub fn gw_add_ignore(ctx: &ReducerContext, actor_guid: u64, target_guid: u64) -> Result<(), String> { require_operator(ctx)?; - let sender = actor(ctx, actor_guid)?; + let sender = contact_actor(ctx, actor_guid)?; crate::chat::add_contact(ctx, sender, target_guid, true) } @@ -783,7 +797,7 @@ pub fn gw_add_ignore(ctx: &ReducerContext, actor_guid: u64, target_guid: u64) -> #[reducer] pub fn gw_del_ignore(ctx: &ReducerContext, actor_guid: u64, target_guid: u64) -> Result<(), String> { require_operator(ctx)?; - let sender = actor(ctx, actor_guid)?; + let sender = contact_actor(ctx, actor_guid)?; crate::chat::remove_contact(ctx, sender, target_guid, true) } @@ -791,6 +805,20 @@ pub fn gw_del_ignore(ctx: &ReducerContext, actor_guid: u64, target_guid: u64) -> // Group (batch B) // =========================================================================================== +/// Taxi flight is an expected group Refusal. Missing actors still carry the generic actor error, +/// which the Gateway treats as a failure with an unknown result. +fn group_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::group::refused( + lyracore_shared::group::GroupRefusal::ActorUnavailable, + &format!("actor {actor_guid} is in taxi flight"), + )); + } + Ok(actor) +} + /// [`crate::group::invite_core`] with the inviter named by guid. #[reducer] pub fn gw_group_invite( @@ -799,7 +827,7 @@ pub fn gw_group_invite( target_guid: u64, ) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + group_actor(ctx, actor_guid)?; crate::group::invite_core(ctx, actor_guid, target_guid) } @@ -807,7 +835,7 @@ pub fn gw_group_invite( #[reducer] pub fn gw_group_decline(ctx: &ReducerContext, actor_guid: u64) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + group_actor(ctx, actor_guid)?; crate::group::decline_invite_for(ctx, actor_guid) } @@ -930,7 +958,7 @@ pub fn gw_duel_cancel(ctx: &ReducerContext, actor_guid: u64, flag_guid: u64) -> #[reducer] pub fn gw_group_leave(ctx: &ReducerContext, actor_guid: u64) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + group_actor(ctx, actor_guid)?; crate::group::leave_group_for(ctx, actor_guid) } @@ -942,7 +970,7 @@ pub fn gw_group_uninvite( target_guid: u64, ) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + group_actor(ctx, actor_guid)?; crate::group::uninvite_from_group(ctx, actor_guid, target_guid) } @@ -956,7 +984,7 @@ pub fn gw_group_loot_method( loot_threshold: u8, ) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + group_actor(ctx, actor_guid)?; crate::group::set_loot_method_for( ctx, actor_guid, From 8e97d222d334173de6b1ea3b99fc2ba1a7b015b7 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Fri, 4 Sep 2026 23:29:25 +0200 Subject: [PATCH 3/8] test(module): verify typed taxi contracts --- module/tests/support/mod.rs | 34 +++++- module/tests/typed_taxi_contracts.rs | 172 +++++++++++++++++++++++++++ 2 files changed, 201 insertions(+), 5 deletions(-) create mode 100644 module/tests/typed_taxi_contracts.rs diff --git a/module/tests/support/mod.rs b/module/tests/support/mod.rs index 8d6ed2e9f..3dc357859 100644 --- a/module/tests/support/mod.rs +++ b/module/tests/support/mod.rs @@ -9,6 +9,7 @@ use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; pub struct Standalone { child: Child, + cli_config: PathBuf, data_dir: PathBuf, spacetime: OsString, server: String, @@ -30,11 +31,13 @@ impl Standalone { std::process::id() )); fs::create_dir(&data_dir).expect("failed to create standalone data directory"); + let cli_config = data_dir.join("cli.toml"); let spacetime = std::env::var_os("SPACETIME_BIN").unwrap_or_else(|| "spacetime".into()); let address = format!("127.0.0.1:{port}"); let server = format!("http://{address}"); let child = Command::new(&spacetime) + .args(["--config-path", cli_config.to_str().unwrap()]) .args([ "start", "--listen-addr", @@ -51,6 +54,7 @@ impl Standalone { let database = format!("{test_name}-{}-{nonce}", std::process::id()); let mut standalone = Self { child, + cli_config, data_dir, spacetime, server, @@ -63,7 +67,7 @@ impl Standalone { pub fn publish_module(&self) { let module_dir = Path::new(env!("CARGO_MANIFEST_DIR")); let workspace = module_dir.parent().unwrap(); - assert_success(Command::new(&self.spacetime).current_dir(workspace).args([ + assert_success(self.command().current_dir(workspace).args([ "publish", "-s", &self.server, @@ -75,11 +79,25 @@ impl Standalone { ])); } + /// Publish one already-built Wasm artifact. The caller can hash and retain this exact file, so + /// the standalone never rebuilds different source during a contract run. + pub fn publish_module_binary(&self, wasm: &Path) { + assert_success(self.command().args([ + "publish", + "-s", + &self.server, + "--bin-path", + wasm.to_str().unwrap(), + "-y", + &self.database, + ])); + } + #[allow(dead_code)] // Used when a developer's cached token is not valid for an isolated server. pub fn publish_module_anonymous(&self) { let module_dir = Path::new(env!("CARGO_MANIFEST_DIR")); let workspace = module_dir.parent().unwrap(); - assert_success(Command::new(&self.spacetime).current_dir(workspace).args([ + assert_success(self.command().current_dir(workspace).args([ "publish", "-s", &self.server, @@ -93,7 +111,7 @@ impl Standalone { } pub fn call(&self, reducer: &str, args: &[&str]) -> Output { - let mut command = Command::new(&self.spacetime); + let mut command = self.command(); command.args(["call", "-s", &self.server, &self.database, reducer]); command.args(args); command.output().expect("failed to call reducer") @@ -105,7 +123,7 @@ impl Standalone { #[allow(dead_code)] // Paired with `publish_module_anonymous` for isolated local servers. pub fn assert_call_anonymous(&self, reducer: &str, args: &[&str]) { - let mut command = Command::new(&self.spacetime); + let mut command = self.command(); command.args([ "call", "-s", @@ -166,7 +184,7 @@ impl Standalone { } fn sql(&self, query: &str) -> Output { - Command::new(&self.spacetime) + self.command() .args([ "sql", "-s", @@ -179,6 +197,12 @@ impl Standalone { .output() .expect("failed to run SQL query") } + + fn command(&self) -> Command { + let mut command = Command::new(&self.spacetime); + command.args(["--config-path", self.cli_config.to_str().unwrap()]); + command + } } impl Drop for Standalone { diff --git a/module/tests/typed_taxi_contracts.rs b/module/tests/typed_taxi_contracts.rs new file mode 100644 index 000000000..01d35adf9 --- /dev/null +++ b/module/tests/typed_taxi_contracts.rs @@ -0,0 +1,172 @@ +mod support; + +use std::path::PathBuf; +use std::process::{Command, Output}; + +use support::Standalone; + +const ACTOR: &str = "1"; +const MISSING_ACTOR: &str = "999999"; + +#[test] +#[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] +fn gateway_taxi_gates_keep_refusals_typed_and_invariants_fatal() { + let wasm = build_frozen_module(); + let standalone = Standalone::start("typed-taxi-contracts"); + standalone.publish_module_binary(&wasm); + standalone.assert_call("claim_operator", &[]); + standalone.assert_call("debug_spawn_player_entity", &[ACTOR]); + standalone.assert_sql( + "INSERT INTO game_active_taxi_flight \ + (character_guid,path_id,source_node_id,destination_node_id,mount_display_id,fare,\ + current_node_index,started_micros) \ + VALUES (1,5090102,5090100,5090101,1147,25,0,1)", + ); + + for (reducer, args, tag) in taxi_cases() { + assert_refusal(&standalone, reducer, args, tag); + } + + for (reducer, args) in [ + ("gw_trainer_buy", &[MISSING_ACTOR, "0", "0"][..]), + ("gw_use_item", &[MISSING_ACTOR, "0"][..]), + ("gw_take_loot", &[MISSING_ACTOR, "0", "0"][..]), + ("gw_group_leave", &[MISSING_ACTOR][..]), + ("gw_add_friend", &[MISSING_ACTOR, "2"][..]), + ] { + let output = standalone.call(reducer, args); + let text = failed_text(reducer, output); + assert!(text.contains("mover not in world"), "{reducer}: {text}"); + for prefix in ["trainer:", "item:", "loot:", "group:", "social:"] { + assert!(!text.contains(prefix), "{reducer} leaked {prefix}: {text}"); + } + } + + standalone.assert_sql("DELETE FROM game_active_taxi_flight WHERE character_guid = 1"); + standalone.assert_call("realm_group_op", &["0", "1", "2", "0", "0"]); + standalone.assert_call("realm_group_op", &["1", "2", "0", "0", "0"]); + standalone.assert_sql("DELETE FROM game_group"); + + let text = failed_text( + "realm_group_op", + standalone.call("realm_group_op", &["0", "1", "3", "0", "0"]), + ); + assert!(text.contains("group invariant failed"), "{text}"); + assert!(!text.contains("group:database"), "{text}"); + assert!( + standalone + .query_rows("SELECT * FROM game_group_invite WHERE target_guid = 3") + .is_empty(), + "the failed invariant check must not leave a partial invite" + ); +} + +fn taxi_cases() -> Vec<(&'static str, &'static [&'static str], &'static str)> { + vec![ + ("gw_trainer_buy", &[ACTOR, "0", "0"], "trainer:unavailable"), + ("gw_use_item", &[ACTOR, "0"], "item:not_right_now"), + ("gw_equip_item", &[ACTOR, "0"], "item:not_right_now"), + ("gw_move_item", &[ACTOR, "0", "1"], "item:not_right_now"), + ("gw_unequip_item", &[ACTOR, "0"], "item:not_right_now"), + ( + "gw_take_loot", + &[ACTOR, "0", "0"], + "loot:looter_unavailable", + ), + ( + "gw_open_creature_loot", + &[ACTOR, "0"], + "loot:looter_unavailable", + ), + ("gw_loot_money", &[ACTOR, "0"], "loot:looter_unavailable"), + ( + "gw_use_gameobject", + &[ACTOR, "0"], + "loot:looter_unavailable", + ), + ("gw_skin", &[ACTOR, "0"], "loot:looter_unavailable"), + ( + "gw_loot_roll", + &[ACTOR, "0", "0", "0"], + "loot:looter_unavailable", + ), + ( + "gw_loot_master_give", + &[ACTOR, "0", "0", "0"], + "loot:looter_unavailable", + ), + ( + "gw_accept_group_invite", + &[ACTOR], + "group:actor_unavailable", + ), + ( + "gw_party_chat", + &[ACTOR, "\"taxi\""], + "group:actor_unavailable", + ), + ("gw_group_invite", &[ACTOR, "2"], "group:actor_unavailable"), + ("gw_group_decline", &[ACTOR], "group:actor_unavailable"), + ("gw_group_leave", &[ACTOR], "group:actor_unavailable"), + ( + "gw_group_uninvite", + &[ACTOR, "2"], + "group:actor_unavailable", + ), + ( + "gw_group_loot_method", + &[ACTOR, "0", "0", "0"], + "group:actor_unavailable", + ), + ("gw_add_friend", &[ACTOR, "2"], "social:actor_unavailable"), + ("gw_del_friend", &[ACTOR, "2"], "social:actor_unavailable"), + ("gw_add_ignore", &[ACTOR, "2"], "social:actor_unavailable"), + ("gw_del_ignore", &[ACTOR, "2"], "social:actor_unavailable"), + ] +} + +fn assert_refusal(standalone: &Standalone, reducer: &str, args: &[&str], tag: &str) { + let text = failed_text(reducer, standalone.call(reducer, args)); + assert!(text.contains(tag), "{reducer} did not return {tag}: {text}"); +} + +fn failed_text(reducer: &str, output: Output) -> String { + let text = format!( + "stdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!( + !output.status.success(), + "{reducer} unexpectedly succeeded: {text}" + ); + text +} + +fn build_frozen_module() -> PathBuf { + let module_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + let workspace = module_dir.parent().unwrap(); + let status = Command::new("cargo") + .current_dir(workspace) + .args([ + "build", + "--locked", + "--release", + "--target", + "wasm32-unknown-unknown", + "-p", + "lyracore-module", + "--features=debug_reducers", + ]) + .status() + .expect("failed to run the Wasm preflight build"); + assert!(status.success(), "the Wasm preflight build failed"); + + let target = std::env::var_os("CARGO_TARGET_DIR") + .map(PathBuf::from) + .unwrap_or_else(|| workspace.join("target")); + let wasm = target.join("wasm32-unknown-unknown/release/lyracore_module.wasm"); + let bytes = std::fs::read(&wasm).expect("the Wasm preflight output is missing"); + assert!(bytes.starts_with(b"\0asm"), "the frozen module is not Wasm"); + wasm +} From 40c97869fe35ba6a94e0c7f369687204d21e9b7b Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Fri, 4 Sep 2026 23:35:42 +0200 Subject: [PATCH 4/8] test(module): preserve published fixture bytes --- module/tests/support/mod.rs | 27 +++++++++++++++++++++++---- module/tests/typed_taxi_contracts.rs | 24 ++++++++++++++++-------- 2 files changed, 39 insertions(+), 12 deletions(-) diff --git a/module/tests/support/mod.rs b/module/tests/support/mod.rs index 3dc357859..da478230c 100644 --- a/module/tests/support/mod.rs +++ b/module/tests/support/mod.rs @@ -1,6 +1,7 @@ use std::collections::BTreeMap; use std::ffi::OsString; use std::fs; +use std::io::Write; use std::net::{TcpListener, TcpStream}; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Output, Stdio}; @@ -79,15 +80,33 @@ impl Standalone { ])); } - /// Publish one already-built Wasm artifact. The caller can hash and retain this exact file, so - /// the standalone never rebuilds different source during a contract run. - pub fn publish_module_binary(&self, wasm: &Path) { + /// Copy built Wasm bytes into this standalone's private directory and publish that copy. + pub fn publish_module_bytes(&self, wasm: &[u8]) { + let path = self.data_dir.join("published-module.wasm"); + let mut file = fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + .expect("failed to create private Wasm artifact"); + file.write_all(wasm) + .expect("failed to copy private Wasm artifact"); + file.sync_all() + .expect("failed to sync private Wasm artifact"); + let mut permissions = file + .metadata() + .expect("failed to inspect private Wasm artifact") + .permissions(); + permissions.set_readonly(true); + fs::set_permissions(&path, permissions) + .expect("failed to make private Wasm artifact read-only"); + drop(file); + assert_success(self.command().args([ "publish", "-s", &self.server, "--bin-path", - wasm.to_str().unwrap(), + path.to_str().unwrap(), "-y", &self.database, ])); diff --git a/module/tests/typed_taxi_contracts.rs b/module/tests/typed_taxi_contracts.rs index 01d35adf9..a80211939 100644 --- a/module/tests/typed_taxi_contracts.rs +++ b/module/tests/typed_taxi_contracts.rs @@ -11,9 +11,9 @@ const MISSING_ACTOR: &str = "999999"; #[test] #[ignore = "requires the SpacetimeDB 2.7.1 CLI and Wasm toolchain"] fn gateway_taxi_gates_keep_refusals_typed_and_invariants_fatal() { - let wasm = build_frozen_module(); + let wasm = build_module_bytes(); let standalone = Standalone::start("typed-taxi-contracts"); - standalone.publish_module_binary(&wasm); + standalone.publish_module_bytes(&wasm); standalone.assert_call("claim_operator", &[]); standalone.assert_call("debug_spawn_player_entity", &[ACTOR]); standalone.assert_sql( @@ -143,7 +143,7 @@ fn failed_text(reducer: &str, output: Output) -> String { text } -fn build_frozen_module() -> PathBuf { +fn build_module_bytes() -> Vec { let module_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); let workspace = module_dir.parent().unwrap(); let status = Command::new("cargo") @@ -162,11 +162,19 @@ fn build_frozen_module() -> PathBuf { .expect("failed to run the Wasm preflight build"); assert!(status.success(), "the Wasm preflight build failed"); - let target = std::env::var_os("CARGO_TARGET_DIR") - .map(PathBuf::from) - .unwrap_or_else(|| workspace.join("target")); + let target = match std::env::var_os("CARGO_TARGET_DIR") { + Some(path) => { + let path = PathBuf::from(path); + if path.is_absolute() { + path + } else { + workspace.join(path) + } + } + None => workspace.join("target"), + }; let wasm = target.join("wasm32-unknown-unknown/release/lyracore_module.wasm"); let bytes = std::fs::read(&wasm).expect("the Wasm preflight output is missing"); - assert!(bytes.starts_with(b"\0asm"), "the frozen module is not Wasm"); - wasm + assert!(bytes.starts_with(b"\0asm"), "the built module is not Wasm"); + bytes } From 1c89458b850a3a043ccfa05a78c3cdcab35d0ab6 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Fri, 4 Sep 2026 23:45:35 +0200 Subject: [PATCH 5/8] test(module): keep private fixture copy simple --- module/tests/support/mod.rs | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/module/tests/support/mod.rs b/module/tests/support/mod.rs index da478230c..bee14f2df 100644 --- a/module/tests/support/mod.rs +++ b/module/tests/support/mod.rs @@ -1,7 +1,6 @@ use std::collections::BTreeMap; use std::ffi::OsString; use std::fs; -use std::io::Write; use std::net::{TcpListener, TcpStream}; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Output, Stdio}; @@ -81,25 +80,10 @@ impl Standalone { } /// Copy built Wasm bytes into this standalone's private directory and publish that copy. + #[allow(dead_code)] // Used by tests that build their own Wasm artifact. pub fn publish_module_bytes(&self, wasm: &[u8]) { let path = self.data_dir.join("published-module.wasm"); - let mut file = fs::OpenOptions::new() - .write(true) - .create_new(true) - .open(&path) - .expect("failed to create private Wasm artifact"); - file.write_all(wasm) - .expect("failed to copy private Wasm artifact"); - file.sync_all() - .expect("failed to sync private Wasm artifact"); - let mut permissions = file - .metadata() - .expect("failed to inspect private Wasm artifact") - .permissions(); - permissions.set_readonly(true); - fs::set_permissions(&path, permissions) - .expect("failed to make private Wasm artifact read-only"); - drop(file); + fs::write(&path, wasm).expect("failed to copy private Wasm artifact"); assert_success(self.command().args([ "publish", From c575c4dfdbff992b64b2e6b0dcfe232cddae9c76 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Sat, 5 Sep 2026 00:05:01 +0200 Subject: [PATCH 6/8] test(module): verify fatal loot gate outcomes --- module/tests/typed_taxi_contracts.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/module/tests/typed_taxi_contracts.rs b/module/tests/typed_taxi_contracts.rs index a80211939..49bb4222c 100644 --- a/module/tests/typed_taxi_contracts.rs +++ b/module/tests/typed_taxi_contracts.rs @@ -14,6 +14,7 @@ fn gateway_taxi_gates_keep_refusals_typed_and_invariants_fatal() { let wasm = build_module_bytes(); let standalone = Standalone::start("typed-taxi-contracts"); standalone.publish_module_bytes(&wasm); + assert_loot_boundary_failure(&standalone, ACTOR, "loot:boundary_operator_rejected"); standalone.assert_call("claim_operator", &[]); standalone.assert_call("debug_spawn_player_entity", &[ACTOR]); standalone.assert_sql( @@ -26,11 +27,11 @@ fn gateway_taxi_gates_keep_refusals_typed_and_invariants_fatal() { for (reducer, args, tag) in taxi_cases() { assert_refusal(&standalone, reducer, args, tag); } + assert_loot_boundary_failure(&standalone, MISSING_ACTOR, "loot:boundary_missing_actor"); for (reducer, args) in [ ("gw_trainer_buy", &[MISSING_ACTOR, "0", "0"][..]), ("gw_use_item", &[MISSING_ACTOR, "0"][..]), - ("gw_take_loot", &[MISSING_ACTOR, "0", "0"][..]), ("gw_group_leave", &[MISSING_ACTOR][..]), ("gw_add_friend", &[MISSING_ACTOR, "2"][..]), ] { @@ -125,6 +126,18 @@ fn taxi_cases() -> Vec<(&'static str, &'static [&'static str], &'static str)> { ] } +fn assert_loot_boundary_failure(standalone: &Standalone, actor: &str, tag: &str) { + for (reducer, args, refusal) in taxi_cases() { + if !refusal.starts_with("loot:") { + continue; + } + let mut args = args.to_vec(); + args[0] = actor; + let text = failed_text(reducer, standalone.call(reducer, &args)); + assert!(text.contains(tag), "{reducer} did not return {tag}: {text}"); + } +} + fn assert_refusal(standalone: &Standalone, reducer: &str, args: &[&str], tag: &str) { let text = failed_text(reducer, standalone.call(reducer, args)); assert!(text.contains(tag), "{reducer} did not return {tag}: {text}"); From 978dbd281be07a1fe1bb2e6036d6fdc2701fbeaa Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Sat, 5 Sep 2026 00:49:51 +0200 Subject: [PATCH 7/8] fix(group): reject dangling memberships --- module/src/group.rs | 63 +++++++++++++++------------- module/tests/typed_taxi_contracts.rs | 47 +++++++++++++++++---- 2 files changed, 73 insertions(+), 37 deletions(-) diff --git a/module/src/group.rs b/module/src/group.rs index f719ab59f..675765dd8 100644 --- a/module/src/group.rs +++ b/module/src/group.rs @@ -460,6 +460,29 @@ pub(crate) fn group_of(ctx: &ReducerContext, character_guid: u64) -> Option Result, GroupOpError> { + let Some(member) = group_of(ctx, character_guid) else { + return Ok(None); + }; + let group = ctx + .db + .game_group() + .group_id() + .find(member.group_id) + .ok_or_else(|| { + GroupOpError::Invariant(format!( + "member {character_guid} points to missing group {}", + member.group_id + )) + })?; + Ok(Some((member, group))) +} + /// The leader-authorization sequence shared by `invite_core_on` / `uninvite_on` / /// `set_loot_method_on`: resolve `guid`'s group membership, its `Group` row, and confirm /// `guid` actually IS that group's leader. [`GroupRefusal::NotInGroup`] if `guid` has no group at @@ -470,18 +493,7 @@ fn led_group_of( ctx: &ReducerContext, guid: u64, ) -> Result<(GroupMember, Group), GroupOpError> { - let m = group_of(ctx, guid).ok_or(GroupRefusal::NotInGroup)?; - let group = ctx - .db - .game_group() - .group_id() - .find(m.group_id) - .ok_or_else(|| { - GroupOpError::Invariant(format!( - "member {guid} points to missing group {}", - m.group_id - )) - })?; + let (m, group) = checked_group_membership(ctx, guid)?.ok_or(GroupRefusal::NotInGroup)?; if group.leader_guid != guid { return Err(GroupRefusal::NotLeader.into()); } @@ -575,7 +587,7 @@ fn invite_core_on( return Err(GroupRefusal::TargetOffline.into()); } } - if group_of(ctx, target_guid).is_some() { + if checked_group_membership(ctx, target_guid)?.is_some() { return Err(GroupRefusal::AlreadyInGroup.into()); } // The inviter having NO group yet is fine (they'll lead a brand-new one) — only propagate @@ -655,7 +667,7 @@ fn accept_invite_on( .next() .ok_or(GroupRefusal::NoPendingInvite)?; invites.id().delete(invite.id); - if group_of(ctx, acceptor_guid).is_some() { + if checked_group_membership(ctx, acceptor_guid)?.is_some() { return Err(GroupRefusal::AlreadyInGroup.into()); } let inviter_guid = invite.inviter_guid; @@ -677,23 +689,12 @@ fn accept_invite_on( Plane::RealmCore => None, }; let members = ctx.db.game_group_member(); - let group_id = match group_of(ctx, inviter_guid) { - Some(m) => { + let group_id = match checked_group_membership(ctx, inviter_guid)? { + Some((m, group)) => { // Re-run the invite-time leadership gate: the invite was issued when the inviter was // the leader (or ungrouped and about to lead). If they since joined a DIFFERENT group // as a plain member, honoring the stale invite would smuggle the acceptor into a group // whose leader never invited them. - let group = ctx - .db - .game_group() - .group_id() - .find(m.group_id) - .ok_or_else(|| { - GroupOpError::Invariant(format!( - "member {inviter_guid} points to missing group {}", - m.group_id - )) - })?; if group.leader_guid != inviter_guid { return Err(GroupRefusal::InviterUnavailable.into()); } @@ -783,7 +784,7 @@ pub(crate) fn leave_group_for(ctx: &ReducerContext, leaver_guid: u64) -> Result< } fn leave_group_on(ctx: &ReducerContext, leaver_guid: u64) -> Result<(), GroupOpError> { - if group_of(ctx, leaver_guid).is_none() { + if checked_group_membership(ctx, leaver_guid)?.is_none() { return Err(GroupRefusal::NotInGroup.into()); } remove_member(ctx, leaver_guid); @@ -810,7 +811,8 @@ fn uninvite_on( target_guid: u64, ) -> Result<(), GroupOpError> { let (m, _group) = led_group_of(ctx, leader_guid)?; - let target = group_of(ctx, target_guid).ok_or(GroupRefusal::TargetNotInGroup)?; + let (target, _target_group) = checked_group_membership(ctx, target_guid)? + .ok_or(GroupRefusal::TargetNotInGroup)?; if target.group_id != m.group_id { return Err(GroupRefusal::TargetNotInGroup.into()); } @@ -851,7 +853,8 @@ fn set_loot_method_on( return Err(GroupRefusal::InvalidLootRules.into()); } let resolved_master = if loot_setting == loot_method::MASTER { - let target = group_of(ctx, master_guid).ok_or(GroupRefusal::TargetNotInGroup)?; + let (target, _target_group) = checked_group_membership(ctx, master_guid)? + .ok_or(GroupRefusal::TargetNotInGroup)?; if target.group_id != m.group_id { return Err(GroupRefusal::TargetNotInGroup.into()); } diff --git a/module/tests/typed_taxi_contracts.rs b/module/tests/typed_taxi_contracts.rs index 49bb4222c..4665f45ab 100644 --- a/module/tests/typed_taxi_contracts.rs +++ b/module/tests/typed_taxi_contracts.rs @@ -46,20 +46,53 @@ fn gateway_taxi_gates_keep_refusals_typed_and_invariants_fatal() { standalone.assert_sql("DELETE FROM game_active_taxi_flight WHERE character_guid = 1"); standalone.assert_call("realm_group_op", &["0", "1", "2", "0", "0"]); standalone.assert_call("realm_group_op", &["1", "2", "0", "0", "0"]); - standalone.assert_sql("DELETE FROM game_group"); + standalone.assert_call("realm_group_op", &["0", "3", "4", "0", "0"]); + standalone.assert_call("realm_group_op", &["1", "4", "0", "0", "0"]); + standalone.assert_call("realm_group_op", &["0", "3", "5", "0", "0"]); + standalone.assert_sql("DELETE FROM game_group WHERE leader_guid = 3"); - let text = failed_text( - "realm_group_op", - standalone.call("realm_group_op", &["0", "1", "3", "0", "0"]), + let valid_group_before = standalone.query_rows("SELECT * FROM game_group WHERE leader_guid = 1"); + for args in [ + &["0", "1", "3", "0", "0"][..], + &["1", "5", "0", "0", "0"][..], + &["3", "3", "0", "0", "0"][..], + &["4", "1", "3", "0", "0"][..], + &["5", "1", "3", "2", "2"][..], + &["0", "3", "6", "0", "0"][..], + ] { + assert_group_invariant(&standalone, args); + } + assert_eq!( + standalone.query_rows("SELECT * FROM game_group WHERE leader_guid = 1"), + valid_group_before, + "failed membership checks must not change the valid Group" ); - assert!(text.contains("group invariant failed"), "{text}"); - assert!(!text.contains("group:database"), "{text}"); assert!( standalone .query_rows("SELECT * FROM game_group_invite WHERE target_guid = 3") .is_empty(), - "the failed invariant check must not leave a partial invite" + "a dangling target must not receive an invite" + ); + assert_eq!( + standalone + .query_rows("SELECT * FROM game_group_invite WHERE target_guid = 5") + .len(), + 1, + "a failed accept must roll its invite deletion back" ); + assert_eq!( + standalone + .query_rows("SELECT * FROM game_group_member WHERE character_guid = 3") + .len(), + 1, + "a failed leave or kick must preserve the dangling membership for repair" + ); +} + +fn assert_group_invariant(standalone: &Standalone, args: &[&str]) { + let text = failed_text("realm_group_op", standalone.call("realm_group_op", args)); + assert!(text.contains("group invariant failed"), "{args:?}: {text}"); + assert!(!text.contains("group:"), "{args:?}: {text}"); } fn taxi_cases() -> Vec<(&'static str, &'static [&'static str], &'static str)> { From a0c484db7f0c7caaec2929b53c5df0278f1891a1 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Sat, 5 Sep 2026 01:09:23 +0200 Subject: [PATCH 8/8] test(module): use Cargo reported Wasm artifact --- module/tests/typed_taxi_contracts.rs | 35 ++++++++++++++++------------ 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/module/tests/typed_taxi_contracts.rs b/module/tests/typed_taxi_contracts.rs index 4665f45ab..f786dc865 100644 --- a/module/tests/typed_taxi_contracts.rs +++ b/module/tests/typed_taxi_contracts.rs @@ -192,7 +192,7 @@ fn failed_text(reducer: &str, output: Output) -> String { fn build_module_bytes() -> Vec { let module_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR")); let workspace = module_dir.parent().unwrap(); - let status = Command::new("cargo") + let output = Command::new("cargo") .current_dir(workspace) .args([ "build", @@ -203,23 +203,28 @@ fn build_module_bytes() -> Vec { "-p", "lyracore-module", "--features=debug_reducers", + "--message-format=json-render-diagnostics", ]) - .status() + .output() .expect("failed to run the Wasm preflight build"); - assert!(status.success(), "the Wasm preflight build failed"); + assert!( + output.status.success(), + "the Wasm preflight build failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); - let target = match std::env::var_os("CARGO_TARGET_DIR") { - Some(path) => { - let path = PathBuf::from(path); - if path.is_absolute() { - path - } else { - workspace.join(path) - } - } - None => workspace.join("target"), - }; - let wasm = target.join("wasm32-unknown-unknown/release/lyracore_module.wasm"); + let wasm = String::from_utf8(output.stdout) + .expect("Cargo artifact output was not UTF-8") + .lines() + .filter_map(|line| serde_json::from_str::(line).ok()) + .filter(|message| message["reason"] == "compiler-artifact") + .filter(|message| message["target"]["name"] == "lyracore_module") + .filter_map(|message| message["filenames"].as_array().cloned()) + .flatten() + .filter_map(|filename| filename.as_str().map(PathBuf::from)) + .find(|path| path.extension().is_some_and(|extension| extension == "wasm")) + .expect("Cargo did not report the built Module Wasm artifact"); let bytes = std::fs::read(&wasm).expect("the Wasm preflight output is missing"); assert!(bytes.starts_with(b"\0asm"), "the built module is not Wasm"); bytes