CLB: Firkraag, Cunning Instigator and support (#1019)

* firkraag_cunning_instigator.txt

* CardProperty.cardHasProperty add "mustAttack"

* AttackRequirement.hasRequirement fix (thanks TRT)

* Add timestamp check

* Don't treat Trove of Temptation Requirement as Restriction

* Clean up

Co-authored-by: tool4EvEr <tool4EvEr@192.168.0.59>

forge-game/src/main/java/forge/game/ability/AbilityUtils.java

@@ -147,7 +147,7 @@ public class AbilityUtils {
         } else if (defined.equals("TopOfGraveyard")) {
             final CardCollectionView grave = hostCard.getController().getCardsIn(ZoneType.Graveyard);
 
-            if (grave.size() > 0) { // TopOfLibrary or BottomOfLibrary
+            if (grave.size() > 0) {
                 c = grave.getLast();
             } else {
                 // we don't want this to fall through and return the "Self"

forge-game/src/main/java/forge/game/card/CardProperty.java

@@ -13,6 +13,7 @@ import forge.game.*;
 import forge.game.ability.AbilityKey;
 import forge.game.ability.AbilityUtils;
 import forge.game.card.CardPredicates.Presets;
+import forge.game.combat.AttackRequirement;
 import forge.game.combat.AttackingBand;
 import forge.game.combat.Combat;
 import forge.game.combat.CombatUtil;
@@ -1689,6 +1690,11 @@ public class CardProperty {
             if (band == null || !band.getAttackers().contains(card)) {
                 return false;
             }
+        } else if (property.equals("hadToAttackThisCombat")) {
+            AttackRequirement e = game.getCombat().getAttackConstraints().getRequirements().get(card);
+            if (e == null || !e.hasCreatureRequirement() || !e.getAttacker().equalsWithTimestamp(card)) {
+                return false;
+            }
         } else if (property.equals("couldAttackButNotAttacking")) {
             if (!game.getPhaseHandler().isPlayerTurn(controller)) return false;
             return CombatUtil.couldAttackButNotAttacking(combat, card);

forge-game/src/main/java/forge/game/combat/AttackConstraints.java

@@ -160,7 +160,7 @@ public class AttackConstraints {
                 }
             }
         }
-        myPossibleAttackers.removeAll((Iterable<Card>) attackersToRemove);
+        myPossibleAttackers.removeAll(attackersToRemove);
         for (final Card toRemove : attackersToRemove) {
             reqs.removeAll(findAll(reqs, toRemove));
         }
@@ -186,8 +186,9 @@ public class AttackConstraints {
         // Now try all others (plus empty attack) and count their violations
         final FCollection<Map<Card, GameEntity>> legalAttackers = collectLegalAttackers(reqs, myMax);
         possible.putAll(Maps.asMap(legalAttackers.asSet(), FN_COUNT_VIOLATIONS));
-        if (countViolations(Collections.emptyMap()) != -1) {
+        int empty = countViolations(Collections.emptyMap());
-            possible.put(Collections.emptyMap(), countViolations(Collections.emptyMap()));
+        if (empty != -1) {
+            possible.put(Collections.emptyMap(), empty);
         }
  
         // take the case with the fewest violations
@@ -392,7 +393,7 @@ public class AttackConstraints {
      *         restriction is violated.
      */
     public final int countViolations(final Map<Card, GameEntity> attackers) {
-        if (!globalRestrictions.isLegal(attackers, possibleAttackers)) {
+        if (!globalRestrictions.isLegal(attackers)) {
             return -1;
         }
         for (final Entry<Card, GameEntity> attacker : attackers.entrySet()) {

forge-game/src/main/java/forge/game/combat/AttackRequirement.java

@@ -107,8 +107,17 @@ public class AttackRequirement {
         }
     }
 
+    public Card getAttacker() {
+        return attacker;
+    }
+
     public boolean hasRequirement() {
-        return !defenderSpecific.isEmpty() || !causesToAttack.isEmpty() || !defenderOrPWSpecific.isEmpty();
+        return defenderSpecific.countAll() > 0 || causesToAttack.countAll() > 0 || defenderOrPWSpecific.countAll() > 0;
+    }
+
+    //  according to Firkraag ruling Trove of Temptation applies to players, not creatures
+    public boolean hasCreatureRequirement() {
+        return defenderSpecific.countAll() > 0 || causesToAttack.countAll() > 0;
     }
 
     public final MapToAmount<Card> getCausesToAttack() {
@@ -124,29 +133,24 @@ public class AttackRequirement {
         int violations = 0;
 
         // first. check to see if "must attack X or Y with at least one creature" requirements are satisfied
-        //List<GameEntity> toRemoveFromDefSpecific = Lists.newArrayList();
+        for (GameEntity def : defenderOrPWSpecific.keySet()) {
-        if (!defenderOrPWSpecific.isEmpty()) {
+            boolean isAttackingDefender = false;
-            for (GameEntity def : defenderOrPWSpecific.keySet()) {
+            outer: for (Card atk : attackers.keySet()) {
-                if (defenderSpecificAlternatives.containsKey(def)) {
+                // is anyone attacking this defender or any of the alternative defenders?
-                    boolean isAttackingDefender = false;
+                if (attackers.get(atk).equals(def)) {
-                    outer: for (Card atk : attackers.keySet()) {
+                    isAttackingDefender = true;
-                        // is anyone attacking this defender or any of the alternative defenders?
+                    break;
-                        if (attackers.get(atk).equals(def)) {
+                }
-                            isAttackingDefender = true;
+                for (GameEntity altDef : defenderSpecificAlternatives.get(def)) {
-                            break;
+                    if (attackers.get(atk).equals(altDef)) {
-                        }
+                        isAttackingDefender = true;
-                        for (GameEntity altDef : defenderSpecificAlternatives.get(def)) {
+                        break outer;
-                            if (attackers.get(atk).equals(altDef)) {
-                                isAttackingDefender = true;
-                                break outer;
-                            }
-                        }
-                    }
-                    if (!isAttackingDefender && CombatUtil.getAttackCost(attacker.getGame(), attacker, def) == null) {
-                        violations++; // no one is attacking that defender or any of his PWs
                     }
                 }
             }
+            if (!isAttackingDefender) {
+                violations++; // no one is attacking that defender or any of his PWs
+            }
         }
 
         // now, count everything else

forge-game/src/main/java/forge/game/combat/GlobalAttackRestrictions.java

@@ -9,9 +9,7 @@ import forge.game.Game;
 import forge.game.GameEntity;
 import forge.game.GlobalRuleChange;
 import forge.game.card.Card;
-import forge.game.card.CardCollection;
 import forge.game.player.Player;
-import forge.game.player.PlayerCollection;
 import forge.game.zone.ZoneType;
 import forge.util.collect.FCollectionView;
 import forge.util.maps.LinkedHashMapToAmount;
@@ -22,11 +20,9 @@ public class GlobalAttackRestrictions {
 
     private final int max;
     private final MapToAmount<GameEntity> defenderMax;
-    private final PlayerCollection mustBeAttackedByEachOpp;
+    private GlobalAttackRestrictions(final int max, final MapToAmount<GameEntity> defenderMax) {
-    private GlobalAttackRestrictions(final int max, final MapToAmount<GameEntity> defenderMax, PlayerCollection mustBeAttackedByEachOpp) {
         this.max = max;
         this.defenderMax = defenderMax;
-        this.mustBeAttackedByEachOpp = mustBeAttackedByEachOpp;
     }
 
     public int getMax() {
@@ -36,17 +32,17 @@ public class GlobalAttackRestrictions {
         return defenderMax;
     }
 
-    public boolean isLegal(final Map<Card, GameEntity> attackers, final CardCollection possibleAttackers) {
+    public boolean isLegal(final Map<Card, GameEntity> attackers) {
-        return !getViolations(attackers, possibleAttackers, true).isViolated();
+        return !getViolations(attackers, true).isViolated();
     }
 
-    public GlobalAttackRestrictionViolations getViolations(final Map<Card, GameEntity> attackers, final CardCollection possibleAttackers) {
+    public GlobalAttackRestrictionViolations getViolations(final Map<Card, GameEntity> attackers) {
-        return getViolations(attackers, possibleAttackers, false);
+        return getViolations(attackers, false);
     }
-    private GlobalAttackRestrictionViolations getViolations(final Map<Card, GameEntity> attackers, final CardCollection possibleAttackers, final boolean returnQuickly) {
+    private GlobalAttackRestrictionViolations getViolations(final Map<Card, GameEntity> attackers, final boolean returnQuickly) {
         final int nTooMany = max < 0 ? 0 : attackers.size() - max;
         if (returnQuickly && nTooMany > 0) {
-            return new GlobalAttackRestrictionViolations(nTooMany, MapToAmountUtil.emptyMap(), MapToAmountUtil.emptyMap());
+            return new GlobalAttackRestrictionViolations(nTooMany, MapToAmountUtil.emptyMap());
         }
 
         final MapToAmount<GameEntity> defenderTooMany = new LinkedHashMapToAmount<>(defenderMax.size());
@@ -77,49 +73,18 @@ public class GlobalAttackRestrictions {
             }
         }
 
-        final MapToAmount<GameEntity> defenderTooFew = new LinkedHashMapToAmount<>(defenderMax.size());
+        return new GlobalAttackRestrictionViolations(nTooMany, defenderTooMany);
-        for (final GameEntity mandatoryDef : mustBeAttackedByEachOpp) {
-            // check to ensure that this defender can even legally be attacked in the first place
-            boolean canAttackThisDef = false;
-            for (Card c : possibleAttackers) {
-                if (CombatUtil.canAttack(c, mandatoryDef) && null == CombatUtil.getAttackCost(c.getGame(), c, mandatoryDef)) {
-                    canAttackThisDef = true;
-                    break;
-                }
-            }
-            if (!canAttackThisDef) {
-                continue;
-            }
-
-            boolean isAttacked = false;
-            for (final GameEntity defender : attackers.values()) {
-                if (defender.equals(mandatoryDef)) {
-                    isAttacked = true;
-                    break;
-                } else if (defender instanceof Card && ((Card)defender).getController().equals(mandatoryDef)) {
-                    isAttacked = true;
-                    break;
-                }
-            }
-            if (!isAttacked) {
-                defenderTooFew.add(mandatoryDef);
-            }
-        }
-
-        return new GlobalAttackRestrictionViolations(nTooMany, defenderTooMany, defenderTooFew);
     }
 
     final class GlobalAttackRestrictionViolations {
         private final boolean isViolated;
         private final int globalTooMany;
         private final MapToAmount<GameEntity> defenderTooMany;
-        private final MapToAmount<GameEntity> defenderTooFew;
 
-        public GlobalAttackRestrictionViolations(final int globalTooMany, final MapToAmount<GameEntity> defenderTooMany, final MapToAmount<GameEntity> defenderTooFew) {
+        public GlobalAttackRestrictionViolations(final int globalTooMany, final MapToAmount<GameEntity> defenderTooMany) {
-            this.isViolated = globalTooMany > 0 || !defenderTooMany.isEmpty() || !defenderTooFew.isEmpty();
+            this.isViolated = globalTooMany > 0 || !defenderTooMany.isEmpty();
             this.globalTooMany = globalTooMany;
             this.defenderTooMany = defenderTooMany;
-            this.defenderTooFew = defenderTooFew;
         }
         public boolean isViolated() {
             return isViolated;
@@ -127,9 +92,6 @@ public class GlobalAttackRestrictions {
         public int getGlobalTooMany() {
             return globalTooMany;
         }
-        public MapToAmount<GameEntity> getDefenderTooFew() {
-            return defenderTooFew;
-        }
         public MapToAmount<GameEntity> getDefenderTooMany() {
             return defenderTooMany;
         }
@@ -147,7 +109,6 @@ public class GlobalAttackRestrictions {
     public static GlobalAttackRestrictions getGlobalRestrictions(final Player attackingPlayer, final FCollectionView<GameEntity> possibleDefenders) {
         int max = -1;
         final MapToAmount<GameEntity> defenderMax = new LinkedHashMapToAmount<>(possibleDefenders.size());
-        final PlayerCollection mustBeAttacked = new PlayerCollection();
         final Game game = attackingPlayer.getGame();
 
         /* if (game.getStaticEffects().getGlobalRuleChange(GlobalRuleChange.onlyOneAttackerATurn)) {
@@ -171,14 +132,6 @@ public class GlobalAttackRestrictions {
             }
         }
 
-        for (final Card card : game.getCardsIn(ZoneType.Battlefield)) {
-            if (card.hasKeyword("Each opponent must attack you or a planeswalker you control with at least one creature each combat if able.")) {
-                if (attackingPlayer.isOpponentOf(card.getController())) {
-                    mustBeAttacked.add(card.getController());
-                }
-            }
-        }
-
         for (final GameEntity defender : possibleDefenders) {
             final int defMax = getMaxAttackTo(defender);
             if (defMax != -1) {
@@ -190,7 +143,7 @@ public class GlobalAttackRestrictions {
             max = Ints.min(max, defenderMax.countAll());
         }
 
-        return new GlobalAttackRestrictions(max, defenderMax, mustBeAttacked);
+        return new GlobalAttackRestrictions(max, defenderMax);
     }
 
     /**

forge-gui/res/cardsfolder/upcoming/firkraag_cunning_instigator.txt

@@ -0,0 +1,14 @@
+Name:Firkraag, Cunning Instigator
+ManaCost:3 U R
+Types:Legendary Creature Dragon
+PT:3/3
+K:Flying
+K:Haste
+T:Mode$ AttackersDeclaredOneTarget | ValidAttackers$ Dragon.YouCtrl | AttackedTarget$ Opponent | Execute$ TrigGoad | TriggerZones$ Battlefield | TriggerDescription$ Whenever one or more Dragons you control attack an opponent, goad target creature that player controls.
+SVar:TrigGoad:DB$ Goad | ValidTgts$ Creature | TargetsWithDefinedController$ TriggeredAttackedTarget | TgtPrompt$ Select target creature that player controls
+T:Mode$ DamageDone | ValidSource$ Creature.hadToAttackThisCombat | ValidTarget$ Opponent | CombatDamage$ True | TriggerZones$ Battlefield | Execute$ TrigPutCounter | TriggerDescription$ Whenever a creature deals combat damage to one of your opponents, if that creature had to attack this combat, you put a +1/+1 counter on CARDNAME and you draw a card.
+SVar:TrigPutCounter:DB$ PutCounter | CounterType$ P1P1 | SubAbility$ DBDraw
+SVar:DBDraw:DB$ Draw
+DeckHints:Type$Dragon
+DeckHas:Ability$Counters
+Oracle:Flying, haste\nWhenever one or more Dragons you control attack an opponent, goad target creature that player controls.\nWhenever a creature deals combat damage to one of your opponents, if that creature had to attack this combat, you put a +1/+1 counter on Firkraag, Cunning Instigator and you draw a card.