diff --git a/crates/benilla-app/src/creature_anim/driver.rs b/crates/benilla-app/src/creature_anim/driver.rs index 7894880d..cea472ae 100644 --- a/crates/benilla-app/src/creature_anim/driver.rs +++ b/crates/benilla-app/src/creature_anim/driver.rs @@ -423,11 +423,10 @@ pub(super) fn drive_animations( let catalog = anim_data.as_deref().map(|d| &d.0); // `WOW_ANIM_COST=1` — the memo-sizing counter (1370 item 9's bracket): counts the frame's // RESTING rows, the shape an input-memo fast path would skip. Sizing only — no skip is - // taken here. (An `AnimParked` filter was law-barred by 0448's off-screen-events law; that - // law's premise — "the reference keeps every off-screen unit audible" — fell with wow-re's - // election correction, which keeps only `MORE_AUDIBLE` creatures ticking. Until 1473's - // event-gate leg lands as its own record, the driver still arms clips for every parked - // unit, and the memo remains the only safe skip shape.) + // taken here. (The EVENT scan is 1482's to gate now — a parked unflagged unit's tracks go + // unscanned — but the driver still arms clips for every parked rig: the wake pose must be + // the absolute-clock pose of the RIGHT clip, and only the state machine knows which clip + // that is. The memo remains the only safe skip shape for these rows.) let anim_cost = anim_cost_enabled(); let (mut cost_rows, mut cost_resting) = (0u32, 0u32); for ( diff --git a/crates/benilla-app/src/creature_anim/events.rs b/crates/benilla-app/src/creature_anim/events.rs index aab4fdf5..b6a637c3 100644 --- a/crates/benilla-app/src/creature_anim/events.rs +++ b/crates/benilla-app/src/creature_anim/events.rs @@ -9,6 +9,13 @@ use bevy::prelude::*; use super::{resolved_id, AnimData, AnimDriver}; +/// Bit `0x20` of the cached creature template's `type_flags` word — the one flag the reference's +/// pass-2 election re-links a model for tick-only (`0x607da0`'s `0x623b70` arm), so an off-screen +/// flagged creature's combat stays audible. The *name* is vmangos corroboration +/// (`CREATURE_TYPEFLAGS_MORE_AUDIBLE`, `CreatureDefines.h:151`); the byte fact is the bit +/// (wow-re `outdoor-object-pass-election.md` §4). +const CREATURE_TYPEFLAGS_MORE_AUDIBLE: u32 = 0x20; + /// A model **event keyframe** crossed during playback this frame (the M2 `$xxx` tags — decision /// 0070 slice 3): the animation timeline's outbound trigger surface. The sound subsystem routes /// the audio tags (`$SND` kits, footsteps, `$CSS` swings, fidgets); future consumers (camera @@ -83,8 +90,16 @@ pub(crate) type TrackMemory = bevy::ecs::entity::EntityHashMap; /// Arming is [`advance_track`]'s: an arm frame fires nothing, the frame after it opens the clip's /// head window (so `t = 0` keyframes are real), and a clip that doesn't survive its arm frame /// fires nothing at all — the reference's own walker rule, byte-cited there. +#[allow(clippy::type_complexity)] // one Bevy query tuple — the house convention pub(super) fn fire_anim_events( - units: Query<(Entity, &ModelAnimations, &AnimationPlayer, &AnimDriver)>, + units: Query<( + Entity, + &ModelAnimations, + &AnimationPlayer, + &AnimDriver, + Has, + Option<&crate::net::Guid>, + )>, mut last: Local, // The **masked overlay** track's own memory (decision 0087): a swing/emote routed to the // SpineLow overlay plays *beside* the base, so its events (a swing's `$CSS`, an emote's `$CSD`) @@ -92,9 +107,30 @@ pub(super) fn fire_anim_events( mut last_overlay: Local, mut out: MessageWriter, anim_data: Option>, + names: Res, ) { let catalog = anim_data.as_deref().map(|d| &d.0); - for (entity, anims, player, drv) in &units { + for (entity, anims, player, drv, parked, guid) in &units { + // The election's TICK half (decision 1482): a parked unit's event tracks are not + // scanned — the reference's pass-2 walk never inserts the model into the tick worklist + // (`0x683dd0` walk 2 skips `0x710b90`) — unless its cached template carries + // `MORE_AUDIBLE`, the `0x607da0` re-link arm that keeps an off-screen flagged + // creature's combat audible. A missing template record reads NOT audible (the + // reference's `0x623b70` null leg — fail closed; the record lands within a second of + // streaming anyway). The track memories are dropped so a waking track re-ARMS — firing + // nothing on the wake frame — instead of scanning the whole parked gap as one + // crossing; the reference's re-admission is likewise a fresh record. + if parked + && !guid.is_some_and(|g| { + names + .peek_type_flags(g.0) + .is_some_and(|f| f & CREATURE_TYPEFLAGS_MORE_AUDIBLE != 0) + }) + { + last.remove(&entity); + last_overlay.remove(&entity); + continue; + } // Base track: the **resolved** id (decision 0082) — the clip whose timeline is actually // advancing, which can differ from the requested `active_anim()` when this model falls back — // then the id's **playing variation** (decision 0114: a one-shot rolled one of the id's diff --git a/crates/benilla-app/src/creature_anim/lod.rs b/crates/benilla-app/src/creature_anim/lod.rs index 2c892f17..7a12d494 100644 --- a/crates/benilla-app/src/creature_anim/lod.rs +++ b/crates/benilla-app/src/creature_anim/lod.rs @@ -17,13 +17,14 @@ //! re-appearing unit shows the pose "now" dictates. Nothing here pauses an [`AnimationPlayer`]: //! Bevy's `advance_animations` keeps ticking every seek clock, so waking is just sampling //! again — there is no frozen state to resume from. -//! - **(ii) off-screen combat stays audible — for EVERY unit**, which is now known to be MORE -//! than the reference grants (it silences all but `MORE_AUDIBLE` creatures): the event scanner -//! ([`super::events::fire_anim_events`]) reads the player's seek, not the bones, and the driver -//! state machine keeps arming clips for parked rigs; `$CSS`/`$CAH`/`$HIT` (0075), footsteps, -//! `$BWP`/`$BWR`, and the `$CSL`-keyed missile release (0430) all fire regardless of the -//! camera. Narrowing this to the flagged set is 1473's event-gate leg — its own record when it -//! lands. +//! - **(ii) off-screen combat is audible for `MORE_AUDIBLE` creatures — and only them** +//! (decision 1482, the tick half of the election): a parked rig's event tracks are not +//! scanned ([`super::events::fire_anim_events`] skips it, memory and all), except when its +//! cached creature template carries the `0x20` flag — the reference's `0x607da0` re-link arm. +//! The driver state machine still arms clips for every parked rig (pose correctness on wake — +//! the memo design of 1370 item 9 is the only safe skip there), so a flagged creature's +//! `$CSS`/`$CAH`/`$HIT` (0075), `$BWP`/`$BWR`, and `$CSL` (0430) fire off-screen exactly as +//! the reference's do. //! //! The park mechanism is the [`AnimParked`] marker alone (decision 0712 — the evaluator took over //! from `animate_targets`, and the old per-joint `AnimatedBy` repoint died with the targets): the @@ -352,6 +353,9 @@ mod tests { )); app.init_asset::(); app.add_message::(); + // The event scanner's MORE_AUDIBLE read (decision 1482) — empty ⇒ every parked rig + // reads unflagged, the fail-closed default. + app.init_resource::(); // The live schedule's shape: the gate ahead of the frame's pose evaluation; the event // scanner reading the advanced seek after it. app.add_systems(PostUpdate, gate_rig_animation.before(AnimationSystems)); @@ -384,13 +388,14 @@ mod tests { .translation } - /// The three gate laws in one flow (decision 0448): an off-frustum rig parks (joints - /// repointed at the park entity, bones frozen) while its seek clock keeps advancing and its - /// event keyframes keep firing (the 0075 trap — off-screen swings must not go silent); a - /// woken rig and a never-parked twin sample the SAME pose — the absolute-clock snap, not - /// resume-from-freeze. + /// The gate laws in one flow (0448, events re-lawed by 1482): an off-frustum rig parks + /// (joints repointed at the park entity, bones frozen) while its seek clock keeps advancing — + /// and its event keyframes FALL SILENT, because the reference's pass-2 walk never ticks an + /// unflagged model (the 0448 "off-screen swings must not go silent" law rested on the verdict + /// wow-re refuted on 2026-08-13; the flagged exception is the next test's). A woken rig and a + /// never-parked twin sample the SAME pose — the absolute-clock snap, not resume-from-freeze. #[test] - fn parked_rig_keeps_clock_and_events_and_wakes_to_the_absolute_pose() { + fn parked_rig_keeps_its_clock_falls_silent_and_wakes_to_the_absolute_pose() { let mut app = app(); spawn_camera(&mut app); // `behind` starts off-frustum (+Z, behind the camera); `twin` stays in frame at −Z. @@ -408,25 +413,29 @@ mod tests { !app.world().entity(twin).contains::(), "the in-frame twin stays live" ); - // While parked: the bone holds, the clock runs, the mid-loop event keyframe still fires. + // While parked: the bone holds, the clock runs — and the event track is not scanned. let frozen = bone(&app, behind_joint); let seek_at_park = seek(&app, behind); let mut cursor = app .world() .resource::>() .get_cursor_current(); - let mut fired = 0usize; + let (mut fired_parked, mut fired_twin) = (0usize, 0usize); for _ in 0..6 { // > one full loop in total, so the keyframe is crossed whatever the phase. std::thread::sleep(std::time::Duration::from_secs_f32(DUR * 0.3)); app.update(); let messages = app.world().resource::>(); - fired += cursor - .read(messages) - .filter(|ev| ev.entity == behind && &ev.ident == b"$TST") - .count(); + for ev in cursor.read(messages).filter(|ev| &ev.ident == b"$TST") { + fired_parked += usize::from(ev.entity == behind); + fired_twin += usize::from(ev.entity == twin); + } } - assert!(fired > 0, "a parked rig's event keyframes keep firing"); + assert_eq!( + fired_parked, 0, + "an unflagged parked rig's event keyframes are not scanned (1482)" + ); + assert!(fired_twin > 0, "the live twin keeps firing — the control"); assert_eq!( frozen, bone(&app, behind_joint), @@ -461,6 +470,63 @@ mod tests { ); } + /// The exception the silence is built around (1482): a parked creature whose cached template + /// carries `MORE_AUDIBLE` (`0x20`) keeps firing — the reference's `0x607da0` pass-2 re-link, + /// the arm that keeps an off-screen flagged creature's combat audible. + #[test] + fn a_more_audible_parked_rig_keeps_firing() { + let mut app = app(); + spawn_camera(&mut app); + let (behind, _) = spawn_rig(&mut app, Vec3::Z * 50.0); + // A creature guid whose entry the cache answers with the flag set — composed the way + // vmangos composes one (`counter | entry << 24 | high << 48`). + const ENTRY: u32 = 69; + let guid = + 7u64 | (u64::from(ENTRY) << 24) | (u64::from(benilla_protocol::guid::HIGH_UNIT) << 48); + app.world_mut() + .entity_mut(behind) + .insert(crate::net::Guid(guid)); + app.world_mut() + .resource_mut::() + .insert_creature( + ENTRY, + Some(crate::names::CreatureRecord { + name: "Growler".into(), + subname: None, + creature_type: 1, + pet_family: 0, + rank: 0, + type_flags: 0x20, + civilian: false, + racial_leader: false, + }), + ); + app.update(); + advance(&mut app, PARK_AFTER_SECS + 0.2, 4); + assert!( + app.world().entity(behind).contains::(), + "flagged or not, the POSE still parks — the flag is tick-only" + ); + let mut cursor = app + .world() + .resource::>() + .get_cursor_current(); + let mut fired = 0usize; + for _ in 0..6 { + std::thread::sleep(std::time::Duration::from_secs_f32(DUR * 0.3)); + app.update(); + let messages = app.world().resource::>(); + fired += cursor + .read(messages) + .filter(|ev| ev.entity == behind && &ev.ident == b"$TST") + .count(); + } + assert!( + fired > 0, + "MORE_AUDIBLE keeps the off-screen event track scanned" + ); + } + /// The 1447 regression: a staged rig (a booth doll — off every world frustum by /// construction) is never this gate's to park. Its `AnimParked` belongs to the booth camera /// gate alone; the night the 1443 collapse put `RigPose` on booth roots, this gate froze diff --git a/crates/benilla-app/src/death.rs b/crates/benilla-app/src/death.rs index b8531f56..d2a825fc 100644 --- a/crates/benilla-app/src/death.rs +++ b/crates/benilla-app/src/death.rs @@ -369,13 +369,27 @@ fn drive_death_look( self_q: Query<&ObjectStore, With>, mut ffx: ResMut, ) { - let ghost = self_q.single().is_ok_and(|store| store.0.player_is_ghost()); + let ghost = ghost_probe() + .unwrap_or_else(|| self_q.single().is_ok_and(|store| store.0.player_is_ghost())); let target = if ghost { 1.0 } else { 0.0 }; if ffx.0 != target { ffx.0 = target; } } +/// `WOW_GHOST_PROBE=1|0` — pin the ghost **screen pass** on or off without dying, so the +/// ghost-world look (and everything that must NOT inherit it — the portrait bakes, decision 1481) +/// can be A/B'd from a capture instead of from a corpse run. It overrides only this look; the +/// death arc's state machine, its events and its UI are untouched and still follow the wire. +fn ghost_probe() -> Option { + static PROBE: std::sync::OnceLock> = std::sync::OnceLock::new(); + *PROBE.get_or_init(|| match std::env::var("WOW_GHOST_PROBE").ok()?.trim() { + "1" | "true" => Some(true), + "0" | "false" => Some(false), + _ => None, + }) +} + /// Per-frame (after `UiInput`, so this frame's clicks drain this frame): map the queued Lua death /// intents onto the wire. fn drain_death( diff --git a/crates/benilla-app/src/go_anim.rs b/crates/benilla-app/src/go_anim.rs index 51456210..c344a7bb 100644 --- a/crates/benilla-app/src/go_anim.rs +++ b/crates/benilla-app/src/go_anim.rs @@ -774,11 +774,27 @@ fn drive_go_collision( /// head, a loop wrap fires tail-then-head — and a frozen rate-0 leg never advances, so it never /// fires. fn fire_go_anim_events( - gos: Query<(Entity, &ModelAnimations, &AnimationPlayer), With>, + gos: Query< + ( + Entity, + &ModelAnimations, + &AnimationPlayer, + Has, + ), + With, + >, mut last: Local, mut out: MessageWriter, ) { - for (entity, anims, player) in &gos { + for (entity, anims, player, parked) in &gos { + // The election's tick half, GO twin (decision 1482): a parked GO's event track is not + // scanned — and there is no `MORE_AUDIBLE` exception here, because the flag lives on + // CREATURE templates only (the reference's re-link arm reads the creature query cache). + // Memory dropped so a wake re-arms instead of scanning the parked gap as one crossing. + if parked { + last.remove(&entity); + continue; + } let playing = anims .clips .iter() diff --git a/crates/benilla-app/src/names.rs b/crates/benilla-app/src/names.rs index 4c45a2dc..dee533fb 100644 --- a/crates/benilla-app/src/names.rs +++ b/crates/benilla-app/src/names.rs @@ -194,6 +194,19 @@ impl NameCache { } } + /// The cached creature template's `type_flags` for a live guid — read-only, no query on a + /// miss. `None` = not a creature guid, or its template answer hasn't landed; the caller that + /// exists for (the parked-event gate, decision 1482) fails CLOSED on `None`, exactly as the + /// reference's `0x623b70` does on a null cached query record. + pub(crate) fn peek_type_flags(&self, guid_val: u64) -> Option { + if guid::pet_number(guid_val).is_some() || !guid::is_creature_or_pet(guid_val) { + return None; // a pet's cache entry is name-only; players/GOs carry no template flags + } + self.creatures + .get(&guid::entry(guid_val)?) + .and_then(|n| n.as_ref().map(|r| r.type_flags)) + } + /// Record a player-name answer. An empty wire name means the server doesn't know the guid — /// cached as a negative answer. /// diff --git a/crates/benilla-app/src/net/apply.rs b/crates/benilla-app/src/net/apply.rs index 06904be4..e39f4a22 100644 --- a/crates/benilla-app/src/net/apply.rs +++ b/crates/benilla-app/src/net/apply.rs @@ -300,7 +300,9 @@ pub(super) fn apply_net_updates( ResMut, Res>, Res, - Query<&mut super::UnitSpeeds>, + // READ-only: every write to a mover's speed set goes through this drain's + // `objects::SpeedStage` and lands in one flush at the end (decision 1478). + Query<&super::UnitSpeeds>, // The PlayAnimation call-order counter (`creature_anim::PlaySeq`): every // animation-bearing message this drain emits stamps `next()`, in packet order. ResMut, @@ -378,6 +380,10 @@ pub(super) fn apply_net_updates( // This also removes a latent clobber: a plain per-delta `insert` on a not-yet-spawned entity would // overwrite an earlier partial rather than merge it (decision 0061). let mut pending: HashMap = HashMap::new(); + // The same deferral trap for movers' speeds, and the one B213 fell into: a create's + // `UnitSpeeds` insert is a Command, so a `SMSG_FORCE_*_SPEED_CHANGE` arriving later in this + // same drain could not land on top of it. Both stage here in packet order (decision 1478). + let mut speed_stage = objects::SpeedStage::default(); for ev in events.0.try_iter() { match ev { SessionEvent::LoginStage { stage } => session::login_stage(stage, &mut login_stages), @@ -479,6 +485,7 @@ pub(super) fn apply_net_updates( &mut transforms, &mut stores, &mut pending, + &mut speed_stage, &mut names, &mut go_templates, &net_commands, @@ -1369,12 +1376,13 @@ pub(super) fn apply_net_updates( counter, speed, &index, - &mut aura.3, + &aura.3, + &mut speed_stage, &self_guid, &mut speed_changes, ), SessionEvent::SpeedChanged { guid, kind, speed } => { - objects::speed_changed(guid, kind, speed, &index, &mut aura.3) + objects::speed_changed(guid, kind, speed, &index, &aura.3, &mut speed_stage) } // ── The mount arc (decision 0441) — arm bodies in `mount` ───────────────────────── SessionEvent::MountResult { mount, code } => { @@ -1453,6 +1461,9 @@ pub(super) fn apply_net_updates( commands.entity(e).insert(ObjectStore(fields)); } } + // And the movers' speed sets, for the same reason (decision 1478) — one insert each, carrying + // the create block and every force-change this drain saw, folded in the order they arrived. + speed_stage.flush(&mut commands, &index); } /// Tag our own player's streamed entity with [`SelfPlayer`] once we know our guid — by matching the diff --git a/crates/benilla-app/src/net/apply/objects.rs b/crates/benilla-app/src/net/apply/objects.rs index d9f7f171..927e9160 100644 --- a/crates/benilla-app/src/net/apply/objects.rs +++ b/crates/benilla-app/src/net/apply/objects.rs @@ -3,6 +3,7 @@ //! and the creature path packet. Each `pub(super)` fn here is exactly one arm's body; the match at //! the call site stays the dispatcher, one call per arm. +use std::collections::hash_map::Entry; use std::collections::HashMap; use benilla_assets::coords::bevy_to_wow; @@ -78,6 +79,7 @@ pub(super) fn object_create( transforms: &mut Query<&mut Transform>, stores: &mut Query<&mut ObjectStore>, pending: &mut HashMap, + speed_stage: &mut SpeedStage, names: &mut NameCache, go_templates: &mut GameObjectTemplates, net_commands: &NetCommands, @@ -175,8 +177,11 @@ pub(super) fn object_create( // Re-create of a tracked guid: refresh identity + pose. A create is a fresh server snapshot, so // any in-flight extrapolation is stale too — clear it. commands.entity(e).insert(net).remove::(); + // Speeds go through the drain's stage, never straight onto the entity — a + // `SMSG_FORCE_*_SPEED_CHANGE` riding the same tick as this create has to be able to land + // on top of it (decision 1478, B213). if let Some(s) = speeds { - commands.entity(e).insert(UnitSpeeds(s)); + speed_stage.seed(guid, s); } if let Some(anchor) = transport_anchor { commands.entity(e).insert(anchor); @@ -227,7 +232,7 @@ pub(super) fn object_create( // root's `Visibility` through `Mut` writes, which don't re-add the sweep row. entity.vis_chain_only(); if let Some(s) = speeds { - entity.insert(UnitSpeeds(s)); + speed_stage.seed(guid, s); // staged, like the re-create arm above (1478) } if let Some(anchor) = transport_anchor { entity.insert(anchor); @@ -589,17 +594,55 @@ fn merge_fields( } } -/// Write one speed-kind slot on a mover's speed set — the two speed arms' shared body (the -/// extrapolator + the anim selector read the set). -fn set_speed( - index: &GuidIndex, - speeds: &mut Query<&mut UnitSpeeds>, - guid: u64, - kind: SpeedKind, - speed: f32, -) { - if let Some(s) = index.0.get(&guid).and_then(|&e| speeds.get_mut(e).ok()) { - let s = &mut s.into_inner().0; +/// Per-drain staging for every mover's [`UnitSpeeds`] — the drain's single speed writer +/// (decision 1478). +/// +/// The two speed sources used to disagree about *when* they land. [`object_create`] inserts a whole +/// speed set through `Commands`, which is deferred to the drain's sync point; a +/// `SMSG_FORCE_*_SPEED_CHANGE` edited one slot of the live component immediately. Mixed in one +/// drain, the **later packet loses** either way: on an entity born this drain the component isn't +/// there yet so the query miss was silent, and on one that already existed the create's deferred +/// insert landed afterwards and overwrote it. +/// +/// vmangos puts exactly those two packets back to back on purpose. `HandleMoveWorldportAckOpcode` +/// sends the self create block inside `Map::Add` → `SendInitSelf` (carrying the speeds the player +/// still has) and then, three statements later, strips the mount on a map that forbids one +/// (`if (!mEntry->IsMountAllowed()) RemoveSpellsCausingAura(SPELL_AURA_MOUNTED)`) — whose +/// `SMSG_FORCE_RUN_SPEED_CHANGE` therefore rides the same tick, and so the same drain. That is +/// B213: `.tele` into BWL/LBRS on a mount auto-dismounted you and left the mount's run speed on +/// foot, because the create's 11.2 yd/s landed last. +/// +/// So both sources stage here instead, **in packet order**, and the drain flushes once at the end — +/// the `pending: HashMap` pattern (decision 0061) applied to speeds. Nothing +/// writes the component directly any more, which is what makes the order the wire's order. +#[derive(Default)] +pub(super) struct SpeedStage(HashMap); + +impl SpeedStage { + /// A create block's whole speed set. A create is the server's newest snapshot of the mover, so + /// it **replaces** anything staged for that guid rather than merging into it. + fn seed(&mut self, guid: u64, speeds: MoveSpeeds) { + self.0.insert(guid, speeds); + } + + /// Write one speed-kind slot, seeding the staged set from the live component on first touch. + /// `live` is only called when nothing is staged yet; `None` from it means there is nowhere to + /// write — an untracked guid, or a mover whose create carried no movement block — and the + /// change is reported unapplied rather than silently dropped. + fn set( + &mut self, + guid: u64, + kind: SpeedKind, + speed: f32, + live: impl FnOnce() -> Option, + ) -> bool { + let s = match self.0.entry(guid) { + Entry::Occupied(o) => o.into_mut(), + Entry::Vacant(v) => match live() { + Some(s) => v.insert(s), + None => return false, + }, + }; match kind { SpeedKind::Walk => s.walk = speed, SpeedKind::Run => s.run = speed, @@ -608,10 +651,31 @@ fn set_speed( SpeedKind::SwimBack => s.swim_back = speed, SpeedKind::TurnRate => s.turn_rate = speed, } + true + } + + /// Land every staged set on its entity, now that this drain's spawn `Command`s have run. + /// `try_insert`: the same drain may have queued a despawn for it (a stream-out, the worldport + /// purge), and a guid the purge dropped from the index resolves to nothing at all. + pub(super) fn flush(self, commands: &mut Commands, index: &GuidIndex) { + for (guid, speeds) in self.0 { + if let Some(&e) = index.0.get(&guid) { + commands.entity(e).try_insert(UnitSpeeds(speeds)); + } + } } } -/// A forced speed change on a mover (aura/mount/GM `.modify speed`): apply the new value to the +/// Read a mover's live speed set — [`SpeedStage::set`]'s seed for a mover that already exists. +fn live_speeds(index: &GuidIndex, speeds: &Query<&UnitSpeeds>, guid: u64) -> Option { + index + .0 + .get(&guid) + .and_then(|&e| speeds.get(e).ok()) + .map(|s| s.0) +} + +/// A forced speed change on a mover (aura/mount/GM `.modify speed`): stage the new value onto the /// entity's speed set, and — when the mover is our own player — forward to the controller, which /// answers the mandatory ack with its live pose (the TeleportMessage pattern). An unknown guid /// still acks if it's ours-by-guid; a foreign mover (we never control others) is only applied, @@ -623,13 +687,23 @@ pub(super) fn force_speed_change( counter: u32, speed: f32, index: &GuidIndex, - speeds: &mut Query<&mut UnitSpeeds>, + speeds: &Query<&UnitSpeeds>, + stage: &mut SpeedStage, self_guid: &SelfGuid, speed_changes: &mut MessageWriter, ) { - set_speed(index, speeds, guid, kind, speed); + let applied = stage.set(guid, kind, speed, || live_speeds(index, speeds, guid)); if self_guid.0 == Some(guid) { info!("net: force {kind:?} speed change -> {speed} yd/s (counter {counter})"); + // Our own mover having no speed set to write is B213's failure shape, and it was silent + // for as long as it existed. It should be unreachable now that the create stages too — + // so if it ever fires, the next report starts from a line instead of from a guess. + if !applied { + warn!( + "net: force {kind:?} speed change for OUR mover {guid:#x} landed nowhere \ + (no speed set staged or live) — we still ack, so the server and we now disagree" + ); + } speed_changes.write(SpeedChangeMessage { guid, kind, @@ -637,19 +711,142 @@ pub(super) fn force_speed_change( speed, }); } else { - debug!("net: force speed change for foreign mover {guid:#x} — applied, no ack"); + debug!("net: force speed change for foreign mover {guid:#x} — applied {applied}, no ack"); } } /// An observed unit's speed changed (the SPLINE_SET / MOVE_SET families — another player -/// mounting up, a hastened creature): apply to its speed set, nothing to ack (decision 0441). -/// The MOVE_SET flavour's pose already arrived as its own UnitMove. +/// mounting up, a hastened creature): stage it onto that unit's speed set, nothing to ack +/// (decision 0441). The MOVE_SET flavour's pose already arrived as its own UnitMove. pub(super) fn speed_changed( guid: u64, kind: SpeedKind, speed: f32, index: &GuidIndex, - speeds: &mut Query<&mut UnitSpeeds>, + speeds: &Query<&UnitSpeeds>, + stage: &mut SpeedStage, ) { - set_speed(index, speeds, guid, kind, speed); + if !stage.set(guid, kind, speed, || live_speeds(index, speeds, guid)) { + // Ordinary: a unit's speed broadcast can outrun its create block. Nothing to ack and the + // create that follows carries the same set, so this self-heals. + debug!("net: {kind:?} speed change for untracked mover {guid:#x} — nothing to write"); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + const ME: u64 = 0x0000_0000_0000_002A; + + /// A dismounted human's set — 1.12.1's base run 7.0 (the `--speed` probe's own golden). + fn on_foot() -> MoveSpeeds { + MoveSpeeds { + walk: 2.5, + run: 7.0, + run_back: 4.5, + swim: 4.722_222, + swim_back: 2.5, + turn_rate: std::f32::consts::PI, + } + } + + /// The same set with a 60% mount's run speed on it — what `SendInitSelf` puts in the create + /// block of a worldport that is *about* to strip the mount. + fn mounted() -> MoveSpeeds { + MoveSpeeds { + run: 11.2, + ..on_foot() + } + } + + /// **B213, pinned.** vmangos's `HandleMoveWorldportAckOpcode` sends the self create block + /// (`Map::Add` → `SendInitSelf`, still carrying the mount's 11.2 yd/s) and then, three + /// statements later, strips the mount because the destination map forbids one — so + /// `SMSG_FORCE_RUN_SPEED_CHANGE` 7.0 rides the same tick and lands in the same drain. + /// Staged in packet order the change wins; before 1478 the create's *deferred* `UnitSpeeds` + /// insert landed last and `.tele` into BWL left the avatar running at mount speed on foot. + #[test] + fn a_force_change_beats_a_create_from_the_same_drain() { + let mut stage = SpeedStage::default(); + stage.seed(ME, mounted()); + assert!(stage.set(ME, SpeedKind::Run, 7.0, || panic!( + "the create staged a set this drain — the live component must not be consulted" + ))); + stage.flush_into(|guid, s| { + assert_eq!(guid, ME); + assert_eq!(s.run, 7.0, "the dismount is the newer packet"); + assert_eq!( + s.walk, 2.5, + "the other slots still come from the create block" + ); + }); + } + + /// The reverse order is just as much the wire's order: a create block is the server's newest + /// snapshot of the mover, so one arriving *after* a change replaces it whole rather than + /// merging under it. + #[test] + fn a_create_after_a_change_replaces_it() { + let mut stage = SpeedStage::default(); + assert!(stage.set(ME, SpeedKind::Run, 7.0, || Some(mounted()))); + stage.seed(ME, mounted()); + stage.flush_into(|_, s| assert_eq!(s.run, 11.2)); + } + + /// Nothing staged yet (an entity that has existed since an earlier drain): the live component + /// seeds the cell, so a one-slot change keeps every other slot it isn't addressing. + #[test] + fn a_change_on_a_live_mover_seeds_from_the_component() { + let mut stage = SpeedStage::default(); + assert!(stage.set(ME, SpeedKind::Run, 11.2, || Some(on_foot()))); + stage.flush_into(|_, s| { + assert_eq!(s.run, 11.2); + assert_eq!(s.run_back, on_foot().run_back); + assert_eq!(s.turn_rate, on_foot().turn_rate); + }); + } + + /// A change with nowhere to land — an untracked guid, or a mover whose create carried no + /// movement block — reports itself unapplied rather than vanishing. It stages nothing, so a + /// later create can't inherit a slot invented for a set that never existed. + #[test] + fn a_change_with_no_speed_set_is_reported_not_swallowed() { + let mut stage = SpeedStage::default(); + assert!(!stage.set(ME, SpeedKind::Run, 7.0, || None)); + let mut flushed = 0; + stage.flush_into(|_, _| flushed += 1); + assert_eq!(flushed, 0); + } + + /// The trap itself, machine-checked rather than asserted in prose: a component inserted + /// through this drain's `Commands` is **not** visible to a query in the same drain — the spawn + /// only runs at the sync point. That is why staging is the fix and why a live-component write + /// could never have been the fix: the force-change arm had nothing to write onto. + #[test] + fn a_component_this_drain_queued_is_invisible_until_the_sync_point() { + use bevy::ecs::system::RunSystemOnce; + let mut world = World::new(); + let missed = world + .run_system_once(|mut commands: Commands, live: Query<&UnitSpeeds>| { + let e = commands.spawn(UnitSpeeds(mounted())).id(); + live.get(e).is_err() + }) + .expect("the probe system runs"); + assert!( + missed, + "a create's UnitSpeeds must be unreadable later in the same drain — if this ever \ + passes, `Commands` stopped being deferred and 1478's staging can be reconsidered" + ); + } + + impl SpeedStage { + /// [`SpeedStage::flush`] without the ECS half — the staged sets in the shape the flush + /// would insert them, so the ordering rules above are testable as the pure thing they are. + fn flush_into(self, mut f: impl FnMut(u64, MoveSpeeds)) { + for (guid, speeds) in self.0 { + f(guid, speeds); + } + } + } } diff --git a/crates/benilla-protocol/src/bin/benilla-world/main.rs b/crates/benilla-protocol/src/bin/benilla-world/main.rs index b1af4964..c86961fd 100644 --- a/crates/benilla-protocol/src/bin/benilla-world/main.rs +++ b/crates/benilla-protocol/src/bin/benilla-world/main.rs @@ -22,9 +22,9 @@ use benilla_protocol::{decode, EntityKind, WorldSession, WORLD_PORT}; use clap::Parser; use probes::{ - Attack, Aura, Charge, Ctx, Death, EquipPackSlot, GiverStatus, GroundFx, Loot, OpenItem, Probe, - QueryNames, Quest, QuestItem, QuestLog, QuestTimer, Speed, Spells, Spirit, SwapPackSlots, - UsePackSlot, Vendor, WorldState, + Attack, Aura, Charge, Ctx, Death, EquipPackSlot, GiverStatus, GroundFx, Loot, MountTele, + OpenItem, Probe, QueryNames, Quest, QuestItem, QuestLog, QuestTimer, Speed, Spells, Spirit, + SwapPackSlots, UsePackSlot, Vendor, WorldState, }; use world::{DeathArc, Tracked, World}; @@ -182,6 +182,19 @@ struct Cli { #[arg(long)] speed: bool, + /// Live-verify what the server sends when a teleport DISMOUNTS you (B213, decision 1478): + /// `.aura 458` (Brown Horse — a real `SPELL_AURA_MOUNTED` holder, unlike `.modify mount`), + /// require the mounted `SMSG_FORCE_RUN_SPEED_CHANGE`, then `.go xyz` into Ragefire Chasm + /// (map 389 — a dungeon, so `MapEntry::IsMountAllowed()` is false) and require, in this order: + /// `SMSG_NEW_WORLD`, our own create block on the new map **still carrying the mount's** run + /// speed (vmangos sends it from `Map::Add` → `SendInitSelf`), and then the strip's + /// `SMSG_FORCE_RUN_SPEED_CHANGE` back at 7.0 — printing the gap between the two. Both are + /// written by one `HandleMoveWorldportAckOpcode` call, which is why a client draining its + /// socket once a frame sees them in a single drain. Leaves the character unmounted and back on + /// map 0. Needs a GM account (the deploy's probes are gmlevel 6); pair with `--seconds 30`+. + #[arg(long)] + mount_tele: bool, + /// Live-verify the aura wire (decision 0255 phase 1): GM-apply Mark of the Wild (1126, a /// cancelable buff) and Shadow Word: Pain (589, a periodic-damage DoT, unambiguously negative) /// to ourselves with explicit durations via `.aura`, then require — from the *live* descriptor @@ -398,6 +411,9 @@ fn main() -> Result<()> { if cli.speed { probes.push(Box::new(Speed::default())); } + if cli.mount_tele { + probes.push(Box::new(MountTele::default())); + } if cli.death { probes.push(Box::new(Death::default())); } diff --git a/crates/benilla-protocol/src/bin/benilla-world/probes/mod.rs b/crates/benilla-protocol/src/bin/benilla-world/probes/mod.rs index 7e252747..75c01288 100644 --- a/crates/benilla-protocol/src/bin/benilla-world/probes/mod.rs +++ b/crates/benilla-protocol/src/bin/benilla-world/probes/mod.rs @@ -67,6 +67,7 @@ mod equip_pack_slot; mod giverstatus; mod groundfx; mod loot; +mod mount_tele; mod open_item; mod query_names; mod quest; @@ -89,6 +90,7 @@ pub(crate) use equip_pack_slot::EquipPackSlot; pub(crate) use giverstatus::GiverStatus; pub(crate) use groundfx::GroundFx; pub(crate) use loot::Loot; +pub(crate) use mount_tele::MountTele; pub(crate) use open_item::OpenItem; pub(crate) use query_names::QueryNames; pub(crate) use quest::Quest; diff --git a/crates/benilla-protocol/src/bin/benilla-world/probes/mount_tele.rs b/crates/benilla-protocol/src/bin/benilla-world/probes/mount_tele.rs new file mode 100644 index 00000000..731d9cf4 --- /dev/null +++ b/crates/benilla-protocol/src/bin/benilla-world/probes/mount_tele.rs @@ -0,0 +1,216 @@ +//! `--mount-tele`: **what the server actually sends when a teleport dismounts you** — B213's wire. +//! +//! The report: mount up, teleport somewhere that forbids mounting, and the auto-dismount leaves the +//! mount's run speed on your feet. vmangos does that strip inside the worldport ack handler, three +//! statements after the destination map's own create block goes out +//! (`MovementHandler.cpp` `HandleMoveWorldportAckOpcode`: `Map::Add` → `SendInitSelf`, then +//! `if (!mEntry->IsMountAllowed()) RemoveSpellsCausingAura(SPELL_AURA_MOUNTED)`), so the two land +//! back to back in one tick. Reading the source says they *should*; this probe measures that they +//! *do*, on the running deploy, and prints the gap. +//! +//! The scenario: `.aura` a real mount spell on open ground (a genuine `SPELL_AURA_MOUNTED` holder — +//! `.modify mount` sets a display id with no aura and would never be stripped), require the mounted +//! `SMSG_FORCE_RUN_SPEED_CHANGE`, then `.go xyz` into a dungeon map and require, in order: the +//! `SMSG_NEW_WORLD`, a self create block still carrying the **mounted** run speed, and the +//! dismount's `SMSG_FORCE_RUN_SPEED_CHANGE` back at base. Needs a GM account (the deploy's probes +//! are gmlevel 6). + +use std::time::Instant; + +use anyhow::{ensure, Result}; +use benilla_protocol::{SessionEvent, SpeedKind}; + +use crate::probes::{Ctx, Probe}; +use crate::world::ATTACK_TP; + +/// Brown Horse (the 60% apprentice mount): `SPELL_AURA_MOUNTED` + `SPELL_AURA_MOD_INCREASE_MOUNTED_SPEED`, +/// so `.aura` on it produces the same holder a real mount cast would — the one +/// `RemoveSpellsCausingAura(SPELL_AURA_MOUNTED)` goes looking for. +const MOUNT_SPELL: u32 = 458; + +/// Ragefire Chasm. A dungeon, and not one of `MapEntry::IsMountAllowed`'s four exceptions +/// (Zul'Gurub, Zul'Farrak, AQ Ruins, Caverns of Time), so arriving there strips the mount. +const DUNGEON_MAP: u32 = 389; + +/// Inside Ragefire Chasm's entrance. The landing spot does not matter — the packets we want are +/// sent by the arrival itself — but a real one keeps the character somewhere sane if a run is +/// interrupted before the exit teleport in `verify`. +const DUNGEON_TP: &str = ".go xyz 3.0 -14.0 -18.0 389"; + +/// 1.12.1's base run speed (yd/s) — the value the strip must come back to. +const BASE_RUN: f32 = 7.0; + +/// The map the scenario has to start on — open ground where a mount is allowed. +const OUTDOOR_MAP: u32 = 0; + +#[derive(Default)] +pub(crate) struct MountTele { + /// Sent `.aura` once we were confirmed standing on [`OUTDOOR_MAP`]. + mount_requested: bool, + /// The mounted run speed the pre-teleport `SMSG_FORCE_RUN_SPEED_CHANGE` announced. + mounted_run: Option, + ported: bool, + /// `SMSG_NEW_WORLD`'s map, and when it landed. + arrived: Option<(u32, Instant)>, + /// Our own create block on the destination map: its `LIVING` run speed + arrival instant. + create_after: Option<(f32, Instant)>, + /// The first self Run force-change after that create: its speed + arrival instant. + strip_after: Option<(f32, Instant)>, +} + +impl Probe for MountTele { + fn stage(&mut self, cx: &mut Ctx) -> Result<()> { + // Get onto open ground and clean of any leftover holder FIRST — an interrupted earlier run + // can leave the character mounted inside the dungeon, and from there the scenario's own + // teleport is a same-map port that sends no `SMSG_NEW_WORLD` at all. The map id is + // explicit for the same reason `verify`'s exit is (`.go xyz`'s map argument defaults to + // the map you are standing on). Mounting waits for the arrival, in `poll`. + cx.session.send_chat(&format!(".unaura {MOUNT_SPELL}"))?; + cx.session + .send_chat(&format!("{ATTACK_TP} {OUTDOOR_MAP}"))?; + cx.world.attack_tp_staged = true; + Ok(()) + } + + fn on_event(&mut self, ev: &SessionEvent, cx: &mut Ctx) -> Result<()> { + match ev { + // The mount landed: its speed change is our cue to teleport. Gate on the *tracked* + // self like `--speed` does — World only acks once our pose is known, and an unacked + // change is one vmangos has not applied yet. + SessionEvent::ForceSpeedChange { + guid, kind, speed, .. + } if *guid == cx.world.self_guid && *kind == SpeedKind::Run => { + if !cx.world.tracked.contains_key(guid) { + return Ok(()); + } + if self.ported { + // Post-arrival: the first Run change after the destination's create block is + // the dismount we came for. + if self.create_after.is_some() && self.strip_after.is_none() { + self.strip_after = Some((*speed, Instant::now())); + } + } else if self.mounted_run.is_none() && *speed > BASE_RUN + 0.05 { + self.mounted_run = Some(*speed); + println!("mounted: run {speed} yd/s — teleporting into map {DUNGEON_MAP}"); + } + } + // `SMSG_LOGIN_VERIFY_WORLD` decodes to this too, so login itself announces map 0 — + // gate on having actually sent the teleport, or the whole scenario latches onto the + // login and every later check reads the wrong packets. + SessionEvent::Worldport { map_id, .. } if self.ported && self.arrived.is_none() => { + self.arrived = Some((*map_id, Instant::now())); + println!("SMSG_NEW_WORLD: map {map_id}"); + } + // Our own create on the destination map — sent by `Map::Add` → `SendInitSelf`, before + // the strip. Its LIVING block is the speed set the client seeds a fresh entity with. + SessionEvent::ObjectCreate { guid, speeds, .. } + if *guid == cx.world.self_guid && self.ported && self.arrived.is_some() => + { + if let (None, Some(s)) = (self.create_after, speeds) { + self.create_after = Some((s.run, Instant::now())); + println!("self create block on the new map: run {} yd/s", s.run); + } + } + _ => {} + } + Ok(()) + } + + fn poll(&mut self, cx: &mut Ctx) -> Result<()> { + // Mount only once the staging teleport has actually landed us outdoors — proving the run's + // starting state from a packet rather than assuming it (`method.md` step 6). + if !self.mount_requested && cx.world.self_map == OUTDOOR_MAP { + self.mount_requested = true; + cx.session.send_chat(&format!(".aura {MOUNT_SPELL}"))?; + println!( + "on map {OUTDOOR_MAP}; sent GM: .aura {MOUNT_SPELL} (Brown Horse) — expecting the \ + mounted run speed" + ); + } + if self.mounted_run.is_some() && !self.ported { + self.ported = true; + cx.session.send_chat(DUNGEON_TP)?; + } + Ok(()) + } + + fn verify(&mut self, cx: &mut Ctx) -> Result<()> { + // Leave the character as found before asserting: a failed run must not strand it in a + // dungeon on a mount. The exit names map 0 explicitly — `.go xyz`'s map argument is + // OPTIONAL and defaults to the map you are standing on, so the bare [`ATTACK_TP`] would + // port us to Northshire's coordinates *inside Ragefire Chasm*. + cx.session.send_chat(&format!(".unaura {MOUNT_SPELL}"))?; + cx.session + .send_chat(&format!("{ATTACK_TP} {OUTDOOR_MAP}"))?; + // …and stay on the wire long enough to ACK that exit port. A far teleport the client never + // acks does not happen: dropping the session here would leave the probe character standing + // in a dungeon for the next run to find. Bounded, and only advisory — `stage` recovers from + // a stranded character anyway, so a quiet stream here is not a failure. + let until = Instant::now() + std::time::Duration::from_secs(5); + while Instant::now() < until && cx.world.self_map != OUTDOOR_MAP { + match cx.session.recv() { + Ok(msg) => { + for ev in benilla_protocol::decode(msg) { + cx.world.on_event(&ev, cx.session)?; + } + } + Err(_) => continue, + } + } + + ensure!( + self.mount_requested, + "--mount-tele: never reached map {OUTDOOR_MAP} — the staging teleport did not land \ + (raise --seconds, or check the `.go` GM level)" + ); + let mounted = self.mounted_run.ensure_some( + "--mount-tele: no mounted run speed arrived — did `.aura` take (GM account?), or does \ + this build's mount spell differ?", + )?; + let (map, _) = self + .arrived + .ensure_some("--mount-tele: no SMSG_NEW_WORLD — the `.go xyz ` never ported us")?; + ensure!( + map == DUNGEON_MAP, + "--mount-tele: ported to map {map}, wanted {DUNGEON_MAP}" + ); + let (create_run, create_at) = self.create_after.ensure_some( + "--mount-tele: no self create block on the destination map — was the worldport acked?", + )?; + let (strip_run, strip_at) = self.strip_after.ensure_some( + "--mount-tele: the arrival sent no Run force-change — this map did not strip the mount", + )?; + + ensure!( + (create_run - mounted).abs() < 0.01, + "--mount-tele: the destination's create block should still carry the MOUNTED run \ + speed ({mounted}), got {create_run} — the strip would then precede the create and \ + B213's ordering would not be this one" + ); + ensure!( + (strip_run - BASE_RUN).abs() < 0.01, + "--mount-tele: the dismount should restore the {BASE_RUN} base run speed, got {strip_run}" + ); + let gap = strip_at.duration_since(create_at); + println!( + "\n--mount-tele PASS: arrived on map {map}; create block carried the mount's \ + {create_run} yd/s, the dismount's SMSG_FORCE_RUN_SPEED_CHANGE followed {} µs later \ + at {strip_run} yd/s.\n Both packets are written by one HandleMoveWorldportAckOpcode \ + call, so a client that drains its socket once a frame sees them in ONE drain — which \ + is what B213 (decision 1478) turns on.", + gap.as_micros() + ); + Ok(()) + } +} + +/// `Option::ok_or_else` with the probe's message shape, so each `verify` line reads as one claim. +trait EnsureSome { + fn ensure_some(&self, msg: &str) -> Result; +} + +impl EnsureSome for Option { + fn ensure_some(&self, msg: &str) -> Result { + self.ok_or_else(|| anyhow::anyhow!("{msg}")) + } +} diff --git a/crates/benilla-protocol/src/bin/benilla-world/world.rs b/crates/benilla-protocol/src/bin/benilla-world/world.rs index e74c8f00..37bbcaef 100644 --- a/crates/benilla-protocol/src/bin/benilla-world/world.rs +++ b/crates/benilla-protocol/src/bin/benilla-world/world.rs @@ -386,6 +386,34 @@ impl World { ); self.speed_changes_seen.push((*kind, *counter, *speed)); } + // A cross-map port. Unacked, `HandleMoveWorldportAckOpcode` never runs, so the + // destination map streams NOTHING — no self create, and none of the arrival's own + // side effects (`--mount-tele`'s subject: the mount strip a map that forbids mounting + // performs right there). The tracked set is deliberately NOT purged here: the ack for + // whatever the arrival sends needs a pose, and ours is the landing point the packet + // just gave us. + SessionEvent::Worldport { + map_id, + position, + orientation, + needs_ack, + } => { + self.self_map = *map_id; + if let Some(t) = self.tracked.get_mut(&self.self_guid) { + t.position = *position; + t.orientation = *orientation; + } + if *needs_ack { + session.worldport_ack()?; + } + println!( + "SMSG_NEW_WORLD: map {map_id} at ({:.1}, {:.1}, {:.1}){}", + position[0], + position[1], + position[2], + if *needs_ack { " — ack sent" } else { "" } + ); + } SessionEvent::Teleport { guid, counter, diff --git a/crates/benilla-protocol/src/world/session.rs b/crates/benilla-protocol/src/world/session.rs index 8ae15c42..b25640e4 100644 --- a/crates/benilla-protocol/src/world/session.rs +++ b/crates/benilla-protocol/src/world/session.rs @@ -659,6 +659,15 @@ impl WorldSession { ) } + /// Echo a cross-map worldport ack — the unsplit twin of [`WorldWriter::worldport_ack`] + /// (`MSG_MOVE_WORLDPORT_ACK`, empty body). Without it the server never runs + /// `HandleMoveWorldportAckOpcode`, so nothing on the destination map is ever streamed: no self + /// create block, and none of the arrival's own side effects (the `--mount-tele` probe's whole + /// subject — the mount strip a map that forbids mounting performs right there). + pub fn worldport_ack(&mut self) -> Result<()> { + self.send(opcode::MSG_MOVE_WORLDPORT_ACK, &[]) + } + /// **Acknowledge a granted mover mode** — root, water-walk, feather-fall or hover (the ack'd /// family; decision 0866) — the unsplit twin of [`WorldWriter::move_mode_ack`], for the /// `--death` probe: the server roots us at death, unroots and grants walk-on-water at release, diff --git a/crates/benilla-world/src/ffx_glow.rs b/crates/benilla-world/src/ffx_glow.rs index 24648161..34426228 100644 --- a/crates/benilla-world/src/ffx_glow.rs +++ b/crates/benilla-world/src/ffx_glow.rs @@ -12,6 +12,12 @@ //! (plus its debug toggle) is gone. Applied to both the world camera and the portrait-booth bake //! cameras (`portrait::mod`): the booth rides the same final node so its bake reads at exact world //! parity, the FFXGlow combine owning the frame's ONE gamma decode. +//! +//! **What a bake does NOT inherit.** The node's two other lanes are keyed on the *viewer's own +//! state*, not on the scene — the drunk/underwater haze and the ghost's FFXDeath combine — and the +//! reference runs its FFX pass inside the WorldFrame's paint, with every UI frame compositing +//! afterwards. [`FfxGlow::state_scale`] is that whole class in one field, `0` on every bake +//! (decision 1481). use bevy::core_pipeline::core_3d::graph::{Core3d, Node3d}; use bevy::core_pipeline::FullscreenShader; @@ -42,27 +48,41 @@ use crate::view::WorldCamera; pub struct FfxGlow { /// Per-view multiplier on the zone glow weight. See [`Self::WORLD`] / [`Self::UI_PANE`]. pub(crate) gain_scale: f32, - /// Per-view multiplier on the haze mix `z` ([`FfxHazeMix`] — the drunk/underwater - /// screen-toward-blur cross-fade). `1.0` only on the true world view: the haze is a - /// *player-state* effect on the viewed scene, and a booth bake that stands in for a UI pane - /// must never inherit it (the reference paints UI model widgets after the FFX pass). - pub(crate) haze_scale: f32, + /// Per-view multiplier on the whole **player-state** class of screen effects: the haze mix + /// `z` ([`FfxHazeMix`] — the drunk/underwater screen-toward-blur cross-fade) *and* the + /// FFXDeath gate `y` ([`FfxDeathFade`] — the ghost combine). `1.0` only on the true world + /// view. + /// + /// **The law, and why it is ONE field.** The reference runs its FFX pass inside the + /// WorldFrame's own paint (`0x48350e` → the apply hook `0x6cd890`, wow-re `ffxeffects.md`); + /// every UI frame — a `` pane, and our portrait bakes standing in for one — + /// composites *afterwards*, so no effect keyed on the **viewer's own state** can reach a bake. + /// The zone glow is not in this class (it is authored scene data, and a bake wants world + /// parity for it — decision 0638), which is why [`Self::gain_scale`] stays separate. + /// + /// The haze had its own `haze_scale` and the death gate had none, so a released ghost's + /// portraits baked through the FFXDeath combine and came back steel-blue luma (report B49, + /// decision 1481). Naming the *class* rather than the member is the fix: the next + /// player-state pass is gated by construction. + pub(crate) state_scale: f32, } impl FfxGlow { /// The reference's own full-screen glow, at the zone's authored weight — the world camera: - /// zone glow AND the player-state haze (drunk / camera-eye-submerged blur). + /// zone glow AND the player-state passes (drunk / camera-eye-submerged blur, ghost death + /// combine). The only view that carries the latter. pub const WORLD: Self = Self { gain_scale: 1.0, - haze_scale: 1.0, + state_scale: 1.0, }; /// A portrait/booth bake at world parity for *lighting and glow* (decision 0638) — but never - /// the haze: a bake stands in for a UI model widget, which the reference composites after - /// the WorldFrame's FFX pass, so a drunk player's unit frame stays sharp while the world - /// swims. + /// a player-state pass: a bake stands in for a UI model widget, which the reference + /// composites after the WorldFrame's FFX pass. So a drunk player's unit frame stays sharp + /// while the world swims, and a **ghost's portrait keeps its living face** while the world + /// goes steel-blue (decision 1481, report B49). pub const BOOTH: Self = Self { gain_scale: 1.0, - haze_scale: 0.0, + state_scale: 0.0, }; /// **Decode only, no glow** — for a bake that stands in for a 1.12 *UI model widget*. The /// reference applies its FFX pass inside the WorldFrame's own paint (`0x48350e` → the apply @@ -71,7 +91,7 @@ impl FfxGlow { /// glows itself. (The give-away in-game: the reference's chat text and buttons don't bloom.) pub const UI_PANE: Self = Self { gain_scale: 0.0, - haze_scale: 0.0, + state_scale: 0.0, }; } @@ -89,7 +109,9 @@ pub struct FfxGlowGain(pub f32); /// The FFXDeath gate (decision 0308 §7, byte-VERIFIED wow-re death-pass.md): `1.0` while the /// player is a released ghost — the combine swaps to the FFXDeath program whole — else `0.0`. /// INSTANT on both edges (the client has no time ramp; the ghost tint is a shader constant). -/// Driven by [`crate::death`] off `PLAYER_FLAGS_GHOST`; uploaded as the combine uniform's `y`. +/// Driven by `benilla-app`'s death arc off `PLAYER_FLAGS_GHOST`; uploaded as the combine +/// uniform's `y`, scaled per view by [`FfxGlow::state_scale`] — it is a **player-state** pass and +/// reaches the world view only (decision 1481). #[derive(Resource, Clone, Default, ExtractResource)] pub struct FfxDeathFade(pub f32); @@ -100,7 +122,7 @@ pub struct FfxDeathFade(pub f32); /// `z = max(min(drunkByte,100)/100, submerged ? 84/255 : 0)` — fully blurred at 100 inebriation, /// and a fixed ≈0.329 floor whenever the **camera eye** is in any liquid (the vanilla underwater /// blur; `0x672470`'s eye-liquid probe, `0xf` = dry). Synced by [`sync_haze`]; uploaded as the -/// combine uniform's `z`, scaled per view by [`FfxGlow::haze_scale`]. +/// combine uniform's `z`, scaled per view by [`FfxGlow::state_scale`]. #[derive(Resource, Clone, Default, ExtractResource)] pub struct FfxHazeMix(pub f32); @@ -358,6 +380,25 @@ fn prepare_textures( } } +/// The combine's `(x, y, z, w)` uniform for one view — the whole per-view scaling law in one +/// pure function, so it can be tested without a render world. +/// +/// - **x** — the zone glow weight, scaled by [`FfxGlow::gain_scale`]: authored *scene* data, so a +/// portrait bake carries it at world parity (decision 0638) and only a UI model pane drops it, +/// keeping the combine for its gamma decode alone. +/// - **y/z** — the FFXDeath gate and the haze mix, both scaled by [`FfxGlow::state_scale`]: these +/// are keyed on the **viewer's own state**, which the reference paints inside the WorldFrame and +/// every UI frame composites over, so neither may reach a bake (decision 1481). +/// - **w** — reserved. +fn combine_uniform(zone_gain: f32, death: f32, haze: f32, glow: &FfxGlow) -> [f32; 4] { + [ + zone_gain * glow.gain_scale, + death * glow.state_scale, + haze * glow.state_scale, + 0.0, + ] +} + #[derive(Default)] struct FfxGlowNode; @@ -377,13 +418,12 @@ impl ViewNode for FfxGlowNode { ) -> Result<(), NodeRunError> { let pipelines = world.resource::(); let pipeline_cache = world.resource::(); - // The zone weight, scaled per view: 1× on the world (and the portrait bakes), 0× on a UI - // model pane, where the combine runs only for its gamma decode ([`FfxGlow::UI_PANE`]). - let gain = world.resource::().0 * glow.gain_scale; - let death = world.resource::().0; - // The haze mix, world-view only ([`FfxGlow::haze_scale`]): a booth bake never inherits - // the drunk/underwater screen blur. - let haze = world.resource::().0 * glow.haze_scale; + let uniform = combine_uniform( + world.resource::().0, + world.resource::().0, + world.resource::().0, + glow, + ); let (Some(downsample), Some(gauss_h), Some(gauss_v), Some(combine)) = ( pipeline_cache.get_render_pipeline(pipelines.downsample), pipeline_cache.get_render_pipeline(pipelines.gauss_h), @@ -396,7 +436,7 @@ impl ViewNode for FfxGlowNode { world.resource::().write_buffer( &textures.gain_buf, 0, - bytemuck::cast_slice(&[gain, death, haze, 0.0]), + bytemuck::cast_slice(&uniform), ); let post = view_target.post_process_write(); let layout_filter = pipeline_cache.get_bind_group_layout(&pipelines.layout_filter); @@ -520,3 +560,58 @@ impl Plugin for FfxGlowPlugin { ); } } + +#[cfg(test)] +mod tests { + use super::*; + + /// The zone glow is scene data — a portrait bake wants it at world parity (decision 0638) — + /// while the death and haze lanes are player state and must stop at the world view + /// (decision 1481). One preset table, checked as a whole so a new preset can't quietly join + /// the wrong side. + #[test] + fn only_the_world_view_carries_the_player_state_lanes() { + for (name, view) in [("BOOTH", FfxGlow::BOOTH), ("UI_PANE", FfxGlow::UI_PANE)] { + assert_eq!(view.state_scale, 0.0, "{name} must not carry player state"); + } + assert_eq!(FfxGlow::WORLD.state_scale, 1.0); + // The glow half is unchanged by that law: a bake still glows like the world. + assert_eq!(FfxGlow::BOOTH.gain_scale, 1.0); + assert_eq!(FfxGlow::UI_PANE.gain_scale, 0.0); + } + + /// B49: a released ghost's portrait baked through the FFXDeath combine and came back steel-blue + /// luma. The gate reaches the world view and nothing else — while the zone glow still does. + #[test] + fn a_ghosts_bake_is_not_death_combined() { + let (zone, ghost, sober) = (0.5, 1.0, 0.0); + assert_eq!( + combine_uniform(zone, ghost, sober, &FfxGlow::WORLD), + [0.5, 1.0, 0.0, 0.0] + ); + assert_eq!( + combine_uniform(zone, ghost, sober, &FfxGlow::BOOTH), + [0.5, 0.0, 0.0, 0.0], + "the booth keeps the zone glow and drops the death gate" + ); + assert_eq!( + combine_uniform(zone, ghost, sober, &FfxGlow::UI_PANE), + [0.0, 0.0, 0.0, 0.0] + ); + } + + /// The control this refactor must not disturb: the drunk/underwater haze was already + /// world-only, and still is. + #[test] + fn a_drunk_players_bake_stays_sharp() { + let (zone, alive, drunk) = (0.5, 0.0, 1.0); + assert_eq!( + combine_uniform(zone, alive, drunk, &FfxGlow::WORLD), + [0.5, 0.0, 1.0, 0.0] + ); + assert_eq!( + combine_uniform(zone, alive, drunk, &FfxGlow::BOOTH), + [0.5, 0.0, 0.0, 0.0] + ); + } +} diff --git a/crates/benilla-world/src/particles/model.rs b/crates/benilla-world/src/particles/model.rs index 497638f6..25cf77ea 100644 --- a/crates/benilla-world/src/particles/model.rs +++ b/crates/benilla-world/src/particles/model.rs @@ -64,6 +64,15 @@ pub(super) fn update_model_particles( let Some(geometry) = emitter.geometry.clone() else { continue; }; + // A GATED emitter's pool is frozen and the sim already edge-hid its instances + // (decision 1480): running this body anyway re-showed them the same frame — + // `Visibility::Inherited` below undid the hide, so a frozen emitter's shards drew + // forever — and kept every instance's Transform/GlobalTransform/MeshTag/material a + // per-frame write. The sim clears `gated` before this system in the same chained set, + // so a thawed pool resumes the frame it wakes. + if emitter.gated { + continue; + } let Some(model) = models.get(&geometry) else { continue; // still loading — particles simulate meanwhile, nothing draws yet }; diff --git a/crates/benilla-world/src/rig_palette.rs b/crates/benilla-world/src/rig_palette.rs index 2ebcfd4d..a802f0a5 100644 --- a/crates/benilla-world/src/rig_palette.rs +++ b/crates/benilla-world/src/rig_palette.rs @@ -184,22 +184,32 @@ pub fn rig_cost_enabled() -> bool { *ON.get_or_init(|| std::env::var_os("WOW_RIG_COST").is_some()) } -/// `Arc::make_mut` on the shared rows, timing the calls that actually deep-copy: between a -/// publish and the frame's first write the extract holds a second reference, so that write -/// clones the whole ~6.3 MB vec. A static helper (not a method) so call sites keep their -/// split-field borrows. +/// Copy-on-write on the shared rows, timing the calls that actually copy: between a publish and +/// the frame's first write the extract holds a second reference, so that write must clone. A +/// static helper (not a method) so call sites keep their split-field borrows. +/// +/// **The clone is bounded by the bone watermark, never the capacity** (decision 1479). +/// `Arc::make_mut` cloned the whole 3×131072-row vec — ~6.3 MB, once per frame, at every spot in +/// the world, however few rigs were awake (1473's fixed-tax finding). A fresh zeroed vec is a +/// calloc — the kernel hands back zero pages untouched — so the real cost is the live-prefix +/// memcpy alone: ~250 KB at a busy pin. Everything at or above the watermark is bitwise zero by +/// invariant (see [`RigPalettes::bone_watermark`]), so the prefix IS the whole content. fn rows_make_mut<'a>( rows: &'a mut Arc>, + watermark_bones: u32, copies: &mut u32, copy_us: &mut f32, ) -> &'a mut Vec<[f32; 4]> { if Arc::strong_count(rows) > 1 || Arc::weak_count(rows) > 0 { let t = std::time::Instant::now(); - let out = Arc::make_mut(rows); + let live = 3 * watermark_bones as usize; + let mut new = vec![[0.0f32; 4]; rows.len()]; + new[..live].copy_from_slice(&rows[..live]); + *rows = Arc::new(new); *copy_us += t.elapsed().as_secs_f32() * 1e6; *copies += 1; - return out; } + // Sole owner here either way: no clone left to happen. Arc::make_mut(rows) } @@ -237,6 +247,21 @@ pub(crate) fn rebase_origin(root_world: Vec3) -> Vec3 { impl RigPalettes { /// Claim a slot + a bone range for a rig. `None` when either slab is exhausted — the caller + /// The bone offset above which every row is bitwise ZERO — the COW clone's bound (1479). + /// + /// The invariant, and why it holds: [`Self::alloc`] is first-fit over a sorted, coalescing + /// free list, so allocations pack low; [`Self::free`] zeroes the rows it returns **before** + /// the range re-enters the free list (its own `rows_make_mut` call still counts the range as + /// live, so the zeroing lands inside the copied prefix); and rows above everything ever + /// allocated are zeros from construction. So a tail free range that touches capacity is all + /// zeros, and its base bounds the content. + fn bone_watermark(&self) -> u32 { + match self.free_ranges.last() { + Some(&(base, len)) if base + len == MAX_PALETTE_BONES as u32 => base, + _ => MAX_PALETTE_BONES as u32, + } + } + /// warns and falls back to the static (bind-pose) mesh, graceful and visible in the log /// rather than wrong. pub(crate) fn alloc(&mut self, bones: u32) -> Option<(u16, u32)> { @@ -287,8 +312,12 @@ impl RigPalettes { self.free_slots.push(slot); self.live_bones -= len; self.table_generation += 1; + // Watermark BEFORE the range re-enters the free list — the zeroing below must land + // inside the copied prefix (the invariant's own ordering, [`Self::bone_watermark`]). + let wm = self.bone_watermark(); let rows = rows_make_mut( &mut self.rows, + wm, &mut self.cost_copies, &mut self.cost_copy_us, ); @@ -353,8 +382,10 @@ impl RigPalettes { let n = (joints.len().min(ibp.len()) as u32).min(rig.len); self.cost_rows += n; self.set_origin(slot, origin); + let wm = self.bone_watermark(); let rows = rows_make_mut( &mut self.rows, + wm, &mut self.cost_copies, &mut self.cost_copy_us, ); @@ -395,8 +426,10 @@ impl RigPalettes { let (slot, base) = (rig.slot, rig.base); self.cost_rows += n; self.set_origin(slot, origin); + let wm = self.bone_watermark(); let rows = rows_make_mut( &mut self.rows, + wm, &mut self.cost_copies, &mut self.cost_copy_us, ); @@ -1005,6 +1038,65 @@ mod tests { assert!(p.dirty.contains(&(base, 2, false))); } + /// The COW clone copies the live prefix and nothing else (decision 1479): a shared-Arc write + /// used to `Arc::make_mut` all 3×131072 rows — ~6.3 MB, once per frame, wherever any rig + /// wrote — where the bone watermark bounds the content. The invariant it leans on is pinned + /// here too: rows at or above the watermark are bitwise zero, through alloc, write, free and + /// coalesce — including free()'s own ordering (watermark before the range re-enters the + /// list, so the zeroing lands inside the copied prefix). + #[test] + fn a_shared_arc_write_copies_only_the_live_prefix() { + let mut p = RigPalettes::default(); + let (_s1, b1) = p.alloc(2).unwrap(); + let (s2, b2) = p.alloc(3).unwrap(); + for r in 3 * b1 as usize..3 * (b1 + 2) as usize { + Arc::make_mut(&mut p.rows)[r] = [1.0; 4]; + } + for r in 3 * b2 as usize..3 * (b2 + 3) as usize { + Arc::make_mut(&mut p.rows)[r] = [2.0; 4]; + } + assert_eq!(p.bone_watermark(), 5, "first-fit packs the content low"); + // The extract's held reference — the shared state every first-write-of-a-frame sees. + let held = p.rows.clone(); + let wm = p.bone_watermark(); + let rows = rows_make_mut(&mut p.rows, wm, &mut p.cost_copies, &mut p.cost_copy_us); + rows[0] = [9.0; 4]; + assert_eq!(p.cost_copies, 1, "the shared write copied"); + assert_eq!( + p.rows[3 * b2 as usize], + [2.0; 4], + "the neighbour survived the prefix copy" + ); + assert_eq!(held[0], [1.0; 4], "the held (extracted) side is untouched"); + // Free the top rig while shared again: the zeroing must land inside the copied prefix. + let held2 = p.rows.clone(); + p.free(s2); + assert_eq!( + p.rows[3 * b2 as usize], + [0.0; 4], + "freed rows zeroed through the COW" + ); + assert_eq!( + held2[3 * b2 as usize], + [2.0; 4], + "the held side keeps the old frame" + ); + assert_eq!( + p.bone_watermark(), + 2, + "the tail grew back down to the live rig" + ); + // A fresh shared write after the shrink still carries the survivor whole. + let _held3 = p.rows.clone(); + let wm = p.bone_watermark(); + let rows = rows_make_mut(&mut p.rows, wm, &mut p.cost_copies, &mut p.cost_copy_us); + rows[1] = [7.0; 4]; + assert_eq!( + p.rows[0], [9.0; 4], + "the live rig's rows survive every bounded copy" + ); + } + #[test] fn alloc_fails_gracefully_at_capacity() { let mut p = RigPalettes::default();