From 2d9ac4e732c602734e5f6cdd75ea0f86df9fab09 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Thu, 24 Oct 2024 15:31:04 +0400 Subject: [PATCH] refactor: removed outdated Player::assignDamage by multi amount dialog, fixed getMultiAmount to work with min values, added additional checks --- .../java/mage/player/ai/ComputerPlayer.java | 13 +--- .../mage/player/ai/SimulatedPlayerMCTS.java | 32 ---------- .../src/mage/player/human/HumanPlayer.java | 61 ++++++------------- .../src/mage/cards/g/GrandWarlordRadha.java | 2 +- .../mage/cards/k/KlauthUnrivaledAncient.java | 2 +- .../java/org/mage/test/player/TestPlayer.java | 16 ++--- .../AddConditionalManaOfAnyColorEffect.java | 2 +- .../mana/AddManaInAnyCombinationEffect.java | 2 +- .../effects/mana/DynamicManaEffect.java | 2 +- .../java/mage/constants/MultiAmountType.java | 4 +- Mage/src/main/java/mage/players/Player.java | 34 ++++++----- .../main/java/mage/players/StubPlayer.java | 7 +-- 12 files changed, 50 insertions(+), 127 deletions(-) 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 386e9a29c24..bb1fd669b17 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 @@ -2232,13 +2232,6 @@ public class ComputerPlayer extends PlayerImpl { return null; } - @Override - public void assignDamage(int damage, List targets, String singleTargetName, UUID attackerId, Ability source, Game game) { - log.debug("assignDamage"); - //TODO: improve this - game.getPermanent(targets.get(0)).damage(damage, attackerId, source, game); - } - @Override // TODO: add AI support with outcome and replace random with min/max public int getAmount(int min, int max, String message, Game game) { @@ -2254,11 +2247,11 @@ public class ComputerPlayer extends PlayerImpl { @Override public List getMultiAmountWithIndividualConstraints(Outcome outcome, List messages, - int min, int max, MultiAmountType type, Game game) { + int totalMin, int totalMax, MultiAmountType type, Game game) { log.debug("getMultiAmount"); int needCount = messages.size(); - List defaultList = MultiAmountType.prepareDefaltValues(messages, min, max); + List defaultList = MultiAmountType.prepareDefaltValues(messages, totalMin, totalMax); if (needCount == 0) { return defaultList; } @@ -2273,7 +2266,7 @@ public class ComputerPlayer extends PlayerImpl { // GOOD effect // values must be stable, so AI must able to simulate it and choose correct actions // fill max values as much as possible - return MultiAmountType.prepareMaxValues(messages, min, max); + return MultiAmountType.prepareMaxValues(messages, totalMin, totalMax); } @Override 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 4cc146e6fa6..3bb1a34df10 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 @@ -390,38 +390,6 @@ public final class SimulatedPlayerMCTS extends MCTSPlayer { return super.chooseBlockerOrder(blockers, combatGroup, blockerOrder, game); } - @Override - public void assignDamage(int damage, List targets, String singleTargetName, UUID attackerId, Ability source, Game game) { - if (this.isHuman()) { - int remainingDamage = damage; - UUID targetId; - int amount; - while (remainingDamage > 0) { - if (targets.size() == 1) { - targetId = targets.get(0); - amount = remainingDamage; - } else { - targetId = targets.get(RandomUtil.nextInt(targets.size())); - amount = RandomUtil.nextInt(damage + 1); - } - Permanent permanent = game.getPermanent(targetId); - if (permanent != null) { - permanent.damage(amount, attackerId, source, game, false, true); - remainingDamage -= amount; - } else { - Player player = game.getPlayer(targetId); - if (player != null) { - player.damage(amount, attackerId, source, game); - remainingDamage -= amount; - } - } - targets.remove(targetId); - } - } else { - super.assignDamage(damage, targets, singleTargetName, attackerId, source, game); - } - } - @Override public int getAmount(int min, int max, String message, Game game) { if (this.isHuman()) { 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 01e7b642b81..05153906c93 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 @@ -41,7 +41,6 @@ import mage.target.Target; import mage.target.TargetAmount; import mage.target.TargetCard; import mage.target.TargetPermanent; -import mage.target.common.TargetAnyTarget; import mage.target.common.TargetAttackingCreature; import mage.target.common.TargetDefender; import mage.target.targetpointer.TargetPointer; @@ -1140,7 +1139,7 @@ public class HumanPlayer extends PlayerImpl { } // ask and assign new amount - List targetValues = getMultiAmount(outcome, targetNames, 1, amountTotal, multiAmountType, game); + List targetValues = getMultiAmount(outcome, targetNames, 1, amountTotal, amountTotal, multiAmountType, game); for (int i = 0; i < targetValues.size(); i++) { int newAmount = targetValues.get(i); UUID targetId = targets.get(i); @@ -1916,7 +1915,7 @@ public class HumanPlayer extends PlayerImpl { // check if enough attackers are declared // check if players have to be attacked Set playersToAttackIfAble = new HashSet<>(); - + // or if active player must attack with anything boolean mustAttack = false; @@ -1927,7 +1926,7 @@ public class HumanPlayer extends PlayerImpl { if (playerToAttack != null) { playersToAttackIfAble.add(playerToAttack); } - if (effect.mustAttack(game)){ + if (effect.mustAttack(game)) { mustAttack = true; } } @@ -1967,10 +1966,10 @@ public class HumanPlayer extends PlayerImpl { } } - if (mustAttack && game.getCombat().getAttackers().isEmpty()){ + if (mustAttack && game.getCombat().getAttackers().isEmpty()) { // no attackers, but required to attack with something -- check if anything can attack for (Permanent attacker : game.getBattlefield().getAllActivePermanents(StaticFilters.FILTER_PERMANENT_CREATURE, getId(), game)) { - if (attacker.canAttackInPrinciple(null, game)){ + if (attacker.canAttackInPrinciple(null, game)) { game.informPlayer(this, "You are forced to attack with at least one creature, e.g. " + attacker.getIdName() + "."); return false; } @@ -2206,33 +2205,6 @@ public class HumanPlayer extends PlayerImpl { } } - @Override - public void assignDamage(int damage, java.util.List targets, String singleTargetName, UUID attackerId, Ability source, Game game) { - int remainingDamage = damage; - while (remainingDamage > 0 && canRespond()) { - Target target = new TargetAnyTarget(); - target.withNotTarget(true); - if (singleTargetName != null) { - target.withTargetName(singleTargetName); - } - this.choose(Outcome.Damage, target, source, game); - if (targets.isEmpty() || targets.contains(target.getFirstTarget())) { - int damageAmount = this.getAmount(0, remainingDamage, "Select amount", game); - Permanent permanent = game.getPermanent(target.getFirstTarget()); - if (permanent != null) { - permanent.damage(damageAmount, attackerId, source, game, false, true); - remainingDamage -= damageAmount; - } else { - Player player = game.getPlayer(target.getFirstTarget()); - if (player != null) { - player.damage(damageAmount, attackerId, source, game); - remainingDamage -= damageAmount; - } - } - } - } - } - @Override public int getAmount(int min, int max, String message, Game game) { if (gameInCheckPlayableState(game)) { @@ -2262,15 +2234,16 @@ public class HumanPlayer extends PlayerImpl { public List getMultiAmountWithIndividualConstraints( Outcome outcome, List messages, - int min, - int max, + int totalMin, + int totalMax, MultiAmountType type, Game game ) { int needCount = messages.size(); - List defaultList = MultiAmountType.prepareDefaltValues(messages, min, max); - if (needCount == 0 || (needCount == 1 && min == max) + List defaultList = MultiAmountType.prepareDefaltValues(messages, totalMin, totalMax); + if (needCount == 0 || (needCount == 1 && totalMin == totalMax) || messages.stream().map(m -> m.min == m.max).reduce(true, Boolean::logicalAnd)) { + // nothing to choose return defaultList; } @@ -2288,19 +2261,19 @@ public class HumanPlayer extends PlayerImpl { if (type.isCanCancel()) { options.put("canCancel", true); } - game.fireGetMultiAmountEvent(playerId, messages, min, max, options); + game.fireGetMultiAmountEvent(playerId, messages, totalMin, totalMax, options); } waitForResponse(game); // waiting correct values only if (response.getString() != null) { - answer = MultiAmountType.parseAnswer(response.getString(), messages, min, max, false); - if (MultiAmountType.isGoodValues(answer, messages, min, max)) { + answer = MultiAmountType.parseAnswer(response.getString(), messages, totalMin, totalMax, false); + if (MultiAmountType.isGoodValues(answer, messages, totalMin, totalMax)) { break; } else { // it's not normal: can be cheater or a wrong GUI checks answer = null; - logger.error(String.format("GUI return wrong MultiAmountType values: %d %d %d - %s", needCount, min, max, response.getString())); + logger.error(String.format("GUI return wrong MultiAmountType values: %d %d %d - %s", needCount, totalMin, totalMax, response.getString())); game.informPlayer(this, "Error, you must enter correct values."); } } else if (type.isCanCancel() && response.getBoolean() != null) { @@ -2594,8 +2567,8 @@ public class HumanPlayer extends PlayerImpl { modeText = Character.toUpperCase(modeText.charAt(0)) + modeText.substring(1); } StringBuilder sb = new StringBuilder(); - if (mode.getPawPrintValue() > 0){ - for (int i = 0; i < mode.getPawPrintValue(); ++i){ + if (mode.getPawPrintValue() > 0) { + for (int i = 0; i < mode.getPawPrintValue(); ++i) { sb.append("{P}"); } sb.append(": "); @@ -2617,7 +2590,7 @@ public class HumanPlayer extends PlayerImpl { // prepare dialog String message; - if (modes.getMaxPawPrints() == 0){ + if (modes.getMaxPawPrints() == 0) { message = "Choose mode (selected " + modes.getSelectedModes().size() + " of " + modes.getMaxModes(game, source) + ", min " + modes.getMinModes() + ")"; } else { diff --git a/Mage.Sets/src/mage/cards/g/GrandWarlordRadha.java b/Mage.Sets/src/mage/cards/g/GrandWarlordRadha.java index 8ff143a584a..ad6f8a815ba 100644 --- a/Mage.Sets/src/mage/cards/g/GrandWarlordRadha.java +++ b/Mage.Sets/src/mage/cards/g/GrandWarlordRadha.java @@ -73,7 +73,7 @@ class GrandWarlordRadhaEffect extends OneShotEffect { if (controller == null || amount < 1) { return false; } - List manaList = controller.getMultiAmount(this.outcome, Arrays.asList("R", "G"), 0, amount, MultiAmountType.MANA, game); + List manaList = controller.getMultiAmount(this.outcome, Arrays.asList("R", "G"), 0, amount, amount, MultiAmountType.MANA, game); Mana mana = new Mana(); mana.add(new Mana(ColoredManaSymbol.R, manaList.get(0))); diff --git a/Mage.Sets/src/mage/cards/k/KlauthUnrivaledAncient.java b/Mage.Sets/src/mage/cards/k/KlauthUnrivaledAncient.java index 3d19c5d4f8f..ab264b4b42b 100644 --- a/Mage.Sets/src/mage/cards/k/KlauthUnrivaledAncient.java +++ b/Mage.Sets/src/mage/cards/k/KlauthUnrivaledAncient.java @@ -94,7 +94,7 @@ class KlauthUnrivaledAncientEffect extends OneShotEffect { .mapToInt(MageInt::getValue) .sum(); List manaList = player.getMultiAmount( - outcome, manaSymbols, 0, attackerPower, MultiAmountType.MANA, game + outcome, manaSymbols, 0, attackerPower, attackerPower, MultiAmountType.MANA, game ); player.getManaPool().addMana( new KlauthUnrivaledAncientConditionalMana(manaList), game, source, true 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 65f5cbcb63e..df2f3cf3958 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 @@ -2875,11 +2875,11 @@ public class TestPlayer implements Player { @Override public List getMultiAmountWithIndividualConstraints(Outcome outcome, List messages, - int min, int max, MultiAmountType type, Game game) { + int totalMin, int totalMax, MultiAmountType type, Game game) { assertAliasSupportInChoices(false); int needCount = messages.size(); - List defaultList = MultiAmountType.prepareDefaltValues(messages, min, max); + List defaultList = MultiAmountType.prepareDefaltValues(messages, totalMin, totalMax); if (needCount == 0) { return defaultList; } @@ -2905,7 +2905,7 @@ public class TestPlayer implements Player { } // extra check - if (!MultiAmountType.isGoodValues(answer, messages, min, max)) { + if (!MultiAmountType.isGoodValues(answer, messages, totalMin, totalMax)) { Assert.fail("Wrong choices in multi amount: " + answer .stream() .map(String::valueOf) @@ -2916,7 +2916,7 @@ public class TestPlayer implements Player { } this.chooseStrictModeFailed("choice", game, "Multi amount: " + type.getHeader()); - return computerPlayer.getMultiAmountWithIndividualConstraints(outcome, messages, min, max, type, game); + return computerPlayer.getMultiAmountWithIndividualConstraints(outcome, messages, totalMin, totalMax, type, game); } @Override @@ -4384,14 +4384,6 @@ public class TestPlayer implements Player { return computerPlayer.chooseBlockerOrder(blockers, combatGroup, blockerOrder, game); } - @Override - public void assignDamage(int damage, List targets, - String singleTargetName, UUID attackerId, Ability source, - Game game - ) { - computerPlayer.assignDamage(damage, targets, singleTargetName, attackerId, source, game); - } - @Override public void sideboard(Match match, Deck deck ) { diff --git a/Mage/src/main/java/mage/abilities/effects/mana/AddConditionalManaOfAnyColorEffect.java b/Mage/src/main/java/mage/abilities/effects/mana/AddConditionalManaOfAnyColorEffect.java index 3d920a70455..7f33b5a1953 100644 --- a/Mage/src/main/java/mage/abilities/effects/mana/AddConditionalManaOfAnyColorEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/mana/AddConditionalManaOfAnyColorEffect.java @@ -104,7 +104,7 @@ public class AddConditionalManaOfAnyColorEffect extends ManaEffect { manaStrings.add("B"); manaStrings.add("R"); manaStrings.add("G"); - List choices = controller.getMultiAmount(this.outcome, manaStrings, 0, value, MultiAmountType.MANA, game); + List choices = controller.getMultiAmount(this.outcome, manaStrings, 0, value, value, MultiAmountType.MANA, game); Mana mana = new Mana(choices.get(0), choices.get(1), choices.get(2), choices.get(3), choices.get(4), 0, 0, 0); return manaBuilder.setMana(mana, source, game).build(); } diff --git a/Mage/src/main/java/mage/abilities/effects/mana/AddManaInAnyCombinationEffect.java b/Mage/src/main/java/mage/abilities/effects/mana/AddManaInAnyCombinationEffect.java index ad63976b497..e81528cfa43 100644 --- a/Mage/src/main/java/mage/abilities/effects/mana/AddManaInAnyCombinationEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/mana/AddManaInAnyCombinationEffect.java @@ -117,7 +117,7 @@ public class AddManaInAnyCombinationEffect extends ManaEffect { // Ask player for color distribution int manaAmount = amount.calculate(game, source, this); - List manaList = player.getMultiAmount(this.outcome, manaStrings, 0, manaAmount, MultiAmountType.MANA, game); + List manaList = player.getMultiAmount(this.outcome, manaStrings, 0, manaAmount, manaAmount, MultiAmountType.MANA, game); // Convert choices to mana for (int i = 0; i < size; i++) { diff --git a/Mage/src/main/java/mage/abilities/effects/mana/DynamicManaEffect.java b/Mage/src/main/java/mage/abilities/effects/mana/DynamicManaEffect.java index 25e4e56003e..0e14cfc2d63 100644 --- a/Mage/src/main/java/mage/abilities/effects/mana/DynamicManaEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/mana/DynamicManaEffect.java @@ -159,7 +159,7 @@ public class DynamicManaEffect extends ManaEffect { manaStrings.add("B"); manaStrings.add("R"); manaStrings.add("G"); - List choices = controller.getMultiAmount(this.outcome, manaStrings, 0, count, MultiAmountType.MANA, game); + List choices = controller.getMultiAmount(this.outcome, manaStrings, 0, count, count, MultiAmountType.MANA, game); computedMana.add(new Mana(choices.get(0), choices.get(1), choices.get(2), choices.get(3), choices.get(4), 0, 0, 0)); } } diff --git a/Mage/src/main/java/mage/constants/MultiAmountType.java b/Mage/src/main/java/mage/constants/MultiAmountType.java index f45cb363012..ed5b1d5b34b 100644 --- a/Mage/src/main/java/mage/constants/MultiAmountType.java +++ b/Mage/src/main/java/mage/constants/MultiAmountType.java @@ -140,7 +140,7 @@ public enum MultiAmountType { return res; } - public static boolean isGoodValues(List values, List constraints, int min, int max) { + public static boolean isGoodValues(List values, List constraints, int totalMin, int totalMax) { if (values.size() != constraints.size()) { return false; } @@ -156,7 +156,7 @@ public enum MultiAmountType { currentSum += value; } - return currentSum >= min && currentSum <= max; + return currentSum >= totalMin && currentSum <= totalMax; } public static List parseAnswer(String answerToParse, List constraints, int min, diff --git a/Mage/src/main/java/mage/players/Player.java b/Mage/src/main/java/mage/players/Player.java index d31f51d849d..b147806a0e3 100644 --- a/Mage/src/main/java/mage/players/Player.java +++ b/Mage/src/main/java/mage/players/Player.java @@ -413,7 +413,7 @@ public interface Player extends MageItem, Copyable { /** * Draw cards. If you call it in replace events then use method with event param instead (for appliedEffects) * - * @param num cards to draw + * @param num cards to draw * @param source can be null for game default draws (non effects, example: start of the turn) * @return number of cards drawn, including as a result of replacement effects */ @@ -422,7 +422,7 @@ public interface Player extends MageItem, Copyable { /** * Draw cards with applied effects, for replaceEvent * - * @param num cards to draw + * @param num cards to draw * @param source can be null for game default draws (non effects, example: start of the turn) * @param event original draw event in replacement code * @return number of cards drawn, including as a result of replacement effects @@ -760,26 +760,28 @@ public interface Player extends MageItem, Copyable { */ UUID chooseBlockerOrder(List blockers, CombatGroup combatGroup, List blockerOrder, Game game); - void assignDamage(int damage, List targets, String singleTargetName, UUID attackerId, Ability source, Game game); - int getAmount(int min, int max, String message, Game game); /** * Player distributes amount among multiple options * - * @param outcome AI hint - * @param messages List of options to distribute amount among - * @param min Minimum value per option - * @param max Total amount to be distributed - * @param type MultiAmountType enum to set dialog options such as title and header - * @param game Game + * @param outcome AI hint + * @param messages List of options to distribute amount among + * @param optionMin Minimum value per option + * @param totalMin Minimum total amount to be distributed + * @param totalMax Total amount to be distributed + * @param type MultiAmountType enum to set dialog options such as title and header + * @param game Game * @return List of integers with size equal to messages.size(). The sum of the integers is equal to max. */ - default List getMultiAmount(Outcome outcome, List messages, int min, int max, MultiAmountType type, Game game) { + default List getMultiAmount(Outcome outcome, List messages, int optionMin, int totalMin, int totalMax, MultiAmountType type, Game game) { + if (optionMin > totalMax || optionMin * messages.size() > totalMin) { + throw new IllegalArgumentException(String.format("Wrong code usage: getMultiAmount found bad option min/max values: %d/%d", optionMin, totalMax)); + } List constraints = messages.stream() - .map(s -> new MultiAmountMessage(s, min, max)) + .map(s -> new MultiAmountMessage(s, optionMin, totalMax)) .collect(Collectors.toList()); - return getMultiAmountWithIndividualConstraints(outcome, constraints, min, max, type, game); + return getMultiAmountWithIndividualConstraints(outcome, constraints, totalMin, totalMax, type, game); } /** @@ -787,13 +789,13 @@ public interface Player extends MageItem, Copyable { * * @param outcome AI hint * @param messages List of options to distribute amount among. Each option has a constraint on the min, max chosen for it - * @param min Total minimum amount to be distributed - * @param max Total amount to be distributed + * @param totalMin Total minimum amount to be distributed + * @param totalMax Total amount to be distributed * @param type MultiAmountType enum to set dialog options such as title and header * @param game Game * @return List of integers with size equal to messages.size(). The sum of the integers is equal to max. */ - List getMultiAmountWithIndividualConstraints(Outcome outcome, List messages, int min, int max, MultiAmountType type, Game game); + List getMultiAmountWithIndividualConstraints(Outcome outcome, List messages, int totalMin, int totalMax, MultiAmountType type, Game game); void sideboard(Match match, Deck deck); diff --git a/Mage/src/main/java/mage/players/StubPlayer.java b/Mage/src/main/java/mage/players/StubPlayer.java index 7ddc102e262..d1fa0f7d57b 100644 --- a/Mage/src/main/java/mage/players/StubPlayer.java +++ b/Mage/src/main/java/mage/players/StubPlayer.java @@ -200,11 +200,6 @@ public class StubPlayer extends PlayerImpl { return null; } - @Override - public void assignDamage(int damage, List targets, String singleTargetName, UUID attackerId, Ability source, Game game) { - - } - @Override public int getAmount(int min, int max, String message, Game game) { return 0; @@ -212,7 +207,7 @@ public class StubPlayer extends PlayerImpl { @Override public List getMultiAmountWithIndividualConstraints(Outcome outcome, List messages, - int min, int max, MultiAmountType type, Game game) { + int totalMin, int totalMax, MultiAmountType type, Game game) { return null; }