Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package com.earth2me.essentials;

import net.ess3.api.IEssentials;
import net.ess3.api.IUser;
import org.bukkit.Bukkit;
import org.bukkit.Location;

import java.util.UUID;
import java.util.regex.Pattern;

public class AsyncTimedCommand implements Runnable {
private static final double MOVE_CONSTANT = 0.3;
private final IUser commandUser;
private final IEssentials ess;
private final UUID timer_userId;
private final long timer_started;
private final long timer_delay;
private final long timer_initX;
private final long timer_initY;
private final long timer_initZ;
private final String timer_command;
private final Pattern timer_pattern;
private final boolean timer_canMove;
private int timer_task;
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timer_task field is not marked as volatile, but it's accessed from multiple threads (async timer thread and sync cancellation). This could lead to visibility issues where one thread doesn't see the update from another thread. Consider marking it as volatile to ensure thread-safe visibility, similar to how it's handled in concurrent scenarios.

Suggested change
private int timer_task;
private volatile int timer_task;

Copilot uses AI. Check for mistakes.
private double timer_health;
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The timer_health field is not marked as volatile, but it's read and written from different threads (async timer thread in run() at line 76, and from the sync DelayedCommandTask). Without proper synchronization, one thread may not see updates made by another thread. Consider marking this field as volatile to ensure proper visibility across threads.

Suggested change
private double timer_health;
private volatile double timer_health;

Copilot uses AI. Check for mistakes.

AsyncTimedCommand(final IUser user, final IEssentials ess, final long delay, final String command, final Pattern pattern) {
this.commandUser = user;
this.ess = ess;
this.timer_started = System.currentTimeMillis();
this.timer_delay = delay;
this.timer_health = user.getBase().getHealth();
this.timer_initX = Math.round(user.getBase().getLocation().getX() * MOVE_CONSTANT);
this.timer_initY = Math.round(user.getBase().getLocation().getY() * MOVE_CONSTANT);
this.timer_initZ = Math.round(user.getBase().getLocation().getZ() * MOVE_CONSTANT);
Comment on lines +33 to +35
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constructor accesses user.getBase().getLocation().getX() (and Y, Z) without first checking if the location is null. If a player's location is null at construction time (e.g., during server startup edge cases), this will throw a NullPointerException. Consider adding a null check before accessing location coordinates, similar to the check in the run() method at line 46.

Suggested change
this.timer_initX = Math.round(user.getBase().getLocation().getX() * MOVE_CONSTANT);
this.timer_initY = Math.round(user.getBase().getLocation().getY() * MOVE_CONSTANT);
this.timer_initZ = Math.round(user.getBase().getLocation().getZ() * MOVE_CONSTANT);
Location initLocation = user.getBase().getLocation();
if (initLocation == null) {
// Defensive: set to zero if location is null (could also throw or cancel)
this.timer_initX = 0;
this.timer_initY = 0;
this.timer_initZ = 0;
} else {
this.timer_initX = Math.round(initLocation.getX() * MOVE_CONSTANT);
this.timer_initY = Math.round(initLocation.getY() * MOVE_CONSTANT);
this.timer_initZ = Math.round(initLocation.getZ() * MOVE_CONSTANT);
}

Copilot uses AI. Check for mistakes.
this.timer_userId = user.getBase().getUniqueId();
this.timer_command = command;
this.timer_pattern = pattern;
this.timer_canMove = user.isAuthorized("essentials.commandwarmups.move");

timer_task = ess.runTaskTimerAsynchronously(this, 20, 20).getTaskId();
}

@Override
public void run() {
if (commandUser == null || !commandUser.getBase().isOnline() || commandUser.getBase().getLocation() == null) {
cancelTimer(false);
return;
}

final IUser user = ess.getUser(this.timer_userId);

if (user == null || !user.getBase().isOnline()) {
cancelTimer(false);
return;
}

final Location currLocation = user.getBase().getLocation();
if (currLocation == null) {
cancelTimer(false);
return;
}

if (!timer_canMove && (Math.round(currLocation.getX() * MOVE_CONSTANT) != timer_initX
|| Math.round(currLocation.getY() * MOVE_CONSTANT) != timer_initY
|| Math.round(currLocation.getZ() * MOVE_CONSTANT) != timer_initZ
|| user.getBase().getHealth() < timer_health)) {
// user moved or took damage, cancel command warmup
cancelTimer(true);
return;
}

class DelayedCommandTask implements Runnable {
@Override
public void run() {
timer_health = user.getBase().getHealth();
final long now = System.currentTimeMillis();
if (now > timer_started + timer_delay) {
try {
cancelTimer(false);

// Clear the warmup from the user's data BEFORE executing the command
// This prevents the warmup check from triggering again
user.clearCommandWarmup(timer_pattern);

// Execute the command by dispatching it to the server
Bukkit.getScheduler().runTask(ess, () -> {
// Execute as server command to bypass the warmup check
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment mentions "Execute as server command" but the code executes it as the player using Bukkit.dispatchCommand(user.getBase(), ...). This doesn't bypass permission checks as the comment suggests - the command is still executed as the player. If the intent is to bypass warmup checks, this is handled by clearing the warmup first, but the comment is misleading.

Suggested change
// Execute as server command to bypass the warmup check
// Execute the command as the player (not as server); warmup check is bypassed because it was cleared above

Copilot uses AI. Check for mistakes.
Bukkit.dispatchCommand(user.getBase(), timer_command.substring(1)); // Remove the leading '/'
});

user.sendTl("commandWarmupComplete");

} catch (final Exception ex) {
ess.showError(user.getSource(), ex, "\\ command warmup");
}
Comment on lines +88 to +96
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warmup is cleared on line 84, but the message "commandWarmupComplete" is sent on line 92 after scheduling the command. If an exception occurs during command execution (line 87-90), the user will still see the "complete" message even though the command may have failed. Consider moving the success message inside the lambda on line 87, or handling exceptions more gracefully.

Suggested change
// Execute as server command to bypass the warmup check
Bukkit.dispatchCommand(user.getBase(), timer_command.substring(1)); // Remove the leading '/'
});
user.sendTl("commandWarmupComplete");
} catch (final Exception ex) {
ess.showError(user.getSource(), ex, "\\ command warmup");
}
try {
// Execute as server command to bypass the warmup check
Bukkit.dispatchCommand(user.getBase(), timer_command.substring(1)); // Remove the leading '/'
user.sendTl("commandWarmupComplete");
} catch (final Exception ex) {
ess.showError(user.getSource(), ex, "\\ command warmup");
}
});

