From 2d7349a7bb6b9ec949acb3baf62ff806c38ac6a1 Mon Sep 17 00:00:00 2001 From: Susucre <34709007+Susucre@users.noreply.github.com> Date: Fri, 5 Apr 2024 11:52:15 +0200 Subject: [PATCH] Fix faulty logic in AddCounters effects amount computation. When set with DynamicValue, and that value computes to 0, the amount of counters added was incorrectly the Counters amount. --- Mage.Sets/src/mage/cards/g/GruffTriplets.java | 2 +- .../mage/cards/v/VojaJawsOfTheConclave.java | 3 +- .../test/cards/single/dmu/BriarHydraTest.java | 58 +++++++++++++++++++ .../common/counter/AddCountersAllEffect.java | 41 ++++++------- .../counter/AddCountersTargetEffect.java | 38 ++++++------ 5 files changed, 102 insertions(+), 40 deletions(-) create mode 100644 Mage.Tests/src/test/java/org/mage/test/cards/single/dmu/BriarHydraTest.java diff --git a/Mage.Sets/src/mage/cards/g/GruffTriplets.java b/Mage.Sets/src/mage/cards/g/GruffTriplets.java index 1a267762b26..b9501eb5827 100644 --- a/Mage.Sets/src/mage/cards/g/GruffTriplets.java +++ b/Mage.Sets/src/mage/cards/g/GruffTriplets.java @@ -55,7 +55,7 @@ public final class GruffTriplets extends CardImpl { // When Gruff Triplets dies, put a number of +1/+1 counters equal to its power on each creature you control named Gruff Triplets. this.addAbility(new DiesSourceTriggeredAbility( new AddCountersAllEffect( - CounterType.P1P1.createInstance(0), + CounterType.P1P1.createInstance(), new SourcePermanentPowerCount(), filterNamedGruffTriplets ).setText("put a number of +1/+1 counters equal to its power on each creature you control named Gruff Triplets.") diff --git a/Mage.Sets/src/mage/cards/v/VojaJawsOfTheConclave.java b/Mage.Sets/src/mage/cards/v/VojaJawsOfTheConclave.java index abe72d6c645..90a07a38ed1 100644 --- a/Mage.Sets/src/mage/cards/v/VojaJawsOfTheConclave.java +++ b/Mage.Sets/src/mage/cards/v/VojaJawsOfTheConclave.java @@ -58,8 +58,7 @@ public final class VojaJawsOfTheConclave extends CardImpl { // where X is the number of Elves you control. Draw a card for each Wolf you control. Ability ability = new AttacksTriggeredAbility( new AddCountersAllEffect( - // Set amount to 0, otherwise AddCountersAllEffect.apply() will default to amount = 1 if xValueElves = 0 - CounterType.P1P1.createInstance(0), + CounterType.P1P1.createInstance(), xValueElves, StaticFilters.FILTER_CONTROLLED_CREATURE ).setText("put X +1/+1 counters on each creature you control, where X is the number of Elves you control")); diff --git a/Mage.Tests/src/test/java/org/mage/test/cards/single/dmu/BriarHydraTest.java b/Mage.Tests/src/test/java/org/mage/test/cards/single/dmu/BriarHydraTest.java new file mode 100644 index 00000000000..0576b42656b --- /dev/null +++ b/Mage.Tests/src/test/java/org/mage/test/cards/single/dmu/BriarHydraTest.java @@ -0,0 +1,58 @@ +package org.mage.test.cards.single.dmu; + +import mage.constants.PhaseStep; +import mage.constants.Zone; +import org.junit.Test; +import org.mage.test.serverside.base.CardTestPlayerBase; + +/** + * @author Susucr + */ +public class BriarHydraTest extends CardTestPlayerBase { + + private static final String thranPortal = "Thran Portal"; + + /** + * {@link mage.cards.b.BriarHydra Briar Hydra} {5}{G} + * Creature — Plant Hydra + * Trample + * Domain — Whenever Briar Hydra deals combat damage to a player, put X +1/+1 counters on target creature you control, where X is the number of basic land types among lands you control. + * 6/6 + */ + private static final String hydra = "Briar Hydra"; + + /** + * There was a bug when the Domain value was 0, the Hydra would incorrectly + * add a +1/+1 counter to the target. + */ + @Test + public void test_NoDomain() { + setStrictChooseMode(true); + + addCard(Zone.BATTLEFIELD, playerA, hydra); + + attack(1, playerA, hydra, playerB); + addTarget(playerA, hydra); // trigger does target + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPowerToughness(playerA, hydra, 6, 6); + } + + @Test + public void test_Domain_2() { + setStrictChooseMode(true); + + addCard(Zone.BATTLEFIELD, playerA, hydra); + addCard(Zone.BATTLEFIELD, playerA, "Bayou"); + + attack(1, playerA, hydra, playerB); + addTarget(playerA, hydra); // trigger does target + + setStopAt(1, PhaseStep.END_TURN); + execute(); + + assertPowerToughness(playerA, hydra, 6 + 2, 6 + 2); + } +} diff --git a/Mage/src/main/java/mage/abilities/effects/common/counter/AddCountersAllEffect.java b/Mage/src/main/java/mage/abilities/effects/common/counter/AddCountersAllEffect.java index f632a005bce..b24eea20227 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/counter/AddCountersAllEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/counter/AddCountersAllEffect.java @@ -44,29 +44,30 @@ public class AddCountersAllEffect extends OneShotEffect { public boolean apply(Game game, Ability source) { Player controller = game.getPlayer(source.getControllerId()); MageObject sourceObject = game.getObject(source); - if (controller != null && sourceObject != null) { - if (counter != null) { - for (Permanent permanent : game.getBattlefield().getActivePermanents(filter, source.getControllerId(), source, game)) { - Counter newCounter = counter.copy(); - int calculated = amount.calculate(game, source, this); // 0 -- you must use default counter - if (calculated < 0) { - continue; - } else if (calculated == 0) { - // use original counter - } else { - // increase to calculated value - newCounter.remove(newCounter.getCount()); - newCounter.add(calculated); - } + if (controller != null && sourceObject != null && counter != null) { + Counter newCounter = counter.copy(); + int calculated = amount.calculate(game, source, this); + if (!(amount instanceof StaticValue) || calculated > 0) { + // If dynamic, or static and set to a > 0 value, we use that instead of the counter's internal amount. + newCounter.remove(newCounter.getCount()); + newCounter.add(calculated); + } else { + // StaticValue 0 -- the default counter has the amount, so no adjustment. + } - permanent.addCounters(newCounter, source.getControllerId(), source, game); - if (!game.isSimulation() && newCounter.getCount() > 0) { - game.informPlayers(sourceObject.getLogName() + ": " + controller.getLogName() + " puts " + newCounter.getCount() + ' ' + newCounter.getName() - + (newCounter.getCount() == 1 ? " counter" : " counters") + " on " + permanent.getLogName()); - } + if (newCounter.getCount() <= 0) { + return false; // no need to iterate on targets, no counters will be put on them + } + + for (Permanent permanent : game.getBattlefield().getActivePermanents(filter, source.getControllerId(), source, game)) { + Counter newCounterForPermanent = counter.copy(); + + permanent.addCounters(newCounterForPermanent, source.getControllerId(), source, game); + if (!game.isSimulation() && newCounterForPermanent.getCount() > 0) { + game.informPlayers(sourceObject.getLogName() + ": " + controller.getLogName() + " puts " + newCounterForPermanent.getCount() + ' ' + newCounterForPermanent.getName() + + (newCounterForPermanent.getCount() == 1 ? " counter" : " counters") + " on " + permanent.getLogName()); } } - return true; } return false; } diff --git a/Mage/src/main/java/mage/abilities/effects/common/counter/AddCountersTargetEffect.java b/Mage/src/main/java/mage/abilities/effects/common/counter/AddCountersTargetEffect.java index d3c8314802e..883f5daed5f 100644 --- a/Mage/src/main/java/mage/abilities/effects/common/counter/AddCountersTargetEffect.java +++ b/Mage/src/main/java/mage/abilities/effects/common/counter/AddCountersTargetEffect.java @@ -56,38 +56,42 @@ public class AddCountersTargetEffect extends OneShotEffect { Player controller = game.getPlayer(source.getControllerId()); MageObject sourceObject = game.getObject(source); if (controller != null && sourceObject != null && counter != null) { + Counter newCounter = counter.copy(); + int calculated = amount.calculate(game, source, this); + if (!(amount instanceof StaticValue) || calculated > 0) { + // If dynamic, or static and set to a > 0 value, we use that instead of the counter's internal amount. + newCounter.remove(newCounter.getCount()); + newCounter.add(calculated); + } else { + // StaticValue 0 -- the default counter has the amount, so no adjustment. + } + + if (newCounter.getCount() <= 0) { + return false; // no need to iterate on targets, no counters will be put on them + } + int affectedTargets = 0; for (UUID uuid : getTargetPointer().getTargets(game, source)) { - Counter newCounter = counter.copy(); - int calculated = amount.calculate(game, source, this); // 0 -- you must use default couner - if (calculated < 0) { - continue; - } else if (calculated == 0) { - // use original counter - } else { - // increase to calculated value - newCounter.remove(newCounter.getCount()); - newCounter.add(calculated); - } + Counter newCounterForTarget = newCounter.copy(); Permanent permanent = game.getPermanent(uuid); Player player = game.getPlayer(uuid); Card card = game.getCard(getTargetPointer().getFirst(game, source)); if (permanent != null) { - permanent.addCounters(newCounter, source.getControllerId(), source, game); + permanent.addCounters(newCounterForTarget, source.getControllerId(), source, game); affectedTargets++; game.informPlayers(sourceObject.getLogName() + ": " + controller.getLogName() + " puts " - + newCounter.getCount() + ' ' + newCounter.getName() + " counters on " + permanent.getLogName()); + + newCounterForTarget.getCount() + ' ' + newCounterForTarget.getName() + " counters on " + permanent.getLogName()); } else if (player != null) { - player.addCounters(newCounter, source.getControllerId(), source, game); + player.addCounters(newCounterForTarget, source.getControllerId(), source, game); affectedTargets++; game.informPlayers(sourceObject.getLogName() + ": " + controller.getLogName() + " puts " - + newCounter.getCount() + ' ' + newCounter.getName() + " counters on " + player.getLogName()); + + newCounterForTarget.getCount() + ' ' + newCounterForTarget.getName() + " counters on " + player.getLogName()); } else if (card != null) { - card.addCounters(newCounter, source.getControllerId(), source, game); + card.addCounters(newCounterForTarget, source.getControllerId(), source, game); affectedTargets++; game.informPlayers(sourceObject.getLogName() + ": " + controller.getLogName() + " puts " - + newCounter.getCount() + ' ' + newCounter.getName() + " counters on " + card.getLogName()); + + newCounterForTarget.getCount() + ' ' + newCounterForTarget.getName() + " counters on " + card.getLogName()); } } return affectedTargets > 0;