Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
139 changes: 139 additions & 0 deletions Mage.Sets/src/mage/cards/t/TheFiveDoctors.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
package mage.cards.t;

import mage.MageObject;
import mage.abilities.Ability;
import mage.abilities.condition.common.KickedCondition;
import mage.abilities.effects.OneShotEffect;
import mage.abilities.keyword.KickerAbility;
import mage.abilities.decorator.ConditionalOneShotEffect;
import mage.cards.CardImpl;
import mage.cards.Card;
import mage.cards.Cards;
import mage.cards.CardsImpl;
import mage.cards.CardSetInfo;
import mage.constants.*;
import mage.filter.FilterCard;
import mage.game.Game;
import mage.players.Player;
import mage.target.TargetCard;
import mage.target.common.TargetCardInLibrary;
import mage.target.common.TargetCardInYourGraveyard;
import mage.util.CardUtil;

import java.util.UUID;

/**
* @author padfoothelix
*/
public final class TheFiveDoctors extends CardImpl {

private static final FilterCard filter = new FilterCard("Doctor cards");

static {
filter.add(SubType.DOCTOR.getPredicate());
}

public TheFiveDoctors(UUID ownerId, CardSetInfo setInfo) {
super(ownerId, setInfo, new CardType[]{CardType.SORCERY}, "{5}{G}");


// Kicker {5}
this.addAbility(new KickerAbility("{5}"));

// Search your library and/or graveyard for up to five Doctor cards, reveal them, and put them into your hand. If you search your library this way, shuffle. If this spell was kicked, put those cards onto the battlefield instead of putting them into your hand.
this.getSpellAbility().addEffect(
new TheFiveDoctorsSearchLibraryGraveyardEffect(5, filter)
);
}

private TheFiveDoctors(final TheFiveDoctors card) {
super(card);
}

@Override
public TheFiveDoctors copy() {
return new TheFiveDoctors(this);
}
}

class TheFiveDoctorsSearchLibraryGraveyardEffect extends OneShotEffect {
Copy link
Member

@JayDi85 JayDi85 May 28, 2025

Choose a reason for hiding this comment

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

  1. Use multizones search logic from refactor PutCardFromOneOfTwoZonesOntoBattlefieldEffect #11213 (ask about library, fill cards list, ask to select, move and shuffle)
  2. If you found buggy GUI then report about it — do not try to fix it by target/notTarget or other workarounds. choose and chooseTarget had an important diff — one for non-target choices (color protection do not work) and another for target choices (color protection works) due mtg rules

Copy link
Member

Choose a reason for hiding this comment

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

For info: up to target/choice selection logic in GUI - human can select and unselect objects until their press a done button or their select max amount. There aren’t double dones. Maybe some dialogs calls multiple times.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I hadn't seen that code, thanks for pointing it out. I'm not sure if it can be used for [[The Five Doctors]] though, because if I'm reading this correctly, it doesn't exactly let the player search the library, it just selects cards from it with the getCards. When a player searches his library, he/she should be able to see all the cards in it, correct ? What's more, would triggered abilities that trigger whenever a player searches trigger correctly ?

  2. The change from choose to chooseTarget method is not to work around a bug, it's just that the common method that uses choose is not meant to select more than one card.

  3. Thanks for the precision on the double done issue. I believe this might be a bug in a common class ; I'll try and understand what's going on and I'll report about it if it's necessary.

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, search is important, yes. Then try to use another approach instead custom classes -- use two conditional standard effects and custom text like that:
shot_250528_204011

There are:

  • SearchLibraryGraveyardPutInHandEffect with InvertCondition + KickedCondition
  • SearchLibraryGraveyardPutOntoBattlefieldEffect with KickedCondition

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used code from those methods, but I can't use them directly, because they are coded to find just one card with a specific name. I need up to five cards with a given type. That's why I use a custom class. I didn't find a common class that does the trick.

Copy link
Member

Choose a reason for hiding this comment

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

well, then keep custom class

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have done a little more research and found this about the graveyard search :

