-
Notifications
You must be signed in to change notification settings - Fork 329
FIX: Address the TreeView compilation warnings when used with Unity 6.2 beta #2196
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
Conversation
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.
Pull Request Overview
This pull request addresses the 6.2 compilation issues for the treeview by updating several package dependencies and adding conditional using directives for generics-based TreeView types to support newer Unity versions. It also includes minor lambda formatting changes in the debugger components to improve code clarity.
- Updated dependency versions in Packages/manifest.json to ensure compatibility.
- Added UNITY_6000_2_OR_NEWER conditional blocks with generic TreeView type aliases across several editor files.
- Reformatted lambda expressions in InputDebuggerWindow.cs for improved readability.
Reviewed Changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
Packages/manifest.json | Updated package dependency versions to resolve compilation issues. |
Packages/com.unity.inputsystem/InputSystem/Plugins/HID/HIDDescriptorWindow.cs | Added conditional generic TreeView type aliases. |
Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionMapDrawer.cs | Added conditional generic TreeViewItem alias. |
Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionDrawerBase.cs | Added conditional generic TreeViewItem alias. |
Packages/com.unity.inputsystem/InputSystem/Editor/PropertyDrawers/InputActionDrawer.cs | Added conditional generic TreeViewItem alias. |
Packages/com.unity.inputsystem/InputSystem/Editor/Internal/TreeViewHelpers.cs | Added conditional generic TreeView and TreeViewItem aliases. |
Packages/com.unity.inputsystem/InputSystem/Editor/Internal/InputStateWindow.cs | Added conditional generic TreeViewState alias. |
Packages/com.unity.inputsystem/InputSystem/Editor/Internal/InputEventTreeView.cs | Added conditional generic TreeView, TreeViewItem, and TreeViewState aliases. |
Packages/com.unity.inputsystem/InputSystem/Editor/Internal/InputControlTreeView.cs | Added conditional generic TreeView, TreeViewItem, and TreeViewState aliases. |
Packages/com.unity.inputsystem/InputSystem/Editor/Debugger/InputDeviceDebuggerWindow.cs | Added conditional generic TreeViewState alias. |
Packages/com.unity.inputsystem/InputSystem/Editor/Debugger/InputDebuggerWindow.cs | Reformatted lambda expressions and added conditional generic using directives for TreeView types. |
Packages/com.unity.inputsystem/InputSystem/Editor/AssetEditor/InputActionTreeViewItems.cs | Added conditional generic TreeViewItem alias. |
Packages/com.unity.inputsystem/InputSystem/Editor/AssetEditor/InputActionTreeView.cs | Added conditional generic TreeView, TreeViewItem, and TreeViewState aliases. |
Packages/com.unity.inputsystem/InputSystem/Editor/AssetEditor/InputActionEditorWindow.cs | Added conditional generic TreeView and TreeViewState aliases. |
"com.unity.test-framework.performance": "3.1.0", | ||
"com.unity.test-framework.utp-reporter": "1.1.0-preview", | ||
"com.unity.textmeshpro": "3.0.6", | ||
"com.unity.ugui": "2.0.0", |
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.
Ok this is was not intentional, I'll revert
#if UNITY_6000_2_OR_NEWER | ||
using TreeView = UnityEditor.IMGUI.Controls.TreeView<int>; | ||
using TreeViewState = UnityEditor.IMGUI.Controls.TreeViewState<int>; | ||
#endif |
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.
This using section was the intentional scope of changes, but VS code formatted some code as well behind the scenes - adding some spaces here and there. Initially I thought to revert, but looking through them I liked the changes so why revert? Left them in the end.
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, were there some files missing in Stefans attempt? why were there errors trying this before?
I don't know what errors Paulius met, but what I saw was that you needed to add usings into all the files where we have the TreeView stuff and unless you do that, the compiler keeps complaining that some methods are missing somewhere. Apparently we have about a dozen classes that depend on TreeView and you have to update them in one go, can't change one file, get green run - because other 9 classes are missing stuff now etc. |
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 checked on 22.3 and latest trunk
Codecov ReportAttention: Patch coverage is @@ Coverage Diff @@
## develop #2196 +/- ##
===========================================
- Coverage 67.80% 67.80% -0.01%
===========================================
Files 367 367
Lines 53536 53538 +2
===========================================
Hits 36299 36299
- Misses 17237 17239 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Description
Unity 6.2 deprecated some older
TreeView
functionality in favour of generic versions, and it's advised to use eitherTreeView<int>
orTreeView<InstanceID>
where appropriate. In our case, using the<int>
variants seemed to be enough.Testing status & QA
Local testing, pending QA pass
Overall Product Risks
Checklist
Before review:
Changed
,Fixed
,Added
sections.Area_CanDoX
,Area_CanDoX_EvenIfYIsTheCase
,Area_WhenIDoX_AndYHappens_ThisIsTheResult
.During merge:
NEW: ___
.FIX: ___
.DOCS: ___
.CHANGE: ___
.RELEASE: 1.1.0-preview.3
.After merge: