Skip to content

NEW: Allow selection of InputActionReferences from project-wide actions #1779

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Nov 1, 2023
Merged
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
2 changes: 2 additions & 0 deletions Assets/Tests/InputSystem/CoreTests_ProjectWideActions.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS

using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using NUnit.Framework;
using UnityEditor;
using UnityEngine;
Expand Down
2 changes: 2 additions & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ however, it has to be formatted properly to pass verification tests.
### Added
- Support for [Game rotation vector](https://developer.android.com/reference/android/hardware/Sensor#TYPE_GAME_ROTATION_VECTOR) sensor on Android
- Duplicate Input Action Items in the new Input Action Asset Editor with Ctrl+D (Windows) or Cmd+D (Mac)
- Selection of InputActionReferences from project-wide actions on fields that are of type InputActionReference. Uses a new advanced object picker that allows better searching and filtering of actions.

### Fixed
- Partially fixed case ISX-1357 (Investigate performance regressing over time). A sample showed that leaving an InputActionMap enabled could lead to an internal list of listeners growing. This leads to slow-down, so we now warn if we think this is happening.
Expand All @@ -38,6 +39,7 @@ however, it has to be formatted properly to pass verification tests.
- Fixed case [ISX-1668] (The Profiler shows incorrect data and spams the console with "Missing Profiler.EndSample" errors when there is an Input System Component in Scene).
- Fixed [ISX-1661](https://jira.unity3d.com/browse/ISX-1661) where undoing duplications of action maps caused console errors
- Fix for BindingSyntax `WithInteraction()` which was incorrectly using processors.
- Fixed issue of visual elements being null during editing project-wide actions in project settings which prompted console errors.


## [1.8.0-pre.1] - 2023-09-04
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ public override string ToString()
return base.ToString();
}

private static string GetDisplayName(InputAction action)
internal static string GetDisplayName(InputAction action)
{
return !string.IsNullOrEmpty(action?.actionMap?.name) ? $"{action.actionMap?.name}/{action.name}" : action?.name;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#if UNITY_EDITOR
using UnityEditor;

namespace UnityEngine.InputSystem.Editor
{
/// <summary>
/// Provides access to icons associated with <see cref="InputActionAsset"/> and <see cref="InputActionReference"/>.
/// </summary>
internal static class InputActionAssetIconLoader
{
private const string kActionIcon = "Packages/com.unity.inputsystem/InputSystem/Editor/Icons/InputAction.png";
private const string kAssetIcon = "Packages/com.unity.inputsystem/InputSystem/Editor/Icons/InputActionAsset.png";

/// <summary>
/// Attempts to load the icon associated with an <see cref="InputActionAsset"/>.
/// </summary>
/// <returns>Icon resource reference or <code>null</code> if the resource could not be loaded.</returns>
internal static Texture2D LoadAssetIcon()
{
return (Texture2D)EditorGUIUtility.Load(kAssetIcon);
}

/// <summary>
/// Attempts to load the icon associated with an <see cref="InputActionReference"/> sub-asset of an
/// <see cref="InputActionAsset"/>.
/// </summary>
/// <returns>Icon resource reference or <code>null</code> if the resource could not be loaded.</returns>
internal static Texture2D LoadActionIcon()
{
return (Texture2D)EditorGUIUtility.Load(kActionIcon);
}
}
}

#endif // #if UNITY_EDITOR

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#if UNITY_EDITOR
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using UnityEditor;
Expand Down Expand Up @@ -29,9 +30,6 @@ internal class InputActionImporter : ScriptedImporter
{
private const int kVersion = 13;

private const string kActionIcon = "Packages/com.unity.inputsystem/InputSystem/Editor/Icons/InputAction.png";
private const string kAssetIcon = "Packages/com.unity.inputsystem/InputSystem/Editor/Icons/InputActionAsset.png";

[SerializeField] private bool m_GenerateWrapperCode;
[SerializeField] private string m_WrapperCodePath;
[SerializeField] private string m_WrapperClassName;
Expand Down Expand Up @@ -88,8 +86,8 @@ public override void OnImportAsset(AssetImportContext ctx)

// Load icons.
////REVIEW: the icons won't change if the user changes skin; not sure it makes sense to differentiate here
var assetIcon = (Texture2D)EditorGUIUtility.Load(kAssetIcon);
var actionIcon = (Texture2D)EditorGUIUtility.Load(kActionIcon);
var assetIcon = InputActionAssetIconLoader.LoadAssetIcon();
var actionIcon = InputActionAssetIconLoader.LoadActionIcon();

// Add asset.
ctx.AddObjectToAsset("<root>", asset, assetIcon);
Expand Down Expand Up @@ -212,12 +210,51 @@ public override void OnImportAsset(AssetImportContext ctx)
InputActionEditorWindow.RefreshAllOnAssetReimport();
}

internal static IEnumerable<InputActionReference> LoadInputActionReferencesFromAsset(InputActionAsset asset)
{
//Get all InputActionReferences are stored at the same asset path as InputActionAsset
return AssetDatabase.LoadAllAssetsAtPath(AssetDatabase.GetAssetPath(asset)).Where(
o => o is InputActionReference && o.name != "InputManager").Cast<InputActionReference>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this part?
&& o.name != "InputManager

InputManager data shouldn't be an InputActionReference should it ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You're right, in theory it shouldn't.
But when removing an input action reference from the asset, it seems to show InputManage in the picker window. This was a just a quick fix. Something to improve, depending on the time we have to understand what's happening in InputManager.asset.

}

// Get all InputActionReferences from assets in the project. By default it only gets the assets in the "Assets" folder.
internal static IEnumerable<InputActionReference> LoadInputActionReferencesFromAssetDatabase(string[] foldersPath = null)
{
string[] searchFolders = null;
// If folderPath is null, search in "Assets" folder.
if (foldersPath == null)
{
searchFolders = new string[] { "Assets" };
}

// Get all InputActionReference from assets in "Asset" folder. It does not search inside "Packages" folder.
var inputActionReferenceGUIDs = AssetDatabase.FindAssets($"t:{typeof(InputActionReference).Name}", searchFolders);

// To find all the InputActionReferences, the GUID of the asset containing at least one action reference is
// used to find the asset path. This is because InputActionReferences are stored in the asset database as sub-assets of InputActionAsset.
// Then the whole asset is loaded and all the InputActionReferences are extracted from it.
// Also, the action references are duplicated to have backwards compatibility with the 1.0.0-preview.7. That
// is why we look for references withouth the `HideFlags.HideInHierarchy` flag.
var inputActionReferencesList = new List<InputActionReference>();
foreach (var guid in inputActionReferenceGUIDs)
{
var assetPath = AssetDatabase.GUIDToAssetPath(guid);
var assetInputActionReferenceList = AssetDatabase.LoadAllAssetsAtPath(assetPath).Where(
o => o is InputActionReference &&
!((InputActionReference)o).hideFlags.HasFlag(HideFlags.HideInHierarchy))
.Cast<InputActionReference>().ToList();

inputActionReferencesList.AddRange(assetInputActionReferenceList);
}
return inputActionReferencesList;
}

// Add item to plop an .inputactions asset into the project.
[MenuItem("Assets/Create/Input Actions")]
public static void CreateInputAsset()
{
ProjectWindowUtil.CreateAssetWithContent("New Controls." + InputActionAsset.Extension,
InputActionAsset.kDefaultAssetLayoutJson, (Texture2D)EditorGUIUtility.Load(kAssetIcon));
InputActionAsset.kDefaultAssetLayoutJson, InputActionAssetIconLoader.LoadAssetIcon());
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,8 +86,16 @@ private static InputActionAsset CreateNewActionAsset()
}
}

