diff --git a/crates/benilla-app/src/ui_script/world_entry_tests.rs b/crates/benilla-app/src/ui_script/world_entry_tests.rs index f15c560e..adee4c3f 100644 --- a/crates/benilla-app/src/ui_script/world_entry_tests.rs +++ b/crates/benilla-app/src/ui_script/world_entry_tests.rs @@ -987,6 +987,9 @@ fn a_repeating_error_is_one_row_with_a_count_not_a_flood() { /// userPlaced bit (`SetUserPlaced` refuses a frame that is neither movable nor resizable). /// Parentless, so its anchor is the screen root — the file's `-` target — which is what lets this /// need no FrameXML and no install. +/// **Both flags, deliberately**: the layout cache's apply is gated per arm — position behind +/// `movable`, size behind `resizable` (decision 2193) — so a probe standing in for a window the +/// player both moved and resized has to carry both, or half its geometry is correctly left behind. fn place_a_window(world: &mut World) { world .get_non_send_resource_mut::() @@ -995,7 +998,7 @@ fn place_a_window(world: &mut World) { "local f = CreateFrame(\"Frame\", \"B353Probe\") \ f:SetWidth(413) f:SetHeight(147) \ f:SetPoint(\"BOTTOMLEFT\", 61, 29) \ - f:SetMovable(true) f:SetUserPlaced(true)", + f:SetMovable(true) f:SetResizable(true) f:SetUserPlaced(true)", ) .expect("place the probe window"); } @@ -1099,6 +1102,10 @@ fn each_character_gets_its_own_layout_cache() { /// A window as FrameXML would author it — the shape the restore has to overwrite. Same name as /// [`place_a_window`]'s, different geometry, and **not** user-placed: this is the fresh tree a /// relog meets. +/// +/// It carries the same `movable`/`resizable` pair, because those are the window's **authored** +/// state — XML attributes on a real resizable window, rebuilt with it — and the layout cache's +/// apply reads them off the live frame to decide which arm runs (decision 2193). fn author_a_window(world: &mut World) { world .get_non_send_resource_mut::() @@ -1107,7 +1114,7 @@ fn author_a_window(world: &mut World) { "local f = CreateFrame(\"Frame\", \"B353Probe\") \ f:SetWidth(100) f:SetHeight(100) \ f:SetPoint(\"BOTTOMLEFT\", 0, 0) \ - f:SetMovable(true)", + f:SetMovable(true) f:SetResizable(true)", ) .expect("author the probe window"); } diff --git a/crates/benilla-app/src/ui_script/world_map_tests.rs b/crates/benilla-app/src/ui_script/world_map_tests.rs index b2b9149c..9e697e79 100644 --- a/crates/benilla-app/src/ui_script/world_map_tests.rs +++ b/crates/benilla-app/src/ui_script/world_map_tests.rs @@ -814,3 +814,104 @@ fn the_real_feralas_catalog_names_dire_maul_under_the_cursor() { "the stock OnUpdate puts the hovered sub-area's name in the label" ); } + +/// **A layout row an addon stamped does not outlive the addon** — director report: the world map +/// stopped opening full screen, "leaving the left and bottom open", on a character whose addons +/// were all disabled. +/// +/// `Cartographer/Modules/LookNFeel.lua` windows the map on enable — `SetMovable(true)`, +/// `SetResizable(true)`, `SetWidth(1024)`, `SetHeight(768)`, `StartMoving(); StopMovingOrSizing()`, +/// then `SetPoint("CENTER", UIParent, "CENTER", db.x, db.y)` — and the drag entry stamps the +/// userPlaced bit on the way through (`0x7652b0` @`0x7652e5`, unconditionally). benilla's layout +/// cache was gated on that bit **alone**, so it persisted the row and re-seated it at every login +/// on every character, replacing the stock `setAllPoints="true"` forever; the blackout, which +/// `WorldMapFrame_OnLoad` sizes to the screen and anchors BOTTOMLEFT of the map, slid up-and-right +/// with it and left the world showing at the left and the bottom. +/// +/// The reference gates both ends on `movable|resizable` as well (`0x490e97 test ah,0x3` at the +/// writer; `0x490600 test ah,0x1` / `0x490689 test ah,0x2` at the two arms of the apply), and +/// stock `WorldMapFrame` carries neither flag — so the stale row is inert the first session the +/// addon does not load, and falls out of the file at that logout. Decision 2193. +#[test] +fn a_stale_layout_row_cannot_seat_a_stock_frame() { + let _data = benilla_formats::wow_data_or_skip!(); + let mut s = UiScript::new().unwrap(); + s.set_screen_size(1600.0, 900.0); + s.set_unit( + "player", + Some(benilla_ui::script::UnitState { + exists: true, + name: Some("Probefour".into()), + level: 60, + ..Default::default() + }), + ); + let failures = super::load_default_ui(&s); + assert!(failures.is_empty(), "manifest load errors: {failures:#?}"); + s.resolve(); + + // The row verbatim as an affected `benilla-config/layout/-.txt` carries it: + // Cartographer's own saved `LookNFeel` profile offsets, and the size it forces. + let row = || benilla_ui::script::FrameLayout { + name: "WorldMapFrame".into(), + width: 1024.0, + height: 768.0, + points: vec![benilla_ui::script::LayoutPoint { + point: "CENTER".into(), + relative_to: Some("UIParent".into()), + relative_point: "CENTER".into(), + x: -0.6369222, + y: 53.861614, + }], + }; + + s.restore_user_placed_layouts([row()]); + s.resolve(); + s.run("ShowUIPanel(WorldMapFrame)").unwrap(); + s.resolve(); + + let (w, h, sw, sh) = s + .eval::<(f64, f64, f64, f64)>( + "return WorldMapFrame:GetWidth(), WorldMapFrame:GetHeight(), \ + GetScreenWidth(), GetScreenHeight()", + ) + .unwrap(); + assert_eq!( + (w, h), + (sw, sh), + "the stale row seated nothing — the map is still the screen, as its setAllPoints authored it" + ); + assert!( + !s.eval::("return WorldMapFrame:IsUserPlaced()") + .unwrap(), + "and the apply's userPlaced stamp lives inside the position arm it never took" + ); + assert!( + s.user_placed_layouts().is_empty(), + "so the row falls out of the file at this logout instead of being written again" + ); + + // The positive control: the gate is the frame's own flags, not a special case for this frame. + // With the addon's `SetMovable`/`SetResizable` in place the same row seats exactly as before. + s.run("WorldMapFrame:SetMovable(true) WorldMapFrame:SetResizable(true)") + .unwrap(); + s.restore_user_placed_layouts([row()]); + s.resolve(); + let (w, h) = s + .eval::<(f64, f64)>("return WorldMapFrame:GetWidth(), WorldMapFrame:GetHeight()") + .unwrap(); + assert_eq!( + (w, h), + (1024.0, 768.0), + "a movable+resizable frame takes it" + ); + assert!( + s.eval::("return WorldMapFrame:IsUserPlaced()") + .unwrap(), + "and the position arm stamps the bit it was written under" + ); + let saved = s.user_placed_layouts(); + assert_eq!(saved.len(), 1, "which is what puts the row back: {saved:?}"); + assert_eq!(saved[0], row(), "unchanged through the round trip"); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} diff --git a/crates/benilla-ui/src/script/layout_cache.rs b/crates/benilla-ui/src/script/layout_cache.rs index d4514050..b62bac21 100644 --- a/crates/benilla-ui/src/script/layout_cache.rs +++ b/crates/benilla-ui/src/script/layout_cache.rs @@ -1,10 +1,16 @@ //! The **layout cache** — the consumer `Frame::user_placed` never had. //! //! `SetUserPlaced` / the drag entries set the client's userPlaced bit (`frame+0xb4 & 0x1000`, -//! [`super::object::movable`]), and its whole meaning is *"the player put this window here; put it -//! back next time"*. The real client honours that by writing a per-character `layout-cache.txt` at -//! logout and seating the frames from it at load; benilla had the bit and nothing that read it, so -//! a dragged or resized window went back to its authored anchors on the next login. +//! [`super::object::movable`]), and together with the frame's own `movable`/`resizable` flags it +//! means *"the player put this window here; put it back next time"*. The real client honours that +//! by writing a per-character `layout-cache.txt` at logout and seating the frames from it at load; +//! benilla had the bit and nothing that read it, so a dragged or resized window went back to its +//! authored anchors on the next login. +//! +//! **The bit is not the whole gate**, at either end — see [`super::UiScript::user_placed_layouts`] +//! and [`super::UiScript::restore_user_placed_layouts`] for the conjuncts and their byte sites. +//! Reading it as the whole gate is what let one addon's stamp seat a stock frame forever +//! (decision 2193). //! //! This module is the **engine half** of that: a snapshot out, a restore in, and a dirty bit for //! the host to debounce on. Where the file lives, when it is written and which character owns it @@ -81,18 +87,30 @@ pub struct FrameLayout { } impl super::UiScript { - /// Snapshot every frame carrying the userPlaced bit — the save path. + /// Snapshot every frame the reference's writer would write — the save path. /// /// Sorted by name so the file is stable: a rewrite that reorders its own lines makes every /// diff useless and every "did this change?" check a false positive. Frames whose anchors /// cannot be re-addressed by name are dropped (see the module doc), as are unnamed ones — /// there is nothing to write them under. + /// + /// **The filter is `userPlaced AND (movable OR resizable)`, not the bit alone.** The + /// reference's per-frame writer `0x490e60` tests four conjuncts in a row: a non-NULL name + /// (`0x490e79`), a non-empty one (`0x490e82`), `userPlaced` (`0x490e8e test ah,0x10` = + /// `0x1000`) and **`movable|resizable`** (`0x490e97 test ah,0x3` = `0x100|0x200`). The bit + /// alone was benilla's whole filter until decision 2193, and the missing fourth conjunct is + /// what made this cache a one-way ratchet: **every** drag entry stamps `userPlaced` + /// (`0x7652b0` @`0x7652e5`, with no cursor-movement dependency), so an addon that windows a + /// stock frame and stamps it in passing — `Cartographer/Modules/LookNFeel.lua`'s + /// `StartMoving(); StopMovingOrSizing()` over `WorldMapFrame` — wrote a row that outlived + /// the addon on every character, forever. The reference drops that row at the first logout + /// the addon does not load, because the frame's own `movable`/`resizable` flags went with it. pub fn user_placed_layouts(&self) -> Vec { let model = self.model_ref(); let mut out: Vec = model .arena .iter_frames() - .filter(|(_, f)| f.user_placed) + .filter(|(_, f)| f.user_placed && (f.movable || f.resizable)) .filter_map(|(h, f)| { let name = f.name.clone()?; let input = model.layout_inputs.get(&h)?; @@ -121,6 +139,18 @@ impl super::UiScript { /// the position: `UIParent_ManageFramePositions` skips a user-placed frame, so the managed /// bottom-stack pass does not re-seat the window the player moved. /// + /// **The apply is gated per ARM, not per row** (`0x4905e0`, decision 2193): the position arm + /// is behind the frame's own `movable` bit (`0x490600 test ah,0x1`) and the size arm behind + /// `resizable` (`0x490689 test ah,0x2`) — a frame carrying neither is left entirely alone, + /// however old and however stamped its row is. The userPlaced stamp itself lives **inside** + /// each arm (`0x49067e` in the position arm, `0x490706` in the size one), after that arm's + /// own "the row carries no position" bail + /// (`0x490613`, the reference's `X == 0 && Y == 0`; the size arm's twin is `0x49069a`'s + /// `W == 0 && H == 0`). Both bails are spelled in this payload's own terms rather than the + /// reference's integer sentinels: a row that carries no anchors, and a row that carries no + /// size, are the same two states in a format that stores an anchor list instead of an X/Y + /// pair. + /// /// It marks nothing dirty — the values came *from* the file, and an echo would re-dirty what /// was just read (`set_chat_window_looks`' reasoning, one store over). pub fn restore_user_placed_layouts(&mut self, layouts: impl IntoIterator) { @@ -156,17 +186,37 @@ impl super::UiScript { if !usable { continue; } + // The two arm gates, read before the layout input is borrowed mutably. + let Some((movable, resizable)) = model.arena.frame(h).map(|f| (f.movable, f.resizable)) + else { + continue; + }; + let seat_position = movable && !anchors.is_empty(); + let seat_size = resizable && (l.width != 0.0 || l.height != 0.0); + if !seat_position && !seat_size { + continue; + } let Some(input) = model.layout_inputs.get_mut(&h) else { continue; }; - let changed = input.anchors != anchors - || input.width.to_bits() != l.width.to_bits() - || input.height.to_bits() != l.height.to_bits(); - input.anchors = anchors; - input.width = l.width; - input.height = l.height; - if let Some(f) = model.arena.frame_mut(h) { - f.user_placed = true; + let mut changed = false; + if seat_position { + changed |= input.anchors != anchors; + input.anchors = anchors; + } + if seat_size { + changed |= input.width.to_bits() != l.width.to_bits() + || input.height.to_bits() != l.height.to_bits(); + input.width = l.width; + input.height = l.height; + } + // The stamp is EACH arm's, not the row's: the position arm's is `0x49067e` and the + // size arm's `0x490706`, both inside their own gate. A frame that took neither is + // left un-stamped, which is what lets its row fall out of the file. + if seat_position || seat_size { + if let Some(f) = model.arena.frame_mut(h) { + f.user_placed = true; + } } if changed { // The whole graph, not the frame alone: a restore REPOINTS anchors (a saved row diff --git a/crates/benilla-ui/src/script/tests/movable.rs b/crates/benilla-ui/src/script/tests/movable.rs index 56c67408..f3462a61 100644 --- a/crates/benilla-ui/src/script/tests/movable.rs +++ b/crates/benilla-ui/src/script/tests/movable.rs @@ -615,3 +615,117 @@ fn region_getters_answer_in_the_owners_units_under_scale() { ); assert!((cx - (fl + fr) * 0.5).abs() < 1e-3); } + +/// **The layout cache's filter is the flags AND the bit, at both ends** (decision 2193). +/// +/// `SetUserPlaced` is already guarded by `movable|resizable` at its own setter (`0x776adb`), but +/// the drag entry (`0x7652b0` @`0x7652e5`) and the cache's own apply stamp the bit without going +/// through it — so a frame can carry the stamp while carrying neither flag, and the reference's +/// writer tests for both: `0x490e8e test ah,0x10` (userPlaced) AND `0x490e97 test ah,0x3` +/// (`movable|resizable`). Clearing the flags is how a window stops being persisted, which is what +/// an addon's `:OnDisable` does on the way out. +#[test] +fn the_write_filter_is_user_placed_and_movable_or_resizable() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + F = CreateFrame("Frame", "CachePanel") + F:SetPoint("BOTTOMLEFT", 10, 10); F:SetWidth(50); F:SetHeight(50) + F:SetResizable(true); F:SetUserPlaced(true) + "#, + ) + .unwrap(); + let names = |s: &UiScript| { + s.user_placed_layouts() + .into_iter() + .map(|l| l.name) + .collect::>() + }; + assert_eq!(names(&s), ["CachePanel"], "stamped and resizable — written"); + + s.run("CachePanel:SetResizable(false)").unwrap(); + assert!( + s.eval::("return CachePanel:IsUserPlaced()").unwrap(), + "the stamp itself survives the flag going away — nothing clears it" + ); + assert!( + names(&s).is_empty(), + "but the row does not: the fourth conjunct is gone" + ); + + s.run("CachePanel:SetMovable(true)").unwrap(); + assert_eq!(names(&s), ["CachePanel"], "either flag satisfies it"); + assert!(s.errors().is_empty(), "{:?}", s.errors()); +} + +/// **The apply is gated per ARM** (decision 2193): position behind `movable` (`0x490600 test +/// ah,0x1`), size behind `resizable` (`0x490689 test ah,0x2`), and each arm stamps the userPlaced +/// bit itself (`0x49067e` / `0x490706`) only if it ran. A stock frame carrying neither flag is +/// left entirely alone, however old its row — which is what stops one addon's stamp from seating +/// a window forever. +#[test] +fn the_apply_seats_position_behind_movable_and_size_behind_resizable() { + use crate::script::{FrameLayout, LayoutPoint}; + + let row = |name: &str| FrameLayout { + name: name.to_owned(), + width: 200.0, + height: 80.0, + points: vec![LayoutPoint { + point: "BOTTOMLEFT".into(), + relative_to: None, + relative_point: "BOTTOMLEFT".into(), + x: 300.0, + y: 200.0, + }], + }; + // Three frames, one per flag state, all authored identically. + let mut s = script(); + s.set_screen_size(800.0, 600.0); + for (name, flags) in [ + ("Neither", ""), + ("Movable", "F:SetMovable(true)"), + ("Sizable", "F:SetResizable(true)"), + ] { + s.run(&format!( + r#"F = CreateFrame("Frame", "{name}") + F:SetPoint("BOTTOMLEFT", 10, 10); F:SetWidth(50); F:SetHeight(50) + {flags}"# + )) + .unwrap(); + s.restore_user_placed_layouts([row(name)]); + } + s.resolve(); + let read = |s: &UiScript, n: &str| { + s.eval::<(f64, f64, f64, bool)>(&format!( + "local f = getglobal('{n}') \ + return f:GetLeft(), f:GetWidth(), f:GetHeight(), f:IsUserPlaced()" + )) + .unwrap() + }; + assert_eq!( + read(&s, "Neither"), + (10.0, 50.0, 50.0, false), + "neither flag: the row is inert, and leaves no stamp behind either" + ); + assert_eq!( + read(&s, "Movable"), + (300.0, 50.0, 50.0, true), + "movable: seated at the saved position, still its authored size" + ); + assert_eq!( + read(&s, "Sizable"), + (10.0, 200.0, 80.0, true), + "resizable: given the saved size, left on its authored anchors" + ); + assert_eq!( + s.user_placed_layouts() + .into_iter() + .map(|l| l.name) + .collect::>(), + ["Movable", "Sizable"], + "and only the two that took an arm are written back" + ); + assert!(s.errors().is_empty(), "{:?}", s.errors()); +} diff --git a/crates/benilla-ui/src/widget/mod.rs b/crates/benilla-ui/src/widget/mod.rs index f4c08727..033f28c2 100644 --- a/crates/benilla-ui/src/widget/mod.rs +++ b/crates/benilla-ui/src/widget/mod.rs @@ -325,9 +325,14 @@ pub struct Frame { /// The upper twin of [`Frame::min_resize`] — see its doc, including the `0.0` sentinel. pub max_resize: (f32, f32), /// `SetUserPlaced` — the client's "the user placed this frame; persist its position across - /// sessions" bit. Default false. Stored and readable (`IsUserPlaced`); **nothing consumes it - /// yet** — persisting a frame's position belongs with the layout cache, not with the drag that - /// moved it, so the flag lands here and the saving lands with the cache. + /// sessions" bit. Default false, readable through `IsUserPlaced`, and consumed by the layout + /// cache ([`crate::script::layout_cache`]) — which is where persisting a frame's position + /// belongs, rather than with the drag that moved it. + /// + /// **Necessary, not sufficient.** Every drag entry stamps this bit unconditionally + /// (`0x7652b0` @`0x7652e5`), so an addon that drags a stock frame once stamps it too; the + /// cache's own filter is this bit AND [`Frame::movable`]`|`[`Frame::resizable`], at both the + /// write and the apply, which is what lets the stamp fall off again (decision 2193). pub user_placed: bool, /// `SetToplevel` / XML `toplevel` — flag word `[frame+0xb4]` **bit `0x1`**, the same word as /// [`Frame::movable`] (`0x100`) and [`Frame::resizable`] (`0x200`), written by the same pure