From 2743b64a2b9048842ab584aa5c43a10b852c4fb9 Mon Sep 17 00:00:00 2001
From: Leon Friedrich <60421075+ElectroJr@users.noreply.github.com>
Date: Tue, 7 Nov 2023 15:05:32 +1100
Subject: [PATCH] Mark container methods as obsolete (#4551)

---
 Robust.Shared/Containers/BaseContainer.cs     | 28 ++-------------
 .../SharedContainerSystem.Insert.cs           | 33 +++++++++++++++++-
 .../SharedContainerSystem.Remove.cs           | 34 +++++++++++++++++++
 3 files changed, 69 insertions(+), 26 deletions(-)

diff --git a/Robust.Shared/Containers/BaseContainer.cs b/Robust.Shared/Containers/BaseContainer.cs
index 8108930b5..d9f48a7a0 100644
--- a/Robust.Shared/Containers/BaseContainer.cs
+++ b/Robust.Shared/Containers/BaseContainer.cs
@@ -79,20 +79,7 @@ namespace Robust.Shared.Containers
             Owner = owner;
         }
 
-        /// <summary>
-        /// Attempts to insert the entity into this container.
-        /// </summary>
-        /// <remarks>
-        /// If the insertion is successful, the inserted entity will end up parented to the
-        /// container entity, and the inserted entity's local position will be set to the zero vector.
-        /// </remarks>
-        /// <param name="toinsert">The entity to insert.</param>
-        /// <param name="entMan"></param>
-        /// <returns>False if the entity could not be inserted.</returns>
-        /// <exception cref="InvalidOperationException">
-        /// Thrown if this container is a child of the entity,
-        /// which would cause infinite loops.
-        /// </exception>
+        [Obsolete("Use container system method")]
         public bool Insert(
             EntityUid toinsert,
             IEntityManager? entMan = null,
@@ -270,16 +257,7 @@ namespace Robust.Shared.Containers
         /// <param name="assumeEmpty">Whether to assume that the container is currently empty.</param>
         protected internal virtual bool CanInsert(EntityUid toInsert, bool assumeEmpty, IEntityManager entMan) => true;
 
-        /// <summary>
-        /// Attempts to remove the entity from this container.
-        /// </summary>
-        /// <param name="reparent">If false, this operation will not rigger a move or parent change event. Ignored if
-        /// destination is not null</param>
-        /// <param name="force">If true, this will not perform can-remove checks.</param>
-        /// <param name="destination">Where to place the entity after removing. Avoids unnecessary broadphase updates.
-        /// If not specified, and reparent option is true, then the entity will either be inserted into a parent
-        /// container, the grid, or the map.</param>
-        /// <param name="localRotation">Optional final local rotation after removal. Avoids redundant move events.</param>
+        [Obsolete("Use container system method")]
         public bool Remove(
             EntityUid toRemove,
             IEntityManager? entMan = null,
@@ -359,7 +337,7 @@ namespace Robust.Shared.Containers
             return true;
         }
 
-        [Obsolete("use force option in Remove()")]
+        [Obsolete("Use container system method")]
         public void ForceRemove(EntityUid toRemove, IEntityManager? entMan = null, MetaDataComponent? meta = null)
             => Remove(toRemove, entMan, meta: meta, reparent: false, force: true);
 
diff --git a/Robust.Shared/Containers/SharedContainerSystem.Insert.cs b/Robust.Shared/Containers/SharedContainerSystem.Insert.cs
index 681696fb6..8f53dd076 100644
--- a/Robust.Shared/Containers/SharedContainerSystem.Insert.cs
+++ b/Robust.Shared/Containers/SharedContainerSystem.Insert.cs
@@ -1,10 +1,41 @@
+using System;
 using Robust.Shared.GameObjects;
-using Robust.Shared.Utility;
+using Robust.Shared.Physics.Components;
 
 namespace Robust.Shared.Containers;
 
 public abstract partial class SharedContainerSystem
 {
+
+    /// <summary>
+    /// Attempts to insert the entity into this container.
+    /// </summary>
+    /// <remarks>
+    /// If the insertion is successful, the inserted entity will end up parented to the
+    /// container entity, and the inserted entity's local position will be set to the zero vector.
+    /// </remarks>
+    /// <param name="toInsert">The entity to insert.</param>
+    /// <param name="container">The container to insert into.</param>
+    /// <param name="containerXform">The container's transform component.</param>
+    /// <param name="force">Whether to bypass normal insertion checks.</param>
+    /// <returns>False if the entity could not be inserted.</returns>
+    /// <exception cref="InvalidOperationException">
+    /// Thrown if this container is a child of the entity,
+    /// which would cause infinite loops.
+    /// </exception>
+    public bool Insert(Entity<TransformComponent?, MetaDataComponent?, PhysicsComponent?> toInsert,
+        BaseContainer container,
+        TransformComponent? containerXform = null,
+        bool force = false)
+    {
+        // Cannot Use Resolve(ref toInsert) as the physics component is optional
+        if (!Resolve(toInsert.Owner, ref toInsert.Comp1, ref toInsert.Comp2))
+            return false;
+
+        // TODO move logic over to the system.
+        return container.Insert(toInsert, EntityManager, toInsert, containerXform, toInsert, toInsert, force);
+    }
+
     /// <summary>
     /// Checks if the entity can be inserted into the given container.
     /// </summary>
diff --git a/Robust.Shared/Containers/SharedContainerSystem.Remove.cs b/Robust.Shared/Containers/SharedContainerSystem.Remove.cs
index 7662ae7b5..2deedc6af 100644
--- a/Robust.Shared/Containers/SharedContainerSystem.Remove.cs
+++ b/Robust.Shared/Containers/SharedContainerSystem.Remove.cs
@@ -1,9 +1,43 @@
 using Robust.Shared.GameObjects;
+using Robust.Shared.Map;
+using Robust.Shared.Maths;
 
 namespace Robust.Shared.Containers;
 
 public abstract partial class SharedContainerSystem
 {
+    /// <summary>
+    /// Attempts to remove the entity from this container.
+    /// </summary>
+    /// <remarks>
+    /// If the insertion is successful, the inserted entity will end up parented to the
+    /// container entity, and the inserted entity's local position will be set to the zero vector.
+    /// </remarks>
+    /// <param name="toRemove">The entity to remove.</param>
+    /// <param name="container">The container to remove from.</param>
+    /// <param name="reparent">If false, this operation will not rigger a move or parent change event. Ignored if
+    /// destination is not null</param>
+    /// <param name="force">If true, this will not perform can-remove checks.</param>
+    /// <param name="destination">Where to place the entity after removing. Avoids unnecessary broadphase updates.
+    /// If not specified, and reparent option is true, then the entity will either be inserted into a parent
+    /// container, the grid, or the map.</param>
+    /// <param name="localRotation">Optional final local rotation after removal. Avoids redundant move events.</param>
+    public bool Insert(
+        Entity<TransformComponent?, MetaDataComponent?> toRemove,
+        BaseContainer container,
+        bool reparent = true,
+        bool force = false,
+        EntityCoordinates? destination = null,
+        Angle? localRotation = null)
+    {
+        // Cannot Use Resolve(ref toInsert) as the physics component is optional
+        if (!Resolve(toRemove.Owner, ref toRemove.Comp1, ref toRemove.Comp2))
+            return false;
+
+        // TODO move logic over to the system.
+        return container.Remove(toRemove, EntityManager, toRemove, toRemove, reparent, force, destination, localRotation);
+    }
+
     /// <summary>
     /// Checks if the entity can be removed from this container.
     /// </summary>