feat(crafting): add /craft command for duplicate card tier-up (#49) #171

Claude · 2026-04-12T03:10:55Z

Claude commented

2026-04-12 03:10:55 +00:00

Closes #49

Summary

Implements the /craft command — combine 2 cards of the same rarity into 1 random card of the next tier up.

Craft chain: Replacement → Reserve → Starter → All-Star → MVP → HoF

What's in this PR

cogs/economy_new/crafting.py — new Crafting cog with /craft slash command
paperdynasty.py — registers cogs.economy_new.crafting in COGS list

Command flow

/craft [rarity] — rarity is optional; if omitted, shows an info embed + 5 rarity buttons
Bot fetches team's cards of the chosen rarity, sorted A–Z
Discord Select menu (min=2, max=2) shows up to 25 eligible cards
Confirmation embed shows both cards to consume and the target rarity
On confirm: calls GET /v2/teams/{id}/craft?ts=...&ids={id1},{id2} (same pattern as sell/cards and buy/players)
Result card displayed via get_card_embeds()

API dependency

This PR requires a new API endpoint on cal/paper-dynasty-database:

GET /v2/teams/{team_id}/craft?ts={hash}&ids={card1_id},{card2_id}

Expected behavior:

Validate ts hash (same as buy/sell)
Delete both source cards from the team
Select a random player of the next rarity tier from LIVE_CARDSET_ID
Create and return a new card for that player

Expected response shape: {"card": { ...card object... }}

Other observations

Discord type stubs produce interaction.channel.send Pyright warnings — identical to the same pattern used throughout marketplace.py, economy.py, players.py. Not a new regression.
No tests added: crafting requires live API interaction (same as buy/sell/packs). Covered by manual integration testing once the API endpoint exists.

Closes #49 ## Summary Implements the `/craft` command — combine 2 cards of the same rarity into 1 random card of the next tier up. **Craft chain:** Replacement → Reserve → Starter → All-Star → MVP → HoF ## What's in this PR - **`cogs/economy_new/crafting.py`** — new `Crafting` cog with `/craft` slash command - **`paperdynasty.py`** — registers `cogs.economy_new.crafting` in COGS list ## Command flow 1. `/craft [rarity]` — rarity is optional; if omitted, shows an info embed + 5 rarity buttons 2. Bot fetches team's cards of the chosen rarity, sorted A–Z 3. Discord Select menu (min=2, max=2) shows up to 25 eligible cards 4. Confirmation embed shows both cards to consume and the target rarity 5. On confirm: calls `GET /v2/teams/{id}/craft?ts=...&ids={id1},{id2}` (same pattern as `sell/cards` and `buy/players`) 6. Result card displayed via `get_card_embeds()` ## API dependency This PR requires a **new API endpoint** on `cal/paper-dynasty-database`: ``` GET /v2/teams/{team_id}/craft?ts={hash}&ids={card1_id},{card2_id} ``` Expected behavior: - Validate `ts` hash (same as buy/sell) - Delete both source cards from the team - Select a random player of the next rarity tier from `LIVE_CARDSET_ID` - Create and return a new card for that player Expected response shape: `{"card": { ...card object... }}` ## Other observations - Discord type stubs produce `interaction.channel.send` Pyright warnings — identical to the same pattern used throughout marketplace.py, economy.py, players.py. Not a new regression. - No tests added: crafting requires live API interaction (same as buy/sell/packs). Covered by manual integration testing once the API endpoint exists.

Claude added 1 commit 2026-04-12 03:10:57 +00:00

feat(crafting): add /craft command for duplicate card tier-up (#49 )

Ruff Lint / lint (pull_request) Successful in 38s

Details

e9b1c3da98

Closes #49

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

Claude referenced this pull request

2026-04-12 03:11:06 +00:00

Feature: Duplicate Card Crafting (tier-up system) #49

claude-reviewer added the

ai-reviewing

label 2026-04-12 03:15:54 +00:00

claude-reviewer approved these changes 2026-04-12 03:18:45 +00:00

claude-reviewer left a comment

AI Code Review

Files Reviewed

cogs/economy_new/crafting.py (added)
paperdynasty.py (modified — cog registration)

Findings

Correctness

No issues found.

All imports resolve: db_get, team_hash, PD_PLAYERS, get_team_by_owner, get_card_embeds, get_channel, get_team_embed, Confirm, ButtonOptions, SelectView all confirmed present in their respective modules.
Interaction response/followup path is correct in both branches: send_message + followup.send (rarity=None path) and defer + followup.send (rarity provided). No double-respond risk.
CRAFT_TIER_UP KeyError is not reachable — rarity is constrained to valid dict keys by both app_commands.choices and hardcoded ButtonOptions labels. HoF correctly excluded as a source.
new_card = result.get("card", result) handles the expected {"card": {...}} response shape correctly. Defensive fallback is fine.
SelectCraftCards.callback uses edit_message(view=None) correctly to remove the select after submission — prevents re-triggering.
min_values=2, max_values=2 enforces exactly 2 cards at the Discord protocol level.

Security

No issues found.

team_hash auth matches the sell/cards and buy/players pattern exactly — no auth bypass possible.
No user-supplied strings interpolated into SQL or API paths. card1_id / card2_id are cast to int from select values, then formatted as integers in the query param. Safe.

Style & Conventions

No issues found.

Channel restriction check matches marketplace.py exactly.
db_get for a mutating endpoint is the established project pattern (same as buy/sell).
Module structure (class SelectCraftCards, class Crafting, async def setup) follows marketplace.py and scouting.py conventions.
Logger name "discord_app" is correct.

Suggestions

Craft API call uses default 5s timeout. sell/cards uses timeout=10, buy/players uses timeout=10. Crafting involves validate + delete 2 cards + create 1 card — consider timeout=10 or timeout=15 to reduce false "didn't go through" failures under API load.
```
result = await db_get(
    f"teams/{self.owner_team['id']}/craft",
    params=[...],
    timeout=10,  # add this
)
```
Stale info embed after rarity button click. When rarity is None, send_message(embed=embed) posts the info embed. After the user picks a rarity button, the embed stays visible as a stale message. Minor UX inconsistency — consider await interaction.edit_original_response(content="Fetching your cards...", embed=None) after question.delete(). Not a blocker.
SelectView not cleaned up on timeout. After followup.send(content=..., view=view), if the user ignores the select for 120s, the message is abandoned. Standard pattern in this codebase — not a blocker.

Verdict: APPROVED

Clean implementation that follows established project patterns throughout. The crafting tier chain, auth, response handling, and UI flow are all correct. The three suggestions above are non-blocking quality improvements for a follow-up.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/economy_new/crafting.py` (added) - `paperdynasty.py` (modified — cog registration) ### Findings #### Correctness No issues found. - All imports resolve: `db_get`, `team_hash`, `PD_PLAYERS`, `get_team_by_owner`, `get_card_embeds`, `get_channel`, `get_team_embed`, `Confirm`, `ButtonOptions`, `SelectView` all confirmed present in their respective modules. - Interaction response/followup path is correct in both branches: `send_message` + `followup.send` (rarity=None path) and `defer` + `followup.send` (rarity provided). No double-respond risk. - `CRAFT_TIER_UP` KeyError is not reachable — rarity is constrained to valid dict keys by both `app_commands.choices` and hardcoded `ButtonOptions` labels. HoF correctly excluded as a source. - `new_card = result.get("card", result)` handles the expected `{"card": {...}}` response shape correctly. Defensive fallback is fine. - `SelectCraftCards.callback` uses `edit_message(view=None)` correctly to remove the select after submission — prevents re-triggering. - `min_values=2, max_values=2` enforces exactly 2 cards at the Discord protocol level. #### Security No issues found. - `team_hash` auth matches the `sell/cards` and `buy/players` pattern exactly — no auth bypass possible. - No user-supplied strings interpolated into SQL or API paths. `card1_id` / `card2_id` are cast to `int` from select values, then formatted as integers in the query param. Safe. #### Style & Conventions No issues found. - Channel restriction check matches `marketplace.py` exactly. - `db_get` for a mutating endpoint is the established project pattern (same as buy/sell). - Module structure (`class SelectCraftCards`, `class Crafting`, `async def setup`) follows `marketplace.py` and `scouting.py` conventions. - Logger name `"discord_app"` is correct. #### Suggestions 1. **Craft API call uses default 5s timeout.** `sell/cards` uses `timeout=10`, buy/players uses `timeout=10`. Crafting involves validate + delete 2 cards + create 1 card — consider `timeout=10` or `timeout=15` to reduce false "didn't go through" failures under API load. ```python result = await db_get( f"teams/{self.owner_team['id']}/craft", params=[...], timeout=10, # add this ) ``` 2. **Stale info embed after rarity button click.** When `rarity is None`, `send_message(embed=embed)` posts the info embed. After the user picks a rarity button, the embed stays visible as a stale message. Minor UX inconsistency — consider `await interaction.edit_original_response(content="Fetching your cards...", embed=None)` after `question.delete()`. Not a blocker. 3. **SelectView not cleaned up on timeout.** After `followup.send(content=..., view=view)`, if the user ignores the select for 120s, the message is abandoned. Standard pattern in this codebase — not a blocker. ### Verdict: APPROVED Clean implementation that follows established project patterns throughout. The crafting tier chain, auth, response handling, and UI flow are all correct. The three suggestions above are non-blocking quality improvements for a follow-up. --- *Automated review by Claude PR Reviewer*

claude-reviewer added

ai-reviewed

and removed

ai-reviewing

labels 2026-04-12 03:18:57 +00:00

Ruff Lint / lint (pull_request) Successful in 38s

Details

This pull request is blocked because it's outdated.

This branch is out-of-date with the base branch

You are not authorized to merge this pull request.

View command line instructions.