diff --git a/crates/benilla-app/assets/ui/ScreenshotStatus.xml b/crates/benilla-app/assets/ui/ScreenshotStatus.xml new file mode 100644 index 00000000..8ad6b797 --- /dev/null +++ b/crates/benilla-app/assets/ui/ScreenshotStatus.xml @@ -0,0 +1,110 @@ + + + + + + + + + + + + + + + + + + + + + ScreenshotStatus_OnLoad(); + + + ScreenshotStatus_OnEvent(event); + + + ScreenshotStatus_OnUpdate(arg1); + + + + diff --git a/crates/benilla-app/assets/ui/benilla.toc b/crates/benilla-app/assets/ui/benilla.toc index 4eca000e..31895cbb 100644 --- a/crates/benilla-app/assets/ui/benilla.toc +++ b/crates/benilla-app/assets/ui/benilla.toc @@ -132,6 +132,11 @@ MinimapCluster.xml # TOC loads GameTime.xml right after Minimap.xml. After MinimapCluster (its parent) and # GameTooltip (the hover tooltip); driven by the app's game clock (`minimap::feed_game_time`). GameTime.xml +# Print screen's UI half (decision 1487): the centred "Screen Captured"/"Screen Capture Failed" +# line, and the `TakeScreenshot()` the SCREENSHOT binding runs. Needs Fonts.xml (SystemFont) and +# UIParent.xml (its seat) — both far above — and nothing else; seated beside ZoneText.xml, whose +# AutoFollowStatus is the same full-screen-frame-with-a-centred-line shape. +ScreenshotStatus.xml # The zone-entry splash (decision 0287): ZoneTextFrame/SubZoneTextFrame + the inlined # FadingFrame kit, driven by the ZONE_CHANGED family `crate::area` fires. Needs only # Fonts (ZoneTextFont/SubZoneTextFont) + UIParent; kept by the minimap cluster — the diff --git a/crates/benilla-app/src/bindings/chord.rs b/crates/benilla-app/src/bindings/chord.rs index 792e0f4a..a1d21f84 100644 --- a/crates/benilla-app/src/bindings/chord.rs +++ b/crates/benilla-app/src/bindings/chord.rs @@ -205,6 +205,18 @@ pub(crate) fn key_token(k: KeyCode) -> Option<&'static str> { NumpadDecimal => "NUMPADDECIMAL", NumLock => "NUMLOCK", PrintScreen => "PRINTSCREEN", + // **On a Mac keyboard the print-screen key IS F13, and the reference says so itself.** + // 1.12 ships `KEY_PRINTSCREEN_MAC = "F13"` beside a whole family of Mac key-label + // overrides (GlobalStrings.lua:2476-2526 — Backspace→Delete, Insert→Help, NumLock→Clear, + // Pause→F15, ScrollLock→F14), which is the Mac client mapping its physical keys onto the + // PC binding TOKENS and relabelling them for display. macOS has no PrintScreen keycode at + // all — a PC keyboard's PrtSc arrives as F13 — so without this the faithful `PRINTSCREEN` + // default is a dead binding on every Mac, which is not fidelity, it is a broken key. + // `_MAC` is a label table, so this is the mapping the reference implies rather than one it + // was read out of; the falsifier is cheap (bind F13 in the Keybindings page — it must + // write `PRINTSCREEN`, not `F13`). + #[cfg(target_os = "macos")] + F13 => "PRINTSCREEN", // ScrollLock and Pause are deliberately absent: the reference's namer calls their key // codes (`0x210`/`0x211`) `UNKNOWN`, so they are unbindable there (wow-re // `keybinding-dispatch-law.md` §2.3), and `IsValidBindingKeyString` would refuse the @@ -230,6 +242,12 @@ pub(crate) fn key_token(k: KeyCode) -> Option<&'static str> { pub(crate) fn normalize_key(k: KeyCode) -> KeyCode { match k { KeyCode::NumpadEnter => KeyCode::Enter, + // The Mac print-screen key — see [`key_token`]'s arm for the evidence. Folded here as well + // as there because the two arms serve different halves: `key_token` names the key the + // capture arm just swallowed, this one makes a press DISPATCH against a chord parsed from + // the token. Only both together make `PRINTSCREEN` a working binding on a Mac. + #[cfg(target_os = "macos")] + KeyCode::F13 => KeyCode::PrintScreen, other => other, } } @@ -480,6 +498,29 @@ mod tests { assert_eq!(token_key("ENTER"), Some(BindKey::Key(KeyCode::Enter))); } + /// The Mac print-screen fold (decision 1487). macOS delivers no `PrintScreen` at all — a PC + /// keyboard's PrtSc arrives as F13 — so BOTH arms have to agree that F13 *is* `PRINTSCREEN`, + /// or the shipped `PRINTSCREEN SCREENSHOT` default is a key nobody on a Mac can press. + #[cfg(target_os = "macos")] + #[test] + fn f13_is_print_screen_on_a_mac() { + assert_eq!( + key_token(KeyCode::F13), + Some("PRINTSCREEN"), + "the capture arm" + ); + assert_eq!( + normalize_key(KeyCode::F13), + KeyCode::PrintScreen, + "the dispatch arm" + ); + assert_eq!( + token_key("PRINTSCREEN"), + Some(BindKey::Key(normalize_key(KeyCode::F13))), + "a chord parsed from the shipped default matches an F13 press" + ); + } + #[test] fn chords_parse_with_the_112_prefix_order_and_punctuation_bases() { assert_eq!( diff --git a/crates/benilla-app/src/bindings/commands.rs b/crates/benilla-app/src/bindings/commands.rs index e4c28af9..d6283774 100644 --- a/crates/benilla-app/src/bindings/commands.rs +++ b/crates/benilla-app/src/bindings/commands.rs @@ -4,7 +4,7 @@ //! //! **Honest tree**: a command appears here only over a real engine action — the same law as the //! options rows (0954). The 1.12 commands with no benilla mechanism yet (pitch keys, walk toggle, -//! action pages, the right multibars' MULTIACTIONBAR3/4, camera views, screenshot, combat log, …) +//! action pages, the right multibars' MULTIACTIONBAR3/4, camera views, combat log, …) //! are absent, not stubbed; the page shows only what's here, and only non-empty categories (era //! law). Labels/headers are the 1.12 //! GlobalStrings (`BINDING_NAME_*`/`BINDING_HEADER_*`), defined in the window's XML. @@ -98,8 +98,8 @@ pub(crate) mod cmd { // `the_cmd_handles_index_their_rows` is what makes that loud and cheap instead of silent: it // names the handle and the row it actually points at. pub(crate) const TOGGLE_UI: Cmd = Cmd(88); - pub(crate) const CAMERA_ZOOM_IN: Cmd = Cmd(89); - pub(crate) const CAMERA_ZOOM_OUT: Cmd = Cmd(90); + pub(crate) const CAMERA_ZOOM_IN: Cmd = Cmd(90); + pub(crate) const CAMERA_ZOOM_OUT: Cmd = Cmd(91); } /// The registry, 1.12 `Bindings.xml` order. Sub-tables (action buttons, shapeshift, raid @@ -721,6 +721,25 @@ pub(crate) static SPECS: &[Spec] = &[ // descend from one profile whose TOGGLEUI had been rebound; ALT-Z is the shipped default // (the one command whose default does NOT trust the cache file). spec!("TOGGLEUI", MISC, Kind::Host, Some("ALT-Z"), None), + // Print screen (decision 1487). The body is the reference's own one-liner because our + // `TakeScreenshot` has the same contract its does (ScreenshotStatus.xml) — hide the last + // shot's confirmation, then ask the engine. + // + // `PRINTSCREEN` comes from the SHIPPED default, not a player's cache: `DefaultBindings.wtf` + // lives inside `patch.MPQ` and its line 128 is `bind PRINTSCREEN SCREENSHOT` (wow-re's + // dispatch on this feature; the install's account-ONE `bindings-cache.wtf` agrees). `Edge`, + // not `EdgeUpDown`, is also byte-real: the `` carries no `runOnUp`, and the + // reference's dispatcher returns on key-up unless that flag is set (`0x4b7bea`). + // + // On a Mac keyboard the token arrives as F13, which is the reference's own Mac mapping rather + // than an accommodation (`KEY_PRINTSCREEN_MAC = "F13"`); `super::chord` does the translation. + spec!( + "SCREENSHOT", + MISC, + Kind::Edge("TakeScreenshot();"), + Some("PRINTSCREEN"), + None + ), // ── Camera (BINDING_HEADER_CAMERA) ────────────────────────────────────────────────── spec!( "CAMERAZOOMIN", diff --git a/crates/benilla-app/src/capture/probes/act.rs b/crates/benilla-app/src/capture/probes/act.rs index f69b5a35..f7d1a923 100644 --- a/crates/benilla-app/src/capture/probes/act.rs +++ b/crates/benilla-app/src/capture/probes/act.rs @@ -204,6 +204,13 @@ fn probe_key_by_name(name: &str) -> Option { // membership had to be found by the director, twice. `WOW_CHAR` is a deliberate one-shot // (`run_mode`), so synthesizing the keypress is the only way back into the world in-process. "Enter" => KeyCode::Enter, + // Print screen (decision 1487). The whole player path — key → SCREENSHOT binding → + // `TakeScreenshot()` → `Screenshot()` → the writer → SCREENSHOT_SUCCEEDED — is reachable + // no other way: a Lua chunk can call the verb but skips the binding, and B261's contract + // ("the message is not in the file") is a property of the KEY press, since the whole + // reason `TakeScreenshot` hides the status line is a second press landing on a live one. + // On macOS this arrives as F13 (`bindings::chord`'s fold) and the token is the same. + "PrintScreen" => KeyCode::PrintScreen, _ => return None, }) } diff --git a/crates/benilla-app/src/entities/equipment/resolve.rs b/crates/benilla-app/src/entities/equipment/resolve.rs index 25f1c5dc..529c2669 100644 --- a/crates/benilla-app/src/entities/equipment/resolve.rs +++ b/crates/benilla-app/src/entities/equipment/resolve.rs @@ -99,17 +99,39 @@ fn resolve_glow( visual } +/// The resolve inputs that live OUTSIDE the unit's own descriptor and the global caches — the +/// settled sheath byte, the ceremony's per-arm visual state, and the nock lane. Compared each +/// frame by [`resolve_equipment`]'s skip gate (1490): a unit whose descriptor tick, key, and +/// cache epochs all held still resolves to the same loadout **by construction** — every other +/// read in the rebuild is either a field of the store or a static-after-load catalog +/// (`ItemDisplays::catalog`, the enchant rows; the `models` map is write-only from here) — so +/// the rebuild (the template probes, the 7-slot enchant scans, the quiver bag walk, the +/// ensure/glow calls) is skipped whole. The output diff below stays as the last fence: this +/// gate may only ever skip work, never change what lands. +#[derive(Component, Clone, Copy, PartialEq)] +pub(in crate::entities) struct ResolveKey { + committed_sheath: u8, + visual_sheath: Option<[u8; 2]>, + nocked: Option, + latched: bool, +} + /// Resolve every unit's held items from its descriptor. Creatures read display/invType/sheath straight /// from the virtual-item fields; players go visible-item entry → [`crate::items::Items`] (ask-once query on /// a miss). Ensures each needed display id has a [`DisplayModel`] entry in [`ItemDisplays`] (built /// by [`super::update_display_models`] once the asset loads) and writes [`HeldItems`] on change. +/// +/// **Skip-gated per unit** ([`ResolveKey`]): the full rebuild runs only when the unit's own +/// descriptor changed, its key changed, or a global input moved — the [`Items`] epochs (an +/// object ingest covers the quiver bag walk, a landed template answers every pending ask) and +/// the two client-data load edges. The idle crowd costs one tick check and one small compare. #[allow(clippy::type_complexity, clippy::too_many_arguments)] pub(in crate::entities) fn resolve_equipment( mut commands: Commands, units: Query<( Entity, &NetEntity, - &ObjectStore, + Ref, Option<&HeldItems>, Option<&Wielded>, Option<&Equipment>, @@ -117,6 +139,7 @@ pub(in crate::entities) fn resolve_equipment( Option<&crate::creature_anim::AnimDriver>, Option<&NockedAmmo>, Has, + Option<&ResolveKey>, )>, held: Option>, mut templates: ResMut, @@ -130,10 +153,19 @@ pub(in crate::entities) fn resolve_equipment( // The enchant column rides its own resource (decision 0915) — shared with the tooltip lane. glows: Option>, enchants: Option>, + // The [`Items`] epochs as of the last run — the skip gate's global half. Deliberately the + // explicit counters and never resource change ticks: `templates` is `ResMut` in this very + // system (ask-once misses write it), so a tick gate would read its own writes and never + // close. + mut last_epochs: Local>, ) { let Some(mut held) = held else { return; }; + let epochs = (templates.object_epoch(), templates.template_epoch()); + let caches_moved = last_epochs.replace(epochs) != Some(epochs) + || creatures.as_ref().is_some_and(|c| c.is_changed()) + || enchants.as_ref().is_some_and(|e| e.is_changed()); let mut glows = glows; let enchant_rows = enchants.as_deref().map(|e| &e.0); for ( @@ -147,12 +179,32 @@ pub(in crate::entities) fn resolve_equipment( driver, nocked, nock_latched, + current_key, ) in &units { if !matches!(net_entity.kind, EntityKind::Unit | EntityKind::Player) { continue; } let s = &store.0; + // The settled sheath state: the anim layer's **client-side committed state** (the + // setter/reconcile cache, decision 0080 — the descriptor byte plus the policy's forces); + // else, before the driver first runs, the raw descriptor byte. + let committed = driver + .and_then(|d| d.sheath_state()) + .or_else(|| s.unit_sheath_state()) + .unwrap_or(0); + let key = ResolveKey { + committed_sheath: committed, + visual_sheath: visual_sheath.map(|v| v.0), + nocked: nocked.map(|n| n.display_id), + latched: nock_latched, + }; + if !caches_moved && !store.is_changed() && current_key == Some(&key) { + continue; + } + if current_key != Some(&key) { + commands.entity(entity).insert(key); + } // The two **equipment-display preferences** (decision 1472, B123): `PLAYER_FLAGS`' // `HIDE_HELM 0x400` / `HIDE_CLOAK 0x800`, read off THIS unit's own descriptor. The field is // public, so this is per rendered body and not a local setting — a remote player who hides @@ -216,14 +268,8 @@ pub(in crate::entities) fn resolve_equipment( commands.entity(entity).insert(eq); } } - // The settled sheath state: the anim layer's **client-side committed state** (the - // setter/reconcile cache, decision 0080 — the descriptor byte plus the policy's forces); - // else, before the driver first runs, the raw descriptor byte. - let committed = driver - .and_then(|d| d.sheath_state()) - .or_else(|| s.unit_sheath_state()) - .unwrap_or(0); - // …and the *visual* one governing a given slot's placement, which during a draw/stow + // …and the *visual* sheath governing a given slot's placement (`committed` was read + // into the key above), which during a draw/stow // ceremony is **per arm** ([`VisualSheath`]): each hand's weapon moves at its own clip's // authored $SHL/$SHR moment, not at the byte change. A melee → ranged toggle therefore // has the sword already on the back while the bow is still on its way to the other hand — @@ -657,4 +703,66 @@ mod tests { assert_eq!((cloak_only.helm, cloak_only.cloak), (900, 0)); assert!(helm_attached); } + + /// **The skip gate** (1490): a unit whose descriptor tick, [`super::ResolveKey`] and cache + /// epochs held still is not rebuilt at all. Proven the sharp way: a descriptor edit smuggled + /// past change detection — a thing the wire can never do — must NOT land, because the only + /// system that could land it skipped; the same edit under a normal (marked) touch lands on + /// the next pass. The control half is what guards against the gate ever wrongly holding. + #[test] + fn an_unchanged_unit_is_not_rebuilt_and_a_real_change_still_lands() { + let mut app = App::new(); + app.add_plugins((MinimalPlugins, bevy::asset::AssetPlugin::default())); + let mut items = Items::default(); + items.insert_template(100, Some(worn(900, 1))); // INVTYPE_HEAD + items.insert_template(101, Some(worn(901, 1))); // the swap target + let (tx, rx) = crossbeam_channel::unbounded::(); + app.insert_resource(items); + app.insert_resource(NetCommands(tx)); + app.insert_resource(ItemDisplays::icons_for_tests( + benilla_formats::ItemDisplayCatalog::from_displays(std::collections::HashMap::new()), + )); + let player = app + .world_mut() + .spawn(( + NetEntity { + kind: EntityKind::Player, + display_id: Some(49), + scale: 1.0, + }, + wearing(0, &[(0, 100)]), + )) + .id(); + app.add_systems(Update, resolve_equipment); + app.update(); // resolve (caches_moved: first run) + app.update(); // steady state — this frame already skips + assert_eq!(app.world().get::(player).unwrap().helm, 900); + + // The smuggled edit: helm entry 100 → 101 with the change tick left untouched. + app.world_mut() + .get_mut::(player) + .unwrap() + .bypass_change_detection() + .0 + .merge(ObjectFields::from_pairs(&[(258 + 2, 101)])); + app.update(); + assert_eq!( + app.world().get::(player).unwrap().helm, + 900, + "an unmarked store must not be rebuilt — the gate held" + ); + + // The wire's shape: the same store, now MARKED changed — the swap lands. + app.world_mut() + .get_mut::(player) + .unwrap() + .set_changed(); + app.update(); + assert_eq!( + app.world().get::(player).unwrap().helm, + 901, + "a marked store rebuilds — the gate opens" + ); + drop(rx); + } } diff --git a/crates/benilla-app/src/lib.rs b/crates/benilla-app/src/lib.rs index 53f3865e..7b561deb 100644 --- a/crates/benilla-app/src/lib.rs +++ b/crates/benilla-app/src/lib.rs @@ -88,6 +88,7 @@ mod probe_shield; mod quest_markers; mod raid_marks; mod run_mode; +mod screenshot; mod shaders; mod smart_rect; mod sound; @@ -681,7 +682,12 @@ pub fn run(build: BuildId) -> AppExit { // PLAYER_QUEST_LOG descriptor slots + the SMSG_QUEST_QUERY_RESPONSE template cache, and drives // QuestLogFrame.xml over the Era quest-log API. .add_plugins(UiQuestLogPlugin) - .add_plugins(UiChatPlugin); + .add_plugins(UiChatPlugin) + // Print screen (decision 1487): the SCREENSHOT binding's engine half — one PNG per + // `Screenshot()` call into `benilla-config/Screenshots/` (never the install — decision 1486), + // answered to the UI as SCREENSHOT_SUCCEEDED/FAILED so the status text can never be in the + // frame it announces. + .add_plugins(screenshot::ScreenshotPlugin); // Register benilla-assets' loaders AFTER `AssetPlugin` (they go into the live `AssetServer`). benilla_assets::register_asset_loaders(&mut app); diff --git a/crates/benilla-app/src/local_state.rs b/crates/benilla-app/src/local_state.rs index 4a1d153d..db1d33f0 100644 --- a/crates/benilla-app/src/local_state.rs +++ b/crates/benilla-app/src/local_state.rs @@ -219,6 +219,23 @@ pub(crate) fn shots_path() -> Option { home().map(|h| h.join("shots.txt")) } +/// `benilla-config/Screenshots/` — where the print-screen key writes (decisions 1486, 1487). +/// +/// **The reference writes `Screenshots\\` inside the install and we deliberately do not.** benilla +/// reads a WoW install; it never writes to one (decision 1486, the director's rule) — the folder is +/// somebody else's, it is shared with the sibling RE repo on this machine, and a client that +/// scatters its output through it makes "what here is benilla's?" unanswerable. So the reference's +/// own folder NAME is kept, capital S and all, and only its parent moves: a player who knows where +/// WoW put screenshots finds the same folder one level over. +/// +/// Unlike every other resident here this is a DIRECTORY, not a file, and it is not written through +/// [`write_atomic`] — an image is bytes, not a settings diff, and it is written once and never +/// rewritten, so the tmp-then-rename dance buys nothing. It resolves through [`home`] like +/// everything else, which is what makes a capture run hermetic ([`home`] answers `None`). +pub(crate) fn screenshots_dir() -> Option { + home().map(|h| h.join("Screenshots")) +} + /// Make an arbitrary realm/character name safe as one path component: anything outside /// `[A-Za-z0-9_]` becomes `_`, so a realm called `Hydraxian Waterlords` or one with a slash cannot /// escape the folder or collide with the path separator. @@ -335,6 +352,12 @@ mod tests { Some(tmp.join("benilla-config/account")) ); assert_eq!(shots_path(), Some(tmp.join("benilla-config/shots.txt"))); + // The print-screen folder (decisions 1486/1487) — the one resident that is a DIRECTORY, + // and the one whose reference lives inside the install we refuse to write to. + assert_eq!( + screenshots_dir(), + Some(tmp.join("benilla-config/Screenshots")) + ); // 0 · hermetic: a capture run resolves nothing, even with an override set. The account // name is the reason this line now matters more than it did: `config_base` had no such @@ -348,6 +371,11 @@ mod tests { "a capture reads no saved account" ); assert_eq!(shots_path(), None); + assert_eq!( + screenshots_dir(), + None, + "a capture writes no player screenshots" + ); std::fs::remove_dir_all(&tmp).ok(); } diff --git a/crates/benilla-app/src/nameplates.rs b/crates/benilla-app/src/nameplates.rs index 880f50f1..20359d2d 100644 --- a/crates/benilla-app/src/nameplates.rs +++ b/crates/benilla-app/src/nameplates.rs @@ -74,6 +74,12 @@ const SCALE_FLOOR: f32 = 0.2; // [0x80679c] const SCALE_KNEE: f32 = 4.0; // [0x8112a8] const SCALE_RATE: f32 = 1.5; // [0x8112ac] +/// The plate cull's squared comparand — the reference's `[0xc4d988] = 400.0 = 20²` yd², written +/// once from `.rdata 0x80c448 = 20.0`, which no CVar registrar references: **hardcoded**, and +/// not a distance shrink either (plate size never scales with range — verified by exhaustive +/// absence). wow-re `nameplate-vkey.md` §5 / `nameplate-offscreen-cull.md` §2. +const NAMEPLATE_RANGE_SQ: f32 = 400.0; + /// benilla's nameplate config (the client's `0xce8720` cvar mask, reduced to what streams today), /// player-settable since 0992: 1.12's own `UnitNamePlayer`/`UnitNameNPC`/`UnitNameOwn` CVars over /// these gates (the Options window's Nameplates page, through the 0954 store — the arms live in @@ -365,6 +371,9 @@ pub(crate) fn drive_nameplates( Option<&InheritedVisibility>, )>, self_store: Query<&ObjectStore, With>, + // The 20-yard cap's anchor: the reference measures from the LOCAL PLAYER, not the camera + // (`0x60f600`'s 3-D squared distance). + self_tf: Query<&Transform, With>, // The show-gate inputs (one tuple param — Bevy's 16-param ceiling). gates: ( Res, @@ -459,6 +468,23 @@ pub(crate) fn drive_nameplates( if !drawn.is_none_or(|v| v.get()) { continue; } + // **The 20-yard cap** (wow-re `nameplate-offscreen-cull.md` §2, VERIFIED): `0x60f600` + // destroys the frame when the 3-D squared player→unit distance exceeds + // `[0xc4d988] = 400 = 20²` — hardcoded (`.rdata 0x80c448`, no CVar registrar ref), + // measured from the LOCAL PLAYER, never the camera, with no line-of-sight term. Beyond + // it the reference DESTROYS the plate rather than hiding it; skipping here keeps this + // unit out of `seen`, and the retain sweep below IS that destroy. The reference runs + // this gate LAST in its admission ladder (after the projection gate) — every rung is + // verdict-independent, so hoisting it changes no outcome, only how much of the ladder + // the far crowd pays. With no self body (a capture rig without a player) the gate + // stands down — the reference's "no local player ⇒ destroy" rung is the fixtures' call, + // not this one's (named residual, 1490). + if let Ok(anchor) = self_tf.single() { + if !is_self && tf.translation.distance_squared(anchor.translation) > NAMEPLATE_RANGE_SQ + { + continue; + } + } // The ShouldShowName gate, in the client's own order (own-unit answers its cvar BEFORE // the rescue; everyone else: the current-TARGET bypass — `[0xb4e2d8]` is the selection, // not the mouseover — then the kind cvar). A unit carrying a V-key nameplate never also diff --git a/crates/benilla-app/src/net/apply/objects.rs b/crates/benilla-app/src/net/apply/objects.rs index 927e9160..0b77f324 100644 --- a/crates/benilla-app/src/net/apply/objects.rs +++ b/crates/benilla-app/src/net/apply/objects.rs @@ -382,6 +382,7 @@ pub(super) fn unit_move( relay: Default::default(), last_apply_ms: now_ms, last_apply_pos: mv.position, + settled_at: None, }; let fire_ms = rm.relay.schedule(mv.wire_ms, now_ms, 0, true); debug_assert_eq!(fire_ms, now_ms, "a seeding packet fires at arrival"); diff --git a/crates/benilla-app/src/net/motion/remote.rs b/crates/benilla-app/src/net/motion/remote.rs index 285b74e3..3aa89d5e 100644 --- a/crates/benilla-app/src/net/motion/remote.rs +++ b/crates/benilla-app/src/net/motion/remote.rs @@ -73,6 +73,18 @@ pub(crate) struct RemoteMotion { /// not anything the server said. Read by the runaway watch in [`extrapolate_remote_units`]. pub(crate) last_apply_ms: f64, pub(crate) last_apply_pos: [f32; 3], + /// **The settled memo** — the position at which this mover's last world resolve was a + /// fixed point: flag-still, grounded on a real support, and returned bitwise where it + /// started. While the frame's pre-resolve position still equals it, the resolve is skipped + /// whole — a standing NPC was paying a depenetration contact enumeration (heap Vec + broad + /// phase) plus a capsule down-cast every frame for a result proven identical (1473 §3's + /// idle-remote row; the coordinate round-trip is sign/permutation-lossless and + /// `grounded_step` is pure in its inputs, so bitwise equality is the honest test, the + /// `facing.rs` dead-band's shape). Anything that moves the mover — a packet apply, a + /// direction flag, the reconcile lerp, a jump arc — changes the position or flags and + /// misses the memo; a mover standing where ground has not streamed in never arms it + /// (`ground` was `None`) and keeps resolving until the world exists under it. + pub(crate) settled_at: Option<[f32; 3]>, } /// What [`crate::net::apply`]'s `unit_move` did with an inbound relayed move — the `out=` field of @@ -480,7 +492,10 @@ pub(in crate::net) fn extrapolate_remote_units( // fight it). let airborne = rm.flags & move_flags::FALLING != 0; let afloat = rm.flags & move_flags::SWIMMING != 0; - if !afloat && !riding && !flat_extrapolation() { + // The settled memo ([`RemoteMotion::settled_at`]): an idle mover standing exactly where + // its last resolve left it re-derives nothing — the resolve is skipped, `held` honestly + // stays 0. + if !afloat && !riding && rm.settled_at != Some(pos) && !flat_extrapolation() { let half_h = Vec3::Y * (crate::player::CAPSULE_HEIGHT * 0.5); let from = wow_to_bevy(rm.wow_pos) + half_h; // The frame's velocity by construction. Grounded, [`RemoteMotion::advance`] never moves @@ -491,10 +506,19 @@ pub(in crate::net) fn extrapolate_remote_units( } else { Vec3::ZERO }; - let resolved_center = if airborne { - crate::player::mover::airborne_step(&world, &capsule.0, from, vel, time.delta()) + let (resolved_center, supported) = if airborne { + ( + crate::player::mover::airborne_step( + &world, + &capsule.0, + from, + vel, + time.delta(), + ), + false, + ) } else { - crate::player::mover::grounded_step( + let g = crate::player::mover::grounded_step( &world, &capsule.0, from, @@ -509,10 +533,16 @@ pub(in crate::net) fn extrapolate_remote_units( // watched player descending a steep face gets the ordinary cone reach, and the // next packet's wire Z corrects whatever that misses. crate::player::mover::Support::default(), - ) - .center + ); + (g.center, g.ground.is_some()) }; let resolved = bevy_to_wow(resolved_center - half_h); + // Arm the memo only at the fixed point: flag-still, on real ground, and the resolve + // handed back exactly its input (one snap frame after stopping, the second resolve + // does — deterministically, the wow↔bevy round-trip being lossless). A miss on any + // term keeps resolving every frame. + rm.settled_at = (supported && rm.flags & move_flags::ANY_MOVE == 0 && resolved == pos) + .then_some(resolved); held = (pos[0] - resolved[0]).hypot(pos[1] - resolved[1]); pos = resolved; } diff --git a/crates/benilla-app/src/net/motion/tests.rs b/crates/benilla-app/src/net/motion/tests.rs index fe6f8561..2f2c4e60 100644 --- a/crates/benilla-app/src/net/motion/tests.rs +++ b/crates/benilla-app/src/net/motion/tests.rs @@ -62,6 +62,7 @@ fn motion(flags: u32, orientation: f32) -> RemoteMotion { relay: Default::default(), last_apply_ms: 0.0, last_apply_pos: [0.0, 0.0, 0.0], + settled_at: None, } } @@ -882,3 +883,94 @@ fn a_gameobject_without_a_usable_quaternion_falls_back_to_its_facing() { ); } } + +/// **The settled memo** ([`RemoteMotion::settled_at`], 1490 item 2): an idle grounded remote +/// arms it once its resolve reaches the fixed point, holds bitwise still while armed, keeps +/// resolving where no ground exists, and disarms the moment a direction flag moves it — so the +/// per-frame depenetration + down-cast run only for movers that can actually change. +#[test] +fn an_idle_remote_settles_and_stops_sweeping() { + use avian3d::prelude::{Collider, RigidBody}; + use bevy::prelude::*; + use bevy::transform::TransformPlugin; + + let mut app = App::new(); + app.add_plugins(( + MinimalPlugins, + TransformPlugin, + bevy::asset::AssetPlugin::default(), + bevy::scene::ScenePlugin, + avian3d::prelude::PhysicsPlugins::new(bevy::app::PostUpdate), + )); + app.init_asset::() + .init_resource::(); + app.finish(); + app.cleanup(); + app.insert_resource(crate::player::PlayerCapsule(Collider::capsule( + crate::player::CAPSULE_RADIUS, + crate::player::CAPSULE_HEIGHT - 2.0 * crate::player::CAPSULE_RADIUS, + ))); + app.add_systems(Update, super::remote::extrapolate_remote_units); + // A 10×10 up-wound floor at bevy y = 0 (wow z = 0). + let verts = vec![ + Vec3::new(-5.0, 0.0, -5.0), + Vec3::new(5.0, 0.0, -5.0), + Vec3::new(5.0, 0.0, 5.0), + Vec3::new(-5.0, 0.0, 5.0), + ]; + app.world_mut().spawn(( + RigidBody::Static, + Collider::trimesh(verts, vec![[0u32, 2, 1], [0, 3, 2]]), + Transform::default(), + )); + let idle = app + .world_mut() + .spawn(( + Transform::default(), + motion(0, 0.0), + // Real speeds, so the FORWARD leg below actually translates — a flag with no speed + // moves nothing and would (correctly) stay settled. + crate::net::UnitSpeeds(speeds()), + )) + .id(); + let mut over_nothing = motion(0, 0.0); + over_nothing.wow_pos = [50.0, 50.0, 10.0]; + let stranded = app + .world_mut() + .spawn((Transform::default(), over_nothing)) + .id(); + // Seed the idle mover a hair above the floor — inside the standing snap reach (at zero + // speed the whole reach IS `STEP_SNAP_SLACK` ≈ 0.028), like a wire Z, which always arrives + // on the ground. The first resolve snaps it on, the second arms the memo. + app.world_mut() + .entity_mut(idle) + .get_mut::() + .unwrap() + .wow_pos = [0.0, 0.0, 0.01]; + + for _ in 0..4 { + app.update(); + } + let rm = app.world().entity(idle).get::().unwrap(); + let armed_at = rm.settled_at.expect("an idle grounded mover arms the memo"); + assert_eq!(armed_at, rm.wow_pos, "the memo is the pose it stands at"); + let parked = rm.wow_pos; + for _ in 0..3 { + app.update(); + } + let rm = app.world().entity(idle).get::().unwrap(); + assert_eq!(rm.wow_pos, parked, "armed ⇒ bitwise still"); + assert_eq!(rm.settled_at, Some(parked), "…and still armed"); + // No ground in reach ⇒ never arms; it keeps asking until the world exists under it. + let rm = app.world().entity(stranded).get::().unwrap(); + assert_eq!(rm.settled_at, None, "no support ⇒ no memo"); + // A direction flag moves the mover and the memo falls with it. + app.world_mut() + .entity_mut(idle) + .get_mut::() + .unwrap() + .flags = move_flags::FORWARD; + app.update(); + let rm = app.world().entity(idle).get::().unwrap(); + assert_eq!(rm.settled_at, None, "a moving mover resolves every frame"); +} diff --git a/crates/benilla-app/src/player.rs b/crates/benilla-app/src/player.rs index ebd751fd..adad4aa3 100644 --- a/crates/benilla-app/src/player.rs +++ b/crates/benilla-app/src/player.rs @@ -79,18 +79,19 @@ pub(crate) use follow::{FollowRequest, FollowState}; // The shared avatar state + movement constants live in [`state`]; the private re-imports below are // what lets this module and the concern modules beside it keep naming them `super::X` unchanged. use state::{ - MoveSpeed, PlayerRide, AIR_NUDGE_SPEED, CAPSULE_RADIUS, FALL_FAR_DROP, FALL_FAR_TIME, - FOOT_CONE_HEIGHT, GROUND_COS, GROUND_PROBE, JUMP_SPEED, LAND_PROBE, MOUSELOOK_PITCH_CLAMP, - RUN_BACK_RATIO, SKIN_WIDTH, STATIONARY_CHASE_RATE, STEP_SLOPE_RATIO, STEP_SNAP_SLACK, - STEP_UP_ADVANCE, STEP_UP_HEIGHT, TURN_RATE, TURN_RATE_MOVING, WEDGE_MIN_FALL, - WEDGE_STALL_RATIO, WEDGE_STILL_FRAMES, + MoveSpeed, PlayerRide, AIR_NUDGE_SPEED, FALL_FAR_DROP, FALL_FAR_TIME, FOOT_CONE_HEIGHT, + GROUND_COS, GROUND_PROBE, JUMP_SPEED, LAND_PROBE, MOUSELOOK_PITCH_CLAMP, RUN_BACK_RATIO, + SKIN_WIDTH, STATIONARY_CHASE_RATE, STEP_SLOPE_RATIO, STEP_SNAP_SLACK, STEP_UP_ADVANCE, + STEP_UP_HEIGHT, TURN_RATE, TURN_RATE_MOVING, WEDGE_MIN_FALL, WEDGE_STALL_RATIO, + WEDGE_STILL_FRAMES, }; // `SETTLE_TIMEOUT` is `pub(crate)`: the settle release lives in the terrain streamer (decision // 0737 — residency releases the hold, not ground contact), which owns the deadline push while the // resident world is still the departed map's (0710). pub(crate) use state::{ - Player, PlayerCapsule, CAPSULE_HEIGHT, DEFAULT_COLLISION_HEIGHT, FEATHER_TERMINAL_VELOCITY, - GRAVITY, HOVER_CLIMB_RATE, HOVER_HEIGHT, SETTLE_TIMEOUT, TERMINAL_VELOCITY, + Player, PlayerCapsule, CAPSULE_HEIGHT, CAPSULE_RADIUS, DEFAULT_COLLISION_HEIGHT, + FEATHER_TERMINAL_VELOCITY, GRAVITY, HOVER_CLIMB_RATE, HOVER_HEIGHT, SETTLE_TIMEOUT, + TERMINAL_VELOCITY, }; /// The swim boundary `0.75·h` — and therefore the **wade ceiling**, since wading is the implicit /// in-liquid-but-not-swimming state and the two cannot be different numbers. Read by the creature diff --git a/crates/benilla-app/src/player/embody.rs b/crates/benilla-app/src/player/embody.rs index 6364029b..75358093 100644 --- a/crates/benilla-app/src/player/embody.rs +++ b/crates/benilla-app/src/player/embody.rs @@ -180,6 +180,7 @@ mod tests { relay: Default::default(), last_apply_ms: 0.0, last_apply_pos: [0.0; 3], + settled_at: None, } } diff --git a/crates/benilla-app/src/player/state.rs b/crates/benilla-app/src/player/state.rs index c47176f6..51ea32de 100644 --- a/crates/benilla-app/src/player/state.rs +++ b/crates/benilla-app/src/player/state.rs @@ -64,7 +64,7 @@ pub(super) const STATIONARY_CHASE_RATE: f32 = 8.0; /// radius **0.30555**, human female **0.20835** (VERIFIED, wow-re `mover-collision-scalars.md`; /// decision 1125). Adopting them is the same movement-fidelity question [`CAPSULE_HEIGHT`] describes /// below, and this radius is its other half. -pub(super) const CAPSULE_RADIUS: f32 = 1.0 / 3.0; +pub(crate) const CAPSULE_RADIUS: f32 = 1.0 / 3.0; /// Player capsule total height (yd) — the **movement** capsule avian sweeps, deliberately a /// constant. Numerically the vanilla ctor-default collision height it was derived from /// ([`DEFAULT_COLLISION_HEIGHT`]), but it is not the same quantity and no longer stands in for one: diff --git a/crates/benilla-app/src/screenshot.rs b/crates/benilla-app/src/screenshot.rs new file mode 100644 index 00000000..d5a2cabc --- /dev/null +++ b/crates/benilla-app/src/screenshot.rs @@ -0,0 +1,350 @@ +//! **Print screen** (decision 1487) — the capture key, the writer, and the "Screen Captured" text +//! that must never appear in the file it announces. +//! +//! The reference's shape, which this reproduces exactly (its own `Bindings.xml` + `WorldFrame.lua`, +//! read off the 1.12.1 install): +//! +//! ```text +//! PRINTSCREEN → binding SCREENSHOT → TakeScreenshot() [FrameXML] +//! ScreenshotStatus:Hide() ← last frame's text, gone first +//! Screenshot() [engine] +//! … the frame is captured … +//! engine → SCREENSHOT_SUCCEEDED / SCREENSHOT_FAILED → ScreenshotStatus shows, fades over 1.5 s +//! ``` +//! +//! **That ordering is most of the answer to B261's third clause, and the rest is ours to arrange.** +//! For a single press nothing can leak: `Screenshot()` returns nothing and the outcome comes back +//! as an *event*, frames later, because the readback is asynchronous — the confirmation does not +//! exist yet when the shutter fires. The leak is the SECOND press inside the 1.5 s fade, when the +//! previous shot's "Screen Captured" is still on screen; that is what the reference's `Hide()` +//! before the capture is for. +//! +//! **And on our pipeline that `Hide()` lands one frame too late** — measured, not reasoned: +//! a live double press wrote the text straight into the second PNG. `ui_script`'s `drive_script` +//! ticks the VM and builds this frame's UI quad list at the TOP of the `UiInput` set; the binding +//! dispatch that runs `TakeScreenshot()` sits at the BOTTOM of that same set. So by the time the +//! binding hides the frame, the quads carrying its text are already built, and the frame that then +//! renders — and gets captured — still has the line in it. Nothing in the reference's design says +//! otherwise; the real client simply does not build its draw list in that order. +//! +//! So [`ask_for_captures`] **holds each ask for one frame** ([`ScreenshotState::pending`]). The +//! capture is requested on the frame AFTER the press, whose `drive_script` has rebuilt the quads +//! with the frame hidden. One frame is the whole fix and also the minimum: any less and the stale +//! quads are what gets photographed, any more and the shutter drifts from the keypress for no +//! reason. `ui_script::screenshot_tests` pins the UI contract; this file's own test pins the +//! deferral, and the live falsifier is a double press 0.6 s apart (decision 1487). +//! +//! **Two deliberate divergences, both recorded in 1487:** +//! +//! 1 · **PNG, where the reference writes TGA.** An uncompressed 32-bit Targa of a modern window is +//! ~8 MB and no ordinary tool on a 2026 desktop previews one. This is the "mandatory unfaithful +//! change" the report came in with; the mechanism, the folder and the naming are otherwise the +//! reference's. +//! 2 · **`benilla-config/Screenshots/`, not `/Screenshots/`** — decision 1486's rule: +//! benilla reads a WoW install and never writes to one. Same folder name, different parent. +//! +//! **One reference behaviour deliberately NOT transcribed.** The real client fires +//! `SCREENSHOT_SUCCEEDED` whether or not the file was written — it discards the writer's return +//! value, so the event means "the device gave us a buffer" — and it has already deleted the +//! partial file. That is a bug, not a contract: [`report_captures`] answers `SCREENSHOT_FAILED` +//! when the encode or the write actually failed, and puts the reason in the log. +//! +//! **Two gaps that ARE faithful**, named so they are not mistaken for oversights: a raw +//! `Screenshot()` from a macro or addon skips `TakeScreenshot`'s `Hide()` and so can photograph a +//! fading confirmation (the reference's hide is in Lua too, and inventing an engine-side one would +//! be inventing behaviour); and the key does nothing at the glue screens, where the reference +//! captures through hard-coded, unrebindable `OnKeyDown` handlers that print nothing — a separate +//! slice, in the glue arc. +//! +//! Nothing here is dev-gated: this is a player feature, and it links in the `--no-default-features` +//! player build like the rest of the UI arc. + +use bevy::prelude::*; +use bevy::render::view::screenshot::{Screenshot, ScreenshotCaptured}; +use bevy::tasks::IoTaskPool; + +use benilla_ui::script::UiScript; + +use crate::ui_script::UiInput; + +/// The reference's own file stem (`WoWScrnShot_`), kept: a player who knows what WoW screenshots +/// are called finds the same names here, and so does every screenshot-organizing tool the +/// community has. Only the extension changes. +const STEM: &str = "WoWScrnShot"; + +/// What the writer thread reported back. The outcome is a two-state answer because the UI's is: +/// `SCREENSHOT_SUCCEEDED` or `SCREENSHOT_FAILED`, nothing in between. +enum Outcome { + Saved(std::path::PathBuf), + Failed(String), +} + +/// The capture arc's state: the channel the writer threads report through, and just enough clock +/// memory to keep two captures in the same second from landing on the same name. +#[derive(Resource)] +struct ScreenshotState { + tx: crossbeam_channel::Sender, + rx: crossbeam_channel::Receiver, + /// The epoch second the last name was minted for, and how many have been minted for it. + /// **Held here rather than probed off the filesystem**: the write happens on a worker thread + /// and may not have created the file by the time the next name is needed, so "does it exist + /// yet?" is a race that silently overwrites. A counter on the main thread cannot lose. + last_second: i64, + within_second: u32, + /// Captures asked for on the PREVIOUS frame, waiting for this one to render. The one-frame + /// hold that keeps the status line out of the picture — see the module docs for the measured + /// reason it is needed at all. + pending: u32, +} + +impl Default for ScreenshotState { + fn default() -> Self { + let (tx, rx) = crossbeam_channel::unbounded(); + Self { + tx, + rx, + last_second: i64::MIN, + within_second: 0, + pending: 0, + } + } +} + +impl ScreenshotState { + /// The next file name, from the wall clock: `WoWScrnShot_MMDDYY_HHMMSS.png`, the reference's + /// own field order and zero padding. + /// + /// **A second capture inside the same second gets `_2`, `_3`, … rather than overwriting.** + /// That is a divergence and a deliberate one: the reference's fixed-name write means a burst + /// of key presses leaves one file, which is a silent data loss the player only discovers + /// later. The suffix is outside the reference's grammar, so nothing that parses the faithful + /// name is broken by a name it will simply not match. + /// + /// **UTC, not local time** — the reference stamps local. The workspace carries no date + /// dependency and a local offset needs a timezone source, not different arithmetic; the Lua + /// `date()` global records the same divergence for the same reason + /// ([`benilla_ui::civil`]). Names stay unique and sort correctly either way. + fn next_name(&mut self, now: i64) -> String { + if now == self.last_second { + self.within_second += 1; + } else { + self.last_second = now; + self.within_second = 0; + } + let c = benilla_ui::civil::from_unix(now); + let base = format!( + "{STEM}_{:02}{:02}{:02}_{:02}{:02}{:02}", + c.month, + c.day, + c.year.rem_euclid(100), + c.hour, + c.min, + c.sec + ); + match self.within_second { + 0 => format!("{base}.png"), + n => format!("{base}_{}.png", n + 1), + } + } +} + +/// Ask the renderer for one frame per queued `Screenshot()` call — **one frame after the call**. +/// +/// The order within a frame is: spawn last frame's asks first, then drain this frame's. Both halves +/// matter. Spawning first means the entity is in the world before the render extract at the end of +/// THIS frame, whose UI quads `drive_script` rebuilt with the status line already hidden. Draining +/// second means an ask made by the binding dispatch earlier in this same frame waits its turn +/// rather than being photographed against the quads that were built before the hide — which is the +/// bug this deferral exists for (module docs). +/// +/// Ordered `.after(BindingSet)` so the drain deterministically sees an ask the binding just made, +/// instead of depending on where an unconstrained system happened to land. +fn ask_for_captures( + mut commands: Commands, + script: Option>, + mut state: ResMut, +) { + for _ in 0..std::mem::take(&mut state.pending) { + commands + .spawn(Screenshot::primary_window()) + .observe(write_capture); + } + if let Some(mut script) = script { + state.pending += script.take_screenshot_asks(); + } +} + +/// The readback landed: name the file and hand the encode+write to a worker. +/// +/// **Off the main thread on purpose.** A 2560×1440 PNG encode is tens of milliseconds and this +/// runs inside the frame; doing it here would drop frames every time the player takes a shot, +/// which is the one moment they are looking at the picture. The answer comes back through the +/// channel and is reported on whichever later frame it arrives — the UI already expects that, +/// because the reference's outcome is an event too. +fn write_capture(captured: On, mut state: ResMut) { + // Nothing to write to: a hermetic capture/probe run (`$WOW_CAPTURE`), or a platform with no + // discoverable exe directory. Report the failure rather than dropping it silently — the UI + // says "Screen Capture Failed" and the player learns something. + let Some(dir) = crate::local_state::screenshots_dir() else { + let _ = state.tx.send(Outcome::Failed( + "no benilla-config folder to write into".into(), + )); + return; + }; + let path = dir.join(state.next_name(benilla_ui::civil::unix_seconds())); + + // `to_rgb8` drops alpha, which on an HDR target carries brightness rather than opacity — the + // same choice bevy's own `save_to_disk` makes, and for the same reason: kept, the picture is + // wrong. + let dynamic = match captured.image.clone().try_into_dynamic() { + Ok(img) => img.to_rgb8(), + Err(e) => { + let _ = state + .tx + .send(Outcome::Failed(format!("unreadable frame: {e}"))); + return; + } + }; + + let tx = state.tx.clone(); + IoTaskPool::get() + .spawn(async move { + let outcome = std::fs::create_dir_all(&dir) + .and_then(|()| { + dynamic + .save_with_format(&path, image::ImageFormat::Png) + .map_err(std::io::Error::other) + }) + .map(|()| Outcome::Saved(path.clone())) + .unwrap_or_else(|e| Outcome::Failed(format!("{}: {e}", path.display()))); + let _ = tx.send(outcome); + }) + .detach(); +} + +/// Report finished writes to the UI — the reference's `SCREENSHOT_SUCCEEDED` / `SCREENSHOT_FAILED`. +/// +/// **Before [`UiInput`]**, the shape every other feed in this crate uses: the event is delivered at +/// the top of a frame so the status text is laid out with everything else, and — the part that +/// matters — this can only ever be a frame *after* the one that was captured, so the text it puts +/// on screen cannot be in the picture. +fn report_captures(script: Option>, state: Res) { + let Some(mut script) = script else { + return; + }; + while let Ok(outcome) = state.rx.try_recv() { + match outcome { + Outcome::Saved(path) => { + // Announced at info: "where did my screenshot go" is the first question a player + // asks about a folder that is deliberately not where WoW put it (decision 1486). + info!("screenshot: wrote {}", path.display()); + script.fire_event("SCREENSHOT_SUCCEEDED", Vec::new()); + } + Outcome::Failed(why) => { + warn!("screenshot: FAILED — {why}"); + script.fire_event("SCREENSHOT_FAILED", Vec::new()); + } + } + } +} + +pub(crate) struct ScreenshotPlugin; + +impl Plugin for ScreenshotPlugin { + fn build(&self, app: &mut App) { + app.init_resource::().add_systems( + Update, + ( + report_captures.before(UiInput), + ask_for_captures + .after(UiInput) + .after(crate::bindings::BindingSet), + ), + ); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// The reference's field order and padding, and the anti-clobber suffix — the two halves of + /// [`ScreenshotState::next_name`] that a reader would otherwise have to take on trust. + #[test] + fn names_follow_the_reference_and_never_collide() { + let mut s = ScreenshotState::default(); + // 2026-08-21T13:45:07Z — single digits in the day and the hour would be the padding bug. + assert_eq!(s.next_name(1_787_319_907), "WoWScrnShot_082126_134507.png"); + // Same second again: a suffix, not an overwrite. The reference would have written the + // same name twice and left one file. + assert_eq!( + s.next_name(1_787_319_907), + "WoWScrnShot_082126_134507_2.png" + ); + assert_eq!( + s.next_name(1_787_319_907), + "WoWScrnShot_082126_134507_3.png" + ); + // A new second resets the counter — the plain name is the overwhelmingly common one. + assert_eq!(s.next_name(1_787_319_908), "WoWScrnShot_082126_134508.png"); + // 2001-01-02T03:04:05Z: every field at its narrowest, and a year whose last two digits + // need the leading zero the reference's `%02d` gives them. + assert_eq!(s.next_name(978_404_645), "WoWScrnShot_010201_030405.png"); + } + + /// **The one-frame hold, as a test** — the fix for the leak a live double press actually + /// produced (module docs). An ask made during frame N must not be spawned until frame N+1, + /// because frame N's UI quads were built before the binding hid the status line. + /// + /// Driven against the real system in a minimal app rather than by calling a helper: the claim + /// is about *scheduling*, and a helper that runs both halves in one call would pass whether or + /// not the deferral survives a future reorder. + #[test] + fn an_ask_is_held_for_one_frame_before_the_shutter() { + let mut app = App::new(); + app.init_resource::(); + // The observer is irrelevant here — what is under test is when the entity appears — so the + // system runs with no VM and the asks are pushed by hand. + app.add_systems(Update, spawn_pending_only); + + // Frame N: the binding asked. Nothing may be spawned yet. + app.world_mut().resource_mut::().pending = 0; + app.world_mut().resource_mut::().pending += 1; + let staged = app.world().resource::().pending; + assert_eq!(staged, 1, "the ask is staged, not yet a capture"); + + // Frame N+1: now it fires, against quads rebuilt with the line hidden. + app.update(); + assert_eq!( + app.world().resource::().pending, + 0, + "the held ask is consumed on the next frame" + ); + assert_eq!( + app.world_mut() + .query::<&Screenshot>() + .iter(app.world()) + .count(), + 1, + "and exactly one capture is requested" + ); + + // A quiet frame asks for nothing. + app.update(); + assert_eq!( + app.world_mut() + .query::<&Screenshot>() + .iter(app.world()) + .count(), + 1 + ); + } + + /// [`ask_for_captures`]'s spawn half alone — the drain half needs a live VM, which a bare + /// `App` has no business building. Keeping the two in one system in production and splitting + /// only here would be the usual lie, so this calls the real thing with the VM absent, which is + /// exactly how it behaves at the character-select screen. + fn spawn_pending_only(commands: Commands, state: ResMut) { + ask_for_captures(commands, None, state); + } +} diff --git a/crates/benilla-app/src/target/hover.rs b/crates/benilla-app/src/target/hover.rs index ed866803..fb448ae6 100644 --- a/crates/benilla-app/src/target/hover.rs +++ b/crates/benilla-app/src/target/hover.rs @@ -189,8 +189,12 @@ pub(super) fn update_hover( continue; } let Some(children) = children else { continue }; - let skinned: Vec<_> = children.iter().filter_map(|c| parts.get(c).ok()).collect(); - if skinned.is_empty() { + // Ownership probe only — no collect yet. The `skinned` Vec used to be built HERE, for + // every drawn unit in the scene, ahead of the few-FLOP broad phase that rejects nearly + // all of them (1473 §3's hover row); the survivors alone pay it now, below. Membership + // in `faithful` must still be decided pre-reject — a broad-phase-rejected skinned unit + // stays excluded from the AABB fallback by RULE, not by accident. + if !children.iter().any(|c| parts.contains(c)) { continue; // static/cube unit → the AABB fallback below } faithful.insert(entity); @@ -215,6 +219,7 @@ pub(super) fn update_hover( continue; } } + let skinned: Vec<_> = children.iter().filter_map(|c| parts.get(c).ok()).collect(); // The posed joint palette (parts of one skeleton share the rig): the same // world-from-bind-pose matrices GPU skinning applies, read from the owned palette rows // (decision 0720 — last frame's propagated pose, exactly what the previous @@ -346,6 +351,20 @@ pub(super) fn update_hover( /// ties by distance. Both passes stay world-occlusion-clamped. (Residual, same slice as the /// eligibility note below: the reference's sticky slot is cross-type, so a stuck GO pick can /// outrank a unit's halo there; our split picks give the unit that frame instead.) +/// The GO picker's own state, bundled — the pick-set cache with the stream edges that rebuild +/// it, plus the sticky-hover memory — one [`SystemParam`] because [`update_hovered_object`] +/// sits at Bevy's 16-param function-system ceiling. See the maintenance comment at the top of +/// that system for the cache contract. +#[derive(bevy::ecs::system::SystemParam)] +pub(super) struct GoPickSet<'w, 's> { + added: Query<'w, 's, (), Added>, + removed: RemovedComponents<'w, 's, GoPickPart>, + cache: Local<'s, bevy::platform::collections::HashSet>, + /// Last frame's GO pick, for pass 2's sticky-hover (the reference's anti-flicker cache, by + /// net entity — the same rung the unit picker keeps). + last_pick: Local<'s, Option>, +} + #[allow(clippy::too_many_arguments)] pub(super) fn update_hovered_object( camera: Query<(&Camera, &GlobalTransform), With>, @@ -370,10 +389,39 @@ pub(super) fn update_hovered_object( factions: Option>, self_q: Query<&ObjectStore, With>, parts: PickParts, - // Last frame's GO pick, for pass 2's sticky-hover (the reference's anti-flicker cache, by net - // entity — the same rung the unit picker keeps). - mut last_pick: Local>, + // The picker's own state, one bundled param ([`GoPickSet`] — the fn sits at Bevy's + // 16-param ceiling): the pick-set cache, its stream edges, and the sticky-hover memory. + mut cache: GoPickSet, ) { + let added_parts = &cache.added; + let removed_parts = &mut cache.removed; + let pickable = &mut *cache.cache; + let last_pick = &mut *cache.last_pick; + // The pick set (bevy's `HashSet` — the caster's type, not the `std` one this module uses + // elsewhere), rebuilt on GO-part stream edges ONLY — the `WaterIndex` shape. It used to be + // collected from every streamed GO part on every cursor frame (ChildOf hop + store probe + + // hash insert each); membership only actually changes when GO models spawn or despawn, and + // a GO's type id never changes after create. Maintained FIRST, ahead of every early return: + // an `Added` edge is only visible on the frame it happens (the run advances this system's + // change tick even through a `return`), so a rebuild parked behind the cursor gates would + // silently drop parts streamed in while the pointer sat on UI. A despawned entry between + // edges self-filters at the caster (`parts.get` on a dead entity misses). + // + // A transport-family GO (TRANSPORT 11 / MAP_OBJECT 14 / MO_TRANSPORT 15) never joins the + // set: in the reference it has no pick geometry at all — the GO model resolver `0x5f80e0` + // returns 0 for these three types (they render via the WMO/spline path, not an M2) and their + // strategy's interaction predicates are constant-false (w2c1 + go-render-gate + the +0x14 + // vtable dump). Concretely: a ship's hull must not swallow the pick — an NPC on deck stays + // hoverable through the railing, and the hull shows no gear or tooltip. + if removed_parts.read().next().is_some() || !added_parts.is_empty() { + pickable.clear(); + pickable.extend(go_parts.iter().filter(|&e| { + let root = child_of.get(e).map_or(e, |c| c.parent()); + !stores + .get(root) + .is_ok_and(|s| matches!(s.0.gameobject_type_id(), 11 | 14 | 15)) + })); + } hovered.target = None; hovered.guid = None; hovered.distance = f32::MAX; @@ -387,22 +435,6 @@ pub(super) fn update_hovered_object( let Some(cursor) = window.cursor_position() else { return; }; - // The caster takes bevy's `HashSet` (not the `std` one this module uses elsewhere). - // A transport-family GO (TRANSPORT 11 / MAP_OBJECT 14 / MO_TRANSPORT 15) never joins the pick - // set: in the reference it has no pick geometry at all — the GO model resolver `0x5f80e0` - // returns 0 for these three types (they render via the WMO/spline path, not an M2) and their - // strategy's interaction predicates are constant-false (w2c1 + go-render-gate + the +0x14 - // vtable dump). Concretely: a ship's hull must not swallow the pick — an NPC on deck stays - // hoverable through the railing, and the hull shows no gear or tooltip. - let pickable: bevy::platform::collections::HashSet = go_parts - .iter() - .filter(|&e| { - let root = child_of.get(e).map_or(e, |c| c.parent()); - !stores - .get(root) - .is_ok_and(|s| matches!(s.0.gameobject_type_id(), 11 | 14 | 15)) - }) - .collect(); if pickable.is_empty() { *last_pick = None; return; @@ -424,7 +456,7 @@ pub(super) fn update_hovered_object( // Pass 1 — the exact resident geometry, pure nearest-wins (priority-independent). let mut best: Option<(f32, Entity)> = - benilla_world::interact::cast_pick_ray(ray, &pickable, &parts, false) + benilla_world::interact::cast_pick_ray(ray, pickable, &parts, false) .into_iter() .next() .map(|(e, h)| (h.distance, e)); @@ -437,7 +469,7 @@ pub(super) fn update_hovered_object( return; } let mut best2: Option<(f32, u32, Entity)> = None; - for (part, hit) in benilla_world::interact::cast_pick_ray_inflated(ray, &pickable, &parts) { + for (part, hit) in benilla_world::interact::cast_pick_ray_inflated(ray, pickable, &parts) { let net = resolve_net(part); let prio = if *last_pick == Some(net) { u32::MAX // the sticky-hover cache outranks everything diff --git a/crates/benilla-app/src/ui_script/mod.rs b/crates/benilla-app/src/ui_script/mod.rs index 1cbec852..47d8103e 100644 --- a/crates/benilla-app/src/ui_script/mod.rs +++ b/crates/benilla-app/src/ui_script/mod.rs @@ -805,6 +805,9 @@ mod quest_timer_tests; #[cfg(test)] mod durability_tests; +#[cfg(test)] +mod screenshot_tests; + #[cfg(test)] mod questlog_tests; diff --git a/crates/benilla-app/src/ui_script/screenshot_tests.rs b/crates/benilla-app/src/ui_script/screenshot_tests.rs new file mode 100644 index 00000000..51e45bab --- /dev/null +++ b/crates/benilla-app/src/ui_script/screenshot_tests.rs @@ -0,0 +1,128 @@ +//! Print screen's UI half (`assets/ui/ScreenshotStatus.xml`) against the shipped XML — and above +//! all **B261's third clause: the "Screen Captured" line must not be in the file it announces.** +//! +//! That contract is an ORDERING, so it is tested as one. Two paths could put text in a picture and +//! both are pinned here: the ordinary press (nothing is shown until the engine reports back, which +//! it can only do after the readback) and the double press inside the 1.5 s fade (the previous +//! shot's line is still up, and `TakeScreenshot` takes it down *before* asking). + +use benilla_ui::script::UiScript; + +fn load_xml(s: &UiScript, file: &str) { + let text = std::fs::read_to_string( + std::path::Path::new(env!("CARGO_MANIFEST_DIR")) + .join("assets/ui") + .join(file), + ) + .unwrap(); + let doc = benilla_ui::framexml::parse(&text).unwrap(); + let report = benilla_ui::loader::load(s, &doc, &|_| None); + assert!( + report.errors.is_empty(), + "{file}: loader errors: {:?}", + report.errors + ); +} + +fn harness() -> UiScript { + let mut s = UiScript::new().unwrap(); + s.set_screen_size(1024.0, 768.0); + for f in [ + "Fonts.xml", + "UiPanels.xml", + "UIParent.xml", + "ScreenshotStatus.xml", + ] { + load_xml(&s, f); + } + s +} + +fn shown(s: &UiScript) -> bool { + s.eval::("return ScreenshotStatus:IsVisible()") + .unwrap() +} + +fn text(s: &UiScript) -> String { + s.eval::("return ScreenshotStatusText:GetText() or \"\"") + .unwrap() +} + +fn alpha(s: &UiScript) -> f64 { + s.eval::("return ScreenshotStatus:GetAlpha()").unwrap() +} + +/// **The bug, as a test.** A press asks the engine and shows NOTHING; the line appears only when +/// the engine reports back, which in the real client is frames later — so the frame that was +/// captured cannot contain it. +#[test] +fn the_capture_is_asked_for_silently_and_only_the_answer_speaks() { + let mut s = harness(); + assert!(!shown(&s), "the status line starts hidden"); + + s.run("TakeScreenshot()").unwrap(); + assert_eq!( + s.take_screenshot_asks(), + 1, + "the binding body reaches the engine verb" + ); + assert!( + !shown(&s), + "NOTHING is on screen at the moment of capture — this is B261's whole contract" + ); + + // The engine's answer, one or more frames later. + s.fire_event("SCREENSHOT_SUCCEEDED", Vec::new()); + assert!(shown(&s)); + assert_eq!(text(&s), "Screen Captured"); + assert_eq!(alpha(&s), 1.0); +} + +/// **The second leak path, and the reason `TakeScreenshot` hides rather than waiting for the +/// fade.** Press again while the last confirmation is still on screen and it comes off BEFORE the +/// engine is asked — otherwise that shot would have "Screen Captured" printed across it. +#[test] +fn a_second_press_inside_the_fade_clears_the_line_before_capturing() { + let mut s = harness(); + s.fire_event("SCREENSHOT_SUCCEEDED", Vec::new()); + s.tick(0.5); + assert!(shown(&s), "half a second in, the line is still up"); + assert!(alpha(&s) < 1.0, "and already fading"); + + s.run("TakeScreenshot()").unwrap(); + assert!( + !shown(&s), + "hidden BEFORE the engine is asked, not after it answers" + ); + assert_eq!(s.take_screenshot_asks(), 1); + + // The new answer restarts the fade at full rather than inheriting the old alpha. + s.fire_event("SCREENSHOT_SUCCEEDED", Vec::new()); + assert_eq!(alpha(&s), 1.0); +} + +/// The 1.5 s fade: alpha falls with elapsed time and the frame takes itself off screen at the end. +#[test] +fn the_line_fades_out_over_the_reference_s_second_and_a_half() { + let mut s = harness(); + s.fire_event("SCREENSHOT_SUCCEEDED", Vec::new()); + + s.tick(0.75); + let half = alpha(&s); + assert!( + (half - 0.5).abs() < 1e-3, + "halfway through 1.5 s the line is at half alpha, got {half}" + ); + + s.tick(0.75); + assert!(!shown(&s), "and it is gone at the end of the fade"); +} + +/// The failure path says so, in the reference's own words. +#[test] +fn a_failed_capture_shows_the_failure_string() { + let mut s = harness(); + s.fire_event("SCREENSHOT_FAILED", Vec::new()); + assert!(shown(&s)); + assert_eq!(text(&s), "Screen Capture Failed"); +} diff --git a/crates/benilla-ui/src/civil.rs b/crates/benilla-ui/src/civil.rs new file mode 100644 index 00000000..c050a795 --- /dev/null +++ b/crates/benilla-ui/src/civil.rs @@ -0,0 +1,142 @@ +//! **The tree's one civil-time conversion** — broken-down UTC from an epoch second. +//! +//! It lives here because this crate already had to have one: 1.12's `date()` global is Lua 5.0's +//! `os.date` hoisted, and the sandbox strips `os` ([`crate::script`]'s stdlib), so the calendar +//! had to be written by hand. The screenshot writer is its second caller (decision 1487), and one +//! correct closed form with two callers beats two hand-rolled loops. +//! +//! **UTC, deliberately, and it is a stated divergence wherever a caller wanted local time.** The +//! workspace has no date dependency at all — every format crate in it is in-repo — and resolving a +//! local offset needs a timezone source, not a different algorithm. `date()` records the same +//! divergence for the same reason. + +/// Broken-down UTC time. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub struct Civil { + pub year: i64, + /// 1–12. + pub month: u32, + /// 1–31. + pub day: u32, + pub hour: u32, + pub min: u32, + pub sec: u32, + /// 0 = Sunday. + pub weekday: u32, + /// 1–366. + pub yearday: u32, +} + +/// Break an epoch second down into UTC fields. +/// +/// The civil-from-days conversion is Howard Hinnant's, shifted to a March-based year so the leap +/// day lands at the end and no month-length table is needed. Chosen over a hand-rolled loop +/// because it is a known-correct closed form with no accumulation error, which matters for the +/// "persisted last session, compared this session" use every corpus `date()` caller has. +pub fn from_unix(secs: i64) -> Civil { + let days = secs.div_euclid(86_400); + let rem = secs.rem_euclid(86_400); + let (hour, min, sec) = ( + (rem / 3600) as u32, + ((rem % 3600) / 60) as u32, + (rem % 60) as u32, + ); + // 1970-01-01 was a Thursday (4). + let weekday = (days + 4).rem_euclid(7) as u32; + + let z = days + 719_468; + let era = z.div_euclid(146_097); + let doe = z.rem_euclid(146_097); + let yoe = (doe - doe / 1460 + doe / 36_524 - doe / 146_096) / 365; + let y = yoe + era * 400; + let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); + let mp = (5 * doy + 2) / 153; + let day = (doy - (153 * mp + 2) / 5 + 1) as u32; + let month = if mp < 10 { mp + 3 } else { mp - 9 } as u32; + let year = if month <= 2 { y + 1 } else { y }; + + // Day of the year, from the same conversion applied to Jan 1. + let yearday = (days - days_from_civil(year, 1, 1) + 1) as u32; + Civil { + year, + month, + day, + hour, + min, + sec, + weekday, + yearday, + } +} + +/// Days since the epoch for a civil date — the inverse of the above, used only for `yearday`. +fn days_from_civil(y: i64, m: u32, d: u32) -> i64 { + let y = if m <= 2 { y - 1 } else { y }; + let era = y.div_euclid(400); + let yoe = y.rem_euclid(400); + let mp = if m > 2 { m - 3 } else { m + 9 } as i64; + let doy = (153 * mp + 2) / 5 + d as i64 - 1; + let doe = yoe * 365 + yoe / 4 - yoe / 100 + doy; + era * 146_097 + doe - 719_468 +} + +/// Wall-clock seconds since the Unix epoch. Before the epoch is not a state a game client is in; a +/// clock that somehow reports it clamps to 0 rather than raising inside an addon's file scope. +pub fn unix_seconds() -> i64 { + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs() as i64) + .unwrap_or(0) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Four instants that between them cover the epoch itself, a leap day, a century boundary and + /// an ordinary afternoon — the cases a closed-form conversion gets wrong when it is wrong. + #[test] + fn known_instants_break_down() { + let cases = [ + (0_i64, 1970, 1, 1, 0, 0, 0, 4, 1), + (1_787_319_907, 2026, 8, 21, 13, 45, 7, 5, 233), + (1_709_251_199, 2024, 2, 29, 23, 59, 59, 4, 60), + (946_598_400, 1999, 12, 31, 0, 0, 0, 5, 365), + ]; + for (secs, year, month, day, hour, min, sec, weekday, yearday) in cases { + assert_eq!( + from_unix(secs), + Civil { + year, + month, + day, + hour, + min, + sec, + weekday, + yearday + }, + "epoch second {secs}" + ); + } + } + + /// Before 1970 is not a state a client is in, but the arithmetic must not wrap into nonsense + /// if a clock reports it — `div_euclid`/`rem_euclid` are load-bearing here, not stylistic. + #[test] + fn negative_seconds_stay_civil() { + assert_eq!( + from_unix(-1), + Civil { + year: 1969, + month: 12, + day: 31, + hour: 23, + min: 59, + sec: 59, + weekday: 3, + yearday: 365 + } + ); + } +} diff --git a/crates/benilla-ui/src/lib.rs b/crates/benilla-ui/src/lib.rs index ddbd3878..73b6687e 100644 --- a/crates/benilla-ui/src/lib.rs +++ b/crates/benilla-ui/src/lib.rs @@ -22,6 +22,7 @@ //! to text only where a Rust parser forces it (decision 1193). pub mod bindings_xml; +pub mod civil; pub mod framexml; pub mod justify; pub mod layout; diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 059ebcab..6a3fc702 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -71,6 +71,7 @@ mod font_block; mod gossip; mod guild; mod handler_prof; +mod screenshot; pub use handler_prof::HandlerRow; mod inspect; mod item_stats; @@ -503,6 +504,7 @@ impl UiScript { channel::install(&lua)?; chat_window::install(&lua)?; client::install(&lua)?; + screenshot::install(&lua)?; stdlib::sandbox(&lua)?; // Before the stdlib layer, so its aliases bind the 5.0-shaped functions (decision 1194). lua50::install(&lua)?; diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index eecf7d23..c455c2d2 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -1136,6 +1136,10 @@ pub(crate) struct Model { /// `CMSG_PLAYED_TIME`. A COUNT, not a payload, for [`super::pvp`]'s reason: the packet is /// empty, so two asks in a frame are two sends rather than one collapsed intent. pub(crate) played_time_asks: u32, + /// `Screenshot()` calls queued since the app last drained them — each is one capture + /// (decision 1487). A COUNT for [`Self::played_time_asks`]'s reason: the request carries no + /// payload, so two calls in a frame are two captures. + pub(crate) screenshot_asks: u32, pub(crate) realm_name: String, /// The hearthstone bind location's NAME, behind `GetBindLocation()` — the app resolves the /// `SMSG_BINDPOINTUPDATE` area id through the same AreaTable catalog the hearthstone's `$z` @@ -1439,6 +1443,7 @@ impl Model { chat_sends: Vec::new(), addon_sends: Vec::new(), played_time_asks: 0, + screenshot_asks: 0, realm_name: String::new(), bind_location: String::new(), binder_confirms: 0, diff --git a/crates/benilla-ui/src/script/screenshot.rs b/crates/benilla-ui/src/script/screenshot.rs new file mode 100644 index 00000000..7a69c3ee --- /dev/null +++ b/crates/benilla-ui/src/script/screenshot.rs @@ -0,0 +1,41 @@ +//! **`Screenshot()`** — the engine verb behind the print-screen key (decision 1487). +//! +//! One global and one counter. The reference's binding is `SCREENSHOT` → `TakeScreenshot()` +//! (FrameXML `Bindings.xml`), whose body hides the status text and then calls this engine +//! function; the engine answers later with `SCREENSHOT_SUCCEEDED` or `SCREENSHOT_FAILED`, which +//! is what puts "Screen Captured" on screen. **The answer is an EVENT, never a return value** — +//! and that is the whole mechanism that keeps the message out of the file it announces: the +//! capture is already taken by the time anything can say so. +//! +//! A **count**, like [`super::client`]'s `RequestTimePlayed` and `ConfirmBinder`: the request +//! carries no payload (the app owns the window, the folder and the clock), so two calls in a frame +//! are two captures rather than one collapsed intent. The app drains it in +//! [`crate::script::UiScript::take_screenshot_asks`]. + +use mlua::Lua; + +use super::Model; + +pub(super) fn install(lua: &Lua) -> mlua::Result<()> { + // `Screenshot()` — take one. Returns nothing: the reference's does not either, because the + // capture completes after the frame that asked for it and the outcome arrives as + // SCREENSHOT_SUCCEEDED / SCREENSHOT_FAILED. + lua.globals().set( + "Screenshot", + lua.create_function(|lua, ()| { + lua.app_data_mut::() + .expect("model app_data") + .screenshot_asks += 1; + Ok(()) + })?, + )?; + Ok(()) +} + +impl super::UiScript { + /// Drain the `Screenshot()` calls queued since the last call — each one is one capture. + /// [`super::UiScript::take_played_time_asks`]'s shape, for the same reason. + pub fn take_screenshot_asks(&mut self) -> u32 { + std::mem::take(&mut self.model_mut().screenshot_asks) + } +} diff --git a/crates/benilla-ui/src/script/stdlib.rs b/crates/benilla-ui/src/script/stdlib.rs index d0e3c1db..61664503 100644 --- a/crates/benilla-ui/src/script/stdlib.rs +++ b/crates/benilla-ui/src/script/stdlib.rs @@ -443,13 +443,16 @@ end /// they shift by a constant. Fixing it means a tz source, not a different algorithm. fn install_time(lua: &Lua) -> mlua::Result<()> { let g = lua.globals(); - g.set("time", lua.create_function(|_, ()| Ok(unix_seconds()))?)?; + g.set( + "time", + lua.create_function(|_, ()| Ok(crate::civil::unix_seconds()))?, + )?; g.set( "date", lua.create_function(|lua, (fmt, when): (Option, Option)| { // Bare `date()` is `%c`, as in Lua — `Recap.lua:2690` calls it with no arguments. let fmt = fmt.unwrap_or_else(|| "%c".to_string()); - let secs = when.unwrap_or_else(unix_seconds); + let secs = when.unwrap_or_else(crate::civil::unix_seconds); // A leading `!` selects UTC. Every timestamp this engine holds is already UTC — there // is no local-time conversion anywhere here — so the flag only has to be CONSUMED, not // acted on. Left in the format string it would print as a literal `!`. @@ -465,18 +468,18 @@ fn install_time(lua: &Lua) -> mlua::Result<()> { // the client — which is exactly what it did to the survey the moment a separate fix let // Accountant reach this function at all. if body == "*t" { - let (year, month, day, hour, min, sec, wday, yday) = civil_from_epoch(secs); + let c = crate::civil::from_unix(secs); let t = lua.create_table()?; - t.set("year", year)?; - t.set("month", month)?; - t.set("day", day)?; - t.set("hour", hour)?; - t.set("min", min)?; - t.set("sec", sec)?; + t.set("year", c.year)?; + t.set("month", c.month)?; + t.set("day", c.day)?; + t.set("hour", c.hour)?; + t.set("min", c.min)?; + t.set("sec", c.sec)?; // Lua counts weekdays 1..7 from SUNDAY; ours is the 0-based index into `DAYS`. - t.set("wday", wday + 1)?; + t.set("wday", c.weekday + 1)?; // `yday` is already 1-based out of the conversion, which is Lua's convention too. - t.set("yday", yday)?; + t.set("yday", c.yearday)?; // No timezone and no DST rules here, and `false` is the honest answer rather than // the nil an absent field would give: this clock never observes daylight saving. t.set("isdst", false)?; @@ -490,15 +493,6 @@ fn install_time(lua: &Lua) -> mlua::Result<()> { Ok(()) } -/// Wall-clock seconds since the Unix epoch. Before the epoch is not a state a game client is in; -/// a clock that somehow reports it clamps to 0 rather than raising inside an addon's file scope. -fn unix_seconds() -> i64 { - std::time::SystemTime::now() - .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as i64) - .unwrap_or(0) -} - const DAYS: [&str; 7] = [ "Sunday", "Monday", @@ -523,52 +517,6 @@ const MONTHS: [&str; 12] = [ "December", ]; -/// Broken-down UTC time from an epoch second: (year, month 1-12, day 1-31, hour, min, sec, -/// weekday 0=Sunday, yearday 1-366). -/// -/// The civil-from-days conversion is Howard Hinnant's, shifted to a March-based year so the leap -/// day lands at the end and no month-length table is needed. Chosen over a hand-rolled loop because -/// it is a known-correct closed form with no accumulation error, which matters for the "persisted -/// last session, compared this session" use every corpus caller has. -fn civil_from_epoch(secs: i64) -> (i64, u32, u32, u32, u32, u32, u32, u32) { - let days = secs.div_euclid(86_400); - let rem = secs.rem_euclid(86_400); - let (hour, min, sec) = ( - (rem / 3600) as u32, - ((rem % 3600) / 60) as u32, - (rem % 60) as u32, - ); - // 1970-01-01 was a Thursday (4). - let weekday = (days + 4).rem_euclid(7) as u32; - - let z = days + 719_468; - let era = z.div_euclid(146_097); - let doe = z.rem_euclid(146_097); - let yoe = (doe - doe / 1460 + doe / 36_524 - doe / 146_096) / 365; - let y = yoe + era * 400; - let doy = doe - (365 * yoe + yoe / 4 - yoe / 100); - let mp = (5 * doy + 2) / 153; - let day = (doy - (153 * mp + 2) / 5 + 1) as u32; - let month = if mp < 10 { mp + 3 } else { mp - 9 } as u32; - let year = if month <= 2 { y + 1 } else { y }; - - // Day of the year, from the same conversion applied to Jan 1. - let jan1 = days_from_civil(year, 1, 1); - let yearday = (days - jan1 + 1) as u32; - (year, month, day, hour, min, sec, weekday, yearday) -} - -/// Days since the epoch for a civil date — the inverse of the above, used only for `%j`. -fn days_from_civil(y: i64, m: u32, d: u32) -> i64 { - let y = if m <= 2 { y - 1 } else { y }; - let era = y.div_euclid(400); - let yoe = y.rem_euclid(400); - let mp = if m > 2 { m - 3 } else { m + 9 } as i64; - let doy = (153 * mp + 2) / 5 + d as i64 - 1; - let doe = yoe * 365 + yoe / 4 - yoe / 100 + doy; - era * 146_097 + doe - 719_468 -} - /// The `strftime` subset the corpus actually uses, plus the obvious neighbours. /// /// Bounded on purpose: the specifiers here are the ones read off real call sites — @@ -576,7 +524,16 @@ fn days_from_civil(y: i64, m: u32, d: u32) -> i64 { /// `%c` (bare `date()`). An unknown specifier is emitted VERBATIM rather than swallowed, so a /// format this does not know shows up in the output instead of silently vanishing. fn format_epoch(secs: i64, fmt: &str) -> String { - let (year, month, day, hour, min, sec, wday, yday) = civil_from_epoch(secs); + let crate::civil::Civil { + year, + month, + day, + hour, + min, + sec, + weekday: wday, + yearday: yday, + } = crate::civil::from_unix(secs); let mut out = String::with_capacity(fmt.len() + 16); let mut chars = fmt.chars().peekable(); while let Some(c) = chars.next() { diff --git a/crates/benilla-world/src/entity_shade.rs b/crates/benilla-world/src/entity_shade.rs index 823744bc..2a165f29 100644 --- a/crates/benilla-world/src/entity_shade.rs +++ b/crates/benilla-world/src/entity_shade.rs @@ -137,6 +137,12 @@ pub struct GroundShade { /// The last effective target the `WOW_INTERIOR_LOG` instrument printed (log-on-change; starts /// off-scale so the first resolved target always prints). logged_target: f32, + /// The shade byte the descendant walk last pushed to this root's parts — the settled gate + /// (1490): while it matches this frame's byte and no other lane rewrote a part's tag (the + /// reclaim detector in [`update_ground_shade`]), the walk is skipped whole. `None` = the + /// parts are not known to carry any byte — fresh, or hidden by the election (a wake must + /// re-assert even at an unchanged byte). + asserted: Option, } impl Default for GroundShade { @@ -151,6 +157,7 @@ impl Default for GroundShade { ambient: Vec3::ZERO, ambient_target: Vec3::ZERO, logged_target: -1.0, + asserted: None, } } } @@ -197,13 +204,55 @@ impl GroundShade { } } +/// Roots owed a shade re-assert this frame even at an unchanged byte: some OTHER lane rewrote a +/// descendant part's `MeshTag` since [`detect_shade_reclaims`] last ran — the interior +/// classifier's exterior reclaim (1358), a late-attached part (`Added` counts as changed). +/// What lets [`update_ground_shade`]'s per-part walk skip the settled crowd (1490): steady +/// frames have no foreign tag writes, so this is empty and every settled drawn body costs the +/// ramp arithmetic and one map insert. +#[derive(Resource, Default)] +pub(crate) struct ShadeDirtyRoots(bevy::ecs::entity::EntityHashSet); + +/// The reclaim detector — its own system because `Changed` is read access and the +/// walk's query holds `&mut MeshTag` (B0001). Runs between the classifier and the walk, so a +/// same-frame reclaim is caught the frame it happens; a writer scheduled after the walk +/// (a PostUpdate fade) is caught on the next frame, exactly when the old unconditional walk +/// would have repaired it too. The walk's own writes land after this system's baseline and so +/// echo back as dirty for ONE extra frame — that echo walk compares every byte equal, writes +/// nothing, and the chain goes quiet; it cannot oscillate. +pub(crate) fn detect_shade_reclaims( + changed_parts: Query, Without)>, + shade_roots: Query<(), With>, + child_of: Query<&ChildOf>, + mut dirty: ResMut, +) { + dirty.0.clear(); + for part in &changed_parts { + let mut node = part; + loop { + if shade_roots.contains(node) { + dirty.0.insert(node); + break; + } + let Ok(up) = child_of.get(node) else { break }; + node = up.parent(); + } + } +} + pub(crate) struct EntityShadePlugin; impl Plugin for EntityShadePlugin { fn build(&self, app: &mut App) { // After the interior classifier: its exterior reclaim writes a fresh tag (shade byte 0) the - // same frame this re-asserts the ramped byte, so the pair can't fight across frames. - app.add_systems(Update, update_ground_shade.after(classify_entity_interior)); + // same frame this re-asserts the ramped byte, so the pair can't fight across frames. The + // detector sits between the two so the reclaim is observed the frame it happens. + app.init_resource::().add_systems( + Update, + (detect_shade_reclaims, update_ground_shade) + .chain() + .after(classify_entity_interior), + ); } } @@ -243,6 +292,9 @@ pub(crate) fn update_ground_shade( // The card pass's up-walk: a card can follow a deep JOINT (`following_joint` — the swinging // lamp's glow), and a joint is not a shade root. child_of: Query<&ChildOf>, + // The reclaim detector's verdict ([`detect_shade_reclaims`], one stage earlier): roots owed + // a re-assert even at an unchanged byte. + dirty_roots: Res, mut self_log: Local, ) { root_shade.clear(); @@ -328,20 +380,31 @@ pub(crate) fn update_ground_shade( ramp_toward(a.z, at.z, ambient_step), ); let byte = (shade.t * 255.0).round().clamp(0.0, 255.0) as u8; + // Every root enters the card map, hidden or not — the insert is one hash write, and a + // hidden body's card used to walk its whole parent chain to a `None` precisely because + // its root was left out. + root_shade.insert(root, byte); // A root the election hid (1270/1475) draws nothing, so the re-assert walk below is // skipped whole — 1473's audit found it running over every part of every off-view body. - // The ramps above kept stepping, so the byte is current the frame the body wakes; this - // system runs one stage before the election (Update vs PostUpdate), so the wake frame's - // walk re-asserts at most one frame late, into parts whose tags retained their last byte. + // The ramps above kept stepping, so the byte is current the frame the body wakes; the + // cleared `asserted` is what makes the wake frame's walk unconditional, into parts + // whose tags retained their last byte. if root_vis.is_some_and(|v| *v == Visibility::Hidden) { + shade.asserted = None; continue; } + // The settled gate (1490): every part below already carries this byte (we pushed it — + // `asserted`) and no other lane rewrote a tag since (`dirty_roots`). The walk used to be + // unconditional for a drawn root as the MeshTag re-assert over the classifier's + // exterior reclaim (1358); the reclaim detector above answers that by observation + // instead of by re-walking every body every frame. + if shade.asserted == Some(byte) && !dirty_roots.0.contains(&root) { + continue; + } + shade.asserted = Some(byte); // Push to every part below the root (body submeshes are direct children; held items/helm ride // joint entities deeper down — same full-tree walk as the self-fade). Change-gated per part on - // the byte, so a settled entity writes nothing and never re-triggers render extraction. The - // walk itself is unconditional for a DRAWN root: it is the MeshTag re-assert over the - // classifier's exterior reclaim (1358), not a change-only push. - root_shade.insert(root, byte); + // the byte, so a settled entity writes nothing and never re-triggers render extraction. for part in children.iter_descendants(root) { let Ok((mut tag, lit)) = parts.get_mut(part) else { continue; @@ -417,6 +480,39 @@ mod tests { card_root_shade(map, &state.get(world), follows) } + /// The reclaim detector's tick contract (1490): a fresh part flags its root (Added counts + /// as changed); a quiet frame flags nothing; a foreign tag write flags the root again — and + /// only the root above the touched part, not every root in the world. This is what lets + /// the shade walk skip the settled crowd without ever missing 1358's exterior reclaim. + #[test] + fn a_foreign_tag_write_flags_exactly_its_root() { + let mut app = App::new(); + app.init_resource::() + .add_systems(Update, detect_shade_reclaims); + let root = app.world_mut().spawn(GroundShade::default()).id(); + let part = app.world_mut().spawn((MeshTag(7), ChildOf(root))).id(); + let other = app.world_mut().spawn(GroundShade::default()).id(); + app.world_mut().spawn((MeshTag(9), ChildOf(other))); + + app.update(); // both parts are Added ⇒ both roots flagged + let dirty = |app: &App| { + let d = app.world().resource::(); + (d.0.contains(&root), d.0.contains(&other)) + }; + assert_eq!(dirty(&app), (true, true), "a fresh part flags its root"); + + app.update(); // nobody wrote ⇒ quiet + assert_eq!(dirty(&app), (false, false), "a settled frame flags nothing"); + + app.world_mut().get_mut::(part).unwrap().0 = 42; + app.update(); + assert_eq!( + dirty(&app), + (true, false), + "a foreign write flags its own root and nobody else's" + ); + } + /// A card's owner can sit arbitrarily deep in its root's tree (a held torch's flame card /// follows a hand JOINT under an item entity under the unit) — the up-walk finds the root's /// byte; the root itself, as an owner, is the walk's zero-hop case. diff --git a/crates/benilla-world/src/liquid/mod.rs b/crates/benilla-world/src/liquid/mod.rs index 7d7ea02f..961a688b 100644 --- a/crates/benilla-world/src/liquid/mod.rs +++ b/crates/benilla-world/src/liquid/mod.rs @@ -92,7 +92,7 @@ pub use query::{ FoamPatch, LiquidClaim, LiquidHit, LiquidSource, RoomPlacements, Underwater, WaterChunkInfo, WmoPool, }; -pub(crate) use spatial::WaterIndex; +pub(crate) use spatial::{maintain_water_index, WaterIndex}; pub(crate) use surface::{spawn_liquids, spawn_wmo_liquids, LiquidAssets, LiquidSoundSource}; /// The frame slot where [`Underwater`] is written — the label every consumer of the submersion @@ -117,7 +117,7 @@ impl Plugin for LiquidPlugin { // PreUpdate: surfaces stream in/out via Update-side commands, so the edge is visible // here the frame after — before any of that frame's consumers ask. A despawn's stale // entry in between self-filters at the consumer (`Query::get` misses). - .add_systems(PreUpdate, spatial::maintain_water_index) + .add_systems(PreUpdate, maintain_water_index) .add_systems(Startup, surface::setup_liquid.after(AssetSet::Open)) .add_systems( Update, diff --git a/crates/benilla-world/src/liquid/spatial.rs b/crates/benilla-world/src/liquid/spatial.rs index e24c6f09..d99cac85 100644 --- a/crates/benilla-world/src/liquid/spatial.rs +++ b/crates/benilla-world/src/liquid/spatial.rs @@ -49,12 +49,33 @@ impl WaterIndex { .get(&Self::cell_of(x, y)) .map_or(&[], Vec::as_slice) } + + /// Every surface registered in any cell the WoW-space box `[lo, hi]` touches, deduplicated — + /// the candidate set for a small box query (the foam patch builder), with [`Self::over`]'s + /// same superset guarantee: a surface overlapping the box overlaps one of its cells, and a + /// surface is registered in every cell its footprint overlaps. Boxes here are a few yards + /// (1–4 cells), buckets are short — the linear dedup is the cheap shape. + pub(crate) fn over_box(&self, lo: [f32; 2], hi: [f32; 2]) -> Vec { + let [x0, y0] = Self::cell_of(lo[0], lo[1]); + let [x1, y1] = Self::cell_of(hi[0], hi[1]); + let mut out = Vec::new(); + for cx in x0..=x1 { + for cy in y0..=y1 { + for &e in self.cells.get(&[cx, cy]).map_or(&[][..], Vec::as_slice) { + if !out.contains(&e) { + out.push(e); + } + } + } + } + out + } } /// Rebuild [`WaterIndex`] when the surface population changed — tile stream edges only. A full /// rebuild over ~2k surfaces is microseconds, so incremental bookkeeping would be complexity /// with nothing to buy. -pub(super) fn maintain_water_index( +pub(crate) fn maintain_water_index( mut index: ResMut, added: Query<(), Added>, mut removed: RemovedComponents, diff --git a/crates/benilla-world/src/water_fx/mod.rs b/crates/benilla-world/src/water_fx/mod.rs index dd756982..8ab7c37f 100644 --- a/crates/benilla-world/src/water_fx/mod.rs +++ b/crates/benilla-world/src/water_fx/mod.rs @@ -49,7 +49,7 @@ use bevy::render::render_resource::{Extent3d, TextureDimension, TextureFormat}; use benilla_assets::coords::{bevy_to_wow, wow_to_bevy}; -use crate::liquid::{FoamPatch, WaterChunkInfo}; +use crate::liquid::{FoamPatch, WaterChunkInfo, WaterIndex}; use crate::particles::buffer::{ begin_effect_frame, EffectBlend, EffectDrawSpec, EffectFog, EffectQuads, EffectVertex, }; @@ -308,7 +308,8 @@ fn drive_unit( scale: f32, // The unit's collision height (yd) — both depth lines below are fractions of it (0645). h: f32, - chunks: &[(Entity, &WaterChunkInfo, &FoamPatch)], + water: &Query<(Entity, &WaterChunkInfo, &FoamPatch)>, + index: &WaterIndex, now: f32, ) { let gate = (GATE_DEPTH_FRAC * h).max(1.0); @@ -316,11 +317,15 @@ fn drive_unit( let wow = bevy_to_wow(pos); // The surface height under the unit, from the wet cell it stands on — not the chunk's box and // not the chunk's highest vertex, which on a sloped river put the wade depth ~2 yd out - // (decision 0642). - let Some(surface) = chunks - .iter() - .find_map(|(_, info, _)| info.surface_z_at(wow[0], wow[1])) - else { + // (decision 0642). Through [`WaterIndex`], not the full chunk walk: this line used to be a + // linear scan of every loaded surface (~2.2k at a city pin) PER UNIT PER FRAME — exactly the + // per-consumer shape `liquid/spatial.rs`'s module doc warns detonates; a dry-land unit now + // costs one hash miss. A dead index entry self-filters at `water.get` (the index contract), + // and overlapping surfaces were already first-match-in-arbitrary-order before this. + let Some(surface) = index.over(wow[0], wow[1]).iter().find_map(|&e| { + let (_, info, _) = water.get(e).ok()?; + info.surface_z_at(wow[0], wow[1]) + }) else { foam_state.wading = false; return; }; @@ -345,7 +350,19 @@ fn drive_unit( }; let final_size = p.size0 + p.growth * p.lifetime; let center = [wow[0], wow[1]]; - if let Some((verts, chunk)) = build_patch(center, final_size, chunks) { + // Candidates for the patch box only — resolved here, on the EMISSION path (past the depth + // gate and the cooldown), never on the every-frame classify above. `over_box` unions the + // 1–4 cells a final-size box touches, so the set `build_patch` filters with `overlaps` is + // the same one the full walk offered it. + let candidates: Vec<_> = index + .over_box( + [center[0] - final_size, center[1] - final_size], + [center[0] + final_size, center[1] + final_size], + ) + .into_iter() + .filter_map(|e| water.get(e).ok()) + .collect(); + if let Some((verts, chunk)) = build_patch(center, final_size, &candidates) { alloc(FoamRecord { center, heading, @@ -377,6 +394,7 @@ fn drive_unit( /// Per frame: run the driver for the avatar (its real movement flags — the reference's own /// selection bits) and for every streamed unit (the velocity/yaw proxy), emitting pool records. +#[allow(clippy::too_many_arguments)] // the emitter's real input set; the index ride-along tipped it fn emit_water_foam( time: Res