-2

I'm trying to create a loop that outputs 9 numbers in a random order with no repeating numbers. When I run it it creates an infinite loop and I can't figure out where I went wrong.

    public void randomizer()
    {
        resetArray();
        for (int i = 0; i < 3; i++)
        {
            for (int j = 0; j < 3; j++)
            {
                int randomNumber = UnityEngine.Random.Range(1, 9);
                if(!numCheck(randomNumber))
                {
                    buttons[i, j] = randomNumber;
                }
                else
                {
                    j--;
                }
                Debug.Log(randomNumber);
            }
        }
    }

    public void resetArray()
    {
        for (int i = 0; i < 3; i++)
        {
            for (int j = 0; j < 3; j++)
            {
                buttons[i,j] = 0;

            }
        }
    }

    public bool numCheck(int num)
    {
        for (int i = 0; i < 3; i++)
        {
            for (int j = 0; j < 3; j++)
            {
                if (buttons[i, j] == num)
                {
                    return true;
                }

            }
        }
        return false;
    }

The code loops infinitely, however when my friend mimic'd it in c# it worked fine on his end. What is causing the infinite loop?

  • 1
    The infinite loop may happen if the numCheck(randomNumbers) is true, and cause J to decrease by 1 and then increase by 1 again by the iterator, resulting in infinite loop since J is never changed essentially. Put a debugger in numCheck and find out if it is always true. – Spencer Ovetsky May 10 '23 at 03:07
  • Note that `.Range` can only return 1, 2, 3, 4, 5, 6, 7, and 8. – ProgrammingLlama May 10 '23 at 03:23

2 Answers2

3

Looping to ensure that the current random number hasn't already been generated is never going to be a good way of doing things. The problem is that as you approach "all numbers generated", the more collisions (an already-used number being generated) you encounter. You essentially need to loop more and more towards the end.

For a finite set of numbers, it's generally better to use something like the Fisher-Yates shuffle on an existing set of numbers. I'll provide an example with your specific problem using vanilla C#, though you can easily convert this to Unity's Random class.

First we'll generate an array of numbers 1-9, and then we'll shuffle them.

var random = new Random();

// Fill the array with numbers 1-9
int [] numbers = new int[9];
for (int i = 0; i < numbers.Length; ++i)
{
    numbers[i] = (i + 1);
}
// the above is equivalent to int [] numbers = Enumerable.Range(1, 9).ToArray();

Console.WriteLine(string.Join(", ", numbers));

// Shuffle the array
for (int i = 0; i < numbers.Length - 1; ++i)
{
    int pos = random.Next(i, numbers.Length);
    int tmp = numbers[pos];
    numbers[pos] = numbers[i];
    numbers[i] = tmp;
}

Console.WriteLine(string.Join(", ", numbers));

When I ran it, the first WriteLine prints 1, 2, 3, 4, 5, 6, 7, 8, 9 as we expect, but the second line printed WriteLine printed 8, 2, 1, 7, 9, 4, 6, 3, 5, indicating that our shuffle was successful.

Now we can use this data to populate our 2D array.

int[,] buttons = new int[3, 3];
for (int i = 0; i < 3; ++i)
{
    for (int j = 0; j < 3; ++j)
    {
        // i * 3 + j basically divides the numbers array into 
        // 3 3-digit sections, and then chooses the number
        // at position j from the section referenced by i
        buttons[i, j] = numbers[i * 3 + j];
    }
}

// the above is equivalent to Buffer.BlockCopy(numbers, 0, buttons, 0, 9 * sizeof(int));

We can verify that the array has been populated correctly using this code to write each position:

for (int i = 0; i < 3; ++i)
{
    for (int j = 0; j < 3; ++j)
    {
        if (j != 0)
        {
            Console.Write(", ");
        }
        Console.Write(buttons[i, j]);
    }
    Console.WriteLine();
}

As a short version, we could write the following (I've also substituted the Unity random method here):

// Generate an array with numbers 1-9
int [] numbers = Enumerable.Range(1, 9).ToArray();

// Shuffle the array
for (int i = 0; i < numbers.Length - 1; ++i)
{
    int pos = UnityEngine.Random.Range(i, numbers.Length);
    int tmp = numbers[pos];
    numbers[pos] = numbers[i];
    numbers[i] = tmp;
}

// Populate the 2D buttons array with the values in the 1D number sarray
Buffer.BlockCopy(numbers, 0, buttons, 0, 9 * sizeof(int));
ProgrammingLlama
  • 36,677
  • 7
  • 67
  • 86
1

Please go with ProgrammingLlama's excellent answer, but here is a slightly more simplified approach that might be useful to you:

public int[,] Randomizer()
{
    int[] shuffled =
        Enumerable
            .Range(1, 9)
            .OrderBy(x => Random.Shared.Next())
            .ToArray();
    
    int[,] output = new int[3, 3]; 
    
    for (int i = 0; i < 3; i++)
        for (int j = 0; j < 3; j++)
            output[i, j] = shuffled[i * 3 + j];
            
    return output;
}

You'd just go ahead and call that with buttons = Randomizer();.

Enigmativity
  • 113,464
  • 11
  • 89
  • 172