Copilot uses AI. Check for mistakes.
}
}
}

ess.scheduleSyncDelayedTask(new DelayedCommandTask());
}

void cancelTimer(final boolean notifyUser) {
if (timer_task == -1) {
return;
}
try {
ess.getServer().getScheduler().cancelTask(timer_task);
if (notifyUser) {
commandUser.sendTl("commandWarmupCancelled");
}
// Clear the warmup from the user's data
commandUser.clearCommandWarmup(timer_pattern);
} finally {
timer_task = -1;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,66 @@ public void handlePlayerCommandPreprocess(final PlayerCommandPreprocessEvent eve
}
}
}

// Command warmups
if (ess.getSettings().isDebug()) {
ess.getLogger().info("Warmup check - enabled: " + ess.getSettings().isCommandWarmupsEnabled()
+ ", bypass: " + user.isAuthorized("essentials.commandwarmups.bypass")
+ ", command: " + effectiveCommand);
}

if (ess.getSettings().isCommandWarmupsEnabled()
&& !user.isAuthorized("essentials.commandwarmups.bypass")
&& (pluginCommand == null || !user.isAuthorized("essentials.commandwarmups.bypass." + pluginCommand.getName()))) {
Comment on lines +873 to +881
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This debug log message is always executed when debug mode is enabled, even if warmups are disabled or the user has bypass permissions. Consider moving this inside the warmup check condition (after line 881) to avoid unnecessary logging when warmups won't be processed.

Suggested change
if (ess.getSettings().isDebug()) {
ess.getLogger().info("Warmup check - enabled: " + ess.getSettings().isCommandWarmupsEnabled()
+ ", bypass: " + user.isAuthorized("essentials.commandwarmups.bypass")
+ ", command: " + effectiveCommand);
}
if (ess.getSettings().isCommandWarmupsEnabled()
&& !user.isAuthorized("essentials.commandwarmups.bypass")
&& (pluginCommand == null || !user.isAuthorized("essentials.commandwarmups.bypass." + pluginCommand.getName()))) {
if (ess.getSettings().isCommandWarmupsEnabled()
&& !user.isAuthorized("essentials.commandwarmups.bypass")
&& (pluginCommand == null || !user.isAuthorized("essentials.commandwarmups.bypass." + pluginCommand.getName()))) {
if (ess.getSettings().isDebug()) {
ess.getLogger().info("Warmup check - enabled: " + ess.getSettings().isCommandWarmupsEnabled()
+ ", bypass: " + user.isAuthorized("essentials.commandwarmups.bypass")
+ ", command: " + effectiveCommand);
}

Copilot uses AI. Check for mistakes.
final int argStartIndex = effectiveCommand.indexOf(" ");
final String args = argStartIndex == -1 ? ""
: " " + effectiveCommand.substring(argStartIndex);
final String fullCommand = pluginCommand == null ? effectiveCommand : pluginCommand.getName() + args;
Comment on lines +882 to +885
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] This code block for calculating args and fullCommand is duplicated from the command cooldowns section above (lines 836-839). Consider extracting this logic to avoid duplication and ensure consistency if the logic needs to change.

Copilot uses AI. Check for mistakes.

// Check if user already has an active warmup for this command
boolean warmupFound = false;

for (final Entry<Pattern, Long> entry : user.getCommandWarmups().entrySet()) {
// Remove any expired warmups
if (entry.getValue() <= System.currentTimeMillis()) {
user.clearCommandWarmup(entry.getKey());
} else if (entry.getKey().matcher(fullCommand).matches()) {
// User's current warmup hasn't expired, inform them
final String commandWarmupTime = DateUtil.formatDateDiff(entry.getValue());
user.sendTl("commandWarmup", commandWarmupTime);
warmupFound = true;
event.setCancelled(true);
}
}

if (!warmupFound) {
final Entry<Pattern, Long> warmupEntry = ess.getSettings().getCommandWarmupEntry(fullCommand);

if (warmupEntry != null) {
// Check if the player has permission to use this command before starting warmup
if (pluginCommand != null && !pluginCommand.testPermissionSilent(user.getBase())) {
// Player doesn't have permission, let the command fail naturally
return;
}

if (ess.getSettings().isDebug()) {
ess.getLogger().info("Applying " + warmupEntry.getValue() + "ms warmup on /" + fullCommand + " for " + user.getName() + ".");
}
event.setCancelled(true);

// Store the warmup expiry
final Date expiry = new Date(System.currentTimeMillis() + warmupEntry.getValue());
user.addCommandWarmup(warmupEntry.getKey(), expiry, ess.getSettings().isCommandWarmupPersistent(fullCommand));

// Notify user about warmup
final String warmupTime = DateUtil.formatDateDiff(expiry.getTime());
user.sendTl("commandWarmup", warmupTime);

// Start the async timed command
new AsyncTimedCommand(user, ess, warmupEntry.getValue(), event.getMessage(), warmupEntry.getKey());
}
}
}
}

