From 8098dd690c2f7cfe89fd5d495509bbdfa6a41d8c Mon Sep 17 00:00:00 2001 From: LevelX2 Date: Mon, 28 Dec 2020 18:36:05 +0100 Subject: [PATCH] * Fixed that effects with custom duration are not automatically removed from the game if source permanents leaves the game (fixes #6997). --- Mage.Sets/src/mage/cards/a/ArborElf.java | 2 + .../continuous/PlayerLeavesGameTest.java | 6 +- .../PlayerLeftGameRangeAllTest.java | 62 ++++++++++++++++++- .../effects/ContinuousEffectsList.java | 19 +++--- 4 files changed, 76 insertions(+), 13 deletions(-) diff --git a/Mage.Sets/src/mage/cards/a/ArborElf.java b/Mage.Sets/src/mage/cards/a/ArborElf.java index f5ca63aee9..34ede348e8 100644 --- a/Mage.Sets/src/mage/cards/a/ArborElf.java +++ b/Mage.Sets/src/mage/cards/a/ArborElf.java @@ -31,6 +31,8 @@ public final class ArborElf extends CardImpl { this.power = new MageInt(1); this.toughness = new MageInt(1); + + // (T): Untap target Forest. Ability ability = new SimpleActivatedAbility(Zone.BATTLEFIELD, new UntapTargetEffect(), new TapSourceCost()); TargetLandPermanent target = new TargetLandPermanent(filter); ability.addTarget(target); diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/continuous/PlayerLeavesGameTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/continuous/PlayerLeavesGameTest.java index c041a278aa..e5d710d3a0 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/continuous/PlayerLeavesGameTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/continuous/PlayerLeavesGameTest.java @@ -110,7 +110,11 @@ public class PlayerLeavesGameTest extends CardTestMultiPlayerBaseWithRangeAll { @Test public void test_PlayerLeaveGameWithOwnPermanentAndCustomEffect() { - prepareAndRunLeaveGameWithLongEffectTest(Duration.Custom); + // Using Custom duration without implementing specific duration rules makes no sense + // This conflicts rule 800.4a (only any effects which give that player control of any objects or players end.) + // See PlayerLeftGameRangeAllTest.TestContinuousEffectStaysAfterCreatingPlayerLeft as example why custom effects may not end, if the source permanent of the effect leaves the game + + // prepareAndRunLeaveGameWithLongEffectTest(Duration.Custom); } @Test diff --git a/Mage.Tests/src/test/java/org/mage/test/multiplayer/PlayerLeftGameRangeAllTest.java b/Mage.Tests/src/test/java/org/mage/test/multiplayer/PlayerLeftGameRangeAllTest.java index d01e7938ed..361728e324 100644 --- a/Mage.Tests/src/test/java/org/mage/test/multiplayer/PlayerLeftGameRangeAllTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/multiplayer/PlayerLeftGameRangeAllTest.java @@ -1,5 +1,6 @@ package org.mage.test.multiplayer; +import java.io.FileNotFoundException; import mage.constants.MultiplayerAttackOption; import mage.constants.PhaseStep; import mage.constants.RangeOfInfluence; @@ -13,8 +14,6 @@ import org.junit.Assert; import org.junit.Test; import org.mage.test.serverside.base.CardTestMultiPlayerBase; -import java.io.FileNotFoundException; - /** * @author LevelX2 */ @@ -405,7 +404,7 @@ public class PlayerLeftGameRangeAllTest extends CardTestMultiPlayerBase { setStopAt(4, PhaseStep.BEGIN_COMBAT); execute(); - Assert.assertFalse("Player D is no longer in the game", playerA.isInGame()); + Assert.assertFalse("Player A is no longer in the game", playerA.isInGame()); assertLife(playerA, 2); assertPermanentCount(playerD, "Juggernaut", 1); @@ -418,4 +417,61 @@ public class PlayerLeftGameRangeAllTest extends CardTestMultiPlayerBase { } + /** + * https://github.com/magefree/mage/issues/6997 + Some continuous effects should stay in play even after the player that set them leaves the game. + Example: + * Player A: Casts Vorinclex, Voice of Hunger + * Player D: Taps all lands and do stuff (lands shouldn't untap during his next untap step) + * Player C: Kills Player A Player D: Lands untapped normally, though they shouldn't + * + * This happened playing commander against 3 AIs. One of the AIs played Vorinclex, I tapped all my lands during my turn to do stuff. + * Next AI killed the one that had Vorinclex. When the game got to my turn, my lands untapped normally. + */ + @Test + public void TestContinuousEffectStaysAfterCreatingPlayerLeft() { + // Player order: A -> D -> C -> B + setStrictChooseMode(true); + + addCard(Zone.BATTLEFIELD, playerA, "Forest", 8); + // Trample + // Whenever you tap a land for mana, add one mana of any type that land produced. + // Whenever an opponent taps a land for mana, that land doesn't untap during its controller's next untap step. + addCard(Zone.HAND, playerA, "Vorinclex, Voice of Hunger"); // Creature 7/6 {6}{G}{G} + + addCard(Zone.HAND, playerD, "Silvercoat Lion"); + addCard(Zone.BATTLEFIELD, playerD, "Plains", 2); + + addCard(Zone.HAND, playerC, "Lightning Bolt", 1); + addCard(Zone.BATTLEFIELD, playerC, "Mountain", 1); + + addCard(Zone.HAND, playerB, "Silvercoat Lion", 1); + addCard(Zone.BATTLEFIELD, playerB, "Plains", 2); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Vorinclex, Voice of Hunger"); + + castSpell(2, PhaseStep.PRECOMBAT_MAIN, playerD, "Silvercoat Lion"); + + setChoice(playerA, "Whenever an opponent taps a land for mana"); + castSpell(3, PhaseStep.PRECOMBAT_MAIN, playerC, "Lightning Bolt", playerA); + + setStopAt(5, PhaseStep.BEGIN_COMBAT); + + execute(); + + assertAllCommandsUsed(); + + assertPermanentCount(playerD, "Silvercoat Lion", 1); + + assertGraveyardCount(playerC, "Lightning Bolt", 1); + + Assert.assertFalse("Player A is no longer in the game", playerA.isInGame()); + + Assert.assertTrue("Player D is the active player",currentGame.getActivePlayerId().equals(playerD.getId())); + + assertTappedCount("Plains", true, 2); // Do not untap because of Vorinclex do not untap effect + + } + + } diff --git a/Mage/src/main/java/mage/abilities/effects/ContinuousEffectsList.java b/Mage/src/main/java/mage/abilities/effects/ContinuousEffectsList.java index c3e2cd3429..a8e118d3ca 100644 --- a/Mage/src/main/java/mage/abilities/effects/ContinuousEffectsList.java +++ b/Mage/src/main/java/mage/abilities/effects/ContinuousEffectsList.java @@ -1,5 +1,6 @@ package mage.abilities.effects; +import java.util.*; import mage.MageObject; import mage.abilities.Ability; import mage.abilities.MageSingleton; @@ -10,8 +11,6 @@ import mage.game.Game; import mage.players.Player; import org.apache.log4j.Logger; -import java.util.*; - /** * @param * @author BetaSteward_at_googlemail.com @@ -121,10 +120,10 @@ public class ContinuousEffectsList extends ArrayList // They neither expire immediately nor last indefinitely. MageObject object = game.getObject(ability.getSourceId()); boolean isObjectInGame = ability.getSourceId() == null || object != null; // Commander effects have no sourceId - boolean isOwnerLeaveGame = false; + boolean hasOwnerLeftGame = false; if (object instanceof Card) { Player owner = game.getPlayer(((Card) object).getOwnerId()); - isOwnerLeaveGame = !owner.isInGame(); + hasOwnerLeftGame = !owner.isInGame(); } switch (effect.getDuration()) { @@ -136,18 +135,20 @@ public class ContinuousEffectsList extends ArrayList case EndOfCombat: case EndOfGame: // if the related source object does no longer exist in game - the effect has to be removed - if (isOwnerLeaveGame || !isObjectInGame) { + if (hasOwnerLeftGame || !isObjectInGame) { it.remove(); } break; case OneUse: - if (isOwnerLeaveGame || effect.isUsed()) { + if (hasOwnerLeftGame || effect.isUsed()) { it.remove(); } break; case Custom: - // custom effects must process it's own inactive method (override), but can'be missied by devs - if (isOwnerLeaveGame || effect.isInactive(ability, game)) { + // custom effects must process it's own inactive method (override) + // custom effects may not end, if the source permanent of the effect has left the game + // 800.4a (only any effects which give that player control of any objects or players end) + if (effect.isInactive(ability, game)) { it.remove(); } break; @@ -166,7 +167,7 @@ public class ContinuousEffectsList extends ArrayList } break; case UntilSourceLeavesBattlefield: - if (isOwnerLeaveGame || Zone.BATTLEFIELD != game.getState().getZone(ability.getSourceId())) { + if (hasOwnerLeftGame || Zone.BATTLEFIELD != game.getState().getZone(ability.getSourceId())) { it.remove(); } break;