diff --git a/buildsystem-api/src/main/java/de/eintosti/buildsystem/api/world/access/WorldSetting.java b/buildsystem-api/src/main/java/de/eintosti/buildsystem/api/world/access/WorldSetting.java index 89534d93..c728dcd0 100644 --- a/buildsystem-api/src/main/java/de/eintosti/buildsystem/api/world/access/WorldSetting.java +++ b/buildsystem-api/src/main/java/de/eintosti/buildsystem/api/world/access/WorldSetting.java @@ -33,17 +33,17 @@ public enum WorldSetting { /** * Whether blocks may be broken in the world. */ - BLOCK_BREAKING("buildsystem.bypass.settings", WorldDataKey.BLOCK_BREAKING), + BLOCK_BREAKING(bypassSettings(), WorldDataKey.BLOCK_BREAKING), /** * Whether blocks may be placed in the world. */ - BLOCK_PLACEMENT("buildsystem.bypass.settings", WorldDataKey.BLOCK_PLACEMENT), + BLOCK_PLACEMENT(bypassSettings(), WorldDataKey.BLOCK_PLACEMENT), /** * Whether blocks may be interacted with in the world. */ - BLOCK_INTERACTIONS("buildsystem.bypass.settings", WorldDataKey.BLOCK_INTERACTIONS); + BLOCK_INTERACTIONS(bypassSettings(), WorldDataKey.BLOCK_INTERACTIONS); private final String bypassPermission; private final WorldDataKey key; @@ -53,6 +53,13 @@ public enum WorldSetting { this.key = key; } + /** + * The single source of truth for the permission node shared by every {@link WorldSetting}. + */ + private static String bypassSettings() { + return "buildsystem.bypass.settings"; + } + /** * Gets the permission node that lets a player modify the world even when this setting would otherwise deny it. * diff --git a/buildsystem-core/build.gradle.kts b/buildsystem-core/build.gradle.kts index 239b71bb..3b3de5ca 100644 --- a/buildsystem-core/build.gradle.kts +++ b/buildsystem-core/build.gradle.kts @@ -240,7 +240,6 @@ bukkit { } register("buildsystem.create") { children = listOf( - "buildsystem.create.private", "buildsystem.create.type.normal", "buildsystem.create.type.flat", "buildsystem.create.type.nether", @@ -250,6 +249,11 @@ bukkit { description = "Permission for creating world types." default = BukkitPluginDescription.Permission.Default.TRUE } + register("buildsystem.create.category") { + description = + "Create a world in a navigator category. Category ids are dynamic; grant buildsystem.create.category. to allow a specific category." + default = BukkitPluginDescription.Permission.Default.OP + } register("buildsystem.create.template") { description = "Select a template when creating a world. Template names are dynamic; deny buildsystem.create.template. to restrict a specific template." default = BukkitPluginDescription.Permission.Default.TRUE diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/BuildSystemPlugin.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/BuildSystemPlugin.java index 0fd58db4..6e692908 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/BuildSystemPlugin.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/BuildSystemPlugin.java @@ -119,16 +119,19 @@ public void onDisable() { reloadConfigData(false); saveConfig(); - try { - saveBuildConfig().join(); - } catch (CompletionException e) { - getLogger().severe("Error while waiting for saves: " + e.getCause()); - } + // Cancelling only stops future ticks, not one already running, + // so this must happen before the join() below if (this.configSaveTask != null) { this.configSaveTask.cancel(); } + try { + saveBuildConfig().join(); + } catch (CompletionException e) { + getLogger().severe("Error while waiting for saves: %s".formatted(e.getCause())); + } + // Shut the shared background pool down only after the final saves above have completed. services.scheduler().shutdown(); @@ -149,26 +152,42 @@ private void performUpdateCheck() { return; } - updateChecker.requestUpdateCheck().whenComplete((result, e) -> { - if (result.requiresUpdate()) { - Bukkit.getConsoleSender() - .sendMessage(ChatColor.YELLOW + "[BuildSystem] Great! a new update is available: " - + ChatColor.GREEN + "v" + result.getNewestVersion()); - Bukkit.getConsoleSender() - .sendMessage(ChatColor.YELLOW + " ➥ Your current version: " + ChatColor.RED - + this.getDescription().getVersion()); - return; - } - - UpdateChecker.UpdateReason reason = result.getReason(); - switch (reason) { - case COULD_NOT_CONNECT, INVALID_JSON, UNAUTHORIZED_QUERY, UNKNOWN_ERROR, UNSUPPORTED_VERSION_SCHEME -> - Bukkit.getConsoleSender() - .sendMessage(ChatColor.RED - + "[BuildSystem] Could not check for a new version of BuildSystem. Reason: " - + reason); - } - }); + updateChecker + .requestUpdateCheck() + .whenCompleteAsync( + (result, e) -> { + if (result == null) { + return; + } + + if (result.requiresUpdate()) { + Bukkit.getConsoleSender() + .sendMessage("%s[BuildSystem] Great! a new update is available: %sv%s" + .formatted( + ChatColor.YELLOW, ChatColor.GREEN, result.getNewestVersion())); + Bukkit.getConsoleSender() + .sendMessage("%s ➥ Your current version: %s%s" + .formatted( + ChatColor.YELLOW, + ChatColor.RED, + this.getDescription().getVersion())); + return; + } + + UpdateChecker.UpdateReason reason = result.getReason(); + switch (reason) { + case COULD_NOT_CONNECT, + INVALID_JSON, + UNAUTHORIZED_QUERY, + UNKNOWN_ERROR, + UNSUPPORTED_VERSION_SCHEME -> + Bukkit.getConsoleSender() + .sendMessage( + "%s[BuildSystem] Could not check for a new version of BuildSystem. Reason: %s" + .formatted(ChatColor.RED, reason)); + } + }, + services.scheduler().mainThread()); } private void createTemplateFolder() { diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/AddBuilderSubCommand.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/AddBuilderSubCommand.java index da4f188b..f5dee070 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/AddBuilderSubCommand.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/AddBuilderSubCommand.java @@ -63,16 +63,13 @@ public AddBuilderSubCommand( public void execute(Player player, String worldName, String[] args) { BuildWorld buildWorld = worldService.getWorldStorage().getBuildWorld(player.getWorld().getName()); - if (buildWorld != null - && !buildWorld - .getPermissions() - .canPerformCommand(player, getArgument().getPermission())) { - messages.sendPermissionError(player); + if (buildWorld == null) { + messages.sendMessage(player, "worlds_addbuilder_unknown_world"); return; } - if (buildWorld == null) { - messages.sendMessage(player, "worlds_addbuilder_unknown_world"); + if (!hasAddBuilderPermission(player, buildWorld)) { + messages.sendPermissionError(player); return; } @@ -146,12 +143,27 @@ private void applyBuilder( } public void getAddBuilderInput(Player player, BuildWorld buildWorld, boolean closeInventory) { + if (!hasAddBuilderPermission(player, buildWorld)) { + messages.sendPermissionError(player); + return; + } + prompts.prompt(player).title("enter_player_name").request(input -> { String builderName = input.trim(); addBuilder(player, buildWorld, builderName, closeInventory); }); } + /** + * Whether {@code player} may add builders to {@code buildWorld}, per the argument's permission node. Shared by the + * command entry point and the GUI prompt so both are gated by the same check. + */ + private boolean hasAddBuilderPermission(Player player, BuildWorld buildWorld) { + return buildWorld + .getPermissions() + .canPerformCommand(player, getArgument().getPermission()); + } + @Override public List complete(Player player, String[] args) { if (args.length != 2) { diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetPermissionSubCommand.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetPermissionSubCommand.java index f5fc42d2..be8c382a 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetPermissionSubCommand.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetPermissionSubCommand.java @@ -89,6 +89,13 @@ public void getPermissionInput(Player player, BuildWorld buildWorld, boolean clo XSound.ENTITY_PLAYER_LEVELUP.play(player); messages.sendMessage(player, "worlds_setpermission_set", Placeholders.of("%world%", buildWorld.getName())); + // A folder override means the stored permission is not the one being enforced. + String effective = buildWorld.getData().get(WorldDataKey.PERMISSION); + if (!permission.equals(effective)) { + messages.sendMessage( + player, "worlds_setpermission_overridden", Placeholders.of("%permission%", effective)); + } + if (closeInventory) { player.closeInventory(); } else { diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetProjectSubCommand.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetProjectSubCommand.java index 841ea689..eb93d216 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetProjectSubCommand.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/command/subcommand/worlds/SetProjectSubCommand.java @@ -64,12 +64,19 @@ public void execute(Player player, String worldName, String[] args) { public void getProjectInput(Player player, BuildWorld buildWorld, boolean closeInventory) { prompts.prompt(player).title("enter_world_project").request(input -> { - buildWorld.getData().set(WorldDataKey.PROJECT, input.trim()); + String project = input.trim(); + buildWorld.getData().set(WorldDataKey.PROJECT, project); settingsService.forceUpdateSidebar(buildWorld); XSound.ENTITY_PLAYER_LEVELUP.play(player); messages.sendMessage(player, "worlds_setproject_set", Placeholders.of("%world%", buildWorld.getName())); + // The world's folder can override this value, in which case the stored project is not the one shown. + String effective = buildWorld.getData().get(WorldDataKey.PROJECT); + if (!project.equals(effective)) { + messages.sendMessage(player, "worlds_setproject_overridden", Placeholders.of("%project%", effective)); + } + if (closeInventory) { player.closeInventory(); } else { diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java index d00f8844..6748bed1 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/PluginConfig.java @@ -77,6 +77,14 @@ public record World( */ public record VoidBlock(boolean enabled, Material material) {} + /** + * Fallback world-creation limits, applied only to players holding no + * {@code buildsystem.create..} permission node, so a permission grant always wins. Counted + * per player and per visibility; {@code -1} means unlimited. + * + * @param publicWorlds Default maximum number of public worlds one player may create + * @param privateWorlds Default maximum number of private worlds one player may create + */ public record Limits(int publicWorlds, int privateWorlds) {} public record Defaults( @@ -133,7 +141,17 @@ public record Sftp( @Nullable String username, @Nullable String password, @Nullable String path) - implements StorageSettings {} + implements StorageSettings { + + /** + * Overridden so the password never appears in a log or pasted support output. + */ + @Override + public String toString() { + return "Sftp[host=%s, port=%d, username=%s, password=%s, path=%s]" + .formatted(host, port, username, password == null ? null : "***", path); + } + } public record S3( @Nullable String url, @@ -142,7 +160,23 @@ public record S3( @Nullable String region, @Nullable String bucket, @Nullable String path) - implements StorageSettings {} + implements StorageSettings { + + /** + * Overridden so the access key and secret key never appear in a log or pasted support output. + */ + @Override + public String toString() { + return "S3[url=%s, accessKey=%s, secretKey=%s, region=%s, bucket=%s, path=%s]" + .formatted( + url, + accessKey == null ? null : "***", + secretKey == null ? null : "***", + region, + bucket, + path); + } + } public record AutoBackup(boolean enabled, boolean onlyActiveWorlds, int interval) {} } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/ConfigMigrationManager.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/ConfigMigrationManager.java index 19002261..f2399f6e 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/ConfigMigrationManager.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/config/migration/ConfigMigrationManager.java @@ -18,9 +18,14 @@ package de.eintosti.buildsystem.config.migration; import de.eintosti.buildsystem.BuildSystemPlugin; +import java.io.File; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.StandardCopyOption; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.logging.Level; import java.util.logging.Logger; import org.jspecify.annotations.NullMarked; @@ -68,22 +73,60 @@ private void registerMigration(int fromVersion, Migration migration) { */ public void migrate() { Logger logger = plugin.getLogger(); + int fromVersion = plugin.getConfig().getInt("version", 1); + if (fromVersion >= LATEST_VERSION) { + logger.info("Config is at the latest version: %d".formatted(fromVersion)); + return; + } + + if (!backupConfig(fromVersion, logger)) { + return; + } while (plugin.getConfig().getInt("version", 1) < LATEST_VERSION) { int currentVersion = plugin.getConfig().getInt("version", 1); Migration migration = migrations.get(currentVersion); if (migration == null) { throw new IllegalStateException( - "Missing migration from version " + currentVersion + " to " + (currentVersion + 1)); + "Missing migration from version %d to %d".formatted(currentVersion, currentVersion + 1)); } - logger.info("Migrating from version " + currentVersion + " to " + (currentVersion + 1) + "..."); + logger.info("Migrating from version %d to %d...".formatted(currentVersion, currentVersion + 1)); migration.migrate(plugin.getConfig()); plugin.getConfig().set("version", currentVersion + 1); plugin.getConfig().setComments("version", List.of("Internal, do not change manually!")); plugin.saveConfig(); } - logger.info("Config is at the latest version: " + plugin.getConfig().getInt("version", 1)); + logger.info("Config is at the latest version: %d" + .formatted(plugin.getConfig().getInt("version", 1))); + } + + /** + * Copies {@code config.yml} to a sibling {@code config.yml.v.bak} before the first migration + * mutates it in place. + * + * @return {@code true} if the backup succeeded (or the config file does not exist yet), {@code false} if + * migration must be aborted because the pre-migration state could not be preserved + */ + private boolean backupConfig(int fromVersion, Logger logger) { + File configFile = new File(plugin.getDataFolder(), "config.yml"); + if (!configFile.exists()) { + return true; + } + + File backup = new File(plugin.getDataFolder(), "config.yml.v%d.bak".formatted(fromVersion)); + try { + Files.copy(configFile.toPath(), backup.toPath(), StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e) { + logger.log( + Level.SEVERE, + "Failed to back up config.yml to %s; aborting migration".formatted(backup.getName()), + e); + return false; + } + + logger.info("Backed up config.yml to %s".formatted(backup.getName())); + return true; } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/listener/player/PlayerJoinListener.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/listener/player/PlayerJoinListener.java index ec72d0ad..898d5107 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/listener/player/PlayerJoinListener.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/listener/player/PlayerJoinListener.java @@ -216,16 +216,21 @@ private void performUpdateCheck(Player player) { return; } - updateChecker.requestUpdateCheck().whenComplete((result, e) -> { - if (result.requiresUpdate()) { - StringBuilder stringBuilder = new StringBuilder(); - messages.getStringList("update_available", player) - .forEach(line -> stringBuilder - .append(line.replace("%new_version%", result.getNewestVersion()) - .replace("%current_version%", updateChecker.getCurrentVersion())) - .append("\n")); - player.sendMessage(stringBuilder.toString()); - } - }); + updateChecker + .requestUpdateCheck() + .whenCompleteAsync( + (result, e) -> { + if (result == null || !result.requiresUpdate()) { + return; + } + + Placeholders placeholders = Placeholders.of() + .add("%new_version%", result.getNewestVersion()) + .add("%current_version%", updateChecker.getCurrentVersion()) + .build(); + player.sendMessage(String.join( + "\n", messages.getStringList("update_available", player, placeholders))); + }, + scheduler.mainThread()); } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/ButtonMenu.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/ButtonMenu.java index ecbc6b1e..3b83650f 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/ButtonMenu.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/ButtonMenu.java @@ -19,8 +19,10 @@ import com.cryptomorin.xseries.XSound; import de.eintosti.buildsystem.i18n.Messages; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; +import java.util.Set; import org.bukkit.entity.Player; import org.bukkit.event.inventory.InventoryClickEvent; import org.bukkit.inventory.Inventory; @@ -45,6 +47,12 @@ public abstract class ButtonMenu extends Menu { private final Map buttons = new LinkedHashMap<>(); + /** + * Slots registered since the last {@link #renderButtons} (or {@link #clearButtons}). Bounds collision detection to a + * single population pass, so re-populating a menu with the same slots on reopen is not mistaken for a collision. + */ + private final Set registeredThisPass = new HashSet<>(); + /** * Creates the menu and its backing inventory. * @@ -57,12 +65,18 @@ protected ButtonMenu(Messages messages, int size, String title) { } /** - * Registers (or replaces) the button shown at the given slot. + * Registers the button shown at the given slot. * * @param slot The inventory slot * @param button The button to place at the slot + * @throws IllegalStateException if the slot was already registered in this population pass (i.e. since the last + * {@link #renderButtons} or {@link #clearButtons}), which almost always means two catalogs disagree about which + * slot a button belongs to */ protected final void register(int slot, B button) { + if (!registeredThisPass.add(slot)) { + throw new IllegalStateException("Slot %d is already registered".formatted(slot)); + } buttons.put(slot, button); } @@ -72,6 +86,7 @@ protected final void register(int slot, B button) { */ protected final void clearButtons() { buttons.clear(); + registeredThisPass.clear(); } /** @@ -83,14 +98,17 @@ protected final Map buttons() { } /** - * Renders every registered button into this menu's inventory. Subclasses typically call this from - * {@link #populate(Player)} after filling any background items. + * Renders every registered button into this menu's inventory and closes out the current population pass, so the next + * round of {@link #register} calls (e.g. from a later reopen) is free to reuse the same slots. + * + *

Subclasses typically call this from {@link #populate(Player)} after filling any background items. * * @param player The viewing player */ protected final void renderButtons(Player player) { Inventory inventory = getInventory(); buttons.forEach((slot, button) -> button.render(player, inventory, slot)); + registeredThisPass.clear(); } /** @@ -111,8 +129,7 @@ public void handleClick(InventoryClickEvent event) { return; } - String permission = button.permission(); - if (permission != null && !player.hasPermission(permission)) { + if (!button.canClick(player)) { onPermissionDenied(player, event); return; } @@ -121,10 +138,9 @@ public void handleClick(InventoryClickEvent event) { } /** - * Hook for a click on a button whose {@link MenuButton#permission() permission} the player lacks. The default - * closes the inventory, sends the permission error and plays the deny sound, matching - * the guard it replaces. Menus that must keep the inventory open on a denied click (e.g. per-toggle - * settings) override this. + * Hook for a click the player is not allowed to make, i.e. one rejected by + * {@link MenuButton#canClick(Player)}. The default closes the inventory, sends the permission error and plays the deny sound, matching the guard it + * replaces. Menus that must keep the inventory open on a denied click (e.g. per-toggle settings) override this. * * @param player The clicking player * @param event The click event (already cancelled) diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/MenuButton.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/MenuButton.java index 67460308..3b46e6fb 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/MenuButton.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/MenuButton.java @@ -17,6 +17,7 @@ */ package de.eintosti.buildsystem.menu; +import java.util.function.Predicate; import org.bukkit.entity.Player; import org.bukkit.event.inventory.InventoryClickEvent; import org.bukkit.inventory.Inventory; @@ -55,14 +56,45 @@ public interface MenuButton { /** * {@return the permission required to click this button, or {@code null} if it is unrestricted} * - *

This is enforced: {@link ButtonMenu#handleClick} checks it before dispatching to + *

This is enforced through {@link #canClick} before {@link ButtonMenu#handleClick} dispatches to * {@link #onClick}, so a button that declares a permission never has to check it again. Denial is handled by - * {@link ButtonMenu#onPermissionDenied}. + * {@link ButtonMenu#onPermissionDenied}. For access a single node cannot express, see {@link #usableBy()}. */ default @Nullable String permission() { return null; } + /** + * {@return a test the clicking player must pass, or {@code null} if there is none} + * + *

Exists because {@link #permission()} can only express a single node, while access is often scoped to the + * resource the menu is acting on — whether the player created this world, whether they are under their world + * limit. Both are inputs to {@link #canClick}; neither is enforced on its own. + */ + default @Nullable Predicate usableBy() { + return null; + } + + /** + * {@return whether the player is allowed to click this button} + * + *

The single place {@link #permission()} and {@link #usableBy()} are combined, and the only thing + * {@link ButtonMenu#handleClick} consults — so declaring either one is enough to have it enforced, and a + * button never re-checks access inside {@link #onClick}. Hand-rolled checks in a click handler are what let + * authorization drift out of step with what was rendered. + * + * @param player The clicking player + */ + default boolean canClick(Player player) { + String permission = permission(); + if (permission != null && !player.hasPermission(permission)) { + return false; + } + + Predicate usableBy = usableBy(); + return usableBy == null || usableBy.test(player); + } + /** * {@return a new {@link Builder} for assembling a {@code MenuButton} from a renderer and a click handler} Either part * may be omitted: an unset renderer draws nothing and an unset click handler does nothing. @@ -111,6 +143,7 @@ final class Builder { private Renderer renderer = (player, inventory, slot) -> {}; private ClickHandler clickHandler = (player, event) -> {}; private @Nullable String permission; + private @Nullable Predicate usableBy; private Builder() {} @@ -126,6 +159,19 @@ public Builder permission(@Nullable String permission) { return this; } + /** + * Restricts the button to players passing the given test, for access that a single permission node cannot + * express because it depends on the resource being acted on. Combined with the permission by + * {@link MenuButton#canClick(Player)}. + * + * @param usableBy The test, or {@code null} for no restriction + * @return This builder + */ + public Builder usableBy(@Nullable Predicate usableBy) { + this.usableBy = usableBy; + return this; + } + /** * Sets how the button renders into its slot. * @@ -155,6 +201,7 @@ public MenuButton build() { Renderer builtRenderer = renderer; ClickHandler builtClickHandler = clickHandler; String builtPermission = permission; + Predicate builtUsableBy = usableBy; return new MenuButton() { @Override @@ -171,6 +218,11 @@ public void onClick(Player player, InventoryClickEvent event) { public @Nullable String permission() { return builtPermission; } + + @Override + public @Nullable Predicate usableBy() { + return builtUsableBy; + } }; } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/MenuItems.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/MenuItems.java index e484b01d..b3508a87 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/MenuItems.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/menu/MenuItems.java @@ -112,7 +112,20 @@ public void renderDisplayable(Inventory inventory, int slot, Displayable display ItemBuilder.of(XMaterial.PLAYER_HEAD).name(name).lore(lore).into(inventory, slot); } - private void applyHeadProfileAsync( + /** + * Renders a placeholder head into a menu slot immediately, then resolves the given head profile asynchronously and + * swaps in the finished stack on the main thread once it's ready. For callers that derive both the icon type and + * the name/lore from a {@link Displayable}, use {@link #renderDisplayable} instead; this method is for callers + * that already know they want a head and supply their own name/lore (e.g. an editor's world-icon button). + * + * @param inventory The inventory to add the item to + * @param slot The slot to add the item at + * @param profile The head profile to resolve + * @param fallback The profile to fall back to if {@code profile} cannot be resolved, or {@code null} for none + * @param name The already-styled display name to apply + * @param lore The lore to apply + */ + public void applyHeadProfileAsync( Inventory inventory, int slot, Profileable profile, diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/player/PlayerServiceImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/player/PlayerServiceImpl.java index b90750bb..853b7629 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/player/PlayerServiceImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/player/PlayerServiceImpl.java @@ -22,16 +22,17 @@ import de.eintosti.buildsystem.api.storage.PlayerStorage; import de.eintosti.buildsystem.api.world.data.Visibility; import de.eintosti.buildsystem.config.ConfigService; +import de.eintosti.buildsystem.config.PluginConfig; import de.eintosti.buildsystem.storage.PlayerStorageImpl; import de.eintosti.buildsystem.storage.WorldStorageImpl; import de.eintosti.buildsystem.storage.yaml.YamlPlayerStorage; import de.eintosti.buildsystem.util.TaskScheduler; import de.eintosti.buildsystem.world.WorldServiceImpl; import java.util.Collections; -import java.util.HashSet; import java.util.Set; import java.util.UUID; import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Supplier; import java.util.logging.Level; import org.bukkit.entity.Player; @@ -58,7 +59,7 @@ public PlayerServiceImpl( this.worldService = worldService; this.playerStorage = new YamlPlayerStorage(plugin, scheduler); this.maxWorldsResolver = new MaxWorldsResolver(plugin.getLogger()); - this.buildModePlayers = new HashSet<>(); + this.buildModePlayers = ConcurrentHashMap.newKeySet(); } public void init() { @@ -92,23 +93,28 @@ public boolean leaveBuildMode(UUID playerId) { @Override public boolean canCreateWorld(Player player, Visibility visibility) { - boolean showPrivateWorlds = visibility == Visibility.ADDED_PLAYERS; - WorldStorageImpl worldStorage = worldService.get().getWorldStorage(); + if (player.hasPermission(BuildSystemPlugin.ADMIN_PERMISSION)) { + return true; + } - int maxWorldAmountConfig = showPrivateWorlds - ? configService.current().world().limits().privateWorlds() - : configService.current().world().limits().publicWorlds(); - if (maxWorldAmountConfig >= 0 && worldStorage.getBuildWorlds().size() >= maxWorldAmountConfig) { - return false; + int max = getMaxWorlds(player, visibility); + if (max < 0) { + max = configuredLimit(visibility); + } + if (max < 0) { + return true; } - int maxWorldAmountPlayer = - getMaxWorlds(player, showPrivateWorlds ? Visibility.ADDED_PLAYERS : Visibility.EVERYONE); - return maxWorldAmountPlayer < 0 - || worldStorage - .getBuildWorldsCreatedByPlayer(player, visibility) - .size() - < maxWorldAmountPlayer; + WorldStorageImpl worldStorage = worldService.get().getWorldStorage(); + return worldStorage.getBuildWorldsCreatedByPlayer(player, visibility).size() < max; + } + + /** + * {@return the configured fallback limit for the given visibility, or {@code -1} for unlimited} + */ + private int configuredLimit(Visibility visibility) { + PluginConfig.World.Limits limits = configService.current().world().limits(); + return visibility == Visibility.ADDED_PLAYERS ? limits.privateWorlds() : limits.publicWorlds(); } @Override diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/AbstractYamlStorage.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/AbstractYamlStorage.java index bb0bba92..0b6a0b78 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/AbstractYamlStorage.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/AbstractYamlStorage.java @@ -38,11 +38,11 @@ public AbstractYamlStorage(BuildSystemPlugin plugin, String fileName) { } public void loadFile() { - store.reload(); + store.locked(store::reload); } public void saveFile() { - store.save(); + store.atomicSave(() -> {}); } public @Nullable FileConfiguration getFile() { diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlStore.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlStore.java index 69f8c557..dc428f92 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlStore.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/storage/yaml/YamlStore.java @@ -19,7 +19,10 @@ import java.io.File; import java.io.IOException; +import java.nio.file.AtomicMoveNotSupportedException; import java.nio.file.Files; +import java.nio.file.Path; +import java.nio.file.StandardCopyOption; import java.util.function.Supplier; import java.util.logging.Level; import java.util.logging.Logger; @@ -76,16 +79,52 @@ public boolean reload() { return true; } - /** Writes the configuration to disk. Callers needing atomicity with a preceding mutation use {@link #atomicSave}. */ + /** + * Writes the configuration to disk. Callers needing this synchronized with a preceding mutation use + * {@link #atomicSave}. + * + *

Writes to a sibling temp file first and moves it onto {@code file}, so a crash or power loss mid-write + * never leaves a truncated file in place — {@link #reload} either reads the previous complete file or the new + * complete one, never a partial one. + */ public void save() { + File temp = new File(file.getParentFile(), file.getName() + ".tmp"); + try { + configuration.save(temp); + } catch (IOException e) { + logger.log(Level.SEVERE, "Failed to save configuration file: %s".formatted(file.getName()), e); + deleteQuietly(temp); + return; + } + + Path tempPath = temp.toPath(); + Path targetPath = file.toPath(); try { - configuration.save(file); + Files.move(tempPath, targetPath, StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING); + } catch (AtomicMoveNotSupportedException e) { + try { + Files.move(tempPath, targetPath, StandardCopyOption.REPLACE_EXISTING); + } catch (IOException e2) { + logger.log(Level.SEVERE, "Failed to save configuration file: %s".formatted(file.getName()), e2); + deleteQuietly(temp); + } } catch (IOException e) { - logger.log(Level.SEVERE, "Failed to save configuration file: " + file.getName(), e); + logger.log(Level.SEVERE, "Failed to save configuration file: %s".formatted(file.getName()), e); + deleteQuietly(temp); } } - /** Applies {@code mutation} to the configuration and persists it, both under the I/O lock. */ + private void deleteQuietly(File file) { + try { + Files.deleteIfExists(file.toPath()); + } catch (IOException ignored) { + // A leftover temp file is harmless: the next successful save overwrites it. + } + } + + /** + * Applies {@code mutation} to the configuration and persists it, both under the I/O lock guarding the file. + */ public void atomicSave(Runnable mutation) { synchronized (ioLock) { mutation.run(); @@ -93,7 +132,9 @@ public void atomicSave(Runnable mutation) { } } - /** Runs {@code work} under the I/O lock — for read/load sequences that must not race a concurrent save. */ + /** + * Runs {@code work} under the I/O lock — for read/load sequences that must not race a concurrent save. + */ public T locked(Supplier work) { synchronized (ioLock) { return work.get(); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/FileUtils.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/FileUtils.java index 8c0b4a5c..dc4704cd 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/FileUtils.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/FileUtils.java @@ -32,9 +32,8 @@ import java.util.logging.Level; import java.util.logging.Logger; import java.util.stream.Stream; -import java.util.zip.ZipEntry; -import java.util.zip.ZipOutputStream; import net.lingala.zip4j.ZipFile; +import net.lingala.zip4j.io.outputstream.ZipOutputStream; import net.lingala.zip4j.model.ExcludeFileFilter; import net.lingala.zip4j.model.ZipParameters; import org.bukkit.Bukkit; @@ -296,7 +295,9 @@ static byte[] zipDirectoryToMemory(Path worldPath, @Nullable Path excludedSubtre .toList(); for (Path file : files) { Path relativePath = worldPath.relativize(file); - zipOut.putNextEntry(new ZipEntry(relativePath.toString().replace("\\", "/"))); + ZipParameters zipParameters = new ZipParameters(); + zipParameters.setFileNameInZip(relativePath.toString().replace("\\", "/")); + zipOut.putNextEntry(zipParameters); Files.copy(file, zipOut); zipOut.closeEntry(); } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/Permissions.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/Permissions.java index 40000f45..a583e066 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/Permissions.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/Permissions.java @@ -21,12 +21,12 @@ import org.jspecify.annotations.NullMarked; /** - * Every permission node the plugin checks. Nodes are declared here rather than inline so a typo is a compile error - * instead of a check that silently never passes, and so the full set the plugin ships can be read in one place. + * The complete, fixed set of permission nodes is the static constants below; declaring them here turns a typo into a + * compile error instead of a check that silently never passes. * - *

Nodes whose last segment is supplied at runtime (world types, templates, generators, navigator categories, - * statuses, settings toggles, gamemodes) are built by the helper methods at the bottom rather than by concatenating - * a prefix at the call site. + *

The factory methods at the bottom are different: each produces an open-ended family of nodes whose members are + * not fixed here but depend on ids minted elsewhere at runtime (world types, templates, generators, navigator + * categories, statuses, settings toggles, gamemodes). */ @NullMarked public final class Permissions { @@ -44,11 +44,9 @@ private Permissions() {} public static final String BUILDERS = "buildsystem.builders"; public static final String BYPASS_ARCHIVE = "buildsystem.bypass.archive"; public static final String BYPASS_BUILDERS = "buildsystem.bypass.builders"; - public static final String BYPASS_PERMISSION = "buildsystem.bypass.permission"; public static final String BYPASS_PERMISSION_ARCHIVE = "buildsystem.bypass.permission.archive"; public static final String BYPASS_PERMISSION_PRIVATE = "buildsystem.bypass.permission.private"; public static final String BYPASS_PERMISSION_PUBLIC = "buildsystem.bypass.permission.public"; - public static final String BYPASS_SETTINGS = "buildsystem.bypass.settings"; public static final String COLOR_CHAT = "buildsystem.color.chat"; public static final String COLOR_SIGN = "buildsystem.color.sign"; public static final String CONFIG = "buildsystem.config"; @@ -142,21 +140,24 @@ public static String createGenerator(String generatorName) { } /** - * {@return the permission to create a world in the given navigator category} + * {@return the permission to create a world in the given navigator category} Namespaced under {@code category} so + * a category id can never collide with the reserved {@code buildsystem.create.*} nodes (e.g. {@link #CREATE_FOLDER}). * * @param categoryId The category id */ public static String createInCategory(String categoryId) { - return "buildsystem.create." + categoryId; + return "buildsystem.create.category." + categoryId; } /** * {@return the permission to see the given navigator category and use its {@code /worlds } shortcut} + * Namespaced under {@code category} so a category id can never collide with the reserved + * {@code buildsystem.navigator.*} nodes (e.g. {@link #NAVIGATOR_ITEM}). * * @param categoryId The category id */ public static String navigatorCategory(String categoryId) { - return "buildsystem.navigator." + categoryId; + return "buildsystem.navigator.category." + categoryId; } /** diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/UpdateChecker.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/UpdateChecker.java index 35376e69..1f3547da 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/UpdateChecker.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/util/UpdateChecker.java @@ -22,10 +22,12 @@ import com.google.gson.JsonParser; import com.google.gson.stream.JsonReader; import java.io.IOException; -import java.io.InputStreamReader; -import java.net.HttpURLConnection; +import java.io.StringReader; import java.net.URI; -import java.net.URL; +import java.net.http.HttpClient; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.time.Duration; import java.util.concurrent.CompletableFuture; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -46,11 +48,12 @@ public final class UpdateChecker { private static final String USER_AGENT = "CHOCO-update-checker"; private static final String UPDATE_URL = "https://api.spigotmc.org/simple/0.1/index.php?action=getResource&id=%d"; private static final Pattern DECIMAL_SCHEME_PATTERN = Pattern.compile("\\d+(?:\\.\\d+)*"); + private static final Duration TIMEOUT = Duration.ofSeconds(5); /** * The default version scheme for this update checker */ - public static final @Nullable VersionScheme VERSION_SCHEME_DECIMAL = (first, second) -> { + public static final VersionScheme VERSION_SCHEME_DECIMAL = (first, second) -> { String[] firstSplit = splitVersionInfo(first), secondSplit = splitVersionInfo(second); if (firstSplit == null || secondSplit == null) { return null; @@ -72,6 +75,7 @@ public final class UpdateChecker { private final JavaPlugin plugin; private final int pluginID; private final VersionScheme versionScheme; + private final HttpClient httpClient; public UpdateChecker(JavaPlugin plugin, int pluginID) { this(plugin, pluginID, VERSION_SCHEME_DECIMAL); @@ -82,6 +86,7 @@ public UpdateChecker(JavaPlugin plugin, int pluginID, VersionScheme versionSchem this.plugin = plugin; this.pluginID = pluginID; this.versionScheme = versionScheme; + this.httpClient = HttpClient.newBuilder().connectTimeout(TIMEOUT).build(); } /** @@ -109,14 +114,18 @@ public CompletableFuture requestUpdateCheck() { int responseCode; try { - URL url = URI.create(UPDATE_URL.formatted(pluginID)).toURL(); - HttpURLConnection connection = (HttpURLConnection) url.openConnection(); - connection.addRequestProperty("User-Agent", USER_AGENT); - responseCode = connection.getResponseCode(); - - JsonReader reader = new JsonReader(new InputStreamReader(connection.getInputStream())); - JsonElement json = JsonParser.parseReader(reader); - reader.close(); + HttpRequest request = HttpRequest.newBuilder(URI.create(UPDATE_URL.formatted(pluginID))) + .timeout(TIMEOUT) + .header("User-Agent", USER_AGENT) + .GET() + .build(); + HttpResponse response = httpClient.send(request, HttpResponse.BodyHandlers.ofString()); + responseCode = response.statusCode(); + + JsonElement json; + try (JsonReader reader = new JsonReader(new StringReader(response.body()))) { + json = JsonParser.parseReader(reader); + } if (!json.isJsonObject()) { return new UpdateResult(UpdateReason.INVALID_JSON); @@ -139,6 +148,9 @@ public CompletableFuture requestUpdateCheck() { } } catch (IOException e) { return new UpdateResult(UpdateReason.COULD_NOT_CONNECT); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return new UpdateResult(UpdateReason.COULD_NOT_CONNECT); } return new UpdateResult(responseCode == 401 ? UpdateReason.UNAUTHORIZED_QUERY : UpdateReason.UNKNOWN_ERROR); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/type/Bypassable.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/WorldNameInputOptions.java similarity index 62% rename from buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/type/Bypassable.java rename to buildsystem-core/src/main/java/de/eintosti/buildsystem/world/WorldNameInputOptions.java index 762823f8..ba5b85db 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/type/Bypassable.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/WorldNameInputOptions.java @@ -15,15 +15,16 @@ * You should have received a copy of the GNU General Public License * along with this program. If not, see . */ -package de.eintosti.buildsystem.world.data.type; +package de.eintosti.buildsystem.world; import org.jspecify.annotations.NullMarked; /** - * A {@link Capability} that marks a {@link Property} as being bypassable with a specific permission. + * The two flags that shape a {@link WorldServiceImpl#startWorldNameInput} prompt flow, grouped so call sites carry + * named components instead of two adjacent, easily transposed booleans. * - * @param permission The permission node required to bypass this type - * @since 3.0.1 + * @param privateWorld Whether the created world should be private + * @param promptSeed Whether the flow should ask the player for a seed before building the world */ @NullMarked -public record Bypassable(String permission) implements Capability {} +public record WorldNameInputOptions(boolean privateWorld, boolean promptSeed) {} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/WorldServiceImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/WorldServiceImpl.java index 644274fa..e0e8d027 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/WorldServiceImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/WorldServiceImpl.java @@ -34,6 +34,7 @@ import de.eintosti.buildsystem.api.world.creation.generator.CustomGenerator; import de.eintosti.buildsystem.api.world.creation.generator.Generator; import de.eintosti.buildsystem.api.world.data.BuildWorldType; +import de.eintosti.buildsystem.api.world.data.Visibility; import de.eintosti.buildsystem.api.world.display.Folder; import de.eintosti.buildsystem.api.world.lifecycle.SaveBehavior; import de.eintosti.buildsystem.i18n.Messages; @@ -43,6 +44,7 @@ import de.eintosti.buildsystem.storage.yaml.YamlFolderStorage; import de.eintosti.buildsystem.storage.yaml.YamlWorldStorage; import de.eintosti.buildsystem.util.FileUtils; +import de.eintosti.buildsystem.util.Permissions; import de.eintosti.buildsystem.util.StringCleaner; import de.eintosti.buildsystem.world.creation.WorldBuilderImpl; import de.eintosti.buildsystem.world.creation.WorldCreationPrompts; @@ -112,32 +114,47 @@ public WorldStorageImpl getWorldStorage() { @Override @Contract("_ -> new") public WorldBuilder newWorld(String name) { - // Defense-in-depth: the menus sanitize names before they reach here, but this is public API. Reject any name - // that would resolve outside the world container so a caller cannot create a directory in, say, plugins/. - if (StringCleaner.isReservedName(name)) { - throw new IllegalArgumentException("World name '" + name + "' is reserved and cannot be used"); - } - File worldDirectory = new File(Bukkit.getWorldContainer(), name); - if (StringCleaner.isPathEscape(Bukkit.getWorldContainer(), worldDirectory)) { - throw new IllegalArgumentException("World name '" + name + "' resolves outside the world container"); - } + validateWorldName(name); return new WorldBuilderImpl(services.worldContext(), worldStorage, plugin.getDataFolder(), name); } @Override @Contract("_ -> new") public WorldImporter importWorld(String name) { + validateWorldName(name); return new WorldImporterImpl(services.worldContext(), worldStorage, name); } + /** + * Rejects a world name that is reserved or would resolve outside the world container. Defense-in-depth: the + * menus sanitize names before they reach here, but both {@link #newWorld} and {@link #importWorld(String)} are + * public API, so a caller must not be able to create or import into a directory outside the world container + * (e.g. {@code plugins/}) by passing a crafted name. + */ + private static void validateWorldName(String name) { + if (StringCleaner.isReservedName(name)) { + throw new IllegalArgumentException("World name '%s' is reserved and cannot be used".formatted(name)); + } + File worldDirectory = new File(Bukkit.getWorldContainer(), name); + if (StringCleaner.isPathEscape(Bukkit.getWorldContainer(), worldDirectory)) { + throw new IllegalArgumentException("World name '%s' resolves outside the world container".formatted(name)); + } + } + public void startWorldNameInput( Player player, BuildWorldType worldType, @Nullable String template, - boolean privateWorld, - boolean promptSeed, + WorldNameInputOptions options, @Nullable Folder folder) { - this.creationPrompts.startWorldNameInput(player, worldType, template, privateWorld, promptSeed, folder); + Visibility visibility = Visibility.matchVisibility(options.privateWorld()); + if (!services.player().canCreateWorld(player, visibility)) { + messages.sendMessage(player, "worlds_create_limit_reached"); + return; + } + + this.creationPrompts.startWorldNameInput( + player, worldType, template, options.privateWorld(), options.promptSeed(), folder); } public boolean importWorld( @@ -176,9 +193,11 @@ public boolean importWorld( if (world == null) { return false; } + if (single) { world.getTeleporter().teleport(player); } + return true; } @@ -204,7 +223,18 @@ public CompletableFuture unimportWorld(BuildWorld buildWorld, SaveBehavior return this.worldStorage.delete(buildWorld); } + /** + * Deletes a world on behalf of a player, having first checked that they are allowed to. This overload exists + * for menu/command call sites where the player is the source of the request; {@link #deleteWorld(BuildWorld)} + * has no such check because it is the public API entry point, used by internal callers and API consumers who + * have either already authorized the request themselves or are not acting on behalf of a specific player. + */ public void deleteWorld(Player player, BuildWorld buildWorld) { + if (!buildWorld.getPermissions().canPerformCommand(player, Permissions.DELETE)) { + messages.sendPermissionError(player); + return; + } + String worldName = buildWorld.getName(); messages.sendMessage(player, "worlds_delete_started", Placeholders.of("%world%", worldName)); deleteWorld(buildWorld) @@ -224,7 +254,8 @@ public void deleteWorld(Player player, BuildWorld buildWorld) { plugin.getLogger() .log( Level.SEVERE, - "An unexpected error occurred while deleting the world: " + worldName, + "An unexpected error occurred while deleting the world: %s" + .formatted(worldName), cause); } } @@ -250,7 +281,7 @@ public CompletableFuture deleteWorld(BuildWorld buildWorld) { Bukkit.getServer().getPluginManager().callEvent(deleteEvent); if (deleteEvent.isCancelled()) { return CompletableFuture.failedFuture(new WorldDeletionCancelledException( - "Deletion of world '" + worldName + "' was cancelled by an event listener")); + "Deletion of world '%s' was cancelled by an event listener".formatted(worldName))); } buildWorld.setFolder(null); @@ -270,8 +301,8 @@ public CompletableFuture deleteWorld(BuildWorld buildWorld) { FileUtils.deleteDirectory(deleteFolder); } catch (IOException e) { throw new CompletionException(new WorldDeletionException( - "An unexpected error occurred during directory deletion for world: " - + worldName, + "An unexpected error occurred during directory deletion for world: %s" + .formatted(worldName), e)); } scheduler.runTask( diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupProfileImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupProfileImpl.java index e3d8ef89..82a5c319 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupProfileImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupProfileImpl.java @@ -104,13 +104,17 @@ public CompletableFuture> listBackups() { @Override public CompletableFuture createBackup() { - this.buildWorld.getWorld().ifPresent(World::save); - synchronized (this.creationLock) { // handle() before the compose: a failed backup must not poison every later backup of this world. + // World::save is main-thread-only and this is public API, so it must not run on the caller's thread. CompletableFuture next = this.pendingCreation .handle((backup, throwable) -> null) - .thenCompose(ignored -> storeWithRetention()); + .thenComposeAsync( + ignored -> { + this.buildWorld.getWorld().ifPresent(World::save); + return storeWithRetention(); + }, + mainThreadExecutor()); this.pendingCreation = next.handle((backup, throwable) -> backup); return next; } @@ -218,12 +222,19 @@ private void applyRestore( Location spawn = spawnService.getSpawn(); boolean isSpawn = spawn != null && Objects.equals(spawn.getWorld(), world); - this.buildWorld.getUnloader().forceUnload(SaveBehavior.DISCARD); File targetDirectory = FileUtils.worldFolder(worldName); + + // Must happen before the world is deleted: a corrupt archive would otherwise only be detected once there + // was nothing left to restore. + validateBackup(backupFile, targetDirectory); + + this.buildWorld.getUnloader().forceUnload(SaveBehavior.DISCARD); try { FileUtils.deleteDirectory(targetDirectory); } catch (IOException e) { - plugin.getLogger().log(Level.SEVERE, "Error while deleting world directory before restore", e); + // Extracting over a half-deleted world would produce a corrupt mix of both. + throw new IOException( + "Aborting restore: failed to delete world directory %s".formatted(targetDirectory), e); } if (!targetDirectory.isDirectory() && !targetDirectory.mkdirs()) { @@ -249,18 +260,40 @@ private void applyRestore( } /** - * Extracts a backup archive into {@code targetDirectory}, rejecting any entry whose resolved path escapes that - * directory (zip-slip / path traversal) before anything is written to disk. + * Checks that {@code backupFile} is a readable archive and that no entry's resolved path escapes + * {@code targetDirectory} (zip-slip / path traversal). + * + *

Called before the world is deleted, so a corrupt archive fails the restore while the world is still + * intact. Reading the central directory is what detects truncation. + * + * @param backupFile The downloaded archive + * @param targetDirectory The directory the archive would be extracted into + * @throws IOException If the archive cannot be read or an entry escapes the target directory */ - private void extractBackup(File backupFile, File targetDirectory) throws IOException { + private void validateBackup(File backupFile, File targetDirectory) throws IOException { try (ZipFile zip = new ZipFile(backupFile)) { - for (FileHeader header : zip.getFileHeaders()) { + List headers = zip.getFileHeaders(); + if (headers.isEmpty()) { + throw new IOException( + "Refusing to restore backup: archive contains no entries: %s".formatted(backupFile)); + } + + for (FileHeader header : headers) { File resolved = new File(targetDirectory, header.getFileName()); if (StringCleaner.isPathEscape(targetDirectory, resolved)) { - throw new IOException("Refusing to restore backup: archive entry escapes the world directory: " - + header.getFileName()); + throw new IOException("Refusing to restore backup: archive entry escapes the world directory: %s" + .formatted(header.getFileName())); } } + } + } + + /** + * Extracts a backup archive into {@code targetDirectory}. Entries are validated by + * {@link #validateBackup(File, File)} before the world is deleted. + */ + private void extractBackup(File backupFile, File targetDirectory) throws IOException { + try (ZipFile zip = new ZipFile(backupFile)) { zip.extractAll(targetDirectory.getPath()); } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java index 224d02ce..1f8283fa 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/BackupServiceImpl.java @@ -249,14 +249,29 @@ private void incrementTimeSinceBackup() { worlds.addAll(worldStorage.getBuildWorlds()); } - worlds.forEach(buildWorld -> { + boolean backedUpOneThisTick = false; + for (BuildWorld buildWorld : worlds) { WorldData worldData = buildWorld.getData(); int elapsed = worldData.get(WorldDataKey.TIME_SINCE_BACKUP) + (int) UPDATE_PERIOD_SECONDS; if (elapsed > autoBackup.interval()) { - getProfile(buildWorld).createBackup(); + if (backedUpOneThisTick) { + worldData.set(WorldDataKey.TIME_SINCE_BACKUP, elapsed); + continue; + } + backedUpOneThisTick = true; + autoBackup(buildWorld); elapsed = 0; } worldData.set(WorldDataKey.TIME_SINCE_BACKUP, elapsed); + } + } + + private void autoBackup(BuildWorld buildWorld) { + getProfile(buildWorld).createBackup().whenComplete((backup, throwable) -> { + if (throwable != null) { + String message = "Automatic backup failed for world '%s'".formatted(buildWorld.getName()); + plugin.getLogger().log(Level.SEVERE, message, throwable); + } }); } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/SftpBackupStorage.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/SftpBackupStorage.java index 9208dfb0..601e1500 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/SftpBackupStorage.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/SftpBackupStorage.java @@ -197,16 +197,23 @@ public synchronized CompletableFuture storeBackup(BuildWorld buildWorld) long timestamp = System.currentTimeMillis(); String backupDirectory = getBackupDirectory(buildWorld); String remotePath = backupDirectory + backupName(timestamp); + String tempRemotePath = remotePath + ".part"; byte[] zipBytes = FileUtils.zipWorldToMemory(buildWorld); SftpClient sftp = getSftpClient(); createDirectoryIfNotExists(sftp, backupDirectory); - try (OutputStream out = sftp.write(remotePath); - BufferedOutputStream bufferedOut = new BufferedOutputStream(out, BUFFER_SIZE)) { - bufferedOut.write(zipBytes); - bufferedOut.flush(); + try { + try (OutputStream out = sftp.write(tempRemotePath); + BufferedOutputStream bufferedOut = new BufferedOutputStream(out, BUFFER_SIZE)) { + bufferedOut.write(zipBytes); + bufferedOut.flush(); + } + sftp.rename(tempRemotePath, remotePath, SftpClient.CopyMode.Overwrite); + } catch (IOException e) { + removePartialUpload(sftp, tempRemotePath, e); + throw e; } logDuration(buildWorld, timestamp); @@ -214,6 +221,14 @@ public synchronized CompletableFuture storeBackup(BuildWorld buildWorld) }); } + private void removePartialUpload(SftpClient sftp, String tempRemotePath, IOException cause) { + try { + sftp.remove(tempRemotePath); + } catch (IOException suppressed) { + cause.addSuppressed(suppressed); + } + } + @Override public synchronized CompletableFuture downloadBackup(Backup backup) { return supply("download SFTP backup " + backup.key(), () -> { diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Client.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Client.java index 56aec43c..44a61e13 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Client.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/backup/storage/s3/S3Client.java @@ -133,7 +133,14 @@ public void put(String key, byte[] content) throws IOException { * @throws IOException If the request fails or S3 returns an error */ public void get(String key, Path target) throws IOException { - HttpResponse response = request("GET", key, Map.of(), Payload.empty(), BodyHandlers.ofFile(target)); + HttpResponse response; + try { + response = request("GET", key, Map.of(), Payload.empty(), BodyHandlers.ofFile(target)); + } catch (IOException e) { + Files.deleteIfExists(target); + throw e; + } + if (!isSuccess(response)) { // The handler has already written the error document to the target; it is not a backup. Files.deleteIfExists(target); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldDataImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldDataImpl.java index 11b3343d..4fac160a 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldDataImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldDataImpl.java @@ -23,8 +23,6 @@ import de.eintosti.buildsystem.api.world.data.WorldData; import de.eintosti.buildsystem.api.world.data.WorldDataKey; import de.eintosti.buildsystem.api.world.display.Folder; -import de.eintosti.buildsystem.util.Permissions; -import de.eintosti.buildsystem.world.data.type.Bypassable; import de.eintosti.buildsystem.world.data.type.ConfigurableProperty; import de.eintosti.buildsystem.world.data.type.Overridable; import de.eintosti.buildsystem.world.data.type.PersistentProperty; @@ -87,7 +85,6 @@ private WorldDataImpl(WorldDataBuilder builder) { register( WorldDataKey.PERMISSION, new ConfigurableProperty<>(builder.permission) - .withCapability(Bypassable.class, new Bypassable(Permissions.BYPASS_PERMISSION)) .withCapability( Overridable.class, folderOverride(builder.permissionOverrideEnabled, Folder::getPermission))); @@ -107,12 +104,11 @@ private WorldDataImpl(WorldDataBuilder builder) { register( WorldDataKey.STATUS, new ConfigurableProperty<>(Objects.requireNonNull(builder.status, "status")) - .withConfigFormatter(BuildWorldStatus::getId) - .withCapability(Bypassable.class, new Bypassable(Permissions.BYPASS_ARCHIVE))); + .withConfigFormatter(BuildWorldStatus::getId)); - register(WorldDataKey.BLOCK_BREAKING, settingsBypassable(builder.blockBreaking)); - register(WorldDataKey.BLOCK_INTERACTIONS, settingsBypassable(builder.blockInteractions)); - register(WorldDataKey.BLOCK_PLACEMENT, settingsBypassable(builder.blockPlacement)); + register(WorldDataKey.BLOCK_BREAKING, new ConfigurableProperty<>(builder.blockBreaking)); + register(WorldDataKey.BLOCK_INTERACTIONS, new ConfigurableProperty<>(builder.blockInteractions)); + register(WorldDataKey.BLOCK_PLACEMENT, new ConfigurableProperty<>(builder.blockPlacement)); register(WorldDataKey.BUILDERS_ENABLED, new ConfigurableProperty<>(builder.buildersEnabled)); register(WorldDataKey.EXPLOSIONS, new ConfigurableProperty<>(builder.explosions)); register(WorldDataKey.MOB_AI, new ConfigurableProperty<>(builder.mobAi)); @@ -170,14 +166,6 @@ private PersistentProperty property(WorldDataKey key) { return (PersistentProperty) property; } - /** - * Builds a boolean setting property that may be bypassed with the {@code buildsystem.bypass.settings} permission. - */ - private static ConfigurableProperty settingsBypassable(boolean defaultValue) { - return new ConfigurableProperty<>(defaultValue) - .withCapability(Bypassable.class, new Bypassable(Permissions.BYPASS_SETTINGS)); - } - /** * Builds an {@link Overridable} capability that draws its override value from this world's assigned folder, or * {@code null} when the world has no folder. diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldStatusRegistryImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldStatusRegistryImpl.java index 55570ee6..42fe9f51 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldStatusRegistryImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/data/WorldStatusRegistryImpl.java @@ -304,7 +304,9 @@ private int firstFreeSlot() { } public void persist(BuildWorldStatus status) { - // The registry only ever hands out its own instances, so anything it is asked to persist is one of them. + if (!this.statuses.containsKey(status.getId())) { + return; + } storage.save((WorldStatusImpl) status); } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/display/CategoryPermissions.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/display/CategoryPermissions.java index fe4cf614..d839a068 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/display/CategoryPermissions.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/display/CategoryPermissions.java @@ -27,7 +27,8 @@ * so an admin who hides a category from the navigator also disables its shortcut, and vice versa. * *

The node is default-allow: like the navigator, which shows every category to everyone out of the - * box, a player may access a category unless {@code buildsystem.navigator.} has been explicitly set to {@code false}. + * box, a player may access a category unless {@code buildsystem.navigator.category.} has been explicitly set to + * {@code false}. * It is consulted through {@link Player#isPermissionSet(String)} rather than registered with a default, because category * ids are dynamic and cannot be declared up front in {@code plugin.yml}. */ diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/display/NavigatorCategoryRegistryImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/display/NavigatorCategoryRegistryImpl.java index 76eef19f..90f59288 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/display/NavigatorCategoryRegistryImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/display/NavigatorCategoryRegistryImpl.java @@ -257,7 +257,9 @@ public NavigatorCategoryImpl create(String displayName) { } public void persist(NavigatorCategory category) { - // The registry only ever hands out its own instances, so anything it is asked to persist is one of them. + if (!this.categories.containsKey(category.getId())) { + return; + } storage.save((NavigatorCategoryImpl) category); } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/lifecycle/WorldPermissionsImpl.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/lifecycle/WorldPermissionsImpl.java index 04d0e1fe..6426570e 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/lifecycle/WorldPermissionsImpl.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/lifecycle/WorldPermissionsImpl.java @@ -86,7 +86,7 @@ private boolean evaluateModify(Player player, @Nullable WorldSetting setting) { return true; } - if (hasAdminPermission(player) || canBypassBuildRestriction(player)) { + if (canBypassBuildRestriction(player) || hasAdminPermission(player)) { return true; } @@ -96,12 +96,12 @@ private boolean evaluateModify(Player player, @Nullable WorldSetting setting) { } if (setting != null) { + if (!setting.isEnabled(buildWorld.getData())) { + return player.hasPermission(setting.getBypassPermission()); + } if (player.hasPermission(setting.getBypassPermission())) { return true; } - if (!setting.isEnabled(buildWorld.getData())) { - return false; - } } Builders builders = buildWorld.getBuilders(); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/BackupsConfirmationMenu.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/BackupsConfirmationMenu.java index 63e2c29c..22635bb2 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/BackupsConfirmationMenu.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/BackupsConfirmationMenu.java @@ -25,6 +25,7 @@ import de.eintosti.buildsystem.menu.ButtonMenu; import de.eintosti.buildsystem.menu.ItemBuilder; import de.eintosti.buildsystem.menu.MenuButton; +import de.eintosti.buildsystem.util.Permissions; import org.bukkit.entity.Player; import org.jspecify.annotations.NullMarked; @@ -34,11 +35,8 @@ public class BackupsConfirmationMenu extends ButtonMenu { private static final int SLOT_CONFIRM = 11; private static final int SLOT_CANCEL = 15; - private final Backup backup; - public BackupsConfirmationMenu(Messages messages, Backup backup, Player player) { super(messages, 27, messages.getString("restore_backup_title", player)); - this.backup = backup; register( SLOT_CONFIRM, @@ -50,6 +48,7 @@ public BackupsConfirmationMenu(Messages messages, Backup backup, Player player) p, Placeholders.of("%timestamp%", messages.formatDateTime(backup.creationTime())))) .into(inventory, slot)) + .permission(Permissions.BACKUP) .onClick((p, event) -> { p.closeInventory(); XSound.ENTITY_PLAYER_LEVELUP.play(p); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/BuilderMenu.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/BuilderMenu.java index bee6b0c2..0e7e710d 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/BuilderMenu.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/BuilderMenu.java @@ -129,11 +129,14 @@ private MenuButton creatorInfoButton() { .build(); } + private boolean canManageBuilders(Player player) { + return buildWorld.getBuilders().isCreator(player) || player.hasPermission(BuildSystemPlugin.ADMIN_PERMISSION); + } + private MenuButton addBuilderButton() { return MenuButton.builder() .render((player, inventory, slot) -> { - if (buildWorld.getBuilders().isCreator(player) - || player.hasPermission(BuildSystemPlugin.ADMIN_PERMISSION)) { + if (canManageBuilders(player)) { inventory.setItem( slot, ItemBuilder.skull(Profileable.detect(SkullTextures.ADD_ITEM)) @@ -146,12 +149,8 @@ private MenuButton addBuilderButton() { .build()); } }) + .usableBy(this::canManageBuilders) .onClick((player, event) -> { - if (event.getCurrentItem() == null - || event.getCurrentItem().getType() != XMaterial.PLAYER_HEAD.get()) { - returnToEditor(player); - return; - } XSound.ENTITY_CHICKEN_EGG.play(player); menus.promptAddBuilder(buildWorld, player); }) @@ -170,6 +169,7 @@ private MenuButton builderButton(Builder builder) { .lore(messages.getStringList("worldeditor_builders_builder_lore", player)) .pdc(this.builderNameKey, PersistentDataType.STRING, builder.getName()) .build())) + .usableBy(this::canManageBuilders) .onClick((player, event) -> { // Only a shift-click removes a builder; a plain click returns to the editor. if (!event.isShiftClick()) { @@ -186,6 +186,16 @@ protected void onUnhandledClick(Player player, InventoryClickEvent event) { returnToEditor(player); } + /** + * Sends a player who may not manage builders back to the editor instead of closing the inventory with an error. + * They see a filler pane rather than the add-builder head, so a click on it reads as "go back", not as an attempt + * to do something forbidden. + */ + @Override + protected void onPermissionDenied(Player player, InventoryClickEvent event) { + returnToEditor(player); + } + private void returnToEditor(Player player) { if (buildWorld.getPermissions().canPerformCommand(player, WorldsArgument.EDIT.getPermission())) { XSound.BLOCK_CHEST_OPEN.play(player); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/CategoryWorldsMenu.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/CategoryWorldsMenu.java index 721948a0..078b58e9 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/CategoryWorldsMenu.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/CategoryWorldsMenu.java @@ -38,7 +38,8 @@ *

World creation is offered dynamically: a category shows the "create world" item only when a freshly created world * — which always starts at the registry's {@link de.eintosti.buildsystem.api.world.data.WorldStatusRegistry#getDefault() * default status} — would actually be grouped into this category, and the player holds the per-category create - * permission {@code buildsystem.create.} (e.g. {@code buildsystem.create.public}). This is why the archive + * permission {@code buildsystem.create.category.} (e.g. {@code buildsystem.create.category.public}). This + * is why the archive * category, which never contains the default status, never offers world creation. */ @NullMarked @@ -47,9 +48,7 @@ public class CategoryWorldsMenu extends DisplayablesMenu { static final String CREATE_WORLD_PROFILE = SkullTextures.ADD_ITEM; static final String CREATE_FOLDER_PROFILE = "69b861aabb316c4ed73b4e5428305782e735565ba2a053912e1efd834fa5a6f"; - private static final int SLOT_CREATE_WORLD = 48; - private static final int SLOT_CREATE_FOLDER = 50; - private static final int SLOT_CREATE_CENTER = 49; + private static final int SLOT_CREATE_CENTER = FIRST_CREATE_FOLDER_SLOT; private final WorldStatusRegistryImpl worldStatusRegistry; @@ -79,7 +78,7 @@ protected void addExtraItems(Inventory inventory, Player player) { } if (player.hasPermission(Permissions.CREATE_FOLDER)) { // With the create-world button hidden, centre the lone folder button instead of leaving it off to the side. - int folderSlot = createWorld ? SLOT_CREATE_FOLDER : SLOT_CREATE_CENTER; + int folderSlot = createWorld ? LAST_CREATE_FOLDER_SLOT : SLOT_CREATE_CENTER; ItemBuilder.skull(Profileable.detect(CREATE_FOLDER_PROFILE)) .name(messages.getString("world_navigator_create_folder", player)) .into(inventory, folderSlot); @@ -100,7 +99,8 @@ private boolean canCreateWorldHere(Player player) { /** * Admins may create worlds in any category; everyone else needs the per-category create node - * {@code buildsystem.create.}. This mirrors how the admin permission grants an unlimited world count. + * {@code buildsystem.create.category.}. This mirrors how the admin permission grants an unlimited + * world count. */ private boolean hasCreatePermission(Player player) { return player.hasPermission(BuildSystemPlugin.ADMIN_PERMISSION) diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/CreateMenu.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/CreateMenu.java index f01f1cab..2d1dfbe2 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/CreateMenu.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/CreateMenu.java @@ -33,6 +33,7 @@ import de.eintosti.buildsystem.menu.SkullTextures; import de.eintosti.buildsystem.util.FileUtils; import de.eintosti.buildsystem.util.Permissions; +import de.eintosti.buildsystem.world.WorldNameInputOptions; import de.eintosti.buildsystem.world.WorldServiceImpl; import de.eintosti.buildsystem.world.display.CustomizableIcons; import java.io.File; @@ -41,6 +42,7 @@ import org.bukkit.ChatColor; import org.bukkit.Material; import org.bukkit.entity.Player; +import org.bukkit.event.inventory.InventoryClickEvent; import org.bukkit.inventory.ItemStack; import org.jspecify.annotations.NullMarked; import org.jspecify.annotations.Nullable; @@ -165,7 +167,8 @@ private MenuButton predefinedButton(BuildWorldType worldType) { Material material = canCreate ? customizableIcons.getIcon(worldType) : Material.BARRIER; String displayName = messages.getString(PREDEFINED_MESSAGE_KEYS.get(worldType), player); if (!canCreate) { - displayName = "§c§m" + ChatColor.stripColor(displayName); + displayName = "%s%s%s" + .formatted(ChatColor.RED, ChatColor.STRIKETHROUGH, ChatColor.stripColor(displayName)); } ItemBuilder itemBuilder = ItemBuilder.of(material).name(displayName); if (canCreate) { @@ -173,13 +176,14 @@ private MenuButton predefinedButton(BuildWorldType worldType) { } itemBuilder.into(inventory, slot); }) + .usableBy(player -> canCreateType(player, worldType)) .onClick((player, event) -> { - if (!canCreateType(player, worldType)) { - XSound.ENTITY_ITEM_BREAK.play(player); - return; - } worldService.startWorldNameInput( - player, worldType, null, createPrivateWorld, event.isShiftClick(), folder); + player, + worldType, + null, + new WorldNameInputOptions(createPrivateWorld, event.isShiftClick()), + folder); XSound.ENTITY_CHICKEN_EGG.play(player); }) .build(); @@ -189,6 +193,24 @@ private static boolean canCreateType(Player player, BuildWorldType worldType) { return player.hasPermission(Permissions.createType(worldType.name())); } + /** + * Template names are dynamic and cannot be pre-registered in {@code plugin.yml}, so default-allow is emulated: a + * template is permitted unless an admin has explicitly denied its specific node. + */ + private static boolean isTemplateAllowed(Player player, String rawTemplateName) { + String templateNode = Permissions.createTemplate(rawTemplateName); + return !player.isPermissionSet(templateNode) || player.hasPermission(templateNode); + } + + /** + * Plays the deny sound but keeps the menu open, so a click on a barred world type or template does not eject the + * player from the creation flow. + */ + @Override + protected void onPermissionDenied(Player player, InventoryClickEvent event) { + XSound.ENTITY_ITEM_BREAK.play(player); + } + private void registerGenerator(Player player) { for (int slot = FIRST_PREDEFINED_SLOT; slot <= LAST_PREDEFINED_SLOT; slot++) { if (slot != SLOT_GENERATOR_CREATE) { @@ -203,7 +225,11 @@ private void registerGenerator(Player player) { .into(inventory, slot)) .onClick((p, event) -> { worldService.startWorldNameInput( - p, BuildWorldType.CUSTOM, null, createPrivateWorld, false, folder); + p, + BuildWorldType.CUSTOM, + null, + new WorldNameInputOptions(createPrivateWorld, false), + folder); XSound.ENTITY_CHICKEN_EGG.play(p); }) .build()); @@ -243,15 +269,8 @@ private MenuButton templateButton(String rawTemplateName) { .name(messages.getString( "create_template", player, Placeholders.of("%template%", rawTemplateName))) .into(inventory, slot)) + .usableBy(player -> isTemplateAllowed(player, rawTemplateName)) .onClick((player, event) -> { - // Template names are dynamic and cannot be pre-registered in plugin.yml, so default-allow is - // emulated: a template is permitted unless an admin has explicitly denied its specific node. - String templateNode = Permissions.createTemplate(rawTemplateName); - if (player.isPermissionSet(templateNode) && !player.hasPermission(templateNode)) { - XSound.ENTITY_ITEM_BREAK.play(player); - return; - } - ItemStack itemStack = event.getCurrentItem(); if (itemStack == null || itemStack.getItemMeta() == null) { return; @@ -261,8 +280,7 @@ private MenuButton templateButton(String rawTemplateName) { player, BuildWorldType.TEMPLATE, itemStack.getItemMeta().getDisplayName(), - createPrivateWorld, - false, + new WorldNameInputOptions(createPrivateWorld, false), folder); }) .build(); diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/DisplayablesMenu.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/DisplayablesMenu.java index 888fbab0..2b378099 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/DisplayablesMenu.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/DisplayablesMenu.java @@ -50,21 +50,21 @@ @NullMarked public abstract class DisplayablesMenu extends PaginatedMenu { - private static final int MAX_WORLDS_PER_PAGE = 36; - private static final int FIRST_WORLD_SLOT = 9; - private static final int LAST_WORLD_SLOT = 44; - - private static final int SLOT_NO_WORLDS = 22; - private static final int SLOT_WORLD_SORT = 45; - private static final int SLOT_WORLD_FILTER = 46; - private static final int SLOT_CREATE_WORLD = 48; - private static final int FIRST_CREATE_FOLDER_SLOT = 49; - private static final int LAST_CREATE_FOLDER_SLOT = 50; - private static final int SLOT_BACK = 51; - private static final int SLOT_PREVIOUS_PAGE = 52; - private static final int SLOT_NEXT_PAGE = 53; - private static final int FIRST_BOTTOM_BAR_SLOT = 45; - private static final int LAST_BOTTOM_BAR_SLOT = 53; + protected static final int MAX_WORLDS_PER_PAGE = 36; + protected static final int FIRST_WORLD_SLOT = 9; + protected static final int LAST_WORLD_SLOT = 44; + + protected static final int SLOT_NO_WORLDS = 22; + protected static final int SLOT_WORLD_SORT = 45; + protected static final int SLOT_WORLD_FILTER = 46; + protected static final int SLOT_CREATE_WORLD = 48; + protected static final int FIRST_CREATE_FOLDER_SLOT = 49; + protected static final int LAST_CREATE_FOLDER_SLOT = 50; + protected static final int SLOT_BACK = 51; + protected static final int SLOT_PREVIOUS_PAGE = 52; + protected static final int SLOT_NEXT_PAGE = 53; + protected static final int FIRST_BOTTOM_BAR_SLOT = 45; + protected static final int LAST_BOTTOM_BAR_SLOT = 53; private static final String NO_WORLDS_SKULL_PROFILE = "2e3f50ba62cbda3ecf5479b62fedebd61d76589771cc19286bf2745cd71e47c6"; @@ -85,6 +85,8 @@ public abstract class DisplayablesMenu extends PaginatedMenu { private final @Nullable String noWorldsMessage; private @Nullable List cachedDisplayables; + private final DisplayBar displayBar = new DisplayBar(); + protected DisplayablesMenu(DisplayablesContext context, Player player, Options options) { super(context.messages(), 54, options.title()); this.playerService = context.playerService(); @@ -163,15 +165,32 @@ protected int totalItems() { return cachedDisplayables != null ? cachedDisplayables.size() : 0; } + /** + * Recomputes {@link #cachedDisplayables} from the current folder/world storage state. Called on every {@link #open}, + * so navigating to or reopening this menu always reflects the latest data. + * + *

A page flip does not go through here: {@link PaginatedMenu}'s page-arrow buttons call {@link #populate} directly, + * which re-renders the already-collected list for the new page instead of recollecting and re-filtering everything. + */ + private void refreshData() { + this.cachedDisplayables = collectDisplayables(); + } + + @Override + public void open(Player player) { + refreshData(); + super.open(player); + } + @Override protected void populate(Player player) { - this.cachedDisplayables = collectDisplayables(); + List displayables = cachedDisplayables != null ? cachedDisplayables : List.of(); Inventory inv = getInventory(); clearButtons(); menuItems.fillWithGlass(inv, player); - addWorldSortItem(inv); - addWorldFilterItem(inv); + displayBar.renderSort(inv); + displayBar.renderFilter(inv); addExtraItems(inv, player); register(SLOT_PREVIOUS_PAGE, previousPageButton(SkullTextures.PREVIOUS_PAGE, MAX_WORLDS_PER_PAGE)); register(SLOT_NEXT_PAGE, nextPageButton(SkullTextures.NEXT_PAGE, MAX_WORLDS_PER_PAGE)); @@ -180,12 +199,12 @@ protected void populate(Player player) { inv.setItem(i, null); } - if (cachedDisplayables.isEmpty() && noWorldsMessage != null) { + if (displayables.isEmpty() && noWorldsMessage != null) { ItemBuilder.skull(Profileable.detect(NO_WORLDS_SKULL_PROFILE)) .name(noWorldsMessage) .into(inv, SLOT_NO_WORLDS); } else { - registerPageItems(FIRST_WORLD_SLOT, MAX_WORLDS_PER_PAGE, cachedDisplayables, this::displayableButton); + registerPageItems(FIRST_WORLD_SLOT, MAX_WORLDS_PER_PAGE, displayables, this::displayableButton); } renderButtons(player); @@ -257,50 +276,6 @@ private boolean isWorldValidForDisplay(BuildWorld buildWorld) { return Bukkit.getWorld(buildWorld.getName()) != null || !buildWorld.isLoaded(); } - private void addWorldSortItem(Inventory inventory) { - Settings settings = settingsManager.getSettings(player); - WorldSort worldSort = settings.getWorldDisplay().getWorldSort(); - - String messageKey = - switch (worldSort) { - case NAME_A_TO_Z -> "world_sort_name_az"; - case NAME_Z_TO_A -> "world_sort_name_za"; - case PROJECT_A_TO_Z -> "world_sort_project_az"; - case PROJECT_Z_TO_A -> "world_sort_project_za"; - case STATUS_NOT_STARTED -> "world_sort_status_not_started"; - case STATUS_FINISHED -> "world_sort_status_finished"; - case NEWEST_FIRST -> "world_sort_date_newest"; - case OLDEST_FIRST -> "world_sort_date_oldest"; - }; - - ItemBuilder.of(XMaterial.BOOK) - .name(messages.getString("world_sort_title", player)) - .lore(messages.getString(messageKey, player)) - .into(inventory, SLOT_WORLD_SORT); - } - - private void addWorldFilterItem(Inventory inventory) { - Settings settings = settingsManager.getSettings(player); - WorldFilter worldFilter = settings.getWorldDisplay().getWorldFilter(); - - String loreKey = - switch (worldFilter.getMode()) { - case NONE -> "world_filter_mode_none"; - case STARTS_WITH -> "world_filter_mode_starts_with"; - case CONTAINS -> "world_filter_mode_contains"; - case MATCHES -> "world_filter_mode_matches"; - }; - - List lore = new ArrayList<>(); - lore.add(messages.getString(loreKey, player, Placeholders.of("%text%", worldFilter.getText()))); - lore.addAll(messages.getStringList("world_filter_lore", player)); - - ItemBuilder.of(XMaterial.HOPPER) - .name(messages.getString("world_filter_title", player)) - .lore(lore) - .into(inventory, SLOT_WORLD_FILTER); - } - @Override protected void onUnhandledClick(Player player, InventoryClickEvent event) { // Page arrows are registered buttons; everything else is handled here. Ignore clicks outside this inventory. @@ -317,29 +292,11 @@ protected void onUnhandledClick(Player player, InventoryClickEvent event) { WorldDisplay worldDisplay = settings.getWorldDisplay(); switch (slot) { - case SLOT_WORLD_SORT -> { - WorldSort currentSort = worldDisplay.getWorldSort(); - worldDisplay.setWorldSort(event.isLeftClick() ? currentSort.getNext() : currentSort.getPrevious()); - resetPage(); - open(player); - } - case SLOT_WORLD_FILTER -> handleFilterClick(event, worldDisplay); - case SLOT_CREATE_WORLD -> { - if (itemStack.getType() == XMaterial.PLAYER_HEAD.get()) { - XSound.ENTITY_CHICKEN_EGG.play(player); - beginWorldCreation(); - return; - } - goBack(player, itemStack); - } - case FIRST_CREATE_FOLDER_SLOT, LAST_CREATE_FOLDER_SLOT -> { - if (itemStack.getType() == XMaterial.PLAYER_HEAD.get()) { - XSound.ENTITY_CHICKEN_EGG.play(player); - beginFolderCreation(player); - return; - } - goBack(player, itemStack); - } + case SLOT_WORLD_SORT -> displayBar.handleSortClick(event, worldDisplay); + case SLOT_WORLD_FILTER -> displayBar.handleFilterClick(event, worldDisplay); + case SLOT_CREATE_WORLD -> handleCreateButtonClick(itemStack, this::beginWorldCreation); + case FIRST_CREATE_FOLDER_SLOT, LAST_CREATE_FOLDER_SLOT -> + handleCreateButtonClick(itemStack, () -> beginFolderCreation(player)); case SLOT_BACK -> goBack(player, itemStack); default -> { if (slot >= FIRST_BOTTOM_BAR_SLOT && slot <= LAST_BOTTOM_BAR_SLOT) { @@ -349,6 +306,20 @@ protected void onUnhandledClick(Player player, InventoryClickEvent event) { } } + /** + * Runs {@code onCreate} for a click on a create-world/create-folder slot, but only when that slot actually holds + * the create button. The same slot renders as filler glass instead when the player lacks permission or the + * category doesn't offer creation, and glass in the bottom bar behaves like every other slot there: it goes back. + */ + private void handleCreateButtonClick(ItemStack itemStack, Runnable onCreate) { + if (itemStack.getType() == XMaterial.PLAYER_HEAD.get()) { + XSound.ENTITY_CHICKEN_EGG.play(player); + onCreate.run(); + return; + } + goBack(player, itemStack); + } + private void goBack(Player player, ItemStack itemStack) { if (itemStack.getType() != XMaterial.PLAYER_HEAD.get()) { XSound.BLOCK_CHEST_OPEN.play(player); @@ -387,32 +358,6 @@ protected void returnToPreviousInventory() { menus.openNavigator(this.player); } - private void handleFilterClick(InventoryClickEvent event, WorldDisplay worldDisplay) { - WorldFilter worldFilter = worldDisplay.getWorldFilter(); - Mode currentMode = worldFilter.getMode(); - - if (event.isShiftClick()) { - worldFilter.setMode(Mode.NONE); - worldFilter.setText(""); - } else if (event.isLeftClick()) { - player.closeInventory(); - prompts.prompt(player) - .title("world_filter_title") - .onCancel(() -> open(player)) - .request(input -> { - worldFilter.setText(input.replace("\"", "")); - resetPage(); - open(player); - }); - return; - } else if (event.isRightClick()) { - worldFilter.setMode(currentMode.getNext()); - } - - resetPage(); - open(player); - } - private void manageWorldItemClick(InventoryClickEvent event, BuildWorld buildWorld) { Player player = (Player) event.getWhoClicked(); if (event.isLeftClick() @@ -431,4 +376,90 @@ private void manageWorldItemClick(InventoryClickEvent event, BuildWorld buildWor player.sendTitle(" ", messages.getString("world_not_loaded", player), 5, 70, 20); } } + + /** + * The sort and filter items in the bottom bar of every {@link DisplayablesMenu}: their rendering reflects the + * player's current {@link WorldDisplay} settings, and clicking them cycles or edits those settings before + * refreshing the menu. Kept as an inner class, rather than threading callbacks through a standalone one, since + * every operation here ends by calling back into the enclosing menu's {@link #resetPage()} and {@link #open}. + */ + private final class DisplayBar { + + void renderSort(Inventory inventory) { + Settings settings = settingsManager.getSettings(player); + WorldSort worldSort = settings.getWorldDisplay().getWorldSort(); + + String messageKey = + switch (worldSort) { + case NAME_A_TO_Z -> "world_sort_name_az"; + case NAME_Z_TO_A -> "world_sort_name_za"; + case PROJECT_A_TO_Z -> "world_sort_project_az"; + case PROJECT_Z_TO_A -> "world_sort_project_za"; + case STATUS_NOT_STARTED -> "world_sort_status_not_started"; + case STATUS_FINISHED -> "world_sort_status_finished"; + case NEWEST_FIRST -> "world_sort_date_newest"; + case OLDEST_FIRST -> "world_sort_date_oldest"; + }; + + ItemBuilder.of(XMaterial.BOOK) + .name(messages.getString("world_sort_title", player)) + .lore(messages.getString(messageKey, player)) + .into(inventory, SLOT_WORLD_SORT); + } + + void renderFilter(Inventory inventory) { + Settings settings = settingsManager.getSettings(player); + WorldFilter worldFilter = settings.getWorldDisplay().getWorldFilter(); + + String loreKey = + switch (worldFilter.getMode()) { + case NONE -> "world_filter_mode_none"; + case STARTS_WITH -> "world_filter_mode_starts_with"; + case CONTAINS -> "world_filter_mode_contains"; + case MATCHES -> "world_filter_mode_matches"; + }; + + List lore = new ArrayList<>(); + lore.add(messages.getString(loreKey, player, Placeholders.of("%text%", worldFilter.getText()))); + lore.addAll(messages.getStringList("world_filter_lore", player)); + + ItemBuilder.of(XMaterial.HOPPER) + .name(messages.getString("world_filter_title", player)) + .lore(lore) + .into(inventory, SLOT_WORLD_FILTER); + } + + void handleSortClick(InventoryClickEvent event, WorldDisplay worldDisplay) { + WorldSort currentSort = worldDisplay.getWorldSort(); + worldDisplay.setWorldSort(event.isLeftClick() ? currentSort.getNext() : currentSort.getPrevious()); + resetPage(); + open(player); + } + + void handleFilterClick(InventoryClickEvent event, WorldDisplay worldDisplay) { + WorldFilter worldFilter = worldDisplay.getWorldFilter(); + Mode currentMode = worldFilter.getMode(); + + if (event.isShiftClick()) { + worldFilter.setMode(Mode.NONE); + worldFilter.setText(""); + } else if (event.isLeftClick()) { + player.closeInventory(); + prompts.prompt(player) + .title("world_filter_title") + .onCancel(() -> open(player)) + .request(input -> { + worldFilter.setText(input.replace("\"", "")); + resetPage(); + open(player); + }); + return; + } else if (event.isRightClick()) { + worldFilter.setMode(currentMode.getNext()); + } + + resetPage(); + open(player); + } + } } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/EditMenu.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/EditMenu.java index 82d4fd2f..57a76cfd 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/EditMenu.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/EditMenu.java @@ -23,7 +23,6 @@ import com.google.common.collect.Sets; import de.eintosti.buildsystem.BuildSystemPlugin; import de.eintosti.buildsystem.api.world.BuildWorld; -import de.eintosti.buildsystem.api.world.data.BuildWorldStatus; import de.eintosti.buildsystem.api.world.data.Visibility; import de.eintosti.buildsystem.api.world.data.WorldData; import de.eintosti.buildsystem.api.world.data.WorldDataKey; @@ -34,7 +33,6 @@ import de.eintosti.buildsystem.menu.*; import de.eintosti.buildsystem.player.PlayerServiceImpl; import de.eintosti.buildsystem.util.Permissions; -import de.eintosti.buildsystem.util.color.ColorAPI; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -165,6 +163,7 @@ EditButton build() { private final Prompts prompts; private final Menus menus; private final BuildWorld buildWorld; + private final EditMenuRenderer renderer; public EditMenu( Messages messages, @@ -182,6 +181,7 @@ public EditMenu( this.prompts = prompts; this.menus = menus; this.buildWorld = buildWorld; + this.renderer = new EditMenuRenderer(messages, menuItems, configService, buildWorld); buildButtons(); } @@ -191,7 +191,7 @@ private void buildButtons() { EditButton.builder() .permission(Permissions.EDIT_ICON) .outcome(ClickOutcome.SUBMENU) - .render(this::renderWorldInfo) + .render((player, inventory) -> renderer.renderWorldInfo(player, inventory, SLOT_WORLD_INFO)) .onClick(this::onWorldInfoClick) .build()); @@ -213,7 +213,7 @@ private void buildButtons() { EditButton.builder() .permission(Permissions.EDIT_TIME) .outcome(ClickOutcome.REOPEN) - .render(this::renderTime) + .render((player, inventory) -> renderer.renderTime(player, inventory, SLOT_TIME)) .onClick((player, event) -> { changeTime(player); reopen(player); @@ -225,7 +225,7 @@ private void buildButtons() { EditButton.builder() .permission(Permissions.EDIT_ENTITIES) .outcome(ClickOutcome.CLOSE) - .render(this::renderButcher) + .render((player, inventory) -> renderer.renderButcher(player, inventory, SLOT_BUTCHER)) .onClick((player, event) -> removeEntities(player)) .build()); @@ -268,7 +268,7 @@ private void buildButtons() { EditButton.builder() .permission(Permissions.EDIT_GAMERULES) .outcome(ClickOutcome.SUBMENU) - .render(this::renderGameRules) + .render((player, inventory) -> renderer.renderGameRules(player, inventory, SLOT_GAMERULES)) .onClick((player, event) -> { XSound.BLOCK_CHEST_OPEN.play(player); menus.openGameRules(buildWorld, player); @@ -280,7 +280,7 @@ private void buildButtons() { EditButton.builder() .permission(Permissions.EDIT_DIFFICULTY) .outcome(ClickOutcome.REOPEN) - .render(this::renderDifficulty) + .render((player, inventory) -> renderer.renderDifficulty(player, inventory, SLOT_DIFFICULTY)) .onClick((player, event) -> { cycleDifficulty(); reopen(player); @@ -292,7 +292,7 @@ private void buildButtons() { EditButton.builder() .permission(Permissions.EDIT_STATUS) .outcome(ClickOutcome.SUBMENU) - .render(this::renderStatus) + .render((player, inventory) -> renderer.renderStatus(player, inventory, SLOT_STATUS)) .onClick((player, event) -> { XSound.ENTITY_CHICKEN_EGG.play(player); menus.openStatus(buildWorld, player); @@ -304,7 +304,7 @@ private void buildButtons() { EditButton.builder() .permission(Permissions.EDIT_PROJECT) .outcome(ClickOutcome.INPUT) - .render(this::renderProject) + .render((player, inventory) -> renderer.renderProject(player, inventory, SLOT_PROJECT)) .onClick((player, event) -> { XSound.ENTITY_CHICKEN_EGG.play(player); menus.promptWorldProject(buildWorld, player); @@ -316,7 +316,7 @@ private void buildButtons() { EditButton.builder() .permission(Permissions.EDIT_PERMISSION) .outcome(ClickOutcome.INPUT) - .render(this::renderPermission) + .render((player, inventory) -> renderer.renderPermission(player, inventory, SLOT_PERMISSION)) .onClick((player, event) -> { XSound.ENTITY_CHICKEN_EGG.play(player); menus.promptWorldPermission(buildWorld, player); @@ -330,17 +330,6 @@ protected void populate(Player player) { renderButtons(player); } - private void renderWorldInfo(Player player, Inventory inventory) { - String displayName = - messages.getString("worldeditor_world_item", player, Placeholders.of("%world%", buildWorld.getName())); - boolean isHead = buildWorld.getIcon() == Material.PLAYER_HEAD; - String loreKey = isHead ? "worldeditor_world_head_lore" : "worldeditor_world_lore"; - ItemBuilder.icon(buildWorld, player) - .name(displayName) - .lore(messages.getStringList(loreKey, player, Placeholders.of("%texture%", iconTextureLabel(player)))) - .into(inventory, SLOT_WORLD_INFO); - } - /** * The world-icon button mirrors the category icon control: left-click opens the item picker to choose the material, * and when that material is a player head a right-click prompts for the head texture (a texture, {@code viewer} for @@ -351,6 +340,7 @@ private void onWorldInfoClick(Player player, InventoryClickEvent event) { promptIconTexture(player); return; } + XSound.BLOCK_CHEST_OPEN.play(player); menus.openMaterialPicker( player, @@ -382,50 +372,12 @@ private void applyIconTexture(String rawInput) { } } - private String iconTextureLabel(Player player) { - String texture = buildWorld.getIconSkullTexture(); - if (texture == null || texture.isBlank()) { - return messages.getString("worldeditor_world_skull_none", player); - } - if (ItemBuilder.VIEWER_HEAD.equals(texture)) { - return messages.getString("worldeditor_world_skull_viewer", player); - } - return messages.getString("worldeditor_world_skull_custom", player); - } - - private void renderTime(Player player, Inventory inventory) { - XMaterial material; - String value; - switch (getWorldTime()) { - case NIGHT -> { - material = XMaterial.BLUE_STAINED_GLASS; - value = messages.getString("worldeditor_time_lore_night", player); - } - case NOON -> { - material = XMaterial.YELLOW_STAINED_GLASS; - value = messages.getString("worldeditor_time_lore_noon", player); - } - default -> { - material = XMaterial.ORANGE_STAINED_GLASS; - value = messages.getString("worldeditor_time_lore_sunrise", player); - } - } - - ItemBuilder.of(material) - .name(messages.getString("worldeditor_time_item", player)) - .lore(messages.getStringList("worldeditor_time_lore", player, Placeholders.of("%time%", value))) - .into(inventory, SLOT_TIME); - } - - private void renderButcher(Player player, Inventory inventory) { - ItemBuilder.of(XMaterial.DIAMOND_SWORD) - .name(messages.getString("worldeditor_butcher_item", player)) - .lore(messages.getStringList("worldeditor_butcher_lore", player)) - .into(inventory, SLOT_BUTCHER); + private boolean canManageBuilders(Player player) { + return buildWorld.getBuilders().isCreator(player) || player.hasPermission(BuildSystemPlugin.ADMIN_PERMISSION); } private void renderBuilders(Player player, Inventory inventory) { - if (buildWorld.getBuilders().isCreator(player) || player.hasPermission(BuildSystemPlugin.ADMIN_PERMISSION)) { + if (canManageBuilders(player)) { menuItems.addToggleItem( player, inventory, @@ -442,11 +394,15 @@ private void renderBuilders(Player player, Inventory inventory) { } } + private boolean canChangeVisibility(Player player, boolean isPrivate) { + return playerManager.canCreateWorld(player, Visibility.matchVisibility(isPrivate)); + } + private void renderVisibility(Player player, Inventory inventory) { String displayName = messages.getString("worldeditor_visibility_item", player); boolean isPrivate = buildWorld.getData().get(WorldDataKey.VISIBILITY).isPrivate(); - if (!playerManager.canCreateWorld(player, Visibility.matchVisibility(isPrivate))) { + if (!canChangeVisibility(player, isPrivate)) { ItemBuilder.of(XMaterial.BARRIER) .name("§c§m" + ChatColor.stripColor(displayName)) .into(inventory, SLOT_VISIBILITY); @@ -460,71 +416,6 @@ private void renderVisibility(Player player, Inventory inventory) { ItemBuilder.of(material).name(displayName).lore(lore).into(inventory, SLOT_VISIBILITY); } - private void renderGameRules(Player player, Inventory inventory) { - ItemBuilder.of(XMaterial.FILLED_MAP) - .name(messages.getString("worldeditor_gamerules_item", player)) - .lore(messages.getStringList("worldeditor_gamerules_lore", player)) - .into(inventory, SLOT_GAMERULES); - } - - private void renderDifficulty(Player player, Inventory inventory) { - XMaterial material = - switch (buildWorld.getData().get(WorldDataKey.DIFFICULTY)) { - case EASY -> XMaterial.GOLDEN_HELMET; - case NORMAL -> XMaterial.IRON_HELMET; - case HARD -> XMaterial.DIAMOND_HELMET; - default -> XMaterial.LEATHER_HELMET; - }; - - ItemBuilder.of(material) - .name(messages.getString("worldeditor_difficulty_item", player)) - .lore(messages.getStringList( - "worldeditor_difficulty_lore", - player, - Placeholders.of("%difficulty%", getDifficultyName(player)))) - .into(inventory, SLOT_DIFFICULTY); - } - - private void renderStatus(Player player, Inventory inventory) { - BuildWorldStatus status = buildWorld.getData().get(WorldDataKey.STATUS); - ItemBuilder.of(status.getIcon()) - .name(messages.getString("worldeditor_status_item", player)) - .lore(messages.getStringList( - "worldeditor_status_lore", - player, - Placeholders.of("%status%", ColorAPI.process(status.getStyledName())))) - .into(inventory, SLOT_STATUS); - } - - private void renderProject(Player player, Inventory inventory) { - ItemBuilder.of(XMaterial.ANVIL) - .name(messages.getString("worldeditor_project_item", player)) - .lore(messages.getStringList( - "worldeditor_project_lore", - player, - Placeholders.of("%project%", buildWorld.getData().get(WorldDataKey.PROJECT)))) - .into(inventory, SLOT_PROJECT); - } - - private void renderPermission(Player player, Inventory inventory) { - ItemBuilder.of(XMaterial.PAPER) - .name(messages.getString("worldeditor_permission_item", player)) - .lore(messages.getStringList( - "worldeditor_permission_lore", - player, - Placeholders.of("%permission%", buildWorld.getData().get(WorldDataKey.PERMISSION)))) - .into(inventory, SLOT_PERMISSION); - } - - private String getDifficultyName(Player player) { - return switch (buildWorld.getData().get(WorldDataKey.DIFFICULTY)) { - case PEACEFUL -> messages.getString("difficulty_peaceful", player); - case EASY -> messages.getString("difficulty_easy", player); - case NORMAL -> messages.getString("difficulty_normal", player); - case HARD -> messages.getString("difficulty_hard", player); - }; - } - @Override public void handleClick(InventoryClickEvent event) { ItemStack itemStack = event.getCurrentItem(); @@ -552,13 +443,12 @@ private void onPhysicsClick(Player player, InventoryClickEvent event) { } /** - * The builders button mixes behaviors, so it cannot share the toggle closure: a barrier icon (player is not the - * creator) only plays the deny sound, a right-click opens the {@link BuilderMenu}, and a left-click flips the - * builders flag and re-opens. Re-opening happens only on the successful left-click path. + * The builders button mixes behaviors, so it cannot share the toggle closure: a player who cannot manage builders + * only plays the deny sound, a right-click opens the {@link BuilderMenu}, and a left-click flips the builders flag + * and re-opens. Re-opening happens only on the successful left-click path. */ private void onBuildersClick(Player player, InventoryClickEvent event) { - ItemStack itemStack = event.getCurrentItem(); - if (itemStack != null && itemStack.getType() == XMaterial.BARRIER.get()) { + if (!canManageBuilders(player)) { XSound.ENTITY_ITEM_BREAK.play(player); return; } @@ -575,21 +465,17 @@ private void onBuildersClick(Player player, InventoryClickEvent event) { } /** - * The visibility button guards against the barrier icon (player cannot create the target visibility): a barrier - * click only plays the deny sound and does not re-open. Every other click re-opens, matching the original behavior - * even when the permission check itself denies. + * The visibility button guards against a player who cannot create the target visibility: such a click only plays + * the deny sound and does not re-open. Every other click re-opens. */ private void onVisibilityClick(Player player, InventoryClickEvent event) { - ItemStack itemStack = event.getCurrentItem(); - if (itemStack != null && itemStack.getType() == XMaterial.BARRIER.get()) { + boolean isPrivate = buildWorld.getData().get(WorldDataKey.VISIBILITY).isPrivate(); + if (!canChangeVisibility(player, isPrivate)) { XSound.ENTITY_ITEM_BREAK.play(player); return; } - WorldData worldData = buildWorld.getData(); - worldData.set( - WorldDataKey.VISIBILITY, - worldData.get(WorldDataKey.VISIBILITY).isPrivate() ? Visibility.EVERYONE : Visibility.ADDED_PLAYERS); + buildWorld.getData().set(WorldDataKey.VISIBILITY, isPrivate ? Visibility.EVERYONE : Visibility.ADDED_PLAYERS); reopen(player); } diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/EditMenuRenderer.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/EditMenuRenderer.java new file mode 100644 index 00000000..a0a2fe3c --- /dev/null +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/EditMenuRenderer.java @@ -0,0 +1,203 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.world.menu; + +import com.cryptomorin.xseries.XMaterial; +import com.cryptomorin.xseries.profiles.objects.Profileable; +import de.eintosti.buildsystem.api.world.BuildWorld; +import de.eintosti.buildsystem.api.world.data.BuildWorldStatus; +import de.eintosti.buildsystem.api.world.data.WorldDataKey; +import de.eintosti.buildsystem.config.ConfigService; +import de.eintosti.buildsystem.i18n.Messages; +import de.eintosti.buildsystem.i18n.Placeholders; +import de.eintosti.buildsystem.menu.HeadProfileSource; +import de.eintosti.buildsystem.menu.ItemBuilder; +import de.eintosti.buildsystem.menu.MenuItems; +import de.eintosti.buildsystem.util.color.ColorAPI; +import java.util.List; +import org.bukkit.Material; +import org.bukkit.entity.Player; +import org.bukkit.inventory.Inventory; +import org.jspecify.annotations.NullMarked; +import org.jspecify.annotations.Nullable; + +/** + * Renders {@link EditMenu}'s render-only, non-authorization-sensitive slots: the world-icon button, time, butcher, + * game rules, difficulty, status, project, and permission items. Split out of {@link EditMenu} so the menu itself is + * left holding the slot layout, click handling, and the builders/visibility slots whose render and click paths share + * an authorization predicate. + */ +@NullMarked +final class EditMenuRenderer { + + private final Messages messages; + private final MenuItems menuItems; + private final ConfigService configService; + private final BuildWorld buildWorld; + + EditMenuRenderer(Messages messages, MenuItems menuItems, ConfigService configService, BuildWorld buildWorld) { + this.messages = messages; + this.menuItems = menuItems; + this.configService = configService; + this.buildWorld = buildWorld; + } + + void renderWorldInfo(Player player, Inventory inventory, int slot) { + String displayName = + messages.getString("worldeditor_world_item", player, Placeholders.of("%world%", buildWorld.getName())); + boolean isHead = buildWorld.getIcon() == Material.PLAYER_HEAD; + String loreKey = isHead ? "worldeditor_world_head_lore" : "worldeditor_world_lore"; + List lore = + messages.getStringList(loreKey, player, Placeholders.of("%texture%", iconTextureLabel(player))); + + Profileable headProfile = defaultHeadProfile(); + if (headProfile == null) { + ItemBuilder.icon(buildWorld, player).name(displayName).lore(lore).into(inventory, slot); + return; + } + + Profileable fallback = ((HeadProfileSource) buildWorld).getHeadFallbackProfile(); + menuItems.applyHeadProfileAsync(inventory, slot, headProfile, fallback, displayName, lore); + } + + /** + * The head profile {@link #renderWorldInfo} should resolve asynchronously: {@code null} when the icon isn't an + * untextured player head, or the world's default head profile (if any) otherwise. + */ + private @Nullable Profileable defaultHeadProfile() { + XMaterial material = XMaterial.matchXMaterial(buildWorld.getIcon()); + String texture = buildWorld.getIconSkullTexture(); + if (material != XMaterial.PLAYER_HEAD || (texture != null && !texture.isBlank())) { + return null; + } + return buildWorld instanceof HeadProfileSource source ? source.getHeadProfile() : null; + } + + private String iconTextureLabel(Player player) { + String texture = buildWorld.getIconSkullTexture(); + if (texture == null || texture.isBlank()) { + return messages.getString("worldeditor_world_skull_none", player); + } + if (ItemBuilder.VIEWER_HEAD.equals(texture)) { + return messages.getString("worldeditor_world_skull_viewer", player); + } + return messages.getString("worldeditor_world_skull_custom", player); + } + + void renderTime(Player player, Inventory inventory, int slot) { + XMaterial material; + String value; + switch (getWorldTime()) { + case NIGHT -> { + material = XMaterial.BLUE_STAINED_GLASS; + value = messages.getString("worldeditor_time_lore_night", player); + } + case NOON -> { + material = XMaterial.YELLOW_STAINED_GLASS; + value = messages.getString("worldeditor_time_lore_noon", player); + } + default -> { + material = XMaterial.ORANGE_STAINED_GLASS; + value = messages.getString("worldeditor_time_lore_sunrise", player); + } + } + + ItemBuilder.of(material) + .name(messages.getString("worldeditor_time_item", player)) + .lore(messages.getStringList("worldeditor_time_lore", player, Placeholders.of("%time%", value))) + .into(inventory, slot); + } + + private TimeOfDay getWorldTime() { + int worldTime = (int) buildWorld.getWorld().orElseThrow().getTime(); + int noonTime = configService.current().world().defaults().time().noon(); + return TimeOfDay.fromTicks(worldTime, noonTime); + } + + void renderButcher(Player player, Inventory inventory, int slot) { + ItemBuilder.of(XMaterial.DIAMOND_SWORD) + .name(messages.getString("worldeditor_butcher_item", player)) + .lore(messages.getStringList("worldeditor_butcher_lore", player)) + .into(inventory, slot); + } + + void renderGameRules(Player player, Inventory inventory, int slot) { + ItemBuilder.of(XMaterial.FILLED_MAP) + .name(messages.getString("worldeditor_gamerules_item", player)) + .lore(messages.getStringList("worldeditor_gamerules_lore", player)) + .into(inventory, slot); + } + + void renderDifficulty(Player player, Inventory inventory, int slot) { + XMaterial material = + switch (buildWorld.getData().get(WorldDataKey.DIFFICULTY)) { + case EASY -> XMaterial.GOLDEN_HELMET; + case NORMAL -> XMaterial.IRON_HELMET; + case HARD -> XMaterial.DIAMOND_HELMET; + default -> XMaterial.LEATHER_HELMET; + }; + + ItemBuilder.of(material) + .name(messages.getString("worldeditor_difficulty_item", player)) + .lore(messages.getStringList( + "worldeditor_difficulty_lore", + player, + Placeholders.of("%difficulty%", getDifficultyName(player)))) + .into(inventory, slot); + } + + private String getDifficultyName(Player player) { + return switch (buildWorld.getData().get(WorldDataKey.DIFFICULTY)) { + case PEACEFUL -> messages.getString("difficulty_peaceful", player); + case EASY -> messages.getString("difficulty_easy", player); + case NORMAL -> messages.getString("difficulty_normal", player); + case HARD -> messages.getString("difficulty_hard", player); + }; + } + + void renderStatus(Player player, Inventory inventory, int slot) { + BuildWorldStatus status = buildWorld.getData().get(WorldDataKey.STATUS); + ItemBuilder.of(status.getIcon()) + .name(messages.getString("worldeditor_status_item", player)) + .lore(messages.getStringList( + "worldeditor_status_lore", + player, + Placeholders.of("%status%", ColorAPI.process(status.getStyledName())))) + .into(inventory, slot); + } + + void renderProject(Player player, Inventory inventory, int slot) { + ItemBuilder.of(XMaterial.ANVIL) + .name(messages.getString("worldeditor_project_item", player)) + .lore(messages.getStringList( + "worldeditor_project_lore", + player, + Placeholders.of("%project%", buildWorld.getData().get(WorldDataKey.PROJECT)))) + .into(inventory, slot); + } + + void renderPermission(Player player, Inventory inventory, int slot) { + ItemBuilder.of(XMaterial.PAPER) + .name(messages.getString("worldeditor_permission_item", player)) + .lore(messages.getStringList( + "worldeditor_permission_lore", + player, + Placeholders.of("%permission%", buildWorld.getData().get(WorldDataKey.PERMISSION)))) + .into(inventory, slot); + } +} diff --git a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/StatusMenu.java b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/StatusMenu.java index f9460e71..f5056026 100644 --- a/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/StatusMenu.java +++ b/buildsystem-core/src/main/java/de/eintosti/buildsystem/world/menu/StatusMenu.java @@ -30,6 +30,7 @@ import de.eintosti.buildsystem.menu.MenuItems; import de.eintosti.buildsystem.menu.Menus; import de.eintosti.buildsystem.player.settings.SettingsService; +import de.eintosti.buildsystem.util.Permissions; import de.eintosti.buildsystem.util.color.ColorAPI; import de.eintosti.buildsystem.world.data.WorldStatusRegistryImpl; import org.bukkit.ChatColor; @@ -89,13 +90,15 @@ private static String formatWorldName(BuildWorld buildWorld) { private MenuButton statusButton(BuildWorldStatus status) { return MenuButton.builder() .permission(status.getPermission()) + .usableBy(player -> buildWorld.getPermissions().canPerformCommand(player, Permissions.SETSTATUS)) .render((player, inventory, slot) -> { Material material = status.getIcon(); String displayName = ColorAPI.process(status.getStyledName()); if (!player.hasPermission(status.getPermission())) { material = Material.BARRIER; - displayName = "§c§m" + ChatColor.stripColor(displayName); + displayName = "%s%s%s" + .formatted(ChatColor.RED, ChatColor.STRIKETHROUGH, ChatColor.stripColor(displayName)); } ItemBuilder.of(material) diff --git a/buildsystem-core/src/main/resources/messages.yml b/buildsystem-core/src/main/resources/messages.yml index 25050aaa..1ad76afe 100644 --- a/buildsystem-core/src/main/resources/messages.yml +++ b/buildsystem-core/src/main/resources/messages.yml @@ -342,6 +342,7 @@ worlds_setproject_usage: "%prefix% &7Usage: &b/worlds setProject " worlds_setproject_unknown_world: "%prefix% &cUnknown world." worlds_setproject_error: "%prefix% &cPlease try again." worlds_setproject_set: "%prefix% &b%world%&7's project was successfully changed." +worlds_setproject_overridden: "%prefix% &7Saved, but this world's folder currently overrides it with &b%project%&7." worlds_setstatus_usage: "%prefix% &7Usage: &b/worlds setStatus " worlds_setstatus_unknown_world: "%prefix% &cUnknown world." @@ -352,6 +353,9 @@ worlds_setpermission_unknown_world: "%prefix% &cUnknown world." worlds_setpermission_error: "%prefix% &cPlease try again." worlds_setpermission_set: "%prefix% &b%world%&7's permission was successfully changed." worlds_setpermission_not_allowed: "%prefix% &cThat permission is not allowed." +worlds_setpermission_overridden: "%prefix% &7Saved, but this world's folder currently overrides it with &b%permission%&7." + +worlds_create_limit_reached: "%prefix% &cYou have reached the maximum number of worlds you may create." worlds_setspawn_world_not_imported: "%prefix% &cWorld must be imported: /worlds import " worlds_setspawn_world_spawn_set: "%prefix% &b%world%&7's spawnpoint was set." diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/BuildSystemPluginSingletonGuardTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/BuildSystemPluginNoStaticSelfReferenceTest.java similarity index 89% rename from buildsystem-core/src/test/java/de/eintosti/buildsystem/BuildSystemPluginSingletonGuardTest.java rename to buildsystem-core/src/test/java/de/eintosti/buildsystem/BuildSystemPluginNoStaticSelfReferenceTest.java index c5fbf0ed..31dbf191 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/BuildSystemPluginSingletonGuardTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/BuildSystemPluginNoStaticSelfReferenceTest.java @@ -32,12 +32,13 @@ import org.junit.jupiter.api.Test; /** - * Compile-guard: ensures BuildSystemPlugin has no static self-referencing singleton. Fails if someone re-adds a static - * BuildSystemPlugin instance field or get() method. Parses class file structure without loading the class (avoids full - * plugin classpath). + * Bytecode-level guard against reintroducing a static singleton accessor on {@code BuildSystemPlugin}: it parses the + * compiled class file and asserts there is no static field of type {@code BuildSystemPlugin} and no static + * {@code get()} method returning one. This is a compile-time guard only — it never loads or instantiates the plugin, + * so it does not exercise (and cannot verify) any runtime double-enable protection. */ @NullMarked -class BuildSystemPluginSingletonGuardTest { +class BuildSystemPluginNoStaticSelfReferenceTest { private static final String CLASS_RESOURCE = "de/eintosti/buildsystem/BuildSystemPlugin.class"; private static final String SELF_DESCRIPTOR = "Lde/eintosti/buildsystem/BuildSystemPlugin;"; @@ -50,8 +51,9 @@ class BuildSystemPluginSingletonGuardTest { @BeforeAll static void loadClassFile() throws Exception { - InputStream is = - BuildSystemPluginSingletonGuardTest.class.getClassLoader().getResourceAsStream(CLASS_RESOURCE); + InputStream is = BuildSystemPluginNoStaticSelfReferenceTest.class + .getClassLoader() + .getResourceAsStream(CLASS_RESOURCE); assumeTrue(is != null, "BuildSystemPlugin.class not found on classpath — skipping guard"); try (DataInputStream dis = new DataInputStream(is)) { diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/menu/PromptFlowTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/menu/PromptFlowTest.java index f8529da6..d9869226 100644 --- a/buildsystem-core/src/test/java/de/eintosti/buildsystem/menu/PromptFlowTest.java +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/menu/PromptFlowTest.java @@ -31,6 +31,7 @@ import java.util.ArrayList; import java.util.HashSet; import java.util.List; +import java.util.UUID; import org.bukkit.entity.Player; import org.bukkit.event.player.AsyncPlayerChatEvent; import org.bukkit.plugin.Plugin; @@ -41,16 +42,22 @@ import org.mockbukkit.mockbukkit.ServerMock; /** - * Drives a {@link Prompts.PromptFlow} through the real {@link PlayerChatInput} machinery under MockBukkit, pinning the - * routing contract: a step that rejects its input is re-prompted, an accepting step advances, and the completion - * callback fires only after the final step accepts. + * Drives a {@link Prompts.PromptFlow} through the real {@link PlayerChatInput} machinery, pinning the routing + * contract: a step that rejects its input is re-prompted, an accepting step advances, and the completion callback + * fires only after the final step accepts. + * + *

The player is a Mockito mock rather than a real MockBukkit player, so that the sound {@link PlayerChatInput} + * plays no-ops instead of hitting MockBukkit's unimplemented {@code playSound}. Chat lines are delivered straight to + * {@link PlayerChatInput.ChatInputListener#onPlayerChat} instead of through the plugin manager's event dispatch, + * since nothing under test depends on Bukkit's own event routing. */ class PromptFlowTest { - private ServerMock server; private TaskScheduler scheduler; private Prompts prompts; private Player player; + private PlayerChatInput.ChatInputListener listener; + private ServerMock server; @BeforeEach void setUp() { @@ -62,8 +69,9 @@ void setUp() { when(messages.getString(anyString(), any())).thenReturn("Title"); prompts = new Prompts(messages, mock(ConfigService.class), scheduler); - server.getPluginManager().registerEvents(new PlayerChatInput.ChatInputListener(), plugin); - player = server.addPlayer(); + listener = new PlayerChatInput.ChatInputListener(); + player = mock(Player.class); + when(player.getUniqueId()).thenReturn(UUID.randomUUID()); } @AfterEach @@ -72,9 +80,9 @@ void tearDown() { MockBukkit.unmock(); } - /** Sends a chat line, then ticks the scheduler so the queued completion callback runs. */ + /** Delivers a chat line to the listener directly, then ticks the scheduler so the queued callback runs. */ private void chat(String message) { - server.getPluginManager().callEvent(new AsyncPlayerChatEvent(false, player, message, new HashSet<>())); + listener.onPlayerChat(new AsyncPlayerChatEvent(false, player, message, new HashSet<>())); server.getScheduler().performOneTick(); } diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/player/PlayerServiceLimitsTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/player/PlayerServiceLimitsTest.java new file mode 100644 index 00000000..73cb16e6 --- /dev/null +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/player/PlayerServiceLimitsTest.java @@ -0,0 +1,131 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.player; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import de.eintosti.buildsystem.BuildSystemPlugin; +import de.eintosti.buildsystem.api.world.BuildWorld; +import de.eintosti.buildsystem.api.world.data.Visibility; +import de.eintosti.buildsystem.config.ConfigService; +import de.eintosti.buildsystem.config.PluginConfig; +import de.eintosti.buildsystem.storage.WorldStorageImpl; +import de.eintosti.buildsystem.util.TaskScheduler; +import de.eintosti.buildsystem.world.WorldServiceImpl; +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import java.util.Set; +import java.util.logging.Logger; +import java.util.stream.Collectors; +import org.bukkit.entity.Player; +import org.bukkit.permissions.PermissionAttachmentInfo; +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.Test; + +/** + * Pins the world-creation limit ladder: a {@code buildsystem.create..} node wins, the + * {@code world.limits.*} config value is the fallback for players holding no such node, and creation is unlimited when + * neither is set. Counting is per player and per visibility, so a limit on one visibility never blocks the other. + */ +@NullMarked +class PlayerServiceLimitsTest { + + private static final int UNLIMITED = -1; + + private PlayerServiceImpl service(int configPublic, int configPrivate, int ownedOfVisibility) { + BuildSystemPlugin plugin = mock(BuildSystemPlugin.class); + when(plugin.getLogger()).thenReturn(Logger.getLogger("test")); + + PluginConfig.World.Limits limits = new PluginConfig.World.Limits(configPublic, configPrivate); + PluginConfig.World world = mock(PluginConfig.World.class); + when(world.limits()).thenReturn(limits); + PluginConfig config = mock(PluginConfig.class); + when(config.world()).thenReturn(world); + ConfigService configService = mock(ConfigService.class); + when(configService.current()).thenReturn(config); + + List owned = Collections.nCopies(ownedOfVisibility, mock(BuildWorld.class)); + WorldStorageImpl worldStorage = mock(WorldStorageImpl.class); + when(worldStorage.getBuildWorldsCreatedByPlayer(any(), any())).thenReturn(owned); + WorldServiceImpl worldService = mock(WorldServiceImpl.class); + when(worldService.getWorldStorage()).thenReturn(worldStorage); + + return new PlayerServiceImpl(plugin, configService, () -> worldService, mock(TaskScheduler.class)); + } + + private Player playerWith(String... permissionStrings) { + Player player = mock(Player.class); + when(player.hasPermission(BuildSystemPlugin.ADMIN_PERMISSION)).thenReturn(false); + Set perms = Arrays.stream(permissionStrings) + .map(p -> { + PermissionAttachmentInfo pai = mock(PermissionAttachmentInfo.class); + when(pai.getPermission()).thenReturn(p); + return pai; + }) + .collect(Collectors.toSet()); + when(player.getEffectivePermissions()).thenReturn(perms); + return player; + } + + @Test + void permissionNodeOverridesTheConfiguredDefault() { + // Config allows 1, the node allows 5, the player owns 3. + PlayerServiceImpl service = service(1, 1, 3); + assertTrue(service.canCreateWorld(playerWith("buildsystem.create.public.5"), Visibility.EVERYONE)); + } + + @Test + void configuredDefaultAppliesWhenThePlayerHoldsNoNode() { + PlayerServiceImpl service = service(2, 2, 2); + assertFalse(service.canCreateWorld(playerWith(), Visibility.EVERYONE)); + } + + @Test + void configuredDefaultStillAllowsCreationBelowTheLimit() { + PlayerServiceImpl service = service(3, 3, 2); + assertTrue(service.canCreateWorld(playerWith(), Visibility.EVERYONE)); + } + + @Test + void unlimitedWhenNeitherNodeNorConfigSetsALimit() { + PlayerServiceImpl service = service(UNLIMITED, UNLIMITED, 500); + assertTrue(service.canCreateWorld(playerWith(), Visibility.EVERYONE)); + } + + @Test + void aPrivateLimitDoesNotBlockPublicCreation() { + // Private capped at 1 and already met; public is unlimited, so a public world is still allowed. Counting used + // to ignore visibility, which made either limit block both. + PlayerServiceImpl service = service(UNLIMITED, 1, 1); + assertTrue(service.canCreateWorld(playerWith(), Visibility.EVERYONE)); + assertFalse(service.canCreateWorld(playerWith(), Visibility.ADDED_PLAYERS)); + } + + @Test + void adminHasNoLimit() { + PlayerServiceImpl service = service(1, 1, 99); + Player admin = playerWith(); + when(admin.hasPermission(BuildSystemPlugin.ADMIN_PERMISSION)).thenReturn(true); + assertTrue(service.canCreateWorld(admin, Visibility.EVERYONE)); + } +} diff --git a/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/backup/BackupProfileImplTest.java b/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/backup/BackupProfileImplTest.java new file mode 100644 index 00000000..d0f0dfe1 --- /dev/null +++ b/buildsystem-core/src/test/java/de/eintosti/buildsystem/world/backup/BackupProfileImplTest.java @@ -0,0 +1,160 @@ +/* + * Copyright (c) 2018-2026, Thomas Meaney + * Copyright (c) contributors + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package de.eintosti.buildsystem.world.backup; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.RETURNS_DEEP_STUBS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import de.eintosti.buildsystem.BuildSystemPlugin; +import de.eintosti.buildsystem.api.world.BuildWorld; +import de.eintosti.buildsystem.api.world.backup.Backup; +import de.eintosti.buildsystem.api.world.backup.BackupStorage; +import de.eintosti.buildsystem.config.ConfigService; +import de.eintosti.buildsystem.i18n.Messages; +import de.eintosti.buildsystem.world.WorldServiceImpl; +import de.eintosti.buildsystem.world.spawn.SpawnService; +import java.util.List; +import java.util.Optional; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; +import org.bukkit.Bukkit; +import org.bukkit.plugin.PluginManager; +import org.bukkit.scheduler.BukkitScheduler; +import org.jspecify.annotations.NullMarked; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.MockedStatic; + +/** + * Pins the retention contract of {@link BackupProfileImpl#createBackup()}: archives over {@code maxBackupsPerWorld} + * are deleted oldest-first by {@link Backup#creationTime()}, and the cap reserves exactly one slot for the backup + * being created. + * + *

{@code createBackup()} hops onto the main thread via {@link Bukkit#getScheduler()}; rather than pull in + * MockBukkit for a single scheduler hop, {@link Bukkit} is mocked statically with a scheduler stub that runs the + * submitted task immediately, so the whole chain resolves synchronously. + */ +@NullMarked +class BackupProfileImplTest { + + private BuildSystemPlugin plugin; + private ConfigService configService; + private BackupStorage backupStorage; + private BuildWorld buildWorld; + private MockedStatic bukkit; + + @BeforeEach + void setUp() { + plugin = mock(BuildSystemPlugin.class); + configService = mock(ConfigService.class, RETURNS_DEEP_STUBS); + backupStorage = mock(BackupStorage.class); + buildWorld = mock(BuildWorld.class); + when(buildWorld.getWorld()).thenReturn(Optional.empty()); + + BukkitScheduler scheduler = mock(BukkitScheduler.class); + when(scheduler.runTask(any(), any(Runnable.class))).thenAnswer(invocation -> { + invocation.getArgument(1, Runnable.class).run(); + return null; + }); + PluginManager pluginManager = mock(PluginManager.class); + + bukkit = mockStatic(Bukkit.class); + bukkit.when(Bukkit::getScheduler).thenReturn(scheduler); + bukkit.when(Bukkit::getPluginManager).thenReturn(pluginManager); + } + + @AfterEach + void tearDown() { + bukkit.close(); + } + + private BackupProfileImpl profile(int maxBackupsPerWorld) { + when(configService.current().world().backup().maxBackupsPerWorld()).thenReturn(maxBackupsPerWorld); + return new BackupProfileImpl( + plugin, + configService, + mock(Messages.class), + mock(WorldServiceImpl.class), + mock(SpawnService.class), + () -> backupStorage, + buildWorld); + } + + private static Backup backup(long creationTime) { + Backup backup = mock(Backup.class); + when(backup.creationTime()).thenReturn(creationTime); + return backup; + } + + private void stubListingAndStore(List existingBackups) { + Backup stored = backup(Long.MAX_VALUE); + when(backupStorage.listBackups(buildWorld)).thenReturn(CompletableFuture.completedFuture(existingBackups)); + when(backupStorage.storeBackup(buildWorld)).thenReturn(CompletableFuture.completedFuture(stored)); + when(backupStorage.deleteBackup(any())).thenReturn(CompletableFuture.completedFuture(null)); + } + + @Test + void atCap_deletesOnlyTheOldest() throws Exception { + Backup oldest = backup(1_000L); + Backup middle = backup(2_000L); + Backup newest = backup(3_000L); + // Shuffled insertion order: a "delete the first N of the list" implementation would delete the wrong one. + stubListingAndStore(List.of(newest, oldest, middle)); + + profile(3).createBackup().get(5, TimeUnit.SECONDS); + + verify(backupStorage, times(1)).deleteBackup(oldest); + verify(backupStorage, never()).deleteBackup(middle); + verify(backupStorage, never()).deleteBackup(newest); + } + + @Test + void underCap_deletesNothing() throws Exception { + Backup older = backup(1_000L); + Backup newer = backup(2_000L); + stubListingAndStore(List.of(older, newer)); + + profile(3).createBackup().get(5, TimeUnit.SECONDS); + + verify(backupStorage, never()).deleteBackup(any()); + } + + @Test + void capLoweredBelowExisting_deletesTheTwoOldest() throws Exception { + Backup oldest = backup(1_000L); + Backup secondOldest = backup(2_000L); + Backup secondNewest = backup(3_000L); + Backup newest = backup(4_000L); + // Shuffled insertion order: a "delete the first N of the list" implementation would delete the wrong ones. + stubListingAndStore(List.of(secondNewest, oldest, newest, secondOldest)); + + profile(3).createBackup().get(5, TimeUnit.SECONDS); + + verify(backupStorage, times(1)).deleteBackup(oldest); + verify(backupStorage, times(1)).deleteBackup(secondOldest); + verify(backupStorage, never()).deleteBackup(secondNewest); + verify(backupStorage, never()).deleteBackup(newest); + } +}