diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsTest.java index d024dea0c2..6e80acc0e3 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/cost/modaldoublefaces/ModalDoubleFacesCardsTest.java @@ -549,4 +549,48 @@ public class ModalDoubleFacesCardsTest extends CardTestPlayerBase { assertPermanentCount(playerA, "Balduvian Bears", 2); } + + @Test + public void test_ETB_OnlySideCardsCanAddAbilitiesToGame() { + // possible bug: double triggers (loadCard adds abilities from main + side cards instead side card only) + // https://github.com/magefree/mage/issues/7187 + + // Skyclave Cleric + // creature 1/3 + // When Skyclave Cleric enters the battlefield, you gain 2 life. + addCard(Zone.HAND, playerA, "Skyclave Cleric"); // {1}{W} + addCard(Zone.BATTLEFIELD, playerA, "Plains", 2); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Skyclave Cleric"); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + assertAllCommandsUsed(); + + assertLife(playerA, 20 + 2); // +2 from 1 etb trigger + } + + @Test + public void test_ETB_OnlyActualSideCardCanRaiseTriggers() { + // possible bug: you play one card but game raise triggers from another side too + // https://github.com/magefree/mage/issues/7180 + + // Kazandu Mammoth + // creature 3/3 + // Landfall — Whenever a land enters the battlefield under your control, Kazandu Mammoth gets +2/+2 until end of turn. + // + // Kazandu Valley + // land + addCard(Zone.HAND, playerA, "Kazandu Mammoth"); // {1}{G}{G} + + // play land, but no landfall triggers from other side + playLand(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Kazandu Valley"); + checkStackSize("no triggers", 1, PhaseStep.PRECOMBAT_MAIN, playerA, 0); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.END_TURN); + execute(); + assertAllCommandsUsed(); + } } \ No newline at end of file diff --git a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java index dabb3552a0..35b1475c08 100644 --- a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java +++ b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java @@ -1149,7 +1149,11 @@ public class TestPlayer implements Player { } private void printAbilities(Game game, List abilities) { - System.out.println("Total abilities: " + (abilities != null ? abilities.size() : 0)); + printAbilities("Total abilities", game, abilities); + } + + private void printAbilities(String info, Game game, List abilities) { + System.out.println(info + ": " + (abilities != null ? abilities.size() : 0)); if (abilities == null) { return; } @@ -1484,6 +1488,9 @@ public class TestPlayer implements Player { } private void assertStackSize(PlayerAction action, Game game, int needStackSize) { + if (game.getStack().size() != needStackSize) { + printAbilities("Current stack", game, game.getStack().stream().map(StackObject::getStackAbility).collect(Collectors.toList())); + } Assert.assertEquals(action.getActionName() + " - stack size must be " + needStackSize + " but is " + game.getStack().size(), needStackSize, game.getStack().size()); } diff --git a/Mage/src/main/java/mage/game/GameImpl.java b/Mage/src/main/java/mage/game/GameImpl.java index ce581eb583..88d73500bd 100644 --- a/Mage/src/main/java/mage/game/GameImpl.java +++ b/Mage/src/main/java/mage/game/GameImpl.java @@ -241,40 +241,43 @@ public abstract class GameImpl implements Game, Serializable { if (card instanceof PermanentCard) { card = ((PermanentCard) card).getCard(); } + + // main card card.setOwnerId(ownerId); - gameCards.put(card.getId(), card); - state.addCard(card); + addCardToState(card); + + // parts if (card instanceof SplitCard) { // left Card leftCard = ((SplitCard) card).getLeftHalfCard(); leftCard.setOwnerId(ownerId); - gameCards.put(leftCard.getId(), leftCard); - state.addCard(leftCard); + addCardToState(leftCard); // right Card rightCard = ((SplitCard) card).getRightHalfCard(); rightCard.setOwnerId(ownerId); - gameCards.put(rightCard.getId(), rightCard); - state.addCard(rightCard); + addCardToState(rightCard); } else if (card instanceof ModalDoubleFacesCard) { // left Card leftCard = ((ModalDoubleFacesCard) card).getLeftHalfCard(); leftCard.setOwnerId(ownerId); - gameCards.put(leftCard.getId(), leftCard); - state.addCard(leftCard); + addCardToState(leftCard); // right Card rightCard = ((ModalDoubleFacesCard) card).getRightHalfCard(); rightCard.setOwnerId(ownerId); - gameCards.put(rightCard.getId(), rightCard); - state.addCard(rightCard); + addCardToState(rightCard); } else if (card instanceof AdventureCard) { Card spellCard = ((AdventureCard) card).getSpellCard(); spellCard.setOwnerId(ownerId); - gameCards.put(spellCard.getId(), spellCard); - state.addCard(spellCard); + addCardToState(spellCard); } } } + private void addCardToState(Card card) { + gameCards.put(card.getId(), card); + state.addCard(card); + } + @Override public Collection getCards() { return gameCards.values(); diff --git a/Mage/src/main/java/mage/game/GameState.java b/Mage/src/main/java/mage/game/GameState.java index 7b135157ad..22bb3e86b4 100644 --- a/Mage/src/main/java/mage/game/GameState.java +++ b/Mage/src/main/java/mage/game/GameState.java @@ -614,7 +614,7 @@ public class GameState implements Serializable, Copyable { } public void addEffect(ContinuousEffect effect, Ability source) { - effects.addEffect(effect, source); + addEffect(effect, null, source); } public void addEffect(ContinuousEffect effect, UUID sourceId, Ability source) { @@ -625,9 +625,17 @@ public class GameState implements Serializable, Copyable { } } -// public void addMessage(String message) { -// this.messages.add(message); -// } + private void addTrigger(TriggeredAbility ability, MageObject attachedTo) { + addTrigger(ability, null, attachedTo); + } + + private void addTrigger(TriggeredAbility ability, UUID sourceId, MageObject attachedTo) { + if (sourceId == null) { + triggers.add(ability, attachedTo); + } else { + triggers.add(ability, sourceId, attachedTo); + } + } /** * Returns a list of all players of the game ignoring range or if a player @@ -839,6 +847,14 @@ 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()) { addAbility(ability, null, card); } @@ -888,7 +904,7 @@ public class GameState implements Serializable, Copyable { } } } else if (ability instanceof TriggeredAbility) { - this.triggers.add((TriggeredAbility) ability, attachedTo); + addTrigger((TriggeredAbility) ability, null, attachedTo); } } @@ -911,8 +927,7 @@ public class GameState implements Serializable, Copyable { } } } else if (ability instanceof TriggeredAbility) { - // TODO: add sources for triggers - the same way as in addEffect: sources - this.triggers.add((TriggeredAbility) ability, sourceId, attachedTo); + addTrigger((TriggeredAbility) ability, sourceId, attachedTo); } List watcherList = new ArrayList<>(ability.getWatchers()); // Workaround to prevent ConcurrentModificationException, not clear to me why this is happening now diff --git a/Mage/src/main/java/mage/game/ZonesHandler.java b/Mage/src/main/java/mage/game/ZonesHandler.java index 25e00db149..8b5f6090c9 100644 --- a/Mage/src/main/java/mage/game/ZonesHandler.java +++ b/Mage/src/main/java/mage/game/ZonesHandler.java @@ -117,25 +117,57 @@ public final class ZonesHandler { Card targetCard = getTargetCard(game, event.getTargetId()); Cards cardsToMove = null; // moving real cards - Cards cardsToUpdate = null; // updating all card's parts + Map cardsToUpdate = new LinkedHashMap<>(); // updating all card's parts (must be ordered LinkedHashMap) + cardsToUpdate.put(toZone, new CardsImpl()); + cardsToUpdate.put(Zone.OUTSIDE, new CardsImpl()); // if we're moving a token it shouldn't be put into any zone as an object. if (!(targetCard instanceof Permanent) && targetCard != null) { if (targetCard instanceof MeldCard) { // meld/group cards must be independent (use can choose order) cardsToMove = ((MeldCard) targetCard).getHalves(); - cardsToUpdate = cardsToMove; + cardsToUpdate.get(toZone).addAll(cardsToMove); } else if (targetCard instanceof ModalDoubleFacesCard || targetCard instanceof ModalDoubleFacesCardHalf) { - // mdf cards must be moved as single object, but each half must be updated separetly + // mdf cards must be moved as single object, but each half must be updated separately ModalDoubleFacesCard mdfCard = (ModalDoubleFacesCard) targetCard.getMainCard(); cardsToMove = new CardsImpl(mdfCard); - cardsToUpdate = new CardsImpl(mdfCard); - cardsToUpdate.add(mdfCard.getLeftHalfCard()); - cardsToUpdate.add(mdfCard.getRightHalfCard()); + cardsToUpdate.get(toZone).add(mdfCard); + // example: cast left side + // result: + // * main to battlefield + // * left to battlefield + // * right to outside (it helps to ignore all triggers and other effects from that card) + switch (toZone) { + case STACK: + case BATTLEFIELD: + if (targetCard.getId().equals(mdfCard.getLeftHalfCard().getId())) { + // play left + cardsToUpdate.get(toZone).add(mdfCard.getLeftHalfCard()); + cardsToUpdate.get(Zone.OUTSIDE).add(mdfCard.getRightHalfCard()); + } else if (targetCard.getId().equals(mdfCard.getRightHalfCard().getId())) { + // play right + cardsToUpdate.get(toZone).add(mdfCard.getRightHalfCard()); + cardsToUpdate.get(Zone.OUTSIDE).add(mdfCard.getLeftHalfCard()); + } else { + // cast mdf (only on stack) + if (!toZone.equals(Zone.STACK)) { + throw new IllegalStateException("Wrong mdf card move to " + toZone + " in placeInDestinationZone"); + } + cardsToUpdate.get(toZone).add(mdfCard.getLeftHalfCard()); + cardsToUpdate.get(toZone).add(mdfCard.getRightHalfCard()); + } + break; + default: + // move all parts + cardsToUpdate.get(toZone).add(mdfCard.getLeftHalfCard()); + cardsToUpdate.get(toZone).add(mdfCard.getRightHalfCard()); + break; + } } else { cardsToMove = new CardsImpl(targetCard); - cardsToUpdate = cardsToMove; + cardsToUpdate.get(toZone).addAll(cardsToMove); } + Player owner = game.getPlayer(targetCard.getOwnerId()); switch (toZone) { case HAND: @@ -208,13 +240,13 @@ public final class ZonesHandler { game.setZone(event.getTargetId(), event.getToZone()); // update zone in other parts (meld cards, mdf half cards) - if (cardsToUpdate != null) { - for (Card card : cardsToUpdate.getCards(game)) { + cardsToUpdate.entrySet().forEach(entry -> { + for (Card card : entry.getValue().getCards(game)) { if (!card.getId().equals(event.getTargetId())) { - game.setZone(card.getId(), event.getToZone()); + game.setZone(card.getId(), entry.getKey()); } } - } + }); // reset meld status if (targetCard instanceof MeldCard) {