Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve trigger auto ordering and aura attachment (Lynde QOL fixes) #10648

Merged
merged 7 commits into from
Nov 2, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import mage.abilities.costs.common.TapSourceCost;
import mage.abilities.costs.mana.ManaCost;
import mage.abilities.costs.mana.ManaCostsImpl;
import mage.abilities.effects.Effects;
import mage.abilities.effects.RequirementEffect;
import mage.abilities.hint.HintUtils;
import mage.abilities.mana.ActivatedManaAbilityImpl;
Expand Down Expand Up @@ -44,6 +45,7 @@
import mage.target.common.TargetAnyTarget;
import mage.target.common.TargetAttackingCreature;
import mage.target.common.TargetDefender;
import mage.target.targetpointer.TargetPointer;
import mage.util.*;
import org.apache.log4j.Logger;

Expand Down Expand Up @@ -1341,12 +1343,14 @@ public TriggeredAbility chooseTriggeredAbility(java.util.List<TriggeredAbility>
return null;
}

// automatically order triggers with same ability, rules text, and targets
String autoOrderRuleText = null;
xenohedron marked this conversation as resolved.
Show resolved Hide resolved
Ability autoOrderAbility = null;
boolean autoOrderUse = getControllingPlayersUserData(game).isAutoOrderTrigger();
while (canRespond()) {
// try to set trigger auto order
java.util.List<TriggeredAbility> abilitiesWithNoOrderSet = new ArrayList<>();
java.util.List<TriggeredAbility> abilitiesOrderLast = new ArrayList<>();
List<TriggeredAbility> abilitiesWithNoOrderSet = new ArrayList<>();
List<TriggeredAbility> abilitiesOrderLast = new ArrayList<>();
for (TriggeredAbility ability : abilities) {
if (triggerAutoOrderAbilityFirst.contains(ability.getOriginalId())) {
return ability;
Expand All @@ -1366,12 +1370,33 @@ public TriggeredAbility chooseTriggeredAbility(java.util.List<TriggeredAbility>
continue;
}
if (autoOrderUse) {
// multiple triggers with same rule text will be auto-ordered
// multiple triggers with same rule text will be auto-ordered if possible
// if different, must use choose dialog
if (autoOrderRuleText == null) {
// first trigger, store rule text and ability to compare subsequent triggers
autoOrderRuleText = rule;
xenohedron marked this conversation as resolved.
Show resolved Hide resolved
autoOrderAbility = ability;
} else if (!rule.equals(autoOrderRuleText)) {
// diff triggers, so must use choose dialog
// disable auto order if rule text is different
autoOrderUse = false;
xenohedron marked this conversation as resolved.
Show resolved Hide resolved
} else {
// disable auto order if targets are different
Effects effects1 = autoOrderAbility.getEffects();
xenohedron marked this conversation as resolved.
Show resolved Hide resolved
Effects effects2 = ability.getEffects();
if (effects1.size() != effects2.size()) {
autoOrderUse = false;
} else {
for (int i = 0; i < effects1.size(); i++) {
TargetPointer targetPointer1 = effects1.get(i).getTargetPointer();
TargetPointer targetPointer2 = effects2.get(i).getTargetPointer();
List<UUID> targets1 = (targetPointer1 == null) ? new ArrayList<>() : targetPointer1.getTargets(game, autoOrderAbility);
List<UUID> targets2 = (targetPointer2 == null) ? new ArrayList<>() : targetPointer2.getTargets(game, ability);
if (!targets1.equals(targets2)) {
autoOrderUse = false;
break;
}
}
}
}
}
abilitiesWithNoOrderSet.add(ability);
Expand All @@ -1382,8 +1407,7 @@ public TriggeredAbility chooseTriggeredAbility(java.util.List<TriggeredAbility>
return abilitiesOrderLast.stream().findFirst().orElse(null);
}

if (abilitiesWithNoOrderSet.size() == 1
|| autoOrderUse) {
if (abilitiesWithNoOrderSet.size() == 1 || autoOrderUse) {
return abilitiesWithNoOrderSet.iterator().next();
}

Expand Down
5 changes: 4 additions & 1 deletion Mage.Sets/src/mage/cards/l/LyndeCheerfulTormentor.java
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import mage.players.Player;
import mage.target.TargetPermanent;
import mage.target.common.TargetOpponent;
import mage.target.targetpointer.FixedTarget;

/**
*
Expand Down Expand Up @@ -91,7 +92,9 @@ public boolean checkTrigger(GameEvent event, Game game) {
if (zEvent.isDiesEvent()) {
Permanent permanent = zEvent.getTarget();
if (permanent != null && permanent.hasSubtype(SubType.CURSE, game) && permanent.isOwnedBy(controllerId)) {
getEffects().setValue("curse", new MageObjectReference(game.getCard(zEvent.getTargetId()), game));
MageObjectReference curse = new MageObjectReference(game.getCard(zEvent.getTargetId()), game);
getEffects().setValue("curse", curse);
getEffects().setTargetPointer(new FixedTarget(curse));
return true;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,78 @@ public void onlyBringsBackCursesFromGraveyard() {
assertPermanentCount(playerA, curse, 0);
assertExileCount(playerA, curse, 1);
}

/**
* When Lynde brings back a card which is a copy of a curse and enters again as a copy of a curse, the game mistakenly asks who to attach the curse to,
* when it should automatically be attached to the controller of Lynde.
*/
@Test
public void copyCardTarget() {
// {1}{R} - Curse. On upkeep, deal 1 damage to enchanted player
String curse = "Curse of the Pierced Heart";
// {2}{U}{U} - Enters as a copy of a curse
String copy = "Clever Impersonator";
// {1} - Sacrifice a permanent
String sac = "Claws of Gix";
String island = "Island";
String mountain = "Mountain";

// The necessary cards for the test
addCard(Zone.HAND, playerA, curse);
addCard(Zone.HAND, playerB, copy);
addCard(Zone.BATTLEFIELD, playerB, lynde);
addCard(Zone.BATTLEFIELD, playerB, sac);

// Mana needed for player A to cast curse and player B to cast copy and sac
addCard(Zone.BATTLEFIELD, playerA, mountain, 2);
addCard(Zone.BATTLEFIELD, playerB, island, 5);

// Player A plays the curse
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, curse, playerB);
waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN, 1);

// Do not use Lynde's ability on upkeep
setChoice(playerB, false);

// Player B took one damage from Player A's curse
checkLife("Turn 2 Upkeep", 2, PhaseStep.PRECOMBAT_MAIN, playerB, 20 - 1);

// Player B casts the copy spell, choosing to copy the curse and enchant player A
castSpell(2, PhaseStep.PRECOMBAT_MAIN, playerB, copy);
setChoice(playerB, true);
setChoice(playerB, curse);
setChoice(playerB, playerA.getName());
waitStackResolved(2, PhaseStep.PRECOMBAT_MAIN, 1);

// Now there should be two curses
checkPermanentCount("After copy", 2, PhaseStep.PRECOMBAT_MAIN, playerA, curse, 1);
checkPermanentCount("After copy", 2, PhaseStep.PRECOMBAT_MAIN, playerB, curse, 1);

// Player B sacrifices their copy of the curse
activateAbility(2, PhaseStep.PRECOMBAT_MAIN, playerB, "{1}, Sacrifice a permanent");
setChoice(playerB, curse);

// Make sure Lynde triggers
checkStackObject("After sac", 2, PhaseStep.PRECOMBAT_MAIN, playerB,
"Whenever a Curse is put into your graveyard from the battlefield", 1);
waitStackResolved(2, PhaseStep.PRECOMBAT_MAIN, 2);

// Have copy come back as copy of curse. It should be attached to playerB automatically, and no choice should be offered on who to attach it to.
setChoice(playerB, true);
setChoice(playerB, curse);

// At the beginning of turn 4, player B should have two triggers
setChoice(playerB, "At the beginning of enchanted");
setChoice(playerB, false);

setStopAt(4, PhaseStep.END_TURN);
setStrictChooseMode(true);
execute();

assertPermanentCount(playerA, curse, 1);
assertPermanentCount(playerB, curse, 1);

assertLife(playerA, 20);
assertLife(playerB, 20 - 2);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,15 @@ public boolean apply(Game game, Ability source) {

// select new target
auraTarget.withNotTarget(true);
if (!controller.choose(auraOutcome, auraTarget, source, game)) {
return true;

UUID targetId = (UUID) game.getState().getValue("attachTo:" + sourcePermanent.getId());
if (targetId == null) {
if (!controller.choose(auraOutcome, auraTarget, source, game)) {
return true;
}
targetId = auraTarget.getFirstTarget();
}
UUID targetId = auraTarget.getFirstTarget();

Permanent targetPermanent = game.getPermanent(targetId);
Player targetPlayer = game.getPlayer(targetId);
if (targetPermanent != null) {
Expand Down