From ab909df6d961baa7c2c36e415c299a6be6e50cf4 Mon Sep 17 00:00:00 2001 From: Mathih13 Date: Mon, 5 Oct 2026 16:57:08 +0200 Subject: [PATCH] fix: TargetUnit("npc") selects the interaction NPC, as 1.12.1 does (#800) `TargetUnit("npc")`, `AssistUnit("npc")` and `SpellTargetUnit("npc")` did nothing with a merchant, gossip or quest window open: the app's unit-token resolver answered nobody for `npc`. In 1.12.1 the resolver (`0x515970`) matches `npc` as a whole token (`0x515bec`, an unbounded compare, so `npctarget` stays unknown) and takes the interaction NPC's guid `[0xb4e2d0]`, which `0x4930d0` writes when the window opens. `npc` now resolves to the held unit `InteractNpc` names. Tests cover the three verbs, a closed window and a streamed-out NPC, and `/script TargetUnit("npc")` typed into the stock chat box. Fixes #707 Co-Authored-By: Claude Opus 5.5 (1M context) --- crates/benilla-app/src/spell/targeting/mod.rs | 41 ++++++ crates/benilla-app/src/target/script.rs | 120 ++++++++++++++++++ .../benilla-app/src/ui_script/chat_tests.rs | 2 +- crates/benilla-app/src/ui_script/mod.rs | 2 +- crates/benilla-app/src/ui_unit.rs | 11 +- 5 files changed, 168 insertions(+), 8 deletions(-) diff --git a/crates/benilla-app/src/spell/targeting/mod.rs b/crates/benilla-app/src/spell/targeting/mod.rs index c29b2c34..cea6ca82 100644 --- a/crates/benilla-app/src/spell/targeting/mod.rs +++ b/crates/benilla-app/src/spell/targeting/mod.rs @@ -750,6 +750,47 @@ mod tests { } } + /// `SpellTargetUnit("npc")` binds the interaction NPC (`0x6e6de1` into `0x515970`, whose `npc` + /// compare reads `[0xb4e2d0]`), not the selection; with no window open it names no unit and + /// the cast ends with "Out of range.". + #[test] + fn spell_target_unit_binds_the_interaction_npc() { + use crate::ui_session::InteractNpc; + + let (mut world, rx, ally) = unit_world(10.0); + world + .resource_mut::() + .0 + .insert(ALLY, ally); + world.insert_resource(InteractNpc(Some(ally), Some(ALLY))); + arm(&mut world, HEAL, 0x0002); + spell_target_unit(&mut world, "npc"); + assert!(matches!( + rx.try_recv(), + Ok(ClientCommand::CastSpell { + spell_id: HEAL, + target: Some(ALLY), + }) + )); + assert!(errors(&mut world).is_empty()); + assert!(!world.resource::().active()); + + let (mut world, rx, ally) = unit_world(10.0); + world + .resource_mut::() + .0 + .insert(ALLY, ally); + world.init_resource::(); + arm(&mut world, HEAL, 0x0002); + spell_target_unit(&mut world, "npc"); + assert!(rx.try_recv().is_err(), "no window, no send"); + assert_eq!( + errors(&mut world), + vec![crate::ui_action::CastFail::local(HEAL, 0x59)] + ); + assert!(!world.resource::().active()); + } + /// `SpellCanTargetUnit` resolves a chain as `SpellTargetUnit` does and answers the verdict of /// the unit it ends on (`0x6e6d3a` into `0x515970`, then `0x6e6460`): the ally we and our pet /// target, in range of the spell or not, and nobody past a unit with no target. diff --git a/crates/benilla-app/src/target/script.rs b/crates/benilla-app/src/target/script.rs index 9aca36fb..bc154faf 100644 --- a/crates/benilla-app/src/target/script.rs +++ b/crates/benilla-app/src/target/script.rs @@ -443,4 +443,124 @@ mod tests { "THIRD targets nobody: silent, nothing deselected" ); } + + /// `TargetUnit("npc")` and `AssistUnit("npc")` name the interaction NPC, `[0xb4e2d0]`, which + /// `0x515970` reads on a whole-token `npc` compare (`0x515bec`-`0x515c00`): the vendor whose + /// window is open, and for an assist the unit it targets. With no window open, or the NPC no + /// longer held, `npc` names nobody and the selection stays. + #[test] + fn target_and_assist_unit_name_the_interaction_npc() { + use crate::net::{Guid, ObjectStore, SelfPlayer}; + use crate::ui_session::InteractNpc; + use benilla_ui::script::UiScript; + use bevy::ecs::system::RunSystemOnce; + + const ME: u64 = 1; + const VENDOR: u64 = 0xF130_0000_9800_0001; + const GUARD: u64 = 0xF130_0000_0000_0002; + // `OBJECT_FIELD_TYPE` 2 and `UNIT_FIELD_TARGET` 16, a 2-field guid. + let unit = |target: u64| { + ObjectStore(benilla_protocol::ObjectFields::from_pairs(&[ + (2, 0x09), + (16, target as u32), + (17, (target >> 32) as u32), + ])) + }; + + let mut world = script_world(); + world.spawn((SelfPlayer, Guid(ME), unit(0))); + let vendor = world.spawn((Guid(VENDOR), unit(GUARD))).id(); + let guard = world.spawn((Guid(GUARD), unit(0))).id(); + let index = &mut world.resource_mut::().0; + index.insert(VENDOR, vendor); + index.insert(GUARD, guard); + world.insert_resource(InteractNpc(Some(vendor), Some(VENDOR))); + + let run = |world: &mut World, lua: &str| { + world + .non_send_resource_mut::() + .eval::<()>(lua) + .expect("the binding runs"); + world + .run_system_once( + |mut script: NonSendMut, mut select: ScriptSelect| { + for request in script.take_selection_requests() { + select.select(request); + } + }, + ) + .expect("the applier runs as a one-shot system"); + world.resource::().guid + }; + let reset = |world: &mut World| { + let mut sel = world.resource_mut::(); + sel.target = None; + sel.guid = None; + }; + + assert_eq!(run(&mut world, r#"TargetUnit("npc")"#), Some(VENDOR)); + reset(&mut world); + assert_eq!(run(&mut world, r#"TargetUnit("NPC")"#), Some(VENDOR)); + reset(&mut world); + assert_eq!(run(&mut world, r#"AssistUnit("npc")"#), Some(GUARD)); + + // The NPC streamed out with its window still up: the object manager holds no unit. + world + .resource_mut::() + .0 + .remove(&VENDOR); + assert_eq!(run(&mut world, r#"TargetUnit("npc")"#), Some(GUARD)); + + // The window closed: nobody, and the selection stays. + world + .resource_mut::() + .0 + .insert(VENDOR, vendor); + world.insert_resource(InteractNpc::default()); + assert_eq!(run(&mut world, r#"TargetUnit("npc")"#), Some(GUARD)); + } + + /// The path a player takes: `/script TargetUnit("npc")` typed into the stock chat box runs + /// `SlashCmdList["SCRIPT"]` (`ChatFrame.lua:1074-1075`) and selects the vendor whose window is + /// open. + #[test] + fn typing_target_unit_npc_in_the_stock_chat_box_selects_the_vendor() { + benilla_formats::wow_data_or_skip!(); + use crate::net::{Guid, ObjectStore, SelfPlayer}; + use crate::ui_session::InteractNpc; + use benilla_ui::script::UiScript; + use bevy::ecs::system::RunSystemOnce; + + const ME: u64 = 1; + const VENDOR: u64 = 0xF130_0000_9800_0001; + let unit = || ObjectStore(benilla_protocol::ObjectFields::from_pairs(&[(2, 0x09)])); + + let mut world = script_world(); + world.insert_non_send_resource(crate::ui_script::chat_tests::chat_frame()); + world.spawn((SelfPlayer, Guid(ME), unit())); + let vendor = world.spawn((Guid(VENDOR), unit())).id(); + world + .resource_mut::() + .0 + .insert(VENDOR, vendor); + world.insert_resource(InteractNpc(Some(vendor), Some(VENDOR))); + + let mut script = world.non_send_resource_mut::(); + assert!(script.focus_editbox("ChatFrameEditBox")); + script.char_input(r#"/script TargetUnit("npc")"#); + assert!(script.key_input("ENTER"), "the box consumes ENTER"); + world + .run_system_once( + |mut script: NonSendMut, mut select: ScriptSelect| { + for request in script.take_selection_requests() { + select.select(request); + } + }, + ) + .expect("the applier runs as a one-shot system"); + assert_eq!( + world.resource::().guid, + Some(VENDOR) + ); + } } diff --git a/crates/benilla-app/src/ui_script/chat_tests.rs b/crates/benilla-app/src/ui_script/chat_tests.rs index 0d07bae2..7103db07 100644 --- a/crates/benilla-app/src/ui_script/chat_tests.rs +++ b/crates/benilla-app/src/ui_script/chat_tests.rs @@ -6,7 +6,7 @@ use benilla_ui::script::{ExtractedQuad, QuadContent, UiScript}; use super::test_ui::load_ui as load_xml; /// The chat stack, fonts first so `inherits="ChatFontNormal"` resolves. -fn chat_frame() -> UiScript { +pub(crate) fn chat_frame() -> UiScript { let mut s = UiScript::new().unwrap(); load_xml(&s, "Interface\\FrameXML\\GlobalStrings.lua"); load_xml(&s, "Interface\\FrameXML\\Fonts.xml"); diff --git a/crates/benilla-app/src/ui_script/mod.rs b/crates/benilla-app/src/ui_script/mod.rs index 79338c8d..a443ecf2 100644 --- a/crates/benilla-app/src/ui_script/mod.rs +++ b/crates/benilla-app/src/ui_script/mod.rs @@ -794,7 +794,7 @@ mod loot_tests; mod group_loot_tests; #[cfg(test)] -mod chat_tests; +pub(crate) mod chat_tests; /// The chat bubble's `UIMenu` kit driven as a menu: the rows' label and shortcut anchoring. #[cfg(test)] diff --git a/crates/benilla-app/src/ui_unit.rs b/crates/benilla-app/src/ui_unit.rs index df12b421..8102fe82 100644 --- a/crates/benilla-app/src/ui_unit.rs +++ b/crates/benilla-app/src/ui_unit.rs @@ -583,15 +583,15 @@ pub(crate) fn player_token_guid( /// The one unit-token resolver, the reference's `0x515970`: case-insensitive compares, then the /// object manager; a caller wanting a type tests the resolved unit. [`Selection`] is a parameter -/// because [`crate::target::SelectCommit`] holds it as `ResMut`. `npc` is recognised but -/// unresolved here, a quiet nil. +/// because [`crate::target::SelectCommit`] holds it as `ResMut`. #[derive(bevy::ecs::system::SystemParam)] pub(crate) struct UnitTokens<'w, 's> { - /// `Option`, like the two below: a UI-only harness lacks their plugins; a bare `Res` panics. + /// `Option`, like those below: a UI-only harness lacks their plugins; a bare `Res` panics. index: Option>, pet: Option>, hovered: Option>, hovered_go: Option>, + interact: Option>, group: Res<'w, crate::ui_party::GroupState>, pub(crate) stores: Query<'w, 's, &'static ObjectStore>, me: Query<'w, 's, (Entity, &'static Guid), With>, @@ -645,8 +645,8 @@ impl UnitTokens<'_, '_> { .and_then(|member| self.pet_of(member, true)), UnitBase::Raid(row) => self.raid_member(row).and_then(|guid| self.held(guid)), UnitBase::Party(row) => self.party_member(row).and_then(|guid| self.held(guid)), - // The interaction NPC is unresolved here, a quiet nil. - UnitBase::Npc => None, + // The interaction NPC's guid, `[0xb4e2d0]` (`0x515c00`). + UnitBase::Npc => self.held(self.interact.as_ref()?.1?), } } @@ -2204,7 +2204,6 @@ mod tests { "targetfoo", "mouseoverx", "raid3targettargetfoo", - "npc", "npctarget", "bogus", "",