FIX: uum 138143 button press points for stickcontrol and vector2 add functionality#2411
Conversation
…ld used for stick controls and Vector2. Test
…at a user declared as an interaction.
|
The diff contains approximately 325,668 tokens (~977,005 characters), which exceeds the maximum limit of 150,000 tokens. Please consider splitting this PR into smaller, focused changes. 🤖 Helpful? 👍/👎 |
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #2411 +/- ##
===========================================
+ Coverage 78.13% 78.94% +0.81%
===========================================
Files 483 756 +273
Lines 98770 139174 +40404
===========================================
+ Hits 77169 109866 +32697
- Misses 21601 29308 +7707 Flags with carried forward coverage won't be shown. Click here to find out more.
|
Darren-Kelly-Unity
changed the title
…-stickcontrol-and-vector2-add-functionality # Conflicts: # Packages/com.unity.inputsystem/InputSystem/Runtime/Controls/InputControlExtensions.cs
ekcoh
left a comment
ekcoh
left a comment
There was a problem hiding this comment.
@Darren-Kelly-Unity Would it be possible to get rid of all the whitespace diffs on this PR? It would be much easier to review if this was corrected.
…-stickcontrol-and-vector2-add-functionality
…-stickcontrol-and-vector2-add-functionality
… it was implicitly implemented.
ekcoh
left a comment
ekcoh
left a comment
There was a problem hiding this comment.
Reviewed, I had quite a lot of questions and some remaining from last review so leaving this as comment-only for now.
…ctor2Control.cs Co-authored-by: Håkan Sidenvall <hakan.sidenvall@unity3d.com>
…ons.md Co-authored-by: Sue Arkin <85237015+suearkinunity@users.noreply.github.com>
…ons.md Co-authored-by: Sue Arkin <85237015+suearkinunity@users.noreply.github.com>
…ctor2Control.cs Co-authored-by: Morgan Hoarau <122548697+MorganHoarau@users.noreply.github.com>
ekcoh
left a comment
ekcoh
left a comment
There was a problem hiding this comment.
Reducing interface to a single method makes a lot of sense. Added some remaining suggestions/questions - once those are sorted I will approve this PR.
ekcoh
left a comment
ekcoh
left a comment
There was a problem hiding this comment.
Discussed some simplifications to be done before. Will approve now to unblock.
We no longer need to have press point for Vector2Control's and we instead just need to check for interactions instead.
… the same as develop was initially.
|
/review |
PR Reviewer Guide 🔍(Review updated until commit a73b5f5)Here are some key observations to aid the review process:
🤖 Helpful? Please react with 👍/👎 | Questions❓Please reach out in Slack #ask-u-pr |
I have checked this & ran it through an AI check too. This is not completely true what's said and it's documented for this case also. |
|
Persistent review updated to latest commit a73b5f5 |
6 participants
Purpose of this PR
Purpose of this PR
This change fixes inconsistent press detection between action-level helpers (InputAction.IsPressed, WasPressedThisFrame, WasReleasedThisFrame) and bindings that use a Press interaction with its own pressPoint. Previously, those APIs could use a ButtonControl’s pressPoint while the interaction used a different threshold, so “pressed” could disagree with the interaction’s press/release behavior.
It also aligns Vector2 / stick bindings with the intended model: action-level press polling uses InputSettings.defaultButtonPressPoint unless a Press interaction sets an explicit threshold (no separate per-vector pressPoint on the composite control for this path).
Tracking: UUM-138143 (adjust link if your team uses Jira instead of Issue Tracker).
Docs touched: RespondingToActions.md, Interactions.md; implementation in InputAction, InputActionState, InputControlExtensions, ButtonControl, PressInteraction, StickControl (XML remarks), plus focused tests in ActuationPressPointTests and updates in CoreTests_Actions / CoreTests_Controls.
Release Notes
Release Notes
Fixed: InputAction.IsPressed, WasPressedThisFrame, and WasReleasedThisFrame now respect a binding’s Press interaction pressPoint when set, so they stay consistent with the interaction’s press and release behavior instead of always following the bound ButtonControl’s pressPoint when both are set. UUM-138143
Changed: For bindings to Vector2 / stick controls, action-level press APIs no longer use a per-control pressPoint on the vector; use a Press interaction for a custom threshold or rely on defaultButtonPressPoint. UUM-138143
Functional Testing status
ActuationPressPointTestsand updates inCoreTests_Actions/CoreTests_Controlscover press-point actuation paths.IsPressedbefore processed magnitude reaches the configured press threshold for vector / stick cases.Performance Testing Status
I have added performance testing for the hot path methods that Hakan flagged and I don't see any major issues when comparing. There is a slight increase in cost of performance of about 0.1ms for 1000 iterations on the GetActuationPressThreshold() method that was asked to be checked.
No new per-frame allocations identified in scope.
Overall Product Risks
Technical Risk: 2 — Touches core action state, actuation thresholds, and several control paths; behavior change for anyone relying on the old mismatch or on vector-level pressPoint for these APIs.
Halo Effect: 2 — Affects any project using Press + custom pressPoint with IsPressed / frame-edge helpers, and Vector2/stick bindings that assumed the old threshold source.
Documentation Impact
Changes analyzed:
origin/develop...HEAD(4 commits).User-facing: New public
IActuationPressPoint;InputActionpress-helper remarks/behavior;ButtonControl,Vector2Control,StickControldocs aroundpressPoint;RespondingToActions.mdupdated for polling vspressPoint.Documentation updates in this PR
RespondingToActions.md— ClarifiesIsPressed/ frame helpers vs interactions andpressPoint; review wording and cross-refs after merge.Follow-up (optional)
IActuationPressPointcallout in a control reference page, add a short subsection and link from Responding to Actions.Jira (fetched via MCP): Bug, In Progress, TBD priority, assignee Darren Kelly, label Input-Actions. Ticket summary: InputAction.IsPressed() and IsInProgress() triggers on wrong values when rewriting the default values of Vector2 interactions.