From 9f4cbb771e093d3461e4f9afb9e8ad50f991034b Mon Sep 17 00:00:00 2001 From: cuinhellcat <217210902+cuinhellcat@users.noreply.github.com> Date: Wed, 19 Aug 2026 19:47:04 +0200 Subject: [PATCH 1/2] fix(engine): resolve a token image ref when every matching preset agrees (#7552) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A Role token created by a source card the catalog does not list resolved no `token_image_ref` — the preset scan's source gate turned "unknown source" into a silent `None`, and the client's name-search fallback then pulsed forever ("Wicked Role" is the engine name; every printing is titled by the bare face, CR 111.10 / `role_normalized_display_name`). The catalog-wide tail now shares one reduction with the related-ids path: `semantically_unique_ref` — when every remaining candidate is semantically identical, the pick is presentation-only and the first is taken deterministically. Applied twice: source-gated matches no longer die on the two-flip-sheet duplicate every Role has, and a source no preset lists falls back to a body-only match iff ALL body-matching presets agree. Bodies with semantically different presets still resolve nothing — the ambiguity protection is preserved (by shape; no real catalog body exercises it end to end, stated in the test doc). Red-first: the unlisted-source row fails on the old tail (`None`), passes now; counter-probe with the body-only fallback removed fails ONLY that row while the listed-source row stays green. Token suites (311 + 146 + 11) unchanged. Stated remainder (#7552): the second face of a Role flip sheet (Cursed on "Wicked // Cursed") resolves the shared image unrotated — face-aware rotation for flip-layout token printings is a separate display gap. Found by live playtest (the human half of this account), reproduced on phase-rs.dev. Co-Authored-By: Claude Fable 5 --- crates/engine/src/game/token_presets.rs | 70 +++++++++--- .../issue_7552_role_token_image_ref.rs | 104 ++++++++++++++++++ crates/engine/tests/integration/main.rs | 1 + 3 files changed, 158 insertions(+), 17 deletions(-) create mode 100644 crates/engine/tests/integration/issue_7552_role_token_image_ref.rs diff --git a/crates/engine/src/game/token_presets.rs b/crates/engine/src/game/token_presets.rs index d150021fc9..c76f8d0250 100644 --- a/crates/engine/src/game/token_presets.rs +++ b/crates/engine/src/game/token_presets.rs @@ -372,24 +372,60 @@ fn find_token_ref_with_mode( return first.token_image_ref.clone(); } - let mut matches = known_token_presets().iter().filter(|preset| { - if !token_body_matches(&preset.body, body) { - return false; - } - if let Some(oracle_id) = source_oracle { - return token_preset_has_source_ref(preset, oracle_id, source_face); - } - if let Some(name) = source_name { - return preset - .source_card_names - .iter() - .any(|candidate| candidate.eq_ignore_ascii_case(name)); - } - false - }); + let source_gated: Vec<&TokenPreset> = known_token_presets() + .iter() + .filter(|preset| { + if !token_body_matches(&preset.body, body) { + return false; + } + if let Some(oracle_id) = source_oracle { + return token_preset_has_source_ref(preset, oracle_id, source_face); + } + if let Some(name) = source_name { + return preset + .source_card_names + .iter() + .any(|candidate| candidate.eq_ignore_ascii_case(name)); + } + false + }) + .collect(); + if !source_gated.is_empty() { + // Same CR 111.10 dedup the related-ids path applies: multiple presets + // that are semantically identical differ only in printing — the pick + // is presentation-only, so take the first deterministically instead of + // silently resolving nothing (a Role exists on two flip sheets, so a + // source listed on both used to land here). + return semantically_unique_ref(&source_gated); + } - let first = matches.next()?; - if matches.next().is_some() { + // No preset lists this source. When EVERY body-matching preset is + // semantically identical, the token's identity is fully determined by its + // body alone (the live face: a Role token — CR 111.10 fixes all seven + // kinds by name, and `role_normalized_display_name` already reconciled the + // engine's " Role" naming). The source gate would add nothing but a + // silent `None`, which strands the display on a name search no printing + // can satisfy (#7552). Bodies with semantically DIFFERENT presets (art + // variants with different abilities) still resolve nothing here — the + // gate's ambiguity protection is preserved. + let body_only: Vec<&TokenPreset> = known_token_presets() + .iter() + .filter(|preset| token_body_matches(&preset.body, body)) + .collect(); + semantically_unique_ref(&body_only) +} + +/// The shared "all remaining candidates agree" reduction: `Some` ref iff every +/// preset in `matches` is semantically identical to the first — the choice is +/// then presentation-only and deterministic. Empty or disagreeing sets resolve +/// nothing. +fn semantically_unique_ref(matches: &[&TokenPreset]) -> Option { + let first = matches.first()?; + if !matches + .iter() + .skip(1) + .all(|preset| token_preset_semantics_match(first, preset)) + { return None; } first.token_image_ref.clone() diff --git a/crates/engine/tests/integration/issue_7552_role_token_image_ref.rs b/crates/engine/tests/integration/issue_7552_role_token_image_ref.rs new file mode 100644 index 0000000000..01c2d025f6 --- /dev/null +++ b/crates/engine/tests/integration/issue_7552_role_token_image_ref.rs @@ -0,0 +1,104 @@ +//! #7552: a Role token's catalog `token_image_ref` must survive an UNLISTED +//! source card. Every Role exists on two flip sheets, so the preset scan finds +//! two semantically identical candidates; the source gate used to turn that +//! into a silent `None`, stranding the display on a name search no printing +//! satisfies (the engine names Roles " Role", printings are titled by the +//! bare face — CR 111.10 / `role_normalized_display_name`). +//! +//! NOT proven here: the ambiguity protection for semantically DIFFERENT +//! body-matches (that `None` path is preserved by `semantically_unique_ref`'s +//! shape, and no real catalog body exercises it end to end). + +use engine::game::scenario::{GameScenario, P0}; +use engine::types::identifiers::ObjectId; +use engine::types::mana::{ManaType, ManaUnit}; +use engine::types::phase::Phase; + +fn pool(n: usize) -> Vec { + (0..n) + .map(|_| ManaUnit::new(ManaType::Colorless, ObjectId(0), false, vec![])) + .collect() +} + +#[test] +fn a_role_from_an_unlisted_source_still_carries_its_catalog_image_ref() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool(P0, pool(4)); + let host = scenario.add_creature(P0, "Chosen Host", 2, 2).id(); + let caster = scenario + .add_creature_to_hand_from_oracle( + P0, + "Wicked Bard", + 1, + 1, + "When this creature enters, create a Wicked Role token attached to target creature.", + ) + .id(); + let mut runner = scenario.build(); + runner.cast(caster).target_object(host).resolve(); + runner.advance_until_stack_empty(); + + let role = runner + .state() + .battlefield + .iter() + .find(|id| { + runner.state().objects[id] + .card_types + .subtypes + .iter() + .any(|sub| sub == "Role") + }) + .copied() + .expect("the Wicked Role token exists"); + let obj = &runner.state().objects[&role]; + assert!( + obj.token_image_ref.is_some(), + "the catalog carries a Wicked image ref; the created token must too \ + (name={:?}, colors={:?}, subtypes={:?})", + obj.name, + obj.color, + obj.card_types.subtypes + ); +} + +/// The positive gate: a source the catalog DOES list keeps resolving exactly as +/// before — this row is what keeps the fallback from being the whole mechanism. +#[test] +fn a_role_from_a_listed_source_resolves_its_image_ref() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool(P0, pool(4)); + let host = scenario.add_creature(P0, "Chosen Host", 2, 2).id(); + // "Monstrous Rage" is in the Monster preset's `source_card_names`. + let caster = scenario + .add_spell_to_hand_from_oracle( + P0, + "Monstrous Rage", + true, + "Target creature gets +2/+0 until end of turn. Create a Monster Role token attached to it.", + ) + .id(); + let mut runner = scenario.build(); + runner.cast(caster).target_object(host).resolve(); + runner.advance_until_stack_empty(); + + let role = runner + .state() + .battlefield + .iter() + .find(|id| { + runner.state().objects[id] + .card_types + .subtypes + .iter() + .any(|sub| sub == "Role") + }) + .copied() + .expect("the Monster Role token exists"); + assert!( + runner.state().objects[&role].token_image_ref.is_some(), + "the listed-source path must keep resolving" + ); +} diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 8044dc5871..937b3276b5 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -1217,6 +1217,7 @@ mod issue_4945_zada_hedron_grinder; mod issue_4948_samwise_gamgee_sacrifice_target_order; mod issue_5263_chaos_warp; mod issue_6367_thassas_oracle; +mod issue_7552_role_token_image_ref; mod kang_dynasty_until_next_turn_rider; mod karplusan_yeti_fight_back; mod kav_landseeker_delayed_sacrifice; From 23842751aa168dfdecf3b4515b3c766d90ad50bb Mon Sep 17 00:00:00 2001 From: cuinhellcat <217210902+cuinhellcat@users.noreply.github.com> Date: Wed, 19 Aug 2026 21:16:50 +0200 Subject: [PATCH 2/2] =?UTF-8?q?test(engine):=20prove=20the=20ambiguity=20g?= =?UTF-8?q?uard=20end=20to=20end=20=E2=80=94=20a=20real=20body=20with=20di?= =?UTF-8?q?sagreeing=20presets=20resolves=20None=20(#7552)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit stated the body-only fallback's ambiguity protection was preserved "by shape; no real catalog body exercises it end to end". That statement was wrong: the catalog DOES carry same-body twins with different semantics — a bare 1/1 red Goblin body matches the plain Goblin preset, Goblin Spymaster's "Creatures you control attack each combat if able." token AND Hold the Perimeter's "This token can't block." token (Treasure, Clue and Pest carry further pairs). Two rows close the gap the maintainer review named (a first-candidate implementation satisfied every existing test): - an unlisted source creating the bare Goblin body resolves NO image ref — the fallback refuses to pick between disagreeing candidates; - reach guard: the SAME body resolves TWO DISTINCT presets from their listed sources (Krenko's Command -> plain, Goblin Spymaster -> the must-attack variant, asserted by differing preset_ids), so the None above cannot mean "no body-matching candidate" — the body-only fallback saw both candidates and took the disagreement branch. Counter-probe: with semantically_unique_ref replaced by an unconditional first pick, ONLY the new unlisted-source row fails; all existing rows stay green — exactly the silent-first-pick regression the row now pins. Test doc header updated to retract the "not proven" paragraph. Co-Authored-By: Claude Fable 5 --- .../issue_7552_role_token_image_ref.rs | 100 +++++++++++++++++- 1 file changed, 97 insertions(+), 3 deletions(-) diff --git a/crates/engine/tests/integration/issue_7552_role_token_image_ref.rs b/crates/engine/tests/integration/issue_7552_role_token_image_ref.rs index 01c2d025f6..a9f6ae2f49 100644 --- a/crates/engine/tests/integration/issue_7552_role_token_image_ref.rs +++ b/crates/engine/tests/integration/issue_7552_role_token_image_ref.rs @@ -5,9 +5,12 @@ //! satisfies (the engine names Roles " Role", printings are titled by the //! bare face — CR 111.10 / `role_normalized_display_name`). //! -//! NOT proven here: the ambiguity protection for semantically DIFFERENT -//! body-matches (that `None` path is preserved by `semantically_unique_ref`'s -//! shape, and no real catalog body exercises it end to end). +//! Also proven here (#7555 review): the ambiguity protection for semantically +//! DIFFERENT body-matches. The catalog really carries such twins — a bare +//! 1/1 red Goblin body matches the plain Goblin preset, Goblin Spymaster's +//! "attack each combat if able" token AND Hold the Perimeter's "can't block" +//! token — so an unlisted source resolving that body must get `None`, never a +//! silent first pick. use engine::game::scenario::{GameScenario, P0}; use engine::types::identifiers::ObjectId; @@ -102,3 +105,94 @@ fn a_role_from_a_listed_source_resolves_its_image_ref() { "the listed-source path must keep resolving" ); } + +/// #7555 review: a body the catalog holds with SEMANTICALLY DIFFERENT presets +/// (same name/types/colors/P/T, different rules text) must resolve NO image +/// ref from an unlisted source — a deterministic first pick would show art +/// (and reminder text) of a token the game never made. +#[test] +fn an_ambiguous_body_from_an_unlisted_source_resolves_no_image_ref() { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool(P0, pool(4)); + let caster = scenario + .add_creature_to_hand_from_oracle( + P0, + "Backstreet Recruiter", + 1, + 1, + "When this creature enters, create a 1/1 red Goblin creature token.", + ) + .id(); + let mut runner = scenario.build(); + runner.cast(caster).resolve(); + runner.advance_until_stack_empty(); + + let goblin = runner + .state() + .battlefield + .iter() + .find(|id| runner.state().objects[id].name == "Goblin") + .copied() + .expect("the Goblin token exists — the ambiguous body WAS created"); + assert!( + runner.state().objects[&goblin].token_image_ref.is_none(), + "an ambiguous body (plain / Spymaster / Hold the Perimeter Goblins) \ + must resolve no ref from an unlisted source" + ); +} + +/// Reach guard for the row above (both-candidates proof): the SAME bare body +/// resolves TWO DISTINCT presets when the source is listed — Krenko's Command +/// is in the plain Goblin preset's `source_card_names`, Goblin Spymaster only +/// in the "Creatures you control attack each combat if able." variant's. Both +/// listed rows pass the resolver's `token_body_matches` gate, so at least two +/// semantically different presets match this body. The `None` above therefore +/// cannot mean "no body-matching candidate": the body-only fallback saw the +/// candidates and refused to pick between them. +/// +/// The Spymaster create clause is simplified to an ETB line: the subject here +/// is the resolver's name gate, not the card's opponent-end-step timing. The +/// token's quoted grant is omitted deliberately — preset BODIES exclude rules +/// text (that lives in `rules_text`, the semantic tie-breaker under test). +#[test] +fn the_same_body_resolves_two_distinct_presets_from_their_listed_sources() { + let resolve_for = |source_name: &str, oracle: &str| { + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::PreCombatMain); + scenario.with_mana_pool(P0, pool(4)); + let caster = scenario + .add_creature_to_hand_from_oracle(P0, source_name, 1, 1, oracle) + .id(); + let mut runner = scenario.build(); + runner.cast(caster).resolve(); + runner.advance_until_stack_empty(); + + let goblin = runner + .state() + .battlefield + .iter() + .find(|id| runner.state().objects[id].name == "Goblin") + .copied() + .expect("the Goblin token exists"); + runner.state().objects[&goblin] + .token_image_ref + .clone() + .expect("a listed source resolves this body") + }; + + let plain = resolve_for( + "Krenko's Command", + "When this creature enters, create a 1/1 red Goblin creature token.", + ); + let must_attack = resolve_for( + "Goblin Spymaster", + "When this creature enters, create a 1/1 red Goblin creature token.", + ); + + assert_ne!( + plain.preset_id, must_attack.preset_id, + "the two listed sources reach two DISTINCT body-matching presets — \ + the pair the unlisted row's ambiguity guard refuses to pick between" + ); +}