Architecture review fixes: game rules off the event bus, save v4, game-over screens

config/settings_manager.py

@@ -13,7 +13,6 @@
     "reduce_motion": False,
     "hints": True,  # in-game inline affordances (→ take/cat/cd)
     "seen_selection_mode": False,  # combat selection-mode modal: show once ever
-    "difficulty": "medium",  # easy | medium | hard — scales enemy stats + XP
 }
 
 # Palette definitions: name → (primary, success, error, warning, accent)
@@ -119,17 +118,6 @@ def set_hints(self, enabled: bool) -> None:
         import config.dev_config as dev_cfg
         dev_cfg.SHOW_HINTS = enabled
 
-    def set_difficulty(self, mode: str) -> None:
-        """Set the difficulty mode (easy/medium/hard) and apply it live."""
-        from src import difficulty
-        self.settings["difficulty"] = mode if mode in difficulty.MODES else "medium"
-        difficulty.set_mode(self.settings["difficulty"])
-
-    def apply_all(self) -> None:
-        """Push loaded settings into runtime config. Call once after load()."""
-        self.set_hints(self.settings.get("hints", True))
-        self.set_difficulty(self.settings.get("difficulty", "medium"))
-
     def save(self) -> None:
         """Write current settings to JSON file."""
         try:

engine/api.py

@@ -14,7 +14,7 @@
 
 from typing import Any
 
-from src.events import EventBus, EventType
+from engine.events import EventBus, EventType
 from src.game_engine import ImprovedGameEngine
 from src.game_states import GameState
 
@@ -45,7 +45,7 @@ def submit(self, command: str) -> list[str]:
         self.ui.clear_console()
         self.bus.emit_event(
             EventType.COMMAND_ENTERED,
-            {"command": command, "game_state": self.state},
+            {"command": command},
             "GameSession",
         )
         return self.ui.drain()

src/events.py → engine/events.py

@@ -6,11 +6,12 @@
 without direct dependencies.
 """
 
-from typing import Dict, List, Callable, Any
-from dataclasses import dataclass
-from enum import Enum, auto
 import logging
 import time
+from collections.abc import Callable
+from dataclasses import dataclass
+from enum import Enum, auto
+from typing import Any
 
 logger = logging.getLogger(__name__)
 
@@ -32,9 +33,11 @@ class EventType(Enum):
     # Data: basic game start info
 
     GAME_OVER = auto()
-    # Emitted by: game_engine.py, game_flow.py
-    # Subscribed by: game_engine.py, textual_ui.py
-    # Data: {"message": str, "action": str (optional)}
+    # Emitted by: game_engine.py (death in combat, F5 restart)
+    # Subscribed by: textual_ui.py
+    # Data: {"reason": "defeat" | "restart", "message": str (optional)}
+    # The UI shows the GAME OVER card only for reason == "defeat". Nothing in the
+    # game listens to it: the game-over screen's choices call the engine directly.
 
     TUTORIAL_HINT = auto()
     # Emitted by: tutorial_coach.py (show_hint)
@@ -46,11 +49,6 @@ class EventType(Enum):
     # Subscribed by: textual_ui.py (finale), engine/headless/ui.py (text passthrough)
     # Data: {"ending_id": str, "sections": list[str], "stats": dict}
 
-    GAME_SAVED = auto()
-    # Emitted by: save.py, textual_ui.py
-    # Subscribed by: game_engine.py
-    # Data: {"trigger": str, "filename": str (optional)}
-
     GAME_RESTART_REQUESTED = auto()
     # Emitted by: textual_ui.py
     # Subscribed by: game_engine.py
@@ -83,12 +81,12 @@ class EventType(Enum):
     # Data: StatsView dict
 
     PLAYER_STATS_CHANGED = auto()
-    # Emitted by: game_engine.py, item_effects.py, commands/items.py
+    # Emitted by: game_engine.py (once after every command), combat.py (each turn)
     # Subscribed by: textual_ui.py
     # Data: StatsView dict
 
     PLAYER_INVENTORY_CHANGED = auto()
-    # Emitted by: game_engine.py, command_handler.py
+    # Emitted by: game_engine.py (once after every command)
     # Subscribed by: textual_ui.py
     # Data: InventoryView dict
 
@@ -99,12 +97,7 @@ class EventType(Enum):
     COMMAND_ENTERED = auto()
     # Emitted by: textual_ui.py
     # Subscribed by: game_engine.py
-    # Data: {"command": str, "game_state": GameState}
-
-    UI_ERROR = auto()
-    # Emitted by: textual_ui.py
-    # Subscribed by: game_engine.py
-    # Data: {"error": str}
+    # Data: {"command": str}. The engine routes on its own state, not the sender's.
 
     UI_READY = auto()
     # Emitted by: textual_ui.py
@@ -121,38 +114,25 @@ class EventType(Enum):
     # ========================================
 
     ROOM_ENTERED = auto()
-    # Emitted by: game_engine.py, command_handler.py
-    # Subscribed by: command_handler.py, textual_ui.py
+    # Emitted by: CommandHandler.announce_room (cd, flee, new game, load, ls -a
+    #             reveal, post-victory redraw)
+    # Subscribed by: textual_ui.py. A notification only: arrival rules run from
+    #             CommandHandler.arrive(), called directly.
     # Data: {"room": RoomView dict, "player_name": str}
 
-    ROOM_CHANGED = auto()
-    # Emitted by: command_handler.py
-    # Subscribed by: command_handler.py
-    # Data: {"player_name": str, "from_room": str, "to_room": str}
-
-    DELAYED_ROOM_REFRESH = auto()
-    # Emitted by: commands/items.py (cat, after a story-beat read) so the "✦ Memory
-    # restored / ✓ saved" message stays on screen before the room re-lists.
-    # Subscribed by: textual_ui.py (schedules an `ls` via set_timer). Headless ignores it.
-    # Data: {"room_id": str}
-
     ENEMY_DEFEATED = auto()
-    # Emitted by: combat.py
-    # Subscribed by: command_handler.py
-    # Data: {"enemy_id": str, "room": str, "player_name": str}
-
-    ALL_ENEMIES_DEFEATED = auto()
-    # Emitted by: game_world.py
-    # Subscribed by: command_handler.py
-    # Data: {"room": str}
+    # Emitted by: combat.py, before it calls CommandHandler.on_kill directly
+    # Subscribed by: textual_ui.py (observer; loot and removal run from on_kill)
+    # Data: {"enemy_id": str, "player_name": str}
 
     # ========================================
     # Combat Events
     # ========================================
 
     COMBAT_STARTED = auto()
     # Emitted by: combat.py
-    # Subscribed by: game_engine.py, textual_ui.py
+    # Subscribed by: textual_ui.py. The engine enters combat via the session's
+    #             on_start callback, not this event.
     # Data: CombatView dict (includes enemy info, player health, available attacks)
 
     COMBAT_ACTION_SELECTED = auto()
@@ -171,23 +151,30 @@ class EventType(Enum):
     # Data: CombatView dict — updated health values and cooldowns for current frame
 
     COMBAT_ENDED = auto()
-    # Emitted by: combat.py
-    # Subscribed by: command_handler.py, tutorial_coach.py, game_engine.py, textual_ui.py
-    # Data: {"victory": bool, "defeat": bool, "fled": bool, "enemy_id": str, "enemies_defeated": int}
+    # Emitted by: combat.py, before it calls CommandHandler.end_combat directly
+    # Subscribed by: textual_ui.py, tutorial_coach.py (observers only; the game's
+    #             reaction runs from end_combat in a fixed order)
+    # Data: {"victory": bool, "defeat": bool, "fled": bool, "enemy_id": str,
+    #        "enemies_defeated": int}
 
 @dataclass
 class Event:
     """Represents an event with data."""
     type: EventType
-    data: Dict[str, Any]
+    data: dict[str, Any]
     source: str = "unknown"
 
 class EventBus:
     """Central event bus for decoupled communication."""
-
-    def __init__(self) -> None:
-        self._listeners: Dict[EventType, List[Callable[[Event], None]]] = {}
-        self._event_history: List[Event] = []
+
+    #: Default for new buses. The test suite turns this on (tests/conftest.py)
+    #: so a listener that raises fails the test instead of only being logged.
+    strict_by_default: bool = False
+
+    def __init__(self, strict: bool | None = None) -> None:
+        self.strict = self.strict_by_default if strict is None else strict
+        self._listeners: dict[EventType, list[Callable[[Event], None]]] = {}
+        self._event_history: list[Event] = []
         self._max_history = 100
 
     def subscribe(self, event_type: EventType, callback: Callable[[Event], None]) -> None:
@@ -209,7 +196,10 @@ def unsubscribe(self, event_type: EventType, callback: Callable[[Event], None])
     def emit(self, event: Event) -> None:
         """Emit an event to all subscribers."""
         start_time = time.time()
-        logger.debug(f"Emitting event: {event.type} from {event.source} to {len(self._listeners.get(event.type, []))} listeners")
+        listener_count = len(self._listeners.get(event.type, []))
+        logger.debug(
+            f"Emitting event: {event.type} from {event.source} to {listener_count} listeners"
+        )
 
         # Add to history
         self._event_history.append(event)
@@ -237,6 +227,8 @@ def emit(self, event: Event) -> None:
                     logger.warning(f"Slow callback for {event.type}: {callback_time:.3f}s")
 
             except Exception as e:
+                if self.strict:
+                    raise
                 callback_errors += 1
                 logger.error(f"Error in event callback for {event.type}: {e}")
 
@@ -248,13 +240,15 @@ def emit(self, event: Event) -> None:
         if callback_errors:
             logger.warning(f"{event.type}: {callback_errors} callback error(s)")
 
-    def emit_event(self, event_type: EventType, data: Dict[str, Any] | None = None, source: str = "unknown") -> None:
+    def emit_event(
+        self, event_type: EventType, data: dict[str, Any] | None = None, source: str = "unknown",
+    ) -> None:
         """Convenience method to emit an event."""
         event = Event(type=event_type, data=data or {}, source=source)
         logger.debug(f"Emitting event: {event_type} from {source} with data: {data}")
         self.emit(event)
 
-    def get_event_history(self) -> List[Event]:
+    def get_event_history(self) -> list[Event]:
         """Get the event history."""
         return self._event_history.copy()
 

engine/headless/ui.py

@@ -41,7 +41,7 @@ def __init__(self) -> None:
         self.bus: Any = None
 
     def attach_bus(self, bus: Any, state_manager: Any) -> None:
-        from src.events import EventType
+        from engine.events import EventType
         self.shutdown()
         self.bus = bus
         self.state_manager = state_manager
@@ -96,29 +96,11 @@ def run(self) -> None:  # pragma: no cover - lifecycle no-op
     def shutdown(self) -> None:
         if self.bus is None:
             return
-        from src.events import EventType
+        from engine.events import EventType
         self.bus.unsubscribe(EventType.GAME_WON, self._on_game_won)
         self.bus.unsubscribe(EventType.GAME_QUIT, self._on_game_quit)
         self.bus.unsubscribe(EventType.TUTORIAL_HINT, self._on_tutorial_hint)
         self.bus = None
 
-    def update_inventory(self, content: str) -> None:
-        pass
-
-    def update_stats(self, content: str) -> None:
-        pass
-
-    def update_exits(self, exits: list[object]) -> None:
-        pass
-
-    def update_player_name(self, name: str) -> None:
-        pass
-
-    def display_game_over(self) -> None:
-        pass
-
-    def save_current_game(self) -> None:
-        pass
-
     def _display_title_screen(self) -> None:
         self.output_log.append("[title screen]")

src/viewmodels/view_models.py → engine/view_models.py

@@ -8,8 +8,8 @@
 
 from __future__ import annotations
 
-from dataclasses import dataclass, field, asdict
-from typing import Any, List, Optional
+from dataclasses import asdict, dataclass, field
+from typing import Any
 
 
 @dataclass(frozen=True)
@@ -44,8 +44,8 @@ class InventoryItemView:
     item_type: str
     rarity: str = "common"
     is_equipped: bool = False
-    damage: Optional[int] = None
-    healing: Optional[int] = None
+    damage: int | None = None
+    healing: int | None = None
 
     def to_dict(self) -> dict[str, Any]:
         """Convert to dictionary for event serialization."""
@@ -61,7 +61,7 @@ def from_dict(cls, data: dict[str, Any]) -> InventoryItemView:
 @dataclass(frozen=True)
 class InventoryView:
     """Full inventory representation."""
-    items: List[InventoryItemView] = field(default_factory=list)
+    items: list[InventoryItemView] = field(default_factory=list)
 
     def to_dict(self) -> dict[str, Any]:
         """Convert to dictionary for event serialization."""
@@ -83,11 +83,11 @@ class RoomView:
     description: str
     id: str = ""
     zone: str = ""
-    exits: List[str] = field(default_factory=list)
-    enemies: List[str] = field(default_factory=list)      # display names
-    npcs: List[str] = field(default_factory=list)         # display names
-    enemy_ids: List[str] = field(default_factory=list)    # same order as enemies
-    npc_ids: List[str] = field(default_factory=list)      # same order as npcs
+    exits: list[str] = field(default_factory=list)
+    enemies: list[str] = field(default_factory=list)      # display names
+    npcs: list[str] = field(default_factory=list)         # display names
+    enemy_ids: list[str] = field(default_factory=list)    # same order as enemies
+    npc_ids: list[str] = field(default_factory=list)      # same order as npcs
 
     def to_dict(self) -> dict[str, Any]:
         """Convert to dictionary for event serialization."""
@@ -131,8 +131,8 @@ class CombatView:
     player_health: int
     player_max_health: int
     enemy_id: str = ""    # for scene sprite lookup
-    available_attacks: List[AttackView] = field(default_factory=list)
-    usable_items: List[InventoryItemView] = field(default_factory=list)
+    available_attacks: list[AttackView] = field(default_factory=list)
+    usable_items: list[InventoryItemView] = field(default_factory=list)
 
     def to_dict(self) -> dict[str, Any]:
         """Convert to dictionary for event serialization."""

main.py

@@ -1,8 +1,13 @@
 #!/usr/bin/env python3
 import os
 import shutil
-from src.game_engine import main
-from src.ui.textual_ui import TextualGameUI  # composition root: the frontend is chosen HERE
+
+# Content, saves, logs and user settings are all addressed relative to the
+# repo, and some are opened at import time, so this runs before any game import.
+os.chdir(os.path.dirname(os.path.abspath(__file__)))
+
+from src.game_engine import main  # noqa: E402
+from src.ui.textual_ui import TextualGameUI  # noqa: E402  composition root: the frontend is chosen HERE
 
 if __name__ == "__main__":
     # Create necessary directories if they don't exist