// Create sub-asset for each action. This is so that users can select individual input actions from the asset when they're
// trying to assign to a field that accepts only one action.
CreateInputActionReferences(asset);

AssetDatabase.SaveAssets();

return asset;
}

private static void CreateInputActionReferences(InputActionAsset asset)
{
var maps = asset.actionMaps;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need a check here to make sure the asset doesn't already have the InputActionReferences before they are created?

var actionReference = ScriptableObject.CreateInstance();

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess not as its only called from CreateNewActionAsset

foreach (var map in maps)
{
foreach (var action in map.actions)
Expand All @@ -97,10 +105,51 @@ private static InputActionAsset CreateNewActionAsset()
AssetDatabase.AddObjectToAsset(actionReference, asset);
}
}
}

AssetDatabase.SaveAssets();
/// <summary>
/// Updates the input action references in the asset by updating names, removing dangling references
/// and adding new ones.
/// </summary>
/// <param name="asset"></param>
internal static void UpdateInputActionReferences()
{
var asset = GetOrCreate();
var existingReferences = InputActionImporter.LoadInputActionReferencesFromAsset(asset).ToList();

return asset;
// Check if referenced input action exists in the asset and remove the reference if it doesn't.
foreach (var actionReference in existingReferences)
{
var action = asset.FindAction(actionReference.action.id);
Copy link
Collaborator

Choose a reason for hiding this comment

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

When testing the branch I have a NullReferenceException here in PlayMode after some edits and entering PlayMode (Not sure about exact steps to reproduce, will update his comment if I can reproduce again):

NullReferenceException: Object reference not set to an instance of an object
UnityEngine.InputSystem.Editor.ProjectWideActionsAsset.UpdateInputActionReferences () (at ./Packages/com.unity.inputsystem/InputSystem/Editor/ProjectWideActions/ProjectWideActionsAsset.cs:123)
UnityEngine.InputSystem.Editor.InputActionsEditorWindowUtils.SaveAsset (UnityEditor.SerializedObject serializedAsset) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindowUtils.cs:21)
UnityEngine.InputSystem.Editor.InputActionsEditorSettingsProvider.OnEditFocusLost (UnityEngine.UIElements.FocusOutEvent event) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorSettingsProvider.cs:79)
UnityEngine.InputSystem.Editor.InputActionsEditorSettingsProvider.OnDeactivate () (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorSettingsProvider.cs:54)
UnityEditor.SettingsWindow.OnDisable () (at /Users/bokken/build/output/unity/unity/Editor/Mono/Settings/SettingsWindow.cs:130)
UnityEditor.ContainerWindow:InternalCloseWindow() (at /Users/bokken/build/output/unity/unity/Editor/Mono/ContainerWindow.cs:360)

