GH-110 Add parcel receiving logic #118
Conversation
Jakubk15
added
🆕 feature
WalkthroughThis update focuses on standardizing naming conventions across the codebase, particularly for GUI-related classes, which have been renamed to use camel case (e.g., changing "GUI" to "Gui"). The configuration files have been updated to introduce new fields that support parcel collection, while logic related to recipients has been removed from the parcel data structure and associated methods. The changes also include improvements in code formatting, a transition from abstract classes to interfaces in some instances, and enhancements to exception handling. New methods have been added to facilitate paginated retrieval of parcels, and corresponding test cases have been updated to reflect these modifications. Overall, the changes maintain existing functionality while enhancing consistency and clarity throughout the code. ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Jakubk15
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReceiverSelectionGui.java (1)
61-64: Would be helpful to add a brief class description.Consider adding a simple comment block at the class level to explain what this GUI does and how it fits into the parcel receiving flow.
+/** + * GUI for selecting a parcel receiver from a paginated list of users. + * Shows player heads with selection state and handles receiver updates. + */ public class ReceiverSelectionGui implements GuiView {src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java (1)
107-112: Consider using toList() for consistencyThe findPage method uses collect(Collectors.toList()) while the new findByReceiver uses toList(). Let's keep it consistent.
- .stream().map(ParcelWrapper::toParcel) - .collect(Collectors.toList()); + .stream() + .map(ParcelWrapper::toParcel) + .toList();src/main/java/com/eternalcode/parcellockers/gui/GuiView.java (1)
9-10: Consider if final is still needed.
These arrays could remainfinalif they're meant to be constants.src/main/java/com/eternalcode/parcellockers/parcel/util/ParcelPlaceholderUtil.java (1)
10-10: Wildcard imports might be overkill.
You can keep them if you like brevity, but explicit imports can be clearer.src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerMainGui.java (1)
80-91: Consider using a builder pattern for ParcelSendingGui creationThe current instantiation has many dependencies. A builder pattern would make this more maintainable.
- gui.setItem(22, this.config.guiSettings.parcelLockerSendItem.toGuiItem(event -> new ParcelSendingGui(this.plugin, - this.config, - this.miniMessage, - this.itemStorageRepository, - this.parcelRepository, - this.lockerRepository, - this.announcer, - this.parcelContentRepository, - this.userRepository, - this.skullAPI, - new ParcelSendingGuiState() - ).show(player))); + gui.setItem(22, this.config.guiSettings.parcelLockerSendItem.toGuiItem(event -> + ParcelSendingGui.builder() + .plugin(this.plugin) + .config(this.config) + .miniMessage(this.miniMessage) + .itemStorageRepository(this.itemStorageRepository) + .parcelRepository(this.parcelRepository) + .lockerRepository(this.lockerRepository) + .announcer(this.announcer) + .parcelContentRepository(this.parcelContentRepository) + .userRepository(this.userRepository) + .skullAPI(this.skullAPI) + .state(new ParcelSendingGuiState()) + .build() + .show(player)));src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/DestinationSelectionGui.java (1)
89-90: Make timeout configurable and add retry mechanismThe 5-second timeout is hardcoded and might be insufficient. Consider making it configurable and adding retries.
}).whenComplete(ExceptionHandler.handler()) - .orTimeout(5, TimeUnit.SECONDS); + .orTimeout(config.guiSettings.destinationSelectionTimeout, TimeUnit.SECONDS) + .exceptionally(throwable -> { + if (throwable instanceof TimeoutException) { + this.show(player, page); // Retry once + } + return null; + });src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelSendingGui.java (1)
211-213: Consider using a builder pattern for Parcel creation.The direct constructor call with many parameters could be error-prone. A builder pattern would make the code more maintainable.
-Parcel parcel = new Parcel(UUID.randomUUID(), player.getUniqueId(), this.state.getParcelName(), - this.state.getParcelDescription(), this.state.isPriority(), this.state.getReceiver(), - this.state.getSize(), this.state.getEntryLocker(), this.state.getDestinationLocker()); +Parcel parcel = Parcel.builder() + .uuid(UUID.randomUUID()) + .sender(player.getUniqueId()) + .name(this.state.getParcelName()) + .description(this.state.getParcelDescription()) + .priority(this.state.isPriority()) + .receiver(this.state.getReceiver()) + .size(this.state.getSize()) + .entryLocker(this.state.getEntryLocker()) + .destinationLocker(this.state.getDestinationLocker()) + .build();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
src/main/java/com/eternalcode/parcellockers/ParcelLockers.java(3 hunks)src/main/java/com/eternalcode/parcellockers/configuration/implementation/PluginConfiguration.java(4 hunks)src/main/java/com/eternalcode/parcellockers/database/persister/ItemStackPersister.java(3 hunks)src/main/java/com/eternalcode/parcellockers/gui/GuiView.java(1 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/DestinationSelectionGui.java(3 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerMainGui.java(4 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java(1 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelItemStorageGui.java(4 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelSendingGui.java(8 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelSendingGuiState.java(2 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReceiverSelectionGui.java(2 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/MainGui.java(3 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/ParcelListGui.java(2 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/SentParcelsGui.java(3 hunks)src/main/java/com/eternalcode/parcellockers/locker/controller/LockerInteractionController.java(2 hunks)src/main/java/com/eternalcode/parcellockers/parcel/Parcel.java(0 hunks)src/main/java/com/eternalcode/parcellockers/parcel/command/ParcelCommand.java(2 hunks)src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java(1 hunks)src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java(2 hunks)src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelWrapper.java(2 hunks)src/main/java/com/eternalcode/parcellockers/parcel/util/ParcelPlaceholderUtil.java(2 hunks)src/test/java/com/eternalcode/parcellockers/database/ParcelDatabaseServiceIntegrationTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/eternalcode/parcellockers/parcel/Parcel.java
✅ Files skipped from review due to trivial changes (7)
- src/main/java/com/eternalcode/parcellockers/database/persister/ItemStackPersister.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelSendingGuiState.java
- src/main/java/com/eternalcode/parcellockers/locker/controller/LockerInteractionController.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelItemStorageGui.java
- src/main/java/com/eternalcode/parcellockers/parcel/command/ParcelCommand.java
- src/main/java/com/eternalcode/parcellockers/ParcelLockers.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/ParcelListGui.java
🧰 Additional context used
📓 Learnings (1)
src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelWrapper.java (1)
Learnt from: Jakubk15
PR: EternalCodeTeam/ParcelLockers#116
File: src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelWrapper.java:59-61
Timestamp: 2025-01-15T20:13:38.696Z
Learning: The ParcelWrapper class currently only stores a single sender, while the Parcel class supports multiple allowed senders. This limitation will be addressed in a future PR.
🪛 GitHub Actions: Java CI with Gradle
src/test/java/com/eternalcode/parcellockers/database/ParcelDatabaseServiceIntegrationTest.java
[error] 49-49: cannot find symbol: method builder() in class Parcel
🔇 Additional comments (17)
src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReceiverSelectionGui.java (1)
29-29: Nice work on the class structure updates!The switch to implementing
GuiViewand the consistent naming of GUI-related types make the code cleaner and easier to follow.Also applies to: 40-42, 50-50
src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java (1)
23-23: Nice addition of paginated receiver lookup!The new method aligns well with the existing pagination pattern in the interface.
src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java (1)
68-86: Implementation looks good!The pagination logic matches the pattern used in findPage method.
src/test/java/com/eternalcode/parcellockers/database/ParcelDatabaseServiceIntegrationTest.java (2)
68-68: Good use of getFirst()!Using getFirst() is cleaner than iterator().next().
Also applies to: 72-72, 76-76
49-60: Fix the builder() method errorThe pipeline is failing because it can't find the builder() method in Parcel class.
Let's check if the Parcel class has Lombok annotations:
🧰 Tools
🪛 GitHub Actions: Java CI with Gradle
[error] 49-49: cannot find symbol: method builder() in class Parcel
src/main/java/com/eternalcode/parcellockers/gui/GuiView.java (2)
7-7: Interfaces simplify usage.
Nice decision to switch from an abstract class to an interface here.
12-12: Nice method signature adjustment.
This is a clean fit for an interface.src/main/java/com/eternalcode/parcellockers/parcel/util/ParcelPlaceholderUtil.java (1)
31-31: Good placeholder registration.
Straightforward and neat addition for parcel descriptions.src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/MainGui.java (3)
18-18: Class name aligns with naming style.
This clearly follows the camel case convention.
28-28: Constructor name matches the class.
Thanks for making it consistent.
75-76: Renamed GUI classes look consistent.
Neat switch from the older naming to the new style.src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelSendingGui.java (1)
321-328: Nice improvement in null handling!The code now properly handles null or empty receiver names.
src/main/java/com/eternalcode/parcellockers/configuration/implementation/PluginConfiguration.java (2)
383-394: Good configuration for parcel collection item.The configuration provides clear and relevant information for parcel collection.
395-400: Nice user feedback for empty state.The no-parcels item provides clear feedback when there are no parcels to collect.
src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelWrapper.java (3)
40-41: LGTM! Default constructor added for ORM.The empty constructor is needed for the database mapping to work properly.
43-53: LGTM! Constructor looks good.All fields are properly initialized in a clean, organized way.
59-61: Verify the sender handling change.The code now passes the sender directly instead of wrapping it in a Set. Make sure this change aligns with the Parcel class's new structure.
✅ Verification successful
🏁 Script executed:
#!/bin/bash # Check if Parcel constructor matches this usage ast-grep --pattern 'class Parcel { $$$ Parcel($_, UUID sender, $$$) { $$$ } $$$ }'Length of output: 96
I'll run an additional script to inspect the Parcel constructor in detail and confirm that the sender is now handled as a UUID rather than a Set. Please hold on as we gather the necessary details.
🏁 Script executed:
#!/bin/bash # List the lines containing the Parcel constructor in Parcel.java to inspect the sender parameter type rg -n "public Parcel\(" src/main/java/com/eternalcode/parcellockers/parcel/Parcel.javaLength of output: 88
I'll run another script to search for the Parcel constructor without assuming it’s public (it might be package-private). Please hold on as we gather more details.
🏁 Script executed:
#!/bin/bash # Search for the Parcel constructor in Parcel.java regardless of its access modifier rg -n "^[[:space:]]*Parcel\(" src/main/java/com/eternalcode/parcellockers/parcel/Parcel.javaLength of output: 94
I'll run another script that searches the whole repository for classes named
Parceland then prints any lines containing"UUID sender". This should help us confirm that the Parcel constructor is now expecting a direct UUID sender parameter.
🏁 Script executed:
#!/bin/bash # Find all Java files containing a class named Parcel and then search for "UUID sender" within them. rg -l "class Parcel" -t java | while read file; do echo "Inspecting $file" rg -n "UUID sender" "$file" doneLength of output: 8092
Let's run a new script to verify whether the Parcel class's constructor now directly accepts a UUID sender. We'll search for the file
Parcel.javaand then print its constructor signature. Please hold on.
🏁 Script executed:
#!/bin/bash # Locate Parcel.java in the repository fd -e java "Parcel.java" src/main/java/com/eternalcode/parcellockers # Display the first 150 lines of the Parcel.java file to inspect the constructor signature head -n 150 $(fd -e java "Parcel.java" src/main/java/com/eternalcode/parcellockers | head -n 1)Length of output: 705
Sender Type Verification Complete
The Parcel record now expects a UUID sender, and the ParcelWrapper passes the sender directly. Everything aligns correctly.
src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/SentParcelsGui.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/SentParcelsGui.java (1)
108-108: Add a null check for descriptionConsider handling null descriptions to prevent potential issues:
- .register("{DESCRIPTION}", parcel.description()); + .register("{DESCRIPTION}", parcel.description() != null ? parcel.description() : "-");Also applies to: 108-108
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
build.gradle.kts(5 hunks)gradle/wrapper/gradle-wrapper.properties(1 hunks)src/main/java/com/eternalcode/parcellockers/ParcelLockers.java(3 hunks)src/main/java/com/eternalcode/parcellockers/configuration/implementation/PluginConfiguration.java(5 hunks)src/main/java/com/eternalcode/parcellockers/database/wrapper/AbstractRepositoryOrmLite.java(1 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/DestinationSelectionGui.java(4 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerMainGui.java(5 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java(1 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelItemStorageGui.java(8 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelSendingGui.java(10 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReceiverSelectionGui.java(4 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/ParcelListGui.java(4 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/SentParcelsGui.java(5 hunks)src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java(1 hunks)src/main/java/com/eternalcode/parcellockers/shared/SentryExceptionHandler.java(1 hunks)src/main/java/com/eternalcode/parcellockers/updater/UpdaterNotificationController.java(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- gradle/wrapper/gradle-wrapper.properties
🚧 Files skipped from review as they are similar to previous changes (6)
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/DestinationSelectionGui.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelItemStorageGui.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/ParcelListGui.java
- src/main/java/com/eternalcode/parcellockers/ParcelLockers.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerMainGui.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelSendingGui.java
🔇 Additional comments (22)
src/main/java/com/eternalcode/parcellockers/updater/UpdaterNotificationController.java (2)
4-4: Nice update to the import!The switch to SentryExceptionHandler aligns with the project's standardization efforts.
46-46: Good improvement in error handling!The change to SentryExceptionHandler will help track and monitor errors better.
src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/SentParcelsGui.java (4)
10-10: Nice work on the naming updates!The class name now follows Java's standard naming conventions, and the switch to Sentry for error handling is a good choice.
Also applies to: 27-27
35-35: Looking good!The field and constructor parameter names are now consistent with the new naming style.
Also applies to: 38-38
89-89: Good switch to Sentry for error handling!Also applies to: 89-89
98-99: Move name resolution to async operationThe getName operations are blocking and could cause GUI lag. Consider resolving names asynchronously.
build.gradle.kts (7)
7-7: New Blossom Plugin Added.
The new plugin "net.kyori.blossom" version "2.1.0" is added to enhance text replacement tasks during the build. It looks good – just double-check that it aligns with your build system.
107-107: Checkstyle Configuration Updated.
The Checkstyle config file path has been changed to "${rootDir}/gradle/checkstyle/checkstyle.xml", which streamlines the project structure nicely.
119-121: Bukkit Metadata Updated.
The plugin description and version ("0.0.1-SNAPSHOT") have been updated for clarity and consistency. Ensure these values match your current release strategy.
131-137: SourceSets for Blossom Configured.
The new sourceSets.main block for blossom.javaSources neatly injects project properties like the name, version, and developer mode. This is a clear and effective approach.
156-157: ShadowJar Exclusions Updated.
Excluding "javassist/" and "org/h2/util/" helps prevent unwanted classes from being bundled. Just verify that these exclusions don’t eliminate any needed files.
165-185: Improved ShadowJar Relocation Logic.
The relocation logic now dynamically prefixes a wider set of packages with "com.eternalcode.parcellockers.libs", which enhances modularity and reduces potential conflicts. Make sure the relocated packages work as expected at runtime.
188-190: Minor Formatting Adjustments.
The slight formatting changes toward the end of the file are non-impactful and help keep the file tidy.src/main/java/com/eternalcode/parcellockers/shared/SentryExceptionHandler.java (1)
7-17: Neat rename and usage.Renaming the handler adds clarity to its purpose. Looks good to me.
src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java (1)
79-83: Add a null check.Please handle a potential null result before checking parcels. This was mentioned earlier as well.
src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReceiverSelectionGui.java (4)
7-7: LGTM! Good improvements to consistency.The changes to use
implementsinstead ofextendsand the consistent GUI naming look good.Also applies to: 29-29
40-42: LGTM! Consistent naming.Field names now follow the consistent naming pattern.
50-50: LGTM! Constructor parameters updated correctly.Parameter types match the renamed field types.
105-106: LGTM! Better error handling.Good update to use SentryExceptionHandler for consistent error tracking.
src/main/java/com/eternalcode/parcellockers/configuration/implementation/PluginConfiguration.java (3)
112-114: LGTM! Clear user feedback messages.The new messages provide good feedback for parcel collection actions.
125-125: LGTM! Simplified parcel info.The parcel info message has been correctly updated to remove recipient information.
147-402: LGTM! Well-structured GUI configurations.The new GUI elements for parcel collection maintain a consistent style and provide clear feedback.
src/main/java/com/eternalcode/parcellockers/database/wrapper/AbstractRepositoryOrmLite.java
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java
Outdated
Show resolved
Hide resolved
…/locker/ParcelCollectionGui.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
src/main/java/com/eternalcode/parcellockers/util/InventoryUtil.java (1)
9-18: Fix the item dropping logicThere's a bug in the code where items might be dropped even when there are empty slots available. The drop logic is also inside the loop, which could cause multiple drops.
Here's a simpler way to handle this:
public static void addItem(Player player, ItemStack itemStack) { Inventory inventory = player.getInventory(); for (ItemStack item : inventory.getStorageContents()) { if (item == null) { inventory.addItem(itemStack); return; } - player.getWorld().dropItem(player.getLocation(), itemStack); } + player.getWorld().dropItem(player.getLocation(), itemStack); }
🧹 Nitpick comments (8)
src/main/java/com/eternalcode/parcellockers/util/InventoryUtil.java (1)
20-28: Add input validation and documentationThe method works well but could be more robust with a few small changes.
Here's a suggestion to make it even better:
+ /** + * Counts the number of empty slots in a player's inventory. + * @param player the player whose inventory to check + * @return the number of empty slots + * @throws IllegalArgumentException if player is null + */ public static int freeSlotsInInventory(Player player) { + if (player == null) { + throw new IllegalArgumentException("Player cannot be null"); + } int freeSlots = 0; for (ItemStack item : player.getInventory().getStorageContents()) { if (item == null) { freeSlots++; } } return freeSlots; }src/main/java/com/eternalcode/parcellockers/parcel/util/ParcelPlaceholderUtil.java (2)
10-10: Let's be more specific with our imports! 🎯Instead of using
java.util.*, it would be clearer to list out exactly which classes we need (like ArrayList, List, Optional, etc.).-import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Optional; +import java.util.UUID;
21-22: Let's catch those potential errors! 🎯The
join()calls could throw exceptions if something goes wrong. We should wrap them in try-catch blocks to handle errors gracefully.- String senderName = getName(parcel.sender(), userManager).join(); - String receiver = getName(parcel.receiver(), userManager).join(); + String senderName = "Unknown"; + String receiver = "Unknown"; + try { + senderName = getName(parcel.sender(), userManager).join(); + receiver = getName(parcel.receiver(), userManager).join(); + } catch (Exception e) { + // Log the error but continue with default values + }src/main/java/com/eternalcode/parcellockers/itemstorage/repository/ItemStorageRepository.java (1)
16-17: Looks good! Consider adding documentation.The new
removeAll()method matches the pattern used across other repositories.Consider adding a Javadoc comment to explain that the returned Integer represents the number of items removed.
src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java (1)
21-23: Consider deprecating the non-paginated findByReceiver.Having both paginated and non-paginated versions of
findByReceivermight be confusing. Consider marking the non-paginated version as deprecated if pagination is the preferred approach.src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java (2)
31-40: Simplify the async handlingThe empty thenAccept blocks can be removed to make the code cleaner.
Here's a simpler version for all methods:
- this.parcelRepository.removeAll().thenAccept(v -> { - }).whenComplete((v, throwable) -> { + this.parcelRepository.removeAll().whenComplete((v, throwable) -> {Also applies to: 42-52, 54-64, 66-76
34-36: Include error details in messagesIt would be helpful to show what went wrong when operations fail.
Update the error messages:
- this.announcer.sendMessage(player, "&cFailed to delete parcels"); + this.announcer.sendMessage(player, "&cFailed to delete parcels: " + throwable.getMessage());Also applies to: 46-48, 58-60, 70-72
src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java (1)
68-86: Consider adding sorting to paginated resultsThe query results should be ordered consistently for reliable pagination. Consider adding an ORDER BY clause to ensure consistent results across pages.
List<Parcel> parcels = dao.queryBuilder() .limit((long) page.getLimit() + 1) .offset((long) page.getOffset()) .where() .eq(RECEIVER_COLUMN, receiver) + .orderBy("created_at", true) // or any other appropriate column .query()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
build.gradle.kts(4 hunks)gradle.properties(1 hunks)settings.gradle.kts(1 hunks)src/main/java/com/eternalcode/parcellockers/ParcelLockers.java(3 hunks)src/main/java/com/eternalcode/parcellockers/ParcelLockersCommand.java(1 hunks)src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java(1 hunks)src/main/java/com/eternalcode/parcellockers/content/repository/ParcelContentRepository.java(1 hunks)src/main/java/com/eternalcode/parcellockers/content/repository/ParcelContentRepositoryOrmLite.java(1 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerMainGui.java(7 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java(1 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelItemStorageGui.java(9 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelSendingGui.java(12 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReceiverSelectionGui.java(4 hunks)src/main/java/com/eternalcode/parcellockers/itemstorage/repository/ItemStorageRepository.java(1 hunks)src/main/java/com/eternalcode/parcellockers/itemstorage/repository/ItemStorageRepositoryOrmLite.java(1 hunks)src/main/java/com/eternalcode/parcellockers/locker/repository/LockerRepository.java(1 hunks)src/main/java/com/eternalcode/parcellockers/locker/repository/LockerRepositoryOrmLite.java(1 hunks)src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java(1 hunks)src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java(1 hunks)src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java(3 hunks)src/main/java/com/eternalcode/parcellockers/parcel/util/ParcelPlaceholderUtil.java(2 hunks)src/main/java/com/eternalcode/parcellockers/util/InventoryUtil.java(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- settings.gradle.kts
🚧 Files skipped from review as they are similar to previous changes (8)
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ReceiverSelectionGui.java
- src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerMainGui.java
- src/main/java/com/eternalcode/parcellockers/ParcelLockers.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelItemStorageGui.java
- build.gradle.kts
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelSendingGui.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (10)
src/main/java/com/eternalcode/parcellockers/parcel/util/ParcelPlaceholderUtil.java (1)
31-31: Nice null handling! 👍Good job using "-" as a default value when there's no description. This matches how we handle missing values elsewhere in the code.
src/main/java/com/eternalcode/parcellockers/content/repository/ParcelContentRepository.java (1)
17-17: LGTM!The
removeAll()method follows the consistent pattern used across repositories.src/main/java/com/eternalcode/parcellockers/locker/repository/LockerRepository.java (1)
26-27: LGTM!The
removeAll()method complements the existing remove methods and follows the consistent pattern.src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepository.java (1)
29-30: LGTM!The
removeAll()method follows the consistent pattern used across repositories.src/main/java/com/eternalcode/parcellockers/ParcelLockersCommand.java (1)
15-15: Nice cleanup!The command structure is now simpler with just one main command name.
src/main/java/com/eternalcode/parcellockers/itemstorage/repository/ItemStorageRepositoryOrmLite.java (1)
43-46: Good addition!The new removeAll method fits well with the existing repository pattern.
src/main/java/com/eternalcode/parcellockers/content/repository/ParcelContentRepositoryOrmLite.java (1)
38-41: Clean implementation!The removeAll method matches the pattern used in other repositories.
src/main/java/com/eternalcode/parcellockers/locker/repository/LockerRepositoryOrmLite.java (1)
102-105: Looks good!The new
removeAll()method is simple and consistent with other repository implementations.src/main/java/com/eternalcode/parcellockers/parcel/repository/ParcelRepositoryOrmLite.java (1)
129-132: Looks good!The
removeAll()method is simple and follows the repository pattern.gradle.properties (1)
1-9: Verify Gradle settings match your system resourcesThe build settings look good but please ensure:
- Your system has enough memory for the 4GB heap
- The worker count (8) matches your available CPU cores
✅ Verification successful
🌐 Web query:
What are the recommended Gradle memory settings for different system configurations?💡 Result:
Gradle memory configuration depends on project size and system resources. Here are optimized settings for different scenarios: ## Core Configuration Settings **Default values**: - Build VM: `-Xmx512m -XX:MaxMetaspaceSize=384m` ([2][6]) - Test tasks: `-Xmx512m` per process ([2][7]) **Key properties**: ```properties # gradle.properties org.gradle.jvmargs=-Xmx2048m -XX:MaxMetaspaceSize=512m org.gradle.workers.max=4 # Match to available RAM org.gradle.caching=true ``` ## System-Specific Recommendations | System Configuration | Memory Settings | Rationale | |-----------------------|-----------------|-----------| | Developer workstation (16GB+ RAM) | `-Xmx4096m`<br>`workers.max=CPU cores` | Supports large builds + IDE ([1][9]) | | CI Server (8GB RAM) | `-Xmx1536m`<br>`workers.max=2` | Balances parallelism and memory ([7][8]) | | Android projects | `-Xmx2048m`<br>Test tasks: `maxHeapSize=1024m` | Handles dexing + instrumentation ([4][9]) | | Low-memory systems (<4GB) | `-Xmx768m`<br>`org.gradle.parallel=false` | Prevents OOM errors ([6][9]) | ## Task-Specific Optimization For memory-intensive tasks: ```groovy // build.gradle test { maxHeapSize = "2g" // Per-test process } javaCompile { options.compilerArgs += ["-Xlint:none"] } ``` ## Monitoring & Validation 1. Use build scans with `--scan` flag for memory analysis ([2][6]) 2. Check daemon status: ```bash gradle --status ``` 3. Enable GC logging: ```properties org.gradle.jvmargs=-XX:+PrintGCDetails -XX:+PrintGCTimeStamps ``` For Android Studio users, adjust IDE memory separately under *Settings > Appearance & Behavior > System Settings > Memory Settings* while keeping Gradle configuration in `gradle.properties` ([9]). Citations: - 1: https://docs.gradle.org/current/userguide/performance.html - 2: https://docs.gradle.org/current/userguide/config_gradle.html - 3: https://tomgregory.com/gradle/gradle-project-properties-best-practices/ - 4: https://community.openfl.org/t/how-to-increase-memory-for-gradle-builds/11865 - 5: https://www.jetbrains.com/help/idea/gradle-settings.html - 6: https://docs.gradle.org/current/userguide/build_environment.html - 7: https://discuss.gradle.org/t/max-workers-and-memory-usage/28447 - 8: https://discuss.gradle.org/t/tons-of-gradle-daemons-exhausting-memory/20579 - 9: https://developer.android.com/studio/intro/studio-config - 10: https://github.com/gradle/gradle/issues/9329
Gradle Settings Verification
The configuration is appropriate if you're running on a system with sufficient resources—ideally, a workstation with 16GB+ RAM and at least 8 CPU cores. The 4GB heap and 8 workers match the recommended settings for a developer machine. If you're on a lower-spec system or a CI server, consider adjusting these values accordingly.
src/main/java/com/eternalcode/parcellockers/content/repository/ParcelContentRepository.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (1)
src/main/java/com/eternalcode/parcellockers/util/InventoryUtil.java (1)
14-23:⚠️ Potential issueFix the drop item logic.
The drop item code is inside the loop, which means it might drop items even when there's space in the inventory.
Here's a simple fix:
public static void addItem(Player player, ItemStack itemStack) { Inventory inventory = player.getInventory(); for (ItemStack item : inventory.getStorageContents()) { if (item == null) { inventory.addItem(itemStack); return; } - player.getWorld().dropItem(player.getLocation(), itemStack); } + player.getWorld().dropItem(player.getLocation(), itemStack); }
🧹 Nitpick comments (3)
src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java (2)
22-42: Add some friendly documentation to help future developers.A quick class description and some comments about what each dependency does would make this code more welcoming for new team members.
+/** + * Displays a GUI for players to view and collect their parcels. + * Supports pagination for handling multiple parcels. + */ public class ParcelCollectionGui implements GuiView {
109-113: Make parcel collection more robust.The GUI updates immediately after clicking, but what if the collection fails? Let's wait for success before updating the display.
gui.addItem(item.toGuiItem(event -> { - this.parcelManager.collectParcel(player, parcel); - gui.removeItem(event.getSlot()); - InventoryUtil.shiftItems(event.getSlot(), gui); + this.parcelManager.collectParcel(player, parcel) + .thenAccept(success -> { + if (success) { + gui.removeItem(event.getSlot()); + InventoryUtil.shiftItems(event.getSlot(), gui); + } + }); }));src/main/java/com/eternalcode/parcellockers/util/InventoryUtil.java (1)
35-48: Add input validation and consider simplifying.The method works but could be more robust with a few tweaks.
Here's a simpler version with validation:
public static void shiftItems(int removedSlot, BaseGui gui) { + if (removedSlot < 0 || gui == null) { + return; + } + Map<Integer, GuiItem> itemsToShift = gui.getGuiItems().entrySet() .stream() .filter(entry -> entry.getKey() > removedSlot) .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue)); - int currentShift = removedSlot; for (Map.Entry<Integer, GuiItem> entry : itemsToShift.entrySet()) { - int nextShift = entry.getKey(); GuiItem item = entry.getValue(); - gui.setItem(currentShift, item); - currentShift = nextShift; + gui.setItem(entry.getKey() - 1, item); } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
build.gradle.kts(4 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java(1 hunks)src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java(1 hunks)src/main/java/com/eternalcode/parcellockers/util/InventoryUtil.java(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (11)
src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java (1)
44-47: Nice and clean implementation!The method is simple and does exactly what it needs to do.
src/main/java/com/eternalcode/parcellockers/util/InventoryUtil.java (1)
25-33: Nice work!The method is simple and does its job well.
src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java (3)
25-30: Looking good!The constructor properly initializes all dependencies.
32-41: Great error handling!The method properly handles errors and provides good user feedback.
43-52: Well done!The method handles errors nicely and gives good feedback.
build.gradle.kts (6)
1-1: New Import for AbstractRunImporting
AbstractRunat the top sets you up for the new run task configuration. Looks great!
7-7: Updated Shadow Plugin VersionThe shadow plugin version is bumped to
9.0.0-beta8. Please double-check that the beta release meets your stability needs.
110-112: Revised Bukkit Plugin Description and VersionThe plugin description has been refreshed and the version updated to
0.0.1-SNAPSHOT. Make sure this change aligns with your release strategy.
122-128: New AbstractRun Task ConfigurationThe task configuration for
AbstractRunneatly sets the Java launcher and JVM arguments. This addition is clear and helpful.
147-148: Expanded Exclusions in ShadowJar TaskExcluding
javassist/**andorg/h2/util/**helps keep the jar clean. This looks well thought out.
156-177: Enhanced Relocation LogicThe relocation logic now dynamically applies prefixes to a list of dependencies. Please verify that each relocated package (e.g. for "org.bstats") is correctly mapped to match your usage in the codebase.
src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java
Outdated
Show resolved
Hide resolved
…gCommand.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…/locker/ParcelCollectionGui.java Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…l instead of InventoryUtil
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
build.gradle.kts (1)
11-11:⚠️ Potential issueFix version number inconsistency.
The root project version (1.0.0-SNAPSHOT) doesn't match the bukkit version (0.0.1-SNAPSHOT).
-version = "1.0.0-SNAPSHOT" +version = "0.0.1-SNAPSHOT"Also applies to: 112-112
🧹 Nitpick comments (2)
src/main/java/com/eternalcode/parcellockers/util/InventoryUtil.java (1)
14-22: Consider using streams for a more concise implementation.You can make this method shorter and cleaner.
- public static int freeSlotsInInventory(Player player) { - int freeSlots = 0; - for (ItemStack item : player.getInventory().getStorageContents()) { - if (item == null) { - freeSlots++; - } - } - return freeSlots; - } + public static int freeSlotsInInventory(Player player) { + return (int) Arrays.stream(player.getInventory().getStorageContents()) + .filter(item -> item == null) + .count(); + }build.gradle.kts (1)
135-147: Add file existence checks to clean tasks.The clean tasks should check if directories exist before trying to delete them.
cleanPaperPluginsCache { doLast { - project.file("run/plugins").deleteRecursively() + val pluginsDir = project.file("run/plugins") + if (pluginsDir.exists()) { + pluginsDir.deleteRecursively() + } } } cleanPaperCache { doLast { - project.file("run/cache").deleteRecursively() - project.file("run/logs").deleteRecursively() - project.file("run/versions").deleteRecursively() + listOf("cache", "logs", "versions").forEach { dir -> + val file = project.file("run/$dir") + if (file.exists()) { + file.deleteRecursively() + } + } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
build.gradle.kts(5 hunks)src/main/java/com/eternalcode/parcellockers/ParcelLockers.java(3 hunks)src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java(1 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java(1 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelItemStorageGui.java(10 hunks)src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java(1 hunks)src/main/java/com/eternalcode/parcellockers/util/InventoryUtil.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelCollectionGui.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/ParcelItemStorageGui.java
- src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java
- src/main/java/com/eternalcode/parcellockers/ParcelLockers.java
🔇 Additional comments (1)
src/main/java/com/eternalcode/parcellockers/util/InventoryUtil.java (1)
24-39: LGTM!The implementation is clean and efficient, with proper null checks and good use of streams.
| public void collectParcel(Player player, Parcel parcel) { | ||
| this.parcelContentRepository.findByUUID(parcel.uuid()).thenAccept(optional -> { | ||
| optional.ifPresent(content -> { | ||
| List<ItemStack> items = content.items(); | ||
| if (items.size() > freeSlotsInInventory(player)) { | ||
| player.playSound(player.getLocation(), Sound.ITEM_CHORUS_FRUIT_TELEPORT, 0.5F, 1); | ||
| this.announcer.sendMessage(player, this.config.messages.notEnoughInventorySpace); | ||
| return; | ||
| } | ||
|
|
||
| items.forEach(item -> | ||
| this.scheduler.run(() -> ItemUtil.giveItem(player, item)) | ||
| ); | ||
| this.parcelRepository.remove(parcel) | ||
| .thenCompose(v -> this.parcelContentRepository.remove(content.uniqueId())) | ||
| .whenComplete(SentryExceptionHandler.handler().andThen((v, throwable) -> { | ||
| if (throwable != null) { | ||
| this.announcer.sendMessage(player, this.config.messages.failedToCollectParcel); | ||
| return; | ||
| } | ||
| player.playSound(player.getLocation(), Sound.ENTITY_PLAYER_LEVELUP, 0.5F, 1); | ||
| this.announcer.sendMessage(player, this.config.messages.parcelSuccessfullyCollected); | ||
| } | ||
| )); | ||
| }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add handling for missing parcel content.
The method should inform the player when the parcel content is not found.
public void collectParcel(Player player, Parcel parcel) {
this.parcelContentRepository.findByUUID(parcel.uuid()).thenAccept(optional -> {
+ if (optional.isEmpty()) {
+ player.playSound(player.getLocation(), Sound.ITEM_CHORUS_FRUIT_TELEPORT, 0.5F, 1);
+ this.announcer.sendMessage(player, this.config.messages.failedToCollectParcel);
+ return;
+ }
optional.ifPresent(content -> {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public void collectParcel(Player player, Parcel parcel) { | |
| this.parcelContentRepository.findByUUID(parcel.uuid()).thenAccept(optional -> { | |
| optional.ifPresent(content -> { | |
| List<ItemStack> items = content.items(); | |
| if (items.size() > freeSlotsInInventory(player)) { | |
| player.playSound(player.getLocation(), Sound.ITEM_CHORUS_FRUIT_TELEPORT, 0.5F, 1); | |
| this.announcer.sendMessage(player, this.config.messages.notEnoughInventorySpace); | |
| return; | |
| } | |
| items.forEach(item -> | |
| this.scheduler.run(() -> ItemUtil.giveItem(player, item)) | |
| ); | |
| this.parcelRepository.remove(parcel) | |
| .thenCompose(v -> this.parcelContentRepository.remove(content.uniqueId())) | |
| .whenComplete(SentryExceptionHandler.handler().andThen((v, throwable) -> { | |
| if (throwable != null) { | |
| this.announcer.sendMessage(player, this.config.messages.failedToCollectParcel); | |
| return; | |
| } | |
| player.playSound(player.getLocation(), Sound.ENTITY_PLAYER_LEVELUP, 0.5F, 1); | |
| this.announcer.sendMessage(player, this.config.messages.parcelSuccessfullyCollected); | |
| } | |
| )); | |
| }); | |
| }); | |
| } | |
| public void collectParcel(Player player, Parcel parcel) { | |
| this.parcelContentRepository.findByUUID(parcel.uuid()).thenAccept(optional -> { | |
| if (optional.isEmpty()) { | |
| player.playSound(player.getLocation(), Sound.ITEM_CHORUS_FRUIT_TELEPORT, 0.5F, 1); | |
| this.announcer.sendMessage(player, this.config.messages.failedToCollectParcel); | |
| return; | |
| } | |
| optional.ifPresent(content -> { | |
| List<ItemStack> items = content.items(); | |
| if (items.size() > freeSlotsInInventory(player)) { | |
| player.playSound(player.getLocation(), Sound.ITEM_CHORUS_FRUIT_TELEPORT, 0.5F, 1); | |
| this.announcer.sendMessage(player, this.config.messages.notEnoughInventorySpace); | |
| return; | |
| } | |
| items.forEach(item -> | |
| this.scheduler.run(() -> ItemUtil.giveItem(player, item)) | |
| ); | |
| this.parcelRepository.remove(parcel) | |
| .thenCompose(v -> this.parcelContentRepository.remove(content.uniqueId())) | |
| .whenComplete(SentryExceptionHandler.handler().andThen((v, throwable) -> { | |
| if (throwable != null) { | |
| this.announcer.sendMessage(player, this.config.messages.failedToCollectParcel); | |
| return; | |
| } | |
| player.playSound(player.getLocation(), Sound.ENTITY_PLAYER_LEVELUP, 0.5F, 1); | |
| this.announcer.sendMessage(player, this.config.messages.parcelSuccessfullyCollected); | |
| } | |
| )); | |
| }); | |
| }); | |
| } |
src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/parcel/util/ParcelPlaceholderUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerMainGui.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java
Outdated
Show resolved
Hide resolved
…er.java Co-authored-by: DMK <[email protected]>
…gCommand.java Co-authored-by: DMK <[email protected]>
…gCommand.java Co-authored-by: DMK <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java (1)
69-71: Consider bulk item giving.Instead of scheduling each item separately, you could give all items at once:
-items.forEach(item -> - this.scheduler.run(() -> ItemUtil.giveItem(player, item)) -); +this.scheduler.run(() -> items.forEach(item -> ItemUtil.giveItem(player, item)));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
build.gradle.kts(5 hunks)src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java(1 hunks)
🔇 Additional comments (13)
src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java (4)
3-33: Nice work on the setup!The new dependencies and fields are well-organized and clearly named.
35-46: Good job on the error handling!The updated exception handling with Sentry looks clean and follows the suggested formatting.
48-57: Nice consistency in error handling!The error handling matches the pattern used in createParcel.
59-84: Add handling for missing parcel content.The method should let the player know when the parcel is empty.
build.gradle.kts (9)
1-2: New Import for AbstractRun
This new import supports the added run-task configuration.
7-7: Updated Shadow Plugin Version
The plugin version is bumped to "9.0.0-beta8". Please verify its stability during testing.
11-11: Project Version Change
The project version update to "0.0.1-SNAPSHOT" is clear. Ensure it matches your release strategy.
88-90: New Commons Libraries Dependencies
The addition of these dependencies shows improved consistency. Double-check this change across the modules.
122-128: Added AbstractRun Task Configuration
This new task block sets up your Java launcher with JetBrains at Java 21 and specifies useful JVM arguments.
135-139: New cleanPaperPluginsCache Task
This task simplifies cleaning the plugins directory. It’s a neat addition for a tidy workspace.
141-147: New cleanPaperCache Task
The task cleans cache, logs, and version folders, which is useful for a fresh run environment.
161-163: Expanded Exclusion List in ShadowJar
Excluding "javassist/" and "org/h2/util/" now helps reduce potential conflicts.
170-190: Enhanced Relocation Logic
Using a dynamic prefix for relocations improves flexibility by handling multiple packages efficiently. Make sure all critical packages are relocated as expected.
…gCommand.java Co-authored-by: DMK <[email protected]>
…gCommand.java Co-authored-by: DMK <[email protected]>
…gCommand.java Co-authored-by: DMK <[email protected]>
…gCommand.java Co-authored-by: DMK <[email protected]>
…ParcelLockers into receiving-logic
build.gradle.kts
Outdated
| description = | ||
| "Plugin that provides functionality of parcel lockers in Minecraft, allowing players to send and receive parcels safely." |
There was a problem hiding this comment.
| description = | |
| "Plugin that provides functionality of parcel lockers in Minecraft, allowing players to send and receive parcels safely." | |
| description = "Plugin that provides functionality of parcel lockers in Minecraft, allowing players to send and receive parcels safely." |
| this.parcelRepository.removeAll() | ||
| .exceptionally(throwable -> { | ||
| this.announcer.sendMessage(player, "&cFailed to delete parcels"); | ||
| return null; | ||
| }) | ||
| .thenRun(() -> this.announcer.sendMessage(player, "&aParcels deleted")); |
| this.lockerRepository.removeAll() | ||
| .exceptionally(throwable -> { | ||
| this.announcer.sendMessage(player, "&cFailed to delete lockers"); | ||
| return null; | ||
| }) | ||
| .thenRun(() -> this.announcer.sendMessage(player, "&aLockers deleted")); |
| this.itemStorageRepository.removeAll() | ||
| .exceptionally(throwable -> { | ||
| this.announcer.sendMessage(player, "&cFailed to delete item storages"); | ||
| return null; | ||
| }) | ||
| .thenRun(() -> this.announcer.sendMessage(player, "&aItem storages deleted")); |
| this.contentRepository.removeAll() | ||
| .exceptionally(throwable -> { | ||
| this.announcer.sendMessage(player, "&cFailed to delete parcel contents"); | ||
| return null; | ||
| }) | ||
| .thenRun(() -> this.announcer.sendMessage(player, "&aParcel contents deleted")); |
| @Execute(name = "getrandomitem") | ||
| void getRandomItem(@Context Player player, @Arg int stacks) { | ||
| if (stacks <= 0 || stacks > 36) { | ||
| this.announcer.sendMessage(player, "&cPlease request between 1 and 36 stacks"); | ||
| return; | ||
| } | ||
|
|
||
| List<Material> itemMaterials = Arrays.stream(Material.values()) | ||
| .filter(Material::isItem) | ||
| .toList(); |
src/main/java/com/eternalcode/parcellockers/configuration/implementation/ConfigItem.java
Outdated
Show resolved
Hide resolved
src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerMainGui.java
Outdated
Show resolved
Hide resolved
|
|
…ementation/ConfigItem.java Co-authored-by: DMK <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java (1)
59-84: 🛠️ Refactor suggestionAdd handling for missing parcel content.
The method should inform the player when the parcel content is not found.
public void collectParcel(Player player, Parcel parcel) { this.parcelContentRepository.findByUUID(parcel.uuid()).thenAccept(optional -> { + if (optional.isEmpty()) { + player.playSound(player.getLocation(), Sound.ITEM_CHORUS_FRUIT_TELEPORT, 0.5F, 1); + this.announcer.sendMessage(player, this.config.messages.failedToCollectParcel); + return; + } optional.ifPresent(content -> {
🧹 Nitpick comments (3)
src/test/java/com/eternalcode/parcellockers/TestScheduler.java (2)
17-17: Consider making thread pool size configurableThe fixed thread pool size of 8 might be more than needed for tests. Consider making this configurable or using a smaller default size.
- private final ScheduledExecutorService executorService = Executors.newScheduledThreadPool(8); + private static final int DEFAULT_THREAD_POOL_SIZE = 4; + private final ScheduledExecutorService executorService; + + public TestScheduler() { + this(DEFAULT_THREAD_POOL_SIZE); + } + + public TestScheduler(int threadPoolSize) { + this.executorService = Executors.newScheduledThreadPool(threadPoolSize); + }
19-21: Add automatic resource cleanupConsider implementing AutoCloseable to ensure the executor service is properly shut down.
-public class TestScheduler implements Scheduler { +public class TestScheduler implements Scheduler, AutoCloseable { // ... existing code ... + @Override + public void close() { + shutdown(); + }src/main/java/com/eternalcode/parcellockers/locker/controller/LockerPlaceController.java (1)
89-93: Let's make error messages more helpful!Right now, when something goes wrong with saving a locker, we just print the error and tell the player it failed. We could make this better by:
- Logging the specific error details for troubleshooting
- Giving players more helpful error messages
Here's a simple way to improve this:
if (throwable != null) { - throwable.printStackTrace(); + plugin.getLogger().severe("Failed to save parcel locker: " + throwable.getMessage()); + String errorDetails = throwable instanceof SQLException ? + "database error" : "unexpected error"; this.announcer.sendMessage(player, - this.config.messages.failedToCreateParcelLocker); + this.config.messages.failedToCreateParcelLocker + .replace("{error}", errorDetails)); return; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
README.md(1 hunks)build.gradle.kts(6 hunks)gradle.properties(1 hunks)settings.gradle.kts(1 hunks)src/main/java/com/eternalcode/parcellockers/ParcelLockers.java(4 hunks)src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java(1 hunks)src/main/java/com/eternalcode/parcellockers/configuration/implementation/ConfigItem.java(3 hunks)src/main/java/com/eternalcode/parcellockers/database/DatabaseManager.java(2 hunks)src/main/java/com/eternalcode/parcellockers/database/persister/PositionPersister.java(0 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerMainGui.java(7 hunks)src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/SentParcelsGui.java(5 hunks)src/main/java/com/eternalcode/parcellockers/locker/controller/LockerPlaceController.java(1 hunks)src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java(1 hunks)src/main/java/com/eternalcode/parcellockers/parcel/util/ParcelPlaceholderUtil.java(2 hunks)src/main/java/com/eternalcode/parcellockers/shared/Position.java(1 hunks)src/main/java/com/eternalcode/parcellockers/util/ItemSerdesUtil.java(4 hunks)src/test/java/com/eternalcode/parcellockers/TestScheduler.java(1 hunks)src/test/java/com/eternalcode/parcellockers/parcel/repository/ParcelPageTest.java(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/com/eternalcode/parcellockers/database/persister/PositionPersister.java
✅ Files skipped from review due to trivial changes (4)
- src/main/java/com/eternalcode/parcellockers/database/DatabaseManager.java
- src/main/java/com/eternalcode/parcellockers/shared/Position.java
- README.md
- src/test/java/com/eternalcode/parcellockers/parcel/repository/ParcelPageTest.java
🚧 Files skipped from review as they are similar to previous changes (7)
- settings.gradle.kts
- src/main/java/com/eternalcode/parcellockers/parcel/util/ParcelPlaceholderUtil.java
- gradle.properties
- src/main/java/com/eternalcode/parcellockers/gui/implementation/remote/SentParcelsGui.java
- src/main/java/com/eternalcode/parcellockers/gui/implementation/locker/LockerMainGui.java
- src/main/java/com/eternalcode/parcellockers/command/debug/DebugCommand.java
- src/main/java/com/eternalcode/parcellockers/ParcelLockers.java
🔇 Additional comments (6)
src/test/java/com/eternalcode/parcellockers/TestScheduler.java (1)
23-90: Implementation looks good!The scheduler methods are well-implemented with proper handling of both synchronous and asynchronous operations.
src/main/java/com/eternalcode/parcellockers/configuration/implementation/ConfigItem.java (2)
27-28: Looks good!The formatting changes make the code easier to read.
110-112: Looks good!The exception handling is properly formatted.
src/main/java/com/eternalcode/parcellockers/util/ItemSerdesUtil.java (1)
29-29: Looking good!The formatting changes to the catch blocks are consistent and don't affect functionality.
Also applies to: 38-38, 47-47, 56-56
src/main/java/com/eternalcode/parcellockers/parcel/ParcelManager.java (1)
35-57: Nice error handling!The consistent use of SentryExceptionHandler and user feedback is well done.
build.gradle.kts (1)
134-146: Great cleanup tasks!The new tasks for cleaning Paper cache and plugins will help keep the development environment tidy.
| public GuiItem toGuiItem() { | ||
| return this.toGuiItem(event -> {}); | ||
| return this.toGuiItem(); | ||
| } |
There was a problem hiding this comment.
Fix infinite recursion in toGuiItem()
The method is calling itself endlessly! This will crash your program.
Let's fix it:
- return this.toGuiItem();
+ return this.toBuilder().asGuiItem();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public GuiItem toGuiItem() { | |
| return this.toGuiItem(event -> {}); | |
| return this.toGuiItem(); | |
| } | |
| public GuiItem toGuiItem() { | |
| - return this.toGuiItem(); | |
| + return this.toBuilder().asGuiItem(); | |
| } |
| id("net.minecrell.plugin-yml.bukkit") version "0.6.0" | ||
| id("xyz.jpenilla.run-paper") version "2.3.1" | ||
| id("com.gradleup.shadow") version "8.3.5" | ||
| id("com.gradleup.shadow") version "9.0.0-beta8" |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Consider using a stable version of shadow plugin.
Using beta version 9.0.0-beta8 in production could lead to unexpected issues.
🌐 Web query:
What is the latest stable version of gradle shadow plugin?
💡 Result:
The latest stable version of the Gradle Shadow Plugin is v8.3.1, released on September 10, 2024[1][10]. This version includes compatibility updates and fixes for issues like excluding Gradle APIs in Java Gradle Plugin publications[1][10].
Key Changes in v8.x Series:
- v8.3.x introduced breaking changes to align with Gradle standards:
- Plugin ID changed from
com.github.johnrengelman.shadowtocom.gradleup.shadow[1][5]. - Maven coordinates updated to
com.gradleup.shadow:shadow-gradle-plugin[1][5].
- Plugin ID changed from
- v9.x releases (e.g.,
9.0.0-beta7,9-beta) are still in beta as of February 2025[1][5].
For production use with Java projects targeting Java ≤11 or needing stability, v8.3.x remains recommended until v9 stabilizes[1][10].
Citations:
- 1: https://gradleup.com/shadow/changes/
- 2: https://docs.gradle.org/8.1.1/release-notes.html
- 3: https://github.com/GradleUp/shadow/releases
- 4: https://micronaut-projects.github.io/micronaut-gradle-plugin/latest/
- 5: https://plugins.gradle.org/plugin/com.gradleup.shadow
- 6: https://andresalmiray.com/jreleaser-0-2-0-has-been-released/
- 7: https://discuss.gradle.org/t/gradle-6-4-1-appears-to-have-broken-parts-of-the-shadow-plug-in/36133
- 8: https://plugins.gradle.org/plugin/com.github.johnrengelman.shadow
- 9: https://github.com/GradleUp/shadow
- 10: https://gradleup.com/shadow/
Switch to a Stable Shadow Plugin Version
- In build.gradle.kts at line 7, you're using the beta version
"9.0.0-beta8". - For better production stability, please update the version to the latest stable release
"8.3.1".
- id("com.gradleup.shadow") version "9.0.0-beta8"
+ id("com.gradleup.shadow") version "8.3.1"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| id("com.gradleup.shadow") version "9.0.0-beta8" | |
| id("com.gradleup.shadow") version "8.3.1" |
No description provided.