Schiffe versenken - Wie prüfen, ob ein Schiff genau daneben ist?

Hi, erstmal danke an euch beiden. Vor allem dir volkard für deine
ausführliche Antwort, zu der ich jetzt mal genauer drauf eingehen möchte.

Erstmal, ich hab vergessen zu erwähnen, dass ich es mit der WinAPI programmiere.
Vielleicht sollte ich es lieber im WinApi-Forum posten, also bitte verzheit.

Och, die Laufzeit würde mich da eher weniger jucken. Das ist alles sauschnell.

Laufzeit ist hier völlig irrelevant. Ob der Benutzer 100 oder 500 Mikrosekunden warten muss, er kann es nicht bemerken.

Das ist bei mir leider nicht so der Fall. Beim starten muss ich mehrere Sekunden
warten, bis das Spielfeld + die Schiffe des Gegners gesetzt wurden. Das liegt
wahrscheinlich an folgendem Code, den ich benutze, um die Schiffe zusetzen:

void SchiffPlatzieren(short schiff, bool lage)
{
	int start, i;
	bool test = false;

	while(test != true)
	{
		start = zufall();
		if(lage == 0)
		{
			if(index[start-1] == false && index[start-10] == false  && index[start+10] == false)
				test = true;
			else
				continue;

			for(i = 0; i <= schiff; i++)
			{
				if(index[start+i] == false)
					test = true;
				else
					continue;
			}
		}
		else
		{
			if(index[start-10] == false && index[start-1] == false && index[start+1] == false)
				test = true;
			else
				continue;

			for(i = 0; i <= schiff*10; i += 10)
			{
				if(index[start+i] == false)
					test = true;
				else
					continue;
			}
		}
	}

	if(test == true)
	{
		if(lage == 0)
		{
			for(i = 0; i < schiff; i++)
				index[start+i] = true;
		}
		else
		{
			for(i = 0; i < schiff*10; i += 10)
				index[start+i] = true;
		}
	}
}

Kurz erklärt:
start -> Zufällig generierte Start-Koordinate
i -> Hilfsvariable für Schleifen
test -> Wenn test am Ende true ist, wird das Schiff platziert.
lage -> Die lage soll die Lage beschreiben(0 = Horizontal, 1 = Vertikal)

Ich bin kein Profi, von daher würde ich mich freuen, wenn eure Kritik schonend
rüberkommen würde

Ich benutze diese Funktion, um die Schiffe zufällig zu platzieren, und dabei
die erforderlichen abfragen zu machen. Das mit den blauen Tupfen, sollte
in dem Bild eigentlich nur zeigen, welche Kästchen gemeint sind, aber deine
Idee

Laufe über die vier Nachbarfelder des Roten Kreises und
wenn da kein Roter Kreis ist, setze da einen Blauen Tupfen.

wäre nicht schlecht. Genauso, wie das mit dem "Todesrand".

@hustbaer
Ich mache das, um das gelernte in einem kleinen Programm umzusetzen und zu vertiefen.

Ich bitte euch nochmals, ich bit kein Profi, also bitte Kritik schonend
äußern ;), danke!

Oh, habe eine kleinigkeit vergessen, folgendes zu erklären:

...start+1 -> Der Button, der rechts neben dem Startbutton ist(Horizontal).
...start+10 -> Der Button, der unter dem Startbutton ist(Vertikal).

Die index[] Variable ist ein Bool-Array, -> True = Schiff, False = Kein Schiff

Th69

Hast du in deiner Zufallsfunktion srand(time(0)) drin? Dann würde es mich nicht wundern, wenn deine Routine mehrere Sekunden dauert...

@Th69
Nein, hab kein time(0) drin. Hab es so:

time_t t;

    time(&t);
    srand((unsigned int)t);

camper

in zufall() ?

camper schrieb:

in zufall() ?

Wieso? Verstehe nicht ganz!

camper

Th69 schrieb:

Hast du in deiner Zufallsfunktion srand(time(0)) drin? Dann würde es mich nicht wundern, wenn deine Routine mehrere Sekunden dauert...

helpplease schrieb:

@Th69
Nein, hab kein time(0) drin. Hab es so:
time_t t;

    time(&t);
    srand((unsigned int)t);

camper schrieb:

in zufall() ?

==> Befindet sich dieser Code in der Zufallsfunktion zufall?

@camper
Achso, ja. Das befindet sich in zufall(). Aber das ist doch nun wirklich mehr als offensichtlich, denn

time_t t;

    time(&t);
    srand((unsigned int)t);

Initialisiert ja den Zufallsgenerator.

Nathan

Was man ein einziges Mal tun sollte - am Beginn der main().

LordJaxom

helpplease schrieb:

denn [...] Initialisiert ja den Zufallsgenerator.

Genau, initialisiert. Einmal. Initial.

@LordJaxom && Nathan

Stimmt, muss ich ja ändern, da ich den Zufallsgenerator in der Funktion Initialisiere.
Ich muss das auserhalb nur einmal machen, danke fürs korrigieren, doch denkt
ihr, es ist deswegen so langsam?

LordJaxom

helpplease schrieb:

@LordJaxom && Nathan

Stimmt, muss ich ja ändern, da ich den Zufallsgenerator in der Funktion Initialisiere.
Ich muss das auserhalb nur einmal machen, danke fürs korrigieren, doch denkt
ihr, es ist deswegen so langsam?

Da die Zufallsfunktion eine Sekunde lang immer die gleiche Zufallszahl liefert: ja. (time liefert einen Zeitstempel in Sekunden)

