Clamp rarity redistribution rates at zero #529

Open
opened 2026-09-15 12:26:13 +01:00 by Smithy-bot · 5 comments
Member

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)

  1. Leftover probability when a bucket hits 0: (a) Renormalize all five buckets so they sum to 100%.
  2. Boost under clamp: The boosted rarity stays at exactly 2× its base rate when clamping occurs.
  3. Scope: Same rule for Gold, Legendary, and Manga chance-up effects, for whatever rates are in CardRarityChances at implementation time.
  4. Documentation: Update docs/effects.md to 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.md
    THEN examples and edge-case wording match the code (no negative published rates unless explicitly describing pre-fix behaviour).

Subtasks

  • Implement clamp + renormalization in rarity redistribution (GetCardsHelper / related helpers)
  • Preserve 2× boosted base rate when clamping applies
  • Add/adjust unit tests for large boosts that previously produced negative buckets
  • Update docs/effects.md for the new edge-case behaviour

Notes

**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) 1. **Leftover probability when a bucket hits 0:** **(a)** Renormalize all five buckets so they sum to **100%**. 2. **Boost under clamp:** The boosted rarity stays at **exactly 2×** its base rate when clamping occurs. 3. **Scope:** Same rule for **Gold, Legendary, and Manga** chance-up effects, for whatever rates are in `CardRarityChances` at implementation time. 4. **Documentation:** Update `docs/effects.md` to 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 &lt; 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.md` **THEN** examples and edge-case wording match the code (no negative published rates unless explicitly describing pre-fix behaviour). ## Subtasks - [ ] Implement clamp + renormalization in rarity redistribution (`GetCardsHelper` / related helpers) - [ ] Preserve **2×** boosted base rate when clamping applies - [ ] Add/adjust unit tests for large boosts that previously produced negative buckets - [ ] Update `docs/effects.md` for the new edge-case behaviour ## Notes - Source: https://git.vylpes.xyz/External/card-drop/pulls/527#issuecomment-21271 - Planning Q&A: https://git.vylpes.xyz/External/card-drop/issues/529#issuecomment-23446
Author
Member

Planning — needs/criteria

Context: rarity chance-up effects in GetCardsHelper.GetRandomCard double the boosted rarity and subtract chanceToRemove / 4 from each of the other four buckets. With current base rates, Gold Chance Up can drive Legendary below zero (documented in docs/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:

  1. Leftover reduction when a bucket hits 0 — If subtracting chancePerOther would take a rarity below 0, we clamp that bucket to 0. What should happen to any reduction that could not be applied?

    • (a) Renormalize all five buckets so they sum to 100%
    • (b) Allow the total to be < 100% (wider implicit “miss” band in the roulette)
    • (c) Add unallocated mass back to the boosted rarity
    • (d) Redistribute the surplus across the other non-clamped buckets (equal split or proportional)
  2. 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?

  3. Scope — Apply the same clamping rule to Gold, Legendary, and Manga chance-up effects, for whatever rates are in CardRarityChances at implementation time?

  4. Documentation — Should docs/effects.md be 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 GetCardsHelper
    THEN no rarity bucket is below 0 after redistribution.

  • GIVEN base rates in CardRarityChances
    WHEN 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.md
    THEN 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

## Planning — needs/criteria Context: rarity chance-up effects in `GetCardsHelper.GetRandomCard` double the boosted rarity and subtract `chanceToRemove / 4` from each of the other four buckets. With current base rates, **Gold Chance Up** can drive Legendary below zero (documented in `docs/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: 1. **Leftover reduction when a bucket hits 0** — If subtracting `chancePerOther` would take a rarity below 0, we clamp that bucket to 0. What should happen to any reduction that could not be applied? - **(a)** Renormalize all five buckets so they sum to **100%** - **(b)** Allow the total to be **&lt; 100%** (wider implicit “miss” band in the roulette) - **(c)** Add unallocated mass back to the **boosted** rarity - **(d)** Redistribute the surplus across the other **non-clamped** buckets (equal split or proportional) 2. **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? 3. **Scope** — Apply the same clamping rule to **Gold, Legendary, and Manga** chance-up effects, for whatever rates are in `CardRarityChances` at implementation time? 4. **Documentation** — Should `docs/effects.md` be 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 `GetCardsHelper` THEN no rarity bucket is below **0** after redistribution. - GIVEN base rates in `CardRarityChances` WHEN 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.md` THEN 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 removed their assignment 2026-10-13 12:08:58 +01:00
Owner

@Smithy-bot wrote in #529 (comment):

Planning — needs/criteria

Context: rarity chance-up effects in GetCardsHelper.GetRandomCard double the boosted rarity and subtract chanceToRemove / 4 from each of the other four buckets. With current base rates, Gold Chance Up can drive Legendary below zero (documented in docs/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:

1. **Leftover reduction when a bucket hits 0** — If subtracting `chancePerOther` would take a rarity below 0, we clamp that bucket to 0. What should happen to any reduction that could not be applied?
   
   * **(a)** Renormalize all five buckets so they sum to **100%**
   * **(b)** Allow the total to be **< 100%** (wider implicit “miss” band in the roulette)
   * **(c)** Add unallocated mass back to the **boosted** rarity
   * **(d)** Redistribute the surplus across the other **non-clamped** buckets (equal split or proportional)

2. **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?

3. **Scope** — Apply the same clamping rule to **Gold, Legendary, and Manga** chance-up effects, for whatever rates are in `CardRarityChances` at implementation time?

4. **Documentation** — Should `docs/effects.md` be 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 `GetCardsHelper`
  THEN no rarity bucket is below **0** after redistribution.

* GIVEN base rates in `CardRarityChances`
  WHEN 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.md`
  THEN 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

  1. a
  2. Keep at 2x
  3. Yes
  4. Yes
