Skip to content

Commit

Permalink
Only fire one Targetted event per target
Browse files Browse the repository at this point in the history 
This specifically addresses changing the target of a spell or ability on the stack.
Fixes magefree#6158
  • Loading branch information
@Dilnu
Dilnu committed Feb 3, 2020
1 parent a8d8f4e commit b6af571
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 50 deletions.
41 changes: 1 addition & 40 deletions Mage.Tests/src/test/java/org/mage/test/cards/copy/CopySpellTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public void ZadaHedronGrinderBoost() {
}

@Test
public void BonecrusherGiantChangeTargetsTo() {
public void BonecrusherGiantChangeTargets() {
addCard(Zone.BATTLEFIELD, playerA, "Bonecrusher Giant");
addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears");
addCard(Zone.BATTLEFIELD, playerA, "Savannah Lions");
Expand All @@ -100,45 +100,6 @@ public void BonecrusherGiantChangeTargetsTo() {
assertLife(playerA, 18);
}

@Test
public void BonecrusherGiantChangeTargetsFrom() {
addCard(Zone.BATTLEFIELD, playerA, "Bonecrusher Giant");
addCard(Zone.BATTLEFIELD, playerA, "Grizzly Bears");
addCard(Zone.BATTLEFIELD, playerA, "Savannah Lions");
addCard(Zone.BATTLEFIELD, playerA, "Plains");
addCard(Zone.HAND, playerA, "Barkshell Blessing");

castSpell(1, PhaseStep.UPKEEP, playerA, "Barkshell Blessing");
setChoice(playerA, "Yes");
setChoice(playerA, "Yes");
addTarget(playerA, "Bonecrusher Giant");
addTarget(playerA, "Grizzly Bears");

setStopAt(1, PhaseStep.PRECOMBAT_MAIN);
execute();

assertPowerToughness(playerA, "Bonecrusher Giant", 6, 5);
assertPowerToughness(playerA, "Grizzly Bears", 4, 4);
assertPowerToughness(playerA, "Savannah Lions", 2, 1);
assertLife(playerA, 18);
}

@Test
public void BonecrusherGiantControl() {
addCard(Zone.BATTLEFIELD, playerA, "Bonecrusher Giant");
addCard(Zone.BATTLEFIELD, playerA, "Plains");
addCard(Zone.HAND, playerA, "Barkshell Blessing");

castSpell(1, PhaseStep.UPKEEP, playerA, "Barkshell Blessing");
addTarget(playerA, "Bonecrusher Giant");

setStopAt(1, PhaseStep.PRECOMBAT_MAIN);
execute();

assertPowerToughness(playerA, "Bonecrusher Giant", 6, 5);
assertLife(playerA, 18);
}

/*
* Reported bug: "Silverfur Partisan and fellow wolves did not trigger off
* of copies of Strength of Arms made by Zada, Hedron Grinder. Not sure
Expand Down
14 changes: 8 additions & 6 deletions Mage/src/main/java/mage/game/stack/StackObjImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ public boolean chooseNewTargets(Game game, UUID targetControllerId, boolean forc
*/
private Target chooseNewTarget(Player targetController, Ability ability, Mode mode, Target target, boolean forceChange, FilterPermanent filterNewTarget, Game game) {
Target newTarget = target.copy();
newTarget.setEventReporting(false);
if (!targetController.getId().equals(getControllerId())) {
newTarget.setTargetController(targetController.getId()); // target controller for the change is different from spell controller
newTarget.setAbilityController(getControllerId());
Expand Down Expand Up @@ -199,6 +200,7 @@ private Target chooseNewTarget(Player targetController, Ability ability, Mode mo
} else {
// build a target definition with exactly one possible target to select that replaces old target
Target tempTarget = target.copy();
tempTarget.setEventReporting(false);
if (target instanceof TargetAmount) {
((TargetAmount) tempTarget).setAmountDefinition(StaticValue.get(target.getTargetAmount(targetId)));
}
Expand All @@ -215,7 +217,7 @@ private Target chooseNewTarget(Player targetController, Ability ability, Mode mo
if (!tempTarget.chooseTarget(outcome, getControllerId(), ability, game)) {
if (targetController.chooseUse(Outcome.Benefit, "No target object selected. Reset to original target?", ability, game)) {
// use previous target no target was selected
newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, false);
newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, true);
} else {
again = true;
}
Expand All @@ -225,20 +227,20 @@ private Target chooseNewTarget(Player targetController, Ability ability, Mode mo
if (targetController.isHuman()) {
if (targetController.chooseUse(Outcome.Benefit, "This target was already selected from origin spell. Reset to original target?", ability, game)) {
// use previous target no target was selected
newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, false);
newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, true);
} else {
again = true;
}
} else {
newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, false);
newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, true);
}
} else if (!target.canTarget(getControllerId(), tempTarget.getFirstTarget(), ability, game)) {
if (targetController.isHuman()) {
game.informPlayer(targetController, "This target is not valid!");
again = true;
} else {
// keep the old
newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, false);
newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, true);
}
} else if (newTarget.getFirstTarget() != null && filterNewTarget != null) {
Permanent newTargetPermanent = game.getPermanent(newTarget.getFirstTarget());
Expand All @@ -248,14 +250,14 @@ private Target chooseNewTarget(Player targetController, Ability ability, Mode mo
}
} else {
// valid target was selected, add it to the new target definition
newTarget.addTarget(tempTarget.getFirstTarget(), target.getTargetAmount(targetId), ability, game, false);
newTarget.addTarget(tempTarget.getFirstTarget(), target.getTargetAmount(targetId), ability, game, true);
}
}
} while (again && targetController.canRespond());
}
} // keep the target
else {
newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, false);
newTarget.addTarget(targetId, target.getTargetAmount(targetId), ability, game, true);
}
}
return newTarget;
Expand Down
2 changes: 2 additions & 0 deletions Mage/src/main/java/mage/target/Target.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,4 +136,6 @@ public interface Target extends Serializable {
void setTargetAmount(UUID targetId, int amount, Game game);

Target withChooseHint(String chooseHint);

void setEventReporting(boolean shouldReport);
}
15 changes: 11 additions & 4 deletions Mage/src/main/java/mage/target/TargetImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public abstract class TargetImpl implements Target {

protected int targetTag; // can be set if other target check is needed (AnotherTargetPredicate)
protected String chooseHint = null; // UI choose hints after target name
protected boolean shouldReportEvents = true;

@Override
public abstract TargetImpl copy();
Expand Down Expand Up @@ -65,6 +66,7 @@ public TargetImpl(final TargetImpl target) {
this.abilityController = target.abilityController;
this.targetTag = target.targetTag;
this.chooseHint = target.chooseHint;
this.shouldReportEvents = target.shouldReportEvents;
}

@Override
Expand Down Expand Up @@ -213,12 +215,12 @@ public void addTarget(UUID id, Ability source, Game game, boolean skipEvent) {
//20100423 - 113.3
if (getMaxNumberOfTargets() == 0 || targets.size() < getMaxNumberOfTargets()) {
if (!targets.containsKey(id)) {
if (source != null && !skipEvent) {
if (source != null && !skipEvent && shouldReportEvents) {
if (!game.replaceEvent(GameEvent.getEvent(EventType.TARGET, id, source.getSourceId(), source.getControllerId()))) {
targets.put(id, 0);
rememberZoneChangeCounter(id, game);
chosen = targets.size() >= getNumberOfTargets();
if (!skipEvent) {
if (!skipEvent && shouldReportEvents) {
game.addSimultaneousEvent(GameEvent.getEvent(EventType.TARGETED, id, source.getSourceId(), source.getControllerId()));
}
}
Expand Down Expand Up @@ -251,12 +253,12 @@ public void addTarget(UUID id, int amount, Ability source, Game game, boolean sk
if (targets.containsKey(id)) {
amount += targets.get(id);
}
if (source != null && !skipEvent) {
if (source != null && !skipEvent && shouldReportEvents) {
if (!game.replaceEvent(GameEvent.getEvent(EventType.TARGET, id, source.getSourceId(), source.getControllerId()))) {
targets.put(id, amount);
rememberZoneChangeCounter(id, game);
chosen = targets.size() >= getNumberOfTargets();
if (!skipEvent) {
if (!skipEvent && shouldReportEvents) {
game.fireEvent(GameEvent.getEvent(EventType.TARGETED, id, source.getSourceId(), source.getControllerId()));
}
}
Expand Down Expand Up @@ -551,4 +553,9 @@ public Target withChooseHint(String chooseHint) {
this.chooseHint = chooseHint;
return this;
}

@Override
public void setEventReporting(boolean shouldReport) {
this.shouldReportEvents = shouldReport;
}
}

0 comments on commit b6af571

Please sign in to comment.