Player under control - fixed that it doesn't hide opponent's hand after control lost (part of #13353);

Mage.Tests/src/test/java/org/mage/test/cards/control/PlayerUnderControlTest.java

@@ -0,0 +1,83 @@
+package org.mage.test.cards.control;
+
+import mage.constants.PhaseStep;
+import mage.constants.Zone;
+import mage.view.GameView;
+import org.junit.Assert;
+import org.junit.Test;
+import org.mage.test.serverside.base.CardTestPlayerBase;
+
+/**
+ * Test Framework do not support under control commands, so check only game related info and data
+ *
+ * @author JayDi85
+ */
+public class PlayerUnderControlTest extends CardTestPlayerBase {
+
+    @Test
+    public void test_ClientSideDataMustBeHidden() {
+        // possible bug: after control ends - player still able to view opponent's hands
+
+        // When you cast Emrakul, you gain control of target opponent during that player's next turn.
+        // After that turn, that player takes an extra turn.
+        addCard(Zone.HAND, playerA, "Emrakul, the Promised End"); // {13}
+        addCard(Zone.BATTLEFIELD, playerA, "Forest", 13);
+        //
+        addCard(Zone.HAND, playerB, "Lightning Bolt");
+        addCard(Zone.BATTLEFIELD, playerB, "Mountain", 1);
+
+        // prepare control effect
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Emrakul, the Promised End");
+        addTarget(playerA, playerB);
+        waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN);
+
+        checkUnderControl("turn 1 - A, B normal", 1, PhaseStep.PRECOMBAT_MAIN, false);
+        checkUnderControl("turn 2 - B under A", 2, PhaseStep.PRECOMBAT_MAIN, true);
+        checkUnderControl("turn 3 - A, B normal", 3, PhaseStep.PRECOMBAT_MAIN, false);
+
+        setStrictChooseMode(true);
+        setStopAt(3, PhaseStep.END_TURN);
+        execute();
+    }
+
+    private void checkUnderControl(String info, int turnNum, PhaseStep step, boolean mustHaveControl) {
+        runCode(info, turnNum, step, playerA, (info1, player, game) -> {
+            GameView viewA = getGameView(playerA);
+            GameView viewB = getGameView(playerB);
+            if (mustHaveControl) {
+                Assert.assertTrue(info, playerA.isGameUnderControl());
+                Assert.assertFalse(info, playerB.isGameUnderControl());
+
+                Assert.assertTrue(info, playerA.getPlayersUnderYourControl().contains(playerB.getId()));
+                Assert.assertTrue(info, playerB.getPlayersUnderYourControl().isEmpty());
+
+                Assert.assertTrue(info, playerA.getTurnControllers().isEmpty());
+                Assert.assertTrue(info, playerB.getTurnControllers().contains(playerA.getId()));
+
+                Assert.assertEquals(info, playerA.getTurnControlledBy(), playerA.getId());
+                Assert.assertEquals(info, playerB.getTurnControlledBy(), playerA.getId());
+
+                // client side
+                Assert.assertFalse(info, viewA.getOpponentHands().isEmpty());
+                Assert.assertTrue(info, viewB.getOpponentHands().isEmpty());
+            } else {
+                // A,B normal
+                Assert.assertTrue(info, playerA.isGameUnderControl());
+                Assert.assertTrue(info, playerB.isGameUnderControl());
+
+                Assert.assertTrue(info, playerA.getPlayersUnderYourControl().isEmpty());
+                Assert.assertTrue(info, playerB.getPlayersUnderYourControl().isEmpty());
+
+                Assert.assertTrue(info, playerA.getTurnControllers().isEmpty());
+                Assert.assertTrue(info, playerB.getTurnControllers().isEmpty());
+
+                Assert.assertEquals(info, playerA.getTurnControlledBy(), playerA.getId());
+                Assert.assertEquals(info, playerB.getTurnControlledBy(), playerB.getId());
+
+                // client side
+                Assert.assertTrue(info, viewA.getOpponentHands().isEmpty());
+                Assert.assertTrue(info, viewB.getOpponentHands().isEmpty());
+            }
+        });
+    }
+}

Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java

@@ -3134,13 +3134,13 @@ public class TestPlayer implements Player {
     }
 
     @Override
-    public void setGameUnderYourControl(boolean value) {
+    public void setGameUnderYourControl(Game game, boolean value) {
-        computerPlayer.setGameUnderYourControl(value);
+        computerPlayer.setGameUnderYourControl(game, value);
     }
 
     @Override
-    public void setGameUnderYourControl(boolean value, boolean fullRestore) {
+    public void setGameUnderYourControl(Game game, boolean value, boolean fullRestore) {
-        computerPlayer.setGameUnderYourControl(value, fullRestore);
+        computerPlayer.setGameUnderYourControl(game, value, fullRestore);
     }
 
     @Override

Mage/src/main/java/mage/abilities/AbilityImpl.java

@@ -378,7 +378,7 @@ public abstract class AbilityImpl implements Ability {
 
         // unit tests only: it allows to add targets/choices by two ways:
         // 1. From cast/activate command params (process it here)
-        // 2. From single addTarget/setChoice, it's a preffered method for tests (process it in normal choose dialogs like human player)
+        // 2. From single addTarget/setChoice, it's a preferred method for tests (process it in normal choose dialogs like human player)
         if (controller.isTestsMode()) {
             if (!controller.addTargets(this, game)) {
                 return false;

Mage/src/main/java/mage/abilities/effects/common/LoseControlOnOtherPlayersControllerEffect.java

@@ -1,41 +0,0 @@
-
-
-package mage.abilities.effects.common;
-
-import mage.constants.Outcome;
-import mage.abilities.Ability;
-import mage.abilities.effects.OneShotEffect;
-import mage.game.Game;
-import mage.players.Player;
-
-/**
- * TODO: delete, there are already end turn code with control reset
- * @author nantuko
- */
-public class LoseControlOnOtherPlayersControllerEffect extends OneShotEffect {
-
-    public LoseControlOnOtherPlayersControllerEffect(String controllingPlayerName, String controlledPlayerName) {
-        super(Outcome.Detriment);
-        staticText = controllingPlayerName + " lost control over " + controlledPlayerName;
-    }
-
-    protected LoseControlOnOtherPlayersControllerEffect(final LoseControlOnOtherPlayersControllerEffect effect) {
-        super(effect);
-    }
-
-    @Override
-    public LoseControlOnOtherPlayersControllerEffect copy() {
-        return new LoseControlOnOtherPlayersControllerEffect(this);
-    }
-
-    @Override
-    public boolean apply(Game game, Ability source) {
-        Player player = game.getPlayer(source.getControllerId());
-        if (player != null) {
-            player.resetOtherTurnsControlled();
-            return true;
-        }
-        return false;
-    }
-
-}

Mage/src/main/java/mage/game/GameImpl.java

@@ -1852,7 +1852,7 @@ public abstract class GameImpl implements Game {
                     if (turnController != null) {
                         Player targetPlayer = getPlayer(spellControllerId);
                         if (targetPlayer != null) {
-                            targetPlayer.setGameUnderYourControl(true, false);
+                            targetPlayer.setGameUnderYourControl(this, true, false);
                             informPlayers(turnController.getLogName() + " lost control over " + targetPlayer.getLogName());
                             if (targetPlayer.getTurnControlledBy().equals(turnController.getId())) {
                                 turnController.getPlayersUnderYourControl().remove(targetPlayer.getId());

Mage/src/main/java/mage/game/turn/Turn.java

@@ -233,7 +233,7 @@ public class Turn implements Serializable {
                 if (player != controllingPlayer && controllingPlayer != null) {
                     game.informPlayers(controllingPlayer.getLogName() + " lost control over " + player.getLogName());
                 }
-                player.setGameUnderYourControl(true);
+                player.setGameUnderYourControl(game, true);
             }
         });
 

Mage/src/main/java/mage/players/Player.java

@@ -388,7 +388,7 @@ public interface Player extends MageItem, Copyable<Player> {
      *
      * @param value
      */
-    void setGameUnderYourControl(boolean value);
+    void setGameUnderYourControl(Game game, boolean value);
 
     /**
      * Return player's turn control to prev player
@@ -396,7 +396,7 @@ public interface Player extends MageItem, Copyable<Player> {
      * @param value
      * @param fullRestore return turn control to own
      */
-    void setGameUnderYourControl(boolean value, boolean fullRestore);
+    void setGameUnderYourControl(Game game, boolean value, boolean fullRestore);
 
     void setTestMode(boolean value);
 

Mage/src/main/java/mage/players/PlayerImpl.java

@@ -160,6 +160,7 @@ public abstract class PlayerImpl implements Player, Serializable {
     protected List<AlternativeSourceCosts> alternativeSourceCosts = new ArrayList<>();
 
     // TODO: rework turn controller to use single list (see other todos)
+    //  see PlayerUnderControlTest
     //protected Stack<UUID> allTurnControllers = new Stack<>();
     protected boolean isGameUnderControl = true; // TODO: replace with allTurnControllers.isEmpty
     protected UUID turnController; // null on own control TODO: replace with allTurnControllers.last
@@ -619,7 +620,7 @@ public abstract class PlayerImpl implements Player, Serializable {
         if (!playerUnderControlId.equals(this.getId())) {
             this.playersUnderYourControl.add(playerUnderControlId);
             if (!playerUnderControl.hasLeft() && !playerUnderControl.hasLost()) {
-                playerUnderControl.setGameUnderYourControl(false);
+                playerUnderControl.setGameUnderYourControl(game, false);
             }
             // control will reset on start of the turn
         }
@@ -663,14 +664,15 @@ public abstract class PlayerImpl implements Player, Serializable {
     }
 
     @Override
-    public void setGameUnderYourControl(boolean value) {
+    public void setGameUnderYourControl(Game game, boolean value) {
-        setGameUnderYourControl(value, true);
+        setGameUnderYourControl(game, value, true);
     }
 
     @Override
-    public void setGameUnderYourControl(boolean value, boolean fullRestore) {
+    public void setGameUnderYourControl(Game game, boolean value, boolean fullRestore) {
         this.isGameUnderControl = value;
         if (isGameUnderControl) {
+            removeMeFromPlayersUnderControl(game);
             if (fullRestore) {
                 // to own
                 this.turnControllers.clear();
@@ -687,11 +689,26 @@ public abstract class PlayerImpl implements Player, Serializable {
                 } else {
                     this.turnController = turnControllers.get(turnControllers.size() - 1);
                     isGameUnderControl = false;
+                    addMeToPlayersUnderControl(game, this.turnController);
                 }
             }
         }
     }
 
+    private void removeMeFromPlayersUnderControl(Game game) {
+        game.getPlayers().values().forEach(p -> {
+            p.getPlayersUnderYourControl().remove(this.getId());
+        });
+    }
+
+    private void addMeToPlayersUnderControl(Game game, UUID newTurnController) {
+        game.getPlayers().values().forEach(p -> {
+            if (p.getId().equals(newTurnController)) {
+                p.getPlayersUnderYourControl().add(this.getId());
+            }
+        });
+    }
+
     @Override
     public void endOfTurn(Game game) {
         this.passedTurn = false;

Mage/src/main/java/mage/util/CardUtil.java

@@ -1434,7 +1434,7 @@ public final class CardUtil {
      * @param playerUnderControl
      */
     public static void takeControlUnderPlayerEnd(Game game, Ability source, Player controller, Player playerUnderControl) {
-        playerUnderControl.setGameUnderYourControl(true, false);
+        playerUnderControl.setGameUnderYourControl(game, true, false);
         if (!playerUnderControl.getTurnControlledBy().equals(controller.getId())) {
             game.informPlayers(controller.getLogName() + " return control of the turn to " + playerUnderControl.getLogName() + CardUtil.getSourceLogName(game, source));
             controller.getPlayersUnderYourControl().remove(playerUnderControl.getId());