Skip to content
Draft
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
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/InputSystem/InputSystem.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using UnityEngine.InputSystem.DualShock;
using UnityEngine.InputSystem.EnhancedTouch;
using UnityEngine.InputSystem.HID;
using UnityEngine.InputSystem.UI;
using UnityEngine.InputSystem.Users;
using UnityEngine.InputSystem.XInput;
using UnityEngine.InputSystem.Utilities;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,23 @@
public void OnEnable()
{
InputUser.onChange += OnUserChange;
CacheProperties();
}

private void CacheProperties()
{
m_NotificationBehaviorProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_NotificationBehavior));
m_PlayerJoinedEventProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_PlayerJoinedEvent));
m_PlayerLeftEventProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_PlayerLeftEvent));
m_JoinBehaviorProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_JoinBehavior));
m_JoinActionProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_JoinAction));
m_PlayerPrefabProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_PlayerPrefab));
m_AllowJoiningProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_AllowJoining));
m_MaxPlayerCountProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_MaxPlayerCount));
m_SplitScreenProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_SplitScreen));
m_MaintainAspectRatioProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_MaintainAspectRatioInSplitScreen));
m_FixedNumberOfSplitScreensProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_FixedNumberOfSplitScreens));
m_SplitScreenRectProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_SplitScreenRect));
}

public void OnDisable()
Expand All @@ -35,8 +52,6 @@

public override void OnInspectorGUI()
{
////TODO: cache properties

EditorGUI.BeginChangeCheck();

DoNotificationSectionUI();
Expand All @@ -54,9 +69,8 @@

private void DoNotificationSectionUI()
{
var notificationBehaviorProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_NotificationBehavior));
EditorGUILayout.PropertyField(notificationBehaviorProperty);
switch ((PlayerNotifications)notificationBehaviorProperty.intValue)
EditorGUILayout.PropertyField(m_NotificationBehaviorProperty);
switch ((PlayerNotifications)m_NotificationBehaviorProperty.intValue)

Check warning on line 73 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L72-L73

Added lines #L72 - L73 were not covered by tests
{
case PlayerNotifications.SendMessages:
if (m_SendMessagesHelpText == null)
Expand All @@ -76,11 +90,8 @@
m_EventsExpanded = EditorGUILayout.Foldout(m_EventsExpanded, m_EventsLabel, toggleOnLabelClick: true);
if (m_EventsExpanded)
{
var playerJoinedEventProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_PlayerJoinedEvent));
var playerLeftEventProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_PlayerLeftEvent));

EditorGUILayout.PropertyField(playerJoinedEventProperty);
EditorGUILayout.PropertyField(playerLeftEventProperty);
EditorGUILayout.PropertyField(m_PlayerJoinedEventProperty);
EditorGUILayout.PropertyField(m_PlayerLeftEventProperty);

Check warning on line 94 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L93-L94

Added lines #L93 - L94 were not covered by tests
}
break;
}
Expand All @@ -91,52 +102,47 @@
EditorGUILayout.LabelField(m_JoiningGroupLabel, EditorStyles.boldLabel);

// Join behavior
var joinBehaviorProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_JoinBehavior));
EditorGUILayout.PropertyField(joinBehaviorProperty);
if ((PlayerJoinBehavior)joinBehaviorProperty.intValue != PlayerJoinBehavior.JoinPlayersManually)
EditorGUILayout.PropertyField(m_JoinBehaviorProperty);
if ((PlayerJoinBehavior)m_JoinBehaviorProperty.intValue != PlayerJoinBehavior.JoinPlayersManually)

Check warning on line 106 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L105-L106

Added lines #L105 - L106 were not covered by tests
{
++EditorGUI.indentLevel;

// Join action.
if ((PlayerJoinBehavior)joinBehaviorProperty.intValue ==
if ((PlayerJoinBehavior)m_JoinBehaviorProperty.intValue ==

Check warning on line 111 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L111

Added line #L111 was not covered by tests
PlayerJoinBehavior.JoinPlayersWhenJoinActionIsTriggered)
{
var joinActionProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_JoinAction));
EditorGUILayout.PropertyField(joinActionProperty);
EditorGUILayout.PropertyField(m_JoinActionProperty);

Check warning on line 114 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L114

Added line #L114 was not covered by tests
}

