Skip to content
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

DOCS: improve migration docs #1933

Merged
merged 32 commits into from
Aug 5, 2024
Merged

Conversation

duckets
Copy link
Collaborator

@duckets duckets commented May 22, 2024

Description

https://jira.unity3d.com/browse/DOCF-5410
Clarify and simplify page so that new system does not look more complex than old system.

Changes made

Added text & screenshot explaining differences between systems (eg named axes vs actions)
Improved formatting of tables and links.
Move code samples to linked pages.
Group by theme.
Move most common API to top.
Show examples of functions with parameters.
Remove namespaces where not necessary.

Notes:

Because of the large diff, it may be easier to review the page by viewing the entire formatted page here:
https://github.com/Unity-Technologies/InputSystem/blob/8908df2e3dade953d1b3808eb1a9474d6e2f3a97/Packages/com.unity.inputsystem/Documentation%7E/Migration.md

@duckets duckets requested a review from lyndon-unity May 22, 2024 15:03
Copy link
Collaborator

@lyndon-unity lyndon-unity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This update is a great improvement.
I've made some suggestions but even in this form its a step forward so approving anyway.

I've love to see the updates if you agree, but can somewhat pre approve.


There are various ways to discover the currently connected devices, as shown in the code samples below.

To query a list of all connected devices (does not allocate; read-only access):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't needed for the add/remove onDeviceChange part so could go down with the "To find all gamepads and joysticks" section

Packages/com.unity.inputsystem/Documentation~/Migration.md Outdated Show resolved Hide resolved
[`Input.GetMouseButton`](https://docs.unity3d.com/ScriptReference/Input.GetMouseButton.html)<br/>Example: `Input.GetMouseButton(0)`|Use [`isPressed`](../api/UnityEngine.InputSystem.Controls.ButtonControl.html#UnityEngine_InputSystem_Controls_ButtonControl_isPressed) on the corresponding mouse button.<br/>Example: `InputSystem.Mouse.current.leftButton.isPressed`
[`Input.GetMouseButtonDown`](https://docs.unity3d.com/ScriptReference/Input.GetMouseButtonDown.html)<br/>Example: `Input.GetMouseButtonDown(0)`|Use [`wasPressedThisFrame`](../api/UnityEngine.InputSystem.Controls.ButtonControl.html#UnityEngine_InputSystem_Controls_ButtonControl_wasPressedThisFrame) 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`](../api/UnityEngine.InputSystem.Controls.ButtonControl.html#UnityEngine_InputSystem_Controls_ButtonControl_wasReleasedThisFrame) 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()`](../api/UnityEngine.InputSystem.Mouse.html)<br/>__Note__: Mouse simulation from touch isn't implemented yet.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For a moment I thought the mouse simulation part might be out of date

But I've only sound the revised - "simulating touch from mouse" (rather than mouse from touch)
https://docs.unity3d.com/Packages/[email protected]/manual/Touch.html#touch-simulation

^ so this comment is just a pointless note ;)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest putting a code section with this bit
position = Mouse.current.position.ReadValue();

And keeping the Mouse documentation page link separate.
As I think its more clear what to cut and paste into code.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[`Input.GetMouseButtonDown`](https://docs.unity3d.com/ScriptReference/Input.GetMouseButtonDown.html)<br/>Example: `Input.GetMouseButtonDown(0)`|Use [`wasPressedThisFrame`](../api/UnityEngine.InputSystem.Controls.ButtonControl.html#UnityEngine_InputSystem_Controls_ButtonControl_wasPressedThisFrame) 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`](../api/UnityEngine.InputSystem.Controls.ButtonControl.html#UnityEngine_InputSystem_Controls_ButtonControl_wasReleasedThisFrame) 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()`](../api/UnityEngine.InputSystem.Mouse.html)<br/>__Note__: Mouse simulation from touch isn't implemented yet.
[`Input.mousePresent`](https://docs.unity3d.com/ScriptReference/Input-mousePresent.html)|Use [`Mouse.current != null`](../api/UnityEngine.InputSystem.Mouse.html#UnityEngine_InputSystem_Mouse_current).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest putting a code section with this bit
Mouse.current != null

And keeping the Mouse.Current link separate.
As I think its more clear what to cut and paste into code.

|Input Manager (Old)|Input System (New)|
|--|--|
[`Input.GetMouseButtonUp`](https://docs.unity3d.com/ScriptReference/Input.GetMouseButtonUp.html)|Use [`ButtonControl.wasReleasedThisFrame`](../api/UnityEngine.InputSystem.Controls.ButtonControl.html#UnityEngine_InputSystem_Controls_ButtonControl_wasReleasedThisFrame) on the corresponding mouse button:
[`Input.GetTouch`](https://docs.unity3d.com/ScriptReference/Input.GetTouch.html)|Use [`EnhancedTouch.Touch.activeTouches[i]`](../api/UnityEngine.InputSystem.EnhancedTouch.Touch.html#UnityEngine_InputSystem_EnhancedTouch_Touch_activeTouches)<br/>__Note__: Enable enhanced touch support first by calling [`EnhancedTouch.Enable()`](../api/UnityEngine.InputSystem.EnhancedTouch.EnhancedTouchSupport.html#UnityEngine_InputSystem_EnhancedTouch_EnhancedTouchSupport_Enable).
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given this is more common input for mobile I think its worth an example.

touch = Input.GetTouch(0);
touch.position

vs

EnhancedTouch.Touch.activeTouches[0].position

TBH the new input system touch page itself would benefit from giving a more explicit example (as the table covers various options but doesn't make it super clear what to type to just get the position or state)
https://docs.unity3d.com/Packages/[email protected]/manual/Touch.html

Packages/com.unity.inputsystem/Documentation~/Migration.md Outdated Show resolved Hide resolved
[`Input.compass`](https://docs.unity3d.com/ScriptReference/Input-compass.html)|No corresponding API yet.
[`Input.compensateSensors`](https://docs.unity3d.com/ScriptReference/Input-compensateSensors.html)|[`InputSettings.compensateForScreenOrientation`](../api/UnityEngine.InputSystem.InputSettings.html#UnityEngine_InputSystem_InputSettings_compensateForScreenOrientation).
[`Input.deviceOrientation`](https://docs.unity3d.com/ScriptReference/Input-deviceOrientation.html)|No corresponding API yet.
[`Input.gyro`](https://docs.unity3d.com/ScriptReference/Input-gyro.html)|The `UnityEngine.Gyroscope` class is replaced by multiple separate sensor Devices in the new Input System:<br/>[`Gyroscope`](../api/UnityEngine.InputSystem.Gyroscope.html) to measure angular velocity.<br/>[`GravitySensor`](../api/UnityEngine.InputSystem.GravitySensor.html) to measure the direction of gravity.<br/>[`AttitudeSensor`](../api/UnityEngine.InputSystem.AttitudeSensor.html) to measure the orientation of the device.<br/>[`Accelerometer`](../api/UnityEngine.InputSystem.Accelerometer.html) to measure the total acceleration applied to the device.<br/>[`LinearAccelerationSensor`](../api/UnityEngine.InputSystem.LinearAccelerationSensor.html) to measure acceleration applied to the device, compensating for gravity.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see you have specific examples below the main section. I was wondering if you needed this line. Perhaps splitting the header links into the next few sections.

Happy to leave this bit though as its minor.

Copy link
Collaborator

@ekcoh ekcoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great @duckets, a big improvement to the migration docs. Added some minor comments.

Packages/com.unity.inputsystem/Documentation~/Migration.md Outdated Show resolved Hide resolved
[`Input.GetMouseButton`](https://docs.unity3d.com/ScriptReference/Input.GetMouseButton.html)<br/>Example: `Input.GetMouseButton(0)`|Use [`isPressed`](../api/UnityEngine.InputSystem.Controls.ButtonControl.html#UnityEngine_InputSystem_Controls_ButtonControl_isPressed) on the corresponding mouse button.<br/>Example: `InputSystem.Mouse.current.leftButton.isPressed`
[`Input.GetMouseButtonDown`](https://docs.unity3d.com/ScriptReference/Input.GetMouseButtonDown.html)<br/>Example: `Input.GetMouseButtonDown(0)`|Use [`wasPressedThisFrame`](../api/UnityEngine.InputSystem.Controls.ButtonControl.html#UnityEngine_InputSystem_Controls_ButtonControl_wasPressedThisFrame) 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`](../api/UnityEngine.InputSystem.Controls.ButtonControl.html#UnityEngine_InputSystem_Controls_ButtonControl_wasReleasedThisFrame) 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()`](../api/UnityEngine.InputSystem.Mouse.html)<br/>__Note__: Mouse simulation from touch isn't implemented yet.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Packages/com.unity.inputsystem/Documentation~/Migration.md Outdated Show resolved Hide resolved
@duckets duckets changed the title Docf 5410 improve migration docs DOCS: improve migration docs Jun 5, 2024
@lyndon-unity
Copy link
Collaborator

We seem to lack migration info for GetPenEvent, GetLastPenContactEvent, ResetPenEvents, ClearLastPenContactEvent

All could refer to Pen.current

@lyndon-unity
Copy link
Collaborator

I also think imeCompositionMode should refer to Keyboard.current.SetIMEEnabled(true);

Currently Input.imeIsSelected refers to both Keyboard.current.imeSelected and Keyboard.current.SetIMEEnabled(true);
I think this should only refer to Keyboard.current.imeSelected

@lyndon-unity
Copy link
Collaborator

Some issues remain with KeyControl.displayName being referenced - This field is in InputControl (which KeyControl inherits from)

isPressed in Packages/com.unity.inputsystem/InputSystem/Controls/ButtonControl.cs Line 147wasPressedThisFrame in Packages/com.unity.inputsystem/InputSystem/Controls/ButtonControl.cs Line 185
wasReleasedThisFrame in Packages/com.unity.inputsystem/InputSystem/Controls/ButtonControl.cs Line 21

Copy link
Collaborator

@lyndon-unity lyndon-unity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updates look good

@lyndon-unity lyndon-unity merged commit e72d158 into develop Aug 5, 2024
76 of 79 checks passed
@lyndon-unity lyndon-unity deleted the DOCF-5410-improve-migration-docs branch August 5, 2024 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants