Tipps und Ratschläge für Spiel & für effizientes Programmieren

[Danny92]

Jetzt habe ich das Schiffe versenken soweit fertig gestellt und stelle es euch hier zur Verfügung. Ich bin für jeden Tipp, Verbesserungsvorschlag, konstruktive Kritik oder neuen Einfall jederzeit dankbar. Schlussendlich geht es mir neben der Kritik nun nur noch darum, etwas an Performance rauszuholen, denn bei aktivem Spielen mit der KI läuft meine - wenn auch nicht gerade schnellste - CPU immerhin schon bei ca. 33%. Ist das eurer Meinung nach normal bei einem Programm diesem Formats? Oder ist das schlichtweg "schlecht" programmiert? Wie sieht eure CPU-Auslastung aus? Denn bis jetzt konnte ich nicht erkennen woran das liegt, da ich mit effizientem Programmieren noch nicht so vertraut bin bzw. es selten das Problem war. Das Neuzeichnen der StringGrids oder das Vorausberechnen des nächsten Schusses der KI scheint es schon mal nicht zu sein.
Vielen Dank für eure kompetente Hilfe!

PS: Da das Programm wegen der Resource zu groß ist, muss es hier verlinkt werden. Minen sind noch nicht implementiert.

[Glados]

Probier mal negaH's Delay aus. Vielleicht bringts ja was. http://www.delphipraxis.net/6620-delay.html

[Danny92]

Jaaa die hab ich auch gerade gefunden. Na super, da sinkt die Auslastung der CPU gleich mal um lockere 20 Prozentpunkte...Einwandfrei.

[Aviator]

Nabend,

also das mit dem Delay hast Du ja schon rausgefunden. Das ist eigentlich der Auslöser für die mega CPU Auslastung.

Vielleicht könntest Du, um von der Delay Methode wegzukommen, einen Standard-Timer starten der in einem 25ms Interval läuft. In der OnTimer Methode zählst du dann bspw. die Durchläufe. Je nach Anzahl der Durchläufe befindest du dich einem definierten Zustand und löst das Repaint Event des entsprechenden Grids aus. Im Grid wird der Status abgefragt und das entsprechende Image gemalt.

Kann man sicherlich ausbauen, aber es wäre mal ein Anfang. Zumindest ballerst Du dein Programm nicht mit der Messageverarbeitung voll was in dem Fall dann schon kontraproduktiv ist.


Zudem lädst Du immer wieder die Images aus einer Ressource in ein TImage. Das könntest Du einmalig beim Start des Spiels bzw. beim Programmstart machen. Das würde dann auch nochmal ein kleines bisschen Geschwindigkeit bringen.

Aber ansonsten schönes Spiel für den Anfang.


Allerdings muss ich mich trotzdem ziemlich durch den SourceCode kämpfen. Ich glaube wenn Du das Programm jetzt nochmal im Gesamten überarbeiten würdest und weißt worauf Du achten musst, dann wäre es sicherlich um einige Zeilen und Hilfsvariabeln kürzer. Aber perfekt lesbaren SourceCode schreiben kann sowieso niemand. Man verbessert sich nur mit der Zeit. Und ich würde sagen, dass Du da auf einem guten Weg bist. Weiter so

Noch ein Tipp: Beachte die Meldungen die Dir der Compiler auswirft und bearbeite die so schnell wie möglich. Wenn es irgendwann mal 100 oder gar 1000 sind, dann hast Du da kein Bock mehr drauf.

Zitat von Compiler:

[dcc32 Warnung] USchiff.pas(18): W1010 Methode 'Destroy' verbirgt virtuelle Methode vom Basistyp 'TObject'
[dcc32 Warnung] USchiff.pas(65): W1036 Variable 'index' ist möglicherweise nicht initialisiert worden
[dcc32 Hinweis] USchiff.pas(15): H2219 Das private-Symbol 'TrefferSindZusammenhaengend' wurde deklariert, aber nie verwendet
[dcc32 Warnung] UFlotte.pas(31): W1010 Methode 'Destroy' verbirgt virtuelle Methode vom Basistyp 'TObject'
[dcc32 Warnung] UFlotte.pas(325): W1036 Variable 'j' ist möglicherweise nicht initialisiert worden
[dcc32 Hinweis] Unit1.pas(154): H2164 Variable 'FirstTickCount' wurde deklariert, aber in 'TForm1.Delay' nicht verwendet
[dcc32 Hinweis] Unit1.pas(185): H2164 Variable 'rs' wurde deklariert, aber in 'TForm1.spieleFXab' nicht verwendet
[dcc32 Hinweis] Unit1.pas(186): H2164 Variable 'extension' wurde deklariert, aber in 'TForm1.spieleFXab' nicht verwendet
[dcc32 Hinweis] Unit1.pas(617): H2077 Auf 'spielende' zugewiesener Wert wird niemals benutzt
[dcc32 Hinweis] Unit1.pas(659): H2077 Auf 'spielende' zugewiesener Wert wird niemals benutzt
[dcc32 Hinweis] Unit1.pas(1056): H2164 Variable 'ResStream' wurde deklariert, aber in 'TForm1.FormCreate' nicht verwendet
[dcc32 Hinweis] Unit1.pas(79): H2219 Das private-Symbol 'm_DllDataSize' wurde deklariert, aber nie verwendet
[dcc32 Hinweis] Unit1.pas(80): H2219 Das private-Symbol 'mp_DllData' wurde deklariert, aber nie verwendet
[dcc32 Hinweis] Unit1.pas(81): H2219 Das private-Symbol 'mp_MemoryModule' wurde deklariert, aber nie verwendet

