From be32ec29771ef3c2889bcfea78c687dcac1e9196 Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 11:25:49 +0000 Subject: [PATCH 1/9] feat(nav): retain bot route outcomes and coverage evidence --- crates/lyracore-shared/src/nav.rs | 152 ++++++++++++++-- module/src/nav.rs | 289 ++++++++++++++++++++++++++---- 2 files changed, 389 insertions(+), 52 deletions(-) diff --git a/crates/lyracore-shared/src/nav.rs b/crates/lyracore-shared/src/nav.rs index f8f53502f..9abd1bfb7 100644 --- a/crates/lyracore-shared/src/nav.rs +++ b/crates/lyracore-shared/src/nav.rs @@ -342,7 +342,27 @@ pub fn find_leg_ex( to: (f32, f32), max_expansions: u32, ) -> Option<(Vec<(f32, f32)>, u32, bool)> { - search_leg(fetch, from, to, 0.0, max_expansions) + let search = search_leg(fetch, from, to, 0.0, max_expansions); + match search.outcome { + LegOutcome::Complete(path) => Some((path, search.expansions, true)), + LegOutcome::Partial(path) => Some((path, search.expansions, false)), + LegOutcome::Blocked => None, + } +} + +/// Planning success is distinct from arrival. Partial legs retain safe progress toward the goal. +#[derive(Clone, Debug, PartialEq)] +pub enum LegOutcome { + Complete(Vec<(f32, f32)>), + Partial(Vec<(f32, f32)>), + Blocked, +} + +/// Search work is retained even when no progress is possible. +#[derive(Clone, Debug, PartialEq)] +pub struct LegSearch { + pub outcome: LegOutcome, + pub expansions: u32, } /// Find a leg to a walkable point within the caller's stopping distance of the target. @@ -354,26 +374,58 @@ pub fn find_leg_in_range( stop_dist: f32, max_expansions: u32, ) -> Option> { - search_leg(fetch, from, to, stop_dist.max(0.0), max_expansions).map(|(path, _, _)| path) + match find_leg_in_range_ex(fetch, from, to, stop_dist, max_expansions).outcome { + LegOutcome::Complete(path) | LegOutcome::Partial(path) => Some(path), + LegOutcome::Blocked => None, + } +} + +/// Find a range-limited leg without discarding completeness or failed-search work. +pub fn find_leg_in_range_ex( + fetch: &mut impl FnMut(u16, u16) -> Option, + from: (f32, f32), + to: (f32, f32), + stop_dist: f32, + max_expansions: u32, +) -> LegSearch { + if !stop_dist.is_finite() { + return LegSearch { + outcome: LegOutcome::Blocked, + expansions: 0, + }; + } + search_leg(fetch, from, to, stop_dist.max(0.0), max_expansions) } -#[allow(clippy::type_complexity)] fn search_leg( fetch: &mut impl FnMut(u16, u16) -> Option, from: (f32, f32), to: (f32, f32), stop_dist: f32, max_expansions: u32, -) -> Option<(Vec<(f32, f32)>, u32, bool)> { - let (sx, sy) = (grid_coord(from.0)?, grid_coord(from.1)?); - let (tx, ty) = (grid_coord(to.0)?, grid_coord(to.1)?); +) -> LegSearch { + let blocked = |expansions| LegSearch { + outcome: LegOutcome::Blocked, + expansions, + }; + let (Some(sx), Some(sy), Some(tx), Some(ty)) = ( + grid_coord(from.0), + grid_coord(from.1), + grid_coord(to.0), + grid_coord(to.1), + ) else { + return blocked(0); + }; if !stop_dist.is_finite() { - return None; + return blocked(0); } let mut cache = Cache::new(fetch); let distance = (to.0 - from.0).hypot(to.1 - from.1); if distance <= stop_dist { - return Some((vec![from], 0, true)); + return LegSearch { + outcome: LegOutcome::Complete(vec![from]), + expansions: 0, + }; } let direct = if stop_dist > 0.0 { let scale = (distance - stop_dist) / distance; @@ -385,11 +437,14 @@ fn search_leg( to }; if line_walkable(&mut cache, from, direct) { - return Some((vec![direct], 0, true)); + return LegSearch { + outcome: LegOutcome::Complete(vec![direct]), + expansions: 0, + }; } let target_walkable = grid_walkable(&mut cache, tx, ty); if stop_dist == 0.0 && !target_walkable { - return None; + return blocked(0); } // A* with octile heuristic, integer costs (10 straight / 14 diagonal), corner-cut guard. use std::cmp::Reverse; @@ -420,10 +475,10 @@ fn search_leg( best.0 = (x, y); break; } - expanded += 1; - if expanded > max_expansions { + if expanded == max_expansions { break; // budget exhausted — fall through to the best-effort partial path } + expanded += 1; let hxy = h(x, y); if hxy < best.1 { best = ((x, y), hxy); @@ -467,7 +522,7 @@ fn search_leg( best.0 } else { if best.0 == (sx, sy) { - return None; // zero progress possible (walled-in start) — caller falls back + return blocked(expanded); } best.0 }; @@ -510,7 +565,14 @@ fn search_leg( path.pop(); path.push(to); } - Some((path, expanded, found)) + LegSearch { + outcome: if found { + LegOutcome::Complete(path) + } else { + LegOutcome::Partial(path) + }, + expansions: expanded, + } } #[cfg(test)] @@ -640,6 +702,68 @@ mod runtime_tests { assert!(complete); } + #[test] + fn range_search_completes_before_a_blocked_target() { + let from = at(10, 50); + let to = at(32, 50); + let search = find_leg_in_range_ex(&mut fetcher(), from, to, 4.0, 4096); + let LegOutcome::Complete(path) = search.outcome else { + panic!("the stopping distance ends before the wall"); + }; + assert_eq!(search.expansions, 0); + let endpoint = *path.last().unwrap(); + assert!(((endpoint.0 - to.0).hypot(endpoint.1 - to.1) - 4.0).abs() < 0.01); + assert_ne!(endpoint, from); + } + + #[test] + fn range_search_retains_safe_partial_progress_at_the_expansion_cap() { + let from = at(50, 8); + let to = at(8, 8); + let search = find_leg_in_range_ex(&mut fetcher(), from, to, 0.0, 32); + let LegOutcome::Partial(path) = search.outcome else { + panic!("the sealed pocket permits only partial progress"); + }; + assert_eq!(search.expansions, 32); + assert_ne!(path.last().copied(), Some(from)); + let mut fetch = fetcher(); + let mut cache = Cache::new(&mut fetch); + let mut previous = from; + for point in path { + assert!(line_walkable(&mut cache, previous, point)); + previous = point; + } + } + + #[test] + fn blocked_search_retains_expansions_without_a_straight_fallback() { + let ((cx, cy), mut cell) = walled_cell(); + cell.walk.fill(0); + walk_set(&mut cell.walk, 10, 10, true); + walk_set(&mut cell.walk, 50, 50, true); + let mut fetch = |x, y| (x == cx && y == cy).then(|| cell.clone()); + let search = find_leg_in_range_ex(&mut fetch, at(10, 10), at(50, 50), 0.0, 4096); + assert_eq!(search.outcome, LegOutcome::Blocked); + assert_eq!(search.expansions, 1); + } + + #[test] + fn zero_search_budget_never_expands_a_node() { + let search = find_leg_in_range_ex(&mut fetcher(), at(10, 50), at(54, 50), 0.0, 0); + assert_eq!(search.outcome, LegOutcome::Blocked); + assert_eq!(search.expansions, 0); + } + + #[test] + fn nonfinite_stopping_distance_does_not_produce_a_route() { + for distance in [f32::NAN, f32::INFINITY, -f32::INFINITY] { + let search = + find_leg_in_range_ex(&mut fetcher(), at(10, 50), at(20, 50), distance, 4096); + assert_eq!(search.outcome, LegOutcome::Blocked); + assert_eq!(search.expansions, 0); + } + } + #[test] fn start_inside_a_blocked_margin_cell_still_fast_paths() { // A chaser standing IN an unwalkable margin cell (movement isn't walkability-gated) diff --git a/module/src/nav.rs b/module/src/nav.rs index 75806a40b..42da68a36 100644 --- a/module/src/nav.rs +++ b/module/src/nav.rs @@ -152,7 +152,7 @@ pub fn import_nav_chunks_append(ctx: &ReducerContext, packed: String) -> Result< // Runtime consumption (work-item 243) — the module-side wrappers over `lyracore_shared::nav`'s // pure queries, gated on `game_config.nav_enabled` (default OFF; the 244 benchmark + live // verify flips it). Every wrapper degrades to the pre-243 straight-line behavior when the -// flag is off, the chunk is missing, or no path exists. +// flag is off or a chunk is missing. `route_step` holds position when no path exists. // =========================================================================================== use crate::game_config; @@ -205,32 +205,47 @@ fn merged_cell(terrain: Option, coverage: Option) -> O /// (the same hoist `vmap::fetcher` does with its active-generation lookup), never per cell. fn fetcher(ctx: &ReducerContext, map_id: u32) -> impl FnMut(u16, u16) -> Option + '_ { let generation = coverage_generation(ctx, map_id); - move |cx, cy| { - let key = cell_key(map_id, cx, cy); - let terrain = ctx - .db - .game_nav_chunk() - .key() - .find(key) - .map(|c| NavCellData { - base_z: c.base_z, - walk: c.walk, - obs: c.obs, - }); - let coverage = generation.and_then(|generation_id| { - ctx.db - .game_vmap_nav_coverage() - .by_generation_cell() - .filter((generation_id, key)) - .next() - .map(|c| NavCellData { - base_z: c.base_z, - walk: c.walk, - obs: c.obs, - }) + move |cx, cy| fetch_cell(ctx, map_id, generation, cx, cy).0 +} + +fn fetch_cell( + ctx: &ReducerContext, + map_id: u32, + generation: Option, + cx: u16, + cy: u16, +) -> (Option, bool) { + let key = cell_key(map_id, cx, cy); + let terrain = ctx + .db + .game_nav_chunk() + .key() + .find(key) + .map(|c| NavCellData { + base_z: c.base_z, + walk: c.walk, + obs: c.obs, }); - merged_cell(terrain, coverage) - } + let coverage = generation.and_then(|generation_id| { + ctx.db + .game_vmap_nav_coverage() + .by_generation_cell() + .filter((generation_id, key)) + .next() + }); + let matching = coverage.as_ref().is_some_and(|c| { + c.map_id == map_id + && c.cell_x == cx + && c.cell_y == cy + && c.walk.len() == WALK_BYTES + && (c.obs.is_empty() || c.obs.len() == OBS_BYTES) + }); + let coverage = coverage.map(|c| NavCellData { + base_z: c.base_z, + walk: c.walk, + obs: c.obs, + }); + (merged_cell(terrain, coverage), matching) } /// Closed doors apply even where static geometry falls back to the coarse navigation grid. @@ -255,6 +270,145 @@ pub fn has_los( /// not solve the zone. 244 owns the measured tuning. const LEG_MAX_EXPANSIONS: u32 = 4096; +/// Complete and Partial describe the planned route, including when collision stops this step. +#[derive(spacetimedb::SpacetimeType, Clone, Debug, PartialEq, Eq)] +pub enum RouteStatus { + Complete, + Partial, + Blocked, + /// Navigation is disabled. The collision Gate still applies. + Direct, +} + +#[derive(spacetimedb::SpacetimeType, Clone, Debug, PartialEq, Eq)] +pub enum CoverageEvidence { + Unknown, + /// Every consulted navigation cell has a matching derived row from the active generation's + /// finalized manifest. This does not certify terrain coverage or the world import scope. + VerifiedCells(VerifiedRouteCells), +} + +#[derive(spacetimedb::SpacetimeType, Clone, Debug, PartialEq, Eq)] +pub struct VerifiedRouteCells { + pub generation_id: u64, + pub checked_cells: u32, +} + +#[derive(spacetimedb::SpacetimeType, Clone, Copy, Debug, PartialEq)] +pub struct RoutePoint { + pub x: f32, + pub y: f32, +} + +impl From<(f32, f32)> for RoutePoint { + fn from((x, y): (f32, f32)) -> Self { + Self { x, y } + } +} + +#[derive(spacetimedb::SpacetimeType, Clone, Debug, PartialEq)] +pub struct RouteClip { + pub attempted: RoutePoint, + pub hit: RoutePoint, +} + +/// `endpoint == from` means no movement. Complete means a route was planned, not arrival. +#[derive(spacetimedb::SpacetimeType, Clone, Debug, PartialEq)] +pub struct RouteStep { + pub from: RoutePoint, + pub endpoint: RoutePoint, + pub first_waypoint: Option, + pub status: RouteStatus, + pub expansions: u32, + pub clipping: Option, + pub coverage: CoverageEvidence, +} + +/// Plan one bot movement step. A failed search holds position; sparse missing rows retain +/// unknown coverage. Collision can shorten complete, partial, and direct steps independently. +#[allow(clippy::too_many_arguments)] // A movement step carries its partition, endpoints and distances. +pub fn route_step( + ctx: &ReducerContext, + map_id: u32, + instance_id: u64, + cur: (f32, f32), + dest: (f32, f32), + max_step: f32, + stop_dist: f32, + z: f32, +) -> RouteStep { + let mut result = RouteStep { + from: cur.into(), + endpoint: cur.into(), + first_waypoint: None, + status: RouteStatus::Blocked, + expansions: 0, + clipping: None, + coverage: CoverageEvidence::Unknown, + }; + if ![cur.0, cur.1, dest.0, dest.1, max_step, stop_dist, z] + .iter() + .all(|value| value.is_finite()) + || max_step < 0.0 + || stop_dist < 0.0 + || [cur.0, cur.1, dest.0, dest.1] + .iter() + .any(|&value| cell_index(value).is_none()) + { + return result; + } + let enabled = nav_enabled(ctx); + let generation = enabled.then(|| coverage_generation(ctx, map_id)).flatten(); + let mut checked = std::collections::BTreeSet::new(); + let mut all_covered = generation.is_some(); + let mut fetch = |cx, cy| { + let (cell, covered) = fetch_cell(ctx, map_id, generation, cx, cy); + checked.insert((cx, cy)); + all_covered &= covered; + cell + }; + let attempted = if enabled { + // The search exempts its starting sub-cell and may finish without fetching it. + if let (Some(cx), Some(cy)) = (cell_index(cur.0), cell_index(cur.1)) { + fetch(cx, cy); + } + let search = + nav::find_leg_in_range_ex(&mut fetch, cur, dest, stop_dist, LEG_MAX_EXPANSIONS); + result.expansions = search.expansions; + let path = match search.outcome { + nav::LegOutcome::Complete(path) => { + result.status = RouteStatus::Complete; + path + } + nav::LegOutcome::Partial(path) => { + result.status = RouteStatus::Partial; + path + } + nav::LegOutcome::Blocked => Vec::new(), + }; + result.first_waypoint = path.first().copied().map(RoutePoint::from); + result.first_waypoint.map_or(cur, |wp| { + crate::creatures::chase_step(cur.0, cur.1, wp.x, wp.y, max_step, 0.0) + }) + } else { + result.status = RouteStatus::Direct; + crate::creatures::chase_step(cur.0, cur.1, dest.0, dest.1, max_step, stop_dist) + }; + let (endpoint, clipping) = + step_gate_with_fetch(ctx, map_id, instance_id, cur, attempted, z, &mut fetch); + result.endpoint = endpoint.into(); + result.clipping = clipping; + if all_covered && !checked.is_empty() { + if let Some(generation_id) = generation { + result.coverage = CoverageEvidence::VerifiedCells(VerifiedRouteCells { + generation_id, + checked_cells: checked.len() as u32, + }); + } + } + result +} + /// Steps toward a walkable path's first waypoint, then gates every result against geometry. /// A missing path may aim straight at the goal, but the commit gate stops it at obstructions. /// @@ -311,16 +465,28 @@ fn step_gate( stepped: (f32, f32), z: f32, ) -> (f32, f32) { + let mut fetch = None; + step_gate_with_fetch(ctx, map_id, instance_id, cur, stepped, z, &mut |cx, cy| { + fetch.get_or_insert_with(|| fetcher(ctx, map_id))(cx, cy) + }) + .0 +} + +fn step_gate_with_fetch( + ctx: &ReducerContext, + map_id: u32, + instance_id: u64, + cur: (f32, f32), + stepped: (f32, f32), + z: f32, + fetch: &mut impl FnMut(u16, u16) -> Option, +) -> ((f32, f32), Option) { gate_step(cur, stepped, z, |from, to| { let exact = crate::vmap::collision_ray(ctx, map_id, instance_id, from, to); let grid = if !crate::vmap::vmap_enabled(ctx, map_id) && nav_enabled(ctx) { // The grid query adds eye height itself, so it starts at foot height. - lyracore_shared::nav::step_hit( - &mut fetcher(ctx, map_id), - (from[0], from[1], z), - (to[0], to[1], z), - ) - .map(|p| [p.0, p.1, from[2]]) + lyracore_shared::nav::step_hit(fetch, (from[0], from[1], z), (to[0], to[1], z)) + .map(|p| [p.0, p.1, from[2]]) } else { None }; @@ -333,9 +499,9 @@ fn gate_step( stepped: (f32, f32), z: f32, mut collision: impl FnMut([f32; 3], [f32; 3]) -> Option<(f32, f32)>, -) -> (f32, f32) { +) -> ((f32, f32), Option) { if stepped == cur { - return stepped; + return (stepped, None); } // Probe above walkable steps. A ray at foot height stops on every stair riser. let probe_z = z + lyracore_shared::nav::WALK_STEP_UP; @@ -345,16 +511,23 @@ fn gate_step( let (dx, dy) = (hx - cur.0, hy - cur.1); let hit_dist = (dx * dx + dy * dy).sqrt(); let land_dist = hit_dist - GATE_CLEARANCE_YD; - if land_dist <= 0.0 { + let endpoint = if land_dist <= 0.0 { cur // hit inside the clearance margin — hold in place } else { ( cur.0 + dx / hit_dist * land_dist, cur.1 + dy / hit_dist * land_dist, ) - } + }; + ( + endpoint, + Some(RouteClip { + attempted: stepped.into(), + hit: (hx, hy).into(), + }), + ) } - None => stepped, // clear (or no data under this segment) — the plain step stands + None => (stepped, None), } } @@ -397,6 +570,46 @@ mod tests { use super::*; use lyracore_shared::nav::{walk_get, walk_set, OBS_BYTES, OBS_NONE, WALK_BYTES}; + #[test] + fn collision_evidence_retains_the_attempt_when_the_step_cannot_advance() { + let from = (0.0, 0.0); + let attempted = (5.0, 0.0); + let hit = (0.5, 0.0); + let (endpoint, clipping) = gate_step(from, attempted, 80.0, |_, _| Some(hit)); + assert_eq!(endpoint, from); + assert_eq!( + clipping, + Some(RouteClip { + attempted: attempted.into(), + hit: hit.into() + }) + ); + } + + #[test] + fn collision_evidence_retains_progress_before_the_hit() { + let from = (0.0, 0.0); + let attempted = (5.0, 0.0); + let hit = (3.0, 0.0); + let (endpoint, clipping) = gate_step(from, attempted, 80.0, |_, _| Some(hit)); + assert_eq!(endpoint, (2.0, 0.0)); + assert_eq!( + clipping, + Some(RouteClip { + attempted: attempted.into(), + hit: hit.into() + }) + ); + } + + #[test] + fn an_unclipped_step_has_no_collision_evidence() { + let attempted = (5.0, 0.0); + let (endpoint, clipping) = gate_step((0.0, 0.0), attempted, 80.0, |_, _| None); + assert_eq!(endpoint, attempted); + assert_eq!(clipping, None); + } + #[test] fn movement_steps_over_a_low_riser_but_stops_before_a_wall() { use lyracore_shared::vmap::{cast_ray, RayFlavor, TriClass, VmapTri}; @@ -428,7 +641,7 @@ mod tests { }, }, ]; - let stepped = gate_step(cur, dest, 80.0, |from, to| { + let (stepped, _) = gate_step(cur, dest, 80.0, |from, to| { cast_ray( &mut |_, _| Some(tris.clone()), from, From 6d35ce9d76bba67367f486be6f7b731b56919e79 Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 11:45:55 +0000 Subject: [PATCH 2/9] feat(actor): expose cast lifecycle and typed action refusals --- crates/lyracore-package-delta/src/script.rs | 1 + module/build.rs | 6 + module/src/actor.rs | 85 +++++++---- module/src/combat/engage.rs | 73 +++++++-- module/src/hooks.rs | 9 ++ module/src/items/ops.rs | 41 ++++- module/src/mount.rs | 4 +- module/src/quest.rs | 99 ++++++++---- module/src/spell/cast/resolve.rs | 158 +++++++++++++------- module/src/spell/control.rs | 53 ++++++- module/src/spell/mod.rs | 2 + module/src/spell/outcome.rs | 135 +++++++++++++++++ module/src/spell/scheduler.rs | 20 ++- 13 files changed, 560 insertions(+), 126 deletions(-) create mode 100644 module/src/spell/outcome.rs diff --git a/crates/lyracore-package-delta/src/script.rs b/crates/lyracore-package-delta/src/script.rs index 1a7779906..c88861f9d 100644 --- a/crates/lyracore-package-delta/src/script.rs +++ b/crates/lyracore-package-delta/src/script.rs @@ -97,6 +97,7 @@ pub const HOOK_EVENT_NAMES: &[&str] = &[ "on_kill", "on_aggro", "on_cast_resolved", + "on_cast_finished", "on_loot", "on_quest_accept", "on_quest_turnin", diff --git a/module/build.rs b/module/build.rs index d05c83a12..6c02e2b0f 100644 --- a/module/build.rs +++ b/module/build.rs @@ -179,6 +179,12 @@ const HOOK_EVENTS: &[HookEvent] = &[ "payload.caster_guid", "payload.target_guid", ), + hook( + "on_cast_finished", + "crate::hooks::CastFinishedPayload", + "payload.caster_guid", + "payload.target_guid", + ), // The corpse is a loot container, not a live entity, so `event.target` is normally absent here. hook( "on_loot", diff --git a/module/src/actor.rs b/module/src/actor.rs index 7d91c62eb..dd2553a23 100644 --- a/module/src/actor.rs +++ b/module/src/actor.rs @@ -3,23 +3,17 @@ //! playerbots brains, and the future Tier-2 Lua host API — consumes the SAME verbs the //! player reducers do, with identical gates. //! -//! Design rules (all enforced by construction — every verb IS its core, re-exported): -//! - **No behavior**: each verb is a `pub(crate) use` rename of the existing core; the gates -//! (range, money, validation, CC, faction) live in the cores and CANNOT drift here. The two -//! exceptions below (`cast_at`, `stop_attack`) are shape adapters only and say so. -//! - **Uniform shape**: `fn verb(ctx, actor_guid, ...) -> Result<(), String>` — the actor is -//! always the first arg after ctx, by guid (never identity; sender resolution stays in the -//! player reducers). Cores resolve every OTHER participant by guid too, so no caller ever -//! passes a row. -//! - **NOT authorization**: these are reducer-internal fns. Callers are already inside a reducer -//! transaction that authorized the action (a debug reducer, a bot tick pass, a hook handler). +//! Existing verbs retain their `Result<(), String>` contract. Typed requests expose accepted +//! work and pending cast identity without changing the client operations. All Gates remain +//! in the operation that owns them. Callers authorize the Actor before entering these functions. //! //! | verb | core | gate semantics (unchanged, documented here for consumers) | //! |------|------|------------------------------------------------------------| //! | `attack` | `combat::apply_start_attack` | CC-blocked rejected; no self/corpse/cross-map; friendly (green) target rejected when faction data exists; re-arm retargets | //! | `ranged_attack` | `combat::apply_start_ranged_attack` | `attack` gates + ranged weapon equipped (slot 17) | //! | `stop_attack` | `combat::stop_attack_for` | unconditional disarm of the actor's outgoing melee row | -//! | `cast_at` | `spell::resolve_cast_at` | the full cast core: CC/dead/range/cost/GCD/cooldown gates; level sourced from the live entity | +//! | `cast_at` | `spell::request_cast` | normal cast lifecycle; an existing timed cast waits; level comes from the live entity | +//! | `request_cast` | `spell::request_cast` | typed start, waiting, and Refusal; completion uses `on_cast_finished` | //! | `accept_quest` | `quest::apply_accept_quest` | alive + giver in range offering the quest + level/race/class/prereq/duplicate gates | //! | `stage_quest` | `quest::grant_quest_unchecked` | HARNESS/BOT staging: same row shape, all accept gates SKIPPED (giver-less) | //! | `turn_in_quest` | `quest::apply_turn_in_quest` | alive + giver in range ending the quest + objectives complete; rewards atomic | @@ -81,6 +75,8 @@ macro_rules! package_only { } package_only! { pub(crate) use crate::combat::apply_start_attack as attack; } +package_only! { pub(crate) use crate::combat::request_attack as request_attack; } +package_only! { pub(crate) use crate::spell::request_cast as request_cast; } debug_only! { pub(crate) use crate::combat::apply_start_ranged_attack as ranged_attack; } /// Disarm the actor's outgoing auto-attack (melee or ranged). Shape adapter ONLY: the core returns @@ -91,34 +87,27 @@ pub(crate) fn stop_attack(ctx: &ReducerContext, actor_guid: u64) -> Result<(), S Ok(()) } -/// Resolve a cast from the actor AT `target_guid` through the single cast core (instant packet -/// shape — `is_completion = false`, matching `debug_cast_at`). Shape adapter ONLY: sources the -/// caster level from the live entity, exactly like the player path, then delegates. -#[cfg_attr(not(has_packages), allow(dead_code))] // package-only consumer — see `package_only!` +/// Compatibility adapter for callers that only need acceptance. Use `request_cast` to retain +/// the scheduled identity and observe completion through `on_cast_finished`. +#[allow(dead_code)] // Retained Package API v1 operation. pub(crate) fn cast_at( ctx: &ReducerContext, actor_guid: u64, spell_id: u32, target_guid: u64, ) -> Result<(), String> { - let e = crate::helpers::live_entity(ctx, actor_guid)?; - crate::spell::resolve_cast_at( - ctx, - actor_guid, - spell_id, - e.level as u8, - target_guid, - false, - false, - None, - ) + crate::spell::request_cast(ctx, actor_guid, spell_id, target_guid) + .map(|_| ()) + .map_err(Into::into) } // ---- quests ---- package_only! { pub(crate) use crate::quest::apply_accept_quest as accept_quest; + pub(crate) use crate::quest::request_accept_quest as request_accept_quest; pub(crate) use crate::quest::apply_turn_in_quest as turn_in_quest; + pub(crate) use crate::quest::request_turn_in_quest as request_turn_in_quest; } debug_only! { pub(crate) use crate::quest::grant_quest_unchecked as stage_quest; } @@ -156,3 +145,47 @@ package_only! { pub(crate) use crate::chat::emit_system_message as system_message; pub(crate) use crate::group::accept_invite_for as accept_group_invite; } + +/// A Refusal classified at the operation's Gate. Detail preserves existing client messages. +#[derive(spacetimedb::SpacetimeType, Clone, Debug, PartialEq, Eq)] +pub struct ActionRefusal { + pub kind: ActionRefusalKind, + pub detail: String, +} + +#[derive(spacetimedb::SpacetimeType, Clone, Copy, Debug, PartialEq, Eq)] +pub enum ActionRefusalKind { + MissingActor, + MissingTarget, + DeadActor, + DeadTarget, + CannotAct, + OtherPartition, + OutOfRange, + InventoryFull, + Other, +} + +impl ActionRefusal { + pub(crate) fn new(kind: ActionRefusalKind, detail: impl Into) -> Self { + Self { + kind, + detail: detail.into(), + } + } +} +impl From for ActionRefusal { + fn from(detail: String) -> Self { + Self::new(ActionRefusalKind::Other, detail) + } +} +impl From for String { + fn from(reason: ActionRefusal) -> Self { + reason.detail + } +} +impl std::fmt::Display for ActionRefusal { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.detail.fmt(f) + } +} diff --git a/module/src/combat/engage.rs b/module/src/combat/engage.rs index 9c79da1ed..ce21ba2f6 100644 --- a/module/src/combat/engage.rs +++ b/module/src/combat/engage.rs @@ -18,6 +18,7 @@ use crate::{game_faction_template, game_world_entity, WorldEntity}; // `RangedImpactSchedule` tables' `scheduled(..)` macros below to resolve, since those two reducers are // defined in `swing.rs` (mirrors `spell::tables`'s identical cross-file `scheduled(..)` pattern). use super::*; +use crate::actor::{ActionRefusal, ActionRefusalKind}; // --- Engagement queries over `game_melee_attack` (the single source of truth for who fights whom). // `attacker_guid` is the PK; an engagement "touches" a unit when it is on EITHER side. These three @@ -508,23 +509,29 @@ fn validate_attack_target( ctx: &ReducerContext, attacker: &WorldEntity, target_guid: u64, -) -> Result { - let target = - crate::helpers::live_entity(ctx, target_guid).map_err(|_| "no such target".to_string())?; +) -> Result { + let target = crate::helpers::live_entity(ctx, target_guid) + .map_err(|_| ActionRefusal::new(ActionRefusalKind::MissingTarget, "no such target"))?; if target.map_id != attacker.map_id || target.instance_id != attacker.instance_id { - return Err("target on another map".to_string()); + return Err(ActionRefusal::new( + ActionRefusalKind::OtherPartition, + "target on another map", + )); } if target.dead { // Can't attack a corpse during decay. The gateway maps this exact error to // SMSG_ATTACKSWING_DEADTARGET so the client leaves combat stance (shared constant). - return Err(lyracore_shared::ERR_ATTACK_TARGET_DEAD.to_string()); + return Err(ActionRefusal::new( + ActionRefusalKind::DeadTarget, + lyracore_shared::ERR_ATTACK_TARGET_DEAD, + )); } // Faction gate: reject a swing only at a FRIENDLY target. Hostile (red) AND neutral (yellow — // e.g. Elwynn wolves, which are huntable) stay attackable, matching vanilla; only friendly (green) // units are protected. The gateway maps this to SMSG_ATTACKSWING_CANT_ATTACK so the client leaves // stance. SKIPPED when faction data isn't loaded (table empty) so missing data never blocks combat. if ctx.db.game_faction_template().count() > 0 && !may_harm(ctx, attacker, &target) { - return Err(lyracore_shared::ERR_ATTACK_FRIENDLY.to_string()); + return Err(lyracore_shared::ERR_ATTACK_FRIENDLY.to_string().into()); } Ok(target) } @@ -536,8 +543,36 @@ pub(crate) fn apply_start_attack( attacker_guid: u64, target_guid: u64, ) -> Result<(), String> { - let attacker = crate::helpers::live_entity(ctx, attacker_guid) - .map_err(|_| "attacker not in world".to_string())?; + start_attack(ctx, attacker_guid, target_guid, false) + .map(|_| ()) + .map_err(Into::into) +} + +#[derive(spacetimedb::SpacetimeType, Clone, Copy, Debug, PartialEq, Eq)] +pub enum AttackStart { + Armed, + AlreadyArmed, +} + +/// Arm melee without resetting a current swing timer. Acceptance does not imply a hit. +#[cfg_attr(not(has_packages), allow(dead_code))] +pub(crate) fn request_attack( + ctx: &ReducerContext, + attacker_guid: u64, + target_guid: u64, +) -> Result { + start_attack(ctx, attacker_guid, target_guid, true) +} + +fn start_attack( + ctx: &ReducerContext, + attacker_guid: u64, + target_guid: u64, + retain_current: bool, +) -> Result { + let attacker = crate::helpers::live_entity(ctx, attacker_guid).map_err(|_| { + ActionRefusal::new(ActionRefusalKind::MissingActor, "attacker not in world") + })?; // Crowd control: an ACTION-blocked attacker (stunned/polymorphed/feared) cannot ENTER combat — // arming an engagement is itself an action. This is the player-command twin of the per-swing gate in // `tick_melee` (without it a CC'd player could insert a `game_melee_attack` row whose swings are then @@ -547,13 +582,13 @@ pub(crate) fn apply_start_attack( // match the gateway's desync classifier, so it just rejects the command rather than dropping the // session.) if crate::spell::is_action_blocked(ctx, attacker.guid) { - return Err(format!( - "attacker {} cannot act (stun/poly/fear)", - attacker.guid + return Err(ActionRefusal::new( + ActionRefusalKind::CannotAct, + format!("attacker {} cannot act (stun/poly/fear)", attacker.guid), )); } if target_guid == attacker.guid { - return Err("cannot attack self".to_string()); + return Err("cannot attack self".to_string().into()); } validate_attack_target(ctx, &attacker, target_guid)?; @@ -561,10 +596,20 @@ pub(crate) fn apply_start_attack( // engagement is armed. Every rejection above returned already, so an invalid attack packet leaves // the mount up. No-op for an unmounted attacker, and idempotent on a re-target. crate::mount::dismount(ctx, attacker.guid); + if retain_current + && ctx + .db + .game_melee_attack() + .attacker_guid() + .find(attacker_guid) + .is_some_and(|attack| attack.target_guid == target_guid && attack.ranged_spell_id == 0) + { + return Ok(AttackStart::AlreadyArmed); + } if !attacker.is_player() { if arm_creature_engagement(ctx, attacker.guid, target_guid, false) { - return Ok(()); + return Ok(AttackStart::Armed); } if let Some(mut creature) = ctx.db.game_world_entity().guid().find(attacker.guid) { if creature.target_guid != target_guid { @@ -592,7 +637,7 @@ pub(crate) fn apply_start_attack( } else { melee.insert(row); } - Ok(()) + Ok(AttackStart::Armed) } /// Shared core: arm `attacker_guid`'s RANGED auto-attack on `target_guid` with `spell_id`. Same gates as diff --git a/module/src/hooks.rs b/module/src/hooks.rs index d1df07c76..08d14f640 100644 --- a/module/src/hooks.rs +++ b/module/src/hooks.rs @@ -125,6 +125,15 @@ pub struct CastResolvedPayload { pub target_guid: u64, } +/// One scheduled cast ended. The scheduled identity distinguishes successive casts of one spell. +/// The pending row has been removed before handlers run. +pub struct CastFinishedPayload { + pub caster_guid: u64, + pub target_guid: u64, + pub scheduled_id: u64, + pub outcome: crate::spell::CastFinish, +} + /// A unit LOOTED one item stack from a corpse/gameobject loot window (`items::apply_take_loot` /// success — the player reducer and `debug_take_loot` both route here). Money pickup is a separate /// path and does not fire this. diff --git a/module/src/items/ops.rs b/module/src/items/ops.rs index 31e97f8b7..f232fea86 100644 --- a/module/src/items/ops.rs +++ b/module/src/items/ops.rs @@ -4,6 +4,7 @@ //! reducer and its debug twin (see `reducers.rs`). All effects are additive: they touch only the item //! rows + the actor's health/money. +use crate::actor::{ActionRefusal, ActionRefusalKind}; use spacetimedb::{Identity, ReducerContext, Table}; use lyracore_shared::constants::starter_item; @@ -176,6 +177,16 @@ pub(crate) fn grant_item_property( count: u32, property: Option, ) -> Result<(), String> { + request_grant_item(ctx, player_guid, item_entry, count, property).map_err(Into::into) +} + +pub(crate) fn request_grant_item( + ctx: &ReducerContext, + player_guid: u64, + item_entry: u32, + count: u32, + property: Option, +) -> Result<(), ActionRefusal> { let player = crate::helpers::live_entity(ctx, player_guid) .map_err(|_| "player not in world".to_string())?; let tmpl = ctx @@ -184,7 +195,7 @@ pub(crate) fn grant_item_property( .entry() .find(item_entry) .ok_or_else(|| format!("no such item {item_entry}"))?; - store_item( + store_item_typed( ctx, player_guid, player.owner_identity, @@ -203,10 +214,31 @@ pub(crate) fn store_item( player_guid: u64, owner_identity: spacetimedb::Identity, tmpl: &ItemTemplate, - mut count: u32, + count: u32, force_soulbound: bool, preselected_property: Option, ) -> Result<(), String> { + store_item_typed( + ctx, + player_guid, + owner_identity, + tmpl, + count, + force_soulbound, + preselected_property, + ) + .map_err(Into::into) +} + +fn store_item_typed( + ctx: &ReducerContext, + player_guid: u64, + owner_identity: spacetimedb::Identity, + tmpl: &ItemTemplate, + mut count: u32, + force_soulbound: bool, + preselected_property: Option, +) -> Result<(), ActionRefusal> { if count == 0 { return Ok(()); } @@ -231,7 +263,10 @@ pub(crate) fn store_item( }); let new_stacks = remaining.div_ceil(max_stack); if new_stacks > super::inventory::count_free_inventory_slots(ctx, player_guid) { - return Err(lyracore_shared::mail::INVENTORY_FULL.to_owned()); + return Err(ActionRefusal::new( + ActionRefusalKind::InventoryFull, + lyracore_shared::mail::INVENTORY_FULL, + )); } let mut properties = Vec::with_capacity(new_stacks as usize); for index in 0..new_stacks { diff --git a/module/src/mount.rs b/module/src/mount.rs index 0da21103d..74d67233f 100644 --- a/module/src/mount.rs +++ b/module/src/mount.rs @@ -678,7 +678,7 @@ mod tests { let src = include_str!("combat/engage.rs"); for (signature, gate) in [ ( - "pub(crate) fn apply_start_attack(", + "fn start_attack(", "validate_attack_target(ctx, &attacker, target_guid)?;", ), ( @@ -715,7 +715,7 @@ mod tests { fn an_accepted_cast_dismounts_and_the_mount_cast_is_exempt() { let body = crate::test_scan::code_of( include_str!("spell/cast/resolve.rs"), - "pub(crate) fn resolve_cast_at(", + "pub(crate) fn resolve_cast_at_typed(", ); let gates = body .find("check_cast_gates(ctx, &caster, &hdr, &effects, target_guid, spell_id, level)?;") diff --git a/module/src/quest.rs b/module/src/quest.rs index 1f05fedac..de2c83bf9 100644 --- a/module/src/quest.rs +++ b/module/src/quest.rs @@ -21,6 +21,7 @@ //! [`crate::xp::grant_xp`] + [`crate::items::grant_item`]), and marks the row rewarded (kept, to block //! a repeat). Purely additive: brand-new tables + one hook call in `kill_creature`. +use crate::actor::{ActionRefusal, ActionRefusalKind}; use std::collections::BTreeSet; use spacetimedb::{table, Identity, ReducerContext, Table}; @@ -590,7 +591,7 @@ fn validate_giver( ctx: &ReducerContext, player: &crate::WorldEntity, giver_guid: u64, -) -> Result { +) -> Result { if let Some(giver) = ctx.db.game_world_entity().guid().find(giver_guid) { if giver.is_player() { // Work-item 194 (sharing): the ONLY case a live PLAYER entity is a valid giver — a @@ -598,11 +599,17 @@ fn validate_giver( return validate_party_giver(ctx, player, giver_guid, &giver); } if giver.map_id != player.map_id || giver.instance_id != player.instance_id { - return Err("quest giver on another map".to_string()); + return Err(ActionRefusal::new( + ActionRefusalKind::OtherPartition, + "quest giver on another map", + )); } let (dx, dy, dz) = (giver.x - player.x, giver.y - player.y, giver.z - player.z); if dx * dx + dy * dy + dz * dz > QUEST_GIVER_RANGE_SQ { - return Err("quest giver out of range".to_string()); + return Err(ActionRefusal::new( + ActionRefusalKind::OutOfRange, + "quest giver out of range", + )); } return Ok(GiverKind::Creature(giver.entry)); } @@ -614,11 +621,17 @@ fn validate_giver( // dungeon GOs are per-instance copies, so a giver in another party's Deadmines can never // serve this player). if go.map_id != player.map_id || go.instance_id != player.instance_id { - return Err("quest giver on another map".to_string()); + return Err(ActionRefusal::new( + ActionRefusalKind::OtherPartition, + "quest giver on another map", + )); } let (dx, dy, dz) = (go.x - player.x, go.y - player.y, go.z - player.z); if dx * dx + dy * dy + dz * dz > QUEST_GIVER_RANGE_SQ { - return Err("quest giver out of range".to_string()); + return Err(ActionRefusal::new( + ActionRefusalKind::OutOfRange, + "quest giver out of range", + )); } return Ok(GiverKind::GameObject(go.template_entry)); } @@ -632,7 +645,7 @@ fn validate_giver( .find(giver_guid) .filter(|i| i.owner_guid == player.guid) .map(|i| GiverKind::Item(i.entry)) - .ok_or_else(|| "no such quest giver".to_string()) + .ok_or_else(|| ActionRefusal::new(ActionRefusalKind::MissingTarget, "no such quest giver")) } /// Work-item 194 (sharing): validates a live PLAYER `giver_guid` as a party-share giver for `player` @@ -646,7 +659,7 @@ fn validate_party_giver( player: &crate::WorldEntity, giver_guid: u64, giver: &crate::WorldEntity, -) -> Result { +) -> Result { let same_group = match ( crate::group::group_of(ctx, player.guid), crate::group::group_of(ctx, giver_guid), @@ -655,14 +668,20 @@ fn validate_party_giver( _ => false, }; if !same_group { - return Err("target is not a quest giver".to_string()); + return Err("target is not a quest giver".to_string().into()); } if giver.map_id != player.map_id || giver.instance_id != player.instance_id { - return Err("quest giver on another map".to_string()); + return Err(ActionRefusal::new( + ActionRefusalKind::OtherPartition, + "quest giver on another map", + )); } let (dx, dy, dz) = (giver.x - player.x, giver.y - player.y, giver.z - player.z); if dx * dx + dy * dy + dz * dz > PARTY_SHARE_RANGE_SQ { - return Err("quest giver out of range".to_string()); + return Err(ActionRefusal::new( + ActionRefusalKind::OutOfRange, + "quest giver out of range", + )); } Ok(GiverKind::Party(giver_guid)) } @@ -777,14 +796,28 @@ pub(crate) fn apply_accept_quest( giver_guid: u64, quest_entry: u32, ) -> Result<(), String> { + request_accept_quest(ctx, player_guid, giver_guid, quest_entry).map_err(Into::into) +} + +pub(crate) fn request_accept_quest( + ctx: &ReducerContext, + player_guid: u64, + giver_guid: u64, + quest_entry: u32, +) -> Result<(), ActionRefusal> { let player = crate::helpers::live_entity(ctx, player_guid) - .map_err(|_| "player not in world".to_string())?; + .map_err(|_| ActionRefusal::new(ActionRefusalKind::MissingActor, "player not in world"))?; if player.dead { - return Err("dead players cannot accept quests".to_string()); + return Err(ActionRefusal::new( + ActionRefusalKind::DeadActor, + "dead players cannot accept quests", + )); } let giver = validate_giver(ctx, &player, giver_guid)?; if !giver_has_quest_role(ctx, &giver, quest_entry, quest_role::START) { - return Err("that quest giver does not offer this quest".to_string()); + return Err("that quest giver does not offer this quest" + .to_string() + .into()); } let tmpl = ctx .db @@ -812,6 +845,7 @@ pub(crate) fn apply_accept_quest( existing, deadline_micros, ) + .map_err(Into::into) } /// The accept EFFECTS shared by [`apply_accept_quest`] and [`grant_quest_unchecked`] (the debug twin) — @@ -968,30 +1002,45 @@ pub(crate) fn apply_turn_in_quest( quest_entry: u32, reward_index: u32, ) -> Result<(), String> { + request_turn_in_quest(ctx, player_guid, giver_guid, quest_entry, reward_index) + .map_err(Into::into) +} + +pub(crate) fn request_turn_in_quest( + ctx: &ReducerContext, + player_guid: u64, + giver_guid: u64, + quest_entry: u32, + reward_index: u32, +) -> Result<(), ActionRefusal> { let entities = ctx.db.game_world_entity(); - let mut player = entities - .guid() - .find(player_guid) - .ok_or_else(|| "player not in world".to_string())?; + let mut player = entities.guid().find(player_guid).ok_or_else(|| { + ActionRefusal::new(ActionRefusalKind::MissingActor, "player not in world") + })?; if player.dead { - return Err("dead players cannot turn in quests".to_string()); + return Err(ActionRefusal::new( + ActionRefusalKind::DeadActor, + "dead players cannot turn in quests", + )); } let giver = validate_giver(ctx, &player, giver_guid)?; if !giver_has_quest_role(ctx, &giver, quest_entry, quest_role::END) { - return Err("that quest giver does not complete this quest".to_string()); + return Err("that quest giver does not complete this quest" + .to_string() + .into()); } let mut cq = character_quest_row(ctx, player_guid, quest_entry) .ok_or_else(|| "you are not on that quest".to_string())?; if cq.rewarded { - return Err("quest already turned in".to_string()); + return Err("quest already turned in".to_string().into()); } // Work-item 194 (timed quests): a FAILED (expired) quest can't be turned in — it must be // re-accepted first (the duplicate guard in `apply_accept_quest` lets that through). if cq.failed { - return Err("quest has expired".to_string()); + return Err("quest has expired".to_string().into()); } if !quest_is_complete(ctx, &cq) { - return Err("quest objectives not complete".to_string()); + return Err("quest objectives not complete".to_string().into()); } let tmpl = ctx .db @@ -1014,7 +1063,7 @@ pub(crate) fn apply_turn_in_quest( .find(reward.spell_id) .is_none() { - return Err(format!("quest reward spell {} is missing", reward.spell_id)); + return Err(format!("quest reward spell {} is missing", reward.spell_id).into()); } } let owner_identity = player.owner_identity; @@ -1043,7 +1092,7 @@ pub(crate) fn apply_turn_in_quest( .by_quest() .filter(&quest_entry) { - crate::items::grant_item(ctx, player_guid, r.item_entry, r.count)?; + crate::items::request_grant_item(ctx, player_guid, r.item_entry, r.count, None)?; } // Choice reward (pick-1-of-N): grant the single row whose choice_index == reward_index, IN ADDITION to // the guaranteed items above and atomic with them (any Err rolls the whole tx back, before money/XP). @@ -1057,7 +1106,7 @@ pub(crate) fn apply_turn_in_quest( .map(|c| (c.choice_index, c.item_entry, c.count)) .collect(); if let Some((item_entry, count)) = pick_choice_reward(&choices, reward_index, quest_entry)? { - crate::items::grant_item(ctx, player_guid, item_entry, count)?; + crate::items::request_grant_item(ctx, player_guid, item_entry, count, None)?; } player.money = player.money.saturating_add(tmpl.reward_money); // Quest XP is scaled by the realm xp_rate at the source (like kill XP), so a custom-rate realm diff --git a/module/src/spell/cast/resolve.rs b/module/src/spell/cast/resolve.rs index c4e623811..71009b697 100644 --- a/module/src/spell/cast/resolve.rs +++ b/module/src/spell/cast/resolve.rs @@ -58,6 +58,42 @@ pub(crate) fn resolve_cast_at( // create_ground_area. dest: Option<(f32, f32, f32)>, ) -> Result<(), String> { + resolve_cast_at_typed( + ctx, + caster_guid, + spell_id, + level, + target_guid, + is_completion, + client_initiated, + dest, + ) + .map_err(Into::into) +} + +#[allow(clippy::too_many_arguments)] +pub(crate) fn resolve_cast_at_typed( + ctx: &ReducerContext, + caster_guid: u64, + spell_id: u32, + level: u8, + target_guid: u64, + // true ONLY when called from `fire_pending_cast` (a TIMED cast finishing its cast bar): written onto + // the cast-GO event so the gateway emits GO+COOLDOWN WITHOUT a second SMSG_SPELL_START(0) (the START + // was already sent at begin_cast). false for an instant cast, a channel, a creature cast, and a + // triggered cast — all of which still want the START(0)+GO+COOLDOWN instant packet sequence. + is_completion: bool, + // true ONLY on the player CMSG_CAST_SPELL path (088; threaded via begin_cast from the cast_spell + // reducer): the gateway sent that caster's START/RESULT/GO synchronously, so the relay suppresses + // its duplicate. Channel ticks / triggers / creature / debug / item-use casts pass false and the + // relay DELIVERS the caster's visual (they never had a synchronous send). + client_initiated: bool, + // The clicked GROUND point (118 phase 2), for a ground-targeted cast (CMSG_CAST_SPELL's + // DEST_LOCATION). `Some` → AREA effects splash from it and a ground patch anchors there; `None` for + // every non-ground cast (self/unit-target/creature/trigger). Threaded verbatim to select_targets + + // create_ground_area. + dest: Option<(f32, f32, f32)>, +) -> Result<(), CastRefusal> { let hdr = ctx .db .game_spell() @@ -683,7 +719,8 @@ pub(crate) fn start_creature_spell( false, None, route.admission, - ), + ) + .map_err(Into::into), CreatureSpellStartMode::Triggered => { let origin = match route.admission { CreatureSpellCasterAdmission::Living => CastOrigin::Triggered, @@ -723,7 +760,7 @@ fn check_cast_gates( target_guid: u64, spell_id: u32, level: u8, -) -> Result<(), String> { +) -> Result<(), CastRefusal> { let allow_dead_creature = dead_callback_cast_admitted(ctx, caster.guid, spell_id); check_cast_gate_prefix(ctx, caster, effects, target_guid, allow_dead_creature)?; crate::mount::check_mount_cast(ctx, caster, effects, spell_id)?; @@ -740,7 +777,7 @@ fn check_cast_gates_with_admission( spell_id: u32, level: u8, allow_dead_creature: bool, -) -> Result<(), String> { +) -> Result<(), CastRefusal> { check_cast_gate_prefix(ctx, caster, effects, target_guid, allow_dead_creature)?; crate::mount::check_mount_cast(ctx, caster, effects, spell_id)?; check_cast_gate_suffix(ctx, caster, hdr, effects, target_guid, spell_id, level) @@ -752,7 +789,7 @@ fn check_cast_gate_prefix( effects: &[SpellEffect], target_guid: u64, allow_dead_creature: bool, -) -> Result<(), String> { +) -> Result<(), CastRefusal> { let caster_guid = caster.guid; // CC: an ACTION-blocked caster (stunned, polymorphed, OR feared) cannot cast. This is the chokepoint @@ -761,13 +798,19 @@ fn check_cast_gate_prefix( // early-outs on this in tick_creatures). Baseline-safe: an un-CC'd caster has no CC aura → `false` → // unchanged. if is_action_blocked(ctx, caster_guid) { - return Err(format!("caster {caster_guid} cannot act (stun/poly/fear)")); + return Err(CastRefusal::new( + CastRefusalKind::CannotAct, + format!("caster {caster_guid} cannot act (stun/poly/fear)"), + )); } // Only a committed creature callback that can run after lethal damage may start or finish a cast // from its corpse. Player paths and ordinary creature casts keep the shared dead-caster refusal. if !caster_life_allows(caster.is_player(), caster.dead, allow_dead_creature) { - return Err(format!("caster {caster_guid} is dead")); + return Err(CastRefusal::new( + CastRefusalKind::Dead, + format!("caster {caster_guid} is dead"), + )); } // Taming is completion-gated and revalidated here before cost or any write. The handler repeats @@ -789,7 +832,7 @@ fn check_cast_gate_suffix( target_guid: u64, spell_id: u32, level: u8, -) -> Result<(), String> { +) -> Result<(), CastRefusal> { let caster_guid = caster.guid; for effect in effects .iter() @@ -806,10 +849,10 @@ fn check_cast_gate_suffix( // Defias Rogue Wizard L9-10 casts Frostbolt 13322 spell_level 20), and the leveling spine is a // player-only concept. The guard keeps player casts BYTE-IDENTICAL — see `level_gate_blocks`. [server] if level_gate_blocks(caster.is_player(), hdr.spell_level, level) { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} requires level {} (caster is level {level})", hdr.spell_level - )); + ))); } // Stance gate (Tier 3a — Warrior stances): a stance-restricted ability (Stances usability mask != 0) is @@ -819,10 +862,10 @@ fn check_cast_gate_suffix( // gates (after the level gate, before GCD/cost) so a wrong-stance cast spends nothing — the school-lockout // / react-window precedent. Keyed on the caster's `stance` field + the header mask, NEVER a spell id. if !stance_allows(hdr.stances, caster.stance) { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} cannot be cast in the caster's current stance ({})", caster.stance - )); + ))); } // School-lockout gate (the Kick silence): a cast whose SCHOOL was just interrupted on this caster is @@ -846,10 +889,10 @@ fn check_cast_gate_suffix( }) { let _ = lock; - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} school {} is locked (interrupted) for caster {caster_guid}", hdr.school_mask - )); + ))); } // GCD gate (before cost so an on-cooldown cast spends nothing). The GCD is the per-CASTER row in @@ -859,7 +902,10 @@ fn check_cast_gate_suffix( ctx.timestamp.to_micros_since_unix_epoch(), cd.ready_at.to_micros_since_unix_epoch(), ) { - return Err("spell not ready (global cooldown)".to_string()); + return Err(CastRefusal::new( + CastRefusalKind::Cooldown, + "spell not ready (global cooldown)".to_string(), + )); } } @@ -881,7 +927,10 @@ fn check_cast_gate_suffix( ctx.timestamp.to_micros_since_unix_epoch(), cd.ready_at.to_micros_since_unix_epoch(), ) { - return Err("spell not ready (cooldown)".to_string()); + return Err(CastRefusal::new( + CastRefusalKind::Cooldown, + "spell not ready (cooldown)".to_string(), + )); } } } @@ -898,16 +947,16 @@ fn check_cast_gate_suffix( if hdr.cast_flags & SPELL_ATTR_REQ_OVERPOWER != 0 && !crate::combat::react_window_active(caster.overpower_until_ms, now_ms) { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} requires an Overpower window (the target must have dodged your swing)" - )); + ))); } if hdr.cast_flags & SPELL_ATTR_REQ_REVENGE != 0 && !crate::combat::react_window_active(caster.revenge_until_ms, now_ms) { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} requires a Revenge window (you must have dodged/parried/blocked)" - )); + ))); } } @@ -922,9 +971,12 @@ fn check_cast_gate_suffix( // share a map AND instance (work-item 190 slice 1) — a cross-map/cross-instance target is // unreachable (treat as out of range) before trusting it. if caster.map_id != target.map_id || caster.instance_id != target.instance_id { - return Err(format!( - "target on another map (caster {} != target {})", - caster.map_id, target.map_id + return Err(CastRefusal::new( + CastRefusalKind::OtherPartition, + format!( + "target on another map (caster {} != target {})", + caster.map_id, target.map_id + ), )); } let dist = distance_3d(caster.x, caster.y, caster.z, target.x, target.y, target.z); @@ -937,9 +989,12 @@ fn check_cast_gate_suffix( // tight on a fleeing mob. const CAST_RANGE_LEEWAY_YD: f32 = 2.0 + 8.0 / 3.0; // ≈ 4.67 yd if dist > hdr.range_yd as f32 + CAST_RANGE_LEEWAY_YD { - return Err(format!( - "target out of range ({dist:.1} yd > {} + {:.1} yd leeway)", - hdr.range_yd, CAST_RANGE_LEEWAY_YD + return Err(CastRefusal::new( + CastRefusalKind::OutOfRange, + format!( + "target out of range ({dist:.1} yd > {} + {:.1} yd leeway)", + hdr.range_yd, CAST_RANGE_LEEWAY_YD + ), )); } } @@ -978,9 +1033,9 @@ fn check_cast_gate_suffix( if let Some(class) = faction_target_violation(hits_enemy, hits_ally, friendly, hostile) { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} targets {class} — cannot be cast on this unit ({target_guid})" - )); + ))); } } } @@ -1000,10 +1055,10 @@ fn check_cast_gate_suffix( if let Some(e) = effects.iter().find(|e| { e.kind & KIND_AURA_BIT != 0 && e.p1 != 0 && has_aura(ctx, target_guid, e.p1 as u32) }) { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} refused — target {target_guid} carries the linked debuff {} (Weakened-Soul-style lockout)", e.p1 - )); + ))); } // Blink (116) requires its teleport DISTANCE as data — the effect's DBC radius (`radius_yd`, @@ -1014,10 +1069,10 @@ fn check_cast_gate_suffix( .iter() .find(|e| e.kind == E_BLINK && e.radius_yd <= 0.0) { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} (Blink) effect {} has no radius — teleport distance is unauthored data", e.effect_index - )); + ))); } // A ground-AoE (118) with no radius is unauthored data — reject LOUD rather than spawn a 0-radius // area that silently damages nobody (same fail-loud stance as Blink's distance). @@ -1025,10 +1080,10 @@ fn check_cast_gate_suffix( .iter() .find(|e| e.kind == E_PERSISTENT_AREA && e.radius_yd <= 0.0) { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} (ground-AoE) effect {} has no radius — area size is unauthored data", e.effect_index - )); + ))); } // Scripted-effect gate: a cast carrying an E_SCRIPTED effect with a nonzero script_id must @@ -1047,10 +1102,10 @@ fn check_cast_gate_suffix( .find(e.script_id) .is_some_and(|script| script.enabled) }) { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} effect {} names script {} which is missing or disabled", e.effect_index, e.script_id - )); + ))); } // Rogue cast-gate flags (REQ_BEHIND / REQ_STEALTH + Sap's out-of-combat + humanoid constraints). All @@ -1065,16 +1120,16 @@ fn check_cast_gate_suffix( let target = crate::helpers::live_entity(ctx, target_guid) .map_err(|_| format!("spell {spell_id} target {target_guid} not in world"))?; if !is_behind(caster.x, caster.y, target.x, target.y, target.orientation) { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} must be cast from BEHIND the target ({target_guid})" - )); + ))); } } // REQ_STEALTH (Sap): the caster must be stealthed. Read BEFORE the break below. if hdr.cast_flags & SPELL_ATTR_REQ_STEALTH != 0 && !is_stealthed(ctx, caster_guid) { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} requires stealth (caster {caster_guid} is not stealthed)" - )); + ))); } // Sap-shaped opener constraints: an INCAP_OPENER spell (Sap) additionally requires the target to be // OUT of combat AND a HUMANOID creature (vanilla Sap). Keyed on the SEPARATE SPELL_ATTR_INCAP_OPENER @@ -1084,9 +1139,9 @@ fn check_cast_gate_suffix( if hdr.cast_flags & SPELL_ATTR_INCAP_OPENER != 0 { if let Some(target) = ctx.db.game_world_entity().guid().find(target_guid) { if target.unit_flags & lyracore_shared::constants::unit_flags::IN_COMBAT != 0 { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} cannot target a unit IN COMBAT ({target_guid})" - )); + ))); } // Players are always Humanoid (no creature_template row) — a missing template would // otherwise reject every player target. @@ -1098,9 +1153,9 @@ fn check_cast_gate_suffix( .find(target.entry) .is_some_and(|t| t.creature_type == HUMANOID_TYPE); if !is_humanoid { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} can only target a Humanoid ({target_guid})" - )); + ))); } } } @@ -1127,9 +1182,9 @@ fn check_cast_gate_suffix( }) .unwrap_or(false); if !has_dagger { - return Err(format!( + return Err(CastRefusal::from(format!( "spell {spell_id} requires a dagger equipped in the main hand (caster {caster_guid})" - )); + ))); } } @@ -1139,9 +1194,9 @@ fn check_cast_gate_suffix( // cost 90, Frost Armor cost 10). Gating on `is_player()` (not a spell-id branch) keeps player casts // BYTE-IDENTICAL and avoids wiring a mob mana column — see `cost_gate_blocks`. [server] if cost_gate_blocks(caster.is_player(), caster.power, hdr.cost) { - return Err(format!( - "not enough power: have {}, need {}", - caster.power, hdr.cost + return Err(CastRefusal::new( + CastRefusalKind::InsufficientPower, + format!("not enough power: have {}, need {}", caster.power, hdr.cost), )); } @@ -1202,10 +1257,11 @@ pub(crate) fn begin_cast( dest, CreatureSpellCasterAdmission::Living, ) + .map_err(Into::into) } #[allow(clippy::too_many_arguments)] -fn begin_cast_with_admission( +pub(crate) fn begin_cast_with_admission( ctx: &ReducerContext, caster_guid: u64, spell_id: u32, @@ -1214,7 +1270,7 @@ fn begin_cast_with_admission( client_initiated: bool, dest: Option<(f32, f32, f32)>, admission: CreatureSpellCasterAdmission, -) -> Result<(), String> { +) -> Result<(), CastRefusal> { let hdr = ctx .db .game_spell() @@ -1276,7 +1332,7 @@ fn begin_cast_with_admission( if !completed_channel && (cast_ms == 0 || hdr.cast_flags & SPELL_ATTR_CHANNELED != 0) { // Instant / channel: resolves NOW (no pending cast bar), so it is NOT a completion — the gateway // sends the full instant START(0)+GO+COOLDOWN sequence. - let result = resolve_cast_at( + let result = resolve_cast_at_typed( ctx, caster_guid, spell_id, diff --git a/module/src/spell/control.rs b/module/src/spell/control.rs index 97b50ac78..389f24a5a 100644 --- a/module/src/spell/control.rs +++ b/module/src/spell/control.rs @@ -379,15 +379,23 @@ pub(crate) fn pushback_cast(ctx: &ReducerContext, caster_guid: u64) { /// client-facing cast-bar cancel (an `SMSG_SPELL_FAILED`/interrupted wire push) is a gateway follow-up — /// the server-authoritative effect (the cast simply never lands) is complete here. [entity] pub(crate) fn interrupt_cast(ctx: &ReducerContext, caster_guid: u64) -> bool { + interrupt_cast_with_outcome(ctx, caster_guid, CastFinish::Cancelled) +} + +fn interrupt_cast_with_outcome( + ctx: &ReducerContext, + caster_guid: u64, + outcome: CastFinish, +) -> bool { let pending = ctx.db.game_pending_cast(); // Capture (scheduled_id, spell_id) BEFORE deletion — the spell_id rides the interrupt signal row. - let hits: Vec<(u64, u32)> = pending + let hits: Vec<(u64, u32, u64)> = pending .by_caster() .filter(&caster_guid) - .map(|p| (p.scheduled_id, p.spell_id)) + .map(|p| (p.scheduled_id, p.spell_id, p.target_guid)) .collect(); let interrupted = !hits.is_empty(); - for (id, spell_id) in hits { + for (id, spell_id, target_guid) in hits { pending.scheduled_id().delete(id); clear_dead_callback_cast_admission(ctx, caster_guid, spell_id); // INTERRUPT signal: one game_spell_cast_event row (is_interrupted=true) per cancelled cast → the @@ -397,6 +405,15 @@ pub(crate) fn interrupt_cast(ctx: &ReducerContext, caster_guid: u64) -> bool { is_interrupted: true, ..SpellCastEvent::signal(ctx, caster_guid, spell_id, SpellCastEventKind::Interrupt) }); + crate::hooks::fire_on_cast_finished( + ctx, + &crate::hooks::CastFinishedPayload { + caster_guid, + target_guid, + scheduled_id: id, + outcome: outcome.clone(), + }, + ); } if interrupted { log::info!("cast interrupted: caster {caster_guid}'s in-progress cast cancelled"); @@ -404,6 +421,36 @@ pub(crate) fn interrupt_cast(ctx: &ReducerContext, caster_guid: u64) -> bool { interrupted } +/// Cancel only the named cast. A stale request cannot cancel its replacement. +#[cfg_attr(not(has_packages), allow(dead_code))] +pub(crate) fn cancel_cast_attempt( + ctx: &ReducerContext, + caster_guid: u64, + scheduled_id: u64, +) -> bool { + if !pending_cast(ctx, caster_guid).is_some_and(|cast| cast.scheduled_id == scheduled_id) { + return false; + } + interrupt_cast(ctx, caster_guid) +} + +/// Expiry is a caller's action deadline, separate from the spell's scheduled completion time. +/// No cast is removed before that deadline or after another cast replaced it. +#[cfg_attr(not(all(has_packages, feature = "debug_reducers")), allow(dead_code))] +pub(crate) fn expire_cast_attempt( + ctx: &ReducerContext, + caster_guid: u64, + scheduled_id: u64, + deadline_micros: i64, +) -> bool { + if ctx.timestamp.to_micros_since_unix_epoch() < deadline_micros + || !pending_cast(ctx, caster_guid).is_some_and(|cast| cast.scheduled_id == scheduled_id) + { + return false; + } + interrupt_cast_with_outcome(ctx, caster_guid, CastFinish::Expired) +} + /// BREAK a channel on `caster_guid` — the ONE convergent break path for the four channel-interrupt triggers /// (the caster MOVES / starts a NEW cast / is CC'd / its channel target dies). Deletes the caster's /// `A_PERIODIC_TRIGGER` channel aura(s) so `tick_auras` stops firing the per-tick missile, AND cancels any diff --git a/module/src/spell/mod.rs b/module/src/spell/mod.rs index b4635c1b7..c07bafdea 100644 --- a/module/src/spell/mod.rs +++ b/module/src/spell/mod.rs @@ -32,6 +32,8 @@ //! submodule actually defines it. mod cast; +mod outcome; +pub(crate) use outcome::*; mod control; mod effects; mod math; diff --git a/module/src/spell/outcome.rs b/module/src/spell/outcome.rs new file mode 100644 index 000000000..886d25d15 --- /dev/null +++ b/module/src/spell/outcome.rs @@ -0,0 +1,135 @@ +//! Cast requests retain the scheduled cast identity through completion and cancellation. + +use super::*; +use spacetimedb::{ReducerContext, ScheduleAt}; + +#[derive(spacetimedb::SpacetimeType, Clone, Copy, Debug, PartialEq, Eq)] +pub enum CastRefusalKind { + Other, + CannotAct, + Dead, + Cooldown, + OtherPartition, + OutOfRange, + InsufficientPower, + UnsupportedChannel, +} + +/// The owning Gate supplies the reason. `Other` preserves a Gate with no caller-specific policy. +#[derive(spacetimedb::SpacetimeType, Clone, Debug, PartialEq, Eq)] +pub struct CastRefusal { + pub kind: CastRefusalKind, + pub message: String, +} + +impl CastRefusal { + pub fn new(kind: CastRefusalKind, message: String) -> Self { + Self { kind, message } + } +} + +impl From for CastRefusal { + fn from(message: String) -> Self { + Self::new(CastRefusalKind::Other, message) + } +} + +impl From for String { + fn from(refusal: CastRefusal) -> Self { + refusal.message + } +} + +impl std::fmt::Display for CastRefusal { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + self.message.fmt(f) + } +} + +#[derive(spacetimedb::SpacetimeType, Clone, Debug, PartialEq, Eq)] +pub struct CastHandle { + pub scheduled_id: u64, + pub spell_id: u32, + pub target_guid: u64, + pub due_micros: i64, +} + +impl From for CastHandle { + fn from(cast: PendingCast) -> Self { + let due_micros = match cast.scheduled_at { + ScheduleAt::Time(at) => at.to_micros_since_unix_epoch(), + ScheduleAt::Interval(_) => i64::MAX, + }; + Self { + scheduled_id: cast.scheduled_id, + spell_id: cast.spell_id, + target_guid: cast.target_guid, + due_micros, + } + } +} + +#[derive(spacetimedb::SpacetimeType, Clone, Debug, PartialEq, Eq)] +pub enum CastStart { + /// Effects dispatched. A projectile may still be in flight. + Resolved, + Started(CastHandle), + Waiting(CastHandle), +} + +#[derive(spacetimedb::SpacetimeType, Clone, Debug, PartialEq, Eq)] +pub enum CastFinish { + Resolved, + Refused(CastRefusal), + Cancelled, + Expired, +} + +/// A request never replaces a pending cast. Explicit cancellation is a separate operation. +#[cfg_attr(not(has_packages), allow(dead_code))] +pub(crate) fn request_cast( + ctx: &ReducerContext, + caster_guid: u64, + spell_id: u32, + target_guid: u64, +) -> Result { + if let Some(cast) = pending_cast(ctx, caster_guid) { + return Ok(CastStart::Waiting(cast)); + } + let caster = crate::helpers::live_entity(ctx, caster_guid)?; + if ctx + .db + .game_spell() + .spell_id() + .find(spell_id) + .is_some_and(|spell| spell.cast_flags & SPELL_ATTR_CHANNELED != 0) + { + return Err(CastRefusal::new( + CastRefusalKind::UnsupportedChannel, + "bot cast requests do not yet retain channels".to_string(), + )); + } + begin_cast_with_admission( + ctx, + caster_guid, + spell_id, + caster.level as u8, + target_guid, + false, + None, + CreatureSpellCasterAdmission::Living, + )?; + Ok(match pending_cast(ctx, caster_guid) { + Some(cast) => CastStart::Started(cast), + None => CastStart::Resolved, + }) +} + +pub(crate) fn pending_cast(ctx: &ReducerContext, caster_guid: u64) -> Option { + ctx.db + .game_pending_cast() + .by_caster() + .filter(caster_guid) + .next() + .map(Into::into) +} diff --git a/module/src/spell/scheduler.rs b/module/src/spell/scheduler.rs index 47b3c8e21..ad5c10571 100644 --- a/module/src/spell/scheduler.rs +++ b/module/src/spell/scheduler.rs @@ -20,7 +20,7 @@ pub fn fire_pending_cast(ctx: &ReducerContext, sched: PendingCast) { if ctx.sender() != ctx.database_identity() { return; } - let result = resolve_cast_at( + let result = resolve_cast_at_typed( ctx, sched.caster_guid, sched.spell_id, @@ -41,7 +41,7 @@ pub fn fire_pending_cast(ctx: &ReducerContext, sched: PendingCast) { .then_some((sched.dest_x, sched.dest_y, sched.dest_z)), ); clear_dead_callback_cast_admission(ctx, sched.caster_guid, sched.spell_id); - if let Err(e) = result { + if let Err(e) = &result { log::info!( "pending cast {} (spell {}, caster {}) did not resolve: {}", sched.scheduled_id, @@ -62,6 +62,22 @@ pub fn fire_pending_cast(ctx: &ReducerContext, sched: PendingCast) { ) }); } + ctx.db + .game_pending_cast() + .scheduled_id() + .delete(sched.scheduled_id); + crate::hooks::fire_on_cast_finished( + ctx, + &crate::hooks::CastFinishedPayload { + caster_guid: sched.caster_guid, + target_guid: sched.target_guid, + scheduled_id: sched.scheduled_id, + outcome: match result { + Ok(()) => CastFinish::Resolved, + Err(refusal) => CastFinish::Refused(refusal), + }, + }, + ); } /// One-shot scheduler callback: a projectile's missile travel time elapsed — apply the From 9dd58dd6145c2c4e77eb5d507128cd3d49f31a9b Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 12:02:21 +0000 Subject: [PATCH 3/9] fix(quest): preflight item exchanges before granting rewards --- module/src/items/exchange.rs | 450 ++++++++++++++++++++++++++++++++++ module/src/items/inventory.rs | 60 +++-- module/src/items/mod.rs | 2 + module/src/items/ops.rs | 107 ++------ module/src/items/tables.rs | 6 +- module/src/quest.rs | 59 ++--- 6 files changed, 537 insertions(+), 147 deletions(-) create mode 100644 module/src/items/exchange.rs diff --git a/module/src/items/exchange.rs b/module/src/items/exchange.rs new file mode 100644 index 000000000..b7b585c68 --- /dev/null +++ b/module/src/items/exchange.rs @@ -0,0 +1,450 @@ +//! Plan item consumption and grants together before changing the Character's inventory. + +use crate::actor::{ActionRefusal, ActionRefusalKind}; +use spacetimedb::{Identity, ReducerContext, Table, Timestamp}; + +use super::inventory::{free_inventory_slots, is_carried_slot}; +use super::{ + allocate_item_guids, binds_on_grant, game_item_instance, game_item_template, merge_amount, + select_property, ItemInstance, ItemTemplate, +}; + +/// Quest collect items and all rewards form one exchange. Every Refusal precedes item changes; +/// GUID reservation may consume unused identities, but never returns them to the range. +pub(crate) fn exchange_items( + ctx: &ReducerContext, + character_guid: u64, + consumed: &[(u32, u32)], + granted: &[(u32, u32)], +) -> Result<(), ActionRefusal> { + let character = crate::helpers::live_entity(ctx, character_guid) + .map_err(|_| "player not in world".to_string())?; + let mut plan = ItemStoragePlan::read(ctx, character_guid, character.owner_identity); + for &(entry, count) in consumed { + plan.consume(entry, count)?; + } + for &(entry, count) in granted { + let template = ctx + .db + .game_item_template() + .entry() + .find(entry) + .ok_or_else(|| format!("no such item {entry}"))?; + plan.grant(ctx, &template, count.max(1), false, None)?; + } + plan.commit(ctx) +} + +/// Check the complete carried quantity before changing any stack. Bank copies stay untouched. +pub(super) fn consume_item_stacks( + items: &mut [ItemInstance], + entry: u32, + mut count: u32, +) -> Result<(), ActionRefusal> { + let mut matching: Vec = items + .iter() + .enumerate() + .filter(|(_, item)| is_carried_slot(item.slot) && item.entry == entry) + .map(|(index, _)| index) + .collect(); + let available: u64 = matching + .iter() + .map(|&index| u64::from(items[index].stack_count)) + .sum(); + if available < u64::from(count) { + return Err(format!("missing {} of item {entry}", u64::from(count) - available).into()); + } + matching.sort_by_key(|&index| items[index].slot); + for index in matching { + let item = &mut items[index]; + let take = count.min(item.stack_count); + item.stack_count -= take; + count -= take; + if count == 0 { + break; + } + } + Ok(()) +} + +/// Existing rows retain their identities. New rows receive GUIDs only after the full plan passes. +pub(super) struct ItemStoragePlan { + character_guid: u64, + owner_identity: Identity, + created_at: Timestamp, + items: Vec, + original_stacks: Vec<(u32, bool)>, +} + +impl ItemStoragePlan { + pub(super) fn read( + ctx: &ReducerContext, + character_guid: u64, + owner_identity: Identity, + ) -> Self { + Self::new( + character_guid, + owner_identity, + ctx.timestamp, + ctx.db + .game_item_instance() + .by_owner_guid() + .filter(&character_guid) + .collect(), + ) + } + + fn new( + character_guid: u64, + owner_identity: Identity, + created_at: Timestamp, + mut items: Vec, + ) -> Self { + items.sort_by_key(|item| item.slot); + let original_stacks = items + .iter() + .map(|item| (item.stack_count, item.soulbound)) + .collect(); + Self { + character_guid, + owner_identity, + created_at, + items, + original_stacks, + } + } + + fn consume(&mut self, entry: u32, count: u32) -> Result<(), ActionRefusal> { + consume_item_stacks(&mut self.items, entry, count) + } + + pub(super) fn grant( + &mut self, + ctx: &ReducerContext, + template: &ItemTemplate, + count: u32, + force_soulbound: bool, + preselected_property: Option, + ) -> Result<(), ActionRefusal> { + self.plan_grant( + template, + count, + force_soulbound, + preselected_property, + || select_property(ctx, template), + |entry| { + ctx.db + .game_item_template() + .entry() + .find(entry) + .map_or(0, |bag| bag.container_slots) + }, + ) + } + + fn plan_grant( + &mut self, + template: &ItemTemplate, + mut count: u32, + force_soulbound: bool, + preselected_property: Option, + mut property: impl FnMut() -> Result, + bag_capacity: impl FnMut(u32) -> u8, + ) -> Result<(), ActionRefusal> { + if count == 0 { + return Ok(()); + } + let owned_count: u64 = self + .items + .iter() + .filter(|item| item.entry == template.entry) + .map(|item| u64::from(item.stack_count)) + .sum(); + if template.max_count != 0 && owned_count + u64::from(count) > u64::from(template.max_count) + { + return Err(format!( + "unique item limit {} for item {}", + template.max_count, template.entry + ) + .into()); + } + let max_stack = template.max_stack.max(1); + let random_property_id = match preselected_property { + Some(property) => property, + None => property()?, + }; + let mut partials: Vec = self + .items + .iter() + .enumerate() + .filter(|(_, item)| { + is_carried_slot(item.slot) + && item.stack_count > 0 + && item.entry == template.entry + && item.random_property_id == random_property_id + && item.stack_count < max_stack + }) + .map(|(index, _)| index) + .collect(); + partials.sort_by_key(|&index| self.items[index].slot); + let remaining = partials.iter().fold(count, |left, &index| { + left.saturating_sub(max_stack - self.items[index].stack_count) + }); + let new_stacks = remaining.div_ceil(max_stack) as usize; + let slots = free_inventory_slots(&self.items, bag_capacity); + if new_stacks > slots.len() { + return Err(ActionRefusal::new( + ActionRefusalKind::InventoryFull, + lyracore_shared::mail::INVENTORY_FULL, + )); + } + let properties: Vec = (0..new_stacks) + .map(|index| { + if index == 0 || preselected_property.is_some() { + Ok(random_property_id) + } else { + property() + } + }) + .collect::>()?; + let soulbound = force_soulbound || binds_on_grant(template.bonding); + for index in partials { + let item = &mut self.items[index]; + let add = merge_amount(count, item.stack_count, max_stack); + item.stack_count += add; + item.soulbound |= soulbound; + count -= add; + if count == 0 { + break; + } + } + for (slot, random_property_id) in slots.into_iter().zip(properties) { + let take = count.min(max_stack); + self.items.push(ItemInstance { + guid: 0, + entry: template.entry, + owner_identity: self.owner_identity, + owner_guid: self.character_guid, + slot, + stack_count: take, + durability: template.max_durability, + created_at: self.created_at, + enchant_id: 0, + soulbound, + random_property_id, + }); + count -= take; + } + Ok(()) + } + + pub(super) fn commit(self, ctx: &ReducerContext) -> Result<(), ActionRefusal> { + let mut guids = + allocate_item_guids(ctx, self.items.len() - self.original_stacks.len())?.into_iter(); + let instances = ctx.db.game_item_instance(); + for (index, mut item) in self.items.into_iter().enumerate() { + match self.original_stacks.get(index) { + Some(&(count, soulbound)) + if (count, soulbound) != (item.stack_count, item.soulbound) => + { + if item.stack_count == 0 { + instances.guid().delete(item.guid); + } else { + instances.guid().update(item); + } + } + Some(_) => {} + None => { + // The reservation above issued one GUID for every appended item. + item.guid = guids.next().expect("item plan reserved every new GUID"); + instances.insert(item); + } + } + } + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn item(slot: u8, entry: u32, count: u32) -> ItemInstance { + ItemInstance { + guid: u64::from(slot) + 1, + entry, + owner_identity: Identity::ZERO, + owner_guid: 1, + slot, + stack_count: count, + durability: 50, + created_at: Timestamp::UNIX_EPOCH, + enchant_id: 0, + soulbound: false, + random_property_id: 0, + } + } + + fn plan(items: Vec) -> ItemStoragePlan { + ItemStoragePlan::new(1, Identity::ZERO, Timestamp::UNIX_EPOCH, items) + } + + fn grant( + plan: &mut ItemStoragePlan, + template: &ItemTemplate, + count: u32, + ) -> Result<(), ActionRefusal> { + plan.plan_grant(template, count, false, None, || Ok(0), |_| 0) + } + + #[test] + fn consumption_frees_capacity_for_the_complete_reward_set() { + let mut plan = plan((23..39).map(|slot| item(slot, 1, 1)).collect()); + plan.consume(1, 2).unwrap(); + grant(&mut plan, &crate::seed::base_item(2, "reward"), 1).unwrap(); + grant(&mut plan, &crate::seed::base_item(3, "choice"), 1).unwrap(); + let rewards: Vec<_> = plan + .items + .iter() + .filter(|item| item.entry != 1) + .map(|item| (item.slot, item.entry, item.stack_count)) + .collect(); + assert_eq!(rewards, vec![(23, 2, 1), (24, 3, 1)]); + } + + #[test] + fn reward_set_refuses_when_only_the_first_reward_fits() { + let mut plan = plan((23..39).map(|slot| item(slot, 1, 1)).collect()); + plan.consume(1, 1).unwrap(); + grant(&mut plan, &crate::seed::base_item(2, "reward"), 1).unwrap(); + let refusal = grant(&mut plan, &crate::seed::base_item(3, "choice"), 1).unwrap_err(); + assert_eq!(refusal.kind, ActionRefusalKind::InventoryFull); + assert_eq!(refusal.detail, lyracore_shared::mail::INVENTORY_FULL); + } + + #[test] + fn repeated_collect_objectives_cannot_consume_the_same_units_twice() { + let mut plan = plan(vec![item(23, 1, 5), item(39, 1, 20)]); + plan.consume(1, 3).unwrap(); + let refusal = plan.consume(1, 3).unwrap_err(); + assert_eq!(refusal.detail, "missing 1 of item 1"); + assert_eq!(plan.items[1].stack_count, 20, "bank copies stay owned"); + } + + #[test] + fn insufficient_carried_quantity_leaves_every_stack_unchanged() { + let mut items = vec![item(23, 1, 2), item(24, 2, 3), item(39, 1, 99)]; + let before: Vec<_> = items + .iter() + .map(|item| (item.guid, item.slot, item.entry, item.stack_count)) + .collect(); + let refusal = consume_item_stacks(&mut items, 1, 3).unwrap_err(); + assert_eq!(refusal.detail, "missing 1 of item 1"); + let after: Vec<_> = items + .iter() + .map(|item| (item.guid, item.slot, item.entry, item.stack_count)) + .collect(); + assert_eq!(after, before); + } + + #[test] + fn repeated_rewards_share_partial_stacks_without_exceeding_the_cap() { + let mut plan = plan(vec![item(23, 1, 8)]); + let mut template = crate::seed::base_item(1, "stackable reward"); + template.max_stack = 10; + grant(&mut plan, &template, 7).unwrap(); + grant(&mut plan, &template, 8).unwrap(); + let stacks: Vec<_> = plan + .items + .iter() + .map(|item| (item.slot, item.stack_count)) + .collect(); + assert_eq!(stacks, vec![(23, 10), (24, 10), (25, 3)]); + } + + #[test] + fn bank_ownership_counts_toward_a_unique_reward_limit() { + let mut plan = plan(vec![item(39, 1, 1)]); + let mut template = crate::seed::base_item(1, "unique reward"); + template.max_count = 1; + let refusal = grant(&mut plan, &template, 1).unwrap_err(); + assert_eq!(refusal.detail, "unique item limit 1 for item 1"); + } + + #[test] + fn duplicate_rewards_share_one_unique_limit() { + let mut plan = plan(Vec::new()); + let mut template = crate::seed::base_item(1, "unique reward"); + template.max_count = 1; + grant(&mut plan, &template, 1).unwrap(); + assert!(grant(&mut plan, &template, 1).is_err()); + } + + #[test] + fn consuming_a_unique_item_allows_its_replacement() { + let mut plan = plan(vec![item(23, 1, 1)]); + let mut template = crate::seed::base_item(1, "unique reward"); + template.max_count = 1; + plan.consume(1, 1).unwrap(); + grant(&mut plan, &template, 1).unwrap(); + assert_eq!( + plan.items.iter().map(|item| item.stack_count).sum::(), + 1 + ); + } + + #[test] + fn every_new_stack_resolves_its_property_before_a_grant_changes_items() { + let mut plan = plan(vec![item(23, 1, 8)]); + let mut template = crate::seed::base_item(1, "stackable reward"); + template.max_stack = 10; + let mut properties = [Ok(0), Err("invalid property pool".to_string())].into_iter(); + let refusal = plan + .plan_grant( + &template, + 20, + false, + None, + || properties.next().unwrap(), + |_| 0, + ) + .unwrap_err(); + assert_eq!(refusal.detail, "invalid property pool"); + assert_eq!(plan.items.len(), 1); + assert_eq!(plan.items[0].stack_count, 8); + } + + #[test] + fn property_mismatch_does_not_fill_an_existing_stack() { + let mut plan = plan(vec![item(23, 1, 8)]); + let mut template = crate::seed::base_item(1, "stackable reward"); + template.max_stack = 10; + plan.plan_grant(&template, 5, true, Some(7), || unreachable!(), |_| 0) + .unwrap(); + assert_eq!(plan.items[0].stack_count, 8); + assert_eq!(plan.items[1].stack_count, 5); + assert_eq!(plan.items[1].random_property_id, 7); + assert!(plan.items[1].soulbound); + } + + #[test] + fn equipped_bag_slots_follow_a_full_backpack() { + let mut owned: Vec<_> = (23..39).map(|slot| item(slot, 1, 1)).collect(); + owned.push(item(19, 2, 1)); + owned.push(item(120, 3, 1)); + let mut plan = plan(owned); + plan.plan_grant( + &crate::seed::base_item(4, "reward"), + 1, + false, + None, + || Ok(0), + |entry| { + assert_eq!(entry, 2); + 2 + }, + ) + .unwrap(); + assert_eq!(plan.items.last().unwrap().slot, 121); + } +} diff --git a/module/src/items/inventory.rs b/module/src/items/inventory.rs index ec7141f24..057e49612 100644 --- a/module/src/items/inventory.rs +++ b/module/src/items/inventory.rs @@ -535,34 +535,52 @@ fn occupied_slots(ctx: &ReducerContext, player_guid: u64) -> std::collections::H /// Number of free carry slots across the backpack and all equipped bags. pub(crate) fn count_free_inventory_slots(ctx: &ReducerContext, player_guid: u64) -> u32 { - let templates = ctx.db.game_item_template(); let owned: Vec = ctx .db .game_item_instance() .by_owner_guid() .filter(&player_guid) .collect(); - let occupied: std::collections::HashSet = owned.iter().map(|i| i.slot).collect(); - let backpack = (starter_item::BACKPACK_SLOT_0..BACKPACK_SLOT_END) + free_inventory_slots(&owned, |entry| { + ctx.db + .game_item_template() + .entry() + .find(entry) + .map_or(0, |t| t.container_slots) + }) + .len() as u32 +} + +/// Free storage slots after the planned item consumption. Empty stacks no longer occupy a slot +/// or provide bag capacity. Backpack slots precede equipped bags in their normal storage order. +pub(super) fn free_inventory_slots( + owned: &[ItemInstance], + mut bag_capacity: impl FnMut(u32) -> u8, +) -> Vec { + let occupied: std::collections::HashSet = owned + .iter() + .filter(|item| item.stack_count > 0) + .map(|item| item.slot) + .collect(); + let mut free: Vec = (starter_item::BACKPACK_SLOT_0..BACKPACK_SLOT_END) .filter(|slot| !occupied.contains(slot)) - .count() as u32; - let bags = (0..BAG_SLOT_COUNT) - .filter_map(|bag_idx| { - let bag = owned.iter().find(|i| i.slot == BAG_SLOT_START + bag_idx)?; - let capacity = templates - .entry() - .find(bag.entry)? - .container_slots - .min(MAX_BAG_SIZE); - let base = BAG_CONTENT_OFFSET + bag_idx * MAX_BAG_SIZE; - Some( - (0..capacity) - .filter(|offset| !occupied.contains(&(base + offset))) - .count() as u32, - ) - }) - .sum::(); - backpack + bags + .collect(); + for bag_idx in 0..BAG_SLOT_COUNT { + let Some(bag) = owned + .iter() + .find(|item| item.slot == BAG_SLOT_START + bag_idx && item.stack_count > 0) + else { + continue; + }; + let capacity = bag_capacity(bag.entry).min(MAX_BAG_SIZE); + let base = BAG_CONTENT_OFFSET + bag_idx * MAX_BAG_SIZE; + free.extend( + (0..capacity) + .map(|offset| base + offset) + .filter(|slot| !occupied.contains(slot)), + ); + } + free } pub(crate) fn first_free_backpack_slot(ctx: &ReducerContext, player_guid: u64) -> Option { diff --git a/module/src/items/mod.rs b/module/src/items/mod.rs index 68ef1d5fb..397270b73 100644 --- a/module/src/items/mod.rs +++ b/module/src/items/mod.rs @@ -20,6 +20,7 @@ //! reducers (`gw.rs`) are the only surviving callers of these cores. mod economy; +mod exchange; mod guids; mod inventory; mod ops; @@ -42,6 +43,7 @@ pub(crate) fn refused(refusal: ItemRefusal) -> String { } pub(crate) use economy::*; // the apply_* cores are pub(crate) (no pub items to re-export) +pub(crate) use exchange::exchange_items; pub(crate) use guids::*; pub(crate) use inventory::*; // ditto pub(crate) use ops::*; // ditto diff --git a/module/src/items/ops.rs b/module/src/items/ops.rs index f232fea86..d65d12668 100644 --- a/module/src/items/ops.rs +++ b/module/src/items/ops.rs @@ -4,13 +4,13 @@ //! reducer and its debug twin (see `reducers.rs`). All effects are additive: they touch only the item //! rows + the actor's health/money. -use crate::actor::{ActionRefusal, ActionRefusalKind}; +use crate::actor::ActionRefusal; use spacetimedb::{Identity, ReducerContext, Table}; use lyracore_shared::constants::starter_item; use lyracore_shared::item::ItemRefusal; -use super::{allocate_item_guids, next_item_guid, refuse, select_property}; +use super::{next_item_guid, refuse, select_property}; use crate::game_character; // the durable char holds `class` (the live WorldEntity does not) use crate::game_corpse_loot; // the loot.rs accessor trait — re-exported at crate root (`pub use loot::*`) use crate::game_gameobject; @@ -20,8 +20,7 @@ use crate::game_world_entity; // gameobject (chest) loot source — apply_take_l use super::inventory::{first_free_backpack_slot, first_free_bag_slot, is_carried_slot}; use super::rules::{ - binds_on_grant, equip_slot, meets_required_level, merge_amount, resolve_equip_slot, - template_stat, EquipStat, + binds_on_grant, equip_slot, meets_required_level, resolve_equip_slot, template_stat, EquipStat, }; use super::tables::{ game_item_instance, game_item_template, item_in_slot, item_is_broken, ItemInstance, @@ -157,10 +156,8 @@ pub(crate) fn grant_starter_item( /// quest turn-in rewards ([`crate::quest`]) and available to any other give-an-item path. Looks up the /// player (for the RLS `owner_identity`) and the item template (for durability), then delegates to /// `store_item`, which tops up an existing partial stack first and spills the remainder across as many -/// backpack/bag slots as it takes. Returns `Err` (rolling the whole tx back) if the player isn't in -/// world, the template is missing, or there's no room left — so a reward that can't be delivered fails -/// the turn-in atomically rather than half-applying. `count` is floored at 1. Additive — inserts item -/// row(s). [entity] +/// backpack/bag slots as it takes. A Refusal leaves inventory unchanged. Quest turn-in uses +/// `exchange_items` to plan all consumption and rewards together. `count` is floored at 1. pub(crate) fn grant_item( ctx: &ReducerContext, player_guid: u64, @@ -207,7 +204,7 @@ pub(crate) fn request_grant_item( } /// Add items to matching carried stacks, then free backpack or bag slots. -/// Capacity and every Random Property are resolved before changing any item, so callers may +/// Capacity, uniqueness and every Random Property are resolved before changing any item, so callers may /// handle an inventory-full Refusal without keeping a partial grant. pub(crate) fn store_item( ctx: &ReducerContext, @@ -235,78 +232,16 @@ fn store_item_typed( player_guid: u64, owner_identity: spacetimedb::Identity, tmpl: &ItemTemplate, - mut count: u32, + count: u32, force_soulbound: bool, preselected_property: Option, ) -> Result<(), ActionRefusal> { if count == 0 { return Ok(()); } - let instances = ctx.db.game_item_instance(); - let max_stack = tmpl.max_stack.max(1); - let random_property_id = preselected_property - .map(Ok) - .unwrap_or_else(|| select_property(ctx, tmpl))?; - let mut partials: Vec = instances - .by_owner_guid() - .filter(&player_guid) - .filter(|i| { - is_carried_slot(i.slot) - && i.entry == tmpl.entry - && i.random_property_id == random_property_id - && i.stack_count < max_stack - }) - .collect(); - partials.sort_by_key(|i| i.slot); - let remaining = partials.iter().fold(count, |left, item| { - left.saturating_sub(max_stack - item.stack_count) - }); - let new_stacks = remaining.div_ceil(max_stack); - if new_stacks > super::inventory::count_free_inventory_slots(ctx, player_guid) { - return Err(ActionRefusal::new( - ActionRefusalKind::InventoryFull, - lyracore_shared::mail::INVENTORY_FULL, - )); - } - let mut properties = Vec::with_capacity(new_stacks as usize); - for index in 0..new_stacks { - properties.push(if index == 0 || preselected_property.is_some() { - random_property_id - } else { - select_property(ctx, tmpl)? - }); - } - let guids = allocate_item_guids(ctx, new_stacks as usize)?; - for mut item in partials { - let add = merge_amount(count, item.stack_count, max_stack); - if add == 0 { - break; - } - item.stack_count += add; - item.soulbound |= force_soulbound || binds_on_grant(tmpl.bonding); - count -= add; - instances.guid().update(item); - } - for (random_property_id, guid) in properties.into_iter().zip(guids) { - // The capacity check counted these same slots, and reducers cannot interleave. - let slot = free_slot(ctx, player_guid).expect("preflight reserved a free item slot"); - let take = count.min(max_stack); - instances.insert(ItemInstance { - guid, - entry: tmpl.entry, - owner_identity, - owner_guid: player_guid, - slot, - stack_count: take, - durability: tmpl.max_durability, - created_at: ctx.timestamp, - enchant_id: 0, - soulbound: force_soulbound || binds_on_grant(tmpl.bonding), - random_property_id, - }); - count -= take; - } - Ok(()) + let mut plan = super::exchange::ItemStoragePlan::read(ctx, player_guid, owner_identity); + plan.grant(ctx, tmpl, count, force_soulbound, preselected_property)?; + plan.commit(ctx) } /// The next slot an incoming item can land in: backpack first (23..=38), then the content slots of @@ -406,14 +341,13 @@ pub(crate) fn item_count(ctx: &ReducerContext, owner_guid: u64, item_entry: u32) .sum() } -/// Remove exactly `count` units of `item_entry` from the player's stacks (lowest slot first), deleting an -/// emptied row. `Err` if the player doesn't actually have enough (the caller's reducer rolls back, so a -/// quest turn-in stays un-rewarded). Consumes a collect-quest's required items on turn-in (parity #4). [entity] +/// Remove exactly `count` carried units, lowest slot first. Missing quantity leaves every item +/// unchanged, even when the caller retains the Refusal and continues its reducer. pub(crate) fn remove_items( ctx: &ReducerContext, owner_guid: u64, item_entry: u32, - mut count: u32, + count: u32, ) -> Result<(), String> { let instances = ctx.db.game_item_instance(); let mut stacks: Vec = instances @@ -422,23 +356,18 @@ pub(crate) fn remove_items( // A turn-in consumes carried stacks only; the bank is never silently emptied. .filter(|i| is_carried_slot(i.slot) && i.entry == item_entry) .collect(); - stacks.sort_by_key(|i| i.slot); - for mut inst in stacks { - if count == 0 { - break; + let original_counts: Vec = stacks.iter().map(|item| item.stack_count).collect(); + super::exchange::consume_item_stacks(&mut stacks, item_entry, count).map_err(String::from)?; + for (inst, original_count) in stacks.into_iter().zip(original_counts) { + if inst.stack_count == original_count { + continue; } - let take = count.min(inst.stack_count); - inst.stack_count -= take; - count -= take; if inst.stack_count == 0 { instances.guid().delete(inst.guid); } else { instances.guid().update(inst); } } - if count > 0 { - return Err(format!("missing {count} of item {item_entry}")); - } Ok(()) } diff --git a/module/src/items/tables.rs b/module/src/items/tables.rs index 0b2e56348..78a1cfef3 100644 --- a/module/src/items/tables.rs +++ b/module/src/items/tables.rs @@ -166,9 +166,9 @@ pub struct ItemTemplate { /// treats 0 as "no limit" same as vanilla), the raw `item_template.Flags` bitmask (unique/ /// conjured/etc — `ItemFlag` on the wire), the readable-item page-text id (`PageText` — needs a /// page_text dump table + reader packet, deferred as its own follow-up), the quest-starter link - /// (`startquest` — 194 consumes), and the bag-type restriction bitmask (`BagFamily`). Work-item - /// 213: data plumbing only, no consumer reads these yet (all 0 = unrestricted/no-cap/no-quest, - /// baseline-safe). END-appended + `#[default(0)]` → additive auto-migration. [reference] + /// (`startquest` — 194 consumes), and the bag-type restriction bitmask (`BagFamily`). + /// Storage enforces `max_count` across carried and banked copies. Zero means no limit. + /// END-appended defaults keep existing items unrestricted. [reference] #[default(0)] pub max_count: u32, #[default(0)] diff --git a/module/src/quest.rs b/module/src/quest.rs index de2c83bf9..0cc5aba2a 100644 --- a/module/src/quest.rs +++ b/module/src/quest.rs @@ -845,14 +845,13 @@ pub(crate) fn request_accept_quest( existing, deadline_micros, ) - .map_err(Into::into) } /// The accept EFFECTS shared by [`apply_accept_quest`] and [`grant_quest_unchecked`] (the debug twin) — /// once every accept GATE has been checked (by the real accept) or deliberately skipped (by the harness /// grant), both paths do the identical work: grant the quest's provided "source" item (cmangos /// SrcItemId — the item HANDED to the player on accept, satisfying a COLLECT objective whose ReqItemId -/// == SrcItemId; `?` rolls the whole accept back on a full backpack), size a zeroed progress vector to +/// == SrcItemId; an item Refusal leaves the quest unchanged), size a zeroed progress vector to /// the quest's objective count (capped at [`MAX_OBJECTIVES`]), write the [`CharacterQuest`] row — /// UPDATED in place if `existing` names one (the repeatable/failed reset path, id kept — see /// `apply_accept_quest`'s duplicate guard), else a fresh insert — and fire the accept hook. The debug @@ -865,9 +864,15 @@ fn apply_accept_effects( tmpl: &QuestTemplate, existing: Option, deadline_micros: i64, -) -> Result<(), String> { +) -> Result<(), ActionRefusal> { if tmpl.src_item != 0 { - crate::items::grant_item(ctx, character_guid, tmpl.src_item, tmpl.src_item_count)?; + crate::items::request_grant_item( + ctx, + character_guid, + tmpl.src_item, + tmpl.src_item_count, + None, + )?; } let num_objectives = ctx .db @@ -965,6 +970,7 @@ pub(crate) fn grant_quest_unchecked( None, 0, ) + .map_err(Into::into) } /// Pure choice-reward pick (testable without a live `ReducerContext`): given a quest's choice rows as @@ -988,13 +994,9 @@ fn pick_choice_reward( .ok_or_else(|| format!("invalid reward choice {reward_index} for quest {quest_entry}")) } -/// Turn quest `quest_entry` in to giver `giver_guid` for `player_guid` — the shared core behind the -/// player `turn_in_quest` reducer and its debug twin. Validates: the player is in world + alive; the -/// giver is a real creature in range that ENDS the quest; the player has it ACTIVE (a row, not yet -/// rewarded); and every objective is complete. Then grants the rewards atomically — reward ITEMS first -/// (the only step that can fail, on a full backpack, rolling the whole tx back so nothing is granted -/// without the items landing), then money + XP — and marks the row `rewarded` (kept, to block a -/// repeat). Reuses [`crate::items::grant_item`] + [`crate::xp::grant_xp`]. [entity] +/// Turn in a completed quest through the giver and objective Gates. Resolve the reward choice, +/// spell, and complete item exchange before changing items, money, XP, or the quest row. +/// A caller may retain a Refusal without relying on reducer rollback. pub(crate) fn apply_turn_in_quest( ctx: &ReducerContext, player_guid: u64, @@ -1068,36 +1070,21 @@ pub(crate) fn request_turn_in_quest( } let owner_identity = player.owner_identity; - // Consume the COLLECT objectives' required items BEFORE granting rewards — frees the bag - // space the reward may need, and is atomic with the grant (any later Err rolls the whole tx back, so a - // failed turn-in neither eats the items nor hands out the reward). `quest_is_complete` already verified - // the player holds enough, so `remove_items` won't normally Err (it still does defensively). - let collect: Vec<_> = ctx + let collect: Vec<(u32, u32)> = ctx .db .game_quest_objective() .by_quest() .filter(&quest_entry) .filter(|o| o.kind == objective_kind::COLLECT_ITEM) + .map(|o| (o.target_entry, o.required_count)) .collect(); - for obj in collect { - crate::items::remove_items(ctx, player_guid, obj.target_entry, obj.required_count)?; - } - - // Rewards. Items FIRST (the fallible step — a full backpack must fail the whole turn-in before any - // money/XP/state change; SpacetimeDB rolls the tx back on Err). Then money (saturating) + XP via the - // shared ding loop. The player entity is updated once after grant_xp folds in any level-ups. - for r in ctx + let mut rewards: Vec<(u32, u32)> = ctx .db .game_quest_reward_item() .by_quest() .filter(&quest_entry) - { - crate::items::request_grant_item(ctx, player_guid, r.item_entry, r.count, None)?; - } - // Choice reward (pick-1-of-N): grant the single row whose choice_index == reward_index, IN ADDITION to - // the guaranteed items above and atomic with them (any Err rolls the whole tx back, before money/XP). - // A quest with NO choice rows ignores reward_index; a choice-quest whose index matches no slot is - // REJECTED (the client must pick a valid slot) — never silently grant nothing / the wrong item. + .map(|reward| (reward.item_entry, reward.count)) + .collect(); let choices: Vec<(u8, u32, u32)> = ctx .db .game_quest_reward_choice() @@ -1105,8 +1092,8 @@ pub(crate) fn request_turn_in_quest( .filter(&quest_entry) .map(|c| (c.choice_index, c.item_entry, c.count)) .collect(); - if let Some((item_entry, count)) = pick_choice_reward(&choices, reward_index, quest_entry)? { - crate::items::request_grant_item(ctx, player_guid, item_entry, count, None)?; + if let Some(reward) = pick_choice_reward(&choices, reward_index, quest_entry)? { + rewards.push(reward); } player.money = player.money.saturating_add(tmpl.reward_money); // Quest XP is scaled by the realm xp_rate at the source (like kill XP), so a custom-rate realm @@ -1133,7 +1120,11 @@ pub(crate) fn request_turn_in_quest( player.money = player .money .saturating_add(lyracore_shared::quest::max_level_money_reward(xp)); - } else { + } + // Packages may retain a Refusal and continue their reducer. The exchange resolves all item + // Gates before committing; every quest effect below this point is infallible. + crate::items::exchange_items(ctx, player_guid, &collect, &rewards)?; + if player.level < QUEST_MAX_LEVEL_PAYOUT { // "+N experience" feedback: a non-kill game_xp_event → SMSG_LOG_XPGAIN, mirroring the kill // (award_xp) and exploration paths. grant_xp itself deliberately never emits one, so quest XP // showed no floating text without this. killed_guid 0 = no source unit (the NonKill form). From 5322de73ab06e557618a96b2f6609a9150b9c352 Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 12:24:23 +0000 Subject: [PATCH 4/9] test(playerbots): prove durable action outcomes and progress --- .github/workflows/module-durable.yml | 2 +- CONTEXT.md | 18 + docs/package-api.md | 55 ++- module/src/actor.rs | 1 + module/src/combat/engage.rs | 9 + module/src/spell/cast/resolve.rs | 10 +- module/src/spell/outcome.rs | 8 +- module/src/spell/scheduler.rs | 13 +- module/tests/playerbots_rewrite.rs | 667 +++++++++++++++++++++++++++ 9 files changed, 763 insertions(+), 20 deletions(-) create mode 100644 module/tests/playerbots_rewrite.rs diff --git a/.github/workflows/module-durable.yml b/.github/workflows/module-durable.yml index e86693de2..01c0a5b56 100644 --- a/.github/workflows/module-durable.yml +++ b/.github/workflows/module-durable.yml @@ -74,7 +74,7 @@ jobs: # `lethal_floor`'s ranged phase races the module's own cast-event sweep and health regen, and # is flaky on `main` with or without this change. - name: Run the durable suite - run: cargo test -p lyracore-module --tests -- --ignored --test-threads=4 --skip deadmines --skip personality --skip lethal_floor + run: cargo test -p lyracore-module --tests -- --ignored --test-threads=4 --skip deadmines --skip personality --skip playerbots_ --skip lethal_floor - name: Run the single-shard auction Refusal regression run: cargo test -p lyracore-gateway --bin lyracore-gateway stdb::reducers::auction_reducer_tests::a_local_bid_interaction_refusal_stays_a_client_outcome -- --ignored --exact diff --git a/CONTEXT.md b/CONTEXT.md index 3d021a497..8f5b331df 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -298,6 +298,24 @@ from the old schema. New rows use 1 START, 2 GO, 3 INTERRUPT, 4 PUSHBACK, or 5 P does not guess how to decode an unknown nonzero code. _Avoid_: cast flags, event flags, signal flags +**Item Exchange**: +A complete plan of carried-item consumption and item grants. Capacity, uniqueness, Random Property, +and identity allocation Gates pass before any inventory row changes. Quest rewards commit as one +exchange even when the caller retains a Refusal and continues. + +**Action Outcome**: +An observation returned by the operation that owns an action. Acceptance records that work was +admitted; completion and progress require evidence from that work. A synchronous interaction ends +with success or Refusal. A timed cast also retains waiting and terminal outcomes. + +**Route Step**: +A planned movement leg with its route status, first waypoint, collision clipping, and coverage +evidence. Its endpoint is proposed movement, not proof of advancement or arrival. + +**Cast Handle**: +The scheduled identity, spell, target, and current due time of one timed cast. Repeated bot requests +retain this identity until the cast finishes or an explicit cancellation removes it. + ### Creature AI **Creature-AI Family**: diff --git a/docs/package-api.md b/docs/package-api.md index 9d44218df..85ea8080d 100644 --- a/docs/package-api.md +++ b/docs/package-api.md @@ -61,6 +61,7 @@ authoritative; this list is the set of event names and their payload types. | `on_kill` | `crate::hooks::KillPayload` | | `on_aggro` | `crate::hooks::AggroPayload` | | `on_cast_resolved` | `crate::hooks::CastResolvedPayload` | +| `on_cast_finished` | `crate::hooks::CastFinishedPayload` | | `on_loot` | `crate::hooks::LootPayload` | | `on_quest_accept` | `crate::hooks::QuestAcceptPayload` | | `on_quest_turnin` | `crate::hooks::QuestTurninPayload` | @@ -81,9 +82,32 @@ encounter content; the kernel exists for Packages. ### Actor verbs and helpers -`crate::actor` is the documented verb set over every explicit-guid action: one uniform shape, -`fn verb(ctx, actor_guid, ..) -> Result<(), String>`, with the gates of the core operation it names. -The table at the top of `module/src/actor.rs` lists every verb and its gate semantics. +`crate::actor` holds explicit-guid operations with the Gates of the core operation each names. +Existing verbs keep `fn verb(ctx, actor_guid, ..) -> Result<(), String>`. The table in +`module/src/actor.rs` lists their contracts. + +`actor::request_cast(ctx, actor_guid, spell_id, target_guid)` returns +`Result`. `Started` carries a Cast Handle. `Waiting` carries +an existing cast's original identity, spell, target, and current due time, even when the new request +names a different spell or target. It never restarts that cast. `Resolved` means effects dispatched +synchronously, which does not imply a projectile hit. Channeled spells return `UnsupportedChannel` +until bot requests can retain their lifecycle. Client casts keep their existing channel behavior. +`actor::cast_at` is the compatibility adapter that discards this distinction. These explicit-guid +requests preserve the Actor API's spellbook bypass. They do not prove the Character knows the spell; +capability selection must enforce known, supported spells before proposing them. + +A scheduled cast ends through `on_cast_finished`. Its payload carries the caster, target, scheduled +identity, and `spell::CastFinish`. Packages must match the scheduled identity before updating retained +work. The hook runs after the pending row is removed. An instant cast returns its result directly and +does not fire this hook. `on_cast_resolved` keeps its existing effect-dispatch contract. + +`spell::pending_cast` reads the current Cast Handle by caster. Refusal kinds are supplied by the +owning Gate. `Other` preserves a Gate's message when no current caller needs a separate policy. + +`spell::cancel_cast_attempt(ctx, caster_guid, scheduled_id)` cancels only that scheduled cast. +`spell::expire_cast_attempt(ctx, caster_guid, scheduled_id, deadline_micros)` also requires the +caller's action deadline to have passed. They return whether they removed the cast and report +`Cancelled` or `Expired` through `on_cast_finished`. A stale identity cannot cancel a replacement. `crate::helpers` holds the reads a Package needs before it acts: `live_entity`, `require_character`, `character_by_guid`, `character_by_name`, `entity_by_owner`, `acting_entity_by_guid`, `entities_near`, @@ -177,3 +201,28 @@ so the surface can grow or the Package can move off it. An exemption cannot enable a whole-crate alias or `#[path]`. Either spelling can hide dependencies on other lines or in files the lint cannot locate, so the build always refuses it. + +## Action observations + +`actor::request_attack` returns `AttackStart::Armed` or `AlreadyArmed`. The latter keeps the current +melee swing timer. Both mean an engagement was accepted; neither proves a hit. The existing +`actor::attack` and client operations retain their re-arm behavior. + +`actor::request_accept_quest` and `request_turn_in_quest` complete synchronously. They return +`Result<(), ActionRefusal>`, with the same core Gates and effects as the client operations. They +have no waiting, cancellation, or expiry phase. Attack requests also return `ActionRefusal`. +Its `kind` identifies actionable Gates and `detail` preserves the client message. `Other` is an +opaque refusal; Package policy must not classify its text. + +`nav::route_step` accepts the same arguments as `nav_step` and returns a `RouteStep`. Its `status` +is `Complete`, `Partial`, `Blocked`, or `Direct` when navigation is disabled. Complete and Partial +describe the planned route. `first_waypoint`, `expansions`, and `clipping` retain search and commit +Gate evidence. `endpoint == from` means no movement was approved, even if a complete path was +planned. A blocked search holds position. Creature callers retain `nav_step` and its existing +collision-gated fallback. + +`coverage` is `Unknown` unless every consulted navigation cell matches the active complete +manifest. `VerifiedCells` names that generation and the number of checked cells. It proves only +those cells' derivation, not world coverage or the presence of imported terrain and client +geometry. A Package must measure actual position on later observations to establish advancement +or arrival; the proposed endpoint cannot establish either. diff --git a/module/src/actor.rs b/module/src/actor.rs index dd2553a23..299a92dde 100644 --- a/module/src/actor.rs +++ b/module/src/actor.rs @@ -10,6 +10,7 @@ //! | verb | core | gate semantics (unchanged, documented here for consumers) | //! |------|------|------------------------------------------------------------| //! | `attack` | `combat::apply_start_attack` | CC-blocked rejected; no self/corpse/cross-map; friendly (green) target rejected when faction data exists; re-arm retargets | +//! | `request_attack` | `combat::request_attack` | typed acceptance or Refusal; keeps a matching swing timer; acceptance does not imply damage | //! | `ranged_attack` | `combat::apply_start_ranged_attack` | `attack` gates + ranged weapon equipped (slot 17) | //! | `stop_attack` | `combat::stop_attack_for` | unconditional disarm of the actor's outgoing melee row | //! | `cast_at` | `spell::request_cast` | normal cast lifecycle; an existing timed cast waits; level comes from the live entity | diff --git a/module/src/combat/engage.rs b/module/src/combat/engage.rs index ce21ba2f6..7148a8033 100644 --- a/module/src/combat/engage.rs +++ b/module/src/combat/engage.rs @@ -573,6 +573,12 @@ fn start_attack( let attacker = crate::helpers::live_entity(ctx, attacker_guid).map_err(|_| { ActionRefusal::new(ActionRefusalKind::MissingActor, "attacker not in world") })?; + if attacker.dead { + return Err(ActionRefusal::new( + ActionRefusalKind::DeadActor, + "dead attackers cannot attack", + )); + } // Crowd control: an ACTION-blocked attacker (stunned/polymorphed/feared) cannot ENTER combat — // arming an engagement is itself an action. This is the player-command twin of the per-swing gate in // `tick_melee` (without it a CC'd player could insert a `game_melee_attack` row whose swings are then @@ -652,6 +658,9 @@ pub(crate) fn apply_start_ranged_attack( ) -> Result<(), String> { let attacker = crate::helpers::live_entity(ctx, attacker_guid) .map_err(|_| "attacker not in world".to_string())?; + if attacker.dead { + return Err("dead attackers cannot attack".to_string()); + } if crate::spell::is_action_blocked(ctx, attacker.guid) { return Err(format!( "attacker {} cannot act (stun/poly/fear)", diff --git a/module/src/spell/cast/resolve.rs b/module/src/spell/cast/resolve.rs index 71009b697..7f5d926b6 100644 --- a/module/src/spell/cast/resolve.rs +++ b/module/src/spell/cast/resolve.rs @@ -720,6 +720,7 @@ pub(crate) fn start_creature_spell( None, route.admission, ) + .map(|_| ()) .map_err(Into::into), CreatureSpellStartMode::Triggered => { let origin = match route.admission { @@ -1257,6 +1258,7 @@ pub(crate) fn begin_cast( dest, CreatureSpellCasterAdmission::Living, ) + .map(|_| ()) .map_err(Into::into) } @@ -1270,7 +1272,7 @@ pub(crate) fn begin_cast_with_admission( client_initiated: bool, dest: Option<(f32, f32, f32)>, admission: CreatureSpellCasterAdmission, -) -> Result<(), CastRefusal> { +) -> Result { let hdr = ctx .db .game_spell() @@ -1343,7 +1345,7 @@ pub(crate) fn begin_cast_with_admission( dest, ); clear_dead_callback_cast_admission(ctx, caster_guid, spell_id); - return result; + return result.map(|_| CastStart::Resolved); } let completion_ms = if completed_channel { @@ -1373,7 +1375,7 @@ pub(crate) fn begin_cast_with_admission( Some(p) => (true, p), None => (false, (0.0, 0.0, 0.0)), }; - pending.insert(PendingCast { + let cast = pending.insert(PendingCast { scheduled_id: 0, scheduled_at: ScheduleAt::Time(fire_at), caster_guid, @@ -1396,7 +1398,7 @@ pub(crate) fn begin_cast_with_admission( // does both); the baseline's is_interrupted:false/delay_ms:0 already hold. ..SpellCastEvent::signal(ctx, caster_guid, spell_id, SpellCastEventKind::Start) }); - Ok(()) + Ok(CastStart::Started(cast.into())) } /// True when a spell effect's `kind` should reach the passive-apply path (`apply_spell_auras`'s diff --git a/module/src/spell/outcome.rs b/module/src/spell/outcome.rs index 886d25d15..bebb8d807 100644 --- a/module/src/spell/outcome.rs +++ b/module/src/spell/outcome.rs @@ -71,7 +71,7 @@ impl From for CastHandle { #[derive(spacetimedb::SpacetimeType, Clone, Debug, PartialEq, Eq)] pub enum CastStart { - /// Effects dispatched. A projectile may still be in flight. + /// Effects dispatched. A projectile or channel may remain active. Resolved, Started(CastHandle), Waiting(CastHandle), @@ -118,11 +118,7 @@ pub(crate) fn request_cast( false, None, CreatureSpellCasterAdmission::Living, - )?; - Ok(match pending_cast(ctx, caster_guid) { - Some(cast) => CastStart::Started(cast), - None => CastStart::Resolved, - }) + ) } pub(crate) fn pending_cast(ctx: &ReducerContext, caster_guid: u64) -> Option { diff --git a/module/src/spell/scheduler.rs b/module/src/spell/scheduler.rs index ad5c10571..1bfb933cf 100644 --- a/module/src/spell/scheduler.rs +++ b/module/src/spell/scheduler.rs @@ -20,6 +20,11 @@ pub fn fire_pending_cast(ctx: &ReducerContext, sched: PendingCast) { if ctx.sender() != ctx.database_identity() { return; } + // Release this identity before effect hooks can begin another cast. + ctx.db + .game_pending_cast() + .scheduled_id() + .delete(sched.scheduled_id); let result = resolve_cast_at_typed( ctx, sched.caster_guid, @@ -62,10 +67,6 @@ pub fn fire_pending_cast(ctx: &ReducerContext, sched: PendingCast) { ) }); } - ctx.db - .game_pending_cast() - .scheduled_id() - .delete(sched.scheduled_id); crate::hooks::fire_on_cast_finished( ctx, &crate::hooks::CastFinishedPayload { @@ -133,8 +134,8 @@ pub fn fire_spell_impact(ctx: &ReducerContext, sched: PendingSpellImpact) { /// The spellbook-gated cast core, actor-explicit: everything [`cast_spell`] does after /// resolving WHO is casting — the sender reducer above and `gw::gw_cast_spell` both delegate here. -/// Distinct from [`crate::actor::cast_at`], which drives `resolve_cast_at` and skips the spellbook -/// gate; this is the player-shaped entry (cast-time aware, self-cast on `target_guid == 0`). +/// This entry checks the spellbook and maps `target_guid == 0` to self. Actor requests use the same +/// cast lifecycle but retain the explicit-guid API's existing spellbook bypass. pub(crate) fn do_cast_spell( ctx: &ReducerContext, caster: crate::WorldEntity, diff --git a/module/tests/playerbots_rewrite.rs b/module/tests/playerbots_rewrite.rs new file mode 100644 index 000000000..06cb03227 --- /dev/null +++ b/module/tests/playerbots_rewrite.rs @@ -0,0 +1,667 @@ +//! Real Package action requests against a private, seeded Module. + +mod support; +use support::{poll_until, Standalone, POLL_TIMEOUT}; + +fn record_inputs(node: &Standalone) { + let core = std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .parent() + .unwrap(); + let package = core.join("packages/playerbots"); + let git = |path: &std::path::Path, args: &[&str]| { + let result = std::process::Command::new("git") + .current_dir(path) + .args(args) + .output() + .unwrap(); + assert!(result.status.success()); + String::from_utf8(result.stdout).unwrap().trim().to_string() + }; + fn digest_files(path: &std::path::Path, digest: &mut blake3::Hasher) { + let mut children: Vec<_> = std::fs::read_dir(path) + .unwrap() + .map(|e| e.unwrap().path()) + .collect(); + children.sort(); + digest.update(&(children.len() as u64).to_le_bytes()); + for child in children { + let name = child.file_name().unwrap().as_encoded_bytes(); + digest.update(&(name.len() as u64).to_le_bytes()); + digest.update(name); + digest.update(&[u8::from(child.is_dir())]); + if child.is_dir() { + digest_files(&child, digest); + } else { + let contents = std::fs::read(&child).unwrap(); + digest.update(&(contents.len() as u64).to_le_bytes()); + digest.update(&contents); + } + } + } + let mut digest = blake3::Hasher::new(); + digest_files(&package, &mut digest); + let mut record: serde_json::Value = + serde_json::from_slice(&std::fs::read(package.join("fixtures/actions.json")).unwrap()) + .unwrap(); + record["tested_core"] = git(core, &["rev-parse", "HEAD"]).into(); + record["tested_collection"] = git(&package, &["rev-parse", "HEAD"]).into(); + record["core_dirty"] = (!git(core, &["status", "--porcelain"]).is_empty()).into(); + record["collection_dirty"] = (!git(&package, &["status", "--porcelain"]).is_empty()).into(); + record["package_content_identity"] = digest.finalize().to_hex().to_string().into(); + record["module_wasm_identity"] = blake3::hash(support::module_bytes()) + .to_hex() + .to_string() + .into(); + let path = support::log_dir().join(format!("{}-inputs.json", node.shard_name())); + std::fs::write(&path, serde_json::to_vec_pretty(&record).unwrap()).unwrap(); + eprintln!("fixture inputs: {}", path.display()); +} + +fn record_observations(node: &Standalone) { + let mut record = serde_json::json!({ + "actions": node.query_rows("SELECT * FROM pkg_playerbots_action"), + "goals": node.query_rows("SELECT * FROM pkg_playerbots_goal"), + "quests": node.query_rows("SELECT * FROM game_character_quest"), + "pending_casts": node.query_rows("SELECT * FROM game_pending_cast"), + }); + let bots = node.query_rows("SELECT character_guid FROM pkg_playerbots_bot"); + let positions: Vec<_> = bots.iter().flat_map(|bot| node.query_rows(&format!( + "SELECT guid, map_id, instance_id, x, y, z, health FROM game_world_entity WHERE guid = {}", bot["character_guid"]))).collect(); + record["characters"] = serde_json::to_value(positions).unwrap(); + let path = support::log_dir().join(format!("{}-outcomes.json", node.shard_name())); + std::fs::write(path, serde_json::to_vec_pretty(&record).unwrap()).unwrap(); +} + +fn fixture(name: &str, count: &str) -> (Standalone, Vec) { + fixture_role(name, count, "1") +} + +fn fixture_role(name: &str, count: &str, role: &str) -> (Standalone, Vec) { + let mut node = Standalone::start(name); + node.publish_module(); + record_inputs(&node); + node.assert_call("claim_operator", &[]); + node.assert_call("install_guid_range", &["1000000"]); + node.assert_call( + "playerbots_spawn_role", + &[count, "1200", "1200", "50", role], + ); + node.assert_call("playerbots_fixture_prepare", &[]); + let bots = node + .query_rows("SELECT character_guid FROM pkg_playerbots_bot") + .iter() + .map(|r| r["character_guid"].clone()) + .collect(); + (node, bots) +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_a_long_bot_cast_waits_for_core_completion() { + let (node, bots) = fixture("playerbots-cast", "1"); + let bot = &bots[0]; + node.assert_call("playerbots_fixture_cast", &[&bot, &bot]); + let pending = node.query_rows("SELECT scheduled_id, caster_guid FROM game_pending_cast"); + assert_eq!(pending.len(), 1, "a five-second heal must leave a cast bar"); + assert_eq!( + node.query_rows(&format!( + "SELECT kind FROM game_spell_cast_event WHERE caster_guid = {bot}" + )) + .iter() + .filter(|r| r["kind"] == "1") + .count(), + 1 + ); + assert!( + node.query_rows(&format!( + "SELECT kind FROM game_spell_cast_event WHERE caster_guid = {bot}" + )) + .iter() + .all(|r| r["kind"] != "2"), + "the cast has not resolved yet" + ); + node.assert_call("playerbots_fixture_resume", &[&bot]); + for target in [bot.as_str(), "0"] { + std::thread::sleep(std::time::Duration::from_secs(1)); + node.assert_call("playerbots_fixture_cast", &[&bot, target]); + assert_eq!( + node.query_rows("SELECT scheduled_id, caster_guid FROM game_pending_cast"), + pending, + "intervening requests and brain ticks retain the same cast" + ); + } + poll_until(POLL_TIMEOUT, || { + node.query_rows("SELECT scheduled_id FROM game_pending_cast") + .is_empty() + }); + let events = node.query_rows(&format!( + "SELECT kind FROM game_spell_cast_event WHERE caster_guid = {bot}" + )); + assert_eq!(events.iter().filter(|r| r["kind"] == "2").count(), 1); + assert!( + node.query_rows(&format!( + "SELECT health FROM game_world_entity WHERE guid = {bot}" + ))[0]["health"] + .parse::() + .unwrap() + >= 50 + ); + let result = node.query_rows("SELECT outcome FROM pkg_playerbots_action"); + assert!( + result.iter().any(|r| r["outcome"] == "(castResolved = ())"), + "{result:?}" + ); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_an_expired_cast_cannot_heal_later() { + let (node, bots) = fixture("playerbots-cast-expiry", "2"); + let bot = &bots[0]; + node.assert_call("playerbots_fixture_cast", &[bot, bot]); + node.assert_call("playerbots_fixture_cancel", &[bot, "true"]); + assert!(node + .query_rows("SELECT scheduled_id FROM game_pending_cast") + .is_empty()); + assert_eq!( + node.query_rows("SELECT outcome FROM pkg_playerbots_action")[0]["outcome"], + "(expired = ())" + ); + std::thread::sleep(std::time::Duration::from_secs(6)); + let health = |guid: &str| { + node.query_rows(&format!( + "SELECT health FROM game_world_entity WHERE guid = {guid}" + ))[0]["health"] + .clone() + }; + assert_eq!( + health(bot), + health(&bots[1]), + "the expired cast adds no healing beyond natural recovery" + ); + assert_eq!( + node.query_rows("SELECT outcome FROM pkg_playerbots_action")[0]["outcome"], + "(expired = ())" + ); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_an_out_of_range_heal_reports_the_owning_cast_gate() { + let (node, bots) = fixture("playerbots-cast-range", "2"); + node.assert_call("playerbots_fixture_position", &[&bots[1], "1400"]); + node.assert_call("playerbots_fixture_cast", &[&bots[0], &bots[1]]); + assert!(node + .query_rows("SELECT scheduled_id FROM game_pending_cast") + .is_empty()); + let rows = node.query_rows("SELECT target_guid, spell_id, outcome FROM pkg_playerbots_action"); + assert_eq!(rows.len(), 1); + assert_eq!(rows[0]["target_guid"], bots[1]); + assert_eq!(rows[0]["spell_id"], "5090100"); + assert!(rows[0]["outcome"].contains("outOfRange"), "{rows:?}"); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_explicit_cancellation_removes_the_cast_and_retains_its_outcome() { + let (node, bots) = fixture("playerbots-cast-cancel", "1"); + node.assert_call("playerbots_fixture_cast", &[&bots[0], &bots[0]]); + node.assert_call("playerbots_fixture_cancel", &[&bots[0], "false"]); + assert!(node + .query_rows("SELECT scheduled_id FROM game_pending_cast") + .is_empty()); + assert_eq!( + node.query_rows("SELECT outcome FROM pkg_playerbots_action")[0]["outcome"], + "(cancelled = ())" + ); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_an_accepted_attack_without_a_hit_does_not_clear_quest_stall() { + let (node, bots) = fixture_role("playerbots-blocked-quest", "1", "0"); + let bot = &bots[0]; + node.assert_call("debug_set_nav_enabled", &["true"]); + node.assert_call("playerbots_fixture_blocked_quest", &[bot]); + poll_until(POLL_TIMEOUT, || { + !node + .query_rows("SELECT attacker_guid FROM game_melee_attack") + .is_empty() + }); + std::thread::sleep(std::time::Duration::from_secs(2)); + let actions = node.query_rows("SELECT outcome FROM pkg_playerbots_action"); + assert!( + actions + .iter() + .any(|r| r["outcome"].contains("attackAccepted")), + "{actions:?}" + ); + let quest = node.query_rows(&format!( + "SELECT counts FROM game_character_quest WHERE character_guid = {bot}" + )); + assert_eq!(quest[0]["counts"], "0"); + let read_stall = || { + node.query_rows(&format!( + "SELECT stalled_since_micros FROM pkg_playerbots_goal WHERE character_guid = {bot}" + ))[0]["stalled_since_micros"] + .clone() + }; + let before = read_stall(); + assert_ne!( + before, "0", + "accepted combat with no hit must start the no-progress clock" + ); + std::thread::sleep(std::time::Duration::from_secs(2)); + assert_eq!( + read_stall(), + before, + "another accepted attack cannot reset the clock" + ); + let target = ((0xF130u64 << 48) | (5_090_101u64 << 24) | 1).to_string(); + let health = node.query_rows(&format!( + "SELECT health, max_health FROM game_world_entity WHERE guid = {target}" + )); + assert_eq!( + health[0]["health"], health[0]["max_health"], + "accepted combat dealt no damage" + ); + node.assert_call("playerbots_fixture_move", &[bot, "1210"]); + let route = node.query_rows("SELECT outcome FROM pkg_playerbots_action"); + assert!( + route.iter().any(|r| r["outcome"].contains("blocked")), + "{route:?}" + ); + assert!(node + .query_rows(&format!( + "SELECT guid FROM game_creature_spline WHERE guid = {bot}" + )) + .is_empty()); + node.assert_call("playerbots_fixture_credit_kill", &[bot]); + poll_until(POLL_TIMEOUT, || read_stall() != before); + assert_ne!( + read_stall(), + before, + "actual objective credit ends the previous stall" + ); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_route_evidence_distinguishes_arrival_from_a_planned_leg() { + let (node, bots) = fixture("playerbots-route", "1"); + let bot = &bots[0]; + node.assert_call("debug_set_nav_enabled", &["true"]); + node.assert_call("playerbots_fixture_move", &[bot, "1210"]); + let read = || node.query_rows("SELECT outcome FROM pkg_playerbots_action"); + let first = read(); + assert!(first[0]["outcome"].contains("complete"), "{first:?}"); + assert!( + first[0]["outcome"].contains("unknown"), + "sparse seeded input cannot prove coverage: {first:?}" + ); + poll_until(POLL_TIMEOUT, || { + node.query_rows(&format!( + "SELECT x FROM game_world_entity WHERE guid = {bot}" + ))[0]["x"] + .parse::() + .unwrap() + > 1200.5 + }); + for _ in 0..4 { + node.assert_call("playerbots_fixture_move", &[bot, "1210"]); + std::thread::sleep(std::time::Duration::from_millis(600)); + } + let x = node.query_rows(&format!( + "SELECT x FROM game_world_entity WHERE guid = {bot}" + ))[0]["x"] + .parse::() + .unwrap(); + assert!((x - 1210.0).abs() < 0.1, "actual position {x}"); + node.assert_call("playerbots_fixture_move", &[bot, "1210"]); + assert!(read()[0]["outcome"].contains("arrived = true")); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_instant_casts_resolve_synchronously_and_channels_are_explicitly_refused() { + let (node, bots) = fixture("playerbots-cast-instant", "1"); + node.assert_call( + "playerbots_fixture_cast_mode", + &[&bots[0], &bots[0], "false"], + ); + assert!(node + .query_rows("SELECT scheduled_id FROM game_pending_cast") + .is_empty()); + assert_eq!( + node.query_rows("SELECT outcome FROM pkg_playerbots_action")[0]["outcome"], + "(castResolved = ())" + ); + let events = node.query_rows("SELECT kind FROM game_spell_cast_event"); + assert_eq!(events.iter().filter(|r| r["kind"] == "2").count(), 1); + node.assert_call( + "playerbots_fixture_cast_mode", + &[&bots[0], &bots[0], "true"], + ); + assert!( + node.query_rows("SELECT outcome FROM pkg_playerbots_action")[0]["outcome"] + .contains("unsupportedChannel") + ); + assert!(node + .query_rows("SELECT scheduled_id FROM game_pending_cast") + .is_empty()); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_cast_completion_rechecks_range_and_stale_handles_cannot_cancel() { + let (node, bots) = fixture("playerbots-cast-terminal-range", "2"); + node.assert_call("playerbots_fixture_cast", &[&bots[0], &bots[1]]); + let pending = node.query_rows("SELECT scheduled_id FROM game_pending_cast"); + let id = pending[0]["scheduled_id"].parse::().unwrap(); + node.assert_call( + "playerbots_fixture_cancel_handle", + &[&bots[0], &(id + 1).to_string(), "0"], + ); + node.assert_call( + "playerbots_fixture_cancel_handle", + &[&bots[0], &id.to_string(), &i64::MAX.to_string()], + ); + assert_eq!( + node.query_rows("SELECT scheduled_id FROM game_pending_cast"), + pending + ); + node.assert_call("playerbots_fixture_position", &[&bots[1], "1400"]); + poll_until(POLL_TIMEOUT, || { + node.query_rows("SELECT scheduled_id FROM game_pending_cast") + .is_empty() + }); + let result = node.query_rows("SELECT cast_id, outcome FROM pkg_playerbots_action"); + assert_eq!(result[0]["cast_id"], id.to_string()); + assert!(result[0]["outcome"].contains("outOfRange"), "{result:?}"); + assert!(node + .query_rows("SELECT kind FROM game_spell_cast_event") + .iter() + .all(|r| r["kind"] != "2")); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_a_missing_attack_target_has_a_typed_refusal() { + let (node, bots) = fixture("playerbots-attack-refusal", "1"); + node.assert_call("playerbots_fixture_attack", &[&bots[0], "999999999"]); + let result = node.query_rows("SELECT outcome FROM pkg_playerbots_action"); + assert!(result[0]["outcome"].contains("missingTarget"), "{result:?}"); + assert!(node + .query_rows("SELECT attacker_guid FROM game_melee_attack") + .is_empty()); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_a_partial_route_retains_its_waypoint_and_search_budget() { + let (node, bots) = fixture("playerbots-route-partial", "1"); + node.assert_call("debug_set_nav_enabled", &["true"]); + node.assert_call("playerbots_fixture_partial_route", &[&bots[0]]); + node.assert_call("playerbots_fixture_move", &[&bots[0], "1220"]); + let result = node.query_rows("SELECT outcome FROM pkg_playerbots_action"); + assert!(result[0]["outcome"].contains("partial"), "{result:?}"); + assert!( + result[0]["outcome"].contains("expansions = 4096"), + "{result:?}" + ); + poll_until(POLL_TIMEOUT, || { + node.query_rows(&format!( + "SELECT x FROM game_world_entity WHERE guid = {}", + bots[0] + ))[0]["x"] + .parse::() + .unwrap() + > 1200.5 + }); + record_observations(&node); +} + +fn inventory(node: &Standalone, guid: &str) -> Vec> { + let mut rows = node.query_rows(&format!( + "SELECT guid, entry, slot, stack_count FROM game_item_instance WHERE owner_guid = {guid}" + )); + rows.sort_by(|a, b| a["guid"].cmp(&b["guid"])); + rows +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_caught_turn_in_refusals_leave_inventory_and_rewards_unchanged() { + let (node, bots) = fixture("playerbots-turnin-atomic", "1"); + let bot = &bots[0]; + node.assert_call("debug_set_nav_enabled", &["true"]); + node.assert_call("playerbots_fixture_interaction_stage", &[bot, "false"]); + let before = inventory(&node, bot); + assert_eq!(before.len(), 16); + let read_player = || { + node.query_rows(&format!( + "SELECT money, xp, level FROM game_world_entity WHERE guid = {bot}" + )) + }; + let read_quest = || { + node.query_rows(&format!( + "SELECT counts, rewarded FROM game_character_quest WHERE character_guid = {bot}" + )) + }; + let player = read_player(); + let quest = read_quest(); + for choice in ["99", "0"] { + node.assert_call("playerbots_fixture_interact", &[bot, "false", choice]); + let result = node.query_rows("SELECT outcome FROM pkg_playerbots_action"); + assert!(result[0]["outcome"].contains("refused"), "{result:?}"); + if choice == "0" { + assert!(result[0]["outcome"].contains("inventoryFull"), "{result:?}"); + } + assert_eq!( + inventory(&node, bot), + before, + "caught refusal must not consume or grant an item" + ); + assert_eq!(read_player(), player); + assert_eq!(read_quest(), quest); + } + node.assert_call("playerbots_fixture_free_slot", &[bot]); + node.assert_call("playerbots_fixture_interact", &[bot, "false", "0"]); + assert_eq!( + node.query_rows("SELECT outcome FROM pkg_playerbots_action")[0]["outcome"], + "(completed = ())" + ); + assert_eq!(read_quest()[0]["rewarded"], "true"); + let items = inventory(&node, bot); + assert!(items.iter().all(|r| r["entry"] != "5090120")); + assert_eq!( + items + .iter() + .filter(|r| r["entry"] == "5090121" || r["entry"] == "5090122") + .count(), + 2 + ); + assert_eq!( + read_player()[0]["money"].parse::().unwrap(), + player[0]["money"].parse::().unwrap() + 150 + ); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_acceptance_reports_range_and_capacity_before_granting_a_quest() { + let (node, bots) = fixture("playerbots-accept-atomic", "1"); + let bot = &bots[0]; + node.assert_call("debug_set_nav_enabled", &["true"]); + node.assert_call("playerbots_fixture_interaction_stage", &[bot, "true"]); + let before = inventory(&node, bot); + node.assert_call("playerbots_fixture_position", &[bot, "1400"]); + node.assert_call("playerbots_fixture_interact", &[bot, "true", "0"]); + let result = node.query_rows("SELECT outcome FROM pkg_playerbots_action"); + assert!(result[0]["outcome"].contains("outOfRange"), "{result:?}"); + node.assert_call("playerbots_fixture_position", &[bot, "1200"]); + node.assert_call("playerbots_fixture_interact", &[bot, "true", "0"]); + let result = node.query_rows("SELECT outcome FROM pkg_playerbots_action"); + assert!(result[0]["outcome"].contains("inventoryFull"), "{result:?}"); + assert!(node + .query_rows(&format!( + "SELECT id FROM game_character_quest WHERE character_guid = {bot}" + )) + .is_empty()); + assert_eq!(inventory(&node, bot), before); + node.assert_call("playerbots_fixture_free_slot", &[bot]); + node.assert_call("playerbots_fixture_interact", &[bot, "true", "0"]); + assert_eq!( + node.query_rows("SELECT outcome FROM pkg_playerbots_action")[0]["outcome"], + "(completed = ())" + ); + assert_eq!( + node.query_rows(&format!( + "SELECT id FROM game_character_quest WHERE character_guid = {bot}" + )) + .len(), + 1 + ); + assert_eq!( + inventory(&node, bot) + .iter() + .filter(|r| r["entry"] == "5090120") + .count(), + 1 + ); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_initial_credit_observation_preserves_an_existing_stall() { + let (node, bots) = fixture_role("playerbots-credit-baseline", "1", "0"); + let bot = &bots[0]; + node.assert_call("debug_set_nav_enabled", &["true"]); + node.assert_call("playerbots_fixture_blocked_quest", &[bot]); + let stall = || { + node.query_rows(&format!( + "SELECT stalled_since_micros FROM pkg_playerbots_goal WHERE character_guid = {bot}" + ))[0]["stalled_since_micros"] + .clone() + }; + poll_until(POLL_TIMEOUT, || { + !node + .query_rows("SELECT id FROM pkg_playerbots_goal") + .is_empty() + }); + poll_until(POLL_TIMEOUT, || stall() != "0"); + node.assert_call("playerbots_fixture_freeze", &[bot]); + let before = stall(); + node.assert_call("playerbots_fixture_credit_kill", &[bot]); + assert_eq!( + node.query_rows(&format!( + "SELECT counts FROM game_character_quest WHERE character_guid = {bot}" + ))[0]["counts"], + "1" + ); + node.assert_call("playerbots_fixture_forget_credit", &[bot]); + let credit = || { + node.query_rows(&format!( + "SELECT quest_credit FROM pkg_playerbots_goal WHERE character_guid = {bot}" + ))[0]["quest_credit"] + .clone() + }; + let unknown = credit(); + node.assert_call("playerbots_fixture_resume", &[bot]); + poll_until(POLL_TIMEOUT, || credit() != unknown); + assert_eq!( + stall(), + before, + "initializing the migration default is not new objective credit" + ); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_core_cast_replacement_finishes_the_original_handle() { + let (node, bots) = fixture("playerbots-cast-replacement", "1"); + let bot = &bots[0]; + node.assert_call("playerbots_fixture_cast", &[bot, bot]); + let original = + node.query_rows("SELECT scheduled_id FROM game_pending_cast")[0]["scheduled_id"].clone(); + node.assert_call("debug_begin_cast", &[bot, "5090100", bot]); + assert_ne!( + node.query_rows("SELECT scheduled_id FROM game_pending_cast")[0]["scheduled_id"], + original + ); + let outcome = node.query_rows("SELECT cast_id, outcome FROM pkg_playerbots_action"); + assert_eq!(outcome[0]["cast_id"], original); + assert_eq!(outcome[0]["outcome"], "(cancelled = ())"); + assert_eq!( + node.query_rows("SELECT is_interrupted FROM game_spell_cast_event") + .iter() + .filter(|r| r["is_interrupted"] == "true") + .count(), + 1, + "replacement preserves the existing core interruption signal" + ); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_action_rows_are_bounded_and_deleted_with_the_character() { + let (node, bots) = fixture("playerbots-action-lifecycle", "1"); + let bot = &bots[0]; + for _ in 0..3 { + node.assert_call("playerbots_fixture_cast", &[bot, bot]); + } + assert_eq!( + node.query_rows("SELECT id FROM pkg_playerbots_action") + .len(), + 1 + ); + node.assert_call("playerbots_fixture_cancel", &[bot, "false"]); + node.assert_call("debug_delete_character", &[bot]); + assert!(node + .query_rows("SELECT id FROM pkg_playerbots_action") + .is_empty()); + assert!(node + .query_rows("SELECT scheduled_id FROM game_pending_cast") + .is_empty()); + record_observations(&node); +} + +#[test] +#[ignore = "requires SpacetimeDB, Wasm, and the playerbots Package"] +fn playerbots_a_dead_attacker_is_refused_before_engagement() { + let (node, bots) = fixture_role("playerbots-attack-dead", "1", "0"); + let bot = &bots[0]; + node.assert_call("debug_set_nav_enabled", &["true"]); + node.assert_call("playerbots_fixture_blocked_quest", &[bot]); + node.assert_call("playerbots_fixture_freeze", &[bot]); + node.assert_call("debug_set_health", &[bot, "0"]); + let target = ((0xF130u64 << 48) | (5_090_101u64 << 24) | 1).to_string(); + node.assert_call("playerbots_fixture_attack", &[bot, &target]); + let result = node.query_rows("SELECT outcome FROM pkg_playerbots_action"); + assert!( + result.iter().any(|r| r["outcome"].contains("deadActor")), + "{result:?}" + ); + assert!(node + .query_rows(&format!( + "SELECT attacker_guid FROM game_melee_attack WHERE attacker_guid = {bot}" + )) + .is_empty()); + let ranged = node.call("debug_ranged_attack_nearest", &[bot, "75"]); + assert!(!ranged.status.success()); + assert!(String::from_utf8_lossy(&ranged.stderr).contains("dead attackers cannot attack")); + record_observations(&node); +} From b70d5fd2711ea8dc3acf64a6a3a6be640f2d7687 Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 12:27:59 +0000 Subject: [PATCH 5/9] fix(actor): simplify stale cast checks for clippy --- module/src/spell/control.rs | 4 ++-- module/tests/playerbots_rewrite.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/module/src/spell/control.rs b/module/src/spell/control.rs index 389f24a5a..edf4a9aee 100644 --- a/module/src/spell/control.rs +++ b/module/src/spell/control.rs @@ -428,7 +428,7 @@ pub(crate) fn cancel_cast_attempt( caster_guid: u64, scheduled_id: u64, ) -> bool { - if !pending_cast(ctx, caster_guid).is_some_and(|cast| cast.scheduled_id == scheduled_id) { + if pending_cast(ctx, caster_guid).is_none_or(|cast| cast.scheduled_id != scheduled_id) { return false; } interrupt_cast(ctx, caster_guid) @@ -444,7 +444,7 @@ pub(crate) fn expire_cast_attempt( deadline_micros: i64, ) -> bool { if ctx.timestamp.to_micros_since_unix_epoch() < deadline_micros - || !pending_cast(ctx, caster_guid).is_some_and(|cast| cast.scheduled_id == scheduled_id) + || pending_cast(ctx, caster_guid).is_none_or(|cast| cast.scheduled_id != scheduled_id) { return false; } diff --git a/module/tests/playerbots_rewrite.rs b/module/tests/playerbots_rewrite.rs index 06cb03227..838c806f1 100644 --- a/module/tests/playerbots_rewrite.rs +++ b/module/tests/playerbots_rewrite.rs @@ -100,7 +100,7 @@ fn fixture_role(name: &str, count: &str, role: &str) -> (Standalone, Vec fn playerbots_a_long_bot_cast_waits_for_core_completion() { let (node, bots) = fixture("playerbots-cast", "1"); let bot = &bots[0]; - node.assert_call("playerbots_fixture_cast", &[&bot, &bot]); + node.assert_call("playerbots_fixture_cast", &[bot, bot]); let pending = node.query_rows("SELECT scheduled_id, caster_guid FROM game_pending_cast"); assert_eq!(pending.len(), 1, "a five-second heal must leave a cast bar"); assert_eq!( @@ -120,10 +120,10 @@ fn playerbots_a_long_bot_cast_waits_for_core_completion() { .all(|r| r["kind"] != "2"), "the cast has not resolved yet" ); - node.assert_call("playerbots_fixture_resume", &[&bot]); + node.assert_call("playerbots_fixture_resume", &[bot]); for target in [bot.as_str(), "0"] { std::thread::sleep(std::time::Duration::from_secs(1)); - node.assert_call("playerbots_fixture_cast", &[&bot, target]); + node.assert_call("playerbots_fixture_cast", &[bot, target]); assert_eq!( node.query_rows("SELECT scheduled_id, caster_guid FROM game_pending_cast"), pending, From 257abc5d7d24bb7789e62c54838ab117d28d3fb3 Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 12:30:18 +0000 Subject: [PATCH 6/9] fix(actor): declare the retained Package API compatibility boundary --- module/src/actor.rs | 5 ++++- module/src/tripwires.rs | 5 +++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/module/src/actor.rs b/module/src/actor.rs index 299a92dde..ad71ee185 100644 --- a/module/src/actor.rs +++ b/module/src/actor.rs @@ -90,7 +90,10 @@ pub(crate) fn stop_attack(ctx: &ReducerContext, actor_guid: u64) -> Result<(), S /// Compatibility adapter for callers that only need acceptance. Use `request_cast` to retain /// the scheduled identity and observe completion through `on_cast_finished`. -#[allow(dead_code)] // Retained Package API v1 operation. +#[allow( + dead_code, + reason = "Package API v1 retains the result-only cast_at adapter" +)] pub(crate) fn cast_at( ctx: &ReducerContext, actor_guid: u64, diff --git a/module/src/tripwires.rs b/module/src/tripwires.rs index 797fc209c..35c2cc127 100644 --- a/module/src/tripwires.rs +++ b/module/src/tripwires.rs @@ -2091,7 +2091,8 @@ pub(crate) mod issue_reference_tripwire { /// deliberate residues. A conditional `cfg_attr` allowance does not count here. #[cfg(test)] pub(crate) mod dead_code_allowance_tripwire { - const EXPECTED_UNCONDITIONAL_ALLOWANCES: usize = 4; + // Three taxonomy probes, the stacking boundary, and the retained Package API v1 cast_at adapter. + const EXPECTED_UNCONDITIONAL_ALLOWANCES: usize = 5; fn unconditional_dead_code_allows(content: &str) -> Vec { let mut lines = Vec::new(); @@ -2111,7 +2112,7 @@ pub(crate) mod dead_code_allowance_tripwire { } #[test] - fn module_has_exactly_four_unconditional_dead_code_allowances() { + fn module_dead_code_allowances_stay_at_declared_boundaries() { let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("src"); let mut files = Vec::new(); super::character_owned_tripwire::collect_rs_files(&root, &mut files); From a2834d88e75af38bf6935efc7cca35a9e0fb631d Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 12:35:08 +0000 Subject: [PATCH 7/9] docs(actor): clarify adjacent cast and item comments --- module/src/items/tables.rs | 6 +----- module/src/spell/cast/resolve.rs | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/module/src/items/tables.rs b/module/src/items/tables.rs index 78a1cfef3..d43936ec8 100644 --- a/module/src/items/tables.rs +++ b/module/src/items/tables.rs @@ -162,11 +162,7 @@ pub struct ItemTemplate { pub required_reputation_faction: u32, #[default(0)] pub required_reputation_rank: u32, - /// Unique-count cap (cmangos `maxcount`, 0 = no cap in-game convention but our importer/tooltip - /// treats 0 as "no limit" same as vanilla), the raw `item_template.Flags` bitmask (unique/ - /// conjured/etc — `ItemFlag` on the wire), the readable-item page-text id (`PageText` — needs a - /// page_text dump table + reader packet, deferred as its own follow-up), the quest-starter link - /// (`startquest` — 194 consumes), and the bag-type restriction bitmask (`BagFamily`). + /// Unique-count cap (`maxcount`), item flags, readable-item page, quest starter, and bag family. /// Storage enforces `max_count` across carried and banked copies. Zero means no limit. /// END-appended defaults keep existing items unrestricted. [reference] #[default(0)] diff --git a/module/src/spell/cast/resolve.rs b/module/src/spell/cast/resolve.rs index 7f5d926b6..d97180903 100644 --- a/module/src/spell/cast/resolve.rs +++ b/module/src/spell/cast/resolve.rs @@ -45,7 +45,7 @@ pub(crate) fn resolve_cast_at( // true ONLY when called from `fire_pending_cast` (a TIMED cast finishing its cast bar): written onto // the cast-GO event so the gateway emits GO+COOLDOWN WITHOUT a second SMSG_SPELL_START(0) (the START // was already sent at begin_cast). false for an instant cast, a channel, a creature cast, and a - // triggered cast — all of which still want the START(0)+GO+COOLDOWN instant packet sequence. + // triggered cast, which still use the START(0)+GO+COOLDOWN instant packet sequence. is_completion: bool, // true ONLY on the player CMSG_CAST_SPELL path (088; threaded via begin_cast from the cast_spell // reducer): the gateway sent that caster's START/RESULT/GO synchronously, so the relay suppresses From ab1983192a455fa10e23d64e49ae4abac97d99aa Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 12:37:17 +0000 Subject: [PATCH 8/9] docs(spell): align typed cast parameter comment --- module/src/spell/cast/resolve.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/module/src/spell/cast/resolve.rs b/module/src/spell/cast/resolve.rs index d97180903..a8b572d36 100644 --- a/module/src/spell/cast/resolve.rs +++ b/module/src/spell/cast/resolve.rs @@ -81,7 +81,7 @@ pub(crate) fn resolve_cast_at_typed( // true ONLY when called from `fire_pending_cast` (a TIMED cast finishing its cast bar): written onto // the cast-GO event so the gateway emits GO+COOLDOWN WITHOUT a second SMSG_SPELL_START(0) (the START // was already sent at begin_cast). false for an instant cast, a channel, a creature cast, and a - // triggered cast — all of which still want the START(0)+GO+COOLDOWN instant packet sequence. + // triggered cast, which still use the START(0)+GO+COOLDOWN instant packet sequence. is_completion: bool, // true ONLY on the player CMSG_CAST_SPELL path (088; threaded via begin_cast from the cast_spell // reducer): the gateway sent that caster's START/RESULT/GO synchronously, so the relay suppresses From fe9caef6eb0d132c45a5d151827ab6fe64925f79 Mon Sep 17 00:00:00 2001 From: T3 Code Date: Tue, 8 Sep 2026 12:56:41 +0000 Subject: [PATCH 9/9] test(playerbots): verify repeated interaction staging --- module/tests/playerbots_rewrite.rs | 40 +++++++++++++++++++++++++++--- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/module/tests/playerbots_rewrite.rs b/module/tests/playerbots_rewrite.rs index 838c806f1..068cbcb14 100644 --- a/module/tests/playerbots_rewrite.rs +++ b/module/tests/playerbots_rewrite.rs @@ -445,8 +445,38 @@ fn playerbots_caught_turn_in_refusals_leave_inventory_and_rewards_unchanged() { let bot = &bots[0]; node.assert_call("debug_set_nav_enabled", &["true"]); node.assert_call("playerbots_fixture_interaction_stage", &[bot, "false"]); + let target = ((0xF130u64 << 48) | (5_090_101u64 << 24) | 1).to_string(); + node.assert_call("playerbots_fixture_attack", &[bot, &target]); + node.assert_call( + "debug_encounter_move", + &[&target, "1303", "1200", "50", "false"], + ); + assert_eq!( + node.query_rows(&format!( + "SELECT guid FROM game_creature_spline WHERE guid = {target}" + )) + .len(), + 1 + ); + node.assert_call("playerbots_fixture_interaction_stage", &[bot, "false"]); + assert!(node + .query_rows(&format!( + "SELECT guid FROM game_creature_spline WHERE guid = {target}" + )) + .is_empty()); + assert!(node + .query_rows(&format!( + "SELECT attacker_guid FROM game_melee_attack WHERE target_guid = {target}" + )) + .is_empty()); let before = inventory(&node, bot); assert_eq!(before.len(), 16); + assert_eq!( + node.query_rows("SELECT id FROM game_creature_quest WHERE quest_entry = 50910") + .len(), + 2, + "restaging must retain exactly one relation for each quest role" + ); let read_player = || { node.query_rows(&format!( "SELECT money, xp, level FROM game_world_entity WHERE guid = {bot}" @@ -461,7 +491,8 @@ fn playerbots_caught_turn_in_refusals_leave_inventory_and_rewards_unchanged() { let quest = read_quest(); for choice in ["99", "0"] { node.assert_call("playerbots_fixture_interact", &[bot, "false", choice]); - let result = node.query_rows("SELECT outcome FROM pkg_playerbots_action"); + let result = + node.query_rows("SELECT outcome FROM pkg_playerbots_action WHERE quest_entry = 50910"); assert!(result[0]["outcome"].contains("refused"), "{result:?}"); if choice == "0" { assert!(result[0]["outcome"].contains("inventoryFull"), "{result:?}"); @@ -477,7 +508,8 @@ fn playerbots_caught_turn_in_refusals_leave_inventory_and_rewards_unchanged() { node.assert_call("playerbots_fixture_free_slot", &[bot]); node.assert_call("playerbots_fixture_interact", &[bot, "false", "0"]); assert_eq!( - node.query_rows("SELECT outcome FROM pkg_playerbots_action")[0]["outcome"], + node.query_rows("SELECT outcome FROM pkg_playerbots_action WHERE quest_entry = 50910")[0] + ["outcome"], "(completed = ())" ); assert_eq!(read_quest()[0]["rewarded"], "true"); @@ -503,7 +535,9 @@ fn playerbots_acceptance_reports_range_and_capacity_before_granting_a_quest() { let (node, bots) = fixture("playerbots-accept-atomic", "1"); let bot = &bots[0]; node.assert_call("debug_set_nav_enabled", &["true"]); - node.assert_call("playerbots_fixture_interaction_stage", &[bot, "true"]); + for _ in 0..2 { + node.assert_call("playerbots_fixture_interaction_stage", &[bot, "true"]); + } let before = inventory(&node, bot); node.assert_call("playerbots_fixture_position", &[bot, "1400"]); node.assert_call("playerbots_fixture_interact", &[bot, "true", "0"]);