Skip to content

Commit 3314fcf

Browse files
authored
FIX: Properly reset ActionMap state in OnAfterDeserialize (ISXB-737) (#1832)
* FIX: Properly reset ActionMap state in OnAfterDeserialize (ISXB-737) * The m_EnabledActionsCount field reports "enabled" state and must be reset for ActionMap to be re-enabled * Add regression test cases * Actions_ActionMapDisabledDuringOnAfterSerialization covers the specific scenario * ProjectWideActions_ThrowsWhenAddingOrRemovingWhileEnabled validates Setup cannot change while ActionMap is enabled
1 parent 48879b9 commit 3314fcf

File tree

4 files changed

+77
-1
lines changed

4 files changed

+77
-1
lines changed

Assets/Tests/InputSystem/CoreTests_Actions.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11923,4 +11923,36 @@ public void TODO_Actions_ReResolvingBindings_DoesNotAllocate_IfXXX()
1192311923
{
1192411924
Assert.Fail();
1192511925
}
11926+
11927+
// Validate OnAfterDeserialize() completely disables ActionMap
11928+
// https://jira.unity3d.com/browse/ISXB-737
11929+
[Test]
11930+
[Category("Actions")]
11931+
public void Actions_ActionMapDisabledDuringOnAfterSerialization()
11932+
{
11933+
var map = new InputActionMap("MyMap");
11934+
var action = map.AddAction("MyAction");
11935+
action.AddCompositeBinding("2DVector")
11936+
.With("Up", "<Keyboard>/w")
11937+
.With("Down", "<Keyboard>/s")
11938+
.With("Left", "<Keyboard>/a")
11939+
.With("Right", "<Keyboard>/d");
11940+
11941+
map.Enable();
11942+
11943+
Assert.That(map.enabled, Is.True);
11944+
Assert.That(map.FindAction("MyAction", true).enabled, Is.True);
11945+
Assert.Throws<System.InvalidOperationException>(() => map.AddAction("Something"));
11946+
11947+
// Calling this directly is contrived (and not supported usage) but it should
11948+
// effectively cover this regression scenario.
11949+
map.OnAfterDeserialize();
11950+
11951+
Assert.That(map.enabled, Is.False);
11952+
11953+
map.Enable();
11954+
11955+
Assert.That(map.enabled, Is.True);
11956+
Assert.That(map.FindAction("MyAction", true).enabled, Is.True);
11957+
}
1192611958
}

Assets/Tests/InputSystem/CoreTests_ProjectWideActions.cs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,48 @@ public void ProjectWideActions_CanReplaceExistingActions()
253253
Assert.That(InputSystem.actions.actionMaps[1].actions.Count, Is.EqualTo(1));
254254
Assert.That(InputSystem.actions.actionMaps[1].actions[0].name, Is.EqualTo("replacedAction4"));
255255
}
256-
}
257256

257+
#if UNITY_EDITOR
258+
[Test]
259+
[Category(TestCategory)]
260+
public void ProjectWideActions_ThrowsWhenAddingOrRemovingWhileEnabled()
261+
{
262+
var asset = ProjectWideActionsAsset.GetOrCreate();
263+
264+
// Verify adding ActionMap while enabled throws an exception
265+
Assert.Throws<InvalidOperationException>(() => asset.AddActionMap("AnotherMap").AddAction("AnotherAction"));
266+
267+
asset.Disable();
268+
asset.AddActionMap("AnotherMap").AddAction("AnotherAction");
269+
270+
// Verify enabled state reported correctly
271+
Assert.That(asset.enabled, Is.False);
272+
Assert.That(asset.FindActionMap("AnotherMap", true).enabled, Is.False);
273+
Assert.That(asset.FindAction("AnotherAction", true).enabled, Is.False);
274+
275+
asset.Enable();
276+
277+
Assert.That(asset.enabled, Is.True);
278+
Assert.That(asset.FindActionMap("AnotherMap", true).enabled, Is.True);
279+
Assert.That(asset.FindAction("AnotherAction", true).enabled, Is.True);
280+
281+
// Verify adding/removing actions throws when ActionMap is enabled
282+
Assert.Throws<System.InvalidOperationException>(() => asset.FindActionMap("AnotherMap", true).AddAction("YetAnotherAction"));
283+
Assert.Throws<System.InvalidOperationException>(() => asset.RemoveAction("AnotherAction"));
284+
Assert.Throws<InvalidOperationException>(() => asset.RemoveActionMap("AnotherMap"));
285+
286+
// Verify enabled state when enabling Action directly
287+
asset.Disable();
288+
asset.FindAction("AnotherAction", true).Enable();
289+
290+
Assert.That(asset.FindActionMap("InitialActionMapOne", true).enabled, Is.False);
291+
Assert.That(asset.FindActionMap("AnotherMap", true).enabled, Is.True);
292+
Assert.That(asset.FindAction("AnotherAction", true).enabled, Is.True);
293+
294+
// Verify removing any ActionMap throws if another one is enabled
295+
Assert.Throws<System.InvalidOperationException>(() => asset.RemoveActionMap("InitialActionMapOne"));
296+
}
297+
298+
#endif // UNITY_EDITOR
299+
}
258300
#endif

Packages/com.unity.inputsystem/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ however, it has to be formatted properly to pass verification tests.
3939
- Fixed "Listen" functionality for selecting an input sometimes expecting the wrong input type.
4040
- Fixed console errors that can be produced when opening input package settings from the Inspector.
4141
- Fixed InputManager.asset file growing in size on each Reset call.
42+
- Fixed Opening InputDebugger throws 'Action map must have state at this point' error
4243

4344
## [1.8.0-pre.2] - 2023-11-09
4445

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1984,6 +1984,7 @@ public void OnAfterDeserialize()
19841984
{
19851985
m_State = null;
19861986
m_MapIndexInState = InputActionState.kInvalidIndex;
1987+
m_EnabledActionsCount = 0;
19871988

19881989
// Restore references of actions linking back to us.
19891990
if (m_Actions != null)

0 commit comments

Comments
 (0)