Skip to content
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

FIX: Fixed exceptions being thrown by OnScreenStick and improved warnings related to missing UGUI dependencies for current implementations. #1948

Merged
merged 3 commits into from
Jun 18, 2024
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
7 changes: 6 additions & 1 deletion Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ however, it has to be formatted properly to pass verification tests.

## [Unreleased] - yyyy-mm-dd

### Change
- 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`.
- 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).
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
#if PACKAGE_DOCS_GENERATION || UNITY_INPUT_SYSTEM_ENABLE_UI
using System;
using UnityEngine.EventSystems;
using UnityEngine.InputSystem.Layouts;

Expand Down Expand Up @@ -45,6 +44,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
Original file line number Diff line number Diff line change
Expand Up @@ -299,5 +299,30 @@ public void Destroy()
}

private static InlinedArray<OnScreenDeviceInfo> 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.";
Copy link
Collaborator

Choose a reason for hiding this comment

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

Minor: a property would be sufficient (e.g. string warningMessage => $"{GetType()} needs t.....";)

}
}

internal static class UGUIOnScreenControlUtils
{
public static RectTransform GetCanvasRectTransform(Transform transform)
{
var parentTransform = transform.parent;
return parentTransform != null ? transform.parent.GetComponentInParent<RectTransform>() : 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
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,14 @@ private void Start()
m_PointerMoveAction.Enable();
}

// 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;

m_PointerDownPos = m_StartPos;

var dynamicOrigin = new GameObject(kDynamicOriginClickable, typeof(Image));
Expand All @@ -132,38 +137,39 @@ private void Start()

private void BeginInteraction(Vector2 pointerPosition, Camera uiCamera)
{
var canvasRect = transform.parent?.GetComponentInParent<RectTransform>();
if (canvasRect == null)
var canvasRectTransform = UGUIOnScreenControlUtils.GetCanvasRectTransform(transform);
if (canvasRectTransform == null)
{
Debug.LogError("OnScreenStick needs to be attached as a child to a UI Canvas to function properly.");
Debug.LogError(GetWarningMessage());
return;
}

switch (m_Behaviour)
{
case Behaviour.RelativePositionWithStaticOrigin:
RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRect, pointerPosition, uiCamera, out m_PointerDownPos);
RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRectTransform, pointerPosition, uiCamera, out m_PointerDownPos);
break;
case Behaviour.ExactPositionWithStaticOrigin:
RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRect, pointerPosition, uiCamera, out m_PointerDownPos);
RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRectTransform, pointerPosition, uiCamera, out m_PointerDownPos);
MoveStick(pointerPosition, uiCamera);
break;
case Behaviour.ExactPositionWithDynamicOrigin:
RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRect, pointerPosition, uiCamera, out var pointerDown);
RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRectTransform, pointerPosition, uiCamera, out var pointerDown);
m_PointerDownPos = ((RectTransform)transform).anchoredPosition = pointerDown;
break;
}
}

private void MoveStick(Vector2 pointerPosition, Camera uiCamera)
{
var canvasRect = transform.parent?.GetComponentInParent<RectTransform>();
if (canvasRect == null)
var canvasRectTransform = UGUIOnScreenControlUtils.GetCanvasRectTransform(transform);
if (canvasRectTransform == null)
{
Debug.LogError("OnScreenStick needs to be attached as a child to a UI Canvas to function properly.");
Debug.LogError(GetWarningMessage());
return;
}
RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRect, pointerPosition, uiCamera, out var position);

RectTransformUtility.ScreenPointToLocalPointInRectangle(canvasRectTransform, pointerPosition, uiCamera, out var position);
var delta = position - m_PointerDownPos;

switch (m_Behaviour)
Expand Down Expand Up @@ -253,9 +259,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;

Expand Down Expand Up @@ -457,6 +468,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);
Expand Down