GH-893 Enhance Auto-Warp GUI and Fix Related Bugs #893
Conversation
# Conflicts: # eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java
WalkthroughThis pull request introduces several modifications to the warp-related components in the EternalCore project. Key changes include renaming the Additionally, the 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (
|
![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/configuration/implementation/PluginConfiguration.java (1)
346-348: Consider enhancing the configuration descriptionThe description could be more detailed to help users understand when they might want to enable this option. For example, explain the benefits and potential impact of recalculating slots after warp deletion.
- @Description("# Should item slots be recalculated after warp deletion") + @Description({ + "# Should item slots be recalculated after warp deletion", + "# When enabled, warp slots in the GUI will be reorganized after a warp is deleted", + "# This helps maintain a clean layout but may change existing warp positions" + })eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/command/SetWarpCommand.java (1)
Line range hint
22-22: Consider moving MAX_WARPS_IN_GUI to configurationThe hardcoded value of 56 might be better placed in the configuration file for easier customization.
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java (3)
25-27: Remove unused importThe
Comparatorimport isn't being used anywhere in the code.import java.util.ArrayList; import java.util.Collections; -import java.util.Comparator; import java.util.List;
247-259: Consider adding a method commentNice job extracting this logic into a separate method! Adding a brief comment explaining the slot calculation would make it even better.
+ /** + * Calculates the next available slot based on the border configuration. + * @param warpSection The warp inventory section containing border settings + * @return The calculated slot number + */ private int getSlot(Translation.WarpSection.WarpInventorySection warpSection) {
283-294: Consider simplifying the shift logicThe slot shifting could be simplified using a stream operation.
private void shiftWarpItems(WarpInventoryItem removed, Translation.WarpSection.WarpInventorySection warpSection) { int removedSlot = removed.warpItem.slot; - List<WarpInventoryItem> iterator = warpSection.items().values().stream() + warpSection.items().values().stream() .filter(item -> item.warpItem.slot > removedSlot) - .toList(); - - int currentShift = removedSlot; - for (WarpInventoryItem item : iterator) { - int nextShift = item.warpItem.slot; - item.warpItem.slot = currentShift; - currentShift = nextShift; - } + .forEach(item -> item.warpItem.slot--); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
eternalcore-api/src/main/java/com/eternalcode/core/feature/warp/WarpService.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/configuration/implementation/PluginConfiguration.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java(7 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpServiceImpl.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/command/DelWarpCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/command/SetWarpCommand.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java(1 hunks)
🔇 Additional comments (10)
eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java (1)
179-184: Nice improvement to the removeItem method!Returning the removed item is a helpful change that makes it easier to track what was removed.
eternalcore-core/src/main/java/com/eternalcode/core/configuration/implementation/PluginConfiguration.java (2)
13-13: LGTM!Clean import addition.
351-351: LGTM!Simple formatting improvement.
eternalcore-api/src/main/java/com/eternalcode/core/feature/warp/WarpService.java (1)
21-21: Nice improvement to method naming!The change from
isExisttoexistsfollows better Java naming conventions.eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/command/DelWarpCommand.java (1)
40-40: Looking good!Method call properly updated to match the new interface.
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/command/SetWarpCommand.java (1)
47-47: Perfect update!Method call correctly updated to use the new name.
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpServiceImpl.java (1)
94-96: Clean implementation!The method rename is properly implemented with the same functionality.
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java (3)
44-44: Nice rename!Renaming from
warpManagertowarpServicebetter describes its role.Also applies to: 55-55, 64-64
168-168: LGTM!The change is consistent with the service rename.
262-268: LGTM!Good addition of the config check and consistent method name usage.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java (3)
97-98: Consider a more professional constant nameInstead of
UGLY_BORDER_ROW_COUNT, consider using a more descriptive name likeSINGLE_BORDER_ROW_COUNT.
249-261: Add comments explaining the slot calculationsThe math for calculating slots is complex. Consider adding comments to explain what each calculation does and why.
281-293: Consider simplifying with Java streamsThe shifting logic could be more concise using Java streams with
mapandcollect.- int currentShift = removedSlot; - for (WarpInventoryItem item : itemsToShift) { - int nextShift = item.warpItem.slot; - item.warpItem.slot = currentShift; - currentShift = nextShift; - } + AtomicInteger currentShift = new AtomicInteger(removedSlot); + itemsToShift.forEach(item -> { + int nextShift = item.warpItem.slot; + item.warpItem.slot = currentShift.get(); + currentShift.set(nextShift); + });
📜 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/warp/WarpInventory.java(8 hunks)
🔇 Additional comments (2)
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java (2)
41-42: Nice improvements to naming and constants!The rename to warpService and the new border constants make the code clearer.
Also applies to: 46-46, 57-57, 66-66
222-229:⚠️ Potential issueFix the exists check logic
The early return condition seems backwards. The method returns if the warp exists, but it should probably return if it doesn't exist.
- if (!this.warpService.exists(warp.getName())) { + if (this.warpService.exists(warp.getName())) {Likely invalid or redundant comment.
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/warp/WarpInventory.java
Show resolved
Hide resolved
CitralFlo
left a comment
CitralFlo
left a comment
There was a problem hiding this comment.
Love the hate for ugly ass borders <3
vLuckyyy
changed the title
vLuckyyy
changed the title
No description provided.