From b6a4b53ea73fe17b0335cf692af5d0608ae28c81 Mon Sep 17 00:00:00 2001 From: Efnilite <35348263+Efnilite@users.noreply.github.com> Date: Wed, 9 Jul 2025 00:24:29 +0200 Subject: [PATCH 1/2] fix bad remove logic --- .../lang/function/FunctionRegistry.java | 47 +++++++++++++++---- .../njol/skript/lang/function/Functions.java | 6 ++- .../lang/function/FunctionRegistryTest.java | 30 ++++++++++++ 3 files changed, 74 insertions(+), 9 deletions(-) diff --git a/src/main/java/ch/njol/skript/lang/function/FunctionRegistry.java b/src/main/java/ch/njol/skript/lang/function/FunctionRegistry.java index 271895c9993..b2fe0d95273 100644 --- a/src/main/java/ch/njol/skript/lang/function/FunctionRegistry.java +++ b/src/main/java/ch/njol/skript/lang/function/FunctionRegistry.java @@ -64,6 +64,10 @@ public static FunctionRegistry getRegistry() { /** * Registers a signature. + *
+ * Attempting to register a local signature in the global namespace, or a global signature in + * a local namespace, will throw an {@link IllegalArgumentException}. + *
* * @param namespace The namespace to register the signature in. * If namespace is null, will register this signature globally. @@ -71,14 +75,23 @@ public static FunctionRegistry getRegistry() { * @param signature The signature to register. * @throws SkriptAPIException if a signature with the same name and parameters is already registered * in this namespace. + * @throws IllegalArgumentException if the signature is global and namespace is not null, or + * if the signature is local and namespace is null. */ public void register(@Nullable String namespace, @NotNull Signature> signature) { Preconditions.checkNotNull(signature, "signature cannot be null"); + if (signature.isLocal() && namespace == null) { + throw new IllegalArgumentException("Cannot register a local signature in the global namespace"); + } + if (!signature.isLocal() && namespace != null) { + throw new IllegalArgumentException("Cannot register a global signature in a local namespace"); + } + Skript.debug("Registering signature '%s'", signature.getName()); // namespace NamespaceIdentifier namespaceId; - if (namespace != null && signature.isLocal()) { + if (namespace != null) { namespaceId = new NamespaceIdentifier(namespace); } else { namespaceId = GLOBAL_NAMESPACE; @@ -118,17 +131,29 @@ public void register(@NotNull Function> function) { /** * Registers a function. + *+ * Attempting to register a local function in the global namespace, or a global function in + * a local namespace, will throw an {@link IllegalArgumentException}. + *
* * @param namespace The namespace to register the function in. - * If namespace is null, will register this function globally. + * If namespace is null, will register this function globally, only if the function is global. * Usually represents the path of the script this function is registered in. * @param function The function to register. - * @throws SkriptAPIException if the function name is invalid or if - * a function with the same name and parameters is already registered - * in this namespace. + * @throws SkriptAPIException if the function name is invalid or if + * a function with the same name and parameters is already registered + * in this namespace. + * @throws IllegalArgumentException if the function is global and namespace is not null, or + * if the function is local and namespace is null. */ public void register(@Nullable String namespace, @NotNull Function> function) { Preconditions.checkNotNull(function, "function cannot be null"); + if (function.getSignature().isLocal() && namespace == null) { + throw new IllegalArgumentException("Cannot register a local function in the global namespace"); + } + if (!function.getSignature().isLocal() && namespace != null) { + throw new IllegalArgumentException("Cannot register a global function in a local namespace"); + } Skript.debug("Registering function '%s'", function.getName()); String name = function.getName(); @@ -138,7 +163,7 @@ public void register(@Nullable String namespace, @NotNull Function> function) // namespace NamespaceIdentifier namespaceId; - if (namespace != null && function.getSignature().isLocal()) { + if (namespace != null) { namespaceId = new NamespaceIdentifier(namespace); } else { namespaceId = GLOBAL_NAMESPACE; @@ -487,8 +512,13 @@ public void remove(@NotNull Signature> signature) { String name = signature.getName(); FunctionIdentifier identifier = FunctionIdentifier.of(signature); - Namespace namespace = namespaces.getOrDefault(new NamespaceIdentifier(signature.script), - namespaces.get(GLOBAL_NAMESPACE)); + Namespace namespace; + if (signature.isLocal()) { + namespace = namespaces.get(new NamespaceIdentifier(signature.script)); + } else { + namespace = namespaces.get(GLOBAL_NAMESPACE); + } + if (namespace == null) { return; } @@ -532,6 +562,7 @@ private record NamespaceIdentifier(@Nullable String name) { /** * Returns whether this identifier is for local namespaces. + * * @return Whether this identifier is for local namespaces. */ public boolean local() { diff --git a/src/main/java/ch/njol/skript/lang/function/Functions.java b/src/main/java/ch/njol/skript/lang/function/Functions.java index b9a40367189..b03a2195fd7 100644 --- a/src/main/java/ch/njol/skript/lang/function/Functions.java +++ b/src/main/java/ch/njol/skript/lang/function/Functions.java @@ -104,7 +104,11 @@ public static JavaFunction> registerFunction(JavaFunction> function) { namespace.addFunction(function); } - FunctionRegistry.getRegistry().register(script.getConfig().getFileName(), function); + if (function.getSignature().isLocal()) { + FunctionRegistry.getRegistry().register(script.getConfig().getFileName(), function); + } else { + FunctionRegistry.getRegistry().register(null, function); + } return function; } diff --git a/src/test/java/ch/njol/skript/lang/function/FunctionRegistryTest.java b/src/test/java/ch/njol/skript/lang/function/FunctionRegistryTest.java index 99b7208c3a0..92b9d2434ff 100644 --- a/src/test/java/ch/njol/skript/lang/function/FunctionRegistryTest.java +++ b/src/test/java/ch/njol/skript/lang/function/FunctionRegistryTest.java @@ -388,4 +388,34 @@ public void testIdentifierSignatureOf() { assertEquals(FunctionIdentifier.of(function2.getSignature()), identifier); } + @Test + public void testRemoveGlobal() { + // create empty TEST_SCRIPT namespace such that it is not null + registry.register(TEST_SCRIPT, LOCAL_TEST_FUNCTION); + registry.remove(LOCAL_TEST_FUNCTION.getSignature()); + + assertEquals(RetrievalResult.NOT_REGISTERED, registry.getSignature(TEST_SCRIPT, FUNCTION_NAME).result()); + + // construct a global function with a non-null script, which happens in script functions + Signature