Merge branch 'develop' into use-debug-error-instead-assert-nda-packages

Author: ritamerkl

Packages/com.unity.inputsystem/CHANGELOG.md

@@ -38,6 +38,7 @@ however, it has to be formatted properly to pass verification tests.
 - Fixed an issue where batch jobs would fail with "Error: Error building Player because scripts are compiling" if a source generated .inputactions asset is out of sync with its generated source code (ISXB-1300).
 - Fixed multiple `OnScreenStick` Components that does not work together when using them simultaneously in isolation mode. [ISXB-813](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-813)
 - Fixed an issue in input actions editor window that caused certain fields in custom input composite bindings to require multiple clicks to action / focus. [ISXB-1171](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1171)
+- Fixed an editor/player hang in `InputSystemUIInputModule` due to an infinite loop. This was caused by the assumption that `RemovePointerAtIndex` would _always_ successfully remove the pointer, which is not the case with touch based pointers. [ISXB-1258](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1258)
 
 ### Changed
 - Changed location of the link xml file (code stripping rules), from a temporary directory to the project Library folder (ISX-2140).

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorState.cs

@@ -1,7 +1,6 @@
 #if UNITY_EDITOR && UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
 using System;
 using System.Collections.Generic;
-using System.Collections.ObjectModel;
 using System.Linq;
 using UnityEditor;
 
@@ -92,7 +91,6 @@ public InputActionsEditorState(
             int selectedActionIndex = 0,
             int selectedBindingIndex = 0,
             SelectionType selectionType = SelectionType.Action,
-            Dictionary<(string, string), HashSet<int>> expandedBindingIndices = null,
             InputControlScheme selectedControlScheme = default,
             int selectedControlSchemeIndex = -1,
             int selectedDeviceRequirementIndex = -1,
@@ -112,9 +110,6 @@ public InputActionsEditorState(
             m_selectedControlSchemeIndex = selectedControlSchemeIndex;
             m_selectedDeviceRequirementIndex = selectedDeviceRequirementIndex;
 
-            m_ExpandedCompositeBindings = expandedBindingIndices == null ?
-                new Dictionary<(string, string), HashSet<int>>() :
-                new Dictionary<(string, string), HashSet<int>>(expandedBindingIndices);
             m_CutElements = cutElements;
         }
 
@@ -201,12 +196,6 @@ public InputActionsEditorState(InputActionsEditorState other, SerializedObject a
                 m_ControlScheme = new InputControlScheme();
             }
 
-            // Editor may leave these as null after domain reloads, so recreate them in that case.
-            // If they exist, we attempt to just preserve the same expanded items based on name for now for simplicity.
-            m_ExpandedCompositeBindings = other.m_ExpandedCompositeBindings == null ?
-                new Dictionary<(string, string), HashSet<int>>() :
-                new Dictionary<(string, string), HashSet<int>>(other.m_ExpandedCompositeBindings);
-
             m_CutElements = other.cutElements;
         }
 
