GH-345 Fix enforce region knockback for mounted combat players#345
GH-345 Fix enforce region knockback for mounted combat players#345
Conversation
vLuckyyy
commented
Feb 16, 2026
vLuckyyy
commented
Summary of ChangesHello @vLuckyyy, 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 region-based knockback system by specifically addressing scenarios involving mounted players in combat. It ensures that players engaged in combat cannot bypass region restrictions by riding vehicles, providing a more consistent and robust enforcement of game mechanics. 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. Changelog
Activity
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
|
vLuckyyy
changed the title
vLuckyyy
changed the title
![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 a fix to enforce region knockback for players in mounted combat, handling VehicleMoveEvent and EntityMountEvent to dismount and knock back players in protected regions, and preventing mounting within them. While this is a positive step, the current implementation has security gaps as it only handles VehicleMoveEvent, missing many rideable entities (e.g., horses, other players) that do not implement the Vehicle interface, potentially allowing bypasses. It also doesn't fully account for entity teleportation, which could enable further circumvention of combat region boundaries. Additionally, there is an opportunity to improve maintainability by addressing some code duplication.
| void onVehicleMove(VehicleMoveEvent event) { | ||
| Location locationTo = event.getTo(); | ||
| Location locationFrom = event.getFrom(); | ||
|
|
||
| if (locationTo.getBlockX() == locationFrom.getBlockX() | ||
| && locationTo.getBlockY() == locationFrom.getBlockY() | ||
| && locationTo.getBlockZ() == locationFrom.getBlockZ()) { | ||
| return; | ||
| } | ||
|
|
||
| Optional<Region> regionOptional = this.regionProvider.getRegion(locationTo); | ||
| if (regionOptional.isEmpty()) { | ||
| return; | ||
| } | ||
|
|
||
| Region region = regionOptional.get(); | ||
| for (Entity passenger : event.getVehicle().getPassengers()) { | ||
| if (!(passenger instanceof Player player)) { | ||
| continue; | ||
| } | ||
|
|
||
| if (!this.fightManager.isInCombat(player.getUniqueId())) { | ||
| continue; | ||
| } | ||
|
|
||
| player.leaveVehicle(); | ||
| if (region.contains(locationFrom)) { | ||
| this.knockbackService.knockback(region, player); | ||
| this.knockbackService.forceKnockbackLater(player, region); | ||
| } else { | ||
| this.knockbackService.knockbackLater(region, player, Duration.ofMillis(50)); | ||
| } | ||
|
|
||
| this.noticeService.create() | ||
| .player(player.getUniqueId()) | ||
| .notice(config -> config.messagesSettings.cantEnterOnRegion) | ||
| .send(); | ||
| } | ||
| } |
There was a problem hiding this comment.
The onVehicleMove listener has a security gap: it only handles entities implementing the org.bukkit.entity.Vehicle interface (e.g., Boats, Minecarts), missing many other rideable entities like Horses, Llamas, Pigs, or other Players that do not trigger VehicleMoveEvent. This allows players in combat to bypass region restrictions. To fix this, consider checking for passengers in the onPlayerMove listener or implementing a more comprehensive movement check for all rideable entities.
Furthermore, the logic within this onVehicleMove event handler, especially inside the for loop, is very similar to the onPlayerMove handler, introducing code duplication. Extracting the common logic for applying knockback and sending notifications into a private helper method would improve maintainability and reduce redundancy.
| void onVehicleMove(VehicleMoveEvent event) { | ||
| Location locationTo = event.getTo(); | ||
| Location locationFrom = event.getFrom(); | ||
|
|
||
| if (locationTo.getBlockX() == locationFrom.getBlockX() | ||
| && locationTo.getBlockY() == locationFrom.getBlockY() | ||
| && locationTo.getBlockZ() == locationFrom.getBlockZ()) { | ||
| return; | ||
| } | ||
|
|
||
| Optional<Region> regionOptional = this.regionProvider.getRegion(locationTo); | ||
| if (regionOptional.isEmpty()) { | ||
| return; | ||
| } | ||
|
|
||
| Region region = regionOptional.get(); | ||
| for (Entity passenger : event.getVehicle().getPassengers()) { | ||
| if (!(passenger instanceof Player player)) { | ||
| continue; | ||
| } | ||
|
|
||
| if (!this.fightManager.isInCombat(player.getUniqueId())) { | ||
| continue; | ||
| } | ||
|
|
||
| player.leaveVehicle(); | ||
| if (region.contains(locationFrom)) { | ||
| this.knockbackService.knockback(region, player); | ||
| this.knockbackService.forceKnockbackLater(player, region); | ||
| } else { | ||
| this.knockbackService.knockbackLater(region, player, Duration.ofMillis(50)); | ||
| } | ||
|
|
||
| this.noticeService.create() | ||
| .player(player.getUniqueId()) | ||
| .notice(config -> config.messagesSettings.cantEnterOnRegion) | ||
| .send(); | ||
| } | ||
| } | ||
|
|
||
| @EventHandler(ignoreCancelled = true, priority = EventPriority.HIGHEST) | ||
| void onEntityMount(EntityMountEvent event) { | ||
| if (!(event.getEntity() instanceof Player player)) { | ||
| return; | ||
| } | ||
|
|
||
| if (!this.fightManager.isInCombat(player.getUniqueId())) { | ||
| return; | ||
| } | ||
|
|
||
| if (!this.regionProvider.isInRegion(event.getMount().getLocation())) { | ||
| return; | ||
| } | ||
|
|
||
| event.setCancelled(true); | ||
| this.noticeService.create() | ||
| .player(player.getUniqueId()) | ||
| .notice(config -> config.messagesSettings.cantEnterOnRegion) | ||
| .send(); | ||
| } |
There was a problem hiding this comment.
The current implementation lacks handling for EntityTeleportEvent and VehicleTeleportEvent. If an entity (like a horse or a boat) carrying a player in combat is teleported into a restricted region (e.g., via a portal, a command, or another plugin's feature), the player will enter the region without triggering the knockback logic. This allows players to bypass combat region restrictions. It is recommended to implement listeners for these teleportation events to ensure passengers in combat are properly knocked back when entering restricted regions.
1 participant
