Skip to content

Commit 9898ee8

Browse files
committed
fix delegates lifetime: unsubscribe stale handlers on network despawn
CSync leaves stale change handlers after quitting the hosted game. When the host quits the game back to the main menu, ConfigSyncBehaviour despawns and gets destroyed by Unity (not by .NET GC), but any delegates connected to events in OnNetworkSpawn method remain and go stale. So next time a player hosts a lobby without shutting down and re-launching the game completely, when any of subscribed config files or config entries change, stale delegates execute and try to assign to a dead NetworkList _deltas[index]. Of course it results in an exception being logged to the console by BepInEx (which wraps event handler invocation in a try-catch). To fix this, store subscribed delegates, and properly unsubscribe them during despawn. Note that NetworkList and _syncEnabled variable can not be cleared/reset due to Unity issue which only got resolved in the most recent version. References: - https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/delegates/using-delegates - Unity-Technologies/com.unity.netcode.gameobjects#3502 Fixes #13
1 parent 3ddfcca commit 9898ee8

File tree

1 file changed

+60
-18
lines changed

1 file changed

+60
-18
lines changed

CSync/CSync/Lib/ConfigSyncBehaviour.cs

Lines changed: 60 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
1+
using BepInEx.Configuration;
12
using System;
23
using System.Collections.Generic;
34
using System.Diagnostics.CodeAnalysis;
5+
using System.Linq;
46
using Unity.Netcode;
57
using UnityEngine;
68
using LogLevel = BepInEx.Logging.LogLevel;
@@ -36,7 +38,9 @@ public bool SyncEnabled
3638
}
3739

3840
private readonly NetworkVariable<bool> _syncEnabled = new();
39-
private NetworkList<SyncedEntryDelta> _deltas = null!;
41+
private readonly NetworkList<SyncedEntryDelta> _deltas = new();
42+
private readonly List<(ConfigFile, EventHandler<SettingChangedEventArgs>)> _configFileDelegates = new();
43+
private readonly List<(SyncedEntryBase, EventHandler)> _entrySyncEnabledDelegates = new();
4044

4145
[MemberNotNull(nameof(EntryContainer))]
4246
private void EnsureEntryContainer()
@@ -45,12 +49,6 @@ private void EnsureEntryContainer()
4549
throw new InvalidOperationException("Entry container has not been assigned.");
4650
}
4751

48-
private void Awake()
49-
{
50-
EnsureEntryContainer();
51-
_deltas = new NetworkList<SyncedEntryDelta>();
52-
}
53-
5452
public override void OnNetworkSpawn()
5553
{
5654
EnsureEntryContainer();
@@ -59,28 +57,30 @@ public override void OnNetworkSpawn()
5957
{
6058
_syncEnabled.Value = true;
6159

62-
foreach (var syncedEntryBase in EntryContainer.Values)
60+
foreach (var (syncedEntryBase, index) in EntryContainer.Values.Select((e, i) => (e, i)))
6361
{
64-
var currentIndex = _deltas.Count;
6562
_deltas.Add(syncedEntryBase.ToDelta());
6663

67-
syncedEntryBase.BoxedEntry.ConfigFile.SettingChanged += (_, args) =>
64+
void configFileDelegate(object sender, SettingChangedEventArgs args)
6865
{
6966
if (!ReferenceEquals(syncedEntryBase.BoxedEntry, args.ChangedSetting)) return;
70-
_deltas[currentIndex] = syncedEntryBase.ToDelta();
71-
};
67+
_deltas[index] = syncedEntryBase.ToDelta();
68+
}
69+
var configFile = syncedEntryBase.BoxedEntry.ConfigFile;
70+
configFile.SettingChanged += configFileDelegate;
71+
_configFileDelegates.Add((configFile, configFileDelegate));
7272

73-
syncedEntryBase.SyncEnabledChanged += (_, args) =>
73+
void entrySyncEnabledDelegate(object sender, EventArgs args)
7474
{
75-
_deltas[currentIndex] = syncedEntryBase.ToDelta();
76-
};
75+
_deltas[index] = syncedEntryBase.ToDelta();
76+
}
77+
syncedEntryBase.SyncEnabledChanged += entrySyncEnabledDelegate;
78+
_entrySyncEnabledDelegates.Add((syncedEntryBase, entrySyncEnabledDelegate));
7779
}
7880

7981
InitialSyncCompletedHandler?.Invoke(this, EventArgs.Empty);
80-
return;
8182
}
82-
83-
if (IsClient)
83+
else if (IsClient)
8484
{
8585
_syncEnabled.OnValueChanged += OnSyncEnabledChanged;
8686
_deltas.OnListChanged += OnClientDeltaListChanged;
@@ -94,6 +94,48 @@ public override void OnNetworkSpawn()
9494

9595
InitialSyncCompletedHandler?.Invoke(this, EventArgs.Empty);
9696
}
97+
98+
base.OnNetworkSpawn();
99+
}
100+
101+
public override void OnNetworkDespawn()
102+
{
103+
EnsureEntryContainer();
104+
105+
if (IsServer)
106+
{
107+
foreach (var (configFile, configFileDelegate) in _configFileDelegates)
108+
{
109+
configFile.SettingChanged -= configFileDelegate;
110+
}
111+
112+
foreach (var (syncedEntryBase, entrySyncEnabledDelegate) in _entrySyncEnabledDelegates)
113+
{
114+
syncedEntryBase.SyncEnabledChanged -= entrySyncEnabledDelegate;
115+
}
116+
117+
_configFileDelegates.Clear();
118+
_entrySyncEnabledDelegates.Clear();
119+
// NetworkList and NetworkVariable can not be modified now because the network manager has already shut down.
120+
// See https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/pull/3502
121+
// Should not be needed anyway, since this component isn't being reused between network sessions.
122+
// _deltas.Clear();
123+
// _syncEnabled.Value = false;
124+
}
125+
else if (IsClient)
126+
{
127+
DisableOverrides();
128+
129+
foreach (var delta in _deltas)
130+
{
131+
ResetOverrideValue(delta);
132+
}
133+
134+
_deltas.OnListChanged -= OnClientDeltaListChanged;
135+
_syncEnabled.OnValueChanged -= OnSyncEnabledChanged;
136+
}
137+
138+
base.OnNetworkDespawn();
97139
}
98140

99141
public override void OnDestroy()

0 commit comments

Comments
 (0)