@@ -218,7 +207,6 @@ public InputActionsEditorState With(
             InputControlScheme? selectedControlScheme = null,
             int? selectedControlSchemeIndex = null,
             int? selectedDeviceRequirementIndex = null,
-            Dictionary<(string, string), HashSet<int>> expandedBindingIndices = null,
             List<CutElement> cutElements = null)
         {
             return new InputActionsEditorState(
@@ -228,7 +216,6 @@ public InputActionsEditorState With(
                 selectedActionIndex ?? this.selectedActionIndex,
                 selectedBindingIndex ?? this.selectedBindingIndex,
                 selectionType ?? this.selectionType,
-                expandedBindingIndices ?? m_ExpandedCompositeBindings,
 
                 // Control schemes
                 selectedControlScheme ?? this.selectedControlScheme,
@@ -248,7 +235,6 @@ public InputActionsEditorState ClearCutElements()
                 selectedActionIndex,
                 selectedBindingIndex,
                 selectionType,
-                m_ExpandedCompositeBindings,
                 selectedControlScheme,
                 selectedControlSchemeIndex,
                 selectedDeviceRequirementIndex,
@@ -262,39 +248,6 @@ public SerializedProperty GetActionMapByName(string actionMapName)
                 .FirstOrDefault(p => p.FindPropertyRelative(nameof(InputActionMap.m_Name)).stringValue == actionMapName);
         }
 
-        public InputActionsEditorState ExpandCompositeBinding(SerializedInputBinding binding)
-        {
-            var key = GetSelectedActionMapAndActionKey();
-
-            var expandedCompositeBindings = new Dictionary<(string, string), HashSet<int>>(m_ExpandedCompositeBindings);
-            if (!expandedCompositeBindings.TryGetValue(key, out var expandedStates))
-            {
-                expandedStates = new HashSet<int>();
-                expandedCompositeBindings.Add(key, expandedStates);
-            }
-
-            expandedStates.Add(binding.indexOfBinding);
-
-            return With(expandedBindingIndices: expandedCompositeBindings);
-        }
-
-        public InputActionsEditorState CollapseCompositeBinding(SerializedInputBinding binding)
-        {
-            var key = GetSelectedActionMapAndActionKey();
-
-            if (m_ExpandedCompositeBindings.ContainsKey(key) == false)
-                throw new InvalidOperationException("Trying to collapse a composite binding tree that was never expanded.");
-
-            // do the dance of C# immutability
-            var oldExpandedCompositeBindings = m_ExpandedCompositeBindings;
-            var expandedCompositeBindings = oldExpandedCompositeBindings.Keys.Where(dictKey => dictKey != key)
-                .ToDictionary(dictKey => dictKey, dictKey => oldExpandedCompositeBindings[dictKey]);
-            var newHashset = new HashSet<int>(m_ExpandedCompositeBindings[key].Where(index => index != binding.indexOfBinding));
-            expandedCompositeBindings.Add(key, newHashset);
-
-            return With(expandedBindingIndices: expandedCompositeBindings);
-        }
-
         public InputActionsEditorState SelectAction(string actionName)
         {
             var actionMap = GetSelectedActionMap();
@@ -419,48 +372,11 @@ public readonly List<CutElement> GetCutElements()
             return m_CutElements;
         }
 
-        public ReadOnlyCollection<int> GetOrCreateExpandedState()
-        {
-            return new ReadOnlyCollection<int>(GetOrCreateExpandedStateInternal().ToList());
-        }
-
-        private HashSet<int> GetOrCreateExpandedStateInternal()
-        {
-            var key = GetSelectedActionMapAndActionKey();
-
-            if (m_ExpandedCompositeBindings.TryGetValue(key, out var expandedStates))
-                return expandedStates;
-
-            expandedStates = new HashSet<int>();
-            m_ExpandedCompositeBindings.Add(key, expandedStates);
-            return expandedStates;
-        }
-
-        internal (string, string) GetSelectedActionMapAndActionKey()
-        {
-            var selectedActionMap = GetSelectedActionMap();
-
-            var selectedAction = selectedActionMap
-                .FindPropertyRelative(nameof(InputActionMap.m_Actions))
-                .GetArrayElementAtIndex(selectedActionIndex);
-
-            var key = (
-                selectedActionMap.FindPropertyRelative(nameof(InputActionMap.m_Name)).stringValue,
-                selectedAction.FindPropertyRelative(nameof(InputAction.m_Name)).stringValue
-            );
-            return key;
-        }
-
         private SerializedProperty GetSelectedActionMap()
         {
             return Selectors.GetActionMapAtIndex(serializedObject, selectedActionMapIndex)?.wrappedProperty;
         }
 
-        /// <summary>
-        /// Expanded states for the actions tree view. These are stored per InputActionMap
-        /// </summary>
-        private readonly Dictionary<(string, string), HashSet<int>> m_ExpandedCompositeBindings;
-
         private readonly InputControlScheme m_ControlScheme;
     }
 

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ActionsTreeView.cs

@@ -659,9 +659,12 @@ public static List<TreeViewItemData<ActionOrBindingData>> GetActionsAsTreeViewDa
 
         private static int GetIdForGuid(Guid guid, Dictionary<Guid, int> idDictionary)
         {
+            // This method is used to ensure that the same Guid always gets the same id
+            // We use getHashCode instead of a counter, as we cannot guarantee that the same Guid will always be added in the same order
+            // There is a tiny chance of a collision, but it is it does happen it will only affect the expanded state of the tree view
             if (!idDictionary.TryGetValue(guid, out var id))
             {
-                id = idDictionary.Values.Count > 0 ? idDictionary.Values.Max() + 1 : 0;
+                id = guid.GetHashCode();
                 idDictionary.Add(guid, id);
             }
             return id;

Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs

@@ -1662,9 +1662,11 @@ protected override void OnDisable()
 
         private void ResetPointers()
         {
-            var numPointers = m_PointerStates.length;
-            for (var i = 0; i < numPointers; ++i)
-                SendPointerExitEventsAndRemovePointer(0);
+            for (var i = 0; i < m_PointerStates.length; ++i)
+            {
+                if (SendPointerExitEventsAndRemovePointer(i))
+                    --i;
+            }
 
             m_CurrentPointerId = -1;
             m_CurrentPointerIndex = -1;
@@ -2021,24 +2023,26 @@ private int AllocatePointer(int pointerId, int displayIndex, int touchId, UIPoin
             return m_PointerStates.AppendWithCapacity(new PointerModel(eventData));
         }
 
-        private void SendPointerExitEventsAndRemovePointer(int index)
+        // Returns true if the pointer was successfully removed (ISXB-1258)
+        private bool SendPointerExitEventsAndRemovePointer(int index)
         {
             var eventData = m_PointerStates[index].eventData;
             if (eventData.pointerEnter != null)
                 ProcessPointerMovement(eventData, null);
 
-            RemovePointerAtIndex(index);
+            return RemovePointerAtIndex(index);
         }
 
-        private void RemovePointerAtIndex(int index)
+        private bool RemovePointerAtIndex(int index)
         {
             Debug.Assert(m_PointerStates[index].eventData.pointerEnter == null, "Pointer should have exited all objects before being removed");
 
             // We don't want to release touch pointers on the same frame they are released (unpressed). They get cleaned up one frame later in Process()
             ref var state = ref GetPointerStateForIndex(index);
             if (state.pointerType == UIPointerType.Touch && (state.leftButton.isPressed || state.leftButton.wasReleasedThisFrame))
             {
-                return;
+                // The pointer was not removed
+                return false;
             }
 
             // Retain event data so that we can reuse the event the next time we allocate a PointerModel record.
@@ -2086,6 +2090,8 @@ private void RemovePointerAtIndex(int index)
                 m_PointerStates.firstValue.eventData = eventData;
             else
                 m_PointerStates.additionalValues[m_PointerStates.length - 1].eventData = eventData;
+
+            return true;
         }
 
         // Remove any pointer that no longer has the ability to point.
@@ -2100,8 +2106,9 @@ private void PurgeStalePointers()
                      !HaveControlForDevice(device, trackedDevicePosition) &&
                      !HaveControlForDevice(device, trackedDeviceOrientation)))
                 {
-                    SendPointerExitEventsAndRemovePointer(i);
-                    --i;
+                    // Only decrement 'i' if the pointer was successfully removed
+                    if (SendPointerExitEventsAndRemovePointer(i))
+                        --i;
                 }
             }
 
@@ -2310,8 +2317,8 @@ private void FilterPointerStatesByType()
                         }
                         if (m_PointerStates[i].pointerType != UIPointerType.MouseOrPen && m_PointerStates[i].pointerType != UIPointerType.Touch || (m_PointerStates[i].pointerType == UIPointerType.Touch && !state.leftButton.isPressed && !state.leftButton.wasReleasedThisFrame))
                         {
-                            SendPointerExitEventsAndRemovePointer(i);
-                            --i;
+                            if (SendPointerExitEventsAndRemovePointer(i))
+                                --i;
                         }
                     }
                 }
@@ -2322,8 +2329,8 @@ private void FilterPointerStatesByType()
                     {
                         if (m_PointerStates[i].pointerType == UIPointerType.MouseOrPen)
                         {
-                            SendPointerExitEventsAndRemovePointer(i);
-                            --i;
+                            if (SendPointerExitEventsAndRemovePointer(i))
+                                --i;
                         }
                     }
                 }