Cleanup cargo shuttle/telepad order code (#13591)

Co-authored-by: Eoin Mcloughlin <helloworld@eoinrul.es>

Content.Client/Cargo/BUI/CargoOrderConsoleBoundUserInterface.cs

@@ -158,7 +158,7 @@ namespace Content.Client.Cargo.BUI
             if (args.Button.Parent?.Parent is not CargoOrderRow row || row.Order == null)
                 return;
 
-            SendMessage(new CargoConsoleRemoveOrderMessage(row.Order.OrderIndex));
+            SendMessage(new CargoConsoleRemoveOrderMessage(row.Order.OrderId));
         }
 
         private void ApproveOrder(ButtonEventArgs args)
@@ -169,7 +169,7 @@ namespace Content.Client.Cargo.BUI
             if (OrderCount >= OrderCapacity)
                 return;
 
-            SendMessage(new CargoConsoleApproveOrderMessage(row.Order.OrderIndex));
+            SendMessage(new CargoConsoleApproveOrderMessage(row.Order.OrderId));
             // Most of the UI isn't predicted anyway so.
             // _menu?.UpdateCargoCapacity(OrderCount + row.Order.Amount, OrderCapacity);
         }

Content.Client/Cargo/UI/CargoConsoleMenu.xaml.cs

@@ -141,7 +141,7 @@ namespace Content.Client.Cargo.UI
                         Text = Loc.GetString(
                             "cargo-console-menu-populate-orders-cargo-order-row-product-name-text",
                             ("productName", productName),
-                            ("orderAmount", order.Amount),
+                            ("orderAmount", order.OrderQuantity),
                             ("orderRequester", order.Requester))
                     },
                     Description = {Text = Loc.GetString("cargo-console-menu-order-reason-description",

Content.Client/Cargo/UI/CargoShuttleMenu.xaml.cs

@@ -89,7 +89,7 @@ namespace Content.Client.Cargo.UI
                          Text = Loc.GetString(
                              "cargo-console-menu-populate-orders-cargo-order-row-product-name-text",
                              ("productName", productName),
-                             ("orderAmount", order.Amount),
+                             ("orderAmount", order.OrderQuantity - order.NumDispatched),
                              ("orderRequester", order.Requester))
                      },
                      Description = {Text = Loc.GetString("cargo-console-menu-order-reason-description",

Content.Server/Cargo/Components/StationCargoOrderDatabaseComponent.cs

@@ -17,12 +17,12 @@ public sealed class StationCargoOrderDatabaseComponent : Component
     public int Capacity = 20;
 
     [ViewVariables(VVAccess.ReadWrite), DataField("orders")]
-    public Dictionary<int, CargoOrderData> Orders = new();
+    public List<CargoOrderData> Orders = new();
 
     /// <summary>
-    /// Tracks the next order index available.
+    /// Used to determine unique order IDs
     /// </summary>
-    public int Index;
+    public int NumOrdersCreated;
 
     [DataField("cargoShuttleProto", customTypeSerializer:typeof(PrototypeIdSerializer<CargoShuttlePrototype>))]
     public string? CargoShuttleProto = "CargoShuttle";

Content.Server/Cargo/Systems/CargoSystem.Orders.cs

@@ -1,6 +1,7 @@
-using System.Linq;
+using System.Diagnostics.CodeAnalysis;
 using Content.Server.Access.Systems;
 using Content.Server.Cargo.Components;
+using Content.Server.Labels.Components;
 using Content.Server.MachineLinking.System;
 using Content.Server.Popups;
 using Content.Server.Station.Systems;
@@ -12,9 +13,9 @@ using Content.Shared.Cargo.Events;
 using Content.Shared.Cargo.Prototypes;
 using Content.Shared.Database;
 using Content.Shared.GameTicking;
+using Content.Server.Paper;
 using Robust.Server.GameObjects;
-using Robust.Shared.Audio;
+using Robust.Shared.Map;
-using Robust.Shared.Player;
 using Robust.Shared.Players;
 
 namespace Content.Server.Cargo.Systems
@@ -114,9 +115,12 @@ namespace Content.Server.Cargo.Systems
                 return;
             }
 
-            // No order to approve?
+            // Find our order again. It might have been dispatched or approved already
-            if (!orderDatabase.Orders.TryGetValue(args.OrderIndex, out var order) ||
+            var order = orderDatabase.Orders.Find(order => (args.OrderId == order.OrderId) && !order.Approved);
-                order.Approved) return;
+            if(order == null)
+            {
+                return;
+            }
 
             // Invalid order
             if (!_protoMan.TryIndex<CargoProductPrototype>(order.ProductId, out var product))
@@ -126,7 +130,7 @@ namespace Content.Server.Cargo.Systems
                 return;
             }
 
-            var amount = GetOrderCount(orderDatabase);
+            var amount = GetOutstandingOrderCount(orderDatabase);
             var capacity = orderDatabase.Capacity;
 
             // Too many orders, avoid them getting spammed in the UI.
@@ -138,16 +142,16 @@ namespace Content.Server.Cargo.Systems
             }
 
             // Cap orders so someone can't spam thousands.
-            var orderAmount = Math.Min(capacity - amount, order.Amount);
+            var cappedAmount = Math.Min(capacity - amount, order.OrderQuantity);
 
-            if (orderAmount != order.Amount)
+            if (cappedAmount != order.OrderQuantity)
             {
-                order.Amount = orderAmount;
+                order.OrderQuantity = cappedAmount;
                 ConsolePopup(args.Session, Loc.GetString("cargo-console-snip-snip"));
                 PlayDenySound(uid, component);
             }
 
-            var cost = product.PointCost * order.Amount;
+            var cost = product.PointCost * order.OrderQuantity;
 
             // Not enough balance
             if (cost > bankAccount.Balance)
@@ -163,7 +167,7 @@ namespace Content.Server.Cargo.Systems
 
             // Log order approval
             _adminLogger.Add(LogType.Action, LogImpact.Low,
-                $"{ToPrettyString(player):user} approved order [orderIdx:{order.OrderIndex}, amount:{order.Amount}, product:{order.ProductId}, requester:{order.Requester}, reason:{order.Reason}] with balance at {bankAccount.Balance}");
+                $"{ToPrettyString(player):user} approved order [orderId:{order.OrderId}, quantity:{order.OrderQuantity}, product:{order.ProductId}, requester:{order.Requester}, reason:{order.Reason}] with balance at {bankAccount.Balance}");
 
             DeductFunds(bankAccount, cost);
             UpdateOrders(orderDatabase);
@@ -173,7 +177,7 @@ namespace Content.Server.Cargo.Systems
         {
             var orderDatabase = GetOrderDatabase(component);
             if (orderDatabase == null) return;
-            RemoveOrder(orderDatabase, args.OrderIndex);
+            RemoveOrder(orderDatabase, args.OrderId);
         }
 
         private void OnAddOrderMessage(EntityUid uid, CargoOrderConsoleComponent component, CargoConsoleAddOrderMessage args)
@@ -189,7 +193,7 @@ namespace Content.Server.Cargo.Systems
             var orderDatabase = GetOrderDatabase(component);
             if (orderDatabase == null) return;
 
-            var data = GetOrderData(args, GetNextIndex(orderDatabase));
+            var data = GetOrderData(args, GenerateOrderId(orderDatabase));
 
             if (!TryAddOrder(orderDatabase, data))
             {
@@ -199,7 +203,7 @@ namespace Content.Server.Cargo.Systems
 
             // Log order addition
             _adminLogger.Add(LogType.Action, LogImpact.Low,
-                $"{ToPrettyString(player):user} added order [orderIdx:{data.OrderIndex}, amount:{data.Amount}, product:{data.ProductId}, requester:{data.Requester}, reason:{data.Reason}]");
+                $"{ToPrettyString(player):user} added order [orderId:{data.OrderId}, quantity:{data.OrderQuantity}, product:{data.ProductId}, requester:{data.Requester}, reason:{data.Reason}]");
 
         }
 
@@ -219,10 +223,10 @@ namespace Content.Server.Cargo.Systems
 
             var state = new CargoConsoleInterfaceState(
                 MetaData(station.Value).EntityName,
-                GetOrderCount(orderDatabase),
+                GetOutstandingOrderCount(orderDatabase),
                 orderDatabase.Capacity,
                 bankAccount.Balance,
-                orderDatabase.Orders.Values.ToList());
+                orderDatabase.Orders);
 
             _uiSystem.GetUiOrNull(component.Owner, CargoConsoleUiKey.Orders)?.SetState(state);
         }
@@ -234,19 +238,19 @@ namespace Content.Server.Cargo.Systems
             _audio.PlayPvs(_audio.GetSound(component.ErrorSound), uid);
         }
 
-        private CargoOrderData GetOrderData(CargoConsoleAddOrderMessage args, int index)
+        private CargoOrderData GetOrderData(CargoConsoleAddOrderMessage args, int id)
         {
-            return new CargoOrderData(index, args.ProductId, args.Amount, args.Requester, args.Reason);
+            return new CargoOrderData(id, args.ProductId, args.Amount, args.Requester, args.Reason);
         }
 
-        private int GetOrderCount(StationCargoOrderDatabaseComponent component)
+        private int GetOutstandingOrderCount(StationCargoOrderDatabaseComponent component)
         {
             var amount = 0;
 
-            foreach (var (_, order) in component.Orders)
+            foreach (var order in component.Orders)
             {
                 if (!order.Approved) continue;
-                amount += order.Amount;
+                amount += order.OrderQuantity - order.NumDispatched;
             }
 
             return amount;
@@ -278,22 +282,26 @@ namespace Content.Server.Cargo.Systems
 
         public bool TryAddOrder(StationCargoOrderDatabaseComponent component, CargoOrderData data)
         {
-            component.Orders.Add(data.OrderIndex, data);
+            component.Orders.Add(data);
             UpdateOrders(component);
             return true;
         }
 
-        private int GetNextIndex(StationCargoOrderDatabaseComponent component)
+        private int GenerateOrderId(StationCargoOrderDatabaseComponent orderDB)
         {
-            var index = component.Index;
+            // We need an arbitrary unique ID to idenitfy orders, since they may
-            component.Index++;
+            // want to be cancelled later.
-            return index;
+            return ++orderDB.NumOrdersCreated;
         }
 
-        public void RemoveOrder(StationCargoOrderDatabaseComponent component, int index)
+        public void RemoveOrder(StationCargoOrderDatabaseComponent orderDB, int index)
         {
-            if (!component.Orders.Remove(index)) return;
+            var sequenceIdx = orderDB.Orders.FindIndex(order => order.OrderId == index);
-            UpdateOrders(component);
+            if (sequenceIdx != -1)
+            {
+                orderDB.Orders.RemoveAt(sequenceIdx);
+            }
+            UpdateOrders(orderDB);
         }
 
         public void ClearOrders(StationCargoOrderDatabaseComponent component)
@@ -304,6 +312,64 @@ namespace Content.Server.Cargo.Systems
             Dirty(component);
         }
 
+        private bool PopFrontOrder(StationCargoOrderDatabaseComponent orderDB, [NotNullWhen(true)] out CargoOrderData? orderOut)
+        {
+            var orderIdx = orderDB.Orders.FindIndex(order => order.Approved);
+            if (orderIdx == -1)
+            {
+                orderOut = null;
+                return false;
+            }
+
+            orderOut = orderDB.Orders[orderIdx];
+            orderOut.NumDispatched++;
+
+            if(orderOut.NumDispatched >= orderOut.OrderQuantity)
+            {
+                // Order is complete. Remove from the queue.
+                orderDB.Orders.RemoveAt(orderIdx);
+            }
+            return true;
+        }
+
+        private bool FulfillOrder(StationCargoOrderDatabaseComponent orderDB, EntityCoordinates whereToPutIt,
+                string? paperPrototypeToPrint)
+        {
+            if (PopFrontOrder(orderDB, out var order))
+            {
+                // Create the item itself
+                var item = Spawn(_protoMan.Index<CargoProductPrototype>(order.ProductId).Product, whereToPutIt);
+
+                // Create a sheet of paper to write the order details on
+                var printed = EntityManager.SpawnEntity(paperPrototypeToPrint, whereToPutIt);
+                if (TryComp<PaperComponent>(printed, out var paper))
+                {
+                    // fill in the order data
+                    var val = Loc.GetString("cargo-console-paper-print-name", ("orderNumber", order.OrderId));
+                    MetaData(printed).EntityName = val;
+
+                    _paperSystem.SetContent(printed, Loc.GetString(
+                                "cargo-console-paper-print-text",
+                                ("orderNumber", order.OrderId),
+                                ("itemName", MetaData(item).EntityName),
+                                ("requester", order.Requester),
+                                ("reason", order.Reason),
+                                ("approver", order.Approver ?? string.Empty)),
+                            paper);
+
+                    // attempt to attach the label to the item
+                    if (TryComp<PaperLabelComponent>(item, out var label))
+                    {
+                        _slots.TryInsert(item, label.LabelSlot, printed, null);
+                    }
+                }
+
+                return true;
+            }
+
+            return false;
+        }
+
         private void DeductFunds(StationBankAccountComponent component, int amount)
         {
             component.Balance = Math.Max(0, component.Balance - amount);

Content.Server/Cargo/Systems/CargoSystem.Shuttle.cs

@@ -209,34 +209,27 @@ public sealed partial class CargoSystem
         if (component == null || shuttle == null || component.Orders.Count == 0)
             return orders;
 
-        var space = GetCargoSpace(shuttle);
+        var spaceRemaining = GetCargoSpace(shuttle);
-
+        for( var i = 0; i < component.Orders.Count && spaceRemaining > 0; i++)
-        if (space == 0) return orders;
-
-        var indices = component.Orders.Keys.ToList();
-        indices.Sort();
-        var amount = 0;
-
-        foreach (var index in indices)
         {
-            var order = component.Orders[index];
+            var order = component.Orders[i];
-            if (!order.Approved) continue;
+            if(order.Approved)
-
-            var cappedAmount = Math.Min(space - amount, order.Amount);
-            amount += cappedAmount;
-            DebugTools.Assert(amount <= space);
-
-            if (cappedAmount < order.Amount)
             {
-                var reducedOrder = new CargoOrderData(order.OrderIndex, order.ProductId, cappedAmount, order.Requester, order.Reason);
+                var numToShip = order.OrderQuantity - order.NumDispatched;
-
+                if (numToShip > spaceRemaining)
+                {
+                    // We won't be able to fit the whole oder on, so make one
+                    // which represents the space we do have left:
+                    var reducedOrder = new CargoOrderData(order.OrderId,
+                            order.ProductId, spaceRemaining, order.Requester, order.Reason);
                     orders.Add(reducedOrder);
-                break;
                 }
-
+                else
+                {
                     orders.Add(order);
-
+                }
-            if (amount == space) break;
+                spaceRemaining -= numToShip;
+            }
         }
 
         return orders;
@@ -424,70 +417,18 @@ public sealed partial class CargoSystem
         _sawmill.Info($"Retrieved cargo shuttle {ToPrettyString(shuttle.Owner)} from {ToPrettyString(orderDatabase.Owner)}");
     }
 
-    private void AddCargoContents(CargoShuttleComponent component, StationCargoOrderDatabaseComponent orderDatabase)
+    private void AddCargoContents(CargoShuttleComponent shuttle, StationCargoOrderDatabaseComponent orderDatabase)
     {
         var xformQuery = GetEntityQuery<TransformComponent>();
-        var orders = GetProjectedOrders(orderDatabase, component);
 
-        var pads = GetCargoPallets(component);
+        var pads = GetCargoPallets(shuttle);
-        DebugTools.Assert(orders.Sum(o => o.Amount) <= pads.Count);
+        while (pads.Count > 0)
-
+        {
-        for (var i = 0; i < pads.Count; i++)
+            var coordinates = new EntityCoordinates(shuttle.Owner, xformQuery.GetComponent(_random.PickAndTake(pads).Owner).LocalPosition);
+            if(!FulfillOrder(orderDatabase, coordinates, shuttle.PrinterOutput))
             {
-            if (orders.Count == 0)
                 break;
-
-            var order = orders[0];
-            var coordinates = new EntityCoordinates(component.Owner, xformQuery.GetComponent(_random.PickAndTake(pads).Owner).LocalPosition);
-            var item = Spawn(_protoMan.Index<CargoProductPrototype>(order.ProductId).Product, coordinates);
-            SpawnAndAttachOrderManifest(item, order, coordinates, component);
-            order.Amount--;
-
-            if (order.Amount == 0)
-            {
-                // Yes this is functioning as a stack, I was too lazy to re-jig the shuttle state event.
-                orders.RemoveSwap(0);
-                orderDatabase.Orders.Remove(order.OrderIndex);
             }
-            else
-            {
-                orderDatabase.Orders[order.OrderIndex] = order;
-            }
-        }
-    }
-
-    /// <summary>
-    /// In this method we are printing and attaching order manifests to the orders.
-    /// </summary>
-    private void SpawnAndAttachOrderManifest(EntityUid item, CargoOrderData order, EntityCoordinates coordinates, CargoShuttleComponent component)
-    {
-        if (!_protoMan.TryIndex(order.ProductId, out CargoProductPrototype? prototype))
-            return;
-
-        // spawn a piece of paper.
-        var printed = EntityManager.SpawnEntity(component.PrinterOutput, coordinates);
-
-        if (!TryComp<PaperComponent>(printed, out var paper))
-            return;
-
-        // fill in the order data
-        var val = Loc.GetString("cargo-console-paper-print-name", ("orderNumber", order.PrintableOrderNumber));
-
-        MetaData(printed).EntityName = val;
-
-        _paperSystem.SetContent(printed, Loc.GetString(
-            "cargo-console-paper-print-text",
-            ("orderNumber", order.PrintableOrderNumber),
-            ("itemName", prototype.Name),
-            ("requester", order.Requester),
-            ("reason", order.Reason),
-            ("approver", order.Approver ?? string.Empty)),
-            paper);
-
-        // attempt to attach the label
-        if (TryComp<PaperLabelComponent>(item, out var label))
-        {
-            _slots.TryInsert(item, label.LabelSlot, printed, null);
         }
     }
 

Content.Server/Cargo/Systems/CargoSystem.Telepad.cs

@@ -58,34 +58,16 @@ public sealed partial class CargoSystem
                 continue;
             }
 
-            var orderIndices = new ValueList<int>();
+            var xform = Transform(comp.Owner);
-
+            if(FulfillOrder(orderDatabase, xform.Coordinates,comp.PrinterOutput))
-            foreach (var (oIndex, oOrder) in orderDatabase.Orders)
             {
-                if (!oOrder.Approved) continue;
-                orderIndices.Add(oIndex);
-            }
-
-            if (orderIndices.Count == 0)
-            {
-                comp.Accumulator += comp.Delay;
-                continue;
-            }
-
-            orderIndices.Sort();
-            var index = orderIndices[0];
-            var order = orderDatabase.Orders[index];
-            order.Amount--;
-
-            if (order.Amount <= 0)
-                orderDatabase.Orders.Remove(index);
-
                 _audio.PlayPvs(_audio.GetSound(comp.TeleportSound), comp.Owner, AudioParams.Default.WithVolume(-8f));
-            SpawnProduct(comp, order);
                 UpdateOrders(orderDatabase);
 
                 comp.CurrentState = CargoTelepadState.Teleporting;
                 _appearance.SetData(comp.Owner, CargoTelepadVisuals.State, CargoTelepadState.Teleporting, appearance);
+            }
+
             comp.Accumulator += comp.Delay;
         }
     }
@@ -120,46 +102,4 @@ public sealed partial class CargoSystem
     {
         SetEnabled(component);
     }
-
-    /// <summary>
-    ///     Spawn the product and a piece of paper. Attempt to attach the paper to the product.
-    /// </summary>
-    private void SpawnProduct(CargoTelepadComponent component, CargoOrderData data)
-    {
-        // spawn the order
-        if (!_protoMan.TryIndex(data.ProductId, out CargoProductPrototype? prototype))
-            return;
-
-        var xform = Transform(component.Owner);
-
-        var product = EntityManager.SpawnEntity(prototype.Product, xform.Coordinates);
-
-        Transform(product).Anchored = false;
-
-        // spawn a piece of paper.
-        var printed = EntityManager.SpawnEntity(component.PrinterOutput, xform.Coordinates);
-
-        if (!TryComp<PaperComponent>(printed, out var paper))
-            return;
-
-        // fill in the order data
-        var val = Loc.GetString("cargo-console-paper-print-name", ("orderNumber", data.PrintableOrderNumber));
-
-        MetaData(printed).EntityName = val;
-
-        _paperSystem.SetContent(printed, Loc.GetString(
-            "cargo-console-paper-print-text",
-            ("orderNumber", data.PrintableOrderNumber),
-            ("itemName", prototype.Name),
-            ("requester", data.Requester),
-            ("reason", data.Reason),
-            ("approver", data.Approver ?? string.Empty)),
-            paper);
-
-        // attempt to attach the label
-        if (TryComp<PaperLabelComponent>(product, out var label))
-        {
-            _slots.TryInsert(product, label.LabelSlot, printed, null);
-        }
-    }
 }

Content.Server/Cargo/Systems/CargoSystem.cs

@@ -1,7 +1,6 @@
 using Content.Server.Cargo.Components;
 using Content.Server.Station.Systems;
 using Content.Shared.Cargo;
-using Content.Shared.Cargo.Components;
 using Content.Shared.Containers.ItemSlots;
 using Robust.Shared.Prototypes;
 

Content.Shared/Cargo/CargoOrderData.cs

@@ -6,23 +6,39 @@ namespace Content.Shared.Cargo
     [NetSerializable, Serializable]
     public sealed class CargoOrderData
     {
-        public int OrderIndex;
+        /// <summary>
-        /// The human-readable number, when displaying this order
+        /// A unique (arbitrary) ID which identifies this order.
-        public int PrintableOrderNumber { get { return OrderIndex + 1; } }
+        /// </summary>
-        public string ProductId;
+        public readonly int OrderId;
-        public int Amount;
+
-        public string Requester;
+        /// <summary>
+        /// Prototype id for the item to create
+        /// </summary>
+        public readonly string ProductId;
+
+        /// <summary>
+        /// The number of items in the order. Not readonly, as it might change
+        /// due to caps on the amount of orders that can be placed.
+        /// </summary>
+        public int OrderQuantity;
+
+        /// <summary>
+        /// How many instances of this order that we've already dispatched
+        /// <summary>
+        public int NumDispatched = 0;
+
+        public readonly string Requester;
         // public String RequesterRank; // TODO Figure out how to get Character ID card data
         // public int RequesterId;
-        public string Reason;
+        public readonly string Reason;
         public  bool Approved => Approver is not null;
         public string? Approver;
 
-        public CargoOrderData(int orderIndex, string productId, int amount, string requester, string reason)
+        public CargoOrderData(int orderId, string productId, int amount, string requester, string reason)
         {
-            OrderIndex = orderIndex;
+            OrderId = orderId;
             ProductId = productId;
-            Amount = amount;
+            OrderQuantity = amount;
             Requester = requester;
             Reason = reason;
         }

Content.Shared/Cargo/Events/CargoConsoleApproveOrderMessage.cs

@@ -8,10 +8,10 @@ namespace Content.Shared.Cargo.Events;
 [Serializable, NetSerializable]
 public sealed class CargoConsoleApproveOrderMessage : BoundUserInterfaceMessage
 {
-    public int OrderIndex;
+    public int OrderId;
 
-    public CargoConsoleApproveOrderMessage(int orderIndex)
+    public CargoConsoleApproveOrderMessage(int orderId)
     {
-        OrderIndex = orderIndex;
+        OrderId = orderId;
     }
 }

Content.Shared/Cargo/Events/CargoConsoleRemoveOrderMessage.cs

@@ -8,10 +8,10 @@ namespace Content.Shared.Cargo.Events;
 [Serializable, NetSerializable]
 public sealed class CargoConsoleRemoveOrderMessage : BoundUserInterfaceMessage
 {
-    public int OrderIndex;
+    public int OrderId;
 
-    public CargoConsoleRemoveOrderMessage(int orderIndex)
+    public CargoConsoleRemoveOrderMessage(int orderId)
     {
-        OrderIndex = orderIndex;
+        OrderId = orderId;
     }
 }