Skip to content

Instantly share code, notes, and snippets.

@lucasmeijer
Last active November 10, 2015 15:08
Show Gist options
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save lucasmeijer/7475fd40ff6ac34c7c44 to your computer and use it in GitHub Desktop.
Save lucasmeijer/7475fd40ff6ac34c7c44 to your computer and use it in GitHub Desktop.

Hi,

We have a feature that if you do:

m_MyGUiLayer = GetComponent<GUILayer>();

if it turns out there was no GUILayer component, if you later use m_MyGUILayer, instead of getting a NullReferenceException (what you might expect as a c# dev), you get:

MissingComponentException: There is no 'GUILayer' attached to the "Directional Light" game object, but a script is trying to access it.
You probably need to add a GUILayer to the game object "Directional Light". Or your script needs to check if the component is attached before using it.
NewBehaviourScript.Start () (at Assets/NewBehaviourScript.cs:10)

We are able to give a much better error message that contains the type of the component that was requested, the gameobject name of the gameobject that apparently didn't have the component, and it also has "context" of that gameobject, so if you click this message in the console window, the editor will actually "ping" the gameobject in question. We do this only in the editor.

Awesome.

This feature comes at a (editor-only) cost of a c# allocation for that error message string (as well as the fake-null-object that holds the string). Every week, we get a bugreport about "GetComponent() is allocating, this is madness!" from folks that (probably) don't realise that this allocation only happens in the editor.

We're debating wether or not this error message provides enough value compared to cost it brings. (costs: apparently users getting confused about it a lot, us having to deal with a flow of bugreports about it, editor-allocation patterns being different from the player).

Options on the table are:

  • Drop the feature, and make GetCompoment() return a normal null instead.
  • add GetComponent(bool niceErrorMessageWhenNotFound = true), so that if you care about not having the allocation in the editor, you can tell us to not mak eit.
  • not do anything about it.
  • fake it, and internally mark this allocation as editor-only, and then have the profiler actually hide the allocation.

I'd love to hear from users if they have ever found the MissingComponentException to be especially helpful, and in which scenarios that was, and how one's workflow would be negatively affected by no longer having this detailed exception.

Thanks, Lucas (you can reply here, or on twitter @lucasmeijer)

@Cygon
Copy link

Cygon commented Nov 5, 2015

I say remove it.

The allocation does confuse the profiler, having to disable every single usage of it when optimizing will get cumbersome (especially if 3rd party libraries are involved) and, after all, you can still get most of the benefits of the check by adding a mode that throws an exception right away if the component is not found:

GetComponent<T>(bool throwIfMissing = false) // new parameter
GetRequiredComponent<T>() // alternative method

@TimBasteiLuebbe
Copy link

Remove it.

It makes profiling in the editor a bit harder because you have to remind to ignore these allocations.

@KyleOrth
Copy link

KyleOrth commented Nov 5, 2015

I would to either leave it as it is or fake it in the profiler. I cant think of any specific times when its been especially useful to me, but I'm sure for newcomers it would be much more useful to have the detailed error message rather than just a hard to trace null ref.

@DanPuzey
Copy link

DanPuzey commented Nov 5, 2015

Ultimately - while there is a debugging nicety to the message in the IDE - a NullReferenceException would still quickly draw attention to the issue: an allocation to the variable has allocated null, and (in well-written code, at least) there shouldn't be many candidates for the cause.

For me, consistency between development and built environments is worth more than the nicety of the error. (Also, surely when the IDE logs an exception you can/do include the context anyway? I don't have Unity handy to test...)

However, both of those points ignore that it's generally desirable to expose an error as soon as it occurs: it's much more useful to have the error thrown at the assignment instead of 5 seconds later when referencing the null variable. For that reason, I think that an overload/alternative of GetComponent<T>() that throws when the component is missing would be most useful. (Ignoring backward compatibility issues, I'd argue this should be the default behaviour of the method.)

There are two basic times that GetComponent is called, in my experience:

  1. when grabbing a component at runtime to check for a behaviour (e.g.: I've just pressed "Use"; does the object in front of me have a Usable component?). In this case, you frequently call

    var usable = target.GetComponent<Usable>(); 
    if (usable != null)
    {
       // handle use of object
    }
    

    The GetComponent call would return a null, but you're immediately checking for the null, and the scope of usable is likely to be small/short.

  2. when wiring up long-term component references (e.g. assigning to private fields from Awake or Start) - where you rarely want to accept a missing component, and so throwing the error at point of allocation makes sense. (I typically throw/log errors manually at this point if components are missing, so that the error is thrown immediately instead of causing problems later.) In this case, an overload that throws when the component is missing will make code shorter and more readable.

@Duke74
Copy link

Duke74 commented Nov 5, 2015

Hi,

I think you should keep it because it's easier to understand for new Unity users than just a basic NullReferenceException.

It's also helpfull if you have a script who is using a component present on another gameobject because if the component is not present on the gameobject you target you'll instantly have a good clue of where the problem come from.

@eirikwah
Copy link

eirikwah commented Nov 5, 2015

Drop it! It is confusing to have (non-explicit) different behaviour if you run in editor or not.

But please add a mode that throws an exception right away if the component is not found (as suggested by Crygon above), and that behaves exactly the same in and out of the Editor:

GetComponent(bool throwIfMissing = false) // new parameter

@ffyhlkain
Copy link

I'd suggest to keep it and make the error message more clear that it is only showing up in the editor. Don't add additional parameters (adds not needed complexity).

The error message can be handy if something happened to prefabs/scenes removing script references (happened to our project not only once) and you need to know what component and where it is missing.

@Khan-amil
Copy link

Maybe if the allocation was clearly stated and explained here http://docs.unity3d.com/ScriptReference/Component.GetComponent.html people would complian less? At the very least you'd have an easy/quick way to explain them.

I don't see a real benefit in having the same memory allocation in editor VS build, so just faking it in the profiler would be enough IMO.

On the other hand I second @crygon, having the exception raised when you try to get it would be great.
Maybe couple it with a TryGetComponent (out T comp) for the cases where we want to add and init ourselves?

@garrynewman
Copy link

Return null. Running in the editor should match the behaviour of running standalone as closely as possible. The error message is no more useful than a NRE with a line number.

@jeanfabre
Copy link

Hi,

From a point of view where many users are not experts, I would make it as an option so that middleware publishers can provide a good feedback and keep themselves out of the loop on such problem, else we always get users getting back to the Author to complain on something that is really a scene setup issue.

It's always good to have information, why rely on expertize and painful debugging sessions when the log can lift any guess work on the issue. The fact that is behaves differently in Editor and at Runtime is another part of the problem, and I don't see this as a reason to remove this feature all together.

Thanks for your hardwork :)

Bye,

Jean

@naruse
Copy link

naruse commented Nov 5, 2015

I would drop it if I could chose. I have been using unity enough time and its pretty self explanatory when this happens.

@TrinketBen
Copy link

I'd say remove it.

I think the intention is primarily to help new developers make sense of what's gone wrong with their scripts, but my feeling is that this is hand-holding them too excessively. For the sake of consistency I'd rather teach a beginner about how to handle a NRE than how to handle both this special exception and a NRE.

Moreover, the wording of the exception seems unhelpful or misleading. That is, if I as a neophyte Unity user see that message in the console, how am I to take it? The "You probably need to add a..." portion seems clear enough that it might be a compelling option for a beginner to pursue, but there's no way of knowing without understanding the context of the exception whether that is the right course of action. In other words, I feel like the message might encourage "try this fix without understanding the problem" behaviour.

@jjjuande
Copy link

jjjuande commented Nov 5, 2015

I would remove it.

If you want to help new developers, then it should be better to append a link/button to the error message (row) in the Console (maybe at the right of each row when the row is selected) with the text: "Know more about this error" / "How to fix this?" / "Why this error?" or something similar for the most common exceptions/errors/warnings. And when you click on it, you get redirected to the Unity manual page with this error/warning explanation, when it happens, how to avoid it, etc. (Doing this, the change is only related to the Unity Editor, and at the same time this option deliver much more information, since you can add as much errors/warnings explanation in the docs, as you need.)

You could add a toggle button to the Console and have this enabled by default.

@skateborden
Copy link

I think the best approach would be to keep the error message, internally mark the allocation as editor-only, and then instead of hiding it in the profiler have a clearly labeled editor only section in the profiler that is user hide-able. That way you can have nice error messages and be transparent about what the editor is doing, but also keep a clear separation that shows what will/will not happen in a built player.

@nanuinteractive
Copy link

I think you should do nothing about this, not because I care about the editor message, but because there is no reason to spend time changing.

As someone who uses Unity on mobile and cares about individual bytes, with 15 years of experience doing this sort of profiling on C++, this issue has never yet hit my own radar. GetComponent() is allocating? Who cares. Now, I wonder if the assetbundle resource I loaded then expected to get dropped during a Resources.UnloadUnusedAssets actually DID get unloaded.

You're spending valuable dev resource on making people who profile code-side byte allocations the editor happier.

@Cotoff
Copy link

Cotoff commented Nov 6, 2015

There are two big cases to use GetComponent call:

  1. During initialization, to obtain references to components you KNOW are there.
  2. During gameplay, to check if a gameobject is of a certain kind (i.e. has a component you need)

In first case, you obviously don't expect to get null at all. In fact, if you get a null, the error has already happened, and exception should be thrown right away.
In second case, getting null reference is actually an expected outcome. If this null value gets used later, it's an error, but the problem is NOT "missing component", but you forgetting to check.

The correct way to handle this would be to add some method (GetRequiredComponent?) that throws right away if component is not found, and have GetComponent return plain nulls. Adding a "special kind of null" is helpful neither to experienced programmers (who dont expect this and get confused), nor to newbies (who now have to learn about a special-case and confusing behaviour - nothing works this way outside of unity)

@AngryAnt
Copy link

AngryAnt commented Nov 7, 2015

I don't see the point in the GetComponent variant of this exception. However is this the same exception thrown if a reference is serialised, but the target then goes missing? That one is very handy indeed and I would hate to see it go.

Compared to the GetComponent call, that scenario clearly highlights a specific bug of unexpected state.

@ZimM-LostPolygon
Copy link

There is a yet another issue with this. I've personally wasted half a day trying to track it out down.

Renderer someRenderer = GetComponent<Renderer> ?? someOtherObject.GetComponent<Renderer>

obviously, this resulted in different results between editor and player, as null coalescence operator ignores operator overload and actually returns the fake-error-string-containing-object instead of branching to the right side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment