From fcd0d9ef0f4d3ce43fd97ac204f7e3b0275a79cd Mon Sep 17 00:00:00 2001 From: Leon Friedrich <60421075+ElectroJr@users.noreply.github.com> Date: Thu, 12 Oct 2023 04:50:10 +1100 Subject: [PATCH] Add methods to transfer actions between containers (#20901) --- .../Systems/Actions/ActionUIController.cs | 5 +- .../Actions/ActionContainerSystem.cs | 100 +++++++++++++++--- Content.Shared/Actions/SharedActionsSystem.cs | 10 +- 3 files changed, 98 insertions(+), 17 deletions(-) diff --git a/Content.Client/UserInterface/Systems/Actions/ActionUIController.cs b/Content.Client/UserInterface/Systems/Actions/ActionUIController.cs index b2ff36d05c37db..e2de86201e7273 100644 --- a/Content.Client/UserInterface/Systems/Actions/ActionUIController.cs +++ b/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(entIcon).Icon? - .GetFrame(RsiDirection.South, 0); + _dragShadow.Texture = sprite.Icon?.GetFrame(RsiDirection.South, 0); } else if (action.Icon != null) { diff --git a/Content.Shared/Actions/ActionContainerSystem.cs b/Content.Shared/Actions/ActionContainerSystem.cs index 27cd8dcce68810..bce0836efb6283 100644 --- a/Content.Shared/Actions/ActionContainerSystem.cs +++ b/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,7 +98,61 @@ public bool EnsureAction(EntityUid uid, } /// - /// Adds a pre-existing action to an action container. + /// Transfers an action from one container to another, while keeping the attached entity the same. + /// + /// + /// While the attached entity should be the same at the end, this will actually remove and then re-grant the action. + /// + 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); + } + + /// + /// Transfers all actions from one container to another, while keeping the attached entity the same. + /// + /// <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 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); + } + + /// + /// Adds a pre-existing action to an action container. If the action is already in some container it will first remove it. /// public bool AddAction(EntityUid uid, EntityUid actionId, BaseActionComponent? action = null, ActionsContainerComponent? comp = null) { @@ -104,10 +160,7 @@ public bool AddAction(EntityUid uid, EntityUid actionId, BaseActionComponent? ac 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(uid); @@ -124,6 +177,35 @@ public bool AddAction(EntityUid uid, EntityUid actionId, BaseActionComponent? ac return true; } + /// + /// Removes an action from its container and any action-performer and moves the action to null-space + /// + 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(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; } } diff --git a/Content.Shared/Actions/SharedActionsSystem.cs b/Content.Shared/Actions/SharedActionsSystem.cs index 86379277e23ed7..8d2c8c28e3e44d 100644 --- a/Content.Shared/Actions/SharedActionsSystem.cs +++ b/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);