tests: added verify check for wrong predicates usage in filters (ClassCastException errors like #12774)

This commit is contained in:
Oleg Agafonov 2024-09-19 04:32:21 +04:00
parent e1f76c2b6c
commit cd51954208
14 changed files with 176 additions and 91 deletions

View file

@ -27,9 +27,7 @@ import mage.counters.Counter;
import mage.counters.CounterType;
import mage.designations.CitysBlessing;
import mage.designations.Monarch;
import mage.game.Game;
import mage.game.GameException;
import mage.game.GameImpl;
import mage.game.*;
import mage.game.command.Dungeon;
import mage.game.command.Emblem;
import mage.game.command.Plane;
@ -270,8 +268,8 @@ public class TestCardRenderDialog extends MageDialog {
// prepare fake game and players without real match
// it's a workaround with minimum code and data init
this.match = new TestMatch();
this.game = new TestGame(MultiplayerAttackOption.MULTIPLE, RangeOfInfluence.ALL, MulliganType.GAME_DEFAULT.getMulligan(0), 20, 7);
this.match = new FakeMatch();
this.game = new FakeGame();
Deck deck = new Deck();
Player playerYou = new StubPlayer("player1", RangeOfInfluence.ALL);
playerYou.addDesignation(new CitysBlessing());
@ -977,68 +975,4 @@ public class TestCardRenderDialog extends MageDialog {
private javax.swing.JSpinner spinnerCardIconsAdditionalAmount;
private javax.swing.JSpinner spinnerCardIconsMaxVisible;
// End of variables declaration//GEN-END:variables
}
class TestGame extends GameImpl {
private int numPlayers;
public TestGame(MultiplayerAttackOption attackOption, RangeOfInfluence range, Mulligan mulligan, int startLife, int startHandSize) {
super(attackOption, range, mulligan, 60, startLife, startHandSize);
}
public TestGame(final TestGame game) {
super(game);
this.numPlayers = game.numPlayers;
}
@Override
public MatchType getGameType() {
return new TestGameType();
}
@Override
public int getNumPlayers() {
return numPlayers;
}
@Override
public TestGame copy() {
return new TestGame(this);
}
}
class TestGameType extends MatchType {
public TestGameType() {
this.name = "Test Game Type";
this.maxPlayers = 10;
this.minPlayers = 3;
this.numTeams = 0;
this.useAttackOption = true;
this.useRange = true;
this.sideboardingAllowed = true;
}
protected TestGameType(final TestGameType matchType) {
super(matchType);
}
@Override
public TestGameType copy() {
return new TestGameType(this);
}
}
class TestMatch extends MatchImpl {
public TestMatch() {
super(new MatchOptions("fake match", "fake game type", true, 2));
}
@Override
public void startGame() throws GameException {
throw new IllegalStateException("Can't start fake match");
}
}
}

View file

@ -99,12 +99,12 @@ class ConduitOfRuinWatcher extends Watcher {
}
}
class FirstCastCreatureSpellPredicate implements ObjectSourcePlayerPredicate<Controllable> {
class FirstCastCreatureSpellPredicate implements ObjectSourcePlayerPredicate<Card> {
@Override
public boolean apply(ObjectSourcePlayer<Controllable> input, Game game) {
if (input.getObject() instanceof Card
&& ((Card) input.getObject()).isCreature(game)) {
public boolean apply(ObjectSourcePlayer<Card> input, Game game) {
if (input.getObject() != null
&& input.getObject().isCreature(game)) {
ConduitOfRuinWatcher watcher = game.getState().getWatcher(ConduitOfRuinWatcher.class);
return watcher != null && watcher.creatureSpellsCastThisTurn(input.getPlayerId()) == 0;
}

View file

@ -93,13 +93,13 @@ public final class ElugeTheShorelessSea extends CardImpl {
}
}
enum ElugeTheShorelessSeaPredicate implements ObjectSourcePlayerPredicate<Controllable> {
enum ElugeTheShorelessSeaPredicate implements ObjectSourcePlayerPredicate<Card> {
instance;
@Override
public boolean apply(ObjectSourcePlayer<Controllable> input, Game game) {
if (input.getObject() instanceof Card &&
((Card) input.getObject()).isInstantOrSorcery(game)) {
public boolean apply(ObjectSourcePlayer<Card> input, Game game) {
if (input.getObject() != null &&
input.getObject().isInstantOrSorcery(game)) {
ElugeTheShorelessSeaWatcher watcher = game.getState().getWatcher(ElugeTheShorelessSeaWatcher.class);
return watcher != null &&
watcher.getInstantOrSorcerySpellsCastThisTurn(input.getPlayerId()) == 0;

View file

@ -71,13 +71,13 @@ public final class MelekReforgedResearcher extends CardImpl {
}
}
enum MelekReforgedResearcherPredicate implements ObjectSourcePlayerPredicate<Controllable> {
enum MelekReforgedResearcherPredicate implements ObjectSourcePlayerPredicate<Card> {
instance;
@Override
public boolean apply(ObjectSourcePlayer<Controllable> input, Game game) {
if (input.getObject() instanceof Card &&
((Card) input.getObject()).isInstantOrSorcery(game)) {
public boolean apply(ObjectSourcePlayer<Card> input, Game game) {
if (input.getObject() != null &&
input.getObject().isInstantOrSorcery(game)) {
MelekReforgedResearcherWatcher watcher = game.getState().getWatcher(MelekReforgedResearcherWatcher.class);
return watcher != null &&
watcher.getInstantOrSorcerySpellsCastThisTurn(input.getPlayerId()) == 0;

View file

@ -88,12 +88,12 @@ class ShadowInTheWarpWatcher extends Watcher {
}
}
class FirstCastCreatureSpellPredicate implements ObjectSourcePlayerPredicate<Controllable> {
class FirstCastCreatureSpellPredicate implements ObjectSourcePlayerPredicate<Card> {
@Override
public boolean apply(ObjectSourcePlayer<Controllable> input, Game game) {
if (input.getObject() instanceof Card
&& ((Card) input.getObject()).isCreature(game)) {
public boolean apply(ObjectSourcePlayer<Card> input, Game game) {
if (input.getObject() != null
&& input.getObject().isCreature(game)) {
ShadowInTheWarpWatcher watcher = game.getState().getWatcher(ShadowInTheWarpWatcher.class);
return watcher != null && watcher.creatureSpellsCastThisTurn(input.getPlayerId()) == 0;
}

View file

@ -2769,6 +2769,9 @@ public class VerifyCardDataTest {
public void test_checkCardConstructors() {
// create all cards, can catch additional verify and runtime checks from abilities and effects
// example: wrong code usage errors
//
// warning, look at stack trace logs, not card names -- some error code can be hidden in static methods and can be called from un-related cards
// use test_showCardInfo for detailed errors
Collection<String> errorsList = new ArrayList<>();
Collection<ExpansionSet> sets = Sets.getInstance().values();
for (ExpansionSet set : sets) {
@ -2777,7 +2780,7 @@ public class VerifyCardDataTest {
Card card = CardImpl.createCard(setInfo.getCardClass(), new CardSetInfo(setInfo.getName(), set.getCode(),
setInfo.getCardNumber(), setInfo.getRarity(), setInfo.getGraphicInfo()));
if (card == null) {
errorsList.add("Error: broken constructor " + setInfo.getCardClass());
errorsList.add("Error: can't create card - " + setInfo.getCardClass() + " - see logs for errors");
continue;
}
if (!card.getExpansionSetCode().equals(set.getCode())) {
@ -2785,7 +2788,7 @@ public class VerifyCardDataTest {
}
} catch (Throwable e) {
// CardImpl.createCard don't throw exceptions (only error logs), so that logs are useless here
logger.error("Error: can't create card " + setInfo.getName() + ": " + e.getMessage(), e);
errorsList.add("Error: can't create card - " + setInfo.getCardClass() + " - see logs for errors");
}
}
}

View file

@ -73,7 +73,9 @@ public class FilterCard extends FilterObject<Card> {
throw new UnsupportedOperationException("You may not modify a locked filter");
}
// verify check
checkPredicateIsSuitableForCardFilter(predicate);
Predicates.makeSurePredicateCompatibleWithFilter(predicate, Card.class);
extraPredicates.add(predicate);
}

View file

@ -5,6 +5,7 @@ import mage.constants.SubType;
import mage.filter.predicate.ObjectSourcePlayer;
import mage.filter.predicate.ObjectSourcePlayerPredicate;
import mage.filter.predicate.Predicate;
import mage.filter.predicate.Predicates;
import mage.game.Game;
import mage.game.permanent.Permanent;
@ -12,7 +13,6 @@ import java.util.ArrayList;
import java.util.List;
import java.util.Set;
import java.util.UUID;
import java.util.stream.Collectors;
/**
* @author North
@ -64,6 +64,10 @@ public class FilterPermanent extends FilterObject<Permanent> implements FilterIn
if (isLockedFilter()) {
throw new UnsupportedOperationException("You may not modify a locked filter");
}
// verify check
Predicates.makeSurePredicateCompatibleWithFilter(predicate, Permanent.class);
extraPredicates.add(predicate);
}

View file

@ -4,6 +4,7 @@ import mage.abilities.Ability;
import mage.filter.predicate.ObjectSourcePlayer;
import mage.filter.predicate.ObjectSourcePlayerPredicate;
import mage.filter.predicate.Predicate;
import mage.filter.predicate.Predicates;
import mage.game.Game;
import mage.players.Player;
@ -36,6 +37,10 @@ public class FilterPlayer extends FilterImpl<Player> {
if (isLockedFilter()) {
throw new UnsupportedOperationException("You may not modify a locked filter");
}
// verify check
Predicates.makeSurePredicateCompatibleWithFilter(predicate, Player.class);
extraPredicates.add(predicate);
return this;
}

View file

@ -1,10 +1,13 @@
package mage.filter;
import mage.abilities.Ability;
import mage.cards.Card;
import mage.filter.predicate.ObjectSourcePlayer;
import mage.filter.predicate.ObjectSourcePlayerPredicate;
import mage.filter.predicate.Predicate;
import mage.filter.predicate.Predicates;
import mage.game.Game;
import mage.game.stack.Spell;
import mage.game.stack.StackObject;
import java.util.ArrayList;
@ -43,6 +46,11 @@ public class FilterStackObject extends FilterObject<StackObject> {
if (isLockedFilter()) {
throw new UnsupportedOperationException("You may not modify a locked filter");
}
// verify check
// Spell implements Card interface, so it can use some default predicates like owner
Predicates.makeSurePredicateCompatibleWithFilter(predicate, StackObject.class, Spell.class, Card.class);
extraPredicates.add(predicate);
}

View file

@ -1,4 +1,3 @@
package mage.filter.predicate;
import java.io.Serializable;

View file

@ -2,6 +2,8 @@ package mage.filter.predicate;
import mage.game.Game;
import java.lang.reflect.ParameterizedType;
import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
@ -11,7 +13,7 @@ import java.util.List;
*
* <p>All methods returns serializable predicates as long as they're given serializable parameters.</p>
*
* @author North
* @author North, JayDi85
*/
public final class Predicates {
@ -246,4 +248,48 @@ public final class Predicates {
extraPredicates.forEach(p -> collectAllComponents(p, res));
}
}
/**
* Verify check: try to find filters usage
* Example use case: Player predicate was used for Permanent filter
* Example error: java.lang.ClassCastException: mage.game.permanent.PermanentToken cannot be cast to mage.players.Player
*/
public static void makeSurePredicateCompatibleWithFilter(Predicate predicate, Class... compatibleClasses) {
List<Predicate> list = new ArrayList<>();
Predicates.collectAllComponents(predicate, list);
list.forEach(p -> {
Class predicateGenericParamClass = findGenericParam(predicate);
if (predicateGenericParamClass == null) {
throw new IllegalArgumentException("Somthing wrong. Can't find predicate's generic param for " + predicate.getClass());
}
if (Arrays.stream(compatibleClasses).anyMatch(f -> predicateGenericParamClass.isAssignableFrom(f))) {
// predicate is fine
} else {
// How-to fix: use correct predicates (same type, e.g. getControllerPredicate() instead getPlayerPredicate())
throw new IllegalArgumentException(String.format(
"Wrong code usage: predicate [%s] with generic param [%s] can't be added to filter, allow only %s",
predicate.getClass(),
predicateGenericParamClass,
Arrays.toString(compatibleClasses)
));
}
});
}
private static Class findGenericParam(Predicate predicate) {
Type[] interfaces = predicate.getClass().getGenericInterfaces();
for (Type type : interfaces) {
if (type instanceof ParameterizedType) {
ParameterizedType parameterizedType = (ParameterizedType) type;
Type[] actualTypeArguments = parameterizedType.getActualTypeArguments();
if (actualTypeArguments.length > 0) {
Type actualType = actualTypeArguments[0];
if (actualType instanceof Class) {
return (Class) actualType;
}
}
}
}
return null;
}
}

View file

@ -0,0 +1,63 @@
package mage.game;
import mage.constants.MultiplayerAttackOption;
import mage.constants.RangeOfInfluence;
import mage.game.match.MatchType;
import mage.game.mulligan.MulliganType;
/**
* Fake game for tests and data check, do nothing.
*
* @author JayDi85
*/
public class FakeGame extends GameImpl {
private int numPlayers;
public FakeGame() {
super(MultiplayerAttackOption.MULTIPLE, RangeOfInfluence.ALL, MulliganType.GAME_DEFAULT.getMulligan(0), 60, 20, 7);
}
public FakeGame(final FakeGame game) {
super(game);
this.numPlayers = game.numPlayers;
}
@Override
public MatchType getGameType() {
return new FakeGameType();
}
@Override
public int getNumPlayers() {
return numPlayers;
}
@Override
public FakeGame copy() {
return new FakeGame(this);
}
}
class FakeGameType extends MatchType {
public FakeGameType() {
this.name = "Test Game Type";
this.maxPlayers = 10;
this.minPlayers = 3;
this.numTeams = 0;
this.useAttackOption = true;
this.useRange = true;
this.sideboardingAllowed = true;
}
protected FakeGameType(final FakeGameType matchType) {
super(matchType);
}
@Override
public FakeGameType copy() {
return new FakeGameType(this);
}
}

View file

@ -0,0 +1,21 @@
package mage.game;
import mage.game.match.MatchImpl;
import mage.game.match.MatchOptions;
/**
* Fake match for tests and data check, do nothing.
*
* @author JayDi85
*/
public class FakeMatch extends MatchImpl {
public FakeMatch() {
super(new MatchOptions("fake match", "fake game type", true, 2));
}
@Override
public void startGame() throws GameException {
throw new IllegalStateException("Can't start fake match");
}
}