Add multilingual support for 13 additional languages#48
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 Walkthrough<review_stack_artifact> </review_stack_artifact> ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/java/dev/noah/perplayerkit/commands/core/CommandGuards.java (1)
40-68:⚠️ Potential issue | 🟠 MajorRemove unused two-argument guard overloads or migrate call sites to use localized single-argument versions.
The two-argument overloads
requirePlayer(CommandSender, String)andrequirePlayerInEnabledWorld(CommandSender, String)are currently used inRepairCommand,HealCommand, andCommandGuardsTest, passing hardcoded non-localized messages. Either deprecate and remove these overloads once call sites are migrated to the single-argument versions, or keep them if non-localized messages are intentional.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/noah/perplayerkit/commands/core/CommandGuards.java` around lines 40 - 68, The two-argument overloads requirePlayer(CommandSender, String) and requirePlayerInEnabledWorld(CommandSender, String) are redundant and pass hardcoded non-localized messages from RepairCommand, HealCommand, and CommandGuardsTest; either remove these overloads and update all call sites to use the single-argument requirePlayer(CommandSender) and requirePlayerInEnabledWorld(CommandSender) (which perform localization), or if non-localized messages are intended keep them and add a deprecation note—modify RepairCommand, HealCommand, and CommandGuardsTest to call the single-argument variants and remove the two-arg methods (requirePlayer(...) with message and requirePlayerInEnabledWorld(...) with message) from CommandGuards.java if migrating.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/java/dev/noah/perplayerkit/commands/features/RegearCommand.java`:
- Around line 259-263: The holder currently throws UnsupportedOperationException
in getInventory(), violating InventoryHolder; change the holder (the
RegearCommand inventory holder record/class used by createRegearInventory) to
store a mutable Inventory field and implement getInventory() to return that
field; update createRegearInventory to set the holder's inventory after creating
the Inventory (or pass the Inventory into the holder constructor) so
RegearCommand#createRegearInventory and the holder's getInventory() consistently
return the created inventory.
In `@src/main/java/dev/noah/perplayerkit/ConfigMigrator.java`:
- Around line 127-156: The migration incorrectly compares user values to the
wrong defaults and hardcodes a disabled-message default; in ConfigMigrator
update the two comparisons to read the canonical default from the defaults
config instead of hardcoding or using oldPath: for the broadcast mapping use
defaults.getString(newKey) (not defaults.getString(oldPath)) when computing
defaultValue before putting into diffs, and for the "disabled-command-message"
check fetch the default from defaults (e.g.,
defaults.getString("error.disabled-in-world") or the matching bundled key)
instead of the literal "<red>Kits are disabled here!</red>" so unchanged
defaults are not treated as custom. Ensure you reference userConfig, defaults,
broadcastKeyMap, oldPath, newKey and diffs when making the replacements.
In `@src/main/java/dev/noah/perplayerkit/KitManager.java`:
- Around line 373-374: The current call to loadKitInternal dereferences player
via IDUtil.getPlayerKitId(player.getUniqueId(), slot) before loadKitInternal can
null-check player, causing an NPE; fix by moving the player-dependent
computation behind the null-guard: either perform the null check up-front in the
caller or change the call to pass a Supplier/closure (defer calling
IDUtil.getPlayerKitId) so loadKitInternal can call getUniqueId() only after
verifying player != null; update both occurrences that use
IDUtil.getPlayerKitId(...) (the calls around loadKitInternal and the similar
call at lines noted) and ensure Lang.get().send(...) remains as the error
supplier passed into loadKitInternal.
In `@src/main/java/dev/noah/perplayerkit/listeners/KitMenuCloseListener.java`:
- Around line 47-86: The close handler is fragile because parseTitle parses
localized, styled inventory titles (method parseTitle in KitMenuCloseListener
using Lang.get(), StyleManager.get(), legacyConvert); instead, persist the menu
context at open time and read it on close: create a Map<UUID, MenuContext>
(MenuContext holds menu type and the slot/id/player fields) populated when
opening menus, update all open-time code paths that build these titles to put
the corresponding context into that map, and change the close handlers (replace
calls to parseTitle and any title-based logic in KitMenuCloseListener methods)
to lookup and consume the MenuContext by viewer UUID (and remove fallbacks that
rely on parsing UI text). Apply the same refactor to the other affected blocks
referenced (around lines 93-121, 131-137, 147-174, 184-211, 214-224) so all
save/close logic uses the stored context instead of parsing localized titles.
In `@src/main/java/dev/noah/perplayerkit/util/Lang.java`:
- Around line 186-190: The current rawList method treats empty lists as missing
and always falls back, which prevents intentionally empty lists; change the
fallback check to only trigger when the active language does not define the key.
In rawList, call lang.getStringList(key) as before but replace the condition
"value == null || value.isEmpty()" with a presence check such as
"!lang.contains(key)" (or "!lang.isSet(key)" depending on your API) so that an
explicitly empty list from lang is preserved and only keys absent from lang use
fallback.getStringList(key); ensure you still handle a null fallback safely.
In `@src/main/resources/lang/en.yml`:
- Around line 43-46: Replace the visible typos in the given message keys: change
empty-kit to use "can't" ("You can't save an empty kit!"), change empty-ec to
"You can't save an empty enderchest!", and change kit-slot-not-found to "Kit
{slot} doesn't exist!"; leave kit-deletion-failed as-is if correct. Also search
for other default English strings that use "cant", "doesnt", or awkward phrasing
like "setup ... yet" (the additional occurrence noted in the review) and correct
them to proper contractions and grammar.
In `@src/main/resources/lang/nl.yml`:
- Line 38: The YAML key error.unexpected contains an opened "<red>" color tag
that is not explicitly closed; update the value for error.unexpected to include
a closing "</red>" tag so the color markup is balanced and renders consistently
(locate the error.unexpected entry in src/main/resources/lang/nl.yml and add the
closing </red> to the string).
- Around line 21-23: Escape literal MiniMessage angle-bracket tokens and fix tag
closures in the Dutch language strings: replace occurrences of placeholders like
<slot>, <id>, <kitid>, <speler|uuid>, <load/save> (and any similar <...> tokens)
with escaped-bracket forms (\<slot\>) or alternative placeholders ({slot}) so
MiniMessage doesn't treat them as tags, and ensure color tags such as <aqua> and
<white> are properly closed (e.g., close </aqua> before starting <white>)
throughout the affected strings in src/main/resources/lang/nl.yml (notably the
lines showing "<white>Wil je een kit delen? Gebruik <aqua>/sharekit
<slot><white>." and "<white>Typ <aqua>/kit<white>, <aqua>/k <white>of
<aqua>/pk<white> om te beginnen!" and the other spots referenced).
In `@src/main/resources/lang/zh.yml`:
- Line 38: The localization string for error.unexpected opens a <red> color tag
but doesn't close it; update the value for error.unexpected (the string
"<red>发生意外错误,请重试。") to explicitly close the tag by adding the corresponding
</red> at the end so the color markup is balanced.
---
Outside diff comments:
In `@src/main/java/dev/noah/perplayerkit/commands/core/CommandGuards.java`:
- Around line 40-68: The two-argument overloads requirePlayer(CommandSender,
String) and requirePlayerInEnabledWorld(CommandSender, String) are redundant and
pass hardcoded non-localized messages from RepairCommand, HealCommand, and
CommandGuardsTest; either remove these overloads and update all call sites to
use the single-argument requirePlayer(CommandSender) and
requirePlayerInEnabledWorld(CommandSender) (which perform localization), or if
non-localized messages are intended keep them and add a deprecation note—modify
RepairCommand, HealCommand, and CommandGuardsTest to call the single-argument
variants and remove the two-arg methods (requirePlayer(...) with message and
requirePlayerInEnabledWorld(...) with message) from CommandGuards.java if
migrating.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f9ed4c63-1c9b-49d3-bbe2-80093516f44e
📒 Files selected for processing (51)
src/main/java/dev/noah/perplayerkit/ConfigManager.javasrc/main/java/dev/noah/perplayerkit/ConfigMigrator.javasrc/main/java/dev/noah/perplayerkit/KitManager.javasrc/main/java/dev/noah/perplayerkit/KitShareManager.javasrc/main/java/dev/noah/perplayerkit/PerPlayerKit.javasrc/main/java/dev/noah/perplayerkit/UpdateChecker.javasrc/main/java/dev/noah/perplayerkit/commands/admin/AboutCommandListener.javasrc/main/java/dev/noah/perplayerkit/commands/admin/KitRoomCommand.javasrc/main/java/dev/noah/perplayerkit/commands/admin/PerPlayerKitCommand.javasrc/main/java/dev/noah/perplayerkit/commands/admin/SavePublicKitCommand.javasrc/main/java/dev/noah/perplayerkit/commands/core/CommandGuards.javasrc/main/java/dev/noah/perplayerkit/commands/features/RegearCommand.javasrc/main/java/dev/noah/perplayerkit/commands/inspect/AbstractInspectCommand.javasrc/main/java/dev/noah/perplayerkit/commands/inspect/InspectCommandUtil.javasrc/main/java/dev/noah/perplayerkit/commands/inspect/InspectEcCommand.javasrc/main/java/dev/noah/perplayerkit/commands/inspect/InspectKitCommand.javasrc/main/java/dev/noah/perplayerkit/commands/kits/DeleteKitCommand.javasrc/main/java/dev/noah/perplayerkit/commands/kits/EnderchestCommand.javasrc/main/java/dev/noah/perplayerkit/commands/kits/SwapKitCommand.javasrc/main/java/dev/noah/perplayerkit/commands/share/AbstractShareSlotCommand.javasrc/main/java/dev/noah/perplayerkit/commands/share/CopyKitCommand.javasrc/main/java/dev/noah/perplayerkit/commands/share/ShareECKitCommand.javasrc/main/java/dev/noah/perplayerkit/commands/share/ShareKitCommand.javasrc/main/java/dev/noah/perplayerkit/commands/shortcuts/AbstractShortSlotCommand.javasrc/main/java/dev/noah/perplayerkit/gui/GUI.javasrc/main/java/dev/noah/perplayerkit/gui/GuiMenuFactory.javasrc/main/java/dev/noah/perplayerkit/listeners/JoinListener.javasrc/main/java/dev/noah/perplayerkit/listeners/KitMenuCloseListener.javasrc/main/java/dev/noah/perplayerkit/util/BroadcastManager.javasrc/main/java/dev/noah/perplayerkit/util/DisabledCommand.javasrc/main/java/dev/noah/perplayerkit/util/Lang.javasrc/main/java/dev/noah/perplayerkit/util/PlayerUtil.javasrc/main/resources/config.ymlsrc/main/resources/lang/da.ymlsrc/main/resources/lang/de.ymlsrc/main/resources/lang/en.ymlsrc/main/resources/lang/es.ymlsrc/main/resources/lang/fi.ymlsrc/main/resources/lang/fr.ymlsrc/main/resources/lang/it.ymlsrc/main/resources/lang/nl.ymlsrc/main/resources/lang/pl.ymlsrc/main/resources/lang/pt.ymlsrc/main/resources/lang/ro.ymlsrc/main/resources/lang/sv.ymlsrc/main/resources/lang/uk.ymlsrc/main/resources/lang/zh.ymlsrc/test/java/dev/noah/perplayerkit/ConfigMigratorTest.javasrc/test/java/dev/noah/perplayerkit/commands/AbstractShareSlotCommandTest.javasrc/test/java/dev/noah/perplayerkit/commands/AbstractShortSlotCommandTest.javasrc/test/java/dev/noah/perplayerkit/util/LangTest.java
rossnoah
changed the title
rossnoah
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/main/java/dev/noah/perplayerkit/commands/features/RegearCommand.java (1)
256-276: 💤 Low value
getInventory()may returnnulldespite@NotNullannotation.The
inventoryfield is initiallynullandsetInventoryis package-private. IfgetInventory()is called by Bukkit beforecreateRegearInventorycompletes (or through reflection/other means), it would returnnullviolating the@NotNullcontract.While the current flow ensures proper initialization, defensive coding would prevent potential NPEs.
♻️ Optional defensive initialization
public static class RegearInventoryHolder implements InventoryHolder { private final Player player; - private Inventory inventory; + private Inventory inventory = Bukkit.createInventory(null, 27); public RegearInventoryHolder(Player player) { this.player = player; }Or throw a more informative exception:
`@Override` public `@NotNull` Inventory getInventory() { + if (inventory == null) { + throw new IllegalStateException("Inventory not yet initialized"); + } return inventory; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/dev/noah/perplayerkit/commands/features/RegearCommand.java` around lines 256 - 276, RegearInventoryHolder currently leaves the inventory field null which violates the `@NotNull` contract on getInventory(); fix by initializing inventory in the constructor (e.g., call Bukkit.createInventory(this, <size>, "<title>") and assign to the inventory field) so getInventory() always returns a non-null Inventory, or if you prefer defensive failure, change getInventory() to return Objects.requireNonNull(inventory, "RegearInventoryHolder.inventory not set — call createRegearInventory(player) before use") to throw a clear exception; adjust setInventory(...) and constructor of RegearInventoryHolder accordingly so the holder cannot be observed with a null inventory.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/resources/lang/pt.yml`:
- Line 153: Translate the English Ender Chest labels into Portuguese by updating
the locale keys (e.g., enderchest-editor-title) to a Portuguese string such as
"Baú Ender: {slot}" (and similarly replace the English "ender chest" text in the
other enderchest-related keys mentioned at lines 159 and 163) so all ender chest
UI labels are consistently localized in Portuguese.
---
Nitpick comments:
In `@src/main/java/dev/noah/perplayerkit/commands/features/RegearCommand.java`:
- Around line 256-276: RegearInventoryHolder currently leaves the inventory
field null which violates the `@NotNull` contract on getInventory(); fix by
initializing inventory in the constructor (e.g., call
Bukkit.createInventory(this, <size>, "<title>") and assign to the inventory
field) so getInventory() always returns a non-null Inventory, or if you prefer
defensive failure, change getInventory() to return
Objects.requireNonNull(inventory, "RegearInventoryHolder.inventory not set —
call createRegearInventory(player) before use") to throw a clear exception;
adjust setInventory(...) and constructor of RegearInventoryHolder accordingly so
the holder cannot be observed with a null inventory.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 678eec83-3dda-43fe-b682-8cbfee08b80a
📒 Files selected for processing (20)
src/main/java/dev/noah/perplayerkit/ConfigMigrator.javasrc/main/java/dev/noah/perplayerkit/KitManager.javasrc/main/java/dev/noah/perplayerkit/commands/features/RegearCommand.javasrc/main/java/dev/noah/perplayerkit/gui/GUI.javasrc/main/java/dev/noah/perplayerkit/listeners/KitMenuCloseListener.javasrc/main/java/dev/noah/perplayerkit/util/Lang.javasrc/main/resources/lang/da.ymlsrc/main/resources/lang/de.ymlsrc/main/resources/lang/en.ymlsrc/main/resources/lang/es.ymlsrc/main/resources/lang/fi.ymlsrc/main/resources/lang/fr.ymlsrc/main/resources/lang/it.ymlsrc/main/resources/lang/nl.ymlsrc/main/resources/lang/pl.ymlsrc/main/resources/lang/pt.ymlsrc/main/resources/lang/ro.ymlsrc/main/resources/lang/sv.ymlsrc/main/resources/lang/uk.ymlsrc/main/resources/lang/zh.yml
✅ Files skipped from review due to trivial changes (6)
- src/main/resources/lang/pl.yml
- src/main/resources/lang/nl.yml
- src/main/resources/lang/en.yml
- src/main/resources/lang/it.yml
- src/main/resources/lang/fr.yml
- src/main/resources/lang/fi.yml
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Chores
Tests