Skip to content

Commit 79c430d

Browse files
ekcohekcoh
committed
Improvements to testing, refactoring of tests, bug fixes to InputActionReference, simplification of InputActionImporter.
1 parent f38dd04 commit 79c430d

File tree

7 files changed

+194
-104
lines changed

7 files changed

+194
-104
lines changed

‎Assets/Tests/InputSystem/CoreTests_Actions.cs‎

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -11539,61 +11539,6 @@ public void Actions_CanCloneActionMaps()
1153911539
Assert.That(clone.actions[1].bindings[0].path, Is.EqualTo("/gamepad/rightStick"));
1154011540
}
1154111541

11542-
[Test]
11543-
[Category("Actions")]
11544-
public void Actions_CanResolveActionReference()
11545-
{
11546-
var map = new InputActionMap("map");
11547-
map.AddAction("action1");
11548-
var action2 = map.AddAction("action2");
11549-
var asset = ScriptableObject.CreateInstance<InputActionAsset>();
11550-
asset.AddActionMap(map);
11551-
11552-
var reference = ScriptableObject.CreateInstance<InputActionReference>();
11553-
reference.Set(asset, "map", "action2");
11554-
11555-
var referencedAction = reference.action;
11556-
11557-
Assert.That(referencedAction, Is.SameAs(action2));
11558-
}
11559-
11560-
[Test]
11561-
[Category("Actions")]
11562-
public void Actions_CanResolveActionReference_EvenAfterActionHasBeenRenamed()
11563-
{
11564-
var map = new InputActionMap("map");
11565-
var action = map.AddAction("oldName");
11566-
var asset = ScriptableObject.CreateInstance<InputActionAsset>();
11567-
asset.AddActionMap(map);
11568-
11569-
var reference = ScriptableObject.CreateInstance<InputActionReference>();
11570-
reference.Set(asset, "map", "oldName");
11571-
11572-
action.Rename("newName");
11573-
11574-
var referencedAction = reference.action;
11575-
11576-
Assert.That(referencedAction, Is.SameAs(action));
11577-
}
11578-
11579-
[Test(Description = "https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584")]
11580-
[Category("Actions")]
11581-
public void Actions_CanResolveActionReferenceAndThenSetItToAnotherAction()
11582-
{
11583-
var map = new InputActionMap("map");
11584-
var action1 = map.AddAction("action1");
11585-
var action2 = map.AddAction("action2");
11586-
var asset = ScriptableObject.CreateInstance<InputActionAsset>();
11587-
asset.AddActionMap(map);
11588-
11589-
var reference = ScriptableObject.CreateInstance<InputActionReference>();
11590-
reference.Set(asset, "map", "action2");
11591-
Assert.That(reference.action, Is.SameAs(action2)); // Redundant, but important for test case
11592-
11593-
reference.Set(asset, "map", "action1");
11594-
Assert.That(reference.action, Is.SameAs(action1));
11595-
}
11596-
1159711542
[Test]
1159811543
[Category("Actions")]
1159911544
public void Actions_CanDisableAllEnabledActionsInOneGo()

‎Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs‎

