game engine, tests and other fixes:

tests: fixed wrong permanent structure for battlefield cards (addCard command);
tests: added docs and additional runtime checks;
game: Modal double-faced cards - improved support, no more other side effects on battlefield;
game: Copy abilities - improved stability and cards support;
game: Player under control - improved stability and related cards support (possible NPE errors, additional runtime checks);
server: fixed bloated logs with game timer;
AI: fixed wrong timer in computer games;

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

@@ -9,6 +9,7 @@ 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 {

Mage/src/main/java/mage/cards/ModalDoubleFacedCard.java

@@ -94,15 +94,29 @@ public abstract class ModalDoubleFacedCard extends CardImpl implements CardWithH
     @Override
     public void setCopy(boolean isCopy, MageObject copiedFrom) {
         super.setCopy(isCopy, copiedFrom);
-        leftHalfCard.setCopy(isCopy, copiedFrom);
+        leftHalfCard.setCopy(isCopy, copiedFrom); // TODO: must check copiedFrom and assign sides? (??? related to #8476 ???)
         rightHalfCard.setCopy(isCopy, copiedFrom);
     }
 
+    private void setSideZones(Zone mainZone, Game game) {
+        switch (mainZone) {
+            case BATTLEFIELD:
+            case STACK:
+                throw new IllegalArgumentException("Wrong code usage: you must put to battlefield/stack only real side card (half), not main");
+            default:
+                // must keep both sides in same zone cause xmage need access to cost reduction, spell
+                // and other abilities before put it to stack (in playable calcs)
+                game.setZone(leftHalfCard.getId(), mainZone);
+                game.setZone(rightHalfCard.getId(), mainZone);
+                break;
+        }
+        checkGoodZones(game, this);
+    }
+
     @Override
     public boolean moveToZone(Zone toZone, Ability source, Game game, boolean flag, List<UUID> appliedEffects) {
         if (super.moveToZone(toZone, source, game, flag, appliedEffects)) {
-            game.getState().setZone(leftHalfCard.getId(), toZone);
-            game.getState().setZone(rightHalfCard.getId(), toZone);
+            setSideZones(toZone, game);
             return true;
         }
         return false;
@@ -111,21 +125,69 @@ public abstract class ModalDoubleFacedCard extends CardImpl implements CardWithH
     @Override
     public void setZone(Zone zone, Game game) {
         super.setZone(zone, game);
-        game.setZone(leftHalfCard.getId(), zone);
-        game.setZone(rightHalfCard.getId(), zone);
+        setSideZones(zone, game);
     }
 
     @Override
     public boolean moveToExile(UUID exileId, String name, Ability source, Game game, List<UUID> appliedEffects) {
         if (super.moveToExile(exileId, name, source, game, appliedEffects)) {
-            Zone currentZone = game.getState().getZone(getId());
-            game.getState().setZone(leftHalfCard.getId(), currentZone);
-            game.getState().setZone(rightHalfCard.getId(), currentZone);
+            setSideZones(Zone.EXILED, game);
             return true;
         }
         return false;
     }
 
+    /**
+     * Runtime check for good zones and other MDF data
+     */
+    public static void checkGoodZones(Game game, ModalDoubleFacedCard card) {
+        Card leftPart = card.getLeftHalfCard();
+        Card rightPart = card.getRightHalfCard();
+
+        Zone zoneMain = game.getState().getZone(card.getId());
+        Zone zoneLeft = game.getState().getZone(leftPart.getId());
+        Zone zoneRight = game.getState().getZone(rightPart.getId());
+
+        // runtime check:
+        // * in battlefield and stack - card + one of the sides (another side in outside zone)
+        // * in other zones - card + both sides (need both sides due cost reductions, spell and other access before put to stack)
+        //
+        // 712.8a While a double-faced card is outside the game or in a zone other than the battlefield or stack,
+        // it has only the characteristics of its front face.
+        //
+        // 712.8f While a modal double-faced spell is on the stack or a modal double-faced permanent is on the battlefield,
+        // it has only the characteristics of the face that’s up.
+        Zone needZoneLeft;
+        Zone needZoneRight;
+        switch (zoneMain) {
+            case BATTLEFIELD:
+            case STACK:
+                if (zoneMain == zoneLeft) {
+                    needZoneLeft = zoneMain;
+                    needZoneRight = Zone.OUTSIDE;
+                } else if (zoneMain == zoneRight) {
+                    needZoneLeft = Zone.OUTSIDE;
+                    needZoneRight = zoneMain;
+                } else {
+                    // impossible
+                    needZoneLeft = zoneMain;
+                    needZoneRight = Zone.OUTSIDE;
+                }
+                break;
+            default:
+                needZoneLeft = zoneMain;
+                needZoneRight = zoneMain;
+                break;
+        }
+
+        if (zoneLeft != needZoneLeft || zoneRight != needZoneRight) {
+            throw new IllegalStateException("Wrong code usage: MDF card uses wrong zones - " + card
+                    + "\r\n" + String.format("* main zone: %s", zoneMain)
+                    + "\r\n" + String.format("* left side: need %s, actual %s", needZoneLeft, zoneLeft)
+                    + "\r\n" + String.format("* right side: need %s, actual %s", needZoneRight, zoneRight));
+        }
+    }
+
     @Override
     public boolean removeFromZone(Game game, Zone fromZone, Ability source) {
         // zone contains only one main card

Mage/src/main/java/mage/cards/ModalDoubleFacedCardHalfImpl.java

@@ -71,9 +71,32 @@ public class ModalDoubleFacedCardHalfImpl extends CardImpl implements ModalDoubl
 
     @Override
     public void setZone(Zone zone, Game game) {
+        // see ModalDoubleFacedCard.checkGoodZones for details
         game.setZone(parentCard.getId(), zone);
-        game.setZone(parentCard.getLeftHalfCard().getId(), zone);
-        game.setZone(parentCard.getRightHalfCard().getId(), zone);
+        game.setZone(this.getId(), zone);
+
+        // find another side to sync
+        ModalDoubleFacedCardHalf otherSide;
+        if (!parentCard.getLeftHalfCard().getId().equals(this.getId())) {
+            otherSide = parentCard.getLeftHalfCard();
+        } else if (!parentCard.getRightHalfCard().getId().equals(this.getId())) {
+            otherSide = parentCard.getRightHalfCard();
+        } else {
+            throw new IllegalStateException("Wrong code usage: MDF halves must use different ids");
+        }
+
+        switch (zone) {
+            case STACK:
+            case BATTLEFIELD:
+                // stack and battlefield must have only one side
+                game.setZone(otherSide.getId(), Zone.OUTSIDE);
+                break;
+            default:
+                game.setZone(otherSide.getId(), zone);
+                break;
+        }
+
+        ModalDoubleFacedCard.checkGoodZones(game, parentCard);
     }
 
     @Override

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

@@ -555,7 +555,7 @@ public interface Game extends MageItem, Serializable, Copyable<Game> {
 
     // game cheats (for tests only)
     void cheat(UUID ownerId, Map<Zone, String> commands);
-    void cheat(UUID ownerId, List<Card> library, List<Card> hand, List<PermanentCard> battlefield, List<Card> graveyard, List<Card> command, List<Card> exiled);
+    void cheat(UUID ownerId, List<Card> library, List<Card> hand, List<PutToBattlefieldInfo> battlefield, List<Card> graveyard, List<Card> command, List<Card> exiled);
 
     // controlling the behaviour of replacement effects while permanents entering the battlefield
     void setScopeRelevant(boolean scopeRelevant);

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

@@ -288,6 +288,8 @@ public abstract class GameImpl implements Game {
     public void loadCards(Set<Card> cards, UUID ownerId) {
         for (Card card : cards) {
             if (card instanceof PermanentCard) {
+                // TODO: impossible use case, can be deleted?
+                // trying to put permanent card to battlefield
                 card = ((PermanentCard) card).getCard();
             }
 
@@ -2019,11 +2021,10 @@ public abstract class GameImpl implements Game {
         // save original copy link (handle copy of copies too)
         newBluePrint.setCopy(true, (copyFromPermanent.getCopyFrom() != null ? copyFromPermanent.getCopyFrom() : copyFromPermanent));
 
-        CopyEffect newEffect = new CopyEffect(duration, newBluePrint, copyToPermanentId);
-        newEffect.newId();
-        newEffect.setApplier(applier);
+        CopyEffect newCopyEffect = new CopyEffect(duration, newBluePrint, copyToPermanentId);
+        newCopyEffect.setApplier(applier);
         Ability newAbility = source.copy();
-        newEffect.init(newAbility, this);
+        newCopyEffect.init(newAbility, this);
 
         // If there are already copy effects with duration = Custom to the same object, remove the existing effects because they no longer have any effect
         if (duration == Duration.Custom) {
@@ -2037,7 +2038,7 @@ public abstract class GameImpl implements Game {
                 }
             }
         }
-        state.addEffect(newEffect, newAbility);
+        state.addEffect(newCopyEffect, newAbility);
         return newBluePrint;
     }
 
@@ -3567,20 +3568,27 @@ public abstract class GameImpl implements Game {
     }
 
     @Override
-    public void cheat(UUID ownerId, List<Card> library, List<Card> hand, List<PermanentCard> battlefield, List<Card> graveyard, List<Card> command, List<Card> exiled) {
+    public void cheat(UUID ownerId, List<Card> library, List<Card> hand, List<PutToBattlefieldInfo> battlefield, List<Card> graveyard, List<Card> command, List<Card> exiled) {
         // fake test ability for triggers and events
         Ability fakeSourceAbilityTemplate = new SimpleStaticAbility(Zone.OUTSIDE, new InfoEffect("adding testing cards"));
         fakeSourceAbilityTemplate.setControllerId(ownerId);
 
         Player player = getPlayer(ownerId);
         if (player != null) {
+            // init cards
             loadCards(ownerId, library);
             loadCards(ownerId, hand);
-            loadCards(ownerId, battlefield);
+            loadCards(ownerId, battlefield
+                    .stream()
+                    .map(PutToBattlefieldInfo::getCard)
+                    .collect(Collectors.toList())
+            );
             loadCards(ownerId, graveyard);
             loadCards(ownerId, command);
             loadCards(ownerId, exiled);
 
+            // move cards to zones
+
             for (Card card : library) {
                 player.getLibrary().putOnTop(card, this);
             }
@@ -3610,10 +3618,10 @@ public abstract class GameImpl implements Game {
                 getExile().add(card);
             }
 
-            for (PermanentCard permanentCard : battlefield) {
+            for (PutToBattlefieldInfo info : battlefield) {
                 Ability fakeSourceAbility = fakeSourceAbilityTemplate.copy();
-                fakeSourceAbility.setSourceId(permanentCard.getId());
-                CardUtil.putCardOntoBattlefieldWithEffects(fakeSourceAbility, this, permanentCard, player);
+                fakeSourceAbility.setSourceId(info.getCard().getId());
+                CardUtil.putCardOntoBattlefieldWithEffects(fakeSourceAbility, this, info.getCard(), player, info.isTapped());
             }
 
             applyEffects();

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

@@ -0,0 +1,27 @@
+package mage.game;
+
+import mage.cards.Card;
+
+/**
+ * For tests only: put to battlefield with additional settings like tapped
+ *
+ * @author JayDi85
+ */
+public class PutToBattlefieldInfo {
+
+    private final Card card;
+    private final boolean tapped;
+
+    public PutToBattlefieldInfo(Card card, boolean tapped) {
+        this.card = card;
+        this.tapped = tapped;
+    }
+
+    public Card getCard() {
+        return card;
+    }
+
+    public boolean isTapped() {
+        return tapped;
+    }
+}

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

@@ -439,6 +439,7 @@ public final class ZonesHandler {
         if (Zone.STACK == event.getFromZone()) {
             Spell spell = game.getStack().getSpell(event.getTargetId());
             if (spell != null && !spell.isFaceDown(game)) {
+                // TODO: wtf, why only colors!? Must research and remove colors workaround
                 if (!card.getColor(game).equals(spell.getColor(game))) {
                     // the card that is referenced to in the permanent is copied and the spell attributes are set to this copied card
                     card.getColor(game).setColor(spell.getColor(game));

Mage/src/main/java/mage/game/permanent/PermanentCard.java

@@ -14,6 +14,7 @@ import mage.cards.SplitCard;
 import mage.constants.SpellAbilityType;
 import mage.game.Game;
 import mage.game.events.ZoneChangeEvent;
+import mage.game.permanent.token.Token;
 
 import javax.annotation.processing.SupportedSourceVersion;
 import javax.lang.model.SourceVersion;
@@ -25,18 +26,23 @@ import java.util.UUID;
  *
  * @author BetaSteward_at_googlemail.com
  */
-@SupportedSourceVersion(SourceVersion.RELEASE_8)
 public class PermanentCard extends PermanentImpl {
 
     protected int maxLevelCounters;
-    // A copy of the origin card that was cast (this is not the original card, so it's possible to change some attribute to this blueprint to change attributes to the permanent if it enters the battlefield with e.g. a subtype)
-    protected Card card;
+    // A copy of the original card that was cast (this is not the original card, so it's possible to change some attribute to this blueprint to change attributes to the permanent if it enters the battlefield with e.g. a subtype)
+    protected Card card; // TODO: wtf, it modified on getCard and other places, e.g. on bestow -- must be fixed!
     // the number this permanent instance had
     protected int zoneChangeCounter;
 
     public PermanentCard(Card card, UUID controllerId, Game game) {
         super(card.getId(), card.getOwnerId(), controllerId, card.getName());
 
+        // runtime check: must use real card only inside
+        if (card instanceof PermanentCard) {
+            // TODO: allow?
+            throw new IllegalArgumentException("Wrong code usage: can't use PermanentCard inside another PermanentCard");
+        }
+
         // usage check: you must put to play only real card's part
         // if you use it in test code then call CardUtil.getDefaultCardSideForBattlefield for default side
         // it's a basic check and still allows to create permanent from instant or sorcery
@@ -101,6 +107,7 @@ public class PermanentCard extends PermanentImpl {
     }
 
     protected void copyFromCard(final Card card, final Game game) {
+        // TODO: must research - is it copy all fields or something miss
         this.name = card.getName();
         this.abilities.clear();
         if (this.faceDown) {
@@ -224,6 +231,10 @@ public class PermanentCard extends PermanentImpl {
 
     @Override
     public String toString() {
-        return card.toString();
+        return card.toString()
+                + ", " + ((this instanceof Token) ? "T" : "C")
+                + (this.isCopy() ? ", copy" : "")
+                + ", " + this.getPower() + "/" + this.getToughness()
+                + (this.isTapped() ? ", tapped" : "");
     }
 }

Mage/src/main/java/mage/game/stack/SpellStack.java

@@ -143,6 +143,6 @@ public class SpellStack extends ArrayDeque<StackObject> {
 
     @Override
     public String toString() {
-        return this.size() + (this.isEmpty() ? "" : " (top: " + CardUtil.substring(this.getFirst().toString(), 100) + ")");
+        return this.size() + (this.isEmpty() ? "" : " (top: " + CardUtil.substring(this.getFirst().toString(), 100, "...") + ")");
     }
 }

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

@@ -294,10 +294,8 @@ public interface Player extends MageItem, Copyable<Player> {
     void setTopCardRevealed(boolean topCardRevealed);
 
     /**
-     * Get data from the client Preferences (e.g. avatarId or
-     * showAbilityPickerForce)
-     *
-     * @return
+     * User's settings like avatar or skip buttons.
+     * WARNING, game related code must use controlling player settings only, e.g. getControllingPlayersUserData
      */
     UserData getUserData();
 
@@ -333,6 +331,9 @@ public interface Player extends MageItem, Copyable<Player> {
 
     List<UUID> getTurnControllers();
 
+    /**
+     * Current turn controller for a player (return own id for own control)
+     */
     UUID getTurnControlledBy();
 
     /**
@@ -360,6 +361,11 @@ public interface Player extends MageItem, Copyable<Player> {
      */
     void setGameUnderYourControl(boolean value);
 
+    /**
+     * Return player's turn control to prev player
+     * @param value
+     * @param fullRestore return turn control to own
+     */
     void setGameUnderYourControl(boolean value, boolean fullRestore);
 
     void setTestMode(boolean value);

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

@@ -159,10 +159,12 @@ public abstract class PlayerImpl implements Player, Serializable {
     protected FilterPermanent sacrificeCostFilter;
     protected final List<AlternativeSourceCosts> alternativeSourceCosts = new ArrayList<>();
 
-    protected boolean isGameUnderControl = true;
-    protected UUID turnController;
-    protected List<UUID> turnControllers = new ArrayList<>();
-    protected Set<UUID> playersUnderYourControl = new HashSet<>();
+    // TODO: rework turn controller to use single list (see other todos)
+    //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
+    protected List<UUID> turnControllers = new ArrayList<>(); // TODO: remove
+    protected Set<UUID> playersUnderYourControl = new HashSet<>(); // TODO: replace with game method and search in allTurnControllers
 
     protected Set<UUID> usersAllowedToSeeHandCards = new HashSet<>();
     protected List<UUID> attachments = new ArrayList<>();
@@ -263,14 +265,14 @@ public abstract class PlayerImpl implements Player, Serializable {
         this.storedBookmark = player.storedBookmark;
 
         this.topCardRevealed = player.topCardRevealed;
-        this.playersUnderYourControl.addAll(player.playersUnderYourControl);
         this.usersAllowedToSeeHandCards.addAll(player.usersAllowedToSeeHandCards);
 
         this.isTestMode = player.isTestMode;
-        this.isGameUnderControl = player.isGameUnderControl;
 
+        this.isGameUnderControl = player.isGameUnderControl;
         this.turnController = player.turnController;
         this.turnControllers.addAll(player.turnControllers);
+        this.playersUnderYourControl.addAll(player.playersUnderYourControl);
 
         this.passed = player.passed;
         this.passedTurn = player.passedTurn;
@@ -363,13 +365,14 @@ public abstract class PlayerImpl implements Player, Serializable {
         this.alternativeSourceCosts.addAll(player.getAlternativeSourceCosts());
 
         this.topCardRevealed = player.isTopCardRevealed();
-        this.playersUnderYourControl.clear();
-        this.playersUnderYourControl.addAll(player.getPlayersUnderYourControl());
-        this.isGameUnderControl = player.isGameUnderControl();
 
-        this.turnController = player.getTurnControlledBy();
+        this.isGameUnderControl = player.isGameUnderControl();
+        this.turnController = this.getId().equals(player.getTurnControlledBy()) ? null : player.getTurnControlledBy();
         this.turnControllers.clear();
         this.turnControllers.addAll(player.getTurnControllers());
+        this.playersUnderYourControl.clear();
+        this.playersUnderYourControl.addAll(player.getPlayersUnderYourControl());
+
         this.reachedNextTurnAfterLeaving = player.hasReachedNextTurnAfterLeaving();
 
         this.clearCastSourceIdManaCosts();
@@ -607,6 +610,9 @@ public abstract class PlayerImpl implements Player, Serializable {
 
     @Override
     public void setTurnControlledBy(UUID playerId) {
+        if (playerId == null) {
+            throw new IllegalArgumentException("Can't add unknown player to turn controllers: " + playerId);
+        }
         this.turnController = playerId;
         this.turnControllers.add(playerId);
     }
@@ -618,7 +624,7 @@ public abstract class PlayerImpl implements Player, Serializable {
 
     @Override
     public UUID getTurnControlledBy() {
-        return this.turnController;
+        return this.turnController == null ? this.getId() : this.turnController;
     }
 
     @Override
@@ -647,14 +653,18 @@ public abstract class PlayerImpl implements Player, Serializable {
         this.isGameUnderControl = value;
         if (isGameUnderControl) {
             if (fullRestore) {
+                // to own
                 this.turnControllers.clear();
-                this.turnController = getId();
+                this.turnController = null;
+                this.isGameUnderControl = true;
             } else {
+                // to prev player
                 if (!turnControllers.isEmpty()) {
                     this.turnControllers.remove(turnControllers.size() - 1);
                 }
                 if (turnControllers.isEmpty()) {
-                    this.turnController = getId();
+                    this.turnController = null;
+                    this.isGameUnderControl = true;
                 } else {
                     this.turnController = turnControllers.get(turnControllers.size() - 1);
                     isGameUnderControl = false;
@@ -2515,7 +2525,8 @@ public abstract class PlayerImpl implements Player, Serializable {
     @Override
     public void concede(Game game) {
         game.setConcedingPlayer(playerId);
-        lost(game);
+
+        lost(game); // it's ok to be ignored by "can't lose abilities" here (setConcedingPlayer done all work above)
     }
 
     @Override

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

@@ -1082,16 +1082,26 @@ public final class CardUtil {
     }
 
     /**
-     * Put card to battlefield without resolve (for cheats and tests only)
+     * Put card to battlefield without resolve/ETB (for cheats and tests only)
      *
-     * @param source  must be non null (if you need it empty then use fakeSourceAbility)
+     * @param source  must be non-null (if you need it empty then use fakeSourceAbility)
      * @param game
      * @param newCard
      * @param player
      */
-    public static void putCardOntoBattlefieldWithEffects(Ability source, Game game, Card newCard, Player player) {
+    public static void putCardOntoBattlefieldWithEffects(Ability source, Game game, Card newCard, Player player, boolean tapped) {
         // same logic as ZonesHandler->maybeRemoveFromSourceZone
 
+        // runtime check: must have source
+        if (source == null) {
+            throw new IllegalArgumentException("Wrong code usage: must use source ability or fakeSourceAbility");
+        }
+
+        // runtime check: must use only real cards
+        if (newCard instanceof PermanentCard) {
+            throw new IllegalArgumentException("Wrong code usage: must put to battlefield only real cards, not PermanentCard");
+        }
+
         // workaround to put real permanent from one side (example: you call mdf card by cheats)
         Card permCard = getDefaultCardSideForBattlefield(game, newCard);
 
@@ -1105,15 +1115,16 @@ public final class CardUtil {
             permanent = new PermanentCard(permCard, player.getId(), game);
         }
 
-        // put onto battlefield with possible counters
+        // put onto battlefield with possible counters without ETB
         game.getPermanentsEntering().put(permanent.getId(), permanent);
         permCard.checkForCountersToAdd(permanent, source, game);
         permanent.entersBattlefield(source, game, Zone.OUTSIDE, false);
         game.addPermanent(permanent, game.getState().getNextPermanentOrderNumber());
         game.getPermanentsEntering().remove(permanent.getId());
 
-        // workaround for special tapped status from test framework's command (addCard)
-        if (permCard instanceof PermanentCard && ((PermanentCard) permCard).isTapped()) {
+        // tapped status
+        // warning, "enters the battlefield tapped" abilities will be executed before, so don't set to false here
+        if (tapped) {
             permanent.setTapped(true);
         }