grom/benilla-octo
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

the create screen marks your pick, and a new character inherits addons

Browse Source 
- the character-create screen marks the chosen race, gender and class again — a lit square and its name on the icon, as the reference's LockHighlight draws it; a query term had filtered all eighteen icons out since 2072
- a new character inherits the addon enables the realm's other characters agree on, as the reference computes them: a Disable All saved just before no longer comes back all-checked (2311)
- an auction list-update event names one list: a first search on Browse no longer throws the stock Auctions tab's page error, and a sale or outbid notification never fetches a list the window never asked for (2308)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Sam
2026-09-19 17:09:59 +07:00
co-authored by Claude Fable 5.1
parent c79e2e7ca3
commit baf4e37070
12 changed files with 914 additions and 166 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/benilla-app/src/capture/probe_auction.rs
+60 -16
View File
@@ -161,11 +161,20 @@ struct AuctionProbe {
baseline_money: u32,
/// `AUCTION_OWNED_LIST_UPDATE`'s count immediately before `StartAuction` went in.
///
/// Taken THERE and not when step (6) starts, because the STARTED result queues an owner
/// re-query of its own: by the time (6) is reached the event may already have fired, and a
/// baseline taken then would be waiting for a second one that a repeated identical page will
/// never produce.
/// Taken THERE and not when step (6) starts, so that an owner re-query queued by the STARTED
/// result cannot fire the event before the baseline is read — a baseline taken after it would
/// wait for a second event that a repeated identical page will never produce. (Since 2308 that
/// re-query only happens for a list we already hold, and step 6 is what first asks for this
/// one, so the ordering is belt and braces rather than load-bearing.)
owned_event_baseline: i64,
/// How many red `UI_ERROR_MESSAGE` lines the run had already raised when the auction window
/// opened — the login environment's own, not the arc's.
///
/// Step (6d) asserts the arc raised **no** red line, and it used to read the whole tally: on a
/// `probeN` account that is never zero, because the preflight's `GM mode is ON` and
/// `GOD mode is ON` banners (0679) are red lines too, and they land before the greeting. The
/// assertion was therefore FAILING on every clean run, which is worse than not making it.
red_baseline: i64,
passes: u32,
fails: u32,
/// Latched once [`Phase::Done`] has fired its exit (never re-fire on a later frame).
@@ -287,6 +296,13 @@ fn last_error(script: &UiScript) -> String {
///
/// Compared against `getglobal("ERR_AUCTION_STARTED")` rather than against English: the string is
/// the player's own, and this file must not carry a copy of it.
/// Red `UI_ERROR_MESSAGE` lines raised so far, as the hook's own array counts them.
fn red_count(script: &UiScript) -> i64 {
script
.eval::<i64>("return table.getn(ProbeAuctionErrors)")
.unwrap_or(-1)
}
fn started_chat_check(script: &UiScript, probe: &mut AuctionProbe) {
let said = script
.eval::<i64>(
@@ -298,9 +314,7 @@ fn started_chat_check(script: &UiScript, probe: &mut AuctionProbe) {
return 0",
)
.unwrap_or(-2);
let reds = script
.eval::<i64>("return table.getn(ProbeAuctionErrors)")
.unwrap_or(-1);
let reds = red_count(script) - probe.red_baseline;
match (said, reds) {
(1, 0) => {
info!(
@@ -317,13 +331,16 @@ fn started_chat_check(script: &UiScript, probe: &mut AuctionProbe) {
probe.fails += 1;
}
(said, reds) => {
// The reds are listed from the baseline on, for the same reason `reds` counts from
// there: the login environment's own banners are not this arc's output.
let lines = script
.eval::<String>(
"local t = {} \
.eval::<String>(&format!(
"local t = {{}} \
for i = 1, table.getn(ProbeAuctionChat) do table.insert(t, ProbeAuctionChat[i]) end \
for i = 1, table.getn(ProbeAuctionErrors) do table.insert(t, \"RED:\" .. ProbeAuctionErrors[i]) end \
for i = {} + 1, table.getn(ProbeAuctionErrors) do table.insert(t, \"RED:\" .. ProbeAuctionErrors[i]) end \
return table.concat(t, \" | \")",
)
probe.red_baseline.max(0)
))
.unwrap_or_default();
error!(
"PROBE_AUCTION: FAIL (6d chat) — wanted the STARTED line in chat and zero red \
@@ -342,8 +359,15 @@ fn started_chat_check(script: &UiScript, probe: &mut AuctionProbe) {
/// coin. Derived from `probe.min_bid` rather than hardcoded, because the listing price is computed
/// from the item's own vendor value and a fixed expectation would be a lie the day the item changes.
///
/// `IsShown`, not `IsVisible`: the Auctions pane is behind the Browse tab at this point in the run
/// and the rows are painted either way — visibility would test the tab, not the coins.
/// `IsShown`, not `IsVisible`: the coin buttons' own shown-ness is the subject, not whatever the
/// window is doing around them.
///
/// The pane is the *front* tab by the time this runs, and that is load-bearing rather than
/// incidental: the row is painted by `AuctionFrameAuctions_Update`, which cannot complete before
/// the tab's `OnShow` has given it a `page` (decision 2308). This check read the player's purse
/// out of the row's money frame for as long as the probe asked for the owned list without opening
/// the tab — the frame was still on its `MoneyTypeInfo["PLAYER"]` default because nothing had ever
/// repainted it.
fn owner_money_check(script: &UiScript, probe: &mut AuctionProbe) {
let (gold, silver, copper) = (
probe.min_bid / 10_000,
@@ -601,6 +625,9 @@ fn auction_probe(
auction.auctioneer.unwrap_or(0)
);
probe.passes += 1;
// Everything red before this instant is the login environment (the GM/GOD
// banners); the arc's own tally starts here.
probe.red_baseline = red_count(&script);
} else {
error!(
"PROBE_AUCTION: FAIL (2 greet) — the session opened, but on {:?} (we \
@@ -1151,10 +1178,27 @@ fn auction_probe(
probe.phase = cancel_at(now);
return;
}
// Re-ask on a cadence: the server drops a second in-flight list request silently, and
// the STARTED result has already queued a refresh of its own.
// Re-ask on a cadence: the server drops a second in-flight list request silently.
//
// **The FIRST ask is the tab click, not the raw binding** (decision 2308). Every
// caller of `GetOwnerAuctionItems` in the stock files lives inside the Auctions pane —
// its `OnShow` and its two page turners — and the `OnShow` is also the one place that
// assigns `AuctionFrameAuctions.page`. Calling the binding cold puts the window in a
// state no click can reach, and the reply's `AUCTION_OWNED_LIST_UPDATE` then repaints
// a tab whose `page` is still nil: this probe raised
// `Blizzard_AuctionUI.lua:837: attempt to perform arithmetic on field 'page'` twice a
// run doing exactly that, at WARN, where nothing was looking. Clicking the tab is both
// the faithful drive ("exactly as a click would", this file's header) and what makes
// step (6c) a real assertion instead of a permanently red one.
if last_ask == 0.0 || now - last_ask > OWNER_REASK_SECS {
crate::ui_script::run_or_warn(&script, "GetOwnerAuctionItems()");
let ask = if last_ask == 0.0 {
// The tab's own OnShow issues the query, once per window session
// (`AuctionFrame.gotAuctions`), and sets the page the repaint needs.
"AuctionFrameTab_OnClick(3)"
} else {
"GetOwnerAuctionItems()"
};
crate::ui_script::run_or_warn(&script, ask);
probe.phase = Phase::OwnerList {
since,
last_ask: now,
crates/benilla-app/src/char_create/mod.rs
+115 -1
View File
@@ -326,7 +326,7 @@ impl CreateSelection {
}
/// One clickable control on the screen — a single component so one query dispatches every button.
#[derive(Component, Clone, Copy, PartialEq, Eq)]
#[derive(Component, Clone, Copy, PartialEq, Eq, Debug)]
enum CreateAction {
Race(u8),
Gender(u8),
@@ -808,4 +808,118 @@ mod tests {
"the booth look must differ by class, or the preview cannot re-dress"
);
}
/// **The chosen race / gender / class icon is LOCK-HIGHLIGHTED** — the reference's own verb:
/// `SetCharacterRace`, `SetCharacterClass` and `SetCharacterGender` each call
/// `button:LockHighlight()` on the one that is chosen and `UnlockHighlight()` on the rest
/// (`CharacterCreate.lua` l.171/254/326). In 1.12 that lock *is* the whole selected visual:
/// `CharacterCreateIconButtonTemplate` has its `<CheckedTexture>` commented out, leaving the
/// ADD `ButtonHilight-Square` and the HIGHLIGHT-layer `$parentHighlightText` — both of which
/// the lock is what lights.
///
/// **The regression this exists for** (director, 2026-09-17: *"it doesn't show that mage is
/// selected"*): 2072 moved the sheen to one owner and gave this screen's visuals query a
/// `&mut LockHighlight` term — but [`crate::glue::widgets::icon_button`], the only spawn site
/// for all eighteen of these icons, inserted no such component. A `&mut T` term is a filter,
/// so the query matched **nothing**: no selected sheen and no icon name on race, gender or
/// class, for ten days. Every existing sheen test hand-spawned the flag, so the consumer was
/// covered and the producer was not — hence this one builds the icons with the real
/// `icon_button` and runs the real system.
#[test]
fn the_chosen_icons_are_lock_highlighted() {
use crate::glue::art::GlueArt;
use crate::glue::widgets::{icon_button, LockHighlight};
fn spawn_icons(mut commands: Commands, art: Res<GlueArt>) {
let font = Handle::<Font>::default();
commands.spawn(Node::default()).with_children(|p| {
for race in ALLIANCE {
icon_button(
p,
&font,
CreateAction::Race(race),
None::<parts::DynIcon>,
None,
None::<parts::DynText>,
"",
&art,
1.0,
);
}
for sex in 0..2u8 {
icon_button(
p,
&font,
CreateAction::Gender(sex),
None::<parts::DynIcon>,
None,
None::<parts::DynText>,
"",
&art,
1.0,
);
}
for slot in 0..8u8 {
icon_button(
p,
&font,
CreateAction::ClassSlot(slot),
None::<parts::DynIcon>,
None,
None::<parts::DynText>,
"",
&art,
1.0,
);
}
});
}
let mut app = App::new();
app.init_resource::<GlueArt>()
// Human, female, MAGE — the director's own case. With no catalog loaded
// `race_classes` is the full list, so mage (8) is slot 6.
.insert_resource(CreateSelection {
race: 1,
sex: 1,
class: 8,
..default()
})
.add_systems(Startup, spawn_icons)
.add_systems(Update, refresh::refresh_hover);
app.update();
let locked: Vec<CreateAction> = app
.world_mut()
.query::<(&CreateAction, &LockHighlight)>()
.iter(app.world())
.filter(|(_, l)| l.0)
.map(|(a, _)| *a)
.collect();
assert_eq!(
locked.len(),
3,
"exactly one race, one gender and one class icon is locked — got {locked:?}"
);
assert!(locked.contains(&CreateAction::Race(1)), "Human");
assert!(locked.contains(&CreateAction::Gender(1)), "female");
assert!(
locked.contains(&CreateAction::ClassSlot(6)),
"mage is slot 6 of [1,2,3,4,5,7,8,9,11] — and it is the mage icon the director saw \
unmarked"
);
// …and the selection moving takes the lock with it, rather than lighting a second icon.
app.world_mut().resource_mut::<CreateSelection>().class = 1; // warrior, slot 0
app.update();
let locked: Vec<CreateAction> = app
.world_mut()
.query::<(&CreateAction, &LockHighlight)>()
.iter(app.world())
.filter(|(_, l)| l.0)
.map(|(a, _)| *a)
.collect();
assert!(locked.contains(&CreateAction::ClassSlot(0)));
assert!(!locked.contains(&CreateAction::ClassSlot(6)));
}
}
crates/benilla-app/src/char_create/refresh.rs
+22 -10
View File
@@ -219,11 +219,17 @@ pub(super) fn refresh_dynamic(
}
}
/// Per-frame interaction visuals the CREATE screen owns: highlight overlays (hover — and held
/// while selected, the ref's `LockHighlight`), hover labels, and the Create button's disabled
/// latch while a create is in flight. The screen-agnostic passes — up/down art swaps, the glue
/// buttons' art + caption color, outline mirroring — are [`crate::glue`]'s, registered beside
/// this in the plugin chain.
/// Per-frame interaction visuals the CREATE screen owns: which icon is *chosen* (the ref's
/// `LockHighlight`, which [`crate::glue::glue_hilights`] then renders), hover labels, the no-art
/// fallback shade, and the Create button's disabled latch while a create is in flight. The
/// screen-agnostic passes — up/down art swaps, the glue buttons' art + caption color, outline
/// mirroring — are [`crate::glue`]'s, registered beside this in the plugin chain.
///
/// **The chosen-icon write is its own query, deliberately.** It used to be a sixth term on the
/// hover query below, which meant a button without a [`LockHighlight`] dropped out of the hover
/// visuals too — and since no spawn site on this screen had one, that query matched *nothing*:
/// no selected sheen and no icon name anywhere on the create screen. Two jobs, two queries, so a
/// missing component can only ever cost its own job.
#[allow(clippy::type_complexity)]
pub(super) fn refresh_hover(
sel: Res<CreateSelection>,
@@ -238,10 +244,10 @@ pub(super) fn refresh_hover(
&Children,
&mut BackgroundColor,
Has<FallbackFace>,
&mut LockHighlight,
),
(With<Button>, Without<crate::glue::widgets::GlueBtn>),
>,
mut locks: Query<(&CreateAction, &mut LockHighlight)>,
mut labels: Query<&mut Visibility, (With<HoverLabel>, Without<Hilight>)>,
mut disables: Query<(&CreateAction, &mut GlueDisabled)>,
) {
@@ -254,14 +260,20 @@ pub(super) fn refresh_hover(
_ => false,
};
for (action, interaction, children, mut bg, fallback, mut locked) in &mut buttons {
// `SetCharacterRace`/`SetCharacterClass`/`SetCharacterGender`: `LockHighlight()` on the one
// that is chosen, `UnlockHighlight()` on the rest. Nothing about visibility — the sheen is
// `crate::glue::glue_hilights`' alone.
for (action, mut locked) in &mut locks {
let is_sel = selected(action);
let hovered = *interaction != Interaction::None;
let lit = is_sel || hovered;
// The sheen itself is `crate::glue::glue_hilights`' — this says only which row is chosen.
if locked.0 != is_sel {
locked.0 = is_sel;
}
}
for (action, interaction, children, mut bg, fallback) in &mut buttons {
let is_sel = selected(action);
let hovered = *interaction != Interaction::None;
let lit = is_sel || hovered;
// No-art fallback only: buttons spawned with a plain face get a hover shade. (Every node
// *has* a `BackgroundColor` — only a `FallbackFace`'s belongs to us.)
if fallback {
crates/benilla-app/src/char_select/addons.rs
+177 -54
View File
@@ -38,9 +38,12 @@
//!
//! At character select no addon has been *loaded* — `load_third_party` is a world-entry step
//! (1191 §2) — so `GetNumAddOns()` would answer 0. The screen asks
//! [`crate::ui_script::addons::installed`] instead: the same discovery, the same manifest parser,
//! the same enable files. Two views of one folder, never two folders. With the dropdown that read
//! happens **once per roster character at open** ([`AddonsPanel::open_for`]).
//! [`crate::ui_script::addons::installed_rows`] instead, and resolves the enable bits through
//! [`crate::ui_script::addons::EnableStore`] exactly as the world-entry walk does: the same
//! discovery, the same manifest parser, the same store. Two views of one folder, never two
//! folders. Both reads happen **once at open** ([`AddonsPanel::open_for`]) — the folder walk used
//! to run once per roster character, which decision 2311 retired along with the per-character
//! `installed()` it was there to feed.
use bevy::prelude::*;
use bevy::ui_render::ui_material::MaterialNode;
@@ -224,41 +227,34 @@ impl Default for AddonsPanel {
}
impl AddonsPanel {
/// Open for this realm's roster, reading the folder fresh — once per character, so each
/// staged column is that character's own enable file applied to one shared list. The
/// reference's `AddonList_OnShow` also re-reads (`AddonList_Update`), so a folder that
/// changed under us is picked up.
/// Open for this realm's roster: the folder read **once** into one metadata list, and one
/// staged column per character off the enable store. The reference's `AddonList_OnShow` also
/// re-reads (`AddonList_Update`), so a folder that changed under us is picked up.
///
/// **The store is loaded for the whole roster, not per character** (decision 2311). A column
/// is not "that character's file, everything else enabled": an addon the character has no row
/// for takes what the realm's *other* characters agree on, and only falls to the manifest's
/// `## DefaultState` when they disagree or nobody has said anything. Reading it the other way
/// is what made a newly created character re-enable every addon in the All view — its
/// all-enabled column turned every disabled row Mixed, and Mixed reads as on.
pub(super) fn open_for(&mut self, realm: String, chars: Vec<String>) {
self.list.clear();
self.list = addons::installed_rows();
self.staged.clear();
if chars.is_empty() {
// A fresh account: no character means no enable file to key. `None` is the "nobody's
// file" read (everything enabled), staged into one anonymous column so the boxes
// still toggle; Okay has no file to write ([`Self::save_staged`] walks `chars`).
self.list = addons::installed(None);
self.staged
.push(self.list.iter().map(|a| a.enabled).collect());
let store = addons::EnableStore::load(&realm, &chars);
// A fresh account stages one anonymous column: no character means no file to key, and
// Okay has none to write ([`Self::save_staged`] walks `chars`).
let columns: Vec<Option<&str>> = if chars.is_empty() {
vec![None]
} else {
for (c, name) in chars.iter().enumerate() {
let id = (realm.clone(), name.clone());
let rows = addons::installed(Some(&id));
if c == 0 {
self.staged.push(rows.iter().map(|a| a.enabled).collect());
self.list = rows;
} else {
// Same folder, same deterministic discovery order, read back-to-back:
// every call answers the same rows in the same order, only the `enabled` bits
// differ. That is what lets ONE metadata list carry N columns. The order is
// `addons::sort_by_directory_order` — NTFS's, the reference's own listing
// order — and the point here is only that it is a pure function of the names.
debug_assert_eq!(
rows.len(),
self.list.len(),
"installed() must be deterministic across per-character reads"
);
self.staged.push(rows.iter().map(|a| a.enabled).collect());
}
}
chars.iter().map(|c| Some(c.as_str())).collect()
};
for character in columns {
self.staged.push(
self.list
.iter()
.map(|a| store.enabled_for(&a.name, a.default_state, character))
.collect(),
);
}
self.baseline = self.staged.clone();
self.realm = realm;
@@ -316,7 +312,7 @@ impl AddonsPanel {
/// actually matters.
pub(super) fn any_installed() -> bool {
static ANY: std::sync::OnceLock<bool> = std::sync::OnceLock::new();
*ANY.get_or_init(|| !addons::installed(None).is_empty())
*ANY.get_or_init(|| !addons::installed_rows().is_empty())
}
/// The rows currently visible, as `(index, addon)` — enable state is the view's to answer
@@ -392,10 +388,12 @@ impl AddonsPanel {
}
/// Is this addon enabled *for the current view* — the bit the gate is fed? A single character
/// reads their own column. The All view answers **"any staged character has it on"** — OURS
/// (1293): the reference feeds `AddOn_CanLoad` a NULL character there and that read's exact
/// semantics are un-carved, so we state the natural reading rather than guess at bytes nobody
/// has verified.
/// reads their own column. The All view answers **"any staged character has it on"**, which
/// 1293 stated as the natural reading of an un-carved mechanism and which the bytes have
/// since confirmed: `AddonList_Update` computes `enabled = (checkboxState > 0)` off
/// `GetAddOnEnableState(nil, i)`, whose `1` (enabled for some) therefore counts as on
/// (wow-5875-re `addon-enable-store.md` §4). What 1293 got wrong was not this fold but what
/// each column holds — see [`Self::open_for`] and decision 2311.
fn effective_enabled(&self, i: usize) -> bool {
self.box_state(i) != BoxState::Off
}
@@ -1712,7 +1710,7 @@ mod tests {
dependencies: deps.iter().map(|d| (*d).to_string()).collect(),
interface: iface,
load_on_demand: false,
enabled: true,
default_state: true,
}
}
@@ -1721,7 +1719,7 @@ mod tests {
/// `p.view = Some(c)` themselves.
fn panel_for(chars: usize, list: Vec<InstalledAddOn>) -> AddonsPanel {
let staged: Vec<Vec<bool>> = (0..chars.max(1))
.map(|_| list.iter().map(|a| a.enabled).collect())
.map(|_| list.iter().map(|a| a.default_state).collect())
.collect();
AddonsPanel {
open: true,
@@ -1934,7 +1932,7 @@ mod tests {
assert_eq!(p.staged, vec![vec![true, true]]);
p.staged[0][0] = false;
assert!(
p.list[0].enabled,
p.list[0].default_state,
"the read-in list is never mutated — only `staged` is, which is what makes Cancel free"
);
p.close();
@@ -1982,27 +1980,152 @@ mod tests {
p.staged[1][1] = false;
p.save_staged();
// Read back through the same folder view the world-entry walk uses — the two views of
// one folder that 1197 §3 demands can never disagree.
let alice = ("TestRealm".to_string(), "Alice".to_string());
let bob = ("TestRealm".to_string(), "Bob".to_string());
let carol = ("TestRealm".to_string(), "Carol".to_string());
let read = |id: &(String, String)| -> Vec<bool> {
addons::installed(Some(id))
.iter()
.map(|a| a.enabled)
// Read back through the same store the world-entry walk resolves against — the two views
// of one folder that 1197 §3 demands can never disagree.
let roster = ["Alice".to_string(), "Bob".to_string(), "Carol".to_string()];
let store = addons::EnableStore::load("TestRealm", &roster);
let rows = addons::installed_rows();
let read = |who: &str| -> Vec<bool> {
rows.iter()
.map(|a| store.enabled_for(&a.name, a.default_state, Some(who)))
.collect()
};
assert_eq!(
read(&alice),
read("Alice"),
vec![false, true],
"Alice's file carries HER edit"
);
assert_eq!(read(&bob), vec![true, false], "Bob's carries HIS");
assert_eq!(read("Bob"), vec![true, false], "Bob's carries HIS");
let carol = ("TestRealm".to_string(), "Carol".to_string());
assert!(
!addons::enable_state_path(Some(&carol)).unwrap().exists(),
"an unchanged column writes no file — only the diffs against the open-time baseline"
);
// And Carol, who has no file at all, is NOT "everything enabled": she takes what the
// others agree on, which here is nothing — Alpha and Beta are each on for one and off
// for the other — so both fall to their manifests' `## DefaultState` (2311).
assert_eq!(read("Carol"), vec![true, true]);
let _ = std::fs::remove_dir_all(&tmp);
}
/// **Creating a character does not re-enable everything** — the director's report,
/// 2026-09-17: *"after creating a new char all addons are enabled again, even after disabling
/// all of them just before"*.
///
/// The walk is theirs, verbatim: Disable All in the All view, Okay, create a character, open
/// the list again. What used to happen is the whole bug in one line — the new character had
/// no `AddOns.txt`, benilla read an absent row as **enabled**, and that all-true column turned
/// every row in the All view from Off to Mixed, which paints a check and reads as on
/// (`effective_enabled`). One click there would then have written the all-enabled state back
/// onto every other character's file: a one-way ratchet.
///
/// The reference does not do this and the mechanism is carved (wow-5875-re
/// `addon-enable-store.md` §4): the enable query counts only nodes with an **explicit** entry,
/// so a character with no file contributes no opinion and inherits the aggregate — here, the
/// unanimous `disabled` the player just saved. Decision 2311.
#[test]
fn a_new_character_inherits_the_disable_it_did_not_ask_for() {
let _l = crate::local_state::test_env::ENV_LOCK
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let tmp =
std::env::temp_dir().join(format!("benilla-charsel-newchar-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&tmp);
let home = tmp.join("benilla-config");
for name in ["Alpha", "Beta"] {
let dir = home.join("AddOns").join(name);
std::fs::create_dir_all(&dir).unwrap();
std::fs::write(dir.join(format!("{name}.toc")), "## Interface: 11200\n").unwrap();
}
let _c = crate::local_state::test_env::EnvGuard::unset("WOW_CAPTURE");
let _h =
crate::local_state::test_env::EnvGuard::set("BENILLA_HOME", home.to_str().unwrap());
// The roster before the create, and the player's Disable All + Okay over it.
let mut p = AddonsPanel::default();
p.open_for(
"TestRealm".into(),
vec!["Onemage".into(), "Onerogue".into()],
);
p.set_all(false);
p.save_staged();
p.close();
// …then a character is created, so the roster the panel opens over has one more name and
// that name has no enable file at all.
p.open_for(
"TestRealm".into(),
vec!["Onemage".into(), "Onerogue".into(), "Freshling".into()],
);
assert!(p.view.is_none(), "reopens on All, the reference's default");
assert_eq!(p.staged.len(), 3);
assert_eq!(
p.staged[2],
vec![false, false],
"the new character inherits the unanimous disable, not a blank slate"
);
for i in 0..p.list.len() {
assert_eq!(
p.box_state(i),
BoxState::Off,
"row {i} reads OFF in the All view — Mixed here is the bug, and it paints a check"
);
assert!(!p.effective_enabled(i), "…so nothing is loadable either");
}
let _ = std::fs::remove_dir_all(&tmp);
}
/// The other side of the same law: a new character does **not** inherit a disable the roster
/// never agreed on. One character off, one on, and the newcomer takes the manifest's
/// `## DefaultState` — so this is not "new characters copy whoever is strictest".
#[test]
fn a_new_character_inherits_nothing_when_the_roster_disagrees() {
let _l = crate::local_state::test_env::ENV_LOCK
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let tmp =
std::env::temp_dir().join(format!("benilla-charsel-split-{}", std::process::id()));
let _ = std::fs::remove_dir_all(&tmp);
let home = tmp.join("benilla-config");
for name in ["Alpha", "Beta"] {
let dir = home.join("AddOns").join(name);
std::fs::create_dir_all(&dir).unwrap();
std::fs::write(
dir.join(format!("{name}.toc")),
// Beta ships `## DefaultState: disabled`, so the tie-break is visible.
if name == "Beta" {
"## Interface: 11200\n## DefaultState: disabled\n"
} else {
"## Interface: 11200\n"
},
)
.unwrap();
}
let _c = crate::local_state::test_env::EnvGuard::unset("WOW_CAPTURE");
let _h =
crate::local_state::test_env::EnvGuard::set("BENILLA_HOME", home.to_str().unwrap());
let mut p = AddonsPanel::default();
p.open_for(
"TestRealm".into(),
vec!["Onemage".into(), "Onerogue".into()],
);
p.view = Some(0);
p.set_all(false); // the mage turns both off; the rogue keeps the defaults
p.view = Some(1);
p.set_all(true);
p.save_staged();
p.close();
p.open_for(
"TestRealm".into(),
vec!["Onemage".into(), "Onerogue".into(), "Freshling".into()],
);
assert_eq!(
p.staged[2],
vec![true, false],
"split roster → each row falls to its own `## DefaultState`"
);
let _ = std::fs::remove_dir_all(&tmp);
}
crates/benilla-app/src/glue/widgets.rs
+8
View File
@@ -353,6 +353,13 @@ fn outlined_spans<W: Bundle, T: Bundle>(
/// visual), and the name label along the bottom (the ref's `HighlightText`, `GlueFontNormalSmall`,
/// anchored BOTTOM +1 — over the icon's bottom edge). `dyn_icon`/`label_dyn` are the screen's
/// refresh markers, spawned onto the face / real label text.
///
/// **It carries its own [`LockHighlight`]**, because the reference does: these are the
/// `CheckButton`s that `SetCharacterRace`/`SetCharacterClass`/`SetCharacterGender` lock and
/// unlock by hand (`CharacterCreate.lua` l.171/254/326). Leaving the flag to the screen is how
/// the create screen lost every selected sheen *and* every icon name for ten days — 2072 added a
/// `&mut LockHighlight` term to the screen's own visuals query and this spawn site had none, so
/// the query matched nothing at all.
pub(crate) fn icon_button<A: Component, I: Bundle, L: Bundle>(
parent: &mut ChildSpawnerCommands,
font: &Handle<Font>,
@@ -368,6 +375,7 @@ pub(crate) fn icon_button<A: Component, I: Bundle, L: Bundle>(
let mut b = parent.spawn((
action,
Button,
LockHighlight::default(),
Node {
width: px(48.0),
height: px(48.0),
crates/benilla-app/src/ui_auction/mod.rs
+129 -30
View File
@@ -10,9 +10,15 @@
//! decorative: it keys the deposit rate the sell pane displays, and the six faction houses charge
//! 5% where the neutral goblin house charges 25%.
//!
//! **Three lists, one session.** Browse (`"list"`), Bids (`"bidder"`) and Auctions (`"owner"`) are
//! three independent server queries into one window. Each holds at most one 50-row page — the
//! server's own cap — plus the pre-cap match count the pager needs, plus its own sort stack.
//! **Three lists, one session — and three separate events.** Browse (`"list"`), Bids (`"bidder"`)
//! and Auctions (`"owner"`) are three independent server queries into one window. Each holds at
//! most one 50-row page — the server's own cap — plus the pre-cap match count the pager needs,
//! plus its own sort stack, plus its own update event. The reference keeps them just as far apart
//! (wow-re's auction node §1: three arrays, three counts, three sort stacks, and a fire-site
//! census in which every `AUCTION_*_LIST_UPDATE` is fired by whatever just touched *its* array).
//! So a list-update event here is a statement about **one** list; firing the other two runs the
//! stock addon's other tabs over state they were never given, and the Auctions tab answers that
//! by crashing (decision 2308).
//!
//! **Sorting is ours and paging is the server's**, which is the split that shapes this module: a
//! header click re-orders rows we already hold and sends nothing ([`sort`]), while a page turn
@@ -164,6 +170,11 @@ pub(crate) struct AuctionListSlot {
pub(crate) entries: Vec<AuctionListEntry>,
/// `totalCount` — the pre-cap match count, which is what tells the pager there is a page 2.
pub(crate) total: u32,
/// A result for **this** list has landed since the window opened — the interface asked for it
/// and the server answered. `false` is "we hold no such list", which is *not* "the list is
/// empty": an empty page still arrives, and still counts. It is what the server-driven
/// refreshes gate on (decision 2308).
received: bool,
sort: SortStack,
}
@@ -209,15 +220,23 @@ pub(crate) struct AuctionOpen {
show_requested: bool,
/// Fire `NEW_AUCTION_UPDATE` next feed — the sell slot changed.
sell_slot_dirty: bool,
/// A list result landed, so the list events are owed **whether or not the snapshot changed**.
/// A list result landed **for that list**, so *its* event is owed whether or not the snapshot
/// changed. Indexed [`LIST`]/[`BIDDER`]/[`OWNER`].
///
/// Diffing the snapshot is not enough and the live probe is what proved it: an empty auction
/// house, or re-running the same search, produces a result identical to what we already hold.
/// The window's Browse pane clears its "Searching…" state only on `AUCTION_ITEM_LIST_UPDATE`,
/// so on an empty server a search animated its dots forever and never reported a result. The
/// reference has no diff here at all — one routine invalidates all three lists and fires all
/// three events.
list_result_landed: bool,
/// so on an empty server a search animated its dots forever and never reported a result.
///
/// **Per list — that is the whole point** (decision 2308). This was one flag driving all three
/// fires, on 1511's reading that "one routine invalidates all three lists". The reference has
/// no such routine: three arrays, three counts, three sort stacks, **three separate events**,
/// each fired by whatever just touched *its* array. Firing the other two crashed the stock
/// `Blizzard_AuctionUI` outright — `AuctionFrameAuctions_Update` multiplies by
/// `AuctionFrameAuctions.page`, which the addon initialises only in that tab's `OnShow`, so a
/// browse result arriving while the player had never opened the Auctions tab raised
/// `attempt to perform arithmetic on field 'page' (a nil value)`.
list_result_landed: [bool; 3],
/// Empty the sell slot next feed — a listing was accepted, so the staged item is *gone*.
///
/// Also from the probe: without this the pane kept painting a phantom stack after a successful
@@ -304,10 +323,11 @@ impl AuctionOpen {
self.show_requested = true;
}
/// Replace one list's page. Always owes the list events, even for an identical page.
/// Replace one list's page. Always owes **that list's** event, even for an identical page.
pub(crate) fn set_list(&mut self, which: usize, entries: Vec<AuctionListEntry>, total: u32) {
self.list_result_landed = true;
self.list_result_landed[which] = true;
let slot = &mut self.lists[which];
slot.received = true;
slot.entries = entries;
// A server that reports fewer total matches than it just sent us is not a case worth
// trusting over our own eyes; the pager reads the larger of the two.
@@ -329,13 +349,20 @@ impl AuctionOpen {
}
/// Mark our own listings stale — the drain re-queries next frame.
///
/// **Only a list we hold can be stale** (decision 2308). A notification says "the page you are
/// showing is now wrong"; if the interface has never asked for the owned list, there is no
/// such page, and re-asking would *introduce* one — landing an `AUCTION_OWNED_LIST_UPDATE` on
/// a tab whose `page` field the stock addon has not initialised yet. The reference cannot
/// reach that state at all: its notification handlers patch the cached row in place, so an
/// array it never fetched has nothing to patch and announces nothing.
pub(crate) fn refresh_owner(&mut self) {
self.pending_owner_refresh = true;
self.pending_owner_refresh |= self.lists[OWNER].received;
}
/// Mark the bids we hold stale.
/// Mark the bids we hold stale — under the same rule as [`Self::refresh_owner`].
pub(crate) fn refresh_bidder(&mut self) {
self.pending_bidder_refresh = true;
self.pending_bidder_refresh |= self.lists[BIDDER].received;
}
/// Close the window (a client-side close — vanilla sends nothing).
@@ -348,7 +375,7 @@ impl AuctionOpen {
self.query_gate = None;
self.pending_owner_refresh = false;
self.pending_bidder_refresh = false;
self.list_result_landed = false;
self.list_result_landed = [false; 3];
self.sell_slot_taken = false;
}
@@ -680,8 +707,16 @@ fn feed_auction(
let opened = last_open.is_none() && auction.auctioneer.is_some();
let closed = last_open.is_some() && auction.auctioneer.is_none();
let show_requested = std::mem::take(&mut auction.show_requested);
let result_landed = std::mem::take(&mut auction.list_result_landed);
let landed = std::mem::take(&mut auction.list_result_landed);
let changed = fresh != *last;
// **Which events are owed, per list.** A landed result owes its own list's event outright; a
// changed list owes it because an async name or template just filled one of its rows in. The
// diff is per list rather than over the whole snapshot for the same reason the fires are
// (2308): an event names one list, and firing the other two runs the stock Lua's other tabs
// over state they were never given.
let owed: [bool; 3] = std::array::from_fn(|i| {
landed[i] || fresh.as_ref().map(|s| &s.lists[i]) != last.as_ref().map(|s| &s.lists[i])
});
if changed {
script.set_auction(fresh.clone());
}
@@ -694,19 +729,18 @@ fn feed_auction(
if show_requested {
script.fire_event("AUCTION_HOUSE_SHOW", vec![]);
}
// A landed result owes the events outright; a change owes them because an async name or
// template just filled a row in. Diffing ALONE was the bug: an empty house and a repeated
// search both produce a result identical to what we hold, and the Browse pane waits on
// this event to stop saying "Searching…".
if changed || result_landed {
// One routine invalidates all three lists in the reference too — the three events fire
// together rather than being diffed apart, and each tab's handler repaints only if it
// is the visible one.
// Three literal fires, not a loop over names: the producer tripwire
// (`reference_ui::every_event_a_chain_file_registers_has_a_producer`) reads fire
// sites by their literal, and the stock addon registers all three (1971).
// Three literal fires, not a loop over names: the producer tripwire
// (`reference_ui::every_event_a_chain_file_registers_has_a_producer`) reads fire sites by
// their literal, and the stock addon registers all three (1971). Browse first, then Bids,
// then Auctions — the array's own order; the reference never fires two in one pass either,
// so nothing rests on it.
if owed[LIST] {
script.fire_event("AUCTION_ITEM_LIST_UPDATE", vec![]);
}
if owed[BIDDER] {
script.fire_event("AUCTION_BIDDER_LIST_UPDATE", vec![]);
}
if owed[OWNER] {
script.fire_event("AUCTION_OWNED_LIST_UPDATE", vec![]);
}
}
@@ -926,7 +960,7 @@ mod tests {
);
}
/// An identical list result still owes its events. Diffing the snapshot was the bug: an empty
/// An identical list result still owes its event. Diffing the snapshot was the bug: an empty
/// auction house and a repeated search both produce a page identical to the one we hold, and
/// the Browse pane clears its "Searching…" state only when the event arrives — so on an empty
/// server a search animated forever and never reported a result. Found by the live probe,
@@ -937,19 +971,84 @@ mod tests {
open.open(0x1234, 1);
open.set_list(LIST, Vec::new(), 0);
assert!(open.list_result_landed, "the first empty page");
assert!(open.list_result_landed[LIST], "the first empty page");
// The feed consumes the flag when it fires.
open.list_result_landed = false;
open.list_result_landed = [false; 3];
// The very same empty page again — nothing to diff, and still owed.
open.set_list(LIST, Vec::new(), 0);
assert!(
open.list_result_landed,
open.list_result_landed[LIST],
"an identical page owes the event too — this is the whole bug"
);
}
/// **A result owes its OWN list's event and no other** (decision 2308).
///
/// The regression this guards is a crash, not a repaint: the stock `Blizzard_AuctionUI`'s
/// `AuctionFrameAuctions_Update` multiplies by `AuctionFrameAuctions.page`, and that field is
/// initialised only in the Auctions tab's `OnShow`. A browse result that also fired
/// `AUCTION_OWNED_LIST_UPDATE` therefore ran that tab's repaint before it had ever been
/// shown — `attempt to perform arithmetic on field 'page' (a nil value)`, on screen, on the
/// first search of the session. `auction_frame.rs` holds the Lua half of this.
#[test]
fn a_result_owes_only_its_own_lists_event() {
let mut open = AuctionOpen::default();
open.open(0x1234, 1);
open.set_list(LIST, Vec::new(), 0);
assert_eq!(
open.list_result_landed,
[true, false, false],
"a browse result is not news about the player's bids or listings"
);
open.list_result_landed = [false; 3];
open.set_list(OWNER, Vec::new(), 0);
assert_eq!(open.list_result_landed, [false, false, true]);
open.list_result_landed = [false; 3];
open.set_list(BIDDER, Vec::new(), 0);
assert_eq!(open.list_result_landed, [false, true, false]);
}
/// **A refresh can only re-ask for a list we hold** (decision 2308) — the other half of the
/// same crash.
///
/// "Your auction sold" arrives whenever the server feels like it, including while the player
/// is standing on the Browse tab having never opened the Auctions one. Turning that into a
/// fresh owner query would land an `AUCTION_OWNED_LIST_UPDATE` on that never-shown tab — the
/// nil `page` again, by a slower road. The reference cannot get there: its notification
/// handlers patch the cached row, and an array it never fetched has no row to patch.
#[test]
fn a_refresh_never_introduces_a_list_the_interface_never_asked_for() {
let mut open = AuctionOpen::default();
open.open(0x1234, 1);
open.refresh_owner();
open.refresh_bidder();
assert!(
!open.pending_owner_refresh && !open.pending_bidder_refresh,
"we hold neither list, so neither can have gone stale"
);
// The Auctions tab has now been shown once and the server answered — even with nothing in
// it. From here a sale really does invalidate what the player is looking at.
open.set_list(OWNER, Vec::new(), 0);
open.refresh_owner();
assert!(open.pending_owner_refresh, "a list we hold can go stale");
assert!(
!open.pending_bidder_refresh,
"and the other one still cannot"
);
// Closing forgets all of it — a notice landing after the window is gone re-asks nothing.
open.clear();
open.refresh_owner();
assert!(!open.pending_owner_refresh);
}
/// An accepted listing releases the sell slot. Without it the pane kept painting a phantom
/// stack, Create Auction stayed enabled, and a second click re-resolved the remembered
/// (bag, slot) at send time — auctioning whatever had since landed in that slot.
crates/benilla-app/src/ui_auction/net.rs
+27 -5
View File
@@ -183,11 +183,33 @@ fn auction_command_result(
// tell "the server said STARTED/OK" from "the server said nothing at all".
auction.wire.last_command = Some((auction_id, action, error));
if error == auction_error::OK {
// A successful sell/cancel/bid changes a list we are showing. The reference re-queries
// rather than patching its local copy, and so do we — the server is the only thing that
// knows what the page looks like now. Each also says so, in **chat** — the success arm
// shows `0x178`/`0x179`/`0x17f` keyed on the action field, with zero varargs
// (wow-re §11.2/§11.3).
// A successful sell/cancel/bid changes a list we are showing, so we re-ask: the server is
// the only thing that knows what the page looks like now, and the result carries an
// auction id, not a row.
//
// **The reference re-asks on two of these three, and neither ask is page 0** — VERIFIED
// (wow-re auction node §12: a closed caller census of both list senders, two callers each
// and zero address-takes; decision 2308):
//
// - `STARTED` — `[0xb7263c] = 1`, then `0x4cc4e0 call 0x4cd680` = `CMSG 0x259` at the
// **saved page offset** `[0xb72650]`. No row patch, no event of its own.
// - `BID_PLACED` — drops the id from the outbid list, `[0xb72640] = 1`, then
// `0x4cc528 call 0x4cd720` = `CMSG 0x264` at `[0xb72654]` — and *then* patches the
// browse row optimistically (bid ← the amount we sent, high bidder ← our own guid,
// deadline ← `max(deadline, now + 90 s)`) and fires 424.
// - `REMOVED` — **no query at all**: `0x4cc658 call 0x4cdfe0` deletes the id from all
// three arrays locally, each list firing only if it actually lost a row.
//
// We re-ask on all three, always at page 0, and patch nothing. Two live differences fall
// out: a player on page 2 of their own auctions is thrown back to page 1 by a sale, and
// their own bid does not appear on the row until the next result lands. Both belong to the
// freshness-model slice 2308 leaves open, not to this arm. What 2308 *does* enforce here
// is that a re-ask may never **introduce** a list the interface never asked for — the
// reference cannot, because its notification handlers patch instead of asking
// (`AuctionOpen::refresh_owner`).
//
// Each success also says so, in **chat** — the success arm shows `0x178`/`0x179`/`0x17f`
// keyed on the action field, with zero varargs (wow-re §11.2/§11.3).
match action {
auction_action::STARTED => {
// The item is gone from the bag; the sell slot must stop claiming to hold it.
crates/benilla-app/src/ui_script/addons.rs
+223 -47
View File
@@ -47,7 +47,7 @@
//! key-binding table the Key Bindings window edits, so an addon's binding is a row like any
//! other; the app's dispatch runs its Lua body ([`crate::bindings`]).
use std::collections::HashSet;
use std::collections::{HashMap, HashSet};
use std::path::{Path, PathBuf};
use bevy::prelude::*;
@@ -781,8 +781,14 @@ pub(crate) struct InstalledAddOn {
/// LoadOnDemand addon is not "off", it is waiting for a `LoadAddOn` call (1191 §6).
/// Read by the char-select AddOns screen's rows (`char_select::addons`).
pub(crate) load_on_demand: bool,
/// From this character's enable file; an addon the file never mentions is **enabled** (1191 §7).
pub(crate) enabled: bool,
/// `## DefaultState:` — what this addon is for a character with no opinion, and the tie-break
/// when the realm's characters disagree.
///
/// **This row carries no enable bit**, deliberately (decision 2311). It used to, read out of
/// one character's file with "absent means enabled" baked in — which is the bug: an absent
/// row is not enabled, it is [`EnableStore::enabled_for`]'s question, and the store needs the
/// realm's whole character list to answer it. Metadata here, state there.
pub(crate) default_state: bool,
}
impl InstalledAddOn {
@@ -814,14 +820,13 @@ impl InstalledAddOn {
/// The array's *membership* needs nothing here: it excludes the addons the server hid, which on a
/// stock install is Blizzard's twelve, and this walk is the player's own folder only — they were
/// never in it.
pub(crate) fn installed(identity: Option<&(String, String)>) -> Vec<InstalledAddOn> {
let disabled = disabled_set(identity);
pub(crate) fn installed_rows() -> Vec<InstalledAddOn> {
// The player's own folder only: the glue's list is what a player can toggle, and the
// reference's Blizzard addons are not in it.
let mut rows: Vec<InstalledAddOn> = discover_folder()
.into_iter()
.map(|addon| InstalledAddOn {
enabled: !disabled.contains(&addon.name.to_ascii_lowercase()),
default_state: addon.toc.default_state(),
title: addon.toc.directive("Title").map(str::to_owned),
notes: addon.toc.directive("Notes").map(str::to_owned),
url: addon.toc.directive("URL").map(str::to_owned),
@@ -843,18 +848,115 @@ pub(crate) fn installed(identity: Option<&(String, String)>) -> Vec<InstalledAdd
rows
}
/// The lowercased names this character has turned off.
fn disabled_set(identity: Option<&(String, String)>) -> HashSet<String> {
enable_state_path(identity)
.and_then(|p| std::fs::read(p).ok())
.map(|b| {
parse_enable_state(&benilla_ui::source::decode(&b))
.into_iter()
.filter(|(_, on)| !on)
.map(|(n, _)| n.to_ascii_lowercase())
.collect()
})
.unwrap_or_default()
// ── The enable store — the reference's `ADDONSTATELIST` ──────────────────────────────────────────
/// **One node per character on the realm's character list, each holding the explicit rows that
/// character's `AddOns.txt` carried** — the reference's `ADDONSTATELIST` (anchor `0xbe1bd0`, head
/// `ds:0xbe1bd8`), which `AddOnList_LoadCharacter 0x51ebe0` fills one node per character at
/// char-list population (wow-5875-re `system/ui/scratch/addon-enable-store.md` §1/§5).
///
/// **A node with no file is EMPTY, not absent**, and the difference is the whole point: an empty
/// node contributes no opinion to the aggregate, but it is still a character whose enable bit has
/// to be answered — and the reference answers it from the *other* characters, never with a bare
/// "enabled". That was the bug this type was written for (decision 2311): benilla resolved an
/// absent row as enabled, so creating a character re-enabled every addon the player had just
/// turned off, on every character at once.
#[derive(Default)]
pub(crate) struct EnableStore {
/// `(character, explicit rows)` in character-list order; the map is the node's enable hash,
/// keyed lowercased because every compare in the reference is `SStrCmpI`.
nodes: Vec<(String, HashMap<String, bool>)>,
}
impl EnableStore {
/// Load a node per character, each from its own `AddOns.txt`
/// (`benilla-config/addons/<Realm>-<Char>.txt`, decision 1191 §7). A character with no file
/// still gets its node.
pub(crate) fn load(realm: &str, characters: &[String]) -> Self {
let nodes = characters
.iter()
.map(|character| {
let id = (realm.to_string(), character.clone());
let rows = enable_state_path(Some(&id))
.and_then(|p| std::fs::read(p).ok())
.map(|b| parse_enable_state(&benilla_ui::source::decode(&b)))
.unwrap_or_default();
let hash = rows
.into_iter()
// Last line wins, like the reference's hash insert (§5's duplicate law).
.map(|(name, on)| (name.to_ascii_lowercase(), on))
.collect();
(character.clone(), hash)
})
.collect();
Self { nodes }
}
/// `0x51e470(addon, NULL, useDefault = 0)` — the **explicit-only** aggregate. A node with no
/// entry for this addon is not counted at all (`0x51e5df je 0x51e60a`).
///
/// `Some(v)` is the reference's 2/0 (every counted node agrees on `v`); `None` folds its two
/// *undecided* returns together — the mixed `1` and the `total == 0` epilogue — because both
/// resolve the same way one level up, through `DefaultState`.
fn aggregate(&self, addon: &str) -> Option<bool> {
let key = addon.to_ascii_lowercase();
let mut total = 0usize;
let mut on = 0usize;
for (_, hash) in &self.nodes {
if let Some(&v) = hash.get(&key) {
total += 1;
on += usize::from(v);
}
}
match (total, on) {
(0, _) => None,
(_, 0) => Some(false),
(t, o) if o == t => Some(true),
_ => None,
}
}
/// **The bit a character actually gets** — `0x51e470(addon, character, useDefault = 1)`
/// lowered to a bool (a single-character query returns only 0 or 2, §4).
///
/// Their own explicit row when their file has one; otherwise the aggregate above, and where
/// that is undecided, the addon's `## DefaultState`. `None` is the no-character case (nobody
/// picked yet), which is the aggregate alone.
pub(crate) fn enabled_for(
&self,
addon: &str,
default_state: bool,
character: Option<&str>,
) -> bool {
let key = addon.to_ascii_lowercase();
if let Some(explicit) = character
.and_then(|c| self.node(c))
.and_then(|h| h.get(&key))
{
return *explicit;
}
self.aggregate(addon).unwrap_or(default_state)
}
fn node(&self, character: &str) -> Option<&HashMap<String, bool>> {
self.nodes
.iter()
.find(|(name, _)| name.eq_ignore_ascii_case(character))
.map(|(_, hash)| hash)
}
}
/// The store's node set for a load: the realm's character list, with the loading character
/// appended when the caller had no list to give (a test, a capture, a `.go`-style entry) — so a
/// one-identity caller still gets its own file honoured, and nothing else.
fn store_nodes(identity: Option<&(String, String)>, roster: &[String]) -> Vec<String> {
let mut names = roster.to_vec();
if let Some((_, character)) = identity {
if !names.iter().any(|n| n.eq_ignore_ascii_case(character)) {
names.push(character.clone());
}
}
names
}
/// Write a character's enable state — the AddOns screen's write (decision 1197) and, since 2139,
@@ -967,6 +1069,7 @@ const SAVED_HEADER: &str = "\
pub(super) fn load_third_party(
script: &mut UiScript,
identity: Option<&(String, String)>,
roster: &[String],
version_check: bool,
) -> Vec<String> {
let addons = discover();
@@ -976,15 +1079,19 @@ pub(super) fn load_third_party(
let mut infos: Vec<_> = addons.iter().map(info_for).collect();
// The chain-sourced rows read through the reference's own store (1957).
script.set_addon_chain_reader(Box::new(super::reference_ui::read));
if let Some(text) = enable_state_path(identity).and_then(|p| std::fs::read_to_string(p).ok()) {
for (name, enabled) in parse_enable_state(&text) {
if let Some(i) = infos
.iter_mut()
.find(|i| i.name.eq_ignore_ascii_case(&name))
{
i.enabled = enabled;
}
}
// Every addon's enable bit, through the reference's own query (decision 2311): this
// character's explicit `AddOns.txt` row when they have one, else what the realm's other
// characters agree on, else the manifest's `## DefaultState`. Reading an absent row as a
// bare "enabled" is what re-enabled every addon the moment a character was created.
let store = EnableStore::load(
identity
.map(|(realm, _)| realm.as_str())
.unwrap_or_default(),
&store_nodes(identity, roster),
);
let character = identity.map(|(_, c)| c.as_str());
for (info, addon) in infos.iter_mut().zip(addons.iter()) {
info.enabled = store.enabled_for(&info.name, addon.toc.default_state(), character);
}
let disabled_names: HashSet<String> = infos
.iter()
@@ -1328,7 +1435,7 @@ mod tests {
// No `## Title` at all — sorts under its folder name, the comparator's own fallback.
write_addon(&home, "bravo", "## Interface: 11200\n", &[]);
let rows = installed(None);
let rows = installed_rows();
assert_eq!(
rows.iter().map(|a| a.name.as_str()).collect::<Vec<_>>(),
vec!["Zulu", "bravo", "Mike", "Alpha"],
@@ -1454,7 +1561,7 @@ mod tests {
"benilla.toc lists a Blizzard addon eagerly; the reference loads every one on demand"
);
// The glue's list is the player's folder alone.
assert!(installed(None)
assert!(installed_rows()
.iter()
.all(|a| !a.name.starts_with("Blizzard_")));
}
@@ -2102,7 +2209,7 @@ mod tests {
let mut script = UiScript::new().unwrap();
script.set_screen_size(1024.0, 768.0);
script.run("EventLog = {}").unwrap();
let failures = load_third_party(&mut script, None, true);
let failures = load_third_party(&mut script, None, &[], true);
assert!(failures.is_empty(), "load errors: {failures:?}");
crate::ui_script::finish_ui_load(&mut script);
@@ -2175,7 +2282,7 @@ mod tests {
);
let mut script = UiScript::new().unwrap();
let failures = load_third_party(&mut script, None, true);
let failures = load_third_party(&mut script, None, &[], true);
assert!(
failures.iter().any(|f| f.contains("NeedsOld")),
"the gated dependency is the dependent's failure: {failures:?}"
@@ -2199,7 +2306,7 @@ mod tests {
// Force-load: the same folder, the checkbox's other state — everything loads.
let mut open = UiScript::new().unwrap();
let failures = load_third_party(&mut open, None, false);
let failures = load_third_party(&mut open, None, &[], false);
assert!(failures.is_empty(), "force-load load errors: {failures:?}");
assert_eq!(
open.eval::<bool>(
@@ -2273,7 +2380,7 @@ mod tests {
);
let mut script = UiScript::new().unwrap();
let failures = load_third_party(&mut script, None, true);
let failures = load_third_party(&mut script, None, &[], true);
assert!(failures.is_empty(), "load errors: {failures:?}");
// Discovery saw it at all — the `.toc` decoded rather than read as absent.
@@ -2394,7 +2501,7 @@ mod tests {
&[],
);
let mut script = UiScript::new().unwrap();
let _ = load_third_party(&mut script, None, true);
let _ = load_third_party(&mut script, None, &[], true);
// **One**, not one-plus-the-chain (decision 2175). The registry holds the chain's twelve
// Blizzard rows too, but the Lua index space is a different set: `SMSG_ADDON_INFO` answers
@@ -2496,7 +2603,7 @@ mod tests {
let mut script = UiScript::new().unwrap();
let id = ("Realm".to_string(), "Char".to_string());
let failures = load_third_party(&mut script, Some(&id), true);
let failures = load_third_party(&mut script, Some(&id), &[], true);
assert!(
failures.is_empty(),
@@ -2554,7 +2661,7 @@ mod tests {
);
let mut script = UiScript::new().unwrap();
script.set_screen_size(1024.0, 768.0);
let _ = load_third_party(&mut script, None, true);
let _ = load_third_party(&mut script, None, &[], true);
// Discovered and described, but NOT run.
// **One**, not one-plus-the-chain (decision 2175). The registry holds the chain's twelve
@@ -2650,7 +2757,7 @@ mod tests {
);
let mut script = UiScript::new().unwrap();
script.set_screen_size(1024.0, 768.0);
let _ = load_third_party(&mut script, None, true);
let _ = load_third_party(&mut script, None, &[], true);
// The dependency loaded at startup and the registry knows it — the stamp this rests on.
assert_eq!(
@@ -2704,7 +2811,7 @@ mod tests {
);
let mut script = UiScript::new().unwrap();
script.set_screen_size(1024.0, 768.0);
let _ = load_third_party(&mut script, None, true);
let _ = load_third_party(&mut script, None, &[], true);
assert_eq!(
script
@@ -2766,7 +2873,7 @@ mod tests {
let mut script = UiScript::new().unwrap();
let id = ("Realm".to_string(), "Char".to_string());
let _ = load_third_party(&mut script, Some(&id), true);
let _ = load_third_party(&mut script, Some(&id), &[], true);
for (call, want) in [
("LoadAddOn('NoSuchAddon')", "MISSING"),
@@ -2788,6 +2895,75 @@ mod tests {
let _ = std::fs::remove_dir_all(home.parent().unwrap());
}
/// **A character with no enable file does not load what the realm has unanimously turned
/// off** — the world-entry half of the director's 2026-09-17 report (decision 2311).
///
/// The glue list showing a new character's addons as off would be worth nothing if the world
/// then loaded them anyway, so the load walk resolves through the same [`EnableStore`]: the
/// reference's `AddOn_CanLoad` check 3 is `0x51e470(name, character, useDefault = 1)`, which
/// for a character with no explicit entry takes the explicit-only aggregate over the realm's
/// other characters (wow-5875-re `addon-enable-store.md` §4).
///
/// The control is the second addon: one character left it on, so the roster does **not** agree
/// about it, and the newcomer gets its manifest's `## DefaultState` instead — enabled. Without
/// that half the test would pass on a client that simply disabled everything for new
/// characters.
#[test]
fn a_character_with_no_file_inherits_the_rosters_unanimous_disable() {
let _l = crate::local_state::test_env::ENV_LOCK
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
let _c = crate::local_state::test_env::EnvGuard::unset("WOW_CAPTURE");
let (home, _h) = hermetic_root("inherit");
write_addon(
&home,
"Shunned",
"## Interface: 11200\nran.lua\n",
&[("ran.lua", "ShunnedRan = true")],
);
write_addon(
&home,
"Contested",
"## Interface: 11200\nran.lua\n",
&[("ran.lua", "ContestedRan = true")],
);
// Two characters who have played and saved: both turned `Shunned` off, they disagree
// about `Contested`.
for (who, contested) in [("Onemage", false), ("Onerogue", true)] {
let id = ("Realm".to_string(), who.to_string());
write_enable_state(
Some(&id),
&[("Shunned".into(), false), ("Contested".into(), contested)],
);
}
let roster = [
"Onemage".to_string(),
"Onerogue".to_string(),
"Freshling".to_string(),
];
// …and the character just created, who has no file of their own.
let fresh = ("Realm".to_string(), "Freshling".to_string());
assert!(
!enable_state_path(Some(&fresh)).unwrap().exists(),
"the premise: a newly created character has written nothing"
);
let mut script = UiScript::new().unwrap();
let _ = load_third_party(&mut script, Some(&fresh), &roster, true);
assert_eq!(
script.eval::<bool>("return ShunnedRan == nil").ok(),
Some(true),
"the unanimous disable reaches the character who never expressed one"
);
assert_eq!(
script.eval::<bool>("return ContestedRan == true").ok(),
Some(true),
"…and a contested addon falls to its `## DefaultState`, which is enabled"
);
let _ = std::fs::remove_dir_all(home.parent().unwrap());
}
/// Enable state survives the session: `DisableAddOn` from Lua is written back in the
/// reference's own `AddOns.txt` format, and read back as a disable next time.
#[test]
@@ -2807,7 +2983,7 @@ mod tests {
let id = ("Realm".to_string(), "Char".to_string());
let mut script = UiScript::new().unwrap();
let _ = load_third_party(&mut script, Some(&id), true);
let _ = load_third_party(&mut script, Some(&id), &[], true);
script.run("DisableAddOn('Drop')").unwrap();
save_enable_state(&script, Some(&id));
@@ -2828,7 +3004,7 @@ mod tests {
// A fresh session reads it back and the addon stays off.
let mut next = UiScript::new().unwrap();
let _ = load_third_party(&mut next, Some(&id), true);
let _ = load_third_party(&mut next, Some(&id), &[], true);
assert_eq!(
next.eval::<bool>("return DropRan == nil").ok(),
Some(true),
@@ -2879,7 +3055,7 @@ mod tests {
// ── session one: defaults, then the addon changes them ──
let mut script = UiScript::new().unwrap();
script.set_screen_size(1024.0, 768.0);
let failures = load_third_party(&mut script, Some(&id), true);
let failures = load_third_party(&mut script, Some(&id), &[], true);
assert!(failures.is_empty(), "{failures:?}");
assert_eq!(
script.eval::<i64>("return KeeperSawAtEvent").ok(),
@@ -2894,7 +3070,7 @@ mod tests {
// ── session two: a fresh VM reads them back ──
let mut next = UiScript::new().unwrap();
next.set_screen_size(1024.0, 768.0);
let failures = load_third_party(&mut next, Some(&id), true);
let failures = load_third_party(&mut next, Some(&id), &[], true);
assert!(failures.is_empty(), "{failures:?}");
assert_eq!(
next.eval::<i64>("return KeeperDB.count").ok(),
@@ -2945,7 +3121,7 @@ mod tests {
);
let id = ("Realm".to_string(), "Char".to_string());
let mut script = UiScript::new().unwrap();
let _ = load_third_party(&mut script, Some(&id), true);
let _ = load_third_party(&mut script, Some(&id), &[], true);
script
.run("GramDB = { ['on'] = true, ['n'] = 2, ['s'] = 'a\\\"b', ['t'] = { 1 } } GramChar = 5")
.unwrap();
@@ -3007,7 +3183,7 @@ mod tests {
let id = ("Realm".to_string(), "Char".to_string());
let mut script = UiScript::new().unwrap();
script.set_screen_size(1024.0, 768.0);
let _ = load_third_party(&mut script, Some(&id), true);
let _ = load_third_party(&mut script, Some(&id), &[], true);
crate::ui_script::shutdown_ui_state(&mut script, Some(&id), true);
let written = std::fs::read_to_string(home.join("saved/Last.lua")).unwrap();
@@ -3048,7 +3224,7 @@ mod tests {
let mut script = UiScript::new().unwrap();
script.set_screen_size(1024.0, 768.0);
let _ = load_third_party(&mut script, Some(&id), true);
let _ = load_third_party(&mut script, Some(&id), &[], true);
script.eval::<()>("DisableAddOn('MyAddon')").unwrap();
save_enable_state(&script, Some(&id));
@@ -3132,7 +3308,7 @@ mod tests {
AddonOrder = AddonOrder .. arg1 .. ',' end)",
)
.unwrap();
let _ = load_third_party(&mut script, None, true);
let _ = load_third_party(&mut script, None, &[], true);
// Neither ran at startup — the walk skips LoadOnDemand, which is why a cycle among them
// is never observed until something demand-loads one.
crates/benilla-app/src/ui_script/lifecycle.rs
+9 -1
View File
@@ -457,6 +457,14 @@ pub(crate) fn load_ingame_ui_on_world_entry(world: &mut World) {
let identity = world
.get_resource::<crate::char_select::Roster>()
.and_then(crate::ui_macro::identity);
// …and the realm's whole character list, which is the enable store's node set: the reference
// populates one `ADDONSTATELIST` node per character at char-list time, and an addon this
// character has never had an opinion about is resolved from what the others said
// (decision 2311).
let roster: Vec<String> = world
.get_resource::<crate::char_select::Roster>()
.map(|r| r.chars.iter().map(|c| c.name.clone()).collect())
.unwrap_or_default();
// **The Lua index space, before a single addon file runs** (decision 2175). The reference has
// the array in hand well before `UI_Init 0x48fbf0` reaches the addon walk — `SMSG_ADDON_INFO`
// lands during the handshake — so an addon reading `GetNumAddOns()` at file scope sees a
@@ -576,7 +584,7 @@ pub(crate) fn load_ingame_ui_on_world_entry(world: &mut World) {
// through its whole load edge and only converged a frame later off whatever poll the caller
// had written to survive it.
seat_raster_seam_for_load(world, &mut script);
let _ = load_ingame_ui(&mut script, identity.as_ref(), version_check);
let _ = load_ingame_ui(&mut script, identity.as_ref(), &roster, version_check);
// The Minimap widget was born a moment ago with `MinimapState::default()`; seed its two live
// zoom indices from the persisted CVars now, before anything reads them — the reference's own
// minimap reset path copying each CVar object's int into its live index (decision 1131). Once
crates/benilla-app/src/ui_script/manifest.rs
+6 -2
View File
@@ -303,8 +303,10 @@ pub(crate) fn load_font_registry(script: &UiScript) -> Vec<String> {
/// an addon.
///
/// `identity` is `(realm, character)`, which names this character's AddOn enable-state file — the
/// reference keys `AddOns.txt` per character too. `None` (no pick yet, a capture) means every
/// discovered addon is enabled, the same answer an absent file gives.
/// reference keys `AddOns.txt` per character too — and `roster` is every character on that realm's
/// list, which is the enable store's node set (decision 2311: an addon this character has no row
/// for is resolved from what the *other* characters said, never from a bare "enabled"). `None`
/// with an empty roster is the no-pick case: every addon falls to its own `## DefaultState`.
///
/// `version_check` is the persisted `checkAddonVersion` — the *Load out of date AddOns* toggle,
/// inverted — resolved by the caller because at load time this VM's own CVar table does not
@@ -314,6 +316,7 @@ pub(crate) fn load_font_registry(script: &UiScript) -> Vec<String> {
pub(crate) fn load_ingame_ui(
script: &mut UiScript,
identity: Option<&(String, String)>,
roster: &[String],
version_check: bool,
) -> Vec<String> {
// The whole load edge runs bounded (decision 1306): the reference files sourced off the
@@ -332,6 +335,7 @@ pub(crate) fn load_ingame_ui(
failures.extend(super::addons::load_third_party(
script,
identity,
roster,
version_check,
));
failures
crates/benilla-app/tests/auction_frame.rs
+119
View File
@@ -256,6 +256,125 @@ fn auction_house_show_opens_the_window_on_the_browse_tab() {
assert!(s.errors().is_empty(), "clean open: {:?}", s.errors());
}
/// **The bug the per-list fires were made for** (decision 2308), in the stock addon's own Lua.
///
/// `AuctionFrameAuctions_Update` computes `offset + i + (NUM_AUCTION_ITEMS_PER_PAGE *
/// AuctionFrameAuctions.page)` on its very first line of loop body, and `AuctionFrameAuctions.page`
/// is assigned in exactly one place in the whole addon: that tab's `OnShow`, three lines below its
/// `GetOwnerAuctionItems()`. So the event is only safe to fire *after* the Auctions tab has been
/// shown at least once — which the reference guarantees structurally, because every caller of
/// `GetOwnerAuctionItems` lives *inside* that pane (the `OnShow`, and the two page-turner buttons,
/// which pass `AuctionFrameAuctions.page` as their argument).
///
/// The feed used to fire all three list events on any change, so the first browse result of the
/// session ran this repaint on a tab nobody had opened and put
/// `attempt to perform arithmetic on field 'page' (a nil value)` on the player's screen. This test
/// is the hazard itself, pinned: it fails the day the stock Lua stops caring, and `ui_auction`'s
/// own `a_result_owes_only_its_own_lists_event` is the guarantee that we never hand it that event.
#[test]
fn the_auctions_tab_cannot_repaint_before_it_has_been_shown() {
let _data = benilla_formats::wow_data_or_skip!();
let mut s = UiScript::new().unwrap();
load_ui_with_classes(&mut s);
seat_player(&mut s, 500_000);
s.set_auction(Some(state(vec![row(
"Linen Cloth",
1000,
5000,
0,
"Seller",
)])));
// Open, and land a browse result — the whole of what a search does. Clean.
s.fire_event("AUCTION_HOUSE_SHOW", vec![]);
s.fire_event("AUCTION_ITEM_LIST_UPDATE", vec![]);
assert!(
s.take_errors().is_empty(),
"a browse result on the Browse tab is clean"
);
// The same result also announcing the *owned* list is what crashed: the Auctions tab has never
// been shown, so its `page` is nil and the repaint does arithmetic on it.
s.fire_event("AUCTION_OWNED_LIST_UPDATE", vec![]);
let errors = s.take_errors();
assert!(
errors.iter().any(|e| e.contains("page")),
"the never-shown Auctions tab raises the nil-page error: {errors:?}"
);
// And the same event is harmless the moment the tab has been shown once — which is the only
// state the app can now produce it in, since the owned list is only ever asked for from there.
s.run("AuctionFrameTab_OnClick(3)").unwrap();
assert!(s.take_errors().is_empty(), "opening the tab is clean");
s.fire_event("AUCTION_OWNED_LIST_UPDATE", vec![]);
assert!(
s.take_errors().is_empty(),
"and now the owned list may announce itself"
);
}
/// Why only the Auctions tab was in the screenshot — and why that is an accident of the XML.
///
/// All three panes read their `page` field in `_Update`, and each assigns it in exactly one place:
/// its own `OnShow`. What differs is *when* that `OnShow` first runs, and it is decided by one
/// attribute in the stock file — `AuctionFrameBrowse` and `AuctionFrameAuctions` are declared
/// `hidden="true"`, `AuctionFrameBid` is declared **`hidden="false"`**. So the Bids pane becomes
/// visible with the window itself, its `OnShow` runs on the very first `AUCTION_HOUSE_SHOW` —
/// before `AuctionFrameTab_OnClick(1)` hides it again — and `AuctionFrameBid.page` is `0` while
/// the player is still looking at Browse. The Auctions pane gets nothing at all until tab 3.
///
/// That is the whole reason a mis-aimed `AUCTION_OWNED_LIST_UPDATE` crashed and a mis-aimed
/// `AUCTION_BIDDER_LIST_UPDATE` did not — and the reason the fix is the rule (an event names one
/// list) rather than a guard on the one tab that happened to be reported.
#[test]
fn only_the_bids_pane_gets_its_page_without_being_opened() {
let _data = benilla_formats::wow_data_or_skip!();
let mut s = UiScript::new().unwrap();
load_ui_with_classes(&mut s);
let page = |s: &UiScript, pane: &str| {
s.eval::<String>(&format!("return tostring({pane}.page)"))
.unwrap()
};
// At load, with no session: no pane has been shown, so none has a page.
for pane in [
"AuctionFrameBrowse",
"AuctionFrameBid",
"AuctionFrameAuctions",
] {
assert_eq!(page(&s, pane), "nil", "{pane} before the window opens");
}
// The window opens on tab 1. Browse is shown by the tab click; Bid is shown by the window
// itself and hidden again a moment later, keeping the page its OnShow assigned.
seat_player(&mut s, 500_000);
s.set_auction(Some(state(vec![row(
"Linen Cloth",
1000,
5000,
0,
"Seller",
)])));
s.fire_event("AUCTION_HOUSE_SHOW", vec![]);
assert_eq!(page(&s, "AuctionFrameBrowse"), "0");
assert_eq!(
page(&s, "AuctionFrameBid"),
"0",
"hidden=\"false\": the Bids pane rode the window up and back down"
);
assert!(
!s.eval::<bool>("return AuctionFrameBid:IsShown()").unwrap(),
"and it is not the visible tab"
);
assert_eq!(
page(&s, "AuctionFrameAuctions"),
"nil",
"the Auctions pane is the one an unowed event can still kill"
);
assert!(s.take_errors().is_empty());
}
/// The part that proves it works: a fed snapshot paints the Browse rows.
#[test]
fn the_browse_list_populates_from_the_fed_snapshot() {
crates/benilla-ui/src/toc.rs
+19
View File
@@ -171,6 +171,25 @@ impl Toc {
pub fn load_on_demand(&self) -> bool {
self.directive("LoadOnDemand").map(str::trim) == Some("1")
}
/// `## DefaultState:` — what this addon's enable state is for a character who has never
/// expressed one. The reference's `[rec+0x2b]`, stored by `Toc_Parse` at `0x51d204` from the
/// two literals `"enabled"` (`0x853764`) → `1` and `"disabled"` (`0x853758`) → `0`
/// (wow-5875-re `savedvariables-protocol.md`, the directive table).
///
/// It is load-bearing well beyond a manifest that writes it: the enable query `0x51e470`
/// falls back to this byte whenever the characters disagree, and whenever *none* of them has
/// an opinion at all — which is every addon on a fresh install.
///
/// **A manifest that does not write the line is enabled**, which is what makes a folder
/// dropped into `AddOns/` just work. That is the two literals' fall-through, not a guess at
/// the record's initial byte — a value that is neither `enabled` nor `disabled` reads the
/// same way, because neither compare matches it and the field keeps whatever it had.
pub fn default_state(&self) -> bool {
!self
.directive("DefaultState")
.is_some_and(|v| v.trim().eq_ignore_ascii_case("disabled"))
}
}
#[cfg(test)]