search

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

[GAME] Framework Code aufräumen #357

Closed AMatutat closed 1 year ago

AMatutat commented 1 year ago

Durch den merge des Frameworks und des ECS (also pm-dungeon into game), könnten einige Strukturen entfernt oder vereinfacht werden. Daher sollen sich insbesondere die Verbindungsstellen der beiden Projekte angeschaut werden und geschaut werden, was davon vereinfacht werden kann. Einiges kann bestimmt auch vollkommen raus. Wir haben auch einige Helfermethoden geschrieben (Distanz zwischen zwei Punkte berechnen), die können jetzt in die entsprechenden "Framework"-Klassen geschoben werden.

Wir werden nicht alles auf einmal finden, aber einen ersten großen Rundumschlag sollten wir schon durchführen.

Time: 10

AHeinisch commented 1 year ago
Alle Klassen mal einmal durchgeschaut Auch wenn ich zum Ende immer mehr nur noch darauf geschaut habe, was vielleicht verbessert werden kann. Und nicht mehr was wir dort bereits zur Verfügung stellen. - Tools - Point - bereitstellen von x und y - umwandeln zu Coordinate -> int x,y anstelle on float x,y - erweiterungen derzeit: - EinheitsVector von 2 Punkten - Distanz zwischen 2 Punkten - Skill bzw. Projektilsystem nutzen noch nicht die Point Distanz - Mögliche Erweiterungen - Default ctor für Point 0,0 häufig erstellt z.b. für Berechnungen - Punkte addieren bzw. Punkte summieren ermöglichen - häufiger code ``` p1.x += x; p1.y += y; ``` - z.b. Hitbox addiert Punkte(links -> position + offset, rechtsoben -> position + offset + size) - Punkte mit einem Skalar multiplizieren - VelocityComponent bewegungsgeschwindigkeit - Interaktion mario check einheitsvector * skalar kontrollieren ob Tile betrettbar ist `PointA * i + PointB` - Punkte subtrahieren - Interaction bestimmen der Distanz zwischen 2 Punkten - Hitbox collisions richtung - Mögliche Nutzen - VelocityComponent hat einen bewegungs"vektor" anstelle von xVelocityy, yVelocity - Constant - sammlung von Konstanten zum einfacher wieder finden - Anwendungseigenschaften vielleicht besser aus zu lagern in die Config z.b. - Fenstergröße - Bildwiederholungsrate - Fenstertitle - Logopfad - Virtuelle Größe des Levels - Default_Zoom - Ob true oder False für betrettbar beim Levelelement bedeutet wird nur benutzt beim erstellen daher vielleicht entfernen ? - selten nutzung von außen und auch sehr unstrukturiert vielleicht nötig zu entfernen - starter - DesktopLauncher vielleicht Variablen extrahieren z.b. MaxWindowHeight, MaxWindowWidth - Game - `runloop` immer true daher kann `render` vereinfacht werden - `isOnEndTile` logik für Helden da hier kontrolliert wird ist deer Held auf dem EndTile und dann den Game mittgeteilt wird lade ein neues Level - pauseMenü nutzt nicht das `KeyboardSystem` - graphic - hud - durch entfernen von Removeable vielleicht besser die Klassen doch besser zu kapseln besonders da Framework und Programmieren doch eher zusammen sind. - PainterConfig vielleicht umbau auf Builder da hier nur eine konfugiration des Painters stattfindet und nicht irgendwelche andere Logik. - Painter - nutzt Point.x + config.xOffset sowie Point.y + config.yOffset -> mögliche Methode für Point - Optimierung von batch vielleicht wo das DrawSystem den Batch beginnt und beendet anstelle von jeder Draw befehlein eigenen Start, End macht - Animation - aktueller Nutzen - Anzeigen der Animation - Nächste Animation auswählen - anzahl der animationsframes kontrollieren - kontrollieren ob frame 0 die missingtexture ist - Notwendige erweiterung bereitsstellen ob der Letzte Frame erreicht wurde - HitAnimation - abwandlung nur einmal alle grafiken durchlaufend anstelle von immer wieder holen - ecs - systems - VelocitySystem - `updatePosiiton` - Point.x + Velocity.x & Point.y + Velocity.y -> neue Temporäre Variable - erstellen der Temporären Variable nicht direkt über ctor - zurücksetzen der Velocity vielleicht nicht sinnvoll. - enthält logik eines Projektils sowie einer Animation - PlayerSystem - Skills aufrufen - interaktion zwischen Held und nähesten Interagierbaren Entität ermöglichen - Bewegung anpassen - AktuelleBewegung = Tastenrichtung * maxBewegungsGeschwindigkeit - Mögliche Punkt operatio? - ProjectileSystem - setzen der AktuellenBewegungsgeschwindigkeit auf maxBewegungsgeschwindigkeit - `hasReachedEndPoint` - mehrmals wird die distanz berechnet ohne auf Point.distanz zurückzugreifen - Components - HitboxComponent - `getBottomLeft` -> addition 2 Punkte als neuer Temporärer Punkt - `getTopRight` -> addition 3 Punkte als neuer Temporärer Punkt - `getCenter` -> addiditon 3 Punkte ein Punkt mit scalar Temporär dividiert `position + offset + (size / 2)` - VelocityComponent - Velocity vielleicht als Point / Vector2 umbauen - SkillTools - `calculateLastPositionInRange` - nicht benutzen von Point.dist - temporäer punkt aus PointA + ( skalar * skalar * ((PointB - PointA ) / skalar)) - eigentlich nur eine berechnung von Min(PointStart + PointEnde, PointStart + PointMaxMovement) - `calculateVelocity` - nicht benutzten von Point.dist - nicht benutzen vom Einheitsvektor * geschwindigkeit - `getCursorPositionAsPoint` vielleicht in die Camera? - ai - AITools - `move` - Tile.Direcction to directionalVector vielleicht extrahieren - `alle get Tile` extrahieren nach Level? - `calculatePathToHero` bisher nicht genutzt - `inRange(p1,p2,range)` vielleicht nach Point auslagern da ähnlicher Code vielleicht außerhalb von AI benutzt wird - - Level - Tile - Direction inverse der Direction ermöglichen aktuell für Collision benutzt

Mein Plan wäre nun wie folgt

Das wäre hier jetzt so meine Idee hier brauche ich aber Feedback bzw. vielleicht denke ich auch zu extrem. @AMatutat Hier einmal drauf schauen ob dies mögliche Veränderungen wären die du dir auch so vorstellen kannst. Sobald ich das Ok Für das aufräumen hab laufe ich hier die Checkliste nach ihren primär Klassen ab alle in eigenen PRs damit kleiner und auch nicht zu viel auf einmal.

cagix commented 1 year ago

klingt für mich alles recht sinnvoll. vor allem, dass man endlich punkte und vektoren direkt addieren kann (und dafür nicht außerhalb der klasse die interna rausfischen muss und damit dann arbeiten muss - da hätte man sich die klasse point gleich sparen können ...).

edit: @AHeinisch dennoch solltest du bitte besser auf ein go vom @AMatutat warten, da er viel näher am code ist als ich ...

AMatutat commented 1 year ago

Sieht gut aus.

Zu Point: Da können wir nochcalculateDistance(Point other) (wirst du für inRange vermutlich auch brauchen). @Lena241 hat das bei den Skills auch ein paar mal per Hand geklöppelt.

zurücksetzen der Velocity vielleicht nicht mehr im System machen

wo dann?

Projektillogik im VelocitySystem

Kannst du das etwas ausführen?

AHeinisch commented 1 year ago

zurücksetzen der Velocity vielleicht nicht mehr im System machen

wo dann?

Vielleicht bietet es sich ja auch an ein flag wie "velocityResetsEachFrame" als Alternative einzubauen. Da war ich mir aber nicht sicher, ob dies wirklich notwendig ist es raus zu machen darum ein vielleicht.

Projektillogik im VelocitySystem

Kannst du das etwas ausführen?

In updatePosition der folgende Schnipsel:

        // remove projectiles that hit the wall or other non-accessible
        // tiles
        else if (vsd.e.getComponent(ProjectileComponent.class).isPresent())
            Game.entitiesToRemove.add(vsd.e);

Hier wäre es doch schlauer dieses im ProjectileSystem zu haben, da es hier für mich irgendwie fehl am Platz ist.

Zu Point: Da können wir nochcalculateDistance(Point other) (wirst du für inRange vermutlich auch brauchen). @Lena241 hat das bei den Skills auch ein paar mal per Hand geklöppelt.

Klingt sinnvoll, auch wenn bei den vielen merges ja dieses hier bereits hereingekommen ist.

/**
     * calculates the distance between two points
     *
     * @param p1 Point A
     * @param p2 Point B
     * @return the Distance between the two points
     */
    public static float calculateDistance(Point p1, Point p2) {
        float xDiff = p1.x - p2.x;
        float yDiff = p1.y - p2.y;
        return (float) Math.sqrt(xDiff * xDiff + yDiff * yDiff);
    }
AMatutat commented 1 year ago

Klingt sinnvoll, auch wenn bei den vielen merges ja dieses hier bereits hereingekommen ist.

Oha, das meint ich. Das kann ja dann nach Point geschoben werden.

AMatutat commented 1 year ago

Ich würde erstmal sagen: Mach mal alles. Aber (auch wenn nervig) bitte jeweils ein eigenen PR. Dann kriegen wir die kleinen Sachen schnell rein und bei größeren Änderungen haben wir ein wenig Diskussionszeit.

cagix commented 1 year ago

@AMatutat ich hab mal "meeting" dran gemacht. schaue dir bitte die ergebnisse von @AHeinisch an und stelle das im meeting kurz vor (was, vor-/nachteile).

AMatutat commented 1 year ago

aus diesem ticket sind einige sub-tickets entstanden. Ich glaube dieses Ticket hat seine schuld erfüllt.