-
Notifications
You must be signed in to change notification settings - Fork 330
FIX: ISXB-1306 Fixed a bug within InputActionAsset.cs that prevented actions containing slashes to be found by name. #2160
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
…ntaining slashes to be found by name.
Pauliusd01
left a comment
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, tried finding actions with 1/multiple slashes, special symbols, non latin characters
Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs
Outdated
Show resolved
Hide resolved
Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs
Outdated
Show resolved
Hide resolved
| }, Is.Not.AllocatingGCMemory()); | ||
| } | ||
|
|
||
| // https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1306 |
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.
Alternatively, links could be put into a Description attribute:
[Description("See https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1306")]
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 is a fair change to this test since specific to reported use-case
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.
Addressed in b78188b
Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs
Outdated
Show resolved
Hide resolved
Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs
Outdated
Show resolved
Hide resolved
…Asset.cs Co-authored-by: Anthony Yakovlev <[email protected]>
K-Tone
left a comment
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, thanks for working on the suggestions -- and have a great weekend
Description
Suggested fix for https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1306. Essentially we had a bug preventing actions with names containing a "/" to be found by name since logic couldn't handle both map syntax and this case.
Testing status & QA
Added unit tests that cover the specified behavior (see diff).
Overall Product Risks
Comments to reviewers
Repro case on ticket should be sufficient as a check for manual testing (Also covered by automated existing and new tests) so I believe chance of regression is minimal. Good to look for potential mistakes with names vs path-syntax I might have overlooked.
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.After merge: