Cleanup: GainAbilityControlledSpellsEffect (#10446)

* Hide reminder text on Zhulodok

* Use logic from GainAbilitySpellsEffect, fix so that CastFromZonePredicate works

* Text adjustments

* Show cascade ability in hand for Abaddon the Despoiler

* Remove redundant class

* Simplify Cast Through Time

* Don't add additional instances of redundant abilities

* Remove redundant check

* Add option to ignore mana validation when checking playable objects

* Fix null errors

* Fix GainAbilityControlledSpellsEffect to apply ability to playable cards rather than owned cards

* Add unit test

* Revert bad workaround code

This reverts commit 17f5be6a79.
This reverts commit 7ebd2f1815.
This reverts commit 00969d1fe7.

* Remove ownership check on exiled cards

* Another test (currently failing)

* ignore test

* fix test: strict choose mode
This commit is contained in:
xenohedron 2023-06-24 01:15:58 -04:00 committed by GitHub
parent 90d35e0543
commit ec4c2e2170
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
22 changed files with 154 additions and 226 deletions

View file

@ -1,83 +0,0 @@
package mage.abilities.effects;
import mage.abilities.Ability;
import mage.cards.Card;
import mage.constants.*;
import mage.filter.FilterObject;
import mage.game.Game;
import mage.game.permanent.Permanent;
import mage.game.stack.StackObject;
import mage.players.Player;
public class GainAbilitySpellsEffect extends ContinuousEffectImpl {
private final Ability ability;
private final FilterObject filter;
public GainAbilitySpellsEffect(Ability ability, FilterObject filter) {
super(Duration.WhileOnBattlefield, Layer.AbilityAddingRemovingEffects_6, SubLayer.NA, Outcome.AddAbility);
this.ability = ability;
this.filter = filter;
staticText = filter.getMessage() + " have " + ability.getRule();
}
private GainAbilitySpellsEffect(final GainAbilitySpellsEffect effect) {
super(effect);
this.ability = effect.ability;
this.filter = effect.filter;
}
@Override
public GainAbilitySpellsEffect copy() {
return new GainAbilitySpellsEffect(this);
}
@Override
public boolean apply(Game game, Ability source) {
Player player = game.getPlayer(source.getControllerId());
Permanent permanent = game.getPermanent(source.getSourceId());
if (player == null || permanent == null) {
return false;
}
for (Card card : game.getExile().getAllCards(game)) {
if (card.isOwnedBy(source.getControllerId()) && filter.match(card, game)) {
game.getState().addOtherAbility(card, ability);
}
}
for (Card card : player.getLibrary().getCards(game)) {
if (filter.match(card, game)) {
game.getState().addOtherAbility(card, ability);
}
}
for (Card card : player.getHand().getCards(game)) {
if (filter.match(card, game)) {
game.getState().addOtherAbility(card, ability);
}
}
for (Card card : player.getGraveyard().getCards(game)) {
if (filter.match(card, game)) {
game.getState().addOtherAbility(card, ability);
}
}
// workaround to gain cost reduction abilities to commanders before cast (make it playable)
game.getCommanderCardsFromCommandZone(player, CommanderCardType.ANY)
.stream()
.filter(card -> filter.match(card, game))
.forEach(card -> {
game.getState().addOtherAbility(card, ability);
});
for (StackObject stackObject : game.getStack()) {
if (!stackObject.isControlledBy(source.getControllerId())) {
continue;
}
Card card = game.getCard(stackObject.getSourceId());
if (card == null || !filter.match(stackObject, game)) {
continue;
}
game.getState().addOtherAbility(card, ability);
}
return true;
}
}

View file

@ -23,10 +23,10 @@ public class GainAbilityControlledSpellsEffect extends ContinuousEffectImpl {
super(Duration.WhileOnBattlefield, Layer.AbilityAddingRemovingEffects_6, SubLayer.NA, Outcome.AddAbility);
this.ability = ability;
this.filter = filter;
staticText = filter.getMessage() + " you cast have " + ability.getRule() + '.';
staticText = filter.getMessage() + " have " + ability.getRule();
}
public GainAbilityControlledSpellsEffect(final GainAbilityControlledSpellsEffect effect) {
private GainAbilityControlledSpellsEffect(final GainAbilityControlledSpellsEffect effect) {
super(effect);
this.ability = effect.ability;
this.filter = effect.filter;
@ -45,9 +45,8 @@ public class GainAbilityControlledSpellsEffect extends ContinuousEffectImpl {
return false;
}
for (Card card : game.getExile().getAllCards(game)) {
if (card.isOwnedBy(source.getControllerId())
&& filter.match(card, game)) {
for (Card card : game.getExile().getAllCardsByRange(game, source.getControllerId())) {
if (filter.match(card, game)) {
game.getState().addOtherAbility(card, ability);
}
}
@ -71,22 +70,19 @@ public class GainAbilityControlledSpellsEffect extends ContinuousEffectImpl {
game.getCommanderCardsFromCommandZone(player, CommanderCardType.ANY)
.stream()
.filter(card -> filter.match(card, game))
.forEach(card -> {
game.getState().addOtherAbility(card, ability);
});
.forEach(card -> game.getState().addOtherAbility(card, ability));
for (StackObject stackObject : game.getStack()) {
// only spells cast, so no copies of spells
if ((stackObject instanceof Spell)
&& !stackObject.isCopy()
&& stackObject.isControlledBy(source.getControllerId())) {
Card card = game.getCard(stackObject.getSourceId());
if (filter.match(card, game)) {
game.getState().addOtherAbility(card, ability);
return true;
}
if (!(stackObject instanceof Spell) || !stackObject.isControlledBy(source.getControllerId())) {
continue;
}
// TODO: Distinguish "you cast" to exclude copies
Card card = game.getCard(stackObject.getSourceId());
if (card == null || !filter.match((Spell) stackObject, game)) {
continue;
}
game.getState().addOtherAbility(card, ability);
}
return false; // TODO: Why is this returning false?
return true;
}
}

View file

@ -1183,8 +1183,6 @@ public class GameState implements Serializable, Copyable<GameState> {
* @param ability
*/
public void addOtherAbility(Card attachedTo, Ability ability) {
checkWrongDynamicAbilityUsage(attachedTo, ability);
addOtherAbility(attachedTo, ability, true);
}
@ -1202,6 +1200,12 @@ public class GameState implements Serializable, Copyable<GameState> {
Ability newAbility;
if (ability instanceof MageSingleton || !copyAbility) {
// Avoid adding another instance of an ability where multiple copies are redundant
if (attachedTo.getAbilities().contains(ability)
|| (getAllOtherAbilities(attachedTo.getId()) != null
&& getAllOtherAbilities(attachedTo.getId()).contains(ability))) {
return;
}
newAbility = ability;
} else {
// must use new id, so you can add multiple instances of the same ability