FIX: uum 134737 corrupt input action asset editor window on restart#2371
Conversation
…a save dialog prompt.
…d code. Co-authored-by: Morgan Hoarau <122548697+MorganHoarau@users.noreply.github.com>
There was a problem hiding this comment.
I have reviewed the changes and found several issues that should be addressed. Most notably, there is a potential state leak when the editor quit operation is aborted by other listeners, which could lead to data loss in subsequent window close attempts. Additionally, there are a few instances of commented-out code in the test files that should be cleaned up.
Summary of Findings
- High Importance: 1 bug regarding editor quit state persistence.
- Low Importance: 2 issues regarding dead/commented-out code in tests.
🤖 Helpful? 👍/👎
Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs
Show resolved
Hide resolved
Assets/Tests/InputSystem/DocumentationBasedAPIVerficationTests.cs
Outdated
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #2371 +/- ##
===========================================
- Coverage 77.90% 77.89% -0.01%
===========================================
Files 476 481 +5
Lines 97613 97681 +68
===========================================
+ Hits 76048 76093 +45
- Misses 21565 21588 +23 Flags with carried forward coverage won't be shown. Click here to find out more.
|
…set-on-restart # Conflicts: # Assets/Tests/InputSystem/APIVerificationTests.cs # Assets/Tests/InputSystem/DocumentationBasedAPIVerficationTests.cs
|
/review |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr |
ekcoh
left a comment
There was a problem hiding this comment.
Fix looks good to me but I think u-pr has a valid comment about side-effects that IMO could be addressed on this PR before landing to get rid of that problem.
Assets/Tests/InputSystem/DocumentationBasedAPIVerficationTests.cs
Outdated
Show resolved
Hide resolved
|
/test_plan |
Test Plan
Summary of Changes & Risk AssessmentSummary of ChangesThis PR fixes a state corruption issue in the Input Action Asset editor window that occurs after a Unity restart (or domain reload) when an asset has unsaved changes. The fix corrects the initialization order in Risk Assessment
Test ScenariosFunctional Testing
Regression Testing
Edge Cases
💡 This test plan updates automatically when 🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr |
|
Rechecked the latest commit by having unsaved changes and quitting in different ways, the window worked correctly on restart (To expand on what I did was close the editor with don't save, save, and force close and then checked whether input actions window is acting correctly on restart. If you meant something else let me know and I'll check again) |
Description
Fix an issue where the input action asset editor window was corrupted / in a broken state after restarting Unity when there was a pending change on the input action asset.
Ticket:
https://jira.unity3d.com/browse/UUM-134737
Testing status & QA
Tested manually for now, the sequence of events is too hard to test due to needing to restart Unity to cause the issue.
Overall Product Risks
Not much risk here.
Comments to reviewers
There is a repro video in the ticket linked above.
How to reproduce:
4 Restart the project
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.