Conversation
WalkthroughThe pull request introduces a comprehensive implementation of the BlockVault Minecraft plugin, focusing on creating an interactive museum-like challenge where players can contribute and track their progress. The changes include setting up the project structure, implementing core functionality through utility classes, adding command handlers for various interactions, and configuring the plugin's behavior through configuration files. The plugin enables players to start a vault challenge, submit items, check progress, and view a leaderboard. Changes
Sequence DiagramsequenceDiagram
participant Player
participant BlockVault Plugin
participant VaultUtil
participant FileUtil
Player->>BlockVault Plugin: /bvstart
BlockVault Plugin->>VaultUtil: Check vault status
BlockVault Plugin->>FileUtil: Update configuration
BlockVault Plugin-->>Player: Vault challenge started
Player->>BlockVault Plugin: /bvsubmit
BlockVault Plugin->>VaultUtil: Validate item
BlockVault Plugin->>FileUtil: Save player progress
BlockVault Plugin-->>Player: Item submitted successfully
Player->>BlockVault Plugin: /bvleaderboard
BlockVault Plugin->>VaultUtil: Retrieve player points
BlockVault Plugin-->>Player: Display top contributors
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🔭 Outside diff range comments (1)
pom.xml (1)
Line range hint
31-37: Enhance shade plugin configurationThe shade plugin configuration should include minimization and relocation rules to reduce the final JAR size and avoid potential conflicts.
Add the following configuration:
<plugin> <groupId>org.apache.maven.plugins</groupId> <artifactId>maven-shade-plugin</artifactId> <version>3.5.3</version> <executions> <execution> <phase>package</phase> <goals> <goal>shade</goal> </goals> + <configuration> + <minimizeJar>true</minimizeJar> + <filters> + <filter> + <artifact>*:*</artifact> + <excludes> + <exclude>META-INF/*.SF</exclude> + <exclude>META-INF/*.DSA</exclude> + <exclude>META-INF/*.RSA</exclude> + </excludes> + </filter> + </filters> + <relocations> + <!-- Add relocations if you shade any libraries --> + </relocations> + </configuration> </execution> </executions> </plugin>
🧹 Nitpick comments (17)
src/main/java/me/benrobson/blockvault/util/VaultUtil.java (3)
35-68: Optimize material filtering logic.
Right now, it iterates overMaterial.values(), which can be large, and checksmaterial.isBlock() || material.isItem(). This is fine for smaller plugins, but consider caching or maintaining a refined material list if you anticipate frequent calls or a large environment. Also, note that.toLowerCase()usage for excluded items implies the config’s excluded items must be strictly lowercase.
70-83: Potential performance gain by reusing string builder.
TheformatMaterialNamemethod is solid. As a minor optimization, splitting and capitalizing each part could be done with a single pass, but the current approach is clear and perfectly acceptable for typical usage.
86-113: Consider using a Set for faster lookups inisItemAdded().
Checking each player’s list requires a nested loop. For large numbers of players/items, performance may be impacted. One approach is to store all collected items in aHashSet(or multiple sets by player) to allow faster membership tests.src/main/java/me/benrobson/blockvault/commands/bvupdatestate.java (1)
17-17: Class naming convention.
By convention, Java class names start with an uppercase letter. Consider renamingbvupdatestatetoBvUpdateStatefor clarity and consistency.src/main/java/me/benrobson/blockvault/commands/bvstart.java (1)
11-11: Class naming style.
Similarly tobvupdatestate, renamebvstartto follow Java naming conventions (e.g.,BvStart). This improves readability and consistency with other classes.src/main/java/me/benrobson/blockvault/util/FileUtil.java (2)
30-64: Maintain synchronization between in-memory and plugin config.
Calling bothsaveConfig()andplugin.reloadConfig()is usually fine. However, the plugin’s defaultgetConfig()can diverge from yourYamlConfigurationinstance if updates happen outside yourFileUtilclass. Consider consistently funneling all reads/writes throughFileUtilto prevent confusion.
58-65: Offer fallback if reload fails.
IfreloadConfig()fails or the file is corrupted, the code might proceed with partial data. Consider a fallback strategy (e.g., revert to a known good backup) or more robust error handling to keep the plugin stable.src/main/java/me/benrobson/blockvault/commands/bvprogress.java (1)
30-33: Check for 'items' configuration section to avoid NullPointerException
If "vault_items.yml" does not contain the "items" section,vaultItems.getConfigurationSection("items")could return null, potentially causing a NullPointerException. It might be wise to verify that the section exists before callinggetKeys(false).File vaultItemsFile = new File(plugin.getDataFolder(), "vault_items.yml"); YamlConfiguration vaultItems = YamlConfiguration.loadConfiguration(vaultItemsFile); +if (vaultItems.getConfigurationSection("items") == null) { + sender.sendMessage("§cNo items defined in vault_items.yml. Please contact an admin."); + return true; +} int totalItems = vaultItems.getConfigurationSection("items").getKeys(false).size();src/main/java/me/benrobson/blockvault/commands/bvleaderboard.java (2)
39-43: Suggest verifying that relevant data exists
If"vault_data"is missing or empty, you correctly show an error message. You might also consider logging a warning on the server side so that admins know the data is missing.
46-54: Avoid potential concurrency issues
Although unlikely in a typical Minecraft scenario, reading and updatingvault_data.ymlin different threads could lead to stale or incomplete data. Ensure that any concurrent writes are synchronized or queued on the main thread.src/main/java/me/benrobson/blockvault/commands/bvsubmit.java (2)
30-35: Enhance feedback for non-player command senders
You return early if the sender isn't a player, which is good. For clarity, you might also consider providing a usage message or a link to documentation for non-player contexts.
79-82: Notify admins or log if an item is missing from 'vault_items.yml'
The code assigns the "COMMON" category by default if the item isn't found invault_items.yml, but you may want to log a warning to let admins or developers know that the item is missing, to prevent silent misconfigurations.String category = vaultItems.getString("items." + itemName, "COMMON"); // Default to COMMON int pointsPerCategory = config.getInt("points." + category, 1); // Default to 1 point +if (!vaultItems.contains("items." + itemName)) { + plugin.getLogger().warning("Item '" + itemName + "' not found in vault_items.yml. Defaulting to 'COMMON'."); +}src/main/java/me/benrobson/blockvault/BlockVault.java (2)
26-30: Refactor command registration for better maintainabilityConsider creating a command registry utility to reduce repetition and improve maintainability. This would also make it easier to add new commands in the future.
+ private void registerCommands() { + Map<String, CommandExecutor> commands = Map.of( + "bvstart", new bvstart(this), + "bvsubmit", new bvsubmit(this), + "bvprogress", new bvprogress(this), + "bvleaderboard", new bvleaderboard(this), + "bvupdatestate", new bvupdatestate(this) + ); + + commands.forEach((name, executor) -> + getCommand(name).setExecutor(executor)); + }
34-36: Add logging for critical operationsImportant operations like config saving and vault item generation should be logged for better debugging and monitoring.
+ getLogger().info("Saving default configuration..."); saveDefaultConfig(); + getLogger().info("Generating vault items..."); VaultUtil.generateVaultItems(this); + getLogger().info("Scheduling vault state task..."); scheduleVaultStateTask(this, vaultUtil, fileUtil);README.md (1)
28-32: Enhance command documentationThe commands section should include:
- All required and optional permissions
- Command usage examples
- Command arguments and their descriptions
Consider adding a more detailed command section like this:
## Commands | Command | Description | Permission | Usage Example | |---------|-------------|------------|---------------| | `/bvstart` | Starts the vault challenge | `blockvault.start` | `/bvstart` | | `/bvsubmit` | Submit items to the vault | `blockvault.submit` | `/bvsubmit` | | `/bvprogress` | View vault progress | `blockvault.progress` | `/bvprogress` | | `/bvleaderboard` | View top contributors | `blockvault.leaderboard` | `/bvleaderboard` |src/main/resources/config.yml (2)
17-115: Organize excluded items by categoriesThe excluded items list is quite long and could be better organized using YAML anchors and aliases. Group items by type (e.g., spawn eggs, technical blocks, etc.).
excludeditems: - - debug_stick - - structure_block - # ... many more items ... + technical_blocks: &technical + - debug_stick + - structure_block + - barrier + - command_block + - chain_command_block + - repeating_command_block + + special_blocks: &special + - spawner + - end_portal_frame + - bedrock + - dragon_egg + + spawn_eggs: &spawn_eggs + - piglin_brute_spawn_egg + - parrot_spawn_egg + # ... other spawn eggs ... + + excludeditems: + - *technical + - *special + - *spawn_eggs
128-128: Add newline at end of fileAdd a newline character at the end of the file to comply with YAML standards.
prefix: "&8&l[&3BD&8&l]" +🧰 Tools
🪛 yamllint (1.35.1)
[error] 128-128: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.gitignore(1 hunks)README.md(1 hunks)dependency-reduced-pom.xml(1 hunks)pom.xml(2 hunks)src/main/java/me/benrobson/blockvault/BlockVault.java(1 hunks)src/main/java/me/benrobson/blockvault/Blockvault.java(0 hunks)src/main/java/me/benrobson/blockvault/commands/bvleaderboard.java(1 hunks)src/main/java/me/benrobson/blockvault/commands/bvprogress.java(1 hunks)src/main/java/me/benrobson/blockvault/commands/bvstart.java(1 hunks)src/main/java/me/benrobson/blockvault/commands/bvsubmit.java(1 hunks)src/main/java/me/benrobson/blockvault/commands/bvupdatestate.java(1 hunks)src/main/java/me/benrobson/blockvault/util/FileUtil.java(1 hunks)src/main/java/me/benrobson/blockvault/util/ScheduleUtil.java(1 hunks)src/main/java/me/benrobson/blockvault/util/VaultUtil.java(1 hunks)src/main/resources/config.yml(1 hunks)src/main/resources/plugin.yml(1 hunks)
💤 Files with no reviewable changes (1)
- src/main/java/me/benrobson/blockvault/Blockvault.java
✅ Files skipped from review due to trivial changes (1)
- .gitignore
🧰 Additional context used
🪛 yamllint (1.35.1)
src/main/resources/config.yml
[error] 128-128: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (10)
pom.xml (1)
75-80: Verify WorldEdit version compatibilityThe WorldEdit dependency version 7.2.9 should be verified for compatibility with Paper 1.21.3. Additionally, consider adding version properties for better version management.
+ <properties> + <!-- Existing properties --> + <worldedit.version>7.2.9</worldedit.version> + </properties> <dependency> <groupId>com.sk89q.worldedit</groupId> <artifactId>worldedit-bukkit</artifactId> - <version>7.2.9</version> + <version>${worldedit.version}</version> <scope>provided</scope> </dependency>Let's verify the compatibility:
src/main/java/me/benrobson/blockvault/util/VaultUtil.java (2)
21-29: Ensure FileUtil dependency is used consistently.
The constructor correctly initializes thefileUtildependency, but watch out for any future method calls that also rely on this utility class. Make sure they consistently use thefileUtilinstance rather than creating a new one on the fly to avoid sync issues or unexpected configuration states.
31-33: Validate config loading forhasStarted().
The method returns a boolean from the plugin’s config. Ensure you reload or save config if there's any chance the file changed at runtime before readingvault.started.src/main/java/me/benrobson/blockvault/util/ScheduleUtil.java (2)
17-28: Schedule task usage confirmed.
UsingBukkit.getScheduler().runTaskTimerwith the correct interval in ticks is a standard way to schedule recurring tasks. This design is straightforward and avoids concurrency pitfalls.
17-28: Gracefully handle negative or zero intervals.
IfintervalMinutesis zero or negative, the scheduled task could cause a flood of repeated calls or never run at all. Consider adding a check that logs an error or defaults to a minimum safe interval (e.g., 1 minute).src/main/java/me/benrobson/blockvault/commands/bvupdatestate.java (1)
28-44: Restrict command usage to correct context.
You’ve already enforced player-only usage plus permission checks. This is good. Optionally, add a help message or usage instructions for players who type the command incorrectly or with extra arguments.src/main/java/me/benrobson/blockvault/commands/bvstart.java (1)
22-49: Ensure consistent state updates.
Starting the vault updatesvault.startedtotrueand logs the start time in config. Confirm that any commands checking the vault state re-read the config so they don’t rely on stale data. Your usage offileUtil.updateConfigValuehelps, but verifying no concurrency or ordering issues might be beneficial.src/main/resources/plugin.yml (2)
1-3: Updated plugin name and main class
Renaming the plugin to “BlockVault” and referencingBlockVaultinstead ofBlockvaultis a good improvement for clarity. No issues found here.
5-20: Commands properly registered
All the new commands (bvsubmit,bvleaderboard,bvprogress,bvstart,bvupdatestate) appear to be mapped correctly with usage instructions. This is essential for discoverability and user guidance.src/main/resources/config.yml (1)
9-16: Validate region coordinatesThe region coordinates should be validated to ensure:
- Upper region Y is always higher than lower region Y
- Coordinates form a valid 3D space
Let's check if there's validation logic in the codebase:
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
| <groupId>me.benrobson</groupId> | ||
| <artifactId>blockvault</artifactId> | ||
| <name>blockvault</name> | ||
| <version>1.0.0</version> | ||
| <build> | ||
| <defaultGoal>clean package</defaultGoal> | ||
| <resources> | ||
| <resource> | ||
| <filtering>true</filtering> | ||
| <directory>src/main/resources</directory> | ||
| </resource> | ||
| </resources> | ||
| <plugins> | ||
| <plugin> | ||
| <artifactId>maven-compiler-plugin</artifactId> | ||
| <version>3.13.0</version> | ||
| <configuration> | ||
| <source>${java.version}</source> | ||
| <target>${java.version}</target> | ||
| </configuration> | ||
| </plugin> | ||
| <plugin> | ||
| <artifactId>maven-shade-plugin</artifactId> | ||
| <version>3.5.3</version> | ||
| <executions> | ||
| <execution> | ||
| <phase>package</phase> | ||
| <goals> | ||
| <goal>shade</goal> | ||
| </goals> | ||
| </execution> | ||
| </executions> | ||
| </plugin> | ||
| </plugins> | ||
| </build> | ||
| <repositories> | ||
| <repository> | ||
| <id>papermc-repo</id> | ||
| <url>https://repo.papermc.io/repository/maven-public/</url> | ||
| </repository> | ||
| <repository> | ||
| <id>sonatype</id> | ||
| <url>https://oss.sonatype.org/content/groups/public/</url> | ||
| </repository> | ||
| <repository> | ||
| <id>enginehub-maven</id> | ||
| <url>https://maven.enginehub.org/repo/</url> | ||
| </repository> | ||
| </repositories> | ||
| <dependencies> | ||
| <dependency> | ||
| <groupId>io.papermc.paper</groupId> | ||
| <artifactId>paper-api</artifactId> | ||
| <version>1.21.3-R0.1-SNAPSHOT</version> | ||
| <scope>provided</scope> | ||
| </dependency> | ||
| <dependency> | ||
| <groupId>com.sk89q.worldedit</groupId> | ||
| <artifactId>worldedit-bukkit</artifactId> | ||
| <version>7.2.9</version> | ||
| <scope>provided</scope> | ||
| </dependency> | ||
| </dependencies> | ||
| <properties> | ||
| <java.version>21</java.version> | ||
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | ||
| </properties> | ||
| </project> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove dependency-reduced-pom.xml from version control
This file is automatically generated by the maven-shade-plugin during the build process and should not be committed to version control. Add it to .gitignore to prevent accidental commits.
Create or update .gitignore:
+dependency-reduced-pom.xml📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <?xml version="1.0" encoding="UTF-8"?> | |
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd"> | |
| <modelVersion>4.0.0</modelVersion> | |
| <groupId>me.benrobson</groupId> | |
| <artifactId>blockvault</artifactId> | |
| <name>blockvault</name> | |
| <version>1.0.0</version> | |
| <build> | |
| <defaultGoal>clean package</defaultGoal> | |
| <resources> | |
| <resource> | |
| <filtering>true</filtering> | |
| <directory>src/main/resources</directory> | |
| </resource> | |
| </resources> | |
| <plugins> | |
| <plugin> | |
| <artifactId>maven-compiler-plugin</artifactId> | |
| <version>3.13.0</version> | |
| <configuration> | |
| <source>${java.version}</source> | |
| <target>${java.version}</target> | |
| </configuration> | |
| </plugin> | |
| <plugin> | |
| <artifactId>maven-shade-plugin</artifactId> | |
| <version>3.5.3</version> | |
| <executions> | |
| <execution> | |
| <phase>package</phase> | |
| <goals> | |
| <goal>shade</goal> | |
| </goals> | |
| </execution> | |
| </executions> | |
| </plugin> | |
| </plugins> | |
| </build> | |
| <repositories> | |
| <repository> | |
| <id>papermc-repo</id> | |
| <url>https://repo.papermc.io/repository/maven-public/</url> | |
| </repository> | |
| <repository> | |
| <id>sonatype</id> | |
| <url>https://oss.sonatype.org/content/groups/public/</url> | |
| </repository> | |
| <repository> | |
| <id>enginehub-maven</id> | |
| <url>https://maven.enginehub.org/repo/</url> | |
| </repository> | |
| </repositories> | |
| <dependencies> | |
| <dependency> | |
| <groupId>io.papermc.paper</groupId> | |
| <artifactId>paper-api</artifactId> | |
| <version>1.21.3-R0.1-SNAPSHOT</version> | |
| <scope>provided</scope> | |
| </dependency> | |
| <dependency> | |
| <groupId>com.sk89q.worldedit</groupId> | |
| <artifactId>worldedit-bukkit</artifactId> | |
| <version>7.2.9</version> | |
| <scope>provided</scope> | |
| </dependency> | |
| </dependencies> | |
| <properties> | |
| <java.version>21</java.version> | |
| <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding> | |
| </properties> | |
| </project> | |
| dependency-reduced-pom.xml |
| public void updateVaultState(Player player) { | ||
| try { | ||
| // Parse region coordinates from config | ||
| int upperX = fileUtil.getConfig().getInt("vault.region.upperregion.X"); | ||
| int upperY = fileUtil.getConfig().getInt("vault.region.upperregion.Y"); | ||
| int upperZ = fileUtil.getConfig().getInt("vault.region.upperregion.Z"); | ||
|
|
||
| int lowerX = fileUtil.getConfig().getInt("vault.region.lowerregion.X"); | ||
| int lowerY = fileUtil.getConfig().getInt("vault.region.lowerregion.Y"); | ||
| int lowerZ = fileUtil.getConfig().getInt("vault.region.lowerregion.Z"); | ||
|
|
||
| World world = Bukkit.getWorlds().get(0); | ||
| if (world == null) { | ||
| String errorMsg = "§cWorld not found! Stopping the update process."; | ||
| Bukkit.getLogger().warning(errorMsg); | ||
| if (player != null) player.sendMessage(errorMsg); | ||
| return; | ||
| } | ||
|
|
||
| // Normalize coordinates | ||
| int minX = Math.min(upperX, lowerX); | ||
| int maxX = Math.max(upperX, lowerX); | ||
| int minY = Math.min(upperY, lowerY); | ||
| int maxY = Math.max(upperY, lowerY); | ||
| int minZ = Math.min(upperZ, lowerZ); | ||
| int maxZ = Math.max(upperZ, lowerZ); | ||
|
|
||
| int foundCount = 0; | ||
| int updatedCount = 0; | ||
|
|
||
| // Iterate over entities in the specified region | ||
| for (Entity entity : world.getEntities()) { | ||
| if (entity instanceof ItemFrame) { | ||
| Location loc = entity.getLocation(); | ||
| if (loc.getX() >= minX && loc.getX() <= maxX && | ||
| loc.getY() >= minY && loc.getY() <= maxY && | ||
| loc.getZ() >= minZ && loc.getZ() <= maxZ) { | ||
|
|
||
| ItemFrame itemFrame = (ItemFrame) entity; | ||
| if (itemFrame.getItem() != null && itemFrame.getItem().getType() != Material.AIR) { | ||
| String itemName = itemFrame.getItem().getType().toString().toLowerCase(); | ||
|
|
||
| boolean isInVault = isItemAdded(itemName); | ||
| String frameMessage = String.format("§aItemFrame at [%d, %d, %d] has item: %s (In Vault: %s).", | ||
| loc.getBlockX(), loc.getBlockY(), loc.getBlockZ(), itemName, isInVault ? "Yes" : "No"); | ||
| Bukkit.getLogger().info(frameMessage); | ||
| if (player != null) player.sendMessage(frameMessage); | ||
|
|
||
| // Get the block behind the item frame | ||
| Location blockBehindLoc = itemFrame.getLocation().getBlock().getRelative(itemFrame.getAttachedFace()).getLocation(); | ||
| Material newMaterial = isInVault ? Material.LIME_STAINED_GLASS : Material.RED_STAINED_GLASS; | ||
| blockBehindLoc.getBlock().setType(newMaterial); | ||
|
|
||
| String updateMessage = String.format("§eBlock behind ItemFrame at [%d, %d, %d] replaced with %s.", | ||
| loc.getBlockX(), loc.getBlockY(), loc.getBlockZ(), newMaterial.name()); | ||
| Bukkit.getLogger().info(updateMessage); | ||
| if (player != null) player.sendMessage(updateMessage); | ||
|
|
||
| foundCount++; | ||
| updatedCount++; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| String summaryMessage = String.format("§aFound %d item frames and updated %d blocks in the specified region.", | ||
| foundCount, updatedCount); | ||
| Bukkit.getLogger().info(summaryMessage); | ||
| if (player != null) player.sendMessage(summaryMessage); | ||
| } catch (Exception e) { | ||
| String errorMessage = "§cAn error occurred while processing the region."; | ||
| Bukkit.getLogger().severe(errorMessage); | ||
| e.printStackTrace(); | ||
| if (player != null) player.sendMessage(errorMessage); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid scanning all world entities for region checks.
Calling world.getEntities() can be expensive on large servers, as it returns every entity in every chunk. If possible, use region-specific queries (e.g., World#getNearbyEntities or chunk-based iteration) to limit the cost.
| /** | ||
| * Calculates the collective progress of all players in the vault and generates a progress bar. | ||
| * | ||
| * @param vaultData The YamlConfiguration object for vault data. | ||
| * @param totalItems The total number of items required for the vault. | ||
| * @return A formatted progress string showing the collective percentage and progress bar. | ||
| */ | ||
| public String getProgress(YamlConfiguration vaultData, int totalItems) { | ||
| int totalCollectedItems = 0; | ||
|
|
||
| // Iterate through all player entries in the vault data to sum up collected items | ||
| for (String playerName : vaultData.getConfigurationSection("vault_data").getKeys(false)) { | ||
| List<String> playerCollectedItems = vaultData.getStringList("vault_data." + playerName + ".collected_items"); | ||
| totalCollectedItems += playerCollectedItems.size(); | ||
| } | ||
|
|
||
| // Calculate the percentage of progress for the collective effort | ||
| int progressPercentage = (int) ((double) totalCollectedItems / totalItems * 100); | ||
|
|
||
| // Create a progress bar | ||
| StringBuilder progressBar = new StringBuilder("§a["); | ||
| int barLength = 50; // Total bar length | ||
| int progressLength = (int) ((progressPercentage / 100.0) * barLength); | ||
|
|
||
| for (int i = 0; i < barLength; i++) { | ||
| if (i < progressLength) { | ||
| progressBar.append("§a|"); | ||
| } else { | ||
| progressBar.append("§7|"); | ||
| } | ||
| } | ||
| progressBar.append("§a]"); | ||
|
|
||
| // Create the progress string with total collected and total required items | ||
| String progressInfo = String.format("§e%d/%d collected", totalCollectedItems, totalItems); | ||
|
|
||
| // Return the full progress string | ||
| return progressInfo + " §f(" + progressPercentage + "% complete)\n" + progressBar; | ||
| } |
There was a problem hiding this comment.
Handle potential zero-case total items.
If totalItems is zero or unexpectedly low, (double) totalCollectedItems / totalItems can lead to division by zero. Consider defensive checks to prevent errors or odd behavior.
| String progress = vaultUtil.getProgress(vaultData, totalItems); | ||
|
|
||
| // Display progress | ||
| player.sendMessage("§aVault Progress"); | ||
| player.sendMessage("=================================="); | ||
| player.sendMessage(progress); |
There was a problem hiding this comment.
💡 Codebase verification
Division by zero is possible in getProgress method
The review comment is correct. Looking at the implementation of getProgress in VaultUtil.java, there's no check for zero total items, which could lead to an ArithmeticException when calculating the progress percentage:
int progressPercentage = (int) ((double) totalCollectedItems / totalItems * 100);The suggested fix to add a zero check in the calling code is appropriate to prevent this issue.
- Location:
src/main/java/me/benrobson/blockvault/commands/bvprogress.java - Location:
src/main/java/me/benrobson/blockvault/util/VaultUtil.java(consider adding the zero check here as well for better encapsulation)
🔗 Analysis chain
Handle zero total items scenario
If totalItems is zero, a progress calculation might result in a division by zero or an inaccurate percentage. Add a check to handle that edge case gracefully.
String progress = vaultUtil.getProgress(vaultData, totalItems);
+if (totalItems == 0) {
+ player.sendMessage("§cNo items to track yet. Progress is undefined.");
+ return true;
+}
player.sendMessage("§aVault Progress");
player.sendMessage("==================================");
player.sendMessage(progress);Let me verify the concern by checking the implementation of getProgress method and how it handles zero total items.
Let me check the implementation of getProgress in VaultUtil.java to understand how it handles the zero total items case.
🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash
# Search for getProgress method implementation
ast-grep --pattern 'getProgress($_, $_) {
$$$
}'
Length of output: 52
Script:
#!/bin/bash
# Search for any usage of getProgress method to understand the context
rg "getProgress" -A 3
Length of output: 862
Script:
#!/bin/bash
# Get the content of VaultUtil.java to examine getProgress implementation
cat src/main/java/me/benrobson/blockvault/util/VaultUtil.java
Length of output: 9936
| @Override | ||
| public boolean onCommand(CommandSender sender, Command command, String label, String[] args) { | ||
| if (!(sender instanceof Player)) { | ||
| sender.sendMessage("§cOnly players can use this command!"); | ||
| return true; | ||
| } | ||
|
|
||
| Player player = (Player) sender; | ||
|
|
||
| if (!vaultUtil.hasStarted()) { | ||
| sender.sendMessage("§cYou cannot submit items as the Vault has not been opened yet."); | ||
| return true; | ||
| } | ||
|
|
||
| ItemStack itemInHand = player.getInventory().getItemInMainHand(); | ||
|
|
||
| if (itemInHand == null || itemInHand.getType() == Material.AIR) { | ||
| player.sendMessage("§cYou are not holding any item or block!"); | ||
| return true; | ||
| } | ||
|
|
||
| // Get item display name or fallback to material name | ||
| ItemMeta meta = itemInHand.getItemMeta(); | ||
| String displayName = (meta != null && meta.hasDisplayName()) ? meta.getDisplayName() : formatMaterialName(itemInHand.getType()); | ||
| String itemName = itemInHand.getType().name().toLowerCase(); | ||
|
|
||
| // Load configuration files | ||
| File vaultDataFile = new File(plugin.getDataFolder(), "vault_data.yml"); | ||
| File vaultItemsFile = new File(plugin.getDataFolder(), "vault_items.yml"); | ||
| File configFile = new File(plugin.getDataFolder(), "config.yml"); | ||
|
|
||
| YamlConfiguration vaultData = YamlConfiguration.loadConfiguration(vaultDataFile); | ||
| YamlConfiguration vaultItems = YamlConfiguration.loadConfiguration(vaultItemsFile); | ||
| YamlConfiguration config = YamlConfiguration.loadConfiguration(configFile); | ||
|
|
||
| // Check if item has already been collected globally | ||
| String collector = vaultData.getString("vault_data.global_collected_items." + itemName); | ||
| if (collector != null) { | ||
| player.sendMessage("§cItem " + displayName + " has already been collected by " + collector + "!"); | ||
| return true; | ||
| } | ||
|
|
||
| // Add item to player's collected items | ||
| List<String> playerItems = vaultData.getStringList("vault_data." + player.getName() + ".collected_items"); | ||
| if (playerItems.contains(itemName)) { | ||
| player.sendMessage("§cYou have already collected " + displayName + "!"); | ||
| return true; | ||
| } | ||
|
|
||
| // Determine the item's point category and corresponding points | ||
| String category = vaultItems.getString("items." + itemName, "COMMON"); // Default to COMMON | ||
| int pointsPerCategory = config.getInt("points." + category, 1); // Default to 1 point | ||
|
|
||
| playerItems.add(itemName); | ||
|
|
||
| vaultData.set("vault_data." + player.getName() + ".collected_items", playerItems); | ||
|
|
||
| // Update player points | ||
| int currentPoints = vaultData.getInt("vault_data." + player.getName() + ".points", 0); | ||
| vaultData.set("vault_data." + player.getName() + ".points", currentPoints + pointsPerCategory); | ||
|
|
||
| // Update global collected items | ||
| vaultData.set("vault_data.global_collected_items." + itemName, player.getName()); | ||
|
|
||
| try { | ||
| vaultData.save(vaultDataFile); | ||
|
|
||
| // Remove one item from the player's hand | ||
| if (itemInHand.getAmount() > 1) { | ||
| itemInHand.setAmount(itemInHand.getAmount() - 1); | ||
| } else { | ||
| player.getInventory().setItemInMainHand(null); | ||
| } | ||
|
|
||
| player.sendMessage("§aSuccessfully added " + displayName + " to your vault! (§b+" + pointsPerCategory + " points§a)"); | ||
| } catch (IOException e) { | ||
| player.sendMessage("§cFailed to save collected item. Please try again."); | ||
| e.printStackTrace(); | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider splitting logic into smaller methods
The command method handles multiple tasks—checking preconditions, reading config files, updating data, removing items from inventory. For readability and maintainability, consider splitting these tasks among smaller private methods (e.g., validatePreconditions(), processSubmission(), updatePlayerInventory()).
|
|
||
| @Override | ||
| public void onEnable() { | ||
| getConsoleSender().sendMessage(getConfig().get("lang.prefix") + "§aPlugin is now enabled!"); |
There was a problem hiding this comment.
Add null check for config prefix
The getConfig().get("lang.prefix") call could return null if the prefix is not defined in config.yml. Add a null check or provide a default value.
- getConsoleSender().sendMessage(getConfig().get("lang.prefix") + "§aPlugin is now enabled!");
+ String prefix = getConfig().getString("lang.prefix", "§8§l[§3BD§8§l]");
+ getConsoleSender().sendMessage(prefix + "§aPlugin is now enabled!");📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| getConsoleSender().sendMessage(getConfig().get("lang.prefix") + "§aPlugin is now enabled!"); | |
| String prefix = getConfig().getString("lang.prefix", "§8§l[§3BD§8§l]"); | |
| getConsoleSender().sendMessage(prefix + "§aPlugin is now enabled!"); |
| // Register events | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
Event listeners are missing and need to be implemented
The review comment is correct. After examining the main plugin class and searching through the codebase, there are no event listeners implemented despite the comment indicating they should be registered. This is important because:
- The main class
BlockVault.javahas a comment "Register events" but no event registration code follows - No event listener classes were found in the codebase (no classes implementing/extending Listener)
- No
@EventHandlerannotations were found - The plugin seems to be a vault challenge plugin that would likely need to listen to various game events to function properly
You should:
- Create event listener class(es) implementing
org.bukkit.event.Listener - Implement necessary event handlers with
@EventHandlerannotations - Register the listeners using
getServer().getPluginManager().registerEvents(listener, this)in the main class
🔗 Analysis chain
Implement event listeners
There's a comment indicating event listeners should be registered, but none are implemented. This could lead to missing crucial game events needed for the vault challenge.
Let's check if any event listeners are defined in the codebase:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for event listener implementations
rg -l "implements Listener|extends Listener" --type java
Length of output: 58
Script:
#!/bin/bash
# Let's try a different approach to find event-related code
# 1. Search for event handler annotations
rg "@EventHandler" --type java
# 2. Search for any files in 'events' directories
fd -t f -e java . events/
# 3. Search for event-related method names
rg "on[A-Z][a-zA-Z]+Event" --type java
Length of output: 194
Script:
#!/bin/bash
# Let's check the plugin.yml to understand the plugin structure
# and look for any event registration in the main class
cat src/main/resources/plugin.yml
# Check the main class content
cat src/main/java/me/benrobson/blockvault/BlockVault.java
Length of output: 2044
| ## Features | ||
| ### Interactive Block Displays | ||
| * Sections dedicated to specific block categories (e.g., wood, ores, decorative blocks). | ||
| * Blocks and items can be placed on predefined slots with item frames or display stands, indicating where each item belongs and can be contributed to. | ||
|
|
||
| ### Points System | ||
| * Players earn points for each block they unlock and contribute to the museum. | ||
| * The points awarded vary based on the rarity of the block/item: | ||
| * **Common Blocks:** 1 point per block. | ||
| * **Uncommon Blocks:** 5 points per block. | ||
| * **Rare Blocks/Items:** 10+ points per block. | ||
| * A running total of points is tracked for each player which can be found using `/bvleaderboard`, contributing to their leaderboard position. | ||
|
|
||
| ### Tracking Progress | ||
| * Leaderboard displays the top contributors and their points when using `/bvleaderboard`. | ||
| * A progress bar or percentage indicator shows how close the server is to completing the collection using `/bvprogress`. | ||
|
|
||
| ### Progress Leaderboard | ||
| * Progress can be viewed in real-time using a hologram display, showcasing the top contributors and collection milestones. | ||
| * Use the `/bvleaderboard` command to see a detailed leaderboard of top contributors, including points and blocks contributed. | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add installation and configuration sections
The documentation should include setup instructions and configuration guide to help server administrators.
Consider adding these sections before the Features section:
## Installation
1. Download the latest release from [Releases]
2. Place the JAR file in your server's `plugins` folder
3. Restart your server
4. Configure the plugin in `plugins/BlockVault/config.yml`
## Configuration
The plugin can be configured through `config.yml`:
### Vault Settings
- `vault.region`: Define the physical boundaries of your vault
- `vault.excludeditems`: List of items that cannot be submitted
- `vault.updateinterval`: How often the vault state updates (in seconds)
### Points System
- Configure points for different item rarities
- Customize the scoring system for your server
|
Awaiting for events team to test. |
…ing and file utilities
…plugin; enhance plugin.yml with descriptions and permissions.
…ermission checks Enhance ScheduleUtil to improve task scheduling with configurable startup delay Revamp VaultUtil to streamline vault state updates and remove deprecated methods Update config.yml to reflect new database settings and remove obsolete vault settings Add Database class for managing MySQL connections and operations with HikariCP Implement Manifest and TargetEntry classes for structured target data management Introduce HeadUtil for handling player head placements and profile serialization
…terService and related listeners
…nds and permissions in plugin.yml
…cks; implement StartupValidation utility
…ion tracking and monthly leaderboard functionality
… async reload for chapter dates
…ted datapack files
…ary section assignment
…consumption; improve error handling in database queries and display service
…ronmental griefing and improve shelf cell validation in startup checks
1 participant
Summary by CodeRabbit
Release Notes
New Features
Commands
/bvstart: Begin the vault challenge/bvsubmit: Submit items to the vault/bvprogress: Check vault progress/bvleaderboard: View top contributorsDocumentation
Chores