Lines changed: 126 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,126 @@
1+
using NUnit.Framework;
2+
using UnityEngine;
3+
using UnityEngine.InputSystem;
4+
5+
partial class CoreTests
6+
{
7+
private static InputActionAsset CreateAssetWithTwoActions()
8+
{
9+
var map1 = new InputActionMap("map1");
10+
map1.AddAction("action1");
11+
map1.AddAction("action2");
12+
var asset = ScriptableObject.CreateInstance<InputActionAsset>();
13+
asset.AddActionMap(map1);
14+
return asset;
15+
}
16+
17+
private static InputActionAsset CreateAssetWithSingleAction()
18+
{
19+
var map2 = new InputActionMap("map2");
20+
map2.AddAction("action3");
21+
var asset = ScriptableObject.CreateInstance<InputActionAsset>();
22+
asset.AddActionMap(map2);
23+
return asset;
24+
}
25+
26+
[Test]
27+
public void Actions_Reference_AssetAndActionsReturnsNull_IfNotSet()
28+
{
29+
var reference = ScriptableObject.CreateInstance<InputActionReference>();
30+
Assert.That(reference.asset, Is.Null);
31+
Assert.That(reference.action, Is.Null);
32+
Assert.That(reference.ToDisplayName(), Is.Null);
33+
}
34+
35+
[Test]
36+
public void Actions_Reference_SetNull()
37+
{
38+
var reference = ScriptableObject.CreateInstance<InputActionReference>();
39+
reference.Set(null);
40+
}
41+
42+
[Test]
43+
[Category("Actions")]
44+
public void Actions_Reference_CanResolveAction()
45+
{
46+
var asset = CreateAssetWithTwoActions();
47+
var reference = ScriptableObject.CreateInstance<InputActionReference>();
48+
49+
reference.Set(asset, "map1", "action2");
50+
51+
Assert.That(reference.action, Is.SameAs(asset.FindAction("map1/action2")));
52+
}
53+
54+
[Test]
55+
[Category("Actions")]
56+
public void Actions_Reference_CanResolveAction_EvenAfterActionHasBeenRenamed()
57+
{
58+
var map = new InputActionMap("map");
59+
var action = map.AddAction("oldName");
60+
var asset = ScriptableObject.CreateInstance<InputActionAsset>();
61+
asset.AddActionMap(map);
62+
63+
var reference = ScriptableObject.CreateInstance<InputActionReference>();
64+
reference.Set(asset, "map", "oldName");
65+
66+
action.Rename("newName");
67+
68+
var referencedAction = reference.action;
69+
70+
Assert.That(referencedAction, Is.SameAs(action));
71+
}
72+
73+
[Test(Description = "https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1584")]
74+
[Category("Actions")]
75+
public void Actions_Reference_CanResolveActionAfterReassignment()
76+
{
77+
var asset1 = CreateAssetWithTwoActions();
78+
var asset2 = CreateAssetWithSingleAction();
79+
80+
var reference = ScriptableObject.CreateInstance<InputActionReference>();
81+
reference.Set(asset1, "map1", "action1");
82+
Assert.That(reference.action, Is.Not.Null);
83+
Assert.That(reference.action, Is.SameAs(asset1.FindAction("map1/action1"))); // Redundant, but important for test case
84+
Assert.That(reference.asset, Is.SameAs(asset1));
85+
Assert.That(reference.name, Is.EqualTo("map1/action1"));
86+
Assert.That(reference.ToDisplayName(), Is.EqualTo("map1/action1"));
87+
Assert.That(reference.ToString(), Is.EqualTo(":map1/action1"));
88+
89+
reference.Set(asset2, "map2", "action3");
90+
Assert.That(reference.action, Is.Not.Null);
91+
Assert.That(reference.action, Is.SameAs(asset2.FindAction("map2/action3")));
92+
Assert.That(reference.asset, Is.SameAs(asset2));
93+
Assert.That(reference.name, Is.EqualTo("map2/action3"));
94+
Assert.That(reference.ToDisplayName(), Is.EqualTo("map2/action3"));
95+
Assert.That(reference.ToString(), Is.EqualTo(":map2/action3"));
96+
}
97+
98+
[Test]
99+
public void Actions_Reference_NameShouldReflectReferencedAction()
100+
{
101+
var map = new InputActionMap("map");
102+
var action1 = map.AddAction("action1");
103+
var asset = ScriptableObject.CreateInstance<InputActionAsset>();
104+
asset.AddActionMap(map);
105+
106+
var reference = ScriptableObject.CreateInstance<InputActionReference>();
107+
reference.Set(asset, "map", "action1");
108+
109+
Assert.That(reference.action, Is.SameAs(action1));
110+
Assert.That(reference.asset, Is.SameAs(asset));
111+
Assert.That(reference.name, Is.EqualTo("map/action1"));
112+
Assert.That(reference.ToDisplayName(), Is.EqualTo("map/action1"));
113+
Assert.That(reference.ToString(), Is.EqualTo(":map/action1"));
114+
115+
// Delete the referenced action
116+
117+
asset.RemoveAction("map/action1");
118+
119+
// TODO reference need to react to this
120+
Assert.That(reference.action, Is.SameAs(action1));
121+
Assert.That(reference.asset, Is.SameAs(asset));
122+
Assert.That(reference.name, Is.EqualTo("map/action1")); // Unexpected when no longer existing
123+
Assert.That(reference.ToDisplayName(), Is.EqualTo("map/action1")); // Unexpected when no longer existing
124+
Assert.That(reference.ToString(), Is.EqualTo(":map/action1")); // Unexpected when no longer existing
125+
}
126+
}

