koobas.hobune.stream
Stucked in a infinite loop and don't know why
Hi there, I've been stucked for a while trying to understand why i'm looping through an infinite loop.
Here is the code :
public class RessourcesManager : MonoBehaviour
{
[SerializeField]
private GameObject _woodPrefab;
[SerializeField]
private GameObject _allumettePrefab;
[SerializeField]
private GameObject _snowballPrefab;
[SerializeField]
private GameObject _fieldPrefab;
private GameObject[] _ressourcesPrefabs;
private Vector3 _minMapPos;
private Vector3 _maxMapPos;
private List<GameObject> _fireCamps = new List<GameObject>();
void Start()
{
// Set the array for all ressources available
_ressourcesPrefabs = new GameObject[3];
_ressourcesPrefabs[0] = _woodPrefab;
_ressourcesPrefabs[1] = _allumettePrefab;
_ressourcesPrefabs[2] = _snowballPrefab;
// Set the limitation of the map
_minMapPos = new Vector3(_fieldPrefab.transform.localScale.x / 2 * -1, 0, _fieldPrefab.transform.localScale.z / 2 * -1);
_maxMapPos = new Vector3(_fieldPrefab.transform.localScale.x / 2, 0, _fieldPrefab.transform.localScale.z / 2);
// Start the Coroutine spawning ressources every couple of time
StartCoroutine("RessourceSpawn");
}
IEnumerator RessourceSpawn()
{
int randomTimer;
randomTimer = Random.Range(1, 2);
int randomRessourceValue;
int randomRessourcePrefab;
// Waits random time between 2 ressources
yield return new WaitForSeconds(randomTimer);
randomRessourceValue = Random.Range(1, 5);
randomRessourcePrefab = Random.Range(0, 3);
// Determine where the ressource will be spawned
Vector3 spawnPosition = FindSpawnPosition();
// Spawn the ressource
GameObject newRessource = Instantiate(_ressourcesPrefabs[randomRessourcePrefab], spawnPosition, transform.rotation) as GameObject;
// Gives the ressource an amount and tells it which type it is
switch (randomRessourcePrefab)
{
case 0:
newRessource.GetComponent<Ressource>().woodValue = randomRessourceValue;
break;
case 1:
newRessource.GetComponent<Ressource>().allumetteValue = randomRessourceValue;
break;
case 2:
newRessource.GetComponent<Ressource>().snowballValue = randomRessourceValue;
break;
}
// Start the ressource creating processus again
StartCoroutine("RessourceSpawn");
}
// Finds the right place to spawn a ressource
private Vector3 FindSpawnPosition()
{
Vector3 randomPosition;
bool inLoop = true;
do
{
// Determines a random position accessible on the map limitations
randomPosition = new Vector3(Random.Range(_minMapPos.x, _maxMapPos.x), 0, Random.Range(_minMapPos.z, _maxMapPos.z));
// If a camp is too close, find another position, and do so until the ressource is far enough
int i = 1;
foreach (GameObject fireCamp in _fireCamps)
{
if(_fireCamps.Count == 0)
{
inLoop = false;
}
float dist = Vector3.Distance(randomPosition, fireCamp.transform.position);
if (dist < 5)
{
break;
}
// If that was the last fire Camp, the position is good and you can now exit the loop
if (i == _fireCamps.Count)
{
inLoop = false;
}
else
{
i++;
}
}
}
while (inLoop);
return randomPosition;
}
public void AddFireCamp(GameObject fireCamp)
{
_fireCamps.Add(fireCamp);
Debug.Log(_fireCamps.Count);
}
public void RemoveFireCamp(GameObject fireCamp)
{
_fireCamps.Remove(fireCamp);
Debug.Log(_fireCamps.Count);
}
}
Here is the culprit :
// Finds the right place to spawn a ressource
private Vector3 FindSpawnPosition()
{
Vector3 randomPosition;
bool inLoop = true;
do
{
// Determines a random position accessible on the map limitations
randomPosition = new Vector3(Random.Range(_minMapPos.x, _maxMapPos.x), 0, Random.Range(_minMapPos.z, _maxMapPos.z));
// If a camp is too close, find another position, and do so until the ressource is far enough
int i = 1;
foreach (GameObject fireCamp in _fireCamps)
{
if(_fireCamps.Count == 0)
{
inLoop = false;
}
float dist = Vector3.Distance(randomPosition, fireCamp.transform.position);
if (dist < 5)
{
break;
}
// If that was the last fire Camp, the position is good and you can now exit the loop
if (i == _fireCamps.Count)
{
inLoop = false;
}
else
{
i++;
}
}
}
while (inLoop);
return randomPosition;
}
Everytime a FireCamp is instantiated on a map, it add itself to the list _fireCamps. Everytime a FireCamp dies, it removes itself from the list. There is always a position on the map where the distance is less than 5. The moment all fire Camps die, the game takes maybe a second before freezing completely.
I don't understand why!
Thansk a lot!
Your problem is quite simple. You have a do - while loop that only stops when "inLoop" gets set to false. However that can never happen in your code.
This makes no sense:
foreach (GameObject fireCamp in _fireCamps)
{
if(_fireCamps.Count == 0)
{
inLoop = false;
}
The foreach loop body won't be executed at all if _fireCamps is empty. Therefore a check if the Count is equal to 0 inside the loop will never be true as the loop is only executed when you have at least one element in that list.
You would need to place this if check outside your foreach loop. If the camp fire list is empty you can just quite the whole method. You can add the condition to the while condition of your do - while loop
There are other things in your code that doesn't make much sense or that you want to check again. Especially your Random.Range calls:
randomTimer = Random.Range(1, 2);
randomRessourceValue = Random.Range(1, 5);
randomRessourcePrefab = Random.Range(0, 3);
You use the int version of Random.Range. The max value is exclusive when using the int version. So randomTimer will always be exactly "1". As the result of Random.Range is a value between 1 inclusive and 2 exclusive. So the only possible value left is "1".
"randomRessourceValue" would be either 1, 2, 3 or 4.
"randomRessourcePrefab" would be either 0, 1 or 2
It's generally bad practise to start a coroutine with a string. It's also never recommended to have a coroutine to restart itself all the time. It's possible it has quite a bit overhead and each coroutine generates garbage. It's better to just use an infinite loop. Just make sure you have at least one yield statement inside the loop.
Thanks a lot, that was exactly the problem. I'm still at the start of a long journey of learning how to program correctly, and there is some practices that I still need some time to work on.
The problem with the while is fixed as I check if the count == 0 after the foreach.
The Randoms are correct though. I know that ints in a random are excluding the maximum. I put 1,2 for the timer to have a regular timer while debugging, but it will be changed later. For the 2 others, ther are well set, so no worries for the understanding of Random.Range!
For the coroutine, I have to admit I am pretty new to it. I think I've figured out how to use it, but I think I have a good amount of things to learn. For the string part, what would you recommend to improve as a good habit, and the reason why? As for the loop, how would be the best way to write an infinite loop in this case, to be sure to not overload the garbage collector?
Thanks a lot for your feedbacks, it's really appreciate! I'm a Game Designer and I'm trying to improve on the coding part so I can do my own things and have a better understanding of the program$$anonymous$$g world in general. I'm not only looking to understand how to code, but interested on how to write it in the cleanest way a real programmer would do (let's say a bit like a programmer would). So thanks a lot!! :)
Cheers!
Hi!, In the Resource Manager class, in the ResourceSpawn method, you start the coroutine again and again, but you didnt selected the times that this is going to happen.
So your code could be like this:
public class RessourcesManager : MonoBehaviour {
// here you declare the counter****
int c = 0;
// and the times you want to repeat it****
int times;
[SerializeField]
private GameObject _woodPrefab;
[SerializeField]
private GameObject _allumettePrefab;
[SerializeField]
private GameObject _snowballPrefab;
[SerializeField]
private GameObject _fieldPrefab;
private GameObject[] _ressourcesPrefabs;
private Vector3 _minMapPos;
private Vector3 _maxMapPos;
private List<GameObject> _fireCamps = new List<GameObject>();
void Start()
{
// Set the array for all ressources available
_ressourcesPrefabs = new GameObject[3];
_ressourcesPrefabs[0] = _woodPrefab;
_ressourcesPrefabs[1] = _allumettePrefab;
_ressourcesPrefabs[2] = _snowballPrefab;
// Set the limitation of the map
_minMapPos = new Vector3(_fieldPrefab.transform.localScale.x / 2 * -1, 0, _fieldPrefab.transform.localScale.z / 2 * -1);
_maxMapPos = new Vector3(_fieldPrefab.transform.localScale.x / 2, 0, _fieldPrefab.transform.localScale.z / 2);
//You can assign the times you need it to repeat for example, three times
times = 3;
// Start the Coroutine spawning ressources every couple of time
StartCoroutine("RessourceSpawn");
}
IEnumerator RessourceSpawn()
{
int randomTimer;
randomTimer = Random.Range(1, 2);
int randomRessourceValue;
int randomRessourcePrefab;
// Waits random time between 2 ressources
yield return new WaitForSeconds(randomTimer);
randomRessourceValue = Random.Range(1, 5);
randomRessourcePrefab = Random.Range(0, 3);
// Determine where the ressource will be spawned
Vector3 spawnPosition = FindSpawnPosition();
// Spawn the ressource
GameObject newRessource = Instantiate(_ressourcesPrefabs[randomRessourcePrefab], spawnPosition, transform.rotation) as GameObject;
// Gives the ressource an amount and tells it which type it is
switch (randomRessourcePrefab)
{
case 0:
newRessource.GetComponent<Ressource>().woodValue = randomRessourceValue;
break;
case 1:
newRessource.GetComponent<Ressource>().allumetteValue = randomRessourceValue;
break;
case 2:
newRessource.GetComponent<Ressource>().snowballValue = randomRessourceValue;
break;
}
//so at this point you check the times it has been repeated
if(c <= times){
StartCoroutine("RessourceSpawn");
} else {
c++;
}
//or just use a for loop
//for(int i = 0; i < times; i++){
//All the steps you need to do...
//}
}
// Finds the right place to spawn a ressource
private Vector3 FindSpawnPosition()
{
Vector3 randomPosition;
bool inLoop = true;
do
{
// Determines a random position accessible on the map limitations
randomPosition = new Vector3(Random.Range(_minMapPos.x, _maxMapPos.x), 0, Random.Range(_minMapPos.z, _maxMapPos.z));
// If a camp is too close, find another position, and do so until the ressource is far enough
int i = 1;
foreach (GameObject fireCamp in _fireCamps)
{
if(_fireCamps.Count == 0)
{
inLoop = false;
}
float dist = Vector3.Distance(randomPosition, fireCamp.transform.position);
if (dist < 5)
{
break;
}
// If that was the last fire Camp, the position is good and you can now exit the loop
if (i == _fireCamps.Count)
{
inLoop = false;
}
else
{
i++;
}
}
}
while (inLoop);
return randomPosition;
}
public void AddFireCamp(GameObject fireCamp)
{
_fireCamps.Add(fireCamp);
Debug.Log(_fireCamps.Count);
}
public void RemoveFireCamp(GameObject fireCamp)
{
_fireCamps.Remove(fireCamp);
Debug.Log(_fireCamps.Count);
}
}
Hope it helped, Good Luck!
Hi there! Thanks for the answer!
Unfortunately, that does not correct the problem I have. The coroutine is workign fine, as it needs to loop forever (it spawns a ressource every x to y seconds (need to be balanced)), so adding a timer will not help the situation.
The problem really resides in the do-while loop. The loop checks if there is a fire camp near the ressource it's trying to spawn. It checks out every fire camps, and when it finds a random location where there is a fire camp close, it breaks the foreach and start the while again, chosing another random location. if it goes through every camp and none are close, inLoop becomes false, and everyone goes out of the while. In case there is no fire Camp in the list, then it should just go out of the while loop and everyone is happy.
Everything works fine until there is no more fire camps. At that moment, I think it looks through an empty list, establish a distance with nothing (which is like... 0?) and breaks out of the foreach loop, starting back the while loop, and cant get out. That's the reason why I put a if(_fireCamps.Count == 0) to check if the list is empty, but that does no seems to work.
