Add methods to transfer actions between containers (space-wizards#20901)

Content.Client/UserInterface/Systems/Actions/ActionUIController.cs

@@ -769,10 +769,9 @@ private bool OnMenuBeginDrag()
     {
         if (_actionsSystem != null && _actionsSystem.TryGetActionData(_menuDragHelper.Dragged?.ActionId, out var action))
         {
-            if (action.EntityIcon is {} entIcon)
+            if (EntityManager.TryGetComponent(action.EntityIcon, out SpriteComponent? sprite))
             {
-                _dragShadow.Texture = EntityManager.GetComponent<SpriteComponent>(entIcon).Icon?
-                    .GetFrame(RsiDirection.South, 0);
+                _dragShadow.Texture = sprite.Icon?.GetFrame(RsiDirection.South, 0);
             }
             else if (action.Icon != null)
             {

Content.Shared/Actions/ActionContainerSystem.cs

@@ -1,4 +1,5 @@
 using System.Diagnostics.CodeAnalysis;
+using System.Linq;
 using Robust.Shared.Containers;
 using Robust.Shared.Network;
 using Robust.Shared.Timing;
@@ -15,6 +16,7 @@ public sealed class ActionContainerSystem : EntitySystem
     [Dependency] private readonly SharedContainerSystem _container = default!;
     [Dependency] private readonly SharedActionsSystem _actions = default!;
     [Dependency] private readonly INetManager _netMan = default!;
+    [Dependency] private readonly SharedTransformSystem _transform = default!;
 
     public override void Initialize()
     {
@@ -96,18 +98,69 @@ public bool EnsureAction(EntityUid uid,
     }
 
     /// <summary>
-    /// Adds a pre-existing action to an action container.
+    /// Transfers an action from one container to another, while keeping the attached entity the same.
+    /// </summary>
+    /// <remarks>
+    /// While the attached entity should be the same at the end, this will actually remove and then re-grant the action.
+    /// </remarks>
+    public void TransferAction(
+        EntityUid actionId,
+        EntityUid newContainer,
+        BaseActionComponent? action = null,
+        ActionsContainerComponent? container = null)
+    {
+        if (!_actions.ResolveActionData(actionId, ref action))
+            return;
+
+        if (action.Container == newContainer)
+            return;
+
+        var attached = action.AttachedEntity;
+        if (!AddAction(newContainer, actionId, action, container))
+            return;
+
+        DebugTools.AssertEqual(action.Container, newContainer);
+        DebugTools.AssertNull(action.AttachedEntity);
+
+        if (attached != null)
+            _actions.AddActionDirect(attached.Value, actionId, action: action);
+
+        DebugTools.AssertEqual(action.AttachedEntity, attached);
+    }
+
+    /// <summary>
+    /// Transfers all actions from one container to another, while keeping the attached entity the same.
+    /// </summary>
+    /// &lt;remarks&gt;
+    /// While the attached entity should be the same at the end, this will actually remove and then re-grant the action.
+    /// &lt;/remarks&gt;
+    public void TransferAllActions(
+        EntityUid from,
+        EntityUid to,
+        ActionsContainerComponent? oldContainer = null,
+        ActionsContainerComponent? newContainer = null)
+    {
+        if (!Resolve(from, ref oldContainer) || !Resolve(to, ref newContainer))
+            return;
+
+        foreach (var action in oldContainer.Container.ContainedEntities.ToArray())
+        {
+            TransferAction(action, to, container: newContainer);
+        }
+
+        DebugTools.AssertEqual(oldContainer.Container.Count, 0);
+    }
+
+    /// <summary>
+    /// Adds a pre-existing action to an action container. If the action is already in some container it will first remove it.
     /// </summary>
     public bool AddAction(EntityUid uid, EntityUid actionId, BaseActionComponent? action = null, ActionsContainerComponent? comp = null)
     {
         if (!_actions.ResolveActionData(actionId, ref action))
             return false;
 
         if (action.Container != null)
-        {
-            Log.Error($"Attempted to insert an action {ToPrettyString(actionId)} that was already in a container {ToPrettyString(action.Container.Value)}");
-            return false;
-        }
+            RemoveAction(actionId, action);
 
         DebugTools.Assert(comp == null || comp.Owner == uid);
         comp ??= EnsureComp<ActionsContainerComponent>(uid);
@@ -124,6 +177,35 @@ public bool AddAction(EntityUid uid, EntityUid actionId, BaseActionComponent? ac
         return true;
     }
 
+    /// <summary>
+    /// Removes an action from its container and any action-performer and moves the action to null-space
+    /// </summary>
+    public void RemoveAction(EntityUid actionId, BaseActionComponent? action = null)
+    {
+        if (!_actions.ResolveActionData(actionId, ref action))
+            return;
+
+        if (action.Container == null)
+            return;
+
+        _transform.DetachParentToNull(actionId, Transform(actionId));
+
+        // Container removal events should have removed the action from the action container.
+        // However, just in case the container was already deleted we will still manually clear the container field
+        if (action.Container != null)
+        {
+            if (Exists(action.Container))
+                Log.Error($"Failed to remove action {ToPrettyString(actionId)} from its container {ToPrettyString(action.Container)}?");
+            action.Container = null;
+        }
+
+        // If the action was granted to some entity, then the removal from the container should have automatically removed it.
+        // However, if the action was granted without ever being placed in an action container, it will not have been removed.
+        // Therefore, to ensure that the behaviour of the method is consistent we will also explicitly remove the action.
+        if (action.AttachedEntity != null)
+            _actions.RemoveAction(action.AttachedEntity.Value, actionId, action: action);
+    }
+
     private void OnInit(EntityUid uid, ActionsContainerComponent component, ComponentInit args)
     {
         component.Container = _container.EnsureContainer<Container>(uid, ActionsContainerComponent.ContainerId);
@@ -171,13 +253,7 @@ private void OnEntityRemoved(EntityUid uid, ActionsContainerComponent component,
 
         var ev = new ActionRemovedEvent(args.Entity, data);
         RaiseLocalEvent(uid, ref ev);
-
-        if (_netMan.IsServer)
-        {
-            // TODO Actions
-            // log an error or warning here once gibbing code is fixed.
-            QueueDel(args.Entity);
-        }
+        data.Container = null;
     }
 }
 

Content.Shared/Actions/SharedActionsSystem.cs

@@ -586,9 +586,16 @@ public void RemoveAction(EntityUid performer, EntityUid? actionId, ActionsCompon
         if (!ResolveActionData(actionId, ref action))
             return;
 
+        if (action.AttachedEntity != performer)
+        {
+            Log.Error($"Attempted to remove an action {ToPrettyString(actionId)} from an entity that it was never attached to: {ToPrettyString(performer)}");
+            return;
+        }
+
         if (!Resolve(performer, ref comp, false))
         {
-            DebugTools.AssertNull(action.AttachedEntity);
+            DebugTools.Assert(action.AttachedEntity == null || TerminatingOrDeleted(action.AttachedEntity.Value));
+            action.AttachedEntity = null;
             return;
         }
 
@@ -599,7 +606,6 @@ public void RemoveAction(EntityUid performer, EntityUid? actionId, ActionsCompon
             return;
         }
 
-        DebugTools.Assert(action.AttachedEntity == performer);
         comp.Actions.Remove(actionId.Value);
         action.AttachedEntity = null;
         Dirty(actionId.Value, action);