-
-
Notifications
You must be signed in to change notification settings - Fork 20
GH-993 Add freeze and unfreeze player commands #1128
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Introduces FreezeCommand and UnFreezeCommand for freezing and unfreezing players, including support for self and other actions. Adds localized freeze messages in English and Polish, and integrates freeze message sections into the translation system. Took 47 minutes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces freeze
and unfreeze
commands, which is a great addition. The implementation is solid, with separate logic for self-action and targeting other players, including permission checks and localized messages.
My review includes a few suggestions to improve maintainability and adhere to standard Java conventions. Specifically, I've pointed out areas with duplicated code in both FreezeCommand
and UnFreezeCommand
that could be refactored into helper methods. I've also noted a class naming convention issue in UnFreezeCommand
. Addressing these points will make the code cleaner and easier to manage in the future.
eternalcore-core/src/main/java/com/eternalcode/core/feature/freeze/FreezeCommand.java
Outdated
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/freeze/UnFreezeCommand.java
Outdated
Show resolved
Hide resolved
eternalcore-core/src/main/java/com/eternalcode/core/feature/freeze/UnFreezeCommand.java
Show resolved
Hide resolved
Introduced FreezeService interface and its implementation to centralize freeze/unfreeze logic. Added PlayerFreezeEvent and PlayerUnfreezeEvent for event handling. Refactored FreezeCommand and UnfreezeCommand to use FreezeServiceImpl, improving code modularity and event support. Renamed UnFreezeCommand to UnfreezeCommand for consistency. Took 33 minutes
eternalcore-core/src/main/java/com/eternalcode/core/feature/freeze/FreezeServiceImpl.java
Outdated
Show resolved
Hide resolved
Updated UnfreezeCommand to use freezeServiceImpl.unfreezePlayer instead of freezePlayer with Duration.ZERO for unfreezing logic. This improves code clarity and better reflects the intended operation. Took 2 minutes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking gucci
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolve conflicts
Deleted FreezeService, FreezeServiceImpl, and related PlayerFreezeEvent and PlayerUnfreezeEvent classes. Refactored FreezeCommand and UnfreezeCommand to directly manipulate player freeze ticks, simplifying the freeze feature and removing event-based architecture. Took 6 minutes
For now, this is how it works. I'll add a few minor tweaks, but please share your feedback and suggestions :)
I think splitting the system into two separate commands would be a better approach than creating two subcommands,
/freeze freeze
and/freeze unfreeze
.Also, instead of following the class @vLuckyyy sent, I replaced the
CommandSender
class withPlayer
to make it easier to send personalized messages for the performer and target.