Clamp rarity redistribution rates at zero #529
Labels
No labels
blocked
duplicate
high priority
needs
approval
needs
criteria
needs
estimate
needs
tests
question
step
doing
step
review
step
testing
step
todo
step
uat
type
admin
type
alert
type
bug
type
change
type
defect
type
dependencies
type
development
type/documentation
type
epic
type
idea
type
incident
type
investigation
type
spike
type
story
type
tests
wontfix
No milestone
No project
No assignees
2 participants
Notifications
Due date
No due date set.
Dependencies
No dependencies set
Reference
External/card-drop#529
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Story points: 3
Spun out of the non-blocking review note on PR #527.
Why
Rarity chance-up redistribution doubles the active rarity rate and spreads the delta across the other four rarities. For large boosts, that can drive other rarity buckets below zero (already noted in
docs/effects.md). Clamping at 0 and handling leftover probability explicitly makes the math safer and easier to reason about.Product decisions (2026-10-13)
CardRarityChancesat implementation time.docs/effects.mdto show non-negative final rates and describe the renormalization rule (replace the current negative-bucket / “effectively 0%” wording for those examples).Acceptance criteria
GIVEN a user has an active Gold, Legendary, or Manga chance-up effect
WHEN drop rates are calculated in
GetCardsHelper(e.g.GetRandomCard)THEN no rarity bucket is below 0 after redistribution.
GIVEN a chance-up effect is active
WHEN redistribution runs
THEN the boosted rarity’s rate is exactly 2× its base rate in
CardRarityChances, including when clamping would otherwise be required on other buckets.GIVEN one or more non-boosted buckets would go below zero under the raw subtraction step
WHEN those buckets are clamped to 0
THEN all five buckets are renormalized so their rates sum to 100% (no implicit “miss” band from totals < 100%).
GIVEN base rates and boosts that previously produced negative buckets (e.g. Gold Chance Up vs Legendary in current
CardRarityChances)WHEN unit tests run
THEN they assert non-negative buckets, 2× boosted rate, and a 100% total after renormalization.
GIVEN the behaviour is implemented
WHEN a maintainer reads
docs/effects.mdTHEN examples and edge-case wording match the code (no negative published rates unless explicitly describing pre-fix behaviour).
Subtasks
GetCardsHelper/ related helpers)docs/effects.mdfor the new edge-case behaviourNotes
Planning — needs/criteria
Context: rarity chance-up effects in
GetCardsHelper.GetRandomCarddouble the boosted rarity and subtractchanceToRemove / 4from each of the other four buckets. With current base rates, Gold Chance Up can drive Legendary below zero (documented indocs/effects.md). Today negative widths effectively shrink that slice of the 0–100 roulette rather than storing explicit zero floors.Before acceptance criteria can be finalized, I need decisions on clamping and leftover probability:
Leftover reduction when a bucket hits 0 — If subtracting
chancePerOtherwould take a rarity below 0, we clamp that bucket to 0. What should happen to any reduction that could not be applied?Boost strength under clamp — Must the boosted rarity still be exactly 2× its base rate when clamping occurs, or is a slightly smaller effective boost acceptable so the math stays non-negative without renormalization?
Scope — Apply the same clamping rule to Gold, Legendary, and Manga chance-up effects, for whatever rates are in
CardRarityChancesat implementation time?Documentation — Should
docs/effects.mdbe updated to show non-negative final rates only and describe the chosen leftover rule (replacing the current “effectively 0% or minimum” note for negative examples)?Draft criteria (pending your answers)
GIVEN a user has an active Gold, Legendary, or Manga chance-up effect
WHEN drop rates are calculated in
GetCardsHelperTHEN no rarity bucket is below 0 after redistribution.
GIVEN base rates in
CardRarityChancesWHEN a boost would have produced a negative bucket under today’s algorithm
THEN the implemented leftover rule (from Q1–Q2) is applied consistently and covered by unit tests.
GIVEN the behaviour is implemented
WHEN a maintainer reads
docs/effects.mdTHEN examples and edge-case wording match the code (no negative published rates unless explicitly describing pre-fix behaviour).
Please reply on this issue. Once decided I will lock the acceptance criteria in the description and move to needs/estimate.
@Vylpes
@Smithy-bot wrote in #529 (comment):
Planning — needs/criteria → needs/estimate
Thanks @Vylpes — recorded your answers (renormalize to 100%, keep boosted at 2×, all three chance-up rarities, update
docs/effects.md).Acceptance criteria and subtasks are in the issue description. Removed needs/criteria, added needs/estimate. Smithy-bot will pick up story-point sizing on the next planning run.
Planning — needs/estimate → needs/tests
Story point estimate for clamp + renormalization of rarity chance-up redistribution.
Acceptance criteria are specific enough to size. Breakdown:
GetCardsHelper(and related helpers), preserving 2× boosted base rate when clamping appliesdocs/effects.mdexamples and edge-case wording to match the new behaviourFocused change in one drop-rate code path (no new surfaces or API). Comparable to other small card-drop logic fixes; less than multi-job workflow refactors.
Setting Story Points: 3 on the issue body. Moving to needs/tests for QA scripts on the next planning pass.
Planning — needs/tests → needs/approval
QA scripts mapped to the acceptance criteria in the issue body. Run on a branch with the implementation merged or deployed to stage; unit-test rows can run locally via
yarn test.Preconditions
CardRarityChancesbase rates and Gold / Legendary / Manga chance-up effects.yarn test) and readdocs/effects.md.GetCardsHelper.GetRandomCard(or equivalent) with an active chance-up effect.AC1 — No rarity bucket below zero after redistribution
CardRarityChances)AC2 — Boosted rarity stays exactly 2× base rate
CardRarityChances(before renormalization step, per implementation contract)AC3 — Clamp to 0 and renormalize to 100%
AC4 — Unit tests for formerly negative-bucket cases
yarn test(or targeted test file forGetCardsHelper/ rarity redistribution)AC5 —
docs/effects.mdmatches implementationdocs/effects.mdsections for Gold, Legendary, and Manga chance-up redistributionRegression / scope
Sign-off checklist
Moving to needs/approval for your review before implementation. @Vylpes