Move moving unbuckling to update to avoid an event bus concurrent modification exception (#1509)

Content.IntegrationTests/Tests/BuckleTest.cs

@@ -146,6 +146,22 @@ namespace Content.IntegrationTests.Tests
                 Assert.True(ActionBlockerSystem.CanMove(human));
                 Assert.True(ActionBlockerSystem.CanChangeDirection(human));
                 Assert.True(EffectBlockerSystem.CanFall(human));
+
+                // Re-buckle
+                Assert.True(buckle.TryBuckle(human, chair));
+
+                // Move away from the chair
+                human.Transform.WorldPosition += (1, 0);
+            });
+
+            server.RunTicks(1);
+
+            server.Assert(() =>
+            {
+                // No longer buckled
+                Assert.False(buckle.Buckled);
+                Assert.Null(buckle.BuckledTo);
+                Assert.IsEmpty(strap.BuckledEntities);
             });
 
             await server.WaitIdleAsync();

Content.Server/GameObjects/Components/Buckle/BuckleComponent.cs

@@ -3,7 +3,6 @@ using System;
 using Content.Server.GameObjects.Components.GUI;
 using Content.Server.GameObjects.Components.Mobs;
 using Content.Server.GameObjects.Components.Strap;
-using Content.Server.GameObjects.EntitySystems;
 using Content.Server.Interfaces;
 using Content.Server.Mobs;
 using Content.Server.Utility;
@@ -18,12 +17,13 @@ using Robust.Server.GameObjects.EntitySystemMessages;
 using Robust.Server.GameObjects.EntitySystems;
 using Robust.Shared.Containers;
 using Robust.Shared.GameObjects;
+using Robust.Shared.GameObjects.Components.Transform;
 using Robust.Shared.GameObjects.Systems;
 using Robust.Shared.Interfaces.GameObjects;
+using Robust.Shared.Interfaces.Map;
 using Robust.Shared.Interfaces.Timing;
 using Robust.Shared.IoC;
 using Robust.Shared.Localization;
-using Robust.Shared.Map;
 using Robust.Shared.Maths;
 using Robust.Shared.Serialization;
 using Robust.Shared.ViewVariables;
@@ -38,6 +38,7 @@ namespace Content.Server.GameObjects.Components.Buckle
         [Dependency] private readonly IEntitySystemManager _entitySystem = default!;
         [Dependency] private readonly IGameTiming _gameTiming = default!;
         [Dependency] private readonly IServerNotifyManager _notifyManager = default!;
+        [Dependency] private readonly IMapManager _mapManager = default!;
 #pragma warning restore 649
 
         private int _size;
@@ -90,6 +91,13 @@ namespace Content.Server.GameObjects.Components.Buckle
         [ViewVariables]
         private bool ContainerChanged { get; set; }
 
+        /// <summary>
+        ///     True if the entity was forcefully moved while buckled and should
+        ///     unbuckle next update, false otherwise
+        /// </summary>
+        [ViewVariables]
+        private bool Moved { get; set; }
+
         /// <summary>
         ///     The amount of space that this entity occupies in a
         ///     <see cref="StrapComponent"/>.
@@ -275,6 +283,8 @@ namespace Content.Server.GameObjects.Components.Buckle
 
             SendMessage(new BuckleMessage(Owner, to));
 
+            Owner.EntityManager.EventBus.SubscribeEvent<MoveEvent>(EventSource.Local, this, MoveEvent);
+
             return true;
         }
 
@@ -359,6 +369,8 @@ namespace Content.Server.GameObjects.Components.Buckle
 
             SendMessage(new UnbuckleMessage(Owner, oldBuckledTo.Owner));
 
+            Owner.EntityManager.EventBus.UnsubscribeEvent<MoveEvent>(EventSource.Local, this);
+
             return true;
         }
 
@@ -386,6 +398,33 @@ namespace Content.Server.GameObjects.Components.Buckle
             return TryBuckle(user, to);
         }
 
+        /// <summary>
+        ///     Checks if a buckled entity should be unbuckled from moving
+        ///     too far from its strap.
+        /// </summary>
+        /// <param name="moveEvent">The move event of a buckled entity.</param>
+        private void MoveEvent(MoveEvent moveEvent)
+        {
+            if (moveEvent.Sender != Owner)
+            {
+                return;
+            }
+
+            if (BuckledTo == null || !BuckleOffset.HasValue)
+            {
+                return;
+            }
+
+            var bucklePosition = BuckledTo.Owner.Transform.GridPosition.Offset(BuckleOffset.Value);
+
+            if (moveEvent.NewPosition.InRange(_mapManager, bucklePosition, 0.2f))
+            {
+                return;
+            }
+
+            Moved = true;
+        }
+
         /// <summary>
         ///     Called when the owner is inserted or removed from a container,
         ///     to synchronize the state of buckling.
@@ -409,7 +448,18 @@ namespace Content.Server.GameObjects.Components.Buckle
         /// </summary>
         public void Update()
         {
-            if (!ContainerChanged || BuckledTo == null)
+            if (BuckledTo == null)
+            {
+                return;
+            }
+
+            if (Moved)
+            {
+                TryUnbuckle(Owner, true);
+                return;
+            }
+
+            if (!ContainerChanged)
             {
                 return;
             }

Content.Server/GameObjects/EntitySystems/BuckleSystem.cs

@@ -13,34 +13,6 @@ namespace Content.Server.GameObjects.EntitySystems
     [UsedImplicitly]
     public class BuckleSystem : EntitySystem
     {
-#pragma warning disable 649
-        [Dependency] private readonly IMapManager _mapManager;
-#pragma warning restore 649
-
-        /// <summary>
-        ///     Checks if a buckled entity should be unbuckled from moving
-        ///     too far from its strap.
-        /// </summary>
-        /// <param name="moveEvent">The move event of a buckled entity.</param>
-        private void MoveEvent(MoveEvent moveEvent)
-        {
-            if (!moveEvent.Sender.TryGetComponent(out BuckleComponent buckle) ||
-                buckle.BuckledTo == null ||
-                !buckle.BuckleOffset.HasValue)
-            {
-                return;
-            }
-
-            var bucklePosition = buckle.BuckledTo.Owner.Transform.GridPosition.Offset(buckle.BuckleOffset.Value);
-
-            if (moveEvent.NewPosition.InRange(_mapManager, bucklePosition, 0.2f))
-            {
-                return;
-            }
-
-            buckle.TryUnbuckle(buckle.Owner, true);
-        }
-
         public override void Initialize()
         {
             base.Initialize();
@@ -49,8 +21,6 @@ namespace Content.Server.GameObjects.EntitySystems
 
             UpdatesAfter.Add(typeof(InteractionSystem));
             UpdatesAfter.Add(typeof(InputSystem));
-
-            SubscribeLocalEvent<MoveEvent>(MoveEvent);
         }
 
         public override void Update(float frameTime)