From 72891a5badad74f614c9968efdbb842b589474ed Mon Sep 17 00:00:00 2001 From: Oleg Agafonov Date: Wed, 24 Jun 2020 21:11:49 +0400 Subject: [PATCH] Refactor: removed redundant temporary effects list (#6693, #6618) --- .../abilities/effects/ContinuousEffects.java | 107 ++++-------------- .../effects/ContinuousEffectsList.java | 43 ++++--- 2 files changed, 49 insertions(+), 101 deletions(-) diff --git a/Mage/src/main/java/mage/abilities/effects/ContinuousEffects.java b/Mage/src/main/java/mage/abilities/effects/ContinuousEffects.java index 516cee562bb..6e85bc4e22c 100644 --- a/Mage/src/main/java/mage/abilities/effects/ContinuousEffects.java +++ b/Mage/src/main/java/mage/abilities/effects/ContinuousEffects.java @@ -1,9 +1,5 @@ package mage.abilities.effects; -import java.io.Serializable; -import java.util.*; -import java.util.Map.Entry; -import java.util.stream.Collectors; import mage.MageObject; import mage.MageObjectReference; import mage.abilities.Ability; @@ -30,6 +26,11 @@ import mage.target.common.TargetCardInHand; import mage.util.CardUtil; import org.apache.log4j.Logger; +import java.io.Serializable; +import java.util.*; +import java.util.Map.Entry; +import java.util.stream.Collectors; + /** * @author BetaSteward_at_googlemail.com */ @@ -51,27 +52,21 @@ public class ContinuousEffects implements Serializable { private ContinuousEffectsList spliceCardEffects = new ContinuousEffectsList<>(); private final Map> asThoughEffectsMap = new EnumMap<>(AsThoughEffectType.class); - public final List> allEffectsLists = new ArrayList<>(); + public final List> allEffectsLists = new ArrayList<>(); // contains refs to real effect's list private final ApplyCountersEffect applyCounters; - // private final PlaneswalkerRedirectionEffect planeswalkerRedirectionEffect; private final AuraReplacementEffect auraReplacementEffect; - private final Map> lastEffectsListOnLayer = new HashMap<>(); // helps to find out new effect timestamps - - // note all effect/abilities that were only added temporary - private final Map> temporaryEffects = new HashMap<>(); + private final Map> lastEffectsListOnLayer = new HashMap<>(); // helps to find out new effect timestamps on layers public ContinuousEffects() { applyCounters = new ApplyCountersEffect(); -// planeswalkerRedirectionEffect = new PlaneswalkerRedirectionEffect(); auraReplacementEffect = new AuraReplacementEffect(); collectAllEffects(); } public ContinuousEffects(final ContinuousEffects effect) { - this.applyCounters = effect.applyCounters.copy(); -// this.planeswalkerRedirectionEffect = effect.planeswalkerRedirectionEffect.copy(); - this.auraReplacementEffect = effect.auraReplacementEffect.copy(); + applyCounters = effect.applyCounters.copy(); + auraReplacementEffect = effect.auraReplacementEffect.copy(); layeredEffects = effect.layeredEffects.copy(); continuousRuleModifyingEffects = effect.continuousRuleModifyingEffects.copy(); replacementEffects = effect.replacementEffects.copy(); @@ -85,8 +80,8 @@ public class ContinuousEffects implements Serializable { costModificationEffects = effect.costModificationEffects.copy(); spliceCardEffects = effect.spliceCardEffects.copy(); - for (Map.Entry> entry : effect.temporaryEffects.entrySet()) { - temporaryEffects.put(entry.getKey().copy(), entry.getValue()); + for (Map.Entry> entry : effect.lastEffectsListOnLayer.entrySet()) { + lastEffectsListOnLayer.put(entry.getKey(), entry.getValue().copy()); } collectAllEffects(); order = effect.order; @@ -172,7 +167,7 @@ public class ContinuousEffects implements Serializable { * * @param game * @param timestampGroupName workaround to fix broken timestamps on effect's - * add/remove between different layers + * add/remove between different layers * @return effects list ordered by timestamp */ public synchronized List getLayeredEffects(Game game, String timestampGroupName) { @@ -217,7 +212,7 @@ public class ContinuousEffects implements Serializable { * "actual" meaning it becomes turned on that is defined by * Ability.#isInUseableZone(Game, boolean) method in * #getLayeredEffects(Game). - * + *

* It must be called with different timestamp group name (otherwise sort * order will be changed for add/remove effects, see Urborg and Bloodmoon * test) @@ -226,9 +221,9 @@ public class ContinuousEffects implements Serializable { */ private synchronized void updateTimestamps(String timestampGroupName, List layerEffects) { if (!lastEffectsListOnLayer.containsKey(timestampGroupName)) { - lastEffectsListOnLayer.put(timestampGroupName, new ArrayList<>()); + lastEffectsListOnLayer.put(timestampGroupName, new ContinuousEffectsList<>()); } - List prevs = lastEffectsListOnLayer.get(timestampGroupName); + ContinuousEffectsList prevs = lastEffectsListOnLayer.get(timestampGroupName); for (ContinuousEffect continuousEffect : layerEffects) { // check if it's new, then set order if (!prevs.contains(continuousEffect)) { @@ -341,15 +336,12 @@ public class ContinuousEffects implements Serializable { */ private Map> getApplicableReplacementEffects(GameEvent event, Game game) { Map> replaceEffects = new HashMap<>(); -// if (planeswalkerRedirectionEffect.checksEventType(event, game) && planeswalkerRedirectionEffect.applies(event, null, game)) { -// replaceEffects.put(planeswalkerRedirectionEffect, null); -// } if (auraReplacementEffect.checksEventType(event, game) && auraReplacementEffect.applies(event, null, game)) { replaceEffects.put(auraReplacementEffect, null); } // boolean checkLKI = event.getType().equals(EventType.ZONE_CHANGE) || event.getType().equals(EventType.DESTROYED_PERMANENT); //get all applicable transient Replacement effects - for (Iterator iterator = replacementEffects.iterator(); iterator.hasNext();) { + for (Iterator iterator = replacementEffects.iterator(); iterator.hasNext(); ) { ReplacementEffect effect = iterator.next(); if (!effect.checksEventType(event, game)) { continue; @@ -382,7 +374,7 @@ public class ContinuousEffects implements Serializable { } } - for (Iterator iterator = preventionEffects.iterator(); iterator.hasNext();) { + for (Iterator iterator = preventionEffects.iterator(); iterator.hasNext(); ) { PreventionEffect effect = iterator.next(); if (!effect.checksEventType(event, game)) { continue; @@ -758,8 +750,8 @@ public class ContinuousEffects implements Serializable { * @param event * @param targetAbility ability the event is attached to. can be null. * @param game - * @param silentMode true if the event does not really happen but it's - * checked if the event would be replaced + * @param silentMode true if the event does not really happen but it's + * checked if the event would be replaced * @return */ public boolean preventedByRuleModification(GameEvent event, Ability targetAbility, Game game, boolean silentMode) { @@ -807,7 +799,7 @@ public class ContinuousEffects implements Serializable { do { Map> rEffects = getApplicableReplacementEffects(event, game); // Remove all consumed effects (ability dependant) - for (Iterator it1 = rEffects.keySet().iterator(); it1.hasNext();) { + for (Iterator it1 = rEffects.keySet().iterator(); it1.hasNext(); ) { ReplacementEffect entry = it1.next(); if (consumed.containsKey(entry.getId()) /*&& !(entry instanceof CommanderReplacementEffect) */) { // 903.9. Set consumedAbilitiesIds = consumed.get(entry.getId()); @@ -992,7 +984,7 @@ public class ContinuousEffects implements Serializable { .entrySet() .stream() .filter(entry -> dependentTo.contains(entry.getKey().getId()) - && entry.getValue().contains(effect.getId())) + && entry.getValue().contains(effect.getId())) .forEach(entry -> { entry.getValue().remove(effect.getId()); dependentTo.remove(entry.getKey().getId()); @@ -1026,7 +1018,7 @@ public class ContinuousEffects implements Serializable { continue; } // check if waiting effects can be applied now - for (Iterator>> iterator = waitingEffects.entrySet().iterator(); iterator.hasNext();) { + for (Iterator>> iterator = waitingEffects.entrySet().iterator(); iterator.hasNext(); ) { Map.Entry> entry = iterator.next(); if (!appliedEffects.containsAll(entry.getValue())) { // all dependent to effects are applied now so apply the effect itself continue; @@ -1195,17 +1187,6 @@ public class ContinuousEffects implements Serializable { public synchronized void addEffect(ContinuousEffect effect, UUID sourceId, Ability source) { if (!(source instanceof MageSingleton)) { // because MageSingletons may never be removed by removing the temporary effecs they are not added to the temporaryEffects to prevent this effect.setTemporary(true); - Set abilities = temporaryEffects.get(effect); - if (abilities == null) { - abilities = new HashSet<>(); - temporaryEffects.put(effect, abilities); - } else if (abilities.contains(source)) { - // this ability (for the continuous effect) is already added - return; - } - abilities.add(source); - - // add the effect itself } addEffect(effect, source); } @@ -1293,51 +1274,12 @@ public class ContinuousEffects implements Serializable { for (ContinuousEffectsList effectsList : allEffectsLists) { effectsList.clear(); } - temporaryEffects.clear(); } public synchronized void removeAllTemporaryEffects() { - for (Map.Entry> entry : temporaryEffects.entrySet()) { - switch (entry.getKey().getEffectType()) { - case REPLACEMENT: - case REDIRECTION: - replacementEffects.removeEffects(entry.getKey().getId(), entry.getValue()); - break; - case PREVENTION: - preventionEffects.removeEffects(entry.getKey().getId(), entry.getValue()); - break; - case RESTRICTION: - restrictionEffects.removeEffects(entry.getKey().getId(), entry.getValue()); - break; - case RESTRICTION_UNTAP_NOT_MORE_THAN: - restrictionUntapNotMoreThanEffects.removeEffects(entry.getKey().getId(), entry.getValue()); - break; - case REQUIREMENT: - requirementEffects.removeEffects(entry.getKey().getId(), entry.getValue()); - break; - case ASTHOUGH: - AsThoughEffect newAsThoughEffect = (AsThoughEffect) entry.getKey(); - if (!asThoughEffectsMap.containsKey(newAsThoughEffect.getAsThoughEffectType())) { - break; - } - asThoughEffectsMap.get(newAsThoughEffect.getAsThoughEffectType()).removeEffects(entry.getKey().getId(), entry.getValue()); - break; - case COSTMODIFICATION: - costModificationEffects.removeEffects(entry.getKey().getId(), entry.getValue()); - break; - case SPLICE: - spliceCardEffects.removeEffects(entry.getKey().getId(), entry.getValue()); - break; - case CONTINUOUS_RULE_MODIFICATION: - continuousRuleModifyingEffects.removeEffects(entry.getKey().getId(), entry.getValue()); - break; - default: - layeredEffects.removeEffects(entry.getKey().getId(), entry.getValue()); - break; - } - + for (ContinuousEffectsList effectsList : allEffectsLists) { + effectsList.removeTemporaryEffects(); } - temporaryEffects.clear(); } public Map getReplacementEffectsTexts(Map> rEffects, Game game) { @@ -1354,7 +1296,6 @@ public class ContinuousEffects implements Serializable { } } else { if (!(entry.getKey() instanceof AuraReplacementEffect)) { -// && !(entry.getKey() instanceof PlaneswalkerRedirectionEffect)) { logger.error("Replacement effect without ability: " + entry.getKey().toString()); } } diff --git a/Mage/src/main/java/mage/abilities/effects/ContinuousEffectsList.java b/Mage/src/main/java/mage/abilities/effects/ContinuousEffectsList.java index 6dd37673e09..6353df838a9 100644 --- a/Mage/src/main/java/mage/abilities/effects/ContinuousEffectsList.java +++ b/Mage/src/main/java/mage/abilities/effects/ContinuousEffectsList.java @@ -1,6 +1,5 @@ package mage.abilities.effects; -import java.util.*; import mage.MageObject; import mage.abilities.Ability; import mage.abilities.MageSingleton; @@ -11,6 +10,8 @@ import mage.game.Game; import mage.players.Player; import org.apache.log4j.Logger; +import java.util.*; + /** * @param * @author BetaSteward_at_googlemail.com @@ -46,7 +47,7 @@ public class ContinuousEffectsList extends ArrayList public void removeEndOfTurnEffects(Game game) { // calls every turn on cleanup step (only end of turn duration) // rules 514.2 - for (Iterator i = this.iterator(); i.hasNext();) { + for (Iterator i = this.iterator(); i.hasNext(); ) { T entry = i.next(); boolean canRemove = false; switch (entry.getDuration()) { @@ -65,8 +66,7 @@ public class ContinuousEffectsList extends ArrayList } public void removeEndOfCombatEffects() { - - for (Iterator i = this.iterator(); i.hasNext();) { + for (Iterator i = this.iterator(); i.hasNext(); ) { T entry = i.next(); if (entry.getDuration() == Duration.EndOfCombat) { i.remove(); @@ -76,7 +76,7 @@ public class ContinuousEffectsList extends ArrayList } public void removeInactiveEffects(Game game) { - for (Iterator i = this.iterator(); i.hasNext();) { + for (Iterator i = this.iterator(); i.hasNext(); ) { T entry = i.next(); if (isInactive(entry, game)) { i.remove(); @@ -207,20 +207,13 @@ public class ContinuousEffectsList extends ArrayList return effectAbilityMap.getOrDefault(effectId, new HashSet<>()); } - public void removeEffects(UUID effectIdToRemove, Set abilitiesToRemove) { - Set abilities = effectAbilityMap.get(effectIdToRemove); - if (abilitiesToRemove != null && abilities != null) { - abilities.removeIf(ability -> abilitiesToRemove.stream().anyMatch(a -> a.isSameInstance(ability))); - } - if (abilities == null || abilities.isEmpty()) { - for (Iterator iterator = this.iterator(); iterator.hasNext();) { - ContinuousEffect effect = iterator.next(); - if (effect.getId().equals(effectIdToRemove)) { - iterator.remove(); - break; - } + public void removeTemporaryEffects() { + for (Iterator i = this.iterator(); i.hasNext(); ) { + T entry = i.next(); + if (entry.isTemporary()) { + i.remove(); + effectAbilityMap.remove(entry.getId()); } - effectAbilityMap.remove(effectIdToRemove); } } @@ -229,4 +222,18 @@ public class ContinuousEffectsList extends ArrayList super.clear(); effectAbilityMap.clear(); } + + public boolean contains(Effect effect) { + // search by id + for (Iterator iterator = this.iterator(); iterator.hasNext(); ) { + T test = iterator.next(); + if (effect.getId().equals(test.getId())) { + return true; + } + if (effect.equals(test)) { + return true; + } + } + return false; + } }