From 9b37edd1a9b9e82113e0017b02a9a050fbfa03ad Mon Sep 17 00:00:00 2001
From: TsjipTsjip <19798667+TsjipTsjip@users.noreply.github.com>
Date: Mon, 8 Jan 2024 18:06:58 +0100
Subject: [PATCH] Mapchecker (#844)

* Create pr-labeler.yml

* Update labeler.yml

* Mapchecker initial version

* Create frontier-mapchecker.yml

* Update whitelists

* Unrestrict allowable stamps and cat ears

* Thanos snap illegal shit

* Update frontier-mapchecker.yml

* Fix aggressive matching

* Add TODO note

* Revert "Thanos snap illegal shit"

This reverts commit 20abe7b9d902071878f5f43a1b85fbd2074a585b.

* Introduce technical debt into whitelist.

* Fix conditional checking semantics.

* Add blanket-whitelisting for maps, these are never checked.

* Actually skip blanket-whitelisted files. Also remove a print I left in on accident.

* Fix bugs I introduced before

* Move blackmarket whitelists to their own conditional matching group, to demonstrate this works mostly

* Add verbosity flag to mapchecker

* Merge upstream/master into origin/master, adjust default paths

* Make encryption keys except for common and traffic tagged with DO NOT MAP

* Technical debt for encryption keys

* Some more blacklists: Radios and the x-01 multiphase

* Resolve future merge conflict with #821

* Expand mapchecker to accept group overrides per-shuttle

* Update MapChecker documentation

* Resolve pipeline failures

* De-upset mapchecker
---
 .github/mapchecker/README.md                  | 42 ++++++++++++++++++-
 .github/mapchecker/mapchecker.py              | 14 ++++++-
 .../Shipyard/Prototypes/VesselPrototype.cs    |  7 ++++
 Resources/Prototypes/_NF/Shipyard/cleric.yml  |  1 +
 Resources/Prototypes/_NF/Shipyard/fighter.yml |  1 +
 Resources/Prototypes/_NF/Shipyard/rogue.yml   |  1 +
 6 files changed, 64 insertions(+), 2 deletions(-)

diff --git a/.github/mapchecker/README.md b/.github/mapchecker/README.md
index 0e99a897e26..f31a97246aa 100644
--- a/.github/mapchecker/README.md
+++ b/.github/mapchecker/README.md
@@ -6,8 +6,31 @@ directory which are marked as `DO NOT MAP`, `DEBUG`, ... and verifies that map c
 
 ## Usage
 
-Glad I do not have to write this myself! Get detailed help information by running: `./mapchecker.py --help`
+Glad I do not have to write this myself! Get detailed help information by running:
+`python3 .github/mapchecker/mapchecker.py --help`
 
+The following help block is printed:
+```
+usage: mapchecker.py [-h] [-v] [-p PROTOTYPES_PATH [PROTOTYPES_PATH ...]] [-m MAP_PATH [MAP_PATH ...]] [-w WHITELIST]
+
+Map prototype usage checker for Frontier Station 14.
+
+options:
+  -h, --help            show this help message and exit
+  -v, --verbose         Sets log level to DEBUG if present, spitting out a lot more information. False by default,.
+  -p PROTOTYPES_PATH [PROTOTYPES_PATH ...], --prototypes_path PROTOTYPES_PATH [PROTOTYPES_PATH ...]
+                        Directory holding entity prototypes. Default: All entity prototypes in the Frontier Station 14 codebase.
+  -m MAP_PATH [MAP_PATH ...], --map_path MAP_PATH [MAP_PATH ...]
+                        Map PROTOTYPES or directory of map prototypes to check. Can mix and match.Default: All maps in the Frontier Station 14 codebase.
+  -w WHITELIST, --whitelist WHITELIST
+                        YML file that lists map names and prototypes to allow for them.
+```
+
+You should generally not need to configure `-p`, `-m` or `-w`, as they are autofilled with sensible defaults. You can do
+this:
+- Set `-p` to only check against prototypes in a specific directory.
+- Set `-m` to just check a specific map. (Make sure to **point it at the prototype**, not the map file itself!)
+- Set `-v` with `-m` set as per above to get detailed information about a possible rejection for just that map.
 
 ## Configuration
 
@@ -31,3 +54,20 @@ If a map has a prototype and you believe it should be whitelisted, add a key for
 gameMap prototype), and add the prototype ID's to its list.
 
 The whitelist the checker uses by default is `.github/mapchecker/whitelist.yml`.
+
+## Shuttle group override
+
+It is possible that a shuttle is set to group `None` because it is only used in custom shipyard listings. In this case,
+you can force the MapChecker script to treat it as a different shipyard group by adding the following to the vessel
+prototype:
+
+```yml
+  ...
+  group: None
+  # Add this line below.
+  mapchecker_group_override: ShipyardGroupHere
+  ...
+```
+
+Note that for now this will cause a warning to be generated, but it will not cause a failure if the shuttle matches the
+criteria for the overridden group.
diff --git a/.github/mapchecker/mapchecker.py b/.github/mapchecker/mapchecker.py
index 2ba739d9121..e57606487ec 100755
--- a/.github/mapchecker/mapchecker.py
+++ b/.github/mapchecker/mapchecker.py
@@ -120,6 +120,7 @@
     # Collect all prototypes and sort into the collectors.
     for proto_file in proto_paths:
         with open(proto_file, "r") as proto:
+            logger.debug(f"Reading prototype file '{proto_file}'.")
             file_data = yaml.load(proto, Loader=YamlLoaderIgnoringTags)
             if file_data is None:
                 continue
@@ -146,7 +147,6 @@
         for item in conditionally_illegal_prototypes[key]:
             logger.debug(f" - {item}")
 
-
     # ==================================================================================================================
     # PHASE 2: Check all maps in map_proto_paths for illegal prototypes.
 
@@ -164,6 +164,9 @@
             map_name = map_proto  # The map name that will be reported over output.
             map_file_location = None
             shipyard_group = None  # Shipyard group of this map, if it's a shuttle.
+            # Shipyard override of this map, in the case it's a custom shipyard shuttle but needs to be treated as a
+            # specific group.
+            shipyard_override = None
 
             for item in file_data:
                 if item["type"] == "gameMap":
@@ -174,6 +177,7 @@
                 if item["type"] == "vessel":
                     # This yaml entry is a vessel descriptor!
                     shipyard_group = item["group"] if "group" in item.keys() else None
+                    shipyard_override = item["mapchecker_group_override"] if "mapchecker_group_override" in item.keys() else None
 
             if map_file_location is None:
                 # Silently skip. If the map doesn't have a mapPath, it won't appear in game anyways.
@@ -185,6 +189,14 @@
                 logger.warning(f"Map '{map_name}' (from prototype '{map_proto}') was blanket-whitelisted. Skipping it.")
                 continue
 
+            if shipyard_override is not None:
+                # Log a warning, indicating the override and the normal group this shuttle belongs to, then set
+                # shipyard_group to the override.
+                logger.warning(f"Map '{map_name}' (from prototype '{map_proto}') is using mapchecker_group_override. "
+                               f"This map will be treated as a '{shipyard_override}' shuttle. (Normally: "
+                               f"'{shipyard_group}'))")
+                shipyard_group = shipyard_override
+
             logger.debug(f"Starting checks for '{map_name}' (Path: '{map_file_location}' | Shipyard: '{shipyard_group}')")
 
             # Now construct a temporary list of all prototype ID's that are illegal for this map based on conditionals.
diff --git a/Content.Shared/Shipyard/Prototypes/VesselPrototype.cs b/Content.Shared/Shipyard/Prototypes/VesselPrototype.cs
index 4749f6be7ed..c025c9feacc 100644
--- a/Content.Shared/Shipyard/Prototypes/VesselPrototype.cs
+++ b/Content.Shared/Shipyard/Prototypes/VesselPrototype.cs
@@ -39,6 +39,13 @@ public sealed class VesselPrototype : IPrototype
     [DataField("group")]
     public string Group = string.Empty;
 
+    /// Frontier - Add this field for the MapChecker script.
+    /// <summary>
+    ///     The MapChecker override group for this vessel.
+    /// </summary>
+    [DataField("mapchecker_group_override")]
+    public string MapcheckerGroup = string.Empty;
+
     /// <summary>
     ///     Relative directory path to the given shuttle, i.e. `/Maps/Shuttles/yourshittle.yml`
     /// </summary>
diff --git a/Resources/Prototypes/_NF/Shipyard/cleric.yml b/Resources/Prototypes/_NF/Shipyard/cleric.yml
index f4f706fd8db..77422bbf2ae 100644
--- a/Resources/Prototypes/_NF/Shipyard/cleric.yml
+++ b/Resources/Prototypes/_NF/Shipyard/cleric.yml
@@ -5,6 +5,7 @@
   price: 10800 #Appraisal is 10500
   category: Small
   group: None
+  mapchecker_group_override: Security  # Treat this as a security vessel for mapchecker purposes
   shuttlePath: /Maps/Shuttles/cleric.yml
 
 - type: gameMap
diff --git a/Resources/Prototypes/_NF/Shipyard/fighter.yml b/Resources/Prototypes/_NF/Shipyard/fighter.yml
index ae8ec1a2610..560e4556021 100644
--- a/Resources/Prototypes/_NF/Shipyard/fighter.yml
+++ b/Resources/Prototypes/_NF/Shipyard/fighter.yml
@@ -5,6 +5,7 @@
   price: 9000 #not sure how much mark up % to add but the appraisal is 8491$
   category: Small
   group: None
+  mapchecker_group_override: Security  # Treat this as a security vessel for mapchecker purposes
   shuttlePath: /Maps/Shuttles/fighter.yml
 
 - type: gameMap
diff --git a/Resources/Prototypes/_NF/Shipyard/rogue.yml b/Resources/Prototypes/_NF/Shipyard/rogue.yml
index de155d51d9f..33d40b8bb20 100644
--- a/Resources/Prototypes/_NF/Shipyard/rogue.yml
+++ b/Resources/Prototypes/_NF/Shipyard/rogue.yml
@@ -5,6 +5,7 @@
   price: 8200 #the appraisal is 7941$
   category: Small
   group: None
+  mapchecker_group_override: Security  # Treat this as a security vessel for mapchecker purposes
   shuttlePath: /Maps/Shuttles/rogue.yml
 
 - type: gameMap