npc: the interaction token reaches the VM before the window's own event

HCQuest raised `bad argument #1 to 'strupper' (string expected, got nil)`
at `HCQuest.lua:16` once in the player's 2026-10-16 session, from
`questDetail` on `QUEST_DETAIL`. Its three arguments are `GetTitleText()`,
`QuestFrameNpcNameText:GetText()` and `GetObjectiveText()`, and the `and`
short-circuit means the giver is only read for a quest whose title matches
one of the addon's 49 — which is why it fired once and not on every quest.
So the nil is the name string, and `QuestFrame_SetPortrait` fills it from
`UnitName("npc")` (`QuestFrame.lua:63`).

`feed_interact_npc` seats `InteractNpc` before every feed, but the `"npc"`
token only reaches the VM in `feed_units`, and `feed_units` and `feed_quest`
are two systems in one `UiFeed` with no edge between them: the order was
declared by nothing, so `QUEST_DETAIL` could be signalled into a VM whose
`"npc"` was still absent. `SetText(nil)` then leaves the string empty for
the life of the panel, and the stock quest frame shows no giver name either.

Five window feeds were in that position; each now declares
`.after(crate::ui_unit::UnitFeed)`, as `feed_auction` and `feed_trainer`
already did, with the stock line that reads the token in the comment:
quest, gossip (where the chain's own `Turtle_ArenaUI.lua:285` and
`Turtle_GuildBankUI.lua:378` gate on `UnitName`/`UnitExists("npc")`, so a
late token means the window simply never opens), bank, taxi, tabard.

The reference needs no such edge: `CGGameUI::SetInteractNPC` writes
`[0xb4e2d0]` on the interact itself, so the pointer is live before any
window's `SignalEvent`. The test reads the declared graph and names the
FrameXML site per window, so a new window that reads the token and forgets
the edge fails with the line it reads it at.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
2026-10-18 12:05:47 -06:00
co-authored by Claude Opus 5
parent a2909fb0df
commit 780ca25833
7 changed files with 93 additions and 6 deletions
+1 -1
View File
@@ -546,7 +546,7 @@ pub(crate) mod schedule_tests {
/// Does this build carry type names? Bevy has them only under its `debug` feature, which
/// rides with `benilla-world`'s `dev`. Probed off the census, not a `cfg`, which belongs in
/// `run_mode` alone; name-dependent tests skip without them.
fn type_names_available(c: &Census) -> bool {
pub(crate) fn type_names_available(c: &Census) -> bool {
c.systems.values().any(|s| s.name.contains("benilla_app::"))
}
+4 -1
View File
@@ -72,7 +72,10 @@ impl Plugin for UiBankPlugin {
(
// Range-close first so the clear fires `BANKFRAME_CLOSED` the same frame.
close_npc_session_out_of_range::<BankOpen>.before(feed_bank),
feed_bank.in_set(UiFeed),
// After the unit feed: `BANKFRAME_OPENED` titles the frame from
// `UnitName("npc")` and bakes the portrait off the same token
// (`BankFrame.lua:127-128`).
feed_bank.in_set(UiFeed).after(crate::ui_unit::UnitFeed),
drain_bank.after(UiInput),
),
);
+6 -1
View File
@@ -142,7 +142,12 @@ impl Plugin for UiGossipPlugin {
(
// Range-close first so the clear fires `GOSSIP_CLOSED` the same frame.
close_npc_session_out_of_range::<GossipState>.before(feed_gossip),
feed_gossip.in_set(UiFeed),
// After the unit feed: `GossipFrame_OnEvent` reads `UnitName("npc")` and
// `UnitExists("npc")` for the title and portrait (`GossipFrame.lua:33-35`), and
// the chain's own windows gate on the same token — `Turtle_ArenaUI.lua:285`
// refuses to open unless `UnitName("npc")` answers, `Turtle_GuildBankUI.lua:378`
// on `UnitExists("npc")`.
feed_gossip.in_set(UiFeed).after(crate::ui_unit::UnitFeed),
drain_gossip.after(UiInput),
),
);
+7 -1
View File
@@ -224,7 +224,13 @@ impl Plugin for UiQuestPlugin {
// Range-close before the feed so the close fires the same frame; the feed before
// the input pass and the drain after it, so a click goes out the same frame.
close_npc_session_out_of_range::<QuestGiver>.before(feed_quest),
feed_quest.in_set(UiFeed),
// After the unit feed: `QUEST_DETAIL` reaches `QuestFrame_OnEvent`, whose
// `QuestFrame_SetPortrait` reads `UnitName("npc")` and `UnitExists("npc")`
// (`QuestFrame.lua:62-66`), and the `"npc"` token is pushed in `feed_units`. The
// reference has the pointer before the signal by construction:
// `CGGameUI::SetInteractNPC` writes `[0xb4e2d0]` on the interact, the window's
// event is signalled after.
feed_quest.in_set(UiFeed).after(crate::ui_unit::UnitFeed),
drain_quest.after(UiInput),
),
);
+69
View File
@@ -456,4 +456,73 @@ mod tests {
"drive_display_facing must run after feed_interact_npc"
);
}
/// Every window whose open event's stock handler reads the `"npc"` token runs **after** the
/// unit feed, which is what pushes that token ([`crate::ui_unit::feed_units`]'s `"npc"` block).
///
/// [`feed_interact_npc`] above seats the resource before both, but the token only reaches the
/// VM in the unit feed, and two systems in one set with no edge between them are ordered by
/// nothing: the event can be signalled into a VM whose `"npc"` is still last window's, or
/// nil. The symptom is not a missing name alone — `UnitName("npc")` nil makes
/// `QuestFrameNpcNameText:SetText(nil)` leave the string empty, and HCQuest, which reads that
/// string back on `QUEST_DETAIL`, raises `bad argument #1 to 'strupper'` and loses its PvP
/// warning for the quest (`HCQuest.lua:16`, from the player's 2026-10-16 session).
///
/// The reference needs no such edge: `CGGameUI::SetInteractNPC` writes `[0xb4e2d0]` on the
/// interact itself, so the pointer is live before any window's `SignalEvent`.
#[test]
fn every_window_that_reads_the_npc_token_is_fed_after_the_unit_feed() {
use crate::game_plugins::schedule_tests::{census, SyncPoints};
/// Each window's feed and the stock line that reads the token.
const READERS: &[(&str, &str)] = &[
("ui_quest::feed_quest", "QuestFrame.lua:63"),
("ui_gossip::feed_gossip", "GossipFrame.lua:33"),
("ui_bank::feed_bank", "BankFrame.lua:127"),
("ui_taxi::feed_taxi", "TaxiFrame.lua:27"),
("ui_tabard::feed_tabard", "TabardFrame.lua:28"),
("ui_auction::feed_auction", "AuctionFrame.lua:29"),
("ui_trainer::feed_trainer", "ClassTrainerFrame.lua:59"),
];
const PUSHER: &str = "ui_unit::feed_units";
let mut app = crate::game_plugins::schedule_tests::headless_client();
let c = census(&mut app, bevy::app::Update, SyncPoints::Declared);
if !crate::game_plugins::schedule_tests::type_names_available(&c) {
eprintln!("skipped: this build carries no type names");
return;
}
let one = |suffix: &str| {
let mut hits = c.systems.iter().filter(|(_, s)| s.name.ends_with(suffix));
match (hits.next(), hits.next()) {
(Some((k, _)), None) => *k,
_ => panic!("exactly one system named `…{suffix}` in Update"),
}
};
// Reachability over declared edges: a path means somebody declared the order.
let mut succ: std::collections::HashMap<_, Vec<_>> = std::collections::HashMap::new();
for (a, b) in &c.dependencies {
succ.entry(*a).or_default().push(*b);
}
let pusher = one(PUSHER);
let mut seen = std::collections::HashSet::new();
let mut work = vec![pusher];
while let Some(n) = work.pop() {
if !seen.insert(n) {
continue;
}
work.extend(succ.get(&n).into_iter().flatten().copied());
}
let missing: Vec<String> = READERS
.iter()
.filter(|(feed, _)| !seen.contains(&one(feed)))
.map(|(feed, site)| format!("{feed} (reads it at {site})"))
.collect();
assert!(
missing.is_empty(),
"these window feeds fire an open event whose stock handler reads the `\"npc\"` token, \
but nothing orders them after `{PUSHER}`, which pushes it — \
`.after(crate::ui_unit::UnitFeed)` at the feed's own `add_systems`: {missing:?}"
);
}
}
+3 -1
View File
@@ -306,7 +306,9 @@ impl Plugin for TabardUiPlugin {
(
reset_on_world_enter.before(feed_tabard),
close_npc_session_out_of_range::<TabardOpen>.before(feed_tabard),
feed_tabard.in_set(UiFeed),
// After the unit feed: `TABARDFRAME_OPENED` names the vendor from
// `UnitName("npc")` and bakes its portrait (`TabardFrame.lua:27-28`).
feed_tabard.in_set(UiFeed).after(crate::ui_unit::UnitFeed),
drain_tabard.after(UiInput),
),
);
+3 -1
View File
@@ -298,7 +298,9 @@ impl Plugin for UiTaxiPlugin {
load_taxi_catalogs,
// Range-close first so the clear fires `TAXIMAP_CLOSED` the same frame.
close_npc_session_out_of_range::<TaxiState>.before(feed_taxi),
feed_taxi.in_set(UiFeed),
// After the unit feed: `TAXIMAP_OPENED` names the flight master from
// `UnitName("npc")` and bakes its portrait (`TaxiFrame.lua:27-28`).
feed_taxi.in_set(UiFeed).after(crate::ui_unit::UnitFeed),
feed_on_taxi.in_set(UiFeed),
drain_taxi.after(UiInput),
),