External/mage
1
0
Fork 1
mirror of https://github.com/magefree/mage.git synced 2025-12-25 13:02:06 -08:00
Code Issues Projects Releases Packages Wiki Activity Actions

performance: improved CPU/memory usage with copy of cost's targets (related to #11285), added runtime check for wrong targets usage

Browse source
This commit is contained in:
Oleg Agafonov 2023-11-07 02:47:48 +04:00
parent d6c858ecaf
commit ddc1ec8ef8
32 changed files with 173 additions and 107 deletions
Download patch file Download diff file Expand all files Collapse all files

4
Mage/src/main/java/mage/abilities/AbilityImpl.java
View file

@ -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

9
Mage/src/main/java/mage/abilities/costs/CompositeCost.java
View file

@ -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

4
Mage/src/main/java/mage/abilities/costs/Cost.java
View file

@ -32,6 +32,10 @@ public interface Cost extends Serializable, Copyable<Cost> {
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();

22
Mage/src/main/java/mage/abilities/costs/CostImpl.java
View file

@ -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();
}

8
Mage/src/main/java/mage/abilities/costs/CostsImpl.java
View file

@ -157,13 +157,11 @@ public class CostsImpl<T extends Cost> extends ArrayList<T> implements Costs<T>
@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

2
Mage/src/main/java/mage/abilities/costs/OrCost.java
View file

@ -142,7 +142,7 @@ public class OrCost implements Cost {
if (selectedCost != null) {
return selectedCost.getTargets();
}
return null;
return new Targets().withReadOnly();
}
@Override

20
Mage/src/main/java/mage/abilities/costs/VariableCostImpl.java
View file

@ -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;
}

8
Mage/src/main/java/mage/abilities/costs/common/DiscardTargetCost.java
View file

@ -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

6
Mage/src/main/java/mage/abilities/costs/common/ExileFromGraveCost.java
View file

@ -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

6
Mage/src/main/java/mage/abilities/costs/common/ExileFromHandCost.java
View file

@ -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

6
Mage/src/main/java/mage/abilities/costs/common/ExileTargetCost.java
View file

@ -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

6
Mage/src/main/java/mage/abilities/costs/common/ReturnToHandChosenControlledPermanentCost.java
View file

@ -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<Card> 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

6
Mage/src/main/java/mage/abilities/costs/common/ReturnToHandFromGraveyardCost.java
View file

@ -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<Card> 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

8
Mage/src/main/java/mage/abilities/costs/common/RevealTargetFromHandCost.java
View file

@ -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

12
Mage/src/main/java/mage/abilities/costs/common/SacrificeTargetCost.java
View file

@ -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;

6
Mage/src/main/java/mage/abilities/costs/mana/ManaCostsImpl.java
View file

@ -601,11 +601,11 @@ public class ManaCostsImpl<T extends ManaCost> extends ArrayList<T> 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

6
Mage/src/main/java/mage/abilities/keyword/ChampionAbility.java
View file

@ -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

6
Mage/src/main/java/mage/abilities/keyword/NinjutsuAbility.java
View file

@ -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

53
Mage/src/main/java/mage/target/Targets.java
View file

@ -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<Target> {
public class Targets extends ArrayList<Target> implements Copyable<Targets> {
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<Target> {
}
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<Target> getUnchosen() {
return stream().filter(target -> !target.isChosen()).collect(Collectors.toList());
}
@ -147,7 +153,44 @@ public class Targets extends ArrayList<Target> {
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<? extends Target> c) {
checkReadOnlyModification();
return super.addAll(c);
}
@Override
public boolean addAll(int index, Collection<? extends Target> c) {
checkReadOnlyModification();
return super.addAll(index, c);
}
@Override
public void clear() {
checkReadOnlyModification();
super.clear();
}
}