dies triggers improves:

* tests: added additional tests and verify/runtime checks for wrong die trigger settings;
* refactor: removed some usage of short LKI ;
* fixed dies events support in "or trigger" and "conditional trigger" (use cases like sacrifice cost);
* fixed dies events support in shared triggered abilities (use cases like sacrifice cost);
This commit is contained in:
Oleg Agafonov 2024-11-04 23:55:14 +04:00
parent a2ed52b8de
commit 66b338c6fc
18 changed files with 233 additions and 63 deletions

View file

@ -351,7 +351,12 @@ public interface Ability extends Controllable, Serializable {
void addWatcher(Watcher watcher);
/**
* Returns true if this abilities source is in the zone for the ability
* Allow to control ability/trigger's lifecycle
* <p>
* How-to use:
* - for normal abilities and triggers - keep default
* - for leave battlefield triggers - keep default + set setLeavesTheBattlefieldTrigger(true)
* - for dies triggers - override and use TriggeredAbilityImpl.isInUseableZoneDiesTrigger inside + set setLeavesTheBattlefieldTrigger(true)
*/
boolean isInUseableZone(Game game, MageObject source, GameEvent event);

View file

@ -29,7 +29,9 @@ import mage.game.Game;
import mage.game.command.Dungeon;
import mage.game.command.Emblem;
import mage.game.command.Plane;
import mage.game.events.BatchEvent;
import mage.game.events.GameEvent;
import mage.game.events.ZoneChangeEvent;
import mage.game.permanent.Permanent;
import mage.game.stack.Spell;
import mage.game.stack.StackAbility;
@ -1172,11 +1174,6 @@ public abstract class AbilityImpl implements Ability {
return false;
}
/**
* @param game
* @param source
* @return
*/
@Override
public boolean isInUseableZone(Game game, MageObject source, GameEvent event) {
if (!this.hasSourceObjectAbility(game, source, event)) {
@ -1201,13 +1198,74 @@ public abstract class AbilityImpl implements Ability {
} else {
parameterSourceId = getSourceId();
}
// old code:
// TODO: delete after dies fix
// check against shortLKI for effects that move multiple object at the same time (e.g. destroy all)
if (game.checkShortLivingLKI(getSourceId(), getZone())) {
return true;
//return true; // fix 1
}
// check against current state
Zone test = game.getState().getZone(parameterSourceId);
return zone.match(test);
// 603.10.
// Normally, objects that exist immediately after an event are checked to see if the event matched
// any trigger conditions, and continuous effects that exist at that time are used to determine what the
// trigger conditions are and what the objects involved in the event look like.
// ...
Zone lookingInZone = game.getState().getZone(parameterSourceId);
// 603.10.
// ...
// However, some triggered abilities are exceptions to this rule; the game looks back in time to determine
// if those abilities trigger, using the existence of those abilities and the appearance of objects
// immediately prior to the event. The list of exceptions is as follows:
// 603.10a
// Some zone-change triggers look back in time. These are leaves-the-battlefield abilities,
// abilities that trigger when a card leaves a graveyard, and abilities that trigger when an object that all
// players can see is put into a hand or library.
// TODO: research "leaves a graveyard"
// TODO: research "put into a hand or library"
if (source instanceof Permanent && isTriggerCanFireAfterLeaveBattlefield(event)) {
// support leaves-the-battlefield abilities
lookingInZone = Zone.BATTLEFIELD;
}
// TODO: research use cases and implement shared logic with "looking zone" instead LKI only
// 603.10b Abilities that trigger when a permanent phases out look back in time.
// 603.10c Abilities that trigger specifically when an object becomes unattached look back in time.
// 603.10d Abilities that trigger when a player loses control of an object look back in time.
// 603.10e Abilities that trigger when a spell is countered look back in time.
// 603.10f Abilities that trigger when a player loses the game look back in time.
// 603.10g Abilities that trigger when a player planeswalks away from a plane look back in time.
return zone.match(lookingInZone);
}
public static boolean isTriggerCanFireAfterLeaveBattlefield(GameEvent event) {
if (event == null) {
return false;
}
List<GameEvent> allEvents = new ArrayList<>();
if (event instanceof BatchEvent) {
allEvents.addAll(((BatchEvent) event).getEvents());
} else {
allEvents.add(event);
}
return allEvents.stream().anyMatch(e -> {
// TODO: add more events with zone change logic (or make it even't param)?
switch (e.getType()) {
case DESTROYED_PERMANENT:
case EXPLOITED_CREATURE:
return true;
case ZONE_CHANGE:
return ((ZoneChangeEvent) event).getFromZone() == Zone.BATTLEFIELD;
default:
return false;
}
});
}
@Override

View file

@ -27,7 +27,7 @@ public abstract class StaticAbility extends AbilityImpl {
@Override
public boolean isInUseableZone(Game game, MageObject source, GameEvent event) {
if (game.checkShortLivingLKI(getSourceId(), zone)) {
if (game.checkShortLivingLKI(getSourceId(), zone)) { // TODO: can be deleted? Need research
return true; // maybe this can be a problem if effects removed the ability from the object
}
if (game.getPermanentEntering(getSourceId()) != null && zone == Zone.BATTLEFIELD) {

View file

@ -62,8 +62,17 @@ public interface TriggeredAbility extends Ability {
TriggeredAbility setOptional();
/**
* Allow trigger to fire after source leave the battlefield (example: will use LKI on itself sacrifice)
*/
boolean isLeavesTheBattlefieldTrigger();
/**
* 603.6c,603.6d
* If true the game looks back in time to determine if those abilities trigger
* This has to be set, if the triggered ability has to check back in time if the permanent the ability is connected
* to had the ability on the battlefield while the trigger is checked
*/
void setLeavesTheBattlefieldTrigger(boolean leavesTheBattlefieldTrigger);
@Override

View file

@ -49,7 +49,6 @@ public abstract class TriggeredAbilityImpl extends AbilityImpl implements Trigge
// verify check: DoIfCostPaid effect already asks about action (optional), so no needs to ask it again in triggered ability
if (effect instanceof DoIfCostPaid && (this.optional && ((DoIfCostPaid) effect).isOptional())) {
throw new IllegalArgumentException("DoIfCostPaid effect must have only one optional settings, but it have two (trigger + DoIfCostPaid): " + this.getClass().getSimpleName());
}
}
@ -400,6 +399,7 @@ public abstract class TriggeredAbilityImpl extends AbilityImpl implements Trigge
}
break;
case DESTROYED_PERMANENT:
case EXPLOITED_CREATURE:
if (isLeavesTheBattlefieldTrigger()) {
source = game.getLastKnownInformation(getSourceId(), Zone.BATTLEFIELD);
}
@ -408,20 +408,11 @@ public abstract class TriggeredAbilityImpl extends AbilityImpl implements Trigge
return super.isInUseableZone(game, source, event);
}
/*
603.6c Leaves-the-battlefield abilities, 603.6d
if true the game looks back in time to determine if those abilities trigger,
using the existence of those abilities and the appearance of objects immediately prior to the event (603.10)
*/
@Override
public boolean isLeavesTheBattlefieldTrigger() {
return leavesTheBattlefieldTrigger;
}
/*
603.6c,603.6d
This has to be set, if the triggered ability has to check back in time if the permanent the ability is connected to had the ability on the battlefield while the trigger is checked
*/
@Override
public final void setLeavesTheBattlefieldTrigger(boolean leavesTheBattlefieldTrigger) {
this.leavesTheBattlefieldTrigger = leavesTheBattlefieldTrigger;
@ -453,12 +444,22 @@ public abstract class TriggeredAbilityImpl extends AbilityImpl implements Trigge
}
/**
* Looking object in GRAVEYARD zone only. If you need multi zone then use default isInUseableZone
* - good example: Whenever another creature you control dies
* - bad example: When {this} dies or is put into exile from the battlefield
* <p>
* For triggered abilities that function from the battlefield that must trigger when the source permanent dies
* and/or for any other events that happen simultaneously to the source permanent dying.
* (Similar logic must be used for any leaves-the-battlefield, but this method assumes to graveyard only.)
* NOTE: If your ability functions from another zone (not battlefield) then must use standard logic, not this.
*/
public static boolean isInUseableZoneDiesTrigger(TriggeredAbility source, GameEvent event, Game game) {
// runtime check: wrong trigger settings
if (!source.isLeavesTheBattlefieldTrigger()) {
// TODO: enable after fix
// throw new IllegalArgumentException("Wrong code usage: all dies triggers must use setLeavesTheBattlefieldTrigger(true)");
}
// Get the source permanent of the ability
MageObject sourceObject = null;
if (game.getState().getZone(source.getSourceId()) == Zone.BATTLEFIELD) {
@ -481,7 +482,7 @@ public abstract class TriggeredAbilityImpl extends AbilityImpl implements Trigge
// --!---------------!-------------!-----!-----------!
// -
if (game.checkShortLivingLKI(source.getSourceId(), Zone.BATTLEFIELD)) {
sourceObject = (Permanent) game.getLastKnownInformation(source.getSourceId(), Zone.BATTLEFIELD);
sourceObject = game.getLastKnownInformation(source.getSourceId(), Zone.BATTLEFIELD);
}
}
if (sourceObject == null) { // source is no permanent

View file

@ -48,6 +48,7 @@ public class DiesCreatureTriggeredAbility extends TriggeredAbilityImpl {
super(zone, effect, optional);
this.filter = filter;
this.setTargetPointer = setTargetPointer;
setLeavesTheBattlefieldTrigger(true);
setTriggerPhrase("Whenever " + filter.getMessage() + (filter.getMessage().startsWith("one or more") ? " die, " : " dies, "));
}

View file

@ -30,6 +30,7 @@ public class DiesThisOrAnotherTriggeredAbility extends TriggeredAbilityImpl {
filterMessage = filterMessage.substring(2);
}
setTriggerPhrase("Whenever {this} or another " + filterMessage + " dies, ");
setLeavesTheBattlefieldTrigger(true);
}
protected DiesThisOrAnotherTriggeredAbility(final DiesThisOrAnotherTriggeredAbility ability) {

View file

@ -48,22 +48,6 @@ public class ExploitCreatureTriggeredAbility extends TriggeredAbilityImpl {
return event.getType() == GameEvent.EventType.EXPLOITED_CREATURE;
}
@Override
public boolean isInUseableZone(Game game, MageObject source, GameEvent event) {
Permanent sourcePermanent = null;
if (game.getState().getZone(getSourceId()) == Zone.BATTLEFIELD) {
sourcePermanent = game.getPermanent(getSourceId());
} else {
if (game.checkShortLivingLKI(getSourceId(), Zone.BATTLEFIELD)) {
sourcePermanent = (Permanent) game.getLastKnownInformation(getSourceId(), Zone.BATTLEFIELD);
}
}
if (sourcePermanent == null) {
return false;
}
return hasSourceObjectAbility(game, sourcePermanent, event);
}
@Override
public boolean checkTrigger(GameEvent event, Game game) {
if (event.getSourceId().equals(getSourceId())) {

View file

@ -21,6 +21,7 @@ public class GodEternalDiesTriggeredAbility extends TriggeredAbilityImpl {
public GodEternalDiesTriggeredAbility() {
super(Zone.ALL, null, true);
this.setLeavesTheBattlefieldTrigger(true);
}
private GodEternalDiesTriggeredAbility(GodEternalDiesTriggeredAbility ability) {
@ -49,22 +50,6 @@ public class GodEternalDiesTriggeredAbility extends TriggeredAbilityImpl {
return false;
}
@Override
public boolean isInUseableZone(Game game, MageObject source, GameEvent event) {
Permanent sourcePermanent = null;
if (game.getState().getZone(getSourceId()) == Zone.BATTLEFIELD) {
sourcePermanent = game.getPermanent(getSourceId());
} else {
if (game.checkShortLivingLKI(getSourceId(), Zone.BATTLEFIELD)) {
sourcePermanent = (Permanent) game.getLastKnownInformation(getSourceId(), Zone.BATTLEFIELD);
}
}
if (sourcePermanent == null) {
return false;
}
return hasSourceObjectAbility(game, sourcePermanent, event);
}
@Override
public GodEternalDiesTriggeredAbility copy() {
return new GodEternalDiesTriggeredAbility(this);

View file

@ -259,8 +259,8 @@ public abstract class ManaCostImpl extends CostImpl implements ManaCost {
return false;
}
// TODO: is it require Phyrexian stile effects here for single payment?
//AbilityImpl.preparePhyrexianCost(game, source, player, ability, this);
// no needs to call
//AbilityImpl.handlePhyrexianLikeEffects(game, source, ability, this);
if (!player.getManaPool().isForcedToPay()) {
assignPayment(game, ability, player.getManaPool(), costToPay != null ? costToPay : this);

View file

@ -1,5 +1,6 @@
package mage.abilities.decorator;
import mage.MageObject;
import mage.abilities.Modes;
import mage.abilities.TriggeredAbility;
import mage.abilities.TriggeredAbilityImpl;
@ -7,6 +8,7 @@ import mage.abilities.condition.Condition;
import mage.abilities.effects.Effect;
import mage.abilities.effects.Effects;
import mage.constants.EffectType;
import mage.constants.Zone;
import mage.game.Game;
import mage.game.events.GameEvent;
import mage.util.CardUtil;
@ -133,4 +135,14 @@ public class ConditionalInterveningIfTriggeredAbility extends TriggeredAbilityIm
public boolean caresAboutManaColor() {
return condition.caresAboutManaColor();
}
@Override
public boolean isInUseableZone(Game game, MageObject source, GameEvent event) {
if (isLeavesTheBattlefieldTrigger()) {
// TODO: leaves battlefield and die are not same! Is it possible make a diff logic?
return TriggeredAbilityImpl.isInUseableZoneDiesTrigger(this, event, game);
} else {
return super.isInUseableZone(game, source, event);
}
}
}

View file

@ -1,5 +1,6 @@
package mage.abilities.meta;
import mage.MageObject;
import mage.abilities.Ability;
import mage.abilities.TriggeredAbility;
import mage.abilities.TriggeredAbilityImpl;
@ -10,10 +11,7 @@ import mage.game.events.GameEvent;
import mage.util.CardUtil;
import mage.watchers.Watcher;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.UUID;
import java.util.*;
import java.util.stream.Collectors;
/**
@ -46,6 +44,10 @@ public class OrTriggeredAbility extends TriggeredAbilityImpl {
for(Watcher watcher : ability.getWatchers()) {
super.addWatcher(watcher);
}
if (ability.isLeavesTheBattlefieldTrigger()) {
this.setLeavesTheBattlefieldTrigger(true);
}
}
setTriggerPhrase(generateTriggerPhrase());
}
@ -123,4 +125,19 @@ public class OrTriggeredAbility extends TriggeredAbilityImpl {
ability.addWatcher(watcher);
}
}
@Override
public boolean isInUseableZone(Game game, MageObject source, GameEvent event) {
boolean res = false;
for (TriggeredAbility ability : triggeredAbilities) {
// TODO: call full inner trigger instead like ability.isInUseableZone()?! Need research why it fails
if (ability.isLeavesTheBattlefieldTrigger()) {
// TODO: leaves battlefield and die are not same! Is it possible make a diff logic?
res |= TriggeredAbilityImpl.isInUseableZoneDiesTrigger(this, event, game);
} else {
res |= super.isInUseableZone(game, source, event);
}
}
return res;
}
}

View file

@ -63,7 +63,7 @@ class DackFaydenEmblemTriggeredAbility extends TriggeredAbilityImpl {
@Override
public boolean checkTrigger(GameEvent event, Game game) {
boolean returnValue = false;
List<UUID> targetedPermanentIds = new ArrayList<>(0);
List<UUID> targetedPermanentIds = new ArrayList<>();
Player player = game.getPlayer(this.getControllerId());
if (player != null) {
if (event.getPlayerId().equals(this.getControllerId())) {

View file

@ -15,7 +15,7 @@ import java.util.*;
*/
public abstract class NthTargetPointer extends TargetPointerImpl {
private static final List<UUID> emptyTargets = Collections.unmodifiableList(new ArrayList<>(0));
private static final List<UUID> emptyTargets = Collections.unmodifiableList(new ArrayList<>());
// TODO: rework to list of MageObjectReference instead zcc
private final Map<UUID, Integer> zoneChangeCounter = new HashMap<>();