From a753a8b55dace33f9bb00360b2975740f532f810 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 11 Sep 2026 20:50:59 +0700 Subject: [PATCH] an addon's font draws in its own face, and Bartender2 keeps its profile - text shapes in the face an addon asked for: MikScrollingBattleText draws in its own Porky instead of a system sans, and Arial Narrow and Friz Quadrata are no longer drawn by the Mac's own fonts (2123) - Bartender2 and Cartographer stop resetting their profile at every login: a stack trace opens on the caller, where AceDB reads its addon's folder; a font's reported height is a number before its face resolves, which Tablet-2.0 does arithmetic on (2121, 2129) - a button highlight built from a texture object draws, so Bongos' bar-drag button, _Nameplates' mouseover glow and Quiver's option rows show theirs; clearing a state texture removes it instead of drawing it in every state (2124) - a toc's repeated SavedVariables lines add up, so all 25 of CT_RaidAssist's saved globals persist instead of seven; colour-mapped TGA icons decode, so FonzAppraiser's and FuBar_WeaponRebuffFu's draw (2126, 2128) - MikScrollingBattleText's DoT lines are back with their school tints, and the school word is capitalised; a chat line with no colour stays white, and the short colour-id form of adding a message works (2127, 2125) Co-Authored-By: Claude Fable 5.1 --- crates/benilla-app/assets/ui/OptionsFrame.xml | 2 +- crates/benilla-app/assets/ui/benilla.toc | 2 +- .../benilla-app/src/net/apply/combat_chat.rs | 78 ++++++- crates/benilla-app/src/ui_chat/combat.rs | 53 ++++- .../benilla-app/src/ui_chat/combat/tests.rs | 9 +- crates/benilla-app/src/ui_chat/settings.rs | 145 +++++++------ crates/benilla-app/src/ui_chat/tests.rs | 18 +- crates/benilla-app/src/ui_script/lifecycle.rs | 44 +++- .../benilla-app/src/ui_script/shape_gate.rs | 124 +++++++++-- .../benilla-app/src/ui_text/engine/faces.rs | 38 +++- crates/benilla-app/src/ui_text/engine/mod.rs | 196 +++++++++++++++-- crates/benilla-assets/src/world_assets/mod.rs | 58 +++-- crates/benilla-formats/src/tga.rs | 152 +++++++++++-- crates/benilla-ui/src/script/button.rs | 103 ++++++++- crates/benilla-ui/src/script/font.rs | 11 +- crates/benilla-ui/src/script/font_block.rs | 15 +- crates/benilla-ui/src/script/mail.rs | 45 ++-- .../src/script/messageframe/scrolling.rs | 94 ++++++-- .../src/script/object/events_regions.rs | 7 +- crates/benilla-ui/src/script/region/paint.rs | 36 +++- crates/benilla-ui/src/script/stdlib.rs | 202 +++++++++++++++--- crates/benilla-ui/src/script/tests/button.rs | 78 +++++++ crates/benilla-ui/src/script/tests/regions.rs | 53 +++++ crates/benilla-ui/src/script/tests/stdlib.rs | 137 +++++++++++- crates/benilla-ui/src/toc.rs | 157 ++++++++++---- .../src/widget/kinds/messageframe.rs | 24 ++- 26 files changed, 1579 insertions(+), 302 deletions(-) diff --git a/crates/benilla-app/assets/ui/OptionsFrame.xml b/crates/benilla-app/assets/ui/OptionsFrame.xml index d70f7070..f13c88c5 100644 --- a/crates/benilla-app/assets/ui/OptionsFrame.xml +++ b/crates/benilla-app/assets/ui/OptionsFrame.xml @@ -200,7 +200,7 @@ -- thing instead of an alias onto us. Three things this file used to carry are the stock -- file's, and are gone from here rather than duplicated: -- - -- · **The 38 options uvars.** `UIOptionsFrame_Init` (stock UIOptionsFrame.lua l.93-185) + -- · **The 30 options uvars.** `UIOptionsFrame_Init` (stock UIOptionsFrame.lua l.93-185) -- assigns every one its factory value and `RegisterForSave`s it — SIMPLE_CHAT, -- CHAT_LOCKED, REMOVE_CHAT_DELAY, SHOW_NEWBIE_TIPS, LOCK_ACTIONBAR, -- SHOW_BUFF_DURATIONS, ALWAYS_SHOW_MULTIBARS, SHOW_PARTY_PETS, QUEST_FADING_DISABLE, diff --git a/crates/benilla-app/assets/ui/benilla.toc b/crates/benilla-app/assets/ui/benilla.toc index d0edeef4..0f686215 100644 --- a/crates/benilla-app/assets/ui/benilla.toc +++ b/crates/benilla-app/assets/ui/benilla.toc @@ -442,7 +442,7 @@ Interface\FrameXML\OptionsFrame.lua # honest answer — and it is free, because the file is the player's own. # # IT IS ALSO WHERE THE REFERENCE KEEPS ITS OPTIONS STATE, so loading it retires three duplications -# our own window carried (2115): `UIOptionsFrame_Init` assigns and `RegisterForSave`s all 38 +# our own window carried (2115): `UIOptionsFrame_Init` assigns and `RegisterForSave`s all 30 # options uvars; `UIOptionsFrameCheckButtons` is declared here — the table stock # `MultiActionBars.lua:10` writes five rows into at ITS load, which is why this row must stay # above that one; and `UpdateNameplates` is this file's function. diff --git a/crates/benilla-app/src/net/apply/combat_chat.rs b/crates/benilla-app/src/net/apply/combat_chat.rs index 370dbb11..6eb0282d 100644 --- a/crates/benilla-app/src/net/apply/combat_chat.rs +++ b/crates/benilla-app/src/net/apply/combat_chat.rs @@ -178,7 +178,10 @@ pub(super) fn spell_damage_log( ); } if s.periodic { - let Some(kind) = combat::periodic_kind(attacker, false) else { + // **The TARGET's class, not the caster's** (decision 2127): this leg lands in the shared + // `PERIODICAURADAMAGE` formatter `0x628100`, whose msg-id selector takes `outClassB` + // (`0x628235`), and B is the victim at every call site. + let Some(kind) = combat::periodic_kind(victim, false) else { return; }; let fills = Fills { @@ -462,7 +465,7 @@ pub(super) fn periodic_aura_log( ) } }; - let Some(kind) = combat::periodic_kind(caster, buff) else { + let Some(kind) = combat::periodic_kind(periodic_subject(tick, caster, target), buff) else { continue; }; queue( @@ -1036,6 +1039,24 @@ fn power_gain(power: u32, amount: u32) -> i64 { i64::from(amount) / power_divisor(power) } +/// Which endpoint a periodic tick's formatter hands its **msg-id selector** — the choice that +/// decides whether a line types as `SPELL_PERIODIC_SELF_*` or `SPELL_PERIODIC_CREATURE_*` +/// (decision 2127). +/// +/// It is a function, exhaustive over the wire enum, because the four periodic formatters disagree +/// and one shared `periodic_kind(caster, …)` was wrong for half of them: `0x628100` +/// (`PERIODICAURADAMAGE`) and `0x627240` (`PERIODICAURAHEAL`) hand the selector `outClassB`, the +/// **target** (`0x628235`, `0x62732c`); `0x627520` (`POWERGAIN`) and `0x627930` +/// (`SPELLPOWERLEECH`/`…DRAIN`) hand it `outClassA`, the **caster** (`0x6275fb`, +/// `0x627a0f`/`0x627a3a`). A new tick kind has to answer this question rather than inherit an +/// answer. +fn periodic_subject(tick: &PeriodicTick, caster: UnitClass, target: UnitClass) -> UnitClass { + match tick { + PeriodicTick::Damage { .. } | PeriodicTick::Heal { .. } => target, + PeriodicTick::Energize { .. } | PeriodicTick::ManaLeech { .. } => caster, + } +} + /// The `(drained, gained)` pair a leech/drain sentence words, or `None` when the line is dropped. /// /// One function because the reference has one formatter: `0x627930` serves both @@ -1228,7 +1249,58 @@ fn queue_named( #[cfg(test)] mod tests { - use super::{leech_figures, power_gain}; + use super::{leech_figures, periodic_subject, power_gain, PeriodicTick, UnitClass}; + + /// **A periodic line is typed off the endpoint its own formatter reads** (decision 2127). + /// + /// The report: MikScrollingBattleText showed no DoT lines at all, in either direction. Every + /// periodic tick took `periodic_kind(caster, …)`, so a creature's poison ticking you came out + /// as `CHAT_MSG_SPELL_PERIODIC_CREATURE_DAMAGE` where the reference sends + /// `…_PERIODIC_SELF_DAMAGE`, and your own DoT on a creature came out the other way round — + /// the two swapped. MSBT matches a different GlobalString pattern per event name, so with + /// both events swapped neither line matched anything and both were dropped. + #[test] + fn a_periodic_damage_or_heal_tick_is_typed_off_the_target() { + let dmg = PeriodicTick::Damage { + amount: 12, + school: 3, + absorb: 0, + resist: 0, + }; + let heal = PeriodicTick::Heal { amount: 12 }; + // A creature's DoT ticking me: the subject is ME, so the line types SELF. + assert_eq!( + periodic_subject(&dmg, UnitClass::Creature, UnitClass::Me), + UnitClass::Me + ); + // My DoT ticking a creature: the subject is the CREATURE. + assert_eq!( + periodic_subject(&dmg, UnitClass::Me, UnitClass::Creature), + UnitClass::Creature + ); + assert_eq!( + periodic_subject(&heal, UnitClass::Creature, UnitClass::Me), + UnitClass::Me + ); + // The other two formatters read the CASTER, which is why this is a table and not a rule. + let gain = PeriodicTick::Energize { + power: 1, + amount: 10, + }; + let leech = PeriodicTick::ManaLeech { + power: 0, + amount: 40, + multiplier: 1.0, + }; + assert_eq!( + periodic_subject(&gain, UnitClass::Creature, UnitClass::Me), + UnitClass::Creature + ); + assert_eq!( + periodic_subject(&leech, UnitClass::Creature, UnitClass::Me), + UnitClass::Creature + ); + } /// The report this arithmetic was written for: MSBT read `+10 Rage` off our combat log where /// the reference reads `+1 Rage`, for the same swing against the same server. diff --git a/crates/benilla-app/src/ui_chat/combat.rs b/crates/benilla-app/src/ui_chat/combat.rs index ea32641b..e7ed0f3b 100644 --- a/crates/benilla-app/src/ui_chat/combat.rs +++ b/crates/benilla-app/src/ui_chat/combat.rs @@ -385,13 +385,29 @@ pub(crate) fn spell_kind( /// `0x627d80` (damage) / `0x6274a0` (buffs) — the periodic family, and it is **a different shape**. /// /// Ten rows, not sixteen: there is no PET bucket and no `CREATURE_VS_*` split, so a pet folds into -/// its owner's row and every creature source lands on one `SPELL_PERIODIC_CREATURE_*` row. And the -/// **victim is not consulted at all** — both selectors take a single argument. A DoT you put on a -/// hostile player and a DoT that player put on you are told apart by the *source* alone. -pub(crate) fn periodic_kind(attacker: UnitClass, buff: bool) -> Option { +/// its owner's row and every creature source lands on one `SPELL_PERIODIC_CREATURE_*` row. +/// +/// **Both selectors take ONE class, and WHICH one is the caller's to know** (decision 2127). This +/// doc used to say the endpoint was "the *source* alone", and it is not: `0x626630` fills classA +/// (the sentence's subject, the caster) and classB (the target), and the four periodic formatters +/// disagree about which one they hand this selector. +/// +/// | formatter | msg-id argument | the byte | +/// |---|---|---| +/// | `PERIODICAURADAMAGE*` `0x628100` | **classB — the TARGET** | `0x628235 mov ecx,edi` where `edi = [ebp-0x10]` is `outClassB` | +/// | `PERIODICAURAHEAL*` `0x627240` | **classB — the TARGET** | `0x62732c mov ecx,[ebp-0x4]`, its own `outClassB` | +/// | `POWERGAIN*` `0x627520` | classA — the caster | `0x6275fb mov ecx,esi`, `esi = [ebp-0x18]` = `outClassA` | +/// | `SPELLPOWERLEECH*`/`…DRAIN*` `0x627930` | classA — the caster | `0x627a0f`/`0x627a3a mov ecx,esi`, `esi = [ebp-0x10]` = `outClassA` | +/// +/// Passing the caster for the first two is not a cosmetic slip: a creature's DoT ticking **you** +/// types as `SPELL_PERIODIC_CREATURE_DAMAGE` instead of `SPELL_PERIODIC_SELF_DAMAGE`, and your own +/// DoT on a creature types as SELF instead of CREATURE — the two swap. Every addon that parses the +/// combat log by event name (MikScrollingBattleText matches a *different GlobalString pattern* per +/// event) then finds no pattern for either line and drops both. +pub(crate) fn periodic_kind(subject: UnitClass, buff: bool) -> Option { use ChatEventKind as K; use UnitClass as C; - let damage = match attacker { + let damage = match subject { C::Me | C::MyPet => K::SpellPeriodicSelfDamage, C::Party | C::PartyPet => K::SpellPeriodicPartyDamage, C::FriendlyPlayer | C::FriendlyPet => K::SpellPeriodicFriendlyPlayerDamage, @@ -836,11 +852,30 @@ pub(crate) fn global_string(script: &benilla_ui::script::UiScript, key: &str) -> .filter(|s| !s.is_empty()) } -/// The lowercase school word a `…SCHOOL…` template's `%s` takes — `SPELL_SCHOOL_NAME` -/// ("physical", "holy", "fire", …), resolved through the install's own strings like every other -/// template slot. +/// The school word a `…SCHOOL…` template's `%s` takes — **capitalized** (decision 2127). +/// +/// The reference does not read a GlobalString here at all: `0x6264b0(schoolIndex)` indexes +/// `Resistances.dbc` (row array `[0xc0d9a4]`, 7 rows of 0x30 bytes, localized name at +/// `row + 0xc + locale*4`) — wow-re `combat-log-chat-law.md` §4.5 — and those seven rows are +/// `Physical · Holy · Fire · Nature · Frost · Shadow · Arcane`. `SPELL_SCHOOL_NAME` is a +/// *different* table in GlobalStrings.lua and holds the same seven words **lowercased** +/// (`GlobalStrings.lua:4102-4114`), which is what this used to return: "12 nature damage" where +/// the reference writes "12 Nature damage". +/// +/// It reads as a nicety and is not one. MikScrollingBattleText classifies a parsed line's damage +/// school by an exact string compare against `SPELL_SCHOOL_CAP` +/// (`MikCombatEventHelper.lua:3471-3492`) — an addon written against the live client's own output, +/// so it is independent evidence for the capital. A lowercase word falls through to +/// `DAMAGETYPE_UNKNOWN` and every school-coloured number in the addon loses its tint. +/// +/// **Named residue:** the *source* is still the GlobalString, not the DBC. `SPELL_SCHOOL_CAP` +/// carries the same seven strings as `Resistances.dbc` in enUS (both read, 2127), and this client +/// is enUS-only by construction (`combat_text::law::WORDS` and the rest of the string data are +/// hardcoded enUS). A locale where the two tables disagree would diverge; a `Resistances.dbc` +/// catalog is what closes it, and it is a data change rather than a string one — `compose_line` +/// is handed the Lua VM and nothing else. pub(crate) fn school_word(script: &benilla_ui::script::UiScript, school: u8) -> Option { - global_string(script, &format!("SPELL_SCHOOL{school}_NAME")) + global_string(script, &format!("SPELL_SCHOOL{school}_CAP")) } /// The power word a `POWERGAIN`/`SPELLPOWERLEECH`/`SPELLPOWERDRAIN` template takes, by the vmangos diff --git a/crates/benilla-app/src/ui_chat/combat/tests.rs b/crates/benilla-app/src/ui_chat/combat/tests.rs index b0c01230..97f046e9 100644 --- a/crates/benilla-app/src/ui_chat/combat/tests.rs +++ b/crates/benilla-app/src/ui_chat/combat/tests.rs @@ -158,7 +158,7 @@ fn the_school_and_power_words_resolve() { for school in 0..=6u8 { assert!( school_word(&script, school).is_some(), - "SPELL_SCHOOL{school}_NAME missing" + "SPELL_SCHOOL{school}_CAP missing" ); } for power in 0..=3u32 { @@ -219,7 +219,10 @@ fn the_reported_sentences_read_correctly() { ); assert_eq!( line(PERIODICAURADAMAGE, Variant::SelfOther), - "Victim suffers 120 fire damage from your Fireball." + // **Capitalized** — the reference words this cell out of `Resistances.dbc`, not out of + // `SPELL_SCHOOL_NAME` (decision 2127). Lowercase is what MikScrollingBattleText's + // school compare misses. + "Victim suffers 120 Fire damage from your Fireball." ); assert_eq!( line(POWERGAIN, Variant::SelfSelf), @@ -231,7 +234,7 @@ fn the_reported_sentences_read_correctly() { ); assert_eq!( line(DAMAGESHIELD, Variant::SelfOther), - "You reflect 120 fire damage to Victim." + "You reflect 120 Fire damage to Victim." ); // The double-subject family: the drainer is named twice and the second gain has its own pair. assert_eq!( diff --git a/crates/benilla-app/src/ui_chat/settings.rs b/crates/benilla-app/src/ui_chat/settings.rs index 5aa43e9b..8d9d04ee 100644 --- a/crates/benilla-app/src/ui_chat/settings.rs +++ b/crates/benilla-app/src/ui_chat/settings.rs @@ -534,82 +534,85 @@ pub(crate) fn restore_chat_looks(world: &mut World, script: &mut UiScript) { .map(|r| (r.shortcut.clone(), r.id)) .collect(); let commands = world.get_resource::().map(|c| c.0.clone()); - let Some(mut file) = world.get_resource_mut::() else { - return; - }; - if file.identity.get(script).as_ref() == Some(&id) { - return; // already restored for this character, into the VM that is live now - } - let who = format!("{} on {}", id.1, id.0); - file.path = crate::local_state::chat_character_path(&id.0, &id.1); - *file.identity.get(script) = Some(id); - *file.dirty.get(script) = false; - file.last_change = None; - let text = file - .path - .as_ref() - .and_then(|path| match std::fs::read_to_string(path) { - Ok(t) => Some(t), - Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, - Err(e) => { - warn!("chat cache: cannot read {}: {e}", path.display()); - None - } - }); - let had_file = text.is_some(); - // A file our own pre-2120 writer damaged: no marker, so its `ZONECHANNELS` words may have been - // composed from an empty roster and its window channel lists erased with them. Repaired below, - // once — the next save stamps the marker. - let damaged = text - .as_deref() - .is_some_and(|t| !t.contains(WRITER_GENERATION)); - let mut parsed = text.map(|t| parse(&t, &rows)).unwrap_or_default(); // The DBC seed — every `ChatChannels.dbc` row the client joins by itself (`flags & 1`). let seed_mask = auto_rows.iter().fold(0, |m, (_, id)| m | zone_bit(*id)); - if !had_file { - // The loader's no-file path (§3, `0x4997ad`): the mask is seeded from those rows, and - // window 1's channel slots get each of them as `(Shortcut, id)` — the rows - // `ChatFrame_RegisterForChannels` will match zone speech against by id. The rest of the - // record is the boot init the VM already holds. - let mut general = ChatWindowLook::stock(0); - general.channels = auto_rows.clone(); - parsed.looks.push((0, general)); - } - if !parsed.looks.is_empty() || !parsed.colors.is_empty() { - info!( - "chat cache: {} windows, {} colour rows, {} custom channels restored", - parsed.looks.len(), - parsed.colors.len(), - parsed.joined.len() - ); - } - if damaged { - // **The one-time repair** (decision 2120, [`WRITER_GENERATION`]). Re-seed window 1 the way - // the loader's no-file path seeds it, and OR the DBC bits back into the mask. Additive and - // deduplicated, exactly like the in-window `ZONECHANNELS` arm (`0x499332`-`0x4994e7`), so a - // file that survived intact is left alone and one that was zeroed gets its channels back. - if let Some((_, general)) = parsed.looks.iter_mut().find(|(w, _)| *w == 0) { - for (shortcut, id) in &auto_rows { - if !general - .channels - .iter() - .any(|(c, _)| c.eq_ignore_ascii_case(shortcut)) - { - general.channels.push((shortcut.clone(), *id)); + // `ChatWindowFile`'s borrow is scoped, because the mask has to go home to `ChannelState` + // afterwards and a `&mut World` hands out one resource borrow at a time. + let mut parsed; + { + let Some(mut file) = world.get_resource_mut::() else { + return; + }; + if file.identity.get(script).as_ref() == Some(&id) { + return; // already restored for this character, into the VM that is live now + } + let who = format!("{} on {}", id.1, id.0); + file.path = crate::local_state::chat_character_path(&id.0, &id.1); + *file.identity.get(script) = Some(id); + *file.dirty.get(script) = false; + file.last_change = None; + let text = file + .path + .as_ref() + .and_then(|path| match std::fs::read_to_string(path) { + Ok(t) => Some(t), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => None, + Err(e) => { + warn!("chat cache: cannot read {}: {e}", path.display()); + None + } + }); + let had_file = text.is_some(); + // A file our own pre-2120 writer damaged: no marker, so its `ZONECHANNELS` words may have been + // composed from an empty roster and its window channel lists erased with them. Repaired below, + // once — the next save stamps the marker. + let damaged = text + .as_deref() + .is_some_and(|t| !t.contains(WRITER_GENERATION)); + parsed = text.map(|t| parse(&t, &rows)).unwrap_or_default(); + if !had_file { + // The loader's no-file path (§3, `0x4997ad`): the mask is seeded from those rows, and + // window 1's channel slots get each of them as `(Shortcut, id)` — the rows + // `ChatFrame_RegisterForChannels` will match zone speech against by id. The rest of the + // record is the boot init the VM already holds. + let mut general = ChatWindowLook::stock(0); + general.channels = auto_rows.clone(); + parsed.looks.push((0, general)); + } + if !parsed.looks.is_empty() || !parsed.colors.is_empty() { + info!( + "chat cache: {} windows, {} colour rows, {} custom channels restored", + parsed.looks.len(), + parsed.colors.len(), + parsed.joined.len() + ); + } + if damaged { + // **The one-time repair** (decision 2120, [`WRITER_GENERATION`]). Re-seed window 1 the way + // the loader's no-file path seeds it, and OR the DBC bits back into the mask. Additive and + // deduplicated, exactly like the in-window `ZONECHANNELS` arm (`0x499332`-`0x4994e7`), so a + // file that survived intact is left alone and one that was zeroed gets its channels back. + if let Some((_, general)) = parsed.looks.iter_mut().find(|(w, _)| *w == 0) { + for (shortcut, id) in &auto_rows { + if !general + .channels + .iter() + .any(|(c, _)| c.eq_ignore_ascii_case(shortcut)) + { + general.channels.push((shortcut.clone(), *id)); + } } } + parsed.zone_mask = Some(parsed.zone_mask.unwrap_or(0) | seed_mask); + // Owed a write, so the repair is genuinely ONCE: the next save composes the repaired + // record and stamps [`WRITER_GENERATION`], and this branch never runs for the character + // again. `last_change` is `None` here, so that save is the very next frame's. + *file.dirty.get(script) = true; + info!("chat cache: repaired a pre-2120 file's zone channels for {who}"); } - parsed.zone_mask = Some(parsed.zone_mask.unwrap_or(0) | seed_mask); - // Owed a write, so the repair is genuinely ONCE: the next save composes the repaired - // record and stamps [`WRITER_GENERATION`], and this branch never runs for the character - // again. `last_change` is `None` here, so that save is the very next frame's. - *file.dirty.get(script) = true; - info!("chat cache: repaired a pre-2120 file's zone channels for {who}"); - } - // …and `ChatWindowFile` is finished with, so the mask can go home to `ChannelState`. - drop(file); - // The mask is durable state from here on (decision 2120): the file's word when it carried one, - // the DBC seed when it did not, and from then on the confirmed joins' own OR. + } // …and `ChatWindowFile`'s borrow ends here. + // The mask is durable state from here on (decision 2120): the file's word when it carried one, + // the DBC seed when it did not, and from then on the confirmed joins' own OR. let mask = parsed.zone_mask.unwrap_or(seed_mask); if let Some(mut channels) = world.get_resource_mut::() { channels.zone_mask = mask; diff --git a/crates/benilla-app/src/ui_chat/tests.rs b/crates/benilla-app/src/ui_chat/tests.rs index de30ef82..9c5e470c 100644 --- a/crates/benilla-app/src/ui_chat/tests.rs +++ b/crates/benilla-app/src/ui_chat/tests.rs @@ -2453,17 +2453,22 @@ fn the_chat_cache_restore_is_finished_before_player_login() { .non_send_resource::() .run( r#" - ChatOrderProbe = {} + ChatOrderProbe = { order = "" } local f = CreateFrame("Frame") + f:RegisterEvent("VARIABLES_LOADED") f:RegisterEvent("UPDATE_CHAT_WINDOWS") f:RegisterEvent("UPDATE_CHAT_COLOR") f:RegisterEvent("PLAYER_LOGIN") f:SetScript("OnEvent", function() + -- The burst is 100+ events; record it once so the order string stays readable. + if not string.find(ChatOrderProbe.order, event, 1, 1) then + ChatOrderProbe.order = ChatOrderProbe.order .. event .. " " + end if event == "UPDATE_CHAT_WINDOWS" then ChatOrderProbe.windows = (ChatOrderProbe.windows or 0) + 1 elseif event == "UPDATE_CHAT_COLOR" then ChatOrderProbe.colors = (ChatOrderProbe.colors or 0) + 1 - else + elseif event == "PLAYER_LOGIN" then ChatOrderProbe.loginWindows = ChatOrderProbe.windows or 0 ChatOrderProbe.loginColors = ChatOrderProbe.colors or 0 ChatOrderProbe.loginRegistered = @@ -2530,6 +2535,15 @@ fn the_chat_cache_restore_is_finished_before_player_login() { "ChatFrame1 must carry the SYSTEM message group at PLAYER_LOGIN, not {:?}", read("ChatOrderProbe.loginRegistered") ); + // The reference's own login order, byte-derived (wow-re `login-chat-colour-pipeline.md`; + // decision 2125): addons and their `ADDON_LOADED` (`0x4900a3`), then `VARIABLES_LOADED` + // (`0x4900b2`), then the chat-cache reader's burst (`0x4900d6`), then `PLAYER_LOGIN` + // (`0x490959`). 2119 put the burst ahead of `VARIABLES_LOADED`, one step too early. + assert_eq!( + read("ChatOrderProbe.order"), + "VARIABLES_LOADED UPDATE_CHAT_WINDOWS UPDATE_CHAT_COLOR PLAYER_LOGIN ", + "the reference fires the chat-cache burst BETWEEN VARIABLES_LOADED and PLAYER_LOGIN" + ); drop(world); let _ = std::fs::remove_dir_all(&tmp); diff --git a/crates/benilla-app/src/ui_script/lifecycle.rs b/crates/benilla-app/src/ui_script/lifecycle.rs index 9492c7eb..e5ebcbaf 100644 --- a/crates/benilla-app/src/ui_script/lifecycle.rs +++ b/crates/benilla-app/src/ui_script/lifecycle.rs @@ -432,21 +432,21 @@ pub(crate) fn load_ingame_ui_on_world_entry(world: &mut World) { // back. Startup always precedes this state edge (1038), so the knob is already loaded. let zoom = world.resource::(); script.set_minimap_zoom(zoom.outdoor, zoom.inside); - // **The chat cache restores HERE — after every chat frame exists, before anything prints into - // one** (decision 2119). It is the sole firer of `UPDATE_CHAT_WINDOWS`, which is the only - // thing that registers a chat frame for any `CHAT_MSG_*` (ref `ChatFrame.lua` l.1261-1273), - // and of the `UPDATE_CHAT_COLOR` burst, whose `WHISPER`→`REPLY` mirror repaints every line - // already in the window that was printed with no explicit colour (`ChatTypeInfo["REPLY"].id` - // is 0, and so is such a line's). As an `Update` system both landed too late: the login MOTD - // was routed to a window registered for nothing, and every Ace2 addon's `Print` at - // `PLAYER_LOGIN` came out whisper-pink. Placed after `load_ingame_ui` so an ADDON's chat frame - // is registered too, and before `finish_ui_load` so the burst precedes `PLAYER_LOGIN`. - crate::ui_chat::restore_chat_looks(world, &mut script); // The saved-variables chunk runs HERE — after the XML assigned its file-scope defaults, before // any consumer reads them — then `VARIABLES_LOADED`. That is the reference's own load order // (`AddOn_Load 0x51f240` steps 2 → 4 → 6, decision 1128); reversing it means the defaults // always win and nothing can ever be remembered. - finish_ui_load(&mut script); + // + // **And the chat cache restores inside it, between `VARIABLES_LOADED` and `PLAYER_LOGIN`** — + // the reference's own slot for the reader's `UPDATE_CHAT_WINDOWS` + `UPDATE_CHAT_COLOR` burst + // (`0x4900d6`, after `0x4900b2` and before `0x490959`; decisions 2119 and 2125). It is the + // sole firer of `UPDATE_CHAT_WINDOWS`, which is the only thing that registers a chat frame for + // any `CHAT_MSG_*` (ref `ChatFrame.lua` l.1261-1273) — as an `Update` system it landed after + // the session's first chat had already been routed, and the login MOTD went to a window + // registered for nothing. + finish_ui_load_with(&mut script, |script| { + crate::ui_chat::restore_chat_looks(world, script); + }); // **Say it out loud when an addon didn't load** (decision 1495). Every failure the walk found // is retained now, but a log nobody knows to open does not fix silence — and silence is the // actual defect B293 reports: *"there are a lot of addons that still doesn't work"*, with @@ -754,12 +754,34 @@ pub(crate) fn shutdown_on_exit( /// [`load_ingame_ui_on_world_entry`], on the same edge that built the tree. /// `PLAYER_ENTERING_WORLD` keeps its own per-entry latch in [`crate::ui_unit`] and still lands /// after this, since it waits on the self descriptor arriving over the wire. +/// Test-only since 2125: the production edge is [`finish_ui_load_with`], because the chat-cache +/// restore has to sit inside it. A test that only wants the tail keeps the plain shape. +#[cfg(test)] pub(crate) fn finish_ui_load(script: &mut UiScript) { + finish_ui_load_with(script, |_| {}); +} + +/// [`finish_ui_load`] with the one step that has to land **between** `VARIABLES_LOADED` and +/// `PLAYER_LOGIN`: the chat-cache restore's `UPDATE_CHAT_WINDOWS` + `UPDATE_CHAT_COLOR` burst +/// (decision 2125, correcting 2119's placement). +/// +/// The reference's login is `FrameXML → addons + ADDON_LOADED (0x4900a3) → VARIABLES_LOADED +/// (0x4900b2) → the chat-cache reader's burst (0x4900d6, firing synchronously through the +/// register-or-fire-now trampoline 0x498a20) → PLAYER_LOGIN (0x490959) → PLAYER_ENTERING_WORLD +/// (0x49096a)` — byte-derived in wow-re `system/ui/scratch/login-chat-colour-pipeline.md`, §5 +/// cross-checked. 2119 put the restore ahead of `VARIABLES_LOADED`, which is one step too early: +/// an addon reading its chat colours out of a `VARIABLES_LOADED` handler would see the file's +/// values where the reference shows it the boot ones. +/// +/// A callback rather than a split pair because the budget re-arm and the two fires are one edge, +/// and a caller that forgets the middle step should not be able to compile. +pub(crate) fn finish_ui_load_with(script: &mut UiScript, between: impl FnOnce(&mut UiScript)) { // Still the load edge, so still bounded (1306) — re-armed because the walk's last addon left // an arbitrary amount on the counter, and the saved-variables chunk plus every PLAYER_LOGIN // handler deserve the full allowance. The entry edge disarms after this returns. script.set_instruction_budget(addons::LOAD_INSTRUCTION_BUDGET); crate::ui_saved::load_saved_variables(script); + between(script); script.fire_event("PLAYER_LOGIN", vec![]); } diff --git a/crates/benilla-app/src/ui_script/shape_gate.rs b/crates/benilla-app/src/ui_script/shape_gate.rs index f11074c0..ddcaffe7 100644 --- a/crates/benilla-app/src/ui_script/shape_gate.rs +++ b/crates/benilla-app/src/ui_script/shape_gate.rs @@ -67,33 +67,23 @@ const NOT_YET_ASSERTED: &[(&str, usize, &str)] = &[]; /// The **kind** gate's own shrinking list, same rules as [`NOT_YET_ASSERTED`]: a name, the wrong /// kinds it currently answers, and the dispatched question that will settle it. An entry whose -/// binding has come into agreement fails the gate, so it cannot outlive its fix. +/// binding has come into agreement fails the gate, so it cannot outlive its fix — and both of the +/// two it held did exactly that, within the day. /// -/// Both entries are edge-case *values* the shapes table states the kind of and the recorded wow-re -/// notes state no value for, dispatched into wow-re as one batched question (decision 2118): -/// -/// * `GetSendMailItem` — slots 3 and 4 are `number` on every alternative including the -/// `(nil,nil,…)` empty one, so the no-attachment leg pushes two numbers. Which two is a byte -/// question; ours answers `(nil, nil, 1, nil)` and the slot-4 nil is provably wrong. -const KINDS_NOT_YET_ASSERTED: &[(&str, &str, &str)] = &[( - "GetSendMailItem", - "nil,nil,number,nil", - "the reference's no-attachment leg pushes numbers in slots 3 and 4; the two values are dispatched into wow-re (GetSendMailItem 0x4ae590)", -)]; +/// **It is empty.** It held `GetSendMailItem`, whose no-attachment leg the shapes table types as +/// two numbers and no recorded note gave the values of. wow-re answered it at the bytes — +/// `0x4ae590` pushes `(nil, nil, 0, 0)`, and the stray `1` we had was borrowed from +/// `GetAuctionSellItemInfo`'s empty leg — and 2129 folded the verdict back. +const KINDS_NOT_YET_ASSERTED: &[(&str, &str, &str)] = &[]; /// The widget kind gate's shrinking list — see [`KINDS_NOT_YET_ASSERTED`]. /// -/// `GetFont`'s height slot is `number` in the reference's single alternative, and ours answers nil -/// on a FontString that has never been given a font. wow-re's `font-object-lua-surface.md` §9.3 -/// records that the FontString reads back through its resolved `CGxFont`, so a NULL one yields nil -/// for the path and the flags — but it does not say what the *height* slot then carries, and that -/// is the value the corpus does arithmetic on (`aux-addon`'s font-resize wheel handler, -/// `tabs/search/frame.lua:481`). Dispatched. -const WIDGET_KINDS_NOT_YET_ASSERTED: &[(&str, &str, &str)] = &[( - "GetFont", - "nil,nil,string", - "the reference's height slot is a number even with no CGxFont resolved; the value is dispatched into wow-re (CSimpleFontString::GetFont 0x79d400)", -)]; +/// **Also empty.** It held `GetFont`, whose height slot the table types as a number and which we +/// answered nil on a FontString that was never given a font. The recorded note read as though a +/// NULL `CGxFont` should nil all three slots; the bytes say otherwise — `0x7727b0`'s unconditional +/// `fld [esi+0xe4]` sits in front of the `+0xe0` test, behind a branch `GetFont` never takes — and +/// 2129 folded that back too. +const WIDGET_KINDS_NOT_YET_ASSERTED: &[(&str, &str, &str)] = &[]; /// The widget half's own shrinking list — see [`NOT_YET_ASSERTED`], same rules. Also empty: /// `GetFogColor` came off it with 1845, which found the fog colour was a packed `0xAARRGGBB` dword @@ -757,3 +747,91 @@ fn no_query_binding_answers_a_lua_boolean() { booleans.join("\n ") ); } + +/// The widget half of [`no_query_binding_answers_a_lua_boolean`], and the reason it is worth having +/// separately from the widget *kind* gate: the kind gate calls each method with no arguments and +/// skips whatever raises, so a predicate that gates its argument — `HasScript("OnClick")`, +/// `IsObjectType("Frame")` — is never measured. The argument ladder reaches those, and `boolean` +/// stays wrong under every one of them. +#[test] +fn no_widget_method_answers_a_lua_boolean() { + let tsv = concat!( + env!("CARGO_MANIFEST_DIR"), + "/../../reference/1.12-shapes.tsv" + ); + let text = std::fs::read_to_string(tsv).expect("reference/1.12-shapes.tsv"); + + let s = benilla_ui::script::UiScript::new().expect("VM"); + s.run(KIND_HELPER).expect("kind helper"); + let mut made: std::collections::HashMap<&str, String> = std::collections::HashMap::new(); + for (table, kind, name) in WIDGET_PROBES { + if s.run(&format!( + "{name} = CreateFrame(\"{kind}\", \"{name}\", UIParent)" + )) + .is_ok() + { + made.insert(table, (*name).to_string()); + } + } + for (table, expr) in REGION_PROBES { + let var = format!("PGR{}", made.len()); + if s.run(&format!("{var} = {expr}")).is_ok() { + made.insert(table, var); + } + } + + // A widget predicate's argument is a script or object-type NAME far more often than a number, + // so the ladder differs from the global one. + const WIDGET_ARGS: &[&str] = &["", "\"OnClick\"", "\"Frame\"", "1", "1, 1"]; + + let mut checked = 0usize; + let mut booleans: Vec = Vec::new(); + for line in text + .lines() + .filter(|l| !l.starts_with('#') && !l.starts_with("name\t")) + { + let f: Vec<&str> = line.split('\t').collect(); + if f.len() < 9 || f[4] != "widget" || f[8] != "exact" { + continue; + } + let (name, table) = (f[0], f[3]); + if !["Get", "Is", "Has", "Can", "Num"] + .iter() + .any(|p| name.starts_with(p)) + { + continue; + } + // The reference's own boolean, and the only widget one: `IsPetAttackActive` is a global. + let Some(obj) = made.get(table) else { continue }; + for args in WIDGET_ARGS { + let probe = format!( + "if type({obj}.{name}) ~= 'function' then return '?' end \ + local ok, s = pcall(function() return PGKinds({obj}:{name}({args})) end) \ + if not ok then return '?' end return s" + ); + let Ok(got) = s.eval::(&probe) else { + continue; + }; + if got == "?" { + continue; + } + checked += 1; + if got.split(',').any(|k| k == "boolean") { + booleans.push(format!("{obj}:{name}({args}) answers ({got})")); + } + break; + } + } + + assert!( + checked >= 100, + "the widget boolean gate measured only {checked} methods — it has stopped covering anything" + ); + assert!( + booleans.is_empty(), + "{} widget methods answer a Lua boolean; every predicate row in the reference's widget \ + tables is `(nil) | (number)`:\n {}", + booleans.len(), + booleans.join("\n ") + ); +} diff --git a/crates/benilla-app/src/ui_text/engine/faces.rs b/crates/benilla-app/src/ui_text/engine/faces.rs index 92b9515f..7c977c13 100644 --- a/crates/benilla-app/src/ui_text/engine/faces.rs +++ b/crates/benilla-app/src/ui_text/engine/faces.rs @@ -41,13 +41,30 @@ pub(super) fn hhea_ascent_ratio(bytes: &[u8]) -> Option { (denom > 0.0 && asc > 0.0).then_some(asc / denom) } -/// Registers `bytes` (raw TTF) into `font_system`'s `fontdb`, returning `(face_id, family_name)` — -/// the family name is read back off the just-loaded face so callers build an exact `Attrs::family` -/// match with no reliance on hardcoding Blizzard's font-name strings. +/// What a registered face answers to when a caller has to name it to the shaper: its id, the +/// family string, and the three CSS axes `cosmic-text` matches on. +/// +/// **The axes are not decoration** (decision 2123). `Attrs::matches` in cosmic-text 0.16 filters +/// the candidate set on `style` and `stretch`, and `fontdb::Database::query` then does CSS +/// matching over `weight` as well — so an `Attrs::new().family(…)` built with the *defaults* +/// asks for a NORMAL-weight, NORMAL-style face and quietly gets somebody else when the face it +/// named is bold or italic. Reading them back off the face means the attrs describe the face we +/// actually loaded rather than the one we assumed. +pub(super) struct Registered { + pub(super) id: fontdb::ID, + pub(super) family: String, + pub(super) weight: fontdb::Weight, + pub(super) style: fontdb::Style, + pub(super) stretch: fontdb::Stretch, +} + +/// Registers `bytes` (raw TTF) into `font_system`'s `fontdb`, returning the face's id and the +/// four things a caller needs to name it back to the shaper — all read off the just-loaded face, +/// with no reliance on hardcoding Blizzard's font-name strings. pub(super) fn register_font( font_system: &mut FontSystem, bytes: Vec, -) -> anyhow::Result<(fontdb::ID, String)> { +) -> anyhow::Result { let source = fontdb::Source::Binary( std::sync::Arc::new(bytes) as std::sync::Arc + Sync + Send> ); @@ -55,13 +72,20 @@ pub(super) fn register_font( let id = *ids .first() .ok_or_else(|| anyhow::anyhow!("font source produced no faces (not a valid TTF?)"))?; - let family = font_system + let info = font_system .db_mut() .face(id) - .ok_or_else(|| anyhow::anyhow!("face {id:?} vanished right after loading"))? + .ok_or_else(|| anyhow::anyhow!("face {id:?} vanished right after loading"))?; + let family = info .families .first() .map(|(name, _)| name.clone()) .ok_or_else(|| anyhow::anyhow!("face {id:?} carries no family name"))?; - Ok((id, family)) + Ok(Registered { + id, + family, + weight: info.weight, + style: info.style, + stretch: info.stretch, + }) } diff --git a/crates/benilla-app/src/ui_text/engine/mod.rs b/crates/benilla-app/src/ui_text/engine/mod.rs index 5510a2d1..71c013eb 100644 --- a/crates/benilla-app/src/ui_text/engine/mod.rs +++ b/crates/benilla-app/src/ui_text/engine/mod.rs @@ -141,7 +141,15 @@ const MAX_PPEM: u16 = 256; /// One registered face. struct Face { id: fontdb::ID, + /// The path the caller asked for — carried so the substitution WARN can name it (2123). + path: String, family: String, + /// The three CSS axes `cosmic-text` matches on, read off the face itself. See + /// [`faces::Registered`]: naming only the family asks for a normal-weight, normal-style face + /// and silently gets a different one when the named face is not that. + weight: fontdb::Weight, + style: fontdb::Style, + stretch: fontdb::Stretch, /// `hhea.asc / (asc + |desc|)` — see [`hhea_ascent_ratio`]. Friz's ≈ 0.794 is the fallback for /// a face whose tables would not parse (never the four shipped client fonts). ascent_ratio: f32, @@ -256,17 +264,43 @@ pub(crate) struct TextEngine { /// Characters no face could shape, and sizes past the ceiling — reported once each rather than /// once per frame. complained: HashSet, + /// Faces the shaper answered for with a different font — warned once each (2123). + substituted: HashSet, over_ceiling: bool, stats: CacheStats, } +/// A `cosmic-text` font system over an **empty** database — the client's faces are the only faces. +/// +/// `FontSystem::new()` and `new_with_fonts()` both call `fontdb::Database::load_system_fonts()` +/// (`cosmic-text-0.16.0/src/font/system.rs:400`), which puts every font on the machine into the +/// same pool as the four client TTFs. That is not a harmless extra: `get_font_matches` does not +/// select a face, it *orders* every face in the database and shapes with the first one that can +/// draw the character (`system.rs:326-369`), so a family the query cannot match — or a single +/// character the client face happens to lack — is answered by whatever the machine has. It is +/// invisible by construction: the text draws, at the right size, in a face nobody asked for. +/// Measured on this machine (decision 2123): the reference's own `Fonts\ARIALN.TTF` was being +/// shaped by macOS's Arial Narrow, and Friz Quadrata by `.SFNS-Regular`, the system UI font. +/// +/// So the pool holds exactly what a 1.12 client has: the four TTFs out of `fonts.MPQ`, plus +/// whatever faces an addon ships (decision 2103). A character none of them carries draws nothing +/// and warns once — which is both what the reference does and what our own width law already +/// says ("a character no face can shape contributes 0"): with a system fallback in the pool the +/// shaper-less measure and the shaped draw silently disagreed about that character's width. +/// +/// The locale is `en-US` because everything else about our string data is (the `WORDS` table, the +/// GlobalStrings fallbacks); it only picks which *language's* family name a face answers to. +fn client_font_system() -> FontSystem { + FontSystem::new_with_locale_and_db("en-US".to_string(), fontdb::Database::new()) +} + impl TextEngine { /// Read the client TTFs through the app's own patch chain ([`WorldAssets::chain`] — never /// `std::fs`) and register them. `None` if Friz Quadrata (the fallback face) is unreadable, in /// which case text simply will not render — the same graceful-absence posture /// [`crate::ui_script`]'s extraction takes. fn load(world_assets: &WorldAssets, images: &Assets, dpi: f32) -> Option { - let mut font_system = FontSystem::new_with_fonts(std::iter::empty()); + let mut font_system = client_font_system(); let mut faces: Vec = Vec::new(); let mut path_to_face = HashMap::new(); for &path in CLIENT_FONTS { @@ -282,11 +316,15 @@ impl TextEngine { }; let ascent_ratio = hhea_ascent_ratio(&bytes).unwrap_or(0.794); match register_font(&mut font_system, bytes) { - Ok((id, family)) => { + Ok(r) => { path_to_face.insert(path.to_ascii_lowercase(), faces.len()); faces.push(Face { - id, - family, + id: r.id, + path: path.to_string(), + family: r.family, + weight: r.weight, + style: r.style, + stretch: r.stretch, ascent_ratio, }); } @@ -319,6 +357,7 @@ impl TextEngine { generation: 0, reset_pending: false, complained: HashSet::new(), + substituted: HashSet::new(), over_ceiling: false, stats: CacheStats::default(), }) @@ -367,13 +406,17 @@ impl TextEngine { let source = self.source.as_ref()?; let bytes = read_font_bytes(source, path)?; let ascent_ratio = hhea_ascent_ratio(&bytes).unwrap_or(0.794); - let (id, family) = register_font(&mut self.font_system, bytes) + let r = register_font(&mut self.font_system, bytes) .inspect_err(|e| warn!("ui_text: failed to register {path}: {e:#}")) .ok()?; let index = self.faces.len(); self.faces.push(Face { - id, - family, + id: r.id, + path: path.to_string(), + family: r.family, + weight: r.weight, + style: r.style, + stretch: r.stretch, ascent_ratio, }); self.path_to_face.insert(key.to_string(), index); @@ -523,11 +566,18 @@ impl TextEngine { let Some(f) = self.faces.get(face) else { return; }; - let (face_id, family) = (f.id, f.family.clone()); - let attrs = Attrs::new().family(Family::Name(&family)); + // Owned, because the buffer below borrows `self.font_system` mutably. + let (face_id, family, weight, style, stretch) = + (f.id, f.family.clone(), f.weight, f.style, f.stretch); + let attrs = Attrs::new() + .family(Family::Name(&family)) + .weight(weight) + .style(style) + .stretch(stretch); let px = f32::from(ppem); let mut glyphs: Vec = Vec::new(); let mut floor_sum = 0.0f32; + let mut shaped_by = None; { let mut buf = Buffer::new(&mut self.font_system, Metrics::new(px, px)); buf.set_wrap(&mut self.font_system, Wrap::None); @@ -545,6 +595,7 @@ impl TextEngine { // A lone first glyph on the line has x == y == 0.0, so this is the // zero-subpixel canonical rasterization for (face, glyph, ppem). let physical = g.physical((0.0, 0.0), 1.0); + shaped_by.get_or_insert(g.font_id); glyphs.push(GlyphRef { glyph_id: g.glyph_id, key: physical.cache_key, @@ -555,6 +606,29 @@ impl TextEngine { } } } + // **The shaper's answer is checked against the ask** (decision 2123). Naming a face to + // `cosmic-text` is a *query*, not a selection: it walks every registered face and takes + // the first that can draw the character, so a family it cannot match is answered by some + // other face — correct-looking text in the wrong one, which is exactly the silent failure + // 2103 set out to end and did not, because it never compared what came back. The glyph + // ids and the advances below are then that other face's, so the string measures wrong + // too. Once per face, naming both sides. + if let Some(got) = shaped_by.filter(|&got| got != face_id) { + if self.substituted.insert(face) { + let got_name = self + .font_system + .db() + .face(got) + .map_or_else(|| format!("{got:?}"), |i| i.post_script_name.clone()); + let want = self.faces.get(face).map_or("?", |f| f.path.as_str()); + warn!( + "ui_text: '{want}' loaded as family {family:?} (weight {}, style {style:?}) \ + but {ch:?} shaped in {got_name} instead — text in this face draws and \ + measures in the wrong one", + weight.0 + ); + } + } if glyphs.is_empty() { // No face shaped it — it draws nothing and, per the width law's own note, measures // nothing. Report once, not once per frame. @@ -771,7 +845,7 @@ fn read_font_bytes(source: &FontSource, path: &str) -> Option> { pub(super) fn test_engine(dpi: f32) -> Option { let data = benilla_formats::wow_data()?; let chain = benilla_formats::open_chain(&data).ok()?; - let mut font_system = FontSystem::new_with_fonts(std::iter::empty()); + let mut font_system = client_font_system(); let mut faces: Vec = Vec::new(); let mut path_to_face = HashMap::new(); for &path in CLIENT_FONTS { @@ -779,11 +853,15 @@ pub(super) fn test_engine(dpi: f32) -> Option { continue; }; let ascent_ratio = hhea_ascent_ratio(&bytes).unwrap_or(0.794); - let (id, family) = register_font(&mut font_system, bytes).ok()?; + let r = register_font(&mut font_system, bytes).ok()?; path_to_face.insert(path.to_ascii_lowercase(), faces.len()); faces.push(Face { - id, - family, + id: r.id, + path: path.to_string(), + family: r.family, + weight: r.weight, + style: r.style, + stretch: r.stretch, ascent_ratio, }); } @@ -807,6 +885,7 @@ pub(super) fn test_engine(dpi: f32) -> Option { generation: 0, reset_pending: false, complained: HashSet::new(), + substituted: HashSet::new(), over_ceiling: false, stats: CacheStats::default(), }) @@ -1140,6 +1219,97 @@ mod ppem_tests { assert_ne!(friz, e.face_for(Some(TEST_FACES[1])), "ARIALN is its own"); } + /// Overwrite `OS/2.usWeightClass` (offset 4 in the table) in a raw sfnt, in place. `false` if + /// the table directory has no `OS/2`. Test-only: it makes a *bold* face out of a face we + /// already have, so the weight axis can be exercised without shipping a second TTF. + fn set_weight_class(bytes: &mut [u8], weight: u16) -> bool { + let Some(num) = bytes.get(4..6) else { + return false; + }; + let num = u16::from_be_bytes(num.try_into().unwrap()) as usize; + for i in 0..num { + let Some(rec) = bytes.get(12 + 16 * i..12 + 16 * i + 16) else { + return false; + }; + if &rec[0..4] != b"OS/2" { + continue; + } + let off = u32::from_be_bytes(rec[8..12].try_into().unwrap()) as usize; + let Some(slot) = bytes.get_mut(off + 4..off + 6) else { + return false; + }; + slot.copy_from_slice(&weight.to_be_bytes()); + return true; + } + false + } + /// **The font pool is the CLIENT's faces and nothing else** (decision 2123). + /// + /// `cosmic-text`'s two convenience constructors both call `load_system_fonts()`, and its + /// shaper does not *select* a face — it orders every face in the database and takes the first + /// that can draw the character. So one `FontSystem::new()` anywhere in this file puts every + /// font on the developer's machine in front of the four the client actually has, and the only + /// symptom is text in a face nobody asked for. The count is the pin: four faces registered, + /// four faces in the database. + #[test] + fn the_font_pool_holds_only_the_clients_own_faces() { + let Some(e) = engine_or_skip() else { + return; + }; + assert_eq!( + e.font_system.db().len(), + e.faces.len(), + "the database must hold exactly the faces we registered — a system font in the pool \ + is a face the shaper can silently substitute" + ); + } + + /// **A face the client names is the face that shapes — including a BOLD one** (decision 2123). + /// + /// The report was MSBT drawing in a plain sans instead of its own Porky. Porky declares + /// `OS/2.usWeightClass = 700`; the attrs we handed the shaper were `Attrs::new()`, i.e. weight + /// 400, so `fontdb::Database::query` never matched it and `get_font_matches`' weight-sorted + /// walk put every normal-weight face ahead of it. The face loaded, the WARN never fired, the + /// text drew — in the wrong face, at the wrong widths. + /// + /// The fixture is a real client TTF with its `usWeightClass` patched to 700, so the only thing + /// that differs from the face beside it is the axis that broke. + #[test] + fn a_bold_addon_face_is_the_face_that_shapes() { + let Some(mut e) = engine_or_skip() else { + return; + }; + let mut bytes = { + let source = e.source.as_ref().expect("the test engine carries a chain"); + let chain = source.chain.lock_recover(); + chain.read(TEST_FACES[2]).expect("MORPHEUS is in the chain") + }; + assert!( + set_weight_class(&mut bytes, 700), + "patched OS/2 usWeightClass" + ); + let root = std::env::temp_dir().join(format!("benilla-bold-font-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + let fonts = root.join("Bolded").join("Fonts"); + std::fs::create_dir_all(&fonts).unwrap(); + std::fs::write(fonts.join("heavy.ttf"), &bytes).unwrap(); + e.source.as_mut().unwrap().loose_root = Some(root.clone()); + + let bold = e.face_for(Some("Interface\\AddOns\\Bolded\\Fonts\\heavy.ttf")); + assert_ne!(bold, e.default_face, "the bold face registered"); + assert_eq!( + e.faces[bold].weight, + fontdb::Weight(700), + "and fontdb read the patched weight — otherwise this test proves nothing" + ); + e.ensure_metrics(bold, 18, "Rage"); + assert!( + e.substituted.is_empty(), + "the shaper answered with a different face for a face we named" + ); + let _ = std::fs::remove_dir_all(&root); + } + /// **An addon-shipped TTF loads out of the ONE AddOns root** (decision 2103) — the leg the bug /// was: MSBT ships thirty-one faces and names them `Interface\\Addons\\…\\Fonts\\.ttf`, /// a shape no MPQ carries, and every one of them silently drew as Friz Quadrata. diff --git a/crates/benilla-assets/src/world_assets/mod.rs b/crates/benilla-assets/src/world_assets/mod.rs index 2552406f..c1247e3a 100644 --- a/crates/benilla-assets/src/world_assets/mod.rs +++ b/crates/benilla-assets/src/world_assets/mod.rs @@ -190,14 +190,15 @@ pub fn sprite_dimensions( /// *before* inserting — so the warning is self-limiting by construction (one line per distinct key /// for the process's life), with no separate "already warned" set to keep. /// -/// It exists because the renderer's fallback for an unresolvable path is **silent and looks like -/// art**: a `Texture` region whose file can't be found still pushes a quad, which `ui_pass` draws -/// with the shared 1×1 white image tinted white — an opaque WHITE RECTANGLE at the region's exact -/// rect. That fallback can't itself be made loud (it is what lets flat-shaded quads batch into one -/// texture run), so a miss has to be reported here instead. Bug B221 — macro-chooser icons -/// rendering as white squares — was invisible to every layer of the client until this line existed: -/// `shipped_xml_tests` sweeps only static XML `file=` attributes, never a path that arrives at -/// runtime from a DBC. +/// It exists because the renderer's fallback for an unresolvable path is **silent**. It used to be +/// silent *and* look like art: a `Texture` region whose file could not be found still pushed a +/// quad, which `ui_pass` drew with the shared 1×1 white image tinted white — an opaque WHITE +/// RECTANGLE at the region's exact rect, which is how bug B221's macro-chooser icons looked. That +/// half is gone (`ui_script::extract` now drops the quad outright when the resolve misses), so the +/// miss draws *nothing* rather than a white square. It is no less silent for that: nothing on +/// screen and nothing in Lua says why, and `shipped_xml_tests` sweeps only static XML `file=` +/// attributes, never a path that arrives at runtime from a DBC or from an addon. So the report +/// still belongs here. /// /// Walks [`sprite_candidates`] in order and takes the first that both reads and decodes; only when /// **all** fail is it a miss. A candidate that reads but won't decode falls through exactly like @@ -231,15 +232,38 @@ fn decode_sprite( } } } - warn!( - "texture miss: '{path}' does not resolve in the patch chain{} (tried {})", - if loose_root.is_some() { - " or the AddOns folder" - } else { - "" - }, - candidates.join(", ") - ); + // **"Not there" and "there but would not decode" are different faults, and saying only the + // first sends the reader hunting a path that is sitting on disk.** Three colour-mapped TGAs in + // the addon corpus read fine and failed to decode for years while this line asserted they did + // not resolve (decision 2128). + let found_but_undecodable: Vec<&String> = candidates + .iter() + .filter(|c| { + chain.read_file(c).is_ok() + || loose_root.is_some_and(|root| loose_addon_file(root, c).is_some()) + }) + .collect(); + if found_but_undecodable.is_empty() { + warn!( + "texture miss: '{path}' does not resolve in the patch chain{} (tried {})", + if loose_root.is_some() { + " or the AddOns folder" + } else { + "" + }, + candidates.join(", ") + ); + } else { + warn!( + "texture miss: '{path}' RESOLVES but will not decode ({}) — the file is there and the \ + decoder refused it", + found_but_undecodable + .iter() + .map(|c| c.as_str()) + .collect::>() + .join(", ") + ); + } None } diff --git a/crates/benilla-formats/src/tga.rs b/crates/benilla-formats/src/tga.rs index b5ba2c65..a70bae75 100644 --- a/crates/benilla-formats/src/tga.rs +++ b/crates/benilla-formats/src/tga.rs @@ -7,10 +7,24 @@ //! 2006 and the ecosystem shipped loose `.tga` art constantly, so the loose-file resolve //! (decision 1322) is what finally makes this decoder load-bearing. //! -//! The slice implemented is the slice that art actually uses: image types 2/10 (truecolor, -//! raw/RLE) and 3/11 (grayscale, raw/RLE), 8/24/32-bit pixels, both vertical origins and the -//! (vanishingly rare) right-origin mirror. Color-mapped TGAs (types 1/9) are refused — none have -//! surfaced in the corpus, and a silent palette guess is worse than a loud miss. +//! **The reference's own dispatch is the spec, and ours used to be its mirror image.** `0x5a3a30` +//! forks on `imageType` as `{1/9 → 0x5a39d0 (colour-mapped, raw or RLE, expanded through the +//! palette at 0x5a3820); 2 → 0x5a3c80; 10 → 0x5a3d70; 3/11 → unsupported}` (wow-re +//! `image/scratch/re-wave1.md`). This decoder refused 1/9 — on a doc comment asserting *"none have +//! surfaced in the corpus"*, which was false — and accepted the two the client refuses. +//! +//! Three colour-mapped TGAs are in the 219-addon corpus, all 8-bit indices into a 256-entry +//! 24-bit palette: `FonzAppraiser/img/icon_disable.tga` (type 1, drawn by +//! `mods/gui/minimap.lua:45`), `FuBar_WeaponRebuffFu/icon.tga` (type 9, RLE — the plugin's whole +//! icon, reached through FuBarPlugin's `Interface\AddOns\%s\icon`) and +//! `FonzSummon/img/icon_disable.tga` (type 1, shipped but unreferenced). Each drew **nothing**, +//! with a `warn!` on a terminal, and `SetTexture` still answered `1` because the resolvability +//! probe tests existence rather than decode — so the failure was invisible from Lua as well as on +//! screen (decision 2128). +//! +//! Types 3/11 (grayscale, raw/RLE) stay supported although the reference refuses them: no corpus +//! or client file is one, so this is a superset a decoder cannot be feature-detected on, and +//! removing working code with zero demand buys nothing. Stated rather than implied. //! //! [`tga_to_rgba`] validates the header before touching pixels, so it is safe to use as a //! *fallback* decoder on bytes of uncertain format (TGA has no magic; the caller sniffs `BLP2` @@ -27,6 +41,7 @@ pub fn tga_to_rgba(bytes: &[u8]) -> Result<(u32, u32, Vec)> { let id_length = header[0] as usize; let color_map_type = header[1]; let image_type = header[2]; + let color_map_origin = u16::from_le_bytes([header[3], header[4]]) as usize; let color_map_len = u16::from_le_bytes([header[5], header[6]]) as usize; let color_map_entry_bits = header[7] as usize; let width = u16::from_le_bytes([header[12], header[13]]) as u32; @@ -35,28 +50,48 @@ pub fn tga_to_rgba(bytes: &[u8]) -> Result<(u32, u32, Vec)> { let descriptor = header[17]; let rle = match image_type { - 2 | 3 => false, - 10 | 11 => true, - 1 | 9 => bail!("TGA: color-mapped image (type {image_type}) not supported"), + 1..=3 => false, + 9..=11 => true, t => bail!("TGA: unknown image type {t}"), }; + let paletted = matches!(image_type, 1 | 9); let grayscale = matches!(image_type, 3 | 11); - let bytes_per_pixel = match (bpp, grayscale) { - (24, false) => 3, - (32, false) => 4, - (8, true) => 1, + // A colour-mapped image's "pixel" is an INDEX; the palette entry's own width is separate, and + // both are needed before the stream can be read (`0x5a3b40`'s offset arithmetic uses each). + let bytes_per_pixel = match (bpp, paletted, grayscale) { + (8, true, _) => 1, + (24, false, false) => 3, + (32, false, false) => 4, + (8, false, true) => 1, _ => bail!("TGA: unsupported depth {bpp} for image type {image_type}"), }; + if paletted && color_map_type != 1 { + bail!("TGA: image type {image_type} is colour-mapped but declares no colour map"); + } + let map_entry_bytes = color_map_entry_bits.div_ceil(8); + if paletted && !matches!(color_map_entry_bits, 15 | 16 | 24 | 32) { + bail!("TGA: unsupported colour-map entry width {color_map_entry_bits}"); + } if width == 0 || height == 0 || width > 8192 || height > 8192 { bail!("TGA: implausible dimensions {width}x{height}"); } - // Skip the id field and (for a truecolor image that still carries one) the color map. + // Skip the id field and (for a truecolor image that still carries one) the color map. For a + // colour-mapped image the map is not skipped past — it is READ, and the reference reads it at + // exactly this offset (`0x5a3b40`: `entryBytes*colorMapLength + idLength + 0x12`). let map_bytes = if color_map_type == 1 { - color_map_len * color_map_entry_bits.div_ceil(8) + color_map_len * map_entry_bytes } else { 0 }; + let palette = if paletted { + let Some(p) = bytes.get(18 + id_length..18 + id_length + map_bytes) else { + bail!("TGA: truncated colour map"); + }; + p + } else { + &[][..] + }; let data_start = 18 + id_length + map_bytes; let Some(data) = bytes.get(data_start..) else { bail!("TGA: truncated before pixel data"); @@ -115,6 +150,37 @@ pub fn tga_to_rgba(bytes: &[u8]) -> Result<(u32, u32, Vec)> { }; let s = (src_y * width as usize + src_x) * bytes_per_pixel; let d = (y * width as usize + x) * 4; + if paletted { + // `0x5a3820`: `src = palette + (index - firstIndex) * entryBytes`, then a memcpy + // of one entry. `firstIndex` is the header's colour-map ORIGIN — almost always 0, + // and the arithmetic is wrong without it on the images where it is not. + let idx = (pixels[s] as usize).wrapping_sub(color_map_origin); + let e = idx * map_entry_bytes; + let Some(entry) = palette.get(e..e + map_entry_bytes) else { + bail!("TGA: colour index {} is outside the map", pixels[s]); + }; + rgba[d..d + 4].copy_from_slice(&match map_entry_bytes { + // 15/16-bit entries are ARRRRRGG GGGBBBBB little-endian; 16-bit spends the top + // bit on alpha, 15-bit leaves it unused, and both replicate the high bits down + // so that a full channel reads 0xff rather than 0xf8. + 2 => { + let v = u16::from_le_bytes([entry[0], entry[1]]); + let c = |shift: u32| { + let f = ((v >> shift) & 0x1f) as u8; + (f << 3) | (f >> 2) + }; + let a = if color_map_entry_bits == 16 && v & 0x8000 == 0 { + 0 + } else { + 0xff + }; + [c(10), c(5), c(0), a] + } + 4 => [entry[2], entry[1], entry[0], entry[3]], + _ => [entry[2], entry[1], entry[0], 0xff], + }); + continue; + } match bytes_per_pixel { 1 => { let g = pixels[s]; @@ -141,6 +207,66 @@ pub fn tga_to_rgba(bytes: &[u8]) -> Result<(u32, u32, Vec)> { mod tests { use super::*; + /// A minimal COLOUR-MAPPED TGA: header + palette + index stream. + fn tga_paletted( + image_type: u8, + w: u16, + h: u16, + palette: &[u8], + entry_bits: u8, + idx: &[u8], + ) -> Vec { + let mut out = vec![0u8; 18]; + out[1] = 1; // colour-map type: present + out[2] = image_type; + out[5..7] + .copy_from_slice(&((palette.len() / (entry_bits as usize / 8)) as u16).to_le_bytes()); + out[7] = entry_bits; + out[12..14].copy_from_slice(&w.to_le_bytes()); + out[14..16].copy_from_slice(&h.to_le_bytes()); + out[16] = 8; // 8-bit indices + out[17] = 0x20; // top-origin, so the test reads in file order + out.extend_from_slice(palette); + out.extend_from_slice(idx); + out + } + + /// **Colour-mapped TGAs decode** — types 1 and 9, the two the reference's dispatch sends to + /// `0x5a39d0` and this decoder used to refuse outright (decision 2128). + /// + /// The shape is the corpus's own: 8-bit indices into a 24-bit palette, which is what all three + /// of `FonzAppraiser/img/icon_disable.tga`, `FuBar_WeaponRebuffFu/icon.tga` and + /// `FonzSummon/img/icon_disable.tga` are. + #[test] + fn colour_mapped_images_decode_through_the_palette() { + // Palette as BGR: entry 0 red, entry 1 green, entry 2 blue. + let pal = [ + 0, 0, 255, /* red */ 0, 255, 0, /* green */ 255, 0, 0, /* blue */ + ]; + let raw = tga_paletted(1, 2, 2, &pal, 24, &[0, 1, 2, 0]); + let (w, h, rgba) = tga_to_rgba(&raw).unwrap(); + assert_eq!((w, h), (2, 2)); + assert_eq!( + rgba, + vec![ + 255, 0, 0, 255, 0, 255, 0, 255, // + 0, 0, 255, 255, 255, 0, 0, 255, + ] + ); + + // Type 9 is the same image RLE'd — the run operates on INDEX bytes, not on expanded + // pixels, which is the half a naive port gets wrong. + let rle = tga_paletted(9, 2, 2, &pal, 24, &[0x02, 0, 1, 2, 0x80, 0]); + assert_eq!(tga_to_rgba(&rle).unwrap().2, rgba); + + // An index the map cannot answer is LOUD, not a silent black pixel. + let bad = tga_paletted(1, 1, 1, &pal, 24, &[9]); + assert!(tga_to_rgba(&bad) + .unwrap_err() + .to_string() + .contains("outside the map")); + } + /// A minimal TGA: 18-byte header + pixels. fn tga(image_type: u8, w: u16, h: u16, bpp: u8, descriptor: u8, data: &[u8]) -> Vec { let mut out = vec![0u8; 18]; diff --git a/crates/benilla-ui/src/script/button.rs b/crates/benilla-ui/src/script/button.rs index 8270c8f3..83081c75 100644 --- a/crates/benilla-ui/src/script/button.rs +++ b/crates/benilla-ui/src/script/button.rs @@ -61,15 +61,15 @@ impl Slot { /// `0x778fd0` family, which stores the slot *and* pushes it to the shown pointer when it is /// the current state's. Every other slot is a plain field: the highlight, the checked pair /// and the label are not in the state array and have show rules of their own. - fn set(self, bs: &mut ButtonState, rh: crate::widget::RegionHandle) { + fn set(self, bs: &mut ButtonState, rh: Option) { match self { - Slot::Normal => bs.set_state_slot(ButtonVisualState::Normal, Some(rh)), - Slot::Pushed => bs.set_state_slot(ButtonVisualState::Pushed, Some(rh)), - Slot::Disabled => bs.set_state_slot(ButtonVisualState::Disabled, Some(rh)), - Slot::Highlight => bs.highlight = Some(rh), - Slot::Checked => bs.checked_tex = Some(rh), - Slot::DisabledChecked => bs.disabled_checked = Some(rh), - Slot::Text => bs.text = Some(rh), + Slot::Normal => bs.set_state_slot(ButtonVisualState::Normal, rh), + Slot::Pushed => bs.set_state_slot(ButtonVisualState::Pushed, rh), + Slot::Disabled => bs.set_state_slot(ButtonVisualState::Disabled, rh), + Slot::Highlight => bs.highlight = rh, + Slot::Checked => bs.checked_tex = rh, + Slot::DisabledChecked => bs.disabled_checked = rh, + Slot::Text => bs.text = rh, } } @@ -380,7 +380,7 @@ fn ensure_slot(lua: &Lua, this: &Table, slot: Slot) -> mlua::Result { model.touch_layout(); // a region entered the layout gate's read set (decision 0740) if let Some(frame) = model.arena.frame_mut(h) { if let KindState::Button(bs) = &mut frame.kind_state { - slot.set(bs, rh); + slot.set(bs, Some(rh)); } } // A freshly built slot region gets its creation-path implicit anchor (decision 1310) @@ -401,8 +401,91 @@ fn ensure_slot(lua: &Lua, this: &Table, slot: Slot) -> mlua::Result { Ok(model.region_id(rh)) } -/// The shared body of `SetTexture(path)` / `(r, g, b [, a])`. +/// Point `slot` at `rh` (or at nothing) and hand back whatever it held before, when that is a +/// different region. The reference's slot store `0x778fd0` ends in the outgoing object's +/// vtable-slot-0 dtor, so the caller frees it. +fn swap_slot( + model: &mut Model, + h: FrameHandle, + slot: Slot, + rh: Option, +) -> mlua::Result> { + let frame = model + .arena + .frame_mut(h) + .ok_or_else(|| mlua::Error::runtime("stale frame handle"))?; + let KindState::Button(bs) = &mut frame.kind_state else { + return Err(mlua::Error::runtime("not a Button")); + }; + let outgoing = slot.get(bs).filter(|old| Some(*old) != rh); + slot.set(bs, rh); + model.touch_layout(); + Ok(outgoing) +} + +/// Free the region a slot just gave up — never the one that just moved in. +fn free_outgoing(lua: &Lua, outgoing: Option, incoming: RegionHandle) { + if let Some(old) = outgoing.filter(|old| *old != incoming) { + super::region::free_region( + &mut lua.app_data_mut::().expect("model app_data"), + old, + ); + } +} + +/// The shared body of `SetTexture(texture | "path" | nil)` / `(r, g, b [, a])`. +/// +/// The reference forks on `lua_type(L, 2)` into four legs (`0x781970`, carved instruction by +/// instruction in wow-re `button-state-texture-path-setter.md` §1). **Two of them never touch the +/// slot's own region at all**, so they are resolved *before* [`ensure_slot`] — which would +/// otherwise lazily build a region for a call whose entire purpose is to replace or drop one. fn set_slot_texture(lua: &Lua, this: &Table, slot: Slot, args: &MultiValue) -> mlua::Result<()> { + match args.front() { + // **A widget object installs THAT region into the slot** (`0x781b0b`: `push the Texture + // object; push idx; call 0x778fd0`) — it does not load a path into the slot's own region. + // Three corpus addons build a highlight exactly this way and every one of them was a + // silent no-op here: Bongos' bar-drag button (`bar.lua:64-71`), _Nameplates' mouseover + // glow (`_Nameplates.lua:217-222`) and Quiver's option rows (`Quiver.bundle.lua:8949-53`) + // each `CreateTexture` on the button, colour it, `SetAllPoints`, and hand it over. + Some(Value::Table(t)) => { + let rh = super::region::region_handle_of(lua, t)?; + let h = frame_handle_of(lua, this)?; + let mut model = lua.app_data_mut::().expect("model app_data"); + // The reference type-checks the handed object and raises rather than installing a + // FontString into a texture slot (`0x781ab1`, `0x87a1e0`). + if model.arena.region(rh).map(|r| r.kind) != Some(RegionKind::Texture) { + return Err(mlua::Error::runtime("Wrong object type, expected texture")); + } + let outgoing = swap_slot(&mut model, h, slot, Some(rh))?; + drop(model); + free_outgoing(lua, outgoing, rh); + return Ok(()); + } + // **nil CLEARS the slot** (`0x781b5a`: `push 0; push idx; call 0x778fd0`, whose tail runs + // the outgoing object's dtor). Clearing the pointer alone would be worse than doing + // nothing: `ButtonState::region_visible` shows any region that is not one of the button's + // own slots unconditionally, so an unhooked state texture would start drawing in *every* + // state instead of none. `TheoryCraft/TheoryCraftUI.lua:215-217` strips a talent-rank + // button's normal/pushed/highlight art with three of these. + // + // A call with NO argument is left alone: `LUA_TNONE` is not `LUA_TNIL`, and the + // reference's own nil test is `0x781b4f call 0x6f3400 == 0`. + Some(Value::Nil) => { + let h = frame_handle_of(lua, this)?; + let mut model = lua.app_data_mut::().expect("model app_data"); + let outgoing = swap_slot(&mut model, h, slot, None)?; + drop(model); + if let Some(old) = outgoing { + super::region::free_region( + &mut lua.app_data_mut::().expect("model app_data"), + old, + ); + } + return Ok(()); + } + _ => {} + } + let id = ensure_slot(lua, this, slot)?; let mut model = lua.app_data_mut::().expect("model app_data"); let rh = *model.id_to_region.get(&id).expect("slot region id"); diff --git a/crates/benilla-ui/src/script/font.rs b/crates/benilla-ui/src/script/font.rs index 02951df3..f7ed6e2e 100644 --- a/crates/benilla-ui/src/script/font.rs +++ b/crates/benilla-ui/src/script/font.rs @@ -368,7 +368,14 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { )?, )?; // GetFont() → path, height, flags. `Tablet-2.0.lua:289`'s `_, headerSize = - // GameTooltipHeaderText:GetFont()` is the corpus's single biggest font-object read (268 sites). + // GameTooltipHeaderText:GetFont()` is the corpus's single biggest font-object read (268 sites), + // and it does arithmetic on the second value. + // + // **An unset Font answers `(nil, 0, "")`, all three ctor-determined** — `0x783a40` writes + // `[esi+0x48]` and `[esi+0x4c]` from a zeroed register, and its `0x41e3a0(NULL)` stores the + // shared empty record whose `char*` is NULL, which `lua_pushstring` turns into nil. So the + // height is the NUMBER zero, not nil (wow-re `font-object-lua-surface.md` §9.3, + // §5-cross-checked; decision 2129). m.set( "GetFont", lua.create_function(|lua, this: Table| { @@ -377,7 +384,7 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { Some(p) => Value::String(lua.create_string(&p)?), None => Value::Nil, }; - Ok((path, fo.height, fo.outline.as_str())) + Ok((path, fo.height.unwrap_or(0.0), fo.outline.as_str())) })?, )?; diff --git a/crates/benilla-ui/src/script/font_block.rs b/crates/benilla-ui/src/script/font_block.rs index 83b39fad..f6bff838 100644 --- a/crates/benilla-ui/src/script/font_block.rs +++ b/crates/benilla-ui/src/script/font_block.rs @@ -272,6 +272,19 @@ pub(super) fn install( )?; // GetFont() → 3 values: path, height, flagsString (`mov eax,3` at `0x79f407`). The flags string // is `""` when there are none — built into a zeroed static buffer at `0xceea60` — never nil. + // + // **The HEIGHT slot is a number even on a FontString that was never given a font**, and that is + // not the obvious reading: §9.3 records that the FontString reads back *through* its resolved + // `CGxFont`, so it looks as though a NULL one should nil all three. It does not. `0x7727b0`'s + // fifth instruction is an *unconditional* `fld [esi+0xe4]`, and its `+0xe0` test sits behind a + // branch `GetFont` never takes (`0x79d499 push 0`) — so slot 2 never touches the `CGxFont` at + // all and always pushes a double (wow-re `font-object-lua-surface.md` §9.3a, §5-cross-checked). + // We answer 0 there: `+0xe4` has **no constructor writer** in the reference, so its value on + // this path is a recycled float that nothing can reproduce, and 0 is what the Font object's own + // ctor-determined `+0x48` gives (decision 2129). + // + // A nil here is not a cosmetic difference. `aux-addon/tabs/search/frame.lua:481` computes + // `select(2, child:GetFont()) + arg1*2` in its font-resize wheel handler. m.set( "GetFont", lua.create_function(move |lua, this: Table| { @@ -282,7 +295,7 @@ pub(super) fn install( Some(p) => Value::String(lua.create_string(&p)?), None => Value::Nil, }; - let height = d.and_then(|d| d.font_height); + let height = d.and_then(|d| d.font_height).unwrap_or(0.0); let flags = d.map(|d| d.outline).unwrap_or_default().as_str(); Ok((path, height, flags)) })?, diff --git a/crates/benilla-ui/src/script/mail.rs b/crates/benilla-ui/src/script/mail.rs index 92183c30..e74960cf 100644 --- a/crates/benilla-ui/src/script/mail.rs +++ b/crates/benilla-ui/src/script/mail.rs @@ -453,8 +453,19 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { .is_none_or(|r| super::item_stats::item_usable_by_id(&model, r.item_id)); (row, usable) }; + // **Five values on every path**, and the empty leg is `(nil, nil, 0, 0, nil)` — + // `GetInboxItem 0x4af5d0`, the sibling of the `GetSendMailItem` block above (wow-re + // `mail-interaction.md` §5.1; decision 2129). This answered ONE value, which a caller + // destructuring five reads as four nils — and the reference's own row + // (`(nil,nil,number,number,nil) | …`) has no one-value alternative at all. let Some(r) = row.filter(|r| r.item_id != 0) else { - return Ok(MultiValue::from_vec(vec![Value::Nil])); + return Ok(MultiValue::from_vec(vec![ + Value::Nil, + Value::Nil, + Value::Integer(0), + Value::Integer(0), + Value::Nil, + ])); }; let name = match &r.item_name { Some(n) => Value::String(lua.create_string(n)?), @@ -464,10 +475,8 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { Some(t) => Value::String(lua.create_string(t)?), None => Value::Nil, }; - let quality = match r.item_quality { - Some(q) => Value::Integer(i64::from(q)), - None => Value::Nil, - }; + // A number on every path, like the send tab's. + let quality = Value::Integer(i64::from(r.item_quality.unwrap_or(0))); Ok(MultiValue::from_vec(vec![ name, texture, @@ -643,7 +652,21 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { )?; // GetSendMailItem() → name, texture, stackCount, quality off the attached cursor item - // (MailFrame.lua l.511). All-nil/1 when nothing is attached. + // (MailFrame.lua l.511). + // + // **The empty leg is `(nil, nil, 0, 0)`** — four values, and slots 3 and 4 are NUMBERS. Read at + // `0x4ae590`: `0x4ae6d3`/`0x4ae6da` push nil, then two `push 0; push 0; lua_pushnumber` pairs + // at `0x4ae6df`/`0x4ae6ea`, `eax = 4` (wow-re `mail-interaction.md` §5.1, §5-cross-checked; + // decision 2129). All three of the reference's failure guards share that one block, so "nothing + // attached" and "the item template has not loaded yet" are indistinguishable to a script. + // + // The `1` this used to push in slot 3 was borrowed from the wrong binding: it is + // `GetAuctionSellItemInfo 0x4ce590`'s empty leg (`1.0`/`-1.0`), not this one's. + // + // **Still not faithful, and stated rather than implied:** on the LOADED leg the reference reads + // quality from `[rec+0x1c]` gated on `[rec+0x2c] != 0` = InventoryType, so a **non-equippable** + // item answers quality `-1` (`0x4ae6bf`). We do not carry an inventory type here, so an item + // whose quality we have not cached answers 0 rather than -1. g.set( "GetSendMailItem", lua.create_function(|lua, ()| { @@ -660,8 +683,8 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { return Ok(MultiValue::from_vec(vec![ Value::Nil, Value::Nil, - Value::Integer(1), - Value::Nil, + Value::Integer(0), + Value::Integer(0), ])); }; let name = cursor::item_link_name(it.link.as_deref()); @@ -674,10 +697,8 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { Some(t) => Value::String(lua.create_string(t)?), None => Value::Nil, }; - let quality = match it.quality { - Some(q) => Value::Integer(i64::from(q)), - None => Value::Nil, - }; + // A number on every path — the reference's row has no nil alternative in this slot. + let quality = Value::Integer(i64::from(it.quality.unwrap_or(0))); Ok(MultiValue::from_vec(vec![ name, texture, diff --git a/crates/benilla-ui/src/script/messageframe/scrolling.rs b/crates/benilla-ui/src/script/messageframe/scrolling.rs index 23069ef1..e11ba4b4 100644 --- a/crates/benilla-ui/src/script/messageframe/scrolling.rs +++ b/crates/benilla-ui/src/script/messageframe/scrolling.rs @@ -49,6 +49,17 @@ fn with_smf( } } +/// `lua_isnumber` semantics — a number, or a string Lua would coerce to one. The rgb trio's +/// presence test is three of these (decision 2125), so a caller passing `"1"` still colours the +/// line and a caller passing `nil` or a table does not. +fn is_number(v: &Value) -> bool { + match v { + Value::Integer(_) | Value::Number(_) => true, + Value::String(s) => s.to_str().is_ok_and(|s| s.trim().parse::().is_ok()), + _ => false, + } +} + /// A Lua number-ish → f32 (nil/other → 0.0), for the color args. fn num_f32(v: &Value) -> f32 { match v { @@ -92,9 +103,19 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { // method-table bytes that put all four on BOTH tables. super::install_justify(lua, &m, "ScrollingMessageFrame")?; - // AddMessage(text [, r, g, b [, id]]) — binding 0x792900, the id (5th arg) accepted and - // ignored (v1 has no per-line UpdateColorByID). rgb absent ⇒ white; the state quantizes - // round-half-up and forces the line opaque, then the fade drives its alpha. + // AddMessage(text [, r, g, b [, id]]) — binding 0x792900. rgb absent ⇒ white; the state + // quantizes round-half-up and forces the line opaque, then the fade drives its alpha. + // + // **The rgb gate is three separate `lua_isnumber` calls on indices 3/4/5, and the id's index + // is LEG-DEPENDENT** (decision 2125; wow-re `login-chat-colour-pipeline.md`): 6 when rgb are + // present (`0x792b13 mov edx,6`), **3** when they are absent (`0x792b48 mov edx,3`) — which is + // what makes the documented `AddMessage(text, id)` shorthand work. So `AceConsole-2.0`'s + // `AddMessage(text, nil, nil, nil, nil, 5)` fails the R gate at index 3, takes the absent leg, + // re-reads index 3 as the id — still nil — and stores **0**; its trailing `5` sits at index 7 + // and is never read at all. + // + // Absent or non-numeric is id 0, and 0 is a line `UpdateColorByID` can never find: that guard + // is the reference's own, at `0x788250`'s first branch. // // The TEXT gate is the sibling class's, verbatim — gate `0x79299c`, `je 0x792b81` = this // function's own epilogue — so `DEFAULT_CHAT_FRAME:AddMessage(nil)` is silent too. See @@ -106,24 +127,21 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { let Some(text) = super::message_text(lua, &text) else { return Ok(()); }; - let has_rgb = !matches!( - (&r, &g, &b), - (Value::Nil, _, _) | (_, Value::Nil, _) | (_, _, Value::Nil) - ); + let has_rgb = is_number(&r) && is_number(&g) && is_number(&b); + // The id is the chat-type index (`GetChatTypeIndex`) the line is printed under — + // `ChatFrame.lua` passes `info.id` — read from the leg's OWN stack slot: the + // sixth argument when rgb are present, the third (i.e. where `r` would be) when + // they are not. + let id = match if has_rgb { &id } else { &r } { + Value::Integer(i) => u32::try_from(*i).unwrap_or(0), + Value::Number(n) if n.is_finite() && *n >= 0.0 => *n as u32, + _ => 0, + }; let (r, g, b) = if has_rgb { (num_f32(&r), num_f32(&g), num_f32(&b)) } else { (1.0, 1.0, 1.0) }; - // The fifth argument is the chat-type index (`GetChatTypeIndex`) the line is - // printed under — `ChatFrame.lua` passes `info.id` — and is what - // `UpdateColorByID` keys on. Absent or non-numeric is 0: a line no recolour - // will ever find. - let id = match &id { - Value::Integer(i) => u32::try_from(*i).unwrap_or(0), - Value::Number(n) if n.is_finite() && *n >= 0.0 => *n as u32, - _ => 0, - }; with_smf(lua, &this, |smf| smf.add_with_id(text, r, g, b, id)) }, )?, @@ -807,6 +825,36 @@ mod tests { assert!(!s.add_chat_message("PlainFrame", "x", 1.0, 1.0, 1.0)); } + /// **`AddMessage`'s id lives at a different stack index on each leg** (decision 2125): the + /// sixth argument when r,g,b are present, the THIRD when they are not — which is what makes + /// the `AddMessage(text, id)` shorthand work at all (`0x792b13 mov edx,6` / + /// `0x792b48 mov edx,3`). + /// + /// `AceConsole-2.0`'s `Print` is the case that matters: `AddMessage(text, nil, nil, nil, nil, + /// 5)` fails the R gate at index 3, takes the absent leg, re-reads index 3 as the id — still + /// nil — and stores 0. Its trailing `5` sits at index 7 and is never read. + #[test] + fn the_addmessage_id_index_follows_the_rgb_leg() { + let s = UiScript::new().unwrap(); + s.run( + "BenillaMF = CreateFrame('ScrollingMessageFrame') \ + BenillaMF:AddMessage('shorthand', 5) \ + BenillaMF:AddMessage('aceconsole', nil, nil, nil, nil, 5) \ + BenillaMF:AddMessage('coloured', 1, 1, 1, 7)", + ) + .unwrap(); + let frame: mlua::Table = s.eval("return BenillaMF").unwrap(); + let ids: Vec = super::with_smf(s.lua(), &frame, |smf| { + smf.lines.iter().map(|l| l.id).collect() + }) + .unwrap(); + assert_eq!( + ids, + vec![5, 0, 7], + "shorthand id from index 3; AceConsole's nils are id 0; the coloured line's id 7" + ); + } + /// `UpdateColorByID` recolours exactly the lines printed under that id — the stock /// `ChatFrame_OnEvent` repaints a type's history when its colour changes, and nothing else. #[test] @@ -831,6 +879,20 @@ mod tests { assert_ne!(smf.lines_gen, gen, "a recolour is a redraw"); assert_eq!(smf.update_color_by_id(11, 0.0, 0.5, 1.0), 0, "idempotent"); assert_eq!(smf.update_color_by_id(99, 0.0, 0.0, 0.0), 0, "no such id"); + // **Id 0 matches nothing** — `0x788250`'s own opening guard (decision 2125). Line "d" was + // printed with no id, so it carries 0; so does `ChatTypeInfo["REPLY"]`, which the + // `UPDATE_CHAT_COLOR` handler mirrors WHISPER into. Without the guard that pair repainted + // every colourless line in the window whisper-pink at every login. + assert_eq!( + smf.update_color_by_id(0, 1.0, 0.5, 1.0), + 0, + "id 0 never reaches the record walk" + ); + assert_eq!( + smf.lines[3].color, + [255, 255, 255], + "the colourless line stays the frame's own colour" + ); // And the Lua surface: the fifth argument tags, the method recolours, silently. let s = UiScript::new().unwrap(); diff --git a/crates/benilla-ui/src/script/object/events_regions.rs b/crates/benilla-ui/src/script/object/events_regions.rs index b01258ce..83d612ef 100644 --- a/crates/benilla-ui/src/script/object/events_regions.rs +++ b/crates/benilla-ui/src/script/object/events_regions.rs @@ -106,10 +106,15 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { // Making it exact means per-type SCRIPT_KINDS, which newly RAISES on 30 sites that work today // (9 in our own FrameXML). That is behaviour removal and belongs in its own change, with its // own measurement; it is not a tail on this one. + // + // 1/nil, not a Lua boolean: the reference's row is `(nil) | (number)` like every other + // predicate (decision 2118). m.set( "HasScript", lua.create_function(|_, (_this, name): (Table, String)| { - Ok(SCRIPT_KINDS.iter().any(|k| k.eq_ignore_ascii_case(&name))) + Ok(crate::script::binding_abi::flag( + SCRIPT_KINDS.iter().any(|k| k.eq_ignore_ascii_case(&name)), + )) })?, )?; // RegisterForDrag(...varargs of button names) — the drag-gesture twin of `RegisterForClicks` diff --git a/crates/benilla-ui/src/script/region/paint.rs b/crates/benilla-ui/src/script/region/paint.rs index 4d7ae3b8..b930ab6a 100644 --- a/crates/benilla-ui/src/script/region/paint.rs +++ b/crates/benilla-ui/src/script/region/paint.rs @@ -221,16 +221,38 @@ pub(super) fn install(lua: &Lua, m: &Table) -> mlua::Result<()> { data.fill = None; false } + // **Ask the probe BEFORE writing the slot.** The reference's load-FAILURE + // arm (`cmp [ebp-4],2; jl` at `0x770288` → `0x77028e`–`0x7702b2`) releases + // the handle it just built and returns 0 **leaving `+0xcc` and `+0x128` as + // they were** — "the widget keeps whatever texture it already had" + // (wow-re `texture-service-name-resolution.md` §161-169). Storing the path + // first and letting the verdict drive only the *return value* meant a + // mistyped or not-yet-shipped path ERASED the art it failed to replace, + // and did it silently: `GetTexture()` echoed the missing path and the quad + // was dropped, so the region went blank with nothing said (decision 2124). + // + // The store and the return ask deliberately different questions. A VM with + // **no probe installed** has no backend to ask, so it stores — that is + // every engine-less test's world, and the module head already states that + // such a VM's path form "stays nil", which is the return, not the slot. Value::String(s) => { let path = s.to_str()?.to_string(); - data.texture = Some(path.clone()); - data.fill = None; drop(model); - let model = lua.app_data_ref::().expect("model"); - model - .texture_probe - .as_ref() - .is_some_and(|probe| probe(&path)) + let resolvable = { + let model = lua.app_data_ref::().expect("model"); + model + .texture_probe + .as_ref() + .is_none_or(|probe| probe(&path)) + }; + let mut model = lua.app_data_mut::().expect("model"); + let had_probe = model.texture_probe.is_some(); + if resolvable { + let data = model.region_data.entry(rh).or_default(); + data.texture = Some(path); + data.fill = None; + } + resolvable && had_probe } // The colour form, and the ONLY branch that looks at the trailing three. A // non-numeric there takes the same default a missing one does, which is what diff --git a/crates/benilla-ui/src/script/stdlib.rs b/crates/benilla-ui/src/script/stdlib.rs index ad029078..4ca172dd 100644 --- a/crates/benilla-ui/src/script/stdlib.rs +++ b/crates/benilla-ui/src/script/stdlib.rs @@ -38,39 +38,58 @@ pub(super) fn sandbox(lua: &Lua) -> mlua::Result<()> { g.set(name, Value::Nil)?; } - // `debugstack([start [, count1 [, count2]]])` — a REAL traceback, not the `""` stub this used - // to be. + // `debugstack([start [, count1 [, count2]]])` — the client's own traceback, **frames only**. // - // The stub was justified as "real addons call it in error paths; returning \"\" keeps them - // running", and that was true of the addons that only ever DISPLAY it. It is false of the ones - // that PARSE it, and the biggest family in the corpus is one of those: `FuBarPlugin-2.0.lua:752` - // derives every plugin's own folder with - // `string.find(debugstack(6, 1, 0), "\\AddOns\\(.*)\\")`, then formats the capture into an - // icon path. Against `""` the find returns nil, `folderName` stays nil, and the plugin dies in - // `format` — 20 corpus addons, every one of them a FuBar plugin, and the error surfaced as a - // `format` complaint three frames away from the cause. + // The stub this replaced returned `""`, which was justified as "real addons only display it". + // False: the corpus PARSES it, three different ways, and every one of them reads the string's + // shape rather than its contents. // - // `start` is the stack level to begin at and defaults to 1, matching the client's own callers - // (`debugstack(6, 1, 0)` walks past FuBar's own wrapper frames to the plugin file). The two - // count arguments bound how many frames are shown at the top and bottom; mlua's traceback does - // not take them, so they are accepted and ignored — stated rather than silently dropped, and - // harmless because every corpus caller passes a shape whose whole purpose is "give me the - // frames", never an exact line count. + // **There is no `stack traceback:` header, and line 1 is the caller's frame** (decision 2121). + // mlua's `Lua::traceback` is `luaL_traceback`, which emits that header — and the header cost a + // whole addon family its saved variables. `AceDB-2.0:RegisterDB` (`AceDB-2.0.lua:742`) does + // + // local addonName = string.gsub(debugstack(), ".-\n.-\\AddOns\\(.-)\\.*", "%1") + // + // — skip exactly ONE line (its own frame), read the folder out of the NEXT one (its caller's). + // With a header in front, the "next" line is AceDB's own chunk, so every Ace2 addon resolved to + // whichever addon happened to ship the winning copy of the library. `AceDB.addonsLoaded` has + // that one flagged already, so `RegisterDB` took its *immediate* branch and bound `db.raw` to a + // fresh table at the addon's FILE SCOPE — before the SavedVariables chunk runs. The chunk then + // rebound the global and `db.raw` kept pointing at the orphan, so Bartender2 printed + // `Creating new DB` at every login and its file was frozen at what a first-run reset writes. + // Measured live: `Bartender.db.raw == BarDB` was **false**. + // + // Two more corpus readers pin the same shape independently, and they are the reason this emits + // Lua 5.0's frame wording rather than 5.4's: + // - `AceLibrary.lua:70` takes the FIRST line whole (`string.gsub(stack, "\n.*", "")`) and + // matches it against `".*\\(.*).lua:%d+: .*"` — so line 1 is a frame, `:: …`, + // with no leading newline and no header above it. + // - `AceLibrary.lua:139`'s `argCheck` reads the function name with `"([`<].-['>])"` — a + // BACKTICK or `<` opening it. 5.0 writes ``in function `name'``; 5.4 writes + // `in function 'name'`, which that pattern cannot match at all. + // + // So the body is `db_errorfb`'s (Lua 5.0 `ldblib.c`) with its header line removed: per frame + // `short_src`, then `:line` when there is one, then one of `` in function `name' ``, + // `" in main chunk"`, `" ?"` (a C function) or `" in function "`. + // + // `start` is the level to begin at, default 1 = the caller (level 0 is this binding itself). + // `count1`/`count2` are 5.0's `LEVELS1`/`LEVELS2`: show `count1` frames from the top, then + // `...`, then the last `count2`. They are honoured rather than ignored because + // `FuBarPlugin-2.0.lua:752` passes `debugstack(6, 1, 0)` and reads the capture with a GREEDY + // `"\\AddOns\\(.*)\\"` — extra frames would hand it the LAST addon on the stack instead + // of its own. g.set( "debugstack", lua.create_function(|lua, args: Variadic| { - let start = match args.first() { - Some(Value::Integer(i)) => *i as usize, - Some(Value::Number(n)) => *n as usize, - _ => 1, + let num = |v: Option<&Value>, default: usize| match v { + Some(Value::Integer(i)) => usize::try_from(*i).unwrap_or(0), + Some(Value::Number(n)) if *n >= 0.0 => *n as usize, + _ => default, }; - // A level past the top of the stack is not an error here, it is an empty trace — the - // same shape the old stub returned, so a caller that guessed too deep still gets a - // string rather than a raise. - Ok(lua - .traceback(None, start) - .and_then(|t| t.to_str().map(|s| s.to_owned())) - .unwrap_or_default()) + let start = num(args.first(), 1); + let count1 = num(args.get(1), 12); + let count2 = num(args.get(2), 10); + Ok(traceback_frames(lua, start, count1, count2)) })?, )?; @@ -593,6 +612,135 @@ fn format_epoch(secs: i64, fmt: &str) -> String { } out } +/// `luaO_chunkid` (`0x6f5c40`), the reference's own — because `debugstack`'s frames carry +/// `short_src`, and **it truncates from the FRONT** (decision 2121, wow-re +/// `system/ui/scratch/debugstack-return-shape.md`). +/// +/// An `@`-named chunk longer than [`CHUNKID_KEEP`] characters after the `@` becomes `"..."` plus +/// its LAST [`CHUNKID_KEEP`], so the head of the path is what is lost. That is not a detail: over a +/// 2189-file vanilla corpus, 60% of addon `.lua` chunk names truncate and **38% lose `\AddOns\` +/// entirely**, which is exactly the substring three different corpus libraries search for. A client +/// that keeps the whole name matches at a different frame than the reference for a third of library +/// frames — silently, and in the addon's favour, which is worse than failing the same way. +fn chunk_id(source: &str) -> String { + if let Some(rest) = source.strip_prefix('=') { + // A `=name` chunk is taken verbatim, clipped to the buffer from the front. + return rest.chars().take(LUA_IDSIZE - 1).collect(); + } + if let Some(rest) = source.strip_prefix('@') { + let n = rest.chars().count(); + if n <= CHUNKID_KEEP { + return rest.to_string(); + } + let tail: String = rest.chars().skip(n - CHUNKID_KEEP).collect(); + return format!("...{tail}"); + } + // A string chunk: `[string ""]`. + let first = source.split('\n').next().unwrap_or_default(); + let budget = LUA_IDSIZE - STRING_CHUNK_OVERHEAD; + if first.chars().count() > budget || first.len() < source.len() { + let clipped: String = first.chars().take(budget).collect(); + format!("[string \"{clipped}...\"]") + } else { + format!("[string \"{first}\"]") + } +} + +/// `LUA_IDSIZE` — the `short_src` buffer, 60 bytes including the terminator. +const LUA_IDSIZE: usize = 60; + +/// How many characters of an `@`-named chunk survive truncation, tail-first: the reference keeps +/// the last 52 behind a `"..."` (wow-re's measurement off `0x6f5c40`). +const CHUNKID_KEEP: usize = 52; + +/// What `[string "…"]` costs the budget in the third `luaO_chunkid` arm. +const STRING_CHUNK_OVERHEAD: usize = 17; + +/// One `debugstack` frame, in the reference's own wording (decision 2121): +/// `short_src ":" [currentline ":"] DESC`, with the `\n` pushed **after** it by the caller. +/// +/// `DESC` is the `*namewhat` switch at `0x7038fa`: a `f`/`g`/`l`/`m` name renders +/// `` in function `%s' `` (`0x872c98`) — Lua **5.0**'s backtick quoting, which is what +/// `AceLibrary.lua:139`'s `"([`<].-['>])"` needs and what 5.4's `'%s'` cannot satisfy — and +/// otherwise the `*what` arms decide: `m` → `" in main chunk"` (`0x872c88`), `C` or `t` → `" ?"` +/// (`0x872c6c`), else `" in function <%s:%d>"` (`0x872c70`). +fn traceback_frame(d: &mlua::Debug) -> String { + let src = d.source(); + let names = d.names(); + let short = src + .source + .as_deref() + .map(chunk_id) + .or_else(|| src.short_src.as_deref().map(str::to_owned)) + .unwrap_or_else(|| "?".to_string()); + let mut line = String::with_capacity(short.len() + 32); + line.push_str(&short); + line.push(':'); + if let Some(n) = d.current_line() { + line.push_str(&n.to_string()); + line.push(':'); + } + // A tail call has no calling instruction to name it from. 5.0 gave that frame `what == "tail"` + // and the `" ?"` arm; 5.4 hands us a `(tail call)` pseudo-frame instead, which lands in the + // same arm by the same rule. + let tail_call = src.what == "t" || short == "(tail call)"; + match names.name.as_deref() { + Some(name) if !name.is_empty() && !tail_call => { + line.push_str(" in function `"); + line.push_str(name); + line.push('\''); + } + _ if src.what == "main" => line.push_str(" in main chunk"), + _ if src.what == "C" || tail_call => line.push_str(" ?"), + _ => { + line.push_str(" in function <"); + line.push_str(&short); + line.push(':'); + line.push_str(&src.line_defined.unwrap_or(0).to_string()); + line.push('>'); + } + } + line +} + +/// `debugstack`'s body — `0x703760`'s walk, which is stock Lua 5.0's `db_errorfb` with its header +/// and its `"\n\t"` prefix replaced by a `"\n"` pushed **after** each frame (`0x703971`). That one +/// substitution is the whole difference in shape, and it is why line 1 is a frame and why +/// `AceDB-2.0`'s skip-one-line lands on its caller (decision 2121). +/// +/// **`count1` bounds nothing on its own.** The loop formats while the level is `<= start + count1` +/// (`0x703857` is `jbe`, unsigned ≤), and past that it probes `getstack(level + count2)`: a probe +/// that FAILS steps back and prints that level as an ordinary frame, so `count1 = 1` returns *two* +/// frames on a two-deep stack and one frame plus `"...\n"` on a three-deep one. Clamping to +/// `count1` instead would diverge from the reference at exactly `depth == count1 + 1` — which is +/// `FuBarPlugin-2.0.lua:752`'s `debugstack(6, 1, 0)`, whose greedy `"\\AddOns\\(.*)\\"` reads the +/// LAST path in the string. wow-re `debugstack-return-shape.md` maps the whole regime. +fn traceback_frames(lua: &Lua, start: usize, count1: usize, count2: usize) -> String { + let mut out = String::new(); + let mut level = start; + let mut first_part = true; + while lua.inspect_stack(level, |_| ()).is_some() { + level += 1; + if level > start + count1 && first_part { + first_part = false; + if lua.inspect_stack(level + count2, |_| ()).is_none() { + level -= 1; // the probe found nothing to elide — print this level after all + } else { + out.push_str("...\n"); + while lua.inspect_stack(level + count2, |_| ()).is_some() { + level += 1; + } + } + continue; + } + if let Some(f) = lua.inspect_stack(level - 1, traceback_frame) { + out.push_str(&f); + out.push('\n'); + } + } + out +} + #[cfg(test)] mod date_table_tests { use crate::script::UiScript; diff --git a/crates/benilla-ui/src/script/tests/button.rs b/crates/benilla-ui/src/script/tests/button.rs index a6046500..c3b91c80 100644 --- a/crates/benilla-ui/src/script/tests/button.rs +++ b/crates/benilla-ui/src/script/tests/button.rs @@ -1166,3 +1166,81 @@ fn a_button_labels_own_setfont_survives_the_state_font_repoint() { "an axis the label never set still inherits — severance is per-axis" ); } + +/// **A state-texture setter takes an OBJECT and takes nil**, not only a path — the reference's +/// `0x781970` forks on `lua_type(L, 2)` into four legs and benilla honoured one of them +/// (wow-re `button-state-texture-path-setter.md` §1; decision 2124). +/// +/// Both missing legs are silent no-ops rather than errors, which is why nothing caught them: +/// +/// * **the object leg** (`0x781b0b` → `0x778fd0`) installs the handed Texture *into the slot*. +/// Three corpus addons build a highlight this way and every one of them drew nothing — +/// `Bongos/bar.lua:64-71`, `_Nameplates/_Nameplates.lua:217-222`, +/// `Quiver/Quiver.bundle.lua:8949-8953`. The idiom below is Bongos', verbatim in shape. +/// * **the nil leg** (`0x781b5a` → `0x778fd0` with 0) clears the slot and dtors what was in it. +/// `TheoryCraft/TheoryCraftUI.lua:215-217` strips a talent-rank button with three of these. +/// Clearing the *pointer* alone would be worse than the no-op: `region_visible` draws any region +/// that is not one of the button's slots unconditionally, so a merely unhooked state texture +/// would appear in every state instead of none — which is what this test's last assertion pins. +#[test] +fn a_state_texture_slot_takes_an_object_and_a_nil() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + b = CreateFrame("Button", "SlotBtn") + b:SetPoint("BOTTOMLEFT", 0, 0); b:SetSize(100, 100) + b:SetNormalTexture("Interface\\N.blp") + -- Bongos' own idiom: build the highlight yourself and hand the object over. + hl = b:CreateTexture() + hl:SetTexture("Interface\\OWN.blp") + hl:SetAllPoints(b) + b:SetHighlightTexture(hl) + "#, + ) + .unwrap(); + s.resolve(); + + let drawn = |s: &UiScript| -> Vec { + s.extract() + .iter() + .filter_map(|q| match &q.content { + QuadContent::Texture { path: Some(p), .. } => Some(p.clone()), + _ => None, + }) + .collect() + }; + + // The handed object IS the slot now: it draws on hover and only on hover, exactly as a + // path-loaded highlight does. + assert_eq!(drawn(&s), vec!["Interface\\N.blp".to_string()]); + assert!( + s.eval::("return b:GetHighlightTexture() == hl") + .unwrap(), + "the getter must hand back the object that was installed, not a slot region of our own" + ); + s.mouse_move(50.0, 50.0); // hover, so the highlight slot draws + s.resolve(); + assert_eq!( + drawn(&s), + vec![ + "Interface\\N.blp".to_string(), + "Interface\\OWN.blp".to_string() + ] + ); + + // nil clears — and the cleared art is gone from every state, not merely unhooked. + s.run("b:SetHighlightTexture(nil) b:SetNormalTexture(nil)") + .unwrap(); + s.resolve(); + assert!( + drawn(&s).is_empty(), + "a cleared slot still draws: {:?}", + drawn(&s) + ); + assert!( + s.eval::("return b:GetHighlightTexture() == nil") + .unwrap(), + "the slot must read empty after nil" + ); +} diff --git a/crates/benilla-ui/src/script/tests/regions.rs b/crates/benilla-ui/src/script/tests/regions.rs index 3603a407..5d539add 100644 --- a/crates/benilla-ui/src/script/tests/regions.rs +++ b/crates/benilla-ui/src/script/tests/regions.rs @@ -1333,3 +1333,56 @@ fn an_empty_fontstring_reads_back_nil_and_an_edit_box_does_not() { ); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } + +/// **A `SetTexture` the host cannot resolve leaves the region's art alone** — it does not overwrite +/// it with the path that failed (decision 2124). +/// +/// The reference's load-failure arm is explicit about it: `0x770288 cmp [ebp-4],2; jl` → +/// `0x77028e`–`0x7702b2` releases the handle it just built and returns 0 **without ever touching +/// `+0xcc`** (wow-re `texture-service-name-resolution.md`). Ours stored the path first and used the +/// probe's verdict only for the return value, so a mistyped or not-yet-shipped path erased the art +/// it failed to replace — `GetTexture()` echoed the miss, and the extract dropped the quad, so the +/// region went blank with nothing said anywhere. +/// +/// The control that must not change is the second half: a resolvable path still replaces, and a VM +/// with **no probe at all** still stores, because it has no backend to ask. +#[test] +fn an_unresolvable_set_texture_keeps_the_art_the_region_had() { + let mut s = script(); + s.set_texture_probe(Box::new(|path: &str| !path.contains("Nope"))); + s.run( + r#" + f = CreateFrame("Frame", "ProbeHost") + t = f:CreateTexture("ProbeTex") + t:SetTexture("Interface\\Real") + "#, + ) + .unwrap(); + assert_eq!( + s.eval::("return t:GetTexture()").unwrap(), + "Interface\\Real" + ); + + // The miss: nil back, and the art it could not replace is still there. + assert!( + s.eval::(r#"return t:SetTexture("Interface\\Nope") == nil"#) + .unwrap(), + "an unresolvable path must answer nil" + ); + assert_eq!( + s.eval::("return t:GetTexture()").unwrap(), + "Interface\\Real", + "the failed load overwrote the art the region was holding" + ); + + // The control: a hit still replaces, and an explicit clear still clears. + assert!(s + .eval::(r#"return t:SetTexture("Interface\\Other") == 1"#) + .unwrap()); + assert_eq!( + s.eval::("return t:GetTexture()").unwrap(), + "Interface\\Other" + ); + s.run("t:SetTexture(nil)").unwrap(); + assert!(s.eval::("return t:GetTexture() == nil").unwrap()); +} diff --git a/crates/benilla-ui/src/script/tests/stdlib.rs b/crates/benilla-ui/src/script/tests/stdlib.rs index 131311c4..9c10bfa3 100644 --- a/crates/benilla-ui/src/script/tests/stdlib.rs +++ b/crates/benilla-ui/src/script/tests/stdlib.rs @@ -92,22 +92,137 @@ fn sandbox_removes_dangerous_globals() { ) .unwrap(); assert!(all_nil); - // `debugstack` survives the sandbox — and now returns a REAL traceback, not the `""` this - // used to assert. The stub was fine for addons that only DISPLAY it and wrong for the ones - // that PARSE it: `FuBarPlugin-2.0.lua:752` finds each plugin's own folder in - // `debugstack(6, 1, 0)`, and against `""` that returned nil and killed 20 corpus addons. + // `debugstack` survives the sandbox — and returns a REAL traceback, not the `""` this used to + // assert. The stub was fine for addons that only DISPLAY it and wrong for the ones that PARSE + // it: `FuBarPlugin-2.0.lua:752` finds each plugin's own folder in `debugstack(6, 1, 0)`, and + // against `""` that returned nil and killed 20 corpus addons. let trace = s.eval::("return debugstack()").unwrap(); assert!( - trace.contains("traceback"), + !trace.is_empty(), "debugstack must return a real traceback: {trace:?}" ); - // A level far past the top of the stack yields the bare header and never raises — a caller - // that guesses too deep still gets a string it can `string.find` against, which is exactly how - // every corpus caller uses it. - assert_eq!( - s.eval::("return debugstack(99)").unwrap().trim(), - "stack traceback:" + // **Frames only — no `stack traceback:` header** (decision 2121). Line 1 IS a frame, which is + // what `AceLibrary.lua:70` reads (`string.gsub(stack, "\n.*", "")` then + // `".*\\(.*).lua:%d+: .*"`), and what `AceDB-2.0.lua:742` counts on when it skips exactly one + // line to reach its caller's. + assert!( + !trace.starts_with("stack traceback"), + "the reference has no header line: {trace:?}" ); + // A level far past the top of the stack is an empty string, never a raise — a caller that + // guesses too deep still gets something it can `string.find` against. + assert_eq!(s.eval::("return debugstack(99)").unwrap(), ""); +} + +/// **AceDB-2.0's own capture, run for real across two chunks** (decision 2121). +/// +/// `RegisterDB` reads the calling addon's folder out of `debugstack()` by skipping exactly one +/// line — its own frame — and taking the `\AddOns\\` out of the next. That only works if +/// line 1 is a frame; with mlua's `stack traceback:` header in front, the capture returned the +/// folder of whichever addon shipped the winning copy of the library, `RegisterDB` took its +/// already-loaded branch, and `db.raw` was bound to a fresh table at the addon's file scope — +/// before the SavedVariables chunk ran. Bartender2 then printed `Creating new DB` at every login. +/// +/// The two chunks here are the real configuration: the library lives in one addon's folder, the +/// caller in another's. +#[test] +fn acedbs_capture_names_the_calling_addon_not_the_librarys_owner() { + let s = script(); + s.run_chunk_named( + b"function BenillaProbeRegisterDB() + return string.gsub(debugstack(), \".-\\n.-\\\\AddOns\\\\(.-)\\\\.*\", \"%1\") + end", + &crate::script::addon_chunk_name("AtlasLoot", "Libs\\AceDB-2.0\\AceDB-2.0.lua"), + ) + .unwrap(); + s.run_chunk_named( + b"BenillaProbeCaller = BenillaProbeRegisterDB()", + &crate::script::addon_chunk_name("Bartender2", "Bartender2.lua"), + ) + .unwrap(); + assert_eq!( + s.eval::("return BenillaProbeCaller").unwrap(), + "Bartender2", + "the capture must name the CALLER's addon, not the library owner's" + ); +} + +/// `AceLibrary.lua:70`'s reader: the first line, whole, is a frame it can pull a file name out of. +#[test] +fn the_first_debugstack_line_is_a_frame() { + let s = script(); + s.run_chunk_named( + b"function BenillaProbeFirstLine() + local first = string.gsub(debugstack(), \"\\n.*\", \"\") + return string.gsub(first, \".*\\\\(.*).lua:%d+: .*\", \"%1\") + end", + &crate::script::addon_chunk_name("Atlas", "Libs\\AceLibrary\\AceLibrary.lua"), + ) + .unwrap(); + assert_eq!( + s.eval::("return BenillaProbeFirstLine()").unwrap(), + "AceLibrary", + "line 1 is the calling function's own frame" + ); +} + +/// `AceLibrary.lua:139`'s `argCheck` names the offending function with `"([`<].-['>])"` — a +/// BACKTICK opening it. Lua 5.4 writes `in function 'name'`, which that pattern cannot match; the +/// reference's 5.0 wording is `` in function `name' ``. +#[test] +fn a_named_frame_uses_the_5_0_backtick_quoting() { + let s = script(); + let found: String = s + .eval( + "function BenillaProbeNamed() local _, _, f = string.find(debugstack(), \"([`<].-['>])\") return f or '' end local r = BenillaProbeNamed() return r", + ) + .unwrap(); + assert_eq!(found, "`BenillaProbeNamed'"); +} + +/// **Every frame carries its own trailing `\n`, and `count1` bounds nothing on its own** +/// (decision 2121, wow-re `debugstack-return-shape.md` §3.1). +/// +/// Stock Lua 5.0 pushes `"\n\t"` *before* each frame plus a header once; the reference pushes a +/// single `"\n"` after each frame and no header (`0x703971`). And the walk formats while the level +/// is `<= start + count1` — `0x703857` is `jbe`, unsigned ≤ — then probes +/// `getstack(level + count2)`: a probe that FAILS steps back and prints that level as an ordinary +/// frame. So with `count1 = 1, count2 = 0` a two-deep stack returns TWO frames and no marker, and a +/// three-deep one returns one frame plus `"...\n"` and nothing after. A client that clamped to +/// `count1` would diverge at exactly `depth == count1 + 1` — and that is +/// `FuBarPlugin-2.0.lua:752`'s `debugstack(6, 1, 0)`, whose capture is a GREEDY +/// `"\\AddOns\\(.*)\\"` reading the LAST path in the string. +#[test] +fn debugstack_frames_end_in_newline_and_count1_does_not_clamp() { + let s = script(); + let trace = s.eval::("return debugstack()").unwrap(); + assert!( + trace.ends_with('\n'), + "the newline is pushed AFTER each frame: {trace:?}" + ); + // Two levels at or above `start`, `count1 = 1`: the elision probe fails, so the second frame + // prints in full and no marker appears. + let two: String = s + .eval( + "function BenillaProbeDepth2() local r = debugstack(1, 1, 0) return r end \ + local r = BenillaProbeDepth2() return r", + ) + .unwrap(); + let lines: Vec<&str> = two.trim_end().split('\n').collect(); + assert_eq!(lines.len(), 2, "two frames, no marker: {two:?}"); + assert!(!two.contains("..."), "no elision at depth 2: {two:?}"); + // Deeper: one frame, then the marker on its own line, and nothing after it (count2 = 0). + let deep: String = s + .eval( + "function BenillaProbeC() local r = debugstack(1, 1, 0) return r end \ + function BenillaProbeB() local r = BenillaProbeC() return r end \ + function BenillaProbeA() local r = BenillaProbeB() return r end \ + local r = BenillaProbeA() return r", + ) + .unwrap(); + let lines: Vec<&str> = deep.trim_end().split('\n').collect(); + assert_eq!(lines.len(), 2, "one frame then the marker: {deep:?}"); + assert_eq!(lines[1], "...", "the marker is its own complete line"); } /// **An addon's chunk is named the way the CLIENT names it**, because addons parse that name. diff --git a/crates/benilla-ui/src/toc.rs b/crates/benilla-ui/src/toc.rs index 301033c9..95c8adb2 100644 --- a/crates/benilla-ui/src/toc.rs +++ b/crates/benilla-ui/src/toc.rs @@ -23,6 +23,12 @@ pub struct Toc { pub files: Vec, } +/// Case-insensitive ASCII prefix test — the reference's `SStrCmpI(line, key, SStrLen(key))`. +fn starts_with_ci(s: &str, prefix: &str) -> bool { + let (s, p) = (s.as_bytes(), prefix.as_bytes()); + s.len() >= p.len() && s[..p.len()].eq_ignore_ascii_case(p) +} + impl Toc { /// Parse manifest text. Never fails: unrecognizable lines are file entries by definition of /// the grammar, and an empty input is an empty manifest. @@ -51,12 +57,21 @@ impl Toc { toc } - /// The first directive whose key matches `key` case-insensitively. Duplicate keys: we read the - /// first occurrence (client behavior on duplicates is unverified — flag for the loader work; - /// shipped manifests don't duplicate keys). + /// The directive whose key matches `key` case-insensitively — **the LAST one, when a manifest + /// writes the key twice**. + /// + /// This read the first for as long as the doc here said "shipped manifests don't duplicate + /// keys", which two of them do: `MoveAnything.toc` has `## Notes` on lines 3 and 5, and + /// `AtlasLoot.toc` has `## Author` on lines 6 and 9. The reference hashes each directive into + /// the metadata map at `[rec+0x98]` (`0x51d580`) and, on an existing entry, **frees the old + /// value and stores the new** (`0x51d77b`) — a hash insert, so the last line written wins. + /// + /// Note the asymmetry with [`Self::list`] one function down, and that both halves were wrong + /// here in the same direction: a **scalar** replaces, a **list** appends. pub fn directive(&self, key: &str) -> Option<&str> { self.directives .iter() + .rev() .find(|(k, _)| k.eq_ignore_ascii_case(key)) .map(|(_, v)| v.as_str()) } @@ -85,33 +100,56 @@ impl Toc { .unwrap_or(0) } - /// A comma-separated list directive (`SavedVariables`, `OptionalDeps`, `Dependencies`, …), - /// split and trimmed. Missing directive = empty list. + /// A list directive (`SavedVariables`, `OptionalDeps`, `Dependencies`, …), tokenized the way + /// the reference tokenizes it. Missing directive = empty list. + /// + /// **Two departures from the obvious read, both byte-derived, both of which were losing real + /// data** (decision 2126): + /// + /// * **A repeated directive APPENDS.** The reference `strdup`s each item into a `0x40`-granular + /// array at `[rec+0x80]` with a running count at `[rec+0x7c]` (wow-re + /// `savedvariables-protocol.md` §4), so a second `## SavedVariables:` line continues the + /// first rather than being ignored. Reading only the first line cost `CT_RaidAssist` 18 of + /// its 25 saved globals — it declares them across four lines and got seven, so raid + /// positions, menu state, boss timers, debuff templates, the loot method and the squelch + /// list were never written and never restored. + /// * **A space separates as surely as a comma.** The tokenizer is `SStrTokenize 0x64ae50` over + /// the delimiter SET `" ,"` at `0x8537c4` — readable in the image's own `.rdata`, sitting + /// between the `RequiredDep` and `OptionalDep` literals. `SpecialTalentUI` writes + /// `## SavedVariables: SpecialTalentPlannedSaved SpecialTalentFrameSaved` and we registered + /// one "global" whose name contained a space. pub fn list(&self, key: &str) -> Vec<&str> { - self.directive(key) - .map(|v| { - v.split(',') - .map(str::trim) - .filter(|s| !s.is_empty()) - .collect() - }) - .unwrap_or_default() + self.list_where(|k| k.eq_ignore_ascii_case(key)) } - /// Hard dependencies: the client accepts both `## Dependencies:` and `## RequiredDeps:` - /// (and `## RequiredDep:`) as synonyms. + /// [`Self::list`] over every directive whose key satisfies `pick`, in file order. + fn list_where(&self, mut pick: impl FnMut(&str) -> bool) -> Vec<&str> { + self.directives + .iter() + .filter(|(k, _)| pick(k)) + .flat_map(|(_, v)| v.split([',', ' ']).map(str::trim)) + .filter(|s| !s.is_empty()) + .collect() + } + + /// Hard dependencies — **every directive whose key STARTS WITH `Dep` or `RequiredDep`**. + /// + /// The three dependency keys are the only ones in the reference's directive table that carry + /// **no colon**: the `.rdata` cluster reads + /// `SavedVariablesPerCharacter:\0 SavedVariables:\0 LoadWith:\0 \0\0 Dep\0 RequiredDep\0 " ,"\0\0 OptionalDep\0 Revision:\0`, + /// and a whole-key compare against `"Dep"` could never match a line reading `Dependencies:`. + /// So the compare is a PREFIX compare — `SStrCmpI(line, key, SStrLen(key))` at + /// `0x51cd4e`-`0x51cd5f` — and `## Dependencies`, `## RequiredDependencies` and + /// `## Dependency` all land here (decision 2126). + /// + /// Both keys feed the SAME array in the reference (`[rec+0x50]`), so a manifest writing both + /// gets their union rather than whichever this function happened to test first. pub fn dependencies(&self) -> Vec<&str> { - for key in ["Dependencies", "RequiredDeps", "RequiredDep"] { - let deps = self.list(key); - if !deps.is_empty() { - return deps; - } - } - Vec::new() + self.list_where(|k| starts_with_ci(k, "RequiredDep") || starts_with_ci(k, "Dep")) } - /// Soft dependencies: `## OptionalDeps:` (and `## OptionalDep:` by symmetry with the required - /// pair above). + /// Soft dependencies — every directive whose key starts with `OptionalDep`, on the same + /// prefix rule as [`Self::dependencies`]. /// /// **They load FIRST and their failures are ignored** — `AddOn_Load 0x51f240`'s own order, /// quoted in 1191 §2 — which is what lets `## OptionalDeps: FuBar, Ace2` mean "if Ace2 is @@ -120,18 +158,13 @@ impl Toc { /// lists `FuBarPlugin-2.0.lua` BEFORE `AceLibrary.lua`, which only works because the `Ace2` /// addon went first. /// - /// **`## OptionalDependencies:` is NOT accepted here**, though 4 corpus addons write it. The - /// long form is verified for the required half (`Dependencies`) and merely plausible for this - /// one, and 1204's rule is to be lenient only where the grammar admits no ambiguity. Queued as - /// an RE question with its cost attached: four addons. + /// The long form `## OptionalDependencies:` — which 4 corpus addons write — used to be + /// refused here, on the reasoning that it was "verified for the required half and merely + /// plausible for this one, queued as an RE question". The premise was wrong: the reference + /// matches all three dependency keys by the same colon-less prefix, so the long form was never + /// a separate question. pub fn optional_dependencies(&self) -> Vec<&str> { - for key in ["OptionalDeps", "OptionalDep"] { - let deps = self.list(key); - if !deps.is_empty() { - return deps; - } - } - Vec::new() + self.list_where(|k| starts_with_ci(k, "OptionalDep")) } /// `## LoadOnDemand: 1` (Era loader; absent in 1.12 manifests). @@ -187,8 +220,12 @@ mod tests { assert_eq!(toc.files.len(), 2); } + /// **A scalar directive REPLACES on a repeat; a list directive APPENDS** — the reference's two + /// stores, and this file had both backwards (decision 2126). `MoveAnything.toc` writes + /// `## Notes` twice and `AtlasLoot.toc` writes `## Author` twice, so the scalar half is not + /// hypothetical either. #[test] - fn dependency_synonyms_and_first_occurrence_wins() { + fn a_scalar_directive_replaces_and_a_list_directive_appends() { let toc = Toc::parse( "## RequiredDeps: LibA, LibB\n\ ## Title: first\n\ @@ -196,10 +233,56 @@ mod tests { ## LoadOnDemand: 1\n", ); assert_eq!(toc.dependencies(), vec!["LibA", "LibB"]); - assert_eq!(toc.directive("Title"), Some("first")); + assert_eq!(toc.directive("Title"), Some("second")); assert!(toc.load_on_demand()); } + /// **`CT_RaidAssist`'s own manifest, in shape** — 25 saved globals across four lines, of which + /// we registered the first seven. The rest (raid positions, menu state, boss timers, debuff + /// templates, the loot method, the squelch list) were never written and never restored. + /// + /// Plus `SpecialTalentUI`'s: two names separated by a SPACE, which we read as one global whose + /// name contained a space. The tokenizer is `SStrTokenize 0x64ae50` over the delimiter set + /// `" ,"` at `0x8537c4`. + #[test] + fn a_repeated_list_directive_accumulates_across_lines_and_splits_on_space() { + let toc = Toc::parse( + "## SavedVariables: A, B\n\ + ## SavedVariables: C, D\n\ + ## SavedVariablesPerCharacter: E F\n", + ); + assert_eq!(toc.list("SavedVariables"), vec!["A", "B", "C", "D"]); + assert_eq!(toc.list("SavedVariablesPerCharacter"), vec!["E", "F"]); + } + + /// **The three dependency keys are matched by PREFIX**, because they are the only directives in + /// the reference's table with no colon: the `.rdata` cluster is + /// `… LoadWith:\0 \0\0 Dep\0 RequiredDep\0 " ,"\0\0 OptionalDep\0 Revision:\0 …`, and a + /// whole-key compare against `"Dep"` could never match a line reading `Dependencies:`. + /// + /// Four corpus addons write the long optional form (`FuBar_CRDelayFu`, `FuBar_PetInFu`, + /// `FuBar_ToFu`, `FuBar_WeaponRebuffFu`), which this file used to refuse on the reasoning that + /// the long form was "merely plausible" for the optional half. It was never a separate + /// question. + #[test] + fn the_dependency_keys_match_by_prefix_and_both_required_spellings_union() { + let toc = Toc::parse( + "## Dependencies: LibA\n\ + ## RequiredDependencies: LibB\n\ + ## OptionalDependencies: LibC, LibD\n\ + ## OptionalDeps: LibE\n", + ); + assert_eq!(toc.dependencies(), vec!["LibA", "LibB"]); + assert_eq!( + toc.optional_dependencies(), + vec!["LibC", "LibD", "LibE"], + "the long and short optional spellings are the same key" + ); + // And the prefixes do not bleed into each other: `OptionalDependencies` does not start + // with `Dep`, so it never lands in the required list. + assert!(!toc.dependencies().contains(&"LibC")); + } + #[test] fn empty_and_degenerate_inputs() { assert_eq!(Toc::parse(""), Toc::default()); diff --git a/crates/benilla-ui/src/widget/kinds/messageframe.rs b/crates/benilla-ui/src/widget/kinds/messageframe.rs index 7333891d..cddaccf7 100644 --- a/crates/benilla-ui/src/widget/kinds/messageframe.rs +++ b/crates/benilla-ui/src/widget/kinds/messageframe.rs @@ -124,11 +124,27 @@ impl ScrollingMessageState { /// `timeVisible`/`fadeDuration` onto the new line, and push it at the ring's newest slot, /// dropping the oldest when over `max_lines`. A view scrolled up stays anchored on the same /// content (the ring cursor is a slot, not an offset — msgframe-runtime.md). - /// `ScrollingMessageFrame:UpdateColorByID(id, r, g, b)` (`0x7932b0`) — every line tagged - /// `id` takes the new colour, quantised the way `AddMessage` quantised the old one. - /// Returns how many lines moved; a recolour bumps the generation so a settled frame - /// redraws. + /// `ScrollingMessageFrame:UpdateColorByID(id, r, g, b)` (`0x7932b0` → `0x788250`) — every line + /// tagged `id` takes the new colour, quantised the way `AddMessage` quantised the old one. + /// Returns how many lines moved; a recolour bumps the generation so a settled frame redraws. + /// + /// **`id == 0` matches nothing, and that is a guard the reference opens with, not a + /// consequence** (decision 2125; wow-re `system/ui/scratch/login-chat-colour-pipeline.md`). + /// `0x788250` is `mov edi,[ebp+8]; test edi,edi; je 0x7882a4` → `ret 8`: zero never reaches a + /// comparison and the record walk is not entered at all. + /// + /// Without it this was a live repaint of every line ever printed with no explicit colour, + /// because `AddMessage`'s absent-id case stores an explicit literal `0` (`0x7929b7 xor edi,edi` + /// → `0x78821d mov [edi+0x14],ecx`) and `ChatTypeInfo["REPLY"].id` is 0 too — FrameXML declares + /// REPLY and the engine's 94-row registry does not carry it. So `ChatFrame_OnEvent`'s + /// `UPDATE_CHAT_COLOR` arm mirroring WHISPER into REPLY (`ChatFrame.lua` l.1357-1365) called + /// `UpdateColorByID(0, 1.0, 0.5, 1.0)` and turned every `AceConsole:Print` in the window + /// whisper-pink. Measured `(255,128,255)` on Bartender2's login line where the reference reads + /// `(255,255,255)`. pub fn update_color_by_id(&mut self, id: u32, r: f32, g: f32, b: f32) -> usize { + if id == 0 { + return 0; + } let rgb = [quantize_u8(r), quantize_u8(g), quantize_u8(b)]; let mut moved = 0; for line in self.lines.iter_mut().filter(|l| l.id == id) {