@Smithy-bot wrote in https://git.vylpes.xyz/External/card-drop/issues/529#issuecomment-23446: > ## [](#planning-needs-criteria)Planning — needs/criteria > > Context: rarity chance-up effects in `GetCardsHelper.GetRandomCard` double the boosted rarity and subtract `chanceToRemove / 4` from each of the other four buckets. With current base rates, **Gold Chance Up** can drive Legendary below zero (documented in `docs/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: > > 1. **Leftover reduction when a bucket hits 0** — If subtracting `chancePerOther` would take a rarity below 0, we clamp that bucket to 0. What should happen to any reduction that could not be applied? > > * **(a)** Renormalize all five buckets so they sum to **100%** > * **(b)** Allow the total to be **< 100%** (wider implicit “miss” band in the roulette) > * **(c)** Add unallocated mass back to the **boosted** rarity > * **(d)** Redistribute the surplus across the other **non-clamped** buckets (equal split or proportional) > > 2. **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? > > 3. **Scope** — Apply the same clamping rule to **Gold, Legendary, and Manga** chance-up effects, for whatever rates are in `CardRarityChances` at implementation time? > > 4. **Documentation** — Should `docs/effects.md` be 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)Draft criteria (pending your answers) > > * GIVEN a user has an active Gold, Legendary, or Manga chance-up effect > WHEN drop rates are calculated in `GetCardsHelper` > THEN no rarity bucket is below **0** after redistribution. > > * GIVEN base rates in `CardRarityChances` > WHEN 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.md` > THEN 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 1. a 2. Keep at 2x 3. Yes 4. Yes
Vylpes removed their assignment 2026-10-13 13:58:41 +01:00
Author
Member

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/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.
Author
Member

Planning — needs/estimate → needs/tests

Story point estimate for clamp + renormalization of rarity chance-up redistribution.

Acceptance criteria are specific enough to size. Breakdown:

Area Points
Implement clamp-to-zero + renormalize-to-100% in GetCardsHelper (and related helpers), preserving 2× boosted base rate when clamping applies 1
Unit tests for Gold, Legendary, and Manga chance-up paths that previously produced negative buckets; assert non-negative buckets, 2× boost, and 100% total 1
Update docs/effects.md examples and edge-case wording to match the new behaviour 1
Total 3