‎Assets/Tests/InputSystem/CoreTests_Actions_Reference.cs.meta‎

Lines changed: 3 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,10 @@ public class InputActionReference : ScriptableObject
4040
/// is not initialized or if the asset has been deleted.
4141
/// </summary>
4242
/// <value>InputActionAsset of the referenced action.</value>
43-
public InputActionAsset asset => m_Asset;
43+
public InputActionAsset asset => m_Action?.m_ActionMap != null ? m_Action.m_ActionMap.asset : m_Asset;
4444

4545
/// <summary>
46-
/// The action that the reference resolves to. Null if the action
47-
/// cannot be found.
46+
/// The action that the reference resolves to. Null if the action cannot be found.
4847
/// </summary>
4948
/// <value>The action that reference points to.</value>
5049
/// <remarks>
@@ -54,15 +53,11 @@ public InputAction action
5453
{
5554
get
5655
{
57-
if (m_Action == null)
58-
{
59-
if (m_Asset == null)
60-
return null;
61-
62-
m_Action = m_Asset.FindAction(new Guid(m_ActionId));
63-
}
64-
65-
return m_Action;
56+
if (m_Action != null)
57+
return m_Action;
58+
if (!m_Asset)
59+
return null;
60+
return (m_Action = m_Asset.FindAction(new Guid(m_ActionId)));;
6661
}
6762
}
6863

@@ -76,10 +71,12 @@ public InputAction action
7671
/// <see cref="InputActionMap"/> that is itself contained in an <see cref="InputActionAsset"/>.</exception>
7772
public void Set(InputAction action)
7873
{
74+
// TODO Throw if attempting to set reference on an input action reference that resides in an asset.
75+
7976
if (action == null)
8077
{
81-
m_Asset = default;
82-
m_ActionId = default;
78+
m_Asset = null;
79+
m_ActionId = null;
8380
return;
8481
}
8582

@@ -133,6 +130,8 @@ private void SetInternal(InputActionAsset asset, InputAction action)
133130
throw new ArgumentException(
134131
$"Action '{action}' is not contained in asset '{asset}'", nameof(action));
135132

133+
// If we are setting the reference in edit-mode, we want the state to be reflected in the serialized
134+
// object and hence assign serialized fields. This is a destructive operation.
136135
m_Asset = asset;
137136
m_ActionId = action.id.ToString();
138137
name = GetDisplayName(action);
@@ -149,8 +148,8 @@ public override string ToString()
149148
{
150149
try
151150
{
152-
var action = this.action;
153-
return $"{m_Asset.name}:{action.actionMap.name}/{action.name}";
151+
var value = action;
152+
return $"{m_Asset.name}:{value.actionMap.name}/{value.name}";
154153
}
155154
catch
156155
{
@@ -204,17 +203,21 @@ public static InputActionReference Create(InputAction action)
204203
/// Clears the cached <see cref="m_Action"/> field for all current <see cref="InputActionReference"/> objects.
205204
/// </summary>
206205
/// <remarks>
207-
/// After calling this, the next call to <see cref="action"/> will retrieve a new <see cref="InputAction"/> reference from the existing <see cref="InputActionAsset"/> just as if
208-
/// using it for the first time. The serialized <see cref="m_Asset"/> and <see cref="m_ActionId"/> fields are not touched and will continue to hold their current values.
206+
/// After calling this, the next call to <see cref="action"/> will retrieve a new <see cref="InputAction"/>
207+
/// reference from the existing <see cref="InputActionAsset"/> just as if using it for the first time.
208+
/// The serialized <see cref="m_Asset"/> and <see cref="m_ActionId"/> fields are not touched and will continue
209+
/// to hold their current values.
209210
///
210-
/// This method is used to clear the Action references when exiting PlayMode since those objects are no longer valid.
211+
/// This method is used to clear the Action references when exiting PlayMode since those objects are no
212+
/// longer valid.
211213
/// </remarks>
212214
internal static void ResetCachedAction()
213215
{
214216
var allActionRefs = Resources.FindObjectsOfTypeAll(typeof(InputActionReference));
215-
foreach (InputActionReference obj in allActionRefs)
217+
foreach (var obj in allActionRefs)
216218
{
217-
obj.m_Action = null;
219+
var reference = (InputActionReference)obj;
220+
reference.m_Action = null;
218221
}
219222
}
220223

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ public static void RemoveAction(this InputAction action)
240240
action.m_SingletonActionBindings = bindingsForAction;
241241

242242
// Remove bindings to action from map.
243-
var newActionMapBindingCount = actionMap.m_Bindings.Length - bindingsForAction.Length;
243+
var newActionMapBindingCount = actionMap.m_Bindings != null ? actionMap.m_Bindings.Length - bindingsForAction.Length : 0;
244244
if (newActionMapBindingCount == 0)
245245
{
246246
actionMap.m_Bindings = null;

‎Packages/com.unity.inputsystem/InputSystem/Editor/AssetImporter/InputActionImporter.cs‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -285,8 +285,15 @@ internal static IEnumerable<InputActionReference> LoadInputActionReferencesFromA
285285
.Cast<InputActionReference>();
286286
}
287287

288-
// Get all InputActionReferences from assets in the project. By default it only gets the assets in the "Assets" folder.
289-
internal static IEnumerable<InputActionReference> LoadInputActionReferencesFromAssetDatabase(string[] foldersPath = null, bool skipProjectWide = false)
288+
/// <summary>
289+
/// Gets all <see cref="InputActionReference"/> instances available in assets in the project.
290+
/// By default, it only gets the assets located in the "Assets" folder.
291+
/// </summary>
292+
/// <param name="foldersPath"></param>
293+
/// <param name="skipProjectWide"></param>
294+
/// <returns></returns>
295+
internal static IEnumerable<InputActionReference> LoadInputActionReferencesFromAssetDatabase(
296+
string[] foldersPath = null, bool skipProjectWide = false)
290297
{
291298
string[] searchFolders = null;
292299
// If folderPath is null, search in "Assets" folder.
@@ -307,15 +314,13 @@ internal static IEnumerable<InputActionReference> LoadInputActionReferencesFromA
307314
foreach (var guid in inputActionReferenceGUIDs)
308315
{
309316
var assetPath = AssetDatabase.GUIDToAssetPath(guid);
310-
var assetInputActionReferenceList = LoadInputActionReferencesFromAsset(assetPath).ToList();
311-
312-
if (skipProjectWide && assetInputActionReferenceList.Count() > 0)
317+
foreach (var assetInputActionReference in LoadInputActionReferencesFromAsset(assetPath))
313318
{
314-
if (assetInputActionReferenceList[0].m_Asset == InputSystem.actions)
319+
if (skipProjectWide && assetInputActionReference.m_Asset == InputSystem.actions)
315320
continue;
316-
}
317321

318-
inputActionReferencesList.AddRange(assetInputActionReferenceList);
322+
inputActionReferencesList.Add(assetInputActionReference);
323+
}
319324
}
320325
return inputActionReferencesList;
321326
}

0 commit comments

Comments
 (0)