-
Notifications
You must be signed in to change notification settings - Fork 822
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.
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
base: master
Are you sure you want to change the base?
Fix AI unalives self with Shanid (#9123) #9496
Conversation
| if (card.getType().isLegendary() && wouldLoseWithShanid()) { | ||
| return AiPlayDecision.CurseEffects; | ||
| } | ||
| return canPlaySpellOrLandBasic(card, sa); |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
Fixes #9123 by checking for Shanid, Sleepers' Scourge's life loss and card draw before playing legendary lands or casting legendary spells.