Unity-Technologies / InputSystem

An efficient and versatile input system for Unity.
Other
1.43k stars 310 forks source link

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

Closed AlexTyrer closed 4 months ago

AlexTyrer commented 5 months ago

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:

During merge: CHANGE: FastMouse.OnNextUpdate() now queries the current time and passes it as a parameter to InputState.Change() avoiding a redundant native call (case ISX-1414)

AlexTyrer commented 5 months ago

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

jfreire-unity commented 5 months ago

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.

AlexTyrer commented 5 months ago

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 commented 5 months ago

/ci prv

AlexTyrer commented 4 months ago

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