From 4e498a8df09708cdd6531ef3a0d8b1e0ac15fac3 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Mon, 27 Apr 2026 16:42:15 +0500 Subject: [PATCH 01/16] fix: round 1 of code-review fixes (P0-3 / P1-4 / P1-5+P1-7 / P1-6 / P1-13) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P0-3 — null-guard VariableManagerImpl.shutdown() in onDisable so a mid-enable failure no longer NPEs and masks the real exception. P1-4 — cache AddonManager.availableNotLoaded() with 2s TTL. Tab completion was opening N jars + parsing HOCON synchronously per keystroke. Cache invalidates on loadOne/rescan so successful loads appear immediately. P1-5 + P1-7 — closing the cross-addon sabotage hole: * moved AbstractMenusApiImpl, TypeRegistryImpl, ProviderRegistryImpl out of ru.abstractmenus.api.* (where they leaked package-private visibility into the public API package) into ru.abstractmenus.impl.* * removed unregisterAll(MenuExtension) from public TypeRegistry and ProviderRegistry interfaces — addons can no longer call it via the documented API surface to wipe another extension's registrations. * AddonManager.rollbackRegistrations now casts to *Impl. Also extended to call providers().unregisterAll, which was missing. * Tests + ProviderRegistryImplTest moved alongside. P1-6 — Logger.set is now set-once (throws IllegalStateException on re-install). Previously any addon could replace the global logger to silence or capture core's output. P1-13 — TypeRegistryImpl now actually drops Class entries from NodeSerializers' backing map on unregisterAll (via reflection, since hocon 1.0.6 has no public unregister). Without this, a Class held by NodeSerializers kept the addon's closed AddonClassLoader alive, leaking native FDs and all loaded classes. --- .../java/ru/abstractmenus/api/Logger.java | 14 ++-- .../abstractmenus/api/ProviderRegistry.java | 12 +--- .../ru/abstractmenus/api/TypeRegistry.java | 21 ++---- .../java/ru/abstractmenus/AbstractMenus.java | 6 +- .../ru/abstractmenus/addon/AddonManager.java | 51 +++++++++++--- .../{api => impl}/AbstractMenusApiImpl.java | 9 ++- .../{api => impl}/ProviderRegistryImpl.java | 11 ++- .../{api => impl}/TypeRegistryImpl.java | 70 ++++++++++++++++--- .../addon/AddonManagerIntegrationTest.java | 4 +- .../abstractmenus/core/CoreExtensionTest.java | 4 +- .../actions/TestActionCommandBehavior.java | 2 +- .../ProviderRegistryImplTest.java | 4 +- .../{api => impl}/TypeRegistryImplTest.java | 4 +- .../testsupport/ApiTestSupport.java | 4 +- 14 files changed, 152 insertions(+), 64 deletions(-) rename plugin/src/main/java/ru/abstractmenus/{api => impl}/AbstractMenusApiImpl.java (91%) rename plugin/src/main/java/ru/abstractmenus/{api => impl}/ProviderRegistryImpl.java (94%) rename plugin/src/main/java/ru/abstractmenus/{api => impl}/TypeRegistryImpl.java (54%) rename plugin/src/test/java/ru/abstractmenus/{api => impl}/ProviderRegistryImplTest.java (97%) rename plugin/src/test/java/ru/abstractmenus/{api => impl}/TypeRegistryImplTest.java (96%) diff --git a/api/src/main/java/ru/abstractmenus/api/Logger.java b/api/src/main/java/ru/abstractmenus/api/Logger.java index 8844746..7bcf6fe 100644 --- a/api/src/main/java/ru/abstractmenus/api/Logger.java +++ b/api/src/main/java/ru/abstractmenus/api/Logger.java @@ -37,15 +37,19 @@ private Logger(){} /** * Install the backing JUL logger. Called once by AbstractMenus core during - * plugin {@code onEnable}. - * - *

Addons should not call this — replacing the logger would - * redirect all subsequent log output (including core's) away from - * the plugin's channel. + * plugin {@code onEnable}; throws on any subsequent call so an addon cannot + * replace the logger to silence or capture core's output. * * @param log the JUL logger to delegate to; never {@code null} + * @throws IllegalStateException if a logger has already been installed + * @throws NullPointerException if {@code log} is null */ public static void set(java.util.logging.Logger log){ + if (log == null) throw new NullPointerException("logger"); + if (logger != null) { + throw new IllegalStateException( + "Logger already installed; AbstractMenus does not support replacement"); + } logger = log; } diff --git a/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java b/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java index 73d6df8..47049f6 100644 --- a/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java +++ b/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java @@ -23,7 +23,7 @@ * "playerpoints", * new PlayerPointsEconomy(pp), * 100, // priority — higher wins in auto-resolve - * this); // owner for unregisterAll on reload + * this); // owner — AbstractMenus uses this for cleanup on reload * } * } * } @@ -85,14 +85,4 @@ public interface ProviderRegistry { SkinHandler skins(String id); Collection allSkins(); boolean hasSkins(String id); - - // ---- Cleanup --------------------------------------------------------- - - /** - * Remove every provider registration (across all sections) owned by - * {@code owner}. Called by AddonManager when an addon is disabled. - * - * @param owner the extension whose providers should be cleared - */ - void unregisterAll(MenuExtension owner); } diff --git a/api/src/main/java/ru/abstractmenus/api/TypeRegistry.java b/api/src/main/java/ru/abstractmenus/api/TypeRegistry.java index ecee338..3496ef7 100644 --- a/api/src/main/java/ru/abstractmenus/api/TypeRegistry.java +++ b/api/src/main/java/ru/abstractmenus/api/TypeRegistry.java @@ -13,9 +13,10 @@ * {@link AbstractMenusApi#itemProperties()}, and * {@link AbstractMenusApi#catalogs()} — one per extension surface. * - *

