GH-926 Add teleport timer to TeleportBackCommand, add teleport-here functionality. #926
Conversation
Jakubk15
changed the title
WalkthroughThis update adds a new "teleport here" feature, allowing players to request others to teleport to their location. It introduces new classes and messages to support this, including commands for sending, accepting, and denying these requests. The teleport back command now includes a warmup delay, with a bypass option for certain permissions. Several new messages and interface methods are added to handle notifications for these features. There are also minor build updates, such as a new LuckPerms version constant and plugin dependency. Assessment against linked issues
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TeleportHereRequestService.java (1)
35-45: Consider simplifying the hasRequest methodThe implementation works, but could be simplified by directly using the Cache.getIfPresent method instead of iterating through all entries.
boolean hasRequest(UUID requester, UUID target) { - Map<UUID, UUID> map = this.requests.asMap(); - - for (Map.Entry<UUID, UUID> entry : map.entrySet()) { - if (entry.getKey().equals(requester) && entry.getValue().equals(target)) { - return true; - } - } - - return false; + UUID storedTarget = this.requests.getIfPresent(requester); + return storedTarget != null && storedTarget.equals(target); }eternalcore-core/src/main/java/com/eternalcode/core/feature/teleport/command/TeleportBackCommand.java (2)
50-55: Consider updating the success message timing.The teleport success message is sent immediately even when using the delayed teleport. This might confuse players who haven't teleported yet.
if (player.hasPermission(TIMER_BYPASS_PERMISSION)) { this.teleportService.teleport(player, location.get()); + this.noticeService.player(player.getUniqueId(), translation -> translation.teleport().teleportedToLastLocation()); } else { this.teleportTaskService.createTeleport(player.getUniqueId(), PositionAdapter.convert(player.getLocation()), PositionAdapter.convert(location.get()), this.settings.teleportTime()); } - this.noticeService.player(player.getUniqueId(), translation -> translation.teleport().teleportedToLastLocation());
71-76: Same message timing issue in second method.The success message should be sent after immediate teleport but not for delayed teleports.
if (player.hasPermission(TIMER_BYPASS_PERMISSION)){ this.teleportService.teleport(player, location.get()); + this.noticeService.player(player.getUniqueId(), translation -> translation.teleport().teleportedToLastLocation()); + + this.noticeService.create() + .viewer(viewer) + .notice(translation -> translation.teleport().teleportedSpecifiedPlayerLastLocation()) + .placeholder("{PLAYER}", player.getName()) + .send(); } else { this.teleportTaskService.createTeleport(player.getUniqueId(), PositionAdapter.convert(player.getLocation()), PositionAdapter.convert(location.get()), this.settings.teleportTime()); } - this.noticeService.player(player.getUniqueId(), translation -> translation.teleport().teleportedToLastLocation()); - - this.noticeService.create() - .viewer(viewer) - .notice(translation -> translation.teleport().teleportedSpecifiedPlayerLastLocation()) - .placeholder("{PLAYER}", player.getName()) - .send();eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereCommand.java (2)
51-68: Add error handling for async operations.The CompletableFuture might throw exceptions that aren't being handled.
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(target.getUniqueId()) .notice(translation -> translation.tpa().tpaHereReceivedMessage()) .placeholder("{PLAYER}", sender.getName()) .send(); this.requestService.createRequest(sender.getUniqueId(), target.getUniqueId()); - }); + }).exceptionally(ex -> { + this.noticeService.player(sender.getUniqueId(), translation -> translation.general().error()); + ex.printStackTrace(); + return null; + });
70-72: Consider making method private.The
isIgnoringmethod is only used within this class, so it could be made private.- CompletableFuture<Boolean> isIgnoring(Player target, Player sender) { + private CompletableFuture<Boolean> isIgnoring(Player target, Player sender) { return this.ignoreService.isIgnored(target.getUniqueId(), sender.getUniqueId()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
buildSrc/src/main/kotlin/Versions.kt(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleport/command/TeleportBackCommand.java(4 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/messages/ENTeleportRequestMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/messages/PLTeleportRequestMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/messages/TeleportRequestMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/SelfRequesterArgument.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TeleportHereRequestService.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereActionCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereCommand.java(1 hunks)eternalcore-plugin/build.gradle.kts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build (17)
🔇 Additional comments (15)
buildSrc/src/main/kotlin/Versions.kt (1)
38-38: Good addition of LuckPerms version constant.This addition follows the project's pattern for dependency versioning. It will help maintain consistent version references across the project.
eternalcore-plugin/build.gradle.kts (1)
42-42: Good implementation of LuckPerms plugin download.This correctly references the version constant you added and ensures the server has the right LuckPerms version during development.
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/messages/TeleportRequestMessages.java (1)
12-14: Interface methods added properly for teleport-here feature.These new methods nicely complement the existing teleport request messages and support the new teleport-here functionality mentioned in the PR.
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/messages/PLTeleportRequestMessages.java (1)
19-26: Polish message implementations look good.The implementation follows the existing pattern with clear messages. The interactive elements with hover and click actions will make for a user-friendly experience.
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/messages/ENTeleportRequestMessages.java (1)
19-26: Nice addition of teleport-here messages!The new messages look good and match the style of existing messages. The interactive elements with hover text and click actions will help players easily respond to teleport requests.
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TeleportHereRequestService.java (1)
14-25: Good implementation of request service with cachingThe service implementation looks clean and uses Guava cache with expiration based on settings. This is a good approach for managing temporary teleport requests.
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereActionCommand.java (2)
37-61: Accept method looks goodThe implementation handles the teleport request acceptance properly with appropriate notifications to both players.
83-112: Good implementation of "deny all" functionalityThe method correctly handles the case when no requests exist and properly notifies all involved players.
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/SelfRequesterArgument.java (2)
35-48: Parse method looks solidThe method handles all necessary checks for the teleport requester argument, including validation that the sender is a player and the request exists.
50-61: Nice implementation of suggestions featureThe suggest method provides helpful auto-complete functionality by suggesting only players who have sent teleport requests to the current player.
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleport/command/TeleportBackCommand.java (3)
24-24: Good addition of permission constant.Clear naming for the bypass permission - makes the code more readable and easier to maintain.
26-27: New fields properly defined.The teleport task service and settings are correctly added as fields.
31-35: Constructor updated correctly.The constructor properly initializes the new dependencies.
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereCommand.java (2)
16-28: Well-structured command class.Good implementation with proper dependency injection.
31-43: Good validation checks.The self-teleport and duplicate request checks are well-implemented.
...re-core/src/main/java/com/eternalcode/core/feature/teleport/command/TeleportBackCommand.java
Outdated
Show resolved
Hide resolved
...in/java/com/eternalcode/core/feature/teleportrequest/messages/PLTeleportRequestMessages.java
Outdated
Show resolved
Hide resolved
...in/java/com/eternalcode/core/feature/teleportrequest/messages/ENTeleportRequestMessages.java
Show resolved
Hide resolved
...re/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereActionCommand.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereCommand.java (1)
53-69: Asynchronous ignore check is neat.
The CompletableFuture approach is clean. Just remember to handle cases if a player disconnects mid-check.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleport/command/TeleportBackCommand.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/TeleportRequestPermissionConstant.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereActionCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereCommand.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/TeleportRequestPermissionConstant.java
🚧 Files skipped from review as they are similar to previous changes (2)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereActionCommand.java
- eternalcore-core/src/main/java/com/eternalcode/core/feature/teleport/command/TeleportBackCommand.java
🔇 Additional comments (4)
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereCommand.java (4)
17-19: Command setup looks clean.
Everything here is straightforward. Nice work keeping the command name and permission aligned with the service.
34-38: Friendly self-check.
This is a helpful way to prevent self-request confusion. The message to the sender is concise.
40-44: Good repeated-request check.
This avoids accidental spam or duplicates. Straightforward flow.
72-74: Utility method isIgnoring.
Keeps the logic simple and readable. Clear separation of concerns.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/TeleportRequestPermissionConstant.java (1)
3-14: Looks good, though consider making this a utility classThe permission constants are well-organized and clearly named. Since this is a constants-only class, consider making it final and adding a private constructor to prevent instantiation.
-public class TeleportRequestPermissionConstant { +public final class TeleportRequestPermissionConstant { + + private TeleportRequestPermissionConstant() { + // Utility class + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/TeleportRequestPermissionConstant.java(1 hunks)
🔇 Additional comments (2)
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/TeleportRequestPermissionConstant.java (2)
5-10: Nice alignment of permission stringsThe aligned format makes these constants easy to read and maintain.
12-12: Good separation of bypass permissionThe empty lines before and after this constant help distinguish it as a different category from the request-related permissions above.
...ore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereCommand.java
Outdated
Show resolved
Hide resolved
...ore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereCommand.java
Outdated
Show resolved
Hide resolved
...ain/java/com/eternalcode/core/feature/teleportrequest/TeleportRequestPermissionConstant.java
Outdated
Show resolved
Hide resolved
...re/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereActionCommand.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereActionCommand.java (1)
65-84: Implementation looks goodThe deny logic correctly removes the request and notifies both players appropriately. Permissions are properly configured.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleport/command/TeleportBackCommand.java(4 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereActionCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereCommand.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/teleport/command/TeleportBackCommand.java
🔇 Additional comments (3)
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereCommand.java (2)
30-68: Well implemented teleport here request flow.The logic correctly handles self-teleport prevention, duplicate detection, and async ignore checking. The flow of sending confirmation first, then checking ignore status is user-friendly.
70-72: Clean helper methodSimple and effective delegation to the ignore service. Good encapsulation.
eternalcore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereActionCommand.java (1)
86-112: Well-implemented bulk deny functionalityGood handling of edge cases including empty request lists and offline requesters. The logic flow is clear and robust.
...re/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereActionCommand.java
Show resolved
Hide resolved
...ore-core/src/main/java/com/eternalcode/core/feature/teleportrequest/self/TpaHereCommand.java
Outdated
Show resolved
Hide resolved
…leportrequest/self/TpaHereCommand.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Resolves #904
Film.bez.tytulu.mp4
Film.bez.tytulu.1.mp4