Merge pull request #3094 from BentoBoxWorld/feature/settings-improvements-3092

Settings panel improvements (#3092)

Author: tastybento

src/main/java/world/bentobox/bentobox/api/panels/PanelItem.java

@@ -4,16 +4,20 @@
 import java.util.List;
 import java.util.Optional;
 
+import io.papermc.paper.datacomponent.DataComponentType;
 import io.papermc.paper.datacomponent.DataComponentTypes;
 import io.papermc.paper.datacomponent.item.TooltipDisplay;
 import net.kyori.adventure.text.Component;
 import net.kyori.adventure.text.format.TextDecoration;
 
+import org.bukkit.NamespacedKey;
+import org.bukkit.Registry;
 import org.bukkit.enchantments.Enchantment;
 import org.bukkit.event.inventory.ClickType;
 import org.bukkit.inventory.ItemFlag;
 import org.bukkit.inventory.ItemStack;
 import org.bukkit.inventory.meta.ItemMeta;
+import org.eclipse.jdt.annotation.Nullable;
 
 import world.bentobox.bentobox.api.panels.builders.PanelItemBuilder;
 import world.bentobox.bentobox.api.user.User;
@@ -269,7 +273,7 @@ private void hideAdditionalTooltip() {
         if (existing != null) {
             builder.hideTooltip(existing.hideTooltip());
             builder.addHiddenComponents(
-                    existing.hiddenComponents().toArray(io.papermc.paper.datacomponent.DataComponentType[]::new));
+                    existing.hiddenComponents().toArray(DataComponentType[]::new));
         }
         // Hide additional components that don't have non-deprecated ItemFlag equivalents
         builder.addHiddenComponents(
@@ -282,6 +286,26 @@ private void hideAdditionalTooltip() {
                 DataComponentTypes.BUNDLE_CONTENTS,
                 DataComponentTypes.MAP_DECORATIONS
         );
+        DataComponentType blockEntityData = blockEntityData();
+        if (blockEntityData != null) {
+            builder.addHiddenComponents(blockEntityData);
+        }
         icon.setData(DataComponentTypes.TOOLTIP_DISPLAY, builder.build());
     }
+
+    /**
+     * Hiding this component drops the "Interact with Spawn Egg: Sets Mob Type" lines that a
+     * spawner or trial spawner icon otherwise shows (#3092). Paper has no constant for it because
+     * it has no API value, so it is looked up in the registry.
+     * @return the {@code minecraft:block_entity_data} component type, or null if unavailable
+     */
+    @Nullable
+    private static DataComponentType blockEntityData() {
+        try {
+            return Registry.DATA_COMPONENT_TYPE.get(NamespacedKey.minecraft("block_entity_data"));
+        } catch (RuntimeException e) {
+            // Registry not available, e.g., in a test environment
+            return null;
+        }
+    }
 }

src/main/java/world/bentobox/bentobox/database/objects/Players.java

@@ -296,4 +296,22 @@ public void setLastLogin(Long lastLogin) {
         this.lastLogin = lastLogin;
     }
 
+    /**
+     * @return the display mode the player last chose in the settings panel, never
+     * {@link Flag.Mode#TOP_ROW}
+     * @since 3.23.2
+     */
+    public Flag.Mode getFlagsDisplayMode() {
+        return flagsDisplayMode == null || flagsDisplayMode == Flag.Mode.TOP_ROW ? Flag.Mode.BASIC
+                : flagsDisplayMode;
+    }
+
+    /**
+     * @param flagsDisplayMode the display mode to show the settings panel in
+     * @since 3.23.2
+     */
+    public void setFlagsDisplayMode(Flag.Mode flagsDisplayMode) {
+        this.flagsDisplayMode = flagsDisplayMode;
+    }
+
 }

src/main/java/world/bentobox/bentobox/listeners/flags/clicklisteners/CommandRankClickListener.java

@@ -150,25 +150,32 @@ private List<String> getCommands(World world, User user) {
         return plugin.getCommandsManager().getCommands().values().stream()
                 .filter(c -> c.getWorld() != null && c.getWorld().equals(world)) // Only allow commands in this world
                 .filter(c -> c.testPermission(user.getSender())) // Only allow them to see commands they have permission to see
-                .flatMap(c -> getCmdRecursively("/", c).stream())
+                .flatMap(c -> getCmdRecursively("/", c, user).stream())
                 .filter(label -> user.isOp() || !hiddenItems.contains(CommandCycleClick.COMMAND_RANK_PREFIX + label)) // Hide any hidden commands
                 .limit(49) // Silently limit to 49
                 .toList();
     }
 
     /**
-     * Recursively traverses the command tree looking for any configurable rank command and returns a string list of commands
+     * Recursively traverses the command tree looking for any configurable rank command and returns a string list of commands.
+     * Commands the user has no permission for are skipped, along with their sub-commands, because setting a rank for a
+     * command nobody on the island can run is pointless (#3092).
      * @param labels - preceding command's label list
      * @param cc - composite command
+     * @param user - the user viewing the panel
      * @return string list of commands
      */
-    private List<String> getCmdRecursively(String labels, CompositeCommand cc) {
+    private List<String> getCmdRecursively(String labels, CompositeCommand cc, User user) {
         List<String> result = new ArrayList<>();
+        if (!user.isOp() && cc.getPermission() != null && !cc.getPermission().isEmpty()
+                && !user.hasPermission(cc.getPermission())) {
+            return result;
+        }
         String newLabel = labels + cc.getName();
         if (cc.isConfigurableRankCommand()) {
             result.add(newLabel);
         }
-        cc.getSubCommands().values().forEach(s -> result.addAll(getCmdRecursively(newLabel + " ", s)));
+        cc.getSubCommands().values().forEach(s -> result.addAll(getCmdRecursively(newLabel + " ", s, user)));
         return result;
     }
 

src/main/java/world/bentobox/bentobox/managers/PlayersManager.java

@@ -24,6 +24,7 @@
 
 import world.bentobox.bentobox.BentoBox;
 import world.bentobox.bentobox.api.events.player.PlayerEvent;
+import world.bentobox.bentobox.api.flags.Flag;
 import world.bentobox.bentobox.api.user.User;
 import world.bentobox.bentobox.database.Database;
 import world.bentobox.bentobox.database.objects.Island;
@@ -349,6 +350,32 @@ public int getDeaths(World world, UUID playerUUID) {
         return getPlayer(playerUUID).getDeaths(Util.getWorld(world));
     }
 
+    /**
+     * Returns the display mode the player last chose in the settings panel, so it opens the
+     * way they left it.
+     * @param playerUUID - the player's UUID
+     * @return the basic, advanced or expert display mode
+     * @since 3.23.2
+     */
+    public Flag.Mode getFlagsDisplayMode(UUID playerUUID) {
+        return getPlayer(playerUUID).getFlagsDisplayMode();
+    }
+
+    /**
+     * Remembers the display mode the player chose in the settings panel. Saves only when it
+     * changes.
+     * @param playerUUID - the player's UUID
+     * @param mode - the basic, advanced or expert display mode
+     * @since 3.23.2
+     */
+    public void setFlagsDisplayMode(UUID playerUUID, Flag.Mode mode) {
+        Players p = getPlayer(playerUUID);
+        if (p.getFlagsDisplayMode() != mode) {
+            p.setFlagsDisplayMode(mode);
+            handler.saveObjectAsync(p);
+        }
+    }
+
     /**
      * Sets if a player is mid-teleport or not
      * @param uniqueId - unique ID

src/main/java/world/bentobox/bentobox/panels/customizable/SettingsPanel.java

@@ -172,7 +172,14 @@ public boolean isReadOnly() {
     @Nullable
     private final Island island;
     private final List<TabType> tabs;
-    private final Flag.Mode defaultMode;
+    /** The display mode the viewer chose; shared by every tab. */
+    private Flag.Mode selectedMode;
+    /** Whether a mode change is saved as the viewer's preference for next time. */
+    private boolean remembersMode;
+    /**
+     * Per-tab overrides of {@link #selectedMode}, set when a tab has nothing to show in the
+     * chosen mode and skips ahead.
+     */
     private final Map<TabType, Flag.Mode> modes = new EnumMap<>(TabType.class);
     private final Map<TabType, ItemTemplateRecord> tabTemplates = new EnumMap<>(TabType.class);
     /**
@@ -198,7 +205,7 @@ public boolean isReadOnly() {
      * @param world the world the settings are for
      * @param island the island, or null if the viewer is not on one
      * @param tabs the tabs to show, in order; the first the viewer may see is shown initially
-     * @param defaultMode the display mode each tab starts in
+     * @param defaultMode the display mode the panel starts in
      */
     protected SettingsPanel(@NonNull CompositeCommand command, @NonNull User user, @NonNull String templateName,
             @NonNull World world, @Nullable Island island, @NonNull List<TabType> tabs, Flag.Mode defaultMode) {
@@ -210,13 +217,14 @@ protected SettingsPanel(@NonNull CompositeCommand command, @NonNull User user, @
         this.tabs = tabs.stream()
                 .filter(t -> t.getPermission(prefix).isEmpty() || user.hasPermission(t.getPermission(prefix)))
                 .toList();
-        this.defaultMode = defaultMode;
+        this.selectedMode = defaultMode;
         this.activeTab = this.tabs.isEmpty() ? tabs.get(0) : this.tabs.get(0);
     }
 
     /**
      * Opens the settings panel for a player. With an island the protection and settings tabs are
-     * shown; without one, only the read-only view of the world's protection flags.
+     * shown; without one, only the read-only view of the world's protection flags. It opens in the
+     * display mode the player last chose.
      * @param command the island settings command
      * @param user the viewer
      * @param island the island, or null if the viewer is not on one
@@ -226,7 +234,10 @@ public static boolean openPanel(@NonNull CompositeCommand command, @NonNull User
         List<TabType> tabs = island == null ? List.of(TabType.WORLD_PROTECTION)
                 : List.of(TabType.PROTECTION, TabType.SETTING);
         World world = island == null ? command.getWorld() : island.getWorld();
-        return new SettingsPanel(command, user, SETTINGS_PANEL, world, island, tabs, Mode.BASIC).open();
+        SettingsPanel settingsPanel = new SettingsPanel(command, user, SETTINGS_PANEL, world, island, tabs,
+                command.getPlugin().getPlayers().getFlagsDisplayMode(user.getUniqueId()));
+        settingsPanel.remembersMode = true;
+        return settingsPanel.open();
     }
 
     /**
@@ -441,7 +452,7 @@ private List<Flag> getFlags(Flag.Mode mode) {
     }
 
     private Flag.Mode currentMode() {
-        return modes.getOrDefault(activeTab, defaultMode);
+        return modes.getOrDefault(activeTab, selectedMode);
     }
 
     private boolean isVisibleToUser(Flag flag) {
@@ -623,7 +634,12 @@ private PanelItem createModeButton(@NonNull ItemTemplateRecord template, Templat
                                 user.getTranslation(PROTECTION_PANEL + "mode." + nextKey + ".name")))
                 .clickHandler((p, u, clickType, s) -> {
                     if (clickAllowed(template, clickType)) {
-                        modes.put(activeTab, currentMode().getNext());
+                        // One mode for every tab, remembered for the next time the player opens it
+                        selectedMode = currentMode().getNext();
+                        modes.clear();
+                        if (remembersMode) {
+                            plugin.getPlayers().setFlagsDisplayMode(u.getUniqueId(), selectedMode);
+                        }
                         pageIndex = 0;
                         u.getPlayer().playSound(u.getLocation(), Sound.BLOCK_STONE_BUTTON_CLICK_OFF, 1F, 1F);
                     }

src/test/java/world/bentobox/bentobox/api/commands/island/IslandSettingsCommandTest.java

@@ -29,6 +29,7 @@
 import world.bentobox.bentobox.CommonTestSetup;
 import world.bentobox.bentobox.Settings;
 import world.bentobox.bentobox.api.commands.CompositeCommand;
+import world.bentobox.bentobox.api.flags.Flag.Mode;
 import world.bentobox.bentobox.api.flags.Flag.Type;
 import world.bentobox.bentobox.api.panels.Tab;
 import world.bentobox.bentobox.api.panels.TabbedPanel;
@@ -153,6 +154,7 @@ void testExecuteOpensTemplatedPanel() {
         when(user.getTranslationOrNothing(anyString())).thenReturn("");
         when(im.getIslandAt(any(Location.class))).thenReturn(Optional.of(island));
         when(island.getWorld()).thenReturn(world);
+        when(plugin.getPlayers().getFlagsDisplayMode(any())).thenReturn(Mode.BASIC);
         isc.canExecute(user, "settings", Collections.emptyList());
 
         assertTrue(isc.execute(user, "settings", Collections.emptyList()));

src/test/java/world/bentobox/bentobox/listeners/flags/clicklisteners/CommandRankClickListenerTest.java

@@ -12,6 +12,7 @@
 
 import java.util.Collections;
 import java.util.HashMap;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Optional;
 
@@ -197,6 +198,34 @@ void testOnClickTooManyCommands() {
                 "protection.panel.flag-item.command-instructions.");
     }
 
+    /**
+     * Sub-commands the viewer has no permission for are not listed, so their rank cannot be set (#3092).
+     * The listed commands are "/test" and "/test allowed", so slot 1 is "/test allowed".
+     */
+    @Test
+    void testOnClickHidesSubCommandsWithoutPermission() {
+        when(user.isOp()).thenReturn(false);
+        when(user.hasPermission("oneblock.island.border")).thenReturn(false);
+        CompositeCommand cc = cm.getCommands().get("test");
+        Map<String, CompositeCommand> subCommands = new LinkedHashMap<>();
+        subCommands.put("border", subCommand("border", "oneblock.island.border"));
+        subCommands.put("allowed", subCommand("allowed", "oneblock.island.allowed"));
+        when(cc.getSubCommands()).thenReturn(subCommands);
+
+        assertTrue(crcl.onClick(panel, user, ClickType.LEFT, 1));
+        verify(user).getTranslationOrNothing("protection.panel.flag-item.command-instructions.allowed");
+        verify(user, never()).getTranslationOrNothing("protection.panel.flag-item.command-instructions.border");
+    }
+
+    private CompositeCommand subCommand(String name, String permission) {
+        CompositeCommand sub = mock(CompositeCommand.class);
+        when(sub.isConfigurableRankCommand()).thenReturn(true);
+        when(sub.getName()).thenReturn(name);
+        when(sub.getPermission()).thenReturn(permission);
+        when(sub.getSubCommands()).thenReturn(Collections.emptyMap());
+        return sub;
+    }
+
     /**
      * Test method for {@link world.bentobox.bentobox.listeners.flags.clicklisteners.CommandRankClickListener#getPanelItem(java.lang.String, world.bentobox.bentobox.api.user.User, org.bukkit.World)}.
      */

src/test/java/world/bentobox/bentobox/managers/PlayersManagerTest.java

@@ -54,6 +54,7 @@
 import org.mockito.Mockito;
 
 import world.bentobox.bentobox.CommonTestSetup;
+import world.bentobox.bentobox.api.flags.Flag;
 import world.bentobox.bentobox.api.user.User;
 import world.bentobox.bentobox.database.AbstractDatabaseHandler;
 import world.bentobox.bentobox.database.Database;
@@ -687,6 +688,17 @@ void testSetLocale() {
         assertEquals("en-UK", pm.getLocale(uuid));
     }
 
+    /**
+     * Test method for
+     * {@link world.bentobox.bentobox.managers.PlayersManager#setFlagsDisplayMode(java.util.UUID, world.bentobox.bentobox.api.flags.Flag.Mode)}.
+     */
+    @Test
+    void testFlagsDisplayMode() {
+        assertEquals(Flag.Mode.BASIC, pm.getFlagsDisplayMode(uuid));
+        pm.setFlagsDisplayMode(uuid, Flag.Mode.EXPERT);
+        assertEquals(Flag.Mode.EXPERT, pm.getFlagsDisplayMode(uuid));
+    }
+
     /**
      * Test method for
      * {@link world.bentobox.bentobox.managers.PlayersManager#setPlayerName(world.bentobox.bentobox.api.user.User)}.

src/test/java/world/bentobox/bentobox/panels/customizable/SettingsPanelTest.java

@@ -118,6 +118,7 @@ public void setUp() throws Exception {
             return flags.stream().filter(f -> f.getID().equals(id)).findFirst();
         });
         when(iwm.getHiddenFlags(any())).thenReturn(hidden);
+        when(plugin.getPlayers().getFlagsDisplayMode(any())).thenReturn(Mode.BASIC);
 
         // Command
         when(command.getPlugin()).thenReturn(plugin);
@@ -330,15 +331,49 @@ void testModeCycling() {
     }
 
     /**
-     * The mode is remembered per tab.
+     * One mode is shared by every tab (#3092).
      */
     @Test
-    void testModePerTab() {
+    void testModeSharedAcrossTabs() {
         SettingsPanel sp = openWithIsland();
         click(items(sp), MODE, user, ClickType.LEFT);
         assertEquals(Mode.ADVANCED, sp.getMode());
         click(items(sp), SETTING_TAB, user, ClickType.LEFT);
+        assertEquals(Mode.ADVANCED, sp.getMode());
+    }
+
+    /**
+     * The panel opens in the mode the player last chose, and a change is saved for next time (#3092).
+     */
+    @Test
+    void testModeRemembered() {
+        when(plugin.getPlayers().getFlagsDisplayMode(uuid)).thenReturn(Mode.EXPERT);
+        SettingsPanel sp = openWithIsland();
+        assertEquals(Mode.EXPERT, sp.getMode());
+        assertEquals(Material.NETHER_BRICK, material(items(sp), MODE));
+
+        click(items(sp), MODE, user, ClickType.LEFT);
         assertEquals(Mode.BASIC, sp.getMode());
+        verify(plugin.getPlayers()).setFlagsDisplayMode(uuid, Mode.BASIC);
+    }
+
+    /**
+     * A tab with nothing to show in the chosen mode skips ahead for display only; the player's
+     * choice is neither changed nor saved.
+     */
+    @Test
+    void testSkippedModeNotSaved() {
+        when(iwm.getAddon(any())).thenReturn(Optional.of(gma));
+        flags.removeIf(f -> f.getID().equals("S_GAMMA"));
+        flags.add(flag("S_ADVANCED", Type.SETTING, Mode.ADVANCED));
+        SettingsPanel sp = openWithIsland();
+        click(items(sp), SETTING_TAB, user, ClickType.LEFT);
+        sp.refreshPanel();
+        assertEquals(Mode.ADVANCED, sp.getMode());
+        click(items(sp), PROTECTION_TAB, user, ClickType.LEFT);
+        sp.refreshPanel();
+        assertEquals(Mode.BASIC, sp.getMode());
+        verify(plugin.getPlayers(), never()).setFlagsDisplayMode(any(), any());
     }
 
     /**
@@ -493,6 +528,19 @@ void testUnknownPinnedFlagIsSkipped(@TempDir Path addonFolder) throws IOExceptio
     // Section: Admin panel
     // ---------------------------------------------------------------------
 
+    /**
+     * The admin panel always opens in expert mode and does not touch the admin's own preference.
+     */
+    @Test
+    void testAdminModeNotRemembered() {
+        SettingsPanel sp = openAdmin(island);
+        assertEquals(Mode.EXPERT, sp.getMode());
+        click(items(sp), MODE, user, ClickType.LEFT);
+        assertEquals(Mode.BASIC, sp.getMode());
+        verify(plugin.getPlayers(), never()).getFlagsDisplayMode(any());
+        verify(plugin.getPlayers(), never()).setFlagsDisplayMode(any(), any());
+    }
+
     private SettingsPanel openAdmin(Island target) {
         assertTrue(SettingsPanel.openAdminPanel(command, user, target));
         return (SettingsPanel) PanelListenerManager.getOpenPanels().get(uuid).getListener().orElseThrow();