* Choose an ability - fixed not working cards like Argivian Avenger, Atraxas Skitterfang, Steel Seraph (#10115)

Mage.Sets/src/mage/cards/a/ArgivianAvenger.java

@@ -24,6 +24,7 @@ import mage.game.permanent.Permanent;
 import mage.players.Player;
 
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.UUID;
 
@@ -89,7 +90,7 @@ class ArgivianAvengerEffect extends OneShotEffect {
         game.addEffect(new BoostSourceEffect(-1, -1, Duration.EndOfTurn), source);
         Choice choice = new ChoiceImpl(true);
         choice.setMessage("Choose an ability");
-        choice.setChoices(abilityMap.keySet());
+        choice.setChoices(new HashSet<>(abilityMap.keySet()));
         player.choose(outcome, choice, game);
         Ability ability = abilityMap.getOrDefault(choice.getChoice(), null);
         if (ability != null) {

Mage.Sets/src/mage/cards/a/AtraxasSkitterfang.java

@@ -26,6 +26,7 @@ import mage.players.Player;
 import mage.target.common.TargetControlledCreaturePermanent;
 
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.UUID;
 
@@ -103,7 +104,7 @@ class AtraxasSkitterfangEffect extends OneShotEffect {
         }
         Choice choice = new ChoiceImpl(true);
         choice.setMessage("Choose an ability");
-        choice.setChoices(abilityMap.keySet());
+        choice.setChoices(new HashSet<>(abilityMap.keySet()));
         player.choose(outcome, choice, game);
         Ability ability = abilityMap.getOrDefault(choice.getChoice(), null);
         if (ability != null) {

Mage.Sets/src/mage/cards/i/InvokeTheAncients.java

@@ -15,10 +15,7 @@ import mage.game.permanent.token.SpiritGreenToken;
 import mage.game.permanent.token.Token;
 import mage.players.Player;
 
-import java.util.Arrays;
+import java.util.*;
-import java.util.Locale;
-import java.util.Set;
-import java.util.UUID;
 import java.util.stream.Collectors;
 
 /**
@@ -46,9 +43,9 @@ public final class InvokeTheAncients extends CardImpl {
 class InvokeTheAncientsEffect extends OneShotEffect {
 
     private static final Token token = new SpiritGreenToken();
-    private static final Set<String> choices = Arrays.asList(
+    private static final Set<String> choices = new HashSet<>(Arrays.asList(
             "Vigilance", "Reach", "Trample"
-    ).stream().collect(Collectors.toSet());
+    ));
 
     InvokeTheAncientsEffect() {
         super(Outcome.Benefit);

Mage.Sets/src/mage/cards/s/SteelSeraph.java

@@ -22,6 +22,7 @@ import mage.players.Player;
 import mage.target.common.TargetControlledCreaturePermanent;
 
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Map;
 import java.util.UUID;
 
@@ -93,7 +94,7 @@ class SteelSeraphEffect extends OneShotEffect {
         }
         Choice choice = new ChoiceImpl(true);
         choice.setMessage("Choose an ability");
-        choice.setChoices(map.keySet());
+        choice.setChoices(new HashSet<>(map.keySet()));
         player.choose(outcome, choice, game);
         Ability ability = map.getOrDefault(choice.getChoice(), null);
         if (ability == null) {

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

@@ -27,10 +27,7 @@ import mage.game.stack.Spell;
 import mage.players.Player;
 import mage.target.common.TargetCardInLibrary;
 
-import java.util.Arrays;
+import java.util.*;
-import java.util.Locale;
-import java.util.Set;
-import java.util.UUID;
 import java.util.stream.Collectors;
 
 /**
@@ -77,8 +74,9 @@ public final class VivienMonstersAdvocate extends CardImpl {
 class VivienMonstersAdvocateTokenEffect extends OneShotEffect {
 
     private static final Token token = new BeastToken();
-    private static final Set<String> choices
+    private static final Set<String> choices = new HashSet<>(Arrays.asList(
-            = Arrays.asList("Vigilance", "Reach", "Trample").stream().collect(Collectors.toSet());
+            "Vigilance", "Reach", "Trample"
+    ));
 
     VivienMonstersAdvocateTokenEffect() {
         super(Outcome.Benefit);

Mage.Tests/src/test/java/org/mage/test/serverside/performance/SerializationTest.java

@@ -1,11 +1,14 @@
 package org.mage.test.serverside.performance;
 
-import mage.abilities.keyword.InfectAbility;
+import mage.abilities.Ability;
+import mage.abilities.keyword.*;
 import mage.cards.Card;
 import mage.cards.ExpansionSet;
 import mage.cards.Sets;
 import mage.cards.repository.CardInfo;
 import mage.cards.repository.CardRepository;
+import mage.choices.Choice;
+import mage.choices.ChoiceImpl;
 import mage.constants.PhaseStep;
 import mage.constants.Zone;
 import mage.counters.CounterType;
@@ -21,8 +24,7 @@ import org.junit.Ignore;
 import org.junit.Test;
 import org.mage.test.serverside.base.CardTestPlayerBase;
 
-import java.util.ArrayList;
+import java.util.*;
-import java.util.List;
 import java.util.stream.Collectors;
 
 /**
@@ -73,18 +75,18 @@ public class SerializationTest extends CardTestPlayerBase {
         CardUtil.getObjectPartsAsObjects(newCard).stream()
                 .map(Card.class::cast)
                 .forEach(card -> {
-                    Card testCard = CardUtil.getDefaultCardSideForBattlefield(currentGame, newCard);
                     Card testPermanent = null;
-                    if (!testCard.isInstantOrSorcery()) {
+                    if (!card.isInstantOrSorcery()) {
-                        testPermanent = new PermanentCard(testCard, playerA.getId(), currentGame);
+                        Card battlefieldCard = CardUtil.getDefaultCardSideForBattlefield(currentGame, card);
+                        testPermanent = new PermanentCard(battlefieldCard, playerA.getId(), currentGame);
                     }
 
                     // card
                     {
-                        Object compressed = CompressUtil.compress(testCard);
+                        Object compressed = CompressUtil.compress(card);
                         Assert.assertTrue("Must be zip", compressed instanceof ZippedObjectImpl);
                         Card uncompressed = (Card) CompressUtil.decompress(compressed);
-                        Assert.assertEquals("Must be same", testCard.getName(), uncompressed.getName());
+                        Assert.assertEquals("Must be same", card.getName(), uncompressed.getName());
                     }
 
                     // permanent
@@ -166,4 +168,35 @@ public class SerializationTest extends CardTestPlayerBase {
         GameView uncompressed = (GameView) CompressUtil.decompress(compressed);
         Assert.assertEquals("Must be same", 1, uncompressed.getPlayers().get(0).getBattlefield().size());
     }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void test_Choices_MustHaveProtectionFromKeySetUsage() {
+        Map<String, Ability> abilityMap = new HashMap<>();
+        abilityMap.put("Flying", FlyingAbility.getInstance());
+        Choice choice = new ChoiceImpl(true);
+        choice.setMessage("Choose an ability");
+
+        choice.setChoices(abilityMap.keySet());
+        Assert.fail("Can't be here");
+    }
+
+    @Test
+    public void test_Choices_MustSupportSerialization() {
+        // related issue: https://github.com/magefree/mage/issues/10115
+        // copy-paste structure for the test (example from Atraxas Skitter)
+        Map<String, Ability> abilityMap = new HashMap<>();
+        abilityMap.put("Flying", FlyingAbility.getInstance());
+        abilityMap.put("Vigilance", VigilanceAbility.getInstance());
+        abilityMap.put("Deathtouch", DeathtouchAbility.getInstance());
+        abilityMap.put("Lifelink", LifelinkAbility.getInstance());
+        Choice choice = new ChoiceImpl(true);
+        choice.setMessage("Choose an ability");
+
+        choice.setChoices(new HashSet<>(abilityMap.keySet()));
+
+        Object compressed = CompressUtil.compress(choice);
+        Assert.assertTrue("Must be zip", compressed instanceof ZippedObjectImpl);
+        Choice uncompressed = (Choice) CompressUtil.decompress(compressed);
+        Assert.assertEquals("Must be same", choice.getChoices().size(), uncompressed.getChoices().size());
+    }
 }

Mage/src/main/java/mage/choices/ChoiceImpl.java

@@ -1,5 +1,6 @@
 package mage.choices;
 
+import mage.util.CardUtil;
 import mage.util.RandomUtil;
 import org.apache.log4j.Logger;
 
@@ -111,6 +112,7 @@ public class ChoiceImpl implements Choice {
 
     @Override
     public void setChoices(Set<String> choices) {
+        CardUtil.checkSetParamForSerializationCompatibility(choices);
         this.choices = choices;
         protectFromEmptyChoices();
     }

Mage/src/main/java/mage/game/events/PlayerQueryEvent.java

@@ -16,6 +16,7 @@ import mage.cards.Card;
 import mage.cards.Cards;
 import mage.choices.Choice;
 import mage.game.permanent.Permanent;
+import mage.util.CardUtil;
 
 /**
  *
@@ -65,6 +66,9 @@ public class PlayerQueryEvent extends EventObject implements ExternalEvent, Seri
                              Set<UUID> targets, Cards cards, QueryType queryType, int min, int max, boolean required,
                              Map<String, Serializable> options, List<String> messages) {
         super(playerId);
+
+        CardUtil.checkSetParamForSerializationCompatibility(choices);
+
         this.queryType = queryType;
         this.message = message;
         this.playerId = playerId;

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

@@ -1733,4 +1733,12 @@ public final class CardUtil {
                 return "" + startingLoyalty;
         }
     }
+
+    public static void checkSetParamForSerializationCompatibility(Set<String> data) {
+        // HashMap uses inner class for Keys without serialization support,
+        // so you can't use it for client-server data
+        if (data != null && data.getClass().getName().endsWith("$KeySet")) {
+            throw new IllegalArgumentException("Can't use KeySet as param, use new HashSet<>(data.keySet()) instead");
+        }
+    }
 }