GH-916 Refactor packages in item-edit related commands. #916
Conversation
Signed-off-by: Martin Sulikowski <vLuckyyy.biznes@gmail.com>
WalkthroughThis update restructures the item editing functionality across several components. Package declarations have been revised, moving classes from an "essentials" context to a dedicated "itemedit" context. The commands for managing item flags, lore, and names now refer to updated translation messages, ensuring consistency within the new context. In addition, new methods have been introduced—such as a method for removing a specific lore line—and changes have been made to the suggestions and validation logic for item lore. New classes and interfaces for item editing messages have been added, and existing translation interfaces and implementations have been updated accordingly. The overall control flow in the commands remains unchanged, with the focus being on aligning feedback messages and structural organization with the dedicated item editing features. 📜 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)
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 (
|
vLuckyyy
changed the title
vLuckyyy
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: 0
🧹 Nitpick comments (3)
eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/messages/PLItemEditMessages.java (2)
9-12: Add a class-level comment explaining the fluent accessor pattern.A brief comment would help other developers understand why fluent accessors are used here.
+/** + * Polish messages for item editing operations. + * Uses fluent accessors for a more readable message chain syntax. + */ @Getter @Accessors(fluent = true) @Contextual public class PLItemEditMessages implements ItemEditMessages {
17-18: Add meaningful descriptions for all messages.Some messages have empty descriptions (" "). Adding context would help with maintenance and localization.
For example:
- @Description(" ") + @Description("Message shown when item name is cleared") public Notice itemClearNameMessage = Notice.chat("<green>► <white>Wyczyszczono nazwę przedmiotu!");Also applies to: 23-24, 33-34, 36-37, 39-40
eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/ItemLoreCommand.java (1)
48-49: Smart expansion.
Automatically ensuring the list grows is handy, but watch out for extremely large line values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/ItemFlagCommand.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/ItemLoreArgument.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/ItemLoreCommand.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/ItemNameCommand.java(3 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/messages/ENItemEditMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/messages/ItemEditMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/messages/PLItemEditMessages.java(1 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java(2 hunks)eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java(2 hunks)
🔇 Additional comments (24)
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/PLTranslation.java (2)
8-8: LGTM!The import for
PLItemEditMessagesaligns with the restructuring of item editing functionality into a dedicated package.
115-116: LGTM!The new section for item editing messages is well-documented and properly integrated into the translation structure.
eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/messages/PLItemEditMessages.java (2)
1-8: Nice package organization!The package structure and imports are well-organized and clean.
14-40: Great message formatting consistency!The use of consistent colors and symbols (► for success, ✖ for errors) makes the messages clear and user-friendly.
eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/ItemLoreArgument.java (3)
1-1: Nice package organization!Moving this to the
itemeditpackage makes perfect sense and helps keep related code together.
20-20: Good user experience improvement!Changing suggestions to start from 1 instead of 0 makes more sense for users, as they typically think in terms of line 1, line 2, etc., rather than starting from 0.
33-38:❓ Verification inconclusive
Verify the index conversion logic.
The code now accepts numbers starting from 1 (user-friendly) and converts them to 0-based indices (for internal use). While this is a good approach, let's make sure all related code expects 0-based indices.
🏁 Script executed:
#!/bin/bash # Search for any lore-related operations that might need similar index adjustments rg -l "lore" | xargs rg "(?i)(get|set|remove|add).*lore.*index"Length of output: 63
Please confirm index conversion consistency
The update in
ItemLoreArgument.javacorrectly adjusts a 1-based number to a 0-based index. Since our search didn't return any instances of lore-related index manipulations elsewhere, it's important to manually verify that all parts of the code handling lore indices use the 0-based convention.
- Ensure that any methods or operations involving lore indices (like get, set, remove, or add) are updated or already expect 0-based indices.
- Confirm that this conversion doesn’t inadvertently affect other parts of the system.
eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/ItemLoreCommand.java (9)
1-1: No worries here.
The package name is clear and matches the new structure nicely.
8-11: Looks tidy.
Imports are simple and fit well with the command setup.Also applies to: 13-14
52-52: Nice approach.
Using MiniMessage for rich text keeps lore formatting flexible.
58-58: Good feedback.
Notifying the user with the new message context is a solid choice.
64-66: Straightforward command.
Clearly named for removing lore, which makes it easy to understand.
67-96: Thoughtful edge cases.
Gracefully handles missing lore and invalid lines. This keeps things user-friendly.
99-99: Good clarity.
A dedicated clear command is intuitive for players.
104-104: Consistent messaging.
Using the same no-item notice preserves a unified player experience.
112-112: Helpful final note.
Clear message on lore removal completion.eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/messages/ItemEditMessages.java (1)
1-23: Well-organized interface.
Grouping all item edit notices in one place is a neat step for clarity.eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/messages/ENItemEditMessages.java (1)
1-41: Great localization setup.
Providing descriptive placeholders and straightforward text will help users quickly grasp changes.eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/ItemFlagCommand.java (2)
1-1: Looking good! Package and imports are properly organized.The move from
essentialstoitemeditpackage makes the code structure clearer.Also applies to: 7-7, 9-9
46-47: Nice update to the translation calls!The translation calls now use
itemEdit()instead ofitem(), which aligns with the new package structure.Also applies to: 53-53, 69-69
eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/ItemNameCommand.java (2)
1-1: Package move and new import look good!The code is now in a more logical location with the required imports.
Also applies to: 13-13
49-49: Translation calls updated correctly!The message handling now uses the new
itemEdit()context.Also applies to: 69-69
eternalcore-core/src/main/java/com/eternalcode/core/translation/Translation.java (1)
8-8: Good addition of the new translation section!The new
itemEdit()method with its description helps organize item-related messages better.Also applies to: 227-228
eternalcore-core/src/main/java/com/eternalcode/core/translation/implementation/ENTranslation.java (1)
8-8: Nice implementation of the new translation section!The English translation now properly supports the new item editing messages.
Also applies to: 118-119
eternalcore-core/src/main/java/com/eternalcode/core/feature/itemedit/ItemLoreArgument.java
Outdated
Show resolved
Hide resolved
Continuation of: #906
Date: January 18th
Time: 12:08 PM
Location: Virtual team gathering (read: me + Rollczi, which basically means the whole team, right?)
On this day, in an atmosphere of utmost seriousness (and zero consultation with the rest of the team), we made the decision to restructure the packages. Why? Because the previous PL and EN files, along with the Translation interface, looked like a pile of crap. Especially for the poor developer who had to wade through 2k lines of code just to add one message. Well, a programmer's life is no fairy tale.
Pull request #906 was merged without any review, because we don’t have time for such silly formalities – much like prosecutor Michał Ostrowski doesn’t have time for meticulous investigations.
And today? Today I decided to continue this epic saga of crappy code. I moved the lore package, the
ItemFlagcommand, andItemNamefrom theessentialspackage to theitemeditpackage. It's kind of like an immigrant moving from Germany to Poland – everything has to find a new place.Now it looks like this: