[refactor/bugfix] use rule 802.2a where appropriate. (magefree#13179)

* [refactor/bugfix] use rule 802.2a where appropriate.

Many effects which relied on getDefendingPlayerId would fail if the attacking creature had been removed from combat before they resolved, in which case the defending player ID would be null. This fixes these issues.

* Add test for removing attacking creature with Defending Player triggered ability.

Change allowFormer to be true by default, reduce falses to only necessary cases.

Author: Grath

Mage.Sets/src/mage/cards/b/BeckoningWillOWisp.java

@@ -70,7 +70,7 @@ enum BeckoningWillOWispPredicate implements ObjectSourcePlayerPredicate<Permanen
     @Override
     public boolean apply(ObjectSourcePlayer<Permanent> input, Game game) {
         UUID playerId = (UUID) game.getState().getValue(input.getSourceId() + "_" + game.getState().getZoneChangeCounter(input.getSourceId()) + "_chosenOpponent");
-        return playerId != null && playerId.equals(game.getCombat().getDefendingPlayerId(input.getObject().getId(), game));
+        return playerId != null && playerId.equals(game.getCombat().getDefendingPlayerId(input.getObject().getId(), game, false));
     }
 }
 

Mage.Sets/src/mage/cards/o/OgreMarauder.java

@@ -21,7 +21,6 @@
 import mage.filter.StaticFilters;
 import mage.game.Game;
 import mage.players.Player;
-import mage.target.common.TargetControlledCreaturePermanent;
 
 /**
  *

Mage.Sets/src/mage/cards/s/SkymarkRoc.java

@@ -18,7 +18,6 @@
 import mage.filter.predicate.permanent.ControllerIdPredicate;
 import mage.game.Game;
 import mage.game.events.GameEvent;
-import mage.game.events.GameEvent.EventType;
 import mage.target.common.TargetCreaturePermanent;
 
 /**

Mage.Sets/src/mage/cards/u/UlamogTheCeaselessHunger.java

@@ -19,7 +19,6 @@
 import mage.filter.FilterPermanent;
 import mage.game.Game;
 import mage.game.events.GameEvent;
-import mage.game.events.GameEvent.EventType;
 import mage.game.permanent.Permanent;
 import mage.game.stack.Spell;
 import mage.players.Player;
@@ -113,7 +112,7 @@ public boolean checkEventType(GameEvent event, Game game) {
 
     @Override
     public boolean checkTrigger(GameEvent event, Game game) {
-        Permanent sourcePermanent = game.getPermanent(this.getSourceId());
+        Permanent sourcePermanent = game.getPermanentOrLKIBattlefield(this.getSourceId());
         if (sourcePermanent != null
                 && event.getSourceId() != null
                 && event.getSourceId().equals(this.getSourceId())) {

Mage.Sets/src/mage/cards/w/WardscaleDragon.java

@@ -13,7 +13,6 @@
 import mage.constants.SubType;
 import mage.constants.Duration;
 import mage.constants.Outcome;
-import mage.constants.Zone;
 import mage.game.Game;
 import mage.game.events.GameEvent;
 import mage.game.permanent.Permanent;
@@ -73,7 +72,7 @@ public boolean checksEventType(GameEvent event, Game game) {
     public boolean applies(GameEvent event, Ability source, Game game) {
         Permanent sourcePermanent = game.getPermanent(source.getSourceId());
         if (sourcePermanent != null && sourcePermanent.isAttacking()) {
-            return event.getPlayerId().equals(game.getCombat().getDefendingPlayerId(sourcePermanent.getId(), game));
+            return event.getPlayerId().equals(game.getCombat().getDefendingPlayerId(sourcePermanent.getId(), game, false));
         }
         return false;
     }

Mage.Tests/src/test/java/org/mage/test/combat/RemoveFromCombatTest.java

@@ -106,4 +106,30 @@ public void test_Defender_AttackPlaneswalkerAndRemoveDefender() {
         assertLife(playerB, 20 - 2);
         assertGraveyardCount(playerB, "Jace, Memory Adept", 1);
     }
+
+    /**
+     * Validate rule 806.2a: Abilities which refer to Defending Player still mean that defending player, even if the
+     * attacking creature is removed from combat.
+     */
+    @Test
+    public void test_RemoveAttackerWithDefendingPlayerTriggeredAbilityOnStack() {
+
+        addCard(Zone.HAND, playerA, "Swords to Plowshares", 1);
+        addCard(Zone.BATTLEFIELD, playerA, "Agate-Blade Assassin", 1); // 2/2
+        addCard(Zone.BATTLEFIELD, playerA, "Plains", 1);
+
+        // attack player
+        attack(1, playerA, "Agate-Blade Assassin", playerB);
+        // remove Agate-Blade Assassin from combat
+        castSpell(1, PhaseStep.DECLARE_ATTACKERS, playerA, "Swords to Plowshares");
+        addTarget(playerA, "Agate-Blade Assassin");
+
+        setStrictChooseMode(true);
+        setStopAt(1, PhaseStep.END_TURN);
+        execute();
+
+        assertLife(playerB, 20 - 1);
+        assertLife(playerA, 20 + 1 /* StP */ + 1 /* Agate-Blade Assassin trigger */);
+    }
+
 }

Mage/src/main/java/mage/game/combat/Combat.java

@@ -39,6 +39,7 @@
 import java.io.Serializable;
 import java.util.*;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 /**
  * @author BetaSteward_at_googlemail.com
@@ -59,6 +60,7 @@ public class Combat implements Serializable, Copyable<Combat> {
     private final List<FilterCreaturePermanent> useToughnessForDamageFilters = new ArrayList<>();
 
     protected List<CombatGroup> groups = new ArrayList<>();
+    protected List<CombatGroup> formerGroups = new ArrayList<>();
     protected Map<UUID, CombatGroup> blockingGroups = new HashMap<>();
     // all possible defenders (players, planeswalkers or battle)
     protected Set<UUID> defenders = new HashSet<>();
@@ -83,6 +85,9 @@ protected Combat(final Combat combat) {
         for (CombatGroup group : combat.groups) {
             groups.add(group.copy());
         }
+        for (CombatGroup group : combat.formerGroups) {
+            formerGroups.add(group.copy());
+        }
         defenders.addAll(combat.defenders);
         for (Map.Entry<UUID, CombatGroup> group : combat.blockingGroups.entrySet()) {
             blockingGroups.put(group.getKey(), group.getValue());
@@ -181,6 +186,7 @@ public void checkForRemoveFromCombat(Game game) {
 
     public void clear() {
         groups.clear();
+        formerGroups.clear();
         blockingGroups.clear();
         defenders.clear();
         attackingPlayerId = null;
@@ -1679,6 +1685,36 @@ public UUID getDefenderId(UUID attackerId) {
      * @return
      */
     public UUID getDefendingPlayerId(UUID attackingCreatureId, Game game) {
+        return getDefendingPlayerId(attackingCreatureId, game, true);
+    }
+
+    /**
+     * Returns the playerId of the player that is attacked by given attacking
+     * creature or formerly-attacking creature.
+     *
+     * @param attackingCreatureId
+     * @param game
+     * @return
+     */
+    public UUID getDefendingPlayerId(UUID attackingCreatureId, Game game, boolean allowFormer) {
+        if (allowFormer) {
+            /*
+             * 802.2a. Any rule, object, or effect that refers to a "defending player" refers to one specific defending
+             * player, not to all of the defending players. If an ability of an attacking creature refers to a
+             * defending player, or a spell or ability refers to both an attacking creature and a defending player,
+             * then unless otherwise specified, the defending player it's referring to is the player that creature is
+             * attacking, the controller of the planeswalker that creature is attacking, or the protector of the battle
+             * that player is attacking. If that creature is no longer attacking, the defending player it's referring
+             * to is the player that creature was attacking before it was removed from combat, the controller of the
+             * planeswalker that creature was attacking before it was removed from combat, or the protector of the
+             * battle that player was attacking before it was removed from combat.
+             */
+            return Stream.concat(groups.stream(), formerGroups.stream())
+                    .filter(group -> (group.getAttackers().contains(attackingCreatureId) || group.getFormerAttackers().contains(attackingCreatureId)))
+                    .map(CombatGroup::getDefendingPlayerId)
+                    .findFirst()
+                    .orElse(null);
+        }
         return groups
                 .stream()
                 .filter(group -> group.getAttackers().contains(attackingCreatureId))
@@ -1743,6 +1779,7 @@ public void removeAttacker(UUID attackerId, Game game) {
                     }
                 }
                 if (group.attackers.isEmpty()) {
+                    formerGroups.add(group);
                     groups.remove(group);
                 }
                 return;

Mage/src/main/java/mage/game/combat/CombatGroup.java

@@ -27,6 +27,7 @@
 public class CombatGroup implements Serializable, Copyable<CombatGroup> {
 
     protected List<UUID> attackers = new ArrayList<>();
+    protected List<UUID> formerAttackers = new ArrayList<>();
     protected List<UUID> blockers = new ArrayList<>();
     protected List<UUID> blockerOrder = new ArrayList<>();
     protected List<UUID> attackerOrder = new ArrayList<>();
@@ -49,6 +50,7 @@ public CombatGroup(UUID defenderId, boolean defenderIsPermanent, UUID defendingP
 
     protected CombatGroup(final CombatGroup group) {
         this.attackers.addAll(group.attackers);
+        this.formerAttackers.addAll(group.formerAttackers);
         this.blockers.addAll(group.blockers);
         this.blockerOrder.addAll(group.blockerOrder);
         this.attackerOrder.addAll(group.attackerOrder);
@@ -81,6 +83,10 @@ public List<UUID> getAttackers() {
         return attackers;
     }
 
+    public List<UUID> getFormerAttackers() {
+        return formerAttackers;
+    }
+
     public List<UUID> getBlockers() {
         return blockers;
     }
@@ -737,6 +743,7 @@ public boolean removeAttackedPermanent(UUID permanentId) {
     public boolean remove(UUID creatureId) {
         boolean result = false;
         if (attackers.contains(creatureId)) {
+            formerAttackers.add(creatureId);
             attackers.remove(creatureId);
             result = true;
             attackerOrder.remove(creatureId);