diff --git a/Mage.Sets/src/mage/cards/a/ArgivianAvenger.java b/Mage.Sets/src/mage/cards/a/ArgivianAvenger.java index 15763bba107..0aaee1b9776 100644 --- a/Mage.Sets/src/mage/cards/a/ArgivianAvenger.java +++ b/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) { diff --git a/Mage.Sets/src/mage/cards/a/AtraxasSkitterfang.java b/Mage.Sets/src/mage/cards/a/AtraxasSkitterfang.java index 6a461c818f4..76d0559019c 100644 --- a/Mage.Sets/src/mage/cards/a/AtraxasSkitterfang.java +++ b/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) { diff --git a/Mage.Sets/src/mage/cards/i/InvokeTheAncients.java b/Mage.Sets/src/mage/cards/i/InvokeTheAncients.java index 0ea1288588f..8c2ea369d92 100644 --- a/Mage.Sets/src/mage/cards/i/InvokeTheAncients.java +++ b/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.Locale; -import java.util.Set; -import java.util.UUID; +import java.util.*; 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 choices = Arrays.asList( + private static final Set choices = new HashSet<>(Arrays.asList( "Vigilance", "Reach", "Trample" - ).stream().collect(Collectors.toSet()); + )); InvokeTheAncientsEffect() { super(Outcome.Benefit); diff --git a/Mage.Sets/src/mage/cards/s/SteelSeraph.java b/Mage.Sets/src/mage/cards/s/SteelSeraph.java index 1822b6ecfc7..b2ac823d4e8 100644 --- a/Mage.Sets/src/mage/cards/s/SteelSeraph.java +++ b/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) { diff --git a/Mage.Sets/src/mage/cards/v/VivienMonstersAdvocate.java b/Mage.Sets/src/mage/cards/v/VivienMonstersAdvocate.java index 5349862629b..070af62f900 100644 --- a/Mage.Sets/src/mage/cards/v/VivienMonstersAdvocate.java +++ b/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.Locale; -import java.util.Set; -import java.util.UUID; +import java.util.*; 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 choices - = Arrays.asList("Vigilance", "Reach", "Trample").stream().collect(Collectors.toSet()); + private static final Set choices = new HashSet<>(Arrays.asList( + "Vigilance", "Reach", "Trample" + )); VivienMonstersAdvocateTokenEffect() { super(Outcome.Benefit); diff --git a/Mage.Tests/src/test/java/org/mage/test/serverside/performance/SerializationTest.java b/Mage.Tests/src/test/java/org/mage/test/serverside/performance/SerializationTest.java index e115acd91d8..430d35762fa 100644 --- a/Mage.Tests/src/test/java/org/mage/test/serverside/performance/SerializationTest.java +++ b/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.List; +import java.util.*; 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()) { - testPermanent = new PermanentCard(testCard, playerA.getId(), currentGame); + if (!card.isInstantOrSorcery()) { + 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 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 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()); + } } diff --git a/Mage/src/main/java/mage/choices/ChoiceImpl.java b/Mage/src/main/java/mage/choices/ChoiceImpl.java index b3e837a16d3..56717237bdf 100644 --- a/Mage/src/main/java/mage/choices/ChoiceImpl.java +++ b/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 choices) { + CardUtil.checkSetParamForSerializationCompatibility(choices); this.choices = choices; protectFromEmptyChoices(); } diff --git a/Mage/src/main/java/mage/game/events/PlayerQueryEvent.java b/Mage/src/main/java/mage/game/events/PlayerQueryEvent.java index 4b906c6811b..4a2d65854b7 100644 --- a/Mage/src/main/java/mage/game/events/PlayerQueryEvent.java +++ b/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 targets, Cards cards, QueryType queryType, int min, int max, boolean required, Map options, List messages) { super(playerId); + + CardUtil.checkSetParamForSerializationCompatibility(choices); + this.queryType = queryType; this.message = message; this.playerId = playerId; diff --git a/Mage/src/main/java/mage/util/CardUtil.java b/Mage/src/main/java/mage/util/CardUtil.java index 7eab08b4e2c..8a6058ca568 100644 --- a/Mage/src/main/java/mage/util/CardUtil.java +++ b/Mage/src/main/java/mage/util/CardUtil.java @@ -1733,4 +1733,12 @@ public final class CardUtil { return "" + startingLoyalty; } } + + public static void checkSetParamForSerializationCompatibility(Set 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"); + } + } }