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

Fix AxisControl types without clamps not updating properly #1128

Merged
merged 6 commits into from Jun 8, 2020

Conversation

StayTalm
Copy link
Contributor

@StayTalm StayTalm commented Apr 23, 2020

There was an issue where an AxisControl without a min/max clamp value would only receive the first 'performed' event, and no subsequent events until the action was cancelled. This was because AxisControl.EvaluateMagnitude early outs with -1 and so underlying action systems think that the action has not changed between frames. It feels appropriate that the magnitude of an AxisControl is just the absolute value if there are no other parameters, but I'm not sure if there was a specific reason for it.

This fixes internal bug #1183862

Copy link
Contributor

@jackpr-unity jackpr-unity left a comment

Choose a reason for hiding this comment

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

@StayTalm
Copy link
Contributor Author

StayTalm commented May 27, 2020

@Rene-Damm
Updated with test. Also with additional info:

private bool ShouldIgnoreControlStateChange(ref TriggerState trigger, int actionIndex)

This function relies on EvaluateMagnitude in order to identify a change in the control. When updating the control frame-by-frame, it leaves this function here:
Because the control is either cancelled or the result of EvaluateMagnitude is -1 (so the trigger and action state magnitudes are the same).

Not sure this is the right fix. This test uses a second gamepad so that conflict resolution turns on. This axis won't be normalized, so it will be able within a potentially different range.

(cherry picked from commit a661a1a)
…nto Magnitude-Unclamped-AxisControl

# Conflicts:
#	.yamato/upm-ci.yml
Revert CI Change
@Rene-Damm
Copy link
Contributor

I suspect the right fix for this is in the action system's disambiguation code, not in EvaluateMagnitude. But let's go and merge it. There's some other stuff related to disambiguation not working as it should that needs to be looked at. I'll try to have a look at this when I get around to that.

@Rene-Damm Rene-Damm merged commit 50ea663 into develop Jun 8, 2020
@Rene-Damm Rene-Damm deleted the Magnitude-Unclamped-AxisControl branch June 8, 2020 13:05
js-dignitas added a commit to js-dignitas/InputSystem that referenced this pull request Jul 5, 2021
* github/develop:
  CHANGE: Move Unity min version up to 2019.4 LTS (Unity-Technologies#1168).
  FIX: Stack overflow in SwithCurrentActionMap (case 1232893, Unity-Technologies#1164).
  FIX: Gravity, LinearAcceleration, and Attitude not initialized on iOS (case 1251382, Unity-Technologies#1160).
  FIX: Control picker empty when using 'Supported Devices' (case 1254150, Unity-Technologies#1163).
  NEW: Add concept of 'precompiled layouts' (Unity-Technologies#1156).
  FIX: Missing HelpURLs (Unity-Technologies#1157).
  FIX: Actions not triggering with axis controls that have no limits (Unity-Technologies#1128).
  FIX: Broken CI (Unity-Technologies#1159).
  FIX: CHANGELOG dates not conforming to ISO 8601 (Unity-Technologies#1141).
  FIX: No input coming through in UnityTests (Unity-Technologies#1139).
  FIX: Drag-reordering losing bindings (case 1228000, Unity-Technologies#1150).
  CHANGE: Added Windows Input tests to ensure Mouse movement works usin… (Unity-Technologies#1080)
  FIX: Broken CI from NPM server changes.
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.

None yet

3 participants