From c1ef619295fdf915abdb6a2a5525ff40214add20 Mon Sep 17 00:00:00 2001 From: grom Date: Fri, 16 Oct 2026 09:24:38 -0600 Subject: [PATCH] trainer: a rank-less service subtexts empty, not nil MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `GetTrainerServiceInfo`'s second return was nil whenever the spell had no rank. The client reads that field out of the spell record's string block, where an absent rank is the empty string at offset 0 rather than a null, so it is always a string — and the stock window depends on that: it formats the subtext unguarded at `Blizzard_TrainerUI.lua:275`, twenty lines before its own `not serviceSubText` fallback. So a nil raised inside `ClassTrainer_SelectFirstLearnableSkill`, which `ClassTrainerFrame_Show` calls on the line above `ClassTrainerFrame_Update` (l.48-49). The update never ran, so the list came up with no rows at all, and `SelectTrainerService` (l.277) never ran, so the Train button had nothing to act on. A trainer window that shows nothing and trains nothing, from one nil. Every rank-less service reaches that line: every profession recipe, and class abilities like Parry. The name beside it stays optional, because the stock window says so itself — it tests `serviceName` before using it as a header's (l.209) and falls back to `UNKNOWN` for a service's (l.287). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- crates/benilla-ui/src/script/trainer/mod.rs | 18 ++++++++++- crates/benilla-ui/src/script/trainer/tests.rs | 32 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/crates/benilla-ui/src/script/trainer/mod.rs b/crates/benilla-ui/src/script/trainer/mod.rs index f8117bf1..56a127e7 100644 --- a/crates/benilla-ui/src/script/trainer/mod.rs +++ b/crates/benilla-ui/src/script/trainer/mod.rs @@ -466,8 +466,24 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> { Row::Service(si) => { let s = &t.services[si]; Ok(MultiValue::from_vec(vec![ + // The name may be nil, and the stock window says so itself: it tests + // `serviceName` before using it as a header's (`Blizzard_TrainerUI.lua:209`) + // and falls back to `UNKNOWN` for a service's (l.287). opt_str(lua, s.name.as_ref())?, - opt_str(lua, s.subtext.as_ref())?, + // The subtext may not. It is empty for a spell with no rank, never nil: + // the client reads it from the spell record's string block, where an + // absent rank is the empty string at offset 0 rather than a null. + // + // It has to be, because the stock window formats it **unguarded** — + // `ClassTrainerSubSkillName:SetText(format(PARENS_TEMPLATE, serviceSubText))` + // (l.275) — twenty lines before its own `not serviceSubText` fallback. + // A nil raises there and takes `ClassTrainerFrame_Show` down with it, + // after `ClassTrainer_SelectFirstLearnableSkill` and before + // `ClassTrainerFrame_Update` (l.48-49), so the list never populates and + // `SelectTrainerService` (l.277) never runs: a trainer window that shows + // nothing and trains nothing. Every rank-less service reaches it — every + // profession recipe, and class abilities like Parry. + Value::String(lua.create_string(s.subtext.as_deref().unwrap_or_default())?), Value::String(lua.create_string(s.category.era_str())?), Value::Nil, ])) diff --git a/crates/benilla-ui/src/script/trainer/tests.rs b/crates/benilla-ui/src/script/trainer/tests.rs index def3be58..2c0f71e6 100644 --- a/crates/benilla-ui/src/script/trainer/tests.rs +++ b/crates/benilla-ui/src/script/trainer/tests.rs @@ -660,3 +660,35 @@ fn talent_order_sorts_on_state_within_a_group() { let got: Vec = visible(&mut s).into_iter().map(|(n, _)| n).collect(); assert_eq!(got, ["Riding", "Aye", "Bee", "Cee"]); } + +/// A rank-less service's subtext is the empty string, never nil. `Blizzard_TrainerUI.lua:275` +/// formats it unguarded, so a nil raises inside `ClassTrainer_SelectFirstLearnableSkill` and the +/// `ClassTrainerFrame_Update` on the line below it never runs: an empty window that trains +/// nothing. Every profession recipe and every rank-less ability (Parry) takes this path. +#[test] +fn a_rank_less_service_subtexts_empty_so_the_stock_window_can_format_it() { + let mut parry = svc(3127, "Parry", TrainerServiceCategory::Available, 26, "Arms"); + parry.subtext = None; + let mut s = UiScript::new().unwrap(); + s.set_trainer(Some(TrainerState { + greeting: "What would you like to learn?".into(), + trainer_type: 0, + groups: Vec::new(), + services: vec![parry], + })); + // Row 1 is the group header, row 2 the service. + assert_eq!( + s.eval::>("local _,sub = GetTrainerServiceInfo(2) return sub") + .unwrap() + .as_deref(), + Some(""), + "a service with no rank subtexts empty — nil is what broke the window" + ); + // `ClassTrainerSubSkillName:SetText(format(PARENS_TEMPLATE, serviceSubText))`, as written. + assert_eq!( + s.eval::("local _,sub = GetTrainerServiceInfo(2) return format('(%s)', sub)") + .unwrap(), + "()", + "the stock line must not raise" + ); +}