diff --git a/server/games/yahtzee/game.py b/server/games/yahtzee/game.py index 3f506b3f..4a416722 100644 --- a/server/games/yahtzee/game.py +++ b/server/games/yahtzee/game.py @@ -27,6 +27,7 @@ from ...game_utils.options import IntOption, option_field from ...messages.localization import Localization from server.core.ui.keybinds import KeybindState +from server.core.users.base import MenuItem # Scoring categories @@ -298,6 +299,15 @@ def create_turn_action_set(self, player: YahtzeePlayer) -> ActionSet: is_hidden="_is_view_scoresheet_hidden", ) ) + action_set.add( + Action( + id="view_all_scoresheets", + label=Localization.get(locale, "yahtzee-check-scoresheet"), + handler="_action_view_all_scoresheets", + is_enabled="_is_view_scoresheet_enabled", + is_hidden="_is_view_scoresheet_hidden", + ) + ) return action_set @@ -311,9 +321,9 @@ def setup_keybinds(self) -> None: # Toggle dice (1-5 keys) - from DiceGameMixin self.setup_dice_keybinds() - # View actions - self.define_keybind("d", "View dice", ["view_dice"], state=KeybindState.ACTIVE) - self.define_keybind("c", "View scoresheet", ["view_scoresheet"], state=KeybindState.ACTIVE) + self.define_keybind("d", "View dice", ["view_dice"], state=KeybindState.ACTIVE, include_spectators=True) + self.define_keybind("c", "View scoresheet", ["view_scoresheet"], state=KeybindState.ACTIVE, include_spectators=True) + self.define_keybind("shift+c", "View all scoresheets", ["view_all_scoresheets"], state=KeybindState.ACTIVE, include_spectators=True) # ========================================================================== # Declarative Action Callbacks @@ -542,32 +552,54 @@ def _action_view_dice(self, player: Player, action_id: str) -> None: user.speak_l("yahtzee-your-dice", dice=dice_str) def _action_view_scoresheet(self, player: Player, action_id: str) -> None: - """View scoresheet.""" + """View your own scoresheet.""" + if player.is_spectator: + return self._action_view_all_scoresheets(player, action_id) + self._show_player_scoresheet(player, player) + + def _action_view_all_scoresheets(self, player: Player, action_id: str) -> None: + """View scoresheet menu for all players.""" user = self.get_user(player) if not user: return - # Show current player's scoresheet - current = self.current_player - if not current: + active_players = [p for p in self.players if not p.is_spectator] + if not active_players: + user.speak_l("action-player-not-found") return - ytz_current: YahtzeePlayer = current # type: ignore + items = [MenuItem(text=p.name, id=p.id) for p in active_players] + back_label = Localization.get(user.locale, "back") + items.append(MenuItem(text=back_label, id="transient_display_back")) + self._show_transient_display( + player, + kind="scoresheet_player_select", + items=items, + multiletter=True, + ) + + def _show_player_scoresheet(self, viewer: Player, target: Player) -> None: + """Show a specific player's scoresheet to the viewer.""" + user = self.get_user(viewer) + if not user: + return + + ytz_target: YahtzeePlayer = target # type: ignore locale = user.locale - lines = [Localization.get(locale, "yahtzee-scoresheet-header", player=current.name)] + lines = [Localization.get(locale, "yahtzee-scoresheet-header", player=target.name)] lines.append(Localization.get(locale, "yahtzee-scoresheet-upper")) for cat in UPPER_CATEGORIES: cat_name = Localization.get(locale, CATEGORY_NAMES[cat]) - score = ytz_current.scores.get(cat) + score = ytz_target.scores.get(cat) if score is not None: lines.append(f" {cat_name}: {score}") else: lines.append(f" {cat_name}: -") - upper_total = ytz_current.get_upper_total() - if ytz_current.upper_bonus_awarded: + upper_total = ytz_target.get_upper_total() + if ytz_target.upper_bonus_awarded: lines.append( Localization.get(locale, "yahtzee-scoresheet-upper-total-bonus", total=upper_total) ) @@ -586,18 +618,18 @@ def _action_view_scoresheet(self, player: Player, action_id: str) -> None: for cat in LOWER_CATEGORIES: cat_name = Localization.get(locale, CATEGORY_NAMES[cat]) - score = ytz_current.scores.get(cat) + score = ytz_target.scores.get(cat) if score is not None: lines.append(f" {cat_name}: {score}") else: lines.append(f" {cat_name}: -") - yahtzee_bonus = ytz_current.yahtzee_bonus_count * 100 + yahtzee_bonus = ytz_target.yahtzee_bonus_count * 100 lines.append( Localization.get( locale, "yahtzee-scoresheet-yahtzee-bonus", - count=ytz_current.yahtzee_bonus_count, + count=ytz_target.yahtzee_bonus_count, total=yahtzee_bonus, ) ) @@ -606,11 +638,30 @@ def _action_view_scoresheet(self, player: Player, action_id: str) -> None: Localization.get( locale, "yahtzee-scoresheet-grand-total", - total=ytz_current.get_total_score(), + total=ytz_target.get_total_score(), ) ) - self.status_box(player, lines) + self.status_box(viewer, lines) + + def _handle_transient_display_selection(self, player: Player, selection_id: str) -> None: + """Handle a selection from the shared transient display menu.""" + state = self._get_transient_display_state(player) + if state and state.kind == "scoresheet_player_select": + if selection_id == "transient_display_back": + self._close_transient_display(player) + return + self._close_transient_display(player) + target = self.get_player_by_id(selection_id) + if target: + self._show_player_scoresheet(player, target) + else: + user = self.get_user(player) + if user: + user.speak_l("action-player-not-found") + return + + super()._handle_transient_display_selection(player, selection_id) def on_start(self) -> None: """Called when the game starts.""" diff --git a/server/locales/en/main.ftl b/server/locales/en/main.ftl index e82e4ea6..bf6e2a84 100644 --- a/server/locales/en/main.ftl +++ b/server/locales/en/main.ftl @@ -179,6 +179,7 @@ table-saved-destroying = Table saved! Returning to main menu. game-type-not-found = Game type no longer exists. # Action disabled reasons +action-player-not-found = Player not found. action-not-your-turn = It's not your turn. action-not-playing = The game hasn't started. action-spectator = Spectators cannot do this. diff --git a/server/tests/test_yahtzee_scoresheet_select.py b/server/tests/test_yahtzee_scoresheet_select.py new file mode 100644 index 00000000..f8a397c6 --- /dev/null +++ b/server/tests/test_yahtzee_scoresheet_select.py @@ -0,0 +1,240 @@ +"""Tests for the scoresheet picker flow added in PR #243. + +PR #243 added the ability to view any active player's Yahtzee scoresheet. + +Keybind layout (active players): +- ``c`` -> immediately show the viewer's own scoresheet (status_box). +- ``shift+c`` -> open a transient picker (scoresheet_player_select) listing all + active players + a Back item; selecting a player shows their scoresheet. + +For spectators (no own scoresheet), ``c`` falls back to the picker so the +experience is seamless. +""" + +from __future__ import annotations + +from server.core.users.base import MenuItem +from server.core.users.test_user import MockUser +from server.games.yahtzee.game import YahtzeeGame + + +def _last_show_menu_for(user: MockUser, menu_id: str) -> dict | None: + for m in reversed(user.messages): + if m.type == "show_menu" and m.data.get("menu_id") == menu_id: + return m.data + return None + + +def _item_id(item) -> str: + return item.id if isinstance(item, MenuItem) else item["id"] + + +def _item_text(item) -> str: + return item.text if isinstance(item, MenuItem) else item["text"] + + +def _spoken_texts(user: MockUser) -> list[str]: + return [m.data["text"] for m in user.messages if m.type == "speak"] + + +def _make_game(player_specs: list[tuple[str, str]], spectators: list[str] | None = None): + """Build a started YahtzeeGame. + + player_specs: list of (name, locale) for active players. + spectators: list of names for spectators. + Returns (game, players_by_name, users_by_name). + """ + game = YahtzeeGame() + users: dict[str, MockUser] = {} + players: dict = {} + for name, locale in player_specs: + user = MockUser(name, locale=locale) + users[name] = user + players[name] = game.add_player(name, user) + for name in spectators or []: + user = MockUser(name) + users[name] = user + players[name] = game.add_spectator(name, user) + game.on_start() + # Clear startup chatter so menu-finders don't trip on stale messages. + for u in users.values(): + u.clear_messages() + return game, players, users + + +# --------------------------------------------------------------------------- +# `c` (own sheet) path +# --------------------------------------------------------------------------- + + +def test_view_scoresheet_active_player_shows_own_sheet_immediately(): + """Active player pressing `c` should see their own scoresheet directly, + not the picker.""" + game, players, users = _make_game([("Alice", "en"), ("Bob", "en")]) + alice = players["Alice"] + + game._action_view_scoresheet(alice, "view_scoresheet") + + state = game._get_transient_display_state(alice) + assert state is not None + assert state.kind == "status_box", f"expected status_box, got {state.kind!r}" + + menu = _last_show_menu_for(users["Alice"], "transient_display") + assert menu is not None + text_blob = " ".join(_item_text(it) for it in menu["items"]) + assert "Alice" in text_blob, "viewer's own name should be in the header" + assert "Bob" not in text_blob + + +# --------------------------------------------------------------------------- +# `shift+c` (picker) path +# --------------------------------------------------------------------------- + + +def test_view_all_scoresheets_opens_player_picker(): + game, players, users = _make_game([("Alice", "en"), ("Bob", "en")]) + alice = players["Alice"] + + game._action_view_all_scoresheets(alice, "view_all_scoresheets") + + state = game._get_transient_display_state(alice) + assert state is not None + assert state.kind == "scoresheet_player_select" + + menu = _last_show_menu_for(users["Alice"], "transient_display") + assert menu is not None + item_ids = [_item_id(it) for it in menu["items"]] + assert players["Alice"].id in item_ids + assert players["Bob"].id in item_ids + assert "transient_display_back" in item_ids, "Back item should be last entry" + + +def test_view_all_scoresheets_excludes_spectators_from_picker(): + game, players, users = _make_game( + [("Alice", "en"), ("Bob", "en")], + spectators=["Spec"], + ) + alice = players["Alice"] + + game._action_view_all_scoresheets(alice, "view_all_scoresheets") + + menu = _last_show_menu_for(users["Alice"], "transient_display") + assert menu is not None + item_ids = [_item_id(it) for it in menu["items"]] + assert players["Spec"].id not in item_ids, "spectators should not appear as targets" + assert players["Alice"].id in item_ids + assert players["Bob"].id in item_ids + + +def test_selecting_player_in_picker_renders_their_scoresheet(): + game, players, users = _make_game([("Alice", "en"), ("Bob", "en")]) + alice = players["Alice"] + bob = players["Bob"] + bob.scores["ones"] = 3 + bob.scores["yahtzee"] = 50 + + game._action_view_all_scoresheets(alice, "view_all_scoresheets") + game._handle_transient_display_selection(alice, bob.id) + + state = game._get_transient_display_state(alice) + assert state is not None + assert state.kind == "status_box" + + box_menu = _last_show_menu_for(users["Alice"], "transient_display") + assert box_menu is not None + text_blob = " ".join(_item_text(it) for it in box_menu["items"]) + assert "Bob" in text_blob, "scoresheet should be for Bob (the selected target)" + + +def test_selecting_back_in_picker_closes_it_without_opening_sheet(): + game, players, users = _make_game([("Alice", "en"), ("Bob", "en")]) + alice = players["Alice"] + + game._action_view_all_scoresheets(alice, "view_all_scoresheets") + assert game._get_transient_display_state(alice).kind == "scoresheet_player_select" + + game._handle_transient_display_selection(alice, "transient_display_back") + + assert game._get_transient_display_state(alice) is None + + +# --------------------------------------------------------------------------- +# Spectator fallback: `c` opens picker because spectators have no own sheet. +# --------------------------------------------------------------------------- + + +def test_spectator_pressing_view_scoresheet_falls_back_to_picker(): + game, players, users = _make_game( + [("Alice", "en")], + spectators=["Spec"], + ) + spec = players["Spec"] + + game._action_view_scoresheet(spec, "view_scoresheet") + + state = game._get_transient_display_state(spec) + assert state is not None + assert state.kind == "scoresheet_player_select", ( + "spectator with no own sheet should be routed to the picker" + ) + + menu = _last_show_menu_for(users["Spec"], "transient_display") + assert menu is not None + item_ids = [_item_id(it) for it in menu["items"]] + assert players["Alice"].id in item_ids + assert spec.id not in item_ids, "spectator should not appear as a target" + + +# --------------------------------------------------------------------------- +# Edge cases: silence-is-a-bug feedback. +# --------------------------------------------------------------------------- + + +def test_picker_with_no_active_players_speaks_not_found(): + """If the picker is somehow triggered with no active players, the action + must produce speech (per CLAUDE.md "silence is a bug").""" + game = YahtzeeGame() + user = MockUser("Spec") + spec = game.add_spectator("Spec", user) + user.clear_messages() + + game._action_view_all_scoresheets(spec, "view_all_scoresheets") + + assert game._get_transient_display_state(spec) is None + assert "Player not found." in _spoken_texts(user) + + +def test_picker_with_stale_selection_id_speaks_not_found(): + """If a player is selected from the picker but has since disconnected / + been removed, the action must produce speech rather than silently dropping.""" + game, players, users = _make_game([("Alice", "en"), ("Bob", "en")]) + alice = players["Alice"] + + game._action_view_all_scoresheets(alice, "view_all_scoresheets") + users["Alice"].clear_messages() + + game._handle_transient_display_selection(alice, "nonexistent-player-id") + + assert game._get_transient_display_state(alice) is None + assert "Player not found." in _spoken_texts(users["Alice"]) + + +# --------------------------------------------------------------------------- +# Re-pressing shift+c while a sheet is open should reopen the picker. +# --------------------------------------------------------------------------- + + +def test_pressing_view_all_scoresheets_again_replaces_open_sheet_with_picker(): + game, players, users = _make_game([("Alice", "en"), ("Bob", "en")]) + alice = players["Alice"] + bob = players["Bob"] + + game._action_view_all_scoresheets(alice, "view_all_scoresheets") + game._handle_transient_display_selection(alice, bob.id) + assert game._get_transient_display_state(alice).kind == "status_box" + + game._action_view_all_scoresheets(alice, "view_all_scoresheets") + + state = game._get_transient_display_state(alice) + assert state is not None + assert state.kind == "scoresheet_player_select"