From e48f024909f715535ad156666ba1558421ca9274 Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Sun, 19 Mar 2023 19:55:21 +0400 Subject: [PATCH] * Game: fixed wrong booster's cards ratio in sets with various arts (#7333) --- Mage.Sets/src/mage/sets/MysteryBooster.java | 1 + .../mage/test/sets/BoosterGenerationTest.java | 39 +++++- .../main/java/mage/cards/ExpansionSet.java | 120 +++++++++++++++--- .../mage/cards/repository/CardCriteria.java | 21 ++- 4 files changed, 158 insertions(+), 23 deletions(-) diff --git a/Mage.Sets/src/mage/sets/MysteryBooster.java b/Mage.Sets/src/mage/sets/MysteryBooster.java index 61d51ac77c..f4d71352f9 100644 --- a/Mage.Sets/src/mage/sets/MysteryBooster.java +++ b/Mage.Sets/src/mage/sets/MysteryBooster.java @@ -3515,6 +3515,7 @@ public class MysteryBooster extends ExpansionSet { @Override public List create15CardBooster() { + // ignore special partner generation for 15 booster return this.createBooster(); } } diff --git a/Mage.Tests/src/test/java/org/mage/test/sets/BoosterGenerationTest.java b/Mage.Tests/src/test/java/org/mage/test/sets/BoosterGenerationTest.java index 69fcd7755b..8a84b215ed 100644 --- a/Mage.Tests/src/test/java/org/mage/test/sets/BoosterGenerationTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/sets/BoosterGenerationTest.java @@ -12,6 +12,7 @@ import mage.constants.CardType; import mage.constants.Rarity; import mage.constants.SubType; import mage.sets.*; +import mage.util.CardUtil; import org.junit.Assert; import org.junit.Before; import org.junit.Ignore; @@ -249,6 +250,42 @@ public class BoosterGenerationTest extends MageTestBase { } } + @Test + public void testFallenEmpires_BoosterMustUseVariousArtsButUnique() { + // Related issue: https://github.com/magefree/mage/issues/7333 + // Actual for default boosters without collation + Set cardNumberPosfixes = new HashSet<>(); + for (int i = 0; i < 10; i++) { + List booster = FallenEmpires.getInstance().createBooster(); + + // must have single version of the card for booster generation + Map stats = FallenEmpires.getInstance().getCardsByRarity(Rarity.COMMON) + .stream() + .collect(Collectors.groupingBy(CardInfo::getName, Collectors.counting())); + String multipleCopies = stats.entrySet() + .stream() + .filter(data -> data.getValue() > 1) + .map(Map.Entry::getKey) + .collect(Collectors.joining(", ")); + assertTrue("booster generation must use cards with various arts as one card: " + multipleCopies, multipleCopies.isEmpty()); + + // must have all reprints + booster.forEach(card -> { + // 123c -> c + String postfix = card.getCardNumber().replace(String.valueOf(CardUtil.parseCardNumberAsInt(card.getCardNumber())), ""); + if (!postfix.isEmpty()) { + cardNumberPosfixes.add(postfix); + } + }); + } + assertTrue("booster must use cards with various arts", + cardNumberPosfixes.contains("a") + && cardNumberPosfixes.contains("b") + && cardNumberPosfixes.contains("c") + && cardNumberPosfixes.contains("d") + ); + } + @Test public void testZendikarRising_MDFC() { for (int i = 0; i < 20; i++) { @@ -514,7 +551,7 @@ public class BoosterGenerationTest extends MageTestBase { @Ignore // debug only: collect info about cards in boosters, see https://github.com/magefree/mage/issues/8081 @Test public void test_CollectBoosterStats() { - ExpansionSet setToAnalyse = Innistrad.getInstance(); + ExpansionSet setToAnalyse = FallenEmpires.getInstance(); int openBoosters = 1000; Map resRatio = new HashMap<>(); diff --git a/Mage/src/main/java/mage/cards/ExpansionSet.java b/Mage/src/main/java/mage/cards/ExpansionSet.java index d053f84312..9de582dd2f 100644 --- a/Mage/src/main/java/mage/cards/ExpansionSet.java +++ b/Mage/src/main/java/mage/cards/ExpansionSet.java @@ -144,6 +144,7 @@ public abstract class ExpansionSet implements Serializable { protected final EnumMap> savedCards = new EnumMap<>(Rarity.class); protected final EnumMap> savedSpecialCards = new EnumMap<>(Rarity.class); + protected Map> savedReprints = null; protected final Map inBoosterMap = new HashMap<>(); public ExpansionSet(String name, String code, Date releaseDate, SetType setType) { @@ -223,15 +224,18 @@ public abstract class ExpansionSet implements Serializable { } protected void addToBooster(List booster, List cards) { - if (!cards.isEmpty()) { - CardInfo cardInfo = cards.remove(RandomUtil.nextInt(cards.size())); - if (cardInfo != null) { - Card card = cardInfo.getCard(); - if (card != null) { - booster.add(card); - } - } + if (cards.isEmpty()) { + return; } + + CardInfo cardInfo = cards.remove(RandomUtil.nextInt(cards.size())); + Card card = cardInfo.getCard(); + if (card == null) { + // card with error + return; + } + + booster.add(card); } public BoosterCollator createCollator() { @@ -247,13 +251,13 @@ public abstract class ExpansionSet implements Serializable { for (int i = 0; i < 100; i++) {//don't want to somehow loop forever List booster = tryBooster(); if (boosterIsValid(booster)) { - return booster; + return addReprints(booster); } } // return random booster if can't do valid logger.error(String.format("Can't generate valid booster for set [%s - %s]", this.getCode(), this.getName())); - return tryBooster(); + return addReprints(tryBooster()); } private List createBoosterUsingCollator(BoosterCollator collator) { @@ -280,10 +284,10 @@ public abstract class ExpansionSet implements Serializable { if (!hasBasicLands && parentSet != null) { String parentCode = parentSet.code; CardRepository - .instance - .findCards(new CardCriteria().setCodes(parentCode).rarities(Rarity.LAND)) - .stream() - .forEach(cardInfo -> inBoosterMap.put(parentCode + "_" + cardInfo.getCardNumber(), cardInfo)); + .instance + .findCards(new CardCriteria().setCodes(parentCode).rarities(Rarity.LAND)) + .stream() + .forEach(cardInfo -> inBoosterMap.put(parentCode + "_" + cardInfo.getCardNumber(), cardInfo)); } } @@ -384,7 +388,16 @@ public abstract class ExpansionSet implements Serializable { return ratioBoosterSpecialMythic > 0 && ratioBoosterSpecialMythic * RandomUtil.nextDouble() <= 1; } - public List tryBooster() { + /** + * Generates a single booster by rarity ratio in sets without custom collator + */ + protected List tryBooster() { + // Booster generating proccess must use: + // * unique cards list for ratio calculation (see removeReprints) + // * reprints for final result (see addReprints) + // + // BUT there is possible a card's duplication, see https://www.mtgsalvation.com/forums/magic-fundamentals/magic-general/554944-do-booster-packs-ever-contain-two-of-the-same-card + List booster = new ArrayList<>(); if (!hasBoosters) { return booster; @@ -491,10 +504,75 @@ public abstract class ExpansionSet implements Serializable { return hasBasicLands; } + /** + * Keep only unique cards for booster generation and card ratio calculation + * + * @param list all cards list + */ + private List removeReprints(List list) { + Map usedNames = new HashMap<>(); + List filteredList = new ArrayList<>(); + list.forEach(card -> { + CardInfo foundCard = usedNames.getOrDefault(card.getName(), null); + if (foundCard == null) { + usedNames.put(card.getName(), card); + filteredList.add(card); + } + }); + return filteredList; + } + + /** + * Fill booster with reprints, used for non collator generation + * + * @param booster booster's cards + * @return + */ + private List addReprints(List booster) { + if (booster.stream().noneMatch(Card::getUsesVariousArt)) { + return new ArrayList<>(booster); + } + + // generate possible reprints + if (this.savedReprints == null) { + this.savedReprints = new HashMap<>(); + List needSets = new ArrayList<>(); + needSets.add(this.code); + if (this.parentSet != null) { + needSets.add(this.parentSet.code); + } + List cardInfos = CardRepository.instance.findCards(new CardCriteria() + .setCodes(needSets) + .variousArt(true) + .maxCardNumber(maxCardNumberInBooster) // ignore bonus/extra reprints + ); + cardInfos.forEach(card -> { + this.savedReprints.putIfAbsent(card.getName(), new ArrayList<>()); + this.savedReprints.get(card.getName()).add(card); + }); + } + + // replace normal cards by random reprints + List finalBooster = new ArrayList<>(); + booster.forEach(card -> { + List reprints = this.savedReprints.getOrDefault(card.getName(), null); + if (reprints != null && reprints.size() > 1) { + Card newCard = reprints.get(RandomUtil.nextInt(reprints.size())).getCard(); + if (newCard != null) { + finalBooster.add(newCard); + return; + } + } + finalBooster.add(card); + }); + + return finalBooster; + } + public final synchronized List getCardsByRarity(Rarity rarity) { List savedCardInfos = savedCards.get(rarity); if (savedCardInfos == null) { - savedCardInfos = findCardsByRarity(rarity); + savedCardInfos = removeReprints(findCardsByRarity(rarity)); savedCards.put(rarity, savedCardInfos); } // Return a copy of the saved cards information, as not to let modify the original. @@ -504,7 +582,7 @@ public abstract class ExpansionSet implements Serializable { public final synchronized List getSpecialCardsByRarity(Rarity rarity) { List savedCardInfos = savedSpecialCards.get(rarity); if (savedCardInfos == null) { - savedCardInfos = findSpecialCardsByRarity(rarity); + savedCardInfos = removeReprints(findSpecialCardsByRarity(rarity)); savedSpecialCards.put(rarity, savedCardInfos); } // Return a copy of the saved cards information, as not to let modify the original. @@ -524,7 +602,7 @@ public abstract class ExpansionSet implements Serializable { cardInfos.removeIf(next -> ( next.getCardNumber().contains("*") - || next.getCardNumber().contains("+"))); + || next.getCardNumber().contains("+"))); // special slot cards must not also appear in regular slots of their rarity // special land slot cards must not appear in regular common slots either @@ -540,11 +618,11 @@ public abstract class ExpansionSet implements Serializable { * "Special cards" are cards that have common/uncommon/rare/mythic rarities * but can only appear in a specific slot in boosters. Examples are DFCs in * Innistrad sets and common nonbasic lands in many post-2018 sets. - * + *

* Note that Rarity.SPECIAL and Rarity.BONUS cards are not normally treated * as "special cards" because by default boosters don't even have slots for * those rarities. - * + *

* Also note that getCardsByRarity calls getSpecialCardsByRarity to exclude * special cards from non-special booster slots, so sets that override this * method must not call getCardsByRarity in it or infinite recursion will occur. @@ -572,7 +650,7 @@ public abstract class ExpansionSet implements Serializable { cardInfos.removeIf(next -> ( next.getCardNumber().contains("*") - || next.getCardNumber().contains("+"))); + || next.getCardNumber().contains("+"))); return cardInfos; } diff --git a/Mage/src/main/java/mage/cards/repository/CardCriteria.java b/Mage/src/main/java/mage/cards/repository/CardCriteria.java index 776de0daba..ebc54e6f2d 100644 --- a/Mage/src/main/java/mage/cards/repository/CardCriteria.java +++ b/Mage/src/main/java/mage/cards/repository/CardCriteria.java @@ -29,6 +29,7 @@ public class CardCriteria { private final List notSupertypes; private final List subtypes; private final List rarities; + private Boolean variousArt; private Boolean doubleFaced; private Boolean modalDoubleFaced; private boolean nightCard; @@ -98,6 +99,11 @@ public class CardCriteria { return this; } + public CardCriteria variousArt(boolean variousArt) { + this.variousArt = variousArt; + return this; + } + public CardCriteria doubleFaced(boolean doubleFaced) { this.doubleFaced = doubleFaced; return this; @@ -144,7 +150,11 @@ public class CardCriteria { } public CardCriteria setCodes(String... setCodes) { - this.setCodes.addAll(Arrays.asList(setCodes)); + return setCodes(Arrays.asList(setCodes)); + } + + public CardCriteria setCodes(List setCodes) { + this.setCodes.addAll(setCodes); return this; } @@ -223,6 +233,11 @@ public class CardCriteria { clausesCount++; } + if (variousArt != null) { + where.eq("variousArt", variousArt); + clausesCount++; + } + if (doubleFaced != null) { where.eq("doubleFaced", doubleFaced); clausesCount++; @@ -426,6 +441,10 @@ public class CardCriteria { return rarities; } + public Boolean getVariousArt() { + return variousArt; + } + public Boolean getDoubleFaced() { return doubleFaced; }