Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 25 additions & 2 deletions Assets/Tests/InputSystem.Editor/CustomProcessorEnumTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,19 @@ public override float Process(float value, InputControl control)
internal class CustomProcessorEnumTest : UIToolkitBaseTestWindow<InputActionsEditorWindow>
{
InputActionAsset m_Asset;
InputActionMap m_ActionMap;

public override void OneTimeSetUp()
{
base.OneTimeSetUp();
m_Asset = AssetDatabaseUtils.CreateAsset<InputActionAsset>();

var actionMap = m_Asset.AddActionMap("Action Map");
m_ActionMap = m_Asset.AddActionMap("Action Map");

actionMap.AddAction("Action", InputActionType.Value, processors: "Custom(SomeEnum=10)");
var action = m_ActionMap.AddAction("Action", InputActionType.Value, processors: "InvertVector2(invertX=false), Custom(SomeEnum=10)");

// A binding is needed to resolve during test.
action.AddBinding("<Gamepad>/leftTrigger");
}

public override void OneTimeTearDown()
Expand Down Expand Up @@ -108,5 +112,24 @@ public IEnumerator ProcessorEnum_ShouldSerializeByValue_WhenSerializedToAsset()

yield return null;
}

[Test]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are missing a test case here that tests that an action map containing 2 processors are working. It's the whole reason we are doing this fix. We know that with a single processor everything was working.

public void Migration_ShouldProduceValidActionAsset_WithAllProcessors_AndEnumConverted()
{
m_ActionMap.ResolveBindings();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test passes without the fix, this means this is not testing what we want. The asset in the editor is created with version 1 already, the migration code is not being run at all as part of this test.

There are 2 problems with the way we create the asset:

  • The asset is created once and shared across the different tests, which means the order in which the tests run could impact the results of the test, this is bad.
  • The migration code requires the version to be 0, creating the asset via the editor will create an asset with version 1.

Also the start data already has the CustomProcessor with the value being 10, the initial value should be 1, and then the conversion should run and upgrade it to 10.

This test is not a UI test, but its part of the UIToolkitBaseTestWindow class, I would recommend splitting this test into its own class that has no dependency on the UI.


var action = m_ActionMap.actions.First();
var migratedList = UnityEngine.InputSystem.Utilities.NameAndParameters.ParseMultiple(action.processors).ToList();

Assert.That(migratedList.Count, Is.EqualTo(2), "Expected two processors.");
Assert.That(migratedList[0].name, Is.EqualTo("InvertVector2"), "First processor should be InvertVector2.");

var invertX = migratedList[0].parameters.FirstOrDefault(p => p.name == "invertX");
Assert.That(invertX.name, Is.EqualTo("invertX"));
Assert.That(invertX.value.ToBoolean(), Is.False, "invertX should be false for the first processor.");

Assert.That(m_ActionMap.m_State.processors[1].GetType(), Is.EqualTo(typeof(CustomProcessor)));
Assert.That((m_ActionMap.m_State.processors[1] as CustomProcessor).SomeEnum, Is.EqualTo(SomeEnum.OptionA));
}
}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -1016,6 +1016,11 @@
return;
if ((parsedJson.maps?.Length ?? 0) > 0 && (parsedJson.version) < JsonVersion.Version1)
{
List<NameAndParameters> parsedList = null;
var converted = new List<NameAndParameters>(8);
var updatedParameters = new List<NamedValue>(4);
var enumValuesCache = new Dictionary<Type, Array>(8);

for (var mi = 0; mi < parsedJson.maps.Length; ++mi)
{
var mapJson = parsedJson.maps[mi];
Expand All @@ -1026,44 +1031,49 @@
if (string.IsNullOrEmpty(raw))
continue;

var list = NameAndParameters.ParseMultiple(raw).ToList();
var rebuilt = new List<string>(list.Count);
foreach (var nap in list)
if (!NameAndParameters.ParseMultiple(raw, ref parsedList) || parsedList.Count == 0)
continue;

Check warning on line 1035 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs#L1034-L1035

Added lines #L1034 - L1035 were not covered by tests

converted.Clear();

Check warning on line 1037 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

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

Added line #L1037 was not covered by tests

for (int i = 0; i < parsedList.Count; ++i)

Check warning on line 1039 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

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

Added line #L1039 was not covered by tests
{
var nap = parsedList[i];

Check warning on line 1041 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

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

Added line #L1041 was not covered by tests
var procType = InputSystem.TryGetProcessor(nap.name);
if (nap.parameters.Count == 0 || procType == null)
{
rebuilt.Add(nap.ToString());
converted.Add(nap);

Check warning on line 1045 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

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

Added line #L1045 was not covered by tests
continue;
}

var dict = nap.parameters.ToDictionary(p => p.name, p => p.value.ToString());
var anyChanged = false;
foreach (var field in procType.GetFields(BindingFlags.Public | BindingFlags.Instance).Where(f => f.FieldType.IsEnum))
updatedParameters.Clear();
for (int k = 0; k < nap.parameters.Count; ++k)

Check warning on line 1049 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs#L1048-L1049

Added lines #L1048 - L1049 were not covered by tests
{
if (dict.TryGetValue(field.Name, out var ordS) && int.TryParse(ordS, out var ord))
var param = nap.parameters[k];
var updatedPar = param;

Check warning on line 1052 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs#L1051-L1052

Added lines #L1051 - L1052 were not covered by tests

var fieldInfo = procType.GetField(param.name, BindingFlags.Public | BindingFlags.Instance);
if (fieldInfo != null && fieldInfo.FieldType.IsEnum)

Check warning on line 1055 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs#L1054-L1055

Added lines #L1054 - L1055 were not covered by tests
{
var values = Enum.GetValues(field.FieldType).Cast<object>().ToArray();
if (ord >= 0 && ord < values.Length)
var index = param.value.ToInt32();
if (index >= 0)

Check warning on line 1058 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs#L1057-L1058

Added lines #L1057 - L1058 were not covered by tests
{
dict[field.Name] = Convert.ToInt32(values[ord]).ToString();
anyChanged = true;
if (!enumValuesCache.TryGetValue(fieldInfo.FieldType, out var values))
{
values = Enum.GetValues(fieldInfo.FieldType);
enumValuesCache[fieldInfo.FieldType] = values;
}
if (index < values.Length)
{
var convertedValue = Convert.ToInt32(values.GetValue(index));
updatedPar = NamedValue.From(param.name, convertedValue);
}

Check warning on line 1069 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs#L1060-L1069

Added lines #L1060 - L1069 were not covered by tests
}
}
updatedParameters.Add(updatedPar);

Check warning on line 1072 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

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

Added line #L1072 was not covered by tests
}

if (!anyChanged)
{
rebuilt.Add(nap.ToString());
}
else
{
var paramText = string.Join(",", dict.Select(kv => $"{kv.Key}={kv.Value}"));
rebuilt.Add($"{nap.name}({paramText})");
}
converted.Add(NameAndParameters.Create(nap.name, updatedParameters));

Check warning on line 1074 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

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

Added line #L1074 was not covered by tests
}

actionJson.processors = string.Join(";", rebuilt);
actionJson.processors = NameAndParameters.ToSerializableString(converted);

Check warning on line 1076 in Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

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

Added line #L1076 was not covered by tests
mapJson.actions[ai] = actionJson;
}
parsedJson.maps[mi] = mapJson;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
var newElement = new NameAndParameters() { name = name};
interactionsOrProcessorsList.Add(newElement);

m_ListProperty.stringValue = ToSerializableString(interactionsOrProcessorsList);
m_ListProperty.stringValue = NameAndParameters.ToSerializableString(interactionsOrProcessorsList);

Check warning on line 40 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs#L40

Added line #L40 was not covered by tests
m_ListProperty.serializedObject.ApplyModifiedProperties();
}

Expand All @@ -61,35 +61,26 @@
if (interactionsOrProcessors.Length == 0 || !newIndexIsValid || !oldIndexIsValid)
return;
MemoryHelpers.Swap(ref interactionsOrProcessors[oldIndex], ref interactionsOrProcessors[newIndex]);
m_ListProperty.stringValue = ToSerializableString(interactionsOrProcessors);
m_ListProperty.stringValue = NameAndParameters.ToSerializableString(interactionsOrProcessors);

Check warning on line 64 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs#L64

Added line #L64 was not covered by tests
m_ListProperty.serializedObject.ApplyModifiedProperties();
}

private void DeleteElement(int index)
{
var interactionsOrProcessorsList = NameAndParameters.ParseMultiple(m_ListProperty.stringValue).ToList();
interactionsOrProcessorsList.RemoveAt(index);
m_ListProperty.stringValue = ToSerializableString(interactionsOrProcessorsList);
m_ListProperty.stringValue = NameAndParameters.ToSerializableString(interactionsOrProcessorsList);

Check warning on line 72 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/NameAndParametersListView.cs#L72

Added line #L72 was not covered by tests
m_ListProperty.serializedObject.ApplyModifiedProperties();
}

private void OnParametersChanged(ParameterListView listView, int index)
{
var interactionsOrProcessorsList = NameAndParameters.ParseMultiple(m_ListProperty.stringValue).ToList();
interactionsOrProcessorsList[index] = new NameAndParameters { name = interactionsOrProcessorsList[index].name, parameters = listView.GetParameters() };
m_ListProperty.stringValue = ToSerializableString(interactionsOrProcessorsList);
m_ListProperty.stringValue = NameAndParameters.ToSerializableString(interactionsOrProcessorsList);
m_ListProperty.serializedObject.ApplyModifiedProperties();
}

private static string ToSerializableString(IEnumerable<NameAndParameters> parametersForEachListItem)
{
if (parametersForEachListItem == null)
return string.Empty;

return string.Join(NamedValue.Separator,
parametersForEachListItem.Select(x => x.ToString()).ToArray());
}

public override void RedrawUI(InputActionsEditorState state)
{
if (m_ContentContainer != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,23 @@
return $"{name}({parameterString})";
}

internal static string ToSerializableString(IEnumerable<NameAndParameters> list)
{
if(list == null)
return string.Empty;

Check warning on line 33 in Packages/com.unity.inputsystem/InputSystem/Utilities/NameAndParameters.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Utilities/NameAndParameters.cs#L33

Added line #L33 was not covered by tests

return string.Join(NamedValue.Separator, list.Select(x => x.ToString()).ToArray());
}

internal static NameAndParameters Create(string name, IList<NamedValue> parameters)
{
return new NameAndParameters

Check warning on line 40 in Packages/com.unity.inputsystem/InputSystem/Utilities/NameAndParameters.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Utilities/NameAndParameters.cs#L39-L40

Added lines #L39 - L40 were not covered by tests
{
name = name,
parameters = new ReadOnlyArray<NamedValue>(parameters.ToArray())
};
}

Check warning on line 45 in Packages/com.unity.inputsystem/InputSystem/Utilities/NameAndParameters.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Utilities/NameAndParameters.cs#L45

Added line #L45 was not covered by tests

public static IEnumerable<NameAndParameters> ParseMultiple(string text)
{
List<NameAndParameters> list = null;
Expand Down