[Danny92]

Jaa, vielen Dank für diese glorreichen Worte. Ja die Explosion.gif wird immer aus der Ressource geladen das kann ich noch verbessern. Bloß mit dem Timer im 25ms Intervall verstehe ich noch nicht so ganz. Sinn und Zweck von Delay ist ja nicht, ein Grid irgendwann neu zu zeichnen, sondern die Ausführung an Ort und Stelle im OnMouseDown-Ereignis kurzzeitig zu pausieren, bis die an der Stelle aufgerufenen Soundeffekte abgespielt sind, denn sonst hört man doch z.B. schon den Schuss des Gegners, wo ich doch gerade erst getroffen habe. Wenn ich anstelle von Delay nun einen Timer setze, dann pausiert dieser doch nicht die Ausführung des Codes? Ganz am Anfang habe ich mal mit Timer rumgespielt, bis ich am Ende auch 5-6 Stück hatte, die sich dann nur noch gegenseitig aufgerufen, an- und abgeschalten haben, und es war ein völliges Chaos und total verwirrend. So finde ich das nun eigentlich ganz elegant und effizient gelöst...naja effizient; jedenfalls geht die CPU-Auslastung schon mal in die richtige Richtung...

Und die Compilerhinweise werde ich natürlich noch in Angriff nehmen, bisher war das Programm nur eine einzige Baustelle in dem gar nix ging. Dies ging nicht und das ging nicht. Ich hab mich von einem Problem zum nächsten programmiert. Vor 2 Monaten konnte ich noch nicht ahnen, eine dll Bibliothek in eine Ressource zu laden, um damit Soundeffekte abzuspielen^^

[Danny92]

Im OnCreate-Ereignis habe ich hinzugefügt:

Delphi-Quellcode:

  for i:=low(explosion) to high(explosion) do
  begin
    explosion[i].LoadFromResourceName(hInstance,'exp'+IntToStr(i));
  end;

Sowie im TExplosionTimer habe ich
Image1.Picture.Bitmap.LoadFromResourceName(hInstance,'exp'+IntToStr(expindex)); durch
image1.Picture.Bitmap.Assign(explosion[expindex]); ersetzt. Funktioniert im OnCreate schon nicht. Explosion ist ein Array[0..12] of TBitmap. Ist das nicht ansatzweise richtig?

[Ghostwalker]

Was gibts für eine Fehlermeldung/Fehlverhalten ?

[TiGü]

Zitat von Danny92:

image1.Picture.Bitmap.Assign(explosion[expindex]); ersetzt. Funktioniert im OnCreate schon nicht. Explosion ist ein Array[0..12] of TBitmap. Ist das nicht ansatzweise richtig?

Machst du denn vorher irgendwo sowas wie:

Delphi-Quellcode:

for i := Low(explosion) to High(explosion) do
begin
  explosion[I] := TBitmap.Create;
end;

Du musst die Platzhalter im Array schon mit Leben füllen.

[Danny92]

Natürlich das habe ich vergessen

[haentschman]

Hallo Danny...

Erst mal Glückwunsch, daß du ein Programm, was funktioniert, fertiggestellt hast.

Hinter den Kulissen hast du allerdings ein paar Fehler. Du hast selbst gesagt du willst dich verbessern.

Hinweise:
1: StyleGuide mal durcharbeiten
2: Dich mit DRY dringend beschäftigen.
3. Nicht alle Dateien in einem Ordner. Besser eine Ordnerstruktur für dich definieren und als Template für neue Projekte verwenden.
(Beispiel: siehe Bild1)
4. Codeformatter benutzen. (z.B. CnPack)
5. kein WTITH im Quelltext (Bild4)
6. Hinweise des Beitrages #10, Punkte 1-4 http://www.delphipraxis.net/1379518-post10.html durcharbeiten.
* Styleguide der 2.
* DRY (Bild3) Hier kann man eine procedure machen mit den entsprechenden Parametern.
* KISS
* CamelCase
* Denglisch bitte vermeiden.

