Skip to content

Commit 9c3facd

Browse files
committed
FIX: Fixed a bug within InputActionAsset.cs that prevented actions containing slashes to be found by name.
1 parent badaef8 commit 9c3facd

File tree

2 files changed

+56
-22
lines changed

2 files changed

+56
-22
lines changed

Assets/Tests/InputSystem/CoreTests_Actions.cs

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4780,6 +4780,7 @@ public void Actions_CanLookUpActionInMap()
47804780

47814781
var action1 = map.AddAction("action1");
47824782
var action2 = map.AddAction("action2");
4783+
var yawPitch = map.AddAction("Yaw/Pitch");
47834784

47844785
Assert.That(map.FindAction("action1"), Is.SameAs(action1));
47854786
Assert.That(map.FindAction("action2"), Is.SameAs(action2));
@@ -4788,6 +4789,10 @@ public void Actions_CanLookUpActionInMap()
47884789
// Lookup is case-insensitive.
47894790
Assert.That(map.FindAction("Action1"), Is.SameAs(action1));
47904791
Assert.That(map.FindAction("Action2"), Is.SameAs(action2));
4792+
4793+
// Lookup allows '/' within action name (https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1306)
4794+
// yawPitchAction = _actions.FindAction("Yaw/Pitch");
4795+
Assert.That(map.FindAction("Yaw/Pitch"), Is.SameAs(yawPitch));
47914796
}
47924797

47934798
[Test]
@@ -7917,6 +7922,7 @@ public void Actions_CanLookUpActionInAssetByName()
79177922
var action1 = map1.AddAction("action1");
79187923
var action2 = map1.AddAction("action2");
79197924
var action3 = map2.AddAction("action3");
7925+
var action4 = map2.AddAction("Yaw/Pitch");
79207926

79217927
Assert.That(asset.FindAction("action1"), Is.SameAs(action1));
79227928
Assert.That(asset.FindAction("action2"), Is.SameAs(action2));
@@ -7930,6 +7936,10 @@ public void Actions_CanLookUpActionInAssetByName()
79307936
Assert.That(asset.FindAction($"{{{action2.id.ToString()}}}"), Is.SameAs(action2));
79317937
Assert.That(asset.FindAction($"{{{action3.id.ToString()}}}"), Is.SameAs(action3));
79327938

7939+
// Lookup allows '/' within action name (https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1306)
7940+
// yawPitchAction = _actions.FindAction("Yaw/Pitch");
7941+
Assert.That(asset.FindAction("Yaw/Pitch"), Is.SameAs(action4));
7942+
79337943
// Shouldn't allocate.
79347944
var map1action1 = "map1/action1";
79357945
Assert.That(() =>
@@ -7938,6 +7948,26 @@ public void Actions_CanLookUpActionInAssetByName()
79387948
}, Is.Not.AllocatingGCMemory());
79397949
}
79407950

7951+
// https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1306
7952+
[Test]
7953+
[Category("Actions")]
7954+
public void Actions_CanLookUpActionInAssetByNameIfHavingActionAndMapActionWithSameName()
7955+
{
7956+
var asset = ScriptableObject.CreateInstance<InputActionAsset>();
7957+
7958+
var map1 = new InputActionMap("map1");
7959+
var map2 = new InputActionMap("Yaw");
7960+
7961+
asset.AddActionMap(map1);
7962+
asset.AddActionMap(map2);
7963+
7964+
var action1 = map1.AddAction("Yaw/Pitch");
7965+
var action2 = map2.AddAction("Pitch");
7966+
7967+
// map/action should be selected first
7968+
Assert.That(asset.FindAction("Yaw/Pitch"), Is.SameAs(action2));
7969+
}
7970+
79417971
// Since we allow looking up by action name without any map qualification, ambiguities result when several
79427972
// actions are named the same. We choose to not do anything special other than generally preferring an
79437973
// enabled action over a disabled one. Other than that, we just return the first hit.

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

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ public static InputActionAsset FromJson(string json)
471471
/// in the asset.
472472
/// </summary>
473473
/// <param name="actionNameOrId">Name of the action as either a "map/action" combination (e.g. "gameplay/fire") or
474-
/// a simple name. In the former case, the name is split at the '/' slash and the first part is used to find
474+
/// a simple name e.g. "fire". In the former case, the name is split at the '/' slash and the first part is used to find
475475
/// a map with that name and the second part is used to find an action with that name inside the map. In the
476476
/// latter case, all maps are searched in order and the first action that has the given name in any of the maps
477477
/// is returned. Note that name comparisons are case-insensitive.
@@ -491,6 +491,10 @@ public static InputActionAsset FromJson(string json)
491491
/// An exception is if, of the multiple actions with the same name, some are enabled and some are disabled. In
492492
/// this case, the first action that is enabled is returned.
493493
///
494+
/// If an action name contains a slash "/", e.g. "yaw/pitch" and there is also a map called "yaw" which
495+
/// contains an action "pitch", the action "pitch" within the map "jump" will be returned instead of the
496+
/// action named "yaw/pitch".
497+
///
494498
/// <example>
495499
/// <code>
496500
/// var asset = ScriptableObject.CreateInstance&lt;InputActionAsset&gt;();
@@ -533,28 +537,11 @@ public InputAction FindAction(string actionNameOrId, bool throwIfNotFound = fals
533537

534538
if (m_ActionMaps != null)
535539
{
536-
// Check if we have a "map/action" path.
540+
// Check if we have a "map/action" path. If we do we either has a "map/action" path or a simple
541+
// action name containing a slash. We first attempt matching it to a "map/action" and only if that
542+
// fails do we attempt to search for a "some/action" name.
537543
var indexOfSlash = actionNameOrId.IndexOf('/');
538-
if (indexOfSlash == -1)
539-
{
540-
// No slash so it's just a simple action name. Return either first enabled action or, if
541-
// none are enabled, first action with the given name.
542-
InputAction firstActionFound = null;
543-
for (var i = 0; i < m_ActionMaps.Length; ++i)
544-
{
545-
var action = m_ActionMaps[i].FindAction(actionNameOrId);
546-
if (action != null)
547-
{
548-
if (action.enabled || action.m_Id == actionNameOrId) // Match by ID is always exact.
549-
return action;
550-
if (firstActionFound == null)
551-
firstActionFound = action;
552-
}
553-
}
554-
if (firstActionFound != null)
555-
return firstActionFound;
556-
}
557-
else
544+
if (indexOfSlash >= 0)
558545
{
559546
// Have a path. First search for the map, then for the action.
560547
var mapName = new Substring(actionNameOrId, 0, indexOfSlash);
@@ -583,6 +570,23 @@ public InputAction FindAction(string actionNameOrId, bool throwIfNotFound = fals
583570
break;
584571
}
585572
}
573+
574+
// It's just a simple action name. Return either first enabled action or, if
575+
// none are enabled, first action with the given name.
576+
InputAction firstActionFound = null;
577+
for (var i = 0; i < m_ActionMaps.Length; ++i)
578+
{
579+
var action = m_ActionMaps[i].FindAction(actionNameOrId);
580+
if (action != null)
581+
{
582+
if (action.enabled || action.m_Id == actionNameOrId) // Match by ID is always exact.
583+
return action;
584+
if (firstActionFound == null)
585+
firstActionFound = action;
586+
}
587+
}
588+
if (firstActionFound != null)
589+
return firstActionFound;
586590
}
587591

588592
if (throwIfNotFound)

0 commit comments

Comments
 (0)