  1. it is possible to use the player.choose(outcome, targetcard, source, game)
  2. it is possible to use the player.chooseTarget(outcome, cards, targetcard, source, game)
  3. it is not possible to use the player.choose(outcome, cards, targetcard, source, game) - I believe it is because it returns false if you don't choose all possible targets.

I'm not sure if it's better to use option 1 or 2. Option 2 opens up the graveyard automatically, which is more user-friendly, I suppose. I'm not sure if the target/notTarget is relevant, since we search out of the battlefield anyway, and the targetcard is defined with the boolean notTarget set as true.
Let me know if you think one option is better.

Copy link
Member

Choose a reason for hiding this comment

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

I can’t reproduce your player.choose(cards) bug in gui’s tests dialogs — it’s allow to selects any cards.

I need more details: dialog call code, target setting and cards list for choosing

P.S. target.choose is better way to choose targets - it works same way for any player type

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On this line (114) :

if (controller.chooseTarget(outcome, controller.getGraveyard(), targetGrave, source, game)) {

In you replace chooseTarget with choose, then you can select cards from the graveyard, but then when you click Done, nothing happens (cards are not put in hand or battlefield), unless you have selected the maximum amount of cards.
I don't think this is a bug, I think it's just the way the method is supposed to work. If I understand correctly, it returns false if you interrupt the choosing process.

PS : I used player.choose because it was the method used in SearchLibraryGraveyardPutInHandEffect, but I can try and use target.choose if that's better.


private int cardsToSearch;
private FilterCard filter;

public TheFiveDoctorsSearchLibraryGraveyardEffect(int cardsToSearch, FilterCard filter) {
super(Outcome.Benefit);
this.cardsToSearch = cardsToSearch;
this.filter = filter;
staticText = "search your library and/or graveyard for up to " + CardUtil.numberToText(cardsToSearch) + " " + filter.getMessage() +
", reveal them, and put them into your hand. If you search your library this way, shuffle. " +
"If this spell was kicked, put those cards onto the battlefield instead of putting them into you hand.";
}

protected TheFiveDoctorsSearchLibraryGraveyardEffect(final TheFiveDoctorsSearchLibraryGraveyardEffect effect) {
super(effect);
this.cardsToSearch = effect.cardsToSearch;
this.filter = effect.filter;

}

@Override
public TheFiveDoctorsSearchLibraryGraveyardEffect copy() {
return new TheFiveDoctorsSearchLibraryGraveyardEffect(this);
}

@Override
public boolean apply(Game game, Ability source) {
Player controller = game.getPlayer(source.getControllerId());
MageObject sourceObject = source.getSourceObject(game);
Cards cardsFound = new CardsImpl();
boolean needShuffle = false;
int cardsLeftToSearch = cardsToSearch;
if (controller == null || sourceObject == null) {
return false;
}
if (controller.chooseUse(outcome, "Search your library for up to " + CardUtil.numberToText(cardsToSearch) + " " + filter.getMessage() + '?', source, game)) {
TargetCardInLibrary targetLib = new TargetCardInLibrary(0, cardsToSearch, filter);
targetLib.clearChosen();
Copy link
Member

Choose a reason for hiding this comment

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

No need in clear for new target

if (controller.searchLibrary(targetLib, source, game)) {
if (!targetLib.getTargets().isEmpty()) {
Copy link
Member

Choose a reason for hiding this comment

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

No need for empty check before “for”

for (UUID cardId : targetLib.getTargets()) {
Card card = game.getCard(cardId);
if (card != null) {
cardsFound.add(card);
}
}
}
}
needShuffle = true;
}
cardsLeftToSearch = cardsToSearch - cardsFound.count(filter, game);
if (cardsLeftToSearch > 0 && controller.chooseUse(outcome, "Search your graveyard for up to " + CardUtil.numberToText(cardsLeftToSearch) + " " + filter.getMessage() + '?', source, game)) {
TargetCard targetGrave = new TargetCardInYourGraveyard(0, cardsLeftToSearch, filter, true);
targetGrave.clearChosen();
if (controller.chooseTarget(outcome, controller.getGraveyard(), targetGrave, source, game)) {
if (!targetGrave.getTargets().isEmpty()) {
for (UUID cardId : targetGrave.getTargets()) {
Card card = game.getCard(cardId);
if (card != null) {
cardsFound.add(card);
}
}
}
}
}
if (cardsFound != null) {
controller.revealCards(sourceObject.getIdName(), cardsFound, game);
if (KickedCondition.ONCE.apply(game, source)) {
controller.moveCards(cardsFound, Zone.BATTLEFIELD, source, game);
} else {
controller.moveCards(cardsFound, Zone.HAND, source, game);
}
}
if (needShuffle) {
controller.shuffleLibrary(source, game);
}
return true;
}
}

8 changes: 4 additions & 4 deletions Mage.Sets/src/mage/sets/DoctorWho.java
Original file line number Diff line number Diff line change
Expand Up @@ -927,10 +927,10 @@ private DoctorWho() {
cards.add(new SetCardInfo("The First Doctor", 414, Rarity.RARE, mage.cards.t.TheFirstDoctor.class, NON_FULL_USE_VARIOUS));
cards.add(new SetCardInfo("The First Doctor", 552, Rarity.RARE, mage.cards.t.TheFirstDoctor.class, NON_FULL_USE_VARIOUS));
cards.add(new SetCardInfo("The First Doctor", 733, Rarity.RARE, mage.cards.t.TheFirstDoctor.class, NON_FULL_USE_VARIOUS));
//cards.add(new SetCardInfo("The Five Doctors", 101, Rarity.RARE, mage.cards.t.TheFiveDoctors.class, NON_FULL_USE_VARIOUS));
//cards.add(new SetCardInfo("The Five Doctors", 394, Rarity.RARE, mage.cards.t.TheFiveDoctors.class, NON_FULL_USE_VARIOUS));
//cards.add(new SetCardInfo("The Five Doctors", 706, Rarity.RARE, mage.cards.t.TheFiveDoctors.class, NON_FULL_USE_VARIOUS));
//cards.add(new SetCardInfo("The Five Doctors", 985, Rarity.RARE, mage.cards.t.TheFiveDoctors.class, NON_FULL_USE_VARIOUS));
cards.add(new SetCardInfo("The Five Doctors", 101, Rarity.RARE, mage.cards.t.TheFiveDoctors.class, NON_FULL_USE_VARIOUS));
cards.add(new SetCardInfo("The Five Doctors", 394, Rarity.RARE, mage.cards.t.TheFiveDoctors.class, NON_FULL_USE_VARIOUS));
cards.add(new SetCardInfo("The Five Doctors", 706, Rarity.RARE, mage.cards.t.TheFiveDoctors.class, NON_FULL_USE_VARIOUS));
cards.add(new SetCardInfo("The Five Doctors", 985, Rarity.RARE, mage.cards.t.TheFiveDoctors.class, NON_FULL_USE_VARIOUS));
cards.add(new SetCardInfo("The Flood of Mars", 360, Rarity.RARE, mage.cards.t.TheFloodOfMars.class, NON_FULL_USE_VARIOUS));
cards.add(new SetCardInfo("The Flood of Mars", 45, Rarity.RARE, mage.cards.t.TheFloodOfMars.class, NON_FULL_USE_VARIOUS));
cards.add(new SetCardInfo("The Flood of Mars", 650, Rarity.RARE, mage.cards.t.TheFloodOfMars.class, NON_FULL_USE_VARIOUS));
Expand Down
Loading