Skip to content

Fixed InputActionDrawerBase not initializing the TreeView correctly to fix NullReferenceException when using Prefab Overrides window#1567

Merged
jamesmcgill merged 6 commits intodevelopfrom
fix-action-drawer-prefab-overrides-window
Sep 29, 2022
Merged

Fixed InputActionDrawerBase not initializing the TreeView correctly to fix NullReferenceException when using Prefab Overrides window#1567
jamesmcgill merged 6 commits intodevelopfrom
fix-action-drawer-prefab-overrides-window

Conversation

@chris-massie
Copy link
Copy Markdown
Collaborator

Description

This PR addresses a bug where the Prefab Overrides dropdown window would trigger a NullReferenceException when drawing a component that has an InputActionProperty with a singleton action (i.e. Use Reference unchecked).

Changes made

  • This change makes sure the TreeView.serializedObject matches the property. The value was null which was causing OnGUI to throw an exception.
  • The change to UpdateSerializedObjectDirtyCount was not necessary for this fix but it should protect against cases where the serializedObject becomes null for some unknown reason. The EditorUtility.GetDirtyCount method returns 0 when the argument is null, so this seems like the appropriate thing to do.

Notes

This was causing exceptions when looking at overrides of XRI components.

ArgumentNullException: Value cannot be null.
Parameter name: _unity_self
UnityEngine.InputSystem.Editor.InputActionTreeView.UpdateSerializedObjectDirtyCount () (at Library/PackageCache/com.unity.inputsystem@1.3.0/InputSystem/Editor/AssetEditor/InputActionTreeView.cs:1394)
UnityEngine.InputSystem.Editor.InputActionTreeView.ReloadIfSerializedObjectHasBeenChanged () (at Library/PackageCache/com.unity.inputsystem@1.3.0/InputSystem/Editor/AssetEditor/InputActionTreeView.cs:1400)
UnityEngine.InputSystem.Editor.InputActionTreeView.OnGUI (UnityEngine.Rect rect) (at Library/PackageCache/com.unity.inputsystem@1.3.0/InputSystem/Editor/AssetEditor/InputActionTreeView.cs:1173)
UnityEngine.InputSystem.Editor.InputActionDrawerBase.OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label) (at Library/PackageCache/com.unity.inputsystem@1.3.0/InputSystem/Editor/PropertyDrawers/InputActionDrawerBase.cs:31)
UnityEditor.PropertyDrawer.OnGUISafe (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.PropertyHandler.OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren, UnityEngine.Rect visibleArea) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.PropertyHandler.OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.EditorGUI.PropertyFieldInternal (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.EditorGUI.PropertyField (UnityEngine.Rect position, UnityEditor.SerializedProperty property, System.Boolean includeChildren) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.EditorGUI.PropertyField (UnityEngine.Rect position, UnityEditor.SerializedProperty property) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEngine.InputSystem.Editor.InputActionPropertyDrawer.OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label) (at Library/PackageCache/com.unity.inputsystem@1.3.0/InputSystem/Editor/PropertyDrawers/InputActionPropertyDrawer.cs:77)
UnityEditor.PropertyDrawer.OnGUISafe (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.PropertyHandler.OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren, UnityEngine.Rect visibleArea) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.PropertyHandler.OnGUI (UnityEngine.Rect position, UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.PropertyHandler.OnGUILayout (UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren, UnityEngine.GUILayoutOption[] options) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.EditorGUILayout.PropertyField (UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, System.Boolean includeChildren, UnityEngine.GUILayoutOption[] options) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.EditorGUILayout.PropertyField (UnityEditor.SerializedProperty property, UnityEngine.GUIContent label, UnityEngine.GUILayoutOption[] options) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.XR.Interaction.Toolkit.ActionBasedControllerEditor.DrawTrackingConfiguration () (at C:/UnitySrc/com.unity.xr.interaction.toolkit-2.1/com.unity.xr.interaction.toolkit/Editor/Interaction/Controllers/ActionBasedControllerEditor.cs:93)
UnityEditor.XR.Interaction.Toolkit.XRBaseControllerEditor.DrawProperties () (at C:/UnitySrc/com.unity.xr.interaction.toolkit-2.1/com.unity.xr.interaction.toolkit/Editor/Interaction/Controllers/XRBaseControllerEditor.cs:135)
UnityEditor.XR.Interaction.Toolkit.XRBaseControllerEditor.DrawInspector () (at C:/UnitySrc/com.unity.xr.interaction.toolkit-2.1/com.unity.xr.interaction.toolkit/Editor/Interaction/Controllers/XRBaseControllerEditor.cs:112)
UnityEditor.XR.Interaction.Toolkit.BaseInteractionEditor.OnInspectorGUI () (at C:/UnitySrc/com.unity.xr.interaction.toolkit-2.1/com.unity.xr.interaction.toolkit/Editor/BaseInteractionEditor.cs:28)
UnityEditor.PrefabOverridesTreeView+ComparisonViewPopup.DrawEditor (UnityEngine.Rect rect, UnityEditor.Editor editor, System.Boolean disabled) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.PrefabOverridesTreeView+ComparisonViewPopup.OnGUI (UnityEngine.Rect rect) (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.PopupWindow.OnGUI () (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEditor.HostView.OldOnGUI () (at <a5bc71bb4d98498aba1f868b89d20d47>:0)
UnityEngine.UIElements.IMGUIContainer.DoOnGUI (UnityEngine.Event evt, UnityEngine.Matrix4x4 parentTransform, UnityEngine.Rect clippingRect, System.Boolean isComputingLayout, UnityEngine.Rect layoutSize, System.Action onGUIHandler, System.Boolean canAffectFocus) (at <c735460d95a241ccbea413909634f410>:0)
UnityEngine.GUIUtility:ProcessEvent(Int32, IntPtr, Boolean&)

Checklist

Before review:

  • Changelog entry added.
    • Explains the change in Changed, Fixed, Added sections.
    • For API change contains an example snippet and/or migration example.
    • FogBugz ticket attached, example ([case %number%](https://issuetracker.unity3d.com/issues/...)).
    • FogBugz is marked as "Resolved" with next release version correctly set.
  • Tests added/changed, if applicable.
    • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
    • Performance tests.
    • Integration tests.
  • Docs for new/changed API's.
    • Xmldoc cross references are set correctly.
    • Added explanation how the API works.
    • Usage code examples added.
    • The manual is updated, if needed.

During merge:

  • Commit message for squash-merge is prefixed with one of the list:
    • NEW: ___.
    • FIX: ___.
    • DOCS: ___.
    • CHANGE: ___.
    • RELEASE: 1.1.0-preview.3.

- This change makes sure the TreeView.serializedObject matches the property.
The value was null which was causing `OnGUI` to throw an exception.
- The change to `UpdateSerializedObjectDirtyCount` was not necessary for this
fix but it should protect against cases where the serializedObject becomes
null for some reason. The `EditorUtility.GetDirtyCount` method returns 0
when the argument is null, so this seems like the appropriate thing to do.
@chris-massie chris-massie requested a review from andrew-oc July 26, 2022 23:15
@ekcoh
Copy link
Copy Markdown
Collaborator

ekcoh commented Aug 18, 2022

@chris-massie I updated the changelog conflict and can merge this one as soon as CI is green.

@chris-massie
Copy link
Copy Markdown
Collaborator Author

@ekcoh I updated the changelog conflict again and the CI tests are all passing. This is ready to merge.


### Changed
- Improved performance of HID descriptor parsing by moving json parsing to a simple custom predicitve parser instead of relying on Unity's json parsing. This should improve domain reload times when there are many HID devices connected to a machine.
- Fixed `ArgumentNullException` when opening the Prefab Overrides window and selecting a component with an `InputAction`.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this should be under the [Unreleased] section?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in the middle of the release just now, so develop doesn't have the [Unreleased] section right now. That's probably why it's went here.
I will move this into [Unreleased] after I put it back and bump to v1.4.4 for the next release.

@jamesmcgill jamesmcgill merged commit c1e48c9 into develop Sep 29, 2022
@jamesmcgill jamesmcgill deleted the fix-action-drawer-prefab-overrides-window branch September 29, 2022 07:01
@jamesmcgill
Copy link
Copy Markdown
Collaborator

Thanks for the PR @chris-massie !

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants