Array Index out of range

I am trying to create a lottery simulation in which the user is randomly given 5 numbers and if it matches any of the 5 winning numbers the user receives a prize.
Problems
-------------
1) I am getting an Array Index out of range error which I can't figure out why.
2) I want the random numbers to be nonrepeating (easily adaptable to a large range of numbers) but can't figure out the proper implementation.

Here is my code.

Code (CSharp):

1. using System.Collections;
2. using System.Collections.Generic;
3. using UnityEngine.UI;
4. using UnityEngine;
5.  
6. public class Functions : MonoBehaviour {
7.  
8.     public float wins = 0;
9.     public int RandomValue = 0;
10.     public int[] YourValues = new int[5];
11.     public int[] LotteryValues = new int[5];
12.     public float TimesPlayed = 0;
13.     public float TotalEarnings = 0;
14.     public float WinAverage = 0;
15.     public float WinPercentage = 0;
16.     public Button Button1;
17.     public Button Button2;
18.     public Button Button3;
19.     public Button PlayButton;
20.     public Text GamesPlayed;
21.     public Text WinPercent;
22.     public Text WinAve;
23.  
24.  
25.     // Use this for initialization
26.     void Start () {
27.         GamesPlayed.text = "Games Played: 0";
28.         WinPercent.text = "Win Percent: 0";
29.         WinAve.text = "Average Won: 0";
30.  
31.         for(int i = 0; i < 1000; i++)
32.         {
33.             Play();
34.         }
35.     }
36.    
37.     // Update is called once per frame
38.     void Update () {
39.        
40.     }
41.     void LotteryCard()
42.     {
43.         int picksMade = 0;
44.         while (picksMade <= 4)
45.         {
46.             RandomValue = UnityEngine.Random.Range (1, 11);// Random Values between 1-10.
47.             //Code for no repeats goes here.
48.             YourValues[picksMade] = RandomValue;
49.             picksMade ++;
50.         }
51.            
52.         picksMade = 0;
53.         while (picksMade <= 4)
54.         {
55.             RandomValue = UnityEngine.Random.Range (1, 11);// Random Values between 1-10.
56.             //Code for no repeats goes here.
57.             LotteryValues[picksMade] = RandomValue;
58.             picksMade ++;
59.         }
60.     }
61.     int Matches()
62.     {
63.         int Matches = 0;
64.         for(int i = 0; i < 5; i++)
65.         {
66.             for(int j = 0; j < 5; j++)
67.             {
68.                 if (YourValues[i] == LotteryValues[j])
69.                 {
70.                     Matches += 1;
71.                 }
72.             }
73.         }
74.         return Matches;
75.     }
76.     public void Play()
77.     {
78.         GamePlayed();
79.         LotteryCard();
80.         int x = Matches();
81.         if (x > 0)
82.         {WonGame(); PrizeWon(x);}
83.  
84.         CalculateAverages();
85.         GamesPlayed.text = "Games Played: " + TimesPlayed.ToString();
86.         WinPercent.text = "Win Percent: " + WinPercentage.ToString("F3");
87.         WinAve.text = "Win Average: " + WinAverage.ToString("F3");
88.     }
89.     void WonGame()
90.     {
91.         wins += 1;
92.     }
93.     void GamePlayed()
94.     {
95.         TimesPlayed += 1;
96.     }
97.     void PrizeWon(int x)
98.     {
99.         TotalEarnings += (x * x);
100.         Debug.Log("TE:" + TotalEarnings);
101.     }
102.     void CalculateAverages()
103.     {
104.         WinPercentage = wins / TimesPlayed;
105.         WinAverage = TotalEarnings / TimesPlayed;
106.     }  
107. }
108.  

 

What line is the error on?

 

Re point 1: Should work fine. By any chance did you change your array sizes in the property inspector or something? That's about all I can figure to cause this error with how you have this coded, unless it's not this class erroring out.

 

Code (CSharp):

1. while (picksMade <= 4)
2. while (picksMade <= 4)
3. for(int i = 0; i < 5; i++)
4. for(int j = 0; j < 5; j++)

Try not to use magic values like 4 or 5 here. If you intend to use those loops just for those arrays use their length as a limiter:

Code (CSharp):

1. while (picksMade < YourValues.Length)
2. while (picksMade < LotteryValues.Length)
3. for(int i = 0; i < YourValues.Length; i++)
4. for(int j = 0; j < LotteryValues.Length; j++)

This also makes code a little more clear. As the time passes you'll forget what 4 and 5 means. On the other hand YourValues.Length makes your intentions very clear.

 

That was indeed the issue, it was labeled as size 0 in the inspector, I changed the variables to private and it fixed the issue. I still would like to know how I can adjust the lottery card class to avoid repeats.

An easy solution would be to make a list of all numbers I wanted to pick from shuffling and choosing the first/last 5, but I need this to be adaptable to a large set of numbers (say 200).

Code (CSharp):

1. void LotteryCard()
2.     {
3.         int picksMade = 0;
4.         while (picksMade < YourValues.Length)
5.         {
6.             RandomValue = UnityEngine.Random.Range (1, 11);// Random Values between 1-10.
7.             //Code for no repeats goes here.
8.             YourValues[picksMade] = RandomValue;
9.             picksMade ++;
10.         }
11.          
12.         picksMade = 0;
13.         while (picksMade < LotteryValues.Length)
14.         {
15.             RandomValue = UnityEngine.Random.Range (1, 11);// Random Values between 1-10.
16.             //Code for no repeats goes here.
17.             LotteryValues[picksMade] = RandomValue;
18.             picksMade ++;
19.         }
20.     }

 

You could, for example, create a single method for filling the array with custom values. You've probably noticed that both loops are almost identical. One way to simplify that code would be:

Code (CSharp):

1. void LotteryCard ()
2. {
3.     FillArrayWithRandomValues (YourValues, 1, 11);
4.     FillArrayWithRandomValues (LotteryValues, 1, 11);
5. }
6.  
7. void FillArrayWithRandomValues (int[] array, int minValue, int maxValue)
8. {
9.     if (array != null)
10.     {
11.         for (int i = 0; i < array.Length; i++)
12.         {
13.             int randomValue = UnityEngine.Random.Range (minValue, maxValue);
14.             array[i] = randomValue;
15.         }
16.     }
17. }

Also, LotteryCard may not be the best name for a method. Try to think what your method does, and name it by that. How about InitializeLotteryCard ?

 

i tried this but i am getting the error "the name array does not exist in the current context"

Code (CSharp):

1. void LotteryCard()
2.     {
3.         int picksMade = 0;
4.         while (picksMade < YourValues.Length)
5.         {
6.             RandomValue = UnityEngine.Random.Range (1, 11);// Random Values between 1-10.
7.             if(Array.IndexOf(YourValues, RandomValue) != -1) //try to get the index of the value picked. IndexOf returns -1 if no match.
8.             {
9.                 //doesn't increment the loop, so will pick again.
10.             }
11.             else
12.             {
13.                 YourValues[picksMade] = RandomValue;
14.                 picksMade ++;
15.             }
16.         }

 

You're probably missing the

Code (CSharp):

1. #using System;

Don't mind my last post - it's completely out of the context and off the point.

As for the problem 2), you can fill the array and then shuffle it:

Code (CSharp):

1.     static int[] GetArrayOfUniqueShuffledValues (int count)
2.     {
3.         int[] array = Enumerable.Range (0, count).ToArray ();
4.         Shuffle (array);
5.         return array;
6.     }
7.  
8.     static void Shuffle<T>(T[] array)
9.     {
10.         int n = array.Length;
11.         for (int i = 0; i < n; i++)
12.         {
13.             int r = i + UnityEngine.Random.Range (0, n - i);
14.             T t = array[r];
15.             array[r] = array[i];
16.             array[i] = t;
17.         }
18.     }

 

For your scenario, I'd probably just check if the array already has the number in it or not and if it does, pick a new number. If it doesn't, add the number. But this depends on how many numbers you plan to have. If it's just 11, shuffling isn't going to be a big deal. If it's 1 million, I'd use the check if exist instead.

 

That's quite interesting. To me, intuitively, checking array of 1m intigers whether it already contains a generated one sounds way more expensive than a one-time shuffle. Consider generating the last value out of 1m. Your chance to generate a unique value is ~1 to 1m and that means you might end iterating the array of 1m integers 1m times... or even worse.

Can you provide an example of how you'd do it?

 