Focused 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/estimate → needs/tests Story point estimate for clamp + renormalization of rarity chance-up redistribution. Acceptance criteria are specific enough to size. Breakdown: | Area | Points | |------|--------| | Implement clamp-to-zero + renormalize-to-100% in `GetCardsHelper` (and related helpers), preserving **2×** boosted base rate when clamping applies | 1 | | Unit tests for Gold, Legendary, and Manga chance-up paths that previously produced negative buckets; assert non-negative buckets, 2× boost, and 100% total | 1 | | Update `docs/effects.md` examples and edge-case wording to match the new behaviour | 1 | | **Total** | **3** | Focused 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.
Author
Member

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

  • Familiarity with CardRarityChances base rates and Gold / Legendary / Manga chance-up effects.
  • Ability to run the project test suite (yarn test) and read docs/effects.md.
  • Optional (manual): stage bot or test harness that exercises GetCardsHelper.GetRandomCard (or equivalent) with an active chance-up effect.

AC1 — No rarity bucket below zero after redistribution

ID Steps Expected
T1.1 Run unit tests covering rarity redistribution with Gold, Legendary, and Manga chance-up active All pass
T1.2 For each effect type, inspect test assertions or debug output for final bucket rates Every bucket rate ≥ 0
T1.3 Use a scenario known to have produced negative buckets pre-fix (e.g. Gold Chance Up vs Legendary in current CardRarityChances) No negative values in computed rates
T1.4 (Manual, if available) Trigger drops with each chance-up effect active over many rolls Observed rarity distribution never implies a negative bucket (sanity spot-check)

AC2 — Boosted rarity stays exactly 2× base rate

ID Steps Expected
T2.1 Unit test: active Gold chance-up Gold bucket = 2 × Gold entry in CardRarityChances (before renormalization step, per implementation contract)
T2.2 Unit test: active Legendary chance-up Legendary bucket = 2 × base Legendary rate
T2.3 Unit test: active Manga chance-up Manga bucket = 2 × base Manga rate
T2.4 Unit test: large boost where clamping applies on other buckets Boosted rarity still exactly 2× base; clamping/renormalization does not shrink the boosted bucket below 2×

AC3 — Clamp to 0 and renormalize to 100%

ID Steps Expected
T3.1 Unit test: configuration where raw subtraction would push a non-boosted bucket below zero That bucket(s) clamped to 0
T3.2 Same scenario Sum of all five rarity rates = 100% (within floating-point tolerance used in codebase)
T3.3 Scenario with no clamping needed Rates still sum to 100%; behaviour matches prior expectations except non-negative buckets
T3.4 Confirm no implicit “miss” band from totals < 100% after redistribution Total always renormalized to 100% when clamping occurred

AC4 — Unit tests for formerly negative-bucket cases

ID Steps Expected
T4.1 yarn test (or targeted test file for GetCardsHelper / rarity redistribution) Green
T4.2 Tests explicitly cover large boosts that previously produced negative buckets Assertions: non-negative buckets, 2× boosted rate, 100% total after renormalization
T4.3 CI / Forgejo Actions test job on PR Passes

AC5 — docs/effects.md matches implementation

ID Steps Expected
T5.1 Read docs/effects.md sections for Gold, Legendary, and Manga chance-up redistribution Describes clamp-at-zero and renormalize to 100% rule
T5.2 Compare doc examples to unit-test outputs for the same base rates Published rates are non-negative; no “effectively 0%” wording that contradicts renormalization
T5.3 Confirm edge-case examples updated (replacing negative-bucket / old wording) Maintainer can reconcile doc math with code without ambiguity

Regression / scope

ID Steps Expected
R1 Drop flow without any chance-up effect Unchanged rarity behaviour vs pre-change baseline
R2 Effects outside Gold / Legendary / Manga chance-up Unchanged unless explicitly in scope

Sign-off checklist

  • T1–T5 and R1 pass; T4.3 green in CI.
  • Docs and code agree on clamp + renormalize + 2× boost.

Moving to needs/approval for your review before implementation. @Vylpes

