Rework drawing cards and associated replacement effects; implement [WHO] River Song (#12700)

* remove unused scoring system code

* add test for Alms Collector replacement effect

* flatten draw cards into single method in PlayerImpl

* remove outdated MageAction framework

* clarify game event for drawing two or more cards

* clarify methods for getting cards from library

* implement [WHO] River Song

* fix error

* adjust library methods

* add lots of test cases for draw replacement effects

* fix #12616

* track cards drawn this way through multi draw replacement as well

* add test for River Song

* remove redundant comment
This commit is contained in:
xenohedron 2024-08-24 01:02:55 -04:00 committed by GitHub
parent 34ae226130
commit 9fcbfdeac6
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
22 changed files with 645 additions and 345 deletions

View file

@ -1,113 +0,0 @@
package mage.actions;
import mage.abilities.Ability;
import mage.actions.impl.MageAction;
import mage.actions.score.ArtificialScoringSystem;
import mage.cards.Card;
import mage.constants.Zone;
import mage.game.Game;
import mage.game.events.DrawCardEvent;
import mage.game.events.DrawCardsEvent;
import mage.game.events.DrewCardEvent;
import mage.game.events.GameEvent;
import mage.players.Player;
import mage.util.CardUtil;
import java.util.ArrayList;
import java.util.List;
import java.util.UUID;
/**
* Action for drawing cards.
*
* @author ayrat
*/
public class MageDrawAction extends MageAction {
private static final int NEGATIVE_VALUE = -1000000;
private final Player player;
private final List<Card> drawnCards;
private final GameEvent originalDrawEvent; // for replace effects
private int amount;
public MageDrawAction(Player player, int amount, GameEvent originalDrawEvent) {
this.player = player;
this.amount = amount;
this.drawnCards = new ArrayList<>();
this.originalDrawEvent = originalDrawEvent;
}
/**
* Draw and set action score.
*
* @param source
* @param game Game context.
* @return Number of cards drawn
*/
@Override
public int doAction(Ability source, Game game) {
int numDrawn = 0;
int score = 0;
GameEvent event = new DrawCardsEvent(this.player.getId(), source, this.originalDrawEvent, this.amount);
// TODO: This needs a better description of how it works. Why "amount < 2"?
if (amount < 2 || !game.replaceEvent(event)) {
amount = event.getAmount();
for (int i = 0; i < amount; i++) {
int value = drawCard(source, this.originalDrawEvent, game);
if (value == NEGATIVE_VALUE) {
continue;
}
numDrawn++;
score += value;
}
if (!player.isTopCardRevealed() && numDrawn > 0) {
game.fireInformEvent(player.getLogName() + " draws " + CardUtil.numberToText(numDrawn, "a") + " card" + (numDrawn > 1 ? "s" : ""));
}
setScore(player, score);
}
return numDrawn;
}
/**
* Draw a card if possible (there is no replacement effect that prevent us
* from drawing). Fire event about card drawn.
*
* @param source
* @param originalDrawEvent original draw event for replacement effects, can be null for normal calls
* @param game
* @return
*/
protected int drawCard(Ability source, GameEvent originalDrawEvent, Game game) {
GameEvent event = new DrawCardEvent(this.player.getId(), source, originalDrawEvent);
if (!game.replaceEvent(event)) {
Card card = player.getLibrary().removeFromTop(game);
if (card != null) {
drawnCards.add(card);
card.moveToZone(Zone.HAND, source, game, false); // if you want to use event.getSourceId() here then thinks x10 times
if (player.isTopCardRevealed()) {
game.fireInformEvent(player.getLogName() + " draws a revealed card (" + card.getLogName() + ')');
}
game.fireEvent(new DrewCardEvent(card.getId(), player.getId(), source, originalDrawEvent));
return ArtificialScoringSystem.inst.getCardScore(card);
}
}
return NEGATIVE_VALUE;
}
/**
* Return a card back to top.
*
* @param game Game context
*/
@Override
public void undoAction(Game game) {
for (int index = drawnCards.size() - 1; index >= 0; index--) {
Card card = drawnCards.get(index);
player.getHand().remove(card);
player.getLibrary().putOnTop(card, game);
}
}
}

View file

@ -1,76 +0,0 @@
package mage.actions.impl;
import mage.abilities.Ability;
import mage.game.Game;
import mage.players.Player;
import java.util.UUID;
/**
* Base class for mage actions.
*
* @author ayratn
*/
public abstract class MageAction {
/**
* {@link Player} we count score for.
*/
private Player scorePlayer;
/**
* Current game score for the player.
*/
private int score = 0;
/**
* Set or change action score.
*
* @param scorePlayer Set player.
* @param score Set score value.
*/
protected void setScore(Player scorePlayer, int score) {
this.scorePlayer = scorePlayer;
this.score = score;
}
/**
* Get game score for the {@link Player}. Value depends on the owner of this
* action. In case player and owner differ, negative value is returned.
*
* @param player
* @return
*/
public int getScore(final Player player) {
if (player == null || scorePlayer == null) {
return 0;
}
if (player.getId().equals(scorePlayer.getId())) {
return score;
} else {
return -score;
}
}
/**
* Execute action.
*
*
* @param source
* @param game Game context.
* @return
*/
public abstract int doAction(Ability source, final Game game);
/**
* Undo action.
*
* @param game Game context
*/
public abstract void undoAction(final Game game);
@Override
public String toString() {
return "";
}
}

View file

@ -1,42 +0,0 @@
package mage.actions.score;
import mage.cards.Card;
import mage.game.Game;
import org.apache.log4j.Logger;
/**
* @author ayratn
*/
public class ArtificialScoringSystem implements ScoringSystem {
public static ArtificialScoringSystem inst;
private static final Logger log = Logger.getLogger(ArtificialScoringSystem.class);
static {
inst = new ArtificialScoringSystem();
log.debug("ArtificialScoringSystem has been instantiated.");
}
/**
* Lose score is lowered in function of the turn and phase when it occurs.
* Encourages AI to win as fast as possible.
*
* @param game
* @return
*/
@Override
public int getLoseGameScore(final Game game) {
if (game.getStep() == null) {
return 0;
}
return ScoringConstants.LOSE_GAME_SCORE + game.getTurnNum() * 2500 + game.getTurnStepType().getIndex() * 200;
}
@Override
public int getCardScore(Card card) {
//TODO: implement
return ScoringConstants.UNKNOWN_CARD_SCORE;
}
}

View file

@ -1,13 +0,0 @@
package mage.actions.score;
/**
* Constants for scoring system.
*
* @author ayratn
*/
public final class ScoringConstants {
public static final int WIN_GAME_SCORE = 100000000;
public static final int LOSE_GAME_SCORE = -WIN_GAME_SCORE;
public static final int UNKNOWN_CARD_SCORE = 300;
}

View file

@ -1,13 +0,0 @@
package mage.actions.score;
import mage.cards.Card;
import mage.game.Game;
/**
* @author ayratn
*/
public interface ScoringSystem {
int getLoseGameScore(final Game game);
int getCardScore(final Card card);
}

View file

@ -11,7 +11,6 @@ import mage.abilities.common.delayed.ReflexiveTriggeredAbility;
import mage.abilities.effects.ContinuousEffect;
import mage.abilities.effects.ContinuousEffects;
import mage.abilities.effects.PreventionEffectData;
import mage.actions.impl.MageAction;
import mage.cards.Card;
import mage.cards.Cards;
import mage.cards.MeldCard;
@ -552,8 +551,6 @@ public interface Game extends MageItem, Serializable, Copyable<Game> {
boolean endTurn(Ability source);
int doAction(Ability source, MageAction action);
//game transaction methods
void saveState(boolean bookmark);

View file

@ -22,7 +22,6 @@ import mage.abilities.effects.keyword.StunCounterEffect;
import mage.abilities.keyword.*;
import mage.abilities.mana.DelayedTriggeredManaAbility;
import mage.abilities.mana.TriggeredManaAbility;
import mage.actions.impl.MageAction;
import mage.cards.*;
import mage.cards.decks.Deck;
import mage.cards.decks.DeckCardInfo;
@ -3772,11 +3771,6 @@ public abstract class GameImpl implements Game {
return true;
}
@Override
public int doAction(Ability source, MageAction action) {
return action.doAction(source, this);
}
@Override
public Date getStartTime() {
if (startTime == null) {

View file

@ -9,6 +9,10 @@ import java.util.UUID;
*/
public class DrawCardEvent extends GameEvent {
private boolean fromBottom = false; // for replacement effects that draw from bottom of library instead
private int cardsDrawn = 0; // for replacement effects to keep track for "cards drawn this way"
public DrawCardEvent(UUID playerId, Ability source, GameEvent originalDrawEvent) {
super(GameEvent.EventType.DRAW_CARD, playerId, null, playerId, 0, false);
@ -22,4 +26,21 @@ public class DrawCardEvent extends GameEvent {
this.addAppliedEffects(originalDrawEvent.getAppliedEffects());
}
}
public void setFromBottom(boolean fromBottom) {
this.fromBottom = fromBottom;
}
public boolean isFromBottom() {
return fromBottom;
}
public void incrementCardsDrawn(int cardsDrawn) {
this.cardsDrawn += cardsDrawn;
}
public int getCardsDrawn() {
return cardsDrawn;
}
}

View file

@ -7,10 +7,12 @@ import java.util.UUID;
/**
* @author JayDi85
*/
public class DrawCardsEvent extends GameEvent {
public class DrawTwoOrMoreCardsEvent extends GameEvent {
public DrawCardsEvent(UUID playerId, Ability source, GameEvent originalDrawEvent, int amount) {
super(GameEvent.EventType.DRAW_CARDS, playerId, null, playerId, amount, false);
private int cardsDrawn = 0; // for replacement effects to keep track for "cards drawn this way"
public DrawTwoOrMoreCardsEvent(UUID playerId, Ability source, GameEvent originalDrawEvent, int amount) {
super(GameEvent.EventType.DRAW_TWO_OR_MORE_CARDS, playerId, null, playerId, amount, false);
// source of draw events must be kept between replacements, example: UnpredictableCycloneTest
this.setSourceId(originalDrawEvent == null
@ -22,4 +24,13 @@ public class DrawCardsEvent extends GameEvent {
this.addAppliedEffects(originalDrawEvent.getAppliedEffects());
}
}
public void incrementCardsDrawn(int cardsDrawn) {
this.cardsDrawn += cardsDrawn;
}
public int getCardsDrawn() {
return cardsDrawn;
}
}

View file

@ -75,7 +75,7 @@ public class GameEvent implements Serializable {
ZONE_CHANGE,
ZONE_CHANGE_GROUP, // between two specific zones only; TODO: rework all usages to ZONE_CHANGE_BATCH instead, see #11895
ZONE_CHANGE_BATCH, // all zone changes that occurred from a single effect
DRAW_CARDS, // event calls for multi draws only (if player draws 2+ cards at once)
DRAW_TWO_OR_MORE_CARDS, // event calls for multi draws only (if player draws 2+ cards at once)
DRAW_CARD, DREW_CARD,
EXPLORE, EXPLORED, // targetId is exploring permanent, playerId is its controller
ECHO_PAID,

View file

@ -47,57 +47,51 @@ public class Library implements Serializable {
}
/**
* Removes the top card of the Library and returns it
*
* @param game
* @return Card
* @see Card
* Draws a card from the top of the library, removing it from the library.
* If library is empty, returns null and sets flag for drawing from an empty library.
*/
public Card drawFromTop(Game game) {
Card card = game.getCard(library.pollFirst());
if (card == null) {
emptyDraw = true;
}
return card;
}
/**
* Draws a card from the bottom of the library, removing it from the library.
* If library is empty, returns null and sets flag for drawing from an empty library.
*/
public Card drawFromBottom(Game game) {
Card card = game.getCard(library.pollLast());
if (card == null) {
emptyDraw = true;
}
return card;
}
/**
* Removes the top card from the Library and returns it (can be null if library is empty).
*/
@Deprecated // recommend refactoring methods that re-order library to not require this explicit removal
public Card removeFromTop(Game game) {
UUID cardId = library.pollFirst();
Card card = game.getCard(cardId);
if (card == null) {
emptyDraw = true;
}
return card;
return game.getCard(library.pollFirst());
}
/**
* Removes the bottom card of the Library and returns it
*
* @param game
* @return Card
* @see Card
*/
public Card removeFromBottom(Game game) {
UUID cardId = library.pollLast();
Card card = game.getCard(cardId);
if (card == null) {
emptyDraw = true;
}
return card;
}
/**
* Returns the top card of the Library without removing it
*
* @param game
* @return Card
* @see Card
* Returns the top card of the Library (can be null if library is empty).
* The card is still in the library, until/unless some zone-handling code moves it
*/
public Card getFromTop(Game game) {
return game.getCard(library.peekFirst());
}
/**
* Returns the bottommost card of the Library without removing it
*
* @param game
* @return Card
* @see Card
* Returns the bottom card of the library (can be null if library is empty)
* The card is still in the library, until/unless some zone-handling code moves it
*/
public Card getFromBottom(Game game) {
return game.getCard(library.pollLast());
return game.getCard(library.peekLast());
}
public void putOnTop(Card card, Game game) {
@ -152,10 +146,7 @@ public class Library implements Serializable {
}
/**
* Returns the cards of the library in a list ordered from top to buttom
*
* @param game
* @return
* Returns the cards of the library in a list ordered from top to bottom
*/
public List<Card> getCards(Game game) {
return library.stream().map(game::getCard).filter(Objects::nonNull).collect(Collectors.toList());
@ -235,9 +226,6 @@ public class Library implements Serializable {
/**
* Tests only -- find card position in library
*
* @param cardId
* @return
*/
public int getCardPosition(UUID cardId) {
UUID[] list = library.toArray(new UUID[0]);

View file

@ -407,25 +407,21 @@ public interface Player extends MageItem, Copyable<Player> {
void shuffleLibrary(Ability source, Game game);
/**
* Draw cards. If you call it in replace events then use method with event.appliedEffects param instead.
* Returns 0 if replacement effect triggers on card draw.
* Draw cards. If you call it in replace events then use method with event param instead (for appliedEffects)
*
* @param num
* @param num cards to draw
* @param source can be null for game default draws (non effects, example: start of the turn)
* @param game
* @return
* @return number of cards drawn, including as a result of replacement effects
*/
int drawCards(int num, Ability source, Game game);
/**
* Draw cards with applied effects, for replaceEvent
* Returns 0 if replacement effect triggers on card draw.
*
* @param num
* @param num cards to draw
* @param source can be null for game default draws (non effects, example: start of the turn)
* @param game
* @param event original draw event in replacement code
* @return
* @return number of cards drawn, including as a result of replacement effects
*/
int drawCards(int num, Ability source, Game game, GameEvent event);

View file

@ -19,7 +19,6 @@ import mage.abilities.effects.common.LoseControlOnOtherPlayersControllerEffect;
import mage.abilities.keyword.*;
import mage.abilities.mana.ActivatedManaAbilityImpl;
import mage.abilities.mana.ManaOptions;
import mage.actions.MageDrawAction;
import mage.cards.*;
import mage.cards.decks.Deck;
import mage.choices.Choice;
@ -82,7 +81,7 @@ public abstract class PlayerImpl implements Player, Serializable {
/**
* During some steps we can't play anything
*/
final static Map<PhaseStep, Step.StepPart> SILENT_PHASES_STEPS = ImmutableMap.<PhaseStep, Step.StepPart>builder().
static final Map<PhaseStep, Step.StepPart> SILENT_PHASES_STEPS = ImmutableMap.<PhaseStep, Step.StepPart>builder().
put(PhaseStep.DECLARE_ATTACKERS, Step.StepPart.PRE).build();
/**
@ -737,15 +736,60 @@ public abstract class PlayerImpl implements Player, Serializable {
@Override
public int drawCards(int num, Ability source, Game game) {
if (num > 0) {
return game.doAction(source, new MageDrawAction(this, num, null));
}
return 0;
return drawCards(num, source, game, null);
}
/*
* 614.11. Some effects replace card draws. These effects are applied even if no cards could be drawn because
* there are no cards in the affected player's library.
* 614.11a. If an effect replaces a draw within a sequence of card draws, all actions required by the replacement
* are completed, if possible, before resuming the sequence.
* 614.11b. If an effect would have a player both draw a card and perform an additional action on that card, and
* the draw is replaced, the additional action is not performed on any cards that are drawn as a result of that
* replacement effect.
*/
@Override
public int drawCards(int num, Ability source, Game game, GameEvent event) {
return game.doAction(source, new MageDrawAction(this, num, event));
if (num == 0) {
return 0;
}
if (num >= 2) {
// Event for replacement effects that only apply when two or more cards are drawn
DrawTwoOrMoreCardsEvent multiDrawEvent = new DrawTwoOrMoreCardsEvent(getId(), source, event, num);
if (game.replaceEvent(multiDrawEvent)) {
return multiDrawEvent.getCardsDrawn();
}
num = multiDrawEvent.getAmount();
}
int numDrawn = 0;
for (int i = 0; i < num; i++) {
DrawCardEvent drawCardEvent = new DrawCardEvent(getId(), source, event);
if (game.replaceEvent(drawCardEvent)) {
numDrawn += drawCardEvent.getCardsDrawn();
continue;
}
Card card = drawCardEvent.isFromBottom() ? getLibrary().drawFromBottom(game) : getLibrary().drawFromTop(game);
if (card != null) {
card.moveToZone(Zone.HAND, source, game, false); // if you want to use event.getSourceId() here then thinks x10 times
if (isTopCardRevealed()) {
game.fireInformEvent(getLogName() + " draws a revealed card (" + card.getLogName() + ')');
}
game.fireEvent(new DrewCardEvent(card.getId(), getId(), source, event));
numDrawn++;
}
}
if (!isTopCardRevealed() && numDrawn > 0) {
game.fireInformEvent(getLogName() + " draws " + CardUtil.numberToText(numDrawn, "a") + " card" + (numDrawn > 1 ? "s" : ""));
}
// if this method was called from a replacement event, pass the number of cards back through
// (uncomment conditions if correct ruling is to only count cards drawn by the same player)
if (event instanceof DrawCardEvent /* && event.getPlayerId().equals(getId()) */ ) {
((DrawCardEvent) event).incrementCardsDrawn(numDrawn);
}
if (event instanceof DrawTwoOrMoreCardsEvent /* && event.getPlayerId().equals(getId()) */ ) {
((DrawTwoOrMoreCardsEvent) event).incrementCardsDrawn(numDrawn);
}
return numDrawn;
}
@Override