From 8372c358b20b972cc52be323f99e8b874399afdb Mon Sep 17 00:00:00 2001 From: xenohedron Date: Sat, 15 Jul 2023 17:50:24 -0400 Subject: [PATCH] refactor: some code linting (#10624) * remove obsolete check method * code clean: PermanentImpl * code clean: PlayerImpl * protected constructors in CardImpl --- Mage/src/main/java/mage/cards/CardImpl.java | 12 +- .../mage/game/permanent/PermanentImpl.java | 199 +++++++----------- .../main/java/mage/players/PlayerImpl.java | 154 ++++++-------- 3 files changed, 155 insertions(+), 210 deletions(-) diff --git a/Mage/src/main/java/mage/cards/CardImpl.java b/Mage/src/main/java/mage/cards/CardImpl.java index 677de368049..a84d73f7495 100644 --- a/Mage/src/main/java/mage/cards/CardImpl.java +++ b/Mage/src/main/java/mage/cards/CardImpl.java @@ -54,10 +54,10 @@ public abstract class CardImpl extends MageObjectImpl implements Card { protected boolean morphCard; protected List attachments = new ArrayList<>(); - public CardImpl(UUID ownerId, CardSetInfo setInfo, CardType[] cardTypes, String costs) { + protected CardImpl(UUID ownerId, CardSetInfo setInfo, CardType[] cardTypes, String costs) { this(ownerId, setInfo, cardTypes, costs, SpellAbilityType.BASE); } - public CardImpl(UUID ownerId, CardSetInfo setInfo, CardType[] cardTypes, String costs, SpellAbilityType spellAbilityType) { + protected CardImpl(UUID ownerId, CardSetInfo setInfo, CardType[] cardTypes, String costs, SpellAbilityType spellAbilityType) { this(ownerId, setInfo.getName()); this.rarity = setInfo.getRarity(); @@ -113,7 +113,7 @@ public abstract class CardImpl extends MageObjectImpl implements Card { this.name = name; } - public CardImpl(final CardImpl card) { + protected CardImpl(final CardImpl card) { super(card); ownerId = card.ownerId; rarity = card.rarity; @@ -729,10 +729,8 @@ public abstract class CardImpl extends MageObjectImpl implements Card { } public boolean addCounters(Counter counter, UUID playerAddingCounters, Ability source, Game game, List appliedEffects, boolean isEffect, int maxCounters) { - if (this instanceof Permanent) { - if (!((Permanent) this).isPhasedIn()) { - return false; - } + if (this instanceof Permanent && !((Permanent) this).isPhasedIn()) { + return false; } boolean returnCode = true; diff --git a/Mage/src/main/java/mage/game/permanent/PermanentImpl.java b/Mage/src/main/java/mage/game/permanent/PermanentImpl.java index 177354a4063..1d52ccfb567 100644 --- a/Mage/src/main/java/mage/game/permanent/PermanentImpl.java +++ b/Mage/src/main/java/mage/game/permanent/PermanentImpl.java @@ -111,23 +111,23 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { protected int createOrder; protected boolean legendRuleApplies = true; - private static final List emptyList = Collections.unmodifiableList(new ArrayList()); + private static final List emptyList = Collections.unmodifiableList(new ArrayList<>()); - public PermanentImpl(UUID ownerId, UUID controllerId, String name) { + protected PermanentImpl(UUID ownerId, UUID controllerId, String name) { super(ownerId, name); this.originalControllerId = controllerId; this.controllerId = controllerId; this.counters = new Counters(); } - public PermanentImpl(UUID id, UUID ownerId, UUID controllerId, String name) { + protected PermanentImpl(UUID id, UUID ownerId, UUID controllerId, String name) { super(id, ownerId, name); this.originalControllerId = controllerId; this.controllerId = controllerId; this.counters = new Counters(); } - public PermanentImpl(final PermanentImpl permanent) { + protected PermanentImpl(final PermanentImpl permanent) { super(permanent); this.tapped = permanent.tapped; this.flipped = permanent.flipped; @@ -146,7 +146,7 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { this.markedLifelink = permanent.markedLifelink; for (Map.Entry> entry : permanent.connectedCards.entrySet()) { - this.connectedCards.put(entry.getKey(), entry.getValue()); + this.connectedCards.put(entry.getKey(), new ArrayList<>(entry.getValue())); } if (permanent.dealtDamageByThisTurn != null) { dealtDamageByThisTurn = new HashSet<>(permanent.dealtDamageByThisTurn); @@ -544,12 +544,10 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { @Override public boolean untap(Game game) { //20091005 - 701.15b - if (tapped) { - if (!replaceEvent(EventType.UNTAP, game)) { - this.tapped = false; - fireEvent(EventType.UNTAPPED, game); - return true; - } + if (tapped && !replaceEvent(EventType.UNTAP, game)) { + this.tapped = false; + fireEvent(EventType.UNTAPPED, game); + return true; } return false; } @@ -562,12 +560,10 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { @Override public boolean tap(boolean forCombat, Ability source, Game game) { //20091005 - 701.15a - if (!tapped) { - if (!replaceEvent(EventType.TAP, game)) { - this.tapped = true; - game.fireEvent(new GameEvent(GameEvent.EventType.TAPPED, objectId, source, controllerId, 0, forCombat)); - return true; - } + if (!tapped && !replaceEvent(EventType.TAP, game)) { + this.tapped = true; + game.fireEvent(new GameEvent(GameEvent.EventType.TAPPED, objectId, source, controllerId, 0, forCombat)); + return true; } return false; } @@ -589,12 +585,10 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { @Override public boolean flip(Game game) { - if (!flipped) { - if (!replaceEvent(EventType.FLIP, game)) { - this.flipped = true; - fireEvent(EventType.FLIPPED, game); - return true; - } + if (!flipped && !replaceEvent(EventType.FLIP, game)) { + this.flipped = true; + fireEvent(EventType.FLIPPED, game); + return true; } return false; } @@ -668,23 +662,18 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { @Override public boolean phaseIn(Game game, boolean onlyDirect) { - if (!phasedIn) { - if (!replaceEvent(EventType.PHASE_IN, game) - && (!onlyDirect || !indirectPhase)) { - this.phasedIn = true; - this.indirectPhase = false; - if (!game.isSimulation()) { - game.informPlayers(getLogName() + " phased in"); + if (!phasedIn && !replaceEvent(EventType.PHASE_IN, game) && (!onlyDirect || !indirectPhase)) { + this.phasedIn = true; + this.indirectPhase = false; + game.informPlayers(getLogName() + " phased in"); + for (UUID attachedId : this.getAttachments()) { + Permanent attachedPerm = game.getPermanent(attachedId); + if (attachedPerm != null) { + attachedPerm.phaseIn(game, false); } - for (UUID attachedId : this.getAttachments()) { - Permanent attachedPerm = game.getPermanent(attachedId); - if (attachedPerm != null) { - attachedPerm.phaseIn(game, false); - } - } - fireEvent(EventType.PHASED_IN, game); - return true; } + fireEvent(EventType.PHASED_IN, game); + return true; } return false; } @@ -696,23 +685,19 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { @Override public boolean phaseOut(Game game, boolean indirectPhase) { - if (phasedIn) { - if (!replaceEvent(EventType.PHASE_OUT, game)) { - for (UUID attachedId : this.getAttachments()) { - Permanent attachedPerm = game.getPermanent(attachedId); - if (attachedPerm != null) { - attachedPerm.phaseOut(game, true); - } + if (phasedIn && !replaceEvent(EventType.PHASE_OUT, game)) { + for (UUID attachedId : this.getAttachments()) { + Permanent attachedPerm = game.getPermanent(attachedId); + if (attachedPerm != null) { + attachedPerm.phaseOut(game, true); } - this.removeFromCombat(game); - this.phasedIn = false; - this.indirectPhase = indirectPhase; - if (!game.isSimulation()) { - game.informPlayers(getLogName() + " phased out"); - } - fireEvent(EventType.PHASED_OUT, game); - return true; } + this.removeFromCombat(game); + this.phasedIn = false; + this.indirectPhase = indirectPhase; + game.informPlayers(getLogName() + " phased out"); + fireEvent(EventType.PHASED_OUT, game); + return true; } return false; } @@ -804,13 +789,11 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { } // Losing control of a ring bearer clear its status. - public void removeUncontrolledRingBearer(Game game){ - if(isRingBearer()) { - UUID controllerId = beforeResetControllerId; - Player controller = controllerId == null ? null : game.getPlayer(controllerId); + public void removeUncontrolledRingBearer(Game game) { + if (isRingBearer()) { + Player controller = beforeResetControllerId == null ? null : game.getPlayer(beforeResetControllerId); String controllerName = controller == null ? "" : controller.getLogName(); game.informPlayers(controllerName + " has lost control of " + getLogName() + ". It is no longer a Ring-bearer."); - this.setRingBearer(game, false); } } @@ -824,7 +807,7 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { this.getAbilities(game).setControllerId(controllerId); game.getContinuousEffects().setController(objectId, controllerId); - // the controller of triggered abilites is always set/checked before the abilities triggers so not needed here + // the controller of triggered abilities is always set/checked before the abilities triggers so not needed here game.fireEvent(new GameEvent(GameEvent.EventType.LOST_CONTROL, objectId, null, beforeResetControllerId)); game.fireEvent(new GameEvent(GameEvent.EventType.GAINED_CONTROL, objectId, null, controllerId)); @@ -898,8 +881,8 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { this.attachedTo = attachToObjectId; this.attachedToZoneChangeCounter = game.getState().getZoneChangeCounter(attachToObjectId); for (Ability ability : this.getAbilities()) { - for (Iterator ite = ability.getEffects(game, EffectType.CONTINUOUS).iterator(); ite.hasNext(); ) { - ContinuousEffect effect = (ContinuousEffect) ite.next(); + for (Effect value : ability.getEffects(game, EffectType.CONTINUOUS)) { + ContinuousEffect effect = (ContinuousEffect) value; game.getContinuousEffects().setOrder(effect); // It's important to update the timestamp of the copied effect in ContinuousEffects because it does the action for (ContinuousEffect conEffect : game.getContinuousEffects().getLayeredEffects(game)) { @@ -914,7 +897,7 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { this.addInfo("attachedTo", null, game); if (this.attachedTo != null) { Permanent attachedToPerm = game.getPermanent(this.getAttachedTo()); - // If what this permanent is attached to isn't also a permenent, such as a + // If what this permanent is attached to isn't also a permanent, such as a // player or card in graveyard, it is important to mention what it is attached // to in the tooltip. The rules let you attach a permanent to any kind of object // or player, although currently the only objects it's possible to attach a @@ -978,8 +961,7 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { * @return */ private int doDamage(int damageAmount, UUID attackerId, Ability source, Game game, boolean preventable, boolean combat, boolean markDamage, List appliedEffects) { - int damageDone = 0; - if (damageAmount < 1 || !canDamage(game.getObject(attackerId), game)) { + if (damageAmount < 1) { return 0; } DamageEvent event = new DamagePermanentEvent(objectId, attackerId, controllerId, damageAmount, preventable, combat); @@ -987,8 +969,8 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { if (game.replaceEvent(event)) { return 0; } - int actualDamage = checkProtectionAbilities(event, attackerId, source, game); - if (actualDamage < 1) { + int actualDamageDone = checkProtectionAbilities(event, attackerId, source, game); + if (actualDamageDone < 1) { return 0; } int lethal = getLethalDamage(attackerId, game); @@ -997,22 +979,22 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { if (checkWither(event, attacker, game)) { if (markDamage) { // mark damage only - markDamage(CounterType.M1M1.createInstance(actualDamage), attacker, true); + markDamage(CounterType.M1M1.createInstance(actualDamageDone), attacker, true); } else { Ability damageSourceAbility = null; if (attacker instanceof Permanent) { damageSourceAbility = ((Permanent) attacker).getSpellAbility(); } // deal damage immediately - addCounters(CounterType.M1M1.createInstance(actualDamage), game.getControllerId(attackerId), damageSourceAbility, game); + addCounters(CounterType.M1M1.createInstance(actualDamageDone), game.getControllerId(attackerId), damageSourceAbility, game); } } else { - this.damage = CardUtil.overflowInc(this.damage, actualDamage); + this.damage = CardUtil.overflowInc(this.damage, actualDamageDone); } } if (this.isPlaneswalker(game)) { int loyalty = getCounters(game).getCount(CounterType.LOYALTY); - int countersToRemove = Math.min(actualDamage, loyalty); + int countersToRemove = Math.min(actualDamageDone, loyalty); if (attacker != null && markDamage) { markDamage(CounterType.LOYALTY.createInstance(countersToRemove), attacker, false); } else { @@ -1021,21 +1003,17 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { } if (this.isBattle(game)) { int defense = getCounters(game).getCount(CounterType.DEFENSE); - int countersToRemove = Math.min(actualDamage, defense); + int countersToRemove = Math.min(actualDamageDone, defense); if (attacker != null && markDamage) { markDamage(CounterType.DEFENSE.createInstance(countersToRemove), attacker, false); } else { removeCounters(CounterType.DEFENSE.getName(), countersToRemove, source, game); } } - DamagedEvent damagedEvent = new DamagedPermanentEvent(this.getId(), attackerId, this.getControllerId(), actualDamage, combat); - damagedEvent.setExcess(actualDamage - lethal); + DamagedEvent damagedEvent = new DamagedPermanentEvent(this.getId(), attackerId, this.getControllerId(), actualDamageDone, combat); + damagedEvent.setExcess(actualDamageDone - lethal); game.fireEvent(damagedEvent); game.getState().addSimultaneousDamage(damagedEvent, game); - damageDone = actualDamage; - if (damageDone < 1) { - return 0; - } UUID sourceControllerId = null; Abilities sourceAbilities = null; attacker = game.getPermanentOrLKIBattlefield(attackerId); @@ -1065,10 +1043,10 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { if (attacker != null && sourceAbilities != null) { if (sourceAbilities.containsKey(LifelinkAbility.getInstance().getId())) { if (markDamage) { - game.getPermanent(attackerId).markLifelink(damageDone); + game.getPermanent(attackerId).markLifelink(actualDamageDone); } else { Player player = game.getPlayer(sourceControllerId); - player.gainLife(damageDone, game, source); + player.gainLife(actualDamageDone, game, source); } } if (sourceAbilities.containsKey(DeathtouchAbility.getInstance().getId())) { @@ -1080,16 +1058,16 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { // Unstable ability - Earl of Squirrel if (sourceAbilities.containsKey(SquirrellinkAbility.getInstance().getId())) { Player player = game.getPlayer(sourceControllerId); - new SquirrelToken().putOntoBattlefield(damageDone, game, source, player.getId()); + new SquirrelToken().putOntoBattlefield(actualDamageDone, game, source, player.getId()); } dealtDamageByThisTurn.add(new MageObjectReference(attacker, game)); } if (attacker == null) { - game.informPlayers(getLogName() + " gets " + damageDone + " damage"); + game.informPlayers(getLogName() + " gets " + actualDamageDone + " damage"); } else { - game.informPlayers(attacker.getLogName() + " deals " + damageDone + " damage to " + getLogName()); + game.informPlayers(attacker.getLogName() + " deals " + actualDamageDone + " damage to " + getLogName()); } - return damageDone; + return actualDamageDone; } private static boolean checkWither(DamageEvent event, MageObject attacker, Game game) { @@ -1132,7 +1110,7 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { for (MarkedDamageInfo mdi : markedDamage) { Ability source = null; if (mdi.sourceObject instanceof PermanentToken) { - /* Tokens dont have a spellAbility. We must make a phony one as the source so the events in addCounters + /* Tokens don't have a spellAbility. We must make a phony one as the source so the events in addCounters * can trace the source back to an object/controller. */ source = new SpellAbility(null, ((PermanentToken) mdi.sourceObject).name); @@ -1155,9 +1133,9 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { int lethal = Integer.MAX_VALUE; if (this.isCreature(game)) { if (game.getState().getActivePowerInsteadOfToughnessForDamageLethalityFilters().stream().anyMatch(f -> f.match(this, game))) { - lethal = Math.min(lethal, power.getValue()); + lethal = power.getValue(); } else { - lethal = Math.min(lethal, toughness.getValue()); + lethal = toughness.getValue(); } lethal = Math.max(lethal - this.damage, 0); Card attacker = game.getPermanent(attackerId); @@ -1312,13 +1290,6 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { return game.getContinuousEffects().preventedByRuleModification(new StayAttachedEvent(this.getId(), attachment.getId(), source), null, game, silentMode); } - protected boolean canDamage(MageObject source, Game game) { - //noxx: having protection doesn't prevents from dealing damage - // instead it adds damage prevention - //return (!hasProtectionFrom(source, game)); - return true; - } - @Override public boolean destroy(Ability source, Game game) { return destroy(source, game, false); @@ -1326,7 +1297,7 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { @Override public boolean destroy(Ability source, Game game, boolean noRegen) { - // Only permanets on the battlefield can be destroyed + // Only permanents on the battlefield can be destroyed if (!game.getState().getZone(getId()).equals(Zone.BATTLEFIELD)) { return false; } @@ -1337,21 +1308,19 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { if (!game.replaceEvent(GameEvent.getEvent(GameEvent.EventType.DESTROY_PERMANENT, objectId, source, controllerId, noRegen ? 1 : 0))) { // this means destroy was successful, if object movement to graveyard will be replaced (e.g. commander to command zone) it's still - // handled as successful destroying (but not as sucessful "dies this way" for destroying). + // handled as successful destroying (but not as successful "dies this way" for destroying). if (moveToZone(Zone.GRAVEYARD, source, game, false)) { - if (!game.isSimulation()) { - String logName; - Card card = game.getCard(this.getId()); - if (card != null) { - logName = card.getLogName(); - } else { - logName = this.getLogName(); - } - if (this.isCreature(game)) { - game.informPlayers(logName + " died" + CardUtil.getSourceLogName(game, " by ", source, "", "")); - } else { - game.informPlayers(logName + " was destroyed" + CardUtil.getSourceLogName(game, " by ", source, "", "")); - } + String logName; + Card card = game.getCard(this.getId()); + if (card != null) { + logName = card.getLogName(); + } else { + logName = this.getLogName(); + } + if (this.isCreature(game)) { + game.informPlayers(logName + " died" + CardUtil.getSourceLogName(game, " by ", source, "", "")); + } else { + game.informPlayers(logName + " was destroyed" + CardUtil.getSourceLogName(game, " by ", source, "", "")); } game.fireEvent(GameEvent.getEvent(GameEvent.EventType.DESTROYED_PERMANENT, objectId, source, controllerId)); } @@ -1368,7 +1337,7 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { // so the return value of the moveToZone is not taken into account here moveToZone(Zone.GRAVEYARD, source, game, false); Player player = game.getPlayer(getControllerId()); - if (player != null && !game.isSimulation()) { + if (player != null) { game.informPlayers(player.getLogName() + " sacrificed " + this.getLogName() + CardUtil.getSourceLogName(game, source)); } game.fireEvent(GameEvent.getEvent(GameEvent.EventType.SACRIFICED_PERMANENT, objectId, source, controllerId)); @@ -1593,10 +1562,8 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { public boolean removeFromCombat(Game game, boolean withEvent) { if (this.isAttacking() || this.blocking > 0) { return game.getCombat().removeFromCombat(objectId, game, withEvent); - } else if (this.isPlaneswalker(game)) { - if (game.getCombat().getDefenders().contains(getId())) { - game.getCombat().removeDefendingPermanentFromCombat(objectId, game); - } + } else if (this.isPlaneswalker(game) && game.getCombat().getDefenders().contains(getId())) { + game.getCombat().removeDefendingPermanentFromCombat(objectId, game); } return false; } @@ -1626,11 +1593,7 @@ public abstract class PermanentImpl extends CardImpl implements Permanent { @Override public List getImprinted() { - if (this.connectedCards.containsKey("imprint")) { - return this.connectedCards.get("imprint"); - } else { - return emptyList; - } + return this.connectedCards.getOrDefault("imprint", emptyList); } @Override diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index 72a630aeffc..7e4c838ea62 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/Mage/src/main/java/mage/players/PlayerImpl.java @@ -186,7 +186,7 @@ public abstract class PlayerImpl implements Player, Serializable { protected final Map silentPhaseSteps = ImmutableMap.builder(). put(PhaseStep.DECLARE_ATTACKERS, Step.StepPart.PRE).build(); - public PlayerImpl(String name, RangeOfInfluence range) { + protected PlayerImpl(String name, RangeOfInfluence range) { this(UUID.randomUUID()); this.name = name; this.range = range; @@ -204,7 +204,7 @@ public abstract class PlayerImpl implements Player, Serializable { this.playerId = id; } - public PlayerImpl(final PlayerImpl player) { + protected PlayerImpl(final PlayerImpl player) { this.abort = player.abort; this.playerId = player.playerId; @@ -310,7 +310,7 @@ public abstract class PlayerImpl implements Player, Serializable { // this.timerTimeout = player.hasTimerTimeout(); // can't change so no need to restore // this.isTestMode = player.isTestMode(); - // This is meta data and should'nt be restored by rollback + // This is meta data and shouldn't be restored by rollback // this.userData = player.getUserData(); this.library = player.getLibrary().copy(); this.sideboard = player.getSideboard().copy(); @@ -607,7 +607,7 @@ public abstract class PlayerImpl implements Player, Serializable { this.turnControllers.clear(); this.turnController = getId(); } else { - if (turnControllers.size() > 0) { + if (!turnControllers.isEmpty()) { this.turnControllers.remove(turnControllers.size() - 1); } if (turnControllers.isEmpty()) { @@ -774,13 +774,13 @@ public abstract class PlayerImpl implements Player, Serializable { private Cards getRandomToDiscard(int amount, Ability source, Game game) { Cards toDiscard = new CardsImpl(); - Cards hand = getHand().copy(); + Cards theHand = getHand().copy(); for (int i = 0; i < amount; i++) { - if (hand.isEmpty()) { + if (theHand.isEmpty()) { break; } - Card card = hand.getRandom(game); - hand.remove(card); + Card card = theHand.getRandom(game); + theHand.remove(card); toDiscard.add(card); } return toDiscard; @@ -839,13 +839,11 @@ public abstract class PlayerImpl implements Player, Serializable { if (aura == null) { aura = game.getPermanentEntering(permanentId); } - if (aura != null) { - if (!game.replaceEvent(new EnchantPlayerEvent(playerId, aura, source))) { - this.attachments.add(permanentId); - aura.attachTo(playerId, source, game); - game.fireEvent(new EnchantedPlayerEvent(playerId, aura, source)); - return true; - } + if (aura != null && !game.replaceEvent(new EnchantPlayerEvent(playerId, aura, source))) { + this.attachments.add(permanentId); + aura.attachTo(playerId, source, game); + game.fireEvent(new EnchantedPlayerEvent(playerId, aura, source)); + return true; } } return false; @@ -853,13 +851,12 @@ public abstract class PlayerImpl implements Player, Serializable { @Override public boolean removeAttachment(Permanent attachment, Ability source, Game game) { - if (this.attachments.contains(attachment.getId())) { - if (!game.replaceEvent(new UnattachEvent(playerId, attachment.getId(), attachment, source))) { - this.attachments.remove(attachment.getId()); - attachment.attachTo(null, source, game); - game.fireEvent(new UnattachedEvent(playerId, attachment.getId(), attachment, source)); - return true; - } + if (this.attachments.contains(attachment.getId()) + && !game.replaceEvent(new UnattachEvent(playerId, attachment.getId(), attachment, source))) { + this.attachments.remove(attachment.getId()); + attachment.attachTo(null, source, game); + game.fireEvent(new UnattachedEvent(playerId, attachment.getId(), attachment, source)); + return true; } return false; } @@ -931,7 +928,7 @@ public abstract class PlayerImpl implements Player, Serializable { List ids = new ArrayList<>(cards); Collections.shuffle(ids); for (UUID id : ids) { - moveObjectToLibrary(id, source, game, false, false); + moveObjectToLibrary(id, source, game, false); } } else { // user defined order @@ -945,11 +942,11 @@ public abstract class PlayerImpl implements Player, Serializable { break; } cards.remove(targetObjectId); - moveObjectToLibrary(targetObjectId, source, game, false, false); + moveObjectToLibrary(targetObjectId, source, game, false); target.clearChosen(); } for (UUID c : cards) { - moveObjectToLibrary(c, source, game, false, false); + moveObjectToLibrary(c, source, game, false); } } } @@ -1024,7 +1021,7 @@ public abstract class PlayerImpl implements Player, Serializable { List ids = new ArrayList<>(cards); Collections.shuffle(ids); for (UUID id : ids) { - moveObjectToLibrary(id, source, game, true, false); + moveObjectToLibrary(id, source, game, true); } } else { // user defined order @@ -1039,11 +1036,11 @@ public abstract class PlayerImpl implements Player, Serializable { break; } cards.remove(targetObjectId); - moveObjectToLibrary(targetObjectId, source, game, true, false); + moveObjectToLibrary(targetObjectId, source, game, true); target.clearChosen(); } for (UUID c : cards) { - moveObjectToLibrary(c, source, game, true, false); + moveObjectToLibrary(c, source, game, true); } } } @@ -1058,11 +1055,11 @@ public abstract class PlayerImpl implements Player, Serializable { return true; } - private boolean moveObjectToLibrary(UUID objectId, Ability source, Game game, boolean toTop, boolean withName) { + private void moveObjectToLibrary(UUID objectId, Ability source, Game game, boolean toTop) { MageObject mageObject = game.getObject(objectId); if (mageObject instanceof Spell && mageObject.isCopy()) { // Spell copies are not moved as cards, so here the no copy spell has to be selected to move - // (but because copy and original have the same objectId the wrong sepell can be selected from stack). + // (but because copy and original have the same objectId the wrong spell can be selected from stack). // So let's check if the original spell is on the stack and has to be selected. // TODO: Better handling so each spell could be selected by a unique id Spell spellNoCopy = game.getStack().getSpell(source.getSourceId(), false); if (spellNoCopy != null) { @@ -1072,12 +1069,11 @@ public abstract class PlayerImpl implements Player, Serializable { if (mageObject != null) { Zone fromZone = game.getState().getZone(objectId); if ((mageObject instanceof Permanent)) { - return this.moveCardToLibraryWithInfo((Permanent) mageObject, source, game, fromZone, toTop, withName); + this.moveCardToLibraryWithInfo((Permanent) mageObject, source, game, fromZone, toTop, false); } else if (mageObject instanceof Card) { - return this.moveCardToLibraryWithInfo((Card) mageObject, source, game, fromZone, toTop, withName); + this.moveCardToLibraryWithInfo((Card) mageObject, source, game, fromZone, toTop, false); } } - return false; } @Override @@ -1319,17 +1315,15 @@ public abstract class PlayerImpl implements Player, Serializable { if (!game.replaceEvent(GameEvent.getEvent(GameEvent.EventType.ACTIVATE_ABILITY, ability.getId(), ability, playerId))) { int bookmark = game.bookmarkState(); - if (ability.activate(game, false)) { - if (ability.resolve(game)) { - if (ability.isUndoPossible()) { - if (storedBookmark == -1 || storedBookmark > bookmark) { // e.g. useful for undo Nykthos, Shrine to Nyx - setStoredBookmark(bookmark); - } - } else { - resetStoredBookmark(game); + if (ability.activate(game, false) && ability.resolve(game)) { + if (ability.isUndoPossible()) { + if (storedBookmark == -1 || storedBookmark > bookmark) { // e.g. useful for undo Nykthos, Shrine to Nyx + setStoredBookmark(bookmark); } - return true; + } else { + resetStoredBookmark(game); } + return true; } restoreState(bookmark, ability.getRule(), game); } @@ -1502,7 +1496,6 @@ public abstract class PlayerImpl implements Player, Serializable { int bookmark = game.bookmarkState(); TriggeredAbility ability = triggeredAbility.copy(); ability.adjustTargets(game); - UUID triggerId = null; if (ability.canChooseTarget(game, playerId)) { if (ability.isUsesStack()) { game.getStack().push(new StackAbility(ability, playerId)); @@ -1574,10 +1567,8 @@ public abstract class PlayerImpl implements Player, Serializable { // rules: // If you cast a split card with fuse from your hand without paying its mana cost, // you can choose to use its fuse ability and cast both halves without paying their mana costs. - if (zone == Zone.HAND) { - if (spellAbility.canChooseTarget(game, playerId)) { - useable.put(spellAbility.getId(), spellAbility); - } + if (zone == Zone.HAND && spellAbility.canChooseTarget(game, playerId)) { + useable.put(spellAbility.getId(), spellAbility); } case SPLIT: if (((SplitCard) object).getLeftHalfCard().getSpellAbility().canChooseTarget(game, playerId)) { @@ -1714,7 +1705,8 @@ public abstract class PlayerImpl implements Player, Serializable { } if (postToLog && !game.isSimulation()) { StringBuilder sb = new StringBuilder(getLogName()).append(" reveals "); - int current = 0, last = cards.size(); + int current = 0; + int last = cards.size(); for (Card card : cards.getCards(game)) { current++; sb.append(GameLog.getColoredObjectName(card)); @@ -1854,7 +1846,7 @@ public abstract class PlayerImpl implements Player, Serializable { } } else { - // player selected an permanent that is restricted by another effect, disallow it (so AI can select another one) + // player selected a permanent that is restricted by another effect, disallow it (so AI can select another one) filter.add(Predicates.not(new PermanentIdPredicate(selectedPermanent.getId()))); if (this.isHuman() && !game.isSimulation()) { game.informPlayer(this, "This permanent can't be untapped because of other restricting effect."); @@ -2181,11 +2173,11 @@ public abstract class PlayerImpl implements Player, Serializable { } } if (combatDamage && sourceAbilities != null && sourceAbilities.containsClass(ToxicAbility.class)) { - int counters = CardUtil + int countersToAdd = CardUtil .castStream(sourceAbilities.stream(), ToxicAbility.class) .mapToInt(ToxicAbility::getAmount) .sum(); - addCounters(CounterType.POISON.createInstance(counters), sourceControllerId, source, game); + addCounters(CounterType.POISON.createInstance(countersToAdd), sourceControllerId, source, game); } // Unstable ability - Earl of Squirrel if (sourceAbilities != null && sourceAbilities.containsKey(SquirrellinkAbility.getInstance().getId())) { @@ -2395,7 +2387,6 @@ public abstract class PlayerImpl implements Player, Serializable { public void concede(Game game) { game.setConcedingPlayer(playerId); lost(game); -// this.left = true; } @Override @@ -2530,7 +2521,7 @@ public abstract class PlayerImpl implements Player, Serializable { opponent.lostForced(game); } } - // if no more opponents alive (independant from range), you win and the game ends + // if no more opponents alive (independent from range), you win and the game ends int opponentsAlive = 0; for (UUID playerIdToCheck : game.getPlayerList()) { if (game.isOpponent(this, playerIdToCheck)) { // Check without range @@ -2759,7 +2750,7 @@ public abstract class PlayerImpl implements Player, Serializable { .filter(card -> card.getAbilities(game).containsClass(WhileSearchingPlayFromLibraryAbility.class)) .map(MageItem::getId) .collect(Collectors.toList()); - if (castableCards.size() == 0) { + if (castableCards.isEmpty()) { return false; } @@ -2776,7 +2767,7 @@ public abstract class PlayerImpl implements Player, Serializable { TargetCard targetCard = new TargetCard(0, 1, Zone.LIBRARY, StaticFilters.FILTER_CARD); targetCard.setTargetName("card to cast from library"); targetCard.setNotTarget(true); - while (castableCards.size() > 0) { + while (!castableCards.isEmpty()) { targetCard.clearChosen(); if (!targetPlayer.choose(Outcome.AIDontUseIt, new CardsImpl(castableCards), targetCard, source, game)) { break; @@ -3251,8 +3242,7 @@ public abstract class PlayerImpl implements Player, Serializable { for (Card card : getHand().getCards(game)) { Abilities manaAbilities = card.getAbilities(game).getAvailableActivatedManaAbilities(Zone.HAND, playerId, game); - for (Iterator it = manaAbilities.iterator(); it.hasNext(); ) { - ActivatedManaAbilityImpl ability = it.next(); + for (ActivatedManaAbilityImpl ability : manaAbilities) { Abilities noTapAbilities = new AbilitiesImpl<>(ability); if (ability.getManaCosts().isEmpty() && !ability.isPoolDependant()) { sourceWithoutManaCosts.add(noTapAbilities); @@ -3344,7 +3334,7 @@ public abstract class PlayerImpl implements Player, Serializable { * and cleared thereafter * * @param netManaAvailable the net mana produced by the triggered mana - * abaility + * ability */ @Override public void addAvailableTriggeredMana(List netManaAvailable @@ -3427,7 +3417,6 @@ public abstract class PlayerImpl implements Player, Serializable { * @param ability * @param availableMana if null, it won't be checked if enough mana is * available - * @param sourceObject * @param game * @return */ @@ -3448,10 +3437,8 @@ public abstract class PlayerImpl implements Player, Serializable { // including an effect that allows a player to cast a spell without paying its mana cost, the alternative cost may be paid. canBeCastRegularly = false; } - if (canBeCastRegularly) { - if (canPayMinimumManaCost(copy, availableMana, game)) { - return true; - } + if (canBeCastRegularly && canPayMinimumManaCost(copy, availableMana, game)) { + return true; } // ALTERNATIVE COST FROM dynamic effects @@ -3498,7 +3485,7 @@ public abstract class PlayerImpl implements Player, Serializable { } // Check for pay option with like phyrexian mana if (getPhyrexianColors() != null) { - addPhyrexianLikePayOptions(abilityOptions, availableMana, game); + addPhyrexianLikePayOptions(abilityOptions); } // Get the ability, if any, which allows for spending many as if it were another color. @@ -3518,7 +3505,7 @@ public abstract class PlayerImpl implements Player, Serializable { // TODO: add tests for non any color like Sunglasses of Urza // TODO: Describe this - // Abilities that let us spend mana as if it were any (or other colors/types) must be handled seperately + // Abilities that let us spend mana as if it were any (or other colors/types) must be handled separately // and can't be incorporated into calculating availableMana since the number of combinations would explode. if (approvingObject != null && mana.count() <= avail.count()) { // TODO: I think this is wrong for spell that require colorless @@ -3537,26 +3524,25 @@ public abstract class PlayerImpl implements Player, Serializable { return false; } - private void addPhyrexianLikePayOptions(ManaOptions abilityOptions, ManaOptions availableMana, Game game) { + private void addPhyrexianLikePayOptions(ManaOptions abilityOptions) { int maxLifeMana = getLife() / 2; if (maxLifeMana > 0) { Set phyrexianOptions = new HashSet<>(); for (Mana mana : abilityOptions) { - int availableLifeMana = maxLifeMana; if (getPhyrexianColors().isBlack()) { - createReducedManaPayOption(availableLifeMana, mana, phyrexianOptions, ManaType.BLACK); + createReducedManaPayOption(maxLifeMana, mana, phyrexianOptions, ManaType.BLACK); } if (getPhyrexianColors().isBlue()) { - createReducedManaPayOption(availableLifeMana, mana, phyrexianOptions, ManaType.BLUE); + createReducedManaPayOption(maxLifeMana, mana, phyrexianOptions, ManaType.BLUE); } if (getPhyrexianColors().isRed()) { - createReducedManaPayOption(availableLifeMana, mana, phyrexianOptions, ManaType.RED); + createReducedManaPayOption(maxLifeMana, mana, phyrexianOptions, ManaType.RED); } if (getPhyrexianColors().isGreen()) { - createReducedManaPayOption(availableLifeMana, mana, phyrexianOptions, ManaType.GREEN); + createReducedManaPayOption(maxLifeMana, mana, phyrexianOptions, ManaType.GREEN); } if (getPhyrexianColors().isWhite()) { - createReducedManaPayOption(availableLifeMana, mana, phyrexianOptions, ManaType.WHITE); + createReducedManaPayOption(maxLifeMana, mana, phyrexianOptions, ManaType.WHITE); } } abilityOptions.addAll(phyrexianOptions); @@ -3581,7 +3567,7 @@ public abstract class PlayerImpl implements Player, Serializable { } /** - * Returns a boolean inidicating if any of the alternative mana costs for the given card can be afforded. + * Returns a boolean indicating if any of the alternative mana costs for the given card can be afforded. * * @param sourceObject The card * @param availableMana The mana available for payments. @@ -3763,7 +3749,7 @@ public abstract class PlayerImpl implements Player, Serializable { } // 707.4.Objects that are cast face down are turned face down before they are put onto the stack - // E.g. no lands per turn limit, no cast restrictions, cost reduce, etc + // E.g. no lands per turn limit, no cast restrictions, cost reduce, etc. // Even mana cost can't be checked here without lookahead // So make it available all the time boolean canUse; @@ -3864,9 +3850,9 @@ public abstract class PlayerImpl implements Player, Serializable { && (isPlaySpell || isPlayLand); // spell/hand abilities (play from all zones) - // need permitingObject or canPlayCardsFromGraveyard + // need permittingObject or canPlayCardsFromGraveyard // zone's abilities (play from specific zone) - // no need in permitingObject + // no need in permittingObject if (fromZone != Zone.ALL && ability.getZone().match(fromZone)) { possibleToPlay = true; } @@ -4710,11 +4696,9 @@ public abstract class PlayerImpl implements Player, Serializable { continue; } boolean chooseOrder = false; - if (userData.askMoveToGraveOrder()) { - if (cards.size() > 1) { - chooseOrder = choosingPlayer.chooseUse(Outcome.Neutral, + if (userData.askMoveToGraveOrder() && (cards.size() > 1)) { + chooseOrder = choosingPlayer.chooseUse(Outcome.Neutral, "Choose the order in which the cards go to the graveyard?", source, game); - } } if (chooseOrder) { TargetCard target = new TargetCard(fromZone, @@ -5142,7 +5126,7 @@ public abstract class PlayerImpl implements Player, Serializable { StaticFilters.FILTER_CONTROLLED_RINGBEARER, getId(),null, game) .stream() - .filter(p -> p != null) + .filter(Objects::nonNull) .findFirst() .orElse(null); } @@ -5158,18 +5142,18 @@ public abstract class PlayerImpl implements Player, Serializable { List ids = game.getBattlefield() .getActivePermanents(StaticFilters.FILTER_CONTROLLED_CREATURE, getId(), null, game) .stream() - .filter(p -> p != null) + .filter(Objects::nonNull) .map(p -> p.getId()) .collect(Collectors.toList()); - if(ids.isEmpty()) { + if (ids.isEmpty()) { game.informPlayers(getLogName() + " has no creature to be Ring-bearer."); return; } // There should always be a creature at the end. UUID newBearerId; - if(ids.size() == 1){ + if (ids.size() == 1) { // Only one creature, it will be the Ring-bearer. // The player does not have to make any choice. newBearerId = ids.get(0); @@ -5195,7 +5179,7 @@ public abstract class PlayerImpl implements Player, Serializable { } } - if(currentBearerId != null && currentBearerId == newBearerId) { + if (currentBearerId != null && currentBearerId == newBearerId) { // Oracle Ruling for Call of the Ring // // If the creature you choose as your Ring-bearer was already your Ring-bearer,