MonoDevelop: What's the most professional approach to solve seeing the CS0649 warning?

The script below shows a CS0649 warning in MonoDevelop.

Code (csharp):

1.  
2. public abstract class MyScript : MonoBehaviour{
3.   [SerializeField] GameObject go; << CS0649 Warning on this line
4.   void Start(){
5.     Debug.Log(go.name);
6.   }
7. }

The variable "go" is meant to be set from Unity Editor's inspector.

I can only think of these 2 fixes:
1) Supressing the warning: Configure Rule by disable the warning at monodevelop's source analysis.
2) Initializing the variable: Set go to null in the script.

Can you think of any other fixes?
Which is the most professional approach?
Final question: Does configuring the rule settings in MonoDevelop only configure it for the current Unity project, or does it also apply to all other Unity projects opened in the future?

 

Professional includes code tags and citing the name of the error.

If the error bothers you, just assign the field a default value. Doesn't really matter if you don't use it.

I would advise against suppressing the warnings altogether. They are useful from time to time. But you can use compiler directives to suppress the warnings in a single script.

 

There is no error in the code, just this warning CS0649.

The purpose of asking this question is to know what would professionals do, i.e. the title of this thread.
I interpret that you are suggesting that the most professional approach is to assign a default value to the variable for all instances in all my scripts?
Please correct me if I'm wrong.

Is there any compiler directive that I can use to tell the compiler to suppress the warnings for 1 specific variable, i.e. those that should be set thru the inspector (not the whole script)?

 

We suppress the warning just before the code that generates it and then restore it immediately after.
In your case - something like this:

Code (csharp):

1.  
2. public abstract class MyScript : MonoBehaviour {
3.     #pragma warning disable CS0649
4.     [SerializeField]
5.     private GameObject go;
6.     #pragma warning restore CS0649
7.  
8.     private void Start() {
9.         Debug.Log(go.name);
10.     }
11. }
12.  

https://msdn.microsoft.com/en-us/library/441722ys.aspx

 

Thanks! I'll be using this because I have evaluated the options and find personally found this to be the most suitable solution.

 

Okay, I just had a new problem.
I have explained it in this picture:

Any solution?

 

Remove all of that and just initialize it to null. You can still set it in the inspector just fine, and it'll override the default value (like with any other object type). Suppressing warnings is a really bad habit to get into, and as BoredMormon said the "professional" approach here is not to suppress it.

 

Thanks for the suggested best solution!

I have 4 questions left:
1) Besides the options of a) setting it to a default value, or b) supressing the warning, are there any other good solutions that I'm not aware of?
2) Knowledge Question (please don't question it's practicality): Are there any performance differences during initialization, between each of these 3 lines of code?

3) Any reason why unity doesn't modify MonoDevelop to ignore warnings/suggestions for fields that contains [SerializeField] attributes when running through Unity c# scripts?
4) I see a jagged underline that shows something is wrong/something can be done better. Sorry for the scripting OCD, but what should I do now to satisfy the source analysis?

+

 

Sure. You can not be OCD and just let the warning sit in the console doing nothing.

The first won't compile, and might lead to unexpected behaviour. Better to get a null reference if the reference isn't assigned then to have the wrong object run its course. There is no difference on the second two.

That's a fair bit of effort for not much benefit. Especially considering Unity is slowly moving away from MonoDevelop as the default IDE.

 

1. a) You should always initialize a reference type immediately or soon after you declare it.
b) A great option is Injecting/Inversion of Control. where the reference is passed into the class instead of the class being responsible for it. in a sense thats what you are doing through unity's inspector, Unity is injecting the specific instance you want into that field. but you can also inject through a function call which has a lot of advantages to making clean, modular, and testable code.
c) suppressing a warning/error isn't a "good" solution when you have options to deal with it properly, otherwise you're just throwing a problem under the rug. and that is definitely not a professional solution

2. the performance difference is negligible. its far more important that you have clarity and correctness.

3. cause a warning is still a warning. the compiler can only use the info it has access to at compile time. that warning is there to "warn" you that while something is likely making sense for compile time, it might not work as you intend at runtime and its not 100% sure. its a flag that's telling you that your code can work but it has holes where you can improve.

4. its because you specified "GameObject go" as a private variable (which Unity is allowed to access via reflection because you provided the [SerializeField] attribute) from the compliers perspective nothing outside the class can access the field (since its implictly private) yet nothing inside the class changes it, so its recommending that you make it read-only.

try writing like below

Code (CSharp):

1. public class Gallery:Monobehaviour
2. {
3.     public GameObject go = null;
4.  
5.     private void Start()
6.     {
7.         if(go != null)
8.             Instantiate(go);
9.     }
10. }

 

I should also point out that a compiler warning does not mean your code can be improved or made better. It just means that the compiler encountered a pattern that might not be what the developer intended. Errors mean your code is wrong. Warnings just mean your code is unusual. Unusual is not the same as wrong or needs improvement.

None of the changes anyone has suggested to deal with the error actually make the code objectively better.

 

From most/all comments, I see that supressing warnings is highly unprofessional, and definitely not recommended, so I'll be leaving supression as "not a good solution".

@JoshuaMcKenzie, you say that

, and

.
If the act of "Unity injecting the specific instance I want into that field" is a subset of "always initialize a reference type immediately or soon after you declare it", would setting it to null be counter-intuitive?

Which should take precedence? intuition or satisfying the warning?

Why does C# even allow the setting of private variables with reflection, when the visibility scope clearly states private, beit explicit or implicit.

 

The same reason that you can override a method that is not virtual (using the new keyword) - there are some very specific, niche cases (f.e. when dealing with legacy libraries that you can't modify/don't have source code for) when this can be useful. Hence it's a warning, not an error, as there are valid uses for it.

But it's not common, hell, using reflection at all isn't (or shouldn't be...) commonl. It's just that a private field with the SerializeField attribute can be accessed by the inspector. It's how it was designed.

Readability. I would take readable code 99/100 times over "super correct, but confusing as hell", as long as it works and does not drag on performance.
The usual compiler warnings are useful - it is unusual that we're setting private fields from outside the class without any setter methods involved -, but they have been made for "normal" C#. Unity has some "magic" to it, like this injection, or the null checks, among probably a couple others.

 

@anyone
Taking into consideration that you clearly see that 'go' has the [SerializeField] attribute attached to it, do you think it's more readable to 1) initialize it to null or 2) not initialize it, and ignore the warning?

 

Well, now you're asking about opinions
From the 2 - I would initialize it to null to make the compiler happy, and since this line of code is straightforward.

Or just make the field public and be done with it - private fields being set from outside somehow strike a bad vibe in me anyway, even if I understand why you'd want that.

From other options - I wouldn't rely on Inspector to set the values and would probably use a script in Start() to initialize it (or possibly do a if(go == null) go = [some finding function or get component or w/e], so that it's set only if we didn't do it in Inspector - it's good to check if it's set anyway before trying to access it).
As "bad" as Find is, if used sporadically it is useful and makes it immediately clear how the field is set. Relying solely on the inspector and no checks is risky.
But I guess that's just my opinion -

 

I'm going to throw out the opposite opinion. Variables set in the inspector should be your default method of initialising anything. Only initialise a value in script if that's meant to be an acceptable default value. Unity's workflow is built around the inspector.

You should typically use private varianles for you inspector, unless they are designed to be also set by other scripts.

I'd also advise against null checks, just in case a value is forgotten in the inspector. If someone forgets to assign a value, you want to throw an error quickly so it can be fixed. Not hide it under a rug.

 

I tend to side more with BoredMormon's view here- I set all variables I want exposed to the inspector as private with the SerializeField attribute, otherwise they can be accessed by other components in ways that I don't really intend. If I need to make something available to other components, I use properties and not fields anyways, and if I want both then I'll serialize the backing field for the property (pretty rare, that).

That said, I tend to be more flexible about null-checks. If something absolutely needs to be assigned in the inspector to operate, you should null-check it in Awake and fire off your own warning or error, as appropriate. There are cases where the value won't get used until long after the script has been enabled, and I don't like surprises. However, I also tend to lean toward the use of optional inspector assignments as overrides in the case of things like UI management. I have scripts that will use GetComponent, GetComponentInParent, or GetComponentInChildren (as appropriate) to find a component if one isn't manually assigned- if it's still not found though, then I'll explicitly fire off a warning.

 

It also seems logical (to me) to not set any default value to the variable, because intuition tells me that the act of assigning a value to the variable, in any situation, means that you will use it, but that's not the case here.

From what you said, I presume you do this:
[SerializeFIeld] GameObject go;
How do you cope with warnings? Do you ignore or supress them? Or do you have some other trick that prevents the warning?

 

That's exactly what I do. Normally I just ignore the warnings. They really don't bother me that much.

If it really bothers you you can always fake some code that does an assignment. You really are sacrificing some readability here just to make a warning that makes no difference to the performance of your code go away.

 

I just make inspector variables public. I know some people think this is bad because it "exposes" them, but in my opinion, a variable that can be modified at any time by an outside source like the editor already is exposed. You need to write Unity code assuming that those [SerializeField]private variables are all acting as if they're public anyway, so you're not saving an awful lot by hiding them from yourself. I just avoid ever writing code that directly accesses public variables; code only accesses properties. That default warning and Resharper and many other tools I use all throw a fit about writing code where it's assumed that private variables are getting changed by outside forces, because it's dumb and you shouldn't do it. Almost no one here would write their own code by making variables private and then using reflection to modify them as if they were public. So I don't think it makes that much sense to force Unity to do that instead to enforce the illusion of encapsulation. True encapsulation relies on the idea that private variables won't be randomly modified by an outside force except in extremely rare conditions; it doesn't work well once you make "fake" private variables a common pattern.

 

But what if you get one useful warning, and it got drowned in hundreds of lines of those kinds of warnings that you mean to ignore?

I also realised that Unity's console MAGICALLY!!! ignores all the CS0649 warnings for variables that has the [SerializeField] attribute.

Is there any way that I can apply this magic filter to MonoDevelop?
Anyone knows if you can set the warning levels for the Unity Editor's console window?

 

Interesting take. My set up is different. Something along these lines.

[SerialiseField] private: I use this for all variables set up in the inspector. I basically assume that an inspector variable sets the initial conditions for a script, much the same as a constructor.

public: This indicates that a variable or method is expected to be modified from another script.

I only use properties if i need data verification or some other trickery. I've never quite seen the point of the 'never access a field, only is properties' school of thought.

 

For me, when I want to make a field accessible in the inspector I simply make it public and, of course, initialized. I typically use the initialization as default values to speed up setting up the component when I add it in the inspector, as all the common values are already entered.

If I don't want a field to be seen in the inspector (for example if its some field that's set at runtime anyway) I typically use [hideInInspector] and leave it public just so that it doesn't confuse a level scripter yet still remain accessible for any script that may need it for whatever reason when the occasion calls.

For Monobehaviours I like making them mostly transparent (aside from any backing fields) I never know when I'll eventually make a new script that could really use that one field that I made private. System.Object classes and some ScriptableObjects will usually have degrees of encapsulation depending on the pattern I'm going for. Events for example I usually encapsulate behind a AddListener/RemoveListener/ClearListeners call just so that no other class can invoke it at inappropriate times. My state machines also usually has a heavy amount of encapsulation.

Verification, and auto-caching are a couple of uses, another major use is that properties can be implemented to interfaces. Fields however cannot be binded to specific contracts. so if you want some level of abstraction when passing data then Properties are the way to go. however one downside to properties is unitys inability to consistently find them via reflection when setting up Unity events