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

[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.

[Danny92]

Okay jetzt habe ich die Speicherlecks gefunden. Einmal hab ich in TFlotte.Destroy vergessen, die einzelnen Schiffe im Array wieder freizugeben.
Und das andere Leck war etwas schwieriger zu finden: Im Destroy-Ereignis der Form hat noch ein

Delphi-Quellcode:

BTMemoryFreeLibrary(mp_MemoryModule);
  FreeMemory(mp_DllData);

gefehlt.

[haentschman]

Hallöle...

Zitat:

Ja cnPack hab ich soeben schon mal installiert.

Ein großer Werkzeugkasten, wo man nicht alles braucht, der dir aber das Leben erleichtert. Ein paar Optionen in der IDE solltest du deakivieren. (Optionen1+2)
1: Die Linien von Castalia können weg, es sein denn du magst die dünnen Linien lieber. Dann mußt du die Linien im CnPack abschalten.
2. Programmierhilfe: Die vom CnPack ist deutlich besser. Die findet auch Teilstrings. Nur doppelt suchen ist langsam...

Zitat:

Duplicate Code Finder 1.0 konnte ich lediglich eine Stelle in der SetzeFlotte-Prozedur finden, die sich wiederholt

DRY ist was anderes. Du hast Glück. Für meine Sprachtherapie habe ich einen Mini Vortrag über DRY gehalten. (Anhang) Fällt dir zwischen Seite 5 und 6 was auf?



Nachtrag DRY:

aus deinem Code:

Delphi-Quellcode:

procedure TForm1.LeuchteKnopfTimer(Sender: TObject);
begin
  if not playerhasshot
  then
  if blinkcount<25 then
  begin
    if BoxPlayer.Color=clRed
    then BoxPlayer.Color:=clNavy
    else BoxPlayer.Color:=clRed;
    LeuchteKnopf.Interval:=random(80)+21;
    Inc(blinkcount);
  end else
  begin
    blinkcount:=0;
    LeuchteKnopf.Enabled:=False;
    LeuchteKnopf.Interval:=1;
    BoxPlayer.Color:=clNavy;
  end
  else
  if blinkcount<25 then
  begin
    if BoxKI.Color=clRed
    then BoxKI.Color:=clNavy
    else BoxKI.Color:=clRed;
    LeuchteKnopf.Interval:=random(80)+21;
    Inc(blinkcount);
  end else
  begin
    blinkcount:=0;
    LeuchteKnopf.Enabled:=False;
    LeuchteKnopf.Interval:=1;
    BoxKI.Color:=clNavy;
  end;
end;

besser:

Delphi-Quellcode:

procedure TForm1.LeuchteKnopfTimer(Sender: TObject);
begin
  if not playerhasshot then
  begin
    LightingButton(BoxPlayer);
  end
  else
  begin
    LightingButton(BoxKI);
  end
end;

ausgelagert z.B. als private procedure:

Delphi-Quellcode:

procedure TForm1.LightingButton(GroupBox: TGroupbox);
begin
  if blinkcount < 25 then
  begin
    if GroupBox.Color = clRed then
      GroupBox.Color := clNavy
    else
      GroupBox.Color := clRed;
    LeuchteKnopf.Interval := random(80) + 21;
    Inc(blinkcount);
  end
  else
  begin
    blinkcount := 0;
    LeuchteKnopf.Enabled := False;
    LeuchteKnopf.Interval := 1;
    GroupBox.Color := clNavy;
  end;
end;