From 90b189bb05a29f11ee976924360c7fd758dff3fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Thu, 13 Jun 2024 15:57:17 +0200 Subject: [PATCH 1/3] FIX: Fixed exceptions being thrown by OnScreenStick and improved warnings related to missing UGUI dependencies for current implementations. --- Packages/com.unity.inputsystem/CHANGELOG.md | 7 ++- .../Plugins/OnScreen/OnScreenButton.cs | 33 ++++++++++- .../Plugins/OnScreen/OnScreenControl.cs | 25 +++++++++ .../Plugins/OnScreen/OnScreenStick.cs | 55 +++++++++++++------ 4 files changed, 101 insertions(+), 19 deletions(-) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 4dc3ca5256..1c8474fd5c 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -10,6 +10,9 @@ however, it has to be formatted properly to pass verification tests. ## [Unreleased] - yyyy-mm-dd +### Change +- Added warning messages to `OnScreenStick` and `OnScreenButton` Inspector editors that would display a warning message in case on-screen control components are added to a `GameObject` not part of a valid UI hierarchy. + ### Fixed - Avoid potential crashes from `NullReferenceException` in `FireStateChangeNotifications`. - Fixed an issue where a composite binding would not be consecutively triggered after ResetDevice() has been called from the associated action handler [ISXB-746](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-746). @@ -19,8 +22,10 @@ however, it has to be formatted properly to pass verification tests. - Fixed error thrown when Cancelling Control Scheme creation in Input Actions Editor. - Fixed Scheme Name in Control Scheme editor menu that gets reset when editing devices [ISXB-763](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-763). - Fixed an issue where `InputActionAsset.FindAction(string, bool)` would throw `System.NullReferenceException` instead of returning `null` if searching for a non-existent action with an explicit action path and using `throwIfNotFound: false`, e.g. searching for "Map/Action" when `InputActionMap` "Map" exists but no `InputAction` named "Action" exists within that map [ISXB-895](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-895). +- Fixed an issue where adding a `OnScreenButton` or `OnScreenStick` to a regular GameObject would lead to exception in editor. +- Fixed an issue where adding a `OnScreenStick` to a regular GameObject and entering play-mode would lead to exceptions being generated. -## Added +### Added - Added additional device information when logging the error due to exceeding the maximum number of events processed set by `InputSystem.settings.maxEventsBytesPerUpdate`. This additional information is available in development builds only. diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenButton.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenButton.cs index e20bbf7256..c947f8f408 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenButton.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenButton.cs @@ -1,5 +1,4 @@ #if PACKAGE_DOCS_GENERATION || UNITY_INPUT_SYSTEM_ENABLE_UI -using System; using UnityEngine.EventSystems; using UnityEngine.InputSystem.Layouts; @@ -15,6 +14,15 @@ namespace UnityEngine.InputSystem.OnScreen [HelpURL(InputSystem.kDocUrl + "/manual/OnScreen.html#on-screen-buttons")] public class OnScreenButton : OnScreenControl, IPointerDownHandler, IPointerUpHandler { + protected override void OnEnable() + { + base.OnEnable(); + + // Current implementation has UGUI dependencies (ISXB-915, ISXB-916) + if (UGUIOnScreenControlUtils.GetCanvasRectTransform(transform) == null) + Debug.LogWarning(GetWarningMessage()); + } + public void OnPointerUp(PointerEventData eventData) { SendValueToControl(0.0f); @@ -45,6 +53,29 @@ protected override string controlPathInternal get => m_ControlPath; set => m_ControlPath = value; } + +#if UNITY_EDITOR + [UnityEditor.CustomEditor(typeof(OnScreenButton))] + internal class OnScreenButtonEditor : UnityEditor.Editor + { + private UnityEditor.SerializedProperty m_ControlPathInternal; + + public void OnEnable() + { + m_ControlPathInternal = serializedObject.FindProperty(nameof(OnScreenButton.m_ControlPath)); + } + + public override void OnInspectorGUI() + { + // Current implementation has UGUI dependencies (ISXB-915, ISXB-916) + UGUIOnScreenControlEditorUtils.ShowWarningIfNotPartOfCanvasHierarchy((OnScreenButton)target); + + UnityEditor.EditorGUILayout.PropertyField(m_ControlPathInternal); + + serializedObject.ApplyModifiedProperties(); + } + } +#endif } } #endif diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenControl.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenControl.cs index 85f9756ef1..d1fedda694 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenControl.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenControl.cs @@ -299,5 +299,30 @@ public void Destroy() } private static InlinedArray s_OnScreenDevices; + + internal string GetWarningMessage() + { + return $"{GetType()} needs to be attached as a child to a UI Canvas and have a RectTransform component to function properly."; + } + } + + internal static class UGUIOnScreenControlUtils + { + public static RectTransform GetCanvasRectTransform(Transform transform) + { + var parentTransform = transform.parent; + return parentTransform != null ? transform.parent.GetComponentInParent() : null; + } + } + +#if UNITY_EDITOR + internal static class UGUIOnScreenControlEditorUtils + { + public static void ShowWarningIfNotPartOfCanvasHierarchy(OnScreenControl target) + { + if (UGUIOnScreenControlUtils.GetCanvasRectTransform(target.transform) == null) + UnityEditor.EditorGUILayout.HelpBox(target.GetWarningMessage(), UnityEditor.MessageType.Warning); + } } +#endif } diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs index 978931a15a..6c7c7adf6f 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs @@ -36,6 +36,8 @@ public class OnScreenStick : OnScreenControl, IPointerDownHandler, IPointerUpHan { private const string kDynamicOriginClickable = "DynamicOriginClickable"; + private RectTransform m_CanvasRectTransform; + /// /// Callback to handle OnPointerDown UI events. /// @@ -112,9 +114,15 @@ private void Start() m_PointerMoveAction.Enable(); } + // Unable to setup elements according to settings if a RectTransform is not available. + if (!(transform is RectTransform)) + return; + m_StartPos = ((RectTransform)transform).anchoredPosition; - if (m_Behaviour != Behaviour.ExactPositionWithDynamicOrigin) return; + if (m_Behaviour != Behaviour.ExactPositionWithDynamicOrigin) + return; + m_PointerDownPos = m_StartPos; var dynamicOrigin = new GameObject(kDynamicOriginClickable, typeof(Image)); @@ -130,26 +138,32 @@ private void Start() image.alphaHitTestMinimumThreshold = 0.5f; } + protected override void OnEnable() + { + base.OnEnable(); + + // Current implementation has UGUI dependencies (ISXB-915, ISXB-916) + m_CanvasRectTransform = UGUIOnScreenControlUtils.GetCanvasRectTransform(transform); + if (m_CanvasRectTransform == null) + Debug.LogWarning(GetWarningMessage()); + } + private void BeginInteraction(Vector2 pointerPosition, Camera uiCamera) { - var canvasRect = transform.parent?.GetComponentInParent(); - if (canvasRect == null) - { - Debug.LogError("OnScreenStick needs to be attached as a child to a UI Canvas to function properly."); + if (m_CanvasRectTransform == null) return; - } switch (m_Behaviour) { case Behaviour.RelativePositionWithStaticOrigin: - RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRect, pointerPosition, uiCamera, out m_PointerDownPos); + RectTransformUtility.ScreenPointToLocalPointInRectangle(m_CanvasRectTransform, pointerPosition, uiCamera, out m_PointerDownPos); break; case Behaviour.ExactPositionWithStaticOrigin: - RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRect, pointerPosition, uiCamera, out m_PointerDownPos); + RectTransformUtility.ScreenPointToLocalPointInRectangle(m_CanvasRectTransform, pointerPosition, uiCamera, out m_PointerDownPos); MoveStick(pointerPosition, uiCamera); break; case Behaviour.ExactPositionWithDynamicOrigin: - RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRect, pointerPosition, uiCamera, out var pointerDown); + RectTransformUtility.ScreenPointToLocalPointInRectangle(m_CanvasRectTransform, pointerPosition, uiCamera, out var pointerDown); m_PointerDownPos = ((RectTransform)transform).anchoredPosition = pointerDown; break; } @@ -157,13 +171,10 @@ private void BeginInteraction(Vector2 pointerPosition, Camera uiCamera) private void MoveStick(Vector2 pointerPosition, Camera uiCamera) { - var canvasRect = transform.parent?.GetComponentInParent(); - if (canvasRect == null) - { - Debug.LogError("OnScreenStick needs to be attached as a child to a UI Canvas to function properly."); + if (m_CanvasRectTransform == null) return; - } - RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRect, pointerPosition, uiCamera, out var position); + + RectTransformUtility.ScreenPointToLocalPointInRectangle(m_CanvasRectTransform, pointerPosition, uiCamera, out var position); var delta = position - m_PointerDownPos; switch (m_Behaviour) @@ -253,9 +264,14 @@ private Camera GetCameraFromCanvas() private void OnDrawGizmosSelected() { - Gizmos.matrix = ((RectTransform)transform.parent).localToWorldMatrix; + // This will not produce meaningful results unless we have a rect transform (ISXB-915, ISXB-916). + var parentRectTransform = transform.parent as RectTransform; + if (parentRectTransform == null) + return; + + Gizmos.matrix = parentRectTransform.localToWorldMatrix; - var startPos = ((RectTransform)transform).anchoredPosition; + var startPos = parentRectTransform.anchoredPosition; if (Application.isPlaying) startPos = m_StartPos; @@ -429,6 +445,8 @@ public enum Behaviour [CustomEditor(typeof(OnScreenStick))] internal class OnScreenStickEditor : UnityEditor.Editor { + private const string kWarningMessage = "OnScreenStick needs to be attached as a child to a UI Canvas and have a RectTransform component to function properly."; + private AnimBool m_ShowDynamicOriginOptions; private AnimBool m_ShowIsolatedInputActions; @@ -457,6 +475,9 @@ public void OnEnable() public override void OnInspectorGUI() { + // Current implementation has UGUI dependencies (ISXB-915, ISXB-916) + UGUIOnScreenControlEditorUtils.ShowWarningIfNotPartOfCanvasHierarchy((OnScreenStick)target); + EditorGUILayout.PropertyField(m_MovementRange); EditorGUILayout.PropertyField(m_ControlPathInternal); EditorGUILayout.PropertyField(m_Behaviour); From 9fb7cbd3c5d968ba7be367aa8e29393a80f28e17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Thu, 13 Jun 2024 22:11:18 +0200 Subject: [PATCH 2/3] Simplified fix and removed incorrect caching of canvas rect transform. --- Packages/com.unity.inputsystem/CHANGELOG.md | 2 +- .../Plugins/OnScreen/OnScreenButton.cs | 9 ----- .../Plugins/OnScreen/OnScreenStick.cs | 37 ++++++++----------- 3 files changed, 17 insertions(+), 31 deletions(-) diff --git a/Packages/com.unity.inputsystem/CHANGELOG.md b/Packages/com.unity.inputsystem/CHANGELOG.md index 1c8474fd5c..20084b07dc 100644 --- a/Packages/com.unity.inputsystem/CHANGELOG.md +++ b/Packages/com.unity.inputsystem/CHANGELOG.md @@ -11,7 +11,7 @@ however, it has to be formatted properly to pass verification tests. ## [Unreleased] - yyyy-mm-dd ### Change -- Added warning messages to `OnScreenStick` and `OnScreenButton` Inspector editors that would display a warning message in case on-screen control components are added to a `GameObject` not part of a valid UI hierarchy. +- Added warning messages to both `OnScreenStick` and `OnScreenButton` Inspector editors that would display a warning message in case on-screen control components are added to a `GameObject` not part of a valid UI hierarchy. ### Fixed - Avoid potential crashes from `NullReferenceException` in `FireStateChangeNotifications`. diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenButton.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenButton.cs index c947f8f408..1afd7125eb 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenButton.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenButton.cs @@ -14,15 +14,6 @@ namespace UnityEngine.InputSystem.OnScreen [HelpURL(InputSystem.kDocUrl + "/manual/OnScreen.html#on-screen-buttons")] public class OnScreenButton : OnScreenControl, IPointerDownHandler, IPointerUpHandler { - protected override void OnEnable() - { - base.OnEnable(); - - // Current implementation has UGUI dependencies (ISXB-915, ISXB-916) - if (UGUIOnScreenControlUtils.GetCanvasRectTransform(transform) == null) - Debug.LogWarning(GetWarningMessage()); - } - public void OnPointerUp(PointerEventData eventData) { SendValueToControl(0.0f); diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs index 6c7c7adf6f..cea321c9ec 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs @@ -36,8 +36,6 @@ public class OnScreenStick : OnScreenControl, IPointerDownHandler, IPointerUpHan { private const string kDynamicOriginClickable = "DynamicOriginClickable"; - private RectTransform m_CanvasRectTransform; - /// /// Callback to handle OnPointerDown UI events. /// @@ -114,14 +112,13 @@ private void Start() m_PointerMoveAction.Enable(); } - // Unable to setup elements according to settings if a RectTransform is not available. + // Unable to setup elements according to settings if a RectTransform is not available (ISXB-915, ISXB-916). if (!(transform is RectTransform)) return; m_StartPos = ((RectTransform)transform).anchoredPosition; - if (m_Behaviour != Behaviour.ExactPositionWithDynamicOrigin) - return; + if (m_Behaviour != Behaviour.ExactPositionWithDynamicOrigin) return; m_PointerDownPos = m_StartPos; @@ -138,32 +135,26 @@ private void Start() image.alphaHitTestMinimumThreshold = 0.5f; } - protected override void OnEnable() - { - base.OnEnable(); - - // Current implementation has UGUI dependencies (ISXB-915, ISXB-916) - m_CanvasRectTransform = UGUIOnScreenControlUtils.GetCanvasRectTransform(transform); - if (m_CanvasRectTransform == null) - Debug.LogWarning(GetWarningMessage()); - } - private void BeginInteraction(Vector2 pointerPosition, Camera uiCamera) { - if (m_CanvasRectTransform == null) + var canvasRectTransform = UGUIOnScreenControlUtils.GetCanvasRectTransform(transform); + if (canvasRectTransform == null) + { + Debug.LogError(GetWarningMessage()); return; + } switch (m_Behaviour) { case Behaviour.RelativePositionWithStaticOrigin: - RectTransformUtility.ScreenPointToLocalPointInRectangle(m_CanvasRectTransform, pointerPosition, uiCamera, out m_PointerDownPos); + RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRectTransform, pointerPosition, uiCamera, out m_PointerDownPos); break; case Behaviour.ExactPositionWithStaticOrigin: - RectTransformUtility.ScreenPointToLocalPointInRectangle(m_CanvasRectTransform, pointerPosition, uiCamera, out m_PointerDownPos); + RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRectTransform, pointerPosition, uiCamera, out m_PointerDownPos); MoveStick(pointerPosition, uiCamera); break; case Behaviour.ExactPositionWithDynamicOrigin: - RectTransformUtility.ScreenPointToLocalPointInRectangle(m_CanvasRectTransform, pointerPosition, uiCamera, out var pointerDown); + RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRectTransform, pointerPosition, uiCamera, out var pointerDown); m_PointerDownPos = ((RectTransform)transform).anchoredPosition = pointerDown; break; } @@ -171,10 +162,14 @@ private void BeginInteraction(Vector2 pointerPosition, Camera uiCamera) private void MoveStick(Vector2 pointerPosition, Camera uiCamera) { - if (m_CanvasRectTransform == null) + var canvasRectTransform = UGUIOnScreenControlUtils.GetCanvasRectTransform(transform); + if (canvasRectTransform == null) + { + Debug.LogError(GetWarningMessage()); return; + } - RectTransformUtility.ScreenPointToLocalPointInRectangle(m_CanvasRectTransform, pointerPosition, uiCamera, out var position); + RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRectTransform, pointerPosition, uiCamera, out var position); var delta = position - m_PointerDownPos; switch (m_Behaviour) From aee01e0f82d8e66adccb2fb9526a90067e9ab88a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ha=CC=8Akan=20Sidenvall?= Date: Thu, 13 Jun 2024 22:26:13 +0200 Subject: [PATCH 3/3] Removed obsolete string constant --- .../InputSystem/Plugins/OnScreen/OnScreenStick.cs | 2 -- 1 file changed, 2 deletions(-) diff --git a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs index cea321c9ec..0e5bae8514 100644 --- a/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs +++ b/Packages/com.unity.inputsystem/InputSystem/Plugins/OnScreen/OnScreenStick.cs @@ -440,8 +440,6 @@ public enum Behaviour [CustomEditor(typeof(OnScreenStick))] internal class OnScreenStickEditor : UnityEditor.Editor { - private const string kWarningMessage = "OnScreenStick needs to be attached as a child to a UI Canvas and have a RectTransform component to function properly."; - private AnimBool m_ShowDynamicOriginOptions; private AnimBool m_ShowIsolatedInputActions;