From 88538365527eb7a716118bd9558739bf177ce995 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Florian=20Schr=C3=B6der?=
 <fschroeder@techfak.uni-bielefeld.de>
Date: Fri, 19 Jan 2024 22:10:47 +0100
Subject: [PATCH] fix review suggestions

---
 overcooked_simulator/__init__.py              |  28 +++--
 overcooked_simulator/counters.py              | 113 +++++++++++++-----
 .../game_content/environment_config.yaml      |   2 +-
 overcooked_simulator/game_items.py            |   2 +-
 .../gui_2d_vis/overcooked_gui.py              |   2 +-
 overcooked_simulator/order.py                 |   6 +-
 .../overcooked_environment.py                 |  11 +-
 tests/test_start.py                           |   4 +-
 8 files changed, 109 insertions(+), 59 deletions(-)

diff --git a/overcooked_simulator/__init__.py b/overcooked_simulator/__init__.py
index c2e43128..bcaf4521 100644
--- a/overcooked_simulator/__init__.py
+++ b/overcooked_simulator/__init__.py
@@ -4,17 +4,19 @@ This is the documentation of the Overcooked Simulator.
 
 # About the package
 
-The package contains of an environment for cooperation between players/agents.
-A PyGameGUI visualizes the game to human or visual agents in 2D.
-A 3D web-enabled version (for example for online studies, currently under development) can be found [here](https://gitlab.ub.uni-bielefeld.de/scs/cocosy/godot-overcooked-3d-visualization)
-
-# Background / Literature
-The overcooked/cooking domain is a well established cooperation domain/task.
-There exists environments designed for reinforcement learning agents as well as the game and adaptations of the game for human players in a more "real-time" environment.
-They all mostly differ in the visual and graphics dimension. 2D versions like overcooked-ai, ... are most known in the community.
-But more visual appealing 3D versions for cooperation with humans are getting developed more frequently (cite,...).
-Besides, the general adaptations of the original overcooked game.
-
+The package contains of an environment for cooperation between players/agents. A PyGameGUI visualizes the game to
+human or visual agents in 2D. A 3D web-enabled version (for example for online studies, currently under development)
+can be found [here](https://gitlab.ub.uni-bielefeld.de/scs/cocosy/godot-overcooked-3d-visualization)
+
+# Background / Literature The overcooked/cooking domain is a well established cooperation domain/task. There exists
+environments designed for reinforcement learning agents as well as the game and adaptations of the game for human
+players in a more "real-time" environment. They all mostly differ in the visual and graphics dimension. 2D versions
+like overcooked-ai, ... are most known in the community. But more visual appealing 3D versions for cooperation with
+humans are getting developed more frequently (cite,...). Besides, the general adaptations of the original overcooked
+game.
+With this overcooked-simulator, we want to bring both worlds together: the reinforcement learning and real-time playable
+environment with an appealing visualisation. Enable the potential of developing artificial agents that play with humans
+like a "real" cooperative / human partner.
 
 # Usage / Examples
 Our overcooked simulator is designed for real time interaction but also with reinforcement learning in mind (gymnasium environment).
@@ -45,8 +47,8 @@ On the left you can find the navigation panel that brings you to the implementat
 - in **main**, you find an example how to start a simulation,
 - the **orders**, how to sample incoming orders and their attributes,
 - the **environment**, handles the incoming actions and provides the state,
-- the **player**,
-- a **simulation runner**, that calls the step function of the environment for a real-time interaction,
+- the **player**/agent, that interacts in the environment,
+- a **simulation runner**, that calls the step function of the environment for a real-time interaction, and
 - **util**ity code.
 
 
diff --git a/overcooked_simulator/counters.py b/overcooked_simulator/counters.py
index 6e844d99..d8d5afed 100644
--- a/overcooked_simulator/counters.py
+++ b/overcooked_simulator/counters.py
@@ -23,7 +23,7 @@ import dataclasses
 import logging
 from collections import deque
 from datetime import datetime, timedelta
-from typing import TYPE_CHECKING, Optional, Callable
+from typing import TYPE_CHECKING, Optional, Callable, TypedDict
 
 if TYPE_CHECKING:
     from overcooked_simulator.overcooked_environment import (
@@ -44,10 +44,39 @@ from overcooked_simulator.game_items import (
 log = logging.getLogger(__name__)
 
 
+class TransitionsValueDict(TypedDict):
+    """The dicts that are the values in the transitions dicts of the `CookingEquipment`."""
+
+    seconds: int | float
+    """The needed seconds to progress for the transition."""
+    needs: list[str]
+    """The names of the needed items for the transition."""
+    info: ItemInfo | str
+    """The ItemInfo of the resulting item."""
+
+
+class TransitionsValueByNameDict(TypedDict):
+    """The dicts that are the values in the transitions dicts of the `CuttingBoard` and the `Sink`."""
+
+    seconds: int | float
+    """The needed seconds to progress for the transition."""
+    result: str
+    """The new name of the item after the transition."""
+
+
 class Counter:
-    """Simple class for a counter at a specified position (center of counter). Can hold things on top."""
+    """Simple class for a counter at a specified position (center of counter). Can hold things on top.
+
+    The character `#` in the `layout` file represents the standard Counter.
+    """
 
     def __init__(self, pos: npt.NDArray[float], occupied_by: Optional[Item] = None):
+        """Constructor setting the arguments as attributes.
+
+        Args:
+            pos: Position of the counter in the environment. 2-element vector.
+            occupied_by: The item on top of the counter.
+        """
         self.pos: npt.NDArray[float] = pos
         self.occupied_by: Optional[Item] = occupied_by
 
@@ -55,12 +84,15 @@ class Counter:
     def occupied(self):
         return self.occupied_by is not None
 
-    def pick_up(self, on_hands: bool = True):
+    def pick_up(self, on_hands: bool = True) -> Item | None:
         """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.
 
         Returns: The item which the counter is occupied by. None if nothing is there.
 
+        Args:
+            on_hands: Will the item be put on empty hands or on a cooking equipment.
+
         """
         if on_hands:
             if self.occupied_by:
@@ -92,8 +124,8 @@ class Counter:
         Args:
             item: The item to be placed on the counter.
 
-        Returns: TODO Return information, whether the score is affected (Serving Window?)
-
+        Returns:
+            Item or None what should be put back on the players hand, e.g., the cooking equipment.
         """
         if self.occupied_by is None:
             self.occupied_by = item
@@ -125,14 +157,15 @@ class CuttingBoard(Counter):
       seconds: 4.0
       equipment: CuttingBoard
     ```
-
-
+    The character `C` in the `layout` file represents the CuttingBoard.
     """
 
-    def __init__(self, pos: np.ndarray, transitions: dict):
+    def __init__(
+        self, pos: np.ndarray, transitions: dict[str, TransitionsValueByNameDict]
+    ):
         self.progressing = False
         self.transitions = transitions
-        super().__init__(pos)
+        super().__init__(pos=pos)
 
     def progress(self, passed_time: timedelta, now: datetime):
         """Called by environment step function for time progression.
@@ -187,11 +220,13 @@ class ServingWindow(Counter):
     ordered meals can also be served, if a `serving_not_ordered_meals` function is set in the `environment_config.yml`.
 
     The plate dispenser will put after some time a dirty plate on itself after a meal was served.
+
+    The character `W` in the `layout` file represents the ServingWindow.
     """
 
     def __init__(
         self,
-        pos,
+        pos: npt.NDArray[float],
         order_and_score: OrderAndScoreManager,
         meals: set[str],
         env_time_func: Callable[[], datetime],
@@ -201,7 +236,7 @@ class ServingWindow(Counter):
         self.plate_dispenser = plate_dispenser
         self.meals = meals
         self.env_time_func = env_time_func
-        super().__init__(pos)
+        super().__init__(pos=pos)
 
     def drop_off(self, item) -> Item | None:
         env_time = self.env_time_func()
@@ -217,7 +252,7 @@ class ServingWindow(Counter):
             or (len(item.content_list) == 1 and item.content_list[0].name in self.meals)
         )
 
-    def pick_up(self, on_hands: bool = True):
+    def pick_up(self, on_hands: bool = True) -> Item | None:
         pass
 
     def add_plate_dispenser(self, plate_dispenser):
@@ -243,14 +278,14 @@ class Dispenser(Counter):
     Which also is easier for the visualization of the dispenser.
     """
 
-    def __init__(self, pos, dispensing: ItemInfo):
+    def __init__(self, pos: npt.NDArray[float], dispensing: ItemInfo):
         self.dispensing = dispensing
         super().__init__(
-            pos,
-            self.create_item(),
+            pos=pos,
+            occupied_by=self.create_item(),
         )
 
-    def pick_up(self, on_hands: bool = True):
+    def pick_up(self, on_hands: bool = True) -> Item | None:
         return_this = self.occupied_by
         self.occupied_by = self.create_item()
         return return_this
@@ -265,7 +300,7 @@ class Dispenser(Counter):
     def __repr__(self):
         return f"{self.dispensing.name}Dispenser"
 
-    def create_item(self):
+    def create_item(self) -> Item:
         kwargs = {
             "name": self.dispensing.name,
             "item_info": self.dispensing,
@@ -282,7 +317,7 @@ class PlateConfig:
     dirty_plates: int = 3
     """dirty plates at the start."""
     plate_delay: list[int, int] = dataclasses.field(default_factory=lambda: [5, 10])
-    """The uniform sampling range for the plate delay between serving and return."""
+    """The uniform sampling range for the plate delay between serving and return in seconds."""
 
 
 class PlateDispenser(Counter):
@@ -302,18 +337,23 @@ class PlateDispenser(Counter):
     """
 
     def __init__(
-        self, pos, dispensing, plate_config: PlateConfig, plate_transitions, **kwargs
+        self,
+        pos: npt.NDArray[float],
+        dispensing: ItemInfo,
+        plate_config: PlateConfig,
+        plate_transitions: dict,
+        **kwargs,
     ) -> None:
-        super().__init__(pos, **kwargs)
+        super().__init__(pos=pos, **kwargs)
         self.dispensing = dispensing
         self.occupied_by = deque()
         self.out_of_kitchen_timer = []
         self.plate_config = plate_config
         self.next_plate_time = datetime.max
-        self.plate_transitions = plate_transitions
+        self.plate_transitions: dict[str, TransitionsValueDict] = plate_transitions
         self.setup_plates()
 
-    def pick_up(self, on_hands: bool = True):
+    def pick_up(self, on_hands: bool = True) -> Item | None:
         if self.occupied_by:
             return self.occupied_by.pop()
 
@@ -382,7 +422,7 @@ class PlateDispenser(Counter):
     def __repr__(self):
         return "PlateReturn"
 
-    def create_item(self, clean: bool = False):
+    def create_item(self, clean: bool = False) -> Plate:
         kwargs = {
             "clean": clean,
             "transitions": self.plate_transitions,
@@ -397,7 +437,7 @@ class Trashcan(Counter):
     The character `X` in the `layout` file represents the Trashcan.
     """
 
-    def pick_up(self, on_hands: bool = True):
+    def pick_up(self, on_hands: bool = True) -> Item | None:
         pass
 
     def drop_off(self, item: Item) -> Item | None:
@@ -438,7 +478,7 @@ class Stove(Counter):
 
 
 class Sink(Counter):
-    """The counter in which the dirty plates are in.
+    """The counter in which the dirty plates can be washed to clean plates.
 
     Needs a `SinkAddon`. The closest is calculated during initialisation, should not be seperated by each other (needs
     to touch the sink).
@@ -450,14 +490,20 @@ class Sink(Counter):
     The character `S` in the `layout` file represents the Sink.
     """
 
-    def __init__(self, pos, transitions, sink_addon=None):
-        super().__init__(pos)
+    def __init__(
+        self,
+        pos: npt.NDArray[float],
+        transitions: dict[str, TransitionsValueByNameDict],
+        sink_addon: SinkAddon = None,
+    ):
+        super().__init__(pos=pos)
         self.progressing = False
         self.sink_addon: SinkAddon = sink_addon
         """The connected sink addon which will receive the clean plates"""
         self.occupied_by = deque()
         """The queue of dirty plates. Only the one on the top is progressed."""
         self.transitions = transitions
+        """The allowed transitions for the items in the sink. Here only clean plates transfer from dirty plates."""
 
     @property
     def occupied(self):
@@ -510,10 +556,10 @@ class Sink(Counter):
         self.occupied_by.appendleft(item)
         return None
 
-    def pick_up(self, on_hands: bool = True):
-        return
+    def pick_up(self, on_hands: bool = True) -> Item | None:
+        return None
 
-    def set_addon(self, sink_addon):
+    def set_addon(self, sink_addon: SinkAddon):
         self.sink_addon = sink_addon
 
 
@@ -525,8 +571,9 @@ class SinkAddon(Counter):
     The character `+` in the `layout` file represents the SinkAddon.
     """
 
-    def __init__(self, pos, occupied_by=None):
-        super().__init__(pos)
+    def __init__(self, pos: npt.NDArray[float], occupied_by=None):
+        super().__init__(pos=pos)
+        # maybe check if occupied by is already a list or deque?
         self.occupied_by = deque([occupied_by]) if occupied_by else deque()
 
     def can_drop_off(self, item: Item) -> bool:
@@ -546,6 +593,6 @@ class SinkAddon(Counter):
     def add_clean_plate(self, plate: Plate):
         self.occupied_by.appendleft(plate)
 
-    def pick_up(self, on_hands: bool = True):
+    def pick_up(self, on_hands: bool = True) -> Item | None:
         if self.occupied_by:
             return self.occupied_by.pop()
diff --git a/overcooked_simulator/game_content/environment_config.yaml b/overcooked_simulator/game_content/environment_config.yaml
index 8c29958e..2698e1ff 100644
--- a/overcooked_simulator/game_content/environment_config.yaml
+++ b/overcooked_simulator/game_content/environment_config.yaml
@@ -38,7 +38,7 @@ orders:
         a: 10
         b: 20
     sample_on_serving: false
-    # The sample time for a new incoming order is only generated after a meal was served.
+    # Sample the delay for the next order only after a meal was served.
     score_calc_gen_func: !!python/name:overcooked_simulator.order.simple_score_calc_gen_func ''
     score_calc_gen_kwargs:
       # the kwargs for the score_calc_gen_func
diff --git a/overcooked_simulator/game_items.py b/overcooked_simulator/game_items.py
index 52d196a0..5e9ecd43 100644
--- a/overcooked_simulator/game_items.py
+++ b/overcooked_simulator/game_items.py
@@ -67,7 +67,7 @@ class ItemInfo:
     """The name of the item, is set automatically by the "group" name of the item."""
     seconds: float = dataclasses.field(compare=False, default=0)
     """If progress is needed this argument defines how long it takes to complete the process in seconds."""
-    needs: list[ItemInfo] = dataclasses.field(compare=False, default_factory=list)
+    needs: list[str] = dataclasses.field(compare=False, default_factory=list)
     """The ingredients/items which are needed to create the item/start the progress."""
     equipment: ItemInfo | None = dataclasses.field(compare=False, default=None)
     """On which the item can be created. `null`, `~` (None) converts to Plate."""
diff --git a/overcooked_simulator/gui_2d_vis/overcooked_gui.py b/overcooked_simulator/gui_2d_vis/overcooked_gui.py
index 7c3f4d0c..ed86bfad 100644
--- a/overcooked_simulator/gui_2d_vis/overcooked_gui.py
+++ b/overcooked_simulator/gui_2d_vis/overcooked_gui.py
@@ -241,7 +241,7 @@ class PyGameGUI:
         """
         for key_set in self.player_key_sets:
             if event.key == key_set.pickup_key and event.type == pygame.KEYDOWN:
-                action = Action(key_set.name, ActionType.PICKUP, "pickup")
+                action = Action(key_set.name, ActionType.PUT, "pickup")
                 self.send_action(action)
 
             if event.key == key_set.interact_key:
diff --git a/overcooked_simulator/order.py b/overcooked_simulator/order.py
index ab3f1fdc..9cb7d86a 100644
--- a/overcooked_simulator/order.py
+++ b/overcooked_simulator/order.py
@@ -109,7 +109,7 @@ class OrderGeneration:
 
 
 class ScoreCalcFuncType(Protocol):
-    """Type with kwargs of the expected `Order.score_calc` function and returned function for the
+    """Typed kwargs of the expected `Order.score_calc` function. Which is also returned by the
     `RandomOrderKwarg.score_calc_gen_func`."""
 
     def __call__(self, relative_order_time: timedelta, order: Order) -> float:
@@ -117,7 +117,7 @@ class ScoreCalcFuncType(Protocol):
 
 
 class ScoreCalcGenFuncType(Protocol):
-    """Type with kwargs of the expected function for the `RandomOrderKwarg.score_calc_gen_func`."""
+    """Typed kwargs of the expected function for the `RandomOrderKwarg.score_calc_gen_func`."""
 
     def __call__(
         self,
@@ -223,7 +223,7 @@ class RandomOrderGeneration(OrderGeneration):
                 a: 10
                 b: 20
             sample_on_serving: false
-            # The sample time for a new incoming order is only generated after a meal was served.
+            # Sample the delay for the next order only after a meal was served.
             score_calc_gen_func: !!python/name:overcooked_simulator.order.simple_score_calc_gen_func ''
             score_calc_gen_kwargs:
               # the kwargs for the score_calc_gen_func
diff --git a/overcooked_simulator/overcooked_environment.py b/overcooked_simulator/overcooked_environment.py
index 82accaa2..ed4a204f 100644
--- a/overcooked_simulator/overcooked_environment.py
+++ b/overcooked_simulator/overcooked_environment.py
@@ -43,8 +43,9 @@ class ActionType(Enum):
 
     MOVEMENT = "movement"
     """move the agent."""
-    PICKUP = "pickup"
-    """interaction type 1, e.g., for pickup or drop off."""
+    PUT = "pickup"
+    """interaction type 1, e.g., for pickup or drop off. Maybe other words: transplace?"""
+    # TODO change value to put
     INTERACT = "interact"
     """interaction type 2, e.g., for progressing. Start and stop interaction via `keydown` and `keyup` actions."""
 
@@ -99,7 +100,7 @@ class Environment:
         """The path to the `item_info.yml`. A default file is in `ROOT_DIR / "game_content" / "item_info.yaml"`."""
         self.item_info: dict[str, ItemInfo] = self.load_item_info()
         """The loaded item info dict. Keys are the item names."""
-        self.validate_item_info()
+        # self.validate_item_info()
 
         if self.environment_config["meals"]["all"]:
             self.allowed_meal_names = set(
@@ -263,7 +264,7 @@ class Environment:
 
     def validate_item_info(self):
         """TODO"""
-        pass
+        raise NotImplementedError
         # infos = {t: [] for t in ItemType}
         # graph = nx.DiGraph()
         # for info in self.item_info.values():
@@ -382,7 +383,7 @@ class Environment:
         else:
             counter = self.get_facing_counter(player)
             if player.can_reach(counter):
-                if action.action_type == ActionType.PICKUP:
+                if action.action_type == ActionType.PUT:
                     with self.lock:
                         player.pick_action(counter)
 
diff --git a/tests/test_start.py b/tests/test_start.py
index afc0dfca..13ac792c 100644
--- a/tests/test_start.py
+++ b/tests/test_start.py
@@ -184,7 +184,7 @@ def test_pickup():
 
     move_down = Action("p1", ActionType.MOVEMENT, np.array([0, -1]))
     move_up = Action("p1", ActionType.MOVEMENT, np.array([0, 1]))
-    pick = Action("p1", ActionType.PICKUP, "pickup")
+    pick = Action("p1", ActionType.PUT, "pickup")
 
     sim.enter_action(move_down)
     assert player.can_reach(counter), "Player can reach counter?"
@@ -238,7 +238,7 @@ def test_processing():
         player.holding = tomato
 
         move = Action("p1", ActionType.MOVEMENT, np.array([0, -1]))
-        pick = Action("p1", ActionType.PICKUP, "pickup")
+        pick = Action("p1", ActionType.PUT, "pickup")
 
         sim.enter_action(move)
         sim.enter_action(pick)
-- 
GitLab