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.

Sign up for GitHub

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

Jump to bottom

updated playmode/editor mode change transitions #704

Merged
merged 5 commits into from
Nov 28, 2020
Merged

updated playmode/editor mode change transitions #704

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
51dcfeb
updated playmode/editor mode change transitions
Nov 27, 2020
5107920
use nameof
Nov 27, 2020
119f4d6
fixed project manifest
Nov 27, 2020
b442edc
properly use nameof
Nov 27, 2020
8b1e7db
fixed tests
Nov 28, 2020
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
53 changes: 33 additions & 20 deletions XRTK-Core/Packages/com.xrtk.core/Runtime/Services/MixedRealityToolkit.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,12 @@ private void InitializeInstance()
DontDestroyOnLoad(instance.transform.root);
}

Application.quitting += () => IsApplicationQuitting = true;
Application.quitting += () =>
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't have used a lambda here but I guess it's too late.

Copy link
Member Author

Choose a reason for hiding this comment

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

Why not?

Copy link
Contributor

Choose a reason for hiding this comment

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

Just a personal preference. Application exit and cleanup is an important event and imo deserves a proper method definition. That makes it for others easier to find the code executed when the toolkit is "deinitialized" in the future, instead of having to find it somewhere in between other code.

But as I said, it's not super important. I just wouldn't have done it.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's an event you can subscribe to anywhere. We do provide this public flag for if the application is quitting via the property, but I don't really think it makes a difference if the Application.quitting event can be subscribed from anywhere in the codebase

Copy link
Contributor

Choose a reason for hiding this comment

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

I think you misunderstood. All I am saying is I'd create a definition private void MixedRealityToolkit_OnQuitting() in MixedRealityToolkit instead of an anonymous method. Because that way if someone is looking up "What does the XRTK do, when the application quits?", it is much easier to find. All good.

Copy link
Member Author

Choose a reason for hiding this comment

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

Now that makes sense !👍

{
DisableAllServices();
DestroyAllServices();
IsApplicationQuitting = true;
};

#if UNITY_EDITOR
UnityEditor.EditorApplication.hierarchyChanged += OnHierarchyChanged;
Expand All @@ -284,27 +289,35 @@ void OnHierarchyChanged()
{
if (instance != null)
{
Debug.Assert(instance.transform.parent == null, "The MixedRealityToolkit should not be parented under any other GameObject!");
Debug.Assert(instance.transform.childCount == 0, "The MixedRealityToolkit should not have GameObject children!");
Debug.Assert(instance.transform.parent == null, $"The {nameof(MixedRealityToolkit)} should not be parented under any other GameObject!");
Debug.Assert(instance.transform.childCount == 0, $"The {nameof(MixedRealityToolkit)} should not have GameObject children!");
}
}

void OnPlayModeStateChanged(UnityEditor.PlayModeStateChange playModeState)
{
if (playModeState == UnityEditor.PlayModeStateChange.ExitingEditMode ||
playModeState == UnityEditor.PlayModeStateChange.EnteredEditMode)
switch (playModeState)
{
IsApplicationQuitting = false;
case UnityEditor.PlayModeStateChange.EnteredEditMode:
IsApplicationQuitting = false;
break;
case UnityEditor.PlayModeStateChange.ExitingEditMode:
if (activeProfile == null)
{
UnityEditor.EditorApplication.isPlaying = false;
UnityEditor.Selection.activeObject = Instance;
UnityEditor.EditorApplication.delayCall += () =>
UnityEditor.EditorGUIUtility.PingObject(Instance);
}
break;
case UnityEditor.PlayModeStateChange.EnteredPlayMode:
case UnityEditor.PlayModeStateChange.ExitingPlayMode:
// Nothing for now.
break;
default:
throw new ArgumentOutOfRangeException(nameof(playModeState), playModeState, null);
}

if (activeProfile == null &&
playModeState == UnityEditor.PlayModeStateChange.ExitingEditMode)
{
UnityEditor.EditorApplication.isPlaying = false;
UnityEditor.Selection.activeObject = Instance;
UnityEditor.EditorApplication.delayCall += () =>
UnityEditor.EditorGUIUtility.PingObject(Instance);
}
}
#endif // UNITY_EDITOR

Expand Down Expand Up @@ -335,7 +348,7 @@ void OnPlayModeStateChanged(UnityEditor.PlayModeStateChange playModeState)
/// </summary>
public static void AssertIsInitialized()
{
Debug.Assert(IsInitialized, "The MixedRealityToolkit has not been initialized.");
Debug.Assert(IsInitialized, $"The {nameof(MixedRealityToolkit)} has not been initialized.");
}

