diff --git a/Mage.Sets/src/mage/cards/a/AnuridScavenger.java b/Mage.Sets/src/mage/cards/a/AnuridScavenger.java index 84a9ee660a3..f51f7e1f831 100644 --- a/Mage.Sets/src/mage/cards/a/AnuridScavenger.java +++ b/Mage.Sets/src/mage/cards/a/AnuridScavenger.java @@ -69,8 +69,8 @@ class AnuridScavengerCost extends CostImpl { public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { Player controller = game.getPlayer(controllerId); if (controller != null) { - if (targets.choose(Outcome.Removal, controllerId, source.getSourceId(), source, game)) { - for (UUID targetId: targets.get(0).getTargets()) { + if (this.getTargets().choose(Outcome.Removal, controllerId, source.getSourceId(), source, game)) { + for (UUID targetId: this.getTargets().get(0).getTargets()) { Card card = game.getCard(targetId); if (card == null || game.getState().getZone(targetId) != Zone.GRAVEYARD) { return false; @@ -85,7 +85,7 @@ class AnuridScavengerCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage.Sets/src/mage/cards/a/ArdentDustspeaker.java b/Mage.Sets/src/mage/cards/a/ArdentDustspeaker.java index 4d43126ee77..741ed7974a6 100644 --- a/Mage.Sets/src/mage/cards/a/ArdentDustspeaker.java +++ b/Mage.Sets/src/mage/cards/a/ArdentDustspeaker.java @@ -79,15 +79,15 @@ class ArdentDustspeakerCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { Player controller = game.getPlayer(controllerId); if (controller != null) { - if (controller.chooseTarget(Outcome.Benefit, targets.get(0), source, game)) { - Card card = game.getCard(targets.get(0).getFirstTarget()); + if (controller.chooseTarget(Outcome.Benefit, this.getTargets().get(0), source, game)) { + Card card = game.getCard(this.getTargets().get(0).getFirstTarget()); if (card != null) { controller.putCardsOnBottomOfLibrary(card, game, source, true); paid = true; diff --git a/Mage.Sets/src/mage/cards/b/BackFromTheBrink.java b/Mage.Sets/src/mage/cards/b/BackFromTheBrink.java index a85f7e2eaa9..7e720d5c3c2 100644 --- a/Mage.Sets/src/mage/cards/b/BackFromTheBrink.java +++ b/Mage.Sets/src/mage/cards/b/BackFromTheBrink.java @@ -67,15 +67,15 @@ class BackFromTheBrinkCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { - if (targets.choose(Outcome.Exile, controllerId, source.getSourceId(), source, game)) { + if (this.getTargets().choose(Outcome.Exile, controllerId, source.getSourceId(), source, game)) { Player controller = game.getPlayer(controllerId); if (controller != null) { - Card card = controller.getGraveyard().get(targets.getFirstTarget(), game); + Card card = controller.getGraveyard().get(this.getTargets().getFirstTarget(), game); if (card != null && controller.moveCards(card, Zone.EXILED, ability, game)) { ability.getEffects().get(0).setTargetPointer(new FixedTarget(card.getId(), game.getState().getZoneChangeCounter(card.getId()))); paid = card.getManaCost().pay(ability, game, source, controllerId, noMana); diff --git a/Mage.Sets/src/mage/cards/b/BattlefieldScrounger.java b/Mage.Sets/src/mage/cards/b/BattlefieldScrounger.java index a4c07527c6a..12a98470e52 100644 --- a/Mage.Sets/src/mage/cards/b/BattlefieldScrounger.java +++ b/Mage.Sets/src/mage/cards/b/BattlefieldScrounger.java @@ -68,8 +68,8 @@ class BattlefieldScroungerCost extends CostImpl { public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { Player controller = game.getPlayer(controllerId); if (controller != null) { - if (targets.choose(Outcome.Removal, controllerId, source.getSourceId(), source, game)) { - for (UUID targetId: targets.get(0).getTargets()) { + if (this.getTargets().choose(Outcome.Removal, controllerId, source.getSourceId(), source, game)) { + for (UUID targetId: this.getTargets().get(0).getTargets()) { Card card = game.getCard(targetId); if (card == null || game.getState().getZone(targetId) != Zone.GRAVEYARD) { return false; @@ -84,7 +84,7 @@ class BattlefieldScroungerCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage.Sets/src/mage/cards/d/DragonsFire.java b/Mage.Sets/src/mage/cards/d/DragonsFire.java index af5f6e0d073..1bf5a983e9b 100644 --- a/Mage.Sets/src/mage/cards/d/DragonsFire.java +++ b/Mage.Sets/src/mage/cards/d/DragonsFire.java @@ -90,7 +90,7 @@ class DragonsFireCost extends CostImpl { @Override public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { - this.targets.clear(); + this.getTargets().clear(); dragonZone = DragonZone.NONE; selectedCardId = null; Player controller = game.getPlayer(controllerId); @@ -132,23 +132,23 @@ class DragonsFireCost extends CostImpl { } switch (chosenZone) { case HAND: - targets.add(new TargetCardInHand(handFilter)); - if (targets.choose(Outcome.Benefit, controllerId, source.getSourceId(), source, game)) { - Card card = game.getCard(targets.getFirstTarget()); + this.getTargets().add(new TargetCardInHand(handFilter)); + if (this.getTargets().choose(Outcome.Benefit, controllerId, source.getSourceId(), source, game)) { + Card card = game.getCard(this.getTargets().getFirstTarget()); if (card != null) { dragonZone = DragonZone.HAND; - selectedCardId = targets.getFirstTarget(); + selectedCardId = this.getTargets().getFirstTarget(); controller.revealCards(source, new CardsImpl(card), game); } } break; case BATTLEFIELD: - targets.add(new TargetControlledPermanent(battlefieldFilter)); - if (targets.choose(Outcome.Benefit, controllerId, source.getSourceId(), source, game)) { - Permanent permanent = game.getPermanent(targets.getFirstTarget()); + this.getTargets().add(new TargetControlledPermanent(battlefieldFilter)); + if (this.getTargets().choose(Outcome.Benefit, controllerId, source.getSourceId(), source, game)) { + Permanent permanent = game.getPermanent(this.getTargets().getFirstTarget()); if (permanent != null) { dragonZone = DragonZone.BATTLEFIELD; - selectedCardId = targets.getFirstTarget(); + selectedCardId = this.getTargets().getFirstTarget(); game.informPlayers(controller.getLogName() + " chooses " + permanent.getLogName()); } } diff --git a/Mage.Sets/src/mage/cards/g/Gurzigost.java b/Mage.Sets/src/mage/cards/g/Gurzigost.java index 3cd8a43b887..d681e6bc28b 100644 --- a/Mage.Sets/src/mage/cards/g/Gurzigost.java +++ b/Mage.Sets/src/mage/cards/g/Gurzigost.java @@ -77,8 +77,8 @@ class GurzigostCost extends CostImpl { public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { Player controller = game.getPlayer(controllerId); if (controller != null) { - if (targets.choose(Outcome.Removal, controllerId, source.getSourceId(), source, game)) { - for (UUID targetId: targets.get(0).getTargets()) { + if (this.getTargets().choose(Outcome.Removal, controllerId, source.getSourceId(), source, game)) { + for (UUID targetId: this.getTargets().get(0).getTargets()) { Card card = game.getCard(targetId); if (card == null || game.getState().getZone(targetId) != Zone.GRAVEYARD) { return false; @@ -93,7 +93,7 @@ class GurzigostCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage.Sets/src/mage/cards/i/InfernalHarvest.java b/Mage.Sets/src/mage/cards/i/InfernalHarvest.java index 68fc94b09af..641cf103078 100644 --- a/Mage.Sets/src/mage/cards/i/InfernalHarvest.java +++ b/Mage.Sets/src/mage/cards/i/InfernalHarvest.java @@ -106,11 +106,11 @@ class InfernalHarvestAdditionalCost extends VariableCostImpl { return false; } Player player = game.getPlayer(controllerId); - if (player == null || !targets.choose(Outcome.ReturnToHand, controllerId, source.getSourceId(), source, game)) { + if (player == null || !this.getTargets().choose(Outcome.ReturnToHand, controllerId, source.getSourceId(), source, game)) { return false; } return paid = player.moveCards( - targets.stream() + this.getTargets().stream() .map(Target::getTargets) .flatMap(Collection::stream) .map(game::getCard) diff --git a/Mage.Sets/src/mage/cards/j/JotunGrunt.java b/Mage.Sets/src/mage/cards/j/JotunGrunt.java index 3003d8f799d..1d271b5065b 100644 --- a/Mage.Sets/src/mage/cards/j/JotunGrunt.java +++ b/Mage.Sets/src/mage/cards/j/JotunGrunt.java @@ -62,8 +62,8 @@ class JotunGruntCost extends CostImpl { public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { Player controller = game.getPlayer(controllerId); if (controller != null) { - if (targets.choose(Outcome.Removal, controllerId, source.getSourceId(), source, game)) { - for (UUID targetId: targets.get(0).getTargets()) { + if (this.getTargets().choose(Outcome.Removal, controllerId, source.getSourceId(), source, game)) { + for (UUID targetId: this.getTargets().get(0).getTargets()) { Card card = game.getCard(targetId); if (card == null || game.getState().getZone(targetId) != Zone.GRAVEYARD) { return false; @@ -78,7 +78,7 @@ class JotunGruntCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage.Sets/src/mage/cards/k/KozilekTheGreatDistortion.java b/Mage.Sets/src/mage/cards/k/KozilekTheGreatDistortion.java index c4efa09f696..fbfe26e0980 100644 --- a/Mage.Sets/src/mage/cards/k/KozilekTheGreatDistortion.java +++ b/Mage.Sets/src/mage/cards/k/KozilekTheGreatDistortion.java @@ -123,8 +123,8 @@ class KozilekDiscardCost extends CostImpl { TargetCardInHand target = new TargetCardInHand(filter); this.getTargets().clear(); this.getTargets().add(target); - if (targets.choose(Outcome.Discard, controllerId, source.getSourceId(), source, game)) { - for (UUID targetId : targets.get(0).getTargets()) { + if (this.getTargets().choose(Outcome.Discard, controllerId, source.getSourceId(), source, game)) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { Card card = player.getHand().get(targetId, game); if (card == null) { return false; diff --git a/Mage.Sets/src/mage/cards/m/MossbridgeTroll.java b/Mage.Sets/src/mage/cards/m/MossbridgeTroll.java index 6d515b1223f..a77f7ad3ef5 100644 --- a/Mage.Sets/src/mage/cards/m/MossbridgeTroll.java +++ b/Mage.Sets/src/mage/cards/m/MossbridgeTroll.java @@ -119,8 +119,8 @@ class MossbridgeTrollCost extends CostImpl { @Override public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { int sumPower = 0; - if (targets.choose(Outcome.Tap, controllerId, source.getSourceId(), source, game)) { - for (UUID targetId : targets.get(0).getTargets()) { + if (this.getTargets().choose(Outcome.Tap, controllerId, source.getSourceId(), source, game)) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { Permanent permanent = game.getPermanent(targetId); if (permanent != null && permanent.tap(source, game)) { sumPower += permanent.getPower().getValue(); diff --git a/Mage.Sets/src/mage/cards/n/NivmagusElemental.java b/Mage.Sets/src/mage/cards/n/NivmagusElemental.java index a24cba580b8..a602a4adec4 100644 --- a/Mage.Sets/src/mage/cards/n/NivmagusElemental.java +++ b/Mage.Sets/src/mage/cards/n/NivmagusElemental.java @@ -73,8 +73,8 @@ class NivmagusElementalCost extends CostImpl { if (player == null) { return false; } - player.chooseTarget(Outcome.Exile, targets.get(0), source, game); - Spell spell = game.getSpell(targets.getFirstTarget()); + player.chooseTarget(Outcome.Exile, this.getTargets().get(0), source, game); + Spell spell = game.getSpell(this.getTargets().getFirstTarget()); if (spell == null) { return false; } @@ -91,7 +91,7 @@ class NivmagusElementalCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage.Sets/src/mage/cards/p/PhyrexianDreadnought.java b/Mage.Sets/src/mage/cards/p/PhyrexianDreadnought.java index 4633d03e9b5..da44ececab5 100644 --- a/Mage.Sets/src/mage/cards/p/PhyrexianDreadnought.java +++ b/Mage.Sets/src/mage/cards/p/PhyrexianDreadnought.java @@ -72,8 +72,8 @@ class PhyrexianDreadnoughtSacrificeCost extends CostImpl { @Override public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { int sumPower = 0; - if (targets.choose(Outcome.Sacrifice, controllerId, source.getSourceId(), source, game)) { - for (UUID targetId : targets.get(0).getTargets()) { + if (this.getTargets().choose(Outcome.Sacrifice, controllerId, source.getSourceId(), source, game)) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { Permanent permanent = game.getPermanent(targetId); if (permanent != null && permanent.sacrifice(source, game)) { sumPower += permanent.getPower().getValue(); diff --git a/Mage.Sets/src/mage/cards/s/SynthesisPod.java b/Mage.Sets/src/mage/cards/s/SynthesisPod.java index ba02fa84161..47a8c88ea2d 100644 --- a/Mage.Sets/src/mage/cards/s/SynthesisPod.java +++ b/Mage.Sets/src/mage/cards/s/SynthesisPod.java @@ -78,8 +78,8 @@ class SynthesisPodCost extends CostImpl { if (player == null) { return false; } - player.chooseTarget(Outcome.Exile, targets.get(0), source, game); - Spell spell = game.getSpell(targets.getFirstTarget()); + player.chooseTarget(Outcome.Exile, this.getTargets().get(0), source, game); + Spell spell = game.getSpell(this.getTargets().getFirstTarget()); if (spell == null) { return false; } @@ -93,7 +93,7 @@ class SynthesisPodCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage/src/main/java/mage/abilities/AbilityImpl.java b/Mage/src/main/java/mage/abilities/AbilityImpl.java index 1d2de48df72..29d3722227a 100644 --- a/Mage/src/main/java/mage/abilities/AbilityImpl.java +++ b/Mage/src/main/java/mage/abilities/AbilityImpl.java @@ -914,7 +914,7 @@ public abstract class AbilityImpl implements Ability { if (getModes().getMode() != null) { return getModes().getMode().getTargets(); } - return new Targets(); + return new Targets().withReadOnly(); } @Override @@ -926,7 +926,7 @@ public abstract class AbilityImpl implements Ability { res.addAll(mode.getTargets()); } } - return res; + return res.withReadOnly(); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/CompositeCost.java b/Mage/src/main/java/mage/abilities/costs/CompositeCost.java index 1b9b0a9b5c3..a0e61cfa62a 100644 --- a/Mage/src/main/java/mage/abilities/costs/CompositeCost.java +++ b/Mage/src/main/java/mage/abilities/costs/CompositeCost.java @@ -4,6 +4,7 @@ import mage.abilities.Ability; import mage.game.Game; import mage.target.Targets; +import java.util.Optional; import java.util.UUID; public class CompositeCost implements Cost { @@ -75,10 +76,10 @@ public class CompositeCost implements Cost { @Override public Targets getTargets() { - Targets result = new Targets(); - result.addAll(firstCost.getTargets()); - result.addAll(secondCost.getTargets()); - return result; + Targets res = new Targets(); + res.addAll(firstCost.getTargets()); + res.addAll(secondCost.getTargets()); + return res.withReadOnly(); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/Cost.java b/Mage/src/main/java/mage/abilities/costs/Cost.java index 303dce703fa..9aecb8937f3 100644 --- a/Mage/src/main/java/mage/abilities/costs/Cost.java +++ b/Mage/src/main/java/mage/abilities/costs/Cost.java @@ -32,6 +32,10 @@ public interface Cost extends Serializable, Copyable { void setPaid(); + /** + * Warning, can return copied list in composite costs, so it will be un-changeable + * Use targets list modification only in CostAdjuster for single card/effect + */ Targets getTargets(); Cost copy(); diff --git a/Mage/src/main/java/mage/abilities/costs/CostImpl.java b/Mage/src/main/java/mage/abilities/costs/CostImpl.java index 538d169b2d0..45fdd1a3fb9 100644 --- a/Mage/src/main/java/mage/abilities/costs/CostImpl.java +++ b/Mage/src/main/java/mage/abilities/costs/CostImpl.java @@ -12,19 +12,19 @@ public abstract class CostImpl implements Cost { protected UUID id; protected String text; protected boolean paid; - protected Targets targets; // TODO: optimize performance - use private and null by default + private Targets targets; public CostImpl() { id = UUID.randomUUID(); paid = false; - targets = new Targets(); + targets = null; // rare usage, must be null by default for performance optimization } protected CostImpl(final CostImpl cost) { this.id = cost.id; this.text = cost.text; this.paid = cost.paid; - this.targets = cost.targets.copy(); + this.targets = cost.targets == null ? null : cost.targets.copy(); } @Override @@ -43,14 +43,23 @@ public abstract class CostImpl implements Cost { return this; } - public void addTarget(Target target) { - if (target != null) { - this.targets.add(target); + private void prepareTargets() { + if (this.targets == null) { + this.targets = new Targets(); } } + public void addTarget(Target target) { + if (target == null) { + throw new IllegalArgumentException("Wrong code usage: can't add nullable target to the cost"); + } + prepareTargets(); + this.targets.add(target); + } + @Override public Targets getTargets() { + prepareTargets(); return this.targets; } @@ -62,6 +71,7 @@ public abstract class CostImpl implements Cost { @Override public void clearPaid() { paid = false; + prepareTargets(); targets.clearChosen(); } diff --git a/Mage/src/main/java/mage/abilities/costs/CostsImpl.java b/Mage/src/main/java/mage/abilities/costs/CostsImpl.java index 01ab8e6545d..6cb1e89e092 100644 --- a/Mage/src/main/java/mage/abilities/costs/CostsImpl.java +++ b/Mage/src/main/java/mage/abilities/costs/CostsImpl.java @@ -157,13 +157,11 @@ public class CostsImpl extends ArrayList implements Costs @Override public Targets getTargets() { - Targets targets = new Targets(); + Targets res = new Targets(); for (T cost : this) { - if (cost.getTargets() != null) { - targets.addAll(cost.getTargets()); - } + res.addAll(cost.getTargets()); } - return targets; + return res.withReadOnly(); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/OrCost.java b/Mage/src/main/java/mage/abilities/costs/OrCost.java index 4e34c9a840c..074151c45be 100644 --- a/Mage/src/main/java/mage/abilities/costs/OrCost.java +++ b/Mage/src/main/java/mage/abilities/costs/OrCost.java @@ -142,7 +142,7 @@ public class OrCost implements Cost { if (selectedCost != null) { return selectedCost.getTargets(); } - return null; + return new Targets().withReadOnly(); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/VariableCostImpl.java b/Mage/src/main/java/mage/abilities/costs/VariableCostImpl.java index c065241b0f8..8a1ccfde03c 100644 --- a/Mage/src/main/java/mage/abilities/costs/VariableCostImpl.java +++ b/Mage/src/main/java/mage/abilities/costs/VariableCostImpl.java @@ -37,7 +37,7 @@ public abstract class VariableCostImpl implements Cost, VariableCost { this.id = UUID.randomUUID(); this.costType = costType; this.paid = false; - this.targets = new Targets(); + this.targets = null; // rare usage, must be null by default for performance optimization this.amountPaid = 0; this.xText = xText; this.actionText = actionText; @@ -48,7 +48,7 @@ public abstract class VariableCostImpl implements Cost, VariableCost { this.costType = cost.costType; this.text = cost.text; this.paid = cost.paid; - this.targets = cost.targets.copy(); + this.targets = cost.targets == null ? null : cost.targets.copy(); this.xText = cost.xText; this.actionText = cost.actionText; this.amountPaid = cost.amountPaid; @@ -70,14 +70,23 @@ public abstract class VariableCostImpl implements Cost, VariableCost { return actionText; } - public void addTarget(Target target) { - if (target != null) { - this.targets.add(target); + private void prepareTargets() { + if (this.targets == null) { + this.targets = new Targets(); } } + public void addTarget(Target target) { + if (target == null) { + throw new IllegalArgumentException("Wrong code usage: can't add nullable target to the cost"); + } + prepareTargets(); + this.targets.add(target); + } + @Override public Targets getTargets() { + prepareTargets(); return this.targets; } @@ -89,6 +98,7 @@ public abstract class VariableCostImpl implements Cost, VariableCost { @Override public void clearPaid() { paid = false; + prepareTargets(); targets.clearChosen(); amountPaid = 0; } diff --git a/Mage/src/main/java/mage/abilities/costs/common/DiscardTargetCost.java b/Mage/src/main/java/mage/abilities/costs/common/DiscardTargetCost.java index 3c24baa870c..70d268f8dd5 100644 --- a/Mage/src/main/java/mage/abilities/costs/common/DiscardTargetCost.java +++ b/Mage/src/main/java/mage/abilities/costs/common/DiscardTargetCost.java @@ -42,7 +42,7 @@ public class DiscardTargetCost extends CostImpl { @Override public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { this.cards.clear(); - this.targets.clearChosen(); + this.getTargets().clearChosen(); Player player = game.getPlayer(controllerId); if (player == null) { return false; @@ -50,9 +50,9 @@ public class DiscardTargetCost extends CostImpl { int amount = this.getTargets().get(0).getNumberOfTargets(); if (randomDiscard) { this.cards.addAll(player.discard(amount, true, true, source, game).getCards(game)); - } else if (targets.choose(Outcome.Discard, controllerId, source.getSourceId(), source, game)) { + } else if (this.getTargets().choose(Outcome.Discard, controllerId, source.getSourceId(), source, game)) { Cards toDiscard = new CardsImpl(); - toDiscard.addAll(targets.get(0).getTargets()); + toDiscard.addAll(this.getTargets().get(0).getTargets()); Cards discarded = player.discard(toDiscard, true, source, game); if (!discarded.isEmpty()) { cards.addAll(discarded.getCards(game)); @@ -70,7 +70,7 @@ public class DiscardTargetCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/common/ExileFromGraveCost.java b/Mage/src/main/java/mage/abilities/costs/common/ExileFromGraveCost.java index 97c5db43e3f..8690b000414 100644 --- a/Mage/src/main/java/mage/abilities/costs/common/ExileFromGraveCost.java +++ b/Mage/src/main/java/mage/abilities/costs/common/ExileFromGraveCost.java @@ -78,8 +78,8 @@ public class ExileFromGraveCost extends CostImpl { public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { Player controller = game.getPlayer(controllerId); if (controller != null) { - if (targets.choose(Outcome.Exile, controllerId, source.getSourceId(), source, game)) { - for (UUID targetId : targets.get(0).getTargets()) { + if (this.getTargets().choose(Outcome.Exile, controllerId, source.getSourceId(), source, game)) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { Card card = game.getCard(targetId); if (card == null || game.getState().getZone(targetId) != Zone.GRAVEYARD) { @@ -106,7 +106,7 @@ public class ExileFromGraveCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/common/ExileFromHandCost.java b/Mage/src/main/java/mage/abilities/costs/common/ExileFromHandCost.java index dc29face561..9a9aee8454e 100644 --- a/Mage/src/main/java/mage/abilities/costs/common/ExileFromHandCost.java +++ b/Mage/src/main/java/mage/abilities/costs/common/ExileFromHandCost.java @@ -52,10 +52,10 @@ public class ExileFromHandCost extends CostImpl { @Override public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { - if (targets.choose(Outcome.Exile, controllerId, source.getSourceId(), source, game)) { + if (this.getTargets().choose(Outcome.Exile, controllerId, source.getSourceId(), source, game)) { Player player = game.getPlayer(controllerId); int cmc = 0; - for (UUID targetId : targets.get(0).getTargets()) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { Card card = player.getHand().get(targetId, game); if (card == null) { return false; @@ -82,7 +82,7 @@ public class ExileFromHandCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/common/ExileTargetCost.java b/Mage/src/main/java/mage/abilities/costs/common/ExileTargetCost.java index e2161bdc426..eb87a780d5c 100644 --- a/Mage/src/main/java/mage/abilities/costs/common/ExileTargetCost.java +++ b/Mage/src/main/java/mage/abilities/costs/common/ExileTargetCost.java @@ -45,11 +45,11 @@ public class ExileTargetCost extends CostImpl { @Override public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { Player player = game.getPlayer(ability.getControllerId()); - if (player == null || !targets.choose(Outcome.Exile, controllerId, source.getSourceId(), source, game)) { + if (player == null || !this.getTargets().choose(Outcome.Exile, controllerId, source.getSourceId(), source, game)) { return paid; } Cards cards = new CardsImpl(); - for (UUID targetId : targets.get(0).getTargets()) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { Permanent permanent = game.getPermanent(targetId); if (permanent == null) { return false; @@ -72,7 +72,7 @@ public class ExileTargetCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/common/ReturnToHandChosenControlledPermanentCost.java b/Mage/src/main/java/mage/abilities/costs/common/ReturnToHandChosenControlledPermanentCost.java index 639ca25d24d..df9acb7e687 100644 --- a/Mage/src/main/java/mage/abilities/costs/common/ReturnToHandChosenControlledPermanentCost.java +++ b/Mage/src/main/java/mage/abilities/costs/common/ReturnToHandChosenControlledPermanentCost.java @@ -45,9 +45,9 @@ public class ReturnToHandChosenControlledPermanentCost extends CostImpl { public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { Player controller = game.getPlayer(controllerId); if (controller != null) { - if (targets.choose(Outcome.ReturnToHand, controllerId, source.getSourceId(), source, game)) { + if (this.getTargets().choose(Outcome.ReturnToHand, controllerId, source.getSourceId(), source, game)) { Set permanentsToReturn = new HashSet<>(); - for (UUID targetId : targets.get(0).getTargets()) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { Permanent permanent = game.getPermanent(targetId); if (permanent == null) { return false; @@ -63,7 +63,7 @@ public class ReturnToHandChosenControlledPermanentCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/common/ReturnToHandFromGraveyardCost.java b/Mage/src/main/java/mage/abilities/costs/common/ReturnToHandFromGraveyardCost.java index 691c91f073b..3efc8aec28f 100644 --- a/Mage/src/main/java/mage/abilities/costs/common/ReturnToHandFromGraveyardCost.java +++ b/Mage/src/main/java/mage/abilities/costs/common/ReturnToHandFromGraveyardCost.java @@ -33,9 +33,9 @@ public class ReturnToHandFromGraveyardCost extends CostImpl { public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { Player controller = game.getPlayer(controllerId); if (controller != null) { - if (targets.choose(Outcome.ReturnToHand, controllerId, source.getSourceId(), source, game)) { + if (this.getTargets().choose(Outcome.ReturnToHand, controllerId, source.getSourceId(), source, game)) { Set cardsToMove = new LinkedHashSet<>(); - for (UUID targetId : targets.get(0).getTargets()) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { mage.cards.Card targetCard = game.getCard(targetId); if (targetCard == null) { return false; @@ -52,7 +52,7 @@ public class ReturnToHandFromGraveyardCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/common/RevealTargetFromHandCost.java b/Mage/src/main/java/mage/abilities/costs/common/RevealTargetFromHandCost.java index 8373d08880b..bc0895c69c3 100644 --- a/Mage/src/main/java/mage/abilities/costs/common/RevealTargetFromHandCost.java +++ b/Mage/src/main/java/mage/abilities/costs/common/RevealTargetFromHandCost.java @@ -44,12 +44,12 @@ public class RevealTargetFromHandCost extends CostImpl { @Override public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { - if (targets.choose(Outcome.Benefit, controllerId, source.getSourceId(), source, game)) { + if (this.getTargets().choose(Outcome.Benefit, controllerId, source.getSourceId(), source, game)) { manaValues = 0; numberCardsRevealed = 0; Player player = game.getPlayer(controllerId); Cards cards = new CardsImpl(); - for (UUID targetId : targets.get(0).getTargets()) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { Card card = player.getHand().get(targetId, game); if (card != null) { manaValues += card.getManaValue(); @@ -62,7 +62,7 @@ public class RevealTargetFromHandCost extends CostImpl { MageObject baseObject = game.getBaseObject(source.getSourceId()); player.revealCards(baseObject == null ? "card cost" : baseObject.getIdName(), cards, game); } - if (targets.get(0).getNumberOfTargets() <= numberCardsRevealed) { + if (this.getTargets().get(0).getNumberOfTargets() <= numberCardsRevealed) { paid = true; // e.g. for optional additional costs. example: Dragonlord's Prerogative also true if 0 cards shown return paid; } @@ -89,7 +89,7 @@ public class RevealTargetFromHandCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return allowNoReveal || targets.canChoose(controllerId, source, game); + return allowNoReveal || this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage/src/main/java/mage/abilities/costs/common/SacrificeTargetCost.java b/Mage/src/main/java/mage/abilities/costs/common/SacrificeTargetCost.java index 885b68b2df2..9e11530ab48 100644 --- a/Mage/src/main/java/mage/abilities/costs/common/SacrificeTargetCost.java +++ b/Mage/src/main/java/mage/abilities/costs/common/SacrificeTargetCost.java @@ -54,8 +54,8 @@ public class SacrificeTargetCost extends CostImpl implements SacrificeCost { activator = ((ActivatedAbilityImpl) ability).getActivatorId(); } // can be cancel by user - if (targets.choose(Outcome.Sacrifice, activator, source.getSourceId(), source, game)) { - for (UUID targetId : targets.get(0).getTargets()) { + if (this.getTargets().choose(Outcome.Sacrifice, activator, source.getSourceId(), source, game)) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { Permanent permanent = game.getPermanent(targetId); if (permanent == null) { return false; @@ -63,7 +63,7 @@ public class SacrificeTargetCost extends CostImpl implements SacrificeCost { addSacrificeTarget(game, permanent); paid |= permanent.sacrifice(source, game); } - if (!paid && targets.get(0).getNumberOfTargets() == 0) { + if (!paid && this.getTargets().get(0).getNumberOfTargets() == 0) { paid = true; // e.g. for Devouring Rage } } @@ -87,8 +87,8 @@ public class SacrificeTargetCost extends CostImpl implements SacrificeCost { } int validTargets = 0; - int neededtargets = targets.get(0).getNumberOfTargets(); - for (Permanent permanent : game.getBattlefield().getAllActivePermanents(((TargetControlledPermanent) targets.get(0)).getFilter(), controllerId, game)) { + int neededtargets = this.getTargets().get(0).getNumberOfTargets(); + for (Permanent permanent : game.getBattlefield().getAllActivePermanents(((TargetControlledPermanent) this.getTargets().get(0)).getFilter(), controllerId, game)) { if (game.getPlayer(activator).canPaySacrificeCost(permanent, source, controllerId, game)) { validTargets++; if (validTargets >= neededtargets) { @@ -97,7 +97,7 @@ public class SacrificeTargetCost extends CostImpl implements SacrificeCost { } } // solves issue #8097, if a sacrifice cost is optional and you don't have valid targets, then the cost can be paid - if (validTargets == 0 && targets.get(0).getMinNumberOfTargets() == 0) { + if (validTargets == 0 && this.getTargets().get(0).getMinNumberOfTargets() == 0) { return true; } return false; diff --git a/Mage/src/main/java/mage/abilities/costs/mana/ManaCostsImpl.java b/Mage/src/main/java/mage/abilities/costs/mana/ManaCostsImpl.java index eed7779f46d..8b1c14b0f22 100644 --- a/Mage/src/main/java/mage/abilities/costs/mana/ManaCostsImpl.java +++ b/Mage/src/main/java/mage/abilities/costs/mana/ManaCostsImpl.java @@ -601,11 +601,11 @@ public class ManaCostsImpl extends ArrayList implements M @Override public Targets getTargets() { - Targets targets = new Targets(); + Targets res = new Targets(); for (T cost : this) { - targets.addAll(cost.getTargets()); + res.addAll(cost.getTargets()); } - return targets; + return res.withReadOnly(); } @Override diff --git a/Mage/src/main/java/mage/abilities/keyword/ChampionAbility.java b/Mage/src/main/java/mage/abilities/keyword/ChampionAbility.java index e3cadf08061..8e4ea99d7fb 100644 --- a/Mage/src/main/java/mage/abilities/keyword/ChampionAbility.java +++ b/Mage/src/main/java/mage/abilities/keyword/ChampionAbility.java @@ -134,10 +134,10 @@ class ChampionExileCost extends CostImpl { @Override public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { Player controller = game.getPlayer(controllerId); - if (controller == null || !targets.choose(Outcome.Exile, controllerId, source.getSourceId(), source, game)) { + if (controller == null || !this.getTargets().choose(Outcome.Exile, controllerId, source.getSourceId(), source, game)) { return paid; } - for (UUID targetId : targets.get(0).getTargets()) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { Permanent permanent = game.getPermanent(targetId); if (permanent == null) { return false; @@ -156,7 +156,7 @@ class ChampionExileCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage/src/main/java/mage/abilities/keyword/NinjutsuAbility.java b/Mage/src/main/java/mage/abilities/keyword/NinjutsuAbility.java index 01985c72626..a897d732f4f 100644 --- a/Mage/src/main/java/mage/abilities/keyword/NinjutsuAbility.java +++ b/Mage/src/main/java/mage/abilities/keyword/NinjutsuAbility.java @@ -148,8 +148,8 @@ class ReturnAttackerToHandTargetCost extends CostImpl { @Override public boolean pay(Ability ability, Game game, Ability source, UUID controllerId, boolean noMana, Cost costToPay) { - if (targets.choose(Outcome.ReturnToHand, controllerId, source.getSourceId(), source, game)) { - for (UUID targetId : targets.get(0).getTargets()) { + if (this.getTargets().choose(Outcome.ReturnToHand, controllerId, source.getSourceId(), source, game)) { + for (UUID targetId : this.getTargets().get(0).getTargets()) { Permanent permanent = game.getPermanent(targetId); Player controller = game.getPlayer(controllerId); if (permanent == null @@ -165,7 +165,7 @@ class ReturnAttackerToHandTargetCost extends CostImpl { @Override public boolean canPay(Ability ability, Ability source, UUID controllerId, Game game) { - return targets.canChoose(controllerId, source, game); + return this.getTargets().canChoose(controllerId, source, game); } @Override diff --git a/Mage/src/main/java/mage/target/Targets.java b/Mage/src/main/java/mage/target/Targets.java index 30e1c395438..e39b9328a7e 100644 --- a/Mage/src/main/java/mage/target/Targets.java +++ b/Mage/src/main/java/mage/target/Targets.java @@ -4,17 +4,17 @@ import mage.abilities.Ability; import mage.constants.Outcome; import mage.game.Game; import mage.game.events.GameEvent; +import mage.util.Copyable; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.List; -import java.util.UUID; +import java.util.*; import java.util.stream.Collectors; /** * @author BetaSteward_at_googlemail.com */ -public class Targets extends ArrayList { +public class Targets extends ArrayList implements Copyable { + + private boolean isReadOnly = false; // runtime protect from not working targets modification, e.g. in composite costs public Targets() { // fast constructor @@ -25,12 +25,18 @@ public class Targets extends ArrayList { } protected Targets(final Targets targets) { + this.isReadOnly = targets.isReadOnly; this.ensureCapacity(targets.size()); for (Target target : targets) { this.add(target.copy()); } } + public Targets withReadOnly() { + this.isReadOnly = true; + return this; + } + public List getUnchosen() { return stream().filter(target -> !target.isChosen()).collect(Collectors.toList()); } @@ -147,7 +153,44 @@ public class Targets extends ArrayList { return null; } + @Override public Targets copy() { return new Targets(this); } + + private void checkReadOnlyModification() { + if (this.isReadOnly) { + throw new IllegalArgumentException("Wrong code usage: you can't modify read only targets list, e.g. from composite costs"); + } + } + + @Override + public boolean add(Target target) { + checkReadOnlyModification(); + return super.add(target); + } + + @Override + public void add(int index, Target element) { + checkReadOnlyModification(); + super.add(index, element); + } + + @Override + public boolean addAll(Collection c) { + checkReadOnlyModification(); + return super.addAll(c); + } + + @Override + public boolean addAll(int index, Collection c) { + checkReadOnlyModification(); + return super.addAll(index, c); + } + + @Override + public void clear() { + checkReadOnlyModification(); + super.clear(); + } }