Update build.yml action and enhance hook loading

.github/workflows/build.yml

@@ -9,19 +9,20 @@ on:
 
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
-  cancel-in-progress: true
+  cancel-in-progress: ${{ github.ref_name != 'main' }}
 
 jobs:
   build:
     name: Build and Publish zMenu
-    uses: GroupeZ-dev/actions/.github/workflows/build.yml@main
+    uses: GroupeZ-dev/actions/.github/workflows/build.yml@v1.0.0
     with:
       project-name: "zMenu"
       java-version: '25'
       publish: true
       project-to-publish: "API:publish"
-      build-command: './gradlew build'
-      discord-avatar-url: "https://minecraft-inventory-builder.com/storage/images/9UgcfGZyrmbVrXw5lbj5kXq6fW8F4nhwj6Cx4nVG.png"
+      discord-avatar-url: "https://groupez.dev/storage/images/253.png"
+      artifact-glob: "target/*.jar"
+      changelog-exclude-pattern: '^[a-f0-9]+\s+(build|chore)\(deps\)'
     secrets:
       WEBHOOK_URL: ${{ secrets.WEBHOOK_URL }}
       MAVEN_USERNAME: ${{ secrets.MAVEN_USERNAME }}

API/src/main/java/fr/maxlego08/menu/api/PacketManager.java

