diff --git a/crates/benilla-app/src/ui_models/mod.rs b/crates/benilla-app/src/ui_models/mod.rs index 419acaf7..a8eecc0a 100644 --- a/crates/benilla-app/src/ui_models/mod.rs +++ b/crates/benilla-app/src/ui_models/mod.rs @@ -555,16 +555,93 @@ const GUTTER: u32 = 2; const TILE_CAMERA_ORDER: isize = -10; /// The renderer's per-frame state that is not the bridge. +/// +/// **Most of this is about a VM, and the VM does not live for the process** (decision 1290): it +/// is built at world entry and destroyed at the character screen, and `ReloadUI()` is both edges +/// back to back. [`TileState::session`] is what keeps that honest — see [`TileState::adopt_vm`] +/// and [`TileState::answer_facts`]. #[derive(Default)] struct TileState { + /// The VM these tiles and the bridge's handle-keyed maps belong to + /// ([`UiScript::session`]); `0` = none (the "no VM" branch, and a freshly built state). + session: u64, tiles: HashMap, /// Files the engine asked facts for, loading. pending_facts: HashMap>, - /// Files whose facts were handed over — the handle kept alive for the tiles. - loaded: HashMap>, + /// Files whose facts have been derived: the asset handle (which keeps the file resident for + /// the tiles) beside the facts themselves. + /// + /// **The facts are cached because they are owed to every VM, not to the process.** This map + /// is a HOST fact — "the file is loaded here" — and it stood in for the per-VM fact "this + /// VM's engine has been told about the file" until [`TileState::answer_facts`]; the module + /// doc of `crate::ui_script::session` names that class and why it has no error path. + loaded: HashMap, ModelFileFacts)>, frame: u64, } +impl TileState { + /// Hand the engine the facts for every file it just asked about, and answer with the keys + /// that still need loading (not resident here, and not already in flight). + /// + /// **Every ask gets an answer, whichever VM is asking.** `model_facts_wanted` DRAINS, and the + /// only thing that re-pushes a want is `SetModel` — so a want that is dropped is dropped for + /// the life of that VM. Skipping the answer because the file was already loaded *for an + /// earlier VM* is therefore permanent: [`UiScript::visible_model_panes`] drops a pane whose + /// file the engine knows nothing about, so from the second world entry on, every `` + /// widget in the game — the cooldown pie and the GCD sweep, the autocast shine, the minimap + /// and world-map pings, the item-push card, the map arrow — went dark until the process was + /// restarted. That is decision 1290's class exactly: a host memory standing in for a per-VM + /// one, failing silently, with the window simply empty. + fn answer_facts(&mut self, script: &mut UiScript) -> Vec { + let mut to_load = Vec::new(); + for key in script.model_facts_wanted() { + if let Some((_, facts)) = self.loaded.get(&key) { + if trace_on() { + info!("tile-trace: facts for {key} answered from residency"); + } + script.set_model_facts(&key, facts.clone()); + continue; + } + // Already in flight: the landing below answers whichever VM is asking by then. + if self.pending_facts.contains_key(&key) { + continue; + } + to_load.push(key); + } + to_load + } + + /// Adopt the VM `script` names — **forgetting every handle-keyed memory when it is a + /// different one than these tiles were built against**. + /// + /// A [`FrameHandle`] is a generational index into ONE VM's widget arena, and the VM is + /// rebuilt at every logout, login and `ReloadUI()` (1290/1291). The next VM starts a fresh + /// arena and reissues the SAME indices at the same generations, so a tile or a request that + /// outlives its VM is not merely stale: it names a different frame. `None` is "no VM" — a + /// session in its own right (session `0`), exactly as [`crate::ui_script::VmMemo`] treats the + /// character screen. + /// + /// **When** this runs is load-bearing: [`forget_dead_vm_tiles`], ahead of the extract. + fn adopt_vm( + &mut self, + script: Option<&UiScript>, + bridge: &mut UiModelTiles, + commands: &mut Commands, + table: &mut MatAnimTable, + ) { + let session = script.map_or(0, UiScript::session); + if self.session == session { + return; + } + self.session = session; + for (_, tile) in self.tiles.drain() { + tile.retire(commands, table); + } + bridge.requests.clear(); + bridge.cells.clear(); + } +} + pub(crate) struct UiModelsPlugin; impl Plugin for UiModelsPlugin { @@ -572,9 +649,20 @@ impl Plugin for UiModelsPlugin { app.init_resource::() .init_non_send_resource::() .add_systems(Startup, setup_tiles) + // **Before the extract**, which is where a `` pane's tile request is + // published (`UiInput`'s `drive_script`) — see [`forget_dead_vm_tiles`]. + .add_systems( + Update, + forget_dead_vm_tiles.before(crate::ui_script::UiInput), + ) // After the extract published this frame's requests, and before the pose/palette // passes read the roots' transforms (they run in PostUpdate). - .add_systems(Update, sync_tiles.after(crate::ui_script::UiInput)) + .add_systems( + Update, + sync_tiles + .after(crate::ui_script::UiInput) + .after(forget_dead_vm_tiles), + ) // The composite: this frame's cells, appended in the lane the minimap fill uses — // after the cells are packed, before the mesh rebuild reads the lane. .add_systems(Update, compose_tiles.in_set(UiQuadAppend).after(sync_tiles)) @@ -707,6 +795,26 @@ struct TileRender<'w> { anim_mirrors: ResMut<'w, MatAnimMirrors>, } +/// **The VM edge** ([`TileState::adopt_vm`]) — its own system, and ordered **ahead of the +/// extract**, which is the whole reason it is not a first step inside [`sync_tiles`]. +/// +/// A logout, a login and a `ReloadUI()` all replace the VM in `PreUpdate`; the extract +/// (`ui_script`'s `drive_script`, in `UiInput`) then publishes the NEW tree's tile requests, and +/// `sync_tiles` reads them after that. Clearing the bridge from inside `sync_tiles` would +/// therefore throw away the new VM's very first publish — and the extract's conversion is +/// memoized on the engine's entry list, so an entry that does not change again is never converted +/// again and the request never comes back (decision 2023's defect, from the other side). Running +/// on the frame's way IN puts the clear before the publish instead of after it. +fn forget_dead_vm_tiles( + mut commands: Commands, + script: Option>, + mut state: NonSendMut, + mut bridge: ResMut, + mut table: ResMut, +) { + state.adopt_vm(script.as_deref(), &mut bridge, &mut commands, &mut table); +} + /// The per-frame pass: feed the engine the facts it asked for, keep one tile per visible pane, /// pack the atlas, place every tile at its cell and its play head, and aim the camera. #[allow(clippy::too_many_arguments, clippy::type_complexity)] // a Bevy system's full input set @@ -762,10 +870,9 @@ fn sync_tiles( state.frame += 1; let frame = state.frame; let Some(mut script) = script else { - // No VM: nothing paints. Tear everything down so a dead UI leaves no live camera. - for (_, tile) in state.tiles.drain() { - tile.retire(&mut commands, &mut render.table); - } + // No VM: nothing paints. The tiles were retired on the edge + // ([`forget_dead_vm_tiles`]); every frame after it must still composite nothing and + // leave no live camera. bridge.cells.clear(); set_camera_active(&mut cams, false); for (mut cam, _, _, _, _) in &mut pane_cams { @@ -774,10 +881,11 @@ fn sync_tiles( return; }; - // ── 1. Facts: what the engine asked for, answered when the file lands ─────────────── - for key in script.model_facts_wanted() { - if state.loaded.contains_key(&key) || state.pending_facts.contains_key(&key) { - continue; + // ── 1. Facts: what the engine asked for — answered now when the file is already here + // ([`TileState::answer_facts`]), when it lands otherwise ───────────────────── + for key in state.answer_facts(&mut script) { + if trace_on() { + info!("tile-trace: facts for {key} wanted — loading the file"); } let handle = assets.asset_server.load::(m2_url(&key)); state.pending_facts.insert(key, handle); @@ -789,11 +897,15 @@ fn sync_tiles( .map(|(k, h)| (k.clone(), h.clone())) .collect(); for (key, handle) in landed { - if let Some(model) = assets.m2s.get(&handle) { - script.set_model_facts(&key, facts_of(model)); - } + let Some(model) = assets.m2s.get(&handle) else { + continue; // not readable yet — stay in `pending_facts` rather than fall off the loop + }; + let facts = facts_of(model); + script.set_model_facts(&key, facts.clone()); state.pending_facts.remove(&key); - state.loaded.insert(key, handle); + // Cached beside the handle so the next VM's want is answered without re-reading the + // asset — and so the answer cannot depend on the asset still being resident. + state.loaded.insert(key, (handle, facts)); } // ── 2. The paint list, and one tile per pane on it ────────────────────────────────── @@ -877,7 +989,7 @@ fn sync_tiles( for (i, (handle, req, _)) in live.iter().enumerate() { let key = benilla_ui::widget::model_key(&req.path); - let Some(m2) = state.loaded.get(&key).cloned() else { + let Some(m2) = state.loaded.get(&key).map(|(h, _)| h.clone()) else { if trace_on() { info!("tile-trace: {} facts not landed (key {key})", req.path); } @@ -2062,6 +2174,198 @@ mod tests { ); } + /// `UI-Cooldown-Indicator.m2` as `benilla-extract m2seq` reads it: two clamped 1000 ms + /// sequences, ids 0 (the sweep) and 1 (the flash). + const COOLDOWN_FILE: &str = r"Interface\Cooldown\UI-Cooldown-Indicator.mdx"; + fn cooldown_facts() -> ModelFileFacts { + ModelFileFacts { + sequences: vec![ + SequenceFacts { + anim_id: 0, + duration_ms: 1000, + looping: false, + }, + SequenceFacts { + anim_id: 1, + duration_ms: 1000, + looping: false, + }, + ], + bbox: ([0.0; 3], [0.0; 3]), + cameras: 0, + } + } + + /// One freshly built VM with a shown cooldown pane on it — the shape `Cooldown.xml` builds + /// per action button, and the shape `!OmniCC` wraps `CooldownFrame_SetTimer` around. + fn vm_with_a_cooldown_pane() -> UiScript { + let mut s = UiScript::new().expect("VM"); + s.set_screen_size(1024.0, 768.0); + s.run(&format!( + r#"cd = CreateFrame("Model", "CD", UIParent) cd:SetWidth(36) cd:SetHeight(36) + cd:SetPoint("CENTER", UIParent, "CENTER", 0, 0) cd:SetModel("{}")"#, + COOLDOWN_FILE.replace('\\', "\\\\") + )) + .expect("build the pane"); + s.resolve(); + s + } + + /// **A rebuilt VM is told about a file the host already loaded** — decision 1290's class, + /// found in the tile renderer's facts cache. + /// + /// `UiScript::model_facts_wanted` DRAINS, and the only thing that re-pushes a want is + /// `SetModel`; `visible_model_panes` drops a pane whose file the engine holds no facts for. + /// So an answer skipped once is skipped for that VM's whole life. The host used to skip it + /// whenever the ASSET was resident — a fact about the process, not about the VM — and the VM + /// is rebuilt at every logout, login and `ReloadUI()`. From the second world entry on, every + /// `` widget in the game (the cooldown pie and the GCD sweep, the autocast shine, the + /// minimap and world-map pings, the item-push card, the map arrow) drew nothing at all, and + /// only restarting the client brought them back. + #[test] + fn a_rebuilt_vm_is_told_about_a_file_the_host_already_loaded() { + let key = benilla_ui::widget::model_key(COOLDOWN_FILE); + let mut state = TileState::default(); + + // Session 1 — nothing is resident, so the host is asked to load the file. + let mut first = vm_with_a_cooldown_pane(); + assert!( + first.visible_model_panes().is_empty(), + "no facts yet ⇒ the pane is not on the paint list (the reference's draw gate)" + ); + assert_eq!(state.answer_facts(&mut first), vec![key.clone()]); + // …the asset lands: the facts are handed over and cached beside the handle. + state + .loaded + .insert(key.clone(), (Handle::default(), cooldown_facts())); + first.set_model_facts(COOLDOWN_FILE, cooldown_facts()); + assert_eq!(first.visible_model_panes().len(), 1); + + // Session 2 — the logout/login (or `/reload`) rebuild: a fresh VM, the same file. + let mut second = vm_with_a_cooldown_pane(); + assert!( + second.visible_model_panes().is_empty(), + "a fresh VM starts knowing nothing about any file" + ); + assert!( + state.answer_facts(&mut second).is_empty(), + "the file is resident here — there is nothing left to load" + ); + assert!( + second.has_model_facts(COOLDOWN_FILE), + "the want was drained; if it is not answered NOW it is never answered again" + ); + assert_eq!( + second.visible_model_panes().len(), + 1, + "the second session's cooldown pane must paint exactly like the first's" + ); + } + + /// **A new VM inherits nothing keyed by a `FrameHandle`** — the same edge, its other half. + /// + /// A handle is a generational index into ONE arena; the next VM reissues the same indices at + /// the same generations, so a surviving tile or request does not go stale, it silently + /// re-attaches to a different frame. The tile's tree is despawned and its mat-anim rows go + /// back to the table, exactly as a reaped tile's do. + #[test] + fn a_new_vm_inherits_nothing_keyed_by_a_frame_handle() { + let mut world = World::new(); + let root = world.spawn_empty().id(); + let mut table = MatAnimTable::default(); + let mut bridge = UiModelTiles::default(); + let mut state = TileState::default(); + + let mut arena = benilla_ui::widget::WidgetArena::new(); + let handle = arena.create(benilla_ui::widget::FrameKind::Frame, None, None); + let clip_slot = table.alloc().expect("a fresh table has rows"); + state.tiles.insert( + handle, + Tile { + root, + key: benilla_ui::widget::model_key(COOLDOWN_FILE), + icon: None, + m2: Handle::default(), + built: true, + light_slot: 0, + cam_slot: None, + clips: HashMap::new(), + armed: None, + clip_slot: Some(clip_slot), + alpha_parts: Vec::new(), + uv_parts: Vec::new(), + emitters: Vec::new(), + last_seen: 1, + parked: false, + }, + ); + bridge.requests.insert( + handle, + TileRequest { + path: COOLDOWN_FILE.into(), + size_px: UVec2::new(36, 36), + 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, 36.0, 36.0), + z_key: 1, + alpha: 1.0, + clip: None, + }, + ); + bridge.cells.insert( + handle, + Cell { + origin: UVec2::splat(2), + size: UVec2::splat(36), + }, + ); + + let one = UiScript::new().expect("VM"); + let two = UiScript::new().expect("VM"); + assert_ne!(one.session(), two.session(), "each VM has its own identity"); + + let mut apply = |state: &mut TileState, bridge: &mut UiModelTiles, s: Option<&UiScript>| { + let mut queue = bevy::ecs::world::CommandQueue::default(); + { + let mut commands = Commands::new(&mut queue, &world); + state.adopt_vm(s, bridge, &mut commands, &mut table); + } + queue.apply(&mut world); + }; + + // Adopting the VM these tiles belong to changes nothing… + state.session = one.session(); + apply(&mut state, &mut bridge, Some(&one)); + assert_eq!(state.tiles.len(), 1); + assert_eq!(bridge.requests.len(), 1); + + // …and the rebuild drops the lot. + apply(&mut state, &mut bridge, Some(&two)); + assert!( + state.tiles.is_empty(), + "a dead VM's tiles must not be reused" + ); + assert!(bridge.requests.is_empty() && bridge.cells.is_empty()); + assert!( + world.get_entity(root).is_err(), + "the tile's tree goes with it — a live root would keep drawing into the atlas" + ); + assert_eq!( + table.alloc(), + Some(clip_slot), + "the tile's mat-anim rows go back to the table" + ); + } + /// 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_script/action_bar_tests.rs b/crates/benilla-app/src/ui_script/action_bar_tests.rs index b165c3b1..c6284755 100644 --- a/crates/benilla-app/src/ui_script/action_bar_tests.rs +++ b/crates/benilla-app/src/ui_script/action_bar_tests.rs @@ -367,6 +367,126 @@ fn the_cooldown_sweep_paints_over_the_buttons_icon_and_ring() { ); } +/// **A cooldown-count addon's hook leaves the sweep exactly where it was.** +/// +/// `!OmniCC` 6.8.30 — the cooldown addon on the director's screen — is a single wrap of the +/// FrameXML global: it captures `CooldownFrame_SetTimer` in an upvalue, replaces the global with +/// a function that calls through, and hangs a `Frame` + `FontString` off the button for its own +/// countdown, keeping the handle on a field of the cooldown widget itself (`cd.textFrame`). The +/// shape is transcribed here — the widget verbs it uses, not its source — because that shape +/// touches everything the pie's paint depends on: the global the bar calls, the widget's own +/// Lua fields (`start`/`duration`/`stopping`, which the stock `Cooldown.lua` writes and its +/// `OnUpdateModel` reads back), and the button's frame-level stack. +/// +/// It came in as "with `!OmniCC` installed the pie and the GCD sweep are gone", and this is the +/// half that answers whether the addon's hook itself is what breaks them. It is not: the engine +/// reports the same paint list, the same armed sequence and the same scrub with the wrap in +/// place as without it. (What did break them is the tile renderer's per-VM state — enabling an +/// addon costs a logout and a login, and the pane's model facts did not survive that; see +/// `ui_models`' own tests.) +#[test] +fn a_cooldown_count_addons_hook_leaves_the_sweep_running() { + use benilla_ui::script::ActionState; + + let mut s = UiScript::new().unwrap(); + s.set_screen_size(1024.0, 768.0); + load_action_bar(&s); + // The addon loads after FrameXML (`!` sorts it first among addons, all of which run after the + // interface), so the global it captures is the stock one. + s.run( + r#" + local original = CooldownFrame_SetTimer + seen = 0 + CooldownFrame_SetTimer = function(cd, start, duration, enable) + seen = seen + 1 + original(cd, start, duration, enable) + if start > 0 and duration > 3 and enable > 0 then + local count = cd.textFrame + if not count then + local icon = getglobal(cd:GetParent():GetName() .. "Icon") + if icon then + count = CreateFrame("Frame", nil, cd:GetParent()) + count:SetAllPoints(cd:GetParent()) + count:SetFrameLevel(count:GetFrameLevel() + 1) + count.text = count:CreateFontString(nil, "OVERLAY") + count.text:SetFontObject(GameFontNormal) + count.text:SetPoint("CENTER", count, "CENTER", 0, 1) + count.icon = icon + count:SetScript("OnUpdate", function() end) + cd.textFrame = count + end + end + if count then + count.start = start + count.duration = duration + count:Show() + end + elseif cd.textFrame then + cd.textFrame:Hide() + end + end + "#, + ) + .expect("the addon's hook installs"); + + s.set_action( + 1, + Some(ActionSlot { + texture: Some("Interface\\Icons\\Spell_Fire_FlameBolt".into()), + kind: 0x00, + action: 133, + count: 0, + consumable: false, + }), + ); + s.fire_event("PLAYER_ENTERING_WORLD", vec![]); + s.tick(10.0); + s.set_action_state( + 1, + Some(ActionState { + usable: true, + cooldown: Some((6_000, 10_000, true)), + ..Default::default() + }), + ); + s.fire_event("ACTIONBAR_UPDATE_COOLDOWN", vec![]); + super::test_ui::cooldown_facts(&mut s); + s.tick(0.0); + s.resolve(); + + assert!( + s.eval::("return seen").unwrap() > 0, + "the bar must reach the addon's replacement, not a captured original" + ); + assert!( + s.eval::("return ActionButton1Cooldown.textFrame ~= nil") + .unwrap(), + "the addon hangs its countdown off the cooldown widget — a field the widget must accept" + ); + assert_eq!( + super::test_ui::cooldown_play(&s, "ActionButton1Cooldown"), + Some((0, 400)), + "the wrap calls through, so the pane is on the paint list with sequence 0 at 40 %" + ); + // And it goes cold the reference's way when the cooldown is cleared. + s.set_action_state( + 1, + Some(ActionState { + usable: true, + ..Default::default() + }), + ); + s.fire_event("ACTIONBAR_UPDATE_COOLDOWN", vec![]); + s.tick(0.0); + s.resolve(); + assert_eq!( + super::test_ui::cooldown_play(&s, "ActionButton1Cooldown"), + None, + "an elapsed cooldown hides the pane through the wrap exactly as it does without it" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} + /// An action button is a TWO-button button (decision 0908; director's report B200: "I can't right /// click food on my bar to eat it or right click spells"). The ref's `ActionButton_OnLoad` /// registers `("LeftButtonUp", "RightButtonUp")` (ActionButton.lua:109) and its OnClick body reads diff --git a/crates/benilla-app/src/ui_script/addons.rs b/crates/benilla-app/src/ui_script/addons.rs index cb8ebe98..b55e26e6 100644 --- a/crates/benilla-app/src/ui_script/addons.rs +++ b/crates/benilla-app/src/ui_script/addons.rs @@ -2342,7 +2342,10 @@ mod tests { /// The written bytes are the recorded grammar (decision 1128's, itself byte-verified against /// `0x7043f0`/`0x704480`): `NAME = value`, bracketed keys, TAB indent, a trailing comma on - /// every entry, and the file split by scope. + /// every entry, and the file split by scope. `["t"]`'s `[1] = 1,` is the reference's shape for + /// a list too — its writer emits a bracketed key for every table shape and never a bare + /// positional entry (wow-re `system/ui/scratch/lua-table-storage-and-next-order.md` §Q5) — + /// and it reloads in index order because of what 1.12's *parser* does with it (decision 2111). /// /// Asserted as bytes rather than by re-reading, because "it round-trips through our own /// loader" is exactly the check that passes for a private format. The reference's own client diff --git a/crates/benilla-app/src/ui_script/battlefield_tests.rs b/crates/benilla-app/src/ui_script/battlefield_tests.rs index e9d9cf2f..c37ba6af 100644 --- a/crates/benilla-app/src/ui_script/battlefield_tests.rs +++ b/crates/benilla-app/src/ui_script/battlefield_tests.rs @@ -250,8 +250,10 @@ fn the_list_rows_carry_the_queue_status() { "(In Queue)" ); assert_eq!( - s.eval::("return BattlefieldZone1Status:GetText()") + s.eval::>("return BattlefieldZone1Status:GetText()") .unwrap(), - "" + None, + "an un-queued row's status line is blank, and a blank FontString reads back NIL \ + (`FontString:GetText 0x79d690` substitutes — decision 2110)" ); } diff --git a/crates/benilla-app/src/ui_script/extract/text.rs b/crates/benilla-app/src/ui_script/extract/text.rs index 47260422..d2630708 100644 --- a/crates/benilla-app/src/ui_script/extract/text.rs +++ b/crates/benilla-app/src/ui_script/extract/text.rs @@ -200,13 +200,24 @@ pub(super) fn emit( let ink = crate::ui_text::measure_text(&mut atlas.lock(), draw_text, None, spec).0; format!(" caret={:.1} ink={ink:.1}", ui.caret_x * host.scale) }); + // **The FACE and the FLAGS are on this line for a reason.** It used to print the + // requested height and nothing else about the font, so the one question a "the text + // looks wrong" report actually asks — *which face, at what size, with what outline, did + // this quad draw* — could not be answered from the probe at all; it took a live + // `SetFont` probe, a control plate and a screenshot to establish for MSBT what these + // four fields say directly (decisions 2103, 2112). `px` is the DRAWN logical height + // (`drawn_px`: the requested one through the cap, the 768 seam and the frame scale), + // which is the number that disagrees with `h` whenever a size looks wrong. info!( - "text probe: [{:.0},{:.0} {:.0}x{:.0}] h={:?}{} {:?}", + "text probe: [{:.0},{:.0} {:.0}x{:.0}] h={:?} px={:?} flags={:?} face={:?}{} {:?}", draw_rect.min.x, draw_rect.min.y, host.rect.width(), host.rect.height(), style.font_height, + spec.height, + style.outline.as_str(), + spec.path.unwrap_or(""), ebox_geom.unwrap_or_default(), &draw_text[..draw_text.len().min(60)] ); diff --git a/crates/benilla-app/src/ui_script/mirror_timer_tests.rs b/crates/benilla-app/src/ui_script/mirror_timer_tests.rs index e5cc313f..55382f7c 100644 --- a/crates/benilla-app/src/ui_script/mirror_timer_tests.rs +++ b/crates/benilla-app/src/ui_script/mirror_timer_tests.rs @@ -82,8 +82,10 @@ fn bar_color(s: &UiScript, frame: &str) -> (f64, f64, f64) { .unwrap() } -fn caption(s: &UiScript, frame: &str) -> String { - s.eval::(&format!("return {frame}Text:GetText()")) +/// `Option`, not `String`: a blank caption reads back **nil**, because `FontString:GetText +/// 0x79d690` substitutes nil for an empty string (decision 2110). +fn caption(s: &UiScript, frame: &str) -> Option { + s.eval::>(&format!("return {frame}Text:GetText()")) .unwrap() } @@ -126,7 +128,7 @@ fn breath_takes_the_first_bar_in_the_reference_blue() { assert!(shown(&s, "MirrorTimer1")); assert!(!shown(&s, "MirrorTimer2"), "only one timer is running"); - assert_eq!(caption(&s, "MirrorTimer1"), "Breath"); + assert_eq!(caption(&s, "MirrorTimer1").as_deref(), Some("Breath")); // Milliseconds on the wire, seconds in the bar — the reference's `/1000` on both. assert_eq!(bar_value(&s, "MirrorTimer1"), 45.0); assert_eq!(bar_max(&s, "MirrorTimer1"), 60.0); @@ -147,7 +149,7 @@ fn fatigue_is_the_reference_yellow_under_its_own_caption() { start(&mut s, "EXHAUSTION", 60_000, 60_000, -1, "Fatigue"); assert!(shown(&s, "MirrorTimer1")); - assert_eq!(caption(&s, "MirrorTimer1"), "Fatigue"); + assert_eq!(caption(&s, "MirrorTimer1").as_deref(), Some("Fatigue")); let (r, g, b) = bar_color(&s, "MirrorTimer1"); assert!( (r - 1.0).abs() < 1e-6 && (g - 0.9).abs() < 1e-6 && (b - 0.0).abs() < 1e-6, @@ -253,8 +255,8 @@ fn two_timers_stack_and_restate_in_place() { assert!(shown(&s, "MirrorTimer1") && shown(&s, "MirrorTimer2")); assert!(!shown(&s, "MirrorTimer3")); - assert_eq!(caption(&s, "MirrorTimer1"), "Fatigue"); - assert_eq!(caption(&s, "MirrorTimer2"), "Breath"); + assert_eq!(caption(&s, "MirrorTimer1").as_deref(), Some("Fatigue")); + assert_eq!(caption(&s, "MirrorTimer2").as_deref(), Some("Breath")); // The server re-sends breath on every change; it must reuse frame 2, not take frame 3. start(&mut s, "BREATH", 30_000, 60_000, -1, "Breath"); @@ -283,8 +285,8 @@ fn stop_hides_that_timer_and_frees_its_frame() { assert!(shown(&s, "MirrorTimer1")); assert_eq!( caption(&s, "MirrorTimer1"), - "", - "no FEIGNDEATH_LABEL exists in the 1.12 GlobalStrings" + None, + "no FEIGNDEATH_LABEL exists in the 1.12 GlobalStrings, and a blank caption reads nil" ); } diff --git a/crates/benilla-app/src/ui_script/pet_stable_tests.rs b/crates/benilla-app/src/ui_script/pet_stable_tests.rs index 57a1689b..a557d0a3 100644 --- a/crates/benilla-app/src/ui_script/pet_stable_tests.rs +++ b/crates/benilla-app/src/ui_script/pet_stable_tests.rs @@ -90,8 +90,10 @@ fn hunter_pet_stats() -> PetStats { } } -fn level_text(s: &UiScript) -> String { - s.eval::("return PetStableLevelText:GetText()") +/// `Option`, not `String`: an emptied line reads back **nil**, because `FontString:GetText +/// 0x79d690` substitutes nil for an empty string (decision 2110). +fn level_text(s: &UiScript) -> Option { + s.eval::>("return PetStableLevelText:GetText()") .unwrap() } @@ -123,7 +125,7 @@ fn call_pet_repaints_the_closed_stable_with_unknownobject_while_the_name_is_in_f !s.eval::("return PetStableFrame:IsVisible()").unwrap(), "UNIT_PET must repaint the window without opening it" ); - assert_eq!(level_text(&s), "Unknown Level 58 Boar"); + assert_eq!(level_text(&s).as_deref(), Some("Unknown Level 58 Boar")); assert_eq!( s.eval::("return PetStableCurrentPet.tooltip") .unwrap(), @@ -141,7 +143,7 @@ fn call_pet_repaints_the_closed_stable_with_unknownobject_while_the_name_is_in_f s.set_unit("pet", Some(pet(Some("Snarl")))); s.fire_event("PET_STABLE_UPDATE", vec![]); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); - assert_eq!(level_text(&s), "Snarl Level 58 Boar"); + assert_eq!(level_text(&s).as_deref(), Some("Snarl Level 58 Boar")); assert_eq!( s.eval::("return PetStableCurrentPet.tooltip") .unwrap(), @@ -164,7 +166,7 @@ fn dismissing_the_pet_empties_the_closed_stable_without_raising() { s.set_pet_stats(false, PetStats::default()); s.fire_event("UNIT_PET", vec![ScriptValue::Str("player".into())]); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); - assert_eq!(level_text(&s), ""); + assert_eq!(level_text(&s), None, "an emptied line reads back nil"); assert_eq!( s.eval::("return PetStableCurrentPet.tooltip") .unwrap(), diff --git a/crates/benilla-app/src/ui_script/questlog_tests.rs b/crates/benilla-app/src/ui_script/questlog_tests.rs index 21b27198..b670aaaf 100644 --- a/crates/benilla-app/src/ui_script/questlog_tests.rs +++ b/crates/benilla-app/src/ui_script/questlog_tests.rs @@ -771,22 +771,28 @@ fn the_row_tag_is_its_own_right_flush_string_and_the_state_word_wins() { s.run("ToggleQuestLog()").unwrap(); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); + // `Option`: an untagged row's string is blank, and a blank FontString reads back **nil** — + // `FontString:GetText 0x79d690` substitutes nil for an empty string (decision 2110). let tag = |s: &mut UiScript, i: u32| { - s.eval::(&format!("return QuestLogTitle{i}Tag:GetText()")) + s.eval::>(&format!("return QuestLogTitle{i}Tag:GetText()")) .unwrap() }; assert_eq!( - tag(&mut s, 1), - "(Elite)", + tag(&mut s, 1).as_deref(), + Some("(Elite)"), "the engine's tag, in parentheses" ); assert_eq!( - tag(&mut s, 2), - "(Complete)", + tag(&mut s, 2).as_deref(), + Some("(Complete)"), "COMPLETE overwrites the quest's own tag — ref l.190-192" ); - assert_eq!(tag(&mut s, 3), "(Failed)", "FAILED, same override"); - assert_eq!(tag(&mut s, 4), "", "a header carries no tag"); + assert_eq!( + tag(&mut s, 3).as_deref(), + Some("(Failed)"), + "FAILED, same override" + ); + assert_eq!(tag(&mut s, 4), None, "a header carries no tag"); // The title is the bare (indented) name — the state word lives on the tag string now, so // appending it here too would double it. diff --git a/crates/benilla-app/src/ui_script/resolve_bench.rs b/crates/benilla-app/src/ui_script/resolve_bench.rs index ac0e9ccf..464ff60b 100644 --- a/crates/benilla-app/src/ui_script/resolve_bench.rs +++ b/crates/benilla-app/src/ui_script/resolve_bench.rs @@ -726,3 +726,84 @@ fn an_action_bar_hover_sweep_costs_no_graph_derivation() { its node like any other (decision 1630, extending 1625)." ); } + +/// **The bag-addon hover law** (decision 2114, ledger B06's third idiom): a hover that owns the +/// tooltip WITH an anchor and then re-points it by hand must cost **zero** derivations. +/// +/// The guard above drives `SetOwner(owner, "ANCHOR_NONE")` + `ClearAllPoints()`, and it has always +/// passed — because ANCHOR_NONE already dropped the anchors, so the `ClearAllPoints()` after it +/// finds an empty list and touches nothing at all. It is the third idiom that was never driven: +/// +/// ```lua +/// -- Bagnon_Core/core/Item.lua -> Bagnon_Core/core/Utility.lua +/// ContainerFrameItemButton_OnEnter(item) -- SetOwner(item, "ANCHOR_RIGHT"): sets an anchor +/// Bagnon_AnchorTooltip(item) -- ClearAllPoints(), then GetLeft(), then SetPoint +/// ``` +/// +/// `ClearAllPoints()` with anchors present took the conservative touch, and the `GetLeft()` on the +/// next line settles the layout right there — so the whole graph was re-derived INSIDE the handler, +/// once per bag slot the cursor crossed. Measured live on `Probetwo` with the director's AddOns +/// folder, `WOW_UI_HANDLERS=8` over a 60 Hz sweep of twelve `BagnonItem*` buttons: `OnEnter` self +/// 1.75 ms/frame and `[layout-derive]` naming this site in three of its four samples, against +/// 0.43 ms/frame of total `tick` for the same sweep over stock `ContainerFrame1Item*` with no +/// addons. `derives/frame` read 0.88 with Bagnon and 0.00 without. +#[test] +fn a_bag_addon_hover_sweep_costs_no_graph_derivation() { + let mut s = settled_default_ui(); + s.run( + r#" + for i = 1, 12 do + local b = CreateFrame("Button", "BagOwner" .. i) + b:SetPoint("CENTER", 0, 0); b:SetSize(37, 37) + end + bag_n = 0 + function bag_hover() + bag_n = bag_n + 1 + local item = getglobal("BagOwner" .. (math.mod(bag_n, 12) + 1)) + -- ContainerFrameItemButton_OnEnter's arm: an ANCHORED SetOwner. + GameTooltip:SetOwner(item, "ANCHOR_RIGHT") + GameTooltip:AddLine("Item " .. bag_n, 1, 1, 1) + if math.mod(bag_n, 2) == 0 then + GameTooltip:AddLine("Use: restores health over 21 sec.", 0, 1, 0, 1) + else + GameTooltip:AddLine("Main Hand", 1, 1, 1) + end + GameTooltip:Show() + -- …then Bagnon_AnchorTooltip: drop the anchors it just set, ASK A RESOLVED EDGE + -- (which settles the layout on the spot), and re-point by hand. + GameTooltip:ClearAllPoints() + local left = item:GetLeft() or 0 + if left < (UIParent:GetRight() / 2) then + GameTooltip:SetPoint("TOPLEFT", item, "BOTTOMRIGHT") + else + GameTooltip:SetPoint("TOPRIGHT", item, "BOTTOMLEFT") + end + end + "#, + ) + .unwrap(); + + // Positive control across the births and the line pool's growth — both structural. + let born_at = s.layout_derivations(); + for _ in 0..40 { + s.run("bag_hover()").unwrap(); + app_frame(&mut s); + } + assert!( + s.layout_derivations() > born_at, + "creating twelve slots must derive the graph — zero here makes the assertion vacuous." + ); + + let derives_before = s.layout_derivations(); + for _ in 0..24 { + s.run("bag_hover()").unwrap(); + app_frame(&mut s); + } + let derives = s.layout_derivations() - derives_before; + assert_eq!( + derives, 0, + "24 bag-slot hovers derived the layout graph {derives} times — one per slot crossed. \ + `ClearAllPoints` drops a node's whole anchor-target set, which is a retarget onto the \ + EMPTY set and names its node like any other (decision 2114, completing 1625/1630)." + ); +} 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 fc7cd114..9070a332 100644 --- a/crates/benilla-app/src/ui_script/world_map_tests.rs +++ b/crates/benilla-app/src/ui_script/world_map_tests.rs @@ -173,7 +173,8 @@ fn the_poi_pool_grows_and_parks_its_tail() { /// Hovering a POI names it in the map's area label; the description line beneath carries the /// landmark's status only when it has one — the guard's directions never do, a battleground -/// node's "In Conflict" would — and is blanked otherwise (stock `WorldMapPOI_OnEnter`). +/// node's "In Conflict" would — and is blanked otherwise (stock `WorldMapPOI_OnEnter`), which +/// reads back as **nil**, not `""` (decision 2110). #[test] fn hovering_a_poi_names_it_and_adds_a_status_line_only_when_there_is_one() { let _data = benilla_formats::wow_data_or_skip!(); @@ -188,10 +189,12 @@ fn hovering_a_poi_names_it_and_adds_a_status_line_only_when_there_is_one() { "Lion's Pride Inn" ); assert_eq!( - s.eval::("return WorldMapFrameAreaDescription:GetText()") + s.eval::>("return WorldMapFrameAreaDescription:GetText()") .unwrap(), - "", - "no description → an empty description line" + None, + "no description → a blank line that reads back NIL: `FontString:GetText 0x79d690` \ + substitutes nil for an empty string (decision 2110), and Cartographer 2.02's world-map \ + hover reads exactly this as \"this POI has no status line\"" ); let mut with_status = landmark("Stables", 6, (0.5, 0.5)); diff --git a/crates/benilla-app/src/ui_text/engine/mod.rs b/crates/benilla-app/src/ui_text/engine/mod.rs index b8c6ad57..5510a2d1 100644 --- a/crates/benilla-app/src/ui_text/engine/mod.rs +++ b/crates/benilla-app/src/ui_text/engine/mod.rs @@ -1188,6 +1188,66 @@ mod ppem_tests { let _ = std::fs::remove_dir_all(&root); } + /// **An addon-shipped face wears an OUTLINE like any other** — the two halves of the MSBT + /// look, together, which nothing pinned (decision 2112). 2103 pins that the face LOADS; + /// [`super::outline`]'s own tests pin the composite recipe on a synthetic bitmap; the cell + /// arithmetic was only ever exercised on the fallback face. This runs a face read out of the + /// AddOns root through the outline path and asserts the composite cell it produces: the same + /// glyph, one cell per radius, each grown by `pad` on every side with its bearings moved out + /// to match, and the plain cell untouched. + #[test] + fn an_addon_shipped_face_rasterizes_an_outlined_cell() { + let Some(mut e) = engine_or_skip() else { + return; + }; + let bytes = { + let source = e.source.as_ref().expect("the test engine carries a chain"); + let chain = source.chain.lock_recover(); + chain.read(TEST_FACES[2]).expect("MORPHEUS is in the chain") + }; + let root = + std::env::temp_dir().join(format!("benilla-addon-outline-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + let fonts = root.join("MikScrollingBattleText").join("Fonts"); + std::fs::create_dir_all(&fonts).unwrap(); + std::fs::write(fonts.join("porky.ttf"), &bytes).unwrap(); + e.source.as_mut().unwrap().loose_root = Some(root.clone()); + + let face = e.face_for(Some( + "Interface\\Addons\\MikScrollingBattleText\\Fonts\\porky.ttf", + )); + assert_ne!(face, e.face_for(None), "the addon's own face, not Friz"); + // MSBT's default master size, and its default flag — `SetFont(porky, 18, "OUTLINE")` + // resolves to radius 1 (`super::outline::radius_of`). + let ppem = e.ppem(18.0); + e.ensure_str(face, ppem, 0, "6"); + e.ensure_str(face, ppem, 1, "6"); + let g = e.char_cell(face, ppem, '6').expect("shaped").glyphs[0].glyph_id; + let plain = e.cell(face, ppem, 0, g).expect("a plain cell"); + let ringed = e.cell(face, ppem, 1, g).expect("an outlined cell"); + // `dpi` is 1 here, so NORMAL is one dilation pass: `pad` = 1 texel every side. + assert_eq!( + (ringed.px_w, ringed.px_h), + (plain.px_w + 2.0, plain.px_h + 2.0), + "the ring grows the cell by pad on every side" + ); + assert_eq!( + (ringed.bearing_x, ringed.bearing_top), + (plain.bearing_x - 1.0, plain.bearing_top + 1.0), + "…and the bearings move out with it, so the ink sits where it did" + ); + // THICK is the second pass, and is a THIRD cell — not a re-use of either. + e.ensure_str(face, ppem, 2, "6"); + let thick = e.cell(face, ppem, 2, g).expect("a THICK cell"); + assert_eq!( + (thick.px_w, thick.px_h), + (plain.px_w + 4.0, plain.px_h + 4.0) + ); + assert_ne!(thick.uv, ringed.uv, "each radius packs its own cell"); + assert_ne!(plain.uv, ringed.uv); + let _ = std::fs::remove_dir_all(&root); + } + /// A path that resolves nowhere falls back to Friz and is remembered, so the read and the WARN /// happen once rather than once per measure. #[test] diff --git a/crates/benilla-app/tests/mail_frame.rs b/crates/benilla-app/tests/mail_frame.rs index 1886dc1f..f047c231 100644 --- a/crates/benilla-app/tests/mail_frame.rs +++ b/crates/benilla-app/tests/mail_frame.rs @@ -647,7 +647,12 @@ fn an_auction_invoice_renders_as_a_receipt() { "the buy mode rides the item line here, not the purchaser line" ); assert_eq!(text(&s, "OpenMailInvoicePurchaser"), "FROM: Onewarrior"); - assert_eq!(text(&s, "OpenMailInvoiceBuyMode"), ""); + assert_eq!( + text(&s, "OpenMailInvoiceBuyMode"), + "nil", + "the buy-mode line is blank on a bid win, and a blank FontString reads back nil — \ + `FontString:GetText 0x79d690` substitutes (decision 2110); this helper `tostring`s it" + ); assert_eq!(money(&s, "OpenMailTransactionAmountMoneyFrame"), "9000"); for gone in [ "OpenMailInvoiceSalePrice", diff --git a/crates/benilla-ui/src/script/button.rs b/crates/benilla-ui/src/script/button.rs index 13bd81df..8270c8f3 100644 --- a/crates/benilla-ui/src/script/button.rs +++ b/crates/benilla-ui/src/script/button.rs @@ -476,14 +476,25 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { m.set( "SetText", lua.create_function(|lua, (this, text): (Table, Option)| { + // `CSimpleButton::SetText 0x778dc0` opens `if (!text) return;` (`0x778dcc`, wow-re + // `button-label-build-and-anchor-order.md`) — a nil never reaches the label at all, so + // it neither clears the text nor lazily creates the FontString. That guard is the + // BUTTON's own: a `FontString:SetText(nil)` is not a no-op, it truncates the cell + // (`0x771d80`, decision 2110). Below the guard the button is a pass-through to + // `[button+0x338]->0x771d80`. + let Some(text) = text else { return Ok(()) }; let id = ensure_slot(lua, &this, Slot::Text)?; let mut model = lua.app_data_mut::().expect("model app_data"); let rh = *model.id_to_region.get(&id).expect("text region id"); - model.region_data.entry(rh).or_default().text = text; + model.region_data.entry(rh).or_default().text = Some(text); model.touch_measure(rh); Ok(()) })?, )?; + // GetText — like the FontString's own (`region::text`, decision 2110), an **empty label reads + // back nil**: `Button:GetText 0x780e10` carries the same first-byte substitution at `0x780ec5`, + // over three nil conditions rather than two (no label region, a NULL cell, an empty cell). + // Its EditBox neighbour deliberately does not — the law is per binding. m.set( "GetText", lua.create_function(|lua, this: Table| { @@ -492,6 +503,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { let model = lua.app_data_ref::().expect("model app_data"); rh.and_then(|rh| model.region_data.get(&rh)) .and_then(|d| d.text.clone()) + .filter(|t| !t.is_empty()) }; match text { Some(t) => Ok(Value::String(lua.create_string(&t)?)), diff --git a/crates/benilla-ui/src/script/extract.rs b/crates/benilla-ui/src/script/extract.rs index e1a54fb0..efa01eef 100644 --- a/crates/benilla-ui/src/script/extract.rs +++ b/crates/benilla-ui/src/script/extract.rs @@ -178,7 +178,11 @@ impl UiScript { owner_frame.map(|f| &f.kind_state) { if sl.thumb == Some(rh) { - let tsize = model.region_data.get(&rh).and_then(|d| d.size); + // The thumb's OWN size getters, not its authored `` — the same + // `CSimpleTexture::GetWidth`/`GetHeight` fallback the drag law reads + // (`slider::thumb_extent`), so a Lua-built `SetThumbTexture(path)` knob + // draws at its art's texel span instead of smeared over the whole track. + let tsize = super::region::virtual_span(&model, rh); rect = rect .map(|r| slider::thumb_rect(r, tsize, sl.vertical, sl.fraction())); thumb_fill = true; @@ -333,18 +337,34 @@ impl UiScript { let alpha = owner_frame.map(|f| f.effective_alpha).unwrap_or(1.0) * data.alpha.unwrap_or(1.0); if let Some(fo) = state_font { - // The font object's paint wholesale, except a color the Lua explicitly - // SetTextColor'd (the client's explicitly-set mask keeps it — ui ledger - // FONTINSTANCE+0x38, color bit 0x404). - data.font_path = fo.font.clone().or(data.font_path); - data.font_height = fo.height.or(data.font_height); + // The font object's paint, **behind the severance mask on every axis** — + // the same `font_explicit` gate `font::repaint` applies and the + // `button_font` block below names as the law ("it loses to a face the + // label FontString set for *itself*, which severs one level further + // down"). Three of these six axes did not have it: face and height were + // `fo.x.or(data.x)`, which makes the OBJECT outrank an explicit + // `SetFont`, and the outline was written unconditionally — so a label + // that called `SetFont(path, h, "OUTLINE")` for itself had all three put + // back from the object on the very next extract, every frame, forever. + // That is the *shape* of the report this was found under (decision 2112: + // an addon's `SetFont` silently not taking); MSBT's own strings are not + // a button's label and never met it, but any addon that restyles a + // `` did. + if !data.font_explicit.face { + data.font_path = fo.font.clone().or(data.font_path); + } + if !data.font_explicit.height { + data.font_height = fo.height.or(data.font_height); + } // Same severance as the colour below: a region that called // `SetShadowColor`/`SetShadowOffset` keeps its own, or the font object it // inherits would silently overwrite the value the addon just set. if !data.font_explicit.shadow { data.font_shadow = fo.shadow.or(data.font_shadow); } - data.outline = fo.outline; + if !data.font_explicit.outline { + data.outline = fo.outline; + } // Test the severance MASK, which is what the sentence above claims and what // wow-re pinned, not `vertex_color.is_none()`. The nil-check was an // equivalent proxy for exactly as long as an explicit `SetTextColor` was the diff --git a/crates/benilla-ui/src/script/lua50.rs b/crates/benilla-ui/src/script/lua50.rs index 5e2bb0a1..9fd3b5c0 100644 --- a/crates/benilla-ui/src/script/lua50.rs +++ b/crates/benilla-ui/src/script/lua50.rs @@ -597,4 +597,68 @@ mod tests { "5.0's insert writes index getn+1, which after setn(t, 0) is index 1" ); } + + /// **`{ [1] = a, [2] = b, … }` is a LIST, and `pairs` walks it 1, 2, 3** — decision 2111, the + /// fourth "restore what 5.1 changed" hunk in `third_party/lua-src`. + /// + /// 1.12's `recfield` keeps `cc->nh++` **inside** its `TK_NAME` arm (`0x6fd5a4`'s + /// `cmp [edi+0x10],0x116` / `0x6fd5ad jne 0x6fd5d4` — 5.0's own placement), so a + /// `[expr] = value` field credits **neither** `OP_NEWTABLE` size hint. The table is born with + /// `B = C = 0`, i.e. on the shared dummy node, so the first store finds no free node, takes + /// `luaH_newkey`'s rehash tail, and `rehash` sizes an **array part** for the dense integer + /// keys; `luaH_next` walks the array part first, ascending by index, and the node part after. + /// Stock 5.1 moved that `cc->nh++` out of the arm, pre-sized a node vector big enough that no + /// rehash ever fires, and left all n keys in the hash — where `next` is slot order. + /// + /// wow-5875-re `system/ui/scratch/lua-table-storage-and-next-order.md` — **executed** on + /// `WoW.exe`'s own bytes (`lua_open` → `luaL_loadbuffer` → `lua_pcall` → `lua_next`), not + /// derived, including the exact `Bagnon_Core.lua` below; ascending holds for n = 1..24 and + /// regardless of the order the fields are written in. + /// + /// It matters because every saved-variables file we write is that constructor (decision 1128's + /// grammar, and the reference's own writer's — it never emits a bare positional entry), so + /// before this hunk every list an addon saved came back in hash order. Bagnon 6.10.22 walks + /// its saved bag order with `pairs` and drew the keyring first, ahead of the backpack. + #[test] + fn a_bracketed_key_constructor_is_an_array_and_walks_in_index_order() { + let s = UiScript::new().unwrap(); + // Bagnon's own table, exactly as its saved-variables file spells it. + assert_eq!( + s.eval::( + "local t = { [1] = 0, [2] = 1, [3] = 2, [4] = 3, [5] = 4, [6] = -2 } + local out = '' for _, v in pairs(t) do out = out .. v .. ',' end return out" + ) + .unwrap(), + "0,1,2,3,4,-2,", + "the keyring (-2) is LAST — this is the director's Bagnon grid" + ); + // …and the order the fields are written in does not change it. + assert_eq!( + s.eval::( + "local t = { [3] = 'c', [1] = 'a', [2] = 'b' } + local out = '' for k in pairs(t) do out = out .. k .. ',' end return out" + ) + .unwrap(), + "1,2,3," + ); + // A key the array part cannot hold comes after the run, in hash order. + assert_eq!( + s.eval::( + "local t = { ['a'] = 1, [1] = 10, [2] = 20, [3] = 30 } + local out = '' for k in pairs(t) do out = out .. tostring(k) .. ',' end return out" + ) + .unwrap(), + "1,2,3,a," + ); + // The positional spelling was always an array and still is — the control that keeps the + // first assertion from reading as vacuous. + assert_eq!( + s.eval::( + "local t = { 0, 1, 2, 3, 4, -2 } + local out = '' for _, v in pairs(t) do out = out .. v .. ',' end return out" + ) + .unwrap(), + "0,1,2,3,4,-2," + ); + } } diff --git a/crates/benilla-ui/src/script/object/layout_methods.rs b/crates/benilla-ui/src/script/object/layout_methods.rs index c0bebb09..34053a57 100644 --- a/crates/benilla-ui/src/script/object/layout_methods.rs +++ b/crates/benilla-ui/src/script/object/layout_methods.rs @@ -60,15 +60,24 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { // keeps an idempotent per-frame caller (the classic OnUpdate re-SetPoint idiom) from // pinning the gate open — the same absorption the fingerprint gives, paid once at // the write instead of per-frame over the whole model. - let changed = match model.layout_inputs.get_mut(&h) { + // + // **And it NAMES its node** (decision 2114, completing 1625's migration). Dropping + // every anchor is a retarget whose NEW target list is empty, and both lists are right + // here — so the cached graph's edges get unlinked instead of thrown away. Left on the + // conservative touch, this was the one recurring `[layout-derive]` site in a live + // hover sweep: `Bagnon_AnchorTooltip` opens with `GameTooltip:ClearAllPoints()` and + // then asks `frame:GetLeft()`, so the whole graph was re-derived INSIDE the handler, + // once per hovered item, and billed to `[ui-handlers]`' `OnEnter`. + let old: Option> = match model.layout_inputs.get_mut(&h) { Some(input) if !input.anchors.is_empty() => { + let old = input.anchors.iter().map(|a| a.relative_to).collect(); input.anchors.clear(); - true + Some(old) } - _ => false, + _ => None, }; - if changed { - model.touch_layout(); + if let Some(old) = old { + model.touch_layout_retarget_frame(h, &old, &[]); } Ok(()) })?, diff --git a/crates/benilla-ui/src/script/region/layout.rs b/crates/benilla-ui/src/script/region/layout.rs index 0e3cf91a..ced950e9 100644 --- a/crates/benilla-ui/src/script/region/layout.rs +++ b/crates/benilla-ui/src/script/region/layout.rs @@ -136,10 +136,14 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { let rh = region_handle_of(lua, &this)?; let mut model = lua.app_data_mut::().expect("model"); let d = model.region_data.entry(rh).or_default(); - let changed = !d.anchors.is_empty(); + // Names its node, the frame twin's rule and for the frame twin's reason (decision + // 2114): clearing every anchor is a retarget onto an EMPTY target list, so the cached + // graph's edges are unlinked rather than the whole graph re-derived. + let old: Option> = + (!d.anchors.is_empty()).then(|| d.anchors.iter().map(|a| a.relative_to).collect()); d.anchors.clear(); - if changed { - model.touch_layout(); + if let Some(old) = old { + model.touch_layout_retarget_region(rh, &old, &[]); } Ok(()) })?, diff --git a/crates/benilla-ui/src/script/region/text.rs b/crates/benilla-ui/src/script/region/text.rs index f5875ed1..e5ffdace 100644 --- a/crates/benilla-ui/src/script/region/text.rs +++ b/crates/benilla-ui/src/script/region/text.rs @@ -61,13 +61,47 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { })?, )?; + // GetText — **an EMPTY string comes back as `nil`, and that substitution is the getter's own** + // (`FontString:GetText 0x79d690`, wow-re + // `system/ui/scratch/fontstring-text-cell-and-gettext-contract.md`, §5 five-worker round): + // + // ```text + // 79d72f mov eax,[eax+0xf0] ; the text cell + // 79d735 test eax,eax + // 79d739 je 0x79d740 ; NULL -> substitute + // 79d73b cmp byte ptr [eax],0x0 ; the FIRST-BYTE test + // 79d73e jne 0x79d742 ; non-empty -> keep + // 79d740 xor eax,eax ; EMPTY -> NULL + // 79d746 call 0x6f3890 ; pushstring; NULL -> pushnil + // ``` + // + // So `FontString:GetText` **cannot return `""`**. Not the setter's doing: `SetText 0x771d80` + // never writes NULL to `+0xf0` on any leg — NULL and `""` share one leg that truncates the + // buffer in place (`0x771e7e mov byte ptr [eax],0x0`, the pointer surviving) — so the cell + // really does hold a non-NULL empty string and this getter is what collapses it. + // + // The law is **per binding, not per family**, and the two neighbours differ: + // `Button:GetText 0x780e10` carries the same substitution (`0x780ec5`, three nil conditions — + // see `button.rs`), while `EditBox:GetText 0x7985c0` carries **none** (`0x79867b` reads + // `[edit+0x32c]` straight through), which is what stock `MailFrame.lua`'s + // `GetText() == ""`/`strlen(GetText())` is written against. Do not hoist this. + // + // What it costs to get wrong (decision 2110): the stock world map blanks + // `WorldMapFrameAreaDescription` with `SetText("")` on every POI hover, and Cartographer 2.02 + // reads `if WorldMapFrameAreaDescription:GetText() then` as "this POI has a status line" — + // answering `""` there put the zone's level range on the description's own line under a + // whitened label and left it on screen forever. m.set( "GetText", lua.create_function(|lua, this: Table| { let rh = region_handle_of(lua, &this)?; let text = { let model = lua.app_data_ref::().expect("model"); - model.region_data.get(&rh).and_then(|d| d.text.clone()) + model + .region_data + .get(&rh) + .and_then(|d| d.text.clone()) + .filter(|t| !t.is_empty()) }; match text { Some(t) => Ok(Value::String(lua.create_string(&t)?)), diff --git a/crates/benilla-ui/src/script/saved.rs b/crates/benilla-ui/src/script/saved.rs index e3224ff8..15efd204 100644 --- a/crates/benilla-ui/src/script/saved.rs +++ b/crates/benilla-ui/src/script/saved.rs @@ -146,9 +146,18 @@ fn quote(s: &str) -> String { } /// A table constructor: `{\n[key] = value,\n}`. Keys are always bracketed (the -/// reference's shape, and it sidesteps every reserved-word and non-identifier question); entries -/// are **sorted** — integer keys ascending, then strings alphabetically — so the file is stable -/// across runs instead of following Lua's hash order. +/// reference's shape — its writer emits `[key] = value` for **every** table shape and never a bare +/// positional entry, byte-verified in wow-re `system/ui/scratch/lua-table-storage-and-next-order.md` +/// §Q5 — and it sidesteps every reserved-word and non-identifier question); entries are **sorted** +/// — integer keys ascending, then strings alphabetically — so the file is stable across runs +/// instead of following Lua's hash order. +/// +/// **A list written this way still reloads as a list**, and that is a property of the *parser*, not +/// of this emitter: 1.12's `recfield` gives a `[expr] = v` field neither size hint, so the table is +/// born on the dummy node, the first store rehashes, and the dense integer keys land in an array +/// part that `next` walks ascending. Our vendored parser was restored to that placement in decision +/// 2111 — before it, this exact file shape came back keyed 1..n in the *hash* part and Bagnon's +/// keyring drew first. fn table(t: &Table, depth: usize, seen: &mut HashSet<*const c_void>) -> Option { if depth > MAX_DEPTH || !seen.insert(t.to_pointer()) { return None; @@ -211,6 +220,50 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { mod tests { use crate::script::UiScript; + /// **A saved LIST comes back a list** — Bagnon's inventory grid, at 1:1 scale, and the + /// round-trip half of decision 2111 (the dialect half is `lua50`'s own tests). + /// + /// `Bagnon_Core` keeps its frame's bag order in a saved table, sorts it so the keyring + /// (`KEYRING_CONTAINER` = -2) lands last, and then lays the grid out by walking it with + /// **`pairs`** (`BagnonFrame_Generate`'s `for _, bagID in pairs(BagnonSets[frameName].bags)`). + /// Session one has no file: the table is the literal `{-2,0,1,2,3,4}`, an array table, and + /// `pairs` walks it 1..6 — keyring last, as the reference shows it. Session two reads the + /// serializer's `[k] = v` form back, and before 2111 that came back keyed 1..6 in the **hash** + /// part, walked in slot order, with key 6 — the keyring — **first**, ahead of the backpack. + /// That is the director's report; the writer is unchanged and the parser is what was wrong. + #[test] + fn a_saved_list_still_walks_in_index_order_after_a_restart() { + let s = UiScript::new().unwrap(); + s.run( + r#" + BAGS = { -2, 0, 1, 2, 3, 4 } + table.sort(BAGS, function(a, b) + if a == -2 then return false + elseif b == -2 then return true + else return a < b end + end) + RegisterForSave("BAGS") + "#, + ) + .unwrap(); + let walk = |vm: &UiScript| { + vm.eval::( + "local out = '' for _, v in pairs(BAGS) do out = out .. v .. ',' end return out", + ) + .unwrap() + }; + assert_eq!(walk(&s), "0,1,2,3,4,-2,", "the live table, before any save"); + + let text = s.saved_variables_text(); + let fresh = UiScript::new().unwrap(); + fresh.run(&text).unwrap(); + assert_eq!( + walk(&fresh), + "0,1,2,3,4,-2,", + "the restart must walk the same order the live session did; wrote:\n{text}" + ); + } + /// The round trip that is the whole point: what the writer emits, the loader's own chunk /// restores — through a fresh VM, exactly as a restart does. #[test] diff --git a/crates/benilla-ui/src/script/slider.rs b/crates/benilla-ui/src/script/slider.rs index 19f11e4d..c8ba91bd 100644 --- a/crates/benilla-ui/src/script/slider.rs +++ b/crates/benilla-ui/src/script/slider.rs @@ -282,16 +282,14 @@ fn num_f32(v: &Value) -> f32 { /// track and centered on the cross-axis. The thumb stays **inset** within the track (travel = /// trackLen − thumbLen), the way WoW scrollbar knobs visibly do. VERTICAL runs y-up-inverted — /// `fraction` 0 (value=min) sits the thumb flush at the **top** (a scrollbar at 0 is scrolled to the -/// top), `fraction` 1 flush at the bottom. A thumb with no explicit size fills the slider (zero -/// travel). This is the single geometry source shared by `extract` (render) and [`begin_drag`] -/// (hit-test), so render and input never disagree. Returns a [`Rect`] as `(bottom, left, top, right)`. -pub(super) fn thumb_rect( - r: Rect, - thumb_size: Option<(f32, f32)>, - vertical: bool, - fraction: f32, -) -> Rect { - let (tw, th) = thumb_size.unwrap_or((r.width(), r.height())); +/// top), `fraction` 1 flush at the bottom. This is the single geometry source shared by `extract` +/// (render) and [`begin_drag`] (hit-test), so render and input never disagree. Returns a [`Rect`] as +/// `(bottom, left, top, right)`. +/// +/// `thumb_size` is [`thumb_extent`]'s — the thumb region's OWN size getters, never its authored +/// `` alone. +pub(super) fn thumb_rect(r: Rect, thumb_size: (f32, f32), vertical: bool, fraction: f32) -> Rect { + let (tw, th) = thumb_size; if vertical { let travel = (r.height() - th).max(0.0); let top = r.top - fraction * travel; // f=0 → track top, f=1 → track bottom @@ -305,6 +303,26 @@ pub(super) fn thumb_rect( } } +/// The thumb's extent along both axes — **the thumb region's own `GetWidth`/`GetHeight`**, which +/// is what the client asks for and is not the same thing as its authored ``. +/// +/// `CSimpleSlider`'s pixel→value law reads the thumb through its geometry vtable +/// (`0x789ba0`: `ff 50 1c call [eax+0x1c]` for the horizontal branch, `+0x20` for the vertical), and +/// on a `CSimpleTexture` those slots are `0x770720`/`0x770790` — the **native-texel fallback**: +/// authored span when it is non-zero on that axis, else the art's own texel span through the same +/// `` converter, else `0.0` when there is no art at all (wow-re +/// `region-size-fallback.md` §2, VERIFIED; ours is [`super::region::virtual_span`], decision 1349). +/// +/// Reading `RegionData::size` instead is what broke every Lua-built slider: `SetThumbTexture(path)` +/// authors no size, all four stock ``s declare one, and the old fallback — *the thumb +/// fills the track* — is not a client behaviour at all. It smeared the knob over the whole bar and +/// left `trackLen − thumbLen == 0`, i.e. a slider that cannot move (Dewdrop-2.0's popout, B-report). +/// A thumb with **no region at all** is `None`: `0x789ba0` gates the value math on `+0x328` being +/// non-null, so a thumbless slider takes the press and the capture but never warps. +fn thumb_extent(model: &Model, thumb: Option) -> Option<(f32, f32)> { + Some(super::region::virtual_span(model, thumb?)) +} + /// The in-flight thumb drag (decision 0250 §5): the slider being dragged + the grab offset /// [`slider_grab`] returned, so the thumb tracks the cursor without jumping. Engine C++-equivalent /// — no Lua, like the real client's scrollbar. @@ -340,22 +358,28 @@ pub(super) fn begin_drag( if !enabled { return None; } - let size = thumb - .and_then(|rh| model.region_data.get(&rh)) - .and_then(|d| d.size); + // No thumb region → `0x789ba0`'s `+0x328` gate: the press still captures (the dispatcher's + // `mov [ebx+0x80],esi` is unconditional), the value never moves. + let Some(size) = thumb_extent(model, thumb) else { + model.slider_drag = Some(SliderDrag { + slider: h, + grab_offset: 0.0, + }); + return None; + }; let trect = thumb_rect(r, size, vertical, fraction); let on_thumb = point_in_rect(trect, x, y); // Everything below is stated as distance from the TRACK'S LEADING EDGE — the end the thumb // sits at when the value is `min`. Vertical tracks run downward from `r.top` in this y-up // arena, so that distance is `top − y`; horizontal ones run rightward from `r.left`. - let (cursor, thumb_lead, thumb_extent) = if vertical { + let (cursor, thumb_lead, thumb_len) = if vertical { (r.top - y, r.top - trect.top, trect.top - trect.bottom) } else { (x - r.left, trect.left - r.left, trect.right - trect.left) }; model.slider_drag = Some(SliderDrag { slider: h, - grab_offset: slider_grab(cursor, thumb_lead, thumb_extent), + grab_offset: slider_grab(cursor, thumb_lead, thumb_len), }); if on_thumb { return None; // no value change from the grab itself @@ -378,18 +402,15 @@ pub(super) fn drag_move(model: &mut Model, x: f32, y: f32) -> Option<(u32, f32)> Some(KindState::Slider(s)) => (s.min, s.max, s.vertical, s.thumb), _ => return None, // slider destroyed mid-drag }; - let (tw, th) = thumb - .and_then(|rh| model.region_data.get(&rh)) - .and_then(|d| d.size) - .unwrap_or((r.width(), r.height())); + let (tw, th) = thumb_extent(model, thumb)?; // The same leading-edge frame [`begin_drag`] stored the grab in. - let (cursor, track_extent, thumb_extent) = if vertical { + let (cursor, track_extent, thumb_len) = if vertical { (r.top - y, r.height(), th) } else { (x - r.left, r.width(), tw) }; // `None` = nothing to scroll (a thumb as long as its track). - let fraction = slider_fraction(cursor, grab_offset, track_extent, thumb_extent)?; + let fraction = slider_fraction(cursor, grab_offset, track_extent, thumb_len)?; let value = min + fraction * (max - min); let changed = match model.arena.frame_mut(slider).map(|f| &mut f.kind_state) { Some(KindState::Slider(s)) => s.store_value(value), diff --git a/crates/benilla-ui/src/script/tests/button.rs b/crates/benilla-ui/src/script/tests/button.rs index 3a40a8d7..a6046500 100644 --- a/crates/benilla-ui/src/script/tests/button.rs +++ b/crates/benilla-ui/src/script/tests/button.rs @@ -1073,3 +1073,96 @@ fn a_lazily_made_label_is_anchored_by_the_normal_fonts_justify() { s.run("left:SetTextFontObject(ProbeFontRight)").unwrap(); assert_eq!(point(&s, "left"), ("LEFT".into(), "LEFT".into(), 0.0, 0.0)); } + +/// **A label that set its own face keeps it — the severance mask covers every axis, not three of +/// six** (decision 2112). +/// +/// `font_explicit` is the client's explicitly-set mask (`FONTINSTANCE+0x38`): an axis a widget +/// writes for *itself* severs inheritance from the font instance it reads, and is never restored +/// (wow-re `font-object-lua-surface.md`; the `button_font` block in `script::extract` cites it by +/// name). `font::repaint` honours it on all seven axes. The extract's per-state re-point honoured +/// it on shadow, colour and both justifies — and not on **face, height or outline**: the face and +/// height read `fo.x.or(data.x)`, which makes the object outrank an explicit `SetFont`, and the +/// outline was written unconditionally. A `` that called +/// `SetFont(path, h, "OUTLINE")` for itself therefore had all three silently put back from the +/// button's font object on the very next extract — every frame, so no Lua could win the race. +#[test] +fn a_button_labels_own_setfont_survives_the_state_font_repoint() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.register_font_object( + "TemplateFont", + FontObject { + font: Some("Fonts\\FRIZQT__.TTF".into()), + height: Some(12.0), + color: Some([1.0, 0.82, 0.0, 1.0]), + outline: Outline::None, + ..Default::default() + }, + ); + s.run( + r#" + b = CreateFrame("Button", "SkinnedBtn") + b:SetPoint("CENTER", 0, 0); b:SetSize(100, 20) + b:SetText("Label") + b:SetTextFontObject("TemplateFont") + b:GetFontString():SetFont("Interface\\Addons\\Skin\\Fonts\\porky.ttf", 18, "OUTLINE") + "#, + ) + .unwrap(); + let painted = |s: &mut crate::script::UiScript| { + s.resolve(); + s.extract() + .into_iter() + .find_map(|q| match q.content { + QuadContent::Text { + text: Some(t), + ref font, + font_height, + outline, + .. + } if t == "Label" => Some((font.clone(), font_height, outline)), + _ => None, + }) + .expect("label text quad") + }; + assert_eq!( + painted(&mut s), + ( + Some("Interface\\Addons\\Skin\\Fonts\\porky.ttf".to_string()), + Some(18.0), + Outline::Normal + ), + "the label's own SetFont severs face, height AND outline from the state font object" + ); + // …and it survives a state change, which is what re-runs the re-point. + s.run("b:Disable()").unwrap(); + assert_eq!( + painted(&mut s), + ( + Some("Interface\\Addons\\Skin\\Fonts\\porky.ttf".to_string()), + Some(18.0), + Outline::Normal + ), + "…and a disable re-points the instance without restoring what the label severed" + ); + // The axes the label did NOT set still follow the object: the colour is the template's gold. + s.resolve(); + let color = s + .extract() + .into_iter() + .find_map(|q| match q.content { + QuadContent::Text { + text: Some(t), + color, + .. + } if t == "Label" => Some(color), + _ => None, + }) + .expect("label text quad"); + assert_eq!( + color, + Some([1.0, 0.82, 0.0, 1.0]), + "an axis the label never set still inherits — severance is per-axis" + ); +} diff --git a/crates/benilla-ui/src/script/tests/font_object.rs b/crates/benilla-ui/src/script/tests/font_object.rs index 444fb201..c70c1b7a 100644 --- a/crates/benilla-ui/src/script/tests/font_object.rs +++ b/crates/benilla-ui/src/script/tests/font_object.rs @@ -1081,3 +1081,96 @@ fn set_font_answers_the_hosts_load_verdict_when_there_is_a_host() { ); assert_eq!(height, 18.0, "…while the height, which never fails, is set"); } + +/// **Mik's Scrolling Battle Text, end to end** — the exact sequence the addon runs per event, +/// pinned because it is the shape a whole class of "the addon's font did not take" reports wears +/// (decisions 2103, 2112). +/// +/// MSBT's twenty scroll-area FontStrings are declared `inherits="MasterFont"` — the reference's +/// root font object, which carries a `` and **nothing else**: no face, no height, no +/// outline. Each animation then does, in this order, on the string it recycles: +/// `ClearAllPoints · SetFont(, 18, "OUTLINE") · SetTextColor · SetText · SetAlpha · +/// SetPoint`. Every one of those five paint axes has to reach the extracted quad *unmodified* by +/// the object the string inherits — the addon's face over the object's absent one, the addon's +/// height over the renderer default, the addon's outline over the object's `NONE` (which is +/// indistinguishable from "unset", see [`Outline`]), its colour over the object's absent one, and +/// the animation's fade as the quad's own alpha. The `` is the one thing that IS inherited, +/// and it must still be there. +#[test] +fn msbt_paints_its_own_face_size_outline_and_fade_over_the_font_object_it_inherits() { + let mut s = script(); + s.set_screen_size(1600.0, 900.0); + load( + &s, + r#" + + + + + + + + + + + + + "#, + ); + s.run( + r#" + local fs = getglobal("MSBTFrameIncomingText1") + fs:ClearAllPoints() + fs:SetFont("Interface\\Addons\\MikScrollingBattleText\\Fonts\\porky.ttf", 18, "OUTLINE") + fs:SetTextColor(1, 1, 1) + fs:SetText("-64") + fs:SetAlpha(0.5) + fs:SetPoint("BOTTOMRIGHT", 0, 0) + "#, + ) + .unwrap(); + s.resolve(); + let q = s + .extract() + .into_iter() + .find(|q| matches!(&q.content, QuadContent::Text { text: Some(t), .. } if t == "-64")) + .expect("MSBT's text quad"); + assert_eq!( + q.alpha, 0.5, + "the scroll animation's fade is the quad's alpha" + ); + match q.content { + QuadContent::Text { + ref font, + font_height, + outline, + color, + shadow, + .. + } => { + assert_eq!( + font.as_deref(), + Some("Interface\\Addons\\MikScrollingBattleText\\Fonts\\porky.ttf"), + "the addon's own face, not the fallback" + ); + assert_eq!(font_height, Some(18.0), "the profile's master size"); + assert_eq!(outline, Outline::Normal, "its OUTLINE flag"); + assert_eq!(color, Some([1.0, 1.0, 1.0, 1.0])); + assert!( + shadow.is_some(), + "MasterFont's is the one axis the string never set, so it inherits" + ); + } + ref other => panic!("expected a Text quad, got {other:?}"), + } + // The Lua-visible echo agrees with the quad — the readback an addon branches on. + assert_eq!( + s.eval::<(String, f32, String)>("return MSBTFrameIncomingText1:GetFont()") + .unwrap(), + ( + "Interface\\Addons\\MikScrollingBattleText\\Fonts\\porky.ttf".to_string(), + 18.0, + "OUTLINE".to_string() + ) + ); +} diff --git a/crates/benilla-ui/src/script/tests/regions.rs b/crates/benilla-ui/src/script/tests/regions.rs index d12a3055..3603a407 100644 --- a/crates/benilla-ui/src/script/tests/regions.rs +++ b/crates/benilla-ui/src/script/tests/regions.rs @@ -1233,3 +1233,103 @@ fn set_portrait_texture_folds_the_token_to_lowercase() { assert_eq!(bound.as_deref(), Some("npc")); assert!(s.errors().is_empty(), "{:?}", s.errors()); } + +/// **`FontString:GetText` substitutes nil for an EMPTY string, and that substitution is the +/// getter's own** — `0x79d690` reads the cell, and eleven bytes before the pushstring it does a +/// FIRST-BYTE test and zeroes the pointer on `'\0'`: +/// +/// ```text +/// 79d735 test eax,eax / je 0x79d740 ; NULL -> substitute +/// 79d73b cmp byte ptr [eax],0x0 ; the first-byte test +/// 79d73e jne 0x79d742 ; non-empty -> keep +/// 79d740 xor eax,eax ; EMPTY -> NULL +/// 79d746 call 0x6f3890 ; NULL -> pushnil +/// ``` +/// +/// So it cannot return `""`, whatever the cell holds — and the cell really does hold non-NULL +/// empty strings, because `SetText 0x771d80` never writes NULL to `+0xf0` on any leg: NULL and +/// `""` share one leg that truncates the buffer in place. Per binding, not per family: +/// `Button:GetText 0x780e10` carries the same substitution (`0x780ec5`), `EditBox:GetText +/// 0x7985c0` carries none (wow-re `fontstring-text-cell-and-gettext-contract.md`, a §5 round; +/// decision 2110). +/// +/// The director's shape (Cartographer 2.02, the world map's hover label): the stock +/// `WorldMapFrameAreaDescription` is blanked with `SetText("")` by `WorldMapPOI_OnEnter`/`_OnLeave` +/// on **every** POI hover, and Cartographer's ZoneInfo reads +/// `if WorldMapFrameAreaDescription:GetText() then` as "this POI has a status line, so put the +/// zone's level range there instead of on the label". Answering `""` made that true forever after +/// the first POI touch — the name lost its faction colour, the range moved to the description's own +/// line beneath it, and nothing ever cleared it again. +#[test] +fn an_empty_fontstring_reads_back_nil_and_an_edit_box_does_not() { + let s = script(); + s.run( + r#" + local f = CreateFrame("Frame", "TextCell") + fresh = f:CreateFontString(nil, "OVERLAY") + held = f:CreateFontString(nil, "OVERLAY") + "#, + ) + .unwrap(); + let text_of = |s: &UiScript, which: &str| { + s.eval::>(&format!("return {which}:GetText()")) + .unwrap() + }; + + // Never written, and every shape of an empty write, all nil. + assert_eq!(text_of(&s, "fresh"), None); + for write in [ + r#"fresh:SetText("")"#, + r#"fresh:SetText(nil)"#, + r#"fresh:SetFormattedText("%s", "")"#, + ] { + s.run(write).unwrap(); + assert_eq!(text_of(&s, "fresh"), None, "after `{write}`"); + } + + // A string that HAS held text still reads nil once blanked — the substitution is the getter's, + // so it does not matter that the cell keeps a (truncated) buffer. + s.run(r#"held:SetText("In Conflict")"#).unwrap(); + assert_eq!(text_of(&s, "held").as_deref(), Some("In Conflict")); + s.run(r#"held:SetText("")"#).unwrap(); + assert_eq!( + text_of(&s, "held"), + None, + "a blanked FontString reads back nil, not an empty string" + ); + s.run(r#"held:SetText("back"); held:SetText(nil)"#).unwrap(); + assert_eq!(text_of(&s, "held"), None); + + // Button:GetText carries the SAME substitution; Button:SetText(nil) is its own no-op guard. + s.run( + r#" + local b = CreateFrame("Button", "TextCellButton") + b:SetText("Accept") + b:SetText(nil) + "#, + ) + .unwrap(); + assert_eq!( + text_of(&s, "TextCellButton").as_deref(), + Some("Accept"), + "a nil never reaches the button's label (`0x778dcc`)" + ); + s.run(r#"TextCellButton:SetText("")"#).unwrap(); + assert_eq!(text_of(&s, "TextCellButton"), None, "an empty label is nil"); + + // And the EditBox is the counter-example that keeps this from being hoisted: stock + // `MailFrame.lua` compares `GetText() == ""` and calls `strlen(GetText())` on one. + s.run( + r#" + local e = CreateFrame("EditBox", "TextCellEdit") + e:SetText("") + "#, + ) + .unwrap(); + assert_eq!( + text_of(&s, "TextCellEdit").as_deref(), + Some(""), + "EditBox:GetText 0x7985c0 reads [edit+0x32c] straight through — no substitution" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} diff --git a/crates/benilla-ui/src/script/tests/slider.rs b/crates/benilla-ui/src/script/tests/slider.rs index 2794d34d..b6544dd5 100644 --- a/crates/benilla-ui/src/script/tests/slider.rs +++ b/crates/benilla-ui/src/script/tests/slider.rs @@ -375,3 +375,66 @@ fn slider_onload_range_does_not_run_an_unarmed_onvaluechanged() { ) .unwrap(); } + +#[test] +fn a_thumb_with_no_authored_size_takes_its_arts_texel_span_and_still_drags() { + // Dewdrop-2.0's `OpenSlider` (Cartographer 2.02 → Look 'n' Feel → Overlay transparency), in + // its own construction order — the shape that has no `` anywhere: + // `SetThumbTexture(path)` from Lua and nothing else. The reference asks the thumb for its own + // `GetWidth`/`GetHeight` (`0x789ba0` → `[thumb_vt+0x1c]/[+0x20]` = `CSimpleTexture::0x770720` + // /`0x770790`), which falls back to the art's native texel span — 32×32 for + // `UI-SliderBar-Button-Vertical` — so the thumb is a 32-unit knob on a 128-unit track with + // 96 units of travel. Ours read the *authored* size only and, finding none, sized the thumb to + // the whole track: zero travel, a thumb smeared over the bar, and a drag that cannot move. + let mut s = script(); + s.set_screen_size(1024.0, 768.0); + s.set_texture_size_probe(Box::new(|p| { + p.contains("UI-SliderBar-Button-Vertical") + .then_some((32, 32)) + })); + s.run( + r#" + -- The popout is a parentless FULLSCREEN_DIALOG frame that takes the mouse itself, with + -- the slider one frame level above it — so this also pins that the press resolves to the + -- Slider and not to the mouse-enabled host sitting under it. + host = CreateFrame("Frame", "DdHost", nil) + host:SetPoint("BOTTOMLEFT", nil, "BOTTOMLEFT", 100, 100) + host:SetSize(80, 170) + host:SetFrameStrata("FULLSCREEN_DIALOG") + host:EnableMouse(true) + bar = CreateFrame("Slider", "DdSlider", host) + bar:SetFrameLevel(host:GetFrameLevel() + 1) + bar:SetOrientation("VERTICAL") + bar:SetMinMaxValues(0, 1) + bar:SetValueStep(0.01) + bar:SetValue(0.5) + bar:SetWidth(16) + bar:SetHeight(128) + bar:SetPoint("LEFT", host, "LEFT", 15, 0) + bar:SetThumbTexture("Interface\\Buttons\\UI-SliderBar-Button-Vertical") + -- Dewdrop then seats the open value: overlayAlpha is 100 % of a 25 %..100 % range, and + -- the popout inverts it (`1 - (value-min)/(max-min)`), so the engine value is 0 = the + -- TOP of a vertical track. + bar:SetValue(0) + "#, + ) + .unwrap(); + s.resolve(); + + // host y ∈ [100, 270] ⇒ the bar is centred on y=185, 128 tall ⇒ track y ∈ [121, 249]; + // x: host left 100 + 15 ⇒ [115, 131], so the thumb centres on x=123. + assert_eq!( + thumb_rect(&s.extract(), "UI-SliderBar-Button-Vertical"), + (107.0, 139.0, 217.0, 249.0), + "a sizeless thumb is its art's 32×32, flush at the track top — not the whole 16×128 track" + ); + + // And it drags: grab the thumb at its centre and pull to the bottom of the track. + s.mouse_button(123.0, 233.0, "LeftButton", true); + s.mouse_move(123.0, 137.0); + let v: f32 = s.eval("return DdSlider:GetValue()").unwrap(); + assert!( + (v - 1.0).abs() < 1e-3, + "travel is 128−32 = 96, so a 96-unit pull runs the value min→max (got {v})" + ); +} diff --git a/third_party/lua-src/BENILLA.md b/third_party/lua-src/BENILLA.md index bced651f..2bf4531d 100644 --- a/third_party/lua-src/BENILLA.md +++ b/third_party/lua-src/BENILLA.md @@ -41,18 +41,19 @@ downgrade, and it is the entire reason this is a one-hunk fork rather than a ven ## The delta, and the command that proves it -**Current state: six hunks, in three files.** `src/lib.rs` additionally differs by having Lua +**Current state: seven hunks, in three files.** `src/lib.rs` additionally differs by having Lua 5.2/5.3/5.4/5.5 stripped from the `Version` enum (with their source trees deleted) — benilla builds 5.1 and only 5.1, and a fork that still *offered* the other four would answer a request for one with a missing directory at build time instead of a compile error. -**Three hunks restore what 5.1 removed:** +**Four hunks restore what 5.1 changed or removed:** | file | hunk | why | record | |---|---|---|---| | `lvm.c` | 5.0's `OP_TFORPREP` table→`next` substitution, folded into the top of `OP_TFORLOOP` | `for k, v in someTable do` raised "attempt to call a table value"; it was the first session-start error for **60** corpus addons | 1215 | | `luaconf.h` | `LUA_COMPAT_LSTR` `1` → `2` | 5.1 kept 5.0's nesting machinery and put an advisory error in front of it; two corpus addons died on "nesting of `[[...]]` is deprecated" | — | | `lparser.c` | 5.0's compat-semicolon skip restored at the top of `constructor()`'s field loop — 5.0's own line, verbatim, comment included | one extra `;` after a field separator inside a table constructor (`Back_Title = AL["Factions"];;`, ×20 in AtlasLoot's `ButtonRegistry.lua`) parses on the client and died here with "unexpected symbol near `;`"; statement-level `;;` stays rejected by both dialects | 1315 | +| `lparser.c` | `recfield`'s `cc->nh++` moved back INSIDE its `TK_NAME` arm — 5.0's own placement | a `[expr] = value` constructor field must credit **neither** `OP_NEWTABLE` size hint, so the table is born on the dummy node, the first store rehashes, and the dense integer keys land in an ARRAY part that `next` walks ascending. 5.1 pre-sizes a node vector instead and leaves them in the hash, where `next` is slot order — so every list in every saved-variables file came back scrambled (Bagnon's keyring drew first) | 2111 | **Three delete what 5.1 added** — all in `lparser.c`, all byte-read out of the client's own parser (`simpleexp 0x6fd240`, `getunopr 0x6fe0a0`, `getbinopr 0x6fe0c0`), all landing on 5.0's own @@ -84,7 +85,7 @@ diff -r third_party/lua-src/lua-5.1.5 \ ~/.cargo/registry/src/*/lua-src-550.0.0/lua-5.1.5 ``` -That must print **exactly** the six hunks in the tables above and nothing else. If it ever prints +That must print **exactly** the seven hunks in the tables above and nothing else. If it ever prints more, something crept in. ## Status of the payload diff --git a/third_party/lua-src/lua-5.1.5/lparser.c b/third_party/lua-src/lua-5.1.5/lparser.c index 0d9f251a..13e7d04c 100644 --- a/third_party/lua-src/lua-5.1.5/lparser.c +++ b/third_party/lua-src/lua-5.1.5/lparser.c @@ -448,11 +448,11 @@ static void recfield (LexState *ls, struct ConsControl *cc) { int rkkey; if (ls->t.token == TK_NAME) { luaY_checklimit(fs, cc->nh, MAX_INT, "items in a constructor"); + cc->nh++; /* benilla: 5.0's placement -- `[expr] = v` credits NEITHER size hint */ checkname(ls, &key); } else /* ls->t.token == '[' */ yindex(ls, &key); - cc->nh++; checknext(ls, '='); rkkey = luaK_exp2RK(fs, &key); expr(ls, &val);