Skip to content

Commit 8e248a9

Browse files
Copilotjosepmariapujol-unity
Copilot
and
josepmariapujol-unity
authored
Address PR review comments: enum, private fields, deprecated API, rename, etc.
Agent-Logs-Url: https://github.com/Unity-Technologies/InputSystem/sessions/277fda1c-71db-41b9-8d8c-a918b3c36b9b Co-authored-by: josepmariapujol-unity <59828124+josepmariapujol-unity@users.noreply.github.com>
1 parent 3b15c1e commit 8e248a9

9 files changed

Lines changed: 110 additions & 73 deletions

File tree

‎Packages/com.unity.inputsystem/InputSystem/Editor/InputSystemEditorInitializer.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -292,10 +292,11 @@ private static void InitializeEditorHooks()
292292
private static void RegisterPlayModeHooks()
293293
{
294294
InputSystem.s_ShouldEnableActions = ShouldEnableActions;
295-
InputSystem.s_OnPlayModeChangeCallback = change => OnPlayModeChange((PlayModeStateChange)change);
295+
InputSystem.s_OnPlayModeChangeCallback = change => OnPlayModeChange((PlayModeStateChange)(int)change);
296296
InputSystem.s_OnProjectChangeCallback = OnProjectChange;
297297
InputSystem.s_IsDomainReloadDisabled = IsDomainReloadDisabledForPlayMode;
298298
InputSystem.s_EditorGlobalInitializeCallback = OnGlobalInitialize;
299+
InputSystem.s_HasNativeObjectCallback = HasNativeObject;
299300

300301
InputSystem.s_Manager.m_AddDevicesNotSupportedByProject = () => InputEditorUserSettings.addDevicesNotSupportedByProject;
301302
}
@@ -336,8 +337,7 @@ private static void RegisterNativeInputRuntimeHooks()
336337
nativeRuntime.m_IsEditorPaused = () => EditorApplication.isPaused;
337338
nativeRuntime.m_IsEditorActive = () => InternalEditorUtility.isApplicationActive;
338339

339-
nativeRuntime.m_RegisterWantsToQuit = RegisterWantsToQuit;
340-
nativeRuntime.m_UnregisterWantsToQuit = UnregisterWantsToQuit;
340+
nativeRuntime.SetWantsToQuitCallbacks(RegisterWantsToQuit, UnregisterWantsToQuit);
341341

342342
nativeRuntime.m_SetUnityRemoteMessageHandler = SetUnityRemoteMessageHandler;
343343
nativeRuntime.m_SetUnityRemoteGyroEnabledCallback = SetUnityRemoteGyroEnabled;
@@ -464,7 +464,7 @@ private static void OnEditorPlayModeStateChanged(PlayModeStateChange change)
464464
{
465465
if (InputRuntime.s_Instance is NativeInputRuntime nativeRuntime)
466466
{
467-
nativeRuntime.DispatchPlayModeChange((int)change);
467+
nativeRuntime.DispatchPlayModeChange((InputPlayModeChange)(int)change);
468468
}
469469
}
470470

‎Packages/com.unity.inputsystem/InputSystem/Editor/Plugins/OnScreen/OnScreenButtonEditor.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ internal class OnScreenButtonEditor : UnityEditor.Editor
1212

1313
public void OnEnable()
1414
{
15-
m_ControlPathInternal = serializedObject.FindProperty(nameof(OnScreenButton.m_ControlPath));
15+
m_ControlPathInternal = serializedObject.FindProperty("m_ControlPath");
1616
}
1717

1818
public void OnDisable()

‎Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionReference.cs‎

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -269,40 +269,10 @@ public InputAction ToInputAction()
269269
private void CheckImmutableReference()
270270
{
271271
#if UNITY_EDITOR
272-
// Note that we do a lot of checking here, but it is only for a rather slim (unintended) use case in
273-
// editor and not in final builds. The alternative would be to set a non-serialized field on the reference
274-
// when importing assets which would simplify this class, but it adds complexity to import stage and
275-
// is more difficult to assess from a asset version portability perspective.
276-
bool CanSetReference(InputActionReference reference)
277-
{
278-
// If callbacks aren't set, allow the operation
279-
if (s_IsSubAsset == null || s_GetAssetPath == null || s_LoadMainAssetAtPath == null)
280-
return true;
281-
282-
// "Immutable" input action references are always sub-assets of InputActionAsset.
283-
var isSubAsset = s_IsSubAsset(reference);
284-
if (!isSubAsset)
285-
return true;
286-
287-
// If we cannot get the path of our reference, we cannot be a persisted asset within an InputActionAsset.
288-
var path = s_GetAssetPath(reference);
289-
if (path == null)
290-
return true;
291-
292-
// If we cannot get the main asset we cannot be a persisted asset within an InputActionAsset.
293-
// Also we check that it is the expected type.
294-
var mainAsset = s_LoadMainAssetAtPath(path);
295-
if (!mainAsset)
296-
return true;
297-
298-
// We can only allow setting the reference if it is not part of an persisted InputActionAsset.
299-
return (mainAsset is not InputActionAsset);
300-
}
301-
302272
// Prevent accidental mutation of the source asset if this InputActionReference is a persisted object
303273
// residing as a sub-asset within a .inputactions asset.
304274
// This is not needed for players since scriptable objects aren't serialized back from within a player.
305-
if (!CanSetReference(this))
275+
if (!CanSetReference())
306276
{
307277
throw new InvalidOperationException("Attempting to modify an immutable InputActionReference instance " +
308278
"that is part of an .inputactions asset. This is not allowed since it would modify the source " +
@@ -313,5 +283,37 @@ bool CanSetReference(InputActionReference reference)
313283
}
314284
#endif // UNITY_EDITOR
315285
}
286+
287+
#if UNITY_EDITOR
288+
// Note that we do a lot of checking here, but it is only for a rather slim (unintended) use case in
289+
// editor and not in final builds. The alternative would be to set a non-serialized field on the reference
290+
// when importing assets which would simplify this class, but it adds complexity to import stage and
291+
// is more difficult to assess from a asset version portability perspective.
292+
private bool CanSetReference()
293+
{
294+
// If callbacks aren't set, allow the operation
295+
if (s_IsSubAsset == null || s_GetAssetPath == null || s_LoadMainAssetAtPath == null)
296+
return true;
297+
298+
// "Immutable" input action references are always sub-assets of InputActionAsset.
299+
var isSubAsset = s_IsSubAsset(this);
300+
if (!isSubAsset)
301+
return true;
302+
303+
// If we cannot get the path of our reference, we cannot be a persisted asset within an InputActionAsset.
304+
var path = s_GetAssetPath(this);
305+
if (path == null)
306+
return true;
307+
308+
// If we cannot get the main asset we cannot be a persisted asset within an InputActionAsset.
309+
// Also we check that it is the expected type.
310+
var mainAsset = s_LoadMainAssetAtPath(path);
311+
if (!mainAsset)
312+
return true;
313+
314+
// We can only allow setting the reference if it is not part of an persisted InputActionAsset.
315+
return (mainAsset is not InputActionAsset);
316+
}
317+
#endif // UNITY_EDITOR
316318
}
317319
}

‎Packages/com.unity.inputsystem/InputSystem/Runtime/Actions/InputActionSetupExtensions.cs‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,7 @@ internal enum Api
348348
ControlSchemeOrWithOptionalDevice
349349
}
350350

351-
private static void RegisterApiUsage(int api)
351+
private static void RegisterAnalyticsApi(int api)
352352
{
353353
s_ApiUsageCallback?.Invoke(api);
354354
}
@@ -397,7 +397,7 @@ public static BindingSyntax AddBinding(this InputAction action, InputControl con
397397
public static BindingSyntax AddBinding(this InputAction action, InputBinding binding = default)
398398
{
399399
#if UNITY_EDITOR
400-
RegisterApiUsage((int)Api.AddBinding);
400+
RegisterAnalyticsApi((int)Api.AddBinding);
401401
#endif
402402

403403
if (action == null)
@@ -530,7 +530,7 @@ public static BindingSyntax AddBinding(this InputActionMap actionMap, string pat
530530
public static BindingSyntax AddBinding(this InputActionMap actionMap, InputBinding binding)
531531
{
532532
#if UNITY_EDITOR
533-
RegisterApiUsage((int)Api.AddBinding);
533+
RegisterAnalyticsApi((int)Api.AddBinding);
534534
#endif
535535

536536
if (actionMap == null)
@@ -557,7 +557,7 @@ public static CompositeSyntax AddCompositeBinding(this InputAction action, strin
557557
string interactions = null, string processors = null)
558558
{
559559
#if UNITY_EDITOR
560-
RegisterApiUsage((int)Api.AddCompositeBinding);
560+
RegisterAnalyticsApi((int)Api.AddCompositeBinding);
561561
#endif
562562

563563
if (action == null)
@@ -640,7 +640,7 @@ private static int AddBindingInternal(InputActionMap map, InputBinding binding,
640640
public static BindingSyntax ChangeBinding(this InputAction action, int index)
641641
{
642642
#if UNITY_EDITOR
643-
RegisterApiUsage((int)Api.ChangeBinding);
643+
RegisterAnalyticsApi((int)Api.ChangeBinding);
644644
#endif
645645

646646
if (action == null)
@@ -702,7 +702,7 @@ public static BindingSyntax ChangeBinding(this InputAction action, string name)
702702
public static BindingSyntax ChangeBinding(this InputActionMap actionMap, int index)
703703
{
704704
#if UNITY_EDITOR
705-
RegisterApiUsage((int)Api.ChangeBinding);
705+
RegisterAnalyticsApi((int)Api.ChangeBinding);
706706
#endif
707707

708708
if (actionMap == null)
@@ -904,7 +904,7 @@ public static BindingSyntax ChangeBinding(this InputAction action, InputBinding
904904
public static BindingSyntax ChangeCompositeBinding(this InputAction action, string compositeName)
905905
{
906906
#if UNITY_EDITOR
907-
RegisterApiUsage((int)Api.ChangeCompositeBinding);
907+
RegisterAnalyticsApi((int)Api.ChangeCompositeBinding);
908908
#endif
909909

910910
if (action == null)
@@ -949,7 +949,7 @@ public static BindingSyntax ChangeCompositeBinding(this InputAction action, stri
949949
public static void Rename(this InputAction action, string newName)
950950
{
951951
#if UNITY_EDITOR
952-
RegisterApiUsage((int)Api.Rename);
952+
RegisterAnalyticsApi((int)Api.Rename);
953953
#endif
954954

955955
if (action == null)
@@ -995,7 +995,7 @@ public static void Rename(this InputAction action, string newName)
995995
public static void AddControlScheme(this InputActionAsset asset, InputControlScheme controlScheme)
996996
{
997997
#if UNITY_EDITOR
998-
RegisterApiUsage((int)Api.AddControlScheme);
998+
RegisterAnalyticsApi((int)Api.AddControlScheme);
999999
#endif
10001000

10011001
if (asset == null)
@@ -1067,7 +1067,7 @@ public static ControlSchemeSyntax AddControlScheme(this InputActionAsset asset,
10671067
public static void RemoveControlScheme(this InputActionAsset asset, string name)
10681068
{
10691069
#if UNITY_EDITOR
1070-
RegisterApiUsage((int)Api.RemoveControlScheme);
1070+
RegisterAnalyticsApi((int)Api.RemoveControlScheme);
10711071
#endif
10721072

10731073
if (asset == null)
@@ -1091,7 +1091,7 @@ public static void RemoveControlScheme(this InputActionAsset asset, string name)
10911091
public static InputControlScheme WithBindingGroup(this InputControlScheme scheme, string bindingGroup)
10921092
{
10931093
#if UNITY_EDITOR
1094-
RegisterApiUsage((int)Api.ControlSchemeWithBindingGroup);
1094+
RegisterAnalyticsApi((int)Api.ControlSchemeWithBindingGroup);
10951095
#endif
10961096

10971097
return new ControlSchemeSyntax(scheme).WithBindingGroup(bindingGroup).Done();
@@ -1100,7 +1100,7 @@ public static InputControlScheme WithBindingGroup(this InputControlScheme scheme
11001100
public static InputControlScheme WithDevice(this InputControlScheme scheme, string controlPath, bool required)
11011101
{
11021102
#if UNITY_EDITOR
1103-
RegisterApiUsage((int)Api.ControlSchemeWithDevice);
1103+
RegisterAnalyticsApi((int)Api.ControlSchemeWithDevice);
11041104
#endif
11051105

11061106
if (required)
@@ -1111,7 +1111,7 @@ public static InputControlScheme WithDevice(this InputControlScheme scheme, stri
11111111
public static InputControlScheme WithRequiredDevice(this InputControlScheme scheme, string controlPath)
11121112
{
11131113
#if UNITY_EDITOR
1114-
RegisterApiUsage((int)Api.ControlSchemeWithRequiredDevice);
1114+
RegisterAnalyticsApi((int)Api.ControlSchemeWithRequiredDevice);
11151115
#endif
11161116

11171117
return new ControlSchemeSyntax(scheme).WithRequiredDevice(controlPath).Done();
@@ -1120,7 +1120,7 @@ public static InputControlScheme WithRequiredDevice(this InputControlScheme sche
11201120
public static InputControlScheme WithOptionalDevice(this InputControlScheme scheme, string controlPath)
11211121
{
11221122
#if UNITY_EDITOR
1123-
RegisterApiUsage((int)Api.ControlSchemeWithOptionalDevice);
1123+
RegisterAnalyticsApi((int)Api.ControlSchemeWithOptionalDevice);
11241124
#endif
11251125

11261126
return new ControlSchemeSyntax(scheme).WithOptionalDevice(controlPath).Done();
@@ -1129,7 +1129,7 @@ public static InputControlScheme WithOptionalDevice(this InputControlScheme sche
11291129
public static InputControlScheme OrWithRequiredDevice(this InputControlScheme scheme, string controlPath)
11301130
{
11311131
#if UNITY_EDITOR
1132-
RegisterApiUsage((int)Api.ControlSchemeOrWithRequiredDevice);
1132+
RegisterAnalyticsApi((int)Api.ControlSchemeOrWithRequiredDevice);
11331133
#endif
11341134

11351135
return new ControlSchemeSyntax(scheme).OrWithRequiredDevice(controlPath).Done();
@@ -1138,7 +1138,7 @@ public static InputControlScheme OrWithRequiredDevice(this InputControlScheme sc
11381138
public static InputControlScheme OrWithOptionalDevice(this InputControlScheme scheme, string controlPath)
11391139
{
11401140
#if UNITY_EDITOR
1141-
RegisterApiUsage((int)Api.ControlSchemeOrWithOptionalDevice);
1141+
RegisterAnalyticsApi((int)Api.ControlSchemeOrWithOptionalDevice);
11421142
#endif
11431143

11441144
return new ControlSchemeSyntax(scheme).OrWithOptionalDevice(controlPath).Done();

‎Packages/com.unity.inputsystem/InputSystem/Runtime/IInputRuntime.cs‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -186,16 +186,15 @@ internal unsafe interface IInputRuntime
186186

187187
// If analytics are enabled, the runtime receives analytics events from the input manager.
188188
// See InputAnalytics.
189-
#if UNITY_ANALYTICS || UNITY_EDITOR
189+
#if UNITY_ANALYTICS || UNITY_EDITOR
190190
void SendAnalytic(InputAnalytics.IInputAnalytic analytic);
191-
#endif // UNITY_ANALYTICS || UNITY_EDITOR
191+
#endif // UNITY_ANALYTICS || UNITY_EDITOR
192192

193-
#if UNITY_EDITOR
193+
#if UNITY_EDITOR
194194
/// <summary>
195-
/// Callback for play mode state changes. The int parameter corresponds to PlayModeStateChange enum values:
196-
/// 0 = EnteredEditMode, 1 = ExitingEditMode, 2 = EnteredPlayMode, 3 = ExitingPlayMode
195+
/// Callback for play mode state changes.
197196
/// </summary>
198-
Action<int> onPlayModeChanged { get; set; }
197+
Action<InputPlayModeChange> onPlayModeChanged { get; set; }
199198
Action onProjectChange { get; set; }
200199
bool isInPlayMode { get; }
201200
bool isEditorActive { get; }
@@ -205,7 +204,7 @@ internal unsafe interface IInputRuntime
205204
Func<IntPtr, bool> onUnityRemoteMessage { set; }
206205
void SetUnityRemoteGyroEnabled(bool value);
207206
void SetUnityRemoteGyroUpdateInterval(float interval);
208-
#endif
207+
#endif
209208
}
210209

211210
internal static class InputRuntime
@@ -214,6 +213,20 @@ internal static class InputRuntime
214213
public static double s_CurrentTimeOffsetToRealtimeSinceStartup;
215214
}
216215

216+
#if UNITY_EDITOR
217+
/// <summary>
218+
/// Mirrors <c>UnityEditor.PlayModeStateChange</c> for use in runtime code without a UnityEditor dependency.
219+
/// The underlying int values are identical so the two enums can be cast between each other freely.
220+
/// </summary>
221+
internal enum InputPlayModeChange
222+
{
223+
EnteredEditMode = 0,
224+
ExitingEditMode = 1,
225+
EnteredPlayMode = 2,
226+
ExitingPlayMode = 3,
227+
}
228+
#endif
229+
217230
internal static class InputRuntimeExtensions
218231
{
219232
public static unsafe long DeviceCommand<TCommand>(this IInputRuntime runtime, int deviceId, ref TCommand command)

‎Packages/com.unity.inputsystem/InputSystem/Runtime/InputSystem.cs‎

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3419,16 +3419,13 @@ private static bool ShouldEnableRemoting()
34193419
#if UNITY_EDITOR
34203420
/// <summary>
34213421
/// Callback for handling play mode changes. Set by InputSystemEditorInitializer.
3422-
/// The int parameter corresponds to PlayModeStateChange enum values:
3423-
/// 0 = EnteredEditMode, 1 = ExitingEditMode, 2 = EnteredPlayMode, 3 = ExitingPlayMode
34243422
/// </summary>
3425-
internal static Action<int> s_OnPlayModeChangeCallback;
3423+
internal static Action<LowLevel.InputPlayModeChange> s_OnPlayModeChangeCallback;
34263424

34273425
/// <summary>
34283426
/// Forward to InputSystemEditorInitializer for tests.
3429-
/// Uses int to avoid direct UnityEditor dependency.
34303427
/// </summary>
3431-
internal static void OnPlayModeChange(int change)
3428+
internal static void OnPlayModeChange(LowLevel.InputPlayModeChange change)
34323429
{
34333430
s_OnPlayModeChangeCallback?.Invoke(change);
34343431
}
@@ -3446,6 +3443,25 @@ internal static void OnProjectChange()
34463443
/// </summary>
34473444
internal static Action<bool> s_EditorGlobalInitializeCallback;
34483445

3446+
/// <summary>
3447+
/// Checks whether a UnityEngine.Object still has a valid native representation.
3448+
/// </summary>
3449+
/// <param name="obj">The object to check.</param>
3450+
/// <returns><c>true</c> if the object has a valid native representation, <c>false</c> otherwise.</returns>
3451+
/// <remarks>
3452+
/// This method is now internal to the Input System. Use <c>UnityEditor.EditorUtility.InstanceIDToObject</c>
3453+
/// or the appropriate Unity editor utilities directly instead.
3454+
/// </remarks>
3455+
[Obsolete("HasNativeObject has been moved to editor-only code. " +
3456+
"Use UnityEditor.EditorUtility.InstanceIDToObject(obj.GetInstanceID()) != null instead.", false)]
3457+
public static bool HasNativeObject(Object obj)
3458+
{
3459+
// Delegate to the editor-side implementation via a callback to avoid a UnityEditor dependency.
3460+
return s_HasNativeObjectCallback?.Invoke(obj) ?? true;
3461+
}
3462+
3463+
internal static Func<Object, bool> s_HasNativeObjectCallback;
3464+
34493465

34503466
#endif
34513467

0 commit comments

Comments
 (0)