From 97539d860628d0520178ee5e90fd169eb31bc631 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20Schr=C3=B6der?=
 <fschroeder@techfak.uni-bielefeld.de>
Date: Thu, 7 Mar 2024 11:02:47 +0100
Subject: [PATCH] Refactor code for clarity and type hinting

The code has been refactored to clarify the functionality of various components and improve type hinting. Documentation for methods has been expanded. The return types of to_dict methods in several classes have been changed to return CounterState. The Counter class now correctly handles cases where an Item or Iterable[Item] is occupied. An unnecessary Boolean attribute, all_players_ready, has been commented out. Manual normalization of orientation vectors has been introduced.
---
 cooperative_cuisine/action.py               | 16 ++++++
 cooperative_cuisine/counters.py             | 59 ++++++++++++++++-----
 cooperative_cuisine/effects.py              |  2 +-
 cooperative_cuisine/state_representation.py |  2 +-
 4 files changed, 63 insertions(+), 16 deletions(-)

diff --git a/cooperative_cuisine/action.py b/cooperative_cuisine/action.py
index 1bc0c19f..6d330c80 100644
--- a/cooperative_cuisine/action.py
+++ b/cooperative_cuisine/action.py
@@ -1,3 +1,19 @@
+"""
+The actions a player can perform.
+
+The `Action` class is used to represent the incoming action data. There are three types of actions:
+- `movement`: Move the player for a specified duration into a direction
+- `pick_up_drop`: Based on the situation, pick up or drop off an item from a counter to the players hand or the other way around.
+- `interact`: Interact with a counter to increase the progress, e.g., the CuttingBoard to chop ingredients or the Sink to clean plates.
+
+The `action_data` depends on the `action_type`:
+- `movement`: a 2d list/array in which direction to move. (Movement vector: complete 360° are allowed, e.g. [0.435889894, 0.9]).
+- `pick_up_drop`: None,
+- `interact`: `InterActionData` either "keydown" or "keyup" (start and stop the interaction).
+
+The duration part is only needed for the `movement` action. For real-time interactions/games: 1/fps.
+"""
+
 from __future__ import annotations
 
 import dataclasses
diff --git a/cooperative_cuisine/counters.py b/cooperative_cuisine/counters.py
index 2853e59e..d8a9c522 100644
--- a/cooperative_cuisine/counters.py
+++ b/cooperative_cuisine/counters.py
@@ -41,7 +41,7 @@ from collections import deque
 from collections.abc import Iterable
 from datetime import datetime, timedelta
 from random import Random
-from typing import TYPE_CHECKING, Optional, Callable, Set
+from typing import TYPE_CHECKING, Callable, Set
 
 from cooperative_cuisine.hooks import (
     Hooks,
@@ -63,6 +63,7 @@ from cooperative_cuisine.hooks import (
     PLATE_OUT_OF_KITCHEN_TIME,
     DROP_OFF_ON_COOKING_EQUIPMENT,
 )
+from cooperative_cuisine.state_representation import CounterState
 
 if TYPE_CHECKING:
     from cooperative_cuisine.environment import (
@@ -85,9 +86,6 @@ from cooperative_cuisine.items import (
 log = logging.getLogger(__name__)
 """The logger for this module."""
 
-COUNTER_CATEGORY = "Counter"
-"""The string for the `category` value in the json state representation for all counters."""
-
 
 class Counter:
     """Simple class for a counter at a specified position (center of counter). Can hold things on top.
@@ -99,7 +97,7 @@ class Counter:
         self,
         pos: npt.NDArray[float],
         hook: Hooks,
-        occupied_by: Optional[Item] = None,
+        occupied_by: Item | None = None,
         uid: hex = None,
         **kwargs,
     ):
@@ -113,11 +111,11 @@ class Counter:
         """A unique id for better tracking in GUIs with assets which instance moved or changed."""
         self.pos: npt.NDArray[float] = pos
         """The position of the counter."""
-        self.occupied_by: Optional[Item] = occupied_by
+        self.occupied_by: Item | Iterable[Item] | None = occupied_by
         """What is on top of the counter, e.g., `Item`s."""
         self.active_effects: list[Effect] = []
         """The effects that currently affect the usage of the counter."""
-        self.hook = hook
+        self.hook: Hooks = hook
         """Reference to the hook manager."""
         self.orientation: npt.NDArray[float] = np.array([0, 1], dtype=float)
         """In what direction the counter is facing."""
@@ -127,13 +125,25 @@ class Counter:
         """Is something on top of the counter."""
         return self.occupied_by is not None
 
-    def set_orientation(self, orientation: npt.NDArray[float]) -> None:
+    def set_orientation(self, orientation: npt.NDArray[float]):
+        """This method sets the orientation of an object to the specified numpy array.
+
+        The provided numpy array is normalized if its norm is not equal to 1, ensuring that it represents a valid
+        orientation. The resulting orientation is then assigned to the 'orientation' attribute of the object. If the
+        norm of the provided numpy array is already equal to 1, it is assigned directly to the 'orientation'
+        attribute without normalization.
+
+        Args:
+            orientation: A 2D numpy array representing the orientation of an object.
+        """
         if not np.isclose(np.linalg.norm(orientation), 1):
             self.orientation = orientation / np.linalg.norm(orientation)
         else:
             self.orientation = orientation
 
-    def pick_up(self, on_hands: bool = True, player: str = "0") -> Item | None:
+    def pick_up(
+        self, on_hands: bool = True, player: str = "0"
+    ) -> Item | None | Iterable[Item]:
         """Gets called upon a player performing the pickup action. If the counter can give something to
         the player, it does so. In the standard counter this is when an item is on the counter.
 
@@ -235,6 +245,22 @@ class Counter:
     def _do_single_tool_interaction(
         passed_time: timedelta, tool: Item, target: Item | Counter
     ) -> bool:
+        """This method is used to perform a single tool interaction on a target entity.
+
+        It calculates the progress of the interaction based on the amount of time passed and the properties of the
+        tool and target entity. If the tool has suitable effects for the target entity, the progress percentage is
+        updated and the method returns True. If the progress exceeds the maximum value, the effect is removed from
+        the target entity and the method returns True. Otherwise, the method returns False indicating that the tool
+        interaction was unsuccessful.
+
+        Args:
+            passed_time: A timedelta object representing the amount of time passed.
+            tool: An Item object representing the tool being used.
+            target: An Item or Counter object representing the target entity.
+
+        Returns:
+            A boolean value indicating whether the tool interaction was successful.
+        """
         suitable_effects = [
             e for e in target.active_effects if e.name in tool.item_info.needs
         ]
@@ -249,13 +275,19 @@ class Counter:
         return False
 
     def do_hand_free_interaction(self, passed_time: timedelta, now: datetime):
+        """Called by environment step function for time progression.
+
+        Args:
+            passed_time: the time passed since the last progress call
+            now: the current env time. **Not the same as `datetime.now`**.
+        """
         ...
 
-    def to_dict(self) -> dict:
+    def to_dict(self) -> CounterState:
         """For the state representation. Only the relevant attributes are put into the dict."""
         return {
             "id": self.uuid,
-            "category": COUNTER_CATEGORY,
+            "category": "Counter",
             "type": self.__class__.__name__,
             "pos": self.pos.tolist(),
             "orientation": self.orientation.tolist(),
@@ -468,7 +500,7 @@ class Dispenser(Counter):
         }
         return Item(**kwargs)
 
-    def to_dict(self) -> dict:
+    def to_dict(self) -> CounterState:
         d = super().to_dict()
         d.update((("type", self.__repr__()),))
         return d
@@ -694,7 +726,7 @@ class CookingCounter(Counter):
     def __repr__(self):
         return f"{self.name}(pos={self.pos},occupied_by={self.occupied_by})"
 
-    def to_dict(self) -> dict:
+    def to_dict(self) -> CounterState:
         d = super().to_dict()
         d.update((("type", self.name),))
         return d
@@ -741,7 +773,6 @@ class Sink(Counter):
         return len(self.occupied_by) != 0
 
     def do_hand_free_interaction(self, passed_time: timedelta, now: datetime):
-        """Called by environment step function for time progression"""
         if (
             self.occupied
             and self.occupied_by[-1].name in self.transition_needs
diff --git a/cooperative_cuisine/effects.py b/cooperative_cuisine/effects.py
index fe73ddf4..b9e04a3f 100644
--- a/cooperative_cuisine/effects.py
+++ b/cooperative_cuisine/effects.py
@@ -130,7 +130,7 @@ class FireEffectManager(EffectManager):
         """A boolean indicating whether the fire burns ingredients and meals."""
         self.effect_to_timer: dict[str:datetime] = {}
         """A dictionary mapping effect uuids to their corresponding timers."""
-        self.next_finished_timer = datetime.max
+        self.next_finished_timer: datetime = datetime.max
         """A datetime representing the time when the next effect will finish."""
         self.active_effects: list[Tuple[Effect, Item | Counter]] = []
         """A list of tuples representing the active effects and their target items or counters."""
diff --git a/cooperative_cuisine/state_representation.py b/cooperative_cuisine/state_representation.py
index 25c022fd..277be355 100644
--- a/cooperative_cuisine/state_representation.py
+++ b/cooperative_cuisine/state_representation.py
@@ -182,7 +182,7 @@ class StateRepresentation(BaseModel):
     info_msg: list[tuple[str, str]]
     """Info messages for the players to be displayed."""
     # is added:
-    all_players_ready: bool
+    # all_players_ready: bool
     """Added by the game server, indicate if all players are ready and actions are passed to the environment."""
 
 
-- 
GitLab