- Home /
Problems with removing a random from a list
I am trying to cycle through a list of enum variables and return a random one from it, then remove it from the list however some of them return index out of bounds and I can't get a workaround it can anyone help me? - Regards, Shady
public enum Jobs {miner, builder, blacksmith, cook, None};
public List<Jobs> availableJobs = new List<Jobs>();
private Jobs toReturn;
private Jobs _detach
void Start() {
foreach (Jobs jobs in Enum.GetValues(typeof(Jobs))) { //get all jobs into list
availableJobs.Add(jobs);
}
}
public Jobs lookForAvailableJob() { //function to find a job
if(availableJobs.Count == 0) { //if list is empty
availableJobs.Clear();
return Jobs.None; //default
} else { //if not
_detach = availableJobs[Random.Range( 0, availableJobs.Count )]; //get a random
for (int i=0; i < availableJobs.Count; i++) {
if (availableJobs[i] == _detach) { //loop through to find the one
toReturn = availableJobs[i];
availableJobs.Remove(availableJobs[i]); //remove from list
print("removing: " +availableJobs[i]); //checking
}
}
}
return toReturn;
}
void Update () {
if (lookForAvailableJob() == Jobs.None) {
print("null");
} else {
lookForAvailableJob();
//print(lookForAvailableJob());
}
}
I'm getting these kinds of outputs in the console
Answer by NeverHopeless · Jul 31, 2015 at 01:09 PM
You lookForAvailableJob
function needs some modification. Since you are removing an element from an array on which you are looping through. It is not advice-able to do it like this. Instead you can capture the element then either break the loop and delete it, or, delete it when it comes out of the loop. The one i am suggesting below does not force to implement loop.
public Jobs lookForAvailableJob() { //function to find a job
if(availableJobs.Count == 0) { //if list is empty
availableJobs.Clear();
return Jobs.None; //default
} else { //if not
_detach = availableJobs[Random.Range( 0, availableJobs.Count )]; //get a random
availableJobs.Remove(_detach); //remove from list
}
return _detach;
}
You're absolutely right, I figured that out after dns mentioned the break; that it wouldn't really need a loop. Thankyou!
However, is there another way to return a default value because now I have to do it this way but there is a chance that It will randomly take "none" a bunch of times and that will prolong the removal of the other jobs
_detach = availableJobs[Random.Range( 0, availableJobs.Count )]; //get a random
if(availableJobs.Count == 0) { //if list is empty
availableJobs.Clear();
return Jobs.None; //default
} else if (_detach == Jobs.None) { //if not
return Jobs.None;
} else {
availableJobs.Remove(_detach); //remove from list
}
return _detach;
Perhaps this:
detach = availableJobs[Random.Range(0, availableJobs.Count )]; //get a random
if(availableJobs.Count == 0) { //if list is empty
availableJobs.Clear();
} else if (_detach != Jobs.None) { //if not
availableJobs.Remove(_detach); //remove from list
return _detach;
}
return Jobs.None;
Answer by _dns_ · Jul 31, 2015 at 01:07 PM
Hi, i guess the problem is the line 22: you "print" the name of the element i just after removing it! If you remove the last element of the list you can't print it's value after removing it! Also, the text displayed will be wrong for other elements = you remove element i but will display the name of element i+1 (because you just removed element i)
Also, no need to continue to iterate after you removed the element. A "break" statement would be fine here.
Wow switching 1 line and adding a break; did alot, thankyou it is fixed. I guess I was a little bit too into it, that I didn't notice I placed the print call after I deleted the object. :/ And I am ashamed I didn't use the break method
Well, didn't look at the efficiency of your code: NeverHopeless answer should be better (and fix the bug as well ;-)