Skip to content
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

[STX] Implement Ecological Appreciation #7732

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
148 changes: 148 additions & 0 deletions Mage.Sets/src/mage/cards/e/EcologicalAppreciation.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,148 @@
package mage.cards.e;

import mage.MageObject;
import mage.abilities.Ability;
import mage.abilities.effects.OneShotEffect;
import mage.abilities.effects.common.ExileSpellEffect;
import mage.cards.*;
import mage.constants.CardType;
import mage.constants.Outcome;
import mage.constants.Zone;
import mage.filter.FilterCard;
import mage.filter.StaticFilters;
import mage.filter.common.FilterCreatureCard;
import mage.game.Game;
import mage.players.Player;
import mage.target.TargetCard;
import mage.target.common.TargetOpponent;

import java.util.UUID;
import java.util.stream.Collectors;

/**
*
* @author htrajan
*/
public final class EcologicalAppreciation extends CardImpl {

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

// Search your library and graveyard for up to four creature cards with different names that each have mana value X or less and reveal them. An opponent chooses two of those cards. Shuffle the chosen cards into your library and put the rest onto the battlefield. Exile Ecological Appreciation.
this.getSpellAbility().addEffect(new EcologicalAppreciationEffect());
this.getSpellAbility().addEffect(ExileSpellEffect.getInstance());
}

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

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

class EcologicalAppreciationEffect extends OneShotEffect {

EcologicalAppreciationEffect() {
super(Outcome.Benefit);
staticText = "search your library and graveyard for up to four creature cards with different names that each have mana value X or less and reveal them. An opponent chooses two of those cards. Shuffle the chosen cards into your library and put the rest onto the battlefield";
}

private EcologicalAppreciationEffect(final EcologicalAppreciationEffect effect) {
super(effect);
}

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

@Override
public boolean apply(Game game, Ability source) {
Player player = game.getPlayer(source.getControllerId());
if (player == null) {
return false;
}
int xValue = source.getManaCostsToPay().getX();
TargetCard targetCardsInLibrary = new EcologicalAppreciationTarget(Zone.LIBRARY, 4, xValue);
boolean searched = player.choose(Outcome.PutCreatureInPlay, new CardsImpl(player.getLibrary().getCards(game)), targetCardsInLibrary, game);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

can use player.searchLibrary(...) instead if desired -- went with this approach to avoid having to make two kinds of targets for EcologicalAppreciationTarget: one for TargetCardInLibrary and one for TargetCardInGraveyard

Copy link
Member

Choose a reason for hiding this comment

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

Simple xmage hint:

  • If you can use existed classes and methods then use it;
  • If you need to create a new one then make research and ensure that you really need it;
  • Custom classes (card's specific classes) are tech debt and must be skipped as much as possible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

understood 👍

Cards cards = new CardsImpl(targetCardsInLibrary.getTargets());

if (cards.isEmpty()) {
if (searched) {
Copy link
Member

@JayDi85 JayDi85 Apr 11, 2021

Choose a reason for hiding this comment

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

It's a smell code to call shuffle three times. You can store it the status var and use it at the end. Also don't use searched at all -- always shuffle it. Cause cheating possible: user can open choose dialog and disconnect (see #4263) -- so you will get a false here, but he already saw the cards order.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

player.shuffleLibrary(source, game);
}
return false;
}

int remainingCards = 4 - cards.size();
if (remainingCards > 0) {
TargetCard targetCardsInGY = new EcologicalAppreciationTarget(Zone.GRAVEYARD, remainingCards, xValue);
player.choose(Outcome.PutCreatureInPlay, new CardsImpl(player.getGraveyard().getCards(game)), targetCardsInGY, game);
cards.addAll(targetCardsInGY.getTargets());
}

TargetOpponent targetOpponent = new TargetOpponent();
targetOpponent.setNotTarget(true);
player.choose(outcome, targetOpponent, source.getSourceId(), game);
Player opponent = game.getPlayer(targetOpponent.getFirstTarget());
if (opponent == null) {
if (searched) {
player.shuffleLibrary(source, game);
}
return false;
}

TargetCard chosenCards = new TargetCard(2, Zone.ALL, StaticFilters.FILTER_CARD);
chosenCards.setNotTarget(true);
opponent.choose(outcome, cards, chosenCards, game);
Cards toShuffle = new CardsImpl(chosenCards.getTargets().stream()
.map(game::getCard)
Copy link
Member

Choose a reason for hiding this comment

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

Missing filter by Objects::nonNull

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

.collect(Collectors.toList()));

player.putCardsOnTopOfLibrary(toShuffle, game, source, false);
player.shuffleLibrary(source, game);
cards.removeAll(toShuffle);

player.moveCards(cards, Zone.BATTLEFIELD, source, game);

return true;
}
}

class EcologicalAppreciationTarget extends TargetCard {
Copy link
Member

Choose a reason for hiding this comment

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

Do not extends TargetCard -- there are limited number of default target classes, so extends it instead (if you really needs it, but I can't see needs in your use case -- just use TargetCardInYourGraveyard and TargetCardInLibrary with name and cmc predicates).

It's important cause AI uses only known target type and will raise error on the new one (it's not critical in your use case cause it uses existing cards list but in can be critical in another custom targets).

I recommends to use name and cmc predicates to generate dynamic filter and use it in choose dialogs. If you can't then keep that code (it's ok in your use case).

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 removed the EcologicalAppreciationTarget class and just used TargetCardInLibrary and TargetCardInYourGraveyard. I was able to use the cmc predicate to add a filter. However, I wasn't sure how to do the different names predicate, so I extracted that logic to a method and made both targets anon classes overriding canTarget, which seemed to be a good compromise to avoid dupe code. Also, in the GY case, I needed to also block selecting cards that were chosen from library.

Copy link
Member

Choose a reason for hiding this comment

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

Create new filter on apply (filter.copy) and add dynamic cmc and names predicates to it.

Name predicate usage example:

  • filter.add(Predicates.not(new NamePredicate("ignore name")));

Copy link
Member

Choose a reason for hiding this comment

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

Hmm. But I can see the problem with predicates now -- you still allows to select same names in one step, so canTarget is good approach.


private static final FilterCard filter = new FilterCreatureCard("creature cards with different names that each have mana value X or less");

private final int xValue;

EcologicalAppreciationTarget(Zone zone, int maxTargets, int xValue) {
super(0, maxTargets, zone, filter);
this.xValue = xValue;
}

private EcologicalAppreciationTarget(final EcologicalAppreciationTarget target) {
super(target);
this.xValue = target.xValue;
}

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

@Override
public boolean canTarget(UUID playerId, UUID id, Ability source, Game game) {
if (!super.canTarget(playerId, id, source, game)) {
return false;
}
Card card = game.getCard(id);
return card != null && card.getConvertedManaCost() <= xValue &&
this.getTargets().stream()
.map(game::getCard)
.map(MageObject::getName)
.noneMatch(card.getName()::equals);
Copy link
Member

@JayDi85 JayDi85 Apr 11, 2021

Choose a reason for hiding this comment

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

If you need names compare then use CardUtil.haveSameNames all the time. It's important to use it in all code cause face down cards don't have names and can't be equal to another empty name (but java's compare does).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

}
}
1 change: 1 addition & 0 deletions Mage.Sets/src/mage/sets/StrixhavenSchoolOfMages.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ private StrixhavenSchoolOfMages() {
cards.add(new SetCardInfo("Dream Strix", 42, Rarity.RARE, mage.cards.d.DreamStrix.class));
cards.add(new SetCardInfo("Dueling Coach", 15, Rarity.UNCOMMON, mage.cards.d.DuelingCoach.class));
cards.add(new SetCardInfo("Eager First-Year", 16, Rarity.COMMON, mage.cards.e.EagerFirstYear.class));
cards.add(new SetCardInfo("Ecological Appreciation", 128, Rarity.MYTHIC, mage.cards.e.EcologicalAppreciation.class));
cards.add(new SetCardInfo("Elemental Expressionist", 181, Rarity.RARE, mage.cards.e.ElementalExpressionist.class));
cards.add(new SetCardInfo("Elemental Masterpiece", 182, Rarity.COMMON, mage.cards.e.ElementalMasterpiece.class));
cards.add(new SetCardInfo("Elemental Summoning", 183, Rarity.COMMON, mage.cards.e.ElementalSummoning.class));
Expand Down