Devs: added docs and todo about short living LKI, LKI and current problems;

This commit is contained in:
Oleg Agafonov 2021-09-05 18:44:50 +04:00
parent 107c10fd2c
commit 83017e3c51
10 changed files with 79 additions and 19 deletions

View file

@ -89,7 +89,7 @@ class BlessingOfFrostEffect extends OneShotEffect {
permanent.addCounters(CounterType.P1P1.createInstance(target.getTargetAmount(targetId)), source.getControllerId(), source, game); permanent.addCounters(CounterType.P1P1.createInstance(target.getTargetAmount(targetId)), source.getControllerId(), source, game);
} }
} }
game.applyEffects(); game.getState().processAction(game);
player.drawCards(game.getBattlefield().count( player.drawCards(game.getBattlefield().count(
filter, source.getSourceId(), source.getControllerId(), game filter, source.getSourceId(), source.getControllerId(), game
), source, game); ), source, game);

View file

@ -86,7 +86,7 @@ class BlimComedicGeniusEffect extends OneShotEffect {
game.addEffect(new GainControlTargetEffect( game.addEffect(new GainControlTargetEffect(
Duration.Custom, true, targetPointer.getFirst(game, source) Duration.Custom, true, targetPointer.getFirst(game, source)
).setTargetPointer(new FixedTarget(source.getFirstTarget(), game)), source); ).setTargetPointer(new FixedTarget(source.getFirstTarget(), game)), source);
game.applyEffects(); game.getState().processAction(game);
Map<UUID, Cards> cardsMap = new HashMap<>(); Map<UUID, Cards> cardsMap = new HashMap<>();
for (UUID playerId : game.getState().getPlayersInRange(source.getControllerId(), game)) { for (UUID playerId : game.getState().getPlayersInRange(source.getControllerId(), game)) {
Player player = game.getPlayer(playerId); Player player = game.getPlayer(playerId);

View file

@ -76,7 +76,7 @@ class SelflessExorcistEffect extends OneShotEffect {
return false; return false;
} }
player.moveCards(card, Zone.EXILED, source, game); player.moveCards(card, Zone.EXILED, source, game);
game.applyEffects(); game.getState().processAction(game);
Permanent permanent = source.getSourcePermanentIfItStillExists(game); Permanent permanent = source.getSourcePermanentIfItStillExists(game);
if (permanent == null) { if (permanent == null) {
return true; return true;

View file

@ -109,7 +109,7 @@ class ThievingSkydiverEffect extends OneShotEffect {
|| !artifact.hasSubtype(SubType.EQUIPMENT, game)) { || !artifact.hasSubtype(SubType.EQUIPMENT, game)) {
return false; return false;
} }
game.applyEffects(); game.getState().processAction(game);
permanent.addAttachment(artifact.getId(), source, game); permanent.addAttachment(artifact.getId(), source, game);
return true; return true;
} }

View file

@ -17,7 +17,7 @@ public class GoblinWelderTest extends CardTestPlayerBase {
private static final String relic = "Darksteel Relic"; private static final String relic = "Darksteel Relic";
private static final String aspirant = "Blood Aspirant"; private static final String aspirant = "Blood Aspirant";
@Ignore @Ignore // TODO: related to problems with dies triggers and short living LKI, see TriggeredAbilityImpl for details
@Test @Test
public void testSacrificeDiesTrigger() { public void testSacrificeDiesTrigger() {
addCard(Zone.BATTLEFIELD, playerA, welder); addCard(Zone.BATTLEFIELD, playerA, welder);
@ -25,10 +25,15 @@ public class GoblinWelderTest extends CardTestPlayerBase {
addCard(Zone.BATTLEFIELD, playerA, aspirant); addCard(Zone.BATTLEFIELD, playerA, aspirant);
addCard(Zone.GRAVEYARD, playerA, relic); addCard(Zone.GRAVEYARD, playerA, relic);
addTarget(playerA, relic);
addTarget(playerA, wurmcoil);
activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "{T}:"); activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "{T}:");
addTarget(playerA, wurmcoil);
addTarget(playerA, relic);
waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN, true);
// must have 2 dies triggers on stack: from source and from another, but it have only from another
setChoice(playerA, "Whenever you sacrifice a permanent"); // select from 2 triggers
checkStackSize("check triggers", 1, PhaseStep.PRECOMBAT_MAIN, playerA, 2);
setStrictChooseMode(true);
setStopAt(1, PhaseStep.END_TURN); setStopAt(1, PhaseStep.END_TURN);
execute(); execute();
assertAllCommandsUsed(); assertAllCommandsUsed();
@ -36,6 +41,6 @@ public class GoblinWelderTest extends CardTestPlayerBase {
assertGraveyardCount(playerA, wurmcoil, 1); assertGraveyardCount(playerA, wurmcoil, 1);
assertPermanentCount(playerA, relic, 1); assertPermanentCount(playerA, relic, 1);
assertCounterCount(aspirant, CounterType.P1P1, 1); assertCounterCount(aspirant, CounterType.P1P1, 1);
assertPermanentCount(playerA, "Wurm", 2); // TODO: currently fails here assertPermanentCount(playerA, "Wurm", 2);
} }
} }

View file

@ -1,4 +1,3 @@
package org.mage.test.cards.triggers.dies; package org.mage.test.cards.triggers.dies;
import mage.constants.PhaseStep; import mage.constants.PhaseStep;
@ -7,9 +6,8 @@ import org.junit.Test;
import org.mage.test.serverside.base.CardTestPlayerBase; import org.mage.test.serverside.base.CardTestPlayerBase;
/** /**
*
* @author noxx * @author noxx
* * <p>
* Whenever Blood Artist or another creature dies, target player loses 1 life * Whenever Blood Artist or another creature dies, target player loses 1 life
* and you gain 1 life. * and you gain 1 life.
*/ */
@ -30,11 +28,20 @@ public class BloodArtistTest extends CardTestPlayerBase {
addCard(Zone.BATTLEFIELD, playerB, "Bloodflow Connoisseur", 1); addCard(Zone.BATTLEFIELD, playerB, "Bloodflow Connoisseur", 1);
// 2x blood artist, kill one of it and get 3x dies triggers
// from living artist: 2x triggers
// from killed artist: 1x trigger
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Lightning Bolt", "Blood Artist"); castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Lightning Bolt", "Blood Artist");
setChoice(playerA, "Whenever {this} or another creature"); // 2x dies triggers
addTarget(playerA, playerB, 2); // targets for 2x triggers
castSpell(1, PhaseStep.POSTCOMBAT_MAIN, playerA, "Lightning Bolt", "Bloodflow Connoisseur"); castSpell(1, PhaseStep.POSTCOMBAT_MAIN, playerA, "Lightning Bolt", "Bloodflow Connoisseur");
addTarget(playerA, playerB); // targets for 1x trigger (from living artist)
setStrictChooseMode(true);
setStopAt(1, PhaseStep.END_TURN); setStopAt(1, PhaseStep.END_TURN);
execute(); execute();
assertAllCommandsUsed();
assertLife(playerA, 23); assertLife(playerA, 23);
assertLife(playerB, 17); assertLife(playerB, 17);
@ -53,11 +60,15 @@ public class BloodArtistTest extends CardTestPlayerBase {
addCard(Zone.BATTLEFIELD, playerB, "Pillarfield Ox", 1); addCard(Zone.BATTLEFIELD, playerB, "Pillarfield Ox", 1);
// sac 2x and gen 2x dies triggers
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Bone Splinters", "Pillarfield Ox"); castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Bone Splinters", "Pillarfield Ox");
setChoice(playerA, "Silvercoat Lion"); setChoice(playerA, "Silvercoat Lion"); // sacrifice for cost
addTarget(playerA, playerB, 2);
setStrictChooseMode(true);
setStopAt(1, PhaseStep.BEGIN_COMBAT); setStopAt(1, PhaseStep.BEGIN_COMBAT);
execute(); execute();
assertAllCommandsUsed();
assertGraveyardCount(playerA, "Bone Splinters", 1); assertGraveyardCount(playerA, "Bone Splinters", 1);
assertGraveyardCount(playerA, "Silvercoat Lion", 1); assertGraveyardCount(playerA, "Silvercoat Lion", 1);
@ -77,11 +88,15 @@ public class BloodArtistTest extends CardTestPlayerBase {
addCard(Zone.BATTLEFIELD, playerB, "Pillarfield Ox", 1); addCard(Zone.BATTLEFIELD, playerB, "Pillarfield Ox", 1);
// sac blood artist as a cost and trigger 1x
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Bone Splinters", "Pillarfield Ox"); castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Bone Splinters", "Pillarfield Ox");
setChoice(playerA, "Blood Artist"); setChoice(playerA, "Blood Artist"); // sacrifice for cost
addTarget(playerA, playerB); // dies trigger with lose life
setStrictChooseMode(true);
setStopAt(1, PhaseStep.BEGIN_COMBAT); setStopAt(1, PhaseStep.BEGIN_COMBAT);
execute(); execute();
assertAllCommandsUsed();
assertGraveyardCount(playerA, "Bone Splinters", 1); assertGraveyardCount(playerA, "Bone Splinters", 1);
assertGraveyardCount(playerA, "Blood Artist", 1); assertGraveyardCount(playerA, "Blood Artist", 1);
@ -104,13 +119,17 @@ public class BloodArtistTest extends CardTestPlayerBase {
addCard(Zone.BATTLEFIELD, playerB, "Pillarfield Ox", 1); addCard(Zone.BATTLEFIELD, playerB, "Pillarfield Ox", 1);
addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion", 1); addCard(Zone.BATTLEFIELD, playerB, "Silvercoat Lion", 1);
// sac blood artist for cost, trigger 1x BUT remove blood artist before trigger resolve
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Bone Splinters", "Pillarfield Ox"); castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Bone Splinters", "Pillarfield Ox");
setChoice(playerA, "Blood Artist"); setChoice(playerA, "Blood Artist"); // sacrifice for cost
// Blood Artist may no longer trigger from destroyed creature because already in the graveyard addTarget(playerA, playerB); // dies trigger with lose life, but it will be fizzled
// remove boold artist first - Blood Artist may no longer trigger from destroyed creature because already in the graveyard
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Terror", "Silvercoat Lion", "Bone Splinters"); castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Terror", "Silvercoat Lion", "Bone Splinters");
setStrictChooseMode(true);
setStopAt(1, PhaseStep.BEGIN_COMBAT); setStopAt(1, PhaseStep.BEGIN_COMBAT);
execute(); execute();
assertAllCommandsUsed();
assertGraveyardCount(playerA, "Bone Splinters", 1); assertGraveyardCount(playerA, "Bone Splinters", 1);
assertGraveyardCount(playerA, "Terror", 1); assertGraveyardCount(playerA, "Terror", 1);

View file

@ -27,8 +27,12 @@ public class OmnathLocusOfRageTest extends CardTestPlayerBase {
addCard(Zone.HAND, playerB, "Diabolic Edict", 1); // {1}{B} addCard(Zone.HAND, playerB, "Diabolic Edict", 1); // {1}{B}
addCard(Zone.BATTLEFIELD, playerB, "Swamp", 2); addCard(Zone.BATTLEFIELD, playerB, "Swamp", 2);
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Diabolic Edict", playerA); castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Diabolic Edict");
addTarget(playerB, playerA);
addTarget(playerA, "Omnath, Locus of Rage"); // sacrifice target
addTarget(playerA, playerB); // target for dies trigger with damage
setStrictChooseMode(true);
setStopAt(1, PhaseStep.BEGIN_COMBAT); setStopAt(1, PhaseStep.BEGIN_COMBAT);
execute(); execute();
assertAllCommandsUsed(); assertAllCommandsUsed();
@ -55,7 +59,9 @@ public class OmnathLocusOfRageTest extends CardTestPlayerBase {
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Lightning Bolt", "Lightning Elemental"); // Dying Lightning Elemental does no longer trigger ability of Omnath castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Lightning Bolt", "Lightning Elemental"); // Dying Lightning Elemental does no longer trigger ability of Omnath
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Blastfire Bolt", "Omnath, Locus of Rage"); castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Blastfire Bolt", "Omnath, Locus of Rage");
addTarget(playerA, playerB); // target for dies trigger with damage
setStrictChooseMode(true);
setStopAt(1, PhaseStep.BEGIN_COMBAT); setStopAt(1, PhaseStep.BEGIN_COMBAT);
execute(); execute();
assertAllCommandsUsed(); assertAllCommandsUsed();
@ -67,7 +73,6 @@ public class OmnathLocusOfRageTest extends CardTestPlayerBase {
assertLife(playerA, 20); assertLife(playerA, 20);
assertLife(playerB, 17); assertLife(playerB, 17);
} }
@Test @Test
@ -84,6 +89,7 @@ public class OmnathLocusOfRageTest extends CardTestPlayerBase {
setChoice(playerB, "Green"); setChoice(playerB, "Green");
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Stave Off", "Silvercoat Lion"); castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "Stave Off", "Silvercoat Lion");
setStrictChooseMode(true);
setStopAt(1, PhaseStep.BEGIN_COMBAT); setStopAt(1, PhaseStep.BEGIN_COMBAT);
execute(); execute();
assertAllCommandsUsed(); assertAllCommandsUsed();

View file

@ -296,6 +296,34 @@ public abstract class TriggeredAbilityImpl extends AbilityImpl implements Trigge
if (game.getState().getZone(source.getSourceId()) == Zone.BATTLEFIELD) { if (game.getState().getZone(source.getSourceId()) == Zone.BATTLEFIELD) {
sourceObject = game.getPermanent(source.getSourceId()); sourceObject = game.getPermanent(source.getSourceId());
} else { } else {
// TODO: multiple calls of ApplyEffects all around the code are breaking a short living lki idea
// (PlayerImpl's call to move to battlefield do the worse thing)
// -
// Original idea: short living LKI must help to find a moment in the inner of resolve
// -
// Example:
// --!---------------!-------------!-----!-----------!
// - ! steps ! perm zone ! LKI ! short LKI !
// --!---------------!-------------!-----!-----------!
// - ! resolve start ! battlefield ! no ! no !
// - ! step 1 ! battlefield ! no ! no ! permanent moving to graveyard by step's command
// - ! step 2 ! graveyard ! yes ! yes ! other commands
// - ! step 3 ! graveyard ! yes ! yes ! other commands
// - ! raise triggers! graveyard ! yes ! yes ! handle and put triggers that was raised in resolve steps
// - ! resolve end ! graveyard ! yes ! no !
// - ! resolve next ! graveyard ! yes ! no ! resolve next spell
// - ! empty stack ! graveyard ! no ! no ! no more to resolve
// --!---------------!-------------!-----!-----------!
// -
// - Problem 1: move code (well, not only move) calls ApplyEffects in the middle of the resolve
// - and reset short LKI (after short LKI reset dies trigger will not work)
// - Example: Goblin Welder calls sacrifice and card move in the same effect - but move call do
// - a reset and dies trigger ignored (trigger thinks that permanent already dies)
// -
// - Possible fix:
// - replace ApplyEffects in the move code by game.getState().processAction(game);
// - check and fix many broken (is it was a false positive test or something broken)
//sourceObject = (Permanent) game.getLastKnownInformation(source.getSourceId(), Zone.BATTLEFIELD);
if (game.getShortLivingLKI(source.getSourceId(), Zone.BATTLEFIELD)) { if (game.getShortLivingLKI(source.getSourceId(), Zone.BATTLEFIELD)) {
sourceObject = (Permanent) game.getLastKnownInformation(source.getSourceId(), Zone.BATTLEFIELD); sourceObject = (Permanent) game.getLastKnownInformation(source.getSourceId(), Zone.BATTLEFIELD);
} }

View file

@ -1984,8 +1984,7 @@ public abstract class GameImpl implements Game {
break; break;
} }
} }
state.handleSimultaneousEvent(this); this.getState().processAction(this); // needed e.g if boost effects end and cause creatures to die
applyEffects(); // needed e.g if boost effects end and cause creatures to die
somethingHappened = true; somethingHappened = true;
} }
checkConcede(); checkConcede();

View file

@ -4484,6 +4484,9 @@ public abstract class PlayerImpl implements Player, Serializable {
} }
} }
} }
// TODO: must be replaced by game.getState().processAction(game), see isInUseableZoneDiesTrigger comments
// about short living LKI problem
//game.getState().processAction(game);
game.applyEffects(); game.applyEffects();
break; break;
case HAND: case HAND: