Island#getMembers() should not return banned players

[wellnesscookie]

Description

Describe the bug

I reported this on TeamChat addon issue tracker as well. Banned users were being sent teamchat messages, but they cannot be members. Technically I think banned users are done that way that they actually are considered as members, but some methods should ignore them.

Steps to reproduce the behavior

1. Have a team
2. Ban a user
3. Stream through keySet of members of that island with:
   getIslands().getIsland(player.getWorld(), player.getUniqueId()).getMembers().keySet().stream()
4. See that banned users are in there as well

Expected behavior

Filter banned users out, so some methods should return values as expected.

Or

Rework of how banned users are present

Environment

BentoBox Version (Mandatory)

[16:09:00] [Client thread/INFO]: [CHAT] Running PAPER 1.13.2.
[16:09:00] [Client thread/INFO]: [CHAT] BentoBox version: 1.9.2-1.13.2
[16:09:00] [Client thread/INFO]: [CHAT] Database: MYSQL
[16:09:00] [Client thread/INFO]: [CHAT] Loaded Game Worlds:
[16:09:00] [Client thread/INFO]: [CHAT] acidisland (acidisland): Overworld, Nether, End
[16:09:00] [Client thread/INFO]: [CHAT] skyblock (skyblock): Overworld, Nether, End
[16:09:00] [Client thread/INFO]: [CHAT] Loaded Addons:
[16:09:00] [Client thread/INFO]: [CHAT] AcidIsland 1.9.2 (ENABLED)
[16:09:00] [Client thread/INFO]: [CHAT] BentoBox-InvSwitcher 0.0.4 (ENABLED)
[16:09:00] [Client thread/INFO]: [CHAT] Biomes 1.6.0.1 (ENABLED)
[16:09:00] [Client thread/INFO]: [CHAT] BSkyBlock 1.9.0 (ENABLED)
[16:09:00] [Client thread/INFO]: [CHAT] Challenges 0.8.0-1.6.0 (ENABLED)
[16:09:00] [Client thread/INFO]: [CHAT] Chat 1.0.2-SNAPSHOT-LOCAL (ENABLED)
[16:09:00] [Client thread/INFO]: [CHAT] DimensionalTrees 1.6.0 (ENABLED)
[16:09:00] [Client thread/INFO]: [CHAT] IslandUpgrade 1.0 (ENABLED)
[16:09:00] [Client thread/INFO]: [CHAT] Level 1.9.0 (ENABLED)
[16:09:00] [Client thread/INFO]: [CHAT] Limits 1.9.0 (ENABLED)
[16:09:00] [Client thread/INFO]: [CHAT] SerbCraftAddon 2.5 (ENAB

Plugins (Optional)

No need

Additional context (Optional)

• Link to TeamChat issue: Banned are being sent teamchat messages Chat#12

[wellnesscookie]

Oh, the docs on the method clearly says:
It contains all players that have any rank on this island, including {@link RanksManager#BANNED_RANK BANNED}

But I will leave the closing up to Poslovitch and Tasty. 😕

[BONNe]

There exist 2 methods:
#getMembers() that returns all players with a status in the island and #getMemberSet that returns only members.
This is kinda misleading method name, and it will be used incorrectly as not all will go and read docs with methods that are so self-explanatory.

I think this should be considered again, as #627 was closed, and we run in the same issue again.

[Poslovitch]

I agree to keep this open. This will have to be done as part of a major API rework though.

[tastybento]

Edited

Unfortunately, "major API rework" can mean breaking dependent code and systems already running so if not done carefully can cause problems. If we add new API then they may work but eventually there’s another thing that will come up. That's what happens with public APIs - in a perfect world, we'd change the name but it is what it is.

We have clear javadocs and already and the getIslandMemberSet that gives what is required. In Chat, it was my fault that I used the wrong method. Although it’s be great to magically change things I don't see anything do to here that won't cause more problems than it will solve.

[BONNe]

@tastybento and @Poslovitch

I would suggest either close this issue, and leave everything as it is, or deprecate some methods.

Java has a deprecation feature, which is directly for this usage. We could deprecate #getMembers and create proper methods.
Deprecated code could be kept for 1-2 releases, and then removed.

[tastybento]

I'm open to a PR on this. Deprecate getMembers and replace with something else that has a better description. I think that particular method is likely not used that much.