infinite loop when adding item to list???
Hi everyone, I am currently getting an infinite loop (or at least I believe it is an infinite loop) when I run the following code:
for (int x = 0; x < NumberOfWallsInside; x = x + 1) //This picks the gaps to be made (haven't been made yet)
{
Debug.Log(x);
RandomYCoordinateOne = UnityEngine.Random.Range(1, ColumnLength); //int min inclusive, int max exclusive
RandomYCoordinateTwo = UnityEngine.Random.Range(1, ColumnLength); //int min inclusive, int max exclusive
if (RandomYCoordinateOne == RandomYCoordinateTwo || RandomYCoordinateOne == RandomYCoordinateTwo - 1 || RandomYCoordinateOne == RandomYCoordinateTwo + 1)
{
Debug.Log(x);
x = x - 1;
}
else
{
ListOfYCoorindates.Add(5);
ListOfYCoorindates.Add(RandomYCoordinateOne);
ListOfYCoorindates.Add(RandomYCoordinateTwo);
}
}
What this is meant to do is choose two random points for each column of walls, check if these two random points are next to each other or euqal e.g. 4 and 5, 3 and 2 or 2 and 2....., if they are then the for loop needs to re-do these random points otherwise it adds it to the list 'ListOfYCoordinates' to be used later. From my tests using debug.Log(); I believe that the for loop works when I comment out the list.add(); that is inside the else statement, however I want this to become available so if u have any idea on whats wrong please help, please and thank you. Also, below is some more code about the variables in this for loop.
int RowLength = MapArray.GetLength(0); //MapArray.GetLength is simply a constant
int ColumnLength = MapArray.GetLength(1);
int NumberOfWallsInside = RowLength - ((RowLength - 1)/2) - 2;
List<int> ListOfYCoorindates = new List<int>();
int RandomYCoordinateOne = 0;
int RandomYCoordinateTwo = 0;
If ColumnLength at least 4? Otherwise the difference two generated random values will always be 0 or 1 and the code will enter the "x = x - 1" case => endless loop.
Answer by whynot660 · Jan 10, 2017 at 08:55 AM
Hi doublemax In my opinion, the problem is in the decrementation x = x - 1, on line 9 because every time the condition is meet, you decrement x, and since you are not using an array that would rise an index out of range exception when the index is negative, you will end-up with a sort of infinite loop.
you confused the number of iterations and the number of the expected values, I would suggest you a while loop, and to put the coordinates into a structure,but I tried to keep the structure of your program bellow
for (int x = 0; x < NumberOfWallsInside;/*leave it blank*/)
{
Debug.Log(x);
RandomYCoordinateOne = Random.Range(1, ColumnLength);
RandomYCoordinateTwo = Random.Range(1, ColumnLength);
if (RandomYCoordinateOne == RandomYCoordinateTwo || RandomYCoordinateOne == RandomYCoordinateTwo - 1 || RandomYCoordinateOne == RandomYCoordinateTwo + 1)
{ // we didnt meet the expected coordinates so let try another time
Debug.Log(x);
//x = x - 1; you must do nothing here or continue
continue;
}
else
{ // we have values that we can add to the list
x = x + 1;// the number of the good values found is incremented
ListOfYCoorindates.Add(5);
ListOfYCoorindates.Add(RandomYCoordinateOne);
ListOfYCoorindates.Add(RandomYCoordinateTwo);
}
}
here is better version
int found = 0;// the number of good coordinates
while (found < NumberOfWallsInside){
RandomYCoordinateOne = Random.Range(1, ColumnLength);
RandomYCoordinateTwo = Random.Range(1, ColumnLength);
if (RandomYCoordinateOne == RandomYCoordinateTwo ||
RandomYCoordinateOne == RandomYCoordinateTwo - 1 ||
RandomYCoordinateOne == RandomYCoordinateTwo + 1)
{
continue;// try another time
}else{
found++;// found = found +1;
ListOfYCoorindates.Add(5);
ListOfYCoorindates.Add(RandomYCoordinateOne);
ListOfYCoorindates.Add(RandomYCoordinateTwo);
}
}
It's still mandatory that ColumnLength is bigger than 3. $$anonymous$$g. if it's 3, the random number would always be 1 or 2 and this would run endlessly.
I would also use
if( $$anonymous$$ath.abs( RandomYCoordinateOne - RandomYCoordinateTwo) < 2 )
for the check.
its better with the abs and you can even use it to avoid the undesirable values like this
//we don't check for a value, we sipmly avoid it
RandomYCoordinateOne = Random.Range(1, ColumnLength);
RandomYCoordinateTwo = $$anonymous$$ath.abs(
RandomYCoordinateOne -
Random.Range( 4, ColumnLength));// from 4 to avoid 1 - 3 = 2
this should work for ColumnLength greater then 4
Hello again, thanks for your replies the idea of columnlength having to be greater than 3 was certainly useful, I tested my code including the new suggestion by whynot660, not the one with maths.abs as I am confused on that one and I still get an infinite loop. This time I have made sure that that; columnlength = 25 rowlength = 25 numberofwalls inside = 11 Here is the new function although it is shown above too:
int found = 0;// the number of good coordinates
while (found < NumberOfWallsInside){
RandomYCoordinateOne = UnityEngine.Random.Range(1, ColumnLength);
RandomYCoordinateTwo = UnityEngine.Random.Range(1, ColumnLength);
if (RandomYCoordinateOne == RandomYCoordinateTwo ||
RandomYCoordinateOne == RandomYCoordinateTwo - 1 ||
RandomYCoordinateOne == RandomYCoordinateTwo + 1)
{
continue;// try another time
}else{
found++;// found = found +1;
ListOfYCoorindates.Add(RandomYCoordinateOne);
ListOfYCoorindates.Add(RandomYCoordinateTwo);
}
}
If understood you are trying to create a sort of maze, and the loop is too create gates in the walls
for (int x = 0; x < NumberOfWallsInside;x++){
coorY1 = Random.Range(1, (maxY-1)/2 );// maxY is number of rows
coord2 = Random.Range(coorY1+2,(maxY-1));
ListOfYCoorindates.Add(5);
ListOfYCoorindates.Add( coorY1);
ListOfYCoorindates.Add(coord2);
}
Hi again, I still have any infinite loop going on, as far as I am aware your code seems like it would work so below is my full code in case somewhere else is causing the infinite loop. Do realise that when i comment out the ListOfYCoordinates.Add()'s the program runs from start to end. Anyways here is my full code:
int[,] Create$$anonymous$$apBorder(int $$anonymous$$apRowSize, int $$anonymous$$apColumnSize, List<int> listxCoordinates, List<int> listyCoordinates)
{
Debug.Log("aaaaaaaa");
int[,] $$anonymous$$apArray = new int[$$anonymous$$apRowSize, $$anonymous$$apColumnSize];
for (int x = 0; x < $$anonymous$$apRowSize; x++)
{
$$anonymous$$apArray[x, 0] = 1;
$$anonymous$$apArray[x, $$anonymous$$apArray.GetLength(0) - 1] = 1;
}
for (int y = 0; y < $$anonymous$$apColumnSize; y++)
{
$$anonymous$$apArray[0, y] = 1;
$$anonymous$$apArray[$$anonymous$$apArray.GetLength(1) - 1, y] = 1;
}
Debug.Log("bbbbbbbb");
return $$anonymous$$apArray;
}
Code won't all fit in one reply so here's the rest:
int[,] Create$$anonymous$$apInside(List listxCoordinates, List listyCoordinates, int[,] $$anonymous$$apArray)
{
int RowLength = $$anonymous$$apArray.GetLength(0);
int ColumnLength = $$anonymous$$apArray.GetLength(1);
int NumberOfWallsInside = RowLength - ((RowLength - 1)/2) - 2;
List<int> ListOfYCoorindates = new List<int>();
int RandomYCoordinateOne = 0;
int RandomYCoordinateTwo = 0;
Debug.Log("ccccccccc");
for (int x = 1; x < RowLength; x = x + 2) //This creates verticle walls every odd x value in the $$anonymous$$apArray
{
for (int y = 1; y < ColumnLength; y++)
{
$$anonymous$$apArray[x, y] = 1;
}
}
Debug.Log("dddddddddd");
for (int x = 0; x < NumberOfWallsInside;x++) //This picks the gaps to be made (haven't been made yet)
{
RandomYCoordinateOne = UnityEngine.Random.Range(1, (ColumnLength-1)/2 );// maxY is number of rows
RandomYCoordinateTwo = UnityEngine.Random.Range(RandomYCoordinateOne,(ColumnLength-1));
ListOfYCoorindates.Add(RandomYCoordinateOne);
ListOfYCoorindates.Add(RandomYCoordinateTwo);
}
//Debug.Log (ListOfYCoorindates);
int XCoordinate = 3;
Debug.Log("eeeeeeeee");
for (int x = 0; x < ListOfYCoorindates.Count; x = x + 1) //This creates the gaps chosen previously
{
while (XCoordinate == x + 3 || XCoordinate == x + 2)
{
$$anonymous$$apArray[XCoordinate, ListOfYCoorindates[x]] = 0;
}
XCoordinate = XCoordinate + 2;
}
Debug.Log("ffffffffff");
for (int x = 0; x < ListOfYCoorindates.Count; x = x + 2) //This creates a wall between the two gaps
{
int RandomYPointGaps = UnityEngine.Random.Range(ListOfYCoorindates[x] + 1, ListOfYCoorindates[x + 1]); //int $$anonymous$$ inclusive, int max exclusive
$$anonymous$$apArray[x + 1, RandomYPointGaps] = 1;
}
Debug.Log("ggggggggg");
return $$anonymous$$apArray;
}
you have too many loops in there :) , your code could be shorter and cleaner, you only need modeling it on paper before, code always comes last, some source codes and tutorials on nested loops and 2D arrays manipulation also,
I didn't read it carefully but the error seems to be here :
while (XCoordinate == x + 3 || XCoordinate == x + 2)
{
$$anonymous$$apArray[XCoordinate, ListOfYCoorindates[x]] = 0;
// you need an instruction to modify the x value to get out of the loop Like x++
}
for example to get the initial matrix you can use a nested loop with a few conditions inside
for (int i=0; i<$$anonymous$$axLines; i++){
for(int j = 0; j< $$anonymous$$axColumns; j++){
if( j%2 == 0 ){
// 0 2 4 6 8 10 .... columns set as wall
$$anonymous$$atrix[i,j] = 1;
}else{
if(i == 0 || i == $$anonymous$$axLines -1){
// to set the gaps in the first and last line with 1
$$anonymous$$atrix[i,j] = 1;
}
}else{// set the rest to 0
$$anonymous$$atrix[i,j] = 0;
}
}
}
Good luck, and if you need any more help don't be shy :]
Thank you for everyone's help, as said above the infinite loop was caused due to a different section of my code shown above. I believe it is working fine now.
Answer by Tourist · Jan 10, 2017 at 02:22 PM
In a matter of general coding, you should never modify the iterator variable of a for loop inside the content of the loop. You have other loops that exist for this that would be more appropriate (while, do...while, ...). For loops should always be used when you know, before starting the loop how many times the loop will be processed. For loops should, therefore, never be infinite.
Moreover, try to avoid continue and break keywords, they break the semantic of the loop.
Apart from the philosophical answer, for your case, it is obviously the solution is to stop modifying the x variable that is used as the condition to end the loop.
I fixed the solution you mentioned by using a while loop ins$$anonymous$$d from the responses above but I still get an infite loop, so can you please look at my code again, I added a new comment to the replies above explainging my new code. Thank you.
Your infinite loop is now depending on how much your random values will use the if condition. Less the column length is, more loop process will fall into that case. If the value of column length is too low, this process may be infinite or at least way too long.
I can't figure out why you are using random values for getting the walls inside. If you want some random walls among all the walls inside, start by getting them, and then pick random ones.
I think I understand the idea of that if the columnlength is less than 4 then the process may be infinite or at least way to long, however even when I use the test data of column length being 25 I still get the infinite loop and I don't think 25 is too small. Sorry that I haven't explained what I am trying to do well enough. Below is an explanation of what I'm trying to do with an example:
(1 represents a wall tile, 0 represents a floor tile)
1 1 1 1 1 1 1 1 1 1 1
1 0 1 0 1 0 1 0 1 0 1
1 0 1 0 1 0 1 0 1 0 1
1 0 1 0 1 0 1 0 1 0 1
1 0 1 0 1 0 1 0 1 0 1
1 0 1 0 1 0 1 0 1 0 1
1 0 1 0 1 0 1 0 1 0 1
1 0 1 0 1 0 1 0 1 0 1
1 0 1 0 1 0 1 0 1 0 1
1 0 1 0 1 0 1 0 1 0 1
1 1 1 1 1 1 1 1 1 1 1
Once the $$anonymous$$apArray has been created as said, then the code should randomly choose two y-coordinates for each line of vertical walls created except the first and last wall, while doing this I have to check whether two y-coordinates are next to each other on the same line of vertical walls as if they are the code should choose two new y-coordinates until all the lines of vertical walls have been done, for each y-coordinate chosen, they should be inputted into a list called ‘ListOfYCoordinates’ so that the can be retrieved again for future use. If this is done correctly with the same $$anonymous$$apArray as shown above we should get something as shown below:
1 1 1 1 1 1 1 1 1 1 1
1 0 1 0 1 0 1 0 1 0 1
1 0 0 0 1 0 1 0 1 0 1
1 0 1 0 1 0 0 0 1 0 1
1 0 1 0 0 0 1 0 1 0 1
1 0 1 0 1 0 1 0 0 0 1
1 0 1 0 0 0 0 0 1 0 1
1 0 1 0 1 0 1 0 1 0 1
1 0 0 0 1 0 1 0 1 0 1
1 0 1 0 1 0 1 0 0 0 1
1 1 1 1 1 1 1 1 1 1 1
If you have a map like that, I guess you can iterate through a dimension (column in the example) and if the column is a column representing a wall, go for a random value inside [1; n - 1] where n is the number or rows (still in the example).
The random index is where your door will be. Now, just take the previous row and next row to know what coordinates are bounding your door. If you want to make sure doors are not on the same row, keep a collection of all possible available indexes for your doors (a list of all values in the domain [1; n - 1]), pick a random value inside that collection that will give your the row, and then remove this row index from the collection. This way, you won't have two doors on the same row, and since you are iterating through columns, all columns will be processed.
In your example :
List<int> availableRowIndexesForDoor = new List<int>();
for(int rowIndex = 1; rowIndex <= 10; ++rowIndex)
{
availableRowIndexesForDoor.Add(rowIndex);
}
for(int columnIndex = 2; columnIndex <= 8; columnIndex += 2)
{
int randomIndex = Random.Range(0, availableRowIndexesForDoor.Count);
int rowIndexForDoor = availableRowIndexesForDoor[randomIndex];
availableRowIndexesForDoor.RemoveAt(randomIndex );
// Do Stuff with your lists with rowIndexForDoor, rowIndexForDoor - 1 and rowIndexForDoor + 1
}