Conversation
created Team Settings Inventory Listener; createed Teams Main Menu GUI; created Team Settings CMD
minor fixes
…fter a change; implemented that a deleted team can not be opened
change usage of player to playerData or offlinePlayer where possible;
…team-system # Conflicts: # src/main/java/de/j/stationofdoom/teams/enderchest/TeamEnderchestPreventEnchantedItemsInput.java
…m members of other teams
change TeamsMainMenuGUI usage to get teams and so on to new class HandleTeams
send team members a message when a new location is claimed
also show the location when claiming it
hinder player from joining a locked team
WalkthroughThis update introduces a comprehensive team system to the plugin, enabling players to create, manage, and interact with teams. Key features include team creation, color and name customization, operator roles, a shared team ender chest, location protection with claim radius, and a rank-based permission system. The implementation adds new commands, GUIs, database tables, and event listeners for team management, chunk claiming, and inventory interactions. The configuration and translation systems are extended to support these features. The codebase now includes robust persistence for teams and inventories, enhanced player data handling, and dynamic UI updates for team-related actions. Changes
Sequence Diagram(s)sequenceDiagram
participant Player
participant TeamsMainMenuGUI
participant TeamSettingsGUI
participant HandleTeams
participant TeamsDatabase
participant TeamEnderchestsDatabase
participant ChunkClaimSystem
Player->>TeamsMainMenuGUI: /teams command
TeamsMainMenuGUI->>TeamsDatabase: getTeams()
TeamsDatabase-->>TeamsMainMenuGUI: List<Team>
TeamsMainMenuGUI-->>Player: Show teams menu
Player->>TeamSettingsGUI: Select team
TeamSettingsGUI->>HandleTeams: getTeam(PlayerData)
HandleTeams-->>TeamSettingsGUI: Team
TeamSettingsGUI-->>Player: Show team settings
Player->>TeamSettingsGUI: Rename team / change color / manage members
TeamSettingsGUI->>TeamsDatabase: updateTeam/Player
TeamsDatabase-->>TeamSettingsGUI: Ack
Player->>TeamSettingsGUI: Access enderchest
TeamSettingsGUI->>TeamEnderchestsDatabase: getTeamEnderchest(UUID)
TeamEnderchestsDatabase-->>TeamSettingsGUI: Inventory
TeamSettingsGUI-->>Player: Open enderchest
Player->>ChunkClaimSystem: Claim location
ChunkClaimSystem->>HandleTeams: getTeam(Player)
HandleTeams-->>ChunkClaimSystem: Team
ChunkClaimSystem->>TeamsDatabase: updateTeam(Team)
TeamsDatabase-->>ChunkClaimSystem: Ack
ChunkClaimSystem-->>Player: Show particles, send confirmation
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 38
🔭 Outside diff range comments (1)
src/main/java/de/j/stationofdoom/main/Main.java (1)
109-110:⚠️ Potential issueDuplicate event listener registration detected.
There is a duplicate registration of the PlayerSitListener, which should be removed.
pluginManager.registerEvents(new PlayerSitListener(), this); -pluginManager.registerEvents(new PlayerSitListener(), this);
♻️ Duplicate comments (1)
src/main/java/de/j/deathMinigames/database/TeamEnderchestsDatabase.java (1)
34-39: DDL again executed viainsert()Same remark as for
TeamsDatabase#createTable: use.execute()(or
.update()) for DDL to avoid undefined behaviour.
🧹 Nitpick comments (32)
docker-compose.yml (2)
5-5: Caution: Standard PostgreSQL port might cause conflictsYou've changed the port mapping from a custom port to the standard PostgreSQL port 5432. While this is more conventional, be aware it could potentially conflict with other PostgreSQL instances running on the host machine.
Consider documenting this change and potential conflict in your installation instructions. Alternatively, you could keep using a non-standard port to avoid conflicts.
11-13: Add a newline at the end of the fileThe file is missing a newline at the end, which is a standard practice for text files.
- POSTGRES_DB=stationofdoom - POSTGRES_APPLICATION_NAME=stationofdoom - POSTGRES_SCHEMA=public +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 13-13: no new line character at the end of file
(new-line-at-end-of-file)
README.md (1)
89-100: Documentation is clear but missing a commaThe team system documentation is well-structured and provides a clear overview of the feature. However, there's a minor grammatical issue in line 92.
- Another feature of this plugin is its team system, which lets players create teams, to access it enter the command `/teams`. The teams are saved in the database if one is connected + Another feature of this plugin is its team system, which lets players create teams. To access it, enter the command `/teams`. The teams are saved in the database if one is connected.Also consider adding a period at the end of the sentence.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~92-~92: A comma might be missing here.
Context: ...ch lets players create teams, to access it enter the command/teams. The teams a...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/BlockBreakAndUseCancelListener.java (2)
13-28: Reduce redundantChunkClaimSystem.getInstance()calls.The listener logic is solid, but there are multiple redundant calls to get the singleton instance.
@EventHandler public void onPlayerInteract(PlayerInteractEvent event) { Player player = event.getPlayer(); if(player == null || event.getClickedBlock() == null) return; ChunkClaimSystem chunkClaimSystem = ChunkClaimSystem.getInstance(); if(chunkClaimSystem.checkIfLocationProtectedFromPlayer(event.getClickedBlock().getX(), event.getClickedBlock().getZ(), player)) { - Team team = ChunkClaimSystem.getInstance().getTeam(event.getClickedBlock().getLocation()); + Team team = chunkClaimSystem.getTeam(event.getClickedBlock().getLocation()); if(team == null) { Main.getMainLogger().warning("Team is null when checking if location is protected"); } player.sendMessage(Component.text(new TranslationFactory().getTranslation(player, "chunkClaimedByDifferentTeam", team.getName())).color(NamedTextColor.RED)); event.setCancelled(true); - ChunkClaimSystem.getInstance().showPlayerProtectedLocationViaParticles(player, team.getProtectedLocation()); + chunkClaimSystem.showPlayerProtectedLocationViaParticles(player, team.getProtectedLocation()); } }
24-24: Cache TranslationFactory instance for better performance.Creating a new TranslationFactory for each message is inefficient, especially in frequently triggered event handlers.
-player.sendMessage(Component.text(new TranslationFactory().getTranslation(player, "chunkClaimedByDifferentTeam", team.getName())).color(NamedTextColor.RED)); +private static final TranslationFactory TRANSLATION_FACTORY = new TranslationFactory(); // Add the field at class level, then use: +player.sendMessage(Component.text(TRANSLATION_FACTORY.getTranslation(player, "chunkClaimedByDifferentTeam", team.getName())).color(NamedTextColor.RED));src/main/java/de/j/stationofdoom/teams/TeamsCMD.java (2)
11-12: Make TeamsMainMenuGUI field private.Public fields can lead to unexpected modifications from other classes.
-public TeamsMainMenuGUI teamsMainMenuGUI = new TeamsMainMenuGUI(); +private final TeamsMainMenuGUI teamsMainMenuGUI = new TeamsMainMenuGUI();
18-29: Add help message for unknown arguments.The command doesn't provide guidance when users enter unknown arguments.
@Override public void execute(CommandSourceStack stack, String[] args) { Player player = (Player) stack.getSender(); if(args.length == 0) { teamsMainMenuGUI.showPage(1, player); } else if(args.length == 1 && (args[0].equalsIgnoreCase("e") || args[0].equalsIgnoreCase("enderchest"))) { Team team = HandleTeams.getTeam(HandlePlayers.getInstance().getPlayerData(player.getUniqueId())); if(team == null || !team.isMember(player.getUniqueId())) return; team.accessEnderChest(player); } + else { + player.sendMessage(Component.text(new TranslationFactory().getTranslation(player, "teamCommandUsage")).color(NamedTextColor.YELLOW)); + } }src/main/java/de/j/stationofdoom/teams/enderchest/TeamEnderchestPreventEnchantedItemsInputListener.java (2)
19-20: Add safe cast check.The direct casting of
event.getView().getPlayer()toPlayercould be unsafe in some contexts.-Player player = (Player) event.getView().getPlayer(); -if(player == null) return; +if(!(event.getView().getPlayer() instanceof Player player)) return;
21-21: Cache TranslationFactory for better performance.Similar to the previous listener, creating a new TranslationFactory instance for each message is inefficient.
+private static final TranslationFactory TRANSLATION_FACTORY = new TranslationFactory(); // Add the field at class level, then use: -player.sendMessage(Component.text(new TranslationFactory().getTranslation(player, "teamEnderchestPreventEnchantedItemsInput")).color(NamedTextColor.RED)); +player.sendMessage(Component.text(TRANSLATION_FACTORY.getTranslation(player, "teamEnderchestPreventEnchantedItemsInput")).color(NamedTextColor.RED));src/main/java/de/j/deathMinigames/main/HandlePlayers.java (1)
149-159: Add JavaDoc documentation for the new method.This method is important for the team system functionality but lacks documentation explaining its purpose and behavior.
+/** + * Retrieves player data either from the database (if connected) or from the local cache. + * If the player is not found locally and is online, adds them to the known players. + * + * @param uuidOfPlayer The UUID of the player whose data to retrieve + * @return The player's data, or null if the player cannot be found and is offline + */ public PlayerData getPlayerData(UUID uuidOfPlayer) {src/main/java/de/j/deathMinigames/settings/AnvilUI.java (1)
78-85: Consider simplifying the team name retrieval logic.The current implementation contains nested conditionals and could be simplified for better readability.
- case TEAM_RENAME: - if(HandleTeams.getTeam(HandlePlayers.getInstance().getPlayerData(player.getUniqueId())).getName() != null) { - inputItemName = HandleTeams.getTeam(HandlePlayers.getInstance().getPlayerData(player.getUniqueId())).getName(); - } - else { - inputItemName = ""; - } - break; + case TEAM_RENAME: + Team playerTeam = HandleTeams.getTeam(HandlePlayers.getInstance().getPlayerData(player.getUniqueId())); + inputItemName = playerTeam.getName() != null ? playerTeam.getName() : ""; + break;src/main/java/de/j/deathMinigames/settings/MainMenu.java (1)
88-90: Consider adding synchronized keyword for consistency.The getSetClaimingRadius() method should be synchronized for consistency with other getter methods in the class.
- public static AnvilUI getSetClaimingRadius() { + public synchronized static AnvilUI getSetClaimingRadius() { return setClaimingRadius; }src/main/java/de/j/stationofdoom/teams/TeamCMD.java (1)
14-16: Unused field detected.The teamsMainMenuGUI field is instantiated but never used in this class.
public class TeamCMD implements BasicCommand { - public TeamsMainMenuGUI teamsMainMenuGUI = new TeamsMainMenuGUI();src/main/java/de/j/stationofdoom/teams/TeamsMainMenuGUI.java (1)
59-68: Optimize team inventory filling methodThe early return logic in
addTeamsToInventoryis a bit complex with multiple conditions. Consider simplifying this for better readability.private void addTeamsToInventory(int intToStartFrom) { invSlots.clear(); for(int teamsAdded = 0; teamsAdded < maxSlotsPerPage; teamsAdded++) { - if(teamsAdded >= teamQuantity || teamsAdded >= maxSlotsPerPage || intToStartFrom + teamsAdded >= teamQuantity) return; + // Check if we've reached the end of available teams + int teamIndex = intToStartFrom + teamsAdded; + if(teamIndex >= teamQuantity) return; + - Team currentTeam = TeamsMainMenuGUI.teams.get(intToStartFrom + teamsAdded); + Team currentTeam = TeamsMainMenuGUI.teams.get(teamIndex); ArrayList<String> lore = getTeamInformation(currentTeam); addClickableItemStack(currentTeam.getName(), currentTeam.getColorAsMaterial(), 1, teamsAdded, lore); invSlots.put(teamsAdded, currentTeam); } }src/main/java/de/j/stationofdoom/teams/TeamInventoryReload.java (1)
13-13: Make reload interval configurableConsider making the reload interval configurable through a config file rather than hardcoding it. This would allow server administrators to adjust it based on server performance needs.
- private static final int secondsBetweenReloads = 3; + private static final int secondsBetweenReloads = Main.getPlugin().getConfig().getInt("teams.inventoryReloadInterval", 3);src/main/java/de/j/stationofdoom/teams/TeamSettingsGUI.java (2)
36-66: Handle team deletion gracefullyThe handling of deleted teams is good - showing an error message and redirecting to the main menu. However, consider adding logging when this happens to help with debugging.
if(this.team.isDeleted()) { playerToShowTheInvTo.sendMessage(Component.text(tf.getTranslation(playerToShowTheInvTo, "teamDeleted", team.getName())).color(NamedTextColor.RED)); + Main.getMainLogger().warning("Player " + playerToShowTheInvTo.getName() + " attempted to view deleted team: " + team.getName()); new TeamsMainMenuGUI().showPage(1, playerToShowTheInvTo); return; }
116-139: Use constants for inventory slot positionsThe inventory slot positions are hardcoded throughout the method. Consider using constants for these positions to make the code more maintainable.
public class TeamSettingsGUI extends GUI { + // Slot constants for team function buttons + private static final int SLOT_RENAME = 9; + private static final int SLOT_COLOR = 10; + private static final int SLOT_LOCK = 11; + private static final int SLOT_ENDERCHEST = 12; + private static final int SLOT_CLAIM = 13; + private static final int SLOT_DELETE = 17; // Existing code... private void addFunctionsToInventory(Player player) { - addClickableItemStack(tf.getTranslation(player, "renameTeam"), Material.NAME_TAG, 1, 9); - addClickableItemStack(tf.getTranslation(player, "changeColor"), Material.ORANGE_GLAZED_TERRACOTTA, 1, 10); + addClickableItemStack(tf.getTranslation(player, "renameTeam"), Material.NAME_TAG, 1, SLOT_RENAME); + addClickableItemStack(tf.getTranslation(player, "changeColor"), Material.ORANGE_GLAZED_TERRACOTTA, 1, SLOT_COLOR); // And so on for other slots... }src/main/java/de/j/stationofdoom/teams/HandleTeams.java (1)
84-86: Expose an immutable view of internal state
getAllTeams()hands out the live list. External code can add / remove elements bypassing the validation inaddTeam, breaking invariants.- public List<Team> getAllTeams() { - return teams; + public List<Team> getAllTeams() { + return Collections.unmodifiableList(teams); }src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java (1)
50-53: Column name casing is inconsistentTable definition uses
UUID(uppercase) but queries select/insert usinguuid(lowercase). MySQL is case‑insensitive, but PostgreSQL (and some JDBC drivers) are not. Standardise on one casing to keep cross‑DB support.Also applies to: 71-72, 145-146
src/main/java/de/j/stationofdoom/teams/TeamPlayerSettingsGUI.java (1)
13-16: Minor: mark helpersfinal& remove unnecessaryvolatile
TranslationFactory,playerData, andteamare always accessed on the Bukkit main thread.volatileadds overhead without benefit here;finalcommunicates immutability.-private TranslationFactory tf = new TranslationFactory(); -private volatile PlayerData playerData; -private volatile Team team; +private final TranslationFactory tf = new TranslationFactory(); +private PlayerData playerData; +private Team team;src/main/java/de/j/deathMinigames/database/TeamsDatabase.java (1)
86-115: Duplicate team inserts & missing UNIQUE constraint
insertTeamwill insert a new row every time the plugin shuts down even if the
team already existed. Over time the table will accumulate duplicates.
- Add
PRIMARY KEY (uuid)or aUNIQUEconstraint to the table.- Use
INSERT … ON CONFLICT (uuid) DO UPDATEor library equivalent.src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/ChunkClaimSystem.java (2)
174-195: Division by zero & floating‑point precision in vector normalisation
changeVector = vector.clone().multiply(1 / vector.length());
- If both locations are identical
vector.length()is0→ division by zero.- Re‑calculating
vector.length()every tick inside the runnable is
unnecessary.double length = vector.length(); if (length == 0) return; // nothing to draw Vector changeVector = vector.clone().multiply(1.0 / length);
49-64:checkIfLocationProtectedFromPlayerrebuilds map on every callFor every protection check you rebuild
HashMap<Location,Team>resulting in
O(n)complexity per block interaction.Cache the map once per tick or maintain it incrementally when teams change.
src/main/java/de/j/deathMinigames/main/PlayerData.java (1)
31-37: StalePlayerinstance
getPlayer()caches thePlayerreference forever; after a disconnect the object becomes invalid and many Paper calls will throw.
Consider resolving fresh withBukkit.getPlayer(uuid)each call or at least whenisOnline()returns false.src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java (2)
214-233: Color‑changer GUI fills duplicates every click
addColorsToColorChangerappends all concrete blocks each time the color changer is opened, creating duplicate stacks and wasting memory.Guard with
if (colorChanger.firstEmpty() == -1) return;or populate once during GUI construction.
110-132: Complex permission condition is hard to readThe long
if(slot >= 9 && slot <=13 ... )mixes precedence of&&and||. Extract booleans or add parentheses to avoid subtle bugs.boolean isEditableSlot = (slot >= 9 && slot <= 13) || slot == 17 || (slot >= 18 && slot <= 44 && inv.getItem(slot) != null); if(isEditableSlot){ ... }src/main/java/de/j/stationofdoom/util/Tablist.java (1)
42-62: Consider refactoring the scoreboard creation logicThe
setScoreboard()method recreates all teams on each call, which could be inefficient. Consider implementing a strategy to update only what's needed.Consider implementing a more selective update approach that only recreates teams when team data changes rather than on every call. You could track when teams are added or modified and only update the affected scoreboard teams.
src/main/java/de/j/stationofdoom/teams/Team.java (5)
253-255: Simplify redundant condition check in isTeamOperatorThe method checks if the player is a member twice.
public boolean isTeamOperator(PlayerData playerData) { if(!this.members.containsKey(playerData.getUniqueId())) return false; - return this.members.containsKey(playerData.getUniqueId()) && this.members.get(playerData.getUniqueId()); + return this.members.get(playerData.getUniqueId()); }
262-273: Unnecessary OfflinePlayer retrieval in isMemberThe method retrieves an OfflinePlayer object but never uses it, which is inefficient.
public boolean isMember(UUID uuidOfPlayerTOCheck) { if (uuidOfPlayerTOCheck == null) { return false; } - OfflinePlayer offlinePlayer = Bukkit.getOfflinePlayer(uuidOfPlayerTOCheck); - for (UUID uuid : this.members.keySet()) { - if(uuid.equals(uuidOfPlayerTOCheck)) { - return true; - } - } - return false; + return this.members.containsKey(uuidOfPlayerTOCheck); }
140-146: Improve error handling in setLocked methodThe method simply logs a message and returns if there are no operators, but it would be more helpful to throw an exception or return a status.
public void setLocked(boolean locked) { if(getTeamOperators().isEmpty()) { - Main.getMainLogger().info("Team is not locked because there are no operators"); - return; + Main.getMainLogger().warning("Cannot " + (locked ? "lock" : "unlock") + " team " + name + " because there are no operators"); + throw new IllegalStateException("Cannot change lock state for team without operators"); } this.locked = locked; }
171-179: Improve efficiency in setRandomConcreteMaterialThe method creates a new list and scans all Material values on each call, which is inefficient.
+private static final List<Material> CONCRETE_MATERIALS = new ArrayList<>(); + +static { + for (Material material : Material.values()) { + if (material.name().contains("CONCRETE") && !material.name().contains("POWDER")) { + CONCRETE_MATERIALS.add(material); + } + } +} private void setRandomConcreteMaterial() { - List<Material> materials = new ArrayList<>(); - for (Material material : Material.values()) { - if (material.name().contains("CONCRETE") && !material.name().contains("POWDER")) { - materials.add(material); - } - } - this.colorAsMaterial = materials.get(ThreadLocalRandom.current().nextInt(0, materials.size())); + this.colorAsMaterial = CONCRETE_MATERIALS.get(ThreadLocalRandom.current().nextInt(0, CONCRETE_MATERIALS.size())); + convertMaterialToString(this.colorAsMaterial); }
29-33: Consider thread safety for shared team dataThe class uses
volatilefor the inventory field, but other shared data like members and protected location aren't thread-safe.Consider using more comprehensive thread safety mechanisms for all shared data in the Team class. You're already using volatile for the inventory field, but you should extend thread safety to other fields like members and protectedLocation that might be accessed concurrently.
Options include:
- Using synchronized blocks around methods that modify shared state
- Using concurrent collections like ConcurrentHashMap for members
- Using AtomicReference for fields like protectedLocation
This is especially important if team operations can happen across multiple threads.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (31)
README.md(1 hunks)docker-compose.yml(1 hunks)src/main/java/de/j/deathMinigames/database/Database.java(1 hunks)src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java(6 hunks)src/main/java/de/j/deathMinigames/database/TeamEnderchestsDatabase.java(1 hunks)src/main/java/de/j/deathMinigames/database/TeamsDatabase.java(1 hunks)src/main/java/de/j/deathMinigames/listeners/AnvilListener.java(5 hunks)src/main/java/de/j/deathMinigames/listeners/InventoryListener.java(1 hunks)src/main/java/de/j/deathMinigames/listeners/JoinListener.java(1 hunks)src/main/java/de/j/deathMinigames/main/Config.java(3 hunks)src/main/java/de/j/deathMinigames/main/HandlePlayers.java(4 hunks)src/main/java/de/j/deathMinigames/main/PlayerData.java(7 hunks)src/main/java/de/j/deathMinigames/settings/AnvilUI.java(2 hunks)src/main/java/de/j/deathMinigames/settings/GUI.java(4 hunks)src/main/java/de/j/deathMinigames/settings/MainMenu.java(4 hunks)src/main/java/de/j/stationofdoom/main/Main.java(5 hunks)src/main/java/de/j/stationofdoom/teams/HandleTeams.java(1 hunks)src/main/java/de/j/stationofdoom/teams/Team.java(1 hunks)src/main/java/de/j/stationofdoom/teams/TeamCMD.java(1 hunks)src/main/java/de/j/stationofdoom/teams/TeamInventoryReload.java(1 hunks)src/main/java/de/j/stationofdoom/teams/TeamPlayerSettingsGUI.java(1 hunks)src/main/java/de/j/stationofdoom/teams/TeamSettingsGUI.java(1 hunks)src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java(1 hunks)src/main/java/de/j/stationofdoom/teams/TeamsCMD.java(1 hunks)src/main/java/de/j/stationofdoom/teams/TeamsMainMenuGUI.java(1 hunks)src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/BlockBreakAndUseCancelListener.java(1 hunks)src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/ChunkClaimSystem.java(1 hunks)src/main/java/de/j/stationofdoom/teams/enderchest/EnderchestInvHolder.java(1 hunks)src/main/java/de/j/stationofdoom/teams/enderchest/TeamEnderchestPreventEnchantedItemsInputListener.java(1 hunks)src/main/java/de/j/stationofdoom/util/Tablist.java(2 hunks)src/main/resources/translations.json(4 hunks)
🧰 Additional context used
🧠 Learnings (3)
src/main/java/de/j/deathMinigames/main/HandlePlayers.java (2)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#176
File: src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java:88-110
Timestamp: 2024-12-09T16:43:34.543Z
Learning: In `src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java`, exceptions from database operations (e.g., in methods like `updatePlayerDataDatabase`) are already handled by the database utility tool in use, so explicit exception handling in these methods is unnecessary.
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#176
File: src/main/java/de/j/deathMinigames/deathMinigames/Main.java:65-71
Timestamp: 2024-11-04T20:14:53.574Z
Learning: In the `Main.java` file of the Minecraft plugin, to prevent multithreading issues, objects such as `JumpAndRun`, `Minigame`, `Introduction`, `Config`, and `TranslationFactory` are instantiated within methods like `minigameStart` instead of being class-level fields.
src/main/java/de/j/stationofdoom/main/Main.java (1)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#176
File: src/main/java/de/j/deathMinigames/dmUtil/TestCMD.java:23-27
Timestamp: 2024-12-05T07:34:55.227Z
Learning: In `src/main/java/de/j/deathMinigames/dmUtil/TestCMD.java`, the `TestCMD` command is intended to provide players with database connection information, including connection details, to help users who cannot see the code check if and where the database is connected.
src/main/java/de/j/deathMinigames/settings/GUI.java (1)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#176
File: src/main/java/de/j/deathMinigames/settings/GUI.java:26-26
Timestamp: 2024-11-10T19:55:42.061Z
Learning: In the `GUI` class in `src/main/java/de/j/deathMinigames/settings/GUI.java`, the GUI instances are static and the `InventoryListener` is not, so using the same instance for every GUI is not possible.
🧬 Code Graph Analysis (7)
src/main/java/de/j/deathMinigames/listeners/InventoryListener.java (1)
src/main/java/de/j/deathMinigames/settings/MainMenu.java (1)
MainMenu(16-328)
src/main/java/de/j/deathMinigames/listeners/JoinListener.java (1)
src/main/java/de/j/deathMinigames/main/HandlePlayers.java (1)
HandlePlayers(12-160)
src/main/java/de/j/stationofdoom/teams/TeamsCMD.java (2)
src/main/java/de/j/deathMinigames/main/HandlePlayers.java (1)
HandlePlayers(12-160)src/main/java/de/j/stationofdoom/teams/HandleTeams.java (1)
HandleTeams(11-87)
src/main/java/de/j/deathMinigames/settings/AnvilUI.java (4)
src/main/java/de/j/deathMinigames/main/HandlePlayers.java (1)
HandlePlayers(12-160)src/main/java/de/j/stationofdoom/teams/HandleTeams.java (1)
HandleTeams(11-87)src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/ChunkClaimSystem.java (1)
ChunkClaimSystem(19-196)src/main/java/de/j/stationofdoom/util/Tablist.java (1)
Tablist(20-356)
src/main/java/de/j/deathMinigames/main/HandlePlayers.java (3)
src/main/java/de/j/deathMinigames/database/Database.java (1)
Database(14-143)src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java (1)
PlayerDataDatabase(16-190)src/main/java/de/j/stationofdoom/main/Main.java (1)
Main(44-172)
src/main/java/de/j/deathMinigames/settings/GUI.java (1)
src/main/java/de/j/deathMinigames/main/HandlePlayers.java (1)
HandlePlayers(12-160)
src/main/java/de/j/deathMinigames/database/TeamsDatabase.java (3)
src/main/java/de/j/stationofdoom/main/Main.java (1)
Main(44-172)src/main/java/de/j/stationofdoom/teams/HandleTeams.java (1)
HandleTeams(11-87)src/main/java/de/j/stationofdoom/teams/Team.java (1)
Team(24-293)
🪛 YAMLlint (1.35.1)
docker-compose.yml
[error] 13-13: no new line character at the end of file
(new-line-at-end-of-file)
🪛 LanguageTool
README.md
[uncategorized] ~92-~92: A comma might be missing here.
Context: ...ch lets players create teams, to access it enter the command /teams. The teams a...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🔇 Additional comments (36)
src/main/java/de/j/deathMinigames/listeners/InventoryListener.java (1)
259-261: LGTM: Menu integration for claiming radius settingThe implementation correctly integrates with the MainMenu's claiming radius configuration UI, which is part of the new team system.
This change sensibly extends the main menu with team-related functionality while maintaining the existing pattern of menu navigation.
src/main/java/de/j/deathMinigames/listeners/JoinListener.java (1)
40-40: Improved player data retrieval with abstractionThe change appropriately uses the singleton instance method to get player data, which properly abstracts database access and aligns with the centralized data management approach.
This change enhances maintainability by using the proper accessor that handles both in-memory and database-backed player data retrieval.
src/main/java/de/j/deathMinigames/database/Database.java (1)
80-81: Team database tables integration looks good.The addition of team-related database table initialization is properly integrated into the existing database setup flow.
src/main/java/de/j/deathMinigames/main/Config.java (3)
3-3: New import for ChunkClaimSystem is appropriate.This import is used to reference the ChunkClaimSystem singleton instance in the updated configuration handling.
127-129: Correctly integrates ChunkClaimSystem with config values.The implementation properly checks if the configuration contains the key before attempting to read it, following the pattern used for other configuration values.
233-238: Well-implemented synchronized configuration method.The method includes proper early-return optimization for unchanged values, maintains thread safety with synchronization, and follows the class's established pattern for config modification methods.
src/main/resources/translations.json (3)
68-68: Fixed formatting in German loseMessage.The message formatting has been corrected, ensuring consistent presentation of the death location and difficulty reduction instructions.
188-188: Fixed formatting in English loseMessage.Similar to the German version, the English message formatting has been properly corrected.
87-239: Comprehensive translation keys for the new team system.The translations are well-structured and complete for both German and English locales, covering all aspects of the team system including:
- Team creation and management
- Member roles and permissions
- Team enderchest functionality
- Chunk claiming and protection
- Error messages and notifications
This provides good internationalization support for the new features.
src/main/java/de/j/deathMinigames/main/HandlePlayers.java (3)
3-7: Updated imports to support database integration.The imports have been appropriately updated to include Database and Bukkit classes needed for the enhanced player data management.
61-61: Method name standardization for UUID access.Changed from
getUUID()togetUniqueId(), which is the standard method name in Bukkit's API, improving consistency.
91-91: Immediate database persistence for new players.Adding players to the database immediately after adding them to the local map ensures data consistency between memory and persistent storage.
src/main/java/de/j/deathMinigames/settings/AnvilUI.java (2)
3-5: Appropriate imports added for team system integration.The imports correctly include the necessary dependencies for team functionality and chunk claim system integration.
86-88: Good implementation of claim radius input.The code correctly retrieves the protected location size from the ChunkClaimSystem singleton and converts it to a string.
src/main/java/de/j/deathMinigames/settings/MainMenu.java (3)
35-36: Appropriate enum values added for team system UI integration.The new TEAM_RENAME and SET_CLAIMING_RADIUS enum values are well-integrated with the existing AnvilUIs enum.
103-103: Appropriate initialization of the claim radius setting UI.The setClaimingRadius instance is properly initialized with the SET_CLAIMING_RADIUS enum value.
137-137: Good integration of claim radius setting in the main menu.The added clickable item for SetClaimingRadius with Material.MAP is well-placed in the menu at slot 6.
src/main/java/de/j/stationofdoom/main/Main.java (5)
5-10: Appropriate imports for team system components.The imports correctly include team command classes, listeners, and database access classes needed for the team system.
20-22: Good imports for team-related event handlers.The additional imports for team inventory management and team commands are appropriate.
98-99: Well-documented team command registration.The registration of the 'teams' and 'team' commands with descriptive help text is well-implemented.
125-128: Proper event listener registration for team system.All necessary team-related listeners are registered with the plugin manager.
143-144: Good database persistence for team data.The addition of team database updates during plugin disable ensures team data persistence.
src/main/java/de/j/stationofdoom/teams/TeamCMD.java (3)
1-13: Good package structure and imports for team command.The package structure and imports are appropriate for the team command functionality.
17-20: Good permission check for command.The canUse method correctly restricts the command to player entities only.
22-31: Well-implemented command execution with team validation.The execute method properly validates if the player is in a team before showing the team settings GUI. The error message is appropriately localized.
src/main/java/de/j/deathMinigames/settings/GUI.java (7)
24-25: Improved field visibility for subclass accessThe changes from private to protected for the
inventoryfield allow subclasses to directly access and manipulate the inventory, which is essential for the team GUI implementations. This is a good architectural change that supports extensibility.
27-30: Default constructor implementation looks goodThis new no-arg constructor creates a standard 54-slot inventory, which is a common size in Minecraft (6 rows). This will simplify subclass implementations that don't need custom sizing.
49-63: Flexible inventory sizing constructor addedThis constructor enhances flexibility by allowing subclasses to specify inventory size. It duplicates some code from the original constructor but maintains the same functionality with size customization.
74-74: Improved null checking for player dataThe updated null check prevents potential NullPointerExceptions by verifying both the PlayerData object and its UUID before proceeding.
77-77: Updated player head profile setting methodThe code now uses
setOwnerProfilewithBukkit.createProfileinstead of the previous implementation. This is aligned with the renamed method in PlayerData (getUniqueId()instead ofgetUUID()).
85-94: Useful addition of single player head with lore methodThis new method is a valuable addition that allows adding a single player head with custom lore to a specific slot. This will be particularly useful for the team GUIs that need to display team members with their status information.
148-148: Added explicit 'this' reference for clarityUsing
this.inventoryinstead of justinventoryimproves code clarity by explicitly referencing the class field.src/main/java/de/j/stationofdoom/teams/TeamInventoryReload.java (1)
15-23: Good event handling for inventory openingThe event handler correctly identifies team-related GUIs and initiates the reload timer. The class casting approach is efficient.
src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java (1)
81-96:Details
❓ Verification inconclusive
getPlayerData(UUID)may throw when player not present
.all().getFirst()assumes the query always returns a row; an empty result triggersNoSuchElementException.- .all() - .getFirst(); + .all() + .stream() + .findFirst() + .orElse(null);Return
null(orOptional<PlayerData>) to let callers handle “not in DB” gracefully.
🏁 Script executed:
#!/bin/bash # Locate Query class file QUERY_FILE=$(rg -l "class Query" src/main/java | grep -m1 "Query.java") echo "Query class file: $QUERY_FILE" # Show the beginning of the Query class sed -n '1,200p' "$QUERY_FILE" # Inspect the 'all()' method signature/implementation rg -n "all(" "$QUERY_FILE" # Inspect the 'getFirst' method signature/implementation rg -n "getFirst" "$QUERY_FILE"Length of output: 321
🏁 Script executed:
#!/bin/bash set -e # Locate PlayerDataDatabase.java PDB_FILE=$(rg -l "class PlayerDataDatabase" -g "*.java" src) echo "PlayerDataDatabase file: $PDB_FILE" # Show its imports and Query usage (first 50 lines) echo "----- imports and class declaration -----" sed -n '1,50p' "$PDB_FILE" # Show the specific Query.query import line if present echo "----- Query import line -----" grep -n "import .*Query" "$PDB_FILE" || echo "No explicit import of Query found" # Determine Query class path from package structure QUERY_IMPORT=$(grep "import .*Query" "$PDB_FILE" | sed 's/import \(.*\);/\1/') echo "Queried import: $QUERY_IMPORT" if [ -n "$QUERY_IMPORT" ]; then QUERY_PATH="src/$(echo "$QUERY_IMPORT" | sed 's/\./\//g').java" echo "Guessing Query file at: $QUERY_PATH" if [ -f "$QUERY_PATH" ]; then echo "----- Start of Query.java -----" sed -n '1,200p' "$QUERY_PATH" echo "----- Searching for all( and getFirst in Query.java -----" grep -n "all(" "$QUERY_PATH" || echo "No all( found" grep -n "getFirst" "$QUERY_PATH" || echo "No getFirst found" else echo "Query.java not found at guessed path" fi else echo "Skipping Query class inspection due to missing import" fiLength of output: 3143
Verify
getFirst()behavior before handling empty results
- In
src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java(lines 81–96), calling
.all().getFirst()will throw aNoSuchElementExceptionif the query returns no rows.- Consider replacing with:
- .all() - .getFirst(); + .all() + .stream() + .findFirst() + .orElse(null);
- Or change the method signature to
Optional<PlayerData>and return
.all().stream().findFirst()directly.Please verify how
getFirst()is implemented in yourde.chojo.sadu.querieslibrary and adjust the code so that “player not in DB” is handled gracefully.src/main/java/de/j/deathMinigames/listeners/AnvilListener.java (1)
32-36: 🛠️ Refactor suggestion
⚠️ Potential issue❗ Instance fields cause cross‑player data leaks
serverName,hostName,teamNameandclaimingRadiuslive as ordinary fields in a listener that is shared by every player.
When two players interact with an anvil in parallel the last writer wins – e.g. Alice’steamNamemight be applied to Bob’s team.Use local variables (or metadata attached to the
AnvilView/Player) rather than mutable listener state.- private String teamName; - private int claimingRadius; + // remove – keep data per‑interaction instead⛔ Skipped due to learnings
Learnt from: PrinzPizza42 PR: AtomPilz-DevTeam/StationofDoomPlugin#196 File: src/main/java/de/j/deathMinigames/listeners/AnvilListener.java:27-0 Timestamp: 2025-01-06T19:05:17.436Z Learning: In the `AnvilListener` class (src/main/java/de/j/deathMinigames/listeners/AnvilListener.java), concurrency concerns for storing the `serverName` or `hostName` instance fields are considered non-problematic because only operators can access the relevant menus. Overwriting these fields does not impact the experience negatively.src/main/java/de/j/deathMinigames/main/PlayerData.java (1)
124-131:isConnected()may not exist on all serversPaper provides
Player#isConnected(); Spigot/Bukkit only hasisOnline(). If you aim for broader compatibility preferisOnline()or feature‑detect the method.
src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/BlockBreakAndUseCancelListener.java
Show resolved
Hide resolved
...va/de/j/stationofdoom/teams/enderchest/TeamEnderchestPreventEnchantedItemsInputListener.java
Show resolved
Hide resolved
...va/de/j/stationofdoom/teams/enderchest/TeamEnderchestPreventEnchantedItemsInputListener.java
Show resolved
Hide resolved
...va/de/j/stationofdoom/teams/enderchest/TeamEnderchestPreventEnchantedItemsInputListener.java
Show resolved
Hide resolved
|
The version number still has to be changed, and I need to change some things according to the suggested changes. Other than that there seems to be a problem with the README file, as there are some errors in the code I did not change, and I can therefore not commit the file. |
JonasFranke
force-pushed
the
Create-team-system
branch
from
8e5c344 to
794087f
Compare
I fixed the README.md and removed the failing tests (The repo server is down). |
|
After you look into CodeFactor and the CodeRabbit review, we can merge! |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/gradle.yml(0 hunks).github/workflows/repo.yml(0 hunks)README.md(2 hunks)
💤 Files with no reviewable changes (2)
- .github/workflows/repo.yml
- .github/workflows/gradle.yml
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~92-~92: A comma might be missing here.
Context: ...ch lets players create teams, to access it enter the command /teams. The teams a...
(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🔇 Additional comments (1)
README.md (1)
168-168: Approve Maven repository URL update.The
<url>tag now correctly points to the intended repository endpoint.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (6)
src/main/java/de/j/stationofdoom/teams/TeamCMD.java (1)
14-16: Player null check properly implemented.The code correctly checks if the sender is a Player instance before casting, which addresses the previous review comment.
src/main/java/de/j/deathMinigames/database/TeamsDatabase.java (1)
58-76: 🛠️ Refactor suggestionN+1 query problem affects performance at scale.
The current implementation executes a separate query for each team to fetch its members. This creates N+1 queries for N teams, which can significantly impact performance during startup with many teams.
Consider fetching all team members in a single query and grouping them in memory:
for (Team team : teams) { Main.getMainLogger().info(team.getName()); - try { - Query.query("SELECT isInTeam, uuid, isTeamOperator FROM playerData WHERE uuidOfTeam = ?;") - .single(Call.of() - .bind(team.getUuid(), UUIDAdapter.AS_STRING) - ) - .map(row -> new DBTeamMember( - row.getBoolean("isInTeam"), - row.getString("uuid"), - row.getBoolean("isTeamOperator"))) - .all() - .forEach(team::addMember); - } - catch (Exception e) { - e.printStackTrace(); - Main.getMainLogger().warning("Could not get players from team " + team.getName()); - } } + +// Fetch all team members in one query +Query.query("SELECT uuidOfTeam, isInTeam, uuid, isTeamOperator FROM playerData WHERE uuidOfTeam IS NOT NULL;") + .single() + .map(row -> new Object[]{ + row.getString("uuidOfTeam"), + new DBTeamMember( + row.getBoolean("isInTeam"), + row.getString("uuid"), + row.getBoolean("isTeamOperator")) + }) + .all() + .forEach(data -> { + String teamUuid = (String) data[0]; + DBTeamMember member = (DBTeamMember) data[1]; + teams.stream() + .filter(t -> t.getUuid().toString().equals(teamUuid)) + .findFirst() + .ifPresent(team -> team.addMember(member)); + });src/main/java/de/j/stationofdoom/teams/TeamSettingsGUI.java (2)
24-24: Thread safety properly implemented.Good use of ConcurrentHashMap to ensure thread safety for the invSlots map, which addresses the previous review comment.
108-110: Null material handling properly implemented.The code correctly handles the case where material conversion returns null by providing a fallback to WHITE_STAINED_GLASS_PANE, which addresses the previous review comment.
src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java (1)
195-205: 🛠️ Refactor suggestionUse EXISTS query for more efficient existence check.
The current implementation still fetches data unnecessarily. Use EXISTS for better performance and remove debug logging.
public boolean checkIfPlayerIsInDatabase(UUID uuidOfPlayerToCheck) { if(!Database.getInstance().isConnected) return HandlePlayers.getKnownPlayers().containsKey(uuidOfPlayerToCheck); - List<UUID> data = Query.query("SELECT UUID FROM playerData WHERE UUID = :UUID;") - .single(Call.of() - .bind("UUID", uuidOfPlayerToCheck, UUIDAdapter.AS_STRING)) - .map(row -> UUID.fromString(row.getString("UUID"))) - .all(); - - Main.getMainLogger().warning("CheckIfPlayerIsInDatabase for: " + Bukkit.getOfflinePlayer(uuidOfPlayerToCheck).getName() + ", found " + !data.isEmpty()); - return !data.isEmpty(); + return Query.query("SELECT 1 FROM playerData WHERE UUID = :UUID LIMIT 1;") + .single(Call.of() + .bind("UUID", uuidOfPlayerToCheck, UUIDAdapter.AS_STRING)) + .exists(); }src/main/java/de/j/deathMinigames/main/PlayerData.java (1)
74-82: Good error handling for offline players.Based on our previous discussions,
OfflinePlayer.getLocation()is indeed a valid method. The try-catch approach provides good fallback handling for when the player is offline.
🧹 Nitpick comments (13)
src/main/java/de/j/stationofdoom/teams/TeamCMD.java (1)
19-20: Consider making TranslationFactory a class field.The TranslationFactory is instantiated inline each time it's needed. Consider making it a static or instance field for better performance.
public class TeamCMD implements BasicCommand { + private static final TranslationFactory tf = new TranslationFactory(); + @Override public void execute(CommandSourceStack stack, String[] args) { // ... - player.sendMessage(Component.text(new TranslationFactory().getTranslation(player, "TeamCMDNoTeam")) + player.sendMessage(Component.text(tf.getTranslation(player, "TeamCMDNoTeam")) .color(NamedTextColor.RED));src/main/java/de/j/deathMinigames/database/TeamsDatabase.java (1)
133-146: Consider using transactions to prevent data loss.The current implementation wipes the entire database before re-inserting teams. If an error occurs during re-insertion, all team data could be lost. Consider using database transactions or incremental updates instead.
public void updateTeamsDatabase() { if(!Database.getInstance().isConnected) return; List<Team> teams = HandleTeams.getInstance().getAllTeams(); - wipeDatabase(); - for (Team team : teams) { - if(team.getAllPlayers().isEmpty()) { - Main.getMainLogger().info("Team " + team.getName() + " has no players, removing it"); - } - else { - insertTeam(team); + + // Consider implementing transaction support or: + // 1. Track which teams need to be inserted/updated/deleted + // 2. Perform incremental updates + // 3. Only delete teams that no longer exist + + try { + // Begin transaction if supported + wipeDatabase(); + for (Team team : teams) { + if(team.getAllPlayers().isEmpty()) { + Main.getMainLogger().info("Team " + team.getName() + " has no players, removing it"); + } + else { + insertTeam(team); + } } + // Commit transaction if supported + } catch (Exception e) { + // Rollback transaction if supported + Main.getMainLogger().severe("Failed to update teams database: " + e.getMessage()); + throw e; }src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java (2)
51-56: Consider adding indexes for better query performance.The
UUIDcolumn is frequently used in WHERE clauses throughout the codebase. Adding an index would improve query performance.-Query.query("CREATE TABLE IF NOT EXISTS playerData (name VARCHAR(255), UUID VARCHAR(255), introduction BOOLEAN, usesPlugin BOOLEAN, difficulty INT, bestParkourTime FLOAT, isInTeam BOOLEAN, uuidOfTeam VARCHAR(255), isTeamOperator BOOLEAN);") +Query.query("CREATE TABLE IF NOT EXISTS playerData (name VARCHAR(255), UUID VARCHAR(255), introduction BOOLEAN, usesPlugin BOOLEAN, difficulty INT, bestParkourTime FLOAT, isInTeam BOOLEAN, uuidOfTeam VARCHAR(255), isTeamOperator BOOLEAN, INDEX idx_uuid (UUID));")
71-71: Remove temporary debug logging.The TODO comment indicates this warning log should be removed.
-Main.getMainLogger().warning("used getAllPlayerData"); //TODO removesrc/main/java/de/j/deathMinigames/listeners/AnvilListener.java (1)
74-85: Improve claiming radius validation to avoid duplicate messages and invalid zero value.The current code can send the error message twice, and a radius of 0 doesn't make sense for claiming.
try { claimingRadius = Integer.parseInt(renameText); + if(claimingRadius < 1 || claimingRadius > 256) { + player.sendMessage(Component.text(tf.getTranslation(player, "invalidAnvilInputClaimingRadius"))); + claimingRadius = -1; // Invalid marker + } } catch(NumberFormatException e) { player.sendMessage(Component.text(tf.getTranslation(player, "invalidAnvilInputClaimingRadius"))); - return; + claimingRadius = -1; // Invalid marker } -if(claimingRadius <= 0 || claimingRadius > 256) { - player.sendMessage(Component.text(tf.getTranslation(player, "invalidAnvilInputClaimingRadius"))); - return; -}src/main/java/de/j/stationofdoom/util/Tablist.java (1)
199-275: Refactor repetitive team assignment logic to reduce code duplication.The current implementation repeats the same pattern for each rank. Consider extracting the common logic.
private void setTeam(Player player) { assert !Main.isFolia(); UUID playerUUID = player.getUniqueId(); + + // Determine base team name + String baseTeamName; + if(playerRanks.containsKey(playerUUID)) { + switch (playerRanks.get(playerUUID)) { + case HOST -> baseTeamName = "0Host"; + case ADMIN -> baseTeamName = "1Admin"; + case DEVELOPER -> baseTeamName = "2Developer"; + default -> baseTeamName = "3"; + } + } else { + baseTeamName = "3"; + } + + // Build full team name + StringBuilder teamName = new StringBuilder(); + if(afkPlayers.contains(playerUUID)) { + teamName.append(baseTeamName.equals("3") ? "3AFK_" : baseTeamName.replace(baseTeamName.charAt(0) + "", baseTeamName.charAt(0) + "AFK_")); + } else { + teamName.append(baseTeamName).append(baseTeamName.equals("3") ? "_" : ""); + } + + // Add team suffix if player is in a team + Team playerTeam = HandleTeams.getTeamFromPlayerUUID(playerUUID); + if(playerTeam.isMember(playerUUID)) { + teamName.append(playerTeam.getUuid()); + } else if(teamName.toString().endsWith("_")) { + teamName.setLength(teamName.length() - 1); // Remove trailing underscore + } + + // Add player to team + var team = scoreboard.getTeam(teamName.toString()); + if(team != null) { + team.addPlayer(player); + } - // Remove all the existing switch/if logicsrc/main/java/de/j/deathMinigames/main/PlayerData.java (3)
144-156: Remove debug logging statements.The debug logging statements should be removed before production deployment. These will clutter the logs and may expose internal state information.
public boolean isInTeam() { - Main.getMainLogger().warning("Tried accessing isInTeam of " + this + " isInTeam = " + this.isInTeam); //TODO remove return this.isInTeam; } public void setInTeam(boolean isInTeam) { this.isInTeam = isInTeam; - Main.getMainLogger().warning("Set isInTeam of " + this + " to " + this.isInTeam); //TODO remove }
197-197: Remove debug logging from constructor.This debug logging statement should be removed before production deployment.
this.leftWhileProcessing = false; -Main.getMainLogger().warning("created player data for player " + this.name + " " + this.uuid); //TODO remove
218-218: Remove debug logging from constructor.This debug logging statement should be removed before production deployment.
} -Main.getMainLogger().warning("created player data for player " + this.name + " " + this.uuid + " via long method"); //TODO removesrc/main/java/de/j/stationofdoom/teams/Team.java (4)
51-96: Consider reducing constructor duplication.Multiple constructors have similar initialization logic. Consider extracting common initialization patterns to reduce code duplication.
public Team(String name, String colorAsString, boolean locked, String uuid, Location protectedLocation) { - init(name, UUID.fromString(uuid), Material.valueOf(colorAsString)); - convertMaterialToString(this.colorAsMaterial); - this.locked = locked; - this.inventory.setContents(TeamEnderchestsDatabase.getInstance().getTeamEnderchest(this.getUuid()).getContents()); + initWithDatabase(name, UUID.fromString(uuid), Material.valueOf(colorAsString), locked); if (protectedLocation != null && protectedLocation.getWorld() != null) { this.protectedLocation = protectedLocation; } } +private void initWithDatabase(String name, UUID uuid, Material colorAsMaterial, boolean locked) { + init(name, uuid, colorAsMaterial); + convertMaterialToString(this.colorAsMaterial); + this.locked = locked; + this.inventory.setContents(TeamEnderchestsDatabase.getInstance().getTeamEnderchest(this.getUuid()).getContents()); +}
168-176: Improve random material selection readability.The current implementation works but could be more readable and efficient.
private Material getRandomConcreteMaterial() { - List<Material> materials = new ArrayList<>(); - for (Material material : Material.values()) { - if (material.name().contains("CONCRETE") && !material.name().contains("POWDER")) { - materials.add(material); - } - } - return materials.get(ThreadLocalRandom.current().nextInt(0, materials.size())); + List<Material> concreteMaterials = Arrays.stream(Material.values()) + .filter(material -> material.name().contains("CONCRETE") && !material.name().contains("POWDER")) + .collect(Collectors.toList()); + + return concreteMaterials.get(ThreadLocalRandom.current().nextInt(concreteMaterials.size())); }
277-287: Simplify member checking logic.The current implementation can be simplified using HashMap's containsKey method directly.
public boolean isMember(UUID uuidOfPlayerToCheck) { if (uuidOfPlayerToCheck == null) { return false; } - for (UUID uuid : this.members.keySet()) { - if(uuid.equals(uuidOfPlayerToCheck)) { - return true; - } - } - return false; + return this.members.containsKey(uuidOfPlayerToCheck); }
260-270: Good operator management with edge case handling.The operator management properly handles the case where no operators remain by unlocking the team. The redundant containsKey check in line 269 could be simplified since line 268 already checks this.
public boolean isTeamOperator(PlayerData playerData) { if(!this.members.containsKey(playerData.getUniqueId())) return false; - return this.members.containsKey(playerData.getUniqueId()) && this.members.get(playerData.getUniqueId()); + return this.members.get(playerData.getUniqueId()); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/main/java/de/j/deathMinigames/database/DBTeamMember.java(1 hunks)src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java(6 hunks)src/main/java/de/j/deathMinigames/database/TeamEnderchestsDatabase.java(1 hunks)src/main/java/de/j/deathMinigames/database/TeamsDatabase.java(1 hunks)src/main/java/de/j/deathMinigames/listeners/AnvilListener.java(5 hunks)src/main/java/de/j/deathMinigames/listeners/JoinListener.java(2 hunks)src/main/java/de/j/deathMinigames/main/HandlePlayers.java(8 hunks)src/main/java/de/j/deathMinigames/main/PlayerData.java(7 hunks)src/main/java/de/j/deathMinigames/settings/AnvilUI.java(2 hunks)src/main/java/de/j/stationofdoom/teams/HandleTeams.java(1 hunks)src/main/java/de/j/stationofdoom/teams/Team.java(1 hunks)src/main/java/de/j/stationofdoom/teams/TeamCMD.java(1 hunks)src/main/java/de/j/stationofdoom/teams/TeamPlayerSettingsGUI.java(1 hunks)src/main/java/de/j/stationofdoom/teams/TeamSettingsGUI.java(1 hunks)src/main/java/de/j/stationofdoom/teams/TeamsMainMenuGUI.java(1 hunks)src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/BlockBreakAndUseCancelListener.java(1 hunks)src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/ChunkClaimSystem.java(1 hunks)src/main/java/de/j/stationofdoom/teams/enderchest/EnderchestInvHolder.java(1 hunks)src/main/java/de/j/stationofdoom/teams/enderchest/TeamEnderchestPreventEnchantedItemsInputListener.java(1 hunks)src/main/java/de/j/stationofdoom/util/Tablist.java(2 hunks)src/main/resources/translations.json(4 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/main/java/de/j/deathMinigames/database/DBTeamMember.java
🚧 Files skipped from review as they are similar to previous changes (12)
- src/main/java/de/j/stationofdoom/teams/enderchest/EnderchestInvHolder.java
- src/main/java/de/j/stationofdoom/teams/enderchest/TeamEnderchestPreventEnchantedItemsInputListener.java
- src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/BlockBreakAndUseCancelListener.java
- src/main/java/de/j/deathMinigames/settings/AnvilUI.java
- src/main/resources/translations.json
- src/main/java/de/j/deathMinigames/listeners/JoinListener.java
- src/main/java/de/j/stationofdoom/teams/TeamsMainMenuGUI.java
- src/main/java/de/j/deathMinigames/main/HandlePlayers.java
- src/main/java/de/j/stationofdoom/teams/TeamPlayerSettingsGUI.java
- src/main/java/de/j/stationofdoom/teams/HandleTeams.java
- src/main/java/de/j/deathMinigames/database/TeamEnderchestsDatabase.java
- src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/ChunkClaimSystem.java
🧰 Additional context used
🧠 Learnings (6)
src/main/java/de/j/deathMinigames/database/TeamsDatabase.java (1)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#214
File: src/main/java/de/j/deathMinigames/database/TeamsDatabase.java:34-39
Timestamp: 2025-06-01T19:44:19.351Z
Learning: The chojo.sadu library used in this codebase only supports specific statement methods like `.insert()`, `.delete()`, etc. and does not have a generic `.execute()` method. For DDL statements like CREATE TABLE, `.insert()` is the appropriate method to use with this library.
src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java (1)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#176
File: src/main/java/de/j/deathMinigames/minigames/Minigame.java:47-49
Timestamp: 2024-12-13T07:24:43.026Z
Learning: In `src/main/java/de/j/deathMinigames/minigames/Minigame.java`, `playerInArena` is always null-checked before comparing its UUID with `player.getUniqueId()`. Therefore, additional null checks for `playerInArena` before the UUID comparison are unnecessary.
src/main/java/de/j/stationofdoom/util/Tablist.java (2)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#214
File: src/main/java/de/j/stationofdoom/util/Tablist.java:203-206
Timestamp: 2025-06-06T09:04:41.968Z
Learning: In the HandleTeams.getTeamFromPlayerUUID method, the method never returns null but instead returns either the player's actual team or a new empty Team() object when the player is not in any team. The subsequent isMember() check on an empty team returns false, safely handling the no-team case without causing NullPointerExceptions.
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#214
File: src/main/java/de/j/stationofdoom/util/Tablist.java:191-194
Timestamp: 2025-06-06T09:03:32.059Z
Learning: In the StationofDoomPlugin project, hard-coded UUIDs for developer ranks in the Tablist class are intentionally kept simple since there are only two developers and the values rarely change. The team prefers this approach over configuration-based management for this specific use case.
src/main/java/de/j/deathMinigames/listeners/AnvilListener.java (1)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#214
File: src/main/java/de/j/stationofdoom/util/Tablist.java:203-206
Timestamp: 2025-06-06T09:04:41.968Z
Learning: In the HandleTeams.getTeamFromPlayerUUID method, the method never returns null but instead returns either the player's actual team or a new empty Team() object when the player is not in any team. The subsequent isMember() check on an empty team returns false, safely handling the no-team case without causing NullPointerExceptions.
src/main/java/de/j/stationofdoom/teams/Team.java (1)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#214
File: src/main/java/de/j/stationofdoom/teams/Team.java:214-243
Timestamp: 2025-06-06T10:12:40.804Z
Learning: In the StationofDoomPlugin project, the TeamEnderchestsDatabase automatically cleans itself up during server shutdown, so manual cleanup calls like `TeamEnderchestsDatabase.getInstance().removeTeamEnderchest()` are not necessary in team removal methods.
src/main/java/de/j/deathMinigames/main/PlayerData.java (1)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#214
File: src/main/java/de/j/deathMinigames/main/PlayerData.java:74-82
Timestamp: 2025-06-04T11:42:55.996Z
Learning: OfflinePlayer in the Spigot API has a getLocation() method that returns the player's current location or null if the player has never played before. This method is valid and will compile.
🔇 Additional comments (6)
src/main/java/de/j/stationofdoom/teams/TeamCMD.java (1)
17-18:⚠️ Potential issueAdd null check for team to prevent NPE.
The
HandleTeams.getTeamFromPlayerUUIDmethod could return null, which would cause a NullPointerException when callingisMember().Team team = HandleTeams.getTeamFromPlayerUUID(player.getUniqueId()); +if (team == null) { + player.sendMessage(Component.text(new TranslationFactory().getTranslation(player, "TeamCMDNoTeam")) + .color(NamedTextColor.RED)); + return; +} if (!team.isMember(player.getUniqueId())) {⛔ Skipped due to learnings
Learnt from: PrinzPizza42 PR: AtomPilz-DevTeam/StationofDoomPlugin#214 File: src/main/java/de/j/stationofdoom/util/Tablist.java:203-206 Timestamp: 2025-06-06T09:04:41.968Z Learning: In the HandleTeams.getTeamFromPlayerUUID method, the method never returns null but instead returns either the player's actual team or a new empty Team() object when the player is not in any team. The subsequent isMember() check on an empty team returns false, safely handling the no-team case without causing NullPointerExceptions.Learnt from: PrinzPizza42 PR: AtomPilz-DevTeam/StationofDoomPlugin#214 File: src/main/java/de/j/stationofdoom/teams/TeamsCMD.java:24-28 Timestamp: 2025-06-01T17:13:58.721Z Learning: In the TeamsCMD class for team enderchest access via command, the silent return when a player is not in a team or the team is null is intentional defensive programming against bugs, not user-facing functionality that requires feedback messages.src/main/java/de/j/deathMinigames/main/PlayerData.java (3)
23-29: Good method rename for consistency.The rename from
getUuid()togetUniqueId()improves consistency with Bukkit API naming conventions. The addition ofsetUniqueID()provides the needed setter functionality.
31-36: Excellent lazy initialization pattern.The lazy initialization of the player field improves performance by only fetching the Player object when needed. Good defensive programming practice.
200-217: Good team data initialization logic.The constructor properly handles team data initialization with appropriate null checks and UUID conversion. The conditional logic for team membership is well-structured.
src/main/java/de/j/stationofdoom/teams/Team.java (2)
214-258: Good error handling in team removal.The team removal logic properly handles edge cases and includes exception handling for item dropping. The inventory clearing at the end helps with garbage collection. Based on previous feedback, the database cleanup is handled automatically on server shutdown.
182-197: Complex but correct member join/leave logic.The logic properly handles both joining and leaving scenarios, including team removal when the last member leaves. The integration with PlayerData updates is well-structured.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/ChunkClaimSystem.java (1)
172-193:⚠️ Potential issueBukkit API used from async thread – can crash the server
The method uses
runTaskTimerAsynchronouslybut callsplayer.spawnParticlewhich is not thread-safe. This can cause crashes or unpredictable behavior.- runnable.runTaskTimerAsynchronously(Main.getPlugin(), 0, period); + runnable.runTaskTimer(Main.getPlugin(), 0, period);src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java (1)
85-101:⚠️ Potential issueHandle empty query results to prevent NoSuchElementException.
The
getFirst()call will throw an exception if no player data is found.public PlayerData getPlayerDataFromDB(UUID uuid) { if(!Database.getInstance().isConnected) return null; - return Query.query("SELECT * FROM playerdata WHERE uuid = ?;") + List<PlayerData> results = Query.query("SELECT * FROM playerdata WHERE uuid = ?;") .single(Call.of() .bind(uuid, UUIDAdapter.AS_STRING)) .map(row -> new PlayerData(row.getString("name"), row.getString("uuid"), row.getBoolean("introduction"), row.getBoolean("usesPlugin"), row.getInt("difficulty"), row.getFloat("bestParkourTime"), row.getBoolean("isInTeam"), row.getString("uuidOfTeam"), row.getBoolean("isTeamOperator"))) - .all() - .getFirst(); + .all(); + return results.isEmpty() ? null : results.get(0); }
🧹 Nitpick comments (2)
src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/ChunkClaimSystem.java (1)
91-102: Remove unused variablei.The variable
iis incremented but never used.private HashMap<Location, Team> getAllProtectedLocations() { HashMap<Location, Team> locationsOfTeamsMap = new HashMap<>(); - int i = 0; for (Team team : HandleTeams.getInstance().getAllTeams()){ Location location = team.getProtectedLocation(); if(location != null) { locationsOfTeamsMap.put(team.getProtectedLocation(), team); - i++; } } return locationsOfTeamsMap; }src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java (1)
193-202: Remove debug logging statement.The warning log on line 201 appears to be debug code that should be removed.
- Main.getMainLogger().warning("CheckIfPlayerIsInDatabase for: " + Bukkit.getOfflinePlayer(uuidOfPlayerToCheck).getName() + ", found " + !data.isEmpty()); return !data.isEmpty();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java(6 hunks)src/main/java/de/j/deathMinigames/listeners/JoinListener.java(2 hunks)src/main/java/de/j/deathMinigames/main/HandlePlayers.java(8 hunks)src/main/java/de/j/deathMinigames/main/PlayerData.java(7 hunks)src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java(1 hunks)src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/ChunkClaimSystem.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/main/java/de/j/deathMinigames/listeners/JoinListener.java
- src/main/java/de/j/deathMinigames/main/HandlePlayers.java
- src/main/java/de/j/deathMinigames/main/PlayerData.java
🧰 Additional context used
🧠 Learnings (2)
src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java (1)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#176
File: src/main/java/de/j/deathMinigames/minigames/Minigame.java:47-49
Timestamp: 2024-12-13T07:24:43.026Z
Learning: In `src/main/java/de/j/deathMinigames/minigames/Minigame.java`, `playerInArena` is always null-checked before comparing its UUID with `player.getUniqueId()`. Therefore, additional null checks for `playerInArena` before the UUID comparison are unnecessary.
src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java (1)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#214
File: src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java:31-50
Timestamp: 2025-06-04T11:47:04.808Z
Learning: In the StationofDoomPlugin project, the team operates in a controlled environment where they manage inventory creation through their own plugin, and server owners actively choose which plugins to install. The team considers this sufficient protection against potential issues with inventory manipulation from external sources.
🔇 Additional comments (4)
src/main/java/de/j/stationofdoom/teams/chunkClaimSystem/ChunkClaimSystem.java (1)
29-38: Well-implemented thread-safe singleton pattern.The double-checked locking with volatile instance ensures thread safety and lazy initialization.
src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java (3)
53-53: Table schema properly updated for team support.All necessary team-related columns (isInTeam, uuidOfTeam, isTeamOperator) are included.
119-145: Well-structured team data update logic.The method correctly handles both scenarios - players with and without teams, ensuring data consistency.
166-176: INSERT query correctly includes all columns.The previously missing
isTeamOperatorcolumn is now properly included in the INSERT statement.
src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java
Show resolved
Hide resolved
src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java (1)
37-39: Null check occurs after variable usage.The player is cast and used to get PlayerData before the null check.
🧹 Nitpick comments (3)
src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java (3)
168-169: Remove unnecessary null check for player UUID.
Player.getUniqueId()never returns null in Bukkit/Paper APIs, making this check redundant.- if(player.getUniqueId() == null) { - Main.getMainLogger().info("Player is null in TeamSettingsInventoryListener"); - return; - }
178-184: Simplify complex conditional logic.The nested conditions can be simplified for better readability.
- if(slot >= 9 && slot <=13 || slot == 17 || slot >= 18 && slot <= 44 && inv.getItem(slot) != null) { - if(team.getLocked() && !team.isTeamOperator(HandlePlayers.getInstance().getPlayerData(player.getUniqueId()))) { - player.sendMessage(Component.text(tf.getTranslation(player, "teamLockedAndNotOperator")).color(NamedTextColor.RED)); - player.playSound(Sound.sound(BLOCK_AMETHYST_CLUSTER_BREAK, Sound.Source.PLAYER, 0.5F, 1), net.kyori.adventure.sound.Sound.Emitter.self()); - teamSettingsGUI.showPage(currentPage, player); - } - } + boolean isRestrictedSlot = (slot >= 9 && slot <= 13) || slot == 17 || + (slot >= 18 && slot <= 44 && inv.getItem(slot) != null); + if(isRestrictedSlot && team.getLocked() && !team.isTeamOperator(HandlePlayers.getInstance().getPlayerData(player.getUniqueId()))) { + player.sendMessage(Component.text(tf.getTranslation(player, "teamLockedAndNotOperator")).color(NamedTextColor.RED)); + player.playSound(Sound.sound(BLOCK_AMETHYST_CLUSTER_BREAK, Sound.Source.PLAYER, 0.5F, 1), net.kyori.adventure.sound.Sound.Emitter.self()); + teamSettingsGUI.showPage(currentPage, player); + return; + }
264-267: Extract duplicate permission checking logic.The team lock and operator permission check is duplicated across multiple methods.
+ private boolean hasPermissionForLockedTeam(Team team, Player player) { + return !team.getLocked() || team.isTeamOperator(HandlePlayers.getInstance().getPlayerData(player.getUniqueId())); + } + if(slot == 9 || slot == 10) { - if(!team.isTeamOperator(playerData) && team.getLocked()) { + if(!hasPermissionForLockedTeam(team, player)) { player.sendMessage(Component.text(tf.getTranslation(player, "teamLockedAndNotOperator")).color(NamedTextColor.RED)); return; } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java(6 hunks)src/main/java/de/j/deathMinigames/main/HandlePlayers.java(8 hunks)src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/main/java/de/j/deathMinigames/main/HandlePlayers.java
- src/main/java/de/j/deathMinigames/database/PlayerDataDatabase.java
🧰 Additional context used
🧠 Learnings (1)
src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java (2)
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#214
File: src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java:31-50
Timestamp: 2025-06-04T11:47:04.808Z
Learning: In the StationofDoomPlugin project, the team operates in a controlled environment where they manage inventory creation through their own plugin, and server owners actively choose which plugins to install. The team considers this sufficient protection against potential issues with inventory manipulation from external sources.
Learnt from: PrinzPizza42
PR: AtomPilz-DevTeam/StationofDoomPlugin#214
File: src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java:232-240
Timestamp: 2025-06-06T11:30:50.234Z
Learning: In plugin-controlled inventories where the plugin systematically populates items and players cannot modify the contents, defensive null checks for inventory items may be unnecessary since the plugin guarantees the items will be present.
🔇 Additional comments (1)
src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java (1)
292-294: 🛠️ Refactor suggestionVerify player exists before accessing methods.
The code assumes the player exists when getting by UUID, which could cause NPE if the player is offline.
if(clickedOnPlayerData.isOnline()) { - clickedOnPlayerData.getPlayer().sendMessage(Component.text(tf.getTranslation(clickedOnPlayerData.getPlayer(), "kickedFromTeam", player.getName())).color(NamedTextColor.RED)); - Bukkit.getPlayer(clickedOnPlayerData.getUniqueId()).playSound(Sound.sound(BLOCK_AMETHYST_BLOCK_RESONATE, Sound.Source.PLAYER, 1F, 1), net.kyori.adventure.sound.Sound.Emitter.self()); + Player kickedPlayer = clickedOnPlayerData.getPlayer(); + if (kickedPlayer != null) { + kickedPlayer.sendMessage(Component.text(tf.getTranslation(kickedPlayer, "kickedFromTeam", player.getName())).color(NamedTextColor.RED)); + kickedPlayer.playSound(Sound.sound(BLOCK_AMETHYST_BLOCK_RESONATE, Sound.Source.PLAYER, 1F, 1), net.kyori.adventure.sound.Sound.Emitter.self()); + } }⛔ Skipped due to learnings
Learnt from: PrinzPizza42 PR: AtomPilz-DevTeam/StationofDoomPlugin#176 File: src/main/java/de/j/deathMinigames/settings/GUI.java:43-43 Timestamp: 2024-11-10T20:12:15.303Z Learning: In `GUI.java`, the `getPlayer` method will return the player if they exist and should not return `null` or an `OfflinePlayer` if the player is offline. Therefore, redundant null checks on `getPlayer` are unnecessary in this context.
src/main/java/de/j/stationofdoom/teams/TeamSettingsInventoryListener.java
Show resolved
Hide resolved
2 participants
Implementation of a team system
...
Notes for testing your change
...
Summary by CodeRabbit
New Features
/teamsand/team, enabling team creation, management, member roles, protected location claiming, and team enderchests with inventory persistence.Bug Fixes
Documentation
Chores