From 36b359f0075be4aa2d56d6b4eebb3bb28a866350 Mon Sep 17 00:00:00 2001 From: Sam Sung <51686767+samwhosung@users.noreply.github.com> Date: Mon, 28 Sep 2026 10:11:40 +0700 Subject: [PATCH] Logout and the AddOns screen's Okay write only a changed character's own enable rows, so a character with no row keeps following the realm's others (#418) (#490) A character's AddOns file now holds only the rows the character chose, as in 1.12.1, so a character that never chose an addon keeps following what the realm's other characters agree on (`0x51e470`) instead of freezing its inherited values at the first logout. - Each character has an enable hash and a dirty flag: the loader `0x51ebe0` fills the hash from the file, clean; the setter `0x51ea20` inserts on a miss and dirties the hash only on a change (`0x51ebab`-`0x51ebbc`); the writer `0x51ef20` writes a dirty character's hash, a line per entry, and nothing else (`0x51ef59`). - Logout and exit (`0x490c88`) and the character-select AddOns screen's Okay (`SaveAddOns` `0x46d990`) both write through it, so a session that toggles nothing writes nothing, and no `Blizzard_*` rows appear. - `ResetDisabledAddOns` restores the hash as loaded, clean (`0x48e830`). - On the AddOns screen a character with no row now follows another character's toggle at once, as `AddonList_Update` re-reads each column after every `EnableAddOn`. Fixes #418 Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/benilla-app/src/char_select/addons.rs | 283 +++++++++--------- crates/benilla-app/src/ui_script/addons.rs | 223 ++++++++++---- crates/benilla-ui/src/script/addon.rs | 29 +- crates/benilla-ui/src/script/addon_enable.rs | 135 +++++++++ crates/benilla-ui/src/script/mod.rs | 2 + crates/benilla-ui/src/script/model.rs | 7 + .../src/script/tests/addon_index_space.rs | 17 ++ 7 files changed, 494 insertions(+), 202 deletions(-) create mode 100644 crates/benilla-ui/src/script/addon_enable.rs diff --git a/crates/benilla-app/src/char_select/addons.rs b/crates/benilla-app/src/char_select/addons.rs index d20f0196..9c5c0bd9 100644 --- a/crates/benilla-app/src/char_select/addons.rs +++ b/crates/benilla-app/src/char_select/addons.rs @@ -104,21 +104,20 @@ enum Hover { Check(usize), } -/// The screen's state; edits stage per character and only Okay writes the files. +/// The screen's state; a click writes the in-memory store and only Okay writes the files. #[derive(Resource)] pub(super) struct AddonsPanel { pub(super) open: bool, - /// The installed list read at open, so a row's index is stable; the enable state is `staged`'s. + /// The installed list read at open, so a row's index is stable. list: Vec, - /// The roster's realm, half of every enable file's `(realm, name)` key. - realm: String, /// The roster's character names, the dropdown's entries after "All". chars: Vec, - /// `staged[c][i]` is character `c`'s bit for `list[i]`; with no characters, one anonymous - /// column that Okay does not write. - staged: Vec>, - /// What the files said at open; Okay writes only the columns that moved off it. - baseline: Vec>, + /// Every roster character's enable hash, which a click sets and Okay saves where dirty. + store: addons::EnableStore, + /// `bits[c][i]` is the query's answer for character `c` and `list[i]`, re-read from the store + /// after every change, so a character with no row follows the others' edits at once; with no + /// characters, one anonymous column of `## DefaultState`s. + bits: Vec>, /// The dropdown's selection: `None` is "All", `Some(c)` is `chars[c]`. view: Option, dropdown_open: bool, @@ -145,10 +144,9 @@ impl Default for AddonsPanel { Self { open: false, list: Vec::new(), - realm: String::new(), chars: Vec::new(), - staged: Vec::new(), - baseline: Vec::new(), + store: addons::EnableStore::default(), + bits: Vec::new(), view: None, dropdown_open: false, // The registered default, "1"; a `false` would paint out-of-date rows loadable for a @@ -165,29 +163,14 @@ impl Default for AddonsPanel { } impl AddonsPanel { - /// Open for this realm's roster: read the folder, and one staged column per character, as - /// `AddonList_OnShow` re-reads. The store loads for the whole roster: an addon a character has - /// no row for takes what the other characters agree on, else the manifest's `## DefaultState`. + /// Open for this realm's roster: read the folder and every character's enable file, as + /// `AddonList_OnShow` re-reads. An addon a character has no row for takes what the other + /// characters agree on, else the manifest's `## DefaultState`. pub(super) fn open_for(&mut self, realm: String, chars: Vec) { self.list = addons::installed_rows(); - self.staged.clear(); - let store = addons::EnableStore::load(&realm, &chars); - let columns: Vec> = if chars.is_empty() { - vec![None] - } else { - 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; + self.store = addons::EnableStore::load(&realm, &chars); self.chars = chars; + self.reread(); // "All" is the reference's default selection. self.view = None; self.dropdown_open = false; @@ -201,31 +184,53 @@ impl AddonsPanel { self.open = false; self.list.clear(); self.chars.clear(); - self.staged.clear(); - self.baseline.clear(); + self.store = addons::EnableStore::default(); + self.bits.clear(); self.view = None; self.dropdown_open = false; self.tip = None; self.dirty = true; } - /// Okay, `SaveAddOns`: write the enable file of each character whose column changed. - fn save_staged(&self) { - for (c, name) in self.chars.iter().enumerate() { - if self.staged.get(c) == self.baseline.get(c) { - continue; - } - let id = (self.realm.clone(), name.clone()); - let states: Vec<(String, bool)> = self - .list - .iter() - .zip(self.staged[c].iter()) - .map(|(a, &on)| (a.name.clone(), on)) - .collect(); - addons::write_enable_state(Some(&id), &states); + /// Re-read every checkbox through the query `0x51e470`, as `AddonList_Update` does after each + /// `EnableAddOn`/`DisableAddOn`. + fn reread(&mut self) { + let columns: Vec> = if self.chars.is_empty() { + vec![None] + } else { + self.chars.iter().map(|c| Some(c.as_str())).collect() + }; + self.bits = columns + .into_iter() + .map(|character| { + self.list + .iter() + .map(|a| self.store.enabled_for(&a.name, a.default_state, character)) + .collect() + }) + .collect(); + self.dirty = true; + } + + /// The glue `EnableAddOn`/`DisableAddOn` (`0x46d7b0`, `0x46d8a0`) for the current view: the + /// setter on the viewed character, or on every character in the All view (`nil`). With no + /// characters there is no node to set, so a click changes nothing. + fn set(&mut self, i: usize, on: bool) { + let character = self + .view + .and_then(|c| self.chars.get(c)) + .map(String::as_str); + if let Some(a) = self.list.get(i) { + self.store.set(&a.name, character, on); } } + /// Okay, `SaveAddOns` (`0x46d990` into the writer `0x51ef20`): each character a click changed + /// gets its enable hash written; the rest are left as they are. + fn save(&mut self) { + self.store.save(); + } + /// Whether any addon is installed (`UpdateAddonButton` hides the button otherwise). Cached for /// the process: the screen respawns every frame of a drag-resize. pub(super) fn any_installed() -> bool { @@ -269,7 +274,7 @@ impl AddonsPanel { match self.view { Some(c) => { if self - .staged + .bits .get(c) .is_some_and(|col| col.get(i) == Some(&true)) { @@ -280,13 +285,13 @@ impl AddonsPanel { } None => { let on = self - .staged + .bits .iter() .filter(|col| col.get(i) == Some(&true)) .count(); if on == 0 { BoxState::Off - } else if on == self.staged.len() { + } else if on == self.bits.len() { BoxState::On } else { BoxState::Mixed @@ -339,40 +344,17 @@ impl AddonsPanel { /// A checkbox click. In the All view a mixed box counts as checked, so clicking On or Mixed /// disables for every character and only an unchecked box enables for all. fn click_row(&mut self, i: usize) { - match self.view { - Some(c) => { - if let Some(slot) = self.staged.get_mut(c).and_then(|col| col.get_mut(i)) { - *slot = !*slot; - self.dirty = true; - } - } - None => { - let target = self.box_state(i) == BoxState::Off; - for col in &mut self.staged { - if let Some(slot) = col.get_mut(i) { - *slot = target; - } - } - self.dirty = true; - } - } + let on = self.box_state(i) == BoxState::Off; + self.set(i, on); + self.reread(); } - /// Enable All and Disable All sweep the current view's columns. + /// Enable All and Disable All: the setter for every listed addon, on the current view. fn set_all(&mut self, on: bool) { - match self.view { - Some(c) => { - if let Some(col) = self.staged.get_mut(c) { - col.iter_mut().for_each(|s| *s = on); - } - } - None => { - for col in &mut self.staged { - col.iter_mut().for_each(|s| *s = on); - } - } + for i in 0..self.list.len() { + self.set(i, on); } - self.dirty = true; + self.reread(); } fn view_name<'a>(&'a self, strings: &'a GlueStrings) -> &'a str { @@ -564,7 +546,7 @@ pub(super) fn drive_addons_panel( if close_and_save { // `AddonList_OnOk` and `AddonList_OnCancel` play the realm dialog's pair. sounds.write(GlueSound("gsLoginChangeRealmOK")); - panel.save_staged(); + panel.save(); panel.close(); return; } @@ -1531,27 +1513,36 @@ mod tests { } } - /// A panel over `chars` staged columns, each starting at the manifest defaults, viewing All. + /// A panel over `chars` characters with no enable rows, so every bit starts at the manifest + /// default, viewing All. fn panel_for(chars: usize, list: Vec) -> AddonsPanel { - let staged: Vec> = (0..chars.max(1)) - .map(|_| list.iter().map(|a| a.default_state).collect()) - .collect(); - AddonsPanel { + let chars: Vec = (0..chars).map(|c| format!("Char{c}")).collect(); + let mut p = AddonsPanel { open: true, - realm: "TestRealm".into(), - chars: (0..chars).map(|c| format!("Char{c}")).collect(), - baseline: staged.clone(), - staged, + store: addons::EnableStore::blank("TestRealm", &chars), + chars, list, ..Default::default() - } + }; + p.reread(); + p.dirty = false; // the tests spawn no tree + p } - /// One column, whose All view is that column, so `p.staged[0][i]` drives everything. + /// One character, whose All view is that character. fn panel(list: Vec) -> AddonsPanel { panel_for(1, list) } + /// The glue setter for character `c` (`None`: every character), then the re-read + /// `AddonList_Enable` runs. + fn set(p: &mut AddonsPanel, c: Option, i: usize, on: bool) { + let view = std::mem::replace(&mut p.view, c); + p.set(i, on); + p.view = view; + p.reread(); + } + /// Every fallback label is its token's shipped value (`GlueStrings.lua:44-56`). #[test] fn the_status_labels_are_the_reference_globalstrings_values() { @@ -1611,11 +1602,11 @@ mod tests { assert_eq!(p.verdict(5).token(), None); p.version_check = true; - p.staged[0][1] = false; + set(&mut p, Some(0), 1, false); assert_eq!(p.verdict(1).token().as_deref(), Some("DISABLED")); assert_eq!(p.verdict(2).token().as_deref(), Some("DEP_DISABLED")); - p.staged[0][3] = false; + set(&mut p, Some(0), 3, false); assert_eq!(p.verdict(3).token().as_deref(), Some("DISABLED")); } @@ -1630,7 +1621,7 @@ mod tests { assert_eq!(p.title_colour(0), GOLD, "loadable = gold"); assert_eq!(p.title_colour(2), BROKEN, "enabled + out of date = red"); - p.staged[0][0] = false; + set(&mut p, Some(0), 0, false); assert_eq!(p.title_colour(0), DIM, "player-disabled = grey"); assert_eq!( p.title_colour(1), @@ -1646,7 +1637,9 @@ mod tests { 2, vec![addon("Lib", &[], 11200), addon("Needs", &["Lib"], 11200)], ); - p.staged[0][0] = false; // Char0 turned Lib off, Char1 has not + // Char0 turned Lib off and Char1 on; with no row Char1 would follow Char0. + set(&mut p, Some(0), 0, false); + set(&mut p, Some(1), 0, true); assert_eq!( p.verdict(0).token(), @@ -1668,20 +1661,21 @@ mod tests { let mut p = panel_for(3, vec![addon("A", &[], 11200)]); assert_eq!(p.box_state(0), BoxState::On, "enabled for all three"); - p.staged[1][0] = false; + set(&mut p, None, 0, true); // a row for each, so one can differ + set(&mut p, Some(1), 0, false); assert_eq!( p.box_state(0), BoxState::Mixed, "enabled for SOME (state 1)" ); - p.staged[0][0] = false; - p.staged[2][0] = false; + set(&mut p, Some(0), 0, false); + set(&mut p, Some(2), 0, false); assert_eq!(p.box_state(0), BoxState::Off, "disabled for all"); p.view = Some(1); assert_eq!(p.box_state(0), BoxState::Off); - p.staged[1][0] = true; + set(&mut p, Some(1), 0, true); assert_eq!(p.box_state(0), BoxState::On); } @@ -1689,47 +1683,51 @@ mod tests { #[test] fn an_all_view_click_follows_the_references_checkbutton() { let mut p = panel_for(3, vec![addon("A", &[], 11200)]); - p.staged[1][0] = false; // mixed + set(&mut p, None, 0, true); + set(&mut p, Some(1), 0, false); // mixed p.click_row(0); assert!( - p.staged.iter().all(|col| !col[0]), + p.bits.iter().all(|col| !col[0]), "mixed → disabled for ALL characters" ); p.click_row(0); assert!( - p.staged.iter().all(|col| col[0]), + p.bits.iter().all(|col| col[0]), "unchecked → enabled for ALL characters" ); p.click_row(0); - assert!(p.staged.iter().all(|col| !col[0]), "checked → back off"); + assert!(p.bits.iter().all(|col| !col[0]), "checked → back off"); p.view = Some(2); p.click_row(0); - assert!(p.staged[2][0] && !p.staged[0][0] && !p.staged[1][0]); + assert!(p.bits[2][0] && !p.bits[0][0] && !p.bits[1][0]); } /// No enable file is touched until Okay; Cancel (`ResetAddOns`) is `close`. #[test] fn edits_are_staged_and_cancel_discards_them() { let mut p = panel(vec![addon("A", &[], 11200), addon("B", &[], 11200)]); - assert_eq!(p.staged, vec![vec![true, true]]); - p.staged[0][0] = false; + assert_eq!(p.bits, vec![vec![true, true]]); + set(&mut p, Some(0), 0, false); + assert_eq!(p.bits, vec![vec![false, true]]); assert!( p.list[0].default_state, - "the read-in list is never mutated — only `staged` is, which is what makes Cancel free" + "the read-in list is never mutated — only the store is, which is what makes Cancel free" ); p.close(); assert!(!p.open); assert!( - p.staged.is_empty(), + p.bits.is_empty() && p.store.node("Char0").is_none(), "a cancelled edit leaves nothing behind" ); } - /// Okay writes each changed character's file and creates none for an untouched one. + /// Okay (`SaveAddOns`) writes only the characters whose hash a click changed, and each one's + /// own rows only: the issue's Alice and Bob, with Carol never touched. Bob's disable reaches + /// Alice and Carol through the query, on screen at once and after the files are read back. #[test] fn okay_writes_every_changed_characters_file_and_only_those() { let _l = crate::local_state::test_env::ENV_LOCK @@ -1747,6 +1745,13 @@ mod tests { 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 path = |who: &str| { + addons::enable_state_path(Some(&("TestRealm".to_string(), who.to_string()))).unwrap() + }; + // Alice's file as some other writer left it: a rewrite would drop the stray line. + let alice_file = "Gone: disabled\nstray line\nAlpha: enabled\n"; + std::fs::create_dir_all(path("Alice").parent().unwrap()).unwrap(); + std::fs::write(path("Alice"), alice_file).unwrap(); let mut p = AddonsPanel::default(); p.open_for( @@ -1755,12 +1760,31 @@ mod tests { ); assert!(p.view.is_none(), "the reference's default selection is All"); assert_eq!(p.list.len(), 2, "discovery found the two hermetic addons"); - assert_eq!(p.staged.len(), 3, "one staged column per roster character"); - assert_eq!(p.staged, p.baseline); + assert_eq!(p.bits.len(), 3, "one column per roster character"); - p.staged[0][0] = false; - p.staged[1][1] = false; - p.save_staged(); + set(&mut p, Some(0), 0, true); // Alice re-ticks what her row already says + set(&mut p, Some(1), 1, false); // Bob unticks Beta + assert_eq!( + p.bits, + vec![vec![true, false]; 3], + "Bob is the only one with a Beta row, so Alice and Carol follow him at once" + ); + p.save(); + + assert_eq!( + std::fs::read_to_string(path("Alice")).unwrap(), + alice_file, + "a setter call that changed nothing leaves the node clean and the file unwritten" + ); + assert_eq!( + std::fs::read_to_string(path("Bob")).unwrap(), + "Beta: disabled\n", + "Bob's file carries his one toggle, not a row per listed addon" + ); + assert!( + !path("Carol").exists(), + "an untouched character writes no file" + ); // Read back through the store world entry resolves against. let roster = ["Alice".to_string(), "Bob".to_string(), "Carol".to_string()]; @@ -1771,20 +1795,9 @@ mod tests { .map(|a| store.enabled_for(&a.name, a.default_state, Some(who))) .collect() }; - assert_eq!( - read("Alice"), - vec![false, true], - "Alice's file carries HER edit" - ); - 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" - ); - // Carol, with no file, takes what the others agree on; they split on both, so each falls - // to its manifest's `## DefaultState`. - assert_eq!(read("Carol"), vec![true, true]); + for who in ["Alice", "Bob", "Carol"] { + assert_eq!(read(who), vec![true, false], "{who}"); + } let _ = std::fs::remove_dir_all(&tmp); } @@ -1815,7 +1828,7 @@ mod tests { vec!["Onemage".into(), "Onerogue".into()], ); p.set_all(false); - p.save_staged(); + p.save(); p.close(); // A created character, with no enable file. @@ -1824,9 +1837,9 @@ mod tests { 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.bits.len(), 3); assert_eq!( - p.staged[2], + p.bits[2], vec![false, false], "the new character inherits the unanimous disable, not a blank slate" ); @@ -1879,7 +1892,7 @@ mod tests { 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.save(); p.close(); p.open_for( @@ -1887,7 +1900,7 @@ mod tests { vec!["Onemage".into(), "Onerogue".into(), "Freshling".into()], ); assert_eq!( - p.staged[2], + p.bits[2], vec![true, false], "split roster → each row falls to its own `## DefaultState`" ); diff --git a/crates/benilla-app/src/ui_script/addons.rs b/crates/benilla-app/src/ui_script/addons.rs index f40efae2..bacb360d 100644 --- a/crates/benilla-app/src/ui_script/addons.rs +++ b/crates/benilla-app/src/ui_script/addons.rs @@ -4,12 +4,12 @@ //! FrameXML it loads outside `AddOn_Load` ([`super::manifest::load_ingame_ui`]) and gets no //! `ADDON_LOADED`. -use std::collections::{HashMap, HashSet}; +use std::collections::HashSet; use std::path::{Path, PathBuf}; use bevy::prelude::*; -use benilla_ui::script::{ScriptValue, UiScript}; +use benilla_ui::script::{EnableHash, ScriptValue, UiScript}; use benilla_ui::toc::Toc; use super::content; @@ -527,13 +527,14 @@ pub(crate) fn installed_rows() -> Vec { /// The reference's `ADDONSTATELIST` (`0xbe1bd0`): a node per character in `SMSG_CHAR_ENUM` order, /// rebuilt whole (`0x51f0b0` clears it, then callback `0x472300` fills a node per record through -/// `AddOnList_LoadCharacter 0x51ebe0`), each holding that character's explicit `AddOns.txt` rows. -/// A character with no file is an empty node, whose enable bit is answered from the others. +/// `AddOnList_LoadCharacter 0x51ebe0`), each holding that character's enable hash. A character +/// with no file is an empty node, whose enable bit is answered from the others. #[derive(Default)] pub(crate) struct EnableStore { - /// `(character, explicit rows)` in character-list order; names lowercased, as every compare in - /// the reference is `SStrCmpI`. - nodes: Vec<(String, HashMap)>, + /// The realm half of every node's file. + realm: String, + /// `(character, enable hash)` in character-list order. + nodes: Vec<(String, EnableHash)>, } impl EnableStore { @@ -548,26 +549,36 @@ impl EnableStore { .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 (`0x51eeef`). - .map(|(name, on)| (name.to_ascii_lowercase(), on)) - .collect(); - (character.clone(), hash) + (character.clone(), EnableHash::from_rows(rows)) }) .collect(); - Self { nodes } + Self { + realm: realm.to_string(), + nodes, + } + } + + /// A node per character, none with a row, as for characters with no file; for tests that + /// must not read the state folder. + #[cfg(test)] + pub(crate) fn blank(realm: &str, characters: &[String]) -> Self { + Self { + realm: realm.to_string(), + nodes: characters + .iter() + .map(|c| (c.clone(), EnableHash::default())) + .collect(), + } } /// `0x51e470(addon, NULL, useDefault = 0)`, the explicit-only aggregate over the nodes with a /// row (`0x51e5df je 0x51e60a`): `Some(v)` when all say `v` (the reference's 2 or 0), `None` /// when they are mixed or there are none. fn aggregate(&self, addon: &str) -> Option { - 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) { + if let Some(v) = hash.get(addon) { total += 1; on += usize::from(v); } @@ -581,7 +592,7 @@ impl EnableStore { } /// The bit a character gets, `0x51e470(addon, character, useDefault = 1)` (0 or 2 as a bool): - /// * an explicit row in their file wins; + /// * an explicit row in their hash wins; /// * a node with no row takes [`Self::aggregate`] (`0x51e5f0`'s self-recursion), then /// `## DefaultState` where that is undecided; /// * no node at all, or no character, takes `## DefaultState`: the walk passes each @@ -596,13 +607,33 @@ impl EnableStore { let Some(node) = character.and_then(|c| self.node(c)) else { return default_state; }; - match node.get(&addon.to_ascii_lowercase()) { - Some(&explicit) => explicit, + match node.get(addon) { + Some(explicit) => explicit, None => self.aggregate(addon).unwrap_or(default_state), } } - fn node(&self, character: &str) -> Option<&HashMap> { + /// The setter `0x51ea20`: `None` is every node (the glue's All), a name its one node; a name + /// with no node is a no-op, as the setter never creates one. + pub(crate) fn set(&mut self, addon: &str, character: Option<&str>, on: bool) { + for (name, hash) in &mut self.nodes { + if character.is_none_or(|c| name.eq_ignore_ascii_case(c)) { + hash.set(addon, on); + } + } + } + + /// The writer `0x51ef20`, glue `SaveAddOns` (`0x46d990`): each dirty node's file, rewritten + /// from its hash; a clean node is skipped (`0x51ef59`) and its file left as it is. + pub(crate) fn save(&mut self) { + for (name, hash) in &mut self.nodes { + if let Some(rows) = hash.take_dirty() { + write_enable_state(Some(&(self.realm.clone(), name.clone())), &rows); + } + } + } + + pub(crate) fn node(&self, character: &str) -> Option<&EnableHash> { self.nodes .iter() .find(|(name, _)| name.eq_ignore_ascii_case(character)) @@ -622,40 +653,25 @@ fn store_nodes(identity: Option<&(String, String)>, roster: &[String]) -> Vec, states: &[(String, bool)]) { +/// Write one character's enable hash as its whole file, a line per entry (`0x853968`), as the +/// writer `0x51ef20` creates it afresh (`CREATE_ALWAYS`). The hash holds every row the file had, +/// so an uninstalled addon's row survives and a name keeps its spelling. +pub(crate) fn write_enable_state(identity: Option<&(String, String)>, rows: &[(String, bool)]) { let Some(path) = enable_state_path(identity) else { return; // no character picked, or no state folder }; - let mut merged: Vec<(String, bool)> = std::fs::read(&path) - .ok() - .map(|b| parse_enable_state(&benilla_ui::source::decode(&b))) - .unwrap_or_default(); - for (name, on) in states { - match merged - .iter_mut() - .find(|(n, _)| n.eq_ignore_ascii_case(name)) - { - Some(row) => row.1 = *on, - None => merged.push((name.clone(), *on)), - } - } - match crate::local_state::write_atomic(&path, &render_enable_state(&merged)) { - Ok(()) => info!("addons: wrote {} ({} rows)", path.display(), merged.len()), + match crate::local_state::write_atomic(&path, &render_enable_state(rows)) { + Ok(()) => info!("addons: wrote {} ({} rows)", path.display(), rows.len()), Err(e) => warn!("addons: cannot write {}: {e}", path.display()), } } -/// Write the enable state back at shutdown, the reference's last step (`0x490c88`, after the -/// saved-variables files), through the AddOns screen's merging writer. +/// Write the current character's enable hash at shutdown, the reference's last step (`0x490c88`, +/// after the saved-variables files), only when a verb changed it this session. pub(super) fn save_enable_state(script: &UiScript, identity: Option<&(String, String)>) { - let states = script.addon_enable_states(); - if states.is_empty() { - return; // nothing registered: a glue-only run or a capture + if let Some(rows) = script.take_addon_enable_rows() { + write_enable_state(identity, &rows); } - write_enable_state(identity, &states); } /// Write every loaded addon's declared saved variables at shutdown (`0x490c83`, after the flat @@ -738,6 +754,13 @@ pub(super) fn load_third_party( crate::local_state::addon_saved_account_dir(), identity.and_then(|(r, c)| crate::local_state::addon_saved_character_dir(r, c)), ); + // The verbs write through the character's own hash, so logout saves only what they changed. + script.set_addon_enable_hash( + character + .and_then(|c| store.node(c)) + .cloned() + .unwrap_or_default(), + ); if addons.is_empty() { return Vec::new(); } @@ -1843,9 +1866,10 @@ mod tests { ); } - /// A row for an addon not installed now survives the AddOns screen's write. + /// A row for an addon not installed now survives the AddOns screen's write: the loader put it + /// in the hash, and the writer writes the hash. #[test] - fn the_addons_screen_write_merges_with_what_is_already_on_disk() { + fn the_addons_screen_write_keeps_the_rows_the_file_had() { let _l = crate::local_state::test_env::ENV_LOCK .lock() .unwrap_or_else(std::sync::PoisonError::into_inner); @@ -1855,7 +1879,10 @@ mod tests { std::fs::create_dir_all(path.parent().unwrap()).unwrap(); std::fs::write(&path, "Gone: disabled\nStays: enabled\n").unwrap(); - write_enable_state(Some(&id), &[("Stays".into(), false), ("New".into(), true)]); + let mut store = EnableStore::load("Realm", &["Char".to_string()]); + store.set("Stays", Some("char"), false); + store.set("New", None, true); + store.save(); let written = std::fs::read_to_string(&path).unwrap(); assert!( @@ -2365,16 +2392,10 @@ mod tests { save_enable_state(&script, Some(&id)); let written = std::fs::read_to_string(home.join("addons/Realm-Char.txt")).unwrap(); - // Registry order: the chain's Blizzard rows, then the folder's two (`0x51c777` before - // `0x51c78f`, tail-inserted), one line per registry row. - let mut expected = String::new(); - for a in chain_addons() { - expected.push_str(&format!("{}: enabled\n", a.name)); - } - expected.push_str("Drop: disabled\nKeep: enabled\n"); assert_eq!( - written, expected, - "the reference's own one-line-per-addon format" + written, "Drop: disabled\n", + "one line per hash entry (`0x51f006`), and the hash holds only the toggled addon: no \ + `Keep`, no `Blizzard_*` row" ); let mut next = UiScript::new().unwrap(); @@ -2387,6 +2408,94 @@ mod tests { let _ = std::fs::remove_dir_all(home.parent().unwrap()); } + /// A session whose verbs changed nothing leaves the node clean, and the writer skips a clean + /// node (`0x51ef59`): no file is created, and one that exists keeps its bytes, even through + /// a verb that sets a row to the value it already has and a toggle `ResetDisabledAddOns` + /// reverts (`0x48e830` reloads the hash clean, `0x51ec59`). + #[test] + fn a_session_that_toggles_nothing_writes_nothing() { + 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("untouched"); + write_addon(&home, "Keep", "## Interface: 11200\n", &[]); + write_addon(&home, "Off", "## Interface: 11200\n", &[]); + let fresh = ("Realm".to_string(), "Fresh".to_string()); + let mut script = UiScript::new().unwrap(); + let _ = load_third_party(&mut script, Some(&fresh), &[], true); + crate::ui_script::shutdown_ui_state(&mut script, Some(&fresh), true); + assert!( + !enable_state_path(Some(&fresh)).unwrap().exists(), + "a login and logout with no toggle creates no AddOns file" + ); + + let id = ("Realm".to_string(), "Char".to_string()); + let path = enable_state_path(Some(&id)).unwrap(); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + let before = "off: disabled\nstray line\n"; + std::fs::write(&path, before).unwrap(); + for (lua, why) in [ + ("DisableAddOn('Off')", "a row set to the value it has"), + ( + "DisableAddOn('Keep') ResetDisabledAddOns()", + "a toggle reverted", + ), + ] { + let mut script = UiScript::new().unwrap(); + let _ = load_third_party(&mut script, Some(&id), &[], true); + script.run(lua).unwrap(); + crate::ui_script::shutdown_ui_state(&mut script, Some(&id), true); + assert_eq!( + std::fs::read_to_string(&path).unwrap(), + before, + "{why} leaves the file as it was" + ); + } + let _ = std::fs::remove_dir_all(home.parent().unwrap()); + } + + /// The issue's example end to end, through the logout writer and the query `0x51e470`: Alice + /// and Bob have both played with `Shared` installed, then Bob turns it off. Alice, with no + /// row of her own, and a character created afterwards both follow Bob. + #[test] + fn a_character_that_never_chose_follows_the_realms_other_characters() { + 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("follow"); + write_addon( + &home, + "Shared", + "## Interface: 11200\nran.lua\n", + &[("ran.lua", "SharedRan = true")], + ); + let roster = ["Alice".to_string(), "Bob".to_string()]; + let session = |who: &str, roster: &[String], lua: &str| -> bool { + let id = ("Realm".to_string(), who.to_string()); + let mut script = UiScript::new().unwrap(); + script.set_screen_size(1024.0, 768.0); + let _ = load_third_party(&mut script, Some(&id), roster, true); + let ran = script.eval::("return SharedRan == true").unwrap(); + script.run(lua).unwrap(); + crate::ui_script::shutdown_ui_state(&mut script, Some(&id), true); + ran + }; + assert!(session("Alice", &roster, ""), "enabled by default"); + assert!(session("Bob", &roster, "DisableAddOn('Shared')")); + assert!( + !session("Alice", &roster, ""), + "Alice's first logout wrote no row, so she follows Bob's disable" + ); + let with_new = ["Alice".to_string(), "Bob".to_string(), "Newbie".to_string()]; + assert!( + !session("Newbie", &with_new, ""), + "and so does a character created afterwards" + ); + let _ = std::fs::remove_dir_all(home.parent().unwrap()); + } + // ───────────────────────────── saved variables ───────────────────────────── /// The account file, then the per-character one (`0x51f4b5`, `0x51f53b`), then `ADDON_LOADED` diff --git a/crates/benilla-ui/src/script/addon.rs b/crates/benilla-ui/src/script/addon.rs index 5ee3373c..89b98662 100644 --- a/crates/benilla-ui/src/script/addon.rs +++ b/crates/benilla-ui/src/script/addon.rs @@ -142,9 +142,12 @@ fn row_of(model: &Model, key: &AddonKey) -> Option { } /// The enable setter (`0x51ea20`), the one write every enable verb makes, for the current -/// character's row. +/// character's row: the registry bit the gate reads, and the character's enable hash, which the +/// setter dirties only when the row changes or is new. fn set_enabled(model: &mut Model, i: usize, on: bool) { model.addons[i].enabled = on; + let name = model.addons[i].name.clone(); + model.addon_enable.set(&name, on); } /// The registry as [`super::addon_gate`] rows, the one input every verb's verdict reads. @@ -353,8 +356,8 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { )?; } - // `0x48e830` reloads the enable state from `AddOns.txt`, which is unchanged until the shutdown - // write, so reverting to the state as registered is the same thing. + // `0x48e830` reloads the enable hash from `AddOns.txt`, clean (`0x51ec59`), and the file is + // unchanged until the shutdown write, so reverting to the state as registered is the same. g.set( "ResetDisabledAddOns", lua.create_function(|lua, ()| { @@ -362,6 +365,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { for a in &mut model.addons { a.enabled = a.saved_enabled; } + model.addon_enable = model.addon_enable_saved.clone(); Ok(()) })?, )?; @@ -722,12 +726,17 @@ impl super::UiScript { } } - /// `(name, enabled)` per registered addon, in order: what the host writes to `AddOns.txt`. - pub fn addon_enable_states(&self) -> Vec<(String, bool)> { - self.model_ref() - .addons - .iter() - .map(|a| (a.name.clone(), a.enabled)) - .collect() + /// Seat the current character's enable hash as the loader read it from `AddOns.txt` + /// (`0x51ebe0`, re-read at every UI boot, `0x48ff2a`). + pub fn set_addon_enable_hash(&mut self, hash: super::EnableHash) { + let mut model = self.model_mut(); + model.addon_enable_saved = hash.clone(); + model.addon_enable = hash; + } + + /// The shutdown writer's rows (`0x490c88` into `0x51ef20`): the current character's whole + /// enable hash when a verb changed it this session, else `None`, and nothing is written. + pub fn take_addon_enable_rows(&self) -> Option> { + self.model_mut().addon_enable.take_dirty() } } diff --git a/crates/benilla-ui/src/script/addon_enable.rs b/crates/benilla-ui/src/script/addon_enable.rs new file mode 100644 index 00000000..7833a996 --- /dev/null +++ b/crates/benilla-ui/src/script/addon_enable.rs @@ -0,0 +1,135 @@ +//! One character's addon enable hash, the table an `ADDONSTATELIST` node (`0xbe1bd0`) holds at +//! `+0x10`, with the node's dirty byte (`+0xc`). Both the glue AddOns screen and the in-game verbs +//! write through its setter, and the writer `0x51ef20` saves a node only while it is dirty, one +//! line per entry, so a character that toggled nothing writes nothing and one that never set an +//! addon keeps inheriting what the realm's other characters chose (`0x51e470`). + +use std::collections::HashMap; + +/// A character's explicit enable rows: those `AddOnList_LoadCharacter 0x51ebe0` read from its +/// `AddOns.txt`, whether or not that addon is installed now, plus those the setter `0x51ea20` put +/// there since. An addon with no row here is not the character's choice, and the query answers it +/// from the other characters. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct EnableHash { + /// In insertion order, each name in the spelling that first put it here. + rows: Vec<(String, bool)>, + /// Lowercased name to its row: every compare in the reference is `SStrCmpI` (`0x64a4c0`). + index: HashMap, + /// The node's `+0xc`: set when a setter call changes the hash (`0x51ebbc`), cleared by a + /// reload (`0x51ec59`) and by the writer as it saves (`0x51ef68`). + dirty: bool, +} + +impl EnableHash { + /// The loader's hash, clean. A name on two lines keeps its first spelling and takes the last + /// value, as the reload looks each one up and overwrites its byte (`0x51ee31`, `0x51eeef`). + pub fn from_rows(rows: impl IntoIterator) -> Self { + let mut hash = Self::default(); + for (name, on) in rows { + hash.put(name, on); + } + hash + } + + /// The row for `name`, if this character has one. + pub fn get(&self, name: &str) -> Option { + self.index + .get(&name.to_ascii_lowercase()) + .map(|&i| self.rows[i].1) + } + + /// The setter `0x51ea20` on this node: a row with the same value is left alone + /// (`0x51ebab`-`0x51ebb1`); a changed row is stored and a missing one inserted, and either + /// dirties the node (`0x51ebb3`-`0x51ebbc`, `0x51eb62`). + pub fn set(&mut self, name: &str, on: bool) { + if self.get(name) == Some(on) { + return; + } + self.put(name.to_string(), on); + self.dirty = true; + } + + /// Whether the writer would save this node. + pub fn is_dirty(&self) -> bool { + self.dirty + } + + /// Every row, in order. + pub fn rows(&self) -> &[(String, bool)] { + &self.rows + } + + /// The writer's view of the node (`0x51ef59`-`0x51ef68`): every row when dirty, clearing the + /// flag; `None` when clean, which writes nothing. + pub fn take_dirty(&mut self) -> Option> { + std::mem::take(&mut self.dirty).then(|| self.rows.clone()) + } + + fn put(&mut self, name: String, on: bool) { + match self.index.get(&name.to_ascii_lowercase()) { + Some(&i) => self.rows[i].1 = on, + None => { + self.index + .insert(name.to_ascii_lowercase(), self.rows.len()); + self.rows.push((name, on)); + } + } + } +} + +#[cfg(test)] +mod tests { + use super::EnableHash; + + fn loaded() -> EnableHash { + EnableHash::from_rows([("Kept".to_string(), true), ("Off".to_string(), false)]) + } + + #[test] + fn a_loaded_hash_is_clean_and_keeps_the_first_spelling_with_the_last_value() { + let h = EnableHash::from_rows([ + ("MyAddon".to_string(), true), + ("myaddon".to_string(), false), + ]); + assert!(!h.is_dirty()); + assert_eq!(h.rows(), &[("MyAddon".to_string(), false)]); + assert_eq!(h.get("MYADDON"), Some(false)); + } + + #[test] + fn setting_a_row_to_its_own_value_does_not_dirty() { + let mut h = loaded(); + h.set("kept", true); + h.set("OFF", false); + assert!(!h.is_dirty()); + assert_eq!(h.clone().take_dirty(), None, "a clean node writes nothing"); + } + + #[test] + fn a_changed_row_dirties_and_the_writer_takes_every_row_once() { + let mut h = loaded(); + h.set("Kept", false); + assert!(h.is_dirty()); + assert_eq!( + h.take_dirty(), + Some(vec![ + ("Kept".to_string(), false), + ("Off".to_string(), false) + ]) + ); + assert!(!h.is_dirty(), "the writer clears the flag (`0x51ef68`)"); + assert_eq!(h.take_dirty(), None); + } + + /// A miss inserts even when the value matches what the character inherited: the setter has + /// no view of the aggregate, only of this node's table. + #[test] + fn a_missing_row_is_inserted_and_dirties() { + let mut h = loaded(); + h.set("New", true); + assert!(h.is_dirty()); + assert_eq!(h.get("new"), Some(true)); + assert_eq!(h.rows().len(), 3); + } +} diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 4d6e471b..0c30fe78 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -49,6 +49,7 @@ mod editbox; pub mod instance; pub(crate) use editbox::{adopt_text_region, editbox_text_region_wrapper}; pub(crate) mod addon; +mod addon_enable; pub mod addon_gate; mod client; mod cvars; @@ -140,6 +141,7 @@ mod worn_display; pub use action::{ActionSlot, ActionState, ActionUse}; pub use addon::AddOnInfo; +pub use addon_enable::EnableHash; pub use addon_message::{AddonDistribution, AddonSend}; pub use auction::{ AuctionBid, AuctionCategory, AuctionItemRow, AuctionListState, AuctionQuery, diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index 18f9fe4e..0b709fca 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -62,6 +62,11 @@ pub(crate) struct Model { /// Lowercased names `SMSG_ADDON_INFO` marked `status = 2`; `None` until the reply arrives, and /// until then `GetNumAddOns()` is 0, as in the reference (`[0xbe1b90]`, reset at `0x51fad1`). pub(crate) addon_info_hidden: Option>, + /// The current character's enable hash, which every enable verb writes through and the + /// shutdown writer (`0x490c88` into `0x51ef20`) saves only while dirty. + pub(crate) addon_enable: super::EnableHash, + /// The hash as the host read it from the file, what `ResetDisabledAddOns` reloads (`0x48e830`). + pub(crate) addon_enable_saved: super::EnableHash, /// The AddOns folder, so `LoadAddOn` can read an addon's files from inside a Lua binding. pub(crate) addons_root: Option, /// The host's reader for chain-sourced addon files; without one such an addon is `MISSING`. @@ -967,6 +972,8 @@ impl Model { addons: Vec::new(), addon_index: Vec::new(), addon_info_hidden: None, + addon_enable: Default::default(), + addon_enable_saved: Default::default(), addons_root: None, addons_chain_reader: None, measurer: None, diff --git a/crates/benilla-ui/src/script/tests/addon_index_space.rs b/crates/benilla-ui/src/script/tests/addon_index_space.rs index d2a50c82..efc4a0e4 100644 --- a/crates/benilla-ui/src/script/tests/addon_index_space.rs +++ b/crates/benilla-ui/src/script/tests/addon_index_space.rs @@ -205,3 +205,20 @@ fn the_all_verbs_do_nothing_before_the_server_answers() { s.run("DisableAllAddOns()").unwrap(); assert!(enabled(&s, "Alpha") && enabled(&s, "Mike") && enabled(&s, "Zulu")); } + +/// What a disable-all leaves for the shutdown writer (`0x51ef20`): the setter's one new row per +/// visible addon, and none for the hidden one, whose row it never set. +#[test] +fn disable_all_writes_rows_for_the_visible_addons_only() { + let s = with_hidden_blizzard(true); + assert_eq!( + s.take_addon_enable_rows(), + None, + "nothing set, nothing to write" + ); + s.run("DisableAllAddOns()").unwrap(); + assert_eq!( + s.take_addon_enable_rows(), + Some(vec![("Probe".to_string(), false)]) + ); +}