@LordJaxom

Du hast recht. Jetzt geht es ziehmlich schnell, doch ich hab jetzt ein anderes
Problem. Jetzt setzt er nicht mehr alle Schiffe, sondern nur ein paar bestimmte.
Zum Beispiel 2 Zwei-Kästchen und den fünfer. Mehr nicht. Das einzige, was ich jetzt geändert habe, ich habe die Initialisierung des Zufallsgenerators aus
der Funktion rausgenommen und in die WinMain eingefügt, noch bevor die
Funktion zufall() das erste mal aufgerufen wird! Hast du eine Idee?

Hier noch mal etwas genauer:

Funktion zufall()

int zufall()
{
	izeit = 145 + ( rand() % 100 ); 
	return izeit;
}

Ich habe allerdings schon vor ein paar stunden die SchiffPlatzieren() geändert:

Funktion SchiffPlatzieren()

void SchiffPlatzieren(short schiff, bool lage)
{
	int start, i;

	while(true)
	{
		start = zufall();
		if(lage == 0)
		{
			if(sperre[start])
				continue;
			for(i = 0; i < schiff; i++)
				if(index[start+i])
					continue;
			if(sperre[start-1] || sperre[start+schiff-1] || sperre[start-12] || sperre[start+12])
				continue;

			for(i = 0; i < schiff; i++)
			{
				index[start+i] = true;
			}
			sperre[start-1] = true;	
			sperre[start+schiff-1] = true;
			sperre[start-12] = true;
			sperre[start+12] = true;

			break;
		}
	}
}

"sperre" ist ein Bool-Array, worin ich die umliegenden Felder eines Schiffes markiere!

volkard

Th69 schrieb:

Hast du in deiner Zufallsfunktion srand(time(0)) drin? Dann würde es mich nicht wundern, wenn deine Routine mehrere Sekunden dauert...

Gute Kristallkugel!

volkard

helpplease schrieb:

Hast du eine Idee?

Meine allererste Idee ist, die Windows-Sachen wegzumachen und das Programm erstmal als Konsole-Anwendung zu bauen. Die zweite wäre, alles wegzuwerfen und neu anzufangen, diesmal mit kleineren Funktionen, lokaleren Variablen, treffenderen Bezeichnern und tatsächlich (temporär) goto, wenn du wirklich goto meinst.

...tatsächlich (temporär) goto, wenn du wirklich goto meinst.

Wie kommst du jetzt auf goto? Davon hab ich nichts erwähnt!

Und wieso alles von neu machen? Jetzt, wo ich, was die Geschwindigkeit angeht
schon einen Schritt nach vorne gemacht habe! Wenn man alles von vorne beginnt,
dann leidet darunter zum einen die Motivation und zum anderen auch der Lehrerfolg.
Denn es ist doch besser, schlechten Code zu Optimieren und sich immer weiter zu
verbessern, als wieder von neu anzufangen!

Gute Kristallkugel!

Was meinst du? ich hatte in srand() kein time(0) drin!

helpplease schrieb:

...tatsächlich (temporär) goto, wenn du wirklich goto meinst.

Wie kommst du jetzt auf goto? Davon hab ich nichts erwähnt!

Mit goto kannst Du aber bequem aus einer inneren Schleife von mehreren verschachtelten Schleifen springen.
Mit continue und break ist das nicht so einfach. Siehe zum Beispiel Deinen Code:

for(i = 0; i < schiff; i++)
  if(index[start+i])
    continue;  // quark

Gemeint hast Du wohl:

while(true){
    loop:
      start=zufall();
      //...
      for(i = 0; i < schiff; i++)
        if(index[start+i])
          goto loop;
    //...
  }

Und ganz eigentlich wolltest Du das testen der einzelnen Felder in eine eigene Funktion auslagern.

camper

void SchiffPlatzieren(short schiff, bool lage) // schiff == Länge,  lage == horizontal/vertkal?
{
	int start, i; // Definition besser erst später

	while(true)
	{
		start = zufall();
		if(lage == 0)
		{
			if(sperre[start])
				continue;
// prüft, ob Schiffsfelder schon durch anderes Schiff belegt
// was ist, wenn es sich um benachbarte Felder (sperre) handelt?
			for(i = 0; i < schiff; i++)
				if(index[start+i])
					continue;  // wurde schon kritisiert
// im Allg. mehr als 4 benachbarte Felder
// und warum genau sollte die Belegung fehlschlagen, wenn ein freies Feld zufällig schon Nachbar eines anderen Schiffes ist?
			if(sperre[start-1] || sperre[start+schiff-1] || sperre[start-12] || sperre[start+12])
				continue;
			for(i = 0; i < schiff; i++)
			{
				index[start+i] = true;
			}
// das ist seltsam: ein Schiff hat ja in der Regel mehr als 4 benachbarte Felder eher: 2*Länge+2 od 2*Länge+6 falls Ecken zählen
			sperre[start-1] = true;	
			sperre[start+schiff-1] = true;
			sperre[start-12] = true;
			sperre[start+12] = true;

			break;
		}
	}
}

Die Verwendung zweier Felder scheint nicht besonders sinnvoll. Ein einzelnes Feld mit 3 Zuständen je Feld (leer, benachbart, belegt) dürfte sinnvoller sein. Das vereinfacht auch die Logik: damit ein Schiff gesetzt werden kann, müssen alle Felder, die es belegt, leer sein.