Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved AI Shockland play #6778

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions forge-ai/src/main/java/forge/ai/AiController.java
Original file line number Diff line number Diff line change
Expand Up @@ -542,6 +542,8 @@ private Card chooseBestLandToPlay(CardCollection landList) {
}

// try to skip lands that enter the battlefield tapped if we might want to play something this turn
// TODO figure out a better life level for shocking in lands
boolean check_shocks = player.getLife() > 10;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, good catch! A side effect of coding past midnight 😅

if (!nonLandsInHand.isEmpty()) {
CardCollection nonTappedLands = new CardCollection();
for (Card land : landList) {
Expand All @@ -562,6 +564,14 @@ private Card chooseBestLandToPlay(CardCollection landList) {
if (reSA == null || !ApiType.Tap.equals(reSA.getApi())) {
continue;
}
// check unlesscost parameters. At this point, it should be a tapapi ability
if (reSA.hasParam("UnlessCost")) {
String unlessCost = reSA.getParam("UnlessCost").trim();
Cost cost = AbilityUtils.calculateUnlessCost(reSA, unlessCost, true);
if (SpellApiToAi.Converter.get(reSA).willPayUnlessCost(reSA, player, cost, false, new PlayerCollection(player))) {
continue;
}
}
reSA.setActivatingPlayer(reSA.getHostCard().getController());
if (reSA.metConditions()) {
foundTapped = true;
Expand Down
11 changes: 7 additions & 4 deletions forge-ai/src/main/java/forge/ai/ability/TapAi.java
Original file line number Diff line number Diff line change
Expand Up @@ -105,12 +105,15 @@ public boolean willPayUnlessCost(SpellAbility sa, Player payer, Cost cost, boole
if (landsize != c.getCMC()) {
continue;
}
// Check if the AI intends to play the card and if it can pay for it with the mana it has
boolean willPlay = ComputerUtil.hasReasonToPlayCardThisTurn(payer, c);
boolean canPay = c.getManaCost().canBePaidWithAvailable(ColorSet.fromNames(ComputerUtilCost.getAvailableManaColors(payer, source)).getColor());
if (canPay && willPlay) {
if (payer.getLife() > 10 || landsize < 3) {
return true;
}
// // Check if the AI intends to play the card and if it can pay for it with the mana it has
Copy link
Contributor

@tool4ever tool4ever Jan 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

way too hacky

this logic was working fine in general:
image

if you want to enforce different decisions with other API those should be enhanced instead

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After sleeping on it, I agree that it's a bit too hacky. I think the API that's being called in the original code represents the correct line of thought, but the actual implementation of that API doesn't match up with what I'd think it would. I don't think that the internal canPlaySA stuff should be caring about whether or not the AI wants to play it, just whether it has the ability to. Basically 'can' vs 'should'. I don't know what all is relying on the current can implying should logic there, though. I can try setting up some unit tests to make sure I don't break anything too much in the process. I probably won't be able to work on this much until the weekend.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my experience, those AI ability methods tend to play a bit fast and loose with the responsibilities of "can I play it", "should I play it", and "actually play it" (they handle assigning targets as well for instance). Took me a while to puzzle out which ones were called and when on the couple occasions I've dabbled in them. Maybe I'm not seeing the bigger picture but it'd be nice to have more documentation someday for how they're meant to be used.

// boolean willPlay = ComputerUtil.hasReasonToPlayCardThisTurn(payer, c);
// boolean canPay = c.getManaCost().canBePaidWithAvailable(ColorSet.fromNames(ComputerUtilCost.getAvailableManaColors(payer, source)).getColor());
// if (canPay && willPlay) {
// return true;
// }
}
}
return false;
Expand Down
173 changes: 173 additions & 0 deletions forge-gui-desktop/src/test/java/forge/ai/AITest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
package forge.ai;

import java.util.ArrayList;
import java.util.List;

import com.google.common.collect.Lists;

import forge.GuiDesktop;
import forge.StaticData;
import forge.deck.Deck;
import forge.game.Game;
import forge.game.GameRules;
import forge.game.GameStage;
import forge.game.GameType;
import forge.game.Match;
import forge.game.card.Card;
import forge.game.card.CardCollectionView;
import forge.game.card.CardFactory;
import forge.game.phase.PhaseType;
import forge.game.player.Player;
import forge.game.player.RegisteredPlayer;
import forge.game.spellability.SpellAbility;
import forge.game.zone.ZoneType;
import forge.gui.GuiBase;
import forge.item.IPaperCard;
import forge.item.PaperToken;
import forge.localinstance.properties.ForgePreferences.FPref;
import forge.model.FModel;

public class AITest {
private static boolean initialized = false;

public Game resetGame() {
// need to be done after FModel.initialize, or the Localizer isn't loaded yet
List<RegisteredPlayer> players = Lists.newArrayList();
Deck d1 = new Deck();
players.add(new RegisteredPlayer(d1).setPlayer(new LobbyPlayerAi("p2", null)));
players.add(new RegisteredPlayer(d1).setPlayer(new LobbyPlayerAi("p1", null)));
GameRules rules = new GameRules(GameType.Constructed);
Match match = new Match(rules, players, "Test");
Game game = new Game(players, rules, match);
Player p = game.getPlayers().get(1);
game.setAge(GameStage.Play);
game.getPhaseHandler().devModeSet(PhaseType.MAIN1, p);
game.getPhaseHandler().onStackResolved();
// game.getAction().checkStateEffects(true);

return game;
}

protected Game initAndCreateGame() {
if (!initialized) {
GuiBase.setInterface(new GuiDesktop());
FModel.initialize(null, preferences -> {
preferences.setPref(FPref.LOAD_CARD_SCRIPTS_LAZILY, false);
preferences.setPref(FPref.UI_LANGUAGE, "en-US");
return null;
});
initialized = true;
}

return resetGame();
}

protected int countCardsWithName(Game game, String name) {
int i = 0;
for (Card c : game.getCardsIn(ZoneType.Battlefield)) {
if (c.getName().equals(name)) {
i++;
}
}
return i;
}

protected Card findCardWithName(Game game, String name) {
for (Card c : game.getCardsIn(ZoneType.Battlefield)) {
if (c.getName().equals(name)) {
return c;
}
}
return null;
}

protected SpellAbility findSAWithPrefix(Card c, String prefix) {
return findSAWithPrefix(c.getSpellAbilities(), prefix);
}

protected SpellAbility findSAWithPrefix(Iterable<SpellAbility> abilities, String prefix) {
for (SpellAbility sa : abilities) {
if (sa.getDescription().startsWith(prefix)) {
return sa;
}
}
return null;
}

protected Card createCard(String name, Player p) {
IPaperCard paperCard = FModel.getMagicDb().getCommonCards().getCard(name);
if (paperCard == null) {
StaticData.instance().attemptToLoadCard(name);
paperCard = FModel.getMagicDb().getCommonCards().getCard(name);
}
if (paperCard == null) {
throw new RuntimeException("Card not found: " + name);
}
return Card.fromPaperCard(paperCard, p);
}

protected Card addCardToZone(String name, Player p, ZoneType zone) {
Card c = createCard(name, p);
// card need a new Timestamp otherwise Static Abilities might collide
c.setGameTimestamp(p.getGame().getNextTimestamp());
p.getZone(zone).add(c);
return c;
}

protected Card addCard(String name, Player p) {
return addCardToZone(name, p, ZoneType.Battlefield);
}

protected List<Card> addCards(String name, int count, Player p) {
List<Card> cards = Lists.newArrayList();
for (int i = 0; i < count; i++) {
cards.add(addCard(name, p));
}
return cards;
}

protected Card createToken(String name, Player p) {
PaperToken token = FModel.getMagicDb().getAllTokens().getToken(name);
if (token == null) {
System.out.println("Failed to find token name " + name);
return null;
}
return CardFactory.getCard(token, p, p.getGame());
}

protected List<Card> addTokens(String name, int amount, Player p) {
List<Card> cards = new ArrayList<>();

for(int i = 0; i < amount; i++) {
cards.add(addToken(name, p));
}

return cards;
}

protected Card addToken(String name, Player p) {
Card c = createToken(name, p);
// card need a new Timestamp otherwise Static Abilities might collide
c.setGameTimestamp(p.getGame().getNextTimestamp());
p.getZone(ZoneType.Battlefield).add(c);
return c;
}

protected String gameStateToString(Game game) {
StringBuilder sb = new StringBuilder();
for (ZoneType zone : ZoneType.values()) {
CardCollectionView cards = game.getCardsIn(zone);
if (!cards.isEmpty()) {
sb.append("Zone ").append(zone.name()).append(":\n");
for (Card c : game.getCardsIn(zone)) {
sb.append(" ").append(c);
if (c.isTapped()) {
sb.append(" (T)");
}
sb.append("\n");
}
}
}
return sb.toString();
}
}
Loading
Loading