C# Infinite Loop?

Werbeanzeige

MonkaBoy

Frischling

Beiträge: 21

23.07.2020, 16:32

C# Infinite Loop?

Hallo Leute,

ich bekomme einfach Haarausfall bei dieser Loop, ich weiß nicht wieso die ifinite läuft.

schaut hier:

C#-Quelltext

      public static IEnumerator RemoveAllPickupsOnDeadGround(object sender, TaskEventArgs e)
      {
         var begin = e.BeginOfBrokenGround;
         var end = e.EndOfBrokenGround;
         var size = Helper.GetSize(begin, end);
         int max = size.x * size.y;              
         //int tmp = 0;

         for (int i = 0; i < max; i++)
         {
            int rndX = UnityEngine.Random.Range(begin.x, end.x + 1);
            int rndY = UnityEngine.Random.Range(begin.y, end.y + 1);
            var pos = new GridCell(rndX, rndY, 0);
            var pickup = PLACING_MAP.GetTile<ItemTile>(pos);

            if (pickup == null)
            {
               i--; //somehow this code turns loop into infinite loop
               Debug.Log($"null at {pos} with iteration count: {i}");
               continue;
            }
          yield return new WaitforSeconds(..)
            //mehrzeiliger remove code, der nicht von bedeutung ist!

       }

Die Function läuft wunderbar und tut das was Sie soll bis, sagen wir, 2/3 der Elemente..

Ich habe iwie das Gefühl, die positionen die berechnet werden, iwie ausgeschöpft sind, aber iwie sieht er die restlichen 1/3 der Elemente nicht
(siehe Bild, wie das ungefähr zur Laufzeit aussieht)

»MonkaBoy« hat folgendes Bild angehängt:

Dieser Beitrag wurde bereits 1 mal editiert, zuletzt von »MonkaBoy« (23.07.2020, 16:46)

Zum Seitenanfang

MonkaBoy

Frischling

Beiträge: 21

23.07.2020, 16:47

Ich will anmerken um ganz sicher zu gehen für die die es lesen, meine random positions sind NICHT von "i" abhängig, i soll nur bis max laufen und in abhängigkeit dessen will ich random werte aufbauen, aber nicht abhängig von "i" direkt sondern nur damit die loop endet. Sonst sollen halt alle möglichen positionen abgefrühstückt werden

Zum Seitenanfang

Sylence

Community-Fossil

Beiträge: 1 663

Beruf: Softwareentwickler

23.07.2020, 16:56

Je mehr pickups du entfernst, desto unwahrscheinlich wird es, dass du zufällig eine Koordinate bekommst, an der eins liegt.

Hol dir eine Liste aller Pickups und entferne aus dieser Liste max zufällige Einträge. Das ist deutlich effizienter.

Zum Seitenanfang

MonkaBoy

Frischling

Beiträge: 21

23.07.2020, 16:58

@silence:

erstmal danke für den Input.

Aber, das erklärt dennoch nicht die infinite loop, es würde bissl länger dauern ja, aber nicht freezen oder?

Zum Seitenanfang

Audori

Frischling

Beiträge: 40

23.07.2020, 19:25

	C#-Quelltext
1 2 3 4 5 6	if (pickup == null) { i--; //somehow this code turns loop into infinite loop Debug.Log($"null at {pos} with iteration count: {i}"); continue; }

Da ich nicht genug über die Daten in deinem Spiel weiß, kann ich es nicht beurteilen... aber, bist du dir wirklich sicher, dass du hier "i--;" machen möchtest?

Angenommen an dieser Stelle des Codes hätte i den Wert 27. Dann machst du i-- wodurch es dann 26 wird. Danach erfolgt "countinue". Was dazu führt das sofort das Programm wieder oben bei for, den nächsten Schleifendurchlauf beginnt. Es wird also for(... ; i++) ausgeführt, und die Schleife wird mit "i = 27" erneut durchlaufen. Ich kann es nicht beurteilen... ist das wirklich deine Absicht?

Besonders weil ...
Deine Abbruchbedingung ist "i < max" ... und max = size.x * size.y;

Ist dir klar wie lange er versucht ERFOLGREICH (also ohne in i--; countinue rein zu rutschen) die Schleife zu durchlaufen?
Sagen wir mal size.x ist 15 und size.y ist 10. Dann wäre max = 150.
Er würde also nicht 150x die Schleife durchlaufen, sondern so oft bis er 150 mal ERFOLGREICH war (ohne i--; countinue). Und beim letzten Teil das er finden könnte, wäre die Chance das Random das richtige bringt nurnoch 1 zu 149! Bedeutet, beim letzten zu findenden Get() wird er mit Random extrem ineffizient sein.
Bedeutet, er wird viel viel öfter die Schleife durchlaufen als nur die 150x. Und wenn er es nicht schaft 150x erfolgreich mit "pickup = PLACING_MAP.GetTile<ItemTile>(pos);" ein Teil zu erhalten (weil es weniger als 150 zu findende Teile gibt), dann wird er niemals fertig! Dein infinite loop?

Das ist so ineffizient, da wäre sogar dann dies hier effizienter:

	C#-Quelltext
1 2 3 4 5 6 7 8	for(int y = 0 ; y < size.y ; ++y) for(int x = 0 ; x < size.x ; ++x) { var pos = new GridCell(x, y, 0); var pickup = PLACING_MAP.GetTile<ItemTile>(pos); if(pickup == null) continue; }

In diesem Beispiel würde er zwar immer ganz genau (size.x * size.y) mal die Schleife durchlaufen (und jedes Feld einzeln prüfen). Aber garantiert nicht öfters als eben ganz genau (size.x * size.y) mal. Und damit würde er praktisch garantiert immer weniger Durchläufe brauchen als bei deiner ursprünglichen Schleife wo er (size.x * size.y) mal ERFOLGREICH sein will + "Fehlschläge wo Random Pos eine Niete gezogen hat".

Zur optimierung würde ich dann eher versuchen, innerhalb der Schleife(n) möglichst wenige Funktionsaufrufe (auch Random ist ein Funktionsaufruf der was kostet) und möglichst wenige 'new' und Codeverwzeigungen (if, else) zu machen.

Wie oft rufst du RemoveAllPickupsOnDeadGround() überhaupt auf? Aufgrund des Namens der Funktion habe ich da so einen verdacht

- Mehrmals je Frame? Also mehrmals in verschiedene Update() -> Dann überlege dir wie du 'das Ergebnis' merken kannst, damit du höchstens noch 1x pro Frame diese Funktion aufrufen musst. DAS wäre dann eine Optimierung die sich wirklich lohnt!

- Jeden Frame genau 1 aufruf von RemoveAllPickupsOnDeadGround() -> Muss das sein? Oder reicht es auch wenn du nur bei bestimmten Ereignissen, dann RemoveAllPickupsOnDeadGround() aufrufst, weil sich Zwischenzeitlich nichts relevantes ändert?
Wenn es nicht anders geht und jeden Frame 1x sein muss ...ok, schade. Dann so wie oben beschrieben, versuchen die for Schleife(n) zu optimieren. Und dann ersteinmal gut sein Lassen. Vermutlich wird es sowieso schon schnell laufen.

- Seltener als 1x jeden Frame? -> Dann Pfeif drauf. Es wird garantiert! schnell genug sein! Jeder weitere Gedanke ist verschwendet. Programmiere lieber an anderen Stellen deines Spiels weiter

Dieser Beitrag wurde bereits 1 mal editiert, zuletzt von »Audori« (23.07.2020, 19:35)

Zum Seitenanfang

MonkaBoy

Frischling

Beiträge: 21

23.07.2020, 19:37

Erstmal danke ich Euch sehr für die Mühe, weiß ich sehr zu schätzen!

Nun zur Sache, um es kurz zu halten, was ich EIGENTLICH nur machen will ist FÜR JEDES GEFUNDENE PICKUP innerhalb von "DeadGround" es an EINER ZUFÄLLIG BERECHNETEN stelle es zu deleten, das heisst, wenn man das Grid da normal durchläuft mit 2 nested loops, dann will ich nicht immer an (x, y) deleten sondern einfach innerhalb der Bounds (sagen wir: 10 * 15 felder) einen zufälliges pickup deleten.

Zum Seitenanfang

Audori

Frischling

Beiträge: 40

23.07.2020, 20:25

Verstehe ich das richtig? Wenn er z.B. 5 Teile findet, dann willst du 5 Teile löschen?

Ist das was er sucht (du bezeichnest es als PICKUP), denn etwas anderes als das was er löschen soll?

Denn wenn das zu löschende, von der gleichen Art wäre wie das wonach er sucht.... dann wäre es effektiv gesehen ein: Lösche Alle.
Womit Random nurnoch die Lösch-Reihenfolge ändern könnte, aber das Ergebnis das gleiche wäre: Nichts bleibt übrig.

Anders gefragt, wieviele möchstest du denn löschen? und/oder wieviele gibt es denn von den PICKUP?
Weißt du vorher wieviele PICKUP es gibt? Und (wenn das zu löschende auch PICKUP's sind), wieviele davon möchtest du löschen?

Mal davon ausgehend, dass das wonach du suchst, das gleiche wäre wie das was du löschen willst. Und das du einen Zufälligen Anteil davon löschen möchtest. Dann kannst du nur dann eine zuverläßige Abbruchbedingung codieren, wenn du:

- Entweder: VOR DEM LÖSCHEN, ersteinmal genau weißt wieviele es insgesamt gibt (nötigenfalls musst du erst extra mit einer Schleife alle zählen). Um dann erst mit einer (zweiten) Schleife, zu löschen. Damit wäre das Ergebnis auch exakt definiert. z.B. köntest du dann bestimmen das er Zufällig immer 1/3 löscht und 2/3 übrig läst.

- Oder: Du programmierst eine 'ungefähr' Lösung. Bedeutet, ganz unabhängig davon wieviele es gibt und wieviele er schon gelöscht hat, machst du eine Abbruchbedingung nach Wahrscheinlichkeit. z.B.

	C#-Quelltext
1	for(; Random.Range(0, 100) < 90;) {}

Das wäre dann ein: Ungefähr 10x versuchen blind zu stochern und dabei ein PICKUP zu finden und dieses zu löschen. ca. 10x weil bei jedem Durchlauf gibt es eine 90% Chance das er nochmal eine Runde dreht und eine 10% Chance das er abbricht.

Anmekung: Solche 'ungefähr' Lösungen, sind sehr chaotisch. Mann kann die Parameter darauf optimieren das im Durchschnitt auf viele male, in etwa es sich so verteilt wie gewünscht. Letztlich wird es aber immer auch zwischendring Ausreiser geben: mal löscht er fast alle - mal fast keinen oder sogar garkeinen.

EDIT: Eine 3 Möglichkeit (die Naheliegendste) habe ich fast vergessen.
Wenn es dir nicht auf eine ganz genau Anzahl ankommt die er löschen soll, sondern nur z.B. UNGEFÄHR 34% löschen. Dann wäre noch sowas möglich:

C#-Quelltext

for(int y = 0 ; y < size.y ; ++y)
   for(int x = 0 ; x < size.x ; ++x) {
      if(Random.Range(0, 100) < 66)
         continue;

      var pos = new GridCell(x, y, 0);
      var pickup = PLACING_MAP.GetTile<ItemTile>(pos);

      if(pickup == null)
         continue;
   }

EDIT: Ich habe aus einer if(), nun 2 if() gemacht, da es so effizienter im Programmablauf wäre.

Dieser Beitrag wurde bereits 4 mal editiert, zuletzt von »Audori« (23.07.2020, 20:58)

Zum Seitenanfang

MonkaBoy

Frischling

Beiträge: 21

23.07.2020, 22:18

Ich habe große Neuigkeiten ich habe es geschaft

Ich muss sagen, obwohl die Lösung ne gänzlich andere ist als vorgeschlagen, war dennoch der extrem wichtige Hinweis für meinen Holzkopf, dass er entlang der Wahrscheinlichkeit ein Pickup finden kann gegen unendlich geht, Ausschlag gebend für meine Lösung! Also vielen Dank an Euch alle, falls Ihr Euch dennoch für die Lösung intgeressiert hier der Code mit Erkläung:

C#-Quelltext

       public static IEnumerator RemoveAllPickupsOnDeadGround(object sender, TaskEventArgs e)
       {
         var size = Helper.GetSize(e.BeginOfBrokenGround, e.EndOfBrokenGround);  //compute Custom size (distance of begin and end)
         e.Size = size;

         ReadOnlyList<ItemTile> pickups = new ReadOnlyList<ItemTile>(size.x * size.y); 
         pickups.AddRange(FindAllPickupsOnDeadGround(sender, e)); //find all pickups within those deadground-bounds and yield them back

         var pool = new ProbabilityPool<ItemTile>(tiles, false, false);  //use a custom class which takes a custom list (pool) and makes a 
                                                                           weighted randomness possible from those elements

         for (int i = 0; i < pool.PoolCount; i++)
         {
            var pickup = pool.RandomValue;       //get a valid pickup from that position
            var deleted = DeletePickup(value);   //a 4 line code function which deletes the tilemap from a cuistom datastructer AND from the 
                                                   tilemap itself

            if (!deleted)
            {
               i--; //if u couldnt find the item u cannot also delete it so then u are not allowed to  i++ falsely so i keep the same "i" state
               continue;
            }

            yield return new WaitForSeconds(e.Delay);
         }
      }

Ich habe jetzt eine sehr schöne zufälligfe Zerstörung der Pickups und die Funktion wird in StartCoroutine innerhalb der Start() methode von Unity ausgeführt die wiederum einen unendlichen loop hat innerhalb einer Coroutine also:

C#-Quelltext

IEnumerator internalUpdate()
{
   while (true)
   {
       //do heavy code stuff here...when timer reached some given time!
      yield return RemoveAllPickupsFromGround()..
   }
}

void Start()
{
  StartCoroutine(internalUpdate());
}

Ich danke Euch und bin froh das weitest gehend noch selbstt gelöst zu haben(ist immer gut wenn man die Sachen dann selber hinbekommt, besser für den Lernerfolg) aber zum Rollen gebracht dank den wichtigen oberen Hinweis!

Dieser Beitrag wurde bereits 4 mal editiert, zuletzt von »MonkaBoy« (23.07.2020, 22:28)

Zum Seitenanfang

Audori

Frischling

Beiträge: 40

23.07.2020, 22:33

Danke für den fertigen Code

an sowas habe ich immer interesse!

Kleine Frage noch. RemoveAllPickupsOnDeadGround() ist doch auch ein IEnumerator der mit yield nichts besonderes macht, sondern immer für Coroutine passend WaitForSeconds() zurück gibt. Braucht es da noch die internalUpdate() oder funktioniert das nicht auch schon direkt?
Also mit:

	C#-Quelltext
1 2 3 4	void Start() { StartCoroutine(RemoveAllPickupsOnDeadGround(...)); }

EDIT: Ah ok, meine Frage hat sich wohl erledigt. Ich habe meinen Fehler gesehen :wacko:

Es sind 2 verschiedene Funktionen RemoveAllPickupsOnDeadGround() und RemoveAllPickupsFromGround()

Zum Seitenanfang

MonkaBoy

Frischling

Beiträge: 21

23.07.2020, 22:41

Hi Audori

Freut mich zu hören

ALso ne ne, es ist wirklich nur die RemoveAllPickupsOnDeadGround(..) funktion ABER ich will nicht sie direkt in Start() als Corotuine aufrufen weil ich habe noch 200 andere Zeilen code innerhalb der "internalUpdate()" funktion, deswegen haha.

C#-Quelltext

IEnumerator InternalUpdate()
{
   while (true)
   {
       //do heavy code stuff here...when timer reached some given time!  ===> 200 zeilen code mit heavy tasks and stuff
      yield return RemoveAllPickupsFromGround()..
   }
}

Zum Seitenanfang

Werbeanzeige

spieleprogrammierer.de - Forum und Wiki zur Spieleprogrammierung und Spieleentwicklung

C# Infinite Loop?

C# Infinite Loop?

C#-Quelltext

C#-Quelltext

C#-Quelltext

C#-Quelltext

C#-Quelltext

C#-Quelltext

C#-Quelltext

C#-Quelltext

C#-Quelltext

Ähnliche Themen