Skip to content

Fix AI unalives self with Shanid (#9123) #9496

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.

Sign up for GitHub

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

Open
CTimmerman wants to merge 1 commit into
base: master
Choose a base branch
from
+16 −2
Open

Fix AI unalives self with Shanid (#9123) #9496

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
18 changes: 16 additions & 2 deletions forge-ai/src/main/java/forge/ai/AiController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import forge.game.keyword.Keyword;
import forge.game.mana.ManaCostBeingPaid;
import forge.game.phase.PhaseType;
import forge.game.player.GameLossReason;
import forge.game.player.Player;
import forge.game.player.PlayerActionConfirmMode;
import forge.game.player.PlayerCollection;
Expand Down Expand Up @@ -413,6 +414,15 @@ private static List<SpellAbility> getPlayableCounters(final CardCollection all)
return spellAbility;
}

private boolean wouldLoseWithShanid() {
if (ComputerUtilCard.isNonDisabledCardInPlay(player, "Shanid, Sleepers' Scourge")
&& ((player.getLife() == 1 && player.canLoseLife() && !player.cantLoseForZeroOrLessLife())
|| (player.getCardsIn(ZoneType.Library).size() == 0 && !player.cantLoseCheck(GameLossReason.Milled)))) {
return true;
}
return false;
}

private CardCollection filterLandsToPlay(CardCollection landList) {
final CardCollectionView hand = player.getCardsIn(ZoneType.Hand);
CardCollection nonLandList = CardLists.filter(hand, CardPredicates.NON_LANDS);
Expand Down Expand Up @@ -444,10 +454,11 @@ private CardCollection filterLandsToPlay(CardCollection landList) {
landList = CardLists.filter(landList, c -> {
String name = c.getName();
CardCollectionView battlefield = player.getCardsIn(ZoneType.Battlefield);
if (c.getType().isLegendary() && !name.equals("Flagstones of Trokair")) {
if (battlefield.anyMatch(CardPredicates.nameEquals(name))) {
if (c.getType().isLegendary()) {
if (!name.equals("Flagstones of Trokair") && battlefield.anyMatch(CardPredicates.nameEquals(name))) {
return false;
}
if (wouldLoseWithShanid()) return false;
}

final CardCollectionView hand1 = player.getCardsIn(ZoneType.Hand);
Expand Down Expand Up @@ -987,6 +998,9 @@ public AiPlayDecision canPlaySa(SpellAbility sa) {
}
}
if (sa.isSpell()) {
if (card.getType().isLegendary() && wouldLoseWithShanid()) {
return AiPlayDecision.CurseEffects;
}
return canPlaySpellOrLandBasic(card, sa);
Copy link
Contributor

@tool4ever tool4ever Jan 12, 2026

Choose a reason for hiding this comment

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

you're pretty close to the right util methods already:

forge/forge-ai/src/main/java/forge/ai/AiController.java

Lines 1003 to 1010 in a3b6b15

damage = ComputerUtil.getDamageForPlaying(player, sa);
if (damage >= player.getLife()) {
// TODO even if it doesn't kill AI lower the score
return AiPlayDecision.CurseEffects;
}
}
if (card.isPermanent() && !sa.isMutate()) {
damage += ComputerUtil.getDamageFromETB(player, card);

please modify those instead:
that way you should also be able to leverage findSubAbilityByType and avoid ugly hardcoded logic

Copy link
Contributor Author

@CTimmerman CTimmerman Jan 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

So add lifeloss to getDamageForPlaying or make a new similar function and also for drawing? getLossForDrawing?

Copy link
Contributor

@tool4ever tool4ever Jan 12, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

check both methods, you'll see lifeloss is already handled there

and there is no loss for drawing here, better not mix up too much or you risk making things hard to merge again

Copy link
Contributor Author

@CTimmerman CTimmerman Jan 12, 2026

Choose a reason for hiding this comment

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

You told me not to rebase, and I've only added one commit to an existing MR of mine. Keep your vagueness and DIY. It works for me now and my branch works for my decks. I'm tired of waiting for even approved MRs.

}

Expand Down