From a01fd281f7dadb95a3dd1eae9f2f2c25104ffcb2 Mon Sep 17 00:00:00 2001 From: Jonathanchang31 <55106972+jonathanchang31@users.noreply.github.com> Date: Thu, 18 Jun 2026 15:58:47 +0200 Subject: [PATCH 1/3] fix: Ring-bearer badge hidden behind grouped Army tokens (#3721) Co-authored-by: Matt Evans <1388610+matthewevans@users.noreply.github.com> --- .../__tests__/battlefieldGrouping.test.ts | 17 +++ client/src/viewmodel/battlefieldProps.ts | 18 ++- client/src/viewmodel/gameStateView.ts | 23 ++- crates/engine/tests/integration/main.rs | 1 + .../tests/integration/ninjutsu_cluster.rs | 141 ++++++++++++++++++ 5 files changed, 189 insertions(+), 11 deletions(-) create mode 100644 crates/engine/tests/integration/ninjutsu_cluster.rs diff --git a/client/src/viewmodel/__tests__/battlefieldGrouping.test.ts b/client/src/viewmodel/__tests__/battlefieldGrouping.test.ts index 78ff0a7992..b0e3e09808 100644 --- a/client/src/viewmodel/__tests__/battlefieldGrouping.test.ts +++ b/client/src/viewmodel/__tests__/battlefieldGrouping.test.ts @@ -260,6 +260,23 @@ describe("groupByName", () => { expect(groups.find((g) => g.count === 1)?.ids).toEqual([3]); }); + it("renders the ring-bearer solo even among identical same-named copies (issue #721)", () => { + const objects = [ + makeGameObject({ id: 1, name: "Orc Army" }), + makeGameObject({ id: 2, name: "Orc Army" }), + makeGameObject({ id: 3, name: "Orc Army" }), + ]; + + const groups = groupByName(objects, new Set([2])); + + // The ring-bearer (id 2) never gets hidden behind a non-bearer + // representative in a collapsed group — it always has its own entry so + // PermanentCard's ring-bearer badge is reachable. + expect(groups).toHaveLength(2); + expect(groups.find((g) => g.count === 2)?.ids).toEqual([1, 3]); + expect(groups.find((g) => g.count === 1)?.ids).toEqual([2]); + }); + it("separates copies with different counter amounts", () => { const objects = [ makeGameObject({ id: 1, name: "Grizzly Bears", counters: { Plus1Plus1: 1 } }), diff --git a/client/src/viewmodel/battlefieldProps.ts b/client/src/viewmodel/battlefieldProps.ts index 5cf680cad5..427cd6ade8 100644 --- a/client/src/viewmodel/battlefieldProps.ts +++ b/client/src/viewmodel/battlefieldProps.ts @@ -2,8 +2,11 @@ import type { AttackerInfo, CombatState, GameObject, ObjectId, PlayerId } from " import { publicName, toCardProps } from "./cardProps"; import type { CardViewProps } from "./cardProps"; -function canGroup(obj: GameObject): boolean { - return obj.attachments.length === 0; +function canGroup(obj: GameObject, ringBearerIds: ReadonlySet): boolean { + // Ring-bearers (CR 701.54) must never be hidden behind a same-named + // non-bearer representative in a collapsed/stacked group display — render + // them solo so the ring-bearer badge in PermanentCard is always visible. + return obj.attachments.length === 0 && !ringBearerIds.has(obj.id); } function groupKey(obj: GameObject): string { @@ -82,12 +85,17 @@ export function partitionByType(objects: GameObject[]): BattlefieldPartition { return { creatures, lands, support, planeswalkers, other }; } -export function groupByName(objects: GameObject[]): GroupedPermanent[] { +const NO_RING_BEARERS: ReadonlySet = new Set(); + +export function groupByName( + objects: GameObject[], + ringBearerIds: ReadonlySet = NO_RING_BEARERS, +): GroupedPermanent[] { const groups = new Map(); for (const obj of objects) { - if (!canGroup(obj)) { - // Ungroupable objects (attachments, counters) get their own entry + if (!canGroup(obj, ringBearerIds)) { + // Ungroupable objects (attachments, ring-bearers) get their own entry groups.set(`__solo_${obj.id}`, [obj]); continue; } diff --git a/client/src/viewmodel/gameStateView.ts b/client/src/viewmodel/gameStateView.ts index 217e01b3d8..62aa94f658 100644 --- a/client/src/viewmodel/gameStateView.ts +++ b/client/src/viewmodel/gameStateView.ts @@ -228,11 +228,22 @@ export function buildPlayerBattlefieldView( const playerObjects = battlefieldObjects.filter( (object) => object.controller === playerId, ); - return buildPlayerBattlefieldViewFromObjects(playerObjects); + // CR 701.54: the Ring-bearer must render as its own card even when a + // same-named, identically-statted permanent (e.g. another Army token) + // would otherwise collapse it into a shared group — otherwise the + // ring-bearer badge can land on the wrong representative or disappear + // entirely behind a stack badge. + const ringBearerIds = new Set( + Object.values(gameState.ring_bearer ?? {}).filter( + (id): id is ObjectId => id != null, + ), + ); + return buildPlayerBattlefieldViewFromObjects(playerObjects, ringBearerIds); } export function buildPlayerBattlefieldViewFromObjects( playerObjects: GameObject[], + ringBearerIds: ReadonlySet = new Set(), ): PlayerBattlefieldView { const partition = partitionByType(playerObjects); const objectMap = new Map(playerObjects.map((object) => [object.id, object])); @@ -242,11 +253,11 @@ export function buildPlayerBattlefieldViewFromObjects( .filter(Boolean) as GameObject[]; return { - creatures: groupByName(resolveObjects(partition.creatures)), - lands: groupByName(resolveObjects(partition.lands)), - support: groupByName(resolveObjects(partition.support)), - planeswalkers: groupByName(resolveObjects(partition.planeswalkers)), - other: groupByName(resolveObjects(partition.other)), + creatures: groupByName(resolveObjects(partition.creatures), ringBearerIds), + lands: groupByName(resolveObjects(partition.lands), ringBearerIds), + support: groupByName(resolveObjects(partition.support), ringBearerIds), + planeswalkers: groupByName(resolveObjects(partition.planeswalkers), ringBearerIds), + other: groupByName(resolveObjects(partition.other), ringBearerIds), }; } diff --git a/crates/engine/tests/integration/main.rs b/crates/engine/tests/integration/main.rs index 640ab73c49..de6f2295ce 100644 --- a/crates/engine/tests/integration/main.rs +++ b/crates/engine/tests/integration/main.rs @@ -330,6 +330,7 @@ mod multi_upkeep_triggers_suspend; mod mycoloth_upkeep_trigger; mod mystic_forge_regression; mod narset_jeskai_waymaster_draw_spells_cast; +mod ninjutsu_cluster; mod nix_counter_no_mana_spent; mod ob_nixilis_captive_kingpin_life_loss; mod obeka_splitter_additional_phases; diff --git a/crates/engine/tests/integration/ninjutsu_cluster.rs b/crates/engine/tests/integration/ninjutsu_cluster.rs new file mode 100644 index 0000000000..096b5a7613 --- /dev/null +++ b/crates/engine/tests/integration/ninjutsu_cluster.rs @@ -0,0 +1,141 @@ +//! Integration tests for Ninjutsu cluster issues #3648, #3662, #3661 +//! +//! - #3648: Ninjutsu not being offered from the command zone +//! - #3662: Sakashima's Student paying/returning a creature but not entering +//! - #3661: Turtle Lair mana not making a Ninja spell castable (NOT A BUG - see below) + +use engine::game::combat::{AttackerInfo, CombatState}; +use engine::game::scenario::{GameScenario, P0, P1}; +use engine::types::actions::GameAction; +use engine::types::game_state::WaitingFor; +use engine::types::identifiers::ObjectId; +use engine::types::keywords::Keyword; +use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; +use engine::types::phase::Phase; + +#[test] +fn test_3648_commander_ninjutsu_offered_from_command_zone() { + // CR 702.49d: Commander ninjutsu functions from the command zone + let mut scenario = GameScenario::new(); + scenario.at_phase(Phase::DeclareBlockers); + + // Set up an unblocked attacker + let attacker = scenario.add_creature(P0, "Attacker", 2, 2).id(); + + // Add a regular Ninjutsu card to hand + let hand_ninja = scenario.add_creature_to_hand(P0, "Hand Ninja", 2, 2).id(); + + let mut runner = scenario.build(); + runner.state_mut().debug_mode = true; + + // Configure the hand Ninja with regular Ninjutsu + { + let obj = runner.state_mut().objects.get_mut(&hand_ninja).unwrap(); + let cost = ManaCost::Cost { + shards: vec![ManaCostShard::Blue], + generic: 1, + }; + obj.keywords.push(Keyword::Ninjutsu(cost.clone())); + obj.base_keywords.push(Keyword::Ninjutsu(cost)); + } + + // Add a Commander Ninjutsu card to command zone + let commander_ninja_id = ObjectId(runner.state().next_object_id); + { + use engine::game::game_object::GameObject; + use engine::im::Vector; + use engine::types::card_type::CoreType; + use engine::types::identifiers::CardId; + use engine::types::zones::Zone; + + let state = runner.state_mut(); + + // Create the commander ninja object + let mut commander_obj = GameObject::new( + commander_ninja_id, + CardId(999), // Dummy card ID + P0, + "Commander Ninja".to_string(), + Zone::Command, + ); + commander_obj.base_power = Some(2); + commander_obj.base_toughness = Some(2); + let cost = ManaCost::Cost { + shards: vec![ManaCostShard::Blue], + generic: 1, + }; + commander_obj + .keywords + .push(Keyword::CommanderNinjutsu(cost.clone())); + commander_obj + .base_keywords + .push(Keyword::CommanderNinjutsu(cost)); + commander_obj.card_types.core_types.push(CoreType::Creature); + commander_obj.card_types.subtypes.push("Ninja".to_string()); + + // Add to objects and command zone + state.objects.insert(commander_ninja_id, commander_obj); + // Need to use push_back for im::Vector + let mut new_command_zone = Vector::new(); + new_command_zone.push_back(commander_ninja_id); + state.command_zone = new_command_zone; + } + + // Set up combat state + runner.state_mut().combat = Some(CombatState { + attackers: vec![AttackerInfo::attacking_player(attacker, P1)], + ..Default::default() + }); + runner.state_mut().waiting_for = WaitingFor::Priority { player: P0 }; + runner.state_mut().priority_player = P0; + + // Add mana for Ninjutsu cost + { + let state = runner.state_mut(); + let player_data = state.players.iter_mut().find(|p| p.id == P0).unwrap(); + for _ in 0..2 { + player_data.mana_pool.add(ManaUnit::new( + ManaType::Blue, + ObjectId(0), + false, + Vec::new(), + )); + } + } + + // Get legal actions for P0 + let actions = engine::ai_support::legal_actions(runner.state()); + + // Check if ActivateNinjutsu is available for BOTH regular and Commander Ninjutsu + let ninjutsu_actions: Vec<_> = actions + .iter() + .filter(|action| matches!(action, GameAction::ActivateNinjutsu { .. })) + .collect(); + + if ninjutsu_actions.is_empty() { + println!("Available actions: {:?}", actions); + println!("Command zone: {:?}", runner.state().command_zone); + println!("Player hand: {:?}", runner.state().players[0].hand); + println!("Objects in command zone:"); + for &id in &runner.state().command_zone { + if let Some(obj) = runner.state().objects.get(&id) { + println!(" {:?}: {:?}", id, obj.name); + println!(" Keywords: {:?}", obj.keywords); + } + } + } + + // We should have at least one Ninjutsu action (from the hand ninja) + assert!( + !ninjutsu_actions.is_empty(), + "At least regular Ninjutsu should be offered from hand" + ); + + // If the Commander Ninjutsu card is properly configured, we should see TWO actions + // (one for hand ninja, one for commander ninja) + println!( + "Found {} Ninjutsu actions: {:?}", + ninjutsu_actions.len(), + ninjutsu_actions + ); +} From ff9862a4699d7a638d44e9c172da623fa914b475 Mon Sep 17 00:00:00 2001 From: Jonathanchang31 <55106972+jonathanchang31@users.noreply.github.com> Date: Thu, 18 Jun 2026 20:38:47 +0200 Subject: [PATCH 2/3] fix: Use create_object instead of manual --- .../tests/integration/ninjutsu_cluster.rs | 53 +++++++------------ 1 file changed, 19 insertions(+), 34 deletions(-) diff --git a/crates/engine/tests/integration/ninjutsu_cluster.rs b/crates/engine/tests/integration/ninjutsu_cluster.rs index 096b5a7613..4c97dbc1de 100644 --- a/crates/engine/tests/integration/ninjutsu_cluster.rs +++ b/crates/engine/tests/integration/ninjutsu_cluster.rs @@ -39,46 +39,31 @@ fn test_3648_commander_ninjutsu_offered_from_command_zone() { obj.base_keywords.push(Keyword::Ninjutsu(cost)); } - // Add a Commander Ninjutsu card to command zone - let commander_ninja_id = ObjectId(runner.state().next_object_id); + // Add a Commander Ninjutsu card to command zone using create_object helper + let commander_ninja_id = engine::game::zones::create_object( + runner.state_mut(), + engine::types::identifiers::CardId(999), + P0, + "Commander Ninja".to_string(), + engine::types::zones::Zone::Command, + ); { - use engine::game::game_object::GameObject; - use engine::im::Vector; use engine::types::card_type::CoreType; - use engine::types::identifiers::CardId; - use engine::types::zones::Zone; - - let state = runner.state_mut(); - - // Create the commander ninja object - let mut commander_obj = GameObject::new( - commander_ninja_id, - CardId(999), // Dummy card ID - P0, - "Commander Ninja".to_string(), - Zone::Command, - ); - commander_obj.base_power = Some(2); - commander_obj.base_toughness = Some(2); + let obj = runner + .state_mut() + .objects + .get_mut(&commander_ninja_id) + .unwrap(); + obj.base_power = Some(2); + obj.base_toughness = Some(2); let cost = ManaCost::Cost { shards: vec![ManaCostShard::Blue], generic: 1, }; - commander_obj - .keywords - .push(Keyword::CommanderNinjutsu(cost.clone())); - commander_obj - .base_keywords - .push(Keyword::CommanderNinjutsu(cost)); - commander_obj.card_types.core_types.push(CoreType::Creature); - commander_obj.card_types.subtypes.push("Ninja".to_string()); - - // Add to objects and command zone - state.objects.insert(commander_ninja_id, commander_obj); - // Need to use push_back for im::Vector - let mut new_command_zone = Vector::new(); - new_command_zone.push_back(commander_ninja_id); - state.command_zone = new_command_zone; + obj.keywords.push(Keyword::CommanderNinjutsu(cost.clone())); + obj.base_keywords.push(Keyword::CommanderNinjutsu(cost)); + obj.card_types.core_types.push(CoreType::Creature); + obj.card_types.subtypes.push("Ninja".to_string()); } // Set up combat state From ce9b1135b220ce148446a5997fb66f5da687bbc6 Mon Sep 17 00:00:00 2001 From: Matt Evans Date: Thu, 18 Jun 2026 13:37:54 -0700 Subject: [PATCH 3/3] test(engine): assert commander ninjutsu action --- .../tests/integration/ninjutsu_cluster.rs | 56 ++++++++----------- 1 file changed, 24 insertions(+), 32 deletions(-) diff --git a/crates/engine/tests/integration/ninjutsu_cluster.rs b/crates/engine/tests/integration/ninjutsu_cluster.rs index 4c97dbc1de..c99489525f 100644 --- a/crates/engine/tests/integration/ninjutsu_cluster.rs +++ b/crates/engine/tests/integration/ninjutsu_cluster.rs @@ -6,12 +6,15 @@ use engine::game::combat::{AttackerInfo, CombatState}; use engine::game::scenario::{GameScenario, P0, P1}; +use engine::game::zones; use engine::types::actions::GameAction; +use engine::types::card_type::CoreType; use engine::types::game_state::WaitingFor; -use engine::types::identifiers::ObjectId; +use engine::types::identifiers::{CardId, ObjectId}; use engine::types::keywords::Keyword; use engine::types::mana::{ManaCost, ManaCostShard, ManaType, ManaUnit}; use engine::types::phase::Phase; +use engine::types::zones::Zone; #[test] fn test_3648_commander_ninjutsu_offered_from_command_zone() { @@ -40,15 +43,14 @@ fn test_3648_commander_ninjutsu_offered_from_command_zone() { } // Add a Commander Ninjutsu card to command zone using create_object helper - let commander_ninja_id = engine::game::zones::create_object( + let commander_ninja_id = zones::create_object( runner.state_mut(), - engine::types::identifiers::CardId(999), + CardId(999), P0, "Commander Ninja".to_string(), - engine::types::zones::Zone::Command, + Zone::Command, ); { - use engine::types::card_type::CoreType; let obj = runner .state_mut() .objects @@ -64,6 +66,7 @@ fn test_3648_commander_ninjutsu_offered_from_command_zone() { obj.base_keywords.push(Keyword::CommanderNinjutsu(cost)); obj.card_types.core_types.push(CoreType::Creature); obj.card_types.subtypes.push("Ninja".to_string()); + obj.is_commander = true; } // Set up combat state @@ -91,36 +94,25 @@ fn test_3648_commander_ninjutsu_offered_from_command_zone() { // Get legal actions for P0 let actions = engine::ai_support::legal_actions(runner.state()); - // Check if ActivateNinjutsu is available for BOTH regular and Commander Ninjutsu + // Check if ActivateNinjutsu is available for BOTH regular and Commander Ninjutsu. let ninjutsu_actions: Vec<_> = actions .iter() - .filter(|action| matches!(action, GameAction::ActivateNinjutsu { .. })) - .collect(); - - if ninjutsu_actions.is_empty() { - println!("Available actions: {:?}", actions); - println!("Command zone: {:?}", runner.state().command_zone); - println!("Player hand: {:?}", runner.state().players[0].hand); - println!("Objects in command zone:"); - for &id in &runner.state().command_zone { - if let Some(obj) = runner.state().objects.get(&id) { - println!(" {:?}: {:?}", id, obj.name); - println!(" Keywords: {:?}", obj.keywords); + .filter_map(|action| { + if let GameAction::ActivateNinjutsu { + ninjutsu_object_id, + creature_to_return, + } = action + { + Some((*ninjutsu_object_id, *creature_to_return)) + } else { + None } - } - } - - // We should have at least one Ninjutsu action (from the hand ninja) - assert!( - !ninjutsu_actions.is_empty(), - "At least regular Ninjutsu should be offered from hand" - ); + }) + .collect(); - // If the Commander Ninjutsu card is properly configured, we should see TWO actions - // (one for hand ninja, one for commander ninja) - println!( - "Found {} Ninjutsu actions: {:?}", - ninjutsu_actions.len(), - ninjutsu_actions + assert_eq!( + ninjutsu_actions, + vec![(hand_ninja, attacker), (commander_ninja_id, attacker)], + "regular and commander ninjutsu must both be offered for the unblocked attacker" ); }