Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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,17 @@
package com.eternalcode.core.configuration.migrations;

import eu.okaeri.configs.migrate.builtin.NamedMigration;

import static eu.okaeri.configs.migrate.ConfigMigrationDsl.move;

public class Migration_0010_Move_back_to_dedicated_section extends NamedMigration {
Migration_0010_Move_back_to_dedicated_section() {
super(
"Improve homes config",
move("teleport.teleportedToLastLocation", "back.lastLocationNoExist"),
move("teleport.teleportedSpecifiedPlayerLastLocation", "back.teleportedSpecifiedPlayerLastLocation"),
move("teleport.lastLocationNoExist", "back.lastLocationNoExist")
);
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
package com.eternalcode.core.feature.back;

import com.eternalcode.annotations.scan.command.DescriptionDocs;
import com.eternalcode.core.feature.back.BackService.BackLocation;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.notice.NoticeService;
import com.eternalcode.core.viewer.Viewer;
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 java.util.Optional;
import org.bukkit.entity.Player;
import panda.std.Pair;

@Command(name = "back")
public class BackCommand {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't like this class. I would suggest something like this;

Suggested change
public class BackCommand {
import com.eternalcode.annotations.scan.command.DescriptionDocs;
import com.eternalcode.core.feature.back.BackService;
import com.eternalcode.core.feature.back.BackService.BackLocation;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.notice.NoticeService;
import com.eternalcode.core.viewer.Viewer;
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 panda.std.Pair;
import java.util.Optional;
import java.util.UUID;
import java.util.function.Function;
@Command(name = "back")
public class BackCommand {
private static final String PLAYER_PLACEHOLDER = "{PLAYER}";
private final BackService backService;
private final NoticeService noticeService;
@Inject
public BackCommand(BackService backService, NoticeService noticeService) {
this.backService = backService;
this.noticeService = noticeService;
}
@Execute(name = "death")
@Permission("eternalcore.back.death")
@DescriptionDocs(description = "Teleport to your last death location")
public void executeBackDeath(@Sender Player player) {
teleportSelf(player, this::pickDeath);
}
@Execute(name = "teleport")
@Permission("eternalcore.back.teleport")
@DescriptionDocs(description = "Teleport to your last teleport location")
public void executeBackTeleport(@Sender Player player) {
teleportSelf(player, this::pickTeleport);
}
@Execute(name = "death")
@Permission("eternalcore.back.death.other")
@DescriptionDocs(description = "Teleport specified player to their last death location", arguments = "<player>")
public void executeBackDeathOther(@Sender Viewer viewer, @Arg Player target) {
teleportOther(viewer, target, this::pickDeath);
}
@Execute(name = "teleport")
@Permission("eternalcore.back.teleport.other")
@DescriptionDocs(description = "Teleport specified player to their last teleport location", arguments = "<player>")
public void executeBackTeleportOther(@Sender Viewer viewer, @Arg Player target) {
teleportOther(viewer, target, this::pickTeleport);
}
private void teleportSelf(Player player, Function<Pair<BackLocation, BackLocation>, BackLocation> selector) {
resolveBack(player.getUniqueId(), selector).ifPresentOrElse(
loc -> {
this.backService.teleportBack(player, loc.location());
this.noticeService.player(player.getUniqueId(),
t -> t.back().teleportedToLastLocation());
},
() -> this.noticeService.player(player.getUniqueId(),
t -> t.back().lastLocationNoExist())
);
}
private void teleportOther(Viewer viewer, Player target, Function<Pair<BackLocation, BackLocation>, BackLocation> selector) {
resolveBack(target.getUniqueId(), selector).ifPresentOrElse(
loc -> {
this.backService.teleportBack(target, loc.location());
this.noticeService.player(target.getUniqueId(),
t -> t.back().teleportedToLastLocation());
this.noticeService.create()
.viewer(viewer)
.notice(t -> t.back().teleportedSpecifiedPlayerLastLocation())
.placeholder(PLAYER_PLACEHOLDER, target.getName())
.send();
},
() -> this.noticeService.viewer(viewer, t -> t.back().lastLocationNoExist())
);
}
private Optional<BackLocation> resolveBack(UUID playerId, Function<Pair<BackLocation, BackLocation>, BackLocation> selector) {
return this.backService.getBackLocationPair(playerId)
.map(selector)
.filter(loc -> loc != null);
}
private BackLocation pickDeath(Pair<BackLocation, BackLocation> pair) {
return pair.getFirst();
}
private BackLocation pickTeleport(Pair<BackLocation, BackLocation> pair) {
return pair.getSecond();
}
}


private final BackService backService;
private final NoticeService noticeService;

@Inject
public BackCommand(BackService backService, NoticeService noticeService) {
this.backService = backService;
this.noticeService = noticeService;
}

@Execute(name = "death")
@Permission("eternalcore.back.death")
@DescriptionDocs(description = "Teleport to your last death location")
public void executeBackDeath(@Sender Player player) {
Optional<Pair<BackLocation, BackLocation>> backPair = this.backService.getBackLocationPair(player.getUniqueId());

if (backPair.isEmpty() || backPair.get().getFirst() == null) {
this.noticeService.player(player.getUniqueId(), translation -> translation.back().lastLocationNoExist());
return;
}

BackLocation deathLocation = backPair.get().getFirst();
this.backService.teleportBack(player, deathLocation.location());
this.noticeService.player(player.getUniqueId(), translation -> translation.back().teleportedToLastLocation());
}
Comment on lines +32 to +43
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This method's logic can be simplified by using Optional's functional style methods, making the code more concise and readable. This pattern can also be applied to executeBackTeleport, and both could be extracted into a single helper method to reduce code duplication.

    public void executeBackDeath(@Sender Player player) {
        this.backService.getBackLocationPair(player.getUniqueId())
            .map(Pair::getFirst)
            .filter(java.util.Objects::nonNull)
            .ifPresentOrElse(deathLocation -> {
                this.backService.teleportBack(player, deathLocation.location());
                this.noticeService.player(player.getUniqueId(), translation -> translation.back().teleportedToLastLocation());
            }, () -> this.noticeService.player(player.getUniqueId(), translation -> translation.back().lastLocationNoExist()));
    }

Copy link
Member Author

Choose a reason for hiding this comment

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

doesn't look simplified at all


@Execute(name = "teleport")
@Permission("eternalcore.back.teleport")
@DescriptionDocs(description = "Teleport to your last teleport location")
public void executeBackTeleport(@Sender Player player) {
Optional<Pair<BackLocation, BackLocation>> backPair = this.backService.getBackLocationPair(player.getUniqueId());

if (backPair.isEmpty() || backPair.get().getSecond() == null) {
this.noticeService.player(player.getUniqueId(), translation -> translation.back().lastLocationNoExist());
return;
}

BackLocation teleportLocation = backPair.get().getSecond();
this.backService.teleportBack(player, teleportLocation.location());
this.noticeService.player(player.getUniqueId(), translation -> translation.back().teleportedToLastLocation());
}

@Execute(name = "death")
@Permission("eternalcore.back.death.other")
@DescriptionDocs(description = "Teleport specified player to their last death location", arguments = "<player>")
public void executeBackDeathOther(@Sender Viewer viewer, @Arg Player target) {
Optional<Pair<BackLocation, BackLocation>> backPair = this.backService.getBackLocationPair(target.getUniqueId());

if (backPair.isEmpty() || backPair.get().getFirst() == null) {
this.noticeService.viewer(viewer, translation -> translation.back().lastLocationNoExist());
return;
}

BackLocation deathLocation = backPair.get().getFirst();
this.backService.teleportBack(target, deathLocation.location());
this.noticeService.player(target.getUniqueId(), translation -> translation.back().teleportedToLastLocation());

this.noticeService.create()
.viewer(viewer)
.notice(translation -> translation.back().teleportedSpecifiedPlayerLastLocation())
.placeholder("{PLAYER}", target.getName())
.send();
}
Comment on lines +64 to +81
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

This method contains logic that is very similar to executeBackTeleportOther and could be refactored to reduce duplication. Additionally, using Optional's functional methods can make the implementation cleaner and more readable.

    public void executeBackDeathOther(@Sender Viewer viewer, @Arg Player target) {
        this.backService.getBackLocationPair(target.getUniqueId())
            .map(Pair::getFirst)
            .filter(java.util.Objects::nonNull)
            .ifPresentOrElse(deathLocation -> {
                this.backService.teleportBack(target, deathLocation.location());
                this.noticeService.player(target.getUniqueId(), translation -> translation.back().teleportedToLastLocation());

                this.noticeService.create()
                    .viewer(viewer)
                    .notice(translation -> translation.back().teleportedSpecifiedPlayerLastLocation())
                    .placeholder("{PLAYER}", target.getName())
                    .send();
            }, () -> this.noticeService.viewer(viewer, translation -> translation.back().lastLocationNoExist()));
    }


@Execute(name = "teleport")
@Permission("eternalcore.back.teleport.other")
@DescriptionDocs(description = "Teleport specified player to their last teleport location", arguments = "<player>")
public void executeBackTeleportOther(@Sender Viewer viewer, @Arg Player target) {
Optional<Pair<BackLocation, BackLocation>> backPair = this.backService.getBackLocationPair(target.getUniqueId());

if (backPair.isEmpty() || backPair.get().getSecond() == null) {
this.noticeService.viewer(viewer, translation -> translation.back().lastLocationNoExist());
return;
}

BackLocation teleportLocation = backPair.get().getSecond();
this.backService.teleportBack(target, teleportLocation.location());
this.noticeService.player(target.getUniqueId(), translation -> translation.back().teleportedToLastLocation());

this.noticeService.create()
.viewer(viewer)
.notice(translation -> translation.back().teleportedSpecifiedPlayerLastLocation())
.placeholder("{PLAYER}", target.getName())
.send();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.eternalcode.core.feature.back;

import com.eternalcode.core.injector.annotations.Inject;
import org.bukkit.entity.Player;
import org.bukkit.event.EventHandler;
import org.bukkit.event.EventPriority;
import org.bukkit.event.Listener;
import org.bukkit.event.entity.PlayerDeathEvent;
import org.bukkit.event.player.PlayerTeleportEvent;

public class BackController implements Listener {

private final BackService backService;

@Inject
public BackController(BackService backService) {
this.backService = backService;
}

@EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
public void onPlayerDeath(PlayerDeathEvent event) {
Player entity = event.getEntity();

this.backService.setBackLocation(entity.getUniqueId(), entity.getLocation(), true);
}

@EventHandler(priority = EventPriority.MONITOR, ignoreCancelled = true)
public void onPlayerTeleport(PlayerTeleportEvent event) {
if (event.getCause() == PlayerTeleportEvent.TeleportCause.PLUGIN) {
return;
}

this.backService.setBackLocation(event.getPlayer().getUniqueId(), event.getFrom(), false);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
package com.eternalcode.core.feature.back;

import com.eternalcode.commons.bukkit.position.PositionAdapter;
import com.eternalcode.core.feature.teleport.TeleportService;
import com.eternalcode.core.feature.teleport.TeleportTaskService;
import com.eternalcode.core.feature.teleportrequest.TeleportRequestSettings;
import com.eternalcode.core.injector.annotations.Inject;
import com.eternalcode.core.injector.annotations.component.Service;
import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import org.bukkit.Location;
import org.bukkit.entity.Player;
import panda.std.Pair;

@Service
public class BackService {

private final TeleportService teleportService;
private final TeleportTaskService teleportTaskService;
private final TeleportRequestSettings settings;

private final Cache<UUID, Pair<BackLocation, BackLocation>> backLocationsCache;

@Inject
public BackService(
TeleportService teleportService,
TeleportTaskService teleportTaskService,
TeleportRequestSettings settings
) {
this.teleportService = teleportService;
this.teleportTaskService = teleportTaskService;
this.settings = settings;

this.backLocationsCache = Caffeine.newBuilder()
.maximumSize(1000)
.expireAfterWrite(2, TimeUnit.HOURS)
.build();
}

public Optional<Pair<BackLocation, BackLocation>> getBackLocationPair(UUID playerId) {
return Optional.ofNullable(backLocationsCache.getIfPresent(playerId));
}

public void setBackLocation(UUID playerId, Location location, boolean isFromDeath) {
Pair<BackLocation, BackLocation> existing = backLocationsCache.getIfPresent(playerId);
BackLocation newLocation = new BackLocation(location);
BackLocation deathLocation;
BackLocation normalLocation;
if (existing == null) {
deathLocation = isFromDeath ? newLocation : null;
normalLocation = isFromDeath ? null : newLocation;
}
else {
deathLocation = isFromDeath ? newLocation : existing.getFirst();
normalLocation = isFromDeath ? existing.getSecond() : newLocation;
}
backLocationsCache.put(playerId, Pair.of(deathLocation, normalLocation));
}
Comment on lines +47 to +61
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The logic in this method is a bit complex and can be hard to follow. It can be refactored to be more straightforward and readable, which improves maintainability.

    public void setBackLocation(UUID playerId, Location location, boolean isFromDeath) {
        Pair<BackLocation, BackLocation> pair = Optional.ofNullable(this.backLocationsCache.getIfPresent(playerId))
            .orElse(Pair.of(null, null));

        BackLocation newLocation = new BackLocation(location);

        if (isFromDeath) {
            this.backLocationsCache.put(playerId, Pair.of(newLocation, pair.getSecond()));
        }
        else {
            this.backLocationsCache.put(playerId, Pair.of(pair.getFirst(), newLocation));
        }
    }

Comment on lines +47 to +61
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
public void setBackLocation(UUID playerId, Location location, boolean isFromDeath) {
Pair<BackLocation, BackLocation> existing = backLocationsCache.getIfPresent(playerId);
BackLocation newLocation = new BackLocation(location);
BackLocation deathLocation;
BackLocation normalLocation;
if (existing == null) {
deathLocation = isFromDeath ? newLocation : null;
normalLocation = isFromDeath ? null : newLocation;
}
else {
deathLocation = isFromDeath ? newLocation : existing.getFirst();
normalLocation = isFromDeath ? existing.getSecond() : newLocation;
}
backLocationsCache.put(playerId, Pair.of(deathLocation, normalLocation));
}
public void setBackLocation(UUID playerId, Location location, boolean fromDeath) {
Pair<BackLocation, BackLocation> cached = backLocationsCache.getIfPresent(playerId);
BackLocation newLoc = new BackLocation(location);
BackLocation deathLoc = (cached != null) ? cached.getFirst() : null;
BackLocation normalLoc = (cached != null) ? cached.getSecond() : null;
if (fromDeath) {
deathLoc = newLoc;
} else {
normalLoc = newLoc;
}
backLocationsCache.put(playerId, Pair.of(deathLoc, normalLoc));
}


public void teleportBack(Player player, Location location) {
if (player.hasPermission("eternalcore.teleport.bypass")) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move to private static variable

teleportService.teleport(player, location);
}
else {
teleportTaskService.createTeleport(
player.getUniqueId(),
PositionAdapter.convert(player.getLocation()),
PositionAdapter.convert(location),
settings.tpaTimer()
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

The teleport delay for the /back command uses settings.tpaTimer(), which is a setting from the teleport request (/tpa) feature. It would be better to introduce a dedicated configuration setting for the /back command's teleport delay, for example backTimer. This would give server administrators more granular control over teleport delays for different features.

);
}
}

public record BackLocation(Location location) {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package com.eternalcode.core.feature.back.messages;

import com.eternalcode.multification.notice.Notice;

public interface BackMessages {

Notice lastLocationNoExist();

Notice teleportedToLastLocation();

Notice teleportedSpecifiedPlayerLastLocation();

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.eternalcode.core.feature.back.messages;

import com.eternalcode.multification.notice.Notice;
import eu.okaeri.configs.OkaeriConfig;
import lombok.Getter;
import lombok.experimental.Accessors;

@Getter
@Accessors(fluent = true)
public class ENBackMessages extends OkaeriConfig implements BackMessages {

public Notice lastLocationNoExist = Notice.chat("<red>► <white>You don't have any last location to teleport to!");
public Notice teleportedToLastLocation = Notice.chat("<green>► <white>You have been teleported to your last location!");
public Notice teleportedSpecifiedPlayerLastLocation = Notice.chat("<green>► <white>Player <green>{PLAYER} <white>has been teleported to their last location!");

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package com.eternalcode.core.feature.back.messages;

import com.eternalcode.multification.notice.Notice;
import eu.okaeri.configs.OkaeriConfig;
import lombok.Getter;
import lombok.experimental.Accessors;

@Getter
@Accessors(fluent = true)
public class PLBackMessages extends OkaeriConfig implements BackMessages {

public Notice lastLocationNoExist = Notice.chat("<red>► <white>Nie masz żadnej ostatniej lokalizacji, do której można się teleportować!");
public Notice teleportedToLastLocation = Notice.chat("<green>► <white>Zostałeś przeteleportowany do ostatniej lokalizacji!");
public Notice teleportedSpecifiedPlayerLastLocation = Notice.chat("<green>► <white>Gracz <green>{PLAYER} <white>został przeteleportowany do swojej ostatniej lokalizacji!");

}

This file was deleted.

Loading