Implemented viewing game help documents from inside the game

[mohammad-aloufi]

Summary

This PR implements a universally accessible F1 shortcut that allows players to seamlessly pull up the rulebook for their current game without leaving the table. Modifies both the desktop client functionality and the server's documentation/game event plumbing.

Key Changes

Client Side

• Global Keybind Registration (main_window.py): Added F1 natively to the AcceleratorTable. Previously, F1 could only be parsed if the player's cursor was specifically focused on the game menu list. Elevating this to the accelerator configuration guarantees the signal reliably fires from anywhere in the application.

Server Side

• New Core Action (action_set_creation_mixin.py): Defined an explicit "show_rules" action internally mapped to the inbound {"key": "f1"} packet binding across all game tables.
• Always Available Logic (action_visibility_mixin.py): Handled visibility restrictions by introducing _is_always_enabled, securely ensuring that pressing F1 cannot be blocked regardless of the match's turn state.
• Document Integration (menu_management_mixin.py): Authored _action_show_rules, which serves as a programmatic bridge between live games and the static document system. When fired, it references the table's master _documents manager, dynamically parses the appropriate folder ([game_type]_rules), respects active localization overrides, and ships the Markdown safely back to the client's MarkdownViewerDialog.
• Localization Hook (games.ftl): Populated the master configuration string definitions for "show-rules".

Testing Notes

• Deployed a local test environment matching the new AcceleratorTable logic.
• Traced inbound network packets simulating an f1 keybind payload against a simulated lobby table.

[zarvox32]

Review generated by Claude (Anthropic's coding assistant), posted at the repo owner's request.

Thanks for tackling this — universal F1 for rules is a genuine accessibility win. Claude tested the PR end-to-end (real BackgammonGame wired to a real DocumentManager, real {"type":"keybind","key":"f1"} packet) and the happy path works, including locale fallback (French user with French translation gets French; Spanish user falls back to English) and correct exclusion of non-public locales. Spectators can also use F1, which is good. A few things to address before merge.

Blocking

1. Silent / gibberish failure when _table._server is missing. _action_show_rules calls user.speak_l("action-not-available"), but action-not-available does not exist in server/locales/en/main.ftl. Fluent throws KeyError, the localizer falls through to f"[{message_id}]", and the screen-reader user literally hears "[action-not-available]". Per CLAUDE.md ("silence is a bug"), this isn't acceptable. Add the Fluent message, or reuse an existing one.

2. Dead hasattr guard hides wiring problems. In menu_management_mixin.py:241:

if hasattr(self, "_table") and self._table and hasattr(self._table, "_server") and self._table._server:

self._table is always defined on Game (set to None in __post_init__), so hasattr(self, "_table") is dead code. The triple-guard turns "table not yet wired to server" into the silent failure above. If wiring is broken we want a loud error, not a vague "action not available."

3. Reaches into private API and reinvents existing logic. The handler calls doc_manager._get_visible_locale_codes(...) (underscore-prefixed) and rolls its own locale-selection loop. DocumentManager._select_display_title_locale already does exactly this and is what get_documents_in_category uses. The new loop also has dead code — or locale_code == title_locale never fires, because title_locale is only set right before break.

Design / cleanup

4. The new _is_always_enabled callback is redundant. Two functionally identical callbacks already live in action_visibility_mixin.py:

def _is_leave_game_enabled(self, player):     # line 113
    return None
def _is_show_actions_enabled(self, player):   # line 161
    return None

Both are wired to KeybindState.ALWAYS actions (ctrl+q, escape) — either could have been used for show_rules. A generic _is_always_enabled reads better at the call site than reusing _is_leave_game_enabled for an unrelated action, but then the right move is to migrate leave_game and show_actions over to the new helper and delete the two redundant copies. As written, the PR leaves three "return None" callbacks sitting side-by-side, which is the worst of both worlds.

(For clarity, this is an is_enabled callback on Action — not a new KeybindState enum value. The PR correctly uses the existing KeybindState.ALWAYS.)

5. Tight coupling. self._table._server._documents reaches three layers down. A single accessor on Table (e.g., table.get_documents()) would localize this knowledge.

6. Trailing whitespace on many lines in _action_show_rules.

7. Possible duplicate F1 packet on the desktop client. F1 was already mapped in _map_function_key (line 704) and sent via on_char_hook when focus is on the menu list. The PR adds it as a global accelerator too. wxWidgets accelerators usually consume the event before EVT_CHAR_HOOK, but the order is platform-sensitive — worth manually verifying on Windows that pressing F1 with focus on the menu list doesn't fire two keybind packets. Net effect would be harmless (idempotent editbox open), but a duplicate is a smell.

8. F1 from the lobby (no game) is silent. Client always sends the packet when connected; server drops it if the user isn't at a table. Not a regression, but for a "global rules shortcut" some feedback ("you must be at a table to view rules") would be friendlier.

Testing

The PR description says "Deployed a local test environment" and "Traced inbound network packets," but the PR adds zero automated tests for new code that touches keybind dispatch, document lookup, and locale selection. Claude wrote 8 targeted tests covering the end-to-end path, locale fallback, private-locale exclusion, missing-document handling, and the _table._server is None case (which is what surfaced the missing-Fluent-ID bug). ~150 LOC available if you'd like them attached or pushed as a follow-up commit.

[mohammad-aloufi]

Done.
Summary of fixes:

1. Fixed Silent Failure (KeyError):
   Replaced an unlocalized action-not-available string with the existing, localized action-locked string. This prevents a Fluent KeyError that was causing screen-reader users to hear literal brackets when an action failed.

2. Removed Dead Guards:
   Removed unnecessary hasattr triple-guards (self._table and self._table._server). This ensures that if a game's table wiring is broken during development, it fails loudly with an AttributeError instead of hiding the problem behind a generic "action not available" message.

3. Reused DocumentManager APIs:
   Refactored the custom, manual (and slightly broken) locale-selection loop. The rules handler now correctly delegates to DocumentManager's native _select_display_title_locale and _select_visible_title methods to resolve the best localization, matching the rest of the codebase.

4. Deduplicated Action Callbacks:
   Deleted the redundant _is_leave_game_enabled and _is_show_actions_enabled callbacks (which simply returned None). Both of these actions were migrated to use the newly introduced, generic _is_always_enabled helper.

5. Reduced Tight Coupling:
   Added a get_documents() accessor to the Table class. This abstracts away the server's internal layout, allowing the game rules handler to avoid reaching three layers deep (self._table._server._documents).

6. Cleaned Up Formatting:
   Removed all trailing whitespace across the _action_show_rules method block.

7. Added Lobby Feedback:
   Updated the server's global keybind handler to intercept F1 when a user isn't at a table. Instead of silently dropping the packet, it now gives the friendly, localized feedback: "You must be at a table to view rules."

8. Fixed Desktop Client Duplicate Packet Smell:
   Removed F1 from the manual _map_function_key handler in the desktop client. Because the PR correctly registered F1 as a global application accelerator, leaving it in the manual hook map could cause wxWidgets to fire both events on Windows when the menu list had focus. Relying solely on the global accelerator guarantees exactly one network packet is sent.