A recipe never names the spell it teaches, so "Already known" never fired
He is still told he can learn recipes he already knows. The check was there; it was written against a shape no recipe in the game has. Read off his own item cache, the realm's Plans: Copper Chain Vest (3609) carries exactly one spell: 3349, at trigger 0. The rule looked for trigger 6 carrying the taught spell. Neither half matches — and 3349 is not the craft spell either, it is a learn *wrapper* whose single SPELL_EFFECT_LEARN_SPELL teaches 3321. Nothing in the item record names 3321 at all, so no check that reads only the item can ever answer this question. The app now makes the hop while it has the catalog in hand, through SpellCatalog::learned_spell, and hands the result over as ItemTemplateView::teaches. The tooltip reds ITEM_SPELL_KNOWN when any of those is in the known-spell set. The trigger-6 arm stays for an item that really does carry its taught spell. The old test passed throughout, which is the part worth keeping in mind: its fixture was invented — trigger 6, spell 3330 — and 3330 is Silvered Bronze Shoulders, nothing to do with this item. It now uses the realm's own record, and a second test pins the 3349 -> 3321 hop against the shipped Spell.dbc so the fixture can never drift from the data again. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn
This commit is contained in:
@@ -213,6 +213,14 @@ fn template_view(
|
||||
.filter(|s| s.spell_id != 0)
|
||||
.filter_map(|s| spell_text(s.spell_id).map(|n| (s.trigger, s.spell_id, n)))
|
||||
.collect(),
|
||||
// Each of the item's spells hopped through `SPELL_EFFECT_LEARN_SPELL`, which is the only
|
||||
// place the taught spell is written down — see `ItemTemplateView::teaches`.
|
||||
teaches: t
|
||||
.spells
|
||||
.iter()
|
||||
.filter(|s| s.spell_id != 0)
|
||||
.filter_map(|s| spells.and_then(|c| c.catalog.learned_spell(s.spell_id)))
|
||||
.collect(),
|
||||
charges: charges_count(&t.spells),
|
||||
description: t.description.clone(),
|
||||
page_text: t.page_text,
|
||||
@@ -1876,6 +1884,33 @@ mod tests {
|
||||
);
|
||||
}
|
||||
|
||||
/// A recipe item's taught spell, resolved off the **real** `Spell.dbc`.
|
||||
///
|
||||
/// The realm's Plans: Copper Chain Vest (3609) carries one spell, 3349, at trigger 0 — read
|
||||
/// out of grom's own item cache, not invented. 3349 is a learn wrapper: its single
|
||||
/// `SPELL_EFFECT_LEARN_SPELL` teaches 3321, the craft spell. Nothing in the item record names
|
||||
/// 3321, so a known-recipe check written against the item's own spell id can never fire, and
|
||||
/// that is why he kept being told he could learn recipes he already knew.
|
||||
///
|
||||
/// Pinned here on the shipped DBC rather than a fixture, because the whole defect was a
|
||||
/// fixture that asserted a shape the data does not have.
|
||||
#[test]
|
||||
fn a_recipe_items_taught_spell_is_the_learn_wrappers_on_real_data() {
|
||||
let data = benilla_formats::wow_data_or_skip!();
|
||||
let mut chain = benilla_formats::open_chain(&data).expect("open chain");
|
||||
let catalog = benilla_formats::load_spell_catalog(&mut chain).expect("Spell.dbc");
|
||||
assert_eq!(
|
||||
catalog.learned_spell(3349),
|
||||
Some(3321),
|
||||
"spell 3349, the item's only spell, is the wrapper that teaches the craft spell"
|
||||
);
|
||||
assert_eq!(
|
||||
catalog.learned_spell(3321),
|
||||
None,
|
||||
"and the craft spell teaches nothing — the hop is one step, not a chain"
|
||||
);
|
||||
}
|
||||
|
||||
/// On the real Spell.dbc, an undescribed spell such as a key's `Opening` builds no trigger
|
||||
/// line (`0x52da29`-`0x52da31`), and a described one does.
|
||||
#[test]
|
||||
|
||||
@@ -85,6 +85,17 @@ pub struct ItemTemplateView {
|
||||
/// Green trigger-spell lines `(trigger, spell id, text)` in wire order: 0 and 5 "Use:",
|
||||
/// 1 "Equip:", 2 "Chance on hit:", 6 a taught spell (no line, only the "Already known" red).
|
||||
pub spell_triggers: Vec<(u32, u32, String)>,
|
||||
/// The spells this item **teaches**, resolved app-side through `Spell.dbc`'s
|
||||
/// `SPELL_EFFECT_LEARN_SPELL` (`SpellCatalog::learned_spell`). Knowing any of them is what
|
||||
/// puts the red `ITEM_SPELL_KNOWN` on the plate.
|
||||
///
|
||||
/// It has to be resolved rather than read off the trigger, because a recipe item does not
|
||||
/// carry the craft spell at all: the realm's Plans: Copper Chain Vest (3609) carries exactly
|
||||
/// one spell, **3349 at trigger 0**, and 3349 is a learn *wrapper* whose one effect teaches
|
||||
/// 3321. A rule written against trigger 6 and the item's own spell id — which is what this
|
||||
/// was — matches no recipe in the game, and grom kept being told he could learn recipes he
|
||||
/// already knew.
|
||||
pub teaches: Vec<u32>,
|
||||
pub lock_id: u32,
|
||||
/// "N Charge(s)" for the first spell slot past the builder's gate (`0x52db51`: 0 and -1
|
||||
/// print nothing, else the absolute value); 0 prints no line.
|
||||
|
||||
@@ -120,6 +120,13 @@ fn red_lines_track_player_state() {
|
||||
/// window and not the spellbook — so a check that read `spellbook.slots` answered "unknown" for
|
||||
/// every recipe in the game. The spellbook is left deliberately **empty** here: that is the real
|
||||
/// shape, and it is what makes this test fail if the check goes back to reading it.
|
||||
///
|
||||
/// **The fixture is the realm's own record, and that matters.** This test first shipped with a
|
||||
/// made-up one — trigger 6 carrying the craft spell — and passed while he went on being told he
|
||||
/// could learn recipes he already knew, because no recipe in the game has that shape. Item 3609
|
||||
/// as the realm sends it carries exactly one spell: **3349 at trigger 0**, and 3349 is a learn
|
||||
/// *wrapper* whose single `SPELL_EFFECT_LEARN_SPELL` teaches 3321 (both read out of the shipped
|
||||
/// `Spell.dbc`). The app resolves that hop into `ItemTemplateView::teaches`.
|
||||
#[test]
|
||||
fn a_known_recipe_reds_already_known_with_an_empty_spellbook() {
|
||||
let mut s = script();
|
||||
@@ -128,9 +135,11 @@ fn a_known_recipe_reds_already_known_with_an_empty_spellbook() {
|
||||
3609,
|
||||
ItemTemplateView {
|
||||
name: "Plans: Copper Chain Vest".into(),
|
||||
quality: 1,
|
||||
// Trigger 6 is "use: teaches you"; 3330 is the recipe's trade-skill spell.
|
||||
spell_triggers: vec![(6, 3330, "Copper Chain Vest".into())],
|
||||
quality: 2,
|
||||
// The realm's record: one spell, trigger 0 ("Use:"), the learn wrapper 3349 …
|
||||
spell_triggers: vec![(0, 3349, "Teaches you Copper Chain Vest.".into())],
|
||||
// … which Spell.dbc says teaches 3321, the craft spell he actually knows.
|
||||
teaches: vec![3321],
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
@@ -161,7 +170,7 @@ fn a_known_recipe_reds_already_known_with_an_empty_spellbook() {
|
||||
|
||||
// Learned. The spellbook stays empty, as it does in a real session.
|
||||
s.set_player_req_state(PlayerReqState {
|
||||
known_spells: std::collections::HashSet::from([3330]),
|
||||
known_spells: std::collections::HashSet::from([3321]),
|
||||
..Default::default()
|
||||
});
|
||||
show(&mut s);
|
||||
|
||||
@@ -108,11 +108,13 @@ pub(super) fn render_view(
|
||||
// `SPELL_ATTR_IS_TRADESKILL` and so is never in the book (`PlayerReqState::known_spells`).
|
||||
let knows = |id: u32| model.player_req.known_spells.contains(&id);
|
||||
let known = v.required_spell != 0 && knows(v.required_spell);
|
||||
// A taught spell (trigger 6, learn) the player knows: the always-red ITEM_SPELL_KNOWN.
|
||||
let taught = v
|
||||
.spell_triggers
|
||||
.iter()
|
||||
.any(|&(t, id, _)| t == 6 && id != 0 && knows(id));
|
||||
// A taught spell the player knows: the always-red ITEM_SPELL_KNOWN. Two sources, because
|
||||
// the trigger alone does not name it — see `ItemTemplateView::teaches`. The trigger-6 arm
|
||||
// stays for an item that really does carry the taught spell directly.
|
||||
let taught = v.teaches.iter().any(|&id| id != 0 && knows(id))
|
||||
|| v.spell_triggers
|
||||
.iter()
|
||||
.any(|&(t, id, _)| t == 6 && id != 0 && knows(id));
|
||||
// The set view (a miss records the ask) and the equipped ids the set block counts.
|
||||
let set_view = (v.item_set != 0)
|
||||
.then(|| {
|
||||
|
||||
Reference in New Issue
Block a user