From 80b08486287bce1f32d0cf24ed54a1913b3e7e9e Mon Sep 17 00:00:00 2001 From: goran2711 Date: Thu, 8 Oct 2026 01:16:27 +0100 Subject: [PATCH] fix: trainer services show their spell's description, as 1.12.1 does (#803) - capture: trainer scenarios shoot the service description field (#793) - fix: trainer services show their spell's description, as 1.12.1 does (#793) - review: reuse the caster closure, cut repeated comments, pin the profession rows - trainer: the pet's skill value is the tooltip's one helper, not a copy - trainer: the recipe row's real-data test reads a product that has text Co-Authored-By: Claude Opus 5.5 Co-Authored-By: Sam --- crates/benilla-app/src/capture/fixtures.rs | 128 ++++- crates/benilla-app/src/capture/mod.rs | 4 +- crates/benilla-app/src/capture/scenarios.rs | 46 ++ crates/benilla-app/src/spell/mod.rs | 9 + .../benilla-app/src/ui_tooltip/spell_feed.rs | 15 +- crates/benilla-app/src/ui_trainer/law.rs | 78 ++- crates/benilla-app/src/ui_trainer/mod.rs | 86 +++- crates/benilla-app/src/ui_trainer/tests.rs | 457 +++++++++++++++++- 8 files changed, 766 insertions(+), 57 deletions(-) diff --git a/crates/benilla-app/src/capture/fixtures.rs b/crates/benilla-app/src/capture/fixtures.rs index 758659c5..6ae24160 100644 --- a/crates/benilla-app/src/capture/fixtures.rs +++ b/crates/benilla-app/src/capture/fixtures.rs @@ -1,6 +1,7 @@ //! The capture UI fixtures: each arm seeds one window's synthetic but realistic state. use super::*; +use benilla_protocol::messages::trainer_spell_state; /// The `vplates` fixture's wolf, the reference screenshot's subject (vmangos `creature_template` /// 69 "Timber Wolf": level 2, faction template 32, display 604), at the scenario's look point. @@ -23,6 +24,66 @@ const CHEST_GUID: u64 = (0xF110u64 << 48) | 0x744; /// 45°), so `front` shows the lit side and `rear` the unlit one. const SUBJECT_YAW: f32 = 2.36; +/// One trainer fixture's canned seed: the trainer's identity, its `SMSG_TRAINER_LIST` type, its +/// greeting, the character's hearthstone bind area, and the wire rows +/// `(spell, state, copper cost, required level)`. +struct TrainerSeed { + name: &'static str, + subname: &'static str, + trainer_type: u32, + greeting: &'static str, + bind: u32, + rows: &'static [(u32, u8, u32, u8)], +} + +/// Llane Beshere (entry 911), Northshire's warrior trainer: the rank-1 learn wrappers a vmangos +/// trainer lists (`1605` teaches Heroic Strike 78 and so on), so each row's name and rank mirror +/// the ability while its description is the taught spell's own text. +const TRAINER_WARRIOR: TrainerSeed = TrainerSeed { + name: "Llane Beshere", + subname: "Warrior Trainer", + trainer_type: 0, + greeting: "I can train you in the ways of the warrior.", + bind: 9, // Northshire Valley, where this character's hearthstone is + rows: &[ + (1605, trainer_spell_state::GREEN, 10, 1), // Heroic Strike Rank 1 + (1738, trainer_spell_state::GREEN, 100, 4), // Charge Rank 1 + (1423, trainer_spell_state::GREEN, 100, 4), // Rend Rank 1 + (6674, trainer_spell_state::GREEN, 200, 6), // Battle Shout Rank 1 + (1716, trainer_spell_state::GREEN, 300, 8), // Hamstring Rank 1 + (3128, trainer_spell_state::GREEN, 200, 8), // Parry + ], +}; + +/// A shaman trainer's Astral Recall (wrapper 1352 teaches 556). One service only: the stock +/// window's `ClassTrainer_SelectFirstLearnableSkill` selects row 2, so the row the shot is about +/// must be the first service of the first group. +const TRAINER_SHAMAN: TrainerSeed = TrainerSeed { + name: "Sian'tsu", + subname: "Shaman Trainer", + trainer_type: 0, + greeting: "The spirits are strong within you, shaman.", + bind: 362, // Razor Hill, a Durotar hearthstone + rows: &[(1352, trainer_spell_state::GREEN, 4000, 30)], +}; + +/// Bengus Deepforge, Ironforge's blacksmithing trainer, with the two row shapes a profession +/// trainer lists: the profession-learn wrapper (2020 teaches 2018, whose own text the row shows) +/// and a recipe (7820 teaches 7818, whose text is the description of the Silver Rod it makes). +/// The learn row sorts into group 1 (`Effect` 44 `SKILL_STEP`) ahead of the recipe's group 2 +/// (`0x4d77b6`), so the stock window's row-2 selection lands on it. +const TRAINER_BLACKSMITH: TrainerSeed = TrainerSeed { + name: "Bengus Deepforge", + subname: "Blacksmithing Trainer", + trainer_type: 2, // a tradeskill trainer + greeting: "The forge shapes what the mine provides.", + bind: 1537, // Ironforge + rows: &[ + (2020, trainer_spell_state::GREEN, 9, 5), // Apprentice Blacksmith + (7820, trainer_spell_state::GREEN, 90, 0), // Silver Rod + ], +}; + /// Seeds the fixture window's state once the scene is resident; the real feeds push it into the VM /// during the settle window as live wire data would. Icons resolve through the offline /// `ItemDisplayCatalog`, and names go straight into the caches. @@ -35,9 +96,14 @@ pub(super) fn seed_ui_fixture( mut quest: ResMut, mut quest_log: ResMut, mut loot: ResMut, + mut trainer: ResMut, // One param under Bevy's 16-param cap: item objects are entities in the index, spawned as - // the wire does. - store: (ResMut, ResMut), + // the wire does; the hearthstone bind (`SMSG_BINDPOINTUPDATE`) rides along for the `$z` token. + store: ( + ResMut, + ResMut, + ResMut, + ), mut names: ResMut, icons: Option>, mut script: Option>, @@ -51,7 +117,7 @@ pub(super) fn seed_ui_fixture( ResMut, ), ) { - let (mut items, mut index) = store; + let (mut items, mut index, mut home_bind) = store; // A glue-screen capture has no world scenario, and no glue screen opens a UI fixture. let Some(scenario) = ctx.scenario else { return; @@ -206,6 +272,62 @@ pub(super) fn seed_ui_fixture( }); } } + UiFixture::Trainer(list) => { + // The seed's rows are the wire list: its copper costs, level gates and the states the + // server computed, in wire order. + use benilla_protocol::messages::TrainerSpell; + let seed = match list { + TrainerList::Warrior => &TRAINER_WARRIOR, + TrainerList::Shaman => &TRAINER_SHAMAN, + TrainerList::Blacksmith => &TRAINER_BLACKSMITH, + }; + names.insert_creature( + NPC_ENTRY, + Some(crate::names::CreatureRecord { + name: seed.name.into(), + subname: Some(seed.subname.into()), + creature_type: 7, + pet_family: 0, + rank: 0, + type_flags: 0, + civilian: false, + racial_leader: false, + display_id: 0, + }), + ); + home_bind.0 = Some(seed.bind); + let Some(script) = script.as_mut() else { + return; + }; + // The window's title and portrait read `UnitName("npc")`; no session seats it here. + script.set_unit( + "npc", + Some(benilla_ui::script::UnitState { + exists: true, + name: Some(seed.name.into()), + level: 12, + ..Default::default() + }), + ); + // A purse large enough for every cost above, so the money frame renders white. + script.set_money(10_000); + let services: Vec = seed + .rows + .iter() + .map(|&(spell, state, cost, req_level)| TrainerSpell { + spell, + state, + cost, + can_learn_primary_prof: false, + is_primary_prof_first_rank: false, + req_level, + req_skill: 0, + req_skill_value: 0, + req_spells: [0; 3], + }) + .collect(); + trainer.open(NPC_GUID, seed.trainer_type, services, seed.greeting.into()); + } UiFixture::Gossip => { names.insert_creature( NPC_ENTRY, diff --git a/crates/benilla-app/src/capture/mod.rs b/crates/benilla-app/src/capture/mod.rs index 07a4da74..cc964f8a 100644 --- a/crates/benilla-app/src/capture/mod.rs +++ b/crates/benilla-app/src/capture/mod.rs @@ -117,7 +117,9 @@ pub(crate) use probes::{ }; pub(crate) use scenarios::ui_opted_in; use scenarios::GlueScreen; -use scenarios::{Scenario, SubjectKind, UiFixture, GLUE_SCENARIOS, GROUND_EYE, SCENARIOS}; +use scenarios::{ + Scenario, SubjectKind, TrainerList, UiFixture, GLUE_SCENARIOS, GROUND_EYE, SCENARIOS, +}; pub(crate) mod fxview; // The scripted aim, swim-pitch and camera-rig drivers order themselves before `player::control`: diff --git a/crates/benilla-app/src/capture/scenarios.rs b/crates/benilla-app/src/capture/scenarios.rs index db3258f8..117495c0 100644 --- a/crates/benilla-app/src/capture/scenarios.rs +++ b/crates/benilla-app/src/capture/scenarios.rs @@ -18,6 +18,20 @@ pub(super) struct Scenario { pub(super) ui: Option, } +/// Which trainer [`UiFixture::Trainer`] captures. +#[derive(Clone, Copy, PartialEq, Eq)] +pub(super) enum TrainerList { + /// Llane Beshere (entry 911), Northshire's warrior trainer: the rank-1 learn wrappers a + /// vmangos trainer lists, so the description is the taught ability's own text. + Warrior, + /// A shaman trainer's Astral Recall row (wrapper 1352 teaches 556), the shipped spell whose + /// description carries the `$z` token, so the shot pins the bind area's name too. + Shaman, + /// A profession trainer's two row shapes: the learn wrapper whose text is the taught + /// profession's, and a recipe whose text is its product's. + Blacksmith, +} + /// A UI window opened with synthetic state that mirrors what the server sends, so the capture /// runs the real feed, VM, extract and render chain. #[derive(Clone, Copy, PartialEq)] @@ -29,6 +43,9 @@ pub(super) enum UiFixture { Merchant, Gossip, Quest, + /// The class trainer window fed as `SMSG_TRAINER_LIST` would: the first learnable service + /// selected, its description expanded from `Spell.dbc`. + Trainer(TrainerList), /// The bank window fed through a synthetic self-player descriptor: `PLAYER_FIELD_BANK_SLOT` /// guids, a bank bag, the purchased count in `PLAYER_BYTES_2` byte 2, and coinage. Bank, @@ -936,6 +953,35 @@ pub(super) const ON_DEMAND: &[Scenario] = &[ minute: 720, ui: Some(UiFixture::NameRank), }, + // The warrior trainer's window over the noon ground view, a learnable service selected. + Scenario { + name: "ui-trainer", + map: Some(MAP_AZEROTH), + eye: GROUND_EYE, + look: GROUND_LOOK, + minute: 720, + ui: Some(UiFixture::Trainer(TrainerList::Warrior)), + }, + // A shaman trainer's window: the one shipped service whose description carries `$z`, over a + // bind point, so the shot pins the token against `AreaTable`. + Scenario { + name: "ui-trainer-astral", + map: Some(MAP_AZEROTH), + eye: GROUND_EYE, + look: GROUND_LOOK, + minute: 720, + ui: Some(UiFixture::Trainer(TrainerList::Shaman)), + }, + // A blacksmithing trainer's window: the profession-learn row, whose text is the taught + // profession's, with a recipe row beneath it. + Scenario { + name: "ui-trainer-blacksmith", + map: Some(MAP_AZEROTH), + eye: GROUND_EYE, + look: GROUND_LOOK, + minute: 720, + ui: Some(UiFixture::Trainer(TrainerList::Blacksmith)), + }, ]; /// The `name-close` subject: the `name-water` wolf's name, orbited at any distance. World text is diff --git a/crates/benilla-app/src/spell/mod.rs b/crates/benilla-app/src/spell/mod.rs index ef851862..c05e564f 100644 --- a/crates/benilla-app/src/spell/mod.rs +++ b/crates/benilla-app/src/spell/mod.rs @@ -130,6 +130,15 @@ pub(crate) fn spell_skill_value( skill_line_value(me, line) } +/// A pet's `[vtbl+0xa8]`, `CGUnit`'s `0x60cd80`: `UNIT_FIELD_LEVEL × 5`, which +/// [`benilla_formats::SpellDisplay::skill_level`] caps at `maxLevel × 5` and divides by 5 as +/// `0x60cdb2`-`0x60cdb9` and `0x6e3195` do. No unit reads 0 (`0x6e31a4`). +pub(crate) fn pet_skill_value(pet: Option<&benilla_protocol::ObjectFields>) -> u32 { + pet.and_then(|p| p.unit_level()) + .unwrap_or(0) + .saturating_mul(5) +} + /// The player's skill in `line`, [`spell_skill_value`]'s second half: 0 without a player or a /// slot for it. pub(crate) fn skill_line_value(me: Option<&crate::net::ObjectStore>, line: u32) -> u32 { diff --git a/crates/benilla-app/src/ui_tooltip/spell_feed.rs b/crates/benilla-app/src/ui_tooltip/spell_feed.rs index d19c6125..234afacc 100644 --- a/crates/benilla-app/src/ui_tooltip/spell_feed.rs +++ b/crates/benilla-app/src/ui_tooltip/spell_feed.rs @@ -31,17 +31,6 @@ pub(super) enum ViewCaster<'a> { Pet(Option<&'a benilla_protocol::ObjectFields>), } -impl ViewCaster<'_> { - /// The pet's `[vtbl+0xa8]`, `CGUnit`'s `0x60cd80`: `UNIT_FIELD_LEVEL × 5`, which - /// [`benilla_formats::SpellDisplay::skill_level`] caps at `maxLevel × 5` and divides by 5 as - /// `0x60cdb2`-`0x60cdb9` and `0x6e3195` do. No unit reads 0 (`0x6e31a4`). - fn pet_skill_value(pet: Option<&benilla_protocol::ObjectFields>) -> u32 { - pet.and_then(|p| p.unit_level()) - .unwrap_or(0) - .saturating_mul(5) - } -} - /// The caster-dependent inputs of a spell view: the worn set (`0x5f0c50`), the bags, the form and /// the bind point `$z` names. pub(super) struct ViewCtx<'a, 'w, 's> { @@ -99,7 +88,7 @@ pub(super) fn build_view( line.map_or(0, |line| crate::spell::skill_line_value(vctx.store, line)) } // The pet's level is one of the pet's own inputs. - ViewCaster::Pet(pet) => ViewCaster::pet_skill_value(pet), + ViewCaster::Pet(pet) => crate::spell::pet_skill_value(pet), }; // A cross-referenced spell's own family mask decides its modifiers. let lookup = |id| { @@ -118,7 +107,7 @@ pub(super) fn build_view( vctx.store.and_then(|s| s.0.unit_gender()).unwrap_or(0) }; // `0x6e3130` for this spell: the level the cost's and the cast time's per-level terms read. - let pet_level = |pet| d.skill_level(ViewCaster::pet_skill_value(pet)); + let pet_level = |pet| d.skill_level(crate::spell::pet_skill_value(pet)); let ctx = benilla_formats::TokenContext { durations: &spells.durations, radii: &spells.radii, diff --git a/crates/benilla-app/src/ui_trainer/law.rs b/crates/benilla-app/src/ui_trainer/law.rs index 33539537..250a844c 100644 --- a/crates/benilla-app/src/ui_trainer/law.rs +++ b/crates/benilla-app/src/ui_trainer/law.rs @@ -4,10 +4,11 @@ //! not unify them. use benilla_formats::{ - SkillLineCatalog, SpellCatalog, SPELL_ATTR_IS_TRADESKILL, SPELL_EFFECT_CREATE_ITEM, - SPELL_EFFECT_LEARN_PET_SPELL, SPELL_EFFECT_LEARN_SPELL, SPELL_EFFECT_SKILL_STEP, + substitute, SkillLineCatalog, SpellCatalog, TokenContext, SPELL_ATTR_IS_TRADESKILL, + SPELL_EFFECT_CREATE_ITEM, SPELL_EFFECT_LEARN_PET_SPELL, SPELL_EFFECT_LEARN_SPELL, + SPELL_EFFECT_SKILL_STEP, }; -use benilla_protocol::messages::trainer_spell_state; +use benilla_protocol::messages::{trainer_spell_state, TrainerSpell}; use benilla_ui::script::{TrainerServiceCategory, TrainerTooltip, TRAINER_GROUP_KNOWN}; use crate::entities::ItemDisplays; @@ -92,6 +93,77 @@ pub(super) fn service_tooltip(wire_spell: u32, spells: &SpellCatalog) -> Trainer wire_only } +/// The `$`-token contexts a service description expands over, picked per row by the caster the +/// expander's level terms read (`0x6e3130`'s selector): the active player's, or the pet's, which +/// a `LEARN_PET_SPELL` wrapper selects. +pub(super) struct ServiceText<'a> { + pub(super) player: &'a TokenContext<'a>, + pub(super) pet: &'a TokenContext<'a>, +} + +/// `GetTrainerServiceDescription 0x4d9b40`: the text the stock window shows under the service's +/// cost — `Spell.dbc`'s Description column with its `$`-tokens expanded (`0x5075f0` at +/// `0x4d9d6f`). Which record is expanded depends on the row: the wire spell's own text when it +/// has one (`0x4d9c32`), else the spell the first learn effect teaches — `EffectTriggerSpell` of +/// the first `LEARN_SPELL`/`LEARN_PET_SPELL` slot, whether or not its trigger resolves +/// (`0x4d9bdd`-`0x4d9c18`) — with that spell's own text, else, for a tradeskill recipe, the +/// description of the item the taught spell creates (`0x4d9cd0`-`0x4d9d22`, returned verbatim, +/// never expanded). A used row at a mount trainer skips its own text (`0x4d9c29`), and every +/// failure falls back to the wire text again (`0x4d9d3b`, probing slot 0 alone for the caster). +/// The expansion itself is the spell tooltip's description call (`0x52f717`): level override 0, +/// the Description column, and the modifiers applied. +pub(super) fn service_description( + wire: &TrainerSpell, + trainer_type: u32, + spells: &SpellCatalog, + items: &Items, + commands: &NetCommands, + text: &ServiceText, +) -> String { + let Some(d) = spells.get(wire.spell) else { + return String::new(); + }; + let learn = (0..3).find(|&i| { + matches!( + d.effects[i], + SPELL_EFFECT_LEARN_SPELL | SPELL_EFFECT_LEARN_PET_SPELL + ) + }); + let caster = |slot: usize| { + if d.effects[slot] == SPELL_EFFECT_LEARN_PET_SPELL { + text.pet + } else { + text.player + } + }; + let wire_text = d.description.as_deref().filter(|t| !t.is_empty()); + if !(trainer_type == TRAINER_TYPE_MOUNT && wire.state == trainer_spell_state::GRAY) { + if let Some(t) = wire_text { + return substitute(t, d, learn.map_or(text.player, caster)); + } + } + if let Some(slot) = learn { + if let Some(taught) = spells.get(d.effect_trigger_spell[slot]) { + if let Some(t) = taught.description.as_deref().filter(|t| !t.is_empty()) { + return substitute(t, taught, caster(slot)); + } + // The recipe's product carries the only text a `CREATE_ITEM` spell has. + if taught.attributes & SPELL_ATTR_IS_TRADESKILL != 0 + && taught.effects[0] == SPELL_EFFECT_CREATE_ITEM + { + if let Some(t) = items + .template(taught.effect_item_type[0], 0, commands) + .map(|item| item.description.clone()) + .filter(|t| !t.is_empty()) + { + return t; + } + } + } + } + wire_text.map_or_else(String::new, |t| substitute(t, d, caster(0))) +} + /// The list builder `0x4d7560`'s group key (`0x4d7786`). Type 2 resolves no skill line, so no row /// drops: 1 when the wire spell steps a skill (`0x4d77b6`), else 2, labelled from the table at /// `0x807520`. Type 1 puts a known service in the signed `-1` group (`0x4d77e8`). The rest key on diff --git a/crates/benilla-app/src/ui_trainer/mod.rs b/crates/benilla-app/src/ui_trainer/mod.rs index 011fc598..90143f3c 100644 --- a/crates/benilla-app/src/ui_trainer/mod.rs +++ b/crates/benilla-app/src/ui_trainer/mod.rs @@ -5,7 +5,7 @@ use std::collections::BTreeSet; -use benilla_formats::{SkillLineCatalog, SpellCatalog}; +use benilla_formats::{SkillLineCatalog, SpellCatalog, TokenContext}; use benilla_protocol::messages::TrainerSpell; use bevy::prelude::*; @@ -163,6 +163,7 @@ fn resolve_service( icons: Option<&ItemDisplays>, items: &Items, commands: &NetCommands, + text: &ServiceText, // The VM's own `GlobalStrings.lua`, for the group header labels. get: &dyn Fn(&str) -> Option, ) -> TrainerService { @@ -224,9 +225,7 @@ fn resolve_service( subtext: display.and_then(|d| d.rank.clone()), // Over the wire spell, like the name and subtext ([`service_icon`]). texture: service_icon(wire.spell, trainer_type, spells, icons, items, commands), - // Empty: the 1.12 `GetTrainerServiceDescription` returns `Spell.dbc`'s Description with - // its `$s1`/`$o1`/`$d`/`$a1` tokens substituted, and that substitution is not built. - description: String::new(), + description: service_description(wire, trainer_type, spells, items, commands, text), cost: wire.cost, prof_first_rank: wire.is_primary_prof_first_rank, category: cat, @@ -249,6 +248,7 @@ fn snapshot( icons: Option<&ItemDisplays>, items: &Items, commands: &NetCommands, + text: &ServiceText, get: &dyn Fn(&str) -> Option, ) -> Option { open.trainer?; @@ -268,6 +268,7 @@ fn snapshot( icons, items, commands, + text, get, ) }) @@ -288,6 +289,16 @@ pub(crate) struct ReEvalInputs<'w, 's> { edges: MessageReader<'w, 's, FieldChanged>, } +/// The resources a service description's `$`-tokens read beside the caster's descriptor: the +/// spell-modifier tables and the bind point `$z` names, the same two reads the item and spell +/// tooltip feeds make. +#[derive(SystemParam)] +pub(crate) struct DescriptionReads<'w> { + spell_mods: Res<'w, crate::spell::SpellModifiers>, + home_bind: Option>, + area_names: Option>, +} + /// Push the current trainer into the VM and fire its events on a change. Another trainer while /// open is a close then an open: `ShowUIPanel` returns early on a visible frame. #[allow(clippy::too_many_arguments)] // one Bevy system's full input set @@ -299,6 +310,7 @@ pub(crate) fn feed_trainer( spells: Option>, skill_lines: Option>, mut re_eval: ReEvalInputs, + desc: DescriptionReads, // A tradeskill row shows its created item's icon: the template cache and `ItemDisplayInfo.dbc`. icons: Option>, items: Res, @@ -416,23 +428,63 @@ pub(crate) fn feed_trainer( &player, ); } - let fresh = snapshot( - &open, - &spells.catalog, - Some(&skill_lines.catalog), - &actions.spells, - icons.as_deref(), - &items, - &commands, - &|key: &str| { + // The spell tooltip's description reads (`0x52f717`), and `0x6e3159`'s charm-else-summon for + // a `LEARN_PET_SPELL` row. + let store = self_player.map(|(_, store)| store); + let pet = store + .and_then(|s| s.0.unit_pet_guid()) + .and_then(|guid| re_eval.index.0.get(&guid).copied()) + .and_then(|entity| re_eval.stores.get(entity).ok()); + let fresh = { + let gender = store.and_then(|s| s.0.unit_gender()).unwrap_or(0); + let home_area: Option<&str> = desc + .home_bind + .as_deref() + .and_then(|b| b.0) + .and_then(|id| desc.area_names.as_deref()?.0.resolve(id as i32)); + let player_skill = + |id: u32| crate::spell::spell_skill_value(store, Some(&skill_lines.catalog), id); + let pet_skill = |_: u32| crate::spell::pet_skill_value(pet.map(|p| &p.0)); + let get = |key: &str| { script .lua() .globals() .get::(key) .ok() .filter(|t| !t.is_empty()) - }, - ); + }; + let ctx = TokenContext { + durations: &spells.durations, + radii: &spells.radii, + ranges: Some(&spells.ranges), + skill: &player_skill, + lookup: &|id| spells.catalog.get(id), + mods: Some(&*desc.spell_mods), + unmodified_points: false, + gender: &|| gender, + home_area: &|| home_area, + global: &get, + printf: &crate::ui_script::token_printf, + }; + let pet_ctx = TokenContext { + skill: &pet_skill, + ..ctx + }; + snapshot( + &open, + &spells.catalog, + Some(&skill_lines.catalog), + &actions.spells, + icons.as_deref(), + &items, + &commands, + &ServiceText { + player: &ctx, + pet: &pet_ctx, + }, + &get, + ) + }; // A spell subject's view must be in the store before the detail icon is hovered. let mut fresh_tooltip_subjects = TrainerTooltipSubjects::default(); for service in fresh.iter().flat_map(|state| &state.services) { @@ -528,7 +580,9 @@ fn drain_trainer( } mod law; -use law::{category, service_group, service_icon, service_tooltip}; +use law::{ + category, service_description, service_group, service_icon, service_tooltip, ServiceText, +}; #[cfg(test)] mod tests; diff --git a/crates/benilla-app/src/ui_trainer/tests.rs b/crates/benilla-app/src/ui_trainer/tests.rs index 1bcb7c0f..a86ed923 100644 --- a/crates/benilla-app/src/ui_trainer/tests.rs +++ b/crates/benilla-app/src/ui_trainer/tests.rs @@ -27,6 +27,43 @@ fn probe_strings(key: &str) -> Option { } } +/// Run `f` with the description contexts over the test catalogs: the casters' skill values +/// (`level × 5` for a pet) and the bind point `$z` names. +fn with_text( + spells: &SpellCatalog, + player_skill: u32, + pet_skill: u32, + home: Option<&str>, + f: impl FnOnce(&ServiceText) -> R, +) -> R { + let durations = benilla_formats::SpellDurationCatalog::default(); + let radii = benilla_formats::SpellRadiusCatalog::default(); + let lookup = |id: u32| spells.get(id); + let skill_of = |_: u32| player_skill; + let pet_of = |_: u32| pet_skill; + let ctx = TokenContext { + durations: &durations, + radii: &radii, + ranges: None, + skill: &skill_of, + lookup: &lookup, + mods: None, + unmodified_points: false, + gender: &|| 0, + home_area: &|| home, + global: &probe_strings, + printf: &crate::ui_script::token_printf, + }; + let pet_ctx = TokenContext { + skill: &pet_of, + ..ctx + }; + f(&ServiceText { + player: &ctx, + pet: &pet_ctx, + }) +} + /// [`resolve_service`] with no icon sources. fn resolve_service( wire: &TrainerSpell, @@ -36,17 +73,20 @@ fn resolve_service( known: &BTreeSet, ) -> TrainerService { let deps = Deps::new(); - super::resolve_service( - wire, - trainer_type, - spells, - skill_lines, - known, - None, - &deps.items, - &deps.commands, - &probe_strings, - ) + with_text(spells, 0, 0, None, |text| { + super::resolve_service( + wire, + trainer_type, + spells, + skill_lines, + known, + None, + &deps.items, + &deps.commands, + text, + &probe_strings, + ) + }) } fn wire(spell: u32, state: u8, cost: u32, req_level: u8, req_skill: u32) -> TrainerSpell { @@ -66,16 +106,19 @@ fn wire(spell: u32, state: u8, cost: u32, req_level: u8, req_skill: u32) -> Trai /// [`snapshot`] with no icon sources. fn snap(open: &TrainerOpen, spells: &SpellCatalog) -> Option { let deps = Deps::new(); - snapshot( - open, - spells, - None, - &BTreeSet::new(), - None, - &deps.items, - &deps.commands, - &probe_strings, - ) + with_text(spells, 0, 0, None, |text| { + snapshot( + open, + spells, + None, + &BTreeSet::new(), + None, + &deps.items, + &deps.commands, + text, + &probe_strings, + ) + }) } /// Wrapper 100 teaches 200 by a slot-0 `LEARN_SPELL`, 200 creates item 777, and 777's display 5 @@ -461,6 +504,378 @@ fn ability_req_shows_the_required_rank_on_real_data() { assert_eq!(svc.ability_reqs[0].name, "Heroic Strike (Rank 1)"); } +/// `GetTrainerServiceDescription 0x4d9b40`: a class trainer's plain ability row expands the wire +/// spell's own `Spell.dbc` Description (`0x4d9c32`). +#[test] +fn service_description_expands_the_wire_spells_own_text() { + let d = SpellDisplay { + description: Some("Increases melee damage by $s1.".into()), + // A flat 11: base points are stored one under the total, on one one-sided die. + effect_base_points: [10, 0, 0], + effect_base_dice: [1, 0, 0], + effect_die_sides: [1, 0, 0], + ..Default::default() + }; + let spells = SpellCatalog::from_displays(HashMap::from([(78, d)])); + let deps = Deps::new(); + with_text(&spells, 0, 0, None, |text| { + assert_eq!( + service_description( + &wire(78, trainer_spell_state::GREEN, 10, 1, 0), + 0, + &spells, + &deps.items, + &deps.commands, + text, + ), + "Increases melee damage by 11." + ); + }); +} + +/// A learn wrapper has no text of its own, so the taught spell's Description is expanded +/// (`0x4d9c6f`); the description is the taught record's, not the wrapper's. +#[test] +fn service_description_hops_to_the_taught_spell() { + let wrapper = SpellDisplay { + effects: [SPELL_EFFECT_LEARN_SPELL, 0, 0], + effect_trigger_spell: [200, 0, 0], + ..Default::default() + }; + let taught = SpellDisplay { + name: "Growl".into(), + description: Some("Causes $s1 threat.".into()), + effect_base_points: [9, 0, 0], + effect_base_dice: [1, 0, 0], + effect_die_sides: [1, 0, 0], + ..Default::default() + }; + let spells = SpellCatalog::from_displays(HashMap::from([(100, wrapper), (200, taught)])); + let deps = Deps::new(); + with_text(&spells, 0, 0, None, |text| { + assert_eq!( + service_description( + &wire(100, trainer_spell_state::GREEN, 10, 1, 0), + 0, + &spells, + &deps.items, + &deps.commands, + text, + ), + "Causes 10 threat." + ); + }); +} + +/// The third arm: a tradeskill recipe has no text of its own, so the created item's Description is +/// returned verbatim — never expanded (`0x4d9cd0`-`0x4d9d22`). +#[test] +fn service_description_takes_the_recipes_product_text_verbatim() { + let wrapper = SpellDisplay { + effects: [SPELL_EFFECT_LEARN_SPELL, 0, 0], + effect_trigger_spell: [200, 0, 0], + ..Default::default() + }; + let recipe = SpellDisplay { + attributes: SPELL_ATTR_IS_TRADESKILL, + effects: [SPELL_EFFECT_CREATE_ITEM, 0, 0], + effect_item_type: [777, 0, 0], + ..Default::default() + }; + let spells = SpellCatalog::from_displays(HashMap::from([(100, wrapper), (200, recipe)])); + let mut deps = Deps::new(); + let mut item = crate::items::test_template("Copper Shortsword"); + item.description = "Keeps $s1 for later.".into(); + deps.items.insert_template(777, Some(item)); + with_text(&spells, 0, 0, None, |text| { + assert_eq!( + service_description( + &wire(100, trainer_spell_state::GREEN, 10, 1, 0), + TRAINER_TYPE_TRADESKILL, + &spells, + &deps.items, + &deps.commands, + text, + ), + "Keeps $s1 for later.", + "the product's text is the item's own, tokens and all" + ); + }); +} + +/// A used row at a mount trainer skips its own text (`0x4d9c29`); the same row at a class trainer +/// shows it. +#[test] +fn service_description_skips_the_wire_text_for_a_used_mount_row() { + let d = SpellDisplay { + description: Some("Summons a mount.".into()), + effects: [SPELL_EFFECT_LEARN_SPELL, 0, 0], + effect_trigger_spell: [200, 0, 0], + ..Default::default() + }; + let taught = SpellDisplay { + description: Some("Riding skill.".into()), + ..Default::default() + }; + let spells = SpellCatalog::from_displays(HashMap::from([(100, d), (200, taught)])); + let deps = Deps::new(); + let description = |trainer_type, state| { + with_text(&spells, 0, 0, None, |text| { + service_description( + &wire(100, state, 10, 1, 0), + trainer_type, + &spells, + &deps.items, + &deps.commands, + text, + ) + }) + }; + assert_eq!( + description(0, trainer_spell_state::GREEN), + "Summons a mount." + ); + assert_eq!( + description(TRAINER_TYPE_MOUNT, trainer_spell_state::GRAY), + "Riding skill." + ); +} + +/// The caster selector: a `LEARN_PET_SPELL` wrapper expands against the pet (`0x6e3130`'s nonzero +/// selector), the same taught spell under a plain `LEARN_SPELL` against the player. +#[test] +fn service_description_expands_against_the_pet_for_a_learn_pet_row() { + // One point per level over `baseLevel` 1: level 0 reads 10, a level-2 pet reads 11. + let taught = SpellDisplay { + description: Some("Causes $s1 threat.".into()), + base_level: 1, + effect_base_points: [9, 0, 0], + effect_base_dice: [1, 0, 0], + effect_die_sides: [1, 0, 0], + effect_dice_per_level: [1, 0, 0], + ..Default::default() + }; + let pet_wrapper = SpellDisplay { + effects: [SPELL_EFFECT_LEARN_PET_SPELL, 0, 0], + effect_trigger_spell: [200, 0, 0], + ..Default::default() + }; + let plain_wrapper = SpellDisplay { + effects: [SPELL_EFFECT_LEARN_SPELL, 0, 0], + effect_trigger_spell: [200, 0, 0], + ..Default::default() + }; + let spells = SpellCatalog::from_displays(HashMap::from([ + (100, pet_wrapper), + (101, plain_wrapper), + (200, taught), + ])); + let deps = Deps::new(); + // A level-2 pet (skill 10) against a player with no skill in the taught spell's line at all. + with_text(&spells, 0, 10, None, |text| { + assert_eq!( + service_description( + &wire(100, trainer_spell_state::GREEN, 10, 1, 0), + 0, + &spells, + &deps.items, + &deps.commands, + text, + ), + "Causes 11 threat.", + "the pet's level drives the taught spell's points" + ); + assert_eq!( + service_description( + &wire(101, trainer_spell_state::GREEN, 10, 1, 0), + 0, + &spells, + &deps.items, + &deps.commands, + text, + ), + "Causes 10 threat.", + "the same taught spell under LEARN_SPELL reads the player's" + ); + }); +} + +/// The `$z` token: the bind point's area name (`AreaTable.dbc`), and raw where no bind point has +/// arrived, as the item and tooltip feeds' `$z` behaves. +#[test] +fn service_description_expands_z_to_the_bind_area() { + let d = SpellDisplay { + description: Some("Returns you to $z.".into()), + ..Default::default() + }; + let spells = SpellCatalog::from_displays(HashMap::from([(556, d)])); + let deps = Deps::new(); + for (home, want) in [ + (Some("Razor Hill"), "Returns you to Razor Hill."), + (None, "Returns you to $z."), + ] { + with_text(&spells, 0, 0, home, |text| { + assert_eq!( + service_description( + &wire(556, trainer_spell_state::GREEN, 4000, 30, 0), + 0, + &spells, + &deps.items, + &deps.commands, + text, + ), + want + ); + }); + } +} + +/// On the shipped `Spell.dbc`: the warrior wrapper 1605 teaches 78 Heroic Strike, so the row's +/// description is the taught ability's text, not the wrapper's empty one. Skips without client +/// data. +#[test] +fn service_description_on_real_data_expands_the_taught_ability() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let spells = benilla_formats::load_spell_catalog(&mut chain).expect("load Spell"); + + let svc = resolve_service( + &wire(1605, trainer_spell_state::GREEN, 10, 1, 0), + 0, + &spells, + None, + &BTreeSet::new(), + ); + assert!( + svc.description + .starts_with("A strong attack that increases melee damage by "), + "the taught 78's description, not the wrapper's empty one: {:?}", + svc.description + ); + assert!( + !svc.description.contains('$'), + "every token resolved: {:?}", + svc.description + ); +} + +/// On the shipped `Spell.dbc`: 1352 is the shaman trainer's Astral Recall wrapper, teaching 556, +/// whose text is the one trainer-taught description carrying `$z`. Skips without client data. +#[test] +fn service_description_on_real_data_reads_astral_recalls_bind_area() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let spells = benilla_formats::load_spell_catalog(&mut chain).expect("load Spell"); + + let deps = Deps::new(); + let svc = with_text(&spells, 0, 0, Some("Razor Hill"), |text| { + super::resolve_service( + &wire(1352, trainer_spell_state::GREEN, 4000, 30, 0), + 0, + &spells, + None, + &BTreeSet::new(), + None, + &deps.items, + &deps.commands, + text, + &probe_strings, + ) + }); + assert_eq!( + svc.description, + "Yanks the caster through the twisting nether back to Razor Hill. Speak to an \ + Innkeeper in a different place to change your home location." + ); +} + +/// On the shipped `Spell.dbc`: 2020 is the "Apprentice Blacksmith" wrapper a Blacksmithing trainer +/// lists, teaching 2018 "Blacksmithing" — the profession-learn row whose text is the taught +/// profession's. Skips without client data. +#[test] +fn service_description_on_real_data_reads_the_profession_learn_row() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let spells = benilla_formats::load_spell_catalog(&mut chain).expect("load Spell"); + + // The wrapper carries no text of its own, so the taught profession's is the only one there is. + assert_eq!( + spells.get(2020).and_then(|d| d.description.as_deref()), + None + ); + let svc = resolve_service( + &wire(2020, trainer_spell_state::GREEN, 9, 5, 0), + TRAINER_TYPE_TRADESKILL, + &spells, + None, + &BTreeSet::new(), + ); + assert_eq!( + svc.description, + "Allows a Blacksmith to make basic weapons and armor up to a maximum potential skill of \ + 75. Requires stone and metal found with the mining skill." + ); +} + +/// On the shipped `Spell.dbc`: 7820 is the recipe wrapper a Blacksmithing trainer lists, 7818 the +/// recipe it teaches and 6338 the Silver Rod it makes, so the row's text is that item's +/// description, read out of the item cache (`0x4d9cd0`-`0x4d9d22`). The product's record arrives +/// with the item query, so the template seeded below, with vmangos's text, stands in for it. +/// Skips without client data. +#[test] +fn service_description_on_real_data_reaches_for_the_recipes_product() { + let data = benilla_formats::wow_data_or_skip!(); + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let spells = benilla_formats::load_spell_catalog(&mut chain).expect("load Spell"); + + // Neither the wrapper nor the recipe carries text of its own, so the item is the only source. + assert_eq!( + spells.get(7820).and_then(|d| d.description.as_deref()), + None + ); + assert_eq!( + spells.get(7818).and_then(|d| d.description.as_deref()), + None + ); + + // The law alone, never through `resolve_service`: the icon law asks for the same template + // first, and one ask per pending entry reaches the cache. + let describe = |deps: &Deps| { + with_text(&spells, 0, 0, None, |text| { + service_description( + &wire(7820, trainer_spell_state::GREEN, 90, 0, 164), + TRAINER_TYPE_TRADESKILL, + &spells, + &deps.items, + &deps.commands, + text, + ) + }) + }; + + let deps = Deps::new(); + assert_eq!( + describe(&deps), + "", + "nothing to show until the product's template lands" + ); + assert_eq!( + deps.queried_entries(), + vec![6338], + "the description law reached for the crafted rod's template" + ); + + // With the product's record present, its description is the row's text, verbatim. + let mut product = crate::items::test_template("Silver Rod"); + product.description = "Needed by Enchanters.".into(); + let landed = { + let mut deps = Deps::new(); + deps.items.insert_template(6338, Some(product)); + deps + }; + assert_eq!(describe(&landed), "Needed by Enchanters."); +} + #[test] fn snapshot_is_none_when_closed_and_lists_services_when_open() { let spells = empty_catalog();