Skip to content

Commit 7344965

Browse files
committed
Extend original ID concept to apply to effects as well as abilities
The root cause of the issue here is that when a permanent with a replacement effect is copied, this is applied via a continuous `CopyEffect`. Normally this is not a problem, but for attack taxers, part of choosing whether the replacement effect applies or not is tapping mana sources, which triggers `applyEffects()` to run. The `CopyEffect` is then reapplied, and as part of it, it clears the copied abilities and generates new ones with new IDs from the blueprint. Thus, the loop checking whether all replacement effects have been consumed never terminates, because it sees a new replacement effect with a new ID it needs to apply. This extends the concept of `originalId`, which abilities already have, to also apply to effects. It can be used in any scenarios which need an effect's ID to remain stable even when it is destroyed and recreated via a copy from a continuous effect. Fixes #10372 Fixes #14754
1 parent c1ac67b commit 7344965

4 files changed

Lines changed: 44 additions & 5 deletions

File tree

Mage.Tests/src/test/java/org/mage/test/cards/replacement/PropagandaTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,4 +57,33 @@ public void test_Propaganda2() {
5757
assertTappedCount("Plains", true, 4);
5858
}
5959

60+
// https://github.com/magefree/mage/issues/10372
61+
// https://github.com/magefree/mage/issues/14754
62+
@Test
63+
public void testClone() {
64+
addCard(Zone.BATTLEFIELD, playerB, "Archangel of Tithes");
65+
addCard(Zone.HAND, playerA, "Clone");
66+
addCard(Zone.BATTLEFIELD, playerA, "Island", 4);
67+
addCard(Zone.BATTLEFIELD, playerB, "Balduvian Bears");
68+
addCard(Zone.BATTLEFIELD, playerB, "Wastes");
69+
addCard(Zone.HAND, playerA, "Fell");
70+
addCard(Zone.BATTLEFIELD, playerA, "Swamp", 2);
71+
72+
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Clone");
73+
setChoice(playerA, true);
74+
setChoice(playerA, "Archangel of Tithes");
75+
castSpell(1, PhaseStep.POSTCOMBAT_MAIN, playerA, "Fell");
76+
addTarget(playerA, "Archangel of Tithes[no copy]");
77+
78+
attack(2, playerB, "Balduvian Bears");
79+
setChoice(playerB, true);
80+
81+
setStrictChooseMode(true);
82+
setStopAt(2, PhaseStep.POSTCOMBAT_MAIN);
83+
execute();
84+
85+
assertLife(playerA, 18);
86+
assertTappedCount("Balduvian Bears", true, 1);
87+
}
88+
6089
}

Mage/src/main/java/mage/abilities/effects/ContinuousEffects.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -855,8 +855,8 @@ public boolean replaceEvent(GameEvent event, Game game) {
855855
// Remove all consumed effects (ability dependant)
856856
for (Iterator<ReplacementEffect> it1 = rEffects.keySet().iterator(); it1.hasNext(); ) {
857857
ReplacementEffect entry = it1.next();
858-
if (consumed.containsKey(entry.getId()) /*&& !(entry instanceof CommanderReplacementEffect) */) { // 903.9.
859-
Set<UUID> consumedAbilitiesIds = consumed.get(entry.getId());
858+
if (consumed.containsKey(entry.getOriginalId()) /*&& !(entry instanceof CommanderReplacementEffect) */) { // 903.9.
859+
Set<UUID> consumedAbilitiesIds = consumed.get(entry.getOriginalId());
860860
if (rEffects.get(entry) == null || consumedAbilitiesIds.size() == rEffects.get(entry).size()) {
861861
it1.remove();
862862
} else {
@@ -944,8 +944,8 @@ public boolean replaceEvent(GameEvent event, Game game) {
944944

945945
// add the applied effect to the consumed effects
946946
if (rEffect != null) {
947-
if (consumed.containsKey(rEffect.getId())) {
948-
Set<UUID> set = consumed.get(rEffect.getId());
947+
if (consumed.containsKey(rEffect.getOriginalId())) {
948+
Set<UUID> set = consumed.get(rEffect.getOriginalId());
949949
if (rAbility != null) {
950950
set.add(rAbility.getId());
951951

@@ -955,7 +955,7 @@ public boolean replaceEvent(GameEvent event, Game game) {
955955
if (rAbility != null) { // in case of AuraReplacementEffect or PlaneswalkerReplacementEffect there is no Ability
956956
set.add(rAbility.getId());
957957
}
958-
consumed.put(rEffect.getId(), set);
958+
consumed.put(rEffect.getOriginalId(), set);
959959
}
960960
}
961961
// Must be called here for some effects to be able to work correctly

Mage/src/main/java/mage/abilities/effects/Effect.java

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@ public interface Effect extends Serializable, Copyable<Effect> {
1919

2020
UUID getId();
2121

22+
UUID getOriginalId();
23+
2224
void newId();
2325

2426
/**

Mage/src/main/java/mage/abilities/effects/EffectImpl.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
public abstract class EffectImpl implements Effect {
1818

1919
protected UUID id;
20+
protected UUID originalId;
2021
protected Outcome outcome;
2122
protected EffectType effectType;
2223

@@ -30,13 +31,15 @@ public abstract class EffectImpl implements Effect {
3031

3132
public EffectImpl(Outcome outcome) {
3233
this.id = UUID.randomUUID();
34+
this.originalId = this.id;
3335
this.outcome = outcome;
3436

3537
initNewTargetPointer();
3638
}
3739

3840
protected EffectImpl(final EffectImpl effect) {
3941
this.id = effect.id;
42+
this.originalId = effect.id;
4043
this.outcome = effect.outcome;
4144
this.staticText = effect.staticText;
4245
this.effectType = effect.effectType;
@@ -61,6 +64,11 @@ public UUID getId() {
6164
return id;
6265
}
6366

67+
@Override
68+
public UUID getOriginalId() {
69+
return originalId;
70+
}
71+
6472
@Override
6573
public String getText(Mode mode) {
6674
return staticText;

0 commit comments

Comments
 (0)