Unity Physics.OverlapSphere accessing objects that have already been destroyed-CodePudding

I'm making exploding barrels in Unity and using Physics.OverlapSphere to detect nearby rigidbodies and other exploding barrels. This is to move and trigger other exploding barrels to explode. The issue is when I'm using OverlapSphere in the triggered barrels it's accessing the previous barrel that triggered it which is destroyed, and I'm not entirely sure how.

This is error it gives me on line 67, where it says. colliders = Physics.OverlapSphere(transform.position ,explosionRadius);

IEnumerator explode(bool exploLag)
{
    alreadyExploded = true;

    //moved the yield to the beginning
    //overlapsphere was finding barrels which were destroyed on the previous frame and then trying to access them causing errors
    if (exploLag == true)
    {
        yield return new WaitForSeconds(explodeLag);
        Debug.Log("exploding later on due to explosion lag");
    }

    Debug.Log("explode was called");

    List<GameObject> existingBarrels = new List<GameObject>();
    Debug.Log("calm1");

    Collider[] colliders;
    colliders = Physics.OverlapSphere(transform.position ,explosionRadius);
    Debug.Log("Calm2");

    Rigidbody exploRB;

    foreach (Collider hit in colliders)
    {
        Debug.Log(hit);

        if (hit.GetComponent<Rigidbody>() == null)
        {
            Debug.Log("no rigidbody - cringe");
        }

        else if (hit.GetComponent<explosiveBarrel>() != null)
        {
            if (hit.GetComponent<explosiveBarrel>().alreadyExploded == true)
            {
                Debug.Log("Exploding barrel has already been made to explode");
                //so it doesnt try to explode it again and remove it 
                //will add an explosion force though so that its effected by the blast of this explosion also.

                //exploRB = hit.GetComponent<Rigidbody>();
                //exploRB.AddExplosionForce(explosionForce, explosionPos, explosionRadius, 1f, ForceMode.Impulse);
            }

            else
            {
                Debug.Log("ooo an existing barrel - ill save you later hehe uwu");
                //another barrel has been detected will be exploded on the next frame
                //this is too avoid it referencing this barrel aswell
                existingBarrels.Add(hit.gameObject);
                exploRB = hit.GetComponent<Rigidbody>();
                exploRB.AddExplosionForce(explosionForce, transform.position, explosionRadius, 1f, ForceMode.Impulse);
            }
        }

        else if (hit == null)
        {
            Debug.Log("Object doesnt exist anymore");
        }

        else
        {
            exploRB = hit.GetComponent<Rigidbody>();
            exploRB.AddExplosionForce(explosionForce, transform.position, explosionRadius, 1f, ForceMode.Impulse);
            
            Debug.Log("I moved a non exploding object");
        }
    }       

    foreach (GameObject explosiveB in existingBarrels)
    {
        explosiveB.GetComponent<explosiveBarrel>().StartCoroutine(explode(true));
    }
    
    ps.Play();
    Destroy(gameObject); 
    Debug.Log("explode pog");
    yield return null;
}

My apologies for just flooding my question with my code but I have no idea what's really wrong.

CodePudding user response：

It's the coroutine's fault.

I mocked up some exploding barrels and used similar code, and got the same error. Transferring everything to a regular function fixed the issue, the only safeguard I needed to prevent repeat explosions was the alreadyExploded bool. The existingBarrels list was also useful for ensuring the explosions happened in the right order.

Introducing the coroutine into the mix is what caused that MissingReferenceException- I'm sorry I can't explain exactly why. In theory the explosionLag system should avoid any weirdness, I get what you were going for, but I messed around with it for a while and couldn't make it work.

Best of luck. It doesn't seem like the coroutine was vital to your system, so hopefully its an easy fix.

CodePudding user response：

TL; DR: essentially I believe the problem with your code is that insufficient checks are leading to multiple calls to explode to explode a barrel that was already exploded. Hence the error from Unity.

Opening

I'm not sure why the need for a coroutine for this one. A regular method would have sufficed.

Additionally, coroutines can be risky, ones that invoke themselves without any guard particularly so.¹ ²

Issue

There are a few issues with the design of your code, particularly where and how checks are performed that governs if and when barrels explode.

Rather than checking the alreadyExploded flag and adding conditionally to the existingBarrels list:

if (hit.GetComponent<explosiveBarrel>().alreadyExploded == true) 
{
    .
    .
    .
}
else
{
    .
    .
    .
    existingBarrels.Add(hit.gameObject);
    .
    .
    .
}

...instead, I would perform the check like the following and do away with the list. For extra protection you should use a state machine (via the new BarrelStates enumeration) for the barrels to prevent more than one call to explode a barrel:

// alreadyExploded is insufficent to protect against duplicate explosions 
enum BarrelStates
{
    Idle,
    Exploding,
    Exploded
}

IEnumerator explode(bool exploLag)
{
    if (State != Exploding)  // <-------- new guard to prevent duplicate destruction
        yield break;

   
    foreach (Collider hit in colliders)
    {
        var barrel = hit.GetComponent<explosiveBarrel>();

        // Don't explode a barrel already in the process of exploding
        if (barrel != null && !barrel.State == States.Idle ) 
        { 
            barrel.State = States.Exploding; // <---- new
            exploRB = hit.GetComponent<Rigidbody>();
            exploRB.AddExplosionForce(explosionForce, transform.position, explosionRadius, 1f, ForceMode.Impulse);

        }
    }

    State = States.Exploded; // Finally, set exploded state

    ps.Play();
    Destroy(gameObject); 
    Debug.Log("explode pog");
    yield return null;
}

End notes

You need to be careful on holding references to Unity objects that are subject to destruction by Unity itself. Not only is this because Unity is the thing responsible for the object creation in the first place, but also because Unity is a CLR Host and it can and will zap out all .NET objects (including static) during Domain Reload whilst using the Editor or using Play.³ ⁴

The effect is exactly the same as desktop Windows development using say WinForms/WPF and you attempt to use a Pen, Brush or Font that has already been Disposed. Essentially both GDI objects and Unity have non-managed counterparts.

¹ Nelson, C, (formerly MSFT), "C# Why shouldn't I ever use coroutines?", Stack Overflow, March 2016, retrieved 2022-11-27

² Duncan, M, "Implementing time-based delays in Unity 3D", October 2022, retrieved 2022-11-27

³ "Domain Reloading", Unity, retrieved 2022-11-27

⁴ "Details of disabling Domain and Scene Reload", Unity, retrieved 2022-11-27