Failure/foul-magics
1
0
Fork 0
forked from External/mage
Code Pull requests Releases Packages Activity Actions

server: improved server stability (#11285) and reworked triggers/playable logic (#8426):

Browse source 
* game: now all playable calculations done in game simulation, outside real game (no more freeze and ruined games by wrong Nyxbloom Ancient and other cards with wrong replacement dialog);
* game: fixed multiple problems with triggers (wrong order, duplicated calls or "too many mana" bugs, see #8426, #12087);
* tests: added data integrity checks for game's triggers (3 enabled and 3 disabled due current game engine logic);
This commit is contained in:
Oleg Agafonov 2024-04-16 23:10:04 +04:00
parent f68e435fc4
commit e8e2f23284
23 changed files with 362 additions and 120 deletions
Download patch file Download diff file Expand all files Collapse all files

184
Mage/src/main/java/mage/abilities/TriggeredAbilities.java
View file

@ -8,56 +8,203 @@ import mage.game.events.NumberOfTriggersEvent;
import mage.game.permanent.Permanent;
import mage.game.stack.Spell;
import mage.util.CardUtil;
import mage.util.Copyable;
import org.apache.log4j.Logger;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.stream.Collectors;
/**
* @author BetaSteward_at_googlemail.com
* <p>
* This class uses ConcurrentHashMap to avoid ConcurrentModificationExceptions.
* See ticket https://github.com/magefree/mage/issues/966 and
* https://github.com/magefree/mage/issues/473
* @author BetaSteward_at_googlemail.com, JayDi85
*/
public class TriggeredAbilities extends ConcurrentHashMap<String, TriggeredAbility> {
public class TriggeredAbilities extends LinkedHashMap<String, TriggeredAbility> implements Copyable<TriggeredAbilities> {
private static final Logger logger = Logger.getLogger(TriggeredAbilities.class);
private final Map<String, List<UUID>> sources = new HashMap<>();
// data integrity check for triggers
// reason: game engine can generate additional events and triggers while checking another one,
// it can generate multiple bugs, freeze, etc, see https://github.com/magefree/mage/issues/8426
// all checks can be catches by existing tests
private boolean enableIntegrityCheck1_MustKeepSameTriggersOrder = true; // good
private boolean enableIntegrityCheck2_MustKeepSameTriggersList = false; // bad, impossible to fix due dynamic triggers gen
private boolean enableIntegrityCheck3_CantStartEventProcessingBeforeFinishPrev = false; // bad, impossible to fix due dynamic triggers gen
private boolean enableIntegrityCheck4_EventMustProcessAllOldTriggers = true; // good
private boolean enableIntegrityCheck5_EventMustProcessInSameOrder = true; // good
private boolean enableIntegrityCheck6_EventMustNotProcessNewTriggers = false; // bad, impossible to fix due dynamic triggers gen
private boolean enableIntegrityLogs = false; // debug only
private boolean processingStarted = false;
private GameEvent.EventType processingStartedEvent = null; // null for game state triggers
private List<TriggeredAbility> processingNeed = new ArrayList<>();
private List<TriggeredAbility> processingDone = new ArrayList<>();
public TriggeredAbilities() {
}
protected TriggeredAbilities(final TriggeredAbilities abilities) {
makeSureNotProcessing(null);
for (Map.Entry<String, TriggeredAbility> entry : abilities.entrySet()) {
this.put(entry.getKey(), entry.getValue().copy());
}
for (Map.Entry<String, List<UUID>> entry : abilities.sources.entrySet()) {
sources.put(entry.getKey(), entry.getValue());
}
this.enableIntegrityCheck1_MustKeepSameTriggersOrder = abilities.enableIntegrityCheck1_MustKeepSameTriggersOrder;
this.enableIntegrityCheck2_MustKeepSameTriggersList = abilities.enableIntegrityCheck2_MustKeepSameTriggersList;
this.enableIntegrityCheck3_CantStartEventProcessingBeforeFinishPrev = abilities.enableIntegrityCheck3_CantStartEventProcessingBeforeFinishPrev;
this.enableIntegrityCheck4_EventMustProcessAllOldTriggers = abilities.enableIntegrityCheck4_EventMustProcessAllOldTriggers;
this.enableIntegrityCheck5_EventMustProcessInSameOrder = abilities.enableIntegrityCheck5_EventMustProcessInSameOrder;
this.enableIntegrityCheck6_EventMustNotProcessNewTriggers = abilities.enableIntegrityCheck6_EventMustNotProcessNewTriggers;
this.enableIntegrityLogs = abilities.enableIntegrityLogs;
this.processingStarted = abilities.processingStarted;
this.processingStartedEvent = abilities.processingStartedEvent;
this.processingNeed = CardUtil.deepCopyObject(abilities.processingNeed);
this.processingDone = CardUtil.deepCopyObject(abilities.processingDone);
// runtime check: triggers order (not required by paper rules, by required by xmage to make same result for all game instances)
if (this.enableIntegrityCheck1_MustKeepSameTriggersOrder) {
if (!Objects.equals(this.values().stream().findFirst().orElse(null) + "",
abilities.values().stream().findFirst().orElse(null) + "")) {
// how-to fix: use LinkedHashMap instead HashMap/ConcurrentHashMap
throw new IllegalStateException("Triggers integrity failed: triggers order changed");
}
}
}
public void checkStateTriggers(Game game) {
for (Iterator<TriggeredAbility> it = this.values().iterator(); it.hasNext(); ) {
TriggeredAbility ability = it.next();
if (ability instanceof StateTriggeredAbility && ((StateTriggeredAbility) ability).canTrigger(game)) {
checkTrigger(ability, null, game);
makeSureNotProcessing(null);
processingStart(null);
boolean needErrorChecksOnEnd = true;
try {
for (Iterator<TriggeredAbility> it = this.values().iterator(); it.hasNext(); ) {
TriggeredAbility ability = it.next();
if (ability instanceof StateTriggeredAbility && ((StateTriggeredAbility) ability).canTrigger(game)) {
checkTrigger(ability, null, game);
}
this.processingDone(ability);
}
} catch (Exception e) {
// need additional catch to show inner errors
needErrorChecksOnEnd = false;
throw e;
} finally {
processingEnd(needErrorChecksOnEnd);
}
}
public void checkTriggers(GameEvent event, Game game) {
for (Iterator<TriggeredAbility> it = this.values().iterator(); it.hasNext(); ) {
TriggeredAbility ability = it.next();
if (ability.checkEventType(event, game)) {
checkTrigger(ability, event, game);
processingStart(event);
boolean needErrorChecksOnEnd = true;
// must keep real object refs (not copies), cause check trigger code can change trigger's and effect's data like targets
ArrayList<TriggeredAbility> currentTriggers = new ArrayList<>(this.values());
try {
for (TriggeredAbility ability : currentTriggers) {
if (ability.checkEventType(event, game)) {
checkTrigger(ability, event, game);
}
this.processingDone(ability);
}
} catch (Exception e) {
// need additional catch to show inner errors
needErrorChecksOnEnd = false;
throw e;
} finally {
processingEnd(needErrorChecksOnEnd);
}
}
private void makeSureNotProcessing(GameEvent newEvent) {
if (this.enableIntegrityCheck2_MustKeepSameTriggersList
&& this.processingStarted) {
List<String> info = new ArrayList<>();
info.add("old event: " + this.processingStartedEvent);
info.add("new event: " + newEvent.getType());
// how-to fix: impossible until mana events/triggers rework cause one mana event can generate additional events/triggers
throw new IllegalArgumentException("Triggers integrity failed: triggers can't be modified while processing - "
+ String.join(", ", info));
}
}
private void processingStart(GameEvent newEvent) {
makeSureNotProcessing(newEvent);
this.processingStarted = true;
this.processingStartedEvent = newEvent == null ? null : newEvent.getType();
this.processingNeed.clear();
this.processingNeed.addAll(this.values());
this.processingDone.clear();
}
private void processingDone(TriggeredAbility trigger) {
this.processingDone.add(trigger);
}
private void processingEnd(boolean needErrorChecks) {
if (needErrorChecks) {
if (this.enableIntegrityCheck3_CantStartEventProcessingBeforeFinishPrev
&& !this.processingStarted) {
throw new IllegalArgumentException("Triggers integrity failed: can't finish event before start");
}
// must use ability's id to check equal (rules can be diff due usage of dynamic values - alternative to card hints)
List<UUID> needIds = new ArrayList<>();
String needInfo = this.processingNeed.stream()
.peek(a -> needIds.add(a.getId()))
.map(t -> "- " + t)
.sorted()
.collect(Collectors.joining("\n"));
List<UUID> doneIds = new ArrayList<>();
String doneInfo = this.processingDone.stream()
.peek(a -> doneIds.add(a.getId()))
.map(t -> "- " + t)
.sorted()
.collect(Collectors.joining("\n"));
String errorInfo = ""
+ "\n" + "Need: "
+ "\n" + (needInfo.isEmpty() ? "-" : needInfo)
+ "\n" + "Done: "
+ "\n" + (doneInfo.isEmpty() ? "-" : doneInfo);
if (this.enableIntegrityCheck4_EventMustProcessAllOldTriggers
&& this.processingDone.size() < this.processingNeed.size()) {
throw new IllegalArgumentException("Triggers integrity failed: event processing miss some triggers" + errorInfo);
}
if (this.enableIntegrityCheck5_EventMustProcessInSameOrder
&& this.processingDone.size() > 0
&& this.processingDone.size() == this.processingNeed.size()
&& !needIds.toString().equals(doneIds.toString())) {
throw new IllegalArgumentException("Triggers integrity failed: event processing used wrong order" + errorInfo);
}
if (this.enableIntegrityCheck6_EventMustNotProcessNewTriggers
&& this.processingDone.size() > this.processingNeed.size()) {
throw new IllegalArgumentException("Triggers integrity failed: event processing must not process new triggers" + errorInfo);
}
}
this.processingStarted = false;
this.processingStartedEvent = null;
this.processingNeed.clear();
this.processingDone.clear();
}
private void checkTrigger(TriggeredAbility ability, GameEvent event, Game game) {
// for effects like when leaves battlefield or destroyed use ShortLKI to check if permanent was in the correct zone before (e.g. Oblivion Ring or Karmic Justice)
if (this.enableIntegrityLogs) {
logger.info("---");
logger.info("checking trigger: " + ability);
logger.info("playable state: " + game.inCheckPlayableState());
logger.info(game);
logger.info("battlefield:" + "\n" + game.getBattlefield().getAllPermanents().stream()
.map(p -> "- " + p.toString())
.collect(Collectors.joining("\n")) + "\n");
}
MageObject object = game.getObject(ability.getSourceId());
if (ability.isInUseableZone(game, object, event)) {
if (event == null || !game.getContinuousEffects().preventedByRuleModification(event, ability, game, false)) {
@ -99,6 +246,9 @@ public class TriggeredAbilities extends ConcurrentHashMap<String, TriggeredAbili
if (event == null || !game.replaceEvent(numberOfTriggersEvent, ability)) {
int numTriggers = ability.getTriggersOnceEachTurn() ? 1 : numberOfTriggersEvent.getAmount();
for (int i = 0; i < numTriggers; i++) {
if (this.enableIntegrityLogs) {
logger.info("trigger will be USED: " + ability);
}
ability.trigger(game, ability.getControllerId(), event);
}
}
@ -115,6 +265,7 @@ public class TriggeredAbilities extends ConcurrentHashMap<String, TriggeredAbili
* @param attachedTo - the object that gained the ability
*/
public void add(TriggeredAbility ability, UUID sourceId, MageObject attachedTo) {
makeSureNotProcessing(null);
if (sourceId == null) {
add(ability, attachedTo);
} else if (attachedTo == null) {
@ -130,6 +281,7 @@ public class TriggeredAbilities extends ConcurrentHashMap<String, TriggeredAbili
}
public void add(TriggeredAbility ability, MageObject attachedTo) {
makeSureNotProcessing(null);
this.put(getKey(ability, attachedTo), ability);
}
@ -162,8 +314,8 @@ public class TriggeredAbilities extends ConcurrentHashMap<String, TriggeredAbili
}
@Override
public TriggeredAbilities copy() {
return new TriggeredAbilities(this);
}
}