Remove ConditionalTriggeredAbility and add trigger condition into triggered abilities (#13656)

* remove ConditionalTriggeredAbility

* a few small fixes

* merge fix

* simplify phrase handling

* add documentation

* a few text fixes

* update wording
This commit is contained in:
Evan Kranzler 2025-05-23 07:03:14 -04:00 committed by Failure
parent 7a63f352c9
commit 535f932ee3
47 changed files with 332 additions and 544 deletions

View file

@ -66,10 +66,35 @@ public interface TriggeredAbility extends Ability {
*/
TriggeredAbility withRuleTextReplacement(boolean replaceRuleText);
/**
* 603.4. A triggered ability may read "When/Whenever/At [trigger event], if [condition], [effect]."
* When the trigger event occurs, the ability checks whether the stated condition is true.
* The ability triggers only if it is; otherwise it does nothing. If the ability triggers,
* it checks the stated condition again as it resolves. If the condition isn't true at that time,
* the ability is removed from the stack and does nothing. Note that this mirrors the check for legal targets.
* This rule is referred to as the "intervening 'if' clause" rule.
* (The word "if" has only its normal English meaning anywhere else in the text of a card;
* this rule only applies to an "if" that immediately follows a trigger condition.)
*
* @param condition the condition to be checked
* @return
*/
TriggeredAbility withInterveningIf(Condition condition);
boolean checkInterveningIfClause(Game game);
/**
* Unlike intervening if, this is for a condition that's checked only on trigger and not also on resolution.
*
* @param condition the condition to be checked
* @return
*/
TriggeredAbility withTriggerCondition(Condition condition);
Condition getTriggerCondition();
boolean checkTriggerCondition(Game game);
boolean isOptional();
TriggeredAbility setOptional();

View file

@ -27,6 +27,7 @@ public abstract class TriggeredAbilityImpl extends AbilityImpl implements Trigge
private boolean optional;
private Condition interveningIfCondition;
private Condition triggerCondition;
private boolean leavesTheBattlefieldTrigger;
private int triggerLimitEachTurn = Integer.MAX_VALUE; // for "triggers only once|twice each turn"
private int triggerLimitEachGame = Integer.MAX_VALUE; // for "triggers only once|twice"
@ -57,6 +58,7 @@ public abstract class TriggeredAbilityImpl extends AbilityImpl implements Trigge
super(ability);
this.optional = ability.optional;
this.interveningIfCondition = ability.interveningIfCondition;
this.triggerCondition = ability.triggerCondition;
this.leavesTheBattlefieldTrigger = ability.leavesTheBattlefieldTrigger;
this.triggerLimitEachTurn = ability.triggerLimitEachTurn;
this.triggerLimitEachGame = ability.triggerLimitEachGame;
@ -69,7 +71,7 @@ public abstract class TriggeredAbilityImpl extends AbilityImpl implements Trigge
@Override
public void trigger(Game game, UUID controllerId, GameEvent triggeringEvent) {
//20091005 - 603.4
if (checkInterveningIfClause(game)) {
if (checkInterveningIfClause(game) && checkTriggerCondition(game)) {
updateTurnCount(game);
updateGameCount(game);
game.addTriggeredAbility(this, triggeringEvent);
@ -228,6 +230,28 @@ public abstract class TriggeredAbilityImpl extends AbilityImpl implements Trigge
return interveningIfCondition == null || interveningIfCondition.apply(game, this);
}
@Override
public TriggeredAbility withTriggerCondition(Condition condition) {
this.triggerCondition = condition;
if (this.triggerPhrase != null && !condition.toString().isEmpty()) {
this.setTriggerPhrase(
this.triggerPhrase.substring(0, this.triggerPhrase.length() - 2) + ' ' +
(condition.toString().startsWith("during") ? "" : "while ") + condition + ", "
);
}
return this;
}
@Override
public Condition getTriggerCondition() {
return triggerCondition;
}
@Override
public boolean checkTriggerCondition(Game game) {
return triggerCondition == null || triggerCondition.apply(game, this);
}
@Override
public boolean resolve(Game game) {
if (!checkInterveningIfClause(game)) {

View file

@ -1,6 +1,7 @@
package mage.abilities.common;
import mage.abilities.Ability;
import mage.abilities.TriggeredAbility;
import mage.abilities.condition.CompoundCondition;
import mage.abilities.condition.Condition;
import mage.abilities.condition.common.SolvedSourceCondition;
@ -8,7 +9,6 @@ import mage.abilities.decorator.ConditionalActivatedAbility;
import mage.abilities.decorator.ConditionalAsThoughEffect;
import mage.abilities.decorator.ConditionalContinuousEffect;
import mage.abilities.decorator.ConditionalReplacementEffect;
import mage.abilities.decorator.ConditionalTriggeredAbility;
import mage.abilities.effects.Effect;
import mage.abilities.effects.OneShotEffect;
import mage.abilities.triggers.BeginningOfEndStepTriggeredAbility;
@ -63,13 +63,13 @@ public class CaseAbility extends SimpleStaticAbility {
* The "Solved" ability must be one of the following:
* <ul>
* <li>{@link ConditionalActivatedAbility} using the condition {@link SolvedSourceCondition}.SOLVED</li>
* <li>{@link ConditionalTriggeredAbility} using the condition {@link SolvedSourceCondition}.SOLVED</li>
* <li>{@link TriggeredAbility} using the condition {@link SolvedSourceCondition}.SOLVED</li>
* <li>{@link SimpleStaticAbility} with only {@link ConditionalAsThoughEffect} or {@link ConditionalContinuousEffect} effects</li>
* </ul>
*
* @param initialAbility The ability that a Case has at all times
* @param initialAbility The ability that a Case has at all times
* @param toSolveCondition The condition to be checked when solving
* @param solvedAbility The ability that a solved Case has
* @param solvedAbility The ability that a solved Case has
*/
public CaseAbility(Ability initialAbility, Condition toSolveCondition, Ability solvedAbility) {
super(Zone.ALL, null);
@ -81,22 +81,28 @@ public class CaseAbility extends SimpleStaticAbility {
addSubAbility(new CaseSolveAbility(toSolveCondition));
if (solvedAbility instanceof ConditionalActivatedAbility) {
((ConditionalActivatedAbility) solvedAbility).hideCondition();
} else if (!(solvedAbility instanceof ConditionalTriggeredAbility)) {
if (solvedAbility instanceof SimpleStaticAbility) {
for (Effect effect : solvedAbility.getEffects()) {
if (!(effect instanceof ConditionalContinuousEffect ||
effect instanceof ConditionalAsThoughEffect ||
effect instanceof ConditionalReplacementEffect)) {
throw new IllegalArgumentException("Wrong code usage: solvedAbility must be one of ConditionalActivatedAbility, " +
"ConditionalTriggeredAbility, or StaticAbility with conditional effects.");
}
if (!(solvedAbility instanceof ConditionalActivatedAbility)) {
if (solvedAbility instanceof TriggeredAbility) {
if (!(((TriggeredAbility) solvedAbility).getTriggerCondition() instanceof SolvedSourceCondition)) {
throw new IllegalArgumentException("Wrong code usage: if solvedAbility is a TriggeredAbility it must have SolvedSourceCondition as its trigger condition");
}
} else {
throw new IllegalArgumentException("Wrong code usage: solvedAbility must be one of ConditionalActivatedAbility, " +
"ConditionalTriggeredAbility, or StaticAbility with conditional effects.");
if (solvedAbility instanceof SimpleStaticAbility) {
for (Effect effect : solvedAbility.getEffects()) {
if (!(effect instanceof ConditionalContinuousEffect ||
effect instanceof ConditionalAsThoughEffect ||
effect instanceof ConditionalReplacementEffect)) {
throw new IllegalArgumentException("Wrong code usage: solvedAbility must be one of ConditionalActivatedAbility, " +
"TriggeredAbility, or StaticAbility with conditional effects.");
}
}
} else {
throw new IllegalArgumentException("Wrong code usage: solvedAbility must be one of ConditionalActivatedAbility, " +
"TriggeredAbility, or StaticAbility with conditional effects.");
}
}
} else {
((ConditionalActivatedAbility) solvedAbility).hideCondition();
}
addSubAbility(solvedAbility.withFlavorWord("Solved")); // TODO: Technically this shouldn't be italicized
}

View file

@ -17,6 +17,6 @@ public enum OpponentsTurnCondition implements Condition {
@Override
public String toString() {
return "if it's an opponent's turn";
return "during an opponent's turn";
}
}

View file

@ -1,143 +0,0 @@
package mage.abilities.decorator;
import mage.MageObject;
import mage.abilities.Ability;
import mage.abilities.Modes;
import mage.abilities.TriggeredAbility;
import mage.abilities.TriggeredAbilityImpl;
import mage.abilities.condition.Condition;
import mage.abilities.effects.Effect;
import mage.abilities.effects.Effects;
import mage.abilities.hint.Hint;
import mage.constants.EffectType;
import mage.game.Game;
import mage.game.events.GameEvent;
import mage.util.CardUtil;
import mage.watchers.Watcher;
import java.util.ArrayList;
import java.util.List;
/**
* Adds condition to {@link mage.abilities.effects.ContinuousEffect}. Acts as
* decorator.
*
* @author noahg
*/
public class ConditionalTriggeredAbility extends TriggeredAbilityImpl {
protected TriggeredAbility ability;
protected Condition condition;
protected String abilityText;
/**
* Triggered ability with a condition. Set the optionality for the trigger
* ability itself.
*
* @param ability
* @param condition
* @param text explicit rule text for the ability, if null or empty, the
* rule text generated by the triggered ability itself is used.
*/
public ConditionalTriggeredAbility(TriggeredAbility ability, Condition condition, String text) {
super(ability.getZone(), null);
this.ability = ability;
this.condition = condition;
this.abilityText = text;
if (ability.isLeavesTheBattlefieldTrigger()) {
this.setLeavesTheBattlefieldTrigger(true);
}
}
protected ConditionalTriggeredAbility(final ConditionalTriggeredAbility triggered) {
super(triggered);
this.ability = triggered.ability.copy();
this.condition = triggered.condition;
this.abilityText = triggered.abilityText;
}
@Override
public ConditionalTriggeredAbility copy() {
return new ConditionalTriggeredAbility(this);
}
@Override
public boolean checkEventType(GameEvent event, Game game) {
return ability.checkEventType(event, game);
}
@Override
public boolean checkTrigger(GameEvent event, Game game) {
ability.setSourceId(this.getSourceId());
ability.setControllerId(this.getControllerId());
return ability.checkTrigger(event, game) && condition.apply(game, this);
}
@Override
public String getRule() {
if (abilityText == null || abilityText.isEmpty()) {
return ability.getRule();
}
return (flavorWord != null ? CardUtil.italicizeWithEmDash(flavorWord) : "") +
(abilityWord != null ? abilityWord.formatWord() : "") +
abilityText + (abilityText.endsWith(".") || abilityText.endsWith("\"") || abilityText.endsWith(">") ? "" : ".");
}
@Override
public Effects getEffects() {
return ability.getEffects();
}
@Override
public void addEffect(Effect effect) {
ability.addEffect(effect);
}
@Override
public Modes getModes() {
return ability.getModes();
}
@Override
public List<Watcher> getWatchers() {
return ability.getWatchers();
}
@Override
public void addWatcher(Watcher watcher) {
ability.addWatcher(watcher);
}
@Override
public List<Hint> getHints() {
List<Hint> res = new ArrayList<>(super.getHints());
res.addAll(ability.getHints());
return res;
}
@Override
public Effects getEffects(Game game, EffectType effectType) {
return ability.getEffects(game, effectType);
}
@Override
public boolean isOptional() {
return ability.isOptional();
}
@Override
public Ability withFlavorWord(String flavorWord) {
ability.withFlavorWord(flavorWord);
return this;
}
@Override
public boolean isInUseableZone(Game game, MageObject sourceObject, GameEvent event) {
if (isLeavesTheBattlefieldTrigger()) {
// TODO: leaves battlefield and die are not same! Is it possible make a diff logic?
return TriggeredAbilityImpl.isInUseableZoneDiesTrigger(this, sourceObject, event, game);
} else {
return super.isInUseableZone(game, sourceObject, event);
}
}
}

View file

@ -93,7 +93,7 @@ public class OrTriggeredAbility extends TriggeredAbilityImpl {
for (Effect e : getEffects()) { //Add effects to the sub-abilities so that they can set target pointers
ability.addEffect(e);
}
if (ability.checkEventType(event, game) && ability.checkTrigger(event, game)) {
if (ability.checkEventType(event, game) && ability.checkTrigger(event, game) && ability.checkTriggerCondition(game)) {
toRet = true;
}
ability.getEffects().clear(); //Remove afterwards, ensures that they remain synced even with copying