You check if you already picked the number, not the array of 1 million.

If your Random.Range(0, 1000000) for example, you probably don't need to shuffle that. (I actually have a linq shuffle that I used for a project, but it wasn't for a larger range).

Instead, you pick a number, assign it to index 0 of the array. Then pick a second number, see if it equals index 0. If not, assign to index 1. Then pick another and check index 0 and index 1, assign to index 2 if not equal. etc. If it does equal, pick another number.

Edit, I think we're talking two different things. I'm talking pick 5 from a million numbers, I think you're talking pick a million numbers. (just reread your post)

 

That might be the case. I'm not quite sure though. Isn't the core of the problem to create an array of unique, randomly distributed integers?

In that case, if you generated the same number at index 2, you pick another number (generate new), so you basically have to start all over from index 0 - is that correct?

 

It is, but what is better. Shuffle a million numbers, or pick 5 from a million? (picking 5 you'll probably not double up on numbers)

No. You just repick for index 2 if you already have that number. You don't have to regenerate the rest of your array.

 

Okay, now I get it. We're solving two separate problems. I'm trying to solve the shuffling and generation stuff assuming that you're going to use the whole array, and you're solving the get X unique numbers from (0, n).

And that totally makes sense if @BKGamesStudio wants to use just, say, 5 out of 200. You're right then. I misunderstood the problem.

 

Doesn't repicking for index 2 create a chance to generate something that is at index 0 and 1, leading to a non-unique values?

 

It does. You would pick again till you don't have a match. I don't have a link to the post and unfortunately, I can't find it at the moment, but there was a randomize question a bit back where someone did a test on the different methods of picking. If I remember correctly, repicking a duplicate number generally came out faster than shuffling.

I wish I had bookmarked it at the time as I can't recall at which point (size of collection and amount you want to pick) that one method became faster than the other. (This was a post from sometime last year if I recall). I'll see if I can dig it up.

Edit: Actually found the post I was looking for https://forum.unity3d.com/threads/h...t-repeat-on-same-numbers.435516/#post-2815867

I had a linq shuffle and the other person was using a .contains check. This isn't exactly like our case here as your shuffle is different than mine, but the general idea should be the same.

 

I was about to profile it myself until I actually realized we were talking about different things.

I suppose that would look something like that:

Code (CSharp):

1. static int[] GetArrayOfUniqueNumbersInRange (int count, int minValue, int maxValue_exclusive)
2.     {
3.         // Ensure count > 0
4.         // Ensure maxValue bigger than minValue
5.         // Ensure maxValue - minValue >= count
6.  
7.         var pickedNumbers = new List<int> (count);
8.  
9.         for (int i = 0; i < count; i++)
10.         {
11.             int randomValue = Random.Range (minValue, maxValue_exclusive);
12.             while (pickedNumbers.Contains (randomValue))
13.             {
14.                 randomValue = Random.Range (minValue, maxValue_exclusive);
15.             }
16.             pickedNumbers.Add (randomValue);
17.         }
18.  
19.         return pickedNumbers.ToArray ();
20.     }

 

Yep, I actually did find the link (and added to the previous post) and that was pretty much how they did it I believe.

 

Thanks for the help, I was missing the #using system;
My code now works as intended.

Code (CSharp):

1. void LotteryCard()
2.     {
3.         int picksMade = 0;
4.         while (picksMade < YourValues.Length)
5.         {
6.             RandomValue = UnityEngine.Random.Range (1, 11);// Random Values between 1-10.
7.             if(Array.IndexOf(YourValues, RandomValue) == -1) //If all indexes do not match the RandomValue -1 is returned and the next index is assigned.
8.             {
9.                 YourValues[picksMade] = RandomValue;
10.                 picksMade ++;
11.             }
12.         }
13.         picksMade = 0;
14.         while (picksMade < LotteryValues.Length)
15.         {
16.             RandomValue = UnityEngine.Random.Range (1, 11);// Random Values between 1-10.
17.             if(Array.IndexOf(LotteryValues, RandomValue) == -1) //If all indexes do not match the RandomValue -1 is returned and the next index is assigned.
18.             {
19.                 LotteryValues[picksMade] = RandomValue;
20.                 picksMade ++;
21.             }
22.         }
23.     }