Strange code produced by Unity's null override.

I ran across the following bit of code (paraphrased) written by one of our programmers. I got a kick out of it.

public void SomeMethod()
{
    if (this != null)
    {
        ... do actual work ...
    }
}

Normally, in C#, this can never be null. However, objects that derive from MonoBehaviour or ScriptableObject can have the equality comparison of this == null return true whenever the underlying unity object has been Destroyed, yet the monobehaviour instance has not been garbage collected.

What our enterprising engineer encountered here was a case where some external class or classes had a reference to a monobehaviour that was previously associated with a destroyed gameobject. That external class was wantonly calling SomeMethod() without first checking for null. Instead of locating all calls (of which there are legion in this case) and wrapping those calls in null checks, he chose to wrap the body of the method in a null check. A strange decision, but it works. Honestly, I prefer this choice to peppering the project with null checks.

I truly hope Unity 6 introduces an IsDestroyed method or getter, to help improve the clarity of code such as the above.

3 Likes

Yeah, Unity overloading null was a bad decision. I feel like it was the result of one person saying “Hey this would be a cool way to avoid typing a few letters” which then had huge repercussions on everything down the road.

Don’t post often but just wanted to say I have also encountered this. A slightly different scenario caused me to lose ~ 0.5 days debugging.

I agree completely with makeshiftwings, this seems like a choice to save a few keystrokes at the expense of user sanity.

Personally I think there are mistakes with the .net/CLR equality design and overloading == compounds this.

That’s totally awesome!*

*In the weird way like watching two trains smash into each other at full speed is awesome. Not the good awesome.

I can’t think of a reason to ever check if “this” is null inside the instance… You should be checking for null before even making the method call. Seems like a good way to create bugs. Interesting find though.

Its not that unusual for methods to run their own sanitation on the input parameters before running. Its actually good practice if you have no control over the code that might be calling your method. And there are situations where you can’t do a null check from outside of the method.

As there is currently no isDestroyed property, if(this != null) is the best option you have.

TL;DR: Its the fault of Unity’s unique design, not the fault of the programmers writing the code.

Can you give an example of an instance where you wouldn’t be able to check for null outside the method? The only time I can think of that you wouldn’t have access is when the engine makes the call to the function… In that case the engine should be checking for null. So I’m a bit confused.

You haven’t played much with Unity’s custom null check yet. It gives some very interesting results if you cast MonoBehaviours back to System.Objects. In some situations an object can pass a null check, then throw a null reference error. This can get particularly irksome with generics.

Unity wrote a blog post about the issue here. http://blogs.unity3d.com/2014/05/16/custom-operator-should-we-keep-it/

1 Like

In order to call a function on an object instance you need a reference to that object. If your reference is null then how can you call the function? If the engine is calling the function then it should be checking the reference just like the expected implementation.

The expected implementation.

if (objReference != null)
{
   objReference.SomeMethod();
}
else
{
   //do something else
}

I don’t even know what to say about this.

objReference.SomeMethod();//if objReference is null you will get an exception

public void SomeMethod()
{
   if (this != null)//so what is the point of this?
   {
   }
}

Maybe I’m confused by the custom operator. Are you saying the objReference equality could result in objReference.SomeMethod() being called even if it’s set to null? Still doesn’t explain why you would ever use “this” for null equality though…

Sometimes you won’t get an exception, if the objReference is not actually null, and has only been destroyed. Unity overrides “== null” to return true if either the object is null OR it is pointing to a component that is in the process of being destroyed but has not actually been deleted yet. If you do get an exception, it won’t be a null reference exception, it would be “You are trying to access a component which has already been marked as destroyed”, and sometimes that doesn’t get thrown like it should.

1 Like

The fact that it doesn’t always throw an exception is silly… But that still doesn’t explain why you would ever use “this” for null equality and further proves that you should be checking if the reference is null before doing the function call, especially if the reference could be lost.

interface IDamageable {
    void TakeDamage(int damage);
}

//in SomeClass
void DoDamageTo(IDamageable target) {
    if(target != null)
        target.TakeDamage(CalculateDamage());
}

//later:
public class Monster : MonoBehaviour, IDamageable {
   
    public int health;

    public void TakeDamage(int damage) {
        health -= damage;
        //recoil:
        transform.position -= transform.forward;
    }

}

