Skip to content
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

NotNullFinder fails to report Null entries on Arrays #8

Open
edwardrowe opened this issue Jan 3, 2017 · 6 comments
Open

NotNullFinder fails to report Null entries on Arrays #8

edwardrowe opened this issue Jan 3, 2017 · 6 comments

Comments

@edwardrowe
Copy link
Contributor

When you apply a NotNullAttribute to an array, each element gets the warning in the editor as expected, which is sweet. However, when doing the "NotNull" search no errors are given for null members.

Expected behaviour (WIP):

  1. Arrays should error for every member that is not wired up.
  2. Arrays that have no size should not assert. This is admittedly strange because why is an array of 0 given a require wire attribute? In general, requireWire doesn't make much sense on arrays.
  3. Another implementation is to have an ArrayNotNull attribute that requires the array is a certain size.
@ghost
Copy link

ghost commented Jun 20, 2018

+1

@kobyle69
Copy link

Would really love that, is that something you can add?

@edwardrowe
Copy link
Contributor Author

Thanks for both of your feedback - it's helpful to know what people would want with it, and that people are using this!

I don't have a lot of bandwidth right now to work on this project, but this should be doable since NotNull uses a PropertyDrawer (which replaces the way the property is drawn entirely, so we can do whatever we want).

Here's some notes to get future me started (or anyone who wants to do a Pull Request for Hacktoberfest!)
We'd want to replace the way the property is drawn for arrays (it's drawn here:

private void BuildObjectField(Rect drawArea, SerializedProperty property, GUIContent label)
). Thinking we'd need to use serializedProperty.Next(enterChild) to draw each element with an optional '*' and info box. We'd also need to report the height of the property based on how many elements have errors (and if it's expanded)

@kobyle69
Copy link

Hey @edwardrowe,

Actually, for me there is no problem with the inspector itself, it shows pretty well the null warning even within arrays and dictionaries.

However, the 'Not Null Finder' is a different story, it seems like that ReflectionUtility.GetFieldsWithAttributeFromType won't return fields within arrays/dictionaries.

@edwardrowe
Copy link
Contributor Author

Ok I am able to see what you're saying. I'm going to rename this issue to better reflect the problem.

Also some repro steps to clarify:

  1. Create a MonoBehaviour with a serialized array
  2. Mark the array [NotNull] (and public / serialized)
  3. Create a GameObject instance that uses the MB from step 1
  4. Add a few elements to the array. Note each element shows a warning, as expected.
  5. Run NotNullFinder. Observe the fields don't report in the Console.

I'd expect a log entry like:

[NotNullViolation: Field=Array/Element0, FullName=MyGameObjectInstance]

@edwardrowe edwardrowe changed the title Improve NotNull behavior on arrays NotNullFinder fails to report Null entries on Arrays Sep 28, 2018
@edwardrowe
Copy link
Contributor Author

Apologies for the confusion. My fault totally - the description of the task says it right front and center what you're talking about.

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

No branches or pull requests

2 participants