-
Notifications
You must be signed in to change notification settings - Fork 226
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix S1104 FP: Do not report in Unity serializable classes #9514
Conversation
KnownType.UnityEngine_MonoBehaviour, | ||
KnownType.UnityEngine_ScriptableObject); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding from the docs is that you should include UnityEngine.Object
.
Which, if I'm not wrong, is parent to UnityEngine_ScriptableObject
so you can replace one with the other.
But for custom classes which don’t derive from UnityEngine.Object, Unity includes the state of the instance directly in the serialized data of the MonoBehaviour or ScriptableObject that references them. There are two ways that this can happen: inline and by [SerializeReference]
.```
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding UnityEngine.Object
is not enough, the class should also have the Serializable
attribute.
I can add it so it also supports custom serializable unity class/object.
I will rework a bit the logic then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After a closer look, the required attribute for custom objects is System.Serializable
, and this is already an exception to the rule.
Edit 1: Never mind, I didn't realize we were only checking if the field had the attribute. I have pushed my updates.
Edit 2: I am having strokes, we do check if the parent class has the Serializable field. In any case, I have added more UTs to check for custom Unity serializable classes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good - one minor change.
2dadf8b
to
350fd89
Compare
Quality Gate passed for 'Sonar .NET Java Plugin'Issues Measures |
Quality Gate passed for 'SonarAnalyzer for .NET'Issues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Fixes #7522
Ignore fields in the following Unity classes: