ui+app: the bag walk, the modifier fold, and two shapes the DLL's pool corrected

Built: `GetBagItems`, `GetSpellModifiers`, `GetSpellIdCooldown` — plus
fixes to two verbs that already existed and had the wrong *shape*,
found the same way `GetItemIconTexture`'s wrong argument was: by
comparing what we answer against nampower's own string pool.

`GetBagItems()` hands back every occupied bag slot as
`{ [bagIndex] = { [slot] = row } }`. DoiteAuras asks seven times over
and walks both levels with `pairs`, reading `itemId` and `stackCount` —
it is how that addon answers "do I have this item" without 100
`GetContainerItemInfo` calls per condition evaluation. Empty slots and
empty bags are absent, because a nil value in a Lua table *is* absence
and `pairs` is how both levels are read.

`GetSpellModifiers(spellId, modifierType)` answers
`flat, percent, hasModifier`, where percent is the final multiplier (90
means 90% of the original) — the consumer says so in its own words and
multiplies by `percent / 100`. The 26 modifier types come from the
DLL's usage string. The fold is `spell/mods.rs`' existing reader,
reached through a new `spell_modifier_probe` the lifecycle reinstalls
whenever the wire moves the tables: a VM-side reimplementation of a
four-gate 64-bit fold would be free to drift from the mana cost the
action bar greys a button on. The third return is the point — the
consumer tests it before touching either number, and the duration path
multiplies, so `0, 0` for an unmodified spell would zero every debuff
timer that asks.

`GetSpellIdCooldown(spellId)` answers the twenty-key table, from a new
`Cooldowns::legs`: `info`'s three legs kept apart, same walk and same
arms, so the folded number a swirl draws and the legs an addon reads
cannot describe different timers.

**`GetBagItem`'s row had the wrong key names** — `bag`, `slot`,
`count`, `quality`, `link`, `locked`, this client's own
`GetContainerItemInfo` spellings. nampower's row is contiguous at
`0xa8a68` and is the block `GetEquippedItem` already used, so a caller
reading `.stackCount` got nil and no error. One `bag_slot_table` now
serves both, with three columns absent rather than zeroed: this client
resolves a bag slot's enchants to names at the app boundary and keeps
no random-properties id for a container slot, and a zeroed enchant id
would be a claim.

**`GetItemIdCooldown` returned the wrong kind of value** — the stock
`start, duration, enable` triple. At the version we report it is a
table, and the addon layer settles it: `API.GetItemCooldownInfo` and
`API.GetSpellCooldownInfo` return these calls' results *directly* and
their callers index them. `cd.gcdCategoryRemainingMs` is the primary
path of `API.GetGCDRemainingMs`, which the `[gcd]`/`[nogcd]` macro
conditionals divide by 1000 — against a triple that is
`(0.0).gcdCategoryRemainingMs`, which raises. Latent rather than live
(nothing calls those wrappers today), still wrong.

Two things the schedule gates caught and improved:

- The probe refresher started as an exclusive system, which is
  ambiguous against everything in Update it is not ordered against:
  +274 pairs over the ceiling. `NonSendMut` is all it needs.
- Its memo started as a bare `Local`, which `session.rs`' own gate
  refuses — a memo about what this VM has been told must die with the
  VM. As a `VmMemo` it now reinstalls on the first frame after a
  `/reload`, which is better than the reasoning I had written for the
  `Local`: a fresh VM has no closure at all until something installs
  one.

One count the earlier sweep got wrong, corrected in the doc:
`GetSpellPower`'s "7 call sites" are BetterCharacterStats' own method
`BCS:GetSpellPower(school)`, not nampower's global. Nothing in the
corpus calls the global at all. A bare name match is not a call site.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn

crates/benilla-app/src/ui_script/action_bar_tests.rs

@@ -1056,6 +1056,8 @@ fn shipped_bag_frame_drives_end_to_end() {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,

crates/benilla-app/src/ui_script/bag_tests.rs

@@ -369,6 +369,8 @@ fn bag_tooltip_hangs_left_when_the_slot_sits_in_the_right_half() {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,
@@ -443,6 +445,8 @@ fn hovered_bag_tooltip_fills_itself_when_the_stats_land() {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,
@@ -547,6 +551,8 @@ fn vendor_bag_hover_shows_sell_price_and_arms_the_pouch_cursor() {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,
@@ -570,6 +576,8 @@ fn vendor_bag_hover_shows_sell_price_and_arms_the_pouch_cursor() {
     slots.insert(
         2,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,
@@ -750,6 +758,8 @@ fn drag_across_two_slots_queues_the_same_move_a_click_pickup_would() {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,
@@ -833,6 +843,8 @@ fn a_second_bag_window_feeds_and_paints_via_the_bag_bar() {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,
@@ -962,6 +974,8 @@ fn open_backpack_with_a_five_stack(s: &mut UiScript) -> (f32, f32) {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,

crates/benilla-app/src/ui_script/character_tests.rs

@@ -80,6 +80,8 @@ fn backpack_with_fitting_helm() -> benilla_ui::script::ContainerState {
     slots.insert(
         1,
         benilla_ui::script::ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,

crates/benilla-app/src/ui_script/chat_tests.rs

@@ -312,6 +312,8 @@ fn chat_click_dismisses_a_stuck_spell_but_not_an_item() {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,

crates/benilla-app/src/ui_script/delete_item_tests.rs

@@ -14,6 +14,8 @@ fn one_item_backpack(item_id: u32, name: &str, quality: u32) -> ContainerState {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,

crates/benilla-app/src/ui_script/escape_tests.rs

@@ -12,6 +12,8 @@ fn one_item_backpack() -> ContainerState {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,
@@ -244,6 +246,8 @@ fn escape_closes_an_open_stack_split_frame() {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,

crates/benilla-app/src/ui_script/game_menu_tests.rs

@@ -756,6 +756,8 @@ fn backpack() -> ContainerState {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,

crates/benilla-app/src/ui_script/lifecycle.rs

@@ -38,6 +38,7 @@ fn install_boot_vm(world: &mut World) {
     install_addon_asset_resolvers(world, &mut script);
     install_spell_catalog_probe(world, &mut script);
     install_item_icon_probe(world, &mut script);
+    install_spell_modifier_probe(world, &mut script);
     seed_creature_info(world, &mut script);
     load_global_strings(world, &script);
     load_emote_tokens(world, &script);
@@ -203,6 +204,75 @@ fn install_item_icon_probe(world: &World, script: &mut UiScript) {
     script.set_item_icon_probe(Box::new(move |display_id| icons.get(&display_id).cloned()));
 }
 
+/// nampower's `GetSpellModifiers(spellId, modifierType)`: the `(flat, percent)` pair
+/// `spell/mods.rs`'s own reader folds out of the two `i32[64][29]` talent-modifier tables the
+/// wire fills.
+///
+/// The closure captures a **copy** of those tables and calls the real reader on it, rather than
+/// pushing the cells to Lua for a second fold. The arithmetic has four gates and a 64-bit sum,
+/// and `SpellModifiers::modifiers` is already its one implementation — the one the power cost is
+/// computed with — so a VM-side reimplementation would be free to drift from the number a player
+/// sees on their own action bar.
+///
+/// A copy means it goes stale, which is what [`refresh_spell_modifier_probe`] is for: the tables
+/// are live state (every `SMSG_SET_*_SPELL_MODIFIER`, and the class family as the avatar
+/// resolves), and they arrive **after** world entry has built this VM. Without the refresh this
+/// probe would answer `None` for the whole session, which is exactly the answer a client with no
+/// talents gives — a silent wrong one.
+fn install_spell_modifier_probe(world: &World, script: &mut UiScript) {
+    let Some(mods) = world.get_resource::<crate::spell::SpellModifiers>() else {
+        return;
+    };
+    let Some(spells) = world.get_resource::<crate::ui_action::Spells>() else {
+        return;
+    };
+    let displays = spells.catalog.displays_shared();
+    let snapshot = mods.clone();
+    script.set_spell_modifier_probe(Box::new(move |spell_id, op| {
+        snapshot.modifier_pair(displays.get(&spell_id)?, op)
+    }));
+}
+
+/// Reinstall [`install_spell_modifier_probe`]'s closure when the tables behind it move.
+///
+/// Gated on the resource's own generation rather than Bevy change detection, so a write that
+/// stores the value a cell already held does not rebuild the closure — and so the gate reads the
+/// same way as `Cooldowns::feed_epoch`, which is the precedent here. The rebuild copies ~15 KB
+/// and happens at login and on a respec, not per frame.
+///
+/// **Not an exclusive system**, although swapping a closure on the VM would be the obvious reason
+/// to make it one: an exclusive system in `Update` is ambiguous against every system it is not
+/// explicitly ordered against, and this one cost 274 pairs against
+/// `the_schedules_have_no_more_undeclared_orders_than_the_ceilings_say`'s ceiling. `NonSendMut`
+/// is all it needs.
+///
+/// The memo is a [`VmMemo`], not a bare `Local`, which
+/// `session.rs`'s own gate insists on and is right to: a memo about what this VM has been told
+/// must die with the VM. Here it means a fresh VM reinstalls the probe on its first frame even at
+/// an unchanged generation, which is exactly what a `/reload` needs — the new VM has no closure at
+/// all until something installs one.
+pub(crate) fn refresh_spell_modifier_probe(
+    script: Option<NonSendMut<UiScript>>,
+    mods: Option<Res<crate::spell::SpellModifiers>>,
+    spells: Option<Res<crate::ui_action::Spells>>,
+    mut seen: Local<crate::ui_script::VmMemo<Option<u64>>>,
+) {
+    let (Some(mut script), Some(mods), Some(spells)) = (script, mods, spells) else {
+        return;
+    };
+    let generation = mods.generation();
+    let seen = seen.get(&script);
+    if *seen == Some(generation) {
+        return;
+    }
+    *seen = Some(generation);
+    let displays = spells.catalog.displays_shared();
+    let snapshot = mods.clone();
+    script.set_spell_modifier_probe(Box::new(move |spell_id, op| {
+        snapshot.modifier_pair(displays.get(&spell_id)?, op)
+    }));
+}
+
 /// Install [`install_spell_probe`] from the world's catalog, if it has one. The boot VM built at
 /// `Startup` runs before `ui_action`'s loader has inserted [`crate::ui_action::Spells`] (its
 /// `insert_resource` is a command, applied at the end of that schedule), so the character-screen
@@ -1223,3 +1293,97 @@ mod tests {
         );
     }
 }
+
+#[cfg(test)]
+mod spell_modifier_probe_tests {
+    use super::*;
+    use benilla_formats::{SpellCatalog, SpellDisplay};
+    use bevy::ecs::system::RunSystemOnce;
+    use std::collections::HashMap;
+
+    /// Improved Fireball-shaped: a mage spell (family 3) with family bit 0 set.
+    const FIREBALL: u32 = 133;
+
+    fn app() -> App {
+        let mut app = App::new();
+        let mut spells = crate::ui_action::Spells::empty_for_tests();
+        spells.catalog = SpellCatalog::from_displays(HashMap::from([(
+            FIREBALL,
+            SpellDisplay {
+                spell_family: 3,
+                spell_family_flags: 1,
+                ..Default::default()
+            },
+        )]));
+        app.insert_resource(spells);
+        let mut mods = crate::spell::SpellModifiers::default();
+        mods.set_class_family(3);
+        app.insert_resource(mods);
+        app.insert_non_send_resource(UiScript::new().unwrap());
+        app
+    }
+
+    fn ask(app: &App) -> (i64, i64, i64) {
+        app.world()
+            .non_send_resource::<UiScript>()
+            .eval("return GetSpellModifiers(133, 14)")
+            .unwrap()
+    }
+
+    /// The whole chain, which no unit test on either side can see: a wire write moves the tables,
+    /// the generation moves with it, the refresher reinstalls the closure, and Lua's
+    /// `GetSpellModifiers` answers the pair `spell/mods.rs`' own reader folded.
+    ///
+    /// Worth a test of its own because every link fails quietly. A probe never installed, a
+    /// generation never bumped, or a closure holding a copy from before the talents arrived all
+    /// read as "this spell has no modifier" — which is also the correct answer for most spells,
+    /// so nothing would look wrong.
+    #[test]
+    fn a_wire_write_reaches_lua_through_a_reinstalled_probe() {
+        let mut app = app();
+        // No probe yet: the honest nothing.
+        assert_eq!(ask(&app), (0, 0, 0), "no probe installed yet");
+
+        app.world_mut()
+            .run_system_once(refresh_spell_modifier_probe)
+            .unwrap();
+        assert_eq!(
+            ask(&app),
+            (0, 0, 0),
+            "installed, and the tables really are empty"
+        );
+
+        // The wire writes a cell. Until the refresher runs again the closure holds the old copy —
+        // which is exactly the staleness the generation exists to catch, so it is asserted.
+        app.world_mut()
+            .resource_mut::<crate::spell::SpellModifiers>()
+            .set(true, 0, crate::spell::OP_COST, -30);
+        assert_eq!(
+            ask(&app),
+            (0, 0, 0),
+            "the installed copy predates the write"
+        );
+
+        app.world_mut()
+            .run_system_once(refresh_spell_modifier_probe)
+            .unwrap();
+        assert_eq!(
+            ask(&app),
+            (-30, 100, 1),
+            "the reinstalled closure folds the new cell, and 100 is the identity multiplier"
+        );
+
+        // A fresh VM gets its own closure, although the generation has not moved: the memo is a
+        // `VmMemo`, so it died with the old VM.
+        app.insert_non_send_resource(UiScript::new().unwrap());
+        assert_eq!(ask(&app), (0, 0, 0), "a new VM has no closure at all");
+        app.world_mut()
+            .run_system_once(refresh_spell_modifier_probe)
+            .unwrap();
+        assert_eq!(
+            ask(&app),
+            (-30, 100, 1),
+            "and the first frame after a /reload installs one"
+        );
+    }
+}

crates/benilla-app/src/ui_script/mod.rs

@@ -195,6 +195,29 @@ pub(crate) struct UiClock {
     pub(crate) ui_now: f64,
 }
 
+/// [`crate::spell::CooldownLegs`] on the VM's `GetTime()` clock, the one conversion both feeds
+/// that carry it use — the spellbook's and the bags'.
+///
+/// Here rather than in either feed because both must spell it the same way: a leg's remainder is
+/// derived in the Lua binding from `GetTime()`, so if the two feeds disagreed about the start, the
+/// same item's cooldown would read differently through `GetSpellIdCooldown` and
+/// `GetItemIdCooldown`.
+pub(crate) fn cooldown_legs_view(
+    legs: &crate::spell::CooldownLegs,
+    anchor: std::time::Instant,
+    ui_now: f64,
+) -> benilla_ui::script::CooldownLegsView {
+    let leg =
+        |info: Option<crate::spell::CooldownInfo>| info.and_then(|i| i.ui_triple(anchor, ui_now));
+    benilla_ui::script::CooldownLegsView {
+        individual: leg(legs.individual),
+        category_id: legs.category_id,
+        category: leg(legs.category),
+        gcd_category_id: legs.gcd_category_id,
+        gcd: leg(legs.gcd),
+    }
+}
+
 /// The pre-boot pair, replaced by [`lifecycle::seed_vm_clock`] at the first VM.
 impl Default for UiClock {
     fn default() -> Self {
@@ -337,6 +360,14 @@ impl Plugin for UiScriptPlugin {
             )
             // In the feed phase, so an `OnUpdate` reads this frame's `GetFramerate()`.
             .add_systems(Update, feed_framerate.in_set(UiFeed))
+            // nampower's `GetSpellModifiers` probe, reinstalled when the wire moves the talent
+            // tables behind it (login, a respec). In the feed phase for the ordinary reason —
+            // the VM must answer out of this frame's state — and exclusive because it swaps a
+            // closure on the VM.
+            .add_systems(
+                Update,
+                lifecycle::refresh_spell_modifier_probe.in_set(UiFeed),
+            )
             // `WOW_FLASH_CHURN=1` only; a no-op otherwise. After the tick, because what it
             // watches is the visibility the frame's handlers just produced — a reading taken
             // before them would miss the very edges it exists to count.

crates/benilla-app/src/ui_script/tooltip_compare_tests.rs

@@ -122,6 +122,8 @@ fn seed_items(s: &mut UiScript) {
     slots.insert(
         1,
         ContainerSlot {
+            cooldown_legs: Default::default(),
+            use_spell_id: 0,
             charges: 0,
             duration_ms: None,
             petition: None,