So i was writing code to simulate how animals would eat food and drink water around the map, but i keep getting an error. I believe its because multiple animals are trying to access one lake or one fruit, but i don't know how to avoid this from happening. So here is my code, please help (i know my coding skills are quite bad, but i don't care)
public float hunger = 100;
public float speed;
public float thirst = 100;
// Start is called before the first frame update
void Start()
{
StartCoroutine(hungerdrop());
}
// Update is called once per frame
void Update()
{
if (hunger <= 20 && hunger < thirst)
{
gotoBush();
}
if (thirst <= 20 && thirst < hunger)
{
gotoLake();
}
if (hunger <= 0)
{
Destroy(gameObject);
}
}
IEnumerator hungerdrop()
{
yield return new WaitForSeconds(0.2f);
hunger -= 1;
thirst -= 2;
StartCoroutine(hungerdrop());
}
public void gotoBush()
{
GetClosestFruit();
}
public void gotoLake()
{
GetClosestLake();
}
GameObject GetClosestFruit()
{
GameObject tMin = null;
float minDist = Mathf.Infinity;
Vector3 currentPos = transform.position;
foreach (GameObject t in GameObject.FindGameObjectsWithTag("fruit"))
{
float dist = Vector3.Distance(t.transform.position, currentPos);
if (dist < minDist)
{
tMin = t;
minDist = dist;
}
}
if (tMin != null)
{
float step = speed * Time.deltaTime;
StartCoroutine(walkToBush());
IEnumerator walkToBush()
{
if (tMin != null)
{
while (transform.position != tMin.transform.position && tMin != null)
{
yield return new WaitForSeconds(0.1f);
transform.position = Vector3.MoveTowards(transform.position, tMin.transform.position, step);
}
hunger = 100;
StartCoroutine(bushEat());
}
IEnumerator bushEat()
{
yield return new WaitForSeconds(0.1f);
destroyBush();
}
}
void destroyBush()
{
Destroy(tMin);
}
}
return tMin;
}
GameObject GetClosestLake()
{
GameObject tMin = null;
float minDist = Mathf.Infinity;
Vector3 currentPos = transform.position;
foreach (GameObject t in GameObject.FindGameObjectsWithTag("lake"))
{
float dist = Vector3.Distance(t.transform.position, currentPos);
if (dist < minDist)
{
tMin = t;
minDist = dist;
}
}
if (tMin != null )
{
float step = speed * Time.deltaTime;
StartCoroutine(walkToBush());
IEnumerator walkToBush()
{
if (tMin != null)
{
while (transform.position != tMin.transform.position && tMin != null)
{
yield return new WaitForSeconds(0.1f);
if(tMin != null)
{
transform.position = Vector3.MoveTowards(transform.position, tMin.transform.position, step);
}
}
thirst = 100;
StartCoroutine(bushEat());
}
IEnumerator bushEat()
{
yield return new WaitForSeconds(0.1f);
destroyBush();
}
}
void destroyBush()
{
Destroy(tMin);
}
}
return tMin;
}
}
CodePudding user response：
First: You should use while loops in coroutines instead of recursion like this:
IEnumerator hungerdrop()
{
while(true)
{
hunger -= 1;
thirst -= 2;
yield return new WaitForSeconds(0.2f);
}
}
Second: To check for gameObjects that have already ben destroyed, you can simply use an if statement like:
if(gameObject != null)
{
//do something
}
CodePudding user response：
If I understand correctly you have multiple animal instances all running this component.
I see two huge issues here:
- You might have concurrent Coroutines since you start a new routine every frame while your conditions are
true! This might even mean that you try to move in two opposed directions => you never reach a target. - Multiple of your animals could target the very same resource => while the one is still moving towards it, the other one might already have eaten it and destroys it.
And then there is most probably this issue that you check the alive state of your resources after getting the position in
while (transform.position != tMin.transform.position && tMin != null)
where you should rather check
while (tMin && transform.position != tMin.transform.position)
However, this would still not prevent the rest of your code after the while from being executed even if tMin doesn't exist anymore!
What I would rather do is
- Make sure only one Coroutine is running at a time
- Make sure only one animal can target a resource at a time
Therefor instead of using tags I would rather have two actual components attached to the resources with a common base class:
public abstract class ConsumableResource : MonoBehaviour
{
public bool isOccupied;
}
and then these two go onto the according resource GameObjects:
public class Fruit : ConsumableResource
{
// Doesn't have to do anything else .. but could
}
and
public class Lake : ConsumableResource
{
// Doesn't have to do anything else .. but could
// You could e.g. consider to add a counter so that multiple animals can target this at a time
// and/or add another counter so multiple animals can consume this resource before it is destroyed
}
And then I would do
public class Animal : MonoBehaviour
{
public float hunger = 100;
public float speed;
public float thirst = 100;
// the currently executed coroutine
private Coroutine _currentRoutine;
// the currently targeted resource instance
private ConsumableResource _currentTargetResource;
// Update is called once per frame
private void Update()
{
// first of all I would rather drop the hunger and thirst like this
// This now means hunger drops 5 units per second and thirst drops 10 units per second
// What you had before was a complex way for writing basically the same
hunger -= 5f * Time.deltaTime;
thirst -= 10f * Time.deltaTime;
if (hunger <= 0)
{
Destroy(gameObject);
}
// In order to no end up with multiple concurrent routines check if there is already a routine running first
if (_currentRoutine == null)
{
if (hunger <= 20 && hunger < thirst)
{
// start our resource routine targeting the type Fruit and if we succeed to run the entire routine to and
// then call WhenConsumedFruit afterwards to reset the hunger
_currentRoutine = StartCoroutine(GetResourceRoutine<Fruit>(WhenConsumedFruit));
}
// make your cases exclusive!
else if (thirst <= 20 && thirst < hunger)
{
_currentRoutine = StartCoroutine(GetResourceRoutine<Lake>(WhenConsumedLake));
}
}
}
private void WhenConsumedFruit()
{
hunger = 100;
}
private void WhenConsumedLake()
{
thirst = 100;
}
// Use a generic method to not repeat the same implementation
// This now can be used to get the closest of any inherited type from ConsumableResource
private T FindClosestResource<T>() where T : ConsumableResource
{
// First get all instances in the scene of given resource type
var allFruits = FindObjectsOfType<T>();
// filter out those that are already occupied by another animal instance
var onlyNotOccupiedFruits = allFruits.Where(fruit => !fruit.isOccupied);
// sort the remaining instances by distance
var sortedByDistance = onlyNotOccupiedFruits.OrderBy(fruit => (fruit.transform.position - transform.position).sqrMagnitude);
// take the first one (= closest) or null if there was no remaining instance
return sortedByDistance.FirstOrDefault();
}
// Again use the most generic base class so you can reuse the same code for any inherited type of ConsumableResource
IEnumerator MoveTowardsResource(ConsumableResource target)
{
while (transform.position != target.transform.position)
{
transform.position = Vector3.MoveTowards(transform.position, target.transform.position, speed * Time.deltaTime);
yield return null;
}
transform.position = target.transform.position;
}
// and again do his only once
IEnumerator ConsumeResource(ConsumableResource target)
{
yield return new WaitForSeconds(0.1f);
// You might want to consider to rather move this into the resource itself
// so it could extend the behavior as said e.g. using counters
// without the need that your animal class has to be aware of what exactly
// this means for the resource. Maybe it doesn't destroy it but only disable
// for some time so it can grow back => unlimmitted options ;)
Destroy(target.gameObject);
}
private IEnumerator GetResourceRoutine<T>(Action whenConsumed) where T : ConsumableResource
{
_currentTargetResource = FindClosestResource<T>();
// did we find a closest fruit?
if (!_currentTargetResource)
{
// if not terminate this routine and allow the next one to start
_currentRoutine = null;
yield break;
}
// set closest fruit occupied so no animal can take it anymore
_currentTargetResource.isOccupied = true;
// Move towards the closest fruit
yield return MoveTowardsResource(_currentTargetResource);
// "Eat" the fruit
yield return ConsumeResource(_currentTargetResource);
// when all is done without this object dying before simply invoked the passed callback action
whenConsumed?.Invoke();
// Allow the next routine to start
_currentRoutine = null;
}
private void OnDestroy()
{
// in case we die for whatever reason make sure to release the occupied resources if there are any
// so if you die on the way towards it at least from now on another animal can pick it again as target
if (_currentTargetResource) _currentTargetResource.isOccupied = false;
}
}