Additional fixes for #6771 (modal spells resolve order)

This commit is contained in:
Oleg Agafonov 2020-07-03 01:46:05 +04:00
parent 6dccaee9a4
commit da4a44445b
8 changed files with 120 additions and 64 deletions

View file

@ -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

View file

@ -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);
}
}

View file

@ -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;
}

View file

@ -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);

View file

@ -23,9 +23,9 @@ public class Modes extends LinkedHashMap<UUID, Mode> {
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<UUID> selectedModes = new ArrayList<>(); // all selected modes (this + duplicate)
private final Map<UUID, Mode> duplicateModes = new LinkedHashMap<>(); // for 2x selects: copy mode and put it to duplicate list
private final Map<UUID, UUID> duplicateToOriginalModeRefs = new LinkedHashMap<>(); // for 2x selects: stores ref from duplicate to original mode
private final List<UUID> selectedModes = new ArrayList<>(); // all selected modes (this + duplicate), for all code user getSelectedModes to keep modes order
private final Map<UUID, Mode> selectedDuplicateModes = new LinkedHashMap<>(); // for 2x selects: copy mode and put it to duplicate list
private final Map<UUID, UUID> 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<UUID, Mode> {
for (Map.Entry<UUID, Mode> entry : modes.entrySet()) {
this.put(entry.getKey(), entry.getValue().copy());
}
for (Map.Entry<UUID, Mode> entry : modes.duplicateModes.entrySet()) {
duplicateModes.put(entry.getKey(), entry.getValue().copy());
for (Map.Entry<UUID, Mode> 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<UUID, Mode> {
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<UUID, Mode> {
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<UUID, Mode> {
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<UUID, Mode> {
}
public List<UUID> getSelectedModes() {
return selectedModes;
// sorted as original modes
List<UUID> 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<UUID, Mode> {
// 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<UUID, Mode> {
}
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<UUID, Mode> {
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<Mode> modes = getAvailableModes(source, game);
this.addSelectedMode(modes.get(RandomUtil.nextInt(modes.size())).getId());
@ -230,7 +244,7 @@ public class Modes extends LinkedHashMap<UUID, Mode> {
}
}
if (isEachModeOnlyOnce()) {
setAlreadySelectedModes(selectedModes, source, game);
setAlreadySelectedModes(source, game);
}
return !selectedModes.isEmpty();
}
@ -274,7 +288,7 @@ public class Modes extends LinkedHashMap<UUID, Mode> {
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<UUID, Mode> {
}
}
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<UUID, Mode> {
/**
* Saves the already selected modes to the state value
*
* @param selectedModes
* @param source
* @param game
*/
private void setAlreadySelectedModes(List<UUID> 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<UUID, Mode> {
*
* @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")

View file

@ -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<UUID> iterator = spellAbility.getModes().getSelectedModes().iterator(); iterator.hasNext();) {
for (Iterator<UUID> 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;
}

View file

@ -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()) {

View file

@ -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<SpellAbility> spellAbilityIterator;
protected Integer lastSpellAbilityIndex = null;
protected Iterator<UUID> modeIterator = null;
protected Iterator<UUID> modeIterator = null; // must be read only
protected Modes modes = null;
protected UUID lastMode = null;
protected Iterator<Target> targetIterator = null;