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

[M3C] Implement Desert Warfare #12749

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
200 changes: 200 additions & 0 deletions Mage.Sets/src/mage/cards/d/DesertWarfare.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
package mage.cards.d;

import mage.abilities.Ability;
import mage.abilities.DelayedTriggeredAbility;
import mage.abilities.TriggeredAbilityImpl;
import mage.abilities.common.BeginningOfCombatTriggeredAbility;
import mage.abilities.common.SacrificePermanentTriggeredAbility;
import mage.abilities.common.delayed.AtTheBeginOfNextEndStepDelayedTriggeredAbility;
import mage.abilities.condition.common.PermanentsOnTheBattlefieldCondition;
import mage.abilities.decorator.ConditionalInterveningIfTriggeredAbility;
import mage.abilities.effects.Effect;
import mage.abilities.effects.OneShotEffect;
import mage.abilities.effects.common.CreateDelayedTriggeredAbilityEffect;
import mage.abilities.effects.common.continuous.GainAbilityTargetEffect;
import mage.abilities.hint.common.DesertsYouControlHint;
import mage.abilities.keyword.HasteAbility;
import mage.cards.Card;
import mage.cards.CardImpl;
import mage.cards.CardSetInfo;
import mage.constants.*;
import mage.filter.FilterCard;
import mage.filter.FilterPermanent;
import mage.filter.common.FilterLandPermanent;
import mage.game.Game;
import mage.game.events.GameEvent;
import mage.game.events.ZoneChangeEvent;
import mage.game.permanent.Permanent;
import mage.game.permanent.token.SandWarriorToken;
import mage.game.permanent.token.Token;
import mage.players.Player;
import mage.target.targetpointer.FixedTarget;
import mage.target.targetpointer.FixedTargets;

import java.util.UUID;

/**
* @author Sidorovich77
*/
public final class DesertWarfare extends CardImpl {

private static final FilterPermanent filter = new FilterPermanent();

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

private static final FilterCard filter1 = new FilterCard();

static {
filter1.add(SubType.DESERT.getPredicate());
}

public DesertWarfare(UUID ownerId, CardSetInfo setInfo) {
super(ownerId, setInfo, new CardType[]{CardType.ENCHANTMENT}, "{3}{G}");

// Whenever you sacrifice a Desert and whenever a Desert card is put into your
// graveyard from your hand or library, put that card onto the battlefield under your control at the beginning of your next end step.

//Based on Seraph, Yuma, Proud Protector
Effect effect = new CreateDelayedTriggeredAbilityEffect((DelayedTriggeredAbility) new AtTheBeginOfNextEndStepDelayedTriggeredAbility(new DesertWarfareReturnEffect(), TargetController.YOU).setTriggerPhrase("Put that card onto the battlefield under your control at the beginning of your next end step.").setRuleVisible(false));
Copy link
Contributor

Choose a reason for hiding this comment

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

Hiding the rule is bad style. I kinda see what you're doing here, simulating the compound trigger condition by two abilities, but it is not a clean approach.

You're already writing a custom trigger, so just go all the way and check SACRIFICED_PERMANENT in it as well, and remove extraneous parameters. Setting the trigger phrase in that class and the staticText in the effect class should be sufficient, without all this other manual text setting.


this.addAbility(new SacrificePermanentTriggeredAbility(Zone.BATTLEFIELD, effect, filter, TargetController.YOU, SetTargetPointer.PERMANENT, false).setTriggerPhrase("Whenever you sacrifice a Desert and whenever a Desert card is put into your graveyard from your hand or library, put that card onto the battlefield under your control at the beginning of your next end step.").setRuleVisible(false));

this.addAbility(new PutCardIntoGraveFromHandLibraryAllTriggeredAbility(Zone.BATTLEFIELD, effect,
false, TargetController.YOU, SetTargetPointer.CARD).setTriggerPhrase("Whenever you sacrifice a Desert and whenever a Desert card is put into your graveyard from your hand or library, put that card onto the battlefield under your control at the beginning of your next end step."));

//At the beginning of combat on your turn, if you control five or more Deserts, create that many 1/1 red, green, and white Sand Warrior creature tokens. They gain haste.
//Based on Palani's Hatcher
this.addAbility(new ConditionalInterveningIfTriggeredAbility(
new BeginningOfCombatTriggeredAbility(new DesertWarfareCreateTokensEffect(), TargetController.YOU, false),
new PermanentsOnTheBattlefieldCondition(filter, ComparisonType.OR_GREATER, 5, true),
"At the beginning of combat on your turn, if you control five or more Deserts, create that many 1/1 red, green, and white Sand Warrior creature tokens. They gain haste.").addHint(DesertsYouControlHint.instance));

}

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

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

}

//Based on PutCardIntoGraveFromAnywhereAllTriggeredAbility
class PutCardIntoGraveFromHandLibraryAllTriggeredAbility extends TriggeredAbilityImpl {

private static FilterCard filterCard = new FilterCard();

static {
filterCard.add(SubType.DESERT.getPredicate());
}

private final SetTargetPointer setTargetPointer;

public PutCardIntoGraveFromHandLibraryAllTriggeredAbility(Zone zone, Effect effect, boolean optional, TargetController targetController, SetTargetPointer setTargetPointer) {
super(zone, effect, optional);
FilterCard filter = filterCard.copy();
Copy link
Contributor

Choose a reason for hiding this comment

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

FilterOwnedCard exists.

This is a custom class, so it should include the card name, and params should just be hardcoded.

this.setTargetPointer = setTargetPointer;
filter.add(targetController.getOwnerPredicate());
Copy link
Contributor

Choose a reason for hiding this comment

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

there is no point in copying the filter and adding a redundant predicate

}

protected PutCardIntoGraveFromHandLibraryAllTriggeredAbility(final PutCardIntoGraveFromHandLibraryAllTriggeredAbility ability) {
super(ability);
filterCard = filterCard.copy();
Copy link
Contributor

Choose a reason for hiding this comment

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

don't copy static member (it should be final anyway)

this.setTargetPointer = ability.setTargetPointer;
}

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

@Override
public boolean checkEventType(GameEvent event, Game game) {
return event.getType() == GameEvent.EventType.ZONE_CHANGE;
}

@Override
public boolean checkTrigger(GameEvent event, Game game) {
Card card = game.getCard(event.getTargetId());
if (((ZoneChangeEvent) event).getToZone() == Zone.GRAVEYARD
Copy link
Contributor

Choose a reason for hiding this comment

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

don't repeat checks in both branches of an if-else, perform the common checks first. return false at everything that doesn't trigger, and only have a single setTargetPointer at the end before returning true

also it's simpler to declare ZoneChangeEvent zEvent = (ZoneChangeEvent) event so you only have to cast once

&& zone.match(game.getState().getZone(getSourceId()))
&& ((ZoneChangeEvent) event).getFromZone().match(Zone.LIBRARY)
&& card != null
&& filterCard.match(card, getControllerId(), this, game)) {
this.getEffects().setTargetPointer(new FixedTarget(card, game));
return true;
} else if (((ZoneChangeEvent) event).getToZone() == Zone.GRAVEYARD
&& zone.match(game.getState().getZone(getSourceId()))
&& ((ZoneChangeEvent) event).getFromZone().match(Zone.HAND)
&& card != null
&& filterCard.match(card, getControllerId(), this, game)) {
this.getEffects().setTargetPointer(new FixedTarget(card, game));
return true;
}
return false;
}
}

class DesertWarfareReturnEffect extends OneShotEffect {
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you actually need a custom class, won't ReturnFromGraveyardToBattlefieldTargetEffect suffice?


DesertWarfareReturnEffect() {
super(Outcome.Benefit);
xenohedron marked this conversation as resolved.
Show resolved Hide resolved
}

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

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

@Override
public boolean apply(Game game, Ability source) {
Player controller = game.getPlayer(source.getControllerId());
Card card = game.getCard(getTargetPointer().getFirst(game, source));
if (controller != null && card != null && game.getState().getZone(card.getId()) == Zone.GRAVEYARD) {
controller.moveCards(card, Zone.BATTLEFIELD, source, game, false, false, false, null);
return true;
}
return false;
}
}

