Merge branch 'sacrifice' into 'master'

getPlayers: use APNAP order

See merge request core-developers/forge!5510

forge-game/src/main/java/forge/game/Game.java

@@ -405,7 +405,6 @@ public class Game {
         sbaCheckedCommandList.clear();
     }
 
-
     public final PhaseHandler getPhaseHandler() {
         return phaseHandler;
     }
@@ -766,8 +765,7 @@ public class Game {
                 iAlive = ingamePlayers.indexOf(allPlayers.get(iPlayer));
             } while (iAlive < 0);
             iPlayer = iAlive;
-        }
+        } else { // for the case playerTurn hasn't died
-        else { // for the case playerTurn hasn't died
         	final int numPlayersInGame = ingamePlayers.size();
         	iPlayer = (iPlayer + shift) % numPlayersInGame;
         	if (iPlayer < 0) {

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

@@ -1,5 +1,6 @@
 package forge.game.ability;
 
+import java.util.Collections;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -203,8 +204,14 @@ public abstract class SpellAbilityEffect {
 
     private static PlayerCollection getPlayers(final boolean definedFirst, final String definedParam, final SpellAbility sa) {
         final boolean useTargets = sa.usesTargeting() && (!definedFirst || !sa.hasParam(definedParam));
-        return useTargets ? new PlayerCollection(sa.getTargets().getTargetPlayers())
+        PlayerCollection players = useTargets ? new PlayerCollection(sa.getTargets().getTargetPlayers())
                 : AbilityUtils.getDefinedPlayers(sa.getHostCard(), sa.getParam(definedParam), sa);
+        // try sort in APNAP order
+        int indexAP = players.indexOf(sa.getHostCard().getGame().getPhaseHandler().getPlayerTurn());
+        if (indexAP != -1) {
+            Collections.rotate(players, - indexAP);
+        }
+        return players;
     }
 
     // Spells

forge-game/src/main/java/forge/game/ability/effects/SacrificeEffect.java

@@ -95,20 +95,11 @@ public class SacrificeEffect extends SpellAbilityEffect {
         final boolean exploit = sa.hasParam("Exploit");
         final boolean sacEachValid = sa.hasParam("SacEachValid");
 
-        String valid = sa.getParam("SacValid");
+        String valid = sa.getParamOrDefault("SacValid", "Self");
-        if (valid == null) {
+        String msg = sa.getParamOrDefault("SacMessage", valid);
-            valid = "Self";
-        }
-
-        String msg = sa.getParam("SacMessage");
-        if (msg == null) {
-            msg = valid;
-        }
 
         final boolean destroy = sa.hasParam("Destroy");
         final boolean remSacrificed = sa.hasParam("RememberSacrificed");
-        final String remSVar = sa.getParam("RememberSacrificedSVar");
-        int countSacrificed = 0;
         CardZoneTable table = new CardZoneTable();
         Map<AbilityKey, Object> params = AbilityKey.newMap();
         params.put(AbilityKey.LastStateBattlefield, game.copyLastStateBattlefield());
@@ -116,7 +107,6 @@ public class SacrificeEffect extends SpellAbilityEffect {
         if (valid.equals("Self") && game.getZoneOf(card) != null) {
             if (game.getZoneOf(card).is(ZoneType.Battlefield)) {
                 if (game.getAction().sacrifice(card, sa, table, params) != null) {
-                    countSacrificed++;
                     if (remSacrificed) {
                         card.addRemembered(card);
                     }
@@ -198,22 +188,12 @@ public class SacrificeEffect extends SpellAbilityEffect {
                         game.getTriggerHandler().runTrigger(TriggerType.Exploited, runParams, false);
                     }
                     if (wasDestroyed || wasSacrificed) {
-                        countSacrificed++;
                         if (remSacrificed) {
                             card.addRemembered(lKICopy);
                         }
                     }
                 }
             }
-
-            if (remSVar != null) {
-                card.setSVar(remSVar, String.valueOf(countSacrificed));
-                SpellAbility root = sa;
-                do {
-                    root.setSVar(remSVar, String.valueOf(countSacrificed));
-                    root = root.getSubAbility();
-                } while (root != null);
-            }
         }
 
         table.triggerChangesZoneAll(game, sa);
@@ -225,13 +205,9 @@ public class SacrificeEffect extends SpellAbilityEffect {
 
         final List<Player> tgts = getTargetPlayers(sa);
 
-        String valid = sa.getParam("SacValid");
+        String valid = sa.getParamOrDefault("SacValid", "Self");
-        if (valid == null) {
+        String num = sa.getParamOrDefault("Amount", "1");
-            valid = "Self";
-        }
 
-        String num = sa.getParam("Amount");
-        num = (num == null) ? "1" : num;
         final int amount = AbilityUtils.calculateAmount(sa.getHostCard(), num, sa);
 
         if (valid.equals("Self")) {
@@ -244,10 +220,7 @@ public class SacrificeEffect extends SpellAbilityEffect {
                 sb.append(p.getName()).append(" ");
             }
 
-            String msg = sa.getParam("SacMessage");
+            String msg = sa.getParamOrDefault("SacMessage", valid);
-            if (msg == null) {
-                msg = valid;
-            }
 
             if (sa.hasParam("Destroy")) {
                 sb.append("Destroys ");

forge-gui/res/cardsfolder/w/wanderwine_prophets.txt

@@ -4,10 +4,9 @@ Types:Creature Merfolk Wizard
 PT:4/4
 K:Champion:Merfolk
 T:Mode$ DamageDone | ValidSource$ Card.Self | ValidTarget$ Player | CombatDamage$ True | Execute$ TrigSacrifice | OptionalDecider$ You | TriggerZones$ Battlefield | TriggerDescription$ Whenever CARDNAME deals combat damage to a player, you may sacrifice a Merfolk. If you do, take an extra turn after this one.
-SVar:TrigSacrifice:DB$ Sacrifice | SacValid$ Merfolk | RememberSacrificedSVar$ NumSacrificed | SubAbility$ DBAddTurn
+SVar:TrigSacrifice:DB$ Sacrifice | SacValid$ Merfolk | RememberSacrificed$ True | SubAbility$ DBAddTurn
-SVar:DBAddTurn:DB$ AddTurn | NumTurns$ 1 | ConditionCheckSVar$ NumSacrificed | ConditionSVarCompare$ EQ1 | SubAbility$ DBCleanup
+SVar:DBAddTurn:DB$ AddTurn | NumTurns$ 1 | ConditionDefined$ Remembered | ConditionPresent$ Card | SubAbility$ DBCleanup
-SVar:DBCleanup:DB$ StoreSVar | SVar$ NumSacrificed | Type$ Number | Expression$ 0
+SVar:DBCleanup:DB$ Cleanup | ClearRemembered$ True
-SVar:NumSacrificed:0
 AI:RemoveDeck:Random
 SVar:Picture:http://www.wizards.com/global/images/magic/general/wanderwine_prophets.jpg
 Oracle:Champion a Merfolk (When this enters the battlefield, sacrifice it unless you exile another Merfolk you control. When this leaves the battlefield, that card returns to the battlefield.)\nWhenever Wanderwine Prophets deals combat damage to a player, you may sacrifice a Merfolk. If you do, take an extra turn after this one.