Skip to content

Commit 0fc402a

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. Lethal Company v73 updated Unity Netcode for GameObjects (NGO) to 1.12.0, but the fix is available for NGO 1.14.0+ only. References: - https://learn.microsoft.com/en-us/dotnet/csharp/programming-guide/delegates/using-delegates - Unity-Technologies/com.unity.netcode.gameobjects#3502 Fixes lc-sigurd#13
1 parent 33b273d commit 0fc402a

File tree

1 file changed

+73
-16
lines changed

1 file changed

+73
-16
lines changed

CSync/CSync/Lib/ConfigSyncBehaviour.cs

Lines changed: 73 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using BepInEx.Configuration;
12
using System;
23
using System.Collections.Generic;
34
using System.Diagnostics.CodeAnalysis;
@@ -37,6 +38,7 @@ public bool SyncEnabled
3738

3839
private readonly NetworkVariable<bool> _syncEnabled = new();
3940
private NetworkList<SyncedEntryDelta> _deltas = null!;
41+
private readonly List<EventHandlerHelper> _eventHandlers = new();
4042

4143
[MemberNotNull(nameof(EntryContainer))]
4244
private void EnsureEntryContainer()
@@ -51,6 +53,42 @@ private void Awake()
5153
_deltas = new NetworkList<SyncedEntryDelta>();
5254
}
5355

56+
// Helper class to maintain event subscriptions.
57+
// SyncedEntryDelta is a struct which can't be copied around, so it has to be stored by list+index.
58+
class EventHandlerHelper : IDisposable
59+
{
60+
private readonly NetworkList<SyncedEntryDelta> deltas;
61+
private readonly int index;
62+
private readonly SyncedEntryBase syncedEntryBase;
63+
64+
public EventHandlerHelper(NetworkList<SyncedEntryDelta> deltas, int index, SyncedEntryBase syncedEntryBase)
65+
{
66+
this.deltas = deltas;
67+
this.index = index;
68+
this.syncedEntryBase = syncedEntryBase;
69+
70+
syncedEntryBase.BoxedEntry.ConfigFile.SettingChanged += OnSettingChanged;
71+
syncedEntryBase.SyncEnabledChanged += OnEntrySyncEnabledChanged;
72+
}
73+
74+
public void Dispose()
75+
{
76+
syncedEntryBase.BoxedEntry.ConfigFile.SettingChanged -= OnSettingChanged;
77+
syncedEntryBase.SyncEnabledChanged -= OnEntrySyncEnabledChanged;
78+
}
79+
80+
void OnSettingChanged(object sender, SettingChangedEventArgs args)
81+
{
82+
if (!ReferenceEquals(syncedEntryBase.BoxedEntry, args.ChangedSetting)) return;
83+
deltas[index] = syncedEntryBase.ToDelta();
84+
}
85+
86+
void OnEntrySyncEnabledChanged(object sender, EventArgs args)
87+
{
88+
deltas[index] = syncedEntryBase.ToDelta();
89+
}
90+
}
91+
5492
public override void OnNetworkSpawn()
5593
{
5694
base.OnNetworkSpawn();
@@ -65,17 +103,8 @@ public override void OnNetworkSpawn()
65103
{
66104
var currentIndex = _deltas.Count;
67105
_deltas.Add(syncedEntryBase.ToDelta());
68-
69-
syncedEntryBase.BoxedEntry.ConfigFile.SettingChanged += (_, args) =>
70-
{
71-
if (!ReferenceEquals(syncedEntryBase.BoxedEntry, args.ChangedSetting)) return;
72-
_deltas[currentIndex] = syncedEntryBase.ToDelta();
73-
};
74-
75-
syncedEntryBase.SyncEnabledChanged += (_, args) =>
76-
{
77-
_deltas[currentIndex] = syncedEntryBase.ToDelta();
78-
};
106+
var eventHandlerHelper = new EventHandlerHelper(_deltas, currentIndex, syncedEntryBase);
107+
_eventHandlers.Add(eventHandlerHelper);
79108
}
80109

81110
InitialSyncCompletedHandler?.Invoke(this, EventArgs.Empty);
@@ -98,16 +127,44 @@ public override void OnNetworkSpawn()
98127

99128
public override void OnNetworkDespawn()
100129
{
130+
EnsureEntryContainer();
131+
132+
if (IsServer)
133+
{
134+
foreach (var eventHandlerHelper in _eventHandlers)
135+
{
136+
eventHandlerHelper.Dispose();
137+
}
138+
139+
_eventHandlers.Clear();
140+
141+
// NetworkList and NetworkVariable can not be modified now because the network manager has already shut down.
142+
// See https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/pull/3502
143+
// The fix applies to NGO 1.14.0+, but Lethal Company v73 is on NGO 1.12.0
144+
// Should not be needed anyway, since this component isn't being reused between network sessions.
145+
// _deltas.Clear();
146+
// _syncEnabled.Value = false;
147+
}
148+
else if (IsClient)
149+
{
150+
DisableOverrides();
151+
152+
foreach (var delta in _deltas)
153+
{
154+
ResetOverrideValue(delta);
155+
}
156+
157+
_deltas.OnListChanged -= OnClientDeltaListChanged;
158+
_syncEnabled.OnValueChanged -= OnSyncEnabledChanged;
159+
}
160+
101161
base.OnNetworkDespawn();
102162
}
103163

104164
public override void OnDestroy()
105165
{
106-
DisableOverrides();
107-
foreach (var delta in _deltas)
108-
{
109-
ResetOverrideValue(delta);
110-
}
166+
// The content of this method has moved to OnNetworkDespawn.
167+
// Keep this method around for ABI compatibility though.
111168
base.OnDestroy();
112169
}
113170

0 commit comments

Comments
 (0)