@@ -10,6 +10,10 @@ public interface PacketManager {
 
     void onEnable();
 
+    default boolean isReady() {
+        return true;
+    }
+
     void onPostEnable();
 
     void onDisable();

API/src/main/java/fr/maxlego08/menu/api/loader/ClassRegistry.java

@@ -2,6 +2,7 @@
 
 import org.bukkit.plugin.Plugin;
 
+import java.lang.reflect.InvocationTargetException;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.function.Consumer;
@@ -37,27 +38,38 @@ public ClassRegistry<T, P> tryNoArgsConstructor() {
     }
 
     public boolean load(P plugin, Class<?> clazz) {
+
+        Throwable failure = null;
+
         for (ConstructorStrategy<P> strategy : this.strategies) {
             try {
                 Object instance = strategy.instantiate(clazz, plugin);
                 if (this.expectedType.isInstance(instance)) {
                     this.registrar.accept(this.expectedType.cast(instance));
                     return true;
                 }
-            } catch (Exception ignored) {
+            } catch (NoSuchMethodException ignored) {
+            } catch (InvocationTargetException exception) {
+                if (failure == null) failure = exception.getCause() == null ? exception : exception.getCause();
+            } catch (Throwable throwable) {
+                if (failure == null) failure = throwable;
             }
         }
 
         if (this.errorLogger != null) {
-            StringBuilder stringBuilder = new StringBuilder();
-            stringBuilder.append("Could not find a valid constructor for ").append(clazz.getName()).append(". Available constructors: ");
-            for (int i = 0; i < clazz.getDeclaredConstructors().length; i++) {
-                stringBuilder.append(clazz.getDeclaredConstructors()[i]);
-                if (i < clazz.getDeclaredConstructors().length - 1) {
-                    stringBuilder.append(", ");
+            if (failure != null) {
+                this.errorLogger.accept("Could not load " + clazz.getName() + ", its constructor threw " + failure + ". This usually means the plugin it hooks into is missing or failed to start.");
+            } else {
+                StringBuilder stringBuilder = new StringBuilder();
+                stringBuilder.append("Could not find a valid constructor for ").append(clazz.getName()).append(". Available constructors: ");
+                for (int i = 0; i < clazz.getDeclaredConstructors().length; i++) {
+                    stringBuilder.append(clazz.getDeclaredConstructors()[i]);
+                    if (i < clazz.getDeclaredConstructors().length - 1) {
+                        stringBuilder.append(", ");
+                    }
                 }
+                this.errorLogger.accept(stringBuilder.toString());
             }
-            this.errorLogger.accept(stringBuilder.toString());
         }
 
         return false;

API/src/main/java/fr/maxlego08/menu/api/utils/version/VersionFilter.java

@@ -36,9 +36,15 @@ public static boolean passes(@NotNull Class<?> clazz) {
 
             String requiredVersionStr = requiresPlugin.version();
             if (!requiredVersionStr.isEmpty()) {
-                PluginVersion pluginVersion = PluginVersion.parse(plugin.getDescription().getVersion());
-                PluginVersion requiredVersion = PluginVersion.parse(requiredVersionStr);
-                if (!requiresPlugin.type().compare(pluginVersion.compareTo(requiredVersion))) {
+                try {
+                    PluginVersion pluginVersion = PluginVersion.parse(plugin.getDescription().getVersion());
+                    PluginVersion requiredVersion = PluginVersion.parse(requiredVersionStr);
+                    if (!requiresPlugin.type().compare(pluginVersion.compareTo(requiredVersion))) {
+                        return false;
+                    }
+                } catch (Throwable throwable) {
+                    // An unparsable version from a third-party plugin must not abort the whole scan.
+                    Logger.info("Could not compare the version of " + requiresPlugin.value() + " for " + clazz.getName() + ", skipping it: " + throwable.getMessage(), Logger.LogType.WARNING);
                     return false;
                 }
             }
@@ -79,15 +85,16 @@ public static <T, A extends Annotation, P extends Plugin> int scanAndRegister(
         int count = 0;
 
         for (Class<?> clazz : reflection.getTypesAnnotatedWith(annotation)) {
-            if (!registry.getExpectedType().isAssignableFrom(clazz)) continue;
-            if (!passes(clazz)) continue;
             try {
+                if (!registry.getExpectedType().isAssignableFrom(clazz)) continue;
+                if (!passes(clazz)) continue;
                 if (registry.load(plugin, clazz)) count++;
-            } catch (Exception e) {
-                if (Configuration.enableDebug) {
-                    Logger.error("Failed to load class " + clazz.getName() + " for plugin " + plugin.getName() + " with annotation " + annotation.getSimpleName() + " due to: " + e.getMessage() + ". Please check reporte this error to the plugin developer (" + plugin.getDescription().getAuthors() + ")");
-                    Logger.error(e);
-                }
+            } catch (Throwable throwable) {
+                // Throwable, not Exception: a hook whose plugin is missing or broken fails with
+                // NoClassDefFoundError or ExceptionInInitializerError, and one of those must not
+                // stop the remaining classes from being registered.
+                Logger.error("Failed to load class " + clazz.getName() + " for plugin " + plugin.getName() + " with annotation " + annotation.getSimpleName() + " due to: " + throwable + ". The rest of zMenu is unaffected, please report this to the plugin developer (" + plugin.getDescription().getAuthors() + ")");
+                if (Configuration.enableDebug) Logger.error(throwable);
             }
         }
 

Common/build.gradle.kts

@@ -2,4 +2,5 @@ dependencies {
     api(projects.api)
     api(projects.nms.base)
     compileOnly(libs.paper.api)
-}
+    testImplementation(libs.paper.api)
+}

Common/src/test/java/fr/maxlego08/menu/test/common/ClassRegistryTest.java

@@ -0,0 +1,96 @@
+package fr.maxlego08.menu.test.common;
+
+import fr.maxlego08.menu.api.loader.ClassRegistry;
+import org.bukkit.plugin.Plugin;
+import org.junit.jupiter.api.Test;
+
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * A hook whose plugin is missing or broken fails while its loader is being constructed. Those
+ * failures must stay contained here, and must be reported for what they are, so that one bad hook
+ * neither aborts the scan nor gets logged as a missing constructor.
+ */
+class ClassRegistryTest {
+
+    public static class Working implements Runnable {
+        @Override
+        public void run() {
+        }
+    }
+
+    public static class ThrowsException implements Runnable {
+        public ThrowsException() {
+            throw new NullPointerException("the plugin API is null");
+        }
+
+        @Override
+        public void run() {
+        }
+    }
+
+    public static class ThrowsError implements Runnable {
+        public ThrowsError() {
+            throw new NoClassDefFoundError("com/example/Missing");
+        }
+
+        @Override
+        public void run() {
+        }
+    }
+
+    public static class NoUsableConstructor implements Runnable {
+        public NoUsableConstructor(String unsupported) {
+        }
+
+        @Override
+        public void run() {
+        }
+    }
+
+    private final List<Runnable> registered = new ArrayList<>();
+    private final List<String> errors = new ArrayList<>();
+
+    private ClassRegistry<Runnable, Plugin> registry() {
+        return ClassRegistry.of(Runnable.class, this.registered::add)
+                .tryNoArgsConstructor()
+                .errorLogger(this.errors::add);
+    }
+
+    @Test
+    void aWorkingClassIsRegistered() {
+        assertTrue(this.registry().load(null, Working.class));
+        assertEquals(1, this.registered.size());
+        assertTrue(this.errors.isEmpty(), "a successful load must not log an error: " + this.errors);
+    }
+
+    @Test
+    void aConstructorThrowingAnExceptionIsContained() {
+        assertFalse(this.registry().load(null, ThrowsException.class));
+        assertTrue(this.registered.isEmpty());
+        assertEquals(1, this.errors.size());
+        assertTrue(this.errors.getFirst().contains("constructor threw"), "the real cause must be reported, not a missing constructor: " + this.errors.getFirst());
+        assertTrue(this.errors.getFirst().contains("the plugin API is null"), "the message of the failure must be kept: " + this.errors.getFirst());
+    }
+
+    @Test
+    void aConstructorThrowingAnErrorIsContained() {
+        assertFalse(this.registry().load(null, ThrowsError.class));
+        assertTrue(this.registered.isEmpty());
+        assertEquals(1, this.errors.size());
+        assertTrue(this.errors.getFirst().contains("NoClassDefFoundError"), "the linkage failure must be named: " + this.errors.getFirst());
+    }
+
+    @Test
+    void aMissingConstructorIsStillReportedAsSuch() {
+        assertFalse(this.registry().load(null, NoUsableConstructor.class));
+        assertTrue(this.registered.isEmpty());
+        assertEquals(1, this.errors.size());
+        assertTrue(this.errors.getFirst().contains("Could not find a valid constructor"), "a genuinely missing constructor keeps its own message: " + this.errors.getFirst());
+    }
+}

Hooks/PacketEvents/build.gradle.kts

@@ -3,4 +3,8 @@ group = "Hooks:PacketEvents"
 dependencies {
     compileOnly(projects.common)
     compileOnly(libs.packetevents)
-}
+
+    testImplementation(projects.common)
+    testImplementation(libs.packetevents)
+    testImplementation(libs.paper.api)
+}

Hooks/PacketEvents/src/main/java/fr/maxlego08/menu/hooks/packetevents/PacketUtils.java

@@ -1,6 +1,7 @@
 package fr.maxlego08.menu.hooks.packetevents;
 
 import com.github.retrooper.packetevents.PacketEvents;
+import com.github.retrooper.packetevents.PacketEventsAPI;
 import com.github.retrooper.packetevents.event.EventManager;
 import com.github.retrooper.packetevents.event.PacketListenerPriority;
 import com.github.retrooper.packetevents.manager.player.PlayerManager;
@@ -31,7 +32,6 @@
 import java.util.UUID;
 
 public class PacketUtils implements InventoryListener, PacketManager {
-    private final PlayerManager playerManager = PacketEvents.getAPI().getPlayerManager();
 
     private PacketAnimationListener packetAnimationListener;
     private PacketTitleListener packetTitleListener;
@@ -40,27 +40,62 @@ public class PacketUtils implements InventoryListener, PacketManager {
     public static final Map<UUID, FakeInventory> fakeContents = new HashMap<>();
     private final MenuPlugin plugin;
 
+    private boolean ownsApi;
+    private boolean ready;
+
     public PacketUtils(MenuPlugin plugin) {
         this.plugin = plugin;
     }
 
+
+    private PacketEventsAPI<?> api() {
+        PacketEventsAPI<?> api = PacketEvents.getAPI();
+        if (api == null) {
+            throw new IllegalStateException("The packetevents API is not available, the packetevents plugin most likely failed to load.");
+        }
+        return api;
+    }
+
     @Override
+    @SuppressWarnings("ConstantConditions")
+    public boolean isReady() {
+        return this.ready && PacketEvents.getAPI() != null;
+    }
+
+    @Override
+    @SuppressWarnings("ConstantConditions")
     public void onLoad() {
-        PacketEvents.setAPI(SpigotPacketEventsBuilder.build(this.plugin));
-        PacketEvents.getAPI().load();
+        if (PacketEvents.getAPI() == null) {
+            PacketEvents.setAPI(SpigotPacketEventsBuilder.build(this.plugin));
+            this.ownsApi = true;
+        }
+        if (this.ownsApi) {
+            this.api().load();
+        }
     }
 
     @Override
+    @SuppressWarnings("ConstantConditions")
     public void onEnable() {
-        PacketEvents.getAPI().init();
-        EventManager eventManager = PacketEvents.getAPI().getEventManager();
+        if (PacketEvents.getAPI() == null) {
+            Logger.info("The packetevents API is not available, packet features are disabled.", Logger.LogType.WARNING);
+            return;
+        }
+
+        if (this.ownsApi) {
+            this.api().init();
+        }
+
+        EventManager eventManager = this.api().getEventManager();
 //         eventManager.registerListener(new PacketListener(), PacketListenerPriority.LOW);
         eventManager.registerListener(this.packetAnimationListener = new PacketAnimationListener(this.plugin), PacketListenerPriority.LOW);
         eventManager.registerListener(this.packetTitleListener = new PacketTitleListener(), PacketListenerPriority.LOW);
         if (Configuration.enablePacketEventClickLimiter){
             this.packetEventClickLimiterListener = new PacketEventClickLimiterListener();
             eventManager.registerListener(this.packetEventClickLimiterListener, PacketListenerPriority.HIGH);
         }
+
+        this.ready = true;
     }
 
     @Override
@@ -71,8 +106,12 @@ public void onPostEnable() {
     }
 
     @Override
+    @SuppressWarnings("ConstantConditions")
     public void onDisable() {
-        PacketEvents.getAPI().terminate();
+        this.ready = false;
+        if (this.ownsApi && PacketEvents.getAPI() != null) {
+            this.api().terminate();
+        }
     }
 
     @Override
@@ -132,13 +171,16 @@ public PacketTitleListener getPacketTitleListener() {
 
     @Override
     public void editInventoryTitleName(@NotNull Player player, @NotNull Component title) {
+        if (!this.isReady()) return;
+
         this.packetTitleListener.getPlayerPacketInformation(player.getUniqueId()).ifPresent(playerPacketInformation -> {
             WrapperPlayServerOpenWindow wrapperPlayServerOpenWindow = playerPacketInformation.getWrapperPlayServerOpenWindow();
             WrapperPlayServerOpenWindow newWrapperPlayServerOpenWindow1 = new WrapperPlayServerOpenWindow(wrapperPlayServerOpenWindow.getContainerId(),
                     wrapperPlayServerOpenWindow.getType(),
                     title);
-            this.playerManager.sendPacket(player, newWrapperPlayServerOpenWindow1);
-            this.playerManager.sendPacket(player, playerPacketInformation.getWrapperPlayServerWindowItems());
+            PlayerManager playerManager = this.api().getPlayerManager();
+            playerManager.sendPacket(player, newWrapperPlayServerOpenWindow1);
+            playerManager.sendPacket(player, playerPacketInformation.getWrapperPlayServerWindowItems());
         });
     }