Failure/foul-magics
1
0
Fork 0
forked from External/mage
Code Pull requests Releases Packages Activity Actions

other: reworked target selection (now it use same logic and methods in all places), fixed AI and selection freezes in some use cases (related to #13606, #11285)

Browse source
This commit is contained in:
Oleg Agafonov 2025-05-07 17:34:36 +04:00
parent 62aa310a4f
commit a53eb66b58
10 changed files with 133 additions and 53 deletions
Download patch file Download diff file Expand all files Collapse all files

19
Mage.Server.Plugins/Mage.Player.AI/src/main/java/mage/player/ai/ComputerPlayer.java
View file

@ -251,6 +251,7 @@ public class ComputerPlayer extends PlayerImpl {
for (Permanent permanent : targets) {
if (target.canTarget(abilityControllerId, permanent.getId(), source, game) && !target.getTargets().contains(permanent.getId())) {
// stop to add targets if not needed and outcome is no advantage for AI player
// TODO: need research and improve - is it good to check target.isChosen(game) instead return true?
if (target.getMinNumberOfTargets() == target.getTargets().size()) {
if (outcome.isGood() && hasOpponent(permanent.getControllerId(), game)) {
return true;
@ -2044,19 +2045,20 @@ public class ComputerPlayer extends PlayerImpl {
// we still use playerId when getting cards even if they don't control the search
List<Card> cardChoices = new ArrayList<>(cards.getCards(target.getFilter(), playerId, source, game));
while (!target.doneChoosing(game)) {
do {
Card card = selectCardTarget(abilityControllerId, cardChoices, outcome, target, source, game);
if (card != null) {
target.addTarget(card.getId(), source, game);
cardChoices.remove(card);
} else {
// We don't have any valid target to choose so stop choosing
return target.getTargets().size() >= target.getMinNumberOfTargets();
return target.isChosen(game);
}
// try to fill as much as possible for good effect (see while end) or half for bad (see if)
if (outcome == Outcome.Neutral && target.getTargets().size() > target.getMinNumberOfTargets() + (target.getMaxNumberOfTargets() - target.getMinNumberOfTargets()) / 2) {
return true;
return target.isChosen(game);
}
}
} while (target.getTargets().size() < target.getMaxNumberOfTargets());
return true;
}
@ -2075,19 +2077,20 @@ public class ComputerPlayer extends PlayerImpl {
}
List<Card> cardChoices = new ArrayList<>(cards.getCards(target.getFilter(), abilityControllerId, source, game));
while (!target.doneChoosing(game)) {
do {
Card card = selectCard(abilityControllerId, cardChoices, outcome, target, game);
if (card != null) {
target.add(card.getId(), game);
cardChoices.remove(card); // selectCard don't remove cards (only on second+ tries)
} else {
// We don't have any valid target to choose so stop choosing
return target.getTargets().size() >= target.getMinNumberOfTargets();
return target.isChosen(game);
}
// try to fill as much as possible for good effect (see while end) or half for bad (see if)
if (outcome == Outcome.Neutral && target.getTargets().size() > target.getMinNumberOfTargets() + (target.getMaxNumberOfTargets() - target.getMinNumberOfTargets()) / 2) {
return true;
return target.isChosen(game);
}
}
} while (target.getTargets().size() < target.getMaxNumberOfTargets());
return true;
}

52
Mage.Tests/src/test/java/org/mage/test/cards/abilities/keywords/SaddleTest.java
View file

@ -15,6 +15,11 @@ import org.mage.test.serverside.base.CardTestPlayerBase;
*/
public class SaddleTest extends CardTestPlayerBase {
/**
* Whenever Quilled Charger attacks while saddled, it gets +1/+2 and gains menace until end of turn.
* <p>
* Saddle 2
*/
private static final String charger = "Quilled Charger";
private static final String bear = "Grizzly Bears";
@ -47,29 +52,35 @@ public class SaddleTest extends CardTestPlayerBase {
addCard(Zone.BATTLEFIELD, playerA, charger);
addCard(Zone.BATTLEFIELD, playerA, bear);
setChoice(playerA, bear);
// turn 1 - saddle and trigger on attack
activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Saddle");
setChoice(playerA, bear);
attack(1, playerA, charger, playerB);
runCode("on saddle", 1, PhaseStep.POSTCOMBAT_MAIN, playerA, (info, player, game) -> {
assertTapped(bear, true);
assertTapped(charger, true);
assertSaddled(charger, true);
assertAbility(playerA, charger, new MenaceAbility(false), true);
assertLife(playerB, 20 - 4 - 1);
});
setStrictChooseMode(true);
setStopAt(1, PhaseStep.END_TURN);
execute();
assertTapped(bear, true);
assertTapped(charger, true);
assertSaddled(charger, true);
assertAbility(playerA, charger, new MenaceAbility(false), true);
assertLife(playerB, 20 - 4 - 1);
setStopAt(2, PhaseStep.UPKEEP);
execute();
// turn 2 - saddle ends
assertSaddled(charger, false);
}
/**
* Whenever Rambling Possum attacks while saddled, it gains +1/+2 until end of turn. Then you may return any number
* of creatures that saddled it this turn to their owner's hand.
* <p>
* Saddle 1
*/
private static final String possum = "Rambling Possum";
private static final String lion = "Silvercoat Lion";
private static final String elf = "Arbor Elf";
@Test
public void testSaddledThisTurn() {
@ -77,11 +88,11 @@ public class SaddleTest extends CardTestPlayerBase {
addCard(Zone.BATTLEFIELD, playerA, bear);
addCard(Zone.BATTLEFIELD, playerA, lion);
setChoice(playerA, bear);
activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Saddle");
setChoice(playerA, bear); // to saddle cost
attack(1, playerA, possum, playerB);
setChoice(playerA, bear);
setChoice(playerA, bear); // to return
setStrictChooseMode(true);
setStopAt(1, PhaseStep.END_TURN);
@ -100,19 +111,17 @@ public class SaddleTest extends CardTestPlayerBase {
addCard(Zone.BATTLEFIELD, playerA, possum);
addCard(Zone.BATTLEFIELD, playerA, bear);
addCard(Zone.BATTLEFIELD, playerA, lion);
addCard(Zone.BATTLEFIELD, playerA, elf);
setChoice(playerA, bear);
// turn 1 - saddle x2 and trigger on attack
activateAbility(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Saddle");
setStrictChooseMode(true);
setStopAt(1, PhaseStep.PRECOMBAT_MAIN);
execute();
setChoice(playerA, bear + "^" + lion);
attack(1, playerA, possum, playerB);
setChoice(playerA, lion);
setChoice(playerA, elf); // to return (try to choose a wrong creature, so game must not allow to choose it)
setStrictChooseMode(true);
setStopAt(1, PhaseStep.END_TURN);
// TODO: test framework must have tools to check targeting (as workaround try to check it by look at test command error)
try {
execute();
@ -123,7 +132,8 @@ public class SaddleTest extends CardTestPlayerBase {
}
assertTapped(bear, true);
assertTapped(lion, false);
assertTapped(lion, true);
assertTapped(elf, false);
assertTapped(possum, true);
assertSaddled(possum, true);
}

2
Mage.Tests/src/test/java/org/mage/test/cards/abilities/oneshot/exile/SearchNameExileTests.java
View file

@ -136,6 +136,8 @@ public class SearchNameExileTests extends CardTestPlayerBase {
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerB, "fused Ready // Willing");
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Test of Talents", "Ready // Willing", "Ready // Willing");
// TODO: a non strict cause a good AI choice test - make strict and duplicate as really AI test?
// in non strict mode AI must choose as much as possible in good "up to" target and half in bad target
setStrictChooseMode(false);
setStopAt(1, PhaseStep.BEGIN_COMBAT);
execute();

2
Mage.Tests/src/test/java/org/mage/test/cards/triggers/SacredGroundTest.java
View file

@ -144,7 +144,9 @@ public class SacredGroundTest extends CardTestPlayerBase {
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Molten Rain", "Caves of Koilos");
waitStackResolved(1, PhaseStep.PRECOMBAT_MAIN, 1);
castSpell(1, PhaseStep.PRECOMBAT_MAIN, playerA, "Surgical Extraction", "Caves of Koilos");
setChoice(playerA, true); // Pay 2 life instead of {B}
//setStrictChooseMode(true); TODO: good example of AI choices, so add new AI test instead non strict here
setStopAt(1, PhaseStep.BEGIN_COMBAT);
execute();

7
Mage.Tests/src/test/java/org/mage/test/player/TestPlayer.java
View file

@ -2335,12 +2335,17 @@ public class TestPlayer implements Player {
}
}
if (!targetFound) {
//failOnLastBadChoice(game, source, target, choiceRecord, "unknown or can't target");
}
try {
if (target.isChosen(game)) {
return true;
} else {
// TODO: move check above and fix all fail tests (not after target.isChosen)
if (!targetFound) {
failOnLastBadChoice(game, source, target, choiceRecord, "unknown or can't target");
failOnLastBadChoice(game, source, target, choiceRecord, "selected, but not all required targets");
}
}
} finally {

13
Mage/src/main/java/mage/target/Target.java
View file

@ -7,6 +7,7 @@ import mage.constants.Zone;
import mage.filter.Filter;
import mage.game.Game;
import mage.players.Player;
import mage.util.Copyable;
import java.io.Serializable;
import java.util.Collection;
@ -17,14 +18,23 @@ import java.util.UUID;
/**
* @author BetaSteward_at_googlemail.com
*/
public interface Target extends Serializable {
public interface Target extends Copyable<Target>, Serializable {
/**
* All targets selected by a player
* <p>
* Warning, for "up to" targets it will return true all the time, so make sure your dialog
* use do-while logic and call "choose" one time min or use doneChoosing
*/
boolean isChosen(Game game);
// TODO: combine code or research usages (doneChoosing must be in while cycles, isChosen in other places, see #13606)
boolean doneChoosing(Game game);
void clearChosen();
boolean isChoiceSelected();
boolean isNotTarget();
/**
@ -161,6 +171,7 @@ public interface Target extends Serializable {
UUID getFirstTarget();
@Override
Target copy();
// some targets are chosen from players that are not the controller of the ability (e.g. Pandemonium)

7
Mage/src/main/java/mage/target/TargetAmount.java
View file

@ -109,17 +109,20 @@ public abstract class TargetAmount extends TargetImpl {
if (!amountWasSet) {
setAmount(source, game);
}
chosen = isChosen(game);
chosen = false;
while (remainingAmount > 0) {
if (!player.canRespond()) {
chosen = isChosen(game);
return chosen;
}
if (!getTargetController(game, playerId).chooseTargetAmount(outcome, this, source, game)) {
chosen = isChosen(game);
return chosen;
}
chosen = isChosen(game);
}
return chosen;
return isChosen(game);
}
@Override

62
Mage/src/main/java/mage/target/TargetImpl.java
View file

@ -31,7 +31,16 @@ public abstract class TargetImpl implements Target {
protected int minNumberOfTargets;
protected boolean required = true;
protected boolean requiredExplicitlySet = false;
/**
* Simple chosen state due selected and require targets count
* Complex targets must override isChosen method or set chosen value manually
* For "up to" targets it will be false before first choose dialog:
* - chosen = false - player do not make a choice yet
* - chosen = true, targets.size = 0 - player choose 0 targets in "up to" and it's valid
* - chosen = true, targets.size >= 1 - player choose some targets and it's valid
*/
protected boolean chosen = false;
// is the target handled as targeted spell/ability (notTarget = true is used for not targeted effects like e.g. sacrifice)
protected boolean notTarget = false;
protected boolean atRandom = false; // for inner choose logic
@ -229,15 +238,28 @@ public abstract class TargetImpl implements Target {
@Override
public boolean isChosen(Game game) {
// min = max = 0 - for abilities with X=0, e.g. nothing to choose
if (getMaxNumberOfTargets() == 0 && getMinNumberOfTargets() == 0) {
return true;
}
return getMaxNumberOfTargets() != 0 && targets.size() == getMaxNumberOfTargets() || chosen;
// limit by max amount
if (getMaxNumberOfTargets() > 0 && targets.size() > getMaxNumberOfTargets()) {
return chosen;
}
// limit by min amount
if (getMinNumberOfTargets() > 0 && targets.size() < getMinNumberOfTargets()) {
return chosen;
}
// all fine
return chosen || (targets.size() >= getMinNumberOfTargets() && targets.size() <= getMaxNumberOfTargets());
}
@Override
public boolean doneChoosing(Game game) {
return getMaxNumberOfTargets() != 0 && targets.size() == getMaxNumberOfTargets();
return isChoiceSelected() && isChosen(game);
}
@Override
@ -247,13 +269,19 @@ public abstract class TargetImpl implements Target {
chosen = false;
}
@Override
public boolean isChoiceSelected() {
// min = max = 0 - for abilities with X=0, e.g. nothing to choose
return chosen || getMaxNumberOfTargets() == 0 && getMinNumberOfTargets() == 0;
}
@Override
public void add(UUID id, Game game) {
if (getMaxNumberOfTargets() == 0 || targets.size() < getMaxNumberOfTargets()) {
if (!targets.containsKey(id)) {
targets.put(id, 0);
rememberZoneChangeCounter(id, game);
chosen = targets.size() >= getMinNumberOfTargets();
chosen = isChosen(game);
}
}
}
@ -280,7 +308,7 @@ public abstract class TargetImpl implements Target {
if (!game.replaceEvent(new TargetEvent(id, source))) {
targets.put(id, 0);
rememberZoneChangeCounter(id, game);
chosen = targets.size() >= getMinNumberOfTargets();
chosen = isChosen(game);
if (!skipEvent && shouldReportEvents) {
game.addSimultaneousEvent(GameEvent.getEvent(GameEvent.EventType.TARGETED, id, source, source.getControllerId()));
}
@ -318,14 +346,16 @@ public abstract class TargetImpl implements Target {
if (!game.replaceEvent(GameEvent.getEvent(GameEvent.EventType.TARGET, id, source, source.getControllerId()))) {
targets.put(id, amount);
rememberZoneChangeCounter(id, game);
chosen = targets.size() >= getMinNumberOfTargets();
chosen = isChosen(game);
if (!skipEvent && shouldReportEvents) {
game.fireEvent(GameEvent.getEvent(GameEvent.EventType.TARGETED, id, source, source.getControllerId()));
}
}
} else {
// AI targets simulation
targets.put(id, amount);
rememberZoneChangeCounter(id, game);
chosen = isChosen(game);
}
}
@ -336,17 +366,20 @@ public abstract class TargetImpl implements Target {
return false;
}
chosen = targets.size() >= getMinNumberOfTargets();
chosen = false;
do {
if (!targetController.canRespond()) {
chosen = isChosen(game);
return chosen;
}
if (!targetController.choose(outcome, this, source, game)) {
chosen = isChosen(game);
return chosen;
}
chosen = targets.size() >= getMinNumberOfTargets();
} while (!isChosen(game) && !doneChoosing(game));
return chosen;
chosen = isChosen(game);
} while (!doneChoosing(game));
return isChosen(game);
}
@Override
@ -358,13 +391,15 @@ public abstract class TargetImpl implements Target {
List<UUID> possibleTargets = new ArrayList<>(possibleTargets(playerId, source, game));
chosen = targets.size() >= getMinNumberOfTargets();
chosen = false;
do {
if (!targetController.canRespond()) {
chosen = isChosen(game);
return chosen;
}
if (isRandom()) {
if (possibleTargets.isEmpty()) {
chosen = isChosen(game);
return chosen;
}
// find valid target
@ -384,13 +419,14 @@ public abstract class TargetImpl implements Target {
if (autoChosenId != null) {
addTarget(autoChosenId, source, game);
} else if (!targetController.chooseTarget(outcome, this, source, game)) { // If couldn't autochoose ask player
chosen = isChosen(game);
return chosen;
}
}
chosen = targets.size() >= getMinNumberOfTargets();
} while (!isChosen(game) && !doneChoosing(game));
chosen = isChosen(game);
} while (!doneChoosing(game));
return chosen;
return isChosen(game);
}
@Override

11
Mage/src/main/java/mage/target/Targets.java
View file

@ -4,6 +4,7 @@ import mage.abilities.Ability;
import mage.constants.Outcome;
import mage.game.Game;
import mage.game.events.GameEvent;
import mage.players.Player;
import mage.util.Copyable;
import java.util.*;
@ -38,7 +39,11 @@ public class Targets extends ArrayList<Target> implements Copyable<Targets> {
}
public List<Target> getUnchosen(Game game) {
return stream().filter(target -> !target.isChosen(game)).collect(Collectors.toList());
return stream().filter(target -> !target.isChoiceSelected()).collect(Collectors.toList());
}
public boolean doneChoosing(Game game) {
return stream().allMatch(t -> t.doneChoosing(game));
}
public void clearChosen() {
@ -67,7 +72,7 @@ public class Targets extends ArrayList<Target> implements Copyable<Targets> {
if (!target.choose(outcome, playerId, sourceId, source, game)) {
return false;
}
}
} while (!doneChoosing(game));
}
return true;
}
@ -112,7 +117,7 @@ public class Targets extends ArrayList<Target> implements Copyable<Targets> {
//game.restoreState(state, "Targets");
clearChosen();
}
}
} while (!doneChoosing(game));
}
return true;
}

11
Mage/src/main/java/mage/target/common/TargetCardInLibrary.java
View file

@ -76,17 +76,20 @@ public class TargetCardInLibrary extends TargetCard {
Cards cardsId = new CardsImpl();
cards.forEach(cardsId::add);
chosen = targets.size() >= getMinNumberOfTargets();
chosen = false;
do {
if (!player.canRespond()) {
chosen = isChosen(game);
return chosen;
}
if (!player.chooseTarget(outcome, cardsId, this, source, game)) {
chosen = isChosen(game);
return chosen;
}
chosen = targets.size() >= getMinNumberOfTargets();
} while (!isChosen(game) && !doneChoosing(game));
return chosen;
chosen = isChosen(game);
} while (!doneChoosing(game));
return isChosen(game);
}
@Override