diff --git a/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java b/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java index c702e65dd78..72391453a7e 100644 --- a/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java +++ b/Mage.Server.Plugins/Mage.Player.Human/src/mage/player/human/HumanPlayer.java @@ -2110,7 +2110,7 @@ public class HumanPlayer extends PlayerImpl { // cancel choice (remove all selections) if (Modes.CHOOSE_OPTION_CANCEL_ID.equals(response.getUUID())) { - modes.getSelectedModes().clear(); + modes.clearSelectedModes(); } } else if (canEndChoice) { // end choice by done button in feedback panel diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/modal/OneOrMoreTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/modal/OneOrMoreTest.java index 172950663a9..064ff0fd8e1 100644 --- a/Mage.Tests/src/test/java/org/mage/test/cards/modal/OneOrMoreTest.java +++ b/Mage.Tests/src/test/java/org/mage/test/cards/modal/OneOrMoreTest.java @@ -14,7 +14,6 @@ import org.junit.Test; import org.mage.test.serverside.base.CardTestPlayerBase; /** - * * @author LevelX2 */ public class OneOrMoreTest extends CardTestPlayerBase { @@ -26,7 +25,7 @@ public class OneOrMoreTest extends CardTestPlayerBase { * the games can use last known info of the legal target. */ @Test - public void testSubtleStrikeFirstMode() { + public void test_ChooseModes_AsCardOrder() { addCard(Zone.BATTLEFIELD, playerA, "Island", 6); // Choose one or more — // 1 • Counter target spell @@ -38,24 +37,64 @@ public class OneOrMoreTest extends CardTestPlayerBase { addCard(Zone.BATTLEFIELD, playerA, "Silvercoat Lion"); - castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Sublime Epiphany", "Silvercoat Lion"); - setModeChoice(playerA, "3"); + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Sublime Epiphany"); + setModeChoice(playerA, "3"); setModeChoice(playerA, "4"); - addTarget(playerA, "Silvercoat Lion"); setModeChoice(playerA, "5"); - addTarget(playerA, playerB); + addTarget(playerA, "Silvercoat Lion"); // for 3 + addTarget(playerA, "Silvercoat Lion"); // for 4 + addTarget(playerA, playerB); // for 5 setModeChoice(playerA, null); + setStrictChooseMode(true); setStopAt(1, PhaseStep.BEGIN_COMBAT); execute(); + assertAllCommandsUsed(); assertHandCount(playerB, 1); assertHandCount(playerA, "Silvercoat Lion", 1); - + assertPowerToughness(playerA, "Silvercoat Lion", 2, 2); - + + Permanent perm = getPermanent("Silvercoat Lion"); + Assert.assertTrue("Silvercoat Lion has to be a Token", perm instanceof PermanentToken); + } + + @Test + public void test_ChooseModes_AsCustomOrder() { + // user can select modes in any order, but resolves/targets must be standard (in same order as card's text) + + addCard(Zone.BATTLEFIELD, playerA, "Island", 6); + // Choose one or more — + // 1 • Counter target spell + // 2 • Counter target activated or triggered ability. + // 3 • Return target nonland permanent to its owner's hand. + // 4 • Create a token that's a copy of target creature you control. + // 5 • Target player draws a card. + addCard(Zone.HAND, playerA, "Sublime Epiphany"); // Instant {4}{U}{U} + + addCard(Zone.BATTLEFIELD, playerA, "Silvercoat Lion"); + + castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Sublime Epiphany"); + setModeChoice(playerA, "4"); + setModeChoice(playerA, "5"); + setModeChoice(playerA, "3"); + addTarget(playerA, "Silvercoat Lion"); // for 3 + addTarget(playerA, "Silvercoat Lion"); // for 4 + addTarget(playerA, playerB); // for 5 + setModeChoice(playerA, null); + + setStrictChooseMode(true); + setStopAt(1, PhaseStep.BEGIN_COMBAT); + execute(); + assertAllCommandsUsed(); + + assertHandCount(playerB, 1); + assertHandCount(playerA, "Silvercoat Lion", 1); + + assertPowerToughness(playerA, "Silvercoat Lion", 2, 2); + Permanent perm = getPermanent("Silvercoat Lion"); Assert.assertTrue("Silvercoat Lion has to be a Token", perm instanceof PermanentToken); - } } diff --git a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java index 3a8776ab570..6658c6923e9 100644 --- a/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java +++ b/Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java @@ -1780,10 +1780,10 @@ public class TestPlayer implements Player { modesSet.remove(0); return null; } - int selectedMode = Integer.parseInt(modesSet.get(0)); + int needMode = Integer.parseInt(modesSet.get(0)); int i = 1; for (Mode mode : modes.getAvailableModes(source, game)) { - if (i == selectedMode) { + if (i == needMode) { modesSet.remove(0); return mode; } diff --git a/Mage/src/main/java/mage/abilities/AbilityImpl.java b/Mage/src/main/java/mage/abilities/AbilityImpl.java index 774bf1a25ac..e5d754223c1 100644 --- a/Mage/src/main/java/mage/abilities/AbilityImpl.java +++ b/Mage/src/main/java/mage/abilities/AbilityImpl.java @@ -709,7 +709,6 @@ public abstract class AbilityImpl implements Ability { @Override public void addWatcher(Watcher watcher) { - watcher.setSourceId(this.sourceId); watcher.setControllerId(this.controllerId); getWatchers().add(watcher); diff --git a/Mage/src/main/java/mage/abilities/Modes.java b/Mage/src/main/java/mage/abilities/Modes.java index f05cb990d96..7099f3b8af5 100644 --- a/Mage/src/main/java/mage/abilities/Modes.java +++ b/Mage/src/main/java/mage/abilities/Modes.java @@ -23,9 +23,9 @@ public class Modes extends LinkedHashMap { public static final UUID CHOOSE_OPTION_CANCEL_ID = UUID.fromString("0125bd0c-5610-4eba-bc80-fc6d0a7b9de6"); private Mode currentMode; // the current mode of the selected modes - private final List selectedModes = new ArrayList<>(); // all selected modes (this + duplicate) - private final Map duplicateModes = new LinkedHashMap<>(); // for 2x selects: copy mode and put it to duplicate list - private final Map duplicateToOriginalModeRefs = new LinkedHashMap<>(); // for 2x selects: stores ref from duplicate to original mode + private final List selectedModes = new ArrayList<>(); // all selected modes (this + duplicate), for all code user getSelectedModes to keep modes order + private final Map selectedDuplicateModes = new LinkedHashMap<>(); // for 2x selects: copy mode and put it to duplicate list + private final Map selectedDuplicateToOriginalModeRefs = new LinkedHashMap<>(); // for 2x selects: stores ref from duplicate to original mode private int minModes; private int maxModes; @@ -52,10 +52,10 @@ public class Modes extends LinkedHashMap { for (Map.Entry entry : modes.entrySet()) { this.put(entry.getKey(), entry.getValue().copy()); } - for (Map.Entry entry : modes.duplicateModes.entrySet()) { - duplicateModes.put(entry.getKey(), entry.getValue().copy()); + for (Map.Entry entry : modes.selectedDuplicateModes.entrySet()) { + selectedDuplicateModes.put(entry.getKey(), entry.getValue().copy()); } - duplicateToOriginalModeRefs.putAll(modes.duplicateToOriginalModeRefs); + selectedDuplicateToOriginalModeRefs.putAll(modes.selectedDuplicateToOriginalModeRefs); this.minModes = modes.minModes; this.maxModes = modes.maxModes; @@ -72,7 +72,7 @@ public class Modes extends LinkedHashMap { if (modes.getSelectedModes().isEmpty()) { this.currentMode = values().iterator().next(); } else { - this.currentMode = get(modes.getMode().getId()); + this.currentMode = get(modes.getMode().getId()); // need fix? } } @@ -84,7 +84,7 @@ public class Modes extends LinkedHashMap { public Mode get(Object key) { Mode modeToGet = super.get(key); if (modeToGet == null && eachModeMoreThanOnce) { - modeToGet = duplicateModes.get(key); + modeToGet = selectedDuplicateModes.get(key); } return modeToGet; } @@ -103,7 +103,7 @@ public class Modes extends LinkedHashMap { public UUID getModeId(int index) { int idx = 0; if (eachModeMoreThanOnce) { - for (UUID modeId : this.selectedModes) { + for (UUID modeId : this.getSelectedModes()) { idx++; if (idx == index) { return modeId; @@ -121,7 +121,25 @@ public class Modes extends LinkedHashMap { } public List getSelectedModes() { - return selectedModes; + // sorted as original modes + List res = new ArrayList<>(); + for (Mode mode : this.values()) { + for (UUID selectedId : this.selectedModes) { + // selectedModes contains original mode and 2+ selected as duplicates (new modes) + UUID selectedOriginalId = this.selectedDuplicateToOriginalModeRefs.get(selectedId); + if (Objects.equals(mode.getId(), selectedId) + || Objects.equals(mode.getId(), selectedOriginalId)) { + res.add(selectedId); + } + } + } + return res; + } + + public void clearSelectedModes() { + this.selectedModes.clear(); + this.selectedDuplicateModes.clear(); + this.selectedDuplicateToOriginalModeRefs.clear(); } public int getSelectedStats(UUID modeId) { @@ -133,14 +151,14 @@ public class Modes extends LinkedHashMap { // multiple select (all 2x select generate new duplicate mode) UUID originalId; - if (this.duplicateModes.containsKey(modeId)) { + if (this.selectedDuplicateModes.containsKey(modeId)) { // modeId is duplicate - originalId = this.duplicateToOriginalModeRefs.get(modeId); + originalId = this.selectedDuplicateToOriginalModeRefs.get(modeId); } else { // modeId is original originalId = modeId; } - for (UUID id : this.duplicateToOriginalModeRefs.values()) { + for (UUID id : this.selectedDuplicateToOriginalModeRefs.values()) { if (id.equals(originalId)) { count++; } @@ -183,9 +201,7 @@ public class Modes extends LinkedHashMap { } public void setActiveMode(Mode mode) { - if (selectedModes.contains(mode.getId())) { - this.currentMode = mode; - } + setActiveMode(mode.getId()); } public void setActiveMode(UUID modeId) { @@ -200,9 +216,7 @@ public class Modes extends LinkedHashMap { public boolean choose(Game game, Ability source) { if (this.size() > 1) { - this.selectedModes.clear(); - this.duplicateModes.clear(); - this.duplicateToOriginalModeRefs.clear(); + this.clearSelectedModes(); if (this.isRandom) { List modes = getAvailableModes(source, game); this.addSelectedMode(modes.get(RandomUtil.nextInt(modes.size())).getId()); @@ -230,7 +244,7 @@ public class Modes extends LinkedHashMap { } } if (isEachModeOnlyOnce()) { - setAlreadySelectedModes(selectedModes, source, game); + setAlreadySelectedModes(source, game); } return !selectedModes.isEmpty(); } @@ -274,7 +288,7 @@ public class Modes extends LinkedHashMap { Mode choice = player.chooseMode(this, source, game); if (choice == null) { if (isEachModeOnlyOnce()) { - setAlreadySelectedModes(selectedModes, source, game); + setAlreadySelectedModes(source, game); } return this.selectedModes.size() >= this.getMinModes(); } @@ -284,12 +298,13 @@ public class Modes extends LinkedHashMap { } } if (isEachModeOnlyOnce()) { - setAlreadySelectedModes(selectedModes, source, game); + setAlreadySelectedModes(source, game); } return true; - } else { // only one mode + } else { + // only one mode available if (currentMode == null) { - this.selectedModes.clear(); + this.clearSelectedModes(); Mode mode = this.values().iterator().next(); this.addSelectedMode(mode.getId()); this.setActiveMode(mode); @@ -301,12 +316,11 @@ public class Modes extends LinkedHashMap { /** * Saves the already selected modes to the state value * - * @param selectedModes * @param source * @param game */ - private void setAlreadySelectedModes(List selectedModes, Ability source, Game game) { - for (UUID modeId : selectedModes) { + private void setAlreadySelectedModes(Ability source, Game game) { + for (UUID modeId : getSelectedModes()) { String key = getKey(source, game, modeId); game.getState().setValue(key, true); } @@ -318,19 +332,29 @@ public class Modes extends LinkedHashMap { * * @param modeId */ - private void addSelectedMode(UUID modeId) { + public void addSelectedMode(UUID modeId) { + if (!this.containsKey(modeId)) { + throw new IllegalArgumentException("Unknown modeId to select"); + } + if (selectedModes.contains(modeId) && eachModeMoreThanOnce) { Mode duplicateMode = get(modeId).copy(); UUID originalId = modeId; duplicateMode.setRandomId(); modeId = duplicateMode.getId(); - duplicateModes.put(modeId, duplicateMode); - duplicateToOriginalModeRefs.put(duplicateMode.getId(), originalId); + selectedDuplicateModes.put(modeId, duplicateMode); + selectedDuplicateToOriginalModeRefs.put(duplicateMode.getId(), originalId); } this.selectedModes.add(modeId); } + public void removeSelectedMode(UUID modeId) { + this.selectedModes.remove(modeId); + this.selectedDuplicateModes.remove(modeId); + this.selectedDuplicateToOriginalModeRefs.remove(modeId); + } + // The already once selected modes for a modal card are stored as a state value // That's important for modal abilities with modes that can only selected once while the object stays in its zone @SuppressWarnings("unchecked") diff --git a/Mage/src/main/java/mage/game/stack/Spell.java b/Mage/src/main/java/mage/game/stack/Spell.java index 4757c51e62b..c87120b51c1 100644 --- a/Mage/src/main/java/mage/game/stack/Spell.java +++ b/Mage/src/main/java/mage/game/stack/Spell.java @@ -32,12 +32,7 @@ import mage.players.Player; import mage.util.GameLog; import mage.util.SubTypeList; -import java.util.ArrayList; -import java.util.Iterator; -import java.util.List; -import java.util.Map.Entry; -import java.util.Set; -import java.util.UUID; +import java.util.*; /** * @author BetaSteward_at_googlemail.com @@ -223,14 +218,12 @@ public class Spell extends StackObjImpl implements Card { for (SpellAbility spellAbility : this.spellAbilities) { // legality of targets is checked only as the spell begins to resolve, not in between modes (spliced spells handeled correctly?) if (spellAbilityCheckTargetsAndDeactivateModes(spellAbility, game)) { - for (Mode mode : spellAbility.getModes().values()) { - if (spellAbility.getModes().getSelectedModes().contains(mode.getId())) { - spellAbility.getModes().setActiveMode(mode.getId()); - if (spellAbility.getSpellAbilityType() != SpellAbilityType.SPLICE) { - updateOptionalCosts(index); - } - result |= spellAbility.resolve(game); + for (UUID modeId : spellAbility.getModes().getSelectedModes()) { + spellAbility.getModes().setActiveMode(modeId); + if (spellAbility.getSpellAbilityType() != SpellAbilityType.SPLICE) { + updateOptionalCosts(index); } + result |= spellAbility.resolve(game); } index++; } @@ -333,11 +326,12 @@ public class Spell extends StackObjImpl implements Card { */ private boolean spellAbilityCheckTargetsAndDeactivateModes(SpellAbility spellAbility, Game game) { boolean legalModes = false; - for (Iterator iterator = spellAbility.getModes().getSelectedModes().iterator(); iterator.hasNext();) { + for (Iterator iterator = spellAbility.getModes().getSelectedModes().iterator(); iterator.hasNext(); ) { UUID nextSelectedModeId = iterator.next(); Mode mode = spellAbility.getModes().get(nextSelectedModeId); if (!mode.getTargets().isEmpty()) { if (!mode.getTargets().stillLegal(spellAbility, game)) { + spellAbility.getModes().removeSelectedMode(mode.getId()); iterator.remove(); continue; } diff --git a/Mage/src/main/java/mage/players/PlayerImpl.java b/Mage/src/main/java/mage/players/PlayerImpl.java index cd0650225b6..e04b3e408fb 100644 --- a/Mage/src/main/java/mage/players/PlayerImpl.java +++ b/Mage/src/main/java/mage/players/PlayerImpl.java @@ -3537,8 +3537,8 @@ public abstract class PlayerImpl implements Player, Serializable { // TODO: Support modal spells with more than one selectable mode for (Mode mode : option.getModes().values()) { Ability newOption = option.copy(); - newOption.getModes().getSelectedModes().clear(); - newOption.getModes().getSelectedModes().add(mode.getId()); + newOption.getModes().clearSelectedModes(); + newOption.getModes().addSelectedMode(mode.getId()); newOption.getModes().setActiveMode(mode); if (!newOption.getTargets().getUnchosen().isEmpty()) { if (!newOption.getManaCosts().getVariableCosts().isEmpty()) { diff --git a/Mage/src/main/java/mage/util/TargetAddress.java b/Mage/src/main/java/mage/util/TargetAddress.java index 703d0eaf3ef..5b4518fe40c 100644 --- a/Mage/src/main/java/mage/util/TargetAddress.java +++ b/Mage/src/main/java/mage/util/TargetAddress.java @@ -1,9 +1,5 @@ - package mage.util; -import java.util.Iterator; -import java.util.Objects; -import java.util.UUID; import mage.abilities.Mode; import mage.abilities.Modes; import mage.abilities.SpellAbility; @@ -11,6 +7,10 @@ import mage.cards.Card; import mage.game.stack.Spell; import mage.target.Target; +import java.util.Iterator; +import java.util.Objects; +import java.util.UUID; + /** * @author duncant */ @@ -44,7 +44,7 @@ public class TargetAddress { protected final Iterator spellAbilityIterator; protected Integer lastSpellAbilityIndex = null; - protected Iterator modeIterator = null; + protected Iterator modeIterator = null; // must be read only protected Modes modes = null; protected UUID lastMode = null; protected Iterator targetIterator = null;