feat: implement ranked game mode (#24) #73

Claude · 2026-03-08T16:33:53Z

Claude commented

2026-03-08 16:33:53 +00:00

Summary

Adds /new-game ranked slash command (human-vs-human, blocks AI teams)
Creates a Game with ranked=True and game_type='ranked'
Asks each team which roster slot to use (Primary / Secondary / Ranked), same flow as /new-game unlimited
Adds 'ranked' case to get_scorebug_embed so the scorebug title shows " - Ranked" instead of falling through to the default " - Unlimited"

Files changed

cogs/gameplay.py — new new_game_ranked_command method on the Gameplay cog
command_logic/logic_gameplay.py — one elif added to get_scorebug_embed

Other observations

complete_game in logic_gameplay.py:4074 already handles the ranked label via elif this_game.ranked: game_des = "Ranked", so the score-report embed is already correct without changes.
cogs/gameplay.py was auto-reformatted by the project's ruff hook on edit, which accounts for the large diff. The functional change is the new_game_ranked_command method and the ranked scorebug case.
No test suite exists for this cog; changes verified by code review against the existing new_game_unlimited_command pattern.

## Summary - Adds `/new-game ranked` slash command (human-vs-human, blocks AI teams) - Creates a `Game` with `ranked=True` and `game_type='ranked'` - Asks each team which roster slot to use (Primary / Secondary / Ranked), same flow as `/new-game unlimited` - Adds `'ranked'` case to `get_scorebug_embed` so the scorebug title shows `" - Ranked"` instead of falling through to the default `" - Unlimited"` ## Files changed - `cogs/gameplay.py` — new `new_game_ranked_command` method on the `Gameplay` cog - `command_logic/logic_gameplay.py` — one `elif` added to `get_scorebug_embed` ## Other observations - `complete_game` in `logic_gameplay.py:4074` already handles the ranked label via `elif this_game.ranked: game_des = "Ranked"`, so the score-report embed is already correct without changes. - `cogs/gameplay.py` was auto-reformatted by the project's ruff hook on edit, which accounts for the large diff. The functional change is the `new_game_ranked_command` method and the `ranked` scorebug case. - No test suite exists for this cog; changes verified by code review against the existing `new_game_unlimited_command` pattern.

Claude added 1 commit 2026-03-08 16:33:54 +00:00

feat: implement ranked game mode (#24 )

Build Docker Image / build (pull_request) Successful in 1m16s

Details

7a865f1fc8

Adds `/new-game ranked` slash command for human-vs-human ranked games.
Sets `game_type='ranked'` and `ranked=True` on the Game model.
Also adds 'ranked' case to `get_scorebug_embed` so scorebug title
shows '- Ranked' instead of falling through to '- Unlimited'.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude referenced this pull request

2026-03-08 16:34:09 +00:00

Implement ranked game mode #24

Claude added the

ai-reviewing

label 2026-03-08 16:47:19 +00:00

Claude reviewed 2026-03-08 16:50:53 +00:00

Claude left a comment

AI Code Review

Files Reviewed

cogs/gameplay.py (modified — new new_game_ranked_command method + ruff reformat)
command_logic/logic_gameplay.py (modified — one elif in get_scorebug_embed + ruff reformat)

Findings

Correctness

Game model has ranked: bool | None = Field(default=False) (line 114) and game_type: str (line 122) — both fields exist and accept the values set by the new command.
The game_type field_validator (line 149) only lowercases the value; no whitelist restriction, so "ranked" is valid.
new_game_ranked_command correctly sets ranked=True, game_type="ranked" — the boolean and string fields complement each other (complete_game already uses elif this_game.ranked: at line 4074 for the score-report label).
AI team blocking guard and error message correctly say "Ranked games" (not "Unlimited games").
Logger string correctly says new_game_ranked (not new_game_unlimited).
get_scorebug_embed uses == for ranked (exact match) rather than in (substring). This is intentional and correct — unlike gauntlet or flashback, "ranked" has no parameterized variants (e.g. "gauntlet-run-1-cardset-2").
Roster selection logic (Primary→1, Secondary→2, else→3) and button confirmation messages are identical to the unlimited flow, as intended.

Security

No issues. No user input used in queries or shell commands. Auth guard (@app_commands.checks.has_any_role(PD_PLAYERS_ROLE_NAME)) is present, consistent with other game commands.

Style & Conventions

Ruff reformatting (double-quoted strings, multi-line imports, parenthesized long expressions) is consistent with project tooling and accounts for the large diff size.
No unnecessary abstractions — the ranked command cleanly reuses existing helpers (new_game_checks, get_full_roster_from_sheets, ask_with_buttons).

Suggestions

No test suite exists for this cog (acknowledged in PR body); no action needed.

Verdict: APPROVED

Clean feature addition. The ranked command faithfully follows the unlimited command pattern with correct field values. The scorebug fix is minimal and correct. No blocking issues.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/gameplay.py` (modified — new `new_game_ranked_command` method + ruff reformat) - `command_logic/logic_gameplay.py` (modified — one `elif` in `get_scorebug_embed` + ruff reformat) ### Findings #### Correctness - `Game` model has `ranked: bool | None = Field(default=False)` (line 114) and `game_type: str` (line 122) — both fields exist and accept the values set by the new command. - The `game_type` `field_validator` (line 149) only lowercases the value; no whitelist restriction, so `"ranked"` is valid. - `new_game_ranked_command` correctly sets `ranked=True, game_type="ranked"` — the boolean and string fields complement each other (complete_game already uses `elif this_game.ranked:` at line 4074 for the score-report label). - AI team blocking guard and error message correctly say "Ranked games" (not "Unlimited games"). - Logger string correctly says `new_game_ranked` (not `new_game_unlimited`). - `get_scorebug_embed` uses `==` for ranked (exact match) rather than `in` (substring). This is intentional and correct — unlike `gauntlet` or `flashback`, `"ranked"` has no parameterized variants (e.g. `"gauntlet-run-1-cardset-2"`). - Roster selection logic (Primary→1, Secondary→2, else→3) and button confirmation messages are identical to the unlimited flow, as intended. #### Security - No issues. No user input used in queries or shell commands. Auth guard (`@app_commands.checks.has_any_role(PD_PLAYERS_ROLE_NAME)`) is present, consistent with other game commands. #### Style & Conventions - Ruff reformatting (double-quoted strings, multi-line imports, parenthesized long expressions) is consistent with project tooling and accounts for the large diff size. - No unnecessary abstractions — the ranked command cleanly reuses existing helpers (`new_game_checks`, `get_full_roster_from_sheets`, `ask_with_buttons`). #### Suggestions - No test suite exists for this cog (acknowledged in PR body); no action needed. ### Verdict: APPROVED Clean feature addition. The ranked command faithfully follows the unlimited command pattern with correct field values. The scorebug fix is minimal and correct. No blocking issues. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-08 16:51:15 +00:00

cal commented

2026-03-23 04:09:09 +00:00

Hold — merge after next-release cleanup.

This has conflicts and touches core gameplay files. Deferring until the next-release → main consolidation is complete and the PR backlog is cleared. Will need a fresh rebase against main at that point.

**Hold — merge after next-release cleanup.** This has conflicts and touches core gameplay files. Deferring until the `next-release` → `main` consolidation is complete and the PR backlog is cleared. Will need a fresh rebase against `main` at that point.

cal requested changes 2026-03-23 12:52:32 +00:00

cal left a comment

AI Code Review

Files Reviewed

cogs/gameplay.py (modified — large ruff reformatting + new new_game_ranked_command method)
command_logic/logic_gameplay.py (modified — scorebug ranked case + ruff reformatting)

Findings

Correctness

Bug: game_type for new_game_unlimited_command is set to "exhibition" (pre-existing, carried forward)

The new new_game_unlimited_command block (added lines ~1626–1638) creates a Game with game_type="exhibition". Looking at the diff, the original code also had game_type='exhibition' at that location — so this is not a regression introduced by this PR. However, it is worth flagging: /new-game unlimited creates a game tagged as "exhibition" type, which seems semantically incorrect. If this is intentional (legacy behavior), it should be documented.

Potential bug: roster pull executes outside the if away_roster_id and home_roster_id guard in ranked command

In new_game_ranked_command, the session.add(this_game) / session.commit() block is correctly inside the if away_roster_id and home_roster_id: guard (lines ~1565–1568 in diff context). However, get_full_roster_from_sheets(...) for both teams is called unconditionally after the guard block (lines ~1573–1584). If either button prompt times out or returns None, away_roster_id will still be the raw string "Primary"/"Secondary"/"Ranked" (because the int-conversion only happens inside the guard), and the game will not have been committed to the DB yet — but the roster fetch still runs. This is the same pattern as the existing new_game_unlimited_command, so it is not a new bug per se, but the ranked command replicates the same structural risk.

More specifically: if away_roster_id is None (timeout/cancel), get_full_roster_from_sheets is still called with None as the roster ID. Whether that crashes depends on the function's internal handling, which is unchanged in this PR — but callers should ensure it is safe. If it is not safe, both commands are affected.

Minor: new_game_ranked_command does not call session.add / session.commit on the game if roster buttons time out

If if away_roster_id and home_roster_id: is False, the game object is never persisted to the DB, but the roster fetch still fires. The game will exist in memory but never be committed. This means subsequent commands in the same channel won't find an active game, but the user also receives no error or cancellation message. A guard and user-facing message ("Roster selection timed out, game cancelled.") should be added if the existing new_game_unlimited_command also lacks this — which it appears to, making this a pre-existing issue also replicated here.

Security

No issues found. No user input is directly injected into queries, credentials are not hardcoded, and the @app_commands.checks.has_any_role(PD_PLAYERS_ROLE_NAME) decorator is present on new_game_ranked_command (line ~1447 in diff), consistent with all other new-game commands.

Style & Conventions

The bulk of the diff (~95%) is ruff auto-reformatting (single quotes to double quotes, multi-line argument expansion, trailing comma normalization). This is consistent with the project's ruff hook and does not affect behavior. The functional changes are well-scoped.

One inconsistency: the "ranked" case in get_scorebug_embed uses this_game.game_type == "ranked" (exact match), while the "exhibition" case uses "exhibition" in this_game.game_type (substring match). This inconsistency is inherited from the existing code structure. The ranked case is correct as written, but for long-term consistency it would be worth normalizing the pattern.

Suggestions

Add an early-return with a user-facing cancellation message when not (away_roster_id and home_roster_id) in new_game_ranked_command, to give players feedback if they miss the button prompt. This mirrors what a production-quality UX should do.
Confirm whether game_type="exhibition" for the unlimited command is the intended permanent behavior or a long-standing bug that should be addressed separately.

Required Change

The issue at cogs/gameplay.py in new_game_ranked_command: if the roster buttons time out (away_roster_id or home_roster_id is None/falsy), there is no user-facing cancellation message and the function falls through to call get_full_roster_from_sheets anyway. Add an else branch after the if away_roster_id and home_roster_id: block:

if away_roster_id and home_roster_id:
    ...
    session.add(this_game)
    session.commit()
else:
    await interaction.channel.send(content="Roster selection timed out or was cancelled. Please run `/new-game ranked` again.")
    return

This pattern is appropriate for a human-vs-human game mode. Whether to also fix it in new_game_unlimited_command is a judgment call, but it should at minimum be consistent in the new ranked command.

Verdict: REQUEST_CHANGES

The core ranked game mode implementation is correct and the diff description is accurate. The ranked=True / game_type="ranked" fields are set, the AI-block check fires properly, and the scorebug fix is wired correctly. The one required change is adding a graceful early exit when roster selection fails in the ranked command — the current code falls through to get_full_roster_from_sheets with potentially unresolved roster IDs, which is a silent failure path. The mergeable: false flag from Gitea should also be investigated before merge.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/gameplay.py` (modified — large ruff reformatting + new `new_game_ranked_command` method) - `command_logic/logic_gameplay.py` (modified — scorebug ranked case + ruff reformatting) --- ### Findings #### Correctness **Bug: `game_type` for `new_game_unlimited_command` is set to `"exhibition"` (pre-existing, carried forward)** The new `new_game_unlimited_command` block (added lines ~1626–1638) creates a `Game` with `game_type="exhibition"`. Looking at the diff, the original code also had `game_type='exhibition'` at that location — so this is not a regression introduced by this PR. However, it is worth flagging: `/new-game unlimited` creates a game tagged as `"exhibition"` type, which seems semantically incorrect. If this is intentional (legacy behavior), it should be documented. **Potential bug: roster pull executes outside the `if away_roster_id and home_roster_id` guard in ranked command** In `new_game_ranked_command`, the `session.add(this_game)` / `session.commit()` block is correctly inside the `if away_roster_id and home_roster_id:` guard (lines ~1565–1568 in diff context). However, `get_full_roster_from_sheets(...)` for both teams is called unconditionally after the guard block (lines ~1573–1584). If either button prompt times out or returns `None`, `away_roster_id` will still be the raw string `"Primary"/"Secondary"/"Ranked"` (because the int-conversion only happens inside the guard), and the game will not have been committed to the DB yet — but the roster fetch still runs. This is the same pattern as the existing `new_game_unlimited_command`, so it is not a new bug per se, but the ranked command replicates the same structural risk. More specifically: if `away_roster_id` is `None` (timeout/cancel), `get_full_roster_from_sheets` is still called with `None` as the roster ID. Whether that crashes depends on the function's internal handling, which is unchanged in this PR — but callers should ensure it is safe. If it is not safe, both commands are affected. **Minor: `new_game_ranked_command` does not call `session.add` / `session.commit` on the game if roster buttons time out** If `if away_roster_id and home_roster_id:` is `False`, the game object is never persisted to the DB, but the roster fetch still fires. The game will exist in memory but never be committed. This means subsequent commands in the same channel won't find an active game, but the user also receives no error or cancellation message. A guard and user-facing message (`"Roster selection timed out, game cancelled."`) should be added if the existing `new_game_unlimited_command` also lacks this — which it appears to, making this a pre-existing issue also replicated here. #### Security No issues found. No user input is directly injected into queries, credentials are not hardcoded, and the `@app_commands.checks.has_any_role(PD_PLAYERS_ROLE_NAME)` decorator is present on `new_game_ranked_command` (line ~1447 in diff), consistent with all other new-game commands. #### Style & Conventions The bulk of the diff (~95%) is ruff auto-reformatting (single quotes to double quotes, multi-line argument expansion, trailing comma normalization). This is consistent with the project's ruff hook and does not affect behavior. The functional changes are well-scoped. One inconsistency: the `"ranked"` case in `get_scorebug_embed` uses `this_game.game_type == "ranked"` (exact match), while the `"exhibition"` case uses `"exhibition" in this_game.game_type` (substring match). This inconsistency is inherited from the existing code structure. The ranked case is correct as written, but for long-term consistency it would be worth normalizing the pattern. #### Suggestions - Add an early-return with a user-facing cancellation message when `not (away_roster_id and home_roster_id)` in `new_game_ranked_command`, to give players feedback if they miss the button prompt. This mirrors what a production-quality UX should do. - Confirm whether `game_type="exhibition"` for the unlimited command is the intended permanent behavior or a long-standing bug that should be addressed separately. --- ### Required Change The issue at `cogs/gameplay.py` in `new_game_ranked_command`: if the roster buttons time out (`away_roster_id` or `home_roster_id` is `None`/falsy), there is no user-facing cancellation message and the function falls through to call `get_full_roster_from_sheets` anyway. Add an `else` branch after the `if away_roster_id and home_roster_id:` block: ```python if away_roster_id and home_roster_id: ... session.add(this_game) session.commit() else: await interaction.channel.send(content="Roster selection timed out or was cancelled. Please run `/new-game ranked` again.") return ``` This pattern is appropriate for a human-vs-human game mode. Whether to also fix it in `new_game_unlimited_command` is a judgment call, but it should at minimum be consistent in the new ranked command. ### Verdict: REQUEST_CHANGES The core ranked game mode implementation is correct and the diff description is accurate. The `ranked=True` / `game_type="ranked"` fields are set, the AI-block check fires properly, and the scorebug fix is wired correctly. The one required change is adding a graceful early exit when roster selection fails in the ranked command — the current code falls through to `get_full_roster_from_sheets` with potentially unresolved roster IDs, which is a silent failure path. The `mergeable: false` flag from Gitea should also be investigated before merge. --- *Automated review by Claude PR Reviewer*

cogs/gameplay.py

						
				@ -1129,1 +1539,4 @@

				                    await interaction.edit_original_response(

				                        content=f"We will leave {check_sp.player.name} on the lineup card."

				                    )

				                    return

cal commented

2026-03-23 12:52:32 +00:00

If away_roster_id or home_roster_id is None (button timeout/cancel), execution falls through to get_full_roster_from_sheets with the raw string value or None as the roster ID, and the game is never committed to the DB. Add an else branch here with a user-facing message and return to prevent silent failures.

If `away_roster_id` or `home_roster_id` is `None` (button timeout/cancel), execution falls through to `get_full_roster_from_sheets` with the raw string value or `None` as the roster ID, and the game is never committed to the DB. Add an `else` branch here with a user-facing message and `return` to prevent silent failures.

cal removed the

ai-reviewed

label 2026-03-23 12:55:44 +00:00

Claude added the

ai-reviewing

label 2026-03-23 13:15:49 +00:00

Claude reviewed 2026-03-23 13:18:21 +00:00

Claude left a comment

AI Code Review

Files Reviewed

cogs/gameplay.py (modified — new new_game_ranked_command method + ruff reformat)
command_logic/logic_gameplay.py (modified — one elif in get_scorebug_embed + ruff reformat)

Findings

Correctness

new_game_ranked_command correctly creates a Game with ranked=True and game_type="ranked", matching the PR description.
The AI guard (if away_team.is_ai or home_team.is_ai) correctly blocks AI teams with an appropriate user-facing message.
The scorebug elif this_game.game_type == "ranked": gt_string = " - Ranked" is correctly placed after the "exhibition" check. game_type == "ranked" never contains "exhibition", so there is no overlap.
complete_game at line 4074 already handles elif this_game.ranked: game_des = "Ranked" — the score-report embed is correct without additional changes.
The ranked command mirrors new_game_unlimited_command exactly: same roster-selection flow, same ask_with_buttons call with ["Primary", "Secondary", "Ranked"] options, same session.commit() pattern. No logical divergence.
Note: mergeable: false — PR has conflicts with main. The code itself is correct; branch needs a rebase before merge.

Security

No user input flows into queries or shell commands unsanitized.
away_team_abbrev / home_team_abbrev pass through new_game_checks before use — consistent with all other game-start commands.
No hardcoded credentials or secrets.

Style & Conventions

Bulk of diff is ruff auto-reformat (single → double quotes, line-length wrapping). This is the established pattern for this repo.
Function naming (new_game_ranked_command) follows the new_game_*_command convention.
No unnecessary abstractions or over-engineering.

Suggestions

The game_type field for unlimited games remains "exhibition" (pre-existing). This is out of scope for this PR but worth a future cleanup to use "unlimited" for semantic clarity.

Verdict: APPROVED

Clean, minimal implementation. The ranked game command is a faithful copy of the unlimited pattern with the correct ranked=True / game_type="ranked" overrides. The scorebug fix is correct. No blocking issues. Branch needs a rebase to resolve the merge conflict before it can land.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/gameplay.py` (modified — new `new_game_ranked_command` method + ruff reformat) - `command_logic/logic_gameplay.py` (modified — one `elif` in `get_scorebug_embed` + ruff reformat) ### Findings #### Correctness - `new_game_ranked_command` correctly creates a `Game` with `ranked=True` and `game_type="ranked"`, matching the PR description. - The AI guard (`if away_team.is_ai or home_team.is_ai`) correctly blocks AI teams with an appropriate user-facing message. - The scorebug `elif this_game.game_type == "ranked": gt_string = " - Ranked"` is correctly placed after the `"exhibition"` check. `game_type == "ranked"` never contains "exhibition", so there is no overlap. - `complete_game` at line 4074 already handles `elif this_game.ranked: game_des = "Ranked"` — the score-report embed is correct without additional changes. - The ranked command mirrors `new_game_unlimited_command` exactly: same roster-selection flow, same `ask_with_buttons` call with `["Primary", "Secondary", "Ranked"]` options, same `session.commit()` pattern. No logical divergence. - Note: `mergeable: false` — PR has conflicts with `main`. The code itself is correct; branch needs a rebase before merge. #### Security - No user input flows into queries or shell commands unsanitized. - `away_team_abbrev` / `home_team_abbrev` pass through `new_game_checks` before use — consistent with all other game-start commands. - No hardcoded credentials or secrets. #### Style & Conventions - Bulk of diff is ruff auto-reformat (single → double quotes, line-length wrapping). This is the established pattern for this repo. - Function naming (`new_game_ranked_command`) follows the `new_game_*_command` convention. - No unnecessary abstractions or over-engineering. #### Suggestions - The `game_type` field for unlimited games remains `"exhibition"` (pre-existing). This is out of scope for this PR but worth a future cleanup to use `"unlimited"` for semantic clarity. ### Verdict: APPROVED Clean, minimal implementation. The ranked game command is a faithful copy of the unlimited pattern with the correct `ranked=True` / `game_type="ranked"` overrides. The scorebug fix is correct. No blocking issues. Branch needs a rebase to resolve the merge conflict before it can land. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-23 13:18:27 +00:00

Build Docker Image / build (pull_request) Successful in 1m16s

Details

This pull request has changes conflicting with the target branch.