external-repos/forge
1
0
Fork 0
mirror of https://github.com/Card-Forge/forge.git synced 2025-11-17 11:18:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

A fix for OncePerEffect script ability applying too much when triggered by costs. Affects The Gitrog Monster and Sidisi, Brood Tyrant. Before the fix if multiple lands are discarded/sacrificed to a single cost (e.g Keldon Arzonist, Zombie Infestatio, etc) it would trigger the Girtog Monster's card draw ability multiple times. (Similar for Sidisi milling multiple creatures as part of a cost. Don't know a card that has mill multiple cards as a cost.)

Browse Source 
It implements a new stack for tracking cost payments and a wrapper to track the cost payment instance.  Change zone triggers use this information to de-dupe multiple triggers firing from the same cost.
This commit is contained in:
jje4th
2016-05-03 17:25:13 +00:00
parent bb77ae962c
commit be23361026
7 changed files with 128 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
.gitattributes vendored
View File

@@ -519,6 +519,7 @@ forge-game/src/main/java/forge/game/cost/CostUnattach.java -text
forge-game/src/main/java/forge/game/cost/CostUntap.java -text
forge-game/src/main/java/forge/game/cost/CostUntapType.java -text
forge-game/src/main/java/forge/game/cost/ICostVisitor.java -text
forge-game/src/main/java/forge/game/cost/IndividualCostPaymentInstance.java -text
forge-game/src/main/java/forge/game/cost/PaymentDecision.java -text
forge-game/src/main/java/forge/game/cost/package-info.java svneol=native#text/plain
forge-game/src/main/java/forge/game/event/Event.java -text
@@ -719,6 +720,7 @@ forge-game/src/main/java/forge/game/trigger/TriggerVote.java -text
forge-game/src/main/java/forge/game/trigger/TriggerWaiting.java -text
forge-game/src/main/java/forge/game/trigger/WrappedAbility.java -text
forge-game/src/main/java/forge/game/trigger/package-info.java svneol=native#text/plain
forge-game/src/main/java/forge/game/zone/CostPaymentStack.java -text
forge-game/src/main/java/forge/game/zone/MagicStack.java svneol=native#text/plain
forge-game/src/main/java/forge/game/zone/PlayerZone.java svneol=native#text/plain
forge-game/src/main/java/forge/game/zone/PlayerZoneBattlefield.java svneol=native#text/plain

19
forge-game/src/main/java/forge/game/Game.java
View File