class DesertWarfareCreateTokensEffect extends OneShotEffect {

DesertWarfareCreateTokensEffect() {
super(Outcome.Benefit);
this.staticText = "At the beginning of combat on your turn, if you control five or more Deserts, create that many 1/1 red, green, and white Sand Warrior creature tokens. They gain haste.";
}

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

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

@Override
public boolean apply(Game game, Ability source) {
Player player = game.getPlayer(source.getControllerId());
Permanent permanent = source.getSourcePermanentIfItStillExists(game);
Copy link
Contributor

Choose a reason for hiding this comment

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

why are you checking source permanent existing? it's not necessary for the effect.

if (player == null || permanent == null) {
return false;
}
int deserts = game.getBattlefield().countAll(new FilterLandPermanent(SubType.DESERT, "Deserts"), game.getControllerId(source.getSourceId()), game);
Copy link
Contributor

Choose a reason for hiding this comment

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

can't you just use player.getId() here?

also, "Deserts" does not specify land

Token token = new SandWarriorToken();
token.putOntoBattlefield(deserts, game, source);
game.addEffect(new GainAbilityTargetEffect(HasteAbility.getInstance(), Duration.Custom)
.setTargetPointer(new FixedTargets(token, game)), source);
return true;
}
}
2 changes: 2 additions & 0 deletions Mage.Sets/src/mage/sets/ModernHorizons3Commander.java
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ private ModernHorizons3Commander() {
cards.add(new SetCardInfo("Demolition Field", 335, Rarity.UNCOMMON, mage.cards.d.DemolitionField.class));
cards.add(new SetCardInfo("Desert of the Indomitable", 336, Rarity.COMMON, mage.cards.d.DesertOfTheIndomitable.class));
cards.add(new SetCardInfo("Desert of the Mindful", 337, Rarity.COMMON, mage.cards.d.DesertOfTheMindful.class));
cards.add(new SetCardInfo("Desert Warfare", 64, Rarity.RARE, mage.cards.d.DesertWarfare.class, NON_FULL_USE_VARIOUS));
cards.add(new SetCardInfo("Desert Warfare", 116, Rarity.RARE, mage.cards.d.DesertWarfare.class, NON_FULL_USE_VARIOUS));
cards.add(new SetCardInfo("Disa the Restless", 1, Rarity.MYTHIC, mage.cards.d.DisaTheRestless.class));
cards.add(new SetCardInfo("Dreamroot Cascade", 338, Rarity.RARE, mage.cards.d.DreamrootCascade.class));
cards.add(new SetCardInfo("Dreamstone Hedron", 289, Rarity.UNCOMMON, mage.cards.d.DreamstoneHedron.class));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
package mage.abilities.dynamicvalue.common;

import mage.abilities.Ability;
import mage.abilities.dynamicvalue.DynamicValue;
import mage.abilities.effects.Effect;
import mage.constants.SubType;
import mage.filter.common.FilterControlledPermanent;
import mage.game.Game;

/**
* @author Sidorovich77
*/
public enum DesertsYouControlCount implements DynamicValue {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should not be a common class. You can use existing PermanentsOnBattlefieldCount with a filter.


instance;

@Override
public int calculate(Game game, Ability sourceAbility, Effect effect) {
return game.getBattlefield().count(new FilterControlledPermanent(SubType.DESERT), sourceAbility.getControllerId(), sourceAbility, game);
}

@Override
public DesertsYouControlCount copy() {
return instance;
}

@Override
public String toString() {
return "X";
}

@Override
public String getMessage() {
return "deserts you control";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package mage.abilities.hint.common;

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove these empty files

import mage.abilities.Ability;
import mage.abilities.dynamicvalue.common.DesertsYouControlCount;
import mage.abilities.hint.Hint;
import mage.abilities.hint.ValueHint;
import mage.game.Game;

/**
* @author JayDi85
*/
public enum DesertsYouControlHint implements Hint {
Copy link
Contributor

Choose a reason for hiding this comment

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

This also should not be a common class. Just use ValueConditionHint.


instance;
private static final Hint hint = new ValueHint("Deserts you control", DesertsYouControlCount.instance);

@Override
public String getText(Game game, Ability ability) {
return hint.getText(game, ability);
}

@Override
public Hint copy() {
return instance;
}
}
Loading