/// <summary>
Expand All @@ -361,16 +374,16 @@ private void InitializeServiceLocator()
{
if (isInitializing)
{
Debug.LogWarning("Already attempting to initialize the service locator!");
Debug.LogWarning($"Already attempting to initialize the {nameof(MixedRealityToolkit)}!");
return;
}

isInitializing = true;

//If the Mixed Reality Toolkit is not configured, stop.
// If the Mixed Reality Toolkit is not configured, stop.
if (ActiveProfile == null)
{
Debug.LogError("No Mixed Reality Root Profile found, cannot initialize the Mixed Reality Toolkit");
Debug.LogError($"No {nameof(MixedRealityToolkitRootProfile)} found, cannot initialize the {nameof(MixedRealityToolkit)}");
isInitializing = false;
return;
}
Expand Down Expand Up @@ -636,7 +649,7 @@ private static void EnsureMixedRealityRequirements()

if (raiseWarning)
{
Debug.LogWarning("Found an existing event system in your scene. The Mixed Reality Toolkit requires only one, and must be found on the main camera.");
Debug.LogWarning($"Found an existing event system in your scene. The {nameof(MixedRealityToolkit)} requires only one, and must be found on the main camera.");
}
}
}
Expand Down Expand Up @@ -1894,7 +1907,7 @@ private static bool CanGetService(Type interfaceType, string serviceName)

if (!typeof(IMixedRealityService).IsAssignableFrom(interfaceType))
{
Debug.LogError($"{interfaceType.Name} does not implement {typeof(IMixedRealityService).Name}.");
Debug.LogError($"{interfaceType.Name} does not implement {nameof(IMixedRealityService)}.");
return false;
}

Expand Down
31 changes: 16 additions & 15 deletions XRTK-Core/Packages/com.xrtk.core/Tests/Core/TestFixture_01_MixedRealityToolkitTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using UnityEditor.SceneManagement;
using UnityEngine;
using UnityEngine.TestTools;
using XRTK.Definitions;
using XRTK.Editor.Utilities;
using XRTK.Interfaces;
using XRTK.Services;
Expand Down Expand Up @@ -52,7 +53,7 @@ public void Test_02_TestNoMixedRealityProfileFound()
Assert.IsFalse(MixedRealityToolkit.HasActiveProfile);
Assert.IsNull(MixedRealityToolkit.Instance.ActiveProfile);
Assert.IsFalse(MixedRealityToolkit.HasActiveProfile);
LogAssert.Expect(LogType.Error, "No Mixed Reality Root Profile found, cannot initialize the Mixed Reality Toolkit");
LogAssert.Expect(LogType.Error, $"No {nameof(MixedRealityToolkitRootProfile)} found, cannot initialize the {nameof(MixedRealityToolkit)}");
}

[Test]
Expand Down Expand Up @@ -127,10 +128,10 @@ public void Test_04_02_01_UnregisterMixedRealityServiceAndDataProviderByType()

// Unregister
var successService = MixedRealityToolkit.TryUnregisterServicesOfType<ITestService>();
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestService).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestService)} service.");

var successDataProvider = MixedRealityToolkit.TryUnregisterServicesOfType<ITestDataProvider1>();
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestDataProvider1).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestDataProvider1)} service.");

// Validate non-existent service
var isServiceRegistered = MixedRealityToolkit.IsServiceRegistered<ITestService>();
Expand Down Expand Up @@ -175,9 +176,9 @@ public void Test_04_02_02_UnregisterMixedRealityDataProviderByTypeAndName()

// Unregister
var successService = MixedRealityToolkit.TryUnregisterService<ITestService>(testService1.Name);
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestService).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestService)} service.");
var successDataProvider = MixedRealityToolkit.TryUnregisterService<ITestDataProvider1>(dataProvider1.Name);
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestDataProvider1).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestDataProvider1)} service.");

// Validate non-existent service
var isServiceRegistered = MixedRealityToolkit.IsServiceRegistered<ITestService>();
Expand Down Expand Up @@ -268,11 +269,11 @@ public void Test_04_04_UnregisterMixedRealityDataProvidersByType()

// Validate non-existent service
var isServiceRegistered = MixedRealityToolkit.IsServiceRegistered<ITestService>();
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestService).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestService)} service.");
var isService1Registered = MixedRealityToolkit.IsServiceRegistered<ITestDataProvider1>();
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestDataProvider1).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestDataProvider1)} service.");
var isService2Registered = MixedRealityToolkit.IsServiceRegistered<ITestDataProvider2>();
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestDataProvider2).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestDataProvider2)} service.");

// Tests
Assert.IsFalse(isServiceRegistered);
Expand All @@ -299,7 +300,7 @@ public void Test_04_05_MixedRealityDataProviderDoesNotExist()
var isServiceRegistered = MixedRealityToolkit.IsServiceRegistered<ITestDataProvider2>();

// Tests
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestDataProvider2).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestDataProvider2)} service.");
Assert.IsFalse(isServiceRegistered);
}

Expand Down Expand Up @@ -362,7 +363,7 @@ public void Test_04_08_GetMixedRealityDataProviderCollectionByInterface()

// Add test data provider 2
MixedRealityToolkit.TryRegisterService<ITestDataProvider2>(failService);
LogAssert.Expect(LogType.Error, $"{failService.Name} does not implement {typeof(ITestDataProvider2).Name}");
LogAssert.Expect(LogType.Error, $"{failService.Name} does not implement {nameof(ITestDataProvider2)}");

MixedRealityToolkit.TryRegisterService<ITestDataProvider2>(new TestDataProvider2(testService1, "Test04-08-2.2"));

Expand Down Expand Up @@ -418,7 +419,7 @@ public void Test_05_02_01_UnregisterMixedRealityExtensionServiceByType()

// Validate non-existent service
var isServiceRegistered = MixedRealityToolkit.IsServiceRegistered<ITestExtensionService1>();
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestExtensionService1).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestExtensionService1)} service.");

// Tests
Assert.IsTrue(success);
Expand Down Expand Up @@ -449,7 +450,7 @@ public void Test_05_02_02_UnregisterMixedRealityExtensionServiceByTypeAndName()

// Validate non-existent service
var isServiceRegistered = MixedRealityToolkit.IsServiceRegistered<ITestExtensionService1>();
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestExtensionService1).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestExtensionService1)} service.");

// Tests
Assert.IsTrue(success);
Expand Down Expand Up @@ -509,9 +510,9 @@ public void Test_05_04_UnregisterMixedRealityExtensionServicesByType()

// Validate non-existent service
var isService1Registered = MixedRealityToolkit.IsServiceRegistered<ITestExtensionService1>();
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestExtensionService1).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestExtensionService1)} service.");
var isService2Registered = MixedRealityToolkit.IsServiceRegistered<ITestExtensionService2>();
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestExtensionService2).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestExtensionService2)} service.");

// Tests
Assert.IsTrue(success);
Expand All @@ -533,7 +534,7 @@ public void Test_05_05_MixedRealityExtensionService2DoesNotExist()
var isServiceRegistered = MixedRealityToolkit.IsServiceRegistered<ITestExtensionService2>();

// Tests
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(ITestExtensionService2).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(ITestExtensionService2)} service.");
Assert.IsFalse(isServiceRegistered);
}

Expand Down
2 changes: 1 addition & 1 deletion XRTK-Core/Packages/com.xrtk.core/Tests/InputSystem/TestFixture_03_InputSystemTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public void Test03_TestMixedRealityInputSystemDoesNotExist()

// Tests
Assert.IsFalse(inputSystemExists);
LogAssert.Expect(LogType.Error, $"Unable to find {typeof(IMixedRealityInputSystem).Name} service.");
LogAssert.Expect(LogType.Error, $"Unable to find {nameof(IMixedRealityInputSystem)} service.");
}

[Test]
Expand Down
1 change: 0 additions & 1 deletion XRTK-Core/Packages/manifest.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
"com.unity.test-framework": "1.1.19",
"com.unity.textmeshpro": "2.1.3",
"com.unity.ugui": "1.0.0",
"com.unity.xr.legacyinputhelpers": "2.1.6",
"com.unity.xr.magicleap": "4.1.3",
"com.unity.modules.ai": "1.0.0",
"com.unity.modules.androidjni": "1.0.0",
Expand Down
2 changes: 1 addition & 1 deletion XRTK-Core/Packages/packages-lock.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@
},
"com.unity.xr.legacyinputhelpers": {
"version": "2.1.6",
"depth": 0,
"depth": 1,
"source": "registry",
"dependencies": {},
"url": "https://packages.unity.com"
Expand Down