diff --git a/Mage.Sets/src/mage/cards/a/ArtifactWard.java b/Mage.Sets/src/mage/cards/a/ArtifactWard.java index 919dbc77724..8f4bdfae93c 100644 --- a/Mage.Sets/src/mage/cards/a/ArtifactWard.java +++ b/Mage.Sets/src/mage/cards/a/ArtifactWard.java @@ -1,7 +1,6 @@ package mage.cards.a; -import java.util.UUID; import mage.abilities.common.SimpleStaticAbility; import mage.abilities.effects.common.AttachEffect; import mage.abilities.effects.common.continuous.GainAbilityAttachedEffect; @@ -9,11 +8,16 @@ import mage.abilities.keyword.EnchantAbility; import mage.abilities.keyword.ProtectionAbility; import mage.cards.CardImpl; import mage.cards.CardSetInfo; -import mage.constants.*; +import mage.constants.AttachmentType; +import mage.constants.CardType; +import mage.constants.Outcome; +import mage.constants.SubType; import mage.filter.common.FilterArtifactCard; import mage.target.TargetPermanent; import mage.target.common.TargetCreaturePermanent; +import java.util.UUID; + /** * * @author MarcoMarin @@ -33,7 +37,7 @@ public final class ArtifactWard extends CardImpl { // Enchanted creature can't be blocked by artifact creatures. // Prevent all damage that would be dealt to enchanted creature by artifact sources. // Enchanted creature can't be the target of abilities from artifact sources. - this.addAbility(new SimpleStaticAbility( + this.addAbility(new SimpleStaticAbility( // TODO: Implement as separate abilities, this isn't quite the same as "Enchanted creature gains protection from artifacts" new GainAbilityAttachedEffect(new ProtectionAbility(new FilterArtifactCard("artifacts")), AttachmentType.AURA))); } diff --git a/Mage.Sets/src/mage/cards/b/Blink.java b/Mage.Sets/src/mage/cards/b/Blink.java index 3189879ab0a..98940ff92fc 100644 --- a/Mage.Sets/src/mage/cards/b/Blink.java +++ b/Mage.Sets/src/mage/cards/b/Blink.java @@ -51,7 +51,7 @@ public final class Blink extends CardImpl { this, SagaChapter.CHAPTER_IV, new CreateTokenEffect(new AlienAngelToken()) ); - this.addAbility(sagaAbility); + this.addAbility(sagaAbility); //TODO: These should be a single AddChapterEffect, but currently XMage does not support noncontiguous Saga chapters } private Blink(final Blink card) { diff --git a/Mage.Verify/src/test/java/mage/verify/VerifyCardDataTest.java b/Mage.Verify/src/test/java/mage/verify/VerifyCardDataTest.java index 037fcbb052e..f7b6493ce98 100644 --- a/Mage.Verify/src/test/java/mage/verify/VerifyCardDataTest.java +++ b/Mage.Verify/src/test/java/mage/verify/VerifyCardDataTest.java @@ -5,10 +5,7 @@ import com.google.gson.Gson; import mage.MageObject; import mage.Mana; import mage.ObjectColor; -import mage.abilities.Ability; -import mage.abilities.AbilityImpl; -import mage.abilities.Mode; -import mage.abilities.TriggeredAbility; +import mage.abilities.*; import mage.abilities.common.*; import mage.abilities.condition.Condition; import mage.abilities.costs.Cost; @@ -2081,7 +2078,6 @@ public class VerifyCardDataTest { } } } - private void checkSubtypes(Card card, MtgJsonCard ref) { if (skipListHaveName(SKIP_LIST_SUBTYPE, card.getExpansionSetCode(), card.getName())) { return; @@ -2152,6 +2148,73 @@ public class VerifyCardDataTest { } } + // There are many cards that use the word "target" or "targets" in reference to spells/abilities that target rather than actually themselves having a target. + // Examples include Wall of Shadows, Psychic Battle, Coalition Honor Guard, Aboleth Spawn, Akroan Crusader, Grip of Chaos, and many more + Pattern singularTargetRegexPattern = Pattern.compile("\\b(? recursiveTargetAbilityCheck(ability, depth - 1)); + } + if (obj instanceof Collection) { + return ((Collection) obj).stream().anyMatch(x -> recursiveTargetObjectCheck(x, depth - 1)); + } + return false; + } + + boolean recursiveTargetEffectCheck(Effect effect, int depth) { + if (depth < 0) { + return false; + } + return Arrays.stream(effect.getClass().getDeclaredFields()) + .anyMatch(f -> { + f.setAccessible(true); + try { + return recursiveTargetObjectCheck(f.get(effect), depth); // Intentionally not decreasing depth here + } catch (IllegalAccessException ex) { + throw new RuntimeException(ex); // Should never happen due to setAccessible + } + }); + } + + boolean recursiveTargetAbilityCheck(Ability ability, int depth) { + if (depth < 0) { + return false; + } + Collection modes = ability.getModes().values(); + return modes.stream().flatMap(mode -> mode.getTargets().stream()).anyMatch(target -> !target.isNotTarget()) + || ability.getTargetAdjuster() != null + || modes.stream().flatMap(mode -> mode.getEffects().stream()).anyMatch(effect -> recursiveTargetEffectCheck(effect, depth - 1)); + } + private void checkMissingAbilities(Card card, MtgJsonCard ref) { if (skipListHaveName(SKIP_LIST_MISSING_ABILITIES, card.getExpansionSetCode(), card.getName())) { return; @@ -2315,57 +2378,51 @@ public class VerifyCardDataTest { } } + // special check: wrong targeted ability - // possible fixes: - // * on "must set withNotTarget(true)": - // - check card's ability constructors and fix missing withNotTarget(true) param/field - // - it's can be a keyword action (only mtg rules contains a target word), so add it to the targetedKeywords - // * on "must be targeted": - // - TODO: enable and research checkMissTargeted - too much errors with it (is it possible to use that checks?) - boolean checkMissNonTargeted = true; // must set withNotTarget(true) - boolean checkMissTargeted = false; // must be targeted - List targetedKeywords = Arrays.asList( - "target", - "enchant", - "equip", - "backup", - "modular", - "partner" - ); - // xmage card can contain rules text from both sides, so must search ref card for all sides too - String additionalName; - if (card instanceof CardWithSpellOption) { - // adventure/omen cards - additionalName = ((CardWithSpellOption) card).getSpellCard().getName(); - } else if (card.isTransformable() && !card.isNightCard()) { - additionalName = card.getSecondCardFace().getName(); - } else { - additionalName = null; - } - if (additionalName != null) { - MtgJsonCard additionalRef = MtgJsonService.cardFromSet(card.getExpansionSetCode(), additionalName, card.getCardNumber()); - if (additionalRef == null) { - // how-to fix: add new card type processing for an additionalName searching above - fail(card, "abilities", "can't find second side info for target check"); - } else { - if (additionalRef.text != null && !additionalRef.text.isEmpty()) { - refLowerText += "\r\n" + additionalRef.text.toLowerCase(Locale.ENGLISH); + // Checks that no ability targets use withNotTarget (use OneShotNonTargetEffect if it's a choose effect) + // Checks that, if the text contains the word target, the ability does have a target. + // - In cases involving a target in a reflexive trigger or token or other complex situation, it assumes that it's fine + // - There are two versions of this complexity check, either can trigger: one on card text, one that uses Java reflection to inspect the ability's effects. + String[] excludedCards = {"Lodestone Bauble", // Needs to choose a player before targets are selected + "Blink", // Current XMage code does not correctly support non-consecutive chapter effects, duplicates effects as a workaround + "Artifact Ward"}; // This card is just implemented wrong, but would need significant work to fix + if (Arrays.stream(excludedCards).noneMatch(x -> x.equals(ref.name))) { + for (Ability ability : card.getAbilities()) { + boolean foundNotTarget = ability.getModes().values().stream() + .flatMap(mode -> mode.getTargets().stream()).anyMatch(Target::isNotTarget); + if (foundNotTarget) { + fail(card, "abilities", "notTarget should not be used as ability target, should be inside ability effect"); + } + String abilityText = ability.getRule().toLowerCase(Locale.ENGLISH); + boolean needTargetedAbility = singularTargetRegexPattern.matcher(abilityText).find() || pluralTargetsRegexPattern.matcher(abilityText).find() || targetKeywordRegexPattern.matcher(abilityText).find(); + boolean recursiveAbilityText = indirectTriggerTargetRegexPattern.matcher(abilityText).find() || quotedTargetRegexPattern.matcher(abilityText).find(); + + boolean foundTargetedAbility = recursiveTargetAbilityCheck(ability, 0); + boolean recursiveAbility = recursiveTargetAbilityCheck(ability, 4); + + if (needTargetedAbility && !(foundTargetedAbility || recursiveAbilityText || recursiveAbility) + && card.getAbilities().stream().noneMatch(x -> x instanceof LevelUpAbility)) { // Targeting Level Up abilities' text is put in the power-toughness setting effect + fail(card, "abilities", "wrong target settings (must be targeted, but is not):" + ability.getClass().getSimpleName()); + } + if (!needTargetedAbility && foundTargetedAbility + && !(ability instanceof SpellAbility && abilityText.equals("") && card.getSubtype().contains(SubType.AURA)) // Auras' SpellAbility targets, not the EnchantAbility + && !(ability instanceof SpellAbility && (recursiveTargetAbilityCheck(card.getSpellAbility(), 0))) // SurgeAbility is a modified copy of the main SpellAbility, so it targets + && !(ability instanceof SpellTransformedAbility)) { // DisturbAbility targets if the backside aura targets + fail(card, "abilities", "wrong target settings (targeted ability found but no target in text):" + ability.getClass().getSimpleName()); } } - } - - boolean needTargetedAbility = targetedKeywords.stream().anyMatch(refLowerText::contains); - boolean foundTargetedAbility = card.getAbilities() - .stream() - .map(Ability::getTargets) - .flatMap(Collection::stream) - .anyMatch(target -> !target.isNotTarget()); - boolean foundProblem = needTargetedAbility != foundTargetedAbility; - if (checkMissTargeted && needTargetedAbility && foundProblem) { - fail(card, "abilities", "wrong target settings (must be targeted, but it not)"); - } - if (checkMissNonTargeted && !needTargetedAbility && foundProblem) { - fail(card, "abilities", "wrong target settings (must set withNotTarget(true), but it not)"); + // Also check that the reference text and the final ability text have the same number of "target" + String preparedRefText = refLowerText.replaceAll("\\([^)]+\\)", ""); // Remove reminder text + int refTargetCount = (preparedRefText.length() - preparedRefText.replace("target", "").length()); + String preparedRuleText = cardLowerText.replaceAll("\\([^)]+\\)", ""); + if (!ref.subtypes.contains("Adventure") && !ref.subtypes.contains("Omen")) { + preparedRuleText = preparedRuleText.replaceAll("^(adventure|omen).*", ""); + } + int cardTargetCount = (preparedRuleText.length() - preparedRuleText.replace("target", "").length()); + if (refTargetCount != cardTargetCount) { + fail(card, "abilities", "target count text discrepancy: " + (refTargetCount / 6) + " in reference but " + (cardTargetCount / 6) + " in card."); + } } // special check: missing or wrong ability/effect rules hint