Cleanup activated ability constructors (#11283)

* simplify LoyaltyAbility constructors

* remove unused constructors in ActivatedAbilityImpl

* cleanup

* remove null rule setting

* another unused constructor

* fix escape ability reminder text

* escape text adjustment

* simplify Trigons, remove Costs constructor

* rework Villainous Ogre, remove another class and constructor

* fix test using wrong text for trigon ability

Mage/src/main/java/mage/abilities/ActivatedAbilityImpl.java

@@ -4,10 +4,8 @@ import mage.ApprovingObject;
 import mage.MageObject;
 import mage.abilities.condition.Condition;
 import mage.abilities.costs.Cost;
-import mage.abilities.costs.Costs;
 import mage.abilities.costs.mana.ManaCosts;
 import mage.abilities.effects.Effect;
-import mage.abilities.effects.Effects;
 import mage.abilities.mana.ManaOptions;
 import mage.cards.Card;
 import mage.constants.*;
@@ -59,71 +57,23 @@ public abstract class ActivatedAbilityImpl extends AbilityImpl implements Activa
         condition = ability.condition;
     }
 
-    public ActivatedAbilityImpl(Zone zone) {
-        this(zone, null);
-    }
-
-    public ActivatedAbilityImpl(Zone zone, Effect effect) {
+    protected ActivatedAbilityImpl(Zone zone, Effect effect) {
         super(AbilityType.ACTIVATED, zone);
         this.addEffect(effect);
     }
 
-    public ActivatedAbilityImpl(Zone zone, Effect effect, ManaCosts cost) {
+    protected ActivatedAbilityImpl(Zone zone, Effect effect, ManaCosts cost) {
         super(AbilityType.ACTIVATED, zone);
         this.addEffect(effect);
         this.addManaCost(cost);
     }
 
-    public ActivatedAbilityImpl(Zone zone, Effects effects, ManaCosts cost) {
-        super(AbilityType.ACTIVATED, zone);
-        if (effects != null) {
-            for (Effect effect : effects) {
-                this.addEffect(effect);
-            }
-        }
-        this.addManaCost(cost);
-    }
-
-    public ActivatedAbilityImpl(Zone zone, Effect effect, Cost cost) {
+    protected ActivatedAbilityImpl(Zone zone, Effect effect, Cost cost) {
         super(AbilityType.ACTIVATED, zone);
         this.addEffect(effect);
         this.addCost(cost);
     }
 
-    public ActivatedAbilityImpl(Zone zone, Effect effect, Costs<Cost> costs) {
-        super(AbilityType.ACTIVATED, zone);
-        this.addEffect(effect);
-        if (costs != null) {
-            for (Cost cost : costs) {
-                this.addCost(cost);
-            }
-        }
-    }
-
-    public ActivatedAbilityImpl(Zone zone, Effects effects, Cost cost) {
-        super(AbilityType.ACTIVATED, zone);
-        if (effects != null) {
-            for (Effect effect : effects) {
-                this.addEffect(effect);
-            }
-        }
-        this.addCost(cost);
-    }
-
-    public ActivatedAbilityImpl(Zone zone, Effects effects, Costs<Cost> costs) {
-        super(AbilityType.ACTIVATED, zone);
-        if (effects != null) {
-            for (Effect effect : effects) {
-                this.addEffect(effect);
-            }
-        }
-        if (costs != null) {
-            for (Cost cost : costs) {
-                this.addCost(cost);
-            }
-        }
-    }
-
     @Override
     public abstract ActivatedAbilityImpl copy();
 

Mage/src/main/java/mage/abilities/LoyaltyAbility.java

@@ -4,7 +4,6 @@ import mage.abilities.costs.Cost;
 import mage.abilities.costs.common.PayLoyaltyCost;
 import mage.abilities.costs.common.PayVariableLoyaltyCost;
 import mage.abilities.effects.Effect;
-import mage.abilities.effects.Effects;
 import mage.constants.TimingRule;
 import mage.constants.Zone;
 
@@ -18,21 +17,11 @@ public class LoyaltyAbility extends ActivatedAbilityImpl {
         this.timing = TimingRule.SORCERY;
     }
 
-    public LoyaltyAbility(Effects effects, int loyalty) {
-        super(Zone.BATTLEFIELD, effects, new PayLoyaltyCost(loyalty));
-        this.timing = TimingRule.SORCERY;
-    }
-
     public LoyaltyAbility(Effect effect) {
         super(Zone.BATTLEFIELD, effect, new PayVariableLoyaltyCost());
         this.timing = TimingRule.SORCERY;
     }
 
-    public LoyaltyAbility(Effects effects) {
-        super(Zone.BATTLEFIELD, effects, new PayVariableLoyaltyCost());
-        this.timing = TimingRule.SORCERY;
-    }
-
     protected LoyaltyAbility(final LoyaltyAbility ability) {
         super(ability);
     }

Mage/src/main/java/mage/abilities/common/ActivateIfConditionActivatedAbility.java

@@ -1,4 +1,3 @@
-
 package mage.abilities.common;
 
 import mage.abilities.ActivatedAbilityImpl;
@@ -8,12 +7,12 @@ import mage.abilities.costs.Cost;
 import mage.abilities.effects.Effect;
 import mage.constants.TimingRule;
 import mage.constants.Zone;
-import mage.game.Game;
 
 /**
  * @author LevelX2
  */
 public class ActivateIfConditionActivatedAbility extends ActivatedAbilityImpl {
+
     public ActivateIfConditionActivatedAbility(Zone zone, Effect effect, Cost cost, Condition condition) {
         this(zone, effect, cost, condition, TimingRule.INSTANT);
     }
@@ -28,11 +27,6 @@ public class ActivateIfConditionActivatedAbility extends ActivatedAbilityImpl {
         super(ability);
     }
 
-    @Override
-    public boolean resolve(Game game) {
-        return super.resolve(game);
-    }
-
     @Override
     public String getRule() {
         StringBuilder sb = new StringBuilder(super.getRule());

Mage/src/main/java/mage/abilities/common/EscapesWithAbility.java

@@ -16,7 +16,6 @@ import mage.game.Game;
 import mage.game.permanent.Permanent;
 import mage.util.CardUtil;
 
-import java.util.ArrayList;
 import java.util.List;
 import java.util.UUID;
 
@@ -59,11 +58,10 @@ public class EscapesWithAbility extends EntersBattlefieldAbility {
             sb.append(" on it.");
         }
 
-        if (triggeredAbility == null) {
-            // Do nothing
-        } else if (triggeredAbility instanceof DelayedTriggeredAbility) {
+        if (triggeredAbility instanceof DelayedTriggeredAbility) {
+            sb.append(" ");
             sb.append(this.triggeredAbility.getRule());
-        } else {
+        } else if (triggeredAbility != null) {
             sb.append("\"");
             sb.append(this.triggeredAbility.getRule());
             sb.append("\"");
@@ -107,7 +105,7 @@ class EscapesWithEffect extends OneShotEffect {
         }
 
         if (counter > 0) {
-            List<UUID> appliedEffects = (ArrayList<UUID>) this.getValue("appliedEffects");
+            List<UUID> appliedEffects = (List<UUID>) this.getValue("appliedEffects");
             permanent.addCounters(CounterType.P1P1.createInstance(counter), source.getControllerId(), source, game, appliedEffects);
         }
 

Mage/src/main/java/mage/abilities/common/LimitedTimesPerTurnActivatedAbility.java

@@ -6,7 +6,6 @@ import mage.abilities.costs.Cost;
 import mage.abilities.effects.Effect;
 import mage.constants.TimingRule;
 import mage.constants.Zone;
-import mage.game.Game;
 import mage.util.CardUtil;
 
 /**
@@ -35,11 +34,6 @@ public class LimitedTimesPerTurnActivatedAbility extends ActivatedAbilityImpl {
         this.condition = ability.condition;
     }
 
-    @Override
-    public boolean resolve(Game game) {
-        return super.resolve(game);
-    }
-
     @Override
     public String getRule() {
         StringBuilder sb = new StringBuilder(super.getRule()).append(" Activate ");

Mage/src/main/java/mage/abilities/common/SimpleActivatedAbility.java

@@ -1,10 +1,8 @@
-
 package mage.abilities.common;
 
 import mage.constants.Zone;
 import mage.abilities.ActivatedAbilityImpl;
 import mage.abilities.costs.Cost;
-import mage.abilities.costs.Costs;
 import mage.abilities.costs.mana.ManaCosts;
 import mage.abilities.effects.Effect;
 
@@ -26,15 +24,11 @@ public class SimpleActivatedAbility extends ActivatedAbilityImpl {
         super(zone, effect, cost);
     }
 
-    public SimpleActivatedAbility(Zone zone, Effect effect, Costs<Cost> costs) {
-        super(zone, effect, costs);
-    }
-
     public SimpleActivatedAbility(Zone zone, Effect effect, Cost cost) {
         super(zone, effect, cost);
     }
 
-    protected SimpleActivatedAbility(SimpleActivatedAbility ability) {
+    protected SimpleActivatedAbility(final SimpleActivatedAbility ability) {
         super(ability);
     }
 

Mage/src/main/java/mage/abilities/decorator/ConditionalActivatedAbility.java

@@ -3,7 +3,6 @@ package mage.abilities.decorator;
 import mage.abilities.ActivatedAbilityImpl;
 import mage.abilities.condition.Condition;
 import mage.abilities.costs.Cost;
-import mage.abilities.costs.Costs;
 import mage.abilities.costs.mana.ManaCosts;
 import mage.abilities.effects.Effect;
 import mage.abilities.effects.Effects;
@@ -36,12 +35,6 @@ public class ConditionalActivatedAbility extends ActivatedAbilityImpl {
         this.ruleText = rule;
     }
 
-    public ConditionalActivatedAbility(Zone zone, Effect effect, Costs<Cost> costs, Condition condition, String rule) {
-        super(zone, effect, costs);
-        this.condition = condition;
-        this.ruleText = rule;
-    }
-
     public ConditionalActivatedAbility(Zone zone, Effect effect, Cost cost, Condition condition, String rule) {
         super(zone, effect, cost);
         this.condition = condition;
@@ -80,7 +73,6 @@ public class ConditionalActivatedAbility extends ActivatedAbilityImpl {
         if (conditionText.startsWith("during")
                 || conditionText.startsWith("before")
                 || conditionText.startsWith("if")) {
-            sb.append("");
         } else {
             sb.append("if ");
         }

Mage/src/main/java/mage/abilities/decorator/ConditionalGainActivatedAbility.java

@@ -1,75 +0,0 @@
-
-package mage.abilities.decorator;
-
-import java.util.UUID;
-import mage.abilities.ActivatedAbilityImpl;
-import mage.abilities.condition.Condition;
-import mage.abilities.costs.Cost;
-import mage.abilities.costs.Costs;
-import mage.abilities.costs.mana.ManaCosts;
-import mage.abilities.effects.Effect;
-import mage.abilities.effects.Effects;
-import mage.constants.EffectType;
-import mage.constants.Zone;
-import mage.game.Game;
-
-/**
- * The card / permanent has the ability only, if the condition is true.
- *
- * @author LevelX
- */
-public class ConditionalGainActivatedAbility extends ActivatedAbilityImpl {
-
-    private String staticText = "";
-
-    private static final Effects emptyEffects = new Effects();
-
-    public ConditionalGainActivatedAbility(Zone zone, Effect effect, ManaCosts cost, Condition condition, String rule) {
-        super(zone, effect, cost);
-        this.condition = condition;
-        this.staticText = rule;
-    }
-
-    public ConditionalGainActivatedAbility(Zone zone, Effect effect, Costs costs, Condition condition, String rule) {
-        super(zone, effect, costs);
-        this.condition = condition;
-        this.staticText = rule;
-    }
-
-    public ConditionalGainActivatedAbility(Zone zone, Effect effect, Cost cost, Condition condition, String rule) {
-        super(zone, effect, cost);
-        this.condition = condition;
-        this.staticText = rule;
-    }
-
-    public ConditionalGainActivatedAbility(ConditionalGainActivatedAbility ability) {
-        super(ability);
-        this.staticText = ability.staticText;
-    }
-
-    @Override
-    public Effects getEffects(Game game, EffectType effectType) {
-        if (!condition.apply(game, this)) {
-            return emptyEffects;
-        }
-        return super.getEffects(game, effectType);
-    }
-
-    @Override
-    public ActivationStatus canActivate(UUID playerId, Game game) {
-        if (!condition.apply(game, this)) {
-            return ActivationStatus.getFalse();
-        }
-        return super.canActivate(playerId, game);
-    }
-
-    @Override
-    public ConditionalGainActivatedAbility copy() {
-        return new ConditionalGainActivatedAbility(this);
-    }
-
-    @Override
-    public String getRule() {
-        return staticText;
-    }
-}

Mage/src/main/java/mage/abilities/keyword/EscapeAbility.java

@@ -53,7 +53,7 @@ public class EscapeAbility extends SpellAbility {
         }
 
         text += ", Exile " + CardUtil.numberToText(exileCount) + " other cards from your graveyard."
-                + "<i>(You may cast this card from your graveyard for its escape cost.)</i>";
+                + " <i>(You may cast this card from your graveyard for its escape cost.)</i>";
         this.addCost(new ExileFromGraveCost(new TargetCardInYourGraveyard(exileCount, filter), "")); // hide additional cost text from rules
 
         this.staticText = text;