What exactly do you mean by ‘Memory Leak’? I can see a potential recursive issue, but I’m not sure what you mean by memory leak.
The recursive issue is that this objects could trigger another ExplosiveDestructableObject which will then start the same sequence, but it will also pick up the object that triggered it, meaning the ‘ground zero’ object will start the same process again, even if it is still busy exploding. This will turn into a vicious cycle that will kill your Unity. Then, even if we prevent this, there will be a major drop in frame rate if you have a bunch of these near each other.
To solve the first issue you may need to mark this object as ‘destroyed’ in some way before destroying it. Let’s just put a flag in the code and we don’t run Damage() if this flag is set;
public class ExplosiveDestructableObject : MonoBehaviour
{
[SerializeField] private Transform throwableVFXPrefab;
[SerializeField] private float effectRadius;
[SerializeField] private int effectAmount;
public static event EventHandler OnAnyExplosiveDestroyed;
private GridPosition gridPosition;
private Vector3 explosionOrigin;
private bool isDestroyed = false; // This is our flag
private void Start()
{
explosionOrigin = transform.position;
gridPosition = LevelGrid.Instance.GetGridPosition(transform.position);
}
// Returns grid position to object 'destroyer' to update pathfinding
public GridPosition GetGridPosition() { return gridPosition; }
public void Damage()
{
// Don't destroy if it already is
if (isDestroyed) return;
// Mark this object as destroyed
isDestroyed = true;
DamageNearbyObjects();
Instantiate(throwableVFXPrefab, explosionOrigin, Quaternion.identity);
Destroy(gameObject);
OnAnyExplosiveDestroyed?.Invoke(this, EventArgs.Empty);
}
private void DamageNearbyObjects()
{
Collider[] colliderArray = Physics.OverlapSphere(explosionOrigin, effectRadius);
foreach(Collider collider in colliderArray)
{
if (collider.TryGetComponent<Unit>(out Unit targetUnit))
{ targetUnit.Damage(effectAmount); }
// Does not require effectAmount
if (collider.TryGetComponent<DestructableObject>(out DestructableObject destructableObject))
{ destructableObject.Damage(); }
// Does not require effectAmount
if (collider.TryGetComponent<ExplosiveDestructableObject>(out ExplosiveDestructableObject explosiveDestructableObject))
{ explosiveDestructableObject.Damage(); }
}
}
}
With this, the Damage method won’t run if it is already running, so it will prevent the cycle.
The next issue is the frame-rate drop. I would turn DamageNearbyObjects into a coroutine. I would have to wait for it to complete before destroying the object itself, though. I will go with the pattern we’re currently using and give the coroutine a callback for when it is done
private IEnumerator DamageNearbyObjects(System.Action onDamageComplete)
{
Collider[] colliderArray = Physics.OverlapSphere(explosionOrigin, effectRadius);
foreach(Collider collider in colliderArray)
{
if (collider.TryGetComponent<Unit>(out Unit targetUnit))
{ targetUnit.Damage(effectAmount); }
// Does not require effectAmount
if (collider.TryGetComponent<DestructableObject>(out DestructableObject destructableObject))
{ destructableObject.Damage(); }
// Does not require effectAmount
if (collider.TryGetComponent<ExplosiveDestructableObject>(out ExplosiveDestructableObject explosiveDestructableObject))
{ explosiveDestructableObject.Damage(); }
yield return null; // This may be a little frequent. If there are lots of entries, perhaps only do this every 10 or so
}
onDamageComplete?.Invoke();
}
With this, we don’t really need the flag anymore. We can just hold a reference to the coroutine and if it’s running, don’t execute again. Here’s the updated class
public class ExplosiveDestructableObject : MonoBehaviour
{
[SerializeField] private Transform throwableVFXPrefab;
[SerializeField] private float effectRadius;
[SerializeField] private int effectAmount;
public static event EventHandler OnAnyExplosiveDestroyed;
private GridPosition gridPosition;
private Vector3 explosionOrigin;
private Coroutine damageCoroutine; // changed the flag here
private void Start()
{
explosionOrigin = transform.position;
gridPosition = LevelGrid.Instance.GetGridPosition(transform.position);
}
// Returns grid position to object 'destroyer' to update pathfinding
public GridPosition GetGridPosition() { return gridPosition; }
public void Damage()
{
// Don't destroy if it already is
if (damageCoroutine) return;
// Swapped these around
Instantiate(throwableVFXPrefab, explosionOrigin, Quaternion.identity);
// start coroutine and grab a reference. Also pass the complete method for when it's done
damageCoroutine = StartCoroutine(DamageNearbyObjects(DamageComplete));
}
// Cleanup this object when damage is done
private void DamageComplete()
{
Destroy(gameObject);
OnAnyExplosiveDestroyed?.Invoke(this, EventArgs.Empty);
}
private IEnumerator DamageNearbyObjects(System.Action onDamageComplete)
{
Collider[] colliderArray = Physics.OverlapSphere(explosionOrigin, effectRadius);
foreach(Collider collider in colliderArray)
{
if (collider.TryGetComponent<Unit>(out Unit targetUnit))
{ targetUnit.Damage(effectAmount); }
// Does not require effectAmount
if (collider.TryGetComponent<DestructableObject>(out DestructableObject destructableObject))
{ destructableObject.Damage(); }
// Does not require effectAmount
if (collider.TryGetComponent<ExplosiveDestructableObject>(out ExplosiveDestructableObject explosiveDestructableObject))
{ explosiveDestructableObject.Damage(); }
yield return null; // This may be a little frequent. If there are lots of entries, perhaps only do this every 10 or so
}
onDamageComplete?.Invoke();
}
}
This should solve the issues.
Just a note ('cos it popped into my head); You could sort the colliders by distance from the origin (closest first) before running the loop. This, coupled with the coroutine, will then give a ‘shockwave’ effect, damaging objects nearby just a moment before the ones further away. you could then also scale the damage amount with a lerp, meaning objects further away would take less damage
