diff --git a/crates/benilla-app/src/capture/probes/ground_census.rs b/crates/benilla-app/src/capture/probes/ground_census.rs index 41c5de9d..6856b4db 100644 --- a/crates/benilla-app/src/capture/probes/ground_census.rs +++ b/crates/benilla-app/src/capture/probes/ground_census.rs @@ -8,7 +8,7 @@ //! per unit: //! //! ```text -//! UGD 0x… z=5.02 seat=9.14 drop=+4.12 terrain=5.02 above=9.07 (+4.05) spline=0 swim=0 +//! UGD 0x… unit "Stormwind Guard" z=5.02 seat=9.14 drop=+4.12 terrain=5.02 above=9.07 (+4.05) spline=0 swim=0 //! ``` //! //! - **`seat`** is the server's own Z for this unit — the pose the create block / a move packet / @@ -33,13 +33,23 @@ //! The repeat form is the load-race half: a census at 30 s and again at 45 s says whether a sunk //! unit *stays* sunk once the world has finished arriving, which is what separates a transient from //! the ratchet decision 1384 removed. +//! +//! It lists exactly the bodies the clamp is responsible for ([`ground_derived`]) — since B357 that +//! includes a **player the server moves along a spline**, which the `player` tag names. That is the +//! whole readout for a Playerbots server: a bot mid-path reads `player spline=1` with a small +//! `drop`, and the bug it closes read the wire Z with no row here at all. use benilla_assets::coords::bevy_to_wow; use benilla_protocol::EntityKind; use bevy::prelude::*; use super::ProbeClock; -use crate::net::{CreatureSwimming, GroundClamped, Guid, NetEntity, SelfPlayer, Spline}; +use crate::names::NameCache; +use crate::net::NetCommands; +use crate::net::{ + ground_derived, CreatureSwimming, Embodied, GroundClamped, Guid, NetEntity, RemoteMotion, + SelfPlayer, Spline, +}; /// A drop past this (yd) counts as **sunk** — the headline count. The clamp's honest corrections are /// the small float off a slightly-high wire Z and the little hill a straight spline cuts through; @@ -96,13 +106,24 @@ type CensusQuery = ( Option<&'static GroundClamped>, Option<&'static Spline>, Has, + // The subject test's other two inputs, so the listing is exactly the clamp's own population + // rather than a second, hand-kept guess at it. + Has, + Has, ); -/// One census line per streamed `Unit` within [`GroundCensus::radius`] of the body, worst drop +/// One census line per [`ground_derived`] body within [`GroundCensus::radius`] of ours, worst drop /// first, under a summary line naming the count that matters. +#[allow(clippy::too_many_arguments)] // one Bevy system's full input set fn fire_ground_census( mut probe: ResMut, time: ProbeClock, + // The name each row belongs to. "Which NPC is sunk" is a name question — a guid sends the + // reader back to the server's own tables to answer it — and the cache is already warm: it is + // asked at first *sight* for every unit that streams in. A `None` is a name still in flight, + // never a missing one. + mut names: ResMut, + net_commands: Res, world: benilla_world::collision::WorldCollision, point: benilla_world::world_point::WorldPoint, body: Query<&Transform, With>, @@ -124,9 +145,14 @@ fn fire_ground_census( let radius2 = probe.radius * probe.radius; let mut rows: Vec<(f32, bool, String)> = Vec::new(); - for (guid, net, t, clamped, spline, swimming) in &units { - if net.kind != EntityKind::Unit - || t.translation.distance_squared(body.translation) > radius2 + for (guid, net, t, clamped, spline, swimming, embodied, relayed) in &units { + if !ground_derived( + net.kind, + spline.is_some(), + clamped.is_some(), + embodied, + relayed, + ) || t.translation.distance_squared(body.translation) > radius2 { continue; } @@ -136,13 +162,24 @@ fn fire_ground_census( let terrain = point.terrain_height_under(t.translation); let above = lowest_surface_above(&world, t.translation); let wow = bevy_to_wow(t.translation); + let name = names + .resolve(guid.0, &net_commands) + .unwrap_or("?") + .to_string(); rows.push(( drop, swimming, format!( - "UGD {:#018x} pos=({:.2},{:.2},{:.2}) z={z:.2} seat={seat:.2} drop={drop:+.2} \ - terrain={} above={} spline={} swim={}", + "UGD {:#018x} {} {name:?} pos=({:.2},{:.2},{:.2}) z={z:.2} seat={seat:.2} \ + drop={drop:+.2} terrain={} above={} spline={} swim={}", guid.0, + // Which kind this row is: since B357 the listing is not all creatures, and a + // player row is the one a Playerbots report is about. + if net.kind == EntityKind::Player { + "player" + } else { + "unit" + }, wow[0], wow[1], wow[2], diff --git a/crates/benilla-app/src/death.rs b/crates/benilla-app/src/death.rs index cb68af0b..83b60329 100644 --- a/crates/benilla-app/src/death.rs +++ b/crates/benilla-app/src/death.rs @@ -539,6 +539,7 @@ fn drain_death( net: Res, targeting: crate::ui_action::cast_target::CastTargeting, mut ladder: crate::ui_action::CastLadder, + mut ui_errors: ResMut, // The soulstone leg is a real arm-290 caller in the reference's own census (wow-re // `bind-confirm-law.md`: 290 ← UseSoulstone · UseInventoryItem · UseAction · UseContainerItem), // so it goes through the shared send carrying the same bind gate every other item use does. @@ -622,6 +623,7 @@ fn drain_death( &mut script, &mut gate, false, + &mut ui_errors, ); Ok(()) } diff --git a/crates/benilla-app/src/items.rs b/crates/benilla-app/src/items.rs index fa8be98b..48760339 100644 --- a/crates/benilla-app/src/items.rs +++ b/crates/benilla-app/src/items.rs @@ -23,7 +23,7 @@ use bevy::prelude::*; use benilla_protocol::{ItemInfo, ObjectFields}; -use crate::net::{ClientCommand, NetCommands}; +use crate::net::{ClientCommand, NetCommands, ObjectStore}; /// The slice of an item template that equipment rendering + combat animation consume (decisions /// 0072/0073): the ItemDisplayInfo key, the two placement inputs, and the weapon class pair the @@ -574,6 +574,46 @@ impl TestDeps { } } +/// Equipment slots 15/16 — `EQUIPMENT_SLOT_MAINHAND` / `_OFFHAND` (vmangos `EquipmentSlots`). +pub(crate) const EQUIPMENT_SLOT_MAINHAND: u8 = 15; +pub(crate) const EQUIPMENT_SLOT_OFFHAND: u8 = 16; + +/// One equipment slot's item class, through the guid → entry → template walk. `None` for an empty +/// slot or a template still in flight. +fn equipped_class( + store: &ObjectStore, + items: &mut Items, + commands: &NetCommands, + slot: u8, +) -> Option { + let guid = store.0.player_inv_slot(slot).filter(|&g| g != 0)?; + let entry = items.object(guid).and_then(|o| o.object_entry())?; + Some(items.template(entry, guid, commands)?.class as u8) +} + +/// **Which equipment slot this character's disarm hides** — the ladder +/// ([`crate::creature_anim::disarmed_hand`]) asked of the raw inventory rather than of the +/// resolved hands, which is the form the two *item*-side consumers need: the action bar's +/// equipped-item requirement (`0x5f0c50`, which strips the bit out of its slot mask) and the +/// item-use refusal (`CGItem::Use`'s rung 15, which compares it against the clicked item's own +/// worn position). `None` while the flag is down or neither hand holds a weapon. +/// +/// It lives here, next to the cache it reads, so the ladder is stated once and asked twice rather +/// than re-derived per consumer — the failure decision 0664 names, and the reason the quest fork +/// was once missing from all three of its call sites. +pub(crate) fn disarmed_equipment_slot( + store: &ObjectStore, + items: &mut Items, + commands: &NetCommands, +) -> Option { + if store.0.unit_flags() & crate::creature_anim::UNIT_FLAG_DISARMED == 0 { + return None; + } + let main = equipped_class(store, items, commands, EQUIPMENT_SLOT_MAINHAND); + let off = equipped_class(store, items, commands, EQUIPMENT_SLOT_OFFHAND); + crate::creature_anim::disarmed_hand(main, off).map(|hand| EQUIPMENT_SLOT_MAINHAND + hand as u8) +} + /// A minimal, VALID item template named `name` — the shared test seam for every module that /// needs a landed template (the sentinels that matter are `allowable_*` = −1 and `stackable` = 1). #[cfg(test)] diff --git a/crates/benilla-app/src/net.rs b/crates/benilla-app/src/net.rs index 5330c9ff..c7d4ee3e 100644 --- a/crates/benilla-app/src/net.rs +++ b/crates/benilla-app/src/net.rs @@ -44,7 +44,8 @@ use motion::{ // `creature_anim`, and the cross-seam test that pins the pair runs both systems in one app. pub(crate) use motion::drive_display_facing; pub(crate) use motion::{ - jump_seed, CreatureSwimming, FacingStep, RemoteMotion, Spline, SplineStopped, UnitMoveModes, + ground_derived, grounded_y, jump_seed, CreatureSwimming, FacingStep, RemoteMotion, Spline, + SplineStopped, UnitMoveModes, }; // `GroundClamped`'s only consumer outside `net::motion::spline` is the ground-census probe, and a // probe is an instrument — a build with the instruments compiled out contains nothing that names diff --git a/crates/benilla-app/src/net/apply/spells.rs b/crates/benilla-app/src/net/apply/spells.rs index 370cac88..8880c088 100644 --- a/crates/benilla-app/src/net/apply/spells.rs +++ b/crates/benilla-app/src/net/apply/spells.rs @@ -1487,6 +1487,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() @@ -1853,6 +1854,8 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() diff --git a/crates/benilla-app/src/net/motion.rs b/crates/benilla-app/src/net/motion.rs index 83cb9812..2dc7465e 100644 --- a/crates/benilla-app/src/net/motion.rs +++ b/crates/benilla-app/src/net/motion.rs @@ -45,6 +45,7 @@ pub(super) use spline::{ create_spline, ground_clamp_creatures, mark_swimming_creatures, monster_move_spline, sample_splines, trace_create_spline, trace_move_snap, }; +pub(crate) use spline::{ground_derived, grounded_y}; pub(crate) use spline::{CreatureSwimming, GroundClamped, Spline, SplineStopped}; /// The Bevy rotation of a wire **yaw** — a mover's orientation, in radians about WoW +Z. It maps diff --git a/crates/benilla-app/src/net/motion/spline.rs b/crates/benilla-app/src/net/motion/spline.rs index cd515f39..88fccd06 100644 --- a/crates/benilla-app/src/net/motion/spline.rs +++ b/crates/benilla-app/src/net/motion/spline.rs @@ -490,6 +490,73 @@ fn trace_ride(guid: u64, spline: &Spline, pos: [f32; 3], was: [f32; 3], now: Ins ); } +/// **Whose Z and swim state does benilla derive?** — the one subject test +/// [`mark_swimming_creatures`] and [`ground_clamp_creatures`] share (decision 1921, bug B357). +/// +/// Both used to ask `kind == Unit`, and **kind is the wrong question**. The reference has no such +/// test: `SMSG_MONSTER_MOVE`'s apply `0x6187a0` splices *whatever unit the packet named* into the +/// movement manager's list (`0x618801` installs the spline, `0x618808 call 0x619ca0` links it; its +/// only early-out is `6187bf test ah,0x10`, ROOT — wow-re `remote-swim-decision.md` §1.2, VERIFIED), +/// the per-frame walk `0x615b10 → 0x616620` then runs **once per frame for every registered +/// CMovement** (§1.1), and the vertical-zero gate inside it reads only that CMovement's own flag +/// word — `MI.flags & 0x4`, `MI.flags & 0x200` (the spline's FLYING bit), `[CMovement+0x40] & +/// 0x200800`, else `Δz := 0` (`0x616cec`-`0x616d03`, wow-re `collision/scratch/spec-driver-B.md` +/// K3). Not one of those bytes asks what kind of object it is holding: `mover-is-a-creature.md` +/// says it in one line — *"nothing in the movement system is 'the player's' any more … every +/// scalar, every flag, every packet is the MOVER's"*. +/// +/// So a **Player the server moves along a ground spline has its Z re-derived from the terrain +/// exactly like a creature's**. cmangos's Playerbots drives its bots that way — they are Player +/// objects moved by `SMSG_MONSTER_MOVE` — and with the kind test in place they kept the wire Z +/// verbatim and strode a body-height over the slopes (B357). Nobody saw it on vmangos because a +/// *real* remote player's movement arrives relayed, carrying that player's own client-grounded Z. +/// +/// The set, and why each member is in it: +/// - **`Unit`** — walking or idle. Decision 0059's law. +/// - **`Player`, once a server spline has moved it and until the relay takes it back.** That is +/// what `splined || derived_before` says: a bot enters on its first path and never leaves (it +/// sends no `MSG_MOVE_*`, so nothing ever hands it back), while a remote player yanked by a +/// Charge, a knockback or a fear is derived for the ride and returns to [`RemoteMotion`] on their +/// next packet. Sticky is the *reference's* shape too — `0x619ca0` splices, and nothing in the +/// recorded notes unlinks — and it is what keeps a bot grounded in the frames between paths, +/// including the one where [`sample_splines`] has just dropped the finished [`Spline`]. +/// - **Never the body we steer** ([`crate::net::Embodied`]) — *whatever kind it is*, which is the +/// half decision 1927 corrected. Our own avatar is a Player and was already out; a **possessed +/// creature** is a `Unit` and was not, so the clamp cast a ray at it every frame and wrote a Y +/// that `player::body_pose` then overwrote from the controller's own swept capsule — dead work, +/// and worse than dead once `player::server_ride` began deriving that body's ride Z too: the +/// clamp's output would have become the ride probe's input, the feedback loop 1384 removed here. +/// One body, one owner: `player::` owns everything it is attached to. +/// - **Nobody else.** A GameObject sits at its authored Z (a lamp on a table). And a player under +/// [`RemoteMotion`] is 0059's deliberate deferral: their Z arrives already grounded by the client +/// that sent it, and our dead-reckoning owns their jump arc, which a down-ray would flatten. +/// +/// The one thing that leaves with the `Embodied` exclusion: a body we are attached to but not +/// driving — a free-fly detach (`F`) parks `control` *and* the ride, so a possessed creature left +/// standing there keeps its wire Z. It is the same treatment a remote player gets, and the detach +/// is a dev affordance. +/// +/// `derived_before` is [`GroundClamped`]'s presence — the memo the clamp leaves, which exists on a +/// Player only because it was once a subject. It is membership *and* memo deliberately: the +/// alternative was a second marker with two write sites to keep in step, and the two facts are the +/// same fact. +pub(crate) fn ground_derived( + kind: EntityKind, + splined: bool, + derived_before: bool, + embodied: bool, + relayed: bool, +) -> bool { + if embodied { + return false; + } + match kind { + EntityKind::Unit => true, + EntityKind::Player => !relayed && (splined || derived_before), + _ => false, + } +} + /// Distance (yd) above a creature's current feet that the terrain probe starts. Generous enough to /// clear the small float (a slightly-high server Z) and the "little hill" a straight-line spline Z /// leaves a unit inside, low enough not to grab an overhang/bridge a unit walks *under*. It also @@ -502,6 +569,41 @@ const GROUND_CLAMP_UP: f32 = 2.5; /// *down* onto the surface. Total cast length is `GROUND_CLAMP_UP + GROUND_CLAMP_DOWN`. const GROUND_CLAMP_DOWN: f32 = 4.0; +/// **The Y a grounded mover ends its frame at** — the reference's `0x634040` walk-resolve outcome +/// plus the two granted modes that move it (decision 1780), as a pure function so the two callers +/// cannot drift apart: [`ground_clamp_creatures`] for every streamed mover, and +/// [`crate::player::server_ride`] for the body we are attached to while a server spline drives it +/// (decision 1927). Before that second caller existed, the ride simply kept the wire Z — the same +/// premise B357 corrected for bots, one layer up, and the same defect: a straight chord over a +/// hollow, measured at +0.47 yd on a 16-yd Elwynn charge. +/// +/// - `seat_y` — where the server put the body. A probe **miss** returns it unchanged: no walkable +/// surface in reach is a genuinely airborne pose, or ground that has not streamed in, and an +/// unclamped body belongs exactly where the server said. +/// - `floor` — the walkable surface the down-probe found. +/// - `water_floor` — the liquid surface, and **only** when `MOVEFLAG_WATERWALKING` makes it floor: +/// `0x63162e or edi,0x30000` ORs the two ADT liquid layers into the walk trace's class mask, so +/// one trace elects between water and terrain. Ours queries liquid separately, so the election is +/// written out as the `max` — which is what a single trace against both would have returned. +/// - `hover` — the granted HOVER bit. The reference finalises `z = z_before_snap − max(L − 1.0, 0)` +/// (`0x636e81`-`0x636ea9`), with `L` the achieved probe distance; here `L` is `seat_y − y`, so the +/// same expression is `(y + 1.0).min(seat_y)` — a clearance the body never *climbs* to. +pub(crate) fn grounded_y( + seat_y: f32, + floor: Option, + water_floor: Option, + hover: bool, +) -> f32 { + let mut y = floor.unwrap_or(seat_y); + if let Some(w) = water_floor { + y = y.max(w); + } + if hover { + y = (y + crate::player::HOVER_HEIGHT).min(seat_y); + } + y +} + /// Snap every **grounded creature** onto benilla's own terrain — the path-walkers (a ground spline) /// **and the idle ones standing at their raw spawn Z** (the "NPCs floating a bit"). The real client /// doesn't trust the wire Z for a ground unit: a walker re-derives Z from the surface (byte-verified — @@ -511,13 +613,17 @@ const GROUND_CLAMP_DOWN: f32 = 4.0; /// **walking** colliders — the same set the player stands on ([`benilla_world::collision::WorldCollision::body_filter`]) — and set the /// unit's Y to the hit. /// -/// Scope: **`Unit` creatures only** (a player owns its Z via the controller / `RemoteMotion`; a -/// GameObject sits at its authored Z — a lamp on a table, a chest on a ledge). A **flying** spline -/// keeps its own Z, and so does a **swimming** creature ([`CreatureSwimming`]): its wire Z *is* its -/// swim depth — vmangos paths a water creature in 3D through the volume with a plain non-FLYING -/// spline (verified `MoveSplineInit`/`WaypointMovementGenerator.cpp`: only `CanFly()` sets the flag), -/// so ground-clamping it dragged murlocs to the lakebed. Runs right after [`sample_splines`] so a -/// walker's freshly-sampled XY+Z is what we re-ground. +/// Scope is [`ground_derived`]: every `Unit`, plus a **`Player` the server moves** — a Playerbot, +/// or anyone under a Charge/knockback/fear — because a body the server splines has no other Z +/// authority here and the reference re-derives it the same way for either kind (B357, decision +/// 1921). A **flying** spline keeps its own Z, and so does a **swimming** mover +/// ([`CreatureSwimming`]): its wire Z *is* its swim depth — vmangos paths a water creature in 3D +/// through the volume with a plain non-FLYING spline (verified +/// `MoveSplineInit`/`WaypointMovementGenerator.cpp`: only `CanFly()` sets the flag), so +/// ground-clamping it dragged murlocs to the lakebed, and the reference agrees at the bytes (the +/// vertical-zero gate keeps Δz whenever `[CMovement+0x40] & 0x200000` SWIMMING is set — +/// `0x616cfa`). Runs right after [`sample_splines`] so a walker's freshly-sampled XY+Z is what we +/// re-ground. /// /// **The probe measures from the unit's SEAT, never from the clamp's own last answer** (decision /// 1384). The seat is the Z whoever owns this unit's position last wrote — the create block, an @@ -549,6 +655,10 @@ pub(in crate::net) fn ground_clamp_creatures( // rests the body a yard clear of the floor it found, and WATERWALKING makes the liquid // surface one of the floors it can find. Option<&crate::net::UnitMoveModes>, + // [`ground_derived`]'s other two inputs — the two OTHER Z authorities a body can have: + // the controller (the body we steer) and the relay stream's dead-reckoning. + Has, + Has, )>, ) { let cost = clamp_cost_enabled(); @@ -561,11 +671,17 @@ pub(in crate::net) fn ground_clamp_creatures( // `armed` means the collider set is churning and the gate is holding nothing. let (mut reseat, mut armed) = (0u32, 0u32); - for (entity, net, spline, mut t, mut clamped, swimming, modes) in &mut q { + for (entity, net, spline, mut t, mut clamped, swimming, modes, embodied, relayed) in &mut q { visited += 1; - if net.kind != EntityKind::Unit { + if !ground_derived( + net.kind, + spline.is_some(), + clamped.is_some(), + embodied, + relayed, + ) { skipped += 1; - continue; // players + GameObjects own their Z + continue; // somebody else owns this body's Z — see the predicate } if spline.is_some_and(|s| !s.grounded) { skipped += 1; @@ -599,11 +715,21 @@ pub(in crate::net) fn ground_clamp_creatures( // unit was created moves the epoch and re-arms it (decision 1384 — before the epoch was in // the gate, that unit's wrong answer was cached for the session). A MISS never caches, so a // unit standing on a tile whose collider hasn't streamed in keeps asking until it lands. + // + // **And the answer must still be STANDING, not merely unchanged** (decision 1921). The + // seat compare `c.seat_y == seat_y` cannot see the one case where those two come apart: + // somebody rewrites this unit's Y to a pose whose *seat* equals the cached one, which is + // exactly what a re-sent identical wire pose is (a periodic `SMSG_UPDATE_OBJECT`, a stop + // packet restating where the unit already was). The question was unchanged, so the gate + // held — and left the unit standing at the raw wire Z it had just been handed, un-grounded + // until something moved it. `c.y_written == t.translation.y` is the strictly stronger + // test: it *implies* the seat compare (the derivation above reads `c.seat_y` on precisely + // that condition) and additionally asserts our own answer is the Y still under the unit. if let Some(c) = clamped.as_deref() { let same_question = if legacy { c.y_written == t.translation.y } else { - c.seat_y == seat_y && c.epoch == epoch.get() && c.modes == granted + c.y_written == t.translation.y && c.epoch == epoch.get() && c.modes == granted }; if c.hit && c.xz == xz && same_question { held += 1; @@ -624,43 +750,27 @@ pub(in crate::net) fn ground_clamp_creatures( // face whose winding points away is no floor, or an idle NPC would stand mid-air on the // very shell face the player mover now falls through. let hit = world.ray_body(origin, Dir3::NEG_Y, reach); - // The down-ray's hit point Y = feet on the surface; no surface in reach = the seat, i.e. - // where the server put it. - let mut y = match &hit { - Some(h) => { - hit_n += 1; - origin.y - h.distance - } - None => seat_y, - }; - // **Water walking** (decision 1780): `MOVEFLAG_WATERWALKING` ORs the two ADT liquid layers - // into the reference's walk trace mask (`0x63162e or edi,0x30000`), so the surface simply - // *is* floor and the one trace elects between it and the terrain. Ours queries liquid - // separately, so the election is written out: the higher of the two wins, which is what a - // single trace against both would have returned. Reached only by a creature the swim mark - // has already let through — the reference takes this arm only when the swim bit is clear - // (wow-re `moveflag-family.md` §2.2), and a swimming creature `continue`d above. - if granted.water_walking() { + let floor = hit.as_ref().map(|h| { + hit_n += 1; + origin.y - h.distance + }); + // The water-walker's floor, and **only** for a creature the swim mark already let through: + // the reference takes this arm only when the swim bit is clear (wow-re + // `moveflag-family.md` §2.2), and a swimming creature `continue`d above. + // + // The reference's second, rate-limited hover pass (`0x636fa1`, climbing back toward the + // clearance at 7 yd/s) is deliberately NOT reproduced in [`grounded_y`]: this clamp is a + // pure function of (server pose, colliders) by construction, which is what makes its cache + // gate sound (decision 1384), and a per-frame ramp is state. A creature's seat is the + // server's, so the static answer is the one it converges to anyway. + let water = granted.water_walking().then(|| { let wow = bevy_to_wow(t.translation); let who = benilla_world::world_point::Subject::Unit(entity); - if let Some(l) = points.liquid_at(who, wow) { - y = y.max(t.translation.y + (l.surface_z - wow[2])); - } - } - // **Hover** (decision 1780): the body rests [`crate::player::HOVER_HEIGHT`] above the floor - // — and, faithfully, **does not descend at all when the floor is under a yard away**. The - // reference's finalize is `z_final = z_before_snap − max(L − 1.0, 0)` (`0x636e81`-`0x636ea9`, - // wow-re `moveflag-family.md` §4.1), with `L` the achieved down-probe distance; here `L` is - // `seat_y − y`, so the same expression is `(y + 1.0).min(seat_y)`. - // - // The reference's second, rate-limited pass (`0x636fa1`, climbing back toward the clearance - // at 7 yd/s) is deliberately NOT reproduced: this clamp is a pure function of (server pose, - // colliders) by construction, which is what makes its cache gate sound (decision 1384), and - // a per-frame ramp is state. A creature's seat is the server's, so the static answer is the - // one it converges to anyway. - if granted.hovering() { - y = (y + crate::player::HOVER_HEIGHT).min(seat_y); - } + points + .liquid_at(who, wow) + .map(|l| t.translation.y + (l.surface_z - wow[2])) + }); + let y = grounded_y(seat_y, floor, water.flatten(), granted.hovering()); // Exact bit equality is deliberate, not a sloppy float compare: the question is "would the // write change anything" — Bevy's change detection fires on the DerefMut regardless of // value, so writing an equal Y every frame marked every standing creature's transform @@ -750,6 +860,13 @@ type SwimMarkQuery = ( Option<&'static ObjectStore>, Has, Has, + // [`ground_derived`]'s four other inputs — this mark and the clamp walk the same population, + // and must, or a server-splined body in water would be dragged to the lakebed by a clamp its + // own swim state was never derived to exempt it from (B357). + Has, + Has, + Has, + Has, ); /// This unit has been through [`mark_swimming_creatures`] with its room claim settled — the @@ -769,10 +886,14 @@ pub(in crate::net) struct SwimEvaluated; /// collision height, so it is subtracted from the boundary rather than scaled with it. const CREATURE_SWIM_EXIT_BAND: f32 = 1.0 / 36.0; -/// A **creature** currently past the swim boundary — the client-side derivation of the swim state -/// the wire never carries for creatures: vmangos sets `MOVEFLAG_SWIMMING` only from player packets, -/// and a water creature's create block + splines carry no swim marker at all (verified -/// `vmangos-src`, 2026-07-17). The real client runs its own depth decision `0x6030c0` on **remote +/// A **server-moved body** ([`ground_derived`]) currently past the swim boundary — the client-side +/// derivation of the swim state the wire never carries for one: vmangos sets `MOVEFLAG_SWIMMING` +/// only from player packets, and a water creature's create block + splines carry no swim marker at +/// all (verified `vmangos-src`, 2026-07-17). Neither does a **spline-driven player** — a Playerbot +/// sends no `MSG_MOVE_*` of its own and `SMSG_MONSTER_MOVE` carries no flag word, so the same +/// nothing arrives for it, and it is derived here for the same reason (B357, decision 1921). The +/// gate below admits it: bit 3 PLAYER_CONTROLLED is set on every player object, so +/// [`crate::player::may_swim`] is true. The real client runs its own depth decision `0x6030c0` on **remote /// units and creatures too**, not only on the body it steers — VERIFIED, wow-re /// `collision/scratch/remote-swim-decision.md` §1. `0x6030c0` has exactly one caller and it reaches /// per-unit three ways: a **per-frame registry walk** (`0x616800`, a registered frame callback → @@ -881,9 +1002,12 @@ fn creature_swim_state(marked: bool, depth: f32, boundary: f32, unit_flags: u32) } } -/// Maintain [`CreatureSwimming`] on `Unit` creatures from the water over their feet (module docs on -/// the INTERIM boundary). Runs chained before [`ground_clamp_creatures`] so a fresh mark exempts the -/// clamp the same frame (Bevy inserts the deferred-command sync point for the chain). +/// Maintain [`CreatureSwimming`] on every [`ground_derived`] body from the water over its feet +/// (module docs on the INTERIM boundary). Runs chained before [`ground_clamp_creatures`] so a fresh +/// mark exempts the clamp the same frame (Bevy inserts the deferred-command sync point for the +/// chain) — and walks **the same population**, which is the half B357 would have broken silently: +/// admit a splined player to the clamp without deriving its swim state and a bot swimming a lake +/// is dragged to the bed, the murloc bug wearing a different body. pub(in crate::net) fn mark_swimming_creatures( mut commands: Commands, units: Query, @@ -901,9 +1025,11 @@ pub(in crate::net) fn mark_swimming_creatures( commands.entity(e).remove::(); } } - for (e, net, t, collision, store, marked, evaluated) in &units { - if net.kind != EntityKind::Unit { - continue; // players carry the real flag on the wire; GameObjects don't swim + for (e, net, t, collision, store, marked, evaluated, splined, clamped, embodied, relayed) in + &units + { + if !ground_derived(net.kind, splined, clamped, embodied, relayed) { + continue; // GameObjects don't swim; a relayed player carries the real flag on the wire } // The unit's OWN room decides whose liquid answers (0696). Before it, both sources // answered: Undercity's NPCs read Tirisfal's ADT water 95 yd over their heads and swam on @@ -1108,6 +1234,37 @@ mod under_floor { ); } + /// **A re-sent identical pose must not be held at the wire Z** (decision 1921). The cache gate + /// asks whether the ray's inputs changed; until this test it did not also ask whether its own + /// answer was still *standing*, so a unit whose Y was rewritten to a pose with the **same seat** + /// — which is exactly what a periodic `SMSG_UPDATE_OBJECT` or a stop packet restating a + /// stationary unit's position is — matched the cached question and was left floating at the + /// wire Z until something moved it. + /// + /// Found by B357's reproduction, on a bot between two paths; the defect is the creature's too, + /// which is why it is pinned here. + #[test] + fn a_pose_re_sent_unchanged_is_re_grounded_not_held() { + let (mut app, npc) = half_arrived_world(); + clamp(&mut app); + assert_eq!(y_of(&app, npc), TERRAIN_Y); + + // The wire restates the pose it already sent: same XZ, same Z, so the same seat — and a + // standing Y that is no longer the one the clamp left. + app.world_mut() + .entity_mut(npc) + .get_mut::() + .unwrap() + .translation + .y = WIRE_Y; + clamp(&mut app); + assert_eq!( + y_of(&app, npc), + TERRAIN_Y, + "the question was unchanged, but the answer was no longer applied" + ); + } + /// The water-walker's floor election, and the one that proves it is an *election*: the surface /// wins over a lakebed below it, and loses to ground above it. The reference gets this for /// free — `MOVEFLAG_WATERWALKING` ORs the liquid layers into the walk trace's class mask @@ -1436,3 +1593,370 @@ mod swim_gate { )); } } + +/// **B357, in a world small enough to assert on**: a Playerbot walking a hollow, driven by a +/// grounded server spline, and the four bodies that must *not* be dragged along with it. +/// +/// The reported picture is an orc a body-height clear of a snow slope in mid-stride, and this is +/// the mechanism behind it in three facts: the server's waypoints sit on the ground at the two +/// rims, the spline between them is a **straight chord**, and a hollow is exactly where a chord +/// departs from the surface. A creature riding it is put back on the ground every frame; a Player +/// riding the identical path was skipped by a `kind == Unit` test and kept the chord's Z verbatim. +/// +/// The trench is trapezoidal rather than a V on purpose: its floor is flat under the whole +/// mid-path window, so every assertion below is an exact equality that no sampling instant can +/// move. +#[cfg(test)] +mod server_moved_players { + use std::time::{Duration, Instant}; + + use avian3d::prelude::*; + use benilla_assets::coords::wow_to_bevy; + use benilla_protocol::EntityKind; + use bevy::ecs::system::RunSystemOnce; + use bevy::prelude::*; + + use super::{ground_clamp_creatures, ground_derived, CreatureSwimming, GroundClamped, Spline}; + use crate::net::{Embodied, NetEntity, RemoteMotion}; + use benilla_world::collision::ColliderEpoch; + + /// The rims the server's two waypoints stand on, and the trench floor between them. The 2.5-yd + /// difference is the mid-stride error the report photographed — a body-height, and well inside + /// the probe's own reach so what the test measures is the subject rule, not the window. + const RIM_Z: f32 = 10.0; + const FLOOR_Y: f32 = 7.5; + + /// The two server waypoints, raw WoW. Both stand on a rim, so the chord between them is + /// **level at [`RIM_Z`] for its whole length** — which is what makes every assertion here + /// independent of the instant the spline is sampled at. + const RIM_A: [f32; 3] = [0.0, 9.0, RIM_Z]; + const RIM_B: [f32; 3] = [0.0, -9.0, RIM_Z]; + + /// A ground strip: the `(bevy x, bevy y)` profile extruded ±5 along Bevy Z, wound so its faces + /// point up — the one-sided down-ray (decision 0970) stands only on those. + fn ground_strip(app: &mut App, profile: &[(f32, f32)]) -> Entity { + let mut verts = Vec::new(); + let mut tris = Vec::new(); + for (i, &(x, y)) in profile.iter().enumerate() { + verts.push(Vec3::new(x, y, -5.0)); + verts.push(Vec3::new(x, y, 5.0)); + if i > 0 { + let b = (i as u32 - 1) * 2; + tris.push([b, b + 1, b + 3]); + tris.push([b, b + 3, b + 2]); + } + } + app.world_mut() + .spawn(( + RigidBody::Static, + Collider::trimesh(verts, tris), + Transform::default(), + )) + .id() + } + + /// The hollow, and one body standing on the chord over its floor. `spawn` says what kind of + /// body and what else is driving it; the pose is the one [`sample_splines`] writes mid-path. + fn hollow(kind: EntityKind) -> (App, Entity) { + let mut app = App::new(); + app.init_resource::(); + app.add_plugins(( + MinimalPlugins, + bevy::transform::TransformPlugin, + bevy::asset::AssetPlugin::default(), + bevy::scene::ScenePlugin, + PhysicsPlugins::new(bevy::app::PostUpdate), + )); + app.init_asset::().init_resource::(); + benilla_world::world_point::init_world_point_resources(app.world_mut()); + app.finish(); + app.cleanup(); + // rim ─╮ ╭─ rim + // ╰────────╯ the flat floor the chord flies over + ground_strip( + &mut app, + &[(-9.0, RIM_Z), (-3.0, FLOOR_Y), (3.0, FLOOR_Y), (9.0, RIM_Z)], + ); + let body = app + .world_mut() + .spawn(( + NetEntity { + kind, + display_id: None, + scale: 1.0, + }, + // Mid-chord: what the sampler writes half-way between the two rims. + Transform::from_translation(wow_to_bevy([0.0, 0.0, RIM_Z])), + )) + .id(); + app.update(); // seats Position/Rotation and the collider trees + (app, body) + } + + /// The path the server has this body on — a plain ground walk, the shape `SMSG_MONSTER_MOVE` + /// sends for a bot and for a charged/knocked-back/feared player alike. + fn walking(app: &mut App, e: Entity) { + app.world_mut().entity_mut(e).insert(Spline { + points: vec![RIM_A, RIM_B], + start: Instant::now(), + duration: Duration::from_secs(4), + id: 77, + grounded: true, + run_mode: true, + }); + } + + fn relayed() -> RemoteMotion { + RemoteMotion { + wow_pos: [0.0, 0.0, RIM_Z], + orientation: 0.0, + flags: crate::creature_anim::move_flags::FORWARD, + pitch: 0.0, + speed: 7.0, + vertical_velocity: 0.0, + jump_xy_vel: [0.0; 2], + fall_start_z: None, + pending: std::collections::VecDeque::new(), + relay: Default::default(), + last_apply_ms: 0.0, + last_apply_pos: [0.0; 3], + } + } + + fn clamp(app: &mut App) { + app.world_mut() + .run_system_once(ground_clamp_creatures) + .expect("run the clamp"); + } + + fn y_of(app: &App, e: Entity) -> f32 { + app.world().get::(e).unwrap().translation.y + } + + /// Standing on the trench floor, to a millimetre. The clamp answers `origin.y − hit.distance`, + /// so its result carries the ray's own float error (7.4999995 for this geometry) — and the + /// thing under test is a 2.5-yd wire-vs-terrain disagreement, which no tolerance this side of a + /// yard could hide. The *seat* assertions stay exact: a seat is copied, never computed. + #[track_caller] + fn assert_grounded(app: &App, e: Entity, why: &str) { + let y = y_of(app, e); + assert!( + (y - FLOOR_Y).abs() < 1e-3, + "{why}: expected the trench floor {FLOOR_Y}, got {y}" + ); + } + + /// **The report.** A Player object the server walks along a ground spline is grounded exactly + /// like the creature on the identical path — and the creature is asserted beside it, because + /// "the same answer for both kinds" is the whole claim. + #[test] + fn a_bot_walking_a_hollow_is_put_on_the_ground_not_the_chord() { + for kind in [EntityKind::Player, EntityKind::Unit] { + let (mut app, body) = hollow(kind); + walking(&mut app, body); + assert_eq!( + y_of(&app, body), + RIM_Z, + "{kind:?}: the chord is where the server's spline puts it — the symptom" + ); + + clamp(&mut app); + assert_grounded( + &app, + body, + &format!("{kind:?}: a grounded spline's Z is the terrain's, not the wire's"), + ); + assert_eq!( + app.world().get::(body).unwrap().seat_y, + RIM_Z, + "{kind:?}: the seat stays the server's pose, never the answer we just wrote" + ); + } + } + + /// **A flying path keeps its own Z, for a player too.** The reference's vertical-zero gate + /// keeps Δz when the spline carries `FLYING` (`0x616cec` reads the MI flag word, not the + /// object's kind), so a taxi is not walked into the hillside it passes over. + #[test] + fn a_flying_path_is_left_alone() { + let (mut app, body) = hollow(EntityKind::Player); + app.world_mut().entity_mut(body).insert(Spline { + points: vec![RIM_A, RIM_B], + start: Instant::now(), + duration: Duration::from_secs(4), + id: 78, + grounded: false, + run_mode: true, + }); + clamp(&mut app); + assert_eq!(y_of(&app, body), RIM_Z, "a flight owns its own altitude"); + } + + /// **A bot does not pop back onto the chord between paths.** `sample_splines` drops a finished + /// [`Spline`] before the clamp runs, so on the last frame of every path the live-spline half of + /// the subject rule is already false; membership is carried by the [`GroundClamped`] memo the + /// ride left. That is the reference's shape as well — `0x619ca0` splices the CMovement into the + /// per-frame list and nothing in the recorded notes unlinks it. + #[test] + fn a_bot_stays_grounded_between_paths() { + let (mut app, body) = hollow(EntityKind::Player); + walking(&mut app, body); + clamp(&mut app); + assert_grounded(&app, body, "mid-path"); + + // The path finished: the sampler has taken the spline away. + app.world_mut().entity_mut(body).remove::(); + // The server puts it somewhere new — a fresh pose, still on the chord's level. + app.world_mut() + .entity_mut(body) + .get_mut::() + .unwrap() + .translation + .y = RIM_Z; + clamp(&mut app); + assert_grounded( + &app, + body, + "a body the server has moved once stays ours to ground until the relay takes it back", + ); + } + + /// **The body we steer is the controller's**, spline or no spline. Our own Charge rides + /// `player::server_ride`, whose pose the swept capsule owns; a second opinion from a down-ray + /// would be two systems writing one Y. + #[test] + fn the_body_we_steer_is_left_to_the_controller() { + let (mut app, body) = hollow(EntityKind::Player); + walking(&mut app, body); + app.world_mut().entity_mut(body).insert(Embodied); + clamp(&mut app); + assert_eq!(y_of(&app, body), RIM_Z, "the controller owns its own body"); + assert!( + app.world().get::(body).is_none(), + "and it never joins the derived set, so it cannot inherit membership later" + ); + } + + /// **A relayed remote player is the relay's** — decision 0059's deliberate deferral, kept. Their + /// Z arrives already grounded by the client that sent it, and the dead-reckon owns their jump + /// arc, which a down-ray would flatten. Both halves are asserted: one who has just streamed in + /// and moved nobody's way yet, and one under a live [`RemoteMotion`]. + #[test] + fn a_relayed_player_is_left_to_the_relay() { + let (mut app, body) = hollow(EntityKind::Player); + clamp(&mut app); + assert_eq!( + y_of(&app, body), + RIM_Z, + "streamed in, never moved by anyone: not ours to ground" + ); + + app.world_mut().entity_mut(body).insert(relayed()); + walking(&mut app, body); + clamp(&mut app); + assert_eq!( + y_of(&app, body), + RIM_Z, + "the relay stream outranks a spline that has not displaced it" + ); + } + + /// **A swimming body keeps its wire Z**, and a Player is now one of the bodies that can be + /// marked. This is the half B357's fix would have broken silently: admit a splined player to + /// the clamp without deriving its swim state and a bot crossing a lake is dragged to the bed. + #[test] + fn a_swimming_bot_keeps_its_wire_z() { + let (mut app, body) = hollow(EntityKind::Player); + walking(&mut app, body); + app.world_mut().entity_mut(body).insert(CreatureSwimming); + clamp(&mut app); + assert_eq!( + y_of(&app, body), + RIM_Z, + "in liquid the wire Z IS the depth — the reference keeps Δz on SWIMMING too" + ); + } + + /// **The swim mark walks the clamp's population, not a second one.** Pinned through its EXIT + /// leg, which needs no liquid: in a dry world a marked body must be unmarked, and only a body + /// the mark actually visits can be. The relayed player beside it is the control — its swim flag + /// arrives on the wire, so deriving a second answer for it is not ours to do. + /// + /// The mark's own depth law (`creature_swim_state`) is pinned as a pure function elsewhere; + /// what is new here is *who* it is asked about — and that a player passes the flag gate at all, + /// which is the last assertion: bit 3 PLAYER_CONTROLLED is set on every player object. + #[test] + fn the_swim_mark_reaches_a_bot_and_not_a_relayed_player() { + let (mut app, bot) = hollow(EntityKind::Player); + walking(&mut app, bot); + app.world_mut().entity_mut(bot).insert(CreatureSwimming); + + let stream = app + .world_mut() + .spawn(( + NetEntity { + kind: EntityKind::Player, + display_id: None, + scale: 1.0, + }, + Transform::from_translation(wow_to_bevy([0.0, 0.0, RIM_Z])), + CreatureSwimming, + relayed(), + )) + .id(); + + app.world_mut() + .run_system_once(super::mark_swimming_creatures) + .expect("run the swim mark"); + + assert!( + app.world().get::(bot).is_none(), + "a splined player is the mark's to derive — dry ground unmarks it" + ); + assert!( + app.world().get::(stream).is_some(), + "a relayed player's swim state is the wire's; the mark must not touch it" + ); + assert!( + crate::player::may_swim(0x8), + "PLAYER_CONTROLLED alone admits a player to the depth decision (0x60310b bit 3)" + ); + } + + /// The subject rule as a table, including the rows no world above can reach: a GameObject + /// (authored Z — a lamp on a table) and the two ways a Player leaves the set again. + #[test] + fn the_subject_rule() { + let unit = |splined, memo, embodied, relayed| { + ground_derived(EntityKind::Unit, splined, memo, embodied, relayed) + }; + let player = |splined, memo, embodied, relayed| { + ground_derived(EntityKind::Player, splined, memo, embodied, relayed) + }; + // A creature is ours walking or idle — decision 0059's law, untouched. + assert!(unit(true, true, false, false)); + assert!(unit(false, false, false, false)); + // …until we are steering it. A possessed creature is the controller's (decision 1927). + assert!(!unit(true, true, true, false)); + assert!(!unit(false, false, true, false)); + + assert!(player(true, false, false, false), "a bot's first path"); + assert!( + player(false, true, false, false), + "and every frame after it" + ); + assert!( + !player(false, false, false, false), + "before anything moves it" + ); + assert!(!player(true, true, true, false), "the body we steer"); + assert!(!player(true, true, false, true), "under the relay stream"); + + for kind in [EntityKind::GameObject, EntityKind::Corpse] { + assert!( + !ground_derived(kind, true, true, false, false), + "{kind:?} sits at the Z it was authored at" + ); + } + } +} diff --git a/crates/benilla-app/src/player.rs b/crates/benilla-app/src/player.rs index 24707d13..c9822cb0 100644 --- a/crates/benilla-app/src/player.rs +++ b/crates/benilla-app/src/player.rs @@ -147,9 +147,21 @@ pub(crate) use view_subject::ViewSubject; /// pivot. A pure root (Frost Nova, Entangling Roots) sets only the first — which is why a rooted /// player can still turn and a stunned one cannot, the distinction B179 was reporting. /// -/// It has a second consumer beyond the turn: the idle/fidget selectors bail on it -/// (`0x5eb4f2`/`0x5ec219`), which is what stops even the idle twitch — see -/// [`crate::creature_anim::MovementState::stunned`] (decision 0880). +/// **It has no second consumer — the "idle/fidget" one was a misread, and this doc outlived the +/// correction.** It used to say the idle/fidget selectors bail on the bit at `0x5eb4f2`/`0x5ec219` +/// and pointed at a `MovementState::stunned` that decision **0889** had already deleted along with +/// the gate: re-read at the bytes, those two addresses are inside **`ToggleSheath 0x5eb480`** and +/// **`CanLootNow 0x5ec110`**, and **no site in the whole `0x40000` census touches animation +/// selection**. What stops the idle twitch under a stun is the animation clock being stopped +/// (0889's proc-11 freeze), not this flag. The claim came in quoted verbatim from wow-re +/// `object-layer/scratch/unit-flags-movement-gates.md` §2/§3, which is being corrected there too — +/// a stale line in one repo re-entering the other is exactly the failure mode both contracts warn +/// about, and it survived here for a fortnight because a doc comment is not a gate. +/// +/// The census's other movement-relevant consumers are real and stay: the two relayed +/// `MSG_MOVE_*` wrappers that refuse outright (`0x602b20` StartTurn, `0x602b80` StartPitch — the +/// only two of twelve carrying a stun test), and the auto-face-target smoothing suppression at +/// `0x600dd7`. pub(crate) const UNIT_FLAG_STUNNED: u32 = 0x0004_0000; /// `UNIT_FLAG_IN_COMBAT` — the same `UNIT_FIELD_FLAGS` word, **bit 19** (vmangos diff --git a/crates/benilla-app/src/player/move_trace.rs b/crates/benilla-app/src/player/move_trace.rs index 2fba2d68..6545a231 100644 --- a/crates/benilla-app/src/player/move_trace.rs +++ b/crates/benilla-app/src/player/move_trace.rs @@ -257,6 +257,57 @@ pub(super) fn settle(resident: bool, waited: f32, pos: bevy::prelude::Vec3) { ); } +/// One `rid` line per frame of a **self-spline ride** — the Charge/knockback/fear/taxi instrument, +/// and the A/B that measured decision 1927. +/// +/// It exists because the ride is the one mover in the client the `move` trace cannot see: the +/// controller — and with it the mover's own per-frame line — is parked behind `control`'s ride +/// guard for the whole ride, so a charge across a hillside left no record at all. +/// +/// The three numbers, and why all three: +/// - **`wire`** — the Z `sample_splines` put us at, i.e. the server's chord. +/// - **`ground`** — the walkable surface under that XZ. `miss` is not a failure: nothing in reach +/// is a genuinely airborne pose or ground that has not streamed in, and the ride keeps its own Z +/// there, the same answer the creature clamp gives. +/// - **`z`** — where the ride actually left us, and `gap` is `z − ground`: **the number that says +/// whether the ride is riding the world or the wire.** +/// +/// `chord` is `wire − ground` — the error the wire carried, which stays visible after the fix so +/// the instrument still measures the defect it closed rather than reporting a flat zero. A charge +/// down a gentle Elwynn slope reads `chord` rising to ~0.5 yd at mid-path while `gap` holds at 0; +/// **before 1927 the two were the same number**, because `z` was `wire`. +pub(super) fn ride( + spline_id: u32, + grounded: bool, + pos: bevy::prelude::Vec3, + wire_y: f32, + ground: Option, +) { + if !trace::enabled_for("rid") { + return; + } + let wow = benilla_assets::coords::bevy_to_wow(pos); + trace::line( + "rid", + &format!( + "{spline_id} {} pos=({:8.2},{:8.2},{:7.2}) z={:7.2} wire={wire_y:7.2} {}", + if grounded { "ground" } else { "flying" }, + wow[0], + wow[1], + wow[2], + pos.y, + match ground { + Some(g) => format!( + "ground={g:7.2} gap={:+7.3} chord={:+7.3}", + pos.y - g, + wire_y - g + ), + None => "ground=miss".to_string(), + }, + ), + ); +} + static PREV_GROUNDED: AtomicBool = AtomicBool::new(true); pub(super) fn frame(f: Frame) { diff --git a/crates/benilla-app/src/player/server_ride.rs b/crates/benilla-app/src/player/server_ride.rs index 8a645a60..cf9009b7 100644 --- a/crates/benilla-app/src/player/server_ride.rs +++ b/crates/benilla-app/src/player/server_ride.rs @@ -11,8 +11,9 @@ //! //! Division of labour each frame while riding: //! - [`crate::net`]'s `sample_splines` (Net stage) advances the [`Spline`] into the entity -//! `Transform` — the sole position authority for the ride. A player owns its Z (no creature -//! terrain-reground), and the spline's ground path already carries a walkable Z. +//! `Transform` — the horizontal authority for the ride. **Its Z is not authoritative** and never +//! was (decision 1927): the reference discards a grounded spline's vertical exactly as it does a +//! creature's, and [`ride_z`] re-derives it from the ground under us. //! - [`drive_self_ride`] (Input stage, *before* `control`) owns the *whole* pose while the ride //! lasts: it mirrors that transform into [`Player`] (`pos`/`face_yaw`/`model_yaw`), drives a //! forward-run animation via [`MovementState`], unwinds the strafe counter-twist ([`BodyTwist`]), @@ -29,6 +30,66 @@ use crate::net::{ClientCommand, Embodied, NetCommands, Spline, SplineStopped}; use super::Player; +/// The walkable surface under a ride's pose, measured the way every other mover's ground is — +/// a one-sided down-ray through the same window `net::motion::spline`'s clamp uses, from the +/// server's own Z rather than from any answer of ours. `None` = nothing in reach. +fn ride_ground(world: &benilla_world::collision::WorldCollision, pos: Vec3) -> Option { + const UP: f32 = 2.5; + const DOWN: f32 = 4.0; + let origin = Vec3::new(pos.x, pos.y + UP, pos.z); + world + .ray_body(origin, Dir3::NEG_Y, UP + DOWN) + .map(|h| origin.y - h.distance) +} + +/// **The Z the body we are attached to rides at** — the ground under the spline's XZ, not the +/// spline's own vertical (decision 1927). +/// +/// The reference's integrate loop makes no distinction between the body it steers and any other +/// mover here: `0x616de0`'s path-select produces a displacement from *either* the physics path or +/// the spline path and hands **both** to `0x616cb0` (wow-re `collision/scratch/spec-driver-B.md` +/// K5/K3), which zeroes the vertical component and lets `0x634040`'s swept resolve read Z off the +/// surface. Keeping the chord instead is what B357 fixed for a Playerbot; this is the same wire Z, +/// on the one body that had it left. +/// +/// **The keeps are the reference's own fork**, `0x616cec`-`0x616d03`: +/// - a **flying spline** (`MI.flags & 0x200`) — the taxi owns its altitude, and its sampled Z is +/// the flight path; +/// - **`SWIMMING`** (`[CMovement+0x40] & 0x200000`, one of the two bits at `0x616cfa`) — in liquid +/// the wire Z *is* the depth, the same exemption the creature clamp takes; +/// - a probe **miss** — nothing walkable in reach, so the body stays where the server put it. +/// +/// The other bit at `0x616cfa` (`0x800`) has no expression here: our ride carries no live +/// CMovement flag word of its own, and a knockback — the airborne case that would want it — is not +/// a spline at all on this build (`SMSG_MOVE_KNOCK_BACK` is a ballistic launch through +/// [`super::mover`], decision 1702), so it never reaches this function. +/// +/// The arithmetic itself is [`crate::net::grounded_y`], shared with the creature clamp so the two +/// cannot drift; hover and water-walking come from **our** state, not the granted-mode word — the +/// avatar's modes live on [`super::state::MoveModes`], and the wire family the clamp reads is inert +/// on us. +fn ride_z( + player: &Player, + points: &benilla_world::world_point::WorldPoint, + grounded: bool, + pos: Vec3, + floor: Option, +) -> f32 { + if !grounded || player.swimming { + return pos.y; + } + let wow = bevy_to_wow(pos); + let water = super::mover::water_floor( + player.modes.water_walking, + player.swimming, + player.mover_pitch, + points + .liquid_at(benilla_world::world_point::Subject::Player, wow) + .map(|l| pos.y + (l.surface_z - wow[2])), + ); + crate::net::grounded_y(pos.y, floor, water, player.modes.hover) +} + /// Extract the Bevy Y-yaw of a facing quaternion. The net bridge and `sample_splines` both write /// the self entity's rotation as `Quat::from_rotation_y(facing)` (a pure Y turn), and benilla's /// Bevy yaw equals the WoW orientation (decision 0002), so this recovers both the controller's @@ -46,10 +107,18 @@ pub(super) fn drive_self_ride( net: Res, mut commands: Commands, mut player: ResMut, + // The world under the ride: the surface it stands on ([`ride_z`]) and the liquid a + // water-walker stands on instead. + world: benilla_world::collision::WorldCollision, + points: benilla_world::world_point::WorldPoint, mut q: Query< ( Entity, - &Transform, + // **Written, not just read**: the ride's Z is re-derived here ([`ride_z`]) and the + // entity transform is where it has to land. `control`'s ride guard returns before + // `body_pose::drive`, so nothing downstream would carry a correction made only to + // `Player` — the camera and the wire would move and the rendered body would not. + &mut Transform, Option<&Spline>, Option<&SplineStopped>, Option<&mut MovementState>, @@ -64,7 +133,7 @@ pub(super) fn drive_self_ride( player.server_riding = false; return; } - let Ok((entity, transform, spline, stopped, motion, twist)) = q.single_mut() else { + let Ok((entity, mut transform, spline, stopped, motion, twist)) = q.single_mut() else { return; }; // A teleport landed since last frame: the server relocated us, voiding any in-progress ride @@ -116,11 +185,36 @@ pub(super) fn drive_self_ride( super::move_trace::gait("spline", player.walking, false, player.modes.rooted, true); } let yaw = yaw_of(transform.rotation); + let wire_y = transform.translation.y; + // One probe, one meaning: the floor found from the SERVER's own pose feeds both the + // law and the line that reports it. Measuring again from the corrected pose would be a + // different question, and would report the correction as if it were the ground. + let floor = ride_ground(&world, transform.translation); + let y = ride_z( + &player, + &points, + spline.grounded, + transform.translation, + floor, + ); + if y != wire_y { + transform.translation.y = y; + } + // Traced with the answer in hand, not the question: a line logged before the correction + // measures the wire and reads identically whether the law above runs or not. + super::move_trace::ride( + spline.id, + spline.grounded, + transform.translation, + wire_y, + floor, + ); player.pos = transform.translation; player.face_yaw = yaw; player.model_yaw = yaw; player.server_riding = true; player.ride_spline_id = spline.id; + player.ride_grounded = spline.grounded; // A forward run — the charge reads as a fast run (the gait selector keys on the FORWARD // flag + speed). It also gives a sane baseline for the resume and for observers. player.move_flags = move_flags::FORWARD; @@ -145,6 +239,21 @@ pub(super) fn drive_self_ride( // spline-pending until this arrives, then relocates us and broadcasts a stop to observers — // and clear the ride so `control` resumes its own stream from rest. None if player.server_riding => { + // The endpoint is grounded by the same law as every frame before it: this pose is what + // `CMSG_MOVE_SPLINE_DONE` reports and what `control` resumes from, and a ride that ends + // a hair off our terrain is exactly what decision 0501's settle-probe hunt was about. + // `grounded` is read off the path that just finished — the last spline seen this ride. + let floor = ride_ground(&world, transform.translation); + let y = ride_z( + &player, + &points, + player.ride_grounded, + transform.translation, + floor, + ); + if y != transform.translation.y { + transform.translation.y = y; + } player.pos = transform.translation; player.face_yaw = yaw_of(transform.rotation); player.model_yaw = player.face_yaw; @@ -209,6 +318,21 @@ mod tests { walking: bool, ) -> (App, Entity, crossbeam_channel::Receiver) { let mut app = App::new(); + // The ride reads the world under it (the `rid` probe, and the ground the Z law stands on), + // so the harness has to be a world — an empty one, which answers "no ground in reach" and + // leaves every assertion below about the ride's own bookkeeping, as they were. + app.init_resource::(); + app.add_plugins(( + MinimalPlugins, + bevy::transform::TransformPlugin, + bevy::asset::AssetPlugin::default(), + bevy::scene::ScenePlugin, + avian3d::prelude::PhysicsPlugins::new(bevy::app::PostUpdate), + )); + app.init_asset::(); + benilla_world::world_point::init_world_point_resources(app.world_mut()); + app.finish(); + app.cleanup(); app.add_systems(Update, drive_self_ride); let (tx, rx) = crossbeam_channel::unbounded(); app.insert_resource(NetCommands(tx)); @@ -242,6 +366,160 @@ mod tests { (app, entity, rx) } + /// **The ride's Z law** (decision 1927) — the local half of B357, measured live at +0.47 yd on + /// a 16-yd Elwynn charge before it landed, and 0.000 after. + /// + /// The world is one flat floor and the ride is seated a chord's height above it, which is the + /// whole of the defect: a two-point server path holds its endpoints' Z across everything + /// between them, so on any concave ground the body flies. The reference does not: `0x616de0` + /// hands the spline path's displacement to the same `0x616cb0` the physics path uses, which + /// zeroes the vertical and lets the swept resolve read Z off the surface. + mod ground { + use super::*; + use avian3d::prelude::{Collider, RigidBody}; + + const FLOOR_Y: f32 = 7.5; + /// Where the server's chord holds the body — a body-height over the floor, inside the + /// probe's own reach so what is measured is the law and not the window. + const CHORD_Y: f32 = 9.4; + + /// A 20×20 up-wound floor at [`FLOOR_Y`] — the one-sided down-ray stands only on those. + fn floor(app: &mut App) { + let v = vec![ + Vec3::new(-10.0, FLOOR_Y, -10.0), + Vec3::new(10.0, FLOOR_Y, -10.0), + Vec3::new(10.0, FLOOR_Y, 10.0), + Vec3::new(-10.0, FLOOR_Y, 10.0), + ]; + app.world_mut().spawn(( + RigidBody::Static, + Collider::trimesh(v, vec![[0u32, 2, 1], [0, 3, 2]]), + Transform::default(), + )); + } + + /// Seat the body on the chord, with the collider trees built. The seating `update` runs a + /// ride frame too — harmlessly, from `Transform::default()`, where the probe cannot reach a + /// floor 7.5 yd overhead — so each test below sets its own condition and then takes + /// **exactly one** frame, which is the frame it asserts about. + fn seat(app: &mut App, entity: Entity) { + app.update(); + app.world_mut() + .entity_mut(entity) + .get_mut::() + .unwrap() + .translation + .y = CHORD_Y; + } + + /// The ride app with ground under it (or deliberately without) and the body on the chord. + fn world(with_floor: bool) -> (App, Entity) { + let (mut app, entity, _rx) = ride_app(); + if with_floor { + floor(&mut app); + } + seat(&mut app, entity); + (app, entity) + } + + fn y_of(app: &App, e: Entity) -> f32 { + app.world().get::(e).unwrap().translation.y + } + + #[track_caller] + fn assert_grounded(app: &App, e: Entity, why: &str) { + let y = y_of(app, e); + assert!( + (y - FLOOR_Y).abs() < 1e-3, + "{why}: expected the floor {FLOOR_Y}, got {y}" + ); + } + + /// **The report's shape, on our own body.** The chord is discarded; the ground is the Z — + /// and `Player::pos` carries the same answer, because the camera and the outbound position + /// report read it, not the transform. + #[test] + fn a_grounded_ride_stands_on_the_world_not_the_chord() { + let (mut app, entity) = world(true); + app.update(); + assert_grounded(&app, entity, "a grounded path's Z is the terrain's"); + assert!( + (app.world().resource::().pos.y - FLOOR_Y).abs() < 1e-3, + "the pose the camera and the wire read is the same one" + ); + } + + /// **A flying path owns its own altitude** — the reference keeps the vertical when the + /// spline carries FLYING (`0x616cec` reads the MI flag word). A taxi must not be walked + /// into the hillside it passes over. + #[test] + fn a_flying_ride_keeps_its_altitude() { + let (mut app, entity) = world(true); + app.world_mut() + .entity_mut(entity) + .get_mut::() + .unwrap() + .grounded = false; + app.update(); + assert_eq!(y_of(&app, entity), CHORD_Y, "a flight is not grounded"); + } + + /// **In liquid the wire Z IS the depth** — the same exemption the creature clamp takes, and + /// the reference's own: `[CMovement+0x40] & 0x200000` SWIMMING keeps the vertical at + /// `0x616cfa`. A feared swimmer must not be dragged to the lakebed. + #[test] + fn a_swimming_ride_keeps_its_depth() { + let (mut app, entity) = world(true); + app.world_mut().resource_mut::().swimming = true; + app.update(); + assert_eq!(y_of(&app, entity), CHORD_Y, "swimming keeps the wire Z"); + } + + /// **A probe miss leaves the body where the server put it** — nothing walkable in reach is + /// a genuinely airborne pose or ground that has not streamed in, and an unclamped body + /// belongs at its seat. Same answer the creature clamp gives. + #[test] + fn a_ride_with_no_ground_in_reach_keeps_the_servers_pose() { + let (mut app, entity) = world(false); + app.update(); + assert_eq!(y_of(&app, entity), CHORD_Y, "no floor, no correction"); + } + + /// **The endpoint is grounded too, and it is the pose the ack carries.** The frame a ride + /// ends has already lost its `Spline` (`sample_splines` drops a finished path), so the law + /// reads `Player::ride_grounded` there — without it the last frame of every charge would + /// hand the server the chord's Z and resume the controller from mid-air. + #[test] + fn the_endpoint_the_ack_reports_is_grounded() { + let (mut app, entity, rx) = ride_app(); + floor(&mut app); + seat(&mut app, entity); + app.update(); // one riding frame — arms `server_riding` + `ride_grounded` + app.world_mut().entity_mut(entity).remove::(); + // Put it back on the chord, as the finished path's last sample leaves it. + app.world_mut() + .entity_mut(entity) + .get_mut::() + .unwrap() + .translation + .y = CHORD_Y; + app.update(); + + assert_grounded(&app, entity, "the ride's last pose"); + let ack = rx + .try_iter() + .find_map(|c| match c { + ClientCommand::MoveSplineDone { pos, .. } => Some(pos), + _ => None, + }) + .expect("the ride acks when it ends"); + assert!( + (ack[2] - FLOOR_Y).abs() < 1e-3, + "the ack reports where we actually are, got {ack:?}" + ); + } + } + /// **A spline re-authors the walk gait, inverted from its own RUNMODE bit** (decision 1758). /// The reference does this in the `SMSG_MONSTER_MOVE` commit `0x7c6a50`, unconditionally for /// every incoming spline: `0x7c6ac2 and edi,0x100` takes `SPLINEFLAG_RUNMODE` and `0x7c6acb` diff --git a/crates/benilla-app/src/player/state.rs b/crates/benilla-app/src/player/state.rs index 1d4743ce..083b340d 100644 --- a/crates/benilla-app/src/player/state.rs +++ b/crates/benilla-app/src/player/state.rs @@ -1034,6 +1034,11 @@ pub(crate) struct Player { pub(super) server_riding: bool, /// The `splineId` of the ride in progress (echoed in `CMSG_MOVE_SPLINE_DONE` when it ends). pub(super) ride_spline_id: u32, + /// Was the ride in progress a **ground** path? — the `FLYING` bit of the [`crate::net::Spline`] + /// that drove it, kept because the frame the ride *ends* has already lost the spline + /// (`sample_splines` drops a finished path) and the endpoint still has to be grounded by the + /// same law as every frame before it (decision 1927). A taxi's endpoint keeps its own altitude. + pub(super) ride_grounded: bool, /// Standing on a transport (boat/zepp): the mover lives in that platform's frame (decision /// 0438 phase 2). Attached when the ground support is a [`crate::transport::Transport`] /// collider; kept through jumps above the deck (deck-frame ballistics — a jump on a moving diff --git a/crates/benilla-app/src/preflight.rs b/crates/benilla-app/src/preflight.rs index d47337de..03a45635 100644 --- a/crates/benilla-app/src/preflight.rs +++ b/crates/benilla-app/src/preflight.rs @@ -71,26 +71,52 @@ use benilla_world::world_map::CurrentMap; const PLAYER_FLAGS_GM: u32 = 0x0000_0008; /// The `UNIT_FIELD_FLAGS` bits that mean "the server is driving, or refusing to let you drive" -/// (vmangos `UnitDefines.h`) — each is a distinct reason the mover looks broken. +/// (vmangos `UnitDefines.h`) — each is a distinct reason the **mover** looks broken. +/// +/// **PACIFIED and DISARMED used to sit in here and do not belong** (decision 1903): neither +/// touches movement, and a banner telling a session "movement is server-blocked" because the +/// character is disarmed sends someone hunting a mover bug that is not there. They moved to +/// [`ABILITY_BLOCKERS`]. What is left is movement, and wow-re's +/// `object-layer/scratch/unit-flags-movement-gates.md` is where each row's mechanism lives — +/// note they are **not one gate**: STUNNED is the only bit that reaches the local input tick +/// (`0x5145b0` → `0x514755`, killing the turn and pitch emitters), CONFUSED and FLEEING act +/// through the `IsSelfControlled` predicate `0x5fa550` (mask `0xc00004`, which STUNNED is *not* +/// in), POSSESSED does not refuse at all but redirects to the charmer, and the taxi bit shares +/// no gate with any of them. const MOVE_BLOCKERS: &[(u32, &str)] = &[ - (0x0002_0000, "PACIFIED (no melee, no pacify-blocked casts)"), - (0x0004_0000, "STUNNED"), + (0x0004_0000, "STUNNED (no turning, no pitch)"), ( 0x0010_0000, "on a TAXI FLIGHT (input ignored for the whole ride)", ), - (0x0020_0000, "DISARMED (weapon abilities refuse)"), (0x0040_0000, "CONFUSED (the server drives the movement)"), (0x0080_0000, "FLEEING (the server drives the movement)"), (0x0100_0000, "POSSESSED (another unit holds the reins)"), ]; +/// The `UNIT_FIELD_FLAGS` bits that leave the mover alone and take **abilities** away instead — +/// the other half of the split above. A session that reads "movement is server-blocked" and finds +/// the character walking fine has been misdirected; these say what is actually gone. +const ABILITY_BLOCKERS: &[(u32, &str)] = &[ + (0x0002_0000, "PACIFIED (no melee, no pacify-blocked casts)"), + ( + 0x0020_0000, + "DISARMED (one weapon — the main hand's, else the off hand's — reads as absent: unarmed \ + swing and Ready idle, that weapon off the hand, Spell-Reset on the Attack button, and \ + weapon-requiring abilities greyed)", + ), +]; + /// Worth naming on entry: an unattended probe that logs in already fighting is the exact shape of /// the accident the unattended-combat ban exists for (method.md). The bit itself is declared once /// ([`crate::player`]). use crate::player::UNIT_FLAG_IN_COMBAT; -/// `UNIT_FLAG_SILENCED` — casts silently refuse. +/// `UNIT_FLAG_SILENCED` — the client's OWN local refusal, not just the server's: the CC validator +/// `0x6094f0` (reached from `TryCast 0x6e4b60`, bailing at `0x6e4f42`) refuses before any packet +/// goes out. But it is **per spell** — the arm runs only where `Spell.dbc PreventionType` (column +/// 165) is `1`, so a silence stops the ~4800 rows that declare it and nothing else (decision +/// 1903). The banner said "spell casts will refuse" flat, which is wrong for every other row. const UNIT_FLAG_SILENCED: u32 = 0x0000_2000; /// The GM faction template vmangos swaps in with GM mode (`SetFactionTemplateId(35)`). @@ -415,11 +441,14 @@ fn findings( )); } - let blocked: Vec<&str> = MOVE_BLOCKERS - .iter() - .filter(|(bit, _)| unit_flags & bit != 0) - .map(|(_, label)| *label) - .collect(); + let hits = |table: &[(u32, &'static str)]| -> Vec<&'static str> { + table + .iter() + .filter(|(bit, _)| unit_flags & bit != 0) + .map(|(_, label)| *label) + .collect() + }; + let blocked = hits(MOVE_BLOCKERS); if !blocked.is_empty() { out.push(format!( "MOVEMENT IS SERVER-BLOCKED — {} — the controller will look broken because the server \ @@ -427,6 +456,14 @@ fn findings( blocked.join(", ") )); } + let disabled = hits(ABILITY_BLOCKERS); + if !disabled.is_empty() { + out.push(format!( + "ABILITIES ARE SERVER-BLOCKED — {} — the mover is fine; what will not work is the \ + action bar and the swing.", + disabled.join(", ") + )); + } if unit_flags & UNIT_FLAG_IN_COMBAT != 0 { out.push( @@ -436,7 +473,11 @@ fn findings( ); } if unit_flags & UNIT_FLAG_SILENCED != 0 { - out.push("SILENCED — spell casts will refuse for as long as the aura holds.".into()); + out.push( + "SILENCED — the spells that declare `PreventionType = 1` (most casts) refuse locally \ + for as long as the aura holds; everything else is unaffected." + .into(), + ); } out } @@ -600,4 +641,35 @@ mod tests { assert_eq!(out.len(), 1); assert!(out[0].contains("STUNNED") && out[0].contains("TAXI FLIGHT")); } + + /// **The two tables are separate lines, and neither claims the other's symptom** (decision + /// 1903). PACIFIED and DISARMED touch no movement gate — sending a session to hunt a mover + /// bug is the whole cost of filing them under one banner, and this is what makes that + /// regression loud. + #[test] + fn ability_blockers_are_their_own_line_and_never_say_movement() { + let disarmed = player(&[(HEALTH, 60), (MAXHEALTH, 60), (UNIT_FLAGS, 0x0020_0000)]); + let out = findings(&disarmed, ShieldReport::Armed); + assert_eq!(out.len(), 1); + assert!(out[0].starts_with("ABILITIES ARE SERVER-BLOCKED")); + assert!(out[0].contains("DISARMED")); + assert!( + !out[0].contains("MOVEMENT"), + "a disarm is not a movement block: {}", + out[0] + ); + + // Both halves at once: two lines, each naming only its own flags. + let both = player(&[ + (HEALTH, 60), + (MAXHEALTH, 60), + (UNIT_FLAGS, 0x0004_0000 | 0x0002_0000), // stunned + pacified + ]); + let out = findings(&both, ShieldReport::Armed); + assert_eq!(out.len(), 2); + let movement = out.iter().find(|l| l.starts_with("MOVEMENT")).unwrap(); + let abilities = out.iter().find(|l| l.starts_with("ABILITIES")).unwrap(); + assert!(movement.contains("STUNNED") && !movement.contains("PACIFIED")); + assert!(abilities.contains("PACIFIED") && !abilities.contains("STUNNED")); + } } diff --git a/crates/benilla-app/src/ui_action/cast_send.rs b/crates/benilla-app/src/ui_action/cast_send.rs index 25eedf7a..ebeb65f2 100644 --- a/crates/benilla-app/src/ui_action/cast_send.rs +++ b/crates/benilla-app/src/ui_action/cast_send.rs @@ -472,6 +472,16 @@ fn send_spell_cast( } } } + // The CROWD-CONTROL leg of the requirement validator `0x6094f0` (decision 1903), which sits + // ABOVE its mounted block below — so a stunned mounted caster reads the stun. STUNNED refuses + // every spell; SILENCED and PACIFIED only the rows whose `PreventionType` names them. + if let Some(reason) = + state::cast_cc_refusal(ctx.rel.self_store.map_or(0, |s| s.0.unit_flags()), def) + { + debug!("ui_action: cast {spell_id} refused locally — crowd control ({reason:#x})"); + cast_errors.push_local(spell_id, reason); + return; + } // The client-side mounted gate (decision 0481; wow-re `mounted-action-gate.md` §5: // TryCast's requirement validator `0x6094f0`, mounted block `0x609c6c` — a live // `UNIT_FIELD_MOUNTDISPLAYID` refuses a non-exempt cast with reason 0x39 "You are diff --git a/crates/benilla-app/src/ui_action/drain.rs b/crates/benilla-app/src/ui_action/drain.rs index 605b82a8..144b0dda 100644 --- a/crates/benilla-app/src/ui_action/drain.rs +++ b/crates/benilla-app/src/ui_action/drain.rs @@ -196,7 +196,10 @@ pub(super) fn drain_action_uses( targeting: cast_target::CastTargeting, mut acquire: MessageWriter, // The by-key local error line — the only sink here that is not the ladder's own - // (`ladder.cast_errors` is the reason-coded one, `ladder.ground` the targeting mode). + // (`ladder.cast_errors` is the reason-coded one, `ladder.ground` the targeting mode). It is + // deliberately NOT a `CastLadder` field: a resource reachable twice from one system is a + // `B0002` panic on the first live frame, which compiles and passes every unit test + // (decision 1903). mut ui_errors: ResMut, mut ladder: CastLadder, mut gate: crate::ui_bind_confirm::BindGate, @@ -396,6 +399,7 @@ pub(super) fn drain_action_uses( &mut script, &mut gate, false, + &mut ui_errors, ); } } diff --git a/crates/benilla-app/src/ui_action/state.rs b/crates/benilla-app/src/ui_action/state.rs index 7f9c68a8..a54146d9 100644 --- a/crates/benilla-app/src/ui_action/state.rs +++ b/crates/benilla-app/src/ui_action/state.rs @@ -91,6 +91,46 @@ pub(super) fn cast_range_refusal( None } +/// `PreventionType` values (`Spell.dbc` column 165): which crowd-control flag can refuse this +/// spell locally. `0` = neither. +const PREVENTION_SILENCE: u32 = 1; +const PREVENTION_PACIFY: u32 = 2; + +/// The **crowd-control leg** of the same requirement validator `0x6094f0`, sitting **above** its +/// mounted block (`0x609c6c`) — so a stunned mounted caster is told about the stun (decision 1903; +/// wow-re `disarm-followups-law.md` §3, byte-verified). It refuses before any packet, which is why +/// it must be local: reason codes `0x64` STUNNED, `0x60` SILENCED, `0x5a` PACIFIED. +/// +/// **The three arms are not symmetric, and that is the whole finding.** STUNNED (`0x6099bc`) +/// carries no per-spell gate and refuses *everything*. SILENCED (`0x609a29`) and PACIFIED run only +/// where the spell's `PreventionType` declares them — so a silence stops casts and leaves melee +/// abilities alone, and a pacify does the reverse. Reading `UNIT_FLAG_SILENCED` as "no casting" +/// is the mistake this replaces. +/// +/// Its byte-shape was nearly missed by a census twice: the read is `f6 c4 20 test ah,0x20`, a +/// sub-register byte-lane form with no dword immediate, invisible to an immediate scan. +/// +/// **Unmodelled:** the aura exemption (`0x6e9e60`) that waives an arm and answers `0x8d` with a +/// mechanic value instead — it needs an aura-mechanic join we do not have. +pub(crate) fn cast_cc_refusal(unit_flags: u32, spell: Option<&SpellDisplay>) -> Option { + use crate::player::UNIT_FLAG_STUNNED; + /// `UNIT_FLAG_SILENCED` / `UNIT_FLAG_PACIFIED` (vmangos `UnitDefines.h`). + const UNIT_FLAG_SILENCED: u32 = 0x0000_2000; + const UNIT_FLAG_PACIFIED: u32 = 0x0002_0000; + + if unit_flags & UNIT_FLAG_STUNNED != 0 { + return Some(0x64); + } + let prevention = spell.map_or(0, |d| d.prevention_type); + if unit_flags & UNIT_FLAG_SILENCED != 0 && prevention == PREVENTION_SILENCE { + return Some(0x60); + } + if unit_flags & UNIT_FLAG_PACIFIED != 0 && prevention == PREVENTION_PACIFY { + return Some(0x5a); + } + None +} + /// The **pre-send** mounted refusal (decision 0481) — the requirement validator `0x6094f0`'s /// mounted block (`0x609c6c`, wow-re `mounted-action-gate.md` §5): a live /// `UNIT_FIELD_MOUNTDISPLAYID` refuses the cast with reason `0x39` ("You are mounted") unless @@ -1067,3 +1107,59 @@ mod tests { assert!(!cast_moving_refusal(mf::FORWARD, 1500, None)); } } + +#[cfg(test)] +mod cc_refusal_tests { + use super::*; + + fn spell(prevention_type: u32) -> SpellDisplay { + SpellDisplay { + prevention_type, + ..Default::default() + } + } + + /// **The three arms are not symmetric** (decision 1903): a stun refuses everything, a silence + /// only `PreventionType 1`, a pacify only `2`. Reading `UNIT_FLAG_SILENCED` as "no casting at + /// all" — which is what our own preflight banner used to say — is the mistake this pins. + #[test] + fn crowd_control_refuses_by_prevention_type_except_the_stun() { + const STUNNED: u32 = 0x0004_0000; + const SILENCED: u32 = 0x0000_2000; + const PACIFIED: u32 = 0x0002_0000; + + // Fireball-shaped (silence-preventable) and Heroic-Strike-shaped (pacify-preventable), + // and the auto-attack's neither — the three real values, pinned in `spell_catalog`. + let cast = spell(1); + let melee = spell(2); + let neither = spell(0); + + // STUNNED carries no per-spell gate: every row refuses, `PreventionType` or not. + for d in [&cast, &melee, &neither] { + assert_eq!(cast_cc_refusal(STUNNED, Some(d)), Some(0x64)); + } + + // SILENCED takes the casts and leaves the rest alone. + assert_eq!(cast_cc_refusal(SILENCED, Some(&cast)), Some(0x60)); + assert_eq!(cast_cc_refusal(SILENCED, Some(&melee)), None); + assert_eq!(cast_cc_refusal(SILENCED, Some(&neither)), None); + + // PACIFIED is the mirror. + assert_eq!(cast_cc_refusal(PACIFIED, Some(&melee)), Some(0x5a)); + assert_eq!(cast_cc_refusal(PACIFIED, Some(&cast)), None); + + // The stun wins when several hold at once — it is the first arm. + assert_eq!( + cast_cc_refusal(STUNNED | SILENCED | PACIFIED, Some(&cast)), + Some(0x64) + ); + // Nothing up, nothing refused; and no record claims no prevention. + assert_eq!(cast_cc_refusal(0, Some(&cast)), None); + assert_eq!(cast_cc_refusal(SILENCED, None), None); + assert_eq!( + cast_cc_refusal(STUNNED, None), + Some(0x64), + "the stun needs no record" + ); + } +} diff --git a/crates/benilla-app/src/ui_action/targeting/world.rs b/crates/benilla-app/src/ui_action/targeting/world.rs index ef9783d0..e237e296 100644 --- a/crates/benilla-app/src/ui_action/targeting/world.rs +++ b/crates/benilla-app/src/ui_action/targeting/world.rs @@ -170,6 +170,7 @@ mod tests { world.init_resource::(); world.init_resource::(); world.init_resource::(); + world.init_resource::(); world.init_resource::(); world.init_resource::(); world.init_resource::(); diff --git a/crates/benilla-app/src/ui_action/usable.rs b/crates/benilla-app/src/ui_action/usable.rs index 7b749d43..2584189d 100644 --- a/crates/benilla-app/src/ui_action/usable.rs +++ b/crates/benilla-app/src/ui_action/usable.rs @@ -33,7 +33,6 @@ use crate::target::{can_attack, ring_reaction, Factions}; use super::Spells; /// Leg 8's caster unit-flag test — the shared bit, declared once ([`crate::player`]). -use crate::creature_anim::UNIT_FLAG_DISARMED; use crate::player::UNIT_FLAG_IN_COMBAT; /// The implicit-target enums leg 10b forks on (`0x6e3f8a`/`0x6e3fa2`): 6 = single enemy → @@ -51,25 +50,22 @@ pub(crate) struct UsableCtx<'a> { pub(crate) cooldowns: &'a Cooldowns, } -/// Equipment slots 15/16 — `EQUIPMENT_SLOT_MAINHAND` / `_OFFHAND` (vmangos `EquipmentSlots`), the -/// two the disarm ladder can hide. -const EQUIPMENT_SLOT_MAINHAND: u8 = 15; -const EQUIPMENT_SLOT_OFFHAND: u8 = 16; +/// How many equipment indices the search covers — `0..=22` (`0x5f0c50`'s `cmp ebx,0x17; jl`): the +/// 19 worn slots plus the four equipped bags. We walked 19 before decision 1903. +const EQUIPMENT_SLOTS: u8 = 23; -/// One equipment slot's item class, through the same guid → entry → template walk -/// [`equipped_item_fits`] uses. `None` for an empty slot or a template still in flight — and an -/// unresolved template therefore never triggers the disarm skip, the same benefit-of-the-doubt -/// this file gives everywhere else. -fn equipped_class( - store: &ObjectStore, - items: &mut Items, - commands: &NetCommands, - slot: u8, -) -> Option { - let guid = store.0.player_inv_slot(slot).filter(|&g| g != 0)?; - let entry = items.object(guid).and_then(|o| o.object_entry())?; - Some(items.template(entry, guid, commands)?.class as u8) -} +/// `AttributesEx3`'s two hand restrictions, which is where the reference's **slot mask** comes +/// from (`0x5f0c50`'s callers): `0x400` → main hand only (mask `0x8000`), `0x1000000` → off hand +/// only (mask `0x10000`), neither → every slot. Without this a main-hand-only ability counted a +/// weapon worn anywhere, which is what made the disarm case interesting to get right: strip the +/// hidden hand out of a mask that is already down to one bit and nothing can satisfy it. +const ATTR_EX3_MAIN_HAND_ONLY: u32 = 0x0000_0400; +const ATTR_EX3_OFF_HAND_ONLY: u32 = 0x0100_0000; + +/// `ITEM_FLAG_DEPRECATED` (vmangos `ItemPrototype.h`: *"appears red icon (like when item +/// durability==0)"*) — one of the two rejects the reference's search applies to a worn item before +/// matching its class. The other is being genuinely broken. +const ITEM_FLAG_DEPRECATED: u32 = 0x0000_0010; /// Leg 4's own test (`0x6e40e0`), shared with the spell tooltip's requirement line: does some /// WORN item match `EquippedItemClass` + `EquippedItemSubClassMask`? `true` when the spell asks @@ -85,33 +81,58 @@ pub(crate) fn equipped_item_fits( return true; } let class = d.equipped_item_class as u32; - // The disarm ladder (decision 1863). The reference's own equipped-item test `0x5ea5d0` walks - // its three `GetWeapon` slots with `visFlag = 0`, so the hand `UNIT_FLAG_DISARMED` hides does - // not count toward the requirement — a disarmed warrior's Heroic Strike greys out and its - // tooltip requirement line turns red. Exactly one hand is hidden, main first, so a disarmed - // dual-wielder's off-hand weapon still satisfies it. - let hidden = (store.0.unit_flags() & UNIT_FLAG_DISARMED != 0) - .then(|| { - let main = equipped_class(store, items, commands, EQUIPMENT_SLOT_MAINHAND); - let off = equipped_class(store, items, commands, EQUIPMENT_SLOT_OFFHAND); - crate::creature_anim::disarmed_hand(main, off) + // The **slot mask** first (decision 1903): `AttributesEx3` narrows the search to one hand for + // an ability that names one, and to everything otherwise. + let mut mask: u32 = if d.attributes_ex3 & ATTR_EX3_MAIN_HAND_ONLY != 0 { + 1 << crate::items::EQUIPMENT_SLOT_MAINHAND + } else if d.attributes_ex3 & ATTR_EX3_OFF_HAND_ONLY != 0 { + 1 << crate::items::EQUIPMENT_SLOT_OFFHAND + } else { + u32::MAX + }; + // …then the disarm ladder strips the hidden hand's bit out of it (decision 1863, its citation + // corrected by 1903: the reference does this at `0x5f0c69`/`0x5f0c91` with `visFlag = 1` + // probes and a mask edit, NOT by `GetWeapon` returning NULL — same ladder, same outcome, one + // hand only). A disarmed warrior's Heroic Strike greys out and its tooltip requirement line + // turns red; a disarmed dual-wielder's off-hand weapon still satisfies a hand-agnostic one. + if let Some(hidden) = crate::items::disarmed_equipment_slot(store, items, commands) { + mask &= !(1u32 << hidden); + } + (0..EQUIPMENT_SLOTS) + .filter(|slot| mask & (1u32 << slot) != 0) + .any(|slot| { + let Some(guid) = store.0.player_inv_slot(slot).filter(|&g| g != 0) else { + return false; + }; + // Entry and the durability pair come off the INSTANCE in one borrow — the reference + // reads both from the CGItem (`[item+0x114]+0xa0`/`+0xa4`), not from the template — + // because the template lookup below needs `items` mutably. + let Some((entry, durability, max_durability)) = items.object(guid).and_then(|o| { + Some(( + o.object_entry()?, + o.item_durability(), + o.item_max_durability(), + )) + }) else { + return false; + }; + let Some(t) = items.template(entry, guid, commands) else { + return true; // unresolved template: benefit of the doubt + }; + // The reference's two rejects, applied before the class match: a DEPRECATED item and + // a genuinely BROKEN one (`MaxDurability > 0 && Durability == 0`) cannot satisfy a + // requirement. Note this is the one place durability DOES gate something — it drives + // no animation or model path anywhere (decision 1863). + if t.flags & ITEM_FLAG_DEPRECATED != 0 { + return false; + } + if max_durability.is_some_and(|m| m > 0) && durability == Some(0) { + return false; + } + t.class == class + && (d.equipped_item_subclass_mask == 0 + || d.equipped_item_subclass_mask & (1 << t.subclass) != 0) }) - .flatten() - .map(|hand| EQUIPMENT_SLOT_MAINHAND + hand as u8); - (0..19).filter(|slot| Some(*slot) != hidden).any(|slot| { - let Some(guid) = store.0.player_inv_slot(slot).filter(|&g| g != 0) else { - return false; - }; - let Some(entry) = items.object(guid).and_then(|o| o.object_entry()) else { - return false; - }; - let Some(t) = items.template(entry, guid, commands) else { - return true; // unresolved template: benefit of the doubt - }; - t.class == class - && (d.equipped_item_subclass_mask == 0 - || d.equipped_item_subclass_mask & (1 << t.subclass) != 0) - }) } /// The walk. Returns `(usable, not_enough_mana)` — the `IsUsableAction` pair. @@ -305,7 +326,7 @@ mod tests { fn a_disarmed_hand_does_not_satisfy_the_equipped_item_requirement() { use crate::items::TestDeps; - /// `PLAYER_FIELD_INV_SLOT_HEAD + 2×slot` for equipment slots 15 and 16. + // `PLAYER_FIELD_INV_SLOT_HEAD + 2×slot` for equipment slots 15 and 16. const INV_MAINHAND: u16 = 486 + 2 * 15; const INV_OFFHAND: u16 = 486 + 2 * 16; const DISARMED: u32 = 0x0020_0000; @@ -348,6 +369,143 @@ mod tests { assert!(!fits(true, &[(INV_OFFHAND, 0x2b)])); } + /// **The slot mask and its two rejects** (decision 1903) — the rest of `0x5f0c50`, which the + /// mislabelled census row had hidden behind a three-slot `GetWeapon` loop that does not exist. + /// The search is equipment indices `0..=22`, narrowed by `AttributesEx3` to one hand when the + /// ability names one, and a worn item that is DEPRECATED or genuinely BROKEN cannot satisfy it. + #[test] + fn the_equipped_item_search_masks_by_hand_and_rejects_broken_gear() { + use crate::items::TestDeps; + + const INV_MAINHAND: u16 = 486 + 2 * 15; + const INV_OFFHAND: u16 = 486 + 2 * 16; + // `ITEM_FIELD_DURABILITY` / `_MAXDURABILITY` — both INSTANCE fields (46/47), which is + // where the reference reads the pair from. + const ITEM_DURABILITY: u16 = 46; + const ITEM_MAX_DURABILITY: u16 = 47; + + // A spell requiring a class-2 weapon, with whatever `AttributesEx3` is passed. + let spell = |ex3: u32| SpellDisplay { + equipped_item_class: 2, + equipped_item_subclass_mask: 0, + attributes_ex3: ex3, + ..Default::default() + }; + // `hands`: per equipped weapon — (inv field, guid, template flags, instance max + // durability, instance current durability). + let fits = |d: &SpellDisplay, hands: &[(u16, u64, u32, u32, u32)]| { + let mut deps = TestDeps::new(); + let mut pairs = vec![(46u16, 1u32 << 3)]; + for (i, (field, guid, flags, max_dur, dur)) in hands.iter().enumerate() { + pairs.push((*field, *guid as u32)); + let entry = 500 + i as u32; + deps.items.insert_object( + *guid, + ObjectFields::from_pairs(&[ + (3, entry), + (ITEM_DURABILITY, *dur), + (ITEM_MAX_DURABILITY, *max_dur), + ]), + ); + deps.items.insert_template( + entry, + Some(benilla_protocol::messages::ItemInfo { + class: 2, + subclass: 7, + flags: *flags, + ..crate::items::test_template("Sword") + }), + ); + } + let store = ObjectStore(ObjectFields::from_pairs(&pairs)); + equipped_item_fits(d, &store, &mut deps.items, &deps.commands) + }; + + let sound = |field| (field, 0x2au64, 0u32, 0u32, 0u32); + + // Hand-agnostic: either hand satisfies it. + assert!(fits(&spell(0), &[sound(INV_MAINHAND)])); + assert!(fits(&spell(0), &[sound(INV_OFFHAND)])); + // MAIN-HAND-ONLY (`0x400`): the off-hand weapon no longer counts. + assert!(fits( + &spell(ATTR_EX3_MAIN_HAND_ONLY), + &[sound(INV_MAINHAND)] + )); + assert!(!fits( + &spell(ATTR_EX3_MAIN_HAND_ONLY), + &[sound(INV_OFFHAND)] + )); + // OFF-HAND-ONLY (`0x1000000`): the mirror. + assert!(fits(&spell(ATTR_EX3_OFF_HAND_ONLY), &[sound(INV_OFFHAND)])); + assert!(!fits( + &spell(ATTR_EX3_OFF_HAND_ONLY), + &[sound(INV_MAINHAND)] + )); + + // The two rejects: DEPRECATED, and broken (`MaxDurability > 0 && Durability == 0`). + assert!(!fits( + &spell(0), + &[(INV_MAINHAND, 0x2a, ITEM_FLAG_DEPRECATED, 0, 0)] + )); + assert!(!fits(&spell(0), &[(INV_MAINHAND, 0x2a, 0, 45, 0)])); + // …and a merely damaged one still counts, as does one with no durability at all. + assert!(fits(&spell(0), &[(INV_MAINHAND, 0x2a, 0, 45, 12)])); + assert!(fits(&spell(0), &[(INV_MAINHAND, 0x2a, 0, 0, 0)])); + } + + /// The mask and the disarm strip compose, and that composition is the Heroic Strike case: a + /// main-hand-only ability whose one allowed bit is the hand the disarm hides has nothing left + /// to match, whatever else is worn. + #[test] + fn a_main_hand_only_ability_is_dead_while_that_hand_is_disarmed() { + use crate::items::TestDeps; + + const INV_MAINHAND: u16 = 486 + 2 * 15; + const INV_OFFHAND: u16 = 486 + 2 * 16; + const DISARMED: u32 = 0x0020_0000; + + let fits = |ex3: u32, disarmed: bool| { + let mut deps = TestDeps::new(); + let mut pairs = vec![(46u16, (1 << 3) | if disarmed { DISARMED } else { 0 })]; + for (i, field) in [INV_MAINHAND, INV_OFFHAND].iter().enumerate() { + let guid = 0x2a + i as u64; + pairs.push((*field, guid as u32)); + let entry = 500 + i as u32; + deps.items + .insert_object(guid, ObjectFields::from_pairs(&[(3, entry)])); + deps.items.insert_template( + entry, + Some(benilla_protocol::messages::ItemInfo { + class: 2, + subclass: 7, + ..crate::items::test_template("Sword") + }), + ); + } + let store = ObjectStore(ObjectFields::from_pairs(&pairs)); + equipped_item_fits( + &SpellDisplay { + equipped_item_class: 2, + attributes_ex3: ex3, + ..Default::default() + }, + &store, + &mut deps.items, + &deps.commands, + ) + }; + + // Dual-wielding, armed: both abilities are usable. + assert!(fits(ATTR_EX3_MAIN_HAND_ONLY, false)); + assert!(fits(ATTR_EX3_OFF_HAND_ONLY, false)); + // Disarmed: the ladder hides the MAIN hand (main first), so the main-hand-only ability + // has an empty mask and dies… + assert!(!fits(ATTR_EX3_MAIN_HAND_ONLY, true)); + // …while the off-hand-only one is untouched, and so is a hand-agnostic one. + assert!(fits(ATTR_EX3_OFF_HAND_ONLY, true)); + assert!(fits(0, true)); + } + fn ctx<'a>( store: &'a ObjectStore, cooldowns: &'a Cooldowns, diff --git a/crates/benilla-app/src/ui_items/drain.rs b/crates/benilla-app/src/ui_items/drain.rs index b5589152..8a130df1 100644 --- a/crates/benilla-app/src/ui_items/drain.rs +++ b/crates/benilla-app/src/ui_items/drain.rs @@ -200,6 +200,7 @@ pub(super) fn drain_inventory_uses( self_q: Query<&ObjectStore, With>, targeting: crate::ui_action::cast_target::CastTargeting, mut ladder: crate::ui_action::CastLadder, + mut ui_errors: ResMut, mut gate: crate::ui_bind_confirm::BindGate, ) { let Some(mut script) = script else { @@ -250,6 +251,7 @@ pub(super) fn drain_inventory_uses( &mut script, &mut gate, false, + &mut ui_errors, ); } } @@ -269,6 +271,7 @@ pub(super) fn drain_container_uses( // that lets `SMSG_LOOT_RESPONSE`'s admission gate recognise an item loot (decision 1531). mut loot_latch: ResMut, mut ladder: crate::ui_action::CastLadder, + mut ui_errors: ResMut, mut gate: crate::ui_bind_confirm::BindGate, ) { let Some(mut script) = script else { @@ -459,6 +462,7 @@ pub(super) fn drain_container_uses( &mut script, &mut gate, false, + &mut ui_errors, ); continue; } @@ -573,6 +577,7 @@ pub(super) fn drain_container_uses( &mut script, &mut gate, false, + &mut ui_errors, ); } } @@ -857,6 +862,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() @@ -1024,6 +1030,7 @@ pub(super) fn drain_bind_on_use_confirms( script: Option>, targeting: crate::ui_action::cast_target::CastTargeting, mut ladder: crate::ui_action::CastLadder, + mut ui_errors: ResMut, mut gate: crate::ui_bind_confirm::BindGate, ) { let Some(mut script) = script else { @@ -1046,6 +1053,7 @@ pub(super) fn drain_bind_on_use_confirms( &mut script, &mut gate, true, + &mut ui_errors, ); } @@ -1556,6 +1564,7 @@ mod bind_confirm_tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .init_resource::() .init_resource::() diff --git a/crates/benilla-app/src/ui_items/mod.rs b/crates/benilla-app/src/ui_items/mod.rs index f5477d22..516ddae9 100644 --- a/crates/benilla-app/src/ui_items/mod.rs +++ b/crates/benilla-app/src/ui_items/mod.rs @@ -601,6 +601,9 @@ pub(crate) fn has_key(store: &ObjectFields, items: &mut Items, commands: &NetCom .is_some_and(|t| t.bag_family == BAG_FAMILY_KEYS) }) } +/// The wire's "player array" bag index — `INVENTORY_SLOT_BAG_0`. With it, [`ItemUse::slot`] IS the +/// equipment index (0–18 worn, 19–22 the equipped bags). +pub(crate) const PLAYER_ARRAY: u8 = 255; /// One resolved item-use click, as [`send_item_use`] needs it: the wire position, the two /// template scalars the fork reads, and the on-use SPELL the cast tail is keyed on. @@ -676,49 +679,77 @@ pub(crate) enum ItemUseRoute { /// `ITEM_FLAG_CHARTER` (`0x2000`) opens the petition window — it sends /// `CMSG_PETITION_SHOW_SIGNATURES` for the instance, not `CMSG_USE_ITEM`. /// - /// **INFERRED, and this is the whole of the evidence.** wow-re has not carved `CGItem::Use`'s - /// charter branch, so the *position* of this arm and the opcode it sends are reasoned rather - /// than read: + /// **VERIFIED** (2026-09-03, correcting 1672's INFERRED reading — which reasoned it out and + /// got the opcode, the payload and the gate right, but placed the arm wrongly). wow-re HAD + /// carved this branch; the note simply was not found. Two records carry it: /// - /// - 1.12's shipped FrameXML special-cases a charter **nowhere** — `UseContainerItem` and - /// `UseAction` are entirely generic — so whatever opens the window is engine-side. - /// - The charter template (entry 5863) has no ON_USE spell, no `StartQuest`, and - /// `InventoryType = 0`, so *every* other arm of this fork declines it and the click currently - /// reaches [`Self::Nothing`] and sends nothing at all. Right-clicking a charter in 1.12 - /// plainly does something, so an arm must exist. - /// - `CMSG_PETITION_SHOW_SIGNATURES` is the only opcode that opens the window, and vmangos's - /// `HandleUseItemOpcode` has no charter branch — a `CMSG_USE_ITEM` here would be answered - /// with nothing. - /// - The reference already keys other charter behaviour on this exact flag bit: the item - /// tooltip's `ITEM_SIGNABLE` line and its enchant-line suppression both test `0x2000` - /// (wow-re `system/ui/scratch/tooltip-content-law.md:485-505`). + /// - `ui/scratch/right-click-open.md` §3 row **#9**: gate `0x5d8f95 test ah,0x20` on the + /// template's `Flags & 0x2000` (SIGNABLE / petition) → `0x5eef40`. + /// - `object-layer/ledger.tsv`'s row for `0x5eef40`: the **`CMSG_PETITION_SHOW_SIGNATURES` + /// (`0x1BE`)** sender, `ret 8` — `Put32(0x1BE)` @ `0x5eef79`, `Put64(guid)` @ `0x5eef83`, + /// send @ `0x5eef8e`, and **that is the whole packet: one uint64**. It returns immediately + /// if both guid dwords are zero, and its **sole caller image-wide is `0x5d8fa6`** — this + /// leg. It latches nothing and never reads the item's enchantment block. /// - /// The arm sits directly after the quest fork. Its **order relative to the quest fork is - /// unobservable** — no 1.12 item is both a charter and a quest starter — so nothing here rests - /// on it; a byte read of `0x5d8d00` settles both the position and the opcode. + /// **The position was wrong and is corrected here.** 1672 said "the arm sits directly after + /// the quest fork"; it is rung **#9**, after the requirement check (#4), the two readable + /// forks (#5 template PageText, #6 instance ITEM_TEXT_ID), the lost-control block (#7) and the + /// LOOTABLE → `CMSG_OPEN_ITEM` arm (#8). What our fork actually needs from that ordering still + /// holds — charter **after** quest, **before** the toggle/cast tail — so no behaviour moves; + /// what changes is that this is now read rather than reasoned, and a reader is not sent + /// looking for a byte read that already exists. ShowPetition { item: u64 }, + /// **The disarmed refusal** — `CGItem::Use`'s rung **15 of 20** (decision 1903; wow-re + /// `disarm-followups-law.md` §2, byte-verified): using the very weapon a disarm has taken + /// raises `ERR_CANT_USE_DISARMED` (`0x16b` = 363) at `0x5d926d call 0x496720` and sends + /// **nothing**. This is the client refusing on its own — unlike `ERR_NOT_WHILE_DISARMED` + /// (61), which is a server `SMSG_INVENTORY_CHANGE_FAILURE` reason we only render. + /// + /// The condition is narrow, and none of it is about the clicked item's own class: the item + /// must be **worn in equipment slot 15 or 16**, the flag must be up, and *that hand* must be + /// the one the ladder hides. So right-clicking the sword in your bag is not refused, and + /// neither is a disarmed dual-wielder's off-hand weapon — only the hand the disarm took. + /// + /// It sits **above** the bind-confirm arm (`0x5d91d6`) and the cast tail (`0x5d9258`), so a + /// bind-on-use weapon does not raise its confirm dialog first. + CantUseDisarmed, } /// [`ItemUseRoute`]'s decision. `guid: None` (the instance never resolved) cannot address a /// questgiver, so it falls through to the ordinary path — the same fallback the equip fork makes. /// /// `aura_cancels` is the toggle predicate applied to the item's ON_USE spell id — the caller's -/// [`crate::ui_action::toggle::active_action_toggle`], passed in so this stays a pure function of -/// the click. The **order is the reference's**: the quest offer forks first (`0x5d8dcc`, well -/// above), then the toggle scan (`0x5d9157`), then the cast tail (`0x5d9249`). -pub(crate) fn item_use_route(it: ItemUse, aura_cancels: impl Fn(u32) -> bool) -> ItemUseRoute { +/// [`crate::ui_action::toggle::active_action_toggle`], and `disarmed_hand` the equipment slot this +/// character's disarm hides ([`crate::items::disarmed_equipment_slot`]); both are caster state, +/// passed in so this stays a pure function of the click. The **order is the reference's**: the +/// quest offer forks first (`0x5d8dcc`, well above), then the charter arm (#9, `0x5d8f95`), then +/// the disarmed refusal (rung 15), then the toggle scan (`0x5d9157`), then the cast tail +/// (`0x5d9249`). +pub(crate) fn item_use_route( + it: ItemUse, + aura_cancels: impl Fn(u32) -> bool, + disarmed_hand: Option, +) -> ItemUseRoute { if let Some(npc) = it.guid.filter(|_| it.start_quest != 0) { return ItemUseRoute::QuestOffer { npc, quest: it.start_quest, }; } - // The charter arm (decision 1672) — see `ItemUseRoute::ShowPetition` for its evidence and for - // why its position here is unobservable. A charter with no resolved instance cannot be - // addressed, so it falls through to the ordinary path exactly as the quest fork does. + // The charter arm (decision 1672, its evidence upgraded to VERIFIED — see + // `ItemUseRoute::ShowPetition`): the reference's rung #9, after quest and before the cast + // tail, which is the ordering this fork needs. A charter with no resolved instance cannot be + // addressed, so it falls through to the ordinary path exactly as the quest fork does — and + // the reference agrees at the bytes, `0x5eef40` returning without sending on a zero guid. if let Some(item) = it.guid.filter(|_| it.is_charter) { return ItemUseRoute::ShowPetition { item }; } + // Rung 15 (decision 1903): the clicked item IS the weapon this disarm took. `bag_index == 255` + // is the player array, so `slot` is the equipment index — the reference gets there by scanning + // the equipment guids for the item and asking whether the index it lands on is the hidden one. + if disarmed_hand.is_some() && it.bag_index == PLAYER_ARRAY && Some(it.slot) == disarmed_hand { + return ItemUseRoute::CantUseDisarmed; + } match it.use_spell { Some(spell) if aura_cancels(spell) => ItemUseRoute::ToggleCancel(spell), Some(spell) => ItemUseRoute::Cast(spell), @@ -776,6 +807,9 @@ pub(crate) fn send_item_use( script: &mut benilla_ui::script::UiScript, gate: &mut crate::ui_bind_confirm::BindGate, suppress: bool, + // The by-key local-refusal sink — passed explicitly rather than carried on `CastLadder`, so + // no system can reach it twice (decision 1903). + ui_errors: &mut crate::ui_action::UiErrorKeys, ) -> bool { // The toggle predicate, resolved once against the caster's live aura slots. Both inputs are // already here: the spell's ActiveIconID column and the caster's descriptor. @@ -787,7 +821,21 @@ pub(crate) fn send_item_use( .self_store .is_some_and(|store| crate::ui_action::toggle::active_action_toggle(spell, d, store)) }; - match item_use_route(it, aura_cancels) { + // The disarm ladder, asked of the caster's own inventory (decision 1903). + let disarmed_hand = ctx.rel.self_store.and_then(|store| { + crate::items::disarmed_equipment_slot(store, &mut ladder.items, &ladder.commands) + }); + match item_use_route(it, aura_cancels, disarmed_hand) { + ItemUseRoute::CantUseDisarmed => { + debug!( + "ui_items: item use refused — that hand is disarmed (slot {})", + it.slot + ); + ui_errors + .0 + .push(crate::ui_action::UiError::key("ERR_CANT_USE_DISARMED")); + false + } ItemUseRoute::ToggleCancel(spell) => { debug!("ui_items: item use {spell} re-pressed — its aura cancels, no cast"); let _ = ladder @@ -1181,6 +1229,61 @@ mod tests { assert_eq!(wire_pos(KEYRING_CONTAINER, 0), None); } + /// **The disarmed refusal, rung 15** (decision 1903). The condition is about the item's WORN + /// POSITION, not its class or its spell: only the weapon in the hand the ladder hides is + /// refused, and the refusal beats the cast tail below it. + #[test] + fn using_the_hand_a_disarm_took_is_refused_locally() { + let sword = 0x4000_0000_0000_0001_u64; + // A weapon with an ON_USE spell, worn at `slot`, clicked out of the player array. + let worn = |slot: u8| ItemUse { + entry: 7, + guid: Some(sword), + start_quest: 0, + bag_index: super::PLAYER_ARRAY, + slot, + spell_index: 0, + use_spell: Some(8690), + on_object: None, + is_charter: false, + }; + let never = |_: u32| false; + + // The hidden hand: refused, and the cast tail below it never runs. + assert_eq!( + item_use_route(worn(15), never, Some(15)), + ItemUseRoute::CantUseDisarmed + ); + // The OTHER hand of a disarmed dual-wielder is not hidden — it casts as usual. + assert_eq!( + item_use_route(worn(16), never, Some(15)), + ItemUseRoute::Cast(8690) + ); + // The same item sitting in a BAG is not worn in that hand at all: the reference's scan + // resolves an equipment index or nothing, so a bag click is never this refusal. + assert_eq!( + item_use_route( + ItemUse { + bag_index: 1, + ..worn(15) + }, + never, + Some(15) + ), + ItemUseRoute::Cast(8690) + ); + // Flag down: nothing is hidden. + assert_eq!( + item_use_route(worn(15), never, None), + ItemUseRoute::Cast(8690) + ); + // The off hand as the hidden one — the empty/non-weapon main-hand rung of the ladder. + assert_eq!( + item_use_route(worn(16), never, Some(16)), + ItemUseRoute::CantUseDisarmed + ); + } + /// The pure fork ([`item_use_route`], decisions 0664/0914), all four arms: a non-zero /// `StartQuest` diverts to `CMSG_QUESTGIVER_QUERY_QUEST` addressed to the ITEM's guid (arm #3, /// `0x5d8dd2` — it returns before the cast tail); an ON_USE spell whose aura is live on the @@ -1208,26 +1311,26 @@ mod tests { }; let never = |_| false; assert_eq!( - item_use_route(it(Some(letter), 373, None), never), + item_use_route(it(Some(letter), 373, None), never, None), ItemUseRoute::QuestOffer { npc: letter, quest: 373 } ); assert_eq!( - item_use_route(it(Some(letter), 0, Some(8690)), never), + item_use_route(it(Some(letter), 0, Some(8690)), never, None), ItemUseRoute::Cast(8690), "a hearthstone takes the cast tail" ); assert_eq!( - item_use_route(it(Some(letter), 0, None), never), + item_use_route(it(Some(letter), 0, None), never, None), ItemUseRoute::Nothing, "no ON_USE block — the ref sends nothing" ); // No resolved instance (the template is still in flight): a query against guid 0 is // impossible, so the ordinary path runs — the equip fork's own fallback. assert_eq!( - item_use_route(it(None, 373, Some(8690)), never), + item_use_route(it(None, 373, Some(8690)), never, None), ItemUseRoute::Cast(8690) ); } @@ -1256,24 +1359,24 @@ mod tests { }; let never = |_| false; assert_eq!( - item_use_route(it(Some(charter), true, None), never), + item_use_route(it(Some(charter), true, None), never, None), ItemUseRoute::ShowPetition { item: charter }, "the charter opens its petition window" ); // Without the flag, the very same item is the reference's silent no-op — which is what a // charter click did before this arm existed. assert_eq!( - item_use_route(it(Some(charter), false, None), never), + item_use_route(it(Some(charter), false, None), never, None), ItemUseRoute::Nothing ); // No resolved instance: nothing to address the show-signatures with, so it falls through // exactly as the quest fork does on the same condition. assert_eq!( - item_use_route(it(None, true, None), never), + item_use_route(it(None, true, None), never, None), ItemUseRoute::Nothing ); assert_eq!( - item_use_route(it(None, true, Some(8690)), never), + item_use_route(it(None, true, Some(8690)), never, None), ItemUseRoute::Cast(8690) ); } @@ -1300,25 +1403,28 @@ mod tests { }; let mounted = |spell: u32| spell == SUMMON_HORSE; assert_eq!( - item_use_route(it(0, Some(SUMMON_HORSE)), mounted), + item_use_route(it(0, Some(SUMMON_HORSE)), mounted, None), ItemUseRoute::ToggleCancel(SUMMON_HORSE), "mounted: the click dismounts — CMSG_CANCEL_AURA, no CMSG_USE_ITEM", ); assert_eq!( - item_use_route(it(0, Some(SUMMON_HORSE)), |_| false), + item_use_route(it(0, Some(SUMMON_HORSE)), |_| false, None), ItemUseRoute::Cast(SUMMON_HORSE), "not mounted: the very same click casts", ); // Order: the quest offer forks at `0x5d8dcc`, above the toggle scan at `0x5d9157`. assert_eq!( - item_use_route(it(373, Some(SUMMON_HORSE)), mounted), + item_use_route(it(373, Some(SUMMON_HORSE)), mounted, None), ItemUseRoute::QuestOffer { npc: 0x4000_0000_0000_0BAD, quest: 373 }, ); // …and an item with no ON_USE block has nothing to cancel, whatever the predicate says. - assert_eq!(item_use_route(it(0, None), |_| true), ItemUseRoute::Nothing); + assert_eq!( + item_use_route(it(0, None), |_| true, None), + ItemUseRoute::Nothing + ); } /// A dozen representative `InventoryType`s (decision 0208 phase 1b's own ask), spanning the diff --git a/crates/benilla-app/tests/create_frame_template.rs b/crates/benilla-app/tests/create_frame_template.rs index 43c6a66c..7c0b0ddb 100644 --- a/crates/benilla-app/tests/create_frame_template.rs +++ b/crates/benilla-app/tests/create_frame_template.rs @@ -26,8 +26,21 @@ const UI_DIR: &str = concat!(env!("CARGO_MANIFEST_DIR"), "/assets/ui"); /// chain from `Interface\FrameXML\UIPanelTemplates.xml`, seated below `UiPanels.xml` exactly as /// the manifest seats it. So this list carries the chain pair and the loader below has to be able /// to READ a chain entry, which a disk-only provider under `assets/ui` cannot. -const FILES: [&str; 7] = [ +const FILES: [&str; 8] = [ "Interface\\FrameXML\\Fonts.xml", + // A REGRESSION GUARD, not a dependency (1923). FadingFrame.xml's entire body is a single + // `