From 9b8df48183bf7fcdb7611212613b60af3b37407f Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Sat, 6 Feb 2021 17:07:10 +0400 Subject: [PATCH] Improved commander support for mdf/split/adventure cards (additional fixes for ac98a3a31ae96372302a71f9dc737ae5e4522c07) --- .../ModalDoubleFacesCardsInCommanderTest.java | 24 ++++++++++- Mage/src/main/java/mage/MageObject.java | 18 +++++++- .../main/java/mage/cards/AdventureCard.java | 6 +++ .../java/mage/cards/ModalDoubleFacesCard.java | 42 +++++++++++++------ Mage/src/main/java/mage/cards/SplitCard.java | 12 +++++- .../src/main/java/mage/game/Controllable.java | 8 ++-- Mage/src/main/java/mage/game/GameImpl.java | 3 ++ Mage/src/main/java/mage/game/GameState.java | 28 +++++++------ .../java/mage/game/command/CommandObject.java | 5 +-- .../java/mage/game/command/Commander.java | 35 ++++++++++------ Mage/src/main/java/mage/game/stack/Spell.java | 10 +++-- .../java/mage/game/stack/StackAbility.java | 17 +++++--- 12 files changed, 148 insertions(+), 60 deletions(-) diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsInCommanderTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsInCommanderTest.java index 9e1df5fe11c..73826e2bf1c 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsInCommanderTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsInCommanderTest.java @@ -53,8 +53,6 @@ public class ModalDoubleFacesCardsInCommanderTest extends CardTestCommanderDuelB checkPermanentCount("prepare", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "The Prismatic Bridge", 1); checkLibraryCount("prepare", 1, PhaseStep.PRECOMBAT_MAIN, playerA, "Grizzly Bears", 5); - // possible bug: you can catch choose dialog for duplicated upkeep triggers - // turn 3, first upkeep and bear move checkLibraryCount("after upkeep 1", 3, PhaseStep.PRECOMBAT_MAIN, playerA, "Grizzly Bears", 5 - 1); checkPermanentCount("after upkeep 1", 3, PhaseStep.PRECOMBAT_MAIN, playerA, "Grizzly Bears", 1); @@ -63,9 +61,31 @@ public class ModalDoubleFacesCardsInCommanderTest extends CardTestCommanderDuelB checkLibraryCount("after upkeep 2", 5, PhaseStep.PRECOMBAT_MAIN, playerA, "Grizzly Bears", 5 - 2); checkPermanentCount("after upkeep 2", 5, PhaseStep.PRECOMBAT_MAIN, playerA, "Grizzly Bears", 2); + setStrictChooseMode(true); + setStopAt(5, PhaseStep.END_TURN); + execute(); // possible bug: you can catch choose dialog for duplicated upkeep triggers + assertAllCommandsUsed(); + } + + @Test + public void test_Triggers_MustWorkFromCommandZone() { + // Oloro, Ageless Ascetic + // At the beginning of your upkeep, if Oloro, Ageless Ascetic is in the command zone, you gain 2 life. + addCard(Zone.COMMAND, playerA, "Oloro, Ageless Ascetic"); + + // turn 1, +2 life on upkeep + checkLife("after upkeep 1", 1, PhaseStep.PRECOMBAT_MAIN, playerA, 40 + 2); + + // turn 3, +2 life on upkeep + checkLife("after upkeep 2", 3, PhaseStep.PRECOMBAT_MAIN, playerA, 40 + 2 + 2); + + // turn 5, +2 life on upkeep + checkLife("after upkeep 3", 5, PhaseStep.PRECOMBAT_MAIN, playerA, 40 + 2 + 2 + 2); + setStrictChooseMode(true); setStopAt(5, PhaseStep.END_TURN); execute(); assertAllCommandsUsed(); } + } diff --git a/Mage/src/main/java/mage/MageObject.java b/Mage/src/main/java/mage/MageObject.java index c1850e822b7..532a23b9837 100644 --- a/Mage/src/main/java/mage/MageObject.java +++ b/Mage/src/main/java/mage/MageObject.java @@ -55,13 +55,27 @@ public interface MageObject extends MageItem, Serializable { Set getSuperType(); /** - * For cards: return basic abilities (without dynamic added) For permanents: - * return all abilities (dynamic ability inserts into permanent) + * For cards: return basic abilities (without dynamic added) + * For permanents: return all abilities (dynamic ability inserts into permanent) * * @return */ Abilities getAbilities(); + /** + * Must return object specific abilities that will be added to the game + * It's a one time action on card/command object putting to the game + *

