Refactored restore state code, added additional error check for mana undo param;

This commit is contained in:
Oleg Agafonov 2020-08-06 22:17:37 +04:00
parent 2eeefd91ef
commit 83f7ae377a
12 changed files with 64 additions and 40 deletions

View file

@ -150,13 +150,16 @@ public class ManaCostsImpl<T extends ManaCost> extends ArrayList<T> implements M
*/
@Override
public boolean payOrRollback(Ability ability, Game game, UUID sourceId, UUID payingPlayerId) {
int bookmark = game.bookmarkState();
handlePhyrexianManaCosts(payingPlayerId, ability, game);
if (pay(ability, game, sourceId, payingPlayerId, false, null)) {
game.removeBookmark(bookmark);
return true;
Player player = game.getPlayer(payingPlayerId);
if (player != null) {
int bookmark = game.bookmarkState();
handlePhyrexianManaCosts(payingPlayerId, ability, game);
if (pay(ability, game, sourceId, payingPlayerId, false, null)) {
game.removeBookmark(bookmark);
return true;
}
player.restoreState(bookmark, ability.getRule(), game);
}
game.restoreState(bookmark, ability.getRule());
return false;
}

View file

@ -108,7 +108,7 @@ public class DoIfCostPaid extends OneShotEffect {
player.resetStoredBookmark(game); // otherwise you can e.g. undo card drawn with Mentor of the Meek
} else {
// Paying cost was cancels so try to undo payment so far
game.restoreState(bookmark, DoIfCostPaid.class.getName());
player.restoreState(bookmark, DoIfCostPaid.class.getName(), game);
if (!otherwiseEffects.isEmpty()) {
for (Effect effect : otherwiseEffects) {
effect.setTargetPointer(this.targetPointer);

View file

@ -60,7 +60,7 @@ public class DoWhenCostPaid extends OneShotEffect {
player.resetStoredBookmark(game);
return true;
}
game.restoreState(bookmark, DoWhenCostPaid.class.getName());
player.restoreState(bookmark, DoWhenCostPaid.class.getName(), game);
return true;
}

View file

@ -103,7 +103,7 @@ class CumulativeUpkeepEffect extends OneShotEffect {
game.fireEvent(new GameEvent(EventType.PAID_CUMULATIVE_UPKEEP, permanent.getId(), permanent.getId(), player.getId(), ageCounter, false));
return true;
} else {
game.restoreState(bookmark, source.getRule());
player.restoreState(bookmark, source.getRule(), game);
}
}
game.fireEvent(new GameEvent(EventType.DIDNT_PAY_CUMULATIVE_UPKEEP, permanent.getId(), permanent.getId(), player.getId(), ageCounter, false));

View file

@ -1,25 +1,17 @@
package mage.abilities.mana;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.UUID;
import mage.Mana;
import mage.abilities.Ability;
import mage.abilities.ActivatedAbilityImpl;
import mage.abilities.costs.Cost;
import mage.abilities.effects.Effect;
import mage.abilities.effects.common.ManaEffect;
import mage.constants.AbilityType;
import mage.constants.AsThoughEffectType;
import mage.constants.ManaType;
import mage.constants.TimingRule;
import mage.constants.Zone;
import mage.constants.*;
import mage.game.Game;
import mage.game.stack.Spell;
import mage.game.stack.StackObject;
import java.util.*;
/**
* @author BetaSteward_at_googlemail.com
*/
@ -134,6 +126,9 @@ public abstract class ActivatedManaAbilityImpl extends ActivatedAbilityImpl impl
* Is it allowed to undo the mana creation. It's e.g. not allowed if some
* game revealing information is related (like reveal the top card of the
* library)
* <p>
* TODO: it helps with single mana activate for mana pool, but will not work while activates on paying for casting
* (e.g. user can cheats to see next draw card)
*
* @return
*/

View file

@ -1,12 +1,6 @@
package mage.cards;
import com.google.common.collect.ImmutableList;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.UUID;
import mage.MageObject;
import mage.MageObjectImpl;
import mage.Mana;
@ -33,6 +27,10 @@ import mage.util.SubTypeList;
import mage.watchers.Watcher;
import org.apache.log4j.Logger;
import java.lang.reflect.Constructor;
import java.lang.reflect.InvocationTargetException;
import java.util.*;
public abstract class CardImpl extends MageObjectImpl implements Card {
private static final long serialVersionUID = 1L;
@ -352,6 +350,24 @@ public abstract class CardImpl extends MageObjectImpl implements Card {
for (Ability subAbility : ability.getSubAbilities()) {
abilities.add(subAbility);
}
// verify check: draw effect can't be rollback after mana usage (example: Chromatic Sphere)
// (player can cheat with cancel button to see next card)
// verify test will catch that errors
if (ability instanceof ActivatedManaAbilityImpl) {
ActivatedManaAbilityImpl manaAbility = (ActivatedManaAbilityImpl) ability;
String rule = manaAbility.getRule().toLowerCase(Locale.ENGLISH);
if (manaAbility.getEffects().stream().anyMatch(e -> e.getOutcome().equals(Outcome.DrawCard))
|| rule.contains("reveal ")
|| rule.contains("draw ")) {
if (manaAbility.isUndoPossible()) {
throw new IllegalArgumentException("Ability contains draw/reveal effect, but isUndoPossible is true. Ability: "
+ ability.getClass().getSimpleName() + "; " + ability.getRule());
}
}
}
}
protected void addAbilities(List<Ability> abilities) {
@ -395,10 +411,10 @@ public abstract class CardImpl extends MageObjectImpl implements Card {
* Dynamic cost modification for card (process only own abilities). Example:
* if it need stack related info (like real targets) then must check two
* states (game.inCheckPlayableState):
*
* <p>
* 1. In playable state it must check all possible use cases (e.g. allow to
* reduce on any available target and modes)
*
* <p>
* 2. In real cast state it must check current use case (e.g. real selected
* targets and modes)
*

View file

@ -699,6 +699,12 @@ public abstract class GameImpl implements Game, Serializable {
return savedStates.size();
}
/**
* Warning, for inner usage only, use player.restoreState as much as possible instead
*
* @param bookmark
* @param context additional information for error message
*/
@Override
public void restoreState(int bookmark, String context) {
if (!simulation && !this.hasEnded()) { // if player left or game is over no undo is possible - this could lead to wrong winner
@ -1247,7 +1253,7 @@ public abstract class GameImpl implements Game, Serializable {
if (player != null) {
int bookmark = player.getStoredBookmark();
if (bookmark != -1) {
restoreState(bookmark, "undo");
player.restoreState(bookmark, "undo", this);
player.setStoredBookmark(-1);
fireUpdatePlayersEvent();
}

View file

@ -451,6 +451,13 @@ public interface Player extends MageItem, Copyable<Player> {
void resetStoredBookmark(Game game);
default void restoreState(int bookmark, String text, Game game) {
game.restoreState(bookmark, text);
if (getStoredBookmark() >= bookmark) {
resetStoredBookmark(game);
}
}
void revealCards(Ability source, Cards cards, Game game);
void revealCards(String titelSuffix, Cards cards, Game game);

View file

@ -1385,13 +1385,6 @@ public abstract class PlayerImpl implements Player, Serializable {
return false;
}
public void restoreState(int bookmark, String text, Game game) {
game.restoreState(bookmark, text);
if (storedBookmark >= bookmark) {
resetStoredBookmark(game);
}
}
@Override
public boolean activateAbility(ActivatedAbility ability, Game game) {
if (ability == null) {

View file

@ -549,7 +549,7 @@ public final class ManaUtil {
}
if (!payed) {
game.restoreState(bookmark, restoreContextName);
player.restoreState(bookmark, restoreContextName, game);
game.fireUpdatePlayersEvent();
} else {
game.removeBookmark(bookmark);