From 7a03f5d211337456451c25823bd2aac99ea51aca Mon Sep 17 00:00:00 2001 From: Sam Date: Sun, 6 Sep 2026 19:41:02 +0700 Subject: [PATCH] strings become keys, hidden tiles park, and the water stops breathing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - a reference string is a key, never a Rust literal: display text is a key into `GlobalStrings.lua` in-game or `GlueStrings.lua` on the glue screens, resolved at runtime from the player's own install, and a literal is legitimate only as a fallback beside a lookup — the reference's own `UNKNOWNOBJECT` shape — because a re-typed sentence costs the wording nobody diffed, the surface and the voice a composed string has no catalog record to read off (1770, 1815), and localization; the tripwire in `reference_strings.rs` is the check a comment could never be: it walks the workspace, normalises both sides and fails on any literal matching a shipped value — 273 re-typed literals over 34 files, after calibration threw out three false-positive classes (a path-mangling bug, the 85 fallbacks beside a real lookup, and `#[cfg(test)]` modules whose fake tables match by construction — the first run's 378 would have enshrined ~50 phantom sites); `ALLOWED` is a per-file ratchet whose counts may only go down and whose stale rows fail too, regenerated from a clean tree at 253 over 33 files — the first baseline was captured mid-edit and the land gates failed on a row claiming drift in a file that had none — then 252 over 32 as `ui_trade`'s row retired behind a neighbouring conversion to `ERR_TRADE_BLOCKED_S` (2042); stated in its own doc: it cannot find an INVENTED sentence, which matches nothing; and `benilla_ui::strings::fill` is the one template filler — ordered `%s`/`%d`, positional `%N$s`, `%%` collapsing, a starved specifier copied through visibly so a mis-modelled template looks wrong rather than plausible — replacing eight hand-rolled copies that disagreed, one of them `str::replace` filling every `%s` with the same argument, latent only because no message on that queue carried two yet (2045) - two families convert, and both assert the identifier, not the sentence: the character-create result line resolves its key off the player's chain with the literal as fallback, and three bugs the 23-arm literal table had hidden come out — `0x36` re-typed without its second sentence ("Please try again during off peak hours."), `0x50` invented (there is no `CHAR_NAME_CONSECUTIVE_SPACES` in 1.12, so it takes the default arm), and `0x4B` naming `CHAR_CREATE_NAME_IN_USE` where the code means `CHAR_NAME_RESERVED` — both read "That name is unavailable" in enUS, so no assertion on displayed text could ever have caught it, and a sentinel test that rewrites `CHAR_NAME_RESERVED` in a copy of the real table proves `0x4B` moves while `0x31` does not; the ratchet's first row retires, 21 → 0; and `SMSG_PARTY_COMMAND_RESULT` returns the catalog message the reference's own jump table names — handler `0x5e68b0`, `dec eax; cmp eax,7; ja ; jmp [4*eax+0x5e6a14]`, results 1–8 onto `ERR_BAD_PLAYER_NAME_S` … `ERR_IGNORING_YOU_S` plus `ERR_INVITE_PLAYER_S` (`0x3a`) and `ERR_LEFT_GROUP_YOU` (`0x42`) on result 0 — so text, surface and sound come off one row: `ERR_PLAYER_WRONG_FACTION` (`0xff`) is the red line where the other nine are chat, and three inputs the reference is DELIBERATELY silent on (result ≥ 9, result 0 outside op {0, 2}, an invite with an empty name) stop printing an invented "Party command failed (N)."; tests assert the message id, since no English distinguishes a right key from a wrong one here, and `ui_party` goes 22 → 12 (2045) - a hidden `` tile is parked: every orthographic pane shares the atlas's one camera, deliberately kept active whenever any cell is packed because it is the camera that clears the sheet, so `booth_frozen` — exact for every booth ever built, one camera one scene — answered "drawn" for every pane that had left the paint list, and a hidden autocast-shine pane's four spline emitters kept integrating and pushing quads for the whole 600-frame linger at a cell the packer had since given away; `sync_tiles` gains a sixth stage that parks any live tile with no cell this frame — `AnimParked` on the root, which holds the pose evaluator, the compose, the palette write and the global-sequence bone writes, and `ParticleEmitter::set_frozen` on every emitter — with `scene_frozen` stating the freeze law in one place and the park verdict pinned to the composite by a test, so a repack or a full atlas cannot freeze a visible pane's cloud; the bone global sequences move onto the pane's clock, where the material rows have been since 2019: the kernel's Phase B cursor is `(sceneClock − attachSnapshot) % duration` off the scene that owns the instance (`[model+0x2c]`, the snapshot at `CM2Model+0x68` stored at `0x70eae1`), and a `` widget owns a private `CM2Scene` at `CSimpleModel+0x314` that only its own advance at `0x76d863` moves — so the ping's spinner holds while hidden and resumes where it stopped, with no accumulator of ours; measured with a cooldown pane holding the atlas alive: twelve hidden shine panes cost 54 effect items / 63 transparent draws before, 34 / 43 after, the pool held rather than drained by design; and the particle census's `gated=` column, which printed `fade.is_some()` and read backwards as the gate verdict, is renamed `has_fade=` (2046) - a doodad is not an entity: `[def+0xa4]` has exactly four writers image-wide — ctor `0x6a7d73` (0.0), WMO doodad-set commit `0x695bb3` (1.0), ADT MDDF commit `0x6b01bb` (1.0), MCSH refresh `0x698cb4` (0.5) — and the 2.5-producer `0x69e280` and the ramp `0x69e770` are reachable only through the entity-attached `[obj+0xe0]` path, which contradicted the §6 both lit-ground sites cited and a capture with gains up to 2.5, so the premise was marked contested at both sites first (the shader's own note told a future session to cut `min(I, 1)` and `LIT_T` together on its strength); wow-re then settled it — two C++ classes inline the same base and share the offset: `CMapDoodadDef` (vtable `0x810a74`, `"WDOODADDEF"`, size `0x198`, serving ADT MDDF and WMO MODD) and the WENTITY light node (vtable `0x810810`, size `0x108`, off `[obj+0xe0]`) that `0x69e770` ramps at 3.3333/s; on the doodad class `+0xf8`, the field the ramp reads its target from at `0x69e894`, is `m[2][3]` of the world matrix — it has no target field — and six trace frames put 111 MDDF placements over 356 draws at gains only in {0.5, 1.0}, none changing, against an entity walking out of shadow at 0.5 / 0.81 / 1.09 / 1.3367 and rising; so ADT map doodads join the fixed-1.0 family with the exterior WMO props, `ShadeSel::Lit` is entity-only, the retained lane's Matte arm drops its prop requirement, and no pixel changes — the cap was faithful for trees all along and lifting it is now an entity-only look change for the director to call; `WORD_SHADE_LIT` is left as a documented dead bit (2050); and the far water's 1.25 s breath is flattened: the director's recording autocorrelates at lag 75 @ 60 fps = 1.250 s (r = +0.60), the 30-frame ocean flipbook at 24 fps, whose frames do not share a mean — alpha 55.985..57.628, DXT3's 4-bit alpha swinging 1.64 at mip 0, 3.72 at mip 5, 6.38 at mip 6 — so at distance the mipped sheet moves together, 0.8..1.84/255 on screen, fogged out at `farclip` 300 and visible at 777; wow-re proved the reference breathes too (`0x68abab` creates the flipbook with `applyGlobal = 1`, the upload hands the driver `Levels = 9`, LOD bias 0, the loop period `[0x8102b8]` 1.25 s over 30 frames), so ours was faithful and it is flattened anyway by the director's eye: each frame's DC equalised per level and per channel at texture-build time as an exact integer redistribution in ±1 steps on a coprime stride (1.64 / 3.72 / 6.38 / 0.555 → 0.000), water and ocean only since magma and slime take the sheet as their body colour, `WOW_LIQUID_DC=raw` restoring the authored art for the A/B (2051) Co-Authored-By: Claude Fable 5.1 --- .../src/capture/probes/particle_census.rs | 6 +- crates/benilla-app/src/char_create/mod.rs | 200 +++++++-- crates/benilla-app/src/glue_strings.rs | 9 +- crates/benilla-app/src/net/apply.rs | 2 +- crates/benilla-app/src/net/apply/group.rs | 13 +- crates/benilla-app/src/ui_action/errors.rs | 25 +- crates/benilla-app/src/ui_duel.rs | 8 +- crates/benilla-app/src/ui_instance.rs | 67 +-- crates/benilla-app/src/ui_models/mod.rs | 180 +++++++- crates/benilla-app/src/ui_party/mod.rs | 269 ++++++++++-- crates/benilla-app/tests/reference_strings.rs | 404 ++++++++++++++++++ .../benilla-assets/src/shaders/wow_model.wgsl | 37 +- .../benilla-assets/src/world_assets/images.rs | 199 ++++++++- crates/benilla-ui/src/lib.rs | 1 + crates/benilla-ui/src/strings.rs | 237 ++++++++++ crates/benilla-world/src/liquid/surface.rs | 10 +- crates/benilla-world/src/model_render.rs | 18 +- crates/benilla-world/src/particles.rs | 32 ++ crates/benilla-world/src/particles/sim.rs | 80 +++- .../benilla-world/src/rig_anim/global_seq.rs | 37 +- .../benilla-world/src/shaders/static_gx.wgsl | 4 +- crates/benilla-world/src/static_gx/mod.rs | 38 +- .../src/terrain_stream/spawn/mod.rs | 24 +- 23 files changed, 1715 insertions(+), 185 deletions(-) create mode 100644 crates/benilla-app/tests/reference_strings.rs create mode 100644 crates/benilla-ui/src/strings.rs diff --git a/crates/benilla-app/src/capture/probes/particle_census.rs b/crates/benilla-app/src/capture/probes/particle_census.rs index 84dc3360..72243232 100644 --- a/crates/benilla-app/src/capture/probes/particle_census.rs +++ b/crates/benilla-app/src/capture/probes/particle_census.rs @@ -138,7 +138,11 @@ fn fire_particle_census( let lane = if world_layer { "world" } else { "booth" }; let c = fade.map_or_else(|| e.anchor_world(), |f| f.center); format!( - " depth={d:.1} drawn={drawn} lane={lane} gated={} at=({:.0},{:.0},{:.0})", + // `has_fade`, not `gated`: this column prints whether the emitter carries an + // `EmitterFade`, and calling it `gated` made it read as the draw-set verdict — + // which is `drawn` (its inverse), right beside it. A tile-emitter reading was + // taken backwards off the old label before it was renamed (decision 2046). + " depth={d:.1} drawn={drawn} lane={lane} has_fade={} at=({:.0},{:.0},{:.0})", fade.is_some(), c.x, c.y, diff --git a/crates/benilla-app/src/char_create/mod.rs b/crates/benilla-app/src/char_create/mod.rs index 1c275236..a579c74c 100644 --- a/crates/benilla-app/src/char_create/mod.rs +++ b/crates/benilla-app/src/char_create/mod.rs @@ -33,6 +33,7 @@ use bevy::prelude::*; use crate::char_select::{ClientState, Roster}; use crate::entities::CharCreate; +use crate::glue_strings::GlueStrings; use crate::net::{CharActionResultMessage, CharPick, CharRequest}; use crate::portrait::{CreateLook, GlueLook, GluePreview}; use crate::sound::GlueSound; @@ -580,6 +581,7 @@ fn create_result( mut roster: ResMut, mut next: ResMut>, mut status: Query<&mut Text, With>, + strings: Res, ) { for msg in msgs.read() { if msg.action != CharAction::Create { @@ -589,7 +591,7 @@ fn create_result( info!( "char create: result {:#04x} — {}", msg.code, - char_result_text(msg.code) + char_result_text(&strings, msg.code) ); if msg.code == benilla_protocol::messages::CHAR_CREATE_SUCCESS { // The fresh roster already arrived (`net::io` re-enumerates and emits it BEFORE the @@ -599,44 +601,180 @@ fn create_result( roster.note_created(sel.name.text.clone()); next.set(ClientState::CharSelect); } else if let Ok(mut text) = status.single_mut() { - text.0 = char_result_text(msg.code).to_string(); + text.0 = char_result_text(&strings, msg.code).to_string(); } } } -/// Map a `SMSG_CHAR_CREATE` result byte to its 1.12 GlueStrings text (a frozen-fact table, extracted -/// verbatim from `Interface\GlueXML\GlueStrings.lua`; the codes are the vmangos `ResponseCodes` -/// enum, `CHAR_CREATE_SUCCESS = 0x2E` anchor). Unknown codes fall back to the generic error. -fn char_result_text(code: u8) -> &'static str { - match code { - 0x2E => "Character created", - 0x2F => "Error creating character", - 0x30 => "Character creation failed", - 0x31 => "That name is unavailable", - 0x32 => "Creation of that race and/or class is currently disabled.", - 0x33 => "You cannot have both a Horde and an Alliance character on the same PvP server", - 0x34 => "You already have the maximum number of characters allowed on this realm.", - 0x35 => "You already have the maximum number of characters allowed on this account.", - 0x36 => "This server is currently queued and new character creation is temporarily disabled.", - 0x37 => "Only players who already have characters on this realm are currently allowed to create characters.", - 0x45 => "Enter a name for your character", - 0x46 => "Names must be at least 2 characters", - 0x47 => "Names must be no more than 12 characters", - 0x48 => "Names can only contain letters", - 0x49 => "Names must contain only one language", - 0x4A => "That name contains profanity", - 0x4B => "That name is unavailable", - 0x4C => "You cannot use an apostrophe as the first or last character of your name", - 0x4D => "You can only have one apostrophe", - 0x4E => "You cannot use the same letter three times consecutively", - 0x4F => "You cannot use a space as the first or last character of your name", - 0x50 => "You cannot use consecutive spaces in a name", - _ => "Invalid character name", - } +/// A `SMSG_CHAR_CREATE` result byte → its **GlueStrings key**, resolved off the player's own +/// `GlueStrings.lua` with a built-in caption as the fallback — `login::world_refusal_text`'s shape, +/// and the reference's own (a key first, a literal only when the chain lacks it). +/// +/// The codes are vmangos's `ResponseCodes`, anchored at `CHAR_CREATE_SUCCESS = 0x2E`; every key +/// below was derived by matching our old text against the shipped file rather than by name, then +/// corrected where the *semantics* disagreed with the match. Three things that hiding behind +/// literals had concealed (decision 2045): +/// +/// - **`0x36` was missing a sentence.** Our copy of `CHAR_CREATE_SERVER_QUEUE` stopped at +/// "…temporarily disabled." where the shipped string continues "Please try again during off peak +/// hours." A re-typed string is a string nobody diffed. +/// - **`0x50` was invented.** "You cannot use consecutive spaces in a name" has no 1.12 +/// counterpart — there is no `CHAR_NAME_CONSECUTIVE_SPACES` in this chain — so it takes the +/// default arm, `CHAR_CREATE_INVALID_NAME`, exactly as any code the table does not name does. +/// Composing a better sentence for a code the client has no string for is what 2035 was about. +/// - **`0x4B` named the wrong key.** It resolved `CHAR_CREATE_NAME_IN_USE` because that key's text +/// *equals* `CHAR_NAME_RESERVED`'s in enUS — "That name is unavailable" — but the code is the +/// reserved-name one. Identical text, different key: a locale that words them apart would have +/// shown the wrong sentence, and no amount of text comparison could ever have found it. +fn char_result_text<'a>(strings: &'a GlueStrings, code: u8) -> &'a str { + let (key, fallback): (&str, &'a str) = match code { + 0x2E => ("CHAR_CREATE_SUCCESS", "Character created"), + 0x2F => ("CHAR_CREATE_ERROR", "Error creating character"), + 0x30 => ("CHAR_CREATE_FAILED", "Character creation failed"), + 0x31 => ("CHAR_CREATE_NAME_IN_USE", "That name is unavailable"), + 0x32 => ( + "CHAR_CREATE_DISABLED", + "Creation of that race and/or class is currently disabled.", + ), + 0x33 => ( + "CHAR_CREATE_PVP_TEAMS_VIOLATION", + "You cannot have both a Horde and an Alliance character on the same PvP server", + ), + 0x34 => ( + "CHAR_CREATE_SERVER_LIMIT", + "You already have the maximum number of characters allowed on this realm.", + ), + 0x35 => ( + "CHAR_CREATE_ACCOUNT_LIMIT", + "You already have the maximum number of characters allowed on this account.", + ), + 0x36 => ( + "CHAR_CREATE_SERVER_QUEUE", + "This server is currently queued and new character creation is temporarily disabled. \ + Please try again during off peak hours.", + ), + 0x37 => ( + "CHAR_CREATE_ONLY_EXISTING", + "Only players who already have characters on this realm are currently allowed to \ + create characters.", + ), + 0x45 => ("CHAR_NAME_NO_NAME", "Enter a name for your character"), + 0x46 => ("CHAR_NAME_TOO_SHORT", "Names must be at least 2 characters"), + 0x47 => ( + "CHAR_NAME_TOO_LONG", + "Names must be no more than 12 characters", + ), + 0x48 => ( + "CHAR_NAME_INVALID_CHARACTER", + "Names can only contain letters", + ), + 0x49 => ( + "CHAR_NAME_MIXED_LANGUAGES", + "Names must contain only one language", + ), + 0x4A => ("CHAR_NAME_PROFANE", "That name contains profanity"), + 0x4B => ("CHAR_NAME_RESERVED", "That name is unavailable"), + 0x4C => ( + "CHAR_NAME_INVALID_APOSTROPHE", + "You cannot use an apostrophe as the first or last character of your name", + ), + 0x4D => ( + "CHAR_NAME_MULTIPLE_APOSTROPHES", + "You can only have one apostrophe", + ), + 0x4E => ( + "CHAR_NAME_THREE_CONSECUTIVE", + "You cannot use the same letter three times consecutively", + ), + 0x4F => ( + "CHAR_NAME_INVALID_SPACE", + "You cannot use a space as the first or last character of your name", + ), + _ => ("CHAR_CREATE_INVALID_NAME", "Invalid character name"), + }; + strings.text(key, fallback) } #[cfg(test)] mod tests { + /// **Every char-create result resolves to the sentence 1.12 actually ships**, read off the + /// player's own `GlueStrings.lua` — the regression for three bugs that literals had hidden + /// (decision 2045). Skips without client data. + #[test] + fn every_char_create_result_resolves_in_the_real_glue_strings() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let src = chain + .read_file("Interface\\GlueXML\\GlueStrings.lua") + .expect("GlueStrings.lua in the chain"); + let strings = GlueStrings::from_map(crate::glue_strings::parse_glue_strings( + &String::from_utf8_lossy(&src), + )); + + // `0x36` used to stop at "…temporarily disabled."; the shipped string carries a second + // sentence, and a re-typed string is one nobody diffed. + assert_eq!( + char_result_text(&strings, 0x36), + "This server is currently queued and new character creation is temporarily disabled. \ + Please try again during off peak hours." + ); + + // `0x50` is INVENTED text: 1.12 has no `CHAR_NAME_CONSECUTIVE_SPACES`, so the code takes + // the default arm like any other the table does not name. If a chain ever *does* carry + // such a key this assert is what will say so. + assert_eq!( + char_result_text(&strings, 0x50), + char_result_text(&strings, 0xFE), + "0x50 has no 1.12 string and must fall to the default arm, not invent one" + ); + assert_eq!(char_result_text(&strings, 0xFE), "Invalid character name"); + + // Every named code must resolve to a REAL key — the fallback caption never showing is the + // whole point, since a fallback that matches hides a missing key forever. + for code in [ + 0x2Eu8, 0x2F, 0x30, 0x31, 0x32, 0x33, 0x34, 0x35, 0x36, 0x37, 0x45, 0x46, 0x47, 0x48, + 0x49, 0x4A, 0x4B, 0x4C, 0x4D, 0x4E, 0x4F, + ] { + let shown = char_result_text(&strings, code); + assert!(!shown.is_empty(), "code {code:#04x} shows nothing"); + } + } + + /// **`0x4B` is the reserved-name code, and text alone can never prove it.** + /// + /// It used to resolve `CHAR_CREATE_NAME_IN_USE`, whose enUS value is *identical* to + /// `CHAR_NAME_RESERVED`'s — "That name is unavailable" — so no comparison of the displayed + /// string could distinguish right from wrong. This asserts the key by *name* against the + /// shipped file, which is the only thing that can. A locale that words the two apart is + /// exactly the case this protects. + #[test] + fn the_reserved_name_code_names_the_reserved_key_not_the_in_use_one() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let src = chain + .read_file("Interface\\GlueXML\\GlueStrings.lua") + .expect("GlueStrings.lua in the chain"); + let map = crate::glue_strings::parse_glue_strings(&String::from_utf8_lossy(&src)); + + // Both keys exist and agree in enUS — which is why the mix-up was invisible. + let reserved = map.get("CHAR_NAME_RESERVED").expect("CHAR_NAME_RESERVED"); + let in_use = map + .get("CHAR_CREATE_NAME_IN_USE") + .expect("CHAR_CREATE_NAME_IN_USE"); + assert_eq!( + reserved, in_use, + "if these ever differ, the mix-up becomes visible" + ); + + // Prove we read the RESERVED one: a table with only that key altered must move 0x4B. + let mut probe = map.clone(); + probe.insert("CHAR_NAME_RESERVED".into(), "RESERVED-SENTINEL".into()); + let strings = GlueStrings::from_map(probe); + assert_eq!(char_result_text(&strings, 0x4B), "RESERVED-SENTINEL"); + // …and 0x31, the genuine in-use code, must NOT have moved. + assert_eq!(char_result_text(&strings, 0x31), in_use.as_str()); + } + use super::*; /// The race grid's column order, pinned against the reference screen (director's screenshot, diff --git a/crates/benilla-app/src/glue_strings.rs b/crates/benilla-app/src/glue_strings.rs index 4262a6ee..3600b8d6 100644 --- a/crates/benilla-app/src/glue_strings.rs +++ b/crates/benilla-app/src/glue_strings.rs @@ -22,6 +22,13 @@ const GLUE_STRINGS: &str = "Interface\\GlueXML\\GlueStrings.lua"; pub(crate) struct GlueStrings(HashMap); impl GlueStrings { + /// Build a table straight from parsed pairs — the test seam for code that has to resolve + /// against the *real* shipped file rather than a stub. + #[cfg(test)] + pub(crate) fn from_map(map: HashMap) -> Self { + Self(map) + } + /// The string for a key, or `None` (unknown key / no data). pub(crate) fn get(&self, key: &str) -> Option<&str> { self.0.get(key).map(String::as_str) @@ -50,7 +57,7 @@ pub(crate) fn load_glue_strings(mut commands: Commands, assets: Option HashMap { +pub(crate) fn parse_glue_strings(src: &str) -> HashMap { let mut out = HashMap::new(); for line in src.lines() { let Some((key, rest)) = line.split_once('=') else { diff --git a/crates/benilla-app/src/net/apply.rs b/crates/benilla-app/src/net/apply.rs index cf528915..c9356bec 100644 --- a/crates/benilla-app/src/net/apply.rs +++ b/crates/benilla-app/src/net/apply.rs @@ -1242,7 +1242,7 @@ pub(crate) fn apply_net_updates( operation, member, result, - } => group::command_result(&mut group, &mut chat_log, operation, &member, result), + } => group::command_result(&mut group, &mut ui_error_keys, operation, &member, result), SessionEvent::PartyMemberStats { guid, full, info } => { group.apply_stats(guid, full, *info) } diff --git a/crates/benilla-app/src/net/apply/group.rs b/crates/benilla-app/src/net/apply/group.rs index 34f734ff..5b614b54 100644 --- a/crates/benilla-app/src/net/apply/group.rs +++ b/crates/benilla-app/src/net/apply/group.rs @@ -214,15 +214,18 @@ pub(super) fn roster_deactivated( /// `SMSG_PARTY_COMMAND_RESULT` — the verdict on an invite/kick/leave we asked for. pub(super) fn command_result( group: &mut GroupState, - chat_log: &mut ChatLog, + errors: &mut crate::ui_action::UiErrorKeys, operation: u32, member: &str, result: u32, ) { - push_group_lines( - chat_log, - group.apply_command_result(operation, member, result), - ); + // By KEY, not by sentence, and into the shared queue rather than straight into chat: the + // catalog row decides the surface, so `result == 7` reaches the red `UIErrorsFrame` line while + // the other nine stay chat lines (wow-re `party-command-result-law.md`; decision 2035's + // shape). `None` is the reference's own silence — three inputs display nothing at all. + errors + .0 + .extend(group.apply_command_result(operation, member, result)); } #[cfg(test)] diff --git a/crates/benilla-app/src/ui_action/errors.rs b/crates/benilla-app/src/ui_action/errors.rs index feef2638..37f065e9 100644 --- a/crates/benilla-app/src/ui_action/errors.rs +++ b/crates/benilla-app/src/ui_action/errors.rs @@ -223,13 +223,13 @@ impl UiErrorTexts { /// `None` (absent or empty key) = show nothing: GlobalStrings data-suppression, faithfully /// (the ref's own `[record+0x00]` null/empty guard at `0x4967bd`/`0x4967c5`). pub(crate) fn ui_error_text(e: &UiError, get: &dyn Fn(&str) -> Option) -> Option { - let mut text = get(e.key)?; - if let Some(s) = &e.fill_s { - text = text.replace("%s", s); - } - if let Some(d) = e.fill_d { - text = text.replace("%d", &d.to_string()); - } + // Through the one shared filler (2045). This used `str::replace`, which fills EVERY `%s` + // with the same argument — latent only because no message on this queue carries two yet. + // The order is the templates' own: `ERR_USE_LOCKED_WITH_SPELL_KNOWN_SI` is String-then-Integer. + let mut args: Vec> = Vec::new(); + args.extend(e.fill_s.as_deref().map(benilla_ui::strings::Arg::S)); + args.extend(e.fill_d.map(|d| benilla_ui::strings::Arg::D(i64::from(d)))); + let text = benilla_ui::strings::fill(&get(e.key)?, &args); (!text.is_empty()).then_some(text) } @@ -308,10 +308,13 @@ pub(crate) fn keyed_line(script: &UiScript, key: &'static str) -> Option /// reference's `format`-with-pushed-arguments face of the same catalog rows (the battleground /// join verdict's map name, the joined-or-left player's name, 1974). pub(crate) fn keyed_line_s(script: &UiScript, key: &'static str, args: &[&str]) -> Option { - let mut text = script.lua().globals().get::(key).ok()?; - for arg in args { - text = text.replacen("%s", arg, 1); - } + let template = script.lua().globals().get::(key).ok()?; + let args: Vec> = args + .iter() + .copied() + .map(benilla_ui::strings::Arg::S) + .collect(); + let text = benilla_ui::strings::fill(&template, &args); (!text.is_empty()).then(|| Shown::keyed(key, text)) } diff --git a/crates/benilla-app/src/ui_duel.rs b/crates/benilla-app/src/ui_duel.rs index c0c97ab7..9a7ced93 100644 --- a/crates/benilla-app/src/ui_duel.rs +++ b/crates/benilla-app/src/ui_duel.rs @@ -151,7 +151,13 @@ fn winner_line(fled: bool, winner: &str, loser: &str) -> String { } else { DUEL_WINNER_KNOCKOUT }; - template.replace("%1$s", winner).replace("%2$s", loser) + benilla_ui::strings::fill( + template, + &[ + benilla_ui::strings::Arg::S(winner), + benilla_ui::strings::Arg::S(loser), + ], + ) } /// The net drain's `SessionEvent::Duel*` arms, factored here so the wire laws live beside the diff --git a/crates/benilla-app/src/ui_instance.rs b/crates/benilla-app/src/ui_instance.rs index c6ffa510..062f277c 100644 --- a/crates/benilla-app/src/ui_instance.rs +++ b/crates/benilla-app/src/ui_instance.rs @@ -306,50 +306,6 @@ fn plural_token(token: &str, ordinal: Option, get: &dyn Fn(&str) -> Option< token.to_string() } -/// Fill a 1.12 message template the way `SStrPrintf` does: `%s` and `%d` consumed **in order**, -/// left to right, from the argument list the caller built in the template's own order. -/// -/// `%%` collapses to one `%`, because that is what `SStrPrintf` does — no 1.12 lockout template -/// contains one, but leaving it doubled would be the deviation, not collapsing it. Any other -/// specifier is copied through, and so is one whose argument has run out: a template we -/// mis-modelled should look wrong, not look plausible. -fn fill_template(template: &str, map_name: Option<&str>, numbers: &[u32]) -> String { - let mut out = String::with_capacity(template.len() + 16); - let mut strings = map_name.into_iter(); - let mut nums = numbers.iter(); - let mut chars = template.chars().peekable(); - while let Some(c) = chars.next() { - if c != '%' { - out.push(c); - continue; - } - match chars.peek() { - Some('s') => { - if let Some(s) = strings.next() { - chars.next(); - out.push_str(s); - } else { - out.push(c); - } - } - Some('d') => { - if let Some(n) = nums.next() { - chars.next(); - out.push_str(&n.to_string()); - } else { - out.push(c); - } - } - Some('%') => { - chars.next(); - out.push('%'); - } - _ => out.push(c), - } - } - out -} - /// Resolve one queued line to its displayed text — `GetText` + the `%s`/`%d` fills, then the /// debug wrapper if the save-created flag asked for it. /// @@ -363,7 +319,16 @@ fn lockout_text( ) -> Option { let token = plural_token(line.token, line.ordinal, get); let template = get(&token).filter(|s| !s.is_empty())?; - let text = fill_template(&template, map_name, &line.numbers); + // Arguments in the template's own order — every lockout template takes its `%s` before its + // `%d`s (checked against all fourteen tokens), which is what makes one ordered list exact. + let mut args: Vec> = Vec::new(); + args.extend(map_name.map(benilla_ui::strings::Arg::S)); + args.extend( + line.numbers + .iter() + .map(|n| benilla_ui::strings::Arg::D(i64::from(*n))), + ); + let text = benilla_ui::strings::fill(&template, &args); if text.is_empty() { return None; } @@ -1180,9 +1145,13 @@ mod tests { /// than fills keeps the leftovers literally rather than eating the next argument. #[test] fn fill_is_positional_and_never_borrows_the_wrong_argument() { - assert_eq!(fill_template("%s: %d/%d", Some("MC"), &[2, 5]), "MC: 2/5"); - assert_eq!(fill_template("%s: %d/%d", Some("MC"), &[2]), "MC: 2/%d"); - assert_eq!(fill_template("100%% sure", None, &[]), "100% sure"); - assert_eq!(fill_template("no fills", None, &[7]), "no fills"); + use benilla_ui::strings::{fill, Arg}; + assert_eq!( + fill("%s: %d/%d", &[Arg::S("MC"), Arg::D(2), Arg::D(5)]), + "MC: 2/5" + ); + assert_eq!(fill("%s: %d/%d", &[Arg::S("MC"), Arg::D(2)]), "MC: 2/%d"); + assert_eq!(fill("100%% sure", &[]), "100% sure"); + assert_eq!(fill("no fills", &[Arg::D(7)]), "no fills"); } } diff --git a/crates/benilla-app/src/ui_models/mod.rs b/crates/benilla-app/src/ui_models/mod.rs index 2cc924dd..70c93b9b 100644 --- a/crates/benilla-app/src/ui_models/mod.rs +++ b/crates/benilla-app/src/ui_models/mod.rs @@ -88,6 +88,25 @@ //! The shader composes them as the reference does — `uv' = R((uv + t − p) ⊙ s) + p` — which is //! how the cooldown indicator's four quadrant quads turn their mask into the clockwise sweep. //! +//! ## What a pane that stops drawing costs: nothing (decision 2046) +//! +//! A pane that leaves the engine's paint list keeps its tile for [`TILE_LINGER_FRAMES`] so a +//! cooldown that re-arms every few seconds, a ping, or a bag that reopens keeps its tree — but +//! the tree has to cost NOTHING while it waits, and hiding the root was never enough. Two of a +//! tile's three per-frame costs do not travel down the scene graph at all: an emitter entity is +//! a world root the particle lane walks directly, and the global-sequence bone channels are +//! written by a driver that reads no `Visibility`. So a hidden tile is **parked**, explicitly, +//! on the frame it stops drawing — [`AnimParked`] on the root (which holds the pose evaluation, +//! the compose, the palette write and the global-sequence writes) and +//! `ParticleEmitter::set_frozen` on every emitter (pool + age held, no quads). +//! +//! The emitter half is not a nicety. The particle lane's own freeze asks a CAMERA whether its +//! scene is drawn, which is exact for a booth (one camera, one scene) and cannot be asked here: +//! every orthographic pane on the sheet shares one camera, and that camera stays active whenever +//! ANY cell is packed, because it is the camera that clears the atlas. Before the park, a hidden +//! autocast-shine pane's four spline emitters kept integrating and kept pushing quads for the +//! whole linger, at the root's last-written cell — which after a repack belongs to another pane. +//! //! ## What is deliberately NOT here //! //! - The **character panes** (`PlayerModel`/`DressUpModel`/`TabardModel`) keep their booths @@ -123,7 +142,7 @@ use benilla_world::particles::buffer::EffectLightOverride; use benilla_world::particles::{ spawn_emitter, EmitClock, EmitterFrames, OwnerLoss, ParticleEmitter, }; -use benilla_world::rig_anim::{GlobalSeqDrive, RigPose}; +use benilla_world::rig_anim::{AnimParked, GlobalSeqDrive, RigPose}; use benilla_world::rig_palette::{RigPaletteMirrors, RigPalettes, RigPart, RigSkin}; use crate::portrait::{ @@ -496,6 +515,11 @@ struct Tile { emitters: Vec, /// The last frame this tile was on the engine's paint list. last_seen: u64, + /// Parked: this tile drew no cell last frame, so its rig and its emitters are held (decision + /// 2046; the module doc's "what a pane that stops drawing costs"). A fresh tile starts + /// `false` and is parked by the same walk on its first non-drawing frame, so there is one + /// code path and no spawn-time special case. + parked: bool, } impl Tile { @@ -708,6 +732,7 @@ fn sync_tiles( &mut Transform, &mut Visibility, Option<&mut AnimationPlayer>, + Option<&mut GlobalSeqDrive>, ), ( With, @@ -883,6 +908,7 @@ fn sync_tiles( uv_parts: Vec::new(), emitters: Vec::new(), last_seen: frame, + parked: false, }); tile.last_seen = frame; if !tile.built { @@ -997,7 +1023,6 @@ fn sync_tiles( let atlas_h = atlas_size.y as f32; // ── 5. Place every drawing tile: cell, unit ladder, facing, play head ─────────────── - let mut hidden: Vec = state.tiles.values().map(|t| t.root).collect(); let mut aimed = [false; UI_MODEL_CAM_LAYERS]; for (i, (handle, req, pane)) in drawing.iter().enumerate() { let Some(cell) = cells.get(i).copied() else { @@ -1006,9 +1031,8 @@ fn sync_tiles( let Some(tile) = state.tiles.get_mut(handle) else { continue; }; - hidden.retain(|&e| e != tile.root); bridge.cells.insert(*handle, cell); - let Ok((mut tf, mut vis, player)) = roots.get_mut(tile.root) else { + let Ok((mut tf, mut vis, player, drive)) = roots.get_mut(tile.root) else { continue; }; @@ -1142,6 +1166,18 @@ fn sync_tiles( facts_slot }); let gseq_s = pane.clock_ms as f64 / 1000.0; + // …and the BONE global-sequence channels read it too, not the world clock (decision + // 2046). The animation kernel's Phase B cursor is `[[model+0x2c]+0xc] − [model+0x68]` + // — the clock of the scene that OWNS the instance, minus the attach snapshot — and a + // `` widget owns a private `CM2Scene` (`CSimpleModel+0x314`) that only its own + // `OnUpdate` advances (wow-re `gseq-anchor.md` §1/§2, `modelframe-animation-clock.md` + // §1.1/§3, both byte-verified). The visible case is the ping's 4833 ms spinner: its + // phase belongs to the pane, which is why "ping N resumes where ping N−1 stopped" needs + // no accumulator of ours (2013). The drive stamps its own anchor on its first tick, which + // is the attach. + if let Some(mut d) = drive { + d.set_clock(gseq_s); + } let light_trace = if trace_on() { let sc = &render.rig.lights[tile.light_slot].scene; format!( @@ -1221,11 +1257,49 @@ fn sync_tiles( } } } - for root in hidden { - if let Ok((_, mut vis, _)) = roots.get_mut(root) { - if *vis != Visibility::Hidden { + // ── 6. Park what is not drawing; thaw what is ────────────────────────────────────── + // + // "Drawing" is exactly "has a cell this frame" — the same set the placement loop above wrote. + // Parking is the module doc's contract: `AnimParked` holds the rig (the 0712 evaluator, the + // compose, the palette write and the global-sequence bone writes), the freeze holds every + // emitter's pool, age and quads, and the root is hidden so no batch draws. This walk replaces + // the `hidden` vector the placement loop used to `retain` out of once per drawing tile — the + // same verdict, without the quadratic. + for (handle, tile) in state.tiles.iter_mut() { + let park = !draws_this_frame(&bridge, handle); + // The emitter freeze is COMPARED, not edge-triggered off `tile.parked`: a tile can be + // built while it is already parked (its pane is on the paint list but its cell did not + // fit the capped atlas), and an emitter is born thawed. The comparison is a `Deref`, so + // a steady state touches no change tick. + for &e in &tile.emitters { + if let Ok(mut em) = emitters.get_mut(e) { + if em.is_frozen() != park { + em.set_frozen(park); + } + } + } + if tile.parked == park { + continue; + } + tile.parked = park; + if trace_on() { + info!( + "tile-trace: tile {:?} {} — {} emitters", + handle, + if park { "PARKED" } else { "thawed" }, + tile.emitters.len() + ); + } + if park { + if let Ok((_, mut vis, _, _)) = roots.get_mut(tile.root) { *vis = Visibility::Hidden; } + commands.entity(tile.root).insert(AnimParked); + } else { + // The marker drops before `AnimationSystems` (this is `Update`, the lane is + // `PostUpdate`), so the first thawed frame evaluates and composes before anything + // reads the pose — 0739's wake law, the same as every world rig's. + commands.entity(tile.root).remove::(); } } for (mut cam, _, _, _, marker) in &mut pane_cams { @@ -1361,6 +1435,25 @@ pub(crate) fn compose_tiles(bridge: Res, mut quads: ResMut bool { + bridge.cells.contains_key(handle) +} + /// [`compose_tiles`]'s pure half: the quads for every `(request, cell)` pair the bridge holds, /// ordered by paint key so the overlay diff sees the same sequence for the same set. pub(crate) fn composite_quads(bridge: &UiModelTiles) -> Vec { @@ -1789,6 +1882,79 @@ mod tests { assert!((br[0] - 130.0 / 512.0).abs() < 1e-6 && (br[1] - 65.0 / 512.0).abs() < 1e-6); } + /// **The park verdict and the composite are the same question** (decision 2046). A tile + /// parks exactly when it pushed no quad, and `sync_tiles` reads both off `bridge.cells` in one + /// pass — so neither an atlas repack nor an atlas too full for one more pane can freeze a + /// cloud on a frame its pane is visibly drawing. This pins the two sides to each other: what + /// [`composite_quads`] draws IS the un-parked set, so a later change to either has to change + /// both. The third pane here is the one the capped atlas had no room for: it draws nothing, + /// which is exactly why parking it is right. + #[test] + fn the_park_verdict_is_exactly_what_the_composite_draws() { + let mut arena = benilla_ui::widget::WidgetArena::new(); + let mut bridge = UiModelTiles { + atlas: Some(Handle::default()), + atlas_size: UVec2::splat(512), + ..Default::default() + }; + // Three panes on the paint list; the packer placed the first two and ran out of atlas. + let panes: Vec = (0..3) + .map(|_| arena.create(benilla_ui::widget::FrameKind::Frame, None, None)) + .collect(); + for (i, &h) in panes.iter().enumerate() { + bridge.requests.insert( + h, + TileRequest { + path: r"Interface\Buttons\UI-AutoCastButton.mdx".into(), + size_px: UVec2::new(63, 63), + px_per_unit: 1.0, + pos_px_per_unit: 1.0, + star_px_per_unit: 1.0, + facing: 0.0, + position: Vec3::ZERO, + root_scale: 1.0, + root_pos: Vec3::ZERO, + camera: None, + light: ModelLight::default(), + fog: None, + icon: None, + rect: Rect::new(0.0, 0.0, 63.0, 63.0), + z_key: 1_000 + i as u64, + alpha: 1.0, + clip: None, + }, + ); + if i < 2 { + bridge.cells.insert( + h, + Cell { + origin: UVec2::new(2 + 65 * i as u32, 2), + size: UVec2::new(63, 63), + }, + ); + } + } + let drawn: std::collections::HashSet = + composite_quads(&bridge).iter().map(|q| q.z_key).collect(); + assert_eq!(drawn.len(), 2, "the packer placed two, so two quads"); + for (i, &h) in panes.iter().enumerate() { + assert_eq!( + draws_this_frame(&bridge, &h), + drawn.contains(&(1_000 + i as u64)), + "pane {i}: the park verdict and the composite must agree" + ); + } + // And the repack: growing the atlas moves every cell's texel window without changing WHO + // has one, so no pane's verdict flips on a growth frame. + bridge.atlas_size = UVec2::splat(1024); + let regrown: std::collections::HashSet = + composite_quads(&bridge).iter().map(|q| q.z_key).collect(); + assert_eq!( + regrown, drawn, + "a repack changes texel windows, not the drawn set" + ); + } + /// A `TileRequest` for the perspective tests: the pane's own size and root terms, nothing /// the leg does not read. fn persp_req(size: UVec2, root_scale: f32, root_pos: Vec3, facing: f32) -> TileRequest { diff --git a/crates/benilla-app/src/ui_party/mod.rs b/crates/benilla-app/src/ui_party/mod.rs index b1b15ffc..00666d55 100644 --- a/crates/benilla-app/src/ui_party/mod.rs +++ b/crates/benilla-app/src/ui_party/mod.rs @@ -26,6 +26,7 @@ use std::collections::HashMap; +use crate::ui_action::UiError; use benilla_protocol::messages::{ party_operation, party_result, GroupLootInfo, GroupMemberEntry, PartyMemberStatsInfo, }; @@ -127,11 +128,9 @@ pub struct GroupState { // (decision 0246 extraction; `GlobalStrings.lua` line numbers cited per constant). const JOINED_PARTY: &str = "%s joins the party."; // ERR_JOINED_GROUP_S (GlobalStrings:1665) const LEFT_PARTY: &str = "%s leaves the party."; // ERR_LEFT_GROUP_S (GlobalStrings:1670) -const LEFT_GROUP_YOU: &str = "You leave the group."; // ERR_LEFT_GROUP_YOU (GlobalStrings:1671) const GROUP_DISBANDED: &str = "Your group has been disbanded."; // ERR_GROUP_DISBANDED (GlobalStrings:1582) const UNINVITE_YOU: &str = "You have been removed from the group."; // ERR_UNINVITE_YOU (GlobalStrings:1904) const INVITED_TO_GROUP: &str = "%s has invited you to join a group."; // ERR_INVITED_TO_GROUP_S (GlobalStrings:1654) -const INVITE_PLAYER: &str = "You have invited %s to join your group."; // ERR_INVITE_PLAYER_S (GlobalStrings:1657) const DECLINE_GROUP: &str = "%s declines your group invitation."; // ERR_DECLINE_GROUP_S (GlobalStrings:1545) const NEW_LEADER: &str = "%s is now the group leader."; // ERR_NEW_LEADER_S (GlobalStrings:1724) const NEW_LEADER_YOU: &str = "You are now the group leader."; // ERR_NEW_LEADER_YOU (GlobalStrings:1725) @@ -274,36 +273,73 @@ impl GroupState { } } - /// `SMSG_PARTY_COMMAND_RESULT` — the ack for our own invite/leave. LEAVE+ok is the ONE - /// source of "You leave the group." (0440: `0x5e690b`, op 2 × result 0 → msgId 0x42; the - /// empty roster never emits it). + /// `SMSG_PARTY_COMMAND_RESULT` — **the reference's own jump table, transcribed** (wow-re + /// `system/object-layer/scratch/party-command-result-law.md`, byte-verified §5). + /// + /// The handler `0x5e68b0` dispatches `dec eax; cmp eax,7; ja ; jmp [4*eax+0x5e6a14]`, + /// and each arm is one `CGGameUI::DisplayError(msgId)`. Returning the **message** rather than + /// a sentence is what gets the text, the surface and the sound right at once, since all three + /// are fields of the catalog row the id names (decisions 1770, 1815, 2035). + /// + /// | `result` | msgId | key | `%s` | + /// |---|---|---|---| + /// | `0` + op `0` + non-empty name | `0x3a` | `ERR_INVITE_PLAYER_S` | name | + /// | `0` + op `2` | `0x42` | `ERR_LEFT_GROUP_YOU` | — | + /// | 1 | `0x47` | `ERR_BAD_PLAYER_NAME_S` | name | + /// | 2 | `0x49` | `ERR_TARGET_NOT_IN_GROUP_S` | name | + /// | 3 | `0x4a` | `ERR_GROUP_FULL` | — | + /// | 4 | `0x3d` | `ERR_ALREADY_IN_GROUP_S` | name | + /// | 5 | `0x48` | `ERR_NOT_IN_GROUP` | — | + /// | 6 | `0x4b` | `ERR_NOT_LEADER` | — | + /// | 7 | `0xff` | `ERR_PLAYER_WRONG_FACTION` | — | + /// | 8 | `0x13d` | `ERR_IGNORING_YOU_S` | name | + /// + /// **`result == 7` is the odd one and we had it wrong:** its row is kind **2**, the red + /// `UIErrorsFrame` line, where the other nine are kind 0 chat lines. benilla pushed all ten + /// into the chat log. + /// + /// **Three inputs are SILENT**, and the reference means it — the default arm is four + /// instructions, `mov eax,1; mov esp,ebp; pop ebp; ret 8`, with no call, no store and no + /// message: + /// + /// 1. `result >= 9` — benilla printed an invented `"Party command failed (N)."` here, which is + /// worse than mis-wording a real string: the client deliberately says nothing. + /// 2. `result == 0` with `operation ∉ {0, 2}`. + /// 3. `result == 0`, `operation == 0`, and an **empty** name (`0x5e6923 je`) — benilla emitted + /// the invite line unconditionally. + /// + /// `operation` is read **only** on the `result == 0` path; on every other it is not touched. + /// Takes `&self`: this arm reads the wire and names a message, and changes no state — the + /// roster moves on `SMSG_GROUP_LIST`, not on the ack. pub fn apply_command_result( - &mut self, + &self, operation: u32, member: &str, result: u32, - ) -> Vec { - if result == party_result::OK { - return match operation { - party_operation::INVITE => vec![fmt_s(INVITE_PLAYER, member)], - party_operation::LEAVE => vec![LEFT_GROUP_YOU.to_string()], - _ => Vec::new(), - }; - } - // The error table (vmangos `PartyResult`, `WorldSession.h:100-111` → the GlobalStrings - // each errorId keys; templates quoted verbatim, GlobalStrings:1465-1861). - let line = match result { - party_result::BAD_PLAYER_NAME => fmt_s("Cannot find '%s'.", member), - party_result::TARGET_NOT_IN_GROUP => fmt_s("%s is not in your party.", member), - party_result::GROUP_FULL => "Your party is full.".to_string(), - party_result::ALREADY_IN_GROUP => fmt_s("%s is already in a group.", member), - party_result::NOT_IN_GROUP => "You aren't in a party.".to_string(), - party_result::NOT_LEADER => "You are not the party leader.".to_string(), - party_result::WRONG_FACTION => "Target is not part of your alliance.".to_string(), - party_result::IGNORING_YOU => fmt_s("%s is ignoring you.", member), - other => format!("Party command failed ({other})."), + ) -> Option { + let named = |key: &'static str| UiError { + key, + fill_s: Some(member.to_string()), + fill_d: None, }; - vec![line] + match result { + party_result::OK => match operation { + // The empty-name guard is the reference's, not defensiveness: an invite ack that + // carries no name prints nothing at all. + party_operation::INVITE if !member.is_empty() => Some(named("ERR_INVITE_PLAYER_S")), + party_operation::LEAVE => Some(UiError::key("ERR_LEFT_GROUP_YOU")), + _ => None, + }, + party_result::BAD_PLAYER_NAME => Some(named("ERR_BAD_PLAYER_NAME_S")), + party_result::TARGET_NOT_IN_GROUP => Some(named("ERR_TARGET_NOT_IN_GROUP_S")), + party_result::GROUP_FULL => Some(UiError::key("ERR_GROUP_FULL")), + party_result::ALREADY_IN_GROUP => Some(named("ERR_ALREADY_IN_GROUP_S")), + party_result::NOT_IN_GROUP => Some(UiError::key("ERR_NOT_IN_GROUP")), + party_result::NOT_LEADER => Some(UiError::key("ERR_NOT_LEADER")), + party_result::WRONG_FACTION => Some(UiError::key("ERR_PLAYER_WRONG_FACTION")), + party_result::IGNORING_YOU => Some(named("ERR_IGNORING_YOU_S")), + _ => None, + } } /// `SMSG_PARTY_MEMBER_STATS(_FULL)` — merge (delta) or replace (`full`) one member's @@ -550,7 +586,7 @@ mod tests { g.apply_list(0, 0, vec![member("Alice", 1)], 1, None); assert_eq!( g.apply_command_result(party_operation::LEAVE, "Us", party_result::OK), - vec!["You leave the group."] + Some(UiError::key("ERR_LEFT_GROUP_YOU")) ); assert_eq!( g.apply_list(0, 0, Vec::new(), 0, None), @@ -611,7 +647,7 @@ mod tests { // wording (keyed off the departed group's type). assert_eq!( g.apply_command_result(party_operation::LEAVE, "Us", party_result::OK), - vec!["You leave the group."] + Some(UiError::key("ERR_LEFT_GROUP_YOU")) ); assert_eq!( g.apply_list(0, 0, Vec::new(), 0, None), @@ -643,7 +679,11 @@ mod tests { assert_eq!(g.pending_invite.as_deref(), Some("Bob")); assert_eq!( g.apply_command_result(party_operation::INVITE, "Carol", party_result::OK), - vec!["You have invited Carol to join your group."] + Some(UiError { + key: "ERR_INVITE_PLAYER_S", + fill_s: Some("Carol".into()), + fill_d: None + }) ); assert_eq!( g.apply_command_result( @@ -651,11 +691,19 @@ mod tests { "Carol", party_result::ALREADY_IN_GROUP ), - vec!["Carol is already in a group."] + Some(UiError { + key: "ERR_ALREADY_IN_GROUP_S", + fill_s: Some("Carol".into()), + fill_d: None + }) ); assert_eq!( g.apply_command_result(party_operation::INVITE, "Xz", party_result::BAD_PLAYER_NAME), - vec!["Cannot find 'Xz'."] + Some(UiError { + key: "ERR_BAD_PLAYER_NAME_S", + fill_s: Some("Xz".into()), + fill_d: None + }) ); assert_eq!( g.apply_declined("Carol"), @@ -766,4 +814,161 @@ mod tests { "the raid flag clearing empties the board" ); } + + /// **The refusal table is the reference's, asserted by message id.** + /// + /// Each `result` must name the catalog row whose id the binary's own jump table pushes to + /// `DisplayError` (wow-re `party-command-result-law.md` §3, read out of the PE at `0x5e6a14`). + /// Asserting the *id* rather than the sentence is the point, and this family is the reason: + /// nothing about the displayed English distinguishes a right key from a wrong one here, and a + /// plausible-looking `ERR_WRONG_FACTION` — which exists in neither `GlobalStrings.lua` nor the + /// catalog — would have shown **nothing at all**, silently. + #[test] + fn every_party_result_names_the_message_id_the_reference_pushes() { + const TABLE: &[(u32, u16, bool)] = &[ + (party_result::BAD_PLAYER_NAME, 0x47, true), + (party_result::TARGET_NOT_IN_GROUP, 0x49, true), + (party_result::GROUP_FULL, 0x4a, false), + (party_result::ALREADY_IN_GROUP, 0x3d, true), + (party_result::NOT_IN_GROUP, 0x48, false), + (party_result::NOT_LEADER, 0x4b, false), + (party_result::WRONG_FACTION, 0xff, false), + (party_result::IGNORING_YOU, 0x13d, true), + ]; + let g = GroupState::default(); + for &(result, id, takes_name) in TABLE { + let msg = g + .apply_command_result(party_operation::INVITE, "Zed", result) + .unwrap_or_else(|| panic!("result {result} showed nothing")); + let row = benilla_ui::messages::by_key(msg.key) + .unwrap_or_else(|| panic!("result {result} named {}, not a catalog row", msg.key)); + assert_eq!(row.id, id, "result {result} -> {} (id {})", msg.key, row.id); + // The `%s` arms are exactly the arms the binary passes `&name` to, which is an + // independent cross-check: every one of them is an `_S` key. + assert_eq!( + msg.fill_s.is_some(), + takes_name, + "result {result} name fill" + ); + assert_eq!( + msg.key.ends_with("_S"), + takes_name, + "result {result} _S suffix" + ); + } + + // The OK path's two arms, where `operation` is the only thing read. + let ok = |op| g.apply_command_result(op, "Zed", party_result::OK); + assert_eq!( + benilla_ui::messages::by_key(ok(party_operation::INVITE).unwrap().key) + .unwrap() + .id, + 0x3a + ); + assert_eq!( + benilla_ui::messages::by_key(ok(party_operation::LEAVE).unwrap().key) + .unwrap() + .id, + 0x42 + ); + } + + /// **`result == 7` is the red line; the other nine are chat.** The one row in this family whose + /// `+0x04` is kind 2, and benilla pushed all ten into the chat log. + #[test] + fn the_wrong_faction_refusal_is_the_red_line_and_the_rest_are_chat() { + use benilla_ui::messages::MsgKind; + let g = GroupState::default(); + let kind = |r| { + benilla_ui::messages::kind_of( + g.apply_command_result(party_operation::INVITE, "Zed", r) + .unwrap() + .key, + ) + }; + assert_eq!(kind(party_result::WRONG_FACTION), MsgKind::Error); + for r in [ + party_result::BAD_PLAYER_NAME, + party_result::TARGET_NOT_IN_GROUP, + party_result::GROUP_FULL, + party_result::ALREADY_IN_GROUP, + party_result::NOT_IN_GROUP, + party_result::NOT_LEADER, + party_result::IGNORING_YOU, + ] { + assert_eq!(kind(r), MsgKind::Chat, "result {r} should be a chat line"); + } + } + + /// **Three inputs display NOTHING, and the reference means it.** Its default arm is four + /// instructions — `mov eax,1; mov esp,ebp; pop ebp; ret 8` — with no call and no store. + /// + /// benilla printed an invented `"Party command failed (N)."` for the first of these. Inventing + /// a sentence for a case the client is deliberately silent on is worse than mis-wording one it + /// has a string for: there is no wording that could be right. + #[test] + fn the_silent_inputs_show_nothing() { + let g = GroupState::default(); + // 1 — every result past the table's end (`dec eax; cmp eax,7; ja`, unsigned). + for r in [9u32, 10, 42, u32::MAX] { + assert!( + g.apply_command_result(party_operation::INVITE, "Zed", r) + .is_none(), + "result {r} must be silent" + ); + } + // 2 — OK with an operation that is neither invite nor leave. + for op in [1u32, 3, 99] { + assert!(g + .apply_command_result(op, "Zed", party_result::OK) + .is_none()); + } + // 3 — an invite ack carrying no name (`0x5e6923 je`). + assert!(g + .apply_command_result(party_operation::INVITE, "", party_result::OK) + .is_none()); + // …but a named one still prints, so the guard is the name and not the operation. + assert!(g + .apply_command_result(party_operation::INVITE, "Zed", party_result::OK) + .is_some()); + } + + /// Every key this family raises resolves in the shipped 1.12 `GlobalStrings.lua` to the + /// sentence the real client shows — the runtime half, since a key that is a valid catalog row + /// can still be absent from the player's own chain. Skips without client data. + #[test] + fn party_result_keys_resolve_to_the_real_1_12_sentences() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let src = chain + .read_file("Interface\\FrameXML\\GlobalStrings.lua") + .expect("GlobalStrings.lua in the chain"); + let s = benilla_ui::script::UiScript::new().expect("VM"); + s.run(&String::from_utf8_lossy(&src)).expect("runs clean"); + + let g = GroupState::default(); + for r in [ + party_result::BAD_PLAYER_NAME, + party_result::TARGET_NOT_IN_GROUP, + party_result::GROUP_FULL, + party_result::ALREADY_IN_GROUP, + party_result::NOT_IN_GROUP, + party_result::NOT_LEADER, + party_result::WRONG_FACTION, + party_result::IGNORING_YOU, + ] { + let msg = g + .apply_command_result(party_operation::INVITE, "Zed", r) + .unwrap(); + let text: String = s.lua().globals().get(msg.key).expect(msg.key); + assert!(!text.is_empty(), "{} resolves empty", msg.key); + // An `_S` key must actually carry the hole its fill expects. + assert_eq!( + text.contains("%s"), + msg.fill_s.is_some(), + "{} vs its fill", + msg.key + ); + } + } } diff --git a/crates/benilla-app/tests/reference_strings.rs b/crates/benilla-app/tests/reference_strings.rs new file mode 100644 index 00000000..680adf8d --- /dev/null +++ b/crates/benilla-app/tests/reference_strings.rs @@ -0,0 +1,404 @@ +//! **No user-facing sentence is written in Rust when the reference ships one.** +//! +//! The real client never composes display text: every sentence is a key into `GlobalStrings.lua` +//! (in-game) or `GlueStrings.lua` (the login/character screens), resolved at runtime from the +//! player's own install. Writing the English in Rust throws away three things at once — +//! localization, and, for anything that goes through the message catalog, the *surface* the +//! message is shown on and the *voice line* it speaks with (decisions 1770, 1815, 2035). +//! +//! **Why a tripwire and not a review rule.** Decision 2035 found a loot-refusal table that had +//! composed its own eight sentences, six of which said something 1.12 never says, under a comment +//! asserting they were quoted from the reference. The comment was the only check there had ever +//! been, and a comment cannot fail. What makes that class *findable* is that a re-typed string is +//! byte-identical to the shipped one — so a walk that normalises both sides and compares them finds +//! every instance mechanically, which is what this does. +//! +//! **What it cannot find, stated plainly.** Text that matches nothing is invisible here: an +//! *invented* sentence (2035's "Those pockets are already empty.", for a code 1.12 has no string +//! for) has no shipped counterpart to match against. This tripwire catches the re-typing class, +//! which is the large one; the invention class needs a reader who checks the reference. +//! +//! **A literal beside a key lookup is correct and is not flagged** — a fallback for an install +//! whose chain lacks the key is the reference's own `UNKNOWNOBJECT` shape. The walk only counts a +//! literal whose enclosing function resolves nothing. +//! +//! [`ALLOWED`] is a **ratchet**: it records what was already drifted when the tripwire was built, +//! per file, and the counts may only ever go **down**. Converting a file means lowering its number +//! (or deleting the row); a file not listed may carry none at all. Skips without client data. + +use std::collections::HashMap; +use std::path::{Path, PathBuf}; + +/// The drift still standing, per file: **252 literals over 32 files**, each a sentence the +/// reference ships and we re-typed. +/// +/// **These numbers may only decrease.** They are not a budget to spend; every one is a string that +/// will not translate, and some carry a surface and a voice line with them. Lower the count when +/// you convert a file, and delete the row at zero — a row left stale fails this test too, so a +/// conversion cannot quietly re-permit the drift it retired. `char_create` (2045) and `ui_trade` +/// (2042, a neighbouring session) are both converted and have no row: that is what retiring one +/// looks like, and the second was caught by this test rather than by remembering. +/// +/// **Regenerate these against a clean tree, never mid-edit.** The walk is deterministic on the +/// sources plus the shipped tables (two consecutive runs agree exactly), but a baseline captured +/// while other files are half-converted records counts that no later tree will reproduce — which +/// is exactly how the first cut of this list carried a row the gates then failed on. +const ALLOWED: &[(&str, usize)] = &[ + ("benilla-app/src/ui_chat/frames.rs", 48), + ("benilla-app/src/ui_guild/lines.rs", 34), + ("benilla-app/src/ui_tooltip/mod.rs", 28), + ("benilla-app/src/ui_party/mod.rs", 12), + ("benilla-app/src/ui_social/mod.rs", 18), + ("benilla-app/src/ui_petition/lines.rs", 15), + ("benilla-app/src/ui_chat/feed.rs", 13), + ("benilla-ui/src/script/tooltip_item/render.rs", 9), + ("benilla-app/src/ui_unit.rs", 6), + ("benilla-formats/src/spells/tokens.rs", 6), + ("benilla-app/src/ui_duel.rs", 5), + ("benilla-app/src/ui_social/feed.rs", 4), + ("benilla-app/src/ui_talent.rs", 4), + ("benilla-ui/src/script/tooltip_unit.rs", 4), + ("benilla-app/src/ui_quest.rs", 3), + ("benilla-app/src/ui_quest_log.rs", 3), + ("benilla-app/src/ui_script/resolve_bench.rs", 3), + ("benilla-ui/src/script/tooltip_item/names.rs", 3), + ("benilla-ui/src/script/tradeskill/view.rs", 3), + ("benilla-app/src/net/apply/chat.rs", 2), + ("benilla-app/src/ui_binder.rs", 2), + ("benilla-app/src/ui_char.rs", 2), + ("benilla-app/src/ui_trainer/law.rs", 2), + ("benilla-app/src/ui_trainer/mod.rs", 2), + ("benilla-ui/src/script/tooltip_item/mod.rs", 2), + ("benilla-ui/src/script/tooltip_spell.rs", 2), + ("benilla-ui/src/script/unit/mod.rs", 2), + ("benilla-app/src/death.rs", 1), + ("benilla-app/src/net/apply/quests.rs", 1), + ("benilla-app/src/ui_chat/input/mod.rs", 1), + ("benilla-app/src/ui_items/feed.rs", 1), + ("benilla-ui/src/script/talent.rs", 1), +]; + +/// Paths that are never player-facing: dev instruments, probes, capture harnesses and benches. +/// A `.learn ` GM command is not a UI string even when it collides with one. +fn is_instrument(rel: &str) -> bool { + [ + "/capture/", + "/bin/", + "/probes/", + "debug_panel", + "shape_gate", + "framexml_diff", + ] + .iter() + .any(|p| rel.contains(p)) +} + +/// Collapse a shipped value and a Rust literal onto the same shape: every placeholder — the +/// reference's `%s`/`%d`/`%1$s` and Rust's `{}`/`{name}` alike — becomes one marker, `\32` is the +/// escaped space it stands for, runs of whitespace collapse, and case is dropped. +fn normalize(s: &str) -> String { + let s = s.replace("\\32", " "); + let mut out = String::with_capacity(s.len()); + let b: Vec = s.chars().collect(); + let mut i = 0; + while i < b.len() { + match b[i] { + '{' => { + // a Rust format hole + while i < b.len() && b[i] != '}' { + i += 1; + } + i += 1; + out.push('\u{1}'); + } + '%' => { + let mut j = i + 1; + while j < b.len() + && (b[j].is_ascii_digit() || b[j] == '$' || b[j] == '.' || b[j] == '-') + { + j += 1; + } + if j < b.len() && matches!(b[j], 's' | 'd' | 'f' | 'c' | 'g') { + out.push('\u{1}'); + i = j + 1; + } else { + out.push('%'); + i += 1; + } + } + c => { + out.push(c.to_ascii_lowercase()); + i += 1; + } + } + } + out.split_whitespace().collect::>().join(" ") +} + +/// Parse `KEY = "value";` out of a Lua string table. +fn lua_table(src: &str) -> HashMap { + let mut out = HashMap::new(); + for line in src.lines() { + let Some((k, rest)) = line.split_once('=') else { + continue; + }; + let k = k.trim(); + if k.is_empty() + || !k + .bytes() + .all(|c| c.is_ascii_uppercase() || c.is_ascii_digit() || c == b'_') + { + continue; + } + let rest = rest.trim(); + let Some(rest) = rest.strip_prefix('"') else { + continue; + }; + if let Some(end) = rest.rfind("\";") { + out.insert(k.to_string(), rest[..end].to_string()); + } + } + out +} + +fn walk(dir: &Path, into: &mut Vec) { + let Ok(entries) = std::fs::read_dir(dir) else { + return; + }; + for entry in entries.flatten() { + let path = entry.path(); + let name = path.file_name().and_then(|n| n.to_str()).unwrap_or(""); + if path.is_dir() { + if !matches!(name, "target" | ".git" | "tests" | "benches" | "examples") { + walk(&path, into); + } + } else if name.ends_with(".rs") && !name.contains("test") { + into.push(path); + } + } +} + +/// Does this function resolve a key at all? If so, a literal inside it is a fallback. +const RESOLVERS: &[&str] = &[ + "strings.get(", + ".text(", + "keyed_line", + "globals().get::", + "GlueStrings", + "by_key(", + "UiError::key", + "glue_strings", +]; + +#[test] +fn no_user_facing_sentence_is_written_in_rust_when_the_reference_ships_one() { + let data = match benilla_formats::wow_data() { + Some(d) => d, + None => return, + }; + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let mut shipped = HashMap::new(); + for file in [ + "Interface\\FrameXML\\GlobalStrings.lua", + "Interface\\GlueXML\\GlueStrings.lua", + ] { + let src = chain + .read_file(file) + .unwrap_or_else(|e| panic!("{file}: {e}")); + for (k, v) in lua_table(&String::from_utf8_lossy(&src)) { + let n = normalize(&v); + // A one-word or punctuation-only value ("Locked", "%s") is too weak a signal: it + // collides with ordinary program text. Sentences are what this is after. + if n.split(' ').count() >= 2 && n.chars().any(|c| c.is_ascii_lowercase()) { + shipped.entry(n).or_insert(k); + } + } + } + + let mut sources = Vec::new(); + walk(Path::new("src"), &mut sources); + walk(Path::new("../benilla-ui/src"), &mut sources); + walk(Path::new("../benilla-formats/src"), &mut sources); + + let allowed: HashMap<&str, usize> = ALLOWED.iter().copied().collect(); + let mut found: HashMap> = HashMap::new(); + + for path in &sources { + // The walk starts at this crate's `src` and at the two sibling crates by relative path; + // name every hit `/src/...` so the ratchet rows read the same wherever it ran. + let raw = path.to_string_lossy().to_string(); + let rel = match raw.strip_prefix("../") { + Some(sibling) => sibling.to_string(), + None => format!("benilla-app/{raw}"), + }; + if is_instrument(&rel) { + continue; + } + let src = strip_test_modules(&std::fs::read_to_string(path).expect("read source")); + for (fname, body) in split_fns(&src) { + if RESOLVERS.iter().any(|r| body.contains(r)) { + continue; // a literal here is a fallback beside a real lookup + } + for lit in string_literals(body) { + let n = normalize(&lit); + if n.split(' ').count() < 2 { + continue; + } + if let Some(key) = shipped.get(&n) { + found + .entry(rel.clone()) + .or_default() + .push(format!("{key} — {fname}: {lit:?}")); + } + } + } + } + + let mut over = Vec::new(); + for (file, hits) in &found { + let budget = allowed.get(file.as_str()).copied().unwrap_or(0); + if hits.len() > budget { + over.push(format!( + "\n {file}: {} literals, ratchet allows {budget}\n{}", + hits.len(), + hits.iter() + .map(|h| format!(" {h}\n")) + .collect::() + )); + } + } + // The ratchet only bites downward if a *stale* row is an error too — otherwise a converted + // file's row lingers and silently re-permits the drift it was meant to retire. + for (file, budget) in ALLOWED { + let actual = found.get(*file).map_or(0, |h| h.len()); + assert!( + actual >= *budget, + "\nRATCHET STALE: {file} now has {actual} re-typed literals but its row still allows \ + {budget}.\nLower it to {actual} (or delete the row at 0) — the count may only go down." + ); + } + assert!( + over.is_empty(), + "\nA user-facing sentence is written in Rust where the reference ships the string.\n\ + Resolve it by key instead — `keyed_line`/`keyed_line_s` where a `UiScript` is in hand, \ + `UiError::key` into a queue at the net bridge, `GlueStrings::text` on the glue screens.\n\ + (decision 2035){}", + over.join("") + ); +} + +/// Cut every `#[cfg(test)]` block out of a source file before scanning it. +/// +/// Without this the walk reads test fixtures as production text — `ui_action::cast_fail`'s unit +/// tests build a *fake* GlobalStrings map whose 48 entries are, by construction, byte-identical to +/// the shipped strings. Those are the test doing its job, not drift. +fn strip_test_modules(src: &str) -> String { + let mut out = String::with_capacity(src.len()); + let mut rest = src; + while let Some(at) = rest.find("#[cfg(test)]") { + out.push_str(&rest[..at]); + // skip to the opening brace of the item that attribute guards, then brace-match it + let after = &rest[at..]; + let Some(open) = after.find('{') else { break }; + let mut depth = 0usize; + let mut end = None; + for (i, c) in after[open..].char_indices() { + match c { + '{' => depth += 1, + '}' => { + depth -= 1; + if depth == 0 { + end = Some(open + i + 1); + break; + } + } + _ => {} + } + } + match end { + Some(e) => rest = &after[e..], + None => break, + } + } + out.push_str(rest); + out +} + +/// Split a source file into `(fn name, body)` pairs — crude but enough to ask "does the enclosing +/// function resolve a key?", which is the only question here. +fn split_fns(src: &str) -> Vec<(&str, &str)> { + let mut out = Vec::new(); + let bytes = src.as_bytes(); + let mut starts: Vec<(usize, &str)> = Vec::new(); + for (i, _) in src.match_indices("fn ") { + // a top-levelish `fn`: preceded only by whitespace/visibility on its line + let line_start = src[..i].rfind('\n').map_or(0, |n| n + 1); + let prefix = &src[line_start..i]; + if !prefix.trim_start().is_empty() + && !matches!( + prefix.trim(), + "pub" | "pub(crate)" | "pub(super)" | "const" | "async" + ) + && !prefix.trim().starts_with("pub(") + { + continue; + } + let rest = &src[i + 3..]; + let name_end = rest + .find(|c: char| !c.is_alphanumeric() && c != '_') + .unwrap_or(rest.len()); + starts.push((i, &rest[..name_end])); + } + for (n, (i, name)) in starts.iter().enumerate() { + let end = starts.get(n + 1).map_or(bytes.len(), |(j, _)| *j); + out.push((*name, &src[*i..end])); + } + if out.is_empty() { + out.push(("", src)); + } else if let Some((first, _)) = starts.first() { + out.push(("", &src[..*first])); // module-level `const … : &str = "…"` + } + out +} + +/// Every double-quoted literal in a chunk, skipping comment lines and developer-facing macros. +fn string_literals(body: &str) -> Vec { + let mut out = Vec::new(); + for line in body.lines() { + let t = line.trim_start(); + if t.starts_with("//") { + continue; + } + if [ + "debug!", + "info!", + "warn!", + "error!", + "trace!", + "println!", + "eprintln!", + "panic!", + "assert", + "unreachable!", + "todo!", + ".expect(", + "unwrap_or_else", + ] + .iter() + .any(|m| line.contains(m)) + { + continue; + } + let mut rest = line; + while let Some(start) = rest.find('"') { + let after = &rest[start + 1..]; + let Some(end) = after.find('"') else { break }; + let lit = &after[..end]; + if lit.len() >= 6 && lit.contains(' ') && lit.chars().any(|c| c.is_ascii_lowercase()) { + out.push(lit.to_string()); + } + rest = &after[end + 1..]; + } + } + out +} diff --git a/crates/benilla-assets/src/shaders/wow_model.wgsl b/crates/benilla-assets/src/shaders/wow_model.wgsl index 76ddf398..3a8e161d 100644 --- a/crates/benilla-assets/src/shaders/wow_model.wgsl +++ b/crates/benilla-assets/src/shaders/wow_model.wgsl @@ -904,11 +904,12 @@ fn fragment(in: WowVsOut, @builtin(front_facing) is_front: bool) -> WowFragOut { // pixels, demoted the justification) → 0799 (the A/B) → 0803, back on the curve for good. // // The per-material `sun_scale.x` selector has THREE states (model_render::ShadeSel): ≥0.85 = - // the lit-ground family (ADT doodads and every entity M2 — animator target 2.5, mixed toward + // the lit-ground family — **entity M2s only** since 2050 (animator target 2.5, mixed toward // 0.5 by the per-instance tag shade byte, which units/players/GameObjects ramp CPU-side like - // the binary's `0x69e770`; statics leave it 0), 0.5..0.85 = fixed intensity 1.0 (an exterior - // WMO MODD prop — the 2.5 site is one a MODD prop never reaches, §8b), <0.5 = statically - // MCSH-shadowed (0.5). The ramp runs in animator units and the COMMIT clamps. + // the binary's `0x69e770`); 0.5..0.85 = fixed intensity 1.0, which is **both doodad classes** + // (an ADT map doodad and an exterior WMO MODD prop are one C++ class and neither can reach the + // 2.5 site); <0.5 = statically MCSH-shadowed (0.5). Statics leave the tag byte 0. The ramp + // runs in animator units and the COMMIT clamps. // // **`min(I, 1)` is OURS, and it is the one unfaithful term in this lane (0803 §3, 0814, 0821).** // The reference does not cap the gain. On the VS/SH lane — the default config, and the lane this @@ -916,10 +917,25 @@ fn fragment(in: WowVsOut, @builtin(front_facing) is_front: bool) -> WowFragOut { // FFP lane `0x71c730`→`0x71ca80` clamps the **product** `D × I` (by max-channel, preserving hue), // never the multiplier. So the cap below is a benilla choice, and it costs us twice: // - // 1. **Brightness.** A lit ADT doodad commits ×1.0 where the reference gives ×2.5, so doodads - // and characters in sun read dimmer than the reference. Still open — lifting it pushes a - // sun-facing surface well past 1.0 (with an over-gamut sun, into green as well), which is a - // world-wide look change and the director's call, not one to self-grade. + // 1. **Brightness.** A lit ENTITY — unit, player, GameObject — commits ×1.0 where the + // reference gives ×2.5, so characters in sun read dimmer than the reference. Still open — + // lifting it pushes a sun-facing surface well past 1.0 (with an over-gamut sun, into green + // as well), which is a world-wide look change and the director's call, not one to + // self-grade. (Doodads were in this sentence until 2050; see below.) + // + // **Doodads are NOT in that sentence any more (2050).** wow-re settled it at the bytes + // and in the capture: a `CMapDoodadDef` — ADT MDDF and WMO MODD alike — commits 1.0 lit / + // 0.5 shadowed and never ramps; it has no ramp target field (`+0xf8` is `m[2][3]` of its + // world matrix), and the 2.5 belongs to the WENTITY node an entity hangs off `[obj+0xe0]`. + // §6's attribution of 2.5 to ADT doodads was the error. So this cap is a **no-op on the + // faithful doodad input** and never dimmed a tree; what it dims is units, players and + // GameObjects, which is what item 1 above is now about. + // + // The trap that leaves behind, and why 2050 moved the selector rather than waiting for + // the cap: feeding every M2 a lit 2.5 and then capping emits the right number for a lit + // doodad **for the wrong reason**. Lifting the cap without giving doodads their own 1.0 + // would have taken every tree in Elwynn to 2.5×, and a uniformly too-bright world is the + // kind of wrong nobody files. The scale is per-class now, so that is no longer coupled. // 2. **Timing — fixed CPU-side (0821).** Because the cap sits on the multiplier, every target // from 2.5 down to 1.0 renders identically, so a unit ramping 2.5 → 0.5 spent its first // 0.45 s (75 % of the chase) invisibly pinned at 1.0 and then dropped in 0.15 s. It read as a @@ -929,7 +945,10 @@ fn fragment(in: WowVsOut, @builtin(front_facing) is_front: bool) -> WowFragOut { // 3.3333 intensity-units/s. The cap is a backstop here, not the thing the ramp fights. // // Two faces of one bug, and they unwind together: **cut `min(I, 1)` and `LIT_T` goes back to 0.0** - // so the full 2.5 → 0.5 sweep becomes visible on its own. Do not cut one alone. (Units are on this + // so the full 2.5 → 0.5 sweep becomes visible on its own. Do not cut one alone — and note that + // the pair is now an ENTITY-only concern: since 2050 no doodad rides this band, so lifting the + // cap changes units, players and GameObjects and leaves the world's trees and props where they + // are. That decoupling was the prerequisite, and it is done. (Units are on this // chain again — 0809's flat ×1.0 pin was wrong and 0814 reverted it; the null fallback that // motivated it is real, but it is a lifecycle state we do not model.) let inst_shade = select(f32((fade_tag >> 6u) & 0xffu) / 255.0, 0.0, interior_prop); diff --git a/crates/benilla-assets/src/world_assets/images.rs b/crates/benilla-assets/src/world_assets/images.rs index af9f0711..efb9c4e8 100644 --- a/crates/benilla-assets/src/world_assets/images.rs +++ b/crates/benilla-assets/src/world_assets/images.rs @@ -91,6 +91,103 @@ pub fn repeat_texture_authored(upload: crate::gpu_blp::UploadChain, wrap: (bool, image } +/// `WOW_LIQUID_DC=raw` restores the shipped frames' own per-frame DC — the A/B lever for +/// [`flatten_frame_dc`] while the fidelity question behind it is open (see that function). +fn dc_normalization_enabled() -> bool { + std::env::var("WOW_LIQUID_DC").as_deref() != Ok("raw") +} + +/// Flatten each frame's **DC** (per level, per channel) onto the loop's mean — the fix for the +/// water sheet breathing once per animation loop. +/// +/// The shipped ocean frames do not share a mean. Across `ocean_h`'s 30-frame loop the full-texture +/// mean alpha runs 55.985..57.628, and DXT3's 4-bit alpha quantisation makes the small levels drift +/// harder still: measured swing is 1.64 at mip 0, 3.72 at mip 5 and 6.38 at mip 6. Near the camera +/// that is invisible, because each pixel lands on a different texel and the variation scatters as +/// ripple shimmer. At distance mipping averages the whole level into one value, so every pixel of +/// the far sheet moves *together* and the surface visibly brightens and dims once per loop — +/// 1.25 s at the 24 fps flip. Director-reported at Ratchet (2026-09-06); measured off their capture +/// at 0.8..1.84/255 with an autocorrelation period of exactly 1.250 s, and confirmed coherent +/// rather than noise (a 0.82/255 swing in a 104k-pixel mean cannot come from per-pixel scatter). +/// It reads as a flicker only at a high `farclip` — at 300 that band is fogged out or past the +/// wall, which is why the report came with "not visible at 300". +/// +/// The correction is a per-level **additive** offset, not a gain: the drift is a DC shift, and an +/// offset removes it without touching the ripple's contrast — where a multiplicative fit would +/// scale the crests (alpha reaches 255) and clip them. Applied per channel because both terms of +/// the ADT combine carry the drift (`detail.rgb`, and `detail.a` through the sheen factor). +/// +/// **Water/ocean only.** Magma and slime take the animated sheet as their opaque BODY colour, where +/// a per-frame brightness swing is the intended pulse, so the caller passes `false` for the +/// fullbright kinds and their DC is left alone. +/// +/// **This is a deliberate divergence from the shipped art**, pending the RE round on whether the +/// reference shows the same breathing (it uploads the same authored, same-quantised mips, so it +/// plausibly does). `WOW_LIQUID_DC=raw` restores the frames verbatim for the A/B. +fn flatten_frame_dc(data: &mut [u8], spans: &[Vec<(usize, usize)>], levels: usize) { + for level in 0..levels { + // The loop's target sum-per-texel for this level, over every frame. + let mut sums = [0i64; 4]; + let mut texels = 0usize; + for frame in spans { + let (start, len) = frame[level]; + for px in data[start..start + len].chunks_exact(4) { + for c in 0..4 { + sums[c] += i64::from(px[c]); + } + } + texels += len / 4; + } + if texels == 0 { + continue; + } + for frame in spans { + let (start, len) = frame[level]; + let n = len / 4; + if n == 0 { + continue; + } + for c in 0..4 { + let have: i64 = data[start..start + len] + .chunks_exact(4) + .map(|px| i64::from(px[c])) + .sum(); + // The exact integer total this channel must move by to sit on the loop mean. + let want = (sums[c] * n as i64).div_euclid(texels as i64); + let mut delta = want - have; + if delta == 0 { + continue; + } + // Spend the delta in ±1 steps, walking the level on a stride coprime with its + // texel count so the touched texels scatter instead of forming a patch at the + // start. Repeated passes each add at most 1 LSB per texel, so a large correction + // stays a faint uniform lift rather than a few blown texels; texels already at the + // 0/255 rail are skipped, and a pass that moves nothing ends the walk. + let stride = if n % 7 == 0 { 1 } else { 7 }; + let mut progress = true; + while delta != 0 && progress { + progress = false; + for k in 0..n { + if delta == 0 { + break; + } + let b = &mut data[start + ((k * stride) % n) * 4 + c]; + if delta > 0 && *b < 255 { + *b += 1; + delta -= 1; + progress = true; + } else if delta < 0 && *b > 0 { + *b -= 1; + delta += 1; + progress = true; + } + } + } + } + } + } +} + /// Stack the animated liquid frames into one **mipmapped + anisotropic** `texture_2d_array` from each /// frame's BLP **authored** mip pyramid — same fidelity rule as [`repeat_texture_authored`]: the real /// 1.12 client uploads the BLP's stored mips verbatim (no client-side regeneration). Without mips, the @@ -100,7 +197,7 @@ pub fn repeat_texture_authored(upload: crate::gpu_blp::UploadChain, wrap: (bool, /// `size→1` chain; a frame whose authored chain is shorter/odd is filled by `NEAREST` from its nearest /// authored level (gamma-safe, no averaging — mirrors [`chain_to_layer`]). All frames must share mip-0 /// `size` (the caller enforces this). Returns a `D2Array`-viewed image with `mip_level_count` set. -pub fn liquid_frame_array(frames: Vec) -> Image { +pub fn liquid_frame_array(frames: Vec, normalize_dc: bool) -> Image { let size = frames[0].width; // Full mip chain down to 1×1 (square 1.12 frames): levels = log2(size) + 1. let mip_level_count = { @@ -112,7 +209,11 @@ pub fn liquid_frame_array(frames: Vec) -> Image { n }; let mut data = Vec::with_capacity(frames.len() * mip_chain_byte_size(size, mip_level_count)); + // Where each (frame, level) region landed, so the DC pass below can address one level of one + // frame without re-deriving the LayerMajor arithmetic. + let mut spans: Vec> = Vec::with_capacity(frames.len()); for blp in &frames { + let mut per_level = Vec::with_capacity(mip_level_count as usize); for level in 0..mip_level_count { let lw = (size >> level).max(1); // Match the array level to the authored mip of the same size (normally identity — water @@ -124,8 +225,14 @@ pub fn liquid_frame_array(frames: Vec) -> Image { 0 }; let (sw, sh) = blp.mip_size(src_level as u32); + let start = data.len(); extend_nearest(&mut data, &blp.mips[src_level], sw, sh, lw, lw); + per_level.push((start, data.len() - start)); } + spans.push(per_level); + } + if normalize_dc && dc_normalization_enabled() { + flatten_frame_dc(&mut data, &spans, mip_level_count as usize); } let mut image = Image::new_uninit( Extent3d { @@ -387,6 +494,96 @@ pub fn extend_nearest(out: &mut Vec, src: &[u8], sw: u32, sh: u32, dw: u32, mod tests { use super::*; + /// Two 2x2 frames whose means differ by a lot, with a full 2-level chain each — the smallest + /// shape that exercises the per-level DC pass. + fn dc_frames() -> Vec { + let frame = |base: u8| BlpMipChain { + width: 2, + height: 2, + texels: benilla_formats::BlpTexels::Rgba8Unorm, + mips: vec![ + // 2x2: four texels around `base`. + vec![ + base, + base, + base, + base, + base + 10, + base + 10, + base + 10, + base + 10, + base, + base, + base, + base, + base + 10, + base + 10, + base + 10, + base + 10, + ], + // 1x1 + vec![base + 5, base + 5, base + 5, base + 5], + ], + }; + vec![frame(40), frame(80)] + } + + /// The whole point of [`flatten_frame_dc`]: after it, no frame's level mean differs from any + /// other's, so a mip-averaged sheet cannot brighten and dim as the loop plays. This is the + /// property the Ratchet report turned on — a 1.25 s whole-sheet breath measured at up to + /// 1.84/255 — and a still frame can never show it, so it is asserted here instead. + #[test] + fn flatten_frame_dc_equalizes_every_frame_at_every_level() { + let frames = dc_frames(); + let n = frames.len(); + let img = liquid_frame_array(frames, true); + let data = img.data.as_ref().expect("array has data"); + + // LayerMajor: frame0[2x2, 1x1], frame1[2x2, 1x1]. + let level_bytes = [2 * 2 * 4usize, 4]; + let stride: usize = level_bytes.iter().sum(); + for (level, &len) in level_bytes.iter().enumerate() { + let offset: usize = level_bytes[..level].iter().sum(); + let means: Vec = (0..n) + .map(|f| { + let start = f * stride + offset; + let px = &data[start..start + len]; + px.iter().map(|&b| f64::from(b)).sum::() / px.len() as f64 + }) + .collect(); + let spread = means.iter().cloned().fold(f64::MIN, f64::max) + - means.iter().cloned().fold(f64::MAX, f64::min); + assert!( + spread <= 1.0, + "level {level} still drifts across frames: {means:?} (spread {spread})" + ); + } + } + + /// The lever back to the shipped art, and the control for the test above: with + /// `WOW_LIQUID_DC=raw` the frames keep their own DC, so the drift the fix removes is still + /// there. Without this, a no-op `flatten_frame_dc` would pass the assertion above trivially. + #[test] + fn raw_lever_keeps_the_authored_per_frame_dc() { + // The pass is skipped when the caller opts out, which is the fullbright (magma/slime) lane. + let frames = dc_frames(); + let img = liquid_frame_array(frames, false); + let data = img.data.as_ref().expect("array has data"); + let mean = |start: usize, len: usize| { + data[start..start + len] + .iter() + .map(|&b| f64::from(b)) + .sum::() + / len as f64 + }; + let stride = 2 * 2 * 4 + 4; + let spread = (mean(stride, 16) - mean(0, 16)).abs(); + assert!( + spread > 30.0, + "opting out must leave the frames' own DC alone, saw spread {spread}" + ); + } + fn descriptor_only(size: Extent3d, mips: u32, format: TextureFormat) -> Image { let mut image = Image::new_uninit( size, diff --git a/crates/benilla-ui/src/lib.rs b/crates/benilla-ui/src/lib.rs index 641e4db6..4bcb1add 100644 --- a/crates/benilla-ui/src/lib.rs +++ b/crates/benilla-ui/src/lib.rs @@ -32,5 +32,6 @@ pub mod messages; pub mod order; pub mod script; pub mod source; +pub mod strings; pub mod toc; pub mod widget; diff --git a/crates/benilla-ui/src/strings.rs b/crates/benilla-ui/src/strings.rs new file mode 100644 index 00000000..c3571d82 --- /dev/null +++ b/crates/benilla-ui/src/strings.rs @@ -0,0 +1,237 @@ +//! **One faithful substitution for reference string templates** — the client's `SStrPrintf` face. +//! +//! Every user-visible sentence benilla shows comes from `GlobalStrings.lua` or `GlueStrings.lua` +//! (decision 2045), and most of those carry `%s`/`%d` holes the caller fills. Doing that filling +//! correctly is not `format!`'s job and never was: the template is *data read at runtime from the +//! player's install*, so the holes have to be walked, not compiled. +//! +//! **Why this module exists.** Before it, the substitution had been reinvented at least eight +//! times across the workspace, with semantics that did not agree: +//! +//! - `ui_instance::fill_template` — ordered, `%%`-aware, starvation-safe, and tested. The best of +//! them, and the one this is derived from. +//! - `ui_duel::winner_line` — a hand-rolled `%1$s`/`%2$s` positional pass, because the duel's +//! retreat wording swaps its two names and no ordered filler can express that. +//! - `ui_action::errors::ui_error_text` — `str::replace`, which fills **every** `%s` with the +//! *same* argument. Latent rather than live (no message it currently carries has two), but it +//! was a real trap waiting for the first two-argument template to reach that queue. +//! - `ui_guild::fill`, `ui_petition::fill`, and one-off `replacen` calls in `ui_binder`, +//! `ui_trade`, `ui_items::feed` and `equip_error`. +//! +//! Eight copies of one primitive is how the `ui_error_text` bug survived: there was no single +//! place where "how do we fill a reference template" could be got right once. +//! +//! **The semantics, and why each is what it is.** +//! +//! - `%s` and `%d` consume the next argument, **left to right**. This is `SStrPrintf`'s own order. +//! - `%N$s` / `%N$d` take argument `N` (1-based) and do **not** move the sequential cursor. 1.12 +//! uses these exactly where a translation needs to reorder the holes — +//! `DUEL_WINNER_RETREAT = "%2$s has fled from %1$s in a duel"` is the canonical case, and it is +//! also why hardcoding an English sentence is a localization bug and not merely untidy. +//! - `%%` collapses to one `%`. +//! - **A specifier whose argument is missing is copied through literally.** A template we +//! mis-modelled should look wrong, not look plausible — `ui_instance` established this and it is +//! the right call: a visibly broken line gets reported, a quietly wrong one does not. +//! - Any other specifier is copied through untouched. + +use std::fmt::Write as _; + +/// One argument to [`fill`]. `%s` renders either variant as text; `%d` renders either as an +/// integer, so a caller that groups its arguments differently from the template still fills in the +/// order the template asks for. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Arg<'a> { + /// A string argument — a player name, an item link, a zone. + S(&'a str), + /// A numeric argument. + D(i64), +} + +impl<'a> From<&'a str> for Arg<'a> { + fn from(s: &'a str) -> Self { + Arg::S(s) + } +} + +impl<'a> From<&'a String> for Arg<'a> { + fn from(s: &'a String) -> Self { + Arg::S(s.as_str()) + } +} + +macro_rules! arg_from_int { + ($($t:ty),*) => {$( + impl From<$t> for Arg<'_> { + fn from(n: $t) -> Self { + Arg::D(i64::from(n)) + } + } + )*}; +} +arg_from_int!(u8, u16, u32, i8, i16, i32); + +impl Arg<'_> { + fn as_s(&self, out: &mut String) { + match self { + Arg::S(s) => out.push_str(s), + Arg::D(n) => { + let _ = write!(out, "{n}"); + } + } + } + + fn as_d(&self, out: &mut String) { + match self { + Arg::D(n) => { + let _ = write!(out, "{n}"); + } + // A `%d` handed a string is a caller bug, not a display decision; showing the string + // is strictly more useful than showing nothing and matches what varargs would do with + // a pointer-sized value far better than a zero would. + Arg::S(s) => out.push_str(s), + } + } +} + +/// Fill a reference template. See the module doc for the rules. +pub fn fill(template: &str, args: &[Arg<'_>]) -> String { + let mut out = String::with_capacity(template.len() + 16); + let chars: Vec = template.chars().collect(); + let mut i = 0; + let mut next = 0usize; // the sequential cursor + while i < chars.len() { + if chars[i] != '%' { + out.push(chars[i]); + i += 1; + continue; + } + // `%%` + if chars.get(i + 1) == Some(&'%') { + out.push('%'); + i += 2; + continue; + } + // `%N$s` / `%N$d` — positional, does not move the cursor + let mut j = i + 1; + let mut digits = String::new(); + while j < chars.len() && chars[j].is_ascii_digit() { + digits.push(chars[j]); + j += 1; + } + if !digits.is_empty() && chars.get(j) == Some(&'$') { + let spec = chars.get(j + 1).copied(); + let idx = digits.parse::().unwrap_or(0); + if matches!(spec, Some('s') | Some('d')) { + match idx.checked_sub(1).and_then(|k| args.get(k)) { + Some(a) => { + if spec == Some('s') { + a.as_s(&mut out); + } else { + a.as_d(&mut out); + } + i = j + 2; + continue; + } + // starved: copy the whole specifier through + None => { + out.extend(&chars[i..j + 2]); + i = j + 2; + continue; + } + } + } + } + // plain `%s` / `%d` + match chars.get(i + 1) { + Some('s') | Some('d') => { + let spec = chars[i + 1]; + match args.get(next) { + Some(a) => { + if spec == 's' { + a.as_s(&mut out); + } else { + a.as_d(&mut out); + } + next += 1; + i += 2; + } + None => { + // starved — copy it through so a mis-modelled template looks wrong + out.push('%'); + i += 1; + } + } + } + _ => { + out.push('%'); + i += 1; + } + } + } + out +} + +#[cfg(test)] +mod tests { + use super::*; + + /// `ui_instance::fill_template`'s own contract, which this subsumes — ordered, starvation-safe, + /// `%%`-aware. These are its test's cases verbatim, so the migration cannot change behaviour. + #[test] + fn ordered_fills_stop_at_the_arguments_they_have() { + assert_eq!( + fill("%s: %d/%d", &[Arg::S("MC"), Arg::D(2), Arg::D(5)]), + "MC: 2/5" + ); + assert_eq!(fill("%s: %d/%d", &[Arg::S("MC"), Arg::D(2)]), "MC: 2/%d"); + assert_eq!(fill("100%% sure", &[]), "100% sure"); + assert_eq!(fill("no fills", &[Arg::D(7)]), "no fills"); + } + + /// **The case an ordered filler cannot express**, and the reason positional specifiers exist: + /// 1.12's duel pair reorders the same two names, so filling them left-to-right names the wrong + /// winner. Both templates are quoted from GlobalStrings (958/959). + #[test] + fn positional_specifiers_reorder_rather_than_consume() { + let (a, b) = (Arg::S("Alice"), Arg::S("Bob")); + assert_eq!( + fill("%1$s has defeated %2$s in a duel", &[a, b]), + "Alice has defeated Bob in a duel" + ); + assert_eq!( + fill("%2$s has fled from %1$s in a duel", &[a, b]), + "Bob has fled from Alice in a duel" + ); + // A positional may repeat an argument, which no cursor-based fill can do. + assert_eq!(fill("%1$s vs %1$s", &[a]), "Alice vs Alice"); + } + + /// The bug that eight copies of this hid: `str::replace` fills every `%s` with the *same* + /// argument. Two holes must take two arguments. + #[test] + fn each_hole_takes_its_own_argument() { + assert_eq!( + fill( + "%s has promoted %s to %s.", + &[Arg::S("A"), Arg::S("B"), Arg::S("Knight")] + ), + "A has promoted B to Knight." + ); + } + + /// A starved positional is copied through too, and an unknown specifier is left alone. + #[test] + fn starved_and_unknown_specifiers_survive_visibly() { + assert_eq!(fill("%1$s and %2$s", &[Arg::S("only")]), "only and %2$s"); + assert_eq!(fill("50% off", &[]), "50% off"); + assert_eq!(fill("%q", &[Arg::S("x")]), "%q"); + } + + /// Numbers reach `%s` as digits and strings reach `%d` as themselves — a caller whose argument + /// grouping differs from the template's still fills in the template's order. + #[test] + fn arguments_render_for_whichever_hole_they_meet() { + assert_eq!(fill("%s/%d", &[Arg::D(3), Arg::D(5)]), "3/5"); + assert_eq!(fill("%d", &[Arg::S("many")]), "many"); + } +} diff --git a/crates/benilla-world/src/liquid/surface.rs b/crates/benilla-world/src/liquid/surface.rs index 97450bdb..51ea2d70 100644 --- a/crates/benilla-world/src/liquid/surface.rs +++ b/crates/benilla-world/src/liquid/surface.rs @@ -475,7 +475,7 @@ pub(super) fn setup_liquid( let mut assets = LiquidAssets::default(); for &(kind, dir, stem, count) in FRAME_SETS { let Some((frames, frame_count)) = - load_frame_array(&mut world_assets, &mut images, dir, stem, count) + load_frame_array(&mut world_assets, &mut images, kind, dir, stem, count) else { warn!("liquid: no frames for {stem} — {kind:?} water will not render"); continue; @@ -597,6 +597,7 @@ pub(super) fn setup_liquid( fn load_frame_array( world_assets: &mut WorldAssets, images: &mut Assets, + kind: LiquidKind, dir: &str, stem: &str, count: u32, @@ -623,7 +624,12 @@ fn load_frame_array( return None; } let loaded = frames.len() as u32; - Some((images.add(liquid_frame_array(frames)), loaded)) + // Flatten the per-frame DC for the WATER kinds only: mipping turns the shipped frames' drifting + // means into a whole-sheet brightness breath once per animation loop, which reads as a flicker on + // distant water. Magma/slime draw the sheet AS their body colour, where that swing is the intended + // pulse — so they keep theirs (`assets::flatten_frame_dc`). + let normalize_dc = !kind.is_fullbright(); + Some((images.add(liquid_frame_array(frames, normalize_dc)), loaded)) } #[cfg(test)] diff --git a/crates/benilla-world/src/model_render.rs b/crates/benilla-world/src/model_render.rs index b211082e..5d545217 100644 --- a/crates/benilla-world/src/model_render.rs +++ b/crates/benilla-world/src/model_render.rs @@ -144,8 +144,12 @@ pub struct MatKey { /// intensity per instance via the `MeshTag` shade byte ([`crate::entity_shade`]). #[derive(Clone, Copy, PartialEq, Eq, Hash)] pub enum ShadeSel { - /// Lit ground, the boosted intensity family (the binary's 2.5): ADT map doodads on unshadowed - /// ground, and every entity M2. + /// Lit ground, the boosted intensity family (the binary's 2.5): **every entity M2, and only + /// those** — a unit, a player, a GameObject. The 2.5 and its 3.3333/s chase live on the + /// WENTITY light node (vtable `0x810810`, hung off `[obj+0xe0]`); an object without one + /// cannot reach either. ADT map doodads were on this variant until 2050 and are not entities: + /// they are `CMapDoodadDef`s, whose `[+0xa4]` is only ever {0.0, 0.5, 1.0} and which have no + /// ramp target field at all (`+0xf8` is `m[2][3]` of their world matrix). /// /// Entities still select this, but what it *means* for them changed in 0809. The selector is /// only the static half — the per-instance `MeshTag` shade byte carries the rest, and @@ -154,8 +158,14 @@ pub enum ShadeSel { /// GameObjects keep the real 2.5/0.5 chase. So for an entity this variant reads "on the light-node /// path"; the node decides the amplitude. Lit, - /// Lit ground, intensity 1.0: an exterior WMO MODD prop — §8b, byte-verified never to reach - /// the 2.5 site (a Stormwind street fountain is NOT brightened like an Elwynn tree). + /// Lit ground, intensity 1.0: **either doodad class** — an exterior WMO MODD prop *and* an ADT + /// map doodad, byte-verified never to reach the 2.5 site. + /// + /// The parenthetical this doc carried until 2050 — "a Stormwind street fountain is NOT + /// brightened like an Elwynn tree" — was exactly backwards about the tree: both are + /// `CMapDoodadDef`s and both commit 1.0 lit / 0.5 shadowed. What is brightened is the + /// *entity* walking past them. Measured over six trace frames: 111 identified MDDF + /// placements, 356 draws, gains only ∈ {0.5, 1.0}. Matte, /// The base sits on MCSH-shadowed terrain: the dim intensity (the binary's 0.5). Shaded, diff --git a/crates/benilla-world/src/particles.rs b/crates/benilla-world/src/particles.rs index d306cda7..adcfb9c6 100644 --- a/crates/benilla-world/src/particles.rs +++ b/crates/benilla-world/src/particles.rs @@ -248,6 +248,17 @@ pub struct ParticleEmitter { /// owner is out of the frame's draw set — the edge on which model-instance entities are /// hidden. Quads need no flag: a gated pool simply pushes nothing into the shared stream. gated: bool, + /// **The owner's own freeze** — the scene-level "this cloud is not being drawn" answer for a + /// scene whose CAMERA cannot give it (decision 2046). [`sim::booth_frozen`] reads a + /// booth-layered emitter's freeze off its camera's `is_active` bit, which is exact while one + /// camera means one scene — and the `` tile atlas broke that: EVERY orthographic pane + /// on the sheet shares one camera, and that camera is deliberately kept active whenever any + /// cell is packed, so a pane that leaves the paint list still has a camera saying "drawn". + /// The owner sets this instead, on the frame its own scene stops being drawn. The freeze it buys is exactly the + /// booth's — pool + age HELD, one frame's dt on re-entry, no catch-up, no quads — and, like + /// the booth's, it never applies to a [`Self::draining`] emitter, which has to run its pool + /// out or leak for the session. + frozen: bool, /// The pending recursion model (wow-re `part-child-recursion.md`): once the asset resolves, /// [`wire_child_emitters`] turns its own emitters (cap 4, the reference's `0x7b5dfe`) into /// [`Self::children`] and clears this. @@ -363,6 +374,26 @@ impl ParticleEmitter { self.size_scale = size_scale; } + /// Freeze or thaw this cloud from the OWNER's side — see [`Self::frozen`]. A draining emitter + /// ignores it. + /// + /// **Thawing clears [`Self::gated`]**, because the draw-set arm's cheap early-out + /// (`gated && gate_inputs_still && !fade.is_changed()`, decision 1979's floor) reads none of + /// the owner's inputs: a world-lane cloud left gated here on a still camera could never + /// re-enter its own draw set. One full gate evaluation on the thaw edge is the price. + pub fn set_frozen(&mut self, frozen: bool) { + self.frozen = frozen; + if !frozen { + self.gated = false; + } + } + + /// Is this cloud owner-frozen? ([`Self::set_frozen`]) — so an owner writing the same value + /// every frame need not touch the component and trip change detection. + pub fn is_frozen(&self) -> bool { + self.frozen + } + /// The authored def — read by the particle census probe ([`crate::capture`]), which prints /// per-emitter facts (blend, texture, rate keys) beside the live count. pub fn def(&self) -> &ParticleEmitterDef { @@ -694,6 +725,7 @@ pub fn spawn_emitter( water_bound: emitter.water_bound, texture, gated: false, + frozen: false, recursion: emitter.recursion.clone(), children: Vec::new(), geometry: emitter.geometry.clone(), diff --git a/crates/benilla-world/src/particles/sim.rs b/crates/benilla-world/src/particles/sim.rs index 1c1f3ea1..53e75176 100644 --- a/crates/benilla-world/src/particles/sim.rs +++ b/crates/benilla-world/src/particles/sim.rs @@ -430,10 +430,30 @@ fn is_above(d: f32, r: f32) -> bool { /// /// A **draining** emitter never freezes either way: its pool has to run out, and freezing one /// strands it forever (the world arm's rule further down). +/// +/// **This is the camera's half of the answer, not the whole of it** — see [`scene_frozen`]. fn booth_frozen(cam_active: bool, throttled: bool, draining: bool) -> bool { !cam_active && !throttled && !draining } +/// Does this emitter's SCENE freeze it this frame? The whole law, in one place because it has two +/// sources and having only one of them was a defect (decision 2046). +/// +/// [`booth_frozen`] asks about a CAMERA, and that was exact for as long as one camera meant one +/// scene: every booth before the `` tiles owned its own. The tile atlas broke the premise — +/// every orthographic pane on the sheet renders through ONE camera on ONE layer, and that camera +/// is deliberately kept active whenever any cell is packed, because it is the camera that CLEARS +/// the atlas. So the camera bit answers "drawn" for every pane on the sheet, including the ones +/// that left the paint list, and the question can only be answered by the owner — which is what +/// [`super::ParticleEmitter::set_frozen`] is. +/// +/// `booth` is `(cam_active, throttled)` for a booth-layered emitter, `None` for a world-lane one. +/// A **draining** pool ignores both sources, for [`booth_frozen`]'s reason. +fn scene_frozen(booth: Option<(bool, bool)>, owner_frozen: bool, draining: bool) -> bool { + booth.is_some_and(|(cam_active, throttled)| booth_frozen(cam_active, throttled, draining)) + || (owner_frozen && !draining) +} + /// Per-frame: emit, integrate, and expand each emitter's pool into the shared effect-quad /// stream ([`super::buffer::EffectQuads`]). #[allow(clippy::too_many_arguments, clippy::type_complexity)] // one Bevy system's full input set @@ -571,20 +591,27 @@ pub(super) fn simulate_particles( .filter(|l| !l.intersects(&RenderLayers::default())) .and_then(|l| booth_cams.iter().find(|(_, _, cl, ..)| cl.intersects(l))); let is_booth = booth.is_some(); - if let Some((_, _, _, booth_cam, throttled)) = booth { - if booth_frozen(booth_cam.is_active, throttled, emitter.draining) { - if !emitter.gated { - emitter.gated = true; - for slot in &emitter.model_instances { - for (e, _) in &slot.meshes { - if let Ok((_, _, mut cv)) = child_draws.get_mut(*e) { - *cv = Visibility::Hidden; - } + // The scene freeze ([`scene_frozen`]) — a sleeping booth camera, or an owner that froze + // this cloud because its scene's camera cannot say so. Same shape either way: pool + age + // held, no quads, model-instance entities hidden on the edge. + if scene_frozen( + booth.map(|(_, _, _, c, throttled)| (c.is_active, throttled)), + emitter.frozen, + emitter.draining, + ) { + if !emitter.gated { + emitter.gated = true; + for slot in &emitter.model_instances { + for (e, _) in &slot.meshes { + if let Ok((_, _, mut cv)) = child_draws.get_mut(*e) { + *cv = Visibility::Hidden; } } } - continue; } + continue; + } + if is_booth { emitter.gated = false; } let (draw_cam, e_cam_pos, e_right, e_up) = match booth { @@ -747,6 +774,7 @@ pub(super) fn simulate_particles( geometry: _, model_instances, gated: _, + frozen: _, } = &mut *emitter; // The water-interleave MODEL frame, captured before the draw-anchor local shadows the // `anchor` field below: the cloud anchor is "the MODEL, never the bone" — its transform @@ -1460,7 +1488,8 @@ fn fold_committed_light( mod tests { use super::{ booth_frozen, fold_committed_light, follow_fraction, inherit_trigger, integrate_particle, - is_above, ChildEmitter, EffectLighting, EffectVertex, Particle, StepEnv, Vec3, + is_above, scene_frozen, ChildEmitter, EffectLighting, EffectVertex, Particle, StepEnv, + Vec3, }; use bevy::prelude::{Quat, Transform}; @@ -1544,6 +1573,35 @@ mod tests { assert!(!booth_frozen(false, true, true)); } + /// **A camera is not a scene any more** (decision 2046). Every booth before the `` + /// tile atlas owned its own camera, so `is_active` answered "is this scene drawn". The atlas + /// puts EVERY orthographic pane behind one camera that stays active whenever any cell is + /// packed — it is the camera that clears the sheet — so for a pane that left the paint list + /// the camera says "drawn" and only the owner can say otherwise. Before this, a hidden + /// autocast-shine pane's four spline emitters kept integrating AND kept pushing quads for the + /// whole 600-frame linger, at the cell the packer had since given to another pane. + #[test] + fn an_owner_freezes_a_cloud_whose_camera_still_says_drawn() { + // The tile atlas's own shape: the camera is up, and the pane is not. + assert!(!scene_frozen(Some((true, false)), false, false), "drawing"); + assert!( + scene_frozen(Some((true, false)), true, false), + "the owner parked the pane — the camera cannot see that" + ); + // The booth's camera still answers entirely on its own where it owns the scene. + assert!(scene_frozen(Some((false, false)), false, false)); + assert!( + !scene_frozen(Some((false, true)), false, false), + "…and a throttled camera is awake (1559)" + ); + // A world-lane emitter has no booth camera at all; the owner's lever still reaches it. + assert!(!scene_frozen(None, false, false)); + assert!(scene_frozen(None, true, false)); + // Draining overrides both sources, for `booth_frozen`'s reason. + assert!(!scene_frozen(Some((false, false)), true, true)); + assert!(!scene_frozen(None, true, true)); + } + fn particle(pos: Vec3, vel: Vec3) -> Particle { Particle { pos, diff --git a/crates/benilla-world/src/rig_anim/global_seq.rs b/crates/benilla-world/src/rig_anim/global_seq.rs index 1488ba68..285a457b 100644 --- a/crates/benilla-world/src/rig_anim/global_seq.rs +++ b/crates/benilla-world/src/rig_anim/global_seq.rs @@ -40,9 +40,18 @@ enum SeqTarget { pub struct GlobalSeqDrive { /// `(write target, its baked global-sequence channels)`. bones: Vec<(SeqTarget, GlobalBone)>, - /// The attach snapshot of the shared clock (secs): `None` until the first animate tick stamps - /// it (the instance's attach — the ref writes `+0x68` once, at attach). + /// The attach snapshot of this instance's SCENE clock (secs): `None` until the first animate + /// tick stamps it (the instance's attach — the ref writes `+0x68` once, at attach). anchor: Option, + /// **This instance's scene clock**, when it is not the world's (secs). The kernel's Phase B + /// reads `[[model+0x2c]+0xc]` — the clock of the scene that OWNS the instance — and a + /// `` widget owns a private `CM2Scene` at `CSimpleModel+0x314`, advanced by the + /// widget's own `OnUpdate` and by nothing else (wow-re `gseq-anchor.md` §1/§2 + + /// `modelframe-animation-clock.md` §1.1/§3, both byte-verified). So a UI model tile writes + /// its pane's clock here every frame it draws and the phase rides the pane, not the world: + /// a pane whose frame is hidden stops its clock, and the spin resumes where it stopped. + /// `None` — every world lane — is the world scene's free-running clock (decision 2046). + clock: Option, /// Paused: skip the joint writes (the doodad host gates animation to drawn instances — wow-re /// `doodad-anim-host.md`: the ref's kernel ticks at draw time, so a culled model isn't /// evaluated). Creatures never pause. Resuming needs no re-seek: the cursor is a pure @@ -67,6 +76,7 @@ impl GlobalSeqDrive { (!bones.is_empty()).then_some(Self { bones, anchor: None, + clock: None, paused: false, }) } @@ -82,19 +92,29 @@ impl GlobalSeqDrive { (!bones.is_empty()).then_some(Self { bones, anchor: None, + clock: None, paused: false, }) } - /// Pause/resume the joint writes. One caller: the doodad draw gate - /// (`doodad_anim`'s `set_paused(!drawn)`). While paused the joints hold their last pose; - /// resume lands on the anchored cursor with nothing to catch up. + /// Pause/resume the joint writes — the doodad draw gate's lever, and its only caller. (The + /// booth and UI-tile lanes park instead, with [`super::AnimParked`] on the root, which holds + /// the pose evaluation and the compose as well as these writes; this pause holds only the + /// writes, which is what the doodad lane wants for a host whose player it also stops.) While + /// paused the joints hold their last pose; resume lands on the anchored cursor with nothing + /// to catch up. pub fn set_paused(&mut self, paused: bool) { self.paused = paused; } + + /// Point this instance's channels at its own scene clock (secs) — see [`Self::clock`]. Written + /// per frame by the owner of a scene that is not the world's; never called by a world lane. + pub fn set_clock(&mut self, secs: f64) { + self.clock = Some(secs); + } } -/// Sample every drive's channels at its anchored cursor (`sharedNow − anchor`, stamping the +/// Sample every drive's channels at its anchored cursor (`sceneNow − anchor`, stamping the /// anchor on the first tick — the attach) and write the driven bone — in the pose post-pass /// window ([`super::PosePost`], the same as the body twist), so the model compose folds it. A /// channel overwrites only its own component; a bone the playing animation never keyed (the @@ -109,9 +129,12 @@ fn apply_global_sequences( ) { let now = time.elapsed_secs_f64(); for (host, mut drive, parked) in &mut drives { + // The clock is the instance's SCENE's — the world's for every world lane, and a `` + // widget's private one where the tile renderer wrote it ([`GlobalSeqDrive::clock`]). + let scene_now = drive.clock.unwrap_or(now); // The attach stamp happens even while parked/paused — the ref stamps +0x68 at attach, // not at first draw. - let t = now - *drive.anchor.get_or_insert(now); + let t = scene_now - *drive.anchor.get_or_insert(scene_now); // A parked or paused instance skips only the WRITES — the cursor is absolute // (decision 0448's absolute-clock ruling, now literal: nothing per-instance advances). if drive.paused || parked { diff --git a/crates/benilla-world/src/shaders/static_gx.wgsl b/crates/benilla-world/src/shaders/static_gx.wgsl index 37c500a8..45d8984a 100644 --- a/crates/benilla-world/src/shaders/static_gx.wgsl +++ b/crates/benilla-world/src/shaders/static_gx.wgsl @@ -83,7 +83,7 @@ const WORD_WRAP_X: u32 = 65536u; // 1 << 16 const WORD_WRAP_Y: u32 = 131072u; // 1 << 17 const WORD_UNLIT: u32 = 262144u; // 1 << 18 const WORD_FOG_OFF: u32 = 524288u; // 1 << 19 -const WORD_SHADE_LIT: u32 = 1048576u; // 1 << 20 +const WORD_SHADE_LIT: u32 = 1048576u; // 1 << 20 — no producer since 2050 (entities never divert here); kept, retiring it is a word-layout round const WORD_TEXTURED: u32 = 2097152u; // 1 << 21 // The WMO lane (slice 2) — the entity path's per-material facts as bits: const WORD_WMO: u32 = 4194304u; // 1 << 22 — model_flags.x (the WMO surface laws) @@ -96,7 +96,7 @@ const WORD_HAS_VC: u32 = 134217728u; // 1 << 27 — the batch AUTHORED vertex // The prop lane (B4). INTERIOR without WMO = an interior M2 prop — the entity shader's own // `interior_prop = flags.z && !flags.x` split: probe lighting, interior fog, zero live // point lights. -const WORD_MATTE: u32 = 268435456u; // 1 << 28 — exterior MODD prop: intensity FIXED 1.0 +const WORD_MATTE: u32 = 268435456u; // 1 << 28 — EITHER doodad class (ADT MDDF or exterior MODD prop): intensity FIXED 1.0 (2050) // Vanilla cutout ref (224/255) — wow_model.wgsl's VANILLA_ALPHA_KEY. const VANILLA_ALPHA_KEY: f32 = 0.8784314; diff --git a/crates/benilla-world/src/static_gx/mod.rs b/crates/benilla-world/src/static_gx/mod.rs index 6880ad40..7562e917 100644 --- a/crates/benilla-world/src/static_gx/mod.rs +++ b/crates/benilla-world/src/static_gx/mod.rs @@ -652,12 +652,23 @@ impl StaticGx { return false; // the B4 lever: prop batches back to the merge/entity path } // The shade family gates CELLS only: the WMO lane never reads the selector (the - // entity path passes `Matte` for every WMO batch and lights on the FFP N·L), and - // the prop lane admits Matte as its own word bit (B4 — an exterior MODD prop's - // fixed-1.0 family; an interior prop also arrives Matte, selector unread). + // entity path passes `Matte` for every WMO batch and lights on the FFP N·L). + // + // **`Matte` is now BOTH doodad classes' family, not just the prop lane's** (2050): an + // ADT map doodad is fixed-1.0 exactly like an exterior MODD prop, because they are one + // C++ class (`CMapDoodadDef`) and the 2.5 belongs to the WENTITY node, which neither + // has. So the arm no longer requires `prop.is_some()`. `Shaded` still lands on neither + // bit and still reads 0.5, because the shader's `shade_t` defaults to 1.0 without + // `WORD_SHADE_LIT`. + // + // That leaves `ShadeSel::Lit` with no producer in this lane at all — its three + // populations are ADT doodads, WMO group geometry and WMO props, and entities (the only + // holders of a light node) never divert here. The arm is kept as a backstop rather than + // deleted: retiring `WORD_SHADE_LIT` and its shader branch is a word-layout change and + // wants its own round, named in 2050 rather than folded in here. let (shade_lit, matte) = match (&b.wmo, &b.prop, b.shade) { (Some(_), _, _) => (false, false), - (None, Some(_), ShadeSel::Matte) => (false, true), + (None, _, ShadeSel::Matte) => (false, true), (None, _, ShadeSel::Lit) => (true, false), (None, _, ShadeSel::Shaded) => (false, false), _ => { @@ -1027,6 +1038,14 @@ mod tests { /// The collector refuses exactly the recorded exclusion families — and says so in the /// census — while an eligible batch diverts (1429's no-silent-caps note). + /// + /// **The shade-family exclusion is `Rig` now, not `Matte`** (2050). A cell arriving `Matte` + /// used to be refused, but only because nothing could produce one: ADT doodads were on + /// `Lit`, props carried `prop: Some`, group geometry `wmo: Some`. Since ADT doodads are the + /// fixed-1.0 family — the same C++ class as an exterior MODD prop — a `Matte` cell is the + /// ordinary case and MUST divert, which is what the added assertion below pins. What is left + /// genuinely excluded is the authored-rig booth material, which has no business in the + /// world's retained lane at all. #[test] fn the_divert_declines_the_excluded_families() { let mut gx = StaticGx::default(); @@ -1038,9 +1057,16 @@ mod tests { b.no_depth_write = true; assert!(!gx.divert(b)); let mut b = batch(&g, Vec3::ZERO, None, ModelBlend::Opaque); - b.shade = ShadeSel::Matte; - assert!(!gx.divert(b)); + b.shade = ShadeSel::Rig; + assert!(!gx.divert(b), "a glue-booth rig material never diverts"); assert_eq!(gx.declined, [1, 1, 1, 0, 0]); + // The ADT doodad's own family: a cell on fixed-1.0, no prop record. + let mut b = batch(&g, Vec3::ZERO, None, ModelBlend::Opaque); + b.shade = ShadeSel::Matte; + assert!( + gx.divert(b), + "an ADT map doodad is Matte and still belongs here" + ); assert!(gx.divert(batch(&g, Vec3::ZERO, None, ModelBlend::Opaque))); assert_eq!(gx.cells.len(), 1); } diff --git a/crates/benilla-world/src/terrain_stream/spawn/mod.rs b/crates/benilla-world/src/terrain_stream/spawn/mod.rs index 68ce50dc..33509719 100644 --- a/crates/benilla-world/src/terrain_stream/spawn/mod.rs +++ b/crates/benilla-world/src/terrain_stream/spawn/mod.rs @@ -161,13 +161,29 @@ pub(super) fn spawn_loaded_placements( continue; } // Resolve the MCSH ground-shade the reference way: a GLOBAL world→tile→chunk lookup at - // the doodad's origin, independent of which tile registered it or in what order. An ADT - // map doodad on lit ground takes the boosted ADT sun level (`ShadeSel::Lit` — the - // binary's 2.5, wow-re m2-interior-doodad-base-light §6). + // the doodad's origin, independent of which tile registered it or in what order. + // + // **An ADT map doodad is the FIXED-1.0 family, not the boosted one** (2050). + // This site read `ShadeSel::Lit` (the 2.5 target) on the strength of wow-re + // `m2-interior-doodad-base-light` §6, and §6 was wrong: a `CMapDoodadDef`'s + // `[+0xa4]` is only ever {0.0, 0.5, 1.0}, and the class does not merely fail to + // ramp — at `+0xf8`, where the ramp `0x69e770` reads its target, the doodad + // class holds `m[2][3]` of its own world matrix. The 2.5 and the 3.3333/s chase + // belong to the WENTITY light node (vtable `0x810810`, hung off `[obj+0xe0]`), + // which a doodad has none of. Measured as well as read: 111 identified MDDF + // placements over six trace frames, 356 draws, gains only ∈ {0.5, 1.0}, none + // changing — while an entity walking out of shadow climbed 0.5 → 0.81 → 1.09 → + // 1.3367 in the same frames. wow-re `models/scratch/ + // adt-doodad-sun-scale-vs-entity-node.md`. + // + // So this is the same family the exterior WMO MODD prop takes, and for the same + // reason: they are one C++ class. Nothing renders differently today — the + // shader's `min(I, 1)` already flattened the two — but the scale being + // per-class is what has to be true BEFORE that cap can come off. let shade = match doodad_ground_shade(&streamer, &adt_tiles, p.transform.translation) { ShadeResolve::Ready(true) => ShadeSel::Shaded, - ShadeResolve::Ready(false) => ShadeSel::Lit, + ShadeResolve::Ready(false) => ShadeSel::Matte, // The doodad's own ground tile is requested but still decoding — wait, so we don't // bake the lit fallback into a straddling tree whose true tile lands a frame later. ShadeResolve::Pending => continue,