Should Unity lifecycle methods be annotated with the UsedImplicitly attribute?

Question

Does annotating Unity lifecycle methods with the UsedImplicitly attribute improve the readability of your code?

For example:

public class Example : MonoBehaviour
{
    [UsedImplicitly]
    private void Awake()
    {
        DoSomething();
    }

    private void DoSomething()
    {
        // ...
    }
}

This blog post on "Structuring Your Unity MonoBehaviours" suggests this approach as a useful convention.

8. Annotate any Unity lifecycle methods (Start, Awake, Update, OnDestroy etc.), event methods, and other functions that are implicitly (or automatically) invoked in your code with the [UsedImplicitly] attribute. This attribute, although included in the UnityEngine.dll, is used by ReSharper to disable code cleanup suggestions for methods that are seemingly unreferenced. It also helps to make the code easier to read for people who are newer to Unity and your codebase, especially for events that are referenced via the inspector.

(Note: I don't think ReSharper shows code-cleanup suggestions for seemingly unreferenced Unity lifecycle methods, so that might be outdated advice.)

I agree with the above post that it could be helpful for those newer to Unity. I also think it could be useful to label those Unity lifecycle functions that are not used as frequently as Awake, Start, and Update. But I am wondering if this is actually a good convention for "clean code", or if it's just noise that reduces readability.

Answer 1

It depends on the target demographic.

In the example above, the target demographic is the ReSharper tool, which benefits from these attributes because it suppresses messages which aren't helpful to you. But if you don't feel the need to use ReSharper or a similar tool (although maybe you should, they have some valid critique from time to time), then you don't need to care about that demographic.

Anyone who ever did a basic Unity tutorial knows very well that Start and Update are used implicitly by the Unity engine, so it won't give them any new information. It might in fact confuse the heck out of them if you forget this once. What do you want to say with that? Is this maybe not actually a MonoBehaviour? Or is there some obscure reason why you must call it explicitly which I am not aware of?

It can, however, help if you are working with inexperienced developers who might not be familiar with the more esoteric Unity events like Reset (dangerous, because calling it explicitly might not do what you think it does). The [UsedImplicitly] attribute might then tell them that they don't need to look for the class which calls that method because the engine does. But again, this is only of value if you have the discipline to do this consistently. And if you have that amount of self-discipline, you could just as well agree on adding a comment.

Answer 2

I argue that no you should not use it.

The UsedImplicitly attribute is from the Jetbrains.Annotations namespace so the only case where it might apply is if everyone using the code will be using ReSharper.

ReSharper has a plugin for Unity which already handles this for you, making sure not just to remove warnings that they are not used but also marking them with a little Unity symbol so that anyone reading the code can see that they are called by Unity itself.

I'd also like to add an example when using them might go wrong. Assume you have a class inheriting from MonoBehaviour, but after a refactor it no longer has any inheritance. If you marked private methods with [UsedImplicitly] then you will not receive the correct warnings. If you use the Unity plugin for resharper it will notice that you no longer inherit from MonoBehaviour and will throw warnings as the methods indeed are no longer called.

Answer 3

I'd make the argument that it can't really hurt.

For somebody very familiar with Unity's workings, it probably doesn't add anything. Those people are already going to be pretty familiar with what Unity's runtime calls on your behalf.

But for somebody who isn't, like me, it might be helpful. Even if I didn't know what it meant offhand, I'd go look it up, and that would probably be sufficient to give me a better understanding of what was going on in the code.

Also, if one does use static analysis tools that are incorrectly warning about the methods, then you'll want to quiet those warnings somehow, because "ignorable" warning noise makes it harder to see the real warnings in any such output. It's usually better to ignore such warnings in a surgical, localized fashion (in this case by decorating the offending methods with attributes) than through broader measures like disabling that specific warning project-wide.

Answer 4

The problem with this approach is that it's incredibly error prone.

If it isn't reliable, the tags will fail to convey useful information, and their presence or absence will occasionally succeed at conveying false information, which is harmful.

If you decide to go with it, you must remove the human error, which isn't hard to do, but takes a good 2 hours of work if you haven't done something like this before. There are 2 approaches - each with their own advantages - you can use either one:

1. Verify and reject non-complying code on check in or automated build.
2. Automated edit of code to fix tags on check in or automated build.

Is it worth to have it? Yes. Is it worth 2 hours of your time? Your call.