Search Unity

  1. Unity Asset Manager is now available in public beta. Try it out now and join the conversation here in the forums.
    Dismiss Notice
  2. Megacity Metro Demo now available. Download now.
    Dismiss Notice
  3. Unity support for visionOS is now available. Learn more in our blog post.
    Dismiss Notice

Warning CS0649 not suppressed properly when field is marked as [SerializeField]

Discussion in '2018.3 Beta' started by xVergilx, Sep 15, 2018.

  1. xVergilx

    xVergilx

    Joined:
    Dec 22, 2014
    Posts:
    3,296
    When scripting version is set to 4.x the warning CS0649 is not suppressed for private / protected fields marked with [SerializeField]
     
    Last edited: Sep 17, 2018
  2. JoshPeterson

    JoshPeterson

    Unity Technologies

    Joined:
    Jul 21, 2014
    Posts:
    6,920
    Is the behavior different with the .NET 3.5 Equivalent scripting runtime?
     
  3. xVergilx

    xVergilx

    Joined:
    Dec 22, 2014
    Posts:
    3,296
    Yes, they're completely ignored.
     
  4. JoshPeterson

    JoshPeterson

    Unity Technologies

    Joined:
    Jul 21, 2014
    Posts:
    6,920
  5. Peter77

    Peter77

    QA Jesus

    Joined:
    Jun 12, 2013
    Posts:
    6,589
    As far as I remember, this matches the behavior as if you compile in Visual Studio. I find it useful that Unity spits out the CS0649 warning, as it matches VS. I prefer to write and compile in VS and having Unity and VS match warnings is beneficial to me.
     
  6. Racoon_7

    Racoon_7

    Joined:
    Nov 11, 2017
    Posts:
    11
    Thank you for looking into the issue, but I don't really agree with it being marked as By Design. I mean, yes; without the attribute, it absolutely should output the warning, but with it, the field can be edited through Inspector, so it no longer makes sense.

    Using the attribute in a way which was intended makes it work as intended – but now also produces a warning which cannot be removed – because it's not a mistake.

    In another thread, I posted a few ways I thought of to suppress the warning, but none of it sounds entirely ideal.
    It was a bit annoying seeing the squiggly lines in Visual Studio, but in the current beta, having 70 “warnings” outputted in the Console each time Unity compiles is on entirely different level.
    Matching compile warnings are certainly beneficial, I agree, but if it turns one of Unity's features into a warning By Design, I think I'd rather avoid seeing it.
     
    ModLunar, jashan, TextusGames and 6 others like this.
  7. xVergilx

    xVergilx

    Joined:
    Dec 22, 2014
    Posts:
    3,296
    @Racoon_7 It can be avoided by assigning default values to the properties.
    But going through a ton of code simply to get rid of the 100+ warnings will be real pain.

    Not sure why the heck it should be "By Design". Obviosly Unity's design is different. We're not writing C# bussintheass applications here, please stick to your own enforced previously conventions.
     
  8. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,294
    Gonna second everyone else here, having this be "by design" is insane. You're essentially saying that using the attribute in the intended way, as you show in the docs, should spam warnings.

    What?

    This really drags out a big, big annoyance with the issue tracker. Whenever something annoying is "by design", it's generally really aggravating for users that are affected by the issue. But there's almost never an explanation for why it's by design. Generally, you have pretty good reasons for not fixing an issue, which if spelled out would satisfy most people running into the issue - or at least give enough reasoning for them to let it lie. But when that isn't written down, you end up with frustration (and forum rage) instead.

    Whenever something is marked as either by design, or as "won't fix", there should really be a mandatory field where an explanation for why this isn't a bug that will be fixed is written down. If you add a special field to the issue tracker to explain why something has the label it has (unless it's "will be fixed"), it would be much easier for us to understand your intentions, and generally breed more confidence in the community that you're taking our issues seriously.

    Now I'm guessing that this time it's just a miscommunication - since you replied to the thread - but I just have to assume. If you take the "by design" at face value (which you would do if you didn't happen to read this thread), it seems directly incompetent.
     
  9. joncham

    joncham

    Unity Technologies

    Joined:
    Dec 1, 2011
    Posts:
    276
    Hello,

    I responded in bug with reasoning of why issue was closed and workarounds. It didn't make it to public issue tracker.

    The new C# compiler (Roslyn) is correct in reporting the additional warnings. This is same as you would get in any C# project/code in VS. The mono C# compiler did not report them in past.

    You can disable the warnings with pragmas:

    #pragma warning disable 0649
    // your code
    #pragma warning restore 0649

    Or disable it globally by adding a csc.rsp file to your project and adding command line switch to disable the warning: -nowarn:0649

    Thanks,
    Jonathan
     
    gooby429, Marrt, Baylamon and 5 others like this.
  10. xVergilx

    xVergilx

    Joined:
    Dec 22, 2014
    Posts:
    3,296
    But that would disable the actual useful warnings as well. Going through all source code and marking them as excluded is no solution. I might simply initialize them to default values instead, if that's the case.

    I wonder. Is there any code that performs filtering in native console (Editor native side)?
    It would be much more useful to check if those fields are actually marked as [SerializeFields], then simply don't output the message if it's true.

    Default console is now a broken toaster with all the warnings that will crop out.
    (Send Messages in OnValidate when checking if object is a prefab, this one, multiscene setup warnings. ugh)
     
    Last edited: Sep 26, 2018
  11. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,294
    You really should, every time. This is what I meant earlier - when you don't make replies to bugs public, you cause massive frustration in the community. For the specific person reporting the bug, they get a reason for why the bug was closed without a fix, but for the rest of the community, it's just a big middle finger saying "deal with it".
    As a different example, here's a bug I reported. I had a pretty pleasant back-and-forth with the QA team, and I got a satisfactory reason for why it won't be supported. I'm happy. For the rest of the community, that page says "Undo is broken by design".
    This is a failure of communication, and it's so easy for you to fix it! You have written the reason down somewhere, you literally just have to copy-paste it.

    Also jeez louize this is not the correct thing to do. The compiler is wrong, since it doesn't understand the semantics of the [SerializeField] attribute. The correct thing to do here is to go in and suppress this warning before it hits the console. Rider/Resharper does just that with the Unity plugin, I assume VS'/VS Code's Unity plugin does that same? They should.

    If developers follow the standard community guidelines, every single serializable field that doesn't have a good reason to be public is marked as [SerializeField] private. We'll be spammed with thousands and thousands of warnings for doing the correct thing. Your workarounds have major downsides - either turn the warning off (which is a bad idea, it's a useful warning), or add thousands of #pragmas to projects that clutter everything up.

    As @VergilUa is alluding to, the 2018 cycle is spamming down the console with crap we can't do anything about to the point where it's becoming near useless. It's an important tool for us, and you're essentially degrading it with every update to the engine.
     
  12. joncham

    joncham

    Unity Technologies

    Joined:
    Dec 1, 2011
    Posts:
    276
    I have added the informative text and the public issue tracker info will be updated with it shortly.
     
    optimise likes this.
  13. LeonhardP

    LeonhardP

    Unity Technologies

    Joined:
    Jul 4, 2016
    Posts:
    3,132
    No argument here. This is a problem and we need to solve it. Thanks for the clear words, we're looking into this.
     
    Erethan, tobsam, BradZoob and 11 others like this.
  14. xVergilx

    xVergilx

    Joined:
    Dec 22, 2014
    Posts:
    3,296
    So, it won't be fixed, am I right?

    Should I start refactoring all my code base then?
     
  15. Racoon_7

    Racoon_7

    Joined:
    Nov 11, 2017
    Posts:
    11
    Hm. Thank you for the information. "It's not possible to fix this" sounds better than "we want it to be like this".

    What about Visual Studio Tools for Unity? Could it be fixed from their side? Or is it just “whatever the compiler spits out ends up in the console”?

    @VergilUa – Thanks for the tip about initialising the fields with their default value. The IDE still complains ("Redundant initialization with a default value"), but thankfully the warning in Unity is gone.
    Because this issue involved a bunch of packages from the Asset Store which I didn't want to spend much time refactoring, I ended up "fixing" this "error" with a regular expression.

    Since C# 7.1 you can do something like:
    Code (csharp):
    1. [SerializeField] float runSpeed = default; // → 0f
    2. [SerializeField] Texture2D texture = default; // → null
    So I think I replaced every field without an 'equal' sign on the same line (= unassigned)
    \[SerializeField\]([^=]+?);

    with
    [SerializeField]\1 = default;

    It helped me get rid of a few dozen warnings at once.
     
    ikvv, phobos2077, Lahcene and 7 others like this.
  16. xVergilx

    xVergilx

    Joined:
    Dec 22, 2014
    Posts:
    3,296
    It's not related to the tools, but rather an actual Unity's native console.

    I was able to ignore multiple warnings selectively, but, with a different, custom written console.
    So it's possible, just UT seems to be on the lazy side. Or, because console is C++, it might be hard for them to pull the attributes for the actual field. Who knows, without the source - it's impossible to tell the reason.

    As a result, I ended up disabling default console and using custom one instead. Though, Application.logMessageReceived isn't as reliable as well. Sometimes it looses some messages sent in Editor.

    Although, this is not a scalable solution, as people won't be using same console setup. So it's no go for distributable packages.

    This warning message is definately should be ignored by default when console receives it.
     
  17. Baste

    Baste

    Joined:
    Jan 24, 2013
    Posts:
    6,294
    I hadn't thought of that. To be honest, that makes this a lot less severe than I first thought.
     
    Lahcene likes this.
  18. mdeletrain

    mdeletrain

    Joined:
    Jun 8, 2011
    Posts:
    47
    Alas you cannot inialize fields this way on structs :

    Assets/Scripts/...: error CS0573: '...': cannot have instance property or field initializers in structs


    So the problem remains : either lots of warnings are lots of pragmas for something done the expected way.
     
  19. OleJuergensen

    OleJuergensen

    Joined:
    Mar 21, 2018
    Posts:
    11
    Hi all, Hi @joncham ,

    is there really no way this can be revisited? I personally think the workarounds are not acceptable. This will lead to projects being cluttered by compiler warnings and serious issues will be overlooked. Initializing everything explicitly by the default value just moves the issue to an IDE warning that will have to be ignored globally, with the same result of poor code quality.

    I was under the assumption that this warning was indeed reported by the compiler even before, but was filtered out by the Unity console for fields with the attribute [SerializeField]. At least that could still be done somehow, I am quite convinced. Of course we cannot expect the Unity team to modify the compiler for this, but there should be something possible on the Unity side of this.

    I really would appreciate this to be tackled. Thank you very much for your effort!
     
  20. Rallix

    Rallix

    Joined:
    Mar 17, 2016
    Posts:
    139
    I'd also like to see this problem fixed.
    I can't add much to the good points which have already been said in this thread… it's just a toss-up between using the recommended way + OOP's best practices, and making everything public to avoid an incorrect warning.

    I imagine that after 2018.3 goes out of its beta stage, a couple more complaints about this will pop up since this is a problem which will likely be present in most mid to large-sized projects.
    I just updated an Asset Store package and had to open twelve files just to slap #pragma (…) at the top. It makes absolute sense for a third-party code – and far from only that – not to expose all the variables, yet this encourages it, as it's the simplest solution. The other option is to hide a useful warning in order to avoid a few false positives – and I don't really think the 'proper' way to write a Unity component should contain a step such as: "2) Write a preprocessor directive to disable inevitable warnings".
     
  21. xVergilx

    xVergilx

    Joined:
    Dec 22, 2014
    Posts:
    3,296
    I'd say there will be a bazillion threads on the forum "why my variables are messed up and spit warnings". Not to mention that this will enrage a lot of people.

    In the end, this is thing that can be intercepted by Unity's console.

    For my hobby project, I've ended up disabling highlighting for redundant assignments and setting each variable to default (100+ ones).

    Not really an option for third-party plugins though.
    Maybe global rsp is an answer for this.
     
  22. Rallix

    Rallix

    Joined:
    Mar 17, 2016
    Posts:
    139
    I used a global rsp file for a while, then I deleted it because I didn't like it sitting at the root of Assets folder, not to mention hiding all 0649 warnings everywhere seemed like something I really shouldn't be doing, especially since I was working on a project with two other people and didn't want to force them to use this ‘hack’, nor have warnings resulting from my code pop out.
    So I did what you did, but as @mdeletrain mentioned, you can't set
    default
    in a struct, so I had to go through a few files manually.

    And as for third-party Asset Store publishers, they can't really include a rsp file in a package either.
     
    phobos2077, Lahcene, halley and 2 others like this.
  23. Roycon

    Roycon

    Joined:
    Jul 10, 2012
    Posts:
    50
    I agree, I think this should be fixed somehow, if there was some way to filter the console that would be a good step

    I've been using the betas since they came out and I've just turned off warnings in the console. I'm sure there are some important warnings in there but they are lost in the 900+ field warnings
     
    Lahcene and Havokki like this.
  24. Ziflin

    Ziflin

    Joined:
    Mar 12, 2013
    Posts:
    132
    Just voicing our annoyance for this issue as well. #pragma's everywhere are a waste of time and make the code ugly (they'd end up in the majority of our files at this point). And the csc.rsp doesn't solve the errors in the IDE. This really needs to be fixed correctly by the VS team as it seems like there should be a global fix to this problem with any C# project (not just unity)... Something like a new attribute to prevent that warning placed on the
    SerializeFieldAttribute
    .
     
    futurlab_xbox and halley like this.
  25. joolsa

    joolsa

    Joined:
    Mar 20, 2013
    Posts:
    10
    @joncham @JoshPeterson

    First off great work with IL2CPP, the new runtime, forthcoming GC, etc. The first two of these we're using and the new runtime, in particular, has been rock solid since before it came out of experimental.

    On this thread's topic: I think this is going to be a massive hurdle for people upgrading from 2017.4 to 2018.4, from 2018.2 to 2018.3, and large ongoing quality of life issue.

    I'm going to reiterate the issues :) At least in our office, [SerializeField] is the canonical way of exposing a variable to the inspector, and having it saved without it being public. I think about half of our serialised / inspected variables are like that, rather than being public. We don't want to turn it off globally as it's a valuable warning (in other cases). It helped me out only this week! It's going to be a massive pain to upgrade, and it's going to encourage people to make variables public, rather than private + SerializeField.

    We run warnings as errors: we take the view that the compile needs to be clean to avoid hiding new issues, and that warnings are important enough to fix, or unimportant enough to turn off. (Other people who are definitely smart take this view also: https://twitter.com/lucasmeijer/status/1038048161263173632 :) So one way or the other we're going to need to fix this.

    The old behaviour might be considered a mono compiler bug, but really it's a question of interpretation. The reasoning for it not firing on public fields and SerializeField is basically identical, at least for Unity's case. One potential idea: if [SerializeField] could flag the warning, but have a separate error code for the SerializeField case *then* we could globally turn off that error, but not the existing cases. Hosestly though - why would we ever have it turned on for Unity use? It might be sensible like this for normal C# development, but this is Unity. It is different in many ways: the IDE, setting up dependencies and command line arguments, etc. As Unity grows and dll building and use becomes more prevalent it would be very helpful to have this as a separate warning for that case.

    Yes, this is patching the compiler. Yes, I think it's that important.

    Thanks,

    Jools
     
  26. jRocket

    jRocket

    Joined:
    Jul 12, 2012
    Posts:
    699
    I was able to get rid of these warning by setting the initial value to null. ie.
    Code (CSharp):
    1. [SerializeField] private GameObject m_member = null
    No need to set pragmas and or rsp files, and it didn't take that long to change.
     
  27. xVergilx

    xVergilx

    Joined:
    Dec 22, 2014
    Posts:
    3,296
    Well, default does exactly the same. But its clearly understatement about didn't take that long. In my semi-small project I had to manually add them to 100+ variables. What will happen on complex scale?
     
  28. jRocket

    jRocket

    Joined:
    Jul 12, 2012
    Posts:
    699
    It still shouldn't take that long to paste the = null at the end of every occurrence, especially since the console tells you exactly where they are. Maybe an hour for a larger project with hundreds of variables? More monotonous than anything else.
     
  29. Flavelius

    Flavelius

    Joined:
    Jul 8, 2012
    Posts:
    943
    +1 for fixing this please
     
  30. Rallix

    Rallix

    Joined:
    Mar 17, 2016
    Posts:
    139
    Well, not everything can be set to null (int, bools, floats…) but you can also set them to
    default
    , so let's say that's not a problem. However, somebody mentioned it's not always possible to do that (e.g. in structs).
    So now what, change all structs to classes? That doesn't take much time either, but you'd better check all assignments to make sure there aren't any problems with references. That takes a bit longer.

    Not to mention doing this creates another warning, redundant assignment of a default value – but thankfully, this one doesn't appear in Unity, otherwise, it'd become a game of ‘which warning you like better’.

    It shouldn't be a discussion about how hard it is to bypass the issue though – rather whether or not it should be happening at all.
    Your advice sounds like, “There is a hole in the road. But why should it be fixed when it doesn't take much effort to drive around it?”
     
  31. arvzg

    arvzg

    Joined:
    Jun 28, 2009
    Posts:
    619
    Well I just upgraded my project to 2018.3 beta and since I have quite a number of third party assets, I now have 100+ CS0649 warnings.

    I know the solution is to initialize all variables - but am I going to have to go in and edit tons of third party assets to get rid of these warnings?

    What about when the asset creators release an update to the asset, I update the files, which overrides any changes I made to it, and I again have to apply the fix manually?

    I believe this should be 'fixed' (or more accurately the old behaviour should be restored)
     
    Ony, phobos2077, halley and 2 others like this.
  32. Peter77

    Peter77

    QA Jesus

    Joined:
    Jun 12, 2013
    Posts:
    6,589
    Contact the asset creator and ask him/her to fix those warnings instead? It should be in the interest of the asset creator.
     
  33. gianfun

    gianfun

    Joined:
    Apr 12, 2015
    Posts:
    1
    I'd like to voice my annoyance with this issue as well.

    +1 to fixing this warning from appearing when tagged with [SerializeField]
     
    Lahcene likes this.
  34. Marakino

    Marakino

    Joined:
    Aug 31, 2018
    Posts:
    4
    +1 to this as well.

    It looks like this thread died, and it shouldn't have.

    I was truly amazed by seeing this issue marked 'as designed', when it's clearly a defect.
     
  35. LeonhardP

    LeonhardP

    Unity Technologies

    Joined:
    Jul 4, 2016
    Posts:
    3,132
    phobos2077, Lahcene, yc960 and 7 others like this.
  36. hangar

    hangar

    Joined:
    Feb 1, 2013
    Posts:
    21
    As a workaround until Roslyn adds a way to fix the warning correctly, I would suggest that you suppress 0649 by default when you ship 2018.3. Right now, you're already suppressing 0169.

    Until then, you have to put -nowarn:0649 in csc.rsp at the root of your assets folder, and for visual studio you have to add an editor script: See https://developercommunity.visualstudio.com/content/problem/254578/cant-disable-warning-cs0649.html (Use both the linked file and with the edit suggested by the comment.)