NEW: Input consumption - Priorities instead of complexity [ISX-2510]#2402
NEW: Input consumption - Priorities instead of complexity [ISX-2510]#2402Darren-Kelly-Unity wants to merge 92 commits into
Conversation
…ort and clamping logic. Add tooltip for priority field and update related classes to handle priority in bindings.
This reverts commit 28640eb
…r the shortcut again.
![u-pr[bot]](https://avatars.githubusercontent.com/in/1201017?s=60&v=4){kind=small}
There was a problem hiding this comment.
The review identified several issues ranging from potential runtime exceptions and logic regressions to documentation mismatches. Most notably, there's a priority clamping issue that causes value wrap-around and a potential logic regression in how composite bindings are handled compared to previous versions.
🤖 Helpful? 👍/👎
There was a problem hiding this comment.
Thanks for putting up this draft PR. I looked through the tests only so far and I think we should have a discussion around it which would be more helpful than doing asynchronous feedback on the comments. Test cases generally make sense but I fail to decipher some differences and rules in them I fail to understand. Also I am missing tests using priorities when interaction is not a shortcut modifier, I would assume the same rules apply regardless of binding? I think there is opportunity to reduce code diff here by doing some code reuse in the tests without having tests full of if statements.
I also think it would be good to have tests with repeated input which I mentioned before. I didn't spot any such tests unless I missed something. This would look like e.g. with a binding 'X', here value of X button provided as binary
X: 0 1 1
Action 1 (Prio 1): 0 - 0
Action 2 (Prio 2): 0 1 0
a b c
a: button up, nothing happens
b: button down, triggers both, but only action 2 fires notification since higher prio
c: button down again (repeat), triggers none since action 1 saw and reacted to state transition but did not fire
…e with other test criteria, added the use of passing in InputActions to the tests also.
suearkinunity
left a comment
suearkinunity
left a comment
There was a problem hiding this comment.
There's a lot of repetition between Actions.md and Settings.md; please condense the information to one page and link to it from the other.
Co-authored-by: Sue Arkin <85237015+suearkinunity@users.noreply.github.com>
Co-authored-by: Sue Arkin <85237015+suearkinunity@users.noreply.github.com>
Co-authored-by: Sue Arkin <85237015+suearkinunity@users.noreply.github.com>
Co-authored-by: Sue Arkin <85237015+suearkinunity@users.noreply.github.com>
Co-authored-by: Sue Arkin <85237015+suearkinunity@users.noreply.github.com>
Co-authored-by: Sue Arkin <85237015+suearkinunity@users.noreply.github.com>
Co-authored-by: Sue Arkin <85237015+suearkinunity@users.noreply.github.com>
…ncept/input-consumption
This reverts commit 7d1c21c.
…ncept/input-consumption
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ncept/input-consumption
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
EditorPlatformType is more granular than SystemType — both MacOs13 and MacOs13Arm resolve to SystemType.MacOS, which caused duplicate job key errors after the Wrench 2.12.0 bump added MacOs13Arm to the default platform set. Switching to EditorPlatformType makes each job name unique. Also removes the temporary ExcludeUnsupportedPlatforms workaround for Unity 6.6 Mac jobs, which should no longer be needed with Wrench 2.12.0. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ncept/input-consumption
6 participants
Purpose of this PR
Implements priority-based shortcut overlap resolution for ISX-2510. Adds a new
InputAction.Priorityproperty and ashortcutKeysUseActionPrioritysetting so users can explicitly control which actions win when multiple bindings share the same control — replacing the opaque automatic complexity calculation that caused unpredictable results.Release Notes
InputAction.Priorityproperty (range 0–65535) to let users assign explicit per-action priority for shortcut overlap resolution.InputSettings.shortcutKeysUseActionPrioritysetting. When enabled, overlap resolution uses action priority instead of automatic composite complexity. Priority field is shown in the Input Actions editor when this mode is active.shortcutKeysConsumeInput(complexity-based resolution) andshortcutKeysUseActionPriority(priority-based resolution) are now independent; priority mode takes precedence when both are enabled.Functional Testing status
CoreTests_ActionsPriority.cs(971 lines) covers priority assignment, clamping, overlap suppression, same-priority co-performance, and interaction withshortcutKeysConsumeInput.CoreTests_Actions.csupdated for renamed internalcontrolGroupingAndComplexity→controlGroupingAndPriorityfield.shortcutKeysUseActionPriorityin Project Settings.Performance Testing Status
No performance impact. The priority resolution path follows the same O(n²) grouping loop as the existing complexity path; no additional allocations.
Overall Product Risks
Technical Risk: 2 — Replaces internal grouping logic in
InputActionStatethat has many edge cases; complexity path is preserved as a fallback, but the two paths share the same memory layout (controlGroupingAndPriority). Regression risk is mitigated by the new test suite.Halo Effect: 1 — Change is opt-in via a new setting; existing projects using
shortcutKeysConsumeInputare unaffected unless they also enableshortcutKeysUseActionPriority.Class diagram
Changes across
origin/develop...HEAD— newInputActionStateMonitorIndexstruct,InputAction.Priority,InputSettings.shortcutKeysUseActionPriority, and the internalControlGroupingTable.Class diagram
classDiagram direction TB class InputAction { +int Priority +int MinPriority$ +int MaxPriority$ +ClampPriority(int) int$ -int m_Priority ~OnActionPriorityChanged() } class InputSettings { +bool shortcutKeysConsumeInput +bool shortcutKeysUseActionPriority ~bool IsShortcutResolutionUsingActionPriority ~bool IsShortcutResolutionUsingComplexity ~bool IsShortcutComplexityModifierOrderActive -bool m_ShortcutKeysUseActionPriority } class InputActionState { -ushort* controlGroupingAndPriority +InitializeControlGrouping() ~OnActionPriorityChanged(InputAction) -GetControlMonitorGroupIndex(int) uint -GetControlBindingPriority(int) int } class ControlGroupingTable { <<static>> +int Stride$ +GroupElementIndex(int) int$ +PriorityElementIndex(int) int$ } class InputActionStateMonitorIndex { <<struct>> +long Packed +int ControlIndex +int BindingIndex +int MapIndex +int Priority +Create(int,int,int,int) InputActionStateMonitorIndex$ +FromPacked(long) InputActionStateMonitorIndex$ } class InputActionMap { ~InputActionState m_State } InputActionState --> ControlGroupingTable : uses InputActionState --> InputSettings : reads InputActionState --> InputActionStateMonitorIndex : creates InputAction --> InputActionMap : belongs to InputActionMap --> InputActionState : owns InputAction ..> InputActionState : notifies priorityDocumentation Impact
No new pages required. Both
InputAction.PriorityandInputSettings.shortcutKeysUseActionPriorityare already documented on this branch: