From f3e18e245fcdfd66bd282d0c3fc700c2b5a044bd Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Fri, 20 Jun 2025 18:20:50 +0400 Subject: [PATCH] AI: improved target amount targeting (part of #13638, #13766): - refactor: migrated AI's target amount code to shared selection logic; - ai: fixed game freezes on some use cases; - tests: added AI's testable dialogs for target amount; - tests: improved load tests result table, added game cycles stats; - Dwarven Catapult - fixed game error on usage; --- .../testers/ChooseAmountTestableDialog.java | 81 ++--- .../src/main/java/mage/view/GameView.java | 6 + .../src/mage/player/ai/ComputerPlayer6.java | 14 +- .../java/mage/player/ai/ComputerPlayer.java | 280 +++++++++--------- .../player/ai/PossibleTargetsComparator.java | 22 +- .../player/ai/PossibleTargetsSelector.java | 12 +- .../mage/player/ai/score/MagicAbility.java | 6 +- .../mage/player/ai/SimulatedPlayerMCTS.java | 6 + .../src/mage/player/human/HumanPlayer.java | 40 +-- .../src/mage/cards/d/DwarvenCatapult.java | 12 +- Mage.Sets/src/mage/cards/i/IvoryMask.java | 2 - .../src/mage/cards/n/NecroticPlague.java | 1 + .../test/AI/basic/TargetPriorityTest.java | 109 ++++--- .../test/cards/planeswalker/VivienTest.java | 20 +- .../test/dialogs/TestableDialogsTest.java | 3 +- .../java/org/mage/test/load/LoadTest.java | 38 ++- .../java/org/mage/test/player/TestPlayer.java | 39 ++- .../main/java/mage/abilities/AbilityImpl.java | 5 +- Mage/src/main/java/mage/target/Target.java | 8 +- .../main/java/mage/target/TargetAmount.java | 142 ++++++--- .../src/main/java/mage/target/TargetImpl.java | 33 ++- Mage/src/main/java/mage/target/Targets.java | 8 +- .../target/common/TargetCardInLibrary.java | 5 +- 23 files changed, 502 insertions(+), 390 deletions(-) diff --git a/Mage.Common/src/main/java/mage/utils/testers/ChooseAmountTestableDialog.java b/Mage.Common/src/main/java/mage/utils/testers/ChooseAmountTestableDialog.java index 066e48548a1..57388c4920b 100644 --- a/Mage.Common/src/main/java/mage/utils/testers/ChooseAmountTestableDialog.java +++ b/Mage.Common/src/main/java/mage/utils/testers/ChooseAmountTestableDialog.java @@ -41,6 +41,15 @@ class ChooseAmountTestableDialog extends BaseTestableDialog { this.targetsMax = targetsMax; } + private ChooseAmountTestableDialog aiMustChoose(boolean resStatus, int targetsCount) { + // TODO: AI use default distribution, imrove someday + TargetTestableResult res = ((TargetTestableResult) this.getResult()); + res.aiAssertEnabled = true; + res.aiAssertResStatus = resStatus; + res.aiAssertTargetsCount = targetsCount; + return this; + } + @Override public void showDialog(Player player, Ability source, Game game, Player opponent) { TargetAmount choosingTarget = new TargetAnyTargetAmount(this.distributeAmount, this.targetsMin, this.targetsMax); @@ -65,53 +74,55 @@ class ChooseAmountTestableDialog extends BaseTestableDialog { List isYous = Arrays.asList(false, true); + // current AI will choose 1 target and assign all values to it (except with outcome.Damage) + // TODO: add use cases for damage effects? for (boolean isYou : isYous) { // up to - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 0, 0, 0)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 0, 0, 1)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 0, 0, 3)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 0, 0, 5)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 0, 0, 0).aiMustChoose(false, 0)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 0, 0, 1).aiMustChoose(false, 0)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 0, 0, 3).aiMustChoose(false, 0)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 0, 0, 5).aiMustChoose(false, 0)); // - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to, invalid", 1, 0, 0)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 1, 0, 1)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 1, 0, 3)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 1, 0, 5)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to, invalid", 1, 0, 0).aiMustChoose(false, 0)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 1, 0, 1).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 1, 0, 3).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 1, 0, 5).aiMustChoose(true, 1)); // - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to, invalid", 2, 0, 0)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 2, 0, 1)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 2, 0, 3)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 2, 0, 5)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to, invalid", 2, 0, 0).aiMustChoose(false, 0)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 2, 0, 1).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 2, 0, 3).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 2, 0, 5).aiMustChoose(true, 1)); // - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to, invalid", 3, 0, 0)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 3, 0, 1)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 3, 0, 3)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 3, 0, 5)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to, invalid", 3, 0, 0).aiMustChoose(false, 0)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 3, 0, 1).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 3, 0, 3).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 3, 0, 5).aiMustChoose(true, 1)); // - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to, invalid", 5, 0, 0)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 5, 0, 1)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 5, 0, 3)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 5, 0, 5)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to, invalid", 5, 0, 0).aiMustChoose(false, 0)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 5, 0, 1).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 5, 0, 3).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "up to", 5, 0, 5).aiMustChoose(true, 1)); // need target - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 0, 1, 1)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 0, 1, 3)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 0, 1, 5)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 0, 1, 1).aiMustChoose(false, 0)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 0, 1, 3).aiMustChoose(false, 0)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 0, 1, 5).aiMustChoose(false, 0)); // - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 1, 1, 1)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 1, 1, 3)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 1, 1, 5)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 1, 1, 1).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 1, 1, 3).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 1, 1, 5).aiMustChoose(true, 1)); // - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 2, 1, 1)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 2, 1, 3)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 2, 1, 5)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 2, 1, 1).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 2, 1, 3).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 2, 1, 5).aiMustChoose(true, 1)); // - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 3, 1, 1)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 3, 1, 3)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 3, 1, 5)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 3, 1, 1).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 3, 1, 3).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 3, 1, 5).aiMustChoose(true, 1)); // - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 5, 1, 1)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 5, 1, 3)); - runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 5, 1, 5)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 5, 1, 1).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 5, 1, 3).aiMustChoose(true, 1)); + runner.registerDialog(new ChooseAmountTestableDialog(isYou, "need", 5, 1, 5).aiMustChoose(true, 1)); } } } diff --git a/Mage.Common/src/main/java/mage/view/GameView.java b/Mage.Common/src/main/java/mage/view/GameView.java index 59d47955831..4b6d06c7c95 100644 --- a/Mage.Common/src/main/java/mage/view/GameView.java +++ b/Mage.Common/src/main/java/mage/view/GameView.java @@ -67,6 +67,7 @@ public class GameView implements Serializable { // TODO: implement and support in admin tools private int totalErrorsCount; private int totalEffectsCount; + private int gameCycle; public GameView(GameState state, Game game, UUID createdForPlayerId, UUID watcherUserId) { Player createdForPlayer = null; @@ -214,6 +215,7 @@ public class GameView implements Serializable { this.rollbackTurnsAllowed = game.getOptions().rollbackTurnsAllowed; this.totalErrorsCount = game.getTotalErrorsCount(); this.totalEffectsCount = game.getTotalEffectsCount(); + this.gameCycle = game.getState().getApplyEffectsCounter(); } private void checkPaid(UUID uuid, StackAbility stackAbility) { @@ -358,4 +360,8 @@ public class GameView implements Serializable { public int getTotalEffectsCount() { return this.totalEffectsCount; } + + public int getGameCycle() { + return this.gameCycle; + } } diff --git a/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer6.java b/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer6.java index 4f189da96b7..b606a20d75e 100644 --- a/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer6.java +++ b/Mage.Server.Plugins/Mage.Player.AI.MA/src/mage/player/ai/ComputerPlayer6.java @@ -400,7 +400,7 @@ public class ComputerPlayer6 extends ComputerPlayer { if (effect != null && stackObject.getControllerId().equals(playerId)) { Target target = effect.getTarget(); - if (!target.isChoiceCompleted(game)) { + if (!target.isChoiceCompleted(getId(), (StackAbility) stackObject, game)) { for (UUID targetId : target.possibleTargets(stackObject.getControllerId(), stackObject.getStackAbility(), game)) { Game sim = game.createSimulationForAI(); StackAbility newAbility = (StackAbility) stackObject.copy(); @@ -849,10 +849,12 @@ public class ComputerPlayer6 extends ComputerPlayer { if (targets.isEmpty()) { return super.chooseTarget(outcome, cards, target, source, game); } - if (!target.isChoiceCompleted(game)) { + + UUID abilityControllerId = target.getAffectedAbilityControllerId(getId()); + if (!target.isChoiceCompleted(abilityControllerId, source, game)) { for (UUID targetId : targets) { target.addTarget(targetId, source, game); - if (target.isChoiceCompleted(game)) { + if (target.isChoiceCompleted(abilityControllerId, source, game)) { targets.clear(); return true; } @@ -867,10 +869,12 @@ public class ComputerPlayer6 extends ComputerPlayer { if (targets.isEmpty()) { return super.choose(outcome, cards, target, source, game); } - if (!target.isChoiceCompleted(game)) { + + UUID abilityControllerId = target.getAffectedAbilityControllerId(getId()); + if (!target.isChoiceCompleted(abilityControllerId, source, game)) { for (UUID targetId : targets) { target.add(targetId, game); - if (target.isChoiceCompleted(game)) { + if (target.isChoiceCompleted(abilityControllerId, source, game)) { targets.clear(); return true; } diff --git a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java index 96d4202f9cf..282d424bbd5 100644 --- a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java +++ b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java @@ -20,9 +20,7 @@ import mage.cards.repository.CardInfo; import mage.cards.repository.CardRepository; import mage.choices.Choice; import mage.constants.*; -import mage.counters.CounterType; import mage.filter.FilterPermanent; -import mage.filter.StaticFilters; import mage.filter.common.FilterCreatureForCombatBlock; import mage.filter.common.FilterLandCard; import mage.filter.common.FilterNonlandCard; @@ -158,16 +156,7 @@ public class ComputerPlayer extends PlayerImpl { return false; } - // controller hints: - // - target.getTargetController(), this.getId() -- player that must makes choices (must be same with this.getId) - // - target.getAbilityController(), abilityControllerId -- affected player/controller for all actions/filters - // - affected controller can be different from target controller (another player makes choices for controller) - // sometimes a target selection can be made from a player that does not control the ability - UUID abilityControllerId = playerId; - if (target.getTargetController() != null - && target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); - } + UUID abilityControllerId = target.getAffectedAbilityControllerId(getId()); // nothing to choose, e.g. X=0 if (target.isChoiceCompleted(abilityControllerId, source, game)) { @@ -175,13 +164,11 @@ public class ComputerPlayer extends PlayerImpl { } // default logic for any targets - boolean isAddedSomething = false; PossibleTargetsSelector possibleTargetsSelector = new PossibleTargetsSelector(outcome, target, abilityControllerId, source, game); possibleTargetsSelector.findNewTargets(fromCards); // good targets -- choose as much as possible for (MageItem item : possibleTargetsSelector.getGoodTargets()) { target.add(item.getId(), game); - isAddedSomething = true; if (target.isChoiceCompleted(abilityControllerId, source, game)) { return true; } @@ -192,9 +179,9 @@ public class ComputerPlayer extends PlayerImpl { break; } target.add(item.getId(), game); - isAddedSomething = true; } - return isAddedSomething; + + return target.isChosen(game) && !target.getTargets().isEmpty(); } /** @@ -273,147 +260,154 @@ public class ComputerPlayer extends PlayerImpl { @Override public boolean chooseTargetAmount(Outcome outcome, TargetAmount target, Ability source, Game game) { - // TODO: make same code for chooseTarget (without filter and target type dependence) + // nothing to choose, e.g. X=0 + target.prepareAmount(source, game); if (target.getAmountRemaining() <= 0) { return false; } - - UUID sourceId = source != null ? source.getSourceId() : null; - - // sometimes a target selection can be made from a player that does not control the ability - UUID abilityControllerId = playerId; - if (target.getTargetController() != null - && target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); - } - - // process multiple opponents by random - List opponents = new ArrayList<>(game.getOpponents(getId(), true)); - Collections.shuffle(opponents); - - List targets; - - // ONE KILL PRIORITY: player -> planeswalker -> creature - if (outcome == Outcome.Damage) { - // player kill - for (UUID opponentId : opponents) { - Player opponent = game.getPlayer(opponentId); - if (opponent != null - && target.canTarget(abilityControllerId, opponentId, source, game) - && opponent.getLife() <= target.getAmountRemaining()) { - return tryAddTarget(target, opponentId, opponent.getLife(), source, game); - } - } - - // permanents kill - for (UUID opponentId : opponents) { - targets = threats(opponentId, source, StaticFilters.FILTER_PERMANENT_CREATURE_OR_PLANESWALKER_A, game, target.getTargets()); - - // planeswalker kill - for (Permanent permanent : targets) { - if (permanent.isPlaneswalker(game) && target.canTarget(abilityControllerId, permanent.getId(), source, game)) { - int loy = permanent.getCounters(game).getCount(CounterType.LOYALTY); - if (loy <= target.getAmountRemaining()) { - return tryAddTarget(target, permanent.getId(), loy, source, game); - } - } - } - - // creature kill - for (Permanent permanent : targets) { - if (permanent.isCreature(game) && target.canTarget(abilityControllerId, permanent.getId(), source, game)) { - if (permanent.getToughness().getValue() <= target.getAmountRemaining()) { - return tryAddTarget(target, permanent.getId(), permanent.getToughness().getValue(), source, game); - } - } - } - } - } - - // NORMAL PRIORITY: planeswalker -> player -> creature - // own permanents will be checked multiple times... that's ok - for (UUID opponentId : opponents) { - if (outcome.isGood()) { - targets = threats(getId(), source, StaticFilters.FILTER_PERMANENT, game, target.getTargets()); - } else { - targets = threats(opponentId, source, StaticFilters.FILTER_PERMANENT, game, target.getTargets()); - } - - // planeswalkers - for (Permanent permanent : targets) { - if (permanent.isPlaneswalker(game) && target.canTarget(abilityControllerId, permanent.getId(), source, game)) { - return tryAddTarget(target, permanent.getId(), target.getAmountRemaining(), source, game); - } - } - - // players - if (outcome.isGood() && target.canTarget(abilityControllerId, getId(), source, game)) { - return tryAddTarget(target, getId(), target.getAmountRemaining(), source, game); - } - if (!outcome.isGood() && target.canTarget(abilityControllerId, opponentId, source, game)) { - return tryAddTarget(target, opponentId, target.getAmountRemaining(), source, game); - } - - // creature - for (Permanent permanent : targets) { - if (permanent.isCreature(game) && target.canTarget(abilityControllerId, permanent.getId(), source, game)) { - return tryAddTarget(target, permanent.getId(), target.getAmountRemaining(), source, game); - } - } - } - - // BAD PRIORITY, e.g. need bad target on yourself or good target on opponent - // priority: creature (non killable, killable) -> planeswalker -> player - if (!target.isRequired(sourceId, game)) { + if (target.getMaxNumberOfTargets() == 0 && target.getMinNumberOfTargets() == 0) { return false; } - for (UUID opponentId : opponents) { - if (!outcome.isGood()) { - // bad on yourself, uses the weakest targets - targets = threats(getId(), source, StaticFilters.FILTER_PERMANENT, game, target.getTargets(), false); - } else { - targets = threats(opponentId, source, StaticFilters.FILTER_PERMANENT, game, target.getTargets(), false); - } - // creatures - non killable (TODO: add extra skill checks like indestructible) - for (Permanent permanent : targets) { - if (permanent.isCreature(game) && target.canTarget(abilityControllerId, permanent.getId(), source, game)) { - int safeDamage = Math.min(permanent.getToughness().getValue() - 1, target.getAmountRemaining()); - if (safeDamage > 0) { - return tryAddTarget(target, permanent.getId(), safeDamage, source, game); + UUID abilityControllerId = target.getAffectedAbilityControllerId(getId()); + + // nothing to choose, e.g. X=0 + if (target.isChoiceCompleted(abilityControllerId, source, game)) { + return false; + } + + PossibleTargetsSelector possibleTargetsSelector = new PossibleTargetsSelector(outcome, target, abilityControllerId, source, game); + possibleTargetsSelector.findNewTargets(null); + + // nothing to choose, e.g. no valid targets + if (!possibleTargetsSelector.hasAnyTargets()) { + return false; + } + + // KILL PRIORITY + if (outcome == Outcome.Damage) { + // opponent first + for (MageItem item : possibleTargetsSelector.getGoodTargets()) { + if (target.getAmountRemaining() <= 0) { + break; + } + if (target.contains(item.getId()) || !(item instanceof Player)) { + continue; + } + int leftLife = PossibleTargetsComparator.getLifeForDamage(item, game); + if (leftLife > 0 && leftLife <= target.getAmountRemaining()) { + target.addTarget(item.getId(), leftLife, source, game); + if (target.isChoiceCompleted(abilityControllerId, source, game)) { + return true; } } } - // creatures - all - for (Permanent permanent : targets) { - if (permanent.isCreature(game) && target.canTarget(abilityControllerId, permanent.getId(), source, game)) { - return tryAddTarget(target, permanent.getId(), target.getAmountRemaining(), source, game); + // opponent's creatures second + for (MageItem item : possibleTargetsSelector.getGoodTargets()) { + if (target.getAmountRemaining() <= 0) { + break; + } + if (target.contains(item.getId()) || (item instanceof Player)) { + continue; + } + int leftLife = PossibleTargetsComparator.getLifeForDamage(item, game); + if (leftLife > 0 && leftLife <= target.getAmountRemaining()) { + target.addTarget(item.getId(), leftLife, source, game); + if (target.isChoiceCompleted(abilityControllerId, source, game)) { + return true; + } } } - // planeswalkers - for (Permanent permanent : targets) { - if (permanent.isPlaneswalker(game) && target.canTarget(abilityControllerId, permanent.getId(), source, game)) { - return tryAddTarget(target, permanent.getId(), target.getAmountRemaining(), source, game); + // opponent's any + for (MageItem item : possibleTargetsSelector.getGoodTargets()) { + if (target.getAmountRemaining() <= 0) { + break; + } + if (target.contains(item.getId())) { + continue; + } + target.addTarget(item.getId(), target.getAmountRemaining(), source, game); + if (target.isChoiceCompleted(abilityControllerId, source, game)) { + return true; } } + + // own - non-killable + for (MageItem item : possibleTargetsSelector.getBadTargets()) { + if (target.getAmountRemaining() <= 0) { + break; + } + if (target.contains(item.getId())) { + continue; + } + // stop as fast as possible on bad outcome + if (target.isChosen(game)) { + return !target.getTargets().isEmpty(); + } + int leftLife = PossibleTargetsComparator.getLifeForDamage(item, game); + if (leftLife > 1) { + target.addTarget(item.getId(), Math.min(leftLife - 1, target.getAmountRemaining()), source, game); + if (target.isChoiceCompleted(abilityControllerId, source, game)) { + return true; + } + } + } + + // own - any + for (MageItem item : possibleTargetsSelector.getBadTargets()) { + if (target.getAmountRemaining() <= 0) { + break; + } + if (target.contains(item.getId())) { + continue; + } + // stop as fast as possible on bad outcome + if (target.isChosen(game)) { + return !target.getTargets().isEmpty(); + } + target.addTarget(item.getId(), target.getAmountRemaining(), source, game); + if (target.isChoiceCompleted(abilityControllerId, source, game)) { + return true; + } + } + + return target.isChosen(game); } - // players - for (UUID opponentId : opponents) { - if (target.canTarget(abilityControllerId, getId(), source, game)) { - // on itself - return tryAddTarget(target, getId(), target.getAmountRemaining(), source, game); - } else if (target.canTarget(abilityControllerId, opponentId, source, game)) { - // on opponent - return tryAddTarget(target, opponentId, target.getAmountRemaining(), source, game); + + // non-damage effect like counters - give all to first valid item + for (MageItem item : possibleTargetsSelector.getGoodTargets()) { + if (target.getAmountRemaining() <= 0) { + break; + } + if (target.contains(item.getId())) { + continue; + } + target.addTarget(item.getId(), target.getAmountRemaining(), source, game); + if (target.isChoiceCompleted(abilityControllerId, source, game)) { + return true; + } + } + for (MageItem item : possibleTargetsSelector.getBadTargets()) { + if (target.getAmountRemaining() <= 0) { + break; + } + if (target.contains(item.getId())) { + continue; + } + // stop as fast as possible on bad outcome + if (target.isChosen(game)) { + return !target.getTargets().isEmpty(); + } + target.addTarget(item.getId(), target.getAmountRemaining(), source, game); + if (target.isChoiceCompleted(abilityControllerId, source, game)) { + return true; } } - // it's ok on no targets available - log.warn("No proper AI target handling or can't find permanents/cards to target: " + target.getClass().getName()); - return false; + return target.isChosen(game) && !target.getTargets().isEmpty(); } @Override @@ -1451,9 +1445,9 @@ public class ComputerPlayer extends PlayerImpl { } // sometimes a target selection can be made from a player that does not control the ability - UUID abilityControllerId = playerId; - if (target != null && target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); + UUID abilityControllerId = this.getId(); + if (target != null) { + abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); } Card bestCard = null; @@ -1490,9 +1484,9 @@ public class ComputerPlayer extends PlayerImpl { } // sometimes a target selection can be made from a player that does not control the ability - UUID abilityControllerId = playerId; - if (target != null && target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); + UUID abilityControllerId = this.getId(); + if (target != null) { + abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); } Card worstCard = null; diff --git a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/PossibleTargetsComparator.java b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/PossibleTargetsComparator.java index d8d9583208a..7f4f199b5ab 100644 --- a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/PossibleTargetsComparator.java +++ b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/PossibleTargetsComparator.java @@ -53,8 +53,7 @@ public class PossibleTargetsComparator { } } - private int getScoreFromLife(MageItem item) { - // TODO: replace permanent/card life by battlefield score? + public static int getLifeForDamage(MageItem item, Game game) { int res = 0; if (item instanceof Player) { res = ((Player) item).getLife(); @@ -71,12 +70,16 @@ public class PossibleTargetsComparator { } res = Math.max(0, card.getToughness().getValue() - damage); } - // instant - if (res == 0) { - res = card.getManaValue(); - } } + return res; + } + private int getScoreFromLife(MageItem item) { + // TODO: replace permanent/card life by battlefield score? + int res = getLifeForDamage(item, game); + if (res == 0 && item instanceof Card) { + res = ((Card) item).getManaValue(); + } return res; } @@ -139,13 +142,6 @@ public class PossibleTargetsComparator { .thenComparing(BY_ID); public final Comparator ANY_MOST_VALUABLE_LAST = ANY_MOST_VALUABLE_FIRST.reversed(); - /** - * Default sorting for good effects - put own and biggest items to the top - */ - public final Comparator MY_MOST_VALUABLE_FIRST = BY_ME - .thenComparing(ANY_MOST_VALUABLE_FIRST); - public final Comparator MY_MOST_VALUABLE_LAST = MY_MOST_VALUABLE_FIRST.reversed(); - /** * Sorting for discard effects - put the biggest unplayable at the top, lands at the end anyway */ diff --git a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/PossibleTargetsSelector.java b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/PossibleTargetsSelector.java index 60a5b0ae0a5..49422bcbdbe 100644 --- a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/PossibleTargetsSelector.java +++ b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/PossibleTargetsSelector.java @@ -85,13 +85,13 @@ public class PossibleTargetsSelector { private void sortByMostValuableTargets() { if (isGoodEffect()) { // for good effect must choose the biggest objects - this.me.sort(comparators.MY_MOST_VALUABLE_FIRST); - this.opponents.sort(comparators.MY_MOST_VALUABLE_LAST); + this.me.sort(comparators.ANY_MOST_VALUABLE_FIRST); + this.opponents.sort(comparators.ANY_MOST_VALUABLE_LAST); this.any.sort(comparators.ANY_MOST_VALUABLE_FIRST); } else { // for bad effect must choose the smallest objects - this.me.sort(comparators.MY_MOST_VALUABLE_LAST); - this.opponents.sort(comparators.MY_MOST_VALUABLE_FIRST); + this.me.sort(comparators.ANY_MOST_VALUABLE_LAST); + this.opponents.sort(comparators.ANY_MOST_VALUABLE_FIRST); this.any.sort(comparators.ANY_MOST_VALUABLE_LAST); } } @@ -181,4 +181,8 @@ public class PossibleTargetsSelector { } return false; } + + boolean hasAnyTargets() { + return !this.any.isEmpty(); + } } \ No newline at end of file diff --git a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/score/MagicAbility.java b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/score/MagicAbility.java index 31f71009b60..63346dc3ed0 100644 --- a/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/score/MagicAbility.java +++ b/Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/score/MagicAbility.java @@ -19,10 +19,10 @@ public final class MagicAbility { put(DoubleStrikeAbility.getInstance().getRule(), 100); put(new ExaltedAbility().getRule(), 10); put(FirstStrikeAbility.getInstance().getRule(), 50); - put(FlashAbility.getInstance().getRule(), 0); + put(FlashAbility.getInstance().getRule(), 20); put(FlyingAbility.getInstance().getRule(), 50); put(new ForestwalkAbility().getRule(), 10); - put(HasteAbility.getInstance().getRule(), 0); + put(HasteAbility.getInstance().getRule(), 20); put(IndestructibleAbility.getInstance().getRule(), 150); put(InfectAbility.getInstance().getRule(), 60); put(IntimidateAbility.getInstance().getRule(), 50); @@ -47,7 +47,7 @@ public final class MagicAbility { if (!scores.containsKey(ability.getRule())) { //System.err.println("Couldn't find ability score: " + ability.getClass().getSimpleName() + " - " + ability.toString()); //TODO: add handling protection from ..., levelup, kicker, etc. abilities - return 0; + return 2; // more abilities - more score in any use cases } return scores.get(ability.getRule()); } diff --git a/Mage.Server.Plugins/Mage.Player.AIMCTS/src/mage/player/ai/SimulatedPlayerMCTS.java b/Mage.Server.Plugins/Mage.Player.AIMCTS/src/mage/player/ai/SimulatedPlayerMCTS.java index 9e2b68cc055..f3dd92bb7be 100644 --- a/Mage.Server.Plugins/Mage.Player.AIMCTS/src/mage/player/ai/SimulatedPlayerMCTS.java +++ b/Mage.Server.Plugins/Mage.Player.AIMCTS/src/mage/player/ai/SimulatedPlayerMCTS.java @@ -288,9 +288,15 @@ public final class SimulatedPlayerMCTS extends MCTSPlayer { @Override public boolean chooseTargetAmount(Outcome outcome, TargetAmount target, Ability source, Game game) { + + // nothing to choose + target.prepareAmount(source, game); if (target.getAmountRemaining() <= 0) { return false; } + if (target.getMaxNumberOfTargets() == 0 && target.getMinNumberOfTargets() == 0) { + return false; + } Set possibleTargets = target.possibleTargets(playerId, source, game); if (possibleTargets.isEmpty()) { diff --git a/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java b/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java index 6cd626a034f..cd7b6569cf8 100644 --- a/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java +++ b/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java @@ -690,12 +690,8 @@ public class HumanPlayer extends PlayerImpl { return false; } - // choose one or multiple permanents - UUID abilityControllerId = playerId; - if (target.getTargetController() != null - && target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); - } + // choose one or multiple targets + UUID abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); if (options == null) { options = new HashMap<>(); } @@ -782,11 +778,7 @@ public class HumanPlayer extends PlayerImpl { } // choose one or multiple targets - UUID abilityControllerId = playerId; - if (target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); - } - + UUID abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); Map options = new HashMap<>(); while (canRespond()) { @@ -869,13 +861,7 @@ public class HumanPlayer extends PlayerImpl { return false; } - UUID abilityControllerId; - if (target.getTargetController() != null - && target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); - } else { - abilityControllerId = playerId; - } + UUID abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); while (canRespond()) { @@ -966,13 +952,7 @@ public class HumanPlayer extends PlayerImpl { return false; } - UUID abilityControllerId; - if (target.getTargetController() != null - && target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); - } else { - abilityControllerId = playerId; - } + UUID abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); while (canRespond()) { boolean required = target.isRequiredExplicitlySet() ? target.isRequired() : target.isRequired(source); @@ -1042,18 +1022,20 @@ public class HumanPlayer extends PlayerImpl { return false; } + // nothing to choose + target.prepareAmount(source, game); if (target.getAmountRemaining() <= 0) { return false; } + if (target.getMaxNumberOfTargets() == 0 && target.getMinNumberOfTargets() == 0) { + return false; + } if (source == null) { return false; } - UUID abilityControllerId = playerId; - if (target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); - } + UUID abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); int amountTotal = target.getAmountTotal(game, source); if (amountTotal == 0) { diff --git a/Mage.Sets/src/mage/cards/d/DwarvenCatapult.java b/Mage.Sets/src/mage/cards/d/DwarvenCatapult.java index d08553454ab..221b456587b 100644 --- a/Mage.Sets/src/mage/cards/d/DwarvenCatapult.java +++ b/Mage.Sets/src/mage/cards/d/DwarvenCatapult.java @@ -17,13 +17,12 @@ import mage.util.CardUtil; import java.util.UUID; /** - * * @author MarcoMarin */ public final class DwarvenCatapult extends CardImpl { public DwarvenCatapult(UUID ownerId, CardSetInfo setInfo) { - super(ownerId,setInfo,new CardType[]{CardType.INSTANT},"{X}{R}"); + super(ownerId, setInfo, new CardType[]{CardType.INSTANT}, "{X}{R}"); // Dwarven Catapult deals X damage divided evenly, rounded down, among all creatures target opponent controls. this.getSpellAbility().addTarget(new TargetOpponent()); @@ -55,10 +54,11 @@ class DwarvenCatapultEffect extends OneShotEffect { @Override public boolean apply(Game game, Ability source) { int howMany = game.getBattlefield().getAllActivePermanents(StaticFilters.FILTER_PERMANENT_CREATURES, source.getFirstTarget(), game).size(); - int amount = CardUtil.getSourceCostsTag(game, source, "X", 0)/howMany; - - DamageAllControlledTargetEffect dmgEffect = new DamageAllControlledTargetEffect(amount, new FilterCreaturePermanent()); - return dmgEffect.apply(game, source); + if (howMany > 0) { + int amount = CardUtil.getSourceCostsTag(game, source, "X", 0) / howMany; + return new DamageAllControlledTargetEffect(amount, new FilterCreaturePermanent()).apply(game, source); + } + return false; } @Override diff --git a/Mage.Sets/src/mage/cards/i/IvoryMask.java b/Mage.Sets/src/mage/cards/i/IvoryMask.java index 1d8d3eb5d0c..c178f6ad089 100644 --- a/Mage.Sets/src/mage/cards/i/IvoryMask.java +++ b/Mage.Sets/src/mage/cards/i/IvoryMask.java @@ -1,4 +1,3 @@ - package mage.cards.i; import java.util.UUID; @@ -19,7 +18,6 @@ public final class IvoryMask extends CardImpl { public IvoryMask(UUID ownerId, CardSetInfo setInfo) { super(ownerId,setInfo,new CardType[]{CardType.ENCHANTMENT},"{2}{W}{W}"); - // You have shroud. this.addAbility(new SimpleStaticAbility(new GainAbilityControllerEffect(ShroudAbility.getInstance()))); } diff --git a/Mage.Sets/src/mage/cards/n/NecroticPlague.java b/Mage.Sets/src/mage/cards/n/NecroticPlague.java index 3827b308fc9..91bfa44657b 100644 --- a/Mage.Sets/src/mage/cards/n/NecroticPlague.java +++ b/Mage.Sets/src/mage/cards/n/NecroticPlague.java @@ -85,6 +85,7 @@ enum NecroticPlagueAdjuster implements TargetAdjuster { ability.setControllerId(creatureController.getId()); ability.getTargets().clear(); TargetPermanent target = new TargetOpponentsCreaturePermanent(); + target.setAbilityController(ability.getControllerId()); target.setTargetController(creatureController.getId()); ability.getTargets().add(target); } diff --git a/Mage.Tests/src/test/java/org/mage/test/AI/basic/TargetPriorityTest.java b/Mage.Tests/src/test/java/org/mage/test/AI/basic/TargetPriorityTest.java index 17dc5e580e3..753d4bbc9fe 100644 --- a/Mage.Tests/src/test/java/org/mage/test/AI/basic/TargetPriorityTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/AI/basic/TargetPriorityTest.java @@ -21,52 +21,81 @@ import org.mage.test.serverside.base.CardTestPlayerBaseAI; public class TargetPriorityTest extends CardTestPlayerBaseAI { // TODO: enable _target_ tests after computerPlayer.chooseTarget will be reworks like chooseTargetAmount + @Test - @Ignore - public void test_target_PriorityKillByBigPT() { + public void test_Target_PriorityDamageToGoodOpponent() { addCard(Zone.HAND, playerA, "Lightning Bolt"); addCard(Zone.BATTLEFIELD, playerA, "Mountain", 1); // addCard(Zone.BATTLEFIELD, playerB, "Memnite", 3); // 1/1 addCard(Zone.BATTLEFIELD, playerB, "Balduvian Bears", 3); // 2/2 - addCard(Zone.BATTLEFIELD, playerB, "Ashcoat Bear", 3); // 2/2 with ability - addCard(Zone.BATTLEFIELD, playerB, "Golden Bear", 3); // 4/3 - addCard(Zone.BATTLEFIELD, playerB, "Battering Sliver", 3); // 4/4 with ability + addCard(Zone.BATTLEFIELD, playerA, "Ashcoat Bear", 3); // 2/2 with ability + // AI must make damage to opponent castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Lightning Bolt"); + setStrictChooseMode(false); setStopAt(1, PhaseStep.BEGIN_COMBAT); execute(); + assertLife(playerA, 20); + assertLife(playerB, 20 - 3); + } + + @Test + public void test_Target_PriorityDamageToBadLowestCreature() { + addCard(Zone.HAND, playerA, "Lightning Bolt"); + addCard(Zone.BATTLEFIELD, playerA, "Mountain", 1); + // + // You have shroud. + addCard(Zone.BATTLEFIELD, playerB, "Ivory Mask", 1); + // + addCard(Zone.BATTLEFIELD, playerA, "Balduvian Bears", 3); // 2/2 + addCard(Zone.BATTLEFIELD, playerA, "Memnite", 3); // 1/1 + addCard(Zone.BATTLEFIELD, playerA, "Ashcoat Bear", 3); // 2/2 with ability + + // AI can't target opponent so target own lowest creature + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Lightning Bolt"); + + setStrictChooseMode(false); + setStopAt(1, PhaseStep.BEGIN_COMBAT); + execute(); + + assertLife(playerA, 20); + assertLife(playerB, 20); + assertPermanentCount(playerA, "Memnite", 3 - 1); + } + + @Test + public void test_Target_PriorityDamageToBiggestCreature() { + addCard(Zone.HAND, playerA, "Lightning Bolt"); + addCard(Zone.BATTLEFIELD, playerA, "Mountain", 1); + // + // You have shroud. + addCard(Zone.BATTLEFIELD, playerA, "Ivory Mask", 1); + addCard(Zone.BATTLEFIELD, playerB, "Ivory Mask", 1); + // + addCard(Zone.BATTLEFIELD, playerB, "Memnite", 3); // 1/1 + addCard(Zone.BATTLEFIELD, playerB, "Balduvian Bears", 3); // 2/2 + addCard(Zone.BATTLEFIELD, playerB, "Ashcoat Bear", 3); // 2/2 with ability + addCard(Zone.BATTLEFIELD, playerB, "Golden Bear", 3); // 4/3 + //addCard(Zone.BATTLEFIELD, playerB, "Battering Sliver", 3); // 4/4 with ability TODO: add after AI will simulation simple choices too + + // AI must choose biggest creature to kill from opponent - 4/3 + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Lightning Bolt"); + waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN); + showBattlefield("as", 1, PhaseStep.PRECOMBAT_MAIN, playerB); + + setStrictChooseMode(false); + setStopAt(1, PhaseStep.BEGIN_COMBAT); + execute(); + + assertLife(playerA, 20); + assertLife(playerB, 20); assertPermanentCount(playerB, "Memnite", 3); assertPermanentCount(playerB, "Balduvian Bears", 3); assertPermanentCount(playerB, "Ashcoat Bear", 3); assertPermanentCount(playerB, "Golden Bear", 3 - 1); - assertPermanentCount(playerB, "Battering Sliver", 3); - } - - @Test - @Ignore - public void test_target_PriorityByKillByLowPT() { - addCard(Zone.HAND, playerA, "Lightning Bolt"); - addCard(Zone.BATTLEFIELD, playerA, "Mountain", 1); - // - addCard(Zone.BATTLEFIELD, playerB, "Memnite", 3); // 1/1 - //addCard(Zone.BATTLEFIELD, playerB, "Balduvian Bears", 3); // 2/2 - //addCard(Zone.BATTLEFIELD, playerB, "Ashcoat Bear", 3); // 2/2 with ability - //addCard(Zone.BATTLEFIELD, playerB, "Golden Bear", 3); // 4/3 - addCard(Zone.BATTLEFIELD, playerB, "Battering Sliver", 3); // 4/4 with ability - - castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Lightning Bolt"); - - setStopAt(1, PhaseStep.BEGIN_COMBAT); - execute(); - - assertPermanentCount(playerB, "Memnite", 3 - 1); - //assertPermanentCount(playerB, "Balduvian Bears", 3); - //assertPermanentCount(playerB, "Ashcoat Bear", 3); - //assertPermanentCount(playerB, "Golden Bear", 3); - assertPermanentCount(playerB, "Battering Sliver", 3); } @Test @@ -153,7 +182,7 @@ public class TargetPriorityTest extends CardTestPlayerBaseAI { // TARGET AMOUNT @Test - public void test_targetAmount_PriorityKillByBigPT() { + public void test_TargetAmount_PriorityKillByBigPT() { addCard(Zone.HAND, playerA, "Flames of the Firebrand"); // damage 3 addCard(Zone.BATTLEFIELD, playerA, "Mountain", 3); // @@ -176,7 +205,7 @@ public class TargetPriorityTest extends CardTestPlayerBaseAI { } @Test - public void test_targetAmount_PriorityByKillByLowPT() { + public void test_TargetAmount_PriorityByKillByLowPT() { addCard(Zone.HAND, playerA, "Flames of the Firebrand"); // damage 3 addCard(Zone.BATTLEFIELD, playerA, "Mountain", 3); // @@ -199,7 +228,7 @@ public class TargetPriorityTest extends CardTestPlayerBaseAI { } @Test - public void test_targetAmount_PriorityKillByExtraPoints() { + public void test_TargetAmount_PriorityKillByExtraPoints() { addCard(Zone.HAND, playerA, "Flames of the Firebrand"); // damage 3 addCard(Zone.BATTLEFIELD, playerA, "Mountain", 3); // @@ -222,7 +251,7 @@ public class TargetPriorityTest extends CardTestPlayerBaseAI { } @Test - public void test_targetAmount_NormalCase() { + public void test_TargetAmount_NormalCase() { Ability ability = new SimpleActivatedAbility(Zone.ALL, new DamageMultiEffect(), new ManaCostsImpl<>("{R}")); ability.addTarget(new TargetCreaturePermanentAmount(3, 0, 3)); addCustomCardWithAbility("damage 3", playerA, ability); @@ -247,7 +276,7 @@ public class TargetPriorityTest extends CardTestPlayerBaseAI { } @Test - public void test_targetAmount_BadCase() { + public void test_TargetAmount_BadCase() { // choose targets as enters battlefield (e.g. can't be canceled) SpellAbility spell = new SpellAbility(new ManaCostsImpl<>("{R}"), "damage 3", Zone.HAND); Ability ability = new EntersBattlefieldTriggeredAbility(new DamageMultiEffect()); @@ -263,14 +292,12 @@ public class TargetPriorityTest extends CardTestPlayerBaseAI { castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "damage 3"); - // must damage x3 Balduvian Bears by -1 to keep alive - checkDamage("pt after", 1, PhaseStep.BEGIN_COMBAT, playerA, "Balduvian Bears", 1); - // showBattlefield("after", 1, PhaseStep.BEGIN_COMBAT, playerA); + // up to target is optional, so AI must choose nothing due only bad targets setStopAt(1, PhaseStep.BEGIN_COMBAT); execute(); - assertPermanentCount(playerA, "damage 3", 1); - + assertLife(playerA, 20); + assertLife(playerB, 20); assertPermanentCount(playerA, "Memnite", 3); assertPermanentCount(playerA, "Balduvian Bears", 3); assertPermanentCount(playerA, "Ashcoat Bear", 3); @@ -280,7 +307,7 @@ public class TargetPriorityTest extends CardTestPlayerBaseAI { @Test @Ignore // do not enable it in production, only for devs - public void test_targetAmount_Performance() { + public void test_TargetAmount_Performance() { int cardsMultiplier = 3; Ability ability = new SimpleActivatedAbility(Zone.ALL, new DamageMultiEffect(), new ManaCostsImpl<>("{R}")); diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/planeswalker/VivienTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/planeswalker/VivienTest.java index 3b6f1720316..4650755a7dc 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/planeswalker/VivienTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/planeswalker/VivienTest.java @@ -14,19 +14,22 @@ import org.mage.test.serverside.base.CardTestPlayerBase; public class VivienTest extends CardTestPlayerBase { @Test - public void testVivienArkbowRangerAbility1NoTargets() { - setStrictChooseMode(true); + public void test_Distribute_NoTargets() { // +1: Distribute two +1/+1 counters among up to two target creatures. They gain trample until end of turn. // −3: Target creature you control deals damage equal to its power to target creature or planeswalker. // −5: You may choose a creature card you own from outside the game, reveal it, and put it into your hand. addCard(Zone.HAND, playerA, "Vivien, Arkbow Ranger"); // Planeswalker {1}{G}{G}{G} - starts with 4 Loyality counters addCard(Zone.BATTLEFIELD, playerA, "Forest", 4); + // You can activate Vivien’s first ability without choosing any target creatures. The counters won’t be + // put on anything. This is a change from previous rules regarding distributing counters. + // (2019-07-12) + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Vivien, Arkbow Ranger", true); + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "+1: Distribute"); addTargetAmount(playerA, TestPlayer.TARGET_SKIP); // stop choosing (not targets) - activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "+1: Distribute"); - + setStrictChooseMode(true); setStopAt(1, PhaseStep.BEGIN_COMBAT); execute(); @@ -36,8 +39,7 @@ public class VivienTest extends CardTestPlayerBase { } @Test - public void testVivienArkbowRangerAbilityOnePossibleTargetWithOne() { - setStrictChooseMode(true); + public void test_Distribute_OneTarget() { // +1: Distribute two +1/+1 counters among up to two target creatures. They gain trample until end of turn. // −3: Target creature you control deals damage equal to its power to target creature or planeswalker. // −5: You may choose a creature card you own from outside the game, reveal it, and put it into your hand. @@ -49,16 +51,16 @@ public class VivienTest extends CardTestPlayerBase { castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Vivien, Arkbow Ranger", true); activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "+1: Distribute"); - addTargetAmount(playerA, "Silvercoat Lion", 1); - addTargetAmount(playerA, TestPlayer.TARGET_SKIP); // stop choosing (one target) + addTargetAmount(playerA, "Silvercoat Lion", 2); + setStrictChooseMode(true); setStopAt(1, PhaseStep.BEGIN_COMBAT); execute(); assertPermanentCount(playerA, "Vivien, Arkbow Ranger", 1); assertCounterCount("Vivien, Arkbow Ranger", CounterType.LOYALTY, 5); - assertPowerToughness(playerB, "Silvercoat Lion", 2 + 1, 2 + 1); + assertPowerToughness(playerB, "Silvercoat Lion", 2 + 2, 2 + 2); } @Test diff --git a/Mage.Tests/src/test/java/org/mage/test/dialogs/TestableDialogsTest.java b/Mage.Tests/src/test/java/org/mage/test/dialogs/TestableDialogsTest.java index 939fab44585..7a41de3d740 100644 --- a/Mage.Tests/src/test/java/org/mage/test/dialogs/TestableDialogsTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/dialogs/TestableDialogsTest.java @@ -75,7 +75,6 @@ public class TestableDialogsTest extends CardTestPlayerBaseWithAIHelps { } @Test - @Ignore // TODO: enable and fix all failed dialogs public void test_RunAll_AI() { // it's impossible to setup 700+ dialogs, so all choices made by AI // current AI uses only simple choices in dialogs, not simulations @@ -108,7 +107,7 @@ public class TestableDialogsTest extends CardTestPlayerBaseWithAIHelps { @Test @Ignore // debug only - run single dialog by reg number public void test_RunSingle_Debugging() { - int needRegNumber = 93; + int needRegNumber = 557; prepareCards(); diff --git a/Mage.Tests/src/test/java/org/mage/test/load/LoadTest.java b/Mage.Tests/src/test/java/org/mage/test/load/LoadTest.java index 97b27a023ea..e9965808b0f 100644 --- a/Mage.Tests/src/test/java/org/mage/test/load/LoadTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/load/LoadTest.java @@ -918,12 +918,17 @@ public class LoadTest { public int getTotalEffectsCount() { return finalGameView == null ? 0 : this.finalGameView.getTotalEffectsCount(); } + + public int getGameCycle() { + return finalGameView == null ? 0 : this.finalGameView.getGameCycle(); + } } private static class LoadTestGameResultsList extends HashMap { - private static final String tableFormatHeader = "|%-10s|%-15s|%-20s|%-10s|%-10s|%-10s|%-10s|%-10s|%-15s|%-15s|%-10s|%n"; - private static final String tableFormatData = "|%-10s|%15s|%20s|%10s|%10s|%10s|%10s|%10s|%15s|%15s|%10s|%n"; + // index, name, random sid, game cycle, errors, effects, turn, life p1, life p2, creatures p1, creatures p2, =time, sec, ~time, sec + private static final String tableFormatHeader = "|%-10s|%-15s|%-20s|%-10s|%-10s|%-10s|%-10s|%-10s|%-10s|%-15s|%-15s|%-15s|%-15s|%n"; + private static final String tableFormatData = "|%-10s|%15s|%20s|%10s|%10s|%10s|%10s|%10s|%10s|%15s|%15s|%15s|%15s|%n"; public LoadTestGameResult createGame(int index, String name, long randomSeed) { if (this.containsKey(index)) { @@ -939,6 +944,7 @@ public class LoadTest { "index", "name", "random sid", + "game cycles", "errors", "effects", "turn", @@ -946,8 +952,8 @@ public class LoadTest { "life p2", "creatures p1", "creatures p2", - "time, sec", - "time per turn, sec" + "=time, sec", + "~time, sec" ); System.out.printf(tableFormatHeader, data.toArray()); } @@ -961,6 +967,7 @@ public class LoadTest { String.valueOf(gameResult.index), //"index", gameResult.name, //"name", String.valueOf(gameResult.randomSeed), // "random sid", + String.valueOf(gameResult.getGameCycle()), // "game cycles", String.valueOf(gameResult.getTotalErrorsCount()), // "errors", String.valueOf(gameResult.getTotalEffectsCount()), // "effects", gameResult.getTurnInfo(), //"turn", @@ -979,15 +986,16 @@ public class LoadTest { "TOTAL/AVG", //"index", String.valueOf(this.size()), //"name", "total, secs: " + String.format("%.3f", (float) this.getTotalDurationMs() / 1000), // "random sid", - String.valueOf(this.getTotalErrorsCount()), // errors - String.valueOf(this.getAvgEffectsCount()), // effects - String.valueOf(this.getAvgTurn()), // turn - String.valueOf(this.getAvgLife1()), // life p1 - String.valueOf(this.getAvgLife2()), // life p2 - String.valueOf(this.getAvgCreaturesCount1()), // creatures p1 - String.valueOf(this.getAvgCreaturesCount2()), // creatures p2 - String.valueOf(String.format("%.3f", (float) this.getAvgDurationMs() / 1000)), // time, sec - String.valueOf(String.format("%.3f", (float) this.getAvgDurationPerTurnMs() / 1000)) // time per turn, sec + "~" + this.getAvgGameCycle(), // game cycles + "=" + this.getTotalErrorsCount(), // errors + "~" + this.getAvgEffectsCount(), // effects + "~" + this.getAvgTurn(), // turn + "~" + this.getAvgLife1(), // life p1 + "~" + this.getAvgLife2(), // life p2 + "~" + this.getAvgCreaturesCount1(), // creatures p1 + "~" + this.getAvgCreaturesCount2(), // creatures p2 + "~" + String.format("%.3f", (float) this.getAvgDurationMs() / 1000), // time, sec + "~" + String.format("%.3f", (float) this.getAvgDurationPerTurnMs() / 1000) // time per turn, sec ); System.out.printf(tableFormatData, data.toArray()); } @@ -996,6 +1004,10 @@ public class LoadTest { return this.values().stream().mapToInt(LoadTestGameResult::getTotalErrorsCount).sum(); } + private int getAvgGameCycle() { + return this.size() == 0 ? 0 : this.values().stream().mapToInt(LoadTestGameResult::getGameCycle).sum() / this.size(); + } + private int getAvgEffectsCount() { return this.size() == 0 ? 0 : this.values().stream().mapToInt(LoadTestGameResult::getTotalEffectsCount).sum() / this.size(); } diff --git a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java index 50c202ce6f2..257266776f3 100644 --- a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java +++ b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java @@ -543,7 +543,7 @@ public class TestPlayer implements Player { if (currentTarget.getOriginalTarget() instanceof TargetCreaturePermanentAmount) { // supports only to set the complete amount to one target TargetCreaturePermanentAmount targetAmount = (TargetCreaturePermanentAmount) currentTarget.getOriginalTarget(); - targetAmount.setAmount(ability, game); + targetAmount.prepareAmount(ability, game); int amount = targetAmount.getAmountRemaining(); targetAmount.addTarget(id, amount, ability, game); targetsSet++; @@ -2101,10 +2101,7 @@ public class TestPlayer implements Player { if (target == null) { return "Target: null"; } - UUID abilityControllerId = getId(); - if (target.getTargetController() != null && target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); - } + UUID abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); Set possibleTargets = target.possibleTargets(abilityControllerId, source, game); return "Target: selected " + target.getSize() + ", possible " + possibleTargets.size() @@ -2274,10 +2271,7 @@ public class TestPlayer implements Player { return true; } - UUID abilityControllerId = this.getId(); - if (target.getTargetController() != null && target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); - } + UUID abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); // TODO: warning, some cards call player.choose methods instead target.choose, see #8254 // most use cases - discard and other cost with choice like that method @@ -2509,11 +2503,7 @@ public class TestPlayer implements Player { @Override public boolean chooseTarget(Outcome outcome, Target target, Ability source, Game game) { - UUID abilityControllerId = this.getId(); - if (target.getTargetController() != null && target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); - } - UUID sourceId = source != null ? source.getSourceId() : null; + UUID abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); assertAliasSupportInTargets(true); if (!targets.isEmpty()) { @@ -2817,10 +2807,7 @@ public class TestPlayer implements Player { @Override public boolean chooseTarget(Outcome outcome, Cards cards, TargetCard target, Ability source, Game game) { - UUID abilityControllerId = this.getId(); - if (target.getTargetController() != null && target.getAbilityController() != null) { - abilityControllerId = target.getAbilityController(); - } + UUID abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); assertAliasSupportInTargets(false); if (!targets.isEmpty()) { @@ -4329,12 +4316,20 @@ public class TestPlayer implements Player { // chooseTargetAmount calls for EACH target cycle (e.g. one target per click, see TargetAmount) // if use want to stop choosing then chooseTargetAmount must return false (example: up to xxx) + // nothing to choose + target.prepareAmount(source, game); if (target.getAmountRemaining() <= 0) { return false; } + if (target.getMaxNumberOfTargets() == 0 && target.getMinNumberOfTargets() == 0) { + return false; + } + + UUID abilityControllerId = target.getAffectedAbilityControllerId(this.getId()); assertAliasSupportInTargets(true); - if (!targets.isEmpty()) { + + while (!targets.isEmpty()) { // skip targets if (targets.get(0).equals(TARGET_SKIP)) { @@ -4386,7 +4381,11 @@ public class TestPlayer implements Player { // can select target.addTarget(possibleTarget, targetAmount, source, game); targets.remove(0); - return true; // one target per choose call + // allow test player to choose as much as possible until skip command + if (target.getAmountRemaining() <= 0) { + return true; + } + break; // try next target } } } diff --git a/Mage/src/main/java/mage/abilities/AbilityImpl.java b/Mage/src/main/java/mage/abilities/AbilityImpl.java index ad735b98e74..6366a6a4209 100644 --- a/Mage/src/main/java/mage/abilities/AbilityImpl.java +++ b/Mage/src/main/java/mage/abilities/AbilityImpl.java @@ -1226,10 +1226,7 @@ public abstract class AbilityImpl implements Ability { for (Mode mode : modes.values()) { boolean validTargets = true; for (Target target : mode.getTargets()) { - UUID abilityControllerId = controllerId; - if (target.getTargetController() != null) { - abilityControllerId = target.getTargetController(); - } + UUID abilityControllerId = target.getAffectedAbilityControllerId(controllerId); if (!target.canChoose(abilityControllerId, ability, game)) { validTargets = false; break; diff --git a/Mage/src/main/java/mage/target/Target.java b/Mage/src/main/java/mage/target/Target.java index 2f2c4f78e49..dbac98f943e 100644 --- a/Mage/src/main/java/mage/target/Target.java +++ b/Mage/src/main/java/mage/target/Target.java @@ -26,11 +26,9 @@ public interface Target extends Copyable, Serializable { * Warning, for "up to" targets it will return true all the time, so make sure your dialog * use do-while logic and call "choose" one time min or use isChoiceCompleted */ + @Deprecated // TODO: replace with UUID abilityControllerId, Ability source, Game game boolean isChosen(Game game); - @Deprecated // TODO: replace usage in cards by full version from choose methods - boolean isChoiceCompleted(Game game); - boolean isChoiceCompleted(UUID abilityControllerId, Ability source, Game game); void clearChosen(); @@ -189,14 +187,18 @@ public interface Target extends Copyable, Serializable { Target copy(); // some targets are chosen from players that are not the controller of the ability (e.g. Pandemonium) + // TODO: research usage of setTargetController and setAbilityController - target adjusters must set it both, example: Necrotic Plague void setTargetController(UUID playerId); UUID getTargetController(); + // TODO: research usage of setTargetController and setAbilityController - target adjusters must set it both, example: Necrotic Plague void setAbilityController(UUID playerId); UUID getAbilityController(); + UUID getAffectedAbilityControllerId(UUID choosingPlayerId); + Player getTargetController(Game game, UUID playerId); int getTargetTag(); diff --git a/Mage/src/main/java/mage/target/TargetAmount.java b/Mage/src/main/java/mage/target/TargetAmount.java index ccbfde6d66a..d28b1852459 100644 --- a/Mage/src/main/java/mage/target/TargetAmount.java +++ b/Mage/src/main/java/mage/target/TargetAmount.java @@ -16,13 +16,15 @@ import java.util.*; import java.util.stream.Collectors; /** + * Distribute value between targets list (damage, counters, etc) + * * @author BetaSteward_at_googlemail.com */ public abstract class TargetAmount extends TargetImpl { boolean amountWasSet = false; DynamicValue amount; - int remainingAmount; + int remainingAmount; // before any change to it - make sure you call prepareAmount protected TargetAmount(DynamicValue amount, int minNumberOfTargets, int maxNumberOfTargets) { this.amount = amount; @@ -46,15 +48,42 @@ public abstract class TargetAmount extends TargetImpl { @Override public boolean isChosen(Game game) { - return isChoiceCompleted(game); + if (!super.isChosen(game)) { + return false; + } + + // selection not started + if (!amountWasSet) { + return false; + } + + // distribution + if (getMinNumberOfTargets() == 0 && this.targets.isEmpty()) { + // allow 0 distribution, e.g. for "up to" targets like Vivien, Arkbow Ranger + return true; + } else { + // need full distribution + return remainingAmount == 0; + } } @Override - public boolean isChoiceCompleted(Game game) { - return amountWasSet - && (remainingAmount == 0 - || (getMinNumberOfTargets() < getMaxNumberOfTargets() - && getTargets().size() >= getMinNumberOfTargets())); + public boolean isChoiceCompleted(UUID abilityControllerId, Ability source, Game game) { + // make sure target request called one time minimum (for "up to" targets) + // choice is selected after any addTarget call (by test, AI or human players) + if (!isChoiceSelected()) { + return false; + } + + // make sure selected targets are valid + if (!isChosen(game)) { + return false; + } + + // TODO: need auto-choose here? See super + + // all other use cases are fine + return true; } @Override @@ -68,9 +97,14 @@ public abstract class TargetAmount extends TargetImpl { this.amount = amount; } - public void setAmount(Ability source, Game game) { - remainingAmount = amount.calculate(game, source, null); - amountWasSet = true; + /** + * Prepare new targets for choosing + */ + public void prepareAmount(Ability source, Game game) { + if (!amountWasSet) { + remainingAmount = amount.calculate(game, source, null); + amountWasSet = true; + } } public DynamicValue getAmount() { @@ -83,12 +117,11 @@ public abstract class TargetAmount extends TargetImpl { @Override public void addTarget(UUID id, int amount, Ability source, Game game, boolean skipEvent) { - if (!amountWasSet) { - setAmount(source, game); - } + prepareAmount(source, game); + if (amount <= remainingAmount) { - super.addTarget(id, amount, source, game, skipEvent); remainingAmount -= amount; + super.addTarget(id, amount, source, game, skipEvent); } } @@ -100,37 +133,70 @@ public abstract class TargetAmount extends TargetImpl { } @Override + public boolean choose(Outcome outcome, UUID playerId, UUID sourceId, Ability source, Game game) { + throw new IllegalArgumentException("Wrong code usage. TargetAmount must be called by player.chooseTarget, not player.choose"); + } + + @Override + @Deprecated // TODO: replace by player.chooseTargetAmount call public boolean chooseTarget(Outcome outcome, UUID playerId, Ability source, Game game) { - Player player = game.getPlayer(playerId); - if (player == null) { + Player targetController = getTargetController(game, playerId); + if (targetController == null) { return false; } - if (!amountWasSet) { - setAmount(source, game); - } + prepareAmount(source, game); - while (remainingAmount > 0) { - chosen = false; - if (!player.canRespond()) { - chosen = isChosen(game); + chosen = false; + do { + int prevTargetsCount = this.getTargets().size(); + + // stop by disconnect + if (!targetController.canRespond()) { break; } - if (!getTargetController(game, playerId).chooseTargetAmount(outcome, this, source, game)) { - chosen = isChosen(game); - break; + + // MAKE A CHOICE + if (isRandom()) { + // random choice + throw new IllegalArgumentException("Wrong code usage. TargetAmount do not support random choices"); + } else { + // player's choice + + // TargetAmount do not support auto-choice + + // manual + + // stop by cancel/done + if (!targetController.chooseTargetAmount(outcome, this, source, game)) { + break; + } + + // continue to next target } + chosen = isChosen(game); - } - return isChosen(game); + // stop by full complete + if (isChoiceCompleted(targetController.getId(), source, game)) { + break; + } + + // stop by nothing to choose (actual for human and done button?) + if (prevTargetsCount == this.getTargets().size()) { + break; + } + + // can select next target + } while (true); + + chosen = isChosen(game); + return chosen && !this.getTargets().isEmpty(); } @Override final public List getTargetOptions(Ability source, Game game) { - if (!amountWasSet) { - setAmount(source, game); - } + prepareAmount(source, game); List options = new ArrayList<>(); Set possibleTargets = possibleTargets(source.getControllerId(), source, game); @@ -370,9 +436,8 @@ public abstract class TargetAmount extends TargetImpl { } public void setTargetAmount(UUID targetId, int amount, Ability source, Game game) { - if (!amountWasSet) { - setAmount(source, game); - } + prepareAmount(source, game); + remainingAmount -= (amount - this.getTargetAmount(targetId)); this.setTargetAmount(targetId, amount, game); } @@ -396,4 +461,13 @@ public abstract class TargetAmount extends TargetImpl { // Each of these targets must receive at least one of whatever is being divided. return amount instanceof StaticValue && max == ((StaticValue) amount).getValue(); } + + @Override + public String toString() { + if (amountWasSet) { + return super.toString() + String.format(" (remain amount %d of %s)", this.remainingAmount, this.amount.toString()); + } else { + return super.toString() + String.format(" (remain not prepared, %s)", this.amount.toString()); + } + } } diff --git a/Mage/src/main/java/mage/target/TargetImpl.java b/Mage/src/main/java/mage/target/TargetImpl.java index 19dd98814b0..48f6f4bf73f 100644 --- a/Mage/src/main/java/mage/target/TargetImpl.java +++ b/Mage/src/main/java/mage/target/TargetImpl.java @@ -260,11 +260,6 @@ public abstract class TargetImpl implements Target { return chosen || (targets.size() >= getMinNumberOfTargets() && targets.size() <= getMaxNumberOfTargets()); } - @Override - public boolean isChoiceCompleted(Game game) { - return isChoiceCompleted(null, null, game); - } - @Override public boolean isChoiceCompleted(UUID abilityControllerId, Ability source, Game game) { // make sure target request called one time minimum (for "up to" targets) @@ -406,10 +401,7 @@ public abstract class TargetImpl implements Target { return false; } - UUID abilityControllerId = playerId; - if (this.getTargetController() != null && this.getAbilityController() != null) { - abilityControllerId = this.getAbilityController(); - } + UUID abilityControllerId = this.getAffectedAbilityControllerId(playerId); chosen = false; do { @@ -444,7 +436,7 @@ public abstract class TargetImpl implements Target { } while (true); chosen = isChosen(game); - return this.getTargets().size() > 0; + return chosen && !this.getTargets().isEmpty(); } @Override @@ -454,10 +446,7 @@ public abstract class TargetImpl implements Target { return false; } - UUID abilityControllerId = playerId; - if (this.getTargetController() != null && this.getAbilityController() != null) { - abilityControllerId = this.getAbilityController(); - } + UUID abilityControllerId = this.getAffectedAbilityControllerId(playerId); List randomPossibleTargets = new ArrayList<>(possibleTargets(playerId, source, game)); @@ -527,7 +516,7 @@ public abstract class TargetImpl implements Target { } while (true); chosen = isChosen(game); - return this.getTargets().size() > 0; + return chosen && !this.getTargets().isEmpty(); } @Override @@ -726,6 +715,20 @@ public abstract class TargetImpl implements Target { return abilityController; } + @Override + public UUID getAffectedAbilityControllerId(UUID choosingPlayerId) { + // controller hints: + // - target.getTargetController(), this.getId(), choosingPlayerId -- player that must makes choices (must be same with this.getId) + // - target.getAbilityController(), abilityControllerId -- affected player/controller for all actions/filters + // - affected controller can be different from target controller (another player makes choices for controller) + // sometimes a target selection can be made from a player that does not control the ability + UUID abilityControllerId = choosingPlayerId; + if (this.getAbilityController() != null) { + abilityControllerId = this.getAbilityController(); + } + return abilityControllerId; + } + @Override public Player getTargetController(Game game, UUID playerId) { if (getTargetController() != null) { diff --git a/Mage/src/main/java/mage/target/Targets.java b/Mage/src/main/java/mage/target/Targets.java index 3ae7326bcdc..cba1f867cef 100644 --- a/Mage/src/main/java/mage/target/Targets.java +++ b/Mage/src/main/java/mage/target/Targets.java @@ -63,8 +63,8 @@ public class Targets extends ArrayList implements Copyable { return unchosenIndex < res.size() ? res.get(unchosenIndex) : null; } - public boolean isChoiceCompleted(Game game) { - return stream().allMatch(t -> t.isChoiceCompleted(game)); + public boolean isChoiceCompleted(UUID abilityControllerId, Ability source, Game game) { + return stream().allMatch(t -> t.isChoiceCompleted(abilityControllerId, source, game)); } public void clearChosen() { @@ -101,9 +101,7 @@ public class Targets extends ArrayList implements Copyable { // stop on cancel/done if (!target.choose(outcome, playerId, sourceId, source, game)) { - if (!target.isChosen(game)) { - break; - } + break; } // target done, can take next one diff --git a/Mage/src/main/java/mage/target/common/TargetCardInLibrary.java b/Mage/src/main/java/mage/target/common/TargetCardInLibrary.java index cfb91500bec..a6c111bca01 100644 --- a/Mage/src/main/java/mage/target/common/TargetCardInLibrary.java +++ b/Mage/src/main/java/mage/target/common/TargetCardInLibrary.java @@ -76,10 +76,7 @@ public class TargetCardInLibrary extends TargetCard { Cards cardsId = new CardsImpl(); cards.forEach(cardsId::add); - UUID abilityControllerId = playerId; - if (this.getTargetController() != null && this.getAbilityController() != null) { - abilityControllerId = this.getAbilityController(); - } + UUID abilityControllerId = this.getAffectedAbilityControllerId(playerId); chosen = false; do {