From 5bc57b5d5de5158bf28753599695117a40a33f75 Mon Sep 17 00:00:00 2001 From: Ruzihm Date: Tue, 6 Jan 2026 02:35:19 -0500 Subject: [PATCH 1/7] override containskey/value and remove for verb list classes to make verb removal more correct --- OpenDreamRuntime/Objects/Types/DreamList.cs | 53 +++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/OpenDreamRuntime/Objects/Types/DreamList.cs b/OpenDreamRuntime/Objects/Types/DreamList.cs index 7dc0e2e830..0964f4de69 100644 --- a/OpenDreamRuntime/Objects/Types/DreamList.cs +++ b/OpenDreamRuntime/Objects/Types/DreamList.cs @@ -652,6 +652,21 @@ public override IEnumerable EnumerateValues() { yield return new(verb); } + public override bool ContainsKey(DreamValue value) { + if (!value.TryGetValueAsInteger(out var index)) { + return false; + } + + return 1 <= index && index <= Verbs.Count; + } + + public override bool ContainsValue(DreamValue value) { + if (!value.TryGetValueAsProc(out var verb)) + return false; + + return Verbs.Contains(verb); + } + public override void SetValue(DreamValue key, DreamValue value, bool allowGrowth = false) { throw new Exception("Cannot set the values of a verbs list"); } @@ -667,6 +682,14 @@ public override void AddValue(DreamValue value) { _verbSystem?.UpdateClientVerbs(_client); } + public override void RemoveValue(DreamValue value) { + if (!value.TryGetValueAsProc(out var verb)) + return; + + if (Verbs.Remove(verb)) + _verbSystem?.UpdateClientVerbs(_client); + } + public override void Cut(int start = 1, int end = 0) { int verbCount = Verbs.Count + 1; if (end == 0 || end > verbCount) end = verbCount; @@ -716,6 +739,23 @@ public override IEnumerable EnumerateValues() { } } + public override bool ContainsKey(DreamValue value) { + if (!value.TryGetValueAsInteger(out var index)) { + return false; + } + + return 1 <= index && index <= GetVerbs().Length; + } + + public override bool ContainsValue(DreamValue value) { + if (!value.TryGetValueAsProc(out var verb)) + return false; + if (verb?.VerbId == null) + return false; + + return GetVerbs().Contains(verb.VerbId.Value); + } + public override void SetValue(DreamValue key, DreamValue value, bool allowGrowth = false) { throw new Exception("Cannot set the values of a verbs list"); } @@ -734,6 +774,19 @@ public override void AddValue(DreamValue value) { }); } + public override void RemoveValue(DreamValue value) { + if (!value.TryGetValueAsProc(out var verb)) + return; + + if (verb?.VerbId == null) { + return; + } + + atomManager.UpdateAppearance(atom, appearance => { + appearance.Verbs.Remove(verb.VerbId.Value); + }); + } + public override void Cut(int start = 1, int end = 0) { atomManager.UpdateAppearance(atom, appearance => { int count = appearance.Verbs.Count + 1; From 25906ef9d045a24a865e7a0d5380c47592ff33a9 Mon Sep 17 00:00:00 2001 From: Ruzihm Date: Tue, 6 Jan 2026 02:48:48 -0500 Subject: [PATCH 2/7] spacing --- OpenDreamRuntime/Objects/Types/DreamList.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/OpenDreamRuntime/Objects/Types/DreamList.cs b/OpenDreamRuntime/Objects/Types/DreamList.cs index 0964f4de69..6115e1a304 100644 --- a/OpenDreamRuntime/Objects/Types/DreamList.cs +++ b/OpenDreamRuntime/Objects/Types/DreamList.cs @@ -777,7 +777,6 @@ public override void AddValue(DreamValue value) { public override void RemoveValue(DreamValue value) { if (!value.TryGetValueAsProc(out var verb)) return; - if (verb?.VerbId == null) { return; } From 02a7d35b2dd3018167b501fcdd35b1f9c4a7087a Mon Sep 17 00:00:00 2001 From: Ruzihm Date: Tue, 6 Jan 2026 10:16:07 -0500 Subject: [PATCH 3/7] dont assume verb list is unique when removing, just in case.... and also fix listx.Remove(listx) by copying the argument list before reading --- OpenDreamRuntime/Objects/Types/DreamList.cs | 15 +++++++++++---- .../Procs/Native/DreamProcNativeList.cs | 9 +++++++-- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/OpenDreamRuntime/Objects/Types/DreamList.cs b/OpenDreamRuntime/Objects/Types/DreamList.cs index 6115e1a304..514c3faa28 100644 --- a/OpenDreamRuntime/Objects/Types/DreamList.cs +++ b/OpenDreamRuntime/Objects/Types/DreamList.cs @@ -686,8 +686,12 @@ public override void RemoveValue(DreamValue value) { if (!value.TryGetValueAsProc(out var verb)) return; - if (Verbs.Remove(verb)) + var valueIndex = Verbs.LastIndexOf(verb); + + if (valueIndex != -1) { + Verbs.RemoveAt(valueIndex); _verbSystem?.UpdateClientVerbs(_client); + } } public override void Cut(int start = 1, int end = 0) { @@ -750,7 +754,7 @@ public override bool ContainsKey(DreamValue value) { public override bool ContainsValue(DreamValue value) { if (!value.TryGetValueAsProc(out var verb)) return false; - if (verb?.VerbId == null) + if (verb.VerbId == null) return false; return GetVerbs().Contains(verb.VerbId.Value); @@ -777,12 +781,15 @@ public override void AddValue(DreamValue value) { public override void RemoveValue(DreamValue value) { if (!value.TryGetValueAsProc(out var verb)) return; - if (verb?.VerbId == null) { + if (verb.VerbId == null) { return; } atomManager.UpdateAppearance(atom, appearance => { - appearance.Verbs.Remove(verb.VerbId.Value); + var valueIndex = appearance.Verbs.LastIndexOf(verb.VerbId.Value); + + if (valueIndex != -1) + appearance.Verbs.RemoveAt(valueIndex); }); } diff --git a/OpenDreamRuntime/Procs/Native/DreamProcNativeList.cs b/OpenDreamRuntime/Procs/Native/DreamProcNativeList.cs index 7f7ee507f4..9537f843d0 100644 --- a/OpenDreamRuntime/Procs/Native/DreamProcNativeList.cs +++ b/OpenDreamRuntime/Procs/Native/DreamProcNativeList.cs @@ -1,4 +1,5 @@ -using System.Text; +using System.Linq; +using System.Text; using OpenDreamRuntime.Objects; using OpenDreamRuntime.Objects.Types; using DreamValueTypeFlag = OpenDreamRuntime.DreamValue.DreamValueTypeFlag; @@ -150,7 +151,11 @@ private static int ListRemove(IDreamList list, ReadOnlySpan args) { var itemRemoved = 0; foreach (var argument in args) { if (argument.TryGetValueAsDreamList(out var argumentList)) { - foreach (DreamValue value in argumentList.EnumerateValues()) { + // In case this is a "listx.Remove(listx)" situation, copy the contents here to avoid modification while enumerating + // TODO: check for that case first to avoid unnecessary copy? + var subtraction = argumentList.EnumerateValues().ToList(); + + foreach (DreamValue value in subtraction) { if (list.ContainsValue(value)) { list.RemoveValue(value); From 9ada51d27ccc06544fc38c9dfbbc7a6294dc9ffc Mon Sep 17 00:00:00 2001 From: Ruzihm Date: Tue, 6 Jan 2026 11:31:39 -0500 Subject: [PATCH 4/7] Adds verb refresh to client side appearance update --- OpenDreamClient/ClientVerbSystem.cs | 4 ++++ OpenDreamClient/Rendering/ClientAppearanceSystem.cs | 5 +++++ OpenDreamClient/Rendering/DreamIcon.cs | 1 + 3 files changed, 10 insertions(+) diff --git a/OpenDreamClient/ClientVerbSystem.cs b/OpenDreamClient/ClientVerbSystem.cs index f92212d5d5..154d6bf4cc 100644 --- a/OpenDreamClient/ClientVerbSystem.cs +++ b/OpenDreamClient/ClientVerbSystem.cs @@ -242,6 +242,10 @@ private void OnUpdateClientVerbsEvent(UpdateClientVerbsEvent e) { _interfaceManager.DefaultInfo?.RefreshVerbs(this); } + public void RefreshVerbs() { + _interfaceManager.DefaultInfo?.RefreshVerbs(this); + } + private void OnLocalPlayerAttached(EntityUid obj) { // Our mob changed, update our verb panels // A little hacky, but also wait half a second for verb information about our mob to arrive diff --git a/OpenDreamClient/Rendering/ClientAppearanceSystem.cs b/OpenDreamClient/Rendering/ClientAppearanceSystem.cs index 40d1cc1803..5cd07fa7cb 100644 --- a/OpenDreamClient/Rendering/ClientAppearanceSystem.cs +++ b/OpenDreamClient/Rendering/ClientAppearanceSystem.cs @@ -68,6 +68,7 @@ public int GetAnimationFrame(IGameTiming gameTiming) { [Dependency] private readonly IMapManager _mapManager = default!; [Dependency] private readonly MapSystem _mapSystem = default!; [Dependency] private readonly IPrototypeManager _protoManager = default!; + [Dependency] private readonly ClientVerbSystem _verbSystem = default!; public override void Initialize() { UpdatesOutsidePrediction = true; @@ -363,4 +364,8 @@ public string GetName(ClientObjectReference reference) { public Flick? GetMovableFlick(EntityUid entity) { return _movableFlicks.GetValueOrDefault(entity); } + + public void RefreshVerbs() { + _verbSystem.RefreshVerbs(); + } } diff --git a/OpenDreamClient/Rendering/DreamIcon.cs b/OpenDreamClient/Rendering/DreamIcon.cs index 0d8be428c0..03488d8cb4 100644 --- a/OpenDreamClient/Rendering/DreamIcon.cs +++ b/OpenDreamClient/Rendering/DreamIcon.cs @@ -147,6 +147,7 @@ public void SetAppearance(uint? appearanceId, AtomDirection? parentDir = null) { } Appearance = appearance; + appearanceSystem.RefreshVerbs(); }); } From efb81cee9417c8ff877e78f47d809e00865c9307 Mon Sep 17 00:00:00 2001 From: Ruzihm Date: Tue, 6 Jan 2026 12:06:12 -0500 Subject: [PATCH 5/7] add unit test for list removing self --- Content.Tests/DMProject/Tests/Stdlib/List/remove.dm | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Content.Tests/DMProject/Tests/Stdlib/List/remove.dm b/Content.Tests/DMProject/Tests/Stdlib/List/remove.dm index 0b9180a093..6015d2d914 100644 --- a/Content.Tests/DMProject/Tests/Stdlib/List/remove.dm +++ b/Content.Tests/DMProject/Tests/Stdlib/List/remove.dm @@ -8,3 +8,7 @@ L = list(1,2,3,2,1) L.Remove(list(2)) ASSERT(L ~= list(1,2,3,1)) + + L = list(1,2,3,2,1) + L.Remove(L) + ASSERT(L ~= list()) \ No newline at end of file From 8cd6d268dd5ff349ab524bcb14fd1b2e752e8964 Mon Sep 17 00:00:00 2001 From: Ruzihm Date: Tue, 6 Jan 2026 15:07:49 -0500 Subject: [PATCH 6/7] refresh client verbs on timer instead of on appearance update --- OpenDreamClient/Rendering/ClientAppearanceSystem.cs | 13 +++++++++++-- OpenDreamClient/Rendering/DreamIcon.cs | 1 - 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/OpenDreamClient/Rendering/ClientAppearanceSystem.cs b/OpenDreamClient/Rendering/ClientAppearanceSystem.cs index 5cd07fa7cb..7ad3bacf81 100644 --- a/OpenDreamClient/Rendering/ClientAppearanceSystem.cs +++ b/OpenDreamClient/Rendering/ClientAppearanceSystem.cs @@ -11,6 +11,8 @@ using Robust.Client.Player; using Robust.Shared.Map; using Robust.Shared.Timing; +using Robust.Shared.Asynchronous; +using System.Threading.Tasks; namespace OpenDreamClient.Rendering; @@ -56,6 +58,7 @@ public int GetAnimationFrame(IGameTiming gameTiming) { private readonly Dictionary<(int X, int Y, int Z), Flick> _turfFlicks = new(); private readonly Dictionary _movableFlicks = new(); private bool _receivedAllAppearancesMsg; + private readonly float timeToRefreshVerbs = 3f; [Dependency] private readonly IEntityManager _entityManager = default!; [Dependency] private readonly IDreamResourceManager _dreamResourceManager = default!; @@ -69,6 +72,7 @@ public int GetAnimationFrame(IGameTiming gameTiming) { [Dependency] private readonly MapSystem _mapSystem = default!; [Dependency] private readonly IPrototypeManager _protoManager = default!; [Dependency] private readonly ClientVerbSystem _verbSystem = default!; + [Dependency] private readonly ITaskManager _taskManager = default!; public override void Initialize() { UpdatesOutsidePrediction = true; @@ -78,6 +82,8 @@ public override void Initialize() { SubscribeNetworkEvent(OnAnimation); SubscribeNetworkEvent(OnFlick); SubscribeLocalEvent(OnWorldAABB); + + _ = StartVerbRefresher(); } public override void Shutdown() { @@ -365,7 +371,10 @@ public string GetName(ClientObjectReference reference) { return _movableFlicks.GetValueOrDefault(entity); } - public void RefreshVerbs() { - _verbSystem.RefreshVerbs(); + private async Task StartVerbRefresher() { + while (true) { + await Task.Delay(TimeSpan.FromSeconds(timeToRefreshVerbs)); + _taskManager.RunOnMainThread(_verbSystem.RefreshVerbs); + } } } diff --git a/OpenDreamClient/Rendering/DreamIcon.cs b/OpenDreamClient/Rendering/DreamIcon.cs index 03488d8cb4..0d8be428c0 100644 --- a/OpenDreamClient/Rendering/DreamIcon.cs +++ b/OpenDreamClient/Rendering/DreamIcon.cs @@ -147,7 +147,6 @@ public void SetAppearance(uint? appearanceId, AtomDirection? parentDir = null) { } Appearance = appearance; - appearanceSystem.RefreshVerbs(); }); } From ecd791727abb66c57494cc5564a0969d0d56e42d Mon Sep 17 00:00:00 2001 From: Ruzihm Date: Tue, 6 Jan 2026 16:02:01 -0500 Subject: [PATCH 7/7] lint --- OpenDreamClient/Rendering/ClientAppearanceSystem.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/OpenDreamClient/Rendering/ClientAppearanceSystem.cs b/OpenDreamClient/Rendering/ClientAppearanceSystem.cs index 7ad3bacf81..642915f848 100644 --- a/OpenDreamClient/Rendering/ClientAppearanceSystem.cs +++ b/OpenDreamClient/Rendering/ClientAppearanceSystem.cs @@ -12,6 +12,7 @@ using Robust.Shared.Map; using Robust.Shared.Timing; using Robust.Shared.Asynchronous; +using System.Threading; using System.Threading.Tasks; namespace OpenDreamClient.Rendering; @@ -58,7 +59,7 @@ public int GetAnimationFrame(IGameTiming gameTiming) { private readonly Dictionary<(int X, int Y, int Z), Flick> _turfFlicks = new(); private readonly Dictionary _movableFlicks = new(); private bool _receivedAllAppearancesMsg; - private readonly float timeToRefreshVerbs = 3f; + private readonly float _timeToRefreshVerbs = 3f; [Dependency] private readonly IEntityManager _entityManager = default!; [Dependency] private readonly IDreamResourceManager _dreamResourceManager = default!; @@ -83,7 +84,7 @@ public override void Initialize() { SubscribeNetworkEvent(OnFlick); SubscribeLocalEvent(OnWorldAABB); - _ = StartVerbRefresher(); + _ = StartVerbRefresher(new()); } public override void Shutdown() { @@ -371,9 +372,12 @@ public string GetName(ClientObjectReference reference) { return _movableFlicks.GetValueOrDefault(entity); } - private async Task StartVerbRefresher() { + private async Task StartVerbRefresher(CancellationTokenSource cancelSource) { while (true) { - await Task.Delay(TimeSpan.FromSeconds(timeToRefreshVerbs)); + await Task.Delay(TimeSpan.FromSeconds(_timeToRefreshVerbs)); + if (cancelSource.IsCancellationRequested) + break; + _taskManager.RunOnMainThread(_verbSystem.RefreshVerbs); } }