From f87ef148b8775c3d573958bbbc6f995d64d8b7f5 Mon Sep 17 00:00:00 2001 From: grom Date: Sat, 24 Oct 2026 10:21:31 -0600 Subject: [PATCH] =?UTF-8?q?app+ui:=20the=20trinket=20cooldown=20=E2=80=94?= =?UTF-8?q?=20and=20a=20stock=20verb=20that=20always=20said=20"cold"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Built: GetTrinketCooldown, UseTrinket, GetSpellRangeData, SetLocalRaidTargetIndex, GetQuestLogQuestIds, GetQuestDialogQuestId, CombatLogFlush. The find on the way through is not a nampower verb. **`GetInventoryItemCooldown` is a stock 1.12 verb and it answered `(0, 0, 1)` unconditionally** — right shape, always "cold", so nothing errored and no worn trinket ever drew its cooldown swirl on the character pane. Stock PaperDollFrame.lua:691 calls it on every slot update. Feeding an equipped slot's cooldown for GetTrinketCooldown fixes that one off the same column, and the test runs `CooldownFrame_SetTimer`'s own arithmetic against it. The feed keys its query on the (use spell, item entry) pair exactly as the bag feed does, so one trinket reads the same cooldown from the doll and from a bag. **The risk was the gate, and my first test for it was vacuous.** `feed_char` sits behind a change gate and no descriptor moves when a cooldown is armed, so `Cooldowns::feed_epoch` had to become a gate input. The first test passed with that input deleted: `run_system_once` builds a fresh system each call, so the gate's `Local` memo resets and `vm_reset` opens the gate every time. A test for a gate input has to run the system the way the schedule does. Rewritten with `add_systems` + `update`, it fails with the input removed — verified by probe, and the reason is written at the test. `feed_char` also hit Bevy's sixteen-parameter ceiling, which was a fair signal its parameters wanted grouping by what they are for: `ItemCooldownInputs` and `RepairInputs` replace six. Two decisions recorded at the code: - `GetSpellRangeData(rangeIndex)` answers three values, not a table. Every table-returning verb in this DLL has its key names in the string pool and this row's are not there. `flags` bit 0 is the melee family, where the engine takes the combat-reach sum instead of the authored pair — a caller ignoring the bit reads 0..5 for every melee ability. - `CombatLogFlush()` is a deliberate no-op. Upstream buffers its combat-log writes and this empties the buffer, so what an addon asks for is "my lines are on disk now" — and `ui_chat/logging.rs` writes each line to an unbuffered File as the window shows it, so that holds at all times. Defining it is the honest answer: the capability is present, and a missing global makes `HasFeature` report it absent. Also fed, both one wire column each: `QuestState::quest_id` (the wire names the quest on all three giver panels and 1.12's Lua names it nowhere — a title is not a key) and `SpellDisplay::stack_amount`'s neighbour work from the previous commit. Five names stay out, each now with the field list that makes it a decision rather than a shrug — `GetUnitData` (80 raw descriptor columns, read off the pool, zero call sites, and a partial answer is worse than none), `GetRaidTargets` (the reference keeps a board; this client feeds the mark per unit, and a scan over named tokens would silently omit a marked unit), `DisenchantAll` (a server-paced sequencer, which is the queue), `GetSpellPower`, `StartItemExport` (writes a file — it belongs with the withheld seven). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- crates/benilla-app/src/ui_char.rs | 232 ++++++++- crates/benilla-app/src/ui_quest/mod.rs | 3 + crates/benilla-app/src/ui_script/bag_tests.rs | 3 + .../src/ui_script/character_tests.rs | 3 + crates/benilla-app/src/ui_script/lifecycle.rs | 8 + .../src/ui_script/tooltip_compare_tests.rs | 6 + crates/benilla-formats/src/spells/ranges.rs | 2 +- crates/benilla-ui/src/script/char_stats.rs | 36 +- .../benilla-ui/src/script/compat/nampower.rs | 465 ++++++++++++++++++ crates/benilla-ui/src/script/cursor/doll.rs | 12 + crates/benilla-ui/src/script/mod.rs | 4 +- crates/benilla-ui/src/script/model.rs | 10 + crates/benilla-ui/src/script/quest.rs | 9 + crates/benilla-ui/src/script/spellbook.rs | 5 + .../src/script/tests/reference_surface.rs | 7 + docs/ADDON_COMPAT.md | 57 ++- 16 files changed, 835 insertions(+), 27 deletions(-) diff --git a/crates/benilla-app/src/ui_char.rs b/crates/benilla-app/src/ui_char.rs index 719bc9de..059fcf12 100644 --- a/crates/benilla-app/src/ui_char.rs +++ b/crates/benilla-app/src/ui_char.rs @@ -67,6 +67,61 @@ struct CharFeedMemo { /// flight, this catches the frame a lock clears with no object moving /// (`SMSG_INVENTORY_CHANGE_FAILURE`). pending_epoch: gate::Watch, + /// `Cooldowns::feed_epoch`: an equipped slot's use cooldown is in these views now, and no + /// descriptor moves when one is armed or expires. The epoch is the right input rather than a + /// per-frame hold-open, because the triple itself is absolute — a running cooldown changes + /// nothing until it is armed, re-armed or pruned. + cooldowns_epoch: gate::Watch, +} + +/// The three inputs an equipped slot's **use cooldown** needs, as one [`SystemParam`]: the record +/// list, the spell catalog its query keys on, and the frame's one clock pair. +/// +/// A group rather than three parameters because Bevy's `IntoSystem` stops at sixteen, and +/// [`feed_char`] was already at that ceiling — which is a fair signal that its parameters wanted +/// grouping by what they are *for* rather than listing one by one. +#[derive(bevy::ecs::system::SystemParam)] +pub(crate) struct ItemCooldownInputs<'w> { + cooldowns: Option>, + spells: Option>, + clock: Res<'w, crate::ui_script::UiClock>, +} + +impl ItemCooldownInputs<'_> { + /// The tuple `slot_view` takes, in one place so the doll and the bank bags pass the same + /// thing. + fn parts( + &self, + ) -> ( + Option<&crate::spell::Cooldowns>, + Option<&crate::ui_action::Spells>, + std::time::Instant, + f64, + ) { + ( + self.cooldowns.as_deref(), + self.spells.as_deref(), + self.clock.anchor, + self.clock.ui_now, + ) + } + + /// `Cooldowns::feed_epoch`, or 0 with no list: the gate input that opens this feed on the + /// frame a trinket's cooldown is armed or pruned. + fn epoch(&self) -> u64 { + self.cooldowns.as_ref().map_or(0, |c| c.feed_epoch()) + } +} + +/// The three inputs a slot's **repair cost** needs beyond the item itself, as one [`SystemParam`] +/// for the same reason: the price tables, the open vendor (whose guid is the only thing that turns +/// the discount on) and the reaction catalogs the discount reads. The merchant window's own feeds +/// take exactly the same three. +#[derive(bevy::ecs::system::SystemParam)] +pub(crate) struct RepairInputs<'w> { + tables: Option>, + merchant: Res<'w, crate::ui_merchant::MerchantOpen>, + reactions: crate::target::ReactionInputs<'w>, } /// The character-window feed; its bindings are `benilla-ui`'s `char_stats`. @@ -512,6 +567,15 @@ fn slot_view( // `DurabilityCosts.dbc`/`DurabilityQuality.dbc` and the open merchant's price discount, the // two inputs to `0x4faf30`. Absent tables answer 0, as the merchant window's rows do. repair: (Option<&crate::ui_merchant::RepairTables>, f64), + // The three inputs an equipped slot's use cooldown needs, grouped as `repair` is: the record + // list, the spell catalog its query keys on, and the frame's one clock pair. Absent, every + // slot reads cold — which is what this verb answered unconditionally before it was fed. + cooldowns: ( + Option<&crate::spell::Cooldowns>, + Option<&crate::ui_action::Spells>, + std::time::Instant, + f64, + ), guid: u64, live_id: u32, ) -> Option { @@ -568,6 +632,9 @@ fn slot_view( // already in the slots above. let roll = obj.item_random_properties_id(); let t = items.template(entry, guid, commands); + // The template's **on-use** spell block, the one a cooldown is queried against; the others + // are equip and proc triggers (`ui_items/feed.rs`'s bag cooldown reads the same field). + let use_spell = t.and_then(|t| t.use_spell); let (name, quality, display, link, equip_slots, kind, bar_placeable) = match t { Some(t) => { let name = rolls.name(&t.name, roll); @@ -632,6 +699,29 @@ fn slot_view( random_properties_id: obj.item_random_properties_id(), spell_charges: obj.item_spell_charges(0).unwrap_or(0), }, + // The use cooldown, keyed on the (use spell, item entry) pair exactly as the bag feed's + // is — `Cooldowns::info` matches an item record on both, so a trinket's record is found + // whether it was armed from a bag slot or from the doll. + use_spell_id: use_spell.map_or(0, |u| u.spell_id), + cooldown: use_spell.and_then(|u| { + let (Some(cds), anchor, ui_now) = (cooldowns.0, cooldowns.2, cooldowns.3) else { + return None; + }; + let sd = cooldowns.1.and_then(|s| s.catalog.get(u.spell_id)); + cds.info(u.spell_id, entry, sd, anchor) + .ui_triple(anchor, ui_now) + }), + cooldown_legs: match (use_spell, cooldowns.0) { + (Some(u), Some(cds)) => { + let sd = cooldowns.1.and_then(|s| s.catalog.get(u.spell_id)); + crate::ui_script::cooldown_legs_view( + &cds.legs(u.spell_id, entry, sd, cooldowns.2), + cooldowns.2, + cooldowns.3, + ) + } + _ => Default::default(), + }, }) } @@ -678,6 +768,12 @@ fn inventory_slots( sub_classes: Option<&crate::ui_items::ItemSubClasses>, classes: Option<&benilla_formats::ChrClasses>, repair: (Option<&crate::ui_merchant::RepairTables>, f64), + cooldowns: ( + Option<&crate::spell::Cooldowns>, + Option<&crate::ui_action::Spells>, + std::time::Instant, + f64, + ), ) -> InventorySlots { let has_relic_slot = store .0 @@ -724,6 +820,10 @@ fn inventory_slots( enchants: Vec::new(), duration_ms: None, raw: benilla_ui::script::ItemRawFields::default(), + // Ammo has no on-use spell and so no cooldown of its own. + cooldown: None, + cooldown_legs: Default::default(), + use_spell_id: 0, }); } // Equipment 1..=19, then `Bag0Slot`..`Bag3Slot` (20..=23 in `PaperDollItemFrame.dbc`), the @@ -743,6 +843,7 @@ fn inventory_slots( sub_classes, has_relic_slot, repair, + cooldowns, guid, live, ); @@ -764,6 +865,12 @@ fn bank_bag_slots( names: &crate::names::NameCache, sub_classes: Option<&crate::ui_items::ItemSubClasses>, repair: (Option<&crate::ui_merchant::RepairTables>, f64), + cooldowns: ( + Option<&crate::spell::Cooldowns>, + Option<&crate::ui_action::Spells>, + std::time::Instant, + f64, + ), ) -> BankBagSlots { let mut bags: BankBagSlots = Default::default(); for (i, bag) in bags.iter_mut().enumerate() { @@ -784,6 +891,9 @@ fn bank_bag_slots( // A bag is item class 1, which `DurabilityCosts` has no column for, so every bank bag // answers 0 through the same arithmetic rather than by a special case here. repair, + // A bank bag has no use spell, so this is the cold path either way; passed rather + // than stubbed so there is one shape of call into `slot_view`. + cooldowns, guid, BANK_BAG_LIVE_FIRST + i as u32, ); @@ -906,12 +1016,11 @@ pub(crate) fn feed_char( names: Res, // `ItemSubClass.dbc`, for `GetInventoryItemCount`'s container count bit. sub_classes: Option>, - // The three inputs to a slot's repair cost beyond the item itself: the price tables, the open - // vendor (whose guid is the only thing that turns the discount on) and the reaction catalogs - // the discount reads. The merchant window's own feeds take exactly the same four. - tables: Option>, - merchant: Res, - reactions: crate::target::ReactionInputs, + repair_in: RepairInputs, + // An equipped slot's use cooldown. `GetInventoryItemCooldown` is a **stock** verb and read + // `(0, 0, 1)` unconditionally before this was taken, so no trinket ever drew its swirl on the + // paper doll. + cooldowns: ItemCooldownInputs, ) { let Some(mut script) = script else { return; @@ -958,13 +1067,14 @@ pub(crate) fn feed_char( // // No `UNIT_NPC_FLAG_REPAIR` test, matching `RepairPurse::refuses`: the discount is a property // of the open merchant, not of its ability to repair. - let discount = merchant + let discount = repair_in + .merchant .vendor .and_then(|g| objects.store(g)) .map_or(0.0, |vendor| { crate::target::vendor_price_discount( - reactions.factions.as_deref(), - &reactions.reputations, + repair_in.reactions.factions.as_deref(), + &repair_in.reactions.reputations, vendor, store, ) @@ -984,6 +1094,7 @@ pub(crate) fn feed_char( // Both catalogs load once at startup; one input covers the pair. let enchants_changed = enchants.as_ref().is_some_and(|r| r.is_changed()) || props.as_ref().is_some_and(|r| r.is_changed()); + let cooldowns_moved = memo.cooldowns_epoch.moved(cooldowns.epoch()); let pending_held = !pending.is_empty(); let pending_moved = memo.pending_epoch.moved(pending.epoch()); // The f64 bits, not the value: a discount is one of five exact constants (0, .05, .10, .15, @@ -991,7 +1102,7 @@ pub(crate) fn feed_char( let discount_moved = memo.repair_discount.moved(discount.to_bits()); // The tables load once at startup; a frame before they land every cost reads 0, and this is // what re-runs the build on the frame they arrive. - let tables_added = tables.as_ref().is_some_and(|r| r.is_added()); + let tables_added = repair_in.tables.as_ref().is_some_and(|r| r.is_added()); gate::trace( "feed_char", &[ @@ -1007,6 +1118,7 @@ pub(crate) fn feed_char( ("pending-epoch", pending_moved), ("repair-discount", discount_moved), ("repair-tables", tables_added), + ("cooldowns", cooldowns_moved), ], ); let gate = gate::Gate::new( @@ -1021,7 +1133,8 @@ pub(crate) fn feed_char( || pending_held || pending_moved || discount_moved - || tables_added, + || tables_added + || cooldowns_moved, ); if gate.skip() { return; @@ -1051,7 +1164,8 @@ pub(crate) fn feed_char( &names, sub_classes.as_deref(), classes.as_deref().map(|t| &t.0), - (tables.as_deref(), discount), + (repair_in.tables.as_deref(), discount), + cooldowns.parts(), ); let bank_bags = bank_bag_slots( store, @@ -1066,7 +1180,8 @@ pub(crate) fn feed_char( &pending, &names, sub_classes.as_deref(), - (tables.as_deref(), discount), + (repair_in.tables.as_deref(), discount), + cooldowns.parts(), ); // Both bands push under one `UNIT_INVENTORY_CHANGED`: one wire update, one event. // `PLAYERBANKSLOTS_CHANGED`'s two producers, planned off the old memo: `false` is the @@ -1261,6 +1376,7 @@ mod tests { // The repair-cost inputs: with no vendor open and no tables loaded every slot's cost // is 0, which is what these bank-bag tests want. .init_resource::() + .init_resource::() .init_resource::() .insert_resource(NetCommands(tx)); let fields = match bag { @@ -1342,6 +1458,7 @@ mod tests { .init_resource::() .init_resource::() .init_resource::() + .init_resource::() .init_resource::() .insert_resource(crate::ui_merchant::RepairTables(repair_tables())) .insert_resource(NetCommands(tx)); @@ -1399,6 +1516,95 @@ mod tests { ) } + /// A worn trinket's use cooldown reaches the doll feed, and arming one **opens the gate** — + /// which is the whole risk in this change. No descriptor moves when a cooldown starts, so + /// without `Cooldowns::feed_epoch` as a gate input the swirl would appear only on the next + /// frame some unrelated thing changed. + /// + /// `GetInventoryItemCooldown` is a stock verb and answered `(0, 0, 1)` unconditionally before + /// this feed, so the character pane never drew a trinket's cooldown at all. + #[test] + fn arming_a_worn_trinkets_cooldown_opens_the_gate_and_reaches_the_stock_verb() { + use std::time::Instant; + + const F_TRINKET_1: u16 = benilla_protocol::field::FIELD_PLAYER_INV_SLOT_HEAD + 2 * 12; + const TRINKET: u64 = 0x4000_0000_0000_0d0d; + const TRINKET_ENTRY: u32 = 11_815; + const USE_SPELL: u32 = 11_392; + + let (tx, _rx) = crossbeam_channel::unbounded(); + let mut app = App::new(); + let anchor = Instant::now(); + app.init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .init_resource::() + .insert_resource(crate::ui_script::UiClock { + anchor, + ui_now: 100.0, + }) + .insert_resource(NetCommands(tx)); + app.world_mut().spawn(( + SelfPlayer, + ObjectStore(ObjectFields::from_pairs(&[ + (F_TRINKET_1, TRINKET as u32), + (F_TRINKET_1 + 1, (TRINKET >> 32) as u32), + ])), + )); + let mut t = crate::items::test_template("Hand of Justice"); + t.use_spell = Some(benilla_protocol::messages::ItemUseSpell { + spell_id: USE_SPELL, + cooldown_ms: 120_000, + category: 0, + category_cooldown_ms: 0, + }); + app.world_mut() + .resource_mut::() + .insert_template(TRINKET_ENTRY, Some(t)); + crate::items::test_spawn_item( + app.world_mut(), + TRINKET, + ObjectFields::from_pairs(&[(F_OBJECT_ENTRY, TRINKET_ENTRY)]), + false, + ); + let mut script = UiScript::new().unwrap(); + script.set_now(100.0); + app.insert_non_send_resource(script); + + let ask = |app: &mut App| { + app.world_mut() + .non_send_resource_mut::() + .eval::<(f64, f64, i64)>(r#"return GetInventoryItemCooldown("player", 13)"#) + .unwrap() + }; + // `add_systems` + `update`, not `run_system_once`: the gate's memo is a `Local`, and a + // one-shot run gets a fresh one every time — which makes `vm_reset` true on every call + // and opens the gate whatever its other inputs say. A test for a gate input has to run + // the system the way the schedule does, or it passes with that input deleted. (It did.) + app.add_systems(Update, feed_char); + app.update(); + assert_eq!(ask(&mut app), (0.0, 0.0, 1), "nothing armed yet"); + // A second frame with nothing moving, so the gate is genuinely shut before the arm. + app.update(); + + // The wire arms the item's use cooldown. Nothing about the item object changes. + app.world_mut() + .resource_mut::() + .apply_wire_item_cooldown(TRINKET_ENTRY, USE_SPELL, anchor); + app.update(); + let (start, duration, enable) = ask(&mut app); + assert!( + (start - 100.0).abs() < 0.05 && duration > 0.0 && enable == 1, + "the armed cooldown reaches the stock verb on the frame it is armed, got \ + {start} / {duration} / {enable}" + ); + } + /// The equipped band's repair quote is `0x4faf30`'s own number, not 0 — which is what the /// stock paper-doll tooltip's "Repair Cost" line and pfUI's durability panel both read. With /// no vendor open it is the full list price, because `0x4faf30` takes a discount only once it diff --git a/crates/benilla-app/src/ui_quest/mod.rs b/crates/benilla-app/src/ui_quest/mod.rs index ece064b4..246cd6ea 100644 --- a/crates/benilla-app/src/ui_quest/mod.rs +++ b/crates/benilla-app/src/ui_quest/mod.rs @@ -372,6 +372,7 @@ fn snapshot( } } QuestView::Detail(d) => QuestState { + quest_id: d.quest_id, panel: QuestPanel::Detail, title: sub(&d.title), body: sub(&d.details), @@ -383,6 +384,7 @@ fn snapshot( ..Default::default() }, QuestView::Progress(p) => QuestState { + quest_id: p.quest_id, panel: QuestPanel::Progress, title: sub(&p.title), body: sub(&p.request_text), @@ -392,6 +394,7 @@ fn snapshot( ..Default::default() }, QuestView::Reward(o) => QuestState { + quest_id: o.quest_id, panel: QuestPanel::Reward, title: sub(&o.title), body: sub(&o.offer_text), diff --git a/crates/benilla-app/src/ui_script/bag_tests.rs b/crates/benilla-app/src/ui_script/bag_tests.rs index cea27928..f258c812 100644 --- a/crates/benilla-app/src/ui_script/bag_tests.rs +++ b/crates/benilla-app/src/ui_script/bag_tests.rs @@ -1375,6 +1375,9 @@ fn the_bar_bag_buttons_name_themselves_on_hover() { // inventory slot 20. let mut inv: benilla_ui::script::InventorySlots = Default::default(); inv[20] = Some(benilla_ui::script::InvSlotView { + cooldown: None, + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, already_bound: false, bar_placeable: true, diff --git a/crates/benilla-app/src/ui_script/character_tests.rs b/crates/benilla-app/src/ui_script/character_tests.rs index 94ad6fed..121d4d7d 100644 --- a/crates/benilla-app/src/ui_script/character_tests.rs +++ b/crates/benilla-app/src/ui_script/character_tests.rs @@ -52,6 +52,9 @@ fn combat_stats() -> UnitCombatStats { fn inventory_with_head_item() -> InventorySlots { let mut slots: InventorySlots = Default::default(); slots[1] = Some(InvSlotView { + cooldown: None, + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, already_bound: false, bar_placeable: true, diff --git a/crates/benilla-app/src/ui_script/lifecycle.rs b/crates/benilla-app/src/ui_script/lifecycle.rs index ceafa9b3..cc4b2fe6 100644 --- a/crates/benilla-app/src/ui_script/lifecycle.rs +++ b/crates/benilla-app/src/ui_script/lifecycle.rs @@ -288,6 +288,14 @@ fn install_spell_catalog_probe(world: &World, script: &mut UiScript) { // mechanic *name* needs it, so a missing or unreadable one costs exactly that name — not // the duration, the cost and the icon id beside it, which was the case while this whole // block sat inside `if let Some(mechanics)`. + // nampower's `GetSpellRangeData(rangeIndex)`: `SpellRange.dbc` by row index, which no + // other verb here reaches — `IsSpellInRange` answers the verdict, never the yards. + { + let ranges = spells.ranges.clone(); + script.set_spell_range_probe(Box::new(move |index| { + ranges.get(index).map(|r| (r.min, r.max, r.flags)) + })); + } install_spell_extras_probes( script, &spells.catalog, 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 39791546..c6edb643 100644 --- a/crates/benilla-app/src/ui_script/tooltip_compare_tests.rs +++ b/crates/benilla-app/src/ui_script/tooltip_compare_tests.rs @@ -73,6 +73,9 @@ fn harness_with_bags() -> UiScript { fn seed_items(s: &mut UiScript) { let mut inv: InventorySlots = Default::default(); inv[1] = Some(InvSlotView { + cooldown: None, + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, already_bound: false, bar_placeable: true, @@ -194,6 +197,9 @@ fn doll_hover_renders_the_live_instance() { // A broken helm: instance pair (0, 40), the template at full. let mut inv: InventorySlots = Default::default(); inv[1] = Some(InvSlotView { + cooldown: None, + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, already_bound: false, bar_placeable: true, diff --git a/crates/benilla-formats/src/spells/ranges.rs b/crates/benilla-formats/src/spells/ranges.rs index d8eec939..b530f7a1 100644 --- a/crates/benilla-formats/src/spells/ranges.rs +++ b/crates/benilla-formats/src/spells/ranges.rs @@ -82,7 +82,7 @@ pub fn min_max_range( } /// `SpellRange.dbc`, by row id ([`crate::spells::SpellDisplay::range_index`]). -#[derive(Default)] +#[derive(Clone, Default)] pub struct SpellRangeCatalog { ranges: HashMap, } diff --git a/crates/benilla-ui/src/script/char_stats.rs b/crates/benilla-ui/src/script/char_stats.rs index 3c655392..b9d238ba 100644 --- a/crates/benilla-ui/src/script/char_stats.rs +++ b/crates/benilla-ui/src/script/char_stats.rs @@ -215,6 +215,15 @@ pub struct InvSlotView { pub duration_ms: Option, /// The raw descriptor numbers nampower's equipment getters report ([`ItemRawFields`]). pub raw: ItemRawFields, + /// The slot's use cooldown, `(start_ms on the GetTime clock, duration_ms, enabled)`; `None` + /// is cold. Backs `GetInventoryItemCooldown` — a **stock 1.12 verb** that answered `(0, 0, 1)` + /// unconditionally until this was fed, so the paper doll never drew a trinket's swirl. + pub cooldown: Option<(i64, u32, bool)>, + /// The same query's three legs, for nampower's `GetTrinketCooldown`. + pub cooldown_legs: super::CooldownLegsView, + /// The template's on-use spell id, 0 for none: `GetTrinketCooldown`'s `itemActiveSpellId`, + /// and the spell the legs above were keyed on. + pub use_spell_id: u32, } /// The item-descriptor numbers nampower's `GetEquippedItem`/`GetEquippedItems` report, kept apart @@ -876,13 +885,30 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; - // GetInventoryItemCooldown(unit, slot) → (start, duration, enable). Not fed: with no - // equipped-item cooldown source it answers `(0, 0, 1)`, no cooldown, as the container verb - // does; stock calls it on every slot update (`PaperDollFrame.lua:691`). + // GetInventoryItemCooldown(unit, slot) → (start, duration, enable), the triple stock calls on + // every slot update (`PaperDollFrame.lua:691`) to drive a worn item's cooldown swirl. + // + // **This answered `(0, 0, 1)` unconditionally until the doll feed carried a cooldown.** The + // shape was right and the number was always "cold", so nothing errored and no trinket ever + // swirled on the character pane — the quietest kind of wrong. The feed is + // `benilla-app`'s `ui_char::slot_view`, keyed on the (use spell, item entry) pair exactly as + // the bag feed's is, so one trinket reads the same cooldown from the doll and from a bag. + // + // A cold or absent slot is still `(0, 0, 1)`: the caller does arithmetic on these and stock + // `CooldownFrame_SetTimer` reads a 0 duration as "hide", so nil would be the wrong shape. + // Another unit's slot is cold too — an inspected player's cooldowns are not on the wire. g.set( "GetInventoryItemCooldown", - lua.create_function(|_, (_token, _slot): (Option, i64)| { - Ok((0.0f64, 0.0f64, 1i64)) + lua.create_function(|lua, (token, slot): (Option, i64)| { + let cooldown = player_inv_slot(lua, &token, slot).and_then(|v| v.cooldown); + Ok(match cooldown { + Some((start_ms, duration_ms, enabled)) => ( + start_ms as f64 / 1000.0, + f64::from(duration_ms) / 1000.0, + i64::from(enabled), + ), + None => (0.0, 0.0, 1), + }) })?, )?; diff --git a/crates/benilla-ui/src/script/compat/nampower.rs b/crates/benilla-ui/src/script/compat/nampower.rs index 5ff178a8..5fe9bc6f 100644 --- a/crates/benilla-ui/src/script/compat/nampower.rs +++ b/crates/benilla-ui/src/script/compat/nampower.rs @@ -194,6 +194,53 @@ fn spell_rec_value(lua: &Lua, value: crate::script::SpellRecValue) -> mlua::Resu }) } +/// Which equipped trinket slot `ident` names: `1`/`2` are the two slots themselves, any other +/// number is an item id, and a string is a case-insensitive name match. `None` for an argument +/// that names neither slot's contents. +/// +/// One helper for both trinket verbs, the same way [`find_bag_slot`] serves the bag group: a +/// getter and an action that disagreed about which of two trinkets the argument meant would be +/// worse than either being absent. +fn trinket_slot_index(model: &crate::script::Model, ident: Option<&Value>) -> Option { + let token = inv_token(model, None); + let occupied = |slot: usize| { + model + .inv_slot(&token, slot) + .filter(|v| v.item_id != 0) + .map(|v| (slot, v)) + }; + match ident { + Some(Value::Integer(1)) => occupied(TRINKET_SLOTS[0]).map(|(s, _)| s), + Some(Value::Integer(2)) => occupied(TRINKET_SLOTS[1]).map(|(s, _)| s), + Some(Value::Integer(n)) => { + let id = u32::try_from(*n).ok()?; + TRINKET_SLOTS + .into_iter() + .filter_map(occupied) + .find(|(_, v)| v.item_id == id) + .map(|(s, _)| s) + } + Some(Value::String(s)) => { + let want = s.to_str().ok()?.to_lowercase(); + TRINKET_SLOTS + .into_iter() + .filter_map(occupied) + .find(|(_, v)| v.name.as_deref().is_some_and(|n| n.to_lowercase() == want)) + .map(|(s, _)| s) + } + _ => None, + } +} + +/// [`trinket_slot_index`]'s slot resolved to its view. +fn trinket_slot( + model: &crate::script::Model, + ident: Option<&Value>, +) -> Option { + let slot = trinket_slot_index(model, ident)?; + model.inv_slot(&inv_token(model, None), slot) +} + /// 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 @@ -642,6 +689,125 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { )?, )?; + // `GetSpellRangeData(rangeIndex)` -> `min, max, flags`, the `SpellRange.dbc` row's own three + // columns in order. **A row index, not a spell id**, which is the DLL's own usage string + // (`Usage: GetSpellRangeData(rangeIndex)`) — the caller gets the index from + // `GetSpellRecField(spellId, "rangeIndex")`, the sibling two lines up. + // + // 1.12's Lua reaches a range only through `SpellRangeFunc`-formatted tooltip text, and + // `C_Spell.IsSpellInRange` answers the *verdict* from the live usable map — so the two yard + // numbers have no other path out of this client. `flags` bit 0 is the melee family, where + // `GetMinMaxRange` takes the combat-reach sum instead of the authored pair, so a caller that + // ignores it will read 0..5 for every melee ability. + // + // Three returns rather than a table: there are no field-name strings for this row anywhere in + // the DLL's pool, where every table-returning verb's keys are in it — so a flat row is what + // it answers. Nothing in grom's corpus calls this verb, which is also why the reading is + // recorded here rather than argued for. + lua.globals().set( + "GetSpellRangeData", + lua.create_function(|lua, index: Option| { + let model = lua + .app_data_ref::() + .expect("model app_data"); + Ok( + match index + .and_then(|i| model.spell_range_probe.as_ref().and_then(|probe| probe(i))) + { + Some((min, max, flags)) => ( + Some(f64::from(min)), + Some(f64::from(max)), + Some(i64::from(flags)), + ), + None => (None, None, None), + }, + ) + })?, + )?; + + // `SetLocalRaidTargetIndex(unitStr, raidTargetIndex)`: a mark only this client sees, where + // `SetRaidTarget` asks the server to set one for the group. + // + // The same queue SuperWoW's own spelling goes through — `SetRaidTarget(unit, index, "local")`, + // whose third argument is this flag (`script/party.rs`) — so the two names cannot disagree + // about what a local mark is, and a 0 clears as it does there. Out of range is dropped rather + // than clamped: the DLL's own message is + // `SetLocalRaidTargetIndex: raidTargetIndex must be in range 0-8`, so upstream refuses too. + lua.globals().set( + "SetLocalRaidTargetIndex", + lua.create_function(|lua, (unit, index): (Option, Option)| { + let (Some(unit), Some(index)) = (unit, index.filter(|&i| i <= 8)) else { + return Ok(()); + }; + let set: mlua::Function = lua.globals().get("SetRaidTarget")?; + set.call::<()>((unit, index, 1)) + })?, + )?; + + // `GetQuestLogQuestIds()` -> the quest ids in the log, as a 1-based list. + // + // Header rows are left out: they are not quests, they carry id `0` + // (`GetQuestIDForLogIndex`'s own documented answer for one), and a list with zeros in it is a + // list a caller has to filter. So the list is ids and the indices are its own, **not** log + // indices — `GetQuestIDForLogIndex(i)` is the verb that answers by log index, and it is right + // next door. Nothing in grom's corpus calls this one, so the shape is recorded rather than + // measured. + lua.globals().set( + "GetQuestLogQuestIds", + lua.create_function(|lua, ()| { + let ids: Vec = { + let model = lua + .app_data_ref::() + .expect("model app_data"); + model + .quest_log + .entries + .iter() + .map(|e| e.quest_id) + .filter(|&id| id != 0) + .collect() + }; + let t = lua.create_table()?; + for (i, id) in ids.into_iter().enumerate() { + t.set(i as i64 + 1, id)?; + } + Ok(t) + })?, + )?; + + // `GetQuestDialogQuestId()` -> the quest id of the open quest giver's panel, or nil. + // + // The wire names the quest on all three panels (`SMSG_QUESTGIVER_QUEST_DETAILS`, + // `_REQUEST_ITEMS`, `_OFFER_REWARD`) and 1.12's Lua names it nowhere — `GetTitleText` answers + // the title, and a title is not a key, which is the whole reason an addon wanting to record + // "the player accepted quest N" cannot do it on the stock surface. The **greeting** panel is a + // list of quests and names none of them, so it answers nil rather than one of the rows. + lua.globals().set( + "GetQuestDialogQuestId", + lua.create_function(|lua, ()| { + let model = lua + .app_data_ref::() + .expect("model app_data"); + Ok(model + .quest + .as_ref() + .map(|q| q.quest_id) + .filter(|&id| id != 0)) + })?, + )?; + + // `CombatLogFlush()`: upstream buffers its combat-log writes and this empties the buffer, so + // what an addon is asking for is "my lines are on disk now". + // + // Here that postcondition holds at all times: `ui_chat/logging.rs` writes each line to an + // unbuffered `std::fs::File` as the window shows it, so there is nothing held back to flush. + // Defined rather than left absent because the capability really is present — a caller that + // feature-detects this and then reads the file gets what it asked for — and a no-op that says + // why is better than a missing global that makes `HasFeature` report the capability absent + // when it is not. + lua.globals() + .set("CombatLogFlush", lua.create_function(|_, ()| Ok(()))?)?; + // `GetSpellNameAndRankForId(spellID)` -> name, rank. DoiteAuras' hardest-leaned-on nampower // call (18 sites): every aura it tracks by id resolves its label through this before falling // back to a 200-slot spellbook scan. Both values come off the same `Spell.dbc` display row @@ -1305,6 +1471,65 @@ pub(in crate::script) fn install(lua: &Lua) -> mlua::Result<()> { })?, )?; + // `GetTrinketCooldown(slot|itemIdOrName)` -> the same [`cooldown_table`] the two id-keyed + // cooldown verbs answer, for whichever trinket the argument names. + // + // `1` and `2` are the trinket *slots*, which is what the usage string's `slot|itemIdOrName` + // means and the numbering `GetTrinkets()` beside it keys its two rows on; anything else is an + // item id or a name, matched against the two equipped slots. A number of 1 or 2 is therefore + // a slot and never an item id — no real item has either id, so the overload is unambiguous in + // practice as well as by the usage string. + // + // The legs come off the doll feed, which keys its query on the (use spell, item entry) pair + // exactly as the bag feed does — so a trinket a player carries a second copy of in a bag reads + // one cooldown through both verbs. + lua.globals().set( + "GetTrinketCooldown", + lua.create_function(|lua, ident: Option| { + let now: f64 = lua.globals().get("__benilla_now").unwrap_or(0.0); + let found = { + let model = lua + .app_data_ref::() + .expect("model app_data"); + trinket_slot(&model, ident.as_ref()) + }; + let (legs, item, use_spell) = found.map_or_else(Default::default, |v| { + (v.cooldown_legs, v.item_id, v.use_spell_id) + }); + Ok(Value::Table(cooldown_table( + lua, + &legs, + now, + Some((item, (use_spell != 0).then_some(use_spell))), + )?)) + })?, + )?; + + // `UseTrinket(slot|itemIdOrName, [target])`: the same argument grammar, through the stock + // `UseInventoryItem` so the send, the self-cast flag and the bind confirmation are the ones + // every other item use goes through. `UseItemIdOrName` beside it is the bag-slot sibling. + // + // `target` is accepted and ignored, as `UseItemIdOrName`'s is: 1.12's `UseInventoryItem` takes + // a self-cast flag and not a unit, and the cast's target is resolved from the spell's own + // constraints at the send (`spell/cast_target.rs`). Refusing the argument would be worse — + // the caller passes whatever the macro said. + lua.globals().set( + "UseTrinket", + lua.create_function(|lua, (ident, _target): (Option, Option)| { + let slot = { + let model = lua + .app_data_ref::() + .expect("model app_data"); + trinket_slot_index(&model, ident.as_ref()) + }; + if let Some(slot) = slot { + let use_item: mlua::Function = lua.globals().get("UseInventoryItem")?; + use_item.call::<()>(slot)?; + } + Ok(()) + })?, + )?; + // `GetTrinkets()` (v2.14+) -> the two trinket slots as a table keyed `1` and `2`, the same // row shape `GetEquippedItem` hands back, with an empty slot absent rather than nil-valued // (a nil value in a Lua table *is* absence, and `SuperCleveRoidMacros` walks it with `for i = @@ -2734,6 +2959,246 @@ mod tests { /// `GetSpellRecField`: the columns this client has, and **nil for the ones it does not** — /// asserted, because a plausible zero is worse than nothing here (DoiteAuras' Clearcasting + /// The five small registered verbs: the range row, the local mark, the two quest readers and + /// the flush. Grouped in one test because each is a handful of lines and none has a consumer + /// to transcribe — what is pinned is the shape, the unit and the "nothing to say" answer. + #[test] + fn the_range_row_the_local_mark_the_quest_ids_and_the_flush() { + let mut s = UiScript::new().unwrap(); + + // `GetSpellRangeData(rangeIndex)`: a ROW INDEX, which is the DLL's own usage string. + assert_eq!( + s.eval::<(Option, Option, Option)>("return GetSpellRangeData(4)") + .unwrap(), + (None, None, None), + "no probe: nothing, not a 0..0 range every melee check would pass" + ); + s.set_spell_range_probe(Box::new(|index| match index { + 4 => Some((0.0, 30.0, 0)), + 1 => Some((0.0, 5.0, 1)), + _ => None, + })); + assert_eq!( + s.eval::<(f64, f64, i64)>("return GetSpellRangeData(4)") + .unwrap(), + (0.0, 30.0, 0) + ); + assert_eq!( + s.eval::<(f64, f64, i64)>("return GetSpellRangeData(1)") + .unwrap(), + (0.0, 5.0, 1), + "flags bit 0 is the melee family, where the engine uses the reach sum and not this \ + pair — a caller that ignores the bit reads 0..5 for every melee ability" + ); + assert!(s + .eval::>("return GetSpellRangeData(9999)") + .unwrap() + .is_none()); + + // `SetLocalRaidTargetIndex` goes through `SetRaidTarget`'s own local flag, so the two + // spellings cannot disagree about what a local mark is. Asserted on the queue both write. + s.eval::<()>(r#"SetLocalRaidTargetIndex("target", 8)"#) + .unwrap(); + s.eval::<()>(r#"SetRaidTarget("focus", 3, 1)"#).unwrap(); + // Out of range is dropped, as upstream's own message says it refuses. + s.eval::<()>(r#"SetLocalRaidTargetIndex("target", 9)"#) + .unwrap(); + s.eval::<()>(r#"SetLocalRaidTargetIndex("target")"#) + .unwrap(); + let queued: Vec<(String, u8, bool)> = s + .take_party_requests() + .into_iter() + .filter_map(|r| match r { + crate::script::PartyRequest::SetRaidTarget { unit, index, local } => { + Some((unit, index, local)) + } + _ => None, + }) + .collect(); + assert_eq!( + queued, + vec![ + ("target".to_string(), 8, true), + ("focus".to_string(), 3, true), + ], + "the local flag is set, and the two out-of-range calls queued nothing" + ); + + // `GetQuestLogQuestIds()` is ids, headers left out, 1-based over its own list — not over + // log indices, which is what `GetQuestIDForLogIndex` answers by. + assert_eq!( + s.eval::("return table.getn(GetQuestLogQuestIds())") + .unwrap(), + 0, + "an empty log is an empty table, never nil — the caller walks it" + ); + s.set_quest_log(crate::script::QuestLogState { + entries: vec![ + crate::script::QuestLogEntryView { + quest_id: 0, + title: "Elwynn Forest".into(), + is_header: true, + ..Default::default() + }, + crate::script::QuestLogEntryView { + quest_id: 83, + title: "Wolves Across the Border".into(), + ..Default::default() + }, + crate::script::QuestLogEntryView { + quest_id: 176, + title: "Cloth and Leather Armor".into(), + ..Default::default() + }, + ], + ..Default::default() + }); + assert_eq!( + s.eval::>("return GetQuestLogQuestIds()").unwrap(), + vec![83, 176], + "the header's id 0 is not in the list" + ); + assert_eq!( + s.eval::("return GetQuestIDForLogIndex(2)").unwrap(), + 83, + "and the by-index verb still counts the header, which is why both exist" + ); + + // `GetQuestDialogQuestId()`: nil with no giver open, nil on the greeting panel (which + // names no one quest), the id on a detail panel. + assert!(s + .eval::>("return GetQuestDialogQuestId()") + .unwrap() + .is_none()); + s.set_quest(Some(crate::script::QuestState { + panel: crate::script::QuestPanel::Greeting, + quest_id: 0, + ..Default::default() + })); + assert!( + s.eval::>("return GetQuestDialogQuestId()") + .unwrap() + .is_none(), + "the greeting panel is a list and names none of its rows" + ); + s.set_quest(Some(crate::script::QuestState { + panel: crate::script::QuestPanel::Detail, + quest_id: 83, + ..Default::default() + })); + assert_eq!( + s.eval::>("return GetQuestDialogQuestId()") + .unwrap(), + Some(83) + ); + + // `CombatLogFlush()` is a no-op that must still exist: the capability is present (the + // writes are unbuffered), and a missing global makes `HasFeature` report it absent. + assert!(s.run("CombatLogFlush()").is_ok()); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); + } + + /// The trinket pair and `GetInventoryItemCooldown`, which is the **stock** verb this feed was + /// really for: it answered `(0, 0, 1)` unconditionally, so no worn trinket ever drew its + /// cooldown swirl on the character pane. + /// + /// The three verbs are asserted together because they must agree: one trinket, one cooldown, + /// whether it is asked for by slot, by id, by name, or through the stock paper-doll call. + #[test] + fn the_trinket_verbs_and_the_stock_doll_call_report_one_cooldown() { + let mut s = UiScript::new().unwrap(); + s.set_now(100.0); + let trinket = |item_id: u32, name: &str, legs: crate::script::CooldownLegsView| { + crate::script::InvSlotView { + item_id, + name: Some(name.to_string()), + use_spell_id: 11_392, + cooldown: legs.individual, + cooldown_legs: legs, + ..Default::default() + } + }; + let legs = crate::script::CooldownLegsView { + individual: Some((98_000, 120_000, true)), + category_id: 0, + category: None, + gcd_category_id: 0, + gcd: None, + }; + let mut slots: crate::script::InventorySlots = Default::default(); + // 13 and 14 are `INVSLOT_TRINKET1`/`2`, which is how the doll feed numbers them. + slots[13] = Some(trinket(13_444, "Hand of Justice", legs)); + slots[14] = Some(trinket(11_815, "Hand of Justice Too", Default::default())); + s.set_inventory_slots(slots); + + // By slot, by id and by name, all three reaching the same row. + for call in [ + "GetTrinketCooldown(1)", + "GetTrinketCooldown(13444)", + r#"GetTrinketCooldown("hand of justice")"#, + ] { + assert_eq!( + s.eval::<(i64, f64, i64, i64)>(&format!( + "local c = {call} \ + return c.isOnCooldown, c.individualRemainingMs, c.itemId, \ + c.itemActiveSpellId" + )) + .unwrap(), + (1, 118_000.0, 13_444, 11_392), + "{call}" + ); + } + // Slot 2's trinket has no record: a cold table, not the other slot's numbers. + assert_eq!( + s.eval::<(i64, i64)>("local c = GetTrinketCooldown(2) return c.isOnCooldown, c.itemId") + .unwrap(), + (0, 11_815) + ); + // An argument naming neither slot answers cold with no item at all. + for call in [ + "GetTrinketCooldown(999999)", + r#"GetTrinketCooldown("nope")"#, + ] { + assert_eq!( + s.eval::<(i64, i64)>(&format!("local c = {call} return c.isOnCooldown, c.itemId")) + .unwrap(), + (0, 0), + "{call}" + ); + } + + // The stock verb, off the same fed triple — seconds, not milliseconds, because that is + // what `CooldownFrame_SetTimer` takes. + assert_eq!( + s.eval::<(f64, f64, i64)>(r#"return GetInventoryItemCooldown("player", 13)"#) + .unwrap(), + (98.0, 120.0, 1) + ); + assert_eq!( + s.eval::<(f64, f64, i64)>(r#"return GetInventoryItemCooldown("player", 14)"#) + .unwrap(), + (0.0, 0.0, 1), + "a cold slot is still the triple, because the caller does arithmetic on it" + ); + assert_eq!( + s.eval::<(f64, f64, i64)>(r#"return GetInventoryItemCooldown("target", 13)"#) + .unwrap(), + (0.0, 0.0, 1), + "another unit's cooldowns are not on the wire" + ); + // `CooldownFrame_SetTimer`'s own arithmetic (`Cooldown.lua:3`), run against the verb: + // this is the number the swirl is drawn from, and it was always 0 before the feed. + assert_eq!( + s.eval::( + r#"local start, duration, enable = GetInventoryItemCooldown("player", 13) + if enable == 1 and duration > 0 then return (start + duration) - GetTime() end + return 0"# + ) + .unwrap(), + 118.0 + ); + } + /// `GetSpellRec` and `GetSpellRecField` over the `Spell.dbc` probe, including the seven /// names the corpus asks for that answered nil before it existed. /// diff --git a/crates/benilla-ui/src/script/cursor/doll.rs b/crates/benilla-ui/src/script/cursor/doll.rs index cd5e6cd6..b7c6f77d 100644 --- a/crates/benilla-ui/src/script/cursor/doll.rs +++ b/crates/benilla-ui/src/script/cursor/doll.rs @@ -246,6 +246,9 @@ mod tests { fn doll_slots() -> crate::script::InventorySlots { let mut slots: crate::script::InventorySlots = Default::default(); slots[1] = Some(InvSlotView { + cooldown: None, + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, already_bound: false, bar_placeable: true, @@ -266,6 +269,9 @@ mod tests { raw: Default::default(), }); slots[11] = Some(InvSlotView { + cooldown: None, + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, already_bound: false, bar_placeable: true, @@ -286,6 +292,9 @@ mod tests { raw: Default::default(), }); slots[19] = Some(InvSlotView { + cooldown: None, + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, already_bound: false, bar_placeable: true, @@ -460,6 +469,9 @@ mod tests { }); // An equipped bag in Bag0Slot (id 20): its equip_slots is the four bag slots. slots[20] = Some(InvSlotView { + cooldown: None, + cooldown_legs: Default::default(), + use_spell_id: 0, duration_ms: None, already_bound: false, bar_placeable: true, diff --git a/crates/benilla-ui/src/script/mod.rs b/crates/benilla-ui/src/script/mod.rs index 85be4c74..2d125b02 100644 --- a/crates/benilla-ui/src/script/mod.rs +++ b/crates/benilla-ui/src/script/mod.rs @@ -247,8 +247,8 @@ pub use model::{ }; pub(crate) use model::{Model, PendingTimer}; pub use model::{ - PlayerCacheEntry, SelfCast, SelfMovement, ServerClock, SpellModifierProbe, SpellRecFieldProbe, - SpellRecValue, SpellRecordProbe, + PlayerCacheEntry, SelfCast, SelfMovement, ServerClock, SpellModifierProbe, SpellRangeProbe, + SpellRecFieldProbe, SpellRecValue, SpellRecordProbe, }; pub use party::{PartyMemberInfo, PartyRequest, PartyState, RaidMemberInfo, SavedInstanceInfo}; pub use pet::{PetActionView, PetStats}; diff --git a/crates/benilla-ui/src/script/model.rs b/crates/benilla-ui/src/script/model.rs index bfcfb451..80786960 100644 --- a/crates/benilla-ui/src/script/model.rs +++ b/crates/benilla-ui/src/script/model.rs @@ -71,6 +71,14 @@ 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>; +/// One `SpellRange.dbc` row by its index: `(min, max, flags)`, for nampower's +/// `GetSpellRangeData(rangeIndex)`. `None` for an index no row occupies. +/// +/// 1.12's Lua reaches a range only through `SpellRangeFunc`-formatted tooltip text, never the two +/// yard numbers, and `C_Spell.IsSpellInRange` answers the *verdict* from the live usable map — so +/// the yards themselves have no other path out of this client. +pub type SpellRangeProbe = Box Option<(f32, f32, u32)>>; + /// One `Spell.dbc` column as nampower's `GetSpellRec`/`GetSpellRecField` hands it to Lua. /// /// The array variants are a **1-based Lua table of three**, which is not a guess: @@ -829,6 +837,7 @@ pub(crate) struct Model { pub(crate) spell_modifier_probe: Option, pub(crate) spell_rec_field_probe: Option, pub(crate) spell_record_probe: Option, + pub(crate) spell_range_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`). @@ -1733,6 +1742,7 @@ impl Model { spell_modifier_probe: None, spell_rec_field_probe: None, spell_record_probe: None, + spell_range_probe: None, spell_mechanic_probe: None, player_cache: HashMap::new(), player_cache_enabled: [true, true], diff --git a/crates/benilla-ui/src/script/quest.rs b/crates/benilla-ui/src/script/quest.rs index 1dec4432..579d4e43 100644 --- a/crates/benilla-ui/src/script/quest.rs +++ b/crates/benilla-ui/src/script/quest.rs @@ -83,12 +83,21 @@ pub struct QuestState { /// `GetQuestBackgroundMaterial`, which the app fills from an item or GameObject source, never /// from the wire; nil reads as "Parchment" (`QuestFrame.lua:598-604`). pub background_material: Option, + /// The quest id of whichever panel is live, `0` on the greeting panel and when no quest giver + /// is open: nampower's `GetQuestDialogQuestId()`. + /// + /// The wire carries it on all three quest panels — `SMSG_QUESTGIVER_QUEST_DETAILS`, + /// `_REQUEST_ITEMS` and `_OFFER_REWARD` each name the quest — and 1.12's own Lua carries it + /// nowhere: `GetTitleText` answers the title, and a title is not a key. The greeting panel is + /// a *list* of quests and names none of them, so `0` is the honest answer there. + pub quest_id: u32, } impl Default for QuestState { fn default() -> Self { QuestState { panel: QuestPanel::Detail, + quest_id: 0, greeting: String::new(), active_titles: Vec::new(), active_levels: Vec::new(), diff --git a/crates/benilla-ui/src/script/spellbook.rs b/crates/benilla-ui/src/script/spellbook.rs index aebc4577..6c4d8754 100644 --- a/crates/benilla-ui/src/script/spellbook.rs +++ b/crates/benilla-ui/src/script/spellbook.rs @@ -133,6 +133,11 @@ impl super::UiScript { /// Install nampower's `Spell.dbc` row readers, both built from the one name-to-column mapping /// in `benilla-app` (see [`super::SpellRecFieldProbe`]). + /// Install nampower's `SpellRange.dbc` reader. + pub fn set_spell_range_probe(&mut self, probe: super::SpellRangeProbe) { + self.model_mut().spell_range_probe = Some(probe); + } + pub fn set_spell_rec_probes( &mut self, field: super::SpellRecFieldProbe, diff --git a/crates/benilla-ui/src/script/tests/reference_surface.rs b/crates/benilla-ui/src/script/tests/reference_surface.rs index 0f8e7112..faaa2fd7 100644 --- a/crates/benilla-ui/src/script/tests/reference_surface.rs +++ b/crates/benilla-ui/src/script/tests/reference_surface.rs @@ -110,6 +110,11 @@ fn allowed_beyond_1_12() -> HashSet<&'static str> { // the rest answer nil rather than a plausible zero. DoiteAuras guards each non-name read. "GetSpellRecField", "GetSpellRec", + "GetSpellRangeData", + "SetLocalRaidTargetIndex", + "GetQuestLogQuestIds", + "GetQuestDialogQuestId", + "CombatLogFlush", // nampower v4's spellbook index helpers (script/compat/nampower.rs), all four of them the // fast paths DoiteAuras takes before falling back to a 200-slot `GetSpellName` scan per // lookup — it does one of those per tracked aura per condition evaluation. @@ -345,6 +350,8 @@ fn allowed_beyond_1_12() -> HashSet<&'static str> { "GetSpellIdCooldown", "GetItemIconTexture", "GetTrinkets", + "GetTrinketCooldown", + "UseTrinket", "PlayerIsMoving", "PlayerIsRooted", "PlayerIsSwimming", diff --git a/docs/ADDON_COMPAT.md b/docs/ADDON_COMPAT.md index f62c6a72..f67954e7 100644 --- a/docs/ADDON_COMPAT.md +++ b/docs/ADDON_COMPAT.md @@ -476,7 +476,7 @@ real when something else agrees — an addon calling it, or the mod's own docs. What it found that this file did not already have: **38 names in `nampower.dll` that grom's addons call**, on top of the families the nampower section above already tracks. Unported, and listed -rather than guessed at — `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, +rather than guessed at — `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CombatLogFlush`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `ChannelStopCastingNextTick`, `CombatLogFlush`, `CustomFileExists`, `EncryptPassword`, `ExecuteCustomLuaFile`, `FindPlayerItemSlot`, `GetBagItem`, `GetCastInfo`, `GetCurrentCastingInfo`, `GetItemIconTexture`, `GetItemIdCooldown`, @@ -573,13 +573,11 @@ config export, but they are a different kind of surface from a getter, and the r client that did not have them yesterday is not to grow them silently. `CombatLogFlush` is here only because it is in the same family — it would flush our own `WoWCombatLog.txt`, which is benign. -**Needs a feed this client does not have, with what it would take (4).** +**Needs a feed this client does not have, with what it would take (2).** | 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` | | `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 | **Not ours to add (3).** `QueueSpellByName`, `QueueScript` and `ChannelStopCastingNextTick` are the spell queue itself, which this client deliberately does not have (see the nampower rows above: @@ -763,6 +761,53 @@ runs — still answers a learnt spell's name and rank, which is what `GetSpellNa beside it does from the same `spell_display`. Every other column is a DBC row and has nowhere else to come from. +### The trinket cooldown — and a stock verb that had been answering "cold" since it was written + +**Built: `GetTrinketCooldown`, `UseTrinket`, `GetSpellRangeData`, `SetLocalRaidTargetIndex`, +`GetQuestLogQuestIds`, `GetQuestDialogQuestId`, `CombatLogFlush`.** + +The find on the way through is not a nampower verb. **`GetInventoryItemCooldown` is a stock 1.12 +verb and it answered `(0, 0, 1)` unconditionally** — the shape was right and the number was always +"cold", so nothing errored and no worn trinket ever drew its cooldown swirl on the character pane. +Stock `PaperDollFrame.lua:691` calls it on every slot update. Feeding an equipped slot's cooldown +for `GetTrinketCooldown` fixes that one too, off the same column, and the test runs +`CooldownFrame_SetTimer`'s own arithmetic against it. + +The feed is `ui_char::slot_view`, keyed on the (use spell, item entry) pair exactly as the bag +feed's is — `Cooldowns::info` matches an item record on both — so one trinket reads the same +cooldown from the doll and from a bag. + +**The risk in that change was the gate, and the first test for it was vacuous.** `feed_char` is +behind a change gate, and no descriptor moves when a cooldown is armed, so `Cooldowns::feed_epoch` +had to become a gate input or the swirl would appear only on the next frame something unrelated +changed. The first test passed with that input deleted: `run_system_once` builds a fresh system +each call, so the gate's `Local` memo resets and `vm_reset` opens the gate every time. A test for a +gate input has to run the system through the schedule (`add_systems` + `update`), or it proves +nothing. Rewritten that way, it fails with the input removed — verified by probe. + +Two also worth recording: + +- **`GetSpellRangeData(rangeIndex)` answers three values, not a table.** Every table-returning verb + in this DLL has its key names in the string pool; this row's are not there, so a flat row is what + it answers. `flags` bit 0 is the melee family, where the engine takes the combat-reach sum + instead of the authored pair — a caller that ignores the bit reads 0..5 for every melee ability, + which is why the binding says so. +- **`CombatLogFlush()` is a no-op, deliberately defined.** Upstream buffers its combat-log writes + and this empties the buffer, so what an addon asks for is "my lines are on disk now". + `ui_chat/logging.rs` writes each line to an **unbuffered** `std::fs::File` as the window shows + it, so that postcondition already holds at all times. Defining it is the honest answer: the + capability is present, and a missing global makes `HasFeature` report it absent when it is not. + +**Still out, now with the field lists that make each a decision rather than a shrug:** + +| Name | Why not, and what it would take | +|---|---| +| `GetUnitData(unitToken, [copy])` / `GetUnitField`'s full surface | The names are read: 80 of them at `0x92620` — `charm summon charmedBy summonedBy createdBy persuaded channelObject health power1..5 maxHealth maxPower1..5 level factionTemplate bytes0 auraState baseAttackTime … aura auraFlags auraLevels auraApplications powerCostModifier powerCostMultiplier`, plus five computed extras (`guid status zoneId posX posY`). They are the **raw descriptor**, which lives app-side in `ObjectStore` and not in the VM's resolved `UnitState`, so the full surface is a live per-unit feed. `GetUnitField` already answers the only two names anything reads (`aura`, `auraApplications`) and `GetUnitData` has **zero** corpus call sites, so the feed would be built for nothing. The partial alternative is worse: a caller reading `bytes0` and getting nil while `health` works cannot tell which names are real | +| `GetRaidTargets()` | The reference keeps a raid-target **board** (`0xb712a8`); this client feeds the mark per unit (`UnitState::raid_target`), which `GetRaidTargetIndex` answers from. A scan over currently-named tokens would silently omit a marked unit no token names — plausible and wrong, which is worse than absent. Zero corpus call sites | +| `DisenchantAll(itemIdOrName \| quality, [includeSoulbound])` | **Queue machinery, not a getter.** It picks the next matching bag item, casts Disenchant, waits for the resolution, repeats — a server-paced sequencer with aborts on failure and on a bag change, which is the same thing `QueueSpellByName` is and which this client deliberately does not have. pfUI binds `/disenchantall` to it (`modules/nampower.lua:140`), so the *user-visible* cost is one slash command. Say the word and the sequencer is its own piece of work; it is not a binding | +| `GetSpellPower(school)` | The player's spell damage bonus by school, which needs the `PLAYER_FIELD_MOD_DAMAGE_DONE_POS` band fed per school. Nothing in the corpus calls it — the "7 call sites" an earlier sweep counted are BetterCharacterStats' own method of the same name | +| `StartItemExport` | Writes a file of its own; it belongs with the seven withheld, not with the getters | + ### The whole registered list, accounted for With the registration table read (the section above), the surface is no longer a count of what a @@ -779,10 +824,10 @@ comm -23 /tmp/np /tmp/ours | Bucket | Count | Names | |---|---|---| -| **Built** | 38 | `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CastSpellByName`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `FindPlayerItemSlot`, `GetBagItem`, `GetBagItems`, `GetEquippedItem`, `GetEquippedItems`, `GetItemIconTexture`, `GetItemIdCooldown`, `GetItemLevel`, `GetItemStats`, `GetItemStatsField`, `GetNampowerVersion`, `GetPlayerAuraDuration`, `GetSpellDuration`, `GetSpellIconTexture`, `GetSpellIdCooldown`, `GetSpellIdForName`, `GetSpellModifiers`, `GetSpellNameAndRankForId`, `GetSpellRec`, `GetSpellRecField`, `GetSpellSlotTypeIdForName`, `GetTrinkets`, `GetUnitField`, `GetUnitGUID`, `IsAuraHidden`, `IsSpellInRange`, `IsSpellUsable`, `PlayerIsMoving`, `PlayerIsRooted`, `PlayerIsSwimming`, `SetMouseoverUnit`, `UseItemIdOrName` | +| **Built** | 45 | `CancelPlayerAuraSlot`, `CancelPlayerAuraSpellId`, `CastSpellByName`, `CastSpellByNameNoQueue`, `CastSpellNoQueue`, `FindPlayerItemSlot`, `GetBagItem`, `GetBagItems`, `GetEquippedItem`, `GetEquippedItems`, `GetItemIconTexture`, `GetItemIdCooldown`, `GetItemLevel`, `GetItemStats`, `GetItemStatsField`, `GetNampowerVersion`, `GetPlayerAuraDuration`, `GetQuestDialogQuestId`, `GetQuestLogQuestIds`, `GetSpellDuration`, `GetSpellIconTexture`, `GetSpellIdCooldown`, `GetSpellIdForName`, `GetSpellModifiers`, `GetSpellNameAndRankForId`, `GetSpellRangeData`, `GetSpellRec`, `GetSpellRecField`, `GetSpellSlotTypeIdForName`, `GetTrinketCooldown`, `GetTrinkets`, `GetUnitField`, `GetUnitGUID`, `IsAuraHidden`, `IsSpellInRange`, `IsSpellUsable`, `PlayerIsMoving`, `PlayerIsRooted`, `PlayerIsSwimming`, `SetLocalRaidTargetIndex`, `SetMouseoverUnit`, `UseItemIdOrName`, `UseTrinket` | | **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** | 12 | `DisenchantAll` (5 call sites outside the API layer), then `GetSpellPower`, `GetTrinketCooldown`, `UseTrinket`, `GetUnitData`, `GetSpellRangeData`, `CombatLogFlush`, `GetRaidTargets`, `SetLocalRaidTargetIndex`, `GetQuestLogQuestIds`, `GetQuestDialogQuestId`, `StartItemExport` (0 each — present for completeness, not need) | +| **Measured out, with the reason at each** | 5 | `DisenchantAll` (queue machinery — see the table above), `GetUnitData`, `GetRaidTargets`, `GetSpellPower`, `StartItemExport` | 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.