- Fix ChangeTargets to work better with SpellAbility targets

src/main/java/forge/card/spellability/TargetSelection.java

@@ -94,8 +94,9 @@ public class TargetSelection {
         
         final boolean choiceResult;
         if (zone.size() == 1 && zone.get(0) == ZoneType.Stack) {
-            // If Zone is Stack, the choices are handled slightly differently
-            choiceResult = this.chooseCardFromStack(mandatory);
+            // If Zone is Stack, the choices are handled slightly differently.
+            // Handle everything inside function due to interaction with StackInstance
+            return this.chooseCardFromStack(mandatory);
         } else {
             List<Card> validTargets = this.getValidCardsToTarget();
             if (zone.size() == 1 && zone.get(0) == ZoneType.Battlefield) {
@@ -304,41 +305,40 @@ public class TargetSelection {
 
         final GameState game = ability.getActivatingPlayer().getGame();
         for (int i = 0; i < game.getStack().size(); i++) {
-            SpellAbility stackItem = game.getStack().peekAbility(i); 
-            if( ability.canTargetSpellAbility(stackItem))
+            SpellAbility stackItem = game.getStack().peekAbility(i);
+            if (ability.equals(stackItem)) {
+                // By peeking at stack item, target is set to its SI state. So set it back before adding targets
+                tgt.resetTargets();
+            }
+            else if (ability.canTargetSpellAbility(stackItem)) {
                 selectOptions.add(stackItem);
-        }
-        
-        if (tgt.isMinTargetsChosen(this.ability.getSourceCard(), this.ability)) {
-            selectOptions.add("[FINISH TARGETING]");
+            }
         }
 
-        if (selectOptions.isEmpty()) {
-            return false;
-        } else {
-            final Object madeChoice = GuiChoose.oneOrNone(message, selectOptions);
-            if (madeChoice == null) {
-                return false;
-            }
-            if (madeChoice instanceof SpellAbility) {
-                tgt.addTarget(madeChoice);
-            } else // only 'FINISH TARGETING' remains 
+        while(!bTargetingDone) {
+            if (tgt.isMaxTargetsChosen(this.ability.getSourceCard(), this.ability)) {
                 bTargetingDone = true;
+                return true;
+            }
+
+            if (!selectOptions.contains("[FINISH TARGETING]") && tgt.isMinTargetsChosen(this.ability.getSourceCard(), this.ability)) {
+                selectOptions.add("[FINISH TARGETING]");
+            }
+
+            if (selectOptions.isEmpty()) {
+                // Not enough targets, cancel targeting
+                return false;
+            } else {
+                final Object madeChoice = GuiChoose.oneOrNone(message, selectOptions);
+                if (madeChoice == null) {
+                    return false;
+                }
+                if (madeChoice instanceof SpellAbility) {
+                    tgt.addTarget(madeChoice);
+                } else // 'FINISH TARGETING' chosen 
+                    bTargetingDone = true;
+            }
         }
         return true;
     }
-
-    /**
-     * <p>
-     * matchSpellAbility.
-     * </p>
-     * 
-     * @param sa
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     * @param topSA
-     *            a {@link forge.card.spellability.SpellAbility} object.
-     * @param tgt
-     *            a {@link forge.card.spellability.Target} object.
-     * @return a boolean.
-     */
 }

src/main/java/forge/game/zone/MagicStack.java

@@ -625,17 +625,17 @@ public class MagicStack extends MyObservable {
         // TODO: change to use forge.view.FView?
         //GuiDisplayUtil.updateGUI();
 
-        this.freezeStack(); // freeze the stack while we're in the middle of
-                            // resolving
+        // freeze the stack while we're in the middle of resolving
+        this.freezeStack(); 
         this.setResolving(true);
 
-        final SpellAbility sa = this.pop();
+        final SpellAbility sa = this.top();
+
+        // ActivePlayer gains priority first after Resolve
+        game.getPhaseHandler().resetPriority(); 
 
-        game.getPhaseHandler().resetPriority(); // ActivePlayer gains priority first
-                                            // after Resolve
         final Card source = sa.getSourceCard();
         curResolvingCard = source;
-
         
         boolean thisHasFizzled = this.hasFizzled(sa, source, false);
         String messageForLog = thisHasFizzled ? source.getName() + " ability fizzles." : sa.getStackDescription();
@@ -756,8 +756,9 @@ public class MagicStack extends MyObservable {
      */
     public final void finishResolving(final SpellAbility sa, final boolean fizzle) {
 
-        // remove card from the stack
+        // remove SA and card from the stack
         this.removeCardFromStack(sa, fizzle);
+        this.remove(sa);
 
         // After SA resolves we have to do a handful of things
         this.setResolving(false);
@@ -876,13 +877,15 @@ public class MagicStack extends MyObservable {
     public final SpellAbility pop() {
         final SpellAbilityStackInstance si = this.getStack().pop();
         final SpellAbility sp = si.getSpellAbility();
-        // NOTE (12/04/22): Update Observers here causes multi-targeting bug
-        // We Update Observers after the Stack Finishes Resolving
-        // No need to do it sooner
-        //this.updateObservers();
         return sp;
     }
 
+    public final SpellAbility top() {
+        final SpellAbilityStackInstance si = this.getStack().peek();
+        final SpellAbility sa = si.getSpellAbility();
+        return sa;
+    }
+
     // CAREFUL! Peeking while an SAs Targets are being choosen may cause issues
     // index = 0 is the top, index = 1 is the next to top, etc...
     /**