fix: make donation limit handling consistent across all paths#449
Merged
Conversation
Follow-ups from the 2.28.0 release review of the donation limit feature: - BlockConfig: store blockLimits keys lowercased and lowercase String lookups in getLimit(), so the level calculation (which lowercases donation keys) and the donation command/panel paths (which pass custom IDs as-is) always resolve to the same limit, including for mixed-case custom block IDs. - Extract Utils.calculateDonatedPoints() as the single source of truth for donated points (current values, world overrides, limit capping); used by both tidyUp() and the donation panel info pane, so the "Currently donated" figure now matches what the level actually uses instead of the stored donation-time total. - DonationPanel: carry the resolved blockObj from the slot walk instead of re-deriving it from the donation ID via Material.matchMaterial(), which could mis-resolve colon-less custom IDs. - getReport(): cap donated lines at the current block limit so they sum to the reported donated total, and mark capped entries. - New island.donate.limit-reached-all locale key (all 17 locales): shown when everything offered is already at its limit, instead of a misleading "empty" message (panel) or a 0-point confirm prompt (inv). - Tests: cover the new paths; stub getLimit to return null in setups because Mockito returns 0 for unstubbed Integer methods, which reads as "limit of 0" and silently exercised the wrong path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Y6f59pxCkwS8QXtJSe1nq6
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-ups from the 2.28.0 release review of the donation limit feature. Addresses review issues 1–5.
Changes
blockLimitskeys are now stored lowercased andgetLimit(String)lowercases its lookup, so the level calculation (which lowercases donation keys) and the donation command/panel paths (which pass custom IDs as-is) always resolve to the same limit — including for mixed-case custom block IDs (e.g. Oraxen item IDs).Utils.calculateDonatedPoints()(current values, world overrides, limit capping) used by bothtidyUp()and the donation panel's "Currently donated" info pane, so the GUI figure now matches what the level actually uses instead of the stored donation-time total.DonationPanel.computeDonationTotalscarries the resolvedblockObjfrom the slot walk instead of re-deriving it viadonationId.contains(":") ? … : Material.matchMaterial(…), which could mis-resolve colon-less custom IDs.getReport()caps donated lines at the current block limit and marks capped entries, matching the scanned-blocks section.island.donate.limit-reached-allkey (translated in all 17 locales). The panel confirm button and/island donate invnow send it instead of a misleading "empty" message or a 0-point confirmation prompt.Test note
Mockito returns 0 (not null) for unstubbed
Integer-returning methods — an unstubbedgetLimit()mock reads as "limit of 0". The pre-existingtestExecuteInvShowsConfirmationPromptwas silently exercising the everything-at-limit path with a 0-point prompt because of this. Test setups now stubgetLimit(any())to return null (matching the realBlockConfigwhen no limit is configured), asIslandLevelCalculatorTidyUpTestalready did.New tests:
BlockConfigLimitsTest(case-insensitivity contract),UtilsCalculateDonatedPointsTest(value/limit/cap math), plus an all-at-limit/island donate invcommand test.Full suite: 237 tests, 0 failures.
🤖 Generated with Claude Code
https://claude.ai/code/session_01Y6f59pxCkwS8QXtJSe1nq6