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
Expand Up @@ -15,6 +15,7 @@
import com.eternalcode.example.feature.randomteleport.ApiRandomTeleportCommand;
import com.eternalcode.example.feature.randomteleport.ApiRandomTeleportListener;
import com.eternalcode.example.feature.spawn.ApiSpawnCommand;
import com.eternalcode.example.feature.teleportrequest.ApiTeleportRequestListener;
import dev.rollczi.litecommands.LiteCommands;
import dev.rollczi.litecommands.bukkit.LiteBukkitFactory;
import dev.rollczi.litecommands.bukkit.LiteBukkitMessages;
Expand All @@ -38,32 +39,33 @@ public void onEnable() {
EternalCoreApi provide = EternalCoreApiProvider.provide();

this.liteCommands = LiteBukkitFactory.builder(FALLBACK_PREFIX, this, server)
.message(LiteBukkitMessages.PLAYER_ONLY, input -> "You must be a player to execute this command!")
.message(LiteBukkitMessages.PLAYER_NOT_FOUND, input -> "Player not found!")
.message(LiteMessages.MISSING_PERMISSIONS, input -> "You don't have permission to execute this command!")
.message(LiteBukkitMessages.PLAYER_ONLY, input -> "You must be a player to execute this command!")
.message(LiteBukkitMessages.PLAYER_NOT_FOUND, input -> "Player not found!")
.message(LiteMessages.MISSING_PERMISSIONS,
input -> "You don't have permission to execute this command!")

.commands(
new ApiAfkCommand(provide.getAfkService()),
new ApiIgnoreCommand(provide.getIgnoreService()),
new ApiJailCommand(provide.getJailService()),
new ApiRandomTeleportCommand(provide.getRandomTeleportService()),
new ApiSpawnCommand(provide.getSpawnService()),
new ApiRandomTeleportCommand(provide.getRandomTeleportService()),
new ApiHomeCommand(provide.getHomeService())
)
.commands(
new ApiAfkCommand(provide.getAfkService()),
new ApiIgnoreCommand(provide.getIgnoreService()),
new ApiJailCommand(provide.getJailService()),
new ApiRandomTeleportCommand(provide.getRandomTeleportService()),
new ApiSpawnCommand(provide.getSpawnService()),
new ApiRandomTeleportCommand(provide.getRandomTeleportService()),
new ApiHomeCommand(provide.getHomeService()))
Comment on lines +47 to +54
Copy link
Contributor

Choose a reason for hiding this comment

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

high

ApiRandomTeleportCommand is registered twice. This will likely cause an error during command registration as the same command will be registered multiple times. Please remove the duplicate entry on line 53.

                .commands(
                        new ApiAfkCommand(provide.getAfkService()),
                        new ApiIgnoreCommand(provide.getIgnoreService()),
                        new ApiJailCommand(provide.getJailService()),
                        new ApiRandomTeleportCommand(provide.getRandomTeleportService()),
                        new ApiSpawnCommand(provide.getSpawnService()),
                        new ApiHomeCommand(provide.getHomeService()))


.build();
.build();

Stream.of(
new ApiAfkListener(),
new CatBoyListener(provide.getCatboyService()),
new ApiRandomTeleportListener(provide.getRandomTeleportService()),
new ApiPrivateChatListener(server),
new ApiRandomTeleportListener(provide.getRandomTeleportService()),
new ApiHomeListener(server),
new ApiJailListener(server),
new ApiIgnoreListener()
).forEach(listener -> server.getPluginManager().registerEvents(listener, this));
new ApiAfkListener(),
new CatBoyListener(provide.getCatboyService()),
new ApiRandomTeleportListener(provide.getRandomTeleportService()),
new ApiPrivateChatListener(server),
new ApiRandomTeleportListener(provide.getRandomTeleportService()),
new ApiHomeListener(server),
new ApiJailListener(server),
new ApiIgnoreListener(),
new ApiTeleportRequestListener())
.forEach(listener -> server.getPluginManager().registerEvents(listener, this));
Comment on lines 58 to +68
Copy link
Contributor

Choose a reason for hiding this comment

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

high

ApiRandomTeleportListener is registered twice. This will cause its event handlers to be called twice for each event, which is likely not intended and can lead to bugs. Please remove the duplicate entry on line 63.

        Stream.of(
                new ApiAfkListener(),
                new CatBoyListener(provide.getCatboyService()),
                new ApiRandomTeleportListener(provide.getRandomTeleportService()),
                new ApiPrivateChatListener(server),
                new ApiHomeListener(server),
                new ApiJailListener(server),
                new ApiIgnoreListener(),
                new ApiTeleportRequestListener())
                .forEach(listener -> server.getPluginManager().registerEvents(listener, this));

}

@Override
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package com.eternalcode.example.feature.teleportrequest;

import com.eternalcode.core.feature.teleportrequest.PreTeleportRequestEvent;
import com.eternalcode.core.feature.teleportrequest.TeleportRequestEvent;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.Listener;

public class ApiTeleportRequestListener implements Listener {

@EventHandler
public void onPreTeleportRequest(final PreTeleportRequestEvent event) {
final Player sender = event.getSender();
final Player target = event.getTarget();

sender.sendMessage(String.format(
"PreTeleportRequestEvent: %s wants to teleport to %s (EternalCore API).",
sender.getName(),
target.getName()
));

/*
if (!countryService.areInSameCountry(sender, target)) {
sender.sendMessage("You are not in the same country!");
event.setCancelled(true);
}
*/
}

@EventHandler
public void onTeleportRequest(final TeleportRequestEvent event) {
final Player sender = event.getSender();
final Player target = event.getTarget();

sender.sendMessage(String.format(
"TeleportRequestEvent: %s sent a teleport request to %s (EternalCore API).",
sender.getName(),
target.getName()
));
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
package com.eternalcode.core.feature.teleportrequest;

import org.bukkit.entity.Player;
import org.bukkit.event.Cancellable;
import org.bukkit.event.Event;
import org.bukkit.event.HandlerList;

/**
* Called before a teleport request is sent (via /tpa or /tpahere).
* If cancelled, the teleport request will not be created.
*/
public class PreTeleportRequestEvent extends Event implements Cancellable {

private static final HandlerList HANDLER_LIST = new HandlerList();

private final Player sender;
private final Player target;
private boolean cancelled;

public PreTeleportRequestEvent(Player sender, Player target) {
super(false);

this.sender = sender;
this.target = target;
}

public static HandlerList getHandlerList() {
return HANDLER_LIST;
}

public Player getSender() {
return this.sender;
}

public Player getTarget() {
return this.target;
}

@Override
public boolean isCancelled() {
return this.cancelled;
}

@Override
public void setCancelled(boolean cancel) {
this.cancelled = cancel;
}

@Override
public HandlerList getHandlers() {
return HANDLER_LIST;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package com.eternalcode.core.feature.teleportrequest;

import org.bukkit.entity.Player;
import org.bukkit.event.Event;
import org.bukkit.event.HandlerList;

/**
* Called after a teleport request has been successfully created (via /tpa or
* /tpahere).
* This event is informational and cannot be cancelled.
*/
public class TeleportRequestEvent extends Event {

private static final HandlerList HANDLER_LIST = new HandlerList();

private final Player sender;
private final Player target;

public TeleportRequestEvent(Player sender, Player target) {
super(false);

this.sender = sender;
this.target = target;
}

public static HandlerList getHandlerList() {
return HANDLER_LIST;
}

public Player getSender() {
return this.sender;
}

public Player getTarget() {
return this.target;
}

@Override
public HandlerList getHandlers() {
return HANDLER_LIST;
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.eternalcode.core.feature.teleportrequest;

import com.eternalcode.annotations.scan.command.DescriptionDocs;
import com.eternalcode.core.event.EventCaller;
import com.eternalcode.core.feature.ignore.IgnoreService;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.notice.NoticeService;
Expand All @@ -19,12 +20,19 @@ class TpaCommand {
private final TeleportRequestService requestService;
private final IgnoreService ignoreService;
private final NoticeService noticeService;
private final EventCaller eventCaller;

@Inject
TpaCommand(TeleportRequestService requestService, IgnoreService ignoreService, NoticeService noticeService) {
TpaCommand(
TeleportRequestService requestService,
IgnoreService ignoreService,
NoticeService noticeService,
EventCaller eventCaller
) {
this.requestService = requestService;
this.ignoreService = ignoreService;
this.noticeService = noticeService;
this.eventCaller = eventCaller;
}

@Execute
Expand All @@ -42,6 +50,12 @@ void execute(@Sender Player player, @Arg Player target) {
return;
}

PreTeleportRequestEvent preEvent = this.eventCaller.callEvent(new PreTeleportRequestEvent(player, target));

if (preEvent.isCancelled()) {
return;
}

this.noticeService
.create()
.player(player.getUniqueId())
Expand All @@ -67,6 +81,7 @@ void execute(@Sender Player player, @Arg Player target) {
.send();

this.requestService.createRequest(player.getUniqueId(), target.getUniqueId());
this.eventCaller.callEvent(new TeleportRequestEvent(player, target));
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

You are calling a synchronous Bukkit event (TeleportRequestEvent) from an asynchronous context (CompletableFuture#thenAccept). This is not thread-safe and can lead to severe concurrency issues. Bukkit API calls, including firing events, must be performed on the main server thread unless the event is explicitly asynchronous.

To resolve this, you need to schedule the event call on the main thread. For example, using Bukkit's scheduler:

Bukkit.getScheduler().runTask(pluginInstance, () -> this.eventCaller.callEvent(new TeleportRequestEvent(player, target)));

You will need to get access to your plugin instance to do this. A better approach might be to inject a scheduler service that abstracts this away, which seems to be a pattern in this project.

});
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
package com.eternalcode.core.feature.teleportrequest.self;

import com.eternalcode.annotations.scan.command.DescriptionDocs;
import com.eternalcode.core.event.EventCaller;
import com.eternalcode.core.feature.ignore.IgnoreService;
import com.eternalcode.core.feature.teleportrequest.PreTeleportRequestEvent;
import com.eternalcode.core.feature.teleportrequest.TeleportRequestEvent;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.notice.NoticeService;
import dev.rollczi.litecommands.annotations.argument.Arg;
import dev.rollczi.litecommands.annotations.command.Command;
import dev.rollczi.litecommands.annotations.context.Sender;
import dev.rollczi.litecommands.annotations.execute.Execute;
import dev.rollczi.litecommands.annotations.permission.Permission;
import org.bukkit.entity.Player;

import java.util.concurrent.CompletableFuture;
import org.bukkit.entity.Player;

@Command(name = "tpahere")
@Permission("eternalcore.tpahere")
Expand All @@ -20,19 +22,26 @@ class TpaHereCommand {
private final TeleportHereRequestService requestService;
private final IgnoreService ignoreService;
private final NoticeService noticeService;
private final EventCaller eventCaller;

@Inject
TpaHereCommand(TeleportHereRequestService requestService, IgnoreService ignoreService, NoticeService noticeService) {
TpaHereCommand(
TeleportHereRequestService requestService,
IgnoreService ignoreService,
NoticeService noticeService,
EventCaller eventCaller
) {
this.requestService = requestService;
this.ignoreService = ignoreService;
this.noticeService = noticeService;
this.eventCaller = eventCaller;
}

@Execute
@DescriptionDocs(description = "Send teleport request to player to teleport to you", arguments = "<player>")
void execute(@Sender Player sender, @Arg Player target) {
if (sender.equals(target)) {
this.noticeService.player(sender.getUniqueId() , translation -> translation.tpa().tpaSelfMessage());
this.noticeService.player(sender.getUniqueId(), translation -> translation.tpa().tpaSelfMessage());

return;
}
Expand All @@ -43,34 +52,41 @@ void execute(@Sender Player sender, @Arg Player target) {
return;
}

PreTeleportRequestEvent preEvent = this.eventCaller.callEvent(new PreTeleportRequestEvent(sender, target));

if (preEvent.isCancelled()) {
return;
}

this.isIgnoring(target, sender).thenAccept(isIgnoring -> {
if (isIgnoring) {
this.noticeService.create()
.player(sender.getUniqueId())
.notice(translation -> translation.tpa().tpaTargetIgnoresYou())
.placeholder("{PLAYER}", target.getName())
.send();
return;
}

this.noticeService
.create()
.player(sender.getUniqueId())
.notice(translation -> translation.tpa().tpaHereSent())
.placeholder("{PLAYER}", target.getName())
.send();

this.noticeService.create()
if (isIgnoring) {
this.noticeService.create()
.player(sender.getUniqueId())
.notice(translation -> translation.tpa().tpaTargetIgnoresYou())
.placeholder("{PLAYER}", target.getName())
.send();
return;
}

this.noticeService
.create()
.player(sender.getUniqueId())
.notice(translation -> translation.tpa().tpaHereSent())
.placeholder("{PLAYER}", target.getName())
.send();

this.noticeService.create()
.player(target.getUniqueId())
.notice(translation -> translation.tpa().tpaHereReceived())
.placeholder("{PLAYER}", sender.getName())
.send();

this.requestService.createRequest(sender.getUniqueId(), target.getUniqueId());
this.requestService.createRequest(sender.getUniqueId(), target.getUniqueId());
this.eventCaller.callEvent(new TeleportRequestEvent(sender, target));
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

You are calling a synchronous Bukkit event (TeleportRequestEvent) from an asynchronous context (CompletableFuture#thenAccept). This is not thread-safe and can lead to severe concurrency issues. Bukkit API calls, including firing events, must be performed on the main server thread unless the event is explicitly asynchronous.

To resolve this, you need to schedule the event call on the main thread. For example, using Bukkit's scheduler:

Bukkit.getScheduler().runTask(pluginInstance, () -> this.eventCaller.callEvent(new TeleportRequestEvent(sender, target)));

You will need to get access to your plugin instance to do this. A better approach might be to inject a scheduler service that abstracts this away, which seems to be a pattern in this project.

});
}

@Execute(name = "-all", aliases = { "*" })
@Execute(name = "-all", aliases = {"*"})
@Permission("eternalcore.tpahere.all")
@DescriptionDocs(description = "Send teleport request to all online players to teleport to you")
void executeAll(@Sender Player sender) {
Expand All @@ -83,6 +99,12 @@ void executeAll(@Sender Player sender) {
continue;
}

PreTeleportRequestEvent preEvent = this.eventCaller.callEvent(new PreTeleportRequestEvent(sender, target));

if (preEvent.isCancelled()) {
continue;
}

this.isIgnoring(target, sender).thenAccept(isIgnoring -> {
if (isIgnoring) {
return;
Expand All @@ -95,7 +117,7 @@ void executeAll(@Sender Player sender) {
.send();

this.requestService.createRequest(sender.getUniqueId(), target.getUniqueId());

this.eventCaller.callEvent(new TeleportRequestEvent(sender, target));
Copy link
Contributor

Choose a reason for hiding this comment

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

critical

The same threading issue as in the execute method is present here. You are calling a synchronous Bukkit event (TeleportRequestEvent) from an asynchronous context (CompletableFuture#thenAccept) inside a loop. This is not thread-safe and can lead to severe concurrency issues. Bukkit API calls, including firing events, must be performed on the main server thread.

To resolve this, you need to schedule the event call on the main thread. For example, using Bukkit's scheduler:

Bukkit.getScheduler().runTask(pluginInstance, () -> this.eventCaller.callEvent(new TeleportRequestEvent(sender, target)));

You will need to get access to your plugin instance to do this. A better approach might be to inject a scheduler service that abstracts this away.

});
}

Expand All @@ -105,8 +127,6 @@ void executeAll(@Sender Player sender) {
.send();
}



private CompletableFuture<Boolean> isIgnoring(Player target, Player sender) {
return this.ignoreService.isIgnored(target.getUniqueId(), sender.getUniqueId());
}
Expand Down