@@ -17,16 +17,7 @@
*/
package forge.game;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Random;
import java.util.Set;
import java.util.TreeMap;
import java.util.*;
import com.google.common.base.Predicate;
import com.google.common.base.Predicates;
@@ -46,6 +37,7 @@ import forge.game.card.CardUtil;
import forge.game.card.CardView;
import forge.game.combat.Combat;
import forge.game.cost.Cost;
import forge.game.cost.IndividualCostPaymentInstance;
import forge.game.event.Event;
import forge.game.event.GameEventGameOutcome;
import forge.game.phase.Phase;
@@ -64,16 +56,12 @@ import forge.game.spellability.SpellAbilityStackInstance;
import forge.game.spellability.SpellAbilityView;
import forge.game.trigger.TriggerHandler;
import forge.game.trigger.TriggerType;
import forge.game.zone.MagicStack;
import forge.game.zone.PlayerZone;
import forge.game.zone.Zone;
import forge.game.zone.ZoneType;
import forge.game.zone.*;
import forge.trackable.Tracker;
import forge.util.Aggregates;
import forge.util.collect.FCollection;
import forge.util.collect.FCollectionView;
import forge.util.Visitor;
import java.util.HashMap;
/**
* Represents the state of a <i>single game</i>, a new instance is created for each game.
@@ -91,6 +79,7 @@ public class Game {
public final Untap untap;
public final Upkeep upkeep;
public final MagicStack stack;
public final CostPaymentStack costPaymentStack = new CostPaymentStack();
private final PhaseHandler phaseHandler;
private final StaticEffects staticEffects = new StaticEffects();
private final TriggerHandler triggerHandler = new TriggerHandler(this);

1
forge-game/src/main/java/forge/game/GameAction.java
View File

@@ -288,6 +288,7 @@ public class GameAction {
runParams.put("Origin", zoneFrom != null ? zoneFrom.getZoneType().name() : null);
runParams.put("Destination", zoneTo.getZoneType().name());
runParams.put("SpellAbilityStackInstance", game.stack.peek());
runParams.put("IndividualCostPaymentInstance", game.costPaymentStack.peek());
game.getTriggerHandler().runTrigger(TriggerType.ChangesZone, runParams, false);
if (zoneFrom != null && zoneFrom.is(ZoneType.Battlefield)) {
final HashMap<String, Object> runParams2 = new HashMap<String, Object>();

24
forge-game/src/main/java/forge/game/cost/CostPayment.java
View File

@@ -134,12 +134,18 @@ public class CostPayment {
public boolean payCost(final CostDecisionMakerBase decisionMaker) {
final List<CostPart> costParts = this.getCost().getCostPartsWithZeroMana();
for (final CostPart part : costParts) {
// Wrap the cost and push onto the cost stack
decisionMaker.getPlayer().getGame().costPaymentStack.push(new IndividualCostPaymentInstance(part));
PaymentDecision pd = part.accept(decisionMaker);
if (pd == null || !part.payAsDecided(decisionMaker.getPlayer(), pd, ability)) {
decisionMaker.getPlayer().getGame().costPaymentStack.pop(); // cost is resolved
return false;
}
this.paidCostParts.add(part);
decisionMaker.getPlayer().getGame().costPaymentStack.pop(); // cost is resolved
}
// this clears lists used for undo.
@@ -148,6 +154,7 @@ public class CostPayment {
((CostPartWithList) part1).resetLists();
}
}
return true;
}
@@ -164,21 +171,32 @@ public class CostPayment {
for (final CostPart part : this.cost.getCostParts()) {
PaymentDecision decision = part.accept(decisionMaker);
if (null == decision) return false;
if (decisionMaker.paysRightAfterDecision() && !part.payAsDecided(decisionMaker.getPlayer(), decision, ability))
// wrap the payment and push onto the cost stack
decisionMaker.getPlayer().getGame().costPaymentStack.push(new IndividualCostPaymentInstance(part));
if (decisionMaker.paysRightAfterDecision() && !part.payAsDecided(decisionMaker.getPlayer(), decision, ability)) {
decisionMaker.getPlayer().getGame().costPaymentStack.pop(); // cost is resolved
return false;
}
decisionMaker.getPlayer().getGame().costPaymentStack.pop(); // cost is either paid or deferred
decisions.put(part, decision);
}
for (final CostPart part : this.cost.getCostParts()) {
// wrap the payment and push onto the cost stack
decisionMaker.getPlayer().getGame().costPaymentStack.push(new IndividualCostPaymentInstance(part));
if (!part.payAsDecided(decisionMaker.getPlayer(), decisions.get(part), this.ability)) {
decisionMaker.getPlayer().getGame().costPaymentStack.pop(); // cost is resolved
return false;
}
// abilities care what was used to pay for them
if( part instanceof CostPartWithList ) {
((CostPartWithList) part).resetLists();
}
decisionMaker.getPlayer().getGame().costPaymentStack.pop(); // cost is resolved
}
return true;
}

21
forge-game/src/main/java/forge/game/cost/IndividualCostPaymentInstance.java Normal file
View File

@@ -0,0 +1,21 @@
package forge.game.cost;
import forge.game.IIdentifiable;
public class IndividualCostPaymentInstance implements IIdentifiable {
private static int maxId = 0;
private static int nextId() { return ++maxId; }
private final int id;
private final CostPart cost;
public IndividualCostPaymentInstance(final CostPart cost) {
id = nextId();
this.cost = cost;
}
public int getId() { return id; }
public CostPart getCost() { return cost; }
}

50
forge-game/src/main/java/forge/game/trigger/TriggerChangesZone.java
View File

@@ -20,11 +20,15 @@ package forge.game.trigger;
import forge.game.ability.AbilityUtils;
import forge.game.card.Card;
import forge.game.card.CardFactoryUtil;
import forge.game.cost.CostPayment;
import forge.game.cost.IndividualCostPaymentInstance;
import forge.game.spellability.SpellAbility;
import forge.game.spellability.SpellAbilityStackInstance;
import forge.util.Expressions;
import java.util.HashSet;
import java.util.Map;
import java.util.Set;
/**
* <p>
@@ -36,6 +40,10 @@ import java.util.Map;
*/
public class TriggerChangesZone extends Trigger {
// stores the costs when this trigger has already been run (to prevent multiple card draw triggers for single
// discard event of multiple cards on the Gitrog Moster for instance)
private Set<Integer> processedCostEffects = new HashSet<Integer>();
/**
* <p>
* Constructor for Trigger_ChangesZone.
@@ -131,14 +139,33 @@ public class TriggerChangesZone extends Trigger {
if (this.mapParams.containsKey("OncePerEffect")) {
// A "once per effect" trigger will only trigger once regardless of how many things the effect caused
// to change zones. The SpellAbilityStackInstance keeps track of which host cards with "OncePerEffect"
// triggers already fired as a result of that effect.
//
// TODO This isn't quite ideal, since it really should be keeping track of the SpellAbility of the host
// card, rather than keeping track of the host card itself - but it's good enough for now - since there are
// no cards with multiple different OncePerEffect triggers.
SpellAbilityStackInstance si = (SpellAbilityStackInstance) runParams2.get("SpellAbilityStackInstance");
return si == null || si.attemptOncePerEffectTrigger(this.getHostCard());
// to change zones.
// check if this is triggered by a cost payment & only fire if it isn't a duplicate trigger
IndividualCostPaymentInstance currentPayment = (IndividualCostPaymentInstance) runParams2.get("IndividualCostPaymentInstance");
if (currentPayment != null) { // only if there is an active cost
// each cost in a payment can trigger the effect for example Sinsiter Concoction has five costs:
// {B}, Pay one life, Mill a card, Discard a Card, and sacrifice Sinister Concoction
// If you mill a land and discard a land, The Gitrog Moster should trigger twice since each of these
// costs is an independent action
// however, due to forge implementation multiple triggers may be created for a single cost. For example,
// Zombie Infestation has a cost of "Discard two cards". If you discard two lands, The Gitrog Moster
// should only trigger once because discarding two lands is a single action.
return this.processedCostEffects.add(currentPayment.getId());
}
// otherwise use the stack ability
else {
// The SpellAbilityStackInstance keeps track of which host cards with "OncePerEffect"
// triggers already fired as a result of that effect.
// TODO This isn't quite ideal, since it really should be keeping track of the SpellAbility of the host
// card, rather than keeping track of the host card itself - but it's good enough for now - since there
// are no cards with multiple different OncePerEffect triggers.
SpellAbilityStackInstance si = (SpellAbilityStackInstance) runParams2.get("SpellAbilityStackInstance");
// si == null means the stack is empty
return si == null || si.attemptOncePerEffectTrigger(this.getHostCard());
}
}
/* this trigger can only be activated once per turn, verify it hasn't already run */
@@ -161,4 +188,11 @@ public class TriggerChangesZone extends Trigger {
sb.append("Zone Changer: ").append(sa.getTriggeringObject("Card"));
return sb.toString();
}
@Override
// Resets the state stored each turn for per-instance restriction
public void resetTurnState() {
super.resetTurnState();
this.processedCostEffects.clear();
}
}

37
forge-game/src/main/java/forge/game/zone/CostPaymentStack.java Normal file
View File

@@ -0,0 +1,37 @@
package forge.game.zone;
import forge.game.cost.IndividualCostPaymentInstance;
import java.util.Stack;
/*
* simple stack wrapper class for tracking cost payments (mainly for triggers to use)
*/
public class CostPaymentStack {
private Stack<IndividualCostPaymentInstance> stack;
public CostPaymentStack() {
stack = new Stack<IndividualCostPaymentInstance>();
}
public IndividualCostPaymentInstance push(IndividualCostPaymentInstance costPaymentInstance) {
return stack.push(costPaymentInstance);
}
public IndividualCostPaymentInstance pop(){
return stack.pop();
}
public IndividualCostPaymentInstance peek() {
if (stack.empty()) {
return null;
}
return stack.peek();
}
public void clear() {
stack.clear();
}
}