InputSystem icon indicating copy to clipboard operation
InputSystem copied to clipboard

CHANGE: Remove redundant FastMouse current time call on InputState.Change (case ISX-1414)

Open AlexTyrer opened this issue 9 months ago • 4 comments

Description

Reduce FastMouse calls to get the current time (one of them is redundant).

Changes made

FastMouse.OnNextUpdate() makes two calls to InputState.Change() - one each for delta and scroll.

This results in two calls to InputRuntime.s_Instance.currentTime inside the InputSystem.Change() to pass as a parameter to the InputSystem UpdateState().

Instead query the current time and pass that as a parameter to the new templated Change<>() function which takes a time parameter. Events have their own internalTime which is used if present.

Checklist

Before review:

  • [x] Changelog entry added.
    • Explains the change in Changed, Fixed, Added sections.
    • For API change contains an example snippet and/or migration example.
    • FogBugz ticket attached, example ([case %number%](https://issuetracker.unity3d.com/issues/...)).
    • FogBugz is marked as "Resolved" with next release version correctly set.
  • [x] Tests added/changed, if applicable.
    • Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
    • Performance tests.
    • Integration tests.
  • [x] 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: CHANGE: FastMouse.OnNextUpdate() now passes the current time as a parameter to InputState.Change() improving performance (case ISX-1414)

  • [ ] Commit message for squash-merge is prefixed with one of the list:
    • NEW: ___.
    • FIX: ___.
    • DOCS: ___.
    • CHANGE: ___.
    • RELEASE: 1.1.0-preview.3.

AlexTyrer avatar May 15 '24 12:05 AlexTyrer

Looks like I may need to fix-up some documentation after adding the default parameter.

This change also fails the APIVerificationTests.cs test:

o APIVerificationTests.API_MinorVersionsHaveNoBreakingChanges

AlexTyrer avatar May 15 '24 14:05 AlexTyrer

Looks like I may need to fix-up some documentation after adding the default parameter.

This change also fails the APIVerificationTests.cs test:

o APIVerificationTests.API_MinorVersionsHaveNoBreakingChanges

Yeah.. maybe we need a overload method to not break the API, unfortunately.

jfreire-unity avatar May 15 '24 14:05 jfreire-unity

Looks like I may need to fix-up some documentation after adding the default parameter. This change also fails the APIVerificationTests.cs test: o APIVerificationTests.API_MinorVersionsHaveNoBreakingChanges

Yeah.. maybe we need a overload method to not break the API, unfortunately.

I've added a new Change<>() API that takes a non-optional time parameter - this leaves the existing API unchanged.

AlexTyrer avatar May 17 '24 11:05 AlexTyrer

/ci prv

AlexTyrer avatar May 17 '24 14:05 AlexTyrer

Closing as do not intend to land in trunk due to API change)

AlexTyrer avatar May 28 '24 09:05 AlexTyrer