-
Notifications
You must be signed in to change notification settings - Fork 340
NEW: Pen.isSupported, Mouse.isSupported and Touchscreen.isPressureSupported [ISX-2046, ISX-2079] #2476
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
NEW: Pen.isSupported, Mouse.isSupported and Touchscreen.isPressureSupported [ISX-2046, ISX-2079] #2476
Changes from 2 commits
95d60ca
0a68b76
e46584a
6026581
40f5b7d
3068ef5
0951ea0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -67,6 +67,30 @@ Directly reading hardware controls bypasses the new Input System's action-based | |
| [`Input.imeIsSelected`](https://docs.unity3d.com/ScriptReference/Input-imeIsSelected.html)|Use: [`Keyboard.current.imeSelected`](xref:UnityEngine.InputSystem.Keyboard) | ||
| [`Input.inputString`](https://docs.unity3d.com/ScriptReference/Input-inputString.html)|Subscribe to the [`Keyboard.onTextInput`](xref:UnityEngine.InputSystem.Keyboard) event:<br/>`Keyboard.current.onTextInput += character => /* ... */;` | ||
|
|
||
| ## Device capability and device availability | ||
|
|
||
| Several Input Manager properties, such as [`Input.mousePresent`](https://docs.unity3d.com/ScriptReference/Input-mousePresent.html) and | ||
| [`Input.stylusTouchSupported`](https://docs.unity3d.com/ScriptReference/Input-stylusTouchSupported.html), answered | ||
| two questions at once, and answered them differently depending on the platform. On some platforms they were a | ||
| hardcoded constant meaning roughly "this platform has this kind of device", and on others they performed real | ||
| hardware detection. | ||
|
|
||
| The new Input System separates the two: | ||
| - **Does this platform even support this input interface?** | ||
| Use the capability properties: [`Mouse.isSupported`](xref:UnityEngine.InputSystem.Mouse), | ||
| [`Pen.isSupported`](xref:UnityEngine.InputSystem.Pen) and [`Touchscreen.isPressureSupported`](xref:UnityEngine.InputSystem.Touchscreen). | ||
| These never change while the application runs, so read them once and decide whether to offer device-specific | ||
| functionality. | ||
| - **Is a device available to read from right now?** Use `Device.current != null && Device.current.enabled`. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I can see this causing a lot of trial and error for users, especially the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, and why I mentioned this in the PR description, there called
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might also be - as you say - root class is sufficient. Leaving this open until you have a chance to respond @MorganHoarau
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Make sense, not a fan of "usable" as a term as I don't think I've ever seen any Unity API use it. I'm not sure to understand why an extension method scales better than adding a new property on Device though. In both case we are exposing a new API. But this could be move as a separate ticket to improve the clarity around the topic. However, I fear this would never be tackled on the side, so I would still add it here. It is a small addition and I don't see the concept going away anytime soon.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not a fan of "usable" either, and not a fan of "available" either since I would assume its available as soon as its connected, but this also gates on device being enabled. I take the extension method thing back since I kind of ignored the fact that current design uses OO so having it on the base makes sense in this case instead of doing constrained generics. I intererpret your reply as YES - add it? (Ignoring the open naming headache)
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I built it, as Past a Where it genuinely differs is a cached device reference. So instead of the property, I documented the trap in the migration doc in 40f5b7d, with an example contrasting a cached reference against reading Two shapes I ruled out on the way, in case they come up:
You were right that the extension method does not scale better than a property when the design is already OO, so that part of my earlier answer was wrong regardless of where this lands. OK to continue this discussion offline or in repurposed PR? |
||
| Both parts matter. A non-null `current` only means a device object is registered, which some platforms do | ||
| unconditionally regardless of whether hardware is attached, and `enabled` is what tells you the device is | ||
| active. The Device Simulator is a good example of the difference: while simulating a touch device it disables | ||
| the native mouse and pen without removing them, so `current` stays non-null while `enabled` becomes false. | ||
|
|
||
| Because the old properties mixed these two meanings, the capability properties are **not drop-in replacements**. | ||
| On platforms where the old property performed real detection, the new property answers the capability question | ||
| instead, so it can be `true` where the old one was `false`. The tables below note this per API. | ||
|
|
||
| ## Mouse | ||
|
|
||
| `MonoBehaviour.OnMouse` events, such as [MonoBehaviour.OnMouseDown](https://docs.unity3d.com/ScriptReference/MonoBehaviour.OnMouseDown.html), are supported in Unity 6.4 and later. | ||
|
|
@@ -77,19 +101,19 @@ Directly reading hardware controls bypasses the new Input System's action-based | |
| [`Input.GetMouseButtonDown`](https://docs.unity3d.com/ScriptReference/Input.GetMouseButtonDown.html)<br/>Example: `Input.GetMouseButtonDown(0)`|Use [`wasPressedThisFrame`](xref:UnityEngine.InputSystem.Controls.ButtonControl) on the corresponding mouse button.<br/>Example: `InputSystem.Mouse.current.leftButton.wasPressedThisFrame` | ||
| [`Input.GetMouseButtonUp`](https://docs.unity3d.com/ScriptReference/Input.GetMouseButtonUp.html)<br/>Example: `Input.GetMouseButtonUp(0)`|Use [`wasReleasedThisFrame`](xref:UnityEngine.InputSystem.Controls.ButtonControl) on the corresponding mouse button.<br/>Example: `InputSystem.Mouse.current.leftButton.wasReleasedThisFrame` | ||
| [`Input.mousePosition`](https://docs.unity3d.com/ScriptReference/Input-mousePosition.html)|Use [`Mouse.current.position.ReadValue()`](xref:UnityEngine.InputSystem.Mouse)<br/>Example: `Vector2 position = Mouse.current.position.ReadValue();`<br/> **Note:** Mouse simulation from touch isn't implemented yet. | ||
| [`Input.mousePresent`](https://docs.unity3d.com/ScriptReference/Input-mousePresent.html)|No corresponding API yet. | ||
| [`Input.mousePresent`](https://docs.unity3d.com/ScriptReference/Input-mousePresent.html)|Use [`Mouse.isSupported`](xref:UnityEngine.InputSystem.Mouse) to check whether the platform supports mouse input at all.<br/>Example: `if (Mouse.isSupported) ShowMouseSettings();`<br/>**Note:** Not a drop-in replacement; see [Device capability and device availability](#device-capability-and-device-availability) above. Input System does not currently deliver mouse input on iOS, iPadOS or visionOS, so `Mouse.isSupported` is `false` there even though the platform itself supports indirect mice. Requires a recent Editor version. | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This doesn't sound good to me. Make sure to add @suearkinunity so she can review
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair, it read as a warning label rather than guidance. Reworded in 40f5b7d to say what actually differs. Adding @suearkinunity as part of repurposed PR to decide on how to hand doc - improve it or do what @LeoUnity suggests and handle it outside. |
||
|
|
||
| ## Touch and Pen | ||
|
|
||
| |Input Manager (Old)|Input System (New)| | ||
| |--|--| | ||
| [`Input.GetTouch`](https://docs.unity3d.com/ScriptReference/Input.GetTouch.html)<br/>For example:<br/>`Touch touch = Input.GetTouch(0);`<br/>`Vector2 touchPos = touch.position;`|Use [`EnhancedTouch.Touch.activeTouches[i]`](xref:UnityEngine.InputSystem.EnhancedTouch.Touch)<br/>Example: `Vector2 touchPos = EnhancedTouch.Touch.activeTouches[0].position;`<br/> **Note:** Enable enhanced touch support first by calling [`EnhancedTouch.Enable()`](xref:UnityEngine.InputSystem.EnhancedTouch.EnhancedTouchSupport). | ||
| [`Input.multiTouchEnabled`](https://docs.unity3d.com/ScriptReference/Input-multiTouchEnabled.html)|No corresponding API yet. | ||
| [`Input.multiTouchEnabled`](https://docs.unity3d.com/ScriptReference/Input-multiTouchEnabled.html)|There is no direct equivalent, because this is a setting rather than a hardware capability. To get the same first-touch-wins behaviour, read [`primaryTouch`](xref:UnityEngine.InputSystem.Touchscreen) instead of iterating all touches, or bind to `<Touchscreen>/primaryTouch`.<br/>Example: `if (Touchscreen.current != null && Touchscreen.current.primaryTouch.press.isPressed)`<br/>**Note:** Two differences from setting `Input.multiTouchEnabled = false`. First, `primaryTouch` filters only itself: [`touches`](xref:UnityEngine.InputSystem.Touchscreen), the `<Touchscreen>/touch*` bindings and [`EnhancedTouch`](xref:UnityEngine.InputSystem.EnhancedTouch.Touch) still report every finger, whereas the legacy setting suppressed additional touches globally. Second, when the finger that started the primary touch lifts while other fingers are still down, the primary touch is retained rather than ended until the last finger is released, so a control bound to it stays actuated in the meantime. | ||
| [`Input.simulateMouseWithTouches`](https://docs.unity3d.com/ScriptReference/Input-multiTouchEnabled.html)|No corresponding API yet. | ||
| [`Input.stylusTouchSupported`](https://docs.unity3d.com/ScriptReference/Input-stylusTouchSupported.html)|No corresponding API yet. | ||
| [`Input.stylusTouchSupported`](https://docs.unity3d.com/ScriptReference/Input-stylusTouchSupported.html)|Use [`Pen.isSupported`](xref:UnityEngine.InputSystem.Pen) to check whether the platform supports pen input at all.<br/>Example: `if (Pen.isSupported) ShowPenSettings();`<br/>**Note:** Not a drop-in replacement; see [Device capability and device availability](#device-capability-and-device-availability) above. Requires a recent Editor version. | ||
| [`Input.touchCount`](https://docs.unity3d.com/ScriptReference/Input-touchCount.html)|[`EnhancedTouch.Touch.activeTouches.Count`](xref:UnityEngine.InputSystem.EnhancedTouch.Touch)<br/> **Note:** Enable enhanced touch support first by calling [`EnhancedTouchSupport.Enable()`](xref:UnityEngine.InputSystem.EnhancedTouch.EnhancedTouchSupport) | ||
| [`Input.touches`](https://docs.unity3d.com/scriptreference/input-touches.html)|[`EnhancedTouch.Touch.activeTouches`](xref:UnityEngine.InputSystem.EnhancedTouch.Touch)<br/> **Note:** Enable enhanced touch support first by calling [`EnhancedTouch.Enable()`](xref:UnityEngine.InputSystem.EnhancedTouch.EnhancedTouchSupport) | ||
| [`Input.touchPressureSupported`](https://docs.unity3d.com/ScriptReference/Input-touchPressureSupported.html)|No corresponding API yet. | ||
| [`Input.touchPressureSupported`](https://docs.unity3d.com/ScriptReference/Input-touchPressureSupported.html)|Use [`Touchscreen.isPressureSupported`](xref:UnityEngine.InputSystem.Touchscreen) to check whether the platform delivers a real pressure value with touch input.<br/>Example: `if (Touchscreen.isPressureSupported) UsePressureForBrushWidth();`<br/>**Note:** When this is `false`, [`pressure`](xref:UnityEngine.InputSystem.Controls.TouchControl) reports a constant `1` while a finger is down rather than a measured value. This is a platform-wide answer rather than a per-device one. Requires a recent Editor version. | ||
| [`Input.touchSupported`](https://docs.unity3d.com/ScriptReference/Input-touchSupported.html)|[`Touchscreen.current != null`](xref:UnityEngine.InputSystem.Touchscreen) | ||
| [`Input.backButtonLeavesApp`](https://docs.unity3d.com/ScriptReference/Input-backButtonLeavesApp.html)|No corresponding API yet. | ||
| [`GetPenEvent`](https://docs.unity3d.com/ScriptReference/Input.GetPenEvent.html)<br/>[`GetLastPenContactEvent`](https://docs.unity3d.com/ScriptReference/Input.GetLastPenContactEvent.html)<br/>[`ResetPenEvents`](https://docs.unity3d.com/ScriptReference/Input.ResetPenEvents.html)<br/>[`ClearLastPenContactEvent`](https://docs.unity3d.com/ScriptReference/Input.ClearLastPenContactEvent.html)|Use: [`Pen.current`](xref:UnityEngine.InputSystem.Pen)<br/>See the [Pen, tablet and stylus support](devices-pen.md) docs for more information. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| #if UNITY_INPUTSYSTEM_SUPPORTS_CAPABILITY_QUERIES | ||
| namespace UnityEngine.InputSystem.LowLevel | ||
| { | ||
| /// <summary> | ||
| /// Answer to a platform capability query, meaning what the platform can deliver rather than | ||
| /// what is currently connected. | ||
| /// </summary> | ||
| /// <remarks> | ||
| /// Mirrors <c>CapabilityState</c> in the engine's <c>Modules/Input/InputDeviceIOCTL.h</c>, whose | ||
| /// wire values are pinned by tests on both sides. <see cref="Unknown"/> is zero so that an | ||
| /// unwritten payload, or a platform that has not implemented a query, reads as "we do not know" | ||
| /// rather than as a confident <see cref="NotSupported"/>. | ||
| /// | ||
| /// The value space is open. Treat anything other than <see cref="Supported"/> as not supported | ||
| /// rather than rejecting it, because a newer engine may answer with a value this version of the | ||
| /// package does not know about. | ||
| /// </remarks> | ||
| internal enum InputCapabilitySupport : byte | ||
| { | ||
| /// <summary> | ||
| /// The platform has no answer, typically because it has not implemented the query yet. | ||
| /// </summary> | ||
| Unknown = 0, | ||
|
|
||
| /// <summary> | ||
| /// The platform definitively cannot deliver it. | ||
| /// </summary> | ||
| NotSupported = 1, | ||
|
|
||
| /// <summary> | ||
| /// The platform definitively can deliver it. | ||
| /// </summary> | ||
| Supported = 2 | ||
| } | ||
| } | ||
| #endif // UNITY_INPUTSYSTEM_SUPPORTS_CAPABILITY_QUERIES |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
"What a given platform actually answers is asserted natively in the engine repository, from PlatformDependent"
The way I read it it makes it sound like we have platfromdependent native test, but I don't think thats the case looking at the native PR.
Uh oh!
There was an error while loading. Please reload this page.
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.
You read it right, and the comment was wrong. There are no per-platform tests in the engine's platform code at all. The native branch touches 13 platform files and not one of them is a test. The native tests live with the input module, and what they assert is that whichever platform they happen to run on returns a valid state, plus the endpoint contract itself: payload size validation, device codes rejected, and the enum values the managed mirror assumes.
So no test anywhere asserts the answer a named platform gives. A wrong per-platform answer is caught by cross-platform compilation and review, not by CI. Reworded the comment to say exactly that in e46584a.
Additional related cleanup in 3068ef5.
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.
Better, but I still think this comment is very verbose, do we need to say that:
"The engine's own tests assert that whichever platform they run on returns a
// valid state and that the endpoint rejects malformed payloads. No test asserts the answer a
// named platform gives, so a wrong per-platform answer is caught by review, not by CI."
Its a bit weird to comment what we are not doing and the reasoning is not that strong to justify it...
Uh oh!
There was an error while loading. Please reload this page.
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.
Completely dropped this slop in 0951ea0 - value was zero