+ * If your object contains inner cards/parts then each card will be added separately, + * see GameImpl.loadCards. So you must return only parent specific abilities here. All + * other abilities will be added from the parts. + * + * @return + */ + default Abilities getInitAbilities() { + return getAbilities(); + } + boolean hasAbility(Ability ability, Game game); ObjectColor getColor(); diff --git a/Mage/src/main/java/mage/cards/AdventureCard.java b/Mage/src/main/java/mage/cards/AdventureCard.java index 8f268964033..473d53636a1 100644 --- a/Mage/src/main/java/mage/cards/AdventureCard.java +++ b/Mage/src/main/java/mage/cards/AdventureCard.java @@ -101,6 +101,12 @@ public abstract class AdventureCard extends CardImpl { return allAbilities; } + @Override + public Abilities getInitAbilities() { + // must init only parent related abilities, spell card must be init separately + return super.getAbilities(); + } + @Override public Abilities getAbilities(Game game) { Abilities allAbilities = new AbilitiesImpl<>(); diff --git a/Mage/src/main/java/mage/cards/ModalDoubleFacesCard.java b/Mage/src/main/java/mage/cards/ModalDoubleFacesCard.java index cac84c61da0..dda16ba35f5 100644 --- a/Mage/src/main/java/mage/cards/ModalDoubleFacesCard.java +++ b/Mage/src/main/java/mage/cards/ModalDoubleFacesCard.java @@ -174,7 +174,13 @@ public abstract class ModalDoubleFacesCard extends CardImpl { @Override public Abilities getAbilities() { - return getInnerAbilities(false); + return getInnerAbilities(true, true); + } + + @Override + public Abilities getInitAbilities() { + // must init only parent related abilities, spell card must be init separately + return getInnerAbilities(false, false); } public Abilities getSharedAbilities(Game game) { @@ -184,7 +190,7 @@ public abstract class ModalDoubleFacesCard extends CardImpl { @Override public Abilities getAbilities(Game game) { - return getInnerAbilities(game, false); + return getInnerAbilities(game, true, true); } private boolean isIgnoreDefaultAbility(Ability ability) { @@ -200,7 +206,7 @@ public abstract class ModalDoubleFacesCard extends CardImpl { return ability instanceof PlayLandAbility; } - private Abilities getInnerAbilities(Game game, boolean showOnlyMainSide) { + private Abilities getInnerAbilities(Game game, boolean showLeftSide, boolean showRightSide) { Abilities allAbilites = new AbilitiesImpl<>(); for (Ability ability : super.getAbilities(game)) { @@ -210,15 +216,17 @@ public abstract class ModalDoubleFacesCard extends CardImpl { allAbilites.add(ability); } - allAbilites.addAll(leftHalfCard.getAbilities(game)); - if (!showOnlyMainSide) { + if (showLeftSide) { + allAbilites.addAll(leftHalfCard.getAbilities(game)); + } + if (showRightSide) { allAbilites.addAll(rightHalfCard.getAbilities(game)); } return allAbilites; } - private Abilities getInnerAbilities(boolean showOnlyMainSide) { + private Abilities getInnerAbilities(boolean showLeftSide, boolean showRightSide) { Abilities allAbilites = new AbilitiesImpl<>(); for (Ability ability : super.getAbilities()) { @@ -228,8 +236,11 @@ public abstract class ModalDoubleFacesCard extends CardImpl { allAbilites.add(ability); } - allAbilites.addAll(leftHalfCard.getAbilities()); - if (!showOnlyMainSide) { + if (showLeftSide) { + allAbilites.addAll(leftHalfCard.getAbilities()); + } + + if (showRightSide) { allAbilites.addAll(rightHalfCard.getAbilities()); } @@ -240,8 +251,11 @@ public abstract class ModalDoubleFacesCard extends CardImpl { public List getRules() { // rules must show only main side (another side visible by toggle/transform button in GUI) // card hints from both sides - return CardUtil.getCardRulesWithAdditionalInfo(this.getId(), this.getName(), - this.getInnerAbilities(true), this.getInnerAbilities(false) + return CardUtil.getCardRulesWithAdditionalInfo( + this.getId(), + this.getName(), + this.getInnerAbilities(true, false), + this.getInnerAbilities(true, true) ); } @@ -249,8 +263,12 @@ public abstract class ModalDoubleFacesCard extends CardImpl { public List getRules(Game game) { // rules must show only main side (another side visible by toggle/transform button in GUI) // card hints from both sides - return CardUtil.getCardRulesWithAdditionalInfo(game, this.getId(), this.getName(), - this.getInnerAbilities(game, true), this.getInnerAbilities(game, false) + return CardUtil.getCardRulesWithAdditionalInfo( + game, + this.getId(), + this.getName(), + this.getInnerAbilities(game, true, false), + this.getInnerAbilities(game, true, true) ); } diff --git a/Mage/src/main/java/mage/cards/SplitCard.java b/Mage/src/main/java/mage/cards/SplitCard.java index 694897fe251..9bc7a9abc91 100644 --- a/Mage/src/main/java/mage/cards/SplitCard.java +++ b/Mage/src/main/java/mage/cards/SplitCard.java @@ -127,7 +127,8 @@ public abstract class SplitCard extends CardImpl { public Abilities getAbilities() { Abilities allAbilites = new AbilitiesImpl<>(); for (Ability ability : super.getAbilities()) { - // ignore split abilities TODO: why it here, for GUI's cleanup in card texts? Maybe it can be removed + // ignore split abilities + // TODO: why it here, for GUI's cleanup in card texts? Maybe it can be removed, see mdf cards if (ability instanceof SpellAbility && (((SpellAbility) ability).getSpellAbilityType() == SpellAbilityType.SPLIT || ((SpellAbility) ability).getSpellAbilityType() == SpellAbilityType.SPLIT_AFTERMATH)) { @@ -140,6 +141,12 @@ public abstract class SplitCard extends CardImpl { return allAbilites; } + @Override + public Abilities getInitAbilities() { + // must init only full split card aiblities like fuse, parts must be init separately + return super.getAbilities(); + } + /** * Currently only gets the fuse SpellAbility if there is one, but generally * gets any abilities on a split card as a whole, and not on either half @@ -155,7 +162,8 @@ public abstract class SplitCard extends CardImpl { public Abilities getAbilities(Game game) { Abilities allAbilites = new AbilitiesImpl<>(); for (Ability ability : super.getAbilities(game)) { - // ignore split abilities TODO: why it here, for GUI's cleanup in card texts? Maybe it can be removed + // ignore split abilities + // TODO: why it here, for GUI's cleanup in card texts? Maybe it can be removed, see mdf cards if (ability instanceof SpellAbility && (((SpellAbility) ability).getSpellAbilityType() == SpellAbilityType.SPLIT || ((SpellAbility) ability).getSpellAbilityType() == SpellAbilityType.SPLIT_AFTERMATH)) { diff --git a/Mage/src/main/java/mage/game/Controllable.java b/Mage/src/main/java/mage/game/Controllable.java index ac7507e5082..5ab40c4ed9c 100644 --- a/Mage/src/main/java/mage/game/Controllable.java +++ b/Mage/src/main/java/mage/game/Controllable.java @@ -1,5 +1,3 @@ - - package mage.game; import java.util.UUID; @@ -8,11 +6,13 @@ import java.util.UUID; * @author magenoxx_at_gmail.com */ public interface Controllable { + UUID getControllerId(); + UUID getId(); - default boolean isControlledBy(UUID controllerID){ - if(getControllerId() == null){ + default boolean isControlledBy(UUID controllerID) { + if (getControllerId() == null) { return false; } return getControllerId().equals(controllerID); diff --git a/Mage/src/main/java/mage/game/GameImpl.java b/Mage/src/main/java/mage/game/GameImpl.java index 58882578519..d25672f2cca 100644 --- a/Mage/src/main/java/mage/game/GameImpl.java +++ b/Mage/src/main/java/mage/game/GameImpl.java @@ -244,6 +244,9 @@ public abstract class GameImpl implements Game, Serializable { card = ((PermanentCard) card).getCard(); } + // init each card by parts... if you add new type here then getInitAbilities must be + // implemented too (it allows to split abilities between card and parts) + // main card card.setOwnerId(ownerId); addCardToState(card); diff --git a/Mage/src/main/java/mage/game/GameState.java b/Mage/src/main/java/mage/game/GameState.java index c05e519b903..85848a7825b 100644 --- a/Mage/src/main/java/mage/game/GameState.java +++ b/Mage/src/main/java/mage/game/GameState.java @@ -847,14 +847,8 @@ public class GameState implements Serializable, Copyable { public void addCard(Card card) { setZone(card.getId(), Zone.OUTSIDE); - // dirty hack to fix double triggers, see https://github.com/magefree/mage/issues/7187 - // main mdf card don't have attached abilities, only parts contains it - if (card instanceof ModalDoubleFacesCard) { - return; - } - - // add card's abilities to game - for (Ability ability : card.getAbilities()) { + // add card specific abilities to game + for (Ability ability : card.getInitAbilities()) { addAbility(ability, null, card); } } @@ -912,10 +906,10 @@ public class GameState implements Serializable, Copyable { * span * * @param ability - * @param sourceId + * @param sourceId - if source object can be moved between zones then you must set it here (each game cycle clear all source related triggers) * @param attachedTo */ - public void addAbility(Ability ability, UUID sourceId, Card attachedTo) { + public void addAbility(Ability ability, UUID sourceId, MageObject attachedTo) { if (ability instanceof StaticAbility) { for (UUID modeId : ability.getModes().getSelectedModes()) { Mode mode = ability.getModes().get(modeId); @@ -932,7 +926,13 @@ public class GameState implements Serializable, Copyable { List watcherList = new ArrayList<>(ability.getWatchers()); // Workaround to prevent ConcurrentModificationException, not clear to me why this is happening now for (Watcher watcher : watcherList) { // TODO: Check that watcher for commanderAbility (where attachedTo = null) also work correctly - watcher.setControllerId(attachedTo == null ? ability.getControllerId() : attachedTo.getOwnerId()); + UUID controllerId = ability.getControllerId(); + if (attachedTo instanceof Card) { + controllerId = ((Card) attachedTo).getOwnerId(); + } else if (attachedTo instanceof Controllable) { + controllerId = ((Controllable) attachedTo).getControllerId(); + } + watcher.setControllerId(controllerId); watcher.setSourceId(attachedTo == null ? ability.getSourceId() : attachedTo.getId()); watchers.add(watcher); } @@ -944,7 +944,7 @@ public class GameState implements Serializable, Copyable { public void addDesignation(Designation designation, Game game, UUID controllerId) { getDesignations().add(designation); - for (Ability ability : designation.getAbilities()) { + for (Ability ability : designation.getInitAbilities()) { ability.setControllerId(controllerId); addAbility(ability, designation.getId(), null); } @@ -967,7 +967,9 @@ public class GameState implements Serializable, Copyable { public void addCommandObject(CommandObject commandObject) { getCommand().add(commandObject); setZone(commandObject.getId(), Zone.COMMAND); - for (Ability ability : commandObject.getAbilities()) { + + // must add only command object specific abilities, all other abilities adds from card parts (on loadCards) + for (Ability ability : commandObject.getInitAbilities()) { addAbility(ability, commandObject); } } diff --git a/Mage/src/main/java/mage/game/command/CommandObject.java b/Mage/src/main/java/mage/game/command/CommandObject.java index 2a376c6b3f0..8659aba37ba 100644 --- a/Mage/src/main/java/mage/game/command/CommandObject.java +++ b/Mage/src/main/java/mage/game/command/CommandObject.java @@ -1,12 +1,11 @@ - package mage.game.command; -import java.util.UUID; import mage.MageObject; import mage.game.Controllable; +import java.util.UUID; + /** - * * @author Viserion, nantuko */ public interface CommandObject extends MageObject, Controllable { diff --git a/Mage/src/main/java/mage/game/command/Commander.java b/Mage/src/main/java/mage/game/command/Commander.java index b3a8446792c..1b783d1e80d 100644 --- a/Mage/src/main/java/mage/game/command/Commander.java +++ b/Mage/src/main/java/mage/game/command/Commander.java @@ -19,10 +19,7 @@ import mage.game.events.ZoneChangeEvent; import mage.util.GameLog; import mage.util.SubTypes; -import java.util.ArrayList; -import java.util.List; -import java.util.Set; -import java.util.UUID; +import java.util.*; public class Commander implements CommandObject { @@ -34,6 +31,11 @@ public class Commander implements CommandObject { public Commander(Card card) { this.sourceObject = card; + // All abilities must be added to the game before usage. It adding by addCard and addCommandObject calls + // Example: if commander from mdf card then + // * commander object adds cast/play as commander abilities + // * sourceObject adds normal cast/play abilities and all other things + // replace spell ability by commander cast spell (to cast from command zone) for (Ability ability : card.getAbilities()) { if (ability instanceof SpellAbility) { @@ -78,14 +80,7 @@ public class Commander implements CommandObject { continue; } - // skip triggers - // workaround to fix double triggers for commanders on battlefield (example: Esika, God of the Tree) - // TODO: is commanders on command zone can have triggers (is there a card with triggered ability in all zones)? - if (ability instanceof TriggeredAbility) { - continue; - } - - // OK, can add it (example: activated, static, alternative cost, etc) + // all other abilities must be added to commander (example: triggers from command zone, alternative cost, etc) Ability newAbility = ability.copy(); abilities.add(newAbility); } @@ -186,6 +181,22 @@ public class Commander implements CommandObject { return abilities; } + @Override + public Abilities getInitAbilities() { + // see commander contruction comments for more info + + // collect ignore list + Set ignore = new HashSet<>(); + sourceObject.getAbilities().forEach(ability -> ignore.add(ability.getId())); + + // return only object specific abilities + Abilities res = new AbilitiesImpl<>(); + this.getAbilities().stream() + .filter(ability -> !ignore.contains(ability.getId())) + .forEach(res::add); + return res; + } + @Override public boolean hasAbility(Ability ability, Game game) { if (this.getAbilities().contains(ability)) { diff --git a/Mage/src/main/java/mage/game/stack/Spell.java b/Mage/src/main/java/mage/game/stack/Spell.java index 25b13a94855..9030c886ca6 100644 --- a/Mage/src/main/java/mage/game/stack/Spell.java +++ b/Mage/src/main/java/mage/game/stack/Spell.java @@ -4,10 +4,7 @@ import mage.MageInt; import mage.MageObject; import mage.Mana; import mage.ObjectColor; -import mage.abilities.Abilities; -import mage.abilities.Ability; -import mage.abilities.Mode; -import mage.abilities.SpellAbility; +import mage.abilities.*; import mage.abilities.costs.AlternativeSourceCosts; import mage.abilities.costs.mana.ActivationManaAbilityStep; import mage.abilities.costs.mana.ManaCost; @@ -570,6 +567,11 @@ public class Spell extends StackObjImpl implements Card { return card.getAbilities(); } + @Override + public Abilities getInitAbilities() { + return new AbilitiesImpl<>(); + } + @Override public Abilities getAbilities(Game game) { return card.getAbilities(game); diff --git a/Mage/src/main/java/mage/game/stack/StackAbility.java b/Mage/src/main/java/mage/game/stack/StackAbility.java index 615e70ee8f7..5627022adf2 100644 --- a/Mage/src/main/java/mage/game/stack/StackAbility.java +++ b/Mage/src/main/java/mage/game/stack/StackAbility.java @@ -44,12 +44,12 @@ import java.util.UUID; */ public class StackAbility extends StackObjImpl implements Ability { - private static ArrayList emptyCardType = new ArrayList<>(); - private static List emptyString = new ArrayList<>(); - private static ObjectColor emptyColor = new ObjectColor(); - private static ManaCosts emptyCost = new ManaCostsImpl<>(); - private static Costs emptyCosts = new CostsImpl<>(); - private static Abilities emptyAbilites = new AbilitiesImpl<>(); + private static final ArrayList emptyCardType = new ArrayList<>(); + private static final List emptyString = new ArrayList<>(); + private static final ObjectColor emptyColor = new ObjectColor(); + private static final ManaCosts emptyCost = new ManaCostsImpl<>(); + private static final Costs emptyCosts = new CostsImpl<>(); + private static final Abilities emptyAbilites = new AbilitiesImpl<>(); private final Ability ability; private UUID controllerId; @@ -186,6 +186,11 @@ public class StackAbility extends StackObjImpl implements Ability { return new AbilitiesImpl<>(ability); } + @Override + public Abilities getInitAbilities() { + return new AbilitiesImpl<>(); + } + @Override public boolean hasAbility(Ability ability, Game game) { return false;