diff --git a/crates/benilla-app/assets/ui/ActionBar.xml b/crates/benilla-app/assets/ui/ActionBar.xml index a09af17c..02243303 100644 --- a/crates/benilla-app/assets/ui/ActionBar.xml +++ b/crates/benilla-app/assets/ui/ActionBar.xml @@ -427,6 +427,7 @@ local action = BenillaActionButton_Action(button) if HasAction(action) then button.icon:SetTexture(GetActionTexture(action) or BENILLA_FALLBACK_ICON) + button.icon:Show() button:SetNormalTexture("Interface\\Buttons\\UI-Quickslot2") button:Show() button.rangeTimer = -1 @@ -436,7 +437,12 @@ BenillaActionButton_UpdateCooldown(button) BenillaActionButton_UpdateFlash(button) else - button.icon:SetTexture(nil) + -- ref l.168: the empty arm HIDES the icon region — it does not clear the path. The + -- distinction is load-bearing here: UpdateUsable's SetVertexColor tint survives a + -- SetTexture(nil) ("a tint outlives the art"), and an occupied slot going empty on + -- a character switch left that white tint on a pathless region — the solid white + -- buttons of the 2026-08-07 report (decision 1108, with the engine-side draw gate). + button.icon:Hide() button.count:SetText("") button.cooldown:Hide() button.rangeTimer = nil diff --git a/crates/benilla-app/assets/ui/TrainerFrame.xml b/crates/benilla-app/assets/ui/TrainerFrame.xml index 22ec07f2..52ded5e6 100644 --- a/crates/benilla-app/assets/ui/TrainerFrame.xml +++ b/crates/benilla-app/assets/ui/TrainerFrame.xml @@ -404,16 +404,24 @@ addRow("used", LABEL_USED, GRAY_CODE) end - -- A row's func fires with the row button as `this` (this.value / this.checked = the PRE-click - -- state, since benilla's kit doesn't auto-toggle the check). Flip the engine filter, re-init the - -- open menu to refresh its checks (keepShownOnClick keeps it up), reset scroll, repaint. + -- A row's func fires with the row button as `this`, carrying the PRE-click state (this.value / + -- this.checked): the kit's UIDropDownMenuButton_OnClick runs the func FIRST and only then, for a + -- keepShownOnClick row, flips the check texture + this.checked itself. So the func's whole job is + -- to flip the ENGINE filter to the inverse of the pre-click check, reset scroll, and repaint — + -- the visible check is the kit's to move, and the next open re-derives every row from + -- GetTrainerServiceTypeFilter anyway. + -- + -- It must NOT re-Initialize the open menu here. Doing so re-ran AddButton, which sets + -- `button.checked` (and the Check texture) from the fresh engine state — so the kit's flip + -- immediately afterwards inverted the ALREADY-correct row and put it back. The check never + -- appeared to move, and because `this.checked` then read stale-true forever, a filter switched + -- off could never be switched back on. function BenillaTrainerFilterDropDown_OnClick() if this.checked then SetTrainerServiceTypeFilter(this.value, 0) else SetTrainerServiceTypeFilter(this.value, 1) end - UIDropDownMenu_Initialize(BenillaTrainerFilterDropDown, BenillaTrainerFilterDropDown_Initialize) BenillaFauxScrollFrame_SetOffset(getglobal("BenillaTrainerListScrollFrame"), 0) BenillaTrainerFrame_Update() end diff --git a/crates/benilla-app/src/cooldowns.rs b/crates/benilla-app/src/cooldowns.rs index a530680b..38869d8b 100644 --- a/crates/benilla-app/src/cooldowns.rs +++ b/crates/benilla-app/src/cooldowns.rs @@ -944,7 +944,7 @@ mod tests { cds.start_gcd(772, &fireball(), t0); let mid = t0 + Duration::from_millis(200); let attack = SpellDisplay { - effect_1: 78, // SPELL_EFFECT_ATTACK + effects: [78, 0, 0], // SPELL_EFFECT_ATTACK start_recovery_category: 133, ..Default::default() }; diff --git a/crates/benilla-app/src/entities.rs b/crates/benilla-app/src/entities.rs index 745831df..dcfef534 100644 --- a/crates/benilla-app/src/entities.rs +++ b/crates/benilla-app/src/entities.rs @@ -838,6 +838,10 @@ fn setup_entities( Ok(catalog) => commands.insert_resource(crate::ui_mail::Stationery(catalog)), Err(e) => warn!("stationery catalog unavailable, mail uses the default backdrop: {e:#}"), } + match benilla_formats::load_page_text_material_catalog(&mut chain) { + Ok(catalog) => commands.insert_resource(crate::ui_item_text::PageMaterials(catalog)), + Err(e) => warn!("page text materials unavailable, books read on parchment: {e:#}"), + } } /// For every display id active among the net entities: ensure its [`DisplayModel`] exists (resolve the diff --git a/crates/benilla-app/src/go_templates.rs b/crates/benilla-app/src/go_templates.rs index ac6d0841..d44abbc1 100644 --- a/crates/benilla-app/src/go_templates.rs +++ b/crates/benilla-app/src/go_templates.rs @@ -38,15 +38,24 @@ pub(crate) struct GoTemplate { pub(crate) lock_id: u32, /// The display name — the hover tooltip's gold first line (decision 0276's GO law). pub(crate) name: String, - /// GENERIC (type 5) only: the template's `data[1]`, the vanilla **highlight** column, nonzero - /// on 1387 of the 1870 shipped type-5 templates. It is GENERIC's whole mouseover-eligibility - /// answer (`0x5f4830`, decision 0762) — a road signpost hovers because this is 1, and its - /// neighbours with 0 are scenery the reference never lets you hover at all. - pub(crate) generic_highlight: bool, + /// The vanilla **highlight** column, for the two types whose mouseover-eligibility slot reads + /// it instead of running a predicate (decision 1106): GENERIC(5)'s `data[1]` (`0x5f4830`, + /// decision 0762 — nonzero on 1387 of the 1870 shipped type-5 templates, which is why a road + /// signpost hovers and the scenery beside it never does) and CAPTURE_POINT(29)'s `data[19]` + /// (`0x5f6d80` — byte-for-byte the same shape, a different slot). `false` for every other type, + /// which never consults it. + pub(crate) highlight_column: bool, + /// MEETINGSTONE (type 23) only: the template's `data[2]` = **areaID**, the sole input of that + /// type's own `highlightable` slot (`0x5f6990` — decision 1106). `None` for every other type. + pub(crate) meeting_stone_area: Option, /// MO_TRANSPORT (type 15) path parameters — `Some` only for boats/zeppelins (decision 0438): /// the template's `data0..2` = (taxiPathId, moveSpeed, accelRate), the inputs the transport /// timetable is built from. pub(crate) mo_transport: Option, + /// TEXT (type 9) only: the book/plaque's page chain head + frame material (decision 1105). + /// `Some` with a nonzero `page_id` is what makes a right-click *read* it; a type-9 template + /// with no page (vanilla ships a handful) opens nothing at all, exactly like the reference. + pub(crate) text_page: Option, } /// A MO_TRANSPORT template's path tuple (`gameobject_template.data0..2`, decision 0438). @@ -57,6 +66,18 @@ pub(crate) struct MoTransport { pub(crate) accel_rate: f32, } +/// A TEXT (type 9) template's readable head — `data[0]`/`data[2]` (vmangos `GameObjectInfo::text` +/// = `pageID, language, pageMaterial, allowMounted`; decision 1105). The client reads both through +/// the same per-type attribute→slot table (`0x621b00`, attribute `0x11` = pageMaterial), so the +/// two layouts are the one layout. +#[derive(Clone, Copy)] +pub(crate) struct TextPage { + /// `data[0]` — the first page's `PageText` id; `0` = nothing to read. + pub(crate) page_id: u32, + /// `data[2]` — the `PageTextMaterial.dbc` id the reader's frame paints with. + pub(crate) material: u32, +} + /// `entry → template`, ask-once per connection (mirrors [`crate::names::NameCache`]'s discipline). #[derive(Resource, Default)] pub(crate) struct GameObjectTemplates { @@ -89,21 +110,39 @@ impl GameObjectTemplates { .and_then(|slot| data.get(slot)) .map(|&v| v.max(0) as u32) .unwrap_or(0); + // The highlight column, at the slot its type reads it from (decision 1106): GENERIC(5) + // `data[1]`, CAPTURE_POINT(29) `data[19]`. Both slots are resolved by the same + // `0x621b00(type, semantic 0x12)` lookup in the reference; the two shipped answers are + // inlined here for the same reason [`go_lock_slot`] inlines the lock's. + let highlight_column = match type_id { + 5 => data[1] != 0, + 29 => data[19] != 0, + _ => false, + }; + // MEETINGSTONE (23): data[2] = areaID (vmangos `gameobject_template`), the one input of + // that type's own highlightable slot. + let meeting_stone_area = (type_id == 23).then(|| data[2].max(0) as u32); // MO_TRANSPORT (15): data0..2 = taxiPathId / moveSpeed / accelRate (vmangos // `GameObjectInfo::moTransport`; decision 0438). - let generic_highlight = type_id == 5 && data[1] != 0; let mo_transport = (type_id == 15).then(|| MoTransport { taxi_path_id: data[0].max(0) as u32, move_speed: data[1].max(0) as f32, accel_rate: data[2].max(0) as f32, }); + // TEXT (9): data[0] = pageID, data[2] = pageMaterial (decision 1105). + let text_page = (type_id == 9).then(|| TextPage { + page_id: data[0].max(0) as u32, + material: data[2].max(0) as u32, + }); self.templates.insert( entry, GoTemplate { lock_id, name, - generic_highlight, + highlight_column, + meeting_stone_area, mo_transport, + text_page, }, ); } @@ -163,6 +202,24 @@ mod tests { assert!(t.templates[&2].mo_transport.is_none()); } + /// TEXT (9) captures the readable head — `data[0]` page id, `data[2]` material (decision + /// 1105); no other type does. + #[test] + fn insert_captures_the_text_page_head() { + let mut t = GameObjectTemplates::default(); + let mut data = [0i32; 24]; + data[0] = 1416; // pageID + data[1] = 0; // language + data[2] = 2; // pageMaterial (Stone) + t.insert(2036, 9, "Book".into(), &data); + let page = t.templates[&2036].text_page.expect("type 9 captures"); + assert_eq!(page.page_id, 1416); + assert_eq!(page.material, 2); + // A goober with the same bytes does not — its data[0] is a lockId. + t.insert(2037, 10, "Lever".into(), &data); + assert!(t.templates[&2037].text_page.is_none()); + } + #[test] fn insert_resolves_chest_lockid_from_slot_0() { let mut t = GameObjectTemplates::default(); diff --git a/crates/benilla-app/src/interact/inspect.rs b/crates/benilla-app/src/interact/inspect.rs index 316a3b1d..42617fdf 100644 --- a/crates/benilla-app/src/interact/inspect.rs +++ b/crates/benilla-app/src/interact/inspect.rs @@ -66,6 +66,9 @@ pub(super) struct InspectStores<'w, 's> { lit: Query<'w, 's, EntityLightReadout>, factions: Option>, self_store: Query<'w, 's, &'static ObjectStore, With>, + /// The ask-once GO template cache — the readable head a TEXT object's line reports + /// (decision 1105). + go_templates: Res<'w, crate::go_templates::GameObjectTemplates>, } /// The inspector overlay, drawn only while armed: a weak top-centre "armed" pill (so it's obvious the @@ -139,6 +142,7 @@ pub(super) fn inspect_ui( .and_then(|e| parents.get(e).ok()) .map(|c| c.parent()); let (factions, self_store) = (stores.factions.as_deref(), stores.self_store.single().ok()); + let go_templates = &*stores.go_templates; let (stores, kinds, collision, lit) = ( &stores.stores, &stores.kinds, @@ -212,18 +216,41 @@ pub(super) fn inspect_ui( // types whose `+0x14` is a constant `xor al,al`. let reaction = crate::target::cursor_mode::go_reaction(factions, s.0.gameobject_faction(), self_store); - let channel_owned = crate::target::cursor_mode::fishing_channel_owned( - self_store, - net_entity.and_then(|p| guids.get(p).ok()).map(|g| g.0), - ); - let interact = - if crate::target::cursor_mode::go_highlightable(s, reaction, channel_owned) { + let go_guid = net_entity.and_then(|p| guids.get(p).ok()).map(|g| g.0); + let overrides = crate::target::cursor_mode::GoOverrides { + channel_owned: crate::target::cursor_mode::fishing_channel_owned( + self_store, go_guid, + ), + meeting_stone_queued: crate::target::cursor_mode::meeting_stone_queued( + go_guid.and_then(|g| go_templates.get(g)?.meeting_stone_area), + ), + }; + let interact = if crate::target::cursor_mode::go_highlightable(s, reaction, overrides) { "interact ✓" } else { "interact ✗" }; + // TEXT (type 9) only: the **readable head** (decision 1105). A book that opens no + // window is either "no page in the template" or a fault downstream, and only this + // line tells the two apart — the symptom is identical from the chair, and the first + // one is the reference behaving correctly. `page —` = the template says none; + // `page ?` = its ask-once query hasn't answered yet. + let page_text = if s.0.gameobject_type_id() == crate::target::cursor_mode::GO_TYPE_TEXT + { + let go_guid = net_entity.and_then(|p| guids.get(p).ok()).map(|g| g.0); + match go_guid + .and_then(|g| go_templates.get(g)) + .map(|t| t.text_page.map_or(0, |p| p.page_id)) + { + None => " · page ?".to_string(), + Some(0) => " · page —".to_string(), + Some(id) => format!(" · page {id}"), + } + } else { + String::new() + }; format!( - "go type {} · state {state} {word} · {solidity} · flags {flags:#x}{flag_text} · {interact}", + "go type {} · state {state} {word} · {solidity} · flags {flags:#x}{flag_text} · {interact}{page_text}", s.0.gameobject_type_id() ) }); diff --git a/crates/benilla-app/src/items.rs b/crates/benilla-app/src/items.rs index d448c295..278bf639 100644 --- a/crates/benilla-app/src/items.rs +++ b/crates/benilla-app/src/items.rs @@ -289,6 +289,46 @@ impl Items { } } +/// The ask-once trio every UI resolver needs to be tested: the template cache, the command +/// channel, and — crucially — the channel's live **receiver**, without which every `ItemQuery` +/// send fails and the "asked the server" half of the law goes unobservable. `items` starts empty, +/// so a fresh `TestDeps` reads as "template in flight"; seed one with [`Items::insert_template`] +/// to test the landed arm. +/// +/// Shared because the three icon laws (`ui_trainer::service_icon`, `ui_tradeskill::recipe_icon`, +/// `ui_craft::craft_icon`) all terminate in this same cache — the fixture is the one thing they +/// legitimately have in common, unlike the laws themselves. +#[cfg(test)] +pub(crate) struct TestDeps { + pub(crate) items: Items, + pub(crate) commands: NetCommands, + rx: crossbeam_channel::Receiver, +} + +#[cfg(test)] +impl TestDeps { + pub(crate) fn new() -> Self { + let (tx, rx) = crossbeam_channel::unbounded(); + TestDeps { + items: Items::default(), + commands: NetCommands(tx), + rx, + } + } + + /// The entries the resolver asked the server for — an ask-once gate firing is observable here + /// even when the icon it feeds is still `None`. + pub(crate) fn queried_entries(&self) -> Vec { + self.rx + .try_iter() + .filter_map(|c| match c { + ClientCommand::ItemQuery { entry, .. } => Some(entry), + _ => None, + }) + .collect() + } +} + /// A minimal, VALID item template named `name` — the shared test seam for every module that /// needs a landed template (the sentinels that matter are `allowable_*` = −1 and `stackable` = 1). #[cfg(test)] diff --git a/crates/benilla-app/src/lighting/mod.rs b/crates/benilla-app/src/lighting/mod.rs index b8da6e06..ac768bc0 100644 --- a/crates/benilla-app/src/lighting/mod.rs +++ b/crates/benilla-app/src/lighting/mod.rs @@ -65,16 +65,18 @@ pub(crate) struct WowLighting { /// Fed to the `SkyPlugin` dome material; interpolated across the dome by elevation. pub(crate) sky: [[f32; 3]; 5], /// **Per-kind water-surface tint** `[shallow, deep]` — `Atmosphere.water_river` (IntBand rows - /// 16/17) and `.water_ocean` (rows 14/15), RAW, resolved from the **picked dominant `LightParams`** - /// (`sample`/`pick_light`), NOT the area-light blend — the blend cross-mixes neighbouring params - /// and muddies the zone tint in overlap regions (VERIFIED STV). The from-above depth swatch is a - /// 2-endpoint linear lerp of these by the per-vertex depth `V` (river/lake `V = clamp(byte/42)`; - /// VERIFIED `WoW.exe FUN_0068a830` + `c81768`). Pushed onto the per-kind liquid materials by - /// [`apply_wow_lighting`] via [`WowLighting::water_colors`]. + /// 16/17) and `.water_ocean` (rows 14/15), RAW, resolved from the **area-light blend**, exactly + /// like every other band: the client's gather record carries all 18 colour rows and + /// `dn_record_overblend 0x6d30e0` merges all 18 per light (rows 14–17 are its `+0x34..+0x40` + /// step-9 loop) — there is no single-sphere pick in the water path (decision 1104, superseding + /// the `pick_light` split whose discontinuity snapped the tint at Tirisfal→Silverpine). The + /// from-above depth swatch is a 2-endpoint linear lerp of these by the per-vertex depth `V` + /// (river/lake `V = clamp(byte/42)`; VERIFIED `WoW.exe FUN_0068a830` + `c81768`). Pushed onto the + /// per-kind liquid materials by [`apply_wow_lighting`] via [`WowLighting::water_colors`]. pub(crate) water_river: [[f32; 3]; 2], pub(crate) water_ocean: [[f32; 3]; 2], /// **Per-kind water-blend alphas** `[shallow, deep]` — `LightParams.water/oceanShallow/DeepAlpha`, - /// from the picked param (like the tint). The swatch's depth-alpha ramp endpoints. Per-zone: + /// blended across the same spheres as the tint. The swatch's depth-alpha ramp endpoints. Per-zone: /// Elwynn/Loch Modan shallow ≈0.5, STV ≈0.85; deep ≈1.0. Replaces the hardcoded `*_SHALLOW_ALPHA` /// constants for the live path (they remain as the `setup_liquid` frame-0 seed + DBC-absent default). pub(crate) water_river_alpha: [f32; 2], diff --git a/crates/benilla-app/src/lighting/resolve.rs b/crates/benilla-app/src/lighting/resolve.rs index 79c29bce..4f818f70 100644 --- a/crates/benilla-app/src/lighting/resolve.rs +++ b/crates/benilla-app/src/lighting/resolve.rs @@ -295,19 +295,6 @@ pub(super) fn update_time_lighting( } }) .unwrap_or(Atmosphere::DEFAULT); - // The from-above water swatch is built from a SINGLE dominant `LightParams`' RAW water rows, NOT - // the area-light blend (VERIFIED WoW.21: the STV river swatch = LP 26 exactly; at Elwynn / Loch - // Modan pick == blend so this is a no-op there). `sample_blended` cross-mixes the water rows with - // neighbouring params and muddies the zone tint in overlap regions — the STV jungle river read a - // muddy yellow-green `[80,99,36]` instead of LP 26's grey-green `[90,140,140]`. So resolve the - // water tint via `pick_light` (single most-local sphere); the rest of the atmosphere stays on the - // faithful blend. (Underwater/weather on the surface tint is a deferred edge case — `sample` uses - // the clear slot; the submerged *scene* still comes from `atmo` above.) - let water_atmo = sampler - .as_ref() - .map(|s| s.0.sample(map, wow_pos, minute * 2, false)) - .unwrap_or(atmo); - // Sun direction from vanilla DayNight (fixed azimuth, slight elevation wobble, always up). Sampled // continuously (`minute_f`) — the shared global-light buffer makes per-frame light changes O(1) // (terrain + models read it directly; no per-material re-push), so the interim whole-minute quantize @@ -356,11 +343,16 @@ pub(super) fn update_time_lighting( wmo_fog_start, wmo_fog_end, sky: atmo.sky, - // Water tint + alphas from the PICKED dominant param (see `water_atmo` above), not the blend. - water_river: water_atmo.water_river, - water_ocean: water_atmo.water_ocean, - water_river_alpha: water_atmo.water_river_alpha, - water_ocean_alpha: water_atmo.water_ocean_alpha, + // The water swatch rides the SAME area blend as every other band — byte-VERIFIED, decision + // 1104: the gather record's 18 colour slots (rows 14–17 at `+0x34..+0x40`) are all merged by + // `dn_record_overblend 0x6d30e0`, so there is no single-sphere pick anywhere in the client's + // water path. Resolving it off `pick_light` made the tint DISCONTINUOUS — it snapped whenever + // a tighter sphere's outer radius was crossed, i.e. at exactly the point where that sphere's + // own weight is zero (Tirisfal → Silverpine: green water to near-black brown in one step). + water_river: atmo.water_river, + water_ocean: atmo.water_ocean, + water_river_alpha: atmo.water_river_alpha, + water_ocean_alpha: atmo.water_ocean_alpha, glow: quantize_glow(atmo.glow), // per-zone bloom weight (LightParams.glow) // Dawn/dusk sky-dome warp: `S = dawn_dusk_curve(dayfrac) × highlightSky`. 0 across midday and // in highlightSky=0 zones, so it only warps the sky at ~06:30/21:30 in highlightSky=1 zones diff --git a/crates/benilla-app/src/net.rs b/crates/benilla-app/src/net.rs index af817ede..10d58d14 100644 --- a/crates/benilla-app/src/net.rs +++ b/crates/benilla-app/src/net.rs @@ -861,6 +861,11 @@ pub(crate) enum ClientCommand { /// `guid`. Sent ask-once when a GameObject streams in ([`crate::go_templates`]); the answer's /// `lockId` decides whether a right-click uses it or casts an OPEN_LOCK spell. GameObjectQuery { entry: u32, guid: u64 }, + /// Ask for one page of a book (`CMSG_PAGE_TEXT_QUERY`, decision 1105): the `PageText` id + the + /// asking object's `guid` (an item's or a TEXT GameObject's — the server discards it). Sent + /// ask-once by [`crate::ui_item_text::PageTexts`] when a reader opens on a page it hasn't got; + /// vmangos answers with the whole forward chain, one `SMSG_PAGE_TEXT_QUERY_RESPONSE` per page. + PageTextQuery { page_id: u32, guid: u64 }, /// Cast an OPEN_LOCK spell at a **GameObject** (`CMSG_CAST_SPELL`, decision 0239): the right-click /// on a locked chest / mining vein / herb node. The server runs `EffectOpenLock` → a chest opens /// its loot (`SMSG_LOOT_RESPONSE`); the profession/skill gate is the server's. diff --git a/crates/benilla-app/src/net/apply.rs b/crates/benilla-app/src/net/apply.rs index 27f362a4..b9f717ad 100644 --- a/crates/benilla-app/src/net/apply.rs +++ b/crates/benilla-app/src/net/apply.rs @@ -183,6 +183,9 @@ pub(super) fn apply_net_updates( // through the VM's own GlobalStrings by `ui_action::feed_actions`). ResMut, ResMut, + // The ask-once book-page cache (decision 1105) — every readable's text, keyed by + // `PageText` id; the reader session repaints off it. + ResMut, ), ), // One tuple param (the 16-SystemParam ceiling again): the action-bar- + merchant-facing errors @@ -329,6 +332,7 @@ pub(super) fn apply_net_updates( mut mirror_timers, mut pet_bar, mut ui_error_keys, + mut page_texts, ), ) = caches; let ( @@ -1316,6 +1320,13 @@ pub(super) fn apply_net_updates( SessionEvent::MailItemText { text_id, text } => { mail::mail_item_text(text_id, text, &mut mail_open) } + // The book-page cache (decision 1105) — one page per packet, the whole chain in + // answer to the first ask; the reader repaints off it on the next feed. + SessionEvent::PageText { + page_id, + text, + next_page_id, + } => page_texts.insert(page_id, text, next_page_id), SessionEvent::ReceivedMail { seconds } => { mail::received_mail(seconds, &mut mail_pending, &mail_open, &net_commands) } diff --git a/crates/benilla-app/src/net/io.rs b/crates/benilla-app/src/net/io.rs index 6a37d641..fde8928b 100644 --- a/crates/benilla-app/src/net/io.rs +++ b/crates/benilla-app/src/net/io.rs @@ -760,6 +760,9 @@ fn writer_loop( ClientCommand::GameObjUse { guid } => w.gameobj_use(guid), ClientCommand::AreaTrigger { trigger_id } => w.area_trigger(trigger_id), ClientCommand::GameObjectQuery { entry, guid } => w.gameobject_query(entry, guid), + ClientCommand::PageTextQuery { page_id, guid } => { + w.page_text_query(page_id, guid) + } ClientCommand::CastSpellGameObject { spell_id, go_guid } => { w.cast_spell_gameobject(spell_id, go_guid) } diff --git a/crates/benilla-app/src/target/click.rs b/crates/benilla-app/src/target/click.rs index 8ec05316..efbb9390 100644 --- a/crates/benilla-app/src/target/click.rs +++ b/crates/benilla-app/src/target/click.rs @@ -142,7 +142,8 @@ const EMOTE_TALK: u16 = 60; /// auto-approach yet) — the selection still lands. Attack, by contrast, is never range-gated /// (`unable` only grays): the server holds the swing until we're in reach, as the real client does. /// A right-click on empty ground was just a turn — it never deselects. -#[allow(clippy::too_many_arguments)] +// The `ui_feedback` tuple is the 16-SystemParam ceiling's overflow bundle, commented at its site. +#[allow(clippy::too_many_arguments, clippy::type_complexity)] pub(super) fn act_on_right_click( mut clicks: MessageReader, hovered: Res, @@ -171,9 +172,12 @@ pub(super) fn act_on_right_click( ResMut, ResMut, ResMut, + // The reader session the TEXT branch opens (decision 1105) — like the mailbox, a + // client-side window with no packet behind it. + ResMut, ), ) { - let (mut ui_error_keys, mut cast_errors, mut loot_latch, mut mail) = ui_feedback; + let (mut ui_error_keys, mut cast_errors, mut loot_latch, mut mail, mut item_text) = ui_feedback; if clicks.read().last().is_none() { return; } @@ -209,6 +213,29 @@ pub(super) fn act_on_right_click( } return; } + // TEXT (GO type 9): a book, plaque or sign — READ it, client-side, before the lock + // fork (a readable is never locked). Its strategy overrides the use-slot the same + // way the mailbox does — `0x5f58c0` calls the local page-text opener + // `0x4e32e0(goGuid, 0)` and never the shared `CMSG_GAMEOBJ_USE` sender (decision + // 1105; wow-re cursor-system §4's "TEXT type 9 — its own handler"). Sending USE + // instead is what left every world book dead: vmangos' `GameObject::Use` has no + // type-9 case at all, so the packet is answered with silence. + // + // `arg2 == 0` here means the reference's **toggle**: right-clicking the book whose + // reader is already open closes it. The page id + material are NOT resolved here — + // the reader asks the object's template for them as it paints, like the reference's + // `vtbl+0x74` getter, so a click that beats the ask-once template query still opens. + if go.is_some_and(|(s, _)| s.0.gameobject_type_id() == cursor_mode::GO_TYPE_TEXT) { + if !cursor.unable { + if item_text.toggle_closed(guid) { + debug!("right-click text gameobject: re-click closes {guid:#x}"); + } else { + debug!("right-click text gameobject: read {guid:#x}"); + item_text.open_pages(guid); + } + } + return; + } // Branch on the lock (decisions 0239 / 0545 / 0752): a lockless GameObject is USEd; // a lockable one casts the opener (a known OPEN_LOCK spell, or a carried key's own // ON_USE) at it; an unopenable lock shows the client-local red toast — "The door is diff --git a/crates/benilla-app/src/target/cursor_mode.rs b/crates/benilla-app/src/target/cursor_mode.rs index 7c735388..9256a84e 100644 --- a/crates/benilla-app/src/target/cursor_mode.rs +++ b/crates/benilla-app/src/target/cursor_mode.rs @@ -209,9 +209,35 @@ const GO_TYPE_SPELL_FOCUS: i32 = 8; const GO_TYPE_DUEL_ARBITER: i32 = 16; const GO_TYPE_FISHINGHOLE: i32 = 25; const GO_TYPE_AURA_GENERATOR: i32 = 30; -/// `GAMEOBJECT_TYPE_TEXT` (9) — a readable plaque/sign; its per-type behavior shows the **Inspect** -/// magnifier (wow-re cursor-system §4, `0x5f5890`), not the gear. -const GO_TYPE_TEXT: i32 = 9; +/// The highest GAMEOBJECT_TYPE_ID the type factory has a `case` for. The jump table `0x5f76cc` +/// covers 0..=30; **21 GUARDPOST has no case of its own** and falls to `default:` with everything +/// out of range — the arms' `__LINE__` pushes step three apart across the 30 real types with none +/// for 21 (wow-re `animation/scratch/gameobject-anim-arm.md`, the per-type arm table). +const GO_TYPE_MAX: i32 = 30; +/// `GAMEOBJECT_TYPE_GUARDPOST` (21) — see [`strategy_is_default`]. +const GO_TYPE_GUARDPOST: i32 = 21; + +/// Whether the type factory hands this type the **default** strategy — the shared static placeholder +/// rather than a type of its own. Type 21 and every out-of-range id take the `default:` arm +/// `0x5f76a0`, which allocates nothing: `mov dword ptr [esi+0x210], 0xc4d840` parks one static in +/// `.bss` (given vtable `0x80b188` by the initializer `0x5f36e0`) and logs `"BADBASEGAMEOBJECT|%d"`. +/// That vtable's `+0x14` is `0x5f36f0` = `xor al,al`, and its `+0xc` forwards to it — so a +/// default-strategy object is neither highlightable nor mouseover-eligible: it shows nothing at all. +/// +/// Byte-cited in wow-re `animation/scratch/gameobject-anim-arm.md` (the arm bytes, the initializer, +/// and the `BADBASEGAMEOBJECT` string) and corroborated independently by +/// `object-layer/scratch/w2c1.md` ("case 0x15 falls to `default`"). Unobservable on this server — +/// vanilla ships no type-21 template and a 1.12 server sends nothing out of range — but modelled +/// rather than left to the flag predicate, which would answer `true` for an all-zero descriptor. +fn strategy_is_default(type_id: i32) -> bool { + type_id == GO_TYPE_GUARDPOST || !(0..=GO_TYPE_MAX).contains(&type_id) +} +/// `GAMEOBJECT_TYPE_TEXT` (9) — a readable book/plaque/sign; its per-type behavior shows the +/// **Inspect** magnifier (wow-re cursor-system §4, `0x5f5890`), not the gear. `pub(super)` so the +/// right-click dispatch ([`super::act_on_right_click`]) routes it to the client-side reader off the +/// one type constant (decision 1105), the same shape as the mailbox — and `pub(crate)` beyond that +/// so the inspector's GO card can report the readable head against the same constant. +pub(crate) const GO_TYPE_TEXT: i32 = 9; /// The three GameObject types that show the **Mail** cursor (wow-re cursor-system §4): RITUAL(18) /// and MAILBOX(19) share one behavior (`0x5f6840`), and type 28 (`0x5f6e30`) resolves to Mail too. /// Type 28 has no live 1.12 data but is included for byte-fidelity with the factory switch. @@ -252,27 +278,68 @@ const GO_DYNFLAG_ACTIVATE: u32 = 0x1; /// The GameObject types whose strategy vtable **overrides** the highlightable slot (`+0x14`) with a /// constant `xor al,al` — so `0x5f2f80` is never reached for them and they are never highlightable, -/// whatever their flags, faction or activate bit say. Three families, all byte-read off the 5875 -/// vtables: GENERIC(5) decoration (`0x5f47f0`), the transports (see [`GO_TYPE_TRANSPORT`]), and the -/// marker set (see [`GO_TYPE_SPELL_FOCUS`]). +/// whatever their flags, faction or activate bit say. **Eleven of the thirty-one types**, byte-read +/// off the 5875 vtables (wow-re `object-layer/scratch/go-strategy-vtable-table.md`, the complete +/// 31-row table): GENERIC(5) `0x5f47f0` · the transports 11/14/15 · the marker set 8/16/25/30 · +/// AUCTIONHOUSE(20) `0x5f68a0` · CAPTURE_POINT(29) `0x5f6d40` · and the default strategy +/// ([`strategy_is_default`]) `0x5f36f0`. /// /// This is the whole per-type half of the predicate, kept as one named list because the reference /// keeps it as one vtable slot: adding a type here removes its cursor, its brighten, its right-click /// USE and its pick priority together, exactly as the binary does. fn strategy_never_highlightable(type_id: i32) -> bool { - matches!( - type_id, - GO_TYPE_GENERIC - | GO_TYPE_TRANSPORT - | GO_TYPE_MAP_OBJECT - | GO_TYPE_MO_TRANSPORT - | GO_TYPE_SPELL_FOCUS - | GO_TYPE_DUEL_ARBITER - | GO_TYPE_FISHINGHOLE - | GO_TYPE_AURA_GENERATOR - ) + strategy_is_default(type_id) + || matches!( + type_id, + GO_TYPE_GENERIC + | GO_TYPE_TRANSPORT + | GO_TYPE_MAP_OBJECT + | GO_TYPE_MO_TRANSPORT + | GO_TYPE_SPELL_FOCUS + | GO_TYPE_DUEL_ARBITER + | GO_TYPE_FISHINGHOLE + | GO_TYPE_AURA_GENERATOR + | GO_TYPE_AUCTIONHOUSE + | GO_TYPE_CAPTURE_POINT + ) } +/// `GAMEOBJECT_TYPE_AUCTIONHOUSE` (20) — its `+0x14` is `0x5f68a0`, a bare `xor al,al; ret`, and its +/// `+0xc` forwards straight into it. So an auction-house **GameObject** shows nothing at all: no +/// cursor, no tooltip, no brighten, and `OnUse 0x5f8660` returns at `0x5f8673` without sending. (The +/// auctioneer you actually click in 1.12 is a *unit* with the AUCTIONEER service bit — a different +/// branch entirely — which is why this reads as a surprise and isn't one.) +const GO_TYPE_AUCTIONHOUSE: i32 = 20; +/// `GAMEOBJECT_TYPE_CAPTURE_POINT` (29) — `+0x14` = `0x5f6d40` const-false, so never a cursor; but +/// its `+0xc` is `0x5f6d80` = `data[19] != 0`, byte-for-byte GENERIC's shape at a different slot. +/// So it carries **GENERIC's exact law**: tooltip and brighten iff its highlight column is set, +/// never an interact cursor. Ships in no 1.12 data. +const GO_TYPE_CAPTURE_POINT: i32 = 29; +/// The inputs of the **own-predicate** `+0x14` slots — the two types (so far) whose highlightable +/// slot is neither the shared `0x5f2f80` nor a constant, and so reads state the shared gate never +/// touches. Resolved by the caller, like the reference resolves them from the active player and the +/// UI globals; each field is inert for every type but its own. +/// +/// A struct rather than a tail of bare booleans on purpose: the signature was growing one `bool` per +/// override discovered (17 FISHINGNODE, then 23 MEETINGSTONE), and three adjacent same-typed +/// arguments is a swap waiting to happen at a call site. [`Default`] is "no override applies", which +/// is what every test and every non-override type wants. +#[derive(Clone, Copy, Default)] +pub(crate) struct GoOverrides { + /// FISHINGNODE(17): the local player is channeling at exactly this bobber + /// ([`fishing_channel_owned`]). + pub(crate) channel_owned: bool, + /// MEETINGSTONE(23): this stone's area IS the area we are queued at + /// ([`meeting_stone_queued`]). + pub(crate) meeting_stone_queued: bool, +} + +/// `GAMEOBJECT_TYPE_MEETINGSTONE` (23) — the second type with its **own** `+0x14` +/// (`0x5f6990`), alongside FISHINGNODE(17). It never calls `0x5f2f80`: no faction term, no +/// `GAMEOBJECT_FLAGS`, no INTERACT_COND, no DYN_FLAGS. It is exactly +/// `template.data[2] (areaID) != [0xb72038]` — see [`highlightable_flags`]'s `meeting_stone_queued`. +const GO_TYPE_MEETINGSTONE: i32 = 23; + /// The client's GameObject **highlightable** predicate (decision 0243, wow-re cursor-system §4a, /// `0x5f2f80`) over its wire flags: whether the object shows an interact cursor / is clickable at all. /// The types whose vtable constant-falses the slot never get here at all @@ -289,19 +356,33 @@ fn strategy_never_highlightable(type_id: i32) -> bool { /// at exactly it — NOT a `CREATED_BY` compare — then tail-jumps into this shared gate. Someone /// else's bobber (or yours after the channel drops) produces nothing at all: no cursor, no /// tooltip/brighten (the `+0xc` thunk), and a silent dead right-click. Ignored for every other type. +/// +/// `meeting_stone_queued` is MEETINGSTONE(23)'s override (`0x5f6990`, wow-re +/// `go-strategy-vtable-table.md`): the slot is `template.data[2] (areaID) != [0xb72038]`, and +/// `0xb72038` is the area the player is currently queued at — zero-initialized at `0x4c9eec`, +/// written only by the meeting-stone server-message handler `0x4ca230`, and read back by the Lua +/// binding `IsInMeetingStoneQueue`. So a stone is highlightable **unless it is the stone you are +/// already queued at**, and nothing else about it matters. The caller resolves the equality (as it +/// does `channel_owned`); benilla has no queue yet, so it compares against 0 — which is the +/// reference's own not-queued value, not a stand-in for one. Ignored for every other type. fn highlightable_flags( type_id: i32, flags: u32, dyn_flags: u32, reaction: Option, - channel_owned: bool, + overrides: GoOverrides, ) -> bool { if strategy_never_highlightable(type_id) { return false; } - if type_id == GO_TYPE_FISHINGNODE && !channel_owned { + if type_id == GO_TYPE_FISHINGNODE && !overrides.channel_owned { return false; } + // MEETINGSTONE's slot REPLACES the shared gate rather than guarding it — so this returns + // outright and never falls through to the faction / flags / activate terms below. + if type_id == GO_TYPE_MEETINGSTONE { + return !overrides.meeting_stone_queued; + } // The FACTION term (`0x5f2f80` @ `0x5f3026/29`, decision 0764). `reaction` is the GameObject's // reaction **toward us** ([`go_reaction`]); the ordinary test is `> 1`, i.e. anything but // hostile. `None` = unresolvable (no faction catalog / our store not streamed) and passes, so a @@ -372,7 +453,7 @@ pub(crate) fn go_reaction( /// | `[strat_vtbl+0xc]` | types | /// |---|---| /// | `0x5f9db0` = `jmp [+0x14]` = **[`highlightable_flags`] itself** | 0 DOOR · 1 BUTTON · 2 QUESTGIVER · 3 CHEST · 4 BINDER · 6 TRAP · 7 CHAIR · 9 TEXT · 10 GOOBER · 12 AREADAMAGE · 13 CAMERA · 17 FISHINGNODE · 18 RITUAL · 19 MAILBOX · 20 AUCTIONHOUSE · 22 SPELLCASTER · 23 MEETINGSTONE · 24 FLAGSTAND · 26 FLAGDROP · 27 MINI_GAME · 28 LOTTERY_KIOSK · + the default | -/// | `0x5f4830` = **`template.data[1] != 0`** (the vanilla "highlight" column) | 5 GENERIC | +/// | the **highlight column** — `0x5f4830` = `data[1] != 0` / `0x5f6d80` = `data[19] != 0` | 5 GENERIC · 29 CAPTURE_POINT | /// | `mov al,1` — always | 8 SPELL_FOCUS · 16 DUEL_ARBITER · 25 FISHINGHOLE · 30 AURA_GENERATOR | /// | `xor al,al` — never | 11 TRANSPORT · 14 MAP_OBJECT · 15 MO_TRANSPORT | /// @@ -388,15 +469,16 @@ pub(crate) fn go_reaction( /// type-5 templates carry `data1 = 1`); the chest / IN_USE / INTERACT_COND half is wrong, and this /// is why a pre-quest Stone of Binding or a Stratholme portcullis shows nothing in the reference. /// -/// `generic_highlight` is GENERIC's `data[1]`; `None` = its ask-once template hasn't answered, which -/// reads as eligible so a signpost isn't blank for the first frames of its query. +/// `highlight_column` is the template slot the two data-driven types read — GENERIC's `data[1]`, +/// CAPTURE_POINT's `data[19]`. `None` = the ask-once template hasn't answered, which reads as +/// eligible so a signpost isn't blank for the first frames of its query. pub(crate) fn mouseover_eligible( type_id: i32, flags: u32, dyn_flags: u32, - generic_highlight: Option, + highlight_column: Option, reaction: Option, - channel_owned: bool, + overrides: GoOverrides, ) -> bool { match type_id { GO_TYPE_TRANSPORT | GO_TYPE_MAP_OBJECT | GO_TYPE_MO_TRANSPORT => false, @@ -404,8 +486,11 @@ pub(crate) fn mouseover_eligible( | GO_TYPE_DUEL_ARBITER | GO_TYPE_FISHINGHOLE | GO_TYPE_AURA_GENERATOR => true, - GO_TYPE_GENERIC => generic_highlight.unwrap_or(true), - _ => highlightable_flags(type_id, flags, dyn_flags, reaction, channel_owned), + GO_TYPE_GENERIC | GO_TYPE_CAPTURE_POINT => highlight_column.unwrap_or(true), + // Everything else forwards `+0xc` into its own `+0x14` (`0x5f9db0`) — including + // AUCTIONHOUSE(20), whose `+0x14` is const-false, and MEETINGSTONE(23), whose is its own + // predicate. Both fall out of this one arm because the forward is literal. + _ => highlightable_flags(type_id, flags, dyn_flags, reaction, overrides), } } @@ -423,17 +508,39 @@ pub(crate) fn mouseover_eligible( pub(crate) fn go_highlightable( store: &ObjectStore, reaction: Option, - channel_owned: bool, + overrides: GoOverrides, ) -> bool { highlightable_flags( store.0.gameobject_type_id(), store.0.gameobject_flags(), store.0.gameobject_dynamic_flags(), reaction, - channel_owned, + overrides, ) } +/// The area the player is currently queued at through the meeting-stone system — the reference's +/// `[0xb72038]`, which MEETINGSTONE(23)'s highlightable slot compares its `areaID` against. +/// +/// **Zero until benilla carries the meeting-stone queue.** That is not a placeholder: `0xb72038` is +/// zero-initialized (`0x4c9eec`) and written only by the queue's own server-message handler +/// (`0x4ca230`), so 0 *is* the reference's not-queued value and every stone is correctly +/// highlightable. When the queue lands, this becomes its live area and the predicate is already +/// right. +const MEETING_STONE_QUEUED_AREA: u32 = 0; + +/// MEETINGSTONE(23)'s highlightable override, resolved for one GameObject: the reference's +/// `template.data[2] != [0xb72038]`, expressed as its negation so the caller passes the same shape +/// of boolean as [`fishing_channel_owned`]. +/// +/// A template that hasn't answered yet resolves `false` (⇒ highlightable), the same permissive +/// default the highlight column takes — a stone isn't dead for the first frames of its query. Note a +/// stone whose `data[2]` really is **0** is *not* highlightable when we are unqueued, which is the +/// binary's own `0 != 0` and not an edge case worth smoothing away. +pub(crate) fn meeting_stone_queued(area: Option) -> bool { + area.is_some_and(|a| a == MEETING_STONE_QUEUED_AREA) +} + /// The FISHINGNODE highlightable override's input (`0x5f6710`): is the local player currently /// channeling at exactly this GameObject — `self.UNIT_FIELD_CHANNEL_OBJECT == go_guid`. `false` /// when the self store hasn't streamed or the guid is unknown: an unverifiable bobber shows @@ -616,13 +723,18 @@ pub(super) fn classify_cursor( store.0.gameobject_faction(), Some(self_store), ); - let channel_owned = fishing_channel_owned(Some(self_store), hovered_object.guid); - if !go_highlightable(store, reaction, channel_owned) { + // The ask-once template is read BEFORE the gate now: MEETINGSTONE's override is a template + // slot, so the gate itself needs it (the lock cursor below still does too). + let tmpl = hovered_object.guid.and_then(|g| go_inputs.templates.get(g)); + let overrides = GoOverrides { + channel_owned: fishing_channel_owned(Some(self_store), hovered_object.guid), + meeting_stone_queued: meeting_stone_queued(tmpl.and_then(|t| t.meeting_stone_area)), + }; + if !go_highlightable(store, reaction, overrides) { return None; } // The type's own cursor wins; a base type reads its lock's data-named cursor (needs the // ask-once template — a not-yet-answered GO falls back to the gear until it arrives). - let tmpl = hovered_object.guid.and_then(|g| go_inputs.templates.get(g)); let lock_id = tmpl.map_or(0, |t| t.lock_id); let lock_cursor = go_lock_cursor( lock_id, @@ -786,9 +898,16 @@ mod tests { 0, None, None, - false + GoOverrides::default() + )); + assert!(mouseover_eligible( + t, + 0x10, + 0, + None, + None, + GoOverrides::default() )); - assert!(mouseover_eligible(t, 0x10, 0, None, None, false)); } // The three constant-FALSE slots (`xor al,al`) — the transport family is never a mouseover. for t in [GO_TYPE_TRANSPORT, GO_TYPE_MAP_OBJECT, GO_TYPE_MO_TRANSPORT] { @@ -798,7 +917,7 @@ mod tests { GO_DYNFLAG_ACTIVATE, None, None, - false + GoOverrides::default() )); } // GENERIC answers from its template's `data[1]` alone — the signpost that hovers vs the @@ -809,7 +928,7 @@ mod tests { 0, Some(true), None, - false + GoOverrides::default() )); assert!(!mouseover_eligible( GO_TYPE_GENERIC, @@ -817,10 +936,10 @@ mod tests { 0, Some(false), None, - false + GoOverrides::default() )); assert!( - mouseover_eligible(GO_TYPE_GENERIC, 0, 0, None, None, false), + mouseover_eligible(GO_TYPE_GENERIC, 0, 0, None, None, GoOverrides::default()), "template not answered yet reads eligible, so a signpost isn't blank while it queries" ); // Everything else IS highlightable — which is the whole correction. A pre-quest @@ -832,7 +951,7 @@ mod tests { 0, None, None, - false + GoOverrides::default() )); assert!(mouseover_eligible( 0, @@ -840,44 +959,119 @@ mod tests { GO_DYNFLAG_ACTIVATE, None, None, - false + GoOverrides::default() + )); + assert!(!mouseover_eligible( + 3, + 0x10, + 0, + None, + None, + GoOverrides::default() + )); // NO_INTERACT chest + assert!(!mouseover_eligible( + 3, + 0x1, + 0, + None, + None, + GoOverrides::default() + )); // IN_USE chest + // The FACTION term (decision 0764): a door whose template is hostile to us is not + // eligible — no cursor, no tooltip, no brighten. This is Deadmines' Factory Door + // (faction 114, flags 0x20), which the director could still hover and open. + assert!(!mouseover_eligible( + 0, + 0x20, + 0, + None, + Some(1), + GoOverrides::default() + )); + assert!(mouseover_eligible( + 0, + 0x20, + 0, + None, + Some(3), + GoOverrides::default() + )); + assert!(mouseover_eligible( + 0, + 0x20, + 0, + None, + Some(4), + GoOverrides::default() )); - assert!(!mouseover_eligible(3, 0x10, 0, None, None, false)); // NO_INTERACT chest - assert!(!mouseover_eligible(3, 0x1, 0, None, None, false)); // IN_USE chest - // The FACTION term (decision 0764): a door whose template is hostile to us is not - // eligible — no cursor, no tooltip, no brighten. This is Deadmines' Factory Door - // (faction 114, flags 0x20), which the director could still hover and open. - assert!(!mouseover_eligible(0, 0x20, 0, None, Some(1), false)); - assert!(mouseover_eligible(0, 0x20, 0, None, Some(3), false)); - assert!(mouseover_eligible(0, 0x20, 0, None, Some(4), false)); // TRAP inverts it: hostile-to-us is exactly the case a trap DOES highlight for. - assert!(mouseover_eligible(GO_TYPE_TRAP, 0, 0, None, Some(1), false)); + assert!(mouseover_eligible( + GO_TYPE_TRAP, + 0, + 0, + None, + Some(1), + GoOverrides::default() + )); assert!(!mouseover_eligible( GO_TYPE_TRAP, 0, 0, None, Some(3), - false + GoOverrides::default() )); // An unresolvable reaction never blanks the world. - assert!(mouseover_eligible(0, 0x20, 0, None, None, false)); + assert!(mouseover_eligible( + 0, + 0x20, + 0, + None, + None, + GoOverrides::default() + )); // A plain door is still eligible — the everyday case must not regress. - assert!(mouseover_eligible(0, 0, 0, None, None, false)); - assert!(mouseover_eligible(19, 0, 0, None, None, false)); // mailbox + assert!(mouseover_eligible( + 0, + 0, + 0, + None, + None, + GoOverrides::default() + )); + assert!(mouseover_eligible( + 19, + 0, + 0, + None, + None, + GoOverrides::default() + )); // mailbox } #[test] fn highlightable_gates_the_quest_object_but_not_the_plain_door() { // An ordinary unlocked door (no INTERACT_COND) is highlightable regardless of its zero activate // bit — plain doors must never gray out. - assert!(highlightable_flags(0, 0, 0, None, false)); // DOOR, no flags - // A GENERIC decoration is never highlightable. - assert!(!highlightable_flags(GO_TYPE_GENERIC, 0, 0, None, false)); + assert!(highlightable_flags(0, 0, 0, None, GoOverrides::default())); // DOOR, no flags + // A GENERIC decoration is never highlightable. + assert!(!highlightable_flags( + GO_TYPE_GENERIC, + 0, + 0, + None, + GoOverrides::default() + )); // Neither is the transport family (the byte-dumped constant-false +0x14 slots): no gear, // no tooltip, no USE on a boat / elevator / map object — flags can't make them so. for t in [GO_TYPE_TRANSPORT, GO_TYPE_MAP_OBJECT, GO_TYPE_MO_TRANSPORT] { - assert!(!highlightable_flags(t, 0, GO_DYNFLAG_ACTIVATE, None, false)); + assert!(!highlightable_flags( + t, + 0, + GO_DYNFLAG_ACTIVATE, + None, + GoOverrides::default() + )); } // Nor the marker set, whose `+0x14` is the same byte-dumped `xor al,al` — an anvil / forge / // brazier (SPELL_FOCUS, flags 0x0, faction 0 ⇒ NEUTRAL, every flag term passing) is exactly @@ -888,13 +1082,13 @@ mod tests { GO_TYPE_FISHINGHOLE, GO_TYPE_AURA_GENERATOR, ] { - assert!(!highlightable_flags(t, 0, 0, None, false)); + assert!(!highlightable_flags(t, 0, 0, None, GoOverrides::default())); assert!(!highlightable_flags( t, 0, GO_DYNFLAG_ACTIVATE, Some(3), - false + GoOverrides::default() )); } // And the two slots point OPPOSITE ways for the marker set: hovered (tooltip) but never @@ -905,50 +1099,204 @@ mod tests { 0, None, Some(3), - false + GoOverrides::default() )); assert!(!highlightable_flags( GO_TYPE_SPELL_FOCUS, 0, 0, Some(3), - false + GoOverrides::default() + )); + // The DEFAULT strategy — type 21, and everything outside the factory's 0..=30 jump table. + // It shows NOTHING (both slots false), where the flag predicate would have answered `true` + // for exactly the all-zero descriptor an unknown type arrives with. + for t in [GO_TYPE_GUARDPOST, GO_TYPE_MAX + 1, 99, -1] { + assert!(strategy_is_default(t), "type {t} takes the default arm"); + assert!(!highlightable_flags(t, 0, 0, None, GoOverrides::default())); + assert!(!mouseover_eligible( + t, + 0, + 0, + None, + None, + GoOverrides::default() + )); + } + // …and each of the 30 real cases has its own strategy — 21 is the table's only hole. + for t in (0..=GO_TYPE_MAX).filter(|t| *t != GO_TYPE_GUARDPOST) { + assert!(!strategy_is_default(t), "type {t} has its own strategy"); + } + // AUCTIONHOUSE(20) — `+0x14` = `0x5f68a0`, a bare `xor al,al`, and `+0xc` forwards INTO it, + // so the auction-house GameObject shows nothing at all. (The auctioneer you click is a + // unit; this type is not that.) Flags can't make it so. + assert!(!highlightable_flags( + GO_TYPE_AUCTIONHOUSE, + 0, + GO_DYNFLAG_ACTIVATE, + Some(3), + GoOverrides::default() + )); + assert!(!mouseover_eligible( + GO_TYPE_AUCTIONHOUSE, + 0, + 0, + None, + Some(3), + GoOverrides::default() + )); + // CAPTURE_POINT(29) carries GENERIC's law at a different slot: never a cursor, but a + // tooltip iff its highlight column (`data[19]`) is set. The two slots disagree by design, + // exactly as they do for type 5 — so assert BOTH, or the pairing is untested. + assert!(!highlightable_flags( + GO_TYPE_CAPTURE_POINT, + 0, + 0, + Some(3), + GoOverrides::default() + )); + assert!(mouseover_eligible( + GO_TYPE_CAPTURE_POINT, + 0, + 0, + Some(true), + Some(3), + GoOverrides::default() + )); + assert!(!mouseover_eligible( + GO_TYPE_CAPTURE_POINT, + 0, + 0, + Some(false), + Some(3), + GoOverrides::default() )); // A busy (IN_USE) or NO_INTERACT object is not highlightable. - assert!(!highlightable_flags(3, 0x1, 0, None, false)); // CHEST, IN_USE - assert!(!highlightable_flags(3, 0x10, 0, None, false)); // CHEST, NO_INTERACT - // The quest gate: an INTERACT_COND object is highlightable only with the activate bit set — the - // exact bug, a quest chest without the quest. + assert!(!highlightable_flags( + 3, + 0x1, + 0, + None, + GoOverrides::default() + )); // CHEST, IN_USE + assert!(!highlightable_flags( + 3, + 0x10, + 0, + None, + GoOverrides::default() + )); // CHEST, NO_INTERACT + // The quest gate: an INTERACT_COND object is highlightable only with the activate bit set — the + // exact bug, a quest chest without the quest. assert!(!highlightable_flags( 3, GO_FLAG_INTERACT_COND, 0, None, - false + GoOverrides::default() )); // quest chest, no quest → clear assert!(highlightable_flags( 3, GO_FLAG_INTERACT_COND, GO_DYNFLAG_ACTIVATE, None, - false + GoOverrides::default() )); // quest chest, quest held → usable } + /// MEETINGSTONE(23)'s own `+0x14` (`0x5f6990`): `template.data[2] (areaID) != [0xb72038]`, and + /// **nothing else** — it never calls `0x5f2f80`, so none of the shared gate's terms apply. + #[test] + fn the_meeting_stone_answers_only_the_queued_area() { + let queued = GoOverrides { + meeting_stone_queued: true, + ..Default::default() + }; + // Unqueued (the only state benilla can be in today) → highlightable. + assert!(highlightable_flags( + GO_TYPE_MEETINGSTONE, + 0, + 0, + None, + GoOverrides::default() + )); + // Queued at THIS stone → not highlightable: no cursor, and (via the `+0xc` forward) no + // tooltip or brighten either. + assert!(!highlightable_flags( + GO_TYPE_MEETINGSTONE, + 0, + 0, + None, + queued + )); + assert!(!mouseover_eligible( + GO_TYPE_MEETINGSTONE, + 0, + 0, + None, + None, + queued + )); + // The slot REPLACES the shared gate — it does not ride on top of it. Every term that would + // reject any other type is inert here: hostile faction, NO_INTERACT, IN_USE, and an + // INTERACT_COND with no activate bit all still leave an unqueued stone highlightable. + for (flags, dyn_flags, reaction) in [ + (0x10, 0, Some(1)), + (0x1, 0, Some(1)), + (GO_FLAG_INTERACT_COND, 0, Some(1)), + ] { + assert!( + highlightable_flags( + GO_TYPE_MEETINGSTONE, + flags, + dyn_flags, + reaction, + GoOverrides::default() + ), + "flags {flags:#x} must not reach a meeting stone — its slot never calls 0x5f2f80" + ); + } + // The caller's half: the resolved boolean is `data[2] == the queued area`, with an + // unanswered template reading as not-queued so a stone isn't dead while it queries. Our + // queued area is 0 — the reference's own not-queued value — so a stone whose data[2] is + // genuinely 0 is NOT highlightable, which is the binary's `0 != 0` and not a bug. + assert!(!meeting_stone_queued(None)); + assert!(!meeting_stone_queued(Some(1519))); + assert!(meeting_stone_queued(Some(MEETING_STONE_QUEUED_AREA))); + } + #[test] fn the_bobber_is_channel_gated_and_reaches_a_hundred_yards() { // The FISHINGNODE highlightable override (`0x5f6710`, wow-re fishing-bobber-interaction.md): // only the player whose UNIT_FIELD_CHANNEL_OBJECT names exactly this GO passes — someone // else's bobber (or yours after the channel drops) shows nothing at all. - assert!(highlightable_flags(GO_TYPE_FISHINGNODE, 0, 0, None, true)); - assert!(!highlightable_flags(GO_TYPE_FISHINGNODE, 0, 0, None, false)); + assert!(highlightable_flags( + GO_TYPE_FISHINGNODE, + 0, + 0, + None, + GoOverrides { + channel_owned: true, + ..Default::default() + } + )); + assert!(!highlightable_flags( + GO_TYPE_FISHINGNODE, + 0, + 0, + None, + GoOverrides::default() + )); // The pass tail-jumps the SHARED gate, so the flags still apply on top. assert!(!highlightable_flags( GO_TYPE_FISHINGNODE, 0x10, 0, None, - true + GoOverrides { + channel_owned: true, + ..Default::default() + } )); // The mouseover thunk (`+0xc` → `+0x14`) rides the same predicate: no tooltip either. assert!(!mouseover_eligible( @@ -957,7 +1305,7 @@ mod tests { 0, None, None, - false + GoOverrides::default() )); assert!(mouseover_eligible( GO_TYPE_FISHINGNODE, @@ -965,10 +1313,13 @@ mod tests { 0, None, None, - true + GoOverrides { + channel_owned: true, + ..Default::default() + } )); // Every other type ignores the channel input entirely. - assert!(highlightable_flags(0, 0, 0, None, false)); + assert!(highlightable_flags(0, 0, 0, None, GoOverrides::default())); // The per-type range: the bobber's ctor constant is 100.0 yd (squared at the compare) — // effectively un-range-gated — while everything else stays on the interim reach. assert_eq!(go_interact_range_sq(GO_TYPE_FISHINGNODE), 100.0 * 100.0); diff --git a/crates/benilla-app/src/target/hover.rs b/crates/benilla-app/src/target/hover.rs index 600506cf..de44a5f9 100644 --- a/crates/benilla-app/src/target/hover.rs +++ b/crates/benilla-app/src/target/hover.rs @@ -427,14 +427,17 @@ pub(super) fn update_hovered_object( s.0.gameobject_faction(), self_store, ); - let channel_owned = crate::target::cursor_mode::fishing_channel_owned( - self_store, - guids.get(net).ok().map(|g| g.0), - ); + let go_guid = guids.get(net).ok().map(|g| g.0); + let overrides = crate::target::cursor_mode::GoOverrides { + channel_owned: crate::target::cursor_mode::fishing_channel_owned( + self_store, go_guid, + ), + meeting_stone_queued: crate::target::cursor_mode::meeting_stone_queued( + go_guid.and_then(|g| go_templates.get(g)?.meeting_stone_area), + ), + }; u32::from(crate::target::cursor_mode::go_highlightable( - s, - reaction, - channel_owned, + s, reaction, overrides, )) }) }; @@ -472,7 +475,7 @@ pub(super) fn update_hovered_object( // than the old behaviour (which tooltipped the portcullis itself) and documented rather than // silently accepted; closing it wants the single-pick arbitration, which is its own slice. if let Ok(store) = stores.get(net_entity) { - let generic_highlight = go_templates.get(guid.0).map(|t| t.generic_highlight); + let tmpl = go_templates.get(guid.0); let reaction = crate::target::cursor_mode::go_reaction( factions.as_deref(), store.0.gameobject_faction(), @@ -482,9 +485,17 @@ pub(super) fn update_hovered_object( store.0.gameobject_type_id(), store.0.gameobject_flags(), store.0.gameobject_dynamic_flags(), - generic_highlight, + tmpl.map(|t| t.highlight_column), reaction, - crate::target::cursor_mode::fishing_channel_owned(self_store, Some(guid.0)), + crate::target::cursor_mode::GoOverrides { + channel_owned: crate::target::cursor_mode::fishing_channel_owned( + self_store, + Some(guid.0), + ), + meeting_stone_queued: crate::target::cursor_mode::meeting_stone_queued( + tmpl.and_then(|t| t.meeting_stone_area), + ), + }, ) { return; } diff --git a/crates/benilla-app/src/ui_action/cast_send.rs b/crates/benilla-app/src/ui_action/cast_send.rs index 91616572..534d2756 100644 --- a/crates/benilla-app/src/ui_action/cast_send.rs +++ b/crates/benilla-app/src/ui_action/cast_send.rs @@ -247,7 +247,7 @@ fn send_spell_cast( // special branch (wow-re `wave-cast.md`, VERIFIED) — an `Effect[0] == SPELL_EFFECT_TRADE_SKILL` // cast NEVER reaches the wire; the crafting book opens client-side instead. Before the // cooldown ladder, exactly where the client dispatches it (`6e4bce`, ahead of every gate). - if def.is_some_and(|d| d.effect_1 == benilla_formats::SPELL_EFFECT_TRADE_SKILL) { + if def.is_some_and(|d| d.effects[0] == benilla_formats::SPELL_EFFECT_TRADE_SKILL) { debug!("ui_action: cast {spell_id} is a profession opener — the crafting book opens, no packet"); trade_skill_opens.0.push(spell_id); return; diff --git a/crates/benilla-app/src/ui_action/mod.rs b/crates/benilla-app/src/ui_action/mod.rs index 7fb2265e..2023d800 100644 --- a/crates/benilla-app/src/ui_action/mod.rs +++ b/crates/benilla-app/src/ui_action/mod.rs @@ -227,11 +227,12 @@ fn track_learned_abilities( return; } let first_with = |effect: u32| { - actions - .spells - .iter() - .copied() - .find(|&id| spells.catalog.get(id).is_some_and(|d| d.effect_1 == effect)) + actions.spells.iter().copied().find(|&id| { + spells + .catalog + .get(id) + .is_some_and(|d| d.effects[0] == effect) + }) }; let (skinning, skin_player_corpse, feed_pet) = ( first_with(benilla_formats::SPELL_EFFECT_SKINNING), diff --git a/crates/benilla-app/src/ui_action/targeting/item.rs b/crates/benilla-app/src/ui_action/targeting/item.rs index b45c582b..c6bd6fc2 100644 --- a/crates/benilla-app/src/ui_action/targeting/item.rs +++ b/crates/benilla-app/src/ui_action/targeting/item.rs @@ -108,8 +108,8 @@ pub(crate) fn item_bind_verdict( new: Option, confirmed: bool, ) -> ItemBind { - let perm = def.effect_1 == benilla_formats::SPELL_EFFECT_ENCHANT_ITEM; - if !perm && def.effect_1 != benilla_formats::SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY { + let perm = def.effects[0] == benilla_formats::SPELL_EFFECT_ENCHANT_ITEM; + if !perm && def.effects[0] != benilla_formats::SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY { return ItemBind::Bind; } if def.equipped_item_subclass_mask != 0 { @@ -355,7 +355,7 @@ mod tests { // Enchant Bracer - Minor Health (7418): armor, any subclass, WRIST only. let bracer = SpellDisplay { - effect_1: ENCHANT, + effects: [ENCHANT, 0, 0], equipped_item_class: CLASS_ARMOR as i32, equipped_item_subclass_mask: 0x1f, equipped_item_inventory_type_mask: 1 << INVTYPE_WRIST, @@ -379,7 +379,7 @@ mod tests { // Instant Poison (8679): weapon class, a subclass mask, NO inventory-type requirement. let poison = SpellDisplay { - effect_1: benilla_formats::SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY, + effects: [benilla_formats::SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY, 0, 0], equipped_item_class: CLASS_WEAPON as i32, equipped_item_subclass_mask: 0x2a5f3, equipped_item_inventory_type_mask: 0, @@ -405,7 +405,7 @@ mod tests { // its loop, finds no 53/54 arm, and falls straight through to the bind — anything goes, // and the server judges. let disenchant = SpellDisplay { - effect_1: 99, + effects: [99, 0, 0], equipped_item_class: -1, ..Default::default() }; @@ -443,7 +443,7 @@ mod tests { // A permanent weapon enchant whose row binds the item (the ZG/imbue family's `Flags & 1`). let perm = SpellDisplay { - effect_1: ENCHANT, + effects: [ENCHANT, 0, 0], effect_misc_value: [1900, 0, 0], ..Default::default() }; @@ -525,7 +525,7 @@ mod tests { // 3 · The slot fork (`495ed3: cmpl $0x35 / setne`) — the ONE place effect 53 and 54 // diverge. A permanent enchant asks about PERM; a poison asks about TEMP. let temp_spell = SpellDisplay { - effect_1: TEMP, + effects: [TEMP, 0, 0], effect_misc_value: [1900, 0, 0], ..Default::default() }; @@ -554,7 +554,7 @@ mod tests { // 4 · The equip gate still runs FIRST — a refusal beats both confirms (495e10 < 495e93). let bracer_only = SpellDisplay { - effect_1: ENCHANT, + effects: [ENCHANT, 0, 0], effect_misc_value: [1900, 0, 0], equipped_item_class: 4, equipped_item_subclass_mask: 0x1f, diff --git a/crates/benilla-app/src/ui_action/usable.rs b/crates/benilla-app/src/ui_action/usable.rs index ccb2fffd..c0c83671 100644 --- a/crates/benilla-app/src/ui_action/usable.rs +++ b/crates/benilla-app/src/ui_action/usable.rs @@ -89,7 +89,7 @@ pub(crate) fn spell_usable( commands: &NetCommands, ) -> (bool, bool) { // Early-out (`0x6e3d99`): a tradeskill "spell" is always usable. - if d.effect_1 == SPELL_EFFECT_TRADE_SKILL { + if d.effects[0] == SPELL_EFFECT_TRADE_SKILL { return (true, false); } // Leg 1: dead casters use nothing without the castable-while-dead attribute. @@ -384,7 +384,7 @@ mod tests { // The early-out beats every gate. let tradeskill = SpellDisplay { - effect_1: SPELL_EFFECT_TRADE_SKILL, + effects: [SPELL_EFFECT_TRADE_SKILL, 0, 0], mana_cost: 9999, stances: 0x1, ..Default::default() diff --git a/crates/benilla-app/src/ui_craft.rs b/crates/benilla-app/src/ui_craft.rs index c65dbec5..9d35b063 100644 --- a/crates/benilla-app/src/ui_craft.rs +++ b/crates/benilla-app/src/ui_craft.rs @@ -19,9 +19,7 @@ use bevy::prelude::*; -use benilla_formats::{ - SPELL_EFFECT_CREATE_ITEM, SPELL_EFFECT_ENCHANT_ITEM, SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY, -}; +use benilla_formats::{SPELL_EFFECT_ENCHANT_ITEM, SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY}; use benilla_protocol::messages::PLAYER_SKILL_SLOTS; use benilla_ui::script::{CraftReagent, CraftRecipe, CraftState, UiScript}; @@ -29,7 +27,7 @@ use crate::entities::ItemDisplays; use crate::items::Items; use crate::net::{NetCommands, ObjectStore, SelfPlayer}; use crate::ui_action::{cast_target, CastCommit, CastLadder, PlayerActions, Spells}; -use crate::ui_items::count_of; +use crate::ui_items::{count_of, item_icon}; use crate::ui_script::UiInput; use crate::ui_spellbook::SkillLines; use crate::ui_tradeskill::SpellFocus; @@ -84,6 +82,21 @@ fn skill_rank(store: &ObjectStore, skill_id: u32) -> (u32, u32, i32) { (0, 0, 0) } +/// **Law D** — the Craft window's row icon, transcribing `GetCraftIcon 0x4f7160` (byte-VERIFIED; +/// wow-re `ui/scratch/spell-icon-substitution-law.md` §2, folded back by decision 1107): **always** +/// this recipe's own `SpellIconID`, straight off `Spell.dbc`. +/// +/// The one-liner is the point, and it is not an oversight to be "improved". The Craft window and +/// the TradeSkill window are two translation units of the *same* node, and their icon laws are +/// exact opposites: `+0x19c` (`EffectItemType[0]`) is never read anywhere in `[0x4f7160, +/// 0x4f7204]` — no item lookup, no `"%s%s%s"` path composition, no async callback. So a rod-making +/// `CREATE_ITEM` recipe fronts the *spell's* icon here while the byte-identical recipe in the +/// TradeSkill window fronts the *rod's* ([`crate::ui_tradeskill`]'s Law C). Substituting the +/// product here — which this used to do — is wrong on this surface with no diff to catch it. +fn craft_icon(d: &benilla_formats::SpellDisplay) -> Option { + d.icon.clone() +} + /// Build the craft snapshot — `None` when the window is closed or the catalogs haven't loaded. #[allow(clippy::too_many_arguments)] // a Bevy system's full input set (the feed precedent) fn feed_craft( @@ -126,7 +139,7 @@ fn feed_craft( .filter(|&&s| { spells.catalog.get(s).is_some_and(|d| { (d.cast_ui != 0 || d.attributes & ATTR_IS_TRADESKILL != 0) - && d.effect_1 != benilla_formats::SPELL_EFFECT_TRADE_SKILL + && d.effects[0] != benilla_formats::SPELL_EFFECT_TRADE_SKILL }) && skill_lines.catalog.spell_to_line(s) == Some(line) }) .filter_map(|&s| { @@ -136,13 +149,13 @@ fn feed_craft( let mut num_available = u32::MAX; for &(entry, need) in d.reagents.iter().filter(|&&(e, n)| e != 0 && n != 0) { let have = count_of(&store.0, &items, entry); + // A reagent is an ITEM row, so it terminates in the one genuinely shared chain + // (wow-re §5): ItemTemplate → ItemDisplayInfo → icon. Unlike the *recipe* icon + // above, there is nothing per-binding about this one. let (name, icon) = match items.template(entry, 0, &commands) { Some(t) => ( Some(t.name.clone()), - icons - .as_deref() - .and_then(|i| i.catalog.get(t.display_info_id)) - .and_then(|di| di.icon.clone()), + item_icon(icons.as_deref(), t.display_info_id), ), None => (None, None), }; @@ -174,25 +187,10 @@ fn feed_craft( } } let needs_item_target = matches!( - d.effect_1, + d.effects[0], SPELL_EFFECT_ENCHANT_ITEM | SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY ); - // A rod-making recipe (CREATE_ITEM) fronts the product's icon like a tradeskill - // row; an enchant fronts its spell icon. - let icon = (d.effect_1 == SPELL_EFFECT_CREATE_ITEM && d.effect_item_type[0] != 0) - .then(|| { - items - .template(d.effect_item_type[0], 0, &commands) - .map(|t| t.display_info_id) - .and_then(|di| { - icons - .as_deref() - .and_then(|i| i.catalog.get(di)) - .and_then(|x| x.icon.clone()) - }) - }) - .flatten() - .or_else(|| d.icon.clone()); + let icon = craft_icon(d); Some(CraftRecipe { spell_id: s, name: d.name.clone(), @@ -285,3 +283,36 @@ fn drain_craft( open.line = None; } } + +#[cfg(test)] +mod tests { + use super::*; + use benilla_formats::{SpellDisplay, SPELL_EFFECT_CREATE_ITEM}; + + /// Law D never looks at an item. The regression this pins is the *sibling* one: a rod-making + /// `CREATE_ITEM` recipe — the exact case our old code substituted the product for — still + /// fronts the spell's own icon here, because `EffectItemType[0]` is never read anywhere in + /// `GetCraftIcon`'s extent. Craft and TradeSkill disagree on purpose. + #[test] + fn law_d_is_always_the_spells_own_icon_even_for_a_rod_recipe() { + let rod = SpellDisplay { + name: "Runed Copper Rod".into(), + icon: Some("SPELL".into()), + effects: [SPELL_EFFECT_CREATE_ITEM, 0, 0], + effect_item_type: [6218, 0, 0], + ..Default::default() + }; + assert_eq!(craft_icon(&rod), Some("SPELL".into())); + } + + /// A recipe with no `SpellIconID` row resolved is nil — the binding's `lua_pushnil` return. + /// There is no item arm to fall through to. + #[test] + fn law_d_is_nil_when_the_spell_carries_no_icon() { + let d = SpellDisplay { + effect_item_type: [6218, 0, 0], + ..Default::default() + }; + assert_eq!(craft_icon(&d), None); + } +} diff --git a/crates/benilla-app/src/ui_duel.rs b/crates/benilla-app/src/ui_duel.rs index 170aedaf..322726e9 100644 --- a/crates/benilla-app/src/ui_duel.rs +++ b/crates/benilla-app/src/ui_duel.rs @@ -410,7 +410,7 @@ fn duel_spell( spells .catalog .get(*id) - .is_some_and(|s| s.effect_1 == SPELL_EFFECT_DUEL) + .is_some_and(|s| s.effects[0] == SPELL_EFFECT_DUEL) }) } @@ -498,7 +498,7 @@ mod tests { let catalog = benilla_formats::load_spell_catalog(&mut chain).expect("Spell.dbc"); let mut hits: Vec = catalog .iter() - .filter(|(_, s)| s.effect_1 == SPELL_EFFECT_DUEL) + .filter(|(_, s)| s.effects[0] == SPELL_EFFECT_DUEL) .map(|(id, _)| id) .collect(); hits.sort_unstable(); diff --git a/crates/benilla-app/src/ui_item_text.rs b/crates/benilla-app/src/ui_item_text.rs index a5d3124c..4de84825 100644 --- a/crates/benilla-app/src/ui_item_text.rs +++ b/crates/benilla-app/src/ui_item_text.rs @@ -1,37 +1,106 @@ -//! The item-text reader session (`ItemTextFrame.xml`) — reading a bag letter (a mail-made -//! permanent copy, `ITEM_FIELD_ITEM_TEXT_ID != 0`) in the reference reader window. +//! The item-text reader session (`ItemTextFrame.xml`) — the one window every *readable* opens in: +//! a bag letter, a book in your bags, and a book/plaque lying in the world. //! -//! The right-click routing lives in `ui_items::drain::drain_container_uses`: a bag item whose -//! *instance* carries an item-text id opens a read here instead of sending `CMSG_USE_ITEM` (the -//! real client's fork — vmangos would refuse a `CMSG_READ_ITEM` for it anyway, since that opcode's -//! handler gates on the *template*'s `PageText`, and the Plain Letter template's is 0). The text -//! rides the same ask-once `CMSG_ITEM_TEXT_QUERY` cache mail letter bodies use -//! ([`crate::ui_mail::MailOpen::bodies`] — that map is the client's item-text cache, mail is just -//! its first tenant); the creator line resolves `ITEM_FIELD_CREATOR` through the name cache. +//! **One reader, keyed on an object guid, opened locally with no permission packet.** That is the +//! reference's own shape, byte-verified (decision 1105): every route ends at `0x4e32e0(guid, flag)`, +//! which looks the guid up with typemask `1` (*any* object — item or GameObject), asks the object +//! for its page id through the shared `vtbl+0x74` getter, and pulls the text out of a cache. Which +//! of the two text sources applies is the object's own answer, not the caller's: +//! +//! - **a letter** — an item *instance* carrying `ITEM_FIELD_ITEM_TEXT_ID` (a mail-made permanent +//! copy). One body, no pages; it rides the ask-once `CMSG_ITEM_TEXT_QUERY` cache mail letters use +//! ([`crate::ui_mail::MailOpen::bodies`] — that map is the client's item-text cache, mail is just +//! its first tenant), and its creator line resolves `ITEM_FIELD_CREATOR` through the name cache. +//! - **a page chain** — a readable item *template*'s `PageText`, or a `GAMEOBJECT_TYPE_TEXT` world +//! object's template `data[0]`. Pages chain by `nextPageId` over the ask-once +//! [`PageTexts`] cache (`CMSG_PAGE_TEXT_QUERY`), and the reader's prev/next buttons walk them. +//! Authorless — a book has no `From,` tail. +//! +//! The routes in: `ui_items::drain::drain_container_uses` (the bag click's dispatcher arms #5 and +//! #6) and `target::click::act_on_right_click` (the world right-click's GO type-9 arm). None of +//! them sends a use/read packet — vmangos has no `GameObject::Use` case for type 9 at all, and its +//! `CMSG_READ_ITEM` handler gates on a template `PageText` the Plain Letter doesn't have, so both +//! would be answered with silence even if we asked. //! //! The feed mirrors the reference event flow (ItemTextFrame.lua l.10-96): open → //! `ITEM_TEXT_BEGIN` (title known), everything fetched → `ITEM_TEXT_READY`, `CloseItemText()` → //! session cleared + `ITEM_TEXT_CLOSED`. `ITEM_TEXT_TRANSLATION` (the foreign-language progress -//! bar) never fires — no translation mechanic on a private server's letters. Multi-page -//! `PageText` books (`CMSG_READ_ITEM` / `SMSG_READ_ITEM_OK` + the page chain) are a named -//! follow-up — the page-turn intents drain here and are dropped. +//! bar) never fires — no translation mechanic on a private server's readables, and the `language` +//! column (`data[1]`/`LanguageID`) is carried by neither source here. + +use std::collections::{HashMap, HashSet}; use bevy::prelude::*; use benilla_ui::script::{ItemTextState, UiScript}; +use crate::go_templates::GameObjectTemplates; use crate::items::Items; use crate::names::NameCache; use crate::net::{ClientCommand, NetCommands}; use crate::ui_mail::MailOpen; use crate::ui_script::UiInput; +/// One page of a book as the wire gave it (`SMSG_PAGE_TEXT_QUERY_RESPONSE`). +pub(crate) struct PageText { + pub(crate) text: String, + /// The next page's id; `0` = this is the last page. + pub(crate) next: u32, +} + +/// The ask-once page cache (decision 1105) — the client's `PageText` dbcache (`0xc0e174`). Keyed by +/// page id, filled by [`crate::net::apply`]'s `SessionEvent::PageText` arm. +/// +/// vmangos answers a single `CMSG_PAGE_TEXT_QUERY` with the **whole chain** (one response per page), +/// so asking for page 1 normally lands the entire book at once and a page turn is a cache hit. The +/// per-page fetch is still here for the server that answers one at a time. +#[derive(Resource, Default)] +pub(crate) struct PageTexts { + pages: HashMap, + pending: HashSet, +} + +impl PageTexts { + /// Record a page (and clear its in-flight flag). + pub(crate) fn insert(&mut self, page_id: u32, text: String, next: u32) { + self.pending.remove(&page_id); + self.pages.insert(page_id, PageText { text, next }); + } + + /// The cached page, asking for it once if this is the first asker. `guid` names the object + /// doing the reading (the reference writes it after the page id; vmangos discards it). + fn get_or_ask(&mut self, page_id: u32, guid: u64, commands: &NetCommands) -> Option<&PageText> { + if !self.pages.contains_key(&page_id) && self.pending.insert(page_id) { + debug!("page text: asking page {page_id} (object {guid:#x})"); + let _ = commands + .0 + .send(ClientCommand::PageTextQuery { page_id, guid }); + } + self.pages.get(&page_id) + } +} + +/// Which text a read session is showing — the reference's two sources, which it picks between by +/// asking the *object* (an instance text id, else a page id), not by how the click arrived. +pub(crate) enum ReadSource { + /// An item instance's `ITEM_FIELD_ITEM_TEXT_ID` — a mail-made letter. Single body, has a + /// creator line. + Letter { text_id: u32 }, + /// A page chain — a readable item template's `PageText`, or a TEXT GameObject's `data[0]`. + /// `visited` is the trail of page ids from the first to the one on screen, so **Prev** can walk + /// back a chain that only links forwards (the reference keeps the same array, `[0xbc3fd0]`). + /// **Empty until the object's template answers**: the reference asks the *object* for its page + /// id at paint time (`vtbl+0x74`, with an ask-once template callback that re-enters the whole + /// open — `0x5f59d0`), so a click that lands before the template does still opens a reader. + Pages { visited: Vec }, +} + /// The open read session; `None` in [`ItemTextOpen::pending`] = no reader open. pub(crate) struct ReadSession { - /// The read item's object guid — title/creator resolve off it each frame until ready. - pub(crate) item_guid: u64, - /// `ITEM_FIELD_ITEM_TEXT_ID` — the ask-once cache key. - pub(crate) text_id: u32, + /// The read object's guid — an item's or a GameObject's. Title/creator resolve off it each + /// frame until ready, and re-using the *same* object closes the reader (the reference's toggle). + pub(crate) object_guid: u64, + pub(crate) source: ReadSource, /// `ITEM_TEXT_BEGIN` fired (the reference fires it once per open, before the text lands). begun: bool, /// `ITEM_TEXT_READY` fired — the session is fully painted. @@ -42,15 +111,55 @@ pub(crate) struct ReadSession { #[derive(Resource, Default)] pub(crate) struct ItemTextOpen { pub(crate) pending: Option, + /// A [`ItemTextOpen::toggle_closed`] fired and the frame has not been told yet. The click + /// routes have no `UiScript` in reach, so the close event is handed to the feed — which owns + /// the script — rather than each caller re-implementing it. + closing: bool, } impl ItemTextOpen { - /// Open a read for a bag item (the right-click route in `drain_container_uses`). Re-opening - /// (same or another letter) restarts the reference event flow from `ITEM_TEXT_BEGIN`. - pub(crate) fn open(&mut self, item_guid: u64, text_id: u32) { + /// Open a read for a bag letter (dispatcher arm #6). Re-opening restarts the reference event + /// flow from `ITEM_TEXT_BEGIN`. + pub(crate) fn open_letter(&mut self, item_guid: u64, text_id: u32) { + self.open(item_guid, ReadSource::Letter { text_id }); + } + + /// Open a read for a page chain — a readable item template (arm #5) or a TEXT GameObject. The + /// page head is not passed in: the feed asks the object's template for it, like the reference. + pub(crate) fn open_pages(&mut self, object_guid: u64) { + self.open( + object_guid, + ReadSource::Pages { + visited: Vec::new(), + }, + ); + } + + /// The reference's **toggle** (`0x4e32e0`'s `arg2 == 0` head, which *every* click route passes — + /// the bag readable at `0x5d8e5e` and the TEXT GameObject at `0x5f58e7` both `xor edx,edx`): + /// re-clicking the readable whose reader is already open *closes* it. Returns whether it fired, + /// so the caller stops there. + pub(crate) fn toggle_closed(&mut self, object_guid: u64) -> bool { + let open = self + .pending + .as_ref() + .is_some_and(|s| s.object_guid == object_guid); + if open { + self.pending = None; + self.closing = true; + } + open + } + + /// Take the pending toggle-close, for the feed to turn into `ITEM_TEXT_CLOSED`. + fn take_closing(&mut self) -> bool { + std::mem::take(&mut self.closing) + } + + fn open(&mut self, object_guid: u64, source: ReadSource) { self.pending = Some(ReadSession { - item_guid, - text_id, + object_guid, + source, begun: false, ready: false, }); @@ -61,29 +170,76 @@ pub(crate) struct UiItemTextPlugin; impl Plugin for UiItemTextPlugin { fn build(&self, app: &mut App) { - app.init_resource::().add_systems( - Update, - ( - // Feed before the input pass so an open paints the same frame; drain after it so - // a Close click clears the same frame (the ui_mail ordering). - feed_item_text.before(UiInput), - drain_item_text.after(UiInput), - ), - ); + app.init_resource::() + .init_resource::() + .add_systems( + Update, + ( + // Feed before the input pass so an open paints the same frame; drain after it so + // a Close click clears the same frame (the ui_mail ordering). + feed_item_text.before(UiInput), + drain_item_text.after(UiInput), + ), + ); } } -/// Drive the open session to `ITEM_TEXT_BEGIN`/`ITEM_TEXT_READY` as its pieces land: the title -/// (item template name), the body (the ask-once item-text cache), and the creator name (name -/// cache). BEGIN holds until the title resolves (the template is all but always cached — the bag -/// needed it for the icon); READY holds until the body and the creator line are both in. +/// What the reader's *object* answers, whichever source its text comes from: the window title +/// (`ItemTextGetItem 0x4e38f0` → the looked-up object's `vtbl+0x70` name) and the frame material +/// (`ItemTextGetMaterial 0x4e39f0` → an item template's `PageMaterial`, a GameObject's `data[2]`), +/// plus the page head a page chain starts at. `None` = the template is still in flight. +struct Readable { + title: String, + /// `PageTextMaterial.dbc` id → basename; `None` = the Lua's Parchment default. + material: Option, + /// The first `PageText` id — `0` for a letter (which has no page chain) and for a readable + /// whose template carries no page. + page_head: u32, +} + +/// Resolve the object's title + material + page head, whether it is an item or a GameObject. The +/// reference does exactly this — one guid lookup with typemask 1, then virtual getters — so a +/// single resolve serves both sources. +fn readable( + guid: u64, + items: &mut Items, + go_templates: &GameObjectTemplates, + materials: Option<&PageMaterials>, + commands: &NetCommands, +) -> Option { + let name = |id: u32| materials.and_then(|m| m.0.name(id)).map(str::to_string); + if let Some(go) = go_templates.get(guid) { + return Some(Readable { + title: go.name.clone(), + material: go.text_page.and_then(|p| name(p.material)), + page_head: go.text_page.map_or(0, |p| p.page_id), + }); + } + let entry = items.object(guid)?.object_entry()?; + let t = items.template(entry, guid, commands)?; + Some(Readable { + title: t.name.clone(), + material: name(t.page_material), + page_head: t.page_text, + }) +} + +/// Drive the open session to `ITEM_TEXT_BEGIN`/`ITEM_TEXT_READY` as its pieces land. BEGIN holds +/// until the object's template resolves — it carries the title *and* the material, and the Lua's +/// BEGIN handler picks the page font and text colour off the material, so firing it early would +/// paint a Stone plaque in parchment ink. READY holds until the body and, for a letter, the creator +/// line are both in. (The window itself only shows on READY — `ItemTextFrame.lua` calls +/// `ShowUIPanel` there, not on BEGIN.) #[allow(clippy::too_many_arguments)] fn feed_item_text( script: Option>, mut open: ResMut, mut mail: ResMut, + mut pages: ResMut, mut items: ResMut, mut names: ResMut, + go_templates: Res, + materials: Option>, commands: Res, // The `$`-macro subject for the page body: the local player, as at every panel seam. self_q: Query<(&crate::net::ObjectStore, &crate::net::Guid), With>, @@ -92,61 +248,97 @@ fn feed_item_text( let Some(mut script) = script else { return; }; + // A re-click closed the reader (the reference's toggle → its `0x128`). The click route cleared + // the session; the frame is told here, where the script is. + if open.take_closing() { + script.set_item_text(None); + script.fire_event("ITEM_TEXT_CLOSED", vec![]); + } let Some(sess) = open.pending.as_mut() else { return; }; if sess.ready { return; } - - // The title: the item object's entry → template name. - let title = items - .object(sess.item_guid) - .and_then(|o| o.object_entry()) - .and_then(|entry| { - items - .template(entry, sess.item_guid, &commands) - .map(|t| t.name.clone()) - }); - let Some(title) = title else { - return; // template in flight — BEGIN next frame + let Some(readable) = readable( + sess.object_guid, + &mut items, + &go_templates, + materials.as_deref(), + &commands, + ) else { + return; // template in flight — the reference re-enters the whole open when it lands }; - // The creator line: `ITEM_FIELD_CREATOR` → name cache (ask-once). `None` guid = authorless. - let creator_guid = items.object(sess.item_guid).and_then(|o| o.item_creator()); - let creator = creator_guid.map(|g| names.resolve(g, &commands).map(str::to_string)); - - // The body: the shared ask-once item-text cache (fetch if this is the first asker). - let body = mail.bodies.get(&sess.text_id).cloned(); - if body.is_none() && mail.pending_bodies.insert(sess.text_id) { - let _ = commands.0.send(ClientCommand::ItemTextQuery { - text_id: sess.text_id, - mail_id: 0, - }); + // A page read whose template answers with no page has nothing to show — the reference bails + // before firing anything (`0x4e341d`, both text sources zero). Drop the session silently. + if matches!(sess.source, ReadSource::Pages { .. }) && readable.page_head == 0 { + debug!( + "item text: {:#x} has no page to read — closing", + sess.object_guid + ); + open.pending = None; + return; } if !sess.begun { script.set_item_text(Some(ItemTextState { - item: title.clone(), + item: readable.title.clone(), creator: None, text: String::new(), page: 1, has_next: false, - material: None, + material: readable.material.clone(), })); script.fire_event("ITEM_TEXT_BEGIN", vec![]); sess.begun = true; } - // READY once the body is in and the creator (if any) resolved. - let creator = match creator { - None => None, // authorless - Some(Some(name)) => Some(name), // resolved - Some(None) => return, // name query in flight - }; - let Some(text) = body else { - return; // body query in flight + let (creator, text, page, has_next) = match &mut sess.source { + ReadSource::Letter { text_id } => { + // The creator line: `ITEM_FIELD_CREATOR` → name cache (ask-once). `None` guid = + // authorless. + let creator_guid = items + .object(sess.object_guid) + .and_then(|o| o.item_creator()); + let creator = match creator_guid { + None => None, // authorless + Some(guid) => match names.resolve(guid, &commands) { + Some(name) => Some(name.to_string()), + None => return, // name query in flight + }, + }; + // The body: the shared ask-once item-text cache (fetch if this is the first asker). + let body = mail.bodies.get(text_id).cloned(); + if body.is_none() && mail.pending_bodies.insert(*text_id) { + let _ = commands.0.send(ClientCommand::ItemTextQuery { + text_id: *text_id, + mail_id: 0, + }); + } + let Some(text) = body else { + return; // body query in flight + }; + (creator, text, 1, false) + } + ReadSource::Pages { visited } => { + if visited.is_empty() { + visited.push(readable.page_head); + } + let page_id = *visited.last().expect("seeded just above"); + let Some(page) = pages.get_or_ask(page_id, sess.object_guid, &commands) else { + return; // page query in flight + }; + // A book is authorless — the creator leg is the reference's item-instance one. + ( + None, + page.text.clone(), + visited.len() as u32, + page.next != 0, + ) + } }; + // Page/book text is server-authored, so it runs the `$`-macro expander — the reference does it // from two sites in `ItemTextFrame.cpp` (decision 0754), subject = the local player. let subject = crate::npc_text::player_identity(&self_q, &mut names, &commands); @@ -158,27 +350,139 @@ fn feed_item_text( }, ); script.set_item_text(Some(ItemTextState { - item: title, + item: readable.title, creator, text, - page: 1, - has_next: false, - material: None, + page, + has_next, + material: readable.material, })); script.fire_event("ITEM_TEXT_READY", vec![]); sess.ready = true; } +/// `PageTextMaterial.dbc` as a resource (decision 1105) — the reader frame's material basename, +/// loaded once at startup ([`crate::entities`]); absent when the client data is, in which case every +/// readable falls to the Lua's Parchment default. +#[derive(Resource)] +pub(crate) struct PageMaterials(pub(crate) benilla_formats::PageTextMaterialCatalog); + /// Drain the reader's intents: `CloseItemText()` clears the session (+ `ITEM_TEXT_CLOSED`, the -/// reference C-side answer the frame's OnHide relies on); page turns are dropped (single-page -/// letters — the buttons never show; books are the named follow-up). -fn drain_item_text(script: Option>, mut open: ResMut) { +/// reference C-side answer the frame's OnHide relies on); a page turn walks the chain — Next +/// appends the current page's `nextPageId` to the trail, Prev pops back — and re-runs the feed from +/// `ITEM_TEXT_READY` (the reference repaints the same frame, it does not re-fire BEGIN). +fn drain_item_text( + script: Option>, + mut open: ResMut, + pages: Res, +) { let Some(mut script) = script else { return; }; - let _ = script.take_item_text_page_turns(); + for delta in script.take_item_text_page_turns() { + let Some(sess) = open.pending.as_mut() else { + continue; + }; + let ReadSource::Pages { visited } = &mut sess.source else { + continue; // a letter has no pages; its buttons never show + }; + if turn_page(visited, delta, |id| pages.pages.get(&id).map(|p| p.next)) { + sess.ready = false; // repaint on the next feed, without re-firing BEGIN + } + } if script.take_item_text_close() && open.pending.take().is_some() { script.set_item_text(None); script.fire_event("ITEM_TEXT_CLOSED", vec![]); } } + +/// Walk the visited-page trail one step. A `PageText` chain only links **forwards**, so Prev is a +/// pop off the trail rather than a lookup — the reference keeps the same array (`[0xbc3fd0]`). +/// Returns whether the page actually changed (a no-op turn must not repaint). +/// +/// `next_of` is the page cache: `None` = the page hasn't landed yet, `Some(0)` = last page. Both +/// refuse the turn; neither can normally be clicked, since the Next button only shows while the +/// painted page reported a next. +fn turn_page(visited: &mut Vec, delta: i32, next_of: impl Fn(u32) -> Option) -> bool { + let Some(¤t) = visited.last() else { + return false; // head not resolved yet — no page is painted to turn from + }; + if delta > 0 { + match next_of(current) { + Some(next) if next != 0 => visited.push(next), + _ => return false, + } + } else if visited.len() > 1 { + visited.pop(); + } else { + return false; // already on page 1 + } + true +} + +#[cfg(test)] +mod tests { + use super::*; + + /// A three-page book: 100 → 101 → 102. Next walks it, Prev walks back, and the ends refuse. + #[test] + fn page_turns_walk_the_chain_both_ways() { + let chain = |id: u32| match id { + 100 => Some(101), + 101 => Some(102), + 102 => Some(0), // last page + _ => None, + }; + let mut visited = vec![100]; + assert!(!turn_page(&mut visited, -1, chain), "page 1 has no Prev"); + assert_eq!(visited, [100]); + + assert!(turn_page(&mut visited, 1, chain)); + assert!(turn_page(&mut visited, 1, chain)); + assert_eq!(visited, [100, 101, 102]); + assert!( + !turn_page(&mut visited, 1, chain), + "the last page has no Next" + ); + + assert!(turn_page(&mut visited, -1, chain)); + assert_eq!(visited, [100, 101], "Prev pops the forward-only trail"); + } + + /// A Next clicked before the page landed is refused rather than pushing a bogus id. + #[test] + fn a_page_still_in_flight_refuses_the_turn() { + let mut visited = vec![7]; + assert!(!turn_page(&mut visited, 1, |_| None)); + assert_eq!(visited, [7]); + } + + /// The reference's toggle: re-clicking the readable that is already open closes it; a + /// *different* one does not. + #[test] + fn re_reading_the_same_object_toggles_closed() { + let mut open = ItemTextOpen::default(); + open.open_pages(0xF110_0000_0000_0001); + assert!(!open.toggle_closed(0xF110_0000_0000_0002), "another book"); + assert!(open.pending.is_some()); + assert!(open.toggle_closed(0xF110_0000_0000_0001)); + assert!(open.pending.is_none()); + assert!( + open.take_closing(), + "the frame must be told — a cleared session alone leaves the window on screen" + ); + assert!(!open.take_closing(), "drained"); + assert!(!open.toggle_closed(0xF110_0000_0000_0001), "nothing open"); + assert!(!open.take_closing(), "a no-op toggle closes nothing"); + } + + /// A page chain opens with an EMPTY trail — the head comes from the object's template as the + /// feed paints, so a click that beats the ask-once template query still opens a reader. + #[test] + fn a_page_read_opens_before_its_template_lands() { + let mut open = ItemTextOpen::default(); + open.open_pages(0xF110_0000_0000_0001); + let sess = open.pending.as_ref().expect("open"); + assert!(matches!(&sess.source, ReadSource::Pages { visited } if visited.is_empty())); + } +} diff --git a/crates/benilla-app/src/ui_items/drain.rs b/crates/benilla-app/src/ui_items/drain.rs index 74df12db..45eeb3cf 100644 --- a/crates/benilla-app/src/ui_items/drain.rs +++ b/crates/benilla-app/src/ui_items/drain.rs @@ -251,6 +251,7 @@ pub(super) fn drain_container_uses( use_spell: t.use_spell.map(|u| u.spell_id), unwraps_gift: t.unwraps_gift(inst_flags), opens_loot: t.opens_loot(), + page_text: t.page_text, }) }); @@ -334,23 +335,38 @@ pub(super) fn drain_container_uses( ); continue; } + // #5 — readable: an item TEMPLATE carrying `PageText` (`0x5d8e4c`) — a book — opens the + // reader on its page chain, client-side, no permission packet (`0x4e32e0(itemGuid)`; + // decision 1105). Above #6 and above the open arm, which is the reference's own order and + // the INVERSE of the tooltip's, where OPENABLE wins over READABLE: a template that is both + // readable and lootable *shows* `` and *reads* on click — + // byte-verified, not a slip of ours. + if let Some(c) = clicked.filter(|c| c.page_text != 0) { + if item_text.toggle_closed(c.guid) { + debug!("ui_items: re-click closes the book {:#x}", c.guid); + } else { + debug!( + "ui_items: read book {:#x} (page {}, lua bag {bag} slot {slot})", + c.guid, c.page_text + ); + item_text.open_pages(c.guid); + } + continue; + } // #6 — readable: an item INSTANCE carrying `ITEM_FIELD_ITEM_TEXT_ID` (a mail-made // permanent letter) opens the reader — client-side, no permission packet (vmangos' // `CMSG_READ_ITEM` handler gates on the *template*'s PageText, which is 0 for the Plain // Letter; the text rides the ask-once `CMSG_ITEM_TEXT_QUERY` instead). - // - // The dispatcher's arm **#5** — the same local open off the *template*'s `PageText` - // (`0x5d8e4c`) — has no feed here yet (books/pages aren't built). When it lands it belongs - // immediately above this one, and still above the open arm: the send order is the INVERSE - // of the tooltip's, where OPENABLE wins over READABLE. A template that is both readable - // and lootable therefore *shows* `` and *reads* on click — the - // reference's own inversion, byte-verified, not a slip of ours. if let Some(c) = clicked.filter(|c| c.item_text_id != 0) { - debug!( - "ui_items: read item {:#x} (text {}, lua bag {bag} slot {slot})", - c.guid, c.item_text_id - ); - item_text.open(c.guid, c.item_text_id); + if item_text.toggle_closed(c.guid) { + debug!("ui_items: re-click closes the letter {:#x}", c.guid); + } else { + debug!( + "ui_items: read letter {:#x} (text {}, lua bag {bag} slot {slot})", + c.guid, c.item_text_id + ); + item_text.open_letter(c.guid, c.item_text_id); + } continue; } // #8 — the open arm (`0x5d8f7c: test al,4` → emitter `0x5edc80`): a **bare** template @@ -439,6 +455,10 @@ struct Clicked { unwraps_gift: bool, /// `ItemInfo::opens_loot` for this template — dispatcher arm #8. opens_loot: bool, + /// The template's `PageText` — dispatcher arm #5's book gate (decision 1105); `0` = not a + /// book. The reader re-reads the head (and the material) off the template itself as it paints, + /// like the reference, so only the fork's predicate is carried here. + page_text: u32, } /// Drain the pick/place/swap/split moves `PickupContainerItem`/`SplitContainerItem` queued and diff --git a/crates/benilla-app/src/ui_items/feed.rs b/crates/benilla-app/src/ui_items/feed.rs index 75173724..abcf9a5f 100644 --- a/crates/benilla-app/src/ui_items/feed.rs +++ b/crates/benilla-app/src/ui_items/feed.rs @@ -386,7 +386,7 @@ pub(super) fn feed_player_req( actions .spells .iter() - .any(|&id| s.catalog.get(id).is_some_and(|sd| sd.effect_1 == 40)) + .any(|&id| s.catalog.get(id).is_some_and(|sd| sd.effects[0] == 40)) }); let state = benilla_ui::script::PlayerReqState { level: store.0.unit_level().unwrap_or(0), diff --git a/crates/benilla-app/src/ui_items/mod.rs b/crates/benilla-app/src/ui_items/mod.rs index 799329fc..26b3b3d0 100644 --- a/crates/benilla-app/src/ui_items/mod.rs +++ b/crates/benilla-app/src/ui_items/mod.rs @@ -256,6 +256,24 @@ pub(crate) fn slot_guid_count( } } +/// An item template's icon path — its `DisplayInfoID` joined through `ItemDisplayInfo.dbc` (the +/// [`ItemDisplays`] catalog the equipment feed already loads). +/// +/// This is the **one** thing the client's spell-icon surfaces genuinely share. The *laws* do not: +/// wow-re's `system/ui/scratch/spell-icon-substitution-law.md` settled that there is no shared +/// spell-icon resolver at all — six Lua getters, six laws inlined per binding, disagreeing even +/// between the TradeSkill and Craft windows. But every arm that ends at an item ends *here*, at the +/// same `ItemTemplate+0x18 → 0x5d88b0 → rec+0x14` chain (§5 of that note). So the join lives once, +/// and each window keeps its own law above it. +pub(crate) fn item_icon( + icons: Option<&crate::entities::ItemDisplays>, + display_info_id: u32, +) -> Option { + icons + .and_then(|i| i.catalog.get(display_info_id)) + .and_then(|d| d.icon.clone()) +} + /// Count of item `entry` across the backpack + equipped bags. The quest-log feed /// ([`crate::ui_quest_log`]) needs this for an item-collection objective: unlike a creature/GO /// objective, item-objective progress is *not* one of the `PLAYER_QUEST_LOG` slot's 6-bit counters diff --git a/crates/benilla-app/src/ui_script/action_bar_tests.rs b/crates/benilla-app/src/ui_script/action_bar_tests.rs index 2a19fba2..41c41690 100644 --- a/crates/benilla-app/src/ui_script/action_bar_tests.rs +++ b/crates/benilla-app/src/ui_script/action_bar_tests.rs @@ -824,3 +824,69 @@ fn state_events_leave_empty_wells_untinted() { ); assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } + +/// The white-buttons regression (director-reported 2026-08-07, decision 1108): a slot that was +/// OCCUPIED — UpdateUsable painted its icon's 1/1/1 usable tint — then goes EMPTY (the feed's +/// character-switch diff: `set_action(None)` + `ACTIONBAR_SLOT_CHANGED`) kept the tint on the +/// now-artless icon region and drew it as a solid WHITE square. Two laws close it, both asserted +/// here: the empty arm HIDES the icon (ref ActionButton.lua l.168), and the engine emits nothing +/// for a texture-less region whatever its surviving tint (`0x7706e0` — the draw gate is `+0xcc`, +/// never the colour). +#[test] +fn an_occupied_slot_going_empty_leaves_no_white_plate() { + use benilla_ui::script::ActionState; + + let mut s = UiScript::new().unwrap(); + s.set_screen_size(1024.0, 768.0); + load_action_bar(&s); + s.set_action( + 3, + Some(ActionSlot { + texture: Some("Interface\\Icons\\Spell_Nature_HealingTouch".into()), + kind: 0x00, + action: 5185, + count: 0, + }), + ); + s.set_action_state( + 3, + Some(ActionState { + usable: true, + ..Default::default() + }), + ); + s.fire_event("PLAYER_ENTERING_WORLD", vec![]); + // The usable pass paints the occupied icon's 1/1/1 usable tint — the tint that survives. + s.fire_event("ACTIONBAR_UPDATE_USABLE", vec![]); + s.resolve(); + assert!( + s.extract().iter().any(|q| matches!(&q.content, + QuadContent::Texture { path: Some(p), .. } if p.contains("Spell_Nature_HealingTouch"))), + "the occupied slot draws its icon" + ); + + // The character switch: the new character's table has nothing in slot 3. + s.set_action(3, None); + s.set_action_state(3, None); + s.fire_event("ACTIONBAR_SLOT_CHANGED", vec![ScriptValue::Int(3)]); + s.resolve(); + for q in s.extract() { + match &q.content { + QuadContent::Texture { path: Some(p), .. } + if p.contains("Spell_Nature_HealingTouch") => + { + panic!("the emptied slot still draws the old icon") + } + QuadContent::Texture { + path: None, + color: Some(c), + .. + } if q.rect.is_some_and(|r| r.right - r.left <= 40.0) => { + // Well-sized only, as in the sibling test: page-wide solids are legitimate. + panic!("the emptied slot draws its surviving tint as a solid plate: {c:?}") + } + _ => {} + } + } + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} diff --git a/crates/benilla-app/src/ui_script/exp_bar_tests.rs b/crates/benilla-app/src/ui_script/exp_bar_tests.rs index 543b3ba9..6cdb8327 100644 --- a/crates/benilla-app/src/ui_script/exp_bar_tests.rs +++ b/crates/benilla-app/src/ui_script/exp_bar_tests.rs @@ -3,7 +3,7 @@ //! action-button machinery. Same file, two concerns: the strip along the bar's top is the //! player-progress readout; the buttons are the input surface. -use benilla_ui::script::{ScriptValue, UiScript, UnitState}; +use benilla_ui::script::{QuadContent, ScriptValue, UiScript, UnitState}; /// The XP bar's load set: the manifest prefix it actually needs — fonts, UIParent, the /// tooltip, `TextStatusBar.xml` (the numerals machinery `BenillaExpBar_OnLoad` wires in), @@ -251,6 +251,44 @@ fn the_max_level_rail_replaces_the_xp_bar_at_60() { assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); } +/// The gryphon end caps stay over the strip and the rail across ANY show/hide history — +/// including the faithful hidden→visible tail re-stamp (0x76ae10 / 0884, which a stale-rail +/// recovery exercises: decision 1106's whole bug walked through here). The protection is +/// 0884's own key order: the caps ride the art frame's OVERLAY and the strip's fill its +/// ARTWORK, and within a `(strata, level)` bucket the LAYER outranks every stamp — no +/// re-stamp can lift a lower layer over them. +#[test] +fn the_gryphons_outrank_the_bars_across_hide_show_cycles() { + let mut s = exp_bar_harness(); + s.set_player_xp(300, 400); + s.fire_event("PLAYER_ENTERING_WORLD", vec![]); + // The relog shape: the strip hidden (a stale max-level rail), then re-shown; the rail's + // own arrival is ALWAYS a hidden→visible re-stamp (it is born hidden). + s.run("BenillaExpBar:Hide() BenillaExpBar:Show()").unwrap(); + s.run("BenillaMaxLevelBar:Show()").unwrap(); + s.resolve(); + let quads = s.extract(); + let z_max = |suffix: &str| { + quads + .iter() + .filter(|q| { + matches!(&q.content, QuadContent::Texture { path: Some(p), .. } + if p.ends_with(suffix)) + }) + .map(|q| q.z) + .max() + }; + let fill = z_max("UI-StatusBar").expect("the strip's fill"); + let rail = z_max("UI-MainMenuBar-MaxLevel").expect("the rail's plates"); + let cap = z_max("UI-MainMenuBar-EndCap-Dwarf").expect("the gryphon end caps"); + assert!( + fill < cap && rail < cap, + "the end caps must paint over the re-shown bars \ + (fill z={fill:#x}, rail z={rail:#x}, cap z={cap:#x})" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} + /// The hover numerals (decision 1082, ref TextStatusBar.lua over `MainMenuBarExpText`): entering /// the strip shows "curr / max" centered on the bar, leaving hides it (the lockShow refcount's /// 0↔1 edge — the `statusBarText` cvar that pins it on permanently reads OFF here). diff --git a/crates/benilla-app/src/ui_script/trainer_tests.rs b/crates/benilla-app/src/ui_script/trainer_tests.rs index a1c8ee23..ff606bc9 100644 --- a/crates/benilla-app/src/ui_script/trainer_tests.rs +++ b/crates/benilla-app/src/ui_script/trainer_tests.rs @@ -363,6 +363,76 @@ fn filter_hides_a_state_keeping_headers() { ); } +/// The filter rows toggle **through the real dropdown kit** — the path a mouse takes, which the test +/// above deliberately shortcuts by faking `this`. `UIDropDownMenuButton_OnClick` runs the row's func +/// and only THEN flips the check for a `keepShownOnClick` row, so the func must not repaint the row +/// itself: an in-func `UIDropDownMenu_Initialize` re-derived the check from the fresh engine state and +/// the kit's flip then inverted it straight back — the check never moved on screen, and `this.checked` +/// stuck true, so a filter turned off could never be turned back on. Click, re-click, and re-open all +/// have to agree. +#[test] +fn filter_rows_toggle_through_the_dropdown_kit() { + let mut s = trainer_script(); + s.set_money(50); + s.set_trainer(Some(menu())); + s.fire_event("TRAINER_SHOW", vec![ScriptValue::Str("Sana".into())]); + + // Open the menu the way the capsule's arrow does. Row 2 is "Unavailable" (Initialize's order), + // checked because trainer_script() turned every state on. + s.run("ToggleDropDownMenu(1, nil, BenillaTrainerFilterDropDown)") + .unwrap(); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); + let row_checked = |s: &mut UiScript| { + s.eval::("return DropDownList1Button2Check:IsVisible() and true or false") + .unwrap() + }; + assert!(row_checked(&mut s), "Unavailable starts checked"); + assert_eq!(s.eval::("return GetNumTrainerServices()").unwrap(), 6); + + // Click it: the check clears, the engine filter clears, and Cleave (the unavailable service) + // drops out of the tree. Its Arms header stays. + s.run("this = DropDownList1Button2; UIDropDownMenuButton_OnClick()") + .unwrap(); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); + assert!(!row_checked(&mut s), "the click clears the row's check"); + assert!( + !s.eval::("return GetTrainerServiceTypeFilter('unavailable') == 1") + .unwrap(), + "and the engine's unavailable filter with it" + ); + assert_eq!( + s.eval::("return GetNumTrainerServices()").unwrap(), + 5, + "Cleave is hidden" + ); + + // Click it again: back on, both on screen and in the engine — the case the old code could never + // reach, because `this.checked` never went false. + s.run("this = DropDownList1Button2; UIDropDownMenuButton_OnClick()") + .unwrap(); + assert!(row_checked(&mut s), "the re-click restores the check"); + assert!( + s.eval::("return GetTrainerServiceTypeFilter('unavailable') == 1") + .unwrap(), + "and re-enables the filter" + ); + assert_eq!(s.eval::("return GetNumTrainerServices()").unwrap(), 6); + + // Close and re-open: Initialize re-derives every row from the engine, so the menu agrees with + // what the clicks left behind (the two states left on, "used" still on from trainer_script()). + s.run("this = DropDownList1Button2; UIDropDownMenuButton_OnClick()") + .unwrap(); + s.run("ToggleDropDownMenu(1, nil, BenillaTrainerFilterDropDown)") + .unwrap(); // same owner → closes + s.run("ToggleDropDownMenu(1, nil, BenillaTrainerFilterDropDown)") + .unwrap(); // re-open → re-Initialize + assert!( + !row_checked(&mut s), + "a re-opened menu shows the filter the clicks actually left off" + ); + assert!(s.errors().is_empty(), "script errors: {:?}", s.errors()); +} + /// A menu bigger than the 11 visible rows (one skill line, 15 services → a 16-row tree), so the list /// scrolls. The wheel spins the list even when the cursor is over a row — the engine bubbles the spin /// up the parent chain to the window, which drives the faux-scroll bar. diff --git a/crates/benilla-app/src/ui_spellbook.rs b/crates/benilla-app/src/ui_spellbook.rs index 3c6e90de..f2cf86b8 100644 --- a/crates/benilla-app/src/ui_spellbook.rs +++ b/crates/benilla-app/src/ui_spellbook.rs @@ -492,7 +492,7 @@ mod tests { const ATTACK: u32 = 6603; let mut attack = spell("Attack", None, 0x10); attack.icon = Some("Interface\\Icons\\Temp".into()); // the real DBC placeholder - attack.effect_1 = 78; // SPELL_EFFECT_ATTACK — makes it the melee auto-attack + attack.effects[0] = 78; // SPELL_EFFECT_ATTACK — makes it the melee auto-attack let catalog = SpellCatalog::from_displays(HashMap::from([(ATTACK, attack)])); let known: HashSet = [ATTACK].into_iter().collect(); diff --git a/crates/benilla-app/src/ui_tooltip/mod.rs b/crates/benilla-app/src/ui_tooltip/mod.rs index 3346cc71..ce262cbb 100644 --- a/crates/benilla-app/src/ui_tooltip/mod.rs +++ b/crates/benilla-app/src/ui_tooltip/mod.rs @@ -292,7 +292,7 @@ const EFFECT_ATTACK: u32 = 78; /// gate** while dodge/parry/block require it. That is why Attack — whose `Attributes` carry `0x10`, /// not the passive `0x40` — still shows a crit line, which is exactly the missing line reported. fn chance_line(d: &benilla_formats::SpellDisplay, store: Option<&ObjectStore>) -> Option { - let (label, percentage) = match d.effect_1 { + let (label, percentage) = match d.effects[0] { EFFECT_ATTACK => ("crit", store?.0.player_crit_percentage()?), EFFECT_DODGE if d.passive => ("dodge", store?.0.player_dodge_percentage()?), EFFECT_PARRY if d.passive => ("parry", store?.0.player_parry_percentage()?), diff --git a/crates/benilla-app/src/ui_tooltip/tests.rs b/crates/benilla-app/src/ui_tooltip/tests.rs index 4721703c..88f656ef 100644 --- a/crates/benilla-app/src/ui_tooltip/tests.rs +++ b/crates/benilla-app/src/ui_tooltip/tests.rs @@ -259,7 +259,7 @@ fn the_pinned_c6_lines_on_real_data() { // 2 · Attack (6603) — `Effect[0] == 78` omits the cast|cooldown line WHOLE, even though // `Attributes & 0x40` is clear. Before the §3.4 gate widened, this read "Instant". let d = spells.catalog.get(6603).expect("Attack 6603"); - assert_eq!(d.effect_1, 78, "SPELL_EFFECT_ATTACK"); + assert_eq!(d.effects[0], 78, "SPELL_EFFECT_ATTACK"); assert!(!d.passive, "6603 carries Attributes 0x10, not 0x40"); let v = spell_tooltip_view(6603, &spells, &mut t.ctx(0, None)).expect("Attack view"); assert_eq!(v.cast_time, None, "the law's Effect[0] gate"); @@ -277,7 +277,7 @@ fn the_pinned_c6_lines_on_real_data() { assert_eq!(v.chance.as_deref(), Some("2.62% chance to crit")); // Dodge (81) is passive and reads its own field. let dodge = spells.catalog.get(81).expect("Dodge 81"); - assert_eq!(dodge.effect_1, 20, "SPELL_EFFECT_DODGE"); + assert_eq!(dodge.effects[0], 20, "SPELL_EFFECT_DODGE"); assert!(dodge.passive, "81 carries Attributes 0x40"); let v = spell_tooltip_view(81, &spells, &mut t.ctx_for(0, None, Some(&rated))).expect("Dodge view"); diff --git a/crates/benilla-app/src/ui_tradeskill.rs b/crates/benilla-app/src/ui_tradeskill.rs index d168f99e..9542eeac 100644 --- a/crates/benilla-app/src/ui_tradeskill.rs +++ b/crates/benilla-app/src/ui_tradeskill.rs @@ -38,7 +38,7 @@ use crate::entities::ItemDisplays; use crate::items::Items; use crate::net::{NetCommands, ObjectStore, SelfPlayer}; use crate::ui_action::{cast_target, CastCommit, CastLadder, PlayerActions, Spells}; -use crate::ui_items::count_of; +use crate::ui_items::{count_of, item_icon}; use crate::ui_script::UiInput; use crate::ui_spellbook::SkillLines; use crate::ui_unit::UnitFeed; @@ -194,12 +194,39 @@ pub(crate) fn difficulty(rank: u32, low: u32, high: u32) -> TradeSkillDifficulty } } -/// An item template's icon path — `ItemDisplayInfo.dbc` via the [`ItemDisplays`] catalog (the -/// container feed's own join). -fn item_icon(icons: Option<&ItemDisplays>, display_info_id: u32) -> Option { - icons - .and_then(|i| i.catalog.get(display_info_id)) - .and_then(|d| d.icon.clone()) +/// **Law C** — the TradeSkill window's row icon, transcribing `GetTradeSkillIcon 0x4fdae0` +/// (byte-VERIFIED; wow-re `ui/scratch/spell-icon-substitution-law.md` §2, folded back by decision +/// 1107). Read `EffectItemType[0]` **unconditionally** as an item id and paint that item's icon; +/// on any miss — a zero id, a template not yet landed — return **`None`**. +/// +/// Two things the binding pointedly does *not* do, both of which this used to: +/// +/// - **No `CREATE_ITEM` gate.** `Effect[0]` (`+0xf4`) is never read anywhere in `[0x4fdae0, +/// 0x4fdc29]`. So this takes `d.effect_item_type[0]` directly rather than the `product_item` +/// computed beside it — that variable's `CREATE_ITEM` gate belongs to the separately-verified +/// *made-count* law (wow-re `tradeskill` TU-C) and has no business steering an icon. +/// - **No spell-icon fallback.** `+0x1d4`/`+0x1d8` and the `SpellIcon.dbc` globals appear nowhere +/// in the extent — proof by exhaustion over its three return paths, the last of which is a bare +/// `lua_pushnil`. A row whose item will not resolve shows *nothing*, never the recipe's own art. +/// +/// `None` while the ask-once template is in flight is the client's behaviour too: it pushes nil and +/// repaints when the async item callback rebuilds the list and fires `TRADE_SKILL_UPDATE`. +/// +/// The sibling laws deliberately disagree with this one — see [`crate::ui_craft`] (Law D, always +/// the spell's own icon) and [`crate::ui_trainer::service_icon`] (Law B). There is no shared +/// resolver in the real client and there is none here. +fn recipe_icon( + d: &benilla_formats::SpellDisplay, + icons: Option<&ItemDisplays>, + items: &mut Items, + commands: &NetCommands, +) -> Option { + let item = d.effect_item_type[0]; + if item == 0 { + return None; + } + let display = items.template(item, 0, commands)?.display_info_id; + item_icon(icons, display) } /// Build one recipe row: reagents/tools/product resolved through the ask-once template cache @@ -245,10 +272,10 @@ fn resolve_recipe( } // The product (CREATE_ITEM's EffectItemType, slot 0 — every probed recipe carries it there; - // a multi-slot product is unobserved in 5875) — its icon fronts the row when resolved. The - // made-count is byte-VERIFIED (wow-re `tradeskill` TU-C): min = BasePoints + BaseDice, - // max = BasePoints + DieSides × BaseDice (multiplicative), clamped ≥ 1. - let (product_item, min_made, max_made) = if d.effect_1 == SPELL_EFFECT_CREATE_ITEM { + // a multi-slot product is unobserved in 5875): the tooltip channel's item and the header key's + // source. The made-count is byte-VERIFIED (wow-re `tradeskill` TU-C): min = BasePoints + + // BaseDice, max = BasePoints + DieSides × BaseDice (multiplicative), clamped ≥ 1. + let (product_item, min_made, max_made) = if d.effects[0] == SPELL_EFFECT_CREATE_ITEM { let base = d.effect_base_points[0].max(0) as u32; let dice = d.effect_base_dice[0].max(0) as u32; let die = d.effect_die_sides[0].max(0) as u32; @@ -257,15 +284,7 @@ fn resolve_recipe( } else { (0, 1, 1) }; - let product_icon = (product_item != 0) - .then(|| { - items - .template(product_item, 0, commands) - .map(|t| t.display_info_id) - .and_then(|d| item_icon(icons, d)) - }) - .flatten(); - let icon = product_icon.or_else(|| d.icon.clone()); + let icon = recipe_icon(d, icons, items, commands); // The VERIFIED header key (wow-re `tradeskill` TU-B): the created item's (class, subclass), // named from ItemSubClass.dbc (verbose-first). `None` while the ask-once template is in // flight — the client's own one-frame header deferral; the engine buckets it trailing. The @@ -494,3 +513,98 @@ fn drain_trade_skill( repeat.clear(); } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::items::{test_template, TestDeps}; + use benilla_formats::{ + ItemDisplay, ItemDisplayCatalog, SpellDisplay, SPELL_EFFECT_ENCHANT_ITEM, + }; + use std::collections::HashMap; + + /// A recipe whose `Effect[0]` is `effect` and whose `EffectItemType[0]` is `item`, carrying its + /// own distinct spell icon so a wrong arm is *named* by the assertion, not merely unequal. + fn recipe(effect: u32, item: u32) -> SpellDisplay { + SpellDisplay { + name: "Runed Copper Breastplate".into(), + icon: Some("SPELL".into()), + effects: [effect, 0, 0], + effect_item_type: [item, 0, 0], + ..Default::default() + } + } + + /// Item 777's template + `ItemDisplayInfo` row, landed — the icon Law C actually wants. + fn landed_item(deps: &mut TestDeps) -> ItemDisplays { + let mut t = test_template("Runed Copper Breastplate"); + t.display_info_id = 5; + deps.items.insert_template(777, Some(t)); + ItemDisplays::icons_for_tests(ItemDisplayCatalog::from_displays(HashMap::from([( + 5, + ItemDisplay { + icon: Some("ITEM".into()), + ..Default::default() + }, + )]))) + } + + /// The ordinary arm: a `CREATE_ITEM` recipe fronts its product's item art, never the spell's. + #[test] + fn law_c_paints_the_created_items_icon() { + let mut deps = TestDeps::new(); + let icons = landed_item(&mut deps); + let d = recipe(SPELL_EFFECT_CREATE_ITEM, 777); + assert_eq!( + recipe_icon(&d, Some(&icons), &mut deps.items, &deps.commands), + Some("ITEM".into()), + ); + } + + /// **No `CREATE_ITEM` gate** — `Effect[0]` is never read by `0x4fdae0`. A recipe carrying a + /// non-`CREATE_ITEM` effect in slot 0 still resolves `EffectItemType[0]` as an item id. This is + /// the half our old code got wrong by gating; it is asserted with a deliberately absurd effect + /// so the test fails the moment someone reintroduces a gate of any shape. + #[test] + fn law_c_does_not_gate_on_the_effect_type() { + let mut deps = TestDeps::new(); + let icons = landed_item(&mut deps); + let d = recipe(SPELL_EFFECT_ENCHANT_ITEM, 777); + assert_eq!( + recipe_icon(&d, Some(&icons), &mut deps.items, &deps.commands), + Some("ITEM".into()), + ); + } + + /// **No spell-icon fallback** — the binding's third return is a bare `lua_pushnil`. Every miss + /// arm is `None`, and in particular NOT `"SPELL"`: a zero item id (an enchant recipe), an item + /// whose template will never land, and a missing `ItemDisplayInfo` row. + #[test] + fn law_c_pushes_nil_on_every_miss_never_the_spells_icon() { + let mut deps = TestDeps::new(); + let icons = landed_item(&mut deps); + + // EffectItemType[0] == 0: 0x55ba30 short-circuits on a zero id before hashing. + let none = recipe(SPELL_EFFECT_ENCHANT_ITEM, 0); + assert_eq!( + recipe_icon(&none, Some(&icons), &mut deps.items, &deps.commands), + None, + ); + + // A template that never lands (the async row) — nil, and the ask goes out exactly once. + let missing = recipe(SPELL_EFFECT_CREATE_ITEM, 999); + assert_eq!( + recipe_icon(&missing, Some(&icons), &mut deps.items, &deps.commands), + None, + ); + assert_eq!( + recipe_icon(&missing, Some(&icons), &mut deps.items, &deps.commands), + None, + ); + assert_eq!(deps.queried_entries(), vec![999], "ask-once, not ask-often"); + + // The template landed but ItemDisplayInfo is unresolved — still nil, still not "SPELL". + let d = recipe(SPELL_EFFECT_CREATE_ITEM, 777); + assert_eq!(recipe_icon(&d, None, &mut deps.items, &deps.commands), None); + } +} diff --git a/crates/benilla-app/src/ui_trainer.rs b/crates/benilla-app/src/ui_trainer.rs index cf3ff835..2d1449ee 100644 --- a/crates/benilla-app/src/ui_trainer.rs +++ b/crates/benilla-app/src/ui_trainer.rs @@ -4,9 +4,11 @@ //! //! The net bridge fills [`TrainerOpen`] from the wire (`SMSG_TRAINER_LIST` → the trainer's services + //! greeting, reached through the gossip trainer option). Each frame [`feed_trainer`] resolves each -//! wire [`TrainerSpell`] to a Lua-facing [`TrainerService`] — name/subtext/icon from the spell -//! catalog (`Spell.dbc`, loaded whole at startup, so no ask-once query like the item rows need), the -//! skill-requirement name from the skill-line catalog, the green/red/gray state straight off the wire +//! wire [`TrainerSpell`] to a Lua-facing [`TrainerService`] — name/subtext from the spell catalog +//! (`Spell.dbc`, loaded whole at startup), the **icon** from its own byte-verified law +//! ([`service_icon`], which at a *tradeskill* trainer fronts the taught recipe's created item and so +//! does need the ask-once item-template query the item rows use), the skill-requirement name from +//! the skill-line catalog, the green/red/gray state straight off the wire //! `state` byte — pushes the snapshot ([`UiScript::set_trainer`]), and fires `TRAINER_SHOW` on open / //! `TRAINER_UPDATE` on a content change / `TRAINER_CLOSED` on clear. [`drain_trainer`] pulls the Lua //! intents back out: the Train button's `BuyTrainerService` → `CMSG_TRAINER_BUY_SPELL` for the open @@ -22,7 +24,9 @@ use std::collections::HashSet; -use benilla_formats::{SkillLineCatalog, SpellCatalog}; +use benilla_formats::{ + SkillLineCatalog, SpellCatalog, SPELL_EFFECT_LEARN_PET_SPELL, SPELL_EFFECT_LEARN_SPELL, +}; use benilla_protocol::messages::{trainer_spell_state, TrainerSpell}; use bevy::prelude::*; @@ -31,9 +35,12 @@ use benilla_ui::script::{ TrainerState, UiScript, }; +use crate::entities::ItemDisplays; +use crate::items::Items; use crate::names::NameCache; use crate::net::{ClientCommand, NetCommands}; use crate::ui_action::{PlayerActions, Spells}; +use crate::ui_items::item_icon; use crate::ui_script::UiInput; use crate::ui_session::{close_npc_session_out_of_range, npc_switched, NpcSession}; use crate::ui_spellbook::SkillLines; @@ -129,6 +136,62 @@ fn train_error_text(code: u32) -> String { .to_string() } +/// The trainer's **icon law** — `GetTrainerServiceIcon 0x4d8f50`, byte-verified whole in wow-re +/// (`system/ui/scratch/spell-icon-substitution-law.md` §1; the binding is not a marshal, the entire +/// resolution is inlined there). Three gates, then a fallback: +/// +/// 1. **The trainer type is 2** (tradeskill/profession — `[0xb73a08]`, the trainer-list packet's +/// type dword stored verbatim). A class/mount/pet trainer never substitutes. +/// 2. **The WIRE spell has a learn-wrapper effect** in any of its three slots — +/// `SPELL_EFFECT_LEARN_SPELL` or `SPELL_EFFECT_LEARN_PET_SPELL` (`0x4d8ff5`/`0x4d8ffa`). The +/// **first** matching slot wins; its `EffectTriggerSpell` is the taught spell. +/// 3. **That taught spell creates an item** (`EffectItemType[0] != 0`, `0x4d906a`) → the created +/// item's `ItemDisplayInfo` icon. +/// +/// Otherwise: the **WIRE (wrapper) spell's own `SpellIconID`** (`0x4d9008`). This is the fact +/// benilla had backwards — `esi` is pinned to the wire record at `0x4d8fd7` and is never reassigned +/// on any path reaching the fallback, so **the client never paints the taught spell's own icon at a +/// trainer**. We used to, which is what put a blue `Spell_Shadow_SealOfKings` crown on Copper +/// Shortsword: spell 2756 (the wrapper the server sends) hops to 2739 (the recipe), whose own +/// `SpellIconID` is that crown — while the law wants item 2847's sword. +/// +/// A gate-3 hit whose item template is not cached yet returns `None` (the client pushes Lua `nil` +/// and repaints on the cache callback, `0x4d9140` → `TRAINER_UPDATE`). Our equivalent is free: the +/// template answer changes the snapshot, so [`feed_trainer`]'s diff re-fires `TRAINER_UPDATE` on its +/// own. +fn service_icon( + wire_spell: u32, + trainer_type: u32, + spells: &SpellCatalog, + icons: Option<&ItemDisplays>, + items: &mut Items, + commands: &NetCommands, +) -> Option { + let wire = spells.get(wire_spell)?; + if trainer_type == TRAINER_TYPE_TRADESKILL { + // Gate 2: the first learn-wrapper slot wins — a miss (or a zero trigger) falls straight + // through to the wire icon rather than scanning on, exactly as the loop's `je` does. + let taught = wire + .effects + .iter() + .position(|&e| e == SPELL_EFFECT_LEARN_SPELL || e == SPELL_EFFECT_LEARN_PET_SPELL) + .map(|i| wire.effect_trigger_spell[i]) + .filter(|&t| t != 0); + // Gate 3: the taught spell's product item, slot 0 only. + if let Some(product) = taught + .and_then(|t| spells.get(t)) + .map(|d| d.effect_item_type[0]) + .filter(|&e| e != 0) + { + return items + .template(product, 0, commands) + .map(|t| t.display_info_id) + .and_then(|d| item_icon(icons, d)); + } + } + wire.icon.clone() +} + /// The green/red/gray colour a wire `state` byte maps to (decision 0237): GRAY → known, GREEN → /// learnable, everything else (RED + any unexpected value) → gated. fn category(state: u8) -> TrainerServiceCategory { @@ -145,12 +208,16 @@ fn category(state: u8) -> TrainerServiceCategory { /// the ability-req names + rank from the same spell catalog, the state/cost/gates straight off the /// wire. `known` is the player's known-spell set ([`PlayerActions::spells`]) — each prerequisite /// ability is coloured by whether the player already knows that specific spell (see below). +#[allow(clippy::too_many_arguments)] // the resolver's full catalog set fn resolve_service( wire: &TrainerSpell, trainer_type: u32, spells: &SpellCatalog, skill_lines: Option<&SkillLineCatalog>, known: &HashSet, + icons: Option<&ItemDisplays>, + items: &mut Items, + commands: &NetCommands, ) -> TrainerService { // The trainer offers a LEARN wrapper (decision 0247); the ability it teaches — with the skill // line the tree groups by, and the real display name/icon — is the taught spell. Resolve it once @@ -229,7 +296,10 @@ fn resolve_service( spell_id: wire.spell, name: display.map(|d| d.name.clone()), subtext: display.and_then(|d| d.rank.clone()), - texture: display.and_then(|d| d.icon.clone()), + // The NAME/subtext hop through the taught spell stays (decisions 0247/0252 — the wrapper is + // not in SkillLineAbility, so grouping and display MUST hop). The ICON does not: it is its + // own byte-verified law over the WIRE spell ([`service_icon`]). + texture: service_icon(wire.spell, trainer_type, spells, icons, items, commands), // The detail pane's description body, left empty by design. The real // `GetTrainerServiceDescription` returns the spell's *tooltip* — `Spell.dbc`'s Description // with its `$s1`/`$o1`/`$d`/`$a1` tokens substituted from the spell's effect base points, @@ -251,11 +321,15 @@ fn resolve_service( /// Build the Lua-facing snapshot from [`TrainerOpen`] + the spell/skill catalogs — `None` when no /// trainer is open. +#[allow(clippy::too_many_arguments)] // the resolver's full catalog set fn snapshot( open: &TrainerOpen, spells: &SpellCatalog, skill_lines: Option<&SkillLineCatalog>, known: &HashSet, + icons: Option<&ItemDisplays>, + items: &mut Items, + commands: &NetCommands, ) -> Option { open.trainer?; Some(TrainerState { @@ -264,7 +338,18 @@ fn snapshot( services: open .services .iter() - .map(|w| resolve_service(w, open.trainer_type, spells, skill_lines, known)) + .map(|w| { + resolve_service( + w, + open.trainer_type, + spells, + skill_lines, + known, + icons, + items, + commands, + ) + }) .collect(), // The engine synthesizes the skill-line tree in `set_trainer` — the app pushes only the flat // services (each carrying its resolved skill line above). @@ -283,6 +368,10 @@ fn feed_trainer( actions: Res, spells: Option>, skill_lines: Option>, + // A tradeskill trainer's rows front the CREATED ITEM's icon, so the feed needs the ask-once + // template cache + `ItemDisplayInfo.dbc` — the tradeskill window's own pair ([`service_icon`]). + icons: Option>, + mut items: ResMut, mut errors: ResMut, commands: Res, mut names: ResMut, @@ -316,6 +405,9 @@ fn feed_trainer( &spells.catalog, Some(&skill_lines.catalog), &actions.spells, + icons.as_deref(), + &mut items, + &commands, ); // The trainer's name resolves through the NameCache (a creature-name query, ask-once — the // real client's `UnitName("npc")`). `None`/empty while in flight; the title shows the static @@ -400,6 +492,7 @@ fn drain_trainer( #[cfg(test)] mod tests { use super::*; + use benilla_formats::{ItemDisplay, ItemDisplayCatalog, SpellDisplay}; use std::collections::HashMap; /// An empty spell catalog (`Spell.dbc` not resolved) — the resolver's name/icon lookups miss. @@ -407,6 +500,32 @@ mod tests { SpellCatalog::from_displays(HashMap::new()) } + /// The feed-side trio [`service_icon`] needs — the shared seam, since the tradeskill and craft + /// icon laws terminate in the same ask-once cache. Gate 3's "template in flight" arm is the + /// default; the tests that want a landed template seed one. + use crate::items::TestDeps as Deps; + + /// [`resolve_service`] with the icon trio defaulted — the shape the gate/state tests want. + fn resolve_service( + wire: &TrainerSpell, + trainer_type: u32, + spells: &SpellCatalog, + skill_lines: Option<&SkillLineCatalog>, + known: &HashSet, + ) -> TrainerService { + let mut deps = Deps::new(); + super::resolve_service( + wire, + trainer_type, + spells, + skill_lines, + known, + None, + &mut deps.items, + &deps.commands, + ) + } + fn wire(spell: u32, state: u8, cost: u32, req_level: u8, req_skill: u32) -> TrainerSpell { TrainerSpell { spell, @@ -421,6 +540,246 @@ mod tests { } } + /// [`snapshot`] with the icon trio defaulted. + fn snap(open: &TrainerOpen, spells: &SpellCatalog) -> Option { + let mut deps = Deps::new(); + snapshot( + open, + spells, + None, + &HashSet::new(), + None, + &mut deps.items, + &deps.commands, + ) + } + + /// The icon law's fixture (wow-re `spell-icon-substitution-law.md` §1's shape, synthetic so + /// every gate is reachable): wrapper 100 teaches 200 via a slot-0 `LEARN_SPELL`; 200 creates + /// item 777; item 777's display 5 carries the "real" art. Each spell has a DISTINCT icon so a + /// wrong arm is named by the assertion, not merely unequal. + fn icon_catalog() -> SpellCatalog { + let wrapper = SpellDisplay { + name: "Copper Shortsword".into(), + icon: Some("WRAPPER".into()), + effects: [SPELL_EFFECT_LEARN_SPELL, 0, 0], + effect_trigger_spell: [200, 0, 0], + ..Default::default() + }; + let taught = SpellDisplay { + name: "Copper Shortsword".into(), + icon: Some("TAUGHT".into()), + effect_item_type: [777, 0, 0], + ..Default::default() + }; + SpellCatalog::from_displays(HashMap::from([(100, wrapper), (200, taught)])) + } + + /// The product item's template + `ItemDisplayInfo` row, landed. + fn landed_item(deps: &mut Deps) -> ItemDisplays { + let mut t = crate::items::test_template("Copper Shortsword"); + t.display_info_id = 5; + deps.items.insert_template(777, Some(t)); + ItemDisplays::icons_for_tests(ItemDisplayCatalog::from_displays(HashMap::from([( + 5, + ItemDisplay { + icon: Some("ITEM".into()), + ..Default::default() + }, + )]))) + } + + fn icon_of( + spells: &SpellCatalog, + trainer_type: u32, + wire_spell: u32, + land: bool, + ) -> Option { + let mut deps = Deps::new(); + let icons = land.then(|| landed_item(&mut deps)); + service_icon( + wire_spell, + trainer_type, + spells, + icons.as_ref(), + &mut deps.items, + &deps.commands, + ) + } + + /// The trainer icon law, arm by arm. The director's report — a blue `Spell_Shadow_SealOfKings` + /// crown where the sword belongs — was BOTH halves of this wrong: we substituted nothing, and we + /// fell back to the taught spell's icon, which the client never paints at a trainer on any path. + #[test] + fn trainer_icon_substitutes_the_product_only_at_a_tradeskill_trainer() { + let spells = icon_catalog(); + + // Gate 1+2+3 all pass, template landed → the created ITEM's icon. + assert_eq!( + icon_of(&spells, TRAINER_TYPE_TRADESKILL, 100, true).as_deref(), + Some("ITEM") + ); + + // Gate 1 fails (a class trainer): the WIRE spell's own icon — never the taught spell's. + // This is the corrected fallback; the old code returned "TAUGHT" here. + assert_eq!( + icon_of(&spells, 0, 100, true).as_deref(), + Some("WRAPPER"), + "a class trainer serves the wrapper's own icon, not the taught spell's" + ); + + // Gate 2 fails (no learn-wrapper effect in any slot) → the wire spell's own icon. Spell 200 + // creates an item, so only the missing wrapper effect can be keeping the substitution off. + assert_eq!( + icon_of(&spells, TRAINER_TYPE_TRADESKILL, 200, true).as_deref(), + Some("TAUGHT"), + "200 IS the wire spell here, so its own icon is the right answer" + ); + + // A spell with no record at all → nil (every failure funnels there, 0x4d911c). + assert_eq!(icon_of(&spells, TRAINER_TYPE_TRADESKILL, 999, true), None); + } + + /// Gate 3: a wrapper whose taught spell creates NO item falls back to the wire icon — the + /// class/pet-trainer population, which is why the fallback arm has to be right. + #[test] + fn trainer_icon_falls_back_when_the_taught_spell_makes_no_item() { + let wrapper = SpellDisplay { + icon: Some("WRAPPER".into()), + effects: [SPELL_EFFECT_LEARN_SPELL, 0, 0], + effect_trigger_spell: [200, 0, 0], + ..Default::default() + }; + let taught = SpellDisplay { + icon: Some("TAUGHT".into()), + ..Default::default() // effect_item_type all zero + }; + let spells = SpellCatalog::from_displays(HashMap::from([(100, wrapper), (200, taught)])); + assert_eq!( + icon_of(&spells, TRAINER_TYPE_TRADESKILL, 100, true).as_deref(), + Some("WRAPPER") + ); + } + + /// The wrapper scan covers all three effect slots and accepts `LEARN_PET_SPELL` (57) as well as + /// `LEARN_SPELL` (36) — the paired `cmp ecx,0x24` / `cmp ecx,0x39` at `0x4d8ff5`/`0x4d8ffa`. + #[test] + fn trainer_icon_scans_every_effect_slot_for_either_learn_effect() { + let wrapper = SpellDisplay { + icon: Some("WRAPPER".into()), + effects: [0, SPELL_EFFECT_LEARN_PET_SPELL, 0], + effect_trigger_spell: [0, 200, 0], + ..Default::default() + }; + let taught = SpellDisplay { + effect_item_type: [777, 0, 0], + ..Default::default() + }; + let spells = SpellCatalog::from_displays(HashMap::from([(100, wrapper), (200, taught)])); + assert_eq!( + icon_of(&spells, TRAINER_TYPE_TRADESKILL, 100, true).as_deref(), + Some("ITEM"), + "a pet-learn effect in slot 1 substitutes just the same" + ); + } + + /// The in-flight case: gate 3 passes but the item template is not cached, so the icon reads + /// `None` **and the template gets asked for exactly once**. The real client pushes Lua nil here + /// and repaints from the cache callback (`0x4d9140` → `TRAINER_UPDATE`); our equivalent is the + /// feed's own snapshot diff, which re-fires once the answer changes the texture. + #[test] + fn trainer_icon_is_nil_until_the_product_template_lands_and_asks_once() { + let spells = icon_catalog(); + let mut deps = Deps::new(); + let icons = + ItemDisplays::icons_for_tests(ItemDisplayCatalog::from_displays(HashMap::new())); + + let first = service_icon( + 100, + TRAINER_TYPE_TRADESKILL, + &spells, + Some(&icons), + &mut deps.items, + &deps.commands, + ); + assert_eq!(first, None, "no template yet → nil, not the wrapper's icon"); + assert_eq!( + deps.queried_entries(), + vec![777], + "and the created item's template was asked for" + ); + + // A second frame before the answer lands must not re-ask (the ask-once cache). + let _ = service_icon( + 100, + TRAINER_TYPE_TRADESKILL, + &spells, + Some(&icons), + &mut deps.items, + &deps.commands, + ); + assert!( + deps.queried_entries().is_empty(), + "asked once, not per frame" + ); + } + + /// The director's exact case on the real shipped `Spell.dbc` — the check wow-re's §7 pins: + /// spell 2756 is the wrapper a Blacksmithing trainer sends, 2739 the recipe it teaches, 2847 the + /// sword. At a tradeskill trainer the law must reach for item 2847; at a class trainer it must + /// serve 2756's own icon. Skips without client data. + #[test] + fn trainer_icon_on_real_data_reaches_for_the_crafted_item() { + let data = std::path::PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../WoW/Data"); + if !data.is_dir() { + eprintln!("skipping: vanilla client not present at {}", data.display()); + return; + } + let mut chain = benilla_formats::open_chain(&data).expect("open chain"); + let spells = benilla_formats::load_spell_catalog(&mut chain).expect("load Spell"); + + // The two textures a wrong law produces, straight off the shipped DBC. + let wrapper_icon = spells.get(2756).unwrap().icon.clone(); + let recipe_icon = spells.get(2739).unwrap().icon.clone(); + assert_eq!( + recipe_icon.as_deref(), + Some("Interface\\Icons\\Spell_Shadow_SealOfKings"), + "the blue crown the director saw IS the recipe spell's own icon" + ); + + // A tradeskill trainer: gate 3 fires for item 2847 and the icon waits on the template — + // crucially it is NOT the crown. + let mut deps = Deps::new(); + let icons = + ItemDisplays::icons_for_tests(ItemDisplayCatalog::from_displays(HashMap::new())); + let icon = service_icon( + 2756, + TRAINER_TYPE_TRADESKILL, + &spells, + Some(&icons), + &mut deps.items, + &deps.commands, + ); + assert_eq!( + icon, None, + "waiting on the item template, not painting the crown" + ); + assert_eq!( + deps.queried_entries(), + vec![2847], + "the law reached for the crafted sword's template" + ); + + // A class trainer with the same wire spell: the wrapper's own icon, not the recipe's. + let mut deps = Deps::new(); + let class_icon = service_icon(2756, 0, &spells, None, &mut deps.items, &deps.commands); + assert_eq!(class_icon, wrapper_icon); + assert_ne!( + class_icon, recipe_icon, + "the taught spell's icon is never painted at a trainer" + ); + } + /// The learn-spell hop end-to-end on real 5875 data (decision 0247): a warrior trainer sends the /// LEARN wrapper 1605 ("learn Heroic Strike"), which is not in SkillLineAbility — resolve_service /// must hop through the taught spell (78) to group it under Arms (26) and show its name, while the @@ -587,7 +946,7 @@ mod tests { fn snapshot_is_none_when_closed_and_lists_services_when_open() { let spells = empty_catalog(); let mut open = TrainerOpen::default(); - assert!(snapshot(&open, &spells, None, &HashSet::new()).is_none()); + assert!(snap(&open, &spells).is_none()); open.open( 0x42, @@ -598,7 +957,7 @@ mod tests { ], "Learn from me.".into(), ); - let state = snapshot(&open, &spells, None, &HashSet::new()).expect("open → Some"); + let state = snap(&open, &spells).expect("open → Some"); assert_eq!(state.greeting, "Learn from me."); assert_eq!(state.services.len(), 2); assert_eq!( @@ -608,7 +967,7 @@ mod tests { assert_eq!(state.services[1].category, TrainerServiceCategory::Used); open.clear(); - assert!(snapshot(&open, &spells, None, &HashSet::new()).is_none()); + assert!(snap(&open, &spells).is_none()); assert_eq!(open.trainer, None); } } diff --git a/crates/benilla-app/src/ui_unit.rs b/crates/benilla-app/src/ui_unit.rs index 0280cd72..587e9352 100644 --- a/crates/benilla-app/src/ui_unit.rs +++ b/crates/benilla-app/src/ui_unit.rs @@ -778,33 +778,56 @@ fn feed_units( } // The ding feed: `PLAYER_LEVEL_UP` (arg1 = the new level) when our avatar's - // `UNIT_FIELD_LEVEL` rises — the event the exhaustion tick (1082) and the max-level rail - // (1094) register; it was a dead registration until 1094. Trigger PROVISIONAL (0578's - // pattern): fired off the descriptor diff, which lands in the same update batch as the - // ding's XP fields, so consumers read a coherent picture. The real client plausibly fires - // it from its `SMSG_LEVELUP_INFO` handler instead, with the packet's gain tuple as arg2+ — - // unpinned, and no 1.12 FrameXML consumer reads past arg1 (`ReputationWatchBar_Update` - // takes arg1; the tick's handler takes none), so the extra args wait for a consumer. - // A DROP (a GM `.level` down) fires nothing, like a first sighting — only a rise dings. + // `UNIT_FIELD_LEVEL` CHANGES — the event the exhaustion tick (1082) and the max-level rail + // (1094) register; it was a dead registration until 1094. Any change, not only a rise: + // vmangos `GiveLevel` runs for demotions too (a GM `.character level` down) and sends + // `SMSG_LEVELUP_INFO` unconditionally, so the real client hears every change — 1094's + // rise-only guard left the rail latched shown after a 60→1 demote (the 1106 live repro). + // Trigger PROVISIONAL (0578's pattern): fired off the descriptor diff, which lands in the + // same update batch as the ding's XP fields, so consumers read a coherent picture. The + // real client plausibly fires it from its `SMSG_LEVELUP_INFO` handler instead, with the + // packet's gain tuple as arg2+ — unpinned, and no 1.12 FrameXML consumer reads past arg1 + // (`ReputationWatchBar_Update` takes arg1; the tick's handler takes none), so the extra + // args wait for a consumer. if let Some((store, _)) = self_q.iter().next() { if let Some(level) = store.0.unit_level() { let prev = feed.last_level.replace(level); - if prev.is_some_and(|p| level > p) { + if prev.is_some_and(|p| level != p) { script.fire_event("PLAYER_LEVEL_UP", vec![ScriptValue::Int(i64::from(level))]); } } } - // Initial pull: fire PLAYER_ENTERING_WORLD once so frames do their first paint on their own — - // gated on our avatar's descriptor EXISTING. 1087 stated the real client's guarantee (the - // player object lands before this event) and moved the XP/rest pushes above the fire, but the - // fire itself still went out on frame 1, seconds before login: every one-shot first-paint - // read empty state, and only consumers with their own diff events recovered. The 1094 live - // probe caught the one that couldn't — a level-60 login read UnitLevel()=0 at the fire and - // kept the XP strip. With the gate the guarantee is structural for every consumer. - if !feed.entered_world && self_pair.is_some() { - script.fire_event("PLAYER_ENTERING_WORLD", vec![]); - feed.entered_world = true; + // Initial pull: fire PLAYER_ENTERING_WORLD once PER WORLD ENTRY so frames do their first + // paint on their own — gated on our avatar's descriptor EXISTING. 1087 stated the real + // client's guarantee (the player object lands before this event) and moved the XP/rest + // pushes above the fire, but the fire itself still went out on frame 1, seconds before + // login: every one-shot first-paint read empty state, and only consumers with their own + // diff events recovered. The 1094 live probe caught the one that couldn't — a level-60 + // login read UnitLevel()=0 at the fire and kept the XP strip. With the gate the guarantee + // is structural for every consumer. + // + // The absent arm is the world-EXIT edge (logout / char switch — the self entity despawns + // with the streamed world): the real client fires this event on *every* world entry (it + // tears the whole UI down between them; the once-per-process frame tree is our documented + // interim, `IngameUiLoaded`), so re-arm the fire and forget the player-global diff + // memories. Forgetting them makes every next-login first sighting re-seed SILENTLY — the + // byte-verified fresh-CREATE notify silence (1098 §4), now holding per entry: without it a + // 60→1 char switch latched the max-level rail shown over a level-1 body (1106's live + // repro), and a normal→rested char switch would misfire "You feel rested." at login. + if self_pair.is_some() { + if !feed.entered_world { + script.fire_event("PLAYER_ENTERING_WORLD", vec![]); + feed.entered_world = true; + } + } else if feed.entered_world { + feed.entered_world = false; + feed.last_xp = None; + feed.last_rest = None; + feed.last_level = None; + feed.last_combo = None; + feed.in_combat = None; + feed.pvp_desired = None; } for (token, snap) in [("player", &player), ("target", &target)] { diff --git a/crates/benilla-formats/examples/spell_row.rs b/crates/benilla-formats/examples/spell_row.rs index 3b431340..96dea97c 100644 --- a/crates/benilla-formats/examples/spell_row.rs +++ b/crates/benilla-formats/examples/spell_row.rs @@ -13,11 +13,11 @@ fn main() -> anyhow::Result<()> { continue; }; println!( - "{id} {:?}\n Targets={:#06x} implicitA1={} effect_1={}\n equippedClass={} subclassMask={:#x} invTypeMask={:#x}", + "{id} {:?}\n Targets={:#06x} implicitA1={} effects[0] = {}\n equippedClass={} subclassMask={:#x} invTypeMask={:#x}", d.name, d.targets, d.implicit_target_a1, - d.effect_1, + d.effects[0], d.equipped_item_class, d.equipped_item_subclass_mask, d.equipped_item_inventory_type_mask, diff --git a/crates/benilla-formats/src/lib.rs b/crates/benilla-formats/src/lib.rs index 6c457849..2140a1a9 100644 --- a/crates/benilla-formats/src/lib.rs +++ b/crates/benilla-formats/src/lib.rs @@ -56,6 +56,8 @@ mod exhaustion; pub use exhaustion::{load_exhaustion, ExhaustionRow}; mod bank_bag_slot_prices; pub use bank_bag_slot_prices::{load_bank_bag_slot_prices, BankBagSlotPrices}; +mod page_text_material; +pub use page_text_material::{load_page_text_material_catalog, PageTextMaterialCatalog}; mod stationery; pub use stationery::{load_stationery_catalog, StationeryCatalog, STATIONERY_DEFAULT}; mod lock; @@ -138,7 +140,8 @@ pub use spells::{ SpellDisplay, SpellDuration, SpellDurationCatalog, SpellRadius, SpellRadiusCatalog, SpellRange, SpellRangeCatalog, TokenContext, ATTR_CASTABLE_WHILE_DEAD, ATTR_NOT_IN_COMBAT, ATTR_ONLY_STEALTHED, SPELL_EFFECT_CREATE_ITEM, SPELL_EFFECT_ENCHANT_ITEM, - SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY, SPELL_EFFECT_SKINNING, SPELL_EFFECT_TRADE_SKILL, + SPELL_EFFECT_ENCHANT_ITEM_TEMPORARY, SPELL_EFFECT_LEARN_PET_SPELL, SPELL_EFFECT_LEARN_SPELL, + SPELL_EFFECT_SKINNING, SPELL_EFFECT_TRADE_SKILL, }; mod skill_lines; pub use skill_lines::{ diff --git a/crates/benilla-formats/src/light.rs b/crates/benilla-formats/src/light.rs index 73605fe4..82cf0f83 100644 --- a/crates/benilla-formats/src/light.rs +++ b/crates/benilla-formats/src/light.rs @@ -241,7 +241,11 @@ fn light_params_schema() -> Schema { } /// The client's distance→strength ramp for an area light: `1` within `start`, falling linearly to -/// `0` at `end` (and `0` beyond). Matches `getEnvInfo`'s `blendAlpha`. +/// `0` at `end` (and `0` beyond). Byte-VERIFIED (`dn_light_select 0x6d2d00`, wow-re +/// `system/lighting/scratch/ctb.md`): `w = dist ≤ inner ? 1 : 1 − (dist−inner)/(outer−inner)`, with +/// the containment gate `dist ≤ outer` — so a sphere entering at its outer radius enters at **w = 0**. +/// That is what makes the area blend continuous across a sphere boundary, and it is the invariant +/// decision 1104's water fix rests on. fn blend_alpha(dist: f32, start: f32, end: f32) -> f32 { if dist >= end { 0.0 @@ -491,7 +495,17 @@ impl LightCatalog { .and_then(atmo_of) .unwrap_or(Atmosphere::DEFAULT); - // Collect local spheres covering `pos` with their blendAlpha, strongest first. + // Collect every local sphere CONTAINING `pos` (`dist ≤ outer`), then apply them + // **farthest first** so the nearest lands last and dominates. + // + // The order is by DISTANCE, not by blend weight — byte-VERIFIED (`dn_light_select 0x6d2d00`, + // wow-re `ctb.md` + `merge.md`, the latter's ordering corrected by an emulator difftest + // oracle): the client pushes the in-radius rows into a **max-heap keyed on distance** and + // drains it root-first, calling `dn_record_overblend 0x6d30e0(dst, row, w)` for **every** + // entry including a lone one. Sorting by weight instead (what we did before decision 1104) + // inverts the pair whenever a wide sphere out-weighs a tight near one, and then the near + // zone's own palette gets *diluted* by its neighbour instead of overwriting it — the + // Stranglethorn river that read muddy yellow-green instead of LP 26's grey-green. let mut locals: Vec<(f32, &Light)> = self .lights .iter() @@ -501,15 +515,14 @@ impl LightCatalog { .map(|i| (l.pos[i] - pos[i]).powi(2)) .sum::() .sqrt(); - let a = blend_alpha(d, l.falloff_start, l.falloff_end); - (a > 0.0).then_some((a, l)) + (d <= l.falloff_end).then_some((d, l)) }) .collect(); locals.sort_by(|a, b| b.0.total_cmp(&a.0)); - for (alpha, l) in locals { + for (dist, l) in locals { if let Some(atmo) = atmo_of(l) { - acc = acc.lerp(&atmo, alpha); + acc = acc.lerp(&atmo, blend_alpha(dist, l.falloff_start, l.falloff_end)); } } acc @@ -518,6 +531,12 @@ impl LightCatalog { /// Debug (step 2): dump every light sphere on `map` near `pos` — distance, falloff, `blendAlpha`, /// and its clear-weather ambient + sun colors — then the faithful blended result vs the single /// [`pick_light`] sample, so we can see whether the residual under-red lives in the **values**. + /// + /// Rows print **nearest first** but the blend applies them **farthest first** (see + /// [`Self::sample_blended`]) — the last row listed is the first one merged, and the top row wins. + /// The closing `water` lines are the river/lake and ocean swatch endpoints: a discontinuity there + /// across two nearby positions is the Tirisfal→Silverpine class of bug (decision 1104), and it is + /// invisible in the ambient/sun columns because the water rows are their own bands. pub fn debug_blend(&self, map: u32, pos: [f32; 3], time: u32) { let mut rows: Vec<(f32, f32, &Light)> = self .lights @@ -585,6 +604,22 @@ impl LightCatalog { c(blended.ambient), c(blended.sun_diffuse) ); + // The water swatch is what the liquid materials actually read (`WowLighting.water_*`), and it + // rides this same blend — print it so "the water popped here" is one command, not a session. + println!( + " => water river: shallow {} a={:.2} deep {} a={:.2}", + c(blended.water_river[0]), + blended.water_river_alpha[0], + c(blended.water_river[1]), + blended.water_river_alpha[1], + ); + println!( + " => water ocean: shallow {} a={:.2} deep {} a={:.2}", + c(blended.water_ocean[0]), + blended.water_ocean_alpha[0], + c(blended.water_ocean[1]), + blended.water_ocean_alpha[1], + ); } /// Debug: the **per-weather-slot** resolve at `pos` — for each of the five `Light.dbc` param diff --git a/crates/benilla-formats/src/light/atmosphere.rs b/crates/benilla-formats/src/light/atmosphere.rs index 6f6f0e8d..bd54063e 100644 --- a/crates/benilla-formats/src/light/atmosphere.rs +++ b/crates/benilla-formats/src/light/atmosphere.rs @@ -54,9 +54,10 @@ pub(super) const LP_HIGHLIGHT: usize = 1; /// `LightParams.dbc` field indices of the **water-blend alphas** — the from-above swatch's depth-alpha /// ramp endpoints (`swatch.a = lerp(shallow, deep, V)`; the runtime reads these into `gWorldLight+0x114..` /// via `FUN_006b6b60`). Layout follows the glow shift: glow is field 4 (+0x10), so these are fields 5–8 -/// (+0x14..+0x20). **[VERIFY against a real file/trace]** — confirmed by `probe_water_pickblend` that -/// LP 26 (STV) waterShallow ≈ 0.85 matches the WoW.21 swatch alpha 216/255; if a future file disagrees, -/// re-check the offset (cf. the glow +0x0C-vs-+0x10 trap). +/// (+0x14..+0x20). Cross-check: LP 26 (Stranglethorn) `waterShallow` ≈ 0.85 matches the WoW.21 apitrace +/// swatch alpha 216/255, and the area blend resolves LP 26 whole at that river (test +/// `light_area_blend_order.rs`). If a future file disagrees, re-check the offset (cf. the glow +/// +0x0C-vs-+0x10 trap). pub(super) const LP_WATER_SHALLOW_ALPHA: usize = 5; pub(super) const LP_WATER_DEEP_ALPHA: usize = 6; pub(super) const LP_OCEAN_SHALLOW_ALPHA: usize = 7; diff --git a/crates/benilla-formats/src/page_text_material.rs b/crates/benilla-formats/src/page_text_material.rs new file mode 100644 index 00000000..f70f2415 --- /dev/null +++ b/crates/benilla-formats/src/page_text_material.rs @@ -0,0 +1,61 @@ +//! `PageTextMaterial.dbc` — the book-frame material lookup (decision 1105). +//! +//! A readable carries a material *id* (a book item template's `PageMaterial`, a +//! `GAMEOBJECT_TYPE_TEXT` object's template `data[2]`); the reference resolves it to a **basename** +//! through this table and hands that string to Lua as `ItemTextGetMaterial()`, which paints the +//! four corner textures from `Interface\ItemTextFrame\ItemText--TopLeft` … and picks the +//! page's font/text colour (`ItemTextFrame.lua` l.63-104). `"Parchment"` is the special one: the +//! Lua hides the corner art for it, and it is also the substitute when the getter returns nothing. +//! +//! **The resolve is byte-verified** (`ItemTextGetMaterial 0x4e39f0`): the GameObject leg calls +//! `0x5f5950` — template-attribute `0x11` → the type's `data[]` slot → the id — then bounds-checks +//! `0 < id <= [0xc0da20]` against the DBC's id-indexed table `[0xc0da1c]` and returns `[row+4]`, +//! the row's **second** field. Out of range, or `id <= 0`, pushes `nil` (→ the Lua's Parchment). +//! +//! The 5875 schema was read byte-level from the real file (VERIFIED at decision time): WDBC header +//! `record_count = 6`, `field_count = 2`, `record_size = 8`, string block 48 bytes — two 4-byte +//! fields, the second a string ref. The six rows are `1 Parchment · 2 Stone · 3 Marble · 4 Silver · +//! 5 Bronze · 6 Valentine`. + +use std::collections::HashMap; + +use anyhow::{Context, Result}; +use benilla_dbc::{FieldType, Schema, SchemaField}; + +use crate::chain::Chain; +use crate::dbc::{parse, str_at, u32_at}; + +const PAGE_TEXT_MATERIAL: &str = "DBFilesClient\\PageTextMaterial.dbc"; + +/// `PageTextMaterial.dbc`: material id → basename (the `ItemText--` stem). +pub struct PageTextMaterialCatalog { + by_id: HashMap, +} + +impl PageTextMaterialCatalog { + /// The basename for a material id, or `None` for `0`/an id the table doesn't carry — the + /// reference's own `nil`, which `ItemTextFrame.lua` substitutes with "Parchment" (and then + /// hides the corner art, since Parchment has none). + pub fn name(&self, id: u32) -> Option<&str> { + self.by_id.get(&id).map(String::as_str) + } +} + +/// Load `PageTextMaterial.dbc` into an id → basename map (see the module doc for the verified +/// schema). +pub fn load_page_text_material_catalog(chain: &mut Chain) -> Result { + let bytes = chain + .read_file(PAGE_TEXT_MATERIAL) + .context("reading PageTextMaterial.dbc")?; + let mut schema = Schema::new("PageTextMaterial"); + schema.add_field(SchemaField::new("ID", FieldType::UInt32)); + schema.add_field(SchemaField::new("Name", FieldType::String)); + let set = parse(&bytes, schema, "PageTextMaterial.dbc")?; + let mut by_id = HashMap::new(); + for r in set.records() { + if let (Some(id), Some(name)) = (u32_at(r, 0), str_at(&set, r, 1)) { + by_id.insert(id, name); + } + } + Ok(PageTextMaterialCatalog { by_id }) +} diff --git a/crates/benilla-formats/src/spells/catalog_tests.rs b/crates/benilla-formats/src/spells/catalog_tests.rs index 3e6eca9b..c81a2442 100644 --- a/crates/benilla-formats/src/spells/catalog_tests.rs +++ b/crates/benilla-formats/src/spells/catalog_tests.rs @@ -90,7 +90,10 @@ fn real_spell_catalog_reads_ranged_attributes() { // the client's own melee-substitution trigger (decision 0231); an ordinary spell doesn't. // A column slip on Effect[0] fails here. let attack = cat.get(6603).expect("Attack"); - assert_eq!(attack.effect_1, 78, "6603 Effect[0] == SPELL_EFFECT_ATTACK"); + assert_eq!( + attack.effects[0], 78, + "6603 Effect[0] == SPELL_EFFECT_ATTACK" + ); assert!(attack.is_melee_auto_attack()); assert!(!fireball.is_melee_auto_attack()); assert!( @@ -307,20 +310,23 @@ fn real_spell_catalog_pins_the_skin_latch_effects() { // Skinning (8613) — `0x4b2623: cmp [esi+0xf4], 0x5f`. assert_eq!( - cat.get(8613).unwrap().effect_1, + cat.get(8613).unwrap().effects[0], crate::SPELL_EFFECT_SKINNING, "Skinning carries SPELL_EFFECT_SKINNING (95 == 0x5f)" ); // Remove Insignia (22027) — the `[0xb700e8]` half, `0x4b2632: cmp [esi+0xf4], 0x74`. assert_eq!( - cat.get(22027).unwrap().effect_1, + cat.get(22027).unwrap().effects[0], 0x74, "Remove Insignia carries SPELL_EFFECT_SKIN_PLAYER_CORPSE (116 == 0x74)" ); // Nothing an ordinary caster starts with does — the latch stays empty for a non-skinner. - assert_ne!(cat.get(133).unwrap().effect_1, crate::SPELL_EFFECT_SKINNING); assert_ne!( - cat.get(6247).unwrap().effect_1, + cat.get(133).unwrap().effects[0], + crate::SPELL_EFFECT_SKINNING + ); + assert_ne!( + cat.get(6247).unwrap().effects[0], crate::SPELL_EFFECT_SKINNING ); } @@ -791,7 +797,7 @@ fn real_crafting_columns_read_created_item_and_focus() { ] { let d = cat.get(spell).expect("recipe in the catalog"); assert_eq!( - d.effect_1, SPELL_EFFECT_CREATE_ITEM, + d.effects[0], SPELL_EFFECT_CREATE_ITEM, "spell {spell} creates" ); assert_eq!(d.effect_item_type[0], item, "spell {spell} created item"); @@ -806,7 +812,7 @@ fn real_crafting_columns_read_created_item_and_focus() { // The openers carry effect 47 and no product: Tailoring 3908, Enchanting 7411. for opener in [3908u32, 7411] { let d = cat.get(opener).expect("opener in the catalog"); - assert_eq!(d.effect_1, SPELL_EFFECT_TRADE_SKILL, "opener {opener}"); + assert_eq!(d.effects[0], SPELL_EFFECT_TRADE_SKILL, "opener {opener}"); assert_eq!( d.effect_item_type, [0; 3], "opener {opener} creates nothing" @@ -828,14 +834,14 @@ fn the_tooltip_gates_read_effect_and_mask() { assert!(attribute_passive.tooltip_omits_cast_line()); // 6603 "Attack"'s shape: Effect[0] = 78, attributes 0x10 (NOT the passive bit). let auto_attack = SpellDisplay { - effect_1: 78, + effects: [78, 0, 0], attributes: 0x10, ..Default::default() }; assert!(!auto_attack.passive, "the attribute bit is clear"); assert!(auto_attack.tooltip_omits_cast_line(), "the Effect[0] leg"); let trade_skill = SpellDisplay { - effect_1: 47, + effects: [47, 0, 0], ..Default::default() }; assert!(trade_skill.tooltip_omits_cast_line()); @@ -948,7 +954,7 @@ fn real_item_target_family_and_its_gate_columns() { // (`0x495d60`'s loop, `495de4`–`496050`); [`SpellDisplay`] carries only slot 0. That is // byte-equivalent on shipped data and this is why: across the whole item-target family, not // one row puts its enchant effect anywhere but slot 0. Read raw, since the catalog itself - // only keeps `effect_1`. + // only keeps `effects[0]`. let raw = chain.read_file(SPELL).expect("Spell.dbc"); let set = parse(&raw, spell_schema(), "Spell.dbc").expect("parse Spell.dbc"); let is_enchant = |e: u32| { diff --git a/crates/benilla-formats/src/spells/display.rs b/crates/benilla-formats/src/spells/display.rs index 5a473230..e61354d9 100644 --- a/crates/benilla-formats/src/spells/display.rs +++ b/crates/benilla-formats/src/spells/display.rs @@ -56,9 +56,14 @@ pub struct SpellDisplay { /// three exclusions ([`Self::in_spellbook`]). Nonzero keeps the spell out of the book; reads /// 0 for every ordinary player spell. pub cast_ui: u32, - /// `Effect[0]` (column 61, module docs) — the first effect type. Only - /// [`Self::is_melee_auto_attack`] consumes it (`== SPELL_EFFECT_ATTACK`). - pub effect_1: u32, + /// `Effect[3]` (column 61 == [`COL_EFFECT_1`], module docs) — each effect's type. Slot 0 is what + /// almost every consumer wants (the auto-attack gate [`Self::is_melee_auto_attack`], the + /// tradeskill/enchant/duel classifications); the trainer's icon law is the one caller that scans + /// all three, hunting a learn-wrapper effect in any slot (wow-re + /// `system/ui/scratch/spell-icon-substitution-law.md` §1, `0x4d8fed`'s three-slot loop). Carried + /// as the `[T; 3]` array every other per-effect column already uses — it was a lone `effect_1` + /// until that scan needed the siblings. + pub effects: [u32; 3], /// This spell's `SPELL_EFFECT_OPEN_LOCK` effect, or `None` if it opens no lock. The GameObject /// interact-cast (decisions 0239/0752) matches it against a lock slot across the player's known /// spells: "Opening" for keyless chests, "Mining"/"Herb Gathering"/"Pick Lock" for skill locks. @@ -259,7 +264,7 @@ impl Default for SpellDisplay { attributes_ex3: 0, passive: false, cast_ui: 0, - effect_1: 0, + effects: [0, 0, 0], open_lock: None, spell_level: 0, dispel: 0, @@ -452,7 +457,7 @@ impl SpellDisplay { /// carrying it is 6603 "Attack" (its `SpellIconID` → the `Temp` placeholder the client never /// shows, substituting the main-hand weapon icon instead). pub fn is_melee_auto_attack(&self) -> bool { - self.effect_1 == SPELL_EFFECT_ATTACK + self.effects[0] == SPELL_EFFECT_ATTACK } /// The spell **tooltip's** passive gate (wow-re `tooltip-content-law.md` §3.4): the @@ -469,7 +474,7 @@ impl SpellDisplay { pub fn tooltip_omits_cast_line(&self) -> bool { self.passive || matches!( - self.effect_1, + self.effects[0], SPELL_EFFECT_TRADE_SKILL | SPELL_EFFECT_ATTACK ) } @@ -480,7 +485,7 @@ impl SpellDisplay { /// show a pie AND their presses can never be cooldown-refused. pub fn cooldown_query_excluded(&self) -> bool { matches!( - self.effect_1, + self.effects[0], SPELL_EFFECT_TRADE_SKILL | SPELL_EFFECT_ATTACK ) } diff --git a/crates/benilla-formats/src/spells/mod.rs b/crates/benilla-formats/src/spells/mod.rs index 5ef85405..69418f41 100644 --- a/crates/benilla-formats/src/spells/mod.rs +++ b/crates/benilla-formats/src/spells/mod.rs @@ -259,7 +259,14 @@ const COL_EFFECT_MISC_1: usize = 106; const COL_EFFECT_TRIGGER_1: usize = 109; /// `SpellEffects` value `36` — `SPELL_EFFECT_LEARN_SPELL`: a trainer's learn wrapper carries it, and /// its `EffectTriggerSpell` is the ability the player ends up with (decision 0247's taught spell). -const SPELL_EFFECT_LEARN_SPELL: u32 = 36; +pub const SPELL_EFFECT_LEARN_SPELL: u32 = 36; +/// `SpellEffects` value `57` — `SPELL_EFFECT_LEARN_PET_SPELL`, the learn wrapper's pet twin. The +/// trainer's **icon** law accepts either in its three-slot wrapper scan (byte-verified: the paired +/// `cmp ecx,0x24` / `cmp ecx,0x39` at `0x4d8ff5`/`0x4d8ffa`, wow-re +/// `system/ui/scratch/spell-icon-substitution-law.md` §1). [`SpellCatalog::learned_spell`]'s +/// **display** hop deliberately stays 36-only — that is decision 0247's verified grouping hop, and +/// widening it is a separate question nothing has asked. +pub const SPELL_EFFECT_LEARN_PET_SPELL: u32 = 57; /// `SpellEffects` value `SPELL_EFFECT_OPEN_LOCK` — the lock-opening effect the GameObject /// interact-cast matches (decision 0239; RE `cursor-system.md` §8, the client's @@ -634,7 +641,7 @@ pub fn load_spell_catalog(chain: &mut Chain) -> Result { attributes_ex3: u32_at(r, COL_ATTRIBUTES_EX3).unwrap_or(0), passive: attributes & ATTR_PASSIVE != 0, cast_ui: u32_at(r, COL_CAST_UI).unwrap_or(0), - effect_1: u32_at(r, COL_EFFECT_1).unwrap_or(0), + effects: [0, 1, 2].map(|i| u32_at(r, COL_EFFECT_1 + i).unwrap_or(0)), spell_level: u32_at(r, COL_SPELL_LEVEL).unwrap_or(0), // Scan the three effects for OPEN_LOCK and take that effect's LockType (its // EffectMiscValue) together with the value inputs the lock resolver compares diff --git a/crates/benilla-formats/tests/light_area_blend_order.rs b/crates/benilla-formats/tests/light_area_blend_order.rs new file mode 100644 index 00000000..c32f4ad0 --- /dev/null +++ b/crates/benilla-formats/tests/light_area_blend_order.rs @@ -0,0 +1,135 @@ +//! **The water swatch rides the area blend, and the blend applies spheres farthest-first.** +//! +//! Two halves of one law, byte-VERIFIED in wow-re (`system/lighting/scratch/ctb.md` §`0x6d2d00` +//! + `scratch/merge.md`, the latter's drain reading corrected by an emulator difftest oracle): +//! +//! 1. `dn_light_select 0x6d2d00` pushes every `Light.dbc` row with `dist ≤ outer` into a **max-heap +//! keyed on DISTANCE** and drains it root-first — so the farthest light merges first and the +//! **nearest lands last and dominates**. A point inside the tightest sphere's inner radius +//! therefore resolves that sphere's `LightParams` **exactly**, whatever else overlaps it. +//! 2. `dn_record_overblend 0x6d30e0` merges **all 18 colour slots** of the gather record per light — +//! its step-9 loop `+0x34..+0x40` is precisely IntBand rows 14–17, the ocean/river swatch. The +//! water tint is an ordinary band; the client has no single-sphere pick anywhere in that path. +//! +//! Both were wrong before decision 1104, and the two errors hid each other. Ordering by blend +//! *weight* let a wide distant sphere land last and dilute a zone that fully contained the camera — +//! which made the blended water read muddy, which is what the `pick_light` water split was +//! introduced to dodge. The split then made the tint **discontinuous**: `pick_light` switches at a +//! tighter sphere's *outer* radius, exactly where that sphere's own weight is still zero, so the +//! Tirisfal→Silverpine border snapped green water to near-black brown in a single step (the +//! director's report) while ambient, sun and fog crossed it without a flicker. + +use std::path::PathBuf; + +use benilla_formats::{Chain, LightCatalog, Submersion}; + +fn vanilla_data_dir() -> PathBuf { + PathBuf::from(env!("CARGO_MANIFEST_DIR")).join("../../WoW/Data") +} + +/// Eastern Kingdoms. +const MAP_EK: u32 = 0; +/// Half-minutes; 1440 = noon. +const NOON: u32 = 1440; + +fn rgb(c: [f32; 3]) -> [i32; 3] { + c.map(|v| (v * 255.0).round() as i32) +} + +fn catalog() -> Option { + let data = vanilla_data_dir(); + if !data.is_dir() { + eprintln!("skipping: vanilla client not present at {}", data.display()); + return None; + } + let mut chain = Chain::open(&data).expect("open vanilla patch chain"); + Some(LightCatalog::load(&mut chain).expect("load Light/LightParams/*Band")) +} + +fn blend(cat: &LightCatalog, pos: [f32; 3]) -> benilla_formats::Atmosphere { + cat.sample_blended(MAP_EK, pos, NOON, false, Submersion::Dry, false) +} + +/// The director's two `.go` pins, 16 yd apart across the Tirisfal → Silverpine border. Light 4 +/// (falloff 985→1437 yd, `LightParams` 40) *enters* between them — at pin B the eye is 1434 yd out, +/// three yards inside its outer radius, where its blend weight is 0.006. The area blend must +/// therefore barely move; the old `pick_light` swatch instead switched wholesale to LP 40's rows. +#[test] +fn water_swatch_does_not_snap_at_the_tirisfal_silverpine_border() { + let Some(cat) = catalog() else { return }; + let a = blend(&cat, [1391.07, 641.39, 35.37]); + let b = blend(&cat, [1401.03, 628.90, 35.42]); + + for (label, x, y) in [ + ("river shallow", a.water_river[0], b.water_river[0]), + ("river deep", a.water_river[1], b.water_river[1]), + ("ocean shallow", a.water_ocean[0], b.water_ocean[0]), + ("ocean deep", a.water_ocean[1], b.water_ocean[1]), + ] { + let (x, y) = (rgb(x), rgb(y)); + let step = (0..3).map(|i| (x[i] - y[i]).abs()).max().unwrap_or(0); + assert!( + step <= 2, + "{label} must cross the border continuously: {x:?} -> {y:?} (max channel step {step})" + ); + } + + // The exact swatch on the Tirisfal side, so a future re-derivation moves this on purpose. + assert_eq!( + rgb(a.water_river[0]), + [82, 93, 46], + "river shallow at pin A" + ); + assert_eq!(rgb(a.water_river[1]), [60, 88, 89], "river deep at pin A"); + // The `pick_light` answers the old code committed — the snap, in both its endpoints. + assert_ne!( + rgb(b.water_river[1]), + [35, 28, 37], + "pin B must not commit LightParams 40's raw deep row through a single-sphere pick" + ); +} + +/// Inside the tightest sphere's inner radius the nearest light merges **last** at weight 1, which is +/// a full replace — so the blend equals that sphere's `LightParams` outright, even with two wider +/// spheres also covering the point (here Light 31 / `LightParams` 46 reaches it at weight 0.195). +/// Sorting the merge by weight instead put that 0.195 last and smeared 20% of a distant zone's +/// palette over a zone the camera stands in the middle of. +#[test] +fn the_nearest_sphere_dominates_where_it_is_at_full_weight() { + let Some(cat) = catalog() else { return }; + let pos = [2250.0, -750.0, 50.0]; + let blended = blend(&cat, pos); + let picked = cat.sample(MAP_EK, pos, NOON, false); + + assert_eq!(rgb(blended.ambient), rgb(picked.ambient), "ambient"); + assert_eq!(rgb(blended.sun_diffuse), rgb(picked.sun_diffuse), "diffuse"); + assert_eq!(rgb(blended.fog_color), rgb(picked.fog_color), "fog"); + assert_eq!( + rgb(blended.water_river[0]), + rgb(picked.water_river[0]), + "river shallow" + ); + // LightParams 40's own rows, off the shipped bands — the identity, not just the agreement. + assert_eq!(rgb(blended.ambient), [80, 63, 79]); + assert_eq!(rgb(blended.water_river[0]), [82, 64, 49]); + assert_eq!(rgb(blended.water_river[1]), [35, 28, 37]); +} + +/// The Stranglethorn anchor the `pick_light` split was built on (apitrace WoW.21: the river swatch +/// reads `LightParams` 26 exactly, alpha 216/255). It never needed a pick — Light 9 covers the river +/// at full weight, so the faithful nearest-last blend commits LP 26 whole. +#[test] +fn the_stranglethorn_river_swatch_is_lightparams_26() { + let Some(cat) = catalog() else { return }; + let stv = blend(&cat, [-13333.3, 0.0, 50.0]); + + assert_eq!(rgb(stv.water_river[0]), [90, 140, 140], "LP 26 IntBand 16"); + assert_eq!(rgb(stv.water_river[1]), [28, 55, 64], "LP 26 IntBand 17"); + assert_eq!(rgb(stv.water_ocean[0]), [90, 171, 140], "LP 26 IntBand 14"); + assert_eq!(rgb(stv.water_ocean[1]), [28, 44, 44], "LP 26 IntBand 15"); + assert!( + (stv.water_river_alpha[0] - 216.0 / 255.0).abs() < 0.01, + "the trace's shallow swatch alpha, got {}", + stv.water_river_alpha[0] + ); +} diff --git a/crates/benilla-protocol/src/events.rs b/crates/benilla-protocol/src/events.rs index 0f9f2a95..b1ff56ab 100644 --- a/crates/benilla-protocol/src/events.rs +++ b/crates/benilla-protocol/src/events.rs @@ -1016,6 +1016,15 @@ pub enum SessionEvent { equip_error: Option, item: Option<(u32, u32)>, }, + /// `SMSG_PAGE_TEXT_QUERY_RESPONSE` — one page of a book, answering `CMSG_PAGE_TEXT_QUERY`. + /// The ask-once page cache both readables reach: a readable item template's `PageText` and a + /// `GAMEOBJECT_TYPE_TEXT` object's `data[0]` (decision 1105). `next_page_id == 0` ends the + /// chain; vmangos pushes the whole chain in answer to the first query. + PageText { + page_id: u32, + text: String, + next_page_id: u32, + }, /// `SMSG_ITEM_TEXT_QUERY_RESPONSE` — a letter's body text, answering `CMSG_ITEM_TEXT_QUERY` /// (a mail's nonzero `item_text_id` triggers the ask-once fetch; decision 0544). MailItemText { text_id: u32, text: String }, diff --git a/crates/benilla-protocol/src/events/decode.rs b/crates/benilla-protocol/src/events/decode.rs index aec7d420..1e780bb0 100644 --- a/crates/benilla-protocol/src/events/decode.rs +++ b/crates/benilla-protocol/src/events/decode.rs @@ -650,6 +650,15 @@ pub fn decode(packet: ServerPacket) -> Vec { racial_leader, }] } + ServerPacket::PageTextQueryResponse { + page_id, + text, + next_page_id, + } => vec![SessionEvent::PageText { + page_id, + text, + next_page_id, + }], ServerPacket::GameObjectQueryResponse { entry, info } => { let event = match info { Some(i) => SessionEvent::GameObjectInfo { diff --git a/crates/benilla-protocol/src/messages/mod.rs b/crates/benilla-protocol/src/messages/mod.rs index b9e1e0b7..eff914a4 100644 --- a/crates/benilla-protocol/src/messages/mod.rs +++ b/crates/benilla-protocol/src/messages/mod.rs @@ -37,6 +37,7 @@ mod movement; pub mod opcode; mod opcode_names; mod packet; +mod page_text; mod parse; mod pet; mod pose; @@ -128,6 +129,7 @@ pub use mirror_timer::{ pub use movement::{JumpInfo, MoveMode, MovementInfo, SpeedKind, TransportPose}; pub use opcode_names::opcode_name; pub use packet::{CreatureQueryInfo, MonsterMoveFacing, ServerPacket}; +pub use page_text::page_text_query; pub use parse::parse_server; pub use pet::{ pet_abandon, pet_action, pet_cancel_aura, pet_rename, pet_set_action, pet_spell_autocast, diff --git a/crates/benilla-protocol/src/messages/opcode.rs b/crates/benilla-protocol/src/messages/opcode.rs index 997d2cd8..20d1e91f 100644 --- a/crates/benilla-protocol/src/messages/opcode.rs +++ b/crates/benilla-protocol/src/messages/opcode.rs @@ -23,6 +23,15 @@ pub const SMSG_GAMEOBJECT_QUERY_RESPONSE: u16 = 0x005F; // 95 /// VERIFIED vmangos `Opcodes_1_12_1.h`: 93. Answers `CMSG_QUEST_QUERY`; body in /// [`super::quest::read_quest_query_response`] ([`super::QuestTemplate`]). pub const SMSG_QUEST_QUERY_RESPONSE: u16 = 0x005D; // 93 + +// The **page-text** query pair (VERIFIED vmangos `Opcodes_1_12_1.h`: 90/91) — the ask-once cache +// behind every readable BOOK, and a different one from the mail letter's `CMSG_ITEM_TEXT_QUERY` +// further down: a `PageText.wdb` id names one *page*, chained by `nextPageId`, and it is what a +// readable item TEMPLATE (`PageText`) and a world book/plaque GameObject (`GAMEOBJECT_TYPE_TEXT`'s +// `data[0]`) both read from. Bodies in [`super::page_text`]; decision 1105. +pub const CMSG_PAGE_TEXT_QUERY: u16 = 0x005A; // 90 +pub const SMSG_PAGE_TEXT_QUERY_RESPONSE: u16 = 0x005B; // 91 + pub const SMSG_NEW_WORLD: u16 = 0x003E; // The far-teleport preamble pair (VERIFIED vmangos `Opcodes_1_12_1.h`: 63/64). TRANSFER_PENDING's // optional transport block decides how NEW_WORLD's coordinates read — boat-local when present, diff --git a/crates/benilla-protocol/src/messages/packet.rs b/crates/benilla-protocol/src/messages/packet.rs index 19ff5719..481a0377 100644 --- a/crates/benilla-protocol/src/messages/packet.rs +++ b/crates/benilla-protocol/src/messages/packet.rs @@ -260,6 +260,16 @@ pub enum ServerPacket { entry: u32, info: Option, }, + /// `SMSG_PAGE_TEXT_QUERY_RESPONSE` — one page of a book, answering `CMSG_PAGE_TEXT_QUERY` + /// (the ask-once page cache every readable reaches: a readable item template's `PageText` and a + /// `GAMEOBJECT_TYPE_TEXT` object's `data[0]`, decision 1105). Pages **chain** — + /// `next_page_id == 0` is the last one — and vmangos answers a single query with one of these + /// per page of the whole chain. + PageTextQueryResponse { + page_id: u32, + text: String, + next_page_id: u32, + }, /// `SMSG_GAMEOBJECT_CUSTOM_ANIM` — a GameObject plays a one-shot Custom animation. Payload /// VERIFIED vmangos `GameObject::SendGameObjectCustomAnim`: `u64 guid, u32 animId`. The /// client arms GO substate `8 + animId` (AnimationData 153..156, `animId >= 4` rejected) — @@ -1110,6 +1120,7 @@ impl ServerPacket { ServerPacket::CreatureQueryResponse { .. } => "SMSG_CREATURE_QUERY_RESPONSE".into(), ServerPacket::PetNameQueryResponse { .. } => "SMSG_PET_NAME_QUERY_RESPONSE".into(), ServerPacket::GameObjectQueryResponse { .. } => "SMSG_GAMEOBJECT_QUERY_RESPONSE".into(), + ServerPacket::PageTextQueryResponse { .. } => "SMSG_PAGE_TEXT_QUERY_RESPONSE".into(), ServerPacket::GameObjectCustomAnim { .. } => "SMSG_GAMEOBJECT_CUSTOM_ANIM".into(), ServerPacket::FishNotHooked => "SMSG_FISH_NOT_HOOKED".into(), ServerPacket::FishEscaped => "SMSG_FISH_ESCAPED".into(), diff --git a/crates/benilla-protocol/src/messages/page_text.rs b/crates/benilla-protocol/src/messages/page_text.rs new file mode 100644 index 00000000..c5f8a8fb --- /dev/null +++ b/crates/benilla-protocol/src/messages/page_text.rs @@ -0,0 +1,92 @@ +//! The **page-text** query pair (decision 1105) — the ask-once cache behind every readable *book*. +//! +//! Distinct from the mail letter's `CMSG_ITEM_TEXT_QUERY` ([`super::mail`]), which fetches one +//! whole player-written body by an *instance* text id. A `PageText.wdb` id names a single **page**, +//! and pages **chain**: each answer carries the next page's id (`0` = last). Two things read from +//! this cache, and both reach it the same way — the client opens its reader on an object guid and +//! asks the object for its page id: +//! +//! - a readable **item template** (`ItemPrototype::PageText`) — a book in your bags; +//! - a **`GAMEOBJECT_TYPE_TEXT`** world object (its template `data[0]`) — a book, plaque or sign +//! lying in the world. +//! +//! The client's request carries the asking object's guid after the page id (VERIFIED: the cache's +//! request writer `0x564730` appends the 8-byte guid at `0x56485d` under the per-cache "requests +//! carry a guid" flag `[cache+0x38]` — the very address vmangos cites in +//! `QueryPageText::ReadFromWorldPacket` when it reads the tail as optional). vmangos resolves the +//! page from the id alone and skips the guid. + +use std::io; + +use crate::wire::{read_cstring, read_u32_le}; + +/// Body of `CMSG_PAGE_TEXT_QUERY` (opcode `0x005A`/90 — VERIFIED vmangos +/// `Server/Protocol/Opcodes_1_12_1.h`): `u32 pageId` then the asking object's `u64 guid` (VERIFIED +/// `QueryPageText::ReadFromWorldPacket`, which reads the guid only `if (rpos() < size())` and +/// discards it; the real client writes it — see the module doc). +pub fn page_text_query(page_id: u32, guid: u64) -> Vec { + let mut body = Vec::with_capacity(12); + body.extend_from_slice(&page_id.to_le_bytes()); + body.extend_from_slice(&guid.to_le_bytes()); + body +} + +/// Read `SMSG_PAGE_TEXT_QUERY_RESPONSE` (VERIFIED vmangos `Server/Packets/Query.cpp`, +/// `PageTextQueryResponse::AppendBodyTo`): `u32 pageId, cstr text, u32 nextPageId`. +/// +/// vmangos answers **one query with the whole chain** — `HandlePageTextQueryOpcode` loops on +/// `next_page`, sending one of these per page — so asking for page 1 lands every page of the book +/// unsolicited. A page the server doesn't know answers `"Item page missing."` with `nextPageId = 0`, +/// which terminates the chain rather than erroring. +pub(super) fn read_page_text_query_response(r: &mut &[u8]) -> io::Result<(u32, String, u32)> { + Ok((read_u32_le(r)?, read_cstring(r)?, read_u32_le(r)?)) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::messages::{opcode, parse_server, ServerPacket}; + + #[test] + fn query_body_is_page_id_then_guid() { + assert_eq!( + page_text_query(333, 0xF110_0000_0000_0042), + [ + 0x4D, 0x01, 0x00, 0x00, // pageId 333 + 0x42, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0xF1, // guid + ] + ); + } + + #[test] + fn response_reads_the_chain_link() { + let mut body = 333u32.to_le_bytes().to_vec(); + body.extend_from_slice(b"Page one.\0"); + body.extend_from_slice(&334u32.to_le_bytes()); + match parse_server(opcode::SMSG_PAGE_TEXT_QUERY_RESPONSE, &body).unwrap() { + ServerPacket::PageTextQueryResponse { + page_id, + text, + next_page_id, + } => { + assert_eq!(page_id, 333); + assert_eq!(text, "Page one."); + assert_eq!(next_page_id, 334); + } + other => panic!("expected PageTextQueryResponse, got {}", other.name()), + } + } + + #[test] + fn last_page_terminates_the_chain() { + let mut body = 9u32.to_le_bytes().to_vec(); + body.extend_from_slice(b"The end.\0"); + body.extend_from_slice(&0u32.to_le_bytes()); + match parse_server(opcode::SMSG_PAGE_TEXT_QUERY_RESPONSE, &body).unwrap() { + ServerPacket::PageTextQueryResponse { next_page_id, .. } => { + assert_eq!(next_page_id, 0) + } + other => panic!("expected PageTextQueryResponse, got {}", other.name()), + } + } +} diff --git a/crates/benilla-protocol/src/messages/parse.rs b/crates/benilla-protocol/src/messages/parse.rs index 0726f45d..3cb234ea 100644 --- a/crates/benilla-protocol/src/messages/parse.rs +++ b/crates/benilla-protocol/src/messages/parse.rs @@ -12,7 +12,7 @@ use crate::wire::{ use super::{ action_bar, area_trigger, attack, bank, channel, chat, combat_log, death, duel, gameobject, - gossip, group, items, loot, mail, mirror_timer, monster_move, movement, opcode, pet, + gossip, group, items, loot, mail, mirror_timer, monster_move, movement, opcode, page_text, pet, progression, quest, social, spellbook, spells, taxi, trade, trainer, update_object, vendor, world_state, Character, CreatureQueryInfo, MoveMode, ServerPacket, SpeedKind, }; @@ -808,6 +808,14 @@ pub fn parse_server(opcode: u16, body: &[u8]) -> io::Result { let (entry, info) = gameobject::read_gameobject_query_response(&mut r)?; ServerPacket::GameObjectQueryResponse { entry, info } } + opcode::SMSG_PAGE_TEXT_QUERY_RESPONSE => { + let (page_id, text, next_page_id) = page_text::read_page_text_query_response(&mut r)?; + ServerPacket::PageTextQueryResponse { + page_id, + text, + next_page_id, + } + } opcode::SMSG_GAMEOBJECT_CUSTOM_ANIM => { let (guid, anim_id) = gameobject::read_gameobject_custom_anim(&mut r)?; ServerPacket::GameObjectCustomAnim { guid, anim_id } diff --git a/crates/benilla-protocol/src/world/writer/gameobject.rs b/crates/benilla-protocol/src/world/writer/gameobject.rs index 0364bce9..87f33f90 100644 --- a/crates/benilla-protocol/src/world/writer/gameobject.rs +++ b/crates/benilla-protocol/src/world/writer/gameobject.rs @@ -34,4 +34,17 @@ impl WorldWriter { &messages::gameobject_query(entry, guid), ) } + + /// Ask for one page of a book (`CMSG_PAGE_TEXT_QUERY`, layout in + /// [`messages::page_text_query`]) — the ask-once page cache a readable reaches when the reader + /// opens on it (decision 1105). `guid` names the asking object (the book item or the + /// `GAMEOBJECT_TYPE_TEXT` world object); the server resolves the page from the id alone. + /// Answered by `SMSG_PAGE_TEXT_QUERY_RESPONSE` — once per page of the whole chain, not just + /// the one asked for. + pub fn page_text_query(&mut self, page_id: u32, guid: u64) -> Result<()> { + self.send( + opcode::CMSG_PAGE_TEXT_QUERY, + &messages::page_text_query(page_id, guid), + ) + } } diff --git a/crates/benilla-ui/src/script/extract.rs b/crates/benilla-ui/src/script/extract.rs index 9d81e7d2..ec9102df 100644 --- a/crates/benilla-ui/src/script/extract.rs +++ b/crates/benilla-ui/src/script/extract.rs @@ -209,9 +209,19 @@ impl UiScript { alpha_gradient: data.alpha_gradient, } } else { + // The draw gate is the TEXTURE slot, never the colour (`0x7706e0`: `+0xcc` + // empty -> emit NOTHING — `texture-color-composition.md` §4, VERIFIED). A + // vertex colour is a tint on whatever texture exists; alone it is not + // drawable content — it survives `SetTexture(nil)` by design ("a tint + // outlives the art it was tinting") and used to leak out of here as a + // solid plate the moment the art was cleared (the white action buttons on + // a character switch; the 2026-07-10 grey wells were the same class). + let has_texture = data.texture.is_some() || data.fill.is_some(); QuadContent::Texture { path: data.texture, - color: texture_color(data.fill, data.vertex_color), + color: has_texture + .then(|| texture_color(data.fill, data.vertex_color)) + .flatten(), additive: data.additive, tex_coords: data.tex_coords, circular: data.circular, diff --git a/crates/benilla-ui/src/script/tests/regions.rs b/crates/benilla-ui/src/script/tests/regions.rs index fb0e264f..4ee69360 100644 --- a/crates/benilla-ui/src/script/tests/regions.rs +++ b/crates/benilla-ui/src/script/tests/regions.rs @@ -526,3 +526,51 @@ fn a_solid_colour_texel_multiplies_with_the_vertex_colour() { "the vertex colour outlives the texel it was multiplying" ); } + +/// The draw gate is the TEXTURE slot, never the colour (`texture-color-composition.md` §4, +/// VERIFIED): `0x7706e0` tests `+0xcc` and emits NOTHING when it is empty, whatever the vertex +/// colour holds. Since the tint deliberately survives `SetTexture(nil)` ("a tint outlives the art +/// it was tinting"), a cleared region used to leak its tint out of extract as a solid plate — an +/// occupied action button going empty on a character switch drew its surviving 1/1/1 usable-tint +/// as a solid WHITE square (decision 1108; the 2026-07-10 grey wells were the same class). +#[test] +fn a_vertex_colour_without_a_texture_draws_nothing() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run( + r#" + local f = CreateFrame("Frame", "Owner") + f:SetPoint("BOTTOMLEFT", 0, 0) + f:SetWidth(100) f:SetHeight(50) + icon = f:CreateTexture("Icon", "ARTWORK") + icon:SetTexture("Interface\\Icons\\Spell_Nature_Sleep") + icon:SetVertexColor(1, 1, 1) + "#, + ) + .unwrap(); + s.resolve(); + let drawn = |s: &UiScript| { + s.extract() + .iter() + .filter(|q| { + matches!(&q.content, QuadContent::Texture { path, color, .. } + if path.is_some() || color.is_some()) + }) + .count() + }; + assert_eq!(drawn(&s), 1, "tinted art draws"); + + // The slot empties: the art clears, the tint stays (distinct storage) — and nothing draws. + s.run("icon:SetTexture(nil)").unwrap(); + assert_eq!( + s.eval::<(f32, f32, f32, f32)>("return icon:GetVertexColor()") + .unwrap(), + (1.0, 1.0, 1.0, 1.0), + "the tint survives the clear" + ); + assert_eq!( + drawn(&s), + 0, + "no texture at +0xcc -> emit NOTHING, never a solid plate of the surviving tint" + ); +} diff --git a/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs b/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs index 7a063069..1e578005 100644 --- a/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs +++ b/crates/benilla-ui/src/script/tests/tooltip_item/bindings.rs @@ -260,3 +260,84 @@ fn set_hyperlink_renders_items_and_ignores_other_links() { .unwrap(); assert!(s.take_errors().is_empty()); } + +/// One tradeskill recipe fixture — the product id is `spell_id + 10_000`, so the tooltip's own +/// name names which recipe the channel actually landed on. +fn ts_recipe(spell_id: u32, name: &str, group: (u32, u32, &str)) -> TradeSkillRecipe { + TradeSkillRecipe { + group: Some((group.0, group.1, group.2.to_string())), + spell_id, + name: name.into(), + difficulty: TradeSkillDifficulty::Optimal, + num_available: 1, + icon: None, + min_made: 1, + max_made: 1, + cooldown_secs: None, + product_item: spell_id + 10_000, + product_inv_type: 0, + product_item_level: 0, + reagents: vec![TradeSkillReagent { + item: spell_id + 20_000, + name: Some(format!("{name} Reagent")), + icon: None, + need: 1, + have: 1, + }], + tools: vec![], + } +} + +/// `SetTradeSkillItem`'s index is a **VISIBLE** row index, not a position in `recipes` — headers +/// interleave with rows, so the two differ the moment any group precedes the recipe. This is the +/// exact mis-index the tradeskill module doc once carried as a known gap: with one header above it, +/// visible row 4 is the SECOND group's recipe, and a raw `recipes[4-1]` lookup would land on the +/// first group's second recipe instead. Both the product channel and the reagent channel are +/// pinned, since the detail-icon and reagent-slot hovers are the two callers that pass a genuine +/// visible index straight through. A HEADER row is a no-op, not a shifted hit. +#[test] +fn set_trade_skill_item_indexes_visible_rows_not_raw_recipes() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.run(r#"local tt = CreateFrame("GameTooltip", "TT")"#) + .unwrap(); + + // Two groups, two recipes each. Visible order: [1] "Bolts" header, [2] Alpha Bolt, + // [3] Beta Bolt, [4] "Cloak" header, [5] Alpha Cloak, [6] Beta Cloak. + s.set_trade_skill(Some(TradeSkillState { + line: 197, + line_name: "Tailoring".into(), + rank: 57, + max_rank: 75, + recipes: vec![ + ts_recipe(1, "Beta Bolt", (1, 2, "Bolts")), + ts_recipe(2, "Alpha Bolt", (1, 2, "Bolts")), + ts_recipe(3, "Beta Cloak", (2, 1, "Cloak")), + ts_recipe(4, "Alpha Cloak", (2, 1, "Cloak")), + ], + repeat_count: 1, + })); + assert_eq!(s.eval::("return GetNumTradeSkills()").unwrap(), 6); + + // Visible row 5 is "Alpha Cloak" (spell 4) — its product is 10_004. A raw recipes[5-1] lookup + // would land on recipe index 4 (out of range, a silent no-op); recipes[4-1] on "Beta Cloak". + // `TTTextLeft1` need not exist when a call renders nothing (what an out-of-range RAW index + // does), so read it defensively — that case must surface as an assert_eq naming the row, not + // as a Lua nil-index panic three lines away from the claim. + let name_at = |s: &mut UiScript, row: i64, reagent: &str| -> String { + s.run(&format!("TT:SetTradeSkillItem({row}{reagent})")) + .unwrap(); + s.eval::("return TTTextLeft1 and TTTextLeft1:GetText() or ''") + .unwrap() + }; + assert_eq!(name_at(&mut s, 5, ""), "Alpha Cloak"); + assert_eq!(name_at(&mut s, 6, ""), "Beta Cloak"); + assert_eq!(name_at(&mut s, 2, ""), "Alpha Bolt"); + + // The reagent channel takes the same mapping. + assert_eq!(name_at(&mut s, 5, ", 1"), "Alpha Cloak Reagent"); + + // A header row resolves to nothing — the hover is a no-op, so the previous render stands + // rather than a neighbouring recipe's tooltip appearing under the cursor. + assert_eq!(name_at(&mut s, 4, ""), "Alpha Cloak Reagent"); +} diff --git a/crates/benilla-ui/src/script/tradeskill/mod.rs b/crates/benilla-ui/src/script/tradeskill/mod.rs index bfe06524..bc4e1cf5 100644 --- a/crates/benilla-ui/src/script/tradeskill/mod.rs +++ b/crates/benilla-ui/src/script/tradeskill/mod.rs @@ -71,7 +71,7 @@ //! VISIBLE index from that flat position on every read, [`super::skills`]'s own by-identity //! persistence pattern adapted to a field this module can't change the type of). //! -//! ## The tooltip channel — KNOWN GAP, not fixed here +//! ## The tooltip channel //! //! `GameTooltip:SetTradeSkillItem(skillIndex [, reagentIndex])` is registered in //! [`super::tooltip_item`], beside `SetMerchantItem`, through the same id-keyed item renderer: with @@ -80,16 +80,10 @@ //! the renderer's own name-only line — the recipe's name for the product channel — rather than a //! no-op, since that fallback already exists and reads better than a blank hover. //! -//! **As of this grouping landing, `tooltip_item`'s `SetTradeSkillItem` still indexes -//! [`TradeSkillState::recipes`] directly by the raw `skillIndex` it's called with** — it does NOT -//! route through [`recipe_at`]'s visible-index mapping (that module is outside this change's file -//! scope). The XML's list-row hover (`BenillaTradeSkillSkillButton_OnEnter`) is guarded here to -//! never call it on a header row, but the product-icon and reagent-slot hovers pass -//! `BenillaTradeSkillFrame.selectedSkill` — a genuine VISIBLE index of a selected recipe — straight -//! through, so once any group precedes the selected recipe's own group, those two hovers will show -//! the WRONG item. `tooltip_item::install`'s `SetTradeSkillItem` closure needs a follow-up edit to -//! resolve through [`recipe_at`] (now `pub(crate)` for exactly this) instead of `ts.recipes.get(...)` -//! directly. +//! `skillIndex` is a **VISIBLE** index and is resolved through [`recipe_at`] (`pub(crate)` for +//! exactly this), never a raw [`TradeSkillState::recipes`] position — headers interleave with rows +//! since the TU-B grouping landed, so a raw index would show the wrong item the moment any group +//! precedes the selected recipe's own. A header index resolves to `None` and the hover is a no-op. use std::collections::HashSet;