Collectible Card Game Headquarters

Cards like Vesuva have an optional enter the battlefield effect.
Right now this is done in two stages: first select the option and then
select the card for the effect. If instead the effect is optional then this
can be done in one step.

However, the clone effect (CloneEffect) does not have an optional parameter.
Adding an optional parameter is easy and supports the improvement to UI
mechanics for cards like Vesuva.

A final change improves the mechanics of the optional choice. Right now,
optionally choosing a single entity for effect (chooseSingleEntityForEffect)
when the selection is a card from the player's hand or the battlefield turns
into optionally choosing zero or one cards (InputSelectEntitiesFromList with
min 0 and max 1 and then allowing cancelling). I think that this is not
ideal and there should instead be a minimum of 1. The UI difference is that
in the current situation OK can be used to "cancel" the selection. With the
change, the cancel button is needed to cancel the selection.

The changes for the above are minimal (and I already have done them). What
I haven't done is add the new parameter for the documentation on the clone
effect.

Here is the diff of the changes that I made (hopefully not too mangled).

git diff
diff --git a/forge-game/src/main/java/forge/game/ability/effects/CloneEffect.java b/forge-game/src/main/java/forge/game/ability/effects/CloneEffect.java
index 39c1a9637d..1bf207b010 100644
--- a/forge-game/src/main/java/forge/game/ability/effects/CloneEffect.java
+++ b/forge-game/src/main/java/forge/game/ability/effects/CloneEffect.java
@@ -62,6 +62,7 @@ public class CloneEffect extends SpellAbilityEffect {

// find cloning source i.e. thing to be copied
Card cardToCopy = null;
+ boolean optional = sa.hasParam("Optional");

if (sa.hasParam("Choices")) {
ZoneType choiceZone = ZoneType.Battlefield;
@@ -72,7 +73,8 @@ public class CloneEffect extends SpellAbilityEffect {
choices = CardLists.getValidCards(choices, sa.getParam("Choices"), activator, host);

String title = sa.hasParam("ChoiceTitle") ? sa.getParam("ChoiceTitle") : "Choose a card ";
- cardToCopy = activator.getController().chooseSingleEntityForEffect(choices, sa, title, false);
+ cardToCopy = activator.getController().chooseSingleEntityForEffect(choices, sa, title, optional);
+ optional = false ;
} else if (sa.hasParam("Defined")) {
List<Card> cloneSources = AbilityUtils.getDefinedCards(host, sa.getParam("Defined"), sa);
if (!cloneSources.isEmpty()) {
@@ -85,7 +87,6 @@ public class CloneEffect extends SpellAbilityEffect {
return;
}

- final boolean optional = sa.hasParam("Optional");
if (optional && !host.getController().getController().confirmAction(sa, null, "Do you want to copy " + cardToCopy + "?")) {
return;
}
diff --git a/forge-gui/res/cardsfolder/v/vesuva.txt b/forge-gui/res/cardsfolder/v/vesuva.txt
index 609ef29457..145da9be48 100644
--- a/forge-gui/res/cardsfolder/v/vesuva.txt
+++ b/forge-gui/res/cardsfolder/v/vesuva.txt
@@ -1,8 +1,8 @@
Name:Vesuva
ManaCost:no cost
Types:Land
-K:ETBReplacement:Copy:DBCopy:Optional
-SVar:DBCopy:DB$ Clone | Choices$ Land.Other | IntoPlayTapped$ True | SpellDescription$ You may have CARDNAME enter the battlefield tapped as a copy of any land on the battlefield.
+K:ETBReplacement:Copy:DBCopy
+SVar:DBCopy:DB$ Clone | Choices$ Land.Other | Optional$ True | IntoPlayTapped$ True | SpellDescription$ You may have CARDNAME enter the battlefield tapped as a copy of any land on the battlefield.
SVar:NeedsToPlay:Land.YouDontCtrl+notnamedVesuva,Land.YouCtrl+nonLegendary+notnamedVesuva
SVar:Picture:http://www.wizards.com/global/images/magic/general/vesuva.jpg
Oracle:You may have Vesuva enter the battlefield tapped as a copy of any land on the battlefield.
diff --git a/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java b/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java
index b26a3954f6..f0a3b3d7bc 100644
--- a/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java
+++ b/forge-gui/src/main/java/forge/player/PlayerControllerHuman.java
@@ -431,7 +431,7 @@ public class PlayerControllerHuman extends PlayerController implements IGameCont
reveal(delayedReveal.getCards(), delayedReveal.getZone(), delayedReveal.getOwner(),
delayedReveal.getMessagePrefix());
}
- final InputSelectEntitiesFromList<T> input = new InputSelectEntitiesFromList<T>(this, isOptional ? 0 : 1, 1,
+ final InputSelectEntitiesFromList<T> input = new InputSelectEntitiesFromList<T>(this, 1, 1,
optionList, sa);
input.setCancelAllowed(isOptional);
input.setMessage(MessageUtil.formatMessage(title, player, targetedPlayer));