Feat/command feedback helpers #7
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
|
Warning Rate limit exceeded@visz11 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 1 minutes and 32 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @visz11, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the command API by introducing a set of standardized helper methods for sending command feedback. These additions aim to simplify and unify the process of communicating command execution results (success, failure, or general messages) to players and administrators, ensuring consistency with vanilla Minecraft's behavior regarding message visibility and game rules. The changes include both the API definition and its server-side implementation, along with updated test cases to validate the new functionality. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Sequence DiagramShows the new flow where a command uses CommandSourceStack.sendSuccess/sendFailure/sendToTarget, which the PaperCommandSourceStack defaults forward to the Minecraft server (NMS) via PaperAdventure, and the server then delivers the system message to the target and optionally informs admins/console per vanilla game rules. sequenceDiagram
participant Player/Console
participant Command
participant CommandSourceStack (API)
participant PaperCommandSourceStack (impl)
participant MinecraftServer (NMS)
Player/Console->>Command: execute command
Command->>CommandSourceStack: sendSuccess(message, allowInformingAdmins)
CommandSourceStack->>PaperCommandSourceStack: default impl forwards call
PaperCommandSourceStack->>MinecraftServer (NMS): sendSuccess/sendSystemMessage(message via PaperAdventure)
MinecraftServer (NMS)-->>Player/Console & Admins: deliver system message (respecting GameRule SEND_COMMAND_FEEDBACK / LOG_ADMIN_COMMANDS)
Generated by CodeAnt AI |
codeant-ai
bot
added
the
size:M
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces helpful methods (sendToTarget, sendSuccess, sendFailure) on CommandSourceStack for sending command feedback, which is a great addition for plugin developers. The implementation and the updates to the test plugin are well done. I have one point of feedback regarding the Javadoc for the sendFailure method to ensure its behavior is clear to API users.
|
|
||
| /** | ||
| * Sends a system message indicating a failed command execution to the {@link #getSender()}. | ||
| * Does not apply red styling to the message as vanilla does to allow for custom failure message styling. |
There was a problem hiding this comment.
The Javadoc here states that this method "Does not apply red styling to the message". However, the default implementation in PaperCommandSourceStack calls getHandle().sendFailure(...), which delegates to vanilla's logic. For player command senders, vanilla's sendFailure implementation does apply red styling to the message.
This makes the Javadoc misleading for API consumers. Please update the documentation to accurately reflect that red styling is applied for players, consistent with vanilla behavior. Alternatively, if the intent is truly to avoid styling, the implementation in PaperCommandSourceStack would need to be adjusted.
Nitpicks 🔍
|
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| * | ||
| * @param message the message to send | ||
| */ | ||
| void sendToTarget(ComponentLike message); |
There was a problem hiding this comment.
Suggestion: The new abstract sendToTarget method on this widely-exposed interface can cause AbstractMethodError at runtime for any existing custom CommandSourceStack implementations compiled against an earlier API version, because callers may invoke this method on instances that don't implement it; providing a default implementation preserves binary compatibility while still offering the intended behavior of sending to the player executor when present, otherwise to the sender. [possible bug]
Severity Level: Critical 🚨
| void sendToTarget(ComponentLike message); | |
| default void sendToTarget(ComponentLike message) { | |
| Entity executor = this.getExecutor(); | |
| if (executor instanceof Player player) { | |
| player.sendMessage(message.asComponent()); | |
| } else { | |
| this.getSender().sendMessage(message.asComponent()); | |
| } | |
| } |
Why it matters? ⭐
Adding non-default methods to a widely implemented public interface is a binary compatibility break: any precompiled implementation of CommandSourceStack that doesn't include this new method will throw AbstractMethodError when callers invoke it. Providing a default implementation preserves runtime compatibility while still offering the convenience helper. The suggested default implementation is straightforward and follows the documented intent: if the executor is a Player send it there, otherwise send to the sender.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** paper-api/src/main/java/io/papermc/paper/command/brigadier/CommandSourceStack.java
**Line:** 80:80
**Comment:**
*Possible Bug: The new abstract `sendToTarget` method on this widely-exposed interface can cause `AbstractMethodError` at runtime for any existing custom `CommandSourceStack` implementations compiled against an earlier API version, because callers may invoke this method on instances that don't implement it; providing a default implementation preserves binary compatibility while still offering the intended behavior of sending to the player executor when present, otherwise to the sender.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| * @param message the message to send | ||
| * @param allowInformingAdmins whether admins and console may be informed of this success | ||
| */ | ||
| void sendSuccess(ComponentLike message, boolean allowInformingAdmins); |
There was a problem hiding this comment.
Suggestion: Adding the abstract sendSuccess(ComponentLike, boolean) helper to this interface without a default implementation is a binary-incompatible change that can trigger AbstractMethodError when Paper code or plugins call it on pre-existing CommandSourceStack implementations that were compiled before this method existed; a conservative default that at least sends the message to the sender avoids this runtime failure. [possible bug]
Severity Level: Critical 🚨
| void sendSuccess(ComponentLike message, boolean allowInformingAdmins); | |
| default void sendSuccess(ComponentLike message, boolean allowInformingAdmins) { | |
| this.getSender().sendMessage(message.asComponent()); | |
| } |
Why it matters? ⭐
Same ABI concern as above: adding an abstract method to an existing interface risks AbstractMethodError for precompiled implementors. A conservative default that at minimum delivers the message to the sender prevents runtime failures. Note that the PR's JavaDoc describes additional semantics (game rules, admin informing), so the default should be minimal and safe; full vanilla semantics can be implemented in the platform's core implementation rather than forcing all implementors to do so.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** paper-api/src/main/java/io/papermc/paper/command/brigadier/CommandSourceStack.java
**Line:** 92:92
**Comment:**
*Possible Bug: Adding the abstract `sendSuccess(ComponentLike, boolean)` helper to this interface without a default implementation is a binary-incompatible change that can trigger `AbstractMethodError` when Paper code or plugins call it on pre-existing `CommandSourceStack` implementations that were compiled before this method existed; a conservative default that at least sends the message to the sender avoids this runtime failure.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.| * | ||
| * @param message the message to send | ||
| */ | ||
| void sendFailure(ComponentLike message); |
There was a problem hiding this comment.
Suggestion: Similarly, introducing sendFailure(ComponentLike) as an abstract method on this existing interface may break older CommandSourceStack implementations at runtime with AbstractMethodError when commands call it, so it should provide a default implementation (e.g. sending the message to the sender) to keep the API binary-compatible. [possible bug]
Severity Level: Critical 🚨
| void sendFailure(ComponentLike message); | |
| default void sendFailure(ComponentLike message) { | |
| this.getSender().sendMessage(message.asComponent()); | |
| } |
Why it matters? ⭐
Again, adding this non-default method breaks binary compatibility for preexisting implementors. A default that sends the message to the sender is a safe, minimal fallback that preserves runtime stability. The PR's docstring mentions respecting vanilla semantics; however those policies (styling, suppression) can be applied in the core implementations while the interface provides a safe default.
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** paper-api/src/main/java/io/papermc/paper/command/brigadier/CommandSourceStack.java
**Line:** 102:102
**Comment:**
*Possible Bug: Similarly, introducing `sendFailure(ComponentLike)` as an abstract method on this existing interface may break older `CommandSourceStack` implementations at runtime with `AbstractMethodError` when commands call it, so it should provide a default implementation (e.g. sending the message to the sender) to keep the API binary-compatible.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
|
CodeAnt AI finished reviewing your PR. |
CodeAnt-AI Description
Add vanilla-style command feedback helpers and update tests to use them
What Changed
Impact
✅ Clearer command messages for players✅ Fewer unexpected admin notifications (respects game rules)✅ Easier testing of command feedback behavior💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.