Bounty/history improvements (#2743)

* Remove redundant access specifications

* Remove un-needed override

* Fixup BountyHistoryEntry backing code

* Fix formatting in CargoBountyMenu

* Reformat BountyHistoryData

* Rework TryRemoveBounty to use new Entity type

* Add Enum for showing bounty results

* Rework look and feel of History tab

* Add visible text when no bounties have been completed yet

* Remove control

* Swap default to null

* Reverse ordering of bounties so last entry comes first

Content.Client/Cargo/UI/BountyHistoryEntry.xaml

@@ -1,23 +1,23 @@
-<BoxContainer xmlns="https://spacestation14.io"
-              xmlns:customControls="clr-namespace:Content.Client.Administration.UI.CustomControls"
-              Margin="10 10 10 0"
-              HorizontalExpand="True"
-              Visible="True">
+<BoxContainer xmlns="https://spacestation14.io"
+                xmlns:customControls="clr-namespace:Content.Client.Administration.UI.CustomControls"
+                Margin="10 10 10 0"
+                HorizontalExpand="True"
+                Visible="True">
     <PanelContainer StyleClasses="AngleRect" HorizontalExpand="True">
         <BoxContainer Orientation="Vertical"
                       HorizontalExpand="True">
             <BoxContainer Orientation="Horizontal">
                 <BoxContainer Orientation="Vertical" HorizontalExpand="True">
                     <RichTextLabel Name="RewardLabel"/>
                     <RichTextLabel Name="ManifestLabel"/>
-                    <RichTextLabel Name="NoticeLabel"/>
                 </BoxContainer>
-                <Control MinWidth="10"/>
-                <BoxContainer Orientation="Vertical" MinWidth="120">
-                    <RichTextLabel Name="StatusLabel" HorizontalAlignment="Right" Margin="0 0 5 0"/>
-                    <RichTextLabel Name="IdLabel" HorizontalAlignment="Right" Margin="0 0 5 0"/>
+                <BoxContainer Orientation="Vertical" MinWidth="120" Margin="0 0 10 0">
+                    <RichTextLabel Name="TimestampLabel" HorizontalAlignment="Right" />
+                    <RichTextLabel Name="IdLabel" HorizontalAlignment="Right" />
                 </BoxContainer>
             </BoxContainer>
+            <customControls:HSeparator Margin="5 10 5 10"/>
+            <RichTextLabel Name="NoticeLabel" />
         </BoxContainer>
     </PanelContainer>
 </BoxContainer>

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

@@ -19,7 +19,7 @@ public BountyHistoryEntry(CargoBountyHistoryData bounty)
         RobustXamlLoader.Load(this);
         IoCManager.InjectDependencies(this);
 
-        if (!_prototype.TryIndex<CargoBountyPrototype>(bounty.Bounty, out var bountyPrototype))
+        if (!_prototype.TryIndex(bounty.Bounty, out var bountyPrototype))
             return;
 
         var items = new List<string>();
@@ -29,26 +29,21 @@ public BountyHistoryEntry(CargoBountyHistoryData bounty)
                 ("amount", entry.Amount),
                 ("item", Loc.GetString(entry.Name))));
         }
+
         ManifestLabel.SetMarkup(Loc.GetString("bounty-console-manifest-label", ("item", string.Join(", ", items))));
         RewardLabel.SetMarkup(Loc.GetString("bounty-console-reward-label", ("reward", bountyPrototype.Reward)));
         IdLabel.SetMarkup(Loc.GetString("bounty-console-id-label", ("id", bounty.Id)));
 
-        var stationTime = bounty.Timestamp.ToString("hh\\:mm\\:ss");
-        if (bounty.ActorName == null)
+        TimestampLabel.SetMarkup(bounty.Timestamp.ToString(@"hh\:mm\:ss"));
+
+        if (bounty.Result == CargoBountyHistoryData.BountyResult.Completed)
         {
-            StatusLabel.SetMarkup(Loc.GetString("bounty-console-history-completed-label"));
-            NoticeLabel.SetMarkup(Loc.GetString("bounty-console-history-notice-completed-label", ("time", stationTime)));
+            NoticeLabel.SetMarkup(Loc.GetString("bounty-console-history-notice-completed-label"));
         }
         else
         {
-            StatusLabel.SetMarkup(Loc.GetString("bounty-console-history-skipped-label"));
             NoticeLabel.SetMarkup(Loc.GetString("bounty-console-history-notice-skipped-label",
-                ("id", bounty.ActorName),
-                ("time", stationTime)));
+                ("id", bounty.ActorName ?? "")));
         }
     }
-    protected override void FrameUpdate(FrameEventArgs args)
-    {
-        base.FrameUpdate(args);
-    }
 }

Content.Client/Cargo/UI/CargoBountyMenu.xaml

@@ -13,22 +13,24 @@
             </PanelContainer.PanelOverride>
             <TabContainer Name="MasterTabContainer" VerticalExpand="True" HorizontalExpand="True">
                 <ScrollContainer HScrollEnabled="False"
-                                HorizontalExpand="True"
-                                VerticalExpand="True">
+                                 HorizontalExpand="True"
+                                 VerticalExpand="True">
                     <BoxContainer Name="BountyEntriesContainer"
-                                Orientation="Vertical"
-                                VerticalExpand="True"
-                                HorizontalExpand="True">
-                    </BoxContainer>
+                                  Orientation="Vertical"
+                                  VerticalExpand="True"
+                                  HorizontalExpand="True" />
                 </ScrollContainer>
                 <ScrollContainer HScrollEnabled="False"
-                                HorizontalExpand="True"
-                                VerticalExpand="True">
+                                 HorizontalExpand="True"
+                                 VerticalExpand="True">
+                    <Label Name="NoHistoryLabel"
+                           Text="{Loc 'bounty-console-history-empty-label'}"
+                           Visible="False"
+                           Align="Center" />
                     <BoxContainer Name="BountyHistoryContainer"
-                                Orientation="Vertical"
-                                VerticalExpand="True"
-                                HorizontalExpand="True">
-                    </BoxContainer>
+                                  Orientation="Vertical"
+                                  VerticalExpand="True"
+                                  HorizontalExpand="True" />
                 </ScrollContainer>
             </TabContainer>
         </PanelContainer>

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

@@ -37,10 +37,19 @@ public void UpdateEntries(List<CargoBountyData> bounties, List<CargoBountyHistor
         });
 
         BountyHistoryContainer.Children.Clear();
-        foreach (var h in history)
+        if (history.Count == 0)
         {
-            var entry = new BountyHistoryEntry(h);
-            BountyHistoryContainer.AddChild(entry);
+            NoHistoryLabel.Visible = true;
+        }
+        else
+        {
+            NoHistoryLabel.Visible = false;
+
+            // Show the history in reverse, so last entry is first in the list
+            for (var i = history.Count - 1; i >= 0; i--)
+            {
+                BountyHistoryContainer.AddChild(new BountyHistoryEntry(history[i]));
+            }
         }
     }
 }

Content.Server/Cargo/Components/StationCargoBountyDatabaseComponent.cs

@@ -12,20 +12,20 @@ public sealed partial class StationCargoBountyDatabaseComponent : Component
     /// <summary>
     /// Maximum amount of bounties a station can have.
     /// </summary>
-    [DataField, ViewVariables(VVAccess.ReadWrite)]
+    [DataField]
     public int MaxBounties = 6;
 
     /// <summary>
     /// A list of all the bounties currently active for a station.
     /// </summary>
-    [DataField, ViewVariables(VVAccess.ReadWrite)]
+    [DataField]
     public List<CargoBountyData> Bounties = new();
 
     /// <summary>
     /// A list of all the bounties that have been completed or
     /// skipped for a station.
     /// </summary>
-    [DataField, ViewVariables(VVAccess.ReadWrite)]
+    [DataField]
     public List<CargoBountyHistoryData> History = new();
 
     /// <summary>

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

@@ -98,7 +98,7 @@ private void OnSkipBountyMessage(EntityUid uid, CargoBountyConsoleComponent comp
             return;
         }
 
-        if (!TryRemoveBounty(station, bounty.Value, null, args.Actor))
+        if (!TryRemoveBounty(station, bounty.Value, true, args.Actor))
             return;
 
         FillBountyDatabase(station);
@@ -182,7 +182,7 @@ private void OnSold(ref EntitySoldEvent args)
                 continue;
             }
 
-            TryRemoveBounty(station, bounty.Value);
+            TryRemoveBounty(station, bounty.Value, false);
             FillBountyDatabase(station);
             _adminLogger.Add(LogType.Action, LogImpact.Low, $"Bounty \"{bounty.Value.Bounty}\" (id:{bounty.Value.Id}) was fulfilled");
         }
@@ -437,33 +437,44 @@ public bool TryAddBounty(EntityUid uid, CargoBountyPrototype bounty, StationCarg
     }
 
     [PublicAPI]
-    public bool TryRemoveBounty(EntityUid uid, string dataId, StationCargoBountyDatabaseComponent? component = null, EntityUid? actor = null)
+    public bool TryRemoveBounty(Entity<StationCargoBountyDatabaseComponent?> ent,
+        string dataId,
+        bool skipped,
+        EntityUid? actor = null)
     {
-        if (!TryGetBountyFromId(uid, dataId, out var data, component))
+        if (!TryGetBountyFromId(ent.Owner, dataId, out var data, ent.Comp))
             return false;
 
-        return TryRemoveBounty(uid, data.Value, component, actor);
+        return TryRemoveBounty(ent, data.Value, skipped, actor);
     }
 
-    public bool TryRemoveBounty(EntityUid uid, CargoBountyData data, StationCargoBountyDatabaseComponent? component = null, EntityUid? actor = null)
+    public bool TryRemoveBounty(Entity<StationCargoBountyDatabaseComponent?> ent,
+        CargoBountyData data,
+        bool skipped,
+        EntityUid? actor = null)
     {
-        if (!Resolve(uid, ref component))
+        if (!Resolve(ent, ref ent.Comp))
             return false;
 
-        for (var i = 0; i < component.Bounties.Count; i++)
+        for (var i = 0; i < ent.Comp.Bounties.Count; i++)
         {
-            if (component.Bounties[i].Id == data.Id)
+            if (ent.Comp.Bounties[i].Id == data.Id)
             {
-                string? actorName = default;
+                string? actorName = null;
                 if (actor != null)
                 {
-                    var getIdentityEvent = new TryGetIdentityShortInfoEvent(uid, actor.Value);
+                    var getIdentityEvent = new TryGetIdentityShortInfoEvent(ent.Owner, actor.Value);
                     RaiseLocalEvent(getIdentityEvent);
                     actorName = getIdentityEvent.Title;
                 }
 
-                component.History.Add(new CargoBountyHistoryData(data, _gameTiming.CurTime, actorName));
-                component.Bounties.RemoveAt(i);
+                ent.Comp.History.Add(new CargoBountyHistoryData(data,
+                    skipped
+                        ? CargoBountyHistoryData.BountyResult.Skipped
+                        : CargoBountyHistoryData.BountyResult.Completed,
+                    _gameTiming.CurTime,
+                    actorName));
+                ent.Comp.Bounties.RemoveAt(i);
                 return true;
             }
         }

Content.Shared/Cargo/CargoBountyHistoryData.cs

@@ -1,6 +1,6 @@
-using Robust.Shared.Serialization;
 using Content.Shared.Cargo.Prototypes;
 using Robust.Shared.Prototypes;
+using Robust.Shared.Serialization;
 
 namespace Content.Shared.Cargo;
 
@@ -13,34 +13,50 @@ public readonly partial record struct CargoBountyHistoryData
     /// <summary>
     /// A unique id used to identify the bounty
     /// </summary>
-    [DataField, ViewVariables(VVAccess.ReadWrite)]
+    [DataField]
     public string Id { get; init; } = string.Empty;
 
     /// <summary>
-    /// Optional name of the actor that skipped the bounty.
-    /// Only set when the bounty has been skipped.
+    /// Whether this bounty was completed or skipped.
+    /// </summary>
+    [DataField]
+    public BountyResult Result { get; init; } = BountyResult.Completed;
+
+    /// <summary>
+    /// Optional name of the actor that completed/skipped the bounty.
     /// </summary>
-    [DataField, ViewVariables(VVAccess.ReadWrite)]
+    [DataField]
     public string? ActorName { get; init; } = default;
 
     /// <summary>
     /// Time when this bounty was completed or skipped
     /// </summary>
-    [DataField, ViewVariables(VVAccess.ReadWrite)]
+    [DataField]
     public TimeSpan Timestamp { get; init; } = TimeSpan.MinValue;
 
     /// <summary>
     /// The prototype containing information about the bounty.
     /// </summary>
-    [ViewVariables(VVAccess.ReadWrite)]
     [DataField(required: true)]
     public ProtoId<CargoBountyPrototype> Bounty { get; init; } = string.Empty;
 
-    public CargoBountyHistoryData(CargoBountyData bounty, TimeSpan timestamp, string? actorName)
+    public CargoBountyHistoryData(CargoBountyData bounty, BountyResult result, TimeSpan timestamp, string? actorName)
     {
         Bounty = bounty.Bounty;
+        Result = result;
         Id = bounty.Id;
         ActorName = actorName;
         Timestamp = timestamp;
     }
+
+    /// <summary>
+    /// Covers how a bounty was actually finished.
+    ///     Completed - Bounty was actually fulfilled and the goods sold
+    ///     Skipped - Bounty was explicitly skipped by some actor
+    /// </summary>
+    public enum BountyResult
+    {
+        Completed = 0,
+        Skipped = 1,
+    }
 }

Resources/Locale/en-US/cargo/cargo-bounty-console.ftl

@@ -21,7 +21,6 @@ bounty-manifest-list-start = Item manifest:
 
 bounty-console-tab-available-label = Available
 bounty-console-tab-history-label = History
-bounty-console-history-notice-completed-label = {$time} - Completed
-bounty-console-history-notice-skipped-label = {$time} - Skipped by {$id}
-bounty-console-history-completed-label = [color=limegreen]Completed[/color]
-bounty-console-history-skipped-label = [color=red]Skipped[/color]
+bounty-console-history-empty-label = No bounty history found
+bounty-console-history-notice-completed-label = [color=limegreen]Completed[/color]
+bounty-console-history-notice-skipped-label = [color=red]Skipped[/color] by {$id}