From 1ffba1b7e8342b693a984239ec09b3fc96e808a8 Mon Sep 17 00:00:00 2001
From: cpojer <christoph.pojer@gmail.com>
Date: Fri, 1 Nov 2024 10:15:22 +0900
Subject: [PATCH] Do not hide labels of a hidden objective if a visible
 objective matching the same label exists.

GitOrigin-RevId: 21bd31d10162cdfdd9aad5f093686af381b6cb45
---
 athena/Objectives.tsx                | 28 +++++++++++------------
 hera/editor/lib/ObjectiveCard.tsx    | 24 ++++++++++++--------
 tests/__tests__/EntityLabel.test.tsx | 34 +++++++++++++++++++++++++++-
 3 files changed, 61 insertions(+), 25 deletions(-)

diff --git a/athena/Objectives.tsx b/athena/Objectives.tsx
index 330c9b73..43955244 100644
--- a/athena/Objectives.tsx
+++ b/athena/Objectives.tsx
@@ -923,26 +923,26 @@ export function getHiddenLabels(objectives: Objectives): PlayerIDSet | null {
     return null;
   }
 
-  let labels: Set<PlayerID> | null = null;
+  const labels: Array<PlayerID> = [];
+  const visibleLabels = new Set<PlayerID>();
   for (const [, objective] of objectives) {
-    if (
-      objective.hidden &&
-      objectiveHasLabel(objective) &&
-      objective.label &&
-      (!objective.optional ||
-        !objective.completed ||
-        objective.completed.size < 1)
-    ) {
+    if (objectiveHasLabel(objective) && objective.label) {
       for (const label of objective.label) {
-        if (!labels) {
-          labels = new Set();
+        if (
+          objective.hidden &&
+          (!objective.optional ||
+            !objective.completed ||
+            objective.completed.size < 1)
+        ) {
+          labels.push(label);
+        } else {
+          visibleLabels.add(label);
         }
-
-        labels.add(label);
       }
     }
   }
-  return labels;
+  const hiddenLabels = labels.filter((label) => !visibleLabels.has(label));
+  return hiddenLabels.length ? new Set(hiddenLabels) : null;
 }
 
 export function getInitialObjective(
diff --git a/hera/editor/lib/ObjectiveCard.tsx b/hera/editor/lib/ObjectiveCard.tsx
index 9925875e..79f33d59 100644
--- a/hera/editor/lib/ObjectiveCard.tsx
+++ b/hera/editor/lib/ObjectiveCard.tsx
@@ -267,9 +267,9 @@ export default function ObjectiveCard({
               </Stack>
             </label>
           )}
-          <Stack alignCenter className={lineHeightStyle} gap>
-            <label>
-              <Stack gap start>
+          <Stack className={lineHeightStyle} gap={16} nowrap>
+            <label className={labelStyle}>
+              <Stack gap nowrap start>
                 <span className={labelWidthStyle}>
                   <fbt desc="Label for secret objective checkbox">Secret</fbt>
                 </span>
@@ -285,20 +285,20 @@ export default function ObjectiveCard({
                 />
               </Stack>
             </label>
-            {objective.hidden && hasLabel && (
+            {objective.hidden && hasLabel && objective.label?.size ? (
               <p className={lightStyle}>
                 <fbt desc="Description for secret objective labels">
-                  Labels associated with this objective will be hidden from all
-                  players.
+                  Labels for this objective are hidden from players unless they
+                  are used by a non-secret objective.
                 </fbt>
               </p>
-            )}
+            ) : null}
           </Stack>
           {!isDefaultObjective && (
             <>
-              <Stack alignCenter className={lineHeightStyle} gap>
-                <label>
-                  <Stack gap start>
+              <Stack className={lineHeightStyle} gap={16} nowrap>
+                <label className={labelStyle}>
+                  <Stack gap nowrap start>
                     <span className={labelWidthStyle}>
                       <fbt desc="Label for optional objective checkbox">
                         Optional
@@ -695,3 +695,7 @@ const lineHeightStyle = css`
 const crystalScaleStyle = css`
   transform: scale(0.66);
 `;
+
+const labelStyle = css`
+  height: fit-content;
+`;
diff --git a/tests/__tests__/EntityLabel.test.tsx b/tests/__tests__/EntityLabel.test.tsx
index 45f07f78..bfae8b9d 100644
--- a/tests/__tests__/EntityLabel.test.tsx
+++ b/tests/__tests__/EntityLabel.test.tsx
@@ -82,7 +82,7 @@ test('carries labels forward when creating buildings or units', async () => {
   );
 });
 
-test('carries labels forward when transporting units', async () => {
+test('does not lose labels when transporting units', async () => {
   const from = vec(1, 1);
   const toA = vec(2, 1);
   const initialMap = map.copy({
@@ -167,3 +167,35 @@ test('drops labels from hidden objectives', () => {
 
   expect(mapA.units.get(vecC)!.label).toBe(2);
 });
+
+test('does not drops labels from hidden objectives if they are also part of a visible objective', () => {
+  const vecA = vec(1, 1);
+  const initialMap = map.copy({
+    buildings: map.buildings.set(vecA, Barracks.create(1, { label: 3 })),
+    config: map.config.copy({
+      objectives: ImmutableMap([
+        [
+          0,
+          {
+            hidden: false,
+            label: new Set([3]),
+            optional: false,
+            type: Criteria.CaptureLabel,
+          } as const,
+        ],
+        [
+          1,
+          {
+            hidden: true,
+            label: new Set([3]),
+            optional: false,
+            type: Criteria.CaptureLabel,
+          } as const,
+        ],
+      ]),
+    }),
+  });
+
+  const mapA = dropLabels(initialMap);
+  expect(mapA.buildings.get(vecA)!.label).toBe(3);
+});