Copy link
Collaborator Author

@jfreire-unity jfreire-unity Oct 30, 2023

Choose a reason for hiding this comment

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

Hm, interesting. Good catch! Was this when doing a bunch of Undo/Redo actions? I'll try to replicate it as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure, my main test case have been creating, deleting, undo, redo, renaming, select again in picker, repeat.... Haven't found this particular one again though.

if (action == null)
{
actionReference.Set(null);
AssetDatabase.RemoveObjectFromAsset(actionReference);
}
}

// Check if all actions have a reference
foreach (var action in asset)
{
var actionReference = existingReferences.FirstOrDefault(r => r.m_ActionId == action.id.ToString());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Feels like we could do this faster, e.g. by generating a hash in the above foreach look and then checking if its in the hash here - rather than splining the eixtingReferences list on every action in the asset.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks. We probably can. I'll leave it for the end in case the direction in this PR is accepted.
Are you suggesting to create some kind of dictionary to associate the action ID hash of the reference with the reference itself ? So that we can then use the action.id here as a key ?

// The input action doesn't have a reference, create a new one.
if (actionReference == null)
{
var actionReferenceNew = ScriptableObject.CreateInstance<InputActionReference>();
actionReferenceNew.Set(action);
AssetDatabase.AddObjectToAsset(actionReferenceNew, asset);
}
else
{
// Update the name of the reference if it doesn't match the action name.
if (actionReference.name != InputActionReference.GetDisplayName(action))
{
AssetDatabase.RemoveObjectFromAsset(actionReference);
actionReference.name = InputActionReference.GetDisplayName(action);
AssetDatabase.AddObjectToAsset(actionReference, asset);
}
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Note: If not UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS we do not use a custom property drawer and
// picker for InputActionReferences but rather rely on default (classic) object picker.
#if UNITY_EDITOR && UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS

using UnityEditor;
using UnityEditor.Search;

namespace UnityEngine.InputSystem.Editor
{
/// <summary>
/// Custom property drawer in order to use the "Advanced Picker" from UnityEditor.Search.
/// </summary>
[CustomPropertyDrawer(typeof(InputActionReference))]
internal sealed class InputActionReferencePropertyDrawer : PropertyDrawer
{
private readonly SearchContext m_Context = UnityEditor.Search.SearchService.CreateContext(new[]
{
InputActionReferenceSearchProviders.CreateInputActionReferenceSearchProviderForAssets(),
InputActionReferenceSearchProviders.CreateInputActionReferenceSearchProviderForProjectWideActions(),
}, string.Empty, SearchConstants.PickerSearchFlags);


public override void OnGUI(Rect position, SerializedProperty property, GUIContent label)
{
// Sets the property to null if the action is not found in the asset.
ValidatePropertyWithDanglingInputActionReferences(property);

ObjectField.DoObjectField(position, property, typeof(InputActionReference), label,
m_Context, SearchConstants.PickerViewFlags);
}

static void ValidatePropertyWithDanglingInputActionReferences(SerializedProperty property)
{
if (property?.objectReferenceValue is InputActionReference reference)
{
// Check only if the reference is a project-wide action.
if (reference?.asset?.name == ProjectWideActionsAsset.kAssetName)
{
var action = reference?.asset?.FindAction(reference.action.id);
if (action is null)
{
property.objectReferenceValue = null;
property.serializedObject.ApplyModifiedProperties();
}
}
}
}
}
}

#endif

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
#if UNITY_EDITOR && UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
using System;
using System.Collections.Generic;
using UnityEditor;
using UnityEditor.Search;
using UnityEngine.Search;

namespace UnityEngine.InputSystem.Editor
{
internal static class SearchConstants
{
// SearchFlags: these flags are used to customize how search is performed and how search
// results are displayed in the advanced object picker.
// Note: SearchFlags.Packages is not currently used and hides all results from packages.
internal static readonly SearchFlags PickerSearchFlags = SearchFlags.Sorted | SearchFlags.OpenPicker;

// Search.SearchViewFlags : these flags are used to customize the appearance of the PickerWindow.
internal static readonly Search.SearchViewFlags PickerViewFlags = SearchViewFlags.DisableBuilderModeToggle
| SearchViewFlags.DisableInspectorPreview
| SearchViewFlags.ListView
| SearchViewFlags.DisableSavedSearchQuery;
}

internal static class InputActionReferenceSearchProviders
{
const string k_AssetFolderSearchProviderId = "AssetsInputActionReferenceSearchProvider";
const string k_ProjectWideActionsSearchProviderId = "ProjectWideInputActionReferenceSearchProvider";

// Search provider for InputActionReferences for all assets in the project, without project-wide actions.
internal static SearchProvider CreateInputActionReferenceSearchProviderForAssets()
{
return CreateInputActionReferenceSearchProvider(k_AssetFolderSearchProviderId,
"Asset Input Actions",
// Show the asset path in the description.
(obj) => AssetDatabase.GetAssetPath((obj as InputActionReference).asset),
() => InputActionImporter.LoadInputActionReferencesFromAssetDatabase());
}

// Search provider for InputActionReferences for project-wide actions
internal static SearchProvider CreateInputActionReferenceSearchProviderForProjectWideActions()
{
return CreateInputActionReferenceSearchProvider(k_ProjectWideActionsSearchProviderId,
"Project-Wide Input Actions",
(obj) => "(Project-Wide Input Actions)",
() => InputActionImporter.LoadInputActionReferencesFromAsset(ProjectWideActionsAsset.GetOrCreate()));
}

private static SearchProvider CreateInputActionReferenceSearchProvider(string id, string displayName,
Func<Object, string> createItemFetchDescription, Func<IEnumerable<Object>> fetchAssets)
{
// Match icon used for sub-assets from importer for InputActionReferences.
// We assign description+label in FilteredSearch but also provide a fetchDescription+fetchLabel below.
// This is needed to support all zoom-modes for an unknown reason.
// Also, fetchLabel/fetchDescription and what is provided to CreateItem is playing different
// roles at different zoom levels.
var inputActionReferenceIcon = InputActionAssetIconLoader.LoadActionIcon();

return new SearchProvider(id, displayName)
{
priority = 25,
fetchDescription = FetchLabel,
fetchItems = (context, items, provider) => FilteredSearch(context, provider, FetchLabel, createItemFetchDescription,
fetchAssets, "(Project-Wide Input Actions)"),
fetchLabel = FetchLabel,
fetchPreview = (item, context, size, options) => inputActionReferenceIcon,
fetchThumbnail = (item, context) => inputActionReferenceIcon,
toObject = (item, type) => item.data as Object,
};
}

// Custom search function with label matching filtering.
private static IEnumerable<SearchItem> FilteredSearch(SearchContext context, SearchProvider provider,
Func<Object, string> fetchObjectLabel, Func<Object, string> createItemFetchDescription, Func<IEnumerable<Object>> fetchAssets, string description)
{
foreach (var asset in fetchAssets())
{
var label = fetchObjectLabel(asset);
if (!label.Contains(context.searchText, System.StringComparison.InvariantCultureIgnoreCase))
continue; // Ignore due to filtering
yield return provider.CreateItem(context, asset.GetInstanceID().ToString(), label, createItemFetchDescription(asset),
null, asset);
}
}

// Note that this is overloaded to allow utilizing FetchLabel inside fetchItems to keep label formatting
// consistent between CreateItem and additional fetchLabel calls.
private static string FetchLabel(Object obj)
{
return obj.name;
}

private static string FetchLabel(SearchItem item, SearchContext context)
{
return FetchLabel((item.data as Object) !);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This line doesn't compile.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could you clarify in what circumstances (editor version, OS, etc) ? I haven't seen any compile errors on my side and others, or CI.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry this was my mistake, I was trying to enable global actions on an earlier Unity Editor version that isn't supported.

}
}
}
#endif

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public override void OnActivate(string searchContext, VisualElement rootElement)
// Note that focused element will be set if we are navigating back to
// an existing instance when switching setting in the left project settings panel since
// this doesn't recreate the editor.
if (m_RootVisualElement.focusController.focusedElement != null)
if (m_RootVisualElement?.focusController?.focusedElement != null)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I noticed you picked up this diff from the original branch, just wanted to clarify this is a general bug fix relating to a post around potential focus controller bug and isn't really related to the subject of the PR, I think it is fine to add anyway but might require its own FIX line in the CHANGELOG / commit message.

OnEditFocus(null);
}

Expand Down
Loading