Merge pull request #7021 from magefree/pvs-studio-2020-fixes

Fixes from pvs-studio's article

Mage.Client/src/main/java/mage/client/util/gui/GuiDisplayUtil.java

@@ -191,12 +191,8 @@ public final class GuiDisplayUtil {
         // counters
         if (card.getMageObjectType().canHaveCounters()) {
             java.util.List<CounterView> counters = new ArrayList<>();
-            if (card instanceof PermanentView) {
+            if (card.getCounters() != null) {
-                if (card.getCounters() != null) {
+                counters.addAll(card.getCounters());
-                    counters = new ArrayList<>(card.getCounters());
-                }
-            } else if (card.getCounters() != null) {
-                counters = new ArrayList<>(card.getCounters());
             }
             if (!counters.isEmpty()) {
                 StringBuilder sb = new StringBuilder();
@@ -406,9 +402,7 @@ public final class GuiDisplayUtil {
     }
 
     private static String replaceNamesInRule(String rule, String cardName) {
-        String res = rule.replaceAll("\\{this\\}", cardName.isEmpty() ? "this" : cardName);
+        return rule.replaceAll("\\{this\\}", cardName.isEmpty() ? "this" : cardName);
-        res = res.replaceAll("\\{source\\}", cardName.isEmpty() ? "this" : cardName);
-        return res;
     }
 
     private static String getResourcePath(String image) {

Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/AltMtgOnlTokensImageSource.java

@@ -151,7 +151,18 @@ public enum AltMtgOnlTokensImageSource implements CardImageSource {
         return true;
     }
 
+    @Override
+    public boolean isCardSource() {
+        return false;
+    }
+
     @Override
     public void doPause(String httpImageUrl) {
     }
+
+    @Override
+    public boolean isCardImageProvided(String setCode, String cardName) {
+        // no cards support, only tokens
+        return false;
+    }
 }

Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/CardImageSource.java

@@ -32,9 +32,9 @@ public interface CardImageSource {
         return 0;
     }
 
-    default boolean isTokenSource() {
+    boolean isCardSource();
-        return false;
+
-    }
+    boolean isTokenSource();
 
     default boolean isLanguagesSupport() {
         return false;
@@ -53,13 +53,8 @@ public interface CardImageSource {
         return new ArrayList<>();
     }
 
-    default boolean isSetSupportedComplete(String setCode) {
-        return true;
-    }
 
-    default boolean isCardImageProvided(String setCode, String cardName) {
+    boolean isCardImageProvided(String setCode, String cardName);
-        return false;
-    }
 
     default boolean isTokenImageProvided(String setCode, String cardName, Integer tokenNumber) {
         return false;

Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/CopyPasteImageSource.java

@@ -21,8 +21,8 @@ public enum CopyPasteImageSource implements CardImageSource {
 
     instance;
 
-    private Set<String> supportedSets = new LinkedHashSet<String>();
+    private final Set<String> supportedSets = new LinkedHashSet<String>();
-    private Set<String> missingCards = new LinkedHashSet<String>();
+    private final Set<String> missingCards = new LinkedHashSet<String>();
     Map<String, String> singleLinks = null;
     boolean loadedFromDialog = false;
     boolean viewMissingCards = true;
@@ -221,7 +221,12 @@ public enum CopyPasteImageSource implements CardImageSource {
 
     @Override
     public boolean isTokenSource() {
-        return false;
+        return true;
+    }
+
+    @Override
+    public boolean isCardSource() {
+        return true;
     }
 
     @Override
@@ -254,9 +259,4 @@ public enum CopyPasteImageSource implements CardImageSource {
     public boolean isTokenImageProvided(String setCode, String cardName, Integer tokenNumber) {
         return false;
     }
-
-    @Override
-    public boolean isSetSupportedComplete(String setCode) {
-        return false;
-    }
 }

Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/GrabbagImageSource.java

@@ -463,6 +463,11 @@ public enum GrabbagImageSource implements CardImageSource {
         return true;
     }
 
+    @Override
+    public boolean isCardSource() {
+        return true;
+    }
+
     private String getSourceName(CardDownloadData card, String httpImageUrl) {
         if (card.getSet().equals("MTG")) {
             return "http://static.starcitygames.com/sales/cardscans/";
@@ -495,10 +500,4 @@ public enum GrabbagImageSource implements CardImageSource {
         }
         return singleLinks.containsKey(setCode + "/" + cardName) || singleLinks.containsKey(setCode + "/" + cardName + "-a");
     }
-
-    @Override
-    public boolean isSetSupportedComplete(String setCode) {
-        return false;
-    }
-
 }

Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/MagidexImageSource.java

@@ -284,6 +284,11 @@ public enum MagidexImageSource implements CardImageSource {
         return false;
     }
 
+    @Override
+    public boolean isCardSource() {
+        return true;
+    }
+
     @Override
     public void doPause(String httpImageUrl) {
     }
@@ -292,4 +297,9 @@ public enum MagidexImageSource implements CardImageSource {
     public List<String> getSupportedSets() {
         return new ArrayList<>(supportedSets);
     }
+
+    @Override
+    public boolean isCardImageProvided(String setCode, String cardName) {
+        return supportedSets.contains(setCode);
+    }
 }

Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/MtgOnlTokensImageSource.java

@@ -354,6 +354,11 @@ public enum MtgOnlTokensImageSource implements CardImageSource {
         return true;
     }
 
+    @Override
+    public boolean isCardSource() {
+        return false;
+    }
+
     @Override
     public void doPause(String httpImageUrl) {
     }

Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/MythicspoilerComSource.java

@@ -445,6 +445,11 @@ public enum MythicspoilerComSource implements CardImageSource {
         return false;
     }
 
+    @Override
+    public boolean isCardSource() {
+        return true;
+    }
+
     @Override
     public void doPause(String httpImageUrl) {
     }
@@ -454,4 +459,8 @@ public enum MythicspoilerComSource implements CardImageSource {
         return new ArrayList<>(supportedSets);
     }
 
+    @Override
+    public boolean isCardImageProvided(String setCode, String cardName) {
+        return supportedSets.contains(setCode);
+    }
 }

Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/ScryfallImageSource.java

@@ -219,6 +219,11 @@ public enum ScryfallImageSource implements CardImageSource {
         return true;
     }
 
+    @Override
+    public boolean isCardSource() {
+        return true;
+    }
+
     @Override
     public boolean isLanguagesSupport() {
         return true;

Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/TokensMtgImageSource.java

@@ -146,6 +146,11 @@ public enum TokensMtgImageSource implements CardImageSource {
         return true;
     }
 
+    @Override
+    public boolean isCardSource() {
+        return false;
+    }
+
     @Override
     public void doPause(String httpImageUrl) {
     }
@@ -157,6 +162,7 @@ public enum TokensMtgImageSource implements CardImageSource {
 
     @Override
     public boolean isCardImageProvided(String setCode, String cardName) {
+        // no cards support, only tokens
         return false;
     }
 
@@ -175,11 +181,6 @@ public enum TokensMtgImageSource implements CardImageSource {
         return (tokensData.containsKey(key));
     }
 
-    @Override
-    public boolean isSetSupportedComplete(String setCode) {
-        return false;
-    }
-
     private HashMap<String, List<TokenData>> getTokensData() throws IOException {
         synchronized (tokensDataSync) {
             if (tokensData == null) {

Mage.Client/src/main/java/org/mage/plugins/card/dl/sources/WizardCardsImageSource.java

@@ -690,6 +690,11 @@ public enum WizardCardsImageSource implements CardImageSource {
         return false;
     }
 
+    @Override
+    public boolean isCardSource() {
+        return true;
+    }
+
     @Override
     public boolean isLanguagesSupport() {
         return true;
@@ -714,4 +719,9 @@ public enum WizardCardsImageSource implements CardImageSource {
         return new ArrayList<>(supportedSets);
     }
 
+    @Override
+    public boolean isCardImageProvided(String setCode, String cardName) {
+        return supportedSets.contains(setCode);
+    }
+
 }

Mage.Client/src/main/java/org/mage/plugins/card/images/DownloadPicturesService.java

@@ -272,9 +272,11 @@ public class DownloadPicturesService extends DefaultBoundedRangeModel implements
         List<String> setNames = new ArrayList<>();
 
         // multiple sets selection
-        setNames.add(ALL_IMAGES);
+        if (selectedSource.isCardSource()) {
-        setNames.add(ALL_MODERN_IMAGES);
+            setNames.add(ALL_IMAGES);
-        setNames.add(ALL_STANDARD_IMAGES);
+            setNames.add(ALL_MODERN_IMAGES);
+            setNames.add(ALL_STANDARD_IMAGES);
+        }
         if (selectedSource.isTokenSource()) {
             setNames.add(ALL_TOKENS);
         }
@@ -338,18 +340,17 @@ public class DownloadPicturesService extends DefaultBoundedRangeModel implements
         int numberCardImagesAvailable = 0;
         for (CardDownloadData data : cardsMissing) {
             if (data.isToken()) {
-                if (selectedSource.isTokenSource() && selectedSource.isTokenImageProvided(data.getSet(), data.getName(), data.getType())) {
+                if (selectedSource.isTokenSource()
+                        && selectedSource.isTokenImageProvided(data.getSet(), data.getName(), data.getType())) {
                     numberTokenImagesAvailable++;
                     cardsDownloadQueue.add(data);
-                } else {
-                    //logger.warn("Source do not support token (set " + data.getSet() + ", token " + data.getName() + ")");
                 }
             } else {
-                if (selectedSets != null && selectedSets.contains(data.getSet())) {
+                if (selectedSource.isCardSource()
-                    if (selectedSource.isSetSupportedComplete(data.getSet()) || selectedSource.isCardImageProvided(data.getSet(), data.getName())) {
+                        && selectedSource.isCardImageProvided(data.getSet(), data.getName())
-                        numberCardImagesAvailable++;
+                        && selectedSets.contains(data.getSet())) {
-                        cardsDownloadQueue.add(data);
+                    numberCardImagesAvailable++;
-                    }
+                    cardsDownloadQueue.add(data);
                 }
             }
         }

Mage.Server/src/main/java/mage/server/MageServerImpl.java

@@ -201,7 +201,7 @@ public class MageServerImpl implements MageServer {
                     // check if user can create another table
                     int notStartedTables = user.getNumberOfNotStartedTables();
                     if (notStartedTables > 1) {
-                        user.showUserMessage("Create table", "You have already " + notStartedTables + " not started table" + (notStartedTables == 1 ? "" : "s") + ". You can't create another.");
+                        user.showUserMessage("Create table", "You have already " + notStartedTables + " not started tables. You can't create another.");
                         throw new MageException("No message");
                     }
                     // check AI players max
@@ -1102,7 +1102,7 @@ public class MageServerImpl implements MageServer {
         execute("removeTable", sessionId, () -> {
             SessionManager.instance.getSession(sessionId).ifPresent(session -> {
                 UUID userId = session.getUserId();
-                TableManager.instance.removeTable(userId, tableId);
+                TableManager.instance.removeTable(userId, tableId); // delete account
             });
         });
     }
@@ -1327,7 +1327,7 @@ public class MageServerImpl implements MageServer {
             // check if user can create another table
             int notStartedTables = user.getNumberOfNotStartedTables();
             if (notStartedTables > 1) {
-                user.showUserMessage("Create table", "You have already " + notStartedTables + " not started table" + (notStartedTables == 1 ? "" : "s") + ". You can't create another.");
+                user.showUserMessage("Create table", "You have already " + notStartedTables + " not started tables. You can't create another.");
                 throw new MageException("No message");
             }
             // check if the user itself satisfies the quitRatio requirement.

Mage.Server/src/main/java/mage/server/Session.java

@@ -1,17 +1,11 @@
 package mage.server;
 
-import java.util.*;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.locks.ReentrantLock;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
 import mage.MageException;
 import mage.constants.Constants;
 import mage.interfaces.callback.ClientCallback;
 import mage.interfaces.callback.ClientCallbackMethod;
 import mage.players.net.UserData;
 import mage.players.net.UserGroup;
-import static mage.server.DisconnectReason.LostConnection;
 import mage.server.game.GamesRoom;
 import mage.server.game.GamesRoomManager;
 import mage.server.util.ConfigSettings;
@@ -23,6 +17,14 @@ import org.jboss.remoting.callback.Callback;
 import org.jboss.remoting.callback.HandleCallbackException;
 import org.jboss.remoting.callback.InvokerCallbackHandler;
 
+import java.util.*;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import static mage.server.DisconnectReason.LostConnection;
+
 /**
  * @author BetaSteward_at_googlemail.com
  */
@@ -201,9 +203,11 @@ public class Session {
                 }
             }
         }
+
         Optional<User> selectUser = UserManager.instance.createUser(userName, host, authorizedUser);
         boolean reconnect = false;
-        if (!selectUser.isPresent()) {  // user already exists
+        if (!selectUser.isPresent()) {
+            // user already connected
             selectUser = UserManager.instance.getUserByName(userName);
             if (selectUser.isPresent()) {
                 User user = selectUser.get();
@@ -222,6 +226,9 @@ public class Session {
                 } else {
                     return "User name " + userName + " already in use (or your IP address changed)";
                 }
+            } else {
+                // code never goes here
+                return "Can't find connected user name " + userName;
             }
         }
         User user = selectUser.get();
@@ -240,7 +247,6 @@ public class Session {
         }
 
         return null;
-
     }
 
     public void connectAdmin() {

Mage.Server/src/main/java/mage/server/TableManager.java

@@ -1,17 +1,5 @@
-
 package mage.server;
 
-import java.text.DateFormat;
-import java.text.SimpleDateFormat;
-import java.util.*;
-import java.util.Map.Entry;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.Executors;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.locks.Lock;
-import java.util.concurrent.locks.ReadWriteLock;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
 import mage.MageException;
 import mage.cards.decks.DeckCardLists;
 import mage.constants.TableState;
@@ -31,6 +19,18 @@ import mage.server.game.GamesRoomManager;
 import mage.server.util.ThreadExecutor;
 import org.apache.log4j.Logger;
 
+import java.text.DateFormat;
+import java.text.SimpleDateFormat;
+import java.util.*;
+import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.Executors;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+
 /**
  * @author BetaSteward_at_googlemail.com
  */
@@ -244,7 +244,7 @@ public enum TableManager {
     /**
      * Starts the Match from a non tournament table
      *
-     * @param userId table owner
+     * @param userId  table owner
      * @param roomId
      * @param tableId
      */
@@ -415,7 +415,7 @@ public enum TableManager {
                 if (table.getState() != TableState.FINISHED
                         && ((System.currentTimeMillis() - table.getStartTime().getTime()) / 1000) > 30) { // removeUserFromAllTablesAndChat only if table started longer than 30 seconds ago
                     // removeUserFromAllTablesAndChat tables and games not valid anymore
-                    logger.debug(table.getId() + " [" + table.getName() + "] " + formatter.format(table.getStartTime() == null ? table.getCreateTime() : table.getCreateTime()) + " (" + table.getState().toString() + ") " + (table.isTournament() ? "- Tournament" : ""));
+                    logger.debug(table.getId() + " [" + table.getName() + "] " + formatter.format(table.getStartTime() != null ? table.getStartTime() : table.getCreateTime()) + " (" + table.getState().toString() + ") " + (table.isTournament() ? "- Tournament" : ""));
                     getController(table.getId()).ifPresent(tableController -> {
                         if ((table.isTournament() && !tableController.isTournamentStillValid())
                                 || (!table.isTournament() && !tableController.isMatchTableStillValid())) {

Mage.Sets/src/mage/cards/t/TorrentialGearhulk.java

@@ -1,6 +1,5 @@
 package mage.cards.t;
 
-import java.util.UUID;
 import mage.ApprovingObject;
 import mage.MageInt;
 import mage.abilities.Ability;
@@ -12,11 +11,7 @@ import mage.abilities.keyword.FlashAbility;
 import mage.cards.Card;
 import mage.cards.CardImpl;
 import mage.cards.CardSetInfo;
-import mage.constants.CardType;
+import mage.constants.*;
-import mage.constants.Duration;
-import mage.constants.Outcome;
-import mage.constants.SubType;
-import mage.constants.Zone;
 import mage.filter.FilterCard;
 import mage.game.Game;
 import mage.game.events.GameEvent;
@@ -24,10 +19,10 @@ import mage.game.events.ZoneChangeEvent;
 import mage.players.Player;
 import mage.target.common.TargetCardInYourGraveyard;
 import mage.target.targetpointer.FixedTarget;
-import org.apache.log4j.Logger;
+
+import java.util.UUID;
 
 /**
- *
  * @author fireshoes
  */
 public final class TorrentialGearhulk extends CardImpl {
@@ -85,23 +80,18 @@ class TorrentialGearhulkEffect extends OneShotEffect {
     @Override
     public boolean apply(Game game, Ability source) {
         Player controller = game.getPlayer(source.getControllerId());
-        if (controller != null) {
+        Card card = game.getCard(this.getTargetPointer().getFirst(game, source));
-            Card card = game.getCard(this.getTargetPointer().getFirst(game, source));
+        if (controller != null && card != null) {
-            if (card != null) {
+            if (controller.chooseUse(outcome, "Cast " + card.getLogName() + '?', source, game)) {
-                if (controller.chooseUse(outcome, "Cast " + card.getLogName() + '?', source, game)) {
+                game.getState().setValue("PlayFromNotOwnHandZone" + card.getId(), Boolean.TRUE);
-                    game.getState().setValue("PlayFromNotOwnHandZone" + card.getId(), Boolean.TRUE);
+                boolean cardWasCast = controller.cast(controller.chooseAbilityForCast(card, game, true),
-                    Boolean cardWasCast = controller.cast(controller.chooseAbilityForCast(card, game, true),
+                        game, true, new ApprovingObject(source, game));
-                            game, true, new ApprovingObject(source, game));
+                game.getState().setValue("PlayFromNotOwnHandZone" + card.getId(), null);
-                    game.getState().setValue("PlayFromNotOwnHandZone" + card.getId(), null);
+                if (cardWasCast) {
-                    if (cardWasCast) {
+                    ContinuousEffect effect = new TorrentialGearhulkReplacementEffect(card.getId());
-                        ContinuousEffect effect = new TorrentialGearhulkReplacementEffect(card.getId());
+                    effect.setTargetPointer(new FixedTarget(card.getId(), game.getState().getZoneChangeCounter(card.getId())));
-                        effect.setTargetPointer(new FixedTarget(card.getId(), game.getState().getZoneChangeCounter(card.getId())));
+                    game.addEffect(effect, source);
-                        game.addEffect(effect, source);
-                    }
                 }
-            } else if (card != null) {
-                Logger.getLogger(TorrentialGearhulkEffect.class).error("Torrential Gearhulk - Instant card without spellAbility : " + card.getName());
-                return false;
             }
             return true;
         }

Mage.Sets/src/mage/cards/v/VigeanIntuition.java

@@ -1,9 +1,5 @@
 package mage.cards.v;
 
-import java.util.LinkedHashSet;
-import java.util.Set;
-import java.util.UUID;
-
 import mage.MageObject;
 import mage.abilities.Ability;
 import mage.abilities.effects.OneShotEffect;
@@ -17,15 +13,18 @@ import mage.game.Game;
 import mage.players.Library;
 import mage.players.Player;
 
+import java.util.LinkedHashSet;
+import java.util.Set;
+import java.util.UUID;
+
 /**
- *
  * @author noahg
  */
 public final class VigeanIntuition extends CardImpl {
 
     public VigeanIntuition(UUID ownerId, CardSetInfo setInfo) {
         super(ownerId, setInfo, new CardType[]{CardType.INSTANT}, "{3}{G}{U}");
-        
+
 
         // Choose a card type, then reveal the top four cards of your library. Put all cards of the chosen type revealed this way into your hand and the rest into your graveyard.
         this.getSpellAbility().addEffect(new VigeanIntuitionEffect());
@@ -74,48 +73,55 @@ class VigeanIntuitionEffect extends OneShotEffect {
     public boolean apply(Game game, Ability source) {
         MageObject sourceObject = game.getObject(source.getSourceId());
         Player player = game.getPlayer(source.getControllerId());
+        if (sourceObject == null || player == null) {
+            return false;
+        }
         Library library = player.getLibrary();
-        if (player != null && sourceObject != null && library != null) {
+        if (library == null) {
-            Choice choiceImpl = new ChoiceImpl();
+            return false;
-            choiceImpl.setChoices(choice);
+        }
-            if (player.choose(Outcome.Neutral, choiceImpl, game)) {
-                CardType type = null;
-                String choosenType = choiceImpl.getChoice();
 
-                if (choosenType.equals(CardType.ARTIFACT.toString())) {
+        Choice choiceImpl = new ChoiceImpl();
-                    type = CardType.ARTIFACT;
+        choiceImpl.setChoices(choice);
-                } else if (choosenType.equals(CardType.LAND.toString())) {
+        if (player.choose(Outcome.Neutral, choiceImpl, game)) {
-                    type = CardType.LAND;
+            String choosenType = choiceImpl.getChoice();
-                } else if (choosenType.equals(CardType.CREATURE.toString())) {
+            if (choosenType == null || choosenType.isEmpty()) {
-                    type = CardType.CREATURE;
+                return false;
-                } else if (choosenType.equals(CardType.ENCHANTMENT.toString())) {
+            }
-                    type = CardType.ENCHANTMENT;
+            CardType type = null;
-                } else if (choosenType.equals(CardType.INSTANT.toString())) {
+            if (choosenType.equals(CardType.ARTIFACT.toString())) {
-                    type = CardType.INSTANT;
+                type = CardType.ARTIFACT;
-                } else if (choosenType.equals(CardType.SORCERY.toString())) {
+            } else if (choosenType.equals(CardType.LAND.toString())) {
-                    type = CardType.SORCERY;
+                type = CardType.LAND;
-                } else if (choosenType.equals(CardType.PLANESWALKER.toString())) {
+            } else if (choosenType.equals(CardType.CREATURE.toString())) {
-                    type = CardType.PLANESWALKER;
+                type = CardType.CREATURE;
-                } else if (choosenType.equals(CardType.TRIBAL.toString())) {
+            } else if (choosenType.equals(CardType.ENCHANTMENT.toString())) {
-                    type = CardType.TRIBAL;
+                type = CardType.ENCHANTMENT;
-                }
+            } else if (choosenType.equals(CardType.INSTANT.toString())) {
+                type = CardType.INSTANT;
+            } else if (choosenType.equals(CardType.SORCERY.toString())) {
+                type = CardType.SORCERY;
+            } else if (choosenType.equals(CardType.PLANESWALKER.toString())) {
+                type = CardType.PLANESWALKER;
+            } else if (choosenType.equals(CardType.TRIBAL.toString())) {
+                type = CardType.TRIBAL;
+            }
 
-                if (type != null) {
+            if (type != null) {
-                    Set<Card> top = library.getTopCards(game, 4);
+                Set<Card> top = library.getTopCards(game, 4);
-                    player.revealCards(source, new CardsImpl(top), game);
+                player.revealCards(source, new CardsImpl(top), game);
-                    Cards putInHand = new CardsImpl();
+                Cards putInHand = new CardsImpl();
-                    Cards putInGraveyard = new CardsImpl();
+                Cards putInGraveyard = new CardsImpl();
-                    for (Card card : top) {
+                for (Card card : top) {
-                        if (card != null && card.getCardType().contains(type)) {
+                    if (card != null && card.getCardType().contains(type)) {
-                            putInHand.add(card);
+                        putInHand.add(card);
-                        } else {
+                    } else {
-                            putInGraveyard.add(card);
+                        putInGraveyard.add(card);
-                        }
                     }
-                    player.moveCards(putInHand, Zone.HAND, source, game);
-                    player.moveCards(putInGraveyard, Zone.GRAVEYARD, source, game);
-                    return true;
                 }
+                player.moveCards(putInHand, Zone.HAND, source, game);
+                player.moveCards(putInGraveyard, Zone.GRAVEYARD, source, game);
+                return true;
             }
         }
         return false;

Mage.Sets/src/mage/sets/DragonsMaze.java

@@ -1,8 +1,5 @@
-
 package mage.sets;
 
-import java.util.ArrayList;
-import java.util.List;
 import mage.cards.ExpansionSet;
 import mage.cards.repository.CardCriteria;
 import mage.cards.repository.CardInfo;
@@ -11,8 +8,10 @@ import mage.constants.CardType;
 import mage.constants.Rarity;
 import mage.constants.SetType;
 
+import java.util.ArrayList;
+import java.util.List;
+
 /**
- *
  * @author LevelX2
  */
 public final class DragonsMaze extends ExpansionSet {
@@ -23,7 +22,7 @@ public final class DragonsMaze extends ExpansionSet {
         return instance;
     }
 
-    private List<CardInfo> savedSpecialRares = new ArrayList<>();
+    private final List<CardInfo> savedSpecialRares = new ArrayList<>();
 
     private DragonsMaze() {
         super("Dragon's Maze", "DGM", ExpansionSet.buildDate(2013, 5, 3), SetType.EXPANSION);
@@ -224,42 +223,27 @@ public final class DragonsMaze extends ExpansionSet {
 
     @Override
     public List<CardInfo> getSpecialRare() {
-        if (savedSpecialRares == null) {
+        if (savedSpecialRares.isEmpty()) {
-            CardCriteria criteria = new CardCriteria();
+            fillSpecialRares("GTC", "Breeding Pool");
-            criteria.setCodes("GTC").name("Breeding Pool");
+            fillSpecialRares("GTC", "Godless Shrine");
-            savedSpecialRares.addAll(CardRepository.instance.findCards(criteria));
+            fillSpecialRares("GTC", "Sacred Foundry");
-            criteria = new CardCriteria();
+            fillSpecialRares("GTC", "Stomping Ground");
-            criteria.setCodes("GTC").name("Godless Shrine");
+            fillSpecialRares("GTC", "Watery Grave");
-            savedSpecialRares.addAll(CardRepository.instance.findCards(criteria));
+            fillSpecialRares("RTR", "Blood Crypt");
-            criteria = new CardCriteria();
+            fillSpecialRares("RTR", "Hallowed Fountain");
-            criteria.setCodes("GTC").name("Sacred Foundry");
+            fillSpecialRares("RTR", "Overgrown Tomb");
-            savedSpecialRares.addAll(CardRepository.instance.findCards(criteria));
+            fillSpecialRares("RTR", "Steam Vents");
-            criteria = new CardCriteria();
+            fillSpecialRares("RTR", "Temple Garden");
-            criteria.setCodes("GTC").name("Stomping Ground");
-            savedSpecialRares.addAll(CardRepository.instance.findCards(criteria));
-            criteria = new CardCriteria();
-            criteria.setCodes("GTC").name("Watery Grave");
-            savedSpecialRares.addAll(CardRepository.instance.findCards(criteria));
-
-            criteria = new CardCriteria();
-            criteria.setCodes("RTR").name("Blood Crypt");
-            savedSpecialRares.addAll(CardRepository.instance.findCards(criteria));
-            criteria = new CardCriteria();
-            criteria.setCodes("RTR").name("Hallowed Fountain");
-            savedSpecialRares.addAll(CardRepository.instance.findCards(criteria));
-            criteria = new CardCriteria();
-            criteria.setCodes("RTR").name("Overgrown Tomb");
-            savedSpecialRares.addAll(CardRepository.instance.findCards(criteria));
-            criteria = new CardCriteria();
-            criteria.setCodes("RTR").name("Steam Vents");
-            savedSpecialRares.addAll(CardRepository.instance.findCards(criteria));
-            criteria = new CardCriteria();
-            criteria.setCodes("RTR").name("Temple Garden");
-            savedSpecialRares.addAll(CardRepository.instance.findCards(criteria));
         }
         return new ArrayList<>(savedSpecialRares);
     }
 
+    private void fillSpecialRares(String setCode, String cardName) {
+        CardCriteria criteria = new CardCriteria();
+        criteria.setCodes(setCode).name(cardName);
+        savedSpecialRares.addAll(CardRepository.instance.findCards(criteria));
+    }
+
     @Override
     public List<CardInfo> getSpecialMythic() {
         CardCriteria criteria = new CardCriteria();

Mage.Tests/src/test/java/org/mage/test/cards/continuous/PaintersServantTest.java

@@ -30,7 +30,7 @@ public class PaintersServantTest extends CardTestPlayerBase {
 
         addCard(Zone.HAND, playerB, "Lightning Bolt");
         addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion");
-        addCard(Zone.GRAVEYARD, playerA, "Silvercoat Lion");
+        addCard(Zone.GRAVEYARD, playerB, "Silvercoat Lion");
 
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Painter's Servant");
         setChoice(playerA, "Blue");

Mage.Tests/src/test/java/org/mage/test/cards/continuous/SubTypeChangingEffectsTest.java

@@ -1,4 +1,3 @@
-
 package org.mage.test.cards.continuous;
 
 import mage.cards.Card;
@@ -11,7 +10,6 @@ import org.junit.Test;
 import org.mage.test.serverside.base.CardTestPlayerBase;
 
 /**
- *
  * @author LevelX2
  */
 public class SubTypeChangingEffectsTest extends CardTestPlayerBase {
@@ -29,7 +27,7 @@ public class SubTypeChangingEffectsTest extends CardTestPlayerBase {
 
         addCard(Zone.HAND, playerB, "Silvercoat Lion");
         addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion");
-        addCard(Zone.GRAVEYARD, playerA, "Silvercoat Lion");
+        addCard(Zone.GRAVEYARD, playerB, "Silvercoat Lion");
 
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Conspiracy");
         setChoice(playerA, "Orc");
@@ -105,7 +103,7 @@ public class SubTypeChangingEffectsTest extends CardTestPlayerBase {
 
         addCard(Zone.HAND, playerB, "Silvercoat Lion");
         addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion");
-        addCard(Zone.GRAVEYARD, playerA, "Silvercoat Lion");
+        addCard(Zone.GRAVEYARD, playerB, "Silvercoat Lion");
 
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Conspiracy");
         setChoice(playerA, "Orc");
@@ -159,7 +157,7 @@ public class SubTypeChangingEffectsTest extends CardTestPlayerBase {
 
         addCard(Zone.HAND, playerB, "Silvercoat Lion");
         addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion");
-        addCard(Zone.GRAVEYARD, playerA, "Silvercoat Lion");
+        addCard(Zone.GRAVEYARD, playerB, "Silvercoat Lion");
 
         castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Arcane Adaptation");
         setChoice(playerA, "Orc");
@@ -246,7 +244,7 @@ public class SubTypeChangingEffectsTest extends CardTestPlayerBase {
 
         addCard(Zone.HAND, playerB, "Silvercoat Lion");
         addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion");
-        addCard(Zone.GRAVEYARD, playerA, "Silvercoat Lion");
+        addCard(Zone.GRAVEYARD, playerB, "Silvercoat Lion");
 
         castSpell(1, PhaseStep.UPKEEP, playerA, "Advent of the Wurm");
 

Mage/src/main/java/mage/abilities/effects/Effects.java

@@ -1,13 +1,14 @@
 package mage.abilities.effects;
 
-import java.util.ArrayList;
-import java.util.Arrays;
 import mage.abilities.Ability;
 import mage.abilities.Mode;
 import mage.constants.Outcome;
 import mage.target.targetpointer.TargetPointer;
 import mage.util.CardUtil;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+
 /**
  * @author BetaSteward_at_googlemail.com
  */
@@ -56,42 +57,41 @@ public class Effects extends ArrayList<Effect> {
                 nextRule = concatPrefix + " " + nextRule;
             }
 
-            if (nextRule != null) {
+
-                //check if nextRule is a new sentence or not.
+            //check if nextRule is a new sentence or not.
-                if (nextRule.startsWith("and ") || nextRule.startsWith("with ") || nextRule.startsWith("then ")) {
+            if (nextRule.startsWith("and ") || nextRule.startsWith("with ") || nextRule.startsWith("then ")) {
+                endString = " ";
+            } else if (nextRule.startsWith(",") || nextRule.startsWith(" ")) {
+                endString = "";
+                // nextRule determined to be a new sentence, now check ending of lastRule
+            } else if (lastRule != null && lastRule.length() > 3) {
+                //check if lastRule already has appropriate punctuation, if so, add a space.
+                if (lastRule.endsWith(".\"")
+                        || lastRule.endsWith(".)")
+                        || lastRule.endsWith(".)</i>")
+                        || lastRule.endsWith(".")) {
                     endString = " ";
-                } else if (nextRule.startsWith(",") || nextRule.startsWith(" ")) {
+                    // if lastRule does not have appropriate punctuation, add the default ". "
-                    endString = "";
+                } else if (!lastRule.endsWith(".") && !lastRule.endsWith("<br>")) {
-                    // nextRule determined to be a new sentence, now check ending of lastRule
+                    endString = ". ";
-                } else if (lastRule != null && lastRule.length() > 3) {
-                    //check if lastRule already has appropriate punctuation, if so, add a space.
-                    if (lastRule.endsWith(".\"")
-                            || lastRule.endsWith(".)")
-                            || lastRule.endsWith(".)</i>")
-                            || lastRule.endsWith(".")) {
-                        endString = " ";
-                        // if lastRule does not have appropriate punctuation, add the default ". "
-                    } else if (!lastRule.endsWith(".") && !lastRule.endsWith("<br>")) {
-                        endString = ". ";
-                    }
-                    if (nextRule.length() > 3) {
-                        nextRule = Character.toUpperCase(nextRule.charAt(0)) + nextRule.substring(1);
-                    }
                 }
-
+                if (nextRule.length() > 3) {
-                String currentRule = endString + nextRule;
+                    nextRule = Character.toUpperCase(nextRule.charAt(0)) + nextRule.substring(1);
-                // fix dot in the combined effect like IfDoCost
-                if (sbText.length() > 0 && currentRule.length() > 0) {
-                    boolean prevTextEndsWithDot = sbText.charAt(sbText.length() - 1) == '.';
-                    boolean currentTextStartsWithDot = currentRule.startsWith(",") || currentRule.startsWith(".");
-                    if (prevTextEndsWithDot && currentTextStartsWithDot) {
-                        sbText.delete(sbText.length() - 1, sbText.length());
-                    }
                 }
-
-                sbText.append(currentRule);
             }
 
+            String currentRule = endString + nextRule;
+            // fix dot in the combined effect like IfDoCost
+            if (sbText.length() > 0 && currentRule.length() > 0) {
+                boolean prevTextEndsWithDot = sbText.charAt(sbText.length() - 1) == '.';
+                boolean currentTextStartsWithDot = currentRule.startsWith(",") || currentRule.startsWith(".");
+                if (prevTextEndsWithDot && currentTextStartsWithDot) {
+                    sbText.delete(sbText.length() - 1, sbText.length());
+                }
+            }
+
+            sbText.append(currentRule);
+
             lastRule = nextRule;
 
         }

Mage/src/main/java/mage/abilities/effects/common/continuous/BecomesCreatureTypeTargetEffect.java

@@ -51,7 +51,6 @@ public class BecomesCreatureTypeTargetEffect extends ContinuousEffectImpl {
         super(effect);
         this.subtypes.addAll(effect.subtypes);
         this.loseOther = effect.loseOther;
-        this.loseOther = effect.loseOther;
     }
 
     @Override

Mage/src/main/java/mage/abilities/effects/common/continuous/BoostSourceEffect.java

@@ -1,5 +1,3 @@
-
-
 package mage.abilities.effects.common.continuous;
 
 import mage.MageObjectReference;
@@ -16,13 +14,12 @@ import mage.game.permanent.Permanent;
 import org.apache.log4j.Logger;
 
 /**
- *
  * @author BetaSteward_at_googlemail.com
  */
 public class BoostSourceEffect extends ContinuousEffectImpl implements SourceEffect {
     private DynamicValue power;
     private DynamicValue toughness;
-    private boolean lockedIn;
+    private final boolean lockedIn;
 
     public BoostSourceEffect(int power, int toughness, Duration duration) {
         this(StaticValue.get(power), StaticValue.get(toughness), duration, false);
@@ -36,7 +33,7 @@ public class BoostSourceEffect extends ContinuousEffectImpl implements SourceEff
      * @param power
      * @param toughness
      * @param duration
-     * @param lockedIn if true, power and toughness will be calculated only once, when the ability resolves
+     * @param lockedIn  if true, power and toughness will be calculated only once, when the ability resolves
      */
     public BoostSourceEffect(DynamicValue power, DynamicValue toughness, Duration duration, boolean lockedIn) {
         super(duration, Layer.PTChangingEffects_7, SubLayer.ModifyPT_7c, Outcome.BoostCreature);
@@ -94,18 +91,13 @@ public class BoostSourceEffect extends ContinuousEffectImpl implements SourceEff
         StringBuilder sb = new StringBuilder();
         sb.append("{this} gets ");
         String p = power.toString();
-        if(!p.startsWith("-")) {
+        if (!p.startsWith("-")) {
             sb.append('+');
         }
         sb.append(p).append('/');
         String t = toughness.toString();
-        if(!t.startsWith("-")){
+        if (!t.startsWith("-")) {
-            if(t.startsWith("-")) {
+            sb.append('+');
-                sb.append('-');
-            }
-            else {
-                sb.append('+');
-            }
         }
         sb.append(t);
         if (duration != Duration.WhileOnBattlefield) {

Mage/src/main/java/mage/abilities/effects/common/continuous/BoostTargetEffect.java

@@ -1,4 +1,3 @@
-
 package mage.abilities.effects.common.continuous;
 
 import mage.abilities.Ability;
@@ -25,7 +24,7 @@ public class BoostTargetEffect extends ContinuousEffectImpl {
 
     private DynamicValue power;
     private DynamicValue toughness;
-    private boolean lockedIn;
+    private final boolean lockedIn;
 
     public BoostTargetEffect(int power, int toughness) {
         this(power, toughness, Duration.EndOfTurn);
@@ -104,7 +103,7 @@ public class BoostTargetEffect extends ContinuousEffectImpl {
             }
             sb.append(CardUtil.numberToText(target.getMaxNumberOfTargets())).append(" target ").append(target.getTargetName()).append(" get ");
         } else {
-            if (!target.getTargetName().toUpperCase(Locale.ENGLISH).startsWith("ANOTHER")) {
+            if (!target.getTargetName().toLowerCase(Locale.ENGLISH).startsWith("another")) {
                 sb.append("target ");
             }
             sb.append(target.getTargetName()).append(" gets ");

Mage/src/main/java/mage/abilities/effects/common/continuous/GainAbilityTargetEffect.java

@@ -19,7 +19,7 @@ public class GainAbilityTargetEffect extends ContinuousEffectImpl {
 
     protected Ability ability;
     // shall a card gain the ability (otherwise permanent)
-    private boolean onCard;
+    private final boolean onCard;
 
     // Duration until next phase step of player
     private PhaseStep durationPhaseStep = null;
@@ -140,7 +140,7 @@ public class GainAbilityTargetEffect extends ContinuousEffectImpl {
                 }
                 sb.append(target.getMaxNumberOfTargets()).append(" target ").append(target.getTargetName()).append(" gain ");
             } else {
-                if (!target.getTargetName().toUpperCase(Locale.ENGLISH).startsWith("ANOTHER")) {
+                if (!target.getTargetName().toLowerCase(Locale.ENGLISH).startsWith("another")) {
                     sb.append("target ");
                 }
                 sb.append(target.getTargetName()).append(" gains ");

Mage/src/main/java/mage/abilities/effects/common/continuous/SetPowerToughnessAllEffect.java

@@ -1,9 +1,5 @@
-
-
 package mage.abilities.effects.common.continuous;
 
-import java.util.Iterator;
-import java.util.Locale;
 import mage.MageObjectReference;
 import mage.abilities.Ability;
 import mage.abilities.Mode;
@@ -19,16 +15,18 @@ import mage.filter.common.FilterCreaturePermanent;
 import mage.game.Game;
 import mage.game.permanent.Permanent;
 
+import java.util.Iterator;
+import java.util.Locale;
+
 /**
- *
  * @author LevelX2
  */
 public class SetPowerToughnessAllEffect extends ContinuousEffectImpl {
 
-    private FilterPermanent filter;
+    private final FilterPermanent filter;
     private DynamicValue power;
     private DynamicValue toughness;
-    private boolean lockedInPT;
+    private final boolean lockedInPT;
 
     public SetPowerToughnessAllEffect(int power, int toughness, Duration duration) {
         this(StaticValue.get(power), StaticValue.get(toughness), duration, new FilterCreaturePermanent("Creatures"), true);
@@ -63,7 +61,7 @@ public class SetPowerToughnessAllEffect extends ContinuousEffectImpl {
     public void init(Ability source, Game game) {
         super.init(source, game);
         if (affectedObjectsSet) {
-            for (Permanent perm: game.getBattlefield().getActivePermanents(filter, source.getControllerId(), source.getSourceId(), game)) {
+            for (Permanent perm : game.getBattlefield().getActivePermanents(filter, source.getControllerId(), source.getSourceId(), game)) {
                 affectedObjectList.add(new MageObjectReference(perm, game));
             }
         }
@@ -78,7 +76,7 @@ public class SetPowerToughnessAllEffect extends ContinuousEffectImpl {
         int newPower = power.calculate(game, source, this);
         int newToughness = toughness.calculate(game, source, this);
         if (affectedObjectsSet) {
-            for (Iterator<MageObjectReference> it = affectedObjectList.iterator(); it.hasNext();) {
+            for (Iterator<MageObjectReference> it = affectedObjectList.iterator(); it.hasNext(); ) {
                 Permanent permanent = it.next().getPermanent(game);
                 if (permanent != null) {
                     permanent.getPower().setValue(newPower);
@@ -88,7 +86,7 @@ public class SetPowerToughnessAllEffect extends ContinuousEffectImpl {
                 }
             }
         } else {
-            for (Permanent permanent: game.getBattlefield().getActivePermanents(filter, source.getControllerId(), source.getSourceId(), game)) {
+            for (Permanent permanent : game.getBattlefield().getActivePermanents(filter, source.getControllerId(), source.getSourceId(), game)) {
                 permanent.getPower().setValue(newPower);
                 permanent.getToughness().setValue(newToughness);
             }
@@ -104,7 +102,7 @@ public class SetPowerToughnessAllEffect extends ContinuousEffectImpl {
 
         StringBuilder sb = new StringBuilder();
         sb.append(filter.getMessage());
-        if (filter.getMessage().toLowerCase(Locale.ENGLISH).startsWith("Each ")) {
+        if (filter.getMessage().toLowerCase(Locale.ENGLISH).startsWith("each ")) {
             sb.append(" has base power and toughness ");
         } else {
             sb.append(" have base power and toughness ");