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):
using UnityEngine;
using System.Collections.Generic;
public class Test : MonoBehaviour {
List<Component> components = new List<Component>();
void Start ()
{
var obj = GetComponent<Collider>(); //Get a component that does not exist
components.AddNotNull(obj); //Add it if it is not null
//Check result
Debug.Log("Number of items: " + components.Count + ".");
Debug.Log("First value: " + components[0] + ".");
}
}
public static class IListExtensions
{
public static void AddNotNull<T>(this IList<T> list, T item)
{
if (item != null) //Item is not null, so add it
{
Debug.Log(item + " is not null.");
list.Add(item);
}
}
}
When running the code the following is output:
null is not null.
Number of items: 1.
First value: null.
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).
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.
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!
using UnityEngine;
using System.Collections.Generic;
public class Test : MonoBehaviour {
List<Component> components = new List<Component>();
void Start ()
{
var obj = GetComponent<Collider>(); //Get a component that does not exist
components.AddNotDefault(obj); //Add it if it is not default
//Check result
Debug.Log("Number of items: " + components.Count + ".");
Debug.Log("First value: " + components[0] + ".");
}
}
public static class IListExtensions
{
public static void AddNotDefault<T>(this IList<T> list, T item)
{
if (!item.IsDefault()) //Item is not default, so add it
{
Debug.Log(item + " is not default.");
list.Add(item);
}
}
}
public static class GenericExtensions
{
public static bool IsDefault<T>(this T t)
{
return EqualityComparer<T>.Default.Equals(t, default(T));
}
}
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:
where T: class
Now you have a couple of options… you can change your extension so that it only works on List which would support polymorphism and derived classes as follows:
public static class IListExtensions
{
public static void AddNotNull(this IList<Component> list, Component item)
{
if (item != null) //Item is not null, so add it
{
Debug.Log(item + " is not null.");
list.Add(item);
}
}
}
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:
public static class IListExtensions
{
public static void AddNotNull<T>(this IList<T> list, T item) where T : class
{
if (item != null) //Item is not null, so add it
{
Debug.Log(item + " is not null.");
list.Add(item);
}
}
}
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. In this case, you can’t use a constraint which leaves your method succeptible to error, but you would resolve it as follows:
public static class IListExtensions
{
public static void AddNotNull<T>(this IList<T> list, T item) {
var objType = typeof(T);
//Nullable<T> will still return "true" for value type...
//Need to additional check if it's assignable
if (objType.IsValueType)
{
//Here we determine whether it's nullable<T>
if(Nullable.GetUnderlyingType(objType ) == null)
return;
//Since we know it's Nullable<T> we can let it pass out of this "if" block and go ahead
//and perform the null check below
}
if (item != null) //Item is not null, so add it
{
Debug.Log(item + " is not null.");
list.Add(item);
}
}
}
To summarize, this allows you to setup a generic extension that will only add reference types and will include support for Nullable which will return true for “IsValueType”.
var obj = GetComponent<Collider>();
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:
if(rigidbody)
{
//do something with it
}
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.
var obj = GetComponent<Collider>();
// HACK: This if block is needed as a workaround for a Unity bug.
if (obj == null) {
obj = null;
}
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.
I’ve just had a change to take a look at case 583896 for this issue. We have determined that this is actually the expected, if somewhat odd, behavior. the problem is best summarized in this bog post:
The real issue is that the AddNotNull method has no knowledge of the type of its generic parameter T, so the C# compiler does not know to call the == operator overload for UnityEngine.Component. Instead, the emitted IL code in this case is
IL_0000: ldarg.1 // Load the item argument on to the stack
IL_0001: box !!T // Box the item argument
IL_0006: brfalse IL_0027 // Branch if the boxed item argument is null
Since the Unity editor returns a non-null C# wrapper object in this case the call to box does not push a value of null on to the evaluation stack, so the branch is not taken, and we enter the body of the C# if statement.
The best work around for this problem is to provide a hint to the compiler about the type of T using a where clause:
public static void AddNotNull<T>(this IList<T> list, T item) where T: Component
Of course this work around limits the usefulness of the AddNotNull method, since it will not apply to other C# types.
As was discussed in the blog post this behavior is confusing, and is not something that we necessarily like, but changing this behavior comes with significant costs. So we have decided to stick with it.
@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!
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);
}
}
