-
Notifications
You must be signed in to change notification settings - Fork 74
Add CallbackHelper lifecycle handling and fix unit test execution #1276
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
base: release-6.1.0
Are you sure you want to change the base?
Changes from 4 commits
e454a77
a8b762d
9d9e99f
ef7bdbd
e599483
40a298f
d76ef5f
40cf5b9
7e8d129
580cec9
024764b
c139b1e
393b8bc
9d59e06
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 |
|---|---|---|
|
|
@@ -35,12 +35,14 @@ | |
| #define EOS_CAN_SHUTDOWN | ||
| #endif | ||
|
|
||
| // This define controls if the EOS SDK should be unloaded in the editor at shutdown to work around DLL unload errors. | ||
| //#define EOS_DO_NOT_UNLOAD_SDK_ON_SHUTDOWN | ||
|
|
||
| // On macOS and Linux, there isn't a known reliable way to unload shared libraries, therefore this is the default behavior. | ||
| #if (UNITY_EDITOR_OSX || UNITY_EDITOR_LINUX) | ||
| #define EOS_DO_NOT_UNLOAD_SDK_ON_SHUTDOWN | ||
| // As per Epic's documentation they recommend not calling PlatformInterface.Release / PlatformInterface.Shutdown in editor | ||
| // because Unity doesn't unload managed libraries until the Editor exits. Doing so will cause unintended side effects: | ||
| // https://dev.epicgames.com/docs/epic-online-services/eos-get-started/get-started-guide/next-steps#shut-down-the-eos-sdk | ||
| // This is also required on macOS/Linux always, because there isn't a known reliable way to unload shared libraries | ||
| #if UNITY_EDITOR && (!UNITY_EDITOR_WIN || !EOS_UNLOAD_SDK_ON_SHUTDOWN_IN_WIN_EDITOR) | ||
| // This define controls if the EOS SDK should be unloaded in the editor at shutdown to work around DLL unload errors. | ||
| #define EOS_DO_NOT_UNLOAD_SDK_ON_SHUTDOWN_IN_EDITOR | ||
| #endif | ||
|
|
||
|
Contributor
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. With this change to always define The change above also removes the ability for developers to control whether it's enabled in Editor or not for Windows. One option would be to change the above code to this, and update the docs to refer to this define instead:
Contributor
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. Honestly it would be nice to update these defines whilst we're here:
Contributor
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. Keeping this comment active because this docs file needs updating with the new define |
||
| #if !UNITY_EDITOR | ||
|
|
@@ -220,7 +222,7 @@ public partial class EOSSingleton | |
| static private bool s_hasInitializedPlatform; | ||
|
|
||
| private static readonly bool s_eosUnloadSDKOnShutdown = | ||
| #if EOS_DO_NOT_UNLOAD_SDK_ON_SHUTDOWN | ||
| #if EOS_DO_NOT_UNLOAD_SDK_ON_SHUTDOWN_IN_EDITOR && UNITY_EDITOR | ||
| false | ||
| #else | ||
| true | ||
|
|
@@ -1640,15 +1642,29 @@ public void OnApplicationShutdown() | |
| Log("Waiting for pending finalizers."); | ||
| System.GC.WaitForPendingFinalizers(); | ||
| #endif | ||
| Log("Disposing notification handles before platform release."); | ||
| s_notifyLoginStatusChangedCallbackHandle?.Dispose(); | ||
| s_notifyLoginStatusChangedCallbackHandle = null; | ||
|
|
||
| s_notifyConnectLoginStatusChangedCallbackHandle?.Dispose(); | ||
| s_notifyConnectLoginStatusChangedCallbackHandle = null; | ||
|
|
||
| s_notifyConnectAuthExpirationCallbackHandle?.Dispose(); | ||
| s_notifyConnectAuthExpirationCallbackHandle = null; | ||
|
|
||
| Log("Releasing the EOS Platform Interface."); | ||
| GetEOSPlatformInterface()?.Release(); | ||
|
|
||
| #if UNITY_EDITOR | ||
| if (s_eosUnloadSDKOnShutdown) | ||
| { | ||
| Log("Shutting down the platform interface."); | ||
| ShutdownPlatformInterface(); | ||
| } | ||
|
matt-clarke marked this conversation as resolved.
|
||
|
|
||
| #else | ||
| Log("Shutting down the platform interface."); | ||
| ShutdownPlatformInterface(); | ||
| #endif | ||
|
matt-clarke marked this conversation as resolved.
Outdated
|
||
| SetEOSPlatformInterface(null); | ||
|
|
||
|
|
||
|
|
||
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.
This docs file covers a couple of points which are a bit of a concern:
"It is not the default because if it were, then any edits to the EOSConfig file would require a reboot of the Unity Editor""In the past, not unloading the EOS SDK could also cause issues with state from the previous Unity run not being properly cleaned up."It also carries with it the potential for "leaked" EOS State to carry over between play in editor runs.)