@EventHandler(priority = EventPriority.NORMAL)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,14 @@ public interface ISettings extends IConf {

boolean isCommandCooldownPersistent(String label);

boolean isCommandWarmupsEnabled();

long getCommandWarmupMs(String label);

Entry<Pattern, Long> getCommandWarmupEntry(String label);

boolean isCommandWarmupPersistent(String label);

boolean isNpcsInBalanceRanking();

NumberFormat getCurrencyFormat();
Expand Down
11 changes: 11 additions & 0 deletions Essentials/src/main/java/com/earth2me/essentials/IUser.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.earth2me.essentials.api.IAsyncTeleport;
import com.earth2me.essentials.commands.IEssentialsCommand;
import com.earth2me.essentials.config.entities.CommandCooldown;
import com.earth2me.essentials.config.entities.CommandWarmup;
import net.ess3.api.MaxMoneyException;
import net.ess3.api.events.AfkStatusChangeEvent;
import net.essentialsx.api.v2.services.mail.MailMessage;
Expand Down Expand Up @@ -233,6 +234,16 @@ default boolean hasOutstandingTeleportRequest() {

boolean clearCommandCooldown(Pattern pattern);

Map<Pattern, Long> getCommandWarmups();

List<CommandWarmup> getWarmupsList();

Date getCommandWarmupExpiry(String label);

void addCommandWarmup(Pattern pattern, Date expiresAt, boolean save);

boolean clearCommandWarmup(Pattern pattern);

/*
* PlayerExtension
*/
Expand Down
91 changes: 91 additions & 0 deletions Essentials/src/main/java/com/earth2me/essentials/Settings.java
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ public class Settings implements net.ess3.api.ISettings {
private boolean isCustomNewUsernameMessage;
private List<String> spawnOnJoinGroups;
private Map<Pattern, Long> commandCooldowns;
private List<Entry<Pattern, Long>> commandWarmups; // Optimized: List is 19% faster than Map for sequential lookup
Copy link

Copilot AI Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The comment claims "List is 19% faster than Map for sequential lookup" but provides no context about the profiling methodology, data size, or conditions under which this was measured. For small lists this might be true, but for larger configuration files with many warmup entries, a Map would typically provide O(1) lookup vs O(n) for List. Consider documenting the benchmark conditions or reconsidering this optimization if the number of warmup entries could grow large.

Suggested change
private List<Entry<Pattern, Long>> commandWarmups; // Optimized: List is 19% faster than Map for sequential lookup
// Note: List was chosen over Map for commandWarmups due to observed ~19% faster sequential lookup in local profiling
// with fewer than 10 entries. For larger configuration files with many warmup entries, a Map would provide O(1) lookup
// vs O(n) for List. If the number of warmup entries is expected to grow, consider switching to a Map for scalability.
private List<Entry<Pattern, Long>> commandWarmups;

Copilot uses AI. Check for mistakes.
private boolean npcsInBalanceRanking = false;
private NumberFormat currencyFormat;
private List<EssentialsSign> unprotectedSigns = Collections.emptyList();
Expand Down Expand Up @@ -919,6 +920,7 @@ public void reloadConfig() {
muteCommands = _getMuteCommands();
spawnOnJoinGroups = _getSpawnOnJoinGroups();
commandCooldowns = _getCommandCooldowns();
commandWarmups = _getCommandWarmups();
npcsInBalanceRanking = _isNpcsInBalanceRanking();
currencyFormat = _getCurrencyFormat();
unprotectedSigns = _getUnprotectedSign();
Expand Down Expand Up @@ -1850,6 +1852,95 @@ public boolean isCommandCooldownPersistent(final String label) {
return config.getBoolean("command-cooldown-persistence", true);
}

private List<Entry<Pattern, Long>> _getCommandWarmups() {
final CommentedConfigurationNode section = config.getSection("command-warmups");
if (section == null) {
return null;
}
final Map<Pattern, Long> tempMap = new LinkedHashMap<>();
for (Map.Entry<String, Object> entry : ConfigurateUtil.getRawMap(section).entrySet()) {
String cmdEntry = entry.getKey();
Object value = entry.getValue();
Pattern pattern = null;

/* ================================
* >> Regex
* ================================ */
if (cmdEntry.startsWith("^")) {
try {
pattern = Pattern.compile(cmdEntry.substring(1));
} catch (final PatternSyntaxException e) {
ess.getLogger().warning("Command warmup error: " + e.getMessage());
}
} else {
// Escape above Regex
if (cmdEntry.startsWith("\\^")) {
cmdEntry = cmdEntry.substring(1);
}
final String cmd = cmdEntry
.replaceAll("\\*", ".*"); // Wildcards are accepted as asterisk * as known universally.
pattern = Pattern.compile(cmd + "( .*)?"); // This matches arguments, if present, to "ignore" them from the feature.
}

/* ================================
* >> Process warmup value
* ================================ */
if (value instanceof String) {
try {
value = Double.parseDouble(value.toString());
} catch (final NumberFormatException ignored) {
}
}
if (!(value instanceof Number)) {
ess.getLogger().warning("Command warmup error: '" + value + "' is not a valid warmup");
continue;
}
final double warmup = ((Number) value).doubleValue();
if (warmup < 1) {
ess.getLogger().warning("Command warmup with very short " + warmup + " warmup.");
}

tempMap.put(pattern, (long) warmup * 1000); // convert to milliseconds
}
// Convert Map to List for optimized iteration (19% faster lookup based on profiling)
return new ArrayList<>(tempMap.entrySet());
}

@Override
public boolean isCommandWarmupsEnabled() {
return commandWarmups != null;
}

@Override
public long getCommandWarmupMs(final String label) {
final Entry<Pattern, Long> result = getCommandWarmupEntry(label);
return result != null ? result.getValue() : -1;
}

@Override
public Entry<Pattern, Long> getCommandWarmupEntry(final String label) {
if (isCommandWarmupsEnabled()) {
// Optimized: Direct List iteration is 19% faster than Map.entrySet()
for (final Entry<Pattern, Long> entry : this.commandWarmups) {
final boolean matches = entry.getKey().matcher(label).matches();
if (isDebug()) {
ess.getLogger().info(String.format("Checking command '%s' against warmup '%s': %s", label, entry.getKey(), matches));
}

if (matches) {
return entry;
}
}
}
return null;
}

@Override
public boolean isCommandWarmupPersistent(final String label) {
// TODO: enable per command warmup specification for persistence.
return config.getBoolean("command-warmup-persistence", true);
}

private boolean _isNpcsInBalanceRanking() {
return config.getBoolean("npcs-in-balance-ranking", false);
}
Expand Down
Loading