Minor changes to AffectEntityEvent and ExplosionEvent.Detonate (#2401)

* Make AffectEntityEvent#entities() unmodifiable
* Make Detonate#affectedLocations() unmodifiable
* Fix SpawnEntityEvent and ExplosionEvent.Detonate impl priority
* Exclude SpawnEntityEvent and all sub-events from SpongeEventFactoryTest#testCreate
* Make filterEntities and filterAffectedLocations return void

Author: Yeregorix

src/main/java/org/spongepowered/api/event/entity/AffectEntityEvent.java

@@ -35,8 +35,6 @@
 import org.spongepowered.api.world.explosion.Explosion;
 import org.spongepowered.api.world.server.ServerLocation;
 
-import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
 import java.util.function.Predicate;
 
@@ -68,11 +66,13 @@ public interface AffectEntityEvent extends Event, Cancellable {
     List<EntitySnapshot> entitySnapshots() throws IllegalStateException;
 
     /**
-     * Gets the {@link List} who will be affected after event
+     * Gets the {@link List} of entities who will be affected after event
      * resolution.
+     * This list can only be modified using {@link AffectEntityEvent#filterEntities(Predicate)}.
      *
-     * @return The List
+     * @return The list of entities that will be affected.
      */
+    @PropertySettings(requiredParameter = true, generateMethods = false)
     List<Entity> entities();
 
     /**
@@ -83,20 +83,9 @@ public interface AffectEntityEvent extends Event, Cancellable {
      * be removed from {@link #entities()}.</p>
      *
      * @param predicate The predicate to use for filtering
-     * @return The entities removed from {@link #entities()}
      */
-    default List<Entity> filterEntityLocations(Predicate<ServerLocation> predicate) {
-        final List<Entity> removedEntities = new ArrayList<>();
-
-        final Iterator<Entity> i = this.entities().iterator();
-        while (i.hasNext()) {
-            final Entity entity = i.next();
-            if (!entity.location().onServer().map(predicate::test).orElse(false)) {
-                i.remove();
-                removedEntities.add(entity);
-            }
-        }
-        return removedEntities;
+    default void filterEntityLocations(Predicate<ServerLocation> predicate) {
+        filterEntities(entity -> entity.location().onServer().map(predicate::test).orElse(false));
     }
 
     /**
@@ -107,19 +96,6 @@ default List<Entity> filterEntityLocations(Predicate<ServerLocation> predicate)
      * be removed from {@link #entities()}.</p>
      *
      * @param predicate The predicate to use for filtering
-     * @return The entities removed from {@link #entities()}
      */
-    default List<? extends Entity> filterEntities(Predicate<Entity> predicate) {
-        final List<Entity> removedEntities = new ArrayList<>();
-
-        final Iterator<Entity> i = this.entities().iterator();
-        while (i.hasNext()) {
-            final Entity entity = i.next();
-            if (!predicate.test(entity)) {
-                i.remove();
-                removedEntities.add(entity);
-            }
-        }
-        return removedEntities;
-    }
+    void filterEntities(Predicate<Entity> predicate);
 }

src/main/java/org/spongepowered/api/event/entity/SpawnEntityEvent.java

@@ -44,7 +44,7 @@
  * recommended event to interact with connecting players.</p>
  */
 @GenerateFactoryMethod
-@ImplementedBy(AbstractSpawnEntityEvent.class)
+@ImplementedBy(value = AbstractSpawnEntityEvent.class, priority = 2)
 public interface SpawnEntityEvent extends AffectEntityEvent {
 
     /**

src/main/java/org/spongepowered/api/event/impl/entity/AbstractAffectEntityEvent.java

@@ -32,7 +32,9 @@
 import org.spongepowered.api.event.impl.AbstractEvent;
 import org.spongepowered.api.util.annotation.eventgen.UseField;
 
+import java.util.Collections;
 import java.util.List;
+import java.util.function.Predicate;
 
 public abstract class AbstractAffectEntityEvent extends AbstractEvent implements AffectEntityEvent {
 
@@ -56,4 +58,14 @@ public List<EntitySnapshot> entitySnapshots() {
         }
         return this.entitySnapshots;
     }
+
+    @Override
+    public List<Entity> entities() {
+        return Collections.unmodifiableList(this.entities);
+    }
+
+    @Override
+    public void filterEntities(Predicate<Entity> predicate) {
+        this.entities.removeIf(entity -> !predicate.test(entity));
+    }
 }

src/main/java/org/spongepowered/api/event/impl/entity/AbstractSpawnEntityEvent.java

@@ -26,9 +26,8 @@
 
 import org.spongepowered.api.event.EventContextKeys;
 import org.spongepowered.api.event.entity.SpawnEntityEvent;
-import org.spongepowered.api.event.impl.AbstractEvent;
 
-public abstract class AbstractSpawnEntityEvent extends AbstractEvent implements SpawnEntityEvent {
+public abstract class AbstractSpawnEntityEvent extends AbstractAffectEntityEvent implements SpawnEntityEvent {
 
     @Override
     protected void init() {

src/main/java/org/spongepowered/api/event/impl/world/AbstractDetonateEvent.java

@@ -0,0 +1,50 @@
+/*
+ * This file is part of SpongeAPI, licensed under the MIT License (MIT).
+ *
+ * Copyright (c) SpongePowered <https://www.spongepowered.org>
+ * Copyright (c) contributors
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+package org.spongepowered.api.event.impl.world;
+
+import org.spongepowered.api.event.impl.entity.AbstractAffectEntityEvent;
+import org.spongepowered.api.event.world.ExplosionEvent;
+import org.spongepowered.api.util.annotation.eventgen.UseField;
+import org.spongepowered.api.world.server.ServerLocation;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.function.Predicate;
+
+public abstract class AbstractDetonateEvent extends AbstractAffectEntityEvent implements ExplosionEvent.Detonate {
+
+    @UseField
+    protected List<ServerLocation> affectedLocations;
+
+    @Override
+    public List<ServerLocation> affectedLocations() {
+        return Collections.unmodifiableList(this.affectedLocations);
+    }
+
+    @Override
+    public void filterAffectedLocations(Predicate<ServerLocation> predicate) {
+        this.affectedLocations.removeIf(location -> !predicate.test(location));
+    }
+}

src/main/java/org/spongepowered/api/event/world/ExplosionEvent.java

@@ -26,15 +26,15 @@
 
 import org.spongepowered.api.event.Cancellable;
 import org.spongepowered.api.event.Event;
-import org.spongepowered.api.event.block.ChangeBlockEvent;
 import org.spongepowered.api.event.entity.AffectEntityEvent;
+import org.spongepowered.api.event.impl.world.AbstractDetonateEvent;
+import org.spongepowered.api.util.annotation.eventgen.ImplementedBy;
+import org.spongepowered.api.util.annotation.eventgen.PropertySettings;
 import org.spongepowered.api.world.World;
 import org.spongepowered.api.world.explosion.Explosion;
 import org.spongepowered.api.world.server.ServerLocation;
 import org.spongepowered.api.world.server.ServerWorld;
 
-import java.util.ArrayList;
-import java.util.Iterator;
 import java.util.List;
 import java.util.function.Predicate;
 
@@ -80,6 +80,7 @@ interface Pre extends ExplosionEvent {
      * already calculated all the blocks and entities the explosion should
      * affect.
      */
+    @ImplementedBy(value = AbstractDetonateEvent.class, priority = 2)
     interface Detonate extends ExplosionEvent, AffectEntityEvent {
 
         /**
@@ -91,11 +92,12 @@ interface Detonate extends ExplosionEvent, AffectEntityEvent {
 
         /**
          * Gets the list of calculated affected locations for blocks that will
-         * be removed due to the explosion. Note that the list is mutable.
-         * However, adding new locations may cause unknown effects.
+         * be removed due to the explosion.
+         * This list can only be modified using {@link #filterAffectedLocations(Predicate)}.
          *
          * @return The list of blocks that will be affected by the explosion
          */
+        @PropertySettings(requiredParameter = true, generateMethods = false)
         List<ServerLocation> affectedLocations();
 
         /**
@@ -106,23 +108,8 @@ interface Detonate extends ExplosionEvent, AffectEntityEvent {
          * be removed from {@link #affectedLocations()}.</p>
          *
          * @param predicate The predicate to use for filtering
-         * @return The locations removed from {@link #affectedLocations()}
          */
-        default List<ServerLocation> filterAffectedLocations(Predicate<ServerLocation> predicate) {
-            final List<ServerLocation> removedLocations = new ArrayList<>();
-
-            final Iterator<ServerLocation> iter = this.affectedLocations().iterator();
-            while (iter.hasNext()) {
-                final ServerLocation location = iter.next();
-
-                if (!predicate.test(location)) {
-                    iter.remove();
-                    removedLocations.add(location);
-                }
-            }
-
-            return removedLocations;
-        }
+        void filterAffectedLocations(Predicate<ServerLocation> predicate);
     }
 
 }

src/test/java/org/spongepowered/api/event/SpongeEventFactoryTest.java

@@ -40,6 +40,7 @@
 import org.spongepowered.api.data.DataTransactionResult;
 import org.spongepowered.api.event.entity.AttackEntityEvent;
 import org.spongepowered.api.event.entity.DamageEntityEvent;
+import org.spongepowered.api.event.entity.SpawnEntityEvent;
 import org.spongepowered.api.event.entity.ai.goal.GoalEvent;
 import org.spongepowered.api.event.impl.AbstractEvent;
 import org.spongepowered.api.util.Color;
@@ -75,12 +76,11 @@ class SpongeEventFactoryTest {
     /**
      * Events with types that aren't able to be mocked for one reason or another.
      */
-    private static final Set<Class<?>> EXCLUDED_EVENTS = UnmodifiableCollections.toSet(
+    private static final List<Class<?>> EXCLUDED_EVENTS = UnmodifiableCollections.toList(
         DamageEntityEvent.class,
         GoalEvent.class,
-        GoalEvent.Add.class,
-        GoalEvent.Remove.class,
-        AttackEntityEvent.class
+        AttackEntityEvent.class,
+        SpawnEntityEvent.class
     );
 
     private static final Set<String> EXCLUDED_METHODS = UnmodifiableCollections.toSet("entitySnapshots");
@@ -104,10 +104,19 @@ class SpongeEventFactoryTest {
         return SpongeEventFactoryTest.mockParam(returnType);
     };
 
+    static boolean isExcludedEvent(Class<?> eventClass) {
+        for (Class<?> excludedClass : SpongeEventFactoryTest.EXCLUDED_EVENTS) {
+            if (excludedClass.isAssignableFrom(eventClass)) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     static Stream<Object[]> eventMethods() {
         return Arrays.stream(SpongeEventFactory.class.getMethods())
             .filter(method -> method.getName().startsWith("create") && Modifier.isStatic(method.getModifiers())
-                && !SpongeEventFactoryTest.EXCLUDED_EVENTS.contains(method.getReturnType()))
+                && !isExcludedEvent(method.getReturnType()))
             .map(method -> new Object[] { method.getReturnType().getName(), method});
     }