// Player prefab.
var playerPrefabProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_PlayerPrefab));
EditorGUILayout.PropertyField(playerPrefabProperty);
EditorGUILayout.PropertyField(m_PlayerPrefabProperty);

Check warning on line 118 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L118

Added line #L118 was not covered by tests

ValidatePlayerPrefab(joinBehaviorProperty, playerPrefabProperty);
ValidatePlayerPrefab(m_JoinBehaviorProperty, m_PlayerPrefabProperty);

Check warning on line 120 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L120

Added line #L120 was not covered by tests

--EditorGUI.indentLevel;
}

// Enabled-by-default.
var allowJoiningProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_AllowJoining));
if (m_AllowingJoiningLabel == null)
m_AllowingJoiningLabel = new GUIContent("Joining Enabled By Default", allowJoiningProperty.GetTooltip());
EditorGUILayout.PropertyField(allowJoiningProperty, m_AllowingJoiningLabel);
m_AllowingJoiningLabel = new GUIContent("Joining Enabled By Default", m_AllowJoiningProperty.GetTooltip());
EditorGUILayout.PropertyField(m_AllowJoiningProperty, m_AllowingJoiningLabel);

Check warning on line 128 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L127-L128

Added lines #L127 - L128 were not covered by tests

// Max player count.
var maxPlayerCountProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_MaxPlayerCount));
if (m_EnableMaxPlayerCountLabel == null)
m_EnableMaxPlayerCountLabel = EditorGUIUtility.TrTextContent("Limit Number of Players", maxPlayerCountProperty.GetTooltip());
if (maxPlayerCountProperty.intValue > 0)
m_EnableMaxPlayerCountLabel = EditorGUIUtility.TrTextContent("Limit Number of Players", m_MaxPlayerCountProperty.GetTooltip());
if (m_MaxPlayerCountProperty.intValue > 0)

Check warning on line 133 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L132-L133

Added lines #L132 - L133 were not covered by tests
m_MaxPlayerCountEnabled = true;
m_MaxPlayerCountEnabled = EditorGUILayout.Toggle(m_EnableMaxPlayerCountLabel, m_MaxPlayerCountEnabled);
if (m_MaxPlayerCountEnabled)
{
++EditorGUI.indentLevel;
if (maxPlayerCountProperty.intValue < 0)
maxPlayerCountProperty.intValue = 1;
EditorGUILayout.PropertyField(maxPlayerCountProperty);
if (m_MaxPlayerCountProperty.intValue < 0)
m_MaxPlayerCountProperty.intValue = 1;
EditorGUILayout.PropertyField(m_MaxPlayerCountProperty);

Check warning on line 141 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L139-L141

Added lines #L139 - L141 were not covered by tests
--EditorGUI.indentLevel;
}
else
Copy link
Contributor

Choose a reason for hiding this comment

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

importance-medium

Assigning a value to m_MaxPlayerCountProperty.intValue inside the else block of OnInspectorGUI will result in an unconditional write to the serialized property on every GUI event (Layout, Repaint, etc.) whenever the toggle is disabled. This can cause the SerializedObject to be marked dirty unnecessarily and may interfere with the Undo system or performance in large scenes. Consider wrapping this assignment in a check to see if the value is already -1, or perform the assignment only when m_MaxPlayerCountEnabled changes.

🤖 Helpful? 👍/👎

maxPlayerCountProperty.intValue = -1;
m_MaxPlayerCountProperty.intValue = -1;

Check warning on line 145 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L145

Added line #L145 was not covered by tests
}

