Null check inconsistency (C#)

Found the weirdest bug in Unity today where a simple null check is not working, this is the code I'm using (attached to a simple GameObject):

Code (csharp):

1.  
2. using UnityEngine;
3. using System.Collections.Generic;
4.  
5. public class Test : MonoBehaviour {
6.  
7.     List<Component> components = new List<Component>();
8.  
9.     void Start ()
10.     {
11.         var obj = GetComponent<Collider>(); //Get a component that does not exist
12.  
13.         components.AddNotNull(obj); //Add it if it is not null
14.  
15.         //Check result
16.         Debug.Log("Number of items: " + components.Count + ".");
17.         Debug.Log("First value: " + components[0] + ".");
18.     }
19. }
20.  
21. public static class IListExtensions
22. {
23.     public static void AddNotNull<T>(this IList<T> list, T item)
24.     {
25.         if (item != null) //Item is not null, so add it
26.         {
27.             Debug.Log(item + " is not null.");
28.             list.Add(item);
29.         }
30.     }
31. }
32.  

When running the code the following is output:
null is not null.
Number of items: 1.
First value: null.

I have attached a full project example.

Any ideas?

 

Ouch! You should definitely report it.

I didn't believe it until I tried it out! It is pretty easy to get a workaround by testing whether item is null before you call AddNotNull. But this is a little frightening. Please let me know if you don't report it, because then I would do it.

 

I have reported this but thought it might be interesting for the community to look at as well.

I have noticed two workarounds for the problem, either don't use an extension method or put a constraint on the generic method (where T: Component, for example).

Still this is a severe error that must be fixed!

 

Here's a link to the reported bug: http://fogbugz.unity3d.com/default.asp?583896_mchrnmhjaefdl1pb

 

It's not actually a Unity bug, it's a .Net thing, basically the compiler cannot infer the type of T and so can't create the comparer. When you use generics you have to be a little more precise with what your asking it to do. I believe in generics it would become default(T), but not 100% sure without debugging...anyway to do what you want just use !=default(T) instead of null.

You may think it's stupid, but there are good reasons, consider if T is an int, or any other non-nullable type.

Does that make sense?

 

No, because that works as expected:

Code (csharp):

1. obj = null;
2. components.AddNotNull (obj);

Edit: It should be "obj = null;", but the o is not displayed for some reason...

 

It makes sense, still I would expect the compiler to complain about it or be able to handle the cases where obj is a class.

Doing a default check works correctly!

Code (csharp):

1.  
2. using UnityEngine;
3. using System.Collections.Generic;
4.  
5. public class Test : MonoBehaviour {
6.  
7.     List<Component> components = new List<Component>();
8.  
9.     void Start ()
10.     {
11.         var obj = GetComponent<Collider>(); //Get a component that does not exist
12.  
13.         components.AddNotDefault(obj); //Add it if it is not default
14.  
15.         //Check result
16.         Debug.Log("Number of items: " + components.Count + ".");
17.         Debug.Log("First value: " + components[0] + ".");
18.     }
19. }
20.  
21. public static class IListExtensions
22. {
23.     public static void AddNotDefault<T>(this IList<T> list, T item)
24.     {
25.         if (!item.IsDefault()) //Item is not default, so add it
26.         {
27.             Debug.Log(item + " is not default.");
28.             list.Add(item);
29.         }
30.     }
31. }
32.  
33. public static class GenericExtensions
34. {
35.     public static bool IsDefault<T>(this T t)
36.     {
37.         return EqualityComparer<T>.Default.Equals(t, default(T));
38.     }
39. }
40.  

 

Actually it's not a "bug" at all.... If you were writing this as a class library in Visual Studio for a normal .NET app you would have gotten a compiler warning for "possible compare of value type with null" because it can't guarantee that "T" is a Nullable type. You could use:

Code (csharp):

1.  
2. where T: class
3.  

Now you have a couple of options... you can change your extension so that it only works on List<Component> which would support polymorphism and derived classes as follows:

Code (csharp):

1.  
2. public static class IListExtensions
3. {
4.     public static void AddNotNull(this IList<Component> list, Component item)
5.     {
6.         if (item != null) //Item is not null, so add it
7.         {
8.             Debug.Log(item + " is not null.");
9.             list.Add(item);
10.         }
11.     }
12. }
13.  

Even though we've specified "Component" above, it would support derived classes and do so without the need for generics on the extension. If you want to support classes only, you can do it as follows:

Code (csharp):

1.  
2. public static class IListExtensions
3. {
4.     public static void AddNotNull<T>(this IList<T> list, T item) where T : class
5.     {
6.         if (item != null) //Item is not null, so add it
7.         {
8.             Debug.Log(item + " is not null.");
9.             list.Add(item);
10.         }
11.     }
12. }
13.  

This will allow you to check for null safely because it's a class. This becomes a little more sticky though if you want to support Nullable types such as int? which is actually a shortcut for Nullable<int>. In this case, you can't use a constraint which leaves your method succeptible to error, but you would resolve it as follows:

Code (csharp):

1.  
2. public static class IListExtensions
3. {
4.         public static void AddNotNull<T>(this IList<T> list, T item)    {
5.                 var objType = typeof(T);
6.  
7.                //Nullable<T> will still return "true" for value type...
8.                //Need to additional check if it's assignable
9.                if (objType.IsValueType)
10.                {
11.                         //Here we determine whether it's nullable<T>
12.                         if(Nullable.GetUnderlyingType(objType ) == null)
13.                               return;
14.  
15.                         //Since we know it's Nullable<T> we can let it pass out of this "if" block and go ahead
16.                         //and perform the null check below
17.                 }
18.            
19.  
20.                 if (item != null) //Item is not null, so add it
21.                 {
22.                     Debug.Log(item + " is not null.");
23.                     list.Add(item);
24.                 }
25.     }
26. }
27.  

To summarize, this allows you to setup a generic extension that will only add reference types and will include support for Nullable<T> which will return true for "IsValueType".

 

@Dustin Horne, adding this

Code (csharp):

1. where T : class

doesn't resolve the issue. That's the actual bug!

This one never works as expected:

Code (csharp):

1. var obj = GetComponent<Collider>();
2. components.AddNotNull (obj);

While that one behaves correctly:

Code (csharp):

1. var obj = null;
2. components.AddNotNull (obj);

 

This one never works as expected:

Code (csharp):

1. var obj = GetComponent<Collider>();
2. components.AddNotNull (obj);

This may have more to do with how Unity handles the objects. I'll use rigidbody as an example... Let's say you have a Component that does not have a rigidbody attached to it. If you say:

Code (csharp):

1.  
2. if(rigidbody)
3. {
4.     //do something with it
5. }
6.  

That works fine... However rigidbody isn't exactly truly "null". That's why my JSON .NET serializer can't serialize MonoBehaviors. It doesn't see "rigidbody" for example as actually being null... so it then tries to access the properties of rigidbody and Unity throws an exception for trying to access properties of a component that isn't added. I'm assuming if you tried to do it with rigidbody you'd get the same result. However, if you had a property that was your own class and you didn't initialize it, and you used it instead of a built in type, it would work properly with the where T : class constraint.

 

Here is my first Unity code joke:

Code (csharp):

1. var obj = GetComponent<Collider>();
2.  
3.     // HACK: This if block is needed as a workaround for a Unity bug.
4. if (obj == null) {
5.     obj = null;
6. }
7. components.AddNotNull(obj);

I hope this one eliminates the need for any further discussion whether it is a bug or not.

Edit:
@Dustin Horne, we should not need to think about how Unity handles objects. In .Net an object is an object. It is null if it is null. If in any object oriented language null is null, except for Unity, it's probably a Unity or maybe a Mono bug.

 

Lol. I agree it's a bug... just a Unity bug and not a .NET one.

 

FWIW, there are additional discussions on this topic here and here.

 

@JoshPeterson that blog post was indeed very useful to understand what exactly is going on, there was almost no information available before that.
Thanks for your comment!

 

Wow, just wasted several hours going through every inch of my code because of this bug. Glad to know I'm not going crazy!

Now I'm going to have to create an array behind an array of objects to tell whether or not they should exist.

 

Hi. Work for me. A more versatile method:
public static class Utilities
{
public static bool IsNull(this Object obj)
{
var temp = obj as Component;
return !((temp == null || temp.gameObject) && obj);
}
}