log: the session's warnings are 330 down to 129, and the 201 that went were ours
A full inventory of the player's 2026-10-18 session: **330 WARN lines, 67 distinct
shapes, and not one ERROR**. Read one at a time against the code and the
reference, most of what remained was the client complaining about itself. Three
causes, measured by replaying the new rules over that same log: **330 → 129**.
**A label is not a key (33 lines).** `text=` resolves as a GlobalStrings key and a
miss shows the raw value, which is faithful; the warning on a key-shaped miss is
ours. But a miss is already a string no locale defines, so a single all-caps word
is a label the author typed — `OK`, `UP`, `DOWN`, `MT`, `TPS`, `TOP`, `BOTTOM`,
`WIM`, `TWA`, 42 lines of them in one session, every one rendering exactly as the
reference renders it. The predicate now wants an underscore, which keeps the shape
that cannot be anything else: `ATSW_ALLREAGENTLISTFRAME_CHN`, an addon's own locale
row missing from the locale it shipped, which really does paint its key name on
screen. The shipped-XML sweep keeps its own broader rule, where `text="DELETE"`
must resolve and a collision is a bug either way; its comment now says so.
**Prose is not a missing file (12 lines, and phantom load failures).** A `.toc` is
read line by line, `#` opening a metadata line and everything else a filename, so
a wrapped `## Notes:` paragraph and the `--` section headers many addons write
become entries that no file answers. The reference opens them, fails, and says
nothing. We called each one a load failure, which put it in `/errors` and counted
it in the "N addon load failures" line at every login — ColorPickerPlus shipping
"two files it does not contain" that are really one two-line sentence. A line that
cannot name a file is now a debug line; one that looks like a file and is absent
stays a warning, because then the package really is short a piece (BigWigs'
`Raids\Other\Nerubian.lua`). The addon harness shares the predicate, so its
package audit stops making the same claim.
**One cause speaks once (168 lines → 9).** The stock inspect panel's sixteen slots
each name a frame `Blizzard_InspectUI` has never been asked to build, and
QuickHeal's one undefined template is inherited ten times — per UI load, so 78
lines a session for two causes. A document's log warnings now collapse by shape:
the first two keep their detail, the rest become one "N more like …" line, and
`/errors` still gets every one, because the panel is where detail belongs.
Three shapes were checked and deliberately left, because the client is right:
`SetScript("OnInputLanguageChanged")` must keep raising (an unfired script kind
raises by this project's own rule, and nothing can fire an IME edge here); the
`SetOwner: unknown anchor 'BOTTOMRIGHT'` line is TurtleWoW's own
`Turtle_ShopUI.lua:530`, where our mode-0 fallback already matches the reference
and the line is already once per session; and a `.toc`-listed `Bindings.xml` walks
as FrameXML with `<Binding>` an unknown frame type, which the reference logs too
(the bindings themselves load by their own path and work).
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
@@ -1574,6 +1574,14 @@ fn load_addon_files(script: &UiScript, root: &Path, name: &str, toc: &Toc) -> Fi
|
||||
for file in &toc.files {
|
||||
let path = benilla_ui::loader::join_ref(&base, file);
|
||||
let Some(bytes) = read_under(root, &path) else {
|
||||
// A `.toc` line that cannot name a file is prose, not an incomplete package: the
|
||||
// comments addons write and the wrapped `## Notes:` paragraphs
|
||||
// ([`crate::ui_script::addons::is_file_entry`]). Counting them told the reader that
|
||||
// ColorPickerPlus ships two files it does not contain, when what it ships is a
|
||||
// two-line sentence.
|
||||
if !crate::ui_script::addons::is_file_entry(file) {
|
||||
continue;
|
||||
}
|
||||
errors.push(format!("{file}: not found"));
|
||||
// Whose package is incomplete: `join_ref` has collapsed the `..`s as the 1.12 client
|
||||
// does (`0x6ede10`), so a path still inside the addon's folder is its own manifest
|
||||
|
||||
@@ -121,6 +121,15 @@ impl Addon {
|
||||
// Resolved once into the source's path space, for `read` and the loader alike.
|
||||
let path = benilla_ui::loader::join_ref(&self.prefix(), file);
|
||||
let Some(bytes) = self.read(&path) else {
|
||||
// Prose, not a file: a `.toc` line that cannot name one ([`is_file_entry`]).
|
||||
if !is_file_entry(file) {
|
||||
debug!(
|
||||
"ui_script: {}/{file}: not a file reference — a `.toc` line that is a \
|
||||
comment or a wrapped metadata paragraph, as the reference also reads it",
|
||||
self.name
|
||||
);
|
||||
continue;
|
||||
}
|
||||
let e = format!("{}/{file}: not found", self.name);
|
||||
// Severity follows whose manifest is wrong. Ours (the builtin, or a `benilla.toc`
|
||||
// line the player's chain lacks) is an ERROR. A player's addon is the package's
|
||||
@@ -168,11 +177,35 @@ impl Addon {
|
||||
for t in &report.traces {
|
||||
info!("ui_script({}/{file}): {t}", self.name);
|
||||
}
|
||||
// One document's warnings collapse by shape in the LOG, because a single cause
|
||||
// speaks many times: the stock inspect panel's sixteen slots each name a frame
|
||||
// `Blizzard_InspectUI` has not been asked to build, and QuickHeal's one undefined
|
||||
// template is inherited ten times. The shape is the message without its trailing
|
||||
// identifier, the first two of each keep their detail, and the rest become a count —
|
||||
// 78 lines of the player's session were four causes. `/errors` still gets every one:
|
||||
// the panel is where the detail belongs.
|
||||
const PER_SHAPE: usize = 2;
|
||||
// Ordered, so the tail lines read the same run to run.
|
||||
let mut per_shape: std::collections::BTreeMap<&str, usize> =
|
||||
std::collections::BTreeMap::new();
|
||||
for w in &report.warnings {
|
||||
warn!("ui_script({}/{file}): {w}", self.name);
|
||||
let shape = warning_shape(w);
|
||||
let seen = per_shape.entry(shape).or_default();
|
||||
*seen += 1;
|
||||
if *seen <= PER_SHAPE {
|
||||
warn!("ui_script({}/{file}): {w}", self.name);
|
||||
}
|
||||
// Retained for `/errors` too, here because only this caller knows the file.
|
||||
script.report_warning(&format!("{}/{file}: {w}", self.name));
|
||||
}
|
||||
for (shape, n) in per_shape.iter().filter(|(_, n)| **n > PER_SHAPE) {
|
||||
warn!(
|
||||
"ui_script({}/{file}): {} more like \"{shape}: …\" ({n} in all) — /errors \
|
||||
lists them",
|
||||
self.name,
|
||||
n - PER_SHAPE
|
||||
);
|
||||
}
|
||||
// A file the document names and the provider lacks is a `.toc` line naming no file:
|
||||
// never a script error, as the reference logs `Couldn't open %s` and carries on, but a
|
||||
// `failures` entry, so our boot tests catch one in a `benilla.toc` document.
|
||||
@@ -201,6 +234,40 @@ impl Addon {
|
||||
}
|
||||
}
|
||||
|
||||
/// A document warning's shape: the message without its trailing identifier, which is what makes
|
||||
/// sixteen "Couldn't find relative frame: Inspect<Slot>Slot" lines one cause. A message with no
|
||||
/// `": "` is its own shape, which is how ten identical "unknown template" lines collapse too.
|
||||
fn warning_shape(warning: &str) -> &str {
|
||||
warning.rsplit_once(": ").map_or(warning, |(head, _)| head)
|
||||
}
|
||||
|
||||
/// Whether a `.toc` line is a file reference at all, asked only of a line that resolved to
|
||||
/// nothing. A `.toc` is read line by line — `#` opens a metadata line and everything else is a
|
||||
/// filename — so a wrapped `## Notes:` paragraph and the `--` comments many addons write become
|
||||
/// entries, and the reference tries to open them, fails, and says nothing. Two of the player's own
|
||||
/// addons do it: `ColorPickerPlus`'s Notes wrap onto two bare lines, `DifficultBulletinBoard`
|
||||
/// heads each section with `-- API files`.
|
||||
///
|
||||
/// Those are not missing files, so they are not load failures: counting them put phantom entries
|
||||
/// in `/errors` and inflated the "N addon load failures" line at every login. A line that *is*
|
||||
/// shaped like a file and absent stays a warning, because then the addon really is missing a
|
||||
/// piece (`BigWigs`'s `Raids\Other\Nerubian.lua`).
|
||||
pub(crate) fn is_file_entry(entry: &str) -> bool {
|
||||
let name = entry.trim();
|
||||
!name.starts_with("--")
|
||||
&& name
|
||||
.rsplit(['/', '\\'])
|
||||
.next()
|
||||
.unwrap_or(name)
|
||||
.rsplit_once('.')
|
||||
.is_some_and(|(stem, ext)| {
|
||||
!stem.is_empty()
|
||||
&& ["lua", "xml", "toc"]
|
||||
.iter()
|
||||
.any(|k| ext.eq_ignore_ascii_case(k))
|
||||
})
|
||||
}
|
||||
|
||||
/// Whether a manifest entry is a Lua chunk: by extension, case-insensitively, with `\` a
|
||||
/// separator like `/` (a `.toc` is written for a Windows client).
|
||||
fn is_lua(entry: &str) -> bool {
|
||||
@@ -916,6 +983,53 @@ impl Walk {
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
/// A `.toc` line that cannot name a file is prose, and prose is not a missing file. The cases
|
||||
/// are the real ones: `ColorPickerPlus`'s wrapped `## Notes:` and
|
||||
/// `DifficultBulletinBoard`'s `--` section headers against the files they sit between.
|
||||
#[test]
|
||||
fn a_toc_line_that_is_prose_is_not_a_file_reference() {
|
||||
for file in [
|
||||
"DifficultBulletinBoard.lua",
|
||||
"env\\defaults.lua",
|
||||
"libs\\AceOO-2.0\\AceOO-2.0.lua",
|
||||
"FishingOutfitFrame.xml",
|
||||
"Raids\\Other\\Nerubian.lua",
|
||||
"Bindings.XML",
|
||||
] {
|
||||
assert!(super::is_file_entry(file), "{file} names a file");
|
||||
}
|
||||
for prose in [
|
||||
"-- API files",
|
||||
"-- Environment files (static data, constants, exclusion rules)",
|
||||
"standard Color Picker to provide text entry for colors (RGB and hex values),",
|
||||
"copy and paste, and starting color and copied color swatches.",
|
||||
"",
|
||||
".lua",
|
||||
] {
|
||||
assert!(!super::is_file_entry(prose), "{prose:?} names no file");
|
||||
}
|
||||
}
|
||||
|
||||
/// The log's collapse key: one cause, however many frames it names.
|
||||
#[test]
|
||||
fn repeated_warnings_share_a_shape_and_distinct_causes_do_not() {
|
||||
let shape = super::warning_shape;
|
||||
let inspect = [
|
||||
"BCP_Inspect: Couldn't find relative frame: InspectHeadSlot",
|
||||
"BCP_Inspect: Couldn't find relative frame: InspectFeetSlot",
|
||||
];
|
||||
assert_eq!(shape(inspect[0]), shape(inspect[1]));
|
||||
assert_eq!(
|
||||
shape(inspect[0]),
|
||||
"BCP_Inspect: Couldn't find relative frame"
|
||||
);
|
||||
// No trailing identifier: the whole message is the shape, so ten of them still collapse.
|
||||
let template = "unknown template 'QuickHealMeleeDPSListEntryTemplate' referenced by \
|
||||
inherits; skipping";
|
||||
assert_eq!(shape(template), template);
|
||||
assert_ne!(shape(template), shape(inspect[0]));
|
||||
}
|
||||
|
||||
/// NTFS `$I30` order, not byte order: case folds away and `_` (0x5F) sorts after `Z` (0x5A).
|
||||
#[test]
|
||||
fn the_walk_orders_names_the_way_ntfs_lists_a_directory() {
|
||||
|
||||
@@ -477,7 +477,8 @@ fn every_shipped_text_attribute_answers_against_the_real_global_strings() {
|
||||
let s = benilla_ui::script::UiScript::new().expect("VM");
|
||||
s.run(&String::from_utf8_lossy(&src)).expect("runs clean");
|
||||
|
||||
// The loader's key-shape predicate, restated because it is private.
|
||||
// The key shape, deliberately broader than the loader's runtime predicate (which ignores a
|
||||
// miss with no underscore, a label rather than a key): this sweep is over our own files.
|
||||
let key_shaped = |v: &str| {
|
||||
v.len() >= 2
|
||||
&& v.chars().any(|c| c.is_ascii_uppercase())
|
||||
@@ -524,7 +525,11 @@ fn every_shipped_text_attribute_answers_against_the_real_global_strings() {
|
||||
/// (`GlobalStrings.lua:898`). The `text=` half is the sweep above.
|
||||
#[test]
|
||||
fn no_shipped_script_sets_a_global_string_key_as_display_text() {
|
||||
/// `loader::is_global_string_key`'s predicate, restated because it is private.
|
||||
/// The key shape, deliberately **broader** than `loader::is_global_string_key`: that one gates
|
||||
/// a runtime warning and so ignores a miss with no underscore (`text="OK"` is a label an addon
|
||||
/// author typed). Here every all-caps word counts, because this sweep is over our OWN shipped
|
||||
/// files, where `text="DELETE"` must resolve and a literal that collides with a real key is a
|
||||
/// bug either way. Restated rather than shared, for that reason and because it is private.
|
||||
fn key_shaped(s: &str) -> bool {
|
||||
s.len() >= 2
|
||||
&& s.chars().any(|c| c.is_ascii_uppercase())
|
||||
|
||||
@@ -790,10 +790,21 @@ pub(super) fn children_named_any<'a>(
|
||||
.filter(move |c| tags.iter().any(|t| c.tag.eq_ignore_ascii_case(t)))
|
||||
}
|
||||
|
||||
/// Whether a `text=` value is shaped like a GlobalStrings key: `SCREAMING_SNAKE` with a letter, two
|
||||
/// characters or more (a one-character `text="X"` is a glyph).
|
||||
/// Whether a `text=` value that resolved to nothing is shaped like a GlobalStrings key worth
|
||||
/// warning about: `SCREAMING_SNAKE` **with an underscore**, a letter, and two characters or more.
|
||||
///
|
||||
/// The underscore is the whole discrimination, and it is about the miss rather than the shape. A
|
||||
/// key that exists never reaches here ([`Loader::resolve_text`] returns its value first), so every
|
||||
/// candidate is already a string no `GlobalStrings.lua` defines — and a single all-caps word that
|
||||
/// no locale defines is a label the author typed, not a key they got wrong. One session of the
|
||||
/// player's own addons produced 42 such lines from `OK`, `UP`, `DOWN`, `MT`, `TPS`, `TOP`,
|
||||
/// `BOTTOM`, `WIM` and `TWA`, every one a literal the reference also shows as typed. With the
|
||||
/// underscore required, what is left is the shape that cannot be anything but a key:
|
||||
/// `ATSW_ALLREAGENTLISTFRAME_CHN`, an addon's own locale row missing from the locale it shipped,
|
||||
/// which does render as its own key name on screen.
|
||||
fn is_global_string_key(s: &str) -> bool {
|
||||
s.len() >= 2
|
||||
&& s.contains('_')
|
||||
&& s.chars().any(|c| c.is_ascii_uppercase())
|
||||
&& s.chars()
|
||||
.all(|c| c.is_ascii_uppercase() || c.is_ascii_digit() || c == '_')
|
||||
|
||||
@@ -2052,11 +2052,35 @@ mod loader_tests {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn key_shape_is_screaming_snake_of_two_or_more() {
|
||||
for yes in ["DELETE", "EXIT_GAME", "CHARACTER_POINTS1_COLON", "AB", "A1"] {
|
||||
fn key_shape_is_screaming_snake_with_an_underscore() {
|
||||
for yes in [
|
||||
"EXIT_GAME",
|
||||
"CHARACTER_POINTS1_COLON",
|
||||
"ATSW_ALLREAGENTLISTFRAME_CHN",
|
||||
"A_1",
|
||||
] {
|
||||
assert!(is_global_string_key(yes), "{yes} is key-shaped");
|
||||
}
|
||||
for no in ["X", "", "Send Mail", "No results found.", "Okay", "1", "12"] {
|
||||
// The last nine are the real labels one session of the player's addons warned about:
|
||||
// uppercase, keyless, and exactly what the author meant to show.
|
||||
for no in [
|
||||
"X",
|
||||
"",
|
||||
"Send Mail",
|
||||
"No results found.",
|
||||
"Okay",
|
||||
"1",
|
||||
"12",
|
||||
"OK",
|
||||
"UP",
|
||||
"DOWN",
|
||||
"MT",
|
||||
"TPS",
|
||||
"TOP",
|
||||
"BOTTOM",
|
||||
"WIM",
|
||||
"TWA",
|
||||
] {
|
||||
assert!(!is_global_string_key(no), "{no} is not key-shaped");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user