From 8f134570d5d6552a0ed27faabfca952c2c96f01f Mon Sep 17 00:00:00 2001 From: James Mortemore Date: Fri, 3 Apr 2026 16:15:56 +0100 Subject: [PATCH 1/3] feat: pass kick message through PlayerBannedEvent for plugin modification Add a nullable Message kickMessage field to PlayerBannedEvent on all 6 platforms so external plugins (e.g. WebEnhancer) can modify the kick message before it is sent to the player. BanCommand and TempBanCommand now build the kick message before calling ban(), pass it through PlayerBanStorage.ban() where [id] is set after create(), and fire PlayerBannedEvent with the message attached. After the event completes, the resolved message is used for the kick. This enables the [pin] placeholder to be replaced on the initial kick when banning an online player, not just on reconnect denial. --- .../banmanager/bukkit/BukkitServer.java | 6 +++- .../bukkit/api/events/PlayerBannedEvent.java | 6 ++++ .../banmanager/bungee/BungeeServer.java | 6 +++- .../bungee/api/events/PlayerBannedEvent.java | 6 ++++ .../common/commands/BanCommand.java | 31 +++++++++-------- .../common/commands/TempBanCommand.java | 33 ++++++++++--------- .../common/storage/PlayerBanStorage.java | 11 ++++++- .../banmanager/fabric/BanManagerEvents.java | 6 ++-- .../banmanager/fabric/FabricServer.java | 3 +- .../fabric/listeners/BanListener.java | 3 +- .../fabric/listeners/HookListener.java | 3 +- .../fabric/listeners/WebhookListener.java | 3 +- .../banmanager/sponge/SpongeServer.java | 6 +++- .../sponge/api/events/PlayerBannedEvent.java | 6 ++++ .../banmanager/sponge/SpongeServer.java | 6 +++- .../sponge/api/events/PlayerBannedEvent.java | 6 ++++ .../banmanager/velocity/VelocityServer.java | 6 +++- .../api/events/PlayerBannedEvent.java | 6 ++++ 18 files changed, 111 insertions(+), 42 deletions(-) diff --git a/bukkit/src/main/java/me/confuser/banmanager/bukkit/BukkitServer.java b/bukkit/src/main/java/me/confuser/banmanager/bukkit/BukkitServer.java index e9d3fa87c..e6d070ac7 100644 --- a/bukkit/src/main/java/me/confuser/banmanager/bukkit/BukkitServer.java +++ b/bukkit/src/main/java/me/confuser/banmanager/bukkit/BukkitServer.java @@ -138,7 +138,11 @@ public CommonEvent callEvent(String name, Object... args) { event = new PlayerBanEvent((PlayerBanData) args[0], (boolean) args[1]); break; case "PlayerBannedEvent": - event = new PlayerBannedEvent((PlayerBanData) args[0], (boolean) args[1]); + PlayerBannedEvent bannedEvent = new PlayerBannedEvent((PlayerBanData) args[0], (boolean) args[1]); + if (args.length > 2 && args[2] instanceof Message) { + bannedEvent.setKickMessage((Message) args[2]); + } + event = bannedEvent; break; case "PlayerUnbanEvent": event = new PlayerUnbanEvent((PlayerBanData) args[0], (PlayerData) args[1], (String) args[2], (boolean) args[3]); diff --git a/bukkit/src/main/java/me/confuser/banmanager/bukkit/api/events/PlayerBannedEvent.java b/bukkit/src/main/java/me/confuser/banmanager/bukkit/api/events/PlayerBannedEvent.java index 5eb7081fa..0dcebbfe6 100644 --- a/bukkit/src/main/java/me/confuser/banmanager/bukkit/api/events/PlayerBannedEvent.java +++ b/bukkit/src/main/java/me/confuser/banmanager/bukkit/api/events/PlayerBannedEvent.java @@ -1,7 +1,9 @@ package me.confuser.banmanager.bukkit.api.events; import lombok.Getter; +import lombok.Setter; import me.confuser.banmanager.common.data.PlayerBanData; +import me.confuser.banmanager.common.util.Message; public class PlayerBannedEvent extends SilentEvent { @@ -9,6 +11,10 @@ public class PlayerBannedEvent extends SilentEvent { @Getter private PlayerBanData ban; + @Getter + @Setter + private Message kickMessage; + public PlayerBannedEvent(PlayerBanData ban, boolean isSilent) { super(isSilent, true); this.ban = ban; diff --git a/bungee/src/main/java/me/confuser/banmanager/bungee/BungeeServer.java b/bungee/src/main/java/me/confuser/banmanager/bungee/BungeeServer.java index 9b3a8c180..a325a374c 100755 --- a/bungee/src/main/java/me/confuser/banmanager/bungee/BungeeServer.java +++ b/bungee/src/main/java/me/confuser/banmanager/bungee/BungeeServer.java @@ -119,7 +119,11 @@ public CommonEvent callEvent(String name, Object... args) { event = new PlayerBanEvent((PlayerBanData) args[0], (boolean) args[1]); break; case "PlayerBannedEvent": - event = new PlayerBannedEvent((PlayerBanData) args[0], (boolean) args[1]); + PlayerBannedEvent bannedEvent = new PlayerBannedEvent((PlayerBanData) args[0], (boolean) args[1]); + if (args.length > 2 && args[2] instanceof Message) { + bannedEvent.setKickMessage((Message) args[2]); + } + event = bannedEvent; break; case "PlayerUnbanEvent": event = new PlayerUnbanEvent((PlayerBanData) args[0], (PlayerData) args[1], (String) args[2], (boolean) args[3]); diff --git a/bungee/src/main/java/me/confuser/banmanager/bungee/api/events/PlayerBannedEvent.java b/bungee/src/main/java/me/confuser/banmanager/bungee/api/events/PlayerBannedEvent.java index 86d6a8e1d..472cdcf77 100755 --- a/bungee/src/main/java/me/confuser/banmanager/bungee/api/events/PlayerBannedEvent.java +++ b/bungee/src/main/java/me/confuser/banmanager/bungee/api/events/PlayerBannedEvent.java @@ -1,7 +1,9 @@ package me.confuser.banmanager.bungee.api.events; import lombok.Getter; +import lombok.Setter; import me.confuser.banmanager.common.data.PlayerBanData; +import me.confuser.banmanager.common.util.Message; public class PlayerBannedEvent extends SilentEvent { @@ -9,6 +11,10 @@ public class PlayerBannedEvent extends SilentEvent { @Getter private PlayerBanData ban; + @Getter + @Setter + private Message kickMessage; + public PlayerBannedEvent(PlayerBanData ban, boolean isSilent) { super(isSilent); this.ban = ban; diff --git a/common/src/main/java/me/confuser/banmanager/common/commands/BanCommand.java b/common/src/main/java/me/confuser/banmanager/common/commands/BanCommand.java index ceeb96cb6..93b0b6450 100644 --- a/common/src/main/java/me/confuser/banmanager/common/commands/BanCommand.java +++ b/common/src/main/java/me/confuser/banmanager/common/commands/BanCommand.java @@ -140,8 +140,18 @@ public boolean onCommand(CommonSender sender, CommandParser parser) { final PlayerBanData ban = new PlayerBanData(player, actor, parser.getReason().getMessage(), isSilent); boolean created; + Message kickMessage = null; + if (onlinePlayer != null) { + kickMessage = Message.get("ban.player.kick") + .set("displayName", onlinePlayer.getDisplayName()) + .set("player", player.getName()) + .set("playerId", player.getUUID().toString()) + .set("reason", ban.getReason()) + .set("actor", actor.getName()); + } + try { - created = getPlugin().getPlayerBanStorage().ban(ban); + created = getPlugin().getPlayerBanStorage().ban(ban, false, kickMessage); } catch (SQLException e) { handlePunishmentCreateException(e, sender, Message.get("ban.error.exists").set("player", targetName)); @@ -154,19 +164,12 @@ public boolean onCommand(CommonSender sender, CommandParser parser) { handlePrivateNotes(player, actor, parser.getReason()); - getPlugin().getScheduler().runSync(() -> { - if (onlinePlayer == null) return; - - Message kickMessage = Message.get("ban.player.kick") - .set("displayName", onlinePlayer.getDisplayName()) - .set("player", player.getName()) - .set("playerId", player.getUUID().toString()) - .set("reason", ban.getReason()) - .set("id", ban.getId()) - .set("actor", actor.getName()); - - onlinePlayer.kick(kickMessage.toString()); - }); + if (onlinePlayer != null) { + final Message finalKickMessage = kickMessage; + getPlugin().getScheduler().runSync(() -> { + onlinePlayer.kick(finalKickMessage.toString()); + }); + } }); return true; diff --git a/common/src/main/java/me/confuser/banmanager/common/commands/TempBanCommand.java b/common/src/main/java/me/confuser/banmanager/common/commands/TempBanCommand.java index 3e72b3b39..2acb7b136 100644 --- a/common/src/main/java/me/confuser/banmanager/common/commands/TempBanCommand.java +++ b/common/src/main/java/me/confuser/banmanager/common/commands/TempBanCommand.java @@ -162,8 +162,19 @@ public void run() { final PlayerBanData ban = new PlayerBanData(player, actor, reason.getMessage(), isSilent, expires); boolean created; + Message kickMessage = null; + if (onlinePlayer != null) { + kickMessage = Message.get("tempban.player.kick") + .set("displayName", onlinePlayer.getDisplayName()) + .set("player", player.getName()) + .set("playerId", player.getUUID().toString()) + .set("reason", ban.getReason()) + .set("actor", actor.getName()) + .set("expires", DateUtils.getDifferenceFormat(ban.getExpires())); + } + try { - created = getPlugin().getPlayerBanStorage().ban(ban); + created = getPlugin().getPlayerBanStorage().ban(ban, false, kickMessage); } catch (SQLException e) { handlePunishmentCreateException(e, sender, Message.get("ban.error.exists").set("player", targetName)); @@ -176,20 +187,12 @@ public void run() { handlePrivateNotes(player, actor, reason); - getPlugin().getScheduler().runSync(() -> { - if (onlinePlayer == null) return; - - Message kickMessage = Message.get("tempban.player.kick") - .set("displayName", onlinePlayer.getDisplayName()) - .set("player", player.getName()) - .set("playerId", player.getUUID().toString()) - .set("reason", ban.getReason()) - .set("actor", actor.getName()) - .set("id", ban.getId()) - .set("expires", DateUtils.getDifferenceFormat(ban.getExpires())); - - onlinePlayer.kick(kickMessage.toString()); - }); + if (onlinePlayer != null) { + final Message finalKickMessage = kickMessage; + getPlugin().getScheduler().runSync(() -> { + onlinePlayer.kick(finalKickMessage.toString()); + }); + } } diff --git a/common/src/main/java/me/confuser/banmanager/common/storage/PlayerBanStorage.java b/common/src/main/java/me/confuser/banmanager/common/storage/PlayerBanStorage.java index e71302615..57839c1b9 100644 --- a/common/src/main/java/me/confuser/banmanager/common/storage/PlayerBanStorage.java +++ b/common/src/main/java/me/confuser/banmanager/common/storage/PlayerBanStorage.java @@ -17,6 +17,7 @@ import me.confuser.banmanager.common.ormlite.table.DatabaseTableConfig; import me.confuser.banmanager.common.ormlite.table.TableUtils; import me.confuser.banmanager.common.util.IPUtils; +import me.confuser.banmanager.common.util.Message; import me.confuser.banmanager.common.util.TransactionHelper; import me.confuser.banmanager.common.util.UUIDUtils; @@ -198,6 +199,10 @@ public boolean ban(PlayerBanData ban) throws SQLException { } public boolean ban(PlayerBanData ban, boolean fromSync) throws SQLException { + return ban(ban, fromSync, null); + } + + public boolean ban(PlayerBanData ban, boolean fromSync, Message kickMessage) throws SQLException { CommonEvent event = plugin.getServer().callEvent("PlayerBanEvent", ban, ban.isSilent()); if (event.isCancelled()) { @@ -207,7 +212,11 @@ public boolean ban(PlayerBanData ban, boolean fromSync) throws SQLException { create(ban); bans.put(ban.getPlayer().getUUID(), ban); - plugin.getServer().callEvent("PlayerBannedEvent", ban, event.isSilent() || (fromSync && !plugin.getConfig().isBroadcastOnSync())); + if (kickMessage != null) { + kickMessage.set("id", ban.getId()); + } + + plugin.getServer().callEvent("PlayerBannedEvent", ban, event.isSilent() || (fromSync && !plugin.getConfig().isBroadcastOnSync()), kickMessage); return true; } diff --git a/fabric/src/main/java/me/confuser/banmanager/fabric/BanManagerEvents.java b/fabric/src/main/java/me/confuser/banmanager/fabric/BanManagerEvents.java index 759ad0d38..4f1329299 100644 --- a/fabric/src/main/java/me/confuser/banmanager/fabric/BanManagerEvents.java +++ b/fabric/src/main/java/me/confuser/banmanager/fabric/BanManagerEvents.java @@ -20,9 +20,9 @@ public class BanManagerEvents { }); public static final Event PLAYER_BANNED_EVENT = EventFactory.createArrayBacked(PlayerBannedEvent.class, - (listeners) -> (banData, silent) -> { + (listeners) -> (banData, silent, kickMessage) -> { for (PlayerBannedEvent listener : listeners) { - listener.onPlayerBanned(banData, silent); + listener.onPlayerBanned(banData, silent, kickMessage); } }); @@ -229,7 +229,7 @@ public interface PlayerBanEvent { @FunctionalInterface public interface PlayerBannedEvent { - void onPlayerBanned(PlayerBanData banData, boolean silent); + void onPlayerBanned(PlayerBanData banData, boolean silent, Message kickMessage); } @FunctionalInterface diff --git a/fabric/src/main/java/me/confuser/banmanager/fabric/FabricServer.java b/fabric/src/main/java/me/confuser/banmanager/fabric/FabricServer.java index 8b55023f3..b5d474e99 100644 --- a/fabric/src/main/java/me/confuser/banmanager/fabric/FabricServer.java +++ b/fabric/src/main/java/me/confuser/banmanager/fabric/FabricServer.java @@ -142,7 +142,8 @@ public CommonEvent callEvent(String name, Object... args) { break; case "PlayerBannedEvent": silentValue = new BanManagerEvents.SilentValue((boolean) args[1]); - BanManagerEvents.PLAYER_BANNED_EVENT.invoker().onPlayerBanned((PlayerBanData) args[0], silentValue.isSilent()); + Message bannedKickMessage = args.length > 2 && args[2] instanceof Message ? (Message) args[2] : null; + BanManagerEvents.PLAYER_BANNED_EVENT.invoker().onPlayerBanned((PlayerBanData) args[0], silentValue.isSilent(), bannedKickMessage); commonEvent = new CommonEvent(false, silentValue.isSilent()); break; case "PlayerUnbanEvent": diff --git a/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/BanListener.java b/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/BanListener.java index fc5404df4..41406be97 100644 --- a/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/BanListener.java +++ b/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/BanListener.java @@ -2,6 +2,7 @@ import me.confuser.banmanager.common.BanManagerPlugin; import me.confuser.banmanager.common.listeners.CommonBanListener; +import me.confuser.banmanager.common.util.Message; import me.confuser.banmanager.fabric.BanManagerEvents; import me.confuser.banmanager.common.data.*; @@ -18,7 +19,7 @@ public BanListener(BanManagerPlugin plugin) { BanManagerEvents.NAME_BANNED_EVENT.register(this::notifyOnNameBan); } - private void notifyOnBan(PlayerBanData banData, boolean silent) { + private void notifyOnBan(PlayerBanData banData, boolean silent, Message kickMessage) { listener.notifyOnBan(banData, silent); } diff --git a/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/HookListener.java b/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/HookListener.java index 639ba1715..f7148d187 100644 --- a/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/HookListener.java +++ b/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/HookListener.java @@ -2,6 +2,7 @@ import me.confuser.banmanager.common.BanManagerPlugin; import me.confuser.banmanager.common.listeners.CommonHooksListener; +import me.confuser.banmanager.common.util.Message; import me.confuser.banmanager.fabric.BanManagerEvents; import me.confuser.banmanager.common.data.*; @@ -36,7 +37,7 @@ private boolean onBan(PlayerBanData banData, BanManagerEvents.SilentValue silent return true; } - private void onBan(PlayerBanData banData, boolean silent) { + private void onBan(PlayerBanData banData, boolean silent, Message kickMessage) { listener.onBan(banData, false, silent); } diff --git a/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/WebhookListener.java b/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/WebhookListener.java index 978930883..994ffed05 100644 --- a/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/WebhookListener.java +++ b/fabric/src/main/java/me/confuser/banmanager/fabric/listeners/WebhookListener.java @@ -3,6 +3,7 @@ import me.confuser.banmanager.common.BanManagerPlugin; import me.confuser.banmanager.common.listeners.CommonWebhookListener; import me.confuser.banmanager.common.listeners.CommonWebhookListener.WebhookData; +import me.confuser.banmanager.common.util.Message; import me.confuser.banmanager.fabric.BanManagerEvents; import me.confuser.banmanager.common.data.*; @@ -26,7 +27,7 @@ public WebhookListener(BanManagerPlugin plugin) { BanManagerEvents.PLAYER_REPORTED_EVENT.register(this::notifyOnReport); } - private void notifyOnBan(PlayerBanData banData, boolean silent) { + private void notifyOnBan(PlayerBanData banData, boolean silent, Message kickMessage) { List webhooks = listener.notifyOnBan(banData); sendAll(webhooks, silent); } diff --git a/sponge-api7/src/main/java/me/confuser/banmanager/sponge/SpongeServer.java b/sponge-api7/src/main/java/me/confuser/banmanager/sponge/SpongeServer.java index 40a0c36f9..a3ceafd3d 100644 --- a/sponge-api7/src/main/java/me/confuser/banmanager/sponge/SpongeServer.java +++ b/sponge-api7/src/main/java/me/confuser/banmanager/sponge/SpongeServer.java @@ -123,7 +123,11 @@ public CommonEvent callEvent(String name, Object... args) { event = new PlayerBanEvent((PlayerBanData) args[0], (boolean) args[1]); break; case "PlayerBannedEvent": - event = new PlayerBannedEvent((PlayerBanData) args[0], (boolean) args[1]); + PlayerBannedEvent bannedEvent = new PlayerBannedEvent((PlayerBanData) args[0], (boolean) args[1]); + if (args.length > 2 && args[2] instanceof Message) { + bannedEvent.setKickMessage((Message) args[2]); + } + event = bannedEvent; break; case "PlayerUnbanEvent": event = new PlayerUnbanEvent((PlayerBanData) args[0], (PlayerData) args[1], (String) args[2], (boolean) args[3]); diff --git a/sponge-api7/src/main/java/me/confuser/banmanager/sponge/api/events/PlayerBannedEvent.java b/sponge-api7/src/main/java/me/confuser/banmanager/sponge/api/events/PlayerBannedEvent.java index b36e52fa0..5664dd97d 100644 --- a/sponge-api7/src/main/java/me/confuser/banmanager/sponge/api/events/PlayerBannedEvent.java +++ b/sponge-api7/src/main/java/me/confuser/banmanager/sponge/api/events/PlayerBannedEvent.java @@ -1,7 +1,9 @@ package me.confuser.banmanager.sponge.api.events; import lombok.Getter; +import lombok.Setter; import me.confuser.banmanager.common.data.PlayerBanData; +import me.confuser.banmanager.common.util.Message; public class PlayerBannedEvent extends SilentEvent { @@ -9,6 +11,10 @@ public class PlayerBannedEvent extends SilentEvent { @Getter private PlayerBanData ban; + @Getter + @Setter + private Message kickMessage; + public PlayerBannedEvent(PlayerBanData ban, boolean isSilent) { super(isSilent); this.ban = ban; diff --git a/sponge/src/main/java/me/confuser/banmanager/sponge/SpongeServer.java b/sponge/src/main/java/me/confuser/banmanager/sponge/SpongeServer.java index cd5d4023a..d7031da51 100644 --- a/sponge/src/main/java/me/confuser/banmanager/sponge/SpongeServer.java +++ b/sponge/src/main/java/me/confuser/banmanager/sponge/SpongeServer.java @@ -133,7 +133,11 @@ public CommonEvent callEvent(String name, Object... args) { event = new PlayerBanEvent((PlayerBanData) args[0], (boolean) args[1]); break; case "PlayerBannedEvent": - event = new PlayerBannedEvent((PlayerBanData) args[0], (boolean) args[1]); + PlayerBannedEvent bannedEvent = new PlayerBannedEvent((PlayerBanData) args[0], (boolean) args[1]); + if (args.length > 2 && args[2] instanceof Message) { + bannedEvent.setKickMessage((Message) args[2]); + } + event = bannedEvent; break; case "PlayerUnbanEvent": event = new PlayerUnbanEvent((PlayerBanData) args[0], (PlayerData) args[1], (String) args[2], (boolean) args[3]); diff --git a/sponge/src/main/java/me/confuser/banmanager/sponge/api/events/PlayerBannedEvent.java b/sponge/src/main/java/me/confuser/banmanager/sponge/api/events/PlayerBannedEvent.java index 5ede8e263..30d2bc710 100644 --- a/sponge/src/main/java/me/confuser/banmanager/sponge/api/events/PlayerBannedEvent.java +++ b/sponge/src/main/java/me/confuser/banmanager/sponge/api/events/PlayerBannedEvent.java @@ -1,13 +1,19 @@ package me.confuser.banmanager.sponge.api.events; import lombok.Getter; +import lombok.Setter; import me.confuser.banmanager.common.data.PlayerBanData; +import me.confuser.banmanager.common.util.Message; public class PlayerBannedEvent extends SilentEvent { @Getter private PlayerBanData ban; + @Getter + @Setter + private Message kickMessage; + public PlayerBannedEvent(PlayerBanData ban, boolean isSilent) { super(isSilent); this.ban = ban; diff --git a/velocity/src/main/java/me/confuser/banmanager/velocity/VelocityServer.java b/velocity/src/main/java/me/confuser/banmanager/velocity/VelocityServer.java index c8dc7fa9c..129f723d1 100644 --- a/velocity/src/main/java/me/confuser/banmanager/velocity/VelocityServer.java +++ b/velocity/src/main/java/me/confuser/banmanager/velocity/VelocityServer.java @@ -125,7 +125,11 @@ public CommonEvent callEvent(String name, Object... args) { event = new PlayerBanEvent((PlayerBanData) args[0], (boolean) args[1]); break; case "PlayerBannedEvent": - event = new PlayerBannedEvent((PlayerBanData) args[0], (boolean) args[1]); + PlayerBannedEvent bannedEvent = new PlayerBannedEvent((PlayerBanData) args[0], (boolean) args[1]); + if (args.length > 2 && args[2] instanceof Message) { + bannedEvent.setKickMessage((Message) args[2]); + } + event = bannedEvent; break; case "PlayerUnbanEvent": event = new PlayerUnbanEvent((PlayerBanData) args[0], (PlayerData) args[1], (String) args[2], (boolean) args[3]); diff --git a/velocity/src/main/java/me/confuser/banmanager/velocity/api/events/PlayerBannedEvent.java b/velocity/src/main/java/me/confuser/banmanager/velocity/api/events/PlayerBannedEvent.java index 471c21ae5..e4248ed29 100644 --- a/velocity/src/main/java/me/confuser/banmanager/velocity/api/events/PlayerBannedEvent.java +++ b/velocity/src/main/java/me/confuser/banmanager/velocity/api/events/PlayerBannedEvent.java @@ -1,7 +1,9 @@ package me.confuser.banmanager.velocity.api.events; import lombok.Getter; +import lombok.Setter; import me.confuser.banmanager.common.data.PlayerBanData; +import me.confuser.banmanager.common.util.Message; public class PlayerBannedEvent extends SilentEvent { @@ -9,6 +11,10 @@ public class PlayerBannedEvent extends SilentEvent { @Getter private PlayerBanData ban; + @Getter + @Setter + private Message kickMessage; + public PlayerBannedEvent(PlayerBanData ban, boolean isSilent) { super(isSilent); this.ban = ban; From bac2014636ae4859c186fa875e274983183ee0f2 Mon Sep 17 00:00:00 2001 From: James Mortemore Date: Fri, 3 Apr 2026 17:02:26 +0100 Subject: [PATCH 2/3] fix: replace fixed sleeps with notification-based polling in ban-unban e2e test Async RCON commands (bmban/bmunban) return immediately while processing on a background thread. Fixed sleeps were insufficient on slower CI servers, causing the "unbanned player can join server" test to time out. --- e2e/tests/src/ban-unban.test.ts | 41 ++++++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 8 deletions(-) diff --git a/e2e/tests/src/ban-unban.test.ts b/e2e/tests/src/ban-unban.test.ts index e326929e2..e0214c2fa 100644 --- a/e2e/tests/src/ban-unban.test.ts +++ b/e2e/tests/src/ban-unban.test.ts @@ -9,7 +9,14 @@ import { isPlayerInList, isProxy } from './helpers/rcon' -import { sleep, waitFor } from './helpers/config' +import { sleep, waitFor, waitForOrFalse } from './helpers/config' + +function hasNotification (bot: TestBot, ...keywords: string[]): boolean { + return bot.getSystemMessages().some(m => { + const lower = m.message.toLowerCase() + return keywords.every(k => lower.includes(k.toLowerCase())) + }) +} describe('Ban/Unban E2E Tests', () => { let staffBot: TestBot @@ -45,13 +52,22 @@ describe('Ban/Unban E2E Tests', () => { try { await unbanPlayer(TARGET_USERNAME) } catch { /* ignore */ } - await sleep(500) + // Wait for async unban to complete if player was banned + await waitForOrFalse( + () => hasNotification(staffBot, 'unban', TARGET_USERNAME), + { timeout: 5000, interval: 200 } + ) + + staffBot.clearSystemMessages() }) test('banned player cannot join server', async () => { - // First, ban the target await banPlayer(TARGET_USERNAME, 'Testing ban') - await sleep(1000) + + await waitFor( + () => hasNotification(staffBot, 'ban', TARGET_USERNAME), + { timeout: 10000, interval: 200, message: 'Ban notification not received' } + ) // Try to connect as the banned player try { @@ -73,12 +89,21 @@ describe('Ban/Unban E2E Tests', () => { }, 60000) test('unbanned player can join server', async () => { - // First, ban and then unban the target await banPlayer(TARGET_USERNAME, 'Testing ban') - await sleep(1000) + + await waitFor( + () => hasNotification(staffBot, 'ban', TARGET_USERNAME), + { timeout: 10000, interval: 200, message: 'Ban notification not received' } + ) + + staffBot.clearSystemMessages() + await unbanPlayer(TARGET_USERNAME) - // Allow extra time for async unban processing (especially on Sponge) - await sleep(2000) + + await waitFor( + () => hasNotification(staffBot, 'unban', TARGET_USERNAME), + { timeout: 10000, interval: 200, message: 'Unban notification not received' } + ) // Now try to connect as the unbanned player targetBot = await createBot(TARGET_USERNAME) From 1cd35e73512ba307c85ca93660bf4aa52f5ce16a Mon Sep 17 00:00:00 2001 From: James Mortemore Date: Fri, 3 Apr 2026 17:23:41 +0100 Subject: [PATCH 3/3] fix: use retry-based bot connection in ban-unban e2e test Replace notification-based polling (unreliable across platforms) with createBotWithRetry for the "unbanned player can join server" test and increase sleep durations. Kicked connections resolve in ~1-2s so retries are cheap, and the approach doesn't depend on staffBot message delivery. --- e2e/tests/src/ban-unban.test.ts | 57 ++++++++++++++------------------- 1 file changed, 24 insertions(+), 33 deletions(-) diff --git a/e2e/tests/src/ban-unban.test.ts b/e2e/tests/src/ban-unban.test.ts index e0214c2fa..25dd27051 100644 --- a/e2e/tests/src/ban-unban.test.ts +++ b/e2e/tests/src/ban-unban.test.ts @@ -9,13 +9,26 @@ import { isPlayerInList, isProxy } from './helpers/rcon' -import { sleep, waitFor, waitForOrFalse } from './helpers/config' +import { sleep, waitFor } from './helpers/config' -function hasNotification (bot: TestBot, ...keywords: string[]): boolean { - return bot.getSystemMessages().some(m => { - const lower = m.message.toLowerCase() - return keywords.every(k => lower.includes(k.toLowerCase())) - }) +async function createBotWithRetry ( + username: string, + maxAttempts: number = 5, + retryDelay: number = 3000 +): Promise { + let lastError: Error | null = null + + for (let i = 0; i < maxAttempts; i++) { + try { + return await createBot(username) + } catch (err) { + lastError = err as Error + console.log(`Bot connection attempt ${i + 1}/${maxAttempts} failed: ${lastError.message}`) + if (i < maxAttempts - 1) await sleep(retryDelay) + } + } + + throw lastError ?? new Error(`Failed to connect after ${maxAttempts} attempts`) } describe('Ban/Unban E2E Tests', () => { @@ -52,22 +65,12 @@ describe('Ban/Unban E2E Tests', () => { try { await unbanPlayer(TARGET_USERNAME) } catch { /* ignore */ } - // Wait for async unban to complete if player was banned - await waitForOrFalse( - () => hasNotification(staffBot, 'unban', TARGET_USERNAME), - { timeout: 5000, interval: 200 } - ) - - staffBot.clearSystemMessages() + await sleep(3000) }) test('banned player cannot join server', async () => { await banPlayer(TARGET_USERNAME, 'Testing ban') - - await waitFor( - () => hasNotification(staffBot, 'ban', TARGET_USERNAME), - { timeout: 10000, interval: 200, message: 'Ban notification not received' } - ) + await sleep(3000) // Try to connect as the banned player try { @@ -90,23 +93,11 @@ describe('Ban/Unban E2E Tests', () => { test('unbanned player can join server', async () => { await banPlayer(TARGET_USERNAME, 'Testing ban') - - await waitFor( - () => hasNotification(staffBot, 'ban', TARGET_USERNAME), - { timeout: 10000, interval: 200, message: 'Ban notification not received' } - ) - - staffBot.clearSystemMessages() - + await sleep(3000) await unbanPlayer(TARGET_USERNAME) - await waitFor( - () => hasNotification(staffBot, 'unban', TARGET_USERNAME), - { timeout: 10000, interval: 200, message: 'Unban notification not received' } - ) - - // Now try to connect as the unbanned player - targetBot = await createBot(TARGET_USERNAME) + // Retry connection -- async unban may still be processing + targetBot = await createBotWithRetry(TARGET_USERNAME) await waitFor( async () => isPlayerInList(TARGET_USERNAME), { timeout: 10000, interval: 500, message: 'Unbanned player not in player list' }