From 23a414e0740d50718495e5642379fc825983a027 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Mon, 30 Dec 2024 21:33:54 +0400 Subject: [PATCH] refactor, combat: improved declare blockers code, added docs, added additional runtime checks for AI, added debug info --- .../main/java/mage/game/combat/Combat.java | 72 ++++++++++++++----- .../java/mage/game/combat/CombatGroup.java | 8 +++ 2 files changed, 63 insertions(+), 17 deletions(-) diff --git a/Mage/src/main/java/mage/game/combat/Combat.java b/Mage/src/main/java/mage/game/combat/Combat.java index 7137f686c7b..b62a906a966 100644 --- a/Mage/src/main/java/mage/game/combat/Combat.java +++ b/Mage/src/main/java/mage/game/combat/Combat.java @@ -663,26 +663,47 @@ public class Combat implements Serializable, Copyable { if (defender == null) { continue; } - boolean choose = true; if (blockController == null) { controller = defender; } else { controller = blockController; } - while (choose) { + + // choosing until good block configuration + while (true) { + // declare normal blockers + // TODO: need reseach - is it possible to concede on bad blocker configuration (e.g. user can't continue) controller.selectBlockers(source, game, defenderId); if (game.isPaused() || game.checkIfGameIsOver() || game.executingRollback()) { return; } - if (!game.getCombat().checkBlockRestrictions(defender, game)) { - if (controller.isHuman()) { // only human player can decide to do the block in another way - continue; - } + + // check multiple restrictions by permanents and effects, reset on invalid blocking configuration, try to auto-fix + // TODO: wtf, some checks contains AI related code inside -- it must be reworked and moved to computer classes?! + + // check 1 of 3 + boolean isValidBlock = game.getCombat().checkBlockRestrictions(defender, game); + if (!isValidBlock) { + makeSureItsNotComputer(controller); + continue; } - choose = !game.getCombat().checkBlockRequirementsAfter(defender, controller, game); - if (!choose) { - choose = !game.getCombat().checkBlockRestrictionsAfter(defender, controller, game); + + // check 2 of 3 + isValidBlock = game.getCombat().checkBlockRequirementsAfter(defender, controller, game); + if (!isValidBlock) { + makeSureItsNotComputer(controller); + continue; } + + // check 3 of 3 + isValidBlock = game.getCombat().checkBlockRestrictionsAfter(defender, controller, game); + if (!isValidBlock) { + makeSureItsNotComputer(controller); + continue; + } + + // all valid, can finish now + break; } game.fireEvent(GameEvent.getEvent(GameEvent.EventType.DECLARED_BLOCKERS, defenderId, defenderId)); @@ -695,6 +716,15 @@ public class Combat implements Serializable, Copyable { TraceUtil.traceCombatIfNeeded(game, game.getCombat()); } + private void makeSureItsNotComputer(Player controller) { + if (controller.isComputer() || !controller.isHuman()) { + // TODO: wtf, AI will freeze forever here in games with attacker/blocker restrictions, + // but it pass in some use cases due random choices. AI must deside blocker configuration + // in one attempt + throw new IllegalStateException("AI can't find good blocker configuration, report it to github"); + } + } + /** * Add info about attacker blocked by blocker to the game log */ @@ -852,10 +882,7 @@ public class Combat implements Serializable, Copyable { * attacking creature fulfills both the restriction and the requirement, so * that's the only option. * - * @param player - * @param controller - * @param game - * @return + * @return false on invalid block configuration e.g. player must choose new blockers */ public boolean checkBlockRequirementsAfter(Player player, Player controller, Game game) { // Get once a list of all opponents in range @@ -1274,10 +1301,7 @@ public class Combat implements Serializable, Copyable { * Checks the canBeBlockedCheckAfter RestrictionEffect Is the block still * valid after all block decisions are done * - * @param player - * @param controller - * @param game - * @return + * @return false on invalid block configuration e.g. player must choose new blockers */ public boolean checkBlockRestrictionsAfter(Player player, Player controller, Game game) { // Restrictions applied to blocking creatures @@ -1906,4 +1930,18 @@ public class Combat implements Serializable, Copyable { return new Combat(this); } + @Override + public String toString() { + List res = new ArrayList<>(); + for (int i = 0; i < this.groups.size(); i++) { + res.add(String.format("group %d with %s", + i + 1, + this.groups.get(i) + )); + } + return String.format("%d groups%s", + this.groups.size(), + this.groups.size() > 0 ? ": " + String.join("; ", res) : "" + ); + } } diff --git a/Mage/src/main/java/mage/game/combat/CombatGroup.java b/Mage/src/main/java/mage/game/combat/CombatGroup.java index 9106b2d4abc..c5b5a9cb677 100644 --- a/Mage/src/main/java/mage/game/combat/CombatGroup.java +++ b/Mage/src/main/java/mage/game/combat/CombatGroup.java @@ -976,4 +976,12 @@ public class CombatGroup implements Serializable, Copyable { private static int getLethalDamage(Permanent blocker, Permanent attacker, Game game) { return blocker.getLethalDamage(attacker.getId(), game); } + + @Override + public String toString() { + return String.format("%d attackers, %d blockers", + this.getAttackers().size(), + this.getBlockers().size() + ); + } }