Skip to content

Limit activity reward multipliers to whitelisted pools - #76

Merged
XxFran10xX merged 1 commit into
mainfrom
fix/material-pool-multiplier
Sep 28, 2026
Merged

XxFran10xX merged 1 commit into
mainfrom
fix/material-pool-multiplier

Conversation

@XxFran10xX

@XxFran10xX XxFran10xX commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Weekly activity multipliers currently add draws to every pool, so multiplier 2 also awards two skin scrolls at drop_3. Add rewards.multiplier-pools, an explicit case-insensitive whitelist for extra weekly pool draws. Configure [pool_prologue] to boost materials while skin pools draw once.

Missing, empty, or malformed whitelists enable no extra pool draws. Daily pool rewards and fixed-item amount behavior remain unchanged. The default configuration documents migration and reload behavior.

Validation: regression coverage for the live three-milestone setup at multiplier 2, excluded default/scroll pools at multiplier 64, whitelist parsing, and existing payout failure handling. Validation completed:

  • Local Java 21 mvn clean verify: 773 tests passed, no failures/errors/skips.
  • PR CI: Maven verification and runtime JAR validation passed.
  • Exact CI artifact activity-DEV-20260928-1652.jar installed on TFMCDev01 with multiplier-pools: [pool_prologue]; Paper startup completed, ActivityTF enabled all integrations, version activity confirmed the build, and activity reload completed successfully.
  • Diff reviewed for pool selection, excluded scroll payouts, default-pool fallback, and partial payout accounting.

Deployment: routine main push after release, retaining multiplier: 1 and whitelisting only pool_prologue. Jar and config take effect at the next main restart. Documentation: TF-Minecraft/Docs#71.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: aea46ddc-ff2e-4ebb-a05b-6ce2f8bf64e1

📥 Commits

Reviewing files that changed from the base of the PR and between 583b743 and 86ca0a7.

📒 Files selected for processing (5)
  • src/main/java/net/tfminecraft/activitytf/config/ActivityConfiguration.java
  • src/main/java/net/tfminecraft/activitytf/managers/ActivityManager.java
  • src/main/resources/config.yml
  • src/test/java/net/tfminecraft/activitytf/config/ActivityConfigurationRewardsTest.java
  • src/test/java/net/tfminecraft/activitytf/managers/ActivityManagerRewardItemsTest.java

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Reward configuration now supports a pool-name whitelist for extra milestone reward draws. Milestone payouts pass this whitelist to reward selection. Fixed rewards retain their amount multiplier.

Changes

Reward Pool Multipliers

Layer / File(s) Summary
Load and validate the pool whitelist
src/main/java/net/tfminecraft/activitytf/config/ActivityConfiguration.java, src/main/resources/config.yml, src/test/java/net/tfminecraft/activitytf/config/ActivityConfigurationRewardsTest.java
Configuration loading normalizes valid pool names, deduplicates them, and ignores invalid entries. Missing, empty, or malformed values produce an empty set. Comments and tests describe and check the configuration behavior.
Apply the whitelist to milestone payouts
src/main/java/net/tfminecraft/activitytf/managers/ActivityManager.java, src/test/java/net/tfminecraft/activitytf/managers/ActivityManagerRewardItemsTest.java
Milestone payout calls pass the configured whitelist to reward selection. Listed pools receive multiplier-based draws; unlisted pools draw once. Fixed rewards continue to use the multiplier for their paid amount.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant ActivityConfiguration
  participant ActivityManager
  participant rewardFor
  ActivityConfiguration->>ActivityManager: provide multiplierPools()
  ActivityManager->>rewardFor: pass multiplierPools with reward and multiplier
  rewardFor->>rewardFor: draw multiplier times for listed pools
  rewardFor->>rewardFor: draw once for unlisted pools
Loading

Suggested reviewers: ryanbarlow97

Merge Risk: ⚪ Minimal · up to 86ca0

No actionable merge-blocking issue is established; complete the pending Maven and dev-server checks before merging.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 86ca0

The new whitelist limits extra weekly pool draws to configured pools. No new player-controlled way to increase payouts was identified. Existing payout-recovery limitations remain, and deployment behavior has not been validated.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The changed policy affects rewards dispatched for weekly claims on a server using this configuration. It does not give a claimant authority to select additional pools; configured entries can still dispatch their existing item and command rewards.

Trust Boundaries and Controls

  • observed — The whitelist is loaded from server configuration, normalized, and stored as an immutable set. The player claim path consumes that policy; the production command path for changing it requires administrative permission.

Resilience and Maintainability Implications

  • observed — Claimed-points is persisted before external rewards run. Existing partial-payout and interruption paths cannot guarantee replay of every owed spin; this PR does not change their persistence or recovery mechanism.

Hardening Proposals

  • proposed — If recovery of every individual reward becomes a required guarantee, consider durable per-spin progress or a reconciliation mechanism for interrupted and partially paid claims. This addresses an existing limitation, not an observed regression from the whitelist.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 4 files. (1 skipped: 1… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: restricting activity reward multipliers to explicitly whitelisted pools.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 23 functions across 4 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

A rabbit checks the pool-name set,
Then counts the spins for each reward.
Listed pools get extra draws,
Unlisted pools draw once, no more.
Fixed rewards keep their amount,
The rabbit hops away, content.

Comment @coderabbitai help to get the list of available commands.

@XxFran10xX
XxFran10xX merged commit 3fa0014 into main Sep 28, 2026
2 checks passed
@XxFran10xX
XxFran10xX deleted the fix/material-pool-multiplier branch September 28, 2026 16:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant