Skip to content

DOCS: Added PlayerInput.ActionEvent documentation #2068

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 12 commits into from
Dec 5, 2024
Merged

DOCS: Added PlayerInput.ActionEvent documentation #2068

merged 12 commits into from
Dec 5, 2024

Conversation

@lyndon-unity
Copy link
Collaborator

@lyndon-unity lyndon-unity commented Dec 2, 2024 •
edited
Loading

Description

Added PlayerInput.ActionEvent documentation
Updated PlayerInput documentation to improve, adding missing parameters, adding some examples, moving some see also links into the remarks as part of explanations.

Testing status & QA

Reviewed generated docs using the DocTools package and checked the validation results

Overall Product Risks

  • Complexity: low
  • Halo Effect: low

Comments to reviewers

Checklist

Before review:

  • Changelog entry added.
    • Explains the change in Changed, Fixed, Added sections.
    • For API change contains an example snippet and/or migration example.
    • JIRA ticket linked, example (case %%). If it is a private issue, just add the case ID without a link.
    • Jira port for the next release set as "Resolved".
  • Tests added/changed, if applicable.
    • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
    • Performance tests.
    • Integration 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.

After merge:

  • Create forward/backward port if needed. If you are blocked from creating a forward port now please add a task to ISX-1444.

@lyndon-unity lyndon-unity requested review from duckets and ekcoh December 2, 2024 17:41
ekcoh
ekcoh requested changes Dec 3, 2024
View reviewed changes
Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs Outdated
/// The event will be associated with the specified action. The action must be part of an action asset.
/// </remarks>
/// <param name="action">The action to associate with the event. The action must be part of an action asset.</param>
/// <exception cref="ArgumentNullException">The action is null.</exception>
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe null should have formatting here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this have ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ill add < c > around the null. I assume these whats meant.

I don't understand the '?' query

@ekcoh ekcoh added the DocsQualityWeek2024 Temporary label for docs week label Dec 3, 2024
@ekcoh
Copy link
Collaborator

ekcoh commented Dec 3, 2024

CI errors on docs formatting it seems

@lyndon-unity
Updated PlayerInput documentation to fix documentation errors
9baeaf0 
Extended summaries, added missing parameters, added some examples, removed non recommended documentation fields (moving some into remarks as recommended)
@lyndon-unity
Merge branch 'docs-quality-week-2024-playerinput-actionevent' of gith…
287709c 
…ub.com:Unity-Technologies/InputSystem into docs-quality-week-2024-playerinput-actionevent
@lyndon-unity
Added missing "using" statements
e618a51
@lyndon-unity
Copy link
Collaborator Author

CI errors on docs formatting it seems

Fixed formatting, addressed a whole load more documentation issues from the low score on the documentation assessment.

ekcoh
ekcoh reviewed Dec 3, 2024
View reviewed changes
Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs
/// <remarks>
/// <example>
/// <code>
/// PlayerInput player = PlayerInput.GetPlayerByIndex(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Are we ok with non-compilable examples? @duckets

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should compile but not in isolation. I'll await doc writer feedback

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInput.cs
///
/// <example>
/// <code>
/// PlayerInput.all[0].SwitchCurrentActionMap("Player");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar comment as previously on non-compilable example

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It should compile but not in isolation. I'll await doc writer feedback

@ekcoh
Copy link
Collaborator

ekcoh commented Dec 3, 2024 •
edited
Loading

Still has similar errors in CI as my branch (See dry-run target):

PVP-150-1: UnityEngine.InputSystem.PlayerInput: onActionTriggered: in block context (only allowed in top-level context)
PVP-150-1: UnityEngine.InputSystem.PlayerInput: onDeviceLost: in block context (only allowed in top-level context)
PVP-150-1: UnityEngine.InputSystem.PlayerInput: onDeviceRegained: in block context (only allowed in top-level context)
PVP-150-1: UnityEngine.InputSystem.PlayerInput: onControlsChanged: in block context (only allowed in top-level context)
PVP-150-1: UnityEngine.InputSystem.PlayerInput: TDevice GetDevice(): in block context (only allowed in top-level context)
PVP-150-1: UnityEngine.InputSystem.PlayerInput: void ActivateInput(): in block context (only allowed in top-level context)
PVP-150-1: UnityEngine.InputSystem.PlayerInput: void DeactivateInput(): in block context (only allowed in top-level context)
PVP-150-1: UnityEngine.InputSystem.PlayerInput: void SwitchCurrentActionMap(string): in block context (only allowed in top-level context)
PVP-150-1: UnityEngine.InputSystem.PlayerInput: PlayerInput GetPlayerByIndex(int): in block context (only allowed in top-level context)
PVP-150-1: UnityEngine.InputSystem.PlayerInput: PlayerInput Instantiate(GameObject, int, string, int, InputDevice): in block context (only allowed in top-level context)
(and 3 more errors)

@lyndon-unity
Copy link
Collaborator Author

Still has similar errors in CI as my branch (See dry-run target):

I've fixed these. Several were already there but I've tided it up.

@lyndon-unity lyndon-unity merged commit c9d686d into develop Dec 5, 2024
77 checks passed
@lyndon-unity lyndon-unity deleted the docs-quality-week-2024-playerinput-actionevent branch December 5, 2024 08:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ekcoh ekcoh ekcoh approved these changes

+1 more reviewer

@duckets duckets duckets approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

DocsQualityWeek2024 Temporary label for docs week

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@lyndon-unity @ekcoh @duckets