Fix simulation AI logic related to playing lands.

This got broken by the following refactor:
79c9c914e2

The result was that simulation AI was ignoring certain decision trees that involved
playing lands, leading to not considering certain lines of play and some log
messages printed to standard error. I've added a test that covers this logic to prevent
it breaking again in the future.

Also, a couple small clean ups to related tests.
This commit is contained in:
asvitkine
2022-11-02 10:26:37 -04:00
parent c6e4eaa19c
commit b0120eedf0
4 changed files with 42 additions and 20 deletions

View File

@@ -1,5 +1,6 @@
package forge.ai.simulation; package forge.ai.simulation;
import forge.game.spellability.LandAbility;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
@@ -154,7 +155,7 @@ public class GameSimulator {
} }
public Score simulateSpellAbility(SpellAbility origSa, GameStateEvaluator eval) { public Score simulateSpellAbility(SpellAbility origSa, GameStateEvaluator eval) {
SpellAbility sa; SpellAbility sa;
if (origSa instanceof SpellAbilityPicker.PlayLandAbility) { if (origSa instanceof LandAbility) {
Card hostCard = (Card) copier.find(origSa.getHostCard()); Card hostCard = (Card) copier.find(origSa.getHostCard());
if (!aiPlayer.playLand(hostCard, false)) { if (!aiPlayer.playLand(hostCard, false)) {
System.err.println("Simulation: Couldn't play land! " + origSa); System.err.println("Simulation: Couldn't play land! " + origSa);
@@ -164,7 +165,7 @@ public class GameSimulator {
// TODO: optimize: prune identical SA (e.g. two of the same card in hand) // TODO: optimize: prune identical SA (e.g. two of the same card in hand)
sa = findSaInSimGame(origSa); sa = findSaInSimGame(origSa);
if (sa == null) { if (sa == null) {
System.err.println("Simulation: SA not found! " + origSa); System.err.println("Simulation: SA not found! " + origSa + " / " + origSa.getClass());
return new Score(Integer.MIN_VALUE); return new Score(Integer.MIN_VALUE);
} }

View File

@@ -74,7 +74,7 @@ public class SpellAbilityPicker {
continue; continue;
} }
landsDeDupe.put(land.getName(), land); landsDeDupe.put(land.getName(), land);
all.add(new PlayLandAbility(land)); all.add(new LandAbility(land));
} }
} }
List<SpellAbility> candidateSAs = ComputerUtilAbility.getOriginalAndAltCostAbilities(all, player); List<SpellAbility> candidateSAs = ComputerUtilAbility.getOriginalAndAltCostAbilities(all, player);
@@ -147,8 +147,8 @@ public class SpellAbilityPicker {
private static boolean isSorcerySpeed(SpellAbility sa, Player player) { private static boolean isSorcerySpeed(SpellAbility sa, Player player) {
// TODO: Can we use the actual rules engine for this instead of trying to do the logic ourselves? // TODO: Can we use the actual rules engine for this instead of trying to do the logic ourselves?
if (sa instanceof PlayLandAbility) { if (sa instanceof LandAbility) {
return false; return true;
} }
if (sa.isSpell()) { if (sa.isSpell()) {
return !sa.withFlash(sa.getHostCard(), player); return !sa.withFlash(sa.getHostCard(), player);
@@ -334,7 +334,7 @@ public class SpellAbilityPicker {
} }
private AiPlayDecision canPlayAndPayForSim(final SpellAbility sa) { private AiPlayDecision canPlayAndPayForSim(final SpellAbility sa) {
if (sa instanceof PlayLandAbility) { if (sa instanceof LandAbility) {
return AiPlayDecision.WillPlay; return AiPlayDecision.WillPlay;
} }
if (!sa.canPlay()) { if (!sa.canPlay()) {
@@ -452,13 +452,4 @@ public class SpellAbilityPicker {
} }
return ComputerUtil.chooseSacrificeType(player, type, ability, ability.getTargetCard(), effect, amount, exclude); return ComputerUtil.chooseSacrificeType(player, type, ability, ability.getTargetCard(), effect, amount, exclude);
} }
public static class PlayLandAbility extends LandAbility {
public PlayLandAbility(Card land) {
super(land);
}
@Override
public String toUnsuppressedString() { return "Play land " + (getHostCard() != null ? getHostCard().getName() : ""); }
}
} }

View File

@@ -2242,8 +2242,8 @@ public class GameSimulationTest extends SimulationTest {
simGame.getPhaseHandler().devAdvanceToPhase(PhaseType.MAIN2); simGame.getPhaseHandler().devAdvanceToPhase(PhaseType.MAIN2);
AssertJUnit.assertEquals(21, simGame.getPlayers().get(0).getLife()); AssertJUnit.assertEquals(21, simGame.getPlayers().get(0).getLife());
AssertJUnit.assertEquals(true, simGoblin.isRed() && simGoblin.isBlack()); AssertJUnit.assertTrue(simGoblin.isRed() && simGoblin.isBlack());
AssertJUnit.assertEquals(true, simGoblin.getType().hasSubtype("Zombie")); AssertJUnit.assertTrue(simGoblin.getType().hasSubtype("Zombie"));
} }
@Test @Test

View File

@@ -1,5 +1,6 @@
package forge.ai.simulation; package forge.ai.simulation;
import forge.game.spellability.LandAbility;
import java.util.List; import java.util.List;
import org.testng.AssertJUnit; import org.testng.AssertJUnit;
@@ -79,16 +80,44 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest {
SpellAbilityPicker picker = new SpellAbilityPicker(game, p); SpellAbilityPicker picker = new SpellAbilityPicker(game, p);
SpellAbility sa = picker.chooseSpellAbilityToPlay(null); SpellAbility sa = picker.chooseSpellAbilityToPlay(null);
// assertEquals(game.PLAY_LAND_SURROGATE, sa); AssertJUnit.assertTrue(sa instanceof LandAbility);
AssertJUnit.assertEquals(mountain, sa.getHostCard()); AssertJUnit.assertEquals(mountain, sa.getHostCard());
Plan plan = picker.getPlan(); Plan plan = picker.getPlan();
AssertJUnit.assertEquals(2, plan.getDecisions().size()); AssertJUnit.assertEquals(2, plan.getDecisions().size());
AssertJUnit.assertEquals("Play land Mountain", plan.getDecisions().get(0).saRef.toString()); AssertJUnit.assertEquals("Mountain (1) -> Play land", plan.getDecisions().get(0).saRef.toString(true));
AssertJUnit.assertEquals("Shock deals 2 damage to any target.", plan.getDecisions().get(1).saRef.toString()); AssertJUnit.assertEquals("Shock deals 2 damage to any target.", plan.getDecisions().get(1).saRef.toString());
AssertJUnit.assertTrue(plan.getDecisions().get(1).targets.toString().contains("Runeclaw Bear")); AssertJUnit.assertTrue(plan.getDecisions().get(1).targets.toString().contains("Runeclaw Bear"));
} }
@Test
public void testPlayingLandAfterSpell() {
Game game = initAndCreateGame();
Player p = game.getPlayers().get(1);
addCard("Island", p);
addCard("Island", p);
addCard("Forest", p);
addCard("Forest", p);
addCard("Forest", p);
Card tatyova = addCardToZone("Tatyova, Benthic Druid", p, ZoneType.Hand);
addCardToZone("Forest", p, ZoneType.Hand);
addCardToZone("Forest", p, ZoneType.Library);
game.getPhaseHandler().devModeSet(PhaseType.MAIN1, p);
game.getAction().checkStateEffects(true);
SpellAbilityPicker picker = new SpellAbilityPicker(game, p);
SpellAbility sa = picker.chooseSpellAbilityToPlay(null);
AssertJUnit.assertEquals(tatyova, sa.getHostCard());
Plan plan = picker.getPlan();
AssertJUnit.assertEquals(2, plan.getDecisions().size());
AssertJUnit.assertEquals("Tatyova, Benthic Druid - Creature 3 / 3", plan.getDecisions().get(0).saRef.toString());
AssertJUnit.assertEquals("Play land", plan.getDecisions().get(1).saRef.toString());
}
@Test @Test
public void testModeSelection() { public void testModeSelection() {
Game game = initAndCreateGame(); Game game = initAndCreateGame();
@@ -279,7 +308,8 @@ public class SpellAbilityPickerSimulationTest extends SimulationTest {
AssertJUnit.assertEquals(abbot.getSpellAbilities().get(0), sa); AssertJUnit.assertEquals(abbot.getSpellAbilities().get(0), sa);
Plan plan = picker.getPlan(); Plan plan = picker.getPlan();
AssertJUnit.assertEquals(3, plan.getDecisions().size()); AssertJUnit.assertEquals(3, plan.getDecisions().size());
AssertJUnit.assertEquals("Play land Mountain", plan.getDecisions().get(1).saRef.toString()); AssertJUnit.assertEquals("Mountain (5) -> Play land by Abbot of Keral Keep (3)",
plan.getDecisions().get(1).saRef.toString(true));
AssertJUnit.assertEquals("Lightning Bolt deals 3 damage to any target.", AssertJUnit.assertEquals("Lightning Bolt deals 3 damage to any target.",
plan.getDecisions().get(2).saRef.toString()); plan.getDecisions().get(2).saRef.toString());
} }