Du bist nicht angemeldet.

Stilllegung des Forums
Das Forum wurde am 05.06.2023 nach über 20 Jahren stillgelegt (weitere Informationen und ein kleiner Rückblick).
Registrierungen, Anmeldungen und Postings sind nicht mehr möglich. Öffentliche Inhalte sind weiterhin zugänglich.
Das Team von spieleprogrammierer.de bedankt sich bei der Community für die vielen schönen Jahre.
Wenn du eine deutschsprachige Spieleentwickler-Community suchst, schau doch mal im Discord und auf ZFX vorbei!

Werbeanzeige

Lares

1x Contest-Sieger

  • Private Nachricht senden

21

17.08.2014, 22:30

Ich habe mir den Code mal angesehen. Zunächst in der Datei MiniPong.cs:
Du verwendest viele if-Abfragen, jedoch kein else oder else if. Gerade bei der bei der Peddal-Steuerung ist mir das aufgefallen. Du musst bedenken jedes if wird pro Durchlauf überprüft, dabei könnte es durchaus Fälle geben, die sich ausschließen. Wenn ein Paddle sich nach oben bewegt, wird es bspw. nicht mit dem unteren Spielfeldrand kollidieren (außer natürlich, wenn es bereits im vorherigen Frame eine Kollision gibt).

Zu den Abfragen im Allgemeinen noch: Ich persönlich finde einzeilige Abfragen (also mit Bedingung und Ausführungscode) schwieriger zu lesen, als wenn man zwei Zeilen mit Einrückung verwendet. Deine Code-Basis wird so oder so schnell sehr groß werden, da ist Übersichtlichkeit generell besser, als Zeilen einzusparen. Bei der Steuerung der Paddles hast du es noch gemacht, danach bei der Prüfung mit dem Spielfeldrand jedoch nicht mehr.

Einige deiner Methoden haben auch einen eher suboptimalen Namen mMn. GameLoop() würde ich persönlich eher mit run() ersetzen. Ich wähle die Namen generell so, dass Methoden eher Verben oder Aktionen (getX, setX) darstellen. Nomen hebe ich mir für Objekte und Klassen auf.

Was mir noch aufgefallen ist: Du verwendest im Konstruktor von MiniPong den += Operator auf Methoden mit dem Rückgabewert void. Zudem hast du eine sehr starke Verschachtelung mit den verschiedenen Initialize-Methoden. Ich persönlich erkenne in beidem keinen wirklichen Mehrwert, aber da ich nicht primär in C# entwickle übersehe ich da vllt etwas.

Der letzte Punkt, den ich nicht so wirklich verstanden habe ist, was du genau mit mit dem Wert 750 bei der Berechnung der FPS in SfmlGame.cs erreichen willst. Soweit wie ich es verstehe möchtest du alle 750 Frames einen Text im Spiel mit der aktuellen FPS akualisieren. Da wäre es vermutlich besser, wenn du dieses Update auch zeitabhängig machst (jede Sekunde oder so).

Generell ist die Verwendung von SFMLGame als Container für sämtliche Logik etc. nach außen hin sinnvoll.
In dem jetzigen Spiel ist die Verwendung von eigenen GameObjects natürlich noch nicht erforderlich, da die benötigte Funktionen bereits in der SFML vorhanden sind (Sprite und Shape haben ja die gleichen Eltern). Bei größeren Projekten kann das aber durchaus erforderlich und sinnvoll sein.

22

17.08.2014, 22:47

1) Jo, bei einem größeren Projekt, wo der Rechenaufwand groß ist, würde ich das machen, hier ist jedoch der Rechenaufwand so minimal, dass ich der Meinung bin, dass diese Verkomplizierung des Codes nicht nötig ist.

2) Ich lasse bei If-Clauses mit nur einer Zeile Inhalt normalerweise die Klammern weg, da ich das für mich persönlich übersichtlicher finde. Wenn sich das nicht gehört, dann lasse ich das.

3) Get- und Set- Methoden würde ich eher durch Properties realisieren, wenn möglich. Mit GameLoop gebe ich dir aber Recht, das sollte ich umbenennen.

4) Der +=-Operator abboniert hier ein Event - ist daran etwas falsch? :huh: Das mit den Initialize-Methoden ist halt so, weil das Initialize-Event der SfmlGame-Klasse Argumente hat, welche ich nicht unbedingt nachbauen möchte, nur im alle Elemente zurückzusetzen. Deswegen habe ich eine weitere Methode erstellt, welche nur die benötigten Argumente übernimmt, sodass man sie von überall aus aufrufen kann.

5) Ja, du hast Recht, ich ändere das, sodass es auf der Zeit basiert.

6) Jo, das habe ich nur gemacht, damit man das alles einfach erweitern kann.

Lares

1x Contest-Sieger

  • Private Nachricht senden

23

17.08.2014, 23:25

2) Die {} Klammern kann man weg lassen. Ich meine nur es ist übersichtlicher (und was ich von anderen an Code gesehen hab auch gängiger) anstatt:

C-/C++-Quelltext

1
if(Bedingung) Anweisung;


folgendes zu schreiben

C-/C++-Quelltext

1
2
if(Bedingung)
   Anweisung;


3) Get und Set waren nur Beispiele, hätte vllt eher calculate() oder so nehmen sollen. Für get und set Properties zu verwenden ist soweit ich weiß guter C#-Stil (bin selber kein C#-Programmierer).

4) Ok wenn da tatsächlich eine Funktionalität hintersteht, ist das denke ich ok (wie gesagt bin kein C#-Programmierer und diese Schreibweise habe ich bei den SFML C++ Beispielen bisher nicht gesehen). Das sollten dann lieber andere beurteilen.

NachoMan

Community-Fossil

Beiträge: 3 885

Wohnort: Berlin

Beruf: (Nachhilfe)Lehrer (Mathematik, C++, Java, C#)

  • Private Nachricht senden

24

17.08.2014, 23:33

2) Die {} Klammern kann man weg lassen. Ich meine nur es ist übersichtlicher (und was ich von anderen an Code gesehen hab auch gängiger) anstatt:

C-/C++-Quelltext

1
if(Bedingung) Anweisung;


folgendes zu schreiben

C-/C++-Quelltext

1
2
if(Bedingung)
   Anweisung;

Das finde ich ebenfalls sehr wichtig.
Das ist ein Muster, das sich schnell ins gehirn einbrennt. Bei der ersten Variante sieht man nur eine lange Zeile...
"Der erste Trunk aus dem Becher der Erkenntnis macht einem zum Atheist, doch auf dem Grund des Bechers wartet Gott." - Werner Heisenberg
Biete Privatunterricht in Berlin und Online.
Kommt jemand mit Nach oMan?

Tobiking

1x Rätselkönig

  • Private Nachricht senden

25

18.08.2014, 01:28

Die MiniPong.cs sieht ziemlich wild aus. Die festen Werte im Code sind ein ziemliches Nogo was Flexibilität und Lesbarkeit angeht. Leg dafür Konstanten mit sprechenden Namen an. "WindowWidth - PaddleWidth" ist verständlich, "640 - 15" nicht.

Ich würde auch die meisten Rechnungen aus Funktions- und Konstruktoraufrufen rausnehmen. Sowas wie

C#-Quelltext

1
target.Add(new Ball(new Vector2f((640 - 10) / 2, (360 - 10) / 2)) { DirectionVector = new Vector2f((float)(random.NextDouble() * 1.2 - 0.6), (float)(random.NextDouble() * 1.5 - 0.75)) }, "Ball");

muss man komplett entwirren wenn man es verstehen möchte.

So sieht es zwar mehr aus, aber jeder Einzelschritt ist beim Überfliegen verständlich:

C#-Quelltext

1
2
3
4
5
6
7
8
9
10
11
12
float ballPositionX = (WindowWidth - Border) / 2; // Horizontal center of the screen
float ballPositionY = (WindowHeight - Border) / 2; // Vertical center of the screen

float ballDirectionHorizontal = (float)random....
float ballDirectionVertical = (float)random....

Ball ball = new Ball(new Vector2f(ballPositionX, ballPositionY))
{
    DirectionVector = new Vector2f(ballDirectionHorizontal, ballDirectionVertical);
}

target.Add(ball, "Ball");

Ich würde da sogar noch den Konstruktor von Ball so ändern, dass er direkt einen Positions- und Richtungsvektor annimmt und den Object Initializer sparen.

4) Ok wenn da tatsächlich eine Funktionalität hintersteht, ist das denke ich ok (wie gesagt bin kein C#-Programmierer und diese Schreibweise habe ich bei den SFML C++ Beispielen bisher nicht gesehen). Das sollten dann lieber andere beurteilen.

Das ist so in C# legitim. Dort wird ein Delegate einem Event hinzugefügt. In C++ Jargon wäre das Event eine Liste von Funktionspointern/Functors, die dann als Callback benutzt werden. In C# sind Events und das += syntaktischer Zucker, der den entsprechenden Code dafür generiert.

Werbeanzeige