Skip to content

Fix: Prevent FriendsListToggle null reference after scene changes#1297

Open
JessTello wants to merge 1 commit into
developmentfrom
bug/eosu-1391-friends-list-toggle-null-ref
Open

Fix: Prevent FriendsListToggle null reference after scene changes#1297
JessTello wants to merge 1 commit into
developmentfrom
bug/eosu-1391-friends-list-toggle-null-ref

Conversation

@JessTello

Copy link
Copy Markdown
Contributor
  • Clear static Friends tab UI callbacks on scene unload to avoid retaining old listeners.
  • Safely invoke Friends tab expand/collapse callbacks, skipping destroyed Unity targets.
  • Guard optional UIFriendInteractionSource usage to prevent calls into missing or destroyed references during UI refresh.

- Clear static Friends tab UI callbacks on scene unload to avoid retaining old listeners.
- Safely invoke Friends tab expand/collapse callbacks, skipping destroyed Unity targets.
- Guard optional UIFriendInteractionSource usage to prevent calls into missing or destroyed references during UI refresh.
@JessTello JessTello requested review from a team and Pong-Epic June 11, 2026 21:35

public static void InvokeOnCollapseFriendsTab()
{
InvokeFriendsTabAction(ref OnCollapseFriendsTab);

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.

Just want to make the function names here a bit more generic

Suggested change
InvokeFriendsTabAction(ref OnCollapseFriendsTab);
SafeInvokeAction(ref OnCollapseFriendsTab);


public static void InvokeOnExpandFriendsTab()
{
InvokeFriendsTabAction(ref OnExpandFriendsTab);

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.

Suggested change
InvokeFriendsTabAction(ref OnExpandFriendsTab);
SafeInvokeAction(ref OnExpandFriendsTab);

[RuntimeInitializeOnLoadMethod(RuntimeInitializeLoadType.SubsystemRegistration)]
private static void Initialize()
{
ClearFriendsTabActions();

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.

Suggested change
ClearFriendsTabActions();
ClearActions();


private static void OnSceneUnloaded(Scene scene)
{
ClearFriendsTabActions();

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.

Suggested change
ClearFriendsTabActions();
ClearActions();

ClearFriendsTabActions();
}

private static void ClearFriendsTabActions()

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.

Suggested change
private static void ClearFriendsTabActions()
private static void ClearActions()

OnExpandFriendsTab = null;
}

private static void InvokeFriendsTabAction(ref Action action)

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.

Suggested change
private static void InvokeFriendsTabAction(ref Action action)
private static void SafeInvokeAction(ref Action action)

{
ClearFriendsTabActions();
SceneManager.sceneUnloaded -= OnSceneUnloaded;
SceneManager.sceneUnloaded += OnSceneUnloaded;

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.

Are we able to remove this and fully rely on the null checking in the invoke function?

Clearing all the events when a scene is unloaded might not be a problem at the moment but there are cases where the subscription needs to survive:

  • Objects marked with DontDestroyOnLoad
  • Code that doesn't belong to a Unity object (e.g. static functions)

If multiple scenes are loaded at once at any point then when a scene unloads the actions are nulled too.

@ricardoamores-epic ricardoamores-epic left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM

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.

3 participants