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

[AlexTyrer]

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:

• 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.
• 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:
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)

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

[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

[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.

[ekcoh]

I noticed this is changing public API but with additive default values so I guess package verification tool categories this as a minor API break and not a major. It might require next release to be a minor revision though but not sure. @unity-cliff or @stefanunity might have insight into this?

[ekcoh]

Should we still proceed without return if this scenario occurs?

[AlexTyrer]

Good question - I thought it better to continue rather than just return here.

I haven't actually seen this case happen yet with my limited devices for testing.

What might be better is to simply set the time here in this case:

            if (!eventPtr.valid && time == default)
            {
                Debug.Log($"InputState.Change<>('{control.name}') called with null event and default time\n");
                time = InputRuntime.s_Instance.currentTime;
            }

I think this would preserve the existing behaviour where it would query the time if no event was provided.

[AlexTyrer]

I've refactored this so we no longer need this check.

There are two paths into this function from the two Change<>() templated functions.

The original Change<>() function always passes in an event (with it's own internal time) - if the event is not valid it passes the current time.

The other newly added Change<>() function always passes in a time value (it's a required parameter) along with a dummy event - this is the one used by the FastMouse.OnNextUpdate() function.

[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.

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

[AlexTyrer]

/ci prv

[lyndon-unity]

Is this right - should it actually be set to default if the eventPtr is NOT valid ?

[AlexTyrer]

No, it's correct.

If the eventPtr IS valid then that event's internalTime will be used in the unsafe Change<>() function that's called (the time parameter can be default avoiding the call to get the current time).

If eventPtr is NOT valid then we need to look up the time here and pass that instead for the unsafe Change<>() function to use.

[jfreire-unity]

Looks ok to me.
Just asked some comment improvements in the "new" added Change method so that their need/use case is clearer. We will now have 4 overloaded Change methods so it helps to have some differentiation between them

[AlexTyrer]

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