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: Dokumentation in VelocityComponent und VelocitySystem verwirrend #1527

Open cagix opened 4 months ago

cagix commented 4 months ago

In der Javadoc der VelocityComponent steht https://github.com/Dungeon-CampusMinden/Dungeon/blob/a10bc189942dbe31c31ecd83b7be947c7d1b73c9/game/src/core/components/VelocityComponent.java#L24-L26 - in der Javadoc des VelocitySystem dann https://github.com/Dungeon-CampusMinden/Dungeon/blob/a10bc189942dbe31c31ecd83b7be947c7d1b73c9/game/src/core/systems/VelocitySystem.java#L22-L25

Da die Werte innerhalb der Component nicht zusammenhängen, ist das ein netter Widerspruch :)

Der Chef-Entwickler (@AMatutat) würde für das Setzen der Geschwindigkeit eher currentXVelocity nutzen, und xVelocity wird eher als eine Art Höchstgeschwindigkeit angesehen.

Andererseits wird in den Beispielen die currentXVelocity gesetzt (erstes Codebeispiel im Quickstart unter "Held bewegen") bzw. die xVelocity gesetzt über die PlayerComponent daraus beim Update die currentXVelocity bestimmt (zweites Codebeispiel im Quickstart unter "Held bewegen").

Welche Methode ist denn nun die, die ich als Nutzer der API benutzen soll? currentXVelocity oder xVelocity? Und was hat es mit previousXVelocity auf sich?

VERWIRREND!

Das muss besser abgestimmt und dokumentiert werden. Außerdem sollte es eine sinnvolle Umbenennung geben, so dass man als Nutzer weiss, welche der Methoden man denn nun aufrufen soll. Müssen wirklich drei Methoden in die API getan werden (es gibt ja auch noch previousXVelocity()...)? Und falls ja, sollten diese Methoden irgendwie gekoppelt werden, d.h. ein Setzen der aktuellen Geschwindigkeit aktualisiert automatisch den internen bisherigen/alten Wert?!

AMatutat commented 4 months ago

Andererseits wird in den Beispielen die currentXVelocity gesetzt (erstes Codebeispiel im Quickstart unter "Held bewegen") bzw. die xVelocity gesetzt über die PlayerComponent daraus beim Update die currentXVelocity bestimmt (zweites Codebeispiel im Quickstart unter "Held bewegen").

Hier verwirrt mich deine Verwirrung. In beiden Code Beispielen wird die currentYVelocity gesetzt und zwar auf den Wert von yVelocity.

Die yVelocity ist nur eine Konstante, um dir als Programmiere zu helfen eine feste Bewegungsgeschwindigkeit-Parameter zu speichern. Du willst ja, dass dein Monster WENN es sich bewegt, sich immer gleich schnell bewegt.

Das VelocitySystem juckt sich nicht für die xVelocity bzw yVelocity, das interessiert sich nur für die currentXvelocity und currentYvelocity 

<p>The system will take the {@link VelocityComponent#currentXVelocity()} and {@link
 VelocityComponent#currentYVelocity()} and calculate the new position of the entity based on their
current position stored in the {@link PositionComponent}. If the new position is a valid
position, which means the tile they would stand on is accessible, the new position will be set.
If the new position is walled, the {@link VelocityComponent#onWallHit()} callback will be
executed.

Die Previous Geschichte ist ein helper, manchmal ist es gut zu wissen mit welcher Geschwindigkeit sich die Entität im Frame davor bewegt hat (z.B für Animationen).

Ich finde das in der Dokumentation auch entsprechend wieder, jetzt ist die Frage: Bin ich Betriebsblind oder du mit zu vielen Verhaltens-Annahmen unterwegs.

cagix commented 4 months ago

@AMatutat Ich würde natürlich sofort auf "betriebsblind" tippen 🤣

Dennoch: Die Tatsache, dass ich hier länger als 10 Minuten dran gesessen habe, um die Mechanismen nachzuvollziehen und zu schauen, welche der Methoden ich denn nun nehmen muss, und dass ganz offensichtlich die Javadoc in VelocityComponent nicht wirklich stimmt (oder zumindest was anderes meint als sie sagt) und dass die Zusammenhänge zwischen xVelocity und currentXvelocity nicht wirklich transparent sind ...