/<gamemode> cmd generates island even if the create permission is unset

[wellnesscookie]

Description

Describe the bug

The [gamemode].island.create permission does not prevents /ai from generating an island, instead it prevents only /ai create (Which makes this permission completely senseless)

This is probably related to all of the gamemodes and my issue should have been placed in each gamemode repository, but I find it more comforting to post it here rather than opening an issue per each gamemode section.

Steps to reproduce the behavior

1. Unset acidisland.island.create permission
2. Type /ai create
3. Permission is required - works as intended
4. Type /ai
5. An island starts to generate - not intended (At least not what I would expect)

Expected behavior

That actually makes no sense. I'd rather consider the island.create permission to block any kind of a generation started by a player that hasn't got this permission (even if the generation comes from /ai, and not /ai create)

Environment

Server

• OS: Debian GNU/Linux 9
• Java version:
  Java(TM) SE Runtime Environment 18.3 (build 10.0.1+10)
  Java HotSpot(TM) 64-Bit Server VM 18.3 (build 10.0.1+10, mixed mode)

Plugins

[06:32:17 INFO]: Plugins (50): AnimatedNames*, AreaShop, ArmorStandTools, BentoBox, BungeeTabListPlus*, ChatControl, ChestCommands, ChestShop*, ClearLag*, CrateReloaded, EditableSign, Essentials, EssentialsChat*, EssentialsSpawn*, ExecuteEverywhere*, FastAsyncWorldEdit*, FeatherBoard*, HeadDatabase*, HolographicDisplays, HungerKeeperPlus*, IPWhitelist*, IslandBorder, LeaderHeads*, LibsDisguises, LuckPerms, MobManager*, Multiverse-Core*, MVdWPlaceholderAPI*, NickRemover*, NoSleepCMDs*, OpenInv*, PlayerHeads, PlayerPoints*, ProtocolLib*, PvPManager*, PvPManagerBossBar*, RedstoneClockDetector*, RPGHealthIndicator*, SilkSpawners, SkinsRestorer, Spartan, TradeMe, TrophyHeads*, UnbreakingAnvils*, Vault*, VoidGenerator, WorldEdit, WorldGuard, WorldGuardPistonFix*, Yamler*

BentoBox setup

BentoBox and Addons

[00:54:56] [main/INFO]: [CHAT] Running PAPER 1.13.2.
[00:54:56] [main/INFO]: [CHAT] BentoBox version: 1.5.1
[00:54:56] [main/INFO]: [CHAT] Loaded Game Worlds:
[00:54:56] [main/INFO]: [CHAT] acidisland (acidisland): Overworld, Nether, End
[00:54:56] [main/INFO]: [CHAT] skyblock (skyblock): Overworld, Nether, End
[00:54:56] [main/INFO]: [CHAT] Loaded Addons:
[00:54:56] [main/INFO]: [CHAT] AcidIsland 1.5.0 (ENABLED)
[00:54:56] [main/INFO]: [CHAT] BentoBox-InvSwitcher 0.0.3-SNAPSHOT (ENABLED)
[00:54:56] [main/INFO]: [CHAT] Biomes 1.5.0.0 (ENABLED)
[00:54:56] [main/INFO]: [CHAT] BSkyBlock 1.5.0 (ENABLED)
[00:54:56] [main/INFO]: [CHAT] Challenges 0.7.5 (ENABLED)
[00:54:56] [main/INFO]: [CHAT] Level 1.5.0 (ENABLED)
[00:54:56] [main/INFO]: [CHAT] Limits 0.2.0-SNAPSHOT (ENABLED)
[00:54:56] [main/INFO]: [CHAT] SerbCraftAddon 2.2 (ENABLED)
[00:54:56] [main/INFO]: [CHAT] WelcomeWarps 1.5.0 (ENABLED)

Configuration

• Database: mySQL

Additional context

Please move the issue if it really has to be placed in one of the gamemode addon repo trackers.
Sorry and thanks!

[tastybento]

@wellnesscookie I understand the report but what is it that you are actually trying to do? It sounds like you want to prevent players who don't have an island from being able to use /island? If so, then you can give them the -bskyblock.island permission. That'll block all island commands. Is there a situation where you want to give a player who does not have an island access to island commands, but not island create?

[wellnesscookie]

Yes, I'll give you an example:

I want to give individiual player a permission to generate the island and let him invite as many more players as he wants. This means that those players would have standard access to all of acid commands, except ones that would allow them to anyhow generate their own acid island.

I think that there is a walkthrough by blocking something here and there, but the bug report will stay unchanged since it really doesn't make any sense.

[Poslovitch]

I think it's because the main command might not call the "canExecute" method of each of the subcommand it tries to run.

[Poslovitch]

I was wondering if we could therefore add a call(...) method to the CompositeCommand that would run both canExecute() and execute(), so that it doesn't bloat the code that much.

[Poslovitch]

I'm moving it to the 1.5.3 milestone and reduced priority. @tastybento, I need your feedback on my previous comments.

[tastybento]

@Poslovitch Yes. As this affects BSkyBlock, I went ahead and wrote a proposal. Please check it out. I took your idea of doing a call method. This will check permissions as well as run the canExecute and Execute methods. Note that permission checking was being done in the CompositeCommand execute method.
I tested this all and it works. I also added extra test cases to BSkyBlock to cover this condition. It's now possible for players to have full access to island and island commands with the exception of not being able to make islands.

[Poslovitch]

Now that the API has been implemented into BentoBox, it's going to be the job of gamemode addons to use it. Closing this ticket.