Skip to content

Commit 1833cdd

Browse files
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>
1 parent 722092d commit 1833cdd

25 files changed

+299
-112
lines changed

build.gradle

+2-2
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import org.apache.tools.ant.filters.ReplaceTokens
22

33
plugins {
4-
id 'com.github.johnrengelman.shadow' version '8.1.1'
4+
id 'io.github.goooler.shadow' version "8.1.7"
55
id 'org.cadixdev.licenser' version '0.6.1' apply false
66
id 'org.ajoberstar.grgit' version '5.2.2'
77
id 'maven-publish'
@@ -55,7 +55,7 @@ publishing {
5555
}
5656

5757
allprojects {
58-
apply plugin: 'com.github.johnrengelman.shadow'
58+
apply plugin: 'io.github.goooler.shadow'
5959
apply plugin: 'org.cadixdev.licenser'
6060
apply plugin: 'java'
6161

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

+44-33
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@
4242
import net.william278.husktowns.database.Database;
4343
import net.william278.husktowns.events.BukkitEventDispatcher;
4444
import net.william278.husktowns.hook.*;
45-
import net.william278.husktowns.hook.WorldGuardHook;
45+
import net.william278.husktowns.hook.map.BlueMapHook;
46+
import net.william278.husktowns.hook.map.DynmapHook;
47+
import net.william278.husktowns.hook.map.Pl3xMapHook;
4648
import net.william278.husktowns.listener.BukkitListener;
4749
import net.william278.husktowns.manager.Manager;
4850
import net.william278.husktowns.network.Broker;
@@ -125,8 +127,8 @@ public class BukkitHuskTowns extends JavaPlugin implements HuskTowns, BukkitTask
125127
@Nullable
126128
@Getter(AccessLevel.NONE)
127129
private Advancement advancements;
128-
129-
WorldGuardHook worldGuardHook;
130+
@Setter
131+
private HookManager hookManager;
130132

131133
@TestOnly
132134
@SuppressWarnings("unused")
@@ -136,62 +138,69 @@ private BukkitHuskTowns(@NotNull JavaPluginLoader loader, @NotNull PluginDescrip
136138
}
137139

138140
@Override
139-
public void onEnable() {
140-
// Initialize PaperLib and Adventure
141-
this.paperLib = new MorePaperLib(this);
142-
this.audiences = BukkitAudiences.create(this);
143-
141+
public void onLoad() {
144142
// Load configuration and subsystems
145143
this.loadConfig();
146144
if (this.settings.getGeneral().isDoAdvancements()) {
147145
loadAdvancements();
148146
}
149147

150-
// Prepare the database and networking system
151-
this.database = this.loadDatabase();
152-
if (!database.hasLoaded()) {
153-
log(Level.SEVERE, "Failed to load database! Please check your credentials! Disabling plugin...");
154-
Bukkit.getPluginManager().disablePlugin(this);
155-
return;
156-
}
157-
158-
// Load manager and broker
159-
this.manager = new Manager(this);
160-
this.broker = this.loadBroker();
161-
162148
// Register hooks
149+
this.hookManager = new BukkitHookManager(this);
163150
final PluginManager plugins = Bukkit.getPluginManager();
164151
if (settings.getGeneral().isEconomyHook()) {
165152
if (plugins.getPlugin("Vault") != null) {
166-
this.registerHook(new VaultEconomyHook(this));
153+
hookManager.registerHook(new VaultEconomyHook(this));
167154
}
168155
}
169156
if (settings.getGeneral().getWebMapHook().isEnabled()) {
170157
if (plugins.getPlugin("BlueMap") != null) {
171-
this.registerHook(new BlueMapHook(this));
158+
hookManager.registerHook(new BlueMapHook(this));
172159
} else if (plugins.getPlugin("dynmap") != null) {
173-
this.registerHook(new DynmapHook(this));
160+
hookManager.registerHook(new DynmapHook(this));
174161
} else if (plugins.getPlugin("Pl3xMap") != null) {
175-
this.registerHook(new Pl3xMapHook(this));
162+
hookManager.registerHook(new Pl3xMapHook(this));
176163
}
177164
}
178165
if (settings.getGeneral().isLuckpermsContextsHook() && plugins.getPlugin("LuckPerms") != null) {
179-
this.registerHook(new LuckPermsHook(this));
166+
hookManager.registerHook(new LuckPermsHook(this));
180167
}
181168
if (settings.getGeneral().isPlaceholderapiHook() && plugins.getPlugin("PlaceholderAPI") != null) {
182-
this.registerHook(new PlaceholderAPIHook(this));
169+
hookManager.registerHook(new PlaceholderAPIHook(this));
183170
}
184171
if (settings.getGeneral().isHuskhomesHook() && plugins.getPlugin("HuskHomes") != null) {
185-
this.registerHook(new HuskHomesHook(this));
172+
hookManager.registerHook(new HuskHomesHook(this));
186173
}
187174
if (settings.getGeneral().isPlanHook() && plugins.getPlugin("Plan") != null) {
188-
this.registerHook(new PlanHook(this));
175+
hookManager.registerHook(new PlanHook(this));
189176
}
190177
if (settings.getGeneral().isWorldGuardHook() && plugins.getPlugin("WorldGuard") != null) {
191-
worldGuardHook = new BukkitWorldGuardHook(this);
192-
this.registerHook(worldGuardHook);
178+
hookManager.registerHook(new BukkitWorldGuardHook(this));
193179
}
194180

181+
hookManager.registerOnLoad();
182+
}
183+
184+
@Override
185+
public void onEnable() {
186+
// Initialize PaperLib and Adventure
187+
this.paperLib = new MorePaperLib(this);
188+
this.audiences = BukkitAudiences.create(this);
189+
190+
// Prepare the database and networking system
191+
this.database = this.loadDatabase();
192+
if (!database.hasLoaded()) {
193+
log(Level.SEVERE, "Failed to load database! Please check your credentials! Disabling plugin...");
194+
Bukkit.getPluginManager().disablePlugin(this);
195+
return;
196+
}
197+
198+
// Load manager and broker
199+
this.manager = new Manager(this);
200+
this.broker = this.loadBroker();
201+
202+
hookManager.registerOnEnable();
203+
195204
// Load towns and claim worlds
196205
this.loadData();
197206

@@ -208,6 +217,8 @@ public void onEnable() {
208217
initializeMetrics();
209218
log(Level.INFO, "Enabled HuskTowns v" + getVersion());
210219
checkForUpdates();
220+
221+
runAsyncDelayed(hookManager::registerDelayed, 20L);
211222
}
212223

213224
@Override
@@ -243,8 +254,8 @@ public Optional<Broker> getMessageBroker() {
243254
}
244255

245256
@Override
246-
public WorldGuardHook getWorldGuardHook() {
247-
return worldGuardHook;
257+
public @NotNull HookManager getHookManager() {
258+
return hookManager;
248259
}
249260

250261
@Override
@@ -364,7 +375,7 @@ private void initializeMetrics() {
364375
metrics.addCustomChart(new SimplePie("using_map",
365376
() -> getMapHook().isPresent() ? "true" : "false"));
366377
getMapHook().ifPresent(hook -> metrics.addCustomChart(new SimplePie("map_type",
367-
() -> hook.getName().toLowerCase())));
378+
() -> hook.getHookInfo().id().toLowerCase())));
368379
getMessageBroker().ifPresent(broker -> metrics.addCustomChart(new SimplePie("messenger_type",
369380
() -> settings.getCrossServer().getBrokerType().name().toLowerCase())));
370381
} catch (Exception e) {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* This file is part of HuskTowns, licensed under the Apache License 2.0.
3+
*
4+
* Copyright (c) William278 <will27528@gmail.com>
5+
* Copyright (c) contributors
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
20+
package net.william278.husktowns.hook;
21+
22+
import net.william278.husktowns.HuskTowns;
23+
import org.jetbrains.annotations.NotNull;
24+
25+
import java.util.concurrent.atomic.AtomicInteger;
26+
import java.util.logging.Level;
27+
28+
public class BukkitHookManager extends HookManager {
29+
public BukkitHookManager(@NotNull HuskTowns plugin) {
30+
super(plugin);
31+
}
32+
33+
@Override
34+
public void registerOnLoad() {
35+
plugin.log(Level.INFO, "Loading early hooks...");
36+
AtomicInteger loaded = new AtomicInteger();
37+
registeredHooks.stream().filter(Hook::isDisabled)
38+
.filter((hook) -> hook.getHookInfo().register() == PluginHook.Register.ON_LOAD)
39+
.forEach((hook) -> {
40+
hook.enable();
41+
loaded.getAndIncrement();
42+
});
43+
plugin.log(Level.INFO, "Successfully loaded %s hooks".formatted(loaded.get()));
44+
}
45+
46+
@Override
47+
public void registerOnEnable() {
48+
plugin.log(Level.INFO, "Loading hooks...");
49+
AtomicInteger loaded = new AtomicInteger();
50+
registeredHooks.stream().filter(Hook::isDisabled)
51+
.filter((hook) -> hook.getHookInfo().register() == PluginHook.Register.ON_ENABLE)
52+
.forEach((hook) -> {
53+
hook.enable();
54+
loaded.getAndIncrement();
55+
});
56+
plugin.log(Level.INFO, "Successfully loaded %s hooks".formatted(loaded.get()));
57+
}
58+
59+
@Override
60+
public void registerDelayed() {
61+
plugin.log(Level.INFO, "Loading late hooks...");
62+
AtomicInteger loaded = new AtomicInteger();
63+
registeredHooks.stream().filter(Hook::isDisabled)
64+
.filter((hook) -> hook.getHookInfo().register() == PluginHook.Register.DELAYED)
65+
.forEach((hook) -> {
66+
hook.enable();
67+
loaded.getAndIncrement();
68+
});
69+
plugin.log(Level.INFO, "Successfully loaded %s hooks".formatted(loaded.get()));
70+
}
71+
}

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

+1
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import org.jetbrains.annotations.NotNull;
3535

3636
public class BukkitWorldGuardHook extends WorldGuardHook {
37+
@PluginHook(id = "WorldGuard", register = PluginHook.Register.ON_LOAD, platform = "bukkit")
3738
public BukkitWorldGuardHook(@NotNull HuskTowns plugin) {
3839
super(plugin);
3940
}

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,9 @@
4444
public class HuskHomesHook extends TeleportationHook implements Listener {
4545
@Nullable
4646
private HuskHomesAPI api;
47-
47+
@PluginHook(id = "HuskHomes", register = PluginHook.Register.ON_ENABLE, platform = "bukkit")
4848
public HuskHomesHook(@NotNull HuskTowns plugin) {
49-
super(plugin, "HuskHomes");
49+
super(plugin);
5050
}
5151

5252
@Override

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,12 +46,12 @@
4646
import java.util.logging.Level;
4747

4848
public class LuckPermsHook extends Hook {
49-
5049
private ContextManager contexts;
5150
private final List<ContextCalculator<Player>> calculators = new ArrayList<>();
5251

52+
@PluginHook(id = "LuckPerms", register = PluginHook.Register.ON_ENABLE, platform = "bukkit")
5353
public LuckPermsHook(@NotNull HuskTowns plugin) {
54-
super(plugin, "LuckPerms");
54+
super(plugin);
5555
}
5656

5757
@Override

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

+2-1
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,9 @@
4444
import java.util.stream.Collectors;
4545

4646
public class PlaceholderAPIHook extends Hook {
47+
@PluginHook(id = "PlaceholderAPI", register = PluginHook.Register.ON_ENABLE, platform = "bukkit")
4748
public PlaceholderAPIHook(@NotNull HuskTowns plugin) {
48-
super(plugin, "PlaceholderAPI");
49+
super(plugin);
4950
}
5051

5152
@Override

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

+2-2
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,11 @@
3232
import java.util.logging.Level;
3333

3434
public class VaultEconomyHook extends EconomyHook {
35-
3635
protected Economy economy;
3736

37+
@PluginHook(id = "Vault", register = PluginHook.Register.ON_ENABLE, platform = "bukkit")
3838
public VaultEconomyHook(@NotNull HuskTowns plugin) {
39-
super(plugin, "Vault");
39+
super(plugin);
4040
}
4141

4242
@Override

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

+15-5
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,8 @@ class Async extends Task.Async implements BukkitTask {
6565

6666
private ScheduledTask task;
6767

68-
protected Async(@NotNull HuskTowns plugin, @NotNull Runnable runnable) {
69-
super(plugin, runnable);
68+
protected Async(@NotNull HuskTowns plugin, @NotNull Runnable runnable, long delayTicks) {
69+
super(plugin, runnable, delayTicks);
7070
}
7171

7272
@Override
@@ -84,8 +84,18 @@ public void run() {
8484
return;
8585
}
8686

87+
if (delayTicks > 0) {
88+
this.task = getScheduler().globalRegionalScheduler().runDelayed(runnable, delayTicks);
89+
} else {
90+
this.task = getScheduler().globalRegionalScheduler().run(runnable);
91+
}
92+
8793
if (!cancelled) {
88-
this.task = getScheduler().asyncScheduler().run(runnable);
94+
if (delayTicks > 0) {
95+
this.task = getScheduler().asyncScheduler().runDelayed(runnable, Duration.of(delayTicks / 20 * 1000, ChronoUnit.MILLIS));
96+
} else {
97+
this.task = getScheduler().asyncScheduler().run(runnable);
98+
}
8999
}
90100
}
91101
}
@@ -135,8 +145,8 @@ default Task.Sync getSyncTask(@NotNull Runnable runnable, long delayTicks) {
135145

136146
@NotNull
137147
@Override
138-
default Task.Async getAsyncTask(@NotNull Runnable runnable) {
139-
return new BukkitTask.Async(getPlugin(), runnable);
148+
default Task.Async getAsyncTask(@NotNull Runnable runnable, long delayTicks) {
149+
return new BukkitTask.Async(getPlugin(), runnable, delayTicks);
140150
}
141151

142152
@NotNull

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

-8
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,12 @@
2525
import net.william278.husktowns.audit.Action;
2626
import net.william278.husktowns.audit.Log;
2727
import net.william278.husktowns.claim.*;
28-
import net.william278.husktowns.hook.BukkitWorldGuardHook;
2928
import net.william278.husktowns.map.MapSquare;
3029
import net.william278.husktowns.town.Town;
3130
import net.william278.husktowns.user.BukkitUser;
3231
import net.william278.husktowns.user.OnlineUser;
3332
import net.william278.husktowns.user.Preferences;
3433
import org.bukkit.Location;
35-
import org.bukkit.entity.Husk;
3634
import org.bukkit.entity.Player;
3735
import org.jetbrains.annotations.NotNull;
3836
import org.junit.jupiter.api.*;
@@ -61,12 +59,6 @@ public static void setUpPlugin() {
6159
plugin = MockBukkit.load(BukkitHuskTowns.class);
6260
}
6361

64-
@AfterAll
65-
@DisplayName("Tear down Plugin")
66-
public static void tearDownPlugin() {
67-
MockBukkit.unmock();
68-
}
69-
7062
@Order(1)
7163
@Nested
7264
@DisplayName("Data Validation Tests")

0 commit comments

Comments
 (0)