Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi island api update #2224

Merged
merged 7 commits into from
Nov 12, 2023
Merged

Multi island api update #2224

merged 7 commits into from
Nov 12, 2023

Conversation

tastybento
Copy link
Member

Introduction
This PR removes/deprecates number of API calls because they are ambiguous when the player can have more than one island in the world. Instead, developers will need to either get the current primary island, or whichever one they required to check specifically.

Deprecated Methods

IslandCache:

  • public Set<UUID> getMembers(@NonNull World world, @NonNull UUID uuid, int minimumRank)
  • public UUID getOwner(@NonNull World world, @NonNull UUID uuid)

IslandsManager:

  • public Set<UUID> getMembers(@NonNull World world, @NonNull UUID playerUUID, int minimumRank)
  • public Set<UUID> getMembers(@NonNull World world, @NonNull UUID playerUUID)
  • public Location getSafeHomeLocation(@NonNull World world, @NonNull User user, String name)
  • public UUID getOwner(@NonNull World world, @NonNull UUID playerUUID)
  • public boolean isOwner(@NonNull World world, @NonNull UUID uniqueId)

The methods have been marked as Deprecated, but I recommend they are removed before we release 2.0.0, so they are really just left there so that if any Addons use them they can be sure to change their code. This usually involves getting the primary active island of the player, and checking the member set of the island directly.

As a result, a number of commands had to be rewritten to consider the context of the player making the call. For example, setOwner should run for the island the player is on currently.

Admin Team Commands Will Need Rewriting
Where it gets tricky is with the admin team commands. These were all written with the assumption that the player has just one island. These command enable admins to add to/ remove from /disband teams. For now, they have been adjusted to apply to the user's current island. So, an admin will need to have to player on the island they want to edit. This is not great, and indeed makes it difficult to edit teams if the owner is not online. So, we need to come up with a way to handle admin team editing when there are multiple islands. For now, it's probably going to be "okay" and I'm not sure how often this commands are used anyway.

There was some code I removed around the concept of automatic transfer of island ownership to a player if the owner has not logged on for a long time. This was not actually ever finished and didn't do anything. With multi-islands, this is even more difficult to do, so for now, I removed it.

@tastybento tastybento requested a review from BONNe November 12, 2023 01:41
@BONNe
Copy link
Member

BONNe commented Nov 12, 2023

So in general, it means that all addons should change method to detect player island?

@tastybento
Copy link
Member Author

Yes, however, I have done a search of all the addons (well most of them anyway) and AFAIK these methods are not used by anyone right now.

Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 7 Code Smells

62.3% 62.3% Coverage
1.2% 1.2% Duplication

@tastybento tastybento merged commit 63d092d into develop Nov 12, 2023
3 checks passed
@tastybento tastybento deleted the multi_island_api_update branch January 21, 2024 19:00
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.

2 participants