From 66c9b1b243af9bc830f85f4cee0abb9f215791df Mon Sep 17 00:00:00 2001 From: Sam Date: Sat, 19 Sep 2026 17:10:44 +0700 Subject: [PATCH] widget calls cost one index, and a reparent keeps the layout ledger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - a frame's __index is its kind's method table and the Region map's 19 shared verbs dispatch in Rust: a widget method call costs one table index instead of a Lua re-entry — the idle UI tick ~20 % lighter, client-wide (2310) - SetParent names the subtree it moved instead of giving the layout ledger up: a pooled map note's recycle no longer re-derives the whole interface, and a Questie quest-log change's worst frame halves, 166 → 83 ms (2314) - thirteen single-store windows — honor, talent wipe, the dialog verbs, scoreboard, battlemaster, battle map, book, mirror timers, minimap ping — register their handlers; 18 kinds leave the match (2313) - the addon-enable RE lands: DefaultState is a seeded byte, the node set is the character list, and a character the store has never seen inherits nothing — 2311's one wrong branch corrected (2316) Co-Authored-By: Claude Fable 5.1 --- crates/benilla-app/src/minimap/mod.rs | 1 + crates/benilla-app/src/minimap/ping.rs | 20 ++ crates/benilla-app/src/net/apply.rs | 92 +------- crates/benilla-app/src/net/apply/params.rs | 29 --- crates/benilla-app/src/net/handlers.rs | 3 +- crates/benilla-app/src/ui_battlefield.rs | 30 +++ .../src/ui_battlefield_positions.rs | 21 ++ .../benilla-app/src/ui_battlefield_score.rs | 21 ++ crates/benilla-app/src/ui_dialog_verbs.rs | 69 ++++++ crates/benilla-app/src/ui_honor.rs | 26 +++ crates/benilla-app/src/ui_item_text.rs | 28 +++ crates/benilla-app/src/ui_mirror.rs | 33 +++ crates/benilla-app/src/ui_script/addons.rs | 97 +++++++- crates/benilla-app/src/ui_talent_wipe.rs | 29 +++ crates/benilla-ui/src/script/font.rs | 14 +- crates/benilla-ui/src/script/lua50.rs | 33 +++ crates/benilla-ui/src/script/model.rs | 61 +++++ crates/benilla-ui/src/script/object.rs | 120 +++++++--- .../src/script/object/frame_state.rs | 102 +++++---- .../src/script/object/layout_methods.rs | 212 +++++++++--------- crates/benilla-ui/src/script/region.rs | 74 +++--- crates/benilla-ui/src/script/region/layout.rs | 163 +++++++------- crates/benilla-ui/src/script/region_map.rs | 124 ++++++++-- .../src/script/tests/dispatch_bench.rs | 195 ++++++++++++++++ crates/benilla-ui/src/script/tests/mod.rs | 1 + .../benilla-ui/src/script/tests/setparent.rs | 61 +++++ crates/benilla-ui/src/toc.rs | 47 +++- 27 files changed, 1237 insertions(+), 469 deletions(-) create mode 100644 crates/benilla-ui/src/script/tests/dispatch_bench.rs diff --git a/crates/benilla-app/src/minimap/mod.rs b/crates/benilla-app/src/minimap/mod.rs index 989e04a0..90e972cc 100644 --- a/crates/benilla-app/src/minimap/mod.rs +++ b/crates/benilla-app/src/minimap/mod.rs @@ -1075,6 +1075,7 @@ pub(crate) struct MinimapPlugin; impl Plugin for MinimapPlugin { fn build(&self, app: &mut App) { + ping::register(app); app.add_observer(on_cvar); app.init_resource::() .init_resource::() diff --git a/crates/benilla-app/src/minimap/ping.rs b/crates/benilla-app/src/minimap/ping.rs index 50a71981..166a07ea 100644 --- a/crates/benilla-app/src/minimap/ping.rs +++ b/crates/benilla-app/src/minimap/ping.rs @@ -207,6 +207,26 @@ pub(super) fn drive_minimap_ping( ); } +/// Register the ping's packet handler — called from [`super::MinimapPlugin`] (in the net handler +/// table since 2313). +pub(super) fn register(app: &mut App) { + use crate::net::NetHandlerApp; + app.net_handler( + benilla_protocol::SessionEventKind::MinimapPing, + on_minimap_ping, + ); +} + +/// A group member pinged (decision 1596). The wire carries raw world floats and the relay is +/// stateless in the reference too — we seat them as the pin and the minimap derives the rest. +/// The server only relays a ping between people who are grouped, and a ping from another map +/// would be dropped by the renderer's own map test anyway. +fn on_minimap_ping(In(ev): In, mut ping: ResMut) { + if let benilla_protocol::SessionEvent::MinimapPing { guid, x, y } = ev { + ping.seat((x, y), guid); + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/crates/benilla-app/src/net/apply.rs b/crates/benilla-app/src/net/apply.rs index 1fe60eb6..7313b609 100644 --- a/crates/benilla-app/src/net/apply.rs +++ b/crates/benilla-app/src/net/apply.rs @@ -224,24 +224,11 @@ fn apply_unpeeled( mut world_states, social, mut logout, - mut mirror_timers, mut pet_bar, mut ui_error_keys, - mut page_texts, mut played_time_answer, - mut talent_wipe, - mut pet_unlearn, - mut instance_boot, - mut area_spirit, - mut battlefield_queue, - mut meeting_stone, - mut battlefield_scoreboard, - mut battlefield, mut tutorials, - mut battlefield_positions, mut poi_marker, - mut inspect_honor, - mut ping, } = windows; let Session { mut teleports, @@ -619,13 +606,8 @@ fn apply_unpeeled( session::reputation_visible(list_id, &mut reputations) } SessionEvent::BindPoint { area } => home_bind.0 = Some(area), - // The honor arc's two inbound messages (decision 1512). - // - // The inspect reply REPLACES whatever is held, including for a different player: the - // reference's latch is a single slot, and a pane still showing the last target's - // kills is the failure keeping the old one produces. - SessionEvent::InspectHonorStats(stats) => inspect_honor.0 = Some(stats), - // An honor award: the combat-log line (name-resolved, so it queues) and the floating + // An honor award (decision 1512 — the arc's other inbound message, the inspect reply, + // is `ui_honor`'s own handler): the combat-log line (name-resolved, so it queues) and the floating // number, which are two different surfaces of one packet and are both the reference's. // A DISHONORABLE kill arrives here too, carrying NEGATIVE honor — the floating text // takes it signed, because the shipped `COMBAT_TEXT_HONOR_GAINED` handler prefixes a @@ -642,49 +624,7 @@ fn apply_unpeeled( extra: None, }); } - // A zero trainer guid is vmangos's "you have no talents to reset" refusal, not a - // question — there is nothing to ask about, so nothing goes on screen (decision 1580; - // `crate::ui_talent_wipe`'s header carries why the reference instead re-sends here). - SessionEvent::TalentWipeConfirm { trainer, cost } => { - if trainer == 0 { - debug!("net: talent wipe refused (no talents to reset) — no dialog"); - } else { - debug!("net: trainer {trainer:#x} asks to wipe talents for {cost} copper"); - talent_wipe.ask(trainer, cost); - } - } - // The pet trainer's question (decision 1963) — the talent-wipe twin above; a zero - // guid is the reference's own `ERR_TALENT_WIPE_ERROR` leg, carried over as observed. - SessionEvent::PetUnlearnConfirm { trainer, cost } => { - if trainer == 0 { - debug!("net: pet unlearn refused (zero trainer) — no dialog"); - ui_error_keys - .0 - .push(crate::ui_action::UiError::key("ERR_TALENT_WIPE_ERROR")); - } else { - debug!("net: trainer {trainer:#x} asks to unlearn the pet for {cost} copper"); - pet_unlearn.ask(trainer, cost); - } - } - SessionEvent::RaidGroupOnly { delay_ms, reason } => { - instance_boot.apply(delay_ms, reason, std::time::Instant::now()); - } - SessionEvent::AreaSpiritHealerTime { healer, ms } => { - area_spirit.on_time(healer, ms, std::time::Instant::now()); - } - SessionEvent::BattlefieldStatus(status) => battlefield_queue.apply(status), - SessionEvent::PvpLogData(data) => battlefield_scoreboard.apply(data), - SessionEvent::BattlefieldList(list) => battlefield.apply_list(list), - SessionEvent::GroupJoinedBattleground { result } => battlefield.apply_verdict(result), - SessionEvent::BattlegroundPlayer { guid, joined } => { - battlefield.apply_player(guid, joined); - } - SessionEvent::MeetingStoneSetQueue { area, status } => { - meeting_stone.apply(area, status); - } - SessionEvent::MeetingStoneNotice(notice) => meeting_stone.apply_notice(notice), SessionEvent::TutorialFlags(bytes) => tutorials.apply_flags(&bytes), - SessionEvent::BattlefieldPositions(packet) => battlefield_positions.apply(packet), SessionEvent::Proficiency { item_class, subclass_mask, @@ -949,30 +889,9 @@ fn apply_unpeeled( group::ready_check_request(&mut group, &mut ui_error_keys, &self_guid) } SessionEvent::RaidInstanceInfo { entries } => group.apply_raid_instance_info(entries), - // A group member pinged (decision 1596). The wire carries raw world floats and the - // relay is stateless in the reference too — we seat them as the pin and the minimap - // derives the rest. `map` is the map we are standing on: the server only relays a ping - // between people who are grouped, and a ping from another map would be dropped by the - // renderer's own map test anyway. - SessionEvent::MinimapPing { guid, x, y } => { - ping.seat((x, y), guid); - } SessionEvent::ReadyCheckAnswer { guid, ready } => { group.apply_ready_check_answer(guid, ready != 0) } - // ── The mirror timers (decision 0874): breath / fatigue / feign-death. Pure queue - // arms — every meaning (which bar, what colour, what caption, how fast it drains) - // is resolved at the UI seam in `ui_mirror`, and the countdown itself is the - // FrameXML's own OnUpdate integration ────────────────────────────────────────────── - SessionEvent::MirrorTimerStart(start) => mirror_timers - .0 - .push(crate::ui_mirror::MirrorTimerEdge::Start(start)), - SessionEvent::MirrorTimerPause { kind, paused } => mirror_timers - .0 - .push(crate::ui_mirror::MirrorTimerEdge::Pause { kind, paused }), - SessionEvent::MirrorTimerStop { kind } => mirror_timers - .0 - .push(crate::ui_mirror::MirrorTimerEdge::Stop { kind }), SessionEvent::LootResponse { guid, loot_type, @@ -1633,13 +1552,6 @@ fn apply_unpeeled( opcode, unparseable, } => session::packet_dropped(opcode, unparseable, &mut dropped), - // The book-page cache (decision 1105) — one page per packet, the whole chain in - // answer to the first ask; the reader repaints off it on the next feed. - SessionEvent::PageText { - page_id, - text, - next_page_id, - } => page_texts.insert(page_id, text, next_page_id), SessionEvent::WorldStates { scope, states } => { world::world_states(scope, states, &mut world_states) } diff --git a/crates/benilla-app/src/net/apply/params.rs b/crates/benilla-app/src/net/apply/params.rs index ad29e58c..90d3937e 100644 --- a/crates/benilla-app/src/net/apply/params.rs +++ b/crates/benilla-app/src/net/apply/params.rs @@ -171,47 +171,18 @@ pub(crate) struct WindowStores<'w> { /// The pending logout/quit (decision 0674): the server's response and cancel-ack land here, /// and `crate::ui_logout` turns them into the countdown dialog. pub logout: ResMut<'w, crate::ui_logout::LogoutState>, - /// The mirror-timer queue (decision 0874): the breath/fatigue START/PAUSE/STOP edges, drained - /// into the FrameXML bars by `crate::ui_mirror`. - pub mirror_timers: ResMut<'w, crate::ui_mirror::MirrorTimerFeed>, /// The pet action bar's server-authoritative state + its own cooldown store (decision 0982), /// replaced wholesale on every `SMSG_PET_SPELLS`. pub pet_bar: ResMut<'w, crate::ui_pet::PetBar>, /// The by-key red error queue the pet bar's refused-order feedback rides (the `DisplayError` /// route, resolved through the VM's own GlobalStrings by `ui_action::feed_actions`). pub ui_error_keys: ResMut<'w, crate::ui_action::UiErrorKeys>, - /// The ask-once book-page cache (decision 1105) — every readable's text, keyed by `PageText` - /// id; the reader session repaints off it. - pub page_texts: ResMut<'w, crate::ui_item_text::PageTexts>, pub played_time_answer: ResMut<'w, crate::net::PlayedTimeAnswer>, - /// The class trainer's pending respec question (decision 1580) — the inbound - /// `MSG_TALENT_WIPE_CONFIRM` parks the trainer's guid + cost here and - /// `crate::ui_talent_wipe` turns it into the CONFIRM_TALENT_WIPE dialog, whose Accept is the - /// only thing that unlearns anything. The binder's twin. - pub talent_wipe: ResMut<'w, crate::ui_talent_wipe::TalentWipeState>, - /// The dialog engine's verbs (decision 1963): the pet trainer's latch, the instance-boot - /// clock, the area spirit healer, the battleground queue and the meeting-stone queue — each - /// a feed for a stock dialog. - pub pet_unlearn: ResMut<'w, crate::ui_dialog_verbs::PetUnlearnState>, - pub instance_boot: ResMut<'w, crate::ui_dialog_verbs::InstanceBoot>, - pub area_spirit: ResMut<'w, crate::ui_dialog_verbs::AreaSpiritHealer>, - pub battlefield_queue: ResMut<'w, crate::ui_dialog_verbs::BattlefieldQueue>, - pub meeting_stone: ResMut<'w, crate::ui_dialog_verbs::MeetingStone>, - pub battlefield_scoreboard: ResMut<'w, crate::ui_battlefield_score::BattlefieldScoreboard>, - pub battlefield: ResMut<'w, crate::ui_battlefield::Battlefield>, pub tutorials: ResMut<'w, crate::tutorial::Tutorials>, - pub battlefield_positions: ResMut<'w, crate::ui_battlefield_positions::BattlefieldPositions>, /// The guard's directions marker (`SMSG_GOSSIP_POI`) — the wire carries no map field, so /// "where you were standing when the guard told you" is the client's to remember /// (`crate::poi_marker`). pub poi_marker: ResMut<'w, crate::poi_marker::PoiMarker>, - /// The inspect-honor reply (decision 1512) — `MSG_INSPECT_HONOR_STATS` is the only source of - /// another player's honor numbers, so the reply parks here and `crate::ui_honor` pushes it - /// into the pane and fires `INSPECT_HONOR_UPDATE`. - pub inspect_honor: ResMut<'w, crate::ui_honor::InspectHonor>, - /// The minimap ping (decision 1596) — a group member's `MSG_MINIMAP_PING` seats the world - /// point here and the minimap renderer derives everything else from it. - pub ping: ResMut<'w, crate::minimap::MinimapPing>, } /// The action bar's family: the cast/cooldown state and every error queue the red line drains diff --git a/crates/benilla-app/src/net/handlers.rs b/crates/benilla-app/src/net/handlers.rs index 605c1a3f..d1a1a132 100644 --- a/crates/benilla-app/src/net/handlers.rs +++ b/crates/benilla-app/src/net/handlers.rs @@ -432,7 +432,8 @@ mod tests { continue; }; let kind = kind.trim().rsplit("::").next().unwrap_or_default(); - let handler = handler.trim(); + // rustfmt breaks a long call one argument a line, with a trailing comma. + let handler = handler.trim().trim_end_matches(',').trim_end(); seen += 1; if kind == "Disconnected" { continue; diff --git a/crates/benilla-app/src/ui_battlefield.rs b/crates/benilla-app/src/ui_battlefield.rs index c1dbd71e..c268a5d4 100644 --- a/crates/benilla-app/src/ui_battlefield.rs +++ b/crates/benilla-app/src/ui_battlefield.rs @@ -358,10 +358,40 @@ fn reset_on_world_enter( let _ = commands.0.send(ClientCommand::BattlefieldStatusRequest); } +/// The battlemaster window's packet handlers (in the net handler table since 2313). +mod net { + use benilla_protocol::{SessionEvent, SessionEventKind}; + use bevy::prelude::*; + + use super::Battlefield; + use crate::net::NetHandlerApp; + + /// Register the handlers — called from [`super::BattlefieldPlugin`]. + pub(super) fn register(app: &mut App) { + use SessionEventKind as K; + app.net_handler(K::BattlefieldList, on_packet) + .net_handler(K::GroupJoinedBattleground, on_packet) + .net_handler(K::BattlegroundPlayer, on_packet); + } + + /// One handler for the three: each is a one-line fold into the same state. + fn on_packet(In(ev): In, mut battlefield: ResMut) { + match ev { + SessionEvent::BattlefieldList(list) => battlefield.apply_list(list), + SessionEvent::GroupJoinedBattleground { result } => battlefield.apply_verdict(result), + SessionEvent::BattlegroundPlayer { guid, joined } => { + battlefield.apply_player(guid, joined) + } + _ => {} + } + } +} + pub(crate) struct BattlefieldPlugin; impl Plugin for BattlefieldPlugin { fn build(&self, app: &mut App) { + net::register(app); app.init_resource::().add_systems( Update, ( diff --git a/crates/benilla-app/src/ui_battlefield_positions.rs b/crates/benilla-app/src/ui_battlefield_positions.rs index 0b8aa1c1..f92c340d 100644 --- a/crates/benilla-app/src/ui_battlefield_positions.rs +++ b/crates/benilla-app/src/ui_battlefield_positions.rs @@ -223,10 +223,31 @@ fn reset_on_world_enter( state.last_request = None; } +/// The battlefield map's packet handler (in the net handler table since 2313). +mod net { + use benilla_protocol::{SessionEvent, SessionEventKind}; + use bevy::prelude::*; + + use super::BattlefieldPositions; + use crate::net::NetHandlerApp; + + /// Register the handler — called from [`super::BattlefieldPositionsPlugin`]. + pub(super) fn register(app: &mut App) { + app.net_handler(SessionEventKind::BattlefieldPositions, on_positions); + } + + fn on_positions(In(ev): In, mut positions: ResMut) { + if let SessionEvent::BattlefieldPositions(packet) = ev { + positions.apply(packet); + } + } +} + pub(crate) struct BattlefieldPositionsPlugin; impl Plugin for BattlefieldPositionsPlugin { fn build(&self, app: &mut App) { + net::register(app); app.init_resource::().add_systems( Update, ( diff --git a/crates/benilla-app/src/ui_battlefield_score.rs b/crates/benilla-app/src/ui_battlefield_score.rs index d058da2d..b6fed6ce 100644 --- a/crates/benilla-app/src/ui_battlefield_score.rs +++ b/crates/benilla-app/src/ui_battlefield_score.rs @@ -171,10 +171,31 @@ fn drain_battlefield_score( } } +/// The scoreboard's packet handler (in the net handler table since 2313). +mod net { + use benilla_protocol::{SessionEvent, SessionEventKind}; + use bevy::prelude::*; + + use super::BattlefieldScoreboard; + use crate::net::NetHandlerApp; + + /// Register the handler — called from [`super::BattlefieldScorePlugin`]. + pub(super) fn register(app: &mut App) { + app.net_handler(SessionEventKind::PvpLogData, on_pvp_log_data); + } + + fn on_pvp_log_data(In(ev): In, mut board: ResMut) { + if let SessionEvent::PvpLogData(data) = ev { + board.apply(data); + } + } +} + pub(crate) struct BattlefieldScorePlugin; impl Plugin for BattlefieldScorePlugin { fn build(&self, app: &mut App) { + net::register(app); app.init_resource::().add_systems( Update, ( diff --git a/crates/benilla-app/src/ui_dialog_verbs.rs b/crates/benilla-app/src/ui_dialog_verbs.rs index 214a64bb..95f336d0 100644 --- a/crates/benilla-app/src/ui_dialog_verbs.rs +++ b/crates/benilla-app/src/ui_dialog_verbs.rs @@ -1078,10 +1078,79 @@ fn poll_area_spirit_healer( } } +/// The dialog verbs' packet handlers (in the net handler table since 2313) — each parks a +/// question or a countdown on its own store for the feed to turn into a StaticPopup. +mod net { + use benilla_protocol::{SessionEvent, SessionEventKind}; + use bevy::prelude::*; + + use super::{AreaSpiritHealer, BattlefieldQueue, InstanceBoot, MeetingStone, PetUnlearnState}; + use crate::net::NetHandlerApp; + + /// Register the handlers — called from [`super::UiDialogVerbsPlugin`]. + pub(super) fn register(app: &mut App) { + use SessionEventKind as K; + app.net_handler(K::PetUnlearnConfirm, on_pet_unlearn_confirm) + .net_handler(K::RaidGroupOnly, on_raid_group_only) + .net_handler(K::AreaSpiritHealerTime, on_area_spirit_healer_time) + .net_handler(K::BattlefieldStatus, on_battlefield_status) + .net_handler(K::MeetingStoneSetQueue, on_meeting_stone) + .net_handler(K::MeetingStoneNotice, on_meeting_stone); + } + + /// The pet trainer's question (decision 1963) — the talent wipe's twin + /// ([`crate::ui_talent_wipe`]); a zero guid is the reference's own `ERR_TALENT_WIPE_ERROR` + /// leg, carried over as observed. + fn on_pet_unlearn_confirm( + In(ev): In, + mut unlearn: ResMut, + mut errors: ResMut, + ) { + if let SessionEvent::PetUnlearnConfirm { trainer, cost } = ev { + if trainer == 0 { + debug!("net: pet unlearn refused (zero trainer) — no dialog"); + errors + .0 + .push(crate::ui_action::UiError::key("ERR_TALENT_WIPE_ERROR")); + } else { + debug!("net: trainer {trainer:#x} asks to unlearn the pet for {cost} copper"); + unlearn.ask(trainer, cost); + } + } + } + + fn on_raid_group_only(In(ev): In, mut boot: ResMut) { + if let SessionEvent::RaidGroupOnly { delay_ms, reason } = ev { + boot.apply(delay_ms, reason, std::time::Instant::now()); + } + } + + fn on_area_spirit_healer_time(In(ev): In, mut spirit: ResMut) { + if let SessionEvent::AreaSpiritHealerTime { healer, ms } = ev { + spirit.on_time(healer, ms, std::time::Instant::now()); + } + } + + fn on_battlefield_status(In(ev): In, mut queue: ResMut) { + if let SessionEvent::BattlefieldStatus(status) = ev { + queue.apply(status); + } + } + + fn on_meeting_stone(In(ev): In, mut stone: ResMut) { + match ev { + SessionEvent::MeetingStoneSetQueue { area, status } => stone.apply(area, status), + SessionEvent::MeetingStoneNotice(notice) => stone.apply_notice(notice), + _ => {} + } + } +} + pub(crate) struct UiDialogVerbsPlugin; impl Plugin for UiDialogVerbsPlugin { fn build(&self, app: &mut App) { + net::register(app); app.init_resource::() .init_resource::() .init_resource::() diff --git a/crates/benilla-app/src/ui_honor.rs b/crates/benilla-app/src/ui_honor.rs index 813631ac..8fa2b308 100644 --- a/crates/benilla-app/src/ui_honor.rs +++ b/crates/benilla-app/src/ui_honor.rs @@ -84,10 +84,36 @@ struct HonorFeedMemo { last_inspect: Option, } +/// The honor pane's packet handler (decision 1512; in the net handler table since 2313). The +/// honor arc's other inbound message, the award (`PvpCredit`), is a chat line and a floating +/// number and stays with the chat family. +mod net { + use benilla_protocol::{SessionEvent, SessionEventKind}; + use bevy::prelude::*; + + use super::InspectHonor; + use crate::net::NetHandlerApp; + + /// Register the handler — called from [`super::UiHonorPlugin`]. + pub(super) fn register(app: &mut App) { + app.net_handler(SessionEventKind::InspectHonorStats, on_inspect_stats); + } + + /// The inspect reply REPLACES whatever is held, including for a different player: the + /// reference's latch is a single slot, and a pane still showing the last target's kills is + /// the failure keeping the old one produces. + fn on_inspect_stats(In(ev): In, mut inspect: ResMut) { + if let SessionEvent::InspectHonorStats(stats) = ev { + inspect.0 = Some(stats); + } + } +} + pub(crate) struct UiHonorPlugin; impl Plugin for UiHonorPlugin { fn build(&self, app: &mut App) { + net::register(app); app.init_resource::() .init_resource::() .add_systems(Update, feed_honor.in_set(UiFeed)); diff --git a/crates/benilla-app/src/ui_item_text.rs b/crates/benilla-app/src/ui_item_text.rs index bfb0caff..1e8b1e2a 100644 --- a/crates/benilla-app/src/ui_item_text.rs +++ b/crates/benilla-app/src/ui_item_text.rs @@ -178,10 +178,38 @@ impl ItemTextOpen { } } +/// The book reader's packet handler (decision 1105; in the net handler table since 2313). +mod net { + use benilla_protocol::{SessionEvent, SessionEventKind}; + use bevy::prelude::*; + + use super::PageTexts; + use crate::net::NetHandlerApp; + + /// Register the handler — called from [`super::UiItemTextPlugin`]. + pub(super) fn register(app: &mut App) { + app.net_handler(SessionEventKind::PageText, on_page_text); + } + + /// The book-page cache — one page per packet, the whole chain in answer to the first ask; + /// the reader repaints off it on the next feed. + fn on_page_text(In(ev): In, mut pages: ResMut) { + if let SessionEvent::PageText { + page_id, + text, + next_page_id, + } = ev + { + pages.insert(page_id, text, next_page_id); + } + } +} + pub(crate) struct UiItemTextPlugin; impl Plugin for UiItemTextPlugin { fn build(&self, app: &mut App) { + net::register(app); crate::query_cache::register::(app); app.init_resource::() .init_resource::() diff --git a/crates/benilla-app/src/ui_mirror.rs b/crates/benilla-app/src/ui_mirror.rs index ea3daed0..9138db98 100644 --- a/crates/benilla-app/src/ui_mirror.rs +++ b/crates/benilla-app/src/ui_mirror.rs @@ -191,10 +191,43 @@ fn feed_mirror_timers( /// The mirror-timer UI seam: the queue + its drain, ordered like the cast bar's — before the VM /// ticks, so an edge and its first OnUpdate land on the same frame. +/// The mirror timers' packet handlers (decision 0874; in the net handler table since 2313): +/// breath / fatigue / feign-death. Pure queue handlers — every meaning (which bar, what colour, +/// what caption, how fast it drains) is resolved at the UI seam in this module, and the +/// countdown itself is the FrameXML's own OnUpdate integration. +mod net { + use benilla_protocol::{SessionEvent, SessionEventKind}; + use bevy::prelude::*; + + use super::{MirrorTimerEdge, MirrorTimerFeed}; + use crate::net::NetHandlerApp; + + /// Register the handlers — called from [`super::UiMirrorPlugin`]. + pub(super) fn register(app: &mut App) { + use SessionEventKind as K; + app.net_handler(K::MirrorTimerStart, on_edge) + .net_handler(K::MirrorTimerPause, on_edge) + .net_handler(K::MirrorTimerStop, on_edge); + } + + fn on_edge(In(ev): In, mut feed: ResMut) { + let edge = match ev { + SessionEvent::MirrorTimerStart(start) => MirrorTimerEdge::Start(start), + SessionEvent::MirrorTimerPause { kind, paused } => { + MirrorTimerEdge::Pause { kind, paused } + } + SessionEvent::MirrorTimerStop { kind } => MirrorTimerEdge::Stop { kind }, + _ => return, + }; + feed.0.push(edge); + } +} + pub(crate) struct UiMirrorPlugin; impl Plugin for UiMirrorPlugin { fn build(&self, app: &mut App) { + net::register(app); app.init_resource::() .add_systems(Update, feed_mirror_timers.in_set(UnitFeed)); } diff --git a/crates/benilla-app/src/ui_script/addons.rs b/crates/benilla-app/src/ui_script/addons.rs index 95cb68b0..ce8ed73f 100644 --- a/crates/benilla-app/src/ui_script/addons.rs +++ b/crates/benilla-app/src/ui_script/addons.rs @@ -855,6 +855,11 @@ pub(crate) fn installed_rows() -> Vec { /// `ds:0xbe1bd8`), which `AddOnList_LoadCharacter 0x51ebe0` fills one node per character at /// char-list population (wow-5875-re `system/ui/scratch/addon-enable-store.md` §1/§5). /// +/// That the node set is the **char-list, in wire order, rebuilt whole** is verified rather than +/// assumed (wow-re `addon-defaultstate-and-node-set.md`, decision 2316): the per-record callback +/// `0x472300` is handed to an enumerator that walks every `SMSG_CHAR_ENUM` record with no filter +/// and no early-out, and its driver destroys every existing node first (`0x51f0b0(NULL)`). +/// /// **A node with no file is EMPTY, not absent**, and the difference is the whole point: an empty /// node contributes no opinion to the aggregate, but it is still a character whose enable bit has /// to be answered — and the reference answers it from the *other* characters, never with a bare @@ -917,25 +922,37 @@ impl EnableStore { } /// **The bit a character actually gets** — `0x51e470(addon, character, useDefault = 1)` - /// lowered to a bool (a single-character query returns only 0 or 2, §4). + /// lowered to a bool (a single-character query returns only 0 or 2). /// - /// Their own explicit row when their file has one; otherwise the aggregate above, and where - /// that is undecided, the addon's `## DefaultState`. `None` is the no-character case (nobody - /// picked yet), which is the aggregate alone. + /// Three cases, and the reference distinguishes the last two (decision 2316, which corrects + /// 2311's reading of them as one): + /// + /// * **their file has an explicit row** → that row, and nothing else is consulted; + /// * **they have a node but no row for this addon** → [`Self::aggregate`], the explicit-only + /// fold over the character list (`0x51e5f0`'s self-recursion), falling to `## DefaultState` + /// only where that is undecided; + /// * **no node carries their name at all** → `## DefaultState`, *not* the aggregate. The walk + /// compares the name at every node and advances past each mismatch + /// (`0x51e55a jne 0x51e611`, bypassing the stop-check at `0x51e60a`), so it exhausts with + /// `total == 0` and takes that epilogue. A character the list does not carry inherits + /// nothing. `None` — nobody picked yet — is this case too. + /// + /// That third arm is unreachable from our own callers: every panel column is a node, and + /// [`store_nodes`] seats the loading character. It is written faithfully anyway, because the + /// next caller would otherwise find the wrong branch sitting here and have no way to know. pub(crate) fn enabled_for( &self, addon: &str, default_state: bool, character: Option<&str>, ) -> bool { - let key = addon.to_ascii_lowercase(); - if let Some(explicit) = character - .and_then(|c| self.node(c)) - .and_then(|h| h.get(&key)) - { - return *explicit; + let Some(node) = character.and_then(|c| self.node(c)) else { + return default_state; + }; + match node.get(&addon.to_ascii_lowercase()) { + Some(&explicit) => explicit, + None => self.aggregate(addon).unwrap_or(default_state), } - self.aggregate(addon).unwrap_or(default_state) } fn node(&self, character: &str) -> Option<&HashMap> { @@ -2964,6 +2981,64 @@ mod tests { let _ = std::fs::remove_dir_all(home.parent().unwrap()); } + /// **A name the character list does not carry inherits nothing** — the third arm of + /// `0x51e470(addon, character, useDefault = 1)`, and the one 2311 got wrong (decision 2316). + /// + /// 2311 read "no explicit row" and "no node" as one case and sent both to the aggregate. The + /// reference splits them: the walk compares the name at every node and advances past each + /// mismatch (`0x51e55a jne 0x51e611`, bypassing the stop-check), so an unknown name exhausts + /// the list with `total == 0` and takes the `DefaultState ? 2 : 0` epilogue. Only a character + /// that *has* a node inherits. + /// + /// Unreachable from our own callers — every panel column is a node, and `store_nodes` seats + /// the loading character — so this is the falsifier standing in for the caller that does not + /// exist yet. `Known`, who has a node and no row, is the control that keeps the two arms + /// honestly distinguishable: both addons are unanimous, so an implementation that collapsed + /// the cases would answer `false` for `Stranger` too. + #[test] + fn an_unknown_character_takes_the_manifest_default_not_the_aggregate() { + let _l = crate::local_state::test_env::ENV_LOCK + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + let _c = crate::local_state::test_env::EnvGuard::unset("WOW_CAPTURE"); + let (home, _h) = hermetic_root("unknown-char"); + // One unanimous `disabled`, and one addon whose manifest ships `DefaultState: disabled` + // so the epilogue's two outcomes are told apart rather than both reading `true`. + write_addon(&home, "Shunned", "## Interface: 11200\n", &[]); + write_addon( + &home, + "OptIn", + "## Interface: 11200\n## DefaultState: disabled\n", + &[], + ); + for who in ["Onemage", "Onerogue"] { + let id = ("Realm".to_string(), who.to_string()); + write_enable_state( + Some(&id), + &[("Shunned".into(), false), ("OptIn".into(), true)], + ); + } + let roster = [ + "Onemage".to_string(), + "Onerogue".to_string(), + "Known".to_string(), + ]; + let store = EnableStore::load("Realm", &roster); + + // `Known` has a node and no rows: both addons inherit their unanimous aggregate. + assert!(!store.enabled_for("Shunned", true, Some("Known"))); + assert!(store.enabled_for("OptIn", false, Some("Known"))); + + // `Stranger` is on no node: neither aggregate reaches them — each addon answers with its + // own manifest default, which is the opposite verdict in both cases. + assert!(store.enabled_for("Shunned", true, Some("Stranger"))); + assert!(!store.enabled_for("OptIn", false, Some("Stranger"))); + // …and `None` (nobody picked yet) is the same epilogue. + assert!(store.enabled_for("Shunned", true, None)); + assert!(!store.enabled_for("OptIn", false, None)); + let _ = std::fs::remove_dir_all(home.parent().unwrap()); + } + /// Enable state survives the session: `DisableAddOn` from Lua is written back in the /// reference's own `AddOns.txt` format, and read back as a disable next time. #[test] diff --git a/crates/benilla-app/src/ui_talent_wipe.rs b/crates/benilla-app/src/ui_talent_wipe.rs index fd74b447..6179d589 100644 --- a/crates/benilla-app/src/ui_talent_wipe.rs +++ b/crates/benilla-app/src/ui_talent_wipe.rs @@ -196,10 +196,39 @@ fn drain_talent_wipe( } /// The respec flow: the range guard, the dialog's feed, and its answer. +/// The talent-wipe question's packet handler (decision 1580; in the net handler table since 2313). +mod net { + use benilla_protocol::{SessionEvent, SessionEventKind}; + use bevy::prelude::*; + + use super::TalentWipeState; + use crate::net::NetHandlerApp; + + /// Register the handler — called from [`super::UiTalentWipePlugin`]. + pub(super) fn register(app: &mut App) { + app.net_handler(SessionEventKind::TalentWipeConfirm, on_confirm); + } + + /// A zero trainer guid is vmangos's "you have no talents to reset" refusal, not a question — + /// there is nothing to ask about, so nothing goes on screen (this module's header carries + /// why the reference instead re-sends here). + fn on_confirm(In(ev): In, mut wipe: ResMut) { + if let SessionEvent::TalentWipeConfirm { trainer, cost } = ev { + if trainer == 0 { + debug!("net: talent wipe refused (no talents to reset) — no dialog"); + } else { + debug!("net: trainer {trainer:#x} asks to wipe talents for {cost} copper"); + wipe.ask(trainer, cost); + } + } + } +} + pub(crate) struct UiTalentWipePlugin; impl Plugin for UiTalentWipePlugin { fn build(&self, app: &mut App) { + net::register(app); app.init_resource::().add_systems( Update, ( diff --git a/crates/benilla-ui/src/script/font.rs b/crates/benilla-ui/src/script/font.rs index 35f6ac64..a09647b0 100644 --- a/crates/benilla-ui/src/script/font.rs +++ b/crates/benilla-ui/src/script/font.rs @@ -550,14 +550,14 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; - lua.set_named_registry_value(REG_FONT_METHODS, m)?; - let meta = lua.create_table()?; - let index = lua.create_function(|lua, (_this, key): (Table, Value)| { - let methods: Table = lua.named_registry_value(REG_FONT_METHODS)?; - methods.get::(key) - })?; - meta.set("__index", index)?; + // **`__index` is the method TABLE, not a dispatcher function** (decision 2310). A Rust + // `__index` turns every `fo.GetFont` — a plain table index in the source — into a Lua→Rust→Lua + // round trip plus a named-registry string lookup; measured at ~200 ns against ~9 ns for the + // table form, on a path every widget call in the client begins with. The table is mutated in + // place by nothing after this point, so pointing at it cannot go stale. + meta.set("__index", m.clone())?; + lua.set_named_registry_value(REG_FONT_METHODS, m)?; lua.set_named_registry_value(REG_FONT_META, meta)?; lua.globals() diff --git a/crates/benilla-ui/src/script/lua50.rs b/crates/benilla-ui/src/script/lua50.rs index 3648cc7a..4b9ed079 100644 --- a/crates/benilla-ui/src/script/lua50.rs +++ b/crates/benilla-ui/src/script/lua50.rs @@ -58,6 +58,17 @@ //! answers 2 in this VM. That matters for 1751: a stock file using 5.0 varargs needs nothing from //! this module, and the rewrite was a precaution against a problem we did not have. //! +//! **And the other half of that, which costs an hour to rediscover: `...` is a DECLARATION token +//! here and nothing else.** This VM parses `function(x, ...)`, and then rejects every *use* of +//! `...` as an expression — `return ...`, `f(x, ...)`, and `local a = ...` at chunk level — with +//! `unexpected symbol near '...'`. `arg` and `unpack(arg)` are the only forwarding forms +//! ([`vararg_is_a_declaration_only`] pins all six). That is 5.0's own shape and exactly right for +//! the content we run, but it also binds anything WE write in Lua: an engine-side helper cannot +//! forward an unknown argument list without a table, so a verb whose arity matters (`GetPoint()` +//! vs `GetPoint(nil)`, `SetAllPoints()` vs `SetAllPoints(nil)`) has to stay on the Rust side. +//! Decision 2310 is where that bit — a Lua dispatcher for the Region method map would have been +//! ~4x cheaper than the Rust one and cannot be written in this dialect. +//! //! ## The one known divergence, stated rather than hidden //! //! `table.insert`/`table.remove` stay on 5.1's `#t` border rather than consulting `getn`. **This @@ -780,6 +791,28 @@ mod tests { assert_eq!(s.eval::("return floor(2.7)").unwrap(), 2.0); } + /// **`...` declares a vararg function here and can never be read.** The six forms, together, + /// because the split between them is the whole surprise: the declaration parses, every use + /// does not, and `arg` carries what `...` cannot (the module header's second vararg note). + #[test] + fn vararg_is_a_declaration_only() { + let s = UiScript::new().unwrap(); + let parses = |src: &str| s.run(src).is_ok(); + // Declaration, and the 5.0 way to read what it captured. + assert!(parses("return function(x, ...) return x end")); + assert!(parses("return function(x, ...) return arg.n end")); + assert!(parses( + "local f = tostring return function(x, ...) return f(x, unpack(arg)) end" + )); + // Every use of `...` as an expression — including at chunk level, where 5.1 proper allows + // it and this build does not. + assert!(!parses("return function(x, ...) return ... end")); + assert!(!parses( + "local f = tostring return function(x, ...) return f(x, ...) end" + )); + assert!(!parses("local a = ... return a")); + } + /// The three compat globals the reference has and we did not. #[test] fn sort_foreach_and_foreachi_are_bare_globals_like_the_reference() { diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index 5995ef85..aee5f845 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -2386,6 +2386,67 @@ impl Model { } } + /// A **REPARENT** that names its nodes: `SetParent` moved the effective scale of every frame + /// in the subtree rooted at `root`, and moved nothing else the layout graph is made of + /// (decision 2314, extending 1388). + /// + /// ## Why a reparent is a value-only write here + /// + /// `LayoutInput.scale` is the only layout input a reparent touches: the arena recomputes the + /// subtree's `effective_scale` and the derive syncs it per frame. Nothing else moves — + /// **an anchor's target is an id resolved at `SetPoint` time** ([`super::object::anchor_args:: + /// resolve_rel_target`]), not a live "my parent" link, so no edge is retargeted; no node is + /// born or dies, so the roster is unchanged; and visibility, strata and level are not layout + /// inputs at all (the derive walks liveness, not `effective_visible`). A region carries no + /// scale of its own — it reads its owner frame's — so naming the frames is enough: the dirty + /// closure pulls the regions that read them. + /// + /// ## Why it is worth naming a whole subtree + /// + /// `SetParent` used to take [`Self::touch_layout`], on the reading that a reparent is + /// human-rate. **It is not.** A pooled widget's recycle is a reparent, and an addon's map-note + /// pool recycles hundreds per redraw: with the director's addon set, every quest-log change + /// cost two frames of ~160 ms, of which `WOW_LAYOUT_PROF` attributed **128 ms to 58 full + /// graph derivations** — 5,267 frames and 16,124 anchored regions walked and hashed, ~3.2 ms + /// each, and `WOW_LAYOUT_DERIVE_TRACE` put all fourteen sampled give-ups on this one call. + /// A subtree is a handful of nodes; the roster is twenty thousand. + /// + /// Falls back to the conservative touch the moment a node of the subtree is not a node of the + /// cached graph — [`Self::touch_layout_frame`]'s own rule, and the same "worst case is a + /// derivation" safety every precise touch has. + pub(crate) fn touch_layout_reparent(&mut self, root: FrameHandle) { + // The walk first, then the writing: `touch_layout_frame` takes `&mut self` and the arena + // walk holds `&self`. + let mut subtree = vec![root]; + let mut at = 0; + while at < subtree.len() { + let h = subtree[at]; + at += 1; + if let Some(f) = self.arena.frame(h) { + subtree.extend(f.children.iter().copied()); + } + } + // **The write itself, not only its name.** `LayoutInput.scale` was synced from the arena + // inside [`super::UiScript::resolve_layout`]'s DERIVATION — so a pass that skips the + // derivation never learned the new scale, and a named-but-unwritten node is the one shape + // that ships a stale rect. `WOW_LAYOUT_VERIFY` caught exactly that on the first run of + // `setparent::a_reparent_under_a_scale_change_moves_the_childs_rect`; naming is a claim + // about what moved, and this is what makes the claim true. + let Model { + arena, + layout_inputs, + .. + } = self; + for &h in &subtree { + if let Some(f) = arena.frame(h) { + layout_inputs.entry(h).or_default().scale = f.effective_scale; + } + } + for h in subtree { + self.touch_layout_frame(h); + } + } + /// An anchor RETARGET that names its node: the write moved node `h`'s (or `rh`'s) set of /// anchor TARGETS, and hands over both target lists so the cached graph's edges can be /// re-pointed instead of thrown away (decision 1625, extending 1388). diff --git a/crates/benilla-ui/src/script/object.rs b/crates/benilla-ui/src/script/object.rs index d62eb158..7218d96d 100644 --- a/crates/benilla-ui/src/script/object.rs +++ b/crates/benilla-ui/src/script/object.rs @@ -79,18 +79,22 @@ pub(super) fn frame_wrapper(lua: &Lua, id: u32) -> mlua::Result { } let t = lua.create_table()?; t.raw_set(0, Value::LightUserData(id_to_lud(id)))?; - let meta: Table = lua.named_registry_value(REG_FRAME_META)?; - t.set_metatable(Some(meta))?; - wrappers.set(id, t.clone())?; - - let name: Option = { + // The frame's kind and name in one borrow. **The KIND picks the metatable, here rather than + // per lookup** — the region side's shape since its leaves split, and decision 2310's whole + // change on this side: a wrapper is built once per frame, so resolving the class chain at + // construction costs nothing, while resolving it inside `__index` put a Rust call, a model + // borrow and a registry lookup in front of EVERY widget method access in the client. A frame's + // kind never changes and `id_to_frame` never loses an entry, so the choice cannot go stale. + let (kind, name): (Option, Option) = { let model = lua.app_data_ref::().expect("model app_data"); - model + let frame = model .id_to_frame .get(&id) - .and_then(|h| model.arena.frame(*h)) - .and_then(|f| f.name.clone()) + .and_then(|h| model.arena.frame(*h)); + (frame.map(|f| f.kind), frame.and_then(|f| f.name.clone())) }; + t.set_metatable(Some(frame_meta_for(lua, kind)?))?; + wrappers.set(id, t.clone())?; if let Some(name) = name { publish_global(lua, &name, &t)?; } @@ -409,6 +413,9 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { lua.set_named_registry_value(REG_WRAPPERS, lua.create_table()?)?; lua.set_named_registry_value(REG_SCRIPTS, lua.create_table()?)?; + // The Region-map arm collection, opened before either method table is built and closed by + // `region_map::install` below (its [`super::region_map::Arms`] doc says why it is app_data). + super::region_map::open_arms(lua); install_frame_methods(lua)?; super::region::install(lua)?; super::statusbar::install(lua)?; @@ -418,23 +425,10 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { // implementation each, so a method pulled off a frame works on a texture (decision 1501). super::region_map::install(lua)?; - // Shared frame metatable: __index is a Rust dispatcher over the frame method table (RF-0023), - // checking the frame's *kind-specific* method table first. Per-kind resolution matters beyond - // correctness: addons duck-type widgets (`if frame.SetValue then …`), so a plain frame must - // resolve `SetValue` to nil — one shared table would make every frame quack like every widget. - let frame_meta = lua.create_table()?; - let frame_index = lua.create_function(|lua, (this, key): (Table, Value)| { - for reg in kind_method_registries(lua, &this) { - let methods: Table = lua.named_registry_value(reg)?; - let v = methods.get::(key.clone())?; - if !v.is_nil() { - return Ok(v); - } - } - let methods: Table = lua.named_registry_value(REG_FRAME_METHODS)?; - methods.get::(key) - })?; - frame_meta.set("__index", frame_index)?; + // The per-kind metatable cache ([`frame_meta_for`]) and the base metatable in it: the one a + // plain `Frame` wears, whose `__index` is the shared frame method table itself. + lua.set_named_registry_value(REG_KIND_METAS, lua.create_table()?)?; + let frame_meta = frame_meta_for(lua, None)?; lua.set_named_registry_value(REG_FRAME_META, frame_meta.clone())?; // RF-0023 publishes the shared metatable as _G["__framescript_meta"]. lua.globals().set("__framescript_meta", frame_meta)?; @@ -536,17 +530,71 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { Ok(()) } -/// The registry keys of a frame's kind-specific method tables, in resolution order — the `__index` -/// dispatcher walks them before the shared table, mirroring the client's class chain (CheckButton -/// resolves through Button's map, RF-28). Empty for plain kinds (and for anything that isn't a live -/// frame, e.g. a region wrapper passing through). -fn kind_method_registries(lua: &Lua, this: &Table) -> &'static [&'static str] { - let Ok(id) = decode_id(this) else { return &[] }; - let model = lua.app_data_ref::().expect("model app_data"); - let Some(h) = model.id_to_frame.get(&id) else { - return &[]; - }; - match model.arena.frame(*h).map(|f| f.kind) { +/// The registry table of per-kind frame metatables, keyed by the head of the kind's method chain +/// (`""` for a kind with none). Lua-side, like every other root here — the MAXCSTACK discipline. +const REG_KIND_METAS: &str = "__benilla_frame_meta_by_kind"; + +/// The metatable a frame of `kind` wears — built once per kind, cached in [`REG_KIND_METAS`]. +/// +/// ## `__index` is a TABLE, and that is the whole point (decision 2310) +/// +/// This used to be one shared metatable whose `__index` was a **Rust function** that resolved the +/// receiver's kind and walked its registries per lookup. Every `frame:SetPoint(...)` — every plain +/// `frame.Foo` read, including the `if frame.SetValue then` duck-type probe addons open with — +/// therefore crossed into Rust, took a model borrow, resolved a handle and did one to three +/// named-registry string lookups. Measured in a release build: **217 ns for a plain Frame, 320 ns +/// for a Button, against 8.7 ns for a plain Lua table `__index`.** A live sample of a quest-accept +/// spike with the director's addon set put ~20 % of the whole UI tick inside that metamethod +/// before any method body ran — Questie redraws its map notes with ~19 widget calls per cluster +/// over hundreds of clusters, and pays the tax on every one. +/// +/// A table costs one `luaH_get` inside `luaV_gettable`, in C, with no call at all. +/// +/// ## The chain is the reference's own, expressed as metatables +/// +/// 1.12's widget classes are not a Lua metatable chain — each class owns a flat method map and its +/// lookup tail-calls exactly one base's lookup ([`super::region_map`]'s header has the tables). But +/// `luaV_gettable` follows a table `__index` iteratively, so a chain of metatables IS that probe +/// sequence: `CheckButton`'s map, miss, `Button`'s map, miss, `Frame`'s map. So rather than merging +/// each kind's methods into one flat copy, this links the existing tables — no copies to go stale +/// when a method table is written after the fact, and the per-kind surface stays exactly what +/// [`kind_method_registries`] declares. Duck typing is preserved by construction: a plain Frame's +/// chain never reaches `StatusBar`'s map, so `frame.SetValue` is still nil. +fn frame_meta_for(lua: &Lua, kind: Option) -> mlua::Result
{ + let chain = kind_method_registries(kind); + let key = chain.first().copied().unwrap_or(""); + let metas: Table = lua.named_registry_value(REG_KIND_METAS)?; + if let Value::Table(meta) = metas.raw_get::(key)? { + return Ok(meta); + } + // Link `own -> base -> ... -> Frame`, idempotently: a base table shared by several kinds (every + // button kind reaches `Button`'s) is simply re-pointed at the same place. + let frame_methods: Table = lua.named_registry_value(REG_FRAME_METHODS)?; + let mut below = frame_methods.clone(); + for reg in chain.iter().rev() { + let own: Table = lua.named_registry_value(reg)?; + let link = lua.create_table()?; + link.set("__index", below)?; + own.set_metatable(Some(link))?; + below = own; + } + let meta = lua.create_table()?; + meta.set("__index", below)?; + metas.raw_set(key, meta.clone())?; + Ok(meta) +} + +/// The registry keys of a frame's kind-specific method tables, **in resolution order** — the +/// client's own class chain (CheckButton resolves through Button's map, RF-28). Empty for a plain +/// kind, and for a wrapper whose frame is not live. +/// +/// Every chain here is *its own table followed by its base's whole chain*, which is what lets +/// [`frame_meta_for`] realize it as a metatable chain instead of a search: `CheckButton`'s table +/// gets `Button`'s as its `__index`, `Button`'s gets `Frame`'s, and one `luaV_gettable` walks the +/// lot in C. Keep that property when adding a kind — a chain that is not a suffix of its base's +/// cannot be expressed this way. +fn kind_method_registries(kind: Option) -> &'static [&'static str] { + match kind { Some(FrameKind::StatusBar) => &[super::statusbar::REG_STATUSBAR_METHODS], Some(FrameKind::EditBox) => &[super::editbox::REG_EDITBOX_METHODS], Some(FrameKind::ScrollingMessageFrame) => { diff --git a/crates/benilla-ui/src/script/object/frame_state.rs b/crates/benilla-ui/src/script/object/frame_state.rs index 39930b2d..8634faab 100644 --- a/crates/benilla-ui/src/script/object/frame_state.rs +++ b/crates/benilla-ui/src/script/object/frame_state.rs @@ -6,6 +6,8 @@ use mlua::{Lua, Table, Value}; +use crate::script::region_map::{set_shared, Side}; + use crate::order::Strata; use crate::script::{event, Backdrop, Insets, Model}; use crate::widget::FrameKind; @@ -51,20 +53,17 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { })?, )?; // Identity / hierarchy - m.set( - "GetName", - lua.create_function(|lua, this: Table| { - let h = frame_handle_of(lua, &this)?; - let name = { - let model = lua.app_data_ref::().expect("model"); - model.arena.frame(h).and_then(|f| f.name.clone()) - }; - match name { - Some(n) => Ok(Value::String(lua.create_string(&n)?)), - None => Ok(Value::Nil), - } - })?, - )?; + set_shared(lua, m, Side::Frame, "GetName", |lua, this: Table| { + let h = frame_handle_of(lua, &this)?; + let name = { + let model = lua.app_data_ref::().expect("model"); + model.arena.frame(h).and_then(|f| f.name.clone()) + }; + match name { + Some(n) => Ok(Value::String(lua.create_string(&n)?)), + None => Ok(Value::Nil), + } + })?; // GetID/SetID — the app-meaning-free numeric label (XML `id=`, the client's `+0xb4`): a // dropdown row's list position, a tab index. Default 0 (see `Frame::wow_id`). m.set( @@ -148,19 +147,19 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { .map_or(&["Frame", "Region"][..], |f| type_chain(f.kind))) } - m.set( - "GetObjectType", - lua.create_function(|lua, this: Table| { - Ok(Value::String(lua.create_string(chain_of(lua, &this)?[0])?)) - })?, - )?; + set_shared(lua, m, Side::Frame, "GetObjectType", |lua, this: Table| { + Ok(Value::String(lua.create_string(chain_of(lua, &this)?[0])?)) + })?; // Same four traps as the region twin (see `script::region`): case-insensitive whole-string, // number 1 on a hit and nil on a miss, exactly one value either way, and a non-string // non-number argument raises the reference's own `Usage:` text. - m.set( + set_shared( + lua, + m, + Side::Frame, "IsObjectType", - lua.create_function(|lua, (this, want): (Table, Value)| { + |lua, (this, want): (Table, Value)| { let chain = chain_of(lua, &this)?; let want = match &want { Value::String(s) => s.to_str()?.to_string(), @@ -184,7 +183,7 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { } else { Value::Nil }) - })?, + }, )?; // ── GetFrameType / IsFrameType: the FRAME-side spellings of the pair above (decision 2106) ── @@ -255,24 +254,21 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { })?, )?; - m.set( - "GetParent", - lua.create_function(|lua, this: Table| { - let h = frame_handle_of(lua, &this)?; - let parent_id = { - let mut model = lua.app_data_mut::().expect("model"); - model - .arena - .frame(h) - .and_then(|f| f.parent) - .map(|p| model.frame_id(p)) - }; - match parent_id { - Some(pid) => Ok(Value::Table(frame_wrapper(lua, pid)?)), - None => Ok(Value::Nil), - } - })?, - )?; + set_shared(lua, m, Side::Frame, "GetParent", |lua, this: Table| { + let h = frame_handle_of(lua, &this)?; + let parent_id = { + let mut model = lua.app_data_mut::().expect("model"); + model + .arena + .frame(h) + .and_then(|f| f.parent) + .map(|p| model.frame_id(p)) + }; + match parent_id { + Some(pid) => Ok(Value::Table(frame_wrapper(lua, pid)?)), + None => Ok(Value::Nil), + } + })?; // ── The four structure queries: GetChildren / GetNumChildren / GetRegions / GetNumRegions ── // // Registered bindings in 1.12 (`GetNumRegions 0x773e60`, `GetRegions 0x773f60`, @@ -378,9 +374,12 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { // reference's hide→show round-trip does — `fire_visibility_changes` reads each frame's live // state for direction, so hide and show must be fired from their own phase, never one // combined list. - m.set( + set_shared( + lua, + m, + Side::Frame, "SetParent", - lua.create_function(|lua, args: mlua::MultiValue| { + |lua, args: mlua::MultiValue| { let mut it = args.into_iter(); let this = match it.next() { Some(Value::Table(t)) => t, @@ -472,16 +471,23 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { let shown = { let mut model = lua.app_data_mut::().expect("model"); let shown = model.arena.reparent_finish(h, new_parent, was_visible); - // A real reparent moves the subtree's effective scale and anchors — a - // layout-gate input, and the scale is a measure-key input the subtree's - // FontStrings cannot name one by one (human-rate; the sweep walks once). - model.touch_layout(); + // A real reparent moves the subtree's effective scale — a layout-gate input, + // and a measure-key input the subtree's FontStrings cannot name one by one. + // + // **The layout half NAMES its nodes** (decision 2314). This was the conservative + // touch, on the reading that a reparent is human-rate; a pooled widget's recycle + // is a reparent, and `WOW_LAYOUT_DERIVE_TRACE` put every sampled give-up in a + // quest-accept spike on this one line — 58 whole-graph derivations in one frame, + // 128 ms of a 161 ms hitch. `touch_layout_reparent`'s doc has why the subtree is + // the whole of what moved. The measure sweep keeps the wide form: a measure key + // is not a layout input and its sweep is a different cost. + model.touch_layout_reparent(h); model.touch_measure_all(); shown }; event::fire_visibility_changes(lua, shown); Ok(()) - })?, + }, )?; // Strata / level / scale / alpha m.set( diff --git a/crates/benilla-ui/src/script/object/layout_methods.rs b/crates/benilla-ui/src/script/object/layout_methods.rs index 962c90d6..515bceea 100644 --- a/crates/benilla-ui/src/script/object/layout_methods.rs +++ b/crates/benilla-ui/src/script/object/layout_methods.rs @@ -6,6 +6,7 @@ use mlua::{Lua, MultiValue, Table, Value}; use crate::layout::{Anchor, Point}; +use crate::script::region_map::{set_shared, Side}; use crate::script::{Model, SCREEN}; use crate::widget::FrameHandle; @@ -28,49 +29,52 @@ fn frame_ladder_context(lua: &Lua, h: FrameHandle) -> (String, String) { /// Populate `m`'s layout (anchor/size) methods (see the module doc). pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { // Layout: SetPoint / ClearAllPoints / SetWidth / SetHeight / GetWidth / GetHeight - m.set( + set_shared( + lua, + m, + Side::Frame, "SetPoint", - lua.create_function(|lua, (this, rest): (Table, MultiValue)| set_point(lua, &this, &rest))?, + |lua, (this, rest): (Table, MultiValue)| set_point(lua, &this, &rest), )?; - m.set( - "ClearAllPoints", - lua.create_function(|lua, this: Table| { - let h = frame_handle_of(lua, &this)?; - let mut model = lua.app_data_mut::().expect("model"); - // Every layout setter here follows one law: mutate ONLY on an actual value change, - // and report the change to the tier-1 epoch (`touch_layout`). The compare is what - // 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. - // - // **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(); - Some(old) - } - _ => None, - }; - if let Some(old) = old { - model.touch_layout_retarget_frame(h, &old, &[]); + set_shared(lua, m, Side::Frame, "ClearAllPoints", |lua, this: Table| { + let h = frame_handle_of(lua, &this)?; + let mut model = lua.app_data_mut::().expect("model"); + // Every layout setter here follows one law: mutate ONLY on an actual value change, + // and report the change to the tier-1 epoch (`touch_layout`). The compare is what + // 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. + // + // **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(); + Some(old) } - Ok(()) - })?, - )?; + _ => None, + }; + if let Some(old) = old { + model.touch_layout_retarget_frame(h, &old, &[]); + } + Ok(()) + })?; // GetPoint([n]) → point, relativeTo, relativePoint, xOfs, yOfs — the n-th (1-based, default // first) anchor. relativeTo is nil when the target is the screen root (the client returns // UIParent there; ours is the distinct `script::SCREEN` sentinel, which has no wrapper of its // own — the arena's `UIParent` frame is a different handle — stated, a consensus-list call). - m.set( + set_shared( + lua, + m, + Side::Frame, "GetPoint", - lua.create_function(|lua, (this, n): (Table, Option)| { + |lua, (this, n): (Table, Option)| { let h = frame_handle_of(lua, &this)?; let anchor = { let model = lua.app_data_ref::().expect("model"); @@ -96,28 +100,28 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { Value::Number(f64::from(a.x_off)), Value::Number(f64::from(a.y_off)), )) - })?, + }, )?; // GetNumPoints() → how many anchors this frame carries. On the Region map (`0x87c9b8`), so // every widget answers it — the region twin shipped first and noted this side was missing; // collapsing the map to one implementation each (decision 1501) is what made the gap fatal // rather than merely absent, and this is the arm it was missing. - m.set( - "GetNumPoints", - lua.create_function(|lua, this: Table| { - let h = frame_handle_of(lua, &this)?; - let model = lua.app_data_ref::().expect("model"); - Ok(model - .layout_inputs - .get(&h) - .map_or(0, |i| i.anchors.len() as i64)) - })?, - )?; + set_shared(lua, m, Side::Frame, "GetNumPoints", |lua, this: Table| { + let h = frame_handle_of(lua, &this)?; + let model = lua.app_data_ref::().expect("model"); + Ok(model + .layout_inputs + .get(&h) + .map_or(0, |i| i.anchors.len() as i64)) + })?; // SetAllPoints([relativeTo]) — pin TOPLEFT+BOTTOMRIGHT to the target (default: the parent), // the XML `setAllPoints="true"` behavior as a method (rf24 `0x767800`'s SetAllPoints path). - m.set( + set_shared( + lua, + m, + Side::Frame, "SetAllPoints", - lua.create_function(|lua, (this, rest): (Table, MultiValue)| { + |lua, (this, rest): (Table, MultiValue)| { let h = frame_handle_of(lua, &this)?; // `who`/`$parent` first, then the `_G` read, then the guard — see `set_point` and // `object::NamedTarget`. @@ -145,11 +149,14 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { model.touch_layout(); } Ok(()) - })?, + }, )?; - m.set( + set_shared( + lua, + m, + Side::Frame, "SetWidth", - lua.create_function(|lua, (this, w): (Table, f32)| { + |lua, (this, w): (Table, f32)| { let h = frame_handle_of(lua, &this)?; let mut model = lua.app_data_mut::().expect("model"); let input = model.layout_inputs.entry(h).or_default(); @@ -161,11 +168,14 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { } model.note_authored_size(h); Ok(()) - })?, + }, )?; - m.set( + set_shared( + lua, + m, + Side::Frame, "SetHeight", - lua.create_function(|lua, (this, ht): (Table, f32)| { + |lua, (this, ht): (Table, f32)| { let h = frame_handle_of(lua, &this)?; let mut model = lua.app_data_mut::().expect("model"); let input = model.layout_inputs.entry(h).or_default(); @@ -176,51 +186,42 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { } model.note_authored_size(h); Ok(()) - })?, + }, )?; // **No `SetSize`.** It is an Era geometry verb, in neither the Frame nor the Region method // table of 1.12 — and neither the stock chain nor either addon corpus writes it (decision // 2142's census). The two setters above are the era's whole size surface. - m.set( - "GetWidth", - lua.create_function(|lua, this: Table| { - let h = frame_handle_of(lua, &this)?; - settle(lua); - let model = lua.app_data_ref::().expect("model"); - Ok(size_read(&model, h, true)) - })?, - )?; - m.set( - "GetHeight", - lua.create_function(|lua, this: Table| { - let h = frame_handle_of(lua, &this)?; - settle(lua); - let model = lua.app_data_ref::().expect("model"); - Ok(size_read(&model, h, false)) - })?, - )?; + set_shared(lua, m, Side::Frame, "GetWidth", |lua, this: Table| { + let h = frame_handle_of(lua, &this)?; + settle(lua); + let model = lua.app_data_ref::().expect("model"); + Ok(size_read(&model, h, true)) + })?; + set_shared(lua, m, Side::Frame, "GetHeight", |lua, this: Table| { + let h = frame_handle_of(lua, &this)?; + settle(lua); + let model = lua.app_data_ref::().expect("model"); + Ok(size_read(&model, h, false)) + })?; // GetCenter() → centerX, centerY — the resolved rect's center in LOCAL units (y-up; screen ÷ // the frame's effective scale — the client's convention: coordinate getters report the frame's // own scaled space, and callers divide GetCursorPosition (screen px) by GetEffectiveScale to // meet them there; the ref world map's hover math does exactly that). nil pair before the // first resolve, like the edge readers. - m.set( - "GetCenter", - lua.create_function(|lua, this: Table| { - let h = frame_handle_of(lua, &this)?; - settle(lua); - let model = lua.app_data_ref::().expect("model"); - let inv = 1.0 / eff_scale(&model, h); - Ok(match model.resolved.get(&h) { - Some(r) => ( - Value::Number(f64::from((r.left + r.right) * 0.5 * inv)), - Value::Number(f64::from((r.bottom + r.top) * 0.5 * inv)), - ), - None => (Value::Nil, Value::Nil), - }) - })?, - )?; + set_shared(lua, m, Side::Frame, "GetCenter", |lua, this: Table| { + let h = frame_handle_of(lua, &this)?; + settle(lua); + let model = lua.app_data_ref::().expect("model"); + let inv = 1.0 / eff_scale(&model, h); + Ok(match model.resolved.get(&h) { + Some(r) => ( + Value::Number(f64::from((r.left + r.right) * 0.5 * inv)), + Value::Number(f64::from((r.bottom + r.top) * 0.5 * inv)), + ), + None => (Value::Nil, Value::Nil), + }) + })?; // GetEffectiveScale() — the frame's real effective scale (parentScale · ownScale, the arena's // propagated product). The ROOT factor is 1, and that is not because benilla lacks a `uiScale` @@ -250,23 +251,20 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { ("GetTop", 2u8), ("GetBottom", 3u8), ] { - m.set( - name, - lua.create_function(move |lua, this: Table| { - let h = frame_handle_of(lua, &this)?; - settle(lua); - let model = lua.app_data_mut::().expect("model"); - let inv = 1.0 / eff_scale(&model, h); - Ok(model.resolved.get(&h).map(|r| { - inv * match pick { - 0 => r.left, - 1 => r.right, - 2 => r.top, - _ => r.bottom, - } - })) - })?, - )?; + set_shared(lua, m, Side::Frame, name, move |lua, this: Table| { + let h = frame_handle_of(lua, &this)?; + settle(lua); + let model = lua.app_data_mut::().expect("model"); + let inv = 1.0 / eff_scale(&model, h); + Ok(model.resolved.get(&h).map(|r| { + inv * match pick { + 0 => r.left, + 1 => r.right, + 2 => r.top, + _ => r.bottom, + } + })) + })?; } Ok(()) } diff --git a/crates/benilla-ui/src/script/region.rs b/crates/benilla-ui/src/script/region.rs index 569364d9..3061a8a7 100644 --- a/crates/benilla-ui/src/script/region.rs +++ b/crates/benilla-ui/src/script/region.rs @@ -10,6 +10,7 @@ use super::object::anchor_args::{parse_set_point, resolve_rel_target, UNNAMED}; use super::object::{ anchor_bits_eq, anchor_retarget_is_structural, decode_id, id_to_lud, NamedTarget, }; +use super::region_map::{set_shared, Side}; use super::{ Model, REG_FONTSTRING_META, REG_FONTSTRING_METHODS, REG_REGION_META, REG_REGION_METHODS, REG_TEXTURE_META, REG_TEXTURE_METHODS, REG_TITLE_META, REG_TITLE_METHODS, REG_WRAPPERS, SCREEN, @@ -211,12 +212,17 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; + // **`__index` is the method TABLE, not a dispatcher function** (decision 2310) — here and at + // the three leaf metatables below. A Rust `__index` makes every `t.SetTexture` a Lua→Rust→Lua + // round trip plus a named-registry string lookup; measured at ~195 ns against ~9 ns for the + // table form, paid by every widget method access in the client. Nothing mutates these tables + // after install (`super::region_map::install` writes into them in place, and runs inside + // `super::object::install`), so holding the table itself cannot go stale. let region_meta = lua.create_table()?; - let region_index = lua.create_function(|lua, (_this, key): (Table, Value)| { - let methods: Table = lua.named_registry_value(REG_REGION_METHODS)?; - methods.get::(key) - })?; - region_meta.set("__index", region_index)?; + region_meta.set( + "__index", + lua.named_registry_value::
(REG_REGION_METHODS)?, + )?; lua.set_named_registry_value(REG_REGION_META, region_meta)?; Ok(()) } @@ -236,17 +242,14 @@ fn install_region_methods(lua: &Lua) -> mlua::Result<()> { // // Resolution is [`region_name_of`], shared with `IsObjectType`'s `Usage:` text so the two can // never disagree about what this region is called. - m.set( - "GetName", - lua.create_function(|lua, this: Table| { - let id = decode_id(&this)?; - let model = lua.app_data_ref::().expect("model app_data"); - match region_name_of(&model, id) { - Some(n) => Ok(Value::String(lua.create_string(&n)?)), - None => Ok(Value::Nil), - } - })?, - )?; + set_shared(lua, &m, Side::Region, "GetName", |lua, this: Table| { + let id = decode_id(&this)?; + let model = lua.app_data_ref::().expect("model app_data"); + match region_name_of(&model, id) { + Some(n) => Ok(Value::String(lua.create_string(&n)?)), + None => Ok(Value::Nil), + } + })?; // ── GetObjectType / IsObjectType: the last two of the Region map (1244 §4 closed) ─────────── // @@ -258,13 +261,16 @@ fn install_region_methods(lua: &Lua) -> mlua::Result<()> { // `GetObjectType` is a per-class `.data` `const char*` read through `vtable[+0x1c]` — Texture // `0x773480` → `"Texture"`, FontString `0x7735d0` → `"FontString"` — pushed with // `lua_pushstring`, exactly one value, extra arguments ignored with no arity check. - m.set( + set_shared( + lua, + &m, + Side::Region, "GetObjectType", - lua.create_function(|lua, this: Table| { + |lua, this: Table| { let rh = region_handle_of(lua, &this)?; let model = lua.app_data_ref::().expect("model"); Ok(region_type_name(&model, rh)) - })?, + }, )?; // `IsObjectType(name)` — binding `0x7a1290`. Four traps, all verified, all here: @@ -288,9 +294,12 @@ fn install_region_methods(lua: &Lua) -> mlua::Result<()> { // in place, so `tex:IsObjectType(5)` compares against `"5"` and quietly answers nil — we // format it and compare for real rather than short-circuiting, though no type name is // numeric so the answer is nil either way. - m.set( + set_shared( + lua, + &m, + Side::Region, "IsObjectType", - lua.create_function(|lua, (this, want): (Table, Value)| { + |lua, (this, want): (Table, Value)| { let rh = region_handle_of(lua, &this)?; let model = lua.app_data_ref::().expect("model"); let want = match &want { @@ -308,7 +317,7 @@ fn install_region_methods(lua: &Lua) -> mlua::Result<()> { let leaf = region_type_name(&model, rh); let hit = want.eq_ignore_ascii_case(leaf) || want.eq_ignore_ascii_case("Region"); Ok(if hit { Value::Number(1.0) } else { Value::Nil }) - })?, + }, )?; // SetParent(frame) — **a Texture/FontString really does have this**, and we were the ones @@ -335,9 +344,12 @@ fn install_region_methods(lua: &Lua) -> mlua::Result<()> { // // The mechanism half — full re-link, layer and sub-level preserved, `nil` = orphaned but not // destroyed — is [`crate::widget::WidgetArena::set_region_owner`]'s doc. - m.set( + set_shared( + lua, + &m, + Side::Region, "SetParent", - lua.create_function(|lua, args: mlua::MultiValue| { + |lua, args: mlua::MultiValue| { let mut it = args.into_iter(); let Some(Value::Table(this)) = it.next() else { return Err(mlua::Error::runtime("SetParent: expected a region")); @@ -393,7 +405,7 @@ fn install_region_methods(lua: &Lua) -> mlua::Result<()> { model.touch_layout(); } Ok(()) - })?, + }, )?; // Region-level visibility — the real VisibleRegion Show/Hide on Textures/FontStrings (the @@ -515,20 +527,12 @@ fn install_region_methods(lua: &Lua) -> mlua::Result<()> { (REG_FONTSTRING_META, REG_FONTSTRING_METHODS), ] { let meta = lua.create_table()?; - let index = lua.create_function(move |lua, (_this, key): (Table, Value)| { - let methods: Table = lua.named_registry_value(methods_key)?; - methods.get::(key) - })?; - meta.set("__index", index)?; + meta.set("__index", lua.named_registry_value::
(methods_key)?)?; lua.set_named_registry_value(meta_key, meta)?; } let title_meta = lua.create_table()?; - let title_index = lua.create_function(|lua, (_this, key): (Table, Value)| { - let methods: Table = lua.named_registry_value(REG_TITLE_METHODS)?; - methods.get::(key) - })?; - title_meta.set("__index", title_index)?; + title_meta.set("__index", title.clone())?; lua.set_named_registry_value(REG_TITLE_METHODS, title)?; lua.set_named_registry_value(REG_TITLE_META, title_meta)?; diff --git a/crates/benilla-ui/src/script/region/layout.rs b/crates/benilla-ui/src/script/region/layout.rs index f4e92332..4a9fa15c 100644 --- a/crates/benilla-ui/src/script/region/layout.rs +++ b/crates/benilla-ui/src/script/region/layout.rs @@ -6,6 +6,7 @@ use mlua::{Lua, MultiValue, Table, Value}; use crate::layout::{Anchor, Point}; use crate::script::object::anchor_args::{parse_set_all_points, resolve_rel_target}; use crate::script::object::{anchor_bits_eq, frame_wrapper, point_name}; +use crate::script::region_map::{set_shared, Side}; use crate::script::{Model, SCREEN}; /// Resolve `self` (a region wrapper) to its live [`RegionHandle`]. @@ -18,9 +19,12 @@ use super::{ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { // Region explicit size — fills the axes the region's anchors don't pin (unread under an // implicit SetAllPoints's two corners; decision 1310). - m.set( + set_shared( + lua, + m, + Side::Region, "SetWidth", - lua.create_function(|lua, (this, w): (Table, f32)| { + |lua, (this, w): (Table, f32)| { 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(); @@ -35,12 +39,15 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { model.touch_measure(rh); } Ok(()) - })?, + }, )?; - m.set( + set_shared( + lua, + m, + Side::Region, "SetHeight", - lua.create_function(|lua, (this, h): (Table, f32)| { + |lua, (this, h): (Table, f32)| { 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(); @@ -55,21 +62,19 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { model.touch_measure(rh); } Ok(()) - })?, + }, )?; // **No `SetSize`** — the frame twin's note in `object/layout_methods.rs` applies here // unchanged: an Era verb 1.12's Region map does not carry (decision 2142). - m.set( - "GetWidth", - lua.create_function(|lua, this: Table| Ok(measured_wh(lua, &this)?.0))?, - )?; + set_shared(lua, m, Side::Region, "GetWidth", |lua, this: Table| { + Ok(measured_wh(lua, &this)?.0) + })?; - m.set( - "GetHeight", - lua.create_function(|lua, this: Table| Ok(measured_wh(lua, &this)?.1))?, - )?; + set_shared(lua, m, Side::Region, "GetHeight", |lua, this: Table| { + Ok(measured_wh(lua, &this)?.1) + })?; // GetLeft/GetRight/GetTop/GetBottom — the region's RESOLVED edges in its OWNER's units (y-up; // screen ÷ the owner's effective scale, the frame twin's law in `object/layout_methods.rs` — @@ -84,38 +89,39 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { ("GetTop", 2u8), ("GetBottom", 3u8), ] { - m.set( - name, - lua.create_function(move |lua, this: Table| { - let rh = region_handle_of(lua, &this)?; - let model = lua.app_data_ref::().expect("model"); - let inv = 1.0 / owner_scale(&model, rh); - Ok(model.region_resolved.get(&rh).map(|r| { - inv * match pick { - 0 => r.left, - 1 => r.right, - 2 => r.top, - _ => r.bottom, - } - })) - })?, - )?; + set_shared(lua, m, Side::Region, name, move |lua, this: Table| { + let rh = region_handle_of(lua, &this)?; + let model = lua.app_data_ref::().expect("model"); + let inv = 1.0 / owner_scale(&model, rh); + Ok(model.region_resolved.get(&rh).map(|r| { + inv * match pick { + 0 => r.left, + 1 => r.right, + 2 => r.top, + _ => r.bottom, + } + })) + })?; } // Region anchors: SetPoint/ClearAllPoints/SetAllPoints mirror the frame versions // ([`super::object`]) but write [`super::RegionData::anchors`]. An unspecified `relativeTo` // defaults to the **owner frame**; a named one may be a frame or a sibling region (the real // XML anchors regions to sibling regions everywhere — merchant label plate → `$parentSlot`). - m.set( + set_shared( + lua, + m, + Side::Region, "SetPoint", - lua.create_function(|lua, (this, rest): (Table, MultiValue)| { - region_set_point(lua, &this, &rest) - })?, + |lua, (this, rest): (Table, MultiValue)| region_set_point(lua, &this, &rest), )?; - m.set( + set_shared( + lua, + m, + Side::Region, "ClearAllPoints", - lua.create_function(|lua, this: Table| { + |lua, this: Table| { 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(); @@ -129,12 +135,15 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { model.touch_layout_retarget_region(rh, &old, &[]); } Ok(()) - })?, + }, )?; - m.set( + set_shared( + lua, + m, + Side::Region, "SetAllPoints", - lua.create_function(|lua, (this, rest): (Table, MultiValue)| { + |lua, (this, rest): (Table, MultiValue)| { let rh = region_handle_of(lua, &this)?; // `who`/`$parent` first, then the `_G` read, then the guard — `region_ladder_context`. let (who, base) = region_ladder_context(lua, rh); @@ -160,7 +169,7 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { model.touch_layout(); } Ok(()) - })?, + }, )?; // ── The rest of the Region map `0xcf54b4` (wow-re `font-object-lua-surface.md`) ────────────── @@ -174,17 +183,14 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { // GetParent() → the OWNER frame's wrapper. A region always has one (`region_owner_id` falls // back to the owner for every unresolved case), so unlike the frame twin this never answers nil. - m.set( - "GetParent", - lua.create_function(|lua, this: Table| { - let rh = region_handle_of(lua, &this)?; - let owner = { - let mut model = lua.app_data_mut::().expect("model"); - region_owner_id(&mut model, rh) - }; - frame_wrapper(lua, owner) - })?, - )?; + set_shared(lua, m, Side::Region, "GetParent", |lua, this: Table| { + let rh = region_handle_of(lua, &this)?; + let owner = { + let mut model = lua.app_data_mut::().expect("model"); + region_owner_id(&mut model, rh) + }; + frame_wrapper(lua, owner) + })?; // GetCenter() → the resolved rect's midpoint, or a nil PAIR before the first resolve — the same // contract, and the same source, as the GetLeft/GetRight/GetTop/GetBottom readers above. @@ -193,35 +199,29 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { // readers report raw resolved units; scaling only the centre would make `GetCenter()` disagree // with `(GetLeft() + GetRight()) / 2` on any scaled subtree — a contradiction inside one method // table is worse than a missing division, and regions have no scale of their own to divide by. - m.set( - "GetCenter", - lua.create_function(|lua, this: Table| { - let rh = region_handle_of(lua, &this)?; - let model = lua.app_data_ref::().expect("model"); - let inv = 1.0 / owner_scale(&model, rh); - Ok(match model.region_resolved.get(&rh) { - Some(r) => ( - Value::Number(f64::from(inv * (r.left + r.right) * 0.5)), - Value::Number(f64::from(inv * (r.bottom + r.top) * 0.5)), - ), - None => (Value::Nil, Value::Nil), - }) - })?, - )?; + set_shared(lua, m, Side::Region, "GetCenter", |lua, this: Table| { + let rh = region_handle_of(lua, &this)?; + let model = lua.app_data_ref::().expect("model"); + let inv = 1.0 / owner_scale(&model, rh); + Ok(match model.region_resolved.get(&rh) { + Some(r) => ( + Value::Number(f64::from(inv * (r.left + r.right) * 0.5)), + Value::Number(f64::from(inv * (r.bottom + r.top) * 0.5)), + ), + None => (Value::Nil, Value::Nil), + }) + })?; // GetNumPoints() → how many anchors this region carries. Absent on our FRAMES too, which is the // same drift one table up; this side is what the corpus named. - m.set( - "GetNumPoints", - lua.create_function(|lua, this: Table| { - let rh = region_handle_of(lua, &this)?; - let model = lua.app_data_ref::().expect("model"); - Ok(model - .region_data - .get(&rh) - .map_or(0, |d| d.anchors.len() as i64)) - })?, - )?; + set_shared(lua, m, Side::Region, "GetNumPoints", |lua, this: Table| { + let rh = region_handle_of(lua, &this)?; + let model = lua.app_data_ref::().expect("model"); + Ok(model + .region_data + .get(&rh) + .map_or(0, |d| d.anchors.len() as i64)) + })?; // GetPoint([n]) → point, relativeTo, relativePoint, xOfs, yOfs — the n-th (1-based, default // first) anchor, mirroring the frame twin including its out-of-range answer (five nils). @@ -231,9 +231,12 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { // (`region.rs`'s `resolve_target`: frames first, then the region-name registry). So the id is // matched against both tables and answered with the matching wrapper kind; handing back a frame // wrapper for a region id would be a working-looking handle onto the wrong object. - m.set( + set_shared( + lua, + m, + Side::Region, "GetPoint", - lua.create_function(|lua, (this, n): (Table, Option)| { + |lua, (this, n): (Table, Option)| { let rh = region_handle_of(lua, &this)?; let anchor = { let model = lua.app_data_ref::().expect("model"); @@ -267,7 +270,7 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { Value::Number(f64::from(a.x_off)), Value::Number(f64::from(a.y_off)), )) - })?, + }, )?; Ok(()) } diff --git a/crates/benilla-ui/src/script/region_map.rs b/crates/benilla-ui/src/script/region_map.rs index 7038af59..08bd512a 100644 --- a/crates/benilla-ui/src/script/region_map.rs +++ b/crates/benilla-ui/src/script/region_map.rs @@ -57,7 +57,10 @@ //! opposite reason — in neither table because 1.12 has no such verb at all; decision 2142 removed //! it rather than filing it.) The map is the unit. -use mlua::{Function, Lua, MultiValue, Table, Value}; +use std::collections::HashMap; +use std::rc::Rc; + +use mlua::{FromLuaMulti, IntoLuaMulti, Lua, MultiValue, Table, Value}; use super::object::decode_id; use super::{ @@ -67,7 +70,8 @@ use super::{ /// Which side of the object model a wrapper's `T[0]` id names. Ids come from one counter /// ([`Model::next_id`]), so a region id can never be mistaken for a frame's. -enum Side { +#[derive(Clone, Copy, PartialEq, Eq, Hash)] +pub(super) enum Side { Frame, Region, } @@ -86,6 +90,70 @@ fn side_of(lua: &Lua, this: &Table) -> mlua::Result> { }) } +/// One side's implementation of one of the 19, type-erased to the variadic ABI. +/// +/// **The erasure is the point** (decision 2310). The dispatcher used to hold each side as an +/// `mlua::Function` and reach it with `Function::call`, which re-enters Lua: every argument and +/// every return value round-trips through mlua's ref thread twice, and the whole call is wrapped +/// in a `lua_pcall`. Measured in a release build, a `MultiValue` relay costs **~420 ns** against +/// ~40 ns for a scalar one, and a bridged `GetWidth()` measured **~750 ns against ~99 ns** for an +/// unbridged frame method — a 7x tax on `SetPoint`, `GetWidth`, `ClearAllPoints`, `GetParent`, the +/// verbs every UI calls most. An `Rc` is one ordinary Rust call. +/// +/// The ABI is exactly what `Lua::create_function` builds internally for a typed body — see +/// [`shared`], which is the only thing that makes one — so nothing about argument conversion, +/// arity or coercion changes by going through it. +pub(super) type Arm = Rc mlua::Result>; + +/// The arms the two method-table installers register, collected for [`install`]. +/// +/// It lives in the VM's `app_data` rather than being threaded through four `install` signatures: +/// the installers are deep (`object::install` → `layout_methods::install`, `region::install` → +/// `region::layout::install`) and the collection is install-time scaffolding, removed by +/// [`install`] the moment it has been consumed. +#[derive(Default)] +pub(super) struct Arms(HashMap<(Side, &'static str), Arm>); + +/// Register one of the [`REGION_MAP_METHODS`] into its side's method table **and** record its arm. +/// +/// Every one of the 38 (19 names x 2 sides) goes through here instead of +/// `m.set(name, lua.create_function(body)?)?`, and [`install`] fails loudly if one did not — so a +/// name added to the map without both arms cannot reach an addon as a half-registered method. +/// The table entry it writes is transient: [`install`] replaces all 19 in every table with the +/// shared dispatcher. It exists so the pre-bridge tables are still complete for anything that +/// reads them in between (the leaf tables are *copied* out of the region table, mid-install). +pub(super) fn set_shared( + lua: &Lua, + m: &Table, + side: Side, + name: &'static str, + f: F, +) -> mlua::Result<()> +where + A: FromLuaMulti + 'static, + R: IntoLuaMulti + 'static, + F: Fn(&Lua, A) -> mlua::Result + 'static, +{ + let arm: Arm = + Rc::new(move |lua, args| f(lua, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua)); + let entry = arm.clone(); + m.set( + name, + lua.create_function(move |lua, args: MultiValue| entry(lua, args))?, + )?; + lua.app_data_mut::() + .expect("Region-map arms — installed by `super::object::install`") + .0 + .insert((side, name), arm); + Ok(()) +} + +/// Open the arm collection. Called once, at the top of [`super::object::install`], before either +/// method table is built; [`install`] closes it. +pub(super) fn open_arms(lua: &Lua) { + lua.set_app_data(Arms::default()); +} + /// Replace each of the 19 Region-map entries in every table the chain reaches with one shared /// function that dispatches on the receiver. /// @@ -103,31 +171,45 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { .map(|k| lua.named_registry_value::
(k)) .collect::>()?; + let arms = lua + .remove_app_data::() + .expect("Region-map arms — opened by `open_arms`"); + for name in REGION_MAP_METHODS { - // Both arms must already exist. A missing one is not a thing to paper over with a - // one-sided shared function: the reference gives every widget all 19, so a gap on either - // side is a hole in the surface and says so here rather than at some addon's call site. - // (`GetNumPoints` was exactly that hole on the frame side until this module went in.) - let on_frame: Function = frame.get(name).map_err(|_| { - mlua::Error::runtime(format!("Region map: the FRAME table has no {name}")) - })?; - let on_region: Function = region.get(name).map_err(|_| { - mlua::Error::runtime(format!("Region map: the REGION table has no {name}")) - })?; + // Both arms must exist. A missing one is not a thing to paper over with a one-sided shared + // function: the reference gives every widget all 19, so a gap on either side is a hole in + // the surface and says so here rather than at some addon's call site. (`GetNumPoints` was + // exactly that hole on the frame side until this module went in.) + let arm = |side: Side, which: &str| -> mlua::Result { + arms.0.get(&(side, name)).cloned().ok_or_else(|| { + mlua::Error::runtime(format!( + "Region map: the {which} side never registered {name} through `set_shared`" + )) + }) + }; + let on_frame = arm(Side::Frame, "FRAME")?; + let on_region = arm(Side::Region, "REGION")?; let shared = lua.create_function(move |lua, args: MultiValue| { // The receiver is argument 1 on every one of the 19 — a method call always passes it, - // and a pulled-off `Api._Height(x)` call passes it as the only argument. - let Some(Value::Table(this)) = args.iter().next().cloned() else { - return Err(mlua::Error::runtime( - "expected a frame or region as the first argument", - )); + // and a pulled-off `Api._Height(x)` call passes it as the only argument. Borrowed + // rather than cloned: an mlua `Value::Table` clone registers a second reference in the + // ref thread, and this is the entry point for the verbs a map-note redraw calls + // thousands of times. + let side = { + let Some(Value::Table(this)) = args.iter().next() else { + return Err(mlua::Error::runtime( + "expected a frame or region as the first argument", + )); + }; + side_of(lua, this)? }; - match side_of(lua, &this)? { - Some(Side::Frame) => on_frame.call::(args), - Some(Side::Region) => on_region.call::(args), + match side { + // One ordinary Rust call, not a re-entry into Lua — see [`Arm`]. + Some(Side::Frame) => on_frame(lua, args), + Some(Side::Region) => on_region(lua, args), // A wrapper whose widget is gone. Deliberately ONE message for both sides — at // this point the receiver's kind is exactly what could not be established, and - // each side's old wording claimed it. + // each side's own wording claimed it. None => Err(mlua::Error::runtime("stale or invalid widget handle")), } })?; diff --git a/crates/benilla-ui/src/script/tests/dispatch_bench.rs b/crates/benilla-ui/src/script/tests/dispatch_bench.rs new file mode 100644 index 00000000..31ecdbf2 --- /dev/null +++ b/crates/benilla-ui/src/script/tests/dispatch_bench.rs @@ -0,0 +1,195 @@ +//! **What one widget method lookup costs** — the bench behind the `__index` table (decision 2310). +//! +//! Every `frame:SetPoint(...)` in FrameXML and in every addon begins with a table index on the +//! wrapper, which misses and falls to the metatable's `__index`. That path is on the hottest edge +//! this engine has: Questie redraws its map notes with ~19 widget calls per cluster over hundreds +//! of clusters, and a live sample of a quest-accept spike put **20 % of the whole UI tick** inside +//! the metamethod dispatch alone — before any method body ran. +//! +//! The two rows below are the measurement that claim rests on, and the regression guard for it: +//! +//! - `miss` — `local m = f.NoSuchMethod`, the duck-type probe addons use (`if f.SetValue then`). +//! It is the WORST case: a dispatcher walks every registry in the kind chain and then the shared +//! table before answering nil. +//! - `hit` — `local m = f.GetWidth`, the lookup every real call pays. +//! +//! Run: `cargo test --release -p benilla-ui --lib -- --ignored --nocapture dispatch_bench`. + +use std::time::Instant; + +use super::common::script; + +/// Lookups per timed loop — big enough that the loop's own overhead is a rounding error at the +/// nanosecond scale these rows live at. +const N: usize = 400_000; + +/// Timed repeats per row; the reported number is the fastest (see [`dispatch_bench`]'s `floor`). +const ROUNDS: usize = 5; + +fn ns_per(t: Instant, n: usize) -> f64 { + t.elapsed().as_secs_f64() * 1e9 / n as f64 +} + +#[test] +#[ignore = "bench — run explicitly with --release --nocapture"] +fn dispatch_bench() { + let s = script(); + s.run( + r#" + F = CreateFrame("Frame") + B = CreateFrame("Button") + T = F:CreateTexture() + S = F:CreateFontString() + "#, + ) + .expect("fixtures"); + + // A plain Lua table with a table `__index` — the floor this path could reach, and what the + // reference client's flat `{name, lua_CFunction}` probe costs in C. + // **The MINIMUM of [`ROUNDS`] runs, never the mean.** This machine builds while it measures — + // a neighbouring worktree's `cargo` saturates ten cores — and a mean of a contended loop reads + // whatever else was running. The minimum is the closest thing to the uncontended cost that a + // shared machine can report, and it is what makes two rows of one run comparable at all. + let floor = |what: &str, chunk: &str| { + let mut best = f64::INFINITY; + for _ in 0..ROUNDS { + let t = Instant::now(); + s.run(chunk).expect("bench chunk"); + best = best.min(ns_per(t, N)); + } + println!("[dispatch] {what:>26} {best:7.1} ns"); + }; + + floor( + "baseline (table meta)", + &format!( + "local base = {{ GetWidth = function() end }} + local o = setmetatable({{}}, {{ __index = base }}) + for _ = 1, {N} do local m = o.GetWidth end" + ), + ); + floor( + "Frame hit", + &format!("for _ = 1, {N} do local m = F.GetWidth end"), + ); + floor( + "Frame miss", + &format!("for _ = 1, {N} do local m = F.NoSuchMethod end"), + ); + floor( + "Button hit", + &format!("for _ = 1, {N} do local m = B.GetWidth end"), + ); + floor( + "Button own hit", + &format!("for _ = 1, {N} do local m = B.SetText end"), + ); + floor( + "Button miss", + &format!("for _ = 1, {N} do local m = B.NoSuchMethod end"), + ); + floor( + "Texture hit", + &format!("for _ = 1, {N} do local m = T.GetWidth end"), + ); + floor( + "Texture miss", + &format!("for _ = 1, {N} do local m = T.NoSuchMethod end"), + ); + floor( + "FontString hit", + &format!("for _ = 1, {N} do local m = S.GetWidth end"), + ); + + // The whole call, not just the lookup — what an addon's `f:GetWidth()` actually costs, split + // so the two taxes are separable: `GetFrameLevel` is a frame-only binding and measures one + // Rust hop, while `GetWidth` is one of [`crate::script::region_map`]'s 19 shared names and + // pays a SECOND hop — the bridge re-enters Lua to call the per-side arm. + floor( + "GetFrameLevel() call", + &format!("for _ = 1, {N} do local l = F:GetFrameLevel() end"), + ); + floor( + "GetWidth() call (bridged)", + &format!("for _ = 1, {N} do local w = F:GetWidth() end"), + ); + floor( + "SetPoint() call (bridged)", + &format!("for _ = 1, {N} do F:SetPoint(\"CENTER\", 0, 0) end"), + ); + + // **The settle tax.** Every geometry GETTER (`GetWidth`, `GetHeight`, `GetLeft`, `GetPoint`, + // `GetCenter`) calls `layout_methods::settle`, i.e. a whole `resolve_layout`. The tier-1 epoch + // gate makes that free while nothing has moved — which is what the `GetWidth() call` row above + // measures — but ANY layout write in between reopens it, so the read-after-write idiom pays a + // resolve per read. That idiom is not exotic: Astrolabe repositions a minimap icon with + // `GetWidth` → `ClearAllPoints` → `SetPoint`, per icon, and Questie places dozens. + // + // Here the model holds four frames, so a resolve is nearly free and the gap is the mechanism + // alone. In a live client the roster is thousands of frames and ten thousand anchored regions + // — the same gap, three orders of magnitude wider. + floor( + "write only", + &format!("for i = 1, {N} do F:SetPoint(\"CENTER\", i, 0) end"), + ); + floor( + "write + read (settles)", + &format!("for i = 1, {N} do F:SetPoint(\"CENTER\", i, 0) local w = F:GetWidth() end"), + ); + floor( + "write + read x3 (settles)", + &format!( + "for i = 1, {N} do F:SetPoint(\"CENTER\", i, 0) local w = F:GetWidth() local h = F:GetHeight() local l = F:GetLeft() end" + ), + ); + + // **The control that says WHERE the bridged calls' extra microsecond goes.** `__direct` is one + // mlua closure; `__wrapped` is one mlua closure that calls another through `Function::call` — + // the exact shape `region_map`'s shared arm has. The gap between these two rows is what a + // re-entrant Lua call costs, and it is the number that decides whether those 19 names are worth + // restructuring into a single Rust body with an internal branch (the reference's own shape: one + // C function doing a virtual dispatch on the receiver). + { + let lua = s.lua(); + let direct = lua + .create_function(|_, n: i64| Ok(n + 1)) + .expect("direct fn"); + lua.globals().set("__direct", direct.clone()).expect("set"); + let wrapped = lua + .create_function(move |_, n: i64| direct.call::(n)) + .expect("wrapped fn"); + lua.globals().set("__wrapped", wrapped).expect("set"); + + // The same pair again in the SHAPE the bridge actually has: a variadic `MultiValue` whose + // first value is the receiver table. `Table` and `MultiValue` are the two conversions the + // scalar controls above leave out, and between them they are most of the gap. + let mv_direct = lua + .create_function(|_, args: mlua::MultiValue| Ok(args.len() as i64)) + .expect("mv direct"); + lua.globals() + .set("__mv_direct", mv_direct.clone()) + .expect("set"); + let mv_relay = lua + .create_function(move |_, args: mlua::MultiValue| { + mv_direct.call::(args) + }) + .expect("mv relay"); + lua.globals().set("__mv_relay", mv_relay).expect("set"); + } + floor( + "ctrl: 1 mlua hop", + &format!("for _ = 1, {N} do local v = __direct(1) end"), + ); + floor( + "ctrl: mlua hop + relay", + &format!("for _ = 1, {N} do local v = __wrapped(1) end"), + ); + floor( + "ctrl: variadic hop", + &format!("for _ = 1, {N} do local v = __mv_direct(F, 1, 2) end"), + ); + floor( + "ctrl: variadic hop + relay", + &format!("for _ = 1, {N} do local v = __mv_relay(F, 1, 2) end"), + ); +} diff --git a/crates/benilla-ui/src/script/tests/mod.rs b/crates/benilla-ui/src/script/tests/mod.rs index 2c597e4f..57035f32 100644 --- a/crates/benilla-ui/src/script/tests/mod.rs +++ b/crates/benilla-ui/src/script/tests/mod.rs @@ -13,6 +13,7 @@ mod channel; mod common; mod cooldown; mod create_frame_template; +mod dispatch_bench; mod end_to_end; mod events; mod font_object; diff --git a/crates/benilla-ui/src/script/tests/setparent.rs b/crates/benilla-ui/src/script/tests/setparent.rs index 15802774..29b54489 100644 --- a/crates/benilla-ui/src/script/tests/setparent.rs +++ b/crates/benilla-ui/src/script/tests/setparent.rs @@ -203,3 +203,64 @@ fn the_binding_raises_on_cycle_bad_name_and_absent_argument() { "0x76ab20 skips everything — the level is not re-derived" ); } + +/// **A reparent under a scale change still moves the child's resolved rect** — the falsifier for +/// decision 2314's claim that `SetParent` is a value-only write the ledger can NAME. +/// +/// The gate's two tiers police each other under `WOW_LAYOUT_VERIFY`, which is forced on for this +/// crate's tests: an incremental pass that names its nodes is re-run from scratch and the rects +/// must be identical. So this test does not have to assert the epoch bookkeeping — it only has to +/// make the reparent *observable in geometry*, which is what the conservative touch used to buy +/// wholesale. A child anchored inside a parent inherits `parentScale · ownScale` as its +/// `LayoutInput.scale`; move it to a parent with a different scale and both its offsets and its +/// span change. +#[test] +fn a_reparent_under_a_scale_change_moves_the_childs_rect() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + one = CreateFrame("Frame", "ScaleOne") + one:SetPoint("BOTTOMLEFT", 0, 0); one:SetWidth(800); one:SetHeight(600) + two = CreateFrame("Frame", "ScaleTwo") + two:SetPoint("BOTTOMLEFT", 0, 0); two:SetWidth(800); two:SetHeight(600) + two:SetScale(2) + kid = CreateFrame("Frame", "ScaleKid", one) + kid:SetPoint("BOTTOMLEFT", one, "BOTTOMLEFT", 10, 20); kid:SetWidth(100); kid:SetHeight(50) + "#, + ) + .unwrap(); + s.resolve(); + let at_one: (f32, f32, f32) = s + .eval("return ScaleKid:GetLeft(), ScaleKid:GetBottom(), ScaleKid:GetWidth()") + .unwrap(); + + // The whole point: the write between the two reads is the reparent, and the read after it must + // see it. Scale 2 doubles the offsets and the span in screen units — and `GetWidth` reports in + // the frame's OWN scaled space, so the width reads back unchanged while the edges move. + s.run("kid:SetParent(two)").unwrap(); + s.resolve(); + let at_two: (f32, f32, f32) = s + .eval("return ScaleKid:GetLeft(), ScaleKid:GetBottom(), ScaleKid:GetWidth()") + .unwrap(); + + assert_eq!(at_one, (10.0, 20.0, 100.0), "seated in the unscaled parent"); + assert_eq!( + at_two, + (10.0, 20.0, 100.0), + "own-space coordinates are scale-relative, so these do not move" + ); + // …but the SCREEN rect does. Read it off the extraction, which is in screen units. + let widths: Vec = s + .extract() + .iter() + .filter_map(|q| match q.target { + crate::order::ZTarget::Frame(_) => q.rect.map(|r| r.right - r.left), + _ => None, + }) + .collect(); + assert!( + widths.iter().any(|w| (w - 200.0).abs() < 1e-3), + "the reparented child should be 100 x scale 2 = 200 screen units wide; got {widths:?}" + ); +} diff --git a/crates/benilla-ui/src/toc.rs b/crates/benilla-ui/src/toc.rs index 89623fdb..e8ed8f71 100644 --- a/crates/benilla-ui/src/toc.rs +++ b/crates/benilla-ui/src/toc.rs @@ -181,10 +181,18 @@ impl Toc { /// falls back to this byte whenever the characters disagree, and whenever *none* of them has /// an opinion at all — which is every addon on a fresh install. /// - /// **A manifest that does not write the line is enabled**, which is what makes a folder - /// dropped into `AddOns/` just work. That is the two literals' fall-through, not a guess at - /// the record's initial byte — a value that is neither `enabled` nor `disabled` reads the - /// same way, because neither compare matches it and the field keeps whatever it had. + /// **A manifest that does not write the line is enabled** — which is what makes a folder + /// dropped into `AddOns/` just work, and is the record's initial byte, read at the bytes and + /// not inferred from the two literals' fall-through (wow-re + /// `addon-defaultstate-and-node-set.md`, decision 2316). The ctor `0x520550` seeds this one + /// field to 1 explicitly — `0x5205b9 mov [esi+0x2b],al` with `eax = 1`, where its five + /// neighbours take `bl = 0` — over an allocation that does zero-fill, which is exactly what + /// made "the ctor zeroes it" read as true. + /// + /// Two consequences this function depends on, both verified there: a value matching **neither** + /// literal leaves the 1 (`0x51d21a jne`, no store — there is no "unrecognised means + /// disabled"), and a duplicated directive is **last-wins**, which is [`Self::directive`]'s own + /// rule. pub fn default_state(&self) -> bool { !self .directive("DefaultState") @@ -224,6 +232,37 @@ mod tests { assert_eq!(toc.directives.len(), 4); } + /// **`## DefaultState:` and its three non-obvious answers**, all byte-verified (wow-re + /// `addon-defaultstate-and-node-set.md`): an absent line is *enabled* because the record's + /// ctor seeds `[rec+0x2b]` to 1, a value matching neither literal leaves that 1 rather than + /// meaning disabled, and a duplicated line is last-wins. + /// + /// Only `disabled` disables, in other words — which is the same asymmetry `AddOns.txt`'s own + /// value grammar has, and worth a falsifier because "unrecognised means off" is the reading + /// anyone would write by hand. + #[test] + fn default_state_is_enabled_unless_the_manifest_says_disabled() { + let d = |body: &str| Toc::parse(body).default_state(); + assert!(d("## Interface: 11200\n"), "absent: the ctor's own 1"); + assert!(d("## DefaultState: enabled\n")); + assert!(!d("## DefaultState: disabled\n")); + assert!( + !d("## DefaultState: DISABLED \n"), + "trimmed, case-folded" + ); + assert!( + d("## DefaultState: maybe\n"), + "neither literal: no store, the 1 stands" + ); + assert!( + d("## DefaultState:\n"), + "empty value is not `disabled` either" + ); + // Last-wins, the reference's hash insert — not first-wins, and not an append. + assert!(!d("## DefaultState: enabled\n## DefaultState: disabled\n")); + assert!(d("## DefaultState: disabled\n## DefaultState: enabled\n")); + } + #[test] fn vanilla_1_12_shape() { let toc = Toc::parse(