-
Notifications
You must be signed in to change notification settings - Fork 327
FIX: Redo much of InputSystemUIInputModule. #1082
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
keiramoon
suggested changes
Mar 5, 2020
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.
Added some style suggestions.
…yEngine.PointerType.
karljj1
reviewed
Mar 5, 2020
Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs
Outdated
Show resolved
Hide resolved
Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs
Outdated
Show resolved
Hide resolved
Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs
Show resolved
Hide resolved
Packages/com.unity.inputsystem/InputSystem/Plugins/UI/TrackedDeviceRaycaster.cs
Show resolved
Hide resolved
pcosgrave
reviewed
Mar 5, 2020
Packages/com.unity.inputsystem/InputSystem/Plugins/UI/TrackedDeviceRaycaster.cs
Show resolved
Hide resolved
keiramoon
approved these changes
Mar 5, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is a larger scale rewrite of
InputSystemUIInputModule
.Why?
I found so many issues that after I initially just fixed individual issues as I went, I ultimately decided to take a knife to the current code and cut away a great deal of what I perceived as unnecessary complexity and systematically build out tests (which were greatly lacking) and functionality.
Many of the issues were deviations from
StandaloneInputModule
behavior which were regressions but some of the issues also arose from peculiarities of howInputSystemUIInputModule
sources input and thus needs different handling thanStandaloneInputModule
. For example,StandaloneInputModule
simply usedInput.mousePosition
which by nature is an aggregate whereasInputSystemUIInputModule
sources position values from individual devices and may thus receive multiple conflicting pointer inputs whereas the old input had one already unified input.These changes come in very late for 1.0 but IMO the current
InputSytemUIInputModule
is simply too broken to be 1.0 and UI is too critical to not fix this.Changes
StandaloneInputModule
:)MouseModel
(internal) toPointerModel
and simplified the code.TrackedDeviceModel
(internal) intoPointerModel
as tracked devices are ultimately just a special type of pointers (like touch).trackedDeviceSelect
. Tracked input is pointer input so this is simply equivalent toleftClick
(same as for touch). As part of this change, it is now also possible to right and middle click from tracked devices by simply bindingrightClick
and/ormiddleClick
for them.JoystickModel
(internal) toNavigationModel
and simplified it.ExtendedPointerEventData
which surfaces input system-specific information in UI events. Also replaces the internal TrackedPointerEventData class. Tracked device input and pointer input now sit on the same single codepath.TrackedDevice
input as amaxDistance
setting onTrackedDeviceRaycaster
.TrackedDeviceRaycaster
to the component menu (likeGraphicRaycaster
) and added instructions for how to set it up to the docs.InputSystemUIInputModule
.InputSystemUIInputModule
growing infinitely withTouchscreen
if touch IDs were not reused.lastPress
value on PointerEventData getting lost.TrackedDeviceRaycaster
not settingscreenPosition
inRaycastResult
.InputSystem.onActionChange
getting invoked too many times when actions got enabled/disabled/changed.repeatDelay
andrepeatRate
inInputSystemUIInputModule
tomoveRepeatDelay
andmoveRepeatRate
.InputSystemUIInputModule.pointerBehavior
to give control over how the input module resolves concurrent input from multiple pointers. Default behavior is to unify all pointer input that isn't from touch or tracked devices. This preserves multi-point ability for touch and tracked devices while collapsing input from other pointers such as mice and pens into a single unified pointer.Pretty sure I missed a couple issues I found and fixed.
Known Issues