Refactor scenery group creation.

Breaking up some large methods and shifting error handling to places
where it can catch more mistakes.

game/scenery_group.py

@@ -1,96 +1,109 @@
 from __future__ import annotations
 
-from typing import Iterable, List
+from typing import Iterable
 
+from dcs import Point
 from dcs.triggers import TriggerZoneCircular
 
 from game.theater.theatergroundobject import NAME_BY_CATEGORY
 
 
-class SceneryGroupError(RuntimeError):
-    """Error for when there are insufficient conditions to create a SceneryGroup."""
-
-    pass
-
-
 class SceneryGroup:
     """Store information about a scenery objective."""
 
     def __init__(
         self,
-        group_zone: TriggerZoneCircular,
+        name: str,
-        target_zones: Iterable[TriggerZoneCircular],
+        centroid: Point,
         category: str,
+        target_zones: Iterable[TriggerZoneCircular],
     ) -> None:
+        if not target_zones:
+            raise ValueError(f"{name} has no valid target zones")
 
-        self.group_zone = group_zone
+        if category not in NAME_BY_CATEGORY:
-        self.target_zones = target_zones
+            raise ValueError(
-        self.centroid = group_zone.position
+                f"Campaign objective {name} uses unknown scenery objective "
+                f"category: {category}"
+            )
+
+        self.name = name
+        self.centroid = centroid
         self.category = category
+        self.target_zones = list(target_zones)
+
+    @staticmethod
+    def category_of(group_zone: TriggerZoneCircular) -> str:
+        try:
+            # The first (1-indexed because lua) property of the group zone defines the
+            # TGO category.
+            category = group_zone.properties[1].get("value").lower()
+        except IndexError as ex:
+            raise RuntimeError(
+                f"{group_zone.name} does not define an objective category"
+            ) from ex
+        return category
+
+    @staticmethod
+    def from_group_zone(
+        group_zone: TriggerZoneCircular,
+        unclaimed_target_zones: list[TriggerZoneCircular],
+    ) -> SceneryGroup:
+        return SceneryGroup(
+            group_zone.name,
+            group_zone.position,
+            SceneryGroup.category_of(group_zone),
+            SceneryGroup.claim_targets_for(group_zone, unclaimed_target_zones),
+        )
 
     @staticmethod
     def from_trigger_zones(
         trigger_zones: Iterable[TriggerZoneCircular],
-    ) -> List[SceneryGroup]:
+    ) -> list[SceneryGroup]:
-        """Define scenery objectives based on their encompassing blue/red circle."""
+        """Define scenery objectives based on their encompassing blue circle."""
+        group_zones, target_zones = SceneryGroup.collect_scenery_zones(trigger_zones)
+        return [SceneryGroup.from_group_zone(z, target_zones) for z in group_zones]
+
+    @staticmethod
+    def claim_targets_for(
+        group_zone: TriggerZoneCircular,
+        unclaimed_target_zones: list[TriggerZoneCircular],
+    ) -> list[TriggerZoneCircular]:
+        claimed_zones = []
+        for zone in list(unclaimed_target_zones):
+            if zone.position.distance_to_point(group_zone.position) < group_zone.radius:
+                claimed_zones.append(zone)
+                unclaimed_target_zones.remove(zone)
+        return claimed_zones
+
+    @staticmethod
+    def collect_scenery_zones(
+        zones: Iterable[TriggerZoneCircular],
+    ) -> tuple[list[TriggerZoneCircular], list[TriggerZoneCircular]]:
         group_zones = []
         target_zones = []
-
+        for zone in zones:
-        scenery_groups = []
-
-        # Aggregate trigger zones into different groups based on color.
-        for zone in trigger_zones:
             if SceneryGroup.is_group_zone(zone):
                 group_zones.append(zone)
             if SceneryGroup.is_target_zone(zone):
                 target_zones.append(zone)
+            # No error on else. We're iterating over all the trigger zones in the miz,
+            # and others might be used for something else.
+        return group_zones, target_zones
 
-        # For each objective definition.
+    @staticmethod
-        for group_zone in group_zones:
+    def zone_has_color_rgb(
-
+        zone: TriggerZoneCircular, r: float, g: float, b: float
-            zone_def_radius = group_zone.radius
+    ) -> bool:
-            zone_def_position = group_zone.position
+        # TriggerZone.color is a dict with keys 1 through 4, each being a component of
-            zone_def_name = group_zone.name
+        # RGBA. It's absurd that it's a dict, but that's a lua quirk that's leaking from
-
+        # pydcs.
-            if len(group_zone.properties) == 0:
+        return (zone.color[1], zone.color[2], zone.color[3]) == (r, g, b)
-                raise SceneryGroupError(
-                    "Undefined SceneryGroup category in TriggerZone: " + zone_def_name
-                )
-
-            # Arbitrary campaign design requirement:  First property must define the category.
-            zone_def_category = group_zone.properties[1].get("value").lower()
-
-            valid_target_zones = []
-
-            for zone in list(target_zones):
-                if zone.position.distance_to_point(zone_def_position) < zone_def_radius:
-                    valid_target_zones.append(zone)
-                    target_zones.remove(zone)
-
-            if len(valid_target_zones) > 0 and zone_def_category in NAME_BY_CATEGORY:
-                scenery_groups.append(
-                    SceneryGroup(group_zone, valid_target_zones, zone_def_category)
-                )
-            elif len(valid_target_zones) == 0:
-                raise SceneryGroupError(
-                    "No white triggerzones found in: " + zone_def_name
-                )
-            elif zone_def_category not in NAME_BY_CATEGORY:
-                raise SceneryGroupError(
-                    "Incorrect TriggerZone category definition for: "
-                    + zone_def_name
-                    + " in campaign definition.  TriggerZone category: "
-                    + zone_def_category
-                )
-
-        return scenery_groups
 
     @staticmethod
     def is_group_zone(zone: TriggerZoneCircular) -> bool:
-        # Blue in RGB is [0 Red], [0 Green], [1 Blue]. Ignore the fourth position: Transparency.
+        return SceneryGroup.zone_has_color_rgb(zone, r=0, g=0, b=1)
-        return zone.color[1] == 0 and zone.color[2] == 0 and zone.color[3] == 1
 
     @staticmethod
     def is_target_zone(zone: TriggerZoneCircular) -> bool:
-        # White in RGB is [1 Red], [1 Green], [1 Blue]. Ignore the fourth position: Transparency.
+        return SceneryGroup.zone_has_color_rgb(zone, r=1, g=1, b=1)
-        return zone.color[1] == 1 and zone.color[2] == 1 and zone.color[3] == 1

game/theater/start_generator.py

@@ -380,12 +380,12 @@ class AirbaseGroundObjectGenerator(ControlPointGroundObjectGenerator):
         g = tgo_type(
             namegen.random_objective_name(),
             scenery.category,
-            PresetLocation(scenery.group_zone.name, scenery.centroid),
+            PresetLocation(scenery.name, scenery.centroid),
             self.control_point,
         )
         ground_group = TheaterGroup(
             self.game.next_group_id(),
-            scenery.group_zone.name,
+            scenery.name,
             PointWithHeading.from_point(scenery.centroid, Heading.from_degrees(0)),
             [],
             g,