-
Notifications
You must be signed in to change notification settings - Fork 329
FIX: Address warnings that happened with Unity 6.4 #2233
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
Changes from all commits
b2eb798
c190766
d189199
acf556d
24bda46
f4eb680
e007218
d9c7d88
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3592,8 +3592,7 @@ internal static void InitializeInEditor(IInputRuntime runtime = null) | |
} | ||
|
||
Debug.Assert(settings != null); | ||
Debug.Assert(EditorUtility.InstanceIDToObject(settings.GetInstanceID()) != null, | ||
"InputSettings has lost its native object"); | ||
Debug.Assert(HasNativeObject(settings), "InputSettings has lost its native object"); | ||
|
||
// If native backends for new input system aren't enabled, ask user whether we should | ||
// enable them (requires restart). We only ask once per session and don't ask when | ||
|
@@ -3696,6 +3695,16 @@ internal static void OnPlayModeChange(PlayModeStateChange change) | |
} | ||
} | ||
|
||
// We have this function to hide away instanceId -> entityId migration that happened in Unity 6.4 | ||
public static bool HasNativeObject(Object obj) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: Hmmm.... This diff was hard to review but just a thought. What if this code was changed so that
And then using a conditional alias where needed for EntityId, e.g. #if !UNITY_6000_4_OR_NEWER
using EntityId = int;
#endif Would that eliminate all other branches? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe it just makes things weird if entity id and instance id are fundamentally different, if not maybe its something.... |
||
{ | ||
#if UNITY_6000_4_OR_NEWER | ||
return EditorUtility.EntityIdToObject(obj.GetEntityId()) != null; | ||
#else | ||
return EditorUtility.InstanceIDToObject(obj.GetInstanceID()) != null; | ||
#endif | ||
} | ||
|
||
private static void OnProjectChange() | ||
{ | ||
////TODO: use dirty count to find whether settings have actually changed | ||
|
@@ -3707,7 +3716,7 @@ private static void OnProjectChange() | |
// temporary settings object. | ||
// NOTE: We access m_Settings directly here to make sure we're not running into asserts | ||
// from the settings getter checking it has a valid object. | ||
if (EditorUtility.InstanceIDToObject(s_Manager.m_Settings.GetInstanceID()) == null) | ||
if (!HasNativeObject(s_Manager.m_Settings)) | ||
{ | ||
var newSettings = ScriptableObject.CreateInstance<InputSettings>(); | ||
newSettings.hideFlags = HideFlags.HideAndDontSave; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -421,7 +421,11 @@ protected virtual void Awake() | |
#if UNITY_INPUT_SYSTEM_ENABLE_VR && ENABLE_VR | ||
if (HasStereoCamera(out var cameraComponent)) | ||
{ | ||
// The Unity 6.4+ replacement for this call has to be figured later | ||
// See https://jira.unity3d.com/browse/XR-7591 | ||
#pragma warning disable CS0618 | ||
UnityEngine.XR.XRDevice.DisableAutoXRCameraTracking(cameraComponent, true); | ||
#pragma warning restore CS0618 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also a Slack thread: https://unity.slack.com/archives/C06TQE84B/p1757416691328499 |
||
} | ||
#endif | ||
} | ||
|
@@ -458,7 +462,11 @@ protected virtual void OnDestroy() | |
#if UNITY_INPUT_SYSTEM_ENABLE_VR && ENABLE_VR | ||
if (HasStereoCamera(out var cameraComponent)) | ||
{ | ||
// The Unity 6.4+ replacement for this call has to be figured later | ||
// See https://jira.unity3d.com/browse/XR-7591 | ||
#pragma warning disable CS0618 | ||
UnityEngine.XR.XRDevice.DisableAutoXRCameraTracking(cameraComponent, false); | ||
#pragma warning restore CS0618 | ||
} | ||
#endif | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I read this code myself, I could have left IsInputActionAssetPath in OpenAsset, just the GetAssetPath thing is different. IDK, either way goes fine by my books
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor: But I would agree with you, that if it makes the "fork condition" slimmer it is a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But its also fine as-is