From adaa68821229107d6b6046273585e57994ae074b Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Thu, 3 Feb 2022 17:50:25 -0500 Subject: [PATCH 01/13] Implement initial design --- src/main/java/ch/njol/skript/Skript.java | 48 ++--- src/main/java/ch/njol/skript/SkriptAddon.java | 172 +++++++++++------- .../skript/registration/Module.java | 56 ++++++ 3 files changed, 178 insertions(+), 98 deletions(-) create mode 100644 src/main/java/org/skriptlang/skript/registration/Module.java diff --git a/src/main/java/ch/njol/skript/Skript.java b/src/main/java/ch/njol/skript/Skript.java index 6ebe02f20ae..2913c2054d9 100644 --- a/src/main/java/ch/njol/skript/Skript.java +++ b/src/main/java/ch/njol/skript/Skript.java @@ -480,14 +480,9 @@ public void onEnable() { new DefaultFunctions(); ChatMessages.registerListeners(); - - try { - getAddonInstance().loadClasses("ch.njol.skript", "conditions", "effects", "events", "expressions", "entity", "sections"); - } catch (final Exception e) { - exception(e, "Could not load required .class files: " + e.getLocalizedMessage()); - setEnabled(false); - return; - } + + getAddonInstance().loadClasses("ch.njol.skript", "conditions", "effects", "events", "expressions", "entity", "sections") + .loadModules("org.skriptlang.skript"); Commands.registerListeners(); @@ -500,42 +495,23 @@ public void onEnable() { @Override public void run() { assert Bukkit.getWorlds().get(0).getFullTime() == tick; - + // Load hooks from Skript jar - try { - try (JarFile jar = new JarFile(getFile())) { - for (final JarEntry e : new EnumerationIterable<>(jar.entries())) { - if (e.getName().startsWith("ch/njol/skript/hooks/") && e.getName().endsWith("Hook.class") && StringUtils.count("" + e.getName(), '/') <= 5) { - final String c = e.getName().replace('/', '.').substring(0, e.getName().length() - ".class".length()); - try { - final Class hook = Class.forName(c, true, getClassLoader()); - if (hook != null && Hook.class.isAssignableFrom(hook) && !hook.isInterface() && Hook.class != hook && isHookEnabled((Class>) hook)) { - hook.getDeclaredConstructor().setAccessible(true); - hook.getDeclaredConstructor().newInstance(); - } - } catch (final ClassNotFoundException ex) { - Skript.exception(ex, "Cannot load class " + c); - } catch (final ExceptionInInitializerError err) { - Skript.exception(err.getCause(), "Class " + c + " generated an exception while loading"); - } - } + getAddonInstance().loadClasses(c -> { + if (Hook.class.isAssignableFrom(c) && !c.isInterface() && !Modifier.isAbstract(c.getModifiers())) { + try { + c.getDeclaredConstructor().newInstance(); + } catch (Exception ex) { + Skript.exception(ex, "Failed to load hook class " + c); } } - } catch (final Exception e) { - error("Error while loading plugin hooks" + (e.getLocalizedMessage() == null ? "" : ": " + e.getLocalizedMessage())); - Skript.exception(e); - } + }, false, "ch.njol.skript.hooks", false); finishedLoadingHooks = true; if (TestMode.ENABLED) { info("Preparing Skript for testing..."); tainted = true; - try { - getAddonInstance().loadClasses("ch.njol.skript", "tests"); - } catch (IOException e) { - Skript.exception("Failed to load testing environment."); - Bukkit.getServer().shutdown(); - } + getAddonInstance().loadClasses("ch.njol.skript", "tests"); } stopAcceptingRegistrations(); diff --git a/src/main/java/ch/njol/skript/SkriptAddon.java b/src/main/java/ch/njol/skript/SkriptAddon.java index e85ba9c2e75..72b15faf1ec 100644 --- a/src/main/java/ch/njol/skript/SkriptAddon.java +++ b/src/main/java/ch/njol/skript/SkriptAddon.java @@ -18,116 +18,162 @@ */ package ch.njol.skript; +import ch.njol.skript.localization.Language; +import ch.njol.skript.util.Utils; +import ch.njol.skript.util.Version; +import ch.njol.util.StringUtils; +import ch.njol.util.coll.iterator.EnumerationIterable; +import org.bukkit.plugin.java.JavaPlugin; +import org.eclipse.jdt.annotation.Nullable; +import org.skriptlang.skript.registration.Module; + import java.io.File; import java.io.IOException; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; +import java.util.function.Consumer; import java.util.jar.JarEntry; import java.util.jar.JarFile; import java.util.regex.Matcher; import java.util.regex.Pattern; -import org.bukkit.plugin.java.JavaPlugin; -import org.eclipse.jdt.annotation.Nullable; - -import ch.njol.skript.localization.Language; -import ch.njol.skript.util.Utils; -import ch.njol.skript.util.Version; -import ch.njol.util.coll.iterator.EnumerationIterable; - /** * Utility class for Skript addons. Use {@link Skript#registerAddon(JavaPlugin)} to create a SkriptAddon instance for your plugin. - * - * @author Peter Güttinger */ public final class SkriptAddon { public final JavaPlugin plugin; public final Version version; - private final String name; /** * Package-private constructor. Use {@link Skript#registerAddon(JavaPlugin)} to get a SkriptAddon for your plugin. * - * @param p + * @param plugin The plugin representing the SkriptAddon to be registered. */ - SkriptAddon(final JavaPlugin p) { - plugin = p; - name = "" + p.getName(); - Version v; + SkriptAddon(JavaPlugin plugin) { + this.plugin = plugin; + + Version version; + String descriptionVersion = plugin.getDescription().getVersion(); try { - v = new Version("" + p.getDescription().getVersion()); + version = new Version(descriptionVersion); } catch (final IllegalArgumentException e) { - final Matcher m = Pattern.compile("(\\d+)(?:\\.(\\d+)(?:\\.(\\d+))?)?").matcher(p.getDescription().getVersion()); + Matcher m = Pattern.compile("(\\d+)(?:\\.(\\d+)(?:\\.(\\d+))?)?").matcher(descriptionVersion); if (!m.find()) - throw new IllegalArgumentException("The version of the plugin " + p.getName() + " does not contain any numbers: " + p.getDescription().getVersion()); - v = new Version(Utils.parseInt("" + m.group(1)), m.group(2) == null ? 0 : Utils.parseInt("" + m.group(2)), m.group(3) == null ? 0 : Utils.parseInt("" + m.group(3))); - Skript.warning("The plugin " + p.getName() + " uses a non-standard version syntax: '" + p.getDescription().getVersion() + "'. Skript will use " + v + " instead."); + throw new IllegalArgumentException("The version of the plugin " + plugin.getName() + " does not contain any numbers: " + descriptionVersion); + version = new Version(Utils.parseInt("" + m.group(1)), m.group(2) == null ? 0 : Utils.parseInt("" + m.group(2)), m.group(3) == null ? 0 : Utils.parseInt("" + m.group(3))); + Skript.warning("The plugin " + plugin.getName() + " uses a non-standard version syntax: '" + descriptionVersion + "'. Skript will use " + version + " instead."); } - version = v; + this.version = version; } @Override - public final String toString() { - return name; + public String toString() { + return plugin.getName(); } public String getName() { - return name; + return plugin.getName(); } - + /** - * Loads classes of the plugin by package. Useful for registering many syntax elements like Skript does it. - * - * @param basePackage The base package to add to all sub packages, e.g. "ch.njol.skript". - * @param subPackages Which subpackages of the base package should be loaded, e.g. "expressions", "conditions", "effects". Subpackages of these packages will be loaded - * as well. Use an empty array to load all subpackages of the base package. - * @throws IOException If some error occurred attempting to read the plugin's jar file. + * Loads classes of the plugin by package. Useful for registering many syntax elements like Skript. + * + * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript'). + * @param subPackages Specific subpackages to search in (e.g. 'conditions') + * If no subpackages are provided, all subpackages of the base package will be searched. + * @return This SkriptAddon + */ + public SkriptAddon loadClasses(String basePackage, String... subPackages) { + return loadClasses(null, true, basePackage, true, subPackages); + } + + /** + * Loads classes of the plugin by package. Useful for registering many syntax elements like Skript. + * + * @param withClass A consumer that will run with each found class. + * @param initialize Whether classes found in the package search should be initialized. + * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript'). + * @param recursive Whether to recursively search through the subpackages provided. + * @param subPackages Specific subpackages to search in (e.g. 'conditions') + * If no subpackages are provided, all subpackages of the base package will be searched. * @return This SkriptAddon */ - public SkriptAddon loadClasses(String basePackage, final String... subPackages) throws IOException { - assert subPackages != null; - final JarFile jar = new JarFile(getFile()); + @SuppressWarnings("ThrowableNotThrown") + public SkriptAddon loadClasses(@Nullable Consumer> withClass, boolean initialize, String basePackage, boolean recursive, String... subPackages) { for (int i = 0; i < subPackages.length; i++) subPackages[i] = subPackages[i].replace('.', '/') + "/"; basePackage = basePackage.replace('.', '/') + "/"; - try { - for (final JarEntry e : new EnumerationIterable<>(jar.entries())) { - if (e.getName().startsWith(basePackage) && e.getName().endsWith(".class")) { + + int depth = !recursive ? StringUtils.count(basePackage, '/') + 1 : 0; + + File file = getFile(); + if (file == null) { + Skript.error("Unable to retrieve file from addon '" + getName() + "'. Classes will not be loaded."); + return this; + } + + try (final JarFile jar = new JarFile(file)) { + boolean hasWithClass = withClass != null; + for (JarEntry e : new EnumerationIterable<>(jar.entries())) { + String name = e.getName(); + if (name.startsWith(basePackage) && name.endsWith(".class") && (recursive || StringUtils.count(name, '/') <= depth)) { boolean load = subPackages.length == 0; - for (final String sub : subPackages) { - if (e.getName().startsWith(sub, basePackage.length())) { + for (String subPackage : subPackages) { + if (e.getName().startsWith(subPackage, basePackage.length())) { load = true; break; } } + if (load) { - final String c = e.getName().replace('/', '.').substring(0, e.getName().length() - ".class".length()); + String c = e.getName().replace('/', '.').substring(0, e.getName().length() - ".class".length()); try { - Class.forName(c, true, plugin.getClass().getClassLoader()); - } catch (final ClassNotFoundException ex) { - Skript.exception(ex, "Cannot load class " + c + " from " + this); - } catch (final ExceptionInInitializerError err) { + Class clazz = Class.forName(c, initialize, plugin.getClass().getClassLoader()); + if (hasWithClass) + withClass.accept(clazz); + } catch (ClassNotFoundException ex) { + Skript.exception(ex, "Cannot load class " + c); + } catch (ExceptionInInitializerError err) { Skript.exception(err.getCause(), this + "'s class " + c + " generated an exception while loading"); } - continue; } } } - } finally { - try { - jar.close(); - } catch (final IOException e) {} + } catch (IOException e) { + Skript.exception(e, "Failed to load classes for addon: " + plugin.getName()); } return this; } + + /** + * Loads all module classes found in the package search. + * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript'). + * @param subPackages Specific subpackages to search in (e.g. 'conditions'). + * If no subpackages are provided, all subpackages will be searched. + * Note that the search will go no further than the first layer of subpackages. + */ + @SuppressWarnings("ThrowableNotThrown") + public SkriptAddon loadModules(String basePackage, String... subPackages) { + return loadClasses(c -> { + if (Module.class.isAssignableFrom(c) && !c.isInterface() && !Modifier.isAbstract(c.getModifiers())) { + try { + ((Module) c.getConstructor().newInstance()).register(this); + } catch (Exception e) { + Skript.exception(e, "Failed to load registration " + c); + } + } + }, false, basePackage, false, subPackages); + } @Nullable private String languageFileDirectory = null; /** - * Makes Skript load language files from the specified directory, e.g. "lang" or "skript lang" if you have a lang folder yourself. Localised files will be read from the - * plugin's jar and the plugin's data folder, but the default English file is only taken from the jar and must exist! + * Loads language files from the specified directory (e.g. "lang") into Skript. + * Localized files will be read from the plugin's jar and the plugin's data file, + * but the default.lang file is only taken from the jar and must exist! * * @param directory Directory name * @return This SkriptAddon @@ -142,7 +188,11 @@ public SkriptAddon setLanguageFileDirectory(String directory) { Language.loadDefault(this); return this; } - + + /** + * @return The language file directory set for this addon. + * It must first be set using {@link #setLanguageFileDirectory(String)}. + */ @Nullable public String getLanguageFileDirectory() { return languageFileDirectory; @@ -152,27 +202,25 @@ public String getLanguageFileDirectory() { private File file = null; /** - * @return The jar file of the plugin. The first invocation of this method uses reflection to invoke the protected method {@link JavaPlugin#getFile()} to get the plugin's jar - * file. The file is then cached and returned upon subsequent calls to this method to reduce usage of reflection. + * @return The jar file of the plugin. + * After this method is first called, the file will be cached for future use. */ @Nullable public File getFile() { if (file != null) return file; try { - final Method getFile = JavaPlugin.class.getDeclaredMethod("getFile"); + Method getFile = JavaPlugin.class.getDeclaredMethod("getFile"); getFile.setAccessible(true); file = (File) getFile.invoke(plugin); return file; - } catch (final NoSuchMethodException e) { - Skript.outdatedError(e); - } catch (final IllegalArgumentException e) { + } catch (NoSuchMethodException | IllegalArgumentException e) { Skript.outdatedError(e); - } catch (final IllegalAccessException e) { + } catch (IllegalAccessException e) { assert false; - } catch (final SecurityException e) { + } catch (SecurityException e) { throw new RuntimeException(e); - } catch (final InvocationTargetException e) { + } catch (InvocationTargetException e) { throw new RuntimeException(e.getCause()); } return null; diff --git a/src/main/java/org/skriptlang/skript/registration/Module.java b/src/main/java/org/skriptlang/skript/registration/Module.java new file mode 100644 index 00000000000..7f7f195a7cf --- /dev/null +++ b/src/main/java/org/skriptlang/skript/registration/Module.java @@ -0,0 +1,56 @@ +/** + * This file is part of Skript. + * + * Skript is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Skript is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with Skript. If not, see . + * + * Copyright Peter Güttinger, SkriptLang team and contributors + */ +package org.skriptlang.skript.registration; + +import ch.njol.skript.SkriptAddon; + +import java.io.IOException; + +/** + * A module is a part of a {@link SkriptAddon} containing related syntax, classinfos, converters, etc. + * Modules can be loaded using {@link SkriptAddon#loadModules(String, String...)}. + * Note that when loading 'org.skriptlang.skript.X', the module class should be placed at 'org.skriptlang.skript.X.ModuleClassHere' + * as the mentioned method will not search deeper than the provided subpackages. + */ +public abstract class Module { + + /** + * @param addon The addon responsible for registering this module. + * To be used for registering syntax, classinfos, etc. + */ + public abstract void register(SkriptAddon addon) throws IOException; + + /** + * Loads syntax elements for this module assuming "elements" to be the location of syntax elements. + * @param loader The SkriptAddon to load syntax with. + */ + public final void loadSyntax(SkriptAddon loader) { + loadSyntax(loader, "elements"); + } + + /** + * Loads syntax elements for this module. + * @param loader The SkriptAddon to load syntax with. + * @param packageName The location of syntax elements (ex: "elements") + */ + public final void loadSyntax(SkriptAddon loader, String packageName) { + loader.loadClasses(getClass().getPackage().getName() + "." + packageName); + } + +} From 78b61cad48a47088cfd19e7a36a9a10b922e1ac0 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Fri, 4 Mar 2022 22:06:34 -0500 Subject: [PATCH 02/13] Implement entry cache --- src/main/java/ch/njol/skript/Skript.java | 10 ++++-- src/main/java/ch/njol/skript/SkriptAddon.java | 35 +++++++++++++++++-- 2 files changed, 39 insertions(+), 6 deletions(-) diff --git a/src/main/java/ch/njol/skript/Skript.java b/src/main/java/ch/njol/skript/Skript.java index 4824395b409..85ffd2d45c7 100644 --- a/src/main/java/ch/njol/skript/Skript.java +++ b/src/main/java/ch/njol/skript/Skript.java @@ -509,8 +509,8 @@ public void onEnable() { final long tick = testing() ? Bukkit.getWorlds().get(0).getFullTime() : 0; Bukkit.getScheduler().scheduleSyncDelayedTask(this, new Runnable() { - @SuppressWarnings("synthetic-access") @Override + @SuppressWarnings("synthetic-access") public void run() { assert Bukkit.getWorlds().get(0).getFullTime() == tick; @@ -531,7 +531,7 @@ public void run() { tainted = true; getAddonInstance().loadClasses("ch.njol.skript", "tests"); } - + stopAcceptingRegistrations(); @@ -1190,10 +1190,14 @@ public static void checkAcceptRegistrations() { private static void stopAcceptingRegistrations() { acceptRegistrations = false; - + Converters.createMissingConverters(); Classes.onRegistrationsStop(); + + // Clear each cache + getAddonInstance().resetEntryCache(); // The SkriptAddon representing Skript is not part of the addons list + getAddons().forEach(SkriptAddon::resetEntryCache); } // ================ ADDONS ================ diff --git a/src/main/java/ch/njol/skript/SkriptAddon.java b/src/main/java/ch/njol/skript/SkriptAddon.java index 5879343b215..31086ae3357 100644 --- a/src/main/java/ch/njol/skript/SkriptAddon.java +++ b/src/main/java/ch/njol/skript/SkriptAddon.java @@ -22,7 +22,6 @@ import ch.njol.skript.util.Utils; import ch.njol.skript.util.Version; import ch.njol.util.StringUtils; -import ch.njol.util.coll.iterator.EnumerationIterable; import org.bukkit.plugin.java.JavaPlugin; import org.eclipse.jdt.annotation.Nullable; import org.skriptlang.skript.registration.Module; @@ -33,7 +32,9 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; +import java.util.ListIterator; import java.util.function.Consumer; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -82,6 +83,9 @@ public String getName() { /** * Loads classes of the plugin by package. Useful for registering many syntax elements like Skript. * + * Please note that if you need to load the same class multiple times, + * you should call {@link #resetEntryCache()} each time you call this method. + * * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript'). * @param subPackages Specific subpackages to search in (e.g. 'conditions') * If no subpackages are provided, all subpackages of the base package will be searched. @@ -91,9 +95,26 @@ public SkriptAddon loadClasses(String basePackage, String... subPackages) { return loadClasses(null, true, basePackage, true, subPackages); } + @Nullable + private ListIterator entryCache; + + /** + * This method resets the cache of jar entries used in {@link #loadClasses(Consumer, boolean, String, boolean, String...)}. + * This method is meant for internal use, so you probably don't need it! + * However, if you need loadClasses to load the same class multiple times, you should use this method. + * + * Note that this cache will be cleared when Skript stops accepting registrations. + */ + public void resetEntryCache() { + entryCache = null; + } + /** * Loads classes of the plugin by package. Useful for registering many syntax elements like Skript. * + * Please note that if you need to load the same class multiple times, + * you should call {@link #resetEntryCache()} each time you call this method. + * * @param withClass A consumer that will run with each found class. * @param initialize Whether classes found in the package search should be initialized. * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript'). @@ -119,7 +140,10 @@ public SkriptAddon loadClasses(@Nullable Consumer> withClass, boolean i try (JarFile jar = new JarFile(file)) { List classNames = new ArrayList<>(); boolean hasWithClass = withClass != null; - for (JarEntry e : new EnumerationIterable<>(jar.entries())) { + if (entryCache == null) + entryCache = new ArrayList<>(Arrays.asList(jar.stream().toArray(JarEntry[]::new))).listIterator(); + while (entryCache.hasNext()) { + JarEntry e = entryCache.next(); String name = e.getName(); if (name.startsWith(basePackage) && name.endsWith(".class") && (recursive || StringUtils.count(name, '/') <= depth)) { boolean load = subPackages.length == 0; @@ -129,10 +153,14 @@ public SkriptAddon loadClasses(@Nullable Consumer> withClass, boolean i break; } } - if (load) + if (load) { classNames.add(e.getName().replace('/', '.').substring(0, e.getName().length() - ".class".length())); + entryCache.remove(); // Remove this item from the entry cache as this method will only load it once + } } } + while (entryCache.hasPrevious()) // Return to the beginning for the next time this method is called + entryCache.previous(); classNames.sort(String::compareToIgnoreCase); for (String c : classNames) { try { @@ -157,6 +185,7 @@ public SkriptAddon loadClasses(@Nullable Consumer> withClass, boolean i * @param subPackages Specific subpackages to search in (e.g. 'conditions'). * If no subpackages are provided, all subpackages will be searched. * Note that the search will go no further than the first layer of subpackages. + * @return This SkriptAddon */ @SuppressWarnings("ThrowableNotThrown") public SkriptAddon loadModules(String basePackage, String... subPackages) { From d2f75b77eecf4ee74bc6f11c2e2f290ba6252e76 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Fri, 4 Mar 2022 23:25:50 -0500 Subject: [PATCH 03/13] Use an array instead for the cache --- src/main/java/ch/njol/skript/SkriptAddon.java | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/main/java/ch/njol/skript/SkriptAddon.java b/src/main/java/ch/njol/skript/SkriptAddon.java index 31086ae3357..85fd56ae52a 100644 --- a/src/main/java/ch/njol/skript/SkriptAddon.java +++ b/src/main/java/ch/njol/skript/SkriptAddon.java @@ -32,9 +32,7 @@ import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; -import java.util.ListIterator; import java.util.function.Consumer; import java.util.jar.JarEntry; import java.util.jar.JarFile; @@ -96,7 +94,7 @@ public SkriptAddon loadClasses(String basePackage, String... subPackages) { } @Nullable - private ListIterator entryCache; + private JarEntry @Nullable [] entryCache; /** * This method resets the cache of jar entries used in {@link #loadClasses(Consumer, boolean, String, boolean, String...)}. @@ -141,9 +139,11 @@ public SkriptAddon loadClasses(@Nullable Consumer> withClass, boolean i List classNames = new ArrayList<>(); boolean hasWithClass = withClass != null; if (entryCache == null) - entryCache = new ArrayList<>(Arrays.asList(jar.stream().toArray(JarEntry[]::new))).listIterator(); - while (entryCache.hasNext()) { - JarEntry e = entryCache.next(); + entryCache = jar.stream().toArray(JarEntry[]::new); + for (int i = 0; i < entryCache.length; i++) { + JarEntry e = entryCache[i]; + if (e == null) // This entry has already been loaded before + continue; String name = e.getName(); if (name.startsWith(basePackage) && name.endsWith(".class") && (recursive || StringUtils.count(name, '/') <= depth)) { boolean load = subPackages.length == 0; @@ -155,12 +155,10 @@ public SkriptAddon loadClasses(@Nullable Consumer> withClass, boolean i } if (load) { classNames.add(e.getName().replace('/', '.').substring(0, e.getName().length() - ".class".length())); - entryCache.remove(); // Remove this item from the entry cache as this method will only load it once + entryCache[i] = null; // Remove this item from the entry cache as this method will only load it once } } } - while (entryCache.hasPrevious()) // Return to the beginning for the next time this method is called - entryCache.previous(); classNames.sort(String::compareToIgnoreCase); for (String c : classNames) { try { From 9a51f9f6fbbeea4e31d4568f5490d00e0e5a9d43 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Fri, 3 Jun 2022 15:30:38 -0400 Subject: [PATCH 04/13] It should actually build now --- .../njol/skript/module/worldguard6/WorldGuard6Hook.java | 9 ++------- src/main/java/ch/njol/skript/Skript.java | 1 + 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/skript-worldguard6/src/main/java/ch/njol/skript/module/worldguard6/WorldGuard6Hook.java b/skript-worldguard6/src/main/java/ch/njol/skript/module/worldguard6/WorldGuard6Hook.java index 69bd2779eec..e582d7d470f 100644 --- a/skript-worldguard6/src/main/java/ch/njol/skript/module/worldguard6/WorldGuard6Hook.java +++ b/skript-worldguard6/src/main/java/ch/njol/skript/module/worldguard6/WorldGuard6Hook.java @@ -65,13 +65,8 @@ protected boolean init() { supportsUUIDs = Skript.methodExists(DefaultDomain.class, "getUniqueIds"); // Manually load syntaxes for regions, because we're in module package - try { - Skript.getAddonInstance().loadClasses("ch.njol.skript.hooks.regions"); - } catch (IOException e) { - Skript.exception(e); - return false; - } - return super.init(); + Skript.getAddonInstance().loadClasses("ch.njol.skript.hooks.regions"); + return super.init(); } @Override diff --git a/src/main/java/ch/njol/skript/Skript.java b/src/main/java/ch/njol/skript/Skript.java index 37d96211f25..65550f6ee9e 100644 --- a/src/main/java/ch/njol/skript/Skript.java +++ b/src/main/java/ch/njol/skript/Skript.java @@ -115,6 +115,7 @@ import java.lang.Thread.UncaughtExceptionHandler; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; +import java.lang.reflect.Modifier; import java.net.URL; import java.nio.charset.StandardCharsets; import java.nio.file.Files; From 3767632738eb2d8b6a1b657b4ad601e62f245378 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Mon, 6 Jun 2022 18:49:26 -0400 Subject: [PATCH 05/13] Further documentation updates --- src/main/java/ch/njol/skript/SkriptAddon.java | 10 ++++---- .../skript/registration/Module.java | 23 ++++++++++++------- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/main/java/ch/njol/skript/SkriptAddon.java b/src/main/java/ch/njol/skript/SkriptAddon.java index 85fd56ae52a..20f96a8b834 100644 --- a/src/main/java/ch/njol/skript/SkriptAddon.java +++ b/src/main/java/ch/njol/skript/SkriptAddon.java @@ -87,7 +87,7 @@ public String getName() { * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript'). * @param subPackages Specific subpackages to search in (e.g. 'conditions') * If no subpackages are provided, all subpackages of the base package will be searched. - * @return This SkriptAddon + * @return This SkriptAddon. */ public SkriptAddon loadClasses(String basePackage, String... subPackages) { return loadClasses(null, true, basePackage, true, subPackages); @@ -183,7 +183,9 @@ public SkriptAddon loadClasses(@Nullable Consumer> withClass, boolean i * @param subPackages Specific subpackages to search in (e.g. 'conditions'). * If no subpackages are provided, all subpackages will be searched. * Note that the search will go no further than the first layer of subpackages. - * @return This SkriptAddon + * Note that this method will also clear the entry cache of ALL checked classes, + * even those that are not actually a Module. + * @return This SkriptAddon. */ @SuppressWarnings("ThrowableNotThrown") public SkriptAddon loadModules(String basePackage, String... subPackages) { @@ -206,8 +208,8 @@ public SkriptAddon loadModules(String basePackage, String... subPackages) { * Localized files will be read from the plugin's jar and the plugin's data file, * but the default.lang file is only taken from the jar and must exist! * - * @param directory Directory name - * @return This SkriptAddon + * @param directory The directory containing language files. + * @return This SkriptAddon. */ public SkriptAddon setLanguageFileDirectory(String directory) { if (languageFileDirectory != null) diff --git a/src/main/java/org/skriptlang/skript/registration/Module.java b/src/main/java/org/skriptlang/skript/registration/Module.java index 7f7f195a7cf..80cd03ca764 100644 --- a/src/main/java/org/skriptlang/skript/registration/Module.java +++ b/src/main/java/org/skriptlang/skript/registration/Module.java @@ -24,9 +24,22 @@ /** * A module is a part of a {@link SkriptAddon} containing related syntax, classinfos, converters, etc. + * They are intended for providing organization and structure. * Modules can be loaded using {@link SkriptAddon#loadModules(String, String...)}. * Note that when loading 'org.skriptlang.skript.X', the module class should be placed at 'org.skriptlang.skript.X.ModuleClassHere' * as the mentioned method will not search deeper than the provided subpackages. + * The example below is the structure that a project using Modules should use. + *
+ * potions
+ * |- elements
+ *    |- PotionsExpr.java
+ * |- PotionsModule.java
+ * math
+ * |- elements
+ *    |- MathExpr.java
+ * |- MathModule.java
+ * MyPlugin.java
+ * 
*/ public abstract class Module { @@ -36,18 +49,12 @@ public abstract class Module { */ public abstract void register(SkriptAddon addon) throws IOException; - /** - * Loads syntax elements for this module assuming "elements" to be the location of syntax elements. - * @param loader The SkriptAddon to load syntax with. - */ - public final void loadSyntax(SkriptAddon loader) { - loadSyntax(loader, "elements"); - } - /** * Loads syntax elements for this module. * @param loader The SkriptAddon to load syntax with. * @param packageName The location of syntax elements (ex: "elements") + * Elements should **not** be contained within the main module package. + * They should be within a subpackage of the package containing the Module class. */ public final void loadSyntax(SkriptAddon loader, String packageName) { loader.loadClasses(getClass().getPackage().getName() + "." + packageName); From 2a3f6d006db0b74dddeeeaa0489f9498f53ca1f5 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Fri, 5 Aug 2022 13:57:11 -0400 Subject: [PATCH 06/13] Improvements and fixes from review --- src/main/java/ch/njol/skript/Skript.java | 6 +- src/main/java/ch/njol/skript/SkriptAddon.java | 98 +++++++++++-------- .../skript/registration/Module.java | 6 +- 3 files changed, 62 insertions(+), 48 deletions(-) diff --git a/src/main/java/ch/njol/skript/Skript.java b/src/main/java/ch/njol/skript/Skript.java index b18b7ce24e8..f5a8251c394 100644 --- a/src/main/java/ch/njol/skript/Skript.java +++ b/src/main/java/ch/njol/skript/Skript.java @@ -42,8 +42,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; -import java.util.jar.JarEntry; -import java.util.jar.JarFile; import java.util.logging.Filter; import java.util.logging.Level; import java.util.regex.Matcher; @@ -519,7 +517,7 @@ public void run() { assert Bukkit.getWorlds().get(0).getFullTime() == tick; // Load hooks from Skript jar - getAddonInstance().loadClasses(hook -> { + getAddonInstance().loadClasses("ch.njol.skript.hooks", false, false, hook -> { //noinspection unchecked - We made sure that it's a valid hook if (Hook.class.isAssignableFrom(hook) && !hook.isInterface() && !Modifier.isAbstract(hook.getModifiers()) && isHookEnabled((Class>) hook)) { try { @@ -530,7 +528,7 @@ public void run() { Skript.exception(ex, "Failed to load hook class " + hook); } } - }, false, "ch.njol.skript.hooks", false); + }); finishedLoadingHooks = true; if (TestMode.ENABLED) { diff --git a/src/main/java/ch/njol/skript/SkriptAddon.java b/src/main/java/ch/njol/skript/SkriptAddon.java index 20f96a8b834..2aca8bd8b1c 100644 --- a/src/main/java/ch/njol/skript/SkriptAddon.java +++ b/src/main/java/ch/njol/skript/SkriptAddon.java @@ -90,14 +90,13 @@ public String getName() { * @return This SkriptAddon. */ public SkriptAddon loadClasses(String basePackage, String... subPackages) { - return loadClasses(null, true, basePackage, true, subPackages); + return loadClasses(basePackage, true, true, null, subPackages); } - @Nullable private JarEntry @Nullable [] entryCache; /** - * This method resets the cache of jar entries used in {@link #loadClasses(Consumer, boolean, String, boolean, String...)}. + * This method resets the cache of jar entries used in {@link #loadClasses(String, boolean, boolean, Consumer, String...)}. * This method is meant for internal use, so you probably don't need it! * However, if you need loadClasses to load the same class multiple times, you should use this method. * @@ -109,25 +108,27 @@ public void resetEntryCache() { /** * Loads classes of the plugin by package. Useful for registering many syntax elements like Skript. - * + *

* Please note that if you need to load the same class multiple times, * you should call {@link #resetEntryCache()} each time you call this method. * - * @param withClass A consumer that will run with each found class. - * @param initialize Whether classes found in the package search should be initialized. * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript'). - * @param recursive Whether to recursively search through the subpackages provided. + * @param initialize Whether classes found in the package search should be initialized. + * @param recursive Whether to recursively search through the subpackages provided. + * @param withClass A consumer that will run with each found class. * @param subPackages Specific subpackages to search in (e.g. 'conditions') * If no subpackages are provided, all subpackages of the base package will be searched. * @return This SkriptAddon */ @SuppressWarnings("ThrowableNotThrown") - public SkriptAddon loadClasses(@Nullable Consumer> withClass, boolean initialize, String basePackage, boolean recursive, String... subPackages) { + public SkriptAddon loadClasses(String basePackage, boolean initialize, boolean recursive, @Nullable Consumer> withClass, String... subPackages) { for (int i = 0; i < subPackages.length; i++) subPackages[i] = subPackages[i].replace('.', '/') + "/"; basePackage = basePackage.replace('.', '/') + "/"; - int depth = !recursive ? StringUtils.count(basePackage, '/') + 1 : 0; + // Used for tracking valid classes if a non-recursive search is done + // Depth is the measure of how "deep" from the head package of 'basePackage' a class is + int initialDepth = !recursive ? StringUtils.count(basePackage, '/') + 1 : 0; File file = getFile(); if (file == null) { @@ -136,44 +137,59 @@ public SkriptAddon loadClasses(@Nullable Consumer> withClass, boolean i } try (JarFile jar = new JarFile(file)) { - List classNames = new ArrayList<>(); - boolean hasWithClass = withClass != null; if (entryCache == null) entryCache = jar.stream().toArray(JarEntry[]::new); - for (int i = 0; i < entryCache.length; i++) { - JarEntry e = entryCache[i]; - if (e == null) // This entry has already been loaded before - continue; - String name = e.getName(); - if (name.startsWith(basePackage) && name.endsWith(".class") && (recursive || StringUtils.count(name, '/') <= depth)) { - boolean load = subPackages.length == 0; + } catch (IOException e) { + Skript.exception(e, "Failed to load classes for addon: " + plugin.getName()); + return this; + } + + List classNames = new ArrayList<>(); + for (int i = 0; i < entryCache.length; i++) { + JarEntry e = entryCache[i]; + if (e == null) // This entry has already been loaded before + continue; + + String name = e.getName(); + if (name.startsWith(basePackage) && name.endsWith(".class")) { + boolean load = subPackages.length == 0; + + if (load) { // No subpackages provided + load = recursive || StringUtils.count(name, '/') <= initialDepth; + } else { for (String subPackage : subPackages) { - if (e.getName().startsWith(subPackage, basePackage.length())) { + if ( + // We also need to account for subpackage depths when not doing a recursive search + (recursive || StringUtils.count(name, '/') <= initialDepth + StringUtils.count(subPackage, '/')) + && name.startsWith(subPackage, basePackage.length()) + ) { load = true; break; } } - if (load) { - classNames.add(e.getName().replace('/', '.').substring(0, e.getName().length() - ".class".length())); - entryCache[i] = null; // Remove this item from the entry cache as this method will only load it once - } } - } - classNames.sort(String::compareToIgnoreCase); - for (String c : classNames) { - try { - Class clazz = Class.forName(c, initialize, plugin.getClass().getClassLoader()); - if (hasWithClass) - withClass.accept(clazz); - } catch (ClassNotFoundException ex) { - Skript.exception(ex, "Cannot load class " + c); - } catch (ExceptionInInitializerError err) { - Skript.exception(err.getCause(), this + "'s class " + c + " generated an exception while loading"); + + if (load) { + classNames.add(name.replace('/', '.').substring(0, name.length() - ".class".length())); + entryCache[i] = null; // Remove this item from the entry cache as this method will only load it once } } - } catch (IOException e) { - Skript.exception(e, "Failed to load classes for addon: " + plugin.getName()); } + + classNames.sort(String::compareToIgnoreCase); + + for (String className : classNames) { + try { + Class clazz = Class.forName(className, initialize, plugin.getClass().getClassLoader()); + if (withClass != null) + withClass.accept(clazz); + } catch (ClassNotFoundException ex) { + Skript.exception(ex, "Cannot load class " + className); + } catch (ExceptionInInitializerError err) { + Skript.exception(err.getCause(), this + "'s class " + className + " generated an exception while loading"); + } + } + return this; } @@ -189,15 +205,15 @@ public SkriptAddon loadClasses(@Nullable Consumer> withClass, boolean i */ @SuppressWarnings("ThrowableNotThrown") public SkriptAddon loadModules(String basePackage, String... subPackages) { - return loadClasses(c -> { + return loadClasses(basePackage, false, false, c -> { if (Module.class.isAssignableFrom(c) && !c.isInterface() && !Modifier.isAbstract(c.getModifiers())) { try { ((Module) c.getConstructor().newInstance()).register(this); } catch (Exception e) { - Skript.exception(e, "Failed to load registration " + c); + Skript.exception(e, "Failed to load module " + c); } } - }, false, basePackage, false, subPackages); + }, subPackages); } @Nullable @@ -213,7 +229,7 @@ public SkriptAddon loadModules(String basePackage, String... subPackages) { */ public SkriptAddon setLanguageFileDirectory(String directory) { if (languageFileDirectory != null) - throw new IllegalStateException(); + throw new IllegalStateException("The language file directory may only be set once."); directory = "" + directory.replace('\\', '/'); if (directory.endsWith("/")) directory = "" + directory.substring(0, directory.length() - 1); @@ -224,7 +240,7 @@ public SkriptAddon setLanguageFileDirectory(String directory) { /** * @return The language file directory set for this addon. - * It must first be set using {@link #setLanguageFileDirectory(String)}. + * Null if not yet set using {@link #setLanguageFileDirectory(String)}. */ @Nullable public String getLanguageFileDirectory() { diff --git a/src/main/java/org/skriptlang/skript/registration/Module.java b/src/main/java/org/skriptlang/skript/registration/Module.java index 80cd03ca764..4b57fcb12ec 100644 --- a/src/main/java/org/skriptlang/skript/registration/Module.java +++ b/src/main/java/org/skriptlang/skript/registration/Module.java @@ -52,12 +52,12 @@ public abstract class Module { /** * Loads syntax elements for this module. * @param loader The SkriptAddon to load syntax with. - * @param packageName The location of syntax elements (ex: "elements") + * @param subPackageName The location of syntax elements (ex: "elements") * Elements should **not** be contained within the main module package. * They should be within a subpackage of the package containing the Module class. */ - public final void loadSyntax(SkriptAddon loader, String packageName) { - loader.loadClasses(getClass().getPackage().getName() + "." + packageName); + public void loadSyntax(SkriptAddon loader, String subPackageName) { + loader.loadClasses(getClass().getPackage().getName() + "." + subPackageName); } } From 803eeb9dd24944027af070bac8621163268c9231 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Fri, 5 Aug 2022 13:58:54 -0400 Subject: [PATCH 07/13] Only open JarFile if necessary --- src/main/java/ch/njol/skript/SkriptAddon.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/main/java/ch/njol/skript/SkriptAddon.java b/src/main/java/ch/njol/skript/SkriptAddon.java index 2aca8bd8b1c..501931a2c32 100644 --- a/src/main/java/ch/njol/skript/SkriptAddon.java +++ b/src/main/java/ch/njol/skript/SkriptAddon.java @@ -136,12 +136,13 @@ public SkriptAddon loadClasses(String basePackage, boolean initialize, boolean r return this; } - try (JarFile jar = new JarFile(file)) { - if (entryCache == null) + if (entryCache == null) { + try (JarFile jar = new JarFile(file)) { entryCache = jar.stream().toArray(JarEntry[]::new); - } catch (IOException e) { - Skript.exception(e, "Failed to load classes for addon: " + plugin.getName()); - return this; + } catch (IOException e) { + Skript.exception(e, "Failed to load classes for addon: " + plugin.getName()); + return this; + } } List classNames = new ArrayList<>(); From c08c4a60c4313a1aecdf4a3f7efa0006beb4f5a2 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Sun, 21 Aug 2022 14:46:02 -0400 Subject: [PATCH 08/13] Make entry cache not contain null elements This implements the "current" behavior --- src/main/java/ch/njol/skript/SkriptAddon.java | 25 +++++-------------- 1 file changed, 6 insertions(+), 19 deletions(-) diff --git a/src/main/java/ch/njol/skript/SkriptAddon.java b/src/main/java/ch/njol/skript/SkriptAddon.java index 501931a2c32..d59f2238944 100644 --- a/src/main/java/ch/njol/skript/SkriptAddon.java +++ b/src/main/java/ch/njol/skript/SkriptAddon.java @@ -81,9 +81,6 @@ public String getName() { /** * Loads classes of the plugin by package. Useful for registering many syntax elements like Skript. * - * Please note that if you need to load the same class multiple times, - * you should call {@link #resetEntryCache()} each time you call this method. - * * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript'). * @param subPackages Specific subpackages to search in (e.g. 'conditions') * If no subpackages are provided, all subpackages of the base package will be searched. @@ -93,24 +90,17 @@ public SkriptAddon loadClasses(String basePackage, String... subPackages) { return loadClasses(basePackage, true, true, null, subPackages); } - private JarEntry @Nullable [] entryCache; + private JarEntry @Nullable [] entryCache = null; /** - * This method resets the cache of jar entries used in {@link #loadClasses(String, boolean, boolean, Consumer, String...)}. - * This method is meant for internal use, so you probably don't need it! - * However, if you need loadClasses to load the same class multiple times, you should use this method. - * - * Note that this cache will be cleared when Skript stops accepting registrations. + * Internal method for clearing an addon's entry cache. */ - public void resetEntryCache() { + void resetEntryCache() { entryCache = null; } /** * Loads classes of the plugin by package. Useful for registering many syntax elements like Skript. - *

- * Please note that if you need to load the same class multiple times, - * you should call {@link #resetEntryCache()} each time you call this method. * * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript'). * @param initialize Whether classes found in the package search should be initialized. @@ -146,8 +136,7 @@ public SkriptAddon loadClasses(String basePackage, boolean initialize, boolean r } List classNames = new ArrayList<>(); - for (int i = 0; i < entryCache.length; i++) { - JarEntry e = entryCache[i]; + for (JarEntry e : entryCache) { if (e == null) // This entry has already been loaded before continue; @@ -162,7 +151,7 @@ public SkriptAddon loadClasses(String basePackage, boolean initialize, boolean r if ( // We also need to account for subpackage depths when not doing a recursive search (recursive || StringUtils.count(name, '/') <= initialDepth + StringUtils.count(subPackage, '/')) - && name.startsWith(subPackage, basePackage.length()) + && name.startsWith(subPackage, basePackage.length()) ) { load = true; break; @@ -170,10 +159,8 @@ public SkriptAddon loadClasses(String basePackage, boolean initialize, boolean r } } - if (load) { + if (load) classNames.add(name.replace('/', '.').substring(0, name.length() - ".class".length())); - entryCache[i] = null; // Remove this item from the entry cache as this method will only load it once - } } } From 10973c4f34e1a0383d193e91979c413821fce463 Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Sun, 11 Dec 2022 16:45:14 -0500 Subject: [PATCH 09/13] Change default syntax locations --- src/main/java/ch/njol/skript/Skript.java | 2 +- src/main/java/org/skriptlang/skript/registration/Module.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/ch/njol/skript/Skript.java b/src/main/java/ch/njol/skript/Skript.java index f48c3e138a2..c6cb025cbe2 100644 --- a/src/main/java/ch/njol/skript/Skript.java +++ b/src/main/java/ch/njol/skript/Skript.java @@ -528,7 +528,7 @@ public void onEnable() { ChatMessages.registerListeners(); getAddonInstance().loadClasses("ch.njol.skript", "conditions", "effects", "events", "expressions", "entity", "sections") - .loadModules("org.skriptlang.skript"); + .loadModules("org.skriptlang", "skript.syntax", "skriptbukkit"); Commands.registerListeners(); diff --git a/src/main/java/org/skriptlang/skript/registration/Module.java b/src/main/java/org/skriptlang/skript/registration/Module.java index 4b57fcb12ec..2a40e455759 100644 --- a/src/main/java/org/skriptlang/skript/registration/Module.java +++ b/src/main/java/org/skriptlang/skript/registration/Module.java @@ -28,7 +28,7 @@ * Modules can be loaded using {@link SkriptAddon#loadModules(String, String...)}. * Note that when loading 'org.skriptlang.skript.X', the module class should be placed at 'org.skriptlang.skript.X.ModuleClassHere' * as the mentioned method will not search deeper than the provided subpackages. - * The example below is the structure that a project using Modules should use. + * The example below is a possible organization structure that a project using Modules could use. *

  * potions
  * |- elements
@@ -53,7 +53,7 @@ public abstract class Module {
 	 * Loads syntax elements for this module.
 	 * @param loader The SkriptAddon to load syntax with.
 	 * @param subPackageName The location of syntax elements (ex: "elements")
-	 *                    Elements should **not** be contained within the main module package.
+	 *                    Elements should not be contained within the main module package.
 	 *                    They should be within a subpackage of the package containing the Module class.
 	 */
 	public void loadSyntax(SkriptAddon loader, String subPackageName) {

From d6206e28e47edfc159fee31fbfeb4b6db48a6828 Mon Sep 17 00:00:00 2001
From: APickledWalrus 
Date: Sun, 11 Dec 2022 16:54:43 -0500
Subject: [PATCH 10/13] Fix loading

Forgot to update for Structure API
---
 src/main/java/ch/njol/skript/Skript.java | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/main/java/ch/njol/skript/Skript.java b/src/main/java/ch/njol/skript/Skript.java
index c6cb025cbe2..58146de8e53 100644
--- a/src/main/java/ch/njol/skript/Skript.java
+++ b/src/main/java/ch/njol/skript/Skript.java
@@ -527,7 +527,7 @@ public void onEnable() {
 		
 		ChatMessages.registerListeners();
 
-		getAddonInstance().loadClasses("ch.njol.skript", "conditions", "effects", "events", "expressions", "entity", "sections")
+		getAddonInstance().loadClasses("ch.njol.skript", "conditions", "effects", "events", "expressions", "entity", "sections", "structures")
 			.loadModules("org.skriptlang", "skript.syntax", "skriptbukkit");
 
 		Commands.registerListeners();

From 4ac2d16eed7abc590b8270a2b06fce9e84154b50 Mon Sep 17 00:00:00 2001
From: APickledWalrus 
Date: Tue, 27 Dec 2022 10:38:01 -0500
Subject: [PATCH 11/13] Remove throws clause

It's not guaranteed
---
 src/main/java/org/skriptlang/skript/registration/Module.java | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/src/main/java/org/skriptlang/skript/registration/Module.java b/src/main/java/org/skriptlang/skript/registration/Module.java
index 2a40e455759..07bb9516d55 100644
--- a/src/main/java/org/skriptlang/skript/registration/Module.java
+++ b/src/main/java/org/skriptlang/skript/registration/Module.java
@@ -20,8 +20,6 @@
 
 import ch.njol.skript.SkriptAddon;
 
-import java.io.IOException;
-
 /**
  * A module is a part of a {@link SkriptAddon} containing related syntax, classinfos, converters, etc.
  * They are intended for providing organization and structure.
@@ -47,7 +45,7 @@ public abstract class Module {
 	 * @param addon The addon responsible for registering this module.
 	 *              To be used for registering syntax, classinfos, etc.
 	 */
-	public abstract void register(SkriptAddon addon) throws IOException;
+	public abstract void register(SkriptAddon addon);
 
 	/**
 	 * Loads syntax elements for this module.

From 27122857ab9ec38d5bf466275f317c27a533eb42 Mon Sep 17 00:00:00 2001
From: APickledWalrus 
Date: Fri, 20 Jan 2023 19:02:53 -0500
Subject: [PATCH 12/13] Convert Module to Interface

and formatting improvements
---
 src/main/java/ch/njol/skript/SkriptAddon.java | 43 ++++++++++---------
 .../skript/registration/Module.java           | 12 +++---
 2 files changed, 28 insertions(+), 27 deletions(-)

diff --git a/src/main/java/ch/njol/skript/SkriptAddon.java b/src/main/java/ch/njol/skript/SkriptAddon.java
index d59f2238944..45d51b75625 100644
--- a/src/main/java/ch/njol/skript/SkriptAddon.java
+++ b/src/main/java/ch/njol/skript/SkriptAddon.java
@@ -68,22 +68,25 @@ public final class SkriptAddon {
 		}
 		this.version = version;
 	}
-	
-	@Override
-	public String toString() {
-		return plugin.getName();
-	}
-	
+
+	/**
+	 * @return The name of the {@link JavaPlugin} responsible for this addon.
+	 */
 	public String getName() {
 		return plugin.getName();
 	}
 
+	@Override
+	public String toString() {
+		return getName();
+	}
+
 	/**
 	 * Loads classes of the plugin by package. Useful for registering many syntax elements like Skript.
 	 *
 	 * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript').
 	 * @param subPackages Specific subpackages to search in (e.g. 'conditions')
-	 *                    If no subpackages are provided, all subpackages of the base package will be searched.
+	 *  If no subpackages are provided, all subpackages of the base package will be searched.
 	 * @return This SkriptAddon.
 	 */
 	public SkriptAddon loadClasses(String basePackage, String... subPackages) {
@@ -103,11 +106,11 @@ void resetEntryCache() {
 	 * Loads classes of the plugin by package. Useful for registering many syntax elements like Skript.
 	 *
 	 * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript').
-	 * @param initialize  Whether classes found in the package search should be initialized.
-	 * @param recursive   Whether to recursively search through the subpackages provided.
-	 * @param withClass   A consumer that will run with each found class.
+	 * @param initialize Whether classes found in the package search should be initialized.
+	 * @param recursive Whether to recursively search through the subpackages provided.
+	 * @param withClass A consumer that will run with each found class.
 	 * @param subPackages Specific subpackages to search in (e.g. 'conditions')
-	 *                    If no subpackages are provided, all subpackages of the base package will be searched.
+	 *  If no subpackages are provided, all subpackages of the base package will be searched.
 	 * @return This SkriptAddon
 	 */
 	@SuppressWarnings("ThrowableNotThrown")
@@ -136,11 +139,11 @@ public SkriptAddon loadClasses(String basePackage, boolean initialize, boolean r
 		}
 
 		List classNames = new ArrayList<>();
-		for (JarEntry e : entryCache) {
-			if (e == null) // This entry has already been loaded before
+		for (JarEntry entry : entryCache) {
+			if (entry == null) // This entry has already been loaded before
 				continue;
 
-			String name = e.getName();
+			String name = entry.getName();
 			if (name.startsWith(basePackage) && name.endsWith(".class")) {
 				boolean load = subPackages.length == 0;
 
@@ -151,7 +154,7 @@ public SkriptAddon loadClasses(String basePackage, boolean initialize, boolean r
 						if (
 							// We also need to account for subpackage depths when not doing a recursive search
 							(recursive || StringUtils.count(name, '/') <= initialDepth + StringUtils.count(subPackage, '/'))
-								&& name.startsWith(subPackage, basePackage.length())
+							&& name.startsWith(subPackage, basePackage.length())
 						) {
 							load = true;
 							break;
@@ -185,10 +188,8 @@ public SkriptAddon loadClasses(String basePackage, boolean initialize, boolean r
 	 * Loads all module classes found in the package search.
 	 * @param basePackage The base package to start searching in (e.g. 'ch.njol.skript').
 	 * @param subPackages Specific subpackages to search in (e.g. 'conditions').
-	 *                    If no subpackages are provided, all subpackages will be searched.
-	 *                    Note that the search will go no further than the first layer of subpackages.
-	 *                    Note that this method will also clear the entry cache of ALL checked classes,
-	 *                    	even those that are not actually a Module.
+	 * If no subpackages are provided, all subpackages will be searched.
+	 * Note that the search will go no further than the first layer of subpackages.
 	 * @return This SkriptAddon.
 	 */
 	@SuppressWarnings("ThrowableNotThrown")
@@ -210,7 +211,7 @@ public SkriptAddon loadModules(String basePackage, String... subPackages) {
 	/**
 	 * Loads language files from the specified directory (e.g. "lang") into Skript.
 	 * Localized files will be read from the plugin's jar and the plugin's data file,
-	 * but the default.lang file is only taken from the jar and must exist!
+	 *  but the default.lang file is only taken from the jar and must exist!
 	 * 
 	 * @param directory The directory containing language files.
 	 * @return This SkriptAddon.
@@ -240,7 +241,7 @@ public String getLanguageFileDirectory() {
 	
 	/**
 	 * @return The jar file of the plugin.
-	 * 			After this method is first called, the file will be cached for future use.
+	 * After this method is first called, the file will be cached for future use.
 	 */
 	@Nullable
 	public File getFile() {
diff --git a/src/main/java/org/skriptlang/skript/registration/Module.java b/src/main/java/org/skriptlang/skript/registration/Module.java
index 07bb9516d55..158deffc009 100644
--- a/src/main/java/org/skriptlang/skript/registration/Module.java
+++ b/src/main/java/org/skriptlang/skript/registration/Module.java
@@ -39,22 +39,22 @@
  * MyPlugin.java
  * 
*/ -public abstract class Module { +public interface Module { /** * @param addon The addon responsible for registering this module. - * To be used for registering syntax, classinfos, etc. + * To be used for registering syntax, classinfos, etc. */ - public abstract void register(SkriptAddon addon); + void register(SkriptAddon addon); /** * Loads syntax elements for this module. * @param loader The SkriptAddon to load syntax with. * @param subPackageName The location of syntax elements (ex: "elements") - * Elements should not be contained within the main module package. - * They should be within a subpackage of the package containing the Module class. + * Elements should not be contained within the main module package. + * They should be within a subpackage of the package containing the Module class. */ - public void loadSyntax(SkriptAddon loader, String subPackageName) { + default void loadSyntax(SkriptAddon loader, String subPackageName) { loader.loadClasses(getClass().getPackage().getName() + "." + subPackageName); } From 66de7cd59a79fcffaee70fb1e67433dd34698a2b Mon Sep 17 00:00:00 2001 From: APickledWalrus Date: Fri, 20 Jan 2023 19:06:16 -0500 Subject: [PATCH 13/13] Change default syntax locations "common" and "bukkit" --- src/main/java/ch/njol/skript/Skript.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/ch/njol/skript/Skript.java b/src/main/java/ch/njol/skript/Skript.java index 58146de8e53..85ad37f6057 100644 --- a/src/main/java/ch/njol/skript/Skript.java +++ b/src/main/java/ch/njol/skript/Skript.java @@ -528,7 +528,7 @@ public void onEnable() { ChatMessages.registerListeners(); getAddonInstance().loadClasses("ch.njol.skript", "conditions", "effects", "events", "expressions", "entity", "sections", "structures") - .loadModules("org.skriptlang", "skript.syntax", "skriptbukkit"); + .loadModules("org.skriptlang.skript", "common", "bukkit"); Commands.registerListeners();