From a0cad5fa4870ca32ccc71c601db2d4bcde1e1063 Mon Sep 17 00:00:00 2001 From: kevlahnota Date: Wed, 4 Dec 2024 21:45:05 +0800 Subject: [PATCH] fix FCollection (#6657) --- .../java/forge/ai/AiAttackController.java | 5 +- .../main/java/forge/ai/ability/PlayAi.java | 12 +- .../java/forge/util/collect/FCollection.java | 569 ++++++------------ .../game/ability/effects/PlayEffect.java | 25 +- .../src/test/java/forge/FCollectionTest.java | 7 +- 5 files changed, 203 insertions(+), 415 deletions(-) diff --git a/forge-ai/src/main/java/forge/ai/AiAttackController.java b/forge-ai/src/main/java/forge/ai/AiAttackController.java index 43ac68318ca..97fc566ccb5 100644 --- a/forge-ai/src/main/java/forge/ai/AiAttackController.java +++ b/forge-ai/src/main/java/forge/ai/AiAttackController.java @@ -771,6 +771,7 @@ public class AiAttackController { int currentAttackTax = 0; int trampleDamage = 0; CardCollection remainingBlockers = new CardCollection(blockers); + List removedBlockers = new ArrayList<>(); for (Card attacker : CardLists.getKeyword(blockedAttackers, Keyword.TRAMPLE)) { // TODO might sort by quotient of dmg/cost for best combination Cost tax = CombatUtil.getAttackCost(ai.getGame(), attacker, defendingOpponent); @@ -784,9 +785,11 @@ public class AiAttackController { for (Card blocker : remainingBlockers) { if (CombatUtil.canBlock(attacker, blocker) && damage > 0) { damage -= ComputerUtilCombat.shieldDamage(attacker, blocker); - remainingBlockers.remove(blocker); + removedBlockers.add(blocker); } } + remainingBlockers.removeAll(removedBlockers); + removedBlockers.clear(); if (damage > 0) { trampleDamage += damage; } diff --git a/forge-ai/src/main/java/forge/ai/ability/PlayAi.java b/forge-ai/src/main/java/forge/ai/ability/PlayAi.java index 5e7e9c482ff..90616408537 100644 --- a/forge-ai/src/main/java/forge/ai/ability/PlayAi.java +++ b/forge-ai/src/main/java/forge/ai/ability/PlayAi.java @@ -16,9 +16,9 @@ import forge.game.spellability.*; import forge.game.zone.ZoneType; import forge.util.MyRandom; -import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; public class PlayAi extends SpellAbilityAi { @@ -220,13 +220,9 @@ public class PlayAi extends SpellAbilityAi { if (cards != null & sa.hasParam("ValidSA")) { final String valid[] = sa.getParam("ValidSA").split(","); - final Iterator itr = cards.iterator(); - while (itr.hasNext()) { - final Card c = itr.next(); - if (!Iterables.any(AbilityUtils.getBasicSpellsFromPlayEffect(c, ai), SpellAbilityPredicates.isValid(valid, ai , source, sa))) { - itr.remove(); - } - } + final List invalid = cards.stream().filter(c -> !Iterables.any(AbilityUtils.getBasicSpellsFromPlayEffect(c, ai), SpellAbilityPredicates.isValid(valid, ai, source, sa))).collect(Collectors.toList()); + if (!invalid.isEmpty()) + cards.removeAll(invalid); } // Ensure that if a ValidZone is specified, there's at least something to choose from in that zone. diff --git a/forge-core/src/main/java/forge/util/collect/FCollection.java b/forge-core/src/main/java/forge/util/collect/FCollection.java index d86c2a2b907..4485b7cae5a 100644 --- a/forge-core/src/main/java/forge/util/collect/FCollection.java +++ b/forge-core/src/main/java/forge/util/collect/FCollection.java @@ -1,32 +1,30 @@ package forge.util.collect; import java.io.Serializable; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; import java.util.Comparator; +import java.util.HashSet; import java.util.Iterator; -import java.util.LinkedList; import java.util.List; import java.util.ListIterator; import java.util.NoSuchElementException; import java.util.Set; import java.util.function.Predicate; -import com.google.common.base.Supplier; -import com.google.common.base.Suppliers; import org.apache.commons.lang3.ArrayUtils; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.common.collect.Lists; import com.google.common.collect.Ordering; -import com.google.common.collect.Sets; /** * Collection with unique elements ({@link Set}) that maintains the order in * which the elements are added to it ({@link List}). - *

+ * * This object is serializable if all elements it contains are. * * @param the type of the elements this collection contains. @@ -34,7 +32,6 @@ import com.google.common.collect.Sets; */ public class FCollection implements List, /*Set,*/ FCollectionView, Cloneable, Serializable { private static final long serialVersionUID = -1664555336364294106L; - private final Object lock = new Object(); private static final FCollection EMPTY = new EmptyFCollection<>(); @@ -46,12 +43,12 @@ public class FCollection implements List, /*Set,*/ FCollectionView, /** * The {@link Set} representation of this collection. */ - private final Supplier> set = Suppliers.memoize(Sets::newHashSet); + private final Set set = Collections.synchronizedSet(new HashSet<>()); /** * The {@link List} representation of this collection. */ - private final Supplier> list = Suppliers.memoize(Lists::newLinkedList); + private final List list = Collections.synchronizedList(new ArrayList<>()); /** * Create an empty {@link FCollection}. @@ -62,7 +59,8 @@ public class FCollection implements List, /*Set,*/ FCollectionView, /** * Create an {@link FCollection} containing a single element. * - * @param e the single element the new collection contains. + * @param e + * the single element the new collection contains. */ public FCollection(final T e) { add(e); @@ -72,8 +70,9 @@ public class FCollection implements List, /*Set,*/ FCollectionView, * Create an {@link FCollection} from an array. The order of the elements in * the array is preserved in the new collection. * - * @param c an array, whose elements will be in the collection upon its - * creation. + * @param c + * an array, whose elements will be in the collection upon its + * creation. */ public FCollection(final T[] c) { this.addAll(Arrays.asList(c)); @@ -83,8 +82,9 @@ public class FCollection implements List, /*Set,*/ FCollectionView, * Create an {@link FCollection} from an {@link Iterable}. The order of the * elements in the iterable is preserved in the new collection. * - * @param i an iterable, whose elements will be in the collection upon its - * creation. + * @param i + * an iterable, whose elements will be in the collection upon its + * creation. */ public FCollection(final Iterable i) { this.addAll(i); @@ -93,19 +93,19 @@ public class FCollection implements List, /*Set,*/ FCollectionView, /** * Create an {@link FCollection} from an {@link FCollectionReader}. * - * @param reader a reader used to populate collection + * @param reader + * a reader used to populate collection */ public FCollection(final FCollectionReader reader) { - synchronized (lock) { - reader.readAll(this); - } + reader.readAll(this); } /** * Check whether an {@link Iterable} contains any iterable, silently * returning {@code false} when {@code null} is passed as an argument. * - * @param iterable a card collection. + * @param iterable + * a card collection. */ public static boolean hasElements(final Iterable iterable) { return iterable != null && !Iterables.isEmpty(iterable); @@ -115,8 +115,10 @@ public class FCollection implements List, /*Set,*/ FCollectionView, * Check whether a {@link Collection} contains a particular element, silently * returning {@code false} when {@code null} is passed as the first argument. * - * @param collection a collection. - * @param element a possible element of the collection. + * @param collection + * a collection. + * @param element + * a possible element of the collection. */ public static boolean hasElement(final Collection collection, final T element) { return collection != null && collection.contains(element); @@ -132,12 +134,12 @@ public class FCollection implements List, /*Set,*/ FCollectionView, /** *

This implementation uses the hash code of the backing list.

- *

+ * * {@inheritDoc} */ @Override public int hashCode() { - return list.get().hashCode(); + return list.hashCode(); } /** @@ -147,7 +149,7 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public String toString() { - return list.get().toString(); + return list.toString(); } /** @@ -156,37 +158,35 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public final FCollection clone() { - synchronized (lock) { - return new FCollection<>(list.get()); - } + return new FCollection<>(list); } /** * Get the first object in this {@link FCollection}. * - * @throws NoSuchElementException if the collection is empty. + * @throws NoSuchElementException + * if the collection is empty. */ @Override public T getFirst() { - synchronized (lock) { - if (list.get().isEmpty()) - return null; - return list.get().getFirst(); - } + if (list.isEmpty()) + return null; + return list.get(0); + //return list.getFirst(); } /** * Get the last object in this {@link FCollection}. * - * @throws NoSuchElementException if the collection is empty. + * @throws NoSuchElementException + * if the collection is empty. */ @Override public T getLast() { - synchronized (lock) { - if (list.get().isEmpty()) - return null; - return list.get().getLast(); - } + if (list.isEmpty()) + return null; + return list.get(list.size() - 1); + //return list.getLast(); } /** @@ -194,9 +194,7 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public int size() { - synchronized (lock) { - return set.get().size(); - } + return set.size(); } /** @@ -204,25 +202,24 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public boolean isEmpty() { - return set.get().isEmpty(); + return set.isEmpty(); } public Set asSet() { - return set.get(); + return set; } /** * Check whether this collection contains a particular object. * - * @param o an object. + * @param o + * an object. */ @Override public boolean contains(final Object o) { - synchronized (lock) { - if (o == null) - return false; - return set.get().contains(o); - } + if (o == null) + return false; + return set.contains(o); } /** @@ -230,8 +227,7 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public Iterator iterator() { - return new Itr(); - //return list.get().iterator(); + return list.iterator(); } /** @@ -239,9 +235,7 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public Object[] toArray() { - synchronized (lock) { - return list.get().toArray(); - } + return list.toArray(); } /** @@ -250,58 +244,52 @@ public class FCollection implements List, /*Set,*/ FCollectionView, @Override @SuppressWarnings("hiding") public T[] toArray(final T[] a) { - synchronized (lock) { - return list.get().toArray(a); - } + return list.toArray(a); } /** * Add an element to this collection, if it isn't already present. * - * @param e the object to add. + * @param e + * the object to add. * @return whether the collection changed as a result of this method call. */ @Override public boolean add(final T e) { - synchronized (lock) { - if (e == null) - return false; - if (set.get().add(e)) { - list.get().add(e); - return true; - } + if (e == null) return false; + if (set.add(e)) { + list.add(e); + return true; } + return false; } /** * Remove an element from this collection. * - * @param o the object to remove. + * @param o + * the object to remove. * @return whether the collection changed as a result of this method call. */ @Override public boolean remove(final Object o) { - synchronized (lock) { - if (o == null) - return false; - if (set.get().remove(o)) { - list.get().remove(o); - return true; - } + if (o == null) return false; + if (set.remove(o)) { + list.remove(o); + return true; } + return false; } @Override public boolean removeIf(Predicate filter) { - synchronized (lock) { - if (list.get().removeIf(filter)) { - set.get().removeIf(filter); - return true; - } - return false; + if (list.removeIf(filter)) { + set.removeIf(filter); + return true; } + return false; } /** @@ -309,9 +297,7 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public boolean containsAll(final Collection c) { - synchronized (lock) { - return set.get().containsAll(c); - } + return set.containsAll(c); } /** @@ -326,37 +312,35 @@ public class FCollection implements List, /*Set,*/ FCollectionView, * Add all the elements in the specified {@link Iterator} to this * collection, in the order in which they appear. * - * @param i an iterator. + * @param i + * an iterator. * @return whether this collection changed as a result of this method call. * @see #addAll(Collection) */ public boolean addAll(final Iterable i) { - synchronized (lock) { - boolean changed = false; - if (i == null) - return false; - for (final T e : i) { - changed |= add(e); - } - return changed; + boolean changed = false; + if (i == null) + return false; + for (final T e : i) { + changed |= add(e); } + return changed; } /** * Add all the elements in the specified array to this collection, * respecting the ordering. * - * @param c an array. + * @param c + * an array. * @return whether this collection changed as a result of this method call. */ public boolean addAll(final T[] c) { - synchronized (lock) { - boolean changed = false; - for (final T e : c) { - changed |= add(e); - } - return changed; + boolean changed = false; + for (final T e : c) { + changed |= add(e); } + return changed; } /** @@ -365,24 +349,22 @@ public class FCollection implements List, /*Set,*/ FCollectionView, @SuppressWarnings("unchecked") @Override public boolean addAll(final int index, final Collection c) { - synchronized (lock) { - if (c == null) { - return false; - } - - final List list; - if (c instanceof List) { - list = (List) c; - } else { - list = Lists.newArrayList(c); - } - - boolean changed = false; - for (int i = list.size() - 1; i >= 0; i--) { //must add in reverse order so they show up in the right place - changed |= insert(index, list.get(i)); - } - return changed; + if (c == null) { + return false; } + + final List list; + if (c instanceof List) { + list = (List) c; + } else { + list = Lists.newArrayList(c); + } + + boolean changed = false; + for (int i = list.size() - 1; i >= 0; i--) { //must add in reverse order so they show up in the right place + changed |= insert(index, list.get(i)); + } + return changed; } /** @@ -396,19 +378,18 @@ public class FCollection implements List, /*Set,*/ FCollectionView, /** * Remove all objects appearing in an {@link Iterable}. * - * @param c an iterable. + * @param c + * an iterable. * @return whether this collection changed as a result of this method call. */ public boolean removeAll(final Iterable c) { - synchronized (lock) { - boolean changed = false; - if (c == null) - return false; - for (final Object o : c) { - changed |= remove(o); - } - return changed; + boolean changed = false; + if (c == null) + return false; + for (final Object o : c) { + changed |= remove(o); } + return changed; } /** @@ -416,13 +397,11 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public boolean retainAll(final Collection c) { - synchronized (lock) { - if (set.get().retainAll(c)) { - list.get().retainAll(c); - return true; - } - return false; + if (set.retainAll(c)) { + list.retainAll(c); + return true; } + return false; } /** @@ -430,13 +409,9 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public void clear() { - synchronized (lock) { - if (set.get().isEmpty()) { - return; - } - set.get().clear(); - list.get().clear(); - } + if (set.isEmpty()) { return; } + set.clear(); + list.clear(); } /** @@ -444,9 +419,7 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public T get(final int index) { - synchronized (lock) { - return list.get().get(index); - } + return list.get(index); } /** @@ -456,9 +429,7 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public T set(final int index, final T element) { //assume this isn't called except when changing list order, so don't worry about updating set - synchronized (lock) { - return list.get().set(index, element); - } + return list.set(index, element); } /** @@ -472,29 +443,29 @@ public class FCollection implements List, /*Set,*/ FCollectionView, /** * Helper method to insert an element at a particular index. * - * @param index the index to insert the element at. - * @param element the element to insert. + * @param index + * the index to insert the element at. + * @param element + * the element to insert. * @return whether this collection changed as a result of this method call. */ private boolean insert(int index, final T element) { - synchronized (lock) { - if (set.get().add(element)) { - list.get().add(index, element); - return true; - } - //re-position in list if needed - final int oldIndex = list.get().indexOf(element); - if (index == oldIndex) { - return false; - } - - if (index > oldIndex) { - index--; //account for being removed - } - list.get().remove(oldIndex); - list.get().add(index, element); + if (set.add(element)) { + list.add(index, element); return true; } + //re-position in list if needed + final int oldIndex = list.indexOf(element); + if (index == oldIndex) { + return false; + } + + if (index > oldIndex) { + index--; //account for being removed + } + list.remove(oldIndex); + list.add(index, element); + return true; } /** @@ -502,13 +473,11 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public T remove(final int index) { - synchronized (lock) { - final T removedItem = list.get().remove(index); - if (removedItem != null) { - set.get().remove(removedItem); - } - return removedItem; + final T removedItem = list.remove(index); + if (removedItem != null) { + set.remove(removedItem); } + return removedItem; } /** @@ -516,9 +485,7 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public int indexOf(final Object o) { - synchronized (lock) { - return list.get().indexOf(o); - } + return list.indexOf(o); } /** @@ -526,9 +493,7 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public int lastIndexOf(final Object o) { - synchronized (lock) { - return list.get().lastIndexOf(o); - } + return list.lastIndexOf(o); } /** @@ -536,8 +501,7 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public ListIterator listIterator() { - return new ListItr(0); - //return list.get().listIterator(); + return list.listIterator(); } /** @@ -545,8 +509,7 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ @Override public ListIterator listIterator(final int index) { - return new ListItr(index); - //return list.get().listIterator(index); + return list.listIterator(index); } /** @@ -554,14 +517,12 @@ public class FCollection implements List, /*Set,*/ FCollectionView, * Note This method breaks the contract of {@link List#subList(int, int)} * by returning a static collection, rather than a view, of the sublist. *

- *

+ * * {@inheritDoc} */ @Override public List subList(final int fromIndex, final int toIndex) { - synchronized (lock) { - return ImmutableList.copyOf(list.get().subList(fromIndex, toIndex)); - } + return ImmutableList.copyOf(list.subList(fromIndex, toIndex)); } /** @@ -580,30 +541,25 @@ public class FCollection implements List, /*Set,*/ FCollectionView, * {@inheritDoc} */ public void sort(final Comparator comparator) { - synchronized (lock) { - try { - list.get().sort(comparator); - } catch (Exception e) { - System.err.println("FCollection failed to sort: \n" + comparator + "\n" + e.getMessage()); - } + try { + list.sort(comparator); + } catch (Exception e) { + System.err.println("FCollection failed to sort: \n" + comparator + "\n" + e.getMessage()); } } @Override public T get(final T obj) { - synchronized (lock) { - if (obj == null) { - return null; - } - for (T x : this) { - if (x.equals(obj)) { - return x; - } - } - return obj; + if (obj == null) { + return null; } + for(T x : this) { + if (x.equals(obj)) { + return x; + } + } + return obj; } - /** * An unmodifiable, empty {@link FCollection}. Overrides all methods with * default implementations suitable for an empty collection, to improve @@ -611,155 +567,93 @@ public class FCollection implements List, /*Set,*/ FCollectionView, */ public static class EmptyFCollection extends FCollection { private static final long serialVersionUID = 8667965158891635997L; - public EmptyFCollection() { super(); } - - @Override - public final void add(final int index, final T element) { + @Override public final void add(final int index, final T element) { } - - @Override - public final boolean add(final T e) { + @Override public final boolean add(final T e) { return false; } - - @Override - public final boolean addAll(final Collection c) { + @Override public final boolean addAll(final Collection c) { return false; } - - @Override - public final boolean addAll(final int index, final Collection c) { + @Override public final boolean addAll(final int index, final Collection c) { return false; } - - @Override - public final boolean addAll(final Iterable i) { + @Override public final boolean addAll(final Iterable i) { return false; } - - @Override - public final boolean addAll(final T[] c) { + @Override public final boolean addAll(final T[] c) { return false; } - - @Override - public final void clear() { + @Override public final void clear() { } - - @Override - public final boolean contains(final Object o) { + @Override public final boolean contains(final Object o) { return false; } - - @Override - public final boolean containsAll(final Collection c) { + @Override public final boolean containsAll(final Collection c) { return c.isEmpty(); } - - @Override - public final T get(final int index) { + @Override public final T get(final int index) { throw new IndexOutOfBoundsException("Any index is out of bounds for an empty collection"); } - - @Override - public final T getFirst() { + @Override public final T getFirst() { throw new NoSuchElementException("Collection is empty"); } - - @Override - public final T getLast() { + @Override public final T getLast() { throw new NoSuchElementException("Collection is empty"); } - - @Override - public final int indexOf(final Object o) { + @Override public final int indexOf(final Object o) { return -1; } - - @Override - public final boolean isEmpty() { + @Override public final boolean isEmpty() { return true; } - - @Override - public final Iterator iterator() { + @Override public final Iterator iterator() { return Collections.emptyIterator(); } - - @Override - public final int lastIndexOf(final Object o) { + @Override public final int lastIndexOf(final Object o) { return -1; } - - @Override - public final ListIterator listIterator() { + @Override public final ListIterator listIterator() { return Collections.emptyListIterator(); } - - @Override - public final ListIterator listIterator(final int index) { + @Override public final ListIterator listIterator(final int index) { return Collections.emptyListIterator(); } - - @Override - public final T remove(final int index) { + @Override public final T remove(final int index) { throw new IndexOutOfBoundsException("Any index is out of bounds for an empty collection"); } - - @Override - public final boolean remove(final Object o) { + @Override public final boolean remove(final Object o) { return false; } - - @Override - public boolean removeAll(final Collection c) { + @Override public boolean removeAll(final Collection c) { return false; } - - @Override - public final boolean removeAll(final Iterable c) { + @Override public final boolean removeAll(final Iterable c) { return false; } - - @Override - public final boolean retainAll(final Collection c) { + @Override public final boolean retainAll(final Collection c) { return false; } - - @Override - public final T set(final int index, final T element) { + @Override public final T set(final int index, final T element) { throw new IndexOutOfBoundsException("Any index is out of bounds for an empty collection"); } - - @Override - public final int size() { + @Override public final int size() { return 0; } - - @Override - public final void sort() { + @Override public final void sort() { } - - @Override - public final void sort(final Comparator comparator) { + @Override public final void sort(final Comparator comparator) { } - - @Override - public final List subList(final int fromIndex, final int toIndex) { + @Override public final List subList(final int fromIndex, final int toIndex) { if (fromIndex == 0 && toIndex == 0) { return this; } throw new IndexOutOfBoundsException("Any index is out of bounds for an empty collection"); } - - @Override - public final Object[] toArray() { - return ArrayUtils.EMPTY_OBJECT_ARRAY; - } - + @Override public final Object[] toArray() { return ArrayUtils.EMPTY_OBJECT_ARRAY; } @Override @SuppressWarnings("hiding") public final T[] toArray(final T[] a) { @@ -768,101 +662,8 @@ public class FCollection implements List, /*Set,*/ FCollectionView, } return a; } - - @Override - public final String toString() { + @Override public final String toString() { return "[]"; } } - - private class Itr implements Iterator { - protected int cursor; - protected int lastRet; - final FCollection l; - - public Itr() { - cursor = 0; - lastRet = -1; - l = FCollection.this.clone(); - } - - - @Override - public boolean hasNext() { - return cursor < l.size(); - } - - @Override - public T next() { - int i = cursor; - if (i >= l.size()) { - throw new NoSuchElementException(); - } - cursor = i + 1; - return (T) l.get(lastRet = i); - } - - @Override - public void remove() { - if (lastRet < 0) { - throw new IllegalStateException(); - } - - l.remove(lastRet); - FCollection.this.remove(lastRet); - cursor = lastRet; - lastRet = -1; - } - } - - public class ListItr extends Itr implements ListIterator { - ListItr(int index) { - super(); - cursor = index; - } - - @Override - public boolean hasPrevious() { - return cursor > 0; - } - - @Override - public int nextIndex() { - return cursor; - } - - @Override - public int previousIndex() { - return cursor - 1; - } - - @Override - public T previous() { - int i = cursor - 1; - if (i < 0) { - throw new NoSuchElementException(); - } - cursor = i; - return (T) l.get(lastRet = i); - } - - @Override - public void set(T e) { - if (lastRet < 0) { - throw new IllegalStateException(); - } - - l.set(lastRet, e); - FCollection.this.set(lastRet, e); - } - - @Override - public void add(T e) { - int i = cursor; - l.add(i, e); - FCollection.this.add(i, e); - cursor = i + 1; - lastRet = -1; - } - } -} +} \ No newline at end of file diff --git a/forge-game/src/main/java/forge/game/ability/effects/PlayEffect.java b/forge-game/src/main/java/forge/game/ability/effects/PlayEffect.java index ae2a778eaaf..70f615ea894 100644 --- a/forge-game/src/main/java/forge/game/ability/effects/PlayEffect.java +++ b/forge-game/src/main/java/forge/game/ability/effects/PlayEffect.java @@ -5,6 +5,7 @@ import java.util.HashMap; import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.stream.Collectors; import forge.card.CardStateName; import forge.card.GamePieceType; @@ -192,15 +193,9 @@ public class PlayEffect extends SpellAbilityEffect { if (sa.hasParam("ValidSA")) { final String valid[] = sa.getParam("ValidSA").split(","); - Iterator it = tgtCards.iterator(); - while (it.hasNext()) { - Card c = it.next(); - if (!Iterables.any(AbilityUtils.getBasicSpellsFromPlayEffect(c, controller), SpellAbilityPredicates.isValid(valid, controller , source, sa))) { - // it.remove will only remove item from the list part of CardCollection - tgtCards.asSet().remove(c); - it.remove(); - } - } + final List invalid = tgtCards.stream().filter(c -> !Iterables.any(AbilityUtils.getBasicSpellsFromPlayEffect(c, controller), SpellAbilityPredicates.isValid(valid, controller, source, sa))).collect(Collectors.toList()); + if (!invalid.isEmpty()) + tgtCards.removeAll(invalid); if (tgtCards.isEmpty()) { return; } @@ -233,16 +228,10 @@ public class PlayEffect extends SpellAbilityEffect { while (!tgtCards.isEmpty() && amount > 0 && totalCMCLimit >= 0) { if (hasTotalCMCLimit) { // filter out cards with mana value greater than limit - Iterator it = tgtCards.iterator(); final String [] valid = {"Spell.cmcLE" + totalCMCLimit}; - while (it.hasNext()) { - Card c = it.next(); - if (!Iterables.any(AbilityUtils.getBasicSpellsFromPlayEffect(c, controller), SpellAbilityPredicates.isValid(valid, controller , c, sa))) { - // it.remove will only remove item from the list part of CardCollection - tgtCards.asSet().remove(c); - it.remove(); - } - } + final List invalid = tgtCards.stream().filter(c -> !Iterables.any(AbilityUtils.getBasicSpellsFromPlayEffect(c, controller), SpellAbilityPredicates.isValid(valid, controller, c, sa))).collect(Collectors.toList()); + if (!invalid.isEmpty()) + tgtCards.removeAll(invalid); if (tgtCards.isEmpty()) break; params.put("CMCLimit", totalCMCLimit); diff --git a/forge-gui-desktop/src/test/java/forge/FCollectionTest.java b/forge-gui-desktop/src/test/java/forge/FCollectionTest.java index 5d88377a5f2..d9f7aa29d5a 100644 --- a/forge-gui-desktop/src/test/java/forge/FCollectionTest.java +++ b/forge-gui-desktop/src/test/java/forge/FCollectionTest.java @@ -5,7 +5,6 @@ import forge.game.card.CardCollection; import org.testng.annotations.Test; import java.util.ArrayList; -import java.util.Iterator; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -15,7 +14,7 @@ public class FCollectionTest { /** * Just a quick test for FCollection. */ - @Test + /*@Test void testBadIteratorLogic() { List cards = new ArrayList<>(); for (int i = 1; i < 5; i++) @@ -27,7 +26,7 @@ public class FCollectionTest { assertEquals(cc.size(), 3); } - @Test + /*@Test void testBadIteratorLogicTwo() { List cards = new ArrayList<>(); for (int i = 1; i <= 10; i++) @@ -40,7 +39,7 @@ public class FCollectionTest { i++; } assertEquals(cc.size(), 1); - } + }*/// Commented out since we use synchronized collection and it doesn't support modification while iteration @Test void testCompletableFuture() {