-
Notifications
You must be signed in to change notification settings - Fork 316
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
[Draft] Modified OnScreenControl to support a customisable update mode #1792
base: develop
Are you sure you want to change the base?
Conversation
* FIX: UITK Listen button not working with Any control types * Remove jira links for ISX project issues from changelog
…een queue-based and state-change based updates.
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.
Draft looks good 👍
Unless there is a good reason for keeping the old behaviour I would recommend just making the new behaviour the only behaviour.
Otherwise we have more public API surface and more scenarios/code paths to maintain.
The old behaviour just seems completely wrong to me and I would class it as a bug.
Agree @jamesmcgill that I think it should replace previous behaviour. However I think it could regress existing implementations relying on this, but frankly it should be seen as a bug since previous implementation is not providing what you want. |
Description
Added a new "update mode" property on OnScreenControl.
Updated the OnScreenControl example in /samples.
Changes made
Feature addition to OnScreenControl:
OnScreenControlUpdateMode
which allows the user to specify whether theOnScreenControl
should output its input data by queuing input events to the system (OnScreenControlUpdateMode.Queue
, default) or whether theOnScreenControl
should output its input data by changing the underlying control state viaInputState.Change(...)
directly (OnScreenControlUpdateMode.StateChange
, new optional mode).Updated the OnScreenControl example:
Removed previous FIXME relating to this.
Notes
Note that PR still contains debug logging in
OnScreenStick
andOnScreenButton
.Note that updating via
InputState.Change(...)
avoids delaying generated until the next frame and effectively reduces latency ofOnScreenControl
by one frame or equivalently1 / FrameRate
seconds. However, this cannot be the default value since it would break backwards compability. However,OnScreenControlUpdateMode.StateChange
have been highlighted as the recommended setting.This PR needs significant testing and QA before considered to be merged since potential side-effects or detection of improper control layouts for this update pattern are currently not clear.
Checklist
Before review:
Changed
,Fixed
,Added
sections.([case %number%](https://issuetracker.unity3d.com/issues/...))
.Area_CanDoX
,Area_CanDoX_EvenIfYIsTheCase
,Area_WhenIDoX_AndYHappens_ThisIsTheResult
.During merge:
NEW: ___
.FIX: ___
.DOCS: ___
.CHANGE: ___
.RELEASE: 1.1.0-preview.3
.