From 0ddb9e04aa81a1283faeda6f4054a72a1d6c05c2 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:55:34 +0100
Subject: [PATCH] Add and optimize type hints, refactor functions, improve
readability
Various changes have been made in the codebase to improve readability and maintainability. Type hints have been added to the functions and variables for better understanding of the datatypes. Refactoring of some code segments has also been done to make the code more concise and readable. The 'EffectType', 'ItemType' enumerations have been enriched with docstrings for better understanding of the enums. Certain classes have been updated to better follow the single responsibility principle.
---
.../configs/study/level1/level1_config.yaml | 2 +-
cooperative_cuisine/counter_factory.py | 8 +---
cooperative_cuisine/counters.py | 8 ++--
cooperative_cuisine/environment.py | 2 +-
cooperative_cuisine/game_server.py | 3 +-
cooperative_cuisine/hooks.py | 30 +++++++-------
cooperative_cuisine/items.py | 6 +++
cooperative_cuisine/movement.py | 10 +++--
cooperative_cuisine/orders.py | 39 ++++++++++++++-----
cooperative_cuisine/player.py | 13 +++----
cooperative_cuisine/pygame_2d_vis/gui.py | 2 +
cooperative_cuisine/recording.py | 36 +++++++++--------
.../reinforcement_learning/gym_env.py | 2 +
13 files changed, 93 insertions(+), 68 deletions(-)
diff --git a/cooperative_cuisine/configs/study/level1/level1_config.yaml b/cooperative_cuisine/configs/study/level1/level1_config.yaml
index 94641a0a..4c7188a0 100644
--- a/cooperative_cuisine/configs/study/level1/level1_config.yaml
+++ b/cooperative_cuisine/configs/study/level1/level1_config.yaml
@@ -3,7 +3,7 @@ plates:
dirty_plates: 0
plate_delay: [ 5, 10 ]
# range of seconds until the dirty plate arrives.
- return_dirty: False
+ return_dirty: true
game:
time_limit_seconds: 300
diff --git a/cooperative_cuisine/counter_factory.py b/cooperative_cuisine/counter_factory.py
index ba8ce4b9..5232a5bf 100644
--- a/cooperative_cuisine/counter_factory.py
+++ b/cooperative_cuisine/counter_factory.py
@@ -367,12 +367,9 @@ class CounterFactory:
), f"No SinkAddon connected to Sink at pos {pos}"
counter.set_addon(closest_addon)
- def setup_effect_manger(self, counters: list[Counter]) -> dict[str, EffectManager]:
+ def setup_effect_manger(self) -> dict[str, EffectManager]:
"""Setup effect manager for effects in the item info.
- Args:
- counters (list[Counter]): A list of Counter objects.
-
Returns:
dict[str, EffectManager]: A dictionary where the keys are manager names and the values are EffectManager objects.
@@ -457,9 +454,8 @@ class CounterFactory:
max_width = 0
- lines = list(filter(lambda l: l != "", lines))
for line in lines:
- line = line.replace(" ", "")
+ line = line.strip()
if not line or line.startswith(";"):
break
current_x: float = starting_at
diff --git a/cooperative_cuisine/counters.py b/cooperative_cuisine/counters.py
index 7220f1ba..01421b21 100644
--- a/cooperative_cuisine/counters.py
+++ b/cooperative_cuisine/counters.py
@@ -321,9 +321,9 @@ class Counter:
"occupied_by": None
if self.occupied_by is None
else (
- [o.to_dict() for o in self.occupied_by]
- if isinstance(self.occupied_by, Iterable)
- else self.occupied_by.to_dict()
+ self.occupied_by.to_dict()
+ if isinstance(self.occupied_by, Item)
+ else [o.to_dict() for o in self.occupied_by]
),
"active_effects": [e.to_dict() for e in self.active_effects],
}
@@ -710,7 +710,7 @@ class PlateDispenser(Counter):
"clean": clean,
"transitions": self.plate_transitions,
"item_info": self.dispensing,
- "hook": self.hook
+ "hook": self.hook,
}
return Plate(**kwargs)
diff --git a/cooperative_cuisine/environment.py b/cooperative_cuisine/environment.py
index 2293a840..e898be18 100644
--- a/cooperative_cuisine/environment.py
+++ b/cooperative_cuisine/environment.py
@@ -268,7 +268,7 @@ class Environment:
self.effect_manager: dict[
str, EffectManager
- ] = self.counter_factory.setup_effect_manger(self.counters)
+ ] = self.counter_factory.setup_effect_manger()
"""Dict of effect managers. Currently only the fire effect manager."""
self.overwrite_counters(self.counters)
diff --git a/cooperative_cuisine/game_server.py b/cooperative_cuisine/game_server.py
index f9b2fbdb..28a4d6f6 100644
--- a/cooperative_cuisine/game_server.py
+++ b/cooperative_cuisine/game_server.py
@@ -416,6 +416,7 @@ class EnvironmentHandler:
Returns:
bool: True if all players are ready, False otherwise.
"""
+ self: EnvironmentHandler # Pycharm bug?
return env_id in self.envs and all(
self.player_data[player_hash].connected
and self.player_data[player_hash].ready
@@ -829,7 +830,7 @@ if __name__ == "__main__":
prog="Cooperative Cuisine Game Server",
description="Game Engine Server: Starts overcooked game engine server.",
epilog="For further information, see "
- "https://scs.pages.ub.uni-bielefeld.de/cocosy/overcooked-simulator/cooperative_cuisine.html",
+ "https://scs.pages.ub.uni-bielefeld.de/cocosy/overcooked-simulator/cooperative_cuisine.html",
)
url_and_port_arguments(parser)
diff --git a/cooperative_cuisine/hooks.py b/cooperative_cuisine/hooks.py
index 21519293..8c9cb70b 100644
--- a/cooperative_cuisine/hooks.py
+++ b/cooperative_cuisine/hooks.py
@@ -124,31 +124,21 @@ CONTENT_READY = "content_ready"
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.
+ """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)
+ __init__(self, env: Environment)
Initializes a new instance of Hooks.
- Args:
- env (any): The environment variable to be stored in the env attribute.
-
- __call__(self, hook_ref, **kwargs)
+ __call__(self, hook_ref: str, **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):
+ def __init__(self, env: Environment):
self.hooks = defaultdict(list)
self.env = env
@@ -171,6 +161,7 @@ class Hooks:
def print_hook_callback(text, env, **kwargs):
+ """Dummy hook callback. Print env time and hook ref."""
print(env.env_time, text)
@@ -214,9 +205,16 @@ class HookCallbackClass:
"""
def __init__(self, name: str, env: Environment, **kwargs):
- self.name = name
+ """Constructor of HookCallbackClass.
+
+ Args:
+ name: A string representing the name of the callback.
+ env: An instance of the Environment class representing the reference to the environment.
+ **kwargs: Additional keyword arguments.
+ """
+ self.name: str = name
"""The name of the callback."""
- self.env = env
+ self.env: Environment = env
"""Reference to the environment."""
@abstractmethod
diff --git a/cooperative_cuisine/items.py b/cooperative_cuisine/items.py
index 0679d347..97167ec9 100644
--- a/cooperative_cuisine/items.py
+++ b/cooperative_cuisine/items.py
@@ -57,10 +57,15 @@ COOKING_EQUIPMENT_ITEM_CATEGORY = "ItemCookingEquipment"
class EffectType(Enum):
+ """Enumeration of what types of effects `Effect`s can have on counters and items."""
+
Unusable = "Unusable"
+ """Fire effect makes counters and items unusable."""
class ItemType(Enum):
+ """Enumeration of which types of items exists in *CooperativeCuisine*"""
+
Ingredient = "Ingredient"
"""All ingredients and process ingredients."""
Meal = "Meal"
@@ -263,6 +268,7 @@ class CookingEquipment(Item):
"""The items that the equipment holds."""
self.hook: Hooks = hook
+ """Reference to the hook manager."""
log.debug(f"Initialize {self.name}: {self.transitions}")
diff --git a/cooperative_cuisine/movement.py b/cooperative_cuisine/movement.py
index d8431fae..b0c9d50a 100644
--- a/cooperative_cuisine/movement.py
+++ b/cooperative_cuisine/movement.py
@@ -18,9 +18,9 @@ from cooperative_cuisine.player import Player, PlayerConfig
class Movement:
"""Does the movement of the players."""
- world_borders_lower = None
+ world_borders_lower: npt.NDArray[float] = None
"""World borders lower bounds."""
- world_borders_upper = None
+ world_borders_upper: npt.NDArray[float] = None
"""World borders upper bounds."""
def __init__(
@@ -164,8 +164,10 @@ class Movement:
"""
d_time = duration.total_seconds()
- player_positions = np.array([p.pos for p in players.values()], dtype=float)
- player_movement_vectors = np.array(
+ player_positions: npt.NDArray[float] = np.array(
+ [p.pos for p in players.values()], dtype=float
+ )
+ player_movement_vectors: npt.NDArray[float] = np.array(
[
p.current_movement if env_time <= p.movement_until else [0, 0]
for p in players.values()
diff --git a/cooperative_cuisine/orders.py b/cooperative_cuisine/orders.py
index d65305e1..58db96b6 100644
--- a/cooperative_cuisine/orders.py
+++ b/cooperative_cuisine/orders.py
@@ -119,7 +119,7 @@ class OrderGeneration:
"""
self.available_meals: list[ItemInfo] | None = None
"""Available meals restricted through the `environment_config.yml`."""
- self.hook = hook
+ self.hook: Hooks = hook
"""Reference to the hook manager."""
self.random = random
"""Random instance."""
@@ -184,7 +184,7 @@ class OrderManager:
self.last_expired: list[Order] = []
"""Cache last expired orders for `OrderGeneration.get_orders` call."""
- self.hook = hook
+ self.hook: Hooks = hook
"""Reference to the hook manager."""
def set_available_meals(self, available_meals):
@@ -322,7 +322,7 @@ class RandomFuncConfig(TypedDict):
```
"""
- func: Callable
+ func: str | Callable
"""the name of a functions in the `random` library."""
kwargs: dict
"""the kwargs of the functions in the `random` library."""
@@ -460,16 +460,30 @@ class RandomOrderGeneration(OrderGeneration):
def create_orders_for_meals(
self, meals: list[ItemInfo], now: datetime, no_time_limit: bool = False
) -> list[Order]:
+ """Create order objects for given meals by sampling the duration of the orders.
+
+ Args:
+ meals: A list of ItemInfo objects representing the meals for which orders need to be created.
+ now: A datetime object representing the current env time (the start time of the orders).
+ no_time_limit: A boolean indicating whether there should be no limit on the order duration. Defaults to False.
+
+ Returns:
+ A list of Order objects representing the created orders.
+ """
orders = []
for meal in meals:
if no_time_limit:
duration = datetime.max - now
else:
- duration = timedelta(
- seconds=getattr(
+ if isinstance(self.kwargs.order_duration_random_func["func"], str):
+ seconds = getattr(
self.random, self.kwargs.order_duration_random_func["func"]
)(**self.kwargs.order_duration_random_func["kwargs"])
- )
+ else:
+ seconds = self.kwargs.order_duration_random_func["func"](
+ **self.kwargs.order_duration_random_func["kwargs"]
+ )
+ duration = timedelta(seconds=seconds)
self.hook(
ORDER_DURATION_SAMPLE,
duration=duration,
@@ -486,9 +500,14 @@ class RandomOrderGeneration(OrderGeneration):
return orders
def create_random_next_time_delta(self, now: datetime):
- self.next_order_time = now + timedelta(
- seconds=getattr(self.random, self.kwargs.sample_on_dur_random_func["func"])(
- **self.kwargs.sample_on_dur_random_func["kwargs"]
+ if isinstance(self.kwargs.order_duration_random_func["func"], str):
+ seconds = getattr(
+ self.random, self.kwargs.order_duration_random_func["func"]
+ )(**self.kwargs.order_duration_random_func["kwargs"])
+ else:
+ seconds = self.kwargs.order_duration_random_func["func"](
+ **self.kwargs.order_duration_random_func["kwargs"]
)
- )
+
+ self.next_order_time = now + timedelta(seconds=seconds)
log.info(f"Next order in {self.next_order_time}")
diff --git a/cooperative_cuisine/player.py b/cooperative_cuisine/player.py
index 82133a94..4da8e8b1 100644
--- a/cooperative_cuisine/player.py
+++ b/cooperative_cuisine/player.py
@@ -76,7 +76,7 @@ class Player:
self.holding: Optional[Item] = None
"""What item the player is holding."""
- self.player_config = player_config
+ self.player_config: PlayerConfig = player_config
"""See `PlayerConfig`."""
self.facing_direction: npt.NDArray[float] = np.array([0, 1])
@@ -101,6 +101,7 @@ class Player:
"""Is the player currently interacting with a counter."""
self.hook: Hooks = hook
+ """Hook manager. Register callbacks and create hook points with additional kwargs."""
def set_movement(self, move_vector, move_until):
"""Called by the `perform_action` method. Movements will be performed (pos will be updated) in the `step`
@@ -136,7 +137,7 @@ class Player:
self.facing_direction * self.player_config.radius * 0.5
)
- def can_reach(self, counter: Counter):
+ def can_reach(self, counter: Counter) -> bool:
"""Checks whether the player can reach the counter in question. Simple check if the distance is not larger
than the player interaction range.
@@ -190,12 +191,8 @@ class Player:
self.hook(PLAYER_START_INTERACT, player=self.name, counter=counter)
def perform_interact_stop(self):
- """Stops an interaction with the counter. Should be called for a
- keyup event, for letting go of a keyboard key.
-
- Args:
- counter: The counter to stop the interaction with.
- """
+ """Stops an interaction with the counter. Should be called for a keyup event, for letting go of a keyboard
+ key."""
self.interacting = False
self.hook(
PLAYER_END_INTERACT,
diff --git a/cooperative_cuisine/pygame_2d_vis/gui.py b/cooperative_cuisine/pygame_2d_vis/gui.py
index ccc28ce9..645d5a46 100644
--- a/cooperative_cuisine/pygame_2d_vis/gui.py
+++ b/cooperative_cuisine/pygame_2d_vis/gui.py
@@ -39,6 +39,8 @@ from cooperative_cuisine.utils import (
class MenuStates(Enum):
+ """Enumeration of "Page" types in the 2D pygame vis."""
+
Start = "Start"
ControllerTutorial = "ControllerTutorial"
PreGame = "PreGame"
diff --git a/cooperative_cuisine/recording.py b/cooperative_cuisine/recording.py
index 596060ad..b124a7f2 100644
--- a/cooperative_cuisine/recording.py
+++ b/cooperative_cuisine/recording.py
@@ -82,11 +82,11 @@ class FileRecorder(HookCallbackClass):
**kwargs: Additional keyword arguments.
"""
super().__init__(name, env, **kwargs)
- self.add_hook_ref = add_hook_ref
+ self.add_hook_ref: bool = add_hook_ref
"""Indicates whether to add a hook reference to the recorded data. Default value is False."""
log_path = log_path.replace("LOG_RECORD_NAME", name)
log_path = Path(expand_path(log_path, env_name=env.env_name))
- self.log_path = log_path
+ self.log_path: Path = log_path
"""The path to the log file. Default value is "USER_LOG_DIR/ENV_NAME/LOG_RECORD_NAME.jsonl"."""
log.info(f"Recorder record for {name} in file://{log_path}")
os.makedirs(log_path.parent, exist_ok=True)
@@ -151,30 +151,30 @@ def print_recorded_events_human_readable(jsonl_path: Path):
"""
- def stringify_item(item):
- if isinstance(item, float):
- return str(item)
- if isinstance(item, str):
- return item
- if isinstance(item, list) and len(item) == 1:
- item = item[0]
- if item:
+ def stringify_item(item_):
+ if isinstance(item_, float):
+ return str(item_)
+ if isinstance(item_, str):
+ return item_
+ if isinstance(item_, list) and len(item_) == 1:
+ item_ = item_[0]
+ if item_:
content = None
- if item and item["type"] in [
+ if item_ and item_["type"] in [
"Plate",
"Pot",
"Basket",
"Peel",
"Pan",
]:
- if item != "Plate" and item["content_ready"]:
- content_ready = stringify_item(item["content_ready"])
- return f"{item['type']}{f'({content_ready})'}"
+ if item_ != "Plate" and item_["content_ready"]:
+ content_ready = stringify_item(item_["content_ready"])
+ return f"{item_['type']}{f'({content_ready})'}"
- content = [stringify_item(i) for i in item["content_list"]]
+ content = [stringify_item(i) for i in item_["content_list"]]
if not content:
content = "None"
- return f"{item['type']}{f'({content})' if content else ''}"
+ return f"{item_['type']}{f'({content})' if content else ''}"
else:
return None
@@ -226,5 +226,7 @@ def print_recorded_events_human_readable(jsonl_path: Path):
if __name__ == "__main__":
- json_lines_path: Path = Path("/home/fabian/.local/state/cooperative_cuisine/log/fcb095915c454446b9ee2905ff534610/game_events.jsonl")
+ json_lines_path: Path = Path(
+ "/home/fabian/.local/state/cooperative_cuisine/log/fcb095915c454446b9ee2905ff534610/game_events.jsonl"
+ )
print_recorded_events_human_readable(json_lines_path)
diff --git a/cooperative_cuisine/reinforcement_learning/gym_env.py b/cooperative_cuisine/reinforcement_learning/gym_env.py
index 8361ee9b..4bb079e3 100644
--- a/cooperative_cuisine/reinforcement_learning/gym_env.py
+++ b/cooperative_cuisine/reinforcement_learning/gym_env.py
@@ -31,6 +31,8 @@ from cooperative_cuisine.pygame_2d_vis.drawing import Visualizer
class SimpleActionSpace(Enum):
+ """Enumeration of actions for simple action spaces for an RL agent."""
+
Up = "Up"
Down = "Down"
Left = "Left"
--
GitLab