Improve trigger auto ordering and aura attachment (Lynde QOL fixes) (#10648)

* Lynde trigger references objects

* Changed triggered ability auto-order to care about targets

* Copy card test

* When a copy effect comes in as a copy of an aura,
it checks if it's already attached before asking player to attach

* add comments and null check per review

---------

Co-authored-by: xenohedron <xenohedron@users.noreply.github.com>

Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java

@@ -8,6 +8,7 @@ import mage.abilities.costs.common.SacrificeSourceCost;
 import mage.abilities.costs.common.TapSourceCost;
 import mage.abilities.costs.mana.ManaCost;
 import mage.abilities.costs.mana.ManaCostsImpl;
+import mage.abilities.effects.Effects;
 import mage.abilities.effects.RequirementEffect;
 import mage.abilities.hint.HintUtils;
 import mage.abilities.mana.ActivatedManaAbilityImpl;
@@ -44,6 +45,7 @@ import mage.target.TargetPermanent;
 import mage.target.common.TargetAnyTarget;
 import mage.target.common.TargetAttackingCreature;
 import mage.target.common.TargetDefender;
+import mage.target.targetpointer.TargetPointer;
 import mage.util.*;
 import org.apache.log4j.Logger;
 
@@ -1341,12 +1343,14 @@ public class HumanPlayer extends PlayerImpl {
             return null;
         }
 
+        // automatically order triggers with same ability, rules text, and targets
         String autoOrderRuleText = null;
+        Ability autoOrderAbility = null;
         boolean autoOrderUse = getControllingPlayersUserData(game).isAutoOrderTrigger();
         while (canRespond()) {
             // try to set trigger auto order
-            java.util.List<TriggeredAbility> abilitiesWithNoOrderSet = new ArrayList<>();
+            List<TriggeredAbility> abilitiesWithNoOrderSet = new ArrayList<>();
-            java.util.List<TriggeredAbility> abilitiesOrderLast = new ArrayList<>();
+            List<TriggeredAbility> abilitiesOrderLast = new ArrayList<>();
             for (TriggeredAbility ability : abilities) {
                 if (triggerAutoOrderAbilityFirst.contains(ability.getOriginalId())) {
                     return ability;
@@ -1366,12 +1370,33 @@ public class HumanPlayer extends PlayerImpl {
                     continue;
                 }
                 if (autoOrderUse) {
-                    // multiple triggers with same rule text will be auto-ordered
+                    // multiple triggers with same rule text will be auto-ordered if possible
+                    // if different, must use choose dialog
                     if (autoOrderRuleText == null) {
+                        // first trigger, store rule text and ability to compare subsequent triggers
                         autoOrderRuleText = rule;
+                        autoOrderAbility = ability;
                     } else if (!rule.equals(autoOrderRuleText)) {
-                        // diff triggers, so must use choose dialog
+                        // disable auto order if rule text is different
                         autoOrderUse = false;
+                    } else {
+                        // disable auto order if targets are different
+                        Effects effects1 = autoOrderAbility.getEffects();
+                        Effects effects2 = ability.getEffects();
+                        if (effects1.size() != effects2.size()) {
+                            autoOrderUse = false;
+                        } else {
+                            for (int i = 0; i < effects1.size(); i++) {
+                                TargetPointer targetPointer1 = effects1.get(i).getTargetPointer();
+                                TargetPointer targetPointer2 = effects2.get(i).getTargetPointer();
+                                List<UUID> targets1 = (targetPointer1 == null) ? new ArrayList<>() : targetPointer1.getTargets(game, autoOrderAbility);
+                                List<UUID> targets2 = (targetPointer2 == null) ? new ArrayList<>() : targetPointer2.getTargets(game, ability);
+                                if (!targets1.equals(targets2)) {
+                                    autoOrderUse = false;
+                                    break;
+                                }
+                            }
+                        }
                     }
                 }
                 abilitiesWithNoOrderSet.add(ability);
@@ -1382,8 +1407,7 @@ public class HumanPlayer extends PlayerImpl {
                 return abilitiesOrderLast.stream().findFirst().orElse(null);
             }
 
-            if (abilitiesWithNoOrderSet.size() == 1
+            if (abilitiesWithNoOrderSet.size() == 1 || autoOrderUse) {
-                    || autoOrderUse) {
                 return abilitiesWithNoOrderSet.iterator().next();
             }
 

Mage.Sets/src/mage/cards/l/LyndeCheerfulTormentor.java

@@ -25,6 +25,7 @@ import mage.game.permanent.Permanent;
 import mage.players.Player;
 import mage.target.TargetPermanent;
 import mage.target.common.TargetOpponent;
+import mage.target.targetpointer.FixedTarget;
 
 /**
  *
@@ -91,7 +92,9 @@ class LyndeCheerfulTormentorCurseDiesTriggeredAbility extends TriggeredAbilityIm
         if (zEvent.isDiesEvent()) {
             Permanent permanent = zEvent.getTarget();
             if (permanent != null && permanent.hasSubtype(SubType.CURSE, game) && permanent.isOwnedBy(controllerId)) {
-                getEffects().setValue("curse", new MageObjectReference(game.getCard(zEvent.getTargetId()), game));
+                MageObjectReference curse = new MageObjectReference(game.getCard(zEvent.getTargetId()), game);
+                getEffects().setValue("curse", curse);
+                getEffects().setTargetPointer(new FixedTarget(curse));
                 return true;
             }
         }

Mage.Tests/src/test/java/org/mage/test/cards/single/mic/LyndeCheerfulTormentorTest.java

@@ -83,4 +83,78 @@ public class LyndeCheerfulTormentorTest extends CardTestPlayerBase {
         assertPermanentCount(playerA, curse, 0);
         assertExileCount(playerA, curse, 1);
     }
+
+    /**
+     * When Lynde brings back a card which is a copy of a curse and enters again as a copy of a curse, the game mistakenly asks who to attach the curse to,
+     * when it should automatically be attached to the controller of Lynde.
+     */
+    @Test
+    public void copyCardTarget() {
+        // {1}{R} - Curse. On upkeep, deal 1 damage to enchanted player
+        String curse = "Curse of the Pierced Heart";
+        // {2}{U}{U} - Enters as a copy of a curse
+        String copy = "Clever Impersonator";
+        // {1} - Sacrifice a permanent
+        String sac = "Claws of Gix";
+        String island = "Island";
+        String mountain = "Mountain";
+
+        // The necessary cards for the test
+        addCard(Zone.HAND, playerA, curse);
+        addCard(Zone.HAND, playerB, copy);
+        addCard(Zone.BATTLEFIELD, playerB, lynde);
+        addCard(Zone.BATTLEFIELD, playerB, sac);
+
+        // Mana needed for player A to cast curse and player B to cast copy and sac
+        addCard(Zone.BATTLEFIELD, playerA, mountain, 2);
+        addCard(Zone.BATTLEFIELD, playerB, island, 5);
+
+        // Player A plays the curse
+        castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, curse, playerB);
+        waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN, 1);
+
+        // Do not use Lynde's ability on upkeep
+        setChoice(playerB, false);
+
+        // Player B took one damage from Player A's curse
+        checkLife("Turn 2 Upkeep", 2, PhaseStep.PRECOMBAT_MAIN, playerB, 20 - 1);
+
+        // Player B casts the copy spell, choosing to copy the curse and enchant player A
+        castSpell(2, PhaseStep.PRECOMBAT_MAIN, playerB, copy);
+        setChoice(playerB, true);
+        setChoice(playerB, curse);
+        setChoice(playerB, playerA.getName());
+        waitStackResolved(2, PhaseStep.PRECOMBAT_MAIN, 1);
+
+        // Now there should be two curses
+        checkPermanentCount("After copy", 2, PhaseStep.PRECOMBAT_MAIN, playerA, curse, 1);
+        checkPermanentCount("After copy", 2, PhaseStep.PRECOMBAT_MAIN, playerB, curse, 1);
+
+        // Player B sacrifices their copy of the curse
+        activateAbility(2, PhaseStep.PRECOMBAT_MAIN, playerB, "{1}, Sacrifice a permanent");
+        setChoice(playerB, curse);
+
+        // Make sure Lynde triggers
+        checkStackObject("After sac", 2, PhaseStep.PRECOMBAT_MAIN, playerB,
+                "Whenever a Curse is put into your graveyard from the battlefield", 1);
+        waitStackResolved(2, PhaseStep.PRECOMBAT_MAIN, 2);
+
+        // Have copy come back as copy of curse. It should be attached to playerB automatically, and no choice should be offered on who to attach it to.
+        setChoice(playerB, true);
+        setChoice(playerB, curse);
+
+        // At the beginning of turn 4, player B should have two triggers
+        setChoice(playerB, "At the beginning of enchanted");
+        setChoice(playerB, false);
+
+        setStopAt(4, PhaseStep.END_TURN);
+        setStrictChooseMode(true);
+        execute();
+
+        assertPermanentCount(playerA, curse, 1);
+        assertPermanentCount(playerB, curse, 1);
+
+        assertLife(playerA, 20);
+        assertLife(playerB, 20 - 2);
+    }
 }

Mage/src/main/java/mage/abilities/effects/common/CopyPermanentEffect.java

@@ -157,10 +157,15 @@ public class CopyPermanentEffect extends OneShotEffect {
 
         // select new target
         auraTarget.withNotTarget(true);
-        if (!controller.choose(auraOutcome, auraTarget, source, game)) {
+
-            return true;
+        UUID targetId = (UUID) game.getState().getValue("attachTo:" + sourcePermanent.getId());
+        if (targetId == null) {
+            if (!controller.choose(auraOutcome, auraTarget, source, game)) {
+                return true;
+            }
+            targetId = auraTarget.getFirstTarget();
         }
-        UUID targetId = auraTarget.getFirstTarget();
+
         Permanent targetPermanent = game.getPermanent(targetId);
         Player targetPlayer = game.getPlayer(targetId);
         if (targetPermanent != null) {