fix: don't count transferred island against limit in setowner (#2996)#2997
Merged
Conversation
/is team setowner refused ownership transfer with the "maximum number of islands" error when the concurrent-island limit was reached, even though the target is already a member of the island being transferred and is not gaining a new island. getNumberOfConcurrentIslands counts every island the player owns or is a member of, so the island being transferred is already included in the target's total. The cap check now discounts it: in IslandTeamSetownerCommand the target is always a member (enforced earlier), and in AdminTeamSetownerCommand it is discounted only when the target is already a member. The transfer is rejected only when the target has max OTHER islands, preserving the limit added in #2908. Adds a regression test for a member whose only island is the one being transferred at a limit of 1, and updates the existing cap test to reflect that the transferred island is discounted. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
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.
Fixes #2996
Problem
/is team setowner <player>refused the transfer with "The specified player has reached the maximum number of islands allowed on the server." when the concurrent-island limit was reached — even though the target is already a member of the island being transferred and gains no new island. With a limit of 1, a member whose only island is this one always hit the cap.This blocks mobile players, who have no left/right-click and cannot use the
/is teamGUI, sosetowneris their only way to transfer ownership.Cause
IslandsManager#getNumberOfConcurrentIslandscounts every island the player owns or is a member of. SinceIslandTeamSetownerCommandrequires the target to already be a team member, the island being transferred is already included innum, sonum >= maxfires one island too early.Fix
Discount the island being transferred from the count before the cap check:
IslandTeamSetownerCommand— the target is always a member at this point (enforced earlier), sonumis decremented unconditionally.AdminTeamSetownerCommand— the target may not be a member (admin stands on any island), so it is decremented only whenisland.inTeam(targetUUID).The transfer is now rejected only when the target has
maxother islands, preserving the limit added in #2908.Tests
testCanExecuteTargetIsMemberOfOnlyThisIslandAtLimit— a member whose only island is the one being transferred, at a limit of 1, can now receive ownership. (Fails before the fix, passes after.)testCanExecuteTargetAtConcurrentIslandsCapso the target ownsmaxother islands plus this one, confirming the cap still blocks genuine over-limit transfers.🤖 Generated with Claude Code