From 95500cc8e263dd74f24764da18254856ae770a26 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20Schr=C3=B6der?=
 <fschroeder@techfak.uni-bielefeld.de>
Date: Sat, 9 Mar 2024 21:09:07 +0100
Subject: [PATCH] Refactor 'player_config' usage and improve typing in
 'cooperative_cuisine'

The 'player_config' dictionary is replaced with the PlayerConfig class to provide type-checking and improve readability. This change is applied across different files in the 'cooperative_cuisine' module. Additionally, this commit improves type hints for several variables and parameters to assist with code interpretation and potential error prevention.
---
 cooperative_cuisine/environment.py | 91 ++++++++++++++++++------------
 cooperative_cuisine/movement.py    | 18 +++---
 2 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/cooperative_cuisine/environment.py b/cooperative_cuisine/environment.py
index 04e1409c..2293a840 100644
--- a/cooperative_cuisine/environment.py
+++ b/cooperative_cuisine/environment.py
@@ -17,7 +17,7 @@ from collections import defaultdict
 from datetime import timedelta, datetime
 from pathlib import Path
 from random import Random
-from typing import Literal, TypedDict, Callable
+from typing import Literal, TypedDict, Callable, Set
 
 import numpy as np
 import numpy.typing as npt
@@ -29,6 +29,7 @@ from cooperative_cuisine.counter_factory import (
 )
 from cooperative_cuisine.counters import (
     PlateConfig,
+    Counter,
 )
 from cooperative_cuisine.effects import EffectManager
 from cooperative_cuisine.hooks import (
@@ -101,6 +102,18 @@ class Environment:
     Handles player movement, collision-detection, counters, cooking processes, recipes, incoming orders, time.
     """
 
+    # pdoc does not detect attributes in the tuple assignment in init
+    counters: list[Counter]
+    """Counters of the environment."""
+    designated_player_positions: list[npt.NDArray[float]]
+    """The positions new players can spawn based on the layout (`A` char)."""
+    free_positions: list[npt.NDArray[float]]
+    """list of 2D points of free (no counters) positions in the environment."""
+    kitchen_height: int
+    """Grid height of the kitchen."""
+    kitchen_width: int
+    """Grid width of the kitchen."""
+
     def __init__(
         self,
         env_config: Path | str,
@@ -138,7 +151,7 @@ class Environment:
         self.players: dict[str, Player] = {}
         """the player, keyed by their id/name."""
 
-        self.as_files = as_files
+        self.as_files: bool = as_files
         """Are the configs just the path to the files."""
         if self.as_files:
             with open(env_config, "r") as file:
@@ -152,22 +165,28 @@ class Environment:
             env_config, Loader=yaml.Loader
         )
         """The config of the environment. All environment specific attributes is configured here."""
-
-        self.player_view_restricted: bool = self.environment_config["player_config"][
-            "restricted_view"
-        ]
+        self.environment_config["player_config"] = PlayerConfig(
+            **(
+                self.environment_config["player_config"]
+                if "player_config" in self.environment_config
+                else {}
+            )
+        )
+        self.player_view_restricted: bool = self.environment_config[
+            "player_config"
+        ].restricted_view
         """If field-of-view of players is restricted in this environment."""
         if self.player_view_restricted:
-            self.player_view_angle = self.environment_config["player_config"][
-                "view_angle"
-            ]
-            self.player_view_range = self.environment_config["player_config"][
-                "view_range"
-            ]
+            self.player_view_angle: float = self.environment_config[
+                "player_config"
+            ].view_angle
+            self.player_view_range: float = self.environment_config[
+                "player_config"
+            ].view_range
 
         self.extra_setup_functions()
 
-        self.layout_config = layout_config
+        self.layout_config: str = layout_config
         """The layout config for the environment"""
         # self.counter_side_length = 1  # -> this changed! is 1 now
 
@@ -176,7 +195,7 @@ class Environment:
         self.hook(ITEM_INFO_LOADED, item_info=item_info)
 
         if self.environment_config["orders"]["meals"]["all"]:
-            self.allowed_meal_names = set(
+            self.allowed_meal_names: Set[str] = set(
                 [
                     item
                     for item, info in self.item_info.items()
@@ -184,20 +203,20 @@ class Environment:
                 ]
             )
         else:
-            self.allowed_meal_names = set(
+            self.allowed_meal_names: Set[str] = set(
                 self.environment_config["orders"]["meals"]["list"]
             )
             """The allowed meals depend on the `environment_config.yml` configured behaviour. Either all meals that 
             are possible or only a limited subset."""
 
-        self.order_manager = OrderManager(
+        self.order_manager: OrderManager = OrderManager(
             order_config=self.environment_config["orders"],
             hook=self.hook,
             random=self.random,
         )
         """The manager for the orders and score update."""
 
-        self.counter_factory = CounterFactory(
+        self.counter_factory: CounterFactory = CounterFactory(
             layout_chars_config=self.environment_config["layout_chars"],
             item_info=self.item_info,
             serving_window_additional_kwargs={
@@ -233,7 +252,7 @@ class Environment:
         ) = self.counter_factory.parse_layout_file(self.layout_config)
         self.hook(LAYOUT_FILE_PARSED)
 
-        self.movement = Movement(
+        self.movement: Movement = Movement(
             counter_positions=np.array([c.pos for c in self.counters]),
             player_config=self.environment_config["player_config"],
             world_borders=np.array(
@@ -244,7 +263,7 @@ class Environment:
         )
         """Does the movement of players in each step."""
 
-        self.progressing_counters = []
+        self.progressing_counters: list[Counter] = []
         """Counters that needs to be called in the step function via the `progress` method."""
 
         self.effect_manager: dict[
@@ -260,7 +279,7 @@ class Environment:
             else True
         )
 
-        self.recipe_validation = Validation(
+        self.recipe_validation: Validation = Validation(
             meals=[m for m in self.item_info.values() if m.type == ItemType.Meal]
             if self.environment_config["orders"]["meals"]["all"]
             else [
@@ -280,7 +299,7 @@ class Environment:
         available_meals = {meal: self.item_info[meal] for meal in meals_to_be_ordered}
         self.order_manager.set_available_meals(available_meals)
         self.order_manager.create_init_orders(self.env_time)
-        self.start_time = self.env_time
+        self.start_time: datetime = self.env_time
         """The relative env time when it started."""
         self.env_time_end = self.env_time + timedelta(
             seconds=self.environment_config["game"]["time_limit_seconds"]
@@ -299,6 +318,11 @@ class Environment:
             env_start_time_worldtime=datetime.now(),
         )
 
+    @property
+    def game_ended(self) -> bool:
+        """Whether the game is over or not based on the calculated `Environment.env_time_end`"""
+        return self.env_time >= self.env_time_end
+
     def overwrite_counters(self, counters):
         """Resets counters.
 
@@ -340,12 +364,7 @@ class Environment:
         for manager in self.effect_manager.values():
             manager.set_counters(counters)
 
-    @property
-    def game_ended(self) -> bool:
-        """Whether the game is over or not based on the calculated `Environment.env_time_end`"""
-        return self.env_time >= self.env_time_end
-
-    def get_env_time(self):
+    def get_env_time(self) -> datetime:
         """the internal time of the environment. An environment starts always with the time from `create_init_env_time`.
 
         Utility method to pass a reference to the serving window."""
@@ -405,19 +424,16 @@ class Environment:
         Args:
             player_name: The id/name of the player to reference actions and in the state.
             pos: The optional init position of the player.
+
+        Raises:
+            ValueError: if the player_name already exists.
         """
         if player_name in self.players:
             raise ValueError(f"Player {player_name} already exists.")
         log.debug(f"Add player {player_name} to the game")
         player = Player(
             player_name,
-            player_config=PlayerConfig(
-                **(
-                    self.environment_config["player_config"]
-                    if "player_config" in self.environment_config
-                    else {}
-                )
-            ),
+            player_config=self.environment_config["player_config"],
             hook=self.hook,
             pos=pos,
         )
@@ -464,7 +480,9 @@ class Environment:
                 effect_manager.progress(passed_time=passed_time, now=self.env_time)
         self.hook(POST_STEP, passed_time=passed_time)
 
-    def get_state(self, player_id: str = None, additional_key_values: dict = None):
+    def get_state(
+        self, player_id: str = None, additional_key_values: dict = None
+    ) -> dict:
         """Get the current state of the game environment. The state here is accessible by the current python objects.
 
         Args:
@@ -506,8 +524,7 @@ class Environment:
                 "info_msg": [
                     (msg["msg"], msg["level"])
                     for msg in self.info_msgs_per_player[player_id]
-                    if msg["start_time"] < self.env_time
-                    and msg["end_time"] > self.env_time
+                    if msg["start_time"] < self.env_time < msg["end_time"]
                 ],
                 **(additional_key_values if additional_key_values else {}),
             }
diff --git a/cooperative_cuisine/movement.py b/cooperative_cuisine/movement.py
index 0105065c..d8431fae 100644
--- a/cooperative_cuisine/movement.py
+++ b/cooperative_cuisine/movement.py
@@ -12,7 +12,7 @@ from scipy.spatial import distance_matrix
 
 from cooperative_cuisine.counters import Counter
 from cooperative_cuisine.hooks import Hooks, PLAYERS_COLLIDE
-from cooperative_cuisine.player import Player
+from cooperative_cuisine.player import Player, PlayerConfig
 
 
 class Movement:
@@ -23,7 +23,13 @@ class Movement:
     world_borders_upper = None
     """World borders upper bounds."""
 
-    def __init__(self, counter_positions, player_config, world_borders, hook: Hooks):
+    def __init__(
+        self,
+        counter_positions: npt.NDArray[float],
+        player_config: PlayerConfig,
+        world_borders: npt.NDArray[float],
+        hook: Hooks,
+    ):
         """Constructor of Movement.
 
         Args:
@@ -34,13 +40,11 @@ class Movement:
         """
         self.counter_positions: npt.NDArray[float] = counter_positions
         """Positions of all counters in an environment. Needs to be updated if the counters position changes."""
-        self.player_radius: float = player_config["radius"]
+        self.player_radius: float = player_config.radius
         """The radius of a player (indicating its size and collision sphere). Relative to one grid cell, e.g., `0.4`."""
-        self.player_interaction_range: float = player_config["interaction_range"]
+        self.player_interaction_range: float = player_config.interaction_range
         """The range of how far a player can interact with the closest counter."""
-        self.player_movement_speed: float | int = player_config[
-            "speed_units_per_seconds"
-        ]
+        self.player_movement_speed: float | int = player_config.speed_units_per_seconds
         """How many grid cells a player can move in a second."""
         self.world_borders: npt.NDArray[float] = world_borders
         """The world border arrays. For easier numpy comparison with player position. The outer dimension needs to be 
-- 
GitLab