-
Notifications
You must be signed in to change notification settings - Fork 818
[SLD] Implement The Fifteenth Doctor #13741
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?
[SLD] Implement The Fifteenth Doctor #13741
Conversation
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.
Card logic is fine, require only few small fixes
return controller.moveCards(card, Zone.HAND, source, game); | ||
} | ||
} | ||
return false; |
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.
Must return true here cause milled in any use cases.
if (controller.choose(outcome, cards, target, source, game)) { | ||
Card card = game.getCard(target.getFirstTarget()); | ||
if (card != null) { | ||
return controller.moveCards(card, Zone.HAND, source, game); |
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.
Do not return here, see below return. Apply logic - return true id effect did anything to the game state and return false if it doesn’t (e.g. in wrong conditions or empty result).
String message = "Put " + filter.getMessage() + | ||
" from among the milled cards into your hand?"; | ||
if (cards.isEmpty() | ||
|| !controller.chooseUse(outcome, message, source, game)) { |
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.
Use target 0, 1 with single dialog instead use + choose with two dialogs for better UX
|
||
@Override | ||
public boolean apply(ObjectSourcePlayer<Card> input, Game game) { | ||
if (input.getObject() != null && !input.getObject().isArtifact(game)) { |
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.
No need in object filter in predicates. Player and object filled all the time. Same for source param (there are possible rare use cases with empty source but it not related to predicates).
new GainAbilityControlledSpellsEffect(new ImproviseAbility(), filter) | ||
.setText("The first nonartifact spell you cast each turn has improvise. " + | ||
"<i>(Your artifacts can help cast that spell. Each artifact you " + | ||
"tap after you’re done activating mana abilities pays for {1}.)")), |
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.
Error: (rules) card's rules contains restricted symbol ’ for SLD - The Fifteenth Doctor - 1584
Implements The Fifteenth Doctor