[WIP] Make the plugin folia-compatible #844
Conversation
Jakubk15
changed the title
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
WalkthroughThe changes in this pull request involve updates to version constants, the addition of a new Maven repository, modifications to dependency declarations in the Changes
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: 7
🧹 Outside diff range and nitpick comments (6)
buildSrc/src/main/kotlin/eternalcore-repositories.gradle.kts (1)
11-11: LGTM! Consider adding a comment for clarity.The addition of the Eternal Code snapshots repository is appropriate and aligns with the project's needs. This will allow access to the latest development versions of Eternal Code dependencies, which may be necessary for Folia compatibility.
Consider adding a comment to explain why this snapshot repository is needed:
+ // Snapshot repository for Eternal Code dependencies (required for Folia compatibility) maven { url = uri("https://repo.eternalcode.pl/snapshots/") }This will help other developers understand the purpose of this repository in the future.
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/SkullCommand.java (1)
Line range hint
1-58: LGTM! Consider adding error handling for skull retrieval.The overall implementation of the
SkullCommandis well-structured and follows good practices:
- Proper use of dependency injection
- Clear command and permission annotations
- Asynchronous skull data retrieval for better performance
- Handling different scenarios (player holding a skull or not)
Consider adding error handling for the case when skull data retrieval fails. For example:
@Execute @DescriptionDocs(description = "Gives you a skull of player", arguments = "<player>") void execute(@Context Player sender, @Arg(SkullNicknameArgument.KEY) String name) { - this.skullAPI.acceptSyncSkullData(name, skull -> { + this.skullAPI.acceptSyncSkullData(name, skull -> { ItemStack namedSkull = ItemBuilder.skull() .name(Component.text(name)) .texture(skull.getValue()) .build(); ItemStack mainHand = sender.getInventory().getItemInMainHand(); if (mainHand.getType() == Material.PLAYER_HEAD) { mainHand.setItemMeta(namedSkull.getItemMeta()); } else { sender.getInventory().addItem(namedSkull); } this.noticeService.create() .notice(translation -> translation.item().skullMessage()) .placeholder("{SKULL}", name) .player(sender.getUniqueId()) .send(); + }, error -> { + this.noticeService.create() + .notice(translation -> translation.item().skullRetrievalError()) + .placeholder("{SKULL}", name) + .player(sender.getUniqueId()) + .send(); }); }This addition would improve the user experience by notifying the player when the skull data cannot be retrieved.
eternalcore-core/build.gradle.kts (1)
32-35: LGTM! Consider grouping related dependencies.The addition of EternalCode Commons dependencies (bukkit, folia, and adventure) aligns well with the PR objective of enhancing Folia compatibility. Good job on using a common version variable for easier management.
For improved readability and consistency, consider grouping these related dependencies together, possibly under a comment. For example:
// EternalCode Commons dependencies implementation("com.eternalcode:eternalcode-commons-bukkit:${Versions.ETERNALCODE_COMMONS}") implementation("com.eternalcode:eternalcode-commons-folia:${Versions.ETERNALCODE_COMMONS}") implementation("com.eternalcode:eternalcode-commons-adventure:${Versions.ETERNALCODE_COMMONS}")This grouping would make it easier to identify and manage these related dependencies in the future.
eternalcore-core/src/main/java/com/eternalcode/core/scheduler/SchedulerSetup.java (1)
14-14: Consider renaming theFOLIAconstant for clarity.The constant
FOLIArepresents the fully qualified class name of the Folia server. For improved readability and to clearly convey its purpose, consider renaming it toFOLIA_SERVER_CLASSorFOLIA_CLASS_NAME.eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/weather/RainCommand.java (1)
23-25: Consider adding nullability annotations to constructor parametersFor improved null-safety and clarity, consider adding
@NotNullannotations to the constructor parametersnoticeServiceandscheduler, if your project utilizes nullability annotations. This helps enforce that these dependencies should not be null.eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/weather/SunCommand.java (1)
49-53: Consider extracting magic numbers into constants for better readabilityThe expression
20 * 60 * 10(representing 10 minutes in ticks) can be replaced with a named constant to improve readability and maintainability.Apply this diff to enhance readability:
+ private static final int CLEAR_WEATHER_DURATION_TICKS = 20 * 60 * 10; // 10 minutes private void setSun(Viewer viewer, World world) { this.scheduler.sync(() -> { - world.setClearWeatherDuration(20 * 60 * 10); + world.setClearWeatherDuration(CLEAR_WEATHER_DURATION_TICKS); world.setStorm(false); world.setThundering(false); }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (13)
- buildSrc/src/main/kotlin/Versions.kt (2 hunks)
- buildSrc/src/main/kotlin/eternalcore-repositories.gradle.kts (1 hunks)
- eternalcore-core/build.gradle.kts (1 hunks)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/SkullCommand.java (1 hunks)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/sudo/SudoCommand.java (3 hunks)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/time/DayCommand.java (2 hunks)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/time/NightCommand.java (2 hunks)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/time/TimeCommand.java (4 hunks)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/weather/RainCommand.java (2 hunks)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/weather/SunCommand.java (3 hunks)
- eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/weather/ThunderCommand.java (2 hunks)
- eternalcore-core/src/main/java/com/eternalcode/core/scheduler/SchedulerSetup.java (2 hunks)
- eternalcore-plugin/build.gradle.kts (1 hunks)
🧰 Additional context used
🔇 Additional comments (29)
buildSrc/src/main/kotlin/Versions.kt (2)
6-6: Verify the use of SNAPSHOT version for ETERNALCODE_COMMONSThe version of ETERNALCODE_COMMONS has been updated to "1.1.4-SNAPSHOT". While this allows access to the latest development changes, using SNAPSHOT versions in production can lead to inconsistent builds.
Please confirm if using a SNAPSHOT version is intentional. If this is for testing purposes, consider using a stable version (e.g., "1.1.4") for production builds.
16-16: Approve stable version, but verify compatibilityThe change from "4.18.0-SNAPSHOT" to "4.17.0" for ADVENTURE_TEXT_MINIMESSAGE is good as it moves to a stable version. This should improve build consistency and stability.
However, please ensure that this downgrade doesn't remove any features or introduce compatibility issues with the rest of the codebase. Review the changelog between these versions to confirm no critical changes are lost.
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/item/SkullCommand.java (1)
7-7: LGTM! Verify dependency update project-wide.The import statement update for the
Commandannotation is correct and doesn't affect the functionality of theSkullCommandclass. This change likely reflects an update in the project's dependency structure.To ensure consistency across the project, please run the following script to verify that all imports of the
Commandannotation have been updated:If the script returns any occurrences of the old import statement, those files should be updated as well.
✅ Verification successful
All import statements updated correctly.
TheCommandannotation import has been successfully updated across the entire project with no remaining instances of the old import statement.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all imports of the Command annotation across the project. # Test: Search for old and new import statements echo "Old import statements:" rg --type java "import dev.rollczi.litecommands.command.Command;" echo "New import statements:" rg --type java "import dev.rollczi.litecommands.annotations.command.Command;"Length of output: 12359
eternalcore-core/build.gradle.kts (2)
32-35: Summary of build.gradle.kts changesThe changes to this file primarily involve updating dependencies to support Folia compatibility. Here's a summary of the key points:
- New EternalCode Commons dependencies have been added, which aligns with the PR objectives.
- The removal of certain libraries from the
eternalShadowblock represents a significant change in dependency management.To ensure these changes are implemented correctly:
- Verify that the final artifact still contains all necessary classes (see the verification script in the previous comment).
- Consider grouping related dependencies for improved readability.
- Update project documentation to reflect these changes in dependency management.
- Test thoroughly to ensure that the plugin's functionality remains intact with these new dependency configurations.
Overall, these changes seem to be a step in the right direction for Folia compatibility, but careful verification is needed to prevent potential issues.
32-35: Verify the impact of dependency management changes.The removal of
com.eternalcode:eternalcode-commons-bukkitandcom.eternalcode:eternalcode-commons-adventurefrom theeternalShadowblock, combined with their addition as implementation dependencies, represents a significant change in how these libraries are managed.Please ensure that:
- This change doesn't negatively impact the plugin's functionality or deployment.
- The final artifact still contains all necessary classes from these libraries.
- There are no conflicts with other dependencies that might be shadowing these libraries.
Additionally, consider updating the project documentation to reflect this change in dependency management, explaining the rationale behind it for future maintainers.
To verify the impact of these changes, you can run the following script:
This script will help ensure that the necessary classes from the removed libraries are still present in the final artifact.
eternalcore-core/src/main/java/com/eternalcode/core/scheduler/SchedulerSetup.java (1)
18-19: Ensure theschedulermethod change maintains backward compatibility.By altering the
scheduler(Plugin plugin)method to returngetAdaptiveScheduler(plugin), verify that this modification doesn't introduce unexpected behavior in components relying on the scheduler. It's important to ensure that existing functionality remains unaffected.To confirm seamless operation, consider testing the scheduler on both Folia and non-Folia servers to ensure consistent behavior across different environments.
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/time/DayCommand.java (5)
4-4: ImportingSchedulerdependencyThe import statement
import com.eternalcode.commons.scheduler.Scheduler;is correctly added to provide access to theSchedulerclass.
9-9: ImportingCommandannotationThe import statement
import dev.rollczi.litecommands.annotations.command.Command;is correctly added to support the@Commandannotation.
20-20: Declaration ofSchedulerfieldThe field
private final Scheduler scheduler;is properly declared to store the injectedSchedulerinstance.
41-43: Ensuring thread-safe world time modificationWrapping
world.setTime(100);insidethis.scheduler.sync(() -> { ... });ensures that the time change is executed on the main thread, complying with Folia's threading model and preventing potential concurrency issues.
23-25: Constructor updated to injectSchedulerdependencyThe constructor is correctly updated to include
Scheduler scheduleras a parameter, and the field is properly initialized withthis.scheduler = scheduler;.Ensure that all instantiations of
DayCommandare updated to include the newSchedulerparameter. Run the following script to verify:✅ Verification successful
To further verify the instantiations of
DayCommand, please run the following script:
To ensure all instances of
DayCommandare updated with theSchedulerparameter, please run the following script:
All instantiations of
DayCommandare correctly updated with theSchedulerparameter.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all occurrences where `DayCommand` is instantiated # Test: Search for 'new DayCommand(' in the codebase rg --type java 'new\s+DayCommand\s*\('Length of output: 40
Script:
#!/bin/bash # Description: Find all occurrences where `DayCommand` is instantiated without the `Scheduler` parameter # Step 1: Find all instantiations of DayCommand fd 'DayCommand\.java' | xargs rg 'new\s+DayCommand\s*\(' # Step 2: Optional - If detailed parameter checks are needed, consider inspecting the parameters passed.Length of output: 58
Script:
#!/bin/bash # Description: Find all occurrences where `DayCommand` is instantiated across all Java files rg --type java 'new\s+DayCommand\s*\('Length of output: 40
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/time/NightCommand.java (3)
4-4: Import Scheduler class for task schedulingThe import statement correctly adds the
Schedulerclass, which is necessary for managing scheduled tasks.
20-20: Add Scheduler field to manage schedulingAdding the
Schedulerfield enables the class to schedule tasks appropriately.
23-25: Update constructor to inject Scheduler dependencyThe constructor now accepts a
Schedulerparameter alongside theNoticeService, ensuring that scheduling capabilities are properly injected.eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/weather/ThunderCommand.java (4)
4-4: Import Statement for Scheduler ConfirmedThe import of
Scheduleris appropriate and necessary for the added asynchronous functionality.
20-20: Addition of Scheduler DependencyDeclaring
private final Scheduler scheduler;correctly introduces the scheduler as a dependency.
41-44: Ensuring Thread Safety with Asynchronous ExecutionWrapping the weather-changing operations inside
scheduler.sync(() -> { ... })ensures thatworld.setStorm(true);andworld.setThundering(true);are executed on the main server thread, which is crucial for thread safety in Bukkit API operations. Good practice in handling asynchronous command execution.
23-25: Constructor Updated for Dependency InjectionThe constructor now includes
Scheduler scheduler, and the assignmentthis.scheduler = scheduler;correctly integrates the new dependency.To ensure that all instances of
ThunderCommandare properly instantiated with the newSchedulerdependency, verify that any manual instantiations are updated accordingly. Run the following script to check for potential issues:eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/weather/SunCommand.java (2)
24-24: Addition of Scheduler dependency is correctThe
Scheduleris properly injected and assigned, enabling asynchronous execution of weather-setting operations.Also applies to: 27-29
49-53: Verify necessity of wrapping weather-setting code inscheduler.sync()Since command methods annotated with
@Executeare typically executed on the main server thread, wrapping the weather-setting code inscheduler.sync()may be unnecessary. Please verify if this is required to avoid redundant scheduling.eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/sudo/SudoCommand.java (6)
5-5: Import Scheduler correctlyThe import statement properly adds the
Schedulerclass, which is necessary for scheduling tasks.
24-24: Add Scheduler field for dependencyThe
schedulerfield is correctly declared and marked asfinal, adhering to good practice for dependencies.
27-30: Update constructor to include Scheduler dependencyThe constructor now properly includes the
Schedulerparameter, ensuring that it is injected and available within the class.
37-37: Ensure thread-safe command executionWrapping the command execution in
scheduler.syncensures that commands are executed on the main server thread, which is essential for thread safety in the Bukkit API.
45-45: Ensure thread-safe execution of player commandsBy wrapping the command dispatch in
scheduler.sync, the command executed as the target player runs on the main thread, which is required for thread safety.
59-59: Exclude the command executor from receiving sudo spy notificationsThe added filter ensures that the player executing the sudo command does not receive their own spy message, improving usability.
eternalcore-core/src/main/java/com/eternalcode/core/feature/essentials/time/TimeCommand.java (3)
24-26: Constructor updated to include Scheduler dependencyThe constructor now correctly injects the
Scheduler, allowing time-related commands to schedule tasks appropriately. This enhances compatibility with Folia by ensuring tasks run on the main thread when necessary.
38-40: Ensuring thread safety in theaddmethodBy wrapping
world.setTime(world.getTime() + time);insidethis.scheduler.sync, the time addition operation is correctly scheduled to execute on the main thread. This is essential for thread safety and complies with Folia's requirements for asynchronous task handling.
58-60: Ensuring thread safety in thesetmethodSimilarly, wrapping
world.setTime(time);insidethis.scheduler.syncensures that the time setting operation runs synchronously on the main thread. This modification enhances thread safety and maintains compatibility with the Bukkit API under Folia.
|
|
||
| foliaSupported = true |
There was a problem hiding this comment.
💡 Codebase verification
Potential Folia Compatibility Issue
While foliaSupported = true has been added to the pluginYml, the plugin does not currently utilize any Folia-specific APIs. This may lead to compatibility issues when running on Folia servers.
- Action Required: Implement Folia-specific API usage to ensure full compatibility.
- Recommendation: Review and integrate Folia's threaded regions and other relevant APIs within the plugin's codebase.
🔗 Analysis chain
LGTM: Folia support declaration added.
The addition of foliaSupported = true in the pluginYml block aligns with the PR objective of making the plugin Folia-compatible. This change will be reflected in the generated plugin.yml file, indicating to Folia servers that this plugin is compatible.
To ensure full Folia compatibility:
-
Verify that the plugin's code has been updated to be Folia-aware, using appropriate APIs and avoiding operations that might not be thread-safe in Folia's architecture.
-
Test the plugin on a Folia server to confirm that it functions correctly in a regionalized multithreading environment.
Run the following script to check for Folia-specific API usage:
This script will help identify if Folia-specific APIs are being used in the plugin code. If no results are shown, it might indicate that additional code changes are needed to fully support Folia.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for Folia-specific API usage in the plugin code
# Test: Search for Folia-specific imports and method calls
rg --type java 'import.*io\.papermc\.paper\.threadedregions' -g '!build/'
rg --type java '(getRegionizedServer|getRegionizedScheduler|getRegionizedPlayerList)' -g '!build/'
echo "If no results are shown, the plugin might not be using Folia-specific APIs. Please review the code to ensure Folia compatibility."
Length of output: 445
| @@ -1,6 +1,7 @@ | |||
| package com.eternalcode.core.scheduler; | |||
|
|
|||
| import com.eternalcode.commons.bukkit.scheduler.BukkitSchedulerImpl; | |||
| import com.eternalcode.commons.folia.scheduler.FoliaSchedulerImpl; | |||
There was a problem hiding this comment.
Avoid direct references to FoliaSchedulerImpl to prevent class loading issues on non-Folia servers.
While the isFolia() method checks for the presence of Folia, directly referencing FoliaSchedulerImpl can cause a NoClassDefFoundError at runtime on non-Folia servers. This occurs because the class loader attempts to load all referenced classes when initializing SchedulerSetup, even if those classes are not used at runtime.
To prevent this, use reflection to instantiate FoliaSchedulerImpl only when Folia is detected. Here's how you can modify your code:
public static Scheduler getAdaptiveScheduler(Plugin plugin) {
if (isFolia()) {
- return new FoliaSchedulerImpl(plugin);
+ try {
+ Class<?> clazz = Class.forName("com.eternalcode.commons.folia.scheduler.FoliaSchedulerImpl");
+ Constructor<?> constructor = clazz.getConstructor(Plugin.class);
+ return (Scheduler) constructor.newInstance(plugin);
+ } catch (Exception e) {
+ throw new RuntimeException("Failed to instantiate FoliaSchedulerImpl", e);
+ }
}
else {
return new BukkitSchedulerImpl(plugin);
}
}This adjustment ensures that FoliaSchedulerImpl is only loaded when Folia is present, preventing class loading errors on non-Folia servers.
Also applies to: 23-24
| private static boolean isFolia() { | ||
| try { | ||
| Class.forName(FOLIA); | ||
| return true; | ||
| } | ||
| catch (ClassNotFoundException exception) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Cache the result of isFolia() to enhance performance.
The isFolia() method performs a class lookup each time it's called, which could impact performance if called frequently. To optimize, cache the result so the class check occurs only once.
Here's how you can modify the code:
+ private static Boolean foliaDetected = null;
private static boolean isFolia() {
+ if (foliaDetected != null) {
+ return foliaDetected;
+ }
try {
Class.forName(FOLIA);
- return true;
+ foliaDetected = true;
}
catch (ClassNotFoundException exception) {
- return false;
+ foliaDetected = false;
}
+ return foliaDetected;
}📝 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.
| private static boolean isFolia() { | |
| try { | |
| Class.forName(FOLIA); | |
| return true; | |
| } | |
| catch (ClassNotFoundException exception) { | |
| return false; | |
| } | |
| private static Boolean foliaDetected = null; | |
| private static boolean isFolia() { | |
| if (foliaDetected != null) { | |
| return foliaDetected; | |
| } | |
| try { | |
| Class.forName(FOLIA); | |
| foliaDetected = true; | |
| } | |
| catch (ClassNotFoundException exception) { | |
| foliaDetected = false; | |
| } | |
| return foliaDetected; | |
| } |
| this.scheduler.sync(() -> { | ||
| world.setTime(13700); | ||
| }); |
There was a problem hiding this comment.
Ensure notice is sent after time is set
Currently, the notice is sent immediately after scheduling the time change, which might result in the notice being delivered before the world time is actually set due to asynchronous execution. To guarantee that the viewer receives the notice after the time has been updated, consider moving the notice-sending code inside the scheduler.sync block.
Apply this diff to adjust the code:
private void setNight(Viewer viewer, World world) {
this.scheduler.sync(() -> {
world.setTime(13700);
+ this.noticeService.create()
+ .viewer(viewer)
+ .placeholder("{WORLD}", world.getName())
+ .notice(translation -> translation.timeAndWeather().timeSetNight())
+ .send();
});
-
- this.noticeService.create()
- .viewer(viewer)
- .placeholder("{WORLD}", world.getName())
- .notice(translation -> translation.timeAndWeather().timeSetNight())
- .send();
}📝 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.
| this.scheduler.sync(() -> { | |
| world.setTime(13700); | |
| }); | |
| private void setNight(Viewer viewer, World world) { | |
| this.scheduler.sync(() -> { | |
| world.setTime(13700); | |
| this.noticeService.create() | |
| .viewer(viewer) | |
| .placeholder("{WORLD}", world.getName()) | |
| .notice(translation -> translation.timeAndWeather().timeSetNight()) | |
| .send(); | |
| }); | |
| } |
| this.scheduler.sync(() -> { | ||
| world.setStorm(true); | ||
| world.setThundering(false); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider handling exceptions within the scheduled task
To enhance robustness, consider adding exception handling within the scheduler.sync() lambda to catch and log any unexpected exceptions that may occur during world.setStorm(true) and world.setThundering(false). This ensures any issues are appropriately logged and do not silently fail.
Apply this diff to add exception handling:
private void setRain(Viewer viewer, World world) {
this.scheduler.sync(() -> {
+ try {
world.setStorm(true);
world.setThundering(false);
+ } catch (Exception e) {
+ e.printStackTrace();
+ }
});
this.noticeService.create()
.viewer(viewer)
.placeholder("{WORLD}", world.getName())
.notice(translation -> translation.timeAndWeather().weatherSetRain())
.send();
}📝 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.
| this.scheduler.sync(() -> { | |
| world.setStorm(true); | |
| world.setThundering(false); | |
| }); | |
| this.scheduler.sync(() -> { | |
| try { | |
| world.setStorm(true); | |
| world.setThundering(false); | |
| } catch (Exception e) { | |
| e.printStackTrace(); | |
| } | |
| }); |
| this.sendSudoSpy(viewer, command); | ||
| } | ||
|
|
||
| @Execute | ||
| @Permission("eternalcore.sudo.player") | ||
| @DescriptionDocs(description = "Execute command as player", arguments = "<player> <command>") | ||
| void player(@Context Viewer viewer, @Arg Player target, @Join String command) { | ||
| this.server.dispatchCommand(target, command); | ||
| this.scheduler.sync(() -> this.server.dispatchCommand(target, command)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider handling command execution result
Similarly, consider checking the result of dispatchCommand when executing commands as a player to handle any failures.
Apply this diff to handle the command execution result:
-this.scheduler.sync(() -> this.server.dispatchCommand(target, command));
+this.scheduler.sync(() -> {
+ boolean success = this.server.dispatchCommand(target, command);
+ if (!success) {
+ viewer.sendMessage("Command execution failed.");
+ }
+});📝 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.
| this.scheduler.sync(() -> this.server.dispatchCommand(target, command)); | |
| this.scheduler.sync(() -> { | |
| boolean success = this.server.dispatchCommand(target, command); | |
| if (!success) { | |
| viewer.sendMessage("Command execution failed."); | |
| } | |
| }); |
| } | ||
|
|
||
| @Execute(name = "-console") | ||
| @Permission("eternalcore.sudo.console") | ||
| @DescriptionDocs(description = "Execute command as console", arguments = "<command>") | ||
| void console(@Context Viewer viewer, @Join String command) { | ||
| this.server.dispatchCommand(this.server.getConsoleSender(), command); | ||
| this.scheduler.sync(() -> this.server.dispatchCommand(this.server.getConsoleSender(), command)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider handling command execution result
The dispatchCommand method returns a boolean indicating whether the command was successful. Consider checking this return value to handle unsuccessful command executions and provide feedback to the user.
Apply this diff to handle the command execution result:
-this.scheduler.sync(() -> this.server.dispatchCommand(this.server.getConsoleSender(), command));
+this.scheduler.sync(() -> {
+ boolean success = this.server.dispatchCommand(this.server.getConsoleSender(), command);
+ if (!success) {
+ viewer.sendMessage("Command execution failed.");
+ }
+});📝 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.
| this.scheduler.sync(() -> this.server.dispatchCommand(this.server.getConsoleSender(), command)); | |
| this.scheduler.sync(() -> { | |
| boolean success = this.server.dispatchCommand(this.server.getConsoleSender(), command); | |
| if (!success) { | |
| viewer.sendMessage("Command execution failed."); | |
| } | |
| }); |
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores