From bef24476c0034f3e964e90ef613cc265f5dcf648 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20Schr=C3=B6der?=
 <fschroeder@techfak.uni-bielefeld.de>
Date: Tue, 5 Mar 2024 13:02:40 +0100
Subject: [PATCH] Update code comments and typespecs across modules

Enhanced overall code clarity by adding and revising detailed comments, docstrings, and typespecs in various Python modules. Significant changes include refining functionality of effect management in 'effects.py', modifications to use of 'numpy' arrays and functions in 'movement.py', as well as adjustments to handling hooks. Additionally, minor updates in 'configs/study/level1/level1_config.yaml' and 'counter_factory.py' were made.
---
 .../configs/study/level1/level1_config.yaml   |  3 +-
 cooperative_cuisine/counter_factory.py        |  1 -
 cooperative_cuisine/effects.py                | 69 +++++++++++++++--
 cooperative_cuisine/environment.py            | 24 ++++--
 cooperative_cuisine/game_server.py            |  4 +
 cooperative_cuisine/hooks.py                  | 76 ++++++++++++++++++-
 cooperative_cuisine/info_msg.py               | 23 ++++++
 cooperative_cuisine/items.py                  |  6 +-
 cooperative_cuisine/movement.py               | 56 ++++++++++++--
 cooperative_cuisine/orders.py                 |  9 +++
 cooperative_cuisine/pygame_2d_vis/gui.py      |  1 +
 cooperative_cuisine/recording.py              |  1 +
 cooperative_cuisine/study_server.py           |  9 ++-
 13 files changed, 256 insertions(+), 26 deletions(-)

diff --git a/cooperative_cuisine/configs/study/level1/level1_config.yaml b/cooperative_cuisine/configs/study/level1/level1_config.yaml
index c46f38eb..10b78792 100644
--- a/cooperative_cuisine/configs/study/level1/level1_config.yaml
+++ b/cooperative_cuisine/configs/study/level1/level1_config.yaml
@@ -14,7 +14,8 @@ meals:
   # if all: false -> only orders for these meals are generated
   # TODO: what if this list is empty?
   list:
-    - Burger
+    - TomatoSoup
+    - Salad
 
 layout_chars:
   _: Free
diff --git a/cooperative_cuisine/counter_factory.py b/cooperative_cuisine/counter_factory.py
index 592def76..02f6d5d7 100644
--- a/cooperative_cuisine/counter_factory.py
+++ b/cooperative_cuisine/counter_factory.py
@@ -371,7 +371,6 @@ class CounterFactory:
                     random=self.random,
                     **self.effect_manager_config[effect.manager]["kwargs"],
                 )
-                manager.set_counters(counters)
                 effect_manager[effect.manager] = manager
 
             manager.add_effect(effect)
diff --git a/cooperative_cuisine/effects.py b/cooperative_cuisine/effects.py
index 0e5d3ec6..fe73ddf4 100644
--- a/cooperative_cuisine/effects.py
+++ b/cooperative_cuisine/effects.py
@@ -30,32 +30,87 @@ if TYPE_CHECKING:
 
 
 class EffectManager:
+    """The EffectManager class is responsible for managing effects in an environment.
+
+    It provides methods to add effects, set counters, register new active effects, check if an effect is active,
+    remove active effects, and progress the manager.
+    """
+
     def __init__(self, hook: Hooks, random: Random) -> None:
         self.effects = []
+        """A list of ItemInfo objects representing the effects managed by the manager."""
         self.counters = []
+        """A list of Counter objects representing the counters in the environment."""
         self.hook = hook
+        """An instance of the Hooks class representing the hooks in the environment."""
         self.new_effects: list[Tuple[Effect, Item | Counter]] = []
+        """A list of tuples containing an Effect object and either an Item or Counter object representing the new 
+        active effects."""
         self.random = random
+        """An instance of the Random class representing the random number generator."""
 
     def add_effect(self, effect: ItemInfo):
+        """Add the effect (item) info that the manager handles.
+
+        Args:
+            effect: An object of type ItemInfo representing the effect to be added.
+
+        """
         self.effects.append(effect)
 
     def set_counters(self, counters: list[Counter]):
-        self.counters.extend(counters)
+        """Sets the counters of the environment.
+
+        Args:
+            counters: A list of Counter objects representing the counters.
+        """
+        self.counters = counters.copy()
 
     def register_active_effect(self, effect: Effect, target: Item | Counter):
+        """Register new active effects on a target item or counter.
+
+        Args:
+            effect: An instance of the Effect class representing the active effect to be registered.
+            target: An instance of either the Item or Counter class to which the effect will be applied.
+
+        """
         target.active_effects.append(effect)
         self.new_effects.append((effect, target))
 
     def progress(self, passed_time: timedelta, now: datetime):
+        """Iterative progress call on the manager.
+
+        Similar to the `Counter.progress` method.
+        How often it is called depends on the `env_step_frequency` or how often `step` is called.
+
+        Args:
+            passed_time: The amount of time that has passed since the last call of the method.
+            now: The current env time.
+        """
         ...
 
-    def can_start_effect_transition(
-        self, effect: ItemInfo, target: Item | Counter
-    ) -> bool:
-        return effect.name not in [e.name for e in target.active_effects]
+    @staticmethod
+    def effect_is_active(effect: ItemInfo, target: Item | Counter) -> bool:
+        """Checks if a given effect is active on a specified target.
+
+        Args:
+            effect: An instance of ItemInfo representing the effect to check.
+            target: An instance of Item or Counter representing the target on which the effect is being checked.
+
+        Returns:
+            A boolean value indicating whether the effect is active on the target. True if active, False otherwise.
+
+        """
+        return effect.name in [e.name for e in target.active_effects]
 
     def remove_active_effect(self, effect: Effect, target: Item | Counter):
+        """Removes an active effect from a target item or counter.
+
+        Args:
+            effect (Effect): The effect to be removed.
+            target (Item | Counter): The target item or counter from which the effect will be removed.
+
+        """
         ...
 
 
@@ -89,7 +144,9 @@ class FireEffectManager(EffectManager):
 
         """
         if self.new_effects:
+            # new effects since the last progress call
             for effect, target in self.new_effects:
+                # update next fire spreading
                 self.effect_to_timer[effect.uuid] = now + timedelta(
                     seconds=self.random.uniform(*self.spreading_duration)
                 )
@@ -98,6 +155,7 @@ class FireEffectManager(EffectManager):
                 )
                 self.hook(NEW_FIRE, target=target)
                 self.active_effects.append((effect, target))
+            # reset new effects
             self.new_effects = []
         if self.next_finished_timer < now:
             for effect, target in self.active_effects:
@@ -109,6 +167,7 @@ class FireEffectManager(EffectManager):
                         for counter in touching:
                             if counter.occupied_by:
                                 if isinstance(counter.occupied_by, deque):
+                                    # only the top object on a plate stack is burning
                                     self.apply_effect(effect, counter.occupied_by[-1])
                                 else:
                                     self.apply_effect(effect, counter.occupied_by)
diff --git a/cooperative_cuisine/environment.py b/cooperative_cuisine/environment.py
index 70623f69..c62821d3 100644
--- a/cooperative_cuisine/environment.py
+++ b/cooperative_cuisine/environment.py
@@ -1,3 +1,12 @@
+"""
+The _Cooperative Cuisine_ environment. It is configured via three configs:
+- [`environment_config.yml`](https://gitlab.ub.uni-bielefeld.de/scs/cocosy/overcooked-simulator/-/blob/main/cooperative_cuisine/configs/environment_config.yaml?ref_type=heads)
+- [`xyz.layout`](https://gitlab.ub.uni-bielefeld.de/scs/cocosy/overcooked-simulator/-/blob/main/cooperative_cuisine/configs/layouts/basic.layout?ref_type=heads)
+- [`item_info.yml`](https://gitlab.ub.uni-bielefeld.de/scs/cocosy/overcooked-simulator/-/blob/main/cooperative_cuisine/configs/item_info.yaml?ref_type=heads)
+
+You can pass either the file path or the file content (str) to the `Environment`class of the config files. Set the `as_files` parameter accordingly.
+
+"""
 from __future__ import annotations
 
 import inspect
@@ -60,9 +69,7 @@ from cooperative_cuisine.utils import (
 from cooperative_cuisine.validation import Validation
 
 log = logging.getLogger(__name__)
-
-
-PREVENT_SQUEEZING_INTO_OTHER_PLAYERS = True
+"""The logger for this module."""
 
 
 class EnvironmentConfig(TypedDict):
@@ -214,6 +221,11 @@ class Environment:
 
         self.progressing_counters = []
         """Counters that needs to be called in the step function via the `progress` method."""
+
+        self.effect_manager: dict[
+            str, EffectManager
+        ] = self.counter_factory.setup_effect_manger(self.counters)
+
         self.overwrite_counters(self.counters)
 
         self.recipe_validation = Validation(
@@ -242,10 +254,6 @@ class Environment:
         """The relative env time when it will stop/end"""
         log.debug(f"End time: {self.env_time_end}")
 
-        self.effect_manager: dict[
-            str, EffectManager
-        ] = self.counter_factory.setup_effect_manger(self.counters)
-
         self.info_msgs_per_player: dict[str, list[InfoMsg]] = defaultdict(list)
 
         self.hook(
@@ -276,6 +284,8 @@ class Environment:
                 self.counters,
             )
         )
+        for manager in self.effect_manager.values():
+            manager.set_counters(counters)
 
     @property
     def game_ended(self) -> bool:
diff --git a/cooperative_cuisine/game_server.py b/cooperative_cuisine/game_server.py
index c1e82f2f..c8ee856b 100644
--- a/cooperative_cuisine/game_server.py
+++ b/cooperative_cuisine/game_server.py
@@ -43,11 +43,15 @@ from cooperative_cuisine.utils import (
 )
 
 log = logging.getLogger(__name__)
+"""The logger for this module."""
 
 
 app = FastAPI()
+"""The FastAPI app that runs the game server."""
+
 
 TIME_AFTER_STOP_TO_DEL_ENV = 30
+"""Time after stopping an environment how long it takes to delete the env data from the game server. In seconds."""
 
 
 @dataclasses.dataclass
diff --git a/cooperative_cuisine/hooks.py b/cooperative_cuisine/hooks.py
index 95910360..06b7a53d 100644
--- a/cooperative_cuisine/hooks.py
+++ b/cooperative_cuisine/hooks.py
@@ -21,7 +21,6 @@ from typing import Callable, Any, TYPE_CHECKING, Type
 if TYPE_CHECKING:
     from cooperative_cuisine.environment import Environment
 
-# TODO add player_id as kwarg to all hooks -> pass player id to all methods
 
 ITEM_INFO_LOADED = "item_info_load"
 """Called after the item info is loaded and stored in the env attribute `item_info`. The kwargs are the passed config 
@@ -128,6 +127,30 @@ DROP_OFF_ON_COOKING_EQUIPMENT = "drop_off_on_cooking_equipment"
 
 
 class Hooks:
+    """
+    Class Hooks
+
+    Represents a collection of hooks and provides methods to register callbacks for hooks and invoke the callbacks when hooks are triggered.
+
+    Attributes:
+        hooks (defaultdict[list]): A defaultdict containing lists of callbacks for each hook reference.
+        env (any): The environment variable passed to the Hooks instance.
+
+    Methods:
+        __init__(self, env)
+            Initializes a new instance of Hooks.
+
+            Args:
+                env (any): The environment variable to be stored in the env attribute.
+
+        __call__(self, hook_ref, **kwargs)
+            Invokes the callbacks associated with the specified hook reference.
+
+            Args:
+                hook_ref (str): The hook reference to trigger the callbacks for.
+                **kwargs: Additional keyword arguments to be passed to the callbacks.
+    """
+
     def __init__(self, env):
         self.hooks = defaultdict(list)
         self.env = env
@@ -137,6 +160,12 @@ class Hooks:
             callback(hook_ref=hook_ref, env=self.env, **kwargs)
 
     def register_callback(self, hook_ref: str | list[str], callback: Callable):
+        """Register a callback for a hook which is called when the hook is touched during execution.
+
+        Args:
+            hook_ref: A string or a list of strings representing the reference(s) of the hook(s) to register the callback for.
+            callback: A callable object (function or method) to be registered as a callback.
+        """
         if isinstance(hook_ref, (tuple, list, set)):  # TODO check for iterable
             for ref in hook_ref:
                 self.hooks[ref].append(callback)
@@ -149,6 +178,42 @@ def print_hook_callback(text, env, **kwargs):
 
 
 class HookCallbackClass:
+    """
+    Class: HookCallbackClass
+
+    Represents a callback class for hook events.
+
+    Attributes:
+    - name: A string representing the name of the callback.
+    - env: An Environment object representing the environment in which the callback is being executed.
+
+    Methods:
+    - __init__(self, name: str, env: Environment, **kwargs):
+        Initializes a new instance of HookCallbackClass.
+
+    - __call__(self, hook_ref: str, env: Environment, **kwargs):
+        Abstract method that executes the callback logic when called.
+
+    Note:
+    - This class is meant to be subclassed and the __call__ method implemented according to specific needs.
+    - The **kwargs parameter allows for additional arguments to be passed to the callback function.
+
+    Usage Example:
+        # Create an instance of HookCallbackClass
+        callback = HookCallbackClass("my_callback", my_env)
+
+        # Subclass HookCallbackClass and implement the __call__ method
+        class MyCallback(HookCallbackClass):
+            def __call__(self, hook_ref: str, env: Environment, **kwargs):
+                # Add custom callback logic here
+
+        # Create an instance of the subclass
+        my_callback = MyCallback("my_callback", my_env)
+
+        # Call the callback
+        my_callback("hook_reference", my_env)
+    """
+
     def __init__(self, name: str, env: Environment, **kwargs):
         self.name = name
         self.env = env
@@ -165,6 +230,15 @@ def hooks_via_callback_class(
     callback_class: Type[HookCallbackClass],
     callback_class_kwargs: dict[str, Any],
 ):
+    """Function to reference in the `environment_config.yml` to add functionality via hooks and a configured callback class.
+
+    Args:
+        name: A string representing the name of the callback class instance.
+        env: An instance of the Environment class.
+        hooks: A list of strings representing the hooks for which the callback class instance needs to be registered.
+        callback_class: A type representing the class of the callback instance to be created.
+        callback_class_kwargs: A dictionary containing additional keyword arguments to be passed to the callback class constructor.
+    """
     recorder = callback_class(name=name, env=env, **callback_class_kwargs)
     for hook in hooks:
         env.register_callback_for_hook(hook, recorder)
diff --git a/cooperative_cuisine/info_msg.py b/cooperative_cuisine/info_msg.py
index d28ebfb5..62b98a84 100644
--- a/cooperative_cuisine/info_msg.py
+++ b/cooperative_cuisine/info_msg.py
@@ -28,6 +28,23 @@ from cooperative_cuisine.hooks import HookCallbackClass
 
 
 class InfoMsgManager(HookCallbackClass):
+    """
+    Class for managing info messages in an environment.
+
+    This class inherits from the `HookCallbackClass` class.
+
+    Attributes:
+        msg (str): The message to be displayed.
+        duration (datetime.timedelta): The duration for which the message should be displayed.
+        level (str): The level of the message.
+
+
+    Methods:
+        __init__(name, env, msg, duration, level, **kwargs): Initializes an instance of InfoMsgManager.
+        __call__(hook_ref, env, **kwargs): Adds the message to the info messages list for each player in the environment.
+        remove_old_msgs(env): Removes old messages from the environment.
+    """
+
     def __init__(
         self,
         name: str,
@@ -56,6 +73,12 @@ class InfoMsgManager(HookCallbackClass):
 
     @staticmethod
     def remove_old_msgs(env: Environment):
+        """
+        Removes old messages from the environment.
+
+        Args:
+            env (Environment): The environment object containing the messages.
+        """
         for player_id, msgs in env.info_msgs_per_player.items():
             delete_msgs = []
             for idx, msg in enumerate(msgs):
diff --git a/cooperative_cuisine/items.py b/cooperative_cuisine/items.py
index 3c6bd7e8..1de65506 100644
--- a/cooperative_cuisine/items.py
+++ b/cooperative_cuisine/items.py
@@ -116,7 +116,9 @@ class ItemInfo:
 
     def __post_init__(self):
         if self.seconds < 0.0:
-            raise ValueError(f"Expected seconds >= 0 for item '{self.name}', but got {self.seconds} in item info")
+            raise ValueError(
+                f"Expected seconds >= 0 for item '{self.name}', but got {self.seconds} in item info"
+            )
         self.type = ItemType(self.type)
         if self.effect_type:
             self.effect_type = EffectType(self.effect_type)
@@ -302,7 +304,7 @@ class CookingEquipment(Item):
             if transition.type == ItemType.Effect:
                 if set(ingredients.keys()).issubset(
                     transition.needs
-                ) and transition.manager.can_start_effect_transition(transition, self):
+                ) and not transition.manager.effect_is_active(transition, self):
                     if transition.seconds == 0:
                         transition.manager.register_active_effect(
                             Effect(name=transition.name, item_info=transition), self
diff --git a/cooperative_cuisine/movement.py b/cooperative_cuisine/movement.py
index 3a082ed5..fa554ad5 100644
--- a/cooperative_cuisine/movement.py
+++ b/cooperative_cuisine/movement.py
@@ -1,6 +1,13 @@
+"""
+Implements the moving of the players. Checks collisions and pushing other around.
+For efficiency, we tried to do everything with numpy arrays and functions. 
+"""
+
 from datetime import timedelta, datetime
+from typing import Tuple
 
 import numpy as np
+import numpy.typing as npt
 from scipy.spatial import distance_matrix
 
 from cooperative_cuisine.counters import Counter
@@ -8,18 +15,33 @@ from cooperative_cuisine.player import Player
 
 
 class Movement:
+    """Does the movement of the players."""
+
     world_borders_lower = None
+    """World borders lower bounds."""
     world_borders_upper = None
+    """World borders upper bounds."""
 
     def __init__(self, counter_positions, player_config, world_borders):
         self.counter_positions = counter_positions
+        """Positions of all counters in an environment. Needs to be updated if the counters position changes."""
         self.player_radius = 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 = player_config["interaction_range"]
+        """The range of how far a player can interact with the closest counter."""
         self.player_movement_speed = player_config["speed_units_per_seconds"]
+        """How many grid cells a player can move in a second."""
         self.world_borders = world_borders
+        """The world border arrays. For easier numpy comparison with player position. The outer dimension needs to be 
+        equal to the number of player."""
         self.set_collision_arrays(1)
 
-    def set_collision_arrays(self, number_players):
+    def set_collision_arrays(self, number_players: int):
+        """Sets collision arrays for the given number of players.
+
+        Args:
+            number_players (int): The number of players.
+        """
         self.world_borders_lower = self.world_borders[np.newaxis, :, 0].repeat(
             number_players, axis=0
         )
@@ -27,7 +49,18 @@ class Movement:
             number_players, axis=0
         )
 
-    def get_counter_collisions(self, player_positions):
+    def get_counter_collisions(
+        self, player_positions: npt.NDArray[float]
+    ) -> Tuple[npt.NDArray[bool], npt.NDArray[int], npt.NDArray[float]]:
+        """
+        Args:
+            player_positions: 2D numpy array containing the positions of the players.
+
+        Returns:
+            collided: 1D numpy array indicating whether each player has collided with a counter.
+            relevant_axes: 1D numpy array indicating the relevant axis for each player's collision.
+            nearest_counter_to_player: 2D numpy array indicating the vector from each player to the nearest counter.
+        """
         counter_diff_vecs = (
             player_positions[:, np.newaxis, :]
             - self.counter_positions[np.newaxis, :, :]
@@ -54,7 +87,19 @@ class Movement:
 
         return collided, relevant_axes, nearest_counter_to_player
 
-    def get_player_push(self, player_positions):
+    def get_player_push(
+        self, player_positions: npt.NDArray[float]
+    ) -> Tuple[npt.NDArray[bool], npt.NDArray[float]]:
+        """Calculates the collision and push vectors for each player.
+
+        Args:
+            player_positions (numpy.ndarray): An array of shape (n, 2) representing the positions of the players.
+
+        Returns:
+            tuple: A tuple containing two numpy arrays. The first array (collisions) is a boolean array of shape (n,) indicating
+            whether each player has collided with another player. The second array (push_vectors) is a numpy array of shape (2,)
+            representing the total push vector for each player.
+        """
         distances_players_after_scipy = distance_matrix(
             player_positions, player_positions
         )
@@ -77,8 +122,9 @@ class Movement:
         players: dict[str, Player],
         counters: list[Counter],
     ):
-        """Moves a player in the direction specified in the action.action. If the player collides with a
-        counter or other player through this movement, then they are not moved.
+        """Moves a player in the direction specified in the action.action.
+
+        If the player collides with a counter or other player through this movement, then they are not moved.
         (The extended code with the two ifs is for sliding movement at the counters, which feels a bit smoother.
         This happens, when the player moves diagonally against the counters or world boundary.
         This just checks if the single axis party of the movement could move the player and does so at a lower rate.)
diff --git a/cooperative_cuisine/orders.py b/cooperative_cuisine/orders.py
index 71c3d0fe..249d12ba 100644
--- a/cooperative_cuisine/orders.py
+++ b/cooperative_cuisine/orders.py
@@ -174,6 +174,11 @@ class OrderManager:
         """Reference to the hook manager."""
 
     def set_available_meals(self, available_meals):
+        """Set the available meals from which orders can be generated.
+
+        Args:
+            available_meals (dict): A dictionary containing the available meals and their quantities.
+        """
         self.available_meals = available_meals
         self.order_gen.available_meals = list(available_meals.values())
 
@@ -370,8 +375,11 @@ class RandomOrderGeneration(OrderGeneration):
     ):
         super().__init__(hook, random, **kwargs)
         self.kwargs: RandomOrderKwarg = RandomOrderKwarg(**kwargs["kwargs"])
+        """Configuration og the RandomOrder genration. See `RandomOrderKwarg`"""
         self.next_order_time: datetime | None = datetime.max
+        """For efficient checking to update order removable."""
         self.number_cur_orders: int = 0
+        """How many orders are currently open."""
         self.needed_orders: int = 0
         """For the sample on dur but when it was restricted due to max order number."""
 
@@ -387,6 +395,7 @@ class RandomOrderGeneration(OrderGeneration):
                 now,
                 self.kwargs.sample_on_serving,
             )
+        self.number_cur_orders = 0
         return []
 
     def get_orders(
diff --git a/cooperative_cuisine/pygame_2d_vis/gui.py b/cooperative_cuisine/pygame_2d_vis/gui.py
index 7d3597bd..42753e3f 100644
--- a/cooperative_cuisine/pygame_2d_vis/gui.py
+++ b/cooperative_cuisine/pygame_2d_vis/gui.py
@@ -43,6 +43,7 @@ class MenuStates(Enum):
 
 
 log = logging.getLogger(__name__)
+"""The logger for this module."""
 
 
 class PlayerKeySet:
diff --git a/cooperative_cuisine/recording.py b/cooperative_cuisine/recording.py
index 01f0de86..8ed83ce2 100644
--- a/cooperative_cuisine/recording.py
+++ b/cooperative_cuisine/recording.py
@@ -51,6 +51,7 @@ from cooperative_cuisine.hooks import HookCallbackClass
 from cooperative_cuisine.utils import NumpyAndDataclassEncoder, expand_path
 
 log = logging.getLogger(__name__)
+"""The logger for this module."""
 
 
 class FileRecorder(HookCallbackClass):
diff --git a/cooperative_cuisine/study_server.py b/cooperative_cuisine/study_server.py
index 8b28e45d..50d233f1 100644
--- a/cooperative_cuisine/study_server.py
+++ b/cooperative_cuisine/study_server.py
@@ -41,13 +41,14 @@ from cooperative_cuisine.utils import (
 NUMBER_PLAYER_PER_ENV = 2
 
 log = logging.getLogger(__name__)
-
-app = FastAPI()
+"""The logger for this module."""
 
 
-# HARDCODED_MANAGER_ID = "1234"
+app = FastAPI()
+"""The FastAPI app that runs the study server."""
 
 USE_AAAMBOS_AGENT = False
+"""Use the aaambos random agents instead of the simpler python script agents."""
 
 
 class LevelConfig(TypedDict):
@@ -89,7 +90,7 @@ class StudyState:
         self.next_level_env = None
         self.players_done = {}
 
-        self.use_aaambos_agent = False
+        self.use_aaambos_agent = USE_AAAMBOS_AGENT
 
         self.websocket_url = f"ws://{game_url}:{game_port}/ws/player/"
         print("WS:", self.websocket_url)
-- 
GitLab