GH-202 V2.0/config refactor #202
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe updates focus on standardizing naming conventions and improving in-code documentation across several components of the plugin. Key configuration settings have been renamed for greater clarity, such as switching from names like 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 review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/BridgeService.java (1)
33-33: Update the parameter name for consistencyThe parameter name
blockedRegionsis still using the old naming style. Consider updating it to match the new naming pattern.- () -> this.regionProvider = new WorldGuardRegionProvider(this.pluginConfig.settings.blockedRegions, this.pluginConfig), + () -> this.regionProvider = new WorldGuardRegionProvider(this.pluginConfig.settings.restrictedRegions, this.pluginConfig),eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/RegionController.java (1)
29-71: Consider simplifying the move handlerA few friendly suggestions to make this code even better:
- The coordinate comparison could use a simpler check like
!locationTo.equals(locationFrom)- The knockback calculation could be moved to a helper method for better organization
Here's how you could refactor it:
+ private Vector calculateKnockback(Player player, Region region) { + Location centerOfRegion = region.getCenter(); + Location subtract = player.getLocation().subtract(centerOfRegion); + Vector knockbackVector = new Vector(subtract.getX(), 0, subtract.getZ()).normalize(); + Vector configuredVector = new Vector( + this.pluginConfig.settings.regionKnockbackMultiplier, + 0.5, + this.pluginConfig.settings.regionKnockbackMultiplier); + return knockbackVector.multiply(configuredVector); + } @EventHandler void onPlayerMove(PlayerMoveEvent event) { Player player = event.getPlayer(); if (!this.fightManager.isInCombat(player.getUniqueId())) { return; } - Location locationTo = event.getTo(); - int xTo = locationTo.getBlockX(); - int yTo = locationTo.getBlockY(); - int zTo = locationTo.getBlockZ(); - - Location locationFrom = event.getFrom(); - int xFrom = locationFrom.getBlockX(); - int yFrom = locationFrom.getBlockY(); - int zFrom = locationFrom.getBlockZ(); - - if (xTo != xFrom || yTo != yFrom || zTo != zFrom) { + if (!event.getTo().equals(event.getFrom())) { Optional<Region> regionOptional = this.regionProvider.getRegion(event.getTo()); if (regionOptional.isEmpty()) { return; } Region region = regionOptional.get(); - Location centerOfRegion = region.getCenter(); - Location subtract = player.getLocation().subtract(centerOfRegion); - - Vector knockbackVector = new Vector(subtract.getX(), 0, subtract.getZ()).normalize(); - Vector configuredVector = new Vector( - this.pluginConfig.settings.regionKnockbackMultiplier, - 0.5, - this.pluginConfig.settings.regionKnockbackMultiplier); - - player.setVelocity(knockbackVector.multiply(configuredVector)); + player.setVelocity(calculateKnockback(player, region)); this.announcer.create() .player(player.getUniqueId()) .notice(this.pluginConfig.messages.cantEnterOnRegion) .send(); } }eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/controller/FightTagController.java (1)
66-76: Consider extracting flight disable logicThe code for disabling flight is repeated for both players. We could make this cleaner by moving it to a helper method.
Here's a simpler way to write this:
+ private void disablePlayerFlight(Player player) { + if (player.isFlying()) { + player.setFlying(false); + player.setAllowFlight(false); + } + } + if (this.config.settings.disableFlying) { - if (attackedPlayerByPerson.isFlying()) { - attackedPlayerByPerson.setFlying(false); - attackedPlayerByPerson.setAllowFlight(false); - } - - if (attacker.isFlying()) { - attacker.setFlying(false); - attacker.setAllowFlight(false); - } + this.disablePlayerFlight(attackedPlayerByPerson); + this.disablePlayerFlight(attacker); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/BridgeService.java(1 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java(2 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/FightTagCommand.java(2 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/controller/FightActionBlockerController.java(8 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/controller/FightTagController.java(6 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/controller/FightUnTagController.java(1 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/RegionController.java(1 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/WorldGuardRegionProvider.java(1 hunks)eternalcombat-plugin/src/main/java/com/eternalcode/combat/updater/UpdaterNotificationController.java(1 hunks)
🔇 Additional comments (17)
eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/controller/FightUnTagController.java (1)
37-37: Nice improvement to the config property name! 👍The new name
releaseAttackerOnVictimDeathmakes it super clear what this setting does. It's a good match with the other naming updates across the project.eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/FightTagCommand.java (2)
56-56: Nice rename!The new name
combatTimerDurationis clearer than the oldcombatDuration. It better explains that this setting controls how long the combat timer lasts.
79-79: Good consistency!You've used the same improved name here too. Keeping names consistent helps everyone understand the code better.
eternalcombat-plugin/src/main/java/com/eternalcode/combat/bridge/BridgeService.java (1)
35-35: LGTM! Good naming updateThe change from
blockedRegionRadiustorestrictedRegionRadiusmakes the config naming more consistent.eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/controller/FightActionBlockerController.java (4)
36-36: Nice work on making the block placing code clearer!The renamed properties make it much easier to understand what's happening. The changes from
shouldPreventBlockPlacingtodisableBlockPlacingandspecificBlocksToPreventPlacingtorestrictedBlockTypesare more straightforward.Also applies to: 50-50, 59-60, 73-74
81-83: The block placement mode check looks good!The logic is clear and the new property names
blockPlacementModeandblockPlacementYCoordinateare easy to understand.
88-88: Great consistency in the disable flags!All the action-blocking flags now follow the same pattern with
disableprefix:
disableElytraUsagedisableFlyingdisableElytraOnDamagedisableInventoryAccessThis makes the code more predictable and easier to read.
Also applies to: 108-108, 128-128, 144-144
175-175: Command restriction changes look good!The switch from blocked commands to restricted commands is a nice improvement in clarity:
restrictedCommandsinstead ofblockedCommandscommandRestrictionModeinstead ofcommandBlockingModeAlso applies to: 178-178
eternalcombat-plugin/src/main/java/com/eternalcode/combat/updater/UpdaterNotificationController.java (1)
36-36: LGTM! Property name updated correctly.The configuration property has been updated to use the new name
notifyAboutUpdates.eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/WorldGuardRegionProvider.java (1)
51-51: LGTM! Property name updated correctly.The configuration property has been updated to use the new name
preventPvpInRegions.eternalcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java (3)
23-26: Documentation improvements look great!The comments are now more descriptive and provide better context for each configuration section.
Also applies to: 29-33, 36-40, 43-47
56-56: Variable names are now more intuitive.The renamed variables better describe their purpose:
notifyAboutUpdatesinstead ofshouldReceivePluginUpdatescombatTimerDurationinstead ofcombatDurationpreventPvpInRegionsinstead ofshouldPreventPvpRegions
And many more similar improvements.Also applies to: 62-62, 68-68, 83-83, 89-89, 95-95, 101-101, 108-108, 115-115, 121-121, 127-127, 137-137, 149-149, 155-155, 161-161, 167-167, 173-173, 179-179, 191-191, 197-197, 203-203, 210-210, 222-222
258-404: Message documentation is now more comprehensive.Each message now has clear documentation explaining:
- When the message is displayed
- What placeholders are available
- The purpose of the message
eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/RegionController.java (1)
58-62: Nice naming improvement!The change from
blockedRegionKnockMultipliertoregionKnockbackMultipliermakes the config setting clearer and easier to understand.eternalcombat-plugin/src/main/java/com/eternalcode/combat/fight/controller/FightTagController.java (3)
38-38: Nice work on the clearer config names!The new names like
ignoredProjectileTypesandcombatTimerDurationmake it easier to understand what these settings do.Also applies to: 54-54
84-84: Good improvements to the damage logging names!The renamed settings like
enableDamageCauseLoggingandloggedDamageCausesbetter explain their purpose.Also applies to: 104-105
134-134: Great choice of words for the exclusion settings!Using terms like
ignoredWorldsandexcludeAdminsFromCombatmakes the settings more intuitive.Also applies to: 138-138, 142-142
...alcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java
Outdated
Show resolved
Hide resolved
...alcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java
Show resolved
Hide resolved
...alcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java
Show resolved
Hide resolved
...alcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java
Outdated
Show resolved
Hide resolved
...alcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java
Outdated
Show resolved
Hide resolved
eternalcombat-plugin/src/main/java/com/eternalcode/combat/region/RegionController.java
Show resolved
Hide resolved
...alcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java
Show resolved
Hide resolved
...alcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java
Outdated
Show resolved
Hide resolved
…ig/implementation/PluginConfig.java Co-authored-by: Jakub Kędziora <[email protected]>
CitralFlo
left a comment
CitralFlo
left a comment
There was a problem hiding this comment.
We are planning on creating more features in the future. I would consider splitting Configuration file just like in EternalCore.
...alcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java
Show resolved
Hide resolved
...alcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java
Show resolved
Hide resolved
...alcombat-plugin/src/main/java/com/eternalcode/combat/config/implementation/PluginConfig.java
Outdated
Show resolved
Hide resolved
|
|
No description provided.