"take priority" changed: now on priority SBA is checked, triggers are ordered, then player is asked for spell they want to play, then spell is played. Until player refuses to choose a spell, that means "I pass"

forge-game/src/main/java/forge/ai/AiController.java

@@ -55,6 +55,7 @@ import forge.game.cost.CostPart;
 import forge.game.phase.PhaseType;
 import forge.game.player.Player;
 import forge.game.player.PlayerActionConfirmMode;
+import forge.game.spellability.Ability;
 import forge.game.spellability.AbilityManaPart;
 import forge.game.spellability.Spell;
 import forge.game.spellability.SpellAbility;
@@ -735,21 +736,25 @@ public class AiController {
         return null;
     }
     
-    public void onPriorityRecieved() {
+    public SpellAbility choooseSpellAbilityToPlay() {
         final PhaseType phase = game.getPhaseHandler().getPhase();
-        switch(phase) {
-            case MAIN1:
-            case MAIN2:
-                Log.debug("Computer " + phase.nameForUi);
 
-                if (game.getStack().isEmpty())
+        if (game.getStack().isEmpty() && phase.isMain()) {
-                    playLands();
+            Log.debug("Computer " + phase.nameForUi);
-                // fall through is intended
+            List<Card> landsWannaPlay = getLandsToPlay();
-            default:
+            if(landsWannaPlay != null && !landsWannaPlay.isEmpty() && player.canPlayLand(null)) {
-                playSpellAbilities(game);
+                Card land = chooseBestLandToPlay(landsWannaPlay);
-                break;
+                if (ComputerUtil.damageFromETB(player, land) < player.getLife() || !player.canLoseLife()) {
+                    Ability.PLAY_LAND_SURROGATE.setSourceCard(land);
+                    return Ability.PLAY_LAND_SURROGATE;
                 }
             }
+        }
+
+        SpellAbility sa = getSpellAbilityToPlay();
+        // System.out.println("Chosen to play: " + sa);
+        return sa;
+    }
 
     // declares blockers for given defender in a given combat
     public void declareBlockersFor(Player defender, Combat combat) {
@@ -778,37 +783,6 @@ public class AiController {
         return combat;
     }
 
-    private void playLands() {
-        final Player player = getPlayer();
-        List<Card> landsWannaPlay = getLandsToPlay();
-        
-        while(landsWannaPlay != null && !landsWannaPlay.isEmpty() && player.canPlayLand(null)) {
-            Card land = chooseBestLandToPlay(landsWannaPlay);
-            if (ComputerUtil.damageFromETB(player, land) >= player.getLife() && player.canLoseLife()) {
-                break;
-            }
-            landsWannaPlay.remove(land);
-            player.playLand(land, false);
-            game.getPhaseHandler().setPriority(player);
-            game.getAction().checkStateEffects();
-        }
-    }
-
-    private void playSpellAbilities(final Game game)
-    {
-        SpellAbility sa;
-        do { 
-            if ( game.isGameOver() )
-                return;
-            sa = getSpellAbilityToPlay();
-            if ( sa == null ) break;
-            //System.out.println("Playing sa: " + sa);
-            if (!ComputerUtil.handlePlayingSpellAbility(player, sa, game)) {
-                break;
-            }
-        } while ( sa != null );
-    }
-
     private final SpellAbility getSpellAbilityToPlay() {
         // if top of stack is owned by me
         if (!game.getStack().isEmpty() && game.getStack().peekAbility().getActivatingPlayer().equals(player)) {

forge-game/src/main/java/forge/game/phase/PhaseHandler.java

@@ -975,15 +975,23 @@ public class PhaseHandler implements java.io.Serializable {
                     sw.start();
                 }
 
+                game.fireEvent(new GameEventPlayerPriority(getPlayerTurn(), getPhase(), getPriorityPlayer()));
+                SpellAbility chosenSa = null;
+
+                do {
                     // Rule 704.3  Whenever a player would get priority, the game checks ... for state-based actions,
                     game.getAction().checkStateEffects();
-                game.fireEvent(new GameEventPlayerPriority(getPlayerTurn(), getPhase(), getPriorityPlayer()));
+                    if (game.isGameOver())
-
+                        return; // state-based effects check could lead to game over
-                // SBA could lead to game over
-                if (game.isGameOver()) { return; }
 
                     game.getStack().chooseOrderOfSimultaneousStackEntry(pPlayerPriority);
-                pPlayerPriority.getController().takePriority();
+
+                    chosenSa = pPlayerPriority.getController().chooseSpellAbilityToPlay();
+                    if( null == chosenSa )
+                        break; // that means 'I pass'
+
+                    pPlayerPriority.getController().playChosenSpellAbility(chosenSa);
+                } while (chosenSa != null);
 
                 if (DEBUG_PHASES) {
                     sw.stop();
@@ -1006,18 +1014,7 @@ public class PhaseHandler implements java.io.Serializable {
             if( DEBUG_PHASES )
                 System.out.println(String.format("%s %s: %s passes priority to %s", playerTurn, phase, pPlayerPriority, nextPlayer));
             if (getFirstPriority() == nextPlayer) {
-                
+                if (game.getStack().isEmpty()) {
-                if (game.getStack().hasSimultaneousStackEntries() && givePriorityToPlayer)
-                {
-                    Player ap = nextPlayer;
-                    Player nap = game.getNextPlayerAfter(ap);
-                    game.getStack().chooseOrderOfSimultaneousStackEntry(ap);
-                    do {
-                        game.getStack().chooseOrderOfSimultaneousStackEntry(nap);
-                        nap = game.getNextPlayerAfter(nap);
-                    } while( nap != ap);
-                    // All have passed, but there's something waiting to be added to stack. After that and give them priority again
-                } else if (game.getStack().isEmpty()) {
                     this.setPriority(this.getPlayerTurn()); // this needs to be set early as we exit the phase
 
                     // end phase

forge-game/src/main/java/forge/game/player/PlayerController.java

@@ -180,7 +180,8 @@ public abstract class PlayerController {
 
     public abstract void declareAttackers(Player attacker, Combat combat);
     public abstract void declareBlockers(Player defender, Combat combat);
-    public abstract void takePriority();
+    public abstract SpellAbility chooseSpellAbilityToPlay();
+    public abstract void playChosenSpellAbility(SpellAbility sa);
 
     public abstract List<Card> chooseCardsToDiscardToMaximumHandSize(int numDiscard);
     public abstract boolean payManaOptional(Card card, Cost cost, SpellAbility sa, String prompt, ManaPaymentPurpose purpose);

forge-game/src/main/java/forge/game/player/PlayerControllerAi.java

@@ -394,11 +394,18 @@ public class PlayerControllerAi extends PlayerController {
     }
 
     @Override
-    public void takePriority() {
+    public SpellAbility chooseSpellAbilityToPlay() {
-        if (!game.isGameOver()) {
+        return brains.choooseSpellAbilityToPlay();
-            brains.onPriorityRecieved();
     }
-        // use separate thread for AI?
+    
+    @Override
+    public void playChosenSpellAbility(SpellAbility sa)
+    {
+        // System.out.println("Playing sa: " + sa);
+        if ( sa == Ability.PLAY_LAND_SURROGATE )
+            player.playLand(sa.getSourceCard(), false);
+        else
+            ComputerUtil.handlePlayingSpellAbility(player, sa, game);
     }    
 
     @Override

forge-gui/src/main/java/forge/gui/player/PlayerControllerHuman.java

@@ -669,26 +669,26 @@ public class PlayerControllerHuman extends PlayerController {
     }
 
     @Override
-    public void takePriority() {
+    public SpellAbility chooseSpellAbilityToPlay() {
         PhaseType phase = game.getPhaseHandler().getPhase();
+        
         boolean maySkipPriority = mayAutoPass(phase) || isUiSetToSkipPhase(game.getPhaseHandler().getPlayerTurn(), phase);
         if (game.getStack().isEmpty() && maySkipPriority) {
-            return;
+            return null;
         }
         else {
             autoPassCancel(); // probably cancel, since something has happened
         }
         
-        SpellAbility chosenSa = null;
-        do {
-            if (chosenSa != null) {
-                HumanPlay.playSpellAbility(player, chosenSa);
-                if (game.isGameOver()) { return; } //don't wait to pass priority if player conceded while in middle of playing a spell/ability
-            }
         InputPassPriority defaultInput = new InputPassPriority(player);
         defaultInput.showAndWait();
-            chosenSa = defaultInput.getChosenSa();
+        return defaultInput.getChosenSa();
-        } while (chosenSa != null);
+    }
+    
+    @Override
+    public void playChosenSpellAbility(SpellAbility chosenSa)
+    {
+        HumanPlay.playSpellAbility(player, chosenSa);
     }
 
     @Override

forge-gui/src/test/java/forge/gamesimulationtests/util/PlayerControllerForTests.java

@@ -363,8 +363,9 @@ public class PlayerControllerForTests extends PlayerController {
 	}
 
 	@Override
-	public void takePriority() {
+	public SpellAbility chooseSpellAbilityToPlay() {
-		//TODO: just about everything...
+		//TODO: This method has to return the spellability chosen by player
+	    // It should not play the sa right from here. The code has been left as it is to quickly adapt to changed playercontroller interface 
 		if (playerActions != null) {
 			CastSpellFromHandAction castSpellFromHand = playerActions.getNextActionIfApplicable(player, game, CastSpellFromHandAction.class);
 			if (castSpellFromHand != null) {
@@ -376,6 +377,7 @@ public class PlayerControllerForTests extends PlayerController {
 				activateAbilityAction.activateAbility(player, game);
 			}
 		}
+		return null;
 	}
 
 	@Override
@@ -582,4 +584,10 @@ public class PlayerControllerForTests extends PlayerController {
         // Probably along with deciding how many creatures to tap
         return new HashMap<Card, ManaCostShard>();
     }
+
+    @Override
+    public void playChosenSpellAbility(SpellAbility sa) {
+        // TODO Play abilities from here
+        
+    }
 }