trainer: a service's skill line is the profession, not its group header

`GetTrainerServiceSkillLine` answered the group header text. At a class trainer the two are one
string, so nothing showed; at a tradeskill trainer they are not. Every group there is labelled from
`TRADESKILL_SERVICE_STEP`/`_LEARN` (`0x4d7786`), and `TRADESKILL_SERVICE_STEP` is the string
"Development Skills" — so standing at the mining trainer, the stock confirmation dialog, which
formats this return straight into `PROFESSION_CONFIRMATION2` (`Blizzard_TrainerUI.lua:28`), asked
whether to learn **Development Skills** as a second profession.

`TrainerService` now carries the taught spell's own line beside the group label, resolved through
the same `SkillLineAbility` lookup the group key uses, and the getter prefers it. The header row is
left alone: "Development Skills" there is the client's own label for a skill-stepping tradeskill
group, not a lookup that went wrong.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn

crates/benilla-app/src/ui_script/trainer_tests.rs

@@ -142,6 +142,8 @@ fn service(
         is_trade_skill: false,
         group_key: skill_line,
         group_name: line_name.into(),
+        // At a class trainer the group is the skill line, so the two agree.
+        skill_line_name: Some(line_name.into()),
     }
 }
 

crates/benilla-app/src/ui_trainer/mod.rs

@@ -232,6 +232,11 @@ fn resolve_service(
         is_trade_skill: trainer_type == TRAINER_TYPE_TRADESKILL,
         group_key,
         group_name,
+        // The taught spell's line, resolved whatever the group turned out to be: at a tradeskill
+        // trainer the group is a fixed label and only this says "Mining".
+        skill_line_name: skill_lines
+            .and_then(|c| c.spell_to_line(taught).and_then(|l| c.line(l)))
+            .map(|l| l.name.clone()),
         tooltip: service_tooltip(wire.spell, spells),
     }
 }

crates/benilla-ui/src/script/trainer/mod.rs

@@ -129,6 +129,11 @@ pub struct TrainerService {
     /// The header text: the skill line's name, `KNOWN_TALENTS_HEADER` for the known group, or at
     /// type 2 `TRADESKILL_SERVICE_STEP`/`_LEARN`.
     pub group_name: String,
+    /// The taught spell's own skill line name, which is **not** the header at a tradeskill
+    /// trainer: there the group is labelled `TRADESKILL_SERVICE_STEP` ("Development Skills") for
+    /// every service, while this stays "Mining". `None` when no line resolves, where
+    /// [`Self::group_name`] serves.
+    pub skill_line_name: Option<String>,
     pub tooltip: TrainerTooltip,
 }
 
@@ -492,8 +497,13 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
         })?,
     )?;
 
-    // GetTrainerServiceSkillLine(index): the group name, nil for a header. The reference's return
-    // (`0x4d9160`) is untraced; the stock `CONFIRM_PROFESSION` dialog shows it as the profession.
+    // GetTrainerServiceSkillLine(index): the service's **skill line**, nil for a header. Not the
+    // group header text, which this used to answer: at a tradeskill trainer every group is
+    // labelled from `TRADESKILL_SERVICE_STEP`/`_LEARN` (`0x4d7786`), so a mining trainer's header
+    // reads "Development Skills" — and the stock `PROFESSION_CONFIRMATION2` dialog
+    // (`Blizzard_TrainerUI.lua:28`) formats this very return into "Would you like to learn %s as
+    // your second one?", which then asked whether to learn Development Skills. The reference's
+    // return (`0x4d9160`) is untraced, but the one call site settles what it has to be.
     g.set(
         "GetTrainerServiceSkillLine",
         lua.create_function(|lua, index: usize| {
@@ -508,9 +518,11 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
                 .trainer
                 .as_ref()
                 .expect("a resolved row ⇒ an open trainer");
-            Ok(Value::String(
-                lua.create_string(&t.services[si].group_name)?,
-            ))
+            let svc = &t.services[si];
+            // The group name is the fallback, not the answer: at a class trainer the two are the
+            // same string (the group *is* the skill line), so this changes nothing there.
+            let line = svc.skill_line_name.as_deref().unwrap_or(&svc.group_name);
+            Ok(Value::String(lua.create_string(line)?))
         })?,
     )?;
 

crates/benilla-ui/src/script/trainer/tests.rs

@@ -29,6 +29,7 @@ fn svc(
         is_trade_skill: false,
         group_key,
         group_name: group_name.into(),
+        skill_line_name: Some(group_name.into()),
     }
 }
 
@@ -692,3 +693,48 @@ fn a_rank_less_service_subtexts_empty_so_the_stock_window_can_format_it() {
         "the stock line must not raise"
     );
 }
+
+/// `GetTrainerServiceSkillLine` answers the service's **skill line**, not its group header. The
+/// two differ at a tradeskill trainer: every group there is labelled `TRADESKILL_SERVICE_STEP`
+/// ("Development Skills") or `_LEARN` (`0x4d7786`), while the service's line stays "Mining". The
+/// stock window formats this return straight into `PROFESSION_CONFIRMATION2`
+/// (`Blizzard_TrainerUI.lua:28`), so answering the header asked the player whether they would
+/// like to learn Development Skills as their second profession.
+#[test]
+fn a_tradeskill_services_skill_line_is_the_profession_not_the_group_header() {
+    let mut apprentice = svc(
+        2581,
+        "Apprentice Miner",
+        TrainerServiceCategory::Available,
+        1,
+        "Development Skills",
+    );
+    apprentice.skill_line_name = Some("Mining".into());
+    apprentice.is_trade_skill = true;
+    let mut s = UiScript::new().unwrap();
+    s.set_trainer(Some(TrainerState {
+        greeting: "You have not lived until you have dug deep into the earth.".into(),
+        trainer_type: 2,
+        groups: Vec::new(),
+        services: vec![apprentice],
+    }));
+    // Row 1 is the group header, row 2 the service.
+    assert_eq!(
+        s.eval::<Option<String>>("return GetTrainerServiceSkillLine(1)")
+            .unwrap(),
+        None,
+        "a header has no skill line"
+    );
+    assert_eq!(
+        s.eval::<String>("return GetTrainerServiceSkillLine(2)")
+            .unwrap(),
+        "Mining",
+        "the profession, which is what the confirmation dialog prints"
+    );
+    // The header row still carries the client's own label, which is a different question.
+    assert_eq!(
+        s.eval::<String>("local n = GetTrainerServiceInfo(1) return n")
+            .unwrap(),
+        "Development Skills"
+    );
+}