refactor: rework hook registration (#468)

* refactor: rework hook registration

* Fix hooks registering twice, register the delayed hooks asynchronously

* Catch possible error where someone forgets to annotate a hook constructor with @PluginHook

* fix: resolve tests failing

Removed MockBukkit.unmock(): it was indefinitely checking for task finish

---------

Co-authored-by: Emibergo02 <36164338+Emibergo02@users.noreply.github.com>

build.gradle

@@ -1,7 +1,7 @@
 import org.apache.tools.ant.filters.ReplaceTokens
 
 plugins {
-    id 'com.github.johnrengelman.shadow' version '8.1.1'
+    id 'io.github.goooler.shadow' version "8.1.7"
     id 'org.cadixdev.licenser' version '0.6.1' apply false
     id 'org.ajoberstar.grgit' version '5.2.2'
     id 'maven-publish'
@@ -55,7 +55,7 @@ publishing {
 }
 
 allprojects {
-    apply plugin: 'com.github.johnrengelman.shadow'
+    apply plugin: 'io.github.goooler.shadow'
     apply plugin: 'org.cadixdev.licenser'
     apply plugin: 'java'
 

bukkit/src/main/java/net/william278/husktowns/BukkitHuskTowns.java

@@ -42,7 +42,9 @@
 import net.william278.husktowns.database.Database;
 import net.william278.husktowns.events.BukkitEventDispatcher;
 import net.william278.husktowns.hook.*;
-import net.william278.husktowns.hook.WorldGuardHook;
+import net.william278.husktowns.hook.map.BlueMapHook;
+import net.william278.husktowns.hook.map.DynmapHook;
+import net.william278.husktowns.hook.map.Pl3xMapHook;
 import net.william278.husktowns.listener.BukkitListener;
 import net.william278.husktowns.manager.Manager;
 import net.william278.husktowns.network.Broker;
@@ -125,8 +127,8 @@ public class BukkitHuskTowns extends JavaPlugin implements HuskTowns, BukkitTask
     @Nullable
     @Getter(AccessLevel.NONE)
     private Advancement advancements;
-
-    WorldGuardHook worldGuardHook;
+    @Setter
+    private HookManager hookManager;
 
     @TestOnly
     @SuppressWarnings("unused")
@@ -136,62 +138,69 @@ private BukkitHuskTowns(@NotNull JavaPluginLoader loader, @NotNull PluginDescrip
     }
 
     @Override
-    public void onEnable() {
-        // Initialize PaperLib and Adventure
-        this.paperLib = new MorePaperLib(this);
-        this.audiences = BukkitAudiences.create(this);
-
+    public void onLoad() {
         // Load configuration and subsystems
         this.loadConfig();
         if (this.settings.getGeneral().isDoAdvancements()) {
             loadAdvancements();
         }
 
-        // Prepare the database and networking system
-        this.database = this.loadDatabase();
-        if (!database.hasLoaded()) {
-            log(Level.SEVERE, "Failed to load database! Please check your credentials! Disabling plugin...");
-            Bukkit.getPluginManager().disablePlugin(this);
-            return;
-        }
-
-        // Load manager and broker
-        this.manager = new Manager(this);
-        this.broker = this.loadBroker();
-
         // Register hooks
+        this.hookManager = new BukkitHookManager(this);
         final PluginManager plugins = Bukkit.getPluginManager();
         if (settings.getGeneral().isEconomyHook()) {
             if (plugins.getPlugin("Vault") != null) {
-                this.registerHook(new VaultEconomyHook(this));
+                hookManager.registerHook(new VaultEconomyHook(this));
             }
         }
         if (settings.getGeneral().getWebMapHook().isEnabled()) {
             if (plugins.getPlugin("BlueMap") != null) {
-                this.registerHook(new BlueMapHook(this));
+                hookManager.registerHook(new BlueMapHook(this));
             } else if (plugins.getPlugin("dynmap") != null) {
-                this.registerHook(new DynmapHook(this));
+                hookManager.registerHook(new DynmapHook(this));
             } else if (plugins.getPlugin("Pl3xMap") != null) {
-                this.registerHook(new Pl3xMapHook(this));
+                hookManager.registerHook(new Pl3xMapHook(this));
             }
         }
         if (settings.getGeneral().isLuckpermsContextsHook() && plugins.getPlugin("LuckPerms") != null) {
-            this.registerHook(new LuckPermsHook(this));
+            hookManager.registerHook(new LuckPermsHook(this));
         }
         if (settings.getGeneral().isPlaceholderapiHook() && plugins.getPlugin("PlaceholderAPI") != null) {
-            this.registerHook(new PlaceholderAPIHook(this));
+            hookManager.registerHook(new PlaceholderAPIHook(this));
         }
         if (settings.getGeneral().isHuskhomesHook() && plugins.getPlugin("HuskHomes") != null) {
-            this.registerHook(new HuskHomesHook(this));
+            hookManager.registerHook(new HuskHomesHook(this));
         }
         if (settings.getGeneral().isPlanHook() && plugins.getPlugin("Plan") != null) {
-            this.registerHook(new PlanHook(this));
+            hookManager.registerHook(new PlanHook(this));
         }
         if (settings.getGeneral().isWorldGuardHook() && plugins.getPlugin("WorldGuard") != null) {
-            worldGuardHook = new BukkitWorldGuardHook(this);
-            this.registerHook(worldGuardHook);
+            hookManager.registerHook(new BukkitWorldGuardHook(this));
         }
 
+        hookManager.registerOnLoad();
+    }
+
+    @Override
+    public void onEnable() {
+        // Initialize PaperLib and Adventure
+        this.paperLib = new MorePaperLib(this);
+        this.audiences = BukkitAudiences.create(this);
+
+        // Prepare the database and networking system
+        this.database = this.loadDatabase();
+        if (!database.hasLoaded()) {
+            log(Level.SEVERE, "Failed to load database! Please check your credentials! Disabling plugin...");
+            Bukkit.getPluginManager().disablePlugin(this);
+            return;
+        }
+
+        // Load manager and broker
+        this.manager = new Manager(this);
+        this.broker = this.loadBroker();
+
+        hookManager.registerOnEnable();
+
         // Load towns and claim worlds
         this.loadData();
 
@@ -208,6 +217,8 @@ public void onEnable() {
         initializeMetrics();
         log(Level.INFO, "Enabled HuskTowns v" + getVersion());
         checkForUpdates();
+
+        runAsyncDelayed(hookManager::registerDelayed, 20L);
     }
 
     @Override
@@ -243,8 +254,8 @@ public Optional<Broker> getMessageBroker() {
     }
 
     @Override
-    public WorldGuardHook getWorldGuardHook() {
-        return worldGuardHook;
+    public @NotNull HookManager getHookManager() {
+        return hookManager;
     }
 
     @Override
@@ -364,7 +375,7 @@ private void initializeMetrics() {
             metrics.addCustomChart(new SimplePie("using_map",
                     () -> getMapHook().isPresent() ? "true" : "false"));
             getMapHook().ifPresent(hook -> metrics.addCustomChart(new SimplePie("map_type",
-                    () -> hook.getName().toLowerCase())));
+                    () -> hook.getHookInfo().id().toLowerCase())));
             getMessageBroker().ifPresent(broker -> metrics.addCustomChart(new SimplePie("messenger_type",
                     () -> settings.getCrossServer().getBrokerType().name().toLowerCase())));
         } catch (Exception e) {

bukkit/src/main/java/net/william278/husktowns/hook/BukkitHookManager.java

@@ -0,0 +1,71 @@
+/*
+ * This file is part of HuskTowns, licensed under the Apache License 2.0.
+ *
+ *  Copyright (c) William278 <will27528@gmail.com>
+ *  Copyright (c) contributors
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this file except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+package net.william278.husktowns.hook;
+
+import net.william278.husktowns.HuskTowns;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Level;
+
+public class BukkitHookManager extends HookManager {
+    public BukkitHookManager(@NotNull HuskTowns plugin) {
+        super(plugin);
+    }
+
+    @Override
+    public void registerOnLoad() {
+        plugin.log(Level.INFO, "Loading early hooks...");
+        AtomicInteger loaded = new AtomicInteger();
+        registeredHooks.stream().filter(Hook::isDisabled)
+                .filter((hook) -> hook.getHookInfo().register() == PluginHook.Register.ON_LOAD)
+                .forEach((hook) -> {
+                    hook.enable();
+                    loaded.getAndIncrement();
+                });
+        plugin.log(Level.INFO, "Successfully loaded %s hooks".formatted(loaded.get()));
+    }
+
+    @Override
+    public void registerOnEnable() {
+        plugin.log(Level.INFO, "Loading hooks...");
+        AtomicInteger loaded = new AtomicInteger();
+        registeredHooks.stream().filter(Hook::isDisabled)
+                .filter((hook) -> hook.getHookInfo().register() == PluginHook.Register.ON_ENABLE)
+                .forEach((hook) -> {
+                    hook.enable();
+                    loaded.getAndIncrement();
+                });
+        plugin.log(Level.INFO, "Successfully loaded %s hooks".formatted(loaded.get()));
+    }
+
+    @Override
+    public void registerDelayed() {
+        plugin.log(Level.INFO, "Loading late hooks...");
+        AtomicInteger loaded = new AtomicInteger();
+        registeredHooks.stream().filter(Hook::isDisabled)
+                .filter((hook) -> hook.getHookInfo().register() == PluginHook.Register.DELAYED)
+                .forEach((hook) -> {
+                    hook.enable();
+                    loaded.getAndIncrement();
+                });
+        plugin.log(Level.INFO, "Successfully loaded %s hooks".formatted(loaded.get()));
+    }
+}

bukkit/src/main/java/net/william278/husktowns/hook/BukkitWorldGuardHook.java

@@ -34,6 +34,7 @@
 import org.jetbrains.annotations.NotNull;
 
 public class BukkitWorldGuardHook extends WorldGuardHook {
+    @PluginHook(id = "WorldGuard", register = PluginHook.Register.ON_LOAD, platform = "bukkit")
     public BukkitWorldGuardHook(@NotNull HuskTowns plugin) {
         super(plugin);
     }

bukkit/src/main/java/net/william278/husktowns/hook/HuskHomesHook.java

@@ -44,9 +44,9 @@
 public class HuskHomesHook extends TeleportationHook implements Listener {
     @Nullable
     private HuskHomesAPI api;
-
+    @PluginHook(id = "HuskHomes", register = PluginHook.Register.ON_ENABLE, platform = "bukkit")
     public HuskHomesHook(@NotNull HuskTowns plugin) {
-        super(plugin, "HuskHomes");
+        super(plugin);
     }
 
     @Override

bukkit/src/main/java/net/william278/husktowns/hook/LuckPermsHook.java

@@ -46,12 +46,12 @@
 import java.util.logging.Level;
 
 public class LuckPermsHook extends Hook {
-
     private ContextManager contexts;
     private final List<ContextCalculator<Player>> calculators = new ArrayList<>();
 
+    @PluginHook(id = "LuckPerms", register = PluginHook.Register.ON_ENABLE, platform = "bukkit")
     public LuckPermsHook(@NotNull HuskTowns plugin) {
-        super(plugin, "LuckPerms");
+        super(plugin);
     }
 
     @Override

bukkit/src/main/java/net/william278/husktowns/hook/PlaceholderAPIHook.java

@@ -44,8 +44,9 @@
 import java.util.stream.Collectors;
 
 public class PlaceholderAPIHook extends Hook {
+    @PluginHook(id = "PlaceholderAPI", register = PluginHook.Register.ON_ENABLE, platform = "bukkit")
     public PlaceholderAPIHook(@NotNull HuskTowns plugin) {
-        super(plugin, "PlaceholderAPI");
+        super(plugin);
     }
 
     @Override

bukkit/src/main/java/net/william278/husktowns/hook/VaultEconomyHook.java

@@ -32,11 +32,11 @@
 import java.util.logging.Level;
 
 public class VaultEconomyHook extends EconomyHook {
-
     protected Economy economy;
 
+    @PluginHook(id = "Vault", register = PluginHook.Register.ON_ENABLE, platform = "bukkit")
     public VaultEconomyHook(@NotNull HuskTowns plugin) {
-        super(plugin, "Vault");
+        super(plugin);
     }
 
     @Override

bukkit/src/main/java/net/william278/husktowns/util/BukkitTask.java

@@ -65,8 +65,8 @@ class Async extends Task.Async implements BukkitTask {
 
         private ScheduledTask task;
 
-        protected Async(@NotNull HuskTowns plugin, @NotNull Runnable runnable) {
-            super(plugin, runnable);
+        protected Async(@NotNull HuskTowns plugin, @NotNull Runnable runnable, long delayTicks) {
+            super(plugin, runnable, delayTicks);
         }
 
         @Override
@@ -84,8 +84,18 @@ public void run() {
                 return;
             }
 
+            if (delayTicks > 0) {
+                this.task = getScheduler().globalRegionalScheduler().runDelayed(runnable, delayTicks);
+            } else {
+                this.task = getScheduler().globalRegionalScheduler().run(runnable);
+            }
+
             if (!cancelled) {
-                this.task = getScheduler().asyncScheduler().run(runnable);
+                if (delayTicks > 0) {
+                    this.task = getScheduler().asyncScheduler().runDelayed(runnable, Duration.of(delayTicks / 20 * 1000, ChronoUnit.MILLIS));
+                } else {
+                    this.task = getScheduler().asyncScheduler().run(runnable);
+                }
             }
         }
     }
@@ -135,8 +145,8 @@ default Task.Sync getSyncTask(@NotNull Runnable runnable, long delayTicks) {
 
         @NotNull
         @Override
-        default Task.Async getAsyncTask(@NotNull Runnable runnable) {
-            return new BukkitTask.Async(getPlugin(), runnable);
+        default Task.Async getAsyncTask(@NotNull Runnable runnable, long delayTicks) {
+            return new BukkitTask.Async(getPlugin(), runnable, delayTicks);
         }
 
         @NotNull

bukkit/src/test/java/net/william278/husktowns/BukkitPluginTests.java

@@ -25,14 +25,12 @@
 import net.william278.husktowns.audit.Action;
 import net.william278.husktowns.audit.Log;
 import net.william278.husktowns.claim.*;
-import net.william278.husktowns.hook.BukkitWorldGuardHook;
 import net.william278.husktowns.map.MapSquare;
 import net.william278.husktowns.town.Town;
 import net.william278.husktowns.user.BukkitUser;
 import net.william278.husktowns.user.OnlineUser;
 import net.william278.husktowns.user.Preferences;
 import org.bukkit.Location;
-import org.bukkit.entity.Husk;
 import org.bukkit.entity.Player;
 import org.jetbrains.annotations.NotNull;
 import org.junit.jupiter.api.*;
@@ -61,12 +59,6 @@ public static void setUpPlugin() {
         plugin = MockBukkit.load(BukkitHuskTowns.class);
     }
 
-    @AfterAll
-    @DisplayName("Tear down Plugin")
-    public static void tearDownPlugin() {
-        MockBukkit.unmock();
-    }
-
     @Order(1)
     @Nested
     @DisplayName("Data Validation Tests")