From b4a5d3b38f2449b0f91f331abbbc21cacad0c24e Mon Sep 17 00:00:00 2001 From: grom Date: Wed, 21 Oct 2026 22:17:15 -0600 Subject: [PATCH] ui: "do I know this spell?" asks the known set, not the spellbook MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every "Already known" check read `spellbook.slots`, and `SpellDisplay::in_spellbook` filters out `SPELL_ATTR_IS_TRADESKILL` — correctly, since recipes live in the trade-skill window, not the spellbook. So the question "do I know this spell?" answered **no for every crafting recipe in the game**: a recipe item never showed its `ITEM_SPELL_KNOWN` red line, and nothing stood between the player and consuming it a second time. grom learned Plans: Copper Chain Vest (item 3609) twice. `PlayerReqState` gains `known_spells`, filled from `PlayerActions::spells` — `SMSG_INITIAL_SPELLS` plus every learn, which is the set the reference's own check reads (`0xb700b0`). It belongs there: that struct is already "the player state the tooltip's red lines and the usable gate compare against", already change-gated on the action/spell inputs, and already pushed by the same feed. Both readers move over: the taught-spell line and `item_usable_by_id`'s spell requirement, which had the same hole. The existing `verified_families_signable_locked_resists_known` only passed because it made the recipe known by putting it in the spellbook, which no real session does — it now sets the known set instead, and that is the change's own regression story. The new test leaves the spellbook deliberately empty, which is the real shape and what fails if the check ever reads it again; it asserts the not-yet-learned case too, so it cannot pass by always reading red. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01SgstFWtn1RxEJ5kVrpgYwn --- crates/benilla-app/src/ui_items/feed.rs | 4 + crates/benilla-ui/src/script/item_stats.rs | 8 +- .../src/script/tests/tooltip_item/law.rs | 81 +++++++++++++++++-- .../src/script/tooltip_item/render.rs | 4 +- 4 files changed, 89 insertions(+), 8 deletions(-) diff --git a/crates/benilla-app/src/ui_items/feed.rs b/crates/benilla-app/src/ui_items/feed.rs index 6994a6e7..89bddc32 100644 --- a/crates/benilla-app/src/ui_items/feed.rs +++ b/crates/benilla-app/src/ui_items/feed.rs @@ -517,6 +517,10 @@ pub(super) fn feed_player_req( rep_ranks, can_dual_wield, honor_rank: store.0.player_honor_rank().unwrap_or(0), + // `SMSG_INITIAL_SPELLS` plus every learn, which is the set the reference's own + // "do I know this spell?" reads (`0xb700b0`) — not the spellbook, whose add gate drops + // trade-skill spells and so hides every crafting recipe from the question. + known_spells: actions.spells.iter().copied().collect(), }; if last.as_ref() != Some(&state) { gate.audit("feed_player_req", "the player-requirement state"); diff --git a/crates/benilla-ui/src/script/item_stats.rs b/crates/benilla-ui/src/script/item_stats.rs index 89f850fa..36c4894f 100644 --- a/crates/benilla-ui/src/script/item_stats.rs +++ b/crates/benilla-ui/src/script/item_stats.rs @@ -122,6 +122,11 @@ pub struct PlayerReqState { pub can_dual_wield: bool, /// The highest lifetime honor rank (`PLAYER_FIELD_BYTES` byte 3). pub honor_rank: u8, + /// **Every** spell the player knows, which is not the spellbook: `SpellBook::in_spellbook` + /// drops `SPELL_ATTR_IS_TRADESKILL`, so every crafting recipe in the game is absent from it. + /// Reading the spellbook for "do I know this?" therefore answered no for all of them — a + /// recipe item never showed its `ITEM_SPELL_KNOWN` red and could be consumed twice. + pub known_spells: std::collections::HashSet, } /// [`item_usable`] by item entry: entry 0 and a template not yet answered are usable, as the @@ -129,8 +134,9 @@ pub struct PlayerReqState { pub(in crate::script) fn item_usable_by_id(model: &super::Model, item_id: u32) -> bool { item_id == 0 || model.item_templates.get(&item_id).is_none_or(|v| { + // The known set, not the spellbook: see `PlayerReqState::known_spells`. item_usable(v, &model.player_req, |id| { - model.spellbook.slots.iter().any(|s| s.spell_id == id) + model.player_req.known_spells.contains(&id) }) }) } diff --git a/crates/benilla-ui/src/script/tests/tooltip_item/law.rs b/crates/benilla-ui/src/script/tests/tooltip_item/law.rs index 6146deff..b827c781 100644 --- a/crates/benilla-ui/src/script/tests/tooltip_item/law.rs +++ b/crates/benilla-ui/src/script/tests/tooltip_item/law.rs @@ -111,6 +111,74 @@ fn red_lines_track_player_state() { assert!(s.take_errors().is_empty()); } +/// A crafting recipe you already know must say so, and knowing it is a question about the known +/// spell set, never about the spellbook. +/// +/// grom learned Plans: Copper Chain Vest (item 3609) twice: no "Already known" line, and nothing +/// stopped the second use. The recipe's taught spell carries `SPELL_ATTR_IS_TRADESKILL`, which +/// `SpellDisplay::in_spellbook` filters out — correctly, since recipes live in the trade-skill +/// 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. +#[test] +fn a_known_recipe_reds_already_known_with_an_empty_spellbook() { + let mut s = script(); + s.set_screen_size(800.0, 600.0); + s.set_item_template( + 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())], + ..Default::default() + }, + ); + let show = |s: &mut crate::script::UiScript| { + s.run( + r#" + if not Slot3609 then + local a = CreateFrame("Button", "Slot3609"); a:SetPoint("CENTER", 0, 0) + a:SetWidth(10); a:SetHeight(10) + -- Named `TT` because `lines_of` reads that name's TextLeft children. + CreateFrame("GameTooltip", "TT") + end + TT:SetOwner(Slot3609, "ANCHOR_RIGHT") + TT:BenillaSetItemById(3609) + "#, + ) + .unwrap(); + }; + + // Not yet learned: no line. + show(&mut s); + assert!( + !lines_of(&mut s) + .iter() + .any(|(t, _)| t == "[ITEM_SPELL_KNOWN]"), + "a recipe you have not learned must not claim you know it" + ); + + // 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]), + ..Default::default() + }); + show(&mut s); + let lines = lines_of(&mut s); + let known = lines.iter().find(|(t, _)| t == "[ITEM_SPELL_KNOWN]"); + assert!( + known.is_some(), + "a learned recipe must show ITEM_SPELL_KNOWN; got {:?}", + lines.iter().map(|(t, _)| t.as_str()).collect::>() + ); + assert_eq!( + known.unwrap().1, + [1.0, 32.0 / 255.0, 32.0 / 255.0, 1.0], + "and in the unconditional red" + ); +} + /// The gated families (`0x52b650`): SIGNABLE green, UNIQUE before STARTS_QUEST, LOCKED red, six /// equal resistances as one ALL line (Holy never prints alone), a known taught spell's red /// "Already known", the gold description, and no openable line: the openable, readable and creator @@ -134,12 +202,13 @@ fn verified_families_signable_locked_resists_known() { ..Default::default() }, ); - s.set_spellbook(SpellBookState { - tabs: Vec::new(), - slots: vec![SpellSlotView { - spell_id: 2020, - ..Default::default() - }], + // Known the way the client knows a recipe: in the known-spell set, and **not** in the + // spellbook, whose add gate drops every `SPELL_ATTR_IS_TRADESKILL` spell. This test used to + // make it known by putting it in the spellbook, which no real session would do — and the + // tooltip read the spellbook, so every recipe in the game failed to show this line. + s.set_player_req_state(PlayerReqState { + known_spells: std::collections::HashSet::from([2020]), + ..Default::default() }); s.run( r#" diff --git a/crates/benilla-ui/src/script/tooltip_item/render.rs b/crates/benilla-ui/src/script/tooltip_item/render.rs index 7b8f9585..e5c6c644 100644 --- a/crates/benilla-ui/src/script/tooltip_item/render.rs +++ b/crates/benilla-ui/src/script/tooltip_item/render.rs @@ -104,7 +104,9 @@ pub(super) fn render_view( } let (req, known_spell, taught_known, set_view, equipped) = { let mut model = lua.app_data_mut::().expect("model app_data"); - let knows = |id: u32| model.spellbook.slots.iter().any(|s| s.spell_id == id); + // Every known spell, not the spellbook's displayable subset — a recipe's taught spell is + // `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