private static void ValidatePlayerPrefab(SerializedProperty joinBehaviorProperty,
Expand Down Expand Up @@ -174,51 +180,47 @@
EditorGUILayout.LabelField(m_SplitScreenGroupLabel, EditorStyles.boldLabel);

// Split-screen toggle.
var splitScreenProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_SplitScreen));
if (m_SplitScreenLabel == null)
m_SplitScreenLabel = new GUIContent("Enable Split-Screen", splitScreenProperty.GetTooltip());
EditorGUILayout.PropertyField(splitScreenProperty, m_SplitScreenLabel);
if (!splitScreenProperty.boolValue)
m_SplitScreenLabel = new GUIContent("Enable Split-Screen", m_SplitScreenProperty.GetTooltip());
EditorGUILayout.PropertyField(m_SplitScreenProperty, m_SplitScreenLabel);
if (!m_SplitScreenProperty.boolValue)

Check warning on line 186 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L184-L186

Added lines #L184 - L186 were not covered by tests
return;

++EditorGUI.indentLevel;

// Maintain-aspect-ratio toggle.
var maintainAspectRatioProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_MaintainAspectRatioInSplitScreen));
if (m_MaintainAspectRatioLabel == null)
m_MaintainAspectRatioLabel =
new GUIContent("Maintain Aspect Ratio", maintainAspectRatioProperty.GetTooltip());
EditorGUILayout.PropertyField(maintainAspectRatioProperty, m_MaintainAspectRatioLabel);
new GUIContent("Maintain Aspect Ratio", m_MaintainAspectRatioProperty.GetTooltip());
EditorGUILayout.PropertyField(m_MaintainAspectRatioProperty, m_MaintainAspectRatioLabel);

Check warning on line 195 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L195

Added line #L195 was not covered by tests

// Fixed-number toggle.
var fixedNumberProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_FixedNumberOfSplitScreens));
if (m_EnableFixedNumberOfSplitScreensLabel == null)
m_EnableFixedNumberOfSplitScreensLabel = EditorGUIUtility.TrTextContent("Set Fixed Number", fixedNumberProperty.GetTooltip());
if (fixedNumberProperty.intValue > 0)
m_EnableFixedNumberOfSplitScreensLabel = EditorGUIUtility.TrTextContent("Set Fixed Number", m_FixedNumberOfSplitScreensProperty.GetTooltip());
if (m_FixedNumberOfSplitScreensProperty.intValue > 0)

Check warning on line 200 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L199-L200

Added lines #L199 - L200 were not covered by tests
m_FixedNumberOfSplitScreensEnabled = true;
m_FixedNumberOfSplitScreensEnabled = EditorGUILayout.Toggle(m_EnableFixedNumberOfSplitScreensLabel,
m_FixedNumberOfSplitScreensEnabled);
if (m_FixedNumberOfSplitScreensEnabled)
{
++EditorGUI.indentLevel;
if (fixedNumberProperty.intValue < 0)
fixedNumberProperty.intValue = 4;
if (m_FixedNumberOfSplitScreensProperty.intValue < 0)
m_FixedNumberOfSplitScreensProperty.intValue = 4;

Check warning on line 208 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L207-L208

Added lines #L207 - L208 were not covered by tests
if (m_FixedNumberOfSplitScreensLabel == null)
m_FixedNumberOfSplitScreensLabel = EditorGUIUtility.TrTextContent("Number of Screens",
fixedNumberProperty.tooltip);
EditorGUILayout.PropertyField(fixedNumberProperty, m_FixedNumberOfSplitScreensLabel);
m_FixedNumberOfSplitScreensProperty.tooltip);
Copy link
Contributor

Choose a reason for hiding this comment

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

importance-low

There is an inconsistency in how tooltips are accessed. On line 199, GetTooltip() is used, while here .tooltip is accessed directly. It is generally recommended to stick to one method (likely GetTooltip() if it handles fallbacks or version compatibility) to ensure consistent behavior across the editor.

🤖 Helpful? 👍/👎

EditorGUILayout.PropertyField(m_FixedNumberOfSplitScreensProperty, m_FixedNumberOfSplitScreensLabel);

Check warning on line 212 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L212

Added line #L212 was not covered by tests
--EditorGUI.indentLevel;
}
else
{
fixedNumberProperty.intValue = -1;
m_FixedNumberOfSplitScreensProperty.intValue = -1;

Check warning on line 217 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L217

Added line #L217 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

importance-medium

Similar to the m_MaxPlayerCountProperty assignment, this unconditionally sets the property value to -1 on every repaint when m_FixedNumberOfSplitScreensEnabled is false. This should ideally be guarded by a change check or a value comparison to avoid unnecessary dirtiness of the serialized object.

🤖 Helpful? 👍/👎

}

// Split-screen area.
var splitScreenAreaProperty = serializedObject.FindProperty(nameof(PlayerInputManager.m_SplitScreenRect));
if (m_SplitScreenAreaLabel == null)
m_SplitScreenAreaLabel = new GUIContent("Screen Rectangle", splitScreenAreaProperty.GetTooltip());
EditorGUILayout.PropertyField(splitScreenAreaProperty, m_SplitScreenAreaLabel);
m_SplitScreenAreaLabel = new GUIContent("Screen Rectangle", m_SplitScreenRectProperty.GetTooltip());
EditorGUILayout.PropertyField(m_SplitScreenRectProperty, m_SplitScreenAreaLabel);

Check warning on line 223 in Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/PlayerInput/PlayerInputManagerEditor.cs#L222-L223

Added lines #L222 - L223 were not covered by tests

--EditorGUI.indentLevel;
}
Expand Down Expand Up @@ -251,6 +253,19 @@
[SerializeField] private bool m_MaxPlayerCountEnabled;
[SerializeField] private bool m_FixedNumberOfSplitScreensEnabled;

[NonSerialized] private SerializedProperty m_NotificationBehaviorProperty;
[NonSerialized] private SerializedProperty m_PlayerJoinedEventProperty;
[NonSerialized] private SerializedProperty m_PlayerLeftEventProperty;
[NonSerialized] private SerializedProperty m_JoinBehaviorProperty;
[NonSerialized] private SerializedProperty m_JoinActionProperty;
[NonSerialized] private SerializedProperty m_PlayerPrefabProperty;
[NonSerialized] private SerializedProperty m_AllowJoiningProperty;
[NonSerialized] private SerializedProperty m_MaxPlayerCountProperty;
[NonSerialized] private SerializedProperty m_SplitScreenProperty;
[NonSerialized] private SerializedProperty m_MaintainAspectRatioProperty;
[NonSerialized] private SerializedProperty m_FixedNumberOfSplitScreensProperty;
[NonSerialized] private SerializedProperty m_SplitScreenRectProperty;
Comment on lines +256 to +267
Copy link
Contributor

Choose a reason for hiding this comment

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

importance-low

The [NonSerialized] fields for SerializedProperty are correctly added to avoid serialization overhead. However, since OnEnable caches these, ensure that OnDisable or the class doesn't leak references if the SerializedObject is disposed. While typically handled by Unity's garbage collection for Editor objects, it's a good practice to null them out in OnDisable if the editor lifecycle is complex.

🤖 Helpful? 👍/👎


[NonSerialized] private readonly GUIContent m_JoiningGroupLabel = EditorGUIUtility.TrTextContent("Joining");
[NonSerialized] private readonly GUIContent m_SplitScreenGroupLabel = EditorGUIUtility.TrTextContent("Split-Screen");
[NonSerialized] private readonly GUIContent m_EventsLabel = EditorGUIUtility.TrTextContent("Events");
Expand Down
31 changes: 16 additions & 15 deletions Packages/com.unity.inputsystem/InputSystem/Plugins/UI/UISupport.cs
Original file line number Diff line number Diff line change
@@ -1,21 +1,22 @@
using UnityEngine.InputSystem;

////FIXME: This should be UnityEngine.InputSystem.UI

#if UNITY_DISABLE_DEFAULT_INPUT_PLUGIN_INITIALIZATION
public
#else
internal
#endif
static class UISupport
namespace UnityEngine.InputSystem.UI
{
public static void Initialize()
#if UNITY_DISABLE_DEFAULT_INPUT_PLUGIN_INITIALIZATION
public
#else
internal
#endif
static class UISupport
{
InputSystem.RegisterLayout(@"
{
""name"" : ""VirtualMouse"",
""extend"" : ""Mouse""
}
");
public static void Initialize()
{
InputSystem.RegisterLayout(@"
{
""name"" : ""VirtualMouse"",
""extend"" : ""Mouse""
}
");
}
}
}
Loading