Dungeon-CampusMinden / Dungeon

The "Dungeon" is a tool to gamify classroom content and integrate it into a 2D Rogue-Like role-playing game.
MIT License
15 stars 36 forks source link

Dungeon: Standard Systeme auswählen und laden #1587

Open Flamtky opened 1 month ago

Flamtky commented 1 month ago

Aktuell werden in allen Starter Klassen die folgenden Systeme reingeladen:

private static void createSystems() {
    Game.add(new CollisionSystem());
    Game.add(new AISystem());
    Game.add(new HealthSystem());
    Game.add(new PathSystem());
    Game.add(new ProjectileSystem());
    Game.add(new HealthBarSystem());
    Game.add(new HudSystem());
    Game.add(new SpikeSystem());
    Game.add(new IdleSoundSystem());
 }

Es ist wichtig, die Freiheit zu haben, diese Systeme je nach Bedarf für das Spiel auszuwählen. Allerdings führt dies momentan zu viel Code-Duplizierung.

Vorschlag:

Vielleicht sollten wir eine Art Liste mit "Standard"-Systemen erstellen, die der Entwickler bei Bedarf einfach reinladen kann. So könnte man die Standard-Systeme zentral verwalten und die Starter einfacher anpassen.

Was meint ihr dazu?


Edit: Zusammenfassung der Diskussion

Es erscheint sinnvoll, eine gewisse Standard-Konfiguration für Systeme anzubieten, damit man das nicht immer selbst zusammensuchen muss. Vielleicht gibt es sogar mehrere unterschiedliche Standard-Konfigurationen für bestimmte Zielsetzungen - für Bewegung braucht man Position, Velocity, Draw ...? (Ein Startpunkt hierfür könnte die Analyse in #1565 sein.)

TODO:

tgrothe commented 1 month ago

@Flamtky Das hört sich gut an, denn Code-Duplizierungen (Redundanzen) sind immer unschön. Wie genau hattest du dir "eine Liste erstellen" dafür vorgestellt?

cagix commented 1 month ago

@Flamtky Bin ich bei Dir. Es gibt einfach eine bestimmte Auswahl an Standardsystemen, die man irgendwie immer braucht. Diese könnten vorkonfiguriert sein, so dass man dann nur noch zusätzliche Systeme hinzufügen muss.

Technisch sehe ich ad hoc zwei Ansätze:

(1) Man erstellt irgendwie eine Art Konfigurationsklasse[^1] analog zu den ganzen *Factories, und ergänzt Game um eine Methode, die das Hinzufügen einer Collection von Systems erlaubt (diese käme dann aus der Factory).

(2) Man klöppelt das direkt in Game rein und aktiviert das über eine Art Hilfsmethode.

Variante (2) wäre einfacher und direkter, aber dann hätten wir wieder Konfiguration in der Game-Klasse. Insofern läuft es vermutlich auf (1) raus, außer irgendwer hat eine bessere Idee.

[^1]: Edit: Damit meine ich einfach eine statistische Methode an passender Stelle, die ein (in der Methode fest definiertes) Set der Standard-Systeme zurück liefert.

cagix commented 1 month ago

Mittelfristig sollten wir nochmal drüber nachdenken, ob diese "fetten" Systeme so wirklich Sinn ergeben. Letztlich ist ein System i.d.R. nur eine zustandslose Funktion, die im Game registriert und dann zyklisch ausgeführt wird. Warum sollte ich dafür jeweils eine extra Klasse formulieren (müssen)? @AMatutat

Edit: Die Klasse System könnte diese Funktionen wie eine Factory-Klasse über statische Hilfsmethoden "produzieren", d.h. man hätte einen Ort (Klasse System) und einen Namen (Factory-Method), um an diese Callbacks dran zukommen für die Registrierung im Game.

Flamtky commented 1 month ago

Vlt könnte man auch eine Art abstrakte "Starter-Oberklasse" machen. Weil auch Methoden wie:

private static void onSetup() {
  Game.userOnSetup(
      () -> {
        createSystems();
        try {
          createHero();
        } catch (IOException e) {
          throw new RuntimeException(e);
        }
        setupMusic();
        Crafting.loadRecipes();
      });
}

doppelt sind. Sonst halt wie dein (1) Vorschlag @cagix klingt am sinnvollsten.

Mittelfristig sollten wir nochmal drüber nachdenken, ob diese "fetten" Systeme so wirklich Sinn ergeben. Letztlich ist ein System i.d.R. nur eine zustandslose Funktion, die im Game registriert und dann zyklisch ausgeführt wird. Warum sollte ich dafür jeweils eine extra Klasse formulieren (müssen)?

Aber ich mein wenn das eh noch alles geändert werden soll 😅

tgrothe commented 1 month ago

Eine Liste (oder Map oder eigene Klasse) mit vordefinierten Standardsystemen, die man verwenden und erweitern kann, finde ich eigentlich gar nicht schlecht.

Ich denke auch, das würde dann auf @cagix Vorschlag 1 hinauslaufen. Wobei ich dann aber nicht das Factory-Pattern (wie es z. B. bei java.util.Calendar der Fall ist), sondern eher ein Builder-Pattern zum "Kapseln" einsetzen würde, ohne teleskopierende Parameter und mit Record-Klassen (ähnlich wie in diesem Artikel beschrieben (auch wenns nur ein Artikel ist)) (JM2C).

cagix commented 1 month ago

@tgrothe was genau soll denn hier das builder-pattern leisten?

es geht doch nur darum, die üblichen systeme nicht jedesmal einzeln in game reinstecken zu müssen, sondern diese irgendwo quasi als vordefinition zu sammeln und dann mit einem aufruf in game konfigurieren zu können. (wenn man den frame-work-gedanken relaxen würde, könnte man diese systeme sogar fest in game verdrahten.) weitere ggf. benötigte systeme können (und sollten) dann bei bedarf wie derzeit einzeln vom kunden reingesteckt werden.

wie man diese vorkonfiguration aufschreibt, ist doch fast egal. das könnte einfach ein statisches set irgendwo sein, oder in anlehnung an die hero-/monster-factories eine statische methode an passender stelle.

für größere strukturen fehlt mir grad die fantasie - ich möchte den code gern angemessener und einfacher gestalten und nicht noch komplizierter machen als er eh schon ist mit den wahnsinnigen strukturen und tausenden indirektionen und diesen ewigen hashmaps und strings statt vernünftiger typdefinitionen und so ...

edit: oh, und "factory" ist hier eh nicht wirklich korrekt vom namen her - ich hatte mich nur an der begrifflichkeit der herofactory orientiert.

tgrothe commented 1 month ago

@cagix Ja, das stimmt, auf die konkrete Umsetzung kommt es noch nicht genau an. Und das mit dem "factory pattern" kann ich auch falsch aufgefasst haben.

Prinzipiell brauchen wir doch nur so etwas wie eine Methode initStandardSystems(), und dann noch so etwas wie initAdditionalSystem(System system) - oder täusche ich mich da gerade? Die Frage wäre dann aber, was die StandardSystems wären ... und ob man noch so etwas bräuchte wie initStandardSystemsExcept(System... systemsToExclude) o. Ä.

cagix commented 1 month ago

Vlt könnte man auch eine Art abstrakte "Starter-Oberklasse" machen. Weil auch Methoden wie:


private static void onSetup() {

  Game.userOnSetup(

      () -> {

        createSystems();

        try {

          createHero();

        } catch (IOException e) {

          throw new RuntimeException(e);

        }

        setupMusic();

        Crafting.loadRecipes();

      });

}

doppelt sind. Sonst halt wie dein (1) Vorschlag @cagix klingt am sinnvollsten.

@Flamtky Hmmm. Das würde bedeuten, dass dieser Callback irgendwie "additiv" funktionieren müsste.

Bei einer abstrakten Basisklasse würdest Du ja entweder das geerbte Moped überschreiben und müsstet dann doch wieder alles selbst formulieren. Oder Du würdest die Basismethode in Deiner Überschreibung aufrufen, aber dann müssen sich diese Callbacks "addieren".

Mittelfristig sollten wir nochmal drüber nachdenken, ob diese "fetten" Systeme so wirklich Sinn ergeben. Letztlich ist ein System i.d.R. nur eine zustandslose Funktion, die im Game registriert und dann zyklisch ausgeführt wird. Warum sollte ich dafür jeweils eine extra Klasse formulieren (müssen)?

Aber ich mein wenn das eh noch alles geändert werden soll 😅

@Flamtky Naja, hilft ja nix. Ich bin immer noch begeistert, was @AMatutat mit (und auch trotz 🤣) der Unterstützung durch das Team und trotz einer pingeligen Projektleitung 😎 in der gegebenen Zeit hinbekommen hat. Aber man merkt im Rückspiegel eben auch deutlich, dass an vielen Stellen mit "Autocompletion" gearbeitet wurde statt sich mal Gedanken um passende Strukturen und Abstraktionen zu machen.

cagix commented 1 month ago

Prinzipiell brauchen wir doch nur so etwas wie eine Methode initStandardSystems(), und dann noch so etwas wie initAdditionalSystem(System system) - oder täusche ich mich da gerade?

Es gibt dafür Game.add(). Das reicht völlig, es müsste nur auf VarArgs erweitert werden oder in einer Geschmacksrichtung Set überladen werden.

Und irgendwo gibt es noch ein gibMirAlleStandardSystemsAsSet - entweder direkt als statisches Set oder als Methode.

Der geneigte User muss dann aber noch selbst einmal Game.add(gibMirAlleStandardSystemsAsSet()) aufrufen.

Die Frage wäre dann aber, was die StandardSystems wären ...

Exakt.

und ob man noch so etwas bräuchte wie initStandardSystemsExcept(System... systemsToExclude) o. Ä.

NOPE! Siehe oben - ich möchte den Code einfacher gestalten, nicht komplizierter machen (und ich habe "kompliziert" mit bedacht gewählt!) ... wenn wer das std-set nicht mag, naja, dann ist die person doch frei, sich per einzelner Game.add() das manuell zusammenzuklöppeln.

cagix commented 1 month ago

wenn ich so drüber nachdenke: eigentlich sind die "vielen" starter (wo die beobachtete code-duplizierung herstammt) hier auch falsch.

das projekt (core, contrib) möchte vor allem ein framework sein, d.h. die user bauen sich die starter nach bedarf auf. die starter haben in diesem teil also nichts verloren.

auf der anderen seite haben wir in unserem mono-repo vier konkrete anwendungen oder show-cases: je einen starter für dungeon+dsl, blockly, dojo und bald dev. das sind sehr verschiedene anwendungen, so dass tatsächlich die frage ist, was wohl die gemeinsamen systeme sind und ob es sich lohnt, hier eine gemeinsame standard-konfiguration vorzugeben.

eigentlich müssen neben den code-strukturen auch nochmal die repo- und projekt-strukturen überdacht werden. => #1559

tgrothe commented 1 month ago

und ob man noch so etwas bräuchte wie initStandardSystemsExcept(System... systemsToExclude) o. Ä.

NOPE! Siehe oben - ich möchte den Code einfacher gestalten, nicht komplizierter machen (und ich habe "kompliziert" mit bedacht gewählt!) ... wenn wer das std-set nicht mag, naja, dann ist die person doch frei, sich per einzelner Game.add() das manuell zusammenzuklöppeln.

Das klingt vernünftig. Ich habe dazu einen Pull-Request verlinkt, weil hier bislang keiner zugewiesen war.

Flamtky commented 1 month ago

Wir brauchen eine Lösung, wo man essentielle System laden kann, aber trotzdem eine Auswahl treffen darf. Zum Beispiel muss ich für mein DevDungeon das HealthSystem mit einer eigene Klasse ersetzen (für eigene Logik). Ich glaube das braucht eine grundlegenden anderen Ansatz.

Edit: Vlt. einen Ansatz wo man einfach Einträge weglassen kann?

tgrothe commented 1 month ago

Wir brauchen eine Lösung, wo man essentielle System laden kann, aber trotzdem eine Auswahl treffen darf. Zum Beispiel muss ich für mein DevDungeon das HealthSystem mit einer eigene Klasse ersetzen (für eigene Logik). Ich glaube das braucht eine grundlegenden anderen Ansatz.

Edit: Vlt. einen Ansatz wo man einfach Einträge weglassen kann?

Man könnte die Standardsysteme hinzufügen und danach ein oder zwei Systeme wieder removen oder replacen.

cagix commented 1 month ago

Wir brauchen eine Lösung, wo man essentielle System laden kann, aber trotzdem eine Auswahl treffen darf. Zum Beispiel muss ich für mein DevDungeon das HealthSystem mit einer eigene Klasse ersetzen (für eigene Logik). Ich glaube das braucht eine grundlegenden anderen Ansatz. Edit: Vlt. einen Ansatz wo man einfach Einträge weglassen kann?

Man könnte die Standardsysteme hinzufügen und danach ein oder zwei Systeme wieder removen oder replacen.

Leute, das wird zu "fett" für den Use-Case. Tretet mal nen Schritt zurück und betrachtet das ganze Bild - wir reden hier über den einen Starter in einer bestimmten Applikation bzw. in einem bestimmten Spiel.

Aktuell muss jeder, der einen solchen Starter schreibt, alle gewünschten Systeme selbst registrieren. Ja, mit Blick über alle "fertigen" Projekte sieht das nach Code-Duplizierung aus. Aber aus Sicht eines einzelnen Spiels ist das nix weiter als eine Konfiguration des eigenen Spiels!

Was ihr grad diskutiert, ist eine von-hinten-durch-die-brust-ins-auge-Lösung, die super komplex wird (ich erinnere nur mal kurz an die wahnsinnige Modellierung der Key-Konfiguration) und nur eine winzige Nische abdeckt.

Ich gehe mit, dass man für die Schnittmenge der Systeme eine vordefinierte Konfiguration hinterlegen kann/sollte - also eine Menge an Systemen, die die aktuell verfügbaren Spiele (Dev, Dojo, Blockly, DSL?!) alle gemeinsam haben. Den Rest muss einfach jedes Spiel selbst nachkonfigurieren. Und ja, vielleicht braucht man die Möglichkeit, ein bereits hinzugefügtes System zu irgendeinem Zeitpunkt auch wieder aus dem Spiel zu ziehen, es also wieder zu entfernen. Damit könnte sich @Flamtky ein wenig Arbeit/Code sparen. Aber ob sich das wirklich lohnt?!

Flamtky commented 1 month ago

Damit könnte sich @Flamtky ein wenig Arbeit/Code sparen. Aber ob sich das wirklich lohnt?!

Nicht wirklich, mir ging es nur darum das z.B. wenn PathSystem nicht im Spiel ist, die PathComponents immernoch gesetzt nur dann halt nicht bearbeitet werden. Ich dachte halt nur an paar Systemen wie PathSystem, aber auch HealthSystem etc. die evtl aus den Starter ausgelagert werden sollten?? Falls man die System dann nicht will könnte man die per Game.remove(System) wieder entfernen.

tgrothe commented 1 month ago

Sorry, dass ich vorschnell mit dem PR gewesen bin.

Was ihr grad diskutiert, ist eine von-hinten-durch-die-brust-ins-auge-Lösung, die super komplex wird (ich erinnere nur mal kurz an die wahnsinnige Modellierung der Key-Konfiguration) und nur eine winzige Nische abdeckt.

Ich hab mal gerade etwas herumprobiert.

In der Client-Klasse wäre:

  private static void createSystems() {
    Game.add(StandardSystems.standardSystems());
    Game.remove(SpikeSystem.class);
    Game.remove(IdleSoundSystem.class);
  }

oder

  private static void createSystems() {
      Set<Class<? extends System>> toRemove = Set.of(SpikeSystem.class, IdleSoundSystem.class);
      Set<System> systems = new HashSet<>(StandardSystems.standardSystems());
      systems.removeIf(s -> toRemove.contains(s.getClass()));
      Game.add(systems);
  }

denkbar (und weitere, non-default Systeme hinzufügen, analog dazu). Das finde ich aber ehrlich gesagt beides etwas zu unübersichtlich.

Auf der anderen Seite wird eine andere Lösung wahrscheinlich viel zu komplex werden, wie @cagix bereits geschrieben hat. Also, ich hab zurzeit keine Idee mehr. 🤷🏼

tgrothe commented 1 month ago

Das wäre die aktuelle Schnittmenge:

System Name Location Client DojoStarter RandomDungeon RoomBasedDungeon Starter DevDungeon
AISystem dungeon y y y y y
CollisionSystem dungeon y y y y y
HealthBarSystem dungeon y y y y y
HealthSystem dungeon y y y y y
HudSystem dungeon y y y y y
IdleSoundSystem dungeon y y y y
PathSystem dungeon y y y y y
ProjectileSystem dungeon y y y y y
SpikeSystem dungeon y y y y

Wenn alle Starter relevant sind, hätte die Schnittmenge 7 Elemente. Wenn man einen Starter aus der Betrachtung nimmt (nämlich Client), hätte die Schnittmenge 9 Elemente.

Man könnte mehrere Standardsystememengen anbieten (mit 7 und 9 Elementen), oder nur eine von beiden. Bei der 9-elementigen Menge gäbe es nur einen "Sonderfall", und bei der 7-elementigen Menge gäbe es n-1 Sonderfälle. :D

Flamtky commented 1 month ago

Ich glaube einfach das in Client die beiden Systeme weggelassen wurden weil es die noch nicht gab?? Oder vlt wurde die vergessen? Also ich finde alle oben genannten Systeme relevant.

cagix commented 1 month ago

Ich würde mal sagen, dass es einige Systems einfach noch nicht gab, als die Anwendung/Starter gebastelt wurden.

Und ich bin tatsächlich nicht sicher, ob man sowas wie "Spike" wirklich auch in Blockly braucht, also ob das wirklich zu den Basissystemen gehört. Ich schätze, diese Diskussion muss ich mal mit @AMatutat führen.

Und ich glaube, dass es Sinn macht, sich separate Standardkonfigurationen für verschiedene Szenarien zu definieren: Bewegung, Kampf, Gesundheit, ... Aber auch hier muss man mal diskutieren, was das genau für Szenarien sind am Ende. Vielleicht ist das auch nur ein Gefühl, was so gar nicht zutrifft.

tgrothe commented 1 month ago

Ich glaube einfach das in Client die beiden Systeme weggelassen wurden weil es die noch nicht gab?? Oder vlt wurde die vergessen? Also ich finde alle oben genannten Systeme relevant.

Ja, das wäre durchaus denkbar, und das habe ich so ad hoc nicht gesehen. Dann wäre es noch sinnvoller, (mind.) eine Menge mit Standardsystemen festzulegen, imho. Ich kann leider auch nicht genau sagen, ob jedes System unabhängig voneinander ist, oder ob "Abhängigkeiten" zwischen den einzelnen Systemen bestehen. Aber vermutlich wird das HealthBarSystem nur zusammen mit dem HealthSystem Sinn machen bzw. darauf aufbauen. Vielleicht wäre es auch sinnvoll, manche Systeme zu "fusionieren". Aber i-wie sehe ich die Baustelle gerade größer denn kleiner werden.^^

tgrothe commented 1 month ago

grafik

Oh, das hatte ich nicht mitbekommen, dann ist der verlinkte PR ja redundant ...