Dungeon: improve readability by replacing implicit filtering (entityS…

…tream) with explicit filtering (filteredEntityStream, both System) (#1564)

**fixes #1553**

Überarbeitet den Umgang mit dem Entity-Stream in
`System#execute`-Methoden, um eine bessere Code-Lesbarkeit zu erreichen.

Anmerkung: Durch die gemachten Änderungen kommt es zu Redundanz im
Hinblick auf die Filterregeln. Diese werden weiterhin im System
gespeichert, können jetzt aber optional als Parameter eigenständig
verwendet werden. Wenn sich für das eigenständige Verwenden als
Parameter entschieden wird, muss bei Änderungen an den Filtern darauf
geachtet werden, diese überall einzutragen (im Konstruktor UND in der
Execute-Funktion).

Für leichtere Bedienung und Abwärtskompatibilität ist die alte Variante
aber weiterhin verfügbar.

Änderungen im Überblick:

* Umbenennung der Methode `System#entityStream` in
`System#filteredEntityStream`
* Hinzufügen von parametisierten Varianten der Methode
`System#filteredEntityStream` in zwei Geschmacksrichtungen:
* Array-Style-Parameter: `filteredEntityStream(final Class<? extends
Component>... filterRules)`
* Set-Parameter: `filteredEntityStream(Set<Class<? extends Component>>
filterRules)`
* Tests
* Anwenden der Methode `System#filteredEntityStream(final Class<?
extends Component>... filterRules)` in den `core`- und
`contrib`-Systemen
* Kleinere Refactoring-Änderungen (Kommentare, Namen)

---------

Co-authored-by: Andre Matutat <andre.matutat@hsbi.de>

dungeon/src/contrib/systems/AISystem.java

@@ -19,7 +19,7 @@ public AISystem() {
 
   @Override
   public void execute() {
-    entityStream().forEach(this::executeAI);
+    filteredEntityStream(AIComponent.class).forEach(this::executeAI);
   }
 
   private void executeAI(Entity entity) {

dungeon/src/contrib/systems/CollisionSystem.java

@@ -40,7 +40,9 @@ public CollisionSystem() {
    */
   @Override
   public void execute() {
-    entityStream().flatMap(this::createDataPairs).forEach(this::onEnterLeaveCheck);
+    filteredEntityStream(CollideComponent.class)
+        .flatMap(this::createDataPairs)
+        .forEach(this::onEnterLeaveCheck);
   }
 
   /**
@@ -52,7 +54,7 @@ public void execute() {
    * @return The stream which contains every valid pair of Entities.
    */
   private Stream<CollisionData> createDataPairs(final Entity a) {
-    return entityStream().filter(b -> isSmallerThen(a, b)).map(b -> newDataPair(a, b));
+    return filteredEntityStream().filter(b -> isSmallerThen(a, b)).map(b -> newDataPair(a, b));
   }
 
   /**

dungeon/src/contrib/systems/HealthBarSystem.java

@@ -72,7 +72,9 @@ public HealthBarSystem() {
 
   @Override
   public void execute() {
-    entityStream().map(this::buildDataObject).forEach(this::update);
+    filteredEntityStream(HealthComponent.class, PositionComponent.class)
+        .map(this::buildDataObject)
+        .forEach(this::update);
   }
 
   private void update(final EnemyData ed) {

dungeon/src/contrib/systems/HealthSystem.java

@@ -29,7 +29,7 @@ public HealthSystem() {
 
   @Override
   public void execute() {
-    entityStream()
+    filteredEntityStream(HealthComponent.class, DrawComponent.class)
         // Consider only entities that have a HealthComponent
         // Form triples (e, hc, dc)
         .map(this::buildDataObject)

dungeon/src/contrib/systems/HudSystem.java

@@ -91,7 +91,7 @@ private void addDialogToStage(final Group group, final Stage stage) {
 
   @Override
   public void execute() {
-    if (entityStream().anyMatch(this::pausesGame)) pauseGame();
+    if (filteredEntityStream(UIComponent.class).anyMatch(this::pausesGame)) pauseGame();
     else unpauseGame();
   }
 

dungeon/src/contrib/systems/IdleSoundSystem.java

@@ -48,7 +48,7 @@ public void execute() {
         Game.hero()
             .flatMap(e -> e.fetch(PositionComponent.class).map(PositionComponent::position))
             .orElse(null);
-    entityStream()
+    filteredEntityStream(IdleSoundComponent.class)
         .filter(e -> isEntityNearby(heroPos, e))
         .forEach(
             e ->

dungeon/src/contrib/systems/ProjectileSystem.java

@@ -33,7 +33,8 @@ public ProjectileSystem() {
   /** Sets the velocity and removes entities that have reached their endpoints. */
   @Override
   public void execute() {
-    entityStream()
+    filteredEntityStream(
+            ProjectileComponent.class, PositionComponent.class, VelocityComponent.class)
         // Consider only entities that have a ProjectileComponent
         .map(this::buildDataObject)
         .map(this::setVelocity)

dungeon/src/contrib/systems/SpikeSystem.java

@@ -19,6 +19,7 @@ public SpikeSystem() {
 
   @Override
   public void execute() {
-    entityStream().forEach(e -> e.fetch(SpikyComponent.class).orElseThrow().reduceCoolDown());
+    filteredEntityStream(SpikyComponent.class)
+        .forEach(e -> e.fetch(SpikyComponent.class).orElseThrow().reduceCoolDown());
   }
 }

game/src/core/System.java

@@ -29,7 +29,12 @@
  * inheriting System to implement the corresponding logic for these events.
  */
 public abstract class System {
-  /** WTF? . */
+  /**
+   * Determines how many frames pass between two executions of the {@link #execute()}-loop.
+   *
+   * <p>The value 1 means that no frames are skipped, the {@link #execute()}-loop is executed every
+   * frame.
+   */
   public static final int DEFAULT_EVERY_FRAME_EXECUTE = 1;
 
   protected static final Logger LOGGER = Logger.getLogger(System.class.getSimpleName());
@@ -160,13 +165,53 @@ public final boolean isRunning() {
   }
 
   /**
-   * Use this Stream to iterate over all active entities for this system in the {@link #execute}
-   * method.
+   * Provides a stream of active entities that match the specified filter rules.
+   *
+   * <p>This stream can be used in the {@link #execute} method to iterate over and process entities
+   * that have the required components.
+   *
+   * @param filterRules the component classes that an entity must possess to be included in the
+   *     stream. Entities must have all specified components to be processed. If this Set is empty,
+   *     the Stream will contain all Entities in the Game.
+   * @return a stream of active entities that meet the filter criteria and will be processed by the
+   *     system.
+   */
+  public final Stream<Entity> filteredEntityStream(
+      final Set<Class<? extends Component>> filterRules) {
+    return Game.entityStream(filterRules);
+  }
+
+  /**
+   * Provides a stream of active entities that are relevant to this system.
    *
-   * @return a stream of active entities that will be processed by the system
+   * <p>This stream can be used in the {@link #execute} method to iterate over and process entities
+   * that are managed by this system.
+   *
+   * @return a stream of active entities that will be processed by this system.
    */
-  public final Stream<Entity> entityStream() {
-    return Game.entityStream(this);
+  public final Stream<Entity> filteredEntityStream() {
+    return filteredEntityStream(filterRules);
+  }
+
+  /**
+   * Provides a stream of active entities that match the specified filter rules.
+   *
+   * <p>This stream can be used in the {@link #execute} method to iterate over and process entities
+   * that have the required components.
+   *
+   * <p>Note: Due to method overloading, it is not possible to use no filter rules. If you do not
+   * provide filter rules, the filter rules defined in the system constructor will be used. Use
+   * {@link #filteredEntityStream(Set)} with an empty Set to get all entities in the game.
+   *
+   * @param filterRules the component classes that an entity must possess to be included in the
+   *     stream. Entities must have all specified components to be processed.
+   * @return a stream of active entities that meet the filter criteria and will be processed by the
+   *     system.
+   */
+  @SafeVarargs
+  public final Stream<Entity> filteredEntityStream(
+      final Class<? extends Component>... filterRules) {
+    return filteredEntityStream(Set.of(filterRules));
   }
 
   /**

game/src/core/systems/CameraSystem.java

@@ -79,8 +79,9 @@ public static OrthographicCamera camera() {
 
   @Override
   public void execute() {
-    if (entityStream().findAny().isEmpty()) focus();
-    else entityStream().forEach(this::focus);
+    if (filteredEntityStream(CameraComponent.class, PositionComponent.class).findAny().isEmpty())
+      focus();
+    else filteredEntityStream().forEach(this::focus);
     // Check if Gdx.graphics is null which happens when the game is run in headless mode (e.g.
     // in tests)
     if (Gdx.graphics != null) {

game/src/core/systems/DrawSystem.java

@@ -83,7 +83,7 @@ public static SpriteBatch batch() {
   @Override
   public void execute() {
     Map<Boolean, List<Entity>> partitionedEntities =
-        entityStream()
+        filteredEntityStream(DrawComponent.class, PositionComponent.class)
             .collect(Collectors.partitioningBy(entity -> entity.isPresent(PlayerComponent.class)));
     List<Entity> players = partitionedEntities.get(true);
     List<Entity> npcs = partitionedEntities.get(false);

game/src/core/systems/LevelSystem.java

@@ -282,9 +282,10 @@ private void playSound() {
   public void execute() {
     if (currentLevel == null) {
       loadLevel(levelSize);
-    } else if (entityStream().anyMatch(this::isOnOpenEndTile)) onEndTile.execute();
+    } else if (filteredEntityStream(PlayerComponent.class, PositionComponent.class)
+        .anyMatch(this::isOnOpenEndTile)) onEndTile.execute();
     else
-      entityStream()
+      filteredEntityStream()
           .forEach(
               e -> {
                 isOnDoor(e)

game/src/core/systems/PlayerSystem.java

@@ -27,7 +27,7 @@ public PlayerSystem() {
 
   @Override
   public void execute() {
-    entityStream().forEach(this::execute);
+    filteredEntityStream(PlayerComponent.class).forEach(this::execute);
   }
 
   private void execute(final Entity entity) {

game/src/core/systems/PositionSystem.java

@@ -31,7 +31,7 @@ public PositionSystem() {
 
   @Override
   public void execute() {
-    entityStream()
+    filteredEntityStream(PositionComponent.class)
         .map(this::buildDataObject)
         .filter(data -> data.pc.position().equals(PositionComponent.ILLEGAL_POSITION))
         .forEach(this::randomPosition);
@@ -47,7 +47,7 @@ private void randomPosition(final PSData data) {
     if (Game.currentLevel() != null) {
       Coordinate randomPosition = Game.randomTile(LevelElement.FLOOR).coordinate();
       boolean otherEntityIsOnThisCoordinate =
-          entityStream()
+          filteredEntityStream()
               .map(this::buildDataObject)
               .anyMatch(psData -> psData.pc().position().toCoordinate().equals(randomPosition));
       if (!otherEntityIsOnThisCoordinate) {

game/src/core/systems/VelocitySystem.java

@@ -51,7 +51,9 @@ public VelocitySystem() {
   /** Updates the position of all entities based on their velocity. */
   @Override
   public void execute() {
-    entityStream().map(this::buildDataObject).forEach(this::updatePosition);
+    filteredEntityStream(VelocityComponent.class, PositionComponent.class, DrawComponent.class)
+        .map(this::buildDataObject)
+        .forEach(this::updatePosition);
   }
 
   private void updatePosition(VSData vsd) {

game/test/core/SystemTest.java

@@ -1,33 +1,38 @@
 package core;
 
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import java.util.List;
+import java.util.Set;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
 /** Tests for the {@link System} class. */
 public class SystemTest {
-  private System ts;
+  private System testSystem;
   private final boolean[] onAdd = {false};
   private final boolean[] onRemove = {false};
 
   /** WTF? . */
   @Before
   public void setup() {
-    ts =
+    testSystem =
         new System(DummyComponent.class) {
           @Override
           public void execute() {}
         };
-    ts.onEntityAdd = entity -> onAdd[0] = true;
+    testSystem.onEntityAdd = entity -> onAdd[0] = true;
 
-    ts.onEntityRemove = entity -> onRemove[0] = true;
+    testSystem.onEntityRemove = entity -> onRemove[0] = true;
   }
 
   /** WTF? . */
   @After
   public void cleanup() {
+    Game.removeAllEntities();
+    Game.removeAllSystems();
     onAdd[0] = false;
     onRemove[0] = false;
   }
@@ -36,17 +41,81 @@ public void cleanup() {
   @Test
   public void add() {
     Entity e = new Entity();
-    ts.triggerOnAdd(e);
+    testSystem.triggerOnAdd(e);
     assertTrue(onAdd[0]);
   }
 
   /** WTF? . */
   @Test
   public void remove() {
     Entity e = new Entity();
-    ts.triggerOnRemove(e);
+    testSystem.triggerOnRemove(e);
     assertTrue(onRemove[0]);
   }
 
+  /**
+   * Tests the filteredEntityStream method with no parameters. Ensures that the stream contains
+   * entities matching the default filter rules defined in the system constructor.
+   */
+  @Test
+  public void filteredEntityStream_no_parameter() {
+    Entity e1 = new Entity();
+    Entity e2 = new Entity();
+    e1.add(new DummyComponent());
+    Game.add(e1);
+    Game.add(e2);
+    List<Entity> stream = testSystem.filteredEntityStream().toList();
+    assertTrue(stream.contains(e1));
+    assertFalse(stream.contains(e2));
+  }
+
+  /**
+   * Tests the filteredEntityStream method with an array parameter. Ensures that the stream contains
+   * entities matching the specified filter rule (DummyComponent).
+   */
+  @Test
+  public void filteredEntityStream_array_parameter() {
+    Entity e1 = new Entity();
+    Entity e2 = new Entity();
+    e1.add(new DummyComponent());
+    Game.add(e1);
+    Game.add(e2);
+    List<Entity> stream = testSystem.filteredEntityStream(DummyComponent.class).toList();
+    assertTrue(stream.contains(e1));
+    assertFalse(stream.contains(e2));
+  }
+
+  /**
+   * Tests the filteredEntityStream method with a set parameter. Ensures that the stream contains
+   * entities matching the specified filter rule (DummyComponent).
+   */
+  @Test
+  public void filteredEntityStream_set_parameter() {
+    Entity e1 = new Entity();
+    Entity e2 = new Entity();
+    e1.add(new DummyComponent());
+    Game.add(e1);
+    Game.add(e2);
+    List<Entity> stream = testSystem.filteredEntityStream(Set.of(DummyComponent.class)).toList();
+    assertTrue(stream.contains(e1));
+    assertFalse(stream.contains(e2));
+  }
+
+  /**
+   * Tests the filteredEntityStream method with an empty set parameter. Ensures that the stream
+   * contains all entities as no filter rules are applied.
+   */
+  @Test
+  public void filteredEntityStream_empty_set_parameter() {
+    Entity e1 = new Entity();
+    Entity e2 = new Entity();
+    e1.add(new DummyComponent());
+    Game.add(e1);
+    Game.add(e2);
+    List<Entity> stream = testSystem.filteredEntityStream(Set.of()).toList();
+    assertTrue(stream.contains(e1));
+    assertTrue(stream.contains(e2));
+  }
+
   private static class DummyComponent implements Component {}
 }