From 735ef09d42a9a4aea2258427cfcc0842d4467f83 Mon Sep 17 00:00:00 2001 From: Fildrance Date: Sat, 2 Aug 2025 19:35:38 +0300 Subject: [PATCH] Better unsubscription for multiple ConfigurationManager subscriptions (#6115) * feat: new method or aggregating multiple config changed subscriptions into one disposable object or more slim unsubscribing code * refactor: moved nested private class declaration to bottom of class * refactor: reusing stateful object in tests is not smart * fix: invalid code for forming new array during InvokeList.Remove call * refactor: extracted new sub-multiple builder into configuration manager extensions * refactor: remove unused code * refactor: removed UnSubscribeActionsDelegates * refactor: whitespaces and renaming --------- Co-authored-by: pa.pecherskij --- RELEASE-NOTES.md | 2 +- Robust.Shared/Collections/InvokeList.cs | 14 ++- .../ConfigurationManagerExtensions.cs | 91 +++++++++++++++++++ .../Configuration/ConfigurationManagerTest.cs | 68 +++++++++++++- 4 files changed, 165 insertions(+), 10 deletions(-) create mode 100644 Robust.Shared/Configuration/ConfigurationManagerExtensions.cs diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 59be9be4b..7560d51a7 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -14,7 +14,7 @@ Don't change the format without looking at the script! ### New features -*None yet* +* added **IConfigurationManager**.*SubscribeMultiple* ext. method to provide simpler way to unsubscribe from multiple cvar at once ### Bugfixes diff --git a/Robust.Shared/Collections/InvokeList.cs b/Robust.Shared/Collections/InvokeList.cs index d5a064728..e3d739ce2 100644 --- a/Robust.Shared/Collections/InvokeList.cs +++ b/Robust.Shared/Collections/InvokeList.cs @@ -1,4 +1,4 @@ -using System; +using System; namespace Robust.Shared.Collections; @@ -77,7 +77,7 @@ internal struct InvokeList for (var i = 0; i < _entries.Length; i++) { var entry = _entries[i]; - if (equality.Equals(entry)) + if (equality.Equals(entry.Equality)) { entryIdx = i; break; @@ -94,14 +94,12 @@ internal struct InvokeList // Create new backing array and copy stuff into it. var newEntries = new Entry[_entries.Length - 1]; - for (var i = 0; i < entryIdx; i++) + for (int srcIdx = 0, dstIdx = 0; dstIdx < newEntries.Length; srcIdx++, dstIdx++) { - newEntries[i] = _entries[i]; - } + if (srcIdx == entryIdx) + srcIdx++; - for (var i = entryIdx + 1; i < _entries.Length; i++) - { - newEntries[entryIdx - 1] = _entries[entryIdx]; + newEntries[dstIdx] = _entries[srcIdx]; } return new InvokeList diff --git a/Robust.Shared/Configuration/ConfigurationManagerExtensions.cs b/Robust.Shared/Configuration/ConfigurationManagerExtensions.cs new file mode 100644 index 000000000..54c5838cc --- /dev/null +++ b/Robust.Shared/Configuration/ConfigurationManagerExtensions.cs @@ -0,0 +1,91 @@ +using System; +using System.Collections.Generic; + +namespace Robust.Shared.Configuration; + +public static class ConfigurationManagerExtensions +{ + /// + /// Subscribe to multiple cvar in succession and dispose object to unsubscribe from all of them when needed. + /// + public static ConfigurationMultiSubscriptionBuilder SubscribeMultiple(this IConfigurationManager manager) + { + return new ConfigurationMultiSubscriptionBuilder(manager); + } +} + +/// +/// Container for batch-unsubscription of config changed events. +/// Call Dispose() when subscriptions are not needed anymore. +/// +public sealed class ConfigurationMultiSubscriptionBuilder(IConfigurationManager manager) : IDisposable +{ + private readonly List _unsubscribeActions = []; + + /// > + public ConfigurationMultiSubscriptionBuilder OnValueChanged( + CVarDef cVar, + CVarChanged onValueChanged, + bool invokeImmediately = false + ) + where T : notnull + { + manager.OnValueChanged(cVar, onValueChanged, invokeImmediately); + + _unsubscribeActions.Add(() => manager.UnsubValueChanged(cVar, onValueChanged)); + return this; + } + + /// > + public ConfigurationMultiSubscriptionBuilder OnValueChanged( + string name, + CVarChanged onValueChanged, + bool invokeImmediately = false + ) + where T : notnull + { + manager.OnValueChanged(name, onValueChanged, invokeImmediately); + + _unsubscribeActions.Add(() => manager.UnsubValueChanged(name, onValueChanged)); + return this; + } + + /// > + public ConfigurationMultiSubscriptionBuilder OnValueChanged( + CVarDef cVar, + Action onValueChanged, + bool invokeImmediately = false + ) + where T : notnull + { + manager.OnValueChanged(cVar, onValueChanged, invokeImmediately); + + _unsubscribeActions.Add(() => manager.UnsubValueChanged(cVar, onValueChanged)); + return this; + } + + /// > + public ConfigurationMultiSubscriptionBuilder OnValueChanged( + string name, + Action onValueChanged, + bool invokeImmediately = false + ) + where T : notnull + { + manager.OnValueChanged(name, onValueChanged, invokeImmediately); + + _unsubscribeActions.Add(() => manager.UnsubValueChanged(name, onValueChanged)); + + return this; + } + + /// + public void Dispose() + { + foreach (var action in _unsubscribeActions) + { + action(); + } + _unsubscribeActions.Clear(); + } +} diff --git a/Robust.UnitTesting/Shared/Configuration/ConfigurationManagerTest.cs b/Robust.UnitTesting/Shared/Configuration/ConfigurationManagerTest.cs index dddde03e1..c8f1d12d4 100644 --- a/Robust.UnitTesting/Shared/Configuration/ConfigurationManagerTest.cs +++ b/Robust.UnitTesting/Shared/Configuration/ConfigurationManagerTest.cs @@ -43,10 +43,76 @@ namespace Robust.UnitTesting.Shared.Configuration Assert.That(timesRan, Is.EqualTo(1), "UnsubValueChanged did not unsubscribe!"); } + [Test] + public void TestSubscribe_SubscribeMultipleThenUnsubscribe() + { + var mgr = MakeCfg(); + + mgr.RegisterCVar("foo.bar", 5); + + var lastValueBar1 = 0; + var lastValueBar2 = 0; + var lastValueBar3 = 0; + var lastValueBar4 = 0; + + var subscription = mgr.SubscribeMultiple() + .OnValueChanged("foo.bar", value => lastValueBar1 = value) + .OnValueChanged("foo.bar", value => lastValueBar2 = value) + .OnValueChanged("foo.bar", value => lastValueBar3 = value) + .OnValueChanged("foo.bar", value => lastValueBar4 = value); + + mgr.SetCVar("foo.bar", 1); + + Assert.That(lastValueBar1, Is.EqualTo(1), "OnValueChanged value was wrong!"); + Assert.That(lastValueBar2, Is.EqualTo(1), "OnValueChanged value was wrong!"); + Assert.That(lastValueBar3, Is.EqualTo(1), "OnValueChanged value was wrong!"); + Assert.That(lastValueBar4, Is.EqualTo(1), "OnValueChanged value was wrong!"); + + subscription.Dispose(); + + mgr.SetCVar("foo.bar", 10); + + Assert.That(lastValueBar1, Is.EqualTo(1), "OnValueChanged value was wrong!"); + Assert.That(lastValueBar2, Is.EqualTo(1), "OnValueChanged value was wrong!"); + Assert.That(lastValueBar3, Is.EqualTo(1), "OnValueChanged value was wrong!"); + Assert.That(lastValueBar4, Is.EqualTo(1), "OnValueChanged value was wrong!"); + } + + [Test] + public void TestSubscribe_Unsubscribe() + { + var mgr = MakeCfg(); + + mgr.RegisterCVar("foo.bar", 5); + mgr.RegisterCVar("foo.foo", 2); + + var lastValueBar = 0; + var lastValueFoo = 0; + + var subscription = mgr.SubscribeMultiple() + .OnValueChanged("foo.bar", value => lastValueBar = value) + .OnValueChanged("foo.foo", value => lastValueFoo = value); + + mgr.SetCVar("foo.bar", 1); + mgr.SetCVar("foo.foo", 3); + + Assert.That(lastValueBar, Is.EqualTo(1), "OnValueChanged value was wrong!"); + Assert.That(lastValueFoo, Is.EqualTo(3), "OnValueChanged value was wrong!"); + + subscription.Dispose(); + + mgr.SetCVar("foo.bar", 10); + mgr.SetCVar("foo.foo", 30); + + Assert.That(lastValueBar, Is.EqualTo(1), "OnValueChanged value was wrong!"); + Assert.That(lastValueFoo, Is.EqualTo(3), "OnValueChanged value was wrong!"); + } + [Test] public void TestOverrideDefaultValue() { var mgr = MakeCfg(); + mgr.RegisterCVar("foo.bar", 5); var value = 0; @@ -71,7 +137,7 @@ namespace Robust.UnitTesting.Shared.Configuration Assert.That(mgr.GetCVar("foo.bar"), Is.EqualTo(7)); } - private ConfigurationManager MakeCfg() + private IConfigurationManager MakeCfg() { var collection = new DependencyCollection(); collection.RegisterInstance(new Mock().Object);