Skip to content

Add CallbackHelper lifecycle handling and fix unit test execution#1276

Open
OswaldoApexSystems wants to merge 5 commits intorelease-6.1.0from
Fix/EOSU-1013-Update-Services/EOSSessionsManagerTests.cs
Open

Add CallbackHelper lifecycle handling and fix unit test execution#1276
OswaldoApexSystems wants to merge 5 commits intorelease-6.1.0from
Fix/EOSU-1013-Update-Services/EOSSessionsManagerTests.cs

Conversation

@OswaldoApexSystems
Copy link
Copy Markdown
Contributor

EOSU-1013
image
image

-Introduced a new CallbackHelper class to centralize notification callback lifecycle management and prevent stale callback collisions when the EOS Platform is recreated.

-Updated EOSManager and FriendTest to correctly integrate with the new callback lifecycle behavior, ensuring callbacks are cleared at the proper time during test teardown and platform shutdown.

-These changes fix failures encountered when running the full unit test suite by ensuring isolation between tests and preventing leftover EOS notification state from leaking across test runs.

-Introduced a new CallbackHelper class to centralize notification callback
lifecycle management and prevent stale callback collisions when the EOS
Platform is recreated.

-Updated EOSManager and FriendTest to correctly integrate with the new
callback lifecycle behavior, ensuring callbacks are cleared at the proper
time during test teardown and platform shutdown.

-These changes fix failures encountered when running the full unit test
suite by ensuring isolation between tests and preventing leftover EOS
notification state from leaking across test runs.
Comment thread Assets/Tests/PlayMode/Services/FriendsTests.cs Outdated
Comment thread com.playeveryware.eos/Runtime/EOS_SDK/Core/CallbackHelper.Lifecycle.cs Outdated
Comment thread com.playeveryware.eos/Runtime/EOS_SDK/Core/CallbackHelper.Lifecycle.cs Outdated
@OswaldoApexSystems OswaldoApexSystems marked this pull request as draft April 22, 2026 17:49
…lback disposal

Uncommented EOS_DO_NOT_UNLOAD_SDK_ON_SHUTDOWN to prevent unloading the EOS SDK in the Unity Editor, in line with the official EOS documentation.

This change resolves the unit test failures by avoiding PlatformInterface.Shutdown() during editor test runs, which was corrupting internal SDK state across multiple test suites.
// 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
#endif
Copy link
Copy Markdown
Contributor

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"
    • Can you check if this is accurate?
    • If it is then we would need to think of a solution to this, perhaps when you save the EOSConfig it prompts you to restart the Editor (after checking the new define mentioned in the comment below)
    • I suspect we also need a Unity hook to check if the file was modified outside of Unity (user could've edited it by hand, or pulled latest changes from their repo)
  • "In the past, not unloading the EOS SDK could also cause issues with state from the previous Unity run not being properly cleaned up."
    • This is the opposite of what we're seeing right? I'm wary that there's some edge case issues that we haven't seen with this initial testing. It could definitely be a misunderstanding though
    • The commit message where it was added also says the same, so it doesn't appear to just be a mistake in the docs (It also carries with it the potential for "leaked" EOS State to carry over between play in editor runs.)

Comment thread com.playeveryware.eos/Runtime/Core/EOSManager.cs Outdated
Comment thread com.playeveryware.eos/Runtime/Core/EOSManager.cs Outdated
#if (UNITY_EDITOR_OSX || UNITY_EDITOR_LINUX)
#define EOS_DO_NOT_UNLOAD_SDK_ON_SHUTDOWN
#endif

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

With this change to always define EOS_DO_NOT_UNLOAD_SDK_ON_SHUTDOWN in Editor this means that this docs file is out of date.

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:

#if UNITY_EDITOR && (!UNITY_EDITOR_WIN || !EOS_UNLOAD_SDK_ON_SHUTDOWN)
#define EOS_DO_NOT_UNLOAD_SDK_ON_SHUTDOWN
#endif

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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:

  • EOS_DO_NOT_UNLOAD_SDK_ON_SHUTDOWN_IN_EDITOR
  • EOS_UNLOAD_SDK_ON_SHUTDOWN_IN_WIN_EDITOR

Comment thread com.playeveryware.eos/Runtime/Core/EOSManager.cs
Comment thread com.playeveryware.eos/Runtime/Core/EOSManager.cs Outdated
 The shutdown/unload logic is now Editor‑only, runtime behavior is protected, the hardcoded define was removed, and Windows Editor behavior is aligned with macOS/Linux and Epic documentation.
Comment on lines +1658 to +1667
#if UNITY_EDITOR
if (s_eosUnloadSDKOnShutdown)
{
Log("Shutting down the platform interface.");
ShutdownPlatformInterface();
}

#else
Log("Shutting down the platform interface.");
ShutdownPlatformInterface();
#endif
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for making that change, although this is a little cleaner

Suggested change
#if UNITY_EDITOR
if (s_eosUnloadSDKOnShutdown)
{
Log("Shutting down the platform interface.");
ShutdownPlatformInterface();
}
#else
Log("Shutting down the platform interface.");
ShutdownPlatformInterface();
#endif
#if UNITY_EDITOR
if (s_eosUnloadSDKOnShutdown)
#endif
{
Log("Shutting down the platform interface.");
ShutdownPlatformInterface();
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried this refactor, but it causes Unity to crash.

The issue is that moving the #if UNITY_EDITOR above only the condition causes
ShutdownPlatformInterface() to execute unconditionally outside of the Editor,
without being explicitly guarded. In runtime builds this breaks the expected
SDK shutdown order and results in a native crash.

Keeping the explicit #if UNITY_EDITOR / #else around the entire shutdown block
ensures that:

  • Editor behavior remains configurable
  • Runtime builds always shut down the platform in a safe and explicit path

For this reason I’m keeping the previous structure, which is slightly more verbose
but avoids unsafe execution paths.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you double check the changes you made to test vs my suggestion? This should be functionality identical as your code. It's compiling out the if check so the code will always run in runtime builds.

Is that not the desired functionality? I'm a bit confused because the 2nd paragraph sounds like it shouldn't always run in runtime builds, but the bullet points sounds like it should (and the code lines up with this).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I double‑checked this using the suggested refactor and captured the Editor.log.
While the logic looks equivalent on paper, in practice it changes shutdown ordering.
With the refactor, EOS shutdown occurs while async operations and callbacks are still active.
The log shows EOS attempting to reuse an existing native handle (Found existing handle for EOSSDK‑Win64‑Shipping) after shutdown, which results in native crashes due to use‑after‑free / double teardown.
Keeping the explicit #if UNITY_EDITOR / #else block ensures shutdown only happens in the intended lifecycle phase and avoids teardown during Editor domain reloads and test cleanup.

Editor.log

@OswaldoApexSystems OswaldoApexSystems marked this pull request as ready for review April 24, 2026 16:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants