script errors reach the player, and the bars stop lying

- an addon error shows the red script-error dialog the reference shows, and a runaway load loop can no longer freeze world entry
- food and drink show stack counts from the first login, and a paladin's aura bar lights the aura that is actually on
- logging out no longer erases the character from saved-bag addons: the player's absence is no data
- MPQ patches are discovered the way the client discovers them, and a slow arrival holds in place instead of falling through the world

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Sam
2026-08-14 19:32:44 +07:00
co-authored by Claude Fable 5
parent 047dd3444e
commit e665b0bfc4
39 changed files with 1385 additions and 215 deletions
+12 -16
View File
@@ -46,22 +46,18 @@
_ERRORMESSAGE(text)
end
-- ── AND ONE LINE OF THE REFERENCE DELIBERATELY NOT TRANSCRIBED ─────────────────────────
-- The reference ends this block with `seterrorhandler(_ERRORMESSAGE);`. We do not.
--
-- benilla's default handler is `__benilla_script_error` (decision 1195), which pushes into
-- `UiScript::errors()` — the channel the corpus harness's whole blocker table is built on,
-- and the only place a Lua fault is visible to any instrument we own. Installing
-- `_ERRORMESSAGE` would route every runtime error into a modal dialog instead and blind
-- `blockers`, `demand` and every A/B that depends on them, to buy a dialog nobody has asked
-- for yet.
--
-- The addon-facing behaviour is unchanged either way: `geterrorhandler()` still answers a
-- callable (the 1195 pair), `geterrorhandler()(msg)` still reports, and an addon that
-- installs its OWN handler with `seterrorhandler` still wins outright. What differs is only
-- which handler is installed when nobody has chosen — and that is ours to choose while the
-- error channel is load-bearing for the arc's instruments. Revisit when a player-facing
-- Lua-error surface exists; the one line is the whole change.
-- ── seterrorhandler(_ERRORMESSAGE): the reference's closing line, now transcribed ──────
-- This file withheld exactly this line from when it landed until decision 1305, because
-- the engine's error channel (`UiScript::errors()`) was the only place a Lua fault was
-- visible to any instrument and a modal would have blinded it. That trade is dissolved,
-- not decided differently: the engine now records every caught script error on the host
-- channel FIRST and then dispatches it to whatever `geterrorhandler()` answers
-- (`dispatch_script_errors_to_handler`, 1305) — so the harness's blocker tables read
-- exactly what they always read, and the player gets what the reference always gave them:
-- an addon that errors puts the red ScriptErrors dialog on screen instead of failing
-- somewhere only a terminal can see (B271's report was debugged entirely off WARN lines).
-- An addon that installs its OWN handler still wins outright, exactly as in 1195.
seterrorhandler(_ERRORMESSAGE)
-- ── TEXT (ref l.22-26, verbatim) ──────────────────────────────────────────────────────
-- "This function is used to translate text (actually it's already translated as-is)" is
+6 -3
View File
@@ -394,9 +394,11 @@ fn manifest_path(root: &Path, name: &str) -> Option<PathBuf> {
.map(|e| e.path())
}
/// The per-addon VM-instruction bound (see its use in [`survey_one`]). Measured: the heaviest
/// legitimate corpus addon runs 4M, and 214 of 218 run under 1M.
const ADDON_INSTRUCTION_BUDGET: u64 = 200_000_000;
/// The per-addon VM-instruction bound (see its use in [`survey_one`]) — the ONE number, shared
/// with the live client's world-entry arming (decision 1306) so the corpus measurement behind it
/// (heaviest legitimate addon 4M, 214 of 218 under 1M) cannot drift apart from the bound players
/// actually run under.
const ADDON_INSTRUCTION_BUDGET: u64 = crate::ui_script::addons::LOAD_INSTRUCTION_BUDGET;
fn survey_one(
root: &Path,
@@ -1401,6 +1403,7 @@ fn seat_a_session(script: &mut UiScript) {
kind: 0x00,
action: 100,
count: 0,
consumable: false,
}),
);
script.set_action_state(
@@ -215,6 +215,7 @@ pub(super) fn measure_render(
kind: 0,
action: 403,
count: 0,
consumable: false,
}),
);
script.set_action_state(
+10 -8
View File
@@ -118,17 +118,19 @@ pub(super) fn swim(feet_y: f32, surface_y: f32, swimming: bool, h: f32) {
///
/// `resident` means the destination's world arrived (scene spawned + collider queue quiet —
/// decision 0737's release) and the hold released onto it; `!resident` is the
/// [`super::SETTLE_TIMEOUT`] backstop firing, which switches gravity on with the world never
/// having become resident. The distinction is invisible from inside the game (both look like "the
/// loading screen went away") and it is the difference between a world that streamed in time and
/// one that did not, so the timeout end is also a `warn!` on the ordinary log — a reporter's paste
/// can then name it without owning a trace.
/// [`super::SETTLE_TIMEOUT`] backstop firing, which since decision 1303 (B263) means the stream
/// made **no progress at all** for the whole budget — a genuinely dead stream, never a slow one —
/// and gravity comes on with the world never having become resident. `waited` is measured from
/// the snap either way. The distinction is invisible from inside the game (both look like "the
/// loading screen went away") and it is the difference between a world that arrived and one that
/// died, so the timeout end is also a `warn!` on the ordinary log — a reporter's paste can then
/// name it without owning a trace.
pub(super) fn settle(resident: bool, waited: f32, pos: bevy::prelude::Vec3) {
if !resident {
bevy::log::warn!(
"settle: TIMED OUT after {waited:.2}s with the world never resident at \
({:.1},{:.1},{:.1}) — releasing anyway. If a building stands here, its collider \
had not finished streaming and the body is about to fall through it.",
"settle: TIMED OUT {waited:.2}s after the snap with the stream stalled and the world \
never resident at ({:.1},{:.1},{:.1}) — releasing anyway. If a building stands \
here, its collider never streamed and the body is about to fall through it.",
pos.x,
pos.y,
pos.z,
+17 -7
View File
@@ -252,11 +252,14 @@ pub(super) const FALL_FAR_TIME: f32 = 0.5;
/// Skin width (yd) kept between the capsule and geometry on casts.
pub(super) const SKIN_WIDTH: f32 = 0.02;
/// Max seconds to hold the avatar after a teleport while the world streams in (see [`Player::settling`]).
/// Generous — a dense city's spawn + collider queue drains in a couple seconds; this only backstops a
/// world that never becomes resident (missing data, a stalled stream) so we never hang forever. The
/// release itself is the terrain streamer's (decision 0737), which also pushes the deadline while the
/// resident world is still the departed map's (0710's fail-closed law).
/// Seconds of **stalled** streaming before the post-snap hold gives up (see [`Player::settling`]).
/// A *stall* budget, not a load budget: the deadline is pushed forward while the resident world is
/// still the departed map's (0710's fail-closed law) **and while the destination is visibly still
/// arriving** (any load-progress counter moved — B263, decision 1303). It can therefore only fire
/// against a stream that has made no progress at all for this long — missing data, dead IO — never
/// against a slow machine. As a fixed load budget it was measured 0.01 s from firing on a *fast*
/// machine (a Stormwind arrival consumed 5.99 s of the 6.00 s), which is B263: on any slower
/// machine it released gravity mid-stream and the body fell through the not-yet-collided city.
pub(crate) const SETTLE_TIMEOUT: f32 = 6.0;
/// The player body's collision capsule, built once at startup and swept by avian's `MoveAndSlide`
@@ -580,8 +583,15 @@ pub(crate) struct Player {
/// its own leak patch before 0737 deleted the class).
pub(crate) settling: bool,
/// `Time::elapsed_secs` deadline to give up settling and release (see [`Player::settling`]).
/// Pushed forward by the streamer while [`Player::world_stale`] (0710's fail-closed law).
/// Pushed forward by the streamer while [`Player::world_stale`] (0710's fail-closed law) and
/// while any load-progress counter is still moving (B263, decision 1303) — so it measures
/// *stall*, and a slow arrival can never exhaust it.
pub(crate) settle_deadline: f32,
/// `Time::elapsed_secs` when the current settle hold began (the snap) — what the `sett` trace
/// reports the wait against. The deadline above stopped measuring this the moment it became a
/// stall budget: it is re-pushed all through a live stream, so "deadline minus timeout" names
/// the last push, not the arrival.
pub(crate) settle_since: f32,
/// **The colliders under our feet may still belong to the map we just left.** Set at every
/// snap, cleared by the terrain streamer once the destination's own world is resident.
///
@@ -753,7 +763,7 @@ impl Player {
/// the whole diagnosis of a fall-through report, so it goes through the `sett` trace either way.
pub(crate) fn end_settle(&mut self, resident: bool, now: f32) {
self.settling = false;
let waited = SETTLE_TIMEOUT - (self.settle_deadline - now);
let waited = now - self.settle_since;
super::move_trace::settle(resident, waited, self.pos);
}
+3
View File
@@ -254,6 +254,7 @@ pub(super) fn apply_server_moves(
player.model_yaw = w.orientation; // a teleport snaps the body — no chase across a loading screen
cam.yaw = w.orientation;
player.settling = true; // hold (gravity off) until the new map's ground streams in
player.settle_since = time.elapsed_secs();
player.settle_deadline = time.elapsed_secs() + SETTLE_TIMEOUT;
// …and the physics world under the new position is still the OLD map's until the
// streamer swaps it, one stage from now. See [`Player::world_stale`].
@@ -293,6 +294,7 @@ pub(super) fn apply_server_moves(
player.move_flags = 0;
player.airborne_since = None; // a snap ends any in-progress jump arc (no phantom FALL_LAND)
player.settling = true; // hold (gravity off) until the destination's ground/buildings load
player.settle_since = time.elapsed_secs();
player.settle_deadline = time.elapsed_secs() + SETTLE_TIMEOUT;
// A far same-map teleport relocates us over ground that has not streamed either — the
// tiles we were standing on are still the resident ones. See [`Player::world_stale`].
@@ -409,6 +411,7 @@ pub(super) fn apply_server_moves(
player.active = true;
if first {
player.settling = true; // settle onto the initial ground once it loads (don't fall through)
player.settle_since = time.elapsed_secs();
player.settle_deadline = time.elapsed_secs() + SETTLE_TIMEOUT;
player.world_stale = true; // nothing is streamed yet — see [`Player::world_stale`]
// The streamed spawn pose carries the server's facing (the character's logout
+152 -5
View File
@@ -105,6 +105,7 @@ pub(super) fn release_post_snap_hold(
mut player: ResMut<Player>,
progress: Option<Res<WorldLoadProgress>>,
time: Res<Time>,
mut last_counters: Local<Option<[usize; 4]>>,
) {
let Some(p) = progress else { return };
// The streamer is the only authority on *which map* the colliders under the avatar belong to.
@@ -113,19 +114,165 @@ pub(super) fn release_post_snap_hold(
if p.focus_resident && p.total > 0 {
player.world_stale = false;
}
// Did the stream move since last frame? Any counter changing — a tile spawned, a placement
// up, a collider queued or attached — is the destination still arriving. Tracked every frame
// (not just while settling) so the first settling frame compares against a real baseline.
let counters = [p.ready, p.total, p.colliders_pending, p.placements_pending];
let progressed = last_counters.replace(counters) != Some(counters);
if !player.settling {
return;
}
// The release ends on scene AND colliders, never on ground contact — feet-on-ground dragged
// the whole mover-mode matrix into a loading decision, and a flyer or swimmer never touched
// it. While the resident colliders still belong to the map we just left the deadline is
// pushed (0710's fail-closed law: a world that never arrives keeps the hold and the screen).
// it. The timeout is a STALL budget, twice over (0710's fail-closed law, extended by B263 /
// decision 1303): the deadline is pushed while the resident colliders still belong to the map
// we just left, AND while the destination's own stream is visibly advancing. As a fixed load
// budget it was 0.01 s from firing on a fast machine (a Stormwind arrival used 5.99 s of the
// 6.00), and on a slower one it fired mid-stream — gravity on, the city's collider still in
// the build queue, and the body fell to the canyon under the Valley of Heroes with the cover
// still up. Only a stream that has made NO progress for the whole budget — missing data, dead
// IO — can time out now, which is the case the backstop was always for.
let now = time.elapsed_secs();
if player.world_stale {
player.settle_deadline = now + SETTLE_TIMEOUT;
} else if p.scene_ready && p.colliders_pending == 0 {
if p.scene_ready && p.colliders_pending == 0 && !player.world_stale {
player.end_settle(true, now);
} else if player.world_stale || progressed {
player.settle_deadline = now + SETTLE_TIMEOUT;
} else if now >= player.settle_deadline {
player.end_settle(false, now);
}
}
#[cfg(test)]
mod tests {
use std::time::Duration;
use super::*;
/// A test app with the release system registered (a registered system keeps its `Local`
/// baseline across frames, which `run_system_once` would reset) and a hand-driven clock.
fn app() -> App {
let mut app = App::new();
app.insert_resource(Time::<()>::default())
.insert_resource(WorldLoadProgress::default())
.insert_resource(Player {
settling: true,
settle_deadline: SETTLE_TIMEOUT,
..Player::default()
})
.add_systems(Update, release_post_snap_hold);
app
}
/// One frame: advance the clock, mutate the streamer's published progress, run the release.
fn step(app: &mut App, dt: f32, tweak: impl FnOnce(&mut WorldLoadProgress)) {
app.world_mut()
.resource_mut::<Time>()
.advance_by(Duration::from_secs_f32(dt));
tweak(&mut app.world_mut().resource_mut::<WorldLoadProgress>());
app.update();
}
fn settling(app: &mut App) -> bool {
app.world().resource::<Player>().settling
}
/// B263 (decision 1303): a stream that keeps arriving keeps the hold, however long it takes.
/// The old fixed load budget released gravity at 6 s into a live Stormwind arrival — measured
/// 0.01 s from firing even on a fast machine — and the body fell through the not-yet-collided
/// city to the canyon under the Valley of Heroes, impact heard under the loading screen.
#[test]
fn a_slow_but_advancing_stream_never_times_out() {
let mut app = app();
app.world_mut().resource_mut::<Player>().world_stale = false;
// 5× the budget of wall-clock, with some counter moving every frame — a slow machine
// streaming a big city, five times slower than the budget ever allowed for.
for i in 0..(5.0 * SETTLE_TIMEOUT) as usize {
step(&mut app, 1.0, |p| {
p.total = 2000;
p.ready = i; // tiles/placements landing one at a time
p.colliders_pending = 300 + i % 7;
p.scene_ready = false;
});
assert!(
settling(&mut app),
"the hold gave up at ~{i}s with the stream still visibly advancing"
);
}
}
/// The backstop's one remaining target: a stream that makes NO progress for the whole budget
/// (missing data, dead IO) still releases, so a broken world can never hold the screen forever.
#[test]
fn a_genuinely_stalled_stream_still_times_out() {
let mut app = app();
app.world_mut().resource_mut::<Player>().world_stale = false;
// Frozen counters, scene never presentable. First frame baselines the Local (counts as
// change), then the budget runs undisturbed.
for _ in 0..=(SETTLE_TIMEOUT + 2.0) as usize {
step(&mut app, 1.0, |p| {
p.total = 2000;
p.ready = 500;
p.colliders_pending = 300;
p.scene_ready = false;
});
}
assert!(
!settling(&mut app),
"a dead stream must not hold the body (and the screen) forever"
);
}
/// 0710's fail-closed law is untouched: while the resident world is still the departed map's,
/// frozen counters push the deadline rather than spending it.
#[test]
fn a_stale_world_pushes_the_deadline_before_the_stall_budget_starts() {
let mut app = app();
app.world_mut().resource_mut::<Player>().world_stale = true;
// Twice the budget of stale, frozen frames: no release (focus_resident stays false so
// nothing clears the stale flag).
for _ in 0..(2.0 * SETTLE_TIMEOUT) as usize {
step(&mut app, 1.0, |p| {
p.total = 0;
p.scene_ready = false;
p.focus_resident = false;
});
assert!(settling(&mut app), "released over the departed map's floor");
}
// The destination becomes resident and the stream then stalls: the budget starts HERE.
app.world_mut().resource_mut::<Player>().world_stale = false;
for _ in 0..=(SETTLE_TIMEOUT + 2.0) as usize {
step(&mut app, 1.0, |p| {
p.total = 2000;
p.ready = 500;
p.colliders_pending = 300;
p.scene_ready = false;
p.focus_resident = true;
});
}
assert!(
!settling(&mut app),
"the stall budget never started counting"
);
}
/// The ordinary end: scene presentable and colliders quiet releases at once — even on the very
/// frame the last counter moved, so residency is never delayed by its own arrival.
#[test]
fn residency_releases_on_the_frame_it_lands() {
let mut app = app();
app.world_mut().resource_mut::<Player>().world_stale = false;
step(&mut app, 1.0, |p| {
p.total = 2000;
p.ready = 1999;
p.colliders_pending = 3;
p.scene_ready = false;
});
assert!(settling(&mut app));
step(&mut app, 0.1, |p| {
p.ready = 2000;
p.colliders_pending = 0;
p.scene_ready = true;
});
assert!(!settling(&mut app), "presentable world, hold still on");
}
}
+17 -7
View File
@@ -277,7 +277,7 @@ pub(super) fn feed_actions(
// epoch gate above re-runs this whole resolve the frame the answer lands.
let mut fresh: HashMap<u32, ActionSlot> = HashMap::new();
for (slot, button) in &actions.buttons {
let (texture, count) = match button.kind {
let (texture, count, consumable) = match button.kind {
ACTION_KIND_SPELL => {
let icon = spells.as_ref().and_then(|sp| {
let d = sp.catalog.get(button.action)?;
@@ -291,7 +291,7 @@ pub(super) fn feed_actions(
&commands,
)
});
(icon, 0)
(icon, 0, false)
}
ACTION_KIND_ITEM => {
// The question mark belongs HERE, not in the Lua (decision 0666, correcting
@@ -302,15 +302,23 @@ pub(super) fn feed_actions(
// FrameXML *hides* the icon on a nil texture, feeding nil here and letting a
// Lua `or` paint the fallback would show a BLANK button on faithful
// FrameXML — the placeholder is the engine's, at two sites binary-wide.
let texture = items
.template(button.action, 0, &commands)
.cloned()
let template = items.template(button.action, 0, &commands).cloned();
let texture = template
.as_ref()
.and_then(|t| icons.as_ref()?.catalog.get(t.display_info_id)?.icon.clone())
.unwrap_or_else(|| MISSING_ITEM_ICON.to_string());
let count = store
.map(|s| count_of(&s.0, &items, button.action, InventoryScope::CARRIED))
.unwrap_or(0);
(Some(texture), count)
// The Count fontstring's gate — `IsConsumableAction 0x4e5250`: ammo/thrown by
// InventoryType, or an ON_USE block with NEGATIVE charges
// ([`ItemInfo::is_consumable`], byte-cited there; decision 0926 §3). It comes
// from the SAME ask-once template the icon does, so it belongs on the same
// push: fed from the per-frame state map instead, it answered the Lua one
// frame late for ever and left a fresh character's food with no stack number
// (decision 1301 — the count's half of 0660's login race).
let consumable = template.as_ref().is_some_and(|t| t.is_consumable());
(Some(texture), count, consumable)
}
// A MACRO slot serves **the macro's own icon, never its bound spell's** — the one
// asymmetry in the icon resolver, byte-verified: `0x4e6a50`'s macro arm
@@ -323,8 +331,9 @@ pub(super) fn feed_actions(
.get(button.action as usize)
.and_then(|m| m.texture.clone()),
0,
false,
),
_ => (None, 0),
_ => (None, 0, false),
};
fresh.insert(
u32::from(*slot) + 1,
@@ -333,6 +342,7 @@ pub(super) fn feed_actions(
kind: button.kind,
action: button.action,
count,
consumable,
},
);
}
+108 -1
View File
@@ -12,7 +12,7 @@
use std::collections::HashMap;
use benilla_formats::{ItemDisplay, ItemDisplayCatalog};
use benilla_protocol::messages::{ActionButton, ACTION_KIND_ITEM};
use benilla_protocol::messages::{ActionButton, ItemSpellEntry, ACTION_KIND_ITEM};
use benilla_ui::script::UiScript;
use bevy::prelude::*;
@@ -79,6 +79,14 @@ fn fed_texture(app: &mut App) -> Option<String> {
.unwrap()
}
/// What the VM answers `ActionBar.xml`'s Count gate — the ref's 1/nil, so `None` is "no".
fn fed_consumable(app: &mut App) -> Option<i64> {
app.world_mut()
.non_send_resource::<UiScript>()
.eval::<Option<i64>>(&format!("return IsConsumableAction({JERKY_ACTION})"))
.unwrap()
}
/// The whole bug in one test: the first resolve issues the query and can only show the
/// placeholder; the frame the answer lands, the slot re-resolves and the real icon arrives.
/// Before decision 0660 the second half never happened — the button kept the question mark for the
@@ -115,6 +123,105 @@ fn a_landed_item_template_redisplays_the_action_slot() {
);
}
/// The Count fontstring's **gate** rides the same landed template as the icon (decision 1301).
///
/// `IsConsumableAction 0x4e5250` reads nothing but the slot's item template, so it moves exactly
/// when the icon does. Fed from the per-frame *state* map it could not: that feed runs `.after`
/// this one and fires no event of its own for the flag, so the `ACTIONBAR_SLOT_CHANGED` that
/// repaints the button always carried the previous frame's answer — and at login the previous
/// frame had no template at all. The button kept its icon and lost its stack number for the whole
/// session (the director's report, 2026-08-14). This is 0660's race again, one field over, which
/// is why the guard belongs beside it: the two are one push or they are two bugs.
#[test]
fn a_landed_item_template_also_lands_the_consumable_gate() {
let (mut app, _rx) = app_with_food_on_the_bar();
app.update();
assert_eq!(
fed_consumable(&mut app),
None,
"a cold template cannot answer the gate — the ask is still in flight"
);
// Tough Jerky's real row (vmangos `item_template` 117, read 2026-08-14): one ON_USE block,
// spell 433 Food, SpellCharges **-1** — the destroy-on-use sign `is_consumable` tests.
let mut info = test_template("Tough Jerky");
info.display_info_id = JERKY_DISPLAY;
info.spells = vec![ItemSpellEntry {
index: 0,
spell_id: 433,
trigger: 0,
charges: -1,
cooldown_ms: -1,
category: 0,
category_cooldown_ms: -1,
}];
app.world_mut()
.resource_mut::<Items>()
.insert_template(JERKY, Some(info));
app.update();
assert_eq!(
fed_texture(&mut app).as_deref(),
Some(JERKY_ICON),
"the icon lands (the control — this half never broke)"
);
assert_eq!(
fed_consumable(&mut app),
Some(1),
"…and the gate lands with it, on the same push, so the repaint that follows paints a count"
);
}
/// `IsConsumableAction 0x4e5250` — the gate's own law
/// ([`benilla_protocol::ItemInfo::is_consumable`], fed into [`benilla_ui::script::ActionSlot`]).
/// The director's B201 is the mount row: an on-use item with no charges wore a stack number under
/// it because we tested `Class == 0` instead of the reference's two clauses (decision 0926 §3).
#[test]
fn is_consumable_is_ammo_thrown_or_a_negative_charge_use_block() {
let block = |trigger: u32, charges: i32| ItemSpellEntry {
index: 0,
spell_id: 439,
trigger,
charges,
cooldown_ms: -1,
category: 0,
category_cooldown_ms: -1,
};
// The report: a mount. Class 15 Miscellaneous, InventoryType 0, one ON_USE block whose
// SpellCharges is 0 — the item is not destroyed by using it.
let mut mount = test_template("Red Skeletal Horse");
mount.class = 15;
mount.spells = vec![block(0, 0)];
assert!(!mount.is_consumable(), "a mount has no stack to show");
// A potion: Class 0, but that is not what decides it — the ON_USE block's -1 charges is.
let mut potion = test_template("Minor Healing Potion");
potion.spells = vec![block(0, -1)];
assert!(potion.is_consumable());
// …and Class 0 alone (a conjured-water-shaped template with no on-use block at all) is
// NOT enough, which is exactly what the old `class == 0` read got wrong in reverse.
let classless = test_template("Trade Good");
assert!(!classless.is_consumable());
// The InventoryType clause, both members — ammo and thrown always count, charges or not.
for inv in [24u32, 25] {
let mut ammo = test_template("Rough Arrow");
ammo.inventory_type = inv;
assert!(ammo.is_consumable(), "InventoryType {inv} is consumable");
}
let mut trinket = test_template("Trinket");
trinket.inventory_type = 12;
assert!(!trinket.is_consumable());
// An ON_EQUIP proc with negative charges is not an ON_USE block: the trigger must be 0.
let mut proc_item = test_template("Proc Weapon");
proc_item.spells = vec![block(1, -1)];
assert!(!proc_item.is_consumable());
}
/// The epoch is a *change* gate, not a per-frame re-resolve: once the answer has landed and been
/// fed, an idle frame re-resolves nothing. (Guards the obvious over-correction — turning the feed
/// into an every-frame rebuild of all 120 slots.)
+3 -56
View File
@@ -456,11 +456,9 @@ pub(super) fn feed_action_state(
}
ACTION_KIND_ITEM => {
let template = items.template(button.action, 0, &commands).cloned();
// `IsConsumableAction 0x4e5250` — ammo/thrown by InventoryType, or an ON_USE
// block with NEGATIVE charges ([`ItemInfo::is_consumable`], byte-cited there).
// NOT `Class == 0`: that read was ours, it is not the reference's, and it is what
// put a stack number under a mount (Class 15) on the bar.
st.consumable = template.as_ref().is_some_and(|t| t.is_consumable());
// `IsConsumableAction` is NOT fed from here. It reads nothing but this template
// (`0x4e5250`), so it is slot IDENTITY, and it rides the identity feed's push
// beside the count it gates — `super::feed`'s ITEM arm, decision 1301.
let count = me
.map(|(s, _, _, _)| {
count_of(&s.0, &items, button.action, InventoryScope::CARRIED)
@@ -579,57 +577,6 @@ mod tests {
}
}
/// `IsConsumableAction 0x4e5250` — the gate on the bar's Count fontstring
/// ([`benilla_protocol::ItemInfo::is_consumable`], fed into [`ActionState::consumable`]).
/// The director's B201 is the mount row: an on-use item with no charges wore a stack number
/// under it because we tested `Class == 0` instead of the reference's two clauses.
#[test]
fn is_consumable_is_ammo_thrown_or_a_negative_charge_use_block() {
use benilla_protocol::messages::ItemSpellEntry;
let block = |trigger: u32, charges: i32| ItemSpellEntry {
index: 0,
spell_id: 439,
trigger,
charges,
cooldown_ms: -1,
category: 0,
category_cooldown_ms: -1,
};
// The report: a mount. Class 15 Miscellaneous, InventoryType 0, one ON_USE block whose
// SpellCharges is 0 — the item is not destroyed by using it.
let mut mount = crate::items::test_template("Red Skeletal Horse");
mount.class = 15;
mount.spells = vec![block(0, 0)];
assert!(!mount.is_consumable(), "a mount has no stack to show");
// A potion: Class 0, but that is not what decides it — the ON_USE block's -1 charges is.
let mut potion = crate::items::test_template("Minor Healing Potion");
potion.spells = vec![block(0, -1)];
assert!(potion.is_consumable());
// …and Class 0 alone (a conjured-water-shaped template with no on-use block at all) is
// NOT enough, which is exactly what the old `class == 0` read got wrong in reverse.
let classless = crate::items::test_template("Trade Good");
assert!(!classless.is_consumable());
// The InventoryType clause, both members — ammo and thrown always count, charges or not.
for inv in [24u32, 25] {
let mut ammo = crate::items::test_template("Rough Arrow");
ammo.inventory_type = inv;
assert!(ammo.is_consumable(), "InventoryType {inv} is consumable");
}
let mut trinket = crate::items::test_template("Trinket");
trinket.inventory_type = 12;
assert!(!trinket.is_consumable());
// An ON_EQUIP proc with negative charges is not an ON_USE block: the trigger must be 0.
let mut proc_item = crate::items::test_template("Proc Weapon");
proc_item.spells = vec![block(1, -1)];
assert!(!proc_item.is_consumable());
}
/// The `GetMinMaxRange 0x6e3480` transcription: melee reach floor, the ranged reach pad on
/// both bounds, the self-cast short-circuit, and the rangeless self row.
#[test]
+21 -7
View File
@@ -98,7 +98,16 @@ impl Plugin for UiCharPlugin {
app.init_resource::<CharFeedState>().add_systems(
Update,
(
feed_char.before(UiInput),
// `.before(feed_containers)`: the container feed's `BAG_UPDATE` handlers read the
// inventory surface synchronously — an addon mirroring a bag asks the bag ITEM's
// own row (`GetInventoryItemLink("player", ContainerIDToInventoryID(bag))`,
// Bagnon_Forever's size row) — so the inventory push must land first. Unordered,
// the first in-world burst raced it and recorded every equipped bag linkless
// (`s = "8,0,"` in the director's Bagnon_Forever data). The house "push before
// fire" rule, across the two feeds.
feed_char
.before(crate::ui_items::feed::feed_containers)
.before(UiInput),
watch_skill_ups.before(UiInput),
feed_skills.before(UiInput),
drain_skill_abandons.after(UiInput),
@@ -824,12 +833,17 @@ fn feed_char(
let memo = feed.vm.get(&script);
let Some(store) = self_q.iter().next() else {
if memo.last_stats.is_some() || memo.last_inv.is_some() {
script.set_player_combat_stats(None);
script.set_inventory_slots(Default::default());
memo.last_stats = None;
memo.last_inv = None;
}
// The self store's absence is NO DATA, never "the player has nothing equipped". The one
// window this branch runs with a previously-fed VM is the logout despawn frames —
// `SMSG_LOGOUT_COMPLETE` despawns the entity a full Update before `OnExit(InWorld)` runs
// the shutdown, and the `PLAYER_LOGOUT` handlers that follow still read equipment
// (`GetInventoryItemLink("player", …)` at logout is a stock addon save pattern; the
// reference keeps it valid through its shutdown). The clear this branch used to push was
// a pre-1290 relic: one VM then lived across logouts and the next character must not see
// this one's inventory — now the VM dies with the session and a fresh one starts empty
// ([`crate::ui_script::end_ui_session`]), so the clear only ever fired into a VM about to
// run its logout handlers. Same law as the container feed
// (`ui_items::feed::apply_container_source`).
return;
};
+110 -11
View File
@@ -24,7 +24,7 @@ use super::{
/// The feed's memory of what it last pushed, for per-bag change events. No cooldown-churn gate:
/// the pushed triple carries the ABSOLUTE start, which is frame-stable for a running cooldown.
#[derive(Default)]
pub(super) struct FeedMemory {
pub(crate) struct FeedMemory {
pushed: HashMap<i64, ContainerState>,
/// The last `HasKey()` pushed — kept only so the transition can be logged once instead of
/// every frame. It is the gate the entire keyring UI hangs off, and "the keyring never
@@ -602,7 +602,7 @@ fn bag_family_name(
}
#[allow(clippy::too_many_arguments)]
pub(super) fn feed_containers(
pub(crate) fn feed_containers(
script: Option<NonSendMut<UiScript>>,
mut items: ResMut<Items>,
icons: Option<Res<ItemDisplays>>,
@@ -922,10 +922,61 @@ pub(super) fn feed_containers(
script.set_has_key(key);
}
apply_container_source(
&mut script,
memory,
player.is_some().then_some(fresh),
transitioned,
);
}
/// The feed's outward half: diff `source` against what was last pushed, push each changed bag into
/// the VM and fire the reference's events — `BAG_UPDATE(bagID)` (`PLAYERBANKSLOTS_CHANGED(slot)`
/// for the vault), one `BAG_UPDATE_DELAYED` per batch, then the lock transitions.
///
/// **`source: None` means the self player STORE is absent this frame — "no data source", never
/// "the player has no items."** The two absent windows are pre-arrival at login (the fresh VM's
/// containers are already empty; nothing to say) and the logout despawn frames:
/// `SMSG_LOGOUT_COMPLETE` despawns the self entity (`net/apply/session.rs::logged_out`) at least
/// one full Update before the `OnExit(InWorld)` shutdown, so this runs against a still-live VM.
/// Diffing the absence as an all-empty snapshot fired a full `BAG_UPDATE` burst whose every bag
/// read `GetContainerNumSlots() == 0` — and an addon that mirrors bags into its saved variables
/// deletes a bag's record on size 0 (Bagnon_Forever's `SaveBagData`), so the burst erased the
/// whole record moments before [`crate::ui_script::shutdown_ui_state`] wrote the file. That was
/// the director's offline-bags report: every recently-logged-out character money-only, the view
/// stale. The reference never delivers such a burst — its UI shutdown (`0x490bd0`) runs with the
/// inventory intact — so here the VM simply keeps its last-pushed state and the shutdown writes
/// the bags the player actually had. Lock-clear events still flush either way: they are
/// packet-driven and must not sit around to fire into a later VM.
pub(crate) fn apply_container_source(
script: &mut UiScript,
memory: &mut FeedMemory,
source: Option<HashMap<i64, ContainerState>>,
transitioned: Vec<(i64, u32)>,
) {
// Diff whole bags; push + fire BAG_UPDATE per transition, one BAG_UPDATE_DELAYED per batch. A
// pending-lock transition always flips a slot's `.locked` (part of `ContainerSlot`'s equality),
// so it always shows up here too — but `transitioned`'s ITEM_LOCK_CHANGED fires unconditionally
// below rather than leaning on that invariant.
if let Some(fresh) = source {
diff_and_push(script, memory, fresh);
}
// The lock-transition event (decision 0218: the bag windows' own repaint trigger) — after the
// container push above, so a listener's repaint reads the corrected `.locked` state.
for (bag, slot) in transitioned {
script.fire_event(
"ITEM_LOCK_CHANGED",
vec![ScriptValue::Int(bag), ScriptValue::Int(i64::from(slot))],
);
}
}
/// The present-source half of [`apply_container_source`]: push every changed bag, announce each.
fn diff_and_push(
script: &mut UiScript,
memory: &mut FeedMemory,
fresh: HashMap<i64, ContainerState>,
) {
let changed: Vec<i64> = fresh
.keys()
.chain(memory.pushed.keys())
@@ -973,20 +1024,68 @@ pub(super) fn feed_containers(
memory.pushed = fresh;
script.fire_event("BAG_UPDATE_DELAYED", vec![]);
}
// The lock-transition event (decision 0218: the bag windows' own repaint trigger) — after the
// container push above, so a listener's repaint reads the corrected `.locked` state.
for (bag, slot) in transitioned {
script.fire_event(
"ITEM_LOCK_CHANGED",
vec![ScriptValue::Int(bag), ScriptValue::Int(i64::from(slot))],
);
}
}
#[cfg(test)]
mod tests {
use super::charges_count;
use super::{apply_container_source, charges_count, FeedMemory};
use benilla_protocol::messages::ItemSpellEntry;
use benilla_ui::script::{ContainerState, UiScript};
use std::collections::HashMap;
/// The logout-wipe law (the director's stale-offline-bags report): an **absent** self player
/// store is "no data source", never "the player has no items". `SMSG_LOGOUT_COMPLETE` despawns
/// the self entity a full Update before `OnExit(InWorld)` shuts the VM down, and the feed used
/// to diff that absence as an all-empty snapshot — a `BAG_UPDATE` burst whose every bag read
/// `GetContainerNumSlots() == 0`, which made Bagnon_Forever erase its records right before the
/// saved-variables write. A *present* source that lost a bag is a real transition and must
/// still announce — the law gates on the source's existence, not on its emptiness.
#[test]
fn an_absent_self_player_is_no_source_never_an_empty_bag_burst() {
let mut s = UiScript::new().unwrap();
s.run(
"BAG_EVENTS = 0 \
local f = CreateFrame('Frame') \
f:RegisterEvent('BAG_UPDATE') \
f:SetScript('OnEvent', function() BAG_EVENTS = BAG_EVENTS + 1 end)",
)
.unwrap();
let bag0 = ContainerState {
name: Some("Backpack".into()),
num_slots: 16,
slots: HashMap::new(),
};
let mut memory = FeedMemory::default();
// The in-session push: source present, bag new → pushed + announced.
apply_container_source(
&mut s,
&mut memory,
Some(HashMap::from([(0, bag0)])),
Vec::new(),
);
assert_eq!(s.eval::<i64>("return GetContainerNumSlots(0)").unwrap(), 16);
assert_eq!(s.eval::<i64>("return BAG_EVENTS").unwrap(), 1);
// The logout despawn frame: no store. The VM keeps its last-pushed bags and no event
// fires — an addon reading its bags out of the PLAYER_LOGOUT edge sees them intact.
apply_container_source(&mut s, &mut memory, None, Vec::new());
assert_eq!(
s.eval::<i64>("return GetContainerNumSlots(0)").unwrap(),
16,
"an absent source must not empty the VM's containers"
);
assert_eq!(
s.eval::<i64>("return BAG_EVENTS").unwrap(),
1,
"an absent source must not fire a BAG_UPDATE burst"
);
// A PRESENT source without the bag is a genuine transition: push + announce.
apply_container_source(&mut s, &mut memory, Some(HashMap::new()), Vec::new());
assert_eq!(s.eval::<i64>("return GetContainerNumSlots(0)").unwrap(), 0);
assert_eq!(s.eval::<i64>("return BAG_EVENTS").unwrap(), 2);
}
fn slot(spell_id: u32, charges: i32) -> ItemSpellEntry {
ItemSpellEntry {
+1 -1
View File
@@ -49,7 +49,7 @@ use crate::ui_unit::UnitFeed;
mod drain;
mod equip_error;
mod feed;
pub(crate) mod feed;
use drain::{
drain_container_autoequips, drain_container_destroys, drain_container_moves,
@@ -40,6 +40,7 @@ fn shipped_action_bar_drives_end_to_end() {
kind: 0x00,
action: 100,
count: 0,
consumable: false,
}),
);
s.set_action(
@@ -49,6 +50,7 @@ fn shipped_action_bar_drives_end_to_end() {
kind: 0x00,
action: 101,
count: 0,
consumable: false,
}),
);
s.fire_event("PLAYER_ENTERING_WORLD", vec![]);
@@ -175,6 +177,7 @@ fn state_feedback_drives_cooldown_checked_and_usable_through_the_xml() {
kind: 0x00,
action: 133,
count: 0,
consumable: false,
}),
);
s.fire_event("PLAYER_ENTERING_WORLD", vec![]);
@@ -264,6 +267,7 @@ fn the_cooldown_sweep_paints_over_the_buttons_icon_and_ring() {
kind: 0x00,
action: 133,
count: 0,
consumable: false,
}),
);
s.fire_event("PLAYER_ENTERING_WORLD", vec![]);
@@ -321,6 +325,7 @@ fn a_right_click_on_an_action_button_uses_the_action() {
kind: 0x80, // an ITEM action — the food/mount case the report is about
action: 4540,
count: 5,
consumable: false,
}),
);
s.fire_event("PLAYER_ENTERING_WORLD", vec![]);
@@ -372,6 +377,7 @@ fn shift_click_picks_up_not_uses() {
kind: 0x00,
action: 111,
count: 0,
consumable: false,
}),
);
s.fire_event("PLAYER_ENTERING_WORLD", vec![]);
@@ -435,6 +441,7 @@ fn the_action_bar_lock_stops_the_drag_and_leaves_shift_click_alone() {
kind: 0x00,
action: 111,
count: 0,
consumable: false,
}),
);
s.fire_event("PLAYER_ENTERING_WORLD", vec![]);
@@ -503,6 +510,7 @@ fn drag_drop_onto_another_button_hops_the_displaced_action() {
kind: 0x00,
action: 111,
count: 0,
consumable: false,
}),
);
s.set_action(
@@ -512,6 +520,7 @@ fn drag_drop_onto_another_button_hops_the_displaced_action() {
kind: 0x00,
action: 222,
count: 0,
consumable: false,
}),
);
s.fire_event("PLAYER_ENTERING_WORLD", vec![]);
@@ -553,10 +562,15 @@ fn drag_drop_onto_another_button_hops_the_displaced_action() {
/// on-use spell with zero charges and `InventoryType` 0, so `IsConsumableAction 0x4e5250` answers
/// false and the ref paints nothing at all. It repaints on `ACTIONBAR_SLOT_CHANGED` alongside the
/// icon (the same event the identity resolve fires).
///
/// **The gate rides the SLOT, not the state map** (decision 1301). It used to be pushed through
/// `set_action_state`, and this test set that up *before* the repaint — the opposite of the
/// runtime order, where the identity feed fires `ACTIONBAR_SLOT_CHANGED` a whole system before the
/// state feed writes anything. That inversion is why a passing test sat over a fresh character
/// whose food showed no stack number at all. Every push here is now one `set_action`, which is
/// the only order the runtime can produce.
#[test]
fn count_fontstring_follows_is_consumable_action_not_the_bag_count() {
use benilla_ui::script::ActionState;
let mut s = UiScript::new().unwrap();
s.set_screen_size(1024.0, 768.0);
load_action_bar(&s);
@@ -571,6 +585,7 @@ fn count_fontstring_follows_is_consumable_action_not_the_bag_count() {
kind: 0x00, // SPELL
action: 111,
count: 42, // the app never actually sets this for a spell — proves the XML, not the feed
consumable: false,
}),
);
s.set_action(
@@ -580,17 +595,11 @@ fn count_fontstring_follows_is_consumable_action_not_the_bag_count() {
kind: 0x80, // ITEM — a stack of food
action: 117,
count: 15,
}),
);
s.set_action_state(
2,
Some(ActionState {
consumable: true,
..Default::default()
}),
);
// The report's own shape: an ITEM action the player holds exactly one of, which is NOT
// consumable (a mount). The count is fed all the same; the gate is what must suppress it.
// The report's own shape: an ITEM action the player holds eleven of, which is NOT consumable
// (a mount). The count is fed all the same; the gate is what must suppress it.
s.set_action(
3,
Some(ActionSlot {
@@ -598,13 +607,7 @@ fn count_fontstring_follows_is_consumable_action_not_the_bag_count() {
kind: 0x80,
action: 13332,
count: 11,
}),
);
s.set_action_state(
3,
Some(ActionState {
consumable: false,
..Default::default()
}),
);
s.fire_event("PLAYER_ENTERING_WORLD", vec![]);
@@ -649,6 +652,7 @@ fn count_fontstring_follows_is_consumable_action_not_the_bag_count() {
kind: 0x80,
action: 117,
count: 0,
consumable: true,
}),
);
s.fire_event("ACTIONBAR_SLOT_CHANGED", vec![ScriptValue::Int(2)]);
@@ -866,6 +870,7 @@ fn state_events_leave_empty_wells_untinted() {
kind: 0x00,
action: 133,
count: 0,
consumable: false,
}),
);
s.set_action_state(
@@ -934,6 +939,7 @@ fn an_occupied_slot_going_empty_leaves_no_white_plate() {
kind: 0x00,
action: 5185,
count: 0,
consumable: false,
}),
);
s.set_action_state(
@@ -60,6 +60,15 @@ use super::content;
/// The folder name under each root, and the reference's own spelling.
const ADDONS_DIR: &str = "AddOns";
/// The world-entry load's VM instruction bound, per addon (decision 1306) — and the addon
/// harness's survey bound, one number for both so the measurement stays shared. MEASURED, not
/// guessed (e463649e): across the 218-addon corpus, 214 addons execute under 1M VM instructions
/// and the heaviest legitimate one (Enchantrix) reaches 4M — this is ~50x that. An addon that
/// crosses it is not slow, it is not coming back; the raise fails that addon with a distinctive
/// message and the walk loads everyone else, where before 1306 the client sat frozen on the
/// loading screen with zero diagnostics (B271's class).
pub(crate) const LOAD_INSTRUCTION_BUDGET: u64 = 200_000_000;
/// Where one addon's files come from.
///
/// The two arms are the same question asked of two stores, and keeping them one enum is what lets
@@ -193,6 +202,10 @@ impl Addon {
Err(e) => {
let e = format!("{}/{file}: {e}", self.name);
error!("ui_script: {e}");
// A file-scope failure is a script error, and script errors reach the
// player (1305): queued for the Lua error handler — `_ERRORMESSAGE`'s
// red dialog once BasicControls has run — never only a terminal line.
script.report_script_error(&e);
failures.push(e);
}
}
@@ -218,6 +231,12 @@ impl Addon {
}
for e in &report.errors {
error!("ui_script({}/{file}): {e}", self.name);
// Same contract as the Lua arm above: a `<Script file=>` chunk that raised or an
// OnLoad that errored is a script error the player gets to see (1305).
// `_ERRORMESSAGE`'s own IsVisible guard shows a burst's FIRST failure only,
// which is the reference's behaviour too. Document *parse* failures stay
// log-only, like the reference's FrameXML.log.
script.report_script_error(&format!("{}/{file}: {e}", self.name));
failures.push(format!("{}/{file}: {e}", self.name));
}
info!(
@@ -671,7 +690,22 @@ pub(super) fn load_third_party(
);
continue;
}
// Re-arm the load bound PER ADDON (decision 1306): the budget is a per-addon fact (the
// corpus was measured per addon), and without the reset one runaway would spend the
// whole allowance and fail every addon after it for somebody else's loop. A dependency
// chain loads under its dependent's arming, which is the same accounting the harness's
// per-survey arming gives it.
script.set_instruction_budget(LOAD_INSTRUCTION_BUDGET);
let _ = state.load(script, &addons, &addon.name);
let spent = script.instructions_used();
if spent > 1_000_000 {
// Heavy is worth a line: this counter is how the budget was chosen, and a report
// like B271's needs exactly this number without a harness run.
info!(
"ui_script: {} spent {spent} VM instructions loading",
addon.name
);
}
}
state.failures
}
@@ -1307,3 +1307,87 @@ fn the_character_dropdown_is_as_wide_as_the_names_in_it() {
"a row's text must end inside the list: text right edge {row_r} > list right edge {list_r}"
);
}
/// **Bug 5 — the director's stale offline bags.** Bagnon_Forever mirrors the live bags into
/// `BagnonForeverData` (its saved variable) so the dropdown can show other characters' inventories
/// offline. Its recorder does a full scan only on a character's FIRST login; afterwards it trusts
/// `BAG_UPDATE(bagID)`, and `SaveBagData` **deletes** a bag's record whenever
/// `GetContainerNumSlots(bag)` reads 0. The feed used to hand it exactly that: on the logout
/// despawn frame the self store is gone, and diffing the absence as an all-empty snapshot fired a
/// size-0 `BAG_UPDATE` for every bag — every record erased seconds before the saved-variables
/// write, leaving each recently-logged-out character money-only ("g" has no delete path) and the
/// offline view stale. Here: the login scan records the live backpack, and the record must still
/// be intact after the absent-source frame and the logout events — what the shutdown write
/// actually persists. The mechanism's own law is pinned beside the feed
/// (`ui_items::feed::tests::an_absent_self_player_is_no_source_never_an_empty_bag_burst`).
#[test]
fn bagnon_forevers_records_survive_the_logout_boundary() {
let root = corpus_or_skip!();
let mut s = seat(&root, Seat::BeforeAddons);
// The first-login scan (PLAYER_LOGIN, inside `seat`) recorded the live backpack: 16 slots,
// the pouch in slot 1, in Bagnon_Forever's own short-link shape.
let record = "local r = BagnonForeverData[GetRealmName()][UnitName('player')] \
return tostring(r[0] and r[0].s), tostring(r[0] and r[0][1])";
let (size, item) = s
.eval::<(String, String)>(record)
.expect("the record reads");
assert_eq!(
size, "16,0,",
"the login scan records the backpack's size row"
);
assert_eq!(
item, "4496",
"the login scan records the occupied slot's short link"
);
// An equipped bag arriving on the feed — inventory surface FIRST (the bag item in INV slot
// 20), then the container push and its `BAG_UPDATE`: the order the schedule guarantees
// (`feed_char.before(feed_containers)`, ui_char.rs). The recorder's size row asks the bag
// ITEM's own count and link (`GetInventoryItemLink("player", ContainerIDToInventoryID(1))`);
// raced the other way it recorded every equipped bag linkless — the `s = "8,0,"` rows in the
// director's data.
let mut inv: benilla_ui::script::InventorySlots = Default::default();
inv[20] = Some(benilla_ui::script::InvSlotView {
item_id: 4497,
count: 1,
quality: 1,
name: Some("Small Green Pouch".into()),
link: Some("|cffffffff|Hitem:4497:0:0:0|h[Small Green Pouch]|h|r".into()),
..Default::default()
});
s.set_inventory_slots(inv);
s.set_container(
1,
Some(ContainerState {
name: Some("Small Green Pouch".into()),
num_slots: 6,
slots: Default::default(),
}),
);
s.fire_event("BAG_UPDATE", vec![benilla_ui::script::ScriptValue::Int(1)]);
let bag_row = s
.eval::<String>(
"return tostring(BagnonForeverData[GetRealmName()][UnitName('player')][1].s)",
)
.expect("the bag row reads");
assert_eq!(
bag_row, "6,1,4497",
"an equipped bag's size row carries its own count and short link"
);
// The logout boundary: the despawn frame (no self store) and then the shutdown's own events.
// The record the write persists must be the bags the player actually had.
let mut memory = crate::ui_items::feed::FeedMemory::default();
crate::ui_items::feed::apply_container_source(&mut s, &mut memory, None, Vec::new());
s.fire_event("PLAYER_LEAVING_WORLD", Vec::new());
s.fire_event("PLAYER_LOGOUT", Vec::new());
let (size, item) = s
.eval::<(String, String)>(record)
.expect("the record reads");
assert_eq!(
size, "16,0,",
"the logout boundary must not erase the size row"
);
assert_eq!(item, "4496", "the logout boundary must not erase the slot");
}
@@ -90,28 +90,45 @@ fn error_message_returns_its_argument_and_can_be_replaced() {
assert!(s.errors().is_empty(), "{:?}", s.errors());
}
/// The default error handler is still ours, deliberately — BasicControls.xml drops the
/// reference's `seterrorhandler(_ERRORMESSAGE)` so a runtime fault stays visible to
/// `UiScript::errors()` (and therefore to the corpus harness) instead of becoming a modal.
/// The divergence is stated at the site; this is what pins it.
/// The reference's closing line is transcribed since decision 1305: BasicControls installs
/// `seterrorhandler(_ERRORMESSAGE)`, so a script error pops the dialog like the real client's —
/// and the host channel stays sighted because the ENGINE records every caught error before the
/// dispatch ever runs (that dual channel is what dissolved the old divergence, whose guard this
/// test used to be).
#[test]
fn the_default_error_handler_is_not_replaced_by_error_message() {
let s = basic_controls();
fn error_message_is_the_installed_handler_and_the_host_channel_stays_sighted() {
let mut s = basic_controls();
assert!(
!s.eval::<bool>("return geterrorhandler() == _ERRORMESSAGE")
s.eval::<bool>("return geterrorhandler() == _ERRORMESSAGE")
.unwrap(),
"installing _ERRORMESSAGE as the default would blind every instrument in the arc"
"the reference's own default handler is installed (1305)"
);
// ...but the pair still works, which is what an addon's pcall wrapper actually calls.
// An addon's pcall wrapper calls geterrorhandler()(msg) — with _ERRORMESSAGE installed that
// pops the dialog, exactly as it does in the real client.
s.run(r#"geterrorhandler()("reported")"#).unwrap();
assert!(
s.errors().iter().any(|e| e.contains("reported")),
"geterrorhandler()(msg) reaches the error channel: {:?}",
s.eval::<bool>("return ScriptErrors:IsVisible()").unwrap(),
"the dialog is the report now"
);
assert_eq!(
s.eval::<String>("return ScriptErrors_Message:GetText()")
.unwrap(),
"reported"
);
// And an ENGINE-caught error still reaches `UiScript::errors()` — the instruments' channel —
// with the dispatch adding the dialog on top, not instead.
s.run("BrokenProbe = CreateFrame('Frame') BrokenProbe:RegisterEvent('B271_PROBE') BrokenProbe:SetScript('OnEvent', function() error('fault') end)")
.unwrap();
s.fire_event("B271_PROBE", vec![]);
assert!(
s.errors().iter().any(|e| e.contains("fault")),
"engine-caught errors stay on the host channel: {:?}",
s.errors()
);
s.dispatch_script_errors_to_handler();
assert!(
!s.eval::<bool>("return ScriptErrors:IsVisible()").unwrap(),
"and it does NOT pop the dialog"
s.eval::<bool>("return ScriptErrors:IsVisible()").unwrap(),
"and the dispatch keeps the dialog up for the player"
);
}
@@ -265,6 +265,12 @@ pub(super) fn drive_script(
}
drop(measure_span);
let us_measure = lap();
// The player's half of the error contract before the host's (decision 1305): every caught
// script error goes to the Lua error handler — `_ERRORMESSAGE` → the ScriptErrors dialog
// since BasicControls installs it, an addon's own handler if it chose one — and then the
// same errors drain to the log as always. Dispatch first so a handler that itself fails
// lands in this frame's drain, not the next one's.
script.dispatch_script_errors_to_handler();
for err in script.take_errors() {
warn!("ui_script: {err}");
}
@@ -146,6 +146,10 @@ pub(crate) fn load_ingame_ui_on_world_entry(world: &mut World) {
// (`AddOn_Load 0x51f240` steps 2 → 4 → 6, decision 1128); reversing it means the defaults
// always win and nothing can ever be remembered.
finish_ui_load(&mut script);
// The load edge is over: disarm the instruction bound `load_ingame_ui` installed (decision
// 1306). From here every OnUpdate and event handler runs unhooked — a session must not kill
// a player's addon for being slow; only a load that never returns is fair game.
script.clear_instruction_budget();
world.insert_non_send_resource(script);
world.insert_resource(AddOnIdentity(identity));
}
@@ -390,6 +394,10 @@ pub(crate) fn shutdown_on_exit(
/// `PLAYER_ENTERING_WORLD` keeps its own per-entry latch in [`crate::ui_unit`] and still lands
/// after this, since it waits on the self descriptor arriving over the wire.
pub(crate) fn finish_ui_load(script: &mut UiScript) {
// Still the load edge, so still bounded (1306) — re-armed because the walk's last addon left
// an arbitrary amount on the counter, and the saved-variables chunk plus every PLAYER_LOGIN
// handler deserve the full allowance. The entry edge disarms after this returns.
script.set_instruction_budget(addons::LOAD_INSTRUCTION_BUDGET);
crate::ui_saved::load_saved_variables(script);
script.fire_event("PLAYER_LOGIN", vec![]);
}
@@ -111,6 +111,12 @@ pub(crate) fn load_ingame_ui(
identity: Option<&(String, String)>,
version_check: bool,
) -> Vec<String> {
// The whole load edge runs bounded (decision 1306): the reference files sourced off the
// player's own chain, our builtin, and every addon (which re-arms per addon in
// `load_third_party`). A chunk that never returns fails as a load error instead of freezing
// the client on the loading screen; the caller disarms once the edge is done
// (`lifecycle::load_ingame_ui_on_world_entry`), so the session's steady state runs unhooked.
script.set_instruction_budget(super::addons::LOAD_INSTRUCTION_BUDGET);
// The reference FrameXML this client SOURCES off the patch chain rather than transcribing
// ([`super::reference_ui`], whose header is the rule). It runs FIRST, before our own files,
// precisely so that every global we define ourselves overwrites the reference's — its
+6 -1
View File
@@ -491,7 +491,10 @@ fn demo_unit_feed(script: Option<NonSendMut<UiScript>>, mut fired: Local<VmMemo<
// (buttons 1,2,3,8,12) so the ui-actionbar capture shows icons seated left-to-right. Slot
// 80 is an ITEM-kind action (decision 0216 §7) with a synthetic count of 5, so the capture
// also proves the Count fontstring wires up (`GetActionCount` reads the engine's own
// pushed count directly — no live server template needed in a capture).
// pushed count directly — no live server template needed in a capture). It must also seed
// the `IsConsumableAction` GATE, or the count paints nothing: 0926 put a gate in front of
// it that this seed never fed, so the capture quietly stopped showing the "5" it exists to
// prove (found while fixing decision 1301).
script.set_bonus_bar_offset(1);
for (action, icon, kind, id, count) in [
(
@@ -540,6 +543,8 @@ fn demo_unit_feed(script: Option<NonSendMut<UiScript>>, mut fired: Local<VmMemo<
kind,
action: id,
count,
// The seed's only ITEM slot is the food stack, which is consumable.
consumable: kind == 0x80,
}),
);
}
@@ -67,6 +67,7 @@ fn shipped_multibars_drive_end_to_end() {
kind: 0x00,
action: 100,
count: 0,
consumable: false,
}),
);
s.set_action(
@@ -76,6 +77,7 @@ fn shipped_multibars_drive_end_to_end() {
kind: 0x00,
action: 200,
count: 0,
consumable: false,
}),
);
s.set_action(
@@ -85,6 +87,7 @@ fn shipped_multibars_drive_end_to_end() {
kind: 0x00,
action: 300,
count: 0,
consumable: false,
}),
);
s.fire_event("PLAYER_ENTERING_WORLD", vec![]);
@@ -413,6 +416,7 @@ fn multibar_hover_renders_the_buttons_own_action() {
kind: 0x00,
action: spell,
count: 0,
consumable: false,
}),
);
s.set_spell_tooltip(
@@ -2023,6 +2023,7 @@ fn the_action_bars_page_locks_the_real_bar() {
kind: 0x00,
action: 111,
count: 0,
consumable: false,
}),
);
s.fire_event("PLAYER_ENTERING_WORLD", vec![]);
@@ -475,3 +475,96 @@ fn reload_outside_the_world_is_dropped() {
drop(world);
let _ = std::fs::remove_dir_all(&tmp);
}
/// **B271, the error half.** An addon that raises at file scope while entering world must not
/// take the client with it: the walk reports it, the sibling addon still loads, and the player
/// sees the reference's red ScriptErrors dialog (decision 1305) — the report was debugged
/// entirely off terminal WARN lines because the client showed nothing.
#[test]
fn an_addon_error_while_entering_world_reports_on_screen_and_the_sibling_loads() {
let _l = ENV_LOCK
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let (tmp, _c, _h) = hermetic_probe("addon-error");
// A second addon, alphabetically FIRST, that dies at file scope — so the probe behind it
// proves a broken neighbour drops only itself.
let dir = tmp.join("benilla-config/AddOns/AaBroken");
std::fs::create_dir_all(&dir).expect("broken addon dir");
std::fs::write(dir.join("AaBroken.toc"), "## Interface: 11200\nboom.lua\n").expect("toc");
std::fs::write(dir.join("boom.lua"), "error('B271: file-scope boom')\n").expect("lua");
let mut world = booted_world();
log_in_as(&mut world, "Onehunter", 1);
assert_eq!(
probe_saw(&world).as_deref(),
Some("Onehunter"),
"the addon AFTER the broken one still loads — a neighbour's error drops only itself"
);
// The app's per-frame drain runs the dispatch; the test runs the same call.
let mut script = world
.remove_non_send_resource::<benilla_ui::script::UiScript>()
.expect("VM");
script.dispatch_script_errors_to_handler();
assert!(
script
.eval::<bool>("return ScriptErrors:IsVisible()")
.expect("ScriptErrors exists — BasicControls loaded"),
"the ScriptErrors dialog is on screen — `seterrorhandler(_ERRORMESSAGE)` is installed \
and the engine dispatched the caught error to it"
);
let shown: String = script
.eval::<Option<String>>("return ScriptErrors_Message:GetText()")
.expect("eval")
.unwrap_or_default();
assert!(
shown.contains("B271: file-scope boom"),
"the dialog names the actual error, got: {shown:?}"
);
drop(script);
drop(world);
let _ = std::fs::remove_dir_all(&tmp);
}
/// **B271, the freeze half.** An addon that never returns cannot freeze world entry: the load
/// bound (decision 1306) fails it with the distinctive budget message, the sibling addon still
/// loads, and this test FINISHING is the claim — before 1306 it would hang here forever.
#[test]
fn a_looping_addon_cannot_freeze_world_entry() {
let _l = ENV_LOCK
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let (tmp, _c, _h) = hermetic_probe("addon-loop");
let dir = tmp.join("benilla-config/AddOns/AaSpin");
std::fs::create_dir_all(&dir).expect("spin addon dir");
std::fs::write(dir.join("AaSpin.toc"), "## Interface: 11200\nspin.lua\n").expect("toc");
std::fs::write(dir.join("spin.lua"), "while true do end\n").expect("lua");
let mut world = booted_world();
log_in_as(&mut world, "Onehunter", 1);
assert_eq!(
probe_saw(&world).as_deref(),
Some("Onehunter"),
"the addon after the spinner still loads — the budget failed one addon, not the entry"
);
// The budget raise travelled the load walk's failure arm into the handler queue (1305), so
// the player-facing dialog is where it lands — the frozen loading screen becomes a dialog
// that NAMES the loop.
let mut script = world
.remove_non_send_resource::<benilla_ui::script::UiScript>()
.expect("VM");
script.dispatch_script_errors_to_handler();
let shown: String = script
.eval::<Option<String>>("return ScriptErrors_Message:GetText()")
.expect("eval")
.unwrap_or_default();
assert!(
shown.contains("instruction budget exhausted"),
"the dialog names the runaway loop with the budget's distinctive message, got: {shown:?}"
);
drop(script);
drop(world);
let _ = std::fs::remove_dir_all(&tmp);
}
+231 -21
View File
@@ -8,15 +8,17 @@
//! OR `AttributesEx2 & 0x10` force-admits it). 5875 data: warrior stances (`ex2 0x1`), druid
//! forms (`0x0`), and Stealth (`0x200000`) all pass the exclusion bit; **Ghost Wolf 2645
//! (`ex2 0x2`) is the shipped carrier of the exclusion** — a shaman faithfully gets NO stance
//! bar, and cancels the form at the buff frame instead (verified in the data, 2026-07-31). No
//! shipped spell needs the force-admit bit, so its aura-scan `isActive` leg reads `false` here
//! (a named gap, not a divergence — there is no spell to diverge on).
//! bar, and cancels the form at the buff frame instead (verified in the data, 2026-07-31). The
//! force-admit bit is what builds the **paladin's aura bar**: 465/7294/19746/19876/19888/19891
//! carry `ex2 0x10` with no MOD_SHAPESHIFT effect at all (decision 1302 — correcting 0270's
//! "no 5875 spell uses it", which is what left the aura-scan `isActive` leg unbuilt here).
//! - **Order** (comparator `0x4b2bb0`): ascending `Spell.dbc` `StanceBarOrder`, negative last,
//! spell id tiebreak. (Battle 0 / Def 1 / Berserker 2; Bear 0 … Moonkin 4; Stealth −1 → last.)
//! - **texture**: the form SPELL's icon — `ActiveIconID` while active when nonzero (druid forms'
//! paw), else `SpellIconID`; never `SpellShapeshiftForm.dbc`'s icon.
//! - **isActive**: our form byte (`UNIT_FIELD_BYTES_1` byte 2) == the spell's MOD_SHAPESHIFT
//! `EffectMiscValue`.
//! paw), else `SpellIconID`; never `SpellShapeshiftForm.dbc`'s icon. Elected inside the block
//! BOTH `isActive` arms converge on, so a lit paladin aura wears it too ([`form_texture`]).
//! - **isActive**: two arms on the spell's own `formId` — the form-byte compare for a
//! MOD_SHAPESHIFT spell, the 48-slot aura scan for a force-admitted one ([`form_active`]).
//! - **isCastable**: the active form reads hardcoded-castable; otherwise the usability predicate
//! `0x6e3d60` — the SAME full walk the action bar's `IsUsableAction` runs
//! ([`crate::ui_action::usable`], decision 0269's fold-back: reagents, forms, stealth, aura
@@ -52,9 +54,72 @@ use crate::ui_unit::UnitFeed;
/// no stance bar.
const ATTR_EX2_STANCE_BAR_EXCLUDE: u32 = 0x2;
/// `AttributesEx2` bit `0x10` — FORCE-ADMITS a spell without a MOD_SHAPESHIFT effect (the
/// gate's or-leg). No shipped 5875 spell uses it either.
/// gate's or-leg). 44 shipped 5875 rows carry it; the live ones are the **paladin aura family**
/// (Devotion / Retribution / Concentration / the three resistances / Sanctity / Charismatic) plus
/// Ironweave Battlesuit 27733 and a band of `zzOLD*` rows. Decision 1302 corrects 0270 here.
const ATTR_EX2_STANCE_BAR_FORCE: u32 = 0x10;
/// `GetShapeshiftFormInfo 0x4b45c0`'s **isActive**, both arms (wow-re `shapeshift-bar-api.md`,
/// VERIFIED). The reference forks on the spell's own `formId` — its first `MOD_SHAPESHIFT`
/// effect's `EffectMiscValue`, or 0 when it has no such effect at all:
///
/// - `formId != 0` → active ⇔ the caster's form byte matches it (`4b46a0`–`4b46af`).
/// - `formId == 0` (a **force-admitted** row, `AttributesEx2 & 0x10`) **and `ActiveIconID != 0`**
/// (the gate at `4b46f2`) → active ⇔ this spell's own id is live in the player's 48-slot aura
/// array with **nibble bit 0** of `UNIT_FIELD_AURAFLAGS` set (`4b4739 test al,1` — the
/// *cancelable* bit). That is exactly [`crate::ui_action::toggle::active_action_toggle`], the
/// byte-twin `0x4e55f0` the main action bar's own toggle runs, which is why it is reused rather
/// than open-coded: the §5 confirmed the two scans test the same bit.
///
/// Both arms converge on **one** shared block at `4b4754` — the form-byte match *jumps* there, the
/// aura-scan hit *falls through* into it — and that block is the sole writer of `isActive = 1`.
///
/// The second arm is why a paladin's aura bar can light at all: a paladin Aura carries **no**
/// `MOD_SHAPESHIFT` effect (`formId == 0`), so the form-byte arm can never fire on one, and the
/// button reads "not active" for ever. See decision 1302 — and note that the claim this file used
/// to carry, "no shipped spell needs the force-admit bit", is false: 44 rows carry it and the live
/// ones are exactly the paladin aura family (465 Devotion / 7294 Retribution / 19746
/// Concentration / 19876 Shadow / 19888 Frost / 19891 Fire, `StanceBarOrder` 0..5,
/// `ActiveIconID` 122 apiece — read from the shipped 5875 `Spell.dbc`).
fn form_active(
spell_id: u32,
d: &benilla_formats::SpellDisplay,
form_byte: u8,
store: Option<&ObjectStore>,
) -> bool {
match d.shapeshift_form.unwrap_or(0) {
0 => store.is_some_and(|s| crate::ui_action::toggle::active_action_toggle(spell_id, d, s)),
form => form == u32::from(form_byte),
}
}
/// The button's face: `SpellIconID`, or `ActiveIconID` when the row is active and that column is
/// nonzero — **under either arm of [`form_active`] alike**.
///
/// The election happens inside the one shared block both arms reach (`4b4754`/`4b4763`/`4b4769`);
/// `4b46c6`–`4b46e8` is only the id→path tail and decides nothing.
///
/// Decision **1307** records the correction: 1302 §5 first held this swap to the MOD_SHAPESHIFT
/// arm, reasoning that the icon is picked at a *lower address* than the aura scan and so could
/// not depend on it. That inference was wrong, and instructively so: **MSVC laid
/// the deciding block at a higher address than the scan that feeds it**, so ordering the citations
/// by address yields the opposite control flow. The wow-re §5 on `0x4b45c0` carved it and
/// corrected us — a scan hit at `4b473e` falls *through* `4b4751` into the very block the form-byte
/// match jumps to, and the `mov ebx,[ebp-0xc]` at `4b4751` exists for no reason but to restore the
/// lua_State across that fall-through. So a lit paladin aura **does** wear `ActiveIconID` 122.
///
/// `active_icon` is `None` exactly when the column is 0, so the `or_else` is the reference's own
/// `4b4763 je` — a MOD_SHAPESHIFT row can report active while still painting `SpellIconID`
/// (the two facts are decided by different bytes and do not imply each other). On the force-admit
/// arm that leg is unreachable: entry to the scan is already gated on `ActiveIconID != 0`.
fn form_texture(d: &benilla_formats::SpellDisplay, active: bool) -> Option<String> {
if active {
d.active_icon.clone().or_else(|| d.icon.clone())
} else {
d.icon.clone()
}
}
/// What the feed last pushed (the `state.rs` pattern). No cooldown-churn gate needed: the
/// triple carries the ABSOLUTE start, which is frame-stable for a running cooldown.
#[derive(Default)]
@@ -141,7 +206,7 @@ fn feed_shapeshift_bar(
let fresh: Vec<ShapeshiftFormView> = rows
.into_iter()
.map(|(id, d)| {
let active = form_byte != 0 && d.shapeshift_form == Some(u32::from(form_byte));
let active = form_active(id, d, form_byte, store);
// isCastable: hardcoded true for the ACTIVE form, else the full 0x6e3d60 walk —
// the same predicate (and the same ctx shape) the action bar's IsUsableAction runs.
let castable = active
@@ -155,11 +220,7 @@ fn feed_shapeshift_bar(
};
usable::spell_usable(id, d, &spells, &ctx, &mut items, &commands).0
});
let texture = if active {
d.active_icon.clone().or_else(|| d.icon.clone())
} else {
d.icon.clone()
};
let texture = form_texture(d, active);
let cooldown = cooldowns
.info(id, 0, Some(d), now)
.ui_triple(anchor, ui_now);
@@ -185,8 +246,15 @@ fn feed_shapeshift_bar(
}
}
/// Drain `CastShapeshiftForm`'s queued form spells: cancel the active form (unless its
/// `SpellShapeshiftForm.dbc` flags block it — the silent no-op), cast any other.
/// Drain `CastShapeshiftForm`'s queued form spells. `0x4b4810` forks on the same `formId` the
/// info call does, and each arm has its own cancel test:
///
/// - **`formId != 0`** — the active form cancels, unless the `SpellShapeshiftForm.dbc`
/// `flags1 & 0x2` guard (`0x4b4963`) makes it a silent no-op (warrior stances).
/// - **`formId == 0`, `ActiveIconID != 0`** — the force-admit/aura arm: a live own aura cancels,
/// otherwise cast. There is **no DBC guard on this arm** and there cannot be — a force-admitted
/// spell has no form id to look one up with. Without it a paladin clicking their active aura
/// re-cast it instead of dropping it (decision 1302).
fn drain_shapeshift_casts(
script: Option<NonSendMut<UiScript>>,
targeting: cast_target::CastTargeting,
@@ -197,11 +265,8 @@ fn drain_shapeshift_casts(
return;
};
for spell_id in script.take_shapeshift_casts() {
let form_byte = self_store
.iter()
.next()
.map(|s| s.0.unit_shapeshift_form())
.unwrap_or(0);
let store = self_store.iter().next();
let form_byte = store.map(|s| s.0.unit_shapeshift_form()).unwrap_or(0);
let d = ladder.spells.as_ref().and_then(|s| s.catalog.get(spell_id));
// The active-form fork — shared with the plain `CastSpell` dispatcher's twin
// (`crate::ui_action::toggle`): cancel unless the `0x4b4963` flags1-&-0x2 guard makes
@@ -210,7 +275,18 @@ fn drain_shapeshift_casts(
.spells
.as_ref()
.and_then(|s| s.forms.get(&u32::from(form_byte)));
match d.and_then(|d| crate::ui_action::toggle::form_recast_disposition(d, form_byte, row)) {
// …and its force-admit twin, which is the SAME predicate `form_active` reads for the
// button's latch, so a lit button and a cancelling click can never disagree.
let disposition = d.and_then(|d| {
if d.shapeshift_form.unwrap_or(0) == 0 {
store
.filter(|s| crate::ui_action::toggle::active_action_toggle(spell_id, d, s))
.map(|_| true)
} else {
crate::ui_action::toggle::form_recast_disposition(d, form_byte, row)
}
});
match disposition {
Some(true) => {
debug!("ui_shapeshift: cancel form aura {spell_id}");
let _ = ladder
@@ -226,3 +302,137 @@ fn drain_shapeshift_casts(
ladder.send(spell_id, &targeting.context(), CastCommit::Spell);
}
}
#[cfg(test)]
mod tests {
use super::*;
use benilla_formats::SpellDisplay;
use benilla_protocol::ObjectFields;
/// UNIT_FIELD_AURA 47 / UNIT_FIELD_AURAFLAGS 95 (nibble-packed), as `ui_action::toggle`'s
/// own tests mirror them. `0xb` = occupied (eff-index bits) + cancelable (bit 0).
fn player_with_aura(spell_id: u32) -> ObjectStore {
ObjectStore(ObjectFields::from_pairs(&[(47u16, spell_id), (95, 0xb)]))
}
/// Devotion Aura as the shipped 5875 `Spell.dbc` carries it (read 2026-08-14): force-admitted
/// by `AttributesEx2 0x10`, **no** MOD_SHAPESHIFT effect, `ActiveIconID` 122,
/// `StanceBarOrder` 0.
fn devotion_aura() -> SpellDisplay {
SpellDisplay {
attributes_ex2: ATTR_EX2_STANCE_BAR_FORCE,
shapeshift_form: None,
active_icon_id: 122,
stance_bar_order: 0,
..Default::default()
}
}
/// The bug the director reported: a paladin's aura bar never lights. A paladin Aura has no
/// MOD_SHAPESHIFT effect, so the form-byte arm cannot fire on it — `isActive` is the 48-slot
/// aura scan or it is nothing.
#[test]
fn a_force_admitted_aura_latches_on_its_own_live_aura_not_the_form_byte() {
let devotion = devotion_aura();
let up = player_with_aura(465);
assert!(
form_active(465, &devotion, 0, Some(&up)),
"Devotion Aura is up: the button must read active, with the form byte at 0"
);
assert!(
!form_active(7294, &devotion_aura(), 0, Some(&up)),
"a DIFFERENT aura's button stays dark while Devotion is the one that is up"
);
assert!(
!form_active(
465,
&devotion,
0,
Some(&ObjectStore(ObjectFields::from_pairs(&[])))
),
"no aura, no latch"
);
assert!(
!form_active(465, &devotion, 0, None),
"no player object, no latch"
);
// `ActiveIconID == 0` closes the arm entirely — the reference's own gate on it.
let iconless = SpellDisplay {
active_icon_id: 0,
..devotion_aura()
};
assert!(!form_active(465, &iconless, 0, Some(&up)));
}
/// The `ActiveIconID` election, both arms (1302 §5, carved by the wow-re §5 on `0x4b45c0`).
/// Both determinations converge on the one block that elects it, so a lit aura wears the swirl
/// exactly as a shifted druid wears the paw — and the `ActiveIconID == 0` fallback is the
/// reference's own `4b4763 je`, reachable only on the MOD_SHAPESHIFT arm.
#[test]
fn the_active_icon_swap_reaches_both_arms() {
let shield = Some("Interface\\Icons\\Spell_Holy_DevotionAura".to_string());
let swirl = Some("Interface\\Icons\\Spell_Nature_WispSplode".to_string());
// A druid's Cat Form: the MOD_SHAPESHIFT arm — the paw.
let cat = SpellDisplay {
shapeshift_form: Some(1),
active_icon_id: 122,
active_icon: swirl.clone(),
icon: shield.clone(),
..Default::default()
};
assert_eq!(form_texture(&cat, true), swirl);
assert_eq!(form_texture(&cat, false), shield);
// A paladin aura reaches `active` through the force-admit arm — and elects the SAME
// column. Address order misled us here; the shared block at `4b4754` is the law.
let devotion = SpellDisplay {
active_icon: swirl.clone(),
icon: shield.clone(),
..devotion_aura()
};
assert_eq!(
form_texture(&devotion, true),
swirl,
"the aura-scan hit falls through into the block that elects ActiveIconID"
);
assert_eq!(form_texture(&devotion, false), shield);
// `4b475c` sets isActive BEFORE `4b4763` tests the column, so a MOD_SHAPESHIFT row with
// ActiveIconID 0 is active AND still paints SpellIconID. (A warrior stance is this row.)
let stance = SpellDisplay {
shapeshift_form: Some(17),
active_icon_id: 0,
active_icon: None,
icon: shield.clone(),
..Default::default()
};
assert_eq!(
form_texture(&stance, true),
shield,
"active and unswapped are decided by different bytes and do not imply each other"
);
}
/// The control: a MOD_SHAPESHIFT spell still reads the FORM BYTE, and never the aura array.
/// Battle Stance's own aura being live must not be what lights it (a stance carries
/// `ActiveIconID` 0, so the other arm could not fire anyway — this pins the fork, not the
/// data).
#[test]
fn a_shapeshift_form_still_latches_on_the_form_byte() {
let battle_stance = SpellDisplay {
shapeshift_form: Some(17),
active_icon_id: 0,
..Default::default()
};
let no_auras = ObjectStore(ObjectFields::from_pairs(&[]));
assert!(form_active(2457, &battle_stance, 17, Some(&no_auras)));
assert!(!form_active(2457, &battle_stance, 1, Some(&no_auras)));
assert!(
!form_active(2457, &battle_stance, 0, Some(&player_with_aura(2457))),
"out of the form: a live aura row is not what the MOD_SHAPESHIFT arm reads"
);
}
}
+11 -1
View File
@@ -852,7 +852,17 @@ fn feed_units(
Some(s)
});
script.set_unit("player", player.clone());
// `"player"` is pushed only while the descriptor EXISTS: its absence is "no data source",
// never "the player stopped existing". The two absent windows are pre-arrival at login —
// where a `None` push would erase the roster seat (`seat_from_roster`) that addon file scopes
// and the loading-screen UI read — and the logout despawn frames, where `PLAYER_LOGOUT`
// handlers still key their saved state by `UnitName("player")` (the reference keeps the unit
// valid through its shutdown; a fresh VM starts with no `"player"` anyway, so nothing needs
// the clear). `"target"` keeps the unconditional push: a selection's absence IS data — the
// deselect/despawn transition the real client also reports.
if player.is_some() {
script.set_unit("player", player.clone());
}
script.set_unit("target", target.clone());
// The XP bar's feed: push our own avatar's PLAYER_XP / PLAYER_NEXT_LEVEL_XP (both PRIVATE, only
+2 -2
View File
@@ -1,6 +1,6 @@
//! One-off extraction helper (session tool, not shipped): `mpqx <data-dir> <virtual-path> <out>`.
//! Reads through the SAME patch chain as the runtime ([`benilla_formats::Chain`], the full
//! `VANILLA_LOAD_ORDER`) — it used to hand-roll a partial archive list in `benilla-mpq` and silently
//! Reads through the SAME patch chain as the runtime ([`benilla_formats::Chain`], the full vanilla
//! mount law) — it used to hand-roll a partial archive list in `benilla-mpq` and silently
//! miss whole archives (`wmo.MPQ`: "not found" for every building/ship). A path the top archive
//! **delete-marks** is reported as DELETED (exit 2) — the client doesn't load it; look for a
//! `Blizzard_*` addon replacement (decision 0246).
+144 -14
View File
@@ -7,9 +7,10 @@
//! `&self` (a fresh OS handle per read, no seek-state sharing), so the cache is gone and one `Chain`
//! serves both the `&self` concurrent Bevy `AssetReader` path and the `&mut` streaming-loader path.
//!
//! Later archives override earlier ones for files sharing an internal path (so `patch-2.MPQ` wins); a
//! read resolves a name to the highest-priority archive that holds it. Base content archives carry no
//! `(listfile)`, so resolution is by name **hash**, which works without one.
//! Later archives override earlier ones for files sharing an internal path (so a patch archive
//! wins); a read resolves a name to the highest-priority archive that holds it. Base content
//! archives carry no `(listfile)`, so resolution is by name **hash**, which works without one.
//! Which archives mount, and in what order, is [`mount_order`]'s law (decision 1300).
use std::collections::HashSet;
use std::path::Path;
@@ -17,7 +18,7 @@ use std::path::Path;
use anyhow::{anyhow, bail, Context, Result};
use benilla_mpq::Archive;
use crate::VANILLA_LOAD_ORDER;
use crate::VANILLA_BASE_ORDER;
/// One entry from a chain listing: an internal path and its uncompressed size.
pub struct ChainEntry {
@@ -31,20 +32,75 @@ pub struct Chain {
archives: Vec<Archive>,
}
/// `patch-?.MPQ` with the reference's FindFirstFileW semantics: `?` matches **exactly one**
/// character, case-insensitively — `patch-3.MPQ` mounts, `patch-10.MPQ` does not (VERIFIED at the
/// glob template `0x82edbc` and its wrapper `0x42ad10`; wow-re `patch-mount-order.md`).
fn is_patch_glob_match(name: &str) -> bool {
let lower = name.to_ascii_lowercase();
let Some(mid) = lower
.strip_prefix("patch-")
.and_then(|rest| rest.strip_suffix(".mpq"))
else {
return false;
};
mid.chars().count() == 1
}
/// The vanilla mount law over a `Data` directory listing, **ascending priority** (decision 1300;
/// the mounter `0x403740`, carved in wow-re `system/mpq/scratch/patch-mount-order.md`): the ten
/// [`VANILLA_BASE_ORDER`] archives at their fixed priorities, then `patch.MPQ`, then every
/// `patch-?.MPQ` sorted ascending by case-folded name — the binary sorts its glob matches
/// *descending* (`strnicmp`) and walks the array backwards, so the order is deterministic, never
/// filesystem enumeration; `patch-3` overrides `patch-2` — then `speech2.MPQ` above every patch.
/// Names are matched case-insensitively (the reference runs on a case-insensitive filesystem) and
/// returned as found on disk; absent archives are simply not in the result.
fn mount_order(dir_names: &[String]) -> Vec<String> {
let find = |want: &str| {
dir_names
.iter()
.find(|n| n.eq_ignore_ascii_case(want))
.cloned()
};
let mut order: Vec<String> = VANILLA_BASE_ORDER.iter().filter_map(|b| find(b)).collect();
order.extend(find("patch.MPQ"));
let mut patches: Vec<String> = dir_names
.iter()
.filter(|n| is_patch_glob_match(n))
.cloned()
.collect();
patches.sort_by_key(|n| n.to_ascii_lowercase());
order.extend(patches);
order.extend(find("speech2.MPQ"));
order
}
impl Chain {
/// Open a chain from a vanilla `Data` directory (every archive in [`VANILLA_LOAD_ORDER`] that
/// exists, lowest priority first) or a single `.MPQ` file (just that archive).
/// Open a chain from a vanilla `Data` directory (every archive [`mount_order`] finds in it,
/// lowest priority first) or a single `.MPQ` file (just that archive).
///
/// An archive that exists but fails to open is a hard error, deliberately: the reference logs
/// `"Failed to open archive"` and continues, but a silent skip turns a corrupt `dbc.MPQ` — or a
/// modder's malformed `patch-3.MPQ` — into cryptic missing-file failures far downstream. Same
/// composite on a healthy install; a clear error instead of a quirk on a broken one (1300).
pub fn open(path: &Path) -> Result<Self> {
let mut archives = Vec::new();
if path.is_dir() {
for name in VANILLA_LOAD_ORDER {
let mpq = path.join(name);
if mpq.is_file() {
archives.push(
Archive::open(&mpq)
.with_context(|| format!("opening {}", mpq.display()))?,
);
}
let mut names: Vec<String> = std::fs::read_dir(path)
.with_context(|| format!("listing {}", path.display()))?
.filter_map(|entry| {
let entry = entry.ok()?;
// `path().is_file()` follows symlinks (`read_dir`'s file_type doesn't).
entry.path().is_file().then(|| entry.file_name())
})
.filter_map(|name| name.into_string().ok())
.collect();
// read_dir order is arbitrary; sort so case-variant ties resolve deterministically.
names.sort();
for name in mount_order(&names) {
let mpq = path.join(&name);
archives.push(
Archive::open(&mpq).with_context(|| format!("opening {}", mpq.display()))?,
);
}
if archives.is_empty() {
bail!("no known vanilla MPQs found in {}", path.display());
@@ -137,3 +193,77 @@ impl Chain {
Ok(out)
}
}
#[cfg(test)]
mod tests {
use super::*;
fn owned(names: &[&str]) -> Vec<String> {
names.iter().map(|n| n.to_string()).collect()
}
#[test]
fn patch_glob_matches_exactly_one_character_case_insensitively() {
assert!(is_patch_glob_match("patch-2.MPQ"));
assert!(is_patch_glob_match("patch-3.MPQ"));
assert!(is_patch_glob_match("PATCH-A.mpq"));
// Zero or two-plus characters: FindFirstFileW's `?` is exactly one.
assert!(!is_patch_glob_match("patch-.MPQ"));
assert!(!is_patch_glob_match("patch-10.MPQ"));
assert!(!is_patch_glob_match("patch-33.MPQ"));
// Not the glob's shape at all.
assert!(!is_patch_glob_match("patch.MPQ"));
assert!(!is_patch_glob_match("patch-2.MPQ.bak"));
assert!(!is_patch_glob_match("mypatch-2.MPQ"));
}
#[test]
fn mount_order_is_the_carved_law() {
// A shuffled install with a custom patch, plus files the mounter must ignore:
// base.MPQ (telemetry-only in the reference), backup.MPQ, loose non-archives.
let dir = owned(&[
"patch-2.MPQ",
"backup.MPQ",
"model.MPQ",
"base.MPQ",
"dbc.MPQ",
"patch.MPQ",
"eula.html",
"patch-3.MPQ",
"speech2.MPQ",
"texture.MPQ",
]);
assert_eq!(
mount_order(&dir),
owned(&[
"dbc.MPQ",
"texture.MPQ",
"model.MPQ",
"patch.MPQ",
"patch-2.MPQ",
"patch-3.MPQ",
"speech2.MPQ",
])
);
}
#[test]
fn patch_sort_is_ascending_and_case_folded() {
// Later wins in `Chain`, so ascending case-folded order makes patch-3 override patch-2
// and `patch-B` override `patch-a` ('a' < 'b' after the strnicmp-style fold).
let dir = owned(&["patch-B.MPQ", "patch-3.MPQ", "patch-a.MPQ", "patch-2.MPQ"]);
assert_eq!(
mount_order(&dir),
owned(&["patch-2.MPQ", "patch-3.MPQ", "patch-a.MPQ", "patch-B.MPQ"])
);
}
#[test]
fn base_archives_are_found_case_insensitively() {
let dir = owned(&["DBC.mpq", "Model.MPQ", "PATCH.mpq"]);
assert_eq!(
mount_order(&dir),
owned(&["DBC.mpq", "Model.MPQ", "PATCH.mpq"])
);
}
}
+16 -13
View File
@@ -263,27 +263,30 @@ pub use taxi_nodes::{load_taxi_nodes, TaxiNode, TaxiNodes};
mod taxi_path;
pub use taxi_path::{load_taxi_paths, TaxiPath, TaxiPaths};
/// Vanilla 1.12.1 MPQ load order, **lowest priority first**.
/// The ten vanilla base content archives, **lowest priority first** — the reference mounter's
/// table (`0x82e12c`) at its carved fixed priorities (`dbc.MPQ` = 0x36 … `model.MPQ` = 0x3f),
/// reversed so [`Chain`]'s later-wins order reproduces them (decision 1300).
///
/// Later archives override earlier ones for files sharing an internal path. The base
/// content archives hold the bulk of the data but carry **no `(listfile)`**; the master
/// `(listfile)` (and most overrides) live in `patch.MPQ` / `patch-2.MPQ`, which is why the
/// chain — not a single archive — is the correct unit to read from. Verified against the
/// build-5875 client (`patch.MPQ` listfile = 26,350 entries).
pub const VANILLA_LOAD_ORDER: &[&str] = &[
"base.MPQ",
/// This is only the *base* set: `patch.MPQ`, the `patch-?.MPQ` archives, and the optional
/// `speech2.MPQ` are **discovered**, not listed — the whole mount law lives in [`Chain::open`].
/// `base.MPQ` is deliberately absent: the reference opens it once for `telemetry.dat` and closes
/// it before the mounter runs, so its contents are unreachable as assets (wow-re, VERIFIED).
///
/// The base archives hold the bulk of the data but carry **no `(listfile)`**; the master
/// `(listfile)` (and most overrides) live in the patch archives, which is why the chain — not a
/// single archive — is the correct unit to read from. Verified against the build-5875 client
/// (`patch.MPQ` listfile = 26,350 entries).
pub const VANILLA_BASE_ORDER: &[&str] = &[
"dbc.MPQ",
"speech.MPQ",
"fonts.MPQ",
"interface.MPQ",
"misc.MPQ",
"model.MPQ",
"sound.MPQ",
"speech.MPQ",
"wmo.MPQ",
"terrain.MPQ",
"texture.MPQ",
"wmo.MPQ",
"patch.MPQ",
"patch-2.MPQ",
"model.MPQ",
];
/// Open a [`Chain`] from either a single `.MPQ` file or a vanilla `Data` directory. Thin wrapper over
+19 -3
View File
@@ -44,6 +44,11 @@ pub struct ActionSlot {
/// Bag count for an ITEM-kind slot (`GetActionCount`), `0` for every other kind or an empty
/// bag — the app-resolved value the Count fontstring reads (decision 0216 §7).
pub count: u32,
/// `IsConsumableAction`: the gate the ref's `UpdateCount` puts in front of [`Self::count`]
/// (decision 0926 §3). **Identity, not state** — `0x4e5250` reads nothing but the slot's own
/// item template, so it changes exactly when the icon does and rides the same push (decision
/// 1301; it lived in [`ActionState`] until the login race that split the pair).
pub consumable: bool,
}
/// One action's **dynamic** state — the per-frame half the app's feed pushes beside the slot's
@@ -70,8 +75,6 @@ pub struct ActionState {
pub auto_repeat: bool,
/// `IsAttackAction`: the action is the melee auto-attack.
pub is_attack: bool,
/// `IsConsumableAction`: shows the Count fontstring in the ref's `UpdateCount`.
pub consumable: bool,
/// `IsEquippedAction`: an ITEM action currently worn (the green border).
pub equipped: bool,
/// The action's cooldown as pushed: `(start_ms, duration_ms, enabled)` with `start_ms` the
@@ -360,9 +363,19 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
"IsAttackAction",
lua.create_function(|lua, action: u32| Ok(state_flag(lua, action, |s| s.is_attack)))?,
)?;
// The Count gate reads the SLOT, not the state map: `IsConsumableAction 0x4e5250` is a pure
// query over the item template the icon already came from, so it has to arrive on the same
// push the icon does. Split across the two feeds it answered `nil` for the whole session on a
// freshly logged-in character (decision 1301).
g.set(
"IsConsumableAction",
lua.create_function(|lua, action: u32| Ok(state_flag(lua, action, |s| s.consumable)))?,
lua.create_function(|lua, action: u32| {
let model = lua.app_data_ref::<Model>().expect("model app_data");
match model.actions.get(&action) {
Some(s) if s.consumable => Ok(Value::Integer(1)),
_ => Ok(Value::Nil),
}
})?,
)?;
g.set(
"IsEquippedAction",
@@ -413,6 +426,7 @@ mod tests {
kind: 0x00,
action: 133,
count: 0,
consumable: false,
}),
);
s.set_bonus_bar_offset(1);
@@ -443,6 +457,7 @@ mod tests {
kind: 0x80,
action: 117,
count: 4,
consumable: false,
}),
);
assert_eq!(s.eval::<i64>("return GetActionCount(5)").unwrap(), 4);
@@ -507,6 +522,7 @@ mod tests {
kind,
action,
count: 0,
consumable: false,
})
};
s.set_action(1, slot(0x40, 1)); // macro 1 — "Pull"
+6 -1
View File
@@ -134,7 +134,11 @@ pub(crate) fn place_action(model: &mut Model, id: u32) -> bool {
texture,
kind,
action,
count: 0, // the app's next-frame re-feed resolves the real bag count for an ITEM slot
// The engine holds no item knowledge, so neither half of the Count pair can be
// answered here: the app's next-frame re-feed resolves the real bag count and the
// `IsConsumableAction` gate together, off the item template (decision 1301).
count: 0,
consumable: false,
},
);
model.action_sets.push((id, pack(kind, action)));
@@ -185,6 +189,7 @@ mod tests {
kind,
action,
count: 0,
consumable: false,
}
}
@@ -603,6 +603,7 @@ mod tests {
kind: 0,
action: 133,
count: 0,
consumable: false,
}),
);
+1 -2
View File
@@ -157,8 +157,7 @@ pub(super) fn fire_visibility_changes(lua: &Lua, changed: Vec<FrameHandle>) {
if let Err(e) = fire(lua, id, name, None, Vec::new()) {
lua.app_data_mut::<Model>()
.expect("model")
.errors
.push(e.to_string());
.record_script_error(e.to_string());
}
}
}
+77 -4
View File
@@ -209,6 +209,12 @@ const REG_TEXTURE_METHODS: &str = "__benilla_texture_methods";
const REG_TEXTURE_META: &str = "__benilla_texture_meta";
const REG_FONTSTRING_METHODS: &str = "__benilla_fontstring_methods";
const REG_FONTSTRING_META: &str = "__benilla_fontstring_meta";
/// The stdlib's out-of-the-box error handler, kept by identity so
/// [`UiScript::dispatch_script_errors_to_handler`] can tell "nobody chose a handler" (skip — the
/// default already reports into the host channel) from "FrameXML or an addon installed one"
/// (dispatch — that is what the pair exists for). Stored at [`stdlib::install`] time, the one
/// moment the default is known to be what `geterrorhandler()` answers.
const REG_DEFAULT_ERRORHANDLER: &str = "__benilla_default_errorhandler";
/// The **Region method map** (`0xcf54b4`) — the 19 names every region leaf reaches through its own
/// lookup's fallback, carved in wow-re `system/ui/scratch/font-object-lua-surface.md` and asserted
@@ -572,9 +578,12 @@ impl UiScript {
/// hook raises, and the raise propagates like any other Lua error: the addon reports as failed
/// with a distinctive message, and everything after it still runs.
///
/// **This is opt-in and the app does not use it.** A real session must not kill a player's
/// addon for being slow — the budget exists for harnesses, which run 218 strangers' code in one
/// process and cannot afford one of them to be unbounded.
/// **This is opt-in, and the app arms it only on the world-entry load edge** (decision 1306;
/// it began harness-only, e463649e). A real session must not kill a player's addon for being
/// slow, so steady state — every OnUpdate, every event — runs unhooked; but a load walk that
/// never returns is a client frozen on the loading screen with zero diagnostics (B271's
/// class), so the entry edge is bounded and [`Self::clear_instruction_budget`] disarms before
/// the session's first frame.
///
/// The hook fires every [`INSTRUCTION_HOOK_STEP`] instructions rather than every instruction:
/// mlua's own docs warn that a low value "can incur a very high overhead", and the step is the
@@ -603,6 +612,14 @@ impl UiScript {
);
}
/// Remove an installed instruction budget — the load edge's disarm (decision 1306): the bound
/// covers the world-entry walk, and a session's steady state runs unhooked exactly as before.
/// The counter keeps its last value, so [`Self::instructions_used`] still answers for the
/// phase that just ended.
pub fn clear_instruction_budget(&self) {
self.lua.remove_hook();
}
/// VM instructions executed since the last [`Self::set_instruction_budget`], to the resolution
/// of [`INSTRUCTION_HOOK_STEP`]. Zero when no budget was ever set — the hook is what counts.
///
@@ -1105,6 +1122,62 @@ impl UiScript {
std::mem::take(&mut self.model_mut().warnings)
}
/// Report a script error the host caught **outside** the VM's own dispatch — an addon file
/// that failed to compile or raised at file scope during the load walk. It joins the
/// handler-dispatch queue only: the caller has already logged it (the load walk's per-file
/// `error!` + `failures` contract), so putting it in `errors` too would double-log at the
/// app's per-frame drain.
pub fn report_script_error(&self, msg: &str) {
self.model_mut()
.pending_error_dispatch
.push(msg.to_string());
}
/// Hand every queued script error to the Lua-side error handler — the reference's own shape:
/// `seterrorhandler`/`geterrorhandler` are engine globals (wow-re `scratch/lua-dialect.md`,
/// the captured `_G`), the engine invokes the registered handler on a caught script error
/// (that is the pair's contract — a handler nothing invokes would be two dead globals), and
/// FrameXML answers with `_ERRORMESSAGE` → the red ScriptErrors dialog (decision 1305).
///
/// Called at a safe seam (the app's per-frame drain), never from inside the failed call.
/// Three guards keep it bounded and honest:
/// - **The stdlib default handler is skipped by identity.** It reports into
/// [`UiScript::errors`] — where every queued message already is — so dispatching it would
/// only duplicate. The queue still drains, so a handler installed later starts clean.
/// - **A handler that raises is recorded on the host channel only** and never re-queued:
/// the error path cannot recurse by construction.
/// - **One failure stops the batch** — a broken handler fails the same way for every message,
/// and one line names it.
pub fn dispatch_script_errors_to_handler(&mut self) {
let pending = std::mem::take(&mut self.model_mut().pending_error_dispatch);
if pending.is_empty() {
return;
}
let handler: Option<mlua::Function> = self
.lua
.globals()
.get::<mlua::Function>("geterrorhandler")
.ok()
.and_then(|g| g.call::<mlua::Function>(()).ok());
let Some(handler) = handler else { return };
if let Ok(default) = self
.lua
.named_registry_value::<mlua::Function>(REG_DEFAULT_ERRORHANDLER)
{
if handler == default {
return;
}
}
for msg in pending {
if let Err(e) = handler.call::<()>(msg) {
self.model_mut()
.errors
.push(format!("error handler itself failed: {e}"));
break;
}
}
}
/// Register a named virtual [`FontObject`] (a resolved `<Font>`), overwriting any prior one of
/// the same name, **and publish it as the Lua global `name`** — the same pair
/// `Loader::do_font` performs, so a font registered from Rust is addressable from Lua exactly
@@ -1152,7 +1225,7 @@ impl UiScript {
}
fn push_error(&self, e: mlua::Error) {
self.model_mut().errors.push(e.to_string());
self.model_mut().record_script_error(e.to_string());
}
}
+19
View File
@@ -188,6 +188,14 @@ pub(crate) struct Model {
/// Script errors collected from `pcall`'d handlers (never panics, never prints — decision 0068).
pub(crate) errors: Vec<String>,
/// Script errors awaiting dispatch to the **Lua-side** error handler (decision 1305) — the
/// reference calls `geterrorhandler()`'s function on every caught script error, which is how
/// FrameXML's `_ERRORMESSAGE` puts the red ScriptErrors dialog on the player's screen. Every
/// message recorded through [`Model::record_script_error`] lands here as well as in `errors`;
/// [`super::UiScript::dispatch_script_errors_to_handler`] drains it at a safe seam (never from
/// inside the failed call). A message produced *by* the handler itself goes to `errors` only —
/// that asymmetry is the recursion guard.
pub(crate) pending_error_dispatch: Vec<String>,
/// Non-fatal warnings surfaced to the host (e.g. `CreateFrame`'s ignored `inherits=` template).
pub(crate) warnings: Vec<String>,
/// The screen-root rect (`[bottom, left, top, right]`), the anchor base for top-level frames.
@@ -1009,6 +1017,16 @@ impl Model {
}
}
/// Record one script error on **both** channels: the host's `errors` vec (the instruments'
/// channel — the harness blocker tables, the app's log drain) and the handler-dispatch queue
/// (the player's channel — decision 1305). Every engine-caught script error goes through here;
/// a failure raised by the error handler *itself* is pushed to `errors` directly instead,
/// which is what keeps the dispatch from recursing.
pub(crate) fn record_script_error(&mut self, msg: String) {
self.pending_error_dispatch.push(msg.clone());
self.errors.push(msg);
}
pub(crate) fn unit(&self, token: &str) -> Option<&UnitState> {
if token.bytes().any(|b| b.is_ascii_uppercase()) {
self.units_by_lower.get(&token.to_ascii_lowercase())
@@ -1056,6 +1074,7 @@ impl Model {
last_click: HashMap::new(),
pending_size_changed: Vec::new(),
errors: Vec::new(),
pending_error_dispatch: Vec::new(),
warnings: Vec::new(),
// Classic Era's UIParent virtual space is 1024×768-ish; a sensible default the host can
// override with `set_screen_size`. y-up: [bottom, left, top, right].
+1 -1
View File
@@ -63,7 +63,7 @@ impl super::UiScript {
// The reference passes `strsub(text, strlen(cmdString) + 2)` — everything after the alias
// and one space. The host has already made that split, so `args` IS that string.
if let Err(e) = handler.call::<()>(args.to_string()) {
self.model_mut().errors.push(format!("/{cmd}: {e}"));
self.model_mut().record_script_error(format!("/{cmd}: {e}"));
}
true
}
+6
View File
@@ -128,6 +128,12 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
.set_name("=[benilla wow stdlib]")
.set_mode(mlua::ChunkMode::Text)
.exec()?;
// Remember the default handler BY IDENTITY, right after installing it — the engine-side
// dispatch (decision 1305) skips it (it reports into the host channel, where every dispatched
// message already is) and fires only a handler somebody *chose*: FrameXML's `_ERRORMESSAGE`
// or an addon's own.
let default: mlua::Function = lua.load("return geterrorhandler()").eval()?;
lua.set_named_registry_value(super::REG_DEFAULT_ERRORHANDLER, default)?;
Ok(())
}
+15 -1
View File
@@ -83,7 +83,6 @@ fn action_state_bindings_answer_the_reference_conventions() {
current: true,
auto_repeat: false,
is_attack: false,
consumable: true,
equipped: false,
// 4 s remaining of a 10 s cooldown, running: started at GetTime 94.
cooldown: Some((94_000, 10_000, true)),
@@ -100,6 +99,21 @@ fn action_state_bindings_answer_the_reference_conventions() {
assert!(s
.eval::<bool>("return IsAutoRepeatAction(3) == nil")
.unwrap());
// IsConsumableAction reads the SLOT, not this map (decision 1301) — it is a pure query over
// the item template, so it arrives with the icon it gates the count beside.
assert!(s
.eval::<bool>("return IsConsumableAction(3) == nil")
.unwrap());
s.set_action(
3,
Some(ActionSlot {
texture: None,
kind: 0x80,
action: 117,
count: 4,
consumable: true,
}),
);
assert!(s.eval::<bool>("return IsConsumableAction(3) == 1").unwrap());
// GetActionCooldown: the pushed absolute start, verbatim in seconds.
+81
View File
@@ -164,3 +164,84 @@ fn request_time_played_queues_an_ask_and_the_answer_arrives_as_an_event() {
"total seconds played and seconds since the last level-up, in that order"
);
}
/// **The engine hands caught script errors to the CHOSEN Lua error handler** (decision 1305) —
/// the reference's `seterrorhandler` contract, which is how `_ERRORMESSAGE`'s dialog (or an
/// addon's own ImprovedErrorFrame-style handler) ever hears about a failure the engine caught.
#[test]
fn a_chosen_error_handler_hears_engine_caught_errors_and_the_default_does_not_duplicate() {
let mut s = crate::script::UiScript::new().unwrap();
s.set_screen_size(1024.0, 768.0);
s.run(
"local f = CreateFrame('Frame') \
f:RegisterEvent('B271_PROBE') \
f:SetScript('OnEvent', function() error('boom from a handler') end)",
)
.unwrap();
// Nobody chose a handler: the error lands on the host channel ONCE, and the dispatch —
// recognising the stdlib default by identity — adds nothing.
s.fire_event("B271_PROBE", vec![]);
s.dispatch_script_errors_to_handler();
let errors = s.take_errors();
assert_eq!(
errors.iter().filter(|e| e.contains("boom")).count(),
1,
"one error, once — the default handler must not double-report: {errors:?}"
);
// An addon chose one: the dispatch fires it with the message, and the host channel STILL
// records the error (the dual channel is what keeps the harness's instruments sighted).
s.run("caught = {} seterrorhandler(function(msg) table.insert(caught, msg) end)")
.unwrap();
s.fire_event("B271_PROBE", vec![]);
s.dispatch_script_errors_to_handler();
assert!(
s.eval::<String>("return caught[1]")
.unwrap()
.contains("boom from a handler"),
"the chosen handler received the engine-caught error"
);
assert_eq!(
s.take_errors()
.iter()
.filter(|e| e.contains("boom"))
.count(),
1,
"and the host channel recorded it too"
);
}
/// **A handler that itself raises cannot recurse the error path** — its failure is recorded on
/// the host channel only, never re-queued, and the batch stops on the first handler failure.
#[test]
fn an_error_handler_that_errors_does_not_recurse() {
let mut s = crate::script::UiScript::new().unwrap();
s.set_screen_size(1024.0, 768.0);
s.run(
"seterrorhandler(function() error('the handler is broken too') end) \
local f = CreateFrame('Frame') \
f:RegisterEvent('B271_PROBE') \
f:SetScript('OnEvent', function() error('original fault') end)",
)
.unwrap();
s.fire_event("B271_PROBE", vec![]);
s.dispatch_script_errors_to_handler();
let errors = s.take_errors();
assert!(
errors.iter().any(|e| e.contains("original fault")),
"the original fault is on the host channel: {errors:?}"
);
assert!(
errors
.iter()
.any(|e| e.contains("error handler itself failed")),
"and so is the handler's own failure, named as such: {errors:?}"
);
// A second dispatch finds an EMPTY queue — the handler's failure was never re-queued.
s.dispatch_script_errors_to_handler();
assert!(
s.take_errors().is_empty(),
"nothing recurses: the queue drained and the handler failure did not re-enter it"
);
}