Registration takes a {@link MenuExtension} "owner" so - * {@link #unregisterAll(MenuExtension)} can wipe every entry an addon - * contributed when that addon is disabled or reloaded. + *

Registration takes a {@link MenuExtension} "owner" so AbstractMenus' + * internal addon manager can wipe every entry an addon contributed when + * that addon is disabled or reloaded. Addons themselves cannot trigger + * that wipe; the cleanup hook lives on the impl, not on this interface. * *

Example

* @@ -61,9 +62,9 @@ public interface TypeRegistry { * @param key HOCON-visible name (case-insensitive) * @param type the class token; must be assignable to {@code T} * @param serializer HOCON serializer for {@code type} - * @param owner the registering extension; used for later - * {@link #unregisterAll(MenuExtension)} cleanup. Pass - * the core extension for core registrations. + * @param owner the registering extension; AbstractMenus' addon + * manager uses this for cleanup on disable/reload. + * Pass the core extension for core registrations. */ void register(String key, Class type, @@ -92,12 +93,4 @@ void register(String key, * @return all registered keys (lowercased) */ Set keys(); - - /** - * Remove every entry registered by {@code owner}. Invoked by the addon - * manager when an extension is disabled or reloaded. - * - * @param owner the extension whose entries should be wiped - */ - void unregisterAll(MenuExtension owner); } diff --git a/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java b/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java index 9d70629..b2aab22 100644 --- a/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java +++ b/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java @@ -11,7 +11,7 @@ import org.bukkit.plugin.java.JavaPlugin; import ru.abstractmenus.api.*; import ru.abstractmenus.api.AbstractMenusApi; -import ru.abstractmenus.api.AbstractMenusApiImpl; +import ru.abstractmenus.impl.AbstractMenusApiImpl; import ru.abstractmenus.api.inventory.Menu; import ru.abstractmenus.api.text.Colors; import ru.abstractmenus.api.variables.VariableManager; @@ -207,7 +207,9 @@ public void onDisable() { if (BungeeManager.instance() != null) BungeeManager.instance().stopOnlineTimer(); - VariableManagerImpl.instance().shutdown(); + if (VariableManagerImpl.instance() != null) { + VariableManagerImpl.instance().shutdown(); + } Events.unregisterAll(); getServer().getMessenger().unregisterIncomingPluginChannel(this, "BungeeCord"); diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java index 8747ffe..3011cf1 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java @@ -160,14 +160,23 @@ private ru.abstractmenus.api.MenuExtension instantiate(LoadedAddon la) throws Ex return (ru.abstractmenus.api.MenuExtension) main.getDeclaredConstructor().newInstance(); } - /** Strip any type registrations the failed addon managed to make. */ + /** + * Strip any type registrations the failed addon managed to make. + * + *

Casts each registry to its {@code Impl} because {@code unregisterAll} + * is intentionally not on the public {@link ru.abstractmenus.api.TypeRegistry} + * / {@link ru.abstractmenus.api.ProviderRegistry} interfaces - addons + * shouldn't be able to wipe each other's registrations. + */ private void rollbackRegistrations(LoadedAddon la) { if (la.getExtension() == null) return; - api.actions().unregisterAll(la.getExtension()); - api.rules().unregisterAll(la.getExtension()); - api.activators().unregisterAll(la.getExtension()); - api.itemProperties().unregisterAll(la.getExtension()); - api.catalogs().unregisterAll(la.getExtension()); + ru.abstractmenus.api.MenuExtension ext = la.getExtension(); + ((ru.abstractmenus.impl.TypeRegistryImpl) api.actions()).unregisterAll(ext); + ((ru.abstractmenus.impl.TypeRegistryImpl) api.rules()).unregisterAll(ext); + ((ru.abstractmenus.impl.TypeRegistryImpl) api.activators()).unregisterAll(ext); + ((ru.abstractmenus.impl.TypeRegistryImpl) api.itemProperties()).unregisterAll(ext); + ((ru.abstractmenus.impl.TypeRegistryImpl) api.catalogs()).unregisterAll(ext); + ((ru.abstractmenus.impl.ProviderRegistryImpl) api.providers()).unregisterAll(ext); } /** @@ -361,6 +370,7 @@ public Optional loadOne(String name) { return Optional.empty(); } enableSingle(la); + cachedAvailableAt = 0L; return Optional.of(la); } @@ -384,17 +394,40 @@ public List rescan() { enableSingle(la); newlyLoaded.add(la); } + if (!newlyLoaded.isEmpty()) cachedAvailableAt = 0L; return newlyLoaded; } + /** + * TTL for the availableNotLoaded() cache. Tab completion fires on every + * keystroke; without a cache that is N jar opens + N HOCON parses on the + * main thread per TAB. 2 seconds is short enough that the operator does + * not see staleness in practice (drop a jar, wait a beat, hit TAB). + */ + private static final long AVAILABLE_CACHE_TTL_MS = 2_000L; + private volatile List cachedAvailable = List.of(); + private volatile long cachedAvailableAt = 0L; + /** * Return addon-conf {@code name}s found on disk under the addons * directory but not yet loaded into memory. Used by tab completion - * for {@code /am addons load }. Cost is one jar open and one - * HOCON parse per .jar in the directory - acceptable at typical - * scale (1-20 addons), but be aware this is not free. + * for {@code /am addons load }. + * + *

Result is cached for {@value #AVAILABLE_CACHE_TTL_MS} ms because + * tab completion runs synchronously on the main thread and a cold call + * does one jar open + HOCON parse per *.jar in the addons folder. */ public List availableNotLoaded() { + long now = System.currentTimeMillis(); + if (now - cachedAvailableAt < AVAILABLE_CACHE_TTL_MS) { + return cachedAvailable; + } + cachedAvailable = scanAvailableNotLoaded(); + cachedAvailableAt = now; + return cachedAvailable; + } + + private List scanAvailableNotLoaded() { if (!java.nio.file.Files.isDirectory(addonsDir)) return List.of(); List result = new ArrayList<>(); try (var stream = java.nio.file.Files.newDirectoryStream(addonsDir, "*.jar")) { diff --git a/plugin/src/main/java/ru/abstractmenus/api/AbstractMenusApiImpl.java b/plugin/src/main/java/ru/abstractmenus/impl/AbstractMenusApiImpl.java similarity index 91% rename from plugin/src/main/java/ru/abstractmenus/api/AbstractMenusApiImpl.java rename to plugin/src/main/java/ru/abstractmenus/impl/AbstractMenusApiImpl.java index 922ab3a..a982dc9 100644 --- a/plugin/src/main/java/ru/abstractmenus/api/AbstractMenusApiImpl.java +++ b/plugin/src/main/java/ru/abstractmenus/impl/AbstractMenusApiImpl.java @@ -1,8 +1,15 @@ -package ru.abstractmenus.api; +package ru.abstractmenus.impl; import org.bukkit.entity.Player; import org.bukkit.plugin.Plugin; import ru.abstractmenus.AbstractMenus; +import ru.abstractmenus.api.AbstractMenusApi; +import ru.abstractmenus.api.Action; +import ru.abstractmenus.api.Activator; +import ru.abstractmenus.api.Catalog; +import ru.abstractmenus.api.ProviderRegistry; +import ru.abstractmenus.api.Rule; +import ru.abstractmenus.api.TypeRegistry; import ru.abstractmenus.api.inventory.ItemProperty; import ru.abstractmenus.api.inventory.Menu; import ru.abstractmenus.api.variables.VariableManager; diff --git a/plugin/src/main/java/ru/abstractmenus/api/ProviderRegistryImpl.java b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java similarity index 94% rename from plugin/src/main/java/ru/abstractmenus/api/ProviderRegistryImpl.java rename to plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java index 50ae7b4..2fb925a 100644 --- a/plugin/src/main/java/ru/abstractmenus/api/ProviderRegistryImpl.java +++ b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java @@ -1,5 +1,7 @@ -package ru.abstractmenus.api; +package ru.abstractmenus.impl; +import ru.abstractmenus.api.MenuExtension; +import ru.abstractmenus.api.ProviderRegistry; import ru.abstractmenus.api.handler.EconomyHandler; import ru.abstractmenus.api.handler.LevelHandler; import ru.abstractmenus.api.handler.PermissionsHandler; @@ -96,7 +98,12 @@ private T resolveWithConfig(String kind, Section section) { // ---- Cleanup --------------------------------------------------------- - @Override + /** + * Wipe every provider registered by {@code owner} across all five + * sections. Intentionally NOT on the public {@link ProviderRegistry} + * interface so an addon cannot wipe another extension's providers. + * Called only by AbstractMenus' internal addon manager. + */ public void unregisterAll(MenuExtension owner) { economy.unregisterAll(owner); permissions.unregisterAll(owner); diff --git a/plugin/src/main/java/ru/abstractmenus/api/TypeRegistryImpl.java b/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java similarity index 54% rename from plugin/src/main/java/ru/abstractmenus/api/TypeRegistryImpl.java rename to plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java index 225e081..aea1c5a 100644 --- a/plugin/src/main/java/ru/abstractmenus/api/TypeRegistryImpl.java +++ b/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java @@ -1,14 +1,18 @@ -package ru.abstractmenus.api; +package ru.abstractmenus.impl; +import ru.abstractmenus.api.MenuExtension; +import ru.abstractmenus.api.TypeRegistry; import ru.abstractmenus.hocon.api.serialize.NodeSerializer; import ru.abstractmenus.hocon.api.serialize.NodeSerializers; +import java.lang.reflect.Field; import java.util.Collections; import java.util.HashMap; import java.util.HashSet; import java.util.IdentityHashMap; import java.util.Map; import java.util.Set; +import java.util.logging.Level; import java.util.logging.Logger; /** @@ -17,16 +21,36 @@ * server thread. * *

Note on {@code NodeSerializers.unregister}: hocon 1.0.6 does NOT expose - * an {@code unregister(Class)} method. Therefore stale serializer entries - * survive in {@link NodeSerializers} after {@link #unregisterAll(MenuExtension)}, - * but that is harmless — the {@link #byKey} map is the authoritative lookup - * table, and a subsequent {@link #register} call for the same class token - * overwrites the serializer entry. + * an {@code unregister(Class)} method. We therefore reach into its private + * backing map via reflection (one-time {@link Field} lookup, cached) so + * {@link #unregisterAll(MenuExtension)} can drop stale {@code Class} keys. + * Without this the {@code Class} object keeps the addon's now-closed + * classloader alive forever (native FDs, jar handle, all loaded classes). */ public final class TypeRegistryImpl implements TypeRegistry { private static final Logger LOG = Logger.getLogger(TypeRegistryImpl.class.getName()); + /** + * Reflective handle to {@link NodeSerializers}'s private + * {@code serializers} map field. Resolved once at class init; if hocon + * ever renames it, we log a warning and fall through to the harmless + * "leave the entry" behaviour. + */ + private static final Field NODE_SERIALIZERS_MAP_FIELD; + static { + Field f = null; + try { + f = NodeSerializers.class.getDeclaredField("serializers"); + f.setAccessible(true); + } catch (NoSuchFieldException e) { + LOG.log(Level.WARNING, + "NodeSerializers.serializers field missing; addon-disable will leak classloader references", + e); + } + NODE_SERIALIZERS_MAP_FIELD = f; + } + private final NodeSerializers serializers; /** key (lowercased) → registered class */ @@ -81,7 +105,12 @@ public synchronized Set keys() { return Collections.unmodifiableSet(new HashSet<>(byKey.keySet())); } - @Override + /** + * Wipe every entry registered by {@code owner}. Intentionally NOT on the + * public {@link TypeRegistry} interface so that addons cannot use it to + * unregister another extension's entries. Called only by AbstractMenus' + * internal addon manager via a cast on the impl reference. + */ public synchronized void unregisterAll(MenuExtension owner) { Set keys = keysByOwner.remove(owner); if (keys == null) return; @@ -90,11 +119,30 @@ public synchronized void unregisterAll(MenuExtension owner) { Class type = byKey.remove(k); if (type != null) { byType.remove(type); - // NodeSerializers.unregister(Class) does not exist in hocon 1.0.6. - // The stale serializer entry in NodeSerializers is harmless — - // byKey is authoritative, and re-registration overwrites it. - // serializers.unregister(type); + removeSerializerEntry(type); } } } + + /** + * Drop a {@code Class -> NodeSerializer} entry from the backing + * {@link NodeSerializers}. Done via reflection because hocon 1.0.6 does + * not expose an unregister method. Failure is non-fatal: we log and + * leave the entry, accepting the classloader-leak cost rather than + * crashing the disable path. + */ + private void removeSerializerEntry(Class type) { + if (NODE_SERIALIZERS_MAP_FIELD == null) return; + try { + @SuppressWarnings("unchecked") + Map, NodeSerializer> backing = + (Map, NodeSerializer>) NODE_SERIALIZERS_MAP_FIELD.get(serializers); + backing.remove(type); + } catch (Throwable t) { + LOG.log(Level.WARNING, + "Failed to drop NodeSerializers entry for " + type.getName() + + "; addon classloader may be retained", + t); + } + } } diff --git a/plugin/src/test/java/ru/abstractmenus/addon/AddonManagerIntegrationTest.java b/plugin/src/test/java/ru/abstractmenus/addon/AddonManagerIntegrationTest.java index e021a5f..f09fed4 100644 --- a/plugin/src/test/java/ru/abstractmenus/addon/AddonManagerIntegrationTest.java +++ b/plugin/src/test/java/ru/abstractmenus/addon/AddonManagerIntegrationTest.java @@ -10,9 +10,9 @@ import ru.abstractmenus.api.MenuExtension; import ru.abstractmenus.api.Rule; import ru.abstractmenus.api.ProviderRegistry; -import ru.abstractmenus.api.ProviderRegistryImpl; +import ru.abstractmenus.impl.ProviderRegistryImpl; import ru.abstractmenus.api.TypeRegistry; -import ru.abstractmenus.api.TypeRegistryImpl; +import ru.abstractmenus.impl.TypeRegistryImpl; import ru.abstractmenus.api.inventory.ItemProperty; import ru.abstractmenus.api.inventory.Menu; import ru.abstractmenus.api.variables.VariableManager; diff --git a/plugin/src/test/java/ru/abstractmenus/core/CoreExtensionTest.java b/plugin/src/test/java/ru/abstractmenus/core/CoreExtensionTest.java index d597de3..d94edc7 100644 --- a/plugin/src/test/java/ru/abstractmenus/core/CoreExtensionTest.java +++ b/plugin/src/test/java/ru/abstractmenus/core/CoreExtensionTest.java @@ -14,9 +14,9 @@ import ru.abstractmenus.api.MenuExtension; import ru.abstractmenus.api.Rule; import ru.abstractmenus.api.ProviderRegistry; -import ru.abstractmenus.api.ProviderRegistryImpl; +import ru.abstractmenus.impl.ProviderRegistryImpl; import ru.abstractmenus.api.TypeRegistry; -import ru.abstractmenus.api.TypeRegistryImpl; +import ru.abstractmenus.impl.TypeRegistryImpl; import ru.abstractmenus.api.inventory.ItemProperty; import ru.abstractmenus.api.inventory.Menu; import ru.abstractmenus.api.variables.VariableManager; diff --git a/plugin/src/test/java/ru/abstractmenus/data/actions/TestActionCommandBehavior.java b/plugin/src/test/java/ru/abstractmenus/data/actions/TestActionCommandBehavior.java index ff63a97..553342f 100644 --- a/plugin/src/test/java/ru/abstractmenus/data/actions/TestActionCommandBehavior.java +++ b/plugin/src/test/java/ru/abstractmenus/data/actions/TestActionCommandBehavior.java @@ -106,7 +106,7 @@ void placeholderReplacementRunsExactlyOnce() throws Exception { org.junit.jupiter.api.Assertions.assertEquals(1, callCount[0], "PlaceholderHandler.replace must be called exactly once per command"); } finally { - apiSupport.providers().unregisterAll(scratchOwner); + ((ru.abstractmenus.impl.ProviderRegistryImpl) apiSupport.providers()).unregisterAll(scratchOwner); } } diff --git a/plugin/src/test/java/ru/abstractmenus/api/ProviderRegistryImplTest.java b/plugin/src/test/java/ru/abstractmenus/impl/ProviderRegistryImplTest.java similarity index 97% rename from plugin/src/test/java/ru/abstractmenus/api/ProviderRegistryImplTest.java rename to plugin/src/test/java/ru/abstractmenus/impl/ProviderRegistryImplTest.java index a113940..fbae26f 100644 --- a/plugin/src/test/java/ru/abstractmenus/api/ProviderRegistryImplTest.java +++ b/plugin/src/test/java/ru/abstractmenus/impl/ProviderRegistryImplTest.java @@ -1,7 +1,9 @@ -package ru.abstractmenus.api; +package ru.abstractmenus.impl; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import ru.abstractmenus.api.AbstractMenusApi; +import ru.abstractmenus.api.MenuExtension; import ru.abstractmenus.api.handler.EconomyHandler; import static org.junit.jupiter.api.Assertions.*; diff --git a/plugin/src/test/java/ru/abstractmenus/api/TypeRegistryImplTest.java b/plugin/src/test/java/ru/abstractmenus/impl/TypeRegistryImplTest.java similarity index 96% rename from plugin/src/test/java/ru/abstractmenus/api/TypeRegistryImplTest.java rename to plugin/src/test/java/ru/abstractmenus/impl/TypeRegistryImplTest.java index a422010..f177ab1 100644 --- a/plugin/src/test/java/ru/abstractmenus/api/TypeRegistryImplTest.java +++ b/plugin/src/test/java/ru/abstractmenus/impl/TypeRegistryImplTest.java @@ -1,7 +1,9 @@ -package ru.abstractmenus.api; +package ru.abstractmenus.impl; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; +import ru.abstractmenus.api.AbstractMenusApi; +import ru.abstractmenus.api.MenuExtension; import ru.abstractmenus.hocon.api.ConfigNode; import ru.abstractmenus.hocon.api.serialize.NodeSerializer; import ru.abstractmenus.hocon.api.serialize.NodeSerializers; diff --git a/plugin/src/test/java/ru/abstractmenus/testsupport/ApiTestSupport.java b/plugin/src/test/java/ru/abstractmenus/testsupport/ApiTestSupport.java index 1168305..848d66a 100644 --- a/plugin/src/test/java/ru/abstractmenus/testsupport/ApiTestSupport.java +++ b/plugin/src/test/java/ru/abstractmenus/testsupport/ApiTestSupport.java @@ -9,10 +9,10 @@ import ru.abstractmenus.api.Catalog; import ru.abstractmenus.api.MenuExtension; import ru.abstractmenus.api.ProviderRegistry; -import ru.abstractmenus.api.ProviderRegistryImpl; +import ru.abstractmenus.impl.ProviderRegistryImpl; import ru.abstractmenus.api.Rule; import ru.abstractmenus.api.TypeRegistry; -import ru.abstractmenus.api.TypeRegistryImpl; +import ru.abstractmenus.impl.TypeRegistryImpl; import ru.abstractmenus.api.handler.PlaceholderHandler; import ru.abstractmenus.api.inventory.ItemProperty; import ru.abstractmenus.api.inventory.Menu; From d7af30adc7edb23f026c51f122beca8a9fbd8bd1 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Mon, 27 Apr 2026 17:19:40 +0500 Subject: [PATCH 02/16] fix: round 1 of code-review fixes - Folia correctness (P0-1, P0-2, P1-1, P1-2, P1-3) Previously several entity-touching paths routed through BukkitTasks.runTask or Bukkit.getServer().getGlobalRegionScheduler(). On Folia, the global scheduler is forbidden from touching entity state; doing so causes a threading violation. BukkitTasks now exposes runForEntity / runForEntityLater / runForEntityTimer which dispatch via FoliaLib's entity scheduler on Folia and fall through to runTask* on Spigot/Paper. All scheduling methods also gracefully degrade to synchronous execution when BukkitTasks isn't initialised (pure-unit test contexts). Sites migrated: * MenuManager.UpdateTask - menu.update(player) now dispatched per-player via runForEntity. The outer iteration still runs on the global scheduler; each tick of inventory work happens on the player's region. * InventoryListener.onClick - player.updateInventory now via runForEntityLater. * InventoryListener.onInventoryClose - MenuManager.closeMenu (which calls player.closeInventory) now via runForEntityLater. * ChatListener.onChat - action.input dispatch now via runForEntity (the input callbacks open menus and run actions on the player). * ActionCommand.activate - player.performCommand wrapped in runForEntity; console branch now uses BukkitTasks.runTask instead of direct Bukkit.getServer().getGlobalRegionScheduler() (consistent abstraction). * ActionPlayerChat.activate - messages.forEach(player::chat) wrapped in runForEntity. * ActionTeleport.activate - switched from sync teleport (throws on Folia cross-region) to teleportAsync. TaskHandle gains a NOOP sentinel returned by runTaskTimer/runTaskTimerAsync when BukkitTasks is uninitialised. --- .../data/actions/ActionCommand.java | 28 +++-- .../data/actions/ActionPlayerChat.java | 7 +- .../data/actions/ActionTeleport.java | 5 +- .../abstractmenus/listeners/ChatListener.java | 8 +- .../listeners/InventoryListener.java | 8 +- .../abstractmenus/services/MenuManager.java | 15 ++- .../util/bukkit/BukkitTasks.java | 112 ++++++++++++++++++ .../abstractmenus/util/bukkit/TaskHandle.java | 6 + 8 files changed, 172 insertions(+), 17 deletions(-) diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionCommand.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionCommand.java index bedb95c..07c33c6 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionCommand.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionCommand.java @@ -3,7 +3,6 @@ import org.bukkit.Bukkit; import org.bukkit.entity.Player; -import ru.abstractmenus.AbstractMenus; import ru.abstractmenus.api.Action; import ru.abstractmenus.api.AbstractMenusApi; import ru.abstractmenus.api.inventory.Item; @@ -11,6 +10,8 @@ import ru.abstractmenus.hocon.api.ConfigNode; import ru.abstractmenus.hocon.api.serialize.NodeSerializeException; import ru.abstractmenus.hocon.api.serialize.NodeSerializer; +import ru.abstractmenus.util.bukkit.BukkitTasks; + import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -35,18 +36,29 @@ private void setIgnorePlaceholder(boolean isIgnorePlaceholder) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - for (String command : playerCommands) { - if (command != null) { - String resultCommand = isIgnorePlaceholder ? command : AbstractMenusApi.get().providers().placeholders().replace(player, command); - player.performCommand(resultCommand); - } + if (!playerCommands.isEmpty()) { + // performCommand executes as the player; on Folia this requires + // the player's region thread. + BukkitTasks.runForEntity(player, () -> { + for (String command : playerCommands) { + if (command != null) { + String resultCommand = isIgnorePlaceholder ? command + : AbstractMenusApi.get().providers().placeholders().replace(player, command); + player.performCommand(resultCommand); + } + } + }); } if (!consoleCommands.isEmpty()) { - Bukkit.getServer().getGlobalRegionScheduler().execute(AbstractMenus.instance(), () -> { + // Console commands are not entity-scoped — global scheduler is + // correct on Folia. Route via BukkitTasks for consistency with + // the rest of the codebase. + BukkitTasks.runTask(() -> { for (String command : consoleCommands) { if (command != null) { - String resultCommand = isIgnorePlaceholder ? command : AbstractMenusApi.get().providers().placeholders().replace(player, command); + String resultCommand = isIgnorePlaceholder ? command + : AbstractMenusApi.get().providers().placeholders().replace(player, command); Bukkit.dispatchCommand(Bukkit.getConsoleSender(), resultCommand); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPlayerChat.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPlayerChat.java index 16aacbb..8985598 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPlayerChat.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPlayerChat.java @@ -9,6 +9,7 @@ import ru.abstractmenus.api.inventory.Item; import ru.abstractmenus.api.inventory.Menu; import ru.abstractmenus.util.MiniMessageUtil; +import ru.abstractmenus.util.bukkit.BukkitTasks; import java.util.List; @@ -22,7 +23,11 @@ private ActionPlayerChat(List messages) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - messages.forEach(player::chat); + // player.chat() requires the player's region thread on Folia. The + // activator that fires us may be running on the global scheduler + // (e.g., from a chained chat-input action), so dispatch via the + // entity scheduler. + BukkitTasks.runForEntity(player, () -> messages.forEach(player::chat)); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionTeleport.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionTeleport.java index 9aa3cce..6e7769f 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionTeleport.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionTeleport.java @@ -20,7 +20,10 @@ private ActionTeleport(TypeLocation location) { @Override public void activate(Player player, Menu menu, Item clickedItem) { if (location != null) { - player.teleport(location.getLocation(player, menu)); + // teleportAsync handles cross-region teleports on Folia (the sync + // teleport throws IllegalStateException when crossing regions) and + // is the recommended call on Paper 1.21+ regardless. + player.teleportAsync(location.getLocation(player, menu)); } } diff --git a/plugin/src/main/java/ru/abstractmenus/listeners/ChatListener.java b/plugin/src/main/java/ru/abstractmenus/listeners/ChatListener.java index 2b8d8e6..518e5b5 100644 --- a/plugin/src/main/java/ru/abstractmenus/listeners/ChatListener.java +++ b/plugin/src/main/java/ru/abstractmenus/listeners/ChatListener.java @@ -3,7 +3,6 @@ import io.papermc.paper.event.player.AsyncChatEvent; import org.bukkit.event.EventHandler; import org.bukkit.event.Listener; -import org.bukkit.event.player.AsyncPlayerChatEvent; import ru.abstractmenus.data.actions.ActionInputChat; import ru.abstractmenus.services.MenuManager; import ru.abstractmenus.util.bukkit.BukkitTasks; @@ -16,7 +15,12 @@ public void onChat(AsyncChatEvent event) { .getAndRemoveInputAction(event.getPlayer()); if (action != null) { - BukkitTasks.runTask(() -> action.input(event.signedMessage().message())); + // input() invokes onInput/onCancel actions on the player — + // open menus, give items, etc. On Folia those operations + // require the player's region thread; runTask routes to the + // global scheduler which is forbidden from touching entity state. + BukkitTasks.runForEntity(event.getPlayer(), + () -> action.input(event.signedMessage().message())); event.setCancelled(true); } } diff --git a/plugin/src/main/java/ru/abstractmenus/listeners/InventoryListener.java b/plugin/src/main/java/ru/abstractmenus/listeners/InventoryListener.java index 6c9e2f4..61490b0 100644 --- a/plugin/src/main/java/ru/abstractmenus/listeners/InventoryListener.java +++ b/plugin/src/main/java/ru/abstractmenus/listeners/InventoryListener.java @@ -93,7 +93,9 @@ public void onInventoryClick(InventoryClickEvent event) { } menu.click(event.getSlot(), player, event.getClick()); - BukkitTasks.runTaskLater(player::updateInventory, 2L); + // Pin to player's region: updateInventory writes to the player's + // inventory which on Folia is owned by the player's region thread. + BukkitTasks.runForEntityLater(player, player::updateInventory, 2L); } @EventHandler @@ -146,7 +148,9 @@ public void onInventoryClose(InventoryCloseEvent event) { } if (current != null && current.equals(closed)) { - BukkitTasks.runTaskLater( + // Pin to player's region: closeMenu calls player.closeInventory + // which on Folia must run on the player's region thread. + BukkitTasks.runForEntityLater(player, () -> MenuManager.instance().closeMenu(player, false), 3L ); diff --git a/plugin/src/main/java/ru/abstractmenus/services/MenuManager.java b/plugin/src/main/java/ru/abstractmenus/services/MenuManager.java index b8d9d50..04b89b1 100644 --- a/plugin/src/main/java/ru/abstractmenus/services/MenuManager.java +++ b/plugin/src/main/java/ru/abstractmenus/services/MenuManager.java @@ -430,11 +430,20 @@ private class UpdateTask implements Runnable { public void run() { // openedMenus stores a Player ref alongside each Menu; avoids a // Bukkit.getPlayer(uuid) lookup per entry per tick. + // + // The outer iteration runs on the global region scheduler on + // Folia (or main thread on Spigot/Paper). menu.update() touches + // the player's inventory, which on Folia is owned by the + // player's region thread - so each per-player update is + // dispatched via the entity scheduler. On non-Folia, + // runForEntity falls through to direct invocation, no overhead. for (OpenedMenu entry : openedMenus.values()) { Player player = entry.player(); - if (player.isOnline()) { - entry.menu().update(player); - } + if (!player.isOnline()) continue; + Menu menu = entry.menu(); + BukkitTasks.runForEntity(player, () -> { + if (player.isOnline()) menu.update(player); + }); } } diff --git a/plugin/src/main/java/ru/abstractmenus/util/bukkit/BukkitTasks.java b/plugin/src/main/java/ru/abstractmenus/util/bukkit/BukkitTasks.java index c2b1cf0..8756000 100644 --- a/plugin/src/main/java/ru/abstractmenus/util/bukkit/BukkitTasks.java +++ b/plugin/src/main/java/ru/abstractmenus/util/bukkit/BukkitTasks.java @@ -3,9 +3,27 @@ import com.tcoded.folialib.FoliaLib; import com.tcoded.folialib.wrapper.task.WrappedTask; import lombok.Setter; +import org.bukkit.entity.Entity; import org.bukkit.plugin.Plugin; import org.bukkit.scheduler.BukkitRunnable; +/** + * Scheduling facade. + * + *

Why entity-aware variants exist. On Folia, + * {@link #runTask(Runnable)} / {@link #runTaskLater(Runnable, long)} / + * {@link #runTaskTimer(Runnable, long, long)} route to the global region + * scheduler. The global scheduler is forbidden from touching entity state + * (player.isOnline, inventory.setItem, player.teleport, etc.) - those + * calls require the entity's owning region thread or they throw a + * threading violation at runtime. + * + *

For any task that interacts with a specific player or entity, use the + * {@code runForEntity*} variants below. On Folia they dispatch to the + * entity's region scheduler; on Spigot/Paper non-Folia they fall through + * to the regular main-thread scheduler (where entity affinity isn't a + * concern). + */ public final class BukkitTasks { @Setter @@ -16,7 +34,21 @@ public final class BukkitTasks { private BukkitTasks() { } + /** + * True if the plugin lifecycle has wired up scheduling. False in pure-unit + * tests that exercise actions/rules without booting the plugin. When false, + * every scheduling method falls through to running the {@link Runnable} + * synchronously on the calling thread, which matches what tests assume. + */ + private static boolean initialised() { + return foliaLib != null && plugin != null; + } + public static void runTask(Runnable runnable) { + if (!initialised()) { + runnable.run(); + return; + } if (foliaLib.isFolia()) { foliaLib.getScheduler().runNextTick(task -> runnable.run()); return; @@ -29,6 +61,10 @@ public void run() { } public static void runTaskAsync(Runnable runnable) { + if (!initialised()) { + runnable.run(); + return; + } if (foliaLib.isFolia()) { foliaLib.getScheduler().runAsync(task -> runnable.run()); return; @@ -41,6 +77,10 @@ public void run() { } public static void runTaskLater(Runnable runnable, long delay) { + if (!initialised()) { + runnable.run(); + return; + } if (foliaLib.isFolia()) { foliaLib.getScheduler().runLater(runnable, delay); return; @@ -53,6 +93,10 @@ public void run() { } public static void runTaskLaterAsync(Runnable runnable, long delay) { + if (!initialised()) { + runnable.run(); + return; + } if (foliaLib.isFolia()) { foliaLib.getScheduler().runLaterAsync(runnable, delay); return; @@ -65,6 +109,10 @@ public void run() { } public static TaskHandle runTaskTimer(Runnable runnable, long delay, long period) { + if (!initialised()) { + runnable.run(); + return TaskHandle.NOOP; + } if (foliaLib.isFolia()) { WrappedTask wrappedTask = foliaLib.getScheduler().runTimer(runnable, delay, period); return new TaskHandle() { @@ -100,7 +148,71 @@ public boolean isCancelled() { } } + // ----------------------------------------------------------------- + // Entity-aware variants + // + // Use these when the work touches a specific player/entity. On Folia + // they dispatch via the entity's region scheduler; on non-Folia they + // fall through to runTask*. If the entity has been removed by the + // time the task fires, FoliaLib silently drops it (no work runs). + // ----------------------------------------------------------------- + + /** Run {@code runnable} on {@code entity}'s region (Folia) or next tick (non-Folia). */ + public static void runForEntity(Entity entity, Runnable runnable) { + if (!initialised()) { + runnable.run(); + return; + } + if (foliaLib.isFolia()) { + foliaLib.getScheduler().runAtEntity(entity, task -> runnable.run()); + return; + } + runTask(runnable); + } + + /** Run {@code runnable} on {@code entity}'s region after {@code delay} ticks. */ + public static void runForEntityLater(Entity entity, Runnable runnable, long delay) { + if (!initialised()) { + runnable.run(); + return; + } + if (foliaLib.isFolia()) { + foliaLib.getScheduler().runAtEntityLater(entity, runnable, delay); + return; + } + runTaskLater(runnable, delay); + } + + /** + * Repeating task pinned to {@code entity}'s region (Folia) or main thread + * (non-Folia). On Folia the timer auto-stops if the entity is removed. + */ + public static TaskHandle runForEntityTimer(Entity entity, Runnable runnable, long delay, long period) { + if (!initialised()) { + runnable.run(); + return TaskHandle.NOOP; + } + if (foliaLib.isFolia()) { + WrappedTask wrappedTask = foliaLib.getScheduler() + .runAtEntityTimer(entity, runnable, delay, period); + return new TaskHandle() { + @Override public void cancel() { wrappedTask.cancel(); } + @Override public boolean isCancelled() { return wrappedTask.isCancelled(); } + }; + } + return runTaskTimer(runnable, delay, period); + } + + /** True iff running on Folia. Useful for callers that need to branch on platform directly. */ + public static boolean isFolia() { + return initialised() && foliaLib.isFolia(); + } + public static TaskHandle runTaskTimerAsync(Runnable runnable, long delay, long period) { + if (!initialised()) { + runnable.run(); + return TaskHandle.NOOP; + } if (foliaLib.isFolia()) { WrappedTask wrappedTask = foliaLib.getScheduler().runTimerAsync(runnable, delay, period); return new TaskHandle() { diff --git a/plugin/src/main/java/ru/abstractmenus/util/bukkit/TaskHandle.java b/plugin/src/main/java/ru/abstractmenus/util/bukkit/TaskHandle.java index 2c5d449..4d2f079 100644 --- a/plugin/src/main/java/ru/abstractmenus/util/bukkit/TaskHandle.java +++ b/plugin/src/main/java/ru/abstractmenus/util/bukkit/TaskHandle.java @@ -4,4 +4,10 @@ public interface TaskHandle { void cancel(); boolean isCancelled(); + + /** Sentinel returned for tests / uninitialised contexts where no real timer was scheduled. */ + TaskHandle NOOP = new TaskHandle() { + @Override public void cancel() {} + @Override public boolean isCancelled() { return true; } + }; } \ No newline at end of file From 1f83137e36dec5717ff2bc5970ca000bbc67a3ed Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 12:03:23 +0500 Subject: [PATCH 03/16] release: bump to 2.0.0-alpha.2 Cumulative breaking changes since 2.0.0-alpha: - Logger.set is now set-once (an addon cannot replace the global logger to silence or capture core's output). - TypeRegistry.unregisterAll(MenuExtension) and ProviderRegistry.unregisterAll(MenuExtension) removed from the public API surface. Cleanup is internal to AbstractMenus' addon manager. Addons cannot wipe each other's registrations through the documented API. - AbstractMenusApiImpl, TypeRegistryImpl, ProviderRegistryImpl moved from ru.abstractmenus.api.* to ru.abstractmenus.impl.* so the impl classes no longer leak package-private visibility into the public API package. Plus Folia correctness fixes (entity-aware BukkitTasks variants, sync-teleport replaced with teleportAsync, etc.) and the TypeRegistry NodeSerializers cleanup that was leaking closed classloaders. PlayerPointsAddon needs a rebuild against this version to pick up the relocated impl classes (it doesn't reference them directly, but recompilation refreshes its bundled api jar). README badge bumped from 1.18.0 (very stale) to 2.0.0-alpha.2 and the broken 'AbstractMenus/plugin' org/repo links pointed at the live 'minecraft-plugin' repo. Build output path updated to reflect the multi-module + dist aggregator layout. --- README.md | 8 +++++--- .../main/java/ru/abstractmenus/api/AbstractMenusApi.java | 2 +- build.gradle | 2 +- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index b875795..f9c80c8 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ # AbstractMenus -license -version +license +version minecraft paper java @@ -79,7 +79,9 @@ cd AbstractMenus ./gradlew shadowJar ``` -Output: `build/libs/AbstractMenus-.jar`. +Output: `build/dist/AbstractMenus-.jar` (the aggregator also drops the +api jar + sources + javadoc next to it; per-module jars stay in +`plugin/build/libs/` and `api/build/libs/`). Other useful tasks: diff --git a/api/src/main/java/ru/abstractmenus/api/AbstractMenusApi.java b/api/src/main/java/ru/abstractmenus/api/AbstractMenusApi.java index 611909c..f53962f 100644 --- a/api/src/main/java/ru/abstractmenus/api/AbstractMenusApi.java +++ b/api/src/main/java/ru/abstractmenus/api/AbstractMenusApi.java @@ -134,7 +134,7 @@ static AbstractMenusApi get() { * diagnostic — logged on startup and surfaced in * {@code /am addons list}. * - * @return the API version (e.g. {@code "2.0.0-alpha"}) + * @return the API version (e.g. {@code "2.0.0-alpha.2"}) */ String apiVersion(); } diff --git a/build.gradle b/build.gradle index 997e5f5..2417ef7 100644 --- a/build.gradle +++ b/build.gradle @@ -9,7 +9,7 @@ plugins { allprojects { group = 'ru.abstractmenus' - version = '2.0.0-alpha' + version = '2.0.0-alpha.2' repositories { mavenLocal() From ae2199c45605addd763d6a5fd9fcf827569973f0 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 12:11:14 +0500 Subject: [PATCH 04/16] fix: round 2 of code-review fixes (P1-9 / P1-10 / P1-11 / P1-12) P1-9 - pluginSoftDependencies in addon.conf is now actually consulted. loadAll/enableSingle log a warning per missing soft-dep at addon enable time, then proceed (soft means soft - no markFailed). The field had been parsed and stored since Phase B.2 but no consumer read it. P1-10 - extracted checkPluginDeps(LoadedAddon) private helper. loadAll and enableSingle previously duplicated the iteration with slightly different formulations (different log message wording, different early exit shapes). Now both delegate to one helper. Side benefit: the test mode (plugin == null) skip lives in one place. P1-11 - ProviderRegistryImpl.resolveWithConfig is now atomic. Previously it called configDefaults.apply(kind), then section.byId(), then section.auto() as separate operations, each section call acquiring its own monitor. On Folia (or any context where addon reload is concurrent with provider lookup) there was a window where the configured id read referred to a provider the unregister had just dropped. Section now exposes resolveWithDefault(String) which holds one lock across both the byId-by-configured-id and the auto-resolve fallback. P1-12 - ActionLuckPermsMetaSet/Remove now log a warning when the active permissions provider is not LuckPerms (instead of silently no-op'ing). The operator's menu config did request meta operations, so quietly dropping them is the wrong default - now they get a one-line server-log hint pointing at config.conf providers.permissions = "luckperms". --- .../ru/abstractmenus/addon/AddonManager.java | 85 +++++++++++-------- .../actions/ActionLuckPermsMetaRemove.java | 16 ++-- .../data/actions/ActionLuckPermsMetaSet.java | 17 +++- .../impl/ProviderRegistryImpl.java | 29 +++++-- 4 files changed, 96 insertions(+), 51 deletions(-) diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java index 3011cf1..9545f4e 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java @@ -60,32 +60,13 @@ public void loadAll() { return; } - // Soft-filter: drop addons whose required Bukkit plugin deps are missing. - // In test mode (plugin == null), skip this check — tests must not declare - // pluginDependencies. + // Filter out addons whose hard plugin deps are missing. Soft deps + // log a warning but do not block the addon. var byName = new LinkedHashMap(); - if (plugin != null) { - var pluginManager = plugin.getServer().getPluginManager(); - for (LoadedAddon la : pending.values()) { - AddonConf c = la.getConf(); - boolean missing = false; - for (String dep : c.pluginDependencies()) { - if (pluginManager.getPlugin(dep) == null) { - Logger.warning("Addon " + c.name() - + " requires plugin '" + dep + "' which is not installed — skipping"); - la.markFailed(new IllegalStateException("missing plugin dependency: " + dep)); - missing = true; - break; - } - } - if (missing) { - addons.put(c.name().toLowerCase(), la); // keep the failed entry visible in /am addons list - continue; - } - byName.put(c.name().toLowerCase(), la); + for (LoadedAddon la : pending.values()) { + if (checkPluginDeps(la)) { + byName.put(la.getConf().name().toLowerCase(), la); } - } else { - byName.putAll(pending); } if (byName.isEmpty()) return; @@ -450,6 +431,49 @@ private List scanAvailableNotLoaded() { return result; } + /** + * Verify hard {@code pluginDependencies} are present and log warnings + * for any missing {@code pluginSoftDependencies}. + * + *

If a hard dep is missing the addon is marked FAILED and parked + * in the loaded map (so {@code /am addons list} surfaces it) and the + * method returns false. Soft-dep misses log a warning but do not + * block enabling. + * + *

Skipped entirely in test mode (plugin == null). Tests must not + * declare pluginDependencies; declaring pluginSoftDependencies is fine + * but logs nothing. + * + * @return true if the addon may proceed to enable, false if a hard + * dependency is missing + */ + private boolean checkPluginDeps(LoadedAddon la) { + if (plugin == null) return true; + var pm = plugin.getServer().getPluginManager(); + AddonConf c = la.getConf(); + String key = c.name().toLowerCase(); + + for (String dep : c.pluginDependencies()) { + if (pm.getPlugin(dep) == null) { + String msg = "missing plugin dependency: " + dep; + Logger.warning("Addon " + c.name() + " " + msg + " - skipping"); + la.markFailed(new IllegalStateException(msg)); + addons.put(key, la); + return false; + } + } + + for (String dep : c.pluginSoftDependencies()) { + if (pm.getPlugin(dep) == null) { + Logger.warning("Addon " + c.name() + + " soft-depends on plugin '" + dep + + "' which is not installed - features that need it may no-op"); + } + } + + return true; + } + /** * Verify Bukkit-side and addon-side dependencies, then run * onLoad + onEnable. Installs the result into the loaded map @@ -459,18 +483,7 @@ private List scanAvailableNotLoaded() { private void enableSingle(LoadedAddon la) { String key = la.getConf().name().toLowerCase(); - if (plugin != null) { - var pm = plugin.getServer().getPluginManager(); - for (String dep : la.getConf().pluginDependencies()) { - if (pm.getPlugin(dep) == null) { - String msg = "missing plugin dependency: " + dep; - Logger.warning("Addon " + la.getConf().name() + " " + msg); - la.markFailed(new IllegalStateException(msg)); - addons.put(key, la); - return; - } - } - } + if (!checkPluginDeps(la)) return; for (String dep : la.getConf().addonDependencies()) { LoadedAddon depAddon = addons.get(dep.toLowerCase()); diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaRemove.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaRemove.java index 88a1260..efa114d 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaRemove.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaRemove.java @@ -5,6 +5,8 @@ import org.bukkit.entity.Player; import ru.abstractmenus.api.Action; import ru.abstractmenus.api.AbstractMenusApi; +import ru.abstractmenus.api.Logger; +import ru.abstractmenus.api.handler.PermissionsHandler; import ru.abstractmenus.api.inventory.Item; import ru.abstractmenus.api.inventory.Menu; import ru.abstractmenus.handlers.LuckPermsHandler; @@ -21,11 +23,15 @@ public class ActionLuckPermsMetaRemove implements Action { @Override public void activate(Player player, Menu menu, Item clickedItem) { - metaList.forEach(metaKey -> { - if (AbstractMenusApi.get().providers().permissions() instanceof LuckPermsHandler handler) { - handler.removeMeta(player, metaKey); - } - }); + PermissionsHandler perms = AbstractMenusApi.get().providers().permissions(); + if (!(perms instanceof LuckPermsHandler handler)) { + Logger.warning("lpMetaRemove skipped: active permissions provider " + + (perms == null ? "null" : perms.getClass().getSimpleName()) + + " is not LuckPerms. " + + "Install LuckPerms or set 'providers.permissions = \"luckperms\"' in config.conf."); + return; + } + metaList.forEach(metaKey -> handler.removeMeta(player, metaKey)); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaSet.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaSet.java index 377a474..ab3e9c8 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaSet.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaSet.java @@ -5,6 +5,8 @@ import org.bukkit.entity.Player; import ru.abstractmenus.api.Action; import ru.abstractmenus.api.AbstractMenusApi; +import ru.abstractmenus.api.Logger; +import ru.abstractmenus.api.handler.PermissionsHandler; import ru.abstractmenus.api.inventory.Item; import ru.abstractmenus.api.inventory.Menu; import ru.abstractmenus.data.properties.PropLPMeta; @@ -23,11 +25,18 @@ public class ActionLuckPermsMetaSet implements Action { @Override public void activate(Player player, Menu menu, Item clickedItem) { + PermissionsHandler perms = AbstractMenusApi.get().providers().permissions(); + if (!(perms instanceof LuckPermsHandler handler)) { + Logger.warning("lpMetaSet skipped: active permissions provider " + + (perms == null ? "null" : perms.getClass().getSimpleName()) + + " is not LuckPerms. " + + "Install LuckPerms or set 'providers.permissions = \"luckperms\"' in config.conf."); + return; + } metaList.forEach(meta -> { - String replacedValue = isIgnorePlaceholder ? meta.getValue() : AbstractMenusApi.get().providers().placeholders().replace(player, meta.getValue()); - if (AbstractMenusApi.get().providers().permissions() instanceof LuckPermsHandler handler) { - handler.addMeta(player, meta.getKey(), replacedValue); - } + String replacedValue = isIgnorePlaceholder ? meta.getValue() + : AbstractMenusApi.get().providers().placeholders().replace(player, meta.getValue()); + handler.addMeta(player, meta.getKey(), replacedValue); }); } diff --git a/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java index 2fb925a..f49ca9d 100644 --- a/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java +++ b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java @@ -47,13 +47,11 @@ public void setConfigDefaults(Function lookup) { // ---- Config-default resolution helper -------------------------------- private T resolveWithConfig(String kind, Section section) { + // configDefaults is read here outside the section lock; that's + // intentional. The Function is only mutated once via setConfigDefaults + // during plugin startup, before any concurrent reads can happen. String configured = configDefaults.apply(kind); - if (configured != null && !configured.equalsIgnoreCase("auto")) { - T h = section.byId(configured); - if (h != null) return h; - // Configured id not found — fall back to auto. - } - return section.auto(); + return section.resolveWithDefault(configured); } // ---- Economy --------------------------------------------------------- @@ -141,6 +139,25 @@ synchronized T auto() { return best == null ? null : best.handler; } + /** + * Resolve a provider in one critical section: try the configured id + * first, fall back to {@link #auto()} on miss. Holding the lock + * across both lookups closes the race where a concurrent register / + * unregister could move the configured id under our feet between + * the two separate calls. + */ + synchronized T resolveWithDefault(String configuredId) { + if (configuredId != null && !configuredId.equalsIgnoreCase("auto")) { + Entry e = byId.get(configuredId.toLowerCase()); + if (e != null) return e.handler; + } + Entry best = null; + for (Entry e : byId.values()) { + if (best == null || e.priority > best.priority) best = e; + } + return best == null ? null : best.handler; + } + synchronized Collection all() { List list = new ArrayList<>(byId.size()); for (Entry e : byId.values()) list.add(e.handler); From 3163ffcb34c44b1751f6367e06f07b506b918da0 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 12:26:57 +0500 Subject: [PATCH 05/16] =?UTF-8?q?refactor(api)!:=20ProviderRegistry=20?= =?UTF-8?q?=E2=86=92=20ProviderSection=20parametric=20shape?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ProviderRegistry had 25 parallel methods (5 per section × economy / permissions / levels / placeholders / skins) - each section's register / get / get(id) / all / has lived directly on the registry interface. Adding a sixth provider type would have meant adding 5+ methods on the public interface and threading a new field through every consumer that wanted to enumerate sections. Refactored to ProviderSection with one shape per section: void register(String id, T handler, int priority, MenuExtension owner); T resolve(); T resolve(String id); Collection all(); boolean has(String id); ProviderRegistry now exposes 5 typed-section getters: ProviderSection economy() ProviderSection permissions() ProviderSection levels() ProviderSection placeholders() ProviderSection skins() Migration: api.providers().registerEconomy(id, h, p, o) -> api.providers().economy().register(id, h, p, o) api.providers().economy() (returns EconomyHandler) -> api.providers().economy().resolve() api.providers().economy("vault") (returns EconomyHandler by id) -> api.providers().economy().resolve("vault") api.providers().allEconomy() -> api.providers().economy().all() api.providers().hasEconomy(id) -> api.providers().economy().has(id) (Same shape for permissions / levels / placeholders / skins.) Adopted across 128 in-tree call sites. The atomic resolve-with-default that landed in P1-11 is now baked into ProviderSection.resolve() - the single critical section guarantees that the configured-id read and the priority-fallback selection cannot race with a concurrent register / unregister. Breaking for downstream consumers. PlayerPointsAddon (separate repo) needs a one-line update on the call site: api.providers().registerEconomy(...) -> api.providers().economy().register(...) --- .../abstractmenus/api/ProviderRegistry.java | 81 +++------ .../ru/abstractmenus/api/ProviderSection.java | 95 +++++++++++ .../ru/abstractmenus/command/Command.java | 2 +- .../core/CoreProvidersBundle.java | 14 +- .../data/actions/ActionBookOpen.java | 2 +- .../data/actions/ActionBroadcast.java | 10 +- .../data/actions/ActionBungeeConnect.java | 2 +- .../data/actions/ActionCommand.java | 4 +- .../data/actions/ActionGroupAdd.java | 4 +- .../data/actions/ActionGroupRemove.java | 4 +- .../data/actions/ActionInputChat.java | 2 +- .../data/actions/ActionLevelGive.java | 2 +- .../data/actions/ActionLevelTake.java | 2 +- .../abstractmenus/data/actions/ActionLog.java | 2 +- .../actions/ActionLuckPermsMetaRemove.java | 2 +- .../data/actions/ActionLuckPermsMetaSet.java | 4 +- .../data/actions/ActionMenuOpen.java | 2 +- .../data/actions/ActionMenuOpenCtx.java | 2 +- .../data/actions/ActionMessage.java | 10 +- .../data/actions/ActionMiniMessage.java | 2 +- .../data/actions/ActionMoneyGive.java | 8 +- .../data/actions/ActionMoneyTake.java | 8 +- .../data/actions/ActionPermissionGive.java | 4 +- .../data/actions/ActionPermissionRemove.java | 4 +- .../data/actions/ActionSkinReset.java | 2 +- .../data/actions/ActionSkinSet.java | 6 +- .../data/actions/ActionXpGive.java | 2 +- .../data/actions/ActionXpTake.java | 2 +- .../data/actions/var/ActionVarDec.java | 4 +- .../data/actions/var/ActionVarDiv.java | 4 +- .../data/actions/var/ActionVarInc.java | 4 +- .../data/actions/var/ActionVarMul.java | 4 +- .../data/actions/var/ActionVarRem.java | 4 +- .../data/actions/var/ActionVarSet.java | 8 +- .../data/actions/varp/ActionVarpDec.java | 2 +- .../data/actions/varp/ActionVarpDiv.java | 2 +- .../data/actions/varp/ActionVarpInc.java | 2 +- .../data/actions/varp/ActionVarpMul.java | 2 +- .../data/actions/varp/ActionVarpRem.java | 2 +- .../data/actions/varp/ActionVarpSet.java | 6 +- .../actions/wrappers/ActionPlayerScope.java | 2 +- .../data/activators/OpenChat.java | 2 +- .../data/activators/OpenChatContains.java | 2 +- .../data/activators/OpenClickEntity.java | 2 +- .../data/activators/OpenRegionEnter.java | 2 +- .../data/activators/OpenRegionLeave.java | 2 +- .../data/activators/OpenShiftClickEntity.java | 2 +- .../data/activators/OpenSign.java | 2 +- .../data/catalogs/SliceCatalog.java | 2 +- .../comparator/LegacyValueComparator.java | 8 +- .../comparator/ModernValueComparator.java | 2 +- .../data/properties/PropBookData.java | 6 +- .../data/properties/PropEquipItem.java | 2 +- .../data/properties/PropHDB.java | 2 +- .../data/properties/PropItemsAdder.java | 2 +- .../data/properties/PropLore.java | 2 +- .../data/properties/PropLoreLight.java | 2 +- .../data/properties/PropMmoItem.java | 2 +- .../data/properties/PropName.java | 2 +- .../data/properties/PropNameLight.java | 2 +- .../data/properties/PropOraxen.java | 2 +- .../data/properties/PropSerialized.java | 2 +- .../data/properties/PropSkullOwner.java | 2 +- .../data/properties/PropTexture.java | 2 +- .../data/rules/RuleBungeeIsOnline.java | 2 +- .../data/rules/RuleBungeeOnline.java | 2 +- .../data/rules/RuleExistVar.java | 4 +- .../data/rules/RuleExistVarp.java | 2 +- .../abstractmenus/data/rules/RuleGroup.java | 4 +- .../ru/abstractmenus/data/rules/RuleJS.java | 2 +- .../abstractmenus/data/rules/RuleLevel.java | 2 +- .../abstractmenus/data/rules/RuleMoney.java | 8 +- .../data/rules/RulePermission.java | 4 +- .../data/rules/RulePlayerIsOnline.java | 2 +- .../abstractmenus/data/rules/RuleRegion.java | 2 +- .../ru/abstractmenus/data/rules/RuleXp.java | 2 +- .../data/rules/logical/RulePlayerScope.java | 2 +- .../ru/abstractmenus/datatype/DataType.java | 2 +- .../extractors/PlayerExtractor.java | 2 +- .../impl/ProviderRegistryImpl.java | 160 +++++++----------- .../ru/abstractmenus/menu/AbstractMenu.java | 2 +- .../data/MoneyProviderSelectionTest.java | 4 +- .../actions/TestActionCommandBehavior.java | 2 +- .../impl/ProviderRegistryImplTest.java | 76 ++++----- .../testsupport/ApiTestSupport.java | 4 +- 85 files changed, 352 insertions(+), 324 deletions(-) create mode 100644 api/src/main/java/ru/abstractmenus/api/ProviderSection.java diff --git a/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java b/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java index 47049f6..f238a24 100644 --- a/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java +++ b/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java @@ -6,83 +6,54 @@ import ru.abstractmenus.api.handler.PlaceholderHandler; import ru.abstractmenus.api.handler.SkinHandler; -import java.util.Collection; - /** * Registry of pluggable handler providers (economy, permissions, levels, - * placeholders, skins). Replaces the old static {@code Handlers.set*()/get*()} - * facade with an owner-aware registry that supports multiple providers per - * section plus priority-based auto-resolution. + * placeholders, skins). Replaces the old static {@code Handlers} facade with + * an owner-aware registry that supports multiple providers per section plus + * priority-based auto-resolution and a configurable default. + * + *

Each section ({@link #economy()}, {@link #permissions()}, + * {@link #levels()}, {@link #placeholders()}, {@link #skins()}) returns a + * typed {@link ProviderSection} you register on and resolve from. The + * registry itself is just five getters - all per-type behaviour lives on + * {@link ProviderSection}, so adding a sixth provider type later means + * adding one method here, not five. * *

Registration

* * 
{@code
  * public final class MyEconomyAddon implements MenuExtension {
  *     @Override public void onEnable(AbstractMenusApi api) {
- *         api.providers().registerEconomy(
+ *         api.providers().economy().register(
  *             "playerpoints",
  *             new PlayerPointsEconomy(pp),
- *             100,          // priority — higher wins in auto-resolve
- *             this);        // owner — AbstractMenus uses this for cleanup on reload
+ *             100,          // priority - higher wins in auto-resolve
+ *             this);        // owner - AbstractMenus uses this for cleanup
  *     }
  * }
  * }
* - *

Resolution

- * - *
    - *
  • {@link #economy()} — highest-priority registered handler, or - * first-registered on ties. Returns {@code null} if none registered.
    • - *
  • {@link #economy(String)} — explicit lookup by id.
    • - *
  • {@link #allEconomy()} — every registration, for introspection.
    • - *
  • {@link #hasEconomy(String)} — validation helper (used by - * menu-serializers to fail-at-load when a HOCON file references an - * unknown provider).
    • - *
+ *

Lookup

* - *

Same shape for permissions / levels / placeholders / skins. + * 

{@code
+ * EconomyHandler eco       = api.providers().economy().resolve();
+ * EconomyHandler vault     = api.providers().economy().resolve("vault");
+ * boolean hasPP            = api.providers().economy().has("playerpoints");
+ * Collection all = api.providers().economy().all();
+ * }
* + * @see ProviderSection * @see AbstractMenusApi#providers() */ public interface ProviderRegistry { - // ---- Economy --------------------------------------------------------- - - void registerEconomy(String id, EconomyHandler handler, int priority, MenuExtension owner); - EconomyHandler economy(); - EconomyHandler economy(String id); - Collection allEconomy(); - boolean hasEconomy(String id); - - // ---- Permissions ----------------------------------------------------- - - void registerPermissions(String id, PermissionsHandler handler, int priority, MenuExtension owner); - PermissionsHandler permissions(); - PermissionsHandler permissions(String id); - Collection allPermissions(); - boolean hasPermissions(String id); - - // ---- Levels ---------------------------------------------------------- - - void registerLevels(String id, LevelHandler handler, int priority, MenuExtension owner); - LevelHandler levels(); - LevelHandler levels(String id); - Collection allLevels(); - boolean hasLevels(String id); + ProviderSection economy(); - // ---- Placeholders ---------------------------------------------------- + ProviderSection permissions(); - void registerPlaceholders(String id, PlaceholderHandler handler, int priority, MenuExtension owner); - PlaceholderHandler placeholders(); - PlaceholderHandler placeholders(String id); - Collection allPlaceholders(); - boolean hasPlaceholders(String id); + ProviderSection levels(); - // ---- Skins ----------------------------------------------------------- + ProviderSection placeholders(); - void registerSkins(String id, SkinHandler handler, int priority, MenuExtension owner); - SkinHandler skins(); - SkinHandler skins(String id); - Collection allSkins(); - boolean hasSkins(String id); + ProviderSection skins(); } diff --git a/api/src/main/java/ru/abstractmenus/api/ProviderSection.java b/api/src/main/java/ru/abstractmenus/api/ProviderSection.java new file mode 100644 index 0000000..840e93d --- /dev/null +++ b/api/src/main/java/ru/abstractmenus/api/ProviderSection.java @@ -0,0 +1,95 @@ +package ru.abstractmenus.api; + +import java.util.Collection; + +/** + * One section of the {@link ProviderRegistry}, holding registered handlers + * of a single provider type (economy, permissions, levels, placeholders, + * or skins). + * + *

Each handler is registered under a string id (e.g. {@code "vault"}, + * {@code "playerpoints"}), with a priority for auto-resolution and an + * owning {@link MenuExtension} for cleanup. Sections are mirrors of one + * another structurally; {@code ProviderRegistry.economy()} returns a + * {@code ProviderSection}, and so on. There is no per-type + * boilerplate on the registry interface. + * + *

Resolution

+ * + *
    + *
  • {@link #resolve()} returns the highest-priority registered handler, + * overridden by the {@code config.conf providers.} value if it + * names a registered handler. Returns {@code null} if the section is + * empty.
    • + *
  • {@link #resolve(String)} returns the explicitly-named handler, or + * {@code null} if no handler with that id is registered.
    • + *
+ * + *

Example

+ * + * 
{@code
+ * // Register a custom economy provider in onEnable:
+ * api.providers().economy().register(
+ *     "playerpoints", new PlayerPointsEconomy(pp), 100, this);
+ *
+ * // From an action, ask for the configured/auto-resolved economy:
+ * EconomyHandler eco = api.providers().economy().resolve();
+ *
+ * // Or for a specific provider by id:
+ * EconomyHandler vault = api.providers().economy().resolve("vault");
+ * }
+ * + *

Thread-safety

+ * + * Implementations are expected to be thread-safe for concurrent reads and + * writes - addon enable/disable can race against extractor lookups on Folia + * since regions run independently. The reference implementation + * synchronises on the section instance and resolves with-default + * atomically (so a concurrent unregister cannot leave the resolver + * pointing at a freed handler). + */ +public interface ProviderSection { + + /** + * Register a handler under {@code id}. If an entry already exists for + * {@code id}, it is replaced. {@code priority} controls auto-resolve + * order: highest wins. {@code owner} is the {@link MenuExtension} that + * registered this entry; AbstractMenus' addon manager uses it for + * cleanup on disable / reload. + * + * @param id case-insensitive identifier (e.g. {@code "vault"}) + * @param handler the handler instance + * @param priority higher wins in {@link #resolve()} when no config + * default is set; core providers register at 50, addons + * typically at 100 + * @param owner the registering extension; used for cleanup + */ + void register(String id, T handler, int priority, MenuExtension owner); + + /** + * Resolve the handler that should serve "default" lookups. Tries the + * configured id from {@code config.conf providers.} first; if + * missing or set to {@code "auto"}, falls back to the highest-priority + * registered handler. + * + * @return a registered handler, or {@code null} if the section is empty + */ + T resolve(); + + /** + * Resolve a specific handler by id. + * + * @param id case-insensitive identifier + * @return the registered handler, or {@code null} if not registered + */ + T resolve(String id); + + /** All registered handlers, in registration order. Read-only snapshot. */ + Collection all(); + + /** + * @param id case-insensitive identifier + * @return whether a handler with this id is registered + */ + boolean has(String id); +} diff --git a/plugin/src/main/java/ru/abstractmenus/command/Command.java b/plugin/src/main/java/ru/abstractmenus/command/Command.java index 742f805..a8366de 100644 --- a/plugin/src/main/java/ru/abstractmenus/command/Command.java +++ b/plugin/src/main/java/ru/abstractmenus/command/Command.java @@ -68,7 +68,7 @@ public void execute(CommandSender sender, List args) { } catch (Throwable t) { if (arg.getDef() != null && sender instanceof Player) { Player player = (Player) sender; - String def = AbstractMenusApi.get().providers().placeholders().replace(player, arg.getDef()); + String def = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, arg.getDef()); try { Object defObj = arg.parse(sender, def); diff --git a/plugin/src/main/java/ru/abstractmenus/core/CoreProvidersBundle.java b/plugin/src/main/java/ru/abstractmenus/core/CoreProvidersBundle.java index aa63fc6..2de9473 100644 --- a/plugin/src/main/java/ru/abstractmenus/core/CoreProvidersBundle.java +++ b/plugin/src/main/java/ru/abstractmenus/core/CoreProvidersBundle.java @@ -38,7 +38,7 @@ void register(AbstractMenusApi api, MenuExtension owner) { Bukkit.getServicesManager().getRegistration(Economy.class); if (economyProvider != null) { - api.providers().registerEconomy( + api.providers().economy().register( "vault", new EconomyVaultHandler(economyProvider.getProvider()), CORE_PRIORITY, @@ -57,7 +57,7 @@ void register(AbstractMenusApi api, MenuExtension owner) { Bukkit.getServicesManager().getRegistration(LuckPerms.class); if (provider != null) { - api.providers().registerPermissions( + api.providers().permissions().register( "luckperms", new LuckPermsHandler(provider.getProvider()), CORE_PRIORITY, @@ -67,7 +67,7 @@ void register(AbstractMenusApi api, MenuExtension owner) { Logger.severe("Cannot find registered LuckPerms service"); } } else { - api.providers().registerPermissions( + api.providers().permissions().register( "default", new PermissionDefaultHandler(plugin), CORE_PRIORITY, @@ -77,7 +77,7 @@ void register(AbstractMenusApi api, MenuExtension owner) { } // ---- Levels --------------------------------------------------------- - api.providers().registerLevels( + api.providers().levels().register( "default", new LevelDefaultHandler(), CORE_PRIORITY, @@ -86,7 +86,7 @@ void register(AbstractMenusApi api, MenuExtension owner) { // ---- Placeholders --------------------------------------------------- if (AbstractMenus.checkDependency("PlaceholderAPI")) { PlaceholderCustomHandler papiHandler = new PlaceholderCustomHandler(); - api.providers().registerPlaceholders( + api.providers().placeholders().register( "placeholderapi", papiHandler, CORE_PRIORITY, @@ -95,7 +95,7 @@ void register(AbstractMenusApi api, MenuExtension owner) { Logger.info("Using PlaceholderAPI"); } else { PlaceholderDefaultHandler defaultHandler = new PlaceholderDefaultHandler(); - api.providers().registerPlaceholders( + api.providers().placeholders().register( "default", defaultHandler, CORE_PRIORITY, @@ -106,7 +106,7 @@ void register(AbstractMenusApi api, MenuExtension owner) { // ---- Skins ---------------------------------------------------------- if (AbstractMenus.checkDependency("SkinsRestorer")) { - api.providers().registerSkins( + api.providers().skins().register( "skinsrestorer", new SkinsRestorerHandler(plugin.isProxyMode, plugin), CORE_PRIORITY, diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBookOpen.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBookOpen.java index 308f6aa..4273e0e 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBookOpen.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBookOpen.java @@ -23,7 +23,7 @@ private ActionBookOpen(BookData bookData) { @Override public void activate(Player player, Menu menu, Item clickedItem) { BookData data = bookData.clone(); - PlaceholderHandler handler = AbstractMenusApi.get().providers().placeholders(); + PlaceholderHandler handler = AbstractMenusApi.get().providers().placeholders().resolve(); data.setAuthor(handler.replace(player, bookData.getAuthor())); data.setTitle(handler.replace(player, bookData.getTitle())); diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBroadcast.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBroadcast.java index c10e963..054fef1 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBroadcast.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBroadcast.java @@ -70,7 +70,7 @@ private void setStay(TypeInt stay) { public void activate(Player player, Menu menu, Item clickedItem) { if (player != null) { if (chatMessages != null) { - List replaced = AbstractMenusApi.get().providers().placeholders().replace(player, chatMessages); + List replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, chatMessages); for (Player p : Bukkit.getOnlinePlayers()) { MiniMessageUtil.sendParsed(replaced, p); @@ -79,7 +79,7 @@ public void activate(Player player, Menu menu, Item clickedItem) { if (json != null) { BaseComponent[] component = ComponentSerializer.parse( - AbstractMenusApi.get().providers().placeholders().replace(player, json)); + AbstractMenusApi.get().providers().placeholders().resolve().replace(player, json)); if (component != null) { for (Player p : Bukkit.getOnlinePlayers()) @@ -88,7 +88,7 @@ public void activate(Player player, Menu menu, Item clickedItem) { } if (actionbar != null) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, actionbar); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, actionbar); ActionBar bar = ActionBar.create(); for (Player p : Bukkit.getOnlinePlayers()) @@ -97,10 +97,10 @@ public void activate(Player player, Menu menu, Item clickedItem) { if (!this.title.isEmpty() || !this.subtitle.isEmpty()) { String title = MiniMessageUtil.parseToLegacy( - AbstractMenusApi.get().providers().placeholders().replace(player, this.title) + AbstractMenusApi.get().providers().placeholders().resolve().replace(player, this.title) ); String subtitle = MiniMessageUtil.parseToLegacy( - AbstractMenusApi.get().providers().placeholders().replace(player, this.subtitle) + AbstractMenusApi.get().providers().placeholders().resolve().replace(player, this.subtitle) ); Title t = new Title( title, subtitle, diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBungeeConnect.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBungeeConnect.java index 290e919..a037b05 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBungeeConnect.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionBungeeConnect.java @@ -20,7 +20,7 @@ private ActionBungeeConnect(String serverName) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, serverName); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, serverName); BungeeManager.instance().sendPluginMessage(player, "Connect", replaced); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionCommand.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionCommand.java index 07c33c6..8d054ca 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionCommand.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionCommand.java @@ -43,7 +43,7 @@ public void activate(Player player, Menu menu, Item clickedItem) { for (String command : playerCommands) { if (command != null) { String resultCommand = isIgnorePlaceholder ? command - : AbstractMenusApi.get().providers().placeholders().replace(player, command); + : AbstractMenusApi.get().providers().placeholders().resolve().replace(player, command); player.performCommand(resultCommand); } } @@ -58,7 +58,7 @@ public void activate(Player player, Menu menu, Item clickedItem) { for (String command : consoleCommands) { if (command != null) { String resultCommand = isIgnorePlaceholder ? command - : AbstractMenusApi.get().providers().placeholders().replace(player, command); + : AbstractMenusApi.get().providers().placeholders().resolve().replace(player, command); Bukkit.dispatchCommand(Bukkit.getConsoleSender(), resultCommand); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionGroupAdd.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionGroupAdd.java index 464a29f..e54e974 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionGroupAdd.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionGroupAdd.java @@ -19,8 +19,8 @@ private ActionGroupAdd(String group) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - String group = AbstractMenusApi.get().providers().placeholders().replace(player, this.group); - AbstractMenusApi.get().providers().permissions().addGroup(player, group); + String group = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, this.group); + AbstractMenusApi.get().providers().permissions().resolve().addGroup(player, group); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionGroupRemove.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionGroupRemove.java index 2589caf..7b344b6 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionGroupRemove.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionGroupRemove.java @@ -19,8 +19,8 @@ private ActionGroupRemove(String group) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - String groupName = AbstractMenusApi.get().providers().placeholders().replace(player, group); - AbstractMenusApi.get().providers().permissions().removeGroup(player, groupName); + String groupName = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, group); + AbstractMenusApi.get().providers().permissions().resolve().removeGroup(player, groupName); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionInputChat.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionInputChat.java index ff4a5d3..440984b 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionInputChat.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionInputChat.java @@ -34,7 +34,7 @@ public ActionInputChat(String varName, boolean global, String cancelWord, @Override public void activate(Player player, Menu m, Item clickedItem) { - String name = AbstractMenusApi.get().providers().placeholders().replace(player, varName); + String name = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, varName); InputAction action = new InputAction(player, name, global, cancelWord, onInput, onCancel); MenuManager.instance().saveInputAction(action); m.close(player); diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLevelGive.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLevelGive.java index 1a66c68..d24e1f4 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLevelGive.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLevelGive.java @@ -20,7 +20,7 @@ private ActionLevelGive(TypeInt level) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - AbstractMenusApi.get().providers().levels().giveLevel(player, level.getInt(player, menu)); + AbstractMenusApi.get().providers().levels().resolve().giveLevel(player, level.getInt(player, menu)); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLevelTake.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLevelTake.java index 4cc2281..95e1273 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLevelTake.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLevelTake.java @@ -21,7 +21,7 @@ private ActionLevelTake(TypeInt level) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - AbstractMenusApi.get().providers().levels().takeLevel(player, level.getInt(player, menu)); + AbstractMenusApi.get().providers().levels().resolve().takeLevel(player, level.getInt(player, menu)); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLog.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLog.java index 7ad1bcb..75a6117 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLog.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLog.java @@ -20,7 +20,7 @@ private ActionLog(String message) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - Logger.info(AbstractMenusApi.get().providers().placeholders().replace(player, message)); + Logger.info(AbstractMenusApi.get().providers().placeholders().resolve().replace(player, message)); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaRemove.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaRemove.java index efa114d..575dfcd 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaRemove.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaRemove.java @@ -23,7 +23,7 @@ public class ActionLuckPermsMetaRemove implements Action { @Override public void activate(Player player, Menu menu, Item clickedItem) { - PermissionsHandler perms = AbstractMenusApi.get().providers().permissions(); + PermissionsHandler perms = AbstractMenusApi.get().providers().permissions().resolve(); if (!(perms instanceof LuckPermsHandler handler)) { Logger.warning("lpMetaRemove skipped: active permissions provider " + (perms == null ? "null" : perms.getClass().getSimpleName()) diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaSet.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaSet.java index ab3e9c8..c232116 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaSet.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionLuckPermsMetaSet.java @@ -25,7 +25,7 @@ public class ActionLuckPermsMetaSet implements Action { @Override public void activate(Player player, Menu menu, Item clickedItem) { - PermissionsHandler perms = AbstractMenusApi.get().providers().permissions(); + PermissionsHandler perms = AbstractMenusApi.get().providers().permissions().resolve(); if (!(perms instanceof LuckPermsHandler handler)) { Logger.warning("lpMetaSet skipped: active permissions provider " + (perms == null ? "null" : perms.getClass().getSimpleName()) @@ -35,7 +35,7 @@ public void activate(Player player, Menu menu, Item clickedItem) { } metaList.forEach(meta -> { String replacedValue = isIgnorePlaceholder ? meta.getValue() - : AbstractMenusApi.get().providers().placeholders().replace(player, meta.getValue()); + : AbstractMenusApi.get().providers().placeholders().resolve().replace(player, meta.getValue()); handler.addMeta(player, meta.getKey(), replacedValue); }); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuOpen.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuOpen.java index aa39bbd..30c6fd5 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuOpen.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuOpen.java @@ -21,7 +21,7 @@ private ActionMenuOpen(String menu) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - String name = AbstractMenusApi.get().providers().placeholders().replace(player, this.menu); + String name = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, this.menu); Menu menuToOpen = MenuManager.instance().getMenu(name); if (menuToOpen != null) diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuOpenCtx.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuOpenCtx.java index f332513..047b008 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuOpenCtx.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuOpenCtx.java @@ -22,7 +22,7 @@ private ActionMenuOpenCtx(String menu) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - String name = AbstractMenusApi.get().providers().placeholders().replace(player, this.menu); + String name = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, this.menu); Menu menuToOpen = MenuManager.instance().getMenu(name); if (menuToOpen != null) { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMessage.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMessage.java index dddf207..a04bf72 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMessage.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMessage.java @@ -69,29 +69,29 @@ private void setStay(TypeInt stay) { public void activate(Player player, Menu menu, Item clickedItem) { if (player != null) { if (chatMessages != null) { - List replaced = AbstractMenusApi.get().providers().placeholders().replace(player, chatMessages); + List replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, chatMessages); MiniMessageUtil.sendParsed(replaced, player); } if (json != null) { BaseComponent[] component = ComponentSerializer.parse( - AbstractMenusApi.get().providers().placeholders().replace(player, json)); + AbstractMenusApi.get().providers().placeholders().resolve().replace(player, json)); if (component != null) player.spigot().sendMessage(component); } if (actionbar != null) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, actionbar); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, actionbar); ActionBar.create().send(player, MiniMessageUtil.parseToLegacy(replaced)); } if (!this.title.isEmpty() || !this.subtitle.isEmpty()) { String title = MiniMessageUtil.parseToLegacy( - AbstractMenusApi.get().providers().placeholders().replace(player, this.title) + AbstractMenusApi.get().providers().placeholders().resolve().replace(player, this.title) ); String subtitle = MiniMessageUtil.parseToLegacy( - AbstractMenusApi.get().providers().placeholders().replace(player, this.subtitle) + AbstractMenusApi.get().providers().placeholders().resolve().replace(player, this.subtitle) ); new Title( diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMiniMessage.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMiniMessage.java index 5cfbc30..1a10b4e 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMiniMessage.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMiniMessage.java @@ -22,7 +22,7 @@ private ActionMiniMessage(String message) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, message); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, message); MiniMessageUtil.sendParsed(Collections.singletonList(replaced), player); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyGive.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyGive.java index 6657241..6cd9887 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyGive.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyGive.java @@ -25,8 +25,8 @@ private ActionMoneyGive(TypeDouble money, String provider) { @Override public void activate(Player player, Menu menu, Item clickedItem) { EconomyHandler eco = provider != null - ? AbstractMenusApi.get().providers().economy(provider) - : AbstractMenusApi.get().providers().economy(); + ? AbstractMenusApi.get().providers().economy().resolve(provider) + : AbstractMenusApi.get().providers().economy().resolve(); if (eco == null) { return; } @@ -48,9 +48,9 @@ public ActionMoneyGive deserialize(Class type, ConfigNode node) throws NodeSeria } if (provider != null) { - if (!AbstractMenusApi.get().providers().hasEconomy(provider)) { + if (!AbstractMenusApi.get().providers().economy().has(provider)) { StringBuilder known = new StringBuilder(); - for (EconomyHandler h : AbstractMenusApi.get().providers().allEconomy()) { + for (EconomyHandler h : AbstractMenusApi.get().providers().economy().all()) { if (known.length() > 0) known.append(", "); known.append(h.getClass().getSimpleName()); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyTake.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyTake.java index 757c2fe..4349d37 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyTake.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyTake.java @@ -25,8 +25,8 @@ private ActionMoneyTake(TypeDouble money, String provider) { @Override public void activate(Player player, Menu menu, Item clickedItem) { EconomyHandler eco = provider != null - ? AbstractMenusApi.get().providers().economy(provider) - : AbstractMenusApi.get().providers().economy(); + ? AbstractMenusApi.get().providers().economy().resolve(provider) + : AbstractMenusApi.get().providers().economy().resolve(); if (eco == null) { return; } @@ -48,9 +48,9 @@ public ActionMoneyTake deserialize(Class type, ConfigNode node) throws NodeSeria } if (provider != null) { - if (!AbstractMenusApi.get().providers().hasEconomy(provider)) { + if (!AbstractMenusApi.get().providers().economy().has(provider)) { StringBuilder known = new StringBuilder(); - for (EconomyHandler h : AbstractMenusApi.get().providers().allEconomy()) { + for (EconomyHandler h : AbstractMenusApi.get().providers().economy().all()) { if (known.length() > 0) known.append(", "); known.append(h.getClass().getSimpleName()); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPermissionGive.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPermissionGive.java index f2e5667..bd3a675 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPermissionGive.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPermissionGive.java @@ -27,8 +27,8 @@ public void setIgnorePlaceholder(boolean ignorePlaceholder) { @Override public void activate(Player player, Menu menu, Item clickedItem) { permissions.forEach(perm -> { - String replaced = isIgnorePlaceholder ? perm : AbstractMenusApi.get().providers().placeholders().replace(player, perm); - AbstractMenusApi.get().providers().permissions().addPermission(player, replaced); + String replaced = isIgnorePlaceholder ? perm : AbstractMenusApi.get().providers().placeholders().resolve().replace(player, perm); + AbstractMenusApi.get().providers().permissions().resolve().addPermission(player, replaced); }); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPermissionRemove.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPermissionRemove.java index 4afe92b..c884603 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPermissionRemove.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionPermissionRemove.java @@ -23,8 +23,8 @@ private ActionPermissionRemove(List permissions) { @Override public void activate(Player player, Menu menu, Item clickedItem) { permissions.forEach(perm -> { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, perm); - AbstractMenusApi.get().providers().permissions().removePermission(player, replaced); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, perm); + AbstractMenusApi.get().providers().permissions().resolve().removePermission(player, replaced); }); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionSkinReset.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionSkinReset.java index ae71ab0..b3d28a2 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionSkinReset.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionSkinReset.java @@ -21,7 +21,7 @@ private ActionSkinReset(TypeBool reset) { public void activate(Player player, Menu menu, Item clickedItem) { if (reset.getBool(player, menu)) { - AbstractMenusApi.get().providers().skins().resetSkin(player); + AbstractMenusApi.get().providers().skins().resolve().resetSkin(player); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionSkinSet.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionSkinSet.java index ef7631e..a221b2e 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionSkinSet.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionSkinSet.java @@ -21,9 +21,9 @@ private ActionSkinSet(String texture, String signature) { } public void activate(Player player, Menu menu, Item clickedItem) { - AbstractMenusApi.get().providers().skins().setSkin(player, - AbstractMenusApi.get().providers().placeholders().replace(player, texture), - AbstractMenusApi.get().providers().placeholders().replace(player, signature)); + AbstractMenusApi.get().providers().skins().resolve().setSkin(player, + AbstractMenusApi.get().providers().placeholders().resolve().replace(player, texture), + AbstractMenusApi.get().providers().placeholders().resolve().replace(player, signature)); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionXpGive.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionXpGive.java index 7641dc6..c873612 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionXpGive.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionXpGive.java @@ -21,7 +21,7 @@ private ActionXpGive(TypeInt xp) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - AbstractMenusApi.get().providers().levels().giveXp(player, xp.getInt(player, menu)); + AbstractMenusApi.get().providers().levels().resolve().giveXp(player, xp.getInt(player, menu)); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionXpTake.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionXpTake.java index a3a9a10..099ed59 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionXpTake.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionXpTake.java @@ -20,7 +20,7 @@ private ActionXpTake(TypeInt xp) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - AbstractMenusApi.get().providers().levels().takeXp(player, xp.getInt(player, menu)); + AbstractMenusApi.get().providers().levels().resolve().takeXp(player, xp.getInt(player, menu)); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarDec.java b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarDec.java index 8fb3990..eb0b56f 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarDec.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarDec.java @@ -24,14 +24,14 @@ private ActionVarDec(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarNumData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); double value = data.getValue().getDouble(p, menu); Function func = num -> num - value; if (data.getPlayer() == null) { VariableManagerImpl.instance().modifyNumericGlobal(varName, func); } else { - String playerName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getPlayer()); + String playerName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getPlayer()); VariableManagerImpl.instance().modifyNumericPersonal(playerName, varName, func); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarDiv.java b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarDiv.java index c4131f5..fcefe9c 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarDiv.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarDiv.java @@ -25,7 +25,7 @@ private ActionVarDiv(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarNumData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); double value = data.getValue().getDouble(p, menu); if (value == 0) { @@ -38,7 +38,7 @@ public void activate(Player p, Menu menu, Item clickedItem) { if (data.getPlayer() == null) { VariableManagerImpl.instance().modifyNumericGlobal(varName, func); } else { - String playerName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getPlayer()); + String playerName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getPlayer()); VariableManagerImpl.instance().modifyNumericPersonal(playerName, varName, func); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarInc.java b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarInc.java index 363a635..75ed52d 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarInc.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarInc.java @@ -24,14 +24,14 @@ private ActionVarInc(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarNumData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); double value = data.getValue().getDouble(p, menu); Function func = num -> num + value; if (data.getPlayer() == null) { VariableManagerImpl.instance().modifyNumericGlobal(varName, func); } else { - String playerName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getPlayer()); + String playerName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getPlayer()); VariableManagerImpl.instance().modifyNumericPersonal(playerName, varName, func); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarMul.java b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarMul.java index 44f1c0b..62f9a23 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarMul.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarMul.java @@ -24,14 +24,14 @@ private ActionVarMul(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarNumData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); double value = data.getValue().getDouble(p, menu); Function func = num -> num * value; if (data.getPlayer() == null) { VariableManagerImpl.instance().modifyNumericGlobal(varName, func); } else { - String playerName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getPlayer()); + String playerName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getPlayer()); VariableManagerImpl.instance().modifyNumericPersonal(playerName, varName, func); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarRem.java b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarRem.java index d881d35..df23c48 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarRem.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarRem.java @@ -23,12 +23,12 @@ private ActionVarRem(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); if (data.getPlayer() == null) { VariableManagerImpl.instance().deleteGlobal(varName); } else { - String playerName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getPlayer()); + String playerName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getPlayer()); VariableManagerImpl.instance().deletePersonal(playerName, varName); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarSet.java b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarSet.java index 6d61a24..341ad98 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarSet.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/var/ActionVarSet.java @@ -25,10 +25,10 @@ private ActionVarSet(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); - String varVal = AbstractMenusApi.get().providers().placeholders().replace(p, data.getValue()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); + String varVal = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getValue()); - long time = TimeUtil.parseTime(AbstractMenusApi.get().providers().placeholders().replace(p, data.getTime())); + long time = TimeUtil.parseTime(AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getTime())); boolean replace = data.isReplace().getBool(p, menu); Var var = VariableManagerImpl.instance().createBuilder() @@ -40,7 +40,7 @@ public void activate(Player p, Menu menu, Item clickedItem) { if (data.getPlayer() == null) { VariableManagerImpl.instance().saveGlobal(var, replace); } else { - String playerName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getPlayer()); + String playerName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getPlayer()); VariableManagerImpl.instance().savePersonal(playerName, var, replace); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpDec.java b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpDec.java index 8e52c7d..4dd0a9d 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpDec.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpDec.java @@ -24,7 +24,7 @@ private ActionVarpDec(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarNumData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); double value = data.getValue().getDouble(p, menu); Function func = num -> num - value; diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpDiv.java b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpDiv.java index 869cf3a..d9a02f1 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpDiv.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpDiv.java @@ -25,7 +25,7 @@ private ActionVarpDiv(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarNumData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); double value = data.getValue().getDouble(p, menu); if (value == 0) { diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpInc.java b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpInc.java index dbeb048..eb7934c 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpInc.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpInc.java @@ -24,7 +24,7 @@ private ActionVarpInc(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarNumData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); double value = data.getValue().getDouble(p, menu); Function func = num -> num + value; diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpMul.java b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpMul.java index 2ca0fbf..1f47a03 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpMul.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpMul.java @@ -24,7 +24,7 @@ private ActionVarpMul(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarNumData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); double value = data.getValue().getDouble(p, menu); Function func = num -> num * value; diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpRem.java b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpRem.java index 5256634..73cf5bd 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpRem.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpRem.java @@ -23,7 +23,7 @@ private ActionVarpRem(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); VariableManagerImpl.instance().deletePersonal(p.getName(), varName); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpSet.java b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpSet.java index 1bb5fc2..d2e0fe1 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpSet.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/varp/ActionVarpSet.java @@ -25,10 +25,10 @@ private ActionVarpSet(List dataList) { public void activate(Player p, Menu menu, Item clickedItem) { for (VarData data : dataList) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, data.getName()); - String varVal = AbstractMenusApi.get().providers().placeholders().replace(p, data.getValue()); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getName()); + String varVal = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getValue()); - long time = TimeUtil.parseTime(AbstractMenusApi.get().providers().placeholders().replace(p, data.getTime())); + long time = TimeUtil.parseTime(AbstractMenusApi.get().providers().placeholders().resolve().replace(p, data.getTime())); boolean replace = data.isReplace().getBool(p, menu); Var var = VariableManagerImpl.instance().createBuilder() diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/wrappers/ActionPlayerScope.java b/plugin/src/main/java/ru/abstractmenus/data/actions/wrappers/ActionPlayerScope.java index d63fd85..1de374e 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/wrappers/ActionPlayerScope.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/wrappers/ActionPlayerScope.java @@ -23,7 +23,7 @@ public ActionPlayerScope(String playerName, Actions actions) { @Override public void activate(Player player, Menu menu, Item clickedItem) { - String replacedName = AbstractMenusApi.get().providers().placeholders().replace(player, playerName); + String replacedName = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, playerName); Player target = Bukkit.getPlayerExact(replacedName); if (target != null && target.isOnline()) { diff --git a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenChat.java b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenChat.java index 31f0102..15ee88a 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenChat.java +++ b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenChat.java @@ -23,7 +23,7 @@ private OpenChat(List messages) { @EventHandler public void onChat(AsyncChatEvent event) { for (String str : messages) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(event.getPlayer(), str); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(event.getPlayer(), str); if (event.signedMessage().message().equalsIgnoreCase(replaced)) { BukkitTasks.runTask(() -> openMenu(null, event.getPlayer())); diff --git a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenChatContains.java b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenChatContains.java index 24b5ed2..d5944a6 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenChatContains.java +++ b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenChatContains.java @@ -24,7 +24,7 @@ private OpenChatContains(List messages) { @EventHandler public void onChat(AsyncChatEvent event) { for (String str : messages) { - String msg = AbstractMenusApi.get().providers().placeholders().replace(event.getPlayer(), str); + String msg = AbstractMenusApi.get().providers().placeholders().resolve().replace(event.getPlayer(), str); if (event.signedMessage().message().contains(msg)) { BukkitTasks.runTask(() -> openMenu(null, event.getPlayer())); diff --git a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenClickEntity.java b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenClickEntity.java index 0aad579..016514b 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenClickEntity.java +++ b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenClickEntity.java @@ -38,7 +38,7 @@ public void onEntityClick(PlayerInteractEntityEvent event) { } if (data.getName() != null) { - String expectedName = AbstractMenusApi.get().providers().placeholders().replace(player, data.getName()); + String expectedName = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, data.getName()); if (!clickedEntity.getName().equalsIgnoreCase(expectedName)) { continue; } diff --git a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenRegionEnter.java b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenRegionEnter.java index 34c10d2..5407a93 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenRegionEnter.java +++ b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenRegionEnter.java @@ -22,7 +22,7 @@ private OpenRegionEnter(List regions) { @EventHandler public void onRegionJoin(RegionEnterEvent event) { - List regions = AbstractMenusApi.get().providers().placeholders().replace(event.getPlayer(), this.regions); + List regions = AbstractMenusApi.get().providers().placeholders().resolve().replace(event.getPlayer(), this.regions); if (regions.contains(event.getRegion().getId())) { openMenu(event.getRegion(), event.getPlayer()); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenRegionLeave.java b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenRegionLeave.java index e895719..74cc7d0 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenRegionLeave.java +++ b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenRegionLeave.java @@ -22,7 +22,7 @@ private OpenRegionLeave(List regions) { @EventHandler public void onRegionEnter(RegionLeaveEvent event) { - List regions = AbstractMenusApi.get().providers().placeholders().replace(event.getPlayer(), this.regions); + List regions = AbstractMenusApi.get().providers().placeholders().resolve().replace(event.getPlayer(), this.regions); if (regions.contains(event.getRegion().getId())) { openMenu(event.getRegion(), event.getPlayer()); diff --git a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenShiftClickEntity.java b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenShiftClickEntity.java index 092b4e0..ce8d688 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenShiftClickEntity.java +++ b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenShiftClickEntity.java @@ -39,7 +39,7 @@ public void onEntityClick(PlayerInteractEntityEvent event) { } if (data.getName() == null) { - String name = AbstractMenusApi.get().providers().placeholders().replace(player, data.getName()); + String name = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, data.getName()); if (clickedEntity.getName().equalsIgnoreCase(name)) { openMenu(clickedEntity, player); return; diff --git a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenSign.java b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenSign.java index 6977a20..cda4ff6 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/activators/OpenSign.java +++ b/plugin/src/main/java/ru/abstractmenus/data/activators/OpenSign.java @@ -43,7 +43,7 @@ public void onTableClick(PlayerInteractEvent event) { String[] lines = sign.getLines(); int compareLines = Math.min(text.size(), lines.length); for (int i = 0; i < compareLines; i++) { - String line = AbstractMenusApi.get().providers().placeholders().replace(player, text.get(i)); + String line = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, text.get(i)); if (!line.equalsIgnoreCase(lines[i])) return; } diff --git a/plugin/src/main/java/ru/abstractmenus/data/catalogs/SliceCatalog.java b/plugin/src/main/java/ru/abstractmenus/data/catalogs/SliceCatalog.java index 0720d63..c12151d 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/catalogs/SliceCatalog.java +++ b/plugin/src/main/java/ru/abstractmenus/data/catalogs/SliceCatalog.java @@ -36,7 +36,7 @@ public SliceCatalog(String value, String separator, boolean trim) { @Override public Collection snapshot(Player player, Menu menu) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, value); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, value); String[] values = replaced.split(separator); return Arrays.stream(values) .filter(val -> !val.isEmpty()) diff --git a/plugin/src/main/java/ru/abstractmenus/data/comparator/LegacyValueComparator.java b/plugin/src/main/java/ru/abstractmenus/data/comparator/LegacyValueComparator.java index 74eec4d..c569b52 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/comparator/LegacyValueComparator.java +++ b/plugin/src/main/java/ru/abstractmenus/data/comparator/LegacyValueComparator.java @@ -49,11 +49,11 @@ public Comparator(String param) { } public boolean compare(Player player, Menu menu){ - String param = AbstractMenusApi.get().providers().placeholders().replace(player, getParam()); + String param = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, getParam()); if(equals != null){ for(String str : equals){ - String val = AbstractMenusApi.get().providers().placeholders().replace(player, str); + String val = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, str); try{ if(Double.parseDouble(param) == Double.parseDouble(val)) return true; @@ -65,7 +65,7 @@ public boolean compare(Player player, Menu menu){ if(equalsIgnoreCase != null){ for(String str : equalsIgnoreCase){ - String val = AbstractMenusApi.get().providers().placeholders().replace(player, str); + String val = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, str); if(param.equalsIgnoreCase(val)){ return true; } @@ -74,7 +74,7 @@ public boolean compare(Player player, Menu menu){ if(contains != null){ for(String str : contains){ - String val = AbstractMenusApi.get().providers().placeholders().replace(player, str); + String val = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, str); if(param.toLowerCase().contains(val.toLowerCase())){ return true; } diff --git a/plugin/src/main/java/ru/abstractmenus/data/comparator/ModernValueComparator.java b/plugin/src/main/java/ru/abstractmenus/data/comparator/ModernValueComparator.java index 65e472d..5f335c1 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/comparator/ModernValueComparator.java +++ b/plugin/src/main/java/ru/abstractmenus/data/comparator/ModernValueComparator.java @@ -18,7 +18,7 @@ private ModernValueComparator(String expression) { @Override public boolean compare(Player player, Menu menu) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, expression); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, expression); String result = EVALUATOR.evaluate(replaced); return Boolean.parseBoolean(result); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropBookData.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropBookData.java index f2461d4..de62e10 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropBookData.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropBookData.java @@ -35,9 +35,9 @@ public boolean isApplyMeta() { @Override public void apply(ItemStack itemStack, ItemMeta meta, Player player, Menu menu) { if (meta instanceof BookMeta) { - String author = AbstractMenusApi.get().providers().placeholders().replace(player, data.getAuthor()); - String title = AbstractMenusApi.get().providers().placeholders().replace(player, data.getTitle()); - List pages = AbstractMenusApi.get().providers().placeholders().replace(player, data.getPages()); + String author = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, data.getAuthor()); + String title = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, data.getTitle()); + List pages = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, data.getPages()); ((BookMeta) meta).setAuthor(author); ((BookMeta) meta).setTitle(title); diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropEquipItem.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropEquipItem.java index 44c4d31..4bf0237 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropEquipItem.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropEquipItem.java @@ -60,7 +60,7 @@ public void apply(ItemStack item, ItemMeta meta, Player player, Menu menu) { Player target = player; if (playerName != null) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, playerName); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, playerName); Player found = Bukkit.getPlayerExact(replaced); if (found == null || !found.isOnline()) { diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropHDB.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropHDB.java index 9e7f37c..8066990 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropHDB.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropHDB.java @@ -48,7 +48,7 @@ public boolean isApplyMeta() { @Override public void apply(ItemStack item, ItemMeta meta, Player player, Menu menu) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, id); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, id); ItemStack headItem = api().getItemHead(replaced); if (headItem == null) diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropItemsAdder.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropItemsAdder.java index 7b2ae92..89466c9 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropItemsAdder.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropItemsAdder.java @@ -33,7 +33,7 @@ public boolean isApplyMeta() { @Override public void apply(ItemStack item, ItemMeta meta, Player player, Menu menu) { - String id = AbstractMenusApi.get().providers().placeholders().replace(player, this.id); + String id = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, this.id); CustomStack stack = CustomStack.getInstance(id); if (stack == null) diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropLore.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropLore.java index c21760f..fb8912b 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropLore.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropLore.java @@ -68,7 +68,7 @@ public void apply(ItemStack itemStack, ItemMeta meta, Player player, Menu menu) meta.setLore(preFormatted); return; } - List replaced = AbstractMenusApi.get().providers().placeholders().replace(player, lore); + List replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, lore); meta.setLore(MiniMessageUtil.parseToLegacy(replaced)); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropLoreLight.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropLoreLight.java index 663f650..b1d5933 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropLoreLight.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropLoreLight.java @@ -33,7 +33,7 @@ public boolean isApplyMeta() { @Override public void apply(ItemStack itemStack, ItemMeta meta, Player player, Menu menu) { - List replaced = AbstractMenusApi.get().providers().placeholders().replace(player, lore); + List replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, lore); meta.setLore(replaced); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropMmoItem.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropMmoItem.java index cc54614..21010fa 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropMmoItem.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropMmoItem.java @@ -36,7 +36,7 @@ public boolean isApplyMeta() { @Override public void apply(ItemStack item, ItemMeta meta, Player player, Menu menu) { - String[] arr = AbstractMenusApi.get().providers().placeholders().replace(player, id).split(":"); + String[] arr = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, id).split(":"); if(arr.length >= 2) { ItemStack mmoItem = MMOItems.plugin.getItem(MMOItems.plugin.getTypes().get(arr[0]), arr[1]); diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropName.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropName.java index bda8de0..dd97b9f 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropName.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropName.java @@ -58,7 +58,7 @@ public void apply(ItemStack itemStack, ItemMeta meta, Player player, Menu menu) meta.setDisplayName(preFormatted); return; } - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, name); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, name); meta.setDisplayName(MiniMessageUtil.parseToLegacy(replaced)); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropNameLight.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropNameLight.java index 85100f6..b56dbdd 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropNameLight.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropNameLight.java @@ -31,7 +31,7 @@ public boolean isApplyMeta() { @Override public void apply(ItemStack itemStack, ItemMeta meta, Player player, Menu menu) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, name); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, name); meta.setDisplayName(replaced); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropOraxen.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropOraxen.java index 8ef134b..aae5a55 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropOraxen.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropOraxen.java @@ -62,7 +62,7 @@ public void apply(ItemStack item, ItemMeta meta, Player player, Menu menu) { } private ItemStack getItem(Player player) { - String id = AbstractMenusApi.get().providers().placeholders().replace(player, this.id); + String id = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, this.id); try { Object builder = getItemByIdMethod.invoke(id); if (builder == null) diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropSerialized.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropSerialized.java index 77e4ca3..40323f1 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropSerialized.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropSerialized.java @@ -33,7 +33,7 @@ public boolean isApplyMeta() { @Override public void apply(ItemStack item, ItemMeta meta, Player player, Menu menu) { - String base64 = AbstractMenusApi.get().providers().placeholders().replace(player, value); + String base64 = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, value); ItemStack deserialized = ItemUtil.decodeStack(base64); if (deserialized == null) { diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropSkullOwner.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropSkullOwner.java index 5ba3521..8d1f8e7 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropSkullOwner.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropSkullOwner.java @@ -34,7 +34,7 @@ public boolean isApplyMeta() { @Override public void apply(ItemStack item, ItemMeta meta, Player player, Menu menu) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, owner); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, owner); ItemStack skullItem = Skulls.getPlayerSkull(replaced); if (skullItem == null) { diff --git a/plugin/src/main/java/ru/abstractmenus/data/properties/PropTexture.java b/plugin/src/main/java/ru/abstractmenus/data/properties/PropTexture.java index 7044b8a..2733e1d 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/properties/PropTexture.java +++ b/plugin/src/main/java/ru/abstractmenus/data/properties/PropTexture.java @@ -43,7 +43,7 @@ public boolean isApplyMeta() { @Override public void apply(ItemStack item, ItemMeta meta, Player player, Menu menu) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, texture); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, texture); ItemStack skullItem = Skulls.getCustomSkull(fetchTextureUrl(replaced)); if (skullItem != null) { diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleBungeeIsOnline.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleBungeeIsOnline.java index 469fdae..ac2e2ff 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleBungeeIsOnline.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleBungeeIsOnline.java @@ -20,7 +20,7 @@ private RuleBungeeIsOnline(String server){ @Override public boolean check(Player player, Menu menu, Item clickedItem) { - return BungeeManager.instance().isOnline(AbstractMenusApi.get().providers().placeholders().replace(player, server)); + return BungeeManager.instance().isOnline(AbstractMenusApi.get().providers().placeholders().resolve().replace(player, server)); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleBungeeOnline.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleBungeeOnline.java index 60be6fd..2c2378e 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleBungeeOnline.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleBungeeOnline.java @@ -24,7 +24,7 @@ private RuleBungeeOnline(String server, TypeInt online){ @Override public boolean check(Player player, Menu menu, Item clickedItem) { - return BungeeManager.instance().getOnline(AbstractMenusApi.get().providers().placeholders().replace(player, server)) >= online.getInt(player, menu); + return BungeeManager.instance().getOnline(AbstractMenusApi.get().providers().placeholders().resolve().replace(player, server)) >= online.getInt(player, menu); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleExistVar.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleExistVar.java index d368571..1cf5448 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleExistVar.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleExistVar.java @@ -22,12 +22,12 @@ private RuleExistVar(String player, String name){ @Override public boolean check(Player p, Menu menu, Item clickedItem) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, this.name); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, this.name); if(this.player == null) { return VariableManagerImpl.instance().getGlobal(varName) != null; } else { - String varPlayer = AbstractMenusApi.get().providers().placeholders().replace(p, this.player); + String varPlayer = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, this.player); return VariableManagerImpl.instance().getPersonal(varPlayer, varName) != null; } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleExistVarp.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleExistVarp.java index dc44f77..fed1f42 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleExistVarp.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleExistVarp.java @@ -20,7 +20,7 @@ private RuleExistVarp(String name){ @Override public boolean check(Player p, Menu menu, Item clickedItem) { - String varName = AbstractMenusApi.get().providers().placeholders().replace(p, this.name); + String varName = AbstractMenusApi.get().providers().placeholders().resolve().replace(p, this.name); return VariableManagerImpl.instance().getPersonal(p.getName(), varName) != null; } diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleGroup.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleGroup.java index 95de37f..5f3c14a 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleGroup.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleGroup.java @@ -22,8 +22,8 @@ private RuleGroup(List groups) { @Override public boolean check(Player player, Menu menu, Item clickedItem) { for (String group : groups) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, group); - if (!AbstractMenusApi.get().providers().permissions().hasGroup(player, replaced)) return false; + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, group); + if (!AbstractMenusApi.get().providers().permissions().resolve().hasGroup(player, replaced)) return false; } return true; } diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleJS.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleJS.java index f9d41de..823c088 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleJS.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleJS.java @@ -41,7 +41,7 @@ private RuleJS(String js){ @Override public boolean check(Player player, Menu menu, Item clickedItem) { try{ - Object result = ENGINE.eval(AbstractMenusApi.get().providers().placeholders().replace(player, js), bindings); + Object result = ENGINE.eval(AbstractMenusApi.get().providers().placeholders().resolve().replace(player, js), bindings); return result.toString().equals("true"); } catch (ScriptException e){ Logger.severe("Cannot execute JavaScript code: " + e.getMessage()); diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleLevel.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleLevel.java index 86dcd06..32424aa 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleLevel.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleLevel.java @@ -20,7 +20,7 @@ private RuleLevel(TypeInt level){ @Override public boolean check(Player player, Menu menu, Item clickedItem) { - return AbstractMenusApi.get().providers().levels().getLevel(player) >= level.getInt(player, menu); + return AbstractMenusApi.get().providers().levels().resolve().getLevel(player) >= level.getInt(player, menu); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleMoney.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleMoney.java index 01ec2b5..03c4ff4 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleMoney.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleMoney.java @@ -24,8 +24,8 @@ private RuleMoney(TypeDouble money, String provider){ @Override public boolean check(Player player, Menu menu, Item clickedItem) { EconomyHandler eco = provider != null - ? AbstractMenusApi.get().providers().economy(provider) - : AbstractMenusApi.get().providers().economy(); + ? AbstractMenusApi.get().providers().economy().resolve(provider) + : AbstractMenusApi.get().providers().economy().resolve(); if (eco == null) { return false; } @@ -47,9 +47,9 @@ public RuleMoney deserialize(Class type, ConfigNode node) throws NodeSerializeEx } if (provider != null) { - if (!AbstractMenusApi.get().providers().hasEconomy(provider)) { + if (!AbstractMenusApi.get().providers().economy().has(provider)) { StringBuilder known = new StringBuilder(); - for (EconomyHandler h : AbstractMenusApi.get().providers().allEconomy()) { + for (EconomyHandler h : AbstractMenusApi.get().providers().economy().all()) { if (known.length() > 0) known.append(", "); known.append(h.getClass().getSimpleName()); } diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RulePermission.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RulePermission.java index b843268..f462c2e 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RulePermission.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RulePermission.java @@ -22,8 +22,8 @@ private RulePermission(List permission){ @Override public boolean check(Player player, Menu menu, Item clickedItem) { for (String perm : permission){ - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, perm); - if(!AbstractMenusApi.get().providers().permissions().hasPermission(player, replaced)) return false; + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, perm); + if(!AbstractMenusApi.get().providers().permissions().resolve().hasPermission(player, replaced)) return false; } return true; } diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RulePlayerIsOnline.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RulePlayerIsOnline.java index 939ce1c..3e79b6f 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RulePlayerIsOnline.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RulePlayerIsOnline.java @@ -20,7 +20,7 @@ private RulePlayerIsOnline(String playerName){ @Override public boolean check(Player player, Menu menu, Item clickedItem) { - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, playerName); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, playerName); Player foundPlayer = Bukkit.getPlayerExact(replaced); return foundPlayer != null && foundPlayer.isOnline(); diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleRegion.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleRegion.java index 5c47cd7..8394da7 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleRegion.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleRegion.java @@ -24,7 +24,7 @@ private RuleRegion(List regions){ @Override public boolean check(Player player, Menu menu, Item clickedItem) { for (String reg : regions){ - String replaced = AbstractMenusApi.get().providers().placeholders().replace(player, reg); + String replaced = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, reg); ProtectedRegion region = RegionUtils.getRegion(player.getWorld(), replaced); if(region != null && region.contains( diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleXp.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleXp.java index f9784d0..43e40d3 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleXp.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleXp.java @@ -20,7 +20,7 @@ private RuleXp(TypeFloat xp) { @Override public boolean check(Player player, Menu menu, Item clickedItem) { - return AbstractMenusApi.get().providers().levels().getXp(player) >= xp.getFloat(player, menu); + return AbstractMenusApi.get().providers().levels().resolve().getXp(player) >= xp.getFloat(player, menu); } public static class Serializer implements NodeSerializer { diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/logical/RulePlayerScope.java b/plugin/src/main/java/ru/abstractmenus/data/rules/logical/RulePlayerScope.java index 7578af7..a25ac34 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/logical/RulePlayerScope.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/logical/RulePlayerScope.java @@ -22,7 +22,7 @@ public RulePlayerScope(String playerName, Rule rules) { @Override public boolean check(Player player, Menu menu, Item clickedItem) { - String replacedName = AbstractMenusApi.get().providers().placeholders().replace(player, playerName); + String replacedName = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, playerName); Player target = Bukkit.getPlayerExact(replacedName); if (target != null && target.isOnline()) { diff --git a/plugin/src/main/java/ru/abstractmenus/datatype/DataType.java b/plugin/src/main/java/ru/abstractmenus/datatype/DataType.java index 3fd668d..afe9b41 100644 --- a/plugin/src/main/java/ru/abstractmenus/datatype/DataType.java +++ b/plugin/src/main/java/ru/abstractmenus/datatype/DataType.java @@ -15,7 +15,7 @@ public abstract class DataType implements Cloneable { } public String replaceFor(Player player, Menu menu) { - return AbstractMenusApi.get().providers().placeholders().replace(player, value); + return AbstractMenusApi.get().providers().placeholders().resolve().replace(player, value); } public static boolean hasPlaceholder(String string) { diff --git a/plugin/src/main/java/ru/abstractmenus/extractors/PlayerExtractor.java b/plugin/src/main/java/ru/abstractmenus/extractors/PlayerExtractor.java index 4f21728..ed08d50 100644 --- a/plugin/src/main/java/ru/abstractmenus/extractors/PlayerExtractor.java +++ b/plugin/src/main/java/ru/abstractmenus/extractors/PlayerExtractor.java @@ -12,7 +12,7 @@ public class PlayerExtractor implements ValueExtractor { @Override public String extract(Object obj, String placeholder) { return (obj instanceof Player player && player.isOnline()) - ? AbstractMenusApi.get().providers().placeholders().replacePlaceholder(player, placeholder) + ? AbstractMenusApi.get().providers().placeholders().resolve().replacePlaceholder(player, placeholder) : StringUtils.EMPTY; } } diff --git a/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java index f49ca9d..f844de6 100644 --- a/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java +++ b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java @@ -2,6 +2,7 @@ import ru.abstractmenus.api.MenuExtension; import ru.abstractmenus.api.ProviderRegistry; +import ru.abstractmenus.api.ProviderSection; import ru.abstractmenus.api.handler.EconomyHandler; import ru.abstractmenus.api.handler.LevelHandler; import ru.abstractmenus.api.handler.PermissionsHandler; @@ -20,81 +21,39 @@ import java.util.function.Function; /** - * Default {@link ProviderRegistry} implementation. Five sections sharing an - * inner generic {@link Section} class. Insertion-ordered per section so that - * equal-priority ties resolve to the first-registered entry. + * Default {@link ProviderRegistry} implementation. Five sections backed by + * the inner {@link SectionImpl} class. Insertion-ordered per section so + * that equal-priority ties resolve to the first-registered entry. * - *

Thread-safe for registration/unregistration via per-section synchronized - * methods, but production use expects all mutation to happen on the main - * server thread during plugin / extension enable/disable. + *

Sections are constructed once in this class's constructor with their + * "kind" string ({@code "economy"}, {@code "permissions"}, ...) so each + * one knows which {@code config.conf providers.} entry applies. + * + *

Thread-safe for registration/unregistration via per-section + * {@code synchronized} methods; production use expects all mutation on the + * main server thread during plugin / extension enable / disable. */ public final class ProviderRegistryImpl implements ProviderRegistry { - private final Section economy = new Section<>(); - private final Section permissions = new Section<>(); - private final Section levels = new Section<>(); - private final Section placeholders = new Section<>(); - private final Section skins = new Section<>(); - /** section kind → configured-default id (e.g. "economy" → "playerpoints"). */ - private Function configDefaults = kind -> null; // no-op by default + private Function configDefaults = kind -> null; + + private final SectionImpl economy = new SectionImpl<>("economy", this); + private final SectionImpl permissions = new SectionImpl<>("permissions", this); + private final SectionImpl levels = new SectionImpl<>("levels", this); + private final SectionImpl placeholders = new SectionImpl<>("placeholders", this); + private final SectionImpl skins = new SectionImpl<>("skins", this); /** Wire up the config-backed default source. Called once from AbstractMenusApiImpl. */ public void setConfigDefaults(Function lookup) { this.configDefaults = lookup; } - // ---- Config-default resolution helper -------------------------------- - - private T resolveWithConfig(String kind, Section section) { - // configDefaults is read here outside the section lock; that's - // intentional. The Function is only mutated once via setConfigDefaults - // during plugin startup, before any concurrent reads can happen. - String configured = configDefaults.apply(kind); - return section.resolveWithDefault(configured); - } - - // ---- Economy --------------------------------------------------------- - - @Override public void registerEconomy(String id, EconomyHandler h, int pr, MenuExtension o) { economy.put(id, h, pr, o); } - @Override public EconomyHandler economy() { return resolveWithConfig("economy", economy); } - @Override public EconomyHandler economy(String id) { return economy.byId(id); } - @Override public Collection allEconomy() { return economy.all(); } - @Override public boolean hasEconomy(String id) { return economy.has(id); } - - // ---- Permissions ----------------------------------------------------- - - @Override public void registerPermissions(String id, PermissionsHandler h, int pr, MenuExtension o) { permissions.put(id, h, pr, o); } - @Override public PermissionsHandler permissions() { return resolveWithConfig("permissions", permissions); } - @Override public PermissionsHandler permissions(String id) { return permissions.byId(id); } - @Override public Collection allPermissions() { return permissions.all(); } - @Override public boolean hasPermissions(String id) { return permissions.has(id); } - - // ---- Levels ---------------------------------------------------------- - - @Override public void registerLevels(String id, LevelHandler h, int pr, MenuExtension o) { levels.put(id, h, pr, o); } - @Override public LevelHandler levels() { return resolveWithConfig("levels", levels); } - @Override public LevelHandler levels(String id) { return levels.byId(id); } - @Override public Collection allLevels() { return levels.all(); } - @Override public boolean hasLevels(String id) { return levels.has(id); } - - // ---- Placeholders ---------------------------------------------------- - - @Override public void registerPlaceholders(String id, PlaceholderHandler h, int pr, MenuExtension o) { placeholders.put(id, h, pr, o); } - @Override public PlaceholderHandler placeholders() { return resolveWithConfig("placeholders", placeholders); } - @Override public PlaceholderHandler placeholders(String id) { return placeholders.byId(id); } - @Override public Collection allPlaceholders() { return placeholders.all(); } - @Override public boolean hasPlaceholders(String id) { return placeholders.has(id); } - - // ---- Skins ----------------------------------------------------------- - - @Override public void registerSkins(String id, SkinHandler h, int pr, MenuExtension o) { skins.put(id, h, pr, o); } - @Override public SkinHandler skins() { return resolveWithConfig("skins", skins); } - @Override public SkinHandler skins(String id) { return skins.byId(id); } - @Override public Collection allSkins() { return skins.all(); } - @Override public boolean hasSkins(String id) { return skins.has(id); } - - // ---- Cleanup --------------------------------------------------------- + @Override public ProviderSection economy() { return economy; } + @Override public ProviderSection permissions() { return permissions; } + @Override public ProviderSection levels() { return levels; } + @Override public ProviderSection placeholders() { return placeholders; } + @Override public ProviderSection skins() { return skins; } /** * Wipe every provider registered by {@code owner} across all five @@ -110,45 +69,36 @@ public void unregisterAll(MenuExtension owner) { skins.unregisterAll(owner); } - // ---- Inner section --------------------------------------------------- - - private static final class Section { - private final Map> byId = new LinkedHashMap<>(); - private final Map> keysByOwner = new IdentityHashMap<>(); + // ----------------------------------------------------------------- + // SectionImpl - one instance per provider type + // ----------------------------------------------------------------- - synchronized void put(String id, T handler, int priority, MenuExtension owner) { - String k = id.toLowerCase(); - byId.put(k, new Entry<>(handler, priority)); - keysByOwner.computeIfAbsent(owner, o -> new HashSet<>()).add(k); - } + private static final class SectionImpl implements ProviderSection { - synchronized T byId(String id) { - Entry e = byId.get(id.toLowerCase()); - return e == null ? null : e.handler; - } + private final String kind; + private final ProviderRegistryImpl owner; + private final Map> byId = new LinkedHashMap<>(); + private final Map> keysByExtension = new IdentityHashMap<>(); - synchronized boolean has(String id) { - return byId.containsKey(id.toLowerCase()); + SectionImpl(String kind, ProviderRegistryImpl owner) { + this.kind = kind; + this.owner = owner; } - synchronized T auto() { - Entry best = null; - for (Entry e : byId.values()) { - if (best == null || e.priority > best.priority) best = e; - } - return best == null ? null : best.handler; + @Override + public synchronized void register(String id, T handler, int priority, MenuExtension extOwner) { + String k = id.toLowerCase(); + byId.put(k, new Entry<>(handler, priority)); + keysByExtension.computeIfAbsent(extOwner, o -> new HashSet<>()).add(k); } - /** - * Resolve a provider in one critical section: try the configured id - * first, fall back to {@link #auto()} on miss. Holding the lock - * across both lookups closes the race where a concurrent register / - * unregister could move the configured id under our feet between - * the two separate calls. - */ - synchronized T resolveWithDefault(String configuredId) { - if (configuredId != null && !configuredId.equalsIgnoreCase("auto")) { - Entry e = byId.get(configuredId.toLowerCase()); + @Override + public synchronized T resolve() { + // configDefaults is mutated only once (during plugin startup) + // so reading it without our lock is fine. + String configured = owner.configDefaults.apply(kind); + if (configured != null && !configured.equalsIgnoreCase("auto")) { + Entry e = byId.get(configured.toLowerCase()); if (e != null) return e.handler; } Entry best = null; @@ -158,14 +108,26 @@ synchronized T resolveWithDefault(String configuredId) { return best == null ? null : best.handler; } - synchronized Collection all() { + @Override + public synchronized T resolve(String id) { + Entry e = byId.get(id.toLowerCase()); + return e == null ? null : e.handler; + } + + @Override + public synchronized Collection all() { List list = new ArrayList<>(byId.size()); for (Entry e : byId.values()) list.add(e.handler); return Collections.unmodifiableList(list); } - synchronized void unregisterAll(MenuExtension owner) { - Set keys = keysByOwner.remove(owner); + @Override + public synchronized boolean has(String id) { + return byId.containsKey(id.toLowerCase()); + } + + synchronized void unregisterAll(MenuExtension extOwner) { + Set keys = keysByExtension.remove(extOwner); if (keys == null) return; for (String k : keys) byId.remove(k); } diff --git a/plugin/src/main/java/ru/abstractmenus/menu/AbstractMenu.java b/plugin/src/main/java/ru/abstractmenus/menu/AbstractMenu.java index 6bddde4..7d05bc5 100644 --- a/plugin/src/main/java/ru/abstractmenus/menu/AbstractMenu.java +++ b/plugin/src/main/java/ru/abstractmenus/menu/AbstractMenu.java @@ -358,7 +358,7 @@ protected int getFreeSlot() { } protected void createInventory(Player player, InventoryHolder holder) { - String title = AbstractMenusApi.get().providers().placeholders().replace(player, this.title); + String title = AbstractMenusApi.get().providers().placeholders().resolve().replace(player, this.title); title = MiniMessageUtil.parseToLegacy(title); if (this.type != null) { diff --git a/plugin/src/test/java/ru/abstractmenus/data/MoneyProviderSelectionTest.java b/plugin/src/test/java/ru/abstractmenus/data/MoneyProviderSelectionTest.java index 4339271..e41e7a2 100644 --- a/plugin/src/test/java/ru/abstractmenus/data/MoneyProviderSelectionTest.java +++ b/plugin/src/test/java/ru/abstractmenus/data/MoneyProviderSelectionTest.java @@ -54,8 +54,8 @@ void setUp() { // can resolve 'amount' nodes. In production this is done in Serializers.init(). support.api().serializers().register(TypeDouble.class, new TypeDouble.Serializer()); - support.providers().registerEconomy("vault", vault, VAULT_PRIORITY, support.owner()); - support.providers().registerEconomy("playerpoints", pp, PP_PRIORITY, support.owner()); + support.providers().economy().register("vault", vault, VAULT_PRIORITY, support.owner()); + support.providers().economy().register("playerpoints", pp, PP_PRIORITY, support.owner()); } @AfterEach diff --git a/plugin/src/test/java/ru/abstractmenus/data/actions/TestActionCommandBehavior.java b/plugin/src/test/java/ru/abstractmenus/data/actions/TestActionCommandBehavior.java index 553342f..8a96d01 100644 --- a/plugin/src/test/java/ru/abstractmenus/data/actions/TestActionCommandBehavior.java +++ b/plugin/src/test/java/ru/abstractmenus/data/actions/TestActionCommandBehavior.java @@ -91,7 +91,7 @@ void placeholderReplacementRunsExactlyOnce() throws Exception { @Override public String name() { return "countingTestOwner"; } @Override public void onEnable(ru.abstractmenus.api.AbstractMenusApi api) {} }; - apiSupport.providers().registerPlaceholders("counting", new PlaceholderHandler() { + apiSupport.providers().placeholders().register("counting", new PlaceholderHandler() { @Override public String replacePlaceholder(Player p, String s) { return s; } @Override public String replace(Player p, String s) { callCount[0]++; return s; } @Override public List replace(Player p, List l) { return l; } diff --git a/plugin/src/test/java/ru/abstractmenus/impl/ProviderRegistryImplTest.java b/plugin/src/test/java/ru/abstractmenus/impl/ProviderRegistryImplTest.java index fbae26f..b311e20 100644 --- a/plugin/src/test/java/ru/abstractmenus/impl/ProviderRegistryImplTest.java +++ b/plugin/src/test/java/ru/abstractmenus/impl/ProviderRegistryImplTest.java @@ -25,116 +25,116 @@ void setUp() { @Test void register_singleProvider_resolvesByIdAndAuto() { EconomyHandler vault = mock(EconomyHandler.class); - registry.registerEconomy("vault", vault, 50, ownerA); + registry.economy().register("vault", vault, 50, ownerA); - assertSame(vault, registry.economy()); - assertSame(vault, registry.economy("vault")); - assertEquals(1, registry.allEconomy().size()); - assertTrue(registry.hasEconomy("vault")); + assertSame(vault, registry.economy().resolve()); + assertSame(vault, registry.economy().resolve("vault")); + assertEquals(1, registry.economy().all().size()); + assertTrue(registry.economy().has("vault")); } @Test void auto_returnsNullWhenEmpty() { - assertNull(registry.economy()); - assertFalse(registry.hasEconomy("anything")); + assertNull(registry.economy().resolve()); + assertFalse(registry.economy().has("anything")); } @Test void auto_highestPriorityWins() { EconomyHandler vault = mock(EconomyHandler.class); EconomyHandler pp = mock(EconomyHandler.class); - registry.registerEconomy("vault", vault, 50, ownerA); - registry.registerEconomy("playerpoints", pp, 100, ownerA); + registry.economy().register("vault", vault, 50, ownerA); + registry.economy().register("playerpoints", pp, 100, ownerA); - assertSame(pp, registry.economy()); + assertSame(pp, registry.economy().resolve()); } @Test void auto_tieBreaksToFirstRegistered() { EconomyHandler a = mock(EconomyHandler.class); EconomyHandler b = mock(EconomyHandler.class); - registry.registerEconomy("alpha", a, 50, ownerA); - registry.registerEconomy("beta", b, 50, ownerA); + registry.economy().register("alpha", a, 50, ownerA); + registry.economy().register("beta", b, 50, ownerA); - assertSame(a, registry.economy()); + assertSame(a, registry.economy().resolve()); } @Test void lookupById_caseInsensitive() { EconomyHandler vault = mock(EconomyHandler.class); - registry.registerEconomy("Vault", vault, 50, ownerA); + registry.economy().register("Vault", vault, 50, ownerA); - assertSame(vault, registry.economy("VAULT")); - assertSame(vault, registry.economy("vault")); - assertTrue(registry.hasEconomy("vAuLt")); + assertSame(vault, registry.economy().resolve("VAULT")); + assertSame(vault, registry.economy().resolve("vault")); + assertTrue(registry.economy().has("vAuLt")); } @Test void unregisterAll_removesOnlyThatOwner() { EconomyHandler ea = mock(EconomyHandler.class); EconomyHandler eb = mock(EconomyHandler.class); - registry.registerEconomy("a", ea, 50, ownerA); - registry.registerEconomy("b", eb, 50, ownerB); + registry.economy().register("a", ea, 50, ownerA); + registry.economy().register("b", eb, 50, ownerB); registry.unregisterAll(ownerA); - assertNull(registry.economy("a")); - assertSame(eb, registry.economy("b")); - assertEquals(1, registry.allEconomy().size()); + assertNull(registry.economy().resolve("a")); + assertSame(eb, registry.economy().resolve("b")); + assertEquals(1, registry.economy().all().size()); } @Test void overwrite_replacesPrevious() { EconomyHandler old = mock(EconomyHandler.class); EconomyHandler fresh = mock(EconomyHandler.class); - registry.registerEconomy("vault", old, 50, ownerA); - registry.registerEconomy("vault", fresh, 50, ownerB); + registry.economy().register("vault", old, 50, ownerA); + registry.economy().register("vault", fresh, 50, ownerB); - assertSame(fresh, registry.economy("vault")); + assertSame(fresh, registry.economy().resolve("vault")); } @Test void sectionsAreIndependent() { EconomyHandler e = mock(EconomyHandler.class); - registry.registerEconomy("e", e, 50, ownerA); + registry.economy().register("e", e, 50, ownerA); - assertNull(registry.permissions()); - assertNull(registry.levels()); - assertNull(registry.placeholders()); - assertNull(registry.skins()); + assertNull(registry.permissions().resolve()); + assertNull(registry.levels().resolve()); + assertNull(registry.placeholders().resolve()); + assertNull(registry.skins().resolve()); } @Test void configDefault_prefersConfiguredId() { EconomyHandler vault = mock(EconomyHandler.class); EconomyHandler pp = mock(EconomyHandler.class); - registry.registerEconomy("vault", vault, 50, ownerA); - registry.registerEconomy("playerpoints", pp, 100, ownerA); + registry.economy().register("vault", vault, 50, ownerA); + registry.economy().register("playerpoints", pp, 100, ownerA); // Without config, auto prefers playerpoints (priority 100). - assertSame(pp, registry.economy()); + assertSame(pp, registry.economy().resolve()); // With config override to vault, vault wins despite lower priority. registry.setConfigDefaults(kind -> "economy".equals(kind) ? "vault" : null); - assertSame(vault, registry.economy()); + assertSame(vault, registry.economy().resolve()); } @Test void configDefault_autoKeyword_fallsBackToAuto() { EconomyHandler vault = mock(EconomyHandler.class); - registry.registerEconomy("vault", vault, 50, ownerA); + registry.economy().register("vault", vault, 50, ownerA); registry.setConfigDefaults(kind -> "auto"); - assertSame(vault, registry.economy()); + assertSame(vault, registry.economy().resolve()); } @Test void configDefault_unknownId_fallsBackToAuto() { EconomyHandler vault = mock(EconomyHandler.class); - registry.registerEconomy("vault", vault, 50, ownerA); + registry.economy().register("vault", vault, 50, ownerA); registry.setConfigDefaults(kind -> "ghost"); // not registered - assertSame(vault, registry.economy()); // auto fallback + assertSame(vault, registry.economy().resolve()); // auto fallback } // --- helper --- diff --git a/plugin/src/test/java/ru/abstractmenus/testsupport/ApiTestSupport.java b/plugin/src/test/java/ru/abstractmenus/testsupport/ApiTestSupport.java index 848d66a..16f83ce 100644 --- a/plugin/src/test/java/ru/abstractmenus/testsupport/ApiTestSupport.java +++ b/plugin/src/test/java/ru/abstractmenus/testsupport/ApiTestSupport.java @@ -41,7 +41,7 @@ * @BeforeAll * static void setUp() { * support = ApiTestSupport.install(); - * support.providers().registerPlaceholders("test", myHandler, 100, support.owner()); + * support.providers().placeholders().register("test", myHandler, 100, support.owner()); * } * * @AfterAll @@ -78,7 +78,7 @@ public static ApiTestSupport install() { /** Shortcut: register {@code handler} as the test placeholder provider. */ public void installPlaceholderHandler(PlaceholderHandler handler) { - api.providers().registerPlaceholders("test", handler, 100, owner); + api.providers().placeholders().register("test", handler, 100, owner); } @Override From d1349e0e8b95b2254cb2953e2bf78d001e5ffc74 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 12:36:56 +0500 Subject: [PATCH 06/16] style: replace fully-qualified names with imports, drop column alignment AddonManager had fully-qualified java.util.jar.JarFile, java.nio.file.Files, java.io.IOException, java.nio.charset.StandardCharsets, java.net.URL, and ru.abstractmenus.api.MenuExtension peppered through the body. Hoist them into the import block so the code reads like Java instead of like a textbook example. Drop column-aligned padding in: - AddonConf.parse() variable declarations - ProviderRegistryImpl section field initializers + getters Replace stale {@link ProviderRegistry#registerEconomy} (and friends) javadoc references with {@link ProviderSection#register} since the parametric refactor moved register() onto the section type. Five handler-interface javadocs touched. --- .../abstractmenus/api/ProviderRegistry.java | 6 +- .../api/handler/EconomyHandler.java | 4 +- .../api/handler/LevelHandler.java | 4 +- .../api/handler/PermissionsHandler.java | 4 +- .../api/handler/PlaceholderHandler.java | 4 +- .../api/handler/SkinHandler.java | 4 +- .../ru/abstractmenus/addon/AddonConf.java | 16 +-- .../ru/abstractmenus/addon/AddonManager.java | 99 ++++++++++--------- .../impl/ProviderRegistryImpl.java | 37 +++++-- 9 files changed, 103 insertions(+), 75 deletions(-) diff --git a/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java b/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java index f238a24..f14a945 100644 --- a/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java +++ b/api/src/main/java/ru/abstractmenus/api/ProviderRegistry.java @@ -47,13 +47,13 @@ */ public interface ProviderRegistry { - ProviderSection economy(); + ProviderSection economy(); ProviderSection permissions(); - ProviderSection levels(); + ProviderSection levels(); ProviderSection placeholders(); - ProviderSection skins(); + ProviderSection skins(); } diff --git a/api/src/main/java/ru/abstractmenus/api/handler/EconomyHandler.java b/api/src/main/java/ru/abstractmenus/api/handler/EconomyHandler.java index 382dec9..c20ab62 100644 --- a/api/src/main/java/ru/abstractmenus/api/handler/EconomyHandler.java +++ b/api/src/main/java/ru/abstractmenus/api/handler/EconomyHandler.java @@ -20,7 +20,7 @@ *

Registration

* * A single handler is active at a time. Register yours via - * {@link ru.abstractmenus.api.ProviderRegistry#registerEconomy} inside your + * {@link ru.abstractmenus.api.ProviderSection#register} inside your * addon's {@link ru.abstractmenus.api.MenuExtension#onEnable}: * *

Example — bridging PlayerPoints

@@ -76,7 +76,7 @@ * blocking IO; if the underlying economy plugin hits a database, cache the * last known balance and refresh asynchronously. * - * @see ru.abstractmenus.api.ProviderRegistry#registerEconomy + * @see ru.abstractmenus.api.ProviderSection#register * @see PermissionsHandler * @see LevelHandler */ diff --git a/api/src/main/java/ru/abstractmenus/api/handler/LevelHandler.java b/api/src/main/java/ru/abstractmenus/api/handler/LevelHandler.java index 49f6c19..bfc6919 100644 --- a/api/src/main/java/ru/abstractmenus/api/handler/LevelHandler.java +++ b/api/src/main/java/ru/abstractmenus/api/handler/LevelHandler.java @@ -24,7 +24,7 @@ * * Multiple handlers may coexist — the highest-priority one is picked * when a menu does not name a provider explicitly. Register yours via - * {@link ru.abstractmenus.api.ProviderRegistry#registerLevels} inside your + * {@link ru.abstractmenus.api.ProviderSection#register} inside your * addon's {@link ru.abstractmenus.api.MenuExtension#onEnable}. * *

Example — bridging MMOCore

@@ -91,7 +91,7 @@ * blocking IO; if the underlying levelling plugin hits a database, serve the * last known value from memory and refresh asynchronously. * - * @see ru.abstractmenus.api.ProviderRegistry#registerLevels + * @see ru.abstractmenus.api.ProviderSection#register * @see EconomyHandler * @see PermissionsHandler */ diff --git a/api/src/main/java/ru/abstractmenus/api/handler/PermissionsHandler.java b/api/src/main/java/ru/abstractmenus/api/handler/PermissionsHandler.java index ad3f3cb..bad4101 100644 --- a/api/src/main/java/ru/abstractmenus/api/handler/PermissionsHandler.java +++ b/api/src/main/java/ru/abstractmenus/api/handler/PermissionsHandler.java @@ -27,7 +27,7 @@ * * Multiple handlers may coexist — the highest-priority one is picked * when a menu does not name a provider explicitly. Register yours via - * {@link ru.abstractmenus.api.ProviderRegistry#registerPermissions} inside + * {@link ru.abstractmenus.api.ProviderSection#register} inside * your addon's {@link ru.abstractmenus.api.MenuExtension#onEnable}. * *

If no permissions handler is registered, permission nodes are checked @@ -112,7 +112,7 @@ * asynchronously provided the in-memory view updates before the next * {@code has*} call. * - * @see ru.abstractmenus.api.ProviderRegistry#registerPermissions + * @see ru.abstractmenus.api.ProviderSection#register * @see EconomyHandler * @see LevelHandler */ diff --git a/api/src/main/java/ru/abstractmenus/api/handler/PlaceholderHandler.java b/api/src/main/java/ru/abstractmenus/api/handler/PlaceholderHandler.java index eb049f9..7b57fb6 100644 --- a/api/src/main/java/ru/abstractmenus/api/handler/PlaceholderHandler.java +++ b/api/src/main/java/ru/abstractmenus/api/handler/PlaceholderHandler.java @@ -34,7 +34,7 @@ * Although the placeholder handler lives in * {@link ru.abstractmenus.api.ProviderRegistry} for API symmetry with the * other sections, in practice it is registered once globally - * via {@link ru.abstractmenus.api.ProviderRegistry#registerPlaceholders} at + * via {@link ru.abstractmenus.api.ProviderSection#register} at * {@link ru.abstractmenus.api.MenuExtension#onEnable} time, not selected * per-action. Per-element provider selection is not meaningful for * placeholders — the core engine (PAPI) is chain-of-responsibility by @@ -97,7 +97,7 @@ * cache aggressively and serve stale values. The engine itself should never * block. * - * @see ru.abstractmenus.api.ProviderRegistry#registerPlaceholders + * @see ru.abstractmenus.api.ProviderSection#register * @see EconomyHandler */ public interface PlaceholderHandler { diff --git a/api/src/main/java/ru/abstractmenus/api/handler/SkinHandler.java b/api/src/main/java/ru/abstractmenus/api/handler/SkinHandler.java index ef865eb..f5bc9ba 100644 --- a/api/src/main/java/ru/abstractmenus/api/handler/SkinHandler.java +++ b/api/src/main/java/ru/abstractmenus/api/handler/SkinHandler.java @@ -24,7 +24,7 @@ * * Multiple handlers may coexist — the highest-priority one is picked * when a menu does not name a provider explicitly. Register yours via - * {@link ru.abstractmenus.api.ProviderRegistry#registerSkins} inside your + * {@link ru.abstractmenus.api.ProviderSection#register} inside your * addon's {@link ru.abstractmenus.api.MenuExtension#onEnable}. * *

When no skin handler is registered, {@code setSkin} and {@code resetSkin} @@ -96,7 +96,7 @@ * and apply on the main thread — blocking here freezes every online * player. * - * @see ru.abstractmenus.api.ProviderRegistry#registerSkins + * @see ru.abstractmenus.api.ProviderSection#register * @see EconomyHandler */ public interface SkinHandler { diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonConf.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonConf.java index 455a8e8..f6ffc1e 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonConf.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonConf.java @@ -48,15 +48,15 @@ public static AddonConf parse(String hocon) { throw new AddonConfParseException("Failed to parse addon.conf: " + e.getMessage(), e); } - String name = requireString(root, "name"); - String version = requireString(root, "version"); - String main = requireString(root, "main"); + String name = requireString(root, "name"); + String version = requireString(root, "version"); + String main = requireString(root, "main"); - List authors = optionalStringList(root, "authors"); - String description = root.node("description").getString(""); - String targetApiVersion = optionalString(root, "targetApiVersion"); - List addonDependencies = optionalStringList(root, "addonDependencies"); - List pluginDependencies = optionalStringList(root, "pluginDependencies"); + List authors = optionalStringList(root, "authors"); + String description = root.node("description").getString(""); + String targetApiVersion = optionalString(root, "targetApiVersion"); + List addonDependencies = optionalStringList(root, "addonDependencies"); + List pluginDependencies = optionalStringList(root, "pluginDependencies"); List pluginSoftDependencies = optionalStringList(root, "pluginSoftDependencies"); return new AddonConf( diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java index 9545f4e..39b15e0 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java @@ -3,7 +3,14 @@ import ru.abstractmenus.AbstractMenus; import ru.abstractmenus.api.AbstractMenusApi; import ru.abstractmenus.api.Logger; - +import ru.abstractmenus.api.MenuExtension; +import ru.abstractmenus.impl.ProviderRegistryImpl; +import ru.abstractmenus.impl.TypeRegistryImpl; + +import java.io.IOException; +import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; import java.nio.file.Path; import java.util.ArrayList; import java.util.Collection; @@ -12,6 +19,8 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.jar.JarEntry; +import java.util.jar.JarFile; /** * Loads, enables, and manages AM-loaded addons (the lightweight jars in @@ -23,6 +32,14 @@ */ public final class AddonManager { + /** + * TTL for the availableNotLoaded() cache. Tab completion fires on every + * keystroke; without a cache that is N jar opens + N HOCON parses on the + * main thread per TAB. 2 seconds is short enough that the operator does + * not see staleness in practice (drop a jar, wait a beat, hit TAB). + */ + private static final long AVAILABLE_CACHE_TTL_MS = 2_000L; + private final AbstractMenus plugin; private final Path addonsDir; private final AbstractMenusApi api; @@ -30,6 +47,9 @@ public final class AddonManager { /** name (lowercased) → LoadedAddon, insertion-ordered (matches enable order) */ private final Map addons = new LinkedHashMap<>(); + private volatile List cachedAvailable = List.of(); + private volatile long cachedAvailableAt = 0L; + public AddonManager(AbstractMenus plugin, AbstractMenusApi api) { this.plugin = plugin; this.api = api; @@ -132,13 +152,13 @@ public void loadAll() { * Reflectively instantiate the addon's main class and verify it implements * MenuExtension. */ - private ru.abstractmenus.api.MenuExtension instantiate(LoadedAddon la) throws Exception { + private MenuExtension instantiate(LoadedAddon la) throws Exception { Class main = la.getClassLoader().loadClass(la.getConf().main()); - if (!ru.abstractmenus.api.MenuExtension.class.isAssignableFrom(main)) { + if (!MenuExtension.class.isAssignableFrom(main)) { throw new IllegalStateException("main class " + main.getName() + " does not implement MenuExtension"); } - return (ru.abstractmenus.api.MenuExtension) main.getDeclaredConstructor().newInstance(); + return (MenuExtension) main.getDeclaredConstructor().newInstance(); } /** @@ -151,13 +171,13 @@ private ru.abstractmenus.api.MenuExtension instantiate(LoadedAddon la) throws Ex */ private void rollbackRegistrations(LoadedAddon la) { if (la.getExtension() == null) return; - ru.abstractmenus.api.MenuExtension ext = la.getExtension(); - ((ru.abstractmenus.impl.TypeRegistryImpl) api.actions()).unregisterAll(ext); - ((ru.abstractmenus.impl.TypeRegistryImpl) api.rules()).unregisterAll(ext); - ((ru.abstractmenus.impl.TypeRegistryImpl) api.activators()).unregisterAll(ext); - ((ru.abstractmenus.impl.TypeRegistryImpl) api.itemProperties()).unregisterAll(ext); - ((ru.abstractmenus.impl.TypeRegistryImpl) api.catalogs()).unregisterAll(ext); - ((ru.abstractmenus.impl.ProviderRegistryImpl) api.providers()).unregisterAll(ext); + MenuExtension ext = la.getExtension(); + ((TypeRegistryImpl) api.actions()).unregisterAll(ext); + ((TypeRegistryImpl) api.rules()).unregisterAll(ext); + ((TypeRegistryImpl) api.activators()).unregisterAll(ext); + ((TypeRegistryImpl) api.itemProperties()).unregisterAll(ext); + ((TypeRegistryImpl) api.catalogs()).unregisterAll(ext); + ((ProviderRegistryImpl) api.providers()).unregisterAll(ext); } /** @@ -166,8 +186,8 @@ private void rollbackRegistrations(LoadedAddon la) { */ public void unloadAll() { // Disable in reverse enable order. - var reversed = new java.util.ArrayList<>(addons.values()); - java.util.Collections.reverse(reversed); + var reversed = new ArrayList<>(addons.values()); + Collections.reverse(reversed); for (LoadedAddon la : reversed) { try { if (la.getStatus() == AddonStatus.ENABLED && la.getExtension() != null) { @@ -253,14 +273,14 @@ public Optional reload(String name) { /** Scan addonsDir again, return the first jar whose addon.conf.name matches. */ private Path findJarByName(String name) { - if (!java.nio.file.Files.isDirectory(addonsDir)) return null; - try (var stream = java.nio.file.Files.newDirectoryStream(addonsDir, "*.jar")) { + if (!Files.isDirectory(addonsDir)) return null; + try (var stream = Files.newDirectoryStream(addonsDir, "*.jar")) { for (Path jar : stream) { - try (var jf = new java.util.jar.JarFile(jar.toFile())) { - var entry = jf.getJarEntry("addon.conf"); + try (var jf = new JarFile(jar.toFile())) { + JarEntry entry = jf.getJarEntry("addon.conf"); if (entry == null) continue; String hocon = new String(jf.getInputStream(entry).readAllBytes(), - java.nio.charset.StandardCharsets.UTF_8); + StandardCharsets.UTF_8); AddonConf c = AddonConf.parse(hocon); if (c.name().equalsIgnoreCase(name)) return jar; } catch (Exception ignored) {} @@ -291,16 +311,16 @@ public Optional get(String name) { Map discover() { Map pending = new LinkedHashMap<>(); - if (!java.nio.file.Files.isDirectory(addonsDir)) { + if (!Files.isDirectory(addonsDir)) { try { - java.nio.file.Files.createDirectories(addonsDir); - } catch (java.io.IOException e) { + Files.createDirectories(addonsDir); + } catch (IOException e) { Logger.warning("Could not create addons directory " + addonsDir + ": " + e.getMessage()); } return pending; } - try (var stream = java.nio.file.Files.newDirectoryStream(addonsDir, "*.jar")) { + try (var stream = Files.newDirectoryStream(addonsDir, "*.jar")) { for (Path jar : stream) { try { LoadedAddon addon = readAddonJar(jar); @@ -316,7 +336,7 @@ Map discover() { Logger.warning("Failed to load addon " + jar.getFileName() + ": " + e.getMessage()); } } - } catch (java.io.IOException e) { + } catch (IOException e) { Logger.warning("Failed to scan addons directory: " + e.getMessage()); } @@ -379,16 +399,6 @@ public List rescan() { return newlyLoaded; } - /** - * TTL for the availableNotLoaded() cache. Tab completion fires on every - * keystroke; without a cache that is N jar opens + N HOCON parses on the - * main thread per TAB. 2 seconds is short enough that the operator does - * not see staleness in practice (drop a jar, wait a beat, hit TAB). - */ - private static final long AVAILABLE_CACHE_TTL_MS = 2_000L; - private volatile List cachedAvailable = List.of(); - private volatile long cachedAvailableAt = 0L; - /** * Return addon-conf {@code name}s found on disk under the addons * directory but not yet loaded into memory. Used by tab completion @@ -409,15 +419,15 @@ public List availableNotLoaded() { } private List scanAvailableNotLoaded() { - if (!java.nio.file.Files.isDirectory(addonsDir)) return List.of(); + if (!Files.isDirectory(addonsDir)) return List.of(); List result = new ArrayList<>(); - try (var stream = java.nio.file.Files.newDirectoryStream(addonsDir, "*.jar")) { + try (var stream = Files.newDirectoryStream(addonsDir, "*.jar")) { for (Path jar : stream) { - try (var jf = new java.util.jar.JarFile(jar.toFile())) { - var entry = jf.getJarEntry("addon.conf"); + try (var jf = new JarFile(jar.toFile())) { + JarEntry entry = jf.getJarEntry("addon.conf"); if (entry == null) continue; String hocon = new String(jf.getInputStream(entry).readAllBytes(), - java.nio.charset.StandardCharsets.UTF_8); + StandardCharsets.UTF_8); AddonConf conf = AddonConf.parse(hocon); if (!addons.containsKey(conf.name().toLowerCase())) { result.add(conf.name()); @@ -518,15 +528,15 @@ private void enableSingle(LoadedAddon la) { * Read a single addon jar: extract {@code addon.conf}, parse it, build a * classloader. Throws if addon.conf is missing or malformed. */ - private LoadedAddon readAddonJar(Path jarPath) throws java.io.IOException { + private LoadedAddon readAddonJar(Path jarPath) throws IOException { String hocon; - try (var jar = new java.util.jar.JarFile(jarPath.toFile())) { - var entry = jar.getJarEntry("addon.conf"); + try (var jar = new JarFile(jarPath.toFile())) { + JarEntry entry = jar.getJarEntry("addon.conf"); if (entry == null) { - throw new java.io.IOException("no addon.conf at jar root"); + throw new IOException("no addon.conf at jar root"); } try (var in = jar.getInputStream(entry)) { - hocon = new String(in.readAllBytes(), java.nio.charset.StandardCharsets.UTF_8); + hocon = new String(in.readAllBytes(), StandardCharsets.UTF_8); } } @@ -535,10 +545,9 @@ private LoadedAddon readAddonJar(Path jarPath) throws java.io.IOException { ? plugin.getClass().getClassLoader() : AddonManager.class.getClassLoader(); AddonClassLoader cl = new AddonClassLoader( - new java.net.URL[]{jarPath.toUri().toURL()}, + new URL[]{jarPath.toUri().toURL()}, parent); return new LoadedAddon(conf, cl); } - } diff --git a/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java index f844de6..89bfb81 100644 --- a/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java +++ b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java @@ -38,22 +38,41 @@ public final class ProviderRegistryImpl implements ProviderRegistry { /** section kind → configured-default id (e.g. "economy" → "playerpoints"). */ private Function configDefaults = kind -> null; - private final SectionImpl economy = new SectionImpl<>("economy", this); - private final SectionImpl permissions = new SectionImpl<>("permissions", this); - private final SectionImpl levels = new SectionImpl<>("levels", this); + private final SectionImpl economy = new SectionImpl<>("economy", this); + private final SectionImpl permissions = new SectionImpl<>("permissions", this); + private final SectionImpl levels = new SectionImpl<>("levels", this); private final SectionImpl placeholders = new SectionImpl<>("placeholders", this); - private final SectionImpl skins = new SectionImpl<>("skins", this); + private final SectionImpl skins = new SectionImpl<>("skins", this); /** Wire up the config-backed default source. Called once from AbstractMenusApiImpl. */ public void setConfigDefaults(Function lookup) { this.configDefaults = lookup; } - @Override public ProviderSection economy() { return economy; } - @Override public ProviderSection permissions() { return permissions; } - @Override public ProviderSection levels() { return levels; } - @Override public ProviderSection placeholders() { return placeholders; } - @Override public ProviderSection skins() { return skins; } + @Override + public ProviderSection economy() { + return economy; + } + + @Override + public ProviderSection permissions() { + return permissions; + } + + @Override + public ProviderSection levels() { + return levels; + } + + @Override + public ProviderSection placeholders() { + return placeholders; + } + + @Override + public ProviderSection skins() { + return skins; + } /** * Wipe every provider registered by {@code owner} across all five From ef86e3649bac408b7f98e18d3ee3b2520d65ae7a Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 12:50:43 +0500 Subject: [PATCH 07/16] fix: P2 batch A - AddonManager polish (P2-7 / P2-8 / P2-9 / P2-17) P2-7 - dedup the open-jar-read-addon.conf-parse triple. findJarByName and availableNotLoaded both did the same scan; extracted scanJarConfs() that returns Path -> AddonConf for every parseable jar. The two callers now filter the same map. discover() stays separate because it ALSO builds the AddonClassLoader, which we do not want for the tab-completion path. P2-8 - replaced the nullable plugin field with a PluginDepChecker functional interface plus a parentClassLoader field. The production constructor wires both up from the AbstractMenus instance; the test constructor uses ALL_PRESENT (every name reports loaded) and the addon module's own classloader. Result: zero 'if (plugin != null)' guards in the body, every method works the same way regardless of whether tests or the real plugin is calling. P2-9 - Stage 2 of loadAll now also skips entries whose extension is null. In practice the FAILED check covers this (instantiate() throws inside the catch which calls markFailed), but the local-invariant guard makes the body NPE-proof if Stage 1 ever evolves to leave a non-FAILED-but-null state. P2-17 - reload(name) now goes through enableSingle(fresh) instead of inlining instantiate + onLoad + onEnable. enableSingle re-runs both plugin-dep and addon-dep checks, so a dependency that disappeared between the original load and this reload causes a clean FAILED state with the expected error message instead of a confusing trace deep inside onEnable. Bonus: the cached availableNotLoaded list is invalidated as part of reload. --- .../ru/abstractmenus/addon/AddonManager.java | 119 ++++++++---------- .../abstractmenus/addon/PluginDepChecker.java | 20 +++ 2 files changed, 74 insertions(+), 65 deletions(-) create mode 100644 plugin/src/main/java/ru/abstractmenus/addon/PluginDepChecker.java diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java index 39b15e0..1111f34 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java @@ -40,9 +40,10 @@ public final class AddonManager { */ private static final long AVAILABLE_CACHE_TTL_MS = 2_000L; - private final AbstractMenus plugin; private final Path addonsDir; private final AbstractMenusApi api; + private final PluginDepChecker depChecker; + private final ClassLoader parentClassLoader; /** name (lowercased) → LoadedAddon, insertion-ordered (matches enable order) */ private final Map addons = new LinkedHashMap<>(); @@ -51,21 +52,23 @@ public final class AddonManager { private volatile long cachedAvailableAt = 0L; public AddonManager(AbstractMenus plugin, AbstractMenusApi api) { - this.plugin = plugin; this.api = api; this.addonsDir = plugin.getDataFolder().toPath().resolve("addons"); + this.depChecker = name -> plugin.getServer().getPluginManager().getPlugin(name) != null; + this.parentClassLoader = plugin.getClass().getClassLoader(); } /** - * Test-only overload: inject addonsDir directly, skip Bukkit plugin-dep - * checks (since no {@code plugin.getServer()} is available in pure-unit - * tests). Any addon with a non-empty {@code pluginDependencies} will fail - * under this constructor. + * Test-only overload: inject addonsDir directly. Bukkit-plugin + * dependency checks are stubbed to always report present so a test + * addon can declare {@code pluginDependencies} without booting a + * server. */ AddonManager(Path addonsDir, AbstractMenusApi api) { - this.plugin = null; this.api = api; this.addonsDir = addonsDir; + this.depChecker = PluginDepChecker.ALL_PRESENT; + this.parentClassLoader = AddonManager.class.getClassLoader(); } /** @@ -126,7 +129,10 @@ public void loadAll() { // Stage 2: onEnable in dependency order. for (String k : order) { LoadedAddon la = byName.get(k); - if (la.getStatus() == AddonStatus.FAILED) { + // Skip both FAILED entries and the defensive case where Stage 1 + // somehow returned without an extension instance - either way, + // calling onEnable on a null extension would NPE. + if (la.getStatus() == AddonStatus.FAILED || la.getExtension() == null) { addons.put(k, la); continue; } @@ -214,6 +220,11 @@ public void unloadAll() { * re-parse the jar → enable. Returns the new {@link LoadedAddon}, or * empty if no addon of that name is currently loaded. * + *

Goes through {@link #enableSingle} so plugin-dep and addon-dep + * checks re-run; a dependency that disappeared between the original + * load and this reload causes a clean FAILED state instead of a + * confusing trace deep inside {@code onEnable}. + * * @param name addon name (case-insensitive) * @return the freshly loaded addon, or empty if not found / no jar present */ @@ -234,6 +245,7 @@ public Optional reload(String name) { } try { existing.getClassLoader().close(); } catch (Exception ignored) {} addons.remove(key); + cachedAvailableAt = 0L; // Re-discover: find the jar whose addon.conf name matches. Path freshJar = findJarByName(name); @@ -250,43 +262,17 @@ public Optional reload(String name) { return Optional.empty(); } - // Enable the single addon. We don't re-chain the full topological sort - // for a single-addon reload — assume its addonDependencies are already - // enabled (they were, before this reload). - try { - fresh.setExtension(instantiate(fresh)); - fresh.getExtension().onLoad(api); - fresh.getExtension().onEnable(api); - fresh.markEnabled(); - addons.put(fresh.getConf().name().toLowerCase(), fresh); - Logger.info("Reloaded addon: " + fresh.getConf().name() + " v" + fresh.getConf().version()); - } catch (Throwable t) { - Logger.severe("Addon " + name + " failed during reload: " + t); - t.printStackTrace(); - fresh.markFailed(t); - rollbackRegistrations(fresh); - addons.put(fresh.getConf().name().toLowerCase(), fresh); - } - + enableSingle(fresh); return Optional.of(fresh); } /** Scan addonsDir again, return the first jar whose addon.conf.name matches. */ private Path findJarByName(String name) { - if (!Files.isDirectory(addonsDir)) return null; - try (var stream = Files.newDirectoryStream(addonsDir, "*.jar")) { - for (Path jar : stream) { - try (var jf = new JarFile(jar.toFile())) { - JarEntry entry = jf.getJarEntry("addon.conf"); - if (entry == null) continue; - String hocon = new String(jf.getInputStream(entry).readAllBytes(), - StandardCharsets.UTF_8); - AddonConf c = AddonConf.parse(hocon); - if (c.name().equalsIgnoreCase(name)) return jar; - } catch (Exception ignored) {} - } - } catch (Exception ignored) {} - return null; + return scanJarConfs().entrySet().stream() + .filter(e -> e.getValue().name().equalsIgnoreCase(name)) + .map(Map.Entry::getKey) + .findFirst() + .orElse(null); } public Collection loaded() { @@ -413,14 +399,32 @@ public List availableNotLoaded() { if (now - cachedAvailableAt < AVAILABLE_CACHE_TTL_MS) { return cachedAvailable; } - cachedAvailable = scanAvailableNotLoaded(); + List result = scanJarConfs().values().stream() + .map(AddonConf::name) + .filter(n -> !addons.containsKey(n.toLowerCase())) + .toList(); + cachedAvailable = result; cachedAvailableAt = now; - return cachedAvailable; + return result; } - private List scanAvailableNotLoaded() { - if (!Files.isDirectory(addonsDir)) return List.of(); - List result = new ArrayList<>(); + /** + * One canonical pass over {@code addonsDir}: for each {@code *.jar}, + * read its {@code addon.conf} entry and parse it. Entries that lack + * addon.conf or fail to parse are skipped silently (the operator + * already saw the warning at startup discover() time). + * + *

Sole shared helper for {@link #findJarByName} and + * {@link #availableNotLoaded} to avoid duplicating the open-read-parse + * triple. Note that {@link #discover()} is separate because it ALSO + * builds the {@link AddonClassLoader}, which we don't want for the + * tab-completion path. + * + * @return jar Path → parsed AddonConf, in directory iteration order + */ + private Map scanJarConfs() { + if (!Files.isDirectory(addonsDir)) return Map.of(); + Map result = new LinkedHashMap<>(); try (var stream = Files.newDirectoryStream(addonsDir, "*.jar")) { for (Path jar : stream) { try (var jf = new JarFile(jar.toFile())) { @@ -428,14 +432,8 @@ private List scanAvailableNotLoaded() { if (entry == null) continue; String hocon = new String(jf.getInputStream(entry).readAllBytes(), StandardCharsets.UTF_8); - AddonConf conf = AddonConf.parse(hocon); - if (!addons.containsKey(conf.name().toLowerCase())) { - result.add(conf.name()); - } - } catch (Exception ignored) { - // Malformed jar - skip silently, the operator already saw - // the warning at server-start discover() time. - } + result.put(jar, AddonConf.parse(hocon)); + } catch (Exception ignored) {} } } catch (Exception ignored) {} return result; @@ -450,21 +448,15 @@ private List scanAvailableNotLoaded() { * method returns false. Soft-dep misses log a warning but do not * block enabling. * - *

Skipped entirely in test mode (plugin == null). Tests must not - * declare pluginDependencies; declaring pluginSoftDependencies is fine - * but logs nothing. - * * @return true if the addon may proceed to enable, false if a hard * dependency is missing */ private boolean checkPluginDeps(LoadedAddon la) { - if (plugin == null) return true; - var pm = plugin.getServer().getPluginManager(); AddonConf c = la.getConf(); String key = c.name().toLowerCase(); for (String dep : c.pluginDependencies()) { - if (pm.getPlugin(dep) == null) { + if (!depChecker.isPresent(dep)) { String msg = "missing plugin dependency: " + dep; Logger.warning("Addon " + c.name() + " " + msg + " - skipping"); la.markFailed(new IllegalStateException(msg)); @@ -474,7 +466,7 @@ private boolean checkPluginDeps(LoadedAddon la) { } for (String dep : c.pluginSoftDependencies()) { - if (pm.getPlugin(dep) == null) { + if (!depChecker.isPresent(dep)) { Logger.warning("Addon " + c.name() + " soft-depends on plugin '" + dep + "' which is not installed - features that need it may no-op"); @@ -541,12 +533,9 @@ private LoadedAddon readAddonJar(Path jarPath) throws IOException { } AddonConf conf = AddonConf.parse(hocon); - ClassLoader parent = (plugin != null) - ? plugin.getClass().getClassLoader() - : AddonManager.class.getClassLoader(); AddonClassLoader cl = new AddonClassLoader( new URL[]{jarPath.toUri().toURL()}, - parent); + parentClassLoader); return new LoadedAddon(conf, cl); } diff --git a/plugin/src/main/java/ru/abstractmenus/addon/PluginDepChecker.java b/plugin/src/main/java/ru/abstractmenus/addon/PluginDepChecker.java new file mode 100644 index 0000000..4e996b7 --- /dev/null +++ b/plugin/src/main/java/ru/abstractmenus/addon/PluginDepChecker.java @@ -0,0 +1,20 @@ +package ru.abstractmenus.addon; + +/** + * Bukkit-plugin presence probe used by {@link AddonManager} to verify + * an addon's {@code pluginDependencies} / {@code pluginSoftDependencies} + * before instantiation. + * + *

Lifted to its own interface so the production class doesn't carry + * a nullable {@code plugin} field that every caller has to guard - the + * test mode injects {@link #ALL_PRESENT} which always reports true. + */ +@FunctionalInterface +interface PluginDepChecker { + + /** @return true if a Bukkit plugin with this name is loaded and enabled. */ + boolean isPresent(String pluginName); + + /** Test-mode checker: every name reports as present. No Bukkit calls. */ + PluginDepChecker ALL_PRESENT = name -> true; +} From 245c0990f6f86b7cf84d9e781e28794f143423bb Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 12:55:58 +0500 Subject: [PATCH 08/16] fix: P2 batch B - security defence-in-depth (P2-1 / P2-2 / P2-3 / P2-4 / P2-5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P2-1 - cap addon.conf reads at 64 KB (a real addon.conf is well under 1 KB). readBoundedAddonConf checks both the declared uncompressed size in the zip header AND the actual byte count during read, so a crafted jar can't lie about either to OOM the server. Applied at both readAddonJar (production load path) and scanJarConfs (tab completion + findJarByName paths). P2-2 - add net.minecraft.* and com.mojang.* to AddonClassLoader PARENT_FIRST_PREFIXES. Paperweight-userdev exposes remapped NMS internals to the plugin classloader; without parent-first an addon could ship its own copy of, say, net.minecraft.nbt.CompoundTag and have it hide the real one for any code that crosses the addon boundary, causing ClassCastException-on-cross-loader-cast or, worse, silent class substitution. P2-3 - Colors.init is now set-once. Subsequent calls (whether from a malicious addon trying to flip the RGB mode, or from /reload running onEnable a second time) are silently skipped. The skip-rather-than- throw keeps /reload working; trade-off is that flipping useMiniMessage in config requires a server restart. P2-3 (cont) - same skip-if-set treatment applied to Logger.set, which the previous round had thrown IllegalStateException on second call. The throw broke /reload because Bukkit calls onEnable again without re-creating the static field. P2-4 + P2-5 - CommandAddons now strips legacy &x, section-sign §x, and hex <#RRGGBB> tokens from every addon-supplied string before rendering it through Colors.of into operator chat. Applied to: addon names, versions, descriptions, authors, dep lists, target API versions, exception messages, and the operator-typed name argument echoed back in error responses. Threat: a malicious addon's addon.conf name 'CRITICAL ERROR' formatted in red, or an exception message reading 'OK, password=XYZ' in green, could social-engineer the operator. Operators install third-party addons (semi-trusted) - the bar is not full sandbox but they should not be able to forge legitimate-looking server messages through normal /am addons output. --- .../java/ru/abstractmenus/api/Logger.java | 17 ++-- .../ru/abstractmenus/api/text/Colors.java | 7 ++ .../abstractmenus/addon/AddonClassLoader.java | 7 ++ .../ru/abstractmenus/addon/AddonManager.java | 39 ++++++++-- .../commands/am/CommandAddons.java | 78 ++++++++++++------- 5 files changed, 106 insertions(+), 42 deletions(-) diff --git a/api/src/main/java/ru/abstractmenus/api/Logger.java b/api/src/main/java/ru/abstractmenus/api/Logger.java index 7bcf6fe..61d1d74 100644 --- a/api/src/main/java/ru/abstractmenus/api/Logger.java +++ b/api/src/main/java/ru/abstractmenus/api/Logger.java @@ -37,19 +37,20 @@ private Logger(){} /** * Install the backing JUL logger. Called once by AbstractMenus core during - * plugin {@code onEnable}; throws on any subsequent call so an addon cannot - * replace the logger to silence or capture core's output. + * plugin {@code onEnable}. + * + *

Set-once: subsequent calls are silently skipped so an addon cannot + * replace the logger to silence or capture core's output. The skip + * (rather than throw) keeps Bukkit {@code /reload} working - on reload + * core's {@code onEnable} runs again and re-invokes this method, which + * is now a no-op. * * @param log the JUL logger to delegate to; never {@code null} - * @throws IllegalStateException if a logger has already been installed - * @throws NullPointerException if {@code log} is null + * @throws NullPointerException if {@code log} is null */ public static void set(java.util.logging.Logger log){ if (log == null) throw new NullPointerException("logger"); - if (logger != null) { - throw new IllegalStateException( - "Logger already installed; AbstractMenus does not support replacement"); - } + if (logger != null) return; logger = log; } diff --git a/api/src/main/java/ru/abstractmenus/api/text/Colors.java b/api/src/main/java/ru/abstractmenus/api/text/Colors.java index c711f4e..83d5a87 100644 --- a/api/src/main/java/ru/abstractmenus/api/text/Colors.java +++ b/api/src/main/java/ru/abstractmenus/api/text/Colors.java @@ -82,8 +82,15 @@ public class Colors { * {@code net.md_5.bungee.api.ChatColor} and reflectively looking up the * {@code of(String)} method. Any {@link Throwable} is silently caught, * treating the server as legacy-only. + * + * Set-once: subsequent calls are silently skipped so an addon cannot + * flip the global RGB-handling mode. The skip (rather than throw) + * keeps Bukkit {@code /reload} working; the trade-off is that a + * config flip of {@code useMiniMessage} doesn't take effect until + * a server restart. */ public static void init(boolean replaceRgb) { + if (replacer != null) return; if (isSupportRgb() && replaceRgb) { replacer = new RgbReplacer(); } else { diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonClassLoader.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonClassLoader.java index df9a9b9..9d35418 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonClassLoader.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonClassLoader.java @@ -24,6 +24,13 @@ public final class AddonClassLoader extends URLClassLoader { "io.papermc.", "com.destroystokyo.paper.", "net.kyori.adventure.", + // Paperweight-userdev exposes remapped NMS / Mojang internals to + // the plugin classloader. Keep them parent-first so an addon + // cannot ship its own copy of, say, net.minecraft.nbt.CompoundTag + // and have it hide the real one for any code that crosses the + // addon boundary. + "net.minecraft.", + "com.mojang.", "java.", "javax.", "jdk.", diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java index 1111f34..758d8c9 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java @@ -40,6 +40,15 @@ public final class AddonManager { */ private static final long AVAILABLE_CACHE_TTL_MS = 2_000L; + /** + * Hard cap on the size of {@code addon.conf} read from a jar. A real + * addon.conf is well under a kilobyte; refusing to read more than 64 KB + * defends against a malicious or corrupted jar declaring a huge + * uncompressed size that would OOM the server when {@code readAllBytes} + * tries to allocate the buffer. + */ + private static final int MAX_ADDON_CONF_BYTES = 64 * 1024; + private final Path addonsDir; private final AbstractMenusApi api; private final PluginDepChecker depChecker; @@ -430,15 +439,35 @@ private Map scanJarConfs() { try (var jf = new JarFile(jar.toFile())) { JarEntry entry = jf.getJarEntry("addon.conf"); if (entry == null) continue; - String hocon = new String(jf.getInputStream(entry).readAllBytes(), - StandardCharsets.UTF_8); - result.put(jar, AddonConf.parse(hocon)); + result.put(jar, AddonConf.parse(readBoundedAddonConf(jf, entry))); } catch (Exception ignored) {} } } catch (Exception ignored) {} return result; } + /** + * Read {@code addon.conf} into a String, refusing entries larger than + * {@link #MAX_ADDON_CONF_BYTES}. Both the declared uncompressed size + * (zip header) and the actual byte count are checked so a crafted + * jar can't lie about either one. + */ + private static String readBoundedAddonConf(JarFile jar, JarEntry entry) throws IOException { + long declared = entry.getSize(); + if (declared > MAX_ADDON_CONF_BYTES) { + throw new IOException("addon.conf too large (" + declared + " bytes, cap " + + MAX_ADDON_CONF_BYTES + ")"); + } + try (var in = jar.getInputStream(entry)) { + byte[] bytes = in.readNBytes(MAX_ADDON_CONF_BYTES + 1); + if (bytes.length > MAX_ADDON_CONF_BYTES) { + throw new IOException("addon.conf exceeded " + MAX_ADDON_CONF_BYTES + + " bytes during read (declared size " + declared + ")"); + } + return new String(bytes, StandardCharsets.UTF_8); + } + } + /** * Verify hard {@code pluginDependencies} are present and log warnings * for any missing {@code pluginSoftDependencies}. @@ -527,9 +556,7 @@ private LoadedAddon readAddonJar(Path jarPath) throws IOException { if (entry == null) { throw new IOException("no addon.conf at jar root"); } - try (var in = jar.getInputStream(entry)) { - hocon = new String(in.readAllBytes(), StandardCharsets.UTF_8); - } + hocon = readBoundedAddonConf(jar, entry); } AddonConf conf = AddonConf.parse(hocon); diff --git a/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java b/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java index 54e4f57..c6e5ad4 100644 --- a/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java +++ b/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java @@ -11,6 +11,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.regex.Pattern; /** {@code /am addons [list|reload |info |load |rescan]} */ public class CommandAddons extends Command { @@ -18,6 +19,27 @@ public class CommandAddons extends Command { private static final List SUBCOMMANDS = List.of("list", "reload", "info", "load", "rescan"); + /** + * Strips legacy {@code &x}, section-sign {@code §x}, and hex + * {@code <#RRGGBB>} formatting tokens from a string. Applied to every + * addon-supplied value (names, versions, exception messages, etc.) + * before it is rendered through {@link Colors#of} into operator chat. + * + *

Without this, a malicious addon could put {@code "&aOK, password=XYZ"} + * in its addon.conf name or in an exception message and have it render + * as a green "legitimate-looking" line in the operator's console-out + * mirror. Threat model is operator-installed third-party addons - they + * are semi-trusted but should not be able to social-engineer the + * operator via formatted output. + */ + private static final Pattern UNSAFE_FORMAT = Pattern.compile( + "&[0-9a-fk-orA-FK-OR]|§[0-9a-fk-orA-FK-OR]|<#[0-9a-fA-F]{6}>"); + + private static String safe(String untrusted) { + if (untrusted == null) return ""; + return UNSAFE_FORMAT.matcher(untrusted).replaceAll(""); + } + public CommandAddons() { setUsage( Colors.of("&7/am addons list &e- list all AM-loaded addons"), @@ -42,12 +64,12 @@ public void execute(CommandSender sender, String[] args) { } switch (args[0].toLowerCase()) { - case "list" -> list(sender, am); - case "reload" -> reload(sender, am, args); - case "info" -> info(sender, am, args); - case "load" -> load(sender, am, args); - case "rescan" -> rescan(sender, am); - default -> sender.sendMessage(getUsage()); + case "list" -> list(sender, am); + case "reload" -> reload(sender, am, args); + case "info" -> info(sender, am, args); + case "load" -> load(sender, am, args); + case "rescan" -> rescan(sender, am); + default -> sender.sendMessage(getUsage()); } } @@ -60,13 +82,13 @@ private void list(CommandSender sender, AddonManager am) { sender.sendMessage(Colors.of("&e&lAddons (" + addons.size() + "):")); for (LoadedAddon la : addons) { String color = switch (la.getStatus()) { - case ENABLED -> "&a"; + case ENABLED -> "&a"; case DISABLED -> "&7"; - case FAILED -> "&c"; - case PENDING -> "&e"; + case FAILED -> "&c"; + case PENDING -> "&e"; }; - sender.sendMessage(Colors.of(color + " " + la.getConf().name() - + " &8v" + la.getConf().version() + sender.sendMessage(Colors.of(color + " " + safe(la.getConf().name()) + + " &8v" + safe(la.getConf().version()) + " &7[" + la.getStatus() + "]")); } } @@ -79,15 +101,15 @@ private void reload(CommandSender sender, AddonManager am, String[] args) { String name = args[1]; var result = am.reload(name); if (result.isEmpty()) { - sender.sendMessage(Colors.of("&cAddon '" + name + "' not found or no jar present.")); + sender.sendMessage(Colors.of("&cAddon '" + safe(name) + "' not found or no jar present.")); return; } LoadedAddon la = result.get(); if (la.getStatus() == AddonStatus.ENABLED) { - sender.sendMessage(Colors.of("&aReloaded " + la.getConf().name() + ".")); + sender.sendMessage(Colors.of("&aReloaded " + safe(la.getConf().name()) + ".")); } else { sender.sendMessage(Colors.of("&cReload failed: " - + (la.getError() == null ? "unknown error" : la.getError().getMessage()))); + + (la.getError() == null ? "unknown error" : safe(la.getError().getMessage())))); } } @@ -98,32 +120,32 @@ private void info(CommandSender sender, AddonManager am, String[] args) { } var opt = am.get(args[1]); if (opt.isEmpty()) { - sender.sendMessage(Colors.of("&cAddon '" + args[1] + "' not found.")); + sender.sendMessage(Colors.of("&cAddon '" + safe(args[1]) + "' not found.")); return; } LoadedAddon la = opt.get(); var c = la.getConf(); - sender.sendMessage(Colors.of("&e&l" + c.name() + " &7v" + c.version())); + sender.sendMessage(Colors.of("&e&l" + safe(c.name()) + " &7v" + safe(c.version()))); sender.sendMessage(Colors.of("&7 status: &f" + la.getStatus())); if (!c.authors().isEmpty()) { - sender.sendMessage(Colors.of("&7 authors: &f" + String.join(", ", c.authors()))); + sender.sendMessage(Colors.of("&7 authors: &f" + safe(String.join(", ", c.authors())))); } if (!c.description().isEmpty()) { - sender.sendMessage(Colors.of("&7 description: &f" + c.description())); + sender.sendMessage(Colors.of("&7 description: &f" + safe(c.description()))); } if (c.targetApiVersion() != null) { - sender.sendMessage(Colors.of("&7 targetApiVersion: &f" + c.targetApiVersion())); + sender.sendMessage(Colors.of("&7 targetApiVersion: &f" + safe(c.targetApiVersion()))); } if (!c.addonDependencies().isEmpty()) { sender.sendMessage(Colors.of("&7 addonDependencies: &f" - + String.join(", ", c.addonDependencies()))); + + safe(String.join(", ", c.addonDependencies())))); } if (!c.pluginDependencies().isEmpty()) { sender.sendMessage(Colors.of("&7 pluginDependencies: &f" - + String.join(", ", c.pluginDependencies()))); + + safe(String.join(", ", c.pluginDependencies())))); } if (la.getStatus() == AddonStatus.FAILED && la.getError() != null) { - sender.sendMessage(Colors.of("&7 error: &c" + la.getError().getMessage())); + sender.sendMessage(Colors.of("&7 error: &c" + safe(la.getError().getMessage()))); } } @@ -135,17 +157,17 @@ private void load(CommandSender sender, AddonManager am, String[] args) { String name = args[1]; var result = am.loadOne(name); if (result.isEmpty()) { - sender.sendMessage(Colors.of("&cNo unloaded addon named '" + name + "' found in addons/. " + sender.sendMessage(Colors.of("&cNo unloaded addon named '" + safe(name) + "' found in addons/. " + "Check the jar is in plugins/AbstractMenus/addons/ and addon.conf names it correctly.")); return; } LoadedAddon la = result.get(); if (la.getStatus() == AddonStatus.ENABLED) { - sender.sendMessage(Colors.of("&aLoaded " + la.getConf().name() - + " v" + la.getConf().version() + ".")); + sender.sendMessage(Colors.of("&aLoaded " + safe(la.getConf().name()) + + " v" + safe(la.getConf().version()) + ".")); } else { sender.sendMessage(Colors.of("&cLoad failed: " - + (la.getError() == null ? "unknown error" : la.getError().getMessage()))); + + (la.getError() == null ? "unknown error" : safe(la.getError().getMessage())))); } } @@ -162,8 +184,8 @@ private void rescan(CommandSender sender, AddonManager am) { + (failed > 0 ? ", &c" + failed + " failed" : "") + "&a.")); for (LoadedAddon la : newlyLoaded) { String color = la.getStatus() == AddonStatus.ENABLED ? "&a" : "&c"; - sender.sendMessage(Colors.of(color + " " + la.getConf().name() - + " &8v" + la.getConf().version() + sender.sendMessage(Colors.of(color + " " + safe(la.getConf().name()) + + " &8v" + safe(la.getConf().version()) + " &7[" + la.getStatus() + "]")); } } From ab55c38ab83ca5181941734d3fb8067be8926585 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 13:00:05 +0500 Subject: [PATCH 09/16] build(plugin): drop bundled Adventure - rely on Paper's bundled copy Paper bundles net.kyori.adventure (api / text-minimessage / serializer-legacy / examination / option) directly via paper-api. Our shadow jar was duplicating ~2.7 MB of those same classes for no benefit; on Paper, the classloader picks the plugin's bundled copy first which is wasteful and in some cases collides with internal Paper utilities. Changes: - Removed adventure-platform-bukkit:4.4.1 entirely. Verified zero references in plugin/api source (no BukkitAudiences, no adventure-platform imports anywhere). - Switched adventure-text-minimessage:4.26.1 to compileOnly. Compile resolution still works because paper-api's BOM exposes it; runtime uses Paper's bundled copy. Result: AbstractMenus-2.0.0-alpha.2.jar shrinks from 2.2 MB to 1.1 MB (-50%). No code changes needed, all 248 tests still pass. --- plugin/build.gradle | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/plugin/build.gradle b/plugin/build.gradle index 9092520..ef92f88 100644 --- a/plugin/build.gradle +++ b/plugin/build.gradle @@ -45,8 +45,15 @@ dependencies { implementation project(':api') implementation 'com.fathzer:javaluator:3.0.6' - implementation 'net.kyori:adventure-platform-bukkit:4.4.1' - implementation 'net.kyori:adventure-text-minimessage:4.26.1' + + // Adventure (Component, MiniMessage, LegacyComponentSerializer) is + // bundled INSIDE Paper itself (via paper-api's BOM). Pull it in + // compileOnly so we get the types at compile time but do not bundle + // ~2.7 MB of duplicate Adventure classes into the shadow jar. + // adventure-platform-bukkit was used by nothing in our code so it is + // dropped entirely. + compileOnly 'net.kyori:adventure-text-minimessage:4.26.1' + implementation "com.github.technicallycoded:FoliaLib:0.4.4" shadow files('libs/MMOItems-6.5.4.jar') From e4da28969532b3eb7c8b13d3114a202c1b79207d Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 13:06:28 +0500 Subject: [PATCH 10/16] fix: P2 batch C - money/provider polish (P2-6 / P2-13 / P2-16) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P2-6 - extracted MoneyAmountSpec.parse(ConfigNode) shared helper. The scalar-or-map deserialization plus provider validation block was duplicated verbatim across ActionMoneyTake.Serializer, ActionMoneyGive.Serializer, and RuleMoney.Serializer. ~18 lines × 3. Each serializer is now a 2-line delegation. P2-13 - ProviderSection.resolve() and resolve(id) javadocs already state @return null on empty / missing. Verified the contract is documented; no doc change needed beyond existing. P2-16 - provider error message now lists registered IDs ("vault", "playerpoints") instead of impl class names ("EconomyVaultHandler"). Added ProviderSection.ids() returning a Set snapshot of registered ids; the money parse error builds its 'Registered: [...]' list from that. Operators can now copy from the error message straight into their config.conf without a mental class-name-to-id translation step. --- .../ru/abstractmenus/api/ProviderSection.java | 11 ++++ .../data/actions/ActionMoneyGive.java | 26 +------- .../data/actions/ActionMoneyTake.java | 26 +------- .../data/actions/MoneyAmountSpec.java | 61 +++++++++++++++++++ .../abstractmenus/data/rules/RuleMoney.java | 27 +------- .../impl/ProviderRegistryImpl.java | 6 ++ 6 files changed, 85 insertions(+), 72 deletions(-) create mode 100644 plugin/src/main/java/ru/abstractmenus/data/actions/MoneyAmountSpec.java diff --git a/api/src/main/java/ru/abstractmenus/api/ProviderSection.java b/api/src/main/java/ru/abstractmenus/api/ProviderSection.java index 840e93d..900b5ae 100644 --- a/api/src/main/java/ru/abstractmenus/api/ProviderSection.java +++ b/api/src/main/java/ru/abstractmenus/api/ProviderSection.java @@ -1,6 +1,7 @@ package ru.abstractmenus.api; import java.util.Collection; +import java.util.Set; /** * One section of the {@link ProviderRegistry}, holding registered handlers @@ -87,6 +88,16 @@ public interface ProviderSection { /** All registered handlers, in registration order. Read-only snapshot. */ Collection all(); + /** + * All registered ids, lowercased and in registration order. Useful for + * "did you mean" or "unknown id, registered: [...]" error messages + * where you want to surface the actual configurable names rather than + * impl class names. + * + * @return read-only snapshot of registered ids + */ + Set ids(); + /** * @param id case-insensitive identifier * @return whether a handler with this id is registered diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyGive.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyGive.java index 6cd9887..f545ba3 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyGive.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyGive.java @@ -37,30 +37,8 @@ public static class Serializer implements NodeSerializer { @Override public ActionMoneyGive deserialize(Class type, ConfigNode node) throws NodeSerializeException { - TypeDouble money; - String provider = null; - - if (node.isMap()) { - money = node.node("amount").getValue(TypeDouble.class); - provider = node.node("provider").getString(null); - } else { - money = node.getValue(TypeDouble.class); - } - - if (provider != null) { - if (!AbstractMenusApi.get().providers().economy().has(provider)) { - StringBuilder known = new StringBuilder(); - for (EconomyHandler h : AbstractMenusApi.get().providers().economy().all()) { - if (known.length() > 0) known.append(", "); - known.append(h.getClass().getSimpleName()); - } - throw new NodeSerializeException(node, - "Unknown economy provider '" + provider + "'. Registered: [" - + known + "]. Omit the 'provider' field for default selection."); - } - } - - return new ActionMoneyGive(money, provider); + MoneyAmountSpec spec = MoneyAmountSpec.parse(node); + return new ActionMoneyGive(spec.amount, spec.provider); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyTake.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyTake.java index 4349d37..ce4ecc1 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyTake.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMoneyTake.java @@ -37,30 +37,8 @@ public static class Serializer implements NodeSerializer { @Override public ActionMoneyTake deserialize(Class type, ConfigNode node) throws NodeSerializeException { - TypeDouble money; - String provider = null; - - if (node.isMap()) { - money = node.node("amount").getValue(TypeDouble.class); - provider = node.node("provider").getString(null); - } else { - money = node.getValue(TypeDouble.class); - } - - if (provider != null) { - if (!AbstractMenusApi.get().providers().economy().has(provider)) { - StringBuilder known = new StringBuilder(); - for (EconomyHandler h : AbstractMenusApi.get().providers().economy().all()) { - if (known.length() > 0) known.append(", "); - known.append(h.getClass().getSimpleName()); - } - throw new NodeSerializeException(node, - "Unknown economy provider '" + provider + "'. Registered: [" - + known + "]. Omit the 'provider' field for default selection."); - } - } - - return new ActionMoneyTake(money, provider); + MoneyAmountSpec spec = MoneyAmountSpec.parse(node); + return new ActionMoneyTake(spec.amount, spec.provider); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/MoneyAmountSpec.java b/plugin/src/main/java/ru/abstractmenus/data/actions/MoneyAmountSpec.java new file mode 100644 index 0000000..aaa7feb --- /dev/null +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/MoneyAmountSpec.java @@ -0,0 +1,61 @@ +package ru.abstractmenus.data.actions; + +import ru.abstractmenus.api.AbstractMenusApi; +import ru.abstractmenus.datatype.TypeDouble; +import ru.abstractmenus.hocon.api.ConfigNode; +import ru.abstractmenus.hocon.api.serialize.NodeSerializeException; + +/** + * Shared deserializer for HOCON shapes used by money-aware actions and + * rules ({@code takeMoney}, {@code giveMoney}, {@code hasMoney}). + * + *

Accepts both forms: + * + * 

{@code
+ * takeMoney: 100                                    # scalar - default provider
+ * takeMoney { amount: 100, provider: "vault" }      # map - explicit provider
+ * }
+ * + *

Validates the optional {@code provider} field at parse time so menus + * fail fast if they reference an unregistered economy provider, with an + * error that lists the actually-registered ids ({@code "vault"}, + * {@code "playerpoints"}, etc.) instead of impl class names. Three + * call sites (TakeMoney, GiveMoney, RuleMoney) used to duplicate this + * 18-line block; consolidated here. + */ +public final class MoneyAmountSpec { + + public final TypeDouble amount; + public final String provider; + + private MoneyAmountSpec(TypeDouble amount, String provider) { + this.amount = amount; + this.provider = provider; + } + + /** + * Parse a {@code TypeDouble amount} plus optional {@code provider} from + * either the scalar form or the map form. Throws on unknown provider + * with a helpful message. + */ + public static MoneyAmountSpec parse(ConfigNode node) throws NodeSerializeException { + TypeDouble amount; + String provider = null; + + if (node.isMap()) { + amount = node.node("amount").getValue(TypeDouble.class); + provider = node.node("provider").getString(null); + } else { + amount = node.getValue(TypeDouble.class); + } + + if (provider != null && !AbstractMenusApi.get().providers().economy().has(provider)) { + String known = String.join(", ", AbstractMenusApi.get().providers().economy().ids()); + throw new NodeSerializeException(node, + "Unknown economy provider '" + provider + "'. Registered: [" + + known + "]. Omit the 'provider' field for default selection."); + } + + return new MoneyAmountSpec(amount, provider); + } +} diff --git a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleMoney.java b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleMoney.java index 03c4ff4..2278247 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/rules/RuleMoney.java +++ b/plugin/src/main/java/ru/abstractmenus/data/rules/RuleMoney.java @@ -9,6 +9,7 @@ import ru.abstractmenus.api.inventory.Menu; import ru.abstractmenus.api.inventory.Item; import ru.abstractmenus.api.AbstractMenusApi; +import ru.abstractmenus.data.actions.MoneyAmountSpec; import ru.abstractmenus.datatype.TypeDouble; public class RuleMoney implements Rule { @@ -36,30 +37,8 @@ public static class Serializer implements NodeSerializer { @Override public RuleMoney deserialize(Class type, ConfigNode node) throws NodeSerializeException { - TypeDouble money; - String provider = null; - - if (node.isMap()) { - money = node.node("amount").getValue(TypeDouble.class); - provider = node.node("provider").getString(null); - } else { - money = node.getValue(TypeDouble.class); - } - - if (provider != null) { - if (!AbstractMenusApi.get().providers().economy().has(provider)) { - StringBuilder known = new StringBuilder(); - for (EconomyHandler h : AbstractMenusApi.get().providers().economy().all()) { - if (known.length() > 0) known.append(", "); - known.append(h.getClass().getSimpleName()); - } - throw new NodeSerializeException(node, - "Unknown economy provider '" + provider + "'. Registered: [" - + known + "]. Omit the 'provider' field for default selection."); - } - } - - return new RuleMoney(money, provider); + MoneyAmountSpec spec = MoneyAmountSpec.parse(node); + return new RuleMoney(spec.amount, spec.provider); } } diff --git a/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java index 89bfb81..568aaab 100644 --- a/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java +++ b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java @@ -140,6 +140,12 @@ public synchronized Collection all() { return Collections.unmodifiableList(list); } + @Override + public synchronized Set ids() { + // Snapshot - byId keys may mutate after this returns. + return Collections.unmodifiableSet(new java.util.LinkedHashSet<>(byId.keySet())); + } + @Override public synchronized boolean has(String id) { return byId.containsKey(id.toLowerCase()); From bffc7b072f5bd4e6801ffdd6cbd59e7306862fc4 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 13:16:21 +0500 Subject: [PATCH 11/16] fix: P2 batch D - misc tactical cleanup (P2-10 / P2-11 / P2-12 / P2-14 / P2-19) P2-10 - drop the dead try/catch around CoreActionsBundle's customSound registration. The 'SoundCategory missing on legacy Bukkit' comment referred to pre-1.9 servers; we target Paper 1.21.11. The catch was also broad (Throwable) and would have hidden any real registration error in ActionSoundCustom.Serializer construction. P2-11 - CoreExtension.version() previously returned null with a 'populated at runtime via api.apiVersion()' comment but no code actually populated it. /am addons list rendered the core entry as 'AbstractMenus-Core vnull'. Now captured in onEnable from api.apiVersion(). P2-12 - filterByPrefix moved from CommandAddons-private to a protected static helper on the Command base class. Same five-line filter could appear in any future tab-complete override; now they share one definition. P2-14 - swap java.util.Stack (extends Vector, synchronises every op) for java.util.ArrayDeque in Command.onCommand. Single-threaded dispatch path - no benefit from the lock, just GC noise. P2-19 - the WatchService 'main thread' comment in MenuManager was misleading. On Folia, BukkitTasks.runTask routes to the global region scheduler, not 'main thread'. Updated the comment to spell out what the dispatch actually does (and what it's actually for - ordering against /am reload, not thread-safety, since menus is already a ConcurrentHashMap). --- .../ru/abstractmenus/commands/Command.java | 25 +++++++++++++++++-- .../commands/am/CommandAddons.java | 8 ------ .../abstractmenus/core/CoreActionsBundle.java | 6 +---- .../ru/abstractmenus/core/CoreExtension.java | 14 ++++++++--- .../abstractmenus/services/MenuManager.java | 6 ++++- 5 files changed, 40 insertions(+), 19 deletions(-) diff --git a/plugin/src/main/java/ru/abstractmenus/commands/Command.java b/plugin/src/main/java/ru/abstractmenus/commands/Command.java index ce9a88f..f580a39 100644 --- a/plugin/src/main/java/ru/abstractmenus/commands/Command.java +++ b/plugin/src/main/java/ru/abstractmenus/commands/Command.java @@ -9,10 +9,11 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; +import java.util.Deque; +import java.util.ArrayDeque; import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Stack; public abstract class Command implements CommandExecutor, TabCompleter { @@ -53,7 +54,9 @@ public boolean onCommand(@NotNull CommandSender sender, @NotNull org.bukkit.comm if (!checkPermission(sender, this)) return false; if (args.length > 0) { - Stack stack = new Stack<>(); + // ArrayDeque (not Stack — Stack extends Vector and synchronises + // every op for no benefit on a single-threaded dispatch path). + Deque stack = new ArrayDeque<>(); Command sub; for (String arg : args) { @@ -137,4 +140,22 @@ public List tabComplete(CommandSender sender, String[] args) { return sub.tabComplete(sender, Arrays.copyOfRange(args, 1, args.length)); } + /** + * Helper for subclasses overriding {@link #tabComplete}: filter + * {@code candidates} to entries whose lowercase form starts with + * {@code prefix.toLowerCase()}, and return them sorted alphabetically. + * + *

Pulled up to the base because at least three subcommand + * implementations (CommandAddons being the largest) used to ship a + * private static copy of this same five-line filter. + */ + protected static List filterByPrefix(Iterable candidates, String prefix) { + String lower = prefix.toLowerCase(); + List result = new ArrayList<>(); + for (String s : candidates) { + if (s.toLowerCase().startsWith(lower)) result.add(s); + } + Collections.sort(result); + return result; + } } diff --git a/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java b/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java index c6e5ad4..d663871 100644 --- a/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java +++ b/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java @@ -219,12 +219,4 @@ public List tabComplete(CommandSender sender, String[] args) { return Collections.emptyList(); } - private static List filterByPrefix(List source, String prefix) { - List result = new ArrayList<>(); - for (String s : source) { - if (s.toLowerCase().startsWith(prefix)) result.add(s); - } - Collections.sort(result); - return result; - } } diff --git a/plugin/src/main/java/ru/abstractmenus/core/CoreActionsBundle.java b/plugin/src/main/java/ru/abstractmenus/core/CoreActionsBundle.java index f604a43..ea5328f 100644 --- a/plugin/src/main/java/ru/abstractmenus/core/CoreActionsBundle.java +++ b/plugin/src/main/java/ru/abstractmenus/core/CoreActionsBundle.java @@ -95,11 +95,7 @@ void register(AbstractMenusApi api, MenuExtension owner) { api.actions().register("setHealth", ActionHealthSet.class, new ActionHealthSet.Serializer(), owner); api.actions().register("sound", ActionSound.class, new ActionSound.Serializer(), owner); - try { - // SoundCategory missing on legacy Bukkit - api.actions().register("customSound", ActionSoundCustom.class, new ActionSoundCustom.Serializer(), owner); - } catch (Throwable ignore) { - } + api.actions().register("customSound", ActionSoundCustom.class, new ActionSoundCustom.Serializer(), owner); api.actions().register("takeLevel", ActionLevelTake.class, new ActionLevelTake.Serializer(), owner); api.actions().register("takeMoney", ActionMoneyTake.class, new ActionMoneyTake.Serializer(), owner); diff --git a/plugin/src/main/java/ru/abstractmenus/core/CoreExtension.java b/plugin/src/main/java/ru/abstractmenus/core/CoreExtension.java index 8d5649d..79e83c8 100644 --- a/plugin/src/main/java/ru/abstractmenus/core/CoreExtension.java +++ b/plugin/src/main/java/ru/abstractmenus/core/CoreExtension.java @@ -11,10 +11,18 @@ * *

The five bundles keep registration grouped by surface area for * readability. Each is a pure function of {@code api} and the owning - * {@code CoreExtension} instance — no static state. + * {@code CoreExtension} instance — no static state. */ public final class CoreExtension implements MenuExtension { + /** + * Captured at {@link #onEnable} time from {@link AbstractMenusApi#apiVersion()} + * so {@link #version()} no longer reports {@code null} for the core + * extension. Resolved lazily because at {@link #onLoad} time the API + * is already wired but version-string resolution is cheap regardless. + */ + private String version; + @Override public String name() { return "AbstractMenus-Core"; @@ -22,12 +30,12 @@ public String name() { @Override public String version() { - // Populated from plugin version at runtime via api.apiVersion(). - return null; + return version; } @Override public void onEnable(AbstractMenusApi api) { + this.version = api.apiVersion(); new CoreActionsBundle().register(api, this); new CoreRulesBundle().register(api, this); new CoreItemPropsBundle().register(api, this); diff --git a/plugin/src/main/java/ru/abstractmenus/services/MenuManager.java b/plugin/src/main/java/ru/abstractmenus/services/MenuManager.java index 04b89b1..a53d4e8 100644 --- a/plugin/src/main/java/ru/abstractmenus/services/MenuManager.java +++ b/plugin/src/main/java/ru/abstractmenus/services/MenuManager.java @@ -291,7 +291,11 @@ public boolean serve() throws IOException { if (Files.isRegularFile(file) && System.currentTimeMillis() > lastUpdated + 100) { Logger.info("Detected changes in " + filename + ". Loading ..."); - // Bukkit API / menu map mutation must happen on main thread. + // We are on the WatchService thread - hop back into the + // server scheduler before parsing. On Folia BukkitTasks.runTask + // routes to the global region scheduler (loadFile mutates the + // menus map, which is a ConcurrentHashMap, so the scheduler + // affinity here is about ordering with /am reload, not safety). BukkitTasks.runTask(() -> loadFile(file)); lastUpdated = System.currentTimeMillis(); } From 422026a3368ddcc22caa97437ae055c21895cd17 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 13:23:23 +0500 Subject: [PATCH 12/16] fix: P2 batch E - lifecycle and scheduling (P2-15 / P2-18 / P2-20) P2-15 - addon dependency graph no longer poisons the whole batch when a single addon declares an unknown dep. AddonDependencyGraph.unsatisfied() returns the set of addons whose deps are missing; AddonManager.loadAll now pre-filters those (marking each FAILED individually with a clear 'missing addon dependency: X' message) BEFORE topoSort. topoSort itself is now strict-but-naive (cycle detection only) since all dangling refs are filtered upstream. The most common trigger was an addon depending on a Path 1 plugin-as-addon: that dep is real but lives in plugins/ rather than addons/, so AddonManager never sees it in the graph. Old behaviour: 'Addon X depends on unknown addon Y' kills every other addon in addons/. New behaviour: only X is marked FAILED; the rest enable normally. Test infra - AddonDependencyGraphTest's unknownDependency_throws() was asserting the now-removed throw. Replaced with two focused tests for unsatisfied(): one for missing-dep detection, one for the empty-on-clean case. P2-18 - left disablePlugin() inside onEnable's catch as is, but added a comment explaining the trade-off. The re-entrant disable was the agent finding's concern; P0-3 (null-guard around VariableManagerImpl.shutdown) already mitigated the actual NPE risk. Rethrowing instead would skip ALL our cleanup paths because Bukkit does not call onDisable for plugins whose onEnable threw - resource leaks worse than the cosmetic re-entrancy. P2-20 - similar accept-and-document for determineProxy()'s YAML reads. The files are <10 KB and the call fires once during onEnable before any gameplay can be affected. Comment notes 'acceptable startup-only IO'. --- .../java/ru/abstractmenus/AbstractMenus.java | 14 ++++++- .../addon/AddonDependencyGraph.java | 41 ++++++++++++------- .../ru/abstractmenus/addon/AddonManager.java | 28 +++++++++++-- .../addon/AddonDependencyGraphTest.java | 23 ++++++++--- 4 files changed, 82 insertions(+), 24 deletions(-) diff --git a/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java b/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java index b2aab22..9d57d58 100644 --- a/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java +++ b/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java @@ -175,6 +175,13 @@ public void onEnable() { } catch (Exception e) { Logger.severe("Cannot enable plugin: " + e.getMessage()); e.printStackTrace(); + // disablePlugin() from inside onEnable is re-entrant: Bukkit calls + // our onDisable while we are still in this stack frame. Every + // singleton accessed in onDisable is null-guarded so a partial + // init does not NPE on the way out. Rethrowing instead would + // skip our cleanup (service-manager registrations, listeners, + // database connections) since Bukkit does not call onDisable + // for plugins whose onEnable threw. disablePlugin(); } } @@ -268,7 +275,12 @@ public static AbstractMenus instance() { return instance; } - // from SkinRestorer + // from SkinRestorer. + // + // YamlConfiguration.loadConfiguration below reads spigot.yml / paper.yml + // synchronously on the main thread. These files are <10 KB on a real + // server and the call only fires once, during onEnable, before any + // gameplay can be affected. Acceptable startup-only IO. public boolean determineProxy() { Path spigotFile = Paths.get("spigot.yml"); Path paperFile = Paths.get("paper.yml"); diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonDependencyGraph.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonDependencyGraph.java index e2ae328..5565261 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonDependencyGraph.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonDependencyGraph.java @@ -20,30 +20,43 @@ public final class AddonDependencyGraph { private AddonDependencyGraph() {} /** - * Sort addons into enabling order. + * Find every addon whose declared dependencies include a name that is + * not present in the graph. Useful for pre-filtering before + * {@link #topoSort} so a single bad addon does not poison the whole + * batch. * - * @param dependencies map from addon name → list of names it depends on. - * Iteration order of the input map is preserved - * among independent addons (pass a - * {@link java.util.LinkedHashMap} for determinism). - * @return addon names in dependency-first order (deps before dependants) - * @throws AddonDependencyCycleException if a cycle is detected - * @throws AddonDependencyException if a declared dep refers to a - * name not present in the graph + * @param dependencies graph (same shape as {@link #topoSort}) + * @return set of addon names with at least one missing dependency, in + * iteration order of {@code dependencies} */ - public static List topoSort(Map> dependencies) { + public static Set unsatisfied(Map> dependencies) { Set known = dependencies.keySet(); - - // Validation: every declared dep must exist in the graph. + Set bad = new LinkedHashSet<>(); for (Map.Entry> e : dependencies.entrySet()) { for (String dep : e.getValue()) { if (!known.contains(dep)) { - throw new AddonDependencyException( - "Addon '" + e.getKey() + "' depends on unknown addon '" + dep + "'"); + bad.add(e.getKey()); + break; } } } + return bad; + } + /** + * Sort addons into enabling order. Caller must ensure every dep + * referenced is present in {@code dependencies} - use + * {@link #unsatisfied} first to filter out bad nodes. Cycle detection + * still throws. + * + * @param dependencies map from addon name → list of names it depends on. + * Iteration order of the input map is preserved + * among independent addons (pass a + * {@link java.util.LinkedHashMap} for determinism). + * @return addon names in dependency-first order (deps before dependants) + * @throws AddonDependencyCycleException if a cycle is detected + */ + public static List topoSort(Map> dependencies) { Set permanent = new HashSet<>(); Set temporary = new LinkedHashSet<>(); // order preserved for cycle message List order = new ArrayList<>(); diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java index 758d8c9..ac50b5e 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java @@ -19,6 +19,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -103,18 +104,39 @@ public void loadAll() { if (byName.isEmpty()) return; - // Sort by addon-level dependencies. + // Build the addon-dep graph. Map> depGraph = new LinkedHashMap<>(); for (var e : byName.entrySet()) { List deps = e.getValue().getConf().addonDependencies().stream() .map(String::toLowerCase).toList(); depGraph.put(e.getKey(), deps); } + + // Pre-filter addons whose deps point to names not in the graph - + // typically because the dep is a plugin-as-addon (Path 1) which + // doesn't show up in the addons/ folder, or simply a typo. Mark + // them FAILED individually instead of poisoning the whole batch + // through topoSort. + Set unsatisfied = AddonDependencyGraph.unsatisfied(depGraph); + for (String key : unsatisfied) { + LoadedAddon la = byName.remove(key); + depGraph.remove(key); + String missing = la.getConf().addonDependencies().stream() + .filter(d -> !byName.containsKey(d.toLowerCase())) + .findFirst().orElse("?"); + String msg = "missing addon dependency: " + missing; + Logger.warning("Addon " + la.getConf().name() + " " + msg + " - skipping"); + la.markFailed(new IllegalStateException(msg)); + addons.put(key, la); + } + + if (byName.isEmpty()) return; + List order; try { order = AddonDependencyGraph.topoSort(depGraph); - } catch (AddonDependencyException ex) { - Logger.severe("Addon dependency graph error: " + ex.getMessage()); + } catch (AddonDependencyCycleException ex) { + Logger.severe("Addon dependency cycle detected: " + ex.getMessage()); for (var la : byName.values()) { la.markFailed(ex); addons.put(la.getConf().name().toLowerCase(), la); diff --git a/plugin/src/test/java/ru/abstractmenus/addon/AddonDependencyGraphTest.java b/plugin/src/test/java/ru/abstractmenus/addon/AddonDependencyGraphTest.java index a692054..fceeea4 100644 --- a/plugin/src/test/java/ru/abstractmenus/addon/AddonDependencyGraphTest.java +++ b/plugin/src/test/java/ru/abstractmenus/addon/AddonDependencyGraphTest.java @@ -5,6 +5,7 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Set; import static org.junit.jupiter.api.Assertions.*; @@ -77,11 +78,21 @@ void twoNodeCycle_throws() { } @Test - void unknownDependency_throws() { - Map> deps = Map.of("a", List.of("ghost")); - AddonDependencyException ex = assertThrows( - AddonDependencyException.class, - () -> AddonDependencyGraph.topoSort(deps)); - assertTrue(ex.getMessage().toLowerCase().contains("ghost")); + void unsatisfied_returnsAddonsWithMissingDeps() { + Map> deps = new LinkedHashMap<>(); + deps.put("a", List.of("ghost")); // ghost not in graph + deps.put("b", List.of("a")); // a IS in graph + deps.put("c", List.of()); + + Set bad = AddonDependencyGraph.unsatisfied(deps); + assertEquals(Set.of("a"), bad); + } + + @Test + void unsatisfied_emptyForCleanGraph() { + Map> deps = Map.of( + "a", List.of(), + "b", List.of("a")); + assertTrue(AddonDependencyGraph.unsatisfied(deps).isEmpty()); } } From e44c08240eaff178c3537a8c096402ad50db54c8 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 13:37:00 +0500 Subject: [PATCH 13/16] fix: P3 nits (P3-1 / P3-2 / P3-5 / P3-6) P3-1 - CommandAddons.tabComplete now passes the loaded-names sequence to filterByPrefix as a Supplier-style Iterable instead of materialising a fresh List on every TAB keystroke. Filter still allocates a List for the result, but the source pass is zero-copy. P3-2 - MenuExtension.name() and version() defaults no longer return null. name() falls back to getClass().getSimpleName(), version() to 'unknown'. A Path 1 plugin-as-addon that forgets to override these now shows up as 'PlayerPointsAddon vunknown' in /am addons output instead of 'null vnull'. P3-5 - VariablesDao.init dropped the bogus 'root', '' credentials from its SQLite getConnection call. SQLite ignored them but they read like a MySQL hangover. The two-arg overload is the canonical SQLite shape. P3-6 - AddonConf.parse now regex-validates the 'main' field against a loose Java class-name pattern (letter|underscore|dollar followed by identifier chars and dots). A typo like 'main = my-addon.Main' now fails with a clear 'not a legal Java class name' error during parse, instead of bubbling up as ClassNotFoundException several stages later when AddonManager tries to instantiate. --- .../ru/abstractmenus/api/MenuExtension.java | 26 ++++++++++++------- .../ru/abstractmenus/addon/AddonConf.java | 14 ++++++++++ .../commands/am/CommandAddons.java | 2 +- .../abstractmenus/variables/VariablesDao.java | 5 +++- 4 files changed, 35 insertions(+), 12 deletions(-) diff --git a/api/src/main/java/ru/abstractmenus/api/MenuExtension.java b/api/src/main/java/ru/abstractmenus/api/MenuExtension.java index eb410b4..2af64c6 100644 --- a/api/src/main/java/ru/abstractmenus/api/MenuExtension.java +++ b/api/src/main/java/ru/abstractmenus/api/MenuExtension.java @@ -80,25 +80,31 @@ default void onDisable(AbstractMenusApi api) { } /** - * Display name. Used by {@code /am addons list} and log messages. May - * return {@code null}; for AM-loaded addons (Path 2) the name from - * {@code addon.conf} is used as a fallback. + * Display name. Used by {@code /am addons list} and log messages. * - * @return the extension display name, or {@code null} to defer + *

Default falls back to {@code getClass().getSimpleName()} so a + * Path 1 plugin-as-addon that forgets to override does not render + * as "null" in operator output. Path 2 implementations should + * override this to match the {@code name} field in {@code addon.conf}. + * + * @return the extension display name; never {@code null} by default */ default String name() { - return null; + return getClass().getSimpleName(); } /** - * Extension version string. Used for diagnostics. May return {@code null} - * — Path 2 falls back to the {@code version} from - * {@code addon.conf}. + * Extension version string. Used for diagnostics. * - * @return the version, or {@code null} to defer + *

Default returns {@code "unknown"} so {@code /am addons list} + * does not render "vnull". Path 1 plugins should override to return + * their plugin.yml version; Path 2 implementations should match the + * {@code version} field in {@code addon.conf}. + * + * @return the version; never {@code null} by default */ default String version() { - return null; + return "unknown"; } /** diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonConf.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonConf.java index f6ffc1e..519c554 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonConf.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonConf.java @@ -7,6 +7,7 @@ import java.io.ByteArrayInputStream; import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.regex.Pattern; /** * Immutable metadata parsed from an addon's {@code addon.conf} file. Use @@ -27,6 +28,15 @@ public record AddonConf( List pluginSoftDependencies ) { + /** + * Loose Java class-name pattern: identifier chars + dots, must start + * with a letter / underscore / dollar. Catches obvious typos in + * addon.conf's {@code main} field with a clear error before the + * cleaner-but-deeper {@code ClassNotFoundException} from loadClass. + */ + private static final Pattern CLASS_NAME = + Pattern.compile("[A-Za-z_$][A-Za-z0-9_$]*(\\.[A-Za-z_$][A-Za-z0-9_$]*)*"); + /** * Parse HOCON text into an AddonConf. * @@ -51,6 +61,10 @@ public static AddonConf parse(String hocon) { String name = requireString(root, "name"); String version = requireString(root, "version"); String main = requireString(root, "main"); + if (!CLASS_NAME.matcher(main).matches()) { + throw new AddonConfParseException( + "Field 'main' is not a legal Java class name: '" + main + "'"); + } List authors = optionalStringList(root, "authors"); String description = root.node("description").getString(""); diff --git a/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java b/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java index d663871..1b18f79 100644 --- a/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java +++ b/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java @@ -209,7 +209,7 @@ public List tabComplete(CommandSender sender, String[] args) { String prefix = args[1].toLowerCase(); return switch (args[0].toLowerCase()) { case "reload", "info" -> filterByPrefix( - am.loaded().stream().map(la -> la.getConf().name()).toList(), + () -> am.loaded().stream().map(la -> la.getConf().name()).iterator(), prefix); case "load" -> filterByPrefix(am.availableNotLoaded(), prefix); default -> Collections.emptyList(); diff --git a/plugin/src/main/java/ru/abstractmenus/variables/VariablesDao.java b/plugin/src/main/java/ru/abstractmenus/variables/VariablesDao.java index 540f996..ad70912 100644 --- a/plugin/src/main/java/ru/abstractmenus/variables/VariablesDao.java +++ b/plugin/src/main/java/ru/abstractmenus/variables/VariablesDao.java @@ -104,7 +104,10 @@ void init(Path folder) { } try { - connection = DriverManager.getConnection("jdbc:sqlite:" + path, "root", ""); + // SQLite is a serverless single-file database - no credentials. + // The earlier "root", "" args were a copy-paste leftover from + // a MySQL setup; SQLite ignored them but they read confusingly. + connection = DriverManager.getConnection("jdbc:sqlite:" + path); String sql = FileUtils.getResourceAsString("/variables.sql"); execute(sql); } catch (SQLException e) { From 2eda19da9c00e6ee9d3e5ce4fc822160f6809269 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 14:19:43 +0500 Subject: [PATCH 14/16] fix: round-4 P0+P1 from re-review (Folia + addon lifecycle) P0-1: 4 actions still routed entity work through the global region scheduler (ActionDelay, ActionMenuClose/Refresh/ItemRefresh on the ticks branch). Switched to runForEntityLater so the deferred close / refresh / chained activate runs on the player's region thread on Folia. P0-2: AddonDependencyGraph.unsatisfied() did a single pass, so a chain like A -> B -> missing C only flagged B and let A poison topoSort or onEnable. Now runs to fixed point and flags every node in the bad-closure. P1: TypeRegistryImpl.register() on overwrite kept the old owner's entry in keysByOwner, so the old owner's later unregisterAll wiped the new owner's registration. Strip k from every owner-set on overwrite. P1: Stage 1 onLoad failure now calls rollbackRegistrations - if a misbehaving addon registered a type before throwing, that type shouldn't bleed into the next addon's enable. P1: PlayerQuitEvent now removes the tracked menu directly. The 3-tick deferred close in InventoryListener is silently dropped on Folia if the player quits inside the window, which leaked entries in openedMenus. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../addon/AddonDependencyGraph.java | 33 ++++++++++++------- .../ru/abstractmenus/addon/AddonManager.java | 12 +++++-- .../data/actions/ActionItemRefresh.java | 6 ++-- .../data/actions/ActionMenuClose.java | 4 +-- .../data/actions/ActionMenuRefresh.java | 4 ++- .../data/actions/wrappers/ActionDelay.java | 4 ++- .../abstractmenus/impl/TypeRegistryImpl.java | 10 ++++-- .../listeners/PlayerListener.java | 6 ++++ .../addon/AddonDependencyGraphTest.java | 19 +++++++++-- 9 files changed, 72 insertions(+), 26 deletions(-) diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonDependencyGraph.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonDependencyGraph.java index 5565261..087e4a3 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonDependencyGraph.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonDependencyGraph.java @@ -21,25 +21,34 @@ private AddonDependencyGraph() {} /** * Find every addon whose declared dependencies include a name that is - * not present in the graph. Useful for pre-filtering before - * {@link #topoSort} so a single bad addon does not poison the whole - * batch. + * not present (or transitively unsatisfied) in the graph. Useful for + * pre-filtering before {@link #topoSort} so a single bad addon does + * not poison the whole batch. + * + *

Runs to a fixed point: if A depends on B and B depends on missing + * C, both A and B are reported. A single pass would only catch B, + * leaving A to fail later inside {@code topoSort} or {@code onEnable}. * * @param dependencies graph (same shape as {@link #topoSort}) - * @return set of addon names with at least one missing dependency, in - * iteration order of {@code dependencies} + * @return set of addon names whose dependency closure cannot be + * satisfied, in iteration order of {@code dependencies} */ public static Set unsatisfied(Map> dependencies) { - Set known = dependencies.keySet(); Set bad = new LinkedHashSet<>(); - for (Map.Entry> e : dependencies.entrySet()) { - for (String dep : e.getValue()) { - if (!known.contains(dep)) { - bad.add(e.getKey()); - break; + boolean changed; + do { + changed = false; + for (Map.Entry> e : dependencies.entrySet()) { + if (bad.contains(e.getKey())) continue; + for (String dep : e.getValue()) { + if (!dependencies.containsKey(dep) || bad.contains(dep)) { + bad.add(e.getKey()); + changed = true; + break; + } } } - } + } while (changed); return bad; } diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java index ac50b5e..818621b 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java @@ -116,13 +116,16 @@ public void loadAll() { // typically because the dep is a plugin-as-addon (Path 1) which // doesn't show up in the addons/ folder, or simply a typo. Mark // them FAILED individually instead of poisoning the whole batch - // through topoSort. + // through topoSort. Runs to fixed point so transitive failures + // (A -> B -> missing C) catch both A and B in the same pass. Set unsatisfied = AddonDependencyGraph.unsatisfied(depGraph); + Set originalGraphKeys = Set.copyOf(depGraph.keySet()); for (String key : unsatisfied) { LoadedAddon la = byName.remove(key); depGraph.remove(key); String missing = la.getConf().addonDependencies().stream() - .filter(d -> !byName.containsKey(d.toLowerCase())) + .filter(d -> !originalGraphKeys.contains(d.toLowerCase()) + || unsatisfied.contains(d.toLowerCase())) .findFirst().orElse("?"); String msg = "missing addon dependency: " + missing; Logger.warning("Addon " + la.getConf().name() + " " + msg + " - skipping"); @@ -154,6 +157,11 @@ public void loadAll() { Logger.severe("Addon " + la.getConf().name() + " failed in onLoad: " + t); t.printStackTrace(); la.markFailed(t); + // onLoad shouldn't register types per the contract, but a + // misbehaving addon might have done so before throwing - + // strip whatever it managed so the next addon's enable + // sees a clean registry state. + rollbackRegistrations(la); } } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionItemRefresh.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionItemRefresh.java index 9d19030..aaa9b6a 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionItemRefresh.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionItemRefresh.java @@ -36,9 +36,9 @@ public void activate(Player player, Menu menu, Item clickedItem) { if (ticks != null) { Slot finalSlot = slot; - BukkitTasks.runTaskLater(() -> - menu.refreshItem(finalSlot, player), ticks.getInt(player, menu) - ); + BukkitTasks.runForEntityLater(player, + () -> menu.refreshItem(finalSlot, player), + ticks.getInt(player, menu)); return; } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuClose.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuClose.java index 74aca22..b22a283 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuClose.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuClose.java @@ -29,8 +29,8 @@ private ActionMenuClose(TypeInt ticks) { @Override public void activate(Player player, Menu menu, Item clickedItem) { if (ticks != null) { - BukkitTasks.runTaskLater(() -> - MenuManager.instance().closeMenu(player), + BukkitTasks.runForEntityLater(player, + () -> MenuManager.instance().closeMenu(player), ticks.getInt(player, menu)); } else { if (isClose.getBool(player, menu)) diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuRefresh.java b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuRefresh.java index d6190b6..17fc291 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuRefresh.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/ActionMenuRefresh.java @@ -29,7 +29,9 @@ private ActionMenuRefresh(TypeInt ticks) { @Override public void activate(Player player, Menu menu, Item clickedItem) { if (ticks != null) { - BukkitTasks.runTaskLater(() -> MenuManager.instance().refreshMenu(player), ticks.getInt(player, menu)); + BukkitTasks.runForEntityLater(player, + () -> MenuManager.instance().refreshMenu(player), + ticks.getInt(player, menu)); return; } diff --git a/plugin/src/main/java/ru/abstractmenus/data/actions/wrappers/ActionDelay.java b/plugin/src/main/java/ru/abstractmenus/data/actions/wrappers/ActionDelay.java index 9750da8..5b63b85 100644 --- a/plugin/src/main/java/ru/abstractmenus/data/actions/wrappers/ActionDelay.java +++ b/plugin/src/main/java/ru/abstractmenus/data/actions/wrappers/ActionDelay.java @@ -25,7 +25,9 @@ private ActionDelay(TypeInt delay, Actions actions) { @Override public void activate(Player player, Menu menu, Item clickedItem) { if (actions != null) { - BukkitTasks.runTaskLater(() -> actions.activate(player, menu, clickedItem), delay.getInt(player, menu)); + BukkitTasks.runForEntityLater(player, + () -> actions.activate(player, menu, clickedItem), + delay.getInt(player, menu)); } } diff --git a/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java b/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java index aea1c5a..26a1e05 100644 --- a/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java +++ b/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java @@ -78,9 +78,13 @@ public synchronized void register(String key, LOG.warning("TypeRegistry: overwriting existing entry '" + k + "' (" + existing.getName() + " -> " + type.getName() + ")"); byType.remove(existing); - // Intentionally do not remove from owner tracking — the old owner - // no longer has this key since it's overwritten; cleanup of their - // orphan entries happens on their own unregisterAll. + // Strip k from the old owner's set so their later unregisterAll + // doesn't wipe the new owner's entry. Without this the *new* + // owner's class would silently vanish from the registry the + // first time the *old* owner gets disabled. + for (Set ownerKeys : keysByOwner.values()) { + ownerKeys.remove(k); + } } byKey.put(k, type); diff --git a/plugin/src/main/java/ru/abstractmenus/listeners/PlayerListener.java b/plugin/src/main/java/ru/abstractmenus/listeners/PlayerListener.java index 637fb53..9e63aef 100644 --- a/plugin/src/main/java/ru/abstractmenus/listeners/PlayerListener.java +++ b/plugin/src/main/java/ru/abstractmenus/listeners/PlayerListener.java @@ -19,6 +19,12 @@ public void onPlayerJoin(PlayerJoinEvent event) { @EventHandler public void onPlayerQuit(PlayerQuitEvent event) { + // Drop any tracked menu before the entity is gone. InventoryListener + // also closes via runForEntityLater(player, ..., 3L), but on Folia + // that task is silently dropped if the player quits inside the 3-tick + // window - leaving openedMenus with a stale entry. Removing here + // guarantees cleanup regardless of timing. + MenuManager.instance().removePlayerMenu(event.getPlayer()); MenuManager.instance().getAndRemoveInputAction(event.getPlayer()); ProfileStorage storage = ProfileStorage.instance(); if (storage != null) { diff --git a/plugin/src/test/java/ru/abstractmenus/addon/AddonDependencyGraphTest.java b/plugin/src/test/java/ru/abstractmenus/addon/AddonDependencyGraphTest.java index fceeea4..8713361 100644 --- a/plugin/src/test/java/ru/abstractmenus/addon/AddonDependencyGraphTest.java +++ b/plugin/src/test/java/ru/abstractmenus/addon/AddonDependencyGraphTest.java @@ -81,11 +81,26 @@ void twoNodeCycle_throws() { void unsatisfied_returnsAddonsWithMissingDeps() { Map> deps = new LinkedHashMap<>(); deps.put("a", List.of("ghost")); // ghost not in graph - deps.put("b", List.of("a")); // a IS in graph + deps.put("b", List.of("a")); // a IS in graph but transitively bad deps.put("c", List.of()); + // Both a (direct miss) and b (transitive miss through a) must be + // flagged. A single-pass implementation would only catch a and let + // b leak into topoSort. Set bad = AddonDependencyGraph.unsatisfied(deps); - assertEquals(Set.of("a"), bad); + assertEquals(Set.of("a", "b"), bad); + } + + @Test + void unsatisfied_transitiveChain() { + Map> deps = new LinkedHashMap<>(); + deps.put("d", List.of("c")); + deps.put("c", List.of("b")); + deps.put("b", List.of("ghost")); + deps.put("a", List.of()); + + Set bad = AddonDependencyGraph.unsatisfied(deps); + assertEquals(Set.of("b", "c", "d"), bad); } @Test From d1a9557c23599139ce7367c4e059bf6df3090857 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 14:43:58 +0500 Subject: [PATCH 15/16] fix: round-5 P2+P3 (security hardening + small cleanups) P2-S2: CommandAddons.safe() now strips MiniMessage tags too (, , , etc.). Without this a malicious addon could put a click-to-run-command tag in its name and turn an operator's /am addons list click into privilege escalation. P2-S3: NodeSerializers reflection failure now logs a clearer message that points at the bundled hocon lib (not AbstractMenus) as the source, and explicitly says the side effect is a memory leak until full server restart that compounds across reloads. P2-S4: AddonClassLoader.PARENT_FIRST_PREFIXES extended with org.spigotmc, ca.spottedleaf, io.netty, it.unimi.dsi.fastutil, gson, guava. An addon shipping its own copy of any of these would otherwise hit ClassCastException at the first hand-off back to server code (same FQN, different classloader = different class). P3-N3: CoreExtension.version() now returns "unknown" until onEnable populates it, matching the MenuExtension default and avoiding a "vnull" render in /am addons list during the brief onLoad-to-onEnable window. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../ru/abstractmenus/addon/AddonClassLoader.java | 11 +++++++++++ .../abstractmenus/commands/am/CommandAddons.java | 14 +++++++++++++- .../java/ru/abstractmenus/core/CoreExtension.java | 4 +++- .../ru/abstractmenus/impl/TypeRegistryImpl.java | 14 +++++++++++--- 4 files changed, 38 insertions(+), 5 deletions(-) diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonClassLoader.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonClassLoader.java index 9d35418..f27b3ce 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonClassLoader.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonClassLoader.java @@ -21,6 +21,7 @@ public final class AddonClassLoader extends URLClassLoader { static final String[] PARENT_FIRST_PREFIXES = { "ru.abstractmenus.api.", "org.bukkit.", + "org.spigotmc.", "io.papermc.", "com.destroystokyo.paper.", "net.kyori.adventure.", @@ -31,6 +32,16 @@ public final class AddonClassLoader extends URLClassLoader { // addon boundary. "net.minecraft.", "com.mojang.", + // Server-bundled libraries. If an addon ships its own copy with + // a different version or shaded-but-not-relocated, the same + // class loaded by two classloaders is not the same class - + // ClassCastException at the first hand-off. Force parent-first + // so everyone uses the server's copy. + "ca.spottedleaf.", // Paper concurrent utilities + "io.netty.", // Netty (Paper bundles) + "it.unimi.dsi.fastutil.", + "com.google.gson.", + "com.google.common.", // Guava "java.", "javax.", "jdk.", diff --git a/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java b/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java index 1b18f79..7f9040e 100644 --- a/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java +++ b/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java @@ -35,9 +35,21 @@ public class CommandAddons extends Command { private static final Pattern UNSAFE_FORMAT = Pattern.compile( "&[0-9a-fk-orA-FK-OR]|§[0-9a-fk-orA-FK-OR]|<#[0-9a-fA-F]{6}>"); + /** + * Strips MiniMessage tags ({@code }, {@code }, + * {@code }, etc.). Applied alongside + * {@link #UNSAFE_FORMAT} when MiniMessage rendering is enabled - without + * it a malicious addon could put a {@code } + * tag in its name and turn an operator's {@code /am addons list} click + * into privilege escalation. Conservatively matches anything shaped like + * {@code }; legit addon names don't need angle brackets. + */ + private static final Pattern MM_TAG = Pattern.compile("<[/!?]?[a-zA-Z][^>]*>"); + private static String safe(String untrusted) { if (untrusted == null) return ""; - return UNSAFE_FORMAT.matcher(untrusted).replaceAll(""); + String stripped = UNSAFE_FORMAT.matcher(untrusted).replaceAll(""); + return MM_TAG.matcher(stripped).replaceAll(""); } public CommandAddons() { diff --git a/plugin/src/main/java/ru/abstractmenus/core/CoreExtension.java b/plugin/src/main/java/ru/abstractmenus/core/CoreExtension.java index 79e83c8..7994ee1 100644 --- a/plugin/src/main/java/ru/abstractmenus/core/CoreExtension.java +++ b/plugin/src/main/java/ru/abstractmenus/core/CoreExtension.java @@ -30,7 +30,9 @@ public String name() { @Override public String version() { - return version; + // Null between construction and onEnable - fall through to the + // default-style "unknown" so /am addons list never shows "vnull". + return version != null ? version : "unknown"; } @Override diff --git a/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java b/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java index 26a1e05..90248a2 100644 --- a/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java +++ b/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java @@ -45,7 +45,11 @@ public final class TypeRegistryImpl implements TypeRegistry { f.setAccessible(true); } catch (NoSuchFieldException e) { LOG.log(Level.WARNING, - "NodeSerializers.serializers field missing; addon-disable will leak classloader references", + "NodeSerializers.serializers field not found - bundled hocon lib has " + + "changed shape since AbstractMenus was built. Not an AbstractMenus " + + "bug; needs an upstream hocon change to expose unregister(Class). " + + "Side effect: every addon disable/reload from now on leaks its " + + "classloader until a full server restart.", e); } NODE_SERIALIZERS_MAP_FIELD = f; @@ -144,8 +148,12 @@ private void removeSerializerEntry(Class type) { backing.remove(type); } catch (Throwable t) { LOG.log(Level.WARNING, - "Failed to drop NodeSerializers entry for " + type.getName() - + "; addon classloader may be retained", + "Cannot remove NodeSerializer for " + type.getName() + + ": bundled hocon lib has no public unregister(Class), and " + + "our reflection workaround failed. Not an AbstractMenus bug - " + + "needs an upstream hocon change. Side effect: this addon's " + + "classloader stays in memory until a full server restart; the " + + "leak compounds across reloads.", t); } } From b9bbad5af408e1c2ad33e5990173afd76d7baed1 Mon Sep 17 00:00:00 2001 From: Stanislav Panchenko Date: Tue, 28 Apr 2026 14:49:56 +0500 Subject: [PATCH 16/16] feat(commands): show Path 1 + built-in extensions in /am addons list MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before: /am addons list only surfaced Path 2 (AM-loaded jars in addons/). A plugin-as-addon's contributions were invisible — operators had to cross-reference /plugins to figure out who registered what. Now the list renders three groups: - Path 2 addons unchanged - Path 1 plugin-as-addons with [as-plugin] suffix - The built-in CoreExtension with [built-in] suffix Plumbing: - TypeRegistryImpl + ProviderRegistryImpl expose seenOwners() — the union of every MenuExtension that registered a type/provider. - AddonManager.knownExtensions() unions all six registries. - AbstractMenus keeps a reference to the CoreExtension instance so the list command can identify and label it. - /am addons info works for all three kinds. Path 1 surfaces Bukkit PluginDescriptionFile (authors / depend / softDepend / status). - tab-complete for info covers all three; reload still only completes Path 2 (no jar to re-read for the others). Co-Authored-By: Claude Opus 4.7 (1M context) --- .../java/ru/abstractmenus/AbstractMenus.java | 8 +- .../ru/abstractmenus/addon/AddonManager.java | 26 ++++ .../commands/am/CommandAddons.java | 142 ++++++++++++++++-- .../impl/ProviderRegistryImpl.java | 20 +++ .../abstractmenus/impl/TypeRegistryImpl.java | 10 ++ 5 files changed, 195 insertions(+), 11 deletions(-) diff --git a/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java b/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java index 9d57d58..4af53f8 100644 --- a/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java +++ b/plugin/src/main/java/ru/abstractmenus/AbstractMenus.java @@ -68,6 +68,12 @@ public final class AbstractMenus extends JavaPlugin { private AbstractMenusApi api; private AddonManager addonManager; private MainConfig mainConfig; + /** + * The plugin's own dogfood {@link MenuExtension}. Held as a field so + * {@code /am addons list} can render it as the {@code [built-in]} entry + * and tell it apart from operator-installed Path 1 / Path 2 addons. + */ + private MenuExtension core; @Getter @Setter @@ -146,7 +152,7 @@ public void onEnable() { // Core extension — dogfood: the plugin registers its own types through // the same SPI external addons will use. - MenuExtension core = new CoreExtension(); + core = new CoreExtension(); core.onLoad(api); core.onEnable(api); diff --git a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java index 818621b..572e45c 100644 --- a/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java +++ b/plugin/src/main/java/ru/abstractmenus/addon/AddonManager.java @@ -15,6 +15,7 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -322,6 +323,31 @@ public Optional get(String name) { return Optional.ofNullable(addons.get(name.toLowerCase())); } + /** + * Every {@link MenuExtension} that has registered at least one type or + * provider on the running API. Includes: + *

    + *
  • Path 2 AM-loaded addons (also in {@link #loaded()})
    • + *
  • Path 1 plugin-as-addons (NOT in {@link #loaded()} — they live + * on Bukkit's plugin lifecycle, we only see them through their + * registry footprint)
    • + *
  • Plugin-internal extensions like {@code CoreExtension}
    • + *
+ * + *

Used by {@code /am addons list} to surface Path 1 entries that + * would otherwise be invisible under {@code /am addons}. + */ + public Set knownExtensions() { + Set all = new HashSet<>(); + all.addAll(((TypeRegistryImpl) api.actions()).seenOwners()); + all.addAll(((TypeRegistryImpl) api.rules()).seenOwners()); + all.addAll(((TypeRegistryImpl) api.activators()).seenOwners()); + all.addAll(((TypeRegistryImpl) api.itemProperties()).seenOwners()); + all.addAll(((TypeRegistryImpl) api.catalogs()).seenOwners()); + all.addAll(((ProviderRegistryImpl) api.providers()).seenOwners()); + return all; + } + /** * Scan {@link #addonsDir} for {@code *.jar} files. For each, extract * {@code addon.conf}, parse it, and build a LoadedAddon (without enabling diff --git a/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java b/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java index 7f9040e..3d73967 100644 --- a/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java +++ b/plugin/src/main/java/ru/abstractmenus/commands/am/CommandAddons.java @@ -1,16 +1,21 @@ package ru.abstractmenus.commands.am; import org.bukkit.command.CommandSender; +import org.bukkit.plugin.PluginDescriptionFile; +import org.bukkit.plugin.java.JavaPlugin; import ru.abstractmenus.AbstractMenus; import ru.abstractmenus.addon.AddonManager; import ru.abstractmenus.addon.AddonStatus; import ru.abstractmenus.addon.LoadedAddon; +import ru.abstractmenus.api.MenuExtension; import ru.abstractmenus.api.text.Colors; import ru.abstractmenus.commands.Command; import java.util.ArrayList; import java.util.Collections; +import java.util.LinkedHashSet; import java.util.List; +import java.util.Set; import java.util.regex.Pattern; /** {@code /am addons [list|reload |info |load |rescan]} */ @@ -86,13 +91,20 @@ public void execute(CommandSender sender, String[] args) { } private void list(CommandSender sender, AddonManager am) { - var addons = am.loaded(); - if (addons.isEmpty()) { - sender.sendMessage(Colors.of("&7No AM-loaded addons.")); + var path2 = am.loaded(); + Set path1 = pathOneExtensions(am); + MenuExtension core = AbstractMenus.instance().getCore(); + + int total = path2.size() + path1.size() + (core != null ? 1 : 0); + if (total == 0) { + sender.sendMessage(Colors.of("&7No addons.")); return; } - sender.sendMessage(Colors.of("&e&lAddons (" + addons.size() + "):")); - for (LoadedAddon la : addons) { + + sender.sendMessage(Colors.of("&e&lAddons (" + total + "):")); + + // Path 2 — render unchanged. + for (LoadedAddon la : path2) { String color = switch (la.getStatus()) { case ENABLED -> "&a"; case DISABLED -> "&7"; @@ -103,6 +115,47 @@ private void list(CommandSender sender, AddonManager am) { + " &8v" + safe(la.getConf().version()) + " &7[" + la.getStatus() + "]")); } + + // Path 1 — derived state from the JavaPlugin lifecycle when available. + for (MenuExtension ext : path1) { + boolean enabled = !(ext instanceof JavaPlugin jp) || jp.isEnabled(); + String color = enabled ? "&a" : "&c"; + String status = enabled ? "ENABLED" : "DISABLED"; + sender.sendMessage(Colors.of(color + " " + safe(ext.name()) + + " &8v" + safe(ext.version()) + + " &7[" + status + "] &8[as-plugin]")); + } + + // Built-in (CoreExtension) — last so the operator's eye lands on + // operator-installed addons first. + if (core != null) { + sender.sendMessage(Colors.of("&a " + safe(core.name()) + + " &8v" + safe(core.version()) + + " &7[ENABLED] &8[built-in]")); + } + } + + /** + * Path 1 plugin-as-addons: every {@link MenuExtension} we see in the + * registry footprint that is neither a Path 2 AM-loaded addon nor the + * built-in {@code CoreExtension}. + */ + private static Set pathOneExtensions(AddonManager am) { + // Names of Path 2 addons - their MenuExtension instances must be + // skipped from the "registry-footprint" set. + Set path2Exts = new LinkedHashSet<>(); + for (LoadedAddon la : am.loaded()) { + if (la.getExtension() != null) path2Exts.add(la.getExtension()); + } + MenuExtension core = AbstractMenus.instance().getCore(); + + Set path1 = new LinkedHashSet<>(); + for (MenuExtension ext : am.knownExtensions()) { + if (ext == core) continue; + if (path2Exts.contains(ext)) continue; + path1.add(ext); + } + return path1; } private void reload(CommandSender sender, AddonManager am, String[] args) { @@ -130,12 +183,34 @@ private void info(CommandSender sender, AddonManager am, String[] args) { sender.sendMessage(Colors.of("&cUsage: /am addons info ")); return; } - var opt = am.get(args[1]); - if (opt.isEmpty()) { - sender.sendMessage(Colors.of("&cAddon '" + safe(args[1]) + "' not found.")); + String name = args[1]; + + // Path 2 — full addon.conf metadata. + var opt = am.get(name); + if (opt.isPresent()) { + renderPathTwoInfo(sender, opt.get()); + return; + } + + // Built-in. + MenuExtension core = AbstractMenus.instance().getCore(); + if (core != null && core.name().equalsIgnoreCase(name)) { + sender.sendMessage(Colors.of("&e&l" + safe(core.name()) + " &7v" + safe(core.version()))); + sender.sendMessage(Colors.of("&7 status: &fENABLED &8[built-in]")); + return; + } + + // Path 1 — surface the JavaPlugin description when available. + for (MenuExtension ext : pathOneExtensions(am)) { + if (!ext.name().equalsIgnoreCase(name)) continue; + renderPathOneInfo(sender, ext); return; } - LoadedAddon la = opt.get(); + + sender.sendMessage(Colors.of("&cAddon '" + safe(name) + "' not found.")); + } + + private static void renderPathTwoInfo(CommandSender sender, LoadedAddon la) { var c = la.getConf(); sender.sendMessage(Colors.of("&e&l" + safe(c.name()) + " &7v" + safe(c.version()))); sender.sendMessage(Colors.of("&7 status: &f" + la.getStatus())); @@ -161,6 +236,35 @@ private void info(CommandSender sender, AddonManager am, String[] args) { } } + private static void renderPathOneInfo(CommandSender sender, MenuExtension ext) { + sender.sendMessage(Colors.of("&e&l" + safe(ext.name()) + " &7v" + safe(ext.version()) + + " &8[as-plugin]")); + + if (ext instanceof JavaPlugin jp) { + PluginDescriptionFile desc = jp.getDescription(); + sender.sendMessage(Colors.of("&7 status: &f" + (jp.isEnabled() ? "ENABLED" : "DISABLED"))); + if (!desc.getAuthors().isEmpty()) { + sender.sendMessage(Colors.of("&7 authors: &f" + + safe(String.join(", ", desc.getAuthors())))); + } + if (desc.getDescription() != null && !desc.getDescription().isEmpty()) { + sender.sendMessage(Colors.of("&7 description: &f" + safe(desc.getDescription()))); + } + if (!desc.getDepend().isEmpty()) { + sender.sendMessage(Colors.of("&7 depend: &f" + + safe(String.join(", ", desc.getDepend())))); + } + if (!desc.getSoftDepend().isEmpty()) { + sender.sendMessage(Colors.of("&7 softDepend: &f" + + safe(String.join(", ", desc.getSoftDepend())))); + } + } else { + // Non-JavaPlugin Path 1 - rare but legal (an extension produced + // by a plugin's onEnable that isn't the plugin instance itself). + sender.sendMessage(Colors.of("&7 status: &fENABLED")); + } + } + private void load(CommandSender sender, AddonManager am, String[] args) { if (args.length < 2) { sender.sendMessage(Colors.of("&cUsage: /am addons load ")); @@ -202,6 +306,20 @@ private void rescan(CommandSender sender, AddonManager am) { } } + /** + * Names valid for {@code /am addons info } tab-complete: every + * Path 2 loaded addon, every Path 1 plugin-as-addon, and the built-in + * core extension. + */ + private static List allInfoNames(AddonManager am) { + List names = new ArrayList<>(); + for (LoadedAddon la : am.loaded()) names.add(la.getConf().name()); + for (MenuExtension ext : pathOneExtensions(am)) names.add(ext.name()); + MenuExtension core = AbstractMenus.instance().getCore(); + if (core != null) names.add(core.name()); + return names; + } + @Override public List tabComplete(CommandSender sender, String[] args) { if (args.length == 0) return Collections.emptyList(); @@ -220,9 +338,13 @@ public List tabComplete(CommandSender sender, String[] args) { if (am == null) return Collections.emptyList(); String prefix = args[1].toLowerCase(); return switch (args[0].toLowerCase()) { - case "reload", "info" -> filterByPrefix( + // reload only works on Path 2 (the only ones with a jar in + // addons/ to re-read). + case "reload" -> filterByPrefix( () -> am.loaded().stream().map(la -> la.getConf().name()).iterator(), prefix); + // info works on all three: Path 2, Path 1, built-in core. + case "info" -> filterByPrefix(allInfoNames(am), prefix); case "load" -> filterByPrefix(am.availableNotLoaded(), prefix); default -> Collections.emptyList(); }; diff --git a/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java index 568aaab..3eac0c4 100644 --- a/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java +++ b/plugin/src/main/java/ru/abstractmenus/impl/ProviderRegistryImpl.java @@ -88,6 +88,22 @@ public void unregisterAll(MenuExtension owner) { skins.unregisterAll(owner); } + /** + * Union of every extension that has registered a provider in any of + * the five sections. Used by {@code /am addons list} to discover + * Path 1 plugin-as-addons whose only fingerprint is in the registry + * owner-tracking map. + */ + public Set seenOwners() { + Set all = new HashSet<>(); + all.addAll(economy.seenOwners()); + all.addAll(permissions.seenOwners()); + all.addAll(levels.seenOwners()); + all.addAll(placeholders.seenOwners()); + all.addAll(skins.seenOwners()); + return Collections.unmodifiableSet(all); + } + // ----------------------------------------------------------------- // SectionImpl - one instance per provider type // ----------------------------------------------------------------- @@ -157,6 +173,10 @@ synchronized void unregisterAll(MenuExtension extOwner) { for (String k : keys) byId.remove(k); } + synchronized Set seenOwners() { + return new HashSet<>(keysByExtension.keySet()); + } + private static final class Entry { final T handler; final int priority; diff --git a/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java b/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java index 90248a2..e75efad 100644 --- a/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java +++ b/plugin/src/main/java/ru/abstractmenus/impl/TypeRegistryImpl.java @@ -113,6 +113,16 @@ public synchronized Set keys() { return Collections.unmodifiableSet(new HashSet<>(byKey.keySet())); } + /** + * Snapshot of every extension that has ever registered (and not since + * fully unregistered) at least one entry in this registry. Used by + * {@code /am addons list} to surface Path 1 plugin-as-addons that + * don't sit in the AddonManager's loaded map. + */ + public synchronized Set seenOwners() { + return Collections.unmodifiableSet(new HashSet<>(keysByOwner.keySet())); + } + /** * Wipe every entry registered by {@code owner}. Intentionally NOT on the * public {@link TypeRegistry} interface so that addons cannot use it to