From 9898ee8e5ad1510b22cc27ca5a04845d473a1135 Mon Sep 17 00:00:00 2001 From: ivan tkachenko Date: Fri, 8 Aug 2025 20:42:06 +0300 Subject: [PATCH 1/2] 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 - https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/pull/3502 Fixes #13 --- CSync/CSync/Lib/ConfigSyncBehaviour.cs | 78 ++++++++++++++++++++------ 1 file changed, 60 insertions(+), 18 deletions(-) diff --git a/CSync/CSync/Lib/ConfigSyncBehaviour.cs b/CSync/CSync/Lib/ConfigSyncBehaviour.cs index 29b123a..bb24538 100644 --- a/CSync/CSync/Lib/ConfigSyncBehaviour.cs +++ b/CSync/CSync/Lib/ConfigSyncBehaviour.cs @@ -1,6 +1,8 @@ +using BepInEx.Configuration; using System; using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; +using System.Linq; using Unity.Netcode; using UnityEngine; using LogLevel = BepInEx.Logging.LogLevel; @@ -36,7 +38,9 @@ public bool SyncEnabled } private readonly NetworkVariable _syncEnabled = new(); - private NetworkList _deltas = null!; + private readonly NetworkList _deltas = new(); + private readonly List<(ConfigFile, EventHandler)> _configFileDelegates = new(); + private readonly List<(SyncedEntryBase, EventHandler)> _entrySyncEnabledDelegates = new(); [MemberNotNull(nameof(EntryContainer))] private void EnsureEntryContainer() @@ -45,12 +49,6 @@ private void EnsureEntryContainer() throw new InvalidOperationException("Entry container has not been assigned."); } - private void Awake() - { - EnsureEntryContainer(); - _deltas = new NetworkList(); - } - public override void OnNetworkSpawn() { EnsureEntryContainer(); @@ -59,28 +57,30 @@ public override void OnNetworkSpawn() { _syncEnabled.Value = true; - foreach (var syncedEntryBase in EntryContainer.Values) + foreach (var (syncedEntryBase, index) in EntryContainer.Values.Select((e, i) => (e, i))) { - var currentIndex = _deltas.Count; _deltas.Add(syncedEntryBase.ToDelta()); - syncedEntryBase.BoxedEntry.ConfigFile.SettingChanged += (_, args) => + void configFileDelegate(object sender, SettingChangedEventArgs args) { if (!ReferenceEquals(syncedEntryBase.BoxedEntry, args.ChangedSetting)) return; - _deltas[currentIndex] = syncedEntryBase.ToDelta(); - }; + _deltas[index] = syncedEntryBase.ToDelta(); + } + var configFile = syncedEntryBase.BoxedEntry.ConfigFile; + configFile.SettingChanged += configFileDelegate; + _configFileDelegates.Add((configFile, configFileDelegate)); - syncedEntryBase.SyncEnabledChanged += (_, args) => + void entrySyncEnabledDelegate(object sender, EventArgs args) { - _deltas[currentIndex] = syncedEntryBase.ToDelta(); - }; + _deltas[index] = syncedEntryBase.ToDelta(); + } + syncedEntryBase.SyncEnabledChanged += entrySyncEnabledDelegate; + _entrySyncEnabledDelegates.Add((syncedEntryBase, entrySyncEnabledDelegate)); } InitialSyncCompletedHandler?.Invoke(this, EventArgs.Empty); - return; } - - if (IsClient) + else if (IsClient) { _syncEnabled.OnValueChanged += OnSyncEnabledChanged; _deltas.OnListChanged += OnClientDeltaListChanged; @@ -94,6 +94,48 @@ public override void OnNetworkSpawn() InitialSyncCompletedHandler?.Invoke(this, EventArgs.Empty); } + + base.OnNetworkSpawn(); + } + + public override void OnNetworkDespawn() + { + EnsureEntryContainer(); + + if (IsServer) + { + foreach (var (configFile, configFileDelegate) in _configFileDelegates) + { + configFile.SettingChanged -= configFileDelegate; + } + + foreach (var (syncedEntryBase, entrySyncEnabledDelegate) in _entrySyncEnabledDelegates) + { + syncedEntryBase.SyncEnabledChanged -= entrySyncEnabledDelegate; + } + + _configFileDelegates.Clear(); + _entrySyncEnabledDelegates.Clear(); + // NetworkList and NetworkVariable can not be modified now because the network manager has already shut down. + // See https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/pull/3502 + // Should not be needed anyway, since this component isn't being reused between network sessions. + // _deltas.Clear(); + // _syncEnabled.Value = false; + } + else if (IsClient) + { + DisableOverrides(); + + foreach (var delta in _deltas) + { + ResetOverrideValue(delta); + } + + _deltas.OnListChanged -= OnClientDeltaListChanged; + _syncEnabled.OnValueChanged -= OnSyncEnabledChanged; + } + + base.OnNetworkDespawn(); } public override void OnDestroy() From 1c7cb9b647b042dea9147ec80a14392fbe92a3c0 Mon Sep 17 00:00:00 2001 From: ivan tkachenko Date: Tue, 16 Sep 2025 19:07:23 +0300 Subject: [PATCH 2/2] rename and partially revert Awake --- CSync/CSync/Lib/ConfigSyncBehaviour.cs | 37 +++++++++++++++----------- 1 file changed, 22 insertions(+), 15 deletions(-) diff --git a/CSync/CSync/Lib/ConfigSyncBehaviour.cs b/CSync/CSync/Lib/ConfigSyncBehaviour.cs index bb24538..3fe5fb5 100644 --- a/CSync/CSync/Lib/ConfigSyncBehaviour.cs +++ b/CSync/CSync/Lib/ConfigSyncBehaviour.cs @@ -39,8 +39,8 @@ public bool SyncEnabled private readonly NetworkVariable _syncEnabled = new(); private readonly NetworkList _deltas = new(); - private readonly List<(ConfigFile, EventHandler)> _configFileDelegates = new(); - private readonly List<(SyncedEntryBase, EventHandler)> _entrySyncEnabledDelegates = new(); + private readonly List<(ConfigFile, EventHandler)> _settingChangedListeners = new(); + private readonly List<(SyncedEntryBase, EventHandler)> _entrySyncEnabledChangedListeners = new(); [MemberNotNull(nameof(EntryContainer))] private void EnsureEntryContainer() @@ -49,6 +49,11 @@ private void EnsureEntryContainer() throw new InvalidOperationException("Entry container has not been assigned."); } + private void Awake() + { + EnsureEntryContainer(); + } + public override void OnNetworkSpawn() { EnsureEntryContainer(); @@ -57,25 +62,27 @@ public override void OnNetworkSpawn() { _syncEnabled.Value = true; - foreach (var (syncedEntryBase, index) in EntryContainer.Values.Select((e, i) => (e, i))) + var index = 0; + foreach (var syncedEntryBase in EntryContainer.Values) { _deltas.Add(syncedEntryBase.ToDelta()); - void configFileDelegate(object sender, SettingChangedEventArgs args) + void onSettingChanged(object sender, SettingChangedEventArgs args) { if (!ReferenceEquals(syncedEntryBase.BoxedEntry, args.ChangedSetting)) return; _deltas[index] = syncedEntryBase.ToDelta(); } var configFile = syncedEntryBase.BoxedEntry.ConfigFile; - configFile.SettingChanged += configFileDelegate; - _configFileDelegates.Add((configFile, configFileDelegate)); + configFile.SettingChanged += onSettingChanged; + _settingChangedListeners.Add((configFile, onSettingChanged)); - void entrySyncEnabledDelegate(object sender, EventArgs args) + void onEntrySyncEnabledChanged(object sender, EventArgs args) { _deltas[index] = syncedEntryBase.ToDelta(); } - syncedEntryBase.SyncEnabledChanged += entrySyncEnabledDelegate; - _entrySyncEnabledDelegates.Add((syncedEntryBase, entrySyncEnabledDelegate)); + syncedEntryBase.SyncEnabledChanged += onEntrySyncEnabledChanged; + _entrySyncEnabledChangedListeners.Add((syncedEntryBase, onEntrySyncEnabledChanged)); + index++; } InitialSyncCompletedHandler?.Invoke(this, EventArgs.Empty); @@ -104,18 +111,18 @@ public override void OnNetworkDespawn() if (IsServer) { - foreach (var (configFile, configFileDelegate) in _configFileDelegates) + foreach (var (configFile, onSettingChanged) in _settingChangedListeners) { - configFile.SettingChanged -= configFileDelegate; + configFile.SettingChanged -= onSettingChanged; } - foreach (var (syncedEntryBase, entrySyncEnabledDelegate) in _entrySyncEnabledDelegates) + foreach (var (syncedEntryBase, onEntrySyncEnabledChanged) in _entrySyncEnabledChangedListeners) { - syncedEntryBase.SyncEnabledChanged -= entrySyncEnabledDelegate; + syncedEntryBase.SyncEnabledChanged -= onEntrySyncEnabledChanged; } - _configFileDelegates.Clear(); - _entrySyncEnabledDelegates.Clear(); + _settingChangedListeners.Clear(); + _entrySyncEnabledChangedListeners.Clear(); // NetworkList and NetworkVariable can not be modified now because the network manager has already shut down. // See https://github.com/Unity-Technologies/com.unity.netcode.gameobjects/pull/3502 // Should not be needed anyway, since this component isn't being reused between network sessions.