## 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 - Familiarity with `CardRarityChances` base rates and Gold / Legendary / Manga chance-up effects. - Ability to run the project test suite (`yarn test`) and read `docs/effects.md`. - Optional (manual): stage bot or test harness that exercises `GetCardsHelper.GetRandomCard` (or equivalent) with an active chance-up effect. --- ### AC1 — No rarity bucket below zero after redistribution | ID | Steps | Expected | |----|--------|----------| | T1.1 | Run unit tests covering rarity redistribution with Gold, Legendary, and Manga chance-up active | All pass | | T1.2 | For each effect type, inspect test assertions or debug output for final bucket rates | Every bucket rate ≥ **0** | | T1.3 | Use a scenario known to have produced negative buckets pre-fix (e.g. Gold Chance Up vs Legendary in current `CardRarityChances`) | No negative values in computed rates | | T1.4 | (Manual, if available) Trigger drops with each chance-up effect active over many rolls | Observed rarity distribution never implies a negative bucket (sanity spot-check) | --- ### AC2 — Boosted rarity stays exactly 2× base rate | ID | Steps | Expected | |----|--------|----------| | T2.1 | Unit test: active Gold chance-up | Gold bucket = **2 ×** Gold entry in `CardRarityChances` (before renormalization step, per implementation contract) | | T2.2 | Unit test: active Legendary chance-up | Legendary bucket = **2 ×** base Legendary rate | | T2.3 | Unit test: active Manga chance-up | Manga bucket = **2 ×** base Manga rate | | T2.4 | Unit test: large boost where clamping applies on other buckets | Boosted rarity still **exactly 2×** base; clamping/renormalization does not shrink the boosted bucket below 2× | --- ### AC3 — Clamp to 0 and renormalize to 100% | ID | Steps | Expected | |----|--------|----------| | T3.1 | Unit test: configuration where raw subtraction would push a non-boosted bucket below zero | That bucket(s) clamped to **0** | | T3.2 | Same scenario | Sum of all five rarity rates = **100%** (within floating-point tolerance used in codebase) | | T3.3 | Scenario with no clamping needed | Rates still sum to **100%**; behaviour matches prior expectations except non-negative buckets | | T3.4 | Confirm no implicit “miss” band from totals &lt; 100% after redistribution | Total always renormalized to 100% when clamping occurred | --- ### AC4 — Unit tests for formerly negative-bucket cases | ID | Steps | Expected | |----|--------|----------| | T4.1 | `yarn test` (or targeted test file for `GetCardsHelper` / rarity redistribution) | Green | | T4.2 | Tests explicitly cover large boosts that previously produced negative buckets | Assertions: non-negative buckets, **2×** boosted rate, **100%** total after renormalization | | T4.3 | CI / Forgejo Actions test job on PR | Passes | --- ### AC5 — `docs/effects.md` matches implementation | ID | Steps | Expected | |----|--------|----------| | T5.1 | Read `docs/effects.md` sections for Gold, Legendary, and Manga chance-up redistribution | Describes clamp-at-zero and **renormalize to 100%** rule | | T5.2 | Compare doc examples to unit-test outputs for the same base rates | Published rates are **non-negative**; no “effectively 0%” wording that contradicts renormalization | | T5.3 | Confirm edge-case examples updated (replacing negative-bucket / old wording) | Maintainer can reconcile doc math with code without ambiguity | --- ### Regression / scope | ID | Steps | Expected | |----|--------|----------| | R1 | Drop flow without any chance-up effect | Unchanged rarity behaviour vs pre-change baseline | | R2 | Effects outside Gold / Legendary / Manga chance-up | Unchanged unless explicitly in scope | --- ### Sign-off checklist - [ ] T1–T5 and R1 pass; T4.3 green in CI. - [ ] Docs and code agree on clamp + renormalize + 2× boost. Moving to **needs/approval** for your review before implementation. @Vylpes
Smithy-bot removed their assignment 2026-10-16 12:04:30 +01:00
Vylpes removed their assignment 2026-10-16 15:29:19 +01:00
Sign in to join this conversation.
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
External/card-drop#529
No description provided.