Skip to content

Commit c7c60c5

Browse files
author
Rene Damm
authored
FIX: Runtime errors when action for binding does not exist (case 1213085, #1063).
1 parent 3f46d65 commit c7c60c5

File tree

3 files changed

+93
-69
lines changed

3 files changed

+93
-69
lines changed

Assets/Tests/InputSystem/CoreTests_Actions.cs

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6308,6 +6308,28 @@ public void Actions_CanEnableAndDisableEntireMap()
63086308
Assert.That(action2.enabled, Is.False);
63096309
}
63106310

6311+
// https://fogbugz.unity3d.com/f/cases/1213085 (bindings that refer to non-existent actions should not lead to errors)
6312+
[Test]
6313+
[Category("Actions")]
6314+
public void Actions_CanEnableAndDisableEntireMap_EvenWhenBindingsReferToNonExistentActions()
6315+
{
6316+
var gamepad = InputSystem.AddDevice<Gamepad>();
6317+
6318+
var map = new InputActionMap();
6319+
map.AddAction("action", binding: "<Gamepad>/buttonSouth");
6320+
map.AddBinding("<Gamepad>/buttonNorth", action: "DoesNotExist");
6321+
6322+
// Also try the same for a composite binding.
6323+
map.AddBinding(new InputBinding { path = "1DAxis", isComposite = true, action = "DoesNotExist" });
6324+
map.AddBinding(new InputBinding { name = "Positive", path = "<Gamepad>/leftTrigger", isPartOfComposite = true });
6325+
map.AddBinding(new InputBinding { name = "Negative", path = "<Gamepad>/rightTrigger", isPartOfComposite = true });
6326+
6327+
Assert.That(() => map.Enable(), Throws.Nothing);
6328+
6329+
Assert.That(() => Press(gamepad.buttonNorth), Throws.Nothing);
6330+
Assert.That(() => Press(gamepad.leftTrigger), Throws.Nothing);
6331+
}
6332+
63116333
[Test]
63126334
[Category("Actions")]
63136335
public void Actions_CanEnableAndDisableSingleActionFromMap()

Packages/com.unity.inputsystem/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ however, it has to be formatted properly to pass verification tests.
1515

1616
- Mixing the enabling&disabling of single actions (as, for example, performed by `InputSystemUIInputModule`) with enabling&disabling of entire action maps (as, for example, performed by `PlayerInput`) no longer leaves to unresponsive input and `"should not reach here"` assertions ([forum thread](https://forum.unity.com/threads/error-while-switching-between-action-maps.825204/)).
1717
- Leaving play mode no longer leaves state change monitors lingering around from enabled actions.
18+
- Enabling action maps with bindings that do not refer to an existing action in the map no longer leads to asserts and exceptions when input on the bindings is received ([case 1213085](https://issuetracker.unity3d.com/issues/input-system-input-actions-cause-exceptions-and-should-not-get-here-errors-to-appear-after-deleting-an-action-map)).
1819

1920
## [1.0.0-preview.5] - 2020-02-14
2021

Packages/com.unity.inputsystem/InputSystem/Actions/InputBindingResolver.cs

Lines changed: 70 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -218,23 +218,38 @@ public unsafe void AddActionMap(InputActionMap map)
218218
action = currentCompositeAction;
219219
}
220220

221+
// If it's a composite, start a chain.
222+
if (isComposite)
223+
{
224+
currentCompositeBindingIndex = bindingIndex;
225+
currentCompositeAction = action;
226+
currentCompositeActionIndexInMap = actionIndexInMap;
227+
}
228+
221229
// Determine if the binding is disabled.
222230
// Disabled if path is empty.
223231
var path = unresolvedBinding.effectivePath;
224-
var bindingIsDisabled = string.IsNullOrEmpty(path);
232+
var bindingIsDisabled = string.IsNullOrEmpty(path)
225233

226-
// Also, disabled if binding doesn't match with our binding mask (might be empty).
227-
bindingIsDisabled |= !isComposite && bindingMask != null &&
228-
!bindingMask.Value.Matches(ref unresolvedBinding,
229-
InputBinding.MatchOptions.EmptyGroupMatchesAny);
234+
// Also, if we can't find the action to trigger for the binding, we just go and disable
235+
// the binding.
236+
|| action == null
230237

231-
// Also, disabled if binding doesn't match the binding mask on the map (might be empty).
232-
bindingIsDisabled |= !isComposite && bindingMaskOnThisMap != null &&
233-
!bindingMaskOnThisMap.Value.Matches(ref unresolvedBinding, InputBinding.MatchOptions.EmptyGroupMatchesAny);
238+
// Also, disabled if binding doesn't match with our binding mask (might be empty).
239+
|| (!isComposite && bindingMask != null &&
240+
!bindingMask.Value.Matches(ref unresolvedBinding,
241+
InputBinding.MatchOptions.EmptyGroupMatchesAny))
242+
243+
// Also, disabled if binding doesn't match the binding mask on the map (might be empty).
244+
|| (!isComposite && bindingMaskOnThisMap != null &&
245+
!bindingMaskOnThisMap.Value.Matches(ref unresolvedBinding,
246+
InputBinding.MatchOptions.EmptyGroupMatchesAny))
247+
248+
// Finally, also disabled if binding doesn't match the binding mask on the action (might be empty).
249+
|| (!isComposite && action?.m_BindingMask != null &&
250+
!action.m_BindingMask.Value.Matches(ref unresolvedBinding,
251+
InputBinding.MatchOptions.EmptyGroupMatchesAny));
234252

235-
// Finally, also disabled if binding doesn't match the binding mask on the action (might be empty).
236-
bindingIsDisabled |= !isComposite && action?.m_BindingMask != null &&
237-
!action.m_BindingMask.Value.Matches(ref unresolvedBinding, InputBinding.MatchOptions.EmptyGroupMatchesAny);
238253

239254
// If the binding isn't disabled, resolve its controls, processors, and interactions.
240255
if (!bindingIsDisabled)
@@ -248,7 +263,7 @@ public unsafe void AddActionMap(InputActionMap map)
248263
if (firstProcessorIndex != InputActionState.kInvalidIndex)
249264
numProcessors = totalProcessorCount - firstProcessorIndex;
250265
}
251-
if (action != null && !string.IsNullOrEmpty(action.m_Processors))
266+
if (!string.IsNullOrEmpty(action.m_Processors))
252267
{
253268
// Add processors from action.
254269
var index = ResolveProcessors(action.m_Processors);
@@ -269,7 +284,7 @@ public unsafe void AddActionMap(InputActionMap map)
269284
if (firstInteractionIndex != InputActionState.kInvalidIndex)
270285
numInteractions = totalInteractionCount - firstInteractionIndex;
271286
}
272-
if (action != null && !string.IsNullOrEmpty(action.m_Interactions))
287+
if (!string.IsNullOrEmpty(action.m_Interactions))
273288
{
274289
// Add interactions from action.
275290
var index = ResolveInteractions(action.m_Interactions);
@@ -281,74 +296,58 @@ public unsafe void AddActionMap(InputActionMap map)
281296
}
282297
}
283298

284-
// If it's the start of a composite chain, create the composite.
299+
// If it's the start of a composite chain, create the composite. Otherwise, go and
300+
// resolve controls for the binding.
285301
if (isComposite)
286302
{
287-
var actionIndexForComposite = actionIndexInMap != InputActionState.kInvalidIndex
288-
? actionStartIndex + actionIndexInMap
289-
: InputActionState.kInvalidIndex;
303+
// The composite binding entry itself does not resolve to any controls.
304+
// It creates a composite binding object which is then populated from
305+
// subsequent bindings.
290306

291307
// Instantiate. For composites, the path is the name of the composite.
292308
var composite = InstantiateBindingComposite(unresolvedBinding.path);
293309
currentCompositeIndex =
294310
ArrayHelpers.AppendWithCapacity(ref composites, ref totalCompositeCount, composite);
295-
currentCompositeBindingIndex = bindingIndex;
296-
currentCompositeAction = action;
297-
currentCompositeActionIndexInMap = actionIndexInMap;
298-
299-
*bindingState = new InputActionState.BindingState
300-
{
301-
actionIndex = actionIndexForComposite,
302-
compositeOrCompositeBindingIndex = currentCompositeIndex,
303-
processorStartIndex = firstProcessorIndex,
304-
processorCount = numProcessors,
305-
interactionCount = numInteractions,
306-
interactionStartIndex = firstInteractionIndex,
307-
mapIndex = totalMapCount,
308-
isComposite = true,
309-
// Record where the controls for parts of the composite start.
310-
controlStartIndex = memory.controlCount + resolvedControls.Count,
311-
};
312311

313-
// The composite binding entry itself does not resolve to any controls.
314-
// It creates a composite binding object which is then populated from
315-
// subsequent bindings.
316-
continue;
312+
// Record where the controls for parts of the composite start.
313+
firstControlIndex = memory.controlCount + resolvedControls.Count;
317314
}
318-
319-
// If we've reached the end of a composite chain, finish
320-
// off the current composite.
321-
if (!isPartOfComposite && currentCompositeBindingIndex != InputActionState.kInvalidIndex)
315+
else
322316
{
323-
currentCompositePartCount = 0;
324-
currentCompositeBindingIndex = InputActionState.kInvalidIndex;
325-
currentCompositeIndex = InputActionState.kInvalidIndex;
326-
currentCompositeAction = null;
327-
currentCompositeActionIndexInMap = InputActionState.kInvalidIndex;
328-
}
317+
// If we've reached the end of a composite chain, finish
318+
// off the current composite.
319+
if (!isPartOfComposite && currentCompositeBindingIndex != InputActionState.kInvalidIndex)
320+
{
321+
currentCompositePartCount = 0;
322+
currentCompositeBindingIndex = InputActionState.kInvalidIndex;
323+
currentCompositeIndex = InputActionState.kInvalidIndex;
324+
currentCompositeAction = null;
325+
currentCompositeActionIndexInMap = InputActionState.kInvalidIndex;
326+
}
329327

330-
// Look up controls.
331-
//
332-
// NOTE: We continuously add controls here to `resolvedControls`. Once we've completed our
333-
// pass over the bindings in the map, `resolvedControls` will have all the controls for
334-
// the current map.
335-
firstControlIndex = memory.controlCount + resolvedControls.Count;
336-
if (devicesForThisMap != null)
337-
{
338-
// Search in devices for only this map.
339-
var list = devicesForThisMap.Value;
340-
for (var i = 0; i < list.Count; ++i)
328+
// Look up controls.
329+
//
330+
// NOTE: We continuously add controls here to `resolvedControls`. Once we've completed our
331+
// pass over the bindings in the map, `resolvedControls` will have all the controls for
332+
// the current map.
333+
firstControlIndex = memory.controlCount + resolvedControls.Count;
334+
if (devicesForThisMap != null)
341335
{
342-
var device = list[i];
343-
if (!device.added)
344-
continue; // Skip devices that have been removed.
345-
numControls += InputControlPath.TryFindControls(device, path, 0, ref resolvedControls);
336+
// Search in devices for only this map.
337+
var list = devicesForThisMap.Value;
338+
for (var i = 0; i < list.Count; ++i)
339+
{
340+
var device = list[i];
341+
if (!device.added)
342+
continue; // Skip devices that have been removed.
343+
numControls += InputControlPath.TryFindControls(device, path, 0, ref resolvedControls);
344+
}
345+
}
346+
else
347+
{
348+
// Search globally.
349+
numControls = InputSystem.FindControls(path, ref resolvedControls);
346350
}
347-
}
348-
else
349-
{
350-
// Search globally.
351-
numControls = InputSystem.FindControls(path, ref resolvedControls);
352351
}
353352
}
354353

@@ -381,15 +380,17 @@ public unsafe void AddActionMap(InputActionMap map)
381380
*bindingState = new InputActionState.BindingState
382381
{
383382
controlStartIndex = firstControlIndex,
383+
// For composites, this will be adjusted as we add each part.
384384
controlCount = numControls,
385385
interactionStartIndex = firstInteractionIndex,
386386
interactionCount = numInteractions,
387387
processorStartIndex = firstProcessorIndex,
388388
processorCount = numProcessors,
389+
isComposite = isComposite,
389390
isPartOfComposite = unresolvedBinding.isPartOfComposite,
390391
partIndex = partIndex,
391392
actionIndex = actionIndexForBinding,
392-
compositeOrCompositeBindingIndex = currentCompositeBindingIndex,
393+
compositeOrCompositeBindingIndex = isComposite ? currentCompositeIndex : currentCompositeBindingIndex,
393394
mapIndex = totalMapCount,
394395
wantsInitialStateCheck = action?.wantsInitialStateCheck ?? false
395396
};

0 commit comments

Comments
 (0)