refactor: removed unused data from special action, improved code (rel…

…ated to #11137)
magefree · Sep 17, 2023 · 4e77ccb · 4e77ccb
1 parent a4daad1
commit 4e77ccb
Show file tree

Hide file tree

Showing 7 changed files with 31 additions and 38 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
@@ -1681,20 +1681,21 @@ protected boolean playManaHandling(Ability ability, ManaCost unpaid, final Game
         // pay special mana like convoke cost (tap for pay)
         // GUI: user see "special" button while pay spell's cost
         // TODO: AI can't prioritize special mana types to pay, e.g. it will use first available
-        SpecialAction specialAction = game.getState().getSpecialActions().getControlledBy(this.getId(), true)
-                .values().stream().findFirst().orElse(null);
+        SpecialAction specialAction = game.getState().getSpecialActions().getControlledBy(this.getId(), true).values()
+                .stream()
+                .findFirst()
+                .orElse(null);
         ManaOptions specialMana = specialAction == null ? null : specialAction.getManaOptions(ability, game, unpaid);
         if (specialMana != null) {
             for (Mana netMana : specialMana) {
                 if (cost.testPay(netMana) || approvingObject != null) {
                     if (netMana instanceof ConditionalMana && !((ConditionalMana) netMana).apply(ability, game, getId(), cost)) {
                         continue;
                     }
-                    specialAction.setUnpaidMana(unpaid);
                     if (activateAbility(specialAction, game)) {
                         return true;
                     }
-                    // only one time try to pay
+                    // only one time try to pay to skip infinite AI loop
                     break;
                 }
             }

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
@@ -41,12 +41,7 @@
 import mage.target.common.TargetAnyTarget;
 import mage.target.common.TargetAttackingCreature;
 import mage.target.common.TargetDefender;
-import mage.util.CardUtil;
-import mage.util.GameLog;
-import mage.util.ManaUtil;
-import mage.util.MessageToClient;
-import mage.util.MultiAmountMessage;
-
+import mage.util.*;
 import org.apache.log4j.Logger;
 
 import java.awt.*;
@@ -2058,8 +2053,14 @@ public int getAmount(int min, int max, String message, Game game) {
     }
 
     @Override
-    public List<Integer> getMultiAmountWithIndividualConstraints(Outcome outcome, List<MultiAmountMessage> messages,
-            int min, int max, MultiAmountType type, Game game) {
+    public List<Integer> getMultiAmountWithIndividualConstraints(
+            Outcome outcome,
+            List<MultiAmountMessage> messages,
+            int min,
+            int max,
+            MultiAmountType type,
+            Game game
+    ) {
         int needCount = messages.size();
         List<Integer> defaultList = MultiAmountType.prepareDefaltValues(messages, min, max);
         if (needCount == 0 || (needCount == 1 && min == max)
@@ -2146,14 +2147,9 @@ protected void activateSpecialAction(Game game, ManaCost unpaidForManaAction) {
             waitForResponse(game);
 
             UUID responseId = getFixedResponseUUID(game);
-            if (responseId != null) {
-                if (specialActions.containsKey(responseId)) {
-                    SpecialAction specialAction = specialActions.get(responseId);
-                    if (unpaidForManaAction != null) {
-                        specialAction.setUnpaidMana(unpaidForManaAction);
-                    }
-                    activateAbility(specialAction, game);
-                }
+            SpecialAction specialAction = specialActions.getOrDefault(responseId, null);
+            if (specialAction != null) {
+                activateAbility(specialAction, game);
             }
         }
     }

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
@@ -4294,10 +4294,11 @@ public boolean playMana(Ability ability, ManaCost unpaid,
                             if (specialAction.canActivate(this.getId(), game).canActivate()) {
                                 choices.remove(0);
                                 choiceRemoved = true;
-                                specialAction.setUnpaidMana(unpaid);
                                 if (activateAbility(specialAction, game)) {
                                     choiceUsed = true;
                                 }
+                            } else {
+                                Assert.fail("Found non active special mana action, but must generates only active: " + specialAction.getRule(true));
                             }
                         }
                     }

diff --git a/Mage/src/main/java/mage/abilities/SpecialAction.java b/Mage/src/main/java/mage/abilities/SpecialAction.java
@@ -17,7 +17,6 @@
 public abstract class SpecialAction extends ActivatedAbilityImpl {
 
     private final AlternateManaPaymentAbility manaAbility; // mana actions generates on every pay cycle, no need to copy it
-    protected ManaCost unpaidMana;
 
     public SpecialAction() {
         this(Zone.ALL);
@@ -35,22 +34,13 @@ public SpecialAction(Zone zone, AlternateManaPaymentAbility manaAbility) {
 
     protected SpecialAction(final SpecialAction action) {
         super(action);
-        this.unpaidMana = action.unpaidMana;
         this.manaAbility = action.manaAbility;
     }
 
     public boolean isManaAction() {
         return manaAbility != null;
     }
 
-    public void setUnpaidMana(ManaCost manaCost) {
-        this.unpaidMana = manaCost;
-    }
-
-    public ManaCost getUnpaidMana() {
-        return unpaidMana;
-    }
-
     public ManaOptions getManaOptions(Ability source, Game game, ManaCost unpaid) {
         if (manaAbility != null) {
             return manaAbility.getManaOptions(source, game, unpaid);

diff --git a/Mage/src/main/java/mage/abilities/SpecialActions.java b/Mage/src/main/java/mage/abilities/SpecialActions.java
@@ -1,12 +1,16 @@
-
-
 package mage.abilities;
 
 import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.UUID;
 
 /**
+ * Special actions to activate at any priority time (GUI has special button to show a special commands list)
+ * <p>
+ * Two types of action:
+ * - mana actions (auto-generated on each mana pay cycle, auto-clean)
+ * - another actions (manual added, manual removed - like one short effects)
+ *
  * @author BetaSteward_at_googlemail.com
  */
 public class SpecialActions extends AbilitiesImpl<SpecialAction> {

diff --git a/Mage/src/main/java/mage/abilities/effects/common/CreateSpecialActionEffect.java b/Mage/src/main/java/mage/abilities/effects/common/CreateSpecialActionEffect.java
@@ -14,22 +14,22 @@
 public class CreateSpecialActionEffect extends OneShotEffect {
 
     private final SpecialAction action;
-    private final UUID playerId; // If set, that player can activate the special action. If null, use the source controller instead.
+    private final UUID newActionControllerId; // another player can activate the special action
 
     public CreateSpecialActionEffect(SpecialAction action) {
         this(action, null);
     }
 
-    public CreateSpecialActionEffect(SpecialAction action, UUID playerId) {
+    public CreateSpecialActionEffect(SpecialAction action, UUID newActionControllerId) {
         super(action.getEffects().getOutcome(action));
         this.action = action;
-        this.playerId = playerId;
+        this.newActionControllerId = newActionControllerId;
     }
 
     protected CreateSpecialActionEffect(final CreateSpecialActionEffect effect) {
         super(effect);
         this.action = (SpecialAction) effect.action.copy();
-        this.playerId = effect.playerId;
+        this.newActionControllerId = effect.newActionControllerId;
     }
 
     @Override
@@ -41,7 +41,7 @@ public CreateSpecialActionEffect copy() {
     public boolean apply(Game game, Ability source) {
         SpecialAction newAction = (SpecialAction) action.copy();
         newAction.setSourceId(source.getSourceId());
-        newAction.setControllerId(playerId == null ? source.getControllerId() : playerId);
+        newAction.setControllerId(newActionControllerId != null ? newActionControllerId : source.getControllerId());
         newAction.getTargets().addAll(source.getTargets());
         game.getState().getSpecialActions().add(newAction);
         return true;

diff --git a/Mage/src/main/java/mage/abilities/keyword/ConvokeAbility.java b/Mage/src/main/java/mage/abilities/keyword/ConvokeAbility.java
@@ -228,6 +228,7 @@ public boolean apply(Game game, Ability source) {
                         if (chooseManaType.getChoices().size() > 1) {
                             chooseManaType.getChoices().add("Colorless");
                             chooseManaType.setMessage("Choose mana color to reduce from " + perm.getName());
+                            // TODO: must be AI optimization to pay most rare mana color first
                             if (!controller.choose(Outcome.Benefit, chooseManaType, game)) {
                                 return false;
                             }