The null-check in DoDamageTo will pass, even if the IDamageable sent in is a Monster that’s been destroyed. The recoil line will throw an exception, as transform will be Unity-null.

Since the == and != overloads are static, they get dispatched statically - ie. by the type of the argument, not the type of the actual object. This means that when a MonoBehaviour is sent around as an interface rather than it’s concrete type, the null-check will be the default C# null-check, rather than Unity’s UnityEngine.Object override.

I’ve got this check somewhere in my code base:

if(variable == null || (variable is UnityEngine.Object && ((UnityEngine.Object) variable) == null)) {

}

That replaced my own “if (this == null)” call, which makes any static tools go “What the fuck are you doing, do you even understand C#?”

3 Likes

I think “this == null” is an edge case and not something you should be doing in every single function obviously, but there are times that it makes sense. And I don’t think the OP’s original intention was “hey we should all put “if this == null” in every single function ever”, they were just pointing out how Unity’s overriding of null equality leads to really weird hacks like this one. That said, there are plenty of reasons that a destroyed object might not be equal to null before a call (like BoredMormon’s post above) or that it might become destroyed but not null while the function is happening, like if it’s referenced in another object’s Update or Coroutine.

1 Like

I had no ill intent, was just curious as to the purpose of the code as it seems extremely strange. This is only an issue when using the Destroy method if I’m understanding correctly…? If so, is it an issue with DestroyImmediate as well? What if you manually set the reference to null after calling Destroy? I generally do this anyway because of the description of the Destroy method on the API documentation.

“Actual object destruction is always delayed until after the current Update loop”

The way C# works is you can’t actually destroy something. (Well you could using IDisposable and a bunch of other complex stuff, but its not straight forward). So the C++ GameObject is destroyed, but the C# object remains alive in memory. The C# object will hang around until it is garbage collected, and garbage collection won’t happen until there are no references to the object anywhere. If you have a bunch of references hanging around in different areas, you need to set them all to null before the C# object will disappear.

I think I worded my questions poorly. Let me rephrase… This is an issue because people are expecting all references to the C# object to start returning true that they are null when using Destroy? I’m guessing the null equality override allows this from what has been said in this thread.

I always assumed the Destroy functions were to simply destroy the C++ objects and you should always null references manually, as shown in example 2. I guess this isn’t immediately obvious to people that are completely new to C#.

Example 1:

public GameObject someGameObject = new GameObject();

public void Example1()
{
   Destroy(someGameObject);
   Debug.Log(someGameObject == null);//This could print "True" or "False"...?
}

Example 2:

public GameObject someGameObject = new GameObject();

public void Example2()
{
   Destroy(someGameObject);
   someGameObject = null;
   Debug.Log(someGameObject == null);//This will always print "True"...
}

@Baste your example makes sense but why and how would you be getting an interface from a null or destroyed object in the first place? I guess you could be caching a reference to the interface but it should be nullified when your object is destroyed.

P.S. Exposing an IsDestroyed bool would be completely irrelevant because Unity could handle that inside the equality comparer. Which seems to be what they used to do according to the blog BoredMormon posted. Taken directly from the blog, “for cases where the object is not “really null” but a destroyed object, a nullcheck used to return true”. This would have been a really nice feature to keep…

@jimroberts - I’d suggest actually trying these things on your own to see how they work, rather than continually asking us to explain to you how they work while simultaneously insisting that you know better than everyone else how it should be done.

No need to wait.

using System;
using Object = UnityEngine.Object;

public static class ObjectExtensions
{
    public static bool IsDestroyed(this Object obj)
    {
        if (ReferenceEquals(obj, null))
        {
            throw new NullReferenceException();
        }
        return obj == null;
    }
}

Usage:

if (someObject.IsDestroyed())
{
}
else
{
}
2 Likes

I posted my test examples in my previous reply…?? The results are exactly as expected… Calling Destroy will not result in references returning that they are null. However, setting the reference to null will. Is it really that hard to answer my simple questions? The only thing I can take away from all of this is that you guys have some weird coupling issues until someone explains it.

Baste’s example is the only answer that makes sense to use the equality check within the actual instance. However, it still doesn’t explain why he would keep the reference after calling Destroy on the underlying Unity.Object.

It’s totally normal to have other more then one reference to an object, in completely different systems. It doesn’t always follow that you can make a reference null after calling destroy. Some other class might have called destroy.