fix: use exact rarity targeting for reserve/replacement in pack distribution (#20) #66

cal · 2026-03-05T15:06:27Z

cal commented

2026-03-05 15:06:27 +00:00

Summary

Root cause: The else branch of the reserve/replacement balancing check used max_rarity=1, which allows cards of rarity 0 (Replacement) or 1 (Reserve). When trying to specifically draw a Reserve card, this was incorrect — the API needs both min_rarity and max_rarity set to the same value to target an exact tier.
Fix: Replaced the max_rar = 1 / if ... max_rar = 0 pattern with rar = RARITY['Replacement'] if ... else RARITY['Reserve'], then passed ('min_rarity', rar), ('max_rarity', rar) to players/random in both the infielder and outfielder loops.
Also: Added RARITY to the import in cogs/economy_new/team_setup.py and removed the TODO comments.

Files changed

cogs/economy.py — fixed infielder and outfielder loops
cogs/economy_new/team_setup.py — added RARITY import, fixed infielder and outfielder loops

Test results

No test suite for this area. Changes verified by reading back all four modified loops.

Other observations

cogs/economy_new/team_setup.py was already reformatted by ruff before my edit (pre-existing, not caused by this PR).
Pitcher fetches (five_sps, five_rps) intentionally use max_rarity=1 as an initialization step before any balancing — these were not changed as they are outside the scope of the TODO.

## Summary - **Root cause**: The `else` branch of the reserve/replacement balancing check used `max_rarity=1`, which allows cards of rarity 0 (Replacement) *or* 1 (Reserve). When trying to specifically draw a Reserve card, this was incorrect — the API needs both `min_rarity` and `max_rarity` set to the same value to target an exact tier. - **Fix**: Replaced the `max_rar = 1` / `if ... max_rar = 0` pattern with `rar = RARITY['Replacement'] if ... else RARITY['Reserve']`, then passed `('min_rarity', rar), ('max_rarity', rar)` to `players/random` in both the infielder and outfielder loops. - **Also**: Added `RARITY` to the import in `cogs/economy_new/team_setup.py` and removed the TODO comments. ## Files changed - `cogs/economy.py` — fixed infielder and outfielder loops - `cogs/economy_new/team_setup.py` — added `RARITY` import, fixed infielder and outfielder loops ## Test results No test suite for this area. Changes verified by reading back all four modified loops. ## Other observations - `cogs/economy_new/team_setup.py` was already reformatted by ruff before my edit (pre-existing, not caused by this PR). - Pitcher fetches (`five_sps`, `five_rps`) intentionally use `max_rarity=1` as an initialization step before any balancing — these were not changed as they are outside the scope of the TODO.

cal added 1 commit 2026-03-05 15:06:28 +00:00

fix: use exact rarity targeting for reserve/replacement in pack distribution (#20 )

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

Details

7ec0fc835b

Replace max_rar=1 fallback (which allowed both Reserve and Replacement cards)
with min_rarity=rar, max_rarity=rar so each draw targets exactly the intended
rarity tier. Also removes the TODO comments.

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

cal referenced this pull request

2026-03-05 15:06:37 +00:00

Implement reserve vs. replacement card tracking in pack distribution #20

cal added the

ai-reviewing

label 2026-03-05 15:15:34 +00:00

cal reviewed 2026-03-05 15:17:38 +00:00

cal left a comment

AI Code Review

Files Reviewed

cogs/economy.py (modified — infielder + outfielder reserve/replacement loops)
cogs/economy_new/team_setup.py (modified — same loops + RARITY import added)

Findings

Correctness

Fix is correct. Old code: max_rarity=1 when targeting a Reserve card (rarity=1) silently allowed Replacement (0) OR Reserve (1) to be returned. New code uses min_rarity=rar, max_rarity=rar for exact tier targeting. RARITY values confirmed: Reserve=1, Replacement=0.
The condition roster_counts["Replacement"] < roster_counts["Reserve"] → draw Replacement, else draw Reserve matches the original TODO comment exactly.
none_okay=False preserved on all four db_get calls — errors still surface correctly.
Pitcher fetches (five_sps, five_rps) correctly left with max_rarity=1 only — these are initialization seeds, not balance draws. Out of scope for this fix.

Security

No issues found.

Style & Conventions

RARITY added to existing from helpers.constants import ... in team_setup.py — correct explicit import pattern.
In economy.py, RARITY is available via from helpers import * — no additional import needed. ✓
Ruff reformatting is pre-existing (noted in PR body, not caused by this change). No behavior change.
TODO comments removed — appropriate, the issue is resolved.

Suggestions

No blocking suggestions. The pitcher init blocks (five_sps/five_rps) also use max_rarity=1 without min_rarity, which means they can return Replacement-tier pitchers. That's a separate concern and correctly identified as out of scope here.

Verdict: COMMENT

Fix is correct and minimal in scope. Gitea blocks self-approval, posting as COMMENT.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/economy.py` (modified — infielder + outfielder reserve/replacement loops) - `cogs/economy_new/team_setup.py` (modified — same loops + RARITY import added) ### Findings #### Correctness - **Fix is correct.** Old code: `max_rarity=1` when targeting a Reserve card (rarity=1) silently allowed Replacement (0) OR Reserve (1) to be returned. New code uses `min_rarity=rar, max_rarity=rar` for exact tier targeting. `RARITY` values confirmed: `Reserve=1`, `Replacement=0`. - The condition `roster_counts["Replacement"] < roster_counts["Reserve"]` → draw Replacement, else draw Reserve matches the original TODO comment exactly. - `none_okay=False` preserved on all four `db_get` calls — errors still surface correctly. - Pitcher fetches (`five_sps`, `five_rps`) correctly left with `max_rarity=1` only — these are initialization seeds, not balance draws. Out of scope for this fix. #### Security - No issues found. #### Style & Conventions - `RARITY` added to existing `from helpers.constants import ...` in `team_setup.py` — correct explicit import pattern. - In `economy.py`, `RARITY` is available via `from helpers import *` — no additional import needed. ✓ - Ruff reformatting is pre-existing (noted in PR body, not caused by this change). No behavior change. - TODO comments removed — appropriate, the issue is resolved. #### Suggestions - No blocking suggestions. The pitcher init blocks (`five_sps`/`five_rps`) also use `max_rarity=1` without `min_rarity`, which means they can return Replacement-tier pitchers. That's a separate concern and correctly identified as out of scope here. ### Verdict: COMMENT Fix is correct and minimal in scope. Gitea blocks self-approval, posting as COMMENT. --- *Automated review by Claude PR Reviewer*

cal added

ai-reviewed

and removed

ai-reviewing

labels 2026-03-05 15:17:59 +00:00

cal changed target branch from main to next-release

2026-03-07 07:32:42 +00:00

cal force-pushed ai/paper-dynasty-discord-20 from 7ec0fc835b to 26b0e8395f

2026-03-07 07:33:28 +00:00

Compare

cal changed target branch from next-release to main

2026-03-23 04:14:28 +00:00