From cf0e157f34d314f4620d8c4ca46571f337ad7bce Mon Sep 17 00:00:00 2001 From: grom Date: Sat, 24 Oct 2026 09:00:17 -0600 Subject: [PATCH] ui+app: the bag walk, the modifier fold, and two shapes the DLL's pool corrected MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- crates/benilla-app/src/capture/fixtures.rs | 6 + crates/benilla-app/src/spell/cooldowns.rs | 94 +++ crates/benilla-app/src/spell/mod.rs | 2 +- crates/benilla-app/src/spell/mods.rs | 106 ++- crates/benilla-app/src/ui_items/drain.rs | 2 + crates/benilla-app/src/ui_items/feed.rs | 10 + crates/benilla-app/src/ui_pet_book.rs | 7 + .../src/ui_script/action_bar_tests.rs | 2 + crates/benilla-app/src/ui_script/bag_tests.rs | 14 + .../src/ui_script/character_tests.rs | 2 + .../benilla-app/src/ui_script/chat_tests.rs | 2 + .../src/ui_script/delete_item_tests.rs | 2 + .../benilla-app/src/ui_script/escape_tests.rs | 4 + .../src/ui_script/game_menu_tests.rs | 2 + crates/benilla-app/src/ui_script/lifecycle.rs | 164 +++++ crates/benilla-app/src/ui_script/mod.rs | 31 + .../src/ui_script/tooltip_compare_tests.rs | 2 + crates/benilla-app/src/ui_spellbook.rs | 13 +- .../benilla-ui/src/script/compat/nampower.rs | 634 ++++++++++++++++-- crates/benilla-ui/src/script/container.rs | 12 + crates/benilla-ui/src/script/cursor.rs | 2 + crates/benilla-ui/src/script/cursor/doll.rs | 4 + crates/benilla-ui/src/script/mod.rs | 5 +- crates/benilla-ui/src/script/model.rs | 15 + crates/benilla-ui/src/script/spellbook.rs | 32 + .../src/script/tests/reference_surface.rs | 3 + .../src/script/tests/tooltip_item/bindings.rs | 2 + docs/ADDON_COMPAT.md | 54 +- 28 files changed, 1181 insertions(+), 47 deletions(-) diff --git a/crates/benilla-app/src/capture/fixtures.rs b/crates/benilla-app/src/capture/fixtures.rs index c71162c1..6fae9c0d 100644 --- a/crates/benilla-app/src/capture/fixtures.rs +++ b/crates/benilla-app/src/capture/fixtures.rs @@ -1447,6 +1447,8 @@ fn seed_cooldown_filmstrip( slots.insert( slot, benilla_ui::script::ContainerSlot { + cooldown_legs: Default::default(), + use_spell_id: 0, petition: None, durability: None, duration_ms: None, @@ -1500,6 +1502,8 @@ fn seed_bag_window( }; let slot = |disp: u32, count: u32, id: u32, name: &str, quality: u32| { benilla_ui::script::ContainerSlot { + cooldown_legs: Default::default(), + use_spell_id: 0, petition: None, durability: None, duration_ms: None, @@ -1587,6 +1591,8 @@ fn seed_equipped_bags( }; let slot = |disp: u32, count: u32, name: &str, quality: u32| benilla_ui::script::ContainerSlot { + cooldown_legs: Default::default(), + use_spell_id: 0, petition: None, durability: None, duration_ms: None, diff --git a/crates/benilla-app/src/spell/cooldowns.rs b/crates/benilla-app/src/spell/cooldowns.rs index b75847b7..ebcb5ebf 100644 --- a/crates/benilla-app/src/spell/cooldowns.rs +++ b/crates/benilla-app/src/spell/cooldowns.rs @@ -101,6 +101,22 @@ pub(crate) fn ui_stamp(at: Instant, anchor: Instant, ui_now: f64) -> f64 { } } +/// [`Cooldowns::legs`]' answer: the three legs of one cooldown query, each with the category id +/// it was keyed on. A `None` leg is cold — never a zeroed [`CooldownInfo`], because nampower's +/// table reports an `isOn…` flag per leg and "no record" and "a record with nothing left" are +/// different answers to it. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub(crate) struct CooldownLegs { + /// The spell's own recovery, keyed on the (spell, item) pair. + pub(crate) individual: Option, + /// `Spell.dbc` `Category`, the id the category leg was keyed on; 0 for a spell with none. + pub(crate) category_id: u32, + pub(crate) category: Option, + /// `StartRecoveryCategory`, the id the GCD leg was keyed on. + pub(crate) gcd_category_id: u32, + pub(crate) gcd: Option, +} + /// The player's cooldown list (the client's `SpellHistory` at `0xcecaec`); the pet list has no /// consumer. #[derive(Resource, Default)] @@ -513,6 +529,84 @@ impl Cooldowns { self.info(spell_id, item_entry, spell, now).remaining_ms > 0 } + /// [`Self::info`]'s three legs kept apart, for nampower's `GetSpellIdCooldown` / + /// `GetItemIdCooldown`, which report each separately with its own start, duration, remainder + /// and `isOn…` flag (the twenty key names are in the DLL's string pool at `0xa88d4`). + /// + /// Exactly the same walk and exactly the same arms as [`Self::info`] — including its + /// `cooldown_query_excluded` early-out, the parked record's "full duration, disabled" arm and + /// the GCD leg's indifference to `on_hold` — so the folded answer a cooldown swirl draws and + /// the three legs an addon reads can never describe different timers. A second walk with its + /// own copy of those arms is how they would come to. + pub(crate) fn legs( + &self, + spell_id: u32, + item_entry: u32, + spell: Option<&SpellDisplay>, + now: Instant, + ) -> CooldownLegs { + let category = spell.map_or(0, |s| s.category); + let gcd_category = spell.map_or(0, |s| s.start_recovery_category); + let mut out = CooldownLegs { + individual: None, + category_id: category, + category: None, + gcd_category_id: gcd_category, + gcd: None, + }; + if spell.is_some_and(|s| s.cooldown_query_excluded()) { + return out; + } + // Longest-wins per leg, as `info` picks longest-wins across all three. + let consider = + |best: &mut Option, timer: &Timer, remaining: Duration, enabled: bool| { + let remaining_ms = remaining.as_millis().min(u128::from(u32::MAX)) as u32; + if best.is_none_or(|b| remaining_ms > b.remaining_ms) { + *best = Some(CooldownInfo { + start: timer.start, + remaining_ms, + duration_ms: timer.duration.as_millis().min(u128::from(u32::MAX)) as u32, + enabled, + }); + } + }; + for r in &self.records { + if r.spell_id == spell_id && r.item_id == item_entry { + if r.on_hold { + consider(&mut out.individual, &r.recovery, r.recovery.duration, false); + } else { + consider( + &mut out.individual, + &r.recovery, + r.recovery.remaining(now), + true, + ); + } + } + if (r.category != 0 && r.category == category) || r.category_wildcard { + if r.on_hold { + consider( + &mut out.category, + &r.category_recovery, + r.category_recovery.duration, + false, + ); + } else { + consider( + &mut out.category, + &r.category_recovery, + r.category_recovery.remaining(now), + true, + ); + } + } + if r.gcd_category == gcd_category && !r.gcd.duration.is_zero() { + consider(&mut out.gcd, &r.gcd, r.gcd.remaining(now), true); + } + } + out + } + /// The running global cooldown as one timer — the record whose GCD leg has the most left, /// with its start and duration. `None` when nothing is on the GCD. /// diff --git a/crates/benilla-app/src/spell/mod.rs b/crates/benilla-app/src/spell/mod.rs index 67cd2a67..a8c67c4f 100644 --- a/crates/benilla-app/src/spell/mod.rs +++ b/crates/benilla-app/src/spell/mod.rs @@ -25,7 +25,7 @@ pub(crate) mod validator; // `send_spell_cast` is private to `cast_send`, so no second send path can exist. pub(crate) use cast_send::{CastCommit, CastLadder, TargetedBind}; pub(crate) use cast_target::AutoSelfCast; -pub(crate) use cooldowns::Cooldowns; +pub(crate) use cooldowns::{CooldownInfo, CooldownLegs, Cooldowns}; pub(crate) use inflight::{ inflight, ActiveChannel, AutoRepeatActive, LocalMoveStart, PendingCast, QueuedMeleeSpell, SPELL_INTERRUPT_MOVEMENT, diff --git a/crates/benilla-app/src/spell/mods.rs b/crates/benilla-app/src/spell/mods.rs index ab2f298a..69b8c1dc 100644 --- a/crates/benilla-app/src/spell/mods.rs +++ b/crates/benilla-app/src/spell/mods.rs @@ -73,10 +73,17 @@ impl SpellMod { /// The two tables and the class family the gate compares against (`0xcead60`, `0xcecb30`, /// `0xcecaac`). Written only by the wire, cleared only at world-enter, read live. -#[derive(Resource)] +#[derive(Resource, Clone)] pub(crate) struct SpellModifiers { flat: [i32; CELLS], pct: [i32; CELLS], + /// Bumped on every write and on the world-enter clear: what + /// `ui_script::lifecycle::refresh_spell_modifier_probe` watches, so nampower's + /// `GetSpellModifiers` probe is reinstalled exactly when the tables it folds have moved. + /// + /// The class family counts as a write: the gate compares against it, so a spell that read + /// `None` before the avatar resolved reads a real pair after. + generation: u64, /// The local player's class spell-family (`ChrClasses.dbc` field 15); 0 until the avatar /// resolves, which the reader's `SpellFamilyName != 0` gate covers. class_family: u32, @@ -88,6 +95,7 @@ impl Default for SpellModifiers { flat: [0; CELLS], pct: [0; CELLS], class_family: 0, + generation: 0, } } } @@ -111,10 +119,19 @@ impl SpellModifiers { } else { self.pct[cell] = value; } + self.generation = self.generation.wrapping_add(1); + } + + /// The counter a VM-side probe watches; see [`Self::generation`]. + pub(crate) fn generation(&self) -> u64 { + self.generation } /// Set the gate's class family (`0x6e6ca0`). pub(crate) fn set_class_family(&mut self, family: u32) { + if self.class_family != family { + self.generation = self.generation.wrapping_add(1); + } self.class_family = family; } @@ -123,6 +140,7 @@ impl SpellModifiers { self.flat = [0; CELLS]; self.pct = [0; CELLS]; self.class_family = 0; + self.generation = self.generation.wrapping_add(1); } /// `GetSpellModifiers 0x6e6b30` for one spell and op. `None` covers all four of its false @@ -161,6 +179,24 @@ impl SpellModifiers { pub(crate) fn apply(&self, d: &SpellDisplay, op: u8, value: i32) -> i32 { self.modifiers(d, op).map_or(value, |m| m.apply(value)) } + + /// [`Self::modifiers`] as the pair nampower's `GetSpellModifiers(spellId, modifierType)` + /// hands to Lua: `(flat, percent)`, where `percent` is the **final multiplier** the reader + /// already added 100 to — `SuperCleveRoidMacros` states that in its own words ("Nampower + /// returns percent as final multiplier (90 = 90% of original, not -10% change)", + /// `Utility.lua:6932`) and multiplies by `percent / 100`. + /// + /// `None` passes through unchanged, and it has to: the consumer's third return is a + /// has-modifier flag it tests before using either number + /// (`if not hasModifier or (flat == 0 and percent == 0) then return nil end`), and a `(0, 0)` + /// pair would make every unmodified spell look like one whose duration is multiplied by zero. + /// That is the same trap [`Self::modifiers`]' own doc names for the reference's callers. + /// + /// The one reader of the tables stays the one reader: the Lua probe calls this rather than + /// folding a pushed copy, so nampower's answer and the power cost cannot disagree. + pub(crate) fn modifier_pair(&self, d: &SpellDisplay, op: u8) -> Option<(i32, i32)> { + self.modifiers(d, op).map(|m| (m.flat, m.pct)) + } } /// Entering the world clears the tables and class family: `Spell_C::SystemInitialize 0x6e7150`, @@ -209,6 +245,74 @@ mod tests { mods } + /// [`SpellModifiers::modifier_pair`], nampower's `GetSpellModifiers` return, and the + /// generation that makes the Lua probe follow it. + /// + /// The `None` is the contract: the consumer's third return is a has-modifier flag it tests + /// before touching either number, and the duration path multiplies by `percent / 100`. A + /// `(0, 0)` pair for an unmodified spell would zero every debuff timer that asks. + #[test] + fn the_modifier_pair_is_none_until_a_cell_applies_and_the_generation_follows() { + let mut mods = mage_tables(); + let seen = mods.generation(); + + let fireball = mage(&[0]); + assert_eq!( + mods.modifier_pair(&fireball, OP_COST), + None, + "an empty table is no modifier, not a zeroing one" + ); + + // −30 mana flat on bit 0's spells, which is Improved Fireball-shaped. + mods.set(true, 0, OP_COST, -30); + assert_eq!( + mods.modifier_pair(&fireball, OP_COST), + Some((-30, 100)), + "100 is the identity multiplier the reader adds once, not a doubling" + ); + assert_ne!( + mods.generation(), + seen, + "the write moved the generation, which is what makes the Lua probe reinstall" + ); + + // A percentage on the same cell: the pair carries both, the percent already +100. + mods.set(false, 0, OP_COST, -40); + assert_eq!(mods.modifier_pair(&fireball, OP_COST), Some((-30, 60))); + // And the gates still shut it out: a different op, a spell with no family bits, and a + // spell of another class all read None. + assert_eq!(mods.modifier_pair(&fireball, 1), None); + assert_eq!(mods.modifier_pair(&mage(&[]), OP_COST), None); + assert_eq!( + mods.modifier_pair( + &SpellDisplay { + spell_family: 7, + spell_family_flags: 1, + ..Default::default() + }, + OP_COST + ), + None, + "a warlock spell against a mage's tables" + ); + + // The class family counts as a write, because the gate compares against it. + let before = mods.generation(); + mods.set_class_family(3); + assert_eq!( + mods.generation(), + before, + "an unchanged family is not a write" + ); + mods.set_class_family(7); + assert_ne!(mods.generation(), before); + assert_eq!( + mods.modifier_pair(&fireball, OP_COST), + None, + "and the gate really moved with it" + ); + } + /// Uses the asymmetric pair (5, 14): a transposed index would serve bit 14's spell instead. #[test] fn the_index_is_mask_bit_times_29_plus_op() { diff --git a/crates/benilla-app/src/ui_items/drain.rs b/crates/benilla-app/src/ui_items/drain.rs index 55fdffe6..8f50703f 100644 --- a/crates/benilla-app/src/ui_items/drain.rs +++ b/crates/benilla-app/src/ui_items/drain.rs @@ -1255,6 +1255,8 @@ mod bind_confirm_tests { slots.insert( 1, ContainerSlot { + cooldown_legs: Default::default(), + use_spell_id: 0, charges: 0, duration_ms: None, petition: None, diff --git a/crates/benilla-app/src/ui_items/feed.rs b/crates/benilla-app/src/ui_items/feed.rs index 89bddc32..5afc758b 100644 --- a/crates/benilla-app/src/ui_items/feed.rs +++ b/crates/benilla-app/src/ui_items/feed.rs @@ -662,6 +662,16 @@ fn resolve_slot( .info(u.spell_id, entry, sd, now) .ui_triple(now, ui_now) }), + use_spell_id: t.use_spell.map_or(0, |u| u.spell_id), + // The same query's three legs, for nampower's `GetItemIdCooldown`, off the same `now`. + cooldown_legs: t.use_spell.map_or_else(Default::default, |u| { + let sd = spells.and_then(|s| s.get(u.spell_id)); + crate::ui_script::cooldown_legs_view( + &cooldowns.legs(u.spell_id, entry, sd, now), + now, + ui_now, + ) + }), // Uses left, from the instance's own countdown of the pool block [`charge_pool`] names; // `ITEM_FIELD_SPELL_CHARGES` is signed the same way the template is (negative consumes // the item), so the magnitude is the count. Absent from the descriptor — which our own diff --git a/crates/benilla-app/src/ui_pet_book.rs b/crates/benilla-app/src/ui_pet_book.rs index d0bf3421..b7f2a195 100644 --- a/crates/benilla-app/src/ui_pet_book.rs +++ b/crates/benilla-app/src/ui_pet_book.rs @@ -161,6 +161,13 @@ fn slot_view( .cooldowns .info(spell_id, 0, Some(d), now) .ui_triple(anchor, ui_now), + // The same query's legs, so nampower's `GetSpellIdCooldown` answers for a pet spell as + // it does for a player one — the slot shape is shared and so is the arithmetic. + cooldown_legs: crate::ui_script::cooldown_legs_view( + &bar.cooldowns.legs(spell_id, 0, Some(d), now), + anchor, + ui_now, + ), // Off the raw word; `0x4bdd65` also requires a `Spell.dbc` record, which every slot has. autocast: Some((entry.autocast_allowed(), entry.autocast_on())), packed: entry.packed, diff --git a/crates/benilla-app/src/ui_script/action_bar_tests.rs b/crates/benilla-app/src/ui_script/action_bar_tests.rs index ec59a0cd..168a3201 100644 --- a/crates/benilla-app/src/ui_script/action_bar_tests.rs +++ b/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, diff --git a/crates/benilla-app/src/ui_script/bag_tests.rs b/crates/benilla-app/src/ui_script/bag_tests.rs index b3544de5..cea27928 100644 --- a/crates/benilla-app/src/ui_script/bag_tests.rs +++ b/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, diff --git a/crates/benilla-app/src/ui_script/character_tests.rs b/crates/benilla-app/src/ui_script/character_tests.rs index 7faabecf..94ad6fed 100644 --- a/crates/benilla-app/src/ui_script/character_tests.rs +++ b/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, diff --git a/crates/benilla-app/src/ui_script/chat_tests.rs b/crates/benilla-app/src/ui_script/chat_tests.rs index b16224f3..fefdabd7 100644 --- a/crates/benilla-app/src/ui_script/chat_tests.rs +++ b/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, diff --git a/crates/benilla-app/src/ui_script/delete_item_tests.rs b/crates/benilla-app/src/ui_script/delete_item_tests.rs index abb92336..084f2bfb 100644 --- a/crates/benilla-app/src/ui_script/delete_item_tests.rs +++ b/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, diff --git a/crates/benilla-app/src/ui_script/escape_tests.rs b/crates/benilla-app/src/ui_script/escape_tests.rs index 031ef666..f55c1a72 100644 --- a/crates/benilla-app/src/ui_script/escape_tests.rs +++ b/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, diff --git a/crates/benilla-app/src/ui_script/game_menu_tests.rs b/crates/benilla-app/src/ui_script/game_menu_tests.rs index eac6882f..00ab08dd 100644 --- a/crates/benilla-app/src/ui_script/game_menu_tests.rs +++ b/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, diff --git a/crates/benilla-app/src/ui_script/lifecycle.rs b/crates/benilla-app/src/ui_script/lifecycle.rs index c08db01a..faf81d02 100644 --- a/crates/benilla-app/src/ui_script/lifecycle.rs +++ b/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::() else { + return; + }; + let Some(spells) = world.get_resource::() 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>, + mods: Option>, + spells: Option>, + mut seen: Local>>, +) { + 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::() + .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::() + .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" + ); + } +} diff --git a/crates/benilla-app/src/ui_script/mod.rs b/crates/benilla-app/src/ui_script/mod.rs index 02aa0601..c465541e 100644 --- a/crates/benilla-app/src/ui_script/mod.rs +++ b/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| 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. diff --git a/crates/benilla-app/src/ui_script/tooltip_compare_tests.rs b/crates/benilla-app/src/ui_script/tooltip_compare_tests.rs index 8389eff2..39791546 100644 --- a/crates/benilla-app/src/ui_script/tooltip_compare_tests.rs +++ b/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, diff --git a/crates/benilla-app/src/ui_spellbook.rs b/crates/benilla-app/src/ui_spellbook.rs index 88e3c492..f19630d2 100644 --- a/crates/benilla-app/src/ui_spellbook.rs +++ b/crates/benilla-app/src/ui_spellbook.rs @@ -218,9 +218,17 @@ fn feed_spellbook( // `GetTime` clock, frame-stable per arm, so a running cooldown never churns the diff. let (anchor, ui_now) = (clock.anchor, clock.ui_now); for slot in &mut fresh.slots { + let d = spells.catalog.get(slot.spell_id); slot.cooldown = cooldowns - .info(slot.spell_id, 0, spells.catalog.get(slot.spell_id), anchor) + .info(slot.spell_id, 0, d, anchor) .ui_triple(anchor, ui_now); + // The same query's three legs, for nampower's `GetSpellIdCooldown`. Off the same `anchor` + // as the folded triple above, so the two cannot describe different timers. + slot.cooldown_legs = crate::ui_script::cooldown_legs_view( + &cooldowns.legs(slot.spell_id, 0, d, anchor), + anchor, + ui_now, + ); } if fresh != memory.pushed { gate.audit("feed_spellbook", "the spellbook snapshot"); @@ -374,6 +382,9 @@ fn build_book( }; slots.push(SpellSlotView { spell_id, + // Both cooldown fields are filled by the one walk below, which is where the + // `anchor` lives; this is the slot's identity only. + cooldown_legs: benilla_ui::script::CooldownLegsView::default(), name: d.map(|d| d.name.clone()).unwrap_or_default(), rank: d.and_then(|d| d.rank.clone()), texture, diff --git a/crates/benilla-ui/src/script/compat/nampower.rs b/crates/benilla-ui/src/script/compat/nampower.rs index d942d151..01fe3662 100644 --- a/crates/benilla-ui/src/script/compat/nampower.rs +++ b/crates/benilla-ui/src/script/compat/nampower.rs @@ -169,6 +169,122 @@ fn equipped_table(lua: &Lua, view: &crate::script::InvSlotView) -> mlua::Result< Ok(t) } +/// nampower's cooldown table: twenty keys, read off the DLL's own string pool in table order +/// (`0xa88d4`), shared by `GetSpellIdCooldown` and `GetItemIdCooldown` because upstream's addon +/// layer says so in as many words — `API.GetItemCooldownInfo` "returns same table structure as +/// `GetSpellCooldownInfo`" (`NampowerAPI.lua:2029`). +/// +/// Every `*RemainingMs` is derived **here**, from `GetTime()` and the leg's own absolute start, +/// rather than pushed: a remainder is the one column that changes every frame, and pushing it +/// would churn the spellbook diff sixty times a second for no gain (the feed's own note says the +/// absolute start is what keeps a running cooldown out of that diff). +/// +/// `isOnCooldown`/`cooldownRemainingMs` are the **folded** answer — the longest of the three, +/// which is the number the reference's `GetCooldownInfo 0x6e13e0` returns and the number a +/// cooldown swirl draws. A caller that wants to know *why* a spell is cold reads the legs. +/// +/// `enable` is folded into the flags the other way round from the stock triple: a parked record +/// (`enabled == false`, the grey-while-held arm) is still **on cooldown** — it has its full +/// duration and no end — and upstream has no column for the distinction, so this reports it as on +/// rather than hiding it. +fn cooldown_table( + lua: &Lua, + legs: &crate::script::CooldownLegsView, + now: f64, + item: Option<(u32, Option)>, +) -> mlua::Result { + let t = lua.create_table()?; + // `(remaining_ms, start_s, duration_ms, is_on)` for one leg, all four derived from the same + // triple so they cannot disagree about whether it is running. + let leg = |l: Option<(i64, u32, bool)>| match l { + Some((start_ms, duration_ms, _)) => { + let start_s = start_ms as f64 / 1000.0; + let remaining = ((start_s + f64::from(duration_ms) / 1000.0 - now) * 1000.0).max(0.0); + (remaining.round(), start_s, duration_ms, 1i64) + } + None => (0.0, 0.0, 0, 0), + }; + let individual = leg(legs.individual); + let category = leg(legs.category); + let gcd = leg(legs.gcd); + let folded = individual.0.max(category.0).max(gcd.0); + t.set("isOnCooldown", i64::from(folded > 0.0))?; + t.set("cooldownRemainingMs", folded)?; + for (prefix, (remaining, start_s, duration_ms, on), id) in [ + ("individual", individual, None), + ("category", category, Some(("categoryId", legs.category_id))), + ( + "gcdCategory", + gcd, + Some(("gcdCategoryId", legs.gcd_category_id)), + ), + ] { + if let Some((key, value)) = id { + t.set(key, value)?; + } + t.set(format!("{prefix}StartS"), start_s)?; + t.set(format!("{prefix}DurationMs"), duration_ms)?; + t.set(format!("{prefix}RemainingMs"), remaining)?; + // `isOnIndividualCooldown`, `isOnCategoryCooldown`, `isOnGcdCategoryCooldown` — the DLL's + // own capitalisation, which is why the first letter is upper-cased here rather than the + // prefix being spelled twice. + let mut cap = prefix.to_string(); + cap[..1].make_ascii_uppercase(); + t.set(format!("isOn{cap}Cooldown"), on)?; + } + // The three v2.23+ item columns, absent entirely for a spell query — upstream adds them only + // on the item side, and an `itemId` of 0 on a spell's table would read as a real item. + if let Some((item_id, use_spell)) = item { + t.set("itemId", item_id)?; + t.set("itemHasActiveSpell", i64::from(use_spell.is_some()))?; + t.set("itemActiveSpellId", use_spell.unwrap_or(0))?; + } + Ok(t) +} + +/// One bag slot as nampower's item row, **its own key names**, read off the DLL's string pool +/// (`0xa8a68`, the block that opens `itemId, permanentEnchantId, tempEnchantId, +/// randomPropertiesId, stackCount, duration, spellChargesRemaining, flags, durability, +/// maxDurability, tempEnchantmentTimeLeftMs, tempEnchantmentCharges` and carries `bagIndex`, +/// `slotIndex` for the bag variants). The same block [`equipped_table`] emits, which is the point: +/// `GetEquippedItem`, `GetBagItem` and `GetBagItems` hand back one shape, as upstream's three do. +/// +/// Three columns are **absent rather than zeroed**: `permanentEnchantId`, `tempEnchantId` and +/// `randomPropertiesId`. This client resolves a bag slot's enchants to `SpellItemEnchantment` +/// *names* at the app boundary ([`crate::script::EnchantView`] carries no id) and keeps no random +/// -properties id for a container slot at all, where the equipped path keeps the raw descriptor +/// beside the view. A nil is what a Lua table lookup gives for a key that is not there, and the +/// consumers read `itemId` and `stackCount`; a zeroed enchant id would be a claim that the item +/// has none. +fn bag_slot_table( + lua: &Lua, + bag: i64, + slot: u32, + s: &crate::script::ContainerSlot, +) -> mlua::Result { + let t = lua.create_table()?; + t.set("bagIndex", bag)?; + t.set("slotIndex", slot)?; + t.set("itemId", s.item_id)?; + t.set("stackCount", s.count)?; + t.set("texture", s.texture.clone())?; + t.set("flags", s.flags)?; + let (cur, max) = s.durability.unwrap_or((0, 0)); + t.set("durability", cur)?; + t.set("maxDurability", max)?; + t.set("spellChargesRemaining", s.charges)?; + // `duration` is `ITEM_FIELD_DURATION`, which the reference and the server both count in whole + // seconds (`SMSG_ITEM_TIME_UPDATE`); this client keeps it in ms for its tooltip countdown. + t.set("duration", s.duration_ms.unwrap_or(0) / 1000)?; + let temp = s.enchants.iter().find(|e| e.slot == 1); + t.set( + "tempEnchantmentTimeLeftMs", + temp.and_then(|e| e.remaining_ms).unwrap_or(0), + )?; + t.set("tempEnchantmentCharges", temp.map_or(0, |e| e.charges))?; + Ok(t) +} + /// The item cache row for an id, asking for it exactly as `GetItemInfo` does when it is a miss, so /// the call that follows the query's answer succeeds. /// @@ -926,10 +1042,10 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; - // `GetBagItem(bag, slot)` (v2.14+) -> that slot as a table, nil for an empty or unknown one. - // The columns are the ones a bag slot actually carries here; `GetItemStats(itemId)` beside it - // is the whole cache row, so this deliberately does not re-derive template fields it would - // have to guess at. + // `GetBagItem(bagIndex, slot)` -> that slot as a table, nil for an empty or unknown one, in + // [`bag_slot_table`]'s key names — which are the DLL's own, not a set derived from + // `GetContainerItemInfo`'s returns. `GetItemStats(itemId)` beside it is the whole cache row, + // so this deliberately does not re-derive template fields it would have to guess at. lua.globals().set( "GetBagItem", lua.create_function(|lua, (bag, slot): (Option, Option)| { @@ -950,37 +1066,126 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { let Some(s) = found else { return Ok(Value::Nil); }; - let t = lua.create_table()?; - t.set("bag", bag)?; - t.set("slot", slot)?; - t.set("itemId", s.item_id)?; - t.set("count", s.count)?; - t.set("texture", s.texture.clone())?; - t.set("quality", s.quality)?; - t.set("link", s.link.clone())?; - t.set("locked", s.locked)?; - Ok(Value::Table(t)) + Ok(Value::Table(bag_slot_table(lua, bag, slot, &s)?)) })?, )?; - // `GetItemIdCooldown(itemId)` (v2.14+) -> `start, duration, enable` on the `GetTime` clock, - // `GetContainerItemCooldown`'s triple for whichever bag slot holds that item. A cold or - // absent item answers `0, 0, 1`, which is what the stock verb answers for a slot with no - // cooldown — the consumer does arithmetic on these, so nil would be the wrong shape. + // `GetBagItems()` -> every occupied bag slot at once, as `{ [bagIndex] = { [slot] = row } }`. + // DoiteAuras asks for the whole picture seven times over and walks it exactly that way — + // `bags[b]`, then `for bslot, itemInfo in pairs(bagData)`, reading `itemInfo.itemId` and + // `itemInfo.stackCount` (`Modules/DoiteConditions.lua:1262-1300`, + // `DoiteAuras.lua:952`/`:996`, `Modules/DoiteEdit.lua:5951`/`:7368`). It is also why that + // addon can answer "do I have this item" without 100 `GetContainerItemInfo` calls per + // condition evaluation. + // + // **Empty slots are absent, and an empty bag with them.** A nil value in a Lua table *is* + // absence, and every consumer here iterates with `pairs` rather than counting to + // `GetContainerNumSlots` — so a bag of nils would make `pairs` yield nothing useful and an + // explicit `false` would make `itemInfo.itemId` raise. A bag the player does not have is + // absent for the same reason. + lua.globals().set( + "GetBagItems", + lua.create_function(|lua, ()| { + let out = lua.create_table()?; + let bags: Vec<(i64, Vec<(u32, crate::script::ContainerSlot)>)> = { + let model = lua + .app_data_ref::() + .expect("model app_data"); + (0i64..=4) + .filter_map(|bag| { + let c = model.containers.get(&bag)?; + let mut slots: Vec<(u32, crate::script::ContainerSlot)> = c + .slots + .iter() + .filter(|(_, s)| s.item_id != 0) + .map(|(slot, s)| (*slot, s.clone())) + .collect(); + // Ascending slot, which costs nothing and makes a `pairs` walk's order + // reproducible for anything that logs or diffs it. + slots.sort_unstable_by_key(|(slot, _)| *slot); + (!slots.is_empty()).then_some((bag, slots)) + }) + .collect() + }; + for (bag, slots) in bags { + let t = lua.create_table()?; + for (slot, s) in slots { + t.set(slot, bag_slot_table(lua, bag, slot, &s)?)?; + } + out.set(bag, t)?; + } + Ok(out) + })?, + )?; + + // `GetSpellIdCooldown(spellId)` and `GetItemIdCooldown(itemIdOrName)` (both v2.18+) -> one + // [`cooldown_table`], never a triple. + // + // **The shape was wrong here, and the addon layer is what says so.** + // `API.GetSpellCooldownInfo` and `API.GetItemCooldownInfo` *return these calls directly* + // (`NampowerAPI.lua:1950`, `:2035`) and their callers index the result — + // `cd.gcdCategoryRemainingMs` is the primary path of `API.GetGCDRemainingMs`, which the + // `[gcd]` and `[nogcd]` macro conditionals divide by 1000. A `start, duration, enable` + // triple makes that `(0.0).gcdCategoryRemainingMs`, which raises. The twenty key names are + // the DLL's own; see `cooldown_table`. + // + // A spell outside the player's book, or an item not carried, answers a **cold table** rather + // than nil: the wrappers' own no-native fallback builds `{ isOnCooldown = 0, + // cooldownRemainingMs = 0 }`, so a table is the shape the consumer is written for, and an + // absent record and a lapsed one are the same answer to the question asked. + // + // The reach is the player's book and the player's bags, which is narrower than upstream's — + // it reads the client's whole `SpellHistory`, where this reads the per-slot legs the + // spellbook and bag feeds already carry. For a spell the player has not learnt that is the + // same answer either way (no record), and the one consumer here asks about a book spell by + // construction: `API.GetGCDRemainingMs` probes with `GetSpellName(1, BOOKTYPE_SPELL)`. + lua.globals().set( + "GetSpellIdCooldown", + lua.create_function(|lua, spell_id: Option| { + let now: f64 = lua.globals().get("__benilla_now").unwrap_or(0.0); + let legs = { + let model = lua + .app_data_ref::() + .expect("model app_data"); + spell_id + .filter(|&id| id != 0) + .and_then(|id| { + model + .spellbook + .slots + .iter() + .chain(model.pet_book.slots.iter()) + .find(|s| s.spell_id == id) + }) + .map(|s| s.cooldown_legs) + .unwrap_or_default() + }; + Ok(Value::Table(cooldown_table(lua, &legs, now, None)?)) + })?, + )?; lua.globals().set( "GetItemIdCooldown", lua.create_function(|lua, item_id: Option| { let now: f64 = lua.globals().get("__benilla_now").unwrap_or(0.0); - let model = lua - .app_data_ref::() - .expect("model app_data"); - let cooldown = find_bag_slot(&model, item_id.as_ref()) - .and_then(|(bag, slot)| model.container_cooldowns.get(&(bag, slot)).copied()) - .filter(|&(start, duration, enabled)| start + duration > now || !enabled); - Ok(match cooldown { - Some((start, duration, enabled)) => (start, duration, i32::from(enabled)), - None => (0.0, 0.0, 1), - }) + let found = { + let model = lua + .app_data_ref::() + .expect("model app_data"); + find_bag_slot(&model, item_id.as_ref()).and_then(|(bag, slot)| { + model + .containers + .get(&bag) + .and_then(|c| c.slots.get(&slot)) + .map(|s| (s.cooldown_legs, s.item_id, s.use_spell_id)) + }) + }; + let (legs, item, use_spell) = found.unwrap_or_default(); + Ok(Value::Table(cooldown_table( + lua, + &legs, + now, + Some((item, (use_spell != 0).then_some(use_spell))), + )?)) })?, )?; @@ -1035,6 +1240,54 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; + // `GetSpellModifiers(spellId, modifierType)` -> `flat, percent, hasModifier`. The DLL's own + // usage string enumerates all 26 modifier types it accepts — `0=DAMAGE, 1=DURATION, + // 2=THREAT, 3=ATTACK_POWER, 4=CHARGES, 5=RANGE, 6=RADIUS, 7=CRITICAL_CHANCE, 8=ALL_EFFECTS, + // 9=NOT_LOSE_CASTING_TIME, 10=CASTING_TIME, 11=COOLDOWN, 12=SPEED, 14=COST, + // 15=CRIT_DAMAGE_BONUS, 16=RESIST_MISS_CHANCE, 17=JUMP_TARGETS, 18=CHANCE_OF_SUCCESS, + // 19=ACTIVATION_TIME, 20=EFFECT_PAST_FIRST, 21=CASTING_TIME_OLD, 22=DOT, 23=HASTE, + // 24=SPELL_BONUS_DAMAGE, 27=MULTIPLE_VALUE, 28=RESIST_DISPEL_CHANCE` — which is the same + // `SpellModOp` space `benilla-app`'s `spell/mods.rs` stores all 29 of, and the same numbers + // `SuperCleveRoidMacros` mirrors as `API.MODIFIER_*` constants (`NampowerAPI.lua:2491-2517`). + // + // `percent` is the **final multiplier**, not a delta: the consumer's own comment says so + // ("Nampower returns percent as final multiplier (90 = 90% of original, not -10% change)", + // `Utility.lua:6932`) and it multiplies by `percent / 100`. That is exactly the number the + // reference's reader produces, which adds the `+100` once at `0x6e6bb4`. + // + // **`hasModifier` is the whole point of the third return.** `0, 0, 0` and "a modifier whose + // sums happen to be zero" are different answers, and the consumer tests the flag before + // touching either number (`if not hasModifier or (flat == 0 and percent == 0) then return + // nil end`). Answering `0, 100, 0` for an unmodified spell would be the trap `spell/mods.rs` + // names at its own reader: a bare pair zeroes — or here, *multiplies by zero* — every spell + // no talent touches. + // + // 1.12 has no way to ask: the tables are written by `SMSG_SET_FLAT_SPELL_MODIFIER` and read + // only inside the client's own cost and duration arithmetic, with no binding at all. The fold + // runs in `spell/mods.rs`'s one reader through `Model::spell_modifier_probe`, so this cannot + // disagree with the mana cost the action bar greys a button on. + lua.globals().set( + "GetSpellModifiers", + lua.create_function(|lua, (spell_id, op): (Option, Option)| { + let model = lua + .app_data_ref::() + .expect("model app_data"); + let found = spell_id + .filter(|&id| id != 0) + .zip(op.and_then(|o| u8::try_from(o).ok())) + .and_then(|(id, op)| { + model + .spell_modifier_probe + .as_ref() + .and_then(|probe| probe(id, op)) + }); + Ok(match found { + Some((flat, pct)) => (flat, pct, 1), + None => (0, 0, 0), + }) + })?, + )?; + // ── nampower's own cast state: `GetCurrentCastingInfo` and `GetCastInfo` ────────────────── // // The two most-called nampower functions in grom's corpus, and the pair every other cast @@ -1359,32 +1612,97 @@ mod tests { "not carried" ); - // `GetBagItem` reports the slot it was asked about, not the found one. + // `GetBagItem` reports the slot it was asked about, not the found one, under the DLL's + // own key names — `stackCount` and `bagIndex`/`slotIndex`, never `count`/`bag`/`slot`. + // Asserted by name because that is the whole contract: a caller reading `.stackCount` + // off a row that spells it `count` gets nil and no error. assert_eq!( - s.eval::(r#"return GetBagItem(2, 1).count"#).unwrap(), - 20 + s.eval::<(u32, i64, u32)>( + r#"local r = GetBagItem(2, 1) return r.stackCount, r.bagIndex, r.slotIndex"# + ) + .unwrap(), + (20, 2, 1) ); assert_eq!( s.eval::(r#"return GetBagItem(0, 5).itemId"#).unwrap(), 1234 ); + assert!( + s.eval::>(r#"return GetBagItem(2, 1).count"#) + .unwrap() + .is_none(), + "`count` is this client's own `GetContainerItemInfo` spelling, not nampower's — a \ + row carrying keys the real DLL has not got would let an addon written here break on \ + a real install" + ); assert!(s .eval::>("return GetBagItem(0, 7)") .unwrap() .is_none_or(|v| v.is_nil())); - // A cold item answers the stock triple, not nil: the consumer does arithmetic on these. + // `GetBagItems()` is the same rows in one walk, keyed bag then slot, with empty slots and + // empty bags **absent** — DoiteAuras iterates both levels with `pairs`. assert_eq!( - s.eval::<(f64, f64, i64)>("return GetItemIdCooldown(1234)") - .unwrap(), - (0.0, 0.0, 1) + s.eval::<(u32, u32, u32)>( + "local b = GetBagItems() \ + return b[0][5].itemId, b[0][9].itemId, b[2][1].stackCount" + ) + .unwrap(), + (1234, 555, 20) ); assert_eq!( - s.eval::<(f64, f64, i64)>("return GetItemIdCooldown(4242)") - .unwrap(), - (0.0, 0.0, 1), - "not carried reads cold, as an absent slot does" + s.eval::>( + "local out = {} for bag in pairs(GetBagItems()) do table.insert(out, bag) end \ + table.sort(out) return out" + ) + .unwrap(), + vec![0, 2], + "bags 1, 3 and 4 are empty, so they are not there at all" ); + assert_eq!( + s.eval::("local n = 0 for _ in pairs(GetBagItems()[0]) do n = n + 1 end return n") + .unwrap(), + 2, + "two occupied slots in the backpack, and nothing for the other fourteen" + ); + // DoiteAuras' own walk (`DoiteConditions.lua:1266-1300`), looking for item 555 by id. + assert_eq!( + s.eval::<(i64, i64, i64)>( + r#" + local bags, found, fb, fs = GetBagItems(), 0, -1, -1 + local b = 0 + while b <= 4 do + local bagData = bags[b] + if bagData then + for bslot, itemInfo in pairs(bagData) do + if itemInfo and itemInfo.itemId == 555 then + found = found + (tonumber(itemInfo.stackCount) or 1) + if fb < 0 then fb, fs = b, bslot end + end + end + end + b = b + 1 + end + return found, fb, fs + "# + ) + .unwrap(), + (1, 0, 9) + ); + + // A cold item answers a cold **table**, not nil and not a triple: the wrappers return + // this call's result directly and their callers index it. + for call in ["GetItemIdCooldown(1234)", "GetItemIdCooldown(4242)"] { + assert_eq!( + s.eval::<(i64, f64, f64)>(&format!( + "local c = {call} \ + return c.isOnCooldown, c.cooldownRemainingMs, c.gcdCategoryRemainingMs" + )) + .unwrap(), + (0, 0.0, 0.0), + "{call} — not carried reads cold, as an absent slot does" + ); + } } /// `GetItemIconTexture(displayInfoId)` reads an `ItemDisplayInfo` row id, **not** an item id: @@ -1431,6 +1749,244 @@ mod tests { } } + /// `GetSpellIdCooldown` and `GetItemIdCooldown` answer a **table**, and the twenty keys are + /// asserted by name through `SuperCleveRoidMacros`' own wrappers — which return these calls' + /// results *directly* (`NampowerAPI.lua:1950`, `:2035`) and whose callers then index them. + /// + /// This is the round-trip that was broken: a `start, duration, enable` triple made + /// `API.GetGCDRemainingMs`'s primary path `(0.0).gcdCategoryRemainingMs`, which raises. + #[test] + fn the_cooldown_tables_carry_three_legs_and_a_folded_answer() { + let mut s = UiScript::new().unwrap(); + s.set_now(100.0); + // `NampowerAPI.lua:1898-1921`'s GCD probe, reduced to its native path. + s.run( + r#" + function GetGCDRemainingMs() + local cd = GetSpellIdCooldown(_gcdProbeSpellId) + if cd and cd.gcdCategoryRemainingMs and cd.gcdCategoryRemainingMs > 0 then + return cd.gcdCategoryRemainingMs + end + return 0 + end + _gcdProbeSpellId = 133 + "#, + ) + .unwrap(); + + // Nothing fed: a cold table, so the probe reads 0 rather than raising. + assert_eq!(s.eval::("return GetGCDRemainingMs()").unwrap(), 0.0); + assert_eq!( + s.eval::<(i64, i64, i64, i64)>( + "local c = GetSpellIdCooldown(133) \ + return c.isOnCooldown, c.isOnIndividualCooldown, c.isOnCategoryCooldown, \ + c.isOnGcdCategoryCooldown" + ) + .unwrap(), + (0, 0, 0, 0) + ); + + // Fireball: no cooldown of its own, the ordinary 1.5 s GCD under category 133, armed at + // t=99.4 so 900 ms of it is left at t=100. + s.set_spellbook(crate::script::SpellBookState { + slots: vec![crate::script::SpellSlotView { + spell_id: 133, + name: "Fireball".into(), + cooldown: Some((99_400, 1_500, true)), + cooldown_legs: crate::script::CooldownLegsView { + individual: None, + category_id: 0, + category: None, + gcd_category_id: 133, + gcd: Some((99_400, 1_500, true)), + }, + ..Default::default() + }], + ..Default::default() + }); + assert_eq!( + s.eval::("return GetGCDRemainingMs()").unwrap(), + 900.0, + "the one live consumer's primary path, which it then divides by 1000" + ); + assert_eq!( + s.eval::<(i64, f64, i64, f64, f64, i64)>( + "local c = GetSpellIdCooldown(133) \ + return c.isOnCooldown, c.cooldownRemainingMs, c.gcdCategoryId, \ + c.gcdCategoryStartS, c.gcdCategoryDurationMs, c.isOnGcdCategoryCooldown" + ) + .unwrap(), + (1, 900.0, 133, 99.4, 1_500.0, 1), + "the folded answer is the longest leg; the GCD leg carries its own category id, a \ + start in GetTime SECONDS and a duration in MILLISECONDS, which is the mixed-unit \ + table upstream really hands back" + ); + assert_eq!( + s.eval::<(i64, f64)>( + "local c = GetSpellIdCooldown(133) \ + return c.isOnIndividualCooldown, c.individualRemainingMs" + ) + .unwrap(), + (0, 0.0), + "Fireball has no cooldown of its own — the leg is cold, not a copy of the folded one" + ); + // A spell query carries none of the three item columns: an itemId of 0 on a spell's + // table would read as a real item. + assert!(s + .eval::>("return GetSpellIdCooldown(133).itemId") + .unwrap() + .is_none()); + + // Past the end, the remainders floor at 0 and the flags clear, off `GetTime()` alone — + // the feed pushes absolute starts and nothing per frame. + s.set_now(101.5); + assert_eq!( + s.eval::<(i64, f64)>( + "local c = GetSpellIdCooldown(133) \ + return c.isOnCooldown, c.gcdCategoryRemainingMs" + ) + .unwrap(), + (0, 0.0) + ); + + // An item: the same table plus the three v2.23+ columns. + s.set_now(100.0); + s.set_container( + 0, + Some(crate::script::ContainerState { + name: None, + num_slots: 16, + slots: [( + 1u32, + crate::script::ContainerSlot { + item_id: 13_444, + count: 1, + use_spell_id: 11_392, + cooldown_legs: crate::script::CooldownLegsView { + individual: Some((98_000, 120_000, true)), + category_id: 4, + category: Some((98_000, 30_000, true)), + gcd_category_id: 0, + gcd: None, + }, + ..Default::default() + }, + )] + .into_iter() + .collect(), + }), + ); + assert_eq!( + s.eval::<(i64, f64, f64, f64, i64, i64, i64)>( + "local c = GetItemIdCooldown(13444) \ + return c.isOnCooldown, c.cooldownRemainingMs, c.individualRemainingMs, \ + c.categoryRemainingMs, c.categoryId, c.itemId, c.itemActiveSpellId" + ) + .unwrap(), + (1, 118_000.0, 118_000.0, 28_000.0, 4, 13_444, 11_392), + "the two legs are reported apart and the folded answer is the longer" + ); + assert_eq!( + s.eval::("return GetItemIdCooldown(13444).itemHasActiveSpell") + .unwrap(), + 1 + ); + // By name as well as by id, through the same `find_bag_slot` every verb here shares. + s.set_item_template( + 13_444, + crate::script::ItemTemplateView { + name: "Major Fire Protection Potion".into(), + ..Default::default() + }, + ); + assert_eq!( + s.eval::( + r#"return GetItemIdCooldown("major fire protection potion").individualRemainingMs"# + ) + .unwrap(), + 118_000.0 + ); + } + + /// `GetSpellModifiers(spellId, modifierType)` through `SuperCleveRoidMacros`' own wrapper, + /// which is the only reader of it here. The third return is the point: the wrapper tests it + /// before touching either number, and the duration path multiplies by `percent / 100`, so an + /// unmodified spell answering `0, 0, 1` would make every debuff's duration zero. + #[test] + fn get_spell_modifiers_answers_a_pair_and_a_has_modifier_flag() { + let mut s = UiScript::new().unwrap(); + // `NampowerAPI.lua:2520-2532` and `Utility.lua:6913-6946`, transcribed. + s.run( + r#" + function GetModifiers(spellId, modifierType) + if not GetSpellModifiers then return 0, 0, false end + if not spellId or spellId == 0 then return 0, 0, false end + local flat, percent, ret = GetSpellModifiers(spellId, modifierType) + return flat or 0, percent or 0, ret and ret ~= 0 + end + function ModifiedDuration(spellId, baseDuration) + local flat, percent, hasModifier = GetModifiers(spellId, 1) + if not hasModifier or (flat == 0 and percent == 0) then return nil end + local modified = baseDuration + (flat or 0) + if percent and percent ~= 0 then modified = modified * (percent / 100) end + return modified + end + "#, + ) + .unwrap(); + + // No probe at all: the honest nothing, which the wrapper turns into "leave the duration + // alone" rather than into a zero. + assert_eq!( + s.eval::<(i64, i64, i64)>("return GetSpellModifiers(1120, 1)") + .unwrap(), + (0, 0, 0) + ); + assert!(s + .eval::>("return ModifiedDuration(1120, 18)") + .unwrap() + .is_none()); + + // Improved Corruption-shaped: Drain Soul's duration unchanged, Corruption's cast time + // cut. Only (1120, op 1) and (172, op 10) answer, so the gate is visible. + s.set_spell_modifier_probe(Box::new(|spell_id, op| match (spell_id, op) { + (1120, 1) => Some((6, 100)), + (172, 10) => Some((0, 60)), + _ => None, + })); + assert_eq!( + s.eval::<(i64, i64, i64)>("return GetSpellModifiers(1120, 1)") + .unwrap(), + (6, 100, 1) + ); + assert_eq!( + s.eval::("return ModifiedDuration(1120, 18)").unwrap(), + 24.0, + "flat first, then the percentage — and 100 is the identity multiplier, not a doubling" + ); + assert_eq!( + s.eval::<(i64, i64, i64)>("return GetSpellModifiers(172, 10)") + .unwrap(), + (0, 60, 1), + "percent is the final multiplier: 60 means 60% of the original" + ); + // A spell the probe has nothing for, and an op it has nothing for, both read as no + // modifier rather than as a zeroing one. + for call in [ + "GetSpellModifiers(172, 1)", + "GetSpellModifiers(1120, 10)", + "GetSpellModifiers(0, 1)", + "GetSpellModifiers()", + ] { + assert_eq!( + s.eval::<(i64, i64, i64)>(&format!("return {call}")) + .unwrap(), + (0, 0, 0), + "{call}" + ); + } + } + /// nampower's three posture reads and `IsAuraHidden`, in the shapes its own doc lines give: /// `1` or **nil**, never a boolean. `SuperCleveRoidMacros`'s `NampowerAPI.lua` gates each on /// `HasFeature`, which checks the version *and* `_G[name] ~= nil`, so a wrong return shape diff --git a/crates/benilla-ui/src/script/container.rs b/crates/benilla-ui/src/script/container.rs index c85ac948..bdfae537 100644 --- a/crates/benilla-ui/src/script/container.rs +++ b/crates/benilla-ui/src/script/container.rs @@ -43,6 +43,14 @@ pub struct ContainerSlot { pub durability: Option<(u32, u32)>, /// The use cooldown, `(start_ms on the GetTime clock, duration_ms, enabled)`; `None` if cold. pub cooldown: Option<(i64, u32, bool)>, + /// The same query's three legs kept apart, for nampower's `GetItemIdCooldown`. Fed beside + /// [`Self::cooldown`] from the one walk. + pub cooldown_legs: super::CooldownLegsView, + /// The template's on-use spell id, 0 for an item with none: `GetItemIdCooldown`'s + /// `itemActiveSpellId`, and the spell the cooldown legs above were keyed on — which is why it + /// rides here rather than being looked up again. Not the whole spell block: the other two or + /// three are equip and proc triggers, which no cooldown is queried against. + pub use_spell_id: u32, /// The instance carries `ITEM_FIELD_ITEM_TEXT_ID` (a mail copy): `GetContainerItemInfo`'s /// `readable`, which shows the Inspect cursor (`ContainerFrame.lua:638`). pub readable: bool, @@ -736,6 +744,8 @@ mod tests { slots.insert( 1, ContainerSlot { + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, charges: 0, petition: None, @@ -979,6 +989,8 @@ mod tests { state.slots.insert( 5, ContainerSlot { + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, charges: 0, petition: None, diff --git a/crates/benilla-ui/src/script/cursor.rs b/crates/benilla-ui/src/script/cursor.rs index 7a84ae90..f807ef11 100644 --- a/crates/benilla-ui/src/script/cursor.rs +++ b/crates/benilla-ui/src/script/cursor.rs @@ -572,6 +572,8 @@ mod tests { slots.insert( 1, crate::script::container::ContainerSlot { + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, charges: 0, petition: None, diff --git a/crates/benilla-ui/src/script/cursor/doll.rs b/crates/benilla-ui/src/script/cursor/doll.rs index b91038ce..cd5e6cd6 100644 --- a/crates/benilla-ui/src/script/cursor/doll.rs +++ b/crates/benilla-ui/src/script/cursor/doll.rs @@ -313,6 +313,8 @@ mod tests { slots.insert( 1, ContainerSlot { + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, charges: 0, petition: None, @@ -497,6 +499,8 @@ mod tests { slots.insert( 3, ContainerSlot { + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, charges: 0, petition: None, diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 3b754996..7609dfa1 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -246,7 +246,7 @@ pub use model::{ SpellProbe, SpellUsable, TextureProbe, TextureSizeProbe, }; pub(crate) use model::{Model, PendingTimer}; -pub use model::{PlayerCacheEntry, SelfCast, SelfMovement, ServerClock}; +pub use model::{PlayerCacheEntry, SelfCast, SelfMovement, ServerClock, SpellModifierProbe}; pub use party::{PartyMemberInfo, PartyRequest, PartyState, RaidMemberInfo, SavedInstanceInfo}; pub use pet::{PetActionView, PetStats}; pub use pvp::{HonorState, InspectHonorData}; @@ -266,7 +266,8 @@ pub use skills::{SkillEntry, SkillsState}; pub use social::{FriendInfo, SocialRequest, SocialState, WhoInfo}; pub use sound::{MusicRequest, SoundRequest}; pub use spellbook::{ - resolve_spell_by_name, PetBookState, SpellBookState, SpellSlotView, SpellTabView, + resolve_spell_by_name, CooldownLegsView, PetBookState, SpellBookState, SpellSlotView, + SpellTabView, }; pub use stable::{StableIntent, StablePetSlot, StableState, NUM_STABLE_SLOTS}; pub use summon::SummonConfirmUiState; diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index 45311da1..765886de 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -71,6 +71,19 @@ pub type SpellProbe = Box Option>; /// through (`benilla-app`'s `cursor.rs`). `None` where the row or its icon is absent. pub type ItemIconProbe = Box Option>; +/// nampower's `GetSpellModifiers(spellId, modifierType)`: the `(flat, percent)` pair the client's +/// own `GetSpellModifiers 0x6e6b30` folds out of its two talent-modifier tables, or `None` for +/// any of that reader's four false exits (the spell has no family, it is not the player's class +/// family, it carries `AttributesEx3` bit 29, or both sums came out zero). +/// +/// **A probe, not a pushed snapshot, and reinstalled when the tables change.** The arithmetic has +/// four gates and a 64-bit fold, and it is already written once in `benilla-app`'s +/// `spell/mods.rs` — the only reader of the two `i32[64][29]` tables the wire fills. Pushing the +/// tables here and folding them again would be a second implementation of that reader, free to +/// drift from the one the power cost is computed with. So the closure calls the real one, and +/// `ui_script/lifecycle.rs` reinstalls it whenever the wire writes a cell. +pub type SpellModifierProbe = Box Option<(i32, i32)>>; + /// A spell id to its `Spell.dbc` `Mechanic` column and that mechanic's enUS name from /// `SpellMechanic.dbc`, for ClassicAPI's `C_Spell.GetSpellMechanicByID`. /// @@ -785,6 +798,7 @@ pub(crate) struct Model { /// `SpellIcon.dbc` lookup ([`SpellIconProbe`]). pub(crate) spell_icon_probe: Option, pub(crate) item_icon_probe: Option, + pub(crate) spell_modifier_probe: Option, /// Creature display id to model path ([`CreatureModelProbe`]). pub(crate) creature_model_probe: Option, /// The pet's spellbook, a second array as in the reference (`0xb700f0`, `0xb6f098`). @@ -1686,6 +1700,7 @@ impl Model { texture_probe: None, spell_probe: None, item_icon_probe: None, + spell_modifier_probe: None, spell_mechanic_probe: None, player_cache: HashMap::new(), player_cache_enabled: [true, true], diff --git a/crates/benilla-ui/src/script/spellbook.rs b/crates/benilla-ui/src/script/spellbook.rs index 535d0320..4b45085a 100644 --- a/crates/benilla-ui/src/script/spellbook.rs +++ b/crates/benilla-ui/src/script/spellbook.rs @@ -33,6 +33,27 @@ pub struct SpellTabView { pub num_spells: u32, } +/// One cooldown query's three legs, each on the `GetTime()` clock as +/// `(start_ms, duration_ms, enabled)` and `None` when that leg holds no record — nampower's +/// `GetSpellIdCooldown`/`GetItemIdCooldown` report an `isOn…` flag per leg, and "no record" and +/// "a record with nothing left" are different answers to it. +/// +/// The arithmetic is `benilla-app`'s `spell/cooldowns.rs`, which folds the same three into the one +/// number a cooldown swirl draws; this carries them apart. Starts and durations are absolute, so +/// a running cooldown never churns the diff of whatever snapshot holds this, and a remainder is +/// derived in the binding from `GetTime()` rather than pushed. +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub struct CooldownLegsView { + /// The spell's (or item's) own recovery. + pub individual: Option<(i64, u32, bool)>, + /// `Spell.dbc` `Category`, the id the category leg was keyed on; 0 for none. + pub category_id: u32, + pub category: Option<(i64, u32, bool)>, + /// `StartRecoveryCategory`, the id the GCD leg was keyed on. + pub gcd_category_id: u32, + pub gcd: Option<(i64, u32, bool)>, +} + /// One spell in the flat book, every field resolved by the app. #[derive(Clone, Debug, Default, PartialEq)] pub struct SpellSlotView { @@ -51,6 +72,10 @@ pub struct SpellSlotView { /// `(start_ms on the GetTime clock, duration_ms, enabled)`, the triple the action bar carries; /// `None` is cold. The start is absolute, so a running cooldown never churns the book diff. pub cooldown: Option<(i64, u32, bool)>, + /// The same query's three legs kept apart, for nampower's `GetSpellIdCooldown`. Fed beside + /// [`Self::cooldown`] from the one walk, so the folded number a swirl draws and the legs an + /// addon reads cannot describe different timers. + pub cooldown_legs: CooldownLegsView, /// `GetSpellAutocast`'s `(allowed, enabled)`, pet book only, read off the pet's raw word /// (`0x4bd160`, bits 31 and 30). pub autocast: Option<(bool, bool)>, @@ -99,6 +124,13 @@ impl super::UiScript { self.model_mut().spell_icon_probe = Some(probe); } + /// Install (or replace) nampower's `GetSpellModifiers` fold. Replaced rather than set once: + /// the tables behind it are live wire state, so `ui_script/lifecycle.rs` calls this again + /// whenever a `SMSG_SET_*_SPELL_MODIFIER` lands. + pub fn set_spell_modifier_probe(&mut self, probe: super::SpellModifierProbe) { + self.model_mut().spell_modifier_probe = Some(probe); + } + /// Replace the live per-spell usability map ([`super::SpellUsable`]), nampower's /// `IsSpellUsable`/`IsSpellInRange` source. Pushed by the action feed, which already holds /// every input the reference's own usable walk reads. diff --git a/crates/benilla-ui/src/script/tests/reference_surface.rs b/crates/benilla-ui/src/script/tests/reference_surface.rs index 7ab02366..3825f592 100644 --- a/crates/benilla-ui/src/script/tests/reference_surface.rs +++ b/crates/benilla-ui/src/script/tests/reference_surface.rs @@ -338,7 +338,10 @@ fn allowed_beyond_1_12() -> HashSet<&'static str> { "UseItemIdOrName", "FindPlayerItemSlot", "GetBagItem", + "GetBagItems", + "GetSpellModifiers", "GetItemIdCooldown", + "GetSpellIdCooldown", "GetItemIconTexture", "GetTrinkets", "PlayerIsMoving", diff --git a/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs b/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs index c36264ec..4092c9ba 100644 --- a/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs +++ b/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs @@ -64,6 +64,8 @@ fn merchant_compare_renders_the_compare_shape_and_no_hover_seats_a_plate() { slots.insert( 1, ContainerSlot { + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, charges: 0, petition: None, diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index 13b9452c..74f3c025 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -578,7 +578,6 @@ because it is in the same family — it would flush our own `WoWCombatLog.txt`, | Name | What is missing | |---|---| | `GetSpellRangeData(rangeIndex)` | A `SpellRange.dbc` **row index**, per the DLL's own usage string — not a spell id, which is how this row read until the usage strings were compared against it. `IsSpellInRange` answers the *verdict* from the live usable map, never the yards, so the table is genuinely absent. One more probe beside `install_spell_extras_probes` | -| `GetSpellModifiers(spellId)` | `spell/mods.rs` stores the server's `SPELLMOD_*` cells but publishes no reader | | `GetUnitData(unit)` | The whole descriptor as a table, where `GetUnitField` answers one field. Needs a decision about which fields: the real one returns GUID-typed columns (charm, summon, charmedBy) this client's field reader does not type | | `GetTrinketCooldown()` | Equipped-slot cooldowns. `container_cooldowns` is keyed `(bag, slot)`; the equipped slots are not in it | @@ -678,6 +677,55 @@ variable, and `reference_ui.rs`'s beyond-1.12 event gate reads the string litera it had been blind to that pair for as long as they had existed. Both fire sites are literals now, with their rows; verified by probe (renaming either row makes the gate name it). +### The bag, modifier and cooldown group — and two more shapes the string pool corrected + +**Built: `GetBagItems`, `GetSpellModifiers`, `GetSpellIdCooldown`.** Plus two fixes to verbs that +already existed and had the wrong *shape*, both found the same way as `GetItemIconTexture`'s wrong +argument — by comparing what we answer against the DLL's own string pool. + +| Verb | What it needed | +|---|---| +| `GetBagItems()` | every occupied bag slot as `{ [bagIndex] = { [slot] = row } }`. DoiteAuras asks seven times over and walks both levels with `pairs`, reading `itemInfo.itemId` and `itemInfo.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)` | `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 `Model::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 | +| `GetSpellIdCooldown(spellId)` | the twenty-key cooldown 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. Fed beside the folded triple by the spellbook, the pet book and the bag feeds | + +**`GetBagItem`'s row had the wrong key names.** It answered `bag`, `slot`, `count`, `texture`, +`quality`, `link`, `locked` — this client's own `GetContainerItemInfo` spellings. nampower's row is +`itemId, permanentEnchantId, tempEnchantId, randomPropertiesId, stackCount, duration, +spellChargesRemaining, flags, durability, maxDurability, tempEnchantmentTimeLeftMs, +tempEnchantmentCharges`, plus `bagIndex`/`slotIndex` — contiguous at `0xa8a68`, and the same block +`GetEquippedItem` already used. So a caller reading `.stackCount` got nil and no error. Now one +`bag_slot_table` serves `GetBagItem` and `GetBagItems`, with three columns **absent rather than +zeroed**: this client resolves a bag slot's enchants to `SpellItemEnchantment` *names* at the app +boundary and keeps no random-properties id for a container slot, so `permanentEnchantId`, +`tempEnchantId` and `randomPropertiesId` are not there. A zeroed enchant id would be a claim. + +**`GetItemIdCooldown` returned the wrong *kind* of value.** It answered `start, duration, enable` — +the stock triple. At the version we report it is a table, and the addon layer is what settles it: +`API.GetItemCooldownInfo` and `API.GetSpellCooldownInfo` **return these calls' results directly** +(`NampowerAPI.lua:1950`, `:2035`) and their callers index them. `cd.gcdCategoryRemainingMs` is the +primary path of `API.GetGCDRemainingMs`, which the `[gcd]` and `[nogcd]` macro conditionals divide +by 1000 — against a triple that is `(0.0).gcdCategoryRemainingMs`, which raises. Nothing in the +corpus calls those two wrappers today, so the bug was latent rather than live; it was still wrong. + +Two decisions written down at the code: + +- **Every `*RemainingMs` is derived in the binding, from `GetTime()` and the leg's absolute + start.** A remainder is the one column that changes every frame, and pushing it would churn the + spellbook diff sixty times a second — the feed's own note says the absolute start is what keeps a + running cooldown out of that diff. +- **The reach is the player's book, pet book and bags, which is narrower than upstream's.** It + reads the client's whole `SpellHistory`; this reads the per-slot legs those feeds already carry. + For a spell the player has not learnt the answer is the same either way, and the one live + consumer asks about a book spell by construction (`API.GetGCDRemainingMs` probes with + `GetSpellName(1, BOOKTYPE_SPELL)`). + +**One count the earlier sweep got wrong, worth correcting here**: `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 — the only mention is a comment in +`SuperCleveRoidMacros/ClassicAPI.lua:272` saying it reads the same client field. A bare name match +is not a call site. + ### The whole registered list, accounted for With the registration table read (the section above), the surface is no longer a count of what a @@ -694,10 +742,10 @@ comm -23 /tmp/np /tmp/ours | Bucket | Count | Names | |---|---|---| -| **Built** | 34 | `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CastSpellByName`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `FindPlayerItemSlot`, `GetBagItem`, `GetEquippedItem`, `GetEquippedItems`, `GetItemIconTexture`, `GetItemIdCooldown`, `GetItemLevel`, `GetItemStats`, `GetItemStatsField`, `GetNampowerVersion`, `GetPlayerAuraDuration`, `GetSpellDuration`, `GetSpellIconTexture`, `GetSpellIdForName`, `GetSpellNameAndRankForId`, `GetSpellRecField`, `GetSpellSlotTypeIdForName`, `GetTrinkets`, `GetUnitField`, `GetUnitGUID`, `IsAuraHidden`, `IsSpellInRange`, `IsSpellUsable`, `PlayerIsMoving`, `PlayerIsRooted`, `PlayerIsSwimming`, `SetMouseoverUnit`, `UseItemIdOrName` | +| **Built** | 37 | `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CastSpellByName`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `FindPlayerItemSlot`, `GetBagItem`, `GetBagItems`, `GetEquippedItem`, `GetEquippedItems`, `GetItemIconTexture`, `GetItemIdCooldown`, `GetItemLevel`, `GetItemStats`, `GetItemStatsField`, `GetNampowerVersion`, `GetPlayerAuraDuration`, `GetSpellDuration`, `GetSpellIconTexture`, `GetSpellIdCooldown`, `GetSpellIdForName`, `GetSpellModifiers`, `GetSpellNameAndRankForId`, `GetSpellRecField`, `GetSpellSlotTypeIdForName`, `GetTrinkets`, `GetUnitField`, `GetUnitGUID`, `IsAuraHidden`, `IsSpellInRange`, `IsSpellUsable`, `PlayerIsMoving`, `PlayerIsRooted`, `PlayerIsSwimming`, `SetMouseoverUnit`, `UseItemIdOrName` | | **Withheld pending grom's say-so** | 8 | `WriteCustomFile`, `ReadCustomFile`, `CustomFileExists`, `ImportFile`, `ExportFile`, `ExecuteCustomLuaFile`, `StartItemExport` (writes a file of its own), `EncryptPassword` + `EncryptedServerLogin` | | **Not ours to add** | 4 | `QueueSpellByName`, `QueueScript`, `ChannelStopCastingNextTick`, `LearnTalentRank` | -| **To build, in corpus-call order** | 16 | `GetSpellPower` (7 call sites outside the API layer), `GetBagItems` (7), `GetSpellModifiers` (6), `DisenchantAll` (5), `GetSpellIdCooldown` (3), then `GetSpellRec`, `GetTrinketCooldown`, `UseTrinket`, `GetUnitData`, `GetSpellRangeData`, `CombatLogFlush`, `GetRaidTargets`, `SetLocalRaidTargetIndex`, `GetQuestLogQuestIds`, `GetQuestDialogQuestId` (0 each — present for completeness, not need) | +| **To build, in corpus-call order** | 13 | `DisenchantAll` (5 call sites outside the API layer), then `GetSpellPower`, `GetSpellRec`, `GetTrinketCooldown`, `UseTrinket`, `GetUnitData`, `GetSpellRangeData`, `CombatLogFlush`, `GetRaidTargets`, `SetLocalRaidTargetIndex`, `GetQuestLogQuestIds`, `GetQuestDialogQuestId`, `StartItemExport` (0 each — present for completeness, not need) | The two that were at the head of that last bucket are now built — see "The cast feed" above for why they came first and what each decision cost.