Dungeon: add filter-stream operations to System

[cagix]

aus #1564 (comment):

Ein häufiges Pattern ist das Aufrufen der System#entityStream-Methode (oder nach dem Mergen von #1564 der System#filteredEntityStream-Methode) und Filtern aller Entitäten nach dem Vorhandensein einer bestimmten Component, um danach dann über alle Entitäten mit einer bestimmten Component zu itererieren. Dabei wird exakt diese Component anschließend extrahiert und damit dann irgendwas gemacht, ohne dass die Entität noch benötigt wird:

  public void execute() {
    filteredEntityStream(SpikyComponent.class)
        .forEach(e -> e.fetch(SpikyComponent.class).orElseThrow().reduceCoolDown());
  }

Es wäre einfacher, wenn in System auch ein Filter für Stream<T extends Component> angeboten würde für einfache Filter-Operationen.

Bei komplexeren Filtern (mehrere Components gesucht) müsste ein Stream über passenden Tuples gebildet werden.

---

Edit: Das wird in dem folgenden Beispiel (aus #1567) besonders deutlich:

    filteredEntityStream(SpikyComponent.class)
        .map((e -> e.fetch(SpikyComponent.class)))
        .flatMap((Optional::stream))
        .forEach(SpikyComponent::reduceCoolDown);

In diesem Fall ist offensichtlich, dass die Abstraktionen noch nicht passen: Es wird ja nicht nach den Entitäten gesucht, sondern ihren Components. Es sollte also ein Stream mit den richtigen Objekten, also den gewünschten Components geliefert werden, anstatt einen Stream mit den Container-Objekten zu erzeugen, aus dem man die Dinge dann nochmal separat extrahieren muss.

---

Edit: Hier ein Vergleich mit "professionellen" ECS-Systemen wie Dominion:

Ein System ist einfach eine Funktion, die zyklisch ausgeführt wird:

        // creates a system
        Runnable system = () -> {
            //finds entities
            hello.findEntitiesWith(Position.class, Velocity.class)
                    // stream the results
                    .stream().forEach(result -> {
                        Position position = result.comp1();
                        Velocity velocity = result.comp2();
                        position.x += velocity.x;
                        position.y += velocity.y;
                        System.out.printf("Entity %s moved with %s to %s\n",
                                result.entity().getName(), velocity, position);
                    });
        };

(Quelle: https://github.com/dominion-dev/dominion-ecs-java/tree/main?tab=readme-ov-file#ok-lets-start)

Dabei ist hello das ECS-System und hat in etwa die Rolle von Game bei uns.

Interessant ist die Methode findEntitiesWith, die es für Dominion (bei uns Game) gibt. Diese macht in etwa das, was bei uns das System#filteredEntityStream macht (wieso ist das eigentlich in System und nicht in Game?!). Aber in der Rückgabe wird nicht einfach ein Entitäten-Stream erzeugt, sondern es wird direkt ein Stream mit Daten-Objekten erzeugt, in denen die gesuchten Components und auch die Entität drin sind. Wir definieren dieses Datenobjekt in der Regel in jedem einzelnen System neu und bauen es in jedem System in jedem Filter-Stream selbst. In Dominion gibt es dafür den Typ interface Results<T> extends Iterable<T>, in dem dann verschiedene Daten-Tupel definiert werden, beispielsweise record With2<T1, T2>(T1 comp1, T2 comp2, Entity entity) für zwei Components. Damit ist die Filter-Stream-Methode in der Hauptklasse Dominion das sowas wie <T1, T2, T3> Results<With3<T1, T2, T3>> findEntitiesWith(Class<T1> type1, Class<T2> type2, Class<T3> type3);... Diese WithX-Records gibt es von 1 bis 6, bei uns würde das bei 3 oder 4 enden (vgl. Analyse unten).

Seltsam finde ich, dass in Results mehrere Ergebnisse verpackt werden, d.h. ich kriege beim Filtern exakt ein Results-Objekt zurück. Ich kann darüber streamen, aber hier finde ich unsere API klarer - es kommt ein Stream (oder eine Collection) zurück.

Darüber kann man gut nochmal nachdenken!

1. System#filteredEntityStream sollte nach Game verschwinden. Warum haben wir die Entity-Stream-Methoden an verschiedenen Stellen?
2. Es sollte ein gemeinsames parametrisches Datenobjekt geben mit direktem Zugriff auf die im Filter angegebenen Components und die Entity (analog zu Result).
3. Ideal wäre eine überlagerte Typ-Definition, also Result<T>, Result<T1, T2>, Result<T1, T2, T3>, ... damit würde man sich die innere Verschachtelung mit dem With1<T>, With2<T1,T2>, ... ersparen.

---

Edit: Analyse der Stream-Methoden in Game und System (warum sind die eigentlich nicht alle in einer Klasse?!):

Methode in	wird aufgerufen in	dort tatsächlich benötigt	Bemerkung
Game: Stream entityStream()	contrib.entities.AIFactory	Entity, (HealthComponent, AIComponent)	Eigentlich wird nur die Entität benötigt (welche die beiden Components haben muss) => Einsatz der falschen Filter-Methode!
	contrib.entities.HeroFactory	Entity, UIComponent
	contrib.utils.components.interaction.InteractionTool	InteractionComponent, PositionComponent
Game: Stream entityStream(final System system)	YAGNI
Game: Stream entityStream(final Set<Class<? extends Component>> filter)	dojo.rooms.rooms.riddle.QuaderRoom	(PlayerComponent), HealthComponent	Eigentlich wird nach dem Hero gesucht, um dessen Health verändern zu können => Einsatz der falschen Methode!
	dojo.rooms.rooms.search.KillMonsterRoom	(PlayerComponent), HealthComponent	Eigentlich wird nach dem Hero gesucht, um dessen Health verändern zu können => Einsatz der falschen Methode! (2x)
	System: Stream filteredEntityStream(final Set<Class<? extends Component>> filterRules)		Weiterleitung
System: Stream filteredEntityStream(final Set<Class<? extends Component>> filterRules)	System: Stream filteredEntityStream()		Weiterleitung
	System: Stream filteredEntityStream(final Class<? extends Component>... filterRules)		Weiterleitung
System: Stream filteredEntityStream()	contrib.systems.CollisionSystem	Entity, CollideComponent
	core.systems.LevelSystem	PlayerComponent, PositionComponent	Eigentlich wird nach dem Hero gesucht, um dessen PositionComponent abzufragen => Einsatz der falschen Methode!
	core.systems.PositionSystem	Entity, PositionComponent	Eigentlich wird nur die PositionCompoment benötigt, um Felder mit anderen Entitäten zu erkennen
System: Stream filteredEntityStream(final Class<? extends Component>... filterRules)	contrib.systems.AISystem	Entity, AIComponent
	contrib.systems.CollisionSystem	Entity, CollideComponent
	contrib.systems.HealthBarSystem	(Entity?), HealthComponent, PositionComponent	Wird die Entity wirklich benötigt?
	contrib.systems.HealthSystem	(Entity?), HealthComponent, DrawComponent	Wird die Entity wirklich benötigt?
	contrib.systems.HudSystem	UIComponent
	contrib.systems.IdleSoundSystem	IdleSoundComponent, (Entity, PositionComponent)	Eigentlich wird die PositionComponent gebraucht zur Bestimmung der Nähe zum Hero, und dann die IdleSoundComponent, ob hier ein IdleSound gespielt werden soll.
	contrib.systems.ProjectileSystem	Entity, ProjectileComponent, PositionComponent, VelocityComponent
	contrib.systems.SpikeSystem	SpikyComponent
	core.systems.CameraSystem	CameraComponent, PositionComponent
	core.systems.DrawSystem	DrawComponent, PositionComponent, (PlayerComponent)
	core.systems.LevelSystem	PlayerComponent, PositionComponent	Hier wird eigentlich nach dem Hero gesucht => Falsche Methode.
	core.systems.PlayerSystem	Entity, PlayerComponent
	core.systems.PositionSystem	PositionComponent
	core.systems.VelocitySystem	Entity, VelocityComponent, PositionComponent, DrawComponent

[AMatutat]

Bei komplexeren Filtern (mehrere Components gesucht) müsste ein Stream über passenden Tuples gebildet werden.

Ich würde tatsächlich die Entität als diesen Tupel sehen.
Für komplexere Konstrukte kann man dann einen Builder verwenden

Aus DrawSystem

  private DSData buildDataObject(final Entity entity) {
    DrawComponent dc =
        entity
            .fetch(DrawComponent.class)
            .orElseThrow(() -> MissingComponentException.build(entity, DrawComponent.class));
    PositionComponent pc =
        entity
            .fetch(PositionComponent.class)
            .orElseThrow(() -> MissingComponentException.build(entity, PositionComponent.class));
    return new DSData(entity, dc, pc);
  }

  private record DSData(Entity e, DrawComponent dc, PositionComponent pc) {}

[cagix]

im prinzip bin ich da bei dir.

aber bei entities bin ich prinzipiell unsicher, ob eine component vorhanden ist oder nicht, deshalb ja auch das optional in der rückgabe. beim filtern nun die gesuchten components in eine neue entität zu packen löst uns leider nicht von dem sperrigen umgang mit optional in java, auch wenn wir hier in diesem konkreten fall eigentlich wissen, dass die component/s da ist/sind ...

und man könnte sich auch fragen, warum man hier in diesem fall nicht einfach die ursprüngliche entity zurück liefert...

vielleicht macht das spezielle filtern nach components nur sinn für den fall, dass man nur nach einer component sucht? selbst wenn man eine einfache tupel- oder pair-klasse hätte, das problem mit der reihenfolge bliebe bei komplexeren abfragen. (obwohl man die gefundenen components darin so anordnen könnte wie im filteraufruf übergeben.)

[AMatutat]

dem sperrigen umgang mit optional in java, auch wenn wir hier in diesem konkreten fall eigentlich wissen, dass die component/s da ist/sind

Eigentlich müsste man hier Ansetzen, oder nicht?
Entity#forceFetch(Component.class): Component (also ohne Optional) und dann den Aufrufer in die Pflicht nehmen. Aber in der Praxis wird das dann auch verwendet, wenn eigentlich die Optional Variante richtig wäre, weil Entwickler==Faul.

[cagix]

dem sperrigen umgang mit optional in java, auch wenn wir hier in diesem konkreten fall eigentlich wissen, dass die component/s da ist/sind

Eigentlich müsste man hier Ansetzen, oder nicht? Entity#forceFetch(Component.class): Component (also ohne Optional) und dann den Aufrufer in die Pflicht nehmen. Aber in der Praxis wird das dann auch verwendet, wenn eigentlich die Optional Variante richtig wäre, weil Entwickler==Faul.

Aus meiner Sicht ist das nicht wirklich der richtige Schritt, da wir hier verschiedene "Kreise" haben:

1. Entity als solche ("Entität im Spiel"): Es ist unsicher, ob diese Entity eine bestimmte Component besitzt. In diesem Use-Case ist es sinnvoll, beim fetch() mit Optional zu arbeiten (statt in alter schlechter Java-Manier ggf. stumpf null zu liefern oder (noch schlimmer) mit Exceptions um sich zu werfen).

2. Filtern nach Eigenschaften im System: Hier kommt ein Stream zurück, der bestimmte Eigenschaften der Stream-Elemente "garantiert" (naja, so lange das korrekt implementiert wurde).

   Wenn ich einen Stream<Entity> liefere, dann habe ich an der Stelle die Zusage über die Typen, dass hier nur Entity-Objekte drin sind.

   Wenn ich aber nach Components suche, sollte ich

   • entweder (wenn ich nur nach einer Component suche) einen Stream<ComponentXYZ> liefern - dann kann ich als Kunde direkt damit arbeiten, oder
   • alternativ (wenn ich nach mehreren Components in Kombination suche) eine passende Komposition dieser Components in den Stream packen.

   Entity klingt dabei zunächst irgendwie naheliegend, da hier genau diese Gruppierung bereits gemacht wird, ist aber für mich die falsche Abstraktion: Ich möchte einen Container, in dem ich verlässlich auf Dinge zugreifen kann, weil ich bereits weiss, dass sie da sind. Bei Entity weiss ich das eben nicht. Und hier einen vermeintlichen Shortcut über ein forceFetch einzubauen führt nur (früher oder später) zu neuen Problemen und grässlichem Code - die Leute werden das dann nämlich nutzen und die ganzen Vorteile von Optional gehen flöten.

[cagix]

Nach der Analyse der Verwendung brauchen wir aktuell für die Filter drei verschiedene parametrische (innere) Records zum Halten der gewünschten Daten:

record A(Entity entity, T component)
record B(Entity entity, T1 component1, T2 component2)
record C(Entity entity, T1 component1, T2 component2, T3 component3)

(mit passend beschränkten Typ-Variablen)

Diese Record-Klassen könnten als innere statische Klassen in System definiert werden und dann in drei unterschiedlichen Filtermethoden zurückgeliefert werden:

Stream<A<T>> filteredEntityStream(T Component)
Stream<B<T1, T2>> filteredEntityStream(T1 Component, T2 Component)
Stream<C<T1, T2, T3>> filteredEntityStream(T1 Component, T2 Component, T3 Component)

[cagix]

Hmmm. Lustig: In HealthSystem modifizieren wir im Stream den Stream selbst:

  public void execute() {
    filteredEntityStream(HealthComponent.class, DrawComponent.class)
        ...
        .forEach(this::removeDeadEntities);
  }
  private void removeDeadEntities(final HSData hsd) {
      ...
      Game.remove(hsd.e);
  }

Der Stream geht in ECSManagment in der passenden activeEntityStorage los, und das remove ändert die Einträge in der activeEntityStorage... 😱

Eigentlich möchte man den Stream nach "lebendigen" und "toten" Entitäten partitionieren => zwei Mengen:

• Auf der Menge der lebendigen Entitäten möchte man dann den Damage anwenden. (Wenn sie dabei "sterben", kann das im nächsten Frame bestimmt werden, d.h. diese Entitäten landen im nächsten Durchlauf in der zweiten Menge.)
• Auf der Menge der toten Entitäten möchte man dann die Animation und so machen und die Entitäten aus dem Spiel entfernen.

Das sollte aber dann auf diesen beiden Mengen jeweils erfolgen, nicht auf dem Stream aus filteredEntityStream ...

=> #1578

[cagix]

@AMatutat ich habe in #1579 mal "laut überlegt" und ein paar optionen durchgespielt.

option (1) ist die "traditionelle" variante, d.h. filteredEntityStream liefert einen Stream<Entity> zurück und man muss dann aus diesen entitäten selbst die nötigen components nochmal rausfischen (obwohl man eben erst danach gefiltert hat).

option (2) ist eine variante, die für eine, zwei oder drei components im filtervorgang in filteredEntityStreamX einen stream mit entsprechenden tupeln dieser components (plus der entität) zurückliefert. bei mehr als drei gesuchten components würde dann einfach wieder ein Stream<Entity> kommen und der kunde müsste sich das data object selbst bauen (oder eine neue überladung der filter-methode erstellen). im grunde greift das dem bauen der data objects in den ganzen systemen vor - das bräuchte man dort dann nicht mehr machen.

option (2a) ist eine variante von (2), bei der das neue pattern matching in java verwendet wird. leider geht das nur mit switch, d.h. beim "auspacken" braucht man eine eigene methode. und der switch möchte gern "exchaustive" sein, d.h. es sollte eine default-option dabei sein - und hier kommt dann wieder eine hässliche exception mit ins spiel.

option (3) ist eine variante, wo nur eine component gesucht und benötigt wird - hier wird dann direkt ein Stream<T extends Component> gebaut und die kunden können damit direkt arbeiten. aktuell trifft das aber nur auf wenige systeme zu, wobei man sich alle systeme eh nochmal gründlich anschauen sollte, was dort passiert (s.o. mit HealthSystem). vermutlich stellt sich dann heraus, dass man doch öfter als gedacht nur mit einer component hinkommt?

die spielereien sind alle in system - dort gehören sie aber vermutlich nicht hin und es sollte (wenn überhaupt) nach game umgezogen werden.

[cagix]

@AMatutat Ich bin grad am überlegen, ob die Rückgabe von Streams in Game/System wirklich eine gute Idee war. Aus Game/System müsste man eigentlich nur ein Set.of() herausreichen (dieses ist dann immutable und zudem eine Kopie der internen Datenstruktur), dann könnte man auf diesem Set als Kunde selbst streamen und hätte dann nicht das Problem wie im HealthSystem, dass man sich derweil der Stream noch über die Entitäten läuft diese bereits teilweise aus der Datenstruktur (aka Basis des Streams) löscht ...

[AMatutat]

Ich hinterlass hier mal ein "Ich habs gesehen" Kommentar. Hier muss ich mich mit ner dicken Tasse Kaffe nochmal reinwurschteln.