Delphi-Quellcode:

...
function getLaenge: integer;

besser

Delphi-Quellcode:

...
function IsShip(a: TPoint): Boolean;

Verbesserungen:
* ReportMemoryLeaksOnShutdown := True; in die Projektdatei aufnehmen.
* mehrere Klassen können auch in der selben Unit stehen.
* Sprechende Namen verbessern. Heute haben wir keine Speicherprobleme mehr.
* Formatter immer benutzen
* then immer in der gleichen Zeile wie if (siehe Styleguide)

Delphi-Quellcode:

if a.X=b.X then
begin
  vmin:=min(a.Y,b.Y); vmax:=max(a.Y,b.Y);
  laenge:=vmax-vmin+1;
  if (laenge>5) or (Length(vSchiff[laenge])>=flottengroesse[laenge]) then
    result:=false
  else
  begin
    abbruch:=false;
    i:=vmin;
    repeat
      if feld[a.X-1,i-1]=1 then abbruch:=true;
      Inc(i);
    until abbruch or (i>vmax);
    i:=vmin;
    if a.X>1
    then
      repeat
        if feld[a.X-2,i-1]=1 then abbruch:=true;
        Inc(i);
      until abbruch or (i>vmax);
    i:=vmin;
    if a.X<feldgroesse
    then
      repeat
        if feld[a.X,i-1]=1 then abbruch:=true;
        Inc(i);
      until abbruch or (i>vmax);
    if vmin>1 then
      if feld[a.X-1,vmin-2]=1 then abbruch:=true;
    if vmax<feldgroesse then
      if feld[a.X-1,vmax]=1 then abbruch:=true;
    if abbruch
    then result:=false
    else result:=true
  end;
end else

..wird:

Delphi-Quellcode:

  if a.X = b.X then
  begin
    vmin := min(a.Y, b.Y);
    vmax := max(a.Y, b.Y);
    laenge := vmax - vmin + 1;
    if (laenge > 5) or (Length(vSchiff[laenge]) >= flottengroesse[laenge]) then
      result := false
    else
    begin
      abbruch := false;
      i := vmin;
      repeat
        if feld[a.X - 1, i - 1] = 1 then
          abbruch := true;
        Inc(i);
      until abbruch or (i > vmax);
      i := vmin;
      if a.X > 1 then
        repeat
          if feld[a.X - 2, i - 1] = 1 then
            abbruch := true;
          Inc(i);
        until abbruch or (i > vmax);
      i := vmin;
      if a.X < feldgroesse then
        repeat
          if feld[a.X, i - 1] = 1 then
            abbruch := true;
          Inc(i);
        until abbruch or (i > vmax);
      if vmin > 1 then
        if feld[a.X - 1, vmin - 2] = 1 then
          abbruch := true;
      if vmax < feldgroesse then
        if feld[a.X - 1, vmax] = 1 then
          abbruch := true;
      if abbruch then
        result := false
      else
        result := true
    end;
  end
  else

Fehler:
1. Warnungen abarbeiten
Dringend:

Zitat:

[dcc32 Warnung] USchiff.pas(18): W1010 Methode 'Destroy' verbirgt virtuelle Methode vom Basistyp 'TObject'

Zitat:

[dcc32 Warnung] UFlotte.pas(31): W1010 Methode 'Destroy' verbirgt virtuelle Methode vom Basistyp 'TObject'

destructor Destroy; override; Daher kommen auch die Ressource Leaks. (Bild2)

...ansonsten weiter so. Du hast ja das Forum als Hilfe.

[Danny92]

Ja cnPack hab ich soeben schon mal installiert. Was soll ich sagen? Es sieht schon mal prima aus! Im Destroy-Ereignis gebe ich die Bitmap im Array wieder frei. Damit konnte ich die Speicherleaks auf folgende im Anhang reduzieren. Aber was ist der Rest? Ich vermute mal, das hat wieder irgendwas mit der Bass.dll aus der Ressource zu tun. Ich hab mich damit wie gesagt noch nie so intensiv beschäftigt...
Mit dem StyleGuide werde ich mir mal intensiver beschäftigen. Und mit einem sog. Duplicate Code Finder 1.0 konnte ich lediglich eine Stelle in der SetzeFlotte-Prozedur finden, die sich wiederholt; das werde ich auch noch besser machen und einfach ein Array draus machen, in dem ich beide Flotten speichere. Dann fallen die unterschiedlichen Bezeichner weg, was das Verdoppeln überflüssig macht.

Aber die Speicherleaks sind mir gerade noch ein Rätsel...
PS: Denglisch war schon oft mein Problem.