Skip to content

Commit c9b831e

Browse files
author
Rene Damm
authored
FIX: Exceptions when removing action callback from callback (case 1192972, #1029).
1 parent 8e00ce0 commit c9b831e

File tree

4 files changed

+55
-43
lines changed

4 files changed

+55
-43
lines changed

‎Assets/Tests/InputSystem/CoreTests_Actions.cs‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,35 @@ public void Actions_ValueActionsEnabledInOnEvent_DoNotReactToCurrentStateOfContr
398398
}
399399
}
400400

401+
// https://fogbugz.unity3d.com/f/cases/1192972/
402+
[Test]
403+
[Category("Actions")]
404+
public void Actions_CanRemoveCallbackInCallback()
405+
{
406+
var gamepad = InputSystem.AddDevice<Gamepad>();
407+
408+
var action = new InputAction(type: InputActionType.Value, binding: "<Gamepad>/buttonSouth");
409+
action.Enable();
410+
411+
var callback1Triggered = false;
412+
var callback2Triggered = false;
413+
414+
Action<InputAction.CallbackContext> callback1 = null;
415+
callback1 = _ =>
416+
{
417+
callback1Triggered = true;
418+
action.performed -= callback1;
419+
};
420+
421+
action.performed += callback1;
422+
action.performed += _ => callback2Triggered = true;
423+
424+
Press(gamepad.buttonSouth);
425+
426+
Assert.That(callback1Triggered, Is.True);
427+
Assert.That(callback2Triggered, Is.True);
428+
}
429+
401430
[Test]
402431
[Category("Actions")]
403432
public void Actions_CanBeDisabledInCallback()
@@ -6477,17 +6506,17 @@ public void Actions_ExceptionsInCallbacksAreCaughtAndLogged()
64776506

64786507
LogAssert.Expect(LogType.Error,
64796508
new Regex(
6480-
".*InvalidOperationException thrown during execution of callback for 'Started' phase of 'testAction.*' action in map 'testMap'.*"));
6509+
".*InvalidOperationException while executing 'started' callbacks of 'testMap'"));
64816510
LogAssert.Expect(LogType.Exception, new Regex(".*TEST EXCEPTION FROM MAP.*"));
64826511

64836512
LogAssert.Expect(LogType.Error,
64846513
new Regex(
6485-
".*InvalidOperationException thrown during execution of 'Performed' callback on action 'testMap/testAction.*'.*"));
6514+
".*InvalidOperationException while executing 'performed' callbacks of 'testMap/testAction.*'"));
64866515
LogAssert.Expect(LogType.Exception, new Regex(".*TEST EXCEPTION FROM ACTION.*"));
64876516

64886517
LogAssert.Expect(LogType.Error,
64896518
new Regex(
6490-
".*InvalidOperationException thrown during execution of callback for 'Performed' phase of 'testAction.*' action in map 'testMap'.*"));
6519+
".*InvalidOperationException while executing 'performed' callbacks of 'testMap'"));
64916520
LogAssert.Expect(LogType.Exception, new Regex(".*TEST EXCEPTION FROM MAP.*"));
64926521

64936522
InputSystem.QueueStateEvent(gamepad, new GamepadState().WithButton(GamepadButton.South));

‎Packages/com.unity.inputsystem/CHANGELOG.md‎

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

1414
#### Actions
1515

16+
- Removing a callback from actions from the callback itself no longer throws `ArgumentOutOfRangeException` ([case 1192972](https://issuetracker.unity3d.com/issues/input-system-package-argumentoutofrangeexception-error-is-thrown-when-the-callback-is-removed-while-its-being-triggered)).
1617
- "Invalid user" `ArgumentException` when turning the same `PlayerInput` on and off ([case 1198889](https://issuetracker.unity3d.com/issues/input-system-package-argumentexception-invalid-user-error-is-thrown-when-the-callback-disables-game-object-with-playerinput)).
1718
- The list of device requirements for a control scheme in the action editor no longer displays devices with their internal layout name rather than their external display name.
1819

‎Packages/com.unity.inputsystem/InputSystem/Actions/InputActionState.cs‎

Lines changed: 7 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,21 +1542,21 @@ private void ChangePhaseOfAction(InputActionPhase newPhase, ref TriggerState tri
15421542
{
15431543
case InputActionPhase.Started:
15441544
{
1545-
CallActionListeners(actionIndex, map, newPhase, ref action.m_OnStarted);
1545+
CallActionListeners(actionIndex, map, newPhase, ref action.m_OnStarted, "started");
15461546
break;
15471547
}
15481548

15491549
case InputActionPhase.Performed:
15501550
{
1551-
CallActionListeners(actionIndex, map, newPhase, ref action.m_OnPerformed);
1551+
CallActionListeners(actionIndex, map, newPhase, ref action.m_OnPerformed, "performed");
15521552
if (actionState->phase != InputActionPhase.Disabled) // Action may have been disabled in callback.
15531553
actionState->phase = phaseAfterPerformedOrCanceled;
15541554
break;
15551555
}
15561556

15571557
case InputActionPhase.Canceled:
15581558
{
1559-
CallActionListeners(actionIndex, map, newPhase, ref action.m_OnCanceled);
1559+
CallActionListeners(actionIndex, map, newPhase, ref action.m_OnCanceled, "canceled");
15601560
if (actionState->phase != InputActionPhase.Disabled) // Action may have been disabled in callback.
15611561
actionState->phase = phaseAfterPerformedOrCanceled;
15621562
break;
@@ -1572,7 +1572,7 @@ private void ChangePhaseOfAction(InputActionPhase newPhase, ref TriggerState tri
15721572
}
15731573
}
15741574

1575-
private void CallActionListeners(int actionIndex, InputActionMap actionMap, InputActionPhase phase, ref InlinedArray<InputActionListener> listeners)
1575+
private void CallActionListeners(int actionIndex, InputActionMap actionMap, InputActionPhase phase, ref InlinedArray<InputActionListener> listeners, string callbackName)
15761576
{
15771577
// If there's no listeners, don't bother with anything else.
15781578
var callbacksOnMap = actionMap.m_ActionCallbacks;
@@ -1588,10 +1588,9 @@ private void CallActionListeners(int actionIndex, InputActionMap actionMap, Inpu
15881588
Profiler.BeginSample("InputActionCallback");
15891589

15901590
// Global callback goes first.
1591+
var action = context.action;
15911592
if (s_OnActionChange.length > 0)
15921593
{
1593-
var action = context.action;
1594-
15951594
InputActionChange change;
15961595
switch (phase)
15971596
{
@@ -1614,36 +1613,10 @@ private void CallActionListeners(int actionIndex, InputActionMap actionMap, Inpu
16141613
}
16151614

16161615
// Run callbacks (if any) directly on action.
1617-
var listenerCount = listeners.length;
1618-
for (var i = 0; i < listenerCount; ++i)
1619-
{
1620-
try
1621-
{
1622-
listeners[i](context);
1623-
}
1624-
catch (Exception exception)
1625-
{
1626-
Debug.LogError(
1627-
$"{exception.GetType().Name} thrown during execution of '{phase}' callback on action '{GetActionOrNull(ref actionStates[actionIndex])}'");
1628-
Debug.LogException(exception);
1629-
}
1630-
}
1616+
DelegateHelpers.InvokeCallbacksSafe(ref listeners, context, callbackName, action);
16311617

16321618
// Run callbacks (if any) on action map.
1633-
var listenerCountOnMap = callbacksOnMap.length;
1634-
for (var i = 0; i < listenerCountOnMap; ++i)
1635-
{
1636-
try
1637-
{
1638-
callbacksOnMap[i](context);
1639-
}
1640-
catch (Exception exception)
1641-
{
1642-
Debug.LogError(
1643-
$"{exception.GetType().Name} thrown during execution of callback for '{phase}' phase of '{GetActionOrNull(ref actionStates[actionIndex]).name}' action in map '{actionMap.name}'");
1644-
Debug.LogException(exception);
1645-
}
1646-
}
1619+
DelegateHelpers.InvokeCallbacksSafe(ref callbacksOnMap, context, callbackName, actionMap);
16471620

16481621
Profiler.EndSample();
16491622
}

‎Packages/com.unity.inputsystem/InputSystem/Utilities/DelegateHelpers.cs‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ internal static class DelegateHelpers
88
// InvokeCallbacksSafe protects both against the callback getting removed while being called
99
// and against exceptions being thrown by the callback.
1010

11-
public static void InvokeCallbacksSafe(ref InlinedArray<Action> callbacks, string callbackName)
11+
public static void InvokeCallbacksSafe(ref InlinedArray<Action> callbacks, string callbackName, object context = null)
1212
{
1313
Profiler.BeginSample(callbackName);
1414
for (var i = 0; i < callbacks.length; ++i)
@@ -21,7 +21,10 @@ public static void InvokeCallbacksSafe(ref InlinedArray<Action> callbacks, strin
2121
}
2222
catch (Exception exception)
2323
{
24-
Debug.LogError($"{exception.GetType().Name} while executing '{callbackName}' callbacks");
24+
if (context != null)
25+
Debug.LogError($"{exception.GetType().Name} while executing '{callbackName}' callbacks of '{context}'");
26+
else
27+
Debug.LogError($"{exception.GetType().Name} while executing '{callbackName}' callbacks");
2528
Debug.LogException(exception);
2629
}
2730

@@ -32,7 +35,7 @@ public static void InvokeCallbacksSafe(ref InlinedArray<Action> callbacks, strin
3235
Profiler.EndSample();
3336
}
3437

35-
public static void InvokeCallbacksSafe<TValue>(ref InlinedArray<Action<TValue>> callbacks, TValue argument, string callbackName)
38+
public static void InvokeCallbacksSafe<TValue>(ref InlinedArray<Action<TValue>> callbacks, TValue argument, string callbackName, object context = null)
3639
{
3740
Profiler.BeginSample(callbackName);
3841
for (var i = 0; i < callbacks.length; ++i)
@@ -45,7 +48,10 @@ public static void InvokeCallbacksSafe<TValue>(ref InlinedArray<Action<TValue>>
4548
}
4649
catch (Exception exception)
4750
{
48-
Debug.LogError($"{exception.GetType().Name} while executing '{callbackName}' callbacks");
51+
if (context != null)
52+
Debug.LogError($"{exception.GetType().Name} while executing '{callbackName}' callbacks of '{context}'");
53+
else
54+
Debug.LogError($"{exception.GetType().Name} while executing '{callbackName}' callbacks");
4955
Debug.LogException(exception);
5056
}
5157

@@ -56,7 +62,7 @@ public static void InvokeCallbacksSafe<TValue>(ref InlinedArray<Action<TValue>>
5662
Profiler.EndSample();
5763
}
5864

59-
public static void InvokeCallbacksSafe<TValue1, TValue2>(ref InlinedArray<Action<TValue1, TValue2>> callbacks, TValue1 argument1, TValue2 argument2, string callbackName)
65+
public static void InvokeCallbacksSafe<TValue1, TValue2>(ref InlinedArray<Action<TValue1, TValue2>> callbacks, TValue1 argument1, TValue2 argument2, string callbackName, object context = null)
6066
{
6167
Profiler.BeginSample(callbackName);
6268
for (var i = 0; i < callbacks.length; ++i)
@@ -69,7 +75,10 @@ public static void InvokeCallbacksSafe<TValue1, TValue2>(ref InlinedArray<Action
6975
}
7076
catch (Exception exception)
7177
{
72-
Debug.LogError($"{exception.GetType().Name} while executing '{callbackName}' callbacks");
78+
if (context != null)
79+
Debug.LogError($"{exception.GetType().Name} while executing '{callbackName}' callbacks of '{context}'");
80+
else
81+
Debug.LogError($"{exception.GetType().Name} while executing '{callbackName}' callbacks");
7382
Debug.LogException(exception);
7483
}
7584

0 commit comments

Comments
 (0)