InputSystem
InputSystem copied to clipboard
Isx 1851 remove title from editor window rename - extended
Description
Note that this branch is a sub-branch of https://github.com/Unity-Technologies/InputSystem/pull/1822 and hence it contains all changes from that PR but also contains additional fixes called out under Changes made below.
This PR is a tentative fix to the regressive behaviour reported in https://github.com/Unity-Technologies/InputSystem/pull/1822 and hence if verified to eliminate the problems mentioned in that PR and fulfilling the functionality intended on https://github.com/Unity-Technologies/InputSystem/pull/1822 this PR might be merged instead and the previous PR closed. If not successful this either needs more work or needs to be discarded while the original PR remains intact with the UI changes.
Changes made
Everything mentioned in https://github.com/Unity-Technologies/InputSystem/pull/1822
But also:
- Modified RenameNewActionMaps to no longer be triggered from RedrawUI. Instead Rename is triggered as a shortcut command or via AddActionMap (indirectly through state not in sync with view/state). However this now happens as a continuation of AddActionMap if successful. This is likely the fix corrected the regression.
- Modified RenameNewActionMap to scroll to target.
- Extended StateContainer Dispatch to allow taking an optional action to be executed on the UI scheduler queue directly after the command if and only if the command is successful. The intention is increased exception safety.
- Extended ViewBase with a isRedrawInProgress that may be used to discard any View to Model changes that would not lead to any change early. This basically just eliminates need to unhook/modify/hook patterns in RedrawUI since RedrawUI should only reflect the model/state to the view.
- Made some minor improvements to CollectionViewSelectionChangeFilter so it uses less memory when disabled. Also fixed a compiler error when disabled (dev mode only).
- Made a bunch of methods protected that were public for no reason (only internal code)
- Made a bunch of fields readonly when intention is to not allow modification of them.
Notes
Additional issues found during development. Previously Action Map panel never scrolled to newly added items when list is larger than panel. This has been corrected logically but for some reason the list is always one item off. Unclear why.
Found an issue where adding a number of Actions and directly after exiting renaming mode with enter the up key is pressed causes an exception in UITK code. Is this a new bug?
Checklist
Before review:
- [x] 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.
- Explains the change in
- [ ] Tests added/changed, if applicable.
- Functional tests
Area_CanDoX
,Area_CanDoX_EvenIfYIsTheCase
,Area_WhenIDoX_AndYHappens_ThisIsTheResult
. - Performance tests.
- Integration tests.
- Functional 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
.
-
I am happy to see lots of good changes. I left some comments, while my major concern is that we might miss selections if we interrupt the Selection Callback when a redraw is in process. Maybe I am overseeing something.
Not sure I follow here @ritamerkl could you elaborate a bit since it sounds like you have spotted something about this PR I might have overlooked?
I am still reproducing #1822 (comment) is that expected?
I hoped this would eliminate that which is the whole point of this "sub-branch". Seems like I need to revisit on Windows again then.
Not sure I follow here @ritamerkl could you elaborate a bit since it sounds like you have spotted something about this PR I might have overlooked?
I tried to bring a bit more clarity in my comments at the according threads
I am still reproducing #1822 (comment) is that expected?
I hoped this would eliminate that which is the whole point of this "sub-branch". Seems like I need to revisit on Windows again then.
I was not able to reproduce this error in 2022.3.19 and the same editor version shown in the video. The first error seems to start the exception row, it would be interesting to see the whole log of this one. If you can reproduce this bug again @Pauliusd01, could you look if you can get the whole stack trace?
I believe this PR is obsolete now due to other changes. Will close this PR but leave branch existing for a while longer if there is need to pick any cherries from it.