From 84bf49a6846c72132f4c4d5a7991dca43f7e9763 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Thu, 3 Sep 2026 22:56:59 +0200 Subject: [PATCH 1/3] refactor(trainer): typed refusals across the tier contract The Module refused a trainer buy with a "[N] detail" string and the Gateway searched that text for "[1]" or "[2]". Any failure without a bracket, a reducer timeout included, reached the client as a gameplay refusal. One shared `TrainerRefusal` enum now defines the contract. The Module sends only the tag and keeps the human detail in its log. The Gateway parses the tag from a reducer the Module rejected; a timeout, transport, or SDK failure stays an error and ends the session. `TrainerBuyOutcome` holds the client mapping onto gtker's three `TrainingFailureReason` codes. Co-Authored-By: Claude Fable 5.1 --- crates/lyracore-shared/src/trainer.rs | 56 ++++++++++++ gateway/src/codec/trainer.rs | 37 ++------ gateway/src/stdb/reducers.rs | 64 ++++++++++++-- gateway/src/stdb/world_store.rs | 2 +- gateway/src/world/handlers/mod.rs | 2 +- gateway/src/world/handlers/trainer.rs | 52 +++++++---- gateway/src/world/mod.rs | 2 +- gateway/src/world/store.rs | 5 +- gateway/src/world/tests.rs | 75 +++++++++++++--- module/src/skill.rs | 6 +- module/src/trainer.rs | 120 +++++++++++++++----------- 11 files changed, 300 insertions(+), 121 deletions(-) diff --git a/crates/lyracore-shared/src/trainer.rs b/crates/lyracore-shared/src/trainer.rs index a352bf4a2..bd79ca634 100644 --- a/crates/lyracore-shared/src/trainer.rs +++ b/crates/lyracore-shared/src/trainer.rs @@ -27,6 +27,50 @@ pub mod trainer_type { /// it does not echo the offering's marker id back as a learned spell. pub const RIDING_SKILL_LINE: u32 = 762; +/// Why the Module refused a trainer 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 TrainerRefusal { + /// The trainer will not serve this interaction at all: missing, not a trainer, on another map, + /// out of range, refusing the buyer's class, or asked for riding while teaching something else. + Unavailable, + /// The trainer's offering list does not carry that spell. + NotOffered, + AlreadyKnown, + LevelTooLow, + /// A rank purchase whose `game_spell_chain` predecessor is not known yet. + PreviousRankMissing, + NotEnoughMoney, +} + +impl TrainerRefusal { + pub const ALL: [Self; 6] = [ + Self::Unavailable, + Self::NotOffered, + Self::AlreadyKnown, + Self::LevelTooLow, + Self::PreviousRankMissing, + Self::NotEnoughMoney, + ]; + + pub fn as_tag(self) -> &'static str { + match self { + Self::Unavailable => "trainer:unavailable", + Self::NotOffered => "trainer:not_offered", + Self::AlreadyKnown => "trainer:already_known", + Self::LevelTooLow => "trainer:level_too_low", + Self::PreviousRankMissing => "trainer:previous_rank_missing", + Self::NotEnoughMoney => "trainer:not_enough_money", + } + } + + pub fn parse_tag(tag: &str) -> Option { + Self::ALL + .into_iter() + .find(|refusal| refusal.as_tag() == tag) + } +} + /// Will this trainer serve a character of `player_class`? /// /// Refuses only when all three hold: the trainer is a `CLASS` trainer, its `trainer_class` is @@ -47,6 +91,18 @@ mod tests { /// Every class the 1.12 client can create. 6 and 10 do not exist in vanilla. const CLASSES: [u8; 9] = [1, 2, 3, 4, 5, 7, 8, 9, 11]; + #[test] + fn every_refusal_tag_round_trips() { + for refusal in TrainerRefusal::ALL { + assert_eq!(TrainerRefusal::parse_tag(refusal.as_tag()), Some(refusal)); + } + assert_eq!(TrainerRefusal::parse_tag("trainer:"), None); + assert_eq!( + TrainerRefusal::parse_tag("gw_trainer_buy reducer timed out after 10s"), + None + ); + } + #[test] fn a_class_trainer_serves_its_own_class_and_refuses_every_other() { for &trainer_class in &CLASSES { diff --git a/gateway/src/codec/trainer.rs b/gateway/src/codec/trainer.rs index 9dd4e2fc6..bc14a2911 100644 --- a/gateway/src/codec/trainer.rs +++ b/gateway/src/codec/trainer.rs @@ -76,20 +76,14 @@ pub fn build_trainer_buy_succeeded(trainer_guid: u64, spell_id: u32) -> SMSG_TRA } } -/// Build `SMSG_TRAINER_BUY_FAILED`. `reason` is the gtker `TrainingFailureReason` int the module tagged -/// onto its `Err` ([1]=money, [2]=level/req, else generic). gtker vanilla has only 3 reasons, so -/// already-known/range/flag failures fall to Unavailable — cosmetic, since the client gates the Learn -/// button on the Green state from the list. +/// Build `SMSG_TRAINER_BUY_FAILED`. The caller picks `error` from the Module's typed Refusal; gtker +/// vanilla carries only 3 reasons, so several Refusals share Unavailable — cosmetic, since the client +/// gates the Learn button on the Green state from the list. pub fn build_trainer_buy_failed( trainer_guid: u64, spell_id: u32, - reason: u32, + error: TrainingFailureReason, ) -> SMSG_TRAINER_BUY_FAILED { - let error = match reason { - 1 => TrainingFailureReason::NotEnoughMoney, - 2 => TrainingFailureReason::NotEnoughSkill, - _ => TrainingFailureReason::Unavailable, - }; SMSG_TRAINER_BUY_FAILED { guid: Guid::new(trainer_guid), id: spell_id, @@ -157,23 +151,10 @@ mod tests { } #[test] - fn trainer_buy_failed_maps_reason_code_to_the_closest_failure_reason() { - let money = build_trainer_buy_failed(42, 100, 1); - assert_eq!(money.error, TrainingFailureReason::NotEnoughMoney); - assert_eq!(money.guid, Guid::new(42)); - assert_eq!(money.id, 100); - let skill = build_trainer_buy_failed(42, 100, 2); - assert_eq!(skill.error, TrainingFailureReason::NotEnoughSkill); - // gtker vanilla has only 3 reasons; any other code (already-known/range/flag failures) - // degrades to Unavailable — cosmetic, since the client gates the Learn button on the list's - // Green state anyway. - for other in [0u32, 3, 99] { - let msg = build_trainer_buy_failed(42, 100, other); - assert_eq!( - msg.error, - TrainingFailureReason::Unavailable, - "reason {other} must degrade to Unavailable" - ); - } + fn trainer_buy_failed_addresses_the_trainer_and_the_offering() { + let msg = build_trainer_buy_failed(42, 100, TrainingFailureReason::NotEnoughMoney); + assert_eq!(msg.guid, Guid::new(42)); + assert_eq!(msg.id, 100); + assert_eq!(msg.error, TrainingFailureReason::NotEnoughMoney); } } diff --git a/gateway/src/stdb/reducers.rs b/gateway/src/stdb/reducers.rs index f64f901c5..6e112b2a6 100644 --- a/gateway/src/stdb/reducers.rs +++ b/gateway/src/stdb/reducers.rs @@ -12,6 +12,7 @@ use super::bindings::*; use super::connection::{call_reducer, recv_reducer_on, reducer_refusal_reason, Coordinator}; use super::views::entity_view; use lyracore_shared::auction::AuctionRefusal; +use lyracore_shared::trainer::TrainerRefusal; static NEXT_TAXI_REQUEST_ID: OnceLock = OnceLock::new(); @@ -1977,8 +1978,8 @@ impl Coordinator { } /// Learn `spell_id` from trainer `trainer_guid` (`CMSG_TRAINER_BUY_SPELL`) over the coordinator - /// connection. The module gates it (range / level / cost / not-already-known) and charges copper; - /// the `Err` message carries the module's `[N]` gtker failure-reason tag for the dispatch to forward. + /// connection. The module gates it (range / level / cost / not-already-known) and charges copper. + /// A Refusal the Module tagged comes back as an outcome; anything else stays an error. /// Rides the coordinator connection as `gw_trainer_buy`. pub fn buy_trainer_spell( &self, @@ -1986,16 +1987,23 @@ impl Coordinator { actor_guid: u64, trainer_guid: u64, spell_id: u32, - ) -> Result<()> { + ) -> Result { if actor_guid == 0 { return Err(anyhow!("buy_trainer_spell: actor_guid unresolved")); } let coord = self.0.call_pipe(); - call_reducer!( + let result: Result<()> = call_reducer!( coord.conn.reducers, "gw_trainer_buy", gw_trainer_buy_then(actor_guid, trainer_guid, spell_id) - ) + ); + match result { + Ok(()) => Ok(crate::world::TrainerBuyOutcome::Learned), + Err(error) => match trainer_refusal(&error) { + Some(refusal) => Ok(refusal.into()), + None => Err(error), + }, + } } /// Buy the next bank bag slot from `banker_guid` (`CMSG_BUY_BANK_SLOT`) over the coordinator @@ -3097,6 +3105,12 @@ fn auction_refusal(error: &anyhow::Error) -> Option { reducer_refusal_reason(error).and_then(AuctionRefusal::parse_tag) } +/// The Module's typed trainer Refusal. Only a reducer the Module rejected carries a tag; a timeout, +/// transport, or SDK failure stays an error with an unknown outcome. +fn trainer_refusal(error: &anyhow::Error) -> Option { + reducer_refusal_reason(error).and_then(TrainerRefusal::parse_tag) +} + fn bid_payload_matches(hold: &AuctionBidHold, decision: &AuctionBidDecision) -> bool { hold.operation_id == decision.operation_id && hold.bidder_guid == decision.bidder_guid @@ -3271,7 +3285,10 @@ mod auction_reducer_tests { fn refused_listing_binding_matches_the_generated_commit_listing_shape() { let expected = include_str!("bindings/realm_auction_commit_listing_reducer.rs") .replace("RealmAuctionCommitListing", "RealmAuctionRefundListing") - .replace("realm_auction_commit_listing", "realm_auction_refund_listing"); + .replace( + "realm_auction_commit_listing", + "realm_auction_refund_listing", + ); assert_eq!( include_str!("bindings/realm_auction_refund_listing_reducer.rs"), expected, @@ -3304,3 +3321,38 @@ mod auction_reducer_tests { ); } } + +#[cfg(test)] +mod trainer_reducer_tests { + use super::*; + use crate::stdb::connection::ReducerCallError; + + #[test] + fn only_a_rejected_reducer_carries_a_typed_refusal() { + for refusal in TrainerRefusal::ALL { + let error = anyhow::Error::from(ReducerCallError::Rejected { + operation: "gw_trainer_buy".to_string(), + reason: refusal.as_tag().to_string(), + }) + .context("trainer buy"); + assert_eq!(trainer_refusal(&error), Some(refusal)); + } + + let not_refusals = [ + anyhow::Error::from(ReducerCallError::fatal( + "gw_trainer_buy reducer timed out after 10s".to_string(), + )), + anyhow::Error::from(ReducerCallError::Rejected { + operation: "gw_trainer_buy".to_string(), + reason: "operator only".to_string(), + }), + anyhow!( + "wrapped text that mentions {}", + TrainerRefusal::AlreadyKnown.as_tag() + ), + ]; + for error in not_refusals { + assert_eq!(trainer_refusal(&error), None, "{error:#}"); + } + } +} diff --git a/gateway/src/stdb/world_store.rs b/gateway/src/stdb/world_store.rs index 0b075fd77..86af51a9c 100644 --- a/gateway/src/stdb/world_store.rs +++ b/gateway/src/stdb/world_store.rs @@ -627,7 +627,7 @@ impl WorldStore for Coordinator { self_guid: u64, trainer_guid: u64, spell_id: u32, - ) -> Result<()> { + ) -> Result { self.buy_trainer_spell(account_id, self_guid, trainer_guid, spell_id) } diff --git a/gateway/src/world/handlers/mod.rs b/gateway/src/world/handlers/mod.rs index 6bac0da73..cac925923 100644 --- a/gateway/src/world/handlers/mod.rs +++ b/gateway/src/world/handlers/mod.rs @@ -61,7 +61,7 @@ pub(crate) use taxi::{ dispatch_taxi_action, queue_reply_then_arm, TaxiActionOutcome, TaxiActionPlayer, TaxiActionStore, }; pub(crate) use trade::handle_trade; -pub(crate) use trainer::handle_trainer; +pub(crate) use trainer::{handle_trainer, TrainerBuyOutcome}; pub(crate) use vendor::{ dispatch_vendor_action, VendorActionOutcome, VendorActionPlayer, VendorActionStore, }; diff --git a/gateway/src/world/handlers/trainer.rs b/gateway/src/world/handlers/trainer.rs index 6e4b284c8..ceea7641d 100644 --- a/gateway/src/world/handlers/trainer.rs +++ b/gateway/src/world/handlers/trainer.rs @@ -2,13 +2,38 @@ //! `world/mod.rs`. use super::super::*; +use lyracore_shared::trainer::TrainerRefusal; +use wow_world_messages::vanilla::TrainingFailureReason; + +/// How the Module answered a trainer purchase. A Refusal is a gameplay answer the client can render; +/// a failed Durable Request is not, so it never reaches here. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) enum TrainerBuyOutcome { + Learned, + Refused(TrainingFailureReason), +} + +// gtker vanilla carries three failure reasons, so several gameplay Refusals share one. The client +// pre-gates the Learn button on the list's Green state, which is what keeps the collapse cosmetic. +impl From for TrainerBuyOutcome { + fn from(refusal: TrainerRefusal) -> Self { + Self::Refused(match refusal { + TrainerRefusal::NotEnoughMoney => TrainingFailureReason::NotEnoughMoney, + TrainerRefusal::LevelTooLow | TrainerRefusal::PreviousRankMissing => { + TrainingFailureReason::NotEnoughSkill + } + TrainerRefusal::Unavailable + | TrainerRefusal::NotOffered + | TrainerRefusal::AlreadyKnown => TrainingFailureReason::Unavailable, + }) + } +} /// Class-trainer family: open the trainer window (`CMSG_TRAINER_LIST` → `SMSG_TRAINER_LIST`, each spell /// Green/Red/Gray) and learn a spell (`CMSG_TRAINER_BUY_SPELL` → the module buy → /// `SMSG_TRAINER_BUY_*` + a live `SMSG_LEARNED_SPELL` so it hits the action bar without a relog). /// Needs the in-world player guid (a trainer is only clicked in-world); in CharSelect the opcodes -/// pass through. A buy rejection is per-action — surfaced as `SMSG_TRAINER_BUY_FAILED` (reason -/// parsed from the module's `[N]` tag). +/// pass through. A buy Refusal is per-action — surfaced as `SMSG_TRAINER_BUY_FAILED`. pub(crate) fn handle_trainer( tx: &SessionTx, store: &St, @@ -50,8 +75,10 @@ pub(crate) fn handle_trainer( ClientOpcodeMessage::CMSG_TRAINER_BUY_SPELL(c) => { let trainer_guid = c.guid.guid(); let spell_id = c.id; - match store.buy_trainer_spell(conn.account_id, self_guid, trainer_guid, spell_id) { - Ok(()) => { + // A Refusal arrives as an outcome. An error leaves the durable result unknown, so it + // ends the session instead of posing as a gameplay answer. + match store.buy_trainer_spell(conn.account_id, self_guid, trainer_guid, spell_id)? { + TrainerBuyOutcome::Learned => { // Confirm + push the spell live so it appears on the action bar without a relog. send( tx, @@ -110,18 +137,9 @@ pub(crate) fn handle_trainer( send_armor_proficiency(tx, store, self_guid)?; } } - Err(e) => { - // The module tags its Err with a [N] gtker failure-reason; 1=money, 2=level/req, else generic. - let es = e.to_string(); - let reason = if es.contains("[1]") { - 1 - } else if es.contains("[2]") { - 2 - } else { - 0 - }; + TrainerBuyOutcome::Refused(reason) => { log::debug!( - "world: buy_trainer_spell failed (account {}): {es}", + "world: trainer buy refused (account {}): {reason:?}", conn.account_id ); send( @@ -160,7 +178,9 @@ pub(crate) fn handle_trainer( ClientOpcodeMessage::CMSG_SET_FACTION_ATWAR(c) => { let reputation_index = c.faction.as_int() as u32; let at_war = c.flags.is_at_war(); - if let Err(e) = store.set_faction_at_war(conn.account_id, self_guid, reputation_index, at_war) { + if let Err(e) = + store.set_faction_at_war(conn.account_id, self_guid, reputation_index, at_war) + { log::debug!( "world: set_faction_at_war ignored (account {}): {e}", conn.account_id diff --git a/gateway/src/world/mod.rs b/gateway/src/world/mod.rs index f18490ace..5b2db3b60 100644 --- a/gateway/src/world/mod.rs +++ b/gateway/src/world/mod.rs @@ -62,7 +62,7 @@ use handlers::{ }; pub(crate) use handlers::{ zone_weather_message, AuctionBrowseRequest, AuctionPage, AuctionQuery, CreateAuctionOutcome, - CreateAuctionRequest, PlaceBidOutcome, PlaceBidRequest, WeatherStore, + CreateAuctionRequest, PlaceBidOutcome, PlaceBidRequest, TrainerBuyOutcome, WeatherStore, }; use login_queue::{Admission, LoginQueue}; use social::handle_social; diff --git a/gateway/src/world/store.rs b/gateway/src/world/store.rs index 2f9f12c01..2ec689e6d 100644 --- a/gateway/src/world/store.rs +++ b/gateway/src/world/store.rs @@ -428,14 +428,15 @@ pub trait WorldStore: ) -> Result>; /// Buy/learn `spell_id` from trainer `trainer_guid` (`CMSG_TRAINER_BUY_SPELL`). The module gates it - /// (range / level / cost / not-already-known); `Err` carries a `[N]` gtker failure-reason tag. + /// (range / level / cost / not-already-known) and a Refusal comes back as an outcome; `Err` means + /// the durable result is unknown. fn buy_trainer_spell( &self, account_id: u64, self_guid: u64, trainer_guid: u64, spell_id: u32, - ) -> Result<()>; + ) -> Result; /// The skill line a trainer offering teaches (`game_trainer_spell.learn_skill_line`), or 0 for an /// ordinary spell offering. The buy path reads it to tell a SKILL purchase from a SPELL purchase: diff --git a/gateway/src/world/tests.rs b/gateway/src/world/tests.rs index e7d9e6834..389e4801d 100644 --- a/gateway/src/world/tests.rs +++ b/gateway/src/world/tests.rs @@ -292,6 +292,8 @@ struct InMemoryStore { trainer_refuses_class: bool, /// When set, buy/sell return this error (a gameplay failure) instead of `Ok`. trade_error: Option, + /// When set, the trainer buy answers this Refusal instead of learning the spell. + trainer_buy_refusal: Option, /// Quest-giver evals returned by `giver_quest_evals` (the menu/status input). quest_evals: Vec, /// Quest details `quest_detail_view(id)` resolves from (matched by `quest_id`). @@ -1836,10 +1838,13 @@ impl WorldStore for InMemoryStore { _self_guid: u64, _trainer_guid: u64, _spell_id: u32, - ) -> Result<()> { + ) -> Result { + if let Some(refusal) = self.trainer_buy_refusal { + return Ok(refusal.into()); + } match &self.trade_error { Some(e) => Err(anyhow!("{e}")), - None => Ok(()), + None => Ok(crate::world::TrainerBuyOutcome::Learned), } } fn talent_grant_spell(&self, _talent_id: u32) -> u32 { @@ -7611,15 +7616,20 @@ fn trainer_buy_rank_upgrade_supersedes_the_previous_rank_spell() { } #[test] -fn trainer_buy_failure_parses_the_reason_tag_into_the_failure_code() { - // The module tags its Err with gtker's `[N]` reason: [1]=money, [2]=level/req, else generic. - for (err, want) in [ - ("too poor [1]", TrainingFailureReason::NotEnoughMoney), - ("level too low [2]", TrainingFailureReason::NotEnoughSkill), - ("some other rejection", TrainingFailureReason::Unavailable), - ] { +fn every_module_refusal_has_one_client_failure_reason() { + use lyracore_shared::trainer::TrainerRefusal; + let expected = |refusal| match refusal { + TrainerRefusal::NotEnoughMoney => TrainingFailureReason::NotEnoughMoney, + TrainerRefusal::LevelTooLow | TrainerRefusal::PreviousRankMissing => { + TrainingFailureReason::NotEnoughSkill + } + TrainerRefusal::Unavailable | TrainerRefusal::NotOffered | TrainerRefusal::AlreadyKnown => { + TrainingFailureReason::Unavailable + } + }; + for refusal in TrainerRefusal::ALL { let mut s = quest_store(); - s.trade_error = Some(err.into()); + s.trainer_buy_refusal = Some(refusal); let store = std::sync::Arc::new(s); let (mut client, mut c_enc, mut c_dec, server) = enter_world(store, 1); CMSG_TRAINER_BUY_SPELL { @@ -7630,7 +7640,7 @@ fn trainer_buy_failure_parses_the_reason_tag_into_the_failure_code() { .unwrap(); match ServerOpcodeMessage::read_encrypted(&mut client, &mut c_dec).unwrap() { ServerOpcodeMessage::SMSG_TRAINER_BUY_FAILED(m) => { - assert_eq!(m.error, want, "store error {err:?} must map to {want:?}"); + assert_eq!(m.error, expected(refusal), "{refusal:?}"); assert_eq!(m.id, 1234); } other => panic!("expected SMSG_TRAINER_BUY_FAILED, got {other}"), @@ -7640,6 +7650,49 @@ fn trainer_buy_failure_parses_the_reason_tag_into_the_failure_code() { } } +/// A reducer timeout leaves the durable result unknown, so it must not reach the client as a +/// gameplay Refusal that says the purchase did not happen. +#[test] +fn a_trainer_reducer_timeout_is_not_answered_as_a_refusal() { + let store = std::sync::Arc::new(InMemoryStore { + login_entity: Some(warrior_entity()), + trade_error: Some("gw_trainer_buy reducer timed out after 10s".into()), + ..tester_store(7) + }); + 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_TRAINER_BUY_SPELL { + guid: Guid::new(70), + id: 1234, + } + .write_encrypted_client(&mut client, &mut c_enc) + .unwrap(); + + let error = result_rx + .recv_timeout(std::time::Duration::from_secs(1)) + .expect("an unknown buy outcome must end the session promptly") + .expect_err("a timed-out trainer reducer must be session-fatal"); + assert!(format!("{error:#}").contains("timed out")); + assert!( + ServerOpcodeMessage::read_encrypted(&mut client, &mut c_dec).is_err(), + "the socket closes instead of claiming the purchase failed" + ); +} + #[test] fn learn_talent_with_a_grant_spell_pushes_learned_spell() { // An ability talent (grant_spell_id != 0) + a successful learn → SMSG_LEARNED_SPELL(grant) so diff --git a/module/src/skill.rs b/module/src/skill.rs index f31ba9743..3220b8772 100644 --- a/module/src/skill.rs +++ b/module/src/skill.rs @@ -888,8 +888,8 @@ const LEARN_POLEARM_SPELL_ID: u32 = 50131; // -> learn_skill_line = POLEARM (229 /// trainer's own guid is a u64 > 2^53 the CLI `spacetime call` mangles), then funnels through the SAME /// `trainer::apply_trainer_buy` — exercising the identical range/flag gate, `trainer_buy_check`, and (via /// `is_combat_skill_line`) the WEAPON fork's level-derived cap + row-presence known-check the CMSG-routed -/// buy would. The trainer must offer that marker (the seeded fixture SQL) or the buy returns "[0] trainer -/// does not teach that spell". +/// buy would. The trainer must offer that marker (the seeded fixture SQL) or the buy is refused as +/// `trainer:not_offered`. #[cfg(feature = "debug_reducers")] #[spacetimedb::reducer] pub fn debug_learn_weapon_from_trainer( @@ -962,7 +962,7 @@ pub fn debug_learn_riding( /// `spacetime call` mangles), then funnels through the SAME `trainer::apply_trainer_buy` — exercising the /// identical range/flag gate, the MOUNTS-trainer check, `trainer_buy_check`, and the `learn_riding` grant /// the CMSG-routed buy would. The trainer must offer that marker (the seeded riding-trainer fixture) or -/// the buy returns "[0] trainer does not teach that spell". +/// the buy is refused as `trainer:not_offered`. #[cfg(feature = "debug_reducers")] #[spacetimedb::reducer] pub fn debug_learn_riding_from_trainer( diff --git a/module/src/trainer.rs b/module/src/trainer.rs index 2c382f5f3..e3ddb2078 100644 --- a/module/src/trainer.rs +++ b/module/src/trainer.rs @@ -8,11 +8,11 @@ //! //! Deliberate simplification: the `game_trainer_spell` row carries only spell_id/cost/required_level — //! class spells need no required_skill / prerequisite-spell chain (those are the profession/talent -//! cases; add the columns when professions land). The SMSG_TRAINER_BUY_FAILED reason is best-effort: -//! gtker vanilla's `TrainingFailureReason` has only Unavailable(0)/NotEnoughMoney(1)/NotEnoughSkill(2), so "already -//! known" maps to Unavailable and "level too low" to NotEnoughSkill (cosmetic — the client pre-gates the -//! Learn button on the Green state from the list). [entity] +//! cases; add the columns when professions land). A refused buy names its reason with a +//! [`TrainerRefusal`]; the Gateway owns the mapping onto gtker vanilla's three +//! `TrainingFailureReason` codes. [entity] +use lyracore_shared::trainer::TrainerRefusal; use spacetimedb::{table, ReducerContext}; use crate::{ @@ -26,11 +26,12 @@ use crate::{ /// constant rather than forking a second magic number that could drift from this one. pub(crate) const TRAINER_RANGE_SQ: f32 = 100.0; -// Failure codes — chosen to equal gtker `TrainingFailureReason::as_int` so the gateway can forward them -// verbatim into `SMSG_TRAINER_BUY_FAILED.error` (Unavailable=0, NotEnoughMoney=1, NotEnoughSkill=2). -const FAIL_ALREADY_KNOWN: u32 = 0; // Unavailable — gtker has no AlreadyKnown variant -const FAIL_NOT_ENOUGH_MONEY: u32 = 1; -const FAIL_LEVEL_TOO_LOW: u32 = 2; // NotEnoughSkill — gtker has no LevelTooLow variant (closest "req not met") +/// Reducer edge: only the tag crosses to the gateway; the detail stays in module logs. +fn refused(refusal: TrainerRefusal, detail: &str) -> String { + let tag = refusal.as_tag(); + spacetimedb::log::info!("trainer refused {tag}: {detail}"); + tag.to_string() +} /// One spell a trainer teaches. Static, public, SQL-loadable (no Timestamp), keyed by the trainer's /// CREATURE TEMPLATE entry (every spawned trainer of that entry offers the same list — like vendors). @@ -104,24 +105,23 @@ pub(crate) fn rank_prereq_met(prev_spell: u32, knows_prev: bool) -> bool { } /// Pure purchase gate — decides whether `level`/`money` may learn a spell costing `cost` at `required_level`. -/// `Ok(())` to allow; `Err(code)` with the gtker `TrainingFailureReason` int to reject. Checked in priority -/// order (already-known first, then level, then money) so the player gets the most relevant message. Pure → -/// unit-tested. +/// Checked in priority order (already-known first, then level, then money) so the player gets the most +/// relevant message. Pure → unit-tested. pub(crate) fn trainer_buy_check( known: bool, level: u32, required_level: u32, money: u32, cost: u32, -) -> Result<(), u32> { +) -> Result<(), TrainerRefusal> { if known { - return Err(FAIL_ALREADY_KNOWN); + return Err(TrainerRefusal::AlreadyKnown); } if level < required_level { - return Err(FAIL_LEVEL_TOO_LOW); + return Err(TrainerRefusal::LevelTooLow); } if money < cost { - return Err(FAIL_NOT_ENOUGH_MONEY); + return Err(TrainerRefusal::NotEnoughMoney); } Ok(()) } @@ -165,9 +165,9 @@ pub(crate) fn resolve_learn_target(ctx: &ReducerContext, spell_id: u32) -> u32 { /// which is trainer-gated identically — its own comment used to read "Same gates as apply_trainer_buy"). /// Returns the resolved trainer entity. /// -/// Bare error strings (no gtker `[N]` tag): `apply_trainer_buy` prefixes its own `"[0] "` tag on top so -/// its wire-visible `SMSG_TRAINER_BUY_FAILED` text is unchanged; `do_reset_talents` is untagged (its -/// respec path isn't wired to a gtker failure-reason opcode), also unchanged. +/// The `Err` is a bare human detail, not a Refusal tag: `apply_trainer_buy` turns it into a +/// [`TrainerRefusal::Unavailable`] and logs the detail, while `do_reset_talents` keeps the plain +/// text its respec path already returns. pub(crate) fn validate_trainer_interaction( ctx: &ReducerContext, caster: &WorldEntity, @@ -185,7 +185,7 @@ pub(crate) fn validate_trainer_interaction( if trainer.map_id != caster.map_id || trainer.instance_id != caster.instance_id { return Err("trainer on another map".to_string()); } - if crate::helpers::dist_sq(&caster, &trainer) > TRAINER_RANGE_SQ { + if crate::helpers::dist_sq(caster, &trainer) > TRAINER_RANGE_SQ { return Err("trainer out of range".to_string()); } // The class gate sits at this shared chokepoint so one guard closes both wrong-class training @@ -206,10 +206,8 @@ pub(crate) fn validate_trainer_interaction( } /// Shared buy logic for the player + debug paths: validate `trainer` is a real in-range trainer, that it -/// teaches `spell_id`, run [`trainer_buy_check`], then charge copper + learn the spell. `Err(String)` names -/// the reason; the leading "[N] " tag is the gtker failure code the gateway forwards into -/// `SMSG_TRAINER_BUY_FAILED`. Range/flag failures are not a TrainerSpell-list state (the client wouldn't -/// have shown the trainer), so they tag Unavailable(0). [entity] +/// teaches `spell_id`, run [`trainer_buy_check`], then charge copper + learn the spell. This is the +/// reducer edge, so a Refusal leaves only its [`TrainerRefusal`] tag in the `Err`. [entity] pub(crate) fn apply_trainer_buy( ctx: &ReducerContext, caster_guid: u64, @@ -220,9 +218,9 @@ pub(crate) fn apply_trainer_buy( let mut caster = entities .guid() .find(caster_guid) - .ok_or_else(|| "[0] buyer not in world".to_string())?; - let trainer = - validate_trainer_interaction(ctx, &caster, trainer_guid).map_err(|e| format!("[0] {e}"))?; + .ok_or_else(|| refused(TrainerRefusal::Unavailable, "buyer not in world"))?; + let trainer = validate_trainer_interaction(ctx, &caster, trainer_guid) + .map_err(|detail| refused(TrainerRefusal::Unavailable, &detail))?; // The trainer must actually teach this spell (its template's list). The spawned trainer carries its // creature-template `entry`; the list is keyed by that. let offered = ctx @@ -231,12 +229,17 @@ pub(crate) fn apply_trainer_buy( .by_trainer() .filter(&trainer.entry) .find(|s| s.spell_id == spell_id) - .ok_or_else(|| "[0] trainer does not teach that spell".to_string())?; + .ok_or_else(|| { + refused( + TrainerRefusal::NotOffered, + "trainer does not teach that spell", + ) + })?; // PROFESSION-LEARN BRANCH (professions slice 3): a flagged offering teaches a SKILL, not a spell — // it never resolves a wrapper/rank, never casts, never touches `game_player_spell`. `known` mirrors // `learn_profession`'s presence check (a `game_player_skill` row for that line → a re-buy is the - // idempotent FAIL_ALREADY_KNOWN no-op), and on Ok it grants the skill at 1/75. The `learn_skill_line == 0` + // idempotent already-known no-op), and on Ok it grants the skill at 1/75. The `learn_skill_line == 0` // arm below is the EXISTING spell path verbatim (byte-identical → no class-spell regression). // // WEAPON-MASTER FORK (work-item 202): the SAME `learn_skill_line` column shape also carries weapon @@ -271,7 +274,10 @@ pub(crate) fn apply_trainer_buy( .find(trainer.entry) .is_some_and(|t| t.trainer_type != lyracore_shared::trainer::trainer_type::MOUNTS) { - return Err("[0] that trainer does not teach riding".to_string()); + return Err(refused( + TrainerRefusal::Unavailable, + "that trainer does not teach riding", + )); } // "Known" is the same TIER comparison a profession uses: Apprentice-75 has not met Journeyman-150, // so the second tier is buyable exactly once and a re-buy of the tier you hold is refused. @@ -328,19 +334,19 @@ pub(crate) fn apply_trainer_buy( // `game_spell_chain` row must have the PREVIOUS rank already known (vanilla refuses training // "Fireball Rank 3" while only Rank 1 is known). NO chain row (an un-imported spell, or a rank with // nothing before it) → the gate passes, BYTE-IDENTICAL to before this work item (the 178/212 - // precedent: missing imported data never blocks a purchase that used to succeed). Reuses - // FAIL_LEVEL_TOO_LOW — there is no dedicated "prerequisite not met" code in gtker's 3-value - // `TrainingFailureReason`, and its own doc already calls it the closest "req not met" fit (see the - // const above). Skipped when the spell is ALREADY KNOWN so `trainer_buy_check`'s "already-known - // first" message-priority contract (see its doc) holds even for a rank granted out of order by a - // debug lever or bot kit — the buy is rejected either way; only the message differs (102 review - // finding). + // precedent: missing imported data never blocks a purchase that used to succeed). Skipped when the + // spell is ALREADY KNOWN so `trainer_buy_check`'s "already-known first" message-priority contract + // (see its doc) holds even for a rank granted out of order by a debug lever or bot kit — the buy is + // rejected either way; only the message differs (102 review finding). if !known { if let BuyGrant::Spell(to_learn) = grant { if let Some(chain) = ctx.db.game_spell_chain().spell_id().find(to_learn) { let knows_prev = crate::spell::knows_spell(ctx, caster_guid, chain.prev_spell); if !rank_prereq_met(chain.prev_spell, knows_prev) { - return Err(format!("[{FAIL_LEVEL_TOO_LOW}] requires the previous rank")); + return Err(refused( + TrainerRefusal::PreviousRankMissing, + "requires the previous rank", + )); } } } @@ -373,12 +379,19 @@ pub(crate) fn apply_trainer_buy( } Ok(()) } - Err(FAIL_ALREADY_KNOWN) => Err(format!("[{FAIL_ALREADY_KNOWN}] already known")), - Err(FAIL_LEVEL_TOO_LOW) => Err(format!( - "[{FAIL_LEVEL_TOO_LOW}] requires level {}", - offered.required_level + Err(TrainerRefusal::AlreadyKnown) => { + Err(refused(TrainerRefusal::AlreadyKnown, "already known")) + } + Err(TrainerRefusal::LevelTooLow) => Err(refused( + TrainerRefusal::LevelTooLow, + &format!("requires level {}", offered.required_level), + )), + Err(TrainerRefusal::NotEnoughMoney) => Err(refused( + TrainerRefusal::NotEnoughMoney, + &format!("not enough money (need {})", offered.cost), )), - Err(code) => Err(format!("[{code}] not enough money (need {})", offered.cost)), + // `trainer_buy_check` returns no other Refusal; the arm keeps the match exhaustive. + Err(refusal) => Err(refused(refusal, "purchase gate refused the buy")), } } @@ -413,20 +426,20 @@ mod tests { #[test] fn trainer_buy_check_gates_in_priority_order() { - // already known → Unavailable(0), even if everything else is fine + // already known wins even when everything else is fine assert_eq!( trainer_buy_check(true, 10, 1, 1000, 10), - Err(FAIL_ALREADY_KNOWN) + Err(TrainerRefusal::AlreadyKnown) ); - // level too low → NotEnoughSkill(2) + // then the level gate assert_eq!( trainer_buy_check(false, 2, 6, 1000, 10), - Err(FAIL_LEVEL_TOO_LOW) + Err(TrainerRefusal::LevelTooLow) ); - // not enough money → NotEnoughMoney(1) + // then the money gate assert_eq!( trainer_buy_check(false, 10, 1, 5, 10), - Err(FAIL_NOT_ENOUGH_MONEY) + Err(TrainerRefusal::NotEnoughMoney) ); // all good → Ok assert_eq!(trainer_buy_check(false, 10, 6, 100, 10), Ok(())); @@ -471,7 +484,7 @@ mod tests { // Present (seeded by ensure_player_skills at character creation) -> already-known, rejected. assert_eq!( trainer_buy_check(true, 40, 1, 1000, 100), - Err(FAIL_ALREADY_KNOWN) + Err(TrainerRefusal::AlreadyKnown) ); // Absent (never seeded/learned) -> proceeds regardless of what a stale cap column might say. assert_eq!(trainer_buy_check(false, 40, 1, 1000, 100), Ok(())); @@ -530,13 +543,16 @@ mod tests { /// THE IDEMPOTENT RE-LEARN (professions slice 3): the profession buy keys its already-known gate on the /// PRESENCE of a `game_player_skill` row for the line (the `known` flag the buy path derives), and feeds /// it through the SAME `trainer_buy_check`. So a re-buy of an already-learned profession is rejected with - /// FAIL_ALREADY_KNOWN (no re-charge, no duplicate/reset row) — identical to a re-bought known spell. + /// `TrainerRefusal::AlreadyKnown` (no re-charge, no duplicate/reset row) — identical to a re-bought known spell. #[test] fn profession_rebuy_is_rejected_as_already_known() { // First learn: no skill row yet (known=false), cost 0, level 1 → the gate ALLOWS the grant. assert_eq!(trainer_buy_check(false, 1, 1, 0, 0), Ok(())); // Re-buy: the skill row now exists (known=true) → rejected as already-known, even though cost is 0 // and the level is fine (the idempotent no-op — the row is never reset or duplicated). - assert_eq!(trainer_buy_check(true, 1, 1, 0, 0), Err(FAIL_ALREADY_KNOWN)); + assert_eq!( + trainer_buy_check(true, 1, 1, 0, 0), + Err(TrainerRefusal::AlreadyKnown) + ); } } From af90e6920f6a8182c6dfb91df85c24c072d35e71 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Fri, 4 Sep 2026 23:05:14 +0200 Subject: [PATCH 2/3] fix(trainer): classify taxi flight as a refusal --- module/src/gw.rs | 17 ++++++++++++++++- module/src/trainer.rs | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/module/src/gw.rs b/module/src/gw.rs index 4f72edbce..0b428532d 100644 --- a/module/src/gw.rs +++ b/module/src/gw.rs @@ -470,6 +470,21 @@ pub fn gw_use_gameobject( } /// [`crate::actor::trainer_buy`] behind the gateway gate. +/// +/// Taxi flight is an expected trainer Refusal. Missing actors still carry the generic actor error, +/// which the Gateway treats as a failure with an unknown result. +fn trainer_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::trainer::refused( + lyracore_shared::trainer::TrainerRefusal::Unavailable, + "buyer is in taxi flight", + )); + } + Ok(actor) +} + #[reducer] pub fn gw_trainer_buy( ctx: &ReducerContext, @@ -478,7 +493,7 @@ pub fn gw_trainer_buy( spell_id: u32, ) -> Result<(), String> { require_operator(ctx)?; - actor(ctx, actor_guid)?; + trainer_actor(ctx, actor_guid)?; crate::actor::trainer_buy(ctx, actor_guid, trainer_guid, spell_id) } diff --git a/module/src/trainer.rs b/module/src/trainer.rs index e3ddb2078..dda21f50f 100644 --- a/module/src/trainer.rs +++ b/module/src/trainer.rs @@ -27,7 +27,7 @@ use crate::{ pub(crate) const TRAINER_RANGE_SQ: f32 = 100.0; /// Reducer edge: only the tag crosses to the gateway; the detail stays in module logs. -fn refused(refusal: TrainerRefusal, detail: &str) -> String { +pub(crate) fn refused(refusal: TrainerRefusal, detail: &str) -> String { let tag = refusal.as_tag(); spacetimedb::log::info!("trainer refused {tag}: {detail}"); tag.to_string() From c4017713798b01d3bc399005ed362fbb3ff2e55a Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Fri, 4 Sep 2026 23:09:42 +0200 Subject: [PATCH 3/3] docs(trainer): attach the Gateway Verb contract to its reducer --- module/src/gw.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/module/src/gw.rs b/module/src/gw.rs index 0b428532d..2182f9f0c 100644 --- a/module/src/gw.rs +++ b/module/src/gw.rs @@ -469,8 +469,6 @@ pub fn gw_use_gameobject( crate::actor::use_gameobject(ctx, actor_guid, go_guid) } -/// [`crate::actor::trainer_buy`] behind the gateway gate. -/// /// Taxi flight is an expected trainer Refusal. Missing actors still carry the generic actor error, /// which the Gateway treats as a failure with an unknown result. fn trainer_actor(ctx: &ReducerContext, actor_guid: u64) -> Result { @@ -485,6 +483,7 @@ fn trainer_actor(ctx: &ReducerContext, actor_guid: u64) -> Result