From 8db7a4d307a50cec9509d06b0e61cc3bb691085a Mon Sep 17 00:00:00 2001 From: Susucre <34709007+Susucre@users.noreply.github.com> Date: Fri, 17 May 2024 13:33:45 +0200 Subject: [PATCH 1/4] cleanup DelayedTriggeredAbility on player leave --- ...iggerLastingAfterControllerLeavesTest.java | 67 +++++++++++++++++++ .../abilities/DelayedTriggeredAbilities.java | 8 +-- .../abilities/DelayedTriggeredAbility.java | 8 ++- .../effects/ContinuousEffectImpl.java | 40 +++-------- 4 files changed, 88 insertions(+), 35 deletions(-) create mode 100644 Mage.Tests/src/test/java/org/mage/test/multiplayer/DelayedTriggerLastingAfterControllerLeavesTest.java diff --git a/Mage.Tests/src/test/java/org/mage/test/multiplayer/DelayedTriggerLastingAfterControllerLeavesTest.java b/Mage.Tests/src/test/java/org/mage/test/multiplayer/DelayedTriggerLastingAfterControllerLeavesTest.java new file mode 100644 index 00000000000..bb8c0e61b5f --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/multiplayer/DelayedTriggerLastingAfterControllerLeavesTest.java @@ -0,0 +1,67 @@ +package org.mage.test.multiplayer; + +import mage.constants.MultiplayerAttackOption; +import mage.constants.PhaseStep; +import mage.constants.RangeOfInfluence; +import mage.constants.Zone; +import mage.game.FreeForAll; +import mage.game.Game; +import mage.game.GameException; +import mage.game.mulligan.MulliganType; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestMultiPlayerBase; + +import java.io.FileNotFoundException; + +/** + * @author Susucr + */ +public class DelayedTriggerLastingAfterControllerLeavesTest extends CardTestMultiPlayerBase { + + @Override + protected Game createNewGameAndPlayers() throws GameException, FileNotFoundException { + Game game = new FreeForAll(MultiplayerAttackOption.MULTIPLE, RangeOfInfluence.ALL, MulliganType.GAME_DEFAULT.getMulligan(0), 40, 7); + // Player order: A -> D -> C -> B + playerA = createPlayer(game, "PlayerA"); + playerB = createPlayer(game, "PlayerB"); + playerC = createPlayer(game, "PlayerC"); + playerD = createPlayer(game, "PlayerD"); + return game; + } + + @Test + public void test_UntilYourNextTurn_AfterLeave() { + setStrictChooseMode(true); + + // +1: Until your next turn, whenever a creature an opponent controls attacks, it gets -1/-0 until end of turn. + addCard(Zone.BATTLEFIELD, playerA, "Jace, Architect of Thought"); + + addCard(Zone.BATTLEFIELD, playerD, "Grizzly Bears"); + addCard(Zone.BATTLEFIELD, playerD, "Elite Vanguard"); + addCard(Zone.BATTLEFIELD, playerC, "Grizzly Bears"); + addCard(Zone.BATTLEFIELD, playerC, "Elite Vanguard"); + + activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "+1:"); + + attack(2, playerD, "Grizzly Bears", playerB); + attack(2, playerD, "Elite Vanguard", playerB); + setChoice(playerA, "Until your next turn"); // order trigger + + checkLife("2: after D attack affected by Delayed triggers", 2, PhaseStep.POSTCOMBAT_MAIN, playerB, 40 - 2); + concede(2, PhaseStep.END_TURN, playerA); + + attack(3, playerC, "Grizzly Bears", playerB); + attack(3, playerC, "Elite Vanguard", playerB); + + checkLife("3: after C attack affected by Delayed triggers", 3, PhaseStep.POSTCOMBAT_MAIN, playerB, 40 - 2 - 4); + // No trigger, as triggers from leaved players don't trigger + + attack(5, playerD, "Grizzly Bears", playerB); + attack(5, playerD, "Elite Vanguard", playerB); + + setStopAt(5, PhaseStep.POSTCOMBAT_MAIN); + execute(); + + assertLife(playerB, 40 - 2 - 4 - 4); + } +} diff --git a/Mage/src/main/java/mage/abilities/DelayedTriggeredAbilities.java b/Mage/src/main/java/mage/abilities/DelayedTriggeredAbilities.java index d0a5a20dcd6..23af7412404 100644 --- a/Mage/src/main/java/mage/abilities/DelayedTriggeredAbilities.java +++ b/Mage/src/main/java/mage/abilities/DelayedTriggeredAbilities.java @@ -27,11 +27,9 @@ public class DelayedTriggeredAbilities extends AbilitiesImpl it = this.iterator(); it.hasNext(); ) { DelayedTriggeredAbility ability = it.next(); - if (ability.getDuration() == Duration.Custom) { - if (ability.isInactive(game)) { - it.remove(); - continue; - } + if (ability.isInactive(game)) { + it.remove(); + continue; } if (!ability.checkEventType(event, game)) { continue; diff --git a/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java b/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java index 15d9a52a61c..d7cc29d9e77 100644 --- a/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java +++ b/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java @@ -4,6 +4,7 @@ import mage.abilities.effects.Effect; import mage.constants.Duration; import mage.constants.Zone; import mage.game.Game; +import mage.players.Player; /** * @author BetaSteward_at_googlemail.com @@ -64,6 +65,11 @@ public abstract class DelayedTriggeredAbility extends TriggeredAbilityImpl { } public boolean isInactive(Game game) { - return false; + // discard as soon as possible for leaved player + // 800.4d. If an object that would be owned by a player who has left the game would be created in any zone, it isn't created. + // If a triggered ability that would be controlled by a player who has left the game would be put onto the stack, it isn't put on the stack. + Player player = game.getPlayer(getControllerId()); + boolean canDelete = player == null || (!player.isInGame() && player.hasReachedNextTurnAfterLeaving()); + return canDelete; } } diff --git a/Mage/src/main/java/mage/abilities/effects/ContinuousEffectImpl.java b/Mage/src/main/java/mage/abilities/effects/ContinuousEffectImpl.java index bb0aeabf1d6..ebd2bada333 100644 --- a/Mage/src/main/java/mage/abilities/effects/ContinuousEffectImpl.java +++ b/Mage/src/main/java/mage/abilities/effects/ContinuousEffectImpl.java @@ -310,22 +310,12 @@ public abstract class ContinuousEffectImpl extends EffectImpl implements Continu return false; } - boolean canDelete; Player player = game.getPlayer(startingControllerId); - // discard on start of turn for leaved player // 800.4i When a player leaves the game, any continuous effects with durations that last until that player's next turn // or until a specific point in that turn will last until that turn would have begun. // They neither expire immediately nor last indefinitely. - switch (duration) { - case UntilYourNextTurn: - case UntilEndOfYourNextTurn: - canDelete = player == null || (!player.isInGame() && player.hasReachedNextTurnAfterLeaving()); - break; - default: - canDelete = false; - } - + boolean canDelete = player == null || (!player.isInGame() && player.hasReachedNextTurnAfterLeaving()); if (canDelete) { return true; } @@ -333,28 +323,20 @@ public abstract class ContinuousEffectImpl extends EffectImpl implements Continu // discard on another conditions (start of your turn) switch (duration) { case UntilYourNextTurn: - if (player != null && player.isInGame()) { - return this.isYourNextTurn(game); - } - break; + return this.isYourNextTurn(game); case UntilYourNextEndStep: - if (player != null && player.isInGame()) { - return this.isYourNextEndStep(game); - } - break; + return this.isYourNextEndStep(game); case UntilEndCombatOfYourNextTurn: - if (player != null && player.isInGame()) { - return this.isEndCombatOfYourNextTurn(game); - } - break; + return this.isEndCombatOfYourNextTurn(game); case UntilYourNextUpkeepStep: - if (player != null && player.isInGame()) { - return this.isYourNextUpkeepStep(game); - } - break; + return this.isYourNextUpkeepStep(game); + case UntilEndOfYourNextTurn: + // cleanup handled by ContinuousEffectsList::removeEndOfTurnEffects + // TODO: should those be aligned to all be handled in the same place? + return false; + default: // Should handle all the duration that do pass the first switch. + throw new IllegalStateException("Missing case for isInactive's Duration:" + duration); } - - return canDelete; } @Override From 65dce362b44f21fcdeaab3bd88f7abae3600c723 Mon Sep 17 00:00:00 2001 From: Susucre <34709007+Susucre@users.noreply.github.com> Date: Fri, 17 May 2024 13:42:03 +0200 Subject: [PATCH 2/4] rename test file --- ...est.java => DelayedTriggerAfterControllerLeavesTest.java} | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) rename Mage.Tests/src/test/java/org/mage/test/multiplayer/{DelayedTriggerLastingAfterControllerLeavesTest.java => DelayedTriggerAfterControllerLeavesTest.java} (87%) diff --git a/Mage.Tests/src/test/java/org/mage/test/multiplayer/DelayedTriggerLastingAfterControllerLeavesTest.java b/Mage.Tests/src/test/java/org/mage/test/multiplayer/DelayedTriggerAfterControllerLeavesTest.java similarity index 87% rename from Mage.Tests/src/test/java/org/mage/test/multiplayer/DelayedTriggerLastingAfterControllerLeavesTest.java rename to Mage.Tests/src/test/java/org/mage/test/multiplayer/DelayedTriggerAfterControllerLeavesTest.java index bb8c0e61b5f..11dee4bac20 100644 --- a/Mage.Tests/src/test/java/org/mage/test/multiplayer/DelayedTriggerLastingAfterControllerLeavesTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/multiplayer/DelayedTriggerAfterControllerLeavesTest.java @@ -14,9 +14,12 @@ import org.mage.test.serverside.base.CardTestMultiPlayerBase; import java.io.FileNotFoundException; /** + * 800.4d. If an object that would be owned by a player who has left the game would be created in any zone, it isn't created. + * If a triggered ability that would be controlled by a player who has left the game would be put onto the stack, it isn't put on the stack. + * * @author Susucr */ -public class DelayedTriggerLastingAfterControllerLeavesTest extends CardTestMultiPlayerBase { +public class DelayedTriggerAfterControllerLeavesTest extends CardTestMultiPlayerBase { @Override protected Game createNewGameAndPlayers() throws GameException, FileNotFoundException { From a2344ea78184248e14f66641f3751c788d9e7ba9 Mon Sep 17 00:00:00 2001 From: Susucre <34709007+Susucre@users.noreply.github.com> Date: Fri, 17 May 2024 16:37:18 +0200 Subject: [PATCH 3/4] fix cleanup time --- Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java b/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java index d7cc29d9e77..fd389a0e8db 100644 --- a/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java +++ b/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java @@ -69,7 +69,7 @@ public abstract class DelayedTriggeredAbility extends TriggeredAbilityImpl { // 800.4d. If an object that would be owned by a player who has left the game would be created in any zone, it isn't created. // If a triggered ability that would be controlled by a player who has left the game would be put onto the stack, it isn't put on the stack. Player player = game.getPlayer(getControllerId()); - boolean canDelete = player == null || (!player.isInGame() && player.hasReachedNextTurnAfterLeaving()); + boolean canDelete = player == null || !player.isInGame(); return canDelete; } } From a4eb96383866c652616eb6893679cf2a95060317 Mon Sep 17 00:00:00 2001 From: Susucre <34709007+Susucre@users.noreply.github.com> Date: Sat, 18 May 2024 19:23:50 +0200 Subject: [PATCH 4/4] handle delayed triggers not using the stack --- .../java/mage/abilities/DelayedTriggeredAbility.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java b/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java index fd389a0e8db..c57293926d7 100644 --- a/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java +++ b/Mage/src/main/java/mage/abilities/DelayedTriggeredAbility.java @@ -65,11 +65,16 @@ public abstract class DelayedTriggeredAbility extends TriggeredAbilityImpl { } public boolean isInactive(Game game) { - // discard as soon as possible for leaved player + // discard on stack // 800.4d. If an object that would be owned by a player who has left the game would be created in any zone, it isn't created. // If a triggered ability that would be controlled by a player who has left the game would be put onto the stack, it isn't put on the stack. Player player = game.getPlayer(getControllerId()); - boolean canDelete = player == null || !player.isInGame(); - return canDelete; + if (player != null && player.isInGame()) { + return false; + } + // If using the stack, discard as soon as possible for leaved player + // If not using the stack (for instance return of "exile target player until {this} leaves the battlefield"), + // we wait till the player would have played a next turn to make sure they are debounced after the player leaves. + return usesStack || player.